Fix duplicated strings in ADMX templates.

Also fixed the generated <string> IDs, which can't contain "."
characters.

BUG=chromium:429117

Review URL: https://codereview.chromium.org/702793003

git-svn-id: http://grit-i18n.googlecode.com/svn/trunk@181 7262f16d-afe8-6277-6482-052fa10e57b1

6 files changed, 259 insertions, 36 deletions

diff --git a/grit/format/policy_templates/writers/adm_writer.py b/grit/format/policy_templates/writers/adm_writer.py
index 0413e06..7ab612c 100644
--- a/grit/format/policy_templates/writers/adm_writer.py
+++ b/grit/format/policy_templates/writers/adm_writer.py
@@ -75,7 +75,12 @@ class AdmWriter(template_writer.TemplateWriter):
     'dict': 'EDITTEXT'
   }
 
+  def _Escape(self, string):
+    return string.replace('.', '_')
+
   def _AddGuiString(self, name, value):
+    # The |name| must be escaped.
+    assert name == self._Escape(name)
     # Escape newlines in the value.
     value = value.replace('\n', '\\n')
     if name in self.strings_seen:
@@ -100,7 +105,7 @@ class AdmWriter(template_writer.TemplateWriter):
       key_name: The registry key backing the policy.
       builder: Builder to append lines to.
     '''
-    policy_part_name = policy['name'] + '_Part'
+    policy_part_name = self._Escape(policy['name'] + '_Part')
     self._AddGuiString(policy_part_name, policy['label'])
 
     # Print the PART ... END PART section:
@@ -124,9 +129,9 @@ class AdmWriter(template_writer.TemplateWriter):
           value_text = 'NUMERIC ' + str(item['value'])
         else:
           value_text = '"' + item['value'] + '"'
-        builder.AddLine('NAME !!%s_DropDown VALUE %s' %
-            (item['name'], value_text))
-        self._AddGuiString(item['name'] + '_DropDown', item['caption'])
+        string_id = self._Escape(item['name'] + '_DropDown')
+        builder.AddLine('NAME !!%s VALUE %s' % (string_id, value_text))
+        self._AddGuiString(string_id, item['caption'])
       builder.AddLine('END ITEMLIST', -1)
     builder.AddLine('END PART', -1)
 
@@ -135,10 +140,11 @@ class AdmWriter(template_writer.TemplateWriter):
       # This type can only be set through cloud policy.
       return
 
-    self._AddGuiString(policy['name'] + '_Policy', policy['caption'])
-    builder.AddLine('POLICY !!%s_Policy' % policy['name'], 1)
+    policy_name = self._Escape(policy['name'] + '_Policy')
+    self._AddGuiString(policy_name, policy['caption'])
+    builder.AddLine('POLICY !!%s' % policy_name, 1)
     self._WriteSupported(builder)
-    policy_explain_name = policy['name'] + '_Explain'
+    policy_explain_name = self._Escape(policy['name'] + '_Explain')
     self._AddGuiString(policy_explain_name, policy['desc'])
     builder.AddLine('EXPLAIN !!' + policy_explain_name)
 
@@ -167,7 +173,7 @@ class AdmWriter(template_writer.TemplateWriter):
                       self.recommended_policies)
 
   def BeginPolicyGroup(self, group):
-    category_name = group['name'] + '_Category'
+    category_name = self._Escape(group['name'] + '_Category')
     self._AddGuiString(category_name, group['caption'])
     self.policies.AddLine('CATEGORY !!' + category_name, 1)
 
@@ -176,7 +182,7 @@ class AdmWriter(template_writer.TemplateWriter):
     self.policies.AddLine('')
 
   def BeginRecommendedPolicyGroup(self, group):
-    category_name = group['name'] + '_Category'
+    category_name = self._Escape(group['name'] + '_Category')
     self._AddGuiString(category_name, group['caption'])
     self.recommended_policies.AddLine('CATEGORY !!' + category_name, 1)
 
diff --git a/grit/format/policy_templates/writers/adm_writer_unittest.py b/grit/format/policy_templates/writers/adm_writer_unittest.py
index 27e5314..b126391 100644
--- a/grit/format/policy_templates/writers/adm_writer_unittest.py
+++ b/grit/format/policy_templates/writers/adm_writer_unittest.py
@@ -1020,5 +1020,103 @@ Policy2_Part="Caption of policy2."
 ''')
     self.CompareOutputs(output, expected_output)
 
