Docserver: Override Walk in CachingFileSystem

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

Index: chrome/common/extensions/docs/server2/caching_file_system.py
diff --git a/chrome/common/extensions/docs/server2/caching_file_system.py b/chrome/common/extensions/docs/server2/caching_file_system.py
index 016cac3c7579d94524c98a9eab27c177111a174f..7915d177a0f9d9c7538b6f36988c4ddf35d366ab 100644
--- a/chrome/common/extensions/docs/server2/caching_file_system.py
+++ b/chrome/common/extensions/docs/server2/caching_file_system.py
@@ -7,7 +7,7 @@ import sys
 
 from file_system import FileSystem, StatInfo, FileNotFoundError
 from future import Future
-from path_util import IsDirectory, ToDirectory
+from path_util import AssertIsDirectory, IsDirectory, ToDirectory
 from third_party.json_schema_compiler.memoize import memoize
 
 
@@ -29,6 +29,7 @@ class CachingFileSystem(FileSystem):
     # which starting empty is designed for. Without this optimisation, cron
     # runs are extra slow.
     self._read_object_store = create_object_store('read', start_empty=False)
+    self._walk_cache = create_object_store('walk')

[not at google - send to devlin, 2014/08/29 05:00:14]

Should this be start_empty=False for the same reason as the 'read' cache is?

[ahernandez, 2014/08/29 17:20:00]

Probably, I wasn't sure.

 
   def Refresh(self):
     return self._file_system.Refresh()
@@ -140,6 +141,40 @@ class CachingFileSystem(FileSystem):
     return self._file_system.Read(set(paths) - set(up_to_date_data.iterkeys()),
                                   skip_not_found=skip_not_found).Then(next)
 
+  def Walk(self, root, depth=-1):

[not at google - send to devlin, 2014/08/29 05:00:13]

I'm looking at this quite late at night, so, apologies if this is silly - but
could you write this closer to a delegate style?

Like could this be basically a wrapper around self._file_system.Walk() which
cancels the walk (and runs any cached data) if it runs into anything that isn't
new?

Can't think of quite what that would look like, but it's a shame needing to
implement the walk logic again.

Or maybe you could rephrase FileSystem.Walk so that CachingFileSystem.Walk can
hook into it somehow?

I think it's too late at night for me to review this part of the CL :)

+    '''Overrides FileSystem.Walk() to provide caching functionality.
+    '''
+    AssertIsDirectory(root)
+    basepath = root
+
+    def walk(root, depth):
+      if depth == 0:
+        return
+      AssertIsDirectory(root)
+      res = self._walk_cache.Get(root).Get()
+      cache_stat = self._stat_object_store.Get(root).Get()

[ahernandez, 2014/08/29 02:15:11]

These don't need to be Then-ified, right?

+      root_stat = self.Stat(root)
+
+      if res and cache_stat.version == root_stat.version:
+        dirs, files = res
+      else:
+        dirs, files = [], []
+        for f in self.ReadSingle(root).Get():
+          if IsDirectory(f):
+            dirs.append(f)
+          else:
+            files.append(f)
+        self._walk_cache.Set(root, (dirs, files))
+
+      yield root[len(basepath):].rstrip('/'), dirs, files
+
+      for d in dirs:
+        for walkinfo in walk(root + d, depth - 1):
+          yield walkinfo
+
+    for walkinfo in walk(root, depth):
+      yield walkinfo
+
   def GetIdentity(self):
     return self._file_system.GetIdentity()