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

Support for canonical JSON output in Jackson. #3963

Closed
wants to merge 2 commits into from

Conversation

digulla
Copy link

@digulla digulla commented Jun 4, 2023

Support for canonical JSON output in Jackson.

I kept all classes in the tests folder since there will be many more changes and it's easier when everything is in a single place.

I kept all classes in the tests folder since there will be many more changes and it's easier when everything is in a single place.
@cowtowncoder
Copy link
Member

cowtowncoder commented Jun 5, 2023

Ok cool! Couple of notes/suggestions:

First, master is for Jackson 3.0; we probably would want this for 2.x, so maybe developing against 2.16 would make sense.

Second: it might be possible to package this as Module (3.x JacksonModule); modules can register serializers, change various Features? Registering something like CanonicalJSONModule would be nice way to add on this functionality.
I guess there are parts that wouldn't work (override/delegation for number writing), but maybe majority.

Also: while SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS works for Maps (and different MapperFeature for POJO properties), there'd also be need for a JsonNodeFeature for sorting properties of ObjectNodes on output -- that would be useful in general; I'll file an issue for that.

@digulla
Copy link
Author

digulla commented Jun 5, 2023

Okay. I'll rebase the code to 2.16 and try to move every config that I can to a new CanonicalJSONModule so we can see what's left.

@cowtowncoder
Copy link
Member

Thank you @digulla ! I am starting to think this would be a great addition to 2.16 if we can figure out how to package things.

Sounds like there are at least these things to tackle:

  1. Sorting of Object properties alphabetically (although not 100% sure what specific sorting semantics needed; is configurability needed)
  2. Pretty-printing using settings slightly different than Jackson default one (but achievable with what PrettyPrinter allows)
  3. Limiting format of BigDecimal serialization (and maybe magnitude/scale?)

of which first 2 seem easier to achieve.

}

return text.substring(0, 1) + '.' + text.substring(1) + 'E' + exp;
}
Copy link
Member

@JooHyukKim JooHyukKim Jun 6, 2023

Choose a reason for hiding this comment

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

Is there an existing reference to this implementation or newly created? I think we can use some methods within BigDecimal itself, or JDK methods like java.text.DecimalFormat to simplify the conversion.

Copy link
Author

Choose a reason for hiding this comment

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

I created this from scratch since I couldn't find anything suitable on the classpath.

The toString() methods in BigDecimal itself don't work (they either don't print an exponent or print only some numbers with exponent) and DecimalFormat has weird quirks (like being not thread safe which might or might not cause problems). There are other implementations of DecimalFormat in some Apache commons library but I didn't want to add new dependencies.

This class only exists because https://gibson042.github.io/canonicaljson-spec/ requires it. Personally, I would always print the number in a readable form without exponent OR as an integer with a negative exponent (so it can always be parsed without any rounding errors).

I would be grateful for advice how to proceed here.

public DefaultPrettyPrinter withSeparators(Separators separators) {
_separators = separators;
// TODO it would be great if it was possible to configure this without
// overriding
Copy link
Member

@JooHyukKim JooHyukKim Jun 6, 2023

Choose a reason for hiding this comment

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

True, this whole class seems like it could be all done with a (for example) Builder.
Improvement idea, @cowtowncoder ?

Copy link
Author

Choose a reason for hiding this comment

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

Actually, I would add a new option to Separators like beforeColon. Then I can configure it without any overrides and the code doesn't change much.

Added JsonAssert with several assertion methods that can be used similar to assertEquals().
@digulla
Copy link
Author

digulla commented Jun 6, 2023

I've created a new MR which is based off Jackson 2.16: #3967

Feel free to close this one.

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.

None yet

3 participants