From 32e2e29e62e650db75afd0fccfb685339011a63a Mon Sep 17 00:00:00 2001 From: kgriffs Date: Mon, 7 Oct 2013 16:44:04 -0500 Subject: [PATCH] fix(mongo): Queue listing may include queues from other projects This patch changes the mongo driver so that it explicitly checks that the queue belongs to the correct project, rather than simply getting everything greater than a specific queue name. The associated unit test was updated to catch any regressions in the fix. The test was also updated to take into account the change in queue metadata handling that landed several weeks ago. Change-Id: I6434d8452ff38bb3191ab947f5f143b650e2b103 Closes-Bug: #1236605 --- marconi/queues/storage/mongodb/queues.py | 8 +- .../transport/wsgi/test_queue_lifecycle.py | 82 ++++++++++++++----- 2 files changed, 65 insertions(+), 25 deletions(-) diff --git a/marconi/queues/storage/mongodb/queues.py b/marconi/queues/storage/mongodb/queues.py index cabbe2e3c..eafb34be0 100644 --- a/marconi/queues/storage/mongodb/queues.py +++ b/marconi/queues/storage/mongodb/queues.py @@ -181,16 +181,14 @@ class QueueController(storage.QueueBase): if not scoped_name.startswith('/'): # NOTE(kgriffs): scoped queue, e.g., 'project-id/queue-name' - query['p_q'] = {'$gt': scoped_name} + project_prefix = '^' + project + '/' + query['p_q'] = {'$regex': project_prefix, '$gt': scoped_name} elif scoped_name == '/': # NOTE(kgriffs): list global queues, but exclude scoped ones query['p_q'] = {'$regex': '^/'} else: # NOTE(kgriffs): unscoped queue, e.g., '/my-global-queue' - query['$and'] = [ - {'p_q': {'$regex': '^/'}}, - {'p_q': {'$gt': scoped_name}}, - ] + query['p_q'] = {'$regex': '^/', '$gt': scoped_name} fields = {'p_q': 1, '_id': 0} if detailed: diff --git a/tests/unit/queues/transport/wsgi/test_queue_lifecycle.py b/tests/unit/queues/transport/wsgi/test_queue_lifecycle.py index d6db6c4eb..ea6d03973 100644 --- a/tests/unit/queues/transport/wsgi/test_queue_lifecycle.py +++ b/tests/unit/queues/transport/wsgi/test_queue_lifecycle.py @@ -213,58 +213,100 @@ class QueueLifecycleBaseTest(base.TestBase): path + '/metadata') def test_list(self): - project_id = '644079696574693' - alt_project_id = '644079696574694' + arbitrary_number = 644079696574693 + project_id = str(arbitrary_number) + + # NOTE(kgriffs): It's important that this one sort after the one + # above. This is in order to prove that bug/1236605 is fixed, and + # stays fixed! + alt_project_id = str(arbitrary_number + 1) # List empty self.simulate_get('/v1/queues', project_id) - self.assertEquals(self.srmock.status, falcon.HTTP_204) + self.assertEqual(self.srmock.status, falcon.HTTP_204) # Payload exceeded self.simulate_get('/v1/queues', project_id, query_string='limit=21') - self.assertEquals(self.srmock.status, falcon.HTTP_400) + self.assertEqual(self.srmock.status, falcon.HTTP_400) # Create some - self.simulate_put('/v1/queues/q1', project_id, body='{"_ttl": 30 }') - self.simulate_put('/v1/queues/q2', project_id, body='{}') - self.simulate_put('/v1/queues/q3', project_id, body='{"_ttl": 30 }') + def create_queue(name, project_id, body): + uri = '/v1/queues/' + name + self.simulate_put(uri, project_id) + self.simulate_put(uri + '/metadata', project_id, body=body) - # List (no metadata) + create_queue('g1', None, '{"answer": 42}') + create_queue('g2', None, '{"answer": 42}') + + create_queue('q1', project_id, '{"node": 31}') + create_queue('q2', project_id, '{"node": 32}') + create_queue('q3', project_id, '{"node": 33}') + + create_queue('q3', alt_project_id, '{"alt": 1}') + + # List (global queues) + result = self.simulate_get('/v1/queues', None, + query_string='limit=2&detailed=true') + + result_doc = json.loads(result[0]) + queues = result_doc['queues'] + self.assertEqual(len(queues), 2) + + for queue in queues: + self.assertEqual(queue['metadata'], {'answer': 42}) + + # List (limit) result = self.simulate_get('/v1/queues', project_id, query_string='limit=2') + result_doc = json.loads(result[0]) + self.assertEqual(len(result_doc['queues']), 2) + + # List (no metadata, get all) + result = self.simulate_get('/v1/queues', project_id, + query_string='limit=5') + result_doc = json.loads(result[0]) [target, params] = result_doc['links'][0]['href'].split('?') - self.assertEquals(self.srmock.status, falcon.HTTP_200) - self.assertEquals(self.srmock.headers_dict['Content-Location'], - '/v1/queues?limit=2') + self.assertEqual(self.srmock.status, falcon.HTTP_200) + self.assertEqual(self.srmock.headers_dict['Content-Location'], + '/v1/queues?limit=5') - for queue in result_doc['queues']: + # Ensure we didn't pick up the queue from the alt project. + queues = result_doc['queues'] + self.assertEqual(len(queues), 3) + + for queue in queues: self.simulate_get(queue['href'] + '/metadata', project_id) - self.assertEquals(self.srmock.status, falcon.HTTP_200) + self.assertEqual(self.srmock.status, falcon.HTTP_200) - self.simulate_get(queue['href'] + '/metadata', alt_project_id) - self.assertEquals(self.srmock.status, falcon.HTTP_404) + self.simulate_get(queue['href'] + '/metadata', 'imnothere') + self.assertEqual(self.srmock.status, falcon.HTTP_404) self.assertNotIn('metadata', queue) # List with metadata result = self.simulate_get('/v1/queues', project_id, - query_string=params + '&detailed=true') + query_string='detailed=true') - self.assertEquals(self.srmock.status, falcon.HTTP_200) + self.assertEqual(self.srmock.status, falcon.HTTP_200) result_doc = json.loads(result[0]) [target, params] = result_doc['links'][0]['href'].split('?') - [queue] = result_doc['queues'] + queue = result_doc['queues'][0] result = self.simulate_get(queue['href'] + '/metadata', project_id) result_doc = json.loads(result[0]) - self.assertEquals(result_doc, queue['metadata']) + self.assertEqual(result_doc, queue['metadata']) + self.assertEqual(result_doc, {'node': 31}) # List tail self.simulate_get(target, project_id, query_string=params) - self.assertEquals(self.srmock.status, falcon.HTTP_204) + self.assertEqual(self.srmock.status, falcon.HTTP_204) + + # List manually-constructed tail + self.simulate_get(target, project_id, query_string='marker=zzz') + self.assertEqual(self.srmock.status, falcon.HTTP_204) class QueueLifecycleMongoDBTests(QueueLifecycleBaseTest):