Docserver: Add support for viewing docs with a codereview patch applied

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

Index: chrome/common/extensions/docs/server2/patch_servlet.py
===================================================================
--- chrome/common/extensions/docs/server2/patch_servlet.py	(revision 0)
+++ chrome/common/extensions/docs/server2/patch_servlet.py	(revision 0)
@@ -0,0 +1,38 @@
+# 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.
+
+from object_store_creator import ObjectStoreCreator
+from render_servlet import _ALWAYS_ONLINE, RenderServlet
+from server_instance import ServerInstance
+from servlet import Request, Response, Servlet
+
+class PatchServlet(Servlet):
+  '''Servlet which renders patched docs.
+  '''
+
+  def Get(self):
+    path_with_issue = self._request.path.lstrip('/')

[not at google - send to devlin, 2013/05/03 19:12:22]

'/' is already been lstripped in Handler.

[方觉(Fang Jue), 2013/05/04 02:05:41]

Done.

+    if '/' in path_with_issue:
+      issue, real_path = path_with_issue.split('/', 1)
+    else:
+      issue = path_with_issue
+      real_path = ''

[not at google - send to devlin, 2013/05/03 19:12:22]

I like the pattern of doing "issue, real_path = (path_with_issue, '')" here,
it's a nice parallel to line 17.

[方觉(Fang Jue), 2013/05/04 02:05:41]

Done.

[方觉(Fang Jue), 2013/05/04 02:05:41]

Done.

+    if not issue.isdigit():
+      return Response.Redirect('/' + path_with_issue)

[not at google - send to devlin, 2013/05/03 19:12:22]

Why does the issue need to be a number? We show a 404 when the issue isn't found
right? In which case, this can follow the same eventual path.

[方觉(Fang Jue), 2013/05/04 02:05:41]

Done.

+    elif len(real_path) == 0:

[not at google - send to devlin, 2013/05/03 19:12:22]

these can be if not elif since the preceding statements are returns.

That said, I would hope that they're not necessary, the redirects are an
implementation detail of RenderServlet.Get passing it the real path. If we
inspect the result of RenderServlet and see that it's a redirect then prepend
the _patch/%s stuff to the path and redirect to that (i.e. update the Location
header). Makes sense to add an IsRedirect method to Response.

[方觉(Fang Jue), 2013/05/04 02:05:41]

Done.
When I tested it, I found a bug in RenderServlet.Get. It doesn't correctly
handle paths like 'trunk/extension' (without trailing slash).

+      return Response.Redirect('/_patch/%s/extensions/index.html' % issue)
+    elif real_path in ['apps', 'apps/']:
+      return Response.Redirect('/_patch/%s/apps/about_apps.html' % issue)
+    elif real_path in ['extensions', 'extensions/']:
+      return Response.Redirect('/_patch/%s/extensions/index.html' % issue)
+
+    constructor = (ServerInstance.CreateOnline if _ALWAYS_ONLINE else

[not at google - send to devlin, 2013/05/03 19:12:22]

ALWAYS_ONLINE is about to be deleted, use IsDevServer directly.

[方觉(Fang Jue), 2013/05/04 02:05:41]

Done.

+                   ServerInstance.GetOrCreateOffline)
+    server_instance = constructor('trunk',
+                                  '/_patch/%s/' % issue,
+                                  issue)
+    fake_path = '/trunk/%s' % real_path
+
+    return RenderServlet(Request(fake_path,
+                                 self._request.headers)).Get(server_instance)
Property changes on: chrome/common/extensions/docs/server2/patch_servlet.py
___________________________________________________________________
Added: svn:eol-style
   + LF