+  def testDuplicatedStringEnumPolicy(self):
+    # Verifies that duplicated enum constants get merged, and that
+    # string constants get escaped.
+    grd = self.PrepareTest('''
+      {
+        'policy_definitions': [
+          {
+            'name': 'EnumPolicy.A',
+            'type': 'string-enum',
+            'caption': 'Caption of policy A.',
+            'desc': 'Description of policy A.',
+            'items': [
+              {'name': 'tls1.2', 'value': 'tls1.2', 'caption': 'tls1.2' },
+            ],
+            'supported_on': ['chrome.win:39-'],
+          },
+          {
+            'name': 'EnumPolicy.B',
+            'type': 'string-enum',
+            'caption': 'Caption of policy B.',
+            'desc': 'Description of policy B.',
+            'items': [
+              {'name': 'tls1.2', 'value': 'tls1.2', 'caption': 'tls1.2' },
+            ],
+            'supported_on': ['chrome.win:39-'],
+          },
+        ],
+        'placeholders': [],
+        'messages': {
+          'win_supported_winxpsp2': {
+            'text': 'At least Windows 3.14', 'desc': 'blah'
+          },
+          'doc_recommended': {
+            'text': 'Recommended', 'desc': 'bleh'
+          }
+        }
+      }''')
+    output = self.GetOutput(grd, 'fr', {'_google_chrome': '1'}, 'adm', 'en')
+    expected_output = self.ConstructOutput(
+        ['MACHINE', 'USER'], '''
+  CATEGORY !!google
+    CATEGORY !!googlechrome
+      KEYNAME "Software\\Policies\\Google\\Chrome"
+
+      POLICY !!EnumPolicy_A_Policy
+        #if version >= 4
+          SUPPORTED !!SUPPORTED_WINXPSP2
+        #endif
+        EXPLAIN !!EnumPolicy_A_Explain
+
+        PART !!EnumPolicy_A_Part  DROPDOWNLIST
+          VALUENAME "EnumPolicy.A"
+          ITEMLIST
+            NAME !!tls1_2_DropDown VALUE "tls1.2"
+          END ITEMLIST
+        END PART
+      END POLICY
+
+      POLICY !!EnumPolicy_B_Policy
+        #if version >= 4
+          SUPPORTED !!SUPPORTED_WINXPSP2
+        #endif
+        EXPLAIN !!EnumPolicy_B_Explain
+
+        PART !!EnumPolicy_B_Part  DROPDOWNLIST
+          VALUENAME "EnumPolicy.B"
+          ITEMLIST
+            NAME !!tls1_2_DropDown VALUE "tls1.2"
+          END ITEMLIST
+        END PART
+      END POLICY
+
+    END CATEGORY
+  END CATEGORY
+
+  CATEGORY !!google
+    CATEGORY !!googlechrome_recommended
+      KEYNAME "Software\\Policies\\Google\\Chrome\\Recommended"
+
+    END CATEGORY
+  END CATEGORY
+
+
+''', '''[Strings]
+SUPPORTED_WINXPSP2="At least Windows 3.14"
+google="Google"
+googlechrome="Google Chrome"
+googlechrome_recommended="Google Chrome - Recommended"
+EnumPolicy_A_Policy="Caption of policy A."
+EnumPolicy_A_Explain="Description of policy A."
+EnumPolicy_A_Part="Caption of policy A."
+tls1_2_DropDown="tls1.2"
+EnumPolicy_B_Policy="Caption of policy B."
+EnumPolicy_B_Explain="Description of policy B."
+EnumPolicy_B_Part="Caption of policy B."
+''')
+    self.CompareOutputs(output, expected_output)
+
 if __name__ == '__main__':
   unittest.main()
diff --git a/grit/format/policy_templates/writers/adml_writer.py b/grit/format/policy_templates/writers/adml_writer.py
index 5a6e7ee..64f8562 100644
--- a/grit/format/policy_templates/writers/adml_writer.py
+++ b/grit/format/policy_templates/writers/adml_writer.py
@@ -30,19 +30,24 @@ class ADMLWriter(xml_formatted_writer.XMLFormattedWriter):
   # describe the presentation of Policy-Groups and Policies.
   _presentation_table_elem = None
 
