From 600706194375168800c9879e05bc40cdb54b25d3 Mon Sep 17 00:00:00 2001 From: Darja Shakhray Date: Sat, 21 Nov 2015 17:07:38 +0300 Subject: [PATCH] Added support new v2 API image filters Added support filtering images based on lists using the 'in' operator. Filters: *id *name *container_format *disk_format *status DocImpact ApiImpact Implements bp: in-filtering-operator Change-Id: I9cac81b9d5cbec979e88cf2dd0e3b710ed45630c --- glance/common/utils.py | 46 +++++++++++++++ glance/db/simple/api.py | 14 +++++ glance/db/sqlalchemy/api.py | 14 +++++ glance/tests/functional/db/base.py | 58 +++++++++++++++++++ glance/tests/functional/v2/test_images.py | 35 ++++++++++- glance/tests/unit/common/test_utils.py | 22 +++++++ .../new_image_filters-c888361e6ecf495c.yaml | 16 +++++ 7 files changed, 202 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/new_image_filters-c888361e6ecf495c.yaml diff --git a/glance/common/utils.py b/glance/common/utils.py index 2b8b6f12a0..5f34538e7d 100644 --- a/glance/common/utils.py +++ b/glance/common/utils.py @@ -607,6 +607,52 @@ def split_filter_op(expression): return op, threshold +def validate_quotes(value): + """Validate filter values + + Validation opening/closing quotes in the expression. + """ + open_quotes = True + for i in range(len(value)): + if value[i] == '"': + if i and value[i - 1] == '\\': + continue + if open_quotes: + if i and value[i - 1] != ',': + msg = _("Invalid filter value %s. There is no comma " + "before opening quotation mark.") % value + raise exception.InvalidParameterValue(message=msg) + else: + if i + 1 != len(value) and value[i + 1] != ",": + msg = _("Invalid filter value %s. There is no comma " + "after closing quotation mark.") % value + raise exception.InvalidParameterValue(message=msg) + open_quotes = not open_quotes + if not open_quotes: + msg = _("Invalid filter value %s. The quote is not closed.") % value + raise exception.InvalidParameterValue(message=msg) + + +def split_filter_value_for_quotes(value): + """Split filter values + + Split values by commas and quotes for 'in' operator, according api-wg. + """ + validate_quotes(value) + tmp = re.compile(r''' + "( # if found a double-quote + [^\"\\]* # take characters either non-quotes or backslashes + (?:\\. # take backslashes and character after it + [^\"\\]*)* # take characters either non-quotes or backslashes + ) # before double-quote + ",? # a double-quote with comma maybe + | ([^,]+),? # if not found double-quote take any non-comma + # characters with comma maybe + | , # if we have only comma take empty string + ''', re.VERBOSE) + return [val[0] or val[1] for val in re.findall(tmp, value)] + + def evaluate_filter_op(value, operator, threshold): """Evaluate a comparison operator. Designed for use on a comparative-filtering query field. diff --git a/glance/db/simple/api.py b/glance/db/simple/api.py index 939d04a3bf..132499e345 100644 --- a/glance/db/simple/api.py +++ b/glance/db/simple/api.py @@ -305,6 +305,20 @@ def _filter_images(images, filters, context, threshold = timeutils.normalize_time(parsed_time) to_add = utils.evaluate_filter_op(attr_value, operator, threshold) + elif k in ['name', 'id', 'status', + 'container_format', 'disk_format']: + attr_value = image.get(key) + operator, list_value = utils.split_filter_op(value) + if operator == 'in': + threshold = utils.split_filter_value_for_quotes(list_value) + to_add = attr_value in threshold + elif operator == 'eq': + to_add = (attr_value == list_value) + else: + msg = (_("Unable to filter by unknown operator '%s'.") + % operator) + raise exception.InvalidFilterOperatorValue(msg) + elif k != 'is_public' and image.get(k) is not None: to_add = image.get(key) == value elif k == 'tags': diff --git a/glance/db/sqlalchemy/api.py b/glance/db/sqlalchemy/api.py index 8f9e2f21e9..4815775245 100644 --- a/glance/db/sqlalchemy/api.py +++ b/glance/db/sqlalchemy/api.py @@ -510,6 +510,20 @@ def _make_conditions_from_filters(filters, is_public=None): threshold) image_conditions.append(comparison) + elif k in ['name', 'id', 'status', 'container_format', 'disk_format']: + attr_value = getattr(models.Image, key) + operator, list_value = utils.split_filter_op(filters.pop(k)) + if operator == 'in': + threshold = utils.split_filter_value_for_quotes(list_value) + comparison = attr_value.in_(threshold) + image_conditions.append(comparison) + elif operator == 'eq': + image_conditions.append(attr_value == list_value) + else: + msg = (_("Unable to filter by unknown operator '%s'.") + % operator) + raise exception.InvalidFilterOperatorValue(msg) + for (k, value) in filters.items(): if hasattr(models.Image, k): image_conditions.append(getattr(models.Image, k) == value) diff --git a/glance/tests/functional/db/base.py b/glance/tests/functional/db/base.py index 5ab72ffabc..8709c30e02 100644 --- a/glance/tests/functional/db/base.py +++ b/glance/tests/functional/db/base.py @@ -505,6 +505,64 @@ class DriverTests(object): filters={'updated_at': time_expr}) self.assertEqual(0, len(images)) + def test_filter_image_by_invalid_operator(self): + self.assertRaises(exception.InvalidFilterOperatorValue, + self.db_api.image_get_all, + self.context, filters={'status': 'lala:active'}) + + def test_image_get_all_with_filter_in_status(self): + images = self.db_api.image_get_all(self.context, + filters={'status': 'in:active'}) + self.assertEqual(3, len(images)) + + def test_image_get_all_with_filter_in_name(self): + data = 'in:%s' % self.fixtures[0]['name'] + images = self.db_api.image_get_all(self.context, + filters={'name': data}) + self.assertEqual(3, len(images)) + + def test_image_get_all_with_filter_in_container_format(self): + images = self.db_api.image_get_all(self.context, + filters={'container_format': + 'in:ami,bare,ovf'}) + self.assertEqual(3, len(images)) + + def test_image_get_all_with_filter_in_disk_format(self): + images = self.db_api.image_get_all(self.context, + filters={'disk_format': + 'in:vhd'}) + self.assertEqual(3, len(images)) + + def test_image_get_all_with_filter_in_id(self): + data = 'in:%s,%s' % (UUID1, UUID2) + images = self.db_api.image_get_all(self.context, + filters={'id': data}) + self.assertEqual(2, len(images)) + + def test_image_get_all_with_quotes(self): + fixture = {'name': 'fake\\\"name'} + self.db_api.image_update(self.adm_context, UUID3, fixture) + + fixture = {'name': 'fake,name'} + self.db_api.image_update(self.adm_context, UUID2, fixture) + + fixture = {'name': 'fakename'} + self.db_api.image_update(self.adm_context, UUID1, fixture) + + data = 'in:\"fake\\\"name\",fakename,\"fake,name\"' + + images = self.db_api.image_get_all(self.context, + filters={'name': data}) + self.assertEqual(3, len(images)) + + def test_image_get_all_with_invalid_quotes(self): + invalid_expr = ['in:\"name', 'in:\"name\"name', 'in:name\"dd\"', + 'in:na\"me', 'in:\"name\"\"name\"'] + for expr in invalid_expr: + self.assertRaises(exception.InvalidParameterValue, + self.db_api.image_get_all, + self.context, filters={'name': expr}) + def test_image_get_all_size_min_max(self): images = self.db_api.image_get_all(self.context, filters={ diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py index 41a4ffcca9..922a73dd01 100644 --- a/glance/tests/functional/v2/test_images.py +++ b/glance/tests/functional/v2/test_images.py @@ -2585,13 +2585,15 @@ class TestImages(functional.FunctionalTest): # Create 7 images images = [] fixtures = [ - {'name': 'image-3', 'type': 'kernel', 'ping': 'pong'}, - {'name': 'image-4', 'type': 'kernel', 'ping': 'pong'}, + {'name': 'image-3', 'type': 'kernel', 'ping': 'pong', + 'container_format': 'ami', 'disk_format': 'ami'}, + {'name': 'image-4', 'type': 'kernel', 'ping': 'pong', + 'container_format': 'bare', 'disk_format': 'ami'}, {'name': 'image-1', 'type': 'kernel', 'ping': 'pong'}, {'name': 'image-3', 'type': 'ramdisk', 'ping': 'pong'}, {'name': 'image-2', 'type': 'kernel', 'ping': 'ding'}, {'name': 'image-3', 'type': 'kernel', 'ping': 'pong'}, - {'name': 'image-2', 'type': 'kernel', 'ping': 'pong'}, + {'name': 'image-2,image-5', 'type': 'kernel', 'ping': 'pong'}, ] path = self._url('/v2/images') headers = self._headers({'content-type': 'application/json'}) @@ -2649,6 +2651,33 @@ class TestImages(functional.FunctionalTest): response = requests.get(path, headers=self._headers()) self.assertEqual(400, response.status_code) + # Image list filters by name with in operator + url_template = '/v2/images?name=in:%s' + filter_value = 'image-1,image-2' + path = self._url(url_template % filter_value) + response = requests.get(path, headers=self._headers()) + self.assertEqual(200, response.status_code) + body = jsonutils.loads(response.text) + self.assertGreaterEqual(3, len(body['images'])) + + # Image list filters by container_format with in operator + url_template = '/v2/images?container_format=in:%s' + filter_value = 'bare,ami' + path = self._url(url_template % filter_value) + response = requests.get(path, headers=self._headers()) + self.assertEqual(200, response.status_code) + body = jsonutils.loads(response.text) + self.assertGreaterEqual(2, len(body['images'])) + + # Image list filters by disk_format with in operator + url_template = '/v2/images?disk_format=in:%s' + filter_value = 'bare,ami,iso' + path = self._url(url_template % filter_value) + response = requests.get(path, headers=self._headers()) + self.assertEqual(200, response.status_code) + body = jsonutils.loads(response.text) + self.assertGreaterEqual(2, len(body['images'])) + # Begin pagination after the first image template_url = ('/v2/images?limit=2&sort_dir=asc&sort_key=name' '&marker=%s&type=kernel&ping=pong') diff --git a/glance/tests/unit/common/test_utils.py b/glance/tests/unit/common/test_utils.py index 7fc2d2558b..423b95dfed 100644 --- a/glance/tests/unit/common/test_utils.py +++ b/glance/tests/unit/common/test_utils.py @@ -423,6 +423,28 @@ class SplitFilterOpTestCase(test_utils.BaseTestCase): returned = utils.split_filter_op(expr) self.assertEqual(('eq', 'bar'), returned) + def test_in_operator(self): + expr = 'in:bar' + returned = utils.split_filter_op(expr) + self.assertEqual(('in', 'bar'), returned) + + def test_split_filter_value_for_quotes(self): + expr = '\"fake\\\"name\",fakename,\"fake,name\"' + returned = utils.split_filter_value_for_quotes(expr) + list_values = ['fake\\"name', 'fakename', 'fake,name'] + self.assertEqual(list_values, returned) + + def test_validate_quotes(self): + expr = '\"aaa\\\"aa\",bb,\"cc\"' + returned = utils.validate_quotes(expr) + self.assertIsNone(returned) + + invalid_expr = ['\"aa', 'ss\"', 'aa\"bb\"cc', '\"aa\"\"bb\"'] + for expr in invalid_expr: + self.assertRaises(exception.InvalidParameterValue, + utils.validate_quotes, + expr) + def test_default_operator(self): expr = 'bar' returned = utils.split_filter_op(expr) diff --git a/releasenotes/notes/new_image_filters-c888361e6ecf495c.yaml b/releasenotes/notes/new_image_filters-c888361e6ecf495c.yaml new file mode 100644 index 0000000000..a0cc8fe1d6 --- /dev/null +++ b/releasenotes/notes/new_image_filters-c888361e6ecf495c.yaml @@ -0,0 +1,16 @@ +--- +features: + - Implement the ability to filter images by the properties `id`, + `name`, `status`,`container_format`, `disk_format` using the 'in' + operator between the values. + Following the pattern of existing filters, new filters are specified as + query parameters using the field to filter as the key and the filter + criteria as the value in the parameter. + Filtering based on the principle of full compliance with the template, + for example 'name = in:deb' does not match 'debian'. + Changes apply exclusively to the API v2 Image entity listings + An example of an acceptance criteria using the 'in' operator for name + ?name=in:name1,name2,name3. + These filters were added using syntax that conforms to the latest + guidelines from the OpenStack API Working Group. +