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 66 matching lines @@
     '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."""
-    return '{}/{}/{}?{}'.format(CROWDIN_AP_URL,

[tlucas, 2017/09/28 21:33:58]

as discussed: simpler building of the desired url

[Sebastian Noack, 2017/09/28 22:18:37]

Is this even worth a separate function now? I would rather just inline this
expression in crowdin_request().

[tlucas, 2017/09/29 09:09:37]

I agree, no extra function necessary - Done.

-                                urllib.quote(project_name),
-                                urllib.quote(action),
-                                urllib.urlencode(dict(get, key=key, json=1)))
-
-
 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."""
     request = urllib2.Request(
-        crowdin_url(project_name, action, key, get),
+        '{}/{}/{}?{}'.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()))
 
@@ skipping 219 matching lines @@
     params = urllib.urlencode([('languages[]', locale) for locale in locales])
 
     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 name, data in files:
-        body += ('--{boundary}\r\n'
-                 'Content-Disposition: form-data; name="files[{name}]"; '
-                 'filename="{name}"\r\n'

[Sebastian Noack, 2017/09/28 22:18:37]

Nit: You can avoid wrapping before the end of line (as well as wrapping the
format argument) if you format this code like that:

  body += (
      '...'
      '...'
  ).format(...)

[tlucas, 2017/09/29 09:09:37]

Done.

-                 'Content-Type: application/octet-stream; charset=utf-8\r\n'

[tlucas, 2017/09/28 21:33:58]

as discussed: charset in each file's individual Content-Type

[Sebastian Noack, 2017/09/28 22:18:37]

This should be "application/json", I guess?

[Sebastian Noack, 2017/09/28 22:51:55]

Or even better, you could dynamically detect the type based on the file
extension using mimetypes.guess_type(). It will always be JSON anyway, but it if
we don't have to hardcode it that is better.

[tlucas, 2017/09/29 09:09:37]

Good point - Done.

-                 'Content-Transfer-Encoding: binary\r\n'
-                 '\r\n{data}\r\n'
-                 '--{boundary}--\r\n').format(boundary=boundary, name=name,
-                                              data=data)
+        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')
     return (
         StringIO(body),
         {
             'Content-Type': ('multipart/form-data; boundary=' + boundary),
             'Content-Length': len(body)
         },
     )
 
@@ skipping 20 matching lines @@
                 newName = file + '.json'
 
             if data:
                 if newName in existing:
                     update.append((newName, data))
                     existing.remove(newName)
                 else:
                     add.append((newName, data))
 
     if len(add):
         query = {'titles[{}]'.format(name): os.path.splitext(name)[0]

[tlucas, 2017/09/28 21:33:58]

os.path.splitext as discussed

                  for name, _ in add}
         query['type'] = 'chrome'
         data, headers = crowdin_prepare_upload(add)
         crowdin_request(projectName, 'add-file', key, query, post_data=data,

[tlucas, 2017/09/28 21:33:58]

Hasn't benn noticed before: the query was overwritten by the post data.

                         headers=headers)
     if len(update):
         data, headers = crowdin_prepare_upload(update)
         crowdin_request(projectName, 'update-file', key, post_data=data,
                         headers=headers)
     for file in existing:
         crowdin_request(projectName, 'delete-file', key, {'file': file})
 
 
 def uploadTranslations(localeConfig, metadata, dir, locale, projectName, key):
@@ skipping 92 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)