qntfy / kazaam

Arbitrary transformations of JSON in Golang
MIT License
282 stars 54 forks source link

`Default` overwrites existing value #71

Closed JeanMertz closed 2 years ago

JeanMertz commented 6 years ago

The transformer name default to me implies that the key is set to the value only if no value is already set. Instead, as far as I can tell, right now it simply means "always set this value, no matter what".

I'm writing my own transformer to override this behaviour, but I figured I'd report this issue, in case you agree with me that the current behaviour is unexpected, based on the name.

JeanMertz commented 6 years ago

FYI, here's the transformer I wrote, in case this is helpful to someone (this depends on https://github.com/qntfy/kazaam/pull/72):

func transformSetIfMissing(spec *transform.Config, data []byte) ([]byte, error) {
    for k, v := range *spec.Spec {
        var err error

        // If no error is returned, it means the path was found, and we return the
        // existing value.
        if _, err = transform.GetJSONRaw(data, k, true); err == nil {
            return data, nil
        }

        // If any error other than `NonExistentPath` is returned, something went
        // wrong and we return the error.
        if err != nil && err != transform.NonExistentPath {
            return nil, err
        }

        b, err := json.Marshal(v)
        if err != nil {
            return nil, transform.ParseError(fmt.Sprintf("Warn: Unable to coerce element to json string: %v", v))
        }

        data, err = transform.SetJSONRaw(data, b, k)
        if err != nil {
            return nil, err
        }
    }

    return data, nil
}

and the tests:

func TestTransformSetIfMissing(t *testing.T) {
    t.Parallel()

    tests := map[string]struct {
        spec map[string]interface{}
        in   string
        out  string
    }{
        "set default": {
            map[string]interface{}{"hello": "world"},
            `{ "hi": "universe" }`,
            `{ "hi": "universe", "hello": "world" }`,
        },

        "ignore if exists": {
            map[string]interface{}{"hi": "world"},
            `{ "hi": "universe" }`,
            `{ "hi": "universe" }`,
        },

        "set nested default": {
            map[string]interface{}{"hi.world": false},
            `{ "hi": { "universe": true } }`,
            `{ "hi": { "universe": true, "world": false } }`,
        },

        "complex structure": {
            map[string]interface{}{"hi": map[string]interface{}{"my": "world"}},
            `{ "hello": "universe" }`,
            `{ "hello": "universe", "hi": { "my": "world" } }`,
        },
    }

    for name, tt := range tests {
        t.Run(name, func(t *testing.T) {
            cfg := &transform.Config{Spec: &tt.spec}

            b, err := transformSetIfMissing(cfg, []byte(tt.in))
            require.NoError(t, err)

            assert.JSONEq(t, tt.out, string(b))
        })
    }
}
ryanleary commented 6 years ago

Thanks for the report!

thomas-tharp commented 2 years ago

Closing as this is the documented behavior of the default transformer