diff options
| author | kalman@chromium.org <kalman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-07-19 20:44:04 +0000 |
|---|---|---|
| committer | kalman@chromium.org <kalman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-07-19 20:44:04 +0000 |
| commit | 4105bf75ab0b68f2a9a117cca2e52ac01fb12f77 (patch) | |
| tree | e5e7c21a1e51c66b4382aa4da057010c97001894 | |
| parent | bd931cfa493b88c047e3a0720620331379cd7b4f (diff) | |
| download | chromium_src-4105bf75ab0b68f2a9a117cca2e52ac01fb12f77.zip chromium_src-4105bf75ab0b68f2a9a117cca2e52ac01fb12f77.tar.gz chromium_src-4105bf75ab0b68f2a9a117cca2e52ac01fb12f77.tar.bz2 | |
Docserver: move to a single version served from trunk. Permanently redirect requests
from any channel to the non-channel version (served from trunk). Improve the
redirect logic in other ways. Fix a few bugs in caching classes that cause this all not
to work. Replace the channel switcher (now moot) with a platform switcher.
R=cduvall@chromium.org
BUG=233978
NOTRY=true
Review URL: https://chromiumcodereview.appspot.com/15087006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@212646 0039d316-1c4b-4281-b951-d872f2087c98
55 files changed, 466 insertions, 522 deletions
diff --git a/chrome/common/extensions/docs/server2/app.yaml b/chrome/common/extensions/docs/server2/app.yaml index 2ba3da9..ce627b1 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: 2-10-0 +version: 2-11-0 runtime: python27 api_version: 1 threadsafe: false diff --git a/chrome/common/extensions/docs/server2/app_yaml_helper.py b/chrome/common/extensions/docs/server2/app_yaml_helper.py index 08f7003..63a0cc7 100644 --- a/chrome/common/extensions/docs/server2/app_yaml_helper.py +++ b/chrome/common/extensions/docs/server2/app_yaml_helper.py @@ -20,8 +20,7 @@ class AppYamlHelper(object): app_yaml_path, file_system_at_head, object_store_creator, - host_file_system_creator, - branch): + host_file_system_creator): self._app_yaml_path = app_yaml_path self._file_system_at_head = file_system_at_head self._store = object_store_creator.Create( @@ -29,7 +28,6 @@ class AppYamlHelper(object): category=file_system_at_head.GetIdentity(), start_empty=False) self._host_file_system_creator = host_file_system_creator - self._branch = branch @staticmethod def ExtractVersion(app_yaml, key='version'): @@ -120,7 +118,6 @@ class AppYamlHelper(object): logging.warning('All revisions are greater than %s' % app_version) return 0 next_file_system = self._host_file_system_creator.Create( - self._branch, revision=found - 1) if found is None: diff --git a/chrome/common/extensions/docs/server2/app_yaml_helper_test.py b/chrome/common/extensions/docs/server2/app_yaml_helper_test.py index 7401477..59e2014 100755 --- a/chrome/common/extensions/docs/server2/app_yaml_helper_test.py +++ b/chrome/common/extensions/docs/server2/app_yaml_helper_test.py @@ -95,8 +95,7 @@ class AppYamlHelperTest(unittest.TestCase): helper = AppYamlHelper('server2/app.yaml', file_system_at_head, ObjectStoreCreator.ForTest(disable_wrappers=False), - host_file_system_creator, - 'trunk') + host_file_system_creator) def assert_is_up_to_date(version): self.assertTrue(helper.IsUpToDate(version), diff --git a/chrome/common/extensions/docs/server2/appengine_blobstore.py b/chrome/common/extensions/docs/server2/appengine_blobstore.py index 5781556..f367293 100644 --- a/chrome/common/extensions/docs/server2/appengine_blobstore.py +++ b/chrome/common/extensions/docs/server2/appengine_blobstore.py @@ -14,7 +14,7 @@ class AppEngineBlobstore(object): datastore. """ def __init__(self): - self._datastore = BlobReferenceStore('blobstore') + self._datastore = BlobReferenceStore() def Set(self, key, blob, namespace): """Add a blob to the blobstore. |version| is used as part of the key so diff --git a/chrome/common/extensions/docs/server2/blob_reference_store.py b/chrome/common/extensions/docs/server2/blob_reference_store.py index 97c6827..7edf5ee 100644 --- a/chrome/common/extensions/docs/server2/blob_reference_store.py +++ b/chrome/common/extensions/docs/server2/blob_reference_store.py @@ -14,14 +14,11 @@ class _Model(db.Model): class BlobReferenceStore(object): """A wrapper around the datastore API that can store blob keys. """ - def __init__(self, branch): - self._branch = branch - def _Query(self, namespace, key): return _Model.gql('WHERE key_ = :1', self._MakeKey(namespace, key)).get() def _MakeKey(self, namespace, key): - return '.'.join([self._branch, namespace, key]) + return '.'.join((namespace, key)) def Set(self, namespace, key, value): _Model(key_=self._MakeKey(namespace, key), value=value).put() diff --git a/chrome/common/extensions/docs/server2/branch_utility.py b/chrome/common/extensions/docs/server2/branch_utility.py index 4f49113..ab3dce2 100644 --- a/chrome/common/extensions/docs/server2/branch_utility.py +++ b/chrome/common/extensions/docs/server2/branch_utility.py @@ -18,13 +18,10 @@ class ChannelInfo(object): class BranchUtility(object): def __init__(self, fetch_url, history_url, fetcher, object_store_creator): self._fetcher = fetcher - # BranchUtility is obviously cross-channel, so set the channel to None. - self._branch_object_store = object_store_creator.Create(BranchUtility, - category='branch', - channel=None) - self._version_object_store = object_store_creator.Create(BranchUtility, - category='version', - channel=None) + def create_object_store(category): + return object_store_creator.Create(BranchUtility, category=category) + self._branch_object_store = create_object_store('branch') + self._version_object_store = create_object_store('version') self._fetch_result = self._fetcher.FetchAsync(fetch_url) self._history_result = self._fetcher.FetchAsync(history_url) 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 e7f7ecf..6bb09f8 100755 --- a/chrome/common/extensions/docs/server2/caching_file_system_test.py +++ b/chrome/common/extensions/docs/server2/caching_file_system_test.py @@ -37,8 +37,7 @@ class CachingFileSystemTest(unittest.TestCase): if start_empty: db.clear() return TestObjectStore(namespace, init=db) - object_store_creator = ObjectStoreCreator('test', - start_empty=start_empty, + object_store_creator = ObjectStoreCreator(start_empty=start_empty, store_type=store_type_constructor) return CachingFileSystem(fs, object_store_creator) diff --git a/chrome/common/extensions/docs/server2/caching_rietveld_patcher.py b/chrome/common/extensions/docs/server2/caching_rietveld_patcher.py index dd49482..2c33b33 100644 --- a/chrome/common/extensions/docs/server2/caching_rietveld_patcher.py +++ b/chrome/common/extensions/docs/server2/caching_rietveld_patcher.py @@ -67,12 +67,13 @@ class CachingRietveldPatcher(Patcher): object_store_creator, test_datetime=datetime): self._patcher = rietveld_patcher - self._version_object_store = object_store_creator.Create( - CachingRietveldPatcher, category='version') - self._list_object_store = object_store_creator.Create( - CachingRietveldPatcher, category='list') - self._file_object_store = object_store_creator.Create( - CachingRietveldPatcher, category='file') + def create_object_store(category): + return object_store_creator.Create( + CachingRietveldPatcher, + category='%s/%s' % (rietveld_patcher.GetIdentity(), category)) + self._version_object_store = create_object_store('version') + self._list_object_store = create_object_store('list') + self._file_object_store = create_object_store('file') self._datetime = test_datetime def GetVersion(self): @@ -124,3 +125,6 @@ class CachingRietveldPatcher(Patcher): True, version), self._file_object_store) + + def GetIdentity(self): + return self._patcher.GetIdentity() diff --git a/chrome/common/extensions/docs/server2/caching_rietveld_patcher_test.py b/chrome/common/extensions/docs/server2/caching_rietveld_patcher_test.py index 203392f..2984a57 100755 --- a/chrome/common/extensions/docs/server2/caching_rietveld_patcher_test.py +++ b/chrome/common/extensions/docs/server2/caching_rietveld_patcher_test.py @@ -37,7 +37,7 @@ class CachingRietveldPatcherTest(unittest.TestCase): assert_binary=True) self._patcher = CachingRietveldPatcher( self._test_patcher, - ObjectStoreCreator('test', start_empty=False), + ObjectStoreCreator(start_empty=False), self._datetime) def testGetVersion(self): diff --git a/chrome/common/extensions/docs/server2/chained_compiled_file_system_test.py b/chrome/common/extensions/docs/server2/chained_compiled_file_system_test.py index 2884751..face116d 100755 --- a/chrome/common/extensions/docs/server2/chained_compiled_file_system_test.py +++ b/chrome/common/extensions/docs/server2/chained_compiled_file_system_test.py @@ -28,18 +28,13 @@ _TEST_DATA_NEW = { class ChainedCompiledFileSystemTest(unittest.TestCase): def setUp(self): - self._object_store_creator = ObjectStoreCreator( - 'chained', start_empty=False) - self._base_object_store_creator = ObjectStoreCreator( - 'base', start_empty=False) + object_store_creator = ObjectStoreCreator(start_empty=False) base_file_system = TestFileSystem(_TEST_DATA_BASE) - self._base_factory = CompiledFileSystem.Factory( - base_file_system, - self._base_object_store_creator) + self._base_factory = CompiledFileSystem.Factory(base_file_system, + object_store_creator) self._file_system = TestFileSystem(_TEST_DATA_NEW) - self._patched_factory = CompiledFileSystem.Factory( - self._file_system, - self._object_store_creator) + self._patched_factory = CompiledFileSystem.Factory(self._file_system, + object_store_creator) self._chained_factory = ChainedCompiledFileSystem.Factory( [(self._patched_factory, self._file_system), (self._base_factory, base_file_system)]) diff --git a/chrome/common/extensions/docs/server2/compiled_file_system.py b/chrome/common/extensions/docs/server2/compiled_file_system.py index 087eea8..29b1363 100644 --- a/chrome/common/extensions/docs/server2/compiled_file_system.py +++ b/chrome/common/extensions/docs/server2/compiled_file_system.py @@ -26,12 +26,12 @@ class CompiledFileSystem(object): """ assert isinstance(cls, type) assert not cls.__name__[0].islower() # guard against non-class types - full_name = cls.__name__ + full_name = [cls.__name__, self._file_system.GetIdentity()] if category is not None: - full_name = '%s/%s' % (full_name, category) - def create_object_store(category): + full_name.append(category) + def create_object_store(my_category): return self._object_store_creator.Create( - CompiledFileSystem, category='%s/%s' % (full_name, category)) + CompiledFileSystem, category='/'.join(full_name + [my_category])) return CompiledFileSystem(self._file_system, populate_function, create_object_store('file'), diff --git a/chrome/common/extensions/docs/server2/compiled_file_system_test.py b/chrome/common/extensions/docs/server2/compiled_file_system_test.py index 557adec..8923808 100755 --- a/chrome/common/extensions/docs/server2/compiled_file_system_test.py +++ b/chrome/common/extensions/docs/server2/compiled_file_system_test.py @@ -30,8 +30,7 @@ _TEST_DATA = { def _CreateFactory(): return CompiledFileSystem.Factory( TestFileSystem(deepcopy(_TEST_DATA)), - ObjectStoreCreator('test', - start_empty=False, + ObjectStoreCreator(start_empty=False, store_type=TestObjectStore, disable_wrappers=True)) @@ -40,8 +39,9 @@ class CompiledFileSystemTest(unittest.TestCase): factory = _CreateFactory() compiled_fs = factory.CreateIdentity(CompiledFileSystemTest) self.assertEqual( - 'class=CompiledFileSystem&category=CompiledFileSystemTest/file&' - 'channel=test&app_version=%s' % GetAppVersion(), + 'class=CompiledFileSystem&' + 'category=CompiledFileSystemTest/TestFileSystem/file&' + 'app_version=%s' % GetAppVersion(), compiled_fs._file_object_store.namespace) def testIdentityFromFile(self): @@ -73,16 +73,20 @@ class CompiledFileSystemTest(unittest.TestCase): factory = _CreateFactory() f = lambda x: x CheckNamespace( - 'class=CompiledFileSystem&category=CompiledFileSystemTest/file&' - 'channel=test&app_version=%s' % GetAppVersion(), - 'class=CompiledFileSystem&category=CompiledFileSystemTest/list&' - 'channel=test&app_version=%s' % GetAppVersion(), + 'class=CompiledFileSystem&' + 'category=CompiledFileSystemTest/TestFileSystem/file&' + 'app_version=%s' % GetAppVersion(), + 'class=CompiledFileSystem&' + 'category=CompiledFileSystemTest/TestFileSystem/list&' + 'app_version=%s' % GetAppVersion(), factory.Create(f, CompiledFileSystemTest)) CheckNamespace( - 'class=CompiledFileSystem&category=CompiledFileSystemTest/foo/file&' - 'channel=test&app_version=%s' % GetAppVersion(), - 'class=CompiledFileSystem&category=CompiledFileSystemTest/foo/list&' - 'channel=test&app_version=%s' % GetAppVersion(), + 'class=CompiledFileSystem&' + 'category=CompiledFileSystemTest/TestFileSystem/foo/file&' + 'app_version=%s' % GetAppVersion(), + 'class=CompiledFileSystem&' + 'category=CompiledFileSystemTest/TestFileSystem/foo/list&' + 'app_version=%s' % GetAppVersion(), factory.Create(f, CompiledFileSystemTest, category='foo')) def testPopulateFromFile(self): diff --git a/chrome/common/extensions/docs/server2/cron.yaml b/chrome/common/extensions/docs/server2/cron.yaml index bbfa423..320ef9a 100644 --- a/chrome/common/extensions/docs/server2/cron.yaml +++ b/chrome/common/extensions/docs/server2/cron.yaml @@ -1,20 +1,5 @@ cron: -- description: Load everything for trunk. - url: /_cron/trunk +- description: Repopulates all cached data. + url: /_cron schedule: every 5 minutes - target: 2-10-0 - -- description: Load everything for dev. - url: /_cron/dev - schedule: every 5 minutes - target: 2-10-0 - -- description: Load everything for beta. - url: /_cron/beta - schedule: every 5 minutes - target: 2-10-0 - -- description: Load everything for stable. - url: /_cron/stable - schedule: every 5 minutes - target: 2-10-0 + target: 2-11-0 diff --git a/chrome/common/extensions/docs/server2/cron_servlet.py b/chrome/common/extensions/docs/server2/cron_servlet.py index 915aae5..14d4b39 100644 --- a/chrome/common/extensions/docs/server2/cron_servlet.py +++ b/chrome/common/extensions/docs/server2/cron_servlet.py @@ -25,7 +25,7 @@ class _SingletonRenderServletDelegate(RenderServlet.Delegate): def __init__(self, server_instance): self._server_instance = server_instance - def CreateServerInstanceForChannel(self, channel): + def CreateServerInstance(self): return self._server_instance class CronServlet(Servlet): @@ -33,7 +33,6 @@ class CronServlet(Servlet): ''' def __init__(self, request, delegate_for_test=None): Servlet.__init__(self, request) - self._channel = request.path.strip('/') self._delegate = delegate_for_test or CronServlet.Delegate() class Delegate(object): @@ -70,8 +69,7 @@ class CronServlet(Servlet): # the time these won't have changed since the last cron run, so it's a # little wasteful, but hopefully rendering is really fast (if it isn't we # have a problem). - channel = self._channel - logging.info('cron/%s: starting' % channel) + logging.info('cron: starting') # This is returned every time RenderServlet wants to create a new # ServerInstance. @@ -87,8 +85,7 @@ class CronServlet(Servlet): start_time = time.time() files = dict( CreateURLsFromPaths(server_instance.host_file_system, d, path_prefix)) - logging.info('cron/%s: rendering %s files from %s...' % ( - channel, len(files), d)) + logging.info('cron: rendering %s files from %s...' % (len(files), d)) try: for i, path in enumerate(files): error = None @@ -98,18 +95,17 @@ class CronServlet(Servlet): error = 'Got %s response' % response.status except DeadlineExceededError: logging.error( - 'cron/%s: deadline exceeded rendering %s (%s of %s): %s' % ( - channel, path, i + 1, len(files), traceback.format_exc())) + 'cron: deadline exceeded rendering %s (%s of %s): %s' % ( + path, i + 1, len(files), traceback.format_exc())) raise except error: pass if error: - logging.error('cron/%s: error rendering %s: %s' % ( - channel, path, error)) + logging.error('cron: error rendering %s: %s' % (path, error)) success = False finally: - logging.info('cron/%s: rendering %s files from %s took %s seconds' % ( - channel, len(files), d, time.time() - start_time)) + logging.info('cron: rendering %s files from %s took %s seconds' % ( + len(files), d, time.time() - start_time)) return success success = True @@ -147,24 +143,23 @@ class CronServlet(Servlet): for filename in server_instance.content_cache.GetFromFileListing( svn_constants.EXAMPLES_PATH) if filename.endswith(manifest_json)] - logging.info('cron/%s: rendering %s example zips...' % ( - channel, len(example_zips))) + logging.info('cron: rendering %s example zips...' % len(example_zips)) start_time = time.time() try: success = success and all( get_via_render_servlet('extensions/examples/%s' % z).status == 200 for z in example_zips) finally: - logging.info('cron/%s: rendering %s example zips took %s seconds' % ( - channel, len(example_zips), time.time() - start_time)) + logging.info('cron: rendering %s example zips took %s seconds' % ( + len(example_zips), time.time() - start_time)) except DeadlineExceededError: success = False - logging.info("cron/%s: running Redirector cron..." % channel) + logging.info('cron: running Redirector cron...') server_instance.redirector.Cron() - logging.info('cron/%s: finished' % channel) + logging.info('cron: finished (%s)' % ('success' if success else 'failure',)) return (Response.Ok('Success') if success else Response.InternalError('Failure')) @@ -174,16 +169,14 @@ class CronServlet(Servlet): meaning the last revision that the current running version of the server existed. ''' - channel = self._channel delegate = self._delegate - server_instance_at_head = self._CreateServerInstance(channel, None) + server_instance_at_head = self._CreateServerInstance(None) app_yaml_handler = AppYamlHelper( svn_constants.APP_YAML_PATH, server_instance_at_head.host_file_system, server_instance_at_head.object_store_creator, - server_instance_at_head.host_file_system_creator, - self._GetBranchForChannel(channel)) + server_instance_at_head.host_file_system_creator) if app_yaml_handler.IsUpToDate(delegate.GetAppVersion()): # TODO(kalman): return a new ServerInstance at an explicit revision in @@ -195,37 +188,26 @@ class CronServlet(Servlet): safe_revision = app_yaml_handler.GetFirstRevisionGreaterThan( delegate.GetAppVersion()) - 1 - logging.info('cron/%s: app version %s is out of date, safe is %s' % ( - channel, delegate.GetAppVersion(), safe_revision)) + logging.info('cron: app version %s is out of date, safe is %s' % ( + delegate.GetAppVersion(), safe_revision)) - return self._CreateServerInstance(channel, safe_revision) + return self._CreateServerInstance(safe_revision) - def _CreateObjectStoreCreator(self, channel): - return ObjectStoreCreator(channel, start_empty=True) - - def _GetBranchForChannel(self, channel): - object_store_creator = self._CreateObjectStoreCreator(channel) - return (self._delegate.CreateBranchUtility(object_store_creator) - .GetChannelInfo(channel).branch) - - def _CreateServerInstance(self, channel, revision): - object_store_creator = self._CreateObjectStoreCreator(channel) + def _CreateServerInstance(self, revision): + object_store_creator = ObjectStoreCreator(start_empty=True) branch_utility = self._delegate.CreateBranchUtility(object_store_creator) host_file_system_creator = self._delegate.CreateHostFileSystemCreator( object_store_creator) - host_file_system = host_file_system_creator.Create( - branch_utility.GetChannelInfo(channel).branch, - revision=revision) + host_file_system = host_file_system_creator.Create(revision=revision) app_samples_file_system = self._delegate.CreateAppSamplesFileSystem( object_store_creator) compiled_host_fs_factory = CompiledFileSystem.Factory( host_file_system, object_store_creator) - return ServerInstance(channel, - object_store_creator, + return ServerInstance(object_store_creator, host_file_system, app_samples_file_system, - '' if channel == 'stable' else '/%s' % channel, + '', compiled_host_fs_factory, branch_utility, host_file_system_creator) diff --git a/chrome/common/extensions/docs/server2/cron_servlet_test.py b/chrome/common/extensions/docs/server2/cron_servlet_test.py index a5fb622..07775eb 100755 --- a/chrome/common/extensions/docs/server2/cron_servlet_test.py +++ b/chrome/common/extensions/docs/server2/cron_servlet_test.py @@ -31,8 +31,8 @@ class _TestDelegate(CronServlet.Delegate): return TestBranchUtility.CreateWithCannedData() def CreateHostFileSystemCreator(self, object_store_creator): - def constructor(branch, revision=None): - file_system = self._create_file_system(branch, revision) + def constructor(branch=None, revision=None): + file_system = self._create_file_system(revision) self.file_systems.append(file_system) return file_system return HostFileSystemCreator(object_store_creator, @@ -53,8 +53,7 @@ class CronServletTest(unittest.TestCase): def testEverything(self): # All these tests are dependent (see above comment) so lump everything in # the one test. - delegate = _TestDelegate( - lambda _, __: MockFileSystem(LocalFileSystem.Create())) + delegate = _TestDelegate(lambda _: MockFileSystem(LocalFileSystem.Create())) # Test that the cron runs successfully. response = CronServlet(Request.ForTest('trunk'), @@ -122,7 +121,7 @@ class CronServletTest(unittest.TestCase): storage_html_path = 'docs/templates/public/apps/storage.html' static_txt_path = 'docs/static/static.txt' - def create_file_system(branch, revision=None): + def create_file_system(revision=None): '''Creates a MockFileSystem at |revision| by applying that many |updates| to it. ''' 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 22f5484..50e660a 100644 --- a/chrome/common/extensions/docs/server2/empty_dir_file_system.py +++ b/chrome/common/extensions/docs/server2/empty_dir_file_system.py @@ -21,3 +21,6 @@ class EmptyDirFileSystem(FileSystem): if not path.endswith('/'): raise FileNotFoundError('EmptyDirFileSystem cannot stat %s' % path) return StatInfo(0, child_versions=[]) + + def GetIdentity(self): + return self.__class__.__name__ diff --git a/chrome/common/extensions/docs/server2/file_system.py b/chrome/common/extensions/docs/server2/file_system.py index d899826..3226ebe 100644 --- a/chrome/common/extensions/docs/server2/file_system.py +++ b/chrome/common/extensions/docs/server2/file_system.py @@ -51,7 +51,7 @@ class FileSystem(object): and failing that as latin-1 (some extension docs use latin-1). If binary=True then the contents will be a str. ''' - raise NotImplementedError() + raise NotImplementedError(self.__class__) def ReadSingle(self, path, binary=False): '''Reads a single file from the FileSystem. @@ -64,7 +64,7 @@ class FileSystem(object): is a directory, |StatInfo| will have the versions of all the children of the directory in |StatInfo.child_versions|. ''' - raise NotImplementedError() + raise NotImplementedError(self.__class__) def GetIdentity(self): '''The identity of the file system, exposed for caching classes to @@ -73,7 +73,7 @@ class FileSystem(object): different to that of a SubversionFileSystem with a base path of /bar, is different to a LocalFileSystem with a base path of /usr. ''' - raise NotImplementedError() + raise NotImplementedError(self.__class__) def Walk(self, root): '''Recursively walk the directories in a file system, starting with root. diff --git a/chrome/common/extensions/docs/server2/github_file_system.py b/chrome/common/extensions/docs/server2/github_file_system.py index 1b9569a..c0ffb1e 100644 --- a/chrome/common/extensions/docs/server2/github_file_system.py +++ b/chrome/common/extensions/docs/server2/github_file_system.py @@ -10,6 +10,7 @@ from StringIO import StringIO import appengine_blobstore as blobstore from appengine_url_fetcher import AppEngineUrlFetcher from appengine_wrappers import GetAppVersion, urlfetch +from docs_server_utils import StringIdentity from file_system import FileSystem, StatInfo from future import Future from object_store_creator import ObjectStoreCreator @@ -70,17 +71,16 @@ class GithubFileSystem(FileSystem): @staticmethod def Create(object_store_creator): return GithubFileSystem( - AppEngineUrlFetcher(url_constants.GITHUB_URL), + url_constants.GITHUB_URL, blobstore.AppEngineBlobstore(), object_store_creator) - def __init__(self, fetcher, blobstore, object_store_creator): - # Password store doesn't depend on channel, and if we don't cancel the app - # version then the whole advantage of having it in the first place is - # greatly lessened (likewise it should always start populated). + def __init__(self, url, blobstore, object_store_creator): + # If we key the password store on the app version then the whole advantage + # of having it in the first place is greatly lessened (likewise it should + # always start populated). password_store = object_store_creator.Create( GithubFileSystem, - channel=None, app_version=None, category='password', start_empty=False) @@ -92,12 +92,10 @@ class GithubFileSystem(FileSystem): password_store.SetMulti({'username': USERNAME, 'password': PASSWORD}) self._username, self._password = (USERNAME, PASSWORD) - self._fetcher = fetcher + self._url = url + self._fetcher = AppEngineUrlFetcher(url) self._blobstore = blobstore - # Github has no knowledge of Chrome channels, set channel to None. - self._stat_object_store = object_store_creator.Create( - GithubFileSystem, - channel=None) + self._stat_object_store = object_store_creator.Create(GithubFileSystem) self._version = None self._GetZip(self.Stat(ZIP_KEY).version) @@ -203,3 +201,6 @@ class GithubFileSystem(FileSystem): logging.warning('Problem fetching commit hash from github.') return self._DefaultStat(path) return StatInfo(version) + + def GetIdentity(self): + return '%s@%s' % (self.__class__.__name__, StringIdentity(self._url)) diff --git a/chrome/common/extensions/docs/server2/github_file_system_test.py b/chrome/common/extensions/docs/server2/github_file_system_test.py index 65a6928..54f829d 100755 --- a/chrome/common/extensions/docs/server2/github_file_system_test.py +++ b/chrome/common/extensions/docs/server2/github_file_system_test.py @@ -22,10 +22,7 @@ class GithubFileSystemTest(unittest.TestCase): self._base_path = os.path.join(sys.path[0], 'test_data', 'github_file_system') - self._file_system = GithubFileSystem( - AppEngineUrlFetcher(url_constants.GITHUB_URL), - AppEngineBlobstore(), - ObjectStoreCreator.ForTest()) + self._file_system = GithubFileSystem.Create(ObjectStoreCreator.ForTest()) def _ReadLocalFile(self, filename): with open(os.path.join(self._base_path, filename), 'r') as f: diff --git a/chrome/common/extensions/docs/server2/host_file_system_creator.py b/chrome/common/extensions/docs/server2/host_file_system_creator.py index 1fee20f..381e664 100644 --- a/chrome/common/extensions/docs/server2/host_file_system_creator.py +++ b/chrome/common/extensions/docs/server2/host_file_system_creator.py @@ -23,16 +23,17 @@ class HostFileSystemCreator(object): # Provides custom create behavior, useful in tests. self._constructor_for_test = constructor_for_test - def Create(self, branch, revision=None): + def Create(self, branch='trunk', revision=None): '''Creates either SVN file systems or specialized file systems from the constructor passed into this instance. Wraps the resulting file system in an Offline file system if the offline flag is set, and finally wraps it in a Caching file system. ''' if self._constructor_for_test is not None: - file_system = self._constructor_for_test(branch, revision) + file_system = self._constructor_for_test(branch=branch, revision=revision) else: - file_system = SubversionFileSystem.Create(branch, revision=revision) + file_system = SubversionFileSystem.Create(branch=branch, + revision=revision) if self._offline: file_system = OfflineFileSystem(file_system) return CachingFileSystem(file_system, self._object_store_creator) @@ -43,7 +44,7 @@ class HostFileSystemCreator(object): ''' return HostFileSystemCreator( object_store_creator, - constructor_for_test=lambda _, __: LocalFileSystem.Create()) + constructor_for_test=lambda **_: LocalFileSystem.Create()) @staticmethod def ForTest(file_system, object_store_creator): @@ -53,4 +54,4 @@ class HostFileSystemCreator(object): ''' return HostFileSystemCreator( object_store_creator, - constructor_for_test=lambda _, __: file_system) + constructor_for_test=lambda **_: file_system) diff --git a/chrome/common/extensions/docs/server2/instance_servlet.py b/chrome/common/extensions/docs/server2/instance_servlet.py index 2016e32..0382ce4 100644 --- a/chrome/common/extensions/docs/server2/instance_servlet.py +++ b/chrome/common/extensions/docs/server2/instance_servlet.py @@ -32,23 +32,21 @@ class _OfflineRenderServletDelegate(RenderServlet.Delegate): self._delegate = delegate @memoize - def CreateServerInstanceForChannel(self, channel): - object_store_creator = ObjectStoreCreator(channel, start_empty=False) + def CreateServerInstance(self): + object_store_creator = ObjectStoreCreator(start_empty=False) branch_utility = self._delegate.CreateBranchUtility(object_store_creator) host_file_system_creator = self._delegate.CreateHostFileSystemCreator( object_store_creator) - host_file_system = host_file_system_creator.Create( - branch_utility.GetChannelInfo(channel).branch) + host_file_system = host_file_system_creator.Create() app_samples_file_system = self._delegate.CreateAppSamplesFileSystem( object_store_creator) compiled_host_fs_factory = CompiledFileSystem.Factory( host_file_system, object_store_creator) - return ServerInstance(channel, - object_store_creator, + return ServerInstance(object_store_creator, host_file_system, app_samples_file_system, - '' if channel == 'stable' else '/%s' % channel, + '', compiled_host_fs_factory, branch_utility, host_file_system_creator) diff --git a/chrome/common/extensions/docs/server2/instance_servlet_test.py b/chrome/common/extensions/docs/server2/instance_servlet_test.py index 99252e2..8d9abc3 100755 --- a/chrome/common/extensions/docs/server2/instance_servlet_test.py +++ b/chrome/common/extensions/docs/server2/instance_servlet_test.py @@ -12,6 +12,9 @@ from servlet import Request from test_branch_utility import TestBranchUtility from test_util import DisableLogging +# XXX(kalman): what is this test supposed to be? +# Create a test host file system creator which failz? + # NOTE(kalman): The ObjectStore created by the InstanceServlet is backed onto # our fake AppEngine memcache/datastore, so the tests aren't isolated. class _TestDelegate(InstanceServlet.Delegate): @@ -35,19 +38,19 @@ class InstanceServletTest(unittest.TestCase): def testHostFileSystemNotAccessed(self): delegate = _TestDelegate(_FailOnAccessFileSystem) constructor = InstanceServlet.GetConstructor(delegate_for_test=delegate) - def test_path(path): + def test_path(path, status=404): response = constructor(Request.ForTest(path)).Get() - self.assertEqual(404, response.status) + self.assertEqual(status, response.status) test_path('extensions/storage.html') test_path('apps/storage.html') test_path('extensions/examples/foo.zip') test_path('extensions/examples/foo.html') test_path('static/foo.css') - test_path('beta/extensions/storage.html') - test_path('beta/apps/storage.html') - test_path('beta/extensions/examples/foo.zip') - test_path('beta/extensions/examples/foo.html') - test_path('beta/static/foo.css') + test_path('beta/extensions/storage.html', status=301) + test_path('beta/apps/storage.html', status=301) + test_path('beta/extensions/examples/foo.zip', status=301) + test_path('beta/extensions/examples/foo.html', status=301) + test_path('beta/static/foo.css', status=301) if __name__ == '__main__': unittest.main() diff --git a/chrome/common/extensions/docs/server2/integration_test.py b/chrome/common/extensions/docs/server2/integration_test.py index ea7f305..4721c3b 100755 --- a/chrome/common/extensions/docs/server2/integration_test.py +++ b/chrome/common/extensions/docs/server2/integration_test.py @@ -12,10 +12,12 @@ from itertools import groupby from operator import itemgetter import optparse import os +import posixpath import sys import time import unittest +from branch_utility import BranchUtility from link_error_detector import LinkErrorDetector from local_file_system import LocalFileSystem from local_renderer import LocalRenderer @@ -124,7 +126,23 @@ class IntegrationTest(unittest.TestCase): # that render large files. At least it'll catch zero-length responses. self.assertTrue(len(response.content) >= len(content), 'Content was "%s" when rendering %s' % (response.content, path)) + check_result(Handler(Request.ForTest(path)).Get()) + + # Make sure that leaving out the .html will temporarily redirect to the + # path with the .html. + if path != '/404.html': + redirect_result = Handler( + Request.ForTest(posixpath.splitext(path)[0])).Get() + self.assertEqual((path, False), redirect_result.GetRedirect()) + + # Make sure including a channel will permanently redirect to the same + # path without a channel. + for channel in BranchUtility.GetAllChannelNames(): + redirect_result = Handler( + Request.ForTest('%s/%s' % (channel, path))).Get() + self.assertEqual((path, True), redirect_result.GetRedirect()) + # Samples are internationalized, test some locales. if path.endswith('/samples.html'): for lang in ['en-US', 'es', 'ar']: diff --git a/chrome/common/extensions/docs/server2/local_renderer.py b/chrome/common/extensions/docs/server2/local_renderer.py index 7819e12..be403a8 100644 --- a/chrome/common/extensions/docs/server2/local_renderer.py +++ b/chrome/common/extensions/docs/server2/local_renderer.py @@ -10,7 +10,7 @@ from server_instance import ServerInstance from servlet import Request class _LocalRenderServletDelegate(object): - def CreateServerInstanceForChannel(self, channel): + def CreateServerInstance(self): return ServerInstance.ForLocal() class LocalRenderer(object): @@ -19,13 +19,5 @@ class LocalRenderer(object): @staticmethod def Render(path): assert not '\\' in path - def render_path(path): - return RenderServlet(Request(path, 'http://localhost', {}), - _LocalRenderServletDelegate(), - default_channel='trunk').Get() - response = render_path(path) - while response.status in [301, 302]: - redirect = response.headers['Location'] - sys.stderr.write('<!-- Redirected %s to %s -->\n' % (path, redirect)) - response = render_path(redirect) - return response + return RenderServlet(Request.ForTest(path), + _LocalRenderServletDelegate()).Get() diff --git a/chrome/common/extensions/docs/server2/object_store.py b/chrome/common/extensions/docs/server2/object_store.py index 106dd2d..f33aa91 100644 --- a/chrome/common/extensions/docs/server2/object_store.py +++ b/chrome/common/extensions/docs/server2/object_store.py @@ -25,7 +25,7 @@ class ObjectStore(object): '''Gets a |Future| with values mapped to |keys| from the object store, with any keys not in the object store omitted. ''' - raise NotImplementedError() + raise NotImplementedError(self.__class__) def Set(self, key, value): '''Sets key -> value in the object store. @@ -35,7 +35,7 @@ class ObjectStore(object): def SetMulti(self, items): '''Atomically sets the mapping of keys to values in the object store. ''' - raise NotImplementedError() + raise NotImplementedError(self.__class__) def Del(self, key): '''Deletes a key from the object store. @@ -45,4 +45,4 @@ class ObjectStore(object): def DelMulti(self, keys): '''Deletes |keys| from the object store. ''' - raise NotImplementedError() + raise NotImplementedError(self.__class__) diff --git a/chrome/common/extensions/docs/server2/object_store_creator.py b/chrome/common/extensions/docs/server2/object_store_creator.py index 5bf37f6..6c0d0d2 100644 --- a/chrome/common/extensions/docs/server2/object_store_creator.py +++ b/chrome/common/extensions/docs/server2/object_store_creator.py @@ -18,7 +18,6 @@ class ObjectStoreCreator(object): via the variants of Create which do this automatically). ''' def __init__(self, - channel, # TODO(kalman): rename start_dirty? start_empty=_unspecified, # Override for testing. A custom ObjectStore type to construct @@ -29,7 +28,6 @@ class ObjectStoreCreator(object): # useful to override when specific state tests/manipulations are # being done on the underlying object store. disable_wrappers=False): - self._channel = channel if start_empty is _unspecified: raise ValueError('start_empty must be specified (typically False)') self._start_empty = start_empty @@ -42,8 +40,7 @@ class ObjectStoreCreator(object): def ForTest(start_empty=False, store_type=TestObjectStore, disable_wrappers=True): - return ObjectStoreCreator('test', - start_empty=start_empty, + return ObjectStoreCreator(start_empty=start_empty, store_type=store_type, disable_wrappers=disable_wrappers) @@ -52,20 +49,17 @@ class ObjectStoreCreator(object): category=None, # Override any of these for a custom configuration. start_empty=_unspecified, - channel=_unspecified, app_version=_unspecified): # Resolve namespace components. if start_empty is not _unspecified: start_empty = bool(start_empty) else: start_empty = self._start_empty - if channel is _unspecified: - channel = self._channel if app_version is _unspecified: app_version = GetAppVersion() # Reserve & and = for namespace separators. - for component in (category, channel, app_version): + for component in (category, app_version): if component and ('&' in component or '=' in component): raise ValueError('%s cannot be used in a namespace') @@ -73,7 +67,6 @@ class ObjectStoreCreator(object): '%s=%s' % (key, value) for key, value in (('class', cls.__name__), ('category', category), - ('channel', channel), ('app_version', app_version)) if value is not None) diff --git a/chrome/common/extensions/docs/server2/object_store_creator_test.py b/chrome/common/extensions/docs/server2/object_store_creator_test.py index 680bbff..71ca77e 100755 --- a/chrome/common/extensions/docs/server2/object_store_creator_test.py +++ b/chrome/common/extensions/docs/server2/object_store_creator_test.py @@ -14,35 +14,27 @@ class _FooClass(object): class ObjectStoreCreatorTest(unittest.TestCase): def setUp(self): - self._creator = ObjectStoreCreator('trunk', - start_empty=False, + self._creator = ObjectStoreCreator(start_empty=False, store_type=TestObjectStore, disable_wrappers=True) def testVanilla(self): store = self._creator.Create(_FooClass) self.assertEqual( - 'class=_FooClass&channel=trunk&app_version=%s' % GetAppVersion(), + 'class=_FooClass&app_version=%s' % GetAppVersion(), store.namespace) self.assertFalse(store.start_empty) def testWithCategory(self): store = self._creator.Create(_FooClass, category='hi') self.assertEqual( - 'class=_FooClass&category=hi&channel=trunk&app_version=%s' % - GetAppVersion(), + 'class=_FooClass&category=hi&app_version=%s' % GetAppVersion(), store.namespace) self.assertFalse(store.start_empty) - def testWithoutChannel(self): - store = self._creator.Create(_FooClass, channel=None) - self.assertEqual('class=_FooClass&app_version=%s' % GetAppVersion(), - store.namespace) - self.assertFalse(store.start_empty) - def testWithoutAppVersion(self): store = self._creator.Create(_FooClass, app_version=None) - self.assertEqual('class=_FooClass&channel=trunk', store.namespace) + self.assertEqual('class=_FooClass', store.namespace) self.assertFalse(store.start_empty) def testStartConfiguration(self): @@ -50,12 +42,10 @@ class ObjectStoreCreatorTest(unittest.TestCase): self.assertTrue(store.start_empty) store = self._creator.Create(_FooClass, start_empty=False) self.assertFalse(store.start_empty) - self.assertRaises(ValueError, ObjectStoreCreator, 'foo') + self.assertRaises(ValueError, ObjectStoreCreator) def testIllegalCharacters(self): self.assertRaises(ValueError, - self._creator.Create, _FooClass, channel='foo=') - self.assertRaises(ValueError, self._creator.Create, _FooClass, app_version='1&2') self.assertRaises(ValueError, self._creator.Create, _FooClass, category='a=&b') diff --git a/chrome/common/extensions/docs/server2/patch_servlet.py b/chrome/common/extensions/docs/server2/patch_servlet.py index 8704e38..8207e1d 100644 --- a/chrome/common/extensions/docs/server2/patch_servlet.py +++ b/chrome/common/extensions/docs/server2/patch_servlet.py @@ -28,24 +28,18 @@ class _PatchServletDelegate(RenderServlet.Delegate): self._issue = issue self._delegate = delegate - def CreateServerInstanceForChannel(self, channel): - base_object_store_creator = ObjectStoreCreator(channel, - start_empty=False) - branch_utility = self._delegate.CreateBranchUtility( - base_object_store_creator) + def CreateServerInstance(self): + object_store_creator = ObjectStoreCreator(start_empty=False) + branch_utility = self._delegate.CreateBranchUtility(object_store_creator) host_file_system_creator = self._delegate.CreateHostFileSystemCreator( - base_object_store_creator) + object_store_creator) # TODO(fj): Use OfflineFileSystem here once all json/idl files in api/ # are pulled into data store by cron jobs. - base_file_system = CachingFileSystem( - host_file_system_creator.Create( - branch_utility.GetChannelInfo(channel).branch), - base_object_store_creator) - base_compiled_fs_factory = CompiledFileSystem.Factory( - base_file_system, base_object_store_creator) + base_file_system = CachingFileSystem(host_file_system_creator.Create(), + object_store_creator) + base_compiled_fs_factory = CompiledFileSystem.Factory(base_file_system, + object_store_creator) - object_store_creator = ObjectStoreCreator('trunk@%s' % self._issue, - start_empty=False) rietveld_patcher = CachingRietveldPatcher( RietveldPatcher(svn_constants.EXTENSIONS_PATH, self._issue, @@ -60,11 +54,10 @@ class _PatchServletDelegate(RenderServlet.Delegate): [(patched_compiled_fs_factory, patched_file_system), (base_compiled_fs_factory, base_file_system)]) - return ServerInstance(channel, - object_store_creator, + return ServerInstance(object_store_creator, patched_file_system, self._delegate.CreateAppSamplesFileSystem( - base_object_store_creator), + object_store_creator), '/_patch/%s' % self._issue, compiled_fs_factory, branch_utility, @@ -90,16 +83,16 @@ class PatchServlet(Servlet): path_with_issue = self._request.path.lstrip('/') if '/' in path_with_issue: - issue, real_path = path_with_issue.split('/', 1) + issue, path_without_issue = path_with_issue.split('/', 1) else: return Response.NotFound('Malformed URL. It should look like ' + 'https://developer.chrome.com/_patch/12345/extensions/...') - fake_path = '/trunk/%s' % real_path - try: response = RenderServlet( - Request(fake_path, self._request.host, self._request.headers), + Request(path_without_issue, + self._request.host, + self._request.headers), _PatchServletDelegate(issue, self._delegate)).Get() # Disable cache for patched content. response.headers.pop('cache-control', None) @@ -108,8 +101,6 @@ class PatchServlet(Servlet): redirect_url, permanent = response.GetRedirect() if redirect_url is not None: - if redirect_url.startswith('/trunk/'): - redirect_url = redirect_url.split('/trunk', 1)[1] response = Response.Redirect('/_patch/%s%s' % (issue, redirect_url), permanent) return response diff --git a/chrome/common/extensions/docs/server2/patch_servlet_test.py b/chrome/common/extensions/docs/server2/patch_servlet_test.py index 1c38732..70a6f2f 100755 --- a/chrome/common/extensions/docs/server2/patch_servlet_test.py +++ b/chrome/common/extensions/docs/server2/patch_servlet_test.py @@ -18,7 +18,7 @@ from test_util import DisableLogging _ALLOWED_HOST = 'https://chrome-apps-doc.appspot.com' class _RenderServletDelegate(RenderServlet.Delegate): - def CreateServerInstanceForChannel(self, channel): + def CreateServerInstance(self): return ServerInstance.ForLocal() class _PatchServletDelegate(RenderServlet.Delegate): @@ -36,8 +36,8 @@ class PatchServletTest(unittest.TestCase): ConfigureFakeFetchers() def _RenderWithPatch(self, path, issue): - real_path = '%s/%s' % (issue, path) - return PatchServlet(Request.ForTest(real_path, host=_ALLOWED_HOST), + path_with_issue = '%s/%s' % (issue, path) + return PatchServlet(Request.ForTest(path_with_issue, host=_ALLOWED_HOST), _PatchServletDelegate()).Get() def _RenderWithoutPatch(self, path): @@ -68,8 +68,10 @@ class PatchServletTest(unittest.TestCase): @DisableLogging('warning') def _AssertNotFound(self, path, issue): - self.assertEqual(self._RenderWithPatch(path, issue).status, 404, - 'Path %s with issue %s should have been removed.' % (path, issue)) + response = self._RenderWithPatch(path, issue) + self.assertEqual(response.status, 404, + 'Path %s with issue %s should have been removed for %s.' % ( + path, issue, response)) def _AssertOk(self, path, issue): response = self._RenderWithPatch(path, issue) @@ -79,17 +81,27 @@ class PatchServletTest(unittest.TestCase): 'Rendered result for path %s with issue %s should not be empty.' % (path, issue)) + def _AssertRedirect(self, path, issue, redirect_path): + response = self._RenderWithPatch(path, issue) + self.assertEqual(302, response.status) + self.assertEqual('/_patch/%s/%s' % (issue, redirect_path), + response.headers['Location']) + def testRender(self): # '_patch' is not included in paths below because it's stripped by Handler. issue = '14096030' # extensions_sidenav.json is modified in the patch. self._RenderAndAssertNotEqual('extensions/index.html', issue) + # apps_sidenav.json is not patched. self._RenderAndAssertEqual('apps/about_apps.html', issue) - # extensions/runtime.html is removed in the patch. - self._AssertNotFound('extensions/runtime.html', issue) + # extensions/runtime.html is removed in the patch, should redirect to the + # apps version. + self._AssertRedirect('extensions/runtime.html', issue, + 'apps/runtime.html') + # apps/runtime.html is not removed. self._RenderAndAssertEqual('apps/runtime.html', issue) diff --git a/chrome/common/extensions/docs/server2/patched_file_system.py b/chrome/common/extensions/docs/server2/patched_file_system.py index 5828a7c..c287c58 100644 --- a/chrome/common/extensions/docs/server2/patched_file_system.py +++ b/chrome/common/extensions/docs/server2/patched_file_system.py @@ -156,3 +156,8 @@ class PatchedFileSystem(FileSystem): else: raise FileNotFoundError('%s was not in child versions' % filename) return stat_info + + def GetIdentity(self): + return '%s(%s,%s)' % (self.__class__.__name__, + self._host_file_system.GetIdentity(), + self._patcher.GetIdentity()) diff --git a/chrome/common/extensions/docs/server2/patcher.py b/chrome/common/extensions/docs/server2/patcher.py index 34fd17f..57d1e13 100644 --- a/chrome/common/extensions/docs/server2/patcher.py +++ b/chrome/common/extensions/docs/server2/patcher.py @@ -4,19 +4,25 @@ class Patcher(object): def GetPatchedFiles(self, version=None): - ''' Returns patched files as(added_files, deleted_files, modified_files) + '''Returns patched files as(added_files, deleted_files, modified_files) from the patchset specified by |version|. ''' - raise NotImplementedError() + raise NotImplementedError(self.__class__) def GetVersion(self): - ''' Returns patch version. Returns None when nothing is patched by the + '''Returns patch version. Returns None when nothing is patched by the patcher. ''' - raise NotImplementedError() + raise NotImplementedError(self.__class__) def Apply(self, paths, file_system, binary, version=None): - ''' Apply the patch to added/modified files. Returns Future with patched + '''Apply the patch to added/modified files. Returns Future with patched data. Throws FileNotFoundError if |paths| contains deleted files. ''' - raise NotImplementedError() + raise NotImplementedError(self.__class__) + + def GetIdentity(self): + '''Returns a string that identifies this patch. Typically it would be the + codereview server's ID for this patch. + ''' + raise NotImplementedError(self.__class__) diff --git a/chrome/common/extensions/docs/server2/path_canonicalizer.py b/chrome/common/extensions/docs/server2/path_canonicalizer.py index 8b5f77c..b4498a7 100644 --- a/chrome/common/extensions/docs/server2/path_canonicalizer.py +++ b/chrome/common/extensions/docs/server2/path_canonicalizer.py @@ -2,48 +2,106 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import logging import os +import posixpath +import traceback + +from branch_utility import BranchUtility +from file_system import FileNotFoundError from third_party.json_schema_compiler.model import UnixName import svn_constants +def _SimplifyFileName(file_name): + return (posixpath.splitext(file_name)[0] + .lower() + .replace('.', '') + .replace('-', '') + .replace('_', '')) + class PathCanonicalizer(object): '''Transforms paths into their canonical forms. Since the dev server has had many incarnations - e.g. there didn't use to be apps/ - there may be old paths lying around the webs. We try to redirect those to where they are now. ''' - def __init__(self, channel, compiled_fs_factory): - self._channel = channel - self._identity_fs = compiled_fs_factory.CreateIdentity(PathCanonicalizer) + def __init__(self, compiled_fs_factory): + # Map of simplified API names (for typo detection) to their real paths. + def make_public_apis(_, file_names): + return dict((_SimplifyFileName(name), name) for name in file_names) + self._public_apis = compiled_fs_factory.Create(make_public_apis, + PathCanonicalizer) def Canonicalize(self, path): - starts_with_channel, path_without_channel = (False, path) - if path.startswith('%s/' % self._channel): - starts_with_channel, path_without_channel = ( - True, path[len(self._channel) + 1:]) - - if any(path.startswith(prefix) - for prefix in ('extensions/', 'apps/', 'static/')): - return path - - if '/' in path_without_channel or path_without_channel == '404.html': - return path - - apps_templates = self._identity_fs.GetFromFileListing( - '/'.join((svn_constants.PUBLIC_TEMPLATE_PATH, 'apps'))) - extensions_templates = self._identity_fs.GetFromFileListing( - '/'.join((svn_constants.PUBLIC_TEMPLATE_PATH, 'extensions'))) - - if self._channel is None or not starts_with_channel: - apps_path = 'apps/%s' % path_without_channel - extensions_path = 'extensions/%s' % path_without_channel + '''Returns the canonical path for |path|, and whether that path is a + permanent canonicalisation (e.g. when we redirect from a channel to a + channel-less URL) or temporary (e.g. when we redirect from an apps-only API + to an extensions one - we may at some point enable it for extensions). + ''' + class ReturnType(object): + def __init__(self, path, permanent): + self.path = path + self.permanent = permanent + + # Catch incorrect comparisons by disabling ==/!=. + def __eq__(self, _): raise NotImplementedError() + def __ne__(self, _): raise NotImplementedError() + + # Strip any channel info off it. There are no channels anymore. + for channel_name in BranchUtility.GetAllChannelNames(): + channel_prefix = channel_name + '/' + if path.startswith(channel_prefix): + # Redirect now so that we can set the permanent-redirect bit. Channel + # redirects are the only things that should be permanent redirects; + # anything else *could* change, so is temporary. + return ReturnType(path[len(channel_prefix):], True) + + # No further work needed for static. + if path.startswith('static/'): + return ReturnType(path, False) + + # People go to just "extensions" or "apps". Redirect to the directory. + if path in ('extensions', 'apps'): + return ReturnType(path + '/', False) + + # The rest of this function deals with trying to figure out what API page + # for extensions/apps to redirect to, if any. We see a few different cases + # here: + # - Unqualified names ("browserAction.html"). These are easy to resolve; + # figure out whether it's an extension or app API and redirect. + # - but what if it's both? Well, assume extensions. Maybe later we can + # check analytics and see which is more popular. + # - Wrong names ("apps/browserAction.html"). This really does happen, + # damn it, so do the above logic but record which is the default. + if path.startswith(('extensions/', 'apps/')): + default_platform, reference_path = path.split('/', 1) + else: + default_platform, reference_path = ('extensions', path) + + try: + apps_public = self._public_apis.GetFromFileListing( + '/'.join((svn_constants.PUBLIC_TEMPLATE_PATH, 'apps'))) + extensions_public = self._public_apis.GetFromFileListing( + '/'.join((svn_constants.PUBLIC_TEMPLATE_PATH, 'extensions'))) + except FileNotFoundError: + # Probably offline. + logging.warning(traceback.format_exc()) + return ReturnType(path, False) + + simple_reference_path = _SimplifyFileName(reference_path) + apps_path = apps_public.get(simple_reference_path) + extensions_path = extensions_public.get(simple_reference_path) + + if apps_path is None: + if extensions_path is None: + # No idea. Just return the original path. It'll probably 404. + pass + else: + path = 'extensions/%s' % extensions_path else: - apps_path = '%s/apps/%s' % (self._channel, path_without_channel) - extensions_path = '%s/extensions/%s' % (self._channel, - path_without_channel) - - unix_path = UnixName(os.path.splitext(path_without_channel)[0]) - if unix_path in extensions_templates: - return extensions_path - if unix_path in apps_templates: - return apps_path - return extensions_path + if extensions_path is None: + path = 'apps/%s' % apps_path + else: + assert apps_path == extensions_path + path = '%s/%s' % (default_platform, apps_path) + + return ReturnType(path, False) diff --git a/chrome/common/extensions/docs/server2/path_canonicalizer_test.py b/chrome/common/extensions/docs/server2/path_canonicalizer_test.py index 841dc98..18feb0b 100755 --- a/chrome/common/extensions/docs/server2/path_canonicalizer_test.py +++ b/chrome/common/extensions/docs/server2/path_canonicalizer_test.py @@ -3,92 +3,84 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -from compiled_file_system import CompiledFileSystem -from path_canonicalizer import PathCanonicalizer -import svn_constants -from object_store_creator import ObjectStoreCreator -from test_file_system import TestFileSystem import unittest -_TEST_DATA = TestFileSystem.MoveTo(svn_constants.PUBLIC_TEMPLATE_PATH, { - 'extensions': { - 'browserAction.html': 'yo', - 'storage.html': 'dawg', - }, - 'apps': { - 'bluetooth': 'hey', - 'storage.html': 'wassup', - } -}) +from path_canonicalizer import PathCanonicalizer +from server_instance import ServerInstance +import svn_constants class PathCanonicalizerTest(unittest.TestCase): def setUp(self): - test_fs = TestFileSystem(_TEST_DATA) - compiled_fs_factory = CompiledFileSystem.Factory( - test_fs, - ObjectStoreCreator.ForTest()) - self._path_canonicalizer = PathCanonicalizer('stable', compiled_fs_factory) + self._server_instance = ServerInstance.ForLocal() + + def _Cze(self, path): + return self._server_instance.path_canonicalizer.Canonicalize(path) + + def testSpecifyCorrectly(self): + self._AssertIdentity('extensions/browserAction.html') + self._AssertIdentity('extensions/storage.html') + self._AssertIdentity('extensions/blah.html') + self._AssertIdentity('extensions/index.html') + self._AssertIdentity('extensions/whats_new.html') + self._AssertIdentity('apps/storage.html') + self._AssertIdentity('apps/bluetooth.html') + self._AssertIdentity('apps/blah.html') + self._AssertIdentity('static/browserAction.html') + self._AssertIdentity('static/storage.html') + self._AssertIdentity('static/bluetooth.html') + self._AssertIdentity('static/blah.html') + + def testSpecifyIncorrectly(self): + self._AssertTemporaryRedirect('extensions/browserAction.html', + 'apps/browserAction.html') + self._AssertTemporaryRedirect('apps/bluetooth.html', + 'extensions/bluetooth.html') + self._AssertTemporaryRedirect('extensions/index.html', + 'apps/index.html') + + def testUnspecified(self): + self._AssertTemporaryRedirect('extensions/browserAction.html', + 'browserAction.html') + self._AssertTemporaryRedirect('apps/bluetooth.html', + 'bluetooth.html') + # Extensions are default for now. + self._AssertTemporaryRedirect('extensions/storage.html', + 'storage.html') + # Nonexistent APIs should be left alone. + self._AssertIdentity('blah.html') - def _assertIdentity(self, path): - self.assertEqual(path, self._path_canonicalizer.Canonicalize(path)) + def testSpellingErrors(self): + for spelme in ('browseraction', 'browseraction.htm', 'BrowserAction', + 'BrowserAction.html', 'browseraction.html', 'Browseraction', + 'browser-action', 'Browser.action.html', 'browser_action', + 'browser-action.html', 'Browser_Action.html'): + self._AssertTemporaryRedirect('extensions/browserAction.html', spelme) + self._AssertTemporaryRedirect('extensions/browserAction.html', + 'extensions/%s' % spelme) + self._AssertTemporaryRedirect('extensions/browserAction.html', + 'apps/%s' % spelme) - def testExtensions(self): - self._assertIdentity('extensions/browserAction.html') - self._assertIdentity('extensions/storage.html') - self._assertIdentity('extensions/bluetooth.html') - self._assertIdentity('extensions/blah.html') - self._assertIdentity('stable/extensions/browserAction.html') - self._assertIdentity('stable/extensions/storage.html') - self._assertIdentity('stable/extensions/bluetooth.html') - self._assertIdentity('stable/extensions/blah.html') + def testChannelRedirect(self): + def assert_channel_redirect(channel, path): + self._AssertPermanentRedirect(path, '%s/%s' % (channel, path)) + for channel in ('stable', 'beta', 'dev', 'trunk'): + assert_channel_redirect(channel, 'extensions/browserAction.html') + assert_channel_redirect(channel, 'extensions/storage.html') + assert_channel_redirect(channel, 'apps/bluetooth.html') + assert_channel_redirect(channel, 'apps/storage.html') - def testApps(self): - self._assertIdentity('apps/browserAction.html') - self._assertIdentity('apps/storage.html') - self._assertIdentity('apps/bluetooth.html') - self._assertIdentity('apps/blah.html') - self._assertIdentity('stable/apps/browserAction.html') - self._assertIdentity('stable/apps/storage.html') - self._assertIdentity('stable/apps/bluetooth.html') - self._assertIdentity('stable/apps/blah.html') + def _AssertIdentity(self, path): + self._AssertTemporaryRedirect(path, path) - def testStatic(self): - self._assertIdentity('static/browserAction.html') - self._assertIdentity('static/storage.html') - self._assertIdentity('static/bluetooth.html') - self._assertIdentity('static/blah.html') - self._assertIdentity('stable/static/browserAction.html') - self._assertIdentity('stable/static/storage.html') - self._assertIdentity('stable/static/bluetooth.html') - self._assertIdentity('stable/static/blah.html') + def _AssertTemporaryRedirect(self, to, from_): + result = self._Cze(from_) + self.assertEqual(to, result.path) + self.assertFalse(result.permanent) - def testNeither(self): - self.assertEqual( - 'extensions/browserAction.html', - self._path_canonicalizer.Canonicalize('browserAction.html')) - self.assertEqual( - 'stable/extensions/browserAction.html', - self._path_canonicalizer.Canonicalize('stable/browserAction.html')) - self.assertEqual( - 'extensions/storage.html', - self._path_canonicalizer.Canonicalize('storage.html')) - self.assertEqual( - 'stable/extensions/storage.html', - self._path_canonicalizer.Canonicalize('stable/storage.html')) - self.assertEqual( - 'apps/bluetooth.html', - self._path_canonicalizer.Canonicalize('bluetooth.html')) - self.assertEqual( - 'stable/apps/bluetooth.html', - self._path_canonicalizer.Canonicalize('stable/bluetooth.html')) - # Assign non-existent paths to extensions because they came first, so such - # paths are more likely to be for extensions. - self.assertEqual( - 'extensions/blah.html', - self._path_canonicalizer.Canonicalize('blah.html')) - self.assertEqual( - 'stable/extensions/blah.html', - self._path_canonicalizer.Canonicalize('stable/blah.html')) + def _AssertPermanentRedirect(self, to, from_): + result = self._Cze(from_) + self.assertEqual(to, result.path) + self.assertTrue(result.permanent) if __name__ == '__main__': unittest.main() diff --git a/chrome/common/extensions/docs/server2/preview.py b/chrome/common/extensions/docs/server2/preview.py index d645abb..f09f0cf 100755 --- a/chrome/common/extensions/docs/server2/preview.py +++ b/chrome/common/extensions/docs/server2/preview.py @@ -23,9 +23,6 @@ # ./preview.py -r extensions/tabs.html # # will output the documentation for the tabs API on stdout and exit immediately. -# -# Note: absolute paths into static content (e.g. /static/css/site.css) will be -# relative paths (e.g. static/css/site.css) for convenient sandboxing. # NOTE: RUN THIS FIRST. Or all third_party imports will fail. import build_server @@ -91,9 +88,7 @@ if __name__ == '__main__': if opts.time: print('Took %s seconds' % (time.time() - start_time)) else: - # Static paths will show up as /trunk/static/foo but this only makes - # sense from a webserver. - print(response.content.ToString().replace('/trunk/', '')) + print(response.content.ToString()) exit() print('Starting previewserver on port %s' % opts.port) diff --git a/chrome/common/extensions/docs/server2/render_servlet.py b/chrome/common/extensions/docs/server2/render_servlet.py index 5b29c3d..77245d6 100644 --- a/chrome/common/extensions/docs/server2/render_servlet.py +++ b/chrome/common/extensions/docs/server2/render_servlet.py @@ -8,7 +8,6 @@ import mimetypes import traceback from urlparse import urlsplit -from branch_utility import BranchUtility from file_system import FileNotFoundError from servlet import Servlet, Response import svn_constants @@ -21,43 +20,34 @@ class RenderServlet(Servlet): '''Servlet which renders templates. ''' class Delegate(object): - def CreateServerInstanceForChannel(self, channel): - raise NotImplementedError() + def CreateServerInstance(self): + raise NotImplementedError(self.__class__) - def __init__(self, request, delegate, default_channel='stable'): + def __init__(self, request, delegate): Servlet.__init__(self, request) self._delegate = delegate - self._default_channel = default_channel def Get(self): ''' Render the page for a request. ''' - headers = self._request.headers - channel, path = BranchUtility.SplitChannelNameFromPath(self._request.path) + # TODO(kalman): a consistent path syntax (even a Path class?) so that we + # can stop being so conservative with stripping and adding back the '/'s. + path = self._request.path.lstrip('/') if path.split('/')[-1] == 'redirects.json': return Response.Ok('') - if channel == self._default_channel: - return Response.Redirect('/' + path) - if channel is None: - channel = self._default_channel - - server_instance = self._delegate.CreateServerInstanceForChannel(channel) + server_instance = self._delegate.CreateServerInstance() redirect = server_instance.redirector.Redirect(self._request.host, path) if redirect is not None: - if (channel != self._default_channel and - not urlsplit(redirect).scheme in ('http', 'https')): - redirect = '/%s%s' % (channel, redirect) return Response.Redirect(redirect) - canonical_path = server_instance.path_canonicalizer.Canonicalize(path) - redirect = canonical_path.lstrip('/') + canonical_result = server_instance.path_canonicalizer.Canonicalize(path) + redirect = canonical_result.path.lstrip('/') if path != redirect: - if channel is not None: - redirect = '%s/%s' % (channel, canonical_path) - return Response.Redirect('/' + redirect) + return Response.Redirect('/' + redirect, + permanent=canonical_result.permanent) templates = server_instance.template_data_source_factory.Create( self._request, path) diff --git a/chrome/common/extensions/docs/server2/render_servlet_test.py b/chrome/common/extensions/docs/server2/render_servlet_test.py index 75c91fe..f188671 100755 --- a/chrome/common/extensions/docs/server2/render_servlet_test.py +++ b/chrome/common/extensions/docs/server2/render_servlet_test.py @@ -8,23 +8,25 @@ import unittest from local_file_system import LocalFileSystem from render_servlet import RenderServlet from server_instance import ServerInstance -from servlet import Request +from servlet import Request, Response from test_util import DisableLogging, ReadFile class _RenderServletDelegate(RenderServlet.Delegate): - def CreateServerInstanceForChannel(self, channel): + def CreateServerInstance(self): return ServerInstance.ForTest(LocalFileSystem.Create()) class RenderServletTest(unittest.TestCase): def testExtensionAppRedirect(self): request = Request.ForTest('storage.html') - response = RenderServlet(request, _RenderServletDelegate()).Get() - self.assertEqual(302, response.status) + self.assertEqual( + Response.Redirect('/extensions/storage.html', permanent=False), + RenderServlet(request, _RenderServletDelegate()).Get()) - def testDefaultChannel(self): + def testChannelRedirect(self): request = Request.ForTest('stable/extensions/storage.html') - response = RenderServlet(request, _RenderServletDelegate()).Get() - self.assertEqual(302, response.status) + self.assertEqual( + Response.Redirect('/extensions/storage.html', permanent=True), + RenderServlet(request, _RenderServletDelegate()).Get()) @DisableLogging('warning') def testNotFound(self): diff --git a/chrome/common/extensions/docs/server2/rietveld_patcher.py b/chrome/common/extensions/docs/server2/rietveld_patcher.py index f480c0c..adc1b95 100644 --- a/chrome/common/extensions/docs/server2/rietveld_patcher.py +++ b/chrome/common/extensions/docs/server2/rietveld_patcher.py @@ -174,3 +174,6 @@ class RietveldPatcher(Patcher): paths, binary, self._fetcher)) + + def GetIdentity(self): + return self._issue diff --git a/chrome/common/extensions/docs/server2/samples_data_source.py b/chrome/common/extensions/docs/server2/samples_data_source.py index 9f150e7..790a2cd7 100644 --- a/chrome/common/extensions/docs/server2/samples_data_source.py +++ b/chrome/common/extensions/docs/server2/samples_data_source.py @@ -13,7 +13,7 @@ import third_party.json_schema_compiler.json_comment_eater as json_comment_eater import third_party.json_schema_compiler.model as model import url_constants -DEFAULT_ICON_PATH = '/images/sample-default-icon.png' +DEFAULT_ICON_PATH = 'images/sample-default-icon.png' class SamplesDataSource(object): '''Constructs a list of samples and their respective files and api calls. @@ -23,18 +23,18 @@ class SamplesDataSource(object): Requests. ''' def __init__(self, - channel, host_file_system, compiled_host_fs_factory, app_samples_file_system, compiled_app_samples_fs_factory, ref_resolver_factory, - extension_samples_path): + extension_samples_path, + base_path): self._host_file_system = host_file_system self._app_samples_file_system = app_samples_file_system - self._static_path = '/%s/static' % channel self._ref_resolver = ref_resolver_factory.Create() self._extension_samples_path = extension_samples_path + self._base_path = base_path self._extensions_cache = compiled_host_fs_factory.Create( self._MakeSamplesList, SamplesDataSource, @@ -50,6 +50,7 @@ class SamplesDataSource(object): return SamplesDataSource(self._extensions_cache, self._apps_cache, self._extension_samples_path, + self._base_path, request) def _GetAPIItems(self, js_file): @@ -159,9 +160,9 @@ class SamplesDataSource(object): download_url = sample_base_path + '.zip' if manifest_data['icon'] is None: - icon_path = self._static_path + DEFAULT_ICON_PATH + icon_path = '%s/static/%s' % (self._base_path, DEFAULT_ICON_PATH) else: - icon_path = icon_base + '/' + manifest_data['icon'] + icon_path = '%s/%s' % (icon_base, manifest_data['icon']) manifest_data.update({ 'icon': icon_path, 'id': hashlib.md5(url).hexdigest(), @@ -178,10 +179,12 @@ class SamplesDataSource(object): extensions_cache, apps_cache, extension_samples_path, + base_path, request): self._extensions_cache = extensions_cache self._apps_cache = apps_cache self._extension_samples_path = extension_samples_path + self._base_path = base_path self._request = request def _GetAcceptedLanguages(self): diff --git a/chrome/common/extensions/docs/server2/samples_data_source_test.py b/chrome/common/extensions/docs/server2/samples_data_source_test.py index 1e6052c..7977ca0 100755 --- a/chrome/common/extensions/docs/server2/samples_data_source_test.py +++ b/chrome/common/extensions/docs/server2/samples_data_source_test.py @@ -25,7 +25,7 @@ class SamplesDataSourceTest(unittest.TestCase): return json.loads(self._ReadLocalFile(key)) def testFilterSamples(self): - sds = SamplesDataSource({}, {}, 'fake_path', Request.ForTest('/')) + sds = SamplesDataSource({}, {}, 'fake_path', '.', Request.ForTest('/')) sds.get = self._FakeGet self.assertEquals(json.loads(self._ReadLocalFile('expected.json')), sds.FilterSamples('samples.json', 'bobaloo')) diff --git a/chrome/common/extensions/docs/server2/server_instance.py b/chrome/common/extensions/docs/server2/server_instance.py index 251cee5..379cf98 100644 --- a/chrome/common/extensions/docs/server2/server_instance.py +++ b/chrome/common/extensions/docs/server2/server_instance.py @@ -37,7 +37,6 @@ import url_constants class ServerInstance(object): def __init__(self, - channel, object_store_creator, host_file_system, app_samples_file_system, @@ -45,8 +44,6 @@ class ServerInstance(object): compiled_fs_factory, branch_utility, host_file_system_creator): - self.channel = channel - self.object_store_creator = object_store_creator self.host_file_system = host_file_system @@ -88,14 +85,14 @@ class ServerInstance(object): else: extension_samples_fs = self.host_file_system self.samples_data_source_factory = SamplesDataSource.Factory( - channel, extension_samples_fs, CompiledFileSystem.Factory(extension_samples_fs, object_store_creator), self.app_samples_file_system, CompiledFileSystem.Factory(self.app_samples_file_system, object_store_creator), self.ref_resolver_factory, - svn_constants.EXAMPLES_PATH) + svn_constants.EXAMPLES_PATH, + base_path) self.api_data_source_factory.SetSamplesDataSourceFactory( self.samples_data_source_factory) @@ -107,11 +104,9 @@ class ServerInstance(object): self.sidenav_data_source_factory = SidenavDataSource.Factory( self.compiled_host_fs_factory, - svn_constants.JSON_PATH, - base_path) + svn_constants.JSON_PATH) self.template_data_source_factory = TemplateDataSource.Factory( - channel, self.api_data_source_factory, self.api_list_data_source_factory, self.intro_data_source_factory, @@ -130,9 +125,7 @@ class ServerInstance(object): self.compiled_host_fs_factory, svn_constants.DOCS_PATH) - self.path_canonicalizer = PathCanonicalizer( - channel, - self.compiled_host_fs_factory) + self.path_canonicalizer = PathCanonicalizer(self.compiled_host_fs_factory) # TODO(kalman): delete content cache. self.content_cache = self.compiled_host_fs_factory.CreateIdentity( @@ -146,8 +139,7 @@ class ServerInstance(object): @staticmethod def ForTest(file_system): object_store_creator = ObjectStoreCreator.ForTest() - return ServerInstance('test', - object_store_creator, + return ServerInstance(object_store_creator, file_system, EmptyDirFileSystem(), '', @@ -159,15 +151,12 @@ class ServerInstance(object): @staticmethod def ForLocal(): - channel = 'trunk' - object_store_creator = ObjectStoreCreator(channel, - start_empty=False, + object_store_creator = ObjectStoreCreator(start_empty=False, store_type=TestObjectStore) host_file_system_creator = HostFileSystemCreator.ForLocal( object_store_creator) - trunk_file_system = host_file_system_creator.Create('trunk') + trunk_file_system = host_file_system_creator.Create() return ServerInstance( - channel, object_store_creator, trunk_file_system, EmptyDirFileSystem(), diff --git a/chrome/common/extensions/docs/server2/servlet.py b/chrome/common/extensions/docs/server2/servlet.py index 35399b4..59ebb5b 100644 --- a/chrome/common/extensions/docs/server2/servlet.py +++ b/chrome/common/extensions/docs/server2/servlet.py @@ -15,8 +15,8 @@ class Request(object): return Request(path, host, headers or {}) def __repr__(self): - return 'Request(path=%s, host=%s, headers=%s entries)' % ( - self.path, self.host, len(self.headers.keys())) + return 'Request(path=%s, host=%s, headers=%s)' % ( + self.path, self.host, self.headers) def __str__(self): return repr(self) @@ -105,9 +105,18 @@ class Response(object): return (None, None) return (self.headers.get('Location'), self.status == 301) + def __eq__(self, other): + return (isinstance(other, self.__class__) and + str(other.content) == str(self.content) and + other.headers == self.headers and + other.status == self.status) + + def __ne__(self, other): + return not (self == other) + def __repr__(self): - return 'Response(content=%s bytes, status=%s, headers=%s entries)' % ( - len(self.content), self.status, len(self.headers.keys())) + return 'Response(content=%s bytes, status=%s, headers=%s)' % ( + len(self.content), self.status, self.headers) def __str__(self): return repr(self) diff --git a/chrome/common/extensions/docs/server2/sidenav_data_source.py b/chrome/common/extensions/docs/server2/sidenav_data_source.py index 98d7fd2..bf46727 100644 --- a/chrome/common/extensions/docs/server2/sidenav_data_source.py +++ b/chrome/common/extensions/docs/server2/sidenav_data_source.py @@ -13,21 +13,17 @@ class SidenavDataSource(object): menu. """ class Factory(object): - def __init__(self, compiled_fs_factory, json_path, base_path): + def __init__(self, compiled_fs_factory, json_path): self._cache = compiled_fs_factory.Create(self._CreateSidenavDict, SidenavDataSource) self._json_path = json_path - self._base_path = base_path def Create(self, path): """Create a SidenavDataSource, binding it to |path|. |path| is the url of the page that is being rendered. It is used to determine which item in the sidenav should be highlighted. """ - return SidenavDataSource(self._cache, - self._json_path, - path, - self._base_path) + return SidenavDataSource(self._cache, self._json_path, path) def _AddLevels(self, items, level): """Levels represent how deeply this item is nested in the sidenav. We @@ -43,11 +39,10 @@ class SidenavDataSource(object): self._AddLevels(items, 2); return items - def __init__(self, cache, json_path, path, base_path): + def __init__(self, cache, json_path, path): self._cache = cache self._json_path = json_path self._href = '/' + path - self._file_dir = base_path def _AddSelected(self, items): for item in items: @@ -70,7 +65,7 @@ class SidenavDataSource(object): if not href.startswith('/'): logging.warn('Paths in sidenav must be qualified. %s is not.' % href) href = '/' + href - item['href'] = '%s%s' % (self._file_dir, href) + item['href'] = href def get(self, key): sidenav = copy.deepcopy(self._cache.GetFromFile( diff --git a/chrome/common/extensions/docs/server2/sidenav_data_source_test.py b/chrome/common/extensions/docs/server2/sidenav_data_source_test.py index 827801a5..e7eddc1 100755 --- a/chrome/common/extensions/docs/server2/sidenav_data_source_test.py +++ b/chrome/common/extensions/docs/server2/sidenav_data_source_test.py @@ -35,16 +35,14 @@ class SamplesDataSourceTest(unittest.TestCase): def testLevels(self): sidenav_data_source = SidenavDataSource.Factory(self._compiled_fs_factory, - self._json_path, - '').Create('') + self._json_path).Create('') sidenav_json = sidenav_data_source.get('test') self._CheckLevels(sidenav_json) def testSelected(self): sidenav_data_source = SidenavDataSource.Factory( self._compiled_fs_factory, - self._json_path, - '').Create('www.b.com') + self._json_path).Create('www.b.com') sidenav_json = sidenav_data_source.get('test') # This will be prettier once JSON is loaded with an OrderedDict. for item in sidenav_json: @@ -60,8 +58,7 @@ class SamplesDataSourceTest(unittest.TestCase): def testAbsolutePath(self): sidenav_data_source = SidenavDataSource.Factory( self._compiled_fs_factory, - self._json_path, - '/trunk').Create('absolute_path/test.html') + self._json_path).Create('absolute_path/test.html') sidenav_json = sidenav_data_source.get('absolute_path') self.assertEqual( sidenav_json, diff --git a/chrome/common/extensions/docs/server2/subversion_file_system.py b/chrome/common/extensions/docs/server2/subversion_file_system.py index c0ff68c..62e0fc4 100644 --- a/chrome/common/extensions/docs/server2/subversion_file_system.py +++ b/chrome/common/extensions/docs/server2/subversion_file_system.py @@ -54,12 +54,11 @@ class SubversionFileSystem(FileSystem): '''Class to fetch resources from src.chromium.org. ''' @staticmethod - def Create(branch, revision=None): + def Create(branch='trunk', revision=None): if branch == 'trunk': svn_path = 'trunk/src/%s' % svn_constants.EXTENSIONS_PATH else: - svn_path = 'branches/%s/src/%s' % (branch, - svn_constants.EXTENSIONS_PATH) + svn_path = 'branches/%s/src/%s' % (branch, svn_constants.EXTENSIONS_PATH) return SubversionFileSystem( AppEngineUrlFetcher('%s/%s' % (url_constants.SVN_URL, svn_path)), AppEngineUrlFetcher('%s/%s' % (url_constants.VIEWVC_URL, svn_path)), diff --git a/chrome/common/extensions/docs/server2/template_data_source.py b/chrome/common/extensions/docs/server2/template_data_source.py index 7a1f6e1..ca065d33 100644 --- a/chrome/common/extensions/docs/server2/template_data_source.py +++ b/chrome/common/extensions/docs/server2/template_data_source.py @@ -4,7 +4,6 @@ import logging -from branch_utility import BranchUtility from docs_server_utils import FormatKey from file_system import FileNotFoundError from third_party.handlebar import Handlebar @@ -22,16 +21,6 @@ _STRING_CONSTANTS = { 'properties': 'properties', } -def _MakeChannelDict(channel_name): - channel_dict = { - 'channels': [{'name': name} for name in BranchUtility.GetAllChannelNames()], - 'current': channel_name - } - for channel in channel_dict['channels']: - if channel['name'] == channel_name: - channel['isCurrent'] = True - return channel_dict - class TemplateDataSource(object): """Renders Handlebar templates, providing them with the context in which to render. @@ -49,7 +38,6 @@ class TemplateDataSource(object): individual Requests. """ def __init__(self, - channel_name, api_data_source_factory, api_list_data_source_factory, intro_data_source_factory, @@ -60,7 +48,6 @@ class TemplateDataSource(object): public_template_path, private_template_path, base_path): - self._branch_info = _MakeChannelDict(channel_name) self._api_data_source_factory = api_data_source_factory self._api_list_data_source_factory = api_list_data_source_factory self._intro_data_source_factory = intro_data_source_factory @@ -71,7 +58,7 @@ class TemplateDataSource(object): self._ref_resolver = ref_resolver_factory.Create() self._public_template_path = public_template_path self._private_template_path = private_template_path - self._static_resources = '%s/static' % base_path + self._base_path = base_path def _CreateTemplate(self, template_name, text): return Handlebar(self._ref_resolver.ResolveAllLinks(text)) @@ -80,7 +67,6 @@ class TemplateDataSource(object): """Returns a new TemplateDataSource bound to |request|. """ return TemplateDataSource( - self._branch_info, self._api_data_source_factory.Create(request), self._api_list_data_source_factory.Create(), self._intro_data_source_factory.Create(), @@ -89,10 +75,9 @@ class TemplateDataSource(object): self._cache, self._public_template_path, self._private_template_path, - self._static_resources) + self._base_path) def __init__(self, - branch_info, api_data_source, api_list_data_source, intro_data_source, @@ -101,8 +86,7 @@ class TemplateDataSource(object): cache, public_template_path, private_template_path, - static_resources): - self._branch_info = branch_info + base_path): self._api_list_data_source = api_list_data_source self._intro_data_source = intro_data_source self._samples_data_source = samples_data_source @@ -111,7 +95,7 @@ class TemplateDataSource(object): self._cache = cache self._public_template_path = public_template_path self._private_template_path = private_template_path - self._static_resources = static_resources + self._base_path = base_path def Render(self, template_name): """This method will render a template named |template_name|, fetching all @@ -125,19 +109,13 @@ class TemplateDataSource(object): render_data = template.render({ 'api_list': self._api_list_data_source, 'apis': self._api_data_source, - 'branchInfo': self._branch_info, 'intros': self._intro_data_source, 'sidenavs': self._sidenav_data_source, 'partials': self, 'samples': self._samples_data_source, - 'static': self._static_resources, 'apps_samples_url': url_constants.GITHUB_BASE, - # TODO(kalman): this is wrong, it's always getting from trunk, but meh - # it hardly ever shows up (only in the "cannot fetch samples" message). - # In fact I don't even know if it can show up anymore due the samples data - # being persisent. In any case, when the channel distinctions are gone - # this can go away, so, double meh. 'extensions_samples_url': url_constants.EXTENSIONS_SAMPLES, + 'static': self._base_path + '/static', 'strings': _STRING_CONSTANTS, 'true': True, 'false': False diff --git a/chrome/common/extensions/docs/server2/template_data_source_test.py b/chrome/common/extensions/docs/server2/template_data_source_test.py index 3de80ba..7ac39d0 100755 --- a/chrome/common/extensions/docs/server2/template_data_source_test.py +++ b/chrome/common/extensions/docs/server2/template_data_source_test.py @@ -66,7 +66,6 @@ class TemplateDataSourceTest(unittest.TestCase): path = 'extensions/foo' return factory.Create(Request.ForTest(path), path) return create_from_factory(TemplateDataSource.Factory( - 'fake_channel', api_data_factory, self._fake_api_list_data_source_factory, self._fake_intro_data_source_factory, diff --git a/chrome/common/extensions/docs/server2/test_data/sidenav_data_source/absolute_path_sidenav_expected.json b/chrome/common/extensions/docs/server2/test_data/sidenav_data_source/absolute_path_sidenav_expected.json index 787a96d..4f24152 100644 --- a/chrome/common/extensions/docs/server2/test_data/sidenav_data_source/absolute_path_sidenav_expected.json +++ b/chrome/common/extensions/docs/server2/test_data/sidenav_data_source/absolute_path_sidenav_expected.json @@ -1,11 +1,11 @@ [ { "title": "H1", - "href": "/trunk/absolute_path/h1.html", + "href": "/absolute_path/h1.html", "level": 2 }, { - "href": "/trunk/absolute_path/h2.html", + "href": "/absolute_path/h2.html", "title": "H2", "level": 2, "items": [ @@ -20,7 +20,7 @@ }, { "title": "Y", - "href": "/trunk/absolute_path/y.html", + "href": "/absolute_path/y.html", "level": 4 }, { @@ -30,7 +30,7 @@ }, { "title": "X3", - "href": "/trunk/test.html", + "href": "/test.html", "level": 4 } ] @@ -49,7 +49,7 @@ }, { "title": "X3", - "href": "/trunk/test.html", + "href": "/test.html", "level": 2 } ] diff --git a/chrome/common/extensions/docs/server2/test_patcher.py b/chrome/common/extensions/docs/server2/test_patcher.py index e1516af..2e594df 100644 --- a/chrome/common/extensions/docs/server2/test_patcher.py +++ b/chrome/common/extensions/docs/server2/test_patcher.py @@ -33,3 +33,6 @@ class TestPatcher(Patcher): for path in paths)) except KeyError: raise FileNotFoundError('One of %s is deleted in the patch.' % paths) + + def GetIdentity(self): + return self.__class__.__name__ diff --git a/chrome/common/extensions/docs/static/css/site.css b/chrome/common/extensions/docs/static/css/site.css index 6a4aab7..5528b66 100644 --- a/chrome/common/extensions/docs/static/css/site.css +++ b/chrome/common/extensions/docs/static/css/site.css @@ -205,7 +205,6 @@ dd { font-family: inherit; font-size: inherit; font-style: inherit; - font-weight: bold; margin: 0; padding: 0; outline: none; } @@ -214,25 +213,12 @@ dd { outline: 1px dotted; } -#branch-chooser { +#platform-chooser { display: inline; position: relative; - font-weight: bold; -} -#branch-chooser .stable { - color: #0d68ae; -} -#branch-chooser .beta { - color: #409544; -} -#branch-chooser .dev { - color: #dba30d; -} -#branch-chooser .trunk { - color: #e02629; } -#branch-chooser-popup { +#platform-chooser-popup { /* Make it appear to be an extension of the header. */ background-color: white; border: 1px solid #F5F5F5; @@ -245,14 +231,14 @@ dd { left: -6px; } -#branch-chooser-popup button { +#platform-chooser-popup button { display: block; padding: 6px; width: 100%; text-align: left; } -#branch-chooser-popup button:hover { - background-color: #F5F5F5; +#platform-chooser-popup button:hover { + color: #4787ed; } button.google-button { diff --git a/chrome/common/extensions/docs/static/js/branch.js b/chrome/common/extensions/docs/static/js/branch.js deleted file mode 100644 index bd21497..0000000 --- a/chrome/common/extensions/docs/static/js/branch.js +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) 2012 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. - -(function() { - -function init() { - var branchChooser = document.getElementById('branch-chooser-popup'); - Array.prototype.forEach.call(branchChooser.getElementsByTagName('button'), - function(button) { - button.addEventListener('click', function(event) { - choose(button.className); - event.stopPropagation(); - }); - }); -} - -function choose(branch) { - var currentBranch = window.bootstrap.branchInfo.current; - var path = window.location.pathname.split('/'); - if (path[0] == '') - path = path.slice(1); - var index = path.indexOf(currentBranch); - if (index != -1) - path[index] = branch; - else - path.splice(0, 0, branch); - window.location.pathname = '/' + path.join('/'); -} - -init(); - -})() diff --git a/chrome/common/extensions/docs/static/js/platform_chooser.js b/chrome/common/extensions/docs/static/js/platform_chooser.js new file mode 100644 index 0000000..5492cff --- /dev/null +++ b/chrome/common/extensions/docs/static/js/platform_chooser.js @@ -0,0 +1,18 @@ +// Copyright 2013 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. + +// Intializes the platform chooser, the widget in the top left corner with the +// 'Apps...Extensions' dropdown. +(function() { + +var platformChooser = document.getElementById('platform-chooser-popup'); +Array.prototype.forEach.call(platformChooser.getElementsByTagName('button'), + function(button) { + button.addEventListener('click', function(event) { + window.location.assign(button.getAttribute('data-href')); + event.stopPropagation(); + }); +}); + +})() diff --git a/chrome/common/extensions/docs/templates/private/apps_footer.html b/chrome/common/extensions/docs/templates/private/apps_footer.html index f997b85..155c851 100644 --- a/chrome/common/extensions/docs/templates/private/apps_footer.html +++ b/chrome/common/extensions/docs/templates/private/apps_footer.html @@ -1,8 +1,7 @@ <script> window.bootstrap = { api_names: {{*api_list.apps.chrome}}.concat( - {{*api_list.apps.experimental}}), - branchInfo: {{*branchInfo}} + {{*api_list.apps.experimental}}) }; </script> {{+partials.footer}} diff --git a/chrome/common/extensions/docs/templates/private/extensions_footer.html b/chrome/common/extensions/docs/templates/private/extensions_footer.html index 14d498a..c8f73aa 100644 --- a/chrome/common/extensions/docs/templates/private/extensions_footer.html +++ b/chrome/common/extensions/docs/templates/private/extensions_footer.html @@ -1,8 +1,7 @@ <script> window.bootstrap = { api_names: {{*api_list.extensions.chrome}}.concat( - {{*api_list.extensions.experimental}}), - branchInfo: {{*branchInfo}} + {{*api_list.extensions.experimental}}) }; </script> {{+partials.footer}} diff --git a/chrome/common/extensions/docs/templates/private/footer.html b/chrome/common/extensions/docs/templates/private/footer.html index f713c97..7dd195c 100644 --- a/chrome/common/extensions/docs/templates/private/footer.html +++ b/chrome/common/extensions/docs/templates/private/footer.html @@ -11,7 +11,7 @@ <p> ©2013 Google </p> - <script src="{{static}}/js/branch.js" type="text/javascript"></script> + <script src="{{static}}/js/platform_chooser.js" type="text/javascript"></script> <script src="{{static}}/js/popup.js" type="text/javascript"></script> <script src="{{static}}/js/prettify.js" type="text/javascript"></script> <script src="{{static}}/js/scroll.js" type="text/javascript"></script> diff --git a/chrome/common/extensions/docs/templates/private/header_body.html b/chrome/common/extensions/docs/templates/private/header_body.html index fa7149c..23f6bc9 100644 --- a/chrome/common/extensions/docs/templates/private/header_body.html +++ b/chrome/common/extensions/docs/templates/private/header_body.html @@ -1,21 +1,16 @@ <div id="gc-topnav"> <table><tr> <td id="chrome-logo"> - <a href="." title="Google Chrome {{title}}"> - <img src="{{static}}/images/chrome-logo.png" alt=""> - chrome {{platform}}s + <a href="/{{platform}}s/" title="Google Chrome {{title}}"> + <img src="{{static}}/images/chrome-logo.png" alt=""> chrome </a> - <div id="branch-chooser"> - <button id="branch-chooser-toggle" - class="{{branchInfo.current}}" data-menu="branch-chooser-popup"> - {{branchInfo.current}} ▾  + <div id="platform-chooser"> + <button id="platform-chooser-toggle" data-menu="platform-chooser-popup"> + {{platform}}s ▾  </button> - <div id="branch-chooser-popup"> - {{#branchInfo.channels}} - {{^isCurrent}} - <button class="{{name}}">{{name}}</button> - {{/isCurrent}} - {{/branchInfo.channels}} + <div id="platform-chooser-popup"> + <button class="apps" data-href="/apps/about_apps.html">apps</button> + <button class="extensions" data-href="/extensions">extensions</button> </div> </div> </td> |