-
Notifications
You must be signed in to change notification settings - Fork 84
Binary compatibility broken in 2.9.x in DateTimeSerializer
#99
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
Comments
I am not against adding back a (deprecated) constructor, if needed for backwards compatibility. But I am curious as to how this would cause breakage? Serializers and deserializers included are not meant to be part of public API, but only registered by module itself. But it sounds like something is directly instantiating (de)serializers? |
The code in question is basically as follows (or similar, the specific format and configuration varies somewhat):
There's similar code for deserialization in place as well. The intent here was to globally configure the default date format of the JSON for everything that is read/written using that ObjectMapper. One other issue is that if backwards compatibility is maintained, I would not have to go through all the libraries in our codebase that have constructed the date formats as above. Thanks for the explanation so far. |
@andrewl102 Just to make sure: I do not blame you or anyone for using publicly available classes. Intent is difficult to know, and it is not always easy to use limited mechanisms Java has to limit access. Now: one new mechanism that I think will and should work is "config overrides" , added in 2.8: https://medium.com/@cowtowncoder/jackson-2-8-features-5fc692887944 and these will allow basically associating information equivalent to I'll add the missing constructor as discussed. |
Hi, As someone who has primarily shifted from Java to Scala I understand entirely how difficult package restriction mechanisms can be. Unless I'm missing something though, I don't think I can use JsonFormat on a global basis can I? |
@andrewl102 You can use it on per-type (class) basis via above-mentioned Config Override mechanism. I would not expect anyone to (have to) use actual per-property annotations. So something like: mapper.configOverride(DateTime.class)
.setFormat(JsonFormat.Value.forPattern("yyyy.dd.MM")); For Jackson 3.0 I have the idea of allowing further "type groups" (or whatever term it'd be), in which there would be additional level to allow type group of "date/time". But until then per-type configuration should suffice. |
@cowtowncoder Thanks! |
Fixes #99 : binary compatibility issue
DateTimeSerializer
This is due to commit df3f16c.
Currently, we make use heavy usage of Jackson in various project, often resulting in heterogenous versions of Jackson and jackson-datatype-joda being present in the classpath.
As a result of this commit, binary compatibility was broken which presents a compatibility issue for us.
A previously valid construct call
public DateTimeSerializer(JacksonJodaDateFormat format)
is now invalid, due to the refactoring.There is a very trivial fix that could be applied, by simply reintroducing this construct and calling the new constructor as follows:
DateTimeSerializer(format,0)
.I'm happy to prepare a pull request if desired, though at only 1 line this appears a little redundant.
Is there any possibility of making this change?
Typically breaking binary compatibility in a library happens after first issuing a deprecation of the old constructor/method and/or in a major version bump, so it would be extremely helpful to maintain binary compatibility for now if possible.
The text was updated successfully, but these errors were encountered: