From d28a38daf7cfc77b8963b2050eae1a7798a523ba Mon Sep 17 00:00:00 2001 From: David Read Date: Fri, 28 Sep 2018 18:14:53 +0100 Subject: [PATCH] Convert limits to validators. Using default_pagination_schema seemed to be a slip so I fixed. --- ckan/lib/navl/validators.py | 10 ++++++ ckan/logic/action/get.py | 34 ++++++--------------- ckan/logic/schema.py | 17 +++++++++-- ckan/tests/logic/action/test_get.py | 47 ++++++++++++++++------------- 4 files changed, 59 insertions(+), 49 deletions(-) diff --git a/ckan/lib/navl/validators.py b/ckan/lib/navl/validators.py index 6af970f1a11..6022722ce21 100644 --- a/ckan/lib/navl/validators.py +++ b/ckan/lib/navl/validators.py @@ -85,6 +85,16 @@ def callable(key, data, errors, context): return callable +def configured_default(config_name, default_value_if_not_configured): + '''When key is missing or value is an empty string or None, replace it with + a default value from config, or if that isn't set from the + default_value_if_not_configured.''' + + default_value = config.get(config_name) + if default_value is None: + default_value = default_value_if_not_configured + return default(default_value) + def ignore_missing(key, data, errors, context): '''If the key is missing from the data, ignore the rest of the key's schema. diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index e554617d476..06cd99f87dd 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -2482,10 +2482,7 @@ def user_activity_list(context, data_dict): raise logic.NotFound offset = data_dict.get('offset', 0) - limit = min( - int(data_dict.get('limit', config.get('ckan.activity_list_limit', 31))), - int(config.get('ckan.activity_list_limit_max', 100)) - ) + limit = data_dict['limit'] # defaulted, limited & made an int by schema _activity_objects = model.activity.user_activity_list(user.id, limit=limit, offset=offset) @@ -2527,10 +2524,7 @@ def package_activity_list(context, data_dict): raise logic.NotFound offset = int(data_dict.get('offset', 0)) - limit = min( - int(data_dict.get('limit', config.get('ckan.activity_list_limit', 31))), - int(config.get('ckan.activity_list_limit_max', 100)) - ) + limit = data_dict['limit'] # defaulted, limited & made an int by schema _activity_objects = model.activity.package_activity_list(package.id, limit=limit, offset=offset) @@ -2567,10 +2561,7 @@ def group_activity_list(context, data_dict): model = context['model'] group_id = data_dict.get('id') offset = data_dict.get('offset', 0) - limit = min( - int(data_dict.get('limit', config.get('ckan.activity_list_limit', 31))), - int(config.get('ckan.activity_list_limit_max', 100)) - ) + limit = data_dict['limit'] # defaulted, limited & made an int by schema # Convert group_id (could be id or name) into id. group_show = logic.get_action('group_show') @@ -2609,10 +2600,7 @@ def organization_activity_list(context, data_dict): model = context['model'] org_id = data_dict.get('id') offset = data_dict.get('offset', 0) - limit = min( - int(data_dict.get('limit', config.get('ckan.activity_list_limit', 31))), - int(config.get('ckan.activity_list_limit_max', 100)) - ) + limit = data_dict['limit'] # defaulted, limited & made an int by schema # Convert org_id (could be id or name) into id. org_show = logic.get_action('organization_show') @@ -2626,7 +2614,7 @@ def organization_activity_list(context, data_dict): return model_dictize.activity_list_dictize(activity_objects, context) -@logic.validate(logic.schema.default_pagination_schema) +@logic.validate(logic.schema.default_dashboard_activity_list_schema) def recently_changed_packages_activity_list(context, data_dict): '''Return the activity stream of all recently added or changed packages. @@ -2646,10 +2634,7 @@ def recently_changed_packages_activity_list(context, data_dict): # authorized to read. model = context['model'] offset = data_dict.get('offset', 0) - limit = min( - int(data_dict.get('limit', config.get('ckan.activity_list_limit', 31))), - int(config.get('ckan.activity_list_limit_max', 100)) - ) + limit = data_dict['limit'] # defaulted, limited & made an int by schema _activity_objects = model.activity.recently_changed_packages_activity_list( limit=limit, offset=offset) @@ -3309,7 +3294,7 @@ def _group_or_org_followee_list(context, data_dict, is_org=False): return [model_dictize.group_dictize(group, context) for group in groups] -@logic.validate(logic.schema.default_pagination_schema) +@logic.validate(logic.schema.default_dashboard_activity_list_schema) def dashboard_activity_list(context, data_dict): '''Return the authorized (via login or API key) user's dashboard activity stream. @@ -3337,8 +3322,7 @@ def dashboard_activity_list(context, data_dict): model = context['model'] user_id = model.User.get(context['user']).id offset = data_dict.get('offset', 0) - limit = int( - data_dict.get('limit', config.get('ckan.activity_list_limit', 31))) + limit = data_dict['limit'] # defaulted, limited & made an int by schema # FIXME: Filter out activities whose subject or object the user is not # authorized to read. @@ -3365,7 +3349,7 @@ def dashboard_activity_list(context, data_dict): return activity_dicts -@logic.validate(ckan.logic.schema.default_pagination_schema) +@logic.validate(ckan.logic.schema.default_dashboard_activity_list_schema) def dashboard_activity_list_html(context, data_dict): '''Return the authorized (via login or API key) user's dashboard activity stream as HTML. diff --git a/ckan/logic/schema.py b/ckan/logic/schema.py index d244d5ce8d8..7aa77fdd21d 100644 --- a/ckan/logic/schema.py +++ b/ckan/logic/schema.py @@ -601,16 +601,27 @@ def default_pagination_schema(ignore_missing, natural_number_validator): @validator_args -def default_dashboard_activity_list_schema(unicode_safe): +def default_dashboard_activity_list_schema( + configured_default, natural_number_validator, + limit_to_configured_maximum): schema = default_pagination_schema() - schema['id'] = [unicode_safe] + schema['limit'] = [ + configured_default('ckan.activity_list_limit', 31), + natural_number_validator, + limit_to_configured_maximum('ckan.activity_list_limit_max', 100)] return schema @validator_args -def default_activity_list_schema(not_missing, unicode_safe): +def default_activity_list_schema( + not_missing, unicode_safe, configured_default, + natural_number_validator, limit_to_configured_maximum): schema = default_pagination_schema() schema['id'] = [not_missing, unicode_safe] + schema['limit'] = [ + configured_default('ckan.activity_list_limit', 31), + natural_number_validator, + limit_to_configured_maximum('ckan.activity_list_limit_max', 100)] return schema diff --git a/ckan/tests/logic/action/test_get.py b/ckan/tests/logic/action/test_get.py index a6f6439af77..9c9ba74e6fa 100644 --- a/ckan/tests/logic/action/test_get.py +++ b/ckan/tests/logic/action/test_get.py @@ -2253,10 +2253,11 @@ class TestPackageActivityList(helpers.FunctionalTestBase): def _create_bulk_package_activities(self, count): dataset = factories.Dataset() from ckan import model - objs = [model.Activity( - user_id=None, object_id=dataset['id'], revision_id=None, - activity_type=None, data=None) - for i in range(count)] + objs = [ + model.Activity( + user_id=None, object_id=dataset['id'], revision_id=None, + activity_type=None, data=None) + for i in range(count)] model.Session.add_all(objs) model.repo.commit_and_remove() return dataset['id'] @@ -2285,10 +2286,11 @@ class TestUserActivityList(helpers.FunctionalTestBase): def _create_bulk_user_activities(self, count): user = factories.User() from ckan import model - objs = [model.Activity( - user_id=user['id'], object_id=None, revision_id=None, - activity_type=None, data=None) - for i in range(count)] + objs = [ + model.Activity( + user_id=user['id'], object_id=None, revision_id=None, + activity_type=None, data=None) + for i in range(count)] model.Session.add_all(objs) model.repo.commit_and_remove() return user['id'] @@ -2317,10 +2319,11 @@ class TestGroupActivityList(helpers.FunctionalTestBase): def _create_bulk_group_activities(self, count): group = factories.Group() from ckan import model - objs = [model.Activity( - user_id=None, object_id=group['id'], revision_id=None, - activity_type=None, data=None) - for i in range(count)] + objs = [ + model.Activity( + user_id=None, object_id=group['id'], revision_id=None, + activity_type=None, data=None) + for i in range(count)] model.Session.add_all(objs) model.repo.commit_and_remove() return group['id'] @@ -2349,10 +2352,11 @@ class TestOrganizationActivityList(helpers.FunctionalTestBase): def _create_bulk_org_activities(self, count): org = factories.Organization() from ckan import model - objs = [model.Activity( - user_id=None, object_id=org['id'], revision_id=None, - activity_type=None, data=None) - for i in range(count)] + objs = [ + model.Activity( + user_id=None, object_id=org['id'], revision_id=None, + activity_type=None, data=None) + for i in range(count)] model.Session.add_all(objs) model.repo.commit_and_remove() return org['id'] @@ -2380,10 +2384,11 @@ def test_limit_hits_max(self): class TestRecentlyChangedPackagesActivityList(helpers.FunctionalTestBase): def _create_bulk_package_activities(self, count): from ckan import model - objs = [model.Activity( - user_id=None, object_id=None, revision_id=None, - activity_type='new_package', data=None) - for i in range(count)] + objs = [ + model.Activity( + user_id=None, object_id=None, revision_id=None, + activity_type='new_package', data=None) + for i in range(count)] model.Session.add_all(objs) model.repo.commit_and_remove() @@ -2404,4 +2409,4 @@ def test_limit_hits_max(self): self._create_bulk_package_activities(9) results = logic.get_action('recently_changed_packages_activity_list')( {}, {'limit': '9'}) - eq(len(results), 7) # i.e. ckan.activity_list_limit_max \ No newline at end of file + eq(len(results), 7) # i.e. ckan.activity_list_limit_max