Bug 290858 - [ui] Checking for updates from the about dialog blocks the UI

10 files changed, 129 insertions, 41 deletions

diff --git a/bundles/org.eclipse.equinox.p2.operations/src/org/eclipse/equinox/internal/p2/operations/SearchForUpdatesResolutionJob.java b/bundles/org.eclipse.equinox.p2.operations/src/org/eclipse/equinox/internal/p2/operations/SearchForUpdatesResolutionJob.java
new file mode 100644
index 000000000..b62c46c75
--- /dev/null
+++ b/bundles/org.eclipse.equinox.p2.operations/src/org/eclipse/equinox/internal/p2/operations/SearchForUpdatesResolutionJob.java
@@ -0,0 +1,54 @@
+/*******************************************************************************
+ *  Copyright (c) 2010 IBM Corporation and others.
+ *  All rights reserved. This program and the accompanying materials
+ *  are made available under the terms of the Eclipse Public License v1.0
+ *  which accompanies this distribution, and is available at
+ *  http://www.eclipse.org/legal/epl-v10.html
+ * 
+ *  Contributors:
+ *     IBM Corporation - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.equinox.internal.p2.operations;
+
+import java.lang.reflect.InvocationTargetException;
+import org.eclipse.core.runtime.*;
+import org.eclipse.equinox.internal.provisional.p2.director.ProfileChangeRequest;
+import org.eclipse.equinox.p2.engine.ProvisioningContext;
+import org.eclipse.equinox.p2.operations.ProvisioningSession;
+import org.eclipse.equinox.p2.operations.UpdateOperation;
+import org.eclipse.equinox.p2.repository.IRunnableWithProgress;
+
+public class SearchForUpdatesResolutionJob extends PlannerResolutionJob {
+
+	IRunnableWithProgress searchForUpdatesRunnable;
+	ProfileChangeRequest[] requestHolder;
+	UpdateOperation operation;
+
+	public SearchForUpdatesResolutionJob(String label, ProvisioningSession session, String profileId, ProfileChangeRequest request, ProvisioningContext provisioningContext, MultiStatus additionalStatus, IRunnableWithProgress searchForUpdatesRunnable, ProfileChangeRequest[] requestHolder, UpdateOperation operation) {
+		super(label, session, profileId, request, provisioningContext, additionalStatus);
+		this.searchForUpdatesRunnable = searchForUpdatesRunnable;
+		this.requestHolder = requestHolder;
+		this.operation = operation;
+	}
+
+	public IStatus runModal(IProgressMonitor monitor) {
+		SubMonitor sub = SubMonitor.convert(monitor);
+		try {
+			searchForUpdatesRunnable.run(sub.newChild(500));
+			if (requestHolder.length > 0)
+				this.request = requestHolder[0];
+		} catch (OperationCanceledException e) {
+			return Status.CANCEL_STATUS;
+		} catch (InvocationTargetException e) {
+			// ignore, we don't actually throw this in the supplied runnable
+		}
+		if (request != null)
+			return super.runModal(sub.newChild(500));
+		return operation.getResolutionResult();
+	}
+
+	// This is made public for the automated tests
+	public ProfileChangeRequest getProfileChangeRequest() {
+		return request;
+	}
+}
diff --git a/bundles/org.eclipse.equinox.p2.operations/src/org/eclipse/equinox/p2/operations/ProfileChangeOperation.java b/bundles/org.eclipse.equinox.p2.operations/src/org/eclipse/equinox/p2/operations/ProfileChangeOperation.java
index ae6519cfc..d6502066c 100644
--- a/bundles/org.eclipse.equinox.p2.operations/src/org/eclipse/equinox/p2/operations/ProfileChangeOperation.java
+++ b/bundles/org.eclipse.equinox.p2.operations/src/org/eclipse/equinox/p2/operations/ProfileChangeOperation.java
@@ -150,7 +150,7 @@ public abstract class ProfileChangeOperation implements IProfileChangeJob {
 		// default is to do nothing
 	}
 
-	private void makeResolveJob(IProgressMonitor monitor) {
+	void makeResolveJob(IProgressMonitor monitor) {
 		noChangeRequest = PlanAnalyzer.getProfileChangeAlteredStatus();
 		if (session.hasScheduledOperationsFor(profileId)) {
 			noChangeRequest.add(PlanAnalyzer.getStatus(IStatusCodes.OPERATION_ALREADY_IN_PROGRESS, null));
@@ -204,14 +204,17 @@ public abstract class ProfileChangeOperation implements IProfileChangeJob {
 	 * if resolution has not yet occurred.
 	 */
 	public IStatus getResolutionResult() {
+		if (request == null) {
+			if (noChangeRequest != null) {
+				// If there is only one child message, use the specific message
+				if (noChangeRequest.getChildren().length == 1)
+					return noChangeRequest.getChildren()[0];
+				return noChangeRequest;
+			}
+			return null;
+		}
 		if (job != null && job.getResolutionResult() != null)
 			return job.getResolutionResult().getSummaryStatus();
-		if (request == null && noChangeRequest != null) {
-			// If there is only one child message, use the specific message
-			if (noChangeRequest.getChildren().length == 1)
-				return noChangeRequest.getChildren()[0];
-			return noChangeRequest;
-		}
 		return null;
 	}
 
