bug 397266 that can't cancel downloading if the cancel is raised in collect phase and before Collect.completePhase is called.v20130104-070547

3 files changed, 93 insertions, 10 deletions

diff --git a/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/EngineSession.java b/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/EngineSession.java
index f61905e3f..cdf1e618a 100644
--- a/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/EngineSession.java
+++ b/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/EngineSession.java
@@ -149,10 +149,11 @@ public class EngineSession {
 			monitor.subTask(Messages.rollingback_error);
 
 		MultiStatus status = new MultiStatus(EngineActivator.ID, IStatus.OK, null, null);
+		SubMonitor sub = SubMonitor.convert(monitor, 100 * (phaseActionRecordsPairs.size() + (currentPhaseActive ? 1 : 0) + 1 /* for touchpoint */));
 
 		if (currentPhaseActive) {
 			try {
-				IStatus result = rollBackPhase(currentPhase, currentActionRecords);
+				IStatus result = rollBackPhase(currentPhase, currentActionRecords, sub.newChild(100));
 				if (!result.isOK())
 					status.add(result);
 			} catch (RuntimeException e) {
@@ -175,7 +176,7 @@ public class EngineSession {
 			@SuppressWarnings("unchecked")
 			List<ActionsRecord> actionRecords = (List<ActionsRecord>) pair[1];
 			try {
-				final IStatus result = rollBackPhase(phase, actionRecords);
+				final IStatus result = rollBackPhase(phase, actionRecords, sub.newChild(100));
 				if (!result.isOK())
 					status.add(result);
 			} catch (RuntimeException e) {
@@ -189,6 +190,7 @@ public class EngineSession {
 		}
 
 		phaseActionRecordsPairs.clear();
+		SubMonitor touchpointSub = sub.setWorkRemaining(100).newChild(100).setWorkRemaining(touchpoints.size() + 1);
 		for (Touchpoint touchpoint : touchpoints) {
 			try {
 				IStatus result = touchpoint.rollback(profile);
@@ -201,24 +203,31 @@ public class EngineSession {
 			} catch (LinkageError e) {
 				// Catch linkage errors as these are generally recoverable but let other Errors propagate (see bug 222001)
 				status.add(new Status(IStatus.ERROR, EngineActivator.ID, NLS.bind(Messages.touchpoint_rollback_error, touchpoint.getClass().getName()), e));
+			} finally {
+				touchpointSub.worked(1);
 			}
 		}
 
-		if (status.matches(IStatus.ERROR)) {
-			MultiStatus result = new MultiStatus(EngineActivator.ID, IStatus.ERROR, NLS.bind(Messages.session_commit_error, profile.getProfileId()), null);
-			result.merge(status);
-			return result;
+		try {
+			if (status.matches(IStatus.ERROR)) {
+				MultiStatus result = new MultiStatus(EngineActivator.ID, IStatus.ERROR, NLS.bind(Messages.session_commit_error, profile.getProfileId()), null);
+				result.merge(status);
+				return result;
+			}
+			return status;
+		} finally {
+			touchpointSub.worked(1);
 		}
-		return status;
 	}
 
-	private IStatus rollBackPhase(Phase phase, List<ActionsRecord> actionRecords) {
+	private IStatus rollBackPhase(Phase phase, List<ActionsRecord> actionRecords, IProgressMonitor monitor) {
 		MultiStatus result = new MultiStatus(EngineActivator.ID, IStatus.OK, null, null);
+		SubMonitor sub = SubMonitor.convert(monitor, 10 + 10 + 10 * actionRecords.size());
 		try {
 			phase.actionManager = (ActionManager) agent.getService(ActionManager.SERVICE_NAME);
 
 			if (!currentPhaseActive)
-				phase.prePerform(result, this, new NullProgressMonitor());
+				phase.prePerform(result, this, sub.newChild(10));
 
 			for (ListIterator<ActionsRecord> it = actionRecords.listIterator(actionRecords.size()); it.hasPrevious();) {
 				ActionsRecord record = it.previous();
@@ -234,9 +243,11 @@ public class EngineSession {
 				} catch (LinkageError e) {
 					// Catch linkage errors as these are generally recoverable but let other Errors propagate (see bug 222001)
 					result.add(new Status(IStatus.ERROR, EngineActivator.ID, NLS.bind(Messages.phase_undo_operand_error, phase.getClass().getName(), record.operand), e));
+				} finally {
+					sub.worked(10);
 				}
 			}
-			phase.postPerform(result, this, new NullProgressMonitor());
+			phase.postPerform(result, this, sub.setWorkRemaining(10).newChild(10));
 		} finally {
 			phase.actionManager = null;
 		}
diff --git a/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/phases/Collect.java b/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/phases/Collect.java
index 640bf0807..e53c94754 100644
--- a/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/phases/Collect.java
+++ b/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/phases/Collect.java
@@ -67,6 +67,9 @@ public class Collect extends InstallableUnitPhase {
 	}
 
 	protected IStatus completePhase(IProgressMonitor monitor, IProfile profile, Map<String, Object> parameters) {
+		// do nothing for rollback if the provisioning has been cancelled
+		if (monitor.isCanceled())
+			return Status.OK_STATUS;
 		@SuppressWarnings("unchecked")
 		List<IArtifactRequest[]> artifactRequests = (List<IArtifactRequest[]>) parameters.get(PARM_ARTIFACT_REQUESTS);
 		// it happens when rollbacking
diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/engine/PhaseTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/engine/PhaseTest.java
index 9b20dc28d..0ccdbe100 100644
--- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/engine/PhaseTest.java
+++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/engine/PhaseTest.java
@@ -15,9 +15,15 @@ import java.io.IOException;
 import java.util.*;
 import org.eclipse.core.runtime.*;
 import org.eclipse.equinox.internal.p2.engine.*;
+import org.eclipse.equinox.internal.p2.engine.phases.Collect;
+import org.eclipse.equinox.internal.provisional.p2.core.eventbus.ProvisioningListener;
 import org.eclipse.equinox.p2.engine.*;
 import org.eclipse.equinox.p2.engine.spi.ProvisioningAction;
 import org.eclipse.equinox.p2.metadata.*;
+import org.eclipse.equinox.p2.query.QueryUtil;
+import org.eclipse.equinox.p2.repository.artifact.IArtifactRepositoryManager;
+import org.eclipse.equinox.p2.repository.metadata.IMetadataRepository;
+import org.eclipse.equinox.p2.repository.metadata.IMetadataRepositoryManager;
 import org.eclipse.equinox.p2.tests.AbstractProvisioningTest;
 
 /**
@@ -303,4 +309,67 @@ public class PhaseTest extends AbstractProvisioningTest {
 		assertEquals(TestAction.class, ((ParameterizedProvisioningAction) actionsList2.get(0)).getAction().getClass());
 	}
 
+	public void testCancelHappenBeforeCompleteCollectPhase() {
+		final String testDataLocation = "testData/mirror/mirrorSourceRepo3";
+		Set<IInstallableUnit> ius = null;
+		try {
+			IArtifactRepositoryManager mgr = getArtifactRepositoryManager();
+			mgr.loadRepository((getTestData("test artifact repo", testDataLocation).toURI()), null);
+			IMetadataRepositoryManager metaManager = getMetadataRepositoryManager();
+			IMetadataRepository metaRepo = metaManager.loadRepository((getTestData("test metadata repo", testDataLocation).toURI()), null);
+			ius = metaRepo.query(QueryUtil.ALL_UNITS, null).toUnmodifiableSet();
+		} catch (Exception e) {
+			fail("1.0", e);
+		}
+		class MyCollect extends Collect {
+			boolean isCancelled = false;
+			int progress = 0;
+			final static int THREHOLD = 2;
+
+			public MyCollect(int weight) {
+				super(weight);
+			}
+
+			@Override
+			protected List<ProvisioningAction> getActions(InstallableUnitOperand operand) {
+				List<ProvisioningAction> actions = super.getActions(operand);
+				if (actions != null)
+					progress++;
+				if (progress > THREHOLD)
+					isCancelled = true;
+				return actions;
+			}
+		}
+		final MyCollect collect = new MyCollect(100);
+		PhaseSet phaseSet = new TestPhaseSet(new Phase[] {collect});
+		IProfile profile = createProfile("PhaseTest");
+		IProvisioningPlan plan = engine.createPlan(profile, null);
+		for (IInstallableUnit iu : ius)
+			plan.addInstallableUnit(iu);
+		class TestListener implements ProvisioningListener {
+			boolean collectEvent = false;
+
+			public void notify(EventObject o) {
+				if (o instanceof CollectEvent)
+					collectEvent = true;
+			}
+
+		}
+		TestListener listener = new TestListener();
+		getEventBus().addListener(listener);
+		try {
+			IStatus status = engine.perform(plan, phaseSet, new NullProgressMonitor() {
+				@Override
+				public boolean isCanceled() {
+					return collect.isCancelled;
+				}
+			});
+			if (!status.matches(IStatus.CANCEL)) {
+				fail(status.toString());
+			}
+			assertFalse("Collect actually happened!", listener.collectEvent);
+		} finally {
+			getEventBus().removeListener(listener);
+		}
+	}
 }