Skip to content
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

Fix Json integration test and 2.8 highlights docs #10155

Merged
merged 1 commit into from Apr 9, 2020

Conversation

mkurz
Copy link
Member

@mkurz mkurz commented Mar 31, 2020

See comments.

.build()
.injector
.instanceOf[ObjectMapper]

"ObjectMapper" should {
"respect the custom configuration" in new JsonScope {
Json.mapper().isEnabled(SerializationFeature.WRITE_DATES_WITH_ZONE_ID) must beFalse
Json.mapper().isEnabled(SerializationFeature.WRITE_DURATIONS_AS_TIMESTAMPS) must beTrue
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://github.com/playframework/playframework/pull/9494/files#r400895796
This test is just not working. Not only we need to change to WRITE_DURATIONS_AS_TIMESTAMPS in the test, but also configure it to true, because false is the default anyway, so the test would still be useless.

Json.mapper().isEnabled(SerializationFeature.WRITE_DURATIONS_AS_TIMESTAMPS) must beFalse
Json.mapper().isEnabled(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS) must beFalse
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just confirms that WRITE_DURATIONS_AS_TIMESTAMPS is false by default and the above (now fixed) test makes sense now.

```

And if you need to write numbers as strings, add the following configuration:

```HOCON
akka.serialization.jackson.serialization-features.WRITE_NUMBERS_AS_STRINGS=true
akka.serialization.jackson.play.serialization-features.WRITE_NUMBERS_AS_STRINGS=true
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use the play binding name here, because that's what we do in

as well. Just to keep things consistent.

More info here: https://doc.akka.io/docs/akka/2.6/serialization-jackson.html?language=scala#additional-configuration
Implementation here:

private val BINDING_NAME = "play"
lazy val get: ObjectMapper = {
val mapper = JacksonObjectMapperProvider.get(actorSystem).getOrCreate(BINDING_NAME, Option.empty)

@mkurz mkurz added this to the Play 2.8.2 milestone Apr 2, 2020
@mkurz mkurz requested a review from gmethvin April 9, 2020 07:32
Copy link
Member

@gmethvin gmethvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mergify mergify bot merged commit e5ec3cc into playframework:master Apr 9, 2020
@mkurz mkurz deleted the fixJsonItTest branch April 9, 2020 19:36
@mkurz
Copy link
Member Author

mkurz commented Apr 9, 2020

@Mergifyio backport 2.8.x

@mergify
Copy link
Contributor

mergify bot commented Apr 9, 2020

Command backport 2.8.x: success

Backports have been created

mergify bot added a commit that referenced this pull request Apr 9, 2020
Fix Json integration test and 2.8 highlights docs (bp #10155)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants