Merge "Implements field validation for complex query functionality"

ceilometer/api/controllers/v2.py

@@ -1023,10 +1023,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
 
 
@@ -1034,10 +1033,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"},
@@ -1092,8 +1092,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):
@@ -1106,6 +1120,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)
 
@@ -1115,6 +1130,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
@@ -1130,6 +1146,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:
@@ -1180,6 +1200,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)
 
@@ -2077,7 +2112,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)
@@ -2095,7 +2132,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)
@@ -2115,7 +2154,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)

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.

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)

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,

ceilometer/tests/storage/test_models.py

@@ -57,6 +57,33 @@ class ModelTest(test.BaseTestCase):
         x = models.Event("1", "name", "now", None)
         self.assertEqual("<Event: 1, name, now, >", 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):
 

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"}, ...].