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

@@ -10,20 +10,17 @@
 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
 from inyoka.markup.utils import simple_filter
 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

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 = '<div><h3 id="Pages-with-tag-Ubuntu-Touch" class="head">Pages with tag “Ubuntu Touch”' \
                  '<a href="#Pages-with-tag-Ubuntu-Touch" class="headerlink">¶</a></h3><ul class="taglist"></ul></div>'
         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 = '<div class="error"><strong>No result</strong><p>The metadata filter has found no results. Query: X-Link: foo</p></div>'
+        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 = '<ul><li><a href="http://wiki.ubuntuusers.local:8080/bar/" class="internal">bar</a></li></ul>'
+        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 = '<ul><li><a href="http://wiki.ubuntuusers.local:8080/bar/" class="internal">bar</a></li></ul>'
+        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 = '<div class="error"><strong>No result</strong><p>Invalid filter syntax. Query: X-Link:: foo</p></div>'
+        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 = '<ul><li><a href="http://wiki.ubuntuusers.local:8080/linkbaz/" class="internal">linkbaz</a></li></ul>'
+        ## 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 = '<ul><li><a href="http://wiki.ubuntuusers.local:8080/foo/" class="internal">foo</a></li></ul>'
+        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 = '''<ul>
+        <li><a href="http://wiki.ubuntuusers.local:8080/baz/" class="internal">baz</a></li>
+        <li><a href="http://wiki.ubuntuusers.local:8080/foo/" class="internal">foo</a></li>
+        </ul>'''
+        ## 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 = '''<ul>
+        <li><a href="http://wiki.ubuntuusers.local:8080/bar/" class="internal">bar</a></li>
+        <li><a href="http://wiki.ubuntuusers.local:8080/intbar/" class="internal">intbar</a></li>
+        </ul>
+        '''
+        # 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 = '''<ul>
+        <li><a href="http://wiki.ubuntuusers.local:8080/bar/" class="internal">bar</a></li>
+        <li><a href="http://wiki.ubuntuusers.local:8080/intbar/" class="internal">intbar</a></li>
+        <li><a href="http://wiki.ubuntuusers.local:8080/old_bar/" class="internal">old bar</a></li>
+        </ul>
+        '''
+        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 = '''<ul>
+        <li><a href="http://wiki.ubuntuusers.local:8080/bar/" class="internal">bar</a></li>
+        <li><a href="http://wiki.ubuntuusers.local:8080/intbar/" class="internal">intbar</a></li>
+        <li><a href="http://wiki.ubuntuusers.local:8080/old_bar/" class="internal">old bar</a></li>
+        </ul>
+        '''
+        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 = '''<ul>
+        <li><a href="http://wiki.ubuntuusers.local:8080/bar/" class="internal">bar</a></li>
+        </ul>
+        '''
+        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 = '''<ul>
+        <li><a href="http://wiki.ubuntuusers.local:8080/bar/" class="internal">bar</a></li>
+        </ul>
+        '''
+        # TODO should foo be also listed?
+        self.assertInHTML(needle, html)

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.