From 6163a2d9a1096a8eee82eb7ad1da38376be3d166 Mon Sep 17 00:00:00 2001 From: Christian Zosel Date: Fri, 4 Oct 2024 17:07:33 +0200 Subject: [PATCH] feat(visibilities): make visibilities suppressable on n:1 relationships Visibilities may incur a heavy tax on DB load, but are not always needed. This will allow you to define, in your visibility classes, a list of lookups that won't use the visibility layer, and thus improve performance. Co-Authored-By: David Vogt --- caluma/caluma_core/tests/test_visibility.py | 66 ++++++++++++++++++ caluma/caluma_core/visibilities.py | 27 ++++++++ caluma/caluma_form/schema.py | 15 ++++ caluma/caluma_workflow/schema.py | 17 +++++ caluma/utils.py | 77 +++++++++++++++++++++ 5 files changed, 202 insertions(+) diff --git a/caluma/caluma_core/tests/test_visibility.py b/caluma/caluma_core/tests/test_visibility.py index 65a9c8d72..d99a688e5 100644 --- a/caluma/caluma_core/tests/test_visibility.py +++ b/caluma/caluma_core/tests/test_visibility.py @@ -2,6 +2,8 @@ from django.core.exceptions import ImproperlyConfigured from django.db.models import CharField, QuerySet +from caluma.caluma_form.schema import Question + from .. import models from ..types import DjangoObjectType, Node from ..visibilities import BaseVisibility, Union, filter_queryset_for @@ -252,3 +254,67 @@ def filter_queryset_for_custom_node(self, node, queryset, info): assert queryset.count() == 1 queryset = CustomVisibility().filter_queryset(CustomNode2, FakeModel2.objects, None) assert queryset.count() == 1 + + +@pytest.mark.xfail( + reason="Didn't find a way to actually test the visibility aware resolver" +) +@pytest.mark.parametrize("suppress, expect_calls", [(True, False), (False, True)]) +def test_suppress_visibility( + db, + history_mock, + mocker, + suppress, + expect_calls, + answer_factory, + question_factory, + schema_executor, +): # pragma: no cover + class CustomVisibility(BaseVisibility): + was_called = False + + @filter_queryset_for(Question) + def filter_queryset_for_custom_node(self, node, queryset, info): + CustomVisibility.was_called = True + # only care about being called, not actual filtering + return queryset.all() + + @property + def suppress_visibilities(self): + return ["Answer.question"] if suppress else [] + + mocker.patch("caluma.caluma_core.types.Node.visibility_classes", [CustomVisibility]) + + # Just so we have something to fetch (document and + # question created implicitly) + answer_factory(question__type="integer") + + # This should trigger the `question` resolver on the answer, which + # we test the suppressor on + res = schema_executor( + """ + query foo { + allDocuments { + edges { + node { + answers { + edges { + node { + id + ... on IntegerAnswer { + question { + slug + } + } + } + } + } + } + } + } + } + """ + ) + assert not res.errors + + assert expect_calls == CustomVisibility.was_called diff --git a/caluma/caluma_core/visibilities.py b/caluma/caluma_core/visibilities.py index 6bfb47306..e60e0f4d8 100644 --- a/caluma/caluma_core/visibilities.py +++ b/caluma/caluma_core/visibilities.py @@ -37,8 +37,18 @@ class BaseVisibility(object): ... @filter_queryset_for(Form) ... def filter_queryset_for_form(self, node, queryset, info): ... return queryset.exclude(slug='protected-form') + ... + ... # Do not trigger visibility when looking up case from workitem + ... suppress_visibilities = [ + ... "WorkItem.case", + ... "WorkItem.child_case", + ... ] """ + # Used by the @suppressable_visibility decorator to store + # the *allowed* values for the `suppress_visibilities` property + _suppressable_visibilities = set() + def __init__(self): queryset_fns = inspect.getmembers(self, lambda m: hasattr(m, "_visibilities")) @@ -56,6 +66,20 @@ def __init__(self): node: fn for _, fn in queryset_fns for node in fn._visibilities } + self._validate_suppress_visibility() + + def _validate_suppress_visibility(self): + requested_suppressors = set(self.suppress_visibilities) + available_suppressors = type(self)._suppressable_visibilities + + invalid = requested_suppressors - available_suppressors + if invalid: # pragma: no cover + invalid_str_list = ", ".join(f"`{x}`" for x in (sorted(invalid))) + raise ImproperlyConfigured( + f"`{type(self).__name__}` contains invalid `suppress_visibilities`: " + f"{invalid_str_list}" + ) + def filter_queryset(self, node, queryset, info): for cls in node.mro(): if cls in self._filter_querysets_for: @@ -63,6 +87,9 @@ def filter_queryset(self, node, queryset, info): return queryset + # Default: suppress no visibilities + suppress_visibilities = [] + class Any(BaseVisibility): """No restrictions, all nodes are exposed.""" diff --git a/caluma/caluma_form/schema.py b/caluma/caluma_form/schema.py index 37c6adb02..dde23b1f0 100644 --- a/caluma/caluma_form/schema.py +++ b/caluma/caluma_form/schema.py @@ -4,6 +4,8 @@ from graphene.types import ObjectType, generic from graphene_django.rest_framework import serializer_converter +from caluma.utils import suppressable_visibility_resolver + from ..caluma_core.filters import ( CollectionFilterSetFactory, DjangoFilterConnectionField, @@ -147,6 +149,8 @@ class Question(Node, graphene.Interface): ) source = graphene.Field("caluma.caluma_form.schema.Question") + resolve_source = suppressable_visibility_resolver() + @classmethod def get_queryset(cls, queryset, info): queryset = super().get_queryset(queryset, info) @@ -166,6 +170,8 @@ def resolve_type(cls, instance, info): class Option(FormDjangoObjectType): meta = generic.GenericScalar() + resolve_source = suppressable_visibility_resolver() + class Meta: model = models.Option interfaces = (relay.Node,) @@ -638,6 +644,8 @@ class Form(FormDjangoObjectType): ) meta = generic.GenericScalar() + resolve_source = suppressable_visibility_resolver() + class Meta: model = models.Form interfaces = (relay.Node,) @@ -809,6 +817,8 @@ class Answer(Node, graphene.Interface): question = graphene.Field(Question, required=True) meta = generic.GenericScalar(required=True) + resolve_question = suppressable_visibility_resolver() + @classmethod def resolve_type(cls, instance, info): return resolve_answer(instance) @@ -911,6 +921,11 @@ class Document(FormDjangoObjectType): modified_content_by_user = graphene.String() modified_content_by_group = graphene.String() + resolve_form = suppressable_visibility_resolver() + resolve_case = suppressable_visibility_resolver() + resolve_source = suppressable_visibility_resolver() + resolve_work_item = suppressable_visibility_resolver() + class Meta: model = models.Document exclude = ("family", "dynamicoption_set") diff --git a/caluma/caluma_workflow/schema.py b/caluma/caluma_workflow/schema.py index 1d0d07140..615d0dcc5 100644 --- a/caluma/caluma_workflow/schema.py +++ b/caluma/caluma_workflow/schema.py @@ -6,6 +6,8 @@ from graphene.types import generic from graphene_django.rest_framework import serializer_converter +from caluma.utils import suppressable_visibility_resolver + from ..caluma_core.filters import ( CollectionFilterSetFactory, DjangoFilterConnectionField, @@ -98,6 +100,7 @@ def resolve_type(cls, instance, info): return TASK_TYPE[instance.type] + resolve_form = suppressable_visibility_resolver() Meta = InterfaceMetaFactory() @@ -206,6 +209,12 @@ class WorkItem(DjangoObjectType): ) ) + resolve_case = suppressable_visibility_resolver() + resolve_child_case = suppressable_visibility_resolver() + resolve_task = suppressable_visibility_resolver() + resolve_document = suppressable_visibility_resolver() + resolve_previous_work_item = suppressable_visibility_resolver() + def resolve_is_redoable(self, *args, **kwargs): return ( self.status != models.WorkItem.STATUS_READY @@ -237,6 +246,14 @@ class Case(DjangoObjectType): meta = generic.GenericScalar() status = CaseStatus(required=True) + resolve_document = suppressable_visibility_resolver() + + resolve_parent_work_item = suppressable_visibility_resolver() + + resolve_family = suppressable_visibility_resolver() + + resolve_workflow = suppressable_visibility_resolver() + def resolve_family_work_items(self, info, **args): return models.WorkItem.objects.filter(case__family=self.family) diff --git a/caluma/utils.py b/caluma/utils.py index a3846516c..0286d1e5f 100644 --- a/caluma/utils.py +++ b/caluma/utils.py @@ -3,6 +3,8 @@ from django.db.models import Model +from caluma.caluma_core.types import Node + log = getLogger(__name__) @@ -102,3 +104,78 @@ def wrapper(*args, **kwargs): signal_handler(*args, **kwargs) return wrapper + + +def suppressable_visibility_resolver(): + """Make a resolver that can be suppressed in the visibility layer. + + The visibility layer may cause additional load on the database, so + in cases where it's not needed, the visibility classes can choose to + skip filtering where it's not absolutely needed. + + For example, if the visibility of a child case is always dependent + on the associated work item, you can disable enforcing visibility + on that relationship. + + Example in the GraphQL schema: + + >>> class Form(FormDjangoObjectType): + >>> ... + >>> resolve_child_case = suppressable_visibility_resolver() + + Example in the visibility class: + + >>> class MyCustomVisibility(BaseVisibility): + >>> @filter_queryset_for(Case) + >>> def filter_queryset_for_case(self, node, queryset, info): + >>> # do the filtering as usual + >>> ... + >>> ... + >>> suppress_visibilities = [ + >>> "WorkItem.child_case", + >>> ... + >>> ] + + With the above setting, when looking up child cases from work items, + the visibility layer would not be run, and the child case would always + be shown (if the workitem can be shown, of course). + + You may also define it as a `@property` on the visibility class. Note + however that it's initialized on app startup, you can't change it during + runtime and make suppression work in some cases and not in others. + """ + + # Avoid circular imports + from caluma.caluma_core.visibilities import BaseVisibility + + # TODO: This is currently untested (as in unit test), because + # the checks are done at startup (where the schema is initialized) + # and we can't swap out the visibility class during test runs + # to validate the behavour as we'd usually do. + class SuppressableResolver: + def __call__(self, inner_self, *args, **kwargs): + return getattr(inner_self, self.prop) + + @property + def _bypass_get_queryset(self): # pragma: no cover + """Tell Graphene to bypass get_queryset().""" + # If any visibility class tells us to bypass the + # queryset of this property, we return True + return any( + self.qualname in vis_class().suppress_visibilities + for vis_class in Node.visibility_classes + ) + + def __repr__(self): # pragma: no cover + return f"SuppressableResolver({self.qualname})" + + def __set_name__(self, owner, name): + # Magic. This is called by Python after setting the attribute on + # the node class, and we use it to figure out which resolver we're + # building. + self.name = owner.__name__ + self.prop = name.replace("resolve_", "") + self.qualname = f"{self.name}.{self.prop}" + BaseVisibility._suppressable_visibilities.add(self.qualname) + + return SuppressableResolver()