Bug 234016 - Half an artifact written to my bundlepool

4 files changed, 99 insertions, 32 deletions

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 4905294cf..03084c2b7 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
@@ -107,7 +107,15 @@ public class MirrorRequest extends ArtifactRequest {
 			destinationDescriptor.addRepositoryProperties(targetRepositoryProperties);
 
 		IStatus status = transfer(destinationDescriptor, descriptor, monitor);
-		// if ok or transfer has already been done with the canonical form return with status set 
+		// if ok, cancelled or transfer has already been done with the canonical form return with status set 
+		if (status.getSeverity() == IStatus.CANCEL) {
+			setResult(status);
+			return;
+		}
+		if (monitor.isCanceled()) {
+			setResult(Status.CANCEL_STATUS);
+			return;
+		}
 		if (status.isOK() || descriptor == canonical || canonical == null) {
 			setResult(status);
 			return;
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 2130112c1..c4a6c36fa 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
@@ -54,6 +54,7 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 		private OutputStream destination;
 		private File file;
 		private IStatus status = Status.OK_STATUS;
+		private OutputStream firstLink;
 
 		public ArtifactOutputStream(OutputStream os, IArtifactDescriptor descriptor) {
 			this(os, descriptor, null);
@@ -85,7 +86,8 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 			// TODO the count check is a bit bogus but helps in some error cases (e.g., the optimizer)
 			// where errors occurred in a processing step earlier in the chain.  We likely need a better
 			// or more explicit way of handling this case.
-			if (getStatus().isOK() && ProcessingStepHandler.checkStatus(destination).isOK() && count > 0) {
+			OutputStream testStream = firstLink == null ? this : firstLink;
+			if (ProcessingStepHandler.checkStatus(testStream).isOK() && count > 0) {
 				((ArtifactDescriptor) descriptor).setProperty(IArtifactDescriptor.DOWNLOAD_SIZE, Long.toString(count));
 				addDescriptor(descriptor);
 			} else if (file != null)
@@ -97,6 +99,10 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 			return status;
 		}
 
+		public OutputStream getDestination() {
+			return destination;
+		}
+
 		public void setStatus(IStatus status) {
 			this.status = status == null ? Status.OK_STATUS : status;
 		}
@@ -115,6 +121,10 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 			destination.write(b);
 			count++;
 		}
+
+		public void setFirstLink(OutputStream value) {
+			firstLink = value;
+		}
 	}
 
 	// TODO: optimize
@@ -298,6 +308,7 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 
 	private OutputStream addPreSteps(ProcessingStepHandler handler, IArtifactDescriptor descriptor, OutputStream destination, IProgressMonitor monitor) {
 		ArrayList steps = new ArrayList();
+		//		steps.add(new ZipVerifierStep());
 		// Add steps here if needed
 		if (steps.isEmpty())
 			return destination;
@@ -409,6 +420,10 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 		IStatus result = getTransport().download(mirrorLocation, destination, monitor);
 		if (mirrors != null)
 			mirrors.reportResult(mirrorLocation, result);
+		if (result.getSeverity() == IStatus.CANCEL)
+			return result;
+		if (monitor.isCanceled())
+			return Status.CANCEL_STATUS;
 		if (result.isOK() || baseLocation.equals(mirrorLocation))
 			return result;
 		//maybe we hit a bad mirror - try the base location
@@ -756,7 +771,7 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
 			return new Status(IStatus.ERROR, Activator.ID, NLS.bind(Messages.sar_reportStatus, descriptor.getArtifactKey().toExternalForm()), e);
 		}
 
-		IStatus stepStatus = ((ProcessingStep) destination).getStatus(true);
+		IStatus stepStatus = ProcessingStepHandler.getStatus(destination, true);
 		// if the steps all ran ok and there is no interesting information, return the status from this method
 		if (!stepStatus.isMultiStatus() && stepStatus.isOK())
 			return status;
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/provisional/p2/artifact/repository/processing/ProcessingStep.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/provisional/p2/artifact/repository/processing/ProcessingStep.java
index d6126e1ad..d1eab65f4 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/provisional/p2/artifact/repository/processing/ProcessingStep.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/provisional/p2/artifact/repository/processing/ProcessingStep.java
@@ -13,9 +13,7 @@ package org.eclipse.equinox.internal.provisional.p2.artifact.repository.processi
 
 import java.io.IOException;
 import java.io.OutputStream;
-import java.util.ArrayList;
 import org.eclipse.core.runtime.*;
-import org.eclipse.equinox.internal.p2.artifact.repository.Activator;
 import org.eclipse.equinox.internal.provisional.p2.artifact.repository.IArtifactDescriptor;
 import org.eclipse.equinox.internal.provisional.p2.artifact.repository.IStateful;
 
@@ -90,8 +88,12 @@ public abstract class ProcessingStep extends OutputStream implements IStateful {
 		return status;
 	}
 
-	public void setStatus(IStatus status) {
-		this.status = status == null ? Status.OK_STATUS : status;
+	public void setStatus(IStatus value) {
+		if (value == null)
+			value = Status.OK_STATUS;
+		if (status != null && status.getSeverity() >= value.getSeverity())
+			return;
+		status = value;
 	}
 
 	/**
@@ -119,26 +121,6 @@ public abstract class ProcessingStep extends OutputStream implements IStateful {
 	 * @return the requested status 
 	 */
 	public IStatus getStatus(boolean deep) {
-		if (!deep)
-			return getStatus();
-		ArrayList list = new ArrayList();
-		int severity = collectStatus(list);
-		if (severity == IStatus.OK)
-			return Status.OK_STATUS;
-		IStatus[] result = (IStatus[]) list.toArray(new IStatus[list.size()]);
-		return new MultiStatus(Activator.ID, severity, result, "Result of processing steps", null);
-	}
-
-	private int collectStatus(ArrayList list) {
-		list.add(getStatus());
-		if (!(destination instanceof ProcessingStep))
-			return getStatus().getSeverity();
-		int result = ((ProcessingStep) destination).collectStatus(list);
-		// TODO greater than test here is a little brittle but it is very unlikely that we will add
-		// a new status severity.
-		if (getStatus().getSeverity() > result)
-			return getStatus().getSeverity();
-		return result;
-
+		return ProcessingStepHandler.getStatus(this, deep);
 	}
 }
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/provisional/p2/artifact/repository/processing/ProcessingStepHandler.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/provisional/p2/artifact/repository/processing/ProcessingStepHandler.java
index c003820a7..3b29aff0e 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/provisional/p2/artifact/repository/processing/ProcessingStepHandler.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/provisional/p2/artifact/repository/processing/ProcessingStepHandler.java
@@ -12,9 +12,12 @@
 package org.eclipse.equinox.internal.provisional.p2.artifact.repository.processing;
 
 import java.io.OutputStream;
+import java.util.ArrayList;
 import org.eclipse.core.runtime.*;
 import org.eclipse.equinox.internal.p2.artifact.repository.Activator;
+import org.eclipse.equinox.internal.p2.artifact.repository.simple.SimpleArtifactRepository.ArtifactOutputStream;
 import org.eclipse.equinox.internal.provisional.p2.artifact.repository.IArtifactDescriptor;
+import org.eclipse.equinox.internal.provisional.p2.artifact.repository.IStateful;
 
 /**
  * Creates processing step instances from extensions and executes them.
@@ -22,11 +25,9 @@ import org.eclipse.equinox.internal.provisional.p2.artifact.repository.IArtifact
 public class ProcessingStepHandler {
 
 	private static final String PROCESSING_STEPS_EXTENSION_ID = "org.eclipse.equinox.p2.artifact.repository.processingSteps"; //$NON-NLS-1$
-
+	//TODO This method can go
 	public static IStatus checkStatus(OutputStream output) {
-		if (!(output instanceof ProcessingStep))
-			return Status.OK_STATUS;
-		return ((ProcessingStep) output).getStatus(true);
+		return getStatus(output, true);
 	}
 
 	/**
@@ -47,6 +48,50 @@ public class ProcessingStepHandler {
 		return true;
 	}
 
+	/**
+	 * Return the status of this step.  The status will be <code>null</code> if the
+	 * step has not yet executed. If the step has executed the returned status
+	 * indicates the success or failure of the step.
+	 * @param deep whether or not to aggregate the status of any linked steps
+	 * @return the requested status 
+	 */
+	public static IStatus getStatus(OutputStream stream, boolean deep) {
+		if (!deep)
+			return getStatus(stream);
+		ArrayList list = new ArrayList();
+		int severity = collectStatus(stream, list);
+		if (severity == IStatus.OK)
+			return Status.OK_STATUS;
+		IStatus[] result = (IStatus[]) list.toArray(new IStatus[list.size()]);
+		return new MultiStatus(Activator.ID, severity, result, "Result of processing steps", null);
+	}
+
+	public static IStatus getStatus(OutputStream stream) {
+		if (stream instanceof IStateful)
+			return ((IStateful) stream).getStatus();
+		return Status.OK_STATUS;
+	}
+
+	private static int collectStatus(OutputStream stream, ArrayList list) {
+		IStatus status = getStatus(stream);
+		list.add(status);
+		OutputStream destination = getDestination(stream);
+		if (destination == null || !(destination instanceof IStateful))
+			return status.getSeverity();
+		int result = collectStatus(destination, list);
+		// TODO greater than test here is a little brittle but it is very unlikely that we will add
+		// a new status severity.
+		return status.getSeverity() > result ? status.getSeverity() : result;
+	}
+
+	private static OutputStream getDestination(OutputStream stream) {
+		if (stream instanceof ProcessingStep)
+			return ((ProcessingStep) stream).getDestination();
+		if (stream instanceof ArtifactOutputStream)
+			return ((ArtifactOutputStream) stream).getDestination();
+		return null;
+	}
+
 	public ProcessingStep[] create(ProcessingStepDescriptor[] descriptors, IArtifactDescriptor context) {
 		ProcessingStep[] result = new ProcessingStep[descriptors.length];
 		for (int i = 0; i < descriptors.length; i++)
@@ -91,9 +136,26 @@ public class ProcessingStepHandler {
 			step.link(previous, monitor);
 			previous = step;
 		}
+		if (steps.length == 0)
+			return previous;
+		// now link the artifact stream to the first stream in the new chain 
+		ArtifactOutputStream lastLink = getArtifactStream(previous);
+		if (lastLink != null)
+			lastLink.setFirstLink(previous);
 		return previous;
 	}
 
+	// Traverse the chain of processing steps and return the stream served up by
+	// the artifact repository or null if one cannot be found.
+	private ArtifactOutputStream getArtifactStream(OutputStream stream) {
+		OutputStream current = stream;
+		while (current instanceof ProcessingStep)
+			current = ((ProcessingStep) current).getDestination();
+		if (current instanceof ArtifactOutputStream)
+			return (ArtifactOutputStream) current;
+		return null;
+	}
+
 	protected static final class EmptyProcessingStep extends ProcessingStep {
 		// Just to hold the status
 	}