Remove image service show_by_name method

This method isn't worth keeping around. It's used in one place and
consists of a try/except block. We also shouldn't encourage clients
of GlanceImageService to depend on finding a single unique image
by a non-unique attribute.

Change-Id: I02347adef7bc7ac70407226ea150000e55a798bc

8 files changed, 41 insertions, 43 deletions

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 68f3b5b3d..75f28d510 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -1245,9 +1245,11 @@ class CloudController(object):
             internal_id = ec2utils.ec2_id_to_id(ec2_id)
             image = self.image_service.show(context, internal_id)
         except (exception.InvalidEc2Id, exception.ImageNotFound):
+            filters = {'name': ec2_id}
+            images = self.image_service.detail(context, filters=filters)
             try:
-                return self.image_service.show_by_name(context, ec2_id)
-            except exception.NotFound:
+                return images[0]
+            except IndexError:
                 raise exception.ImageNotFound(image_id=ec2_id)
         image_type = ec2_id.split('-')[0]
         if ec2utils.image_type(image.get('container_format')) != image_type:
diff --git a/nova/image/glance.py b/nova/image/glance.py
index 9ad6da528..3a4099338 100644
--- a/nova/image/glance.py
+++ b/nova/image/glance.py
@@ -250,14 +250,6 @@ class GlanceImageService(object):
         base_image_meta = self._translate_from_glance(image_meta)
         return base_image_meta
 
-    def show_by_name(self, context, name):
-        """Returns a dict containing image data for the given name."""
-        image_metas = self.detail(context, filters={'name': name})
-        try:
-            return image_metas[0]
-        except (IndexError, TypeError):
-            raise exception.ImageNotFound(image_id=name)
-
     def get(self, context, image_id, data):
         """Calls out to Glance for metadata and data and writes data."""
         try:
diff --git a/nova/image/s3.py b/nova/image/s3.py
index b5a94cb3f..318847791 100644
--- a/nova/image/s3.py
+++ b/nova/image/s3.py
@@ -142,10 +142,11 @@ class S3ImageService(object):
         images = self.service.index(context, sort_dir='asc')
         return self._translate_uuids_to_ids(context, images)
 
-    def detail(self, context):
+    def detail(self, context, **kwargs):
         #NOTE(bcwaldon): sort asc to make sure we assign lower ids
         # to older images
-        images = self.service.detail(context, sort_dir='asc')
+        kwargs.setdefault('sort_dir', 'asc')
+        images = self.service.detail(context, **kwargs)
         return self._translate_uuids_to_ids(context, images)
 
     def show(self, context, image_id):
@@ -153,10 +154,6 @@ class S3ImageService(object):
         image = self.service.show(context, image_uuid)
         return self._translate_uuid_to_id(context, image)
 
-    def show_by_name(self, context, name):
-        image = self.service.show_by_name(context, name)
-        return self._translate_uuid_to_id(context, image)
-
     @staticmethod
     def _conn(context):
         # NOTE(vish): access and secret keys for s3 server are not
diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py
index 180faeeaa..90724977a 100644
--- a/nova/tests/api/ec2/test_cloud.py
+++ b/nova/tests/api/ec2/test_cloud.py
@@ -103,8 +103,13 @@ class CloudTestCase(test.TestCase):
                         'type': 'machine',
                         'image_state': 'available'}}
 
+        def fake_detail(_self, context, **kwargs):
+            image = fake_show(None, context, None)
+            image['name'] = kwargs.get('filters', {}).get('name')
+            return [image]
+
         self.stubs.Set(fake._FakeImageService, 'show', fake_show)
-        self.stubs.Set(fake._FakeImageService, 'show_by_name', fake_show)
+        self.stubs.Set(fake._FakeImageService, 'detail', fake_detail)
         fake.stub_out_image_service(self.stubs)
 
         def dumb(*args, **kwargs):
@@ -1128,6 +1133,9 @@ class CloudTestCase(test.TestCase):
         def fake_show_none(meh, context, id):
             raise exception.ImageNotFound(image_id='bad_image_id')
 
+        def fake_detail_none(self, context, **kwargs):
+            return []
+
         self.stubs.Set(fake._FakeImageService, 'detail', fake_detail)
         # list all
         result1 = describe_images(self.context)
@@ -1143,7 +1151,7 @@ class CloudTestCase(test.TestCase):
         # provide a non-existing image_id
         self.stubs.UnsetAll()
         self.stubs.Set(fake._FakeImageService, 'show', fake_show_none)
-        self.stubs.Set(fake._FakeImageService, 'show_by_name', fake_show_none)
+        self.stubs.Set(fake._FakeImageService, 'detail', fake_detail_none)
         self.assertRaises(exception.ImageNotFound, describe_images,
                           self.context, ['ami-fake'])
 
@@ -1211,7 +1219,7 @@ class CloudTestCase(test.TestCase):
                     return i
             raise exception.ImageNotFound(image_id=image_id)
 
-        def fake_detail(meh, context):
+        def fake_detail(meh, context, **kwargs):
             return [copy.deepcopy(image1), copy.deepcopy(image2)]
 
         self.stubs.Set(fake._FakeImageService, 'show', fake_show)
@@ -1313,8 +1321,13 @@ class CloudTestCase(test.TestCase):
                     'container_format': 'ami',
                     'is_public': True}
 
+        def fake_detail(self, context, **kwargs):
+            image = fake_show(None, context, None)
+            image['name'] = kwargs.get('filters', {}).get('name')
+            return [image]
+
         self.stubs.Set(fake._FakeImageService, 'show', fake_show)
