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

Unable to convert to TypedValue #511

Open
tusharmath opened this issue Feb 10, 2023 · 14 comments
Open

Unable to convert to TypedValue #511

tusharmath opened this issue Feb 10, 2023 · 14 comments
Labels
💎 Bounty bug Something isn't working

Comments

@tusharmath
Copy link
Contributor

If the schema is rebuilt from the AST, it doesn't correctly convert a dynamic value back to a typed value. This is specifically happening with records.

import zio.schema._


final case class User(name: String, age: Int)


implicit val schema = DeriveSchema.gen[User]


val user = User("John", 30)
val userDynamic = schema.toDynamic(user)


println(userDynamic)
println(userDynamic.toTypedValue(schema))
println(userDynamic.toTypedValue(schema.ast.toSchema))

https://scastie.scala-lang.org/TJmOjesrS5KY5ZYHOwJTUg

@tusharmath tusharmath added the bug Something isn't working label Feb 10, 2023
@jdegoes
Copy link
Member

jdegoes commented Apr 25, 2023

/bounty $200

@algora-pbc
Copy link

algora-pbc bot commented Apr 25, 2023

💎 $200 bounty • ZIO

Steps to solve:

  1. Start working: Comment /attempt #511 with your implementation plan
  2. Submit work: Create a pull request including /claim #511 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

Additional opportunities:

Thank you for contributing to zio/zio-schema!

Add a bountyShare on socials

Attempt Started (GMT+0) Solution
🔴 @alankritdabral Nov 12, 2023, 12:25:29 PM WIP
🔴 @foxmoder Jan 7, 2024, 10:52:07 PM WIP

@andrzejressel
Copy link
Contributor

Looking at zio-schema architecture I don't understand how would it work. Using schema.toDynamic and schema.ast move object and schema from typesafe to typeless world.

While toTypedValue(schema) can return it back to typesafe because schema has type Schema[A], userDynamic.toTypedValue(schema.ast.toSchema) combines typeless object with typeless schema.

@alankritdabral
Copy link

alankritdabral commented Nov 12, 2023

/attempt #511

Options

Copy link

algora-pbc bot commented Nov 19, 2023

@alankritdabral: 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 Nov 26, 2023

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

@pqcfox
Copy link

pqcfox commented Jan 7, 2024

Going to go ahead and toss my hat in the ring! /attempt #511

Options

@pqcfox
Copy link

pqcfox commented Jan 8, 2024

Just dove in to make sure I understand the problem--agree with @andrzejressel on those concerns. Apologies in advance that I'm very new to ZIO, so if anything below doesn't make sense, please let me know!

For the purposes of this MR, @jdegoes would it make sense to add additional typing information to MetaSchema and change the record method in Schema to leverage this (i.e. to construct e.g. a CaseClass2 via deriveCaseObject based on this typing information, rather than a GenericRecord)?

The alternatives I thought of were just adding a new, separate TypedMetaSchema class that carries more typing information, or the more conservative option of somehow adding warnings when trying to use toTypedValue on typeless Schemas.

Any of these seem like good options for me to implement?

@jdegoes
Copy link
Member

jdegoes commented Jan 8, 2024

@andrzejressel @pqcfox

I think the only way to support this would be to do so using reflection. The class name can be stored (indeed, we already have TypeId), but then to use it when creating the record seems to require reflection.

An alternative might be to allow passing a 'registry' to toTypedValue, something like a PartialFunction[TypeId, Constructor], which would avoid java reflection at the cost of introducing our own version of that.

@tusharmath I know you since moved on but did you have any ideas on how you originally wanted this issue to be solved?

@tusharmath
Copy link
Contributor Author

Hey John!
I realized that it was not possible to do this a while back. Basically, any custom type can't be decoded this way. We shouldn't allow generating schema of any schema and instead trigger a compile-time error. For eg: Schema[Schema[List[String]]] should be allowed but Schema[Schema[User]] should not be possible to create ideally. That would be a reasonable developer experience to have, because currently, this introduces runtime bugs.

@pqcfox
Copy link

pqcfox commented Jan 8, 2024

Makes sense to me! I can implement that--any list of allowed types I should work off when modifying Metaschema?

@pqcfox
Copy link

pqcfox commented Jan 14, 2024

(Additionally, if I wanted to address this in a PR, would I do so under this issue, and would it be in scope for the existing bounty?)

Copy link

algora-pbc bot commented Jan 14, 2024

@foxmoder: 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 Jan 21, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💎 Bounty bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants