Extensions Docs Server: Internationalized samples

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

 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 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)
+
+  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 _GetApiItems(self, js_file):
+      return set(re.findall('(chrome\.[a-zA-Z0-9\.]+)', js_file))
+
+    def _MakeApiLink(self, prefix, item):
+      api, name = item.replace('chrome.', '').split('.', 1)
+      return api + '.html#' + prefix + '-' + name
+
+    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
+
+    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)

[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 _GetApiItems(self, js_file):
-    return set(re.findall('(chrome\.[a-zA-Z0-9\.]+)', js_file))
-
-  def _MakeApiLink(self, prefix, item):
-    api, name = item.replace('chrome.', '').split('.', 1)
-    return api + '.html#' + prefix + '-' + name
-
-  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 _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))
-
-      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)
-          })
-      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
+  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