launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/command-line-multiple-args-cli into lp:maas.

Commit message:
Permit multiple data option of the same name to be specified at the command-line.

Previously the data provided at maascli's command-line were collected into a dict, so that only one value per name would work. This now keeps the data as a list of name/value pairs. Tests have been added to demonstrate that prepare_payload() works with a list of data.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~allenap/maas/command-line-multiple-args-cli/+merge/126333
-- 
https://code.launchpad.net/~allenap/maas/command-line-multiple-args-cli/+merge/126333
Your team MAAS Maintainers is requested to review the proposed merge of lp:~allenap/maas/command-line-multiple-args-cli into lp:maas.

=== modified file 'src/maascli/api.py'
--- src/maascli/api.py	2012-09-21 15:10:26 +0000
+++ src/maascli/api.py	2012-09-25 20:30:32 +0000
@@ -210,13 +210,13 @@
         # <https://github.com/uri-templates/uritemplate-py> here?
         uri = self.uri.format(**vars(options))
 
-        # Parse data out of the positional arguments.
-        data = dict(options.data)
+        # Add the operation to the data set.
         if self.op is not None:
-            data["op"] = self.op
+            options.data.append(("op", self.op))
 
         # Bundle things up ready to throw over the wire.
-        uri, body, headers = self.prepare_payload(uri, data)
+        uri, body, headers = self.prepare_payload(
+            self.method, self.restful, uri, options.data)
 
         # Sign request if credentials have been provided.
         if self.credentials is not None:
@@ -246,15 +246,17 @@
             raise CommandError(
                 "%r is not a name=value pair" % string)
 
-    def prepare_payload(self, uri, data):
+    @staticmethod
+    def prepare_payload(method, restful, uri, data):
         """Return the URI (modified perhaps) and body and headers.
 
         :param method: The HTTP method.
+        :param restful: Is this a ReSTful operation?
         :param uri: The URI of the action.
         :param data: A dict or iterable of name=value pairs to pack into the
             body or headers, depending on the type of request.
         """
-        if self.method == "POST" and not self.restful:
+        if method == "POST" and not restful:
             # Encode the data as multipart for non-ReSTful POST requests; all
             # others should use query parameters. TODO: encode_multipart_data
             # insists on a dict for the data, which prevents specifying

=== modified file 'src/maascli/tests/test_api.py'
--- src/maascli/tests/test_api.py	2012-09-17 00:46:49 +0000
+++ src/maascli/tests/test_api.py	2012-09-25 20:30:32 +0000
@@ -120,3 +120,48 @@
         self.assertEqual(
             "Expected application/json, got: text/css",
             "%s" % error)
+
+
+class TestActionReSTful(TestCase):
+    """Tests for ReSTful operations in `maascli.api.Action`."""
+
+    scenarios = (
+        ("create", dict(method="POST")),
+        ("read", dict(method="GET")),
+        ("update", dict(method="PUT")),
+        ("delete", dict(method="DELETE")),
+        )
+
+    def test_prepare_payload_without_data(self):
+        uri_base = "http://example.com/MAAS/api/1.0/";
+        uri, body, headers = api.Action.prepare_payload(
+            method=self.method, restful=True, uri=uri_base, data=[])
+        self.assertEqual(uri_base, uri)
+        self.assertIsNone(body)
+        self.assertEqual({}, headers)
+
+    def test_prepare_payload_with_data(self):
+        # Given data is always encoded as query parameters.
+        uri_base = "http://example.com/MAAS/api/1.0/";
+        uri, body, headers = api.Action.prepare_payload(
+            method=self.method, restful=True, uri=uri_base,
+            data=[("foo", "bar"), ("foo", "baz")])
+        self.assertEqual(uri_base + "?foo=bar&foo=baz", uri)
+        self.assertIsNone(body)
+        self.assertEqual({}, headers)
+
+
+class TestActionOperations(TestCase):
+    """Tests for non-ReSTful operations in `maascli.api.Action`."""
+
+    def test_prepare_payload_POST_non_restful(self):
+        # Non-ReSTful POSTs encode the given data using encode_multipart_data.
+        encode_multipart_data = self.patch(api, "encode_multipart_data")
+        encode_multipart_data.return_value = sentinel.body, sentinel.headers
+        uri_base = "http://example.com/MAAS/api/1.0/";
+        uri, body, headers = api.Action.prepare_payload(
+            method="POST", restful=False, uri=uri_base,
+            data=[("foo", "bar"), ("foo", "baz")])
+        self.assertEqual(uri_base, uri)
+        self.assertIs(sentinel.body, body)
+        self.assertIs(sentinel.headers, headers)