Skip to content

Commit a9e710b

Browse files
BerSomBenCopilotstefanseifert
authored
add support for merging maps with key property (#149)
Co-authored-by: Copilot <[email protected]> Co-authored-by: Stefan Seifert <[email protected]>
1 parent 4b51ecf commit a9e710b

File tree

4 files changed

+199
-11
lines changed

4 files changed

+199
-11
lines changed

‎changes.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@
2424
xsi:schemaLocation="http://maven.apache.org/changes/2.0.0 https://maven.apache.org/xsd/changes-2.0.0.xsd">
2525
<body>
2626

27+
<release version="1.19.0" date="not released">
28+
<action type="add" dev="bsommerfeld" issue="149">
29+
Support deep merge of list entries with 'key' field: When merging lists with map objects containing a 'key' field, entries with the same key are now merged instead of replaced, preserving fields from base configuration that are not overridden in variants.
30+
</action>
31+
</release>
32+
2733
<release version="1.18.0" date="2025-09-25">
2834
<action type="update" dev="sseifert">
2935
Switch to Java 17.

‎model/src/main/java/io/wcm/devops/conga/model/util/MergingList.java‎

Lines changed: 67 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
*/
2020
package io.wcm.devops.conga.model.util;
2121

22+
import java.util.HashMap;
2223
import java.util.LinkedList;
24+
import java.util.Map;
2325

2426
/**
2527
* Special list that marks a list as "mergeable" in downstream and preservers the merge position.
@@ -52,7 +54,7 @@ void addCheckMergeToken(T item) {
5254
}
5355
}
5456
else {
55-
this.addIgnoreDuplicates(item);
57+
this.addIgnoreDuplicates(Integer.MAX_VALUE, item);
5658
}
5759
}
5860

@@ -73,19 +75,19 @@ public boolean add(T item) {
7375
return false;
7476
}
7577
else {
76-
return this.addIgnoreDuplicates(item);
78+
return this.addIgnoreDuplicates(Integer.MAX_VALUE, item);
7779
}
7880
}
7981

80-
private boolean addIgnoreDuplicates(T item) {
81-
if (!this.contains(item)) {
82-
super.add(item);
83-
return true;
84-
}
85-
return false;
86-
}
87-
8882
private boolean addIgnoreDuplicates(int index, T item) {
83+
// Check for key-based duplicate in case of Map objects with "key" field
84+
int existingIndex = findIndexByKey(item);
85+
if (existingIndex >= 0) {
86+
// Item with same key already exists - merge the maps
87+
mergeItemAtIndex(existingIndex, item);
88+
return false;
89+
}
90+
// Standard duplicate check
8991
if (!this.contains(item)) {
9092
if (index > this.size() - 1) {
9193
super.add(item);
@@ -98,6 +100,61 @@ private boolean addIgnoreDuplicates(int index, T item) {
98100
return false;
99101
}
100102

103+
/**
104+
* Merges a new item into an existing item at the given index.
105+
* For Map objects, performs a deep merge where the existing item takes precedence.
106+
* This is because the existing item is typically from a variant (higher priority),
107+
* and the new item is from a base config (lower priority).
108+
* @param index Index of existing item
109+
* @param newItem New item to merge in
110+
*/
111+
@SuppressWarnings("unchecked")
112+
private void mergeItemAtIndex(int index, T newItem) {
113+
T existingItem = this.get(index);
114+
if (existingItem instanceof Map && newItem instanceof Map) {
115+
Map<Object, Object> existingMap = (Map<Object, Object>)existingItem;
116+
Map<?, ?> newMap = (Map<?, ?>)newItem;
117+
118+
// Merge: Start with new (base), then override with existing (variant)
119+
// This preserves fields from base that are not in variant
120+
Map<Object, Object> mergedMap = new HashMap<>(newMap);
121+
mergedMap.putAll(existingMap);
122+
123+
super.set(index, (T)mergedMap);
124+
}
125+
// For non-Map objects, keep the existing one
126+
// (do nothing, existing item has higher priority)
127+
}
128+
129+
/**
130+
* Finds an existing item in the list that has the same "key" value as the given item.
131+
* This applies only to Map objects that contain a "key" field.
132+
* @param item Item to check
133+
* @return Index of existing item with same key, or -1 if not found
134+
*/
135+
private int findIndexByKey(T item) {
136+
if (!(item instanceof Map)) {
137+
return -1;
138+
}
139+
Map<?, ?> itemMap = (Map<?, ?>)item;
140+
Object itemKey = itemMap.get("key");
141+
if (itemKey == null) {
142+
return -1;
143+
}
144+
145+
for (int i = 0; i < this.size(); i++) {
146+
T existing = this.get(i);
147+
if (existing instanceof Map) {
148+
Map<?, ?> existingMap = (Map<?, ?>)existing;
149+
Object existingKey = existingMap.get("key");
150+
if (itemKey.equals(existingKey)) {
151+
return i;
152+
}
153+
}
154+
}
155+
return -1;
156+
}
157+
101158
/**
102159
* @return true if list has a merge position
103160
*/

‎model/src/test/java/io/wcm/devops/conga/model/util/MapMergerTest.java‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,131 @@ void testMergeList_EliminateDuplicates_Map() {
265265
map("k1", list(map("p2", "v2"), map("p3", "v3")))));
266266
}
267267

268+
@Test
269+
@SuppressWarnings("unchecked")
270+
void testMergeList_KeyValueObjects_Override() {
271+
// Test case for key-value objects: objects with same "key" should merge
272+
Map<String, Object> baseConfig = map(
273+
"key", "magnolia.bootstrap.dir",
274+
"value", "WEB-INF/bootstrap/author WEB-INF/bootstrap/common",
275+
"comment", "the directories in which the bootstrap files are searched"
276+
);
277+
278+
Map<String, Object> variantConfig = map(
279+
"key", "magnolia.bootstrap.dir",
280+
"value", "C:/user/test/dir"
281+
);
282+
283+
// When merging with _merge_ token, variant values override base values, but base fields are preserved
284+
Map<String, Object> result = merge(
285+
map("proj.magnolia.cms.properties", list(LIST_MERGE_ENTRY, variantConfig)),
286+
map("proj.magnolia.cms.properties", list(baseConfig))
287+
);
288+
289+
// Should have only one entry with merged values: value from variant, comment from base
290+
List<Object> resultList = (List<Object>)result.get("proj.magnolia.cms.properties");
291+
assertEquals(1, resultList.size());
292+
293+
Map<String, Object> expectedMerged = map(
294+
"key", "magnolia.bootstrap.dir",
295+
"value", "C:/user/test/dir",
296+
"comment", "the directories in which the bootstrap files are searched"
297+
);
298+
assertEquals(expectedMerged, resultList.get(0));
299+
}
300+
301+
@Test
302+
@SuppressWarnings("unchecked")
303+
void testMergeList_KeyValueObjects_OverrideComment() {
304+
// Test that comment can be explicitly overridden if provided in variant
305+
Map<String, Object> baseConfig = map(
306+
"key", "magnolia.bootstrap.dir",
307+
"value", "WEB-INF/bootstrap/author WEB-INF/bootstrap/common",
308+
"comment", "old comment"
309+
);
310+
311+
Map<String, Object> variantConfig = map(
312+
"key", "magnolia.bootstrap.dir",
313+
"value", "woanders",
314+
"comment", "new comment"
315+
);
316+
317+
Map<String, Object> result = merge(
318+
map("props", list(LIST_MERGE_ENTRY, variantConfig)),
319+
map("props", list(baseConfig))
320+
);
321+
322+
// Comment should be overridden because it was explicitly provided in variant
323+
List<Object> resultList = (List<Object>)result.get("props");
324+
assertEquals(1, resultList.size());
325+
assertEquals(variantConfig, resultList.get(0));
326+
}
327+
328+
@Test
329+
@SuppressWarnings("unchecked")
330+
void testMergeList_KeyValueObjects_MultipleKeys() {
331+
// Test with multiple different keys - they should all be preserved
332+
Map<String, Object> config1 = map(
333+
"key", "prop1",
334+
"value", "value1"
335+
);
336+
337+
Map<String, Object> config2 = map(
338+
"key", "prop2",
339+
"value", "value2"
340+
);
341+
342+
Map<String, Object> config3 = map(
343+
"key", "prop3",
344+
"value", "value3"
345+
);
346+
347+
Map<String, Object> result = merge(
348+
map("props", list(LIST_MERGE_ENTRY, config3)),
349+
map("props", list(config1, config2))
350+
);
351+
352+
// Should have all three entries
353+
// When LIST_MERGE_ENTRY is at start of l1, l2 elements are inserted at the beginning
354+
// Result: [config1, config2, config3]
355+
List<Object> resultList = (List<Object>)result.get("props");
356+
assertEquals(3, resultList.size());
357+
assertEquals(config1, resultList.get(0));
358+
assertEquals(config2, resultList.get(1));
359+
assertEquals(config3, resultList.get(2));
360+
}
361+
362+
@Test
363+
@SuppressWarnings("unchecked")
364+
void testMergeList_KeyValueObjects_MixedOverrideAndNew() {
365+
// Test with mix of overriding and new keys
366+
Map<String, Object> config1 = map("key", "prop1", "value", "value1", "comment", "comment1");
367+
Map<String, Object> config2 = map("key", "prop2", "value", "value2", "comment", "comment2");
368+
Map<String, Object> config2Override = map("key", "prop2", "value", "value2-override");
369+
Map<String, Object> config3 = map("key", "prop3", "value", "value3");
370+
371+
Map<String, Object> result = merge(
372+
map("props", list(LIST_MERGE_ENTRY, config2Override, config3)),
373+
map("props", list(config1, config2))
374+
);
375+
376+
// l1 = [LIST_MERGE_ENTRY, config2Override, config3] -> MergingList = [config2Override, config3] with mergePos=0
377+
// l2 = [config1, config2]
378+
// When adding l2:
379+
// - config1 is inserted at position 0 -> [config1, config2Override, config3]
380+
// - config2 is merged into config2Override, preserving comment from config2
381+
// Result: [config1, merged(config2, config2Override), config3]
382+
List<Object> resultList = (List<Object>)result.get("props");
383+
assertEquals(3, resultList.size());
384+
assertEquals(config1, resultList.get(0));
385+
386+
// config2Override should have merged with config2, keeping comment from config2
387+
Map<String, Object> expectedConfig2Merged = map("key", "prop2", "value", "value2-override", "comment", "comment2");
388+
assertEquals(expectedConfig2Merged, resultList.get(1));
389+
390+
assertEquals(config3, resultList.get(2));
391+
}
392+
268393
private static List<Object> list(Object... items) {
269394
return Arrays.asList(items);
270395
}

‎src/site/markdown/yaml-definitions.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ Inheritance order (higher number has higher precedence):
105105
7. Variant configuration from node
106106
8. Configuration from multiply plugins, e.g. the tenant-specific configuration
107107

108-
There is a special support when merging list parameter. By default a list value on a deeper lever overwrites a list inherited from a parameter map on a higher level completely. If you insert the keyword `_merge_` as list item on either of the list values, they are merged and the special keyword entry is removed.
108+
There is a special support when merging list parameter. By default a list value on a deeper lever overwrites a list inherited from a parameter map on a higher level completely. If you insert the keyword `_merge_` as list item on either of the list values, they are merged and the special keyword entry is removed. If a merged list contains child objects with `key` properties, merging logic checks for the keys and merges the list as key-value list.
109109

110110

111111
### Variable references

0 commit comments

Comments
 (0)