Filter out None values from v2 API image entity

Prevent core image attributes with None values from being returned
to v2 API clients. Most attribute schemas are of type 'string' and
returning None is invalid.

Fixes bug 1034790

Change-Id: I2885cabf14307e46ff711bdabfcd3ff3253e6628

glance/api/v2/images.py

@@ -261,24 +261,30 @@ class ResponseSerializer(wsgi.JSONResponseSerializer):
         ]
 
     def _format_image(self, image):
-        _image = image['properties']
-        for key in ['id', 'name', 'created_at', 'updated_at', 'tags', 'size',
-                    'checksum', 'status']:
-            _image[key] = image[key]
-        if CONF.show_image_direct_url and image['location']:
-            _image['direct_url'] = image['location']
+        #NOTE(bcwaldon): merge the contained properties dict with the
+        # top-level image object
+        image_view = image['properties']
+        attributes = ['id', 'name', 'disk_format', 'container_format',
+                      'size', 'status', 'checksum', 'tags',
+                      'created_at', 'updated_at']
+        for key in attributes:
+            image_view[key] = image[key]
 
-        for key in ['container_format', 'disk_format']:
-            if image.get(key):
-                _image[key] = image[key]
+        location = image['location']
+        if CONF.show_image_direct_url and location is not None:
+            image_view['direct_url'] = location
 
-        _image['visibility'] = 'public' if image['is_public'] else 'private'
-        _image = self.schema.filter(_image)
-        _image['self'] = self._get_image_href(image)
-        _image['file'] = self._get_image_href(image, 'file')
-        _image['schema'] = '/v2/schemas/image'
-        self._serialize_datetimes(_image)
-        return _image
+        visibility = 'public' if image['is_public'] else 'private'
+        image_view['visibility'] = visibility
+
+        image_view['self'] = self._get_image_href(image)
+        image_view['file'] = self._get_image_href(image, 'file')
+        image_view['schema'] = '/v2/schemas/image'
+
+        self._serialize_datetimes(image_view)
+        image_view = self.schema.filter(image_view)
+
+        return image_view
 
     @staticmethod
     def _serialize_datetimes(image):

glance/schema.py

@@ -36,10 +36,14 @@ class Schema(object):
     def filter(self, obj):
         filtered = {}
         for key, value in obj.iteritems():
-            if key in self.properties:
+            if self._filter_func(self.properties, key) and value is not None:
                 filtered[key] = value
         return filtered
 
+    @staticmethod
+    def _filter_func(properties, key):
+        return key in properties
+
     def merge_properties(self, properties):
         # Ensure custom props aren't attempting to override base props
         original_keys = set(self.properties.keys())
@@ -67,8 +71,9 @@ class Schema(object):
 
 
 class PermissiveSchema(Schema):
-    def filter(self, obj):
-        return obj
+    @staticmethod
+    def _filter_func(properties, key):
+        return True
 
     def raw(self):
         raw = super(PermissiveSchema, self).raw()

glance/tests/functional/v2/test_images.py

@@ -85,8 +85,8 @@ class TestImages(functional.FunctionalTest):
         self.assertEqual(200, response.status_code)
         image = json.loads(response.text)
         self.assertEqual(image_id, image['id'])
-        self.assertEqual(None, image['checksum'])
-        self.assertEqual(None, image['size'])
+        self.assertFalse('checksum' in image)
+        self.assertFalse('size' in image)
         self.assertEqual('bar', image['foo'])
         self.assertTrue(image['created_at'])
         self.assertTrue(image['updated_at'])

glance/tests/unit/v2/test_images_resource.py

@@ -48,7 +48,7 @@ TENANT4 = 'c6c87f25-8a94-47ed-8c83-053c25f42df4'
 def _fixture(id, **kwargs):
     obj = {
         'id': id,
-        'name': 'image-1',
+        'name': None,
         'is_public': False,
         'properties': {},
         'checksum': None,
@@ -57,11 +57,10 @@ def _fixture(id, **kwargs):
         'tags': [],
         'size': None,
         'location': None,
-        'min_ram': None,
-        'min_disk': None,
         'protected': False,
         'disk_format': None,
         'container_format': None,
+        'deleted': False,
     }
     obj.update(kwargs)
     return obj
@@ -635,12 +634,15 @@ class TestImagesSerializer(test_utils.BaseTestCase):
         super(TestImagesSerializer, self).setUp()
         self.serializer = glance.api.v2.images.ResponseSerializer()
         self.fixtures = [
+            #NOTE(bcwaldon): This first fixture has every property defined
             _fixture(UUID1, name='image-1', size=1024, tags=['one', 'two'],
                     created_at=DATETIME, updated_at=DATETIME, owner=TENANT1,
-                    is_public=True, container_format='ami', disk_format='ami'),
-            _fixture(UUID2, name='image-2',
-                    created_at=DATETIME, updated_at=DATETIME,
+                    is_public=True, container_format='ami', disk_format='ami',
                     checksum='ca425b88f047ce8ec45ee90e813ada91'),
+
+            #NOTE(bcwaldon): This second fixture depends on default behavior
+            # and sets most values to None
+            _fixture(UUID2, created_at=DATETIME, updated_at=DATETIME),
         ]
 
     def test_index(self):
@@ -651,30 +653,27 @@ class TestImagesSerializer(test_utils.BaseTestCase):
                     'name': 'image-1',
                     'status': 'queued',
                     'visibility': 'public',
-                    'created_at': ISOTIME,
-                    'updated_at': ISOTIME,
                     'tags': ['one', 'two'],
                     'size': 1024,
+                    'checksum': 'ca425b88f047ce8ec45ee90e813ada91',
                     'container_format': 'ami',
                     'disk_format': 'ami',
+                    'created_at': ISOTIME,
+                    'updated_at': ISOTIME,
                     'self': '/v2/images/%s' % UUID1,
                     'file': '/v2/images/%s/file' % UUID1,
                     'schema': '/v2/schemas/image',
-                    'checksum': None,
                 },
                 {
                     'id': UUID2,
-                    'name': 'image-2',
                     'status': 'queued',
                     'visibility': 'private',
-                    'checksum': 'ca425b88f047ce8ec45ee90e813ada91',
+                    'tags': [],
                     'created_at': ISOTIME,
                     'updated_at': ISOTIME,
-                    'tags': [],
                     'self': '/v2/images/%s' % UUID2,
                     'file': '/v2/images/%s/file' % UUID2,
                     'schema': '/v2/schemas/image',
-                    'size': None,
                 },
             ],
             'first': '/v2/images',
@@ -707,44 +706,60 @@ class TestImagesSerializer(test_utils.BaseTestCase):
         expect_next = '/v2/images?sort_key=id&sort_dir=asc&limit=10&marker=%s'
         self.assertEqual(expect_next % UUID2, output['next'])
 
-    def test_show(self):
+    def test_show_full_fixture(self):
         expected = {
             'id': UUID1,
             'name': 'image-1',
             'status': 'queued',
             'visibility': 'public',
-            'checksum': None,
-            'created_at': ISOTIME,
-            'updated_at': ISOTIME,
             'tags': ['one', 'two'],
             'size': 1024,
+            'checksum': 'ca425b88f047ce8ec45ee90e813ada91',
+            'container_format': 'ami',
+            'disk_format': 'ami',
+            'created_at': ISOTIME,
+            'updated_at': ISOTIME,
             'self': '/v2/images/%s' % UUID1,
             'file': '/v2/images/%s/file' % UUID1,
             'schema': '/v2/schemas/image',
-            'container_format': 'ami',
-            'disk_format': 'ami',
         }
         response = webob.Response()
         self.serializer.show(response, self.fixtures[0])
         self.assertEqual(expected, json.loads(response.body))
         self.assertEqual('application/json', response.content_type)
 
+    def test_show_minimal_fixture(self):
+        expected = {
+            'id': UUID2,
+            'status': 'queued',
+            'visibility': 'private',
+            'tags': [],
+            'created_at': ISOTIME,
+            'updated_at': ISOTIME,
+            'self': '/v2/images/%s' % UUID2,
+            'file': '/v2/images/%s/file' % UUID2,
+            'schema': '/v2/schemas/image',
+        }
+        response = webob.Response()
+        self.serializer.show(response, self.fixtures[1])
+        self.assertEqual(expected, json.loads(response.body))
+
     def test_create(self):
         expected = {
             'id': UUID1,
             'name': 'image-1',
             'status': 'queued',
             'visibility': 'public',
-            'checksum': None,
-            'created_at': ISOTIME,
-            'updated_at': ISOTIME,
             'tags': ['one', 'two'],
             'size': 1024,
+            'checksum': 'ca425b88f047ce8ec45ee90e813ada91',
+            'container_format': 'ami',
+            'disk_format': 'ami',
+            'created_at': ISOTIME,
+            'updated_at': ISOTIME,
             'self': '/v2/images/%s' % UUID1,
             'file': '/v2/images/%s/file' % UUID1,
             'schema': '/v2/schemas/image',
-            'container_format': 'ami',
-            'disk_format': 'ami',
         }
         response = webob.Response()
         self.serializer.create(response, self.fixtures[0])
@@ -758,16 +773,16 @@ class TestImagesSerializer(test_utils.BaseTestCase):
             'name': 'image-1',
             'status': 'queued',
             'visibility': 'public',
-            'checksum': None,
-            'created_at': ISOTIME,
-            'updated_at': ISOTIME,
             'tags': ['one', 'two'],
             'size': 1024,
+            'checksum': 'ca425b88f047ce8ec45ee90e813ada91',
+            'container_format': 'ami',
+            'disk_format': 'ami',
+            'created_at': ISOTIME,
+            'updated_at': ISOTIME,
             'self': '/v2/images/%s' % UUID1,
             'file': '/v2/images/%s/file' % UUID1,
             'schema': '/v2/schemas/image',
-            'container_format': 'ami',
-            'disk_format': 'ami',
         }
         response = webob.Response()
         self.serializer.update(response, self.fixtures[0])
@@ -802,11 +817,11 @@ class TestImagesSerializerWithExtendedSchema(test_utils.BaseTestCase):
             'status': 'queued',
             'visibility': 'private',
             'checksum': 'ca425b88f047ce8ec45ee90e813ada91',
-            'created_at': ISOTIME,
-            'updated_at': ISOTIME,
             'tags': [],
             'size': 1024,
             'color': 'green',
+            'created_at': ISOTIME,
+            'updated_at': ISOTIME,
             'self': '/v2/images/%s' % UUID2,
             'file': '/v2/images/%s/file' % UUID2,
             'schema': '/v2/schemas/image',
@@ -823,11 +838,11 @@ class TestImagesSerializerWithExtendedSchema(test_utils.BaseTestCase):
             'status': 'queued',
             'visibility': 'private',
             'checksum': 'ca425b88f047ce8ec45ee90e813ada91',
-            'created_at': ISOTIME,
-            'updated_at': ISOTIME,
             'tags': [],
             'size': 1024,
             'color': 'invalid',
+            'created_at': ISOTIME,
+            'updated_at': ISOTIME,
             'self': '/v2/images/%s' % UUID2,
             'file': '/v2/images/%s/file' % UUID2,
             'schema': '/v2/schemas/image',
@@ -855,11 +870,11 @@ class TestImagesSerializerWithAdditionalProperties(test_utils.BaseTestCase):
             'status': 'queued',
             'visibility': 'private',
             'checksum': 'ca425b88f047ce8ec45ee90e813ada91',
-            'created_at': ISOTIME,
-            'updated_at': ISOTIME,
             'marx': 'groucho',
             'tags': [],
             'size': 1024,
+            'created_at': ISOTIME,
+            'updated_at': ISOTIME,
             'self': '/v2/images/%s' % UUID2,
             'file': '/v2/images/%s/file' % UUID2,
             'schema': '/v2/schemas/image',
@@ -880,11 +895,11 @@ class TestImagesSerializerWithAdditionalProperties(test_utils.BaseTestCase):
             'status': 'queued',
             'visibility': 'private',
             'checksum': 'ca425b88f047ce8ec45ee90e813ada91',
-            'created_at': ISOTIME,
-            'updated_at': ISOTIME,
             'marx': 123,
             'tags': [],
             'size': 1024,
+            'created_at': ISOTIME,
+            'updated_at': ISOTIME,
             'self': '/v2/images/%s' % UUID2,
             'file': '/v2/images/%s/file' % UUID2,
             'schema': '/v2/schemas/image',
@@ -902,10 +917,10 @@ class TestImagesSerializerWithAdditionalProperties(test_utils.BaseTestCase):
             'status': 'queued',
             'visibility': 'private',
             'checksum': 'ca425b88f047ce8ec45ee90e813ada91',
-            'created_at': ISOTIME,
-            'updated_at': ISOTIME,
             'tags': [],
             'size': 1024,
+            'created_at': ISOTIME,
+            'updated_at': ISOTIME,
             'self': '/v2/images/%s' % UUID2,
             'file': '/v2/images/%s/file' % UUID2,
             'schema': '/v2/schemas/image',