From e85eb80ce14cff9dc60a91b44411968584d8957a Mon Sep 17 00:00:00 2001 From: Jackie Greenbaum Date: Wed, 6 Nov 2024 09:54:15 -0500 Subject: [PATCH 1/2] Implement `lid` support --- atomic_operations/parsers.py | 32 ++++++++++++++++++++------- atomic_operations/views.py | 43 ++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/atomic_operations/parsers.py b/atomic_operations/parsers.py index ac9625e..0846d56 100644 --- a/atomic_operations/parsers.py +++ b/atomic_operations/parsers.py @@ -43,12 +43,24 @@ class AtomicOperationParser(JSONParser): renderer_class = renderers.JSONRenderer def check_resource_identifier_object(self, idx: int, resource_identifier_object: Dict, operation_code: str): - if operation_code in ["update", "remove"] and not resource_identifier_object.get("id"): - raise JsonApiParseError( - id="missing-id", - detail="The resource identifier object must contain an `id` member", - pointer=f"/{ATOMIC_OPERATIONS}/{idx}/{'data' if operation_code == 'update' else 'ref'}" - ) + if operation_code in ["update", "remove"]: + resource_id = resource_identifier_object.get("id") + resource_lid = resource_identifier_object.get("lid") + + if not (resource_id or resource_lid): + raise JsonApiParseError( + id="missing-id", + detail="The resource identifier object must contain an `id` member or a `lid` member", + pointer=f"/{ATOMIC_OPERATIONS}/{idx}/{'data' if operation_code == 'update' else 'ref'}" + ) + + if resource_id and resource_lid: + raise JsonApiParseError( + id="multiple-id-fields", + detail="Only one of `id`, `lid` may be specified", + pointer=f"/{ATOMIC_OPERATIONS}/{idx}/{'data' if operation_code == 'update' else 'ref'}" + ) + if not resource_identifier_object.get("type"): raise JsonApiParseError( id="missing-type", @@ -150,10 +162,14 @@ def check_operation(self, idx: int, operation: Dict): pointer=f"/{ATOMIC_OPERATIONS}/{idx}/op" ) - def parse_id_and_type(self, resource_identifier_object): + def parse_id_lid_and_type(self, resource_identifier_object): parsed_data = {"id": resource_identifier_object.get( "id")} if "id" in resource_identifier_object else {} parsed_data["type"] = resource_identifier_object.get("type") + + if lid := resource_identifier_object.get("lid", None): + parsed_data["lid"] = lid + return parsed_data def check_root(self, result): @@ -173,7 +189,7 @@ def check_root(self, result): ) def parse_operation(self, resource_identifier_object, result): - _parsed_data = self.parse_id_and_type(resource_identifier_object) + _parsed_data = self.parse_id_lid_and_type(resource_identifier_object) _parsed_data.update(self.parse_attributes(resource_identifier_object)) _parsed_data.update(self.parse_relationships(resource_identifier_object)) _parsed_data.update(self.parse_metadata(result)) diff --git a/atomic_operations/views.py b/atomic_operations/views.py index 9eec1b1..d3e76ad 100644 --- a/atomic_operations/views.py +++ b/atomic_operations/views.py @@ -1,4 +1,5 @@ from typing import Dict, List +from collections import defaultdict from django.core.exceptions import ImproperlyConfigured, ObjectDoesNotExist from django.db.transaction import atomic @@ -27,6 +28,8 @@ class AtomicOperationView(APIView): sequential = True response_data: List[Dict] = [] + lid_to_id = defaultdict(dict) + # TODO: proof how to check permissions for all operations # permission_classes = TODO # call def check_permissions for `add` operation @@ -94,8 +97,15 @@ def post(self, request, *args, **kwargs): def handle_sequential(self, serializer, operation_code): if operation_code in ["add", "update", "update-relationship"]: + lid = serializer.initial_data.get("lid", None) + serializer.is_valid(raise_exception=True) serializer.save() + + if operation_code == "add" and lid: + resource_type = serializer.initial_data["type"] + self.lid_to_id[resource_type][lid] = serializer.data["id"] + if operation_code != "update-relationship": self.response_data.append(serializer.data) else: @@ -139,6 +149,36 @@ def handle_bulk(self, serializer, current_operation_code, bulk_operation_data): bulk_operation_data["serializer_collection"][0], current_operation_code) bulk_operation_data["serializer_collection"] = [] + def substitute_lids(self, data, idx, should_raise_unknown_lid_error): + if not isinstance(data, dict): + return + + try: + lid = data.get("lid", None) + if lid: + resource_type = data["type"] + data["id"] = self.lid_to_id[resource_type][lid] + except KeyError: + if should_raise_unknown_lid_error: + raise UnprocessableEntity([ + { + "id": "unknown-lid", + "detail": f'Object with lid `{lid}` received for operation with index `{idx}` does not exist', + "source": { + "pointer": f"/{ATOMIC_OPERATIONS}/{idx}/data/lid" + }, + "status": "422" + } + ]) + + for _, value in data.items(): + if isinstance(value, dict): + self.substitute_lids(value, idx, should_raise_unknown_lid_error=True) + elif isinstance(value, list): + [self.substitute_lids(value, idx, should_raise_unknown_lid_error=True) for value in value] + + return data + def perform_operations(self, parsed_operations: List[Dict]): self.response_data = [] # reset local response data storage @@ -154,6 +194,9 @@ def perform_operations(self, parsed_operations: List[Dict]): operation_code = next(iter(operation)) obj = operation[operation_code] + should_raise_unknown_lid_error = operation_code != "add" + self.substitute_lids(obj, idx, should_raise_unknown_lid_error) + serializer = self.get_serializer( idx=idx, data=obj, From 13bdc72028ca053c59439d6ffc9a47aa02971057 Mon Sep 17 00:00:00 2001 From: Jackie Greenbaum Date: Wed, 6 Nov 2024 17:31:22 -0500 Subject: [PATCH 2/2] Update tests for lid support --- tests/test_parsers.py | 100 +++++++++- tests/test_views.py | 453 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 548 insertions(+), 5 deletions(-) diff --git a/tests/test_parsers.py b/tests/test_parsers.py index 7be9247..7978405 100644 --- a/tests/test_parsers.py +++ b/tests/test_parsers.py @@ -228,7 +228,7 @@ def test_using_href(self): } ) - def test_primary_data_without_id(self): + def test_primary_data_without_id_or_lid(self): data = { ATOMIC_OPERATIONS: [ { @@ -242,7 +242,7 @@ def test_primary_data_without_id(self): stream = BytesIO(json.dumps(data).encode("utf-8")) self.assertRaisesRegex( JsonApiParseError, - "The resource identifier object must contain an `id` member", + "The resource identifier object must contain an `id` member or a `lid` member", self.parser.parse, **{ "stream": stream, @@ -263,7 +263,7 @@ def test_primary_data_without_id(self): stream = BytesIO(json.dumps(data).encode("utf-8")) self.assertRaisesRegex( JsonApiParseError, - "The resource identifier object must contain an `id` member", + "The resource identifier object must contain an `id` member or a `lid` member", self.parser.parse, **{ "stream": stream, @@ -284,7 +284,7 @@ def test_primary_data_without_id(self): stream = BytesIO(json.dumps(data).encode("utf-8")) self.assertRaisesRegex( JsonApiParseError, - "The resource identifier object must contain an `id` member", + "The resource identifier object must contain an `id` member or a `lid` member", self.parser.parse, **{ "stream": stream, @@ -365,3 +365,95 @@ def test_is_atomic_operations(self): "parser_context": self.parser_context } ) + + def test_parse_with_lid(self): + data = { + ATOMIC_OPERATIONS: [ + { + "op": "add", + "data": { + "lid": "1", + "type": "articles", + "attributes": { + "title": "JSON API paints my bikeshed!" + } + } + }, + { + "op": "update", + "data": { + "lid": "1", + "type": "articles", + "attributes": { + "title": "JSON API supports lids!" + } + } + }, + { + "op": "remove", + "ref": { + "lid": "1", + "type": "articles", + } + } + ] + } + stream = BytesIO(json.dumps(data).encode("utf-8")) + + result = self.parser.parse(stream, parser_context=self.parser_context) + expected_result = [ + { + "add": { + "type": "articles", + "lid": "1", + "title": "JSON API paints my bikeshed!" + } + }, + { + "update": { + "lid": "1", + "type": "articles", + "title": "JSON API supports lids!" + } + }, + { + "remove": { + "lid": "1", + "type": "articles" + } + } + ] + self.assertEqual(expected_result, result) + + def test_primary_data_with_id_and_lid(self): + data = { + ATOMIC_OPERATIONS: [ + { + "op": "add", + "data": { + "lid": "1", + "type": "articles", + "title": "JSON API paints my bikeshed!" + } + }, + { + "op": "update", + "data": { + "lid": "1", + "id": "1", + "type": "articles", + "title": "JSON API supports lids!" + } + } + ] + } + stream = BytesIO(json.dumps(data).encode("utf-8")) + self.assertRaisesRegex( + JsonApiParseError, + "Only one of `id`, `lid` may be specified", + self.parser.parse, + **{ + "stream": stream, + "parser_context": self.parser_context + } + ) \ No newline at end of file diff --git a/tests/test_views.py b/tests/test_views.py index b78efd4..e9ece15 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -421,7 +421,7 @@ def test_parser_exception_with_pointer(self): "errors": [ { "id": "missing-id", - "detail": "The resource identifier object must contain an `id` member", + "detail": "The resource identifier object must contain an `id` member or a `lid` member", "status": "400", "source": { "pointer": f"/{ATOMIC_OPERATIONS}/2/ref" @@ -498,3 +498,454 @@ def test_view_204_response(self): self.assertEqual(204, response.status_code) self.assertFalse(response.content) + + def test_view_processing_with_lid_substitution(self): + operations = [ + { + "op": "add", + "data": { + "lid": "valid-lid-1", + "type": "BasicModel", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + } + }, { + "op": "add", + "data": { + "lid": "valid-lid-2", + "type": "BasicModel", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + } + }, { + "op": "add", + "data": { + "lid": "valid-lid-1", + "type": "RelatedModel", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + } + }, + { + "op": "add", + "data": { + "lid": "valid-lid-1", + "type": "RelatedModelTwo", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + } + }, + { + "op": "add", + "data": { + "lid": "valid-lid-2", + "type": "RelatedModelTwo", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + } + },{ + "op": "update", + "data": { + "lid": "valid-lid-1", + "type": "BasicModel", + "attributes": { + "text": "JSON API paints my bikeshed2!" + } + } + }, { + "op": "remove", + "ref": { + "lid": "valid-lid-1", + "type": "BasicModel", + } + }, + { + "op": "update", + "ref": { + "lid": "valid-lid-2", + "type": "BasicModel", + "relationship": "to_one" + }, + "data": {"type": "RelatedModel", "lid": "valid-lid-1"} + }, { + "op": "update", + "ref": { + "lid": "valid-lid-2", + "type": "BasicModel", + "relationship": "to_many" + }, + "data": [{"type": "RelatedModelTwo", "lid": "valid-lid-1"}, {"type": "RelatedModelTwo", "lid": "valid-lid-2"}] + } + ] + + data = { + ATOMIC_OPERATIONS: operations + } + + response = self.client.post( + path="/", + data=data, + content_type=ATOMIC_CONTENT_TYPE, + + **{"HTTP_ACCEPT": ATOMIC_CONTENT_TYPE} + ) + + # check response + self.assertEqual(200, response.status_code) + + expected_result = { + ATOMIC_RESULTS: [ + { + "data": { + "id": "1", + + "type": "BasicModel", + "attributes": { + "text": "JSON API paints my bikeshed!" + }, + "relationships": { + "to_many": {'data': [], 'meta': {'count': 0}}, + "to_one": {'data': None}, + } + } + }, + { + "data": { + "id": "2", + "type": "BasicModel", + "attributes": { + "text": "JSON API paints my bikeshed!" + }, + "relationships": { + "to_many": {'data': [], 'meta': {'count': 0}}, + "to_one": {'data': None}, + } + } + }, + { + "data": { + "id": "1", + "type": "RelatedModel", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + } + }, + { + "data": { + "id": "1", + "type": "RelatedModelTwo", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + } + }, + { + "data": { + "id": "2", + "type": "RelatedModelTwo", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + } + }, + { + "data": { + "id": "1", + "type": "BasicModel", + "attributes": { + "text": "JSON API paints my bikeshed2!" + }, + "relationships": { + "to_many": { + 'data': [], 'meta': {'count': 0}}, + "to_one": { + "data": None + } + }, + } + } + ] + } + + self.assertDictEqual(expected_result, + json.loads(response.content)) + + + # check db content + self.assertEqual(1, BasicModel.objects.count()) + self.assertEqual(1, RelatedModel.objects.count()) + self.assertEqual(2, RelatedModelTwo.objects.count()) + + self.assertEqual(RelatedModel.objects.get(pk=1), + BasicModel.objects.get(pk=2).to_one) + + major, minor, _, _, _ = VERSION + if int(major) <= 4 and int(minor) <= 1: + self.assertQuerysetEqual(RelatedModelTwo.objects.filter(pk__in=[1, 2]), + BasicModel.objects.get(pk=2).to_many.all()) + else: + # with django 4.2 TransactionTestCase.assertQuerysetEqual() is deprecated in favor of assertQuerySetEqual(). + self.assertQuerySetEqual(RelatedModelTwo.objects.filter(pk__in=[1, 2]), + BasicModel.objects.get(pk=2).to_many.all()) + + + def test_view_processing_with_invalid_lid(self): + operations = [ + { + "op": "add", + "data": { + "lid": "invalid-lid-test-1", + "type": "BasicModel", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + } + }, + { + "op": "update", + "data": { + "type": "BasicModel", + "lid": "invalid-lid-test-2", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + } + } + ] + + data = { + ATOMIC_OPERATIONS: operations + } + + response = self.client.post( + path="/", + data=data, + content_type=ATOMIC_CONTENT_TYPE, + + **{"HTTP_ACCEPT": ATOMIC_CONTENT_TYPE} + ) + + # check response + error = json.loads(response.content) + expected_error = { + "errors": [ + { + "id": "unknown-lid", + "detail": f'Object with lid `invalid-lid-test-2` received for operation with index `1` does not exist', + "source": { + "pointer": f"/{ATOMIC_OPERATIONS}/1/data/lid" + }, + "status": "422" + } + ] + } + self.assertEqual(422, response.status_code) + self.assertDictEqual(expected_error, error) + + def test_view_processing_with_lid_type_mismatch(self): + operations = [ + { + "op": "add", + "data": { + "lid": "lid-type-mismatch-1", + "type": "BasicModel", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + } + }, + { + "op": "add", + "data": { + "type": "RelatedModel", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + } + }, + { + "op": "update", + "data": { + "type": "RelatedModel", + "lid": "lid-type-mismatch-1", + "attributes": { + "text": "JSON API paints my bikeshed3!" + } + } + } + ] + + data = { + ATOMIC_OPERATIONS: operations + } + + response = self.client.post( + path="/", + data=data, + content_type=ATOMIC_CONTENT_TYPE, + + **{"HTTP_ACCEPT": ATOMIC_CONTENT_TYPE} + ) + + # check response + error = json.loads(response.content) + expected_error = { + "errors": [ + { + "id": "unknown-lid", + "detail": f'Object with lid `lid-type-mismatch-1` received for operation with index `2` does not exist', + "source": { + "pointer": f"/{ATOMIC_OPERATIONS}/2/data/lid" + }, + "status": "422" + } + ] + } + self.assertEqual(422, response.status_code) + self.assertDictEqual(expected_error, error) + + def test_adding_resource_with_lid_relationship(self): + operations = [ + { + "op": "add", + "data": { + "type": "RelatedModel", + "lid": "relationship-lid-test-1", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + }, + }, + { + "op": "add", + "data": { + "type": "RelatedModelTwo", + "lid": "relationship-lid-test-1", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + }, + }, + { + "op": "add", + "data": { + "type": "RelatedModelTwo", + "lid": "relationship-lid-test-2", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + }, + },{ + "op": "add", + "data": { + "type": "BasicModel", + "attributes": { + "text": "JSON API paints my bikeshed!" + }, + "relationships": { + "to_one": { + "data": { + "lid": "relationship-lid-test-1", + "type": "RelatedModel" + } + }, + "to_many": { + "data": [ + { + "lid": "relationship-lid-test-1", + "type": "RelatedModelTwo" + }, + { + "lid": "relationship-lid-test-2", + "type": "RelatedModelTwo" + } + ] + } + } + } + }, + ] + + data = { + ATOMIC_OPERATIONS: operations + } + + response = self.client.post( + path="/", + data=data, + content_type=ATOMIC_CONTENT_TYPE, + + **{"HTTP_ACCEPT": ATOMIC_CONTENT_TYPE} + ) + + # check response + self.assertEqual(200, response.status_code) + + expected_result = { + ATOMIC_RESULTS: [ + { + "data": { + "id": "1", + "type": "RelatedModel", + "attributes": { + "text": "JSON API paints my bikeshed!" + }, + } + }, + { + "data": { + "id": "1", + "type": "RelatedModelTwo", + "attributes": { + "text": "JSON API paints my bikeshed!" + }, + } + }, + { + "data": { + "id": "2", + "type": "RelatedModelTwo", + "attributes": { + "text": "JSON API paints my bikeshed!" + } + } + }, + { + "data": { + "id": "1", + "type": "BasicModel", + "attributes": { + "text": "JSON API paints my bikeshed!" + }, + "relationships": { + "to_many": { + 'data': [ + { + "id": "1", + "type": "RelatedModelTwo" + }, + { + "id": "2", + "type": "RelatedModelTwo" + } + ], 'meta': {'count': 2}}, + "to_one": { + "data": { + "id": "1", + "type": "RelatedModel" + } + } + }, + } + }, + ] + } + + self.assertDictEqual(expected_result, + json.loads(response.content)) \ No newline at end of file