Issue 3076 - [CMS] Map locale names to match Crowdin's expectations

cms/bin/translate.py

 # coding: utf-8
 
 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-2015 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
 # GNU General Public License for more details.
 #
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
 import collections
 import io
 import itertools
 import json
 import logging
 import os
 import posixpath
+import shutil
 import sys
 import urllib
 import zipfile
 
 import urllib3
 
 import cms.utils
 from cms.sources import FileSource
 
 logger = logging.getLogger("cms.bin.translate")
@@ skipping 75 matching lines @@
       comment += ", ".join("{%d}: %s" % i_s
                            for i_s in enumerate(fixed_strings, 1))
     if comment:
       store[name]["description"] = comment
 
   for page, format in source.list_pages():
     cms.utils.process_page(source, defaultlocale, page,
                            format=format, localized_string_callback=record_string)
   return page_strings
 
-def configure_locales(crowdin_api, required_locales, enabled_locales,
+def configure_locales(crowdin_api, local_locales, enabled_locales,
                       defaultlocale):
   logger.info("Checking which locales are supported by Crowdin...")
   response = crowdin_api.request("GET", "supported-languages")
 
   supported_locales = {l["crowdin_code"] for l in response}
-  skipped_locales = required_locales - supported_locales
+
+  # We need to map the locale names we use to the ones that Crowdin is expecting
+  # and at the same time ensure that they are supported.
+  required_locales = {}
+  skipped_locales = []
+  for locale in local_locales:
+    if "_" in locale:

[Sebastian Noack, 2015/09/16 19:37:37]

This can be simplified:

if "_" in locale:
  crowdin_locale = locale.replace("_", "-")
elif locale in supported_locales:
  crowdin_locale = locale
else:
  crowdin_locale = "%s-%s" % (locale, locale.upper())

[kzar, 2015/09/16 20:02:41]

Yea, I did it kinda like that initially but then it bugged me that if locale ==
crowdin_locale we're doing the "in supported_locales" check twice. I'm not sure
which is better, maybe you're right.

[Sebastian Noack, 2015/09/17 09:45:14]

Complexity-wise you have two |in supported_locales| checks either way. But with
the current implementation you are duplicating |required_locales[locale] =| as
well and have an unnecessary loop control instruction, making the code flow
harder to follow. And performance-wise, set lookups have constant time. Not that
performance matters here anyway.

[kzar, 2015/09/17 10:16:16]

Done.

+      crowdin_locale = locale.replace("_", "-")
+    else:
+      if locale in supported_locales:
+        required_locales[locale] = locale
+        continue
+      crowdin_locale = "%s-%s" % (locale, locale.upper())
+
+    if crowdin_locale in supported_locales:
+      required_locales[locale] = crowdin_locale
+    else:
+      skipped_locales.append(locale)
 
   if skipped_locales:
     logger.warning("Ignoring locales that Crowdin doesn't support: %s",
                    ", ".join(skipped_locales))
-    required_locales -= skipped_locales
 
-  if not required_locales.issubset(enabled_locales):
+  required_crowdin_locales = set(required_locales.values())
+  if not required_crowdin_locales.issubset(enabled_locales):
     logger.info("Enabling the required locales for the Crowdin project...")
     crowdin_api.request(
       "POST", "edit-project",
-      data={"languages": enabled_locales | required_locales}
+      data={"languages": enabled_locales | required_crowdin_locales}
     )
 
   return required_locales
 
 def list_remote_files(project_info):
   def parse_file_node(node, path=""):
     if node["node_type"] == "file":
       remote_files.add(path + node["name"])
     elif node["node_type"] == "directory":
       dir_name = path + node["name"]
@@ skipping 43 matching lines @@
 
 def upload_translations(crowdin_api, source_dir, new_files, required_locales):
   def open_locale_files(locale, files):
     for file_name in files:
       path = os.path.join(source_dir, "locales", locale, file_name)
       if os.path.isfile(path):
         with open(path, "rb") as f:
           yield (file_name, f.read(), "application/json")
 
   if new_files:
-    for locale in required_locales:
+    for locale, crowdin_locale in required_locales.iteritems():
       for files in grouper(open_locale_files(locale, new_files),
                            crowdin_api.FILES_PER_REQUEST):
         logger.info("Uploading %d existing translation "
                     "files for locale %s...", len(files), locale)
         crowdin_api.request("POST", "upload-translation", files=files,
-                            data={"language": locale})
+                            data={"language": crowdin_locale})
 
 def remove_old_files(crowdin_api, old_files):
   for file_name in old_files:
     logger.info("Removing old file %s", file_name)
     crowdin_api.request("POST", "delete-file", data={"file": file_name})
 
 def remove_old_directories(crowdin_api, old_directories):
   for directory in reversed(sorted(old_directories, key=len)):
     logger.info("Removing old directory %s", directory)
     crowdin_api.request("POST", "delete-directory", data={"name": directory})
 
 def download_translations(crowdin_api, source_dir, required_locales):
   logger.info("Requesting generation of fresh translations archive...")
   result = crowdin_api.request("GET", "export")
   if result.get("success", {}).get("status") == "skipped":
     logger.warning("Archive generation skipped, either "
                    "no changes or API usage excessive")
 
   logger.info("Downloading translations archive...")
   response = crowdin_api.raw_request("GET", "download/all.zip")
 
