Use 0-based indices for location entries

Glance server currently uses 1-based indices for indexing location
entries. This violates the JSON-pointer RFC (RFC 6901) which states that
indicies should be 0-based. Furthermore, since this deviation is not
mentioned in the docs any application directly calling the REST APIs
would trip over it.

This commit makes Glance use zero-based indexing for location entries.

Note that the "show_multiple_locations" config option should be set to
True to expose location related REST operations.

DocImpact: Since this change modifies user-visible Glance behaviour it
should be explicitly documented because the previous behaviour (1-based
indexing), though not explicitly documented, was exposed through the
REST APIs.

Change-Id: I6326455874c381fcb0d8babf9cc4fa8311e219d2
Closes-bug: #1282437
This commit is contained in:
David Koo 2014-02-21 10:56:27 +08:00
parent 8762a7b072
commit eafa186b49
2 changed files with 10 additions and 11 deletions

View File

@ -204,8 +204,7 @@ class ImagesController(object):
return None
pos = max_pos if allow_max else max_pos - 1
if path_pos.isdigit():
# NOTE(zhiyan): locations index from '1' by client perspective
pos = int(path_pos) - 1
pos = int(path_pos)
elif path_pos != '-':
return None
if (not allow_max) and (pos not in range(max_pos)):

View File

@ -1353,7 +1353,7 @@ class TestImagesController(base.IsolatedUnitTest):
def test_update_add_locations_insertion(self):
new_location = {'url': '%s/fake_location' % BASE_URI, 'metadata': {}}
request = unit_test_utils.get_fake_request()
changes = [{'op': 'add', 'path': ['locations', '1'],
changes = [{'op': 'add', 'path': ['locations', '0'],
'value': new_location}]
output = self.controller.update(request, UUID1, changes)
self.assertEqual(output.image_id, UUID1)
@ -1454,7 +1454,7 @@ class TestImagesController(base.IsolatedUnitTest):
# We must remove two properties to avoid being
# over the limit of 1 property
changes = [
{'op': 'remove', 'path': ['locations', '1']},
{'op': 'remove', 'path': ['locations', '0']},
{'op': 'add', 'path': ['locations', '-'],
'value': {'url': '%s/fake_location_3' % BASE_URI,
'metadata': {}}},
@ -1499,8 +1499,8 @@ class TestImagesController(base.IsolatedUnitTest):
# We must remove two locations to avoid being over
# the limit of 1 location
changes = [
{'op': 'remove', 'path': ['locations', '1']},
{'op': 'remove', 'path': ['locations', '1']},
{'op': 'remove', 'path': ['locations', '0']},
{'op': 'remove', 'path': ['locations', '0']},
]
output = self.controller.update(request, UUID1, changes)
self.assertEqual(output.image_id, UUID1)
@ -1532,8 +1532,8 @@ class TestImagesController(base.IsolatedUnitTest):
# We must remove two properties to avoid being
# over the limit of 1 property
changes = [
{'op': 'remove', 'path': ['locations', '1']},
{'op': 'remove', 'path': ['locations', '1']},
{'op': 'remove', 'path': ['locations', '0']},
{'op': 'remove', 'path': ['locations', '0']},
{'op': 'add', 'path': ['locations', '-'],
'value': {'url': '%s/fake_location_3' % BASE_URI,
'metadata': {}}},
@ -1582,7 +1582,7 @@ class TestImagesController(base.IsolatedUnitTest):
unit_test_utils.fake_get_size_from_backend)
request = unit_test_utils.get_fake_request()
changes = [{'op': 'remove', 'path': ['locations', '1']}]
changes = [{'op': 'remove', 'path': ['locations', '0']}]
output = self.controller.update(request, UUID1, changes)
self.assertEqual(output.image_id, UUID1)
self.assertEqual(len(output.locations), 0)
@ -1603,7 +1603,7 @@ class TestImagesController(base.IsolatedUnitTest):
changes = [{'op': 'remove', 'path': ['locations', None]}]
self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
request, UUID1, changes)
changes = [{'op': 'remove', 'path': ['locations', '0']}]
changes = [{'op': 'remove', 'path': ['locations', '-1']}]
self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
request, UUID1, changes)
changes = [{'op': 'remove', 'path': ['locations', '99']}]
@ -1621,7 +1621,7 @@ class TestImagesController(base.IsolatedUnitTest):
fake_delete_image_from_backend)
request = unit_test_utils.get_fake_request()
changes = [{'op': 'remove', 'path': ['locations', '1']}]
changes = [{'op': 'remove', 'path': ['locations', '0']}]
self.assertRaises(webob.exc.HTTPInternalServerError,
self.controller.update, request, UUID1, changes)