ugorji / go

idiomatic codec and rpc lib for msgpack, cbor, json, etc. msgpack.org[Go]
MIT License
1.86k stars 295 forks source link

Further Canonical / MissingFielder issue #355

Closed jim-minter closed 3 years ago

jim-minter commented 3 years ago

Following #354, I think there's still a lurking issue (tested running against codec commit a4afc2bdd282794b23fb6c12de668563a3b9a0d4). With Canonical enabled, I'd expect to see root map keys sorted lexicographically, but they don't appear to be.

package test

import (
    "bytes"
    "testing"

    "github.com/ugorji/go/codec"
)

type missingFields struct {
    m map[string]interface{}
}

func (mf *missingFields) CodecMissingField(field []byte, value interface{}) bool {
    if mf.m == nil {
        mf.m = map[string]interface{}{}
    }

    (mf.m)[string(field)] = value

    return true
}

func (mf *missingFields) CodecMissingFields() map[string]interface{} {
    return mf.m
}

var _ codec.MissingFielder = (*missingFields)(nil)

type s1 struct {
    missingFields
    B int
}

type s2 struct {
    A int
    B int
}

func TestCanonicalMissingFielder2(t *testing.T) {
    h := codec.JsonHandle{
        BasicHandle: codec.BasicHandle{
            EncodeOptions: codec.EncodeOptions{
                Canonical: true,
            },
        },
    }

    var b1 []byte
    err := codec.NewEncoderBytes(&b1, &h).Encode(&s1{
        missingFields: missingFields{
            m: map[string]interface{}{
                "A": 1,
            },
        },
        B: 2,
    })
    if err != nil {
        t.Fatal(err)
    }

    var b2 []byte
    err = codec.NewEncoderBytes(&b2, &h).Encode(&s2{
        A: 1,
        B: 2,
    })
    if err != nil {
        t.Fatal(err)
    }

    if !bytes.Equal(b1, b2) {
        t.Errorf("bytes differ:\n%s\n%s", string(b1), string(b2))
    }
}

I would expect the two byte slices above to be equal, but they are not:

=== RUN   TestCanonicalMissingFielder
    main_test.go:72: bytes differ:
        {"B":2,"A":1}
        {"A":1,"B":2}
--- FAIL: TestCanonicalMissingFielder (0.00s)
ugorji commented 3 years ago

Good catch. I missed this scenario, and initially thought it was sufficient to just sort the missing fields independently. I now collate both missing fields and struct fields together, sort them, and then encode as one. This should resolve the issue. I updated the tests also.

Thanks for diligently following up on it.

jim-minter commented 3 years ago

Thanks again - all looks good now.