Bug 207976 - [prov] Installation does not fail when artifacts are missingv20071031-1750

4 files changed, 69 insertions, 84 deletions

diff --git a/bundles/org.eclipse.equinox.p2.ui.admin.rcp/src/org/eclipse/equinox/p2/ui/admin/rcp/ApplicationWorkbenchWindowAdvisor.java b/bundles/org.eclipse.equinox.p2.ui.admin.rcp/src/org/eclipse/equinox/p2/ui/admin/rcp/ApplicationWorkbenchWindowAdvisor.java
index 28921e22e..7216a9ca9 100644
--- a/bundles/org.eclipse.equinox.p2.ui.admin.rcp/src/org/eclipse/equinox/p2/ui/admin/rcp/ApplicationWorkbenchWindowAdvisor.java
+++ b/bundles/org.eclipse.equinox.p2.ui.admin.rcp/src/org/eclipse/equinox/p2/ui/admin/rcp/ApplicationWorkbenchWindowAdvisor.java
@@ -12,6 +12,7 @@ package org.eclipse.equinox.p2.ui.admin.rcp;
 
 import org.eclipse.equinox.internal.p2.ui.admin.rcp.ProvAdminUIMessages;
 import org.eclipse.swt.graphics.Point;
+import org.eclipse.ui.PlatformUI;
 import org.eclipse.ui.application.*;
 
 public class ApplicationWorkbenchWindowAdvisor extends WorkbenchWindowAdvisor {
@@ -31,5 +32,9 @@ public class ApplicationWorkbenchWindowAdvisor extends WorkbenchWindowAdvisor {
 		configurer.setShowStatusLine(false);
 		configurer.setShowProgressIndicator(true);
 		configurer.setTitle(ProvAdminUIMessages.ApplicationWindowTitle);
+		// The workbench progress manager won't get initialized until I refer to it.
+		// Refer to it so that jobs will get their fail cases reported by the progress service.
+		// TODO open a platform UI bug since this relies on a side effect
+		PlatformUI.getWorkbench().getProgressService();
 	}
 }
diff --git a/bundles/org.eclipse.equinox.p2.ui.admin/src/org/eclipse/equinox/internal/p2/ui/admin/ProfilesView.java b/bundles/org.eclipse.equinox.p2.ui.admin/src/org/eclipse/equinox/internal/p2/ui/admin/ProfilesView.java
index a3dc706a5..ed870c469 100644
--- a/bundles/org.eclipse.equinox.p2.ui.admin/src/org/eclipse/equinox/internal/p2/ui/admin/ProfilesView.java
+++ b/bundles/org.eclipse.equinox.p2.ui.admin/src/org/eclipse/equinox/internal/p2/ui/admin/ProfilesView.java
@@ -61,10 +61,7 @@ public class ProfilesView extends ProvView {
 				}
 			}
 			ProfileOperation op = new RemoveProfilesOperation(ProvAdminUIMessages.Ops_RemoveProfileOperationLabel, (Profile[]) profilesOnly.toArray(new Profile[profilesOnly.size()]));
-			ProvisioningOperationResult result = ProvisioningOperationRunner.execute(op, ProfilesView.this.getShell(), null);
-			if (result.getStatus().isOK()) {
-				viewer.refresh();
-			}
+			ProvisioningOperationRunner.execute(op, ProfilesView.this.getShell(), null);
 		}
 	}
 
diff --git a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/ProvisioningOperationRunner.java b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/ProvisioningOperationRunner.java
index bbb709a41..086251223 100644
--- a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/ProvisioningOperationRunner.java
+++ b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/ProvisioningOperationRunner.java
@@ -11,16 +11,16 @@
 
 package org.eclipse.equinox.p2.ui;
 
-import java.lang.reflect.InvocationTargetException;
 import org.eclipse.core.commands.ExecutionException;
 import org.eclipse.core.commands.operations.IUndoableOperation;
