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

Codec derivation issues upgrading from v6.0.0 to v7.0.0 #71

Open
marko-asplund opened this issue Jun 14, 2020 · 14 comments
Open

Codec derivation issues upgrading from v6.0.0 to v7.0.0 #71

marko-asplund opened this issue Jun 14, 2020 · 14 comments
Labels

Comments

@marko-asplund
Copy link

I'm bumped into two issues when upgrading to v7.0.0:

  1. When upgrading from play-json-derived-codecs v6.0.0 to v7.0.0 Play JSON picks up a different JSON serializer in a scenario involving sum types when codecs for both the case classes and the sealed trait are in scope

  2. codec derived for sealed trait doesn't appear to work for the case classes

Both of these issues can be reproduced with code found in this repository:
https://github.com/marko-asplund/play-json-derived-codecs-issue

@julienrf julienrf added the bug label Jun 15, 2020
@julienrf
Copy link
Owner

Thanks for the report @marko-asplund. Would you be interested in investigating what’s going on?

@marko-asplund
Copy link
Author

@julienrf This issue is currently blocking Play v2.8 upgrade, so I'll need to figure out some kind of workaround. Any pointers for investigating the issue?

@julienrf
Copy link
Owner

I think you did a great job of sharing the test cases that fail. Thank you for this! The next step would be to debug which implicit definitions are used in both cases (with v6 and v7) to see where they diverge. The compiler plugin tek/splain could help a lot in this area.

@marko-asplund
Copy link
Author

Thanks for the tip! 👍 I'll look into debugging using the plugin.

🤔 How do I get the plugin to output implicit definitions? I've tried setting this up in build.sbt, but e.g. compile doesn't seem to generate any additional output.

@julienrf
Copy link
Owner

Hmm, I don’t know what’s wrong. Your setup looks good to me. Did you reload sbt?

@marko-asplund
Copy link
Author

Yeah, I did try reloading and restarting sbt several times.

@marko-asplund
Copy link
Author

This issue seems to have been caused by variance changes in play.api.libs.json.Writes between play-json v2.7 and v2.8.
Writes was made invariant on A whereas before it was contravariant.

play-json v2.7.4

package play.api.libs.json

trait Writes[-A] { self =>
...

play-json v2.8.1

package play.api.libs.json

trait Writes[A] { self =>
...

This changed in commit playframework/play-json@ba86577#diff-43f2e42c6ce469bcd932a93a187413fe

Because of this change Json.toJson doesn't allow serializing SubType using OWrites[SuperType] anymore.

@julienrf
Copy link
Owner

julienrf commented Jun 27, 2020

Thanks for the investigation @marko-asplund!

I guess for the second problem you’ll have to manually upcast your values:

-    val b2 = Json.toJson(Bar2("hello", 55))
+    val b2 = Json.toJson(Bar2("hello", 55): Foo2)

The issue 1 is more problematic. I’m not sure how to fix it.

@marko-asplund
Copy link
Author

marko-asplund commented Jun 27, 2020

@julienrf Thanks for the feedback!

issue 1: Writes was changed to be invariant on A. Before this change both Writes[SubType] and Writes[SuperType] were eligible for implicit resolution, but after the change only Writes[SubType] is, effectively forcing Writes[SubType] to get picked up.

issue 2: only Writes[SuperType] is defined. Since Writes is now invariant it can't be used with SubType.

IMO, though it's not an ideal solution, passing the desired type parameter to Json.toJson (Json.toJson[SuperType]) or upcasting to SuperType can be used as a workaround and force use of Writes[SuperType] in both cases.

🤔 Trying to figure out what play-json-derived-codecs could do to address this issue.
Not sure if there's much that can be done in terms of addressing this issue in terms of play-json-derived-codecs features.
As I guess this issue has the potential to hit many play-json-derived-codecs users who are using it with sum types, one thing could be to add a caveat in the README about upgrading from v6 to v7.

@julienrf
Copy link
Owner

As I guess this issue has the potential to hit many play-json-derived-codecs users who are using it with sum types, one thing could be to add a caveat in the README about upgrading from v6 to v7.

Yes, that’s a good idea, thanks for proposing! We should provide the workarounds in the README.

@marko-asplund
Copy link
Author

🤔 Also wondering what was the exact rationale for play-json to go this way. It would've been very helpful to publish a heads up about this e.g. in the form of breaking changes in v2.8 release notes. I couldn't find anything on this, at least not with a quick search and couldn't find release notes for v2.8 either.

@julienrf
Copy link
Owner

That’s a good point. Personally, I don’t understand this decision.

@marko-asplund
Copy link
Author

Here's a possible workaround marko-asplund/play-json-derived-codecs-issue@fc0c7b0

@julienrf
Copy link
Owner

It seems that they are aware of the problem: playframework/play-json#404

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants