Patchwork [5,of,5] hgweb: centralize registration of web commands into webcommands.commands (API)

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Aug. 22, 2018, 2:21 a.m.
Message ID <ae3877647d3c07748d96.1534904516@blacknile>
Download mbox | patch
Permalink /patch/33957/
State New
Headers show

Comments

Katsunori FUJIWARA - Aug. 22, 2018, 2:21 a.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1534896968 -32400
#      Wed Aug 22 09:16:08 2018 +0900
# Node ID ae3877647d3c07748d96d8bc2f74d96e7d9fbc9c
# Parent  afe84a051a391724a7d11f07513cf69f2e46b563
# Available At https://bitbucket.org/foozy/mercurial-wip
#              hg pull https://bitbucket.org/foozy/mercurial-wip -r ae3877647d3c
# EXP-Topic use-decorator-for-webcommand
hgweb: centralize registration of web commands into webcommands.commands (API)

Valid web commands are managed in multiple ways below:

  - setting web command function as an attribute of webcommands module:
    - guess default command
    - get the function corresponded to requested command
  - webcommands.__all__ list:
    - examine validity of requested command
  - webcommands.commands dict:
    - get the list of web command functions for online help

This should be centralized to avoid complication and redundancy, and
webcommands.commands dict seems reasonable for such purpose, because:

  - using attributes of webcommands module requires some kind of
    black/white list mechanism to avoid unintentional execution of
    internal function

    Before this patch, webcommands.__all__ works as "a white list". If
    it is disused, some additional examinations are needed for safety:
    e.g.  checking "callable", checking "_" prefix in name, and so on.

  - using webcommands.__all__ is inefficient, because list costs O(N)

    In addition to it, there is no need to use Python specific __all__
    for this purpose, because there is no code path using "import *"
    (at least in Mercurial source tree)

This patch centralizes registration of web commands into
webcommands.commands, in order to validate requested command and to
get the function corresponded to it.

For consistency, this patch does below at once.

  - use registrar.webcommand to register web command function
  - replace extensions.wrapfunction() by:
    - registrar.wrapfunc(), for original (web command) function
    - registrar.wrapwebcommand(), for web command in dispatach table

This patch also avoids adding web command name to webcommands.__all__
for efficiency, because the shorter it is, the smaller cost to examine
existence is.

This patch still keeps wrapping original web command functions,
because other code paths might directly invoke wrapped function via
other than webcommand dispatch table.

After this patch, old-style web command registration causes
deprecwarn(). This is reason why this patch is marked as API.

  .. api:

     web command function should be registered by registrar.webcommand

Patch

diff --git a/hgext/highlight/__init__.py b/hgext/highlight/__init__.py
--- a/hgext/highlight/__init__.py
+++ b/hgext/highlight/__init__.py
@@ -35,7 +35,7 @@  from mercurial.hgweb import (
 )
 
 from mercurial import (
-    extensions,
+    registrar,
 )
 
 # Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' for
@@ -44,6 +44,12 @@  from mercurial import (
 # leave the attribute unspecified.
 testedwith = 'ships-with-hg-core'
 
+webcommand = registrar.webcommand()
+
+hgwrapperlist = []
+wrapfunc = registrar.wrapfunc(hgwrapperlist)
+wrapwebcommand = registrar.wrapwebcommand(hgwrapperlist)
+
 def pygmentize(web, field, fctx, tmpl):
     style = web.config('web', 'pygments_style', 'colorful')
     expr = web.config('web', 'highlightfiles', "size('<5M')")
@@ -55,6 +61,7 @@  def pygmentize(web, field, fctx, tmpl):
         highlight.pygmentize(field, fctx, style, tmpl,
                 guessfilenameonly=filenameonly)
 
+@wrapfunc('_filerevision', webcommands)
 def filerevision_highlight(orig, web, fctx):
     mt = web.res.headers['Content-Type']
     # only pygmentize for mimetype containing 'html' so we both match
@@ -69,6 +76,8 @@  def filerevision_highlight(orig, web, fc
 
     return orig(web, fctx)
 
+@wrapwebcommand('annotate')
+@wrapfunc('annotate', webcommands)
 def annotate_highlight(orig, web):
     mt = web.res.headers['Content-Type']
     if 'html' in mt:
@@ -77,6 +86,7 @@  def annotate_highlight(orig, web):
 
     return orig(web)
 
+@webcommand('highlightcss')
 def generate_css(web):
     pg_style = web.config('web', 'pygments_style', 'colorful')
     fmter = highlight.HtmlFormatter(style=pg_style)
@@ -86,11 +96,3 @@  def generate_css(web):
         fmter.get_style_defs(''),
     ]))
     return web.res.sendresponse()
