From b15de49a4d916b54e1792a9d8c316cc4d33e79f8 Mon Sep 17 00:00:00 2001 From: Flavio Percoco Date: Fri, 4 Oct 2013 10:34:21 +0200 Subject: [PATCH] Complete refactor of api.py This patch introduces validation through jsonschema instead of using a custom format. In order for other API versions to extend this, this patch implements a base API under transport.api that should be extended for every API version. An example of this can be found in test_api.py, future patches will have more realistic examples and extensions of this class. The patch also moves api.py under v1/ since Api definitions are version specific. Partially-Implements blueprint python-marconiclient-v1 Change-Id: Ie7eacc63deeacb67edfada6ceb5c4956c5adc0b0 --- marconiclient/errors.py | 6 ++- marconiclient/{common => queues/v1}/api.py | 0 marconiclient/transport/api.py | 61 ++++++++++++++++++++++ marconiclient/transport/request.py | 30 ++--------- requirements.txt | 1 + tests/unit/transport/test_api.py | 54 +++++++++++++++++++ tests/unit/transport/test_request.py | 25 --------- 7 files changed, 126 insertions(+), 51 deletions(-) rename marconiclient/{common => queues/v1}/api.py (100%) create mode 100644 marconiclient/transport/api.py create mode 100644 tests/unit/transport/test_api.py diff --git a/marconiclient/errors.py b/marconiclient/errors.py index 6b23fc3f..4dc3c57c 100644 --- a/marconiclient/errors.py +++ b/marconiclient/errors.py @@ -12,8 +12,12 @@ # License for the specific language governing permissions and limitations # under the License. -__all__ = ['MarconiError'] +__all__ = ['MarconiError', 'InvalidOperation'] class MarconiError(Exception): """Base class for errors.""" + + +class InvalidOperation(MarconiError): + """Raised when attempted a non existent operation.""" diff --git a/marconiclient/common/api.py b/marconiclient/queues/v1/api.py similarity index 100% rename from marconiclient/common/api.py rename to marconiclient/queues/v1/api.py diff --git a/marconiclient/transport/api.py b/marconiclient/transport/api.py new file mode 100644 index 00000000..90c1b8d2 --- /dev/null +++ b/marconiclient/transport/api.py @@ -0,0 +1,61 @@ +# Copyright (c) 2013 Red Hat, 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. + +import jsonschema +from jsonschema import validators + +from marconiclient import errors + + +class Api(object): + + schema = {} + validators = {} + + def validate(self, operation, params): + """Validates the request data + + This method relies on jsonschema and exists + just as a way for third-party transport to validate + the request. It's not recommended to validate every + request since they are already validated server side. + + :param operation: Operation's for which params need + to be validated. + :type operation: `six.text_type` + :param params: Params to validate + :type params: dict + + :returns: True if the schema is valid, False otherwise + :raises: `errors.InvalidOperation` if the operation + does not exist + """ + + if operation not in self.validators: + try: + schema = self.schema[operation] + self.validators[operation] = validators.Draft4Validator(schema) + except KeyError: + # TODO(flaper87): gettext support + msg = '{0} is not a valid operation'.format(operation) + raise errors.InvalidOperation(msg) + + try: + self.validators[operation].validate(params) + except jsonschema.ValidationError: + # TODO(flaper87): Log error + return False + + return True diff --git a/marconiclient/transport/request.py b/marconiclient/transport/request.py index 28e91521..768b3a75 100644 --- a/marconiclient/transport/request.py +++ b/marconiclient/transport/request.py @@ -17,7 +17,6 @@ import json from marconiclient import auth -from marconiclient.common import api def prepare_request(conf, data=None): @@ -73,7 +72,9 @@ class Request(object): """ def __init__(self, endpoint='', operation='', - content=None, params=None, headers=None): + content=None, params=None, + headers=None, api=None): + self.api = api self.endpoint = endpoint self.operation = operation self.content = content @@ -81,26 +82,5 @@ class Request(object): self.headers = headers or {} def validate(self): - """`None` if the request data is valid, an error message otherwise. - Checks the `operation` and the presence of the `params`. - """ - api_info_data = api.info() - if self.operation not in api_info_data: - return "Invalid operation '%s'" % self.operation - - api_info = api_info_data[self.operation] - - param_names = set() if not self.params else set(self.params.keys()) - # NOTE(al-maisan) Do we have all the mandatory params? - if not api_info.mandatory.issubset(param_names): - missing = sorted(api_info.mandatory - param_names) - return "Missing mandatory params: '%s'" % ', '.join(missing) - - # NOTE(al-maisan) Our params must be either in the mandatory or the - # optional subset. - all_permissible_params = api_info.mandatory.union(api_info.optional) - if not param_names.issubset(all_permissible_params): - invalid = sorted(param_names - all_permissible_params) - return "Invalid params: '%s'" % ', '.join(invalid) - - return None + return self.api.validate(params=self.params, + content=self.content) diff --git a/requirements.txt b/requirements.txt index 1754a0b2..3622aebc 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,7 @@ pbr>=0.5.21,<1.0 six>=1.3.0 stevedore>=0.10 +jsonschema>=1.3.0,!=1.4.0 python-keystoneclient diff --git a/tests/unit/transport/test_api.py b/tests/unit/transport/test_api.py new file mode 100644 index 00000000..4be51a0e --- /dev/null +++ b/tests/unit/transport/test_api.py @@ -0,0 +1,54 @@ +# Copyright (c) 2013 Red Hat, 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 marconiclient import errors +from marconiclient.tests import base +from marconiclient.transport import api + + +class FakeApi(api.Api): + schema = { + 'test_operation': { + 'properties': { + 'name': {'type': 'string'} + }, + + 'additionalProperties': False, + 'required': ['name'] + } + } + + +class TestApi(base.TestBase): + + def setUp(self): + super(TestApi, self).setUp() + self.api = FakeApi() + + def test_valid_params(self): + self.assertTrue(self.api.validate('test_operation', + {'name': 'Sauron'})) + + def test_invalid_params(self): + self.assertFalse(self.api.validate('test_operation', + {'name': 'Sauron', + 'lastname': 'From Mordor'})) + + def test_missing_params(self): + self.assertFalse(self.api.validate('test_operation', {})) + + def test_invalid_operation(self): + self.assertRaises(errors.InvalidOperation, self.api.validate, + 'super_secret_op', {}) diff --git a/tests/unit/transport/test_request.py b/tests/unit/transport/test_request.py index f3d2ee87..58ebbb31 100644 --- a/tests/unit/transport/test_request.py +++ b/tests/unit/transport/test_request.py @@ -23,31 +23,6 @@ HREF = '/v1/queue/' class TestRequest(base.TestBase): - def test_valid_operation(self): - req = request.Request(endpoint=HREF, operation='create_queue', - params=dict(queue_name='high_load')) - self.assertIs(None, req.validate()) - - def test_invalid_operation(self): - req = request.Request(endpoint=HREF, operation='jump_queue', - params=dict(name='high_load')) - self.assertEqual("Invalid operation 'jump_queue'", req.validate()) - - def test_missing_mandatory_param(self): - req = request.Request(endpoint=HREF, operation='get_message', - params=dict()) - self.assertEqual("Missing mandatory params: 'message_id, queue_name'", - req.validate()) - - def test_missing_optional_param(self): - req = request.Request(endpoint=HREF, operation='delete_message', - params=dict(queue_name='abc', message_id='1')) - self.assertIs(None, req.validate()) - - def test_invalid__param(self): - req = request.Request(endpoint=HREF, operation='delete_queue', - params=dict(queue_name='xy', WAT='!?')) - self.assertEqual("Invalid params: 'WAT'", req.validate()) def test_prepare_request(self): req = request.prepare_request(self.conf)