Skip to content

Commit

Permalink
fix(utils): do not fill empty records
Browse files Browse the repository at this point in the history
follow up of: #466, prevent unset
records to be filled with empty tables: {}
  • Loading branch information
samugi committed Sep 11, 2024
1 parent 76af65d commit ceae801
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 1 deletion.
7 changes: 6 additions & 1 deletion kong/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,12 @@ func fillConfigRecord(schema gjson.Result, config Configuration, opts FillRecord
fieldConfig = subConfig.(map[string]interface{})
}
newSubConfig := fillConfigRecord(value.Get(fname), fieldConfig, opts)
res[fname] = map[string]interface{}(newSubConfig)
// When we are not filling defaults, only assign the subconfig if it's not empty.
// This is to avoid having records that are assigned empty map values when defaults
// are not supposed to be filled.
if opts.FillDefaults || len(newSubConfig) > 0 {
res[fname] = map[string]interface{}(newSubConfig)
}
return true
}

Expand Down
56 changes: 56 additions & 0 deletions kong/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1971,6 +1971,48 @@ const fillConfigRecordTestSchemaWithAutoFields = `{
}
`

const fillConfigRecordTestSchemaWithRecord = `{
"fields": {
"config": {
"type": "record",
"fields": [
{
"some_record": {
"required": true,
"fields": [
{
"some_field": {
"default": "kong",
"type": "string"
}
}
],
"type": "record"
}
},
{
"some_other_record": {
"fields": [
{
"some_field": {
"type": "string"
}
}
],
"type": "record"
}
},
{
"string_1": {
"type": "string",
}
}
]
}
}
}
`

func Test_fillConfigRecord_shorthand_fields(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -2098,6 +2140,20 @@ func Test_fillConfigRecord_auto_only(t *testing.T) {
// defalt_string missing
},
},
{
name: "not passing record field leaves field unset",
schema: gjson.Parse(fillConfigRecordTestSchemaWithRecord),
config: Configuration{
// some_record missing
"some_other_record": map[string]any{}, // explicitly set to empty record
"string_1": "abc",
},
expected: Configuration{
// some_record was not filled
"some_other_record": map[string]any{}, // empty record remained unchanged
"string_1": "abc",
},
},
}

for _, tc := range tests {
Expand Down

0 comments on commit ceae801

Please sign in to comment.