From 2a33f8d8cefdbde50f89e6d4084d513eadf3a97a Mon Sep 17 00:00:00 2001
From: Mark McLoughlin <markmc@redhat.com>
Date: Wed, 5 Dec 2012 23:05:59 +0000
Subject: Fix positional arg swallow decorator

The update() method in the hosts extension is decorated with @check_host
but the function returned by decorator takes a kwarg which shadows the
'body' positional arg of the update() method.

i.e. if you call:

  controller.update(req, id, body)

then the body arg gets passed to the wrapper as its service kwarg and
never passed through to update(). We can see how the tests are all
passing body as a kwarg to avoid this.

Remove the service arg from check_host() but also from _list_hosts()
since the only place it is used is in the tests.

Change-Id: I12aab582c15c25fd35ecda005341291450c7efdc
---
 nova/api/openstack/compute/contrib/hosts.py           |  9 +++------
 .../tests/api/openstack/compute/contrib/test_hosts.py | 19 +++++++------------
 2 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/nova/api/openstack/compute/contrib/hosts.py b/nova/api/openstack/compute/contrib/hosts.py
index 8982f5724..0d2b68f11 100644
--- a/nova/api/openstack/compute/contrib/hosts.py
+++ b/nova/api/openstack/compute/contrib/hosts.py
@@ -93,7 +93,7 @@ class HostUpdateDeserializer(wsgi.XMLDeserializer):
         return dict(body=updates)
 
 
-def _list_hosts(req, service=None):
+def _list_hosts(req):
     """Returns a summary list of hosts, optionally filtering
     by service type.
     """
@@ -108,16 +108,13 @@ def _list_hosts(req, service=None):
     for host in services:
         hosts.append({"host_name": host['host'], 'service': host['topic'],
                       'zone': host['availability_zone']})
-    if service:
-        hosts = [host for host in hosts
-                 if host["service"] == service]
     return hosts
 
 
 def check_host(fn):
     """Makes sure that the host exists."""
-    def wrapped(self, req, id, service=None, *args, **kwargs):
-        listed_hosts = _list_hosts(req, service)
+    def wrapped(self, req, id, *args, **kwargs):
+        listed_hosts = _list_hosts(req)
         hosts = [h["host_name"] for h in listed_hosts]
         if id in hosts:
             return fn(self, req, id, *args, **kwargs)
diff --git a/nova/tests/api/openstack/compute/contrib/test_hosts.py b/nova/tests/api/openstack/compute/contrib/test_hosts.py
index f56170a06..b91715190 100644
--- a/nova/tests/api/openstack/compute/contrib/test_hosts.py
+++ b/nova/tests/api/openstack/compute/contrib/test_hosts.py
@@ -124,7 +124,7 @@ class HostTestCase(test.TestCase):
 
     def _test_host_update(self, host, key, val, expected_value):
         body = {key: val}
-        result = self.controller.update(self.req, host, body=body)
+        result = self.controller.update(self.req, host, body)
         self.assertEqual(result[key], expected_value)
 
     def test_list_hosts(self):
@@ -132,11 +132,6 @@ class HostTestCase(test.TestCase):
         hosts = os_hosts._list_hosts(self.req)
         self.assertEqual(hosts, HOST_LIST['hosts'])
 
-        compute_hosts = os_hosts._list_hosts(self.req, "compute")
-        expected = [host for host in HOST_LIST['hosts']
-                if host["service"] == "compute"]
-        self.assertEqual(compute_hosts, expected)
-
     def test_list_hosts_with_zone(self):
         req = FakeRequestWithNovaZone()
         hosts = os_hosts._list_hosts(req)
@@ -173,31 +168,31 @@ class HostTestCase(test.TestCase):
     def test_bad_status_value(self):
         bad_body = {"status": "bad"}
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
-                self.req, "host_c1", body=bad_body)
+                self.req, "host_c1", bad_body)
         bad_body2 = {"status": "disablabc"}
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
-                self.req, "host_c1", body=bad_body2)
+                self.req, "host_c1", bad_body2)
 
     def test_bad_update_key(self):
         bad_body = {"crazy": "bad"}
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
-                self.req, "host_c1", body=bad_body)
+                self.req, "host_c1", bad_body)
 
     def test_bad_update_key_and_correct_udpate_key(self):
         bad_body = {"status": "disable", "crazy": "bad"}
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
-                self.req, "host_c1", body=bad_body)
+                self.req, "host_c1", bad_body)
 
     def test_good_udpate_keys(self):
         body = {"status": "disable", "maintenance_mode": "enable"}
-        result = self.controller.update(self.req, 'host_c1', body=body)
+        result = self.controller.update(self.req, 'host_c1', body)
         self.assertEqual(result["host"], "host_c1")
         self.assertEqual(result["status"], "disabled")
         self.assertEqual(result["maintenance_mode"], "on_maintenance")
 
     def test_bad_host(self):
         self.assertRaises(webob.exc.HTTPNotFound, self.controller.update,
-                self.req, "bogus_host_name", body={"status": "disable"})
+                self.req, "bogus_host_name", {"status": "disable"})
 
     def test_show_forbidden(self):
         self.req.environ["nova.context"].is_admin = False
-- 
cgit