+  inverted_required_locales = {crowdin: local for local, crowdin in
+                               required_locales.iteritems()}
   logger.info("Extracting translations archive...")
   with zipfile.ZipFile(io.BytesIO(response.data), "r") as archive:
     locale_path = os.path.join(source_dir, "locales")
     # First clear existing translation files
     for root, dirs, files in os.walk(locale_path, topdown=True):
       if root == locale_path:
         dirs[:] = [d for d in dirs if d in required_locales]
       for f in files:
         if f.lower().endswith(".json"):
           os.remove(os.path.join(root, f))
     # Then extract the new ones in place
     for member in archive.namelist():
       path, file_name = posixpath.split(member)
       ext = posixpath.splitext(file_name)[1]
-      locale = path.split(posixpath.sep)[0]
-      if ext.lower() == ".json" and locale in required_locales:
-        archive.extract(member, locale_path)
+      path_parts = path.split(posixpath.sep)
+      locale, file_path = path_parts[0], path_parts[1:]
+      if ext.lower() == ".json" and locale in inverted_required_locales:
+        output_path = os.path.join(
+          locale_path, inverted_required_locales[locale],
+          *file_path + [file_name]
+        )
+        with archive.open(member) as f, open(output_path, "wb") as f_target:

[Sebastian Noack, 2015/09/16 19:37:37]

Why not |archive.extract(member, output_path)|?

[kzar, 2015/09/16 20:02:41]

I believe the path parameter to the extract method is just for the base path,
the path of the file inside the archive and the file name is joined to that. So
if we did what you're suggestion wouldn't the path end up being something like
".../locales/es/some_page.json/es-ES/some_page.json"?

[kzar, 2015/09/16 20:10:29]

(I just tested that to make sure, unfortunately my fears where confirmed :/ )

+          shutil.copyfileobj(f, f_target)
 
 def crowdin_sync(source_dir, crowdin_api_key):
   with FileSource(source_dir) as source:
     config = source.read_config()
     defaultlocale = config.get("general", "defaultlocale")
     crowdin_project_name = config.get("general", "crowdin-project-name")
 
     crowdin_api = CrowdinAPI(crowdin_api_key, crowdin_project_name)
 
     logger.info("Requesting project information...")
     project_info = crowdin_api.request("GET", "info")
     page_strings = extract_strings(source, defaultlocale)
 
-    required_locales = {l for l in source.list_locales() if l != defaultlocale}
+    local_locales = {l for l in source.list_locales() if l != defaultlocale}

[Sebastian Noack, 2015/09/16 19:37:37]

Nit: set(source.list_locales()) - {defaultlocale}

[kzar, 2015/09/16 20:02:41]

Done.

     enabled_locales = {l["code"] for l in project_info["languages"]}
 
-  required_locales = configure_locales(crowdin_api, required_locales,
+  required_locales = configure_locales(crowdin_api, local_locales,
                                        enabled_locales, defaultlocale)
 
   remote_files, remote_directories = list_remote_files(project_info)
   local_files, local_directories = list_local_files(page_strings)
 
   # Avoid deleting all remote content if there was a problem listing local files
   if not local_files:
     logger.error("No existing strings found, maybe the project directory is "
                  "not set up correctly? Aborting!")
     sys.exit(1)
@@ skipping 18 matching lines @@
 if __name__ == "__main__":
   if len(sys.argv) < 3:
     print >>sys.stderr, "Usage: python -m cms.bin.translate www_directory crowdin_project_api_key [logging_level]"
     sys.exit(1)
 
   logging.basicConfig()
   logger.setLevel(sys.argv[3] if len(sys.argv) > 3 else logging.INFO)
 
   source_dir, crowdin_api_key = sys.argv[1:3]
   crowdin_sync(source_dir, crowdin_api_key)