search

bizzabo / play-json-extensions

+22 field case class formatter and more for play-json
http://cvogt.org/play-json-extensions/api/
Other
196 stars 44 forks source link

Jsonx.formatCaseClass[A] should return OFormat not Format #32

Closed jmess4 closed 7 years ago

jmess4 commented 7 years ago

Play 2.5 macros use OFormat not Format.

Something like implicit val sampleFormt = Jsonx.formatCaseClass[Sample] will compile but fail at runtime to do any conversions.

cvogt commented 7 years ago

could you give me a code sample to reproduce this? Then I'll turn it into a test and release a fix.

apatzer commented 7 years ago

@jmess4 is correct, Play-Json 2.5+ uses OWrites[T] and OFormat[T]. I'm running into a similar issue on any case class, and further issue now that I've bumped up to "com.typesafe.play" %% "play-json" % "2.6.0-M3" which has built-in support for defaults (but not 22+ fields).

clamothe commented 7 years ago

I agree. For type safety, I'd prefer formatCaseClass to explicitly return an OFormat, if this is the underlying type. This way, I don't need to call jsVal.asInstanceOf[JsObject] for it's value.

cvogt commented 7 years ago

could someone create a PR ideally with a filing test case? I don't completely understand the issue or solution. Haven't been following 2.5 closely. Happy to get a new version out asap after.

clamothe commented 7 years ago

I'll do so if I can find the time. Thanks for the quick reply.

Sorry if this is pedantic. I'd rather be specific than not explain it sufficiently. OFormat is a Format restricted to reading and writing JsObjects (one case of JsValue). If formatCaseClass in fact creates a Format that always reads/writes JsObjects rather than another JsValue such as JsString, it could return OFormat instead. Consumers of formatCaseClass would then be able to assume in a type-safe manner that it reads/writes JsObjects. We have in our code an OFormat implementation that wraps formatCaseClass, providing some customized serialization. Since formatCaseClass returns a JsValue, our writes method looks like:

override def writes(o: T): JsObject = writer.writes(o).asInstanceOf[JsObject]

On Thu, Jun 1, 2017 at 6:57 PM, Jan Christopher Vogt < notifications@github.com> wrote:

could someone create a PR ideally with a filing test case? I don't completely understand the issue or solution. Haven't been following 2.5 closely. Happy to get a new version out asap after.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/xdotai/play-json-extensions/issues/32#issuecomment-305668227, or mute the thread https://github.com/notifications/unsubscribe-auth/AACTYuiPpy1gkQLq6_LRAGWMTCK20DKFks5r_2wegaJpZM4MzTEe .

clamothe commented 7 years ago

.. and if formatCaseClass doesn't in fact always read/write JsObjects, then our code would break at runtime if a non-JsObject is returned! Hence if we can make this assumption in a type-safe manner, it's best.

On Thu, Jun 1, 2017 at 7:06 PM, Charlie La Mothe charlie@clamothe.com wrote:

I'll do so if I can find the time. Thanks for the quick reply.

Sorry if this is pedantic. I'd rather be specific than not explain it sufficiently. OFormat is a Format restricted to reading and writing JsObjects (one case of JsValue). If formatCaseClass in fact creates a Format that always reads/writes JsObjects rather than another JsValue such as JsString, it could return OFormat instead. Consumers of formatCaseClass would then be able to assume in a type-safe manner that it reads/writes JsObjects. We have in our code an OFormat implementation that wraps formatCaseClass, providing some customized serialization. Since formatCaseClass returns a JsValue, our writes method looks like:

override def writes(o: T): JsObject = writer.writes(o).asInstanceOf[JsObject]

On Thu, Jun 1, 2017 at 6:57 PM, Jan Christopher Vogt < notifications@github.com> wrote:

could someone create a PR ideally with a filing test case? I don't completely understand the issue or solution. Haven't been following 2.5 closely. Happy to get a new version out asap after.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/xdotai/play-json-extensions/issues/32#issuecomment-305668227, or mute the thread https://github.com/notifications/unsubscribe-auth/AACTYuiPpy1gkQLq6_LRAGWMTCK20DKFks5r_2wegaJpZM4MzTEe .

clamothe commented 7 years ago

https://github.com/xdotai/play-json-extensions/pull/35