Don't fetch entire list when looking up by ID

Blazar APIs support directly fetching a resource (whether host, floating
ip, or lease) with its ID. However, the default logic for this includes
a block that supports additionally looking up the resource by name (to
allow looking up e.g., a lease by it's name.) This requires fetching the
entire list of all resources and iterating over them until a match is
found, which is very inefficient. Moreover, this branch would be taken
even if the input was already a UUID.

This commit changes that behavior so that if a UUID is provided, it is
used to directly fetch the resource, which seems to be what the original
intent was in the first place.

Change-Id: I5b1ccf4abfe4aa9068bc011764cac4b3507ef697

blazarclient/tests/v1/shell_commands/test_leases.py

@@ -22,6 +22,10 @@ from blazarclient import tests
 from blazarclient.v1.shell_commands import leases
 
 
+FIRST_LEASE = 'd1e43d6d-8f6f-4c2e-b0a9-2982b39dc698'
+SECOND_LEASE = '424d21c3-45a2-448a-81ad-32eddc888375'
+
+
 class CreateLeaseTestCase(tests.TestCase):
 
     def setUp(self):
@@ -304,3 +308,81 @@ class UpdateLeaseTestCase(tests.TestCase):
         }
 
         self.assertDictEqual(self.cl.args2body(args), expected)
+
+
+class ShowLeaseTestCase(tests.TestCase):
+
+    def create_show_command(self):
+        mock_lease_manager = mock.Mock()
+        mock_client = mock.Mock()
+        mock_client.lease = mock_lease_manager
+
+        blazar_shell = shell.BlazarShell()
+        blazar_shell.client = mock_client
+        return (leases.ShowLease(blazar_shell, mock.Mock()),
+                mock_lease_manager)
+
+    def test_show_lease(self):
+        show_lease, lease_manager = self.create_show_command()
+        lease_manager.get.return_value = {'id': FIRST_LEASE}
+        mock.seal(lease_manager)
+
+        args = argparse.Namespace(id=FIRST_LEASE)
+        expected = [('id',), (FIRST_LEASE,)]
+
+        self.assertEqual(show_lease.get_data(args), expected)
+        lease_manager.get.assert_called_once_with(FIRST_LEASE)
+
+    def test_show_lease_by_name(self):
+        show_lease, lease_manager = self.create_show_command()
+        lease_manager.list.return_value = [
+            {'id': FIRST_LEASE, 'name': 'first-lease'},
+            {'id': SECOND_LEASE, 'name': 'second-lease'},
+        ]
+        lease_manager.get.return_value = {'id': SECOND_LEASE}
+        mock.seal(lease_manager)
+
+        args = argparse.Namespace(id='second-lease')
+        expected = [('id',), (SECOND_LEASE,)]
+
+        self.assertEqual(show_lease.get_data(args), expected)
+        lease_manager.list.assert_called_once_with()
+        lease_manager.get.assert_called_once_with(SECOND_LEASE)
+
+
+class DeleteLeaseTestCase(tests.TestCase):
+
+    def create_delete_command(self):
+        mock_lease_manager = mock.Mock()
+        mock_client = mock.Mock()
+        mock_client.lease = mock_lease_manager
+
+        blazar_shell = shell.BlazarShell()
+        blazar_shell.client = mock_client
+        return (leases.DeleteLease(blazar_shell, mock.Mock()),
+                mock_lease_manager)
+
+    def test_delete_lease(self):
+        delete_lease, lease_manager = self.create_delete_command()
+        lease_manager.delete.return_value = None
+        mock.seal(lease_manager)
+
+        args = argparse.Namespace(id=FIRST_LEASE)
+        delete_lease.run(args)
+
+        lease_manager.delete.assert_called_once_with(FIRST_LEASE)
+
+    def test_delete_lease_by_name(self):
+        delete_lease, lease_manager = self.create_delete_command()
+        lease_manager.list.return_value = [
+            {'id': FIRST_LEASE, 'name': 'first-lease'},
+            {'id': SECOND_LEASE, 'name': 'second-lease'},
+        ]
+        lease_manager.delete.return_value = None
+        mock.seal(lease_manager)
+
+        args = argparse.Namespace(id='second-lease')
+        delete_lease.run(args)
+
+        lease_manager.list.assert_called_once_with()
+        lease_manager.delete.assert_called_once_with(SECOND_LEASE)

blazarclient/utils.py

@@ -104,15 +104,9 @@ def get_item_properties(item, fields, mixed_case_fields=None, formatters=None):
 
 def find_resource_id_by_name_or_id(client, resource_type, name_or_id,
                                    name_key, id_pattern):
-    resource_manager = getattr(client, resource_type)
-    is_id = re.match(id_pattern, name_or_id)
-    if is_id:
-        resources = resource_manager.list()
-        for resource in resources:
-            if resource['id'] == name_or_id:
-                return name_or_id
-        raise exception.BlazarClientException('No resource found with ID %s' %
-                                              name_or_id)
+    if re.match(id_pattern, name_or_id):
+        return name_or_id
+
     return _find_resource_id_by_name(client, resource_type, name_or_id,
                                      name_key)
 

lower-constraints.txt

@@ -12,7 +12,7 @@ iso8601==0.1.11
 keystoneauth1==3.4.0
 linecache2==1.0.0
 mccabe==0.2.1
-mock==2.0.0
+mock==3.0.0
 monotonic==0.6
 mox3==0.20.0
 msgpack-python==0.4.0

test-requirements.txt

@@ -2,7 +2,7 @@
 # of appearance. Changing the order has an impact on the overall integration
 # process, which may cause wedges in the gate later.
 hacking>=1.1.0,<1.2.0 # Apache-2.0
-mock>=2.0.0 # BSD
+mock>=3.0.0 # BSD
 oslotest>=3.2.0 # Apache-2.0
 fixtures>=3.0.0 # Apache-2.0/BSD
 testrepository>=0.0.18 # Apache-2.0/BSD