From 9434c57df416c4df9aff28eb269a30583bba5c7a Mon Sep 17 00:00:00 2001 From: Flavio Percoco Date: Tue, 20 May 2014 16:09:14 +0200 Subject: [PATCH] Revert "Disable Metadata write operations on v1.1" This reverts commit 23fa8e80c7a24abdfffadded1bc6eda0c5c071d0. Based on the feedback we got from the summit, it seems that removing queues and their metadata is not a good idea. I'd like to revert this patch until we either choose to go 'The stubborn way' or accept this actually makes sense. Conflicts: marconi/queues/transport/wsgi/v1_1/metadata.py Change-Id: Ic407e104a4108acc262a5a35c317ef6a9fc529a4 --- .../queues/transport/wsgi/v1_1/metadata.py | 45 ++++++++++---- .../transport/wsgi/test_queue_lifecycle.py | 57 +++++++----------- tests/unit/queues/transport/wsgi/test_v1_1.py | 60 ------------------- 3 files changed, 53 insertions(+), 109 deletions(-) diff --git a/marconi/queues/transport/wsgi/v1_1/metadata.py b/marconi/queues/transport/wsgi/v1_1/metadata.py index cc617f9d6..608edf1b3 100644 --- a/marconi/queues/transport/wsgi/v1_1/metadata.py +++ b/marconi/queues/transport/wsgi/v1_1/metadata.py @@ -14,12 +14,15 @@ # limitations under the License. import falcon +import six from marconi.openstack.common.gettextutils import _ import marconi.openstack.common.log as logging from marconi.queues.storage import errors as storage_errors from marconi.queues.transport import utils +from marconi.queues.transport import validation from marconi.queues.transport.wsgi import errors as wsgi_errors +from marconi.queues.transport.wsgi import utils as wsgi_utils LOG = logging.getLogger(__name__) @@ -60,16 +63,34 @@ class Resource(object): u'project: %(project)s', {'queue': queue_name, 'project': project_id}) + try: + # Place JSON size restriction before parsing + self._validate.queue_metadata_length(req.content_length) + except validation.ValidationFailed as ex: + LOG.debug(ex) + raise wsgi_errors.HTTPBadRequestAPI(six.text_type(ex)) + + # Deserialize queue metadata + metadata, = wsgi_utils.filter_stream(req.stream, + req.content_length, + spec=None) + + try: + self.queue_ctrl.set_metadata(queue_name, + metadata=metadata, + project=project_id) + + except validation.ValidationFailed as ex: + LOG.debug(ex) + raise wsgi_errors.HTTPBadRequestAPI(six.text_type(ex)) + + except storage_errors.QueueDoesNotExist: + raise falcon.HTTPNotFound() + + except Exception as ex: + LOG.exception(ex) + description = _(u'Metadata could not be updated.') + raise wsgi_errors.HTTPServiceUnavailable(description) + + resp.status = falcon.HTTP_204 resp.location = req.path - - description = ("Queue metadata has been deprecated. " - "It will be removed completely in the next " - "version of the API.") - - # TODO(kgriffs): There is a falcon bug that causes - # HTTPMethodNotAllowed to always ignore the kwargs, such - # as "description". Once that is fixed, we can use - # that class instead of the generic error. - raise falcon.HTTPError(falcon.HTTP_405, 'Method not allowed', - headers={'Allow': 'GET'}, - description=description) diff --git a/marconi/tests/queues/transport/wsgi/test_queue_lifecycle.py b/marconi/tests/queues/transport/wsgi/test_queue_lifecycle.py index a2d8855d8..0c947d5b1 100644 --- a/marconi/tests/queues/transport/wsgi/test_queue_lifecycle.py +++ b/marconi/tests/queues/transport/wsgi/test_queue_lifecycle.py @@ -27,15 +27,6 @@ class QueueLifecycleBaseTest(base.TestBase): config_file = None - # NOTE(flaper87): This is temporary. Ideally, each version - # of the API should have its own lifecycle tests. The v1.1 - # of our API removes the support for the API. Although most of - # the test methods were overwritten in the test definition, there - # are some that need to be disabled in-lieu for other tests to be - # excuted. Also, ddt plays dirty and makes it impossible to override - # a test case. - metadata_support = True - def setUp(self): super(QueueLifecycleBaseTest, self).setUp() @@ -82,18 +73,17 @@ class QueueLifecycleBaseTest(base.TestBase): self.assertEqual(self.srmock.status, falcon.HTTP_204) # Add metadata - if self.metadata_support: - doc = '{"messages": {"ttl": 600}}' - self.simulate_put(gumshoe_queue_path_metadata, - project_id, body=doc) - self.assertEqual(self.srmock.status, falcon.HTTP_204) + doc = '{"messages": {"ttl": 600}}' + self.simulate_put(gumshoe_queue_path_metadata, + project_id, body=doc) + self.assertEqual(self.srmock.status, falcon.HTTP_204) - # Fetch metadata - result = self.simulate_get(gumshoe_queue_path_metadata, - project_id) - result_doc = json.loads(result[0]) - self.assertEqual(self.srmock.status, falcon.HTTP_200) - self.assertEqual(result_doc, json.loads(doc)) + # Fetch metadata + result = self.simulate_get(gumshoe_queue_path_metadata, + project_id) + result_doc = json.loads(result[0]) + self.assertEqual(self.srmock.status, falcon.HTTP_200) + self.assertEqual(result_doc, json.loads(doc)) # Stats empty queue self.simulate_get(gumshoe_queue_path_stats, project_id) @@ -112,9 +102,8 @@ class QueueLifecycleBaseTest(base.TestBase): self.assertEqual(self.srmock.status, falcon.HTTP_404) # Get non-existent metadata - if self.metadata_support: - self.simulate_get(gumshoe_queue_path_metadata, project_id) - self.assertEqual(self.srmock.status, falcon.HTTP_404) + self.simulate_get(gumshoe_queue_path_metadata, project_id) + self.assertEqual(self.srmock.status, falcon.HTTP_404) def test_name_restrictions(self): self.simulate_put(self.queue_path + '/Nice-Boat_2') @@ -169,9 +158,6 @@ class QueueLifecycleBaseTest(base.TestBase): @ddt.data('{', '[]', '.', ' ', '') def test_bad_metadata(self, document): - if not self.metadata_support: - return - self.simulate_put(self.fizbat_queue_path, '7e55e1a7e') self.assertEqual(self.srmock.status, falcon.HTTP_201) @@ -275,8 +261,7 @@ class QueueLifecycleBaseTest(base.TestBase): def create_queue(name, project_id, body): uri = self.queue_path + '/' + name self.simulate_put(uri, project_id) - if self.metadata_support: - self.simulate_put(uri + '/metadata', project_id, body=body) + self.simulate_put(uri + '/metadata', project_id, body=body) create_queue('g1', None, '{"answer": 42}') create_queue('g2', None, '{"answer": 42}') @@ -295,9 +280,8 @@ class QueueLifecycleBaseTest(base.TestBase): queues = result_doc['queues'] self.assertEqual(len(queues), 2) - if self.metadata_support: - for queue in queues: - self.assertEqual(queue['metadata'], {'answer': 42}) + for queue in queues: + self.assertEqual(queue['metadata'], {'answer': 42}) # List (limit) result = self.simulate_get(self.queue_path, project_id, @@ -338,12 +322,11 @@ class QueueLifecycleBaseTest(base.TestBase): result_doc = json.loads(result[0]) [target, params] = result_doc['links'][0]['href'].split('?') - if self.metadata_support: - queue = result_doc['queues'][0] - result = self.simulate_get(queue['href'] + '/metadata', project_id) - result_doc = json.loads(result[0]) - self.assertEqual(result_doc, queue['metadata']) - self.assertEqual(result_doc, {'node': 31}) + queue = result_doc['queues'][0] + result = self.simulate_get(queue['href'] + '/metadata', project_id) + result_doc = json.loads(result[0]) + self.assertEqual(result_doc, queue['metadata']) + self.assertEqual(result_doc, {'node': 31}) # List tail self.simulate_get(target, project_id, query_string=params) diff --git a/tests/unit/queues/transport/wsgi/test_v1_1.py b/tests/unit/queues/transport/wsgi/test_v1_1.py index d943a4ef6..df8656687 100644 --- a/tests/unit/queues/transport/wsgi/test_v1_1.py +++ b/tests/unit/queues/transport/wsgi/test_v1_1.py @@ -76,42 +76,10 @@ class TestQueueFaultyDriver(wsgi.TestQueueFaultyDriver): # sort of a pain; is there a better way? class TestQueueLifecycleMongoDB(wsgi.TestQueueLifecycleMongoDB): url_prefix = URL_PREFIX - metadata_support = False - - def test_no_metadata(self): - """Metadata deprecated.""" - - def test_too_much_metadata(self): - """Metadata deprecated.""" - - def test_way_too_much_metadata(self): - """Metadata deprecated.""" - - def test_custom_metadata(self): - """Metadata deprecated.""" - - def test_update_metadata(self): - """Metadata deprecated.""" class TestQueueLifecycleSqlalchemy(wsgi.TestQueueLifecycleSqlalchemy): url_prefix = URL_PREFIX - metadata_support = False - - def test_no_metadata(self): - """Metadata deprecated.""" - - def test_too_much_metadata(self): - """Metadata deprecated.""" - - def test_way_too_much_metadata(self): - """Metadata deprecated.""" - - def test_custom_metadata(self): - """Metadata deprecated.""" - - def test_update_metadata(self): - """Metadata deprecated.""" class TestShardsMongoDB(wsgi.TestShardsMongoDB): @@ -159,31 +127,3 @@ class TestHealth(wsgi.TestBase): response = self.simulate_head('/v1.1/health') self.assertEqual(self.srmock.status, falcon.HTTP_204) self.assertEqual(response, []) - - -class TestDeprecated(wsgi.TestBase): - - config_file = 'wsgi_sqlalchemy.conf' - queue_path = '/v1.1/queues/test-queue' - - def setUp(self): - super(TestDeprecated, self).setUp() - - self.simulate_put(self.queue_path) - self.assertEqual(self.srmock.status, falcon.HTTP_201) - - def tearDown(self): - self.simulate_delete(self.queue_path) - self.assertEqual(self.srmock.status, falcon.HTTP_204) - - super(TestDeprecated, self).tearDown() - - def test_queue_metadata(self): - self.simulate_get(self.queue_path + '/metadata') - self.assertEqual(self.srmock.status, falcon.HTTP_200) - - # NOTE(kgriffs): Ensure that metadata created in v1.0 - # is read-only in v1.1. - self.simulate_put(self.queue_path + '/metadata') - self.assertEqual(self.srmock.status, falcon.HTTP_405) - self.assertEqual(self.srmock.headers_dict['Allow'], 'GET')