From bbd73ce86919bffae3e33ec8c05d1e5418db6e48 Mon Sep 17 00:00:00 2001 From: Kota Tsuyuzaki Date: Tue, 13 Jan 2015 02:25:11 -0800 Subject: [PATCH] Assemble similar stuffs into a validation method Currently, controllers have similar stuffs to validate query parameter either to limit the listing result or to make a marker starts with next to it. This patch intoduces a validation method 'get_validated_param' for those stuffs to get a pre-validated value with some parameters (e.g. default, limit) to keep the code clean. Change-Id: Ia5e939ee342bf4fb8533f1aa97419f5aad9b7fe1 --- swift3/controllers/bucket.py | 6 ++-- swift3/controllers/multi_upload.py | 53 ++++-------------------------- swift3/request.py | 18 ++++++++++ swift3/test/unit/test_request.py | 19 +++++++++++ 4 files changed, 45 insertions(+), 51 deletions(-) diff --git a/swift3/controllers/bucket.py b/swift3/controllers/bucket.py index beeca09d..b5c01503 100644 --- a/swift3/controllers/bucket.py +++ b/swift3/controllers/bucket.py @@ -45,11 +45,9 @@ class BucketController(Controller): """ Handle GET Bucket (List Objects) request """ - if 'max-keys' in req.params: - if req.params.get('max-keys').isdigit() is False: - raise InvalidArgument('max-keys', req.params['max-keys']) - max_keys = int(req.params.get('max-keys', CONF.max_bucket_listing)) + max_keys = req.get_validated_param('max-keys', CONF.max_bucket_listing) + # TODO: Separate max_bucket_listing and default_bucket_listing max_keys = min(max_keys, CONF.max_bucket_listing) encoding_type = req.params.get('encoding-type') diff --git a/swift3/controllers/multi_upload.py b/swift3/controllers/multi_upload.py index 684cd3ea..1c2fcfc3 100644 --- a/swift3/controllers/multi_upload.py +++ b/swift3/controllers/multi_upload.py @@ -145,20 +145,8 @@ class UploadsController(Controller): keymarker = req.params.get('key-marker', '') uploadid = req.params.get('upload-id-marker', '') - maxuploads = DEFAULT_MAX_UPLOADS - - if 'max-uploads' in req.params: - try: - maxuploads = int(req.params['max-uploads']) - if maxuploads < 0 or DEFAULT_MAX_UPLOADS < maxuploads: - err_msg = 'Argument max-uploads must be an integer ' \ - 'between 0 and %d' % DEFAULT_MAX_UPLOADS - raise InvalidArgument('max-uploads', maxuploads, err_msg) - except ValueError: - err_msg = 'Provided max-uploads not an integer or within ' \ - 'integer range' - raise InvalidArgument('max-uploads', req.params['max-uploads'], - err_msg) + maxuploads = req.get_validated_param( + 'max-uploads', DEFAULT_MAX_UPLOADS, DEFAULT_MAX_UPLOADS) query = { 'format': 'json', @@ -296,39 +284,10 @@ class UploadController(Controller): upload_id = req.params['uploadId'] _check_upload_info(req, self.app, upload_id) - maxparts = DEFAULT_MAX_PARTS - part_num_marker = 0 - - if 'max-parts' in req.params: - try: - maxparts = int(req.params['max-parts']) - if maxparts < 0 or CONF.max_parts < maxparts: - err_msg = 'Argument max-parts must be an integer between 0 and' \ - ' %d' % CONF.max_parts - raise InvalidArgument('max-parts', - req.params['max-parts'], - err_msg) - except ValueError: - err_msg = 'Provided max-parts not an integer or within ' \ - 'integer range' - raise InvalidArgument('max-parts', req.params['max-parts'], - err_msg) - - if 'part-number-marker' in req.params: - try: - part_num_marker = int(req.params['part-number-marker']) - if part_num_marker < 0 or CONF.max_parts < part_num_marker: - err_msg = 'Argument part-number-marker must be an integer ' \ - 'between 0 and %d' % CONF.max_parts - raise InvalidArgument('part-number-marker', - req.params['part-number-marker'], - err_msg) - except ValueError: - err_msg = 'Provided part-number-marker not an integer or ' \ - 'within integer range' - raise InvalidArgument('part-number-marker', - req.params['part-number-marker'], - err_msg) + maxparts = req.get_validated_param( + 'max-parts', DEFAULT_MAX_PARTS, CONF.max_parts) + part_num_marker = req.get_validated_param( + 'part-number-marker', 0, CONF.max_parts) query = { 'format': 'json', diff --git a/swift3/request.py b/swift3/request.py index 368beb58..617c3b30 100644 --- a/swift3/request.py +++ b/swift3/request.py @@ -652,6 +652,24 @@ class Request(swob.Request): return self._get_response(app, method, container, obj, headers, body, query) + def get_validated_param(self, param, default, limit=None): + value = default + if param in self.params: + try: + value = int(self.params[param]) + if value < 0 or (limit is not None and limit < value): + err_msg = 'Argument %s must be an integer between 0 and' \ + ' %d' % (param, limit) + raise InvalidArgument(param, + self.params[param], + err_msg) + except ValueError: + err_msg = 'Provided %s not an integer or within ' \ + 'integer range' % param + raise InvalidArgument(param, self.params[param], + err_msg) + return value + class S3AclRequest(Request): """ diff --git a/swift3/test/unit/test_request.py b/swift3/test/unit/test_request.py index e723c012..4b6fd4c3 100644 --- a/swift3/test/unit/test_request.py +++ b/swift3/test/unit/test_request.py @@ -24,6 +24,7 @@ from swift3.test.unit.test_middleware import Swift3TestCase from swift3.cfg import CONF from swift3.request import Request as S3_Request from swift3.request import S3AclRequest +from swift3.response import InvalidArgument Fake_ACL_MAP = { @@ -160,6 +161,24 @@ class TestRequest(Swift3TestCase): permission = args[1] self.assertEqual(permission, 'READ') + def test_get_validate_param(self): + def create_s3request_with_param(param, value): + req = Request.blank( + '/bucket?%s=%s' % (param, value), + environ={'REQUEST_METHOD': 'GET'}, + headers={'Authorization': 'AWS test:tester:hmac'}) + return S3_Request(req.environ, True) + + s3req = create_s3request_with_param('max-keys', '1') + + # a param in the range + self.assertEquals(s3req.get_validated_param('max-keys', 1000, 1000), 1) + self.assertEquals(s3req.get_validated_param('max-keys', 0, 1), 1) + + # a param in the out of the range + with self.assertRaises(InvalidArgument): + s3req.get_validated_param('max-keys', 0, 0) + if __name__ == '__main__': unittest.main()