Patchwork D2796: hgweb: always return iterable from @webcommand functions (API)

login
register
mail settings
Submitter phabricator
Date March 11, 2018, 5:24 a.m.
Message ID <differential-rev-PHID-DREV-m4mbozokezeyueniwyil-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/29292/
State Superseded
Headers show

Comments

phabricator - March 11, 2018, 5:24 a.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  We had to hack up this function to support our transition to the
  new response API. Now that we're done with the transition (!!),
  we can return to returning an iterator of content chunks from
  these functions.
  
  It is tempting to return a normal object and not a generator.
  However, as the keyword extension demonstrates, extensions may
  wish to wrap commands and have a try..finally block around
  execution. Since there is a generator producing content and
  that generator could be executing code, the try..finally needs
  to live for as long as the generator is running. That means we
  have to return a generator so wrappers can consume the generator
  inside a try..finally.
  
  .. api::
  
    hgweb @webcommand functions must use the new response object
    passed in via ``web.res`` to initiate sending of a response.
    The hgweb WSGI application will no longer start sending the
    response automatically.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2796

AFFECTED FILES
  hgext/highlight/__init__.py
  hgext/keyword.py
  mercurial/hgweb/hgweb_mod.py
  mercurial/hgweb/webcommands.py

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/mercurial/hgweb/webcommands.py b/mercurial/hgweb/webcommands.py
--- a/mercurial/hgweb/webcommands.py
+++ b/mercurial/hgweb/webcommands.py
@@ -57,12 +57,9 @@ 
     The functions should populate the ``rctx.res`` object with details
     about the HTTP response.
 
-    The function can return the ``requestcontext.res`` instance to signal
-    that it wants to use this object to generate the response. If an iterable
-    is returned, the ``wsgirequest`` instance will be used and the returned
-    content will constitute the response body. ``True`` can be returned to
-    indicate that the function already sent output and the caller doesn't
-    need to do anything more to send the response.
+    The function returns a generator to be consumed by the WSGI application.
+    For most commands, this should be the result from
+    ``web.res.sendresponse()``.
 
     Usage:
 
@@ -135,7 +132,7 @@ 
                 .replace('\\', '\\\\').replace('"', '\\"'))
     web.res.headers['Content-Disposition'] = 'inline; filename="%s"' % filename
     web.res.setbodybytes(text)
-    return web.res
+    return web.res.sendresponse()
 
 def _filerevision(web, req, tmpl, fctx):
     f = fctx.path()
@@ -165,7 +162,7 @@ 
         ishead=int(ishead),
         **pycompat.strkwargs(webutil.commonentry(web.repo, fctx))))
 
-    return web.res
+    return web.res.sendresponse()
 
 @webcommand('file')
 def file(web, req, tmpl):
@@ -355,7 +352,7 @@ 
         showforcekw=showforcekw,
         showunforcekw=showunforcekw))
 
-    return web.res
+    return web.res.sendresponse()
 
 @webcommand('changelog')
 def changelog(web, req, tmpl, shortlog=False):
@@ -455,7 +452,7 @@ 
         lessvars=lessvars,
         query=query))
 
-    return web.res
+    return web.res.sendresponse()
 
 @webcommand('shortlog')
 def shortlog(web, req, tmpl):
@@ -490,7 +487,7 @@ 
     ctx = webutil.changectx(web.repo, req)
     web.res.setbodygen(tmpl('changeset',
                             **webutil.changesetentry(web, req, tmpl, ctx)))
-    return web.res
+    return web.res.sendresponse()
 
 rev = webcommand('rev')(changeset)
 
@@ -602,7 +599,7 @@ 
         archives=web.archivelist(hex(node)),
         **pycompat.strkwargs(webutil.commonentry(web.repo, ctx))))
 
-    return web.res
+    return web.res.sendresponse()
 
 @webcommand('tags')
 def tags(web, req, tmpl):
@@ -638,7 +635,7 @@ 
         entriesnotip=lambda **x: entries(True, False, **x),
         latestentry=lambda **x: entries(True, True, **x)))
 
-    return web.res
+    return web.res.sendresponse()
 
 @webcommand('bookmarks')
 def bookmarks(web, req, tmpl):
@@ -679,7 +676,7 @@ 
         entries=lambda **x: entries(latestonly=False, **x),
         latestentry=lambda **x: entries(latestonly=True, **x)))
 
-    return web.res
+    return web.res.sendresponse()
 
 @webcommand('branches')
 def branches(web, req, tmpl):
@@ -704,7 +701,7 @@ 
         entries=entries,
         latestentry=latestentry))
 
-    return web.res
+    return web.res.sendresponse()
 
 @webcommand('summary')
 def summary(web, req, tmpl):
