Issue 5777 - Update crowdin interface

localeTools.py

 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import re
 import os
 import sys
 import codecs
 import json
 import urlparse
 import urllib
 import urllib2
+import mimetypes
 from StringIO import StringIO
 from ConfigParser import SafeConfigParser
 from zipfile import ZipFile
 from xml.parsers.expat import ParserCreate, XML_PARAM_ENTITY_PARSING_ALWAYS
 
 langMappingGecko = {
     'bn-BD': 'bn',
     'br': 'br-FR',
     'dsb': 'dsb-DE',
     'fj-FJ': 'fj',
@@ skipping 63 matching lines @@
     'ta',
     'te',
     'th',
     'tr',
     'uk',
     'vi',
     'zh-CN',
     'zh-TW',
 ]
 
-CROWDIN_AP_URL = 'https://api.crowdin.com/api/project/{}/{}'
-
-
-def crowdin_url(project_name, action, key, get={}):
-    """Create a valid url for a crowdin endpoint."""
-    url = CROWDIN_AP_URL.format(project_name, action)
-    get['key'] = key
-
-    scheme, netloc, path, params, query, fragment = urlparse.urlparse(url)
-
-    query = dict(urlparse.parse_qsl(query))
-    query.update(get)
-
-    return urlparse.urlunparse((
-        scheme, netloc, path, params, urllib.urlencode(query), fragment
-    ))
-
-
-def crowdin_post(post_data, project_name, action, key, get={}, raises=False):
+CROWDIN_AP_URL = 'https://api.crowdin.com/api/project'
+
+
+def crowdin_request(project_name, action, key, get={}, post_data=None,
+                    headers={}, raw=False):
     """Perform a call to crowdin and raise an Exception on failure."""
-
-    result = urllib2.urlopen(crowdin_url(project_name, action, key, get),
-                             data=post_data).read()
-
-    if raises and result.find('<success') < 0:
-        raise Exception(
-            'Server indicated that the operation was not successful\n' + result
-        )
+    request = urllib2.Request(
+        '{}/{}/{}?{}'.format(CROWDIN_AP_URL,
+                             urllib.quote(project_name),
+                             urllib.quote(action),
+                             urllib.urlencode(dict(get, key=key, json=1))),
+        post_data,
+        headers,
+    )
+
+    try:
+        result = urllib2.urlopen(request).read()
+    except urllib2.HTTPError as e:
+        raise Exception('Server returned HTTP Error {}:\n{}'.format(e.code,
+                                                                    e.read()))
+
+    if not raw:
+        return json.loads(result)
 
     return result
-
-
-def crowdin_get(*args, **kwargs):

[Vasily Kuznetsov, 2017/09/26 13:11:36]

This get via post with None as a first argument looks kind of sloppy. I mildly
prefer renaming `crowdin_post` to `crowdin_request` and then creating an
additional `crowdin_post` that delegates to it (or is an alias for it). It would
be functionally equivalent to the current code as far as interpreter is
concerned, but would express the intention better for the human readers of the
code.

[tlucas, 2017/09/26 14:40:29]

Done.

-    return crowdin_post(None, *args, **kwargs)
 
 
 class OrderedDict(dict):
     def __init__(self):
         self.__order = []
 
     def __setitem__(self, key, value):
         self.__order.append(key)
         dict.__setitem__(self, key, value)
 
@@ skipping 186 matching lines @@
         for match in re.finditer(r'&lang=([\w\-]+)"', firefoxLocales):
             locales.add(mapLocale('BCP-47', match.group(1)))
         langPacks = urllib2.urlopen('https://addons.mozilla.org/en-US/firefox/language-tools/').read()
         for match in re.finditer(r'<tr>.*?</tr>', langPacks, re.S):
             if match.group(0).find('Install Language Pack') >= 0:
                 match2 = re.search(r'lang="([\w\-]+)"', match.group(0))
                 if match2:
                     locales.add(mapLocale('BCP-47', match2.group(1)))
 
     allowed = set()
