Skip to content
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

Java 8 Optional not working with @JsonUnwrapped on unwrappable type #2565

Closed
yushijinhun opened this issue Dec 8, 2019 · 4 comments
Closed
Milestone

Comments

@yushijinhun
Copy link

public static class Bean {
	public @JsonUnwrapped Optional<String> value;
}

var b = new Bean();
b.value = Optional.of("str");
jackson.writeValueAsString(b);
com.fasterxml.jackson.core.JsonGenerationException: Can not write a string, expecting field name (context: Object)
	at [email protected]/com.fasterxml.jackson.core.JsonGenerator._reportError(JsonGenerator.java:2080)
	at [email protected]/com.fasterxml.jackson.core.json.JsonGeneratorImpl._reportCantWriteValueExpectName(JsonGeneratorImpl.java:248)
	at [email protected]/com.fasterxml.jackson.core.json.JsonGeneratorImpl._verifyPrettyValueWrite(JsonGeneratorImpl.java:238)
	at [email protected]/com.fasterxml.jackson.core.json.WriterBasedJsonGenerator._verifyValueWrite(WriterBasedJsonGenerator.java:894)
	at [email protected]/com.fasterxml.jackson.core.json.WriterBasedJsonGenerator.writeString(WriterBasedJsonGenerator.java:400)
	at [email protected]/com.fasterxml.jackson.databind.ser.std.StringSerializer.serialize(StringSerializer.java:41)
	at [email protected]/com.fasterxml.jackson.databind.ser.std.ReferenceTypeSerializer.serialize(ReferenceTypeSerializer.java:381)
	at [email protected]/com.fasterxml.jackson.databind.ser.impl.UnwrappingBeanPropertyWriter.serializeAsField(UnwrappingBeanPropertyWriter.java:127)
	at [email protected]/com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeFields(BeanSerializerBase.java:722)
	at [email protected]/com.fasterxml.jackson.databind.ser.BeanSerializer.serialize(BeanSerializer.java:166)
	at [email protected]/com.fasterxml.jackson.databind.ser.DefaultSerializerProvider._serialize(DefaultSerializerProvider.java:480)
	at [email protected]/com.fasterxml.jackson.databind.ser.DefaultSerializerProvider.serializeValue(DefaultSerializerProvider.java:319)
	at [email protected]/com.fasterxml.jackson.databind.ObjectMapper._configAndWriteValue(ObjectMapper.java:4094)
	at [email protected]/com.fasterxml.jackson.databind.ObjectMapper.writeValueAsString(ObjectMapper.java:3404)

I think change the expression to

_unwrapper != null && _valueSerializer.isUnwrappingSerializer()

may fix it, but _valueSerializer can be null here. Hoping anyone familiar with this can fix it.

@cowtowncoder
Copy link
Member

Thank you for reporting this. I'll have to see what might be the correct behavior, given constraints; that is, whether it is possible to reliable support combination of ReferenceType and unwrapped POJOs.

@cowtowncoder cowtowncoder added 2.10 and removed 2.10 labels Dec 10, 2019
@cowtowncoder cowtowncoder added this to the 2.11.0 milestone Dec 10, 2019
@cowtowncoder cowtowncoder changed the title Java 8 Optional not working with @JsonUnwrapped Java 8 Optional not working with @JsonUnwrapped on unwrappable type Dec 10, 2019
@cowtowncoder
Copy link
Member

Ok, so I was able to resolve the issue as reported (used AtomicReference for testing as databind can not depend on JDK8 types, but should work the same for Optional). But since I am bit uneasy on whether this might cause issues elsewhere -- there are no test failures but combination of unwrapped and reference type is not super well tests -- decided to fix only for 2.11 (next minor version) and not for 2.10.x patches. Trying to get bit more cautious on what goes in patch vs minor version.

Was also wondering if this case should instead throw an exception (use of @JsonUnwrapped on type that does not actually support unwrapping), but decided not to change semantics as no such checks have been made before.

@yushijinhun
Copy link
Author

Was also wondering if this case should instead throw an exception (use of @JsonUnwrapped on type that does not actually support unwrapping), but decided not to change semantics as no such checks have been made before.

I prefer to keep the current behavior. It is useful when I do not know the exact type of the field to be unwrapped. For example:

class TimestampedValue<T> {
    @JsonProperty("_timestamp") long timestamp;
    @JsonUnwrapped T value;
}

I prefer to unwrap the value field since it's succinct. But this is not always possible, so I need this feature as a fallback.

@cowtowncoder
Copy link
Member

Ok. Since it has not been blocked before, it seems reasonable to keep it that way even if this specific style of usage is not something I thought as original use case.

@cowtowncoder cowtowncoder removed the 2.11 label Apr 12, 2020
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

No branches or pull requests

2 participants