Change actions.txt to actions.xml

tools/metrics/actions/extract_actions.py

Index: tools/metrics/actions/extract_actions.py
diff --git a/tools/metrics/actions/extract_actions.py b/tools/metrics/actions/extract_actions.py
index 1a4893d045be878ba3353725b4a05d9855412b96..69a7ae3ad33eeb3cc8c0b6349f278f990281db49 100755
--- a/tools/metrics/actions/extract_actions.py
+++ b/tools/metrics/actions/extract_actions.py
@@ -16,20 +16,31 @@ See also:
   content/browser/user_metrics.h
   http://wiki.corp.google.com/twiki/bin/view/Main/ChromeUserExperienceMetrics
 
-If run with a "--hash" argument, chromeactions.txt will be updated.
+After extracting all actions, the content will go through a pretty print
+function to make sure it's well formatted. If the file content needs be changed,
+a window will be prompted asking for user's consent. The old version will also
+be saved in a backup file.
 """
 
 __author__ = 'evanm (Evan Martin)'
 
-import hashlib
 from HTMLParser import HTMLParser
+import logging
 import os
 import re
+import shutil
 import sys
+from xml.dom import minidom
+
 
 sys.path.insert(1, os.path.join(sys.path[0], '..', '..', 'python'))
 from google import path_utils
 
+# Import the metrics/common module for pretty print xml.
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'common'))
+import diffutil

[Alexei Svitkine (slow), 2014/02/03 19:37:34]

Hmm, can diffutil and pretty_print_xml use the same naming convention? (i.e.
standardize on either underscores, or no underscores).

[yao, 2014/02/04 19:08:12]

Done.

+import pretty_print_xml
+
 # Files that are known to use content::RecordComputedAction(), which means
 # they require special handling code in this script.
 # To add a new file, add it to this list and add the appropriate logic to
@@ -121,6 +132,34 @@ REPOSITORY_ROOT = os.path.join(path_utils.ScriptDir(), '..', '..', '..')
 
 number_of_files_total = 0
 
+# Tags that need to be inserted to each 'action' tag and their default content.
+TAGS = {'description': 'Please enter the description of this user action.',
+        'owner': 'Please specify the owner of this user action.'}
+
+# Doc for actions.xml
+_DOC = 'User action XML'
+
+# Desired order for tag and tag attributes.
+# { tag_name: [attribute_name, ...] }
+ATTRIBUTE_ORDER = {
+    'action': ['name'],
+    'owner': [],
+    'description': [],
+}
+
+# Tag names for top-level nodes whose children we don't want to indent.
+TAGS_THAT_DONT_INDENT = ['actions']
+
+# Extra vertical spacing rules for special tag names.
+# {tag_name: (newlines_after_open, newlines_before_close, newlines_after_close)}
+TAGS_THAT_HAVE_EXTRA_NEWLINE = {
+    'actions': (2, 1, 1),
+    'action': (1, 1, 1),
+}
+
+# Tags that we allow to be squished into a single line for brevity.
+TAGS_THAT_ALLOW_SINGLE_LINE = ['owner']
+
 
 def AddComputedActions(actions):
   """Add computed actions to the actions list.
@@ -573,27 +612,76 @@ def AddAutomaticResetBannerActions(actions):
   actions.add('AutomaticReset_WebUIBanner_ManuallyClosed')
   actions.add('AutomaticReset_WebUIBanner_ResetClicked')
 
-def main(argv):
-  if '--hash' in argv:
-    hash_output = True
+
+class Action(object):
+  def __init__(self, name, description, owner):
+    self.name = name
+    self.values = {'description': description, 'owner': owner}
+
+
+class Error(Exception):
+  pass
+
+
+def _ExtractText(parent_dom, tag_name):
+  """Extract the text value enclosed by |tag_name| under |parent_dom|"""
+  child_dom = parent_dom.getElementsByTagName(tag_name)
+  if child_dom.length == 1:
+    text_dom = child_dom[0].childNodes
+    if text_dom.length != 1:
+      logging.error('More than 1 child node exist under %s' % tag_name)

[Alexei Svitkine (slow), 2014/02/03 19:37:34]

Nit: exist -> exists

[yao, 2014/02/04 19:08:12]

Done.

+      raise Error()

[Alexei Svitkine (slow), 2014/02/03 19:37:34]

Can't the message above be included in the Error?

[yao, 2014/02/04 19:08:12]

Done.