-  def _AddString(self, parent, id, text):
-    ''' Adds an ADML "string" element to the passed parent. The following
+  def _AddString(self, id, text):
+    ''' Adds an ADML "string" element to _string_table_elem. The following
     ADML snippet contains an example:
 
     <string id="$(id)">$(text)</string>
 
     Args:
-      parent: Parent element to which the new "string" element is added.
       id: ID of the newly created "string" element.
       text: Value of the newly created "string" element.
     '''
-    string_elem = self.AddElement(parent, 'string', {'id': id})
-    string_elem.appendChild(self._doc.createTextNode(text))
+    id = id.replace('.', '_')
+    if id in self.strings_seen:
+      assert text == self.strings_seen[id]
+    else:
+      self.strings_seen[id] = text
+      string_elem = self.AddElement(
+          self._string_table_elem, 'string', {'id': id})
+      string_elem.appendChild(self._doc.createTextNode(text))
 
   def WritePolicy(self, policy):
     '''Generates the ADML elements for a Policy.
@@ -75,9 +80,8 @@ class ADMLWriter(xml_formatted_writer.XMLFormattedWriter):
     else:
       policy_label = policy_name
 
-    self._AddString(self._string_table_elem, policy_name, policy_caption)
-    self._AddString(self._string_table_elem, policy_name + '_Explain',
-                    policy_description)
+    self._AddString(policy_name, policy_caption)
+    self._AddString(policy_name + '_Explain', policy_description)
     presentation_elem = self.AddElement(
         self._presentation_table_elem, 'presentation', {'id': policy_name})
 
@@ -95,14 +99,12 @@ class ADMLWriter(xml_formatted_writer.XMLFormattedWriter):
       textbox_elem.appendChild(self._doc.createTextNode(policy_label + ':'))
     elif policy_type in ('int-enum', 'string-enum'):
       for item in policy['items']:
-        self._AddString(self._string_table_elem, item['name'], item['caption'])
+        self._AddString(item['name'], item['caption'])
       dropdownlist_elem = self.AddElement(presentation_elem, 'dropdownList',
                                           {'refId': policy_name})
       dropdownlist_elem.appendChild(self._doc.createTextNode(policy_label))
     elif policy_type in ('list', 'string-enum-list'):
-      self._AddString(self._string_table_elem,
-                      policy_name + 'Desc',
-                      policy_caption)
+      self._AddString(policy_name + 'Desc', policy_caption)
       listbox_elem = self.AddElement(presentation_elem, 'listBox',
                                      {'refId': policy_name + 'Desc'})
       listbox_elem.appendChild(self._doc.createTextNode(policy_label))
