Docserver manifest follow up (rewrite)

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

Index: chrome/common/extensions/docs/server2/features_model.py
diff --git a/chrome/common/extensions/docs/server2/features_model.py b/chrome/common/extensions/docs/server2/features_model.py
new file mode 100644
index 0000000000000000000000000000000000000000..b8c8a0c7ce5c875604784b66a95d8dd4133dd7d0
--- /dev/null
+++ b/chrome/common/extensions/docs/server2/features_model.py
@@ -0,0 +1,79 @@
+# 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.
+
+from copy import deepcopy
+
+class FeaturesModel(object):
+  '''Load and manipulate the contents of a features file. FeaturesModel is
+  immutable.
+  '''
+  def __init__(self, features={}):
+    self._features = features

[not at google - send to devlin, 2013/07/31 18:50:20]

I meant for the features to be typed, but I forgot to mention that. Dealing with
objects is awkward and unsafe; see model.py in tools/json_schema_compiler for
what I meant. Sorry.

It's a small change though. Define another class like Feature at the end, and
have properties "platforms" and "name" plus whatever else you need.

Feature would take all of those in its constructor.

When constructing a new Feature via those Filter and MergeWith methods,
construct the new Feature from the filtered/merged properties, many of which
would be the same.

That would make Get() unnecessary (we won't expose the internals of feature),
instead you could expose methods like Keys/Values/Items which return (or yield)
the keys and/or values of each feature.

[jshumway, 2013/08/01 22:27:50]

As discussed via email, here is the solution with a new dictionary type that has
properties for 'name' and 'platform'.

+
+  @staticmethod
+  def LoadFeaturesJson(features_json):

[not at google - send to devlin, 2013/07/31 18:50:20]

prefer FromJson here, it looks better.

[jshumway, 2013/08/01 22:27:50]

Done.

+    '''Take the raw json of a features file and clean it up. This involves
+    reducing any keys with lists as values to sinlge dicts, converting
+    'extension_types' to a list that contains 'app', 'extension', or both,
+    adding a 'name' field, and ignoring features that have a 'whitelist'
+    attribute.
+    '''
+    features = {}
+
+    for name, value in features_json.iteritems():
+      if isinstance(value, list):

[not at google - send to devlin, 2013/07/31 18:50:20]

needs some comments.

[jshumway, 2013/08/01 22:27:50]

Done.

+        temp = None
+        for subvalue in value:
+          if not 'whitelist' in subvalue:
+            temp = subvalue
+            break
+        else:
+          continue

[not at google - send to devlin, 2013/07/31 18:50:20]

wow else after a for loop looks weird. can you structure it as a list
comprehension or something.

[jshumway, 2013/08/01 22:27:50]

Done.

+        value = temp
+
+      if 'whitelist' in value:
+        continue

[not at google - send to devlin, 2013/07/31 18:50:20]

wouldn't need this, it would be implicit

[jshumway, 2013/08/01 22:27:50]

This still required for the case where value is not a list and contains a
whitelist.

+      features[name] = { 'platform': [] }
+
+      platforms = value.pop('extension_types')
+      if platforms == 'all' or 'extension' in platforms:
+        features[name]['platform'].append('extension')
+      if platforms == 'all' or 'platform_app' in platforms:
+        features[name]['platform'].append('app')
+
+      features[name]['name'] = name
+      features[name].update(value)
+
+    return FeaturesModel(features)
+
+  def Filter(self, platform=None):
+    '''Filter out all features that are not relevant to a specific platform
+    ('app' or 'extension').
+    '''
+    features = {}
+
+    for name, feature in self._features.items():
+      if not platform or platform in feature['platform']:
+        features[name] = feature
+
+    return FeaturesModel(features)
+
+  def MergeWith(self, other):
+    '''Do a semi-recursive merge. Any keys common to both |self| and |other|
+    will have their dictionary values merged, instead of overwritten.
+    '''
+    features = self._features.copy()
+
+    for key, value in deepcopy(other).iteritems():
+      if key in self._features:
+        features[key].update(value)
+      else:
+        features[key] = value
+
+    return FeaturesModel(features)
+
+  def Get(self):
+    '''Returns a dictionary of the features file.
+    '''
+    return deepcopy(self._features)