This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3202

wireproto: define and expose types of wire command arguments
ClosedPublic

Authored by indygreg on Apr 9 2018, 2:58 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG69e46c1834ac: wireproto: define and expose types of wire command arguments
Summary

Exposing the set of argument names is cool. But with wire protocol
version 2, we're using CBOR to transport arguments and this allows us
to have typing for arguments.

Typed arguments are much nicer because they will cut down on transfer
overhead and processing overhead for decoding values.

This commit teaches @wireprotocommand to accept a dictionary for
arguments. The arguments registered for version 2 transports are
canonically stored as dictionaries rather than a space-delimited string.

It is an error to defined arguments with a dictionary for commands using
version 1 transports. This reinforces my intent to fully decouple command
handlers for version 2 transports.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Apr 9 2018, 2:58 PM
durin42 accepted this revision.Apr 9 2018, 6:28 PM
This revision is now accepted and ready to land.Apr 9 2018, 6:28 PM
Closed by commit rHG69e46c1834ac: wireproto: define and expose types of wire command arguments (authored by indygreg). · Explain WhyApr 11 2018, 12:34 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 7906

mercurial/help/internals/wireprotocol.txt

commands commands
A map defining available wire protocol commands on this server. A map defining available wire protocol commands on this server.
Keys in the map are the names of commands that can be invoked. Values Keys in the map are the names of commands that can be invoked. Values
are maps defining information about that command. The bytestring keys are maps defining information about that command. The bytestring keys
are: are:
args args
An array of argument names accepted by this command. A map of argument names and their expected types.
Types are defined as a representative value for the expected type.
e.g. an argument expecting a boolean type will have its value
set to true. An integer type will have its value set to 42. The
actual values are arbitrary and may not have meaning.
permissions permissions
An array of permissions required to execute this command. An array of permissions required to execute this command.
compression compression
An array of maps defining available compression format support. An array of maps defining available compression format support.
The array is sorted from most preferred to least preferred. The array is sorted from most preferred to least preferred.

mercurial/wireproto.py

