Skip to content

Commit

Permalink
datastore_search limit has a default which is NOW configurable with c…
Browse files Browse the repository at this point in the history
…kan.datastore.search.rows_max
  • Loading branch information
David Read committed Nov 23, 2018
1 parent 5917388 commit e6d6361
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 34 deletions.
1 change: 1 addition & 0 deletions ckanext/datastore/backend/postgres.py
Expand Up @@ -1702,6 +1702,7 @@ def datastore_search(self, context, data_dict, fields_types, query_dict):
else:
field_ids = fields_types.keys()

# add default limit here just in case - already defaulted in the schema
limit = data_dict.get('limit', 100)
offset = data_dict.get('offset', 0)

Expand Down
4 changes: 3 additions & 1 deletion ckanext/datastore/logic/action.py
Expand Up @@ -392,7 +392,9 @@ def datastore_search(context, data_dict):
:param language: language of the full text query
(optional, default: english)
:type language: string
:param limit: maximum number of rows to return (optional, default: 100)
:param limit: maximum number of rows to return
(optional, default: ``100``, upper limit: ``10000`` unless set in
site's configuration ``ckan.datastore.search.rows_max``)
:type limit: int
:param offset: offset this number of rows (optional)
:type offset: int
Expand Down
6 changes: 5 additions & 1 deletion ckanext/datastore/logic/schema.py
Expand Up @@ -21,6 +21,8 @@
OneOf = get_validator('OneOf')
unicode_only = get_validator('unicode_only')
default = get_validator('default')
natural_number_validator = get_validator('natural_number_validator')
limit_to_configured_maximum = get_validator('limit_to_configured_maximum')


def rename(old, new):
Expand Down Expand Up @@ -161,7 +163,9 @@ def datastore_search_schema():
'plain': [ignore_missing, boolean_validator],
'filters': [ignore_missing, json_validator],
'language': [ignore_missing, text_type],
'limit': [ignore_missing, int_validator],
'limit': [default(100), natural_number_validator,
limit_to_configured_maximum('ckan.datastore.search.rows_max',
10000)],
'offset': [ignore_missing, int_validator],
'fields': [ignore_missing, list_of_strings_or_string],
'sort': [ignore_missing, list_of_strings_or_string],
Expand Down
110 changes: 78 additions & 32 deletions ckanext/datastore/tests/test_search.py
Expand Up @@ -10,6 +10,7 @@
import ckan.plugins as p
import ckan.lib.create_test_data as ctd
import ckan.model as model
import ckan.logic as logic
import ckan.tests.legacy as tests

from ckan.common import config
Expand Down Expand Up @@ -118,6 +119,83 @@ def test_search_without_total(self):
result = helpers.call_action('datastore_search', **search_data)
assert 'total' not in result

def test_search_limit(self):
resource = factories.Resource()
data = {
'resource_id': resource['id'],
'force': True,
'records': [
{'the year': 2014},
{'the year': 2013},
],
}
result = helpers.call_action('datastore_create', **data)
search_data = {
'resource_id': resource['id'],
'limit': 1
}
result = helpers.call_action('datastore_search', **search_data)
assert_equals(result['total'], 2)
assert_equals(result['records'], [{u'the year': 2014, u'_id': 1}])

def test_search_limit_invalid(self):
resource = factories.Resource()
data = {
'resource_id': resource['id'],
'force': True,
'records': [
{'the year': 2014},
{'the year': 2013},
],
}
helpers.call_action('datastore_create', **data)
search_data = {
'resource_id': resource['id'],
'limit': 'bad'
}
with assert_raises(logic.ValidationError) as exc:
helpers.call_action('datastore_search', **search_data)
assert_equals(exc.error_msg, {'limit': [u'Invalid integer']})

def test_search_limit_invalid_negative(self):
resource = factories.Resource()
data = {
'resource_id': resource['id'],
'force': True,
'records': [
{'the year': 2014},
{'the year': 2013},
],
}
helpers.call_action('datastore_create', **data)
search_data = {
'resource_id': resource['id'],
'limit': -1
}
with assert_raises(logic.ValidationError) as exc:
helpers.call_action('datastore_search', **search_data)
assert_equals(exc.error_msg, {'limit': [u'Invalid integer']})

@helpers.change_config('ckan.datastore.search.rows_max', '1')
def test_search_limit_config_default(self):
resource = factories.Resource()
data = {
'resource_id': resource['id'],
'force': True,
'records': [
{'the year': 2014},
{'the year': 2013},
],
}
result = helpers.call_action('datastore_create', **data)
search_data = {
'resource_id': resource['id'],
# limit not specified - leaving to the configured default of 1
}
result = helpers.call_action('datastore_search', **search_data)
assert_equals(result['total'], 2)
assert_equals(result['records'], [{u'the year': 2014, u'_id': 1}])


class TestDatastoreSearchLegacyTests(DatastoreLegacyTestBase):
sysadmin_user = None
Expand Down Expand Up @@ -418,38 +496,6 @@ def test_search_invalid(self):
assert u'f\xfc\xfc' in error_msg, \
'Expected "{0}" to contain "{1}"'.format(error_msg, u'f\xfc\xfc')

def test_search_limit(self):
data = {'resource_id': self.data['resource_id'],
'limit': 1}
postparams = '%s=1' % json.dumps(data)
auth = {'Authorization': str(self.normal_user.apikey)}
res = self.app.post('/api/action/datastore_search', params=postparams,
extra_environ=auth)
res_dict = json.loads(res.body)
assert res_dict['success'] is True
result = res_dict['result']
assert result['total'] == 2
assert result['records'] == [self.expected_records[0]]

def test_search_invalid_limit(self):
data = {'resource_id': self.data['resource_id'],
'limit': 'bad'}
postparams = '%s=1' % json.dumps(data)
auth = {'Authorization': str(self.normal_user.apikey)}
res = self.app.post('/api/action/datastore_search', params=postparams,
extra_environ=auth, status=409)
res_dict = json.loads(res.body)
assert res_dict['success'] is False

data = {'resource_id': self.data['resource_id'],
'limit': -1}
postparams = '%s=1' % json.dumps(data)
auth = {'Authorization': str(self.normal_user.apikey)}
res = self.app.post('/api/action/datastore_search', params=postparams,
extra_environ=auth, status=409)
res_dict = json.loads(res.body)
assert res_dict['success'] is False

def test_search_offset(self):
data = {'resource_id': self.data['resource_id'],
'limit': 1,
Expand Down
15 changes: 15 additions & 0 deletions doc/maintaining/configuration.rst
Expand Up @@ -285,6 +285,21 @@ Default value: ``True``
This option allows you to disable the datastore_search_sql action function, and
corresponding API endpoint if you do not wish it to be activated.

.. _ckan.datastore.search.rows_max:

ckan.datastore.search.rows_max
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Example::

ckan.datastore.search.rows_max = 1000000

Default value: ``10000``

Maximum allowed value for the number of rows returned by the datastore.
Specifically this limits ``datastore_search``'s ``limit`` parameter.
(``datastore_search_sql`` is not currently constrained.)

Site Settings
-------------

Expand Down

0 comments on commit e6d6361

Please sign in to comment.