golang / protobuf

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

marshall/unmarshal time.Time to *timestamppb.Timestamp fails #1593

Closed go-aegian closed 4 months ago

go-aegian commented 4 months ago

Here is an example of what I'm facing as problem, some others have said to use protojson to unmarshall into a proto message but that is not what I need.

How can this be resolved?

Any way to have timestamppb.Timestamp to be able to unmarshall from a marshalled date/time string?


package main

import (
        "encoding/json"
    "fmt"
    "time"

    "google.golang.org/protobuf/runtime/protoimpl"
    "google.golang.org/protobuf/types/known/timestamppb"
)

type Test struct {
    Id        string    `json:"id,omitempty"`
    ExpiresOn time.Time `json:"expiresOn,omitempty"`
}

type TestPB struct {
    state         protoimpl.MessageState
    sizeCache     protoimpl.SizeCache
    unknownFields protoimpl.UnknownFields
    Id            string                 `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"`
    ExpiresOn     *timestamppb.Timestamp `protobuf:"bytes,2,opt,name=expiresOn,proto3" json:"expiresOn,omitempty"`
}

func main() {
    u := Test{Id: "123", ExpiresOn: time.Now().UTC()}

    b, err := json.Marshal(u)
    if err != nil {
        panic(err)
    }

    var m TestPB
    err = json.Unmarshal(b, &m)
    if err != nil {
        panic(err)
    }

    fmt.Printf("PB is %v", m)
}
puellanivis commented 4 months ago

In order to parse JSON encoded according to the standardized proto3 JSON mapping, you have to use protojson. There is no option. The existing standard encoding/json does not allow all the features necessary to be compliant with the standard, and so we only support that encoding via the newer protojson package.

dsnet commented 4 months ago

If the v2 JSON design eventually gets merged into the stdlib, you would be able to do something like:

jsonv2.Marshal(v, jsonv2.MarshalFuncV1(protojson.Marshal))

which would allow you to handled mixed data structures that include both protobuf messages and non-protobuf Go types.

aktau commented 4 months ago

Attempting to unmarshal a byte stream that was marshaled by (stdlib) package json into a protobuf isn't guaranteed to work, as @puellanivis said. You should use protojson all the way if you have proto message types (type TestPB in your example), or you should use json all the way if you have JSON-annotated structs (type Test in your example).

It's not clear to me how you're running into the case where you have both type Test and type TestPB. What is generating these two messages?

go-aegian commented 4 months ago

@aktau look at annotations in TestPB has both json and protobuf for a field thus I don't see why it should be handled "all the way one way or another" as mutually exclusive. Both proto and json should be able to handle the datatype although the problem was some way introduced by a datatype as struct from protobuf, not carefully thought as solution when data mapping this kind of structs

aktau commented 4 months ago

Yes, TestPB has annotations for both. I assume this stems from before the time when protojson existed, and the need for some backwards compatibility there. Regardless of all that, there is no guaranteed interoperability between stdlin package encoding/json and protojson. With proto structs, such as your TestPB, please use protojson.

Do you have a problem if you only use package protojson?

go-aegian commented 4 months ago

I do and I think it would be good if protojson can unmarshall a string datetime into a timetamp format and fail only if the date/time string is blank/nil/invalid

aktau commented 4 months ago

I'm still not sure what exactly you'd like, in case it is supported. And I'll note again that we do not support unmarshaling encoding/json encoded json values with protojson. Nor do we support unmarshaling into proto types with anything except for the proto, protojson or prototext packages (I believe that's all).

go-aegian commented 4 months ago

@aktau, thanks for the support and info provided. Closing this question then.