-
-
Notifications
You must be signed in to change notification settings - Fork 798
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
Add support to get canonical JSON output #1037
Comments
First of all, yes, there is need for some sort of Canonical output. I don't think this should be done at Since This could also be an extension module, possibly providing either new type(s), or simply overriding default serializer of |
Re canonical JSON: I found this: https://gibson042.github.io/canonicaljson-spec/ I think this functionality doesn't belong inside of So it's on top of I agree the sorting of the keys can be done outside of the generator in a preparation step. But the rest (handling of trailing zeros, pretty printing) is already handled in As a developer, I would prefer something like If this is implemented this as a The other option is a special Maybe a simple solution for the sorting: Let If All that's then missing is a |
Well, from my perspective it certainly does not belong to Streaming API as it cannot be implemented in streaming manner. But I disagree on it not fitting in So: to make it simple -- I will not add canonical JSON handling at level of Another thing to consider, too, is whether Canonical handling would be useful for other dataformats. I would assume that for some (binary JSON ones, CBOR and Smile) it would make sense; and for most others (CSV, XML, Protobuf etc) not. |
Ah, now I get where you're at. My ideas were purely from a developer perspective. I was assuming What do you think about my proposal to move more code from That might have the additional benefit that it might be useful for other use cases. The other option would be a tree visitor which then delegates to a |
Right, if Unfortunately no, One could probably register a custom Delegates for I may be missing some other approaches. |
This sounds like the best option is then to preconfigure Right now,
The first one will add a lot of code in the existing class which doesn't really fit into the existing desing. The second one isn't very flexible since you have to change your existing code to use the new type. The last one is very flexible, isolates concerns but (as far as I can tell) would add a new concept. |
I've started with unit tests and a bit of code. It's more ugly than I hoped. You can have a look here: FasterXML/jackson-databind#3963 I left TODOs in all places where something was unclear or I think the solution could be better. One other thing that I just noticed: The pipeline tried to deploy the Java 8 build! I hope this happens in my repo and not in the one from Jackson. Can I turn this off or should I just ignore it? |
@digulla Ok on deploy... that is weird. It should filter out PRs and not try deploy. Feel free to ignore it, it's not your fault; main pipeline does need to do better job. |
@digulla One quick not on canonical JSON (from https://gibson042.github.io/canonicaljson-spec/) vs changes so far.
would work without Pretty Printer, and the additional configuration that adds single white-space after I suppose similarly the requirement to always force scientific notation (Spec) is different from your preferences. This leads to something I realized: maybe Canonicalizer (however implemented) should have some configurability wrt specific commonly desired aspects? |
I think there will be two main use cases:
For the former, we want a compact format. For the latter, we want pretty printing. Ideally, there should be a builder which presets everything for the two cases and then configuration methods or support configurers so developers can tweak the standard. For example, I would really like to be able to configure the number formatter. There are several useful ways to present numbers:
The first one is required by the standard. I'm not sure how useful that format is in real life. The format is also prone to rounding errors while parsing. The second form is easy to understand for human and it's also prone to rounding errors during parsing. The third form is a compromise between readability and keeping rounding errors in check since you can first read the number as (big)integer and then just multiply by a power of 10. The last form represents the number perfectly, is very fast to parse but maybe not portable between, say, Java and JavaScript. That's why I'm trying to add a generic API to format&parse numbers to Jackson. |
Yeah I don't think there will ever be generic API for number formatting, since that inevitably would result in additional overhead and complexity. Worse, since conversions between binary ( If limiting canonical handling to be accessed using Then again, if only using I guess I am still not quite sure what a workable solution should look like. |
I often need to compare the output of some REST API endpoint against a "known correct result" file.
This allows me to share the file with business, QAs and customers to get early feedback. It also helps them to keep track of changes since they get to see the current result without having to deploy and run the application.
For this to be efficient, I need a way to update the file when the output changes.
Right now, I can use JsonNode.equals() to check a whole tree for any changes but that doesn't tell me exactly where and what those changes are.
I can create an ObjectMapper which sorts (most) keys and which serializes BigDecimal consistently but there are still several corner cases:
I think the ideal solution would be a factory method to build a CanonicalJsonGenerator. It must
Optionally, it would be great to have a DefaultPrettyPrinter where _objectFieldValueSeparatorWithSpaces is just colon-space instead of space-colon-space. Maybe Separators should be extended to handle this case there.
I also like to convert date&time to ISO to make it easier to understand the values.
My current setup for reference is below. With this setup, I have to do this before I can compare the JSON strings:
The text was updated successfully, but these errors were encountered: