Skip to content

Commit 9c093d1

Browse files
committed
fix: RemoveItems should preserve empty list and map
Signed-off-by: Peter Jiang <peterjiang823@gmail.com>
1 parent b88846d commit 9c093d1

File tree

2 files changed

+36
-6
lines changed

2 files changed

+36
-6
lines changed

typed/remove.go

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ func (w *removingWalker) doList(t *schema.List) (errs ValidationErrors) {
7575
}
7676

7777
var newItems []interface{}
78+
hadMatches := false
7879
iter := l.RangeUsing(w.allocator)
7980
defer w.allocator.Free(iter)
8081
for iter.Next() {
@@ -98,12 +99,26 @@ func (w *removingWalker) doList(t *schema.List) (errs ValidationErrors) {
9899
continue
99100
}
100101
if isPrefixMatch {
102+
// Removing nested items WITHIN this list item - preserve if it becomes empty
103+
hadMatches = true
104+
wasMap := item.IsMap()
105+
wasList := item.IsList()
101106
item = removeItemsWithSchema(item, w.toRemove.WithPrefix(pe), w.schema, t.ElementType, w.shouldExtract)
107+
// If recursive call returned null but we're removing items within (not the item itself),
108+
// preserve the empty container structure
109+
if item.IsNull() && !w.shouldExtract {
110+
if wasMap {
111+
item = value.NewValueInterface(map[string]interface{}{})
112+
} else if wasList {
113+
item = value.NewValueInterface([]interface{}{})
114+
}
115+
}
102116
}
103117
newItems = append(newItems, item.Unstructured())
104118
}
105119
}
106-
if len(newItems) > 0 {
120+
// Preserve empty lists (non-nil) instead of converting to null when items were matched and removed
121+
if len(newItems) > 0 || (hadMatches && !w.shouldExtract) {
107122
w.out = newItems
108123
}
109124
return nil
@@ -141,6 +156,7 @@ func (w *removingWalker) doMap(t *schema.Map) ValidationErrors {
141156
}
142157

143158
newMap := map[string]interface{}{}
159+
hadMatches := false
144160
m.Iterate(func(k string, val value.Value) bool {
145161
pe := fieldpath.PathElement{FieldName: &k}
146162
path, _ := fieldpath.MakePath(pe)
@@ -151,14 +167,27 @@ func (w *removingWalker) doMap(t *schema.Map) ValidationErrors {
151167
// save values on the path when we shouldExtract
152168
// but ignore them when we are removing (i.e. !w.shouldExtract)
153169
if w.toRemove.Has(path) {
170+
// Exact match: removing this field itself, not items within it
154171
if w.shouldExtract {
155172
newMap[k] = removeItemsWithSchema(val, w.toRemove, w.schema, fieldType, w.shouldExtract).Unstructured()
156173

157174
}
158175
return true
159176
}
160177
if subset := w.toRemove.WithPrefix(pe); !subset.Empty() {
178+
hadMatches = true
179+
wasMap := val.IsMap()
180+
wasList := val.IsList()
161181
val = removeItemsWithSchema(val, subset, w.schema, fieldType, w.shouldExtract)
182+
// If recursive call returned null but we're removing items within (not the field itself),
183+
// preserve the empty container structure
184+
if val.IsNull() && !w.shouldExtract {
185+
if wasMap {
186+
val = value.NewValueInterface(map[string]interface{}{})
187+
} else if wasList {
188+
val = value.NewValueInterface([]interface{}{})
189+
}
190+
}
162191
} else {
163192
// don't save values not on the path when we shouldExtract.
164193
if w.shouldExtract {
@@ -168,7 +197,8 @@ func (w *removingWalker) doMap(t *schema.Map) ValidationErrors {
168197
newMap[k] = val.Unstructured()
169198
return true
170199
})
171-
if len(newMap) > 0 {
200+
// Preserve empty maps (non-nil) instead of converting to null when items were matched and removed
201+
if len(newMap) > 0 || (hadMatches && !w.shouldExtract) {
172202
w.out = newMap
173203
}
174204
return nil

typed/remove_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ var removeCases = []removeTestCase{{
281281
quadruplets: []removeQuadruplet{{
282282
`{"setBool":[false]}`,
283283
_NS(_P("setBool", _V(false))),
284-
`{"setBool":null}`,
284+
`{"setBool":[]}`,
285285
`{"setBool":[false]}`,
286286
}, {
287287
`{"setBool":[false]}`,
@@ -671,23 +671,23 @@ var removeCases = []removeTestCase{{
671671
_NS(
672672
_P("mapOfMapsRecursive", "a"),
673673
),
674-
`{"mapOfMapsRecursive"}`,
674+
`{"mapOfMapsRecursive":{}}`,
675675
`{"mapOfMapsRecursive": {"a":null}}`,
676676
}, {
677677
// second-level map
678678
`{"mapOfMapsRecursive": {"a":{"b":{"c":null}}}}`,
679679
_NS(
680680
_P("mapOfMapsRecursive", "a", "b"),
681681
),
682-
`{"mapOfMapsRecursive":{"a":null}}`,
682+
`{"mapOfMapsRecursive":{"a":{}}}`,
683683
`{"mapOfMapsRecursive": {"a":{"b":null}}}`,
684684
}, {
685685
// third-level map
686686
`{"mapOfMapsRecursive": {"a":{"b":{"c":null}}}}`,
687687
_NS(
688688
_P("mapOfMapsRecursive", "a", "b", "c"),
689689
),
690-
`{"mapOfMapsRecursive":{"a":{"b":null}}}`,
690+
`{"mapOfMapsRecursive":{"a":{"b":{}}}}`,
691691
`{"mapOfMapsRecursive": {"a":{"b":{"c":null}}}}`,
692692
}, {
693693
// empty list

0 commit comments

Comments
 (0)