From 048a8b1162417331091e338e491ee15ec14eadc8 Mon Sep 17 00:00:00 2001 From: ulixxe <1309337+ulixxe@users.noreply.github.com> Date: Sun, 15 Aug 2021 09:28:40 +0200 Subject: [PATCH] improve tag query performance (#142) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * changed query on tag search for speedup related to issues #112 and #141 * fix tests and only conditionally append tag filter * add bookmark tags query tests * reuse bookmark queries for tag queries * fix tag query test setup Co-authored-by: Sascha Ißbrücker --- bookmarks/queries.py | 40 ++++++--------------------------- bookmarks/tests/test_queries.py | 34 ++++++++++++++++++++++++---- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/bookmarks/queries.py b/bookmarks/queries.py index bd4ab12..17a7a35 100644 --- a/bookmarks/queries.py +++ b/bookmarks/queries.py @@ -62,43 +62,17 @@ def _base_bookmarks_query(user: User, query_string: str) -> QuerySet: def query_bookmark_tags(user: User, query_string: str) -> QuerySet: - return _base_bookmark_tags_query(user, query_string) \ - .filter(bookmark__is_archived=False) \ - .distinct() + bookmarks_query = query_bookmarks(user, query_string) + + query_set = Tag.objects.filter(bookmark__in=bookmarks_query) + + return query_set.distinct() def query_archived_bookmark_tags(user: User, query_string: str) -> QuerySet: - return _base_bookmark_tags_query(user, query_string) \ - .filter(bookmark__is_archived=True) \ - .distinct() + bookmarks_query = query_archived_bookmarks(user, query_string) - -def _base_bookmark_tags_query(user: User, query_string: str) -> QuerySet: - query_set = Tag.objects - - # Filter for user - query_set = query_set.filter(owner=user) - - # Only show tags which have bookmarks - query_set = query_set.filter(bookmark__isnull=False) - - # Split query into search terms and tags - query = _parse_query_string(query_string) - - # Filter for search terms and tags - for term in query['search_terms']: - query_set = query_set.filter( - Q(bookmark__title__contains=term) - | Q(bookmark__description__contains=term) - | Q(bookmark__website_title__contains=term) - | Q(bookmark__website_description__contains=term) - | Q(bookmark__url__contains=term) - ) - - for tag_name in query['tag_names']: - query_set = query_set.filter( - bookmark__tags__name__iexact=tag_name - ) + query_set = Tag.objects.filter(bookmark__in=bookmarks_query) return query_set.distinct() diff --git a/bookmarks/tests/test_queries.py b/bookmarks/tests/test_queries.py index eeca8a0..542828e 100644 --- a/bookmarks/tests/test_queries.py +++ b/bookmarks/tests/test_queries.py @@ -17,6 +17,7 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin): def setup_bookmark_search_data(self) -> None: tag1 = self.setup_tag(name='tag1') tag2 = self.setup_tag(name='tag2') + self.setup_tag(name='unused_tag1') self.other_bookmarks = [ self.setup_bookmark(), @@ -65,6 +66,7 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin): def setup_tag_search_data(self): tag1 = self.setup_tag(name='tag1') tag2 = self.setup_tag(name='tag2') + self.setup_tag(name='unused_tag1') self.other_bookmarks = [ self.setup_bookmark(tags=[self.setup_tag()]), @@ -204,6 +206,18 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin): query = queries.query_bookmarks(self.get_or_create_test_user(), '#tag3') self.assertQueryResult(query, []) + # Unused tag + query = queries.query_bookmarks(self.get_or_create_test_user(), '#unused_tag1') + self.assertQueryResult(query, []) + + # Unused tag combined with tag that is used + query = queries.query_bookmarks(self.get_or_create_test_user(), '#tag1 #unused_tag1') + self.assertQueryResult(query, []) + + # Unused tag combined with term that is used + query = queries.query_bookmarks(self.get_or_create_test_user(), 'term1 #unused_tag1') + self.assertQueryResult(query, []) + def test_query_bookmarks_should_not_return_archived_bookmarks(self): bookmark1 = self.setup_bookmark() bookmark2 = self.setup_bookmark() @@ -351,16 +365,28 @@ class QueriesTestCase(TestCase, BookmarkFactoryMixin): def test_query_bookmark_tags_should_return_no_matches(self): self.setup_tag_search_data() - query = queries.query_bookmarks(self.get_or_create_test_user(), 'term3') + query = queries.query_bookmark_tags(self.get_or_create_test_user(), 'term3') self.assertQueryResult(query, []) - query = queries.query_bookmarks(self.get_or_create_test_user(), 'term1 term3') + query = queries.query_bookmark_tags(self.get_or_create_test_user(), 'term1 term3') self.assertQueryResult(query, []) - query = queries.query_bookmarks(self.get_or_create_test_user(), 'term1 #tag2') + query = queries.query_bookmark_tags(self.get_or_create_test_user(), 'term1 #tag2') self.assertQueryResult(query, []) - query = queries.query_bookmarks(self.get_or_create_test_user(), '#tag3') + query = queries.query_bookmark_tags(self.get_or_create_test_user(), '#tag3') + self.assertQueryResult(query, []) + + # Unused tag + query = queries.query_bookmark_tags(self.get_or_create_test_user(), '#unused_tag1') + self.assertQueryResult(query, []) + + # Unused tag combined with tag that is used + query = queries.query_bookmark_tags(self.get_or_create_test_user(), '#tag1 #unused_tag1') + self.assertQueryResult(query, []) + + # Unused tag combined with term that is used + query = queries.query_bookmark_tags(self.get_or_create_test_user(), 'term1 #unused_tag1') self.assertQueryResult(query, []) def test_query_bookmark_tags_should_return_tags_for_unarchived_bookmarks_only(self):