Better indentation when adding GCP libraries to pom.xml

plugins/com.google.cloud.tools.eclipse.appengine.libraries.test/src/com/google/cloud/tools/eclipse/appengine/libraries/POMFormatTest.java

@@ -0,0 +1,134 @@
+/*
+ * Copyright 2019 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.cloud.tools.eclipse.appengine.libraries;
+
+import static org.junit.Assert.fail;
+
+import com.google.cloud.tools.eclipse.appengine.libraries.model.Library;
+import com.google.cloud.tools.eclipse.appengine.libraries.model.LibraryFile;
+import com.google.cloud.tools.eclipse.test.util.project.TestProjectCreator;
+import com.google.cloud.tools.eclipse.util.ArtifactRetriever;
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import org.eclipse.core.resources.IFile;
+import org.eclipse.core.resources.IProject;
+import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.runtime.FileLocator;
+import org.eclipse.core.runtime.Platform;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.osgi.framework.Bundle;
+import org.xml.sax.SAXException;
+
+public class POMFormatTest {
+
+  @Rule
+  public final TestProjectCreator projectCreator = new TestProjectCreator();
+  private Pom pom;
+  private IFile pomFile;
+
+  @Before
+  public void setUp() throws SAXException, IOException, CoreException {
+    IProject project = projectCreator.getProject();
+    pomFile = project.getFile("pom.xml");
+    try (InputStream in =
+        Files.newInputStream(Paths.get("testdata/testPomEmpty.xml").toAbsolutePath())) {
+      pomFile.create(in, IFile.FORCE, null);
+      pom = Pom.parse(pomFile);
+    }
+
+    Logger logger = Logger.getLogger(ArtifactRetriever.class.getName());
+    logger.setLevel(Level.OFF);
+  }
+
+
+  @Test
+  public void testAddDependencies() throws Exception {
+    Library library0 = PomTest.newLibrary("id0",
+        new LibraryFile(PomTest.coordinates("com.example.group0", "artifact0", "1.2.3")));
+    Library library1 = PomTest.newLibrary("id1",
+        new LibraryFile(PomTest.coordinates("com.example.group1", "artifact1")));
+    Library library2 = PomTest.newLibrary("id2",
+        new LibraryFile(PomTest.coordinates("com.example.group2", "artifact2")),
+        new LibraryFile(PomTest.coordinates("com.example.group3", "artifact3")));
+
+    List<Library> libraries = Arrays.asList(library0, library1, library2);
+    pom.addDependencies(libraries);
+
+    Bundle bundle = Platform.getBundle("com.google.cloud.tools.eclipse.appengine.libraries.test");
+    URL fileURL = bundle.getEntry("/testdata/formatAdd.xml");
+    File expected = new File(FileLocator.resolve(fileURL).toURI());
+    assertFileContentsEqual(pomFile.getLocation().toFile(), expected);
+
+  }
+
+  @Test
+  public void testRemoveUnusedDependencies() throws Exception {
+    LibraryFile file1 = new LibraryFile(PomTest.coordinates("com.example.group1", "artifact1"));
+    LibraryFile file2 = new LibraryFile(PomTest.coordinates("com.example.group2", "artifact2"));
+    Library library1 = PomTest.newLibrary("id1", file1);
+    Library library2 = PomTest.newLibrary("id2", file1, file2);
+
+    pom.addDependencies(Arrays.asList(library1, library2));
+
+    Bundle bundle = Platform.getBundle("com.google.cloud.tools.eclipse.appengine.libraries.test");
+    URL fileURL = bundle.getEntry("/testdata/formatRemove.xml");
+    File expected = new File(FileLocator.resolve(fileURL).toURI());
+    assertFileContentsEqual(pomFile.getLocation().toFile(), expected);
+  }
+
+  public static void assertFileContentsEqual(File actual, File expected) throws IOException {
+    boolean areEqual = true;
+    try (BufferedReader reader1 = new BufferedReader(new FileReader(actual));
+        BufferedReader reader2 = new BufferedReader(new FileReader(expected));) {
+      String line1 = reader1.readLine();
+      String line2 = reader2.readLine();
+
+      int lineNum = 1;
+
+      while (line1 != null || line2 != null) {
+        if (line1 == null || line2 == null) {
+          areEqual = false;
+          break;
+        } else if (!line1.equalsIgnoreCase(line2)) {
+          areEqual = false;
+          break;
+        }
+        line1 = reader1.readLine();
+        line2 = reader2.readLine();
+        lineNum++;
+      }
+      if (!areEqual) {
+        String message = "Two files have different content. They differ at line " + lineNum + " \""
+            + actual.getName() + "\" has " + line1 + " and \"" + expected.getName() + "\" has "
+            + line2 + " at line " + lineNum + "\n";
+        fail(message);
+      }
+    }
+  }
+}

plugins/com.google.cloud.tools.eclipse.appengine.libraries.test/src/com/google/cloud/tools/eclipse/appengine/libraries/PomTest.java

@@ -373,17 +373,17 @@ private static Element getOnlyChild(Element element, String name) {
     return (Element) children.item(0);
   }
 
-  private static Library newLibrary(String libraryId, LibraryFile... libraryFiles) {
+  static Library newLibrary(String libraryId, LibraryFile... libraryFiles) {
     Library library = new Library(libraryId);
     library.setLibraryFiles(Arrays.asList(libraryFiles));
     return library;
   }
 
-  private static MavenCoordinates coordinates(String groupId, String artifactId) {
+  static MavenCoordinates coordinates(String groupId, String artifactId) {
     return new MavenCoordinates.Builder().setGroupId(groupId).setArtifactId(artifactId).build();
   }
 
-  private static MavenCoordinates coordinates(String groupId, String artifactId, String version) {
+  static MavenCoordinates coordinates(String groupId, String artifactId, String version) {
     return new MavenCoordinates.Builder().setGroupId(groupId).setArtifactId(artifactId)
         .setVersion(version).build();
   }

plugins/com.google.cloud.tools.eclipse.appengine.libraries.test/testdata/formatAdd.xml

@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <dependencyManagement>
+    <dependencies>
+      <dependency>
+        <groupId>com.google.cloud</groupId>
+        <artifactId>google-cloud-bom</artifactId>
+        <type>pom</type>
+        <scope>import</scope>
+        <version>0.99.0-alpha</version>
+      </dependency>
+    </dependencies>
+  </dependencyManagement>
+  <dependencies>
+    <dependency>
+      <groupId>com.example.group0</groupId>
+      <artifactId>artifact0</artifactId>
+      <version>1.2.3</version>
+    </dependency>
+    <dependency>
+      <groupId>com.example.group1</groupId>
+      <artifactId>artifact1</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>com.example.group2</groupId>
+      <artifactId>artifact2</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>com.example.group3</groupId>
+      <artifactId>artifact3</artifactId>
+    </dependency>
+  </dependencies>
+</project>

plugins/com.google.cloud.tools.eclipse.appengine.libraries.test/testdata/formatRemove.xml

@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <dependencyManagement>
+    <dependencies>
+      <dependency>
+        <groupId>com.google.cloud</groupId>
+        <artifactId>google-cloud-bom</artifactId>
+        <type>pom</type>
+        <scope>import</scope>
+        <version>0.99.0-alpha</version>
+      </dependency>
+    </dependencies>
+  </dependencyManagement>
+  <dependencies>
+    <dependency>
+      <groupId>com.example.group1</groupId>
+      <artifactId>artifact1</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>com.example.group2</groupId>
+      <artifactId>artifact2</artifactId>
+    </dependency>
+  </dependencies>
+</project>

plugins/com.google.cloud.tools.eclipse.appengine.libraries.test/testdata/testPomEmpty.xml

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project
+	xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"
+	xmlns="http://maven.apache.org/POM/4.0.0"
+	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+</project>

plugins/com.google.cloud.tools.eclipse.appengine.libraries/src/com/google/cloud/tools/eclipse/appengine/libraries/Pom.java

@@ -234,7 +234,7 @@ void updateDependencies(Collection<Library> selectedLibraries,
           dependency.appendChild(artifactIdElement);
 
           if (testComment == null) {
-            dependencies.appendChild(dependency);
+            addChild(dependencies, dependency);
           } else {
             dependencies.insertBefore(dependency, testComment);
           }
@@ -279,7 +279,7 @@ private void handleDependencyManaged(LibraryFile artifact, Element dependency) {
       }
     } else {
       if (versionNode != null) {
-        dependency.removeChild(versionNode);
+        removeChild(dependency, versionNode);
       }
     }
   }
@@ -305,8 +305,8 @@ private void createBOMIfNeeded(XPath xpath) throws CoreException {
             dependencyManagement = document.createElementNS("http://maven.apache.org/POM/4.0.0",
                 "dependencyManagement");
           }
-          dependencyManagement.appendChild(dependencies);
-          document.getDocumentElement().appendChild(dependencyManagement);
+          addChild(dependencyManagement, dependencies);
+          addChild(document.getDocumentElement(), dependencyManagement);
         }
 
         Element dependency =
@@ -321,7 +321,7 @@ private void createBOMIfNeeded(XPath xpath) throws CoreException {
         if (bom != null) {
           boms.add(bom);
         }
-        dependencies.appendChild(dependency);
+        addChild(dependencies, dependency);
       }
     } catch (XPathExpressionException ex) {
       IStatus status = StatusUtil.error(Pom.class, ex.getMessage(), ex);
@@ -444,7 +444,7 @@ static void removeUnusedDependencies(Element dependencies,
     }
 
     for (Node node : nodesToRemove) {
-      dependencies.removeChild(node);
+      removeChild(dependencies, node);
     }
   }
 
@@ -482,10 +482,29 @@ private static String getValue(Element dependency, String childName) {
   private void writeDocument() throws CoreException, TransformerException {
     Transformer transformer = transformerFactory.newTransformer();
     transformer.setOutputProperty(OutputKeys.INDENT, "yes");
+    transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount","2"); //$NON-NLS-1$ //$NON-NLS-2$
     ByteArrayOutputStream out = new ByteArrayOutputStream();
     transformer.transform(new DOMSource(document), new StreamResult(out));
     InputStream in = new ByteArrayInputStream(out.toByteArray());
 
     pomFile.setContents(in, true, true, null);
   }
+
+  private void addChild(Node parent, Node newChild) {
+    parent.appendChild(newChild);
+    removeWhiteSpaceBefore(newChild);// for child indentation
+  }
+
+  private static void removeChild(Node parent, Node oldChild) {
+    removeWhiteSpaceBefore(oldChild);
+    parent.removeChild(oldChild);
+  }
+
+  private static void removeWhiteSpaceBefore(Node node) {
+    Node prevElement = node.getPreviousSibling();
+    if (prevElement != null && prevElement.getNodeType() == Node.TEXT_NODE
+        && prevElement.getNodeValue().trim().length() == 0) {
+      node.getParentNode().removeChild(prevElement);
+    }
+  }
 }