Repository navigation
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
9b48b9e to
9eb7288
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
@gaeljw @mkurz Jackson 2 is stable and still supported. I don't see much reason to move to Jackson 3 yet. It could take a while for it to become stable. In Pekko, I'm retaining Jackson 2 support while adding Jackson 3 support in Pekko 2 as an option. |
|
The intent is only to be "up to date". I'm a bit surprised when you say Jackson 3 might not be stable yet: it's the default in Spring Boot for a few weeks now, a couple of libraries I know have already switched to Jackson 3... Do you have something specific in mind or it's just the fact that it's a major version and takes time to get feedbacks and detect issues? I don't think there's any major reason to upgrade as soon as possible but on the other hand I don't see a reason to wait. The next minor version of Play could be a good candidate for the upgrade, isn't it? Especially, as Jackson 2 and 3 can live together in the same codebase, this doesn't force users to have all their dependencies aligned on Jackson 3. FWIW, we've been using Jackson 3 + Jackson 2 (both present at runtime, Play using 2.x and other libraries using 3.x) at work in a few projects for a few weeks and didn't have any issue so far. |
|
@mkurz @gaeljw Jackson 3 is now onto v3.2.1 and can be regarded as being as stable as Jackson 2 latest releases. |
|
I can spend a bit of time to rebase the PR on top of the latest changes (both Play JSON & Jackson) so that the status is up to date, I believe a few things changed in the meantime. |
31152c3 to
5680fc9
Compare
|
PR updated, tests are working. I'm working on a PR for playframework as well. |
Pull Request Checklist
Fixes
Fixes #1258
Purpose
Upgrade Jackson from 2.x to 3.x.
Background Context
build.sbt: I merged declaration ofjackson-databindwith the other Jackson dependencies, not sure why it was separate but there doesn't seem to be any reason anymoreJacksonJson:ObjectMappertoJsonMapperso that we can use.enable(JsonWriteFeature.ESCAPE_NON_ASCII)(not available onObjectMapperas it's a JSON specific feature). This makes sense anyway IMHO but it'll be a source breaking change for users ofsetObjectMapper(Play Framework?)serializemethod forJsNumber: with Jackson 3, the actual implementation we get forJsonGeneratoris aTreeBuildingGeneratorthat does not supportwriterNumber(String)(it used to support it with a wrong implementation, see also some context inTreeBuildingGenerator#writeNumber(String)writes Number as String FasterXML/jackson-databind#5528). Hence keeping aBigDecimalto usewriteNumber(BigDecimal).shouldWritePlainistrue, there's an extraBigDecimalallocation compared to before.TokenBufferbranch as I didn't see any reason to handle it specifically anymoreJsonSpec: changed max depth as per Jackson change (1000 -> 500)ParameterNamesModule,Jdk8Module,JavaTimeModule, hence we no longer need to enable them explicitly (we couldn't anyway)Remaining work
setObjectMapperReferences