Skip to content

Commit d0c4a94

Browse files
authored
MPT-18065 - Fix backward incompatible issues related to RQL: Support of property comparison (#213)
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> Closes [MPT-18065](https://softwareone.atlassian.net/browse/MPT-18065) - Add RQLProperty and RQLValue wrapper classes in mpt_api_client.rql.query_builder and export them from mpt_api_client.rql - Support property-to-property comparisons and null() via RQLProperty.null() - Quote string and date/datetime literals in RQL serialization (e.g., eq(status,active) → eq(status,'active')) - Broaden query_value_str signature to accept Any and delegate to RQLValue/RQLProperty when appropriate - Update rql_encode to handle RQLValue/RQLProperty and list operands correctly - Update and add unit tests to reflect quoting and Property/Value behavior <!-- end of auto-generated comment: release notes by coderabbit.ai --> [MPT-18065]: https://softwareone.atlassian.net/browse/MPT-18065?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
2 parents a94e68d + 92cd854 commit d0c4a94

File tree

12 files changed

+149
-60
lines changed

12 files changed

+149
-60
lines changed

mpt_api_client/rql/__init__.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
from mpt_api_client.rql.query_builder import RQLQuery
1+
from mpt_api_client.rql.query_builder import RQLProperty, RQLQuery, RQLValue
22

3-
__all__ = ["RQLQuery"] # noqa: WPS410
3+
__all__ = ["RQLProperty", "RQLQuery", "RQLValue"] # noqa: WPS410

mpt_api_client/rql/query_builder.py

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,43 @@
99
QueryValue = str | bool | dt.date | dt.datetime | Numeric
1010

1111

12+
class RQLProperty:
13+
"""Wrapper for model properties in RQL queries."""
14+
15+
def __init__(self, value: str) -> None:
16+
self.value = value
17+
18+
@override
19+
def __str__(self) -> str:
20+
return self.value
21+
22+
@classmethod
23+
def null(cls) -> Self:
24+
"""Returns `null()` operator."""
25+
return cls("null()")
26+
27+
28+
class RQLValue:
29+
"""Wrapper for literal values in RQL queries."""
30+
31+
def __init__(self, value: QueryValue) -> None:
32+
self.value = value
33+
34+
@override
35+
def __str__(self) -> str:
36+
if isinstance(self.value, str):
37+
return f"'{self.value}'"
38+
if isinstance(self.value, bool):
39+
return "true" if self.value else "false"
40+
41+
if isinstance(self.value, dt.date | dt.datetime):
42+
str_time = self.value.isoformat()
43+
return f"'{str_time}'"
44+
45+
# Matching: if isinstance(value, int | float | Decimal):
46+
return str(self.value)
47+
48+
1249
def parse_kwargs(query_dict: dict[str, QueryValue]) -> list[str]: # noqa: WPS231
1350
"""
1451
Parse keyword arguments into RQL query expressions.
@@ -62,16 +99,14 @@ def parse_kwargs(query_dict: dict[str, QueryValue]) -> list[str]: # noqa: WPS23
6299
return query
63100

64101

65-
def query_value_str(value: QueryValue) -> str:
102+
def query_value_str(value: Any) -> str:
66103
"""Converts a value to string for use in RQL queries."""
67-
if isinstance(value, str):
68-
return value
69-
if isinstance(value, bool):
70-
return "true" if value else "false"
71-
72-
if isinstance(value, dt.date | dt.datetime):
73-
return value.isoformat()
74-
# Matching: if isinstance(value, int | float | Decimal):
104+
if isinstance(value, QueryValue):
105+
value = RQLValue(value)
106+
if isinstance(value, RQLValue):
107+
return str(value)
108+
if isinstance(value, RQLProperty):
109+
return str(value)
75110
return str(value)
76111

77112

@@ -104,10 +139,10 @@ def rql_encode(op: str, value: Any) -> str:
104139
rql_encode('in', ['a', 'b', 'c'])
105140
'a,b,c'
106141
"""
107-
if op not in constants.LIST and isinstance(value, QueryValue):
142+
if op not in constants.LIST and isinstance(value, QueryValue | RQLValue | RQLProperty):
108143
return query_value_str(value)
109144
if op in constants.LIST and isinstance(value, list | tuple | set):
110-
return ",".join(str(el) for el in value)
145+
return ",".join(query_value_str(el) for el in value)
111146

112147
raise TypeError(f"the `{op}` operator doesn't support the {type(value)} type.")
113148

tests/unit/http/mixins/test_collection_mixin.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ def test_col_mx_fetch_one_with_filters(
7373
assert first_request.url == (
7474
"https://api.example.com/api/v1/test"
7575
"?limit=1&offset=0&order=created"
76-
"&select=id,name&eq(status,active)"
76+
"&select=id,name&eq(status,'active')"
7777
)
7878

7979

@@ -91,7 +91,7 @@ def test_col_mx_fetch_page_with_filter(
9191
"https://api.example.com/api/v1/test?limit=10&offset=5"
9292
"&order=-created,name"
9393
"&select=-audit,product.agreements,-product.agreements.product"
94-
"&eq(status,active)"
94+
"&eq(status,'active')"
9595
)
9696
with respx.mock:
9797
mock_route = respx.get("https://api.example.com/api/v1/test").mock(
@@ -213,7 +213,7 @@ def test_col_mx_iterate_with_filters(
213213
request = mock_route.calls[0].request
214214
assert (
215215
str(request.url) == "https://api.example.com/api/v1/test"
216-
"?limit=100&offset=0&order=created&select=id,name&eq(status,active)"
216+
"?limit=100&offset=0&order=created&select=id,name&eq(status,'active')"
217217
)
218218

219219

@@ -322,7 +322,7 @@ async def test_async_col_mx_fetch_one_with_filters(
322322
assert first_request.url == (
323323
"https://api.example.com/api/v1/test"
324324
"?limit=1&offset=0&order=created"
325-
"&select=id,name&eq(status,active)"
325+
"&select=id,name&eq(status,'active')"
326326
)
327327

328328

@@ -342,7 +342,7 @@ async def test_async_col_mx_fetch_page_with_filter(
342342
"https://api.example.com/api/v1/test?limit=10&offset=5"
343343
"&order=-created,name"
344344
"&select=-audit,product.agreements,-product.agreements.product"
345-
"&eq(status,active)"
345+
"&eq(status,'active')"
346346
)
347347
with respx.mock:
348348
mock_route = respx.get("https://api.example.com/api/v1/test").mock(
@@ -464,7 +464,7 @@ async def test_async_col_mx_iterate_with_filters(
464464
request = mock_route.calls[0].request
465465
assert (
466466
str(request.url) == "https://api.example.com/api/v1/test"
467-
"?limit=100&offset=0&order=created&select=id,name&eq(status,active)"
467+
"?limit=100&offset=0&order=created&select=id,name&eq(status,'active')"
468468
)
469469

470470

tests/unit/http/test_base_service.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ def test_build_url_with_query_state(http_client, filter_status_active):
5252

5353
result = service_with_state.build_path()
5454

55-
assert result == "/api/v1/test?order=created,-name&select=id,name&eq(status,active)"
55+
assert result == "/api/v1/test?order=created,-name&select=id,name&eq(status,'active')"
5656

5757

5858
def test_build_url_with_query_state_and_params(http_client, filter_status_active):
@@ -65,7 +65,7 @@ def test_build_url_with_query_state_and_params(http_client, filter_status_active
6565

6666
result = service_with_state.build_path(query_params)
6767

68-
assert result == "/api/v2/test/T-123?limit=5&eq(status,active)"
68+
assert result == "/api/v2/test/T-123?limit=5&eq(status,'active')"
6969

7070

7171
def test_build_url_with_chained_methods(dummy_service, filter_status_active):
@@ -79,6 +79,6 @@ def test_build_url_with_chained_methods(dummy_service, filter_status_active):
7979
result = chained_service.build_path({"limit": "10"})
8080

8181
expected_url = (
82-
"/api/v1/test?limit=10&order=-created,name&select=id,name,-audit&eq(status,active)"
82+
"/api/v1/test?limit=10&order=-created,name&select=id,name,-audit&eq(status,'active')"
8383
)
8484
assert result == expected_url

tests/unit/http/test_query_state.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ def test_build_url(filter_status_active):
3030
assert result == (
3131
"order=-created,name"
3232
"&select=-audit,product.agreements,-product.agreements.product"
33-
"&eq(status,active)"
33+
"&eq(status,'active')"
3434
)
3535

3636

@@ -46,4 +46,4 @@ def test_build_with_params(filter_status_active):
4646

4747
result = query_state.build(query_params)
4848

49-
assert result == "limit=10&order=created&select=name&eq(status,active)"
49+
assert result == "limit=10&order=created&select=name&eq(status,'active')"

tests/unit/rql/query_builder/test_create_rql.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ def test_create_with_field():
1515
query.eq("value") # act
1616

1717
assert query.op == RQLQuery.OP_EXPRESSION
18-
assert str(query) == "eq(field,value)"
18+
assert str(query) == "eq(field,'value')"
1919

2020

2121
def test_create_single_kwarg():
2222
result = RQLQuery(id="ID")
2323

2424
assert result.op == RQLQuery.OP_EXPRESSION
25-
assert str(result) == "eq(id,ID)"
25+
assert str(result) == "eq(id,'ID')"
2626
assert result.children == []
2727
assert result.negated is False
2828

@@ -31,14 +31,14 @@ def test_create_multiple_kwargs(): # noqa: WPS218
3131
result = RQLQuery(id="ID", status__in=("a", "b"), ok=True)
3232

3333
assert result.op == RQLQuery.OP_AND
34-
assert str(result) == "and(eq(id,ID),in(status,(a,b)),eq(ok,true))"
34+
assert str(result) == "and(eq(id,'ID'),in(status,('a','b')),eq(ok,true))"
3535
assert len(result.children) == 3
3636
assert result.children[0].op == RQLQuery.OP_EXPRESSION
3737
assert result.children[0].children == []
38-
assert str(result.children[0]) == "eq(id,ID)"
38+
assert str(result.children[0]) == "eq(id,'ID')"
3939
assert result.children[1].op == RQLQuery.OP_EXPRESSION
4040
assert result.children[1].children == []
41-
assert str(result.children[1]) == "in(status,(a,b))"
41+
assert str(result.children[1]) == "in(status,('a','b'))"
4242
assert result.children[2].op == RQLQuery.OP_EXPRESSION
4343
assert result.children[2].children == []
4444
assert str(result.children[2]) == "eq(ok,true)"

tests/unit/rql/query_builder/test_multiple_ops.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,23 +11,26 @@ def test_and_or(): # noqa: WPS218 WPS473 AAA01
1111
r5 = r1 & r2 & (r3 | r4)
1212

1313
assert r5.op == RQLQuery.OP_AND
14-
assert str(r5) == "and(eq(id,ID),eq(field,value),or(eq(other,value2),in(inop,(a,b))))" # noqa: WPS204
14+
assert str(r5) == "and(eq(id,'ID'),eq(field,'value'),or(eq(other,'value2'),in(inop,('a','b'))))" # noqa: WPS204
1515

1616
r5 = r1 & r2 | r3
1717

18-
assert str(r5) == "or(and(eq(id,ID),eq(field,value)),eq(other,value2))"
18+
assert str(r5) == "or(and(eq(id,'ID'),eq(field,'value')),eq(other,'value2'))"
1919

2020
r5 = r1 & (r2 | r3)
2121

22-
assert str(r5) == "and(eq(id,ID),or(eq(field,value),eq(other,value2)))"
22+
assert str(r5) == "and(eq(id,'ID'),or(eq(field,'value'),eq(other,'value2')))"
2323

2424
r5 = (r1 & r2) | (r3 & r4)
2525

26-
assert str(r5) == "or(and(eq(id,ID),eq(field,value)),and(eq(other,value2),in(inop,(a,b))))"
26+
assert (
27+
str(r5)
28+
== "or(and(eq(id,'ID'),eq(field,'value')),and(eq(other,'value2'),in(inop,('a','b'))))"
29+
)
2730

2831
r5 = (r1 & r2) | ~r3
2932

30-
assert str(r5) == "or(and(eq(id,ID),eq(field,value)),not(eq(other,value2)))"
33+
assert str(r5) == "or(and(eq(id,'ID'),eq(field,'value')),not(eq(other,'value2')))"
3134

3235

3336
def test_and_merge(): # noqa: WPS210 AAA01
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
from mpt_api_client.rql import RQLProperty, RQLQuery, RQLValue
2+
3+
4+
def test_compare_default_value():
5+
query = RQLQuery(agreement__product__id="order.product.id")
6+
7+
result = str(query)
8+
9+
assert result == "eq(agreement.product.id,'order.product.id')"
10+
11+
12+
def test_compare_quoted():
13+
query = RQLQuery(agreement__product__id=RQLValue("order.product.id"))
14+
15+
result = str(query)
16+
17+
assert result == "eq(agreement.product.id,'order.product.id')"
18+
19+
20+
def test_compare_property():
21+
query = RQLQuery(agreement__product__id=RQLProperty("order.product.id"))
22+
23+
result = str(query)
24+
25+
assert result == "eq(agreement.product.id,order.product.id)"
26+
27+
28+
def test_compare_null():
29+
query = RQLQuery(agreement__product__id=RQLProperty.null())
30+
31+
result = str(query)
32+
33+
assert result == "eq(agreement.product.id,null())"
34+
35+
36+
def test_ne_quoted():
37+
query = RQLQuery("agreement.product.id")
38+
39+
result = str(query.ne(RQLValue("order.product.id")))
40+
41+
assert result == "ne(agreement.product.id,'order.product.id')"
42+
43+
44+
def test_ne_property():
45+
query = RQLQuery("agreement.product.id")
46+
47+
result = str(query.ne(RQLProperty("order.product.id")))
48+
49+
assert result == "ne(agreement.product.id,order.product.id)"

tests/unit/rql/query_builder/test_rql.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
1-
from mpt_api_client.rql import RQLQuery
1+
from mpt_api_client.rql import RQLProperty, RQLQuery, RQLValue
22

33

44
def test_repr(): # noqa: AAA01
5-
products = ["PRD-1", "PRD-2"]
6-
product_ids = ",".join(products)
5+
products = ["PRD-1", RQLValue("PRD-2"), RQLProperty("agreement.product.id")]
76
expression_query = RQLQuery(product__id__in=products)
87
or_expression = RQLQuery(name="Albert") | RQLQuery(surname="Einstein")
98

10-
assert repr(expression_query) == f"<RQLQuery(expr) in(product.id,({product_ids}))>"
9+
assert (
10+
repr(expression_query)
11+
== "<RQLQuery(expr) in(product.id,('PRD-1','PRD-2',agreement.product.id))>"
12+
)
1113
assert repr(or_expression) == "<RQLQuery(or)>"
1214

1315

@@ -28,9 +30,9 @@ def test_bool(): # noqa: AAA01
2830

2931

3032
def test_str(): # noqa: AAA01
31-
assert str(RQLQuery(id="ID")) == "eq(id,ID)"
32-
assert str(~RQLQuery(id="ID")) == "not(eq(id,ID))"
33-
assert str(~RQLQuery(id="ID", field="value")) == "not(and(eq(id,ID),eq(field,value)))"
33+
assert str(RQLQuery(id="ID")) == "eq(id,'ID')"
34+
assert str(~RQLQuery(id="ID")) == "not(eq(id,'ID'))"
35+
assert str(~RQLQuery(id="ID", field="value")) == "not(and(eq(id,'ID'),eq(field,'value')))"
3436
assert not str(RQLQuery())
3537

3638

tests/unit/rql/query_builder/test_rql_dot_path.py

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import pytest
55

6-
from mpt_api_client.rql import RQLQuery
6+
from mpt_api_client.rql import RQLProperty, RQLQuery
77

88

99
@pytest.mark.parametrize("op", ["eq", "ne", "gt", "ge", "le", "lt"])
@@ -15,8 +15,8 @@ class Test: # noqa: WPS431
1515
test = Test()
1616
today = dt.datetime.now(dt.UTC).date()
1717
now = dt.datetime.now(dt.UTC)
18-
today_expected_result = f"{op}(asset.id,{today.isoformat()})"
19-
now_expected_result = f"{op}(asset.id,{now.isoformat()})"
18+
today_expected_result = f"{op}(asset.id,'{today.isoformat()}')"
19+
now_expected_result = f"{op}(asset.id,'{now.isoformat()}')"
2020

2121
with pytest.raises(TypeError):
2222
getattr(RQLQuery().asset.id, op)(test)
@@ -29,7 +29,7 @@ class Test: # noqa: WPS431
2929
def test_dotted_path_comp_bool_and_str(op):
3030
result = getattr(RQLQuery().asset.id, op)
3131

32-
assert str(result("value")) == f"{op}(asset.id,value)"
32+
assert str(result("value")) == f"{op}(asset.id,'value')"
3333
assert str(result(True)) == f"{op}(asset.id,true)" # noqa: FBT003
3434
assert str(result(False)) == f"{op}(asset.id,false)" # noqa: FBT003
3535

@@ -52,10 +52,10 @@ def test_dotted_path_comp_numerics(op):
5252
def test_dotted_path_search(op):
5353
result = getattr(RQLQuery().asset.id, op)
5454

55-
assert str(result("value")) == f"{op}(asset.id,value)"
56-
assert str(result("*value")) == f"{op}(asset.id,*value)"
57-
assert str(result("value*")) == f"{op}(asset.id,value*)"
58-
assert str(result("*value*")) == f"{op}(asset.id,*value*)"
55+
assert str(result("value")) == f"{op}(asset.id,'value')"
56+
assert str(result("*value")) == f"{op}(asset.id,'*value')"
57+
assert str(result("value*")) == f"{op}(asset.id,'value*')"
58+
assert str(result("*value*")) == f"{op}(asset.id,'*value*')"
5959

6060

6161
@pytest.mark.parametrize(
@@ -67,14 +67,15 @@ def test_dotted_path_search(op):
6767
],
6868
)
6969
def test_dotted_path_list(method, op): # noqa: AAA01
70-
rexpr_set = getattr(RQLQuery().asset.id, method)(("first", "second"))
71-
rexpr_list = getattr(RQLQuery().asset.id, method)(["first", "second"])
70+
third = RQLProperty("third")
71+
rexpr_set = getattr(RQLQuery().asset.id, method)(("first", "second", third))
72+
rexpr_list = getattr(RQLQuery().asset.id, method)(["first", "second", third])
7273

7374
with pytest.raises(TypeError):
7475
getattr(RQLQuery().asset.id, method)("Test")
7576

76-
assert str(rexpr_set) == f"{op}(asset.id,(first,second))"
77-
assert str(rexpr_list) == f"{op}(asset.id,(first,second))"
77+
assert str(rexpr_set) == f"{op}(asset.id,('first','second',third))"
78+
assert str(rexpr_list) == f"{op}(asset.id,('first','second',third))"
7879

7980

8081
@pytest.mark.parametrize(

0 commit comments

Comments
 (0)