Bug 567674 - replaced URI/URL in config area handling with FileY20201109-0220Y20201108-1200Y20201107-1200Y20201106-1200I20201108-1800I20201107-1800I20201107-0750I20201106-1800I20201106-0710

Simplifies code and fixes broken File -> URL -> URI -> File roundtrip
when path contains special characters.

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

5 files changed, 39 insertions, 55 deletions

diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/P2Utils.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/P2Utils.java
index 6c757f5056..bd0be54a94 100644
--- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/P2Utils.java
+++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/P2Utils.java
@@ -15,10 +15,12 @@
 package org.eclipse.pde.internal.core;
 
 import java.io.File;
-import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -93,7 +95,7 @@ public class P2Utils {
 	 * @return URLs of all bundles in the installation or <code>null</code> if not able
 	 * 	to locate a bundles.info
 	 */
-	public static URL[] readBundlesTxt(String platformHome, URL configurationArea) {
+	public static URL[] readBundlesTxt(String platformHome, File configurationArea) {
 		if (configurationArea == null) {
 			return null;
 		}
@@ -131,15 +133,15 @@ public class P2Utils {
 	 * @return all bundles in the installation or <code>null</code> if not able
 	 * 	to locate a bundles.info
 	 */
-	public static BundleInfo[] readBundles(String platformHome, URL configurationArea) {
+	public static BundleInfo[] readBundles(String platformHome, File configurationArea) {
 		IPath basePath = new Path(platformHome);
 		if (configurationArea == null) {
 			return null;
 		}
 		try {
-			URL bundlesTxt = new URL(configurationArea.getProtocol(), configurationArea.getHost(), new File(configurationArea.getFile(), SimpleConfiguratorManipulator.BUNDLES_INFO_PATH).getAbsolutePath());
+			File bundlesTxt = new File(configurationArea, SimpleConfiguratorManipulator.BUNDLES_INFO_PATH);
 			File home = basePath.toFile();
-			BundleInfo bundles[] = getBundlesFromFile(bundlesTxt, home);
+			BundleInfo[] bundles = getBundlesFromFile(bundlesTxt, home);
 			if (bundles == null || bundles.length == 0) {
 				return null;
 			}
@@ -160,15 +162,15 @@ public class P2Utils {
 	 * @return all source bundles in the installation or <code>null</code> if not able
 	 * 	to locate a source.info
 	 */
-	public static BundleInfo[] readSourceBundles(String platformHome, URL configurationArea) {
+	public static BundleInfo[] readSourceBundles(String platformHome, File configurationArea) {
 		IPath basePath = new Path(platformHome);
 		if (configurationArea == null) {
 			return null;
 		}
 		try {
 			File home = basePath.toFile();
-			URL srcBundlesTxt = new URL(configurationArea.getProtocol(), configurationArea.getHost(), configurationArea.getFile().concat(SimpleConfiguratorManipulator.SOURCE_INFO_PATH));
-			BundleInfo srcBundles[] = getBundlesFromFile(srcBundlesTxt, home);
+			File srcBundlesTxt = new File(configurationArea, SimpleConfiguratorManipulator.SOURCE_INFO_PATH);
+			BundleInfo[] srcBundles = getBundlesFromFile(srcBundlesTxt, home);
 			if (srcBundles == null || srcBundles.length == 0) {
 				return null;
 			}
@@ -194,24 +196,26 @@ public class P2Utils {
 	}
 
 	/**
-	 * Returns a list of {@link BundleInfo} for each bundle entry or <code>null</code> if there
-	 * is a problem reading the file.
+	 * Returns a list of {@link BundleInfo} for each bundle entry or
+	 * <code>null</code> if there is a problem reading the file.
 	 *
-	 * @param fileURL the URL of the file to read
-	 * @param home the path describing the base location of the platform install
+	 * @param filePath
+	 *            the file to read
+	 * @param home
+	 *            the path describing the base location of the platform install
 	 * @return list containing URL locations or <code>null</code>
 	 * @throws IOException
 	 */
-	private static BundleInfo[] getBundlesFromFile(URL fileURL, File home) throws IOException {
+	private static BundleInfo[] getBundlesFromFile(File filePath, File home) throws IOException {
 		SimpleConfiguratorManipulator manipulator = PDECore.getDefault()
 				.acquireService(SimpleConfiguratorManipulator.class);
 		if (manipulator == null) {
 			return null;
 		}
-		// the input stream will be buffered and closed for us
-		try {
-			return manipulator.loadConfiguration(fileURL.openStream(), home.toURI());
-		} catch (FileNotFoundException e) {
+		// the input stream will be buffered for us
+		try (InputStream in = Files.newInputStream(filePath.toPath())) {
+			return manipulator.loadConfiguration(in, home.toURI());
+		} catch (NoSuchFileException e) {
 			return null;
 		}
 	}
diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/Messages.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/Messages.java
index a8f09260da..c59be5175d 100755
--- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/Messages.java
+++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/Messages.java
@@ -53,7 +53,6 @@ public class Messages extends NLS {
 	public static String LocalTargetHandle_5;
 	public static String P2TargetUtils_ProvisioningSourceTask;
 	public static String ProfileBundleContainer_0;
-	public static String ProfileBundleContainer_1;
 	public static String ProfileBundleContainer_2;
 	public static String TargetBundle_ErrorReadingManifest;
 	public static String TargetDefinition_0;
diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/Messages.properties b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/Messages.properties
index ad24cd0cdb..282984fe4e 100755
--- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/Messages.properties
+++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/Messages.properties
@@ -45,7 +45,6 @@ LocalTargetHandle_4=Error saving target definition {0}
 LocalTargetHandle_5=Error creating target file
 P2TargetUtils_ProvisioningSourceTask=Provisioning source bundles
 ProfileBundleContainer_0=Installation directory does not exist: {0}
-ProfileBundleContainer_1=Unable resolve configuration area in {0}
 ProfileBundleContainer_2=Configuration directory does not exist: {0}
 TargetBundle_ErrorReadingManifest=Error reading manifest for {0}
 TargetDefinition_0=Error reading target definition
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 fb654f4469..2e3bc0cdf3 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
@@ -20,10 +20,7 @@ import static java.util.stream.Stream.concat;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
-import java.net.MalformedURLException;
 import java.net.URI;
-import java.net.URISyntaxException;
-import java.net.URL;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -121,23 +118,19 @@ public class ProfileBundleContainer extends AbstractBundleContainer {
 			throw new CoreException(new Status(IStatus.ERROR, PDECore.PLUGIN_ID, NLS.bind(Messages.ProfileBundleContainer_0, home)));
 		}
 
-		URL configUrl = getConfigurationArea();
-		if (configUrl != null) {
-			if (!new File(configUrl.getFile()).isDirectory()) {
+		File configurationArea = getConfigurationArea();
+		if (configurationArea != null) {
+			if (!configurationArea.isDirectory()) {
 				throw new CoreException(new Status(IStatus.ERROR, PDECore.PLUGIN_ID, NLS.bind(Messages.ProfileBundleContainer_2, home)));
 			}
 		}
 
-		BundleInfo[] infos = P2Utils.readBundles(home, configUrl);
+		BundleInfo[] infos = P2Utils.readBundles(home, configurationArea);
 		if (infos == null) {
-			if (configUrl != null) {
-				try {
-					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));
+			if (configurationArea != null) {
+				Collection<TargetBundle> osgiBundles = readBundleInfosFromConfigIni(configurationArea);
+				if (!osgiBundles.isEmpty()) {
+					return osgiBundles.toArray(new TargetBundle[0]);
 				}
 			}
 			DirectoryBundleContainer directoryBundleContainer = new DirectoryBundleContainer(home);
@@ -153,7 +146,7 @@ public class ProfileBundleContainer extends AbstractBundleContainer {
 			return new TargetBundle[0];
 		}
 
-		BundleInfo[] source = P2Utils.readSourceBundles(home, configUrl);
+		BundleInfo[] source = P2Utils.readSourceBundles(home, configurationArea);
 		if (source == null) {
 			source = new BundleInfo[0];
 		}
@@ -174,9 +167,8 @@ public class ProfileBundleContainer extends AbstractBundleContainer {
 		}).filter(Objects::nonNull).toArray(TargetBundle[]::new);
 	}
 
-	private Collection<TargetBundle> readBundleInfosFromConfigIni(URI configArea) {
-		File configIni = new File(configArea);
-		configIni = new File(configIni, CONFIG_INI);
+	private Collection<TargetBundle> readBundleInfosFromConfigIni(File configArea) {
+		File configIni = new File(configArea, CONFIG_INI);
 		if (!configIni.isFile()) {
 			return emptyList();
 		}
@@ -273,7 +265,7 @@ public class ProfileBundleContainer extends AbstractBundleContainer {
 	 * @return configuration area URL or <code>null</code>
 	 * @throws CoreException if unable to generate a URL or the user specified location does not exist
 	 */
-	private URL getConfigurationArea() throws CoreException {
+	private File getConfigurationArea() throws CoreException {
 		IPath home = resolveHomeLocation();
 		IPath configuration = null;
 		if (fConfiguration == null) {
@@ -283,11 +275,7 @@ public class ProfileBundleContainer extends AbstractBundleContainer {
 		}
 		File file = configuration.toFile();
 		if (file.exists()) {
-			try {
-				return file.toURL();
-			} catch (MalformedURLException e) {
-				throw new CoreException(new Status(IStatus.ERROR, PDECore.PLUGIN_ID, NLS.bind(Messages.ProfileBundleContainer_1, home.toOSString()), e));
-			}
+			return file;
 		} else if (fConfiguration != null) {
 			// If the user specified config area does not exist throw an error
 			throw new CoreException(new Status(IStatus.ERROR, PDECore.PLUGIN_ID, NLS.bind(Messages.ProfileBundleContainer_2, configuration.toOSString())));
@@ -330,11 +318,8 @@ public class ProfileBundleContainer extends AbstractBundleContainer {
 		if (!new File(home).isDirectory()) {
 			throw new CoreException(new Status(IStatus.ERROR, PDECore.PLUGIN_ID, NLS.bind(Messages.ProfileBundleContainer_0, home)));
 		}
-		File configArea = null;
-		URL configURL = getConfigurationArea();
-		if (configURL != null) {
-			configArea = new File(configURL.getFile());
-		} else {
+		File configArea = getConfigurationArea();
+		if (configArea == null) {
 			configArea = new File(home);
 		}
 		if (!configArea.isDirectory()) {
diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/LocalTargetDefinitionTests.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/LocalTargetDefinitionTests.java
index 47876f8b9e..bcee0516cd 100644
--- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/LocalTargetDefinitionTests.java
+++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/LocalTargetDefinitionTests.java
@@ -133,8 +133,7 @@ public class LocalTargetDefinitionTests extends AbstractTargetTest {
 
 		// the old way
 		IPath location = new Path(TargetPlatform.getDefaultLocation());
-		URL[] pluginPaths = P2Utils.readBundlesTxt(location.toOSString(),
-				location.append("configuration").toFile().toURI().toURL());
+		URL[] pluginPaths = P2Utils.readBundlesTxt(location.toOSString(), location.append("configuration").toFile());
 		// pluginPaths will be null (and NPE) when self-hosting and the target
 		// platform is not a real installation
 		assertBundlePathsEqual(pluginPaths, uris);
@@ -258,8 +257,7 @@ public class LocalTargetDefinitionTests extends AbstractTargetTest {
 
 		// the old way
 		IPath location = new Path(TargetPlatform.getDefaultLocation());
-		URL[] pluginPaths = P2Utils.readBundlesTxt(location.toOSString(),
-				location.append("configuration").toFile().toURI().toURL());
+		URL[] pluginPaths = P2Utils.readBundlesTxt(location.toOSString(), location.append("configuration").toFile());
 		// pluginPaths will be null (and NPE) when self-hosting and the target
 		// platform is not a real installation
 		assertBundlePathsEqual(pluginPaths, uris);
@@ -284,8 +282,7 @@ public class LocalTargetDefinitionTests extends AbstractTargetTest {
 
 		// the old way
 		IPath location = new Path(TargetPlatform.getDefaultLocation());
-		URL[] pluginPaths = P2Utils.readBundlesTxt(location.toOSString(),
-				location.append("configuration").toFile().toURI().toURL());
+		URL[] pluginPaths = P2Utils.readBundlesTxt(location.toOSString(), location.append("configuration").toFile());
 		// pluginPaths will be null (and NPE) when self-hosting and the target
 		// platform is not a real installation
 		assertBundlePathsEqual(pluginPaths, uris);