Docserver: Fixes for GitilesFileSystem

This patch resolves a few issues with GitilesFileSystem.

BUG=404239
NOTRY=True

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

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

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

@@ -79,14 +79,20 @@ class _FakeSubversionServer(_FakeFetcher):
 
 _GITILES_BASE_RE = re.escape(url_constants.GITILES_BASE)
 _GITILES_BRANCH_BASE_RE = re.escape(url_constants.GITILES_BRANCH_BASE)
-_GITILES_URL_TO_PATH_PATTERN = re.compile(
-    r'(' + _GITILES_BASE_RE + r'|' + _GITILES_BRANCH_BASE_RE + r').+?/(.*)')
+# NOTE: _GITILES_BRANCH_BASE_RE must be first, because _GITILES_BASE_RE is
+# a more general pattern.
+_GITILES_URL_RE = r'(%s|%s)/' % (_GITILES_BRANCH_BASE_RE, _GITILES_BASE_RE)
+_GITILES_URL_TO_COMMIT_PATTERN = re.compile(
+    r'%s[^/]+\?format=JSON' % _GITILES_URL_RE)
+_GITILES_URL_TO_PATH_PATTERN = re.compile(r'%s.+?/(.*)' % _GITILES_URL_RE)
 def _ExtractPathFromGitilesUrl(url):
   return _GITILES_URL_TO_PATH_PATTERN.match(url).group(2)
 
 
 class _FakeGitilesServer(_FakeFetcher):
   def fetch(self, url):
+    if _GITILES_URL_TO_COMMIT_PATTERN.match(url) is not None:
+      return json.dumps({'commit': '1' * 40})
     path = _ExtractPathFromGitilesUrl(url)
     chromium_path = ChromiumPath(path)
     if self._IsDir(chromium_path):
@@ -97,9 +103,11 @@ class _FakeGitilesServer(_FakeFetcher):
       for f in self._ListDir(chromium_path):
         if f.startswith('.'):
           continue
+        f_path = os.path.join(chromium_path, f)
         jsn['entries'].append({
-          'id': self._Stat(os.path.join(chromium_path, f)),
-          'name': f
+          'id': self._Stat(f_path),
+          'name': f,
+          'type': 'tree' if self._IsDir(f_path) else 'blob'
         })
       return json.dumps(jsn)
     try:

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

@@ -16,7 +16,7 @@ from file_system import (FileNotFoundError,
                          FileSystemError,
                          StatInfo)
 from future import All, Future
-from path_util import AssertIsValid, IsDirectory, SplitParent, ToDirectory
+from path_util import AssertIsValid, IsDirectory, ToDirectory
 from third_party.json_schema_compiler.memoize import memoize
 from url_constants import GITILES_BASE, GITILES_BRANCH_BASE
 
@@ -104,9 +104,11 @@ class GitilesFileSystem(FileSystem):
     #         "name": ".gitignore"
     #       },
     #       ...
+    #     ]
     #   }
     def list_dir(json_data):
-      return [e['name'] for e in _ParseGitilesJson(json_data)['entries']]
+      entries = _ParseGitilesJson(json_data).get('entries', [])
+      return [e['name'] + ('/' if e['type'] == 'tree' else '') for e in entries]
 
     def fixup_url_format(path):
       # By default, Gitiles URLs display resources in HTML. To get resources
@@ -135,8 +137,29 @@ class GitilesFileSystem(FileSystem):
     return Future(value=())
 
   @memoize
