From 0ca0a0ca656cffc257e27b2de53596a69998bc38 Mon Sep 17 00:00:00 2001 From: Ildiko Vancsa Date: Thu, 2 Jan 2014 10:48:49 +0100 Subject: [PATCH] Implements field validation for complex query functionality Implements: blueprint complex-filter-expressions-in-api-queries Change-Id: Id186995b4c7106f9e3ceb596b3faf46cb2231790 --- ceilometer/api/controllers/v2.py | 61 +++++-- ceilometer/storage/models.py | 6 + ceilometer/tests/api/v2/test_complex_query.py | 160 ++++++++++++++---- .../api/v2/test_complex_query_scenarios.py | 74 +++++++- ceilometer/tests/storage/test_models.py | 27 +++ doc/source/webapi/v2.rst | 5 +- 6 files changed, 284 insertions(+), 49 deletions(-) diff --git a/ceilometer/api/controllers/v2.py b/ceilometer/api/controllers/v2.py index 9af4b2b11..14d3df68b 100644 --- a/ceilometer/api/controllers/v2.py +++ b/ceilometer/api/controllers/v2.py @@ -993,10 +993,9 @@ class ComplexQuery(_Base): ) -def _list_to_regexp(items): +def _list_to_regexp(items, regexp_prefix=""): regexp = ["^%s$" % item for item in items] - regexp = "|".join(regexp) - regexp = "(?i)" + regexp + regexp = regexp_prefix + "|".join(regexp) return regexp @@ -1004,10 +1003,11 @@ class ValidatedComplexQuery(object): complex_operators = ["and", "or"] order_directions = ["asc", "desc"] simple_ops = ["=", "!=", "<", ">", "<=", "=<", ">=", "=>"] + regexp_prefix = "(?i)" - complex_ops = _list_to_regexp(complex_operators) - simple_ops = _list_to_regexp(simple_ops) - order_directions = _list_to_regexp(order_directions) + complex_ops = _list_to_regexp(complex_operators, regexp_prefix) + simple_ops = _list_to_regexp(simple_ops, regexp_prefix) + order_directions = _list_to_regexp(order_directions, regexp_prefix) schema_value = { "oneOf": [{"type": "string"}, @@ -1062,8 +1062,22 @@ class ValidatedComplexQuery(object): "maxProperties": 1}} timestamp_fields = ["timestamp", "state_timestamp"] + name_mapping = {"user": "user_id", + "project": "project_id", + "resource": "resource_id"} + + def __init__(self, query, db_model, additional_valid_keys): + valid_keys = db_model.get_field_names() + valid_keys = list(valid_keys) + additional_valid_keys + valid_fields = _list_to_regexp(valid_keys) + + self.schema_field["patternProperties"] = { + valid_fields: self.schema_value} + + self.orderby_schema["items"]["patternProperties"] = { + valid_fields: {"type": "string", + "pattern": self.order_directions}} - def __init__(self, query): self.original_query = query def validate(self, visibility_field): @@ -1076,6 +1090,7 @@ class ValidatedComplexQuery(object): self._validate_filter(self.filter_expr) self._replace_isotime_with_datetime(self.filter_expr) self._convert_operator_to_lower_case(self.filter_expr) + self._normalize_field_names_for_db_model(self.filter_expr) self._force_visibility(visibility_field) @@ -1085,6 +1100,7 @@ class ValidatedComplexQuery(object): self.orderby = json.loads(self.original_query.orderby) self._validate_orderby(self.orderby) self._convert_orderby_to_lower_case(self.orderby) + self._normalize_field_names_in_orderby(self.orderby) if self.original_query.limit is wtypes.Unset: self.limit = None @@ -1100,6 +1116,10 @@ class ValidatedComplexQuery(object): for orderby_field in orderby: utils.lowercase_values(orderby_field) + def _normalize_field_names_in_orderby(self, orderby): + for orderby_field in orderby: + self._replace_field_names(orderby_field) + def _traverse_postorder(self, tree, visitor): op = tree.keys()[0] if op.lower() in self.complex_operators: @@ -1150,6 +1170,21 @@ class ValidatedComplexQuery(object): self._traverse_postorder(filter_expr, replace_isotime) + def _normalize_field_names_for_db_model(self, filter_expr): + def _normalize_field_names(subfilter): + op = subfilter.keys()[0] + if op.lower() not in self.complex_operators: + self._replace_field_names(subfilter.values()[0]) + self._traverse_postorder(filter_expr, + _normalize_field_names) + + def _replace_field_names(self, subfilter): + field = subfilter.keys()[0] + value = subfilter[field] + if field in self.name_mapping: + del subfilter[field] + subfilter[self.name_mapping[field]] = value + def _convert_operator_to_lower_case(self, filter_expr): self._traverse_postorder(filter_expr, utils.lowercase_keys) @@ -2043,7 +2078,9 @@ class QuerySamplesController(rest.RestController): :param body: Query rules for the samples to be returned. """ - query = ValidatedComplexQuery(body) + query = ValidatedComplexQuery(body, + storage.models.Sample, + ["user", "project", "resource"]) query.validate(visibility_field="project_id") conn = pecan.request.storage_conn return [Sample.from_db_model(s) @@ -2061,7 +2098,9 @@ class QueryAlarmHistoryController(rest.RestController): :param body: Query rules for the alarm history to be returned. """ - query = ValidatedComplexQuery(body) + query = ValidatedComplexQuery(body, + storage.models.AlarmChange, + ["user", "project"]) query.validate(visibility_field="on_behalf_of") conn = pecan.request.storage_conn return [AlarmChange.from_db_model(s) @@ -2081,7 +2120,9 @@ class QueryAlarmsController(rest.RestController): :param body: Query rules for the alarms to be returned. """ - query = ValidatedComplexQuery(body) + query = ValidatedComplexQuery(body, + storage.models.Alarm, + ["user", "project"]) query.validate(visibility_field="project_id") conn = pecan.request.storage_conn return [Alarm.from_db_model(s) diff --git a/ceilometer/storage/models.py b/ceilometer/storage/models.py index 5ff33759a..ea6e935a4 100644 --- a/ceilometer/storage/models.py +++ b/ceilometer/storage/models.py @@ -17,6 +17,7 @@ # under the License. """Model classes for use in the storage API. """ +import inspect from ceilometer.openstack.common import timeutils @@ -44,6 +45,11 @@ class Model(object): def __eq__(self, other): return self.as_dict() == other.as_dict() + @classmethod + def get_field_names(cls): + fields = inspect.getargspec(cls.__init__)[0] + return set(fields) - set(["self"]) + class Event(Model): """A raw event from the source system. Events have Traits. diff --git a/ceilometer/tests/api/v2/test_complex_query.py b/ceilometer/tests/api/v2/test_complex_query.py index ac48f5616..4c72c6f69 100644 --- a/ceilometer/tests/api/v2/test_complex_query.py +++ b/ceilometer/tests/api/v2/test_complex_query.py @@ -25,11 +25,15 @@ import wsme from ceilometer.api.controllers import v2 as api from ceilometer.openstack.common import test +from ceilometer import storage as storage class FakeComplexQuery(api.ValidatedComplexQuery): - def __init__(self): - super(FakeComplexQuery, self).__init__(query=None) + def __init__(self, db_model, additional_valid_keys): + super(FakeComplexQuery, self).__init__(query=None, + db_model=db_model, + additional_valid_keys= + additional_valid_keys) class TestComplexQuery(test.BaseTestCase): @@ -37,7 +41,13 @@ class TestComplexQuery(test.BaseTestCase): super(TestComplexQuery, self).setUp() self.useFixture(fixtures.MonkeyPatch( 'pecan.response', mock.MagicMock())) - self.query = api.ValidatedComplexQuery(FakeComplexQuery()) + self.query = FakeComplexQuery(storage.models.Sample, + ["user", "project", "resource"]) + self.query_alarm = FakeComplexQuery(storage.models.Alarm, + ["user", "project"]) + self.query_alarmchange = FakeComplexQuery( + storage.models.AlarmChange, + ["user", "project"]) def test_replace_isotime_utc(self): filter_expr = {"=": {"timestamp": "2013-12-05T19:38:29Z"}} @@ -92,37 +102,118 @@ class TestComplexQuery(test.BaseTestCase): self.assertEqual("or", filter_expr.keys()[0]) self.assertEqual("and", filter_expr["or"][1].keys()[0]) + def test_invalid_filter_misstyped_field_name_samples(self): + filter = {"=": {"project_id11": 42}} + self.assertRaises(jsonschema.ValidationError, + self.query._validate_filter, + filter) + + def test_invalid_filter_misstyped_field_name_alarms(self): + filter = {"=": {"enabbled": True}} + self.assertRaises(jsonschema.ValidationError, + self.query_alarm._validate_filter, + filter) + + def test_invalid_filter_misstyped_field_name_alarmchange(self): + filter = {"=": {"tpe": "rule change"}} + self.assertRaises(jsonschema.ValidationError, + self.query_alarmchange._validate_filter, + filter) + + def test_invalid_complex_filter_wrong_field_names(self): + filter = {"and": + [{"=": {"non_existing_field": 42}}, + {"=": {"project_id": 42}}]} + self.assertRaises(jsonschema.ValidationError, + self.query._validate_filter, + filter) + + filter = {"and": + [{"=": {"project_id": 42}}, + {"=": {"non_existing_field": 42}}]} + self.assertRaises(jsonschema.ValidationError, + self.query_alarm._validate_filter, + filter) + + filter = {"and": + [{"=": {"project_id11": 42}}, + {"=": {"project_id": 42}}]} + self.assertRaises(jsonschema.ValidationError, + self.query_alarmchange._validate_filter, + filter) + + filter = {"or": + [{"=": {"non_existing_field": 42}}, + {"and": + [{"=": {"project_id": 44}}, + {"=": {"project_id": 42}}]}]} + self.assertRaises(jsonschema.ValidationError, + self.query._validate_filter, + filter) + + filter = {"or": + [{"=": {"project_id": 43}}, + {"and": + [{"=": {"project_id": 44}}, + {"=": {"non_existing_field": 42}}]}]} + self.assertRaises(jsonschema.ValidationError, + self.query_alarm._validate_filter, + filter) + def test_convert_orderby(self): orderby = [] self.query._convert_orderby_to_lower_case(orderby) self.assertEqual([], orderby) - orderby = [{"field1": "DESC"}] + orderby = [{"project_id": "DESC"}] self.query._convert_orderby_to_lower_case(orderby) - self.assertEqual([{"field1": "desc"}], orderby) + self.assertEqual([{"project_id": "desc"}], orderby) - orderby = [{"field1": "ASC"}, {"field2": "DESC"}] + orderby = [{"project_id": "ASC"}, {"resource_id": "DESC"}] self.query._convert_orderby_to_lower_case(orderby) - self.assertEqual([{"field1": "asc"}, {"field2": "desc"}], orderby) + self.assertEqual([{"project_id": "asc"}, {"resource_id": "desc"}], + orderby) def test_validate_orderby_empty_direction(self): - orderby = [{"field1": ""}] + orderby = [{"project_id": ""}] self.assertRaises(jsonschema.ValidationError, self.query._validate_orderby, orderby) - orderby = [{"field1": "asc"}, {"field2": ""}] + orderby = [{"project_id": "asc"}, {"resource_id": ""}] self.assertRaises(jsonschema.ValidationError, self.query._validate_orderby, orderby) def test_validate_orderby_wrong_order_string(self): - orderby = [{"field1": "not a valid order"}] + orderby = [{"project_id": "not a valid order"}] self.assertRaises(jsonschema.ValidationError, self.query._validate_orderby, orderby) def test_validate_orderby_wrong_multiple_item_order_string(self): - orderby = [{"field2": "not a valid order"}, {"field1": "ASC"}] + orderby = [{"project_id": "not a valid order"}, {"resource_id": "ASC"}] + self.assertRaises(jsonschema.ValidationError, + self.query._validate_orderby, + orderby) + + def test_validate_orderby_empty_field_name(self): + orderby = [{"": "ASC"}] + self.assertRaises(jsonschema.ValidationError, + self.query._validate_orderby, + orderby) + orderby = [{"project_id": "asc"}, {"": "desc"}] + self.assertRaises(jsonschema.ValidationError, + self.query._validate_orderby, + orderby) + + def test_validate_orderby_wrong_field_name(self): + orderby = [{"project_id11": "ASC"}] + self.assertRaises(jsonschema.ValidationError, + self.query._validate_orderby, + orderby) + + def test_validate_orderby_wrong_field_name_multiple_item_orderby(self): + orderby = [{"project_id": "asc"}, {"resource_id11": "ASC"}] self.assertRaises(jsonschema.ValidationError, self.query._validate_orderby, orderby) @@ -131,28 +222,29 @@ class TestComplexQuery(test.BaseTestCase): class TestFilterSyntaxValidation(test.BaseTestCase): def setUp(self): super(TestFilterSyntaxValidation, self).setUp() - self.query = api.ValidatedComplexQuery(FakeComplexQuery()) + self.query = FakeComplexQuery(storage.models.Sample, + ["user", "project", "resource"]) def test_simple_operator(self): - filter = {"=": {"field_name": "string_value"}} + filter = {"=": {"project_id": "string_value"}} self.query._validate_filter(filter) - filter = {"=>": {"field_name": "string_value"}} + filter = {"=>": {"project_id": "string_value"}} self.query._validate_filter(filter) def test_invalid_simple_operator(self): - filter = {"==": {"field_name": "string_value"}} + filter = {"==": {"project_id": "string_value"}} self.assertRaises(jsonschema.ValidationError, self.query._validate_filter, filter) - filter = {"": {"field_name": "string_value"}} + filter = {"": {"project_id": "string_value"}} self.assertRaises(jsonschema.ValidationError, self.query._validate_filter, filter) def test_more_than_one_operator_is_invalid(self): - filter = {"=": {"field_name": "string_value"}, + filter = {"=": {"project_id": "string_value"}, "<": {"": ""}} self.assertRaises(jsonschema.ValidationError, self.query._validate_filter, @@ -181,7 +273,7 @@ class TestFilterSyntaxValidation(test.BaseTestCase): filter) def test_more_than_one_field_is_invalid(self): - filter = {"=": {"field": "value", "field2": "value"}} + filter = {"=": {"project_id": "value", "resource_id": "value"}} self.assertRaises(jsonschema.ValidationError, self.query._validate_filter, filter) @@ -193,30 +285,30 @@ class TestFilterSyntaxValidation(test.BaseTestCase): filter) def test_and_or(self): - filter = {"and": [{"=": {"field_name": "string_value"}}, - {"=": {"field2": "value"}}]} + filter = {"and": [{"=": {"project_id": "string_value"}}, + {"=": {"resource_id": "value"}}]} self.query._validate_filter(filter) - filter = {"or": [{"and": [{"=": {"field_name": "string_value"}}, - {"=": {"field2": "value"}}]}, - {"=": {"field3": "value"}}]} + filter = {"or": [{"and": [{"=": {"project_id": "string_value"}}, + {"=": {"resource_id": "value"}}]}, + {"=": {"counter_name": "value"}}]} self.query._validate_filter(filter) - filter = {"or": [{"and": [{"=": {"field_name": "string_value"}}, - {"=": {"field2": "value"}}, - {"<": {"field3": 42}}]}, - {"=": {"field3": "value"}}]} + filter = {"or": [{"and": [{"=": {"project_id": "string_value"}}, + {"=": {"resource_id": "value"}}, + {"<": {"counter_name": 42}}]}, + {"=": {"counter_name": "value"}}]} self.query._validate_filter(filter) def test_invalid_complex_operator(self): - filter = {"xor": [{"=": {"field_name": "string_value"}}, - {"=": {"field2": "value"}}]} + filter = {"xor": [{"=": {"project_id": "string_value"}}, + {"=": {"resource_id": "value"}}]} self.assertRaises(jsonschema.ValidationError, self.query._validate_filter, filter) def test_and_or_with_one_child_is_invalid(self): - filter = {"or": [{"=": {"field_name": "string_value"}}]} + filter = {"or": [{"=": {"project_id": "string_value"}}]} self.assertRaises(jsonschema.ValidationError, self.query._validate_filter, filter) @@ -228,10 +320,10 @@ class TestFilterSyntaxValidation(test.BaseTestCase): filter) def test_more_than_one_complex_operator_is_invalid(self): - filter = {"and": [{"=": {"field_name": "string_value"}}, - {"=": {"field2": "value"}}], - "or": [{"=": {"field_name": "string_value"}}, - {"=": {"field2": "value"}}]} + filter = {"and": [{"=": {"project_id": "string_value"}}, + {"=": {"resource_id": "value"}}], + "or": [{"=": {"project_id": "string_value"}}, + {"=": {"resource_id": "value"}}]} self.assertRaises(jsonschema.ValidationError, self.query._validate_filter, filter) diff --git a/ceilometer/tests/api/v2/test_complex_query_scenarios.py b/ceilometer/tests/api/v2/test_complex_query_scenarios.py index c910585b3..8f9aff293 100644 --- a/ceilometer/tests/api/v2/test_complex_query_scenarios.py +++ b/ceilometer/tests/api/v2/test_complex_query_scenarios.py @@ -54,7 +54,7 @@ class TestQueryMetersController(tests_api.FunctionalTest, 'cumulative', '', 1, - 'user-id', + 'user-id1', 'project-id1', 'resource-id1', timestamp=datetime.datetime(2012, 7, 2, 10, 40), @@ -68,7 +68,7 @@ class TestQueryMetersController(tests_api.FunctionalTest, 'cumulative', '', 1, - 'user-id', + 'user-id2', 'project-id2', 'resource-id2', timestamp=datetime.datetime(2012, 7, 2, 10, 41), @@ -171,6 +171,33 @@ class TestQueryMetersController(tests_api.FunctionalTest, self.assertEqual(["project-id2", "project-id1"], [s["project_id"] for s in data.json]) + def test_query_with_field_name_project(self): + data = self.post_json(self.url, + params={"filter": + '{"=": {"project": "project-id2"}}'}) + + self.assertEqual(1, len(data.json)) + for sample in data.json: + self.assertIn(sample['project_id'], set(["project-id2"])) + + def test_query_with_field_name_resource(self): + data = self.post_json(self.url, + params={"filter": + '{"=": {"resource": "resource-id2"}}'}) + + self.assertEqual(1, len(data.json)) + for sample in data.json: + self.assertIn(sample['resource_id'], set(["resource-id2"])) + + def test_query_with_field_name_user(self): + data = self.post_json(self.url, + params={"filter": + '{"=": {"user": "user-id2"}}'}) + + self.assertEqual(1, len(data.json)) + for sample in data.json: + self.assertIn(sample['user_id'], set(["user-id2"])) + def test_query_with_lower_and_upper_case_orderby(self): data = self.post_json(self.url, params={"orderby": '[{"project_id": "DeSc"}]'}) @@ -179,6 +206,14 @@ class TestQueryMetersController(tests_api.FunctionalTest, self.assertEqual(["project-id2", "project-id1"], [s["project_id"] for s in data.json]) + def test_query_with_user_field_name_orderby(self): + data = self.post_json(self.url, + params={"orderby": '[{"user": "aSc"}]'}) + + self.assertEqual(2, len(data.json)) + self.assertEqual(["user-id1", "user-id2"], + [s["user_id"] for s in data.json]) + def test_query_with_missing_order_in_orderby(self): data = self.post_json(self.url, params={"orderby": '[{"project_id": ""}]'}, @@ -318,6 +353,24 @@ class TestQueryAlarmsController(tests_api.FunctionalTest, for alarm in data.json: self.assertIn(alarm['project_id'], set(["project-id2"])) + def test_query_with_field_project(self): + data = self.post_json(self.alarm_url, + params={"filter": + '{"=": {"project": "project-id2"}}'}) + + self.assertEqual(6, len(data.json)) + for sample in data.json: + self.assertIn(sample['project_id'], set(["project-id2"])) + + def test_query_with_field_user_in_orderby(self): + data = self.post_json(self.alarm_url, + params={"filter": '{"=": {"state": "alarm"}}', + "orderby": '[{"user": "DESC"}]'}) + + self.assertEqual(4, len(data.json)) + self.assertEqual(["user-id2", "user-id2", "user-id1", "user-id1"], + [s["user_id"] for s in data.json]) + def test_query_with_filter_orderby_and_limit(self): orderby = '[{"state_timestamp": "DESC"}]' data = self.post_json(self.alarm_url, @@ -424,6 +477,21 @@ class TestQueryAlarmsHistoryController( self.assertIn(history['on_behalf_of'], (["project-id1", "project-id2"])) + def test_query_with_filter_for_project_orderby_with_user(self): + data = self.post_json(self.url, + params={"filter": + '{"=": {"project": "project-id1"}}', + "orderby": '[{"user": "DESC"}]', + "limit": 3}) + + self.assertEqual(3, len(data.json)) + self.assertEqual(["user-id1", + "user-id1", + "user-id1"], + [h["user_id"] for h in data.json]) + for history in data.json: + self.assertEqual("project-id1", history['project_id']) + def test_query_with_filter_orderby_and_limit(self): data = self.post_json(self.url, params={"filter": '{"=": {"type": "creation"}}', @@ -436,7 +504,7 @@ class TestQueryAlarmsHistoryController( "2013-01-01T00:00:00"], [h["timestamp"] for h in data.json]) for history in data.json: - self.assertEqual("creation", history["type"]) + self.assertEqual("creation", history['type']) def test_limit_should_be_positive(self): data = self.post_json(self.url, diff --git a/ceilometer/tests/storage/test_models.py b/ceilometer/tests/storage/test_models.py index 9780c4963..832f58a1a 100644 --- a/ceilometer/tests/storage/test_models.py +++ b/ceilometer/tests/storage/test_models.py @@ -57,6 +57,33 @@ class ModelTest(test.BaseTestCase): x = models.Event("1", "name", "now", None) self.assertEqual("", repr(x)) + def test_get_field_names_of_sample(self): + sample_fields = ["source", "counter_name", "counter_type", + "counter_unit", "counter_volume", "user_id", + "project_id", "resource_id", "timestamp", + "resource_metadata", "message_id", + "message_signature"] + + self.assertEqual(set(sample_fields), + set(models.Sample.get_field_names())) + + def test_get_field_names_of_alarm(self): + alarm_fields = ["alarm_id", "type", "enabled", "name", "description", + "timestamp", "user_id", "project_id", "state", + "state_timestamp", "ok_actions", "alarm_actions", + "insufficient_data_actions", "repeat_actions", "rule"] + + self.assertEqual(set(alarm_fields), + set(models.Alarm.get_field_names())) + + def test_get_field_names_of_alarmchange(self): + alarmchange_fields = ["event_id", "alarm_id", "type", "detail", + "user_id", "project_id", "on_behalf_of", + "timestamp"] + + self.assertEqual(set(alarmchange_fields), + set(models.AlarmChange.get_field_names())) + class TestTraitModel(test.BaseTestCase): diff --git a/doc/source/webapi/v2.rst b/doc/source/webapi/v2.rst index 88cfaa2d0..ca94d9cae 100644 --- a/doc/source/webapi/v2.rst +++ b/doc/source/webapi/v2.rst @@ -95,8 +95,9 @@ Complex Query +++++++++++++ The filter expressions of the Complex Query feature operate on the fields of *Sample*, *Alarm* and *AlarmChange*. The following comparison operators are -supported: *=*, *!=*, *<*, *<=*, *>* and *>=*; and the following logical operators -can be used: *and* and *or*. +supported: *=*, *!=*, *<*, *<=*, *>* and *>=*; and the following logical +operators can be used: *and* and *or*. The field names are validated against +the database models. Complex Query supports defining the list of orderby expressions in the form of [{"field_name": "asc"}, {"field_name2": "desc"}, ...].