From 8e5ff71fb319780fa659cd0d4a16f47c6a0ff864 Mon Sep 17 00:00:00 2001 From: Christoph Volkert Date: Mon, 6 Jan 2025 21:45:01 +0100 Subject: [PATCH] WIP: Refactor FilterByMetaData to use no Q Start at the Page object, as we want to get page names. The DB will already sort pages by name by default (see meta-inner-class of Page) Added basic tests. TODOs there to be discussed --- inyoka/wiki/macros.py | 42 +++---- tests/apps/wiki/test_macros.py | 199 +++++++++++++++++++++++++++++++++ tests/apps/wiki/test_models.py | 5 + 3 files changed, 222 insertions(+), 24 deletions(-) diff --git a/inyoka/wiki/macros.py b/inyoka/wiki/macros.py index 20afe396f..5beb76892 100644 --- a/inyoka/wiki/macros.py +++ b/inyoka/wiki/macros.py @@ -10,12 +10,9 @@ import itertools import operator -from django.db.models import Q from django.conf import settings from django.utils.translation import gettext as _ -from functools import reduce - from inyoka.markup import macros, nodes from inyoka.markup.parsertools import MultiMap, flatten_iterator from inyoka.markup.templates import expand_page_template @@ -23,7 +20,7 @@ from inyoka.utils.imaging import parse_dimensions from inyoka.utils.text import get_pagetitle, join_pagename, normalize_pagename from inyoka.utils.urls import href, is_safe_domain, urlencode -from inyoka.wiki.models import MetaData, Page, is_privileged_wiki_page +from inyoka.wiki.models import Page, is_privileged_wiki_page from inyoka.wiki.signals import build_picture_node from inyoka.wiki.views import fetch_real_target @@ -253,43 +250,40 @@ def build_node(self, context, format): mapping.extend([(key.strip(), x.strip()) for x in values]) mapping = MultiMap(mapping) - pages_query = Q() - exclude_query = Q() + pages = Page.objects.all() + for key in list(mapping.keys()): values = list(flatten_iterator(mapping[key])) includes = {x for x in values if not x.startswith(('NOT ', 'EXACT',))} - exclude_values = set() - kwargs = {'key': key} + + kwargs = {'metadata__key': key} if values[0].startswith("EXACT"): exact_value = values[0].removeprefix("EXACT ") - kwargs['value__exact'] = exact_value + kwargs['metadata__value'] = exact_value elif values[0].startswith("NOT"): - exclude_values.add(values[0].removeprefix("NOT ")) + exclude_value = values[0].removeprefix("NOT ") + pages = pages.exclude(metadata__key=key, metadata__value=exclude_value) else: - kwargs['value__in'] = includes - pages_query |= Q(**kwargs) - exclude_query |= Q(**{"key": key, "value__in": exclude_values}) + kwargs['metadata__value__in'] = includes - exclude_privileged_query = Q() - for prefix in settings.WIKI_PRIVILEGED_PAGES: - exclude_privileged_query |= Q(**{"page__name__startswith": prefix}) + pages = pages.filter(**kwargs) - pages = MetaData.objects.select_related('page').filter(pages_query).exclude( - exclude_query).exclude(exclude_privileged_query) + # exclude privileged pages + for prefix in settings.WIKI_PRIVILEGED_PAGES: + pages = pages.exclude(name__startswith=prefix) - names = {p.page.name for p in pages} - names = sorted(names, key=lambda s: s.lower()) + page_names = pages.values_list('name', flat=True) - if not names: + if not page_names: return nodes.error_box(_('No result'), _('The metadata filter has found no results. Query: %(query)s') % { 'query': '; '.join(self.filters)}) # build the node result = nodes.List('unordered') - for page in names: - title = [nodes.Text(get_pagetitle(page))] - link = nodes.InternalLink(page, title, force_existing=True) + for p in page_names: + title = [nodes.Text(get_pagetitle(p))] + link = nodes.InternalLink(p, title, force_existing=True) result.children.append(nodes.ListItem([link])) return result diff --git a/tests/apps/wiki/test_macros.py b/tests/apps/wiki/test_macros.py index a393ac909..8131b696f 100644 --- a/tests/apps/wiki/test_macros.py +++ b/tests/apps/wiki/test_macros.py @@ -7,6 +7,8 @@ :copyright: (c) 2012-2024 by the Inyoka Team, see AUTHORS for more details. :license: BSD, see LICENSE for more details. """ +from django.test import override_settings + from inyoka.markup import macros from inyoka.markup.base import RenderContext, parse from inyoka.utils.test import TestCase @@ -39,3 +41,200 @@ def test_taglist(self): needle = '

Pages with tag “Ubuntu Touch”' \ '

' self.assertInHTML(needle, html) + + def test_filterbymetadata__no_result(self): + page = Page(name='Something') + html = parse("""[[FilterByMetaData("X-Link: foo")]]""").render( + RenderContext(wiki_page=page, application='wiki'), + format='html' + ) + + needle = '
No result

The metadata filter has found no results. Query: X-Link: foo

' + self.assertInHTML(needle, html) + + def test_filterbymetadata__simple_query(self): + Page.objects.create('foo', 'some text') + Page.objects.create('bar', 'test [:foo:content]') + + page = Page(name='Something') + html = parse("""[[FilterByMetaData("X-Link: foo")]]""").render( + RenderContext(wiki_page=page, application='wiki'), + format='html' + ) + + needle = '' + self.assertInHTML(needle, html) + + @override_settings(WIKI_PRIVILEGED_PAGES=['internal', 'Trash']) + def test_filterbymetadata__exclude_privileged_pages(self): + Page.objects.create('foo', 'some text') + Page.objects.create('bar', 'test [:foo:content]') + Page.objects.create('internal/bar', 'test [:foo:link]') + Page.objects.create('Trash/old_bar', 'test [:foo:link]') + + page = Page(name='Something') + html = parse("""[[FilterByMetaData("X-Link: foo")]]""").render( + RenderContext(wiki_page=page, application='wiki'), + format='html' + ) + + needle = '' + self.assertInHTML(needle, html) + + def test_filterbymetadata__invalid_filter_syntax(self): + page = Page(name='Something') + html = parse("""[[FilterByMetaData("X-Link:: foo")]]""").render( + RenderContext(wiki_page=page, application='wiki'), + format='html' + ) + + needle = '
No result

Invalid filter syntax. Query: X-Link:: foo