-import org.eclipse.core.runtime.*;
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.equinox.p2.ui.operations.ProvisioningOperation;
 import org.eclipse.jface.operation.IRunnableContext;
-import org.eclipse.jface.operation.IRunnableWithProgress;
 import org.eclipse.swt.widgets.Shell;
 import org.eclipse.ui.PlatformUI;
+import org.eclipse.ui.progress.WorkbenchJob;
 
 /**
  * Utility methods for running provisioning operations
@@ -29,53 +29,39 @@ import org.eclipse.ui.PlatformUI;
  */
 public class ProvisioningOperationRunner {
 
-	private static Object FAMILY_PROVISIONING_OPERATIONS = new Object();
-
-	private static class JobRunnableContext implements IRunnableContext {
-
-		private class ProvisioningJob extends Job {
-			private IRunnableWithProgress runnable;
-
-			ProvisioningJob(String name, IRunnableWithProgress runnable) {
-				super(name);
-				this.runnable = runnable;
-			}
-
-			protected IStatus run(IProgressMonitor monitor) {
-				try {
-					runnable.run(monitor);
-				} catch (InvocationTargetException e) {
-					return ProvUI.handleException(e, null);
-				} catch (InterruptedException e) {
-					return Status.CANCEL_STATUS;
-				}
-				return Status.OK_STATUS;
-			}
-
-			public boolean belongsTo(Object family) {
-				return family == FAMILY_PROVISIONING_OPERATIONS;
-			}
-
-		}
-
+	private static class ProvisioningJob extends Job {
 		private ProvisioningOperation op;
 		private ProvisioningOperationResult result;
 
-		JobRunnableContext(ProvisioningOperation operation, ProvisioningOperationResult result) {
-			this.op = operation;
+		ProvisioningJob(ProvisioningOperation op, ProvisioningOperationResult result) {
+			super(op.getLabel());
+			this.op = op;
 			this.result = result;
 		}
 
-		public void run(boolean fork, boolean cancelable, IRunnableWithProgress runnable) {
-			Job job = new ProvisioningJob(op.getLabel(), runnable);
-			job.setUser(op.isUser());
-			job.setPriority(Job.DECORATE); // this is the prio that the old update manager used
-			job.schedule();
-			result.setJob(job);
+		protected IStatus run(IProgressMonitor monitor) {
+			try {
+				IStatus status;
+				if (op instanceof IUndoableOperation) {
+					status = PlatformUI.getWorkbench().getOperationSupport().getOperationHistory().execute((IUndoableOperation) op, monitor, null);
+				} else {
+					status = op.execute(monitor, null);
+				}
+				result.setStatus(status);
+				return status;
+			} catch (ExecutionException e) {
+				return ProvUI.handleException(e.getCause(), null);
+			}
+		}
+
+		public boolean belongsTo(Object family) {
+			return family == FAMILY_PROVISIONING_OPERATIONS;
 		}
 
 	}
 
+	private static Object FAMILY_PROVISIONING_OPERATIONS = new Object();
+
 	/**
 	 * Execute the supplied ProvisioningOperation, and add it to the
 	 * undo history if it supports undo.
@@ -87,37 +73,42 @@ public class ProvisioningOperationRunner {
 	 *            a runnable context will be created based on whether the operation should be run in the foreground or background.
 	 *            Callers typically need not supply a context unless special handling (such as wizard-based progress reporting) is required.
 	*/
+	// TODO get rid of context parameter and result after M3
+	// should just return the job and callers can hook listeners if they care what's happening
 	public static ProvisioningOperationResult execute(final ProvisioningOperation op, final Shell shell, IRunnableContext context) {
+		final ProvisioningOperationResult result = new ProvisioningOperationResult(op);
+		Job job;
 
-		final ProvisioningOperationResult[] result = new ProvisioningOperationResult[1];
-		result[0] = new ProvisioningOperationResult(op);
-		result[0].setStatus(Status.OK_STATUS);
-		IRunnableWithProgress runnable = new IRunnableWithProgress() {
-			public void run(IProgressMonitor monitor) {
-				try {
-					if (op instanceof IUndoableOperation) {
-						result[0].setStatus(PlatformUI.getWorkbench().getOperationSupport().getOperationHistory().execute((IUndoableOperation) op, monitor, ProvUI.getUIInfoAdapter(shell)));
-					} else {
-						result[0].setStatus(op.execute(monitor, ProvUI.getUIInfoAdapter(shell)));
+		if (op.runInBackground()) {
+			job = new ProvisioningJob(op, result);
+			job.setPriority(Job.DECORATE); // this is the prio that the old update manager used
+		} else {
+			job = new WorkbenchJob(op.getLabel()) {
+
+				public IStatus runInUIThread(IProgressMonitor monitor) {
+					try {
+						IStatus status;
+						if (op instanceof IUndoableOperation) {
+							status = PlatformUI.getWorkbench().getOperationSupport().getOperationHistory().execute((IUndoableOperation) op, monitor, ProvUI.getUIInfoAdapter(shell));
+						} else {
+							status = op.execute(monitor, ProvUI.getUIInfoAdapter(shell));
+						}
+						result.setStatus(status);
+						return status;
+					} catch (ExecutionException e) {
+						return ProvUI.handleException(e.getCause(), null);
 					}
-				} catch (ExecutionException e) {
-					result[0].setStatus(ProvUI.handleException(e.getCause(), null));
 				}
-			}
-		};
-		if (context == null) {
-			if (op.runInBackground())
-				context = new JobRunnableContext(op, result[0]);
-			else
-				context = PlatformUI.getWorkbench().getProgressService();
-		}
-		try {
-			context.run(op.runInBackground(), true, runnable);
-		} catch (InterruptedException e) {
-			// do nothing
-		} catch (InvocationTargetException e) {
-			result[0].setStatus(ProvUI.handleException(e.getCause(), null));
+
+				public boolean belongsTo(Object family) {
+					return family == FAMILY_PROVISIONING_OPERATIONS;
+				}
+
+			};
 		}
-		return result[0];
+		job.setUser(op.isUser());
+		job.schedule();
+		result.setJob(job);
+		return result;
 	}
 }
diff --git a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/dialogs/AddRepositoryDialog.java b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/dialogs/AddRepositoryDialog.java
index dea208af1..85d07540a 100644
--- a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/dialogs/AddRepositoryDialog.java
+++ b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/p2/ui/dialogs/AddRepositoryDialog.java
@@ -20,7 +20,6 @@ import org.eclipse.equinox.p2.ui.ProvisioningOperationRunner;
 import org.eclipse.equinox.p2.ui.operations.ProvisioningOperation;
 import org.eclipse.jface.dialogs.*;
 import org.eclipse.jface.dialogs.Dialog;
-import org.eclipse.jface.window.Window;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.events.*;
 import org.eclipse.swt.layout.GridData;
@@ -125,21 +124,14 @@ public abstract class AddRepositoryDialog extends StatusDialog {
 	}
 
 	protected void okPressed() {
-		if (addRepository()) {
-			setReturnCode(Window.OK);
-			super.okPressed();
-		} else {
-			setReturnCode(Window.CANCEL);
-		}
+		addRepository();
+		super.okPressed();
 	}
 
-	protected boolean addRepository() {
+	protected void addRepository() {
 		URL newURL = makeRepositoryURL(url.getText().trim());
-		if (newURL == null) {
-			return false;
-		}
-
-		return (ProvisioningOperationRunner.execute(getOperation(newURL), getShell(), null)).getStatus().isOK();
+		if (newURL != null)
+			ProvisioningOperationRunner.execute(getOperation(newURL), getShell(), null);
 	}
 
 	protected abstract ProvisioningOperation getOperation(URL repoURL);