diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetDefinition.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetDefinition.java index 5b56dc08f1..61fd8719b0 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetDefinition.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetDefinition.java @@ -1270,15 +1270,12 @@ private void updateIUContainerElements(Element containersElement, List for (int i = 0; i < nodes.getLength(); i++) { Node node = nodes.item(i); if (node instanceof Element) { - if (repoURL == null - && node.getNodeName().equalsIgnoreCase(TargetDefinitionPersistenceHelper.REPOSITORY)) { + String nodeName = node.getNodeName(); + if (repoURL == null && nodeName.equalsIgnoreCase(TargetDefinitionPersistenceHelper.REPOSITORY)) { repoURL = ((Element) node).getAttribute(TargetDefinitionPersistenceHelper.LOCATION); - if (!oldContainersByRepo.containsKey(repoURL)) { - oldContainersByRepo.put(repoURL, new ArrayList<>()); - } + oldContainersByRepo.putIfAbsent(repoURL, new ArrayList<>()); oldContainersByRepo.get(repoURL).add(container); - } else if (node.getNodeName() - .equalsIgnoreCase(TargetDefinitionPersistenceHelper.INSTALLABLE_UNIT)) { + } else if (nodeName.equalsIgnoreCase(TargetDefinitionPersistenceHelper.INSTALLABLE_UNIT)) { units.add((Element) node); } } @@ -1290,6 +1287,8 @@ private void updateIUContainerElements(Element containersElement, List } } + // The containers that were only updated. Doesn't include brand-new ones + List updatedContainers = new ArrayList<>(newContainers); for (Element container : newContainers) { NodeList nodes = container.getChildNodes(); List units = new ArrayList<>(); @@ -1297,11 +1296,10 @@ private void updateIUContainerElements(Element containersElement, List for (int i = 0; i < nodes.getLength(); i++) { Node node = nodes.item(i); if (node instanceof Element) { - if (repoURL == null - && node.getNodeName().equalsIgnoreCase(TargetDefinitionPersistenceHelper.REPOSITORY)) { + String nodeName = node.getNodeName(); + if (repoURL == null && nodeName.equalsIgnoreCase(TargetDefinitionPersistenceHelper.REPOSITORY)) { repoURL = ((Element) node).getAttribute(TargetDefinitionPersistenceHelper.LOCATION); - } else if (node.getNodeName() - .equalsIgnoreCase(TargetDefinitionPersistenceHelper.INSTALLABLE_UNIT)) { + } else if (nodeName.equalsIgnoreCase(TargetDefinitionPersistenceHelper.INSTALLABLE_UNIT)) { units.add((Element) node); } } @@ -1321,10 +1319,16 @@ private void updateIUContainerElements(Element containersElement, List } else { Node movedContainer = fDocument.importNode(container, true); TargetDefinitionDocumentTools.addChildWithIndent(containersElement, movedContainer); + updatedContainers.remove(container); } } } + // Use a mock comparator, we only want to use the + // "elementAttributesComparator" also used in updateElements + TargetDefinitionDocumentTools.updateElements(containersElement, oldContainers, updatedContainers, + (e1, e2) -> 0); + for (Entry> entry : oldContainersByRepo.entrySet()) { entry.getValue().forEach(TargetDefinitionDocumentTools::removeChildAndWhitespace); } diff --git a/ui/org.eclipse.pde.genericeditor.extension.tests/src/org/eclipse/pde/genericeditor/extension/tests/Bug531602FormattingTests.java b/ui/org.eclipse.pde.genericeditor.extension.tests/src/org/eclipse/pde/genericeditor/extension/tests/Bug531602FormattingTests.java index 9a7282623a..e2b9b9d2fd 100644 --- a/ui/org.eclipse.pde.genericeditor.extension.tests/src/org/eclipse/pde/genericeditor/extension/tests/Bug531602FormattingTests.java +++ b/ui/org.eclipse.pde.genericeditor.extension.tests/src/org/eclipse/pde/genericeditor/extension/tests/Bug531602FormattingTests.java @@ -50,8 +50,7 @@ public void testSettingNullPersists() throws Exception { ByteArrayOutputStream actualOutput = new ByteArrayOutputStream(); targetDefinition.setProgramArguments(null); TargetDefinitionPersistenceHelper.persistXML(targetDefinition, actualOutput); - assertEquals(expectedOutput.toString(StandardCharsets.UTF_8.toString()), - actualOutput.toString(StandardCharsets.UTF_8.toString())); + assertEquals(expectedOutput.toString(StandardCharsets.UTF_8), actualOutput.toString(StandardCharsets.UTF_8)); } @Test @@ -105,9 +104,6 @@ public void testITargetLocationExtensionSerialization() throws Exception { confirmMatch(targetDefinition, "ITargetLocationExtensionTestCaseTarget.txt"); } - public static void assertEqualStringIgnoreDelim(String actual, String expected) throws IOException { - StringAsserts.assertEqualStringIgnoreDelim(actual, expected); - } private void confirmMatch(ITargetDefinition targetDefinition, String expectedDefinitionPath) throws Exception { try (Scanner s = new Scanner(FrameworkUtil.getBundle(this.getClass()) .getEntry("testing-files/target-files/" + expectedDefinitionPath).openStream()).useDelimiter("\\A")) { @@ -123,7 +119,7 @@ private void confirmMatch(ITargetDefinition targetDefinition, String expectedDef ByteArrayOutputStream actualOutput = new ByteArrayOutputStream(); TargetDefinitionPersistenceHelper.persistXML(targetDefinition, actualOutput); - assertEqualStringIgnoreDelim(result, actualOutput.toString(StandardCharsets.UTF_8.toString())); + StringAsserts.assertEqualStringIgnoreDelim(actualOutput.toString(StandardCharsets.UTF_8), result); } catch (IOException e) { } } diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/IUBundleContainerTests.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/IUBundleContainerTests.java index f23a2e374e..a48cbe9a99 100644 --- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/IUBundleContainerTests.java +++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/IUBundleContainerTests.java @@ -15,6 +15,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -592,9 +593,10 @@ public void testSerialization2() throws Exception { @Test public void testSerialization3() throws Exception { URI uri = getURI("/tests/sites/site.a.b"); - String[] unitIds = new String[]{"feature.b.feature.group"}; + String[] unitIds = new String[] { "feature.b.feature.group" }; IInstallableUnit[] units = getUnits(unitIds, uri); - IUBundleContainer location = createContainer(units, new URI[] {uri}, IUBundleContainer.INCLUDE_ALL_ENVIRONMENTS | IUBundleContainer.INCLUDE_SOURCE); + IUBundleContainer location = createContainer(units, new URI[] { uri }, + IUBundleContainer.INCLUDE_ALL_ENVIRONMENTS | IUBundleContainer.INCLUDE_SOURCE); String xml = location.serialize(); assertIncludeAllPlatform(xml, true); assertIncludeMode(xml, "slicer"); @@ -602,6 +604,44 @@ public void testSerialization3() throws Exception { deserializationTest(location); } + @Test + public void testSerializationOnlyLocationAttributeChanged() throws Exception { + URI uri = getURI("/tests/sites/site.a.b"); + String[] unitIds = new String[]{"feature.b.feature.group"}; + IInstallableUnit[] units = getUnits(unitIds, uri); + + IUBundleContainer location1 = createContainer(units, new URI[] { uri }, + IUBundleContainer.INCLUDE_ALL_ENVIRONMENTS | IUBundleContainer.INCLUDE_SOURCE); + String xml1 = location1.serialize(); + assertIncludeAllPlatform(xml1, true); + assertIncludeMode(xml1, "slicer"); + assertIncludeSource(xml1, true); + + IUBundleContainer location2 = createContainer(units, new URI[] { uri }, + IUBundleContainer.INCLUDE_ALL_ENVIRONMENTS); // no source + String xml2 = location2.serialize(); + assertIncludeAllPlatform(xml2, true); + assertIncludeMode(xml2, "slicer"); + assertIncludeSource(xml2, false); // no source + + ITargetDefinition td = getTargetService().newTarget(); + td.setTargetLocations(new ITargetLocation[] { location1 }); + ByteArrayOutputStream out1 = new ByteArrayOutputStream(); + TargetDefinitionPersistenceHelper.persistXML(td, out1); + String resultXmlOld = new String(out1.toByteArray()); + + td.setTargetLocations(new ITargetLocation[] { location2 }); + ByteArrayOutputStream out2 = new ByteArrayOutputStream(); + TargetDefinitionPersistenceHelper.persistXML(td, out2); + String resultXmlNew = new String(out2.toByteArray()); + + String normalizedOld = resultXmlOld.replaceAll("\r?\n[ \t]*", ""); + String normalizedNew = resultXmlNew.replaceAll("\r?\n[ \t]*", ""); + + assertNotEquals(normalizedOld, normalizedNew); + assertEquals(normalizedOld, normalizedNew.replace("includeSource=\"false\"", "includeSource=\"true\"")); + } + public void deserializationTest(IUBundleContainer location) throws Exception { ITargetDefinition td = getTargetService().newTarget(); td.setTargetLocations(new ITargetLocation[] {location});