From 6194dcb74b71c5131a74ea81133fddccb95e8dcd Mon Sep 17 00:00:00 2001 From: Lars van de Kerkhof Date: Wed, 18 Mar 2020 12:05:22 +0100 Subject: [PATCH] Do not download entire search index on search terms with no hits When a slice is specifically set to [0:0], which would yield an empty list, instead wagtail will clear the slice, effectively downloading the entire search index. This scenario happens when a search term does not return any results. So every time you enter a search term that has no results, the entire search index is downloaded, you don't really feel it because wagtail only downloads the pk values, and it is unlikely a site has millions of pages, but it is still very inefficient and unwanted. --- CHANGELOG.txt | 1 + docs/releases/2.10.rst | 1 + wagtail/search/backends/base.py | 4 ++-- wagtail/search/tests/test_page_search.py | 11 +++++++++++ 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index b7ebcb47d5..958043e482 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -31,6 +31,7 @@ Changelog * Add reference documentation for `wagtail.contrib.redirects` (LB (Ben Johnston)) * `bulk_delete` page permission is no longer required to move pages, even if those pages have children (Robert Rollins, LB (Ben Johnston)) * Add `after_edit_snippet`, `after_create_snippet` and `after_delete_snippet` hooks and documentation (Kalob Taulien) + * Improve performance of empty search results by avoiding downloading the entire search index in these scenarios (Lars van de Kerkhof, Coen van der Kamp) * Fix: Support IPv6 domain (Alex Gleason, Coen van der Kamp) * Fix: Ensure link to add a new user works when no users are visible in the users list (LB (Ben Johnston)) * Fix: `AbstractEmailForm` saved submission fields are now aligned with the email content fields, `form.cleaned_data` will be used instead of `form.fields` (Haydn Greatnews) diff --git a/docs/releases/2.10.rst b/docs/releases/2.10.rst index 5a7bda79d7..ed8f543573 100644 --- a/docs/releases/2.10.rst +++ b/docs/releases/2.10.rst @@ -40,6 +40,7 @@ Other features * Add reference documentation for ``wagtail.contrib.redirects`` See :ref:`redirects`. (LB (Ben Johnston)) * ``bulk_delete`` page permission is no longer required to move pages, even if those pages have children (Robert Rollins, LB (Ben Johnston)) * Add ``after_edit_snippet``, ``after_create_snippet`` and ``after_delete_snippet`` hooks and documentation (Kalob Taulien) + * Improve performance of empty search results by avoiding downloading the entire search index in these scenarios (Lars van de Kerkhof, Coen van der Kamp) Bug fixes diff --git a/wagtail/search/backends/base.py b/wagtail/search/backends/base.py index 7bc0da31be..9e73f1199b 100644 --- a/wagtail/search/backends/base.py +++ b/wagtail/search/backends/base.py @@ -220,8 +220,8 @@ class BaseSearchResults: if isinstance(key, slice): # Set limits - start = int(key.start) if key.start else None - stop = int(key.stop) if key.stop else None + start = int(key.start) if key.start is not None else None + stop = int(key.stop) if key.stop is not None else None new._set_limits(start, stop) # Copy results cache diff --git a/wagtail/search/tests/test_page_search.py b/wagtail/search/tests/test_page_search.py index 53ac110ffa..38834a17d9 100644 --- a/wagtail/search/tests/test_page_search.py +++ b/wagtail/search/tests/test_page_search.py @@ -5,6 +5,7 @@ from django.test import TestCase from wagtail.core.models import Page from wagtail.search.backends import get_search_backend +from wagtail.search.backends.base import BaseSearchQueryCompiler, BaseSearchResults class PageSearchTests: @@ -47,3 +48,13 @@ class PageSearchTests: for backend_name in settings.WAGTAILSEARCH_BACKENDS.keys(): test_name = str("Test%sBackend" % backend_name.title()) globals()[test_name] = type(test_name, (PageSearchTests, TestCase,), {'backend_name': backend_name}) + + +class TestBaseSearchResults(TestCase): + + def test_get_item_no_results(self): + # Ensure that, if there are no results, we do not attempt to get the entire search index. + base_search_results = BaseSearchResults("BackendIrrelevant", BaseSearchQueryCompiler) + obj = base_search_results[0:0] + self.assertEqual(obj.start, 0) + self.assertEqual(obj.stop, 0)