diff --git a/news/1282.bugfix b/news/1282.bugfix new file mode 100644 index 0000000000..a7aa00a619 --- /dev/null +++ b/news/1282.bugfix @@ -0,0 +1,2 @@ +Improve support and meaning for `default` and `missing_value` in serializers/deserializers +[sneridagh] diff --git a/src/plone/restapi/deserializer/dxcontent.py b/src/plone/restapi/deserializer/dxcontent.py index 88258ab044..30d53ced74 100644 --- a/src/plone/restapi/deserializer/dxcontent.py +++ b/src/plone/restapi/deserializer/dxcontent.py @@ -76,7 +76,7 @@ def __call__( return self.context - def get_schema_data(self, data, validate_all, create=False): + def get_schema_data(self, data, validate_all, create=False): # noqa: ignore=C901 schema_data = {} errors = [] @@ -159,6 +159,43 @@ def get_schema_data(self, data, validate_all, create=False): if name == "changeNote": continue dm = queryMultiAdapter((self.context, field), IDataManager) + + # Covering here default/missing_value edge cases + if ( + create + and name not in data + and ( + dm.field.defaultFactory is not None + or field.default is not None + ) + and not field.required + ): + # precondition: + # - On creation + # - name was not handled before + # - not required, otherwise there has to be a value + # - one of defaultFactory or default are defined, so at least one of them can be set + if dm.field.defaultFactory: + dm.set(dm.field.defaultFactory(self.context)) + self.mark_field_as_changed(schema, name) + elif field.default: + dm.set(field.default) + self.mark_field_as_changed(schema, name) + elif ( + name not in data + and field.missing_value is not None + and not field.required + ): + # precondition: + # - name was not handled before + # - not required, otherwise there has to be a value + # - missing_value is defined, so it can be set + dm_value = dm.get() + if dm_value is None: + # if there's no value at all currently in the object + # then it sets the missing value + dm.set(field.missing_value) + bound = field.bind(self.context) try: bound.validate(dm.get()) diff --git a/src/plone/restapi/tests/dxtypes.py b/src/plone/restapi/tests/dxtypes.py index 493801524b..61178f7ddf 100644 --- a/src/plone/restapi/tests/dxtypes.py +++ b/src/plone/restapi/tests/dxtypes.py @@ -279,6 +279,10 @@ class IDXTestDocumentSchema(model.Schema): required=False, missing_value="missing", default="default" ) + test_missing_value_field_and_no_default = schema.List( + required=False, value_type=schema.Choice(values=[1, 2, 3]), missing_value=[] + ) + test_missing_value_required_field = schema.TextLine( required=True, missing_value="missing", default="some value" ) diff --git a/src/plone/restapi/tests/http-examples/content_get.resp b/src/plone/restapi/tests/http-examples/content_get.resp index dbc59a7bb2..85de82b31a 100644 --- a/src/plone/restapi/tests/http-examples/content_get.resp +++ b/src/plone/restapi/tests/http-examples/content_get.resp @@ -38,7 +38,10 @@ Content-Type: application/json "expires": null, "id": "my-document", "is_folderish": false, - "language": "", + "language": { + "title": "English", + "token": "en" + }, "layout": "document_view", "lock": { "locked": false, diff --git a/src/plone/restapi/tests/http-examples/content_patch_representation.resp b/src/plone/restapi/tests/http-examples/content_patch_representation.resp index 2d7b234483..3095aa4280 100644 --- a/src/plone/restapi/tests/http-examples/content_patch_representation.resp +++ b/src/plone/restapi/tests/http-examples/content_patch_representation.resp @@ -38,7 +38,10 @@ Content-Type: application/json "expires": null, "id": "my-document", "is_folderish": false, - "language": "", + "language": { + "title": "English", + "token": "en" + }, "layout": "document_view", "lock": { "locked": false, diff --git a/src/plone/restapi/tests/http-examples/content_post.resp b/src/plone/restapi/tests/http-examples/content_post.resp index 474fea1302..9e752b1fa1 100644 --- a/src/plone/restapi/tests/http-examples/content_post.resp +++ b/src/plone/restapi/tests/http-examples/content_post.resp @@ -39,7 +39,10 @@ Location: http://localhost:55001/plone/folder/my-document "expires": null, "id": "my-document", "is_folderish": false, - "language": "", + "language": { + "title": "English", + "token": "en" + }, "layout": "document_view", "lock": { "locked": false, diff --git a/src/plone/restapi/tests/test_dxcontent_deserializer.py b/src/plone/restapi/tests/test_dxcontent_deserializer.py index e9cc6fdf14..f7f8f3688d 100644 --- a/src/plone/restapi/tests/test_dxcontent_deserializer.py +++ b/src/plone/restapi/tests/test_dxcontent_deserializer.py @@ -186,6 +186,44 @@ def test_deserializer_sets_missing_value_when_receiving_null(self): self.deserialize(body='{"test_missing_value_field": null}') self.assertEqual("missing", self.portal.doc1.test_missing_value_field) + def test_deserializer_sets_missing_value_when_receiving_nothing_at_all(self): + # If the field is not set in the request data, it has no value set either, nor default, + # and missing_value is defined, it sets the missing value + self.deserialize(body='{"test_required_field": "My Value"}', validate_all=True) + self.assertEqual([], self.portal.doc1.test_missing_value_field_and_no_default) + + def test_deserializer_has_default_and_missing_value_sets_default_when_receiving_nothing_at_all( + self, + ): + # If the field is not set in the request data, it has a default + # and a missing_value defined, it sets nothing, thus Dexterity returns default on validation + self.deserialize(body='{"test_required_field": "My Value"}', validate_all=True) + self.assertEqual("default", self.portal.doc1.test_missing_value_field) + + def test_deserializer_on_conflictive_field_language(self): + """In the wild, there are conflictive field declarations that has both a default and + a missing_value set, that in some cases the deserializer when values are missing in the + request. Handling default nullish values is hard.""" + self.portal.invokeFactory( + "Document", + id="doc_language", + ) + self.deserialize( + body='{"title": "The title"}', + context=self.portal["doc_language"], + validate_all=True, + create=True, + ) + self.assertEqual("en", self.portal.doc_language.language) + + self.deserialize( + body='{"title": "The title", "language": "en"}', + context=self.portal["doc_language"], + validate_all=True, + create=True, + ) + self.assertEqual("en", self.portal.doc_language.language) + def test_deserializer_sets_missing_value_on_required_field(self): """We don't set missing_value if the field is required""" self.deserialize(body='{"test_missing_value_required_field": "valid value"}') diff --git a/src/plone/restapi/tests/test_dxcontent_serializer.py b/src/plone/restapi/tests/test_dxcontent_serializer.py index b57b552dfa..d6948a593e 100644 --- a/src/plone/restapi/tests/test_dxcontent_serializer.py +++ b/src/plone/restapi/tests/test_dxcontent_serializer.py @@ -150,6 +150,16 @@ def test_serializer_includes_field_with_read_permission(self): self.assertIn("test_read_permission_field", obj) self.assertEqual("Secret Stuff", obj["test_read_permission_field"]) + def test_serializer_includes_default_value(self): + obj = self.serialize() + self.assertIn("test_missing_value_field", obj) + self.assertEqual("default", obj["test_missing_value_field"]) + + def test_serializer_returns_None_if_only_missing_value_is_present(self): + obj = self.serialize() + self.assertIn("test_missing_value_field_and_no_default", obj) + self.assertEqual(None, obj["test_missing_value_field_and_no_default"]) + def test_get_layout(self): current_layout = self.portal.doc1.getLayout() obj = self.serialize()