' + self.assertInHTML(needle, html) + + def test_filterbymetadata__not(self): + Page.objects.create('foo', 'some text') + Page.objects.create('baz', 'another page') + Page.objects.create('bar', 'test [:foo:content]') + Page.objects.create('linkbaz', 'links [:baz:]') + + page = Page(name='Something') + html = parse("""[[FilterByMetaData("X-Link: NOT foo")]]""").render( + RenderContext(wiki_page=page, application='wiki'), + format='html' + ) + print(html) + + needle = '' + ## TODO: should foo and baz also show up here? + self.assertInHTML(needle, html) + + def test_filterbymetadata__tag(self): + Page.objects.create('foo', 'some text\n# tag: foo') + Page.objects.create('baz', 'another page\n# tag: baz') + + page = Page(name='Something') + html = parse("""[[FilterByMetaData("tag: foo")]]""").render( + RenderContext(wiki_page=page, application='wiki'), + format='html' + ) + + needle = '' + self.assertInHTML(needle, html) + + def test_filterbymetadata__tag_exact(self): + Page.objects.create('foo', 'some text\n# tag: foo') + Page.objects.create('baz', 'another page\n# tag: foo, baz') + + page = Page(name='Something') + html = parse("""[[FilterByMetaData("tag: EXACT foo")]]""").render( + RenderContext(wiki_page=page, application='wiki'), + format='html' + ) + + needle = '''''' + ## TODO: idea behind EXACT? Should only be baz listed here? + self.assertInHTML(needle, html) + + def test_filterbymetadata__xlink_exact(self): + Page.objects.create('foo', 'some text') + Page.objects.create('bar', 'test [:foo:content]') + Page.objects.create('intbar', 'test [:foo:link]') + Page.objects.create('old_bar', 'test [:bar:link]') + + page = Page(name='Something') + html = parse("""[[FilterByMetaData("X-Link: EXACT foo")]]""").render( + RenderContext(wiki_page=page, application='wiki'), + format='html' + ) + + needle = ''' + ''' + # TODO should there be a difference with and without EXACT? + self.assertInHTML(needle, html) + + def test_filterbymetadata__xlink_and_xlink(self): + Page.objects.create('foo', 'some text') + Page.objects.create('bar', 'test [:foo:content]') + Page.objects.create('intbar', 'test [:foo:link]') + Page.objects.create('old_bar', 'test [:bar:link]') + + page = Page(name='Something') + html = parse("""[[FilterByMetaData("X-Link: foo; X-Link: bar")]]""").render( + RenderContext(wiki_page=page, application='wiki'), + format='html' + ) + + needle = ''' + ''' + self.assertInHTML(needle, html) + + def test_filterbymetadata__xlink_two_values(self): + # TODO should there be semantic difference to `X-Link: foo; X-Link: bar`? Remove possibility for `X-Link: foo,bar`, as tags can also contain a `,`? + Page.objects.create('foo', 'some text') + Page.objects.create('bar', 'test [:foo:content]') + Page.objects.create('intbar', 'test [:foo:link]') + Page.objects.create('old_bar', 'test [:bar:link]') + + page = Page(name='Something') + html = parse("""[[FilterByMetaData("X-Link: foo,bar")]]""").render( + RenderContext(wiki_page=page, application='wiki'), + format='html' + ) + + needle = ''' + ''' + self.assertInHTML(needle, html) + + def test_filterbymetadata__xlink_and_tag(self): + Page.objects.create('foo', 'some text') + Page.objects.create('bar', 'test [:foo:content]\n#tag: gras') + Page.objects.create('intbar', 'test [:foo:link]\n#tag: new') + Page.objects.create('old_bar', 'test [:bar:link]\n#tag: old') + Page.objects.create('odd_bar', 'test [:bar:link]\n#tag: gras') + + page = Page(name='Something') + html = parse("""[[FilterByMetaData("X-Link: foo; tag: gras")]]""").render( + RenderContext(wiki_page=page, application='wiki'), + format='html' + ) + + needle = ''' + ''' + self.assertInHTML(needle, html) + + def test_filterbymetadata__tag_and_not_xlink(self): + Page.objects.create('foo', 'some text\n#tag: gras') + Page.objects.create('bar', 'test [:foo:content]\n#tag: gras') + Page.objects.create('intbar', 'test [:foo:link]\n#tag: new') + Page.objects.create('old_bar', 'test [:bar:link]\n#tag: old') + Page.objects.create('odd_bar', 'test [:bar:link]\n#tag: gras') + + page = Page(name='Something') + with self.assertNumQueries(1): + html = parse("""[[FilterByMetaData("tag: EXACT gras; X-Link: NOT bar")]]""").render( + RenderContext(wiki_page=page, application='wiki'), + format='html' + ) + + needle = ''' + ''' + # TODO should foo be also listed? + self.assertInHTML(needle, html) diff --git a/tests/apps/wiki/test_models.py b/tests/apps/wiki/test_models.py index b9b4d9795..a9373fbee 100644 --- a/tests/apps/wiki/test_models.py +++ b/tests/apps/wiki/test_models.py @@ -5,6 +5,7 @@ from django.core.files import File from django.core.files.uploadedfile import SimpleUploadedFile +from inyoka.markup.parsertools import MultiMap from inyoka.utils.test import TestCase from inyoka.wiki.exceptions import CaseSensitiveException from inyoka.wiki.models import Attachment, Page @@ -14,6 +15,10 @@ class TestPage(TestCase): + def test_tag_with_comma_as_metadata(self): + p1 = Page.objects.create('foo', "some text\n#tag: gra\\,s") + self.assertEqual(p1.metadata, MultiMap([('tag', 'gra,s')])) + def test_update_related_pages__content_updated(self): """ Test, that content of a embedder page will be updated.