Find resource refactoring

The 'name' field for some resources is called something different,
for example 'display_name' for volumes, 'name' for  volumes type.
So class 'resource' has attribute 'NAME_ATTR' wich contains
attribute name for different resources.

This change removes reduntant call find in resource manager, instead
of it checks NAME_ATTR value.

Related-Bug: #1449444
Change-Id: Ide334d7535c73cbdff72c30319eb539d8f5304d6

cinderclient/tests/unit/test_utils.py

@@ -26,6 +26,7 @@ UUID = '8e8ec658-c7b0-4243-bdf8-6f7f2952c0d0'
 
 
 class FakeResource(object):
+    NAME_ATTR = 'name'
 
     def __init__(self, _id, properties):
         self.id = _id
@@ -33,10 +34,6 @@ class FakeResource(object):
             self.name = properties['name']
         except KeyError:
             pass
-        try:
-            self.display_name = properties['display_name']
-        except KeyError:
-            pass
 
 
 class FakeManager(base.ManagerWithFind):
@@ -46,7 +43,6 @@ class FakeManager(base.ManagerWithFind):
     resources = [
         FakeResource('1234', {'name': 'entity_one'}),
         FakeResource(UUID, {'name': 'entity_two'}),
-        FakeResource('4242', {'display_name': 'entity_three'}),
         FakeResource('5678', {'name': '9876'})
     ]
 
@@ -60,6 +56,26 @@ class FakeManager(base.ManagerWithFind):
         return self.resources
 
 
+class FakeDisplayResource(object):
+    NAME_ATTR = 'display_name'
+
+    def __init__(self, _id, properties):
+        self.id = _id
+        try:
+            self.display_name = properties['display_name']
+        except KeyError:
+            pass
+
+
+class FakeDisplayManager(FakeManager):
+
+    resource_class = FakeDisplayResource
+
+    resources = [
+        FakeDisplayResource('4242', {'display_name': 'entity_three'}),
+    ]
+
+
 class FindResourceTestCase(test_utils.TestCase):
 
     def setUp(self):
@@ -72,7 +88,7 @@ class FindResourceTestCase(test_utils.TestCase):
                           utils.find_resource,
                           self.manager,
                           'asdf')
-        self.assertEqual(3, self.manager.find.call_count)
+        self.assertEqual(2, self.manager.find.call_count)
 
     def test_find_by_integer_id(self):
         output = utils.find_resource(self.manager, 1234)
@@ -91,8 +107,9 @@ class FindResourceTestCase(test_utils.TestCase):
         self.assertEqual(self.manager.get('1234'), output)
 
     def test_find_by_str_displayname(self):
-        output = utils.find_resource(self.manager, 'entity_three')
+        display_manager = FakeDisplayManager(None)
-        self.assertEqual(self.manager.get('4242'), output)
+        output = utils.find_resource(display_manager, 'entity_three')
+        self.assertEqual(display_manager.get('4242'), output)
 
 
 class CaptureStdout(object):

cinderclient/tests/unit/v1/fakes.py

@@ -299,8 +299,8 @@ class FakeHTTPClient(base_client.HTTPClient):
 
     def get_volumes(self, **kw):
         return (200, {}, {"volumes": [
-            {'id': 1234, 'name': 'sample-volume'},
+            {'id': 1234, 'display_name': 'sample-volume'},
-            {'id': 5678, 'name': 'sample-volume2'}
+            {'id': 5678, 'display_name': 'sample-volume2'}
         ]})
 
     # TODO(jdg): This will need to change
@@ -308,7 +308,7 @@ class FakeHTTPClient(base_client.HTTPClient):
     def get_volumes_detail(self, **kw):
         return (200, {}, {"volumes": [
             {'id': kw.get('id', 1234),
-             'name': 'sample-volume',
+             'display_name': 'sample-volume',
              'attachments': [{'server_id': 1234}]},
         ]})
 

cinderclient/tests/unit/v1/test_shell.py

