D4615: wireprotov2: declare command arguments richly

indygreg (Gregory Szorc) phabricator at mercurial-scm.org
Thu Sep 20 17:15:15 EDT 2018


indygreg updated this revision to Diff 11243.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D4615?vs=11114&id=11243

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

AFFECTED FILES
  mercurial/wireprotov2server.py
  tests/test-http-protocol.t
  tests/test-wireproto-command-capabilities.t
  tests/test-wireproto-command-filedata.t
  tests/test-wireproto-command-manifestdata.t

CHANGE DETAILS

diff --git a/tests/test-wireproto-command-manifestdata.t b/tests/test-wireproto-command-manifestdata.t
--- a/tests/test-wireproto-command-manifestdata.t
+++ b/tests/test-wireproto-command-manifestdata.t
@@ -65,14 +65,14 @@
   s>     Content-Type: application/mercurial-exp-framing-0005\r\n
   s>     Transfer-Encoding: chunked\r\n
   s>     \r\n
-  s>     45\r\n
-  s>     =\x00\x00\x01\x00\x02\x012
-  s>     \xa2Eerror\xa1GmessageX\x1enodes argument must be definedFstatusEerror
+  s>     4e\r\n
+  s>     F\x00\x00\x01\x00\x02\x012
+  s>     \xa2Eerror\xa1GmessageX\'missing required arguments: nodes, treeFstatusEerror
   s>     \r\n
