From bb49a5f643d486dc36ce06b98c450c1063272e6f Mon Sep 17 00:00:00 2001 From: Pedro Brochado Date: Mon, 28 Oct 2024 16:20:02 -0300 Subject: [PATCH] Add and expose JSONDictField and JSONListField on the plugin API DRF serializers.JSONField can be any json entity, but we want more precise types for better schema/bindings representation. New fields that are supposed to be dict or list structures should use the new JSONDictField or JSONListField field. Some context: --- CHANGES/plugin_api/+expose-json-dict.misc | 7 +++ pulpcore/app/serializers/__init__.py | 2 + pulpcore/app/serializers/fields.py | 45 +++++++++++++++-- pulpcore/plugin/serializers/__init__.py | 4 ++ .../tests/unit/serializers/test_fields.py | 49 +++++++++++++++++++ 5 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 CHANGES/plugin_api/+expose-json-dict.misc create mode 100644 pulpcore/tests/unit/serializers/test_fields.py diff --git a/CHANGES/plugin_api/+expose-json-dict.misc b/CHANGES/plugin_api/+expose-json-dict.misc new file mode 100644 index 0000000000..f605ce992f --- /dev/null +++ b/CHANGES/plugin_api/+expose-json-dict.misc @@ -0,0 +1,7 @@ +Exposed JSONDictField and JSONListField on the plugin API. + +DRF serializers.JSONField can be any json entity, but we want more precise types +for better schema/bindings representation. New fields that are supposed to be dict +or list structures should use the new JSONDictField or JSONListField field. + +Some context: diff --git a/pulpcore/app/serializers/__init__.py b/pulpcore/app/serializers/__init__.py index 75ec2ed84b..3a6047bcc8 100644 --- a/pulpcore/app/serializers/__init__.py +++ b/pulpcore/app/serializers/__init__.py @@ -30,6 +30,8 @@ ImportsIdentityFromImporterField, ImportRelatedField, ImportIdentityField, + JSONDictField, + JSONListField, LatestVersionField, SingleContentArtifactField, RepositoryVersionsIdentityFromRepositoryField, diff --git a/pulpcore/app/serializers/fields.py b/pulpcore/app/serializers/fields.py index 6cd9f92a73..6988c2646f 100644 --- a/pulpcore/app/serializers/fields.py +++ b/pulpcore/app/serializers/fields.py @@ -21,13 +21,48 @@ def relative_path_validator(relative_path): ) +# Prefer JSONDictField and JSONListField over JSONField: +# * Drf serializers.JSONField provides a OpenApi schema type of Any. +# * This can cause problems with bindings and is not helpful to the user. +# * https://github.com/tfranzel/drf-spectacular/issues/1095 + + @extend_schema_field(OpenApiTypes.OBJECT) class JSONDictField(serializers.JSONField): - """A drf JSONField override to force openapi schema to use 'object' type. - - Not strictly correct, but we relied on that for a long time. - See: https://github.com/tfranzel/drf-spectacular/issues/1095 - """ + """A JSONField accepting dicts, specifying as type 'object' in the openapi.""" + + def to_internal_value(self, data): + value = super().to_internal_value(data) + ERROR_MSG = f"Invalid type. Expected a JSON object (dict), got {value!r}." + # This condition is from the JSONField source: + # if it's True, it will return the python representation, + # else the raw data string + returns_python_repr = self.binary or getattr(data, "is_json_string", False) + if returns_python_repr: + if not isinstance(value, dict): + raise serializers.ValidationError(ERROR_MSG) + elif not value.strip().startswith("{"): + raise serializers.ValidationError(ERROR_MSG) + return value + + +@extend_schema_field(serializers.ListField) +class JSONListField(serializers.JSONField): + """A JSONField accepting lists, specifying as type 'array' in the openapi.""" + + def to_internal_value(self, data): + value = super().to_internal_value(data) + ERROR_MSG = f"Invalid type. Expected a JSON array (list), got {value!r}." + # This condition is from the JSONField source: + # if it's True, it will return the python representation, + # else the raw data string + returns_python_repr = self.binary or getattr(data, "is_json_string", False) + if returns_python_repr: + if not isinstance(value, list): + raise serializers.ValidationError(ERROR_MSG) + elif not value.strip().startswith("["): + raise serializers.ValidationError(ERROR_MSG) + return value class SingleContentArtifactField(RelatedField): diff --git a/pulpcore/plugin/serializers/__init__.py b/pulpcore/plugin/serializers/__init__.py index 1f245008c8..5ca3943808 100644 --- a/pulpcore/plugin/serializers/__init__.py +++ b/pulpcore/plugin/serializers/__init__.py @@ -17,6 +17,8 @@ IdentityField, ImporterSerializer, ImportSerializer, + JSONDictField, + JSONListField, ModelSerializer, MultipleArtifactContentSerializer, NestedRelatedField, @@ -62,6 +64,8 @@ "IdentityField", "ImporterSerializer", "ImportSerializer", + "JSONDictField", + "JSONListField", "ModelSerializer", "MultipleArtifactContentSerializer", "NestedRelatedField", diff --git a/pulpcore/tests/unit/serializers/test_fields.py b/pulpcore/tests/unit/serializers/test_fields.py new file mode 100644 index 0000000000..506d4c1361 --- /dev/null +++ b/pulpcore/tests/unit/serializers/test_fields.py @@ -0,0 +1,49 @@ +import pytest +from rest_framework import serializers + +from pulpcore.app.serializers import fields + + +@pytest.mark.parametrize( + "field_and_data", + [ + (fields.JSONDictField, '{"foo": 123, "bar": [1,2,3]}'), + (fields.JSONListField, '[{"foo": 123}, {"bar": 456}]'), + ], +) +@pytest.mark.parametrize("binary_arg", [True, False]) +def test_custom_json_dict_field(field_and_data, binary_arg): + """ + On the happy overlap case, + pulpcore JSONDictField and JSONListField should be compatible with drf JSONField. + """ + custom_field, data = field_and_data + drf_json_field = serializers.JSONField(binary=binary_arg) + custom_field = custom_field(binary=binary_arg) + custom_field_result = custom_field.to_internal_value(data) + drf_field_result = drf_json_field.to_internal_value(data) + assert custom_field_result == drf_field_result + + +@pytest.mark.parametrize( + "field_and_data", + [ + (fields.JSONDictField, '[{"foo": 123}, {"bar": 456}]'), + (fields.JSONDictField, "123"), + (fields.JSONDictField, "false"), + (fields.JSONListField, '{"foo": 123, "bar": [1,2,3]}'), + (fields.JSONListField, "123"), + (fields.JSONListField, "false"), + ], +) +@pytest.mark.parametrize("binary_arg", [True, False]) +def test_custom_json_dict_field_raises(field_and_data, binary_arg): + """ + On the invalid data case, + pulpcore JSONDictField and JSONListField should raise appropriately. + """ + custom_field, data = field_and_data + custom_field = custom_field(binary=binary_arg) + error_msg = "Invalid type" + with pytest.raises(serializers.ValidationError, match=error_msg): + custom_field.to_internal_value(data)