Bug 560068 - parse relative bundle paths correctly from config.iniX20200227-1900X20200227-0700X20200227-0130X20200226-0700X20200225-1900X20200225-0700X20200224-1900X20200224-0700X20200223-1900X20200223-0700X20200223-0250X20200222-0700X20200221-1130X20200221-0700X20200221-0440X20200221-0310X20200221-0050X20200220-1900X20200220-0700X20200219-1900X20200219-0700X20200219-0500X20200218-0510S4_15_0_RC1S4_15_0_M3I20200226-1800I20200226-0600I20200225-1800I20200225-0600I20200224-1800I20200224-0600I20200223-2335I20200223-1800I20200223-0600I20200222-1800I20200222-0600I20200221-1800I20200219-1800I20200219-0600I20200218-1800I20200218-0600

Fixed exception when parsing URIs with relative paths from config.ini.

Filter non-existent files to be safe, as this is only intended as a
fallback for tycho tests.

Change-Id: I0e7dc0618a485b08c52c4f1fe2fc62af1ae41eb1
Signed-off-by: Julian Honnen <julian.honnen@vector.com>

2 files changed, 116 insertions, 26 deletions

diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/ProfileBundleContainer.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/ProfileBundleContainer.java
index b06f7f0c7b..8826104465 100644
--- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/ProfileBundleContainer.java
+++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/ProfileBundleContainer.java
@@ -14,6 +14,7 @@
 package org.eclipse.pde.internal.core.target;
 
 import static java.util.Arrays.stream;
+import static java.util.Collections.emptyList;
 import static java.util.stream.Stream.concat;
 
 import java.io.File;
@@ -24,7 +25,7 @@ import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.Objects;
 import java.util.Properties;