-        self.stubs.Set(fake._FakeImageService, 'show_by_name', fake_show)
+        self.stubs.Set(fake._FakeImageService, 'detail', fake_detail)
         result = describe_image_attribute(self.context, 'ami-00000001',
                                           'launchPermission')
         self.assertEqual([{'group': 'all'}], result['launchPermission'])
@@ -1360,6 +1373,11 @@ class CloudTestCase(test.TestCase):
         def fake_show(meh, context, id):
             return copy.deepcopy(fake_metadata)
 
+        def fake_detail(self, context, **kwargs):
+            image = fake_show(None, context, None)
+            image['name'] = kwargs.get('filters', {}).get('name')
+            return [image]
+
         def fake_update(meh, context, image_id, metadata, data=None):
             self.assertEqual(metadata['properties']['kernel_id'],
                              fake_metadata['properties']['kernel_id'])
@@ -1371,7 +1389,7 @@ class CloudTestCase(test.TestCase):
             return image
 
         self.stubs.Set(fake._FakeImageService, 'show', fake_show)
-        self.stubs.Set(fake._FakeImageService, 'show_by_name', fake_show)
+        self.stubs.Set(fake._FakeImageService, 'detail', fake_detail)
         self.stubs.Set(fake._FakeImageService, 'update', fake_update)
         result = modify_image_attribute(self.context, 'ami-00000001',
                                           'launchPermission', 'add',
@@ -1473,7 +1491,7 @@ class CloudTestCase(test.TestCase):
         # invalid image
         self.stubs.UnsetAll()
 
-        def fake_detail_empty(self, context):
+        def fake_detail_empty(self, context, **kwargs):
             return []
 
         self.stubs.Set(fake._FakeImageService, 'detail', fake_detail_empty)
@@ -1727,7 +1745,11 @@ class CloudTestCase(test.TestCase):
                         'type': 'machine'},
                     'status': 'active'}
 
+        def fake_id_to_glance_id(context, id):
+            return 'cedef40a-ed67-4d10-800e-17455edce175'
+
         self.stubs.Set(fake._FakeImageService, 'show', fake_show_stat_active)
+        self.stubs.Set(ec2utils, 'id_to_glance_id', fake_id_to_glance_id)
 
         result = run_instances(self.context, **kwargs)
         self.assertEqual(len(result['instancesSet']), 1)
diff --git a/nova/tests/api/ec2/test_ec2_validate.py b/nova/tests/api/ec2/test_ec2_validate.py
index ad0de2ca0..cd213dd09 100644
--- a/nova/tests/api/ec2/test_ec2_validate.py
+++ b/nova/tests/api/ec2/test_ec2_validate.py
@@ -80,9 +80,14 @@ class EC2ValidateTestCase(test.TestCase):
                         'type': 'machine',
                         'image_state': 'available'}}
 
+        def fake_detail(self, context, **kwargs):
+            image = fake_show(self, context, None)
+            image['name'] = kwargs.get('name')
+            return [image]
+
         fake.stub_out_image_service(self.stubs)
         self.stubs.Set(fake._FakeImageService, 'show', fake_show)
-        self.stubs.Set(fake._FakeImageService, 'show_by_name', fake_show)
+        self.stubs.Set(fake._FakeImageService, 'detail', fake_detail)
 
         # NOTE(comstud): Make 'cast' behave like a 'call' which will
         # ensure that operations complete
diff --git a/nova/tests/image/fake.py b/nova/tests/image/fake.py
index 017611aa9..4a3e0b931 100644
--- a/nova/tests/image/fake.py
+++ b/nova/tests/image/fake.py
@@ -182,14 +182,6 @@ class _FakeImageService(object):
                  image_id, self.images)
         raise exception.ImageNotFound(image_id=image_id)
 
-    def show_by_name(self, context, name):
-        """Returns a dict containing image data for the given name."""
-        images = copy.deepcopy(self.images.values())
-        for image in images:
-            if name == image.get('name'):
-                return image
-        raise exception.ImageNotFound(image_id=name)
-
     def create(self, context, metadata, data=None):
         """Store the image data and return the new image id.
 
diff --git a/nova/tests/image/test_fake.py b/nova/tests/image/test_fake.py
index eb1136894..e1925557e 100644
--- a/nova/tests/image/test_fake.py
+++ b/nova/tests/image/test_fake.py
@@ -77,12 +77,6 @@ class FakeImageServiceTestCase(test.TestCase):
                           self.context,
                           'this image does not exist')
 
-    def test_show_by_name(self):
-        self.assertRaises(exception.ImageNotFound,
-                          self.image_service.show_by_name,
-                          self.context,
-                          'this image does not exist')
-
     def test_create_adds_id(self):
         index = self.image_service.index(self.context)
         image_count = len(index)
diff --git a/nova/tests/image/test_glance.py b/nova/tests/image/test_glance.py
index 81f1beb60..29c867667 100644
--- a/nova/tests/image/test_glance.py
+++ b/nova/tests/image/test_glance.py
@@ -190,12 +190,6 @@ class TestGlanceImageService(test.TestCase):
                           self.context,
                           'bad image id')
 
-    def test_create_and_show_non_existing_image_by_name(self):
-        self.assertRaises(exception.ImageNotFound,
-                          self.service.show_by_name,
-                          self.context,
-                          'bad image id')
-
     def test_index(self):
         fixture = self._make_fixture(name='test image')
         image_id = self.service.create(self.context, fixture)['id']