Add json: (or custom) struct tag support for field matching

[kb-sp]

Is your feature request related to a problem? Please describe.

Most of the time, struct field names can be made to match. Sometimes the fields differ in capitalization (e.g., Go's entityID vs JSON's entityId) which can be resolved with case insensitivity.

However, there are cases where field names differ beyond the coder's control, like from one protobuf to multiple slightly different "internal" structs, etc.

Describe the solution you'd like
Support for this could be added but using json: struct tags to inform matching. Ideally, struct fields could be annotated with an additional goverter: tag to inform field mapping, so that existing json: tags could be ignored.

Honoring json: would obviously need to be an opt-in flag, but to me it's debatable whether goverter: tag support could be on by default, or also opt-in.

I'd originally tried to use copygen which has tag support, but in general that project just doesn't work with protobuf or complex structs in the mix. YMMV.

Briefly looking through the goverter code, it does look like adding tag support could be a bit involved. I'd be happy to get advice on the project's preferences on how this would be implemented.

Describe alternatives you've considered

There's no real alternative to true field mismatches, other than creating a ghost field to receive the goverted value, and then add a wrapper that manually copies over these ghost values.

Thanks!

[jmattheis]

However, there are cases where field names differ beyond the coder's control, like from one protobuf to multiple slightly different "internal" structs, etc.

For this goverter has support for defining the mapping on the conversion methods, see map. This methods also allows you to define custom methods for the conversion https://goverter.jmattheis.de/reference/map#map-source-path-target-method.

Support for this could be added but using json: struct tags to inform matching. Ideally, struct fields could be annotated with an additional goverter: tag to inform field mapping, so that existing json: tags could be ignored.

Could you explain why you would prefer defining the conversion names on one of the structs used in the conversion? For me it seems that the conversion methods are the better place. E.g. if you have the following conversion A -> B, A -> C. If the mapping is defined directly on the A struct, then you can only define the mapping for one of them.

There's no real alternative to true field mismatches, other than creating a ghost field to receive the goverted value, and then add a wrapper that manually copies over these ghost values.

Would these "true field mismatches" be fixable with map if not could you give an code example what you mean with this?

Briefly looking through the goverter code, it does look like adding tag support could be a bit involved. I'd be happy to get advice on the project's preferences on how this would be implemented.

I think the change would actually not that big, there is one method responsible for matching the field by names. In there we could add the additional checks for the struct tags.

goverter/xtype/type.go

Line 87 in 48f86d9

