Bug 219109 [prov] IArtifactRepository#getOutputStream should declare a thrown exception

4 files changed, 43 insertions, 38 deletions

diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/Messages.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/Messages.java
index ef7ec7f0c..14dbc8b3b 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/Messages.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/Messages.java
@@ -13,33 +13,33 @@ package org.eclipse.equinox.internal.p2.artifact.repository;
 import org.eclipse.osgi.util.NLS;
 
 public class Messages extends NLS {
-
 	private static final String BUNDLE_NAME = "org.eclipse.equinox.internal.p2.artifact.repository.messages"; //$NON-NLS-1$
 
-	static {
-		// initialize resource bundles
-		NLS.initializeMessages(BUNDLE_NAME, Messages.class);
-	}
-
-	private Messages() {
-		// Do not instantiate
-	}
-
 	public static String artifact_not_found;
 	public static String available_already_in;
 	public static String cant_get_outputstream;
 	public static String downloading;
 	public static String error_closing_stream;
+	public static String FileDownloadError;
+	public static String io_failedRead;
+	public static String io_incompatibleVersion;
+	public static String io_parseError;
+	public static String mirroring;
 	public static String repoMan_exists;
 	public static String repoMan_failedRead;
 	public static String repoMan_internalError;
 	public static String repoMan_notExists;
 	public static String repoMan_unknownType;
+	public static String repoFailedWrite;
+	public static String repoReadOnly;
 
-	public static String FileDownloadError;
-	public static String io_failedRead;
-	public static String io_parseError;
-	public static String io_incompatibleVersion;
-	public static String mirroring;
+	static {
+		// initialize resource bundles
+		NLS.initializeMessages(BUNDLE_NAME, Messages.class);
+	}
+
+	private Messages() {
+		// Do not instantiate
+	}
 
 }
\ No newline at end of file
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/MirrorRequest.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/MirrorRequest.java
index 68c6caf01..82c95152b 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/MirrorRequest.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/MirrorRequest.java
@@ -18,6 +18,7 @@ import org.eclipse.core.runtime.*;
 import org.eclipse.equinox.internal.provisional.p2.artifact.repository.*;
 import org.eclipse.equinox.internal.provisional.p2.artifact.repository.processing.ProcessingStepDescriptor;
 import org.eclipse.equinox.internal.provisional.p2.artifact.repository.processing.ProcessingStepHandler;
+import org.eclipse.equinox.internal.provisional.p2.core.ProvisionException;
 import org.eclipse.equinox.internal.provisional.p2.metadata.IArtifactKey;
 import org.eclipse.osgi.util.NLS;
 
