Merge "Have resource find use get if possible"

openstack/resource.py

@@ -937,7 +937,7 @@ class Resource(collections.MutableMapping):
                  than one resource is found for this request.
         """
         def get_one_match(data, attr):
-            if len(info) == 1:
+            if len(data) == 1:
                 result = cls.existing(**data[0])
                 value = getattr(result, attr, False)
                 if value == name_or_id:
@@ -945,12 +945,10 @@ class Resource(collections.MutableMapping):
             return None
 
         try:
-            args = {
-                cls.id_attribute: name_or_id,
-                'path_args': path_args,
-            }
-            info = cls.page(session, limit=2, **args)
-
+            if cls.allow_retrieve:
+                return cls.get_by_id(session, name_or_id, path_args=path_args)
+            args = {cls.id_attribute: name_or_id}
+            info = cls.page(session, limit=2, path_args=path_args, **args)
             result = get_one_match(info, cls.id_attribute)
             if result is not None:
                 return result

openstack/tests/unit/test_resource.py

@@ -1048,18 +1048,6 @@ class ResourceTests(base.TestTransportBase):
             faker.enabled = 'INVALID'
         self.assertRaises(ValueError, set_invalid)
 
-    @mock.patch("openstack.resource.Resource.list")
-    def test_fallthrough(self, mock_list):
-        class FakeResource2(FakeResource):
-            name_attribute = None
-
-            @classmethod
-            def page(cls, session, limit=None, marker=None, path_args=None,
-                     **params):
-                raise exceptions.HttpException("exception")
-
-        self.assertEqual(None, FakeResource2.find("session", "123"))
-
 
 class ResourceMapping(base.TestCase):
 
@@ -1202,7 +1190,7 @@ class TestFind(base.TestCase):
 
     def test_name(self):
         self.mock_get.side_effect = [
-            FakeResponse({FakeResource.resources_key: []}),
+            exceptions.HttpException(404, 'not found'),
             FakeResponse({FakeResource.resources_key: [self.matrix]})
         ]
 
@@ -1218,7 +1206,7 @@ class TestFind(base.TestCase):
 
     def test_id(self):
         self.mock_get.side_effect = [
-            FakeResponse({FakeResource.resources_key: [self.matrix]})
+            FakeResponse({FakeResource.resource_key: self.matrix})
         ]
 
         result = FakeResource.find(self.mock_session, self.ID,
@@ -1227,15 +1215,33 @@ class TestFind(base.TestCase):
         self.assertEqual(self.ID, result.id)
         self.assertEqual(self.PROP, result.prop)
 
+        path = "fakes/rey/data/" + self.ID
+        self.mock_get.assert_any_call(path, service=None)
+
+    def test_id_no_retrieve(self):
+        self.mock_get.side_effect = [
+            FakeResponse({FakeResource.resources_key: [self.matrix]})
+        ]
+
+        class NoRetrieveResource(FakeResource):
+            allow_retrieve = False
+
+        result = NoRetrieveResource.find(self.mock_session, self.ID,
+                                         path_args=fake_arguments)
+
+        self.assertEqual(self.ID, result.id)
+        self.assertEqual(self.PROP, result.prop)
+
         p = {'id': self.ID}
         path = fake_path + "?limit=2"
         self.mock_get.assert_any_call(path, params=p, service=None)
 
     def test_dups(self):
         dup = {'id': 'Larry'}
-        resp = FakeResponse(
-            {FakeResource.resources_key: [self.matrix, dup]})
-        self.mock_get.return_value = resp
+        self.mock_get.side_effect = [
+            exceptions.HttpException(404, 'not found'),
+            FakeResponse({FakeResource.resources_key: [self.matrix, dup]})
+        ]
 
         self.assertRaises(exceptions.DuplicateResource, FakeResource.find,
                           self.mock_session, self.NAME)
@@ -1243,9 +1249,10 @@ class TestFind(base.TestCase):
     def test_id_attribute_find(self):
         floater = {'ip_address': "127.0.0.1", 'prop': self.PROP}
         self.mock_get.side_effect = [
-            FakeResponse({FakeResource.resources_key: [floater]})
+            FakeResponse({FakeResource.resource_key: floater})
         ]
 
+        FakeResource.id_attribute = 'ip_address'
         FakeResource.id_attribute = 'ip_address'
         result = FakeResource.find(self.mock_session, "127.0.0.1",
                                    path_args=fake_arguments)
@@ -1256,17 +1263,19 @@ class TestFind(base.TestCase):
 
         p = {'ip_address': "127.0.0.1"}
         path = fake_path + "?limit=2"
-        self.mock_get.assert_any_call(path, params=p, service=None)
+        self.mock_get.called_once_with(path, params=p, service=None)
 
     def test_nada(self):
-        resp = FakeResponse({FakeResource.resources_key: []})
-        self.mock_get.return_value = resp
+        self.mock_get.side_effect = [
+            exceptions.HttpException(404, 'not found'),
+            FakeResponse({FakeResource.resources_key: []})
+        ]
 
         self.assertEqual(None, FakeResource.find(self.mock_session, self.NAME))
 
     def test_no_name(self):
         self.mock_get.side_effect = [
-            FakeResponse({FakeResource.resources_key: []}),
+            exceptions.HttpException(404, 'not found'),
             FakeResponse({FakeResource.resources_key: [self.matrix]})
         ]
         FakeResource.name_attribute = None