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

Encoding Decoding Map to JSON #447

Closed
carlos-verdes opened this issue Nov 29, 2022 · 12 comments
Closed

Encoding Decoding Map to JSON #447

carlos-verdes opened this issue Nov 29, 2022 · 12 comments

Comments

@carlos-verdes
Copy link

How to reproduce, create a case class with an optional field of type Map:

case class ServerVersion(
    server: String,
    license: String,
    version: String,
    details: Map[String, String] = Map.empty
)

given Schema[ServerVersion] = DeriveSchema.gen[ServerVersion]

Expected test to pass:

      test("Encode/Decode ServerInfo from json using schema") {
        for
          _ <- ZIO.unit
          json = """{ "server": "arango", "license": "community", "version": "3.10.1"}"""
          chunks = Chunk.fromArray(json.getBytes)
          serverInfo <- ZIO.fromEither(
            JsonCodec.decode[ServerVersion](given_Schema_ServerVersion)(chunks)
          )
        yield assertTrue(
          serverInfo == ServerVersion("arango", "community", "3.10.1")
        )
      }

Error: the field details is decoded as Map("" -> "" ) instead of Map.empty:
image

I tried also with next annotations:

import zio.schema.annotation.{fieldDefaultValue, optionalField}

case class ServerVersion(
    server: String,
    license: String,
    version: String,
    @optionalField
    @fieldDefaultValue[Map[String, String]](Map.empty)
    details: Map[String, String] = Map.empty
)
@carlos-verdes
Copy link
Author

I checked the Spec and I see Map is encoded as a sequence of tuples like:

[
  {"key1":  value1},
  {"key2":  value2}
]

However the expectation for me is:

{
  "key1":  value1,
  "key2":  value2
}

I think we should change the full implementation of Map encoder/decoder not just the default values

@carlos-verdes carlos-verdes changed the title Decoding Json empty Map Encoding Decoding Map to JSON Nov 29, 2022
@carlos-verdes
Copy link
Author

Created some tests to reproduce these issues, please let me know if you agree with this Spec before I try to fix the code

image

image

@carlos-verdes
Copy link
Author

carlos-verdes commented Nov 29, 2022

@jdegoes @thinkharderdev @devsprint @tusharmath

This is a big problem to use in any API, I have to receive/pass very often Map[String, String] for extra configuration (for example if you use Spark) and it's really weird that is mapped to json [] instead of {}

I implemented a library for ArangoDB using zio-http and zio-json for serialization and I'm now trying to support zio-schema on top of that (so you can use ArangoDB with any type that has an Schema, and I plan to do dynamic graph queries that bring more than 1 entity just from the Schema abstraction)

@carlos-verdes
Copy link
Author

@jdegoes @thinkharderdev @devsprint @tusharmath

I finally had time to implement the full code, to start I only apply the new encoder/decoder when the Map has String as key
As I created a new PR this issue is reopened

We can apply more improvements on next PRs if you think is worth it:

  • apply similar logic for any key that is primitive converting the key into String (encoder.contramap(_.toString))
  • create a new annotation

@jdegoes
Copy link
Member

jdegoes commented Jan 30, 2024

/bounty $75

Copy link

algora-pbc bot commented Jan 30, 2024

💎 $75 bounty • ZIO

Steps to solve:

  1. Start working: Comment /attempt #447 with your implementation plan
  2. Submit work: Create a pull request including /claim #447 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to zio/zio-schema!

Add a bountyShare on socials

Attempt Started (GMT+0) Solution
🔴 @Samankhalid01 Feb 6, 2024, 7:10:35 PM WIP

@carlos-verdes
Copy link
Author

I think this was fixed on previous PR @jdegoes

@Samankhalid01
Copy link

Samankhalid01 commented Feb 6, 2024

/attempt #447

Copy link

algora-pbc bot commented Feb 13, 2024

@Samankhalid01: Reminder that in 7 days the bounty will become up for grabs, so please submit a pull request before then 🙏

Copy link

algora-pbc bot commented Feb 20, 2024

The bounty is up for grabs! Everyone is welcome to /attempt #447 🙌

@Andrapyre
Copy link
Contributor

Andrapyre commented May 12, 2024

@adamgfraser , maybe you want to claim the bounty for this, based on #531?

@jdegoes jdegoes closed this as completed May 12, 2024
@987Nabil
Copy link
Contributor

@jdegoes this issue still pops up on algora as an open bounty. Maybe you can remove the bounty?

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