Known Issue: customtype that is another protocol buffer message with gogoprotobuf generated functions and methods is not supported

[dennwc]

(Copied from #13)
There is an issue with this combination of options:

message M {
    repeated bytes arr = 1 [(gogoproto.customtype) = "MyType", (gogoproto.nullable) = false];
}

It will generate next code for M:

type MyType struct{}
func (this *MyType) Equal(that interface{}) bool {
    // ...
   that1, ok := that.(*MyType) // auto-generated function assumes pointer to type 
    if !ok {
        return false
    }
    // ...
}

type M struct {
    Arr []MyType `protobuf:"bytes,1,rep,name=arr,customtype=MyType" json:"arr"`
}

func (this *M) Equal(that interface{}) bool {
    // ...
    if !this.Arr[i].Equal(that1.Arr[i]) { // should be &that1.Arr[i]
        return false
    }
    // ...
}

It will fail because Equal method will try to cast a parameter to *MyType. So, the behavior of Equal method is asymmetric - it will try to pass an Arr[i] element, but their auto-generated versions assumes, that parameter will be a pointer to the type.

[awalterschulze]

This should fix your issue
dcff632

Thank you for the idea for the fix :)

[dennwc]

The issue is still there:

• it now assumes non-pointer argument in Equal for custom types for *.pb.go files and a pointer argument for *pb_test.go files (probably the test plugin was not updated)
• it is still asymmetric: assumes a pointer for auto-generated types and non-pointer for custom types

Can we make Equal to have a concrete parameter type instead of empty interface? It will check everything on compile-time at least)

[awalterschulze]

If a type is specified as the parameter an Equaler interface is not possible.

type Equaler interface {
  Equal(interface{}) bool
}

[awalterschulze]

It looks like its working
e492fd3
customtype is supposed to be a type you created yourself and not another protocol buffer.
Obviously it can be, but it you will at least need to edit the Marshal method to not have a pointer receiver.
Also you will need a different NewPopulate method.

[awalterschulze]

To fix this will require a backwards incompatible change.

Could you explain your use case, why must the repeated slice be a customtype and not just the message type?

[dennwc]

We use some custom type for IDs. It can be simplified to this:

// Some interface which is guarantied to be comparable ([20]byte, for example)
type idData interface{}

type ID struct{
  pref string
  id idData
}

We do not want to expose these fields, so the only way to directly use it in PB messages is to define it as a custom type. There is also a custom logic to deconstruct idData type from PB message.

[awalterschulze]

This should be possible?
https://github.com/gogo/protobuf/blob/master/test/custom/custom.go
Or I am missing something ... again

[dennwc]

I haven't stated that custom types are not working, basically :) They are working pretty well. You commit fixed the issue for the except of asymmetry. But if it requires an backward-incompatible change, it's ok to close the issue now.
By the way, there is another issue with jsonpb and same custom type, but I will describe it a bit later, if you don't mind.
And thank you for your time working on this :)

[awalterschulze]

Thank you for reporting.
In hindsight it would have been better to do this in a more standard way.
I think this issue should stay open, since I did not fix it.
It should just stay a known issue.

I look forward to your next report.

[awalterschulze]

Documented a31fa02