func (t Type) findAllFields(path []string, name string, ignoreCase bool) (*StructField, []*StructField) {

---

That said, I do think tag matching can improve the developer experience of goverter. Goverter could support something like this:

// goverter:converter
// goverter:matchIgnoreCase
// goverter:matchTag json
type Converter interface {
	Convert(source []Input) []Output
}

type Input struct {
	Name string `json:"fullName"`
}

type Output struct {
	FullName string
}

the json part would be customizable and you could use yaml or even goverter. If there would be an ambiguity like this

type Input struct {
	Name string `json:"fullName"`
	FullName string
}

type Output struct {
	FullName string
}

Then goverter should fail with an error, but this functionality already exist because of autoMap and matchIgnoreCase so it should be fairly simple to implement.

Would this solve your use-case?

[kb-sp]

For this goverter has support for defining the mapping on the conversion methods, see map. This methods also allows you to define custom methods for the conversion https://goverter.jmattheis.de/reference/map#map-source-path-target-method.

Yes, this would definitely work. But what prompted my thinking on tags is that we have several hundred entities where we would have to apply goverter:map to each method individually, some different than others, which is cumbersome. Especially since the json: tags are already "correct" for our use case.

Could you explain why you would prefer defining the conversion names on one of the structs used in the conversion? For me it seems that the conversion methods are the better place. E.g. if you have the following conversion A -> B, A -> C. If the mapping is defined directly on the A struct, then you can only define the mapping for one of them.

I think because my use case started out with "correct" json tag matching, the goverter:map seems like an extra step, if that makes sense. I don't think the json tag approach is "better" per se, but that given existing json tags used for pre-existing manual marshaling as we've done, we could keep our "source of truth" for mappings as-is and just flip a switch in goverter, if that makes sense.

Would these "true field mismatches" be fixable with map if not could you give an code example what you mean with this?

Yes, totally, one-offs I think are fine. Though slightly awkward because I think you'd need goverter:map source dest and a goverter:map dest source on each of the "directional" copy methods?

I think the change would actually not that big, there is one method responsible for matching the field by names. In there we could add the additional checks for the struct tags.

goverter/xtype/type.go

Line 87 in 48f86d9

func (t Type) findAllFields(path []string, name string, ignoreCase bool) (*StructField, []*StructField) {

Gotcha, I think I wasn't fully grokking that to notice it wasn't just a single name but returned StructField(s).

That said, I do think tag matching can improve the developer experience of goverter. Goverter could support something like this:

// goverter:converter
// goverter:matchIgnoreCase
// goverter:matchTag json
type Converter interface {
	Convert(source []Input) []Output
}

Yep, this makes sense. Would matchIgnoreCase apply to the tags as well as the fields?

Would this solve your use-case?

As you laid out, I agree this would /streamline/ my use case, but not strictly necessary. :)

I'll take a look at this over the next couple of weeks unless someone beats me to it. ;-)

Thanks for the quick response!

[jmattheis]

we could keep our "source of truth" for mappings as-is and just flip a switch in goverter, if that makes sense.

Yeah, in this case I agree. Having tag support in goverter would remove a lot of duplicated mappings there.

I think you'd need goverter:map source dest and a goverter:map dest source on each of the "directional" copy methods?

Yes.

Yep, this makes sense. Would matchIgnoreCase apply to the tags as well as the fields?

Yes.

---

The tags should support this common format tag:"[<key>][,<flag1>[,<flag2>]]", so goverter should only use the first value in a comma separated list.

Do you think it's okay to have tag matching as addition to the normal field name matching? Or should it be explicitly definable, e.g. matchNames tag:json fieldName would only match the json tags of the source types against the actual field name of the target type. I think I rather have this as addition to the existing matching and then the developer has to manually resolve potential ambiguities because it seems easier to configure and implement.

[kb-sp]

Do you think it's okay to have tag matching as addition to the normal field name matching? Or should it be explicitly definable, e.g. matchNames tag:json fieldName would only match the json tags of the source types against the actual field name of the target type. I think I rather have this as addition to the existing matching and then the developer has to manually resolve potential ambiguities because it seems easier to configure and implement.

If I understand you correctly, my first thought was indeed your "in addition" take. I feel like matchNames tag:json fieldName is a lot like the existing map FieldName TheOtherFieldName? It's tag-based, but if I know the tag name I probably know the field name? ;)

Having goverter:matchTag json simply enable tag-sourced names (when defined) as taking precedence over the field name is the least surprising. Obviously it needs to default to off, or it could be very surprising, heh.

Enabling tag processing could also create situations where devs were relying on name matching, but had json tags and get different results. But your suggestion of specifying the tag name (via matchTag) I think gracefully solves for that.

[jmattheis]

I mean it more like a general matching configuration.

// goverter:converter
// goverter:matchIgnoreCase
// goverter:matchNames tag:json fieldName
type Converter interface {
	Convert(source []Input) []Output
}

this would mean that only the json tag is matchend against the fieldName

---

// goverter:converter
// goverter:matchIgnoreCase
// goverter:matchNames tag:json fieldName
// goverter:matchNames fieldName fieldName
type Converter interface {
	Convert(source []Input) []Output
}

this means that both the json tag and fieldname will be matched against the target field. But yeah, I think this is overkill, so i'll say we do it "in addition".

[kb-sp]

Ah, so normal field name matching would happen, except fieldName would be matched by tag==json? There might be a situation where that becomes necessary/useful, it's just that I personally haven't seen a use case, for whatever that's worth. ;)

[morganhein]

I'd like to +1 this request. My issue is that I have a lot of generation in my stack, and due to some unique naming restrictions (due to the domain, even if it doesn't make the most programmatic sense), I have some weird friction points.

One of them is that due to this unique naming restriction, my generated protobuf code has extraneous underscores in the name (due to casing of the source material). This makes it so every field I want to map requires two mapping directives (one for each direction).

My current model that goverter is supporting is over 400 lines (in it's most compact form, this is actual properties/objects that get converted). So now i'm stuck auto-generating my goverter definition file and having it auto-determine correlation based on some known naming conventions. This auto-generated goverter template is over 1500 lines long, mostly full of mapping directives.

This pre-generation workflow could go away if I could just match on the json tags.

In a perfect world i'd make things align, and maybe I will, but that requires forking the protoc-gen-grpc plugin... and i'm not ready to do that. I'd be willing to throw a $50 bounty towards this.

[kb-sp]

I'd like to +1 this request.

See PR #164 @morganhein

[morganhein]

I'd like to +1 this request.

See PR #164 @morganhein

This is great! Lmk when this is merged if you want the bounty or not, and the best way to get it to you! Thanks!