Convert limits to validators. Using default_pagination_schema seemed …

…to be a slip so I fixed.

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.

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.

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
 
 

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
+        eq(len(results), 7)  # i.e. ckan.activity_list_limit_max