(fix) Element was inserted at the wrong location (#114)

* (fix) Element was inserted at the wrong location

* Add minor enhancements

---------

Co-authored-by: George Gastaldi <[email protected]>

Author: murdos

src/main/java/io/fabric8/maven/MavenJDOMWriter.java

@@ -80,12 +80,12 @@ class MavenJDOMWriter {
     /**
      * Field factory.
      */
-    private DefaultJDOMFactory factory;
+    private final DefaultJDOMFactory factory;
 
     /**
      * Field lineSeparator.
      */
-    private String lineSeparator;
+    private final String lineSeparator;
 
     // ----------------/
     // - Constructors -/
@@ -175,7 +175,7 @@ protected Element findAndReplaceSimpleElement(Counter counter, Element parent, S
             }
         }
 
-        boolean shouldExist = (text != null) && (text.trim().length() > 0);
+        boolean shouldExist = (text != null) && (!text.trim().isEmpty());
         Element element = updateElement(counter, parent, name, shouldExist);
         if (shouldExist) {
             element.setText(text);
@@ -262,7 +262,7 @@ protected void insertAtPreferredLocation(Element parent, Element child, Counter
         Iterator it = parent.getContent().iterator();
         Text lastText = null;
         int offset = 0;
-        while (it.hasNext() && (elementCounter <= counter.getCurrentIndex())) {
+        while (it.hasNext() && (elementCounter < counter.getCurrentIndex())) {
             Object next = it.next();
             offset = offset + 1;
             if (next instanceof Element) {
@@ -274,17 +274,13 @@ protected void insertAtPreferredLocation(Element parent, Element child, Counter
                 lastText = (Text) next;
             }
         }
-        if ((lastText != null) && (lastText.getTextTrim().length() == 0)) {
-            lastText = (Text) lastText.clone();
+        if ((lastText != null) && (lastText.getTextTrim().isEmpty())) {
+            lastText = lastText.clone();
         } else {
-            StringBuilder starter = new StringBuilder(lineSeparator);
-            for (int i = 0; i < counter.getDepth(); i++) {
-                starter.append("  ");
-            }
-            lastText = factory.text(starter.toString());
+            lastText = factory.text(lineSeparator + "  ".repeat(Math.max(0, counter.getDepth())));
         }
         if (parent.getContentSize() == 0) {
-            Text finalText = (Text) lastText.clone();
+            Text finalText = lastText.clone();
             finalText.setText(finalText.getText().substring(0, finalText.getText().length() - "  ".length()));
             parent.addContent(contentIndex, finalText);
         }

src/test/java/io/fabric8/maven/MavenTest.java

@@ -13,6 +13,7 @@
 import java.util.Collections;
 import java.util.Properties;
 
+import org.apache.maven.model.Dependency;
 import org.apache.maven.model.Model;
 import org.apache.maven.model.Scm;
 import org.assertj.core.api.Assertions;
@@ -147,4 +148,27 @@ void should_write_scm_tag(@TempDir Path tempDir) throws Exception {
 
     }
 
+    @Test
+    void should_respect_insertion_order(@TempDir Path tempDir) throws Exception {
+        URL resource = getClass().getResource("parent/parent-pom.xml");
+        Path basePom = Paths.get(resource.toURI());
+        Model model = Maven.readModel(basePom);
+
+        Path updatedPom = tempDir.resolve("updated-pom.xml");
+        Files.copy(basePom, updatedPom);
+
+        Dependency dependency = model.getDependencies().stream().filter(d -> d.getArtifactId().equals("quarkus-junit5-internal")).findFirst().orElseThrow();
+        dependency.setVersion("1.0.0");
+        dependency.setOptional("true");
+        Maven.writeModel(model, updatedPom);
+
+        assertThat(Files.readString(updatedPom))
+                .contains("<dependency>\n" +
+                        "      <groupId>io.quarkus</groupId>\n" +
+                        "      <artifactId>quarkus-junit5-internal</artifactId>\n" +
+                        "      <version>1.0.0</version>\n" +
+                        "      <scope>test</scope>\n" +
+                        "      <optional>true</optional>\n" +
+                        "    </dependency>");
+    }
 }