-    allowedLocales = json.loads(crowdin_get(projectName, 'supported-languages',

[Vasily Kuznetsov, 2017/09/26 13:11:36]

For extra style points you could check if there's `{'json': 1}` in parameters in
`crowdin_post` and automatically `json.loads` if there is. This would make this
code even leaner. Totally up to you if you want to do it? :D

[tlucas, 2017/09/26 14:40:29]

Nice idea, done.

-                                            key, {'json': 1}))
+    allowedLocales = crowdin_request(projectName, 'supported-languages', key)
 
     for locale in allowedLocales:
         allowed.add(locale['crowdin_code'])
     if not allowed.issuperset(locales):
         print "Warning, following locales aren't allowed by server: " + ', '.join(locales - allowed)
 
     locales = list(locales & allowed)
     locales.sort()
     params = urllib.urlencode([('languages[]', locale) for locale in locales])
 
-    crowdin_post(params, projectName, 'edit-project', key, raises=True)
-
-
-def postFiles(files, url):
+    crowdin_request(projectName, 'edit-project', key, post_data=params)
+
+
+def crowdin_prepare_upload(files):
+    """Create a post body and matching headers, which Crowdin can handle."""
     boundary = '----------ThIs_Is_tHe_bouNdaRY_$'
     body = ''
-    for file, data in files:
-        body += '--%s\r\n' % boundary
-        body += 'Content-Disposition: form-data; name="files[%s]"; filename="%s"\r\n' % (file, file)
-        body += 'Content-Type: application/octet-stream\r\n'
-        body += 'Content-Transfer-Encoding: binary\r\n'
-        body += '\r\n' + data + '\r\n'
-    body += '--%s--\r\n' % boundary
+    for name, data in files:
+        mimetype = mimetypes.guess_type(name)[0]
+        body += (
+            '--{boundary}\r\n'
+            'Content-Disposition: form-data; name="files[{name}]"; '
+            'filename="{name}"\r\n'
+            'Content-Type: {mimetype}; charset=utf-8\r\n'
+            'Content-Transfer-Encoding: binary\r\n'
+            '\r\n{data}\r\n'
+            '--{boundary}--\r\n'
+        ).format(boundary=boundary, name=name, data=data, mimetype=mimetype)
 
     body = body.encode('utf-8')
-    request = urllib2.Request(url, StringIO(body))
-    request.add_header('Content-Type', 'multipart/form-data; boundary=%s' % boundary)
-    request.add_header('Content-Length', len(body))
-    result = urllib2.urlopen(request).read()

[Vasily Kuznetsov, 2017/09/26 13:11:36]

This could be refactored using `crowdin_post` to be more uniform. Would make
this function take a bunch of parameters, which is kind of annoying, but still
worth considering. What do you think?

[tlucas, 2017/09/26 14:40:29]

I refactored this function to only prepare a body / headers and passed the
result to crowdin_post - how about this? (No parameter flood)

-    if result.find('<success') < 0:
-        raise Exception('Server indicated that the operation was not successful\n' + result)
+    return (
+        StringIO(body),
+        {
+            'Content-Type': ('multipart/form-data; boundary=' + boundary),
+            'Content-Length': len(body)
+        },
+    )
 
 
 def updateTranslationMaster(localeConfig, metadata, dir, projectName, key):
-    result = json.loads(crowdin_get(projectName, 'info', key, {'json': 1}))
+    result = crowdin_request(projectName, 'info', key)
 
     existing = set(map(lambda f: f['name'], result['files']))
     add = []
     update = []
     for file in os.listdir(dir):
         path = os.path.join(dir, file)
         if os.path.isfile(path):
             if localeConfig['file_format'] == 'chrome-json' and file.endswith('.json'):
                 data = preprocessChromeLocale(path, metadata, True)
                 newName = file
             elif localeConfig['file_format'] == 'chrome-json':
                 fileHandle = codecs.open(path, 'rb', encoding='utf-8')
                 data = json.dumps({file: {'message': fileHandle.read()}})
                 fileHandle.close()
                 newName = file + '.json'
             else:
                 data = toJSON(path)
                 newName = file + '.json'
 
             if data:
                 if newName in existing:
                     update.append((newName, data))
                     existing.remove(newName)
                 else:
                     add.append((newName, data))
 
     if len(add):
