launchpad-reviewers team mailing list archive

[John A Meinel <john@xxxxxxxxxxxxxxxxx>]

John A Meinel has proposed merging lp:~jameinel/maas/get-nodes-idempotent into lp:maas.

Commit message:
Turn /tags/TAG/?op=nodes into a proper GET response.
The old infrastructure didn't allow a GET with an op to be mixed with GET without an op. But since Gavin fixed it, we can use it the way we should.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jameinel/maas/get-nodes-idempotent/+merge/127549
-- 
https://code.launchpad.net/~jameinel/maas/get-nodes-idempotent/+merge/127549
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jameinel/maas/get-nodes-idempotent into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-10-02 16:08:07 +0000
+++ src/maasserver/api.py	2012-10-02 17:27:22 +0000
@@ -1334,11 +1334,7 @@
         tag.delete()
         return rc.DELETED
 
-    # XXX: JAM 2012-09-25 This is currently a POST because of bug:
-    #      http://pad.lv/1049933
-    #      Essentially, if you have one 'GET' op, then you can no longer get
-    #      the Tag object itself from a plain 'GET' without op.
-    @operation(idempotent=False)
+    @operation(idempotent=True)
     def nodes(self, request, name):
         """Get the list of nodes that have this tag."""
         return Tag.objects.get_nodes(name, user=request.user)

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-10-02 16:08:07 +0000
+++ src/maasserver/tests/test_api.py	2012-10-02 17:27:22 +0000
@@ -2401,28 +2401,28 @@
         self.assertItemsEqual([], node1.tag_names())
         self.assertItemsEqual([tag.name], node2.tag_names())
 
-    def test_POST_nodes_with_no_nodes(self):
+    def test_GET_nodes_with_no_nodes(self):
         tag = factory.make_tag()
-        response = self.client.post(self.get_tag_uri(tag), {'op': 'nodes'})
+        response = self.client.get(self.get_tag_uri(tag), {'op': 'nodes'})
 
         self.assertEqual(httplib.OK, response.status_code)
         parsed_result = json.loads(response.content)
         self.assertEqual([], parsed_result)
 
-    def test_POST_nodes_returns_nodes(self):
+    def test_GET_nodes_returns_nodes(self):
         tag = factory.make_tag()
         node1 = factory.make_node()
         # Create a second node that isn't tagged.
         factory.make_node()
         node1.tags.add(tag)
-        response = self.client.post(self.get_tag_uri(tag), {'op': 'nodes'})
+        response = self.client.get(self.get_tag_uri(tag), {'op': 'nodes'})
 
         self.assertEqual(httplib.OK, response.status_code)
         parsed_result = json.loads(response.content)
         self.assertEqual([node1.system_id],
                          [r['system_id'] for r in parsed_result])
 
-    def test_POST_nodes_hides_invisible_nodes(self):
+    def test_GET_nodes_hides_invisible_nodes(self):
         user2 = factory.make_user()
         node1 = factory.make_node()
         node1.set_hardware_details('<node><foo /></node>')
@@ -2430,7 +2430,7 @@
         node2.set_hardware_details('<node><bar /></node>')
         tag = factory.make_tag(definition='/node')
         tag.populate_nodes()
-        response = self.client.post(self.get_tag_uri(tag), {'op': 'nodes'})
+        response = self.client.get(self.get_tag_uri(tag), {'op': 'nodes'})
 
         self.assertEqual(httplib.OK, response.status_code)
         parsed_result = json.loads(response.content)
@@ -2438,7 +2438,7 @@
                          [r['system_id'] for r in parsed_result])
         # However, for the other user, they should see the result
         client2 = OAuthAuthenticatedClient(user2)
-        response = client2.post(self.get_tag_uri(tag), {'op': 'nodes'})
+        response = client2.get(self.get_tag_uri(tag), {'op': 'nodes'})
         self.assertEqual(httplib.OK, response.status_code)
         parsed_result = json.loads(response.content)
         self.assertItemsEqual([node1.system_id, node2.system_id],