commandsv2 = commanddict() commandsv2 = commanddict()
def wireprotocommand(name, args='', transportpolicy=POLICY_ALL, def wireprotocommand(name, args='', transportpolicy=POLICY_ALL,
permission='push'): permission='push'):
"""Decorator to declare a wire protocol command. """Decorator to declare a wire protocol command.
``name`` is the name of the wire protocol command being provided. ``name`` is the name of the wire protocol command being provided.
``args`` is a space-delimited list of named arguments that the command ``args`` defines the named arguments accepted by the command. It is
accepts. ``*`` is a special value that says to accept all arguments. ideally a dict mapping argument names to their types. For backwards
compatibility, it can be a space-delimited list of argument names. For
version 1 transports, ``*`` denotes a special value that says to accept
all named arguments.
``transportpolicy`` is a POLICY_* constant denoting which transports ``transportpolicy`` is a POLICY_* constant denoting which transports
this wire protocol command should be exposed to. By default, commands this wire protocol command should be exposed to. By default, commands
are exposed to all wire protocol transports. are exposed to all wire protocol transports.
``permission`` defines the permission type needed to run this command. ``permission`` defines the permission type needed to run this command.
Can be ``push`` or ``pull``. These roughly map to read-write and read-only, Can be ``push`` or ``pull``. These roughly map to read-write and read-only,
respectively. Default is to assume command requires ``push`` permissions respectively. Default is to assume command requires ``push`` permissions
if name == b'batch': if name == b'batch':
transports.add(wireprototypes.SSHV2) transports.add(wireprototypes.SSHV2)
if permission not in ('push', 'pull'): if permission not in ('push', 'pull'):
raise error.ProgrammingError('invalid wire protocol permission; ' raise error.ProgrammingError('invalid wire protocol permission; '
'got %s; expected "push" or "pull"' % 'got %s; expected "push" or "pull"' %
permission) permission)
if 1 in transportversions and not isinstance(args, bytes):
raise error.ProgrammingError('arguments for version 1 commands must '
'be declared as bytes')
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')
def register(func): def register(func):
if 1 in transportversions: if 1 in transportversions:
if name in commands: if name in commands:
raise error.ProgrammingError('%s command already registered ' raise error.ProgrammingError('%s command already registered '
'for version 1' % name) 'for version 1' % name)
commands[name] = commandentry(func, args=args, commands[name] = commandentry(func, args=args,
transports=transports, transports=transports,
permission=permission) permission=permission)
if 2 in transportversions: if 2 in transportversions:
if name in commandsv2: if name in commandsv2:
raise error.ProgrammingError('%s command already registered ' raise error.ProgrammingError('%s command already registered '
'for version 2' % name) 'for version 2' % name)
commandsv2[name] = commandentry(func, args=args,
commandsv2[name] = commandentry(func, args=dictargs,
transports=transports, transports=transports,
permission=permission) permission=permission)
return func return func
return register return register
# TODO define a more appropriate permissions type to use for this. # TODO define a more appropriate permissions type to use for this.
@wireprotocommand('batch', 'cmds *', permission='pull', @wireprotocommand('batch', 'cmds *', permission='pull',
caps = { caps = {
'commands': {}, 'commands': {},
'compression': compression, 'compression': compression,
} }
for command, entry in commandsv2.items(): for command, entry in commandsv2.items():
caps['commands'][command] = { caps['commands'][command] = {
'args': sorted(entry.args.split()) if entry.args else [], 'args': entry.args,
'permissions': [entry.permission], 'permissions': [entry.permission],
} }
return proto.addcapabilities(repo, caps) return proto.addcapabilities(repo, caps)
@wireprotocommand('branchmap', permission='pull', @wireprotocommand('branchmap', permission='pull',
transportpolicy=POLICY_V2_ONLY) transportpolicy=POLICY_V2_ONLY)
def branchmapv2(repo, proto): def branchmapv2(repo, proto):
branchmap = {encoding.fromlocal(k): v branchmap = {encoding.fromlocal(k): v
for k, v in repo.branchmap().iteritems()} for k, v in repo.branchmap().iteritems()}
return wireprototypes.cborresponse(branchmap) return wireprototypes.cborresponse(branchmap)
@wireprotocommand('capabilities', permission='pull', @wireprotocommand('capabilities', permission='pull',
transportpolicy=POLICY_V2_ONLY) transportpolicy=POLICY_V2_ONLY)
def capabilitiesv2(repo, proto): def capabilitiesv2(repo, proto):
caps = _capabilitiesv2(repo, proto) caps = _capabilitiesv2(repo, proto)
return wireprototypes.cborresponse(caps) return wireprototypes.cborresponse(caps)
@wireprotocommand('heads', args='publiconly', permission='pull', @wireprotocommand('heads',
args={
'publiconly': False,
},
permission='pull',
transportpolicy=POLICY_V2_ONLY) transportpolicy=POLICY_V2_ONLY)
def headsv2(repo, proto, publiconly=False): def headsv2(repo, proto, publiconly=False):
if publiconly: if publiconly:
repo = repo.filtered('immutable') repo = repo.filtered('immutable')
return wireprototypes.cborresponse(repo.heads()) return wireprototypes.cborresponse(repo.heads())
@wireprotocommand('known', 'nodes', permission='pull', @wireprotocommand('known',
args={
'nodes': [b'deadbeef'],
},
permission='pull',
transportpolicy=POLICY_V2_ONLY) transportpolicy=POLICY_V2_ONLY)
def knownv2(repo, proto, nodes=None): def knownv2(repo, proto, nodes=None):
nodes = nodes or [] nodes = nodes or []
result = b''.join(b'1' if n else b'0' for n in repo.known(nodes)) result = b''.join(b'1' if n else b'0' for n in repo.known(nodes))
return wireprototypes.cborresponse(result) return wireprototypes.cborresponse(result)
@wireprotocommand('listkeys', 'namespace', permission='pull', @wireprotocommand('listkeys',
args={
'namespace': b'ns',
},
permission='pull',
transportpolicy=POLICY_V2_ONLY) transportpolicy=POLICY_V2_ONLY)
def listkeysv2(repo, proto, namespace=None): def listkeysv2(repo, proto, namespace=None):
keys = repo.listkeys(encoding.tolocal(namespace)) keys = repo.listkeys(encoding.tolocal(namespace))
keys = {encoding.fromlocal(k): encoding.fromlocal(v) keys = {encoding.fromlocal(k): encoding.fromlocal(v)
for k, v in keys.iteritems()} for k, v in keys.iteritems()}
return wireprototypes.cborresponse(keys) return wireprototypes.cborresponse(keys)

mercurial/wireprotoserver.py

self._args = args self._args = args
@property @property
def name(self): def name(self):
return HTTPV2 return HTTPV2
def getargs(self, args): def getargs(self, args):
data = {} data = {}
for k in args.split(): for k, typ in args.items():
if k == '*': if k == '*':
raise NotImplementedError('do not support * args') raise NotImplementedError('do not support * args')
elif k in self._args: elif k in self._args:
# TODO consider validating value types.
data[k] = self._args[k] data[k] = self._args[k]
return data return data
def getprotocaps(self): def getprotocaps(self):
# Protocol capabilities are currently not implemented for HTTP V2. # Protocol capabilities are currently not implemented for HTTP V2.
return set() return set()

tests/test-wireproto-command-capabilities.t

s> HTTP/1.1 200 OK\r\n s> HTTP/1.1 200 OK\r\n
s> Server: testing stub value\r\n s> Server: testing stub value\r\n
s> Date: $HTTP_DATE$\r\n s> Date: $HTTP_DATE$\r\n
s> Content-Type: application/mercurial-exp-framing-0003\r\n s> Content-Type: application/mercurial-exp-framing-0003\r\n
s> Transfer-Encoding: chunked\r\n s> Transfer-Encoding: chunked\r\n
s> \r\n s> \r\n
s> *\r\n (glob) s> *\r\n (glob)
s> *\x00\x01\x00\x02\x01F (glob) s> *\x00\x01\x00\x02\x01F (glob)
s> \xa2Hcommands\xaaEheads\xa2Dargs\x81JpubliconlyKpermissions\x81DpullEknown\xa2Dargs\x81EnodesKpermissions\x81DpullFlookup\xa2Dargs\x81CkeyKpermissions\x81DpullGpushkey\xa2Dargs\x84CkeyInamespaceCnewColdKpermissions\x81DpushHlistkeys\xa2Dargs\x81InamespaceKpermissions\x81DpullHunbundle\xa2Dargs\x81EheadsKpermissions\x81DpushIbranchmap\xa2Dargs\x80Kpermissions\x81DpullIgetbundle\xa2Dargs\x81A*Kpermissions\x81DpullLcapabilities\xa2Dargs\x80Kpermissions\x81DpullLclonebundles\xa2Dargs\x80Kpermissions\x81DpullKcompression\x82\xa1DnameDzstd\xa1DnameDzlib s> \xa2Hcommands\xaaEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyFlegacyKpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyFlegacyCnewFlegacyColdFlegacyInamespaceFlegacyKpermissions\x81DpushHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullHunbundle\xa2Dargs\xa1EheadsFlegacyKpermissions\x81DpushIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullIgetbundle\xa2Dargs\xa1A*FlegacyKpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullLclonebundles\xa2Dargs\xa0Kpermissions\x81DpullKcompression\x82\xa1DnameDzstd\xa1DnameDzlib
s> \r\n s> \r\n
received frame(size=*; request=1; stream=2; streamflags=stream-begin; type=bytes-response; flags=eos|cbor) (glob) received frame(size=*; request=1; stream=2; streamflags=stream-begin; type=bytes-response; flags=eos|cbor) (glob)
s> 0\r\n s> 0\r\n
s> \r\n s> \r\n
response: [{b'commands': {b'branchmap': {b'args': [], b'permissions': [b'pull']}, b'capabilities': {b'args': [], b'permissions': [b'pull']}, b'clonebundles': {b'args': [], b'permissions': [b'pull']}, b'getbundle': {b'args': [b'*'], b'permissions': [b'pull']}, b'heads': {b'args': [b'publiconly'], b'permissions': [b'pull']}, b'known': {b'args': [b'nodes'], b'permissions': [b'pull']}, b'listkeys': {b'args': [b'namespace'], b'permissions': [b'pull']}, b'lookup': {b'args': [b'key'], b'permissions': [b'pull']}, b'pushkey': {b'args': [b'key', b'namespace', b'new', b'old'], b'permissions': [b'push']}, b'unbundle': {b'args': [b'heads'], b'permissions': [b'push']}}, b'compression': [{b'name': b'zstd'}, {b'name': b'zlib'}]}] response: [{b'commands': {b'branchmap': {b'args': {}, b'permissions': [b'pull']}, b'capabilities': {b'args': {}, b'permissions': [b'pull']}, b'clonebundles': {b'args': {}, b'permissions': [b'pull']}, b'getbundle': {b'args': {b'*': b'legacy'}, b'permissions': [b'pull']}, b'heads': {b'args': {b'publiconly': False}, b'permissions': [b'pull']}, b'known': {b'args': {b'nodes': [b'deadbeef']}, b'permissions': [b'pull']}, b'listkeys': {b'args': {b'namespace': b'ns'}, b'permissions': [b'pull']}, b'lookup': {b'args': {b'key': b'legacy'}, b'permissions': [b'pull']}, b'pushkey': {b'args': {b'key': b'legacy', b'namespace': b'legacy', b'new': b'legacy', b'old': b'legacy'}, b'permissions': [b'push']}, b'unbundle': {b'args': {b'heads': b'legacy'}, b'permissions': [b'push']}}, b'compression': [{b'name': b'zstd'}, {b'name': b'zlib'}]}]
$ cat error.log $ cat error.log