diff --git a/keystone/common/wsgi.py b/keystone/common/wsgi.py index 485e31c22..b6daaf1e5 100644 --- a/keystone/common/wsgi.py +++ b/keystone/common/wsgi.py @@ -20,7 +20,6 @@ """Utility methods for working with WSGI servers.""" -import inspect import re import routes.middleware @@ -233,7 +232,6 @@ class Application(BaseApplication): params = self._normalize_dict(params) try: - self._method_inspect(method, params) result = method(context, **params) except exception.Unauthorized as e: LOG.warning( @@ -243,9 +241,10 @@ class Application(BaseApplication): except exception.Error as e: LOG.warning(e) return render_exception(e, user_locale=req.best_match_language()) - except exception.ControllerArgsError as e: + except TypeError as e: LOG.exception(e) - return render_exception(e, user_locale=req.best_match_language()) + return render_exception(exception.ValidationError(e), + user_locale=req.best_match_language()) except Exception as e: LOG.exception(e) return render_exception(exception.UnexpectedError(exception=e), @@ -263,39 +262,6 @@ class Application(BaseApplication): response_code = self._get_response_code(req) return render_response(body=result, status=response_code) - def _method_inspect(self, method, params): - """Validates the number of params passed to controller methods. - - A `ControllerArgsError` will be raised if: - 1. Required arguments are missing - 2. Unexpected keyword args are provided to a method not - expecting **kwargs - """ - params = set(params) - - spec = inspect.getargspec(method) - if spec.defaults: - required_args = set(spec.args[:-len(spec.defaults)]) - else: - required_args = set(spec.args) - - required_args.discard('self') # provided by Python - required_args.discard('context') # provided by caller - - err_args = {} - - missing_required_args = required_args - params - if missing_required_args: - err_args['missing_required_args'] = missing_required_args - - if not spec.keywords: - extra_params = params - set(spec.args) - if extra_params: - err_args['extra_params'] = extra_params - - if err_args: - raise exception.ControllerArgsError(**err_args) - def _get_response_code(self, req): req_method = req.environ['REQUEST_METHOD'] controller = importutils.import_class('keystone.common.controller') diff --git a/keystone/exception.py b/keystone/exception.py index a64c0404e..28b8a4077 100644 --- a/keystone/exception.py +++ b/keystone/exception.py @@ -78,24 +78,6 @@ class ValidationTimeStampError(Error): title = 'Bad Request' -class ControllerArgsError(ValidationError): - """Raised when controller methods receive incorrect argumemnts. - - """ - - def _build_message(self, message, missing_required_args=None, - extra_params=None): - errors = [] - - for param in sorted(extra_params or []): - errors.append(_('%s is not allowed.') % param) - - for arg in sorted(missing_required_args or []): - errors.append(_('%s is required.') % arg) - - return u' '.join(str(e) for e in errors) - - class StringLengthExceeded(ValidationError): message_format = _("String length exceeded.The length of" " string '%(string)s' exceeded the limit" diff --git a/keystone/tests/test_wsgi.py b/keystone/tests/test_wsgi.py index 9a89b0956..df31fd11e 100644 --- a/keystone/tests/test_wsgi.py +++ b/keystone/tests/test_wsgi.py @@ -18,8 +18,6 @@ import uuid from babel import localedata import gettext -from testtools import matchers - from keystone.common import wsgi from keystone import exception @@ -316,168 +314,3 @@ class LocalizedResponseTest(tests.TestCase): # are lazy-translated. self.assertIsInstance(_('The resource could not be found.'), gettextutils.Message) - - -class TestControllerMethodInspection(tests.TestCase): - - def setUp(self): - super(TestControllerMethodInspection, self).setUp() - self.app = wsgi.Application() - self.inspect = self.app._method_inspect - - def assertSuccess(self, func_user_test, params): - self.inspect(func_user_test, params) - - def assertFailure(self, func_user_test, params, - expected_extra=None, expected_missing=None): - # NOTE(dstanek): In Python2.7+ assertRaises can be used as a context - # manager. This would make the code much cleaner. - try: - self.inspect(func_user_test, params) - except exception.ControllerArgsError as e: - message = str(e) - - for param in expected_extra or []: - expected_msg = '%s is not allowed.' % param - self.assertThat(message, matchers.Contains(expected_msg)) - - for param in expected_missing or []: - expected_msg = '%s is required.' % param - self.assertThat(message, matchers.Contains(expected_msg)) - - else: - raise self.failureException('ControllerArgsError not raised') - - def test_all_required_parameters_provided(self): - - def func_under_test(a): - pass - self.assertSuccess(func_under_test, params=['a']) - - def func_under_test(a, b): - pass - self.assertSuccess(func_under_test, params=['a', 'b']) - - def test_optional_parameters_provided(self): - - def func_under_test(a=None): - pass - self.assertSuccess(func_under_test, params=['a']) - - def test_optional_parameters_not_provided(self): - - def func_under_test(a=None): - pass - self.assertSuccess(func_under_test, params=[]) - - def func_under_test(a, b=None): - pass - self.assertSuccess(func_under_test, params=['a']) - - def test_some_required_parameters_missing(self): - - def func_under_test(a, b): - pass - self.assertFailure(func_under_test, params=['b'], - expected_missing=['a']) - - def test_extra_parameter_supplied(self): - - def func_under_test(a): - pass - self.assertFailure(func_under_test, params=['a', 'b'], - expected_extra=['b']) - - def test_extra_parameter_supplied_with_kwargs_defined(self): - - def func_under_test(a, **kw): - pass - self.assertSuccess(func_under_test, params=['a', 'b']) - - def test_some_required_parameters_missing_with_kwargs_defined(self): - - def func_under_test(a, b, **kw): - pass - self.assertFailure(func_under_test, params=['a'], - expected_missing=['b']) - - def test_a_method_works(self): - - class AppUnderTest(object): - def index(self, a): - pass - self.assertSuccess(AppUnderTest().index, params=['a']) - - class AppUnderTest(object): - def index(self, a, b): - pass - self.assertFailure(AppUnderTest().index, params=['a'], - expected_missing=['b']) - - def test_auto_provided_params_are_ignored(self): - - class AppUnderTest(object): - def index(self, context): - pass - self.assertSuccess(AppUnderTest().index, params=[]) - - class AppUnderTest(object): - def index(self, context, a): - pass - self.assertSuccess(AppUnderTest().index, params=['a']) - - -class TestWSGIControllerMethodInspection(BaseWSGITest): - - class FakeAppWithArgs(wsgi.Application): - def index(self, context, arg0, arg1): - return arg0, arg1 - - def _execute_test(self, app, expected_body, params=None): - req = self._make_request() - if params: - req.environ['openstack.params'] = params - resp = req.get_response(app()) - self.assertEqual(jsonutils.loads(resp.body), expected_body) - - def test_controller_method_with_no_args(self): - class FakeApp(wsgi.Application): - def index(self, context): - return ['index'] - - self._execute_test(FakeApp, ['index']) - - def test_controller_method_with_correct_args(self): - self._execute_test(self.FakeAppWithArgs, ['value0', 'value1'], - {'arg0': 'value0', 'arg1': 'value1'}) - - def test_controller_method_with_missing_arg(self): - expected_body = { - "error": { - "message": "arg1 is required.", - "code": 400, - "title": "Bad Request" - } - } - self._execute_test(self.FakeAppWithArgs, expected_body, - {'arg0': 'value0'}) - - def test_controller_method_with_multiple_errors(self): - expected_body = { - "error": { - "message": "arg3 is not allowed. " - "arg0 is required. " - "arg1 is required.", - "code": 400, - "title": "Bad Request" - } - } - self._execute_test(self.FakeAppWithArgs, expected_body, - {'arg3': 'value3'}) - - def test_controller_method_with_default_args(self): - class FakeApp(wsgi.Application): - def index(self, context, arg0, arg1='1'): - return arg0, arg1 - - self._execute_test(FakeApp, ['0', '1'], {'arg0': '0'})