-  def GetCommitID(self):
-    '''Returns a future that resolves to the commit ID for this branch.
+  def _GetCommitInfo(self, key):
+    '''Gets the commit information specified by |key|.
+
+    The JSON view for commit info looks like:
+      {
+        "commit": "8fd578e1a7b142cd10a4387861f05fb9459b69e2", # Commit ID.
+        "tree": "3ade65d8a91eadd009a6c9feea8f87db2c528a53",   # Tree ID.
+        "parents": [
+          "a477c787fe847ae0482329f69b39ce0fde047359" # Previous commit ID.
+        ],
+        "author": {
+          "name": "...",
+          "email": "...",
+          "time": "Tue Aug 12 17:17:21 2014"
+        },
+        "committer": {
+          "name": "...",
+          "email": "...",
+          "time": "Tue Aug 12 17:18:28 2014"
+        },
+        "message": "...",
+        "tree_diff": [...]
+      }
     '''
     # Commit information for a branch is obtained by appending '?format=JSON'
     # to the branch URL. Note that '<gitiles_url>/<branch>?format=JSON' is
@@ -145,17 +168,12 @@ class GitilesFileSystem(FileSystem):
     # commit info JSON content.
     fetch_future = self._fetcher.FetchAsync(self._base_url + _JSON_FORMAT)
     content_future = self._ResolveFetchContent(self._base_url, fetch_future)
-    # The commit info JSON looks like:
-    #
-    #   {
-    #     "commit": "8fd578e1a7b142cd10a4387861f05fb9459b69e2", # Commit ID.
-    #     "tree": "3ade65d8a91eadd009a6c9feea8f87db2c528a53",   # Tree ID.
-    #     "author": {...},
-    #     "committer": {...},
-    #     "message": <codereview message>,
-    #     ...
-    #   }
-    return content_future.Then(lambda json: _ParseGitilesJson(json)['commit'])
+    return content_future.Then(lambda json: _ParseGitilesJson(json)[key])
+
+  def GetCommitID(self):
+    '''Returns a future that resolves to the commit ID for this branch.
+    '''
+    return self._GetCommitInfo('commit')
 
   def StatAsync(self, path):
     dir_, filename = posixpath.split(path)
@@ -173,5 +191,10 @@ class GitilesFileSystem(FileSystem):
     return self._ResolveFetchContent(path, fetch_future).Then(stat)
 
   def GetIdentity(self):
-    return '@'.join((self.__class__.__name__,
-                     StringIdentity(self._commit or self._branch)))
+    # NOTE: Do not use commit information to create the string identity.
+    # Doing so will mess up caching.
+    if self._commit is None and self._branch != 'master':
+      str_id = GITILES_BRANCH_BASE
+    else:
+      str_id = GITILES_BASE
+    return '@'.join((self.__class__.__name__, StringIdentity(str_id)))

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

@@ -13,12 +13,14 @@ from future import Future
 from gitiles_file_system import (_CreateStatInfo,
                                  _ParseGitilesJson,
                                  GitilesFileSystem)
+from path_util import IsDirectory
 from test_file_system import TestFileSystem
 from test_util import ReadFile
 
 
 _BASE_URL = ''
 _REAL_DATA_DIR = 'chrome/common/extensions/docs/templates/public/extensions/'
+_TEST_DATA = (SERVER2, 'test_data', 'gitiles_file_system', 'public_extensions')
 # GitilesFileSystem expects file content to be encoded in base64.
 _TEST_FS = {
   'test1.txt': base64.b64encode('test1'),
@@ -54,8 +56,7 @@ class _FakeGitilesFetcher(object):
       # Fetch urls are of the form <base_url>/<path>. We only want <path>.
       path = path.split('/', 1)[1]
       if path == _REAL_DATA_DIR:
-        return _Response(ReadFile(SERVER2, 'test_data', 'gitiles_file_system',
-            'public_extensions'))
+        return _Response(ReadFile(*_TEST_DATA))
       # ALWAYS skip not found here.
       content = self._fs.Read((path,),
                               skip_not_found=True).Get().get(path, None)
@@ -65,7 +66,11 @@ class _FakeGitilesFetcher(object):
       # GitilesFS expects directory content as a JSON string.
       if 'JSON' in fmt:
         content = json.dumps({
-          'entries': [{'name': name} for name in content]
+          'entries': [{
+            # GitilesFS expects directory names to not have a trailing '/'.
+            'name': name.rstrip('/'),
+            'type': 'tree' if IsDirectory(name) else 'blob'
+          } for name in content]
         })
       return _Response(content)
     return Future(callback=resolve)
@@ -74,7 +79,7 @@ class _FakeGitilesFetcher(object):
 class GitilesFileSystemTest(unittest.TestCase):
   def setUp(self):
     fetcher = _FakeGitilesFetcher(TestFileSystem(_TEST_FS))
-    self._gitiles_fs = GitilesFileSystem(fetcher, _BASE_URL, '', '')
+    self._gitiles_fs = GitilesFileSystem(fetcher, _BASE_URL, 'master', None)
 
   def testParseGitilesJson(self):
     test_json = '\n'.join([
@@ -144,6 +149,15 @@ class GitilesFileSystemTest(unittest.TestCase):
     self.assertEqual(self._gitiles_fs.Stat(_REAL_DATA_DIR).version,
                      'ec21e736a3f00db2c0580e3cf71d91951656caec')
 
+  def testGetIdentity(self):
+    # Test that file systems at different commits still have the same identity.
+    other_gitiles_fs = GitilesFileSystem.Create(commit='abcdefghijklmnop')
+    self.assertEqual(self._gitiles_fs.GetIdentity(),
+                     other_gitiles_fs.GetIdentity())
+
+    yet_another_gitiles_fs = GitilesFileSystem.Create(branch='different')
+    self.assertNotEqual(self._gitiles_fs.GetIdentity(),
+                        yet_another_gitiles_fs.GetIdentity())
 
 if __name__ == '__main__':
   unittest.main()