@@ -133,9 +134,9 @@ public class ProfileBundleContainer extends AbstractBundleContainer {
 		if (infos == null) {
 			if (configUrl != null) {
 				try {
-					TargetBundle[] osgiBundles = readBundleInfosFromConfigIni(configUrl.toURI());
-					if (osgiBundles != null && osgiBundles.length > 0) {
-						return osgiBundles;
+					Collection<TargetBundle> osgiBundles = readBundleInfosFromConfigIni(configUrl.toURI());
+					if (!osgiBundles.isEmpty()) {
+						return osgiBundles.toArray(new TargetBundle[0]);
 					}
 				} catch (URISyntaxException ex) {
 					throw new CoreException(new Status(IStatus.ERROR, PDECore.PLUGIN_ID, ex.getMessage(), ex));
@@ -173,42 +174,78 @@ public class ProfileBundleContainer extends AbstractBundleContainer {
 		}).filter(Objects::nonNull).toArray(TargetBundle[]::new);
 	}
 
-	private TargetBundle[] readBundleInfosFromConfigIni(URI configArea) {
+	private Collection<TargetBundle> readBundleInfosFromConfigIni(URI configArea) {
 		File configIni = new File(configArea);
 		configIni = new File(configIni, CONFIG_INI);
 		if (!configIni.isFile()) {
-			return null;
+			return emptyList();
 		}
 		Properties configProps = new Properties();
 		try (FileInputStream fis = new FileInputStream(configIni)) {
 			configProps.load(fis);
 		} catch (IOException e) {
 			PDECore.log(e);
-			return null;
+			return emptyList();
 		}
+
+		List<File> bundleFiles = parseBundlesFromConfigIni(configProps);
+		ArrayList<TargetBundle> bundles = new ArrayList<>();
+		for (File file : bundleFiles) {
+			if (!file.exists()) {
+				continue;
+			}
+			TargetBundle bundle;
+			try {
+				bundle = new TargetBundle(file);
+			} catch (CoreException e) {
+				bundle = new InvalidTargetBundle(new BundleInfo(file.toURI()), e.getStatus());
+			}
+			bundles.add(bundle);
+		}
+		return bundles;
+	}
+
+	public static List<File> parseBundlesFromConfigIni(Properties configProps) {
 		String osgiBundles = configProps.getProperty("osgi.bundles"); //$NON-NLS-1$
 		if (osgiBundles == null || osgiBundles.isEmpty()) {
-			return null;
+			return emptyList();
 		}
+
+		ArrayList<File> bundles = new ArrayList<>();
+		File baseDir = null;
+
 		String osgiFramework = configProps.getProperty("osgi.framework"); //$NON-NLS-1$
-		if (osgiFramework != null && !osgiFramework.isEmpty()) {
-			osgiBundles = osgiFramework + ',' + osgiBundles;
+		if (osgiFramework != null) {
+			File frameworkBundle = parseBundleLocation(osgiFramework);
+			bundles.add(frameworkBundle);
+			baseDir = frameworkBundle.getParentFile();
 		}
-		return Arrays.stream(osgiBundles.split(",")) //$NON-NLS-1$
-				.map(entry -> entry.split("@")[0]) //$NON-NLS-1$
-				.map(location -> location.startsWith("reference:") ? location.substring("reference:".length()) //$NON-NLS-1$ //$NON-NLS-2$
-						: location)
-				.map(URI::create) //
-				.filter(URI::isAbsolute) //
-				.map(File::new) //
-				.map(file -> {
-					try {
-						return new TargetBundle(file);
-					} catch (CoreException e) {
-						return new InvalidTargetBundle(new BundleInfo(file.toURI()), e.getStatus());
-					}
-				}).filter(Objects::nonNull) //
-				.toArray(TargetBundle[]::new);
+
+		for (String spec : osgiBundles.split(",")) { //$NON-NLS-1$
+			File location = parseBundleLocation(spec);
+			if (baseDir == null || location.isAbsolute()) {
+				bundles.add(location);
+			} else {
+				bundles.add(new File(baseDir, location.getPath()));
+			}
+		}
+
+		return bundles;
+	}
+
+	private static File parseBundleLocation(String spec) {
+		String path = spec.split("@", 2)[0]; //$NON-NLS-1$
+		path = trimPrefix(path, "reference:"); //$NON-NLS-1$
+		path = trimPrefix(path, "file:"); //$NON-NLS-1$
+		return new File(path);
+	}
+
+	private static String trimPrefix(String string, String prefix) {
+		if (string.startsWith(prefix)) {
+			return string.substring(prefix.length());
+		}
+
+		return string;
 	}
 
 	@Override
diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/ProfileContainerTests.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/ProfileContainerTests.java
index c19f999444..fb1c79afc8 100644
--- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/ProfileContainerTests.java
+++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/ProfileContainerTests.java
@@ -10,9 +10,11 @@
  *******************************************************************************/
 package org.eclipse.pde.ui.tests.target;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
+import java.util.*;
 import org.eclipse.core.runtime.NullProgressMonitor;
 import org.eclipse.equinox.simpleconfigurator.manipulator.SimpleConfiguratorManipulator;
 import org.eclipse.pde.core.target.ITargetDefinition;
@@ -24,10 +26,61 @@ public class ProfileContainerTests extends AbstractTargetTest {
 
 	@Test
 	public void testBundleResolutionWithConfigIni() {
-		File bundlesInfo = new File(new ProfileBundleContainer("${eclipse.home}", null).getConfigurationLocation(), SimpleConfiguratorManipulator.BUNDLES_INFO_PATH);
+		File bundlesInfo = new File(new ProfileBundleContainer("${eclipse.home}", null).getConfigurationLocation(),
+				SimpleConfiguratorManipulator.BUNDLES_INFO_PATH);
 		Assume.assumeFalse("Skip test when using regular p2 configurator", bundlesInfo.isFile());
 		ITargetDefinition defaultDefinition = getTargetService().newDefaultTarget();
 		defaultDefinition.resolve(new NullProgressMonitor());
 		assertTrue(defaultDefinition.getBundles().length > 10);
 	}
+
+	@Test
+	public void testParseBundleInfoFromConfigIni() {
+
+		Properties configIni = new Properties();
+		configIni.put("osgi.bundles", absoluteFile("plugins/some.bundle").toURI() + ","//
+				+ "reference:" + absoluteFile("plugins/some.bundle_startlevel").toURI() + "@1:start");
+
+		Collection<File> parsedBundles = ProfileBundleContainer.parseBundlesFromConfigIni(configIni);
+		assertEquals(Arrays.asList( //
+				absoluteFile("plugins/some.bundle"), //
+				absoluteFile("plugins/some.bundle_startlevel")), //
+				parsedBundles);
+	}
+
+	@Test
+	public void testParseBundleInfoFromConfigIni_relative() {
+		Properties configIni = new Properties();
+		configIni.put("osgi.bundles", "reference:file:plugins/some.bundle," //
+				+ "reference:file:plugins/some.bundle_startlevel@1:start," //
+				+ "reference:" + absoluteFile("absolute.bundle").toURI());
+
+		Collection<File> parsedBundles = ProfileBundleContainer.parseBundlesFromConfigIni(configIni);
+		assertEquals(Arrays.asList( //
+				new File("plugins/some.bundle"), //
+				new File("plugins/some.bundle_startlevel"), //
+				absoluteFile("absolute.bundle")), //
+				parsedBundles);
+	}
+
+	@Test
+	public void testParseBundleInfoFromConfigIni_relativeToFramework() {
+		Properties configIni = new Properties();
+		configIni.put("osgi.bundles", "reference:file:some.bundle," //
+				+ "reference:file:some.bundle_startlevel@1:start," //
+				+ "reference:" + absoluteFile("absolute.bundle").toURI());
+		configIni.put("osgi.framework", "file:plugins/o.e.osgi.jar");
+
+		Collection<File> parsedBundles = ProfileBundleContainer.parseBundlesFromConfigIni(configIni);
+		assertEquals(Arrays.asList( //
+				new File("plugins/o.e.osgi.jar"), //
+				new File("plugins/some.bundle"), //
+				new File("plugins/some.bundle_startlevel"), //
+				absoluteFile("absolute.bundle")), //
+				parsedBundles);
+	}
+
+	private static File absoluteFile(String path) {
+		return new File(path).getAbsoluteFile();
+	}
 }