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

Using JsonValue and JsonFormat on one field does not work as expected #2822

Closed
nils-christian opened this issue Aug 19, 2020 · 4 comments
Closed
Milestone

Comments

@nils-christian
Copy link

nils-christian commented Aug 19, 2020

Hi,

We tried to use the both annotations JsonValue and JsonFormat on one field, but the serialization does not work as expected. Assume following code:

public static void main( final String[] args ) throws JsonProcessingException {
	final ObjectMapper objectMapper = new ObjectMapper( );
	objectMapper.enable( SerializationFeature.INDENT_OUTPUT );
	objectMapper.setVisibility( PropertyAccessor.ALL, Visibility.NONE );
	objectMapper.setVisibility( PropertyAccessor.FIELD, Visibility.ANY );

	System.out.println( objectMapper.writeValueAsString( new A( "Some Value", new B( BigDecimal.ONE ) ) ) );
}

private static class A {

	private final String description;
	private final B b;

	public A( final String description, final B b ) {
		this.description = description;
		this.b = b;
	}

}

private static class B {

	@JsonValue
	@JsonFormat( shape = Shape.STRING )
	private final BigDecimal value;

	public B( final BigDecimal value ) {
		this.value = value;
	}

}

Note that the field value of B has two annotations. The resulting string is

{
  "description" : "Some Value",
  "b" : 1
}

We would expect the value of B beeing written as a string, so we would expect

{
  "description" : "Some Value",
  "b" : "1"
}

If we remove the JsonValue annotation, we get the correct (but nested) value

{
  "description" : "Some Value",
  "b" : {
    "value" : "1"
  }
}

Is this a bug or the correct behaviour?

Thank you and best regards

Nils

Edit: We are using Jackson 2.11.2.

@cowtowncoder cowtowncoder added the to-evaluate Issue that has been received but not yet evaluated label Aug 19, 2020
@cowtowncoder
Copy link
Member

I think that this should work, ideally. So sounds like a real bug.
I hope to add a (failing) unit test to verify when I have time, and see how difficult this would be to fix.

Thank you for reporting this!

@cowtowncoder
Copy link
Member

I can reproduce this, and will see if use of @JsonFormat can be supported with @JsonValue (there are potential challenges unfortunately).

In the meantime if anyone is looking for work-arounds, forcing serialization as String for all BigDecimals, you'd use configuration override:

        MAPPER.configOverride(BigDecimal.class)
            .setFormat(JsonFormat.Value.forShape(JsonFormat.Shape.STRING));

@cowtowncoder
Copy link
Member

Ah ha. JsonValueSerializer.createContextual() is missing handling of one branch... seems promising.

@cowtowncoder cowtowncoder modified the milestones: 2.10.0, 2.11.3 Sep 30, 2020
@cowtowncoder
Copy link
Member

Good news: I fixed the problem with annotation propagation. One downside: I realized that the annotation placement to use must be bit different -- it has to be next to actual property, and not within definition next to @JsonValue, so:

private static class A {
        // Needs to be here as this is the actual property through which access occurs
	@JsonFormat( shape = Shape.STRING )
	private final B b;

	public A( final String description, final B b ) {
		this.description = description;
		this.b = b;
	}
}

private static class B {
	@JsonValue
        // CANNOT be here, alas
	private final BigDecimal value;

	public B( final BigDecimal value ) {
		this.value = value;
	}
}

and with that, fix works. Supporting alternative placement would be nice but I am not sure how feasible it is -- conceptually that would be logical: annotation on @JsonValue property should probably be used same way as class annotation of B would be.
That may be worth filing as another RFE issue; but for now I'll close this particular issue as there was an actual fix either way.

@cowtowncoder cowtowncoder removed the to-evaluate Issue that has been received but not yet evaluated label Dec 9, 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