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
This commit is contained in:
@@ -12,8 +12,12 @@
|
|||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
__all__ = ['MarconiError']
|
__all__ = ['MarconiError', 'InvalidOperation']
|
||||||
|
|
||||||
|
|
||||||
class MarconiError(Exception):
|
class MarconiError(Exception):
|
||||||
"""Base class for errors."""
|
"""Base class for errors."""
|
||||||
|
|
||||||
|
|
||||||
|
class InvalidOperation(MarconiError):
|
||||||
|
"""Raised when attempted a non existent operation."""
|
||||||
|
|||||||
61
marconiclient/transport/api.py
Normal file
61
marconiclient/transport/api.py
Normal file
@@ -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
|
||||||
@@ -17,7 +17,6 @@
|
|||||||
import json
|
import json
|
||||||
|
|
||||||
from marconiclient import auth
|
from marconiclient import auth
|
||||||
from marconiclient.common import api
|
|
||||||
|
|
||||||
|
|
||||||
def prepare_request(conf, data=None):
|
def prepare_request(conf, data=None):
|
||||||
@@ -73,7 +72,9 @@ class Request(object):
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
def __init__(self, endpoint='', operation='',
|
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.endpoint = endpoint
|
||||||
self.operation = operation
|
self.operation = operation
|
||||||
self.content = content
|
self.content = content
|
||||||
@@ -81,26 +82,5 @@ class Request(object):
|
|||||||
self.headers = headers or {}
|
self.headers = headers or {}
|
||||||
|
|
||||||
def validate(self):
|
def validate(self):
|
||||||
"""`None` if the request data is valid, an error message otherwise.
|
return self.api.validate(params=self.params,
|
||||||
Checks the `operation` and the presence of the `params`.
|
content=self.content)
|
||||||
"""
|
|
||||||
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
|
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
pbr>=0.5.21,<1.0
|
pbr>=0.5.21,<1.0
|
||||||
six>=1.3.0
|
six>=1.3.0
|
||||||
stevedore>=0.10
|
stevedore>=0.10
|
||||||
|
jsonschema>=1.3.0,!=1.4.0
|
||||||
|
|
||||||
python-keystoneclient
|
python-keystoneclient
|
||||||
|
|
||||||
|
|||||||
54
tests/unit/transport/test_api.py
Normal file
54
tests/unit/transport/test_api.py
Normal file
@@ -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', {})
|
||||||
@@ -23,31 +23,6 @@ HREF = '/v1/queue/'
|
|||||||
|
|
||||||
|
|
||||||
class TestRequest(base.TestBase):
|
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):
|
def test_prepare_request(self):
|
||||||
req = request.prepare_request(self.conf)
|
req = request.prepare_request(self.conf)
|
||||||
|
|||||||
Reference in New Issue
Block a user