Docserver: Add support for viewing docs with a codereview patch applied

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

 # 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 cache_chain_object_store import CacheChainObjectStore
 from memcache_object_store import MemcacheObjectStore
 from persistent_object_store import PersistentObjectStore
 
 class ObjectStoreCreator(object):
   class Factory(object):
     '''Parameters:
     - |branch| The branch to create object stores for. This becomes part of the
       namespace for the object stores that are created.
     - |start_empty| Whether the caching object store that gets created should
       start empty, or start with the content of its delegate object stores.
+    - |issue| Code review issue to preview.
+    - |patchset| Code review patchset to preview.
     '''
-    def __init__(self, app_version, branch, start_empty=False):
+    def __init__(self,
+                 app_version,
+                 branch,
+                 start_empty=False,
+                 issue=None,
+                 patchset=None):

[not at google - send to devlin, 2013/04/26 23:53:53]

I reckon we should make this stuff more generic, like have an "more" argument to
specify the issue and patchset. ObjectStoreCreator is too general a class to
worry about implementation specifies like that.

I.e. here

def __init__(self, ..., more=None):
  ...
  self._more = more

[方觉(Fang Jue), 2013/04/27 01:02:48]

Done.

       self._app_version = app_version
       self._branch = branch
       self._start_empty = start_empty
+      self._issue = issue
+      self._patchset = patchset
 
     def Create(self, cls, store_type=None):
       return ObjectStoreCreator(cls,
                                 self._app_version,
                                 self._branch,
                                 store_type=store_type,
-                                start_empty=self._start_empty)
+                                start_empty=self._start_empty,
+                                issue=self._issue,
+                                patchset=self._patchset)
 
   class SharedFactory(object):
     '''A |Factory| for creating object stores shared across branches.
     '''
     def __init__(self, app_version):
       self._factory = ObjectStoreCreator.Factory(app_version, 'shared')
 
     def Create(self, cls, store_type=None):
       return self._factory.Create(cls, store_type=store_type)
 
   class TestFactory(object):
     '''A |Factory| for creating object stores for tests, with fake defaults.
     '''
     def __init__(self):
       self._factory = ObjectStoreCreator.Factory('test-version', 'test-branch')
 
     def Create(self, cls, store_type=None):
       return self._factory.Create(cls, store_type=store_type)
 
   def __init__(self,
                cls,
                app_version,
                branch,
                store_type=None,
-               start_empty=False):
+               start_empty=False,
+               issue=None,
+               patchset=None):
     '''Creates stores with a top-level namespace given by the name of |cls|
     combined with |branch|. Set an explicit |store_type| if necessary for tests.
 
     By convention this should be the name of the class which owns the object
     store. If a class needs multiple object stores it should use Create with the
     |category| argument.
     '''
     assert isinstance(cls, type)
     assert not cls.__name__[0].islower()  # guard against non-class types
-    self._name = '%s/%s@%s' % (app_version, cls.__name__, branch)
+    self._name = '%s/%s@%s%s' % ( app_version, cls.__name__, branch,
+        '' if issue is None else '.%s.%s' % (issue, patchset))

[not at google - send to devlin, 2013/04/26 23:53:53]

... (continuing from above) then have

if self._more:
  self._name = '%s/%s' % (self._name, '/'.join('%s=%s' % k, self._more[k]
                          for k in sorted(self._more.iterkeys()))

so that you'd get something like

'2-0-23/RietveldFileSystem@trunk/issue=2387419/patchset=3947'

honestly, I'd be tempted to just make it *all* very generic and just have a
generic "components" dictionary. Make this:

self._components = {
  'version': version,
  'branch': branch,
  'cls': cls.__name__,
}
self._components.update(more)

and the whole thing would be

'version=2-0-23/cls=RietveldFileSystem/branch=trunk/issue=2387419/patchset=3947'

well actually it would be that, except the keys would be sorted.

[方觉(Fang Jue), 2013/04/27 01:02:48]

Done.

[方觉(Fang Jue), 2013/04/29 12:46:23]

In the latest patchset, the namespace will be something like:
branch=test/cls=CompiledFileSystem/version=3-0/id/file
Is that a little too long? If that's ok, I'll update the corresponding strings
in *_test.py.

     self._store_type = store_type
     self._start_empty = start_empty
+    self._issue = issue
 
   def Create(self, category=None):
     '''Creates a new object store with the top namespace given in the
     constructor with an optional |category| for classes that need multiple
     object stores (e.g. one for stat and one for read).
     '''
     namespace = self._name
     if category is not None:
       assert not any(c.isdigit() for c in category)
       namespace = '%s/%s' % (namespace, category)
     if self._store_type is not None:
       return self._store_type(namespace)
-    return CacheChainObjectStore(
-        (MemcacheObjectStore(namespace), PersistentObjectStore(namespace)),
-        start_empty=self._start_empty)
+    if self._issue is None:
+      return CacheChainObjectStore(
+          (MemcacheObjectStore(namespace), PersistentObjectStore(namespace)),
+          start_empty=self._start_empty)
+    else:
+      return CacheChainObjectStore((MemcacheObjectStore(namespace),),
+          start_empty=self._start_empty)