Docserver: Implement ContentProvider.GetVersion.

BUG=402903
R=yoz@chromium.org
NOTRY=true

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

Cr-Commit-Position: refs/heads/master@{#289373}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289373 0039d316-1c4b-4281-b951-d872f2087c98

chrome/common/extensions/docs/server2/app.yaml

 application: chrome-apps-doc
-version: 3-39-3
+version: 3-39-4
 runtime: python27
 api_version: 1
 threadsafe: false

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

@@ -11,7 +11,7 @@ from compiled_file_system import SingleFile
 from directory_zipper import DirectoryZipper
 from docs_server_utils import ToUnicode
 from file_system import FileNotFoundError
-from future import Future
+from future import All, Future
 from path_canonicalizer import PathCanonicalizer
 from path_util import AssertIsValid, IsDirectory, Join, ToDirectory
 from special_paths import SITE_VERIFICATION_FILE
@@ -129,64 +129,84 @@ class ContentProvider(object):
     return self._path_canonicalizer.Canonicalize(path)
 
   def GetContentAndType(self, path):
-    '''Returns the ContentAndType of the file at |path|.
+    '''Returns a Future to the ContentAndType of the file at |path|.
+    '''
+    AssertIsValid(path)
+    base, ext = posixpath.splitext(path)
+    if self._directory_zipper and ext == '.zip':
+      return (self._directory_zipper.Zip(ToDirectory(base))
+              .Then(lambda zipped: ContentAndType(zipped,
+                                                  'application/zip',
+                                                  None)))
+    return self._FindFileForPath(path).Then(self._content_cache.GetFromFile)
+
+  def GetVersion(self, path):
+    '''Returns a Future to the version of the file at |path|.
     '''
     AssertIsValid(path)
     base, ext = posixpath.splitext(path)
-
-    # Check for a zip file first, if zip is enabled.
     if self._directory_zipper and ext == '.zip':
-      zip_future = self._directory_zipper.Zip(ToDirectory(base))
-      return Future(callback=
-          lambda: ContentAndType(zip_future.Get(), 'application/zip', None))
-
-    # If there is no file extension, look for a file with one of the default
-    # extensions. If one cannot be found, check if the path is a directory.
-    # If it is, then check for an index file with one of the default
-    # extensions.
-    if not ext:
-      new_path = self._AddExt(path)
-      # Add a trailing / to check if it is a directory and not a file with
-      # no extension.
-      if new_path is None and self.file_system.Exists(ToDirectory(path)).Get():
-        new_path = self._AddExt(Join(path, 'index'))
-        # If an index file wasn't found in this directly then we're never going
-        # to find a file.
-        if new_path is None:
-          return FileNotFoundError.RaiseInFuture('"%s" is a directory' % path)
-      if new_path is not None:
-        path = new_path
-
-    return self._content_cache.GetFromFile(path)
-
-  def _AddExt(self, path):
-    '''Tries to append each of the default file extensions to path and returns
-    the first one that is an existing file.
+      stat_future = self.file_system.StatAsync(ToDirectory(base))
+    else:
+      stat_future = self._FindFileForPath(path).Then(self.file_system.StatAsync)
+    return stat_future.Then(lambda stat: stat.version)
+
+  def _FindFileForPath(self, path):
+    '''Finds the real file backing |path|. This may require looking for the
+    correct file extension, or looking for an 'index' file if it's a directory.
+    Returns None if no path is found.
     '''
-    for default_ext in self._default_extensions:
-      if self.file_system.Exists(path + default_ext).Get():
-        return path + default_ext
-    return None
+    AssertIsValid(path)
+    _, ext = posixpath.splitext(path)
+
+    if ext:
+      # There was already an extension, trust that it's a path. Elsewhere
+      # up the stack this will be caught if it's not.
+      return Future(value=path)
+
+    def find_file_with_name(name):
+      '''Tries to find a file in the file system called |name| with one of the
+      default extensions of this content provider.
+      If none is found, returns None.
+      '''
+      paths = [name + ext for ext in self._default_extensions]
+      def get_first_path_which_exists(existence):
+        for exists, path in zip(existence, paths):
+          if exists:
+            return path
+        return None
+      return (All(self.file_system.Exists(path) for path in paths)
+              .Then(get_first_path_which_exists))
+
+    def find_index_file():
+      '''Tries to find an index file in |path|, if |path| is a directory.
+      If not, or if there is no index file, returns None.
+      '''
+      def get_index_if_directory_exists(directory_exists):
+        if not directory_exists:
+          return None
+        return find_file_with_name(Join(path, 'index'))
+      return (self.file_system.Exists(ToDirectory(path))
+              .Then(get_index_if_directory_exists))
+
+    # Try to find a file with the right name. If not, and it's a directory,
+    # look for an index file in that directory. If nothing at all is found,
+    # return the original |path| - its nonexistence will be caught up the stack.
+    return (find_file_with_name(path)
+            .Then(lambda found: found or find_index_file())
+            .Then(lambda found: found or path))
 
   def Cron(self):
-    futures = [('<path_canonicalizer>',  # semi-arbitrary string since there is
-                                         # no path associated with this Future.
-                self._path_canonicalizer.Cron())]
+    futures = [self._path_canonicalizer.Cron()]
     for root, _, files in self.file_system.Walk(''):
       for f in files:
-        futures.append((Join(root, f),
-                        self.GetContentAndType(Join(root, f))))
+        futures.append(self.GetContentAndType(Join(root, f)))
         # Also cache the extension-less version of the file if needed.
         base, ext = posixpath.splitext(f)
         if f != SITE_VERIFICATION_FILE and ext in self._default_extensions:
-          futures.append((Join(root, base),
-                          self.GetContentAndType(Join(root, base))))
+          futures.append(self.GetContentAndType(Join(root, base)))
       # TODO(kalman): Cache .zip files for each directory (if supported).
-    def resolve():
-      for label, future in futures:
-        try: future.Get()
-        except: logging.error('%s: %s' % (label, traceback.format_exc()))
-    return Future(callback=resolve)
+    return All(futures, except_pass=Exception, except_pass_log=True)
 
   def __repr__(self):
     return 'ContentProvider of <%s>' % repr(self.file_system)

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

@@ -76,15 +76,15 @@ _TEST_DATA = {
 
 class ContentProviderUnittest(unittest.TestCase):
   def setUp(self):
+    self._test_file_system = TestFileSystem(_TEST_DATA)
     self._content_provider = self._CreateContentProvider()
 
   def _CreateContentProvider(self, supports_zip=False):
     object_store_creator = ObjectStoreCreator.ForTest()
-    test_file_system = TestFileSystem(_TEST_DATA)
     return ContentProvider(
         'foo',
         CompiledFileSystem.Factory(object_store_creator),
-        test_file_system,
+        self._test_file_system,
         object_store_creator,
         default_extensions=('.html', '.md'),
         # TODO(kalman): Test supports_templates=False.
@@ -97,16 +97,18 @@ class ContentProviderUnittest(unittest.TestCase):
     self.assertEqual(content, content_and_type.content)
     self.assertEqual(content_type, content_and_type.content_type)
 
-  def _assertTemplateContent(self, content, path):
+  def _assertTemplateContent(self, content, path, version):
     content_and_type = self._content_provider.GetContentAndType(path).Get()
     self.assertEqual(Handlebar, type(content_and_type.content))
     content_and_type.content = content_and_type.content.source
     self._assertContent(content, 'text/html', content_and_type)
+    self.assertEqual(version, self._content_provider.GetVersion(path).Get())
 
-  def _assertMarkdownContent(self, content, path):
+  def _assertMarkdownContent(self, content, path, version):
     content_and_type = self._content_provider.GetContentAndType(path).Get()
     content_and_type.content = content_and_type.content.source
     self._assertContent(content, 'text/html', content_and_type)
+    self.assertEqual(version, self._content_provider.GetVersion(path).Get())
 
   def testPlainText(self):
     self._assertContent(
@@ -129,7 +131,9 @@ class ContentProviderUnittest(unittest.TestCase):
         self._content_provider.GetContentAndType('site.css').Get())
 
   def testTemplate(self):
-    self._assertTemplateContent(u'storage.html content', 'storage.html')
+    self._assertTemplateContent(u'storage.html content', 'storage.html', '0')
+    self._test_file_system.IncrementStat('storage.html')
+    self._assertTemplateContent(u'storage.html content', 'storage.html', '1')
 
   def testImage(self):
     self._assertContent(
@@ -174,9 +178,10 @@ class ContentProviderUnittest(unittest.TestCase):
         zip_content_provider.GetCanonicalPath('diR.zip'))
 
   def testMarkdown(self):
-    self._assertMarkdownContent(
-        '\n'.join(text[1] for text in _MARKDOWN_CONTENT),
-        'markdown')
+    expected_content = '\n'.join(text[1] for text in _MARKDOWN_CONTENT)
+    self._assertMarkdownContent(expected_content, 'markdown', '0')
+    self._test_file_system.IncrementStat('markdown.md')
+    self._assertMarkdownContent(expected_content, 'markdown', '1')
 
   def testNotFound(self):
     self.assertRaises(
@@ -184,12 +189,13 @@ class ContentProviderUnittest(unittest.TestCase):
         self._content_provider.GetContentAndType('oops').Get)
 
   def testIndexRedirect(self):
-    self._assertTemplateContent(u'index.html content', '')
-    self._assertTemplateContent(u'index.html content 1', 'dir4')
-    self._assertTemplateContent(u'dir5.html content', 'dir5')
+    self._assertTemplateContent(u'index.html content', '', '0')
+    self._assertTemplateContent(u'index.html content 1', 'dir4', '0')
+    self._assertTemplateContent(u'dir5.html content', 'dir5', '0')
     self._assertMarkdownContent(
         '\n'.join(text[1] for text in _MARKDOWN_CONTENT),
-        'dir7')
+        'dir7',
+        '0')
     self._assertContent(
         'noextension content', 'text/plain',
         self._content_provider.GetContentAndType('noextension').Get())
@@ -197,5 +203,10 @@ class ContentProviderUnittest(unittest.TestCase):
         FileNotFoundError,
         self._content_provider.GetContentAndType('dir6').Get)
 
+  def testCron(self):
+    # Not entirely sure what to test here, but get some code coverage.
+    self._content_provider.Cron().Get()
+
+
 if __name__ == '__main__':
   unittest.main()

chrome/common/extensions/docs/server2/cron.yaml

@@ -2,4 +2,4 @@ cron:
 - description: Repopulates all cached data.
   url: /_cron
   schedule: every 5 minutes
-  target: 3-39-3
+  target: 3-39-4

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

@@ -2,7 +2,9 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import logging
 import sys
+import traceback
 
 _no_value = object()
 
@@ -11,12 +13,15 @@ def _DefaultErrorHandler(error):
   raise error
 
 
-def All(futures, except_pass=None):
+def All(futures, except_pass=None, except_pass_log=False):
   '''Creates a Future which returns a list of results from each Future in
   |futures|.
 
   If any Future raises an error other than those in |except_pass| the returned
   Future will raise as well.
+
+  If any Future raises an error in |except_pass| then None will be inserted as
+  its result. If |except_pass_log| is True then the exception will be logged.
   '''
   def resolve():
     resolved = []
@@ -25,17 +30,21 @@ def All(futures, except_pass=None):
         resolved.append(f.Get())
       # "except None" will simply not catch any errors.
       except except_pass:
+        if except_pass_log:
+          logging.error(traceback.format_exc())
+        resolved.append(None)
         pass
     return resolved
   return Future(callback=resolve)
 
 
-def Race(futures, except_pass=None):
+def Race(futures, except_pass=None, default=_no_value):
   '''Returns a Future which resolves to the first Future in |futures| that
   either succeeds or throws an error apart from those in |except_pass|.
 
-  If all Futures throw errors in |except_pass| then the returned Future
-  will re-throw one of those errors, for a nice stack trace.
+  If all Futures throw errors in |except_pass| then |default| is returned,
+  if specified. If |default| is not specified then one of the passed errors
+  will be re-thrown, for a nice stack trace.
   '''
   def resolve():
     first_future = None
@@ -47,8 +56,10 @@ def Race(futures, except_pass=None):
       # "except None" will simply not catch any errors.
       except except_pass:
         pass
-    # Everything failed, propagate the first error even though it was
-    # caught by |except_pass|.
+    if default is not _no_value:
+      return default
+    # Everything failed and there is no default value, propagate the first
+    # error even though it was caught by |except_pass|.
     return first_future.Get()
   return Future(callback=resolve)
 

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

@@ -94,12 +94,22 @@ class FutureTest(unittest.TestCase):
     callbacks = (callback_with_value(1),
                  callback_with_value(2),
                  MockFunction(throws_error))
+
     future = All(Future(callback=callback) for callback in callbacks)
     for callback in callbacks:
       self.assertTrue(*callback.CheckAndReset(0))
-    # Can't check that the callbacks were actually run because in theory the
-    # Futures can be resolved in any order.
     self.assertRaises(ValueError, future.Get)
+    for callback in callbacks:
+      # Can't check that the callbacks were actually run because in theory the
+      # Futures can be resolved in any order.
+      callback.CheckAndReset(0)
+
+    # Test throwing an error with except_pass.
+    future = All((Future(callback=callback) for callback in callbacks),
+                 except_pass=ValueError)
+    for callback in callbacks:
+      self.assertTrue(*callback.CheckAndReset(0))
+    self.assertEqual([1, 2, None], future.Get())
 
   def testRaceSuccess(self):
     callback = MockFunction(lambda: 42)
@@ -159,6 +169,19 @@ class FutureTest(unittest.TestCase):
                  except_pass=(ValueError,))
     self.assertRaises(ValueError, race.Get)
 
+    # Test except_pass with default values.
+    race = Race((Future(callback=throws_error),
+                 Future(callback=throws_except_error)),
+                 except_pass=(NotImplementedError,),
+                 default=42)
+    self.assertRaises(ValueError, race.Get)
+
+    race = Race((Future(callback=throws_error),
+                 Future(callback=throws_error)),
+                 except_pass=(ValueError,),
+                 default=42)
+    self.assertEqual(42, race.Get())
+
   def testThen(self):
     def assertIs42(val):
       self.assertEqual(val, 42)