From aa30539562cc88fdf08f2d7bbc20ccfef0521291 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Tue, 15 Apr 2025 10:52:48 +0200 Subject: [PATCH 01/11] fix(slashes_in_pk): in str primary keys slashes are now working --- .../resources/collections/requests.py | 14 ++++++++++++++ .../services/serializers/json_api_serializer.py | 17 +++++++++++++---- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py b/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py index 37c6c0809..8e5874849 100644 --- a/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py +++ b/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py @@ -1,8 +1,10 @@ from typing import Any, Dict, Optional, TypedDict, TypeVar, Union +from urllib.parse import unquote from forestadmin.agent_toolkit.exceptions import AgentToolkitException from forestadmin.agent_toolkit.resources.collections.exceptions import CollectionResourceException from forestadmin.agent_toolkit.utils.context import Request, RequestMethod, User +from forestadmin.agent_toolkit.utils.id import pack_id, unpack_id from forestadmin.datasource_toolkit.collections import Collection, CollectionException from forestadmin.datasource_toolkit.datasource_customizer.collection_customizer import CollectionCustomizer from forestadmin.datasource_toolkit.datasource_customizer.datasource_customizer import DatasourceCustomizer @@ -12,10 +14,12 @@ ManyToOne, OneToMany, OneToOne, + PrimitiveType, is_polymorphic_many_to_one, is_reverse_polymorphic_relation, is_straight_relation, ) +from forestadmin.datasource_toolkit.utils.schema import SchemaUtils from typing_extensions import Self BoundCollection = TypeVar("BoundCollection", bound=Collection) @@ -88,6 +92,16 @@ def pks(self): raise CollectionResourceException("") try: pks = self.query["pks"] + unpacked_pks = unpack_id(self.collection.schema, pks) + pks_names = SchemaUtils.get_primary_keys(self.collection.schema) + record_like_pks = {} + for i, pk_name in enumerate(pks_names): + pk_schema = self.collection.schema["fields"][pk_name] + if pk_schema["column_type"] == PrimitiveType.STRING: + unpacked_pks[i] = unquote(unpacked_pks[i]) + record_like_pks[pk_name] = unpacked_pks[i] + pks = pack_id(self.collection.schema, record_like_pks) + except KeyError: raise CollectionResourceException("primary keys are missing") return pks diff --git a/src/agent_toolkit/forestadmin/agent_toolkit/services/serializers/json_api_serializer.py b/src/agent_toolkit/forestadmin/agent_toolkit/services/serializers/json_api_serializer.py index f015a6ad9..6fcc9bb63 100644 --- a/src/agent_toolkit/forestadmin/agent_toolkit/services/serializers/json_api_serializer.py +++ b/src/agent_toolkit/forestadmin/agent_toolkit/services/serializers/json_api_serializer.py @@ -1,6 +1,7 @@ from ast import literal_eval from datetime import date, datetime, time from typing import Any, Callable, Dict, List, Optional, Tuple, Union, cast +from urllib.parse import quote from uuid import uuid4 from forestadmin.agent_toolkit.forest_logger import ForestLogger @@ -84,6 +85,8 @@ def _serialize_one( ) -> DumpedResult: projection = projection if projection is not None else self.projection pk_value = self._get_id(collection, data) + if isinstance(pk_value, str): + pk_value = quote(pk_value, safe="") ret = { "data": { "id": pk_value, @@ -190,10 +193,11 @@ def _serialize_to_one_relationships( ) -> Tuple[Dict[str, Any], IncludedData]: """return (relationships, included)""" foreign_collection = self.datasource.get_collection(schema["foreign_collection"]) + packed_id = quote(pack_id(foreign_collection.schema, data), safe="") relation = { "data": { - "id": pack_id(foreign_collection.schema, data), + "id": packed_id, # "id": self._get_id(foreign_collection, data), "type": schema["foreign_collection"], }, @@ -207,10 +211,14 @@ def _serialize_to_one_relationships( continue included_attributes[key] = self._serialize_value(value, foreign_collection.schema["fields"][key]) + id_ = self._get_id(foreign_collection, data) + if isinstance(id_, str): + id_ = quote(id_, safe="") + included = { - "id": self._get_id(foreign_collection, data), + "id": id_, "links": { - "self": f"/forest/{foreign_collection.name}/{self._get_id(foreign_collection, data)}", + "self": f"/forest/{foreign_collection.name}/{id_}", }, "type": foreign_collection.name, } @@ -232,9 +240,10 @@ def _serialize_polymorphic_many_to_one_relationship( except DatasourceException: return {"data": None, "links": {"related": {"href": f"{current_link}/relationships/{name}"}}}, None + packed_id = quote(pack_id(foreign_collection.schema, data), safe="") relation = { "data": { - "id": pack_id(foreign_collection.schema, sub_data), # TODO: validate + "id": packed_id, # TODO: validate # "id": self._get_id(foreign_collection, sub_data), "type": data[schema["foreign_key_type_field"]], }, From 357bd8d8f539a3aa937c26d2762c00df1bb1325c Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Tue, 15 Apr 2025 11:42:13 +0200 Subject: [PATCH 02/11] chore: fix mistake --- launch_tests_ci_like.sh | 6 ++++-- .../services/serializers/json_api_serializer.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/launch_tests_ci_like.sh b/launch_tests_ci_like.sh index 478ef96ff..a97f7a807 100755 --- a/launch_tests_ci_like.sh +++ b/launch_tests_ci_like.sh @@ -3,9 +3,10 @@ ARTIFACT_DIR="artifacts_coverages" PACKAGES=("agent_toolkit" "datasource_sqlalchemy" "datasource_toolkit" "flask_agent" "datasource_django" "django_agent") -# PACKAGES=("datasource_sqlalchemy") +PACKAGES=("datasource_rpc") PYTHON_VERSIONS=("3.8" "3.9" "3.10" "3.11" "3.12" "3.13") # PYTHON_VERSIONS=("3.8" "3.13") +PYTHON_VERSIONS=("3.10") # flask related settings # https://pypi.org/project/Flask/#history @@ -33,7 +34,8 @@ for sub_version in {0..21}; do SQLALCHEMY_VERSIONS+=($version) fi done -DJANGO_VERSIONS=("3.2" "4.0" "4.1" "4.2" "5.0", "5.1") +DJANGO_VERSIONS=("3.2" "4.0" "4.1" "4.2" "5.0" "5.1") +DJANGO_VERSIONS=("3.2" "4.0" "4.1") # launch test on all versions only if we test 1 package if [[ ${#PACKAGES[@]} == 1 ]]; then diff --git a/src/agent_toolkit/forestadmin/agent_toolkit/services/serializers/json_api_serializer.py b/src/agent_toolkit/forestadmin/agent_toolkit/services/serializers/json_api_serializer.py index 6fcc9bb63..bce8d895f 100644 --- a/src/agent_toolkit/forestadmin/agent_toolkit/services/serializers/json_api_serializer.py +++ b/src/agent_toolkit/forestadmin/agent_toolkit/services/serializers/json_api_serializer.py @@ -240,7 +240,7 @@ def _serialize_polymorphic_many_to_one_relationship( except DatasourceException: return {"data": None, "links": {"related": {"href": f"{current_link}/relationships/{name}"}}}, None - packed_id = quote(pack_id(foreign_collection.schema, data), safe="") + packed_id = quote(pack_id(foreign_collection.schema, sub_data), safe="") relation = { "data": { "id": packed_id, # TODO: validate From 691b244b4b9215a18ad5b092ae849394402a01e3 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Tue, 15 Apr 2025 12:00:48 +0200 Subject: [PATCH 03/11] chore: this is not supposed to be a changed --- launch_tests_ci_like.sh | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/launch_tests_ci_like.sh b/launch_tests_ci_like.sh index a97f7a807..17969c333 100755 --- a/launch_tests_ci_like.sh +++ b/launch_tests_ci_like.sh @@ -3,7 +3,7 @@ ARTIFACT_DIR="artifacts_coverages" PACKAGES=("agent_toolkit" "datasource_sqlalchemy" "datasource_toolkit" "flask_agent" "datasource_django" "django_agent") -PACKAGES=("datasource_rpc") +# PACKAGES=("datasource_sqlalchemy") PYTHON_VERSIONS=("3.8" "3.9" "3.10" "3.11" "3.12" "3.13") # PYTHON_VERSIONS=("3.8" "3.13") PYTHON_VERSIONS=("3.10") @@ -34,9 +34,7 @@ for sub_version in {0..21}; do SQLALCHEMY_VERSIONS+=($version) fi done -DJANGO_VERSIONS=("3.2" "4.0" "4.1" "4.2" "5.0" "5.1") -DJANGO_VERSIONS=("3.2" "4.0" "4.1") - +DJANGO_VERSIONS=("3.2" "4.0" "4.1" "4.2" "5.0", "5.1") # launch test on all versions only if we test 1 package if [[ ${#PACKAGES[@]} == 1 ]]; then LAUNCH_ALL_FLASK_VERSIONS=true From 61207030027fb11e47c747a3fc0536b23aefa43a Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Tue, 15 Apr 2025 12:01:38 +0200 Subject: [PATCH 04/11] chore: resolve bug --- .../forestadmin/agent_toolkit/resources/collections/requests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py b/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py index 8e5874849..5adeac13d 100644 --- a/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py +++ b/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py @@ -99,7 +99,7 @@ def pks(self): pk_schema = self.collection.schema["fields"][pk_name] if pk_schema["column_type"] == PrimitiveType.STRING: unpacked_pks[i] = unquote(unpacked_pks[i]) - record_like_pks[pk_name] = unpacked_pks[i] + record_like_pks[pk_name] = unpacked_pks[i] pks = pack_id(self.collection.schema, record_like_pks) except KeyError: From adce57ebed9fd04b4d066f4832b08b2c8bb8ca52 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Tue, 15 Apr 2025 13:59:48 +0200 Subject: [PATCH 05/11] chore: this file shouldn't be changed --- launch_tests_ci_like.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launch_tests_ci_like.sh b/launch_tests_ci_like.sh index 17969c333..478ef96ff 100755 --- a/launch_tests_ci_like.sh +++ b/launch_tests_ci_like.sh @@ -6,7 +6,6 @@ PACKAGES=("agent_toolkit" "datasource_sqlalchemy" "datasource_toolkit" "flask_ag # PACKAGES=("datasource_sqlalchemy") PYTHON_VERSIONS=("3.8" "3.9" "3.10" "3.11" "3.12" "3.13") # PYTHON_VERSIONS=("3.8" "3.13") -PYTHON_VERSIONS=("3.10") # flask related settings # https://pypi.org/project/Flask/#history @@ -35,6 +34,7 @@ for sub_version in {0..21}; do fi done DJANGO_VERSIONS=("3.2" "4.0" "4.1" "4.2" "5.0", "5.1") + # launch test on all versions only if we test 1 package if [[ ${#PACKAGES[@]} == 1 ]]; then LAUNCH_ALL_FLASK_VERSIONS=true From 6fa30fe4c5b1cc494f14365f50a0613424670cbf Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Tue, 15 Apr 2025 17:23:42 +0200 Subject: [PATCH 06/11] chore: add tests --- .../tests/resources/collections/test_crud.py | 97 +++++++++++++++++ .../services/serializers/test_jsonapi.py | 100 ++++++++++++++++++ 2 files changed, 197 insertions(+) diff --git a/src/agent_toolkit/tests/resources/collections/test_crud.py b/src/agent_toolkit/tests/resources/collections/test_crud.py index 3b314106c..1534d2667 100644 --- a/src/agent_toolkit/tests/resources/collections/test_crud.py +++ b/src/agent_toolkit/tests/resources/collections/test_crud.py @@ -270,6 +270,29 @@ def _create_collections(cls): }, ) + # str as pk for url encoding + + cls.collection_str_pk = cls._create_collection( + "StrPK", + { + "pk": { + "column_type": PrimitiveType.STRING, + "is_primary_key": True, + "type": FieldType.COLUMN, + "is_read_only": False, + "default_value": None, + "enum_values": None, + "filter_operators": set([Operator.EQUAL, Operator.IN]), + "is_sortable": True, + "validations": [], + }, + "name": { + "column_type": "String", + "type": "Column", + }, + }, + ) + @classmethod def setUpClass(cls) -> None: cls.loop = asyncio.new_event_loop() @@ -293,6 +316,7 @@ def setUpClass(cls) -> None: "tag": cls.collection_tag, # for uuid "author": cls.collection_author, + "StrPK": cls.collection_str_pk, } cls.datasource_composite.add_datasource(cls.datasource) @@ -669,6 +693,58 @@ def test_get_with_polymorphic_relation_should_add_projection_star(self): }, ) + def test_get_should_work_when_primary_key_is_url_encoded(self): + + request = RequestCollection( + RequestMethod.GET, + self.collection_str_pk, + query={"collection_name": "StrPK", "pks": "hello%2Fworld"}, + headers={}, + client_ip="127.0.0.1", + ) + crud_resource = CrudResource( + self.datasource_composite, + self.datasource, + self.permission_service, + self.ip_white_list_service, + self.options, + ) + with patch.object( + self.collection_str_pk, + "list", + new_callable=AsyncMock, + return_value=[{"pk": "hello/world", "name": "hello world"}], + ) as mock_list: + response = self.loop.run_until_complete(crud_resource.get(request)) + mock_list.assert_awaited_with( + request.user, + PaginatedFilter( + { + "condition_tree": ConditionTreeBranch( + "and", + [ + ConditionTreeLeaf("pk", "equal", "hello/world"), + ConditionTreeLeaf("id", "greater_than", 0), + ], + ) + } + ), + ANY, + ) + body_content = json.loads(response.body) + self.assertEqual( + body_content, + { + "data": { + "id": "hello%2Fworld", + "attributes": {"pk": "hello/world"}, + "links": {"self": "/forest/StrPK/hello%2Fworld"}, + "type": "StrPK", + }, + "links": {"self": "/forest/StrPK/hello%2Fworld"}, + }, + ) + # add def test_simple_add(self): mock_order = {"cost": 200} @@ -1349,6 +1425,27 @@ def test_list_errors_jsonapi_error(self): "status": 500, } + request = RequestCollection( + RequestMethod.GET, + self.collection_order, + query={ + "collection_name": "order", + "timezone": "Europe/Paris", + "fields[order]": "id,cost", + }, + headers={}, + client_ip="127.0.0.1", + ) + crud_resource = CrudResource( + self.datasource_composite, + self.datasource, + self.permission_service, + self.ip_white_list_service, + self.options, + ) + with patch.object(self.collection_order, "list", new_callable=AsyncMock, return_value=mock_orders): + response = self.loop.run_until_complete(crud_resource.list(request)) + # count def test_count(self): request = RequestCollection( diff --git a/src/agent_toolkit/tests/services/serializers/test_jsonapi.py b/src/agent_toolkit/tests/services/serializers/test_jsonapi.py index 3343ea7ed..4debdf1d6 100644 --- a/src/agent_toolkit/tests/services/serializers/test_jsonapi.py +++ b/src/agent_toolkit/tests/services/serializers/test_jsonapi.py @@ -456,6 +456,49 @@ def setUpClass(cls) -> None: } ) + cls.collection_str_pk = Collection("StrPK", cls.datasource) # type:ignore + cls.collection_str_pk.add_fields( + { + "pk": Column( + column_type=PrimitiveType.STRING, + is_primary_key=True, + type=FieldType.COLUMN, + is_read_only=False, + default_value=None, + enum_values=None, + filter_operators=set([Operator.EQUAL, Operator.IN]), + is_sortable=True, + validations=[], + ), + "name": Column(column_type="String", type="Column"), + "relation_pk": Column(column_type="String", type="Column"), + "relation": ManyToOne( + foreign_collection="StrPKRelation", + foreign_key="relation_pk", + foreign_key_target="pk", + type=FieldType.MANY_TO_ONE, + ), + } + ) + + cls.collection_str_pk_relation = Collection("StrPKRelation", cls.datasource) # type:ignore + cls.collection_str_pk_relation.add_fields( + { + "pk": Column( + column_type=PrimitiveType.STRING, + is_primary_key=True, + type=FieldType.COLUMN, + is_read_only=False, + default_value=None, + enum_values=None, + filter_operators=set([Operator.EQUAL, Operator.IN]), + is_sortable=True, + validations=[], + ), + "name": Column(column_type="String", type="Column"), + } + ) + cls.datasource.add_collection(cls.collection_order) cls.datasource.add_collection(cls.collection_order_products) cls.datasource.add_collection(cls.collection_product) @@ -464,6 +507,8 @@ def setUpClass(cls) -> None: cls.datasource.add_collection(cls.collection_picture) cls.datasource.add_collection(cls.collection_comment) cls.datasource.add_collection(cls.collection_all_types) + cls.datasource.add_collection(cls.collection_str_pk) + cls.datasource.add_collection(cls.collection_str_pk_relation) class TestJsonApiDeserializer(TestJsonApi): @@ -1253,3 +1298,58 @@ def test_should_ignore_polymorphic_many_to_one_if_type_is_unknown(self): ], }, ) + + def test_string_primary_keys_should_be_url_encoded(self): + serializer = JsonApiSerializer(self.datasource, Projection("pk", "name")) + record = {"pk": "hello/world", "name": "hello world"} + dumped = serializer.serialize(record, self.collection_str_pk) + self.assertEqual( + dumped, + { + "data": { + "type": "StrPK", + "id": "hello%2Fworld", + "attributes": {"pk": "hello/world", "name": "hello world"}, + "links": {"self": "/forest/StrPK/hello%2Fworld"}, + }, + "links": {"self": "/forest/StrPK/hello%2Fworld"}, + }, + ) + + def test_string_foreign_keys_should_be_url_encoded_so_foreign_pk(self): + serializer = JsonApiSerializer( + self.datasource, Projection("pk", "name", "relation_pk", "relation:pk", "relation:name") + ) + record = { + "pk": "hello/world", + "name": "hello world", + "relation_pk": "hello/other/people", + "relation": {"pk": "hello/other/people", "name": "hello other people"}, + } + dumped = serializer.serialize(record, self.collection_str_pk) + self.assertEqual( + dumped, + { + "data": { + "type": "StrPK", + "id": "hello%2Fworld", + "attributes": {"pk": "hello/world", "name": "hello world", "relation_pk": "hello/other/people"}, + "links": {"self": "/forest/StrPK/hello%2Fworld"}, + "relationships": { + "relation": { + "data": {"id": "hello%2Fother%2Fpeople", "type": "StrPKRelation"}, + "links": {"related": {"href": "/forest/StrPK/hello%2Fworld/relationships/relation"}}, + } + }, + }, + "links": {"self": "/forest/StrPK/hello%2Fworld"}, + "included": [ + { + "id": "hello%2Fother%2Fpeople", + "links": {"self": "/forest/StrPKRelation/hello%2Fother%2Fpeople"}, + "type": "StrPKRelation", + "attributes": {"pk": "hello/other/people", "name": "hello other people"}, + } + ], + }, + ) From 952258368a1327fd4096cd9b5b463b136cdd89ab Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Wed, 16 Apr 2025 09:58:27 +0200 Subject: [PATCH 07/11] fix: remove encoded pk from id attribute --- .../serializers/json_api_serializer.py | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/agent_toolkit/forestadmin/agent_toolkit/services/serializers/json_api_serializer.py b/src/agent_toolkit/forestadmin/agent_toolkit/services/serializers/json_api_serializer.py index bce8d895f..08f04536a 100644 --- a/src/agent_toolkit/forestadmin/agent_toolkit/services/serializers/json_api_serializer.py +++ b/src/agent_toolkit/forestadmin/agent_toolkit/services/serializers/json_api_serializer.py @@ -85,18 +85,18 @@ def _serialize_one( ) -> DumpedResult: projection = projection if projection is not None else self.projection pk_value = self._get_id(collection, data) - if isinstance(pk_value, str): - pk_value = quote(pk_value, safe="") + + encoded_pk_value = quote(pk_value, safe="") if isinstance(pk_value, str) else pk_value ret = { "data": { "id": pk_value, "attributes": {}, - "links": {"self": f"/forest/{collection.name}/{pk_value}"}, + "links": {"self": f"/forest/{collection.name}/{encoded_pk_value}"}, "relationships": {}, "type": collection.name, }, "included": [], - "links": {"self": f"/forest/{collection.name}/{pk_value}"}, + "links": {"self": f"/forest/{collection.name}/{encoded_pk_value}"}, } first_level_projection = [*projection.relations.keys(), *projection.columns] @@ -112,7 +112,7 @@ def _serialize_one( key, data, cast(RelationAlias, collection.schema["fields"][key]), - f"/forest/{collection.name}/{pk_value}", + f"/forest/{collection.name}/{encoded_pk_value}", ) ret["data"]["relationships"][key] = relation if included is not None and not self._is_in_included(ret["included"], included): @@ -193,11 +193,10 @@ def _serialize_to_one_relationships( ) -> Tuple[Dict[str, Any], IncludedData]: """return (relationships, included)""" foreign_collection = self.datasource.get_collection(schema["foreign_collection"]) - packed_id = quote(pack_id(foreign_collection.schema, data), safe="") relation = { "data": { - "id": packed_id, + "id": pack_id(foreign_collection.schema, data), # "id": self._get_id(foreign_collection, data), "type": schema["foreign_collection"], }, @@ -212,13 +211,12 @@ def _serialize_to_one_relationships( included_attributes[key] = self._serialize_value(value, foreign_collection.schema["fields"][key]) id_ = self._get_id(foreign_collection, data) - if isinstance(id_, str): - id_ = quote(id_, safe="") + encoded_id = quote(id_, safe="") if isinstance(id_, str) else id_ included = { "id": id_, "links": { - "self": f"/forest/{foreign_collection.name}/{id_}", + "self": f"/forest/{foreign_collection.name}/{encoded_id}", }, "type": foreign_collection.name, } @@ -240,10 +238,9 @@ def _serialize_polymorphic_many_to_one_relationship( except DatasourceException: return {"data": None, "links": {"related": {"href": f"{current_link}/relationships/{name}"}}}, None - packed_id = quote(pack_id(foreign_collection.schema, sub_data), safe="") relation = { "data": { - "id": packed_id, # TODO: validate + "id": pack_id(foreign_collection.schema, sub_data), # TODO: validate # "id": self._get_id(foreign_collection, sub_data), "type": data[schema["foreign_key_type_field"]], }, From ee2e7cd0ef97e7c71d37ab6b989a9e9adeb3e744 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Wed, 16 Apr 2025 10:07:22 +0200 Subject: [PATCH 08/11] chore: adapt tests --- .../tests/resources/collections/test_crud.py | 3 +-- .../tests/services/serializers/test_jsonapi.py | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/agent_toolkit/tests/resources/collections/test_crud.py b/src/agent_toolkit/tests/resources/collections/test_crud.py index 1534d2667..3922dfd36 100644 --- a/src/agent_toolkit/tests/resources/collections/test_crud.py +++ b/src/agent_toolkit/tests/resources/collections/test_crud.py @@ -694,7 +694,6 @@ def test_get_with_polymorphic_relation_should_add_projection_star(self): ) def test_get_should_work_when_primary_key_is_url_encoded(self): - request = RequestCollection( RequestMethod.GET, self.collection_str_pk, @@ -736,7 +735,7 @@ def test_get_should_work_when_primary_key_is_url_encoded(self): body_content, { "data": { - "id": "hello%2Fworld", + "id": "hello/world", "attributes": {"pk": "hello/world"}, "links": {"self": "/forest/StrPK/hello%2Fworld"}, "type": "StrPK", diff --git a/src/agent_toolkit/tests/services/serializers/test_jsonapi.py b/src/agent_toolkit/tests/services/serializers/test_jsonapi.py index 4debdf1d6..51a935455 100644 --- a/src/agent_toolkit/tests/services/serializers/test_jsonapi.py +++ b/src/agent_toolkit/tests/services/serializers/test_jsonapi.py @@ -1308,7 +1308,7 @@ def test_string_primary_keys_should_be_url_encoded(self): { "data": { "type": "StrPK", - "id": "hello%2Fworld", + "id": "hello/world", "attributes": {"pk": "hello/world", "name": "hello world"}, "links": {"self": "/forest/StrPK/hello%2Fworld"}, }, @@ -1332,12 +1332,12 @@ def test_string_foreign_keys_should_be_url_encoded_so_foreign_pk(self): { "data": { "type": "StrPK", - "id": "hello%2Fworld", + "id": "hello/world", "attributes": {"pk": "hello/world", "name": "hello world", "relation_pk": "hello/other/people"}, "links": {"self": "/forest/StrPK/hello%2Fworld"}, "relationships": { "relation": { - "data": {"id": "hello%2Fother%2Fpeople", "type": "StrPKRelation"}, + "data": {"id": "hello/other/people", "type": "StrPKRelation"}, "links": {"related": {"href": "/forest/StrPK/hello%2Fworld/relationships/relation"}}, } }, @@ -1345,7 +1345,7 @@ def test_string_foreign_keys_should_be_url_encoded_so_foreign_pk(self): "links": {"self": "/forest/StrPK/hello%2Fworld"}, "included": [ { - "id": "hello%2Fother%2Fpeople", + "id": "hello/other/people", "links": {"self": "/forest/StrPKRelation/hello%2Fother%2Fpeople"}, "type": "StrPKRelation", "attributes": {"pk": "hello/other/people", "name": "hello other people"}, From 9dbebd171eea8a911f18909566a1dc0f3075a9c8 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Thu, 17 Apr 2025 10:31:21 +0200 Subject: [PATCH 09/11] chore: finish to adapt unencoded id in jsonapi --- .../resources/collections/requests.py | 10 ---------- .../forestadmin/django_agent/urls.py | 16 ++++++++-------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py b/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py index 5adeac13d..e4e6fcb03 100644 --- a/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py +++ b/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py @@ -92,16 +92,6 @@ def pks(self): raise CollectionResourceException("") try: pks = self.query["pks"] - unpacked_pks = unpack_id(self.collection.schema, pks) - pks_names = SchemaUtils.get_primary_keys(self.collection.schema) - record_like_pks = {} - for i, pk_name in enumerate(pks_names): - pk_schema = self.collection.schema["fields"][pk_name] - if pk_schema["column_type"] == PrimitiveType.STRING: - unpacked_pks[i] = unquote(unpacked_pks[i]) - record_like_pks[pk_name] = unpacked_pks[i] - pks = pack_id(self.collection.schema, record_like_pks) - except KeyError: raise CollectionResourceException("primary keys are missing") return pks diff --git a/src/django_agent/forestadmin/django_agent/urls.py b/src/django_agent/forestadmin/django_agent/urls.py index da2765286..bab908fac 100644 --- a/src/django_agent/forestadmin/django_agent/urls.py +++ b/src/django_agent/forestadmin/django_agent/urls.py @@ -1,5 +1,5 @@ from django.conf import settings -from django.urls import path +from django.urls import path, re_path from .views import actions, authentication, capabilities, charts, crud, crud_related, index, native_query, stats @@ -52,24 +52,24 @@ # stats path(f"{prefix}forest/stats/", stats.stats, name="stats"), # crud related - path( - f"{prefix}forest///relationships//count", + re_path( + f"{prefix}forest/(?P[^/]+)/(?P.*)/relationships/(?P[^/]+)/count", crud_related.count, name="crud_related_count", ), - path( - f"{prefix}forest///relationships/.csv", + re_path( + f"{prefix}forest/(?P[^/]+)/(?P.*)/relationships/(?P[^/]+).csv", crud_related.csv, name="crud_related_csv", ), - path( - f"{prefix}forest///relationships/", + re_path( + f"{prefix}forest/(?P[^/]+)/(?P.*)/relationships/(?P[^/]+)", crud_related.list_, name="crud_related_list", ), # crud path(f"{prefix}forest/.csv", crud.csv, name="crud_csv"), path(f"{prefix}forest//count", crud.count, name="crud_count"), - path(f"{prefix}forest//", crud.detail, name="crud_detail"), + re_path(f"{prefix}forest/(?P[^/]+)/(?P.+)", crud.detail, name="crud_detail"), path(f"{prefix}forest/", crud.list_, name="crud_list"), ] From ce33ecb7b3bffc5a3401230e9a7777fab16d8ea0 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Thu, 17 Apr 2025 10:58:09 +0200 Subject: [PATCH 10/11] chore: fix linting --- .../agent_toolkit/resources/collections/requests.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py b/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py index e4e6fcb03..37c6c0809 100644 --- a/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py +++ b/src/agent_toolkit/forestadmin/agent_toolkit/resources/collections/requests.py @@ -1,10 +1,8 @@ from typing import Any, Dict, Optional, TypedDict, TypeVar, Union -from urllib.parse import unquote from forestadmin.agent_toolkit.exceptions import AgentToolkitException from forestadmin.agent_toolkit.resources.collections.exceptions import CollectionResourceException from forestadmin.agent_toolkit.utils.context import Request, RequestMethod, User -from forestadmin.agent_toolkit.utils.id import pack_id, unpack_id from forestadmin.datasource_toolkit.collections import Collection, CollectionException from forestadmin.datasource_toolkit.datasource_customizer.collection_customizer import CollectionCustomizer from forestadmin.datasource_toolkit.datasource_customizer.datasource_customizer import DatasourceCustomizer @@ -14,12 +12,10 @@ ManyToOne, OneToMany, OneToOne, - PrimitiveType, is_polymorphic_many_to_one, is_reverse_polymorphic_relation, is_straight_relation, ) -from forestadmin.datasource_toolkit.utils.schema import SchemaUtils from typing_extensions import Self BoundCollection = TypeVar("BoundCollection", bound=Collection) From c562b588156243b1e31a24e179a448be3b813e84 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Thu, 17 Apr 2025 11:03:06 +0200 Subject: [PATCH 11/11] chore: remove useless test --- .../tests/resources/collections/test_crud.py | 51 ------------------- 1 file changed, 51 deletions(-) diff --git a/src/agent_toolkit/tests/resources/collections/test_crud.py b/src/agent_toolkit/tests/resources/collections/test_crud.py index 3922dfd36..8412ea01a 100644 --- a/src/agent_toolkit/tests/resources/collections/test_crud.py +++ b/src/agent_toolkit/tests/resources/collections/test_crud.py @@ -693,57 +693,6 @@ def test_get_with_polymorphic_relation_should_add_projection_star(self): }, ) - def test_get_should_work_when_primary_key_is_url_encoded(self): - request = RequestCollection( - RequestMethod.GET, - self.collection_str_pk, - query={"collection_name": "StrPK", "pks": "hello%2Fworld"}, - headers={}, - client_ip="127.0.0.1", - ) - crud_resource = CrudResource( - self.datasource_composite, - self.datasource, - self.permission_service, - self.ip_white_list_service, - self.options, - ) - with patch.object( - self.collection_str_pk, - "list", - new_callable=AsyncMock, - return_value=[{"pk": "hello/world", "name": "hello world"}], - ) as mock_list: - response = self.loop.run_until_complete(crud_resource.get(request)) - mock_list.assert_awaited_with( - request.user, - PaginatedFilter( - { - "condition_tree": ConditionTreeBranch( - "and", - [ - ConditionTreeLeaf("pk", "equal", "hello/world"), - ConditionTreeLeaf("id", "greater_than", 0), - ], - ) - } - ), - ANY, - ) - body_content = json.loads(response.body) - self.assertEqual( - body_content, - { - "data": { - "id": "hello/world", - "attributes": {"pk": "hello/world"}, - "links": {"self": "/forest/StrPK/hello%2Fworld"}, - "type": "StrPK", - }, - "links": {"self": "/forest/StrPK/hello%2Fworld"}, - }, - ) - # add def test_simple_add(self): mock_order = {"cost": 200}