Issue 520 - Generate PAD files for download portals when updating download links

sitescripts/extensions/bin/updateDownloadLinks.py

 # coding: utf-8
 
 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-2014 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
@@ skipping 52 matching lines @@
     return (linkTag.firstChild.data, versionTag.firstChild.data)
   else:
     return (None, None)
 
 def getGoogleDownloadLink(galleryID):
   """
   gets download link for a Chrome add-on from the Chrome Gallery site
   """
   galleryID = urlencode(galleryID)
 
-  response = urlopen('https://clients2.google.com/service/update2/crx?x=' + urlencode('id=%s&uc' % galleryID))

[Wladimir Palant, 2014/05/26 10:54:57]

a) Why use an overly long line instead of a temporary param variable?

b) Why use string concatenation rather than substitution here? I seem to
remember that you were generally opposing string concatenation in Python.

[Sebastian Noack, 2014/05/26 12:04:53]

Agreed.
When only concatenating two strings, string concatenation by plus operator feels
more intuitive and less complex. However when concatenating multiple strings,
string formatting is more efficient and IMO easier to read.

[Wladimir Palant, 2014/05/26 13:26:53]

What we have here is a URL template, substitution seems to be a better choice
then - the string to be replaced doesn't necessarily need to be at the end of
the URL.

[Sebastian Noack, 2014/05/27 12:01:23]

Done.

-  document = dom.parse(response)
+  url = 'https://clients2.google.com/service/update2/crx?x=%s' % urlencode('id=%s&uc' % galleryID)
+  document = dom.parse(urlopen(url))
   updateTags = document.getElementsByTagName('updatecheck')
   version = updateTags and updateTags[0].getAttribute('version')
 
   if not version:
     return (None, None)
 
   request = urllib2.Request('https://chrome.google.com/webstore/detail/_/' + galleryID)
   request.get_method = lambda : 'HEAD'
   url = urllib2.urlopen(request).geturl()
 
@@ skipping 31 matching lines @@
       highestVersion = version
 
   return (highestURL, highestVersion)
 
 def getDownloadLink(repo):
   """
   gets the download link to the most current version of an extension
   """
   # you can't easily install extensions from third-party sources on Chrome
   # and Opera. So always get the link for the version on the Web Store.
-  if repo.type in ("chrome", "opera") and repo.galleryID:

[Wladimir Palant, 2014/05/26 10:54:57]

Isn't the repo.type check redundant here? You check the type below anyway.

[Sebastian Noack, 2014/05/26 12:04:53]

You are right.

+  if repo.galleryID:
     if repo.type == "chrome":
       return getGoogleDownloadLink(repo.galleryID)
-
     if repo.type == "opera":
       return getOperaDownloadLink(repo.galleryID)
 
   (localURL, localVersion) = getLocalLink(repo)
 
   # get a link to Firefox Add-Ons, if the latest version has been published there
   if repo.type == 'gecko' and repo.galleryID:
     (galleryURL, galleryVersion) = getMozillaDownloadLink(repo.galleryID)
     if not localVersion or (galleryVersion and
                             compareVersions(galleryVersion, localVersion) >= 0):
@@ skipping 86 matching lines @@
   if len(extensions['android']) > 1:
     print >>sys.stderr, 'Warning: more than one Android app defined, update manifest only works for one'
 
   for repoType in extensions.iterkeys():
     manifestPath = get_config().get('extensions', '%sUpdateManifestPath' % repoType)
     template = get_template(get_config().get('extensions', '%sUpdateManifest' % repoType))
     template.stream({'extensions': extensions[repoType]}).dump(manifestPath)
 
 def writePadFile(links):
   for repo in Configuration.getRepositoryConfigurations():
-    if repo.padTemplate and links.has_section(repo.repositoryName):
+    if repo.pad and links.has_section(repo.repositoryName):
       PadFile.forRepository(repo, links.get(repo.repositoryName, 'version'),
                                   links.get(repo.repositoryName, 'downloadURL')).write()
 
 def updateLinks():
   """
   writes the current extension download links to a file
   """
 
   # Now get download links and save them to file
   result = SafeConfigParser()
   getDownloadLinks(result)
   file = open(get_config().get('extensions', 'downloadLinksFile'), 'wb')
   result.write(file)
   file.close()
 
   writeUpdateManifest(result)
   writePadFile(result)
 
 if __name__ == "__main__":
   updateLinks()