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

Provide a non-Java-serialization-based Borer codec for OffsetDateTime #12

Closed
PawelLipski opened this issue Apr 24, 2021 · 13 comments
Closed
Labels
codecs Relates to useful/missing Circe codec (regardless of Akka serialization) on hold wontfix This will not be worked on

Comments

@PawelLipski
Copy link
Collaborator

PawelLipski commented Apr 24, 2021

There doesn't seem to be anything (?) like that in borer-core library at least... I'm getting Could not find implicit Encoder[java.time.OffsetDateTime] for parameter receivedAtUtc of case class ...

Maybe there is already another library (Borer-official or otherwise) that provides such codecs OOTB? Pls investigate

@PawelLipski PawelLipski added HIGH PRIORITY Should be picked up first codecs Relates to useful/missing Circe codec (regardless of Akka serialization) labels Apr 24, 2021
@MarconZet
Copy link
Collaborator

I tried searching for a library that satisfies this condition, but couldn't find any. The Borer-official libraries include codecs for Akka and Cats, but not Serializable.

I have already implemented Codec for java.time.OffsetDateTime. Looking at my implementation, it may be possible to automatically create Codecs for all subtypes of Serializable, but that's putting a lot of faith in this mechanism.

@PawelLipski
Copy link
Collaborator Author

The Borer-official libraries include codecs for Akka and Cats, but not Serializable.

Yes, that's expected, Serializable is a marker trait for Java serialization, which is a mechanism very different from what Borer offers :)

it may be possible to automatically create Codecs for all subtypes of Serializable

Again, we almost surely don't want that, as is goes against the philosophy of Borer ☹️

@PawelLipski
Copy link
Collaborator Author

Actually... how about https://github.com/plokhotnyuk/jsoniter-scala? Does it have some ready Borer codecs for java.time stuff like OffsetDateTime?

@MarconZet
Copy link
Collaborator

It has a lot of Borer codecs, and some java.time stuff is defined here
Most of these are custom classes, but java.time.*, BigDecimal and BigInt may be useful.

@PawelLipski
Copy link
Collaborator Author

Ok so if you could replace the existing org.virtuslab.akkasaferserializer.StandardCodecs#offsetDateTimeCodec with some ready jsoniter stuff; pls leave it in PR #3 as is, open a separate PR for that

@PawelLipski
Copy link
Collaborator Author

Let's resolve the jsoniter vs borer discussion first though: #7 (comment)

@PawelLipski PawelLipski removed the HIGH PRIORITY Should be picked up first label Apr 28, 2021
@PawelLipski
Copy link
Collaborator Author

PawelLipski commented Apr 28, 2021

Removing the high-priority label since this is currently worked around by a Codec that delegates the work to Java serialization.

This is not a production-ready implementation though, so this issue still remains open.

@PawelLipski
Copy link
Collaborator Author

Actually @sirthias ... can you think of any ready implementation?

@sirthias
Copy link

Writing a codec for java.time.OffsetDateTime is not hard at all.
It's just a question of how you want to represent it.

AFAICS an OffsetDateTime instance consists of basically two parts: a LocalDateTime and a ZoneOffset.
So, ideally we'd write codecs for those and then simply compose the codec for OffsetDateTime from them.

It looks like a LocalDateTime can simply be represented as a Long and a ZoneOffset can be mapped to an Int.
So, quite simple, if I see things correctly.

@PawelLipski PawelLipski changed the title Provide a Borer codec for OffsetDateTime Provide a non-Java-serialization-based Borer codec for OffsetDateTime Apr 30, 2021
@PawelLipski
Copy link
Collaborator Author

Let's wait for #27 to clarify first

@plokhotnyuk
Copy link

Small and more efficient implementations of .parse and .toString for java.time._ types are here.

@PawelLipski
Copy link
Collaborator Author

Hmm @plokhotnyuk is the code you linked zio-json-specific in any way? can it be easily adapted for the use with e.g. Borer? 🤔

@plokhotnyuk
Copy link

plokhotnyuk commented Jul 1, 2021

@PawelLipski No dependencies on zio or zio-json in the production code. Tests of that repo can be easily migrated from zio-test to scalatest or as an option you can choose to adopt these scalatest tests due to better coverage of negative cases.

@PawelLipski PawelLipski added the wontfix This will not be worked on label Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codecs Relates to useful/missing Circe codec (regardless of Akka serialization) on hold wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants