Docs: Add a "skip_not_found" argument to FileSystem.Read, and implement it

for SubversionFileSystem and CachingFileSystem. This is part 1 of a bandaid-ish
solution to cron timeouts; having FileSystem.Read throw an exception if any
file isn't found makes efficient bulk reads impossible given the current
Future architecture.

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

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@261080 0039d316-1c4b-4281-b951-d872f2087c98

18 files changed, 73 insertions, 28 deletions

diff --git a/chrome/common/extensions/docs/server2/app.yaml b/chrome/common/extensions/docs/server2/app.yaml
index 31bb29b..f8b7d6f 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-16-1
+version: 3-16-2
 runtime: python27
 api_version: 1
 threadsafe: false
diff --git a/chrome/common/extensions/docs/server2/caching_file_system.py b/chrome/common/extensions/docs/server2/caching_file_system.py
index 18217a4..feed83e 100644
--- a/chrome/common/extensions/docs/server2/caching_file_system.py
+++ b/chrome/common/extensions/docs/server2/caching_file_system.py
@@ -59,7 +59,7 @@ class CachingFileSystem(FileSystem):
 
     return stat_info
 
-  def Read(self, paths):
+  def Read(self, paths, skip_not_found=False):
     '''Reads a list of files. If a file is in memcache and it is not out of
     date, it is returned. Otherwise, the file is retrieved from the file system.
     '''
@@ -73,6 +73,9 @@ class CachingFileSystem(FileSystem):
         # TODO(cduvall): do a concurrent Stat with the missing stat values.
         try:
           stat_value = self.Stat(path)
+        except FileNotFoundError:
+          if skip_not_found: continue
+          return Future(exc_info=sys.exc_info())
         except:
           return Future(exc_info=sys.exc_info())
       read_value = read_values.get(path)
@@ -88,7 +91,8 @@ class CachingFileSystem(FileSystem):
     if not uncached:
       return Future(value=results)
 
-    uncached_read_futures = self._file_system.Read(uncached.keys())
+    uncached_read_futures = self._file_system.Read(
+        uncached.keys(), skip_not_found=skip_not_found)
     def resolve():
       new_results = uncached_read_futures.Get()
       # Update the cached data in the object store. This is a path -> (read,
diff --git a/chrome/common/extensions/docs/server2/caching_file_system_test.py b/chrome/common/extensions/docs/server2/caching_file_system_test.py
index 316cfba..545a44c 100755
--- a/chrome/common/extensions/docs/server2/caching_file_system_test.py
+++ b/chrome/common/extensions/docs/server2/caching_file_system_test.py
@@ -219,5 +219,21 @@ class CachingFileSystemTest(unittest.TestCase):
     test_fs.IncrementStat()
     run_expecting_stat('1')
 
+  def testSkipNotFound(self):
+    caching_fs = self._CreateCachingFileSystem(TestFileSystem({
+      'bob': {
+        'bob0': 'bob/bob0 contents',
+        'bob1': 'bob/bob1 contents'
+      }
+    }))
+    def read_skip_not_found(paths):
+      return caching_fs.Read(paths, skip_not_found=True).Get()
+    self.assertEqual({}, read_skip_not_found(('grub',)))
+    self.assertEqual({}, read_skip_not_found(('bob/bob2',)))
+    self.assertEqual({
+      'bob/bob0': 'bob/bob0 contents',
+    }, read_skip_not_found(('bob/bob0', 'bob/bob2')))
+
+
 if __name__ == '__main__':
   unittest.main()
diff --git a/chrome/common/extensions/docs/server2/chroot_file_system.py b/chrome/common/extensions/docs/server2/chroot_file_system.py
index 2feae00..af521d8 100644
--- a/chrome/common/extensions/docs/server2/chroot_file_system.py
+++ b/chrome/common/extensions/docs/server2/chroot_file_system.py
@@ -23,7 +23,7 @@ class ChrootFileSystem(FileSystem):
     self._file_system = file_system
     self._root = root.strip('/')
 
-  def Read(self, paths):
+  def Read(self, paths, skip_not_found=False):
     # Maintain reverse mapping so the result can be mapped to the original
     # paths given (the result from |file_system| will include |root| in the
     # result, which would be wrong).
@@ -33,7 +33,8 @@ class ChrootFileSystem(FileSystem):
       prefixed_paths[prefixed] = path
       return prefixed
     future_result = self._file_system.Read(
-        tuple(prefix(path) for path in paths))
+        tuple(prefix(path) for path in paths),
+        skip_not_found=skip_not_found)
     def resolve():
       return dict((prefixed_paths[path], content)
                   for path, content in future_result.Get().iteritems())
diff --git a/chrome/common/extensions/docs/server2/cron.yaml b/chrome/common/extensions/docs/server2/cron.yaml
index 2a8e20a..718a836 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-16-1
+  target: 3-16-2
diff --git a/chrome/common/extensions/docs/server2/empty_dir_file_system.py b/chrome/common/extensions/docs/server2/empty_dir_file_system.py
index 24a5a26..a508747 100644
--- a/chrome/common/extensions/docs/server2/empty_dir_file_system.py
+++ b/chrome/common/extensions/docs/server2/empty_dir_file_system.py
@@ -11,10 +11,11 @@ class EmptyDirFileSystem(FileSystem):
   '''A FileSystem with empty directories. Useful to inject places to disable
   features such as samples.
   '''
-  def Read(self, paths):
+  def Read(self, paths, skip_not_found=False):
     result = {}
     for path in paths:
       if not IsDirectory(path):
+        if skip_not_found: continue
         raise FileNotFoundError('EmptyDirFileSystem cannot read %s' % path)
       result[path] = []
     return Future(value=result)
diff --git a/chrome/common/extensions/docs/server2/fake_url_fetcher.py b/chrome/common/extensions/docs/server2/fake_url_fetcher.py
index a9ae53e3..dbfaa55 100644
--- a/chrome/common/extensions/docs/server2/fake_url_fetcher.py
+++ b/chrome/common/extensions/docs/server2/fake_url_fetcher.py
@@ -25,8 +25,13 @@ class FakeUrlFetcher(object):
     self._async_resolve_count = 0
 
   def _ReadFile(self, filename):
-    with open(os.path.join(self._base_path, filename), 'r') as f:
-      return f.read()
+    # Fake DownloadError, the error that appengine usually raises.
+    class DownloadError(Exception): pass
+    try:
+      with open(os.path.join(self._base_path, filename), 'r') as f:
+        return f.read()
+    except IOError as e:
+      raise DownloadError(e)
 
   def _ListDir(self, directory):
     # In some tests, we need to test listing a directory from the HTML returned
diff --git a/chrome/common/extensions/docs/server2/file_system.py b/chrome/common/extensions/docs/server2/file_system.py
index e21ff1e..3378c65 100644
--- a/chrome/common/extensions/docs/server2/file_system.py
+++ b/chrome/common/extensions/docs/server2/file_system.py
@@ -66,15 +66,19 @@ class StatInfo(object):
 class FileSystem(object):
   '''A FileSystem interface that can read files and directories.
   '''
-  def Read(self, paths):
+  def Read(self, paths, skip_not_found=False):
     '''Reads each file in paths and returns a dictionary mapping the path to the
     contents. If a path in paths ends with a '/', it is assumed to be a
     directory, and a list of files in the directory is mapped to the path.
 
     The contents will be a str.
 
-    If any path cannot be found, raises a FileNotFoundError. This is guaranteed
-    to only happen once the Future has been resolved (Get() called).
+    If any path cannot be found:
+      - If |skip_not_found| is True, the resulting object will not contain any
+        mapping for that path.
+      - Otherwise, and by default, a FileNotFoundError is raised. This is
+        guaranteed to only happen once the Future has been resolved (Get()
+        called).
 
     For any other failure, raises a FileSystemError.
     '''
@@ -82,7 +86,7 @@ class FileSystem(object):
 
   def ReadSingle(self, path):
     '''Reads a single file from the FileSystem. Returns a Future with the same
-    rules as Read().
+    rules as Read(). If |path| is not found raise a FileNotFoundError on Get().
     '''
     AssertIsValid(path)
     read_single = self.Read([path])
diff --git a/chrome/common/extensions/docs/server2/gcs_file_system.py b/chrome/common/extensions/docs/server2/gcs_file_system.py
index 9a16537..36f28fa 100644
--- a/chrome/common/extensions/docs/server2/gcs_file_system.py
+++ b/chrome/common/extensions/docs/server2/gcs_file_system.py
@@ -79,7 +79,7 @@ class CloudStorageFileSystem(FileSystem):
       self._bucket = debug_bucket_prefix + self._bucket
     AssertIsValid(self._bucket)
 
