ugorji / go

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

Canonical and MissingFielder don't work together #354

Closed jim-minter closed 3 years ago

jim-minter commented 3 years ago

AFAICS, the EncodeOptions.Canonical setting doesn't currently apply to structs implementing MissingFielder, and I think it should.

Here's a test case:

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 s struct {
    missingFields
}

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

    var b1 []byte
    for {
        var b2 []byte

        err := codec.NewEncoderBytes(&b2, &h).Encode(&s{
            missingFields: missingFields{
                m: map[string]interface{}{
                    "a": 1,
                    "b": 2,
                },
            },
        })
        if err != nil {
            t.Fatal(err)
        }

        if b1 == nil {
            b1 = b2
        } else {
            if !bytes.Equal(b1, b2) {
                t.Fatalf("bytes differed:\n%s\n%s", string(b1), string(b2))
            }
        }
    }
}

When I run the test case, I see:

--- FAIL: TestCanonicalMissingFielder (0.00s)
    main_test.go:63: bytes differed:
        {"a":1,"b":2}
        {"b":2,"a":1}
FAIL
jim-minter commented 3 years ago

Thanks!

ugorji commented 3 years ago

@jim-minter Thanks to you. Great find, and you clearly made the case for it, and you included a clear test case to reproduce. Made my analysis of it fast, and resolution just as fast.