Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jsonpb: unmarshalling of custom []byte type #220

Closed
FZambia opened this issue Oct 23, 2016 · 15 comments
Closed

jsonpb: unmarshalling of custom []byte type #220

FZambia opened this issue Oct 23, 2016 · 15 comments

Comments

@FZambia
Copy link

FZambia commented Oct 23, 2016

Hi, I just came across an issue with custom type. I saw current custom type issues but I have not understood is my case different or not.

I have custom Raw type that behaves the same way as json.RawMessage, so it's a new type derived from []byte.

type Raw []byte

My struct that uses this type looks like:

type ClientInfo struct {
    DefaultInfo *github_com_centrifugal_centrifugo_libcentrifugo_raw.Raw `protobuf:"bytes,3,opt,name=DefaultInfo,proto3,customtype=github.com/centrifugal/centrifugo/libcentrifugo/raw.Raw" json:"default_info,omitempty"`
}

I just updated gogoprotobuf library (previously I used proto2, now moving to proto3) and my autogenerated tests start failing.

func TestClientInfoJSON(t *testing.T) {
    seed := time.Now().UnixNano()
    popr := math_rand.New(math_rand.NewSource(seed))
    p := NewPopulatedClientInfo(popr, true)
    marshaler := github_com_gogo_protobuf_jsonpb.Marshaler{}
    jsondata, err := marshaler.MarshalToString(p)
    println(string(jsondata))
    if err != nil {
        t.Fatalf("seed = %d, err = %v", seed, err)
    }
    msg := &ClientInfo{}
    err = github_com_gogo_protobuf_jsonpb.UnmarshalString(jsondata, msg)
    if err != nil {
        t.Fatalf("seed = %d, err = %v", seed, err)
    }
    if !p.Equal(msg) {
        t.Fatalf("seed = %d, %#v !Json Equal %#v", seed, msg, p)
    }
}

Raw message encoded as base64 so jsondata looks like:

{"DefaultInfo":"xLs8dWviSSvoKOKq5o08GBYSZOCHpg7xLUe3tzCtdYdZ8krjOLvP345vsQ=="}

But after unmarshalling from string equality test fails because DefaultInfo not unmarshalled from that base64 string to []byte.

As far as I understand my tests were passing previously because jsonpb did not even try to encode bytes before cd72cb9, right?

Is this issue related to #201 ?

@awalterschulze
Copy link
Member

Do you have an UnmarshalJSON method for your customtype Raw?

@awalterschulze
Copy link
Member

I think later this code was added

            if _, ok := outVal.(interface {
                UnmarshalJSON([]byte) error
            }); ok {
                if err := json.Unmarshal(inputValue, outVal); err != nil {
                    return err
                }
                target.Set(outRef.Elem())
                return nil
            }

To check for a customtype with an underlying []byte type

@FZambia
Copy link
Author

FZambia commented Oct 23, 2016

Do you have an UnmarshalJSON method for your customtype Raw

Here is this Raw type - so yes, it has UnmarshalJSON method and its called.

Btw looks like MarshalJSON never called as default info encoded as base64 after MarshalToString

@FZambia
Copy link
Author

FZambia commented Oct 23, 2016

This line is called when marshalling ClientInfo to string but it does not call my MarshalJSON method. I am not too familiar with reflection, so can't figure out why this happens:)

@awalterschulze
Copy link
Member

Have you tried removing the pointer from Raw for the MarshalJSON method?
As the test Uuid type does here https://github.com/gogo/protobuf/blob/master/test/uuid.go#L86

@awalterschulze
Copy link
Member

So basically you will have

// MarshalJSON returns *r as the JSON encoding of r.
func (r Raw) MarshalJSON() ([]byte, error) {
    return r, nil
}

@FZambia
Copy link
Author

FZambia commented Oct 23, 2016

Yep, I tried this before - and tests were failing.. But what I tried now is removing pointer as you suggested and also changed marshaling to:

    // Default handling defers to the encoding/json library.
    var b []byte
    var err error

    if m, ok := v.Interface().(json.Marshaler); ok {
        b, err = m.MarshalJSON()
    } else {
        b, err = json.Marshal(v.Interface())
    }
    if err != nil {
        return err
    }

    /*
        b, err := json.Marshal(v.Interface())
        if err != nil {
            return err
        }
    */

And now tests pass...

@FZambia
Copy link
Author

FZambia commented Oct 23, 2016

update: sorry, looks like I did a mistake, removing pointer is enough, I will check again)) Give me some time

@awalterschulze
Copy link
Member

Ok great. So is removing the pointer an option for you, i.o.w. can we close the issue?

@FZambia
Copy link
Author

FZambia commented Oct 23, 2016

Yes, sorry for disturbing... Removing pointer is enough, looks like when I tried to do this before I also broke something in another place attempting to find solution.

Actually when I did this type I saw that uuid example had no pointer in MarshalJSON but RawMessage in std lib has. So I decided to write it with pointer.

Thanks!

@FZambia FZambia closed this as completed Oct 23, 2016
@awalterschulze
Copy link
Member

I have to say that this solution is not ideal and causes confusion, but it is what we have. Have you tried using casttype instead of customtype?

It might be possible for you to delete quite a bit of code.

@FZambia
Copy link
Author

FZambia commented Oct 23, 2016

No, even have not known about it before this moment, thanks - will try to play with it.

@awalterschulze
Copy link
Member

I think it works better than customtype.

@FZambia
Copy link
Author

FZambia commented Oct 25, 2016

Hi, just tried to use casttype for one of the fields.

string Channel = 3 [(gogoproto.casttype) = "Channel", (gogoproto.jsontag) = "channel"];

where Channel is:

type Channel string

There is a generated function randStringMessage - it returns string so code does not compile because of wrong type used in NewPopulatedX func:

func randStringMessage(r randyMessage) string {
    v3 := r.Intn(100)
    tmps := make([]rune, v3)
    for i := 0; i < v3; i++ {
        tmps[i] = randUTF8RuneMessage(r)
    }
    return string(tmps)
}

func NewPopulatedMessage(r randyMessage, easy bool) *Message {
    this := &Message{}
    ...
    this.Channel = randStringMessage(r)
    ...
    return this
}

Looks like some addition type casting needed here

this.Channel = Channel(randStringMessage(r))

@awalterschulze
Copy link
Member

This should fix the issue
a9cd0c3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants