Fixing API per spec, to get unit-tests to pass

5 files changed, 204 insertions, 66 deletions

diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py
index 98f0dd96b..7b3800429 100644
--- a/nova/api/openstack/images.py
+++ b/nova/api/openstack/images.py
@@ -15,10 +15,13 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import datetime
+
 from webob import exc
 
 from nova import compute
 from nova import flags
+from nova import log
 from nova import utils
 from nova import wsgi
 import nova.api.openstack
@@ -27,6 +30,8 @@ from nova.api.openstack import faults
 import nova.image.service
 
 
+LOG = log.getLogger('nova.api.openstack.images')
+
 FLAGS = flags.FLAGS
 
 
@@ -84,8 +89,6 @@ def _translate_status(item):
         # S3ImageService
         pass
 
-    return item
-
 
 def _filter_keys(item, keys):
     """
@@ -104,6 +107,89 @@ def _convert_image_id_to_hash(image):
         image['id'] = image_id
 
 
+def _translate_s3_like_images(image_metadata):
+    """Work-around for leaky S3ImageService abstraction"""
+    api_metadata = image_metadata.copy()
+    _convert_image_id_to_hash(api_metadata)
+    api_metadata = _translate_keys(api_metadata)
+    _translate_status(api_metadata)
+    return api_metadata
+
+
+def _translate_metadata_for_api_detail(image_metadata):
+    """Translate from ImageService to OpenStack API style attribute names
+
+    This involves 3 steps:
+
+        1. Translating required keys
+
+        2. Translating optional keys (ex. progress, serverId)
+
+        3. Formatting values according to API spec (for example dates must
+           look like "2010-08-10T12:00:00Z")
+    """
+    service_metadata = image_metadata.copy()
+    api_metadata = {}
+
+    # 1. Translate required keys
+    required_image_service2api = {
+        'id': 'id',
+        'name': 'name',
+        'updated_at': 'updated',
+        'created_at': 'created',
+        'status': 'status'}
+    for service_attr, api_attr in required_image_service2api.items():
+        api_metadata[api_attr] = service_metadata[service_attr]
+
+    # 2. Translate optional keys
+    optional_image_service2api = {'instance_id': 'serverId'}
+    for service_attr, api_attr in optional_image_service2api.items():
+        if service_attr in service_metadata:
+            api_metadata[api_attr] = service_metadata[service_attr]
+
+    # 2a. Progress special case
+    # TODO(sirp): ImageService doesn't have a notion of progress yet, so for
+    # now just fake it
+    if service_metadata['status'] == 'saving':
+        api_metadata['progress'] = 0
+
+    # 3. Format values
+
+    # 3a. Format Image Status (API requires uppercase)
+    status_service2api = {'queued': 'QUEUED',
+                          'preparing': 'PREPARING',
+                          'saving': 'SAVING',
+                          'active': 'ACTIVE',
+                          'killed': 'FAILED'}
+    api_metadata['status'] = status_service2api[api_metadata['status']]
+
+    # 3b. Format timestamps
+    def _format_timestamp(dt_str):
+        """Return a timestamp formatted for OpenStack API
+
+        NOTE(sirp):
+
+        ImageService (specifically GlanceImageService) is currently
+        returning timestamps as strings. This should probably be datetime
+        objects. In the mean time, we work around this by using strptime() to
+        create datetime objects.
+        """
+        if dt_str is None:
+            return None
+
+        service_timestamp_fmt = "%Y-%m-%dT%H:%M:%S"
+        api_timestamp_fmt = "%Y-%m-%dT%H:%M:%SZ"
+        dt = datetime.datetime.strptime(dt_str, service_timestamp_fmt)
+        return dt.strftime(api_timestamp_fmt)
+
+    for ts_attr in ('created', 'updated'):
+        if ts_attr in api_metadata:
+            formatted_timestamp = _format_timestamp(api_metadata[ts_attr])
+            api_metadata[ts_attr] = formatted_timestamp
+
+    return api_metadata
+
+
 class Controller(wsgi.Controller):
 
     _serialization_metadata = {
@@ -125,16 +211,28 @@ class Controller(wsgi.Controller):
     def detail(self, req):
         """Return all public images in detail"""
         try:
-            items = self._service.detail(req.environ['nova.context'])
+            service_image_metas = self._service.detail(
+                req.environ['nova.context'])
         except NotImplementedError:
-            items = self._service.index(req.environ['nova.context'])
-        for image in items:
-            _convert_image_id_to_hash(image)
+            service_image_metas = self._service.index(
+                req.environ['nova.context'])
 
-        items = common.limited(items, req)
-        items = [_translate_keys(item) for item in items]
-        items = [_translate_status(item) for item in items]
-        return dict(images=items)
+        service_image_metas = common.limited(service_image_metas, req)
+
+        # FIXME(sirp): The S3ImageService appears to be leaking implementation
+        # details, including its internal attribute names, and internal
+        # `status` values. Working around it for now.
+        s3_like_image = (service_image_metas and
+                         ('imageId' in service_image_metas[0]))
+        if s3_like_image:
+            translate = _translate_s3_like_images
+        else:
+            translate = _translate_metadata_for_api_detail
+
+        api_image_metas = [translate(service_image_meta)
+                           for service_image_meta in service_image_metas]
+
+        return dict(images=api_image_metas)
 
     def show(self, req, id):
         """Return data about the given image id"""
diff --git a/nova/image/glance.py b/nova/image/glance.py
index 8e6ecbc43..63a3faa0f 100644
--- a/nova/image/glance.py
+++ b/nova/image/glance.py
@@ -22,12 +22,12 @@ from glance.common import exception as glance_exception
 
 from nova import exception
 from nova import flags
-from nova import log as logging
+from nova import log
 from nova import utils
 from nova.image import service
 
 
-LOG = logging.getLogger('nova.image.glance')
+LOG = log.getLogger('nova.image.glance')
 
 FLAGS = flags.FLAGS
 
@@ -51,7 +51,10 @@ class GlanceImageService(service.BaseImageService):
         """
         Calls out to Glance for a list of detailed image information
         """
-        return self.client.get_images_detailed()
+        image_metas = self.client.get_images_detailed()
+        return image_metas
+        return [self._depropertify_metadata_from_glance(image_meta)
+                for image_meta in image_metas]
 
     def show(self, context, image_id):
         """
@@ -173,9 +176,10 @@ class GlanceImageService(service.BaseImageService):
         """Return a metadata dict suitable for returning from ImageService
         """
         new_metadata = metadata.copy()
-        properties = new_metadata.pop('properties')
-        for property_ in cls.IMAGE_PROPERTIES:
-            if property_ in properties and property_ not in new_metadata:
-                value = properties[property_]
-                new_metadata[property_] = value
+        if 'properties' in new_metadata:
+            properties = new_metadata.pop('properties')
+            for property_ in cls.IMAGE_PROPERTIES:
+                if property_ in properties and property_ not in new_metadata:
+                    value = properties[property_]
+                    new_metadata[property_] = value
         return new_metadata
diff --git a/nova/test.py b/nova/test.py
index c41551bf3..e0fef6101 100644
--- a/nova/test.py
+++ b/nova/test.py
@@ -157,6 +157,12 @@ class TestCase(unittest.TestCase):
 
         This is a 'deep' match in the sense that it handles nested
         dictionaries appropriately.
+
+        NOTE:
+
+            If you don't care (or don't know) a given value, you can specify
+            the string DONTCARE as the value. This will cause that dict-item
+            to be skipped.
         """
         def raise_assertion(msg):
             d1str = str(d1)
@@ -178,6 +184,26 @@ class TestCase(unittest.TestCase):
             d2value = d2[key]
             if hasattr(d1value, 'keys') and hasattr(d2value, 'keys'):
                 self.assertDictMatch(d1value, d2value)
+            elif 'DONTCARE' in (d1value, d2value):
+                continue
             elif d1value != d2value:
                 raise_assertion("d1['%(key)s']=%(d1value)s != "
                                 "d2['%(key)s']=%(d2value)s" % locals())
+
+    def assertDictListMatch(self, L1, L2):
+        """Assert a list of dicts are equivalent"""
+        def raise_assertion(msg):
+            L1str = str(L1)
+            L2str = str(L2)
+            base_msg = ("List of dictionaries do not match: %(msg)s "
+                        "L1: %(L1str)s L2: %(L2str)s" % locals())
+            raise AssertionError(base_msg)
+
+        L1count = len(L1)
+        L2count = len(L2)
+        if L1count != L2count:
+            raise_assertion("Length mismatch: len(L1)=%(L1count)d != "
+                            "len(L2)=%(L2count)d" % locals())
+
+        for d1, d2 in zip(L1, L2):
+            self.assertDictMatch(d1, d2)
diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py
index 1c7d926ba..ef38b93ca 100644
--- a/nova/tests/api/openstack/fakes.py
+++ b/nova/tests/api/openstack/fakes.py
@@ -138,10 +138,12 @@ def stub_out_glance_add_image(stubs, sent_to_glance):
     in place.
     """
     orig_add_image = glance_client.Client.add_image
+
     def fake_add_image(context, metadata, data=None):
         sent_to_glance['metadata'] = metadata
         sent_to_glance['data'] = data
         return orig_add_image(metadata, data)
+
     stubs.Set(glance_client.Client, 'add_image', fake_add_image)
 
 
@@ -166,10 +168,13 @@ def stub_out_glance(stubs, initial_fixtures=None):
             raise glance_exc.NotFound
 
         def fake_add_image(self, image_meta, data=None):
-            if 'id' not in image_meta:
-                image_id = ''.join(random.choice(string.letters)
-                                   for _ in range(20))
-                image_meta['id'] = image_id
+            if 'id' in image_meta:
+                raise Exception(
+                    _("Cannot set id attribute for Glance image: %s")
+                    % image_meta)
+            image_id = ''.join(random.choice(string.letters)
+                               for _ in range(20))
+            image_meta['id'] = image_id
             self.fixtures.append(image_meta)
             return image_meta
 
diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py
index 0e6d538f9..9b4b5832a 100644
--- a/nova/tests/api/openstack/test_images.py
+++ b/nova/tests/api/openstack/test_images.py
@@ -156,7 +156,7 @@ class LocalImageServiceTest(test.TestCase,
 class GlanceImageServiceTest(test.TestCase,
                              BaseImageServiceTests):
 
-    """Tests the local image service"""
+    """Tests the Glance image service"""
 
     def setUp(self):
         super(GlanceImageServiceTest, self).setUp()
@@ -183,20 +183,23 @@ class GlanceImageServiceTest(test.TestCase,
         first-class attribrutes, but that they are passed to Glance as image
         properties.
         """
-        fixture = {'id': 123, 'instance_id': 42, 'name': 'test image'}
+        fixture = {'instance_id': 42, 'name': 'test image'}
         image_id = self.service.create(self.context, fixture)['id']
 
-        expected = {'id': 123,
+        expected = {'id': image_id,
                     'name': 'test image',
                     'properties': {'instance_id': 42}}
         self.assertDictMatch(self.sent_to_glance['metadata'], expected)
 
         # The ImageService shouldn't leak the fact that the instance_id
         # happens to be stored as a property in Glance
-        expected = {'id': 123, 'instance_id': 42, 'name': 'test image'}
+        expected = {'id': image_id, 'instance_id': 42, 'name': 'test image'}
         image_meta = self.service.show(self.context, image_id)
         self.assertDictMatch(image_meta, expected)
 
+        #image_metas = self.service.detail(self.context)
+        #self.assertDictMatch(image_metas[0], expected)
+
     def test_create_propertified_images_without_instance_id(self):
         """
         Some attributes are passed to Glance as image-properties (ex.
@@ -206,10 +209,10 @@ class GlanceImageServiceTest(test.TestCase,
         first-class attribrutes, but that they are passed to Glance as image
         properties.
         """
-        fixture = {'id': 123, 'name': 'test image'}
+        fixture = {'name': 'test image'}
         image_id = self.service.create(self.context, fixture)['id']
 
-        expected = {'id': 123, 'name': 'test image', 'properties': {}}
+        expected = {'id': image_id, 'name': 'test image', 'properties': {}}
         self.assertDictMatch(self.sent_to_glance['metadata'], expected)
 
 
@@ -217,29 +220,39 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):
 
     """Test of the OpenStack API /images application controller"""
 
-    # Registered images at start of each test.
+    # FIXME(sirp): The ImageService and API use two different formats for
+    # timestamps. Ultimately, the ImageService should probably use datetime
+    # objects
+    NOW_SERVICE_STR = "2010-10-11T10:30:22"
+    NOW_API_STR = "2010-10-11T10:30:22Z"
 
     IMAGE_FIXTURES = [
-        {'id': '23g2ogk23k4hhkk4k42l',
-         'imageId': '23g2ogk23k4hhkk4k42l',
+        {'id': 123,
          'name': 'public image #1',
-         'created_at': str(datetime.datetime.utcnow()),
-         'updated_at': str(datetime.datetime.utcnow()),
+         'created_at': NOW_SERVICE_STR,
+         'updated_at': NOW_SERVICE_STR,
          'deleted_at': None,
          'deleted': False,
          'is_public': True,
-         'status': 'available',
-         'image_type': 'kernel'},
-        {'id': 'slkduhfas73kkaskgdas',
-         'imageId': 'slkduhfas73kkaskgdas',
+         'status': 'saving'},
+        {'id': 124,
          'name': 'public image #2',
-         'created_at': str(datetime.datetime.utcnow()),
-         'updated_at': str(datetime.datetime.utcnow()),
+         'created_at': NOW_SERVICE_STR,
+         'updated_at': NOW_SERVICE_STR,
+         'deleted_at': None,
+         'deleted': False,
+         'is_public': True,
+         'status': 'active',
+         'instance_id': 42},
+        {'id': 125,
+         'name': 'public image #3',
+         'created_at': NOW_SERVICE_STR,
+         'updated_at': NOW_SERVICE_STR,
          'deleted_at': None,
          'deleted': False,
          'is_public': True,
-         'status': 'available',
-         'image_type': 'ramdisk'}]
+         'status': 'killed',
+         'instance_id': 42}]
 
     def setUp(self):
         super(ImageControllerWithGlanceServiceTest, self).setUp()
@@ -262,34 +275,26 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):
     def test_get_image_index(self):
         req = webob.Request.blank('/v1.0/images')
         res = req.get_response(fakes.wsgi_app())
-        res_dict = json.loads(res.body)
+        image_metas = json.loads(res.body)['images']
 
-        fixture_index = [dict(id=f['id'], name=f['name']) for f
-                         in self.IMAGE_FIXTURES]
+        expected = [{'id': 123, 'name': 'public image #1'},
+                    {'id': 124, 'name': 'public image #2'},
+                    {'id': 125, 'name': 'public image #3'}]
 
-        for image in res_dict['images']:
-            self.assertEquals(1, fixture_index.count(image),
-                              "image %s not in fixture index!" % str(image))
+        self.assertDictListMatch(image_metas, expected)
 
     def test_get_image_details(self):
         req = webob.Request.blank('/v1.0/images/detail')
         res = req.get_response(fakes.wsgi_app())
-        res_dict = json.loads(res.body)
-
-        def _is_equivalent_subset(x, y):
-            if set(x) <= set(y):
-                for k, v in x.iteritems():
-                    if x[k] != y[k]:
-                        if x[k] == 'active' and y[k] == 'available':
-                            continue
-                        return False
-                return True
-            return False
-
-        for image in res_dict['images']:
-            for image_fixture in self.IMAGE_FIXTURES:
-                if _is_equivalent_subset(image, image_fixture):
-                    break
-            else:
-                self.assertEquals(1, 2, "image %s not in fixtures!" %
-                                                            str(image))
+        image_metas = json.loads(res.body)['images']
+
+        expected = [
+            {'id': 123, 'name': 'public image #1', 'updated': self.NOW_API_STR,
+             'created': self.NOW_API_STR, 'status': 'SAVING', 'progress': 0},
+            {'id': 124, 'name': 'public image #2', 'updated': self.NOW_API_STR,
+             'created': self.NOW_API_STR, 'status': 'ACTIVE', 'serverId': 42},
+            {'id': 125, 'name': 'public image #3', 'updated': self.NOW_API_STR,
+             'created': self.NOW_API_STR, 'status': 'FAILED', 'serverId': 42},
+        ]
+
+        self.assertDictListMatch(image_metas, expected)