@@ -115,7 +115,7 @@ class ShellTest(utils.TestCase):
         v = cs.volumes.list()[0]
         setattr(v, 'os-vol-tenant-attr:tenant_id', 'fake_tenant')
         setattr(v, '_info', {'attachments': [{'server_id': 1234}],
-                'id': 1234, 'name': 'sample-volume',
+                'id': 1234, 'display_name': 'sample-volume',
                 'os-vol-tenant-attr:tenant_id': 'fake_tenant'})
         shell_v1._translate_volume_keys([v])
         self.assertEqual(v.tenant_id, 'fake_tenant')

cinderclient/utils.py

@@ -178,24 +178,21 @@ def find_resource(manager, name_or_id):
         name_or_id = strutils.safe_decode(name_or_id)
 
     try:
+        try:
+            resource = getattr(manager, 'resource_class', None)
+            name_attr = resource.NAME_ATTR if resource else 'name'
+            return manager.find(**{name_attr: name_or_id})
+        except exceptions.NotFound:
+            pass
+
+        # finally try to find entity by human_id
         try:
             return manager.find(human_id=name_or_id)
         except exceptions.NotFound:
-            pass
-
-        # finally try to find entity by name
-        try:
-            return manager.find(name=name_or_id)
-        except exceptions.NotFound:
-            pass
-
-        # Volumes don't have name, but display_name
-        try:
-            return manager.find(display_name=name_or_id)
-        except (UnicodeDecodeError, exceptions.NotFound):
             msg = "No %s with a name or ID of '%s' exists." % \
                 (manager.resource_class.__name__.lower(), name_or_id)
             raise exceptions.CommandError(msg)
+
     except exceptions.NoUniqueMatch:
         msg = ("Multiple %s matches found for '%s', use an ID to be more"
                " specific." % (manager.resource_class.__name__.lower(),

cinderclient/v1/volume_backups.py

@@ -22,6 +22,8 @@ from cinderclient import base
 
 class VolumeBackup(base.Resource):
     """A volume backup is a block level backup of a volume."""
+    NAME_ATTR = "display_name"
+
     def __repr__(self):
         return "<VolumeBackup: %s>" % self.id
 

cinderclient/v1/volume_snapshots.py

@@ -30,6 +30,8 @@ class Snapshot(base.Resource):
     """
     A Snapshot is a point-in-time snapshot of an openstack volume.
     """
+    NAME_ATTR = "display_name"
+
     def __repr__(self):
         return "<Snapshot: %s>" % self.id
 

cinderclient/v1/volume_transfers.py

@@ -27,6 +27,8 @@ from cinderclient import base
 
 class VolumeTransfer(base.Resource):
     """Transfer a volume from one tenant to another"""
+    NAME_ATTR = "display_name"
+
     def __repr__(self):
         return "<VolumeTransfer: %s>" % self.id
 

cinderclient/v1/volumes.py

@@ -27,6 +27,8 @@ from cinderclient import base
 
 class Volume(base.Resource):
     """A volume is an extra block level storage to the OpenStack instances."""
+    NAME_ATTR = "display_name"
+
     def __repr__(self):
         return "<Volume: %s>" % self.id
 

cinderclient/v2/volume_backups.py

@@ -22,6 +22,8 @@ from cinderclient import base
 
 class VolumeBackup(base.Resource):
     """A volume backup is a block level backup of a volume."""
+    NAME_ATTR = "display_name"
+
     def __repr__(self):
         return "<VolumeBackup: %s>" % self.id
 

cinderclient/v2/volume_snapshots.py

@@ -26,6 +26,8 @@ from cinderclient import base
 
 class Snapshot(base.Resource):
     """A Snapshot is a point-in-time snapshot of an openstack volume."""
+    NAME_ATTR = "display_name"
+
     def __repr__(self):
         return "<Snapshot: %s>" % self.id
 

cinderclient/v2/volume_transfers.py

@@ -27,6 +27,8 @@ from cinderclient import base
 
 class VolumeTransfer(base.Resource):
     """Transfer a volume from one tenant to another"""
+    NAME_ATTR = "display_name"
+
     def __repr__(self):
         return "<VolumeTransfer: %s>" % self.id