Docserver: Additional new github file system cleanup

NOTRY=true

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

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

5 files changed, 69 insertions, 47 deletions

diff --git a/chrome/common/extensions/docs/server2/fake_url_fetcher.py b/chrome/common/extensions/docs/server2/fake_url_fetcher.py
index b6d943b..d6e2573 100644
--- a/chrome/common/extensions/docs/server2/fake_url_fetcher.py
+++ b/chrome/common/extensions/docs/server2/fake_url_fetcher.py
@@ -2,16 +2,20 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+from functools import partial
 import os
 
 from future import Future
+from local_file_system import LocalFileSystem
+
 
 class _Response(object):
-  def __init__(self):
-    self.content = ''
+  def __init__(self, content=''):
+    self.content = content
     self.headers = { 'content-type': 'none' }
     self.status_code = 200
 
+
 class FakeUrlFetcher(object):
   def __init__(self, base_path):
     self._base_path = base_path
@@ -49,3 +53,27 @@ class FakeUrlFetcher(object):
     else:
       result.content = self._ReadFile(url)
     return result
+
+
+class FakeURLFSFetcher(object):
+  '''Use a file_system to resolve fake fetches. Mimics the interface of Google
+  Appengine's urlfetch.
+  '''
+  @staticmethod
+  def Create(file_system):
+    return partial(FakeURLFSFetcher, file_system)
+
+  @staticmethod
+  def CreateLocal():
+    return partial(FakeURLFSFetcher, LocalFileSystem(''))
+
+  def __init__(self, file_system, base_path):
+    self._base_path = base_path
+    self._file_system = file_system
+
+  def FetchAsync(self, url, **kwargs):
+    return Future(value=self.Fetch(url))
+
+  def Fetch(self, url, **kwargs):
+    return _Response(
+        self._file_system.ReadSingle(self._base_path + '/' + url, binary=True))
diff --git a/chrome/common/extensions/docs/server2/file_system.py b/chrome/common/extensions/docs/server2/file_system.py
index 8ff964b..9f02a19 100644
--- a/chrome/common/extensions/docs/server2/file_system.py
+++ b/chrome/common/extensions/docs/server2/file_system.py
@@ -120,3 +120,12 @@ class FileSystem(object):
 
     for walkinfo in walk(root):
       yield walkinfo
+
+  def GetDebugString(self):
+    return ''
+
+  def __repr__(self):
+    return '<%s%s>' % (self.__class__.__name__, self.GetDebugString())
+
+  def __str__(self):
+    return repr(self)
diff --git a/chrome/common/extensions/docs/server2/future.py b/chrome/common/extensions/docs/server2/future.py
index 8048248..cb0a323 100644
--- a/chrome/common/extensions/docs/server2/future.py
+++ b/chrome/common/extensions/docs/server2/future.py
@@ -6,6 +6,17 @@ import sys
 
 _no_value = object()
 
+
+class Gettable(object):
+  '''Allows a Future to accept a callable as a delegate. Wraps |f| in a .Get
+  interface required by Future.
+  '''
+  def __init__(self, f, *args):
+    self._g = lambda: f(*args)
+  def Get(self):
+    return self._g()
+
+
 class Future(object):
   '''Stores a value, error, or delegate to be used later.
   '''
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 8ac2ba4..5739585 100644
--- a/chrome/common/extensions/docs/server2/new_github_file_system.py
+++ b/chrome/common/extensions/docs/server2/new_github_file_system.py
@@ -10,8 +10,9 @@ from zipfile import BadZipfile, ZipFile
 import appengine_blobstore as blobstore
 from appengine_url_fetcher import AppEngineUrlFetcher
 from appengine_wrappers import urlfetch
+from docs_server_utils import StringIdentity
 from file_system import FileNotFoundError, FileSystem, StatInfo
-from future import Future
+from future import Future, Gettable
 from object_store_creator import ObjectStoreCreator
 import url_constants
 
@@ -26,19 +27,9 @@ def _LoadCredentials(object_store_creator):
       app_version=None,
       category='password',
       start_empty=False)
-  # return 'test_username', 'test_password'
   password_data = password_store.GetMulti(('username', 'password')).Get()
-  return password_data.get('username'), password_data.get('password')
-
 
-class _Gettable(object):
-  '''Wrap a callable |f| such that calling .Get on a _Gettable is the same as
-  calling |f| directly.
-  '''
-  def __init__(self, f, *args):
-    self._g = lambda: f(*args)
-  def Get(self):
-    return self._g()
+  return password_data.get('username'), password_data.get('password')
 
 
 class GithubFileSystem(FileSystem):
@@ -57,7 +48,7 @@ class GithubFileSystem(FileSystem):
         AppEngineUrlFetcher)
 
   @staticmethod
