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

Default overwrites existing value #71

Closed
JeanMertz opened this issue Mar 17, 2018 · 3 comments
Closed

Default overwrites existing value #71

JeanMertz opened this issue Mar 17, 2018 · 3 comments

Comments

@JeanMertz
Copy link

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
Copy link
Author

FYI, here's the transformer I wrote, in case this is helpful to someone (this depends on #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
Copy link
Contributor

Thanks for the report!

@thomas-tharp
Copy link

Closing as this is the documented behavior of the default transformer

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

3 participants