From 51fd78effa9d1481b6b34cb92830a4b1b144d7a0 Mon Sep 17 00:00:00 2001 From: Lukasz Lancucki Date: Fri, 13 Feb 2026 14:14:26 +0000 Subject: [PATCH] fix(rql): enforce single-quoted values for eq/ne/gt/ge/lt/le and align tests --- mpt_api_client/rql/query_builder.py | 16 ++++++++++++++-- tests/unit/http/mixins/test_collection_mixin.py | 12 ++++++------ tests/unit/http/test_base_service.py | 6 +++--- tests/unit/http/test_query_state.py | 4 ++-- tests/unit/rql/query_builder/test_create_rql.py | 10 +++++----- .../unit/rql/query_builder/test_multiple_ops.py | 12 +++++++----- tests/unit/rql/query_builder/test_rql.py | 6 +++--- tests/unit/rql/query_builder/test_rql_all_any.py | 4 ++-- .../unit/rql/query_builder/test_rql_dot_path.py | 16 ++++++++-------- .../unit/rql/query_builder/test_rql_from_str.py | 2 +- .../rql/query_builder/test_rql_parse_kwargs.py | 4 ++-- 11 files changed, 53 insertions(+), 39 deletions(-) diff --git a/mpt_api_client/rql/query_builder.py b/mpt_api_client/rql/query_builder.py index c1b05212..72d3041b 100644 --- a/mpt_api_client/rql/query_builder.py +++ b/mpt_api_client/rql/query_builder.py @@ -9,7 +9,13 @@ QueryValue = str | bool | dt.date | dt.datetime | Numeric -def parse_kwargs(query_dict: dict[str, QueryValue]) -> list[str]: # noqa: WPS231 +def quote_rql_value(value: str) -> str: + """Wrap value in single quotes for RQL comparison operators.""" + escaped_value = value.replace("'", r"\'") + return f"'{escaped_value}'" + + +def parse_kwargs(query_dict: dict[str, QueryValue]) -> list[str]: # noqa: WPS231 C901 """ Parse keyword arguments into RQL query expressions. @@ -26,7 +32,7 @@ def parse_kwargs(query_dict: dict[str, QueryValue]) -> list[str]: # noqa: WPS23 Examples: parse_kwargs({'name': 'John', 'age__gt': 25}) - ['eq(name,John)', 'gt(age,25)'] + ["eq(name,'John')", "gt(age,'25')"] parse_kwargs({'status__in': ['active', 'pending']}) ['in(status,(active,pending))'] @@ -37,17 +43,21 @@ def parse_kwargs(query_dict: dict[str, QueryValue]) -> list[str]: # noqa: WPS23 if len(tokens) == 1: field = tokens[0] str_value = rql_encode("eq", value) + str_value = quote_rql_value(str_value) query.append(f"eq({field},{str_value})") continue op = tokens[-1] if op not in constants.KEYWORDS: field = ".".join(tokens) str_value = rql_encode("eq", value) + str_value = quote_rql_value(str_value) query.append(f"eq({field},{str_value})") continue field = ".".join(tokens[:-1]) if op in constants.COMP or op in constants.SEARCH: str_value = rql_encode(op, value) + if op in constants.COMP: + str_value = quote_rql_value(str_value) query.append(f"{op}({field},{str_value})") continue if op in constants.LIST: @@ -462,6 +472,8 @@ def ilike(self, value: QueryValue) -> Self: def _bin(self, op: str, value: QueryValue) -> Self: self._field = ".".join(self._path) value = rql_encode(op, value) + if op in constants.COMP: + value = quote_rql_value(value) self.expr = f"{op}({self._field},{value})" return self diff --git a/tests/unit/http/mixins/test_collection_mixin.py b/tests/unit/http/mixins/test_collection_mixin.py index bafb5acc..646b6fab 100644 --- a/tests/unit/http/mixins/test_collection_mixin.py +++ b/tests/unit/http/mixins/test_collection_mixin.py @@ -73,7 +73,7 @@ def test_col_mx_fetch_one_with_filters( assert first_request.url == ( "https://api.example.com/api/v1/test" "?limit=1&offset=0&order=created" - "&select=id,name&eq(status,active)" + "&select=id,name&eq(status,'active')" ) @@ -91,7 +91,7 @@ def test_col_mx_fetch_page_with_filter( "https://api.example.com/api/v1/test?limit=10&offset=5" "&order=-created,name" "&select=-audit,product.agreements,-product.agreements.product" - "&eq(status,active)" + "&eq(status,'active')" ) with respx.mock: mock_route = respx.get("https://api.example.com/api/v1/test").mock( @@ -213,7 +213,7 @@ def test_col_mx_iterate_with_filters( request = mock_route.calls[0].request assert ( str(request.url) == "https://api.example.com/api/v1/test" - "?limit=100&offset=0&order=created&select=id,name&eq(status,active)" + "?limit=100&offset=0&order=created&select=id,name&eq(status,'active')" ) @@ -322,7 +322,7 @@ async def test_async_col_mx_fetch_one_with_filters( assert first_request.url == ( "https://api.example.com/api/v1/test" "?limit=1&offset=0&order=created" - "&select=id,name&eq(status,active)" + "&select=id,name&eq(status,'active')" ) @@ -342,7 +342,7 @@ async def test_async_col_mx_fetch_page_with_filter( "https://api.example.com/api/v1/test?limit=10&offset=5" "&order=-created,name" "&select=-audit,product.agreements,-product.agreements.product" - "&eq(status,active)" + "&eq(status,'active')" ) with respx.mock: 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( request = mock_route.calls[0].request assert ( str(request.url) == "https://api.example.com/api/v1/test" - "?limit=100&offset=0&order=created&select=id,name&eq(status,active)" + "?limit=100&offset=0&order=created&select=id,name&eq(status,'active')" ) diff --git a/tests/unit/http/test_base_service.py b/tests/unit/http/test_base_service.py index 70c2a15a..612bcc84 100644 --- a/tests/unit/http/test_base_service.py +++ b/tests/unit/http/test_base_service.py @@ -52,7 +52,7 @@ def test_build_url_with_query_state(http_client, filter_status_active): result = service_with_state.build_path() - assert result == "/api/v1/test?order=created,-name&select=id,name&eq(status,active)" + assert result == "/api/v1/test?order=created,-name&select=id,name&eq(status,'active')" 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 result = service_with_state.build_path(query_params) - assert result == "/api/v2/test/T-123?limit=5&eq(status,active)" + assert result == "/api/v2/test/T-123?limit=5&eq(status,'active')" 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): result = chained_service.build_path({"limit": "10"}) expected_url = ( - "/api/v1/test?limit=10&order=-created,name&select=id,name,-audit&eq(status,active)" + "/api/v1/test?limit=10&order=-created,name&select=id,name,-audit&eq(status,'active')" ) assert result == expected_url diff --git a/tests/unit/http/test_query_state.py b/tests/unit/http/test_query_state.py index 98e131cf..210029be 100644 --- a/tests/unit/http/test_query_state.py +++ b/tests/unit/http/test_query_state.py @@ -30,7 +30,7 @@ def test_build_url(filter_status_active): assert result == ( "order=-created,name" "&select=-audit,product.agreements,-product.agreements.product" - "&eq(status,active)" + "&eq(status,'active')" ) @@ -46,4 +46,4 @@ def test_build_with_params(filter_status_active): result = query_state.build(query_params) - assert result == "limit=10&order=created&select=name&eq(status,active)" + assert result == "limit=10&order=created&select=name&eq(status,'active')" diff --git a/tests/unit/rql/query_builder/test_create_rql.py b/tests/unit/rql/query_builder/test_create_rql.py index 08f6e9cd..a04726b1 100644 --- a/tests/unit/rql/query_builder/test_create_rql.py +++ b/tests/unit/rql/query_builder/test_create_rql.py @@ -15,14 +15,14 @@ def test_create_with_field(): query.eq("value") # act assert query.op == RQLQuery.OP_EXPRESSION - assert str(query) == "eq(field,value)" + assert str(query) == "eq(field,'value')" def test_create_single_kwarg(): result = RQLQuery(id="ID") assert result.op == RQLQuery.OP_EXPRESSION - assert str(result) == "eq(id,ID)" + assert str(result) == "eq(id,'ID')" assert result.children == [] assert result.negated is False @@ -31,17 +31,17 @@ def test_create_multiple_kwargs(): # noqa: WPS218 result = RQLQuery(id="ID", status__in=("a", "b"), ok=True) assert result.op == RQLQuery.OP_AND - assert str(result) == "and(eq(id,ID),in(status,(a,b)),eq(ok,true))" + assert str(result) == "and(eq(id,'ID'),in(status,(a,b)),eq(ok,'true'))" assert len(result.children) == 3 assert result.children[0].op == RQLQuery.OP_EXPRESSION assert result.children[0].children == [] - assert str(result.children[0]) == "eq(id,ID)" + assert str(result.children[0]) == "eq(id,'ID')" assert result.children[1].op == RQLQuery.OP_EXPRESSION assert result.children[1].children == [] assert str(result.children[1]) == "in(status,(a,b))" assert result.children[2].op == RQLQuery.OP_EXPRESSION assert result.children[2].children == [] - assert str(result.children[2]) == "eq(ok,true)" + assert str(result.children[2]) == "eq(ok,'true')" def test_new_empty(): diff --git a/tests/unit/rql/query_builder/test_multiple_ops.py b/tests/unit/rql/query_builder/test_multiple_ops.py index c724773c..e31218cb 100644 --- a/tests/unit/rql/query_builder/test_multiple_ops.py +++ b/tests/unit/rql/query_builder/test_multiple_ops.py @@ -11,23 +11,25 @@ def test_and_or(): # noqa: WPS218 WPS473 AAA01 r5 = r1 & r2 & (r3 | r4) assert r5.op == RQLQuery.OP_AND - assert str(r5) == "and(eq(id,ID),eq(field,value),or(eq(other,value2),in(inop,(a,b))))" # noqa: WPS204 + assert str(r5) == "and(eq(id,'ID'),eq(field,'value'),or(eq(other,'value2'),in(inop,(a,b))))" # noqa: WPS204 r5 = r1 & r2 | r3 - assert str(r5) == "or(and(eq(id,ID),eq(field,value)),eq(other,value2))" + assert str(r5) == "or(and(eq(id,'ID'),eq(field,'value')),eq(other,'value2'))" r5 = r1 & (r2 | r3) - assert str(r5) == "and(eq(id,ID),or(eq(field,value),eq(other,value2)))" + assert str(r5) == "and(eq(id,'ID'),or(eq(field,'value'),eq(other,'value2')))" r5 = (r1 & r2) | (r3 & r4) - assert str(r5) == "or(and(eq(id,ID),eq(field,value)),and(eq(other,value2),in(inop,(a,b))))" + assert ( + str(r5) == "or(and(eq(id,'ID'),eq(field,'value')),and(eq(other,'value2'),in(inop,(a,b))))" + ) r5 = (r1 & r2) | ~r3 - assert str(r5) == "or(and(eq(id,ID),eq(field,value)),not(eq(other,value2)))" + assert str(r5) == "or(and(eq(id,'ID'),eq(field,'value')),not(eq(other,'value2')))" def test_and_merge(): # noqa: WPS210 AAA01 diff --git a/tests/unit/rql/query_builder/test_rql.py b/tests/unit/rql/query_builder/test_rql.py index 5e393a18..2019b394 100644 --- a/tests/unit/rql/query_builder/test_rql.py +++ b/tests/unit/rql/query_builder/test_rql.py @@ -28,9 +28,9 @@ def test_bool(): # noqa: AAA01 def test_str(): # noqa: AAA01 - assert str(RQLQuery(id="ID")) == "eq(id,ID)" - assert str(~RQLQuery(id="ID")) == "not(eq(id,ID))" - assert str(~RQLQuery(id="ID", field="value")) == "not(and(eq(id,ID),eq(field,value)))" + assert str(RQLQuery(id="ID")) == "eq(id,'ID')" + assert str(~RQLQuery(id="ID")) == "not(eq(id,'ID'))" + assert str(~RQLQuery(id="ID", field="value")) == "not(and(eq(id,'ID'),eq(field,'value')))" assert not str(RQLQuery()) diff --git a/tests/unit/rql/query_builder/test_rql_all_any.py b/tests/unit/rql/query_builder/test_rql_all_any.py index ae0ea6a6..263551f5 100644 --- a/tests/unit/rql/query_builder/test_rql_all_any.py +++ b/tests/unit/rql/query_builder/test_rql_all_any.py @@ -6,7 +6,7 @@ def test_all(): result = str(query) - assert result == "all(gt(saleDetails.orderQty,1))" + assert result == "all(gt(saleDetails.orderQty,'1'))" def test_any(): @@ -14,4 +14,4 @@ def test_any(): result = str(query) - assert result == "any(gt(saleDetails.orderQty,1))" + assert result == "any(gt(saleDetails.orderQty,'1'))" diff --git a/tests/unit/rql/query_builder/test_rql_dot_path.py b/tests/unit/rql/query_builder/test_rql_dot_path.py index a32c819a..bffcdc72 100644 --- a/tests/unit/rql/query_builder/test_rql_dot_path.py +++ b/tests/unit/rql/query_builder/test_rql_dot_path.py @@ -15,8 +15,8 @@ class Test: # noqa: WPS431 test = Test() today = dt.datetime.now(dt.UTC).date() now = dt.datetime.now(dt.UTC) - today_expected_result = f"{op}(asset.id,{today.isoformat()})" - now_expected_result = f"{op}(asset.id,{now.isoformat()})" + today_expected_result = f"{op}(asset.id,'{today.isoformat()}')" + now_expected_result = f"{op}(asset.id,'{now.isoformat()}')" with pytest.raises(TypeError): getattr(RQLQuery().asset.id, op)(test) @@ -29,9 +29,9 @@ class Test: # noqa: WPS431 def test_dotted_path_comp_bool_and_str(op): result = getattr(RQLQuery().asset.id, op) - assert str(result("value")) == f"{op}(asset.id,value)" - assert str(result(True)) == f"{op}(asset.id,true)" # noqa: FBT003 - assert str(result(False)) == f"{op}(asset.id,false)" # noqa: FBT003 + assert str(result("value")) == f"{op}(asset.id,'value')" + assert str(result(True)) == f"{op}(asset.id,'true')" # noqa: FBT003 + assert str(result(False)) == f"{op}(asset.id,'false')" # noqa: FBT003 @pytest.mark.parametrize("op", ["eq", "ne", "gt", "ge", "le", "lt"]) # noqa: AAA01 @@ -43,9 +43,9 @@ def test_dotted_path_comp_numerics(op): result_float = str(attribute_op_match(10.678937)) decimal_result = str(attribute_op_match(Decimal("32983.328238273"))) - assert integer_result == f"{op}(asset.id,10)" - assert result_float == f"{op}(asset.id,10.678937)" - assert decimal_result == f"{op}(asset.id,{decimal_object!s})" + assert integer_result == f"{op}(asset.id,'10')" + assert result_float == f"{op}(asset.id,'10.678937')" + assert decimal_result == f"{op}(asset.id,'{decimal_object!s}')" @pytest.mark.parametrize("op", ["like", "ilike"]) diff --git a/tests/unit/rql/query_builder/test_rql_from_str.py b/tests/unit/rql/query_builder/test_rql_from_str.py index bc704831..dcc192f7 100644 --- a/tests/unit/rql/query_builder/test_rql_from_str.py +++ b/tests/unit/rql/query_builder/test_rql_from_str.py @@ -2,7 +2,7 @@ def test_rql_from_str(): - str_query = "eq(id,ID)" + str_query = "eq(id,'ID')" result = RQLQuery.from_string(str_query) diff --git a/tests/unit/rql/query_builder/test_rql_parse_kwargs.py b/tests/unit/rql/query_builder/test_rql_parse_kwargs.py index b6160eb6..bcbb94f2 100644 --- a/tests/unit/rql/query_builder/test_rql_parse_kwargs.py +++ b/tests/unit/rql/query_builder/test_rql_parse_kwargs.py @@ -18,7 +18,7 @@ def test_improper_op(mock_product_id_for_expression): result = parse_kwargs(products_expr) - assert str(result) == f"['eq(product.id.inn,{mock_product_id_for_expression})']" + assert str(result) == f"[\"eq(product.id.inn,'{mock_product_id_for_expression}')\"]" def test_parse_eq(mock_product_id_for_expression): @@ -26,7 +26,7 @@ def test_parse_eq(mock_product_id_for_expression): result = parse_kwargs(products_expr) - assert str(result) == f"['eq(product.id,{mock_product_id_for_expression})']" + assert str(result) == f"[\"eq(product.id,'{mock_product_id_for_expression}')\"]" def test_parse_like(mock_product_id_for_expression):