jmattheis / goverter

Generate type-safe Go converters by simply defining an interface
https://goverter.jmattheis.de/
MIT License
502 stars 47 forks source link

Question around conversion for deeply embedded/nested structs #115

Closed kellen-miller closed 10 months ago

kellen-miller commented 10 months ago

Have you read the project documentation?

Describe your question Using goverter, what is the proper way to map source/target fields when the target is using multiple levels of embedded structs? I read through the documentation and also cloned the lib and walked through what is happening under the hood in debug mode, but am still having trouble understanding what interface I need to create in order to get the converter impl I'm looking for.

Source Code

Setting up a somewhat contrived example, but this is similar to some code I am looking to convert.

Source

type MyFlatSource struct {
    Foo string
    Bar int
    Baz bool
}

Target

type MyEmbeddedTarget struct {
    EmbeddedFoo
    EmbeddedBar
    EmbeddedBaz
}

type EmbeddedFoo struct {
    AnotherLevelOfFoo
}

type AnotherLevelOfFoo struct {
    FooB []byte
        IgnoreMe string
}

type EmbeddedBar struct {
    Bar int
        IgnoreMeAlso string
}

type EmbeddedBaz struct {
    Baz bool
}

Attempts

First Attempt

From the documentation, I was thinking I could define the converter like this:

package embedded_conv

// goverter:converter
// goverter:ignoreMissing
type Converter interface {
    // goverter:map Foo FooB | StrToBytes
    ToTarget(input MyFlatSource) (MyEmbeddedTarget, error)
}

func StrToBytes(str string) []byte {
    return []byte(str)
}

Which results in an empty converter impl:

// Code generated by github.com/jmattheis/goverter, DO NOT EDIT.

package generated

import embeddedconv "testing/embedded_conv"

type ConverterImpl struct{}

func (c *ConverterImpl) ToTarget(source embeddedconv.MyFlatSource) (embeddedconv.MyEmbeddedTarget, error) {
    var embedded_convMyEmbeddedTarget embeddedconv.MyEmbeddedTarget
    return embedded_convMyEmbeddedTarget, nil
}

Second Attempt

I realized I probably need to set an explicit path to the target field (since embedded structs are really just syntax sugar for this in Go).

package embedded_conv

// goverter:converter
// goverter:ignoreMissing
type Converter interface {
    // goverter:map Foo EmbeddedFoo.AnotherLevelOfFoo.FooB | StrToBytes
    // goverter:map Bar EmbeddedBar.Bar
    // goverter:map Baz EmbeddedBaz.Baz
    ToTarget(input MyFlatSource) (MyEmbeddedTarget, error)
}

func StrToBytes(str string) []byte {
    return []byte(str)
}

That once again results in an empty converter impl.

Third Attempt:

Since the second version didn't work, I tried using the reverse autoMap with the map . * .

package embedded_conv

// goverter:converter
// goverter:ignoreMissing
type Converter interface {
    // goverter:map Foo EmbeddedFoo.AnotherLevelOfFoo.FooB | StrToBytes
    // goverter:map . EmbeddedFoo
    // goverter:map . EmbeddedBar
    // goverter:map . EmbeddedBaz
    ToTarget(input MyFlatSource) (MyEmbeddedTarget, error)
}

That actually results in some code that is correct, but the multi-level embedded Foo struct does not convert correctly.

// Code generated by github.com/jmattheis/goverter, DO NOT EDIT.

package generated

import embeddedconv "testing/embedded_conv"

type ConverterImpl struct{}

func (c *ConverterImpl) ToTarget(source embeddedconv.MyFlatSource) (embeddedconv.MyEmbeddedTarget, error) {
    var embedded_convMyEmbeddedTarget embeddedconv.MyEmbeddedTarget
    embedded_convMyEmbeddedTarget.EmbeddedFoo = c.embedded_convMyFlatSourceToEmbedded_convEmbeddedFoo(source)
    embedded_convMyEmbeddedTarget.EmbeddedBar = c.embedded_convMyFlatSourceToEmbedded_convEmbeddedBar(source)
    embedded_convMyEmbeddedTarget.EmbeddedBaz = c.embedded_convMyFlatSourceToEmbedded_convEmbeddedBaz(source)
    return embedded_convMyEmbeddedTarget, nil
}
func (c *ConverterImpl) embedded_convMyFlatSourceToEmbedded_convEmbeddedBar(source embeddedconv.MyFlatSource) embeddedconv.EmbeddedBar {
    var embedded_convEmbeddedBar embeddedconv.EmbeddedBar
    embedded_convEmbeddedBar.Bar = source.Bar
    return embedded_convEmbeddedBar
}
func (c *ConverterImpl) embedded_convMyFlatSourceToEmbedded_convEmbeddedBaz(source embeddedconv.MyFlatSource) embeddedconv.EmbeddedBaz {
    var embedded_convEmbeddedBaz embeddedconv.EmbeddedBaz
    embedded_convEmbeddedBaz.Baz = source.Baz
    return embedded_convEmbeddedBaz
}
func (c *ConverterImpl) embedded_convMyFlatSourceToEmbedded_convEmbeddedFoo(source embeddedconv.MyFlatSource) embeddedconv.EmbeddedFoo {
    var embedded_convEmbeddedFoo embeddedconv.EmbeddedFoo
    return embedded_convEmbeddedFoo
}

