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

Support seamless transformation of protobuf oneof types #531

Closed
4 tasks done
ghostdogpr opened this issue May 8, 2024 · 4 comments · Fixed by #568
Closed
4 tasks done

Support seamless transformation of protobuf oneof types #531

ghostdogpr opened this issue May 8, 2024 · 4 comments · Fixed by #568

Comments

@ghostdogpr
Copy link

Checklist

Describe the desired behavior

Let's consider AddressBookType from ProtobufOneOfSpec.

We can convert addressbook.AddressBookType into pb.addressbook.AddressBookType.Value and vice-versa. This works great.

However if AddressBookType is used inside another protobuf message, the Scala object for that message will have a field of type pb.addressbook.AddressBookType, not pb.addressbook.AddressBookType.Value. It means that it still requires a manual implicit transformer to convert that parent object.

It would be great if there was a seamless transformation between the 2 corresponding types, addressbook.AddressBookType and pb.addressbook.AddressBookType.

Use case example

Let's add this to addressbook.proto:

message Parent {
  AddressBookType address_book_type = 1;
}

And this to AddressBook.scala:

case class Parent(addressBookType: AddressBookType)

Now let's try to create a Parent and transform it:

val parent = addressbook.Parent(domainType)
parent.into[pb.addressbook.Parent].transform

It fails with:

no accessor named value in source type io.scalaland.chimney.fixtures.addressbook.AddressBookType

A workaround is to create a Transformer from addressbook.AddressBookType to pb.addressbook.AddressBookType:

implicit def transformer: Transformer[addressbook.AddressBookType, pb.addressbook.AddressBookType] =
  v => pb.addressbook.AddressBookType(v.transformInto[pb.addressbook.AddressBookType.Value])

How it relates to existing features

If there is a simpler workaround, please let me know. I have hundreds of classes from protobuf so I am trying to find a solution that doesn't involve manually defining transformers for each oneof.

@MateuszKubuszok
Copy link
Member

The issue is similar to what is done in https://github.com/scalalandio/chimney/blob/master/chimney-protobufs/src/test/scala/io/scalaland/chimney/ProtobufOneOfSpec.scala#L13 - PB arbitrarily decides what to wrap and what to pass straight forcing users to wrap/unwrap in arbitrary places.

This would require change in macros which would test if something is wrapper and unwrap automatically even if the type is not an AnyVal which is kinda ugly - I'd allow such thing only with a flag as this could generate some subtle bugs and increase the compilation time.

@ghostdogpr
Copy link
Author

Maybe only check if the inner type is a scalapb.GeneratedOneof?
I agree with you, it's quite ugly and unfortunate that scalapb doesn't give us a better way to do that 🤔

@MateuszKubuszok
Copy link
Member

MateuszKubuszok commented May 8, 2024

Maybe only check if the inner type is a scalapb.GeneratedOneof?

That would require making the core, dependency-free, module dependent on some external implementation. Or introducing a way of injecting extensions into makro with some type classes and implicits.

Maybe some workaround like:

implicit def transformAndWrap[
    From,
    To <: scalapb.GeneratedMessage,
    ToValue <: scalapb.GeneratedOneof
](
  implicit transformer: Transformer.AutoDerived[From, ToValue],
  // shapeless.HList or Mirror.ProductOf as evidence + constructor
  // e.g. Generic.Aux[To, ToValue :: HNil]
  // or Mirror.ProductOf[To] { type MirrorElementTypes = ToValue +: EmptyTuple }
): Transformer[From, To] =
  src => {
    val value = transformer.transform(src)
    // wrap value using shapeless/mirror
  }

would pass in the meantime.

@MateuszKubuszok MateuszKubuszok linked a pull request Jul 8, 2024 that will close this issue
7 tasks
@MateuszKubuszok
Copy link
Member

Released in 1.3.0

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

Successfully merging a pull request may close this issue.

2 participants