Skip to content

Commit 48e74cc

Browse files
committed
Fix ExtractItems duplication with WithAppendKeyFields
Fixed a bug where ExtractItems with WithAppendKeyFields option would duplicate list items when both the item path and subset paths were present in the fieldpath set.
1 parent d3e4dc6 commit 48e74cc

File tree

2 files changed

+98
-11
lines changed

2 files changed

+98
-11
lines changed

typed/remove.go

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -80,22 +80,23 @@ func (w *removingWalker) doList(t *schema.List) (errs ValidationErrors) {
8080
path, _ := fieldpath.MakePath(pe)
8181
// save items on the path when we shouldExtract
8282
// but ignore them when we are removing (i.e. !w.shouldExtract)
83-
if w.toRemove.Has(path) {
84-
if w.shouldExtract {
85-
newItems = append(newItems, removeItemsWithSchema(item, w.toRemove, w.schema, t.ElementType, w.shouldExtract).Unstructured())
86-
} else {
87-
continue
83+
if w.shouldExtract {
84+
if w.toRemove.Has(path) || !w.toRemove.WithPrefix(pe).Empty() {
85+
if !w.toRemove.WithPrefix(pe).Empty() {
86+
// Continue if there are subset paths
87+
item = removeItemsWithSchema(item, w.toRemove.WithPrefix(pe), w.schema, t.ElementType, w.shouldExtract)
88+
}
89+
newItems = append(newItems, item.Unstructured())
8890
}
89-
}
90-
if subset := w.toRemove.WithPrefix(pe); !subset.Empty() {
91-
item = removeItemsWithSchema(item, subset, w.schema, t.ElementType, w.shouldExtract)
9291
} else {
93-
// don't save items not on the path when we shouldExtract.
94-
if w.shouldExtract {
92+
if w.toRemove.Has(path) {
9593
continue
9694
}
95+
if !w.toRemove.WithPrefix(pe).Empty() {
96+
item = removeItemsWithSchema(item, w.toRemove.WithPrefix(pe), w.schema, t.ElementType, w.shouldExtract)
97+
}
98+
newItems = append(newItems, item.Unstructured())
9799
}
98-
newItems = append(newItems, item.Unstructured())
99100
}
100101
if len(newItems) > 0 {
101102
w.out = newItems

typed/remove_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -941,6 +941,92 @@ var extractWithKeysCases = []extractWithKeysTestCase{{
941941
},
942942
},
943943
},
944+
}, {
945+
name: "atomicStructures",
946+
rootTypeName: "myRoot",
947+
schema: typed.YAMLObject(associativeAndAtomicSchema),
948+
triplets: []extractTriplet{
949+
{
950+
// extract from atomic list should return entire list
951+
object: `{"atomicList":["a", "b", "c"]}`,
952+
set: _NS(
953+
_P("atomicList", _V("b")),
954+
),
955+
wantOutput: typed.YAMLObject(`{"atomicList":["a", "b", "c"]}`),
956+
},
957+
{
958+
// extract from atomic map should return entire map
959+
object: `{"atomicMap":{"key1": "value1", "key2": "value2"}}`,
960+
set: _NS(
961+
_P("atomicMap", "key1"),
962+
),
963+
wantOutput: typed.YAMLObject(`{"atomicMap":{"key1": "value1", "key2": "value2"}}`),
964+
},
965+
{
966+
// extract with both atomic and associative structures
967+
object: `{"list":[{"key":"nginx","id":1,"nv":2}], "atomicList":["x", "y"]}`,
968+
set: _NS(
969+
_P("list", _KBF("key", "nginx", "id", 1), "nv"),
970+
_P("atomicList", _V("x")),
971+
),
972+
wantOutput: typed.YAMLObject(`{"list":[{"key":"nginx","id":1, "nv":2}], "atomicList":["x", "y"]}`),
973+
},
974+
},
975+
}, {
976+
name: "compositeKeysExtraction",
977+
rootTypeName: "myRoot",
978+
schema: typed.YAMLObject(associativeAndAtomicSchema),
979+
triplets: []extractTriplet{
980+
{
981+
// extract with composite keys - partial field extraction
982+
object: `{"list":[{"key":"a","id":1,"nv":2,"bv":true},{"key":"a","id":2,"nv":3}]}`,
983+
set: _NS(
984+
_P("list", _KBF("key", "a", "id", 1), "nv"),
985+
),
986+
wantOutput: typed.YAMLObject(`{"list":[{"key":"a","id":1,"nv":2}]}`),
987+
},
988+
{
989+
// This test case specifically catches the bug where WithAppendKeyFields
990+
// would duplicate items when extracting with both item key and field paths
991+
object: `{"list":[{"key":"nginx","id":1,"nv":2,"bv":true},{"key":"apache","id":2,"nv":3}]}`,
992+
set: _NS(
993+
_P("list", _KBF("key", "nginx", "id", 1)),
994+
_P("list", _KBF("key", "nginx", "id", 1), "nv"),
995+
),
996+
wantOutput: typed.YAMLObject(`{"list":[{"key":"nginx","id":1,"nv":2}]}`),
997+
},
998+
{
999+
// extract multiple items with composite keys
1000+
object: `{"list":[{"key":"a","id":1,"nv":2},{"key":"a","id":2,"nv":3},{"key":"b","id":1,"nv":4}]}`,
1001+
set: _NS(
1002+
_P("list", _KBF("key", "a", "id", 1)),
1003+
_P("list", _KBF("key", "b", "id", 1)),
1004+
),
1005+
wantOutput: typed.YAMLObject(`{"list":[{"key":"a","id":1},{"key":"b","id":1}]}`),
1006+
},
1007+
},
1008+
}, {
1009+
name: "nestedListsPartialExtraction",
1010+
rootTypeName: "type",
1011+
schema: typed.YAMLObject(nestedTypesSchema),
1012+
triplets: []extractTriplet{
1013+
{
1014+
// extract single field from nested list
1015+
object: `{"listOfMaps":[{"name":"a","value":{"x":"1","y":"2"}},{"name":"b","value":{"z":"3"}}]}`,
1016+
set: _NS(
1017+
_P("listOfMaps", _KBF("name", "a"), "value", "x"),
1018+
),
1019+
wantOutput: typed.YAMLObject(`{"listOfMaps":[{"name":"a","value":{"x":"1"}}]}`),
1020+
},
1021+
{
1022+
// extract from deeply nested structure
1023+
object: `{"mapOfMapsRecursive":{"a":{"b":{"c":null,"d":{"e":null}}}}}`,
1024+
set: _NS(
1025+
_P("mapOfMapsRecursive", "a", "b", "c"),
1026+
),
1027+
wantOutput: typed.YAMLObject(`{"mapOfMapsRecursive":{"a":{"b":{"c":null}}}}`),
1028+
},
1029+
},
9441030
}}
9451031

9461032
func (tt extractWithKeysTestCase) test(t *testing.T) {

0 commit comments

Comments
 (0)