-        data = {'titles[{}]'.format(name): re.sub(r'\.json', '', name)
-                for name, data in add}
-        data['type'] = 'chrome'
-        postFiles(add, crowdin_url(projectName, 'add-file', key, data))
+        query = {'titles[{}]'.format(name): os.path.splitext(name)[0]
+                 for name, _ in add}
+        query['type'] = 'chrome'
+        data, headers = crowdin_prepare_upload(add)
+        crowdin_request(projectName, 'add-file', key, query, post_data=data,
+                        headers=headers)
     if len(update):
-        postFiles(update, crowdin_url(projectName, 'update-file', key))
+        data, headers = crowdin_prepare_upload(update)
+        crowdin_request(projectName, 'update-file', key, post_data=data,
+                        headers=headers)
     for file in existing:
-        result = urllib2.urlopen(

[Vasily Kuznetsov, 2017/09/26 13:11:36]

Isn't this `crowdin_get`?

[tlucas, 2017/09/26 14:40:29]

It is, missed that :) Done.

-            crowdin_url(projectName, 'delete-file', key, {'file': file})
-        ).read()
-        if result.find('<success') < 0:
-            raise Exception('Server indicated that the operation was not successful\n' + result)
+        crowdin_request(projectName, 'delete-file', key, {'file': file})
 
 
 def uploadTranslations(localeConfig, metadata, dir, locale, projectName, key):
     files = []
     for file in os.listdir(dir):
         path = os.path.join(dir, file)
         if os.path.isfile(path):
             if localeConfig['file_format'] == 'chrome-json' and file.endswith('.json'):
                 data = preprocessChromeLocale(path, metadata, False)
                 newName = file
             elif localeConfig['file_format'] == 'chrome-json':
                 fileHandle = codecs.open(path, 'rb', encoding='utf-8')
                 data = json.dumps({file: {'message': fileHandle.read()}})
                 fileHandle.close()
                 newName = file + '.json'
             else:
                 data = toJSON(path)
                 newName = file + '.json'
 
             if data:
                 files.append((newName, data))
     if len(files):
         language = mapLocale(localeConfig['name_format'], locale)
-        url = crowdin_url(projectName, 'upload-translation', key,
-                          {'language': language})
-        postFiles(files, url)
+        data, headers = crowdin_prepare_upload(files)
+        crowdin_request(projectName, 'upload-translation', key,
+                        {'language': language}, post_data=data,
+                        headers=headers)
 
 
 def getTranslations(localeConfig, projectName, key):
-    # let crowdin build the project
-    crowdin_get(projectName, 'export', key, raises=True)
-
-    result = crowdin_get(projectName, 'download/all.zip', key)
+    """Download all available translations from crowdin.
+
+    Trigger crowdin to build the available export, wait for crowdin to
+    finish the job and download the generated zip afterwards.
+    """
+    crowdin_request(projectName, 'export', key)
+
+    result = crowdin_request(projectName, 'download/all.zip', key, raw=True)
     zip = ZipFile(StringIO(result))
     dirs = {}
 
     normalizedDefaultLocale = localeConfig['default_locale']
     if localeConfig['name_format'] == 'ISO-15897':
         normalizedDefaultLocale = normalizedDefaultLocale.replace('_', '-')
     normalizedDefaultLocale = mapLocale(localeConfig['name_format'],
                                         normalizedDefaultLocale)
 
     for info in zip.infolist():
@@ skipping 47 matching lines @@
 
     # Remove any extra files
     for dir, files in dirs.iteritems():
         baseDir = os.path.join(localeConfig['base_path'], dir)
         if not os.path.exists(baseDir):
             continue
         for file in os.listdir(baseDir):
             path = os.path.join(baseDir, file)
             if os.path.isfile(path) and (file.endswith('.json') or file.endswith('.properties') or file.endswith('.dtd')) and not file in files:
                 os.remove(path)