+    if text_dom[0].nodeType != minidom.Node.TEXT_NODE:
+      logging.error('%s\'s child node is not a text node.' % tag_name)
+      raise Error()
+    return text_dom[0].data
+  elif child_dom.length > 1:
+    logging.error('There are more than 1 %s tag.' % tag_name)
+    raise Error()
+
+
+def _CreateTag(top_dom, tag_name, action_name, actions_dict):
+  """Create a new tag
+
+  If action_name is in actions_dict, the values to be inserted is based on the
+  corresponding Action object. If action_name is not in actions_dict, the
+  default value from TAGS is used.
+
+  Args:
+    top_dom: The parent node under which the new tag is created.
+    tag_name: The name of the tag to be created.
+    action_name: The name of an action.
+    actions_dict: A map from action name to Action object.
+  """
+  tag_dom = top_dom.createElement(tag_name)
+  if action_name in actions_dict:
+    tag_dom.appendChild(top_dom.createTextNode(
+        actions_dict[action_name].values[tag_name]))
   else:
-    hash_output = False
-    print >>sys.stderr, "WARNING: If you added new UMA tags, you must" + \
-           " use the --hash option to update chromeactions.txt."
-  # if we do a hash output, we want to only append NEW actions, and we know
-  # the file we want to work on
+    tag_dom.appendChild(top_dom.createTextNode(TAGS.get(tag_name, '')))
+  return tag_dom
+
+
+def main(argv):
+  # A set to store all actions.
   actions = set()
+  # A map from action name to an Action object.
+  actions_dict = {}
 
-  chromeactions_path = os.path.join(path_utils.ScriptDir(), "chromeactions.txt")
+  actions_xml_path = os.path.join(path_utils.ScriptDir(), 'actions.xml')
 
-  if hash_output:
-    f = open(chromeactions_path)
-    for line in f:
-      part = line.rpartition("\t")
-      part = part[2].strip()
-      actions.add(part)
-    f.close()
+  # Save the original file content.
+  with open(actions_xml_path, 'rb') as f:
+    original_xml = f.read()
 
+  # Parse and store the XML data currently stored in file.
+  dom = minidom.parse(actions_xml_path)
+  for action_dom in dom.getElementsByTagName('action'):
+    action_name = action_dom.getAttribute('name')
+    actions.add(action_name)
+    description = _ExtractText(action_dom, 'description')
+    owner = _ExtractText(action_dom, 'owner')
+    actions_dict[action_name] = Action(action_name, description, owner)
 
   AddComputedActions(actions)
   # TODO(fmantek): bring back webkit editor actions.
@@ -615,21 +703,43 @@ def main(argv):
   AddHistoryPageActions(actions)
   AddKeySystemSupportActions(actions)
 
-  if hash_output:
-    f = open(chromeactions_path, "wb")
-
+  # Form new minidom nodes based on updated |actions|.
+  new_dom = minidom.getDOMImplementation().createDocument(None, 'actions', None)
+  top_element = new_dom.documentElement
+  top_element.appendChild(new_dom.createComment(_DOC))
 
   # Print out the actions as a sorted list.
   for action in sorted(actions):
-    if hash_output:
-      hash = hashlib.md5()
-      hash.update(action)
-      print >>f, '0x%s\t%s' % (hash.hexdigest()[:16], action)
-    else:
-      print action
-
-  if hash_output:
-    print "Done. Do not forget to add chromeactions.txt to your changelist"
+    action_dom = new_dom.createElement('action')
+    action_dom.setAttribute('name', action)
+    action_dom.appendChild(_CreateTag(new_dom, 'owner', action, actions_dict))
+    action_dom.appendChild(_CreateTag(
+        new_dom, 'description', action, actions_dict))
+    top_element.appendChild(action_dom)
+
+  # Pretty print the generate minidom node |new_dom|.
+  pretty = pretty_print_xml.PrettyPrintNode(
+      new_dom, pretty_print_xml.XmlStyle(ATTRIBUTE_ORDER,
+                                         TAGS_THAT_HAVE_EXTRA_NEWLINE,
+                                         TAGS_THAT_DONT_INDENT,
+                                         TAGS_THAT_ALLOW_SINGLE_LINE))
+  if original_xml == pretty:
+    print 'actions.xml is correctly pretty-printed.'
+    sys.exit(0)
+
+  # Prompt user to consent on the change.
+  if not diffutil.PromptUserToAcceptDiff(
+      original_xml, pretty, 'Is the new version acceptable?'):
+    logging.error('Aborting')
+    sys.exit(0)
+
+  print 'Creating backup file: actions.before.xml.'
+  shutil.move(actions_xml_path, 'actions.before.xml')
+
+  with open(actions_xml_path, 'wb') as f:
+    f.write(pretty)
+  print ('Updated %s. Do not forget to add it to your changelist' %
+         actions_xml_path)
   return 0