Resolves #1864. Adds Native List as an option for configuring keys. #1880
Conversation
…keys in addition to the trailing comma option already given. Signed-off-by: Lucas Earl <[email protected]>
lucqui
force-pushed
the
adding-native-list-support-for-keys
branch
from
fdb34b7 to
02a8304
Compare
…keys in addition to the trailing comma option already given. Signed-off-by: Lucas Earl <[email protected]>
| case []interface{}: | ||
| result := make([]string, len(v)) | ||
| for i, item := range v { | ||
| result[i] = fmt.Sprintf("%v", item) |
There was a problem hiding this comment.
I would probably also check here whether the value is actually a string (and error out if not).
| } | ||
|
|
||
| // Utility function to handle both string and []string | ||
| func parseKeyField(field interface{}) ([]string, error) { |
There was a problem hiding this comment.
How about providing the field's name to this function? Then it could be inserted into the error messages, which makes it a bit easier to find the source of the problem.
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if cRule.Age != "" { |
There was a problem hiding this comment.
Shouldn't this if now involve ageKeys?
| assert.Nil(t, err) | ||
| assert.Equal(t, k, conf.KeyGroups[0][0].ToString()) | ||
| assert.Nil(t, conf) | ||
| assert.ErrorContains(t, err, "invalid age key configuration: invalid key field type: expected string, []string, or nil, got") |
There was a problem hiding this comment.
Hmm, why is this now an error? The test should still succeed IMO with the same result as before.
| t.Fatal("Expected configuration but got nil") | ||
| } | ||
|
|
||
| assert.True(t, len(conf.KeyGroups) > 0) |
There was a problem hiding this comment.
Shouldn't this be
| assert.True(t, len(conf.KeyGroups) > 0) | |
| assert.True(t, len(conf.KeyGroups) == 1) |
?
| case []string: | ||
| return v, nil | ||
| default: | ||
| return nil, fmt.Errorf("invalid key field type: expected string, []string, or nil, got %T", field) |
There was a problem hiding this comment.
I think you still need to check for nil and not error out if nil is received. nil is a prefectly valid value for a field that wans't specified.
Resolves #1864. Adds Native List as an option for configuring keys in addition to the trailing comma option already given.