Skip to content

Add test case for using JsonUnwrapped on fields of the same class #5182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

kariem
Copy link

@kariem kariem commented Jun 6, 2025

See FasterXML/jackson#278 for context

I would like to include a bean into a class multiple times. For serialization, this bean should prefix its fields in a specific way so that I can reuse this bean in other classes too.

This is the JSON representation I want to use

{
  "name": "name",  // name.value
  "name_l10n": {   // name.value_l10n
    "de": "name",
    "fr": "nom"
  },
  "description": "description",  // description.value
  "description_l10n": {          // description.value_l10n
    "de": "Beschreibung",
    "fr": "description"
  }
}

The pairs name and name_l10n should be from the same bean instance. The java representation should be simple and reusable

public class MyFancyClass {
	@JsonUnwrapped
	private LocalizedField name;        // → name.value and name.value_l10n
	@JsonUnwrapped
	private LocalizedField description; // → description.value and description.value_l10n

    // ... getters and setters
}

I have provided some test cases to demonstrate the problems I have encountered so far.

  1. Without custom serialization configuration, the mapper creates invalid JSON. See testUnwrappingWithDifferentProperties
  2. Serialization works find, see testUnwrappingWithCustomSerializer
  3. Deserialization does not work, see testUnwrappingWithCustomDeserializer

@JooHyukKim
Copy link
Member

Some thoughts...

  1. @JsonUnwrapped can be configured with prefix/suffix if that helps.
  2. Since you made custom serializer work as you wanted, how about spend some more time making custom deserializer to work?

Without custom serialization configuration, the mapper creates invalid JSON. See testUnwrappingWithDifferentProperties

Because that's how it's configured? Current result is what I would expect from plain configuration of @JsonUnwrapped.

.addSerializer(ValueAndMap.class, new ValueAndMapSerializer())
.addDeserializer(ValueAndMap.class, new ValueAndMapDeserializer()));

@JsonIgnoreProperties(ignoreUnknown = true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad idea for tests -- should almost never be used: often masks legitimate problems (in production use useful for defensive handling).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree. Removing this, however, will throw a UnrecognizedPropertyException

Unrecognized field "prop1_l10n"

The idea was to see what the configuration was actually doing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so it did hide a failure.

o.prop2.value = "bar";
o.prop2.value_l10n = Map.of("c", "d");

// currently produces JSON object with duplicate keys JSON
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess ideally it'd fail with exception? Since definition would instruct output like this.

@cowtowncoder
Copy link
Member

What puzzles me is the use of @JsonUnwrapped: if you have custom (de)serializers, there's often no point in using that annotation: custom (de)serializer will replace that handling.
There are ways for custom (de)serializers to support annotation; for JsonSerializer there is

    public JsonSerializer<T> unwrappingSerializer(NameTransformer unwrapper) {
        return this;
    }

that gets called if annotation indicates need; similarly for JsonDeserializer:

    public JsonDeserializer<T> unwrappingDeserializer(NameTransformer unwrapper) {
        return this;
    }

but as I said, these are rarely needed.

So I would just remove @JsonUnwrapped altogether if custom serializer-and-deserializer is needed.

@kariem
Copy link
Author

kariem commented Jun 6, 2025

@JooHyukKim thank you for your quick responses

  1. @JsonUnwrapped can be configured with prefix/suffix if that helps.

Yes, I had thought this too. But that does not result in what I would like to see

public class MyFancyClass {
	@JsonUnwrapped(prefix = "name_")
	private LocalizedField name;
	@JsonUnwrapped(prefix = "description_")
	private LocalizedField description;
    // ...
}

Will create

{
  "name_value": "name",  // name.value
  "name_value_l10n": {   // name.value_l10n
    "de": "name",
    "fr": "nom"
  },
  "description_value": "description",  // description.value
  "description_value_l10n": {          // description.value_l10n
    "de": "Beschreibung",
    "fr": "description"
  }
}
  1. Since you made custom serializer work as you wanted, how about spend some more time making custom deserializer to work?

That's actually my question. If I had the full JSON node, I could select the properties I want to parse. But this is not the case. Is there a way to get the parent node in a JsonUnwrapped property for custom deserialization?

Without custom serialization configuration, the mapper creates invalid JSON. See testUnwrappingWithDifferentProperties

Because that's how it's configured? Current result is what I would expect from plain configuration of @JsonUnwrapped.

I did not expect this. Just wanted to mention it. Wouldn't it be correct to throw a JsonGenerationException?

@cowtowncoder
Copy link
Member

Actually, as per my note on the issue (FasterXML/jackson#278), you might be able to change things overriding method findUnwrappingNameTransformer() of [Jackson]AnnotationIntrospector. I am not 100% sure if that allows flexible enough change but should be easy enough to try out. If it works, no custom (de)serializer would be needed.

@cowtowncoder
Copy link
Member

I did not expect this. Just wanted to mention it. Wouldn't it be correct to throw a JsonGenerationException?

One of DatabindExceptions or possible JsonGenerationException, depending on which layer. But yes, ideally.
Actually, if StreamWriteFeature.STRICT_DUPLICATE_DETECTION was enabled, JsonGenerationException would be thrown, I think. It is not enabled by default as it adds overhead (similar to how parser side does not check dups by default) but can be enabled.

@JooHyukKim
Copy link
Member

JooHyukKim commented Jun 7, 2025

Honestly name_value name_value_l10n seem reasonable. You gotta meet half way.

Can you try both serialxiation deserialziation roundtrip with prefix configured? It might work @kariem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants