From 5c842d592ec4d8fd2bbdc715a34c4ced0c23cbf2 Mon Sep 17 00:00:00 2001 From: Dheeraj Kumar Ketireddy Date: Mon, 6 Oct 2025 23:36:16 +0530 Subject: [PATCH] Chore: Filter backend optimizations (#7900) * refactor: enhance ComplexFilterBackend and BaseFilterSet for Q object filtering - Introduced BaseFilterSet to support Q object construction for complex filtering. - Updated ComplexFilterBackend to utilize Q objects for building querysets. - Improved error handling and validation in filter methods. - Refactored filter evaluation logic to streamline query construction. * fix: improve filter processing in BaseFilterSet to handle empty cleaned_data and optimize filter evaluation - Added handling for cases where cleaned_data is None or empty, returning an empty Q object. - Optimized filter evaluation by only processing filters that are provided in the request data. * update ComplexFilterBackend to pass queryset in filter evaluation --- apps/api/plane/utils/filters/__init__.py | 4 +- .../api/plane/utils/filters/filter_backend.py | 151 +++++++----------- apps/api/plane/utils/filters/filterset.py | 111 +++++++++++-- 3 files changed, 159 insertions(+), 107 deletions(-) diff --git a/apps/api/plane/utils/filters/__init__.py b/apps/api/plane/utils/filters/__init__.py index 48a85829e6..76a96c82c0 100644 --- a/apps/api/plane/utils/filters/__init__.py +++ b/apps/api/plane/utils/filters/__init__.py @@ -3,8 +3,8 @@ # Import all utilities from base modules from .filter_backend import ComplexFilterBackend from .converters import LegacyToRichFiltersConverter -from .filterset import IssueFilterSet +from .filterset import BaseFilterSet, IssueFilterSet # Public API exports -__all__ = ["ComplexFilterBackend", "LegacyToRichFiltersConverter", "IssueFilterSet"] +__all__ = ["ComplexFilterBackend", "LegacyToRichFiltersConverter", "BaseFilterSet", "IssueFilterSet"] diff --git a/apps/api/plane/utils/filters/filter_backend.py b/apps/api/plane/utils/filters/filter_backend.py index e2faca1a6c..2f7a27d360 100644 --- a/apps/api/plane/utils/filters/filter_backend.py +++ b/apps/api/plane/utils/filters/filter_backend.py @@ -1,8 +1,9 @@ import json from django.core.exceptions import ValidationError +from django.db.models import Q from django.http import QueryDict -from django_filters.rest_framework import DjangoFilterBackend +from django_filters.utils import translate_validation from rest_framework import filters @@ -38,7 +39,6 @@ class ComplexFilterBackend(filters.BaseFilterBackend): # Propagate validation errors unchanged raise except Exception as e: - raise # Convert unexpected errors to ValidationError to keep response consistent raise ValidationError(f"Filter error: {str(e)}") @@ -58,7 +58,7 @@ class ComplexFilterBackend(filters.BaseFilterBackend): raise ValidationError(f"Invalid JSON for '{source_label}'. Expected a valid JSON object.") def _apply_json_filter(self, queryset, filter_data, view): - """Process a JSON filter structure using OR/AND/NOT set operations.""" + """Process a JSON filter structure using Q object composition.""" if not filter_data: return queryset @@ -69,11 +69,13 @@ class ComplexFilterBackend(filters.BaseFilterBackend): # Validate against the view's FilterSet (only declared filters are allowed) self._validate_fields(filter_data, view) - # Build combined queryset using FilterSet-driven leaf evaluation - combined_qs = self._evaluate_node(filter_data, queryset, view) - if combined_qs is None: + # Build combined Q object from the filter tree + combined_q = self._evaluate_node(filter_data, view, queryset) + if combined_q is None: return queryset - return combined_qs + + # Apply the combined Q object to the queryset once + return queryset.filter(combined_q) def _validate_fields(self, filter_data, view): """Validate that filtered fields are defined in the view's FilterSet.""" @@ -115,108 +117,76 @@ class ComplexFilterBackend(filters.BaseFilterBackend): return fields return [] - def _evaluate_node(self, node, base_queryset, view): + def _evaluate_node(self, node, view, queryset): """ - Recursively evaluate a JSON node into a combined queryset using branch-based filtering. + Recursively evaluate a JSON node into a combined Q object. Rules: - - leaf dict → evaluated through DjangoFilterBackend as a mini-querystring - - {"or": [...]} → union (|) of children - - {"and": [...]} → collect field conditions per branch and apply together - - {"not": {...}} → exclude child's rows from the base queryset - (complement within base scope) + - leaf dict → evaluated through FilterSet to produce a Q object + - {"or": [...]} → Q() | Q() | ... (OR of children) + - {"and": [...]} → Q() & Q() & ... (AND of children) + - {"not": {...}} → ~Q() (negation of child) + + Returns a Q object that can be applied to a queryset. """ if not isinstance(node, dict): return None - # 'or' combination - requires set operations between children + # 'or' combination - OR of child Q objects if "or" in node: children = node["or"] if not isinstance(children, list) or not children: return None - combined = None + combined_q = Q() for child in children: - child_qs = self._evaluate_node(child, base_queryset, view) - if child_qs is None: + child_q = self._evaluate_node(child, view, queryset) + if child_q is None: continue - combined = child_qs if combined is None else (combined | child_qs) - return combined + combined_q |= child_q + return combined_q - # 'and' combination - collect field conditions per branch + # 'and' combination - AND of child Q objects if "and" in node: children = node["and"] if not isinstance(children, list) or not children: return None - return self._evaluate_and_branch(children, base_queryset, view) + combined_q = Q() + for child in children: + child_q = self._evaluate_node(child, view, queryset) + if child_q is None: + continue + combined_q &= child_q + return combined_q - # 'not' negation + # 'not' negation - negate the child Q object if "not" in node: child = node["not"] if not isinstance(child, dict): return None - child_qs = self._evaluate_node(child, base_queryset, view) - if child_qs is None: + child_q = self._evaluate_node(child, view, queryset) + if child_q is None: return None - # Use subquery instead of pk__in for better performance - # This avoids evaluating child_qs and creating large IN clauses - return base_queryset.exclude(pk__in=child_qs.values("pk")) + return ~child_q - # Leaf dict: evaluate via DjangoFilterBackend using FilterSet - return self._filter_leaf_via_backend(node, base_queryset, view) + # Leaf dict: evaluate via FilterSet to get a Q object + return self._build_leaf_q(node, view, queryset) - def _evaluate_and_branch(self, children, base_queryset, view): - """ - Evaluate an AND branch by collecting field conditions and applying them together. + def _build_leaf_q(self, leaf_conditions, view, queryset): + """Build a Q object from leaf filter conditions using the view's FilterSet. - This approach is more efficient than individual leaf evaluation because: - - Field conditions within the same AND branch are collected and applied together - - Only logical operation children require separate evaluation and set intersection - - Reduces the number of intermediate querysets and database queries - """ - collected_conditions = {} - logical_querysets = [] + We serialize the leaf dict into a QueryDict and let the view's + filterset_class perform validation and build a combined Q object + from all the field filters. - # Separate field conditions from logical operations - for child in children: - if not isinstance(child, dict): - continue - - # Check if this child contains logical operators - has_logical = any(k.lower() in ("or", "and", "not") for k in child.keys() if isinstance(k, str)) - - if has_logical: - # This child has logical operators, evaluate separately - child_qs = self._evaluate_node(child, base_queryset, view) - if child_qs is not None: - logical_querysets.append(child_qs) - else: - # This is a leaf with field conditions, collect them - collected_conditions.update(child) - - # Start with base queryset - result_qs = base_queryset - - # Apply collected field conditions together if any exist - if collected_conditions: - result_qs = self._filter_leaf_via_backend(collected_conditions, result_qs, view) - if result_qs is None: - return None - - # Intersect with any logical operation results - for logical_qs in logical_querysets: - result_qs = result_qs & logical_qs - - return result_qs - - def _filter_leaf_via_backend(self, leaf_conditions, base_queryset, view): - """Evaluate a leaf dict by delegating to DjangoFilterBackend once. - - We serialize the leaf dict into a mini querystring and let the view's - filterset_class perform validation, conversion, and filtering. This returns - a lazy queryset suitable for set-operations with siblings. + Returns a Q object representing all the field conditions in the leaf. """ if not leaf_conditions: - return None + return Q() + + # Get the filterset class from the view + filterset_class = getattr(view, "filterset_class", None) + if not filterset_class: + raise ValidationError("Filtering requires a filterset_class to be defined on the view") # Build a QueryDict from the leaf conditions qd = QueryDict(mutable=True) @@ -231,17 +201,18 @@ class ComplexFilterBackend(filters.BaseFilterBackend): qd = qd.copy() qd._mutable = False - # Temporarily patch request.GET and delegate to DjangoFilterBackend - backend = DjangoFilterBackend() - request = view.request - original_get = request._request.GET if hasattr(request, "_request") else None - try: - if hasattr(request, "_request"): - request._request.GET = qd - return backend.filter_queryset(request, base_queryset, view) - finally: - if hasattr(request, "_request") and original_get is not None: - request._request.GET = original_get + # Instantiate the filterset with the actual queryset + # Custom filter methods may need access to the queryset for filtering + fs = filterset_class(data=qd, queryset=queryset) + + if not fs.is_valid(): + raise translate_validation(fs.errors) + + # Build and return the combined Q object + if not hasattr(fs, "build_combined_q"): + raise ValidationError("FilterSet must have build_combined_q method for complex filtering") + + return fs.build_combined_q() def _get_max_depth(self, view): """Return the maximum allowed nesting depth for complex filters. diff --git a/apps/api/plane/utils/filters/filterset.py b/apps/api/plane/utils/filters/filterset.py index 761916ac79..8fff8de371 100644 --- a/apps/api/plane/utils/filters/filterset.py +++ b/apps/api/plane/utils/filters/filterset.py @@ -1,5 +1,7 @@ import copy +from django.db import models +from django.db.models import Q from django_filters import FilterSet, filters from plane.db.models import Issue @@ -35,6 +37,85 @@ class BaseFilterSet(FilterSet): filters.update(exact_filters) return filters + def build_combined_q(self): + """ + Build a combined Q object from all bound filters. + + For filters with custom methods, we call them and expect Q objects (or wrap + QuerySets as subqueries for backward compatibility). + For standard field filters, we build Q objects directly from field lookups. + + Returns: + Q object representing all filter conditions combined. + """ + # Ensure form validation has occurred + self.errors + + combined_q = Q() + + # Handle case where cleaned_data might be None or empty + if not self.form.cleaned_data: + return combined_q + + # Only process filters that were actually provided in the request data + # This avoids processing all declared filters with None/empty default values + provided_filters = set(self.data.keys()) if self.data else set() + + for name, value in self.form.cleaned_data.items(): + # Skip filters that weren't provided in the request + if name not in provided_filters: + continue + + f = self.filters[name] + + # Build the Q object for this filter + if f.method is not None: + # Custom filter method - call it to get Q object + res = f.filter(self.queryset, value) + if isinstance(res, Q): + q_piece = res + elif isinstance(res, models.QuerySet): + # Backward compatibility: wrap QuerySet as subquery + q_piece = Q(pk__in=res.values("pk")) + else: + raise TypeError( + f"Filter method '{name}' must return Q object or QuerySet, got {type(res).__name__}" + ) + else: + # Standard field filter - build Q object directly + lookup = f"{f.field_name}__{f.lookup_expr}" + q_piece = Q(**{lookup: value}) + + # Apply exclude/include logic + if getattr(f, "exclude", False): + combined_q &= ~q_piece + else: + combined_q &= q_piece + + return combined_q + + def filter_queryset(self, queryset): + """ + Override to use Q-based filtering for compatibility with DjangoFilterBackend. + + This allows the same filterset to work with both ComplexFilterBackend + (which calls build_combined_q directly) and DjangoFilterBackend + (which calls this method). + """ + # Ensure form validation + self.errors + + # Build combined Q and apply to queryset + combined_q = self.build_combined_q() + qs = queryset.filter(combined_q) + + # Apply distinct if any filter requires it (typically for many-to-many relations) + for f in self.filters.values(): + if getattr(f, "distinct", False): + return qs.distinct() + + return qs + class IssueFilterSet(BaseFilterSet): # Custom filter methods to handle soft delete exclusion for relations @@ -88,93 +169,93 @@ class IssueFilterSet(BaseFilterSet): archived=false -> archived_at is null """ if value in (True, "true", "True", 1, "1"): - return queryset.filter(archived_at__isnull=False) + return Q(archived_at__isnull=False) if value in (False, "false", "False", 0, "0"): - return queryset.filter(archived_at__isnull=True) - return queryset + return Q(archived_at__isnull=True) + return Q() # No filter # Filter methods with soft delete exclusion for relations def filter_assignee_id(self, queryset, name, value): """Filter by assignee ID, excluding soft deleted users""" - return queryset.filter( + return Q( issue_assignee__assignee_id=value, issue_assignee__deleted_at__isnull=True, ) def filter_assignee_id_in(self, queryset, name, value): """Filter by assignee IDs (in), excluding soft deleted users""" - return queryset.filter( + return Q( issue_assignee__assignee_id__in=value, issue_assignee__deleted_at__isnull=True, ) def filter_cycle_id(self, queryset, name, value): """Filter by cycle ID, excluding soft deleted cycles""" - return queryset.filter( + return Q( issue_cycle__cycle_id=value, issue_cycle__deleted_at__isnull=True, ) def filter_cycle_id_in(self, queryset, name, value): """Filter by cycle IDs (in), excluding soft deleted cycles""" - return queryset.filter( + return Q( issue_cycle__cycle_id__in=value, issue_cycle__deleted_at__isnull=True, ) def filter_module_id(self, queryset, name, value): """Filter by module ID, excluding soft deleted modules""" - return queryset.filter( + return Q( issue_module__module_id=value, issue_module__deleted_at__isnull=True, ) def filter_module_id_in(self, queryset, name, value): """Filter by module IDs (in), excluding soft deleted modules""" - return queryset.filter( + return Q( issue_module__module_id__in=value, issue_module__deleted_at__isnull=True, ) def filter_mention_id(self, queryset, name, value): """Filter by mention ID, excluding soft deleted users""" - return queryset.filter( + return Q( issue_mention__mention_id=value, issue_mention__deleted_at__isnull=True, ) def filter_mention_id_in(self, queryset, name, value): """Filter by mention IDs (in), excluding soft deleted users""" - return queryset.filter( + return Q( issue_mention__mention_id__in=value, issue_mention__deleted_at__isnull=True, ) def filter_label_id(self, queryset, name, value): """Filter by label ID, excluding soft deleted labels""" - return queryset.filter( + return Q( label_issue__label_id=value, label_issue__deleted_at__isnull=True, ) def filter_label_id_in(self, queryset, name, value): """Filter by label IDs (in), excluding soft deleted labels""" - return queryset.filter( + return Q( label_issue__label_id__in=value, label_issue__deleted_at__isnull=True, ) def filter_subscriber_id(self, queryset, name, value): """Filter by subscriber ID, excluding soft deleted users""" - return queryset.filter( + return Q( issue_subscribers__subscriber_id=value, issue_subscribers__deleted_at__isnull=True, ) def filter_subscriber_id_in(self, queryset, name, value): """Filter by subscriber IDs (in), excluding soft deleted users""" - return queryset.filter( + return Q( issue_subscribers__subscriber_id__in=value, issue_subscribers__deleted_at__isnull=True, )