I've tried some other converter mappings to try and get the EmbeddedFoo struct to map correctly, I'm probably missing something. Or if not, I would love to contribute to get it working.

jmattheis commented 10 months ago

The target field of goverter:map cannot be a path, because a single method is currently only able to map a single struct. By goverter:map . TARGET you instruct goverter to convert the complete source object to the target field. With this you can define another method with the source type to the target field type and then define the mapping there further. Currently, you could solve it like this:

// goverter:converter
// goverter:ignoreMissing
type Converter interface {
    // goverter:map . EmbeddedFoo
    // goverter:map . EmbeddedBar
    // goverter:map . EmbeddedBaz
    ToTarget(input MyFlatSource) (MyEmbeddedTarget, error)
    // goverter:map . AnotherLevelOfFoo
    ToTargetFoo(input MyFlatSource) (EmbeddedFoo, error)
    // goverter:map Foo FooB | StrToBytes
    ToTargetAnotherLevel(input MyFlatSource) (AnotherLevelOfFoo, error)
}

I think this version "forces" more reusablility because the conversions of MyFlatSource -> AnotherLeverOfFoo could be reused in another conversion.

There is https://github.com/jmattheis/goverter/issues/80 that proposes the feature to support paths on the target field side. This is sadly not that easy because goverter is required to error when not all fields are mapped. This is much easier to check when you only have to look at the field of the current struct and can ignore embedded structs / properties.

Also I'd recommend to not use ignoreMissing and rather explicitly ignore fields this makes mapping errors less likely.

// goverter:converter
type Converter interface {
    // goverter:map . EmbeddedFoo
    // goverter:map . EmbeddedBar
    // goverter:map . EmbeddedBaz
    ToTarget(input MyFlatSource) (MyEmbeddedTarget, error)
    // goverter:map . AnotherLevelOfFoo
    ToTargetFoo(input MyFlatSource) (EmbeddedFoo, error)
    // goverter:map Foo FooB | StrToBytes
    // goverter:ignore IgnoreMe
    ToTargetAnotherLevel(input MyFlatSource) (AnotherLevelOfFoo, error)
    // goverter:ignore IgnoreMeAlso
    ToTargetBar(input MyFlatSource) (EmbeddedBar, error)
}
kellen-miller commented 10 months ago

Ah I see, that makes sense. Thanks for that help and the quick response, just saved me hours of trial and error/work!

I'm in the middle of writing a protoc plugin to generate goverter Converter interfaces for proto messages that will convert between the generated Go protobuf and a legacy domain model.

I created a custom proto field option for the mapping between the source/target and am using the cast library for the conversion methods between basic types, unless it is a special type conversion I am looking for or a custom one is supplied.

That is actually the reason I am using ignoreMissing, since the converter interface is going to be generated, I am assuming that if it's not defined in the proto it should be ignored. Otherwise I'd have to do a field diff on the target and source, take into the account the custom mappings, etc. Feel like I would be rewriting part of this lib 😄.

jmattheis commented 10 months ago

Sounds reasonable, I'll plan to add guides to the documentation and will probably add a section about converting embedded structs.

kellen-miller commented 9 months ago

If you need any help or an extra maintainer for this lib let me know, would be happy to lend a hand.

jmattheis commented 9 months ago

@kellen-miller Do you have something specific in mind that you're interested in? I think having another pair of eyes on PRs would be helpful.

kellen-miller commented 9 months ago

Not really anything specific, can definitely start helping with PR reviews.

As I'm writing this protoc plugin (which is not at all ready), I'm realizing it might be nice to add into this library. Will open a PR soon with that code to get your opinion.

jmattheis commented 9 months ago

I've added https://goverter.jmattheis.de/guide/embedded-structs to document the embedded struct behavior.

As I'm writing this protoc plugin (which is not at all ready), I'm realizing it might be nice to add into this library. Will open a PR soon with that code to get your opinion.

I'm not sure if I want protobuf specific code inside goverter. Maybe there is something that can be included into goverter if it's generic enough.