@@ -128,33 +130,27 @@ class ADMLWriter(xml_formatted_writer.XMLFormattedWriter):
     '''
     # Add ADML "string" elements to the string-table that are required by a
     # Policy-Group.
-    self._AddString(self._string_table_elem, group['name'] + '_group',
-                    group['caption'])
+    self._AddString(group['name'] + '_group', group['caption'])
 
-  def _AddBaseStrings(self, string_table_elem, build):
+  def _AddBaseStrings(self, build):
     ''' Adds ADML "string" elements to the string-table that are referenced by
     the ADMX file but not related to any specific Policy-Group or Policy.
     '''
-    self._AddString(string_table_elem, self.config['win_supported_os'],
+    self._AddString(self.config['win_supported_os'],
                     self.messages['win_supported_winxpsp2']['text'])
     recommended_name = '%s - %s' % \
         (self.config['app_name'], self.messages['doc_recommended']['text'])
     if build == 'chrome':
-      self._AddString(string_table_elem,
-                      self.config['win_mandatory_category_path'][0],
+      self._AddString(self.config['win_mandatory_category_path'][0],
                       'Google')
-      self._AddString(string_table_elem,
-                      self.config['win_mandatory_category_path'][1],
+      self._AddString(self.config['win_mandatory_category_path'][1],
                       self.config['app_name'])
-      self._AddString(string_table_elem,
-                      self.config['win_recommended_category_path'][1],
+      self._AddString(self.config['win_recommended_category_path'][1],
                       recommended_name)
     elif build == 'chromium':
-      self._AddString(string_table_elem,
-                      self.config['win_mandatory_category_path'][0],
+      self._AddString(self.config['win_mandatory_category_path'][0],
                       self.config['app_name'])
-      self._AddString(string_table_elem,
-                      self.config['win_recommended_category_path'][0],
+      self._AddString(self.config['win_recommended_category_path'][0],
                       recommended_name)
 
   def BeginTemplate(self):
@@ -173,10 +169,14 @@ class ADMLWriter(xml_formatted_writer.XMLFormattedWriter):
     resources_elem = self.AddElement(policy_definitions_resources_elem,
                                      'resources')
     self._string_table_elem = self.AddElement(resources_elem, 'stringTable')
-    self._AddBaseStrings(self._string_table_elem, self.config['build'])
+    self._AddBaseStrings(self.config['build'])
     self._presentation_table_elem = self.AddElement(resources_elem,
                                                    'presentationTable')
 
+  def Init(self):
+    # Map of all strings seen.
+    self.strings_seen = {}
+
   def GetTemplateText(self):
     # Using "toprettyxml()" confuses the Windows Group Policy Editor
     # (gpedit.msc) because it interprets whitespace characters in text between
diff --git a/grit/format/policy_templates/writers/adml_writer_unittest.py b/grit/format/policy_templates/writers/adml_writer_unittest.py
index ecafece..08bec58 100644
--- a/grit/format/policy_templates/writers/adml_writer_unittest.py
+++ b/grit/format/policy_templates/writers/adml_writer_unittest.py
@@ -376,6 +376,62 @@ class AdmlWriterUnittest(xml_writer_base_unittest.XmlWriterBaseTest):
       ]
     }))
 
+  def testStringEncodings(self):
+    enum_policy_a = {
+      'name': 'EnumPolicy.A',
+      'type': 'string-enum',
+      'caption': 'Enum policy A caption',
+      'label': 'Enum policy A label',
+      'desc': 'This is a test description.',
+      'items': [
+          {
+           'name': 'tls1.2',
+           'value': 'tls1.2',
+           'caption': 'tls1.2',
+          }
+      ],
+    }
+    enum_policy_b = {
+      'name': 'EnumPolicy.B',
+      'type': 'string-enum',
+      'caption': 'Enum policy B caption',
+      'label': 'Enum policy B label',
+      'desc': 'This is a test description.',
+      'items': [
+          {
+           'name': 'tls1.2',
+           'value': 'tls1.2',
+           'caption': 'tls1.2',
+          }
+      ],
+    }
+    self. _InitWriterForAddingPolicies(self.writer, enum_policy_a)
+    self.writer.WritePolicy(enum_policy_a)
+    self.writer.WritePolicy(enum_policy_b)
+    # Assert generated string elements.
+    output = self.GetXMLOfChildren(self.writer._string_table_elem)
+    expected_output = (
+        '<string id="EnumPolicy_A">Enum policy A caption</string>\n'
+        '<string id="EnumPolicy_A_Explain">'
+        'This is a test description.</string>\n'
+        '<string id="tls1_2">tls1.2</string>\n'
+        '<string id="EnumPolicy_B">Enum policy B caption</string>\n'
+        '<string id="EnumPolicy_B_Explain">'
+        'This is a test description.</string>\n')
+    self.AssertXMLEquals(output, expected_output)
+    # Assert generated presentation elements.
+    output = self.GetXMLOfChildren(self.writer._presentation_table_elem)
+    expected_output = (
+        '<presentation id="EnumPolicy.A">\n'
+        '  <dropdownList refId="EnumPolicy.A">'
+        'Enum policy A label</dropdownList>\n'
+        '</presentation>\n'
+        '<presentation id="EnumPolicy.B">\n'
+        '  <dropdownList refId="EnumPolicy.B">'
+        'Enum policy B label</dropdownList>\n'
+        '</presentation>')
+    self.AssertXMLEquals(output, expected_output)
+
 
 if __name__ == '__main__':
   unittest.main()
diff --git a/grit/format/policy_templates/writers/admx_writer.py b/grit/format/policy_templates/writers/admx_writer.py
index 5eb4475..4bfb723 100644
--- a/grit/format/policy_templates/writers/admx_writer.py
+++ b/grit/format/policy_templates/writers/admx_writer.py
@@ -32,6 +32,7 @@ class ADMXWriter(xml_formatted_writer.XMLFormattedWriter):
     Args:
       name: Name of the referenced ADML string.
     '''
