-
Notifications
You must be signed in to change notification settings - Fork 11
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
Json & XML Dbcodecs #49
base: master
Are you sure you want to change the base?
Conversation
object PlayJsonDbCodec: | ||
def derived[A](using jsonCodec: OFormat[A]): PlayJsonDbCodec[A] = new: | ||
def encode(a: A): String = jsonCodec.writes(a).toString | ||
def decode(json: String): A = jsonCodec.reads(Json.parse(json)).get |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we can't provide these instances thanks to the Optional
magic of sbt 🤔
See "Eliminating dependencies" paragraph of https://blog.7mind.io/no-more-orphans
So no matter which JSON lib the user uses, he/she can just write:
case class LastService(mechanic: String, date: LocalDate) derives JsonDbCodec
and the PlayJsonDbCodec
instance would be automatically picked if play-json
is in its dependencies 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow.. that's mind blowing.. thanks for sharing, I'll try it out.
|
||
override def queryRepr: String = "?" | ||
|
||
override val cols: IArray[Int] = IArray(Types.OTHER) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I ask why Types.OTHER
here? 🤔
I implemented this way: IArray(Types.JAVA_OBJECT)
. Not sure to remember why. Probably something I found on internet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PgCodecTests "insert MagCar" test fails when using JAVA_OBJECT but works with OTHER
INSERT INTO mag_car (id, text_colors, text_color_map, last_service, my_json_b, my_xml) VALUES (?, ?, ?, ?, ?, ?)
With message:
org.postgresql.util.PSQLException: Unknown Types value.
magnum-pg/src/main/scala/com/augustnagro/magnum/pg/json/JsonDbCodec.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made a comment you're free to ignore as it can be more complicated to implement: https://github.com/AugustNagro/magnum/pull/49/files#r1807555435
Otherwise, I like the approach :)
Thanks for the review @guizmaii . It's a very interesting suggestion to use https://blog.7mind.io/no-more-orphans , I like the idea a lot. I wonder can we merge these codecs as-is, and then experiment with adding the Optional codecs per https://blog.7mind.io/no-more-orphans in a new MR? I want to make sure I fully understand the implications. |
Resolves #27