From 60a4db61d483f0a84ac3abb1c0b6cfb347c0c977 Mon Sep 17 00:00:00 2001 From: Umut Alisoglu Date: Mon, 26 Oct 2026 06:02:09 +0100 Subject: [PATCH] Add opt-in grouping for ORM filters Group field lookups, Q arguments and preceding chained filters only when group=True. Retain default query serialization and test selected rows with an independent boolean parser. Follow the approach suggested by phanak-sap in #280, originally reported by developer992. --- CHANGELOG.md | 1 + docs/usage/querying.rst | 32 ++++++++ pyodata/v2/service.py | 21 +++-- tests/test_filter_grouping.py | 142 ++++++++++++++++++++++++++++++++++ 4 files changed, 191 insertions(+), 5 deletions(-) create mode 100644 tests/test_filter_grouping.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ee556c0c..82bd1b5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added - service: the chainable filter accepts the OData operator names `__le` / `__ge` as lookups, next to the Django-style `__lte` / `__gte` - Vitor Coelho +- service: opt-in `filter(group=True, ...)` preserves grouping when combining ORM filters and chained calls (#280) - Umut Alisoglu ### Fixed diff --git a/docs/usage/querying.rst b/docs/usage/querying.rst index a55f9543..adbd905f 100644 --- a/docs/usage/querying.rst +++ b/docs/usage/querying.rst @@ -96,6 +96,38 @@ Print unique identification (Id) of all employees with name John Smith: print(smith.EmployeeID) +Grouping ORM filter conditions +------------------------------ + +Pass ``group=True`` to keep each condition grouped when combining ``and`` and +``or``. This is useful for ``__in`` lookups, which expand to alternatives: + +.. code-block:: python + + request = service.entity_sets.Employees.get_entities().filter( + ID__in=[2, 3], NameLast="Smith", group=True) + # $filter: ((ID eq 2 or ID eq 3) and (NameLast eq 'Smith')) + +Both alternatives must now also match ``NameLast``. Without grouping, OData's +operator precedence lets the first alternative match regardless of the name. +The same option groups ``Q`` arguments and conditions inside a ``Q`` object: + +.. code-block:: python + + from pyodata.v2.service import FilterExpression as Q + + request = service.entity_sets.Employees.get_entities().filter( + Q(ID=2) | Q(ID=3), NameLast="Smith", group=True) + +On a chained call, ``group=True`` also groups the whole preceding filter before +adding the new conditions. A string filter still replaces the preceding filter, +as it does without grouping. Grouping is selected per call and defaults to +``False``, preserving existing query serialization. + +``group`` is a filter option; to filter an entity property with that exact name, +pass it inside ``Q(group=value)``. + + Get a count of entities ----------------------- diff --git a/pyodata/v2/service.py b/pyodata/v2/service.py index ba130efd..560353d1 100644 --- a/pyodata/v2/service.py +++ b/pyodata/v2/service.py @@ -1199,10 +1199,11 @@ class GetEntitySetFilterChainable: 'eq' ] - def __init__(self, entity_type, filter_expressions, exprs): + def __init__(self, entity_type, filter_expressions, exprs, group=False): self._entity_type = entity_type self._filter_expressions = filter_expressions self._expressions = exprs + self._group = group @property def expressions(self): @@ -1267,6 +1268,8 @@ def _decode_expression(self, expr, val): return expression def _combine_expressions(self, expressions): + if self._group and len(expressions) > 1: + expressions = [f'({expression})' for expression in expressions] return ' and '.join(expressions) # pylint: disable=too-many-return-statements @@ -1330,16 +1333,24 @@ def __getattr__(self, name): proprty = self._entity_type.proprty(name) return GetEntitySetFilter(proprty) - def _set_filter(self, filter_val): + def _set_filter(self, filter_val, group=False): + if group: + if not filter_val: + return + filter_val = f'({filter_val})' + if self._filter: + self._filter = f'({self._filter})' filter_text = self._filter + ' and ' if self._filter else '' filter_text += filter_val self._filter = filter_text - def filter(self, *args, **kwargs): + def filter(self, *args, group=False, **kwargs): + """Set filters, optionally grouping conditions before joining them with and.""" if args and len(args) == 1 and isinstance(args[0], str): - self._filter = args[0] + self._filter = f'({args[0]})' if group and args[0] else args[0] else: - self._set_filter(str(GetEntitySetFilterChainable(self._entity_type, args, kwargs))) + self._set_filter(str(GetEntitySetFilterChainable(self._entity_type, args, kwargs, group=group)), + group=group) return self diff --git a/tests/test_filter_grouping.py b/tests/test_filter_grouping.py new file mode 100644 index 00000000..46851b54 --- /dev/null +++ b/tests/test_filter_grouping.py @@ -0,0 +1,142 @@ +"""Opt-in grouping must preserve the meaning of combined filter conditions.""" + +import ast +import re + +import pytest +import requests +import responses + +from pyodata.v2.service import FilterExpression as Q, Service + + +ROWS = [ + {'ID': 1, 'NameLast': 'Smith', 'NickName': 'Al'}, + {'ID': 2, 'NameLast': 'Jones', 'NickName': 'Tim'}, + {'ID': 3, 'NameLast': 'Smith', 'NickName': 'Tim'}, + {'ID': 4, 'NameLast': 'Jones', 'NickName': 'Al'}, + {'ID': 5, 'NameLast': 'Smith', 'NickName': 'Other'}, + {'ID': 6, 'NameLast': 'Müller', 'NickName': 'René'}, +] + + +@pytest.fixture +def employees(schema): + """Build requests without making an HTTP connection.""" + return Service('http://odata.example.com', schema, requests).entity_sets.Employees.get_entities() + + +def selected_ids(expression): + """Use Python's independent parser for the tested eq/and/or OData subset.""" + tree = ast.parse(re.sub(r'\beq\b', '==', expression), mode='eval') + + def evaluate(node, row): + if isinstance(node, ast.Constant): + return node.value + if isinstance(node, ast.Name): + return row[node.id] + if isinstance(node, ast.Compare) and len(node.ops) == 1 and isinstance(node.ops[0], ast.Eq): + return evaluate(node.left, row) == evaluate(node.comparators[0], row) + if isinstance(node, ast.BoolOp): + values = (evaluate(value, row) for value in node.values) + if isinstance(node.op, ast.And): + return all(values) + if isinstance(node.op, ast.Or): + return any(values) + raise AssertionError(f'Unexpected filter node: {ast.dump(node)}') + + return {row['ID'] for row in ROWS if evaluate(tree.body, row)} + + +@pytest.mark.parametrize('build, expected, expected_filter, expected_url', [ + pytest.param( + lambda request: request.filter(Q(ID=1) | Q(NickName='Tim'), NameLast='Smith', group=True), + {1, 3}, "((NameLast eq 'Smith') and ((ID eq 1) or (NickName eq 'Tim')))", + 'http://odata.example.com/Employees?%24filter=' + '%28%28NameLast+eq+%27Smith%27%29+and+%28%28ID+eq+1%29+or+%28NickName+eq+%27Tim%27%29%29%29', + id='or-with-keyword'), + pytest.param( + lambda request: request.filter(ID__in=[2, 3], NameLast='Smith', group=True), + {3}, "((ID eq 2 or ID eq 3) and (NameLast eq 'Smith'))", + 'http://odata.example.com/Employees?%24filter=' + '%28%28ID+eq+2+or+ID+eq+3%29+and+%28NameLast+eq+%27Smith%27%29%29', + id='in-with-keyword'), + pytest.param( + lambda request: request.filter(Q(ID__in=[2, 3], NameLast='Smith'), group=True), + {3}, "((ID eq 2 or ID eq 3) and (NameLast eq 'Smith'))", + 'http://odata.example.com/Employees?%24filter=' + '%28%28ID+eq+2+or+ID+eq+3%29+and+%28NameLast+eq+%27Smith%27%29%29', + id='in-within-q'), + pytest.param( + lambda request: request.filter(Q(ID__in=[2, 3], NameLast='Smith') | Q(ID=5), group=True), + {3, 5}, "(((ID eq 2 or ID eq 3) and (NameLast eq 'Smith')) or (ID eq 5))", + 'http://odata.example.com/Employees?%24filter=' + '%28%28%28ID+eq+2+or+ID+eq+3%29+and+%28NameLast+eq+%27Smith%27%29%29+or+%28ID+eq+5%29%29', + id='compound-or'), + pytest.param( + lambda request: request.filter(Q(ID__in=[2, 3]) & Q(NameLast='Smith'), group=True), + {3}, "((ID eq 2 or ID eq 3) and (NameLast eq 'Smith'))", + 'http://odata.example.com/Employees?%24filter=' + '%28%28ID+eq+2+or+ID+eq+3%29+and+%28NameLast+eq+%27Smith%27%29%29', + id='binary-and'), + pytest.param( + lambda request: request.filter(Q(ID=2) | Q(ID=3), Q(NameLast='Smith'), group=True), + {3}, "(((ID eq 2) or (ID eq 3)) and (NameLast eq 'Smith'))", + 'http://odata.example.com/Employees?%24filter=' + '%28%28%28ID+eq+2%29+or+%28ID+eq+3%29%29+and+%28NameLast+eq+%27Smith%27%29%29', + id='multiple-q'), + pytest.param( + lambda request: request.filter(Q(ID=2) | Q(ID=3)).filter(NameLast='Smith', group=True), + {3}, "((ID eq 2) or (ID eq 3)) and (NameLast eq 'Smith')", + 'http://odata.example.com/Employees?%24filter=' + '%28%28ID+eq+2%29+or+%28ID+eq+3%29%29+and+%28NameLast+eq+%27Smith%27%29', + id='group-previous-call'), + pytest.param( + lambda request: request.filter(ID__in=[2, 3], group=True).filter(NameLast='Smith'), + {3}, "(ID eq 2 or ID eq 3) and NameLast eq 'Smith'", + 'http://odata.example.com/Employees?%24filter=' + '%28ID+eq+2+or+ID+eq+3%29+and+NameLast+eq+%27Smith%27', + id='group-before-legacy-tail'), + pytest.param( + lambda request: request.filter('ID eq 2 or ID eq 3').filter(NameLast='Smith', group=True), + {3}, "(ID eq 2 or ID eq 3) and (NameLast eq 'Smith')", + 'http://odata.example.com/Employees?%24filter=' + '%28ID+eq+2+or+ID+eq+3%29+and+%28NameLast+eq+%27Smith%27%29', + id='raw-before-group'), + pytest.param( + lambda request: request.filter('ID eq 2 or ID eq 3', group=True), + {2, 3}, '(ID eq 2 or ID eq 3)', + 'http://odata.example.com/Employees?%24filter=%28ID+eq+2+or+ID+eq+3%29', + id='raw-group'), + pytest.param( + lambda request: request.filter(ID=2).filter('ID eq 3', group=True), + {3}, '(ID eq 3)', + 'http://odata.example.com/Employees?%24filter=%28ID+eq+3%29', + id='raw-replaces-previous'), + pytest.param( + lambda request: request.filter(NameLast='Müller', NickName='René', group=True), + {6}, "((NameLast eq 'Müller') and (NickName eq 'René'))", + 'http://odata.example.com/Employees?%24filter=' + '%28%28NameLast+eq+%27M%C3%BCller%27%29+and+%28NickName+eq+%27Ren%C3%A9%27%29%29', + id='unicode-encoding'), +]) +@responses.activate +def test_grouped_filter_selects_expected_rows(employees, build, expected, expected_filter, expected_url): + query = build(employees) + expression = query.get_query_params()['$filter'] + assert expression == expected_filter + assert selected_ids(expression) == expected + responses.add(responses.GET, expected_url, json={'d': {'results': []}}) + assert query.execute() == [] + assert responses.calls[0].request.url == expected_url + + +def test_default_grouping_keeps_legacy_query(employees): + """Existing applications keep their exact query text unless they opt in.""" + expression = employees.filter(ID__in=[2, 3], NameLast='Smith').get_query_params()['$filter'] + assert expression == "ID eq 2 or ID eq 3 and NameLast eq 'Smith'" + assert selected_ids(expression) == {2, 3} + + +def test_empty_grouped_filter_is_a_noop(employees): + assert '$filter' not in employees.filter(group=True).get_query_params()