wireproto: disallow commands handlers for multiple transport versions
I think it will be more trouble than it is worth to code version 1
and version 2 command handlers to the same interface. It will feel
awkward to shoehorn functionality into e.g. the version 1 protocol
handler interface. This would likely constrain the ability for version
2 to evolve.
Previous commits introduced a clean separation between command handlers
for version 1 and version 2 transports. This commit reinforces that
separation by dropping support for having a single command handler
service both version 1 and version 2 transports.
Differential Revision: https://phab.mercurial-scm.org/D3208
--- a/mercurial/wireproto.py Mon Apr 09 11:57:12 2018 -0700
+++ b/mercurial/wireproto.py Mon Apr 09 11:33:38 2018 -0700
@@ -697,7 +697,6 @@
# Constants specifying which transports a wire protocol command should be
# available on. For use with @wireprotocommand.
-POLICY_ALL = 'all'
POLICY_V1_ONLY = 'v1-only'
POLICY_V2_ONLY = 'v2-only'
@@ -707,7 +706,7 @@
# For version 2 transports.
commandsv2 = commanddict()
-def wireprotocommand(name, args='', transportpolicy=POLICY_V1_ONLY,
+def wireprotocommand(name, args=None, transportpolicy=POLICY_V1_ONLY,
permission='push'):
"""Decorator to declare a wire protocol command.
@@ -729,17 +728,14 @@
because otherwise commands not declaring their permissions could modify
a repository that is supposed to be read-only.
"""
- if transportpolicy == POLICY_ALL:
- transports = set(wireprototypes.TRANSPORTS)
- transportversions = {1, 2}
- elif transportpolicy == POLICY_V1_ONLY:
+ if transportpolicy == POLICY_V1_ONLY:
transports = {k for k, v in wireprototypes.TRANSPORTS.items()
if v['version'] == 1}
- transportversions = {1}
+ transportversion = 1
elif transportpolicy == POLICY_V2_ONLY:
transports = {k for k, v in wireprototypes.TRANSPORTS.items()
if v['version'] == 2}
- transportversions = {2}
+ transportversion = 2
else:
raise error.ProgrammingError('invalid transport policy value: %s' %
transportpolicy)
@@ -755,33 +751,40 @@
'got %s; expected "push" or "pull"' %
permission)
- if 1 in transportversions and not isinstance(args, bytes):
- raise error.ProgrammingError('arguments for version 1 commands must '
- 'be declared as bytes')
+ if transportversion == 1:
+ if args is None:
+ args = ''
- if isinstance(args, bytes):
- dictargs = {arg: b'legacy' for arg in args.split()}
- elif isinstance(args, dict):
- dictargs = args
- else:
- raise ValueError('args must be bytes or a dict')
+ if not isinstance(args, bytes):
+ raise error.ProgrammingError('arguments for version 1 commands '
+ 'must be declared as bytes')
+ elif transportversion == 2:
+ if args is None:
+ args = {}
+
+ if not isinstance(args, dict):
+ raise error.ProgrammingError('arguments for version 2 commands '
+ 'must be declared as dicts')
def register(func):
- if 1 in transportversions:
+ if transportversion == 1:
if name in commands:
raise error.ProgrammingError('%s command already registered '
'for version 1' % name)
commands[name] = commandentry(func, args=args,
transports=transports,
permission=permission)
- if 2 in transportversions:
+ elif transportversion == 2:
if name in commandsv2:
raise error.ProgrammingError('%s command already registered '
'for version 2' % name)
- commandsv2[name] = commandentry(func, args=dictargs,
+ commandsv2[name] = commandentry(func, args=args,
transports=transports,
permission=permission)
+ else:
+ raise error.ProgrammingError('unhandled transport version: %d' %
+ transportversion)
return func
return register