diff --git a/CHANGES/2480.feature b/CHANGES/2480.feature new file mode 100644 index 00000000000..66721c0ce12 --- /dev/null +++ b/CHANGES/2480.feature @@ -0,0 +1 @@ +Added a filter `q` that supports complex filter expressions. diff --git a/CHANGES/3914.feature b/CHANGES/3914.feature new file mode 100644 index 00000000000..66721c0ce12 --- /dev/null +++ b/CHANGES/3914.feature @@ -0,0 +1 @@ +Added a filter `q` that supports complex filter expressions. diff --git a/docs/workflows/advanced-filtering.rst b/docs/workflows/advanced-filtering.rst new file mode 100644 index 00000000000..f9afc396bff --- /dev/null +++ b/docs/workflows/advanced-filtering.rst @@ -0,0 +1,33 @@ +Advanced Filtering +================== + +In addition to the usual querystring filters, Pulp provides a special ``q`` filter, that allows you +to combine other filters with `NOT`, `AND` and `OR` operations. + +For a given list endpoint, all the other existing (non ordering) filters can be used in these +expressions. + +The grammar is basically:: + + EXPRESSION = SIMPLE_EXPR | "(" SIMPLE_EXPR | COMPOSIT_EXPR ")" + COMPOSIT_EXPR = NOT_EXPR | AND_EXPR | OR_EXPR + NOT_EXPR = "NOT" WS EXPRESSION + AND_EXPRESSION = EXPRESSION "AND" EXPRESSION + OR_EXPRESSION = EXPRESSION "OR" EXPRESSION + SIMPLE_EXPRESSION = FILTERNAME "=" STRING + STRING = SIMPLE_STRING | QUOTED_STRING + +Some example ``q`` expressions are:: + + pulp_type__in='core.rbac' + NOT pulp_type="core.rbac" + pulp_type__in=core.rbac,core.content_redirect + pulp_type="core.rbac" OR pulp_type="core.content_redirect" + pulp_type="core.rbac" AND name__contains=GGGG + pulp_type="core.rbac" AND name__iexact={prefix}-gGgG + pulp_type="core.rbac" AND name__icontains=gg AND NOT name__contains=HH + NOT (pulp_type="core.rbac" AND name__icontains=gGgG) + pulp_type="core.rbac" AND name__contains="naïve" + pulp_type="core.rbac" AND NOT name__contains="naïve" + pulp_type="core.rbac" AND( name__icontains=gh OR name__contains="naïve") + pulp_type="core.rbac" OR name__icontains=gh OR name__contains="naïve" diff --git a/docs/workflows/index.rst b/docs/workflows/index.rst index 16f44909677..dc8e822ee0c 100644 --- a/docs/workflows/index.rst +++ b/docs/workflows/index.rst @@ -25,3 +25,4 @@ assumes that the reader is familiar with the fundamentals discussed in the :doc: plugin-removal troubleshooting domains-multi-tenancy + advanced-filtering diff --git a/pulpcore/app/viewsets/base.py b/pulpcore/app/viewsets/base.py index e4003b9f127..de59a3eba82 100644 --- a/pulpcore/app/viewsets/base.py +++ b/pulpcore/app/viewsets/base.py @@ -25,7 +25,15 @@ from pulpcore.tasking.tasks import dispatch # These should be used to prevent duplication and keep things consistent -NAME_FILTER_OPTIONS = ["exact", "in", "icontains", "contains", "startswith"] +NAME_FILTER_OPTIONS = [ + "exact", + "iexact", + "in", + "contains", + "icontains", + "startswith", + "istartswith", +] # e.g. # /?name=foo # /?name__in=foo,bar diff --git a/pulpcore/app/viewsets/task.py b/pulpcore/app/viewsets/task.py index 7e673956602..c99800a1675 100644 --- a/pulpcore/app/viewsets/task.py +++ b/pulpcore/app/viewsets/task.py @@ -60,9 +60,9 @@ class TaskFilter(BaseFilterSet): class Meta: model = Task fields = { - "state": ["exact", "in"], - "worker": ["exact", "in"], - "name": ["exact", "contains", "in"], + "state": ["exact", "in", "ne"], + "worker": ["exact", "in", "isnull"], + "name": ["exact", "contains", "in", "ne"], "logging_cid": ["exact", "contains"], "started_at": DATETIME_FILTER_OPTIONS, "finished_at": DATETIME_FILTER_OPTIONS, diff --git a/pulpcore/filters.py b/pulpcore/filters.py index a9852647136..fed7c489b5c 100644 --- a/pulpcore/filters.py +++ b/pulpcore/filters.py @@ -1,8 +1,11 @@ from gettext import gettext as _ -from functools import lru_cache +import pyparsing as pp + +from functools import lru_cache, partial from urllib.parse import urlparse from uuid import UUID +from django import forms from django.db import models from django.forms.utils import ErrorList from django.urls import Resolver404, resolve @@ -128,7 +131,7 @@ class HREFInFilter(BaseInFilter, filters.CharFilter): pass -class PulpTypeInFilter(BaseInFilter, filters.ChoiceFilter): +class PulpTypeFilter(filters.ChoiceFilter): """Special pulp_type filter only added to generic list endpoints.""" def __init__(self, *args, **kwargs): @@ -150,6 +153,104 @@ def pulp_type_choices(model): return choices +class PulpTypeInFilter(BaseInFilter, PulpTypeFilter): + """Special pulp_type filter only added to generic list endpoints.""" + + +class ExpressionFilterField(forms.CharField): + class _FilterAction: + def __init__(self, filterset, tokens): + key = tokens[0].key + value = tokens[0].value + self.filter = filterset.filters.get(key) + if self.filter is None: + raise forms.ValidationError(_("Filter '{key}' does not exist.").format(key=key)) + if isinstance(self.filter, ExpressionFilter): + raise forms.ValidationError( + _("You cannot use '{key}' in complex filtering.").format(key=key) + ) + if isinstance(self.filter, filters.OrderingFilter): + raise forms.ValidationError( + _("An ordering filter cannot be used in complex filtering.") + ) + form = filterset.form.__class__({key: value}) + if not form.is_valid(): + raise forms.ValidationError(form.errors.as_json()) + self.value = form.cleaned_data[key] + + def evaluate(self, qs): + return self.filter.filter(qs, self.value) + + class _NotAction: + def __init__(self, tokens): + self.expr = tokens[0][0] + + def evaluate(self, qs): + return qs.difference(self.expr.evaluate(qs)) + + class _AndAction: + def __init__(self, tokens): + self.exprs = tokens[0] + + def evaluate(self, qs): + return ( + self.exprs[0] + .evaluate(qs) + .intersection(*[expr.evaluate(qs) for expr in self.exprs[1:]]) + ) + + class _OrAction: + def __init__(self, tokens): + self.exprs = tokens[0] + + def evaluate(self, qs): + return self.exprs[0].evaluate(qs).union(*[expr.evaluate(qs) for expr in self.exprs[1:]]) + + def __init__(self, *args, **kwargs): + self.filterset = kwargs.pop("filter").parent + super().__init__(*args, **kwargs) + + def clean(self, value): + value = super().clean(value) + if value not in EMPTY_VALUES: + slug = pp.Word(pp.alphas, pp.alphanums + "_") + word = pp.Word(pp.alphanums + pp.alphas8bit + ".,_-*") + rhs = word | pp.quoted_string.set_parse_action(pp.remove_quotes) + group = pp.Group( + slug.set_results_name("key") + + pp.Suppress(pp.Literal("=")) + + rhs.set_results_name("value") + ).set_parse_action(partial(self._FilterAction, self.filterset)) + + expr = pp.infix_notation( + group, + [ + (pp.Suppress(pp.Keyword("NOT")), 1, pp.opAssoc.RIGHT, self._NotAction), + (pp.Suppress(pp.Keyword("AND")), 2, pp.opAssoc.LEFT, self._AndAction), + (pp.Suppress(pp.Keyword("OR")), 2, pp.opAssoc.LEFT, self._OrAction), + ], + ) + try: + result = expr.parse_string(value, parse_all=True)[0] + except pp.ParseException: + raise forms.ValidationError(_("Syntax error in expression.")) + return result + return None + + +class ExpressionFilter(filters.CharFilter): + field_class = ExpressionFilterField + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.extra["filter"] = self + + def filter(self, qs, value): + if value is not None: + qs = value.evaluate(qs) + return qs + + class BaseFilterSet(filterset.FilterSet): """ Class to override django_filter's FilterSet and provide a way to set help text @@ -165,6 +266,7 @@ class BaseFilterSet(filterset.FilterSet): help_text = {} pulp_id__in = IdInFilter(field_name="pk", lookup_expr="in") pulp_href__in = HREFInFilter(field_name="pk", method="filter_pulp_href") + q = ExpressionFilter() FILTER_DEFAULTS = { **filterset.FilterSet.FILTER_DEFAULTS, @@ -296,6 +398,7 @@ def get_filterset_class(self, view, queryset=None): if hasattr(view, "is_master_viewset") and view.is_master_viewset(): class PulpTypeFilterSet(filterset_class): + pulp_type = PulpTypeFilter(field_name="pulp_type", model=queryset.model) pulp_type__in = PulpTypeInFilter(field_name="pulp_type", model=queryset.model) return PulpTypeFilterSet diff --git a/pulpcore/tests/functional/api/test_filter.py b/pulpcore/tests/functional/api/test_filter.py index e001e2b64bf..46136a11bb7 100644 --- a/pulpcore/tests/functional/api/test_filter.py +++ b/pulpcore/tests/functional/api/test_filter.py @@ -1,130 +1,219 @@ import pytest -from random import sample -from uuid import uuid4 +import random +import uuid from pulpcore.client.pulpcore.exceptions import ApiException, ApiTypeError +# Warning: Do not use HEX digits here! +NAMES = ( + "GGGG", + "GGHH", + "gggg", + "GgHh", + "áàâãäæÁÀÂÃÄçÇéèêëíìĩïóòôõöúùûũüßþ", +) + + def extract_pk(href): """Simulate what pulpcore.app.util.extract_pk does.""" parts = href.split("/") return parts[-2] -@pytest.fixture -def gen_rbac_and_redirect_guards( +@pytest.fixture(scope="class") +def rbac_and_redirect_guards( redirect_contentguard_api_client, rbac_contentguard_api_client, gen_object_with_cleanup, ): - def _generate(number=5): - rbacs = [] - redis = [] - for _ in range(number): - rbacs.append( - gen_object_with_cleanup(rbac_contentguard_api_client, {"name": str(uuid4())}) - ) - redis.append( - gen_object_with_cleanup(redirect_contentguard_api_client, {"name": str(uuid4())}) - ) - return rbacs, redis - - return _generate - - -@pytest.mark.parallel -@pytest.mark.parametrize( - "filter,f,exception_message", - [("pulp_id__in", extract_pk, "Enter a valid UUID"), ("pulp_href__in", str, "URI not valid")], -) -def test_pulp_id_href_filter( - filter, - f, - exception_message, - content_guards_api_client, - redirect_contentguard_api_client, - rbac_contentguard_api_client, - gen_rbac_and_redirect_guards, -): - """Tests pulp_href__in and pulp_id__in filters.""" - rbacs, redis = gen_rbac_and_redirect_guards() - rbac_extracted = [f(cg.pulp_href) for cg in rbacs] - redi_extracted = [f(cg.pulp_href) for cg in redis] - - rbac_sample = sample(rbac_extracted, 3) - redi_sample = sample(redi_extracted, 3) - - rbac_results = rbac_contentguard_api_client.list(**{filter: rbac_sample}) - assert rbac_results.count == 3 - assert set(rbac_sample) == {f(cg.pulp_href) for cg in rbac_results.results} - - redi_results = redirect_contentguard_api_client.list(**{filter: redi_sample}) - assert redi_results.count == 3 - assert set(redi_sample) == {f(cg.pulp_href) for cg in redi_results.results} - - # Test that generic endpoint can return both - results = content_guards_api_client.list(**{filter: rbac_sample + redi_sample}) - assert results.count == 6 - assert set(redi_sample + rbac_sample) == {f(cg.pulp_href) for cg in results.results} - - # Test swapping rbac & redirect return 0 - rbac_results = rbac_contentguard_api_client.list(**{filter: redi_sample}) - assert rbac_results.count == 0 - - redi_results = redirect_contentguard_api_client.list(**{filter: rbac_sample}) - assert redi_results.count == 0 - - # Test that filter fails when not a valid type - with pytest.raises(ApiException) as exc: - content_guards_api_client.list(**{filter: ["hello"]}) - - assert exc.value.status == 400 - assert exception_message in exc.value.body - - -@pytest.mark.parallel -def test_pulp_type_filter( - content_guards_api_client, - redirect_contentguard_api_client, - rbac_contentguard_api_client, - gen_rbac_and_redirect_guards, -): - """Tests the pulp_type__in filter.""" - gen_rbac_and_redirect_guards() - - # Test filtering by one pulp_type - rbac_result = content_guards_api_client.list(pulp_type__in=["core.rbac"]) - assert rbac_result.count >= 5 - for c in rbac_result.results: - assert "core/rbac" in c.pulp_href - - redirect_result = content_guards_api_client.list(pulp_type__in=["core.content_redirect"]) - assert redirect_result.count >= 5 - for c in redirect_result.results: - assert "core/content_redirect" in c.pulp_href - - # Test filtering by multiple pulp_types - together_result = content_guards_api_client.list( - pulp_type__in=["core.rbac", "core.content_redirect"] + prefix = str(uuid.uuid4()) + rbacs = [] + redis = [] + for name in NAMES: + rbacs.append( + gen_object_with_cleanup(rbac_contentguard_api_client, {"name": prefix + "-" + name}) + ) + redis.append( + gen_object_with_cleanup(redirect_contentguard_api_client, {"name": prefix + "+" + name}) + ) + return prefix, rbacs, redis + + +class TestFilter: + @pytest.mark.parallel + @pytest.mark.parametrize( + "filter,f,exception_message", + [ + ("pulp_id__in", extract_pk, "Enter a valid UUID"), + ("pulp_href__in", str, "URI not valid"), + ], ) - assert together_result.count >= 10 - for c in together_result.results: - assert "core/rbac" in c.pulp_href or "core/content_redirect" in c.pulp_href - - # Test filtering by invalid pulp_type - with pytest.raises(ApiException) as exc: - content_guards_api_client.list(pulp_type__in=["i.invalid"]) - - assert exc.value.status == 400 - assert "Select a valid choice. i.invalid is not one of the available choices." in exc.value.body - - # Test filter does not exist on child viewsets - with pytest.raises(ApiTypeError) as exc: - rbac_contentguard_api_client.list(pulp_type__in=["core.rbac"]) - - assert "Got an unexpected keyword argument 'pulp_type__in'" in str(exc.value) - - with pytest.raises(ApiTypeError) as exc: - redirect_contentguard_api_client.list(pulp_type__in=["core.content_redirect"]) - - assert "Got an unexpected keyword argument 'pulp_type__in'" in str(exc.value) + def test_pulp_id_href_filter( + self, + filter, + f, + exception_message, + content_guards_api_client, + redirect_contentguard_api_client, + rbac_contentguard_api_client, + rbac_and_redirect_guards, + ): + """Tests pulp_href__in and pulp_id__in filters.""" + prefix, rbacs, redis = rbac_and_redirect_guards + rbac_extracted = [f(cg.pulp_href) for cg in rbacs] + redi_extracted = [f(cg.pulp_href) for cg in redis] + + rbac_sample = random.sample(rbac_extracted, 3) + redi_sample = random.sample(redi_extracted, 3) + + rbac_results = rbac_contentguard_api_client.list(**{filter: rbac_sample}) + assert rbac_results.count == 3 + assert set(rbac_sample) == {f(cg.pulp_href) for cg in rbac_results.results} + + redi_results = redirect_contentguard_api_client.list(**{filter: redi_sample}) + assert redi_results.count == 3 + assert set(redi_sample) == {f(cg.pulp_href) for cg in redi_results.results} + + # Test that generic endpoint can return both + results = content_guards_api_client.list(**{filter: rbac_sample + redi_sample}) + assert results.count == 6 + assert set(redi_sample + rbac_sample) == {f(cg.pulp_href) for cg in results.results} + + # Test swapping rbac & redirect return 0 + rbac_results = rbac_contentguard_api_client.list(**{filter: redi_sample}) + assert rbac_results.count == 0 + + redi_results = redirect_contentguard_api_client.list(**{filter: rbac_sample}) + assert redi_results.count == 0 + + # Test that filter fails when not a valid type + with pytest.raises(ApiException) as exc: + content_guards_api_client.list(**{filter: ["hello"]}) + + assert exc.value.status == 400 + assert exception_message in exc.value.body + + @pytest.mark.parallel + def test_pulp_type_filter( + self, + content_guards_api_client, + redirect_contentguard_api_client, + rbac_contentguard_api_client, + rbac_and_redirect_guards, + ): + """Tests the pulp_type__in filter.""" + prefix = rbac_and_redirect_guards[0] + # Test filtering by one pulp_type + rbac_result = content_guards_api_client.list( + name__startswith=prefix, pulp_type__in=["core.rbac"] + ) + assert rbac_result.count == 5 + for c in rbac_result.results: + assert "core/rbac" in c.pulp_href + + redirect_result = content_guards_api_client.list( + name__startswith=prefix, pulp_type__in=["core.content_redirect"] + ) + assert redirect_result.count == 5 + for c in redirect_result.results: + assert "core/content_redirect" in c.pulp_href + + # Test filtering by multiple pulp_types + together_result = content_guards_api_client.list( + name__startswith=prefix, pulp_type__in=["core.rbac", "core.content_redirect"] + ) + assert together_result.count == 10 + for c in together_result.results: + assert "core/rbac" in c.pulp_href or "core/content_redirect" in c.pulp_href + + # Test filtering by invalid pulp_type + with pytest.raises(ApiException) as exc: + content_guards_api_client.list(pulp_type__in=["i.invalid"]) + + assert exc.value.status == 400 + assert ( + "Select a valid choice. i.invalid is not one of the available choices." + in exc.value.body + ) + + # Test filter does not exist on child viewsets + with pytest.raises(ApiTypeError) as exc: + rbac_contentguard_api_client.list(pulp_type__in=["core.rbac"]) + + assert "Got an unexpected keyword argument 'pulp_type__in'" in str(exc.value) + + with pytest.raises(ApiTypeError) as exc: + redirect_contentguard_api_client.list(pulp_type__in=["core.content_redirect"]) + + assert "Got an unexpected keyword argument 'pulp_type__in'" in str(exc.value) + + @pytest.mark.parallel + @pytest.mark.parametrize( + "q,count", + [ + pytest.param(*data, id=data[0]) + for data in [ + ("pulp_type__in='core.rbac'", 5), + ('NOT pulp_type="core.rbac"', 5), + ("pulp_type__in=core.rbac,core.content_redirect", 10), + ('pulp_type="core.rbac" OR pulp_type="core.content_redirect"', 10), + ('pulp_type="core.rbac" AND name__contains=GGGG', 1), + ('pulp_type="core.rbac" AND name__iexact={prefix}-gGgG', 2), + ('pulp_type="core.rbac" AND name__icontains=gg AND NOT name__contains=HH', 3), + ('NOT (pulp_type="core.rbac" AND name__icontains=gGgG)', 8), + ('pulp_type="core.rbac" AND name__contains="{4}"', 1), + ('pulp_type="core.rbac" AND NOT name__contains="{4}"', 4), + ('pulp_type="core.rbac" AND( name__icontains=gh OR name__contains="{4}")', 3), + ('pulp_type="core.rbac" OR name__icontains=gh OR name__contains="{4}"', 8), + ] + ], + ) + def test_q_filter_valid( + self, + q, + count, + content_guards_api_client, + rbac_and_redirect_guards, + ): + """Tests the "q" filter.""" + prefix = rbac_and_redirect_guards[0] + + result = content_guards_api_client.list( + name__startswith=prefix, q=q.format(*NAMES, prefix=prefix) + ) + assert result.count == count + + @pytest.mark.parallel + @pytest.mark.parametrize( + "q,exception_message", + [ + pytest.param(*data, id=data[0]) + for data in [ + ('pulp_type_in="core.rbac"', '{"q":["Filter \'pulp_type_in\' does not exist."]}'), + ( + 'pulp_type="core.foo"', + '{"q":["{\\"pulp_type\\": [{\\"message\\": \\"Select a valid choice. core.foo is not one of the available choices.\\", \\"code\\": \\"invalid_choice\\"}]}"]}', # noqa + ), + ( + 'pulp_type__in="core.rbac,core.foo"', + '{"q":["{\\"pulp_type__in\\": [{\\"message\\": \\"Select a valid choice. core.foo is not one of the available choices.\\", \\"code\\": \\"invalid_choice\\"}]}"]}', # noqa + ), + ('name="test" and', '{"q":["Syntax error in expression."]}'), + ('name="test" AND', '{"q":["Syntax error in expression."]}'), + ('name="test" AND (name="test2"', '{"q":["Syntax error in expression."]}'), + ] + ], + ) + def test_q_filter_invalid( + self, + q, + exception_message, + content_guards_api_client, + ): + """Tests the "q" filter with invalid expressions.""" + + with pytest.raises(ApiException) as exc_info: + content_guards_api_client.list(q=q.format(*NAMES)) + assert exc_info.value.status == 400 + assert exc_info.value.body == exception_message diff --git a/requirements.txt b/requirements.txt index aec029f6bff..7a44d5fde37 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ -asyncio-throttle>=1.0,<=1.0.2 -aiohttp>=3.8.1,<3.8.6 aiodns>=3.0,<=3.0.0 aiofiles>=22.1,<23.3.0 +aiohttp>=3.8.1,<3.8.6 +asyncio-throttle>=1.0,<=1.0.2 backoff>=2.1.2,<2.2.2 click>=8.1.0,<=8.1.6 cryptography>=38.0.1,<41.0.4 @@ -17,19 +17,20 @@ drf-nested-routers>=0.93.4,<=0.93.4 drf-spectacular==0.26.4 # We monkeypatch this so we need a very narrow requirement string dynaconf>=3.1.12,<3.2.1 gunicorn>=20.1,<=21.2.0 +importlib-metadata>=6.0.1,<=6.0.1 # Pinned to fix opentelemetry dependency solving issues with pip jinja2>=3.1,<=3.1.2 naya>=1.1.1,<=1.1.1 -importlib-metadata>=6.0.1,<=6.0.1 # Pinned to fix opentelemetry dependency solving issues with pip opentelemetry-distro[otlp]>=0.38b0,<=0.40b0 opentelemetry-exporter-otlp-proto-http>=1.17.0,<=1.19.0 opentelemetry-instrumentation-django>=0.38b0,<=0.40b0 opentelemetry-instrumentation-wsgi>=0.38b0,<=0.40b0 -pulp-glue>=0.18.0,<0.22 protobuf>=4.21.1,<4.24.1 +pulp-glue>=0.18.0,<0.22 pygtrie>=2.5,<=2.5.0 psycopg[binary]>=3.1.8,<=3.1.10 -PyYAML>=5.1.1,<=6.0.1 +pyparsing>=3.1.0,<=3.1.1 python-gnupg>=0.5,<=0.5.1 +PyYAML>=5.1.1,<=6.0.1 redis>=4.3,<4.6.1 setuptools>=39.2,<68.1.0 url-normalize>=1.4.3,<=1.4.3