-  def ForTest(repo, fake_fetcher, path=None):
+  def ForTest(repo, fake_fetcher, path=None, object_store_creator=None):
     '''Creates a GithubFIleSystem that can be used for testing. It reads zip
     files and commit data from server2/test_data/github_file_system/test_owner
     instead of github.com. It reads from files specified by |repo|.
@@ -66,7 +57,7 @@ class GithubFileSystem(FileSystem):
         path if path is not None else 'test_data/github_file_system',
         'test_owner',
         repo,
-        ObjectStoreCreator.ForTest(),
+        object_store_creator or ObjectStoreCreator.ForTest(),
         fake_fetcher)
 
   def __init__(self, base_url, owner, repo, object_store_creator, Fetcher):
@@ -105,7 +96,10 @@ class GithubFileSystem(FileSystem):
     result = self._fetcher.Fetch(
         'commits/HEAD', username=self._username, password=self._password)
 
-    return json.loads(result.content)['commit']['tree']['sha']
+    try:
+      return json.loads(result.content)['commit']['tree']['sha']
+    except (KeyError, ValueError):
+      logging.warn('Error parsing JSON from repo %s' % self._repo_url)
 
   def Refresh(self):
     '''Compares the cached and live stat versions to see if the cached
@@ -141,7 +135,7 @@ class GithubFileSystem(FileSystem):
     if version != self._stat_cache.Get('stat').Get():
       fetch = self._fetcher.FetchAsync(
           'zipball', username=self._username, password=self._password)
-      return Future(delegate=_Gettable(lambda: persist_fetch(fetch)))
+      return Future(delegate=Gettable(lambda: persist_fetch(fetch)))
 
     return Future(value=None)
 
@@ -204,5 +198,8 @@ class GithubFileSystem(FileSystem):
 
     return StatInfo(version, child_paths or None)
 
+  def GetDebugString(self):
+    return ' %s: %s' % (self._repo_key, self._repo_url)
+
   def GetIdentity(self):
-    return '%s(%s)' % (self.__class__.__name__, self._repo_key)
+    return '%s' % StringIdentity(self.__class__.__name__ + self._repo_key)
diff --git a/chrome/common/extensions/docs/server2/new_github_file_system_test.py b/chrome/common/extensions/docs/server2/new_github_file_system_test.py
index 613a9e9..92bdaa6 100755
--- a/chrome/common/extensions/docs/server2/new_github_file_system_test.py
+++ b/chrome/common/extensions/docs/server2/new_github_file_system_test.py
@@ -10,8 +10,7 @@ from zipfile import ZipFile
 
 from caching_file_system import CachingFileSystem
 from file_system import FileNotFoundError, StatInfo
-from functools import partial
-from future import Future
+from fake_url_fetcher import FakeURLFSFetcher
 from local_file_system import LocalFileSystem
 from new_github_file_system import GithubFileSystem
 from object_store_creator import ObjectStoreCreator
@@ -31,31 +30,9 @@ def _ZipFromFiles(file_dict):
   return string.getvalue()
 
 
-class _FakeURLFetcher(object):
-  class _Response(object):
-    def __init__(self, content):
-      self.content = content
-
-  @staticmethod
-  def Create(file_system=None):
-    return partial(_FakeURLFetcher, file_system or LocalFileSystem(''))
-
-  def __init__(self, file_system, base_path):
-    self._base_path = base_path
-    self._file_system = file_system
-
-  def FetchAsync(self, url, **kwargs):
-    return Future(value=self.Fetch(url))
-
-  def Fetch(self, url, **kwargs):
-    response = self._Response(
-        self._file_system.ReadSingle(self._base_path + '/' + url, binary=True))
-    return response
-
-
 class TestGithubFileSystem(unittest.TestCase):
   def setUp(self):
-    self._gfs = GithubFileSystem.ForTest('repo', _FakeURLFetcher.Create())
+    self._gfs = GithubFileSystem.ForTest('repo', FakeURLFSFetcher.CreateLocal())
     # Start and finish the repository load.
     self._cgfs = CachingFileSystem(
         self._gfs, ObjectStoreCreator.ForTest())
@@ -86,8 +63,8 @@ class TestGithubFileSystem(unittest.TestCase):
   def testReads(self):
     self._gfs.Refresh().Get()
     expected = {
-      '/src/': sorted(['hello.notpy', '__init__.notpy']),
-      '/': sorted(['requirements.txt', '.gitignore', 'README.md', 'src/'])
+        '/src/': sorted(['hello.notpy', '__init__.notpy']),
+        '/': sorted(['requirements.txt', '.gitignore', 'README.md', 'src/'])
     }
 
     read = self._gfs.Read(['/', '/src/']).Get()
@@ -168,7 +145,7 @@ class TestGithubFileSystem(unittest.TestCase):
     test_file_system = TestFileSystem(test_files)
 
     gfs = GithubFileSystem.ForTest(
-        'changing-repo', _FakeURLFetcher.Create(test_file_system), path='')
+        'changing-repo', FakeURLFSFetcher.Create(test_file_system), path='')
 
     gfs.Refresh().Get()
     initial_dir_read = sorted(gfs.ReadSingle('/'))