On Tue, Sep 1, 2020 at 9:51 AM Jonas Konrad <m...@yawk.at> wrote: > Hi, > > This E-Mail contains some thoughts on various aspects of the JSTEP > proposals at https://github.com/FasterXML/jackson-future-ideas/wiki/JSTEP > . >
Cool, thank you very much for sending these! > > # JSTEP 2 > > ## SORT_PROPERTIES_ALPHABETICALLY > > While it's true that there is no guarantee, in practice the order that > reflection returns matches the declaration order. Sticking to > declaration order is sensible for two reasons: > > - Declaration order is already the order that makes the most sense > semantically. Related fields will be close together and such. > - It provides a very simple solution for people that want to > "reorganize" the order of their json fields. With property sorting I > don't know how I would reorder fields, for example when jackson suddenly > spits out "first_name", "last_name", "id" as the property order even > though id makes most sense as the first property. Maybe I would try > using JsonProperty.index, but that only affects stuff like protobuf. > > People shouldn't *rely* on the order of properties in json objects and > they shouldn't rely on the order returned by reflection, but there is > still some value in sticking to that order in my opinion. > The main problem is that JDK does not guarantee any ordering, as far as I know, not even within class (although with Java 8 within one class declaration order of fields appears to be preserved?); and relative ordering of Fields and Methods is undefined even if order within each class was preserved (which, once again, I don't think is specific to be true). Things get even messier with inheritance: figuring what order even means (for methods, declared in interfaces) seems futile. So, in essence, I don't think there is such a thing as stable, predictable declaration ordering. So to me the choice is more between "an arbitrary unstable ordering" vs "a stable ordering with simple rules". > > ## READ_ENUMS_USING_TO_STRING > > What's the reasoning for this change? I can see two arguments here: On > one hand it makes it easy for users to use custom names (name() is > final) but on the other hand existing toStrings on enums may contain > auxiliary data that may be harmful for serialization (I found > > https://java-browser.yawk.at/com.google.guava/guava/29.0-jre/com/google/common/base/StandardSystemProperty.java#com.google.common.base.StandardSystemProperty%23toString() > as an example). > > If those are the reasons, I think I'd probably agree that using toString > is better on the balance. > This is just based on number requests to change it, allowing a simple non-annotated way of custom serializations. It also still works the same way for the simplest case of only declaring values with nothing else. So it seems likely that this would add a bit of functionality without much downside -- although there is something to be said for stability, avoiding unexpected changes to existing behavior (even if existing behavior might be sub-optimal on its own; now it is "the standard"). However: just because many ask for this to be changed obviously does NOT measure people who might prefer keeping setting as-is. So I am open to be convinced differently if anyone wants to argue for not changing this default. > > > ## FAIL_ON_UNKNOWN_PROPERTIES > > This is a weird one. It's super common to realize late that this should > be false for a project, but true seems like sane behavior to avoid bugs. > Also hard to get a general survey on this – sure, many people might > complain about it, but many people that are happy now would potentially > spend a lot more time debugging if this feature was flipped because they > don't realize their properties are being ignored. > > I think the added exception is the smaller issue when compared to a > "fail silent" scenario that disabling this feature introduces. > What I can say here is simply that: 1. My original choice was fully based on my own experiences of "fail silent" that drove me crazy wrt JAXB 2. This truly is the one setting that MANY people think I got wrong, and appears to be set to "ignore unknown" for most test cases (which is frustrating as that is exactly where one should [usually] not ignore them) I suspect this is something that should be studied in some form wrt wide-reaching poll. My thinking is that sometimes Vox Populi should be listened even if it seems wrong for long-term (caveat: if the downside is still limited, not in areas of bigger risk like adding/keeping security holes). > > ## Changes to Date/Time defaults > > See JSTEP 5. > > # JSTEP 3 > > ## LENIENT_SCALAR_CONVERSION > > Sounds like a big can of worms. Type coercion is a pita. Also remember > that the type coercion in js, which would probably drive the coercion > for jackson json parsing, is inconsistent at times. > Is this comment wrt default setting, or for feature itself? I think some control of coercibility is needed even to support evolution of existing operations. I specifically think this feature was to answer question on whether node.booleanValue() should work for `IntNode` or throw exception. However... this may need be re-thought with 2.12 added `CoercionConfig` which will allow more configurability: should be possible to check `CoercionConfig` for `JsonNode` which would allow specifying more granular settings (and ideally also extremes: "no coercions" / "everything and kitchen sink") conveniently. > > ## TRIM_DECIMAL_TRAILING_ZEROES > > true sounds like a sane default. imo values that are printed as scalars > in json are best off being in their "canonical" representation to get > through different json parsers untouched. > > ## Traversal > > I'd follow java collection conventions: forEach method for ObjectNode > and stream method for ArrayNode. Maybe a fieldStream for ObjectNode too, > or even better, a proper entrySet that matches Map.entrySet behavior. > I think I will need help with this particular aspect as I neither have had much time to think it through nor am I actually super-fluent with Java8/Streams implementation (from provider end, i.e. how to expose things in canonical/compatible way). > > btw, you call them JsonObject and JsonArray here, are you thinking about > renaming them? > Ah good point. No, I'll fix the document -- renaming plan, if any, needs to be spelled out. > > ## Transformation > > Maybe make a collector to use with streams? I think adding full blown > transformation methods to the container nodes is a step too far – you > could easily imagine tens of methods for various transformations that > few people would ever use because transforming json nodes directly is > not very common. > Right, I think it is more important to make node types allow streaming in a way in a way that allows re-building. I guess there might be need to also make construction accessible in similar way wrt `JsonNodeFactory`. But I definitely need others to help with ideas here. > > # JSTEP 5 > > Yes pleeeaasseee. > :) > > ## Format strings > > My proposal: Use java.time for initial deserialization everywhere, then > convert to other types as necessary. This would allow using only > java.time format strings. > > Problems: > > - compatibility with jackson 2 since you want to keep the same > annotations – how compatible are simpledateformat format strings with > java.time format strings? > I suspect they are not just subsets, and that there might not be simple or reliable one-way conversion for these. But I do not really have a good knowledge on this. It almost feels like we'd need a small group of experts to rewrite it: bit like how Kotlin module now has 2 owners with actual Kotlin knowledge (as well as help from original Kotlinista, Jayson) who can hash out good solutions, decisions. > - correct conversion. For Date this is easy, always parse as Instant and > convert to Date. But what about Calendar, where there's a weird mix of > time types? > Date has the big problem of it being a coordinate in space but one without TimeZone. Calendar does have TimeZone so in a way it ought to be slightly easier (but obv has horrible API). > > ## Default serialization format > > IMO stick to java.time toString formats, meaning usually ISO 8601 > strings. There's really no need to make the unix time format the default > anymore – ISO 8601 is more readable and the few bytes in savings really > don't matter when printing json > (by unix time format I assume you mean Java timestamp -- both are based on offset from Jan 1, 1970, GMT or so, but I think Unix uses seconds, Java milliseconds) Yes, since users can easily change it to timestamp, default probably should be textual (even if from processing perspective -- not just size, CPU -- timestamp handling can be orders of magnitude faster). That's what users expect, esp. outside Java-to-Java interactions. --- > > A common issue I have with evaluating these changes is that I have only > a narrow view of how people actually use jackson. I don't know how much > better that is for you Tatu, but maybe it would be worth it asking an > org like Pivotal that does lots of consulting involving jackson (as part > of spring) for feedback on how these changes would affect common use cases? > That is a great idea! Any Pivotal folks who could share their perspectives? I'd be happy to set up meetings, whether calls or just over chat (sending email on list obv. works well). -+ Tatu +- > > - Jonas > > -- > You received this message because you are subscribed to the Google Groups > "jackson-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to jackson-dev+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/jackson-dev/91415551-dbdf-8e2c-9192-1eb12d396e4b%40yawk.at > . > -- You received this message because you are subscribed to the Google Groups "jackson-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to jackson-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/jackson-dev/CAGrxA25w27c8noQiT%2B9jbuAGd2RWY8AJKG4F438h0A%3Dh-2DVXQ%40mail.gmail.com.
