Clone operation should run in background

As cloning is a long-running task, it should always run as
background job with a non-modal dialog. Introduced a job
family so we can easily join on the Clone operation to wait
for it to finish.

Bug: 318578
Change-Id: I7163fdf1794c1f0021909477a03917320fe72db2
Signed-off-by: Benjamin Muskalla <bmuskalla@eclipsesource.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

9 files changed, 166 insertions, 128 deletions

diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/RepoPropertiesPage.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/RepoPropertiesPage.java
index c384bedea8..024fecaaa5 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/RepoPropertiesPage.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/RepoPropertiesPage.java
@@ -68,6 +68,6 @@ public class RepoPropertiesPage {
 	public RepoRemoteBranchesPage nextToRemoteBranches(String string) {
 		setURI(string);
 		bot.button("Next >").click();
-		return new RepoRemoteBranchesPage(string);
+		return new RepoRemoteBranchesPage();
 	}
 }
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/RepoRemoteBranchesPage.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/RepoRemoteBranchesPage.java
index 177006c5d7..85b6d41250 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/RepoRemoteBranchesPage.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/RepoRemoteBranchesPage.java
@@ -19,12 +19,6 @@ import org.eclipse.swtbot.swt.finder.widgets.SWTBotTable;
 public class RepoRemoteBranchesPage {
 	private static final SWTWorkbenchBot bot = new SWTWorkbenchBot();
 
-	private final String cloneUrl;
-
-	public RepoRemoteBranchesPage(String cloneUrl) {
-		this.cloneUrl = cloneUrl;
-	}
-
 	public void assertRemoteBranches(String... branches) {
 		SWTBotTable table = bot.table();
 		bot.waitUntil(widgetIsEnabled(table), 20000);
@@ -43,7 +37,7 @@ public class RepoRemoteBranchesPage {
 
 	public WorkingCopyPage nextToWorkingCopy() {
 		bot.button("Next >").click();
-		return new WorkingCopyPage(cloneUrl);
+		return new WorkingCopyPage();
 	}
 
 	public void deselectAllBranches() {
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/WorkingCopyPage.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/WorkingCopyPage.java
index 07938fa8e5..eb6a31ddeb 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/WorkingCopyPage.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/WorkingCopyPage.java
@@ -10,29 +10,23 @@
 package org.eclipse.egit.ui.common;
 
 import static org.eclipse.swtbot.swt.finder.SWTBotAssert.assertText;
-import static org.eclipse.swtbot.swt.finder.waits.Conditions.shellCloses;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.fail;
 
 import java.io.File;
 
+import org.eclipse.core.runtime.NullProgressMonitor;
+import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.egit.ui.Activator;
+import org.eclipse.egit.ui.internal.clone.GitCloneWizard;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.swtbot.eclipse.finder.SWTWorkbenchBot;
-import org.eclipse.swtbot.swt.finder.exceptions.WidgetNotFoundException;
-import org.eclipse.swtbot.swt.finder.widgets.SWTBotShell;
 
 public class WorkingCopyPage {
 
 	private static final SWTWorkbenchBot bot = new SWTWorkbenchBot();
 
-	private final String cloneUrl;
-
-	public WorkingCopyPage(String cloneUrl) {
-		this.cloneUrl = cloneUrl;
-	}
-
 	public void assertDirectory(String localDir) {
 		assertText(localDir, bot.textWithLabel("Directory:"));
 	}
@@ -57,26 +51,22 @@ public class WorkingCopyPage {
 		bot.button("Finish").click();
 
 		try {
-			SWTBotShell shell = bot.shell("Cloning from " + cloneUrl);
-
-			// This is not a performance test. Allow lots of time to complete
-			bot.waitUntil(shellCloses(shell), 120000);
-		} catch (WidgetNotFoundException e1) {
-			// if the "Cloning from" window opens and closes very quickly
-			// (faster than the replay delay), we end up here, and we do an
-			// alternate test to see if the repository was cloned
-			// This is not a performance test. Allow lots of time to complete
-			for (int i = 0; i < 10; i++) {
-				if (Activator.getDefault().getRepositoryUtil()
-						.getConfiguredRepositories().contains(targetDir))
-					return;
-				try {
-					Thread.sleep(1000);
-				} catch (InterruptedException e) {
-					// ignore here
-				}
+			Job.getJobManager().join(GitCloneWizard.CLONE_JOB_FAMILY, new NullProgressMonitor());
+		} catch (Exception e) {
+			fail( "Unable to join cloning job");
+		}
+		// depending on the timing, the clone job may already be run
+		// but the repository is not yet added to our list, of
+		// repositories. Wait until that happend.
+		for (int i = 0; i < 3; i++) {
+			if (Activator.getDefault().getRepositoryUtil()
+					.getConfiguredRepositories().contains(targetDir))
+				return;
+			try {
+				Thread.sleep(1000);
+			} catch (InterruptedException e) {
+				// ignore here
 			}
-			fail("The Repository was not created");
 		}
 	}
 
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/wizards/clone/GitCloneWizardTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/wizards/clone/GitCloneWizardTest.java
index 410ae314ee..a8d2c7eec7 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/wizards/clone/GitCloneWizardTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/wizards/clone/GitCloneWizardTest.java
@@ -236,7 +236,7 @@ public class GitCloneWizardTest extends EGitTestCase {
 	}
 
 	@Test
-	public void clonedRepositoryShouldExistOnFileSystem() throws Exception {
+	public void clonedRepositoryShouldExistOnFileSystem() {
 		importWizard.openWizard();
 		RepoPropertiesPage repoProperties = importWizard.openCloneWizard();
 		RepoRemoteBranchesPage remoteBranches = repoProperties
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/UIText.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/UIText.java
index 2ee394d889..03f12b41b6 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/UIText.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/UIText.java
@@ -187,12 +187,6 @@ public class UIText extends NLS {
 	public static String GitCloneWizard_abortingCloneTitle;
 
 	/** */
-	public static String GitCloneWizard_CloneFailedHeading;
-
-	/** */
-	public static String GitCloneWizard_CloneCanceledMessage;
-
-	/** */
 	public static String GitCloneWizard_title;
 
 	/** */
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitCloneWizard.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitCloneWizard.java
index 4673cbbf85..1a08ed6759 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitCloneWizard.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitCloneWizard.java
@@ -4,6 +4,7 @@
  * Copyright (C) 2008, Marek Zawirski <marek.zawirski@gmail.com>
  * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
  * Copyright (C) 2010, Mathias Kinzler <mathias.kinzler@sap.com>
+ * Copyright (C) 2010, Benjamin Muskalla <bmuskalla@eclipsesource.com>
  *
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
@@ -30,22 +31,32 @@ import org.eclipse.egit.ui.internal.components.RepositorySelectionPage;
 import org.eclipse.jface.dialogs.ErrorDialog;
 import org.eclipse.jface.dialogs.MessageDialog;
 import org.eclipse.jface.operation.IRunnableWithProgress;
+import org.eclipse.jface.wizard.IWizardContainer;
 import org.eclipse.jface.wizard.Wizard;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.transport.URIish;
 import org.eclipse.osgi.util.NLS;
+import org.eclipse.swt.widgets.Display;
 
 /**
  * Import Git Repository Wizard. A front end to a git clone operation.
  */
 public class GitCloneWizard extends Wizard {
+
+	/**
+	 * Job family of the Clone Repository job.
+	 */
+	public static final Object CLONE_JOB_FAMILY = new Object();
+
 	private RepositorySelectionPage cloneSource;
 
 	private SourceBranchPage validSource;
 
 	private CloneDestinationPage cloneDestination;
 
-	String alreadyClonedInto;
+	private String alreadyClonedInto;
+
+	private IWizardContainer parentContainer;
 
 	/**
 	 * The default constructor
@@ -77,6 +88,15 @@ public class GitCloneWizard extends Wizard {
 		};
 	}
 
+	/**
+	 * Sets the parent {@link IWizardContainer} to run the clone job.
+	 *
+	 * @param parentContainer
+	 */
+	public void setParentContainer(IWizardContainer parentContainer) {
+		this.parentContainer = parentContainer;
+	}
+
 	@Override
 	public boolean performCancel() {
 		if (alreadyClonedInto != null) {
@@ -122,13 +142,13 @@ public class GitCloneWizard extends Wizard {
 	@Override
 	public boolean performFinish() {
 		try {
-			return performClone(false);
+			return performClone();
 		} finally {
 			setWindowTitle(UIText.GitCloneWizard_title);
 		}
 	}
 
-	boolean performClone(boolean background) {
+	boolean performClone() {
 		final URIish uri = cloneSource.getSelection().getURI();
 		setWindowTitle(NLS.bind(UIText.GitCloneWizard_jobName, uri.toString()));
 		final boolean allSelected;
@@ -159,66 +179,75 @@ public class GitCloneWizard extends Wizard {
 
 		final CloneOperation op = new CloneOperation(uri, allSelected,
 				selectedBranches, workdir, branch, remoteName);
-
 		alreadyClonedInto = workdir.getPath();
 
-		final RepositoryUtil config = Activator.getDefault()
-				.getRepositoryUtil();
+		cloneSource.saveUriInPrefs();
+		if (parentContainer == null) {
+			runAsJob(uri, op);
+		} else {
+			runInParentContainer(op);
+		}
+		return true;
+	}
 
-		if (background) {
-			final Job job = new Job(NLS.bind(UIText.GitCloneWizard_jobName, uri
-					.toString())) {
-				@Override
-				protected IStatus run(final IProgressMonitor monitor) {
-					try {
-						op.run(monitor);
-						cloneSource.saveUriInPrefs();
-						config.addConfiguredRepository(op.getGitDir());
-						return Status.OK_STATUS;
-					} catch (InterruptedException e) {
-						return Status.CANCEL_STATUS;
-					} catch (InvocationTargetException e) {
-						Throwable thr = e.getCause();
-						return new Status(IStatus.ERROR, Activator
-								.getPluginId(), 0, thr.getMessage(), thr);
-					}
+	private void runInParentContainer(final CloneOperation op) {
+		Runnable runInParentContainer = new Runnable() {
+			public void run() {
+				try {
+					parentContainer.run(true, true,
+							new IRunnableWithProgress() {
+								public void run(IProgressMonitor monitor)
+										throws InvocationTargetException,
+										InterruptedException {
+									executeCloneOperation(op, monitor);
+								}
+							});
+				} catch (InvocationTargetException e) {
+					Activator.handleError(UIText.GitCloneWizard_failed,
+							e.getCause(), true);
+				} catch (InterruptedException e) {
+					// nothing to do
 				}
+			}
+		};
+		// we need to run this async in order to cleanly close the inner wizard
+		Display.getCurrent().asyncExec(runInParentContainer);
+	}
 
-			};
-			job.setUser(true);
-			job.schedule();
-			return true;
-		} else {
-			try {
-				// Perform clone in ModalContext thread with progress
-				// reporting on the wizard.
-				getContainer().run(true, true, new IRunnableWithProgress() {
-					public void run(IProgressMonitor monitor)
-							throws InvocationTargetException,
-							InterruptedException {
-						op.run(monitor);
-						if (monitor.isCanceled())
-							throw new InterruptedException();
-					}
-				});
-
-				cloneSource.saveUriInPrefs();
-				config.addConfiguredRepository(op.getGitDir());
-				return true;
-			} catch (InterruptedException e) {
-				MessageDialog.openInformation(getShell(),
-						UIText.GitCloneWizard_CloneFailedHeading,
-						UIText.GitCloneWizard_CloneCanceledMessage);
-				return false;
-			} catch (InvocationTargetException e) {
-				Activator.handleError(UIText.GitCloneWizard_CloneFailedHeading,
-						e.getTargetException(), true);
-				return false;
-			} catch (Exception e) {
-				Activator.handleError(UIText.GitCloneWizard_CloneFailedHeading,
-						e, true);
-				return false;
+	private void runAsJob(final URIish uri, final CloneOperation op) {
+		final Job job = new Job(NLS.bind(UIText.GitCloneWizard_jobName,
+				uri.toString())) {
+			@Override
+			protected IStatus run(final IProgressMonitor monitor) {
+				try {
+					return executeCloneOperation(op, monitor);
+				} catch (InterruptedException e) {
+					return Status.CANCEL_STATUS;
+				} catch (InvocationTargetException e) {
+					Throwable thr = e.getCause();
+					return new Status(IStatus.ERROR, Activator.getPluginId(),
+							0, thr.getMessage(), thr);
+				}
 			}
-		}
+
+			@Override
+			public boolean belongsTo(Object family) {
+				return CLONE_JOB_FAMILY.equals(family);
+			}
+		};
+		job.setUser(true);
+		job.schedule();
+	}
+
+	private IStatus executeCloneOperation(final CloneOperation op,
+			final IProgressMonitor monitor) throws InvocationTargetException,
+			InterruptedException {
+
+		final RepositoryUtil util = Activator.getDefault()
+				.getRepositoryUtil();
+
+		op.run(monitor);
+		util.addConfiguredRepository(op.getGitDir());
+		return Status.OK_STATUS;
 	}
 }
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitImportWizard.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitImportWizard.java
index 27f613ac03..2e9b63c142 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitImportWizard.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitImportWizard.java
@@ -63,6 +63,8 @@ public class GitImportWizard extends Wizard implements ProjectCreator,
 		setDefaultPageImageDescriptor(UIIcons.WIZBAN_IMPORT_REPO);
 		previousProjects = ResourcesPlugin.getWorkspace().getRoot()
 				.getProjects();
+		selectRepoPage.setWizard(this);
+		setNeedsProgressMonitor(true);
 	}
 
 	@Override
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitSelectRepositoryPage.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitSelectRepositoryPage.java
index 854401e940..d0af81a14a 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitSelectRepositoryPage.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitSelectRepositoryPage.java
@@ -1,5 +1,7 @@
 /*******************************************************************************
- * Copyright (c) 2010 SAP AG.
+ * Copyright (C) 2010 SAP AG.
+ * Copyright (C) 2010, Benjamin Muskalla <bmuskalla@eclipsesource.com>
+ *
  * 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
@@ -15,6 +17,8 @@ import java.io.IOException;
 import java.util.List;
 import java.util.Set;
 
+import org.eclipse.core.runtime.preferences.IEclipsePreferences.IPreferenceChangeListener;
+import org.eclipse.core.runtime.preferences.IEclipsePreferences.PreferenceChangeEvent;
 import org.eclipse.egit.core.RepositoryUtil;
 import org.eclipse.egit.ui.Activator;
 import org.eclipse.egit.ui.UIText;
@@ -41,6 +45,7 @@ import org.eclipse.swt.events.SelectionAdapter;
 import org.eclipse.swt.events.SelectionEvent;
 import org.eclipse.swt.widgets.Button;
 import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Display;
 
 /**
  * Select a repository, add or clone
@@ -55,6 +60,8 @@ public class GitSelectRepositoryPage extends WizardPage {
 
 	private Button cloneRepo;
 
+	private IPreferenceChangeListener configChangeListener;
+
 	/**
 	 *
 	 */
@@ -103,32 +110,15 @@ public class GitSelectRepositoryPage extends WizardPage {
 
 		cloneRepo.addSelectionListener(new SelectionAdapter() {
 			@Override
-			public void widgetSelected(SelectionEvent e) {
-				List<String> dirsBefore = util.getConfiguredRepositories();
+			public void widgetSelected(SelectionEvent event) {
+				GitCloneWizard cloneWizard = new GitCloneWizard();
+				cloneWizard.setParentContainer(getContainer());
 				WizardDialog dlg = new WizardDialog(getShell(),
-						new GitCloneWizard());
-
-				if (dlg.open() == Window.OK) {
-					List<String> dirsAfter = util.getConfiguredRepositories();
-					if (!dirsBefore.containsAll(dirsAfter)) {
-						tv.setInput(dirsAfter);
-						for (String dir : dirsAfter) {
-							if (!dirsBefore.contains(dir)) {
-								try {
-									RepositoryNode node = new RepositoryNode(
-											null, new FileRepository(new File(dir)));
-									tv.setSelection(new StructuredSelection(
-											node));
-								} catch (IOException e1) {
-									Activator.handleError(e1.getMessage(), e1,
-											false);
-								}
-							}
-						}
-					}
-					checkPage();
-				}
+						cloneWizard);
+
+				dlg.open();
 			}
+
 		});
 
 		addRepo = new Button(tb, SWT.PUSH);
@@ -167,6 +157,19 @@ public class GitSelectRepositoryPage extends WizardPage {
 
 		tv.setInput(util.getConfiguredRepositories());
 
+		configChangeListener = new IPreferenceChangeListener() {
+			public void preferenceChange(PreferenceChangeEvent event) {
+				Display display = tv.getControl().getDisplay();
+				display.asyncExec(new Runnable() {
+					public void run() {
+						refreshRepositoryList();
+						checkPage();
+					}
+				});
+			}
+		};
+		util.getPreferences().addPreferenceChangeListener(configChangeListener);
+
 		// we need to select at least a repository to become complete
 		setPageComplete(false);
 		Dialog.applyDialogFont(main);
@@ -174,6 +177,28 @@ public class GitSelectRepositoryPage extends WizardPage {
 
 	}
 
+	private void refreshRepositoryList() {
+		@SuppressWarnings("unchecked")
+		List<String> dirsBefore = (List<String>) tv.getInput();
+		List<String> dirsAfter = util.getConfiguredRepositories();
+		if (!dirsBefore.containsAll(dirsAfter)) {
+			tv.setInput(dirsAfter);
+			for (String dir : dirsAfter) {
+				if (!dirsBefore.contains(dir)) {
+					try {
+						RepositoryNode node = new RepositoryNode(
+								null, new FileRepository(new File(dir)));
+						tv.setSelection(new StructuredSelection(
+								node));
+					} catch (IOException e1) {
+						Activator.handleError(e1.getMessage(), e1,
+								false);
+					}
+				}
+			}
+		}
+	}
+
 	private void checkPage() {
 		setErrorMessage(null);
 		try {
@@ -191,4 +216,10 @@ public class GitSelectRepositoryPage extends WizardPage {
 		}
 	}
 
+	@Override
+	public void dispose() {
+		super.dispose();
+		util.getPreferences().removePreferenceChangeListener(
+				configChangeListener);
+	}
 }
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/uitext.properties b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/uitext.properties
index 8b369e0b10..ee1069f18f 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/uitext.properties
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/uitext.properties
@@ -67,8 +67,6 @@ ExistingOrNewPage_SymbolicValueEmptyMapping=<empty repository mapping>
 
 GitCloneWizard_abortingCloneMsg=A partial or complete clone was already made. Do you want to delete it?
 GitCloneWizard_abortingCloneTitle=Aborting clone
-GitCloneWizard_CloneFailedHeading=Cloning Git Repository failed
-GitCloneWizard_CloneCanceledMessage=Clone operation was canceled by the user
 GitCloneWizard_title=Clone Git Repository
 GitCloneWizard_jobName=Cloning from {0}
 GitCloneWizard_failed=Git repository clone failed.