Extensions Docs Server: Internationalized samples

chrome/common/extensions/docs/server2/samples_data_source.py

Index: chrome/common/extensions/docs/server2/samples_data_source.py
diff --git a/chrome/common/extensions/docs/server2/samples_data_source.py b/chrome/common/extensions/docs/server2/samples_data_source.py
index c8e7fc4d0387711ede2e859a6181e8c34c64fce4..a346f3b0545ba1c0251df8857d136c150cc87fc2 100644
--- a/chrome/common/extensions/docs/server2/samples_data_source.py
+++ b/chrome/common/extensions/docs/server2/samples_data_source.py
@@ -6,67 +6,130 @@ import json
 import re
 
 class SamplesDataSource(object):
-  """Constructs a list of samples and their respective files and api calls.
+  """ Constructs a list of samples and their respective files and api calls.

[not at google - send to devlin, 2012/07/23 13:25:37]

Heh, I think these leading spaces are just me being silly. Apparently you're not
supposed to be leading spaces in docstrings :\ so I'll stop doing it.

Looks better though. Whatever...

[cduvall, 2012/07/23 18:10:37]

Done.

   """
-  def __init__(self, fetcher, cache_builder, samples_path):
-    self._fetcher = fetcher
-    self._cache = cache_builder.build(self._MakeSamplesList)
-    self._samples_path = samples_path
 
-  def _GetApiItems(self, js_file):
-    return set(re.findall('(chrome\.[a-zA-Z0-9\.]+)', js_file))
+  class Factory(object):
+    """ A factory to create SamplesDataSource instances bound to individual
+    Requests.
+    """
+    def __init__(self, file_system, cache_builder, samples_path):
+      self._file_system = file_system
+      self._cache = cache_builder.build(self._MakeSamplesList)
+      self._samples_path = samples_path
+
+    def Create(self, request):
+      """ Returns a new SamplesDataSource bound to |request|.
+      """
+      return SamplesDataSource(self._cache,
+                               self._samples_path,
+                               request)
 
-  def _MakeApiLink(self, prefix, item):
-    api, name = item.replace('chrome.', '').split('.', 1)
-    return api + '.html#' + prefix + '-' + name
+    def _GetApiItems(self, js_file):
+      return set(re.findall('(chrome\.[a-zA-Z0-9\.]+)', js_file))
 
-  def _GetDataFromManifest(self, path):
-    manifest_path = path + '/manifest.json'
-    manifest = self._fetcher.Read([manifest_path]).Get()[manifest_path]
-    manifest_json = json.loads(manifest)
-    return (manifest_json.get('name'), manifest_json.get('description'))
+    def _MakeApiLink(self, prefix, item):
+      api, name = item.replace('chrome.', '').split('.', 1)
+      return api + '.html#' + prefix + '-' + name
 
-  def _MakeSamplesList(self, files):
-    samples_list = []
-    for filename in sorted(files):
-      if filename.rsplit('/')[-1] != 'manifest.json':
-        continue
-      # This is a little hacky, but it makes a sample page.
-      sample_path = filename.rsplit('/', 1)[-2]
-      sample_files = filter(lambda x: x.startswith(sample_path + '/'), files)
-      js_files = filter(lambda x: x.endswith('.js'), sample_files)
-      js_contents = self._fetcher.Read(js_files).Get()
-      api_items = set()
-      for js in js_contents.values():
-        api_items.update(self._GetApiItems(js))
+    def _GetDataFromManifest(self, path):
+      manifest = self._file_system.ReadSingleFile(path + '/manifest.json')
+      manifest_json = json.loads(manifest)
+      l10n_data = {
+        'name': manifest_json.get('name', ''),
+        'description': manifest_json.get('description', ''),
+        'default_locale': manifest_json.get('default_locale', None),
+        'locales': {}
+      }
+      if not l10n_data['default_locale']:
+        return l10n_data
+      locales_path = path + '/_locales/'
+      locales_dir = self._file_system.ReadSingleFile(locales_path)
+      if locales_dir:
+        locales_files = self._file_system.Read(
+            [locales_path + f + 'messages.json' for f in locales_dir]).Get()
+        locales_json = [(path, json.loads(unicode(contents, 'latin-1')))
+                        for path, contents in locales_files.iteritems()]
+        for path, json_ in locales_json:
+          l10n_data['locales'][path[len(locales_path):].split('/')[0]] = json_
+      return l10n_data
 
-      api_calls = []
-      for item in api_items:
-        if len(item.split('.')) < 3:
+    def _MakeSamplesList(self, files):
+      samples_list = []
+      for filename in sorted(files):
+        if filename.rsplit('/')[-1] != 'manifest.json':
           continue
-        if item.endswith('.addListener'):
-          item = item.replace('.addListener', '')
-          api_calls.append({
-            'name': item,
-            'link': self._MakeApiLink('event', item)
-          })
-        else:
-          api_calls.append({
-            'name': item,
-            'link': self._MakeApiLink('method', item)
-          })
-      name, description = self._GetDataFromManifest(sample_path)
-      samples_list.append({
-        'name': name,
-        'description': description,
-        'path': sample_path.split('/', 1)[1],
-        'files': [f.replace(sample_path + '/', '') for f in sample_files],
-        'api_calls': api_calls
-      })
-    return samples_list
+        # This is a little hacky, but it makes a sample page.
+        sample_path = filename.rsplit('/', 1)[-2]
+        sample_files = filter(lambda x: x.startswith(sample_path + '/'), files)

[not at google - send to devlin, 2012/07/23 13:25:37]

randomly browsing code here, but maybe this should be a list comprehension like

[path for path in files if path.startswith(sample_path + '/')]

[cduvall, 2012/07/23 18:10:37]

Done.

+        js_files = filter(lambda x: x.endswith('.js'), sample_files)
+        js_contents = self._file_system.Read(js_files).Get()
+        api_items = set()
+        for js in js_contents.values():
+          api_items.update(self._GetApiItems(js))
+
+        api_calls = []
+        for item in api_items:
+          if len(item.split('.')) < 3:
+            continue
+          if item.endswith('.addListener'):
+            item = item.replace('.addListener', '')
+            api_calls.append({
+              'name': item,
+              'link': self._MakeApiLink('event', item)
+            })
+          else:
+            api_calls.append({
+              'name': item,
+              'link': self._MakeApiLink('method', item)
+            })
+        l10n_data = self._GetDataFromManifest(sample_path)
+        l10n_data.update({
+          'path': sample_path.split('/', 1)[1],
+          'files': [f.replace(sample_path + '/', '') for f in sample_files],
+          'api_calls': api_calls
+        })
+        samples_list.append(l10n_data)
+
+      return samples_list
+
+  def __init__(self, cache, samples_path, request):
+    self._cache = cache
+    self._samples_path = samples_path
+    self._request = request
+
+  def _GetAcceptedLanguages(self):
+    languages = []
+    if 'Accept-Language' in self._request.headers:
+      languages = filter(lambda x: x != 'q',
+                         re.findall('([a-zA-Z]+\-[a-zA-Z]+|[a-zA-Z]+)',
+                                    self._request.headers['Accept-Language']))

[not at google - send to devlin, 2012/07/23 13:25:37]

Using regex is kind of unnecessary here? And almost certainly not compliant.

I think we want to do something along the lines of

accept_language = self._request.headers.get('Accept-Language', None)
if accept_language is None:
  return []
return [lang_with_q.split(';')[0].strip()
        for lang_with_q in accept_language.split(',')]

[cduvall, 2012/07/23 18:10:37]

Done.

+      languages = [lang.replace('-', '_') for lang in languages]
+    return languages
 
   def __getitem__(self, key):
     return self.get(key)
 
   def get(self, key):
-    return self._cache.GetFromFileListing(self._samples_path + '/')
+    samples_list = self._cache.GetFromFileListing(self._samples_path + '/')
+    return_list = []
+    accepted_languages = self._GetAcceptedLanguages()

[not at google - send to devlin, 2012/07/23 13:25:37]

small optimisation: inline this where it's used (don't need to parse header
unless we... actually need to)

[cduvall, 2012/07/23 18:10:37]

Done.

+    for dict_ in samples_list:
+      # Copy the sample dict so we don't change the dict in the cache.
+      sample_data = dict_.copy()

[not at google - send to devlin, 2012/07/23 13:25:37]

small optimisation: can avoid copying this dict until after the "if
name.startswith('__MSG__')" etc check.

[cduvall, 2012/07/23 18:10:37]

Done.

+      name = sample_data['name']
+      description = sample_data['description']
+      if name.startswith('__MSG_') or description.startswith('__MSG_'):
+        name_key = name[len('__MSG_'):-2]

[not at google - send to devlin, 2012/07/23 13:25:37]

nit: instead of -2 have -len('__') perhaps? Makes it clearer that that's what
we're stripping out.

[cduvall, 2012/07/23 18:10:37]

Done.

+        description_key = description[len('__MSG_'):-2]
+        locale = sample_data['default_locale']
+        for lang in accepted_languages:
+          if lang in sample_data['locales']:
+            locale = lang
+            break
+        sample_data['name'] = (
+            sample_data['locales'][locale][name_key]['message'])
+        sample_data['description'] = (
+            sample_data['locales'][locale][description_key]['message'])

[not at google - send to devlin, 2012/07/23 13:25:37]

nit: 
locale_data = sample_data['locales'][locale]
sample_data['name'] = local_data[name_key]['message']
...

[cduvall, 2012/07/23 18:10:37]

Done.

+      return_list.append(sample_data)

[not at google - send to devlin, 2012/07/23 13:25:37]

This is all super cool.

However, there seems to be a bunch of stuff that could go wrong, namely 
examples that have been misconfigured (having an internationalised description
but not name and vice versa, not having a internationalised key for some locale,
etc). These will throw exceptions and get turned into 500s presumably, but it
might be nice to not break the whole samples page because somebody accidentally
misspelled some __MSG_BLAH.

[cduvall, 2012/07/23 18:10:37]

Done. I just caught an exception and reverted the samples_dict to the
un-localized version.

+    return return_list