New policy protobuf protocol.

chrome/app/policy/syntax_check_policy_template_json.py

Index: chrome/app/policy/syntax_check_policy_template_json.py
diff --git a/chrome/app/policy/syntax_check_policy_template_json.py b/chrome/app/policy/syntax_check_policy_template_json.py
index 04d69c9b4a0de24daf18a97a20af2c2736599640..07419cd01e39579f9ff8213172d67d210c0ec989 100644
--- a/chrome/app/policy/syntax_check_policy_template_json.py
+++ b/chrome/app/policy/syntax_check_policy_template_json.py
@@ -81,7 +81,39 @@ class PolicyTemplateChecker(object):
                   container_name, identifier, value)
     return value
 
-  def _CheckPolicy(self, policy, may_contain_groups):
+  def _AddProtobufID(self, protobuf_id, policy, is_in_group):
+    '''
+    Adds |protobuf_id| to either self.protobuf_ids or self.group_protobuf_ids,
+    depending on |is_in_group|. Generates an error message if the protobuf_id
+    exists already; |policy| is needed for this message.
+    '''
+    if is_in_group:
+      if protobuf_id in self.group_protobuf_ids:

[gfeher, 2011/02/02 08:42:45]

How about making |self.group_protobuf_ids| / |is_in_group| a function parameter?
(Both here and in _CheckPolicy())

[Jakob Kummerow, 2011/02/03 14:36:52]

Done. Good idea.

+        self._Error('Duplicate protobuf_id', 'policy', policy.get('name'),
+                    protobuf_id)
+      else:
+        self.group_protobuf_ids.add(protobuf_id)
+    else:
+      if protobuf_id in self.protobuf_ids:
+        self._Error('Duplicate protobuf_id', 'policy', policy.get('name'),
+                    protobuf_id)
+      else:
+        self.protobuf_ids.add(protobuf_id)
+
+  def _CheckProtobufIDs(self, protobuf_ids, group=None):
+    '''
+    Checks a set of protobuf_ids to make sure it contains a continuous range
+    of entries (i.e. no holes).

[gfeher, 2011/02/02 08:42:45]

Isn't it possible that we will have holes later as deprecated policies are
removed?

[Jakob Kummerow, 2011/02/03 14:36:52]

As discussed offline, we don't want to allow holes. Ever. (Or if we do, we'll
change the checker.)

+    '''
+    for i in range(len(protobuf_ids)):
+      if (i + 1) not in protobuf_ids:
+        if group is not None:
+          self._Error('Missing protobuf_id: %s' % (i + 1), 'policy group',
+                      group.get('name'))
+        else:
+          self._Error('No policy with protobuf_id: %s' % (i + 1))
+
+  def _CheckPolicy(self, policy, is_in_group):
     if not isinstance(policy, dict):
       self._Error('Each policy must be a dictionary.', 'policy', None, policy)
       return
@@ -90,7 +122,7 @@ class PolicyTemplateChecker(object):
     for key in policy:
       if key not in ('name', 'type', 'caption', 'desc', 'supported_on',
                      'label', 'policies', 'items', 'example_value', 'features',
-                     'deprecated'):
+                     'deprecated', 'protobuf_id'):
         self.warning_count += 1
         print ('In policy %s: Warning: Unknown key: %s' %
                (policy.get('name'), key))
@@ -115,20 +147,28 @@ class PolicyTemplateChecker(object):
     # If 'label' is present, it must be a string.
     self._CheckContains(policy, 'label', str, True)
 
+    # Each policy must have a protobuf ID.
+    protobuf_id = self._CheckContains(policy, 'protobuf_id', int)
+    self._AddProtobufID(protobuf_id, policy, is_in_group)
+
     # If 'deprecated' is present, it must be a bool.
     self._CheckContains(policy, 'deprecated', bool, True)
 
     if policy_type == 'group':
 
       # Groups must not be nested.
-      if not may_contain_groups:
+      if is_in_group:
         self._Error('Policy groups must not be nested.', 'policy', policy)
 
       # Each policy group must have a list of policies.
       policies = self._CheckContains(policy, 'policies', list)
+
+      # Check sub-policies.
+      self.group_protobuf_ids = set()
       if policies is not None:
         for nested_policy in policies:
-          self._CheckPolicy(nested_policy, False)
+          self._CheckPolicy(nested_policy, True)
+      self._CheckProtobufIDs(self.group_protobuf_ids, policy)
 
       # Statistics.
       self.num_groups += 1
@@ -140,7 +180,7 @@ class PolicyTemplateChecker(object):
         for s in supported_on:
           if not isinstance(s, str):
             self._Error('Entries in "supported_on" must be strings.', 'policy',
-                              policy, supported_on)
+                        policy, supported_on)
 
       # Each policy must have a 'features' dict.
       self._CheckContains(policy, 'features', dict)
@@ -160,7 +200,7 @@ class PolicyTemplateChecker(object):
 
       # Statistics.
       self.num_policies += 1
-      if not may_contain_groups:
+      if is_in_group:
         self.num_policies_in_groups += 1
 
     if policy_type in ('int-enum', 'string-enum'):
@@ -333,8 +373,10 @@ class PolicyTemplateChecker(object):
                                              container_name='The root element',
                                              offending=None)
     if policy_definitions is not None:
+      self.protobuf_ids = set()
       for policy in policy_definitions:
-        self._CheckPolicy(policy, True)
+        self._CheckPolicy(policy, False)
+      self._CheckProtobufIDs(self.protobuf_ids)
 
     # Check (non-policy-specific) message definitions.
     messages = self._CheckContains(data, 'messages', dict,