@@ -789,7 +786,7 @@ 
         archives=web.archivelist('tip'),
         labels=web.configlist('web', 'labels')))
 
-    return web.res
+    return web.res.sendresponse()
 
 @webcommand('filediff')
 def filediff(web, req, tmpl):
@@ -838,7 +835,7 @@ 
         diff=diffs,
         **pycompat.strkwargs(webutil.commonentry(web.repo, ctx))))
 
-    return web.res
+    return web.res.sendresponse()
 
 diff = webcommand('diff')(filediff)
 
@@ -917,7 +914,7 @@ 
         comparison=comparison,
         **pycompat.strkwargs(webutil.commonentry(web.repo, ctx))))
 
-    return web.res
+    return web.res.sendresponse()
 
 @webcommand('annotate')
 def annotate(web, req, tmpl):
@@ -1011,7 +1008,7 @@ 
         diffopts=diffopts,
         **pycompat.strkwargs(webutil.commonentry(web.repo, fctx))))
 
-    return web.res
+    return web.res.sendresponse()
 
 @webcommand('filelog')
 def filelog(web, req, tmpl):
@@ -1151,7 +1148,7 @@ 
         lessvars=lessvars,
         **pycompat.strkwargs(webutil.commonentry(web.repo, fctx))))
 
-    return web.res
+    return web.res.sendresponse()
 
 @webcommand('archive')
 def archive(web, req, tmpl):
@@ -1225,7 +1222,7 @@ 
                      matchfn=match,
                      subrepos=web.configbool("web", "archivesubrepos"))
 
-    return True
+    return []
 
 @webcommand('static')
 def static(web, req, tmpl):
@@ -1240,7 +1237,7 @@ 
         static = [os.path.join(p, 'static') for p in tp]
 
     staticfile(static, fname, web.res)
-    return web.res
+    return web.res.sendresponse()
 
 @webcommand('graph')
 def graph(web, req, tmpl):
@@ -1401,7 +1398,7 @@ 
         node=ctx.hex(),
         changenav=changenav))
 
-    return web.res
+    return web.res.sendresponse()
 
 def _getdoc(e):
     doc = e[0].__doc__
@@ -1463,7 +1460,7 @@ 
             earlycommands=earlycommands,
             othercommands=othercommands,
             title='Index'))
-        return web.res
+        return web.res.sendresponse()
 
     # Render an index of sub-topics.
     if topicname in helpmod.subtopics:
@@ -1480,7 +1477,7 @@ 
             topics=topics,
             title=topicname,
             subindex=True))
-        return web.res
+        return web.res.sendresponse()
 
     u = webutil.wsgiui.load()
     u.verbose = True
@@ -1506,7 +1503,7 @@ 
         topic=topicname,
         doc=doc))
 
-    return web.res
+    return web.res.sendresponse()
 
 # tell hggettext to extract docstrings from these functions:
 i18nfunctions = commands.values()
diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
--- a/mercurial/hgweb/hgweb_mod.py
+++ b/mercurial/hgweb/hgweb_mod.py
@@ -14,7 +14,6 @@ 
 from .common import (
     ErrorResponse,
     HTTP_BAD_REQUEST,
-    HTTP_OK,
     cspvalues,
     permhooks,
     statusmessage,
@@ -405,15 +404,7 @@ 
                 # override easily enough.
                 res.status = '200 Script output follows'
                 res.headers['Content-Type'] = ctype
-                content = getattr(webcommands, cmd)(rctx, wsgireq, tmpl)
-
-                if content is res:
-                    return res.sendresponse()
-                elif content is True:
-                    return []
-                else:
-                    wsgireq.respond(HTTP_OK, ctype)
-                    return content
+                return getattr(webcommands, cmd)(rctx, wsgireq, tmpl)
 
         except (error.LookupError, error.RepoLookupError) as err:
             msg = pycompat.bytestr(err)
diff --git a/hgext/keyword.py b/hgext/keyword.py
--- a/hgext/keyword.py
+++ b/hgext/keyword.py
@@ -621,13 +621,7 @@ 
         origmatch = kwt.match
         kwt.match = util.never
     try:
-        res = orig(web, req, tmpl)
-        if res is web.res:
-            res = res.sendresponse()
-        elif res is True:
-            return
-
-        for chunk in res:
+        for chunk in orig(web, req, tmpl):
             yield chunk
     finally:
         if kwt:
diff --git a/hgext/highlight/__init__.py b/hgext/highlight/__init__.py
--- a/hgext/highlight/__init__.py
+++ b/hgext/highlight/__init__.py
@@ -88,7 +88,7 @@ 
         '/* pygments_style = %s */\n\n' % pg_style,
         fmter.get_style_defs(''),
     ]))
-    return web.res
+    return web.res.sendresponse()
 
 def extsetup():
     # monkeypatch in the new version