diff options
6 files changed, 133 insertions, 68 deletions
diff --git a/chrome/common/extensions/docs/server2/app.yaml b/chrome/common/extensions/docs/server2/app.yaml index 4ba9a44..a4b5dc6 100644 --- a/chrome/common/extensions/docs/server2/app.yaml +++ b/chrome/common/extensions/docs/server2/app.yaml @@ -1,5 +1,5 @@ application: chrome-apps-doc -version: 3-39-3 +version: 3-39-4 runtime: python27 api_version: 1 threadsafe: false diff --git a/chrome/common/extensions/docs/server2/content_provider.py b/chrome/common/extensions/docs/server2/content_provider.py index d62ba4f..84e3d2c 100644 --- a/chrome/common/extensions/docs/server2/content_provider.py +++ b/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) diff --git a/chrome/common/extensions/docs/server2/content_provider_test.py b/chrome/common/extensions/docs/server2/content_provider_test.py index ed7783f..afe47e2 100755 --- a/chrome/common/extensions/docs/server2/content_provider_test.py +++ b/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() diff --git a/chrome/common/extensions/docs/server2/cron.yaml b/chrome/common/extensions/docs/server2/cron.yaml index e34952b..9aa967e 100644 --- a/chrome/common/extensions/docs/server2/cron.yaml +++ b/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 diff --git a/chrome/common/extensions/docs/server2/future.py b/chrome/common/extensions/docs/server2/future.py index 2a13611..289761c 100644 --- a/chrome/common/extensions/docs/server2/future.py +++ b/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) diff --git a/chrome/common/extensions/docs/server2/future_test.py b/chrome/common/extensions/docs/server2/future_test.py index db056c5..33c82b2 100755 --- a/chrome/common/extensions/docs/server2/future_test.py +++ b/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) |