From c49212e17168f5ebb0833d220bea1dddc5bc88a5 Mon Sep 17 00:00:00 2001 From: Renat Akhmerov Date: Thu, 7 Nov 2019 16:09:33 +0700 Subject: [PATCH] Make it possible to set None to REST API filters * Added a list argument to the method get_all() of the executions API controller class that may contain names of columns that a client wants to set to the null value (None internally in the code) in the query string. Thereby we're getting the ability to filter API entities (currently only workflow executions) fetched with the get_all() by some fields that are assigned to the None (null from the API perspective) value, i.e. typically it means that a value of a field is not defined. Closes-Bug: #1702421 Change-Id: I78fbf993519beb63ee9aef7058bdcb40f0a12ec3 --- mistral/api/controllers/v2/execution.py | 10 ++++- mistral/db/sqlalchemy/model_base.py | 22 +++++++++ mistral/tests/unit/api/v2/test_executions.py | 23 ++++++++++ mistral/tests/unit/utils/test_filter_utils.py | 45 +++++++++++++++++++ mistral/utils/filter_utils.py | 6 ++- 5 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 mistral/tests/unit/utils/test_filter_utils.py diff --git a/mistral/api/controllers/v2/execution.py b/mistral/api/controllers/v2/execution.py index 70334157c..387b07829 100644 --- a/mistral/api/controllers/v2/execution.py +++ b/mistral/api/controllers/v2/execution.py @@ -334,14 +334,15 @@ class ExecutionsController(rest.RestController): wtypes.text, types.uuid, wtypes.text, types.jsontype, types.uuid, types.uuid, STATE_TYPES, wtypes.text, types.jsontype, types.jsontype, wtypes.text, - wtypes.text, bool, types.uuid, bool) + wtypes.text, bool, types.uuid, bool, types.list) def get_all(self, marker=None, limit=None, sort_keys='created_at', sort_dirs='asc', fields='', workflow_name=None, workflow_id=None, description=None, params=None, task_execution_id=None, root_execution_id=None, state=None, state_info=None, input=None, output=None, created_at=None, updated_at=None, include_output=None, project_id=None, - all_projects=False): + all_projects=False, nulls=''): + """Return all Executions. :param marker: Optional. Pagination marker for large data sets. @@ -384,13 +385,18 @@ class ExecutionsController(rest.RestController): Admin required. :param all_projects: Optional. Get resources of all projects. Admin required. + :param nulls: Optional. The names of the columns with null value in + the query. """ acl.enforce('executions:list', context.ctx()) + db_models.WorkflowExecution.check_allowed_none_values(nulls) + if all_projects or project_id: acl.enforce('executions:list:all_projects', context.ctx()) filters = filter_utils.create_filters_from_request_params( + none_values=nulls, created_at=created_at, workflow_name=workflow_name, workflow_id=workflow_id, diff --git a/mistral/db/sqlalchemy/model_base.py b/mistral/db/sqlalchemy/model_base.py index 0ad54b06d..fb3b4a0d9 100644 --- a/mistral/db/sqlalchemy/model_base.py +++ b/mistral/db/sqlalchemy/model_base.py @@ -123,6 +123,28 @@ class _MistralModelBase(oslo_models.ModelBase, oslo_models.TimestampMixin): def __repr__(self): return '%s %s' % (type(self).__name__, self.to_dict().__repr__()) + @classmethod + def _get_nullable_column_names(cls): + return [c.name for c in cls.__table__.columns if c.nullable] + + @classmethod + def check_allowed_none_values(cls, column_names): + """Checks if the given columns can be assigned with None value. + + :param column_names: The names of the columns to check. + """ + all_columns = cls.__table__.columns.keys() + nullable_columns = cls._get_nullable_column_names() + + for col in column_names: + if col not in all_columns: + raise ValueError("'{}' is not a valid field name.".format(col)) + + if col not in nullable_columns: + raise ValueError( + "The field '{}' can't hold None value.".format(col) + ) + MistralModelBase = declarative.declarative_base(cls=_MistralModelBase) diff --git a/mistral/tests/unit/api/v2/test_executions.py b/mistral/tests/unit/api/v2/test_executions.py index e81a7ecf7..1f1bfe1e5 100644 --- a/mistral/tests/unit/api/v2/test_executions.py +++ b/mistral/tests/unit/api/v2/test_executions.py @@ -968,3 +968,26 @@ class TestExecutionsController(base.APITest): self.assertTrue( mock_get_execs.call_args[1].get('project_id', fake_project_id) ) + + def test_get_all_with_nulls_not_valid(self): + resp = self.app.get( + '/v2/executions?limit=10&sort_keys=id&sort_dirs=asc&nulls=invalid', + expect_errors=True + ) + + self.assertEqual(500, resp.status_int) + self.assertIn( + "'invalid' is not a valid field name.", + resp.body.decode() + ) + + resp = self.app.get( + '/v2/executions?limit=10&sort_keys=id&sort_dirs=asc&nulls=id', + expect_errors=True + ) + + self.assertEqual(500, resp.status_int) + self.assertIn( + "The field 'id' can't hold None value.", + resp.body.decode() + ) diff --git a/mistral/tests/unit/utils/test_filter_utils.py b/mistral/tests/unit/utils/test_filter_utils.py new file mode 100644 index 000000000..12288880d --- /dev/null +++ b/mistral/tests/unit/utils/test_filter_utils.py @@ -0,0 +1,45 @@ +# Copyright 2018 - Nokia, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +from mistral.tests.unit import base +from mistral.utils import filter_utils + + +class FilterUtilsTest(base.BaseTest): + def test_create_filters_with_nones(self): + expected_filters = { + 'key2': {'eq': 'value2'}, + 'key1': {'eq': None} + } + + filters = filter_utils.create_filters_from_request_params( + none_values=['key1'], + key1=None, + key2='value2', + key3=None, + ) + + self.assertEqual(expected_filters, filters) + + del expected_filters['key1'] + + filters = filter_utils.create_filters_from_request_params( + none_values=[], + key1=None, + key2='value2', + key3=None, + ) + + self.assertEqual(expected_filters, filters) diff --git a/mistral/utils/filter_utils.py b/mistral/utils/filter_utils.py index 45017c45c..ade70c41e 100644 --- a/mistral/utils/filter_utils.py +++ b/mistral/utils/filter_utils.py @@ -15,16 +15,18 @@ import six -def create_filters_from_request_params(**params): +def create_filters_from_request_params(none_values=None, **params): """Create filters from REST request parameters. + :param none_values: field names, where the value is required to be None. :param req_params: REST request parameters. :return: filters dictionary. """ + none_values = none_values or [] filters = {} for column, data in params.items(): - if data is not None: + if (data is None and column in none_values) or data is not None: if isinstance(data, six.string_types): f_type, value = _extract_filter_type_and_value(data)