Issue 4549 - Implement the Windows Store API to upload development builds

sitescripts/extensions/bin/createNightlies.py

 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-2016 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,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
@@ skipping 24 matching lines @@
 import shutil
 import struct
 import subprocess
 import sys
 import tempfile
 import time
 from urllib import urlencode
 import urllib2
 import urlparse
 import zipfile
+import contextlib
 
 from xml.dom.minidom import parse as parseXml
 
 from sitescripts.extensions.utils import (
     compareVersions, Configuration,
     writeAndroidUpdateManifest
 )
 from sitescripts.utils import get_config, get_template
 
 MAX_BUILDS = 50
+
+
+# Google and Microsoft APIs use HTTP error codes with error message in
+# body. So we add the response body to the HTTPError to get more
+# meaningful error messages.
+class HTTPErrorBodyHandler(urllib2.HTTPDefaultErrorHandler):
+    def http_error_default(self, req, fp, code, msg, hdrs):
+        raise urllib2.HTTPError(req.get_full_url(), code,
+                                '{}\n{}'.format(msg, fp.read()), hdrs, fp)
 
 
 class NightlyBuild(object):
     """
       Performs the build process for an extension,
       generating changelogs and documentation.
     """
 
     def __init__(self, config):
         """
@@ skipping 399 matching lines @@
 
         try:
             urllib2.urlopen(request).close()
         except urllib2.HTTPError as e:
             try:
                 logging.error(e.read())
             finally:
                 e.close()
             raise
 
-    # Google and Microsoft APIs use HTTP error codes with error message in
-    # body. So we add the response body to the HTTPError to get more
-    # meaningful error messages.
-
-    class HTTPErrorBodyHandler(urllib2.HTTPDefaultErrorHandler):

[Sebastian Noack, 2017/02/15 11:20:04]

I'd put this class on the top-level rather than nesting it inside the
NigthlyBuild class.

[Oleksandr, 2017/02/16 07:13:12]

Done.

-        def http_error_default(self, req, fp, code, msg, hdrs):
-            raise urllib2.HTTPError(req.get_full_url(), code,
-                                    '{}\n{}'.format(msg, fp.read()), hdrs, fp)
-
     def uploadToChromeWebStore(self):
 
-        opener = urllib2.build_opener(NightlyBuild.HTTPErrorBodyHandler)
+        opener = urllib2.build_opener(HTTPErrorBodyHandler)
 
         # use refresh token to obtain a valid access token
         # https://developers.google.com/accounts/docs/OAuth2WebServer#refresh
 
         response = json.load(opener.open(
             'https://accounts.google.com/o/oauth2/token',
 
             urlencode([
                 ('refresh_token', self.config.refreshToken),
                 ('client_id', self.config.clientID),
@@ skipping 35 matching lines @@
         request.add_header('Authorization', auth_token)
         request.add_header('x-goog-api-version', '2')
         request.add_header('Content-Length', '0')
 
         response = json.load(opener.open(request))
 
         if any(status not in ('OK', 'ITEM_PENDING_REVIEW') for status in response['status']):
             raise Exception({'status': response['status'], 'statusDetail': response['statusDetail']})
 
     def get_windows_store_access_token(self):
-

[Sebastian Noack, 2017/02/15 11:20:04]

Nit: we don't add blank lines at the top of any block.

[Oleksandr, 2017/02/16 07:13:12]

Done.

-        auth_token = ''
         # use refresh token to obtain a valid access token
         # https://docs.microsoft.com/en-us/azure/active-directory/active-directory-protocols-oauth-code#refreshing-the-access-tokens
         server = 'https://login.microsoftonline.com'
         token_path = '{}/{}/oauth2/token'.format(server, self.config.tenantID)
 
-        opener = urllib2.build_opener(NightlyBuild.HTTPErrorBodyHandler)
+        opener = urllib2.build_opener(HTTPErrorBodyHandler)
         post_data = urlencode([
             ('refresh_token', self.config.refreshToken),
             ('client_id', self.config.clientID),
             ('client_secret', self.config.clientSecret),
             ('grant_type', 'refresh_token'),
             ('resource', 'https://graph.windows.net')
         ])
         request = urllib2.Request(token_path, post_data)
-        response = json.load(opener.open(request))

[Sebastian Noack, 2017/02/15 11:20:04]

The response has to be closed, and this time for real, as with urllib2 each
request/response uses a separate connection.

  with contextlib.closing(opener.open(request)) as response:
    data = json.load(response)

[Oleksandr, 2017/02/16 07:13:11]

Done.

-        auth_token = '{0[token_type]} {0[access_token]}'.format(response)
+        with contextlib.closing(opener.open(request)) as response:
+            data = json.load(response)
+            auth_token = '{0[token_type]} {0[access_token]}'.format(data)
 
         return auth_token
 
     def upload_appx_file_to_windows_store(self, file_upload_url):
-
         # Add .appx file to a .zip file
         zip_path = os.path.splitext(self.path)[0] + '.zip'
         with zipfile.ZipFile(zip_path, 'w', zipfile.ZIP_DEFLATED) as zf:
             zf.write(self.path, os.path.basename(self.path))
 
         # Upload that .zip file
         file_upload_url = file_upload_url.replace('+', '%2B')
         request = urllib2.Request(file_upload_url)
         request.get_method = lambda: 'PUT'
         request.add_header('x-ms-blob-type', 'BlockBlob')
 
-        opener = urllib2.build_opener(NightlyBuild.HTTPErrorBodyHandler)
+        opener = urllib2.build_opener(HTTPErrorBodyHandler)
 
         with open(zip_path, 'rb') as file:
-
             request.add_header('Content-Length',
                                os.fstat(file.fileno()).st_size - file.tell())
             request.add_data(file)
-
-            opener.open(request)

[Sebastian Noack, 2017/02/15 11:20:04]

Please don't forget closing the response.

-
+            opener.open(request).close()
+
+    # Clone the previous submission for the new one. Largely based on code
+    # from https://msdn.microsoft.com/en-us/windows/uwp/monetize/python-code-examples-for-the-windows-store-submission-api#create-an-app-submission
     def upload_to_windows_store(self):
-
-        auth_token = self.get_windows_store_access_token()
-
-        opener = urllib2.build_opener(NightlyBuild.HTTPErrorBodyHandler)
-
-        # Clone the previous submission for the new one. Largely based on code
-        # from https://msdn.microsoft.com/en-us/windows/uwp/monetize/python-code-examples-for-the-windows-store-submission-api#create-an-app-submission
-        headers = {'Authorization': auth_token,
+        opener = urllib2.build_opener(HTTPErrorBodyHandler)
+
+        headers = {'Authorization': self.get_windows_store_access_token(),
                    'Content-type': 'application/json'}
-
-        api_server = 'https://manage.devcenter.microsoft.com'

[Sebastian Noack, 2017/02/15 11:20:04]

Since you only use this variable once, perhaps inline it below?

 
         # Get application
         # https://docs.microsoft.com/en-us/windows/uwp/monetize/get-an-app
         api_path = '{}/v1.0/my/applications/{}'.format(
-            api_server,
+            'https://manage.devcenter.microsoft.com',
             self.config.devbuildGalleryID
         )
 
         request = urllib2.Request(api_path, None, headers)
-        app_obj = json.load(opener.open(request))

[Sebastian Noack, 2017/02/15 11:20:05]

Again, please make sure to close the response.

+        with contextlib.closing(opener.open(request)) as response:
+            app_obj = json.load(response)
 
         # Delete existing in-progress submission
         # https://docs.microsoft.com/en-us/windows/uwp/monetize/delete-an-app-submission
         submissions_path = api_path + '/submissions'
         if 'pendingApplicationSubmission' in app_obj:
             remove_id = app_obj['pendingApplicationSubmission']['id']
             remove_path = '{}/{}'.format(submissions_path, remove_id)
             request = urllib2.Request(remove_path, '', headers)
             request.get_method = lambda: 'DELETE'
-            opener.open(request)
+            opener.open(request).close()
 
         # Create submission
         # https://msdn.microsoft.com/en-us/windows/uwp/monetize/create-an-app-submission
         request = urllib2.Request(submissions_path, '', headers)
         request.get_method = lambda: 'POST'
-        submission = json.load(opener.open(request))

[Sebastian Noack, 2017/02/15 11:20:04]

Again, please make sure to close the response.

[Oleksandr, 2017/02/16 07:13:11]

Done.

+        with contextlib.closing(opener.open(request)) as response:
+            submission = json.load(response)
 
         submission_id = submission['id']
         file_upload_url = submission['fileUploadUrl']
 
-        # Update submission
-        old_submission = submission['applicationPackages'][0]
-        old_submission['fileStatus'] = 'PendingDelete'
-        submission['applicationPackages'].append(

[Sebastian Noack, 2017/02/15 11:20:04]

Perhaps cache submission['applicationPackages'] into avariable above. Then you
don't need to wrap this line, and avoid some small repetition as well.

-            {'fileStatus': 'PendingUpload'})
-        added_submission = submission['applicationPackages'][1]
-        added_submission['fileName'] = os.path.basename(self.path)
-
-        old_min_sys_ram = old_submission['minimumSystemRam']
-        added_submission['minimumSystemRam'] = old_min_sys_ram
-
-        old_directx_version = old_submission['minimumDirectXVersion']
-        added_submission['minimumDirectXVersion'] = old_directx_version

[Sebastian Noack, 2017/02/15 11:20:05]

Why do we modify the added_submission dictionary anyway, it seems it isn't used
for anything after this line?

[Oleksandr, 2017/02/16 07:13:10]

Interesting. Initially it was required to modify the submission if we were going
to upload a new file. Here I accidentally missed the submission of the updated
submission and it still works as expected. I am removing the whole update
submission logic then. Thanks for noticing!

-
-        new_submission_path = '{}/{}'.format(
-            submissions_path, submission_id)
+        new_submission_path = '{}/{}'.format(submissions_path,
+                                             submission_id)
 
         request = urllib2.Request(new_submission_path, None, headers)
-        opener.open(request)
+        opener.open(request).close()
 
         self.upload_appx_file_to_windows_store(file_upload_url)
 
         # Commit submission
         # https://msdn.microsoft.com/en-us/windows/uwp/monetize/commit-an-app-submission
         commit_path = '{}/commit'.format(new_submission_path)
         request = urllib2.Request(commit_path, '', headers)
         request.get_method = lambda: 'POST'
-        submission = json.load(opener.open(request))

[Sebastian Noack, 2017/02/15 11:20:04]

Again, please make sure to close the response.

[Oleksandr, 2017/02/16 07:13:11]

Done.

+        with contextlib.closing(opener.open(request)) as response:
+            submission = json.load(response)
 
         if submission['status'] != 'CommitStarted':
             raise Exception({'status': submission['status'],
                              'statusDetails': submission['statusDetails']})
 
     def run(self):
         """
           Run the nightly build process for one extension
         """
         try:
@@ skipping 78 matching lines @@
         except Exception as ex:
             logging.error('The build for %s failed:', repo)
             logging.exception(ex)
 
     file = open(nightlyConfigFile, 'wb')
     nightlyConfig.write(file)
 
 
 if __name__ == '__main__':
     main()