#621: fixed ID fallback for XML merger (#622)

devonfw · Sep 19, 2024 · 28e8782 · 28e8782
1 parent 74b4cc7
commit 28e8782
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 18 deletions.
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
@@ -9,6 +9,7 @@ Release with new features and bugfixes:
 * https://github.com/devonfw/IDEasy/issues/553[#554]: Missmatch of IDE_ROOT
 * https://github.com/devonfw/IDEasy/issues/556[#556]: ProcessContext should compute PATH on run and not in constructor
 * https://github.com/devonfw/IDEasy/issues/557[#557]: Failed to update tomcat: Cannot find a (Map) Key deserializer for type VersionRange
+* https://github.com/devonfw/IDEasy/issues/621[#621]: Xml merger ID fallback not working
 
 The full list of changes for this release can be found in https://github.com/devonfw/IDEasy/milestone/13?closed=1[milestone 2024.09.002].
 

diff --git a/cli/src/main/java/com/devonfw/tools/ide/merge/xmlmerger/matcher/ElementMatcher.java b/cli/src/main/java/com/devonfw/tools/ide/merge/xmlmerger/matcher/ElementMatcher.java
@@ -53,23 +53,6 @@ public MergeElement matchElement(MergeElement sourceElement, MergeElement target
 
     IdComputer idComputer = this.qNameIdMap.get(qName);
     if (idComputer == null) {
-      if (id.isEmpty()) {
-        // handle case where element has no attribute
-        if (sourceElement.getElementAttributes().isEmpty()) {
-          // use name as id
-          id = sourceElement.getElement().getLocalName();
-        } else {
-          // look for id or name attributes
-          String idAttr = sourceElement.getElement().getAttribute("id");
-          if (idAttr.isEmpty()) {
-            idAttr = sourceElement.getElement().getAttribute("name");
-            if (idAttr.isEmpty()) {
-              throw new IllegalStateException(
-                  "No merge:id value defined for element " + sourceElement.getXPath() + " in document " + sourceElement.getDocumentPath());
-            }
-          }
-        }
-      }
       updateId(qName, id);
       idComputer = this.qNameIdMap.get(qName);
     }

diff --git a/cli/src/main/java/com/devonfw/tools/ide/merge/xmlmerger/model/MergeElement.java b/cli/src/main/java/com/devonfw/tools/ide/merge/xmlmerger/model/MergeElement.java
@@ -79,7 +79,29 @@ public String getMergingStrategy() {
    */
   public String getId() {
 
-    return this.element.getAttributeNS(XmlMerger.MERGE_NS_URI, "id");
+    String id = this.element.getAttributeNS(XmlMerger.MERGE_NS_URI, "id");
+    if (id.isEmpty()) {
+      // handle case where element has no attribute
+      if (getElementAttributes().isEmpty()) {
+        // use name as id
+        id = "name()";
+      } else {
+        // look for id or name attributes
+        String idAttr = this.element.getAttribute("id");
+        if (idAttr.isEmpty()) {
+          idAttr = this.element.getAttribute("name");
+          if (idAttr.isEmpty()) {
+            throw new IllegalStateException(
+                "No merge:id value defined for element " + getXPath() + " in document " + getDocumentPath());
+          } else {
+            id = "@name";
+          }
+        } else {
+          id = "@id";
+        }
+      }
+    }
+    return id;
   }
 
   /**

diff --git a/cli/src/test/java/com/devonfw/tools/ide/merge/XmlMergerTest.java b/cli/src/test/java/com/devonfw/tools/ide/merge/XmlMergerTest.java
@@ -10,13 +10,17 @@
 import org.assertj.core.api.SoftAssertions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.devonfw.tools.ide.context.AbstractIdeContextTest;
 import com.devonfw.tools.ide.context.IdeContext;
 import com.devonfw.tools.ide.merge.xmlmerger.XmlMerger;
 
 class XmlMergerTest extends AbstractIdeContextTest {
 
+  private static Logger LOG = LoggerFactory.getLogger(XmlMergerTest.class);
+
   private static final Path TEST_RESOURCES = Path.of("src", "test", "resources", "xmlmerger");
 
   private static final String SOURCE_XML = "source.xml";
@@ -40,6 +44,7 @@ void testMerger(@TempDir Path tempDir) throws Exception {
       // arrange
       SoftAssertions softly = new SoftAssertions();
       folders.forEach(folder -> {
+        LOG.info("Testing XML merger for test-case {}", folder.getFileName());
         Path sourcePath = folder.resolve(SOURCE_XML);
         Path targetPath = tempDir.resolve(TARGET_XML);
         Path resultPath = folder.resolve(RESULT_XML);

diff --git a/cli/src/test/resources/xmlmerger/id-fallback/result.xml b/cli/src/test/resources/xmlmerger/id-fallback/result.xml
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<root>
+  <element attr1="newValue1" name="name1">New text1</element>
+  <element attr1="oldValue2" name="name2">Old text2</element>
+  <parent attr1="newSomething" id="parent">
+    <child>New Child Text</child>
+  </parent>
+  <parent attr1="oldSomething2" id="parent2">
+    <child>Old Child Text2</child>
+  </parent>
+</root>
diff --git a/cli/src/test/resources/xmlmerger/id-fallback/source.xml b/cli/src/test/resources/xmlmerger/id-fallback/source.xml
@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<root xmlns:merge="https://github.com/devonfw/IDEasy/merge" merge:strategy="COMBINE">
+  <element name="name1" attr1="newValue1">New text1</element>
+  <parent id="parent" attr1="newSomething">
+    <child>New Child Text</child>
+  </parent>
+</root>
diff --git a/cli/src/test/resources/xmlmerger/id-fallback/target.xml b/cli/src/test/resources/xmlmerger/id-fallback/target.xml
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<root>
+  <element attr1="oldValue1" name="name1">Old text1</element>
+  <element attr1="oldValue2" name="name2">Old text2</element>
+  <parent id="parent" attr1="oldSomething">
+    <child>Old Child Text</child>
+  </parent>
+  <parent id="parent2" attr1="oldSomething2">
+    <child>Old Child Text2</child>
+  </parent>
+</root>