diff --git a/bundles/org.eclipse.equinox.p2.operations/src/org/eclipse/equinox/p2/operations/UpdateOperation.java b/bundles/org.eclipse.equinox.p2.operations/src/org/eclipse/equinox/p2/operations/UpdateOperation.java
index 45c135df8..f37afb547 100644
--- a/bundles/org.eclipse.equinox.p2.operations/src/org/eclipse/equinox/p2/operations/UpdateOperation.java
+++ b/bundles/org.eclipse.equinox.p2.operations/src/org/eclipse/equinox/p2/operations/UpdateOperation.java
@@ -14,6 +14,7 @@ package org.eclipse.equinox.p2.operations;
 
 import java.util.*;
 import org.eclipse.core.runtime.*;
+import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.equinox.internal.p2.core.helpers.CollectionUtils;
 import org.eclipse.equinox.internal.p2.operations.*;
 import org.eclipse.equinox.internal.provisional.p2.director.ProfileChangeRequest;
@@ -22,6 +23,7 @@ import org.eclipse.equinox.p2.engine.query.UserVisibleRootQuery;
 import org.eclipse.equinox.p2.metadata.IInstallableUnit;
 import org.eclipse.equinox.p2.planner.ProfileInclusionRules;
 import org.eclipse.equinox.p2.query.*;
+import org.eclipse.equinox.p2.repository.IRunnableWithProgress;
 
 /**
  * An UpdateOperation describes an operation that updates {@link IInstallableUnit}s in
@@ -299,4 +301,43 @@ public class UpdateOperation extends ProfileChangeOperation {
 		return queryResult.toUnmodifiableSet();
 	}
 
+	/*
+	 * Overridden to delay computation of the profile change request until the resolution
+	 * occurs.  This is done because computing the request is expensive (it involves searching
+	 * for updates).
+	 * (non-Javadoc)
+	 * @see org.eclipse.equinox.p2.operations.ProfileChangeOperation#makeResolveJob(org.eclipse.core.runtime.IProgressMonitor)
+	 */
+	void makeResolveJob(IProgressMonitor monitor) {
+		// throw away any previous requests
+		request = null;
+		noChangeRequest = PlanAnalyzer.getProfileChangeAlteredStatus();
+		// the requestHolder is a hack to work around the fact that there is no public API
+		// for the resolution job to get the request from the operation after it has been
+		// computed.
+		final ProfileChangeRequest[] requestHolder = new ProfileChangeRequest[1];
+		job = new SearchForUpdatesResolutionJob(getResolveJobName(), session, profileId, request, context, noChangeRequest, new IRunnableWithProgress() {
+			public void run(IProgressMonitor mon) throws OperationCanceledException {
+				// We only check for other jobs running if this job is *not* scheduled
+				if (job.getState() == Job.NONE && session.hasScheduledOperationsFor(profileId)) {
+					noChangeRequest.add(PlanAnalyzer.getStatus(IStatusCodes.OPERATION_ALREADY_IN_PROGRESS, null));
+				} else {
+					computeProfileChangeRequest(noChangeRequest, mon);
+					requestHolder[0] = UpdateOperation.this.request;
+				}
+			}
+		}, requestHolder, this);
+	}
+
+	/*
+	 * Overridden because our resolution job life cycle is different.  We have a job
+	 * before we've computed the profile change request, so we must ensure that we
+	 * have already computed the profile change request.
+	 * 
+	 * (non-Javadoc)
+	 * @see org.eclipse.equinox.p2.operations.ProfileChangeOperation#hasResolved()
+	 */
+	public boolean hasResolved() {
+		return request != null && super.hasResolved();
+	}
 }
