diff --git a/ckan/lib/search/query.py b/ckan/lib/search/query.py index 73a31107544..0cd266304ad 100644 --- a/ckan/lib/search/query.py +++ b/ckan/lib/search/query.py @@ -247,7 +247,14 @@ def run(self, query): query['q'] = "*:*" # number of results - query['rows'] = min(1000, int(query.get('rows', 10))) + rows_to_return = min(1000, int(query.get('rows', 10))) + if rows_to_return > 0: + # #1683 Work around problem of last result being out of order + # in SOLR 1.4 + rows_to_query = rows_to_return + 1 + else: + rows_to_query = rows_to_return + query['rows'] = rows_to_query # order by score if no 'sort' term given order_by = query.get('sort') @@ -297,6 +304,9 @@ def run(self, query): self.count = response.get('numFound', 0) self.results = response.get('docs', []) + # #1683 Filter out the last row that is sometimes out of order + self.results = self.results[:rows_to_return] + # get any extras and add to 'extras' dict for result in self.results: extra_keys = filter(lambda x: x.startswith('extras_'), result.keys()) diff --git a/ckan/tests/functional/test_pagination.py b/ckan/tests/functional/test_pagination.py index adc95ddd9d9..436622d0469 100644 --- a/ckan/tests/functional/test_pagination.py +++ b/ckan/tests/functional/test_pagination.py @@ -1,8 +1,32 @@ +import re + +from nose.tools import assert_equal + from ckan.lib.create_test_data import CreateTestData import ckan.model as model from ckan.tests import TestController, url_for, setup_test_search_index -class TestPagination(TestController): +def scrape_search_results(response, object_type): + assert object_type in ('dataset', 'group', 'user') + results = re.findall('href="/%s/%s_(\d\d)"' % (object_type, object_type), + str(response)) + return results + +def test_scrape(): + html = ''' +
  • + user_00 +
    • + ... +
  • + user_01 +
    • + + ''' + res = scrape_search_results(html, 'user') + assert_equal(res, ['00', '01']) + +class TestPaginationPackage(TestController): @classmethod def setup_class(cls): setup_test_search_index() @@ -10,63 +34,103 @@ def setup_class(cls): # no. entities per page is hardcoded into the controllers, so # create enough of each here so that we can test pagination - cls.num_groups = 21 cls.num_packages_in_large_group = 51 - cls.num_users = 21 - groups = [u'group_%s' % str(i).zfill(2) for i in range(1, cls.num_groups)] - users = [u'user_%s' % str(i).zfill(2) for i in range(cls.num_users)] packages = [] for i in range(cls.num_packages_in_large_group): packages.append({ - 'name': u'package_%s' % str(i).zfill(2), + 'name': u'dataset_%s' % str(i).zfill(2), 'groups': u'group_00' }) - CreateTestData.create_arbitrary( - packages, extra_group_names=groups, extra_user_names = users, - ) + CreateTestData.create_arbitrary(packages) @classmethod def teardown_class(self): model.repo.rebuild_db() - - def test_search(self): + + def test_package_search_p1(self): res = self.app.get(url_for(controller='package', action='search', q='groups:group_00')) assert 'href="/dataset?q=groups%3Agroup_00&page=2"' in res - assert 'href="/dataset/package_00"' in res, res - assert 'href="/dataset/package_19"' in res, res + pkg_numbers = scrape_search_results(res, 'dataset') + assert_equal(['00', '01', '02', '03', '04', '05', '06', '07', '08', '09', '10', '11', '12', '13', '14', '15', '16', '17', '18', '19'], pkg_numbers) + def test_package_search_p2(self): res = self.app.get(url_for(controller='package', action='search', q='groups:group_00', page=2)) assert 'href="/dataset?q=groups%3Agroup_00&page=1"' in res - assert 'href="/dataset/package_20"' in res - assert 'href="/dataset/package_39"' in res + pkg_numbers = scrape_search_results(res, 'dataset') + assert_equal(['20', '21', '22', '23', '24', '25', '26', '27', '28', '29', '30', '31', '32', '33', '34', '35', '36', '37', '38', '39'], pkg_numbers) + + def test_group_read_p1(self): + res = self.app.get(url_for(controller='group', action='read', id='group_00')) + assert 'href="/group/group_00?page=2' in res + pkg_numbers = scrape_search_results(res, 'dataset') + assert_equal(['00', '01', '02', '03', '04', '05', '06', '07', '08', '09', '10', '11', '12', '13', '14', '15', '16', '17', '18', '19', '20', '21', '22', '23', '24', '25', '26', '27', '28', '29'], pkg_numbers) + + def test_group_read_p2(self): + res = self.app.get(url_for(controller='group', action='read', id='group_00', page=2)) + assert 'href="/group/group_00?page=1' in res + pkg_numbers = scrape_search_results(res, 'dataset') + assert_equal(['30', '31', '32', '33', '34', '35', '36', '37', '38', '39', '40', '41', '42', '43', '44', '45', '46', '47', '48', '49', '50'], pkg_numbers) + +class TestPaginationGroup(TestController): + @classmethod + def setup_class(cls): + # no. entities per page is hardcoded into the controllers, so + # create enough of each here so that we can test pagination + cls.num_groups = 21 + + groups = [u'group_%s' % str(i).zfill(2) for i in range(0, cls.num_groups)] + + CreateTestData.create_arbitrary( + [], extra_group_names=groups + ) + + @classmethod + def teardown_class(self): + model.repo.rebuild_db() def test_group_index(self): res = self.app.get(url_for(controller='group', action='index')) - assert 'href="/group?page=2"' in res - assert 'href="/group/group_19"' in res + assert 'href="/group?page=2"' in res, res + grp_numbers = scrape_search_results(res, 'group') + assert_equal(['00', '01', '02', '03', '04', '05', '06', '07', '08', '09', '10', '11', '12', '13', '14', '15', '16', '17', '18', '19'], grp_numbers) res = self.app.get(url_for(controller='group', action='index', page=2)) assert 'href="/group?page=1"' in res - assert 'href="/group/group_20"' in res + grp_numbers = scrape_search_results(res, 'group') + assert_equal(['20'], grp_numbers) - def test_group_read(self): - res = self.app.get(url_for(controller='group', action='read', id='group_00')) - assert 'href="/group/group_00?page=2' in res - assert 'href="/dataset/package_29"' in res +class TestPaginationUsers(TestController): + @classmethod + def setup_class(cls): + # Delete default user as it appears in the first page of results + model.User.by_name(u'logged_in').purge() + model.repo.commit_and_remove() - res = self.app.get(url_for(controller='group', action='read', id='group_00', page=2)) - assert 'href="/group/group_00?page=1' in res - assert 'href="/dataset/package_30"' in res + # no. entities per page is hardcoded into the controllers, so + # create enough of each here so that we can test pagination + cls.num_users = 21 + + users = [u'user_%s' % str(i).zfill(2) for i in range(cls.num_users)] + + CreateTestData.create_arbitrary( + [], extra_user_names = users, + ) + + @classmethod + def teardown_class(self): + model.repo.rebuild_db() def test_users_index(self): # allow for 2 extra users shown on user listing, 'logged_in' and 'visitor' res = self.app.get(url_for(controller='user', action='index')) - assert 'href="/user/user_18"' in res assert 'href="/user?q=&order_by=name&page=2"' in res + user_numbers = scrape_search_results(res, 'user') + assert_equal(['00', '01', '02', '03', '04', '05', '06', '07', '08', '09', '10', '11', '12', '13', '14', '15', '16', '17', '18', '19'], user_numbers) res = self.app.get(url_for(controller='user', action='index', page=2)) - assert 'href="/user/user_20"' in res assert 'href="/user?q=&order_by=name&page=1"' in res + user_numbers = scrape_search_results(res, 'user') + assert_equal(['20'], user_numbers)