From 646e551881c54c54b120f68f36205bb7207b53fc Mon Sep 17 00:00:00 2001 From: Ian Murray Date: Thu, 28 Jun 2012 15:16:24 +0100 Subject: [PATCH 1/4] [#2439] Allow query parameter to be a list of strings As well as a single string. --- ckan/logic/action/get.py | 8 ++++---- ckan/tests/lib/test_tag_search.py | 5 +++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index a8a6fa9c506..f954b73af7d 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -1245,10 +1245,10 @@ def resource_search(context, data_dict): def _tag_search(context, data_dict): model = context['model'] - query = data_dict.get('query') or data_dict.get('q') - if query: - query = query.strip() - terms = [query] if query else [] + terms = data_dict.get('query') or data_dict.get('q') or [] + if isinstance(terms, basestring): + terms = [terms] + terms = [ t.strip() for t in terms if t.strip() ] fields = data_dict.get('fields', {}) offset = data_dict.get('offset') diff --git a/ckan/tests/lib/test_tag_search.py b/ckan/tests/lib/test_tag_search.py index 9047e13e5aa..d6f4177675e 100644 --- a/ckan/tests/lib/test_tag_search.py +++ b/ckan/tests/lib/test_tag_search.py @@ -26,6 +26,11 @@ def test_good_search_query(self): assert 'russian' in result['results'], result assert 'tolstoy' in result['results'], result + def test_good_search_queries(self): + result = search.query_for(model.Tag).run(query=[u'ru', u's']) + assert result['count'] == 1, result + assert 'russian' in result['results'], result + def test_bad_search_query(self): result = search.query_for(model.Tag).run(query=u'asdf') assert result['count'] == 0, result From 903bd62d06392eb211925b4f63d2ee89570bbb78 Mon Sep 17 00:00:00 2001 From: Ian Murray Date: Thu, 28 Jun 2012 15:27:23 +0100 Subject: [PATCH 2/4] [#2439] Mark fields parameter as deprecated ... in tag_search and tag_autocomplete. --- ckan/lib/search/query.py | 9 ++++++++- ckan/logic/action/get.py | 12 ++++++++---- ckan/tests/logic/test_tag.py | 10 ++++++++++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/ckan/lib/search/query.py b/ckan/lib/search/query.py index a3b7adf35fe..feef9622ec1 100644 --- a/ckan/lib/search/query.py +++ b/ckan/lib/search/query.py @@ -156,10 +156,17 @@ def run(self, query=[], fields={}, options=None, **kwargs): else: options.update(kwargs) + if isinstance(query, basestring): + query = [query] + + query = query[:] + for field, value in fields.items(): + if field in ('tag', 'tags'): + query.append(value) + context = {'model': model, 'session': model.Session} data_dict = { 'query': query, - 'fields': fields, 'offset': options.get('offset'), 'limit': options.get('limit') } diff --git a/ckan/logic/action/get.py b/ckan/logic/action/get.py index f954b73af7d..fce9a0c3392 100644 --- a/ckan/logic/action/get.py +++ b/ckan/logic/action/get.py @@ -1250,6 +1250,10 @@ def _tag_search(context, data_dict): terms = [terms] terms = [ t.strip() for t in terms if t.strip() ] + if 'fields' in data_dict: + log.warning('"fields" parameter is deprecated. ' + 'Use the "query" parameter instead') + fields = data_dict.get('fields', {}) offset = data_dict.get('offset') limit = data_dict.get('limit') @@ -1293,12 +1297,12 @@ def tag_search(context, data_dict): searched. If the ``vocabulary_id`` argument is given then only tags belonging to that vocabulary will be searched instead. - :param query: the string to search for - :type query: string + :param query: the string(s) to search for + :type query: string or list of strings :param vocabulary_id: the id or name of the tag vocabulary to search in (optional) :type vocabulary_id: string - :param fields: + :param fields: deprecated :type fields: dictionary :param limit: the maximum number of tags to return :type limit: int @@ -1334,7 +1338,7 @@ def tag_autocomplete(context, data_dict): :param vocabulary_id: the id or name of the tag vocabulary to search in (optional) :type vocabulary_id: string - :param fields: + :param fields: deprecated :type fields: dictionary :param limit: the maximum number of tags to return :type limit: int diff --git a/ckan/tests/logic/test_tag.py b/ckan/tests/logic/test_tag.py index 2e96835abad..1f710ab6563 100644 --- a/ckan/tests/logic/test_tag.py +++ b/ckan/tests/logic/test_tag.py @@ -202,6 +202,16 @@ def test_15a_tag_search_with_one_match(self): assert len(tag_dicts) == 1 assert tag_dicts[0]['name'] == 'russian' + def test_15a_tag_search_with_one_match_using_fields_parameter(self): + paramd = {'fields': {'tags': 'russ'} } + params = json.dumps(paramd) + res = self.app.post('/api/action/tag_search', params=params) + assert res.json['success'] is True + assert res.json['result']['count'] == 1 + tag_dicts = res.json['result']['results'] + assert len(tag_dicts) == 1 + assert tag_dicts[0]['name'] == 'russian' + def test_15a_tag_search_with_many_matches(self): paramd = {'q': 'tol' } params = json.dumps(paramd) From 85310d44b79ddab590e687c23441b7e244243171 Mon Sep 17 00:00:00 2001 From: Ian Murray Date: Thu, 28 Jun 2012 18:05:33 +0100 Subject: [PATCH 3/4] [#2439] Update to docs --- doc/apiv3.rst | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/doc/apiv3.rst b/doc/apiv3.rst index a76c6be6e00..f9b5a580952 100644 --- a/doc/apiv3.rst +++ b/doc/apiv3.rst @@ -81,17 +81,13 @@ will result in the following parameters being sent to the This interface is *slightly* more limited than the POST interface because it doesn't allow passing nested dicts into the action be accessed. As a -consequence of this, currently the *resource_search*, *tag_search* and -*tag_autocomplete* actions are **limited** in their functionality. +consequence of this, currently the *resource_search* action is **limited** in +its functionality when accessed with a GET request. `resource_search`: This action is not currently usable via a GET request as it relies upon a nested dict of fields. -`tag_search` and `tag_autocomplete`: - The `fields` argument is not available when accessing this action with a - GET request. - Also, it is worth bearing this limitation in mind when creating your own actions via the `IActions` interface. From 54e8722bdb38ba66e9566c09c44c25edeb29aa07 Mon Sep 17 00:00:00 2001 From: Ian Murray Date: Fri, 29 Jun 2012 12:28:52 +0100 Subject: [PATCH 4/4] [#2439] Sanitise method parameters. And comment reason for making the list copy. Response to feedback from code-review. --- ckan/lib/search/query.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ckan/lib/search/query.py b/ckan/lib/search/query.py index feef9622ec1..39233423aeb 100644 --- a/ckan/lib/search/query.py +++ b/ckan/lib/search/query.py @@ -150,7 +150,10 @@ def run(self, query=None, terms=[], fields={}, facet_by=[], options=None, **kwar class TagSearchQuery(SearchQuery): """Search for tags.""" - def run(self, query=[], fields={}, options=None, **kwargs): + def run(self, query=None, fields=None, options=None, **kwargs): + query = [] if query is None else query + fields = {} if fields is None else fields + if options is None: options = QueryOptions(**kwargs) else: @@ -159,7 +162,7 @@ def run(self, query=[], fields={}, options=None, **kwargs): if isinstance(query, basestring): query = [query] - query = query[:] + query = query[:] # don't alter caller's query list. for field, value in fields.items(): if field in ('tag', 'tags'): query.append(value)