-  received frame(size=61; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
+  received frame(size=70; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
   s>     0\r\n
   s>     \r\n
-  abort: nodes argument must be defined!
+  abort: missing required arguments: nodes, tree!
   [255]
 
   $ sendhttpv2peer << EOF
@@ -97,14 +97,14 @@
   s>     Content-Type: application/mercurial-exp-framing-0005\r\n
   s>     Transfer-Encoding: chunked\r\n
   s>     \r\n
-  s>     44\r\n
-  s>     <\x00\x00\x01\x00\x02\x012
-  s>     \xa2Eerror\xa1GmessageX\x1dtree argument must be definedFstatusEerror
+  s>     47\r\n
+  s>     ?\x00\x00\x01\x00\x02\x012
+  s>     \xa2Eerror\xa1GmessageX missing required arguments: treeFstatusEerror
   s>     \r\n
-  received frame(size=60; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
+  received frame(size=63; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
   s>     0\r\n
   s>     \r\n
-  abort: tree argument must be defined!
+  abort: missing required arguments: tree!
   [255]
 
 Unknown node is an error
diff --git a/tests/test-wireproto-command-filedata.t b/tests/test-wireproto-command-filedata.t
--- a/tests/test-wireproto-command-filedata.t
+++ b/tests/test-wireproto-command-filedata.t
@@ -69,14 +69,14 @@
   s>     Content-Type: application/mercurial-exp-framing-0005\r\n
   s>     Transfer-Encoding: chunked\r\n
   s>     \r\n
-  s>     45\r\n
-  s>     =\x00\x00\x01\x00\x02\x012
-  s>     \xa2Eerror\xa1GmessageX\x1enodes argument must be definedFstatusEerror
+  s>     4e\r\n
+  s>     F\x00\x00\x01\x00\x02\x012
+  s>     \xa2Eerror\xa1GmessageX\'missing required arguments: nodes, pathFstatusEerror
   s>     \r\n
-  received frame(size=61; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
+  received frame(size=70; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
   s>     0\r\n
   s>     \r\n
-  abort: nodes argument must be defined!
+  abort: missing required arguments: nodes, path!
   [255]
 
   $ sendhttpv2peer << EOF
@@ -101,14 +101,14 @@
   s>     Content-Type: application/mercurial-exp-framing-0005\r\n
   s>     Transfer-Encoding: chunked\r\n
   s>     \r\n
-  s>     44\r\n
-  s>     <\x00\x00\x01\x00\x02\x012
-  s>     \xa2Eerror\xa1GmessageX\x1dpath argument must be definedFstatusEerror
+  s>     47\r\n
+  s>     ?\x00\x00\x01\x00\x02\x012
+  s>     \xa2Eerror\xa1GmessageX missing required arguments: pathFstatusEerror
   s>     \r\n
-  received frame(size=60; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
+  received frame(size=63; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=eos)
   s>     0\r\n
   s>     \r\n
-  abort: path argument must be defined!
+  abort: missing required arguments: path!
   [255]
 
 Unknown node is an error
diff --git a/tests/test-wireproto-command-capabilities.t b/tests/test-wireproto-command-capabilities.t
--- a/tests/test-wireproto-command-capabilities.t
+++ b/tests/test-wireproto-command-capabilities.t
@@ -212,7 +212,7 @@
   s>     Content-Type: application/mercurial-cbor\r\n
   s>     Content-Length: *\r\n (glob)
   s>     \r\n
-  s>     \xa3GapibaseDapi/Dapis\xa1Pexp-http-v2-0001\xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree at Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1Nv1capabilitiesY\x01\xd3batch branchmap $USUAL_BUNDLE2_CAPS$ changegroupsubset compression=$BUNDLE2_COMPRESSIONS$ getbundle httpheader=1024 httpmediatype=0.1rx,0.1tx,0.2tx known lookup pushkey streamreqs=generaldelta,revlogv1 unbundle=HG10GZ,HG10BZ,HG10UN unbundlehash
+  s>     \xa3GapibaseDapi/Dapis\xa1Pexp-http-v2-0001\xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree at Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1Nv1capabilitiesY\x01\xd3batch branchmap $USUAL_BUNDLE2_CAPS$ changegroupsubset compression=$BUNDLE2_COMPRESSIONS$ getbundle httpheader=1024 httpmediatype=0.1rx,0.1tx,0.2tx known lookup pushkey streamreqs=generaldelta,revlogv1 unbundle=HG10GZ,HG10BZ,HG10UN unbundlehash
   cbor> {
     b'apibase': b'api/',
     b'apis': {
@@ -254,10 +254,10 @@
           },
           b'filedata': {
             b'args': {
-              b'fields': [
+              b'fields': set([
                 b'parents',
                 b'revision'
-              ],
+              ]),
               b'haveparents': True,
               b'nodes': [
                 b'0123456...'
@@ -304,10 +304,10 @@
           },
           b'manifestdata': {
             b'args': {
-              b'fields': [
+              b'fields': set([
                 b'parents',
                 b'revision'
-              ],
+              ]),
               b'haveparents': True,
               b'nodes': [
                 b'0123456...'
@@ -369,7 +369,7 @@
   s>     Content-Type: application/mercurial-cbor\r\n
   s>     Content-Length: *\r\n (glob)
   s>     \r\n
-  s>     \xa3GapibaseDapi/Dapis\xa1Pexp-http-v2-0001\xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree at Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1Nv1capabilitiesY\x01\xd3batch branchmap $USUAL_BUNDLE2_CAPS$ changegroupsubset compression=$BUNDLE2_COMPRESSIONS$ getbundle httpheader=1024 httpmediatype=0.1rx,0.1tx,0.2tx known lookup pushkey streamreqs=generaldelta,revlogv1 unbundle=HG10GZ,HG10BZ,HG10UN unbundlehash
+  s>     \xa3GapibaseDapi/Dapis\xa1Pexp-http-v2-0001\xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree at Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1Nv1capabilitiesY\x01\xd3batch branchmap $USUAL_BUNDLE2_CAPS$ changegroupsubset compression=$BUNDLE2_COMPRESSIONS$ getbundle httpheader=1024 httpmediatype=0.1rx,0.1tx,0.2tx known lookup pushkey streamreqs=generaldelta,revlogv1 unbundle=HG10GZ,HG10BZ,HG10UN unbundlehash
   sending capabilities command
   s>     POST /api/exp-http-v2-0001/ro/capabilities HTTP/1.1\r\n
   s>     Accept-Encoding: identity\r\n
@@ -392,11 +392,11 @@
   s>     \xa1FstatusBok
   s>     \r\n
   received frame(size=11; request=1; stream=2; streamflags=stream-begin; type=command-response; flags=continuation)
-  s>     30e\r\n
-  s>     \x06\x03\x00\x01\x00\x02\x001
-  s>     \xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree at Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1
+  s>     314\r\n
+  s>     \x0c\x03\x00\x01\x00\x02\x001
+  s>     \xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree at Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1
   s>     \r\n
-  received frame(size=774; request=1; stream=2; streamflags=; type=command-response; flags=continuation)
+  received frame(size=780; request=1; stream=2; streamflags=; type=command-response; flags=continuation)
   s>     8\r\n
   s>     \x00\x00\x00\x01\x00\x02\x002
   s>     \r\n
@@ -442,10 +442,10 @@
         },
         b'filedata': {
           b'args': {
-            b'fields': [
+            b'fields': set([
               b'parents',
               b'revision'
-            ],
+            ]),
             b'haveparents': True,
             b'nodes': [
               b'0123456...'
@@ -492,10 +492,10 @@
         },
         b'manifestdata': {
           b'args': {
-            b'fields': [
+            b'fields': set([
               b'parents',
               b'revision'
-            ],
+            ]),
             b'haveparents': True,
             b'nodes': [
               b'0123456...'
diff --git a/tests/test-http-protocol.t b/tests/test-http-protocol.t
--- a/tests/test-http-protocol.t
+++ b/tests/test-http-protocol.t
@@ -313,7 +313,7 @@
   s>     Content-Type: application/mercurial-cbor\r\n
   s>     Content-Length: *\r\n (glob)
   s>     \r\n
-  s>     \xa3GapibaseDapi/Dapis\xa1Pexp-http-v2-0001\xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree at Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1Nv1capabilitiesY\x01\xd3batch branchmap $USUAL_BUNDLE2_CAPS$ changegroupsubset compression=$BUNDLE2_COMPRESSIONS$ getbundle httpheader=1024 httpmediatype=0.1rx,0.1tx,0.2tx known lookup pushkey streamreqs=generaldelta,revlogv1 unbundle=HG10GZ,HG10BZ,HG10UN unbundlehash
+  s>     \xa3GapibaseDapi/Dapis\xa1Pexp-http-v2-0001\xa4Hcommands\xaaIbranchmap\xa2Dargs\xa0Kpermissions\x81DpullLcapabilities\xa2Dargs\xa0Kpermissions\x81DpullMchangesetdata\xa2Dargs\xa3Ffields\xd9\x01\x02\x82GparentsHrevisionInoderange\x82\x81J0123456...\x81Iabcdef...Enodes\x81J0123456...Kpermissions\x81DpullHfiledata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...DpathGfoo.txtKpermissions\x81DpullEheads\xa2Dargs\xa1Jpubliconly\xf4Kpermissions\x81DpullEknown\xa2Dargs\xa1Enodes\x81HdeadbeefKpermissions\x81DpullHlistkeys\xa2Dargs\xa1InamespaceBnsKpermissions\x81DpullFlookup\xa2Dargs\xa1CkeyCfooKpermissions\x81DpullLmanifestdata\xa2Dargs\xa4Ffields\xd9\x01\x02\x82GparentsHrevisionKhaveparents\xf5Enodes\x81J0123456...Dtree at Kpermissions\x81DpullGpushkey\xa2Dargs\xa4CkeyCkeyInamespaceBnsCnewCnewColdColdKpermissions\x81DpushKcompression\x81\xa1DnameDzlibQframingmediatypes\x81X&application/mercurial-exp-framing-0005Nrawrepoformats\x82LgeneraldeltaHrevlogv1Nv1capabilitiesY\x01\xd3batch branchmap $USUAL_BUNDLE2_CAPS$ changegroupsubset compression=$BUNDLE2_COMPRESSIONS$ getbundle httpheader=1024 httpmediatype=0.1rx,0.1tx,0.2tx known lookup pushkey streamreqs=generaldelta,revlogv1 unbundle=HG10GZ,HG10BZ,HG10UN unbundlehash
   sending heads command
   s>     POST /api/exp-http-v2-0001/ro/heads HTTP/1.1\r\n
   s>     Accept-Encoding: identity\r\n
diff --git a/mercurial/wireprotov2server.py b/mercurial/wireprotov2server.py
--- a/mercurial/wireprotov2server.py
+++ b/mercurial/wireprotov2server.py
@@ -311,6 +311,10 @@
         action, meta = reactor.oncommandresponsereadyobjects(
             outstream, command['requestid'], objs)
 
+    except error.WireprotoCommandError as e:
+        action, meta = reactor.oncommanderror(
+            outstream, command['requestid'], e.message, e.messageargs)
+
     except Exception as e:
         action, meta = reactor.onservererror(
             outstream, command['requestid'],
@@ -348,13 +352,39 @@
         return HTTP_WIREPROTO_V2
 
     def getargs(self, args):
+        # First look for args that were passed but aren't registered on this
+        # command.
+        extra = set(self._args) - set(args)
+        if extra:
+            raise error.WireprotoCommandError(
+                'unsupported argument to command: %s' %
+                ', '.join(sorted(extra)))
+
+        # And look for required arguments that are missing.
+        missing = {a for a in args if args[a]['required']} - set(self._args)
+
+        if missing:
+            raise error.WireprotoCommandError(
+                'missing required arguments: %s' % ', '.join(sorted(missing)))
+
+        # Now derive the arguments to pass to the command, taking into
+        # account the arguments specified by the client.
         data = {}
-        for k, typ in args.items():
-            if k == '*':
-                raise NotImplementedError('do not support * args')
-            elif k in self._args:
-                # TODO consider validating value types.
-                data[k] = self._args[k]
+        for k, meta in sorted(args.items()):
+            # This argument wasn't passed by the client.
+            if k not in self._args:
+                data[k] = meta['default']()
+                continue
+
+            v = self._args[k]
+
+            # Sets may be expressed as lists. Silently normalize.
+            if meta['type'] == 'set' and isinstance(v, list):
+                v = set(v)
+
+            # TODO consider more/stronger type validation.
+
+            data[k] = v
 
         return data
 
@@ -404,8 +434,10 @@
     # TODO expose available changesetdata fields.
 
     for command, entry in COMMANDS.items():
+        args = {arg: meta['example'] for arg, meta in entry.args.items()}
+
         caps['commands'][command] = {
-            'args': entry.args,
+            'args': args,
             'permissions': [entry.permission],
         }
 
@@ -500,7 +532,23 @@
 
     ``name`` is the name of the wire protocol command being provided.
 
-    ``args`` is a dict of argument names to example values.
+    ``args`` is a dict defining arguments accepted by the command. Keys are
+    the argument name. Values are dicts with the following keys:
+
+       ``type``
+          The argument data type. Must be one of the following string
+          literals: ``bytes``, ``int``, ``list``, ``dict``, ``set``,
+          or ``bool``.
+
+       ``default``
+          A callable returning the default value for this argument. If not
+          specified, ``None`` will be the default value.
+
+       ``required``
+          Bool indicating whether the argument is required.
+
+       ``example``
+          An example value for this argument.
 
     ``permission`` defines the permission type needed to run this command.
     Can be ``push`` or ``pull``. These roughly map to read-write and read-only,
@@ -529,6 +577,36 @@
         raise error.ProgrammingError('arguments for version 2 commands '
                                      'must be declared as dicts')
 
+    for arg, meta in args.items():
+        if arg == '*':
+            raise error.ProgrammingError('* argument name not allowed on '
+                                         'version 2 commands')
+
+        if not isinstance(meta, dict):
+            raise error.ProgrammingError('arguments for version 2 commands '
+                                         'must declare metadata as a dict')
+
+        if 'type' not in meta:
+            raise error.ProgrammingError('%s argument for command %s does not '
+                                         'declare type field' % (arg, name))
+
+        if meta['type'] not in ('bytes', 'int', 'list', 'dict', 'set', 'bool'):
+            raise error.ProgrammingError('%s argument for command %s has '
+                                         'illegal type: %s' % (arg, name,
+                                                               meta['type']))
+
+        if 'example' not in meta:
+            raise error.ProgrammingError('%s argument for command %s does not '
+                                         'declare example field' % (arg, name))
+
+        if 'default' in meta and meta.get('required'):
+            raise error.ProgrammingError('%s argument for command %s is marked '
+                                         'as required but has a default value' %
+                                         (arg, name))
+
+        meta.setdefault('default', lambda: None)
+        meta.setdefault('required', False)
+
     def register(func):
         if name in COMMANDS:
             raise error.ProgrammingError('%s command already registered '
@@ -550,16 +628,25 @@
 def capabilitiesv2(repo, proto):
     yield _capabilitiesv2(repo, proto)
 
- at wireprotocommand('changesetdata',
-                  args={
-                      'noderange': [[b'0123456...'], [b'abcdef...']],
-                      'nodes': [b'0123456...'],
-                      'fields': {b'parents', b'revision'},
-                  },
-                  permission='pull')
-def changesetdata(repo, proto, noderange=None, nodes=None, fields=None):
-    fields = fields or set()
-
+ at wireprotocommand(
+    'changesetdata',
+    args={
+        'noderange': {
+            'type': 'list',
+            'example': [[b'0123456...'], [b'abcdef...']],
+        },
+        'nodes': {
+            'type': 'list',
+            'example': [b'0123456...'],
+        },
+        'fields': {
+            'type': 'set',
+            'default': set,
+            'example': {b'parents', b'revision'},
+        },
+    },
+    permission='pull')
+def changesetdata(repo, proto, noderange, nodes, fields):
     # TODO look for unknown fields and abort when they can't be serviced.
 
     if noderange is None and nodes is None:
@@ -691,24 +778,32 @@
 
     return fl
 
- at wireprotocommand('filedata',
-                  args={
-                      'haveparents': True,
-                      'nodes': [b'0123456...'],
-                      'fields': [b'parents', b'revision'],
-                      'path': b'foo.txt',
-                  },
-                  permission='pull')
-def filedata(repo, proto, haveparents=False, nodes=None, fields=None,
-             path=None):
-    fields = fields or set()
-
-    if nodes is None:
-        raise error.WireprotoCommandError('nodes argument must be defined')
-
-    if path is None:
-        raise error.WireprotoCommandError('path argument must be defined')
-
+ at wireprotocommand(
+    'filedata',
+    args={
+        'haveparents': {
+            'type': 'bool',
+            'default': lambda: False,
+            'example': True,
+        },
+        'nodes': {
+            'type': 'list',
+            'required': True,
+            'example': [b'0123456...'],
+        },
+        'fields': {
+            'type': 'set',
+            'default': set,
+            'example': {b'parents', b'revision'},
+        },
+        'path': {
+            'type': 'bytes',
+            'required': True,
+            'example': b'foo.txt',
+        }
+    },
+    permission='pull')
+def filedata(repo, proto, haveparents, nodes, fields, path):
     try:
         # Extensions may wish to access the protocol handler.
         store = getfilestore(repo, proto, path)
@@ -776,72 +871,97 @@
         except GeneratorExit:
             pass
 
- at wireprotocommand('heads',
-                  args={
-                      'publiconly': False,
-                  },
-                  permission='pull')
-def headsv2(repo, proto, publiconly=False):
+ at wireprotocommand(
+    'heads',
+    args={
+        'publiconly': {
+            'type': 'bool',
+            'default': lambda: False,
+            'example': False,
+        },
+    },
+    permission='pull')
+def headsv2(repo, proto, publiconly):
     if publiconly:
         repo = repo.filtered('immutable')
 
     yield repo.heads()
 
- at wireprotocommand('known',
-                  args={
-                      'nodes': [b'deadbeef'],
-                  },
-                  permission='pull')
-def knownv2(repo, proto, nodes=None):
-    nodes = nodes or []
+ at wireprotocommand(
+    'known',
+    args={
+        'nodes': {
+            'type': 'list',
+            'default': list,
+            'example': [b'deadbeef'],
+        },
+    },
+    permission='pull')
+def knownv2(repo, proto, nodes):
     result = b''.join(b'1' if n else b'0' for n in repo.known(nodes))
     yield result
 
- at wireprotocommand('listkeys',
-                  args={
-                      'namespace': b'ns',
-                  },
-                  permission='pull')
-def listkeysv2(repo, proto, namespace=None):
+ at wireprotocommand(
+    'listkeys',
+    args={
+        'namespace': {
+            'type': 'bytes',
+            'required': True,
+            'example': b'ns',
+        },
+    },
+    permission='pull')
+def listkeysv2(repo, proto, namespace):
     keys = repo.listkeys(encoding.tolocal(namespace))
     keys = {encoding.fromlocal(k): encoding.fromlocal(v)
             for k, v in keys.iteritems()}
 
     yield keys
 
- at wireprotocommand('lookup',
-                  args={
-                      'key': b'foo',
-                  },
-                  permission='pull')
+ at wireprotocommand(
+    'lookup',
+    args={
+        'key': {
+            'type': 'bytes',
+            'required': True,
+            'example': b'foo',
+        },
+    },
+    permission='pull')
 def lookupv2(repo, proto, key):
     key = encoding.tolocal(key)
 
     # TODO handle exception.
     node = repo.lookup(key)
 
     yield node
 
- at wireprotocommand('manifestdata',
-                  args={
-                      'nodes': [b'0123456...'],
-                      'haveparents': True,
-                      'fields': [b'parents', b'revision'],
-                      'tree': b'',
-                  },
-                  permission='pull')
-def manifestdata(repo, proto, haveparents=False, nodes=None, fields=None,
-                 tree=None):
-    fields = fields or set()
-
-    if nodes is None:
-        raise error.WireprotoCommandError(
-            'nodes argument must be defined')
-
-    if tree is None:
-        raise error.WireprotoCommandError(
-            'tree argument must be defined')
-
+ at wireprotocommand(
+    'manifestdata',
+    args={
+        'nodes': {
+            'type': 'list',
+            'required': True,
+            'example': [b'0123456...'],
+        },
+        'haveparents': {
+            'type': 'bool',
+            'default': lambda: False,
+            'example': True,
+        },
+        'fields': {
+            'type': 'set',
+            'default': set,
+            'example': {b'parents', b'revision'},
+        },
+        'tree': {
+            'type': 'bytes',
+            'required': True,
+            'example': b'',
+        },
+    },
+    permission='pull')
+def manifestdata(repo, proto, haveparents, nodes, fields, tree):
     store = repo.manifestlog.getstorage(tree)
 
     # Validate the node is known and abort on unknown revisions.
@@ -905,14 +1025,31 @@
         except GeneratorExit:
             pass
 
- at wireprotocommand('pushkey',
-                  args={
-                      'namespace': b'ns',
-                      'key': b'key',
-                      'old': b'old',
-                      'new': b'new',
-                  },
-                  permission='push')
+ at wireprotocommand(
+    'pushkey',
+    args={
+        'namespace': {
+            'type': 'bytes',
+            'required': True,
+            'example': b'ns',
+        },
+        'key': {
+            'type': 'bytes',
+            'required': True,
+            'example': b'key',
+        },
+        'old': {
+            'type': 'bytes',
+            'required': True,
+            'example': b'old',
+        },
+        'new': {
+            'type': 'bytes',
+            'required': True,
+            'example': 'new',
+        },
+    },
+    permission='push')
 def pushkeyv2(repo, proto, namespace, key, old, new):
     # TODO handle ui output redirection
     yield repo.pushkey(encoding.tolocal(namespace),



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


More information about the Mercurial-devel mailing list