Docserver manifest follow up (rewrite)

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

Index: chrome/common/extensions/docs/server2/features_utility.py
diff --git a/chrome/common/extensions/docs/server2/features_utility.py b/chrome/common/extensions/docs/server2/features_utility.py
new file mode 100644
index 0000000000000000000000000000000000000000..e95b2c4f707c4af0baa740abf6e2ced1e31f5fc4
--- /dev/null
+++ b/chrome/common/extensions/docs/server2/features_utility.py
@@ -0,0 +1,64 @@
+# Copyright 2013 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.
+
+def FeaturesFromJson(features_json):

[not at google - send to devlin, 2013/08/08 16:31:19]

let's go with Parse here and Merge below, then call via features.Merge,
features.Parse, features.Merge when we call them. Less tautology is good.

[jshumway, 2013/08/14 23:38:39]

Done.

+  '''Standardize the raw features file json |features_json| into a more regular
+  format. Return a dictionary of features, keyed by name. Each feature is
+  guaranteed to have a 'name' attribute and a 'platforms' attribute that is a
+  list of platforms the feature is relevant to. Valid platforms are 'app' or
+  'extension'. Other optional keys may be present in each feature.
+
+  Features with a 'whitelist' are only relevant to apps or extensions on that
+  whitelist and are filtered out.
+  '''
+  features = {}
+
+  for name, value in features_json.iteritems():
+    # Some feature names corrispond to a list; force a list down to a single
+    # feature by removing entries that have a 'whitelist'.
+    if isinstance(value, list):
+      values = [subvalue for subvalue in value if not 'whitelist' in subvalue]
+      if values:
+        value = values[0]
+      else:
+        continue
+
+    if 'whitelist' in value:
+      continue
+
+    features[name] = { 'platforms': [] }
+
+    platforms = value.pop('extension_types')
+    if platforms == 'all' or 'platform_app' in platforms:
+      features[name]['platforms'].append('app')
+    if platforms == 'all' or 'extension' in platforms:
+      features[name]['platforms'].append('extension')
+
+    features[name]['name'] = name
+    features[name].update(value)
+
+  return features
+
+def Filter(features, platform=None):
+  '''Remove items from standardized |features| that are not relevant to
+  |platform|. Returns the mutated features.
+  '''

[not at google - send to devlin, 2013/08/08 16:31:19]

How about this idiom: python has "sort" for in-place modification and "sorted"
for non-in-place. The former doesn't return anything, to be clear, while the
latter obviously does.

Let's either call this Filtered and return a copy, or make it not return itself
so that it's not confusing.

The latter is fine with me I suppose but like I said I don't like mutating data.
If it's an easy enough change can we please do that, and for MergeFeatures.
Callers might forget to deepcopy() and then we have a bug.

The server is currently flakily failing all the time and I don't know what the
cause is, but introducing code that may be flaky (dependent on when appengine
decides to restart our process) is going to make it harder to find.

[jshumway, 2013/08/14 23:38:39]

Done.

+  for name, feature in features.items():
+    if platform and not platform in feature['platforms']:
+      del features[name]
+
+  return features
+
+def MergeFeatures(features, other):
+  '''Merge additional information into |features| from |other|. Merge features
+  with the same name with a standard dictionary update, instead of overwritting
+  features present in both dictionaries. Mutates and returns features.
+  '''
+  for key, value in other.iteritems():
+    if key in features:
+      features[key].update(value)
+    else:
+      features[key] = value
+
+  return features