Devserver: allow SubversionFileSystem to be pinned to a specific rev on construction

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

Index: chrome/common/extensions/docs/server2/cron_servlet.py
diff --git a/chrome/common/extensions/docs/server2/cron_servlet.py b/chrome/common/extensions/docs/server2/cron_servlet.py
index e76f07423e2865d738b86011246ca2ca13edfca2..81dac00c265c5d0399c75d506201ff0125166bf0 100644
--- a/chrome/common/extensions/docs/server2/cron_servlet.py
+++ b/chrome/common/extensions/docs/server2/cron_servlet.py
@@ -6,9 +6,12 @@ import logging
 import time
 import traceback
 
-from appengine_wrappers import DeadlineExceededError, IsDevServer, logservice
+from app_yaml_helper import AppYamlHelper
+from appengine_wrappers import (
+    GetAppVersion, DeadlineExceededError, IsDevServer, logservice)
 from branch_utility import BranchUtility
 from caching_file_system import CachingFileSystem
+from empty_dir_file_system import EmptyDirFileSystem
 from github_file_system import GithubFileSystem
 from object_store_creator import ObjectStoreCreator
 from render_servlet import RenderServlet
@@ -18,20 +21,6 @@ from subversion_file_system import SubversionFileSystem
 import svn_constants
 from third_party.json_schema_compiler.memoize import memoize
 
-def _CreateServerInstanceForChannel(channel, delegate):
-  object_store_creator = ObjectStoreCreator(channel, start_empty=True)
-  branch = (delegate.CreateBranchUtility(object_store_creator)
-      .GetBranchForChannel(channel))
-  host_file_system = CachingFileSystem(
-      delegate.CreateHostFileSystemForBranch(branch),
-      object_store_creator)
-  app_samples_file_system = delegate.CreateAppSamplesFileSystem(
-      object_store_creator)
-  return ServerInstance(channel,
-                        object_store_creator,
-                        host_file_system,
-                        app_samples_file_system)
-
 class _SingletonRenderServletDelegate(RenderServlet.Delegate):
   def __init__(self, server_instance):
     self._server_instance = server_instance
@@ -44,16 +33,17 @@ 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):
-    '''Allow runtime dependencies to be overridden for testing.
+    '''CronServlet's runtime dependencies. Override for testing.
     '''
     def CreateBranchUtility(self, object_store_creator):
       return BranchUtility.Create(object_store_creator)
 
-    def CreateHostFileSystemForBranch(self, branch):
-      return SubversionFileSystem.Create(branch)
+    def CreateHostFileSystemForBranchAndRevision(self, branch, revision):
+      return SubversionFileSystem.Create(branch, revision=revision)
 
     def CreateAppSamplesFileSystem(self, object_store_creator):
       # TODO(kalman): CachingFileSystem wrapper for GithubFileSystem, but it's
@@ -61,6 +51,9 @@ class CronServlet(Servlet):
       return (EmptyDirFileSystem() if IsDevServer() else
               GithubFileSystem.Create(object_store_creator))
 
+    def GetAppVersion(self):
+      return GetAppVersion()
+
   def Get(self):
     # Crons often time out, and when they do *and* then eventually try to
     # flush logs they die. Turn off autoflush and manually do so at the end.
@@ -77,12 +70,12 @@ 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._request.path.strip('/')
+    channel = self._channel
     logging.info('cron/%s: starting' % channel)
 
     # This is returned every time RenderServlet wants to create a new
     # ServerInstance.
-    server_instance = _CreateServerInstanceForChannel(channel, self._delegate)
+    server_instance = self._GetSafeServerInstance()
 
     def get_via_render_servlet(path):
       return RenderServlet(
@@ -123,18 +116,23 @@ class CronServlet(Servlet):
     success = True
     try:
       # Render all of the publicly accessible files.
-      for path, path_prefix in (
-          # Note: rendering the public templates will pull in all of the private
-          # templates.
-          (svn_constants.PUBLIC_TEMPLATE_PATH, ''),
-          # Note: rendering the public templates will have pulled in the .js
-          # and manifest.json files (for listing examples on the API reference
-          # pages), but there are still images, CSS, etc.
-          (svn_constants.STATIC_PATH, 'static/'),
-          (svn_constants.EXAMPLES_PATH, 'extensions/examples/')):
-          # Note: don't try to short circuit any of this stuff. We want to run
-          # the cron for all the directories regardless of intermediate
-          # failures.
+      cron_runs = [
+        # Note: rendering the public templates will pull in all of the private
+        # templates.
+        (svn_constants.PUBLIC_TEMPLATE_PATH, ''),
+        # Note: rendering the public templates will have pulled in the .js
+        # and manifest.json files (for listing examples on the API reference
+        # pages), but there are still images, CSS, etc.
+        (svn_constants.STATIC_PATH, 'static/'),
+      ]
+      if not IsDevServer():
+        cron_runs.append(
+            (svn_constants.EXAMPLES_PATH, 'extensions/examples/'))
+
+      # Note: don't try to short circuit any of this stuff. We want to run
+      # the cron for all the directories regardless of intermediate
+      # failures.
+      for path, path_prefix in cron_runs:
         success = run_cron_for_dir(path, path_prefix=path_prefix) and success
 
       # TODO(kalman): Generic way for classes to request cron access. The next
@@ -143,22 +141,23 @@ class CronServlet(Servlet):
 
       # Extension examples have zip files too. Well, so do apps, but the app
       # file system doesn't get the Offline treatment so they don't need cron.
-      manifest_json = '/manifest.json'
-      example_zips = [
-          '%s.zip' % filename[:-len(manifest_json)]
-          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)))
-      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))
+      if not IsDevServer():
+        manifest_json = '/manifest.json'
+        example_zips = [
+            '%s.zip' % filename[:-len(manifest_json)]
+            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)))
+        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))
 
       # Also trigger a redirect so that PathCanonicalizer has an opportunity to
       # cache file listings.
@@ -172,3 +171,63 @@ class CronServlet(Servlet):
 
     return (Response.Ok('Success') if success else
             Response.InternalError('Failure'))
+
+  def _GetSafeServerInstance(self):
+    '''Returns a ServerInstance with a host file system at a safe revision,
+    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)
+
+    get_branch_for_channel = self._GetBranchForChannel
+    class AppYamlHelperDelegate(AppYamlHelper.Delegate):
+      def GetHostFileSystemForRevision(self, revision):
+        return delegate.CreateHostFileSystemForBranchAndRevision(
+            get_branch_for_channel(channel),
+            revision)
+
+    app_yaml_handler = AppYamlHelper(
+        svn_constants.APP_YAML_PATH,
+        server_instance_at_head.host_file_system,
+        AppYamlHelperDelegate(),
+        server_instance_at_head.object_store_creator)
+
+    if app_yaml_handler.IsUpToDate(delegate.GetAppVersion()):
+      # TODO(kalman): return a new ServerInstance at an explicit revision in
+      # case the HEAD version changes underneath us.
+      return server_instance_at_head
+
+    # The version in app.yaml is greater than the currently running app's.
+    # The safe version is the one before it changed.
+    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))
+
+    return self._CreateServerInstance(channel, 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)
+        .GetBranchForChannel(channel))
+
+  def _CreateServerInstance(self, channel, revision):
+    object_store_creator = self._CreateObjectStoreCreator(channel)
+    host_file_system = CachingFileSystem(
+        self._delegate.CreateHostFileSystemForBranchAndRevision(
+            self._GetBranchForChannel(channel),
+            revision),
+        object_store_creator)
+    app_samples_file_system = self._delegate.CreateAppSamplesFileSystem(
+        object_store_creator)
+    return ServerInstance(channel,
+                          object_store_creator,
+                          host_file_system,
+                          app_samples_file_system)