Skip to content
This repository has been archived by the owner on Jan 22, 2019. It is now read-only.

Fix for issue #11 causes inconsistent with default JsonPropertyOrder behavior #74

Closed
Spikhalskiy opened this issue Apr 3, 2015 · 7 comments
Milestone

Comments

@Spikhalskiy
Copy link

For this two classes will be generated schemas with different fields order. Fix for Issue #11 not only broke compatibility - it make inconsistent default behavior of CsvMapper with default JsonPropertyOrder value.

    class Point {
        public int y;
        public int x;
    }

    @JsonPropertyOrder
    public static class PointWithAnnotation extends Point {}
@Spikhalskiy Spikhalskiy changed the title Fix for issue #11 causes inconsistent with default JsonPropertyOrder behaviour Fix for issue #11 causes inconsistent with default JsonPropertyOrder behavior Apr 3, 2015
@cowtowncoder
Copy link
Member

Um... why would you use empty @JsonPropertyOrder here tho?

@Spikhalskiy
Copy link
Author

@cowtowncoder Just to illustrate that behavior for this two classes should be the same

@cowtowncoder
Copy link
Member

Unfortunately, due to Java annotations not having concept of 'null' or 'not define' value, they are not.
@JsonPropertyOrder is semantically same as @JsonPropertyOrder(alphabetic=false) because of default value (and not defining default would meka alphabetic required one to fill.

So while unfortunate, I am not sure much can be done here. Annotations do not allow use of Boolean (for example) -- many JavaSoft architects are heavily anti-null, and in my opinion this is one place where it is really unfortunate as use of default is very difficult with annotations.

@Spikhalskiy
Copy link
Author

@cowtowncoder Behavior for this two situations must be the same. Because I just add annotation with default value which is this situation should not change anything. No specified order == default order. So, default befavior of CsvMapper in case of no JsonPropertyOrder should be == JsonPropertyOrder with default value - "alphabetic=false". As for me fix and Issue #11 was broking + illegal without changing default behavior thru all components and default value in this annotation.

@cowtowncoder
Copy link
Member

Ok. If the behavior is not the same that does sound wrong. I'll see what gives that.

I would suggest, as an additional point, that if using CSV you really should always either specify explicit ordering, or default to using alphabetic ordering. The way 'alphabetic' works for @JsonPropertyOrder is problematic, but I do not see a way to resolve that part, given that most use cases are for explicit ordering and not for enabling alphabetic ordering.

@cowtowncoder
Copy link
Member

Actually no. I disagree.

Point has no annotation, so it defaults to settings apply alphabetic ordering, as CsvMapper instructs as the default.

PointWithAnnotation overrides ordering with @JsonPropertyOrder with "not alphabetic" setting (which is not very useful, agreed, but this is what original setting said). It happens to then produce ordering based on field order, although that is not guaranteed.

One possibility for 2.6 would be to re-define semantics of @JsonPropertyOrder.alphabetic so that 'true' would force sorting override, but 'false' would not change anything. Since it does not seem very useful to force use of an arbitrary, possibly unstable ordering this change could work, even if it is not ideal.
If this change was made it would change behavior to match your expectation. I also do not think it would cause breakage as I do not see value in explicit @JsonPropertyOrder(alphabetic=false).

@cowtowncoder
Copy link
Member

As per

FasterXML/jackson-databind#840

this is now fixed for 2.6.0; will be in 2.6.0-rc3.

@cowtowncoder cowtowncoder added this to the 2.6.0-rc3 milestone Jun 25, 2015
cowtowncoder added a commit that referenced this issue Jun 25, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants