mirror of
https://github.com/makeplane/plane.git
synced 2026-01-29 17:59:31 -06:00
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
This commit is contained in:
Dheeraj Kumar Ketireddy
committed by
GitHub
GitHub
parent
0589ac56d5
commit
5c842d592e
@@ -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"]
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user