+    name = name.replace('.', '_')
     return '$(string.' + name + ')'
 
   def _AdmlStringExplain(self, name):
@@ -39,6 +40,7 @@ class ADMXWriter(xml_formatted_writer.XMLFormattedWriter):
     Args:
       name: Name of the referenced ADML explanation.
     '''
+    name = name.replace('.', '_')
     return '$(string.' + name + '_Explain)'
 
   def _AdmlPresentation(self, name):
diff --git a/grit/format/policy_templates/writers/admx_writer_unittest.py b/grit/format/policy_templates/writers/admx_writer_unittest.py
index 477a9b6..493c715 100644
--- a/grit/format/policy_templates/writers/admx_writer_unittest.py
+++ b/grit/format/policy_templates/writers/admx_writer_unittest.py
@@ -506,6 +506,67 @@ class AdmxWriterUnittest(xml_writer_base_unittest.XmlWriterBaseTest):
       ]
     }))
 
+  def testStringEncodings(self):
+    enum_policy_a = {
+      'name': 'SampleEnumPolicy.A',
+      'type': 'string-enum',
+        'items': [
+          {'name': 'tls1.2', 'value': 'tls1.2'}
+        ]
+    }
+    enum_policy_b = {
+      'name': 'SampleEnumPolicy.B',
+      'type': 'string-enum',
+        'items': [
+          {'name': 'tls1.2', 'value': 'tls1.2'}
+        ]
+    }
+
+    dom_impl = minidom.getDOMImplementation('')
+    self.writer._doc = dom_impl.createDocument(None, 'policyDefinitions', None)
+    self.writer._active_policies_elem = self.writer._doc.documentElement
+    self.writer._active_mandatory_policy_group_name = 'PolicyGroup'
+    self.writer.WritePolicy(enum_policy_a)
+    self.writer.WritePolicy(enum_policy_b)
+    output = self.writer.GetTemplateText()
+    expected_output = (
+        '<?xml version="1.0" ?>\n'
+        '<policyDefinitions>\n'
+        '  <policy class="TestClass" displayName="$(string.SampleEnumPolicy_A)"'
+          ' explainText="$(string.SampleEnumPolicy_A_Explain)"'
+          ' key="Software\\Policies\\Test" name="SampleEnumPolicy.A"'
+          ' presentation="$(presentation.SampleEnumPolicy.A)">\n'
+        '    <parentCategory ref="PolicyGroup"/>\n'
+        '    <supportedOn ref="SUPPORTED_TESTOS"/>\n'
+        '    <elements>\n'
+        '      <enum id="SampleEnumPolicy.A" valueName="SampleEnumPolicy.A">\n'
+        '        <item displayName="$(string.tls1_2)">\n'
+        '          <value>\n'
+        '            <string>tls1.2</string>\n'
+        '          </value>\n'
+        '        </item>\n'
+        '      </enum>\n'
+        '    </elements>\n'
+        '  </policy>\n'
+        '  <policy class="TestClass" displayName="$(string.SampleEnumPolicy_B)"'
+          ' explainText="$(string.SampleEnumPolicy_B_Explain)"'
+          ' key="Software\\Policies\\Test" name="SampleEnumPolicy.B"'
+          ' presentation="$(presentation.SampleEnumPolicy.B)">\n'
+        '    <parentCategory ref="PolicyGroup"/>\n'
+        '    <supportedOn ref="SUPPORTED_TESTOS"/>\n'
+        '    <elements>\n'
+        '      <enum id="SampleEnumPolicy.B" valueName="SampleEnumPolicy.B">\n'
+        '        <item displayName="$(string.tls1_2)">\n'
+        '          <value>\n'
+        '            <string>tls1.2</string>\n'
+        '          </value>\n'
+        '        </item>\n'
+        '      </enum>\n'
+        '    </elements>\n'
+        '  </policy>\n'
+        '</policyDefinitions>')
+    self.AssertXMLEquals(output, expected_output)
+
 
 if __name__ == '__main__':
   unittest.main()