@@ -105,9 +106,11 @@ public class MirrorRequest extends ArtifactRequest {
 		if (targetRepositoryProperties != null)
 			destinationDescriptor.addRepositoryProperties(targetRepositoryProperties);
 
-		OutputStream destination = target.getOutputStream(destinationDescriptor);
-		if (destination == null) {
-			setResult(new Status(IStatus.ERROR, Activator.ID, NLS.bind(Messages.cant_get_outputstream, target, getArtifactKey())));
+		OutputStream destination;
+		try {
+			destination = target.getOutputStream(destinationDescriptor);
+		} catch (ProvisionException e) {
+			setResult(e.getStatus());
 			return;
 		}
 
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/messages.properties b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/messages.properties
index b66d5f7df..91fed5e49 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/messages.properties
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/messages.properties
@@ -8,12 +8,12 @@
 # Contributors:
 #     IBM Corporation - initial API and implementation
 ###############################################################################
-available_already_in=The artifact is already available in the repo {0}.
+artifact_not_found=Artifact requested can't be found {0}.
+available_already_in=The artifact is already available in the repository {0}.
 cant_get_outputstream=Can't get an output stream to {0} store {1}. 
 downloading=Downloading {0}.
-error_closing_stream=Error closing the output stream for {0} on repo {1}. 
+error_closing_stream=Error closing the output stream for {0} on repository {1}. 
 
-artifact_not_found=Artifact requested can't be found {0}.
 FileDownloadError=Error downloading {0} to {1}. 
 io_failedRead=Unable to read repository at {0}.
 io_parseError=\
@@ -21,6 +21,10 @@ io_parseError=\
 io_incompatibleVersion=\
 	Simple artifact repository has incompatible version {0}; expected {1}.
 mirroring=Mirroring: 
+
+repoFailedWrite=Unable to write to repository: {0}.
+repoReadOnly=Cannot write because repository is read only: {0}
+
 repoMan_exists=Repository already exists at {0}.
 repoMan_failedRead=The repository could not be read: {0}.
 repoMan_internalError=Internal error.
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java
index c1a6b3039..396275f4f 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java
@@ -589,13 +589,13 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 		return DEFAULT_MAX_THREADS;
 	}
 
-	public OutputStream getOutputStream(IArtifactDescriptor descriptor) {
+	public OutputStream getOutputStream(IArtifactDescriptor descriptor) throws ProvisionException {
 		assertModifiable();
-		// TODO we need a better way of distinguishing between errors and cases where 
-		// the stuff just already exists
 		// Check if the artifact is already in this repository
-		if (contains(descriptor))
-			return null;
+		if (contains(descriptor)) {
+			String msg = NLS.bind(Messages.available_already_in, getLocation().toExternalForm());
+			throw new ProvisionException(new Status(IStatus.ERROR, Activator.ID, ProvisionException.ARTIFACT_EXISTS, msg, null));
+		}
 
 		// create a copy of the original descriptor that we can manipulate
 		ArtifactDescriptor newDescriptor = new ArtifactDescriptor(descriptor);
@@ -604,9 +604,6 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 
 		// Determine writing location
 		String newLocation = createLocation(newDescriptor);
-		if (newLocation == null)
-			// TODO: Log an error, or throw an exception?
-			return null;
 		String file = null;
 		try {
 			file = new URL(newLocation).getFile();
@@ -615,12 +612,12 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 			Assert.isTrue(false, "Unexpected failure: " + e1); //$NON-NLS-1$
 		}
 
-		// TODO at this point we have to assume that the repo is file-based.  Eventually 
+		// TODO at this point we have to assume that the repository is file-based.  Eventually 
 		// we should end up with writeable URLs...
 		// Make sure that the file does not exist and that the parents do
 		File outputFile = new File(file);
 		if (outputFile.exists()) {
-			System.err.println("Artifact repository out of synch. Overwriting " + outputFile.getAbsoluteFile()); //$NON-NLS-1$
+			System.err.println("Artifact repository out of sync. Overwriting " + outputFile.getAbsoluteFile()); //$NON-NLS-1$
 			delete(outputFile);
 		}
 
@@ -629,26 +626,27 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 			if (isFolderBased(newDescriptor)) {
 				outputFile.mkdirs();
 				if (!outputFile.exists())
-					// TODO: Log an error, or throw an exception?
-					return null;
+					throw failedWrite(null);
 
 				target = new ZippedFolderOutputStream(outputFile);
 			} else {
 				// file based
 				if (!outputFile.getParentFile().exists() && !outputFile.getParentFile().mkdirs())
-					// TODO: Log an error, or throw an exception?
-					return null;
+					throw failedWrite(null);
 				target = new FileOutputStream(file);
 			}
 
 			// finally create and return an output stream suitably wrapped so that when it is 
 			// closed the repository is updated with the descriptor
-
 			return new ArtifactOutputStream(new BufferedOutputStream(target), newDescriptor, outputFile);
 		} catch (IOException e) {
-			e.printStackTrace();
+			throw failedWrite(e);
 		}
-		return null;
+	}
+
+	private ProvisionException failedWrite(Exception e) throws ProvisionException {
+		String msg = NLS.bind(Messages.repoFailedWrite, getLocation().toExternalForm());
+		throw new ProvisionException(new Status(IStatus.ERROR, Activator.ID, ProvisionException.REPOSITORY_FAILED_WRITE, msg, e));
 	}
 
 	public synchronized String[][] getRules() {