Skip to content
New issue

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

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

Already on GitHub? # to your account

Add setting use brackets as a namespace separator #56

Merged

Conversation

alxfv
Copy link
Contributor

@alxfv alxfv commented Jun 10, 2021

Add a new setting that allows using square brackets as a namespace separator.

In a company I work for we use square brackets as a struct separator.

  • [*] Tests exist or have been written that cover this particular change.

@go-playground/admins

@coveralls
Copy link

coveralls commented Jun 24, 2021

Coverage Status

Coverage remained the same at 100.0% when pulling 11d2b17 on alxfv:use-brackets-as-namespace-separator into e56bbb1 on go-playground:master.

@deankarn
Copy link

@alxfv I will have to review carefully, there is a good chance this will conflict with the map and/or array decoding logic which uses square brackets as its separators.

@alxfv
Copy link
Contributor Author

alxfv commented Jun 25, 2021

@alxfv I will have to review carefully, there is a good chance this will conflict with the map and/or array decoding logic which uses square brackets as its separators.

Thanks! I will add more tests to try to find possible conflicts.

@alxfv alxfv force-pushed the use-brackets-as-namespace-separator branch 3 times, most recently from 77cc112 to ad71d56 Compare June 28, 2021 14:52
@alxfv
Copy link
Contributor Author

alxfv commented Jun 28, 2021

Hello again!

Improved some tests:

  1. Use table tests to run tests with and without useBrackest setting.
  2. Added in TestDecoderStruct all cases from TestDecoderMapKeys and haven't found any issues with decoding structs and map with square brackets in the same values.
  3. Added TestTestDecoderStructMapConflict that shows what happens if map key and struct field have the same value:

    form/decoder_test.go

    Lines 537 to 585 in ad71d56

    func TestDecoderEqualStructMapValue(t *testing.T) {
    type PhoneStruct struct {
    Number string
    }
    type PhoneMap map[string]string
    type TestStruct struct {
    PhoneStruct PhoneStruct `form:"Phone"`
    PhoneMap PhoneMap `form:"Phone"`
    }
    testCases := []struct {
    UseBrackets bool
    Values url.Values
    }{{
    UseBrackets: false,
    Values: url.Values{
    "Phone.Number": []string{"111"},
    "Phone[Number]": []string{"222"},
    },
    }, {
    UseBrackets: true,
    Values: url.Values{
    "Phone[Number]": []string{"111"},
    },
    }}
    for _, tc := range testCases {
    tc := tc
    t.Run(fmt.Sprintf("useBrackets_%v", tc.UseBrackets), func(t *testing.T) {
    decoder := NewDecoder()
    decoder.SetUseBrackets(tc.UseBrackets)
    var test TestStruct
    err := decoder.Decode(&test, tc.Values)
    Equal(t, err, nil)
    Equal(t, test.PhoneStruct.Number, "111")
    if tc.UseBrackets == false {
    Equal(t, test.PhoneMap["Number"], "222")
    } else {
    Equal(t, test.PhoneMap["Number"], "111")
    }
    })
    }
    }

"Phone[Number]": []string{"111"}, with usingBrackets = true will be decoded to struct or/and a map and it looks as expected.

Also wanted to tell how this pull request implements this feature. It changes only namespace slice here:

form/decoder.go

Lines 162 to 169 in 91efee7

if d.d.useBrackets {
namespace = append(namespace, '[')
namespace = append(namespace, f.name...)
namespace = append(namespace, ']')
} else {
namespace = append(namespace, namespaceSeparator)
namespace = append(namespace, f.name...)
}

And later on in findAlias we will get Phone[Number] instead of Phone.Number, that's all.

form/decoder.go

Lines 34 to 41 in 91efee7

func (d *decoder) findAlias(ns string) *recursiveData {
for i := 0; i < len(d.dm); i++ {
if d.dm[i].alias == ns {
return d.dm[i]
}
}
return nil
}

@deankarn
Copy link

I think that this will likely be ok then.

I only have one ask to make this more generic for any future requests can the PR be modified to use a struct namespace prefix and struct namespace suffix?

This will help keep the library flexible to future use cases and remove the if logic associated with useBarackets :)

@alxfv
Copy link
Contributor Author

alxfv commented Jun 29, 2021

I think that this will likely be ok then.

I only have one ask to make this more generic for any future requests can the PR be modified to use a struct namespace prefix and struct namespace suffix?

This will help keep the library flexible to future use cases and remove the if logic associated with useBarackets :)

Great idea! Done.

@alxfv alxfv force-pushed the use-brackets-as-namespace-separator branch from ad71d56 to 11d2b17 Compare June 29, 2021 17:49
@deankarn deankarn merged commit 1a71822 into go-playground:master Jul 8, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants