golang / protobuf

Go support for Google's protocol buffers
BSD 3-Clause "New" or "Revised" License
9.64k stars 1.58k forks source link

Reading value from proto struct via protoreflect for optional fields #1572

Closed honzap closed 7 months ago

honzap commented 8 months ago

Hello,

I have more over a question about the behavior of protoreflect.Value for optional fields in proto message.

Let's have a simple message:

message Foo {
  optional string param = 1;
}

This one generates Go code struct like this:

type Foo struct {
    state         protoimpl.MessageState
    sizeCache     protoimpl.SizeCache
    unknownFields protoimpl.UnknownFields

    Param *string `protobuf:"bytes,1,opt,name=param,proto3,oneof" json:"param,omitempty"`
}

The Param here is nillable as expected.

When I try to read the value of that field via protoreflect, I'm basically not able to get the fact that the Param is nil. I'd expect that Value.IsValid() returns false because it can operate with nilType.

Let's have this test snippet:

foo := &Foo{}
descriptor := foo.ProtoReflect().Descriptor()
paramField := descriptor.Fields().ByTextName("param")

value := foo.ProtoReflect().Get(paramField)

assert.Equal(t, false, value.IsValid())
assert.Equal(t, nil, value.Interface())

I'd expect that this test will pass but IsValid returns true and calling Value.Interface() returns empty string "" (probably a default value of that field).

My goal is to know that the value is nil for the field Param. I achieved the result by this code, but I'm not sure it's the correct one:

isNil := !foo.ProtoReflect().Has(paramField) && paramField.HasOptionalKeyword()
assert.Equal(t, true, isNil)

Could you please explain me this behavior and why there is nilType not used in this case?

Thanks a lot.

Best,

Honza

neild commented 8 months ago

protoreflect.Message.Get behaves mostly equivalently to accessor methods, which return the default value for unpopulated scalar fields. For example, in your case foo.GetField() returns "" if the field field is unset.

You can test to see if a field is set with foo.ProtoReflect().Has(paramField).

The protoreflect package operates on the protobuf data model, not the Go data model, so it doesn't have a concept of a field being nil. (For example, in generated messages, the representation of an unset optional string field is (*string)(nil), but a dynamicpb.Message uses a different representation. protoreflect can work with either.)

honzap commented 7 months ago

Thanks for explanation. So my line of code getting to know isNil is basically correct, right?

For me, the curious thing is that protoreflect.Value inside works with nilType but basically, this can never happen, right?

puellanivis commented 7 months ago

The internal implementation of protoreflect.Value may need to consider things like a value being nil as it is sort of the “glue” between the protobuf data model, and the Go data model. But it only surfaces strictly the protobuf data model, and the internal Go data model is strictly an obscured implementation detail.

honzap commented 7 months ago

Thank you for clarification. I'm closing this topic now.