diff --git a/bundles/org.eclipse.equinox.p2.tests.ui/META-INF/MANIFEST.MF b/bundles/org.eclipse.equinox.p2.tests.ui/META-INF/MANIFEST.MF
index 6c1f3d207..af2a3c342 100644
--- a/bundles/org.eclipse.equinox.p2.tests.ui/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.equinox.p2.tests.ui/META-INF/MANIFEST.MF
@@ -20,4 +20,5 @@ Require-Bundle: org.eclipse.core.runtime;bundle-version="3.4.100",
  org.eclipse.equinox.p2.tests;bundle-version="1.2.0"
 Bundle-RequiredExecutionEnvironment: J2SE-1.5
 Import-Package: org.eclipse.equinox.internal.p2.ui.sdk,
- org.eclipse.equinox.p2.operations;version="[2.0.0,3.0.0)"
+ org.eclipse.equinox.p2.operations;version="[2.0.0,3.0.0)",
+ org.eclipse.equinox.internal.p2.operations
diff --git a/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/AutomatedTests.java b/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/AutomatedTests.java
index 98ff2020f..3bf03d037 100644
--- a/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/AutomatedTests.java
+++ b/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/AutomatedTests.java
@@ -25,7 +25,6 @@ public class AutomatedTests extends TestCase {
 		suite.addTest(org.eclipse.equinox.p2.tests.ui.dialogs.AllTests.suite());
 		suite.addTest(org.eclipse.equinox.p2.tests.ui.misc.AllTests.suite());
 		suite.addTest(org.eclipse.equinox.p2.tests.ui.repohandling.AllTests.suite());
-		suite.addTest(org.eclipse.equinox.p2.tests.ui.planning.AllTests.suite());
 		return suite;
 	}
 }
diff --git a/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/dialogs/UpdateWizardTest.java b/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/dialogs/UpdateWizardTest.java
index 0b37fae3d..e6ea8a3e9 100644
--- a/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/dialogs/UpdateWizardTest.java
+++ b/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/dialogs/UpdateWizardTest.java
@@ -10,14 +10,12 @@
  *******************************************************************************/
 package org.eclipse.equinox.p2.tests.ui.dialogs;
 
-import org.eclipse.equinox.p2.metadata.MetadataFactory;
-import org.eclipse.equinox.p2.metadata.MetadataFactory.InstallableUnitDescription;
-
 import java.util.ArrayList;
 import org.eclipse.equinox.internal.p2.metadata.License;
 import org.eclipse.equinox.internal.p2.ui.ProvUI;
 import org.eclipse.equinox.internal.p2.ui.dialogs.*;
 import org.eclipse.equinox.p2.metadata.*;
+import org.eclipse.equinox.p2.metadata.MetadataFactory.InstallableUnitDescription;
 import org.eclipse.equinox.p2.operations.ProfileModificationJob;
 import org.eclipse.equinox.p2.operations.UpdateOperation;
 import org.eclipse.jface.dialogs.IDialogConstants;
diff --git a/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/operations/AllTests.java b/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/operations/AllTests.java
index 331cf99a5..6c7724ebf 100644
--- a/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/operations/AllTests.java
+++ b/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/operations/AllTests.java
@@ -21,6 +21,7 @@ public class AllTests extends TestCase {
 		TestSuite suite = new TestSuite(AllTests.class.getName());
 		suite.addTestSuite(SizingTest.class);
 		suite.addTestSuite(InstallerPlanTest.class);
+		suite.addTestSuite(UpdatePlanning.class);
 		return suite;
 	}
 }
diff --git a/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/planning/UpdatePlanning.java b/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/operations/UpdatePlanning.java
index b7f8ebfc2..5b39df82e 100644
--- a/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/planning/UpdatePlanning.java
+++ b/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/operations/UpdatePlanning.java
@@ -8,15 +8,15 @@
  * Contributors:
  *     IBM Corporation - initial API and implementation
  *******************************************************************************/
-package org.eclipse.equinox.p2.tests.ui.planning;
+package org.eclipse.equinox.p2.tests.ui.operations;
 
 import java.util.ArrayList;
 import java.util.HashSet;
+import org.eclipse.equinox.internal.p2.operations.SearchForUpdatesResolutionJob;
 import org.eclipse.equinox.internal.provisional.p2.director.ProfileChangeRequest;
 import org.eclipse.equinox.p2.core.ProvisionException;
 import org.eclipse.equinox.p2.metadata.*;
-import org.eclipse.equinox.p2.operations.Update;
-import org.eclipse.equinox.p2.operations.UpdateOperation;
+import org.eclipse.equinox.p2.operations.*;
 import org.eclipse.equinox.p2.planner.IProfileChangeRequest;
 import org.eclipse.equinox.p2.tests.ui.AbstractProvisioningUITest;
 
@@ -181,6 +181,22 @@ public class UpdatePlanning extends AbstractProvisioningUITest {
 		op.setSelectedUpdates(new Update[] {updates[0]});
 		op.resolveModal(getMonitor());
 		assertEquals("1.1", 1, op.getSelectedUpdates().length);
+	}
+
+	// bug 290858
+	public void testSearchForUpdatesInJob() throws ProvisionException {
+		createTestMetdataRepository(new IInstallableUnit[] {a1, a130, b1, b12});
+		install(a1, true, false);
+		ArrayList<IInstallableUnit> iusInvolved = new ArrayList<IInstallableUnit>();
+		iusInvolved.add(a1);
+		iusInvolved.add(b1);
+		UpdateOperation op = getProvisioningUI().getUpdateOperation(iusInvolved, null);
+		ProvisioningJob job = op.getResolveJob(getMonitor());
+		assertTrue("1.0", job instanceof SearchForUpdatesResolutionJob);
+		// getting the job should not compute the request.
+		assertNull("1.1", ((SearchForUpdatesResolutionJob) job).getProfileChangeRequest());
+		job.runModal(getMonitor());
+		assertNotNull("1.2", ((SearchForUpdatesResolutionJob) job).getProfileChangeRequest());
 
 	}
 }
diff --git a/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/planning/AllTests.java b/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/planning/AllTests.java
deleted file mode 100644
index 5b8bc2624..000000000
--- a/bundles/org.eclipse.equinox.p2.tests.ui/src/org/eclipse/equinox/p2/tests/ui/planning/AllTests.java
+++ /dev/null
@@ -1,25 +0,0 @@
-/*******************************************************************************
- *  Copyright (c) 2008, 2009 IBM Corporation and others.
- *  All rights reserved. This program and the accompanying materials
- *  are made available under the terms of the Eclipse Public License v1.0
- *  which accompanies this distribution, and is available at
- *  http://www.eclipse.org/legal/epl-v10.html
- * 
- *  Contributors:
- *     IBM Corporation - initial API and implementation
- *******************************************************************************/
-package org.eclipse.equinox.p2.tests.ui.planning;
-
-import junit.framework.*;
-
-/**
- * Performs all UI action tests.
- */
-public class AllTests extends TestCase {
-
-	public static Test suite() {
-		TestSuite suite = new TestSuite(AllTests.class.getName());
-		suite.addTestSuite(UpdatePlanning.class);
-		return suite;
-	}
-}
diff --git a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/Policy.java b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/Policy.java
index fcf3a5d0f..4aab2bbc4 100644
--- a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/Policy.java
+++ b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/Policy.java
@@ -94,7 +94,7 @@ public class Policy {
 	 * not.  It is up to the implementor to report any errors to the user when answering <code>false</code>.
 	 */
 	public boolean continueWorkingWithOperation(ProfileChangeOperation operation, Shell shell) {
-		Assert.isTrue(operation.hasResolved());
+		Assert.isTrue(operation.getResolutionResult() != null);
 		IStatus status = operation.getResolutionResult();
 		// user cancelled
 		if (status.getSeverity() == IStatus.CANCEL)