-
-def extsetup():
-    # monkeypatch in the new version
-    extensions.wrapfunction(webcommands, '_filerevision',
-                            filerevision_highlight)
-    extensions.wrapfunction(webcommands, 'annotate', annotate_highlight)
-    webcommands.highlightcss = generate_css
-    webcommands.__all__.append('highlightcss')
diff --git a/hgext/keyword.py b/hgext/keyword.py
--- a/hgext/keyword.py
+++ b/hgext/keyword.py
@@ -155,6 +155,11 @@  configitem = registrar.configitem(config
 configitem('keywordset', 'svn',
     default=False,
 )
+
+hgwrapperlist = []
+wrapfunc = registrar.wrapfunc(hgwrapperlist)
+wrapwebcommand = registrar.wrapwebcommand(hgwrapperlist)
+
 # date like in cvs' $Date
 @templatefilter('utcdate', intype=templateutil.date)
 def utcdate(date):
@@ -740,7 +745,8 @@  def uisetup(ui):
     extensions.wrapfunction(cmdutil, 'copy', kw_copy)
     extensions.wrapfunction(cmdutil, 'dorecord', kw_dorecord)
     for c in nokwwebcommands.split():
-        extensions.wrapfunction(webcommands, c, kwweb_skip)
+        wrapfunc(c, webcommands)(kwweb_skip)
+        wrapwebcommand(c)(kwweb_skip)
 
 def reposetup(ui, repo):
     '''Sets up repo as kwrepo for keyword substitution.'''
diff --git a/hgext/largefiles/__init__.py b/hgext/largefiles/__init__.py
--- a/hgext/largefiles/__init__.py
+++ b/hgext/largefiles/__init__.py
@@ -152,3 +152,4 @@  def uisetup(ui):
 
 cmdtable = lfcommands.cmdtable
 revsetpredicate = overrides.revsetpredicate
+hgwrapperlist = overrides.hgwrapperlist
diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -29,12 +29,20 @@  from mercurial import (
     util,
 )
 
+from mercurial.hgweb import (
+    webcommands,
+)
+
 from . import (
     lfcommands,
     lfutil,
     storefactory,
 )
 
+hgwrapperlist = []
+wrapfunc = registrar.wrapfunc(hgwrapperlist)
+wrapwebcommand = registrar.wrapwebcommand(hgwrapperlist)
+
 # -- Utility functions: commonly/repeatedly needed functionality ---------------
 
 def composelargefilematcher(match, manifest):
@@ -248,6 +256,7 @@  def removelargefiles(ui, repo, isaddremo
 
 # For overriding mercurial.hgweb.webcommands so that largefiles will
 # appear at their right place in the manifests.
+@wrapfunc('decodepath', webcommands)
 def decodepath(orig, path):
     return lfutil.splitstandin(path) or path
 
@@ -934,6 +943,8 @@  def overridearchivecmd(orig, ui, repo, d
     finally:
         repo.unfiltered().lfstatus = False
 
+@wrapwebcommand('archive')
+@wrapfunc('archive', webcommands)
 def hgwebarchive(orig, web):
     web.repo.lfstatus = True
 
diff --git a/hgext/largefiles/uisetup.py b/hgext/largefiles/uisetup.py
--- a/hgext/largefiles/uisetup.py
+++ b/hgext/largefiles/uisetup.py
@@ -11,10 +11,6 @@  from __future__ import absolute_import
 
 from mercurial.i18n import _
 
-from mercurial.hgweb import (
-    webcommands,
-)
-
 from mercurial import (
     archival,
     cmdutil,
@@ -151,7 +147,6 @@  def uisetup(ui):
     extensions.wrapfunction(archival, 'archive', overrides.overridearchive)
     extensions.wrapfunction(subrepo.hgsubrepo, 'archive',
                             overrides.hgsubrepoarchive)
-    extensions.wrapfunction(webcommands, 'archive', overrides.hgwebarchive)
     extensions.wrapfunction(cmdutil, 'bailifchanged',
                             overrides.overridebailifchanged)
 
@@ -178,8 +173,6 @@  def uisetup(ui):
                             proto.heads)
     # TODO also wrap wireproto.commandsv2 once heads is implemented there.
 
-    extensions.wrapfunction(webcommands, 'decodepath', overrides.decodepath)
-
     extensions.wrapfunction(wireprotov1server, '_capabilities',
                             proto._capabilities)
 
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
@@ -195,6 +195,22 @@  class requestcontext(object):
         self.res.setbodygen(self.tmpl.generate(name, kwargs))
         return self.res.sendresponse()
 
+def _getwebcommand(ui, name):
+    cmd = webcommands.commands.get(name, None)
+    if not cmd:
+        if (name in webcommands.__all__ and
+            util.safehasattr(webcommands, name)):
+            cmd = getattr(webcommands, name)
+            # store into webcommands.commands for subsequent access
+            webcommands.commands[name] = cmd
+
+            # after deprecation limit, meaningless webcommands.__all__
+            # should be also removed
+            ui.deprecwarn('legacy registration for webcommand %s,'
+                          ' use registrar.webcommand decorator' % name,
+                          '5.0')
+    return cmd
+
 class hgweb(object):
     """HTTP server for individual repositories.
 
@@ -354,7 +370,7 @@  class hgweb(object):
                 cmd = cmd[style + 1:]
 
             # avoid accepting e.g. style parameter as command
-            if util.safehasattr(webcommands, cmd):
+            if _getwebcommand(repo.ui, cmd):
                 req.qsparams['cmd'] = cmd
 
             if cmd == 'static':
@@ -416,7 +432,7 @@  class hgweb(object):
 
                 res.headers['ETag'] = tag
 
-            if cmd not in webcommands.__all__:
+            if not _getwebcommand(repo.ui, cmd):
                 msg = 'no such method: %s' % cmd
                 raise ErrorResponse(HTTP_BAD_REQUEST, msg)
             else:
@@ -424,7 +440,7 @@  class hgweb(object):
                 # override easily enough.
                 res.status = '200 Script output follows'
                 res.headers['Content-Type'] = ctype
-                return getattr(webcommands, cmd)(rctx)
+                return _getwebcommand(repo.ui, cmd)(rctx)
 
         except (error.LookupError, error.RepoLookupError) as err:
             msg = pycompat.bytestr(err)
diff --git a/mercurial/hgweb/webcommands.py b/mercurial/hgweb/webcommands.py
--- a/mercurial/hgweb/webcommands.py
+++ b/mercurial/hgweb/webcommands.py
@@ -77,7 +77,6 @@  class webcommand(object):
         self.name = name
 
     def __call__(self, func):
-        __all__.append(self.name)
         commands[self.name] = func
         return func
 
diff --git a/tests/hgweberror.py b/tests/hgweberror.py
--- a/tests/hgweberror.py
+++ b/tests/hgweberror.py
@@ -2,10 +2,13 @@ 
 
 from __future__ import absolute_import
 
-from mercurial.hgweb import (
-    webcommands,
+from mercurial import (
+    registrar,
 )
 
+webcommand = registrar.webcommand()
+
+@webcommand('raiseerror')
 def raiseerror(web):
     '''Dummy web command that raises an uncaught Exception.'''
 
@@ -18,7 +21,3 @@  def raiseerror(web):
         web.res.getbodyfile().write(b'partial content\n')
 
     raise AttributeError('I am an uncaught error!')
-
-def extsetup(ui):
-    setattr(webcommands, 'raiseerror', raiseerror)
-    webcommands.__all__.append('raiseerror')