-  def Read(self, paths):
+  def Read(self, paths, skip_not_found=False):
     def resolve():
       try:
         result = {}
diff --git a/chrome/common/extensions/docs/server2/github_file_system.py b/chrome/common/extensions/docs/server2/github_file_system.py
index 57cd698..eaa08ab 100644
--- a/chrome/common/extensions/docs/server2/github_file_system.py
+++ b/chrome/common/extensions/docs/server2/github_file_system.py
@@ -146,7 +146,7 @@ class GithubFileSystem(FileSystem):
     # Remove all files not directly in this directory.
     return [f for f in filenames if f[:-1].count('/') == 0]
 
-  def Read(self, paths):
+  def Read(self, paths, skip_not_found=False):
     version = self.Stat(ZIP_KEY).version
     if version != self._version:
       self._GetZip(version)
diff --git a/chrome/common/extensions/docs/server2/local_file_system.py b/chrome/common/extensions/docs/server2/local_file_system.py
index fff30a2..8655cfe 100644
--- a/chrome/common/extensions/docs/server2/local_file_system.py
+++ b/chrome/common/extensions/docs/server2/local_file_system.py
@@ -76,7 +76,7 @@ class LocalFileSystem(FileSystem):
   def Create(*path):
     return LocalFileSystem(ChromiumPath(*path))
 
-  def Read(self, paths):
+  def Read(self, paths, skip_not_found=False):
     def resolve():
       result = {}
       for path in paths:
diff --git a/chrome/common/extensions/docs/server2/mock_file_system.py b/chrome/common/extensions/docs/server2/mock_file_system.py
index 9f04cd1..e15f58c 100644
--- a/chrome/common/extensions/docs/server2/mock_file_system.py
+++ b/chrome/common/extensions/docs/server2/mock_file_system.py
@@ -37,12 +37,12 @@ class MockFileSystem(FileSystem):
   # FileSystem implementation.
   #
 
-  def Read(self, paths):
+  def Read(self, paths, skip_not_found=False):
     '''Reads |paths| from |_file_system|, then applies the most recent update
     from |_updates|, if any.
     '''
     self._read_count += 1
-    future_result = self._file_system.Read(paths)
+    future_result = self._file_system.Read(paths, skip_not_found=skip_not_found)
     def resolve():
       self._read_resolve_count += 1
       result = future_result.Get()
diff --git a/chrome/common/extensions/docs/server2/new_github_file_system.py b/chrome/common/extensions/docs/server2/new_github_file_system.py
index e9307b7..5aa1c2d 100644
--- a/chrome/common/extensions/docs/server2/new_github_file_system.py
+++ b/chrome/common/extensions/docs/server2/new_github_file_system.py
@@ -240,7 +240,7 @@ class GithubFileSystem(FileSystem):
   def Refresh(self):
     return self.ReadSingle('')
 
