-
Notifications
You must be signed in to change notification settings - Fork 635
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
Fields name policy converters #33
Comments
This feature is really needed! |
What about adding new class This class can be something like
So, we can set "Fields name policy" in that config class. Or adding new paramter in Why to prefer 1 case: |
Has there been any movement on this feature? |
As a Gson maintainer which was used as justification for this feature, we regret field naming policies and do not recommend their use. Moshi, the Gson successor, does not include field naming policies because we view it as a mistake. |
Jackson library also has field naming policies. |
I would like to propose a 3rd "solution" that is to use an intercepting encoders/decoders. Basically this encoder/decoder would delegate to the actual encoder, but use a delegating Having said that. Related to the GSON issue I would say that there is a reason to not do this a your serialization can be used for two reasons:
Intercepting encoders/decoders could be useful for other cases as well (I saw a bug about encrypted data). Perhaps it would be worthwhile to provide a base class for these, but it is also an issue that would be better implemented outside the main runtime library. |
+1 for the global setting and annotation per class |
Any plans for this? |
@JakeWharton what was the reasoning behind this? I am not sure how would you have any problems if this is used as an optional feature. My guess would be performance concerns? |
Performance is not a concern. The serialization model is only built once. It's more that the magical behavior is needless and you should embrace the naming conventions of the layer you are modeling, even if they break the otherwise normal conventions of the language in which you are doing the modeling. And if you need an alternate name, it can be specified explicitly so as to not break tools like grep. More at https://publicobject.com/2016/01/20/strict-naming-conventions-are-a-liability/ |
I guess the search tools we use should support name policy converters 😁 Because now it seems that we have to do tedious actions in Kotlin only because of some external factors... And now if you even write the whole full-stack app in Kotlin, you still don't have a way to make the code more concise, for example, to minify names automatically (as in #908). |
Minifying names automatically is horrifying. It's impossible to make deterministic such that you retain compatibility across compilations and deployments. Persistence or blue/green deployment is thus impossible. I'm happy to fight against that in addition to field name policies, though. |
It would be nice to give people the option to use casing strategies if their usecase demands it. We have a situation where we have existing systems built differently and so use different casing strategies. We'd like to use kotlinx.serialization to generate contracts (avro IDLs generated using avro4k) in a global format (snake-cased) to enable the systems to speak with each other, but sometimes those IDLs are being generated from systems who already use camel-casing everywhere. With this restriction, we'd be forced to always annotate every field using |
What is the plan here? Is there any alternative way we can prevent having to write hundreds of I absolutely disagree with what is said in the blog post linked to by @JakeWharton by the way. A naming change in an API is a breaking API change and is simply forbidden, but even if it's needed, our backend team isn't supposed to check for it. Instead, they report it to our Mobile team and they will do the check. Everyone there will know that we are converting Considering Android apps, there's typically always an iOS counterpart and in Swifts official |
The only duplicate code is where you've specified a Any kind of automatic field naming conversion policy introduces additional edge cases that have to be considered. What if the JSON contains keys for An easy way to apply a field naming policy today is to specify your serialization format in an IDL and code gen the model objects on all platforms with appropriate policies applied at generation time through these annotations. It also means you're never out-of-sync on any platform and never have to write these model specifications more than once and never for a specific platform. Finally, citing [other library] has this feature is not an argument for or against the feature in this library. Gson has ton of features I can point you to, and the majority of them were bad decisions in retrospect. We include field naming policies as one of them, and they're thankfully absent from |
@JakeWharton I appreciate your answer, but you're just repeating the same arguments but not addressing our question: Why can't we have the choice to say "we don't expect any problems like 'external_id' and 'externalId' and we don't care about the libraries behavior in that case as it won't happen - feel free to crash in that case if needed!". I didn't cite other libraries which have this feature to say this is a good feature because others have it, too. I just said that their existence and wide adoption renders the argument of the blog post irrelevant. Your suggestion to use an IDL just proves again that your view is considering a very restricted set of use cases. I can understand that in some random serializer, but I don't think an official serializer for such a widely applicable language such as Kotlin should be so restricted. I don't know your company, but we don't have the resources to solve this problem with an intermediary layer - our code is working fine, and we don't expect any changes as this is forbidden within an API version as per our requirements. I suspect many others are in a similar situation and using an IDL is just overkill. |
I'm not the library author so I cannot answer the question. I can merely argue against this feature's inclusion as strongly as I can having been one of the maintainers of both Gson and Moshi which take different views on the subject matter. If I were the library author, my answer would be that you can already do this with Moreover, introducing the policy introduces not only edge cases and require careful thought, but severe indirection in the generated code such that I'd be concerned about the performance impact. Not the mention it's unclear whether supporting this is even possible in a binary compatible way in a post-1.0 world.
It doesn't. The argument is for not building it into anything new.
Yeah, not really. We (the maintainers of Gson and Moshi) have thought about the implications of supporting this at the library-level at length.
Considering using one of the many existing open-source tools for it which should require less work that defining models on multiple clients.
The stated goal was for field mapping and reduction of "boilerplate". IDL solves both of these completely eliminating the need to write models on any client. If you write models by hand, it's not unreasonable for the models to need to be explicit "by hand" as well. The layering of responsibility there is actually quite nice. When you move up a level of abstraction you get to define global transforms such as generated field name conversions.
Your opinion is noted. Mine is the opposite. We don't have to agree, and we won't. |
I don't know what your "this problem" is referring to if not "this feature request". These two sentences contradict each other. I thought this is a discussion about "this feature request" or did I misunderstand how GitHub issues work? 55 people have confirmed they have "this problem" and the OP explains very well how it is still unsolved. Stating that it is already solved is nothing else but downplaying this problem and ignoring our voices. But don't get me wrong, I do understand that you're trying to drive forward a specific way of thinking to prevent a given set of problems by design. I just disagree on the question if this is the right choice for the audience of this library. Or to put it in your own words:
😇 |
Interestingly, in the XML format I wrote, I've introduced the concept of a policy. When creating the format you can set an instance of the interface (there is a default). This is used to determine various aspect, including what tag or attribute name to use. It does make it much easier to be compatible with other serialization frameworks (without automatic renaming - although that is possible). Important here is that this is something that the user of the library provides. The library uses it to build the tree structure representing the serialization metadata, and uses that structure for actual serialization (it adds complexity, but is much more manageable than doing the same in various places of the serialization/deserialization itself). |
It does not, @JakeWharton, because you yourself suggested:
While that works for libraries that target a single serialization format (like Gson and Moshi), it does not necessarily work for libraries that target multiple serialization formats (like kotlinx.serialization). Because in case "the naming conventions of the layer you are modeling" conflict, it raises the question on which convention to align. And a configurable property naming strategy is an easy way to do that. A more complicated way is to have output-format-specific IDLs. |
Hey @sandwwraith, this issue has been open for quite some time. Is this feature on the roadmap? While I sympathize with the arguments against its addition, the reality is that many of us work with disparate systems that require differing naming conventions. As @gildor outlined in their original post, and others have discussed, the absence of this feature presents a major blocker for users who require the functionality. |
@rgmz how is it a blocker? Did you mean "inconvenience of verbosity"? If you use non-standard policies then it's only fair if you are inconvenienced for the sake of efficiency and compatibility of everyone else. Yes, we have to work with legacy systems. I'm myself here because I looked for that solution. But a more experienced person says "we tried it and it sucks". IMO, that suffices. @sschuberth do you remember what was the world before standard formats (like json) came? Everyone invented his own. You had to write a serializer/deserializer for pretty much every single format. Json is now more-or-less a common ground for everyone, but json authors failed to enforce naming policies. It's not late to fix that. |
I believe it is too late to enforce JSON naming policies. There's too much JSON in the wild that uses camel-case, snake-case, or whatever (even mixed in the same file) for field names. If you want to at least keep your data model naming clean, you need some sort of field name conversion. |
@sschuberth The question is more on where/how to do this. I can explain a little bit about how/why it is supported in the XML format. The first point is that the xml format creates shadow descriptors. It needs to do this for a number of purposes, including a more complex naming policy that differs for attributes and tags as well as namespace support. The shadow descriptors add an overhead to the format but allow for xml serialization to be quite flexible and follow a hierarchy that could have been hand-designed (they also make implementation more consistent between encoding and decoding). Key here is also that there needs to be a way to determine which tag/attribute names (with namespaces) to use and that choice is somewhat arbitrary - ie we always need a policy that cannot be For JSON the requirements are quite different. JSON maps much more directly upon programming language types and doesn't have to things like random order tags that are named based upon their type, not the attribute they are stored under. As such there is no need to incur the overhead of creating shadow descriptors as part of serialization. The What you are advocating is for a runtime system to map attribute names to serialnames, which however has statically deterministic results. In other words, it can be done statically (and incur no execution overhead). What is suggested is that:
|
Basically, if anyone is using |
Just to add my 2 cents. Literally every json dialect I deal with is using snake casing, so addressing this would make using this framework a bit less of the PITA it is right now for me. For better or for worse, snake casing is the dominant way to do identifiers in a wide range of languages. The JSON specification is not opinionated on what identifiers should and should not look like. Pretty much any valid string is valid to use as a key in a JSON object. Parsers / serializers should not be more opinionated than the standard. I'd recommend implementing the same naming conventions that jackson has. They implement the following strategies:
Along with a default of "as is". That would be a nice optional parameter to have on encode/decode functions and as a parameter on Having to spell this out on a per field basis with |
While I have implemented such a policy in the XML format, there is actually a serious limitation in the case of Json. That is that it is not currently possible for a format to know whether a name was given using One thing to consider though is that there are fundamentally two perspectives on serialization:
|
…nd properties (configured separately) Provide basic implementations: AllLowercase and SnakeCase Fixes #33
Although I agree that global naming strategies are somewhat malicious programming practice, given that there's a significant demand for this feature (including such use cases as migrating from Jackson to kotlinx.serialization), there's a prototype that'll likely get into the next RC: #2111 |
…ies' names. Provide basic implementation of SnakeCase strategy Fixes #33
A very common case of serialization is when API/JSON naming policy conflicts with your code naming, for example, snake case quite popular naming style for JSON fields, but for Kotlin it's camel case. Also, format naming style can be incompatible with JVM (for example field names with dashes).
Gson provides feature to solve that: https://google.github.io/gson/apidocs/com/google/gson/FieldNamingPolicy.html (implementation of FieldNamingStrategy)
In general, it's questionable feature, Moshi doesn't provide it and suggests to use the same case for your model classes.
But it can be a big blocker if you want to migrate to kotlinx.serialization, because you have 3 choices:
@SerialName
for each field. Tedious and error-prone and requires a lot of refactoringPossible solutions:
Possible problems: If you use more than one serialization format, different formats probably require different strategies, so we still should integrate naming policy to format implementation. Also, you sometimes want to use different naming policy for particular requests (from different API), so looks like Serialization format still should provide API to register and use naming policies.
The text was updated successfully, but these errors were encountered: