From d17a1ed78a259789ec7d52658057ca92a6935ead Mon Sep 17 00:00:00 2001 From: Flavio Percoco Date: Mon, 1 Dec 2014 21:52:22 +0100 Subject: [PATCH] Allow None values to be returned from the API Currently, Glance's API v2 doesn't return fields whose value is None. This, unfortunately, is wrong for a client perspective since it would create inconsistencies between calls and images due to the lack of fields in the response. The API should guarantee consistency in its replies and ensure all fields have a value, even if it's None. NOTE: This work is part of the migration to v2. It fixes inconsistencies in the API and improves the interaction between the client library and Glance. NOTE2: A follow-up patch will bump the minor API version, wait for it. ApiImpact DocImpact Closes-bug: #1398314 Change-Id: Ieaddd8a686cf7361f18cb1ee83b7887cdca22bd6 --- glance/schema.py | 2 +- glance/tests/functional/v2/test_images.py | 20 ++++++---- glance/tests/functional/v2/test_tasks.py | 1 + glance/tests/unit/v2/test_images_resource.py | 39 ++++++++++++++++++++ 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/glance/schema.py b/glance/schema.py index 4762c8c6f1..2dd74516ea 100644 --- a/glance/schema.py +++ b/glance/schema.py @@ -45,7 +45,7 @@ class Schema(object): def filter(self, obj): filtered = {} for key, value in six.iteritems(obj): - if self._filter_func(self.properties, key) and value is not None: + if self._filter_func(self.properties, key): filtered[key] = value return filtered diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py index 69840b4f38..c3173814a3 100644 --- a/glance/tests/functional/v2/test_images.py +++ b/glance/tests/functional/v2/test_images.py @@ -132,6 +132,9 @@ class TestImages(functional.FunctionalTest): u'disk_format', u'container_format', u'owner', + u'checksum', + u'size', + u'virtual_size', ]) self.assertEqual(checked_keys, set(image.keys())) expected_image = { @@ -192,6 +195,9 @@ class TestImages(functional.FunctionalTest): u'disk_format', u'container_format', u'owner', + u'checksum', + u'size', + u'virtual_size', ]) self.assertEqual(checked_keys, set(image.keys())) expected_image = { @@ -279,9 +285,9 @@ class TestImages(functional.FunctionalTest): self.assertEqual(200, response.status_code) image = jsonutils.loads(response.text) self.assertEqual(image_id, image['id']) - self.assertFalse('checksum' in image) - self.assertNotIn('size', image) - self.assertNotIn('virtual_size', image) + self.assertIsNone(image['checksum']) + self.assertIsNone(image['size']) + self.assertIsNone(image['virtual_size']) self.assertEqual('bar', image['foo']) self.assertFalse(image['protected']) self.assertEqual('kernel', image['type']) @@ -456,8 +462,8 @@ class TestImages(functional.FunctionalTest): response = requests.patch(path, headers=headers, data=data) self.assertEqual(200, response.status_code, response.text) image = jsonutils.loads(response.text) - self.assertNotIn('size', image) - self.assertNotIn('virtual_size', image) + self.assertIsNone(image['size']) + self.assertIsNone(image['virtual_size']) self.assertEqual('queued', image['status']) # Deletion should work. Deleting image-1 @@ -2111,8 +2117,8 @@ class TestImages(functional.FunctionalTest): image = jsonutils.loads(response.text) image_id = image['id'] self.assertEqual('queued', image['status']) - self.assertNotIn('size', image) - self.assertNotIn('virtual_size', image) + self.assertIsNone(image['size']) + self.assertIsNone(image['virtual_size']) file_path = os.path.join(self.test_dir, 'fake_image') with open(file_path, 'w') as fap: diff --git a/glance/tests/functional/v2/test_tasks.py b/glance/tests/functional/v2/test_tasks.py index 62be3ea762..ffc6a8600f 100644 --- a/glance/tests/functional/v2/test_tasks.py +++ b/glance/tests/functional/v2/test_tasks.py @@ -92,6 +92,7 @@ class TestTasks(functional.FunctionalTest): u'self', u'status', u'type', + u'result', u'updated_at']) self.assertEqual(checked_keys, set(task.keys())) expected_task = { diff --git a/glance/tests/unit/v2/test_images_resource.py b/glance/tests/unit/v2/test_images_resource.py index 956def748f..85a725c92d 100644 --- a/glance/tests/unit/v2/test_images_resource.py +++ b/glance/tests/unit/v2/test_images_resource.py @@ -2743,6 +2743,16 @@ class TestImagesSerializer(test_utils.BaseTestCase): 'self': '/v2/images/%s' % UUID2, 'file': '/v2/images/%s/file' % UUID2, 'schema': '/v2/schemas/image', + 'size': None, + 'name': None, + 'owner': None, + 'min_ram': None, + 'min_disk': None, + 'checksum': None, + 'disk_format': None, + 'virtual_size': None, + 'container_format': None, + }, ], 'first': '/v2/images', @@ -2847,6 +2857,15 @@ class TestImagesSerializer(test_utils.BaseTestCase): 'self': '/v2/images/%s' % UUID2, 'file': '/v2/images/%s/file' % UUID2, 'schema': '/v2/schemas/image', + 'size': None, + 'name': None, + 'owner': None, + 'min_ram': None, + 'min_disk': None, + 'checksum': None, + 'disk_format': None, + 'virtual_size': None, + 'container_format': None, } response = webob.Response() self.serializer.show(response, self.fixtures[1]) @@ -3113,6 +3132,10 @@ class TestImagesSerializerWithExtendedSchema(test_utils.BaseTestCase): 'self': '/v2/images/%s' % UUID2, 'file': '/v2/images/%s/file' % UUID2, 'schema': '/v2/schemas/image', + 'min_ram': None, + 'min_disk': None, + 'disk_format': None, + 'container_format': None, } response = webob.Response() self.serializer.show(response, self.fixture) @@ -3137,6 +3160,10 @@ class TestImagesSerializerWithExtendedSchema(test_utils.BaseTestCase): 'self': '/v2/images/%s' % UUID2, 'file': '/v2/images/%s/file' % UUID2, 'schema': '/v2/schemas/image', + 'min_ram': None, + 'min_disk': None, + 'disk_format': None, + 'container_format': None, } response = webob.Response() self.serializer.show(response, self.fixture) @@ -3173,6 +3200,10 @@ class TestImagesSerializerWithAdditionalProperties(test_utils.BaseTestCase): 'file': '/v2/images/%s/file' % UUID2, 'schema': '/v2/schemas/image', 'owner': '2c014f32-55eb-467d-8fcb-4bd706012f81', + 'min_ram': None, + 'min_disk': None, + 'disk_format': None, + 'container_format': None, } response = webob.Response() serializer.show(response, self.fixture) @@ -3203,6 +3234,10 @@ class TestImagesSerializerWithAdditionalProperties(test_utils.BaseTestCase): 'file': '/v2/images/%s/file' % UUID2, 'schema': '/v2/schemas/image', 'owner': '2c014f32-55eb-467d-8fcb-4bd706012f81', + 'min_ram': None, + 'min_disk': None, + 'disk_format': None, + 'container_format': None, } response = webob.Response() serializer.show(response, self.fixture) @@ -3227,6 +3262,10 @@ class TestImagesSerializerWithAdditionalProperties(test_utils.BaseTestCase): 'self': '/v2/images/%s' % UUID2, 'file': '/v2/images/%s/file' % UUID2, 'schema': '/v2/schemas/image', + 'min_ram': None, + 'min_disk': None, + 'disk_format': None, + 'container_format': None, } response = webob.Response() serializer.show(response, self.fixture)