Docserver: Fix 3 issues relating to finding API references in sample files,

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

 # Copyright (c) 2012 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 file_system import FileNotFoundError
+from copy import deepcopy
 import logging
 import re
 import string
 
+from file_system import FileNotFoundError
+
+
 def _ClassifySchemaNode(node_name, api):
   """Attempt to classify |node_name| in an API, determining whether |node_name|
   refers to a type, function, event, or property in |api|.
   """
   if '.' in node_name:
     node_name, rest = node_name.split('.', 1)
   else:
     rest = None
   for key, group in [('types', 'type'),
                      ('functions', 'method'),
                      ('events', 'event'),
                      ('properties', 'property')]:
     for item in api.get(key, []):
       if item['name'] == node_name:
         if rest is not None:
           ret = _ClassifySchemaNode(rest, item)
           if ret is not None:
             return ret
         else:
           return group, node_name
   return None
 
-def _MakeKey(namespace, ref, title):
-  return '%s.%s.%s' % (namespace, ref, title)
+
+def _MakeKey(namespace, ref):

[not at google - send to devlin, 2013/11/01 17:12:04]

caching the title here is silly. the expensive computation is resolving links,
not assigning a title to them.

+  key = '%s/%s' % (namespace, ref)
+  # AppEngine doesn't like keys > 500, but there will be some other stuff
+  # that goes into this key, so truncate it earlier.  This shoudn't be
+  # happening anyway unless there's a bug, such as http://crbug.com/314102.
+  max_size = 256
+  if len(key) > max_size:
+    logging.error('Key was >%s characters: %s' % (max_size, key))
+    key = key[:max_size]
+  return key
+
 
 class ReferenceResolver(object):
   """Resolves references to $ref's by searching through the APIs to find the
   correct node.
 
   $ref's have two forms:
 
     $ref:api.node - Replaces the $ref with a link to node on the API page. The
                     title is set to the name of the node.
 
@@ skipping 17 matching lines @@
       return ReferenceResolver(
           self._api_data_source_factory.Create(None),
           self._api_list_data_source_factory.Create(),
           self._object_store_creator.Create(ReferenceResolver))
 
   def __init__(self, api_data_source, api_list_data_source, object_store):
     self._api_data_source = api_data_source
     self._api_list_data_source = api_list_data_source
     self._object_store = object_store
 
-  def _GetRefLink(self, ref, api_list, namespace, title):
+  def _GetRefLink(self, ref, api_list, namespace):
     # Check nodes within each API the ref might refer to.
     parts = ref.split('.')
     for i, part in enumerate(parts):
       api_name = '.'.join(parts[:i])
       if api_name not in api_list:
         continue
       try:
         api = self._api_data_source.get(api_name, disable_refs=True)
       except FileNotFoundError:
         continue
@@ skipping 15 matching lines @@
               break
         if node_info is None:
           continue
       else:
         text = ref
       category, node_name = node_info
       if namespace is not None and text.startswith('%s.' % namespace):
         text = text[len('%s.' % namespace):]
       return {
         'href': '%s.html#%s-%s' % (api_name, category, name.replace('.', '-')),
-        'text': title if title else text,
+        'text': text,
         'name': node_name
       }
 
     # If it's not a reference to an API node it might just be a reference to an
     # API. Check this last so that links within APIs take precedence over links
     # to other APIs.
     if ref in api_list:
       return {
         'href': '%s.html' % ref,
-        'text': title if title else ref,
+        'text': ref,
         'name': ref
       }
 
     return None
 
   def GetLink(self, ref, namespace=None, title=None):
     """Resolve $ref |ref| in namespace |namespace| if not None, returning None
     if it cannot be resolved.
     """
-    link = self._object_store.Get(_MakeKey(namespace, ref, title)).Get()
-    if link is not None:
-      return link
-
-    api_list = self._api_list_data_source.GetAllNames()
-    link = self._GetRefLink(ref, api_list, namespace, title)
-
-    if link is None and namespace is not None:
-      # Try to resolve the ref in the current namespace if there is one.
-      link = self._GetRefLink('%s.%s' % (namespace, ref),
-                              api_list,
-                              namespace,
-                              title)
-
-    if link is not None:
-      self._object_store.Set(_MakeKey(namespace, ref, title), link)
+    db_key = _MakeKey(namespace, ref)
+    link = self._object_store.Get(db_key).Get()
+    if link is None:
+      api_list = self._api_list_data_source.GetAllNames()
+      link = self._GetRefLink(ref, api_list, namespace)
+      if link is None and namespace is not None:
+        # Try to resolve the ref in the current namespace if there is one.
+        link = self._GetRefLink('%s.%s' % (namespace, ref), api_list, namespace)
+      if link is None:
+        return None
+      self._object_store.Set(db_key, link)
+    else:
+      link = deepcopy(link)

[Jeffrey Yasskin, 2013/11/01 18:00:24]

Does the object store actually return the same object to multiple callers?

[not at google - send to devlin, 2013/11/04 21:51:01]

The in-memory one does. Copy-if-you-need-to[-mutate-it].

+    if title is not None:

[not at google - send to devlin, 2013/11/01 17:12:04]

this is all-but-identical code. rietveld is just being silly.

+      link['text'] = title
     return link
 
   def SafeGetLink(self, ref, namespace=None, title=None):
     """Resolve $ref |ref| in namespace |namespace|, or globally if None. If it
     cannot be resolved, pretend like it is a link to a type.
     """
     ref_data = self.GetLink(ref, namespace=namespace, title=title)
     if ref_data is not None:
       return ref_data
     logging.error('$ref %s could not be resolved in namespace %s.' %
         (ref, namespace))
     type_name = ref.rsplit('.', 1)[-1]
     return {
       'href': '#type-%s' % type_name,
-      'text': title if title else ref,
+      'text': title or ref,
       'name': ref
     }
 
   def ResolveAllLinks(self, text, namespace=None):
     """This method will resolve all $ref links in |text| using namespace
     |namespace| if not None. Any links that cannot be resolved will be replaced
     using the default link format that |SafeGetLink| uses.
     """
     if text is None or '$ref:' not in text:
       return text
@@ skipping 20 matching lines @@
           ref = ''
           rest = ref_and_rest
         else:
           ref = match.group()
           rest = ref_and_rest[match.end():]
 
       ref_dict = self.SafeGetLink(ref, namespace=namespace, title=title)
       formatted_text.append('<a href="%(href)s">%(text)s</a>%(rest)s' %
           { 'href': ref_dict['href'], 'text': ref_dict['text'], 'rest': rest })
     return ''.join(formatted_text)