Merge pull request #4484 from ckan/4480-limits-v2

Add limits for action functions v2

CHANGELOG.rst

@@ -14,6 +14,11 @@ Minor changes:
 
  * For navl schemas, the 'default' validator no longer applies the default when
    the value is False, 0, [] or {} (#4448)
+ * If you've customized the schema for package_search, you'll need to add to it
+   the limiting of ``row``, as per default_package_search_schema now does (#4484)
+ * Several logic functions now have new limits to how many items can be
+   returned, notably ``group_list`` and ``organization_list`` when
+   ``all_fields=true``. These are all configurable. (#4484)
 
 Bugfixes:
 

ckan/controllers/api.py

@@ -22,7 +22,7 @@
 
 from ckan.views import identify_user
 
-from ckan.common import _, c, request, response
+from ckan.common import _, c, request, response, config
 
 
 log = logging.getLogger(__name__)

ckan/lib/dictization/model_dictize.py

@@ -388,11 +388,12 @@ def get_packages_for_this_group(group_, just_the_count=False):
                     q['include_private'] = True
 
             if not just_the_count:
-                # Is there a packages limit in the context?
+                # package_search limits 'rows' anyway, so this is only if you
+                # want even fewer
                 try:
                     packages_limit = context['limits']['packages']
                 except KeyError:
-                    q['rows'] = 1000  # Only the first 1000 datasets are returned
+                    del q['rows']  # leave it to package_search to limit it
                 else:
                     q['rows'] = packages_limit
 

ckan/lib/navl/validators.py

@@ -4,7 +4,7 @@
 
 import ckan.lib.navl.dictization_functions as df
 
-from ckan.common import _, json
+from ckan.common import _, json, config
 
 missing = df.missing
 StopOnError = df.StopOnError
@@ -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.
@@ -163,3 +173,18 @@ def unicode_safe(value):
             return text_type(value)
         except Exception:
             return u'\N{REPLACEMENT CHARACTER}'
+
+def limit_to_configured_maximum(config_option, default_limit):
+    '''
+    If the value is over a limit, it changes it to the limit. The limit is
+    defined by a configuration option, or if that is not set, a given int
+    default_limit.
+    '''
+    def callable(key, data, errors, context):
+
+        value = data.get(key)
+        limit = int(config.get(config_option, default_limit))
+        if value > limit:
+            data[key] = limit
+
+    return callable

ckan/lib/search/query.py

@@ -312,7 +312,9 @@ def run(self, query, permission_labels=None, **kwargs):
             query['q'] = "*:*"
 
         # number of results
-        rows_to_return = min(1000, int(query.get('rows', 10)))
+        rows_to_return = int(query.get('rows', 10))
+        # query['rows'] should be a defaulted int, due to schema, but make
+        # certain, for legacy tests
         if rows_to_return > 0:
             # #1683 Work around problem of last result being out of order
             #       in SOLR 1.4

ckan/logic/action/get.py

@@ -342,6 +342,15 @@ def _group_or_org_list(context, data_dict, is_org=False):
 
     all_fields = asbool(data_dict.get('all_fields', None))
 
+    if all_fields:
+        # all_fields is really computationally expensive, so need a tight limit
+        max_limit = config.get(
+            'ckan.group_and_organization_list_all_fields_max', 25)
+    else:
+        max_limit = config.get('ckan.group_and_organization_list_max', 1000)
+    if limit is None or limit > max_limit:
+        limit = max_limit
+
     # order_by deprecated in ckan 1.8
     # if it is supplied and sort isn't use order_by and raise a warning
     order_by = data_dict.get('order_by', '')
@@ -438,9 +447,11 @@ def group_list(context, data_dict):
         "name asc" string of field name and sort-order. The allowed fields are
         'name', 'package_count' and 'title'
     :type sort: string
-    :param limit: if given, the list of groups will be broken into pages of
-        at most ``limit`` groups per page and only one page will be returned
-        at a time (optional)
+    :param limit: the maximum number of groups returned (optional)
+        Default: ``1000`` when all_fields=false unless set in site's
+        configuration ``ckan.group_and_organization_list_max``
+        Default: ``25`` when all_fields=true unless set in site's
+        configuration ``ckan.group_and_organization_list_all_fields_max``
     :type limit: int
     :param offset: when ``limit`` is given, the offset to start
         returning groups from
@@ -487,9 +498,11 @@ def organization_list(context, data_dict):
         "name asc" string of field name and sort-order. The allowed fields are
         'name', 'package_count' and 'title'
     :type sort: string
-    :param limit: if given, the list of organizations will be broken into pages
-        of at most ``limit`` organizations per page and only one page will be
-        returned at a time (optional)
+    :param limit: the maximum number of organizations returned (optional)
+        Default: ``1000`` when all_fields=false unless set in site's
+        configuration ``ckan.group_and_organization_list_max``
+        Default: ``25`` when all_fields=true unless set in site's
+        configuration ``ckan.group_and_organization_list_all_fields_max``
     :type limit: int
     :param offset: when ``limit`` is given, the offset to start
         returning organizations from
@@ -1696,8 +1709,9 @@ def package_search(context, data_dict):
         documentation, this is a comma-separated string of field names and
         sort-orderings.
     :type sort: string
-    :param rows: the number of matching rows to return. There is a hard limit
-        of 1000 datasets per query.
+    :param rows: the maximum number of matching rows (datasets) to return.
+        (optional, default: ``10``, upper limit: ``1000`` unless set in
+        site's configuration ``ckan.search.rows_max``)
     :type rows: int
     :param start: the offset in the complete result for where the set of
         returned datasets should begin.
@@ -2463,8 +2477,9 @@ def user_activity_list(context, data_dict):
         (optional, default: ``0``)
     :type offset: int
     :param limit: the maximum number of activities to return
-        (optional, default: ``31``, the default value is configurable via the
-        ckan.activity_list_limit setting)
+        (optional, default: ``31`` unless set in site's configuration
+        ``ckan.activity_list_limit``, upper limit: ``100`` unless set in
+        site's configuration ``ckan.activity_list_limit_max``)
     :type limit: int
 
     :rtype: list of dictionaries
@@ -2482,8 +2497,7 @@ def user_activity_list(context, data_dict):
         raise logic.NotFound
 
     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
 
     _activity_objects = model.activity.user_activity_list(user.id, limit=limit,
             offset=offset)
@@ -2505,8 +2519,9 @@ def package_activity_list(context, data_dict):
         (optional, default: ``0``)
     :type offset: int
     :param limit: the maximum number of activities to return
-        (optional, default: ``31``, the default value is configurable via the
-        ckan.activity_list_limit setting)
+        (optional, default: ``31`` unless set in site's configuration
+        ``ckan.activity_list_limit``, upper limit: ``100`` unless set in
+        site's configuration ``ckan.activity_list_limit_max``)
     :type limit: int
 
     :rtype: list of dictionaries
@@ -2524,8 +2539,7 @@ def package_activity_list(context, data_dict):
         raise logic.NotFound
 
     offset = int(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
 
     _activity_objects = model.activity.package_activity_list(package.id,
             limit=limit, offset=offset)
@@ -2547,8 +2561,9 @@ def group_activity_list(context, data_dict):
         (optional, default: ``0``)
     :type offset: int
     :param limit: the maximum number of activities to return
-        (optional, default: ``31``, the default value is configurable via the
-        ckan.activity_list_limit setting)
+        (optional, default: ``31`` unless set in site's configuration
+        ``ckan.activity_list_limit``, upper limit: ``100`` unless set in
+        site's configuration ``ckan.activity_list_limit_max``)
     :type limit: int
 
     :rtype: list of dictionaries
@@ -2561,8 +2576,7 @@ def group_activity_list(context, data_dict):
     model = context['model']
     group_id = data_dict.get('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
 
     # Convert group_id (could be id or name) into id.
     group_show = logic.get_action('group_show')
@@ -2582,6 +2596,14 @@ def organization_activity_list(context, data_dict):
 
     :param id: the id or name of the organization
     :type id: string
+    :param offset: where to start getting activity items from
+        (optional, default: ``0``)
+    :type offset: int
+    :param limit: the maximum number of activities to return
+        (optional, default: ``31`` unless set in site's configuration
+        ``ckan.activity_list_limit``, upper limit: ``100`` unless set in
+        site's configuration ``ckan.activity_list_limit_max``)
+    :type limit: int
 
     :rtype: list of dictionaries
 
@@ -2593,8 +2615,7 @@ def organization_activity_list(context, data_dict):
     model = context['model']
     org_id = data_dict.get('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
 
     # Convert org_id (could be id or name) into id.
     org_show = logic.get_action('organization_show')
@@ -2608,16 +2629,17 @@ 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.
 
     :param offset: where to start getting activity items from
         (optional, default: ``0``)
     :type offset: int
     :param limit: the maximum number of activities to return
-        (optional, default: ``31``, the default value is configurable via the
-        ckan.activity_list_limit setting)
+        (optional, default: ``31`` unless set in site's configuration
+        ``ckan.activity_list_limit``, upper limit: ``100`` unless set in
+        site's configuration ``ckan.activity_list_limit_max``)
     :type limit: int
 
     :rtype: list of dictionaries
@@ -2627,8 +2649,7 @@ def recently_changed_packages_activity_list(context, data_dict):
     # authorized to read.
     model = context['model']
     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
 
     _activity_objects = model.activity.recently_changed_packages_activity_list(
             limit=limit, offset=offset)
@@ -2667,8 +2688,9 @@ def user_activity_list_html(context, data_dict):
         (optional, default: ``0``)
     :type offset: int
     :param limit: the maximum number of activities to return
-        (optional, default: ``31``, the default value is configurable via the
-        ckan.activity_list_limit setting)
+        (optional, default: ``31`` unless set in site's configuration
+        ``ckan.activity_list_limit``, upper limit: ``100`` unless set in
+        site's configuration ``ckan.activity_list_limit_max``)
     :type limit: int
 
     :rtype: string
@@ -2698,8 +2720,9 @@ def package_activity_list_html(context, data_dict):
         (optional, default: ``0``)
     :type offset: int
     :param limit: the maximum number of activities to return
-        (optional, default: ``31``, the default value is configurable via the
-        ckan.activity_list_limit setting)
+        (optional, default: ``31`` unless set in site's configuration
+        ``ckan.activity_list_limit``, upper limit: ``100`` unless set in
+        site's configuration ``ckan.activity_list_limit_max``)
     :type limit: int
 
     :rtype: string
@@ -2729,8 +2752,9 @@ def group_activity_list_html(context, data_dict):
         (optional, default: ``0``)
     :type offset: int
     :param limit: the maximum number of activities to return
-        (optional, default: ``31``, the default value is configurable via the
-        ckan.activity_list_limit setting)
+        (optional, default: ``31`` unless set in site's configuration
+        ``ckan.activity_list_limit``, upper limit: ``100`` unless set in
+        site's configuration ``ckan.activity_list_limit_max``)
     :type limit: int
 
     :rtype: string
@@ -2756,6 +2780,14 @@ def organization_activity_list_html(context, data_dict):
 
     :param id: the id or name of the organization
     :type id: string
+    :param offset: where to start getting activity items from
+        (optional, default: ``0``)
+    :type offset: int
+    :param limit: the maximum number of activities to return
+        (optional, default: ``31`` unless set in site's configuration
+        ``ckan.activity_list_limit``, upper limit: ``100`` unless set in
+        site's configuration ``ckan.activity_list_limit_max``)
+    :type limit: int
 
     :rtype: string
 
@@ -2784,8 +2816,9 @@ def recently_changed_packages_activity_list_html(context, data_dict):
         (optional, default: ``0``)
     :type offset: int
     :param limit: the maximum number of activities to return
-        (optional, default: ``31``, the default value is configurable via the
-        ckan.activity_list_limit setting)
+        (optional, default: ``31`` unless set in site's configuration
+        ``ckan.activity_list_limit``, upper limit: ``100`` unless set in
+        site's configuration ``ckan.activity_list_limit_max``)
     :type limit: int
 
     :rtype: string
@@ -3276,7 +3309,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.
@@ -3292,8 +3325,9 @@ def dashboard_activity_list(context, data_dict):
         (optional, default: ``0``)
     :type offset: int
     :param limit: the maximum number of activities to return
-        (optional, default: ``31``, the default value is configurable via the
-        :ref:`ckan.activity_list_limit` setting)
+        (optional, default: ``31`` unless set in site's configuration
+        ``ckan.activity_list_limit``, upper limit: ``100`` unless set in
+        site's configuration ``ckan.activity_list_limit_max``)
     :type limit: int
 
     :rtype: list of activity dictionaries
@@ -3304,8 +3338,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.
@@ -3332,7 +3365,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.