Docserver: Generate a table of extension/app API owners

BUG=400760
NOTRY=True

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

Cr-Commit-Position: refs/heads/master@{#292223}

chrome/common/extensions/docs/server2/app.yaml

 application: chrome-apps-doc
-version: 3-40-0
+version: 3-40-1
 runtime: python27
 api_version: 1
 threadsafe: false

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

@@ -207,6 +207,7 @@ class CompiledFileSystem(object):
       cache_data = self._compilation_function(path, files)
       self._file_object_store.Set(path, _CacheEntry(cache_data, version))
       return cache_data
+
     return self._file_system.ReadSingle(
         path, skip_not_found=skip_not_found).Then(compile_)
 

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-40-0
+  target: 3-40-1

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

@@ -13,6 +13,7 @@ from environment import GetAppVersion
 from extensions_paths import (
     APP_YAML, CONTENT_PROVIDERS, CHROME_EXTENSIONS, PUBLIC_TEMPLATES, SERVER2,
     STATIC_DOCS)
+from fake_fetchers import ConfigureFakeFetchers
 from gcs_file_system_provider import CloudStorageFileSystemProvider
 from github_file_system_provider import GithubFileSystemProvider
 from host_file_system_provider import HostFileSystemProvider
@@ -93,47 +94,64 @@ class CronServletTest(unittest.TestCase):
   @IgnoreMissingContentProviders
   def testSafeRevision(self):
     test_data = {
-      'api': {
-        '_api_features.json': '{}',
-        '_manifest_features.json': '{}',
-        '_permission_features.json': '{}',
+      'extensions': {
+        'browser': {
+          'api': {}
+        }
       },
-      'docs': {
-        'examples': {
-          'examples.txt': 'examples.txt contents'
-        },
-        'server2': {
-          'app.yaml': AppYamlHelper.GenerateAppYaml('2-0-8')
+      'chrome': {
+        'browser': {
+          'extensions': {
+            'OWNERS': '',
+            'api': {}
+          }
         },
-        'static': {
-          'static.txt': 'static.txt contents'
-        },
-        'templates': {
-          'articles': {
-            'activeTab.html': 'activeTab.html contents'
-          },
-          'intros': {
-            'browserAction.html': 'activeTab.html contents'
-          },
-          'private': {
-            'table_of_contents.html': 'table_of_contents.html contents',
-          },
-          'public': {
-            'apps': {
-              'storage.html': '<h1>storage.html</h1> contents'
-            },
-            'extensions': {
-              'storage.html': '<h1>storage.html</h1> contents'
+        'common': {
+          'extensions': {
+            'api': {
+              '_api_features.json': '{}',
+              '_manifest_features.json': '{}',
+              '_permission_features.json': '{}',
             },
-          },
-          'json': {
-            'chrome_sidenav.json': '{}',
-            'content_providers.json': ReadFile(CONTENT_PROVIDERS),
-            'manifest.json': '{}',
-            'permissions.json': '{}',
-            'strings.json': '{}',
-            'whats_new.json': '{}',
-          },
+            'docs': {
+              'examples': {
+                'examples.txt': 'examples.txt contents'
+              },
+              'server2': {
+                'app.yaml': AppYamlHelper.GenerateAppYaml('2-0-8')
+              },
+              'static': {
+                'static.txt': 'static.txt contents'
+              },
+              'templates': {
+                'articles': {
+                  'activeTab.html': 'activeTab.html contents'
+                },
+                'intros': {
+                  'browserAction.html': 'activeTab.html contents'
+                },
+                'private': {
+                  'table_of_contents.html': 'table_of_contents.html contents',
+                },
+                'public': {
+                  'apps': {
+                    'storage.html': '<h1>storage.html</h1> contents'
+                  },
+                  'extensions': {
+                    'storage.html': '<h1>storage.html</h1> contents'
+                  },
+                },
+                'json': {
+                  'chrome_sidenav.json': '{}',
+                  'content_providers.json': ReadFile(CONTENT_PROVIDERS),
+                  'manifest.json': '{}',
+                  'permissions.json': '{}',
+                  'strings.json': '{}',
+                  'whats_new.json': '{}',
+                },
+              }
+            }
+          }
         }
       }
     }
@@ -160,8 +178,7 @@ class CronServletTest(unittest.TestCase):
       '''Creates a MockFileSystem at |revision| by applying that many |updates|
       to it.
       '''
-      mock_file_system = MockFileSystem(
-          TestFileSystem(test_data, relative_to=CHROME_EXTENSIONS))
+      mock_file_system = MockFileSystem(TestFileSystem(test_data))
       updates_for_revision = (
           updates if revision is None else updates[:int(revision)])
       for update in updates_for_revision:

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

@@ -6,6 +6,7 @@ from api_data_source import APIDataSource
 from api_list_data_source import APIListDataSource
 from data_source import DataSource
 from manifest_data_source import ManifestDataSource
+from owners_data_source import OwnersDataSource
 from permissions_data_source import PermissionsDataSource
 from samples_data_source import SamplesDataSource
 from sidenav_data_source import SidenavDataSource
@@ -21,6 +22,7 @@ _all_data_sources = {
   'articles': ArticleDataSource,
   'intros': IntroDataSource,
   'manifest_source': ManifestDataSource,
+  'owners': OwnersDataSource,
   'partials': PartialDataSource,
   'permissions': PermissionsDataSource,
   'samples': SamplesDataSource,

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

@@ -10,15 +10,26 @@ from posixpath import join
 EXTENSIONS = 'extensions/common/'
 CHROME_EXTENSIONS = 'chrome/common/extensions/'
 
+BROWSER_EXTENSIONS = 'extensions/browser/'
+BROWSER_CHROME_EXTENSIONS = 'chrome/browser/extensions/'
+
 EXTENSIONS_API = join(EXTENSIONS, 'api/')
 CHROME_API = join(CHROME_EXTENSIONS, 'api/')
 
+BROWSER_EXTENSIONS_API = join(BROWSER_EXTENSIONS, 'api/')
+BROWSER_CHROME_API = join(BROWSER_CHROME_EXTENSIONS, 'api/')
+
 # Note: This determines search order when APIs are resolved in the filesystem.
 API_PATHS = (
   CHROME_API,
   EXTENSIONS_API,
 )
 
+BROWSER_API_PATHS = (
+  BROWSER_CHROME_API,
+  BROWSER_EXTENSIONS_API
+)
+
 DOCS = join(CHROME_EXTENSIONS, 'docs/')
 
 EXAMPLES = join(DOCS, 'examples/')

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

@@ -86,7 +86,8 @@ class FileSystem(object):
 
   def ReadSingle(self, path, skip_not_found=False):
     '''Reads a single file from the FileSystem. Returns a Future with the same
-    rules as Read(). If |path| is not found raise a FileNotFoundError on Get().
+    rules as Read(). If |path| is not found raise a FileNotFoundError on Get(),
+    or if |skip_not_found| is True then return None.
     '''
     AssertIsValid(path)
     read_single = self.Read([path], skip_not_found=skip_not_found)
@@ -154,13 +155,14 @@ class FileSystem(object):
     '''
     raise NotImplementedError(self.__class__)
 
-  def Walk(self, root):
+  def Walk(self, root, depth=-1):
     '''Recursively walk the directories in a file system, starting with root.
 
     Behaviour is very similar to os.walk from the standard os module, yielding
     (base, dirs, files) recursively, where |base| is the base path of |files|,
     |dirs| relative to |root|, and |files| and |dirs| the list of files/dirs in
-    |base| respectively.
+    |base| respectively. If |depth| is specified and greater than 0, Walk will
+    only recurse |depth| times.
 
     Note that directories will always end with a '/', files never will.
 
@@ -170,7 +172,9 @@ class FileSystem(object):
     AssertIsDirectory(root)
     basepath = root
 
-    def walk(root):
+    def walk(root, depth):
+      if depth == 0:
+        return
       AssertIsDirectory(root)
       dirs, files = [], []
 
@@ -183,10 +187,10 @@ class FileSystem(object):
       yield root[len(basepath):].rstrip('/'), dirs, files
 
       for d in dirs:
-        for walkinfo in walk(root + d):
+        for walkinfo in walk(root + d, depth - 1):
           yield walkinfo
 
-    for walkinfo in walk(root):
+    for walkinfo in walk(root, depth):
       yield walkinfo
 
   def __eq__(self, other):

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

@@ -59,6 +59,31 @@ class FileSystemTest(unittest.TestCase):
     self.assertEqual(sorted(expected_files), sorted(all_files))
     self.assertEqual(sorted(expected_dirs), sorted(all_dirs))
 
+  def testWalkDepth(self):
+    all_dirs = []
+    all_files = []
+    for root, dirs, files in file_system.Walk('', depth=0):
+      all_dirs.extend(dirs)
+      all_files.extend(files)
+    self.assertEqual([], all_dirs)
+    self.assertEqual([], all_files)
+
+    for root, dirs, files in file_system.Walk('', depth=1):
+      all_dirs.extend(dirs)
+      all_files.extend(files)
+    self.assertEqual(['templates/'], all_dirs)
+    self.assertEqual(['file.txt'], all_files)
+
+    all_dirs = []
+    all_files = []
+    for root, dirs, files in file_system.Walk('', depth=2):
+      all_dirs.extend(dirs)
+      all_files.extend(files)
+    self.assertEqual(sorted(['templates/', 'public/', 'json/']),
+                     sorted(all_dirs))
+    self.assertEqual(sorted(['file.txt', 'README']), sorted(all_files))
+
+
   def testSubWalk(self):
     expected_files = set([
       '/redirects.json',

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

+# Copyright 2014 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+from operator import itemgetter
+import random
+
+from data_source import DataSource
+from docs_server_utils import MarkLast
+from extensions_paths import BROWSER_API_PATHS, BROWSER_CHROME_EXTENSIONS
+from future import All
+from path_util import Join, Split
+
+
+_COMMENT_START_MARKER = '#'
+_CORE_OWNERS = 'Core Extensions/Apps Owners'
+_OWNERS = 'OWNERS'
+
+
+# Public for testing.
+def ParseOwnersFile(content, randomize):
+  '''Returns a tuple (owners, notes), where
+  |owners| is a list of dicts formed from the owners in |content|,
+  |notes| is a string formed from the comments in |content|.
+  '''
+  if content is None:
+    return [], 'Use one of the ' + _CORE_OWNERS + '.'
+  owners = []
+  notes = []
+  for line in content.splitlines():
+    if line == '':
+      continue
+    if line.startswith(_COMMENT_START_MARKER):
+      notes.append(line[len(_COMMENT_START_MARKER):].lstrip())
+    else:
+      # TODO(ahernandez): Mark owners no longer on the project.
+      owners.append({'email': line, 'username': line[:line.find('@')]})
+  # Randomize the list so owners toward the front of the list aren't
+  # diproportionately inundated with reviews.
+  if randomize:
+    random.shuffle(owners)
+  MarkLast(owners)
+  return owners, '\n'.join(notes)
+
+
+class OwnersDataSource(DataSource):
+  def __init__(self, server_instance, _, randomize=True):
+    self._host_fs = server_instance.host_file_system_provider.GetTrunk()
+    self._cache = server_instance.object_store_creator.Create(OwnersDataSource)
+    self._owners_fs = server_instance.compiled_fs_factory.Create(
+        self._host_fs, self._CreateAPIEntry, OwnersDataSource)
+    self._randomize = randomize
+
+  def _CreateAPIEntry(self, path, content):
+    '''Creates a dict with owners information for an API, specified
+    by |owners_file|.
+    '''
+    owners, notes = ParseOwnersFile(content, self._randomize)
+    api_name = Split(path)[-2][:-1]
+    return {
+      'apiName': api_name,
+      'owners': owners,
+      'notes': notes,
+      'id': api_name
+    }
+
+  def _CollectOwnersData(self):
+    '''Walks through the file system, collecting owners data from
+    API directories.
+    '''
+    def collect(api_owners):
+      if api_owners is not None:
+        return api_owners
+
+      # Get API owners from every OWNERS file that exists.
+      api_owners = []
+      for root in BROWSER_API_PATHS:
+        for base, dirs, _ in self._host_fs.Walk(root, depth=1):
+          for dir_ in dirs:
+            owners_file = Join(root, base, dir_, _OWNERS)
+            api_owners.append(
+                self._owners_fs.GetFromFile(owners_file, skip_not_found=True))
+
+      # Add an entry for the core extensions/apps owners.
+      def fix_core_owners(entry):
+        entry['apiName'] = _CORE_OWNERS
+        entry['id'] = 'core'
+        return entry
+
+      owners_file = Join(BROWSER_CHROME_EXTENSIONS, _OWNERS)
+      api_owners.append(self._owners_fs.GetFromFile(owners_file).Then(
+          fix_core_owners))
+      def sort_and_cache(api_owners):
+        api_owners.sort(key=itemgetter('apiName'))
+        self._cache.Set('api_owners', api_owners)
+        return api_owners
+      return All(api_owners).Then(sort_and_cache)
+    return self._cache.Get('api_owners').Then(collect)
+
+  def get(self, key):
+    return {
+      'apis': self._CollectOwnersData()
+    }.get(key).Get()
+
+  def Cron(self):
+    return self._CollectOwnersData()

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

+#!/usr/bin/env python
+# Copyright 2014 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import unittest
+
+from owners_data_source import ParseOwnersFile, OwnersDataSource
+from server_instance import ServerInstance
+from servlet import Request
+from test_file_system import TestFileSystem
+
+
+_TEST_FS = {
+  'chrome': {
+    'browser': {
+      'extensions': {
+        'OWNERS': '\n'.join([
+          '# Core owners.',
+          'satsuki@revocs.tld'
+        ]),
+        'api': {
+          'some_api': {
+            'OWNERS': '\n'.join([
+              'matoi@owner.tld'
+            ]),
+            'some_api.cc': ''
+          },
+          'another_api': {
+            'another_api.cc': '',
+            'another_api.h': ''
+          },
+          'moar_apis': {
+            'OWNERS': '\n'.join([
+              '# For editing moar_apis.',
+              'satsuki@revocs.tld'
+            ])
+          }
+        }
+      }
+    }
+  },
+  'extensions': {
+    'browser': {
+      'api': {
+        'a_different_api': {
+          'OWNERS': '\n'.join([
+            '# Hallo!',
+            'nonon@owner.tld',
+            'matoi@owner.tld'
+          ])
+        }
+      }
+    }
+  }
+}
+
+
+class OwnersDataSourceTest(unittest.TestCase):
+  def setUp(self):
+    server_instance = ServerInstance.ForTest(
+        file_system=TestFileSystem(_TEST_FS))
+    # Don't randomize the owners to avoid testing issues.
+    self._owners_ds = OwnersDataSource(server_instance,
+                                       Request.ForTest('/'),
+                                       randomize=False)
+
+  def testParseOwnersFile(self):
+    owners_content = '\n'.join([
+      'satsuki@revocs.tld',
+      'mankanshoku@owner.tld',
+      '',
+      'matoi@owner.tld'
+    ])
+    owners, notes = ParseOwnersFile(owners_content, randomize=False)
+    # The order of the owners list should reflect the order of the owners file.
+    self.assertEqual(owners, [
+      {
+        'email': 'satsuki@revocs.tld',
+        'username': 'satsuki'
+      },
+      {
+        'email': 'mankanshoku@owner.tld',
+        'username': 'mankanshoku'
+      },
+      {
+        'email': 'matoi@owner.tld',
+        'username': 'matoi',
+        'last': True
+      }
+    ])
+    self.assertEqual(notes, '')
+
+    owners_content_with_comments = '\n'.join([
+      '# This is a comment concerning this file',
+      '# that should not be ignored.',
+      'matoi@owner.tld',
+      'mankanshoku@owner.tld',
+      '',
+      '# Only bug satsuki if matoi or mankanshoku are unavailable.',
+      'satsuki@revocs.tld'
+    ])
+    owners, notes = ParseOwnersFile(owners_content_with_comments,
+                                    randomize=False)
+    self.assertEqual(owners, [
+      {
+        'email': 'matoi@owner.tld',
+        'username': 'matoi'
+      },
+      {
+        'email': 'mankanshoku@owner.tld',
+        'username': 'mankanshoku'
+      },
+      {
+        'email': 'satsuki@revocs.tld',
+        'username': 'satsuki',
+        'last': True
+      }
+    ])
+    self.assertEqual(notes, '\n'.join([
+      'This is a comment concerning this file',
+      'that should not be ignored.',
+      'Only bug satsuki if matoi or mankanshoku are unavailable.'
+    ]))
+
+
+  def testCollectOwners(self):
+    # NOTE: Order matters. The list should be sorted by 'apiName'.
+    self.assertEqual(self._owners_ds.get('apis'), [{
+      'apiName': 'Core Extensions/Apps Owners',
+      'owners': [
+        {
+          'email': 'satsuki@revocs.tld',
+          'username': 'satsuki',
+          'last': True
+        }
+      ],
+      'notes': 'Core owners.',
+      'id': 'core'
+    },
+    {
+      'apiName': 'a_different_api',
+      'owners': [
+        {
+          'email': 'nonon@owner.tld',
+          'username': 'nonon'
+        },
+        {
+          'email': 'matoi@owner.tld',
+          'username': 'matoi',
+          'last': True
+        }
+      ],
+      'notes': 'Hallo!',
+      'id': 'a_different_api'
+    },
+    {
+      'apiName': 'another_api',
+      'owners': [],
+      'notes': 'Use one of the Core Extensions/Apps Owners.',
+      'id': 'another_api'
+    },
+    {
+      'apiName': 'moar_apis',
+      'owners': [
+        {
+          'email': 'satsuki@revocs.tld',
+          'username': 'satsuki',
+          'last': True
+        }
+      ],
+      'notes': 'For editing moar_apis.',
+      'id': 'moar_apis'
+    },
+    {
+      'apiName': 'some_api',
+      'owners': [
+        {
+          'email': 'matoi@owner.tld',
+          'username': 'matoi',
+          'last': True
+        }
+      ],
+      'notes': '',
+      'id': 'some_api'
+    }])
+
+if __name__ == '__main__':
+  unittest.main()

chrome/common/extensions/docs/templates/public/owners.html

+<!DOCTYPE html>
+<html>
+  <head>
+    <title>Extensions/Apps API Owners</title>
+    <style>
+      .warning {
+        color: #f90101;
+      }
+      html, body {
+        width: 1400px;
+      }
+      table {
+        border-collapse: collapse;
+      }
+      table, th, td {
+        border: 1px solid #eee;
+        padding: 2px 6px;
+      }
+      tr:target {
+        font-weight: bold;
+        background-color: #dcdcdc;
+      }
+      #notes {
+        white-space: pre-line;
+      }
+    </style>
+  </head>
+  <body>
+    <h1><center>Extensions/Apps API Owners</center></h1>
+    <table>
+      <tr>
+        <th>API</th>
+        <th>Owners</th>
+        <th>Notes</th>
+      </tr>
+      {{#entry:owners.apis}}
+      <tr id={{entry.id}}>
+        <td>{{entry.apiName}}</td>
+        <td>
+          {{?entry.owners}}
+            {{#owner:entry.owners}}
+            <a href="https://codereview.chromium.org/user/{{owner.email}}">
+              {{owner.username}}</a>{{^owner.last}}, {{/owner.last}}
+            {{/entry.owners}}
+          {{:}}
+            <a href="#core"><span class="warning">No owners.</span></a>
+          {{/entry.owners}}
+        </td>
+        <td id="notes">
+          {{entry.notes}}
+        </td>
+      </tr>
+      {{/owners.apis}}
+    </table>
+  </body>
+</html>