-  def Read(self, paths):
+  def Read(self, paths, skip_not_found=False):
     '''Returns a directory mapping |paths| to the contents of the file at each
     path. If path ends with a '/', it is treated as a directory and is mapped to
     a list of filenames in that directory.
diff --git a/chrome/common/extensions/docs/server2/offline_file_system.py b/chrome/common/extensions/docs/server2/offline_file_system.py
index 2096de7..4a5e7ec 100644
--- a/chrome/common/extensions/docs/server2/offline_file_system.py
+++ b/chrome/common/extensions/docs/server2/offline_file_system.py
@@ -13,7 +13,8 @@ class OfflineFileSystem(FileSystem):
   def __init__(self, fs):
     self._fs = fs
 
-  def Read(self, paths):
+  def Read(self, paths, skip_not_found=False):
+    if skip_not_found: return Future(value={})
     def raise_file_not_found():
       raise FileNotFoundError('File system is offline, cannot read %s' % paths)
     return Future(callback=raise_file_not_found)
diff --git a/chrome/common/extensions/docs/server2/patched_file_system.py b/chrome/common/extensions/docs/server2/patched_file_system.py
index 02fa27c..728f104 100644
--- a/chrome/common/extensions/docs/server2/patched_file_system.py
+++ b/chrome/common/extensions/docs/server2/patched_file_system.py
@@ -39,7 +39,7 @@ class PatchedFileSystem(FileSystem):
     self._base_file_system = base_file_system
     self._patcher = patcher
 
-  def Read(self, paths):
+  def Read(self, paths, skip_not_found=False):
     patched_files = set()
     added, deleted, modified = self._patcher.GetPatchedFiles()
     if set(paths) & set(deleted):
@@ -52,7 +52,8 @@ class PatchedFileSystem(FileSystem):
     patched_paths = file_paths & patched_files
     unpatched_paths = file_paths - patched_files
     return Future(callback=_GetAsyncFetchCallback(
-        self._base_file_system.Read(unpatched_paths),
+        self._base_file_system.Read(unpatched_paths,
+                                    skip_not_found=skip_not_found),
         self._patcher.Apply(patched_paths, self._base_file_system),
         self._TryReadDirectory(dir_paths),
         self))
diff --git a/chrome/common/extensions/docs/server2/subversion_file_system.py b/chrome/common/extensions/docs/server2/subversion_file_system.py
index 21d1165..4996f6c 100644
--- a/chrome/common/extensions/docs/server2/subversion_file_system.py
+++ b/chrome/common/extensions/docs/server2/subversion_file_system.py
@@ -95,7 +95,7 @@ def _CreateStatInfo(html):
 
   return StatInfo(parent_version, child_versions)
 
-def _GetAsyncFetchCallback(paths, fetcher, args=None):
+def _GetAsyncFetchCallback(paths, fetcher, args=None, skip_not_found=False):
   def apply_args(path):
     return path if args is None else '%s?%s' % (path, args)
 
@@ -115,10 +115,12 @@ def _GetAsyncFetchCallback(paths, fetcher, args=None):
       try:
         result = future.Get()
       except Exception as e:
+        if skip_not_found and IsDownloadError(e): continue
         exc_type = FileNotFoundError if IsDownloadError(e) else FileSystemError
         raise exc_type('%s fetching %s for Get: %s' %
                        (type(e).__name__, path, traceback.format_exc()))
       if result.status_code == 404:
+        if skip_not_found: continue
         raise FileNotFoundError('Got 404 when fetching %s for Get, content %s' %
             (path, result.content))
       if result.status_code != 200:
@@ -153,14 +155,16 @@ class SubversionFileSystem(FileSystem):
     self._svn_path = svn_path
     self._revision = revision
 
-  def Read(self, paths):
+  def Read(self, paths, skip_not_found=False):
     args = None
     if self._revision is not None:
       # |fetcher| gets from svn.chromium.org which uses p= for version.
       args = 'p=%s' % self._revision
-    return Future(callback=_GetAsyncFetchCallback(paths,
-                                                  self._file_fetcher,
-                                                  args=args))
+    return Future(callback=_GetAsyncFetchCallback(
+        paths,
+        self._file_fetcher,
+        args=args,
+        skip_not_found=skip_not_found))
 
   def Refresh(self):
     return Future(value=())
diff --git a/chrome/common/extensions/docs/server2/subversion_file_system_test.py b/chrome/common/extensions/docs/server2/subversion_file_system_test.py
index 2dbcb70..ea9dafa 100755
--- a/chrome/common/extensions/docs/server2/subversion_file_system_test.py
+++ b/chrome/common/extensions/docs/server2/subversion_file_system_test.py
@@ -130,5 +130,12 @@ class SubversionFileSystemTest(unittest.TestCase):
     self.assertEqual('193838', dir_stat.version)
     self.assertEqual({}, dir_stat.child_versions)
 
+  def testSkipNotFound(self):
+    file_system, _ = _CreateSubversionFileSystem(
+        _SUBVERSION_FILE_SYSTEM_TEST_DATA)
+    self.assertEqual({}, file_system.Read(('fakefile',),
+                                          skip_not_found=True).Get())
+
+
 if __name__ == '__main__':
   unittest.main()
diff --git a/chrome/common/extensions/docs/server2/test_file_system.py b/chrome/common/extensions/docs/server2/test_file_system.py
index fecc7c5..227b4da 100644
--- a/chrome/common/extensions/docs/server2/test_file_system.py
+++ b/chrome/common/extensions/docs/server2/test_file_system.py
@@ -114,9 +114,10 @@ class TestFileSystem(FileSystem):
   # FileSystem implementation.
   #
 
-  def Read(self, paths):
+  def Read(self, paths, skip_not_found=False):
     for path in paths:
       if path not in self._path_values:
+        if skip_not_found: continue
         return FileNotFoundError.RaiseInFuture(
             '%s not in %s' % (path, '\n'.join(self._path_values)))
     return Future(value=dict((k, v) for k, v in self._path_values.iteritems()