bug 335485 bug 335486 - Additional comments & cleanup for Restart Policy and Update Configuration

9 files changed, 105 insertions, 51 deletions

diff --git a/org.eclipse.m2e.discovery/META-INF/MANIFEST.MF b/org.eclipse.m2e.discovery/META-INF/MANIFEST.MF
index 7a8e5a55..e693ee2e 100644
--- a/org.eclipse.m2e.discovery/META-INF/MANIFEST.MF
+++ b/org.eclipse.m2e.discovery/META-INF/MANIFEST.MF
@@ -21,6 +21,7 @@ Require-Bundle: org.eclipse.equinox.p2.ui.discovery,
  org.eclipse.equinox.p2.metadata
 Export-Package: org.eclipse.m2e.internal.discovery,
  org.eclipse.m2e.internal.discovery.markers,
+ org.eclipse.m2e.internal.discovery.startup;x-internal:=true,
  org.eclipse.m2e.internal.discovery.wizards
 Import-Package: org.apache.maven.model,
  org.apache.maven.plugin,
diff --git a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/Messages.java b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/Messages.java
index c4c8ba59..5a130c0d 100644
--- a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/Messages.java
+++ b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/Messages.java
@@ -39,6 +39,10 @@ public class Messages extends NLS {
 
   public static String MavenDiscovery_Wizard_MavenTag;
 
+  public static String MavenDiscoveryInstallOperation_Configuring;
+
+  public static String UpdateConfigurationStartup_MarkerError;
+
   static {
     // initialize resource bundle
     NLS.initializeMessages(BUNDLE_NAME, Messages.class);
diff --git a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/messages.properties b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/messages.properties
index df0f7757..df1db75b 100644
--- a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/messages.properties
+++ b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/messages.properties
@@ -15,3 +15,5 @@ MavenDiscovery_Wizard_Applicable_Tag=Applicable
 MavenDiscovery_Wizard_ExtrasTag=Extras
 MavenDiscovery_Wizard_LifecyclesTag=Lifecycles
 MavenDiscovery_Wizard_MavenTag=Maven
+MavenDiscoveryInstallOperation_Configuring=Configuring installation selection
+UpdateConfigurationStartup_MarkerError=Error(s) collecting projects
diff --git a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/operation/MavenDiscoveryInstallOperation.java b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/operation/MavenDiscoveryInstallOperation.java
index 846b7551..29725ce7 100644
--- a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/operation/MavenDiscoveryInstallOperation.java
+++ b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/operation/MavenDiscoveryInstallOperation.java
@@ -27,9 +27,15 @@ import org.eclipse.equinox.p2.operations.ProvisioningJob;
 import org.eclipse.equinox.p2.operations.ProvisioningSession;
 import org.eclipse.equinox.p2.ui.ProvisioningUI;
 import org.eclipse.m2e.internal.discovery.MavenDiscovery;
-import org.eclipse.m2e.internal.discovery.wizards.DiscoveryUi;
+import org.eclipse.m2e.internal.discovery.Messages;
+import org.eclipse.m2e.internal.discovery.wizards.MavenDiscoveryUi;
 import org.eclipse.swt.widgets.Display;
 
+
+/*
+ * This class allows us to open MavenDiscoveryInstallWizard instead of the default p2 wizard 
+ * to support changing the restart policy for the subsequent ProvisioningJob. 
+ */
 @SuppressWarnings("restriction")
 public class MavenDiscoveryInstallOperation extends DiscoveryInstallOperation {
   private List<CatalogItem> installableConnectors;
@@ -44,20 +50,20 @@ public class MavenDiscoveryInstallOperation extends DiscoveryInstallOperation {
   @Override
   public void run(IProgressMonitor progressMonitor) throws InvocationTargetException, InterruptedException {
     try {
-      SubMonitor monitor = SubMonitor.convert(progressMonitor, "Messages.InstallConnectorsJob_task_configuring", 100);
+      SubMonitor monitor = SubMonitor.convert(progressMonitor, Messages.MavenDiscoveryInstallOperation_Configuring, 100);
       try {
         final IInstallableUnit[] ius = computeInstallableUnits(monitor.newChild(50));
 
         checkCancelled(monitor);
 
-        final RestartInstallOperation installOperation = resolve(monitor.newChild(50), ius, new URI[0],
+        final RestartInstallOperation installOperation = createAndResolve(monitor.newChild(50), ius, new URI[0],
             requireRestart(installableConnectors));
 
         checkCancelled(monitor);
 
         Display.getDefault().asyncExec(new Runnable() {
           public void run() {
-            DiscoveryUi.openInstallWizard(Arrays.asList(ius), installOperation, null);
+            MavenDiscoveryUi.openInstallWizard(Arrays.asList(ius), installOperation, null);
           }
         });
       } finally {
@@ -70,6 +76,9 @@ public class MavenDiscoveryInstallOperation extends DiscoveryInstallOperation {
     }
   }
 
+  /*
+   * Restart is required when one or more CatalogItem lacks the norestart tag.
+   */
   public static boolean requireRestart(Iterable<CatalogItem> catalogItems) {
     for(CatalogItem item : catalogItems) {
       if(!item.hasTag(MavenDiscovery.NO_RESTART_TAG)) {
@@ -79,7 +88,11 @@ public class MavenDiscoveryInstallOperation extends DiscoveryInstallOperation {
     return false;
   }
 
-  private RestartInstallOperation resolve(IProgressMonitor monitor, final IInstallableUnit[] ius, URI[] repositories,
+  /*
+   * Create a RestartInstallOperation and resolve
+   */
+  private RestartInstallOperation createAndResolve(IProgressMonitor monitor, final IInstallableUnit[] ius,
+      URI[] repositories,
       boolean requireRestart) throws CoreException {
     SubMonitor mon = SubMonitor.convert(monitor, ius.length);
     try {
diff --git a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/operation/RestartInstallOperation.java b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/operation/RestartInstallOperation.java
index 9df02428..365f2705 100644
--- a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/operation/RestartInstallOperation.java
+++ b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/operation/RestartInstallOperation.java
@@ -14,7 +14,6 @@ import java.util.Collection;
 
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
-import org.eclipse.equinox.p2.engine.IProfileRegistry;
 import org.eclipse.equinox.p2.metadata.IInstallableUnit;
 import org.eclipse.equinox.p2.operations.InstallOperation;
 import org.eclipse.equinox.p2.operations.ProfileModificationJob;
@@ -42,7 +41,7 @@ public class RestartInstallOperation extends InstallOperation {
     ProvisioningJob job = super.getProvisioningJob(monitor);
     if(job != null && job instanceof ProfileModificationJob) {
       ((ProfileModificationJob) job).setRestartPolicy(restartPolicy);
-      UpdateConfigurationProvisioningJob ucJob = new UpdateConfigurationProvisioningJob(((ProfileModificationJob) job),
+      UpdateMavenConfigurationProvisioningJob ucJob = new UpdateMavenConfigurationProvisioningJob(((ProfileModificationJob) job),
           session);
       return ucJob;
     }
@@ -57,12 +56,16 @@ public class RestartInstallOperation extends InstallOperation {
     this.restartPolicy = restartPolicy;
   }
 
-  private static class UpdateConfigurationProvisioningJob extends ProfileModificationJob {
+  /*
+   * The ProfileModificationJob is wrapped to allow us to know when the job finishes successfully so we can 
+   * ensure that early startup for update configuration is enabled.
+   */
+  private static class UpdateMavenConfigurationProvisioningJob extends ProfileModificationJob {
 
     private ProfileModificationJob job;
 
-    public UpdateConfigurationProvisioningJob(ProfileModificationJob job, ProvisioningSession session) {
-      super(job.getName(), session, IProfileRegistry.SELF, null, null);
+    public UpdateMavenConfigurationProvisioningJob(ProfileModificationJob job, ProvisioningSession session) {
+      super(job.getName(), session, job.getProfileId(), null, null);
       this.job = job;
     }
 
diff --git a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/startup/UpdateConfigurationStartup.java b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/startup/UpdateConfigurationStartup.java
index b4601bdb..c76a4e4b 100644
--- a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/startup/UpdateConfigurationStartup.java
+++ b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/startup/UpdateConfigurationStartup.java
@@ -19,15 +19,18 @@ import org.eclipse.core.resources.IResource;
 import org.eclipse.core.resources.IWorkspaceRoot;
 import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.runtime.MultiStatus;
 import org.eclipse.jface.preference.IPreferenceStore;
 import org.eclipse.m2e.core.MavenPlugin;
 import org.eclipse.m2e.core.core.IMavenConstants;
 import org.eclipse.m2e.core.jobs.UpdateConfigurationJob;
 import org.eclipse.m2e.internal.discovery.DiscoveryActivator;
+import org.eclipse.m2e.internal.discovery.Messages;
 import org.eclipse.ui.IStartup;
 import org.eclipse.ui.internal.IPreferenceConstants;
 import org.eclipse.ui.internal.Workbench;
 import org.eclipse.ui.internal.util.PrefUtil;
+import org.eclipse.ui.statushandlers.StatusManager;
 
 
 @SuppressWarnings("restriction")
@@ -37,7 +40,7 @@ public class UpdateConfigurationStartup implements IStartup {
 
   public void earlyStartup() {
     final MavenPlugin plugin = MavenPlugin.getDefault();
-    new UpdateConfigurationJob(plugin, getMarkedProjects()).schedule();
+    new UpdateConfigurationJob(plugin, getSavedProjects()).schedule();
     disableStartup();
   }
 
@@ -45,27 +48,37 @@ public class UpdateConfigurationStartup implements IStartup {
    * Enables the early startup for this bundle
    */
   public static void enableStartup() {
-    updateMarkedProjects();
+    saveMarkedProjects();
+    addEarlyStartup();
+  }
+
+  /*
+   * Disables the early startup for this bundle 
+   */
+  public static void disableStartup() {
+    clearSavedProjects();
+    removeEarlyStartup();
+  }
+
+  private static void addEarlyStartup() {
     String[] disabledEarlyActivation = Workbench.getInstance().getDisabledEarlyActivatedPlugins();
 
+    // If we aren't disabled, nothing to do
     if(!isDisabled(disabledEarlyActivation)) {
       return;
     }
 
-    StringBuffer preference = new StringBuffer();
-    for(String item : disabledEarlyActivation) {
-      if(!DiscoveryActivator.PLUGIN_ID.equals(item)) {
-        preference.append(item).append(IPreferenceConstants.SEPARATOR);
+    String[] disabledPlugins = new String[disabledEarlyActivation.length - 1];
+    int index = 0;
+    for(String plugin : disabledEarlyActivation) {
+      if(!DiscoveryActivator.PLUGIN_ID.equals(plugin)) {
+        disabledPlugins[index] = plugin;
       }
     }
-    setPreference(preference.toString());
+    setEarlyActivationPreference(disabledPlugins);
   }
 
-  /*
-   * Disables the early startup for this bundle 
-   */
-  public static void disableStartup() {
-    clearMarkedProjects();
+  private static void removeEarlyStartup() {
     String[] disabledEarlyActivation = Workbench.getInstance().getDisabledEarlyActivatedPlugins();
 
     // Determine if we're already disabled
@@ -73,13 +86,11 @@ public class UpdateConfigurationStartup implements IStartup {
       return;
     }
 
-    // Add ourself to disabled
-    StringBuffer preference = new StringBuffer();
-    for(String item : disabledEarlyActivation) {
-      preference.append(item).append(IPreferenceConstants.SEPARATOR);
-    }
-    preference.append(DiscoveryActivator.PLUGIN_ID).append(IPreferenceConstants.SEPARATOR);
-    setPreference(preference.toString());
+    String[] disabledPlugins = new String[disabledEarlyActivation.length + 1];
+    System.arraycopy(disabledEarlyActivation, 0, disabledPlugins, 0, disabledEarlyActivation.length);
+    disabledPlugins[disabledPlugins.length - 1] = DiscoveryActivator.PLUGIN_ID;
+
+    setEarlyActivationPreference(disabledPlugins);
   }
 
   private static boolean isDisabled(String[] disabledEarlyActivation) {
@@ -91,13 +102,21 @@ public class UpdateConfigurationStartup implements IStartup {
     return false;
   }
 
-  private static void setPreference(String pref) {
+  private static void setEarlyActivationPreference(String[] disabledPlugins) {// Add ourself to disabled
+    StringBuffer preference = new StringBuffer();
+    for(String item : disabledPlugins) {
+      preference.append(item).append(IPreferenceConstants.SEPARATOR);
+    }
+
     IPreferenceStore store = PrefUtil.getInternalPreferenceStore();
-    store.putValue(IPreferenceConstants.PLUGINS_NOT_ACTIVATED_ON_STARTUP, pref);
+    store.putValue(IPreferenceConstants.PLUGINS_NOT_ACTIVATED_ON_STARTUP, preference.toString());
     PrefUtil.savePrefs();
   }
 
-  public static IProject[] getMarkedProjects() {
+  /*
+   * Get projects we saved previously  
+   */
+  public static IProject[] getSavedProjects() {
     String[] projectNames = DiscoveryActivator.getDefault().getPreferenceStore().getString(PROJECT_PREF)
         .split(String.valueOf(IPreferenceConstants.SEPARATOR));
     List<IProject> projects = new ArrayList<IProject>(projectNames.length);
@@ -113,22 +132,32 @@ public class UpdateConfigurationStartup implements IStartup {
     return projects.toArray(new IProject[projects.size()]);
   }
 
-  public static void updateMarkedProjects() {
+  /*
+   * Save a list of projects which have configuration markers
+   */
+  public static void saveMarkedProjects() {
     StringBuilder sb = new StringBuilder();
+    MultiStatus status = new MultiStatus(DiscoveryActivator.PLUGIN_ID, 0,
+        Messages.UpdateConfigurationStartup_MarkerError, null);
     for(IProject project : ResourcesPlugin.getWorkspace().getRoot().getProjects()) {
       try {
-        if(project.findMarkers(IMavenConstants.MARKER_CONFIGURATION_ID, true, IResource.DEPTH_ONE).length > 0) {
+        if(project.findMarkers(IMavenConstants.MARKER_LIFECYCLEMAPPING_ID, true, IResource.DEPTH_ONE).length > 0) {
           sb.append(project.getName()).append(IPreferenceConstants.SEPARATOR);
         }
       } catch(CoreException e) {
-        // TODO Auto-generated catch block
-        e.printStackTrace();
+        status.add(e.getStatus());
       }
     }
+    if(status.getChildren().length > 0) {
+      StatusManager.getManager().handle(status);
+    }
     DiscoveryActivator.getDefault().getPreferenceStore().putValue(PROJECT_PREF, sb.toString());
   }
 
-  public static void clearMarkedProjects() {
+  /*
+   * Empty the list of saved projects
+   */
+  public static void clearSavedProjects() {
     DiscoveryActivator.getDefault().getPreferenceStore().putValue(PROJECT_PREF, ""); //$NON-NLS-1$
   }
 }
\ No newline at end of file
diff --git a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/wizards/InstallWizard.java b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/wizards/MavenDiscoveryInstallWizard.java
index 052ed004..964b6574 100644
--- a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/wizards/InstallWizard.java
+++ b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/wizards/MavenDiscoveryInstallWizard.java
@@ -22,11 +22,11 @@ import org.eclipse.m2e.internal.discovery.operation.RestartInstallOperation;
 
 
 /*
- * Needed to override getProvisioningContext()
+ * This exists to allow us to return a ProfileChangeOperation which changes the restart policy for provisioning jobs. 
  */
-public class InstallWizard extends PreselectedIUInstallWizard {
+public class MavenDiscoveryInstallWizard extends PreselectedIUInstallWizard {
 
-  public InstallWizard(ProvisioningUI ui, RestartInstallOperation operation,
+  public MavenDiscoveryInstallWizard(ProvisioningUI ui, RestartInstallOperation operation,
       Collection<IInstallableUnit> initialSelections, LoadMetadataRepositoryJob job) {
     super(ui, operation, initialSelections, job);
   }
@@ -39,8 +39,6 @@ public class InstallWizard extends PreselectedIUInstallWizard {
     RestartInstallOperation op = new RestartInstallOperation(ui.getSession(), ElementUtils.elementsToIUs(elements));
     op.setRestartPolicy(((RestartInstallOperation) operation).getRestartPolicy());
     op.setProfileId(getProfileId());
-    //    op.setRootMarkerKey(getRootMarkerKey());
-    op.setProvisioningContext(getProvisioningContext());
     return op;
   }
 }
diff --git a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/wizards/DiscoveryUi.java b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/wizards/MavenDiscoveryUi.java
index 3c5a2d8d..f55dc509 100644
--- a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/wizards/DiscoveryUi.java
+++ b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/wizards/MavenDiscoveryUi.java
@@ -28,20 +28,23 @@ import org.eclipse.equinox.p2.ui.ProvisioningUI;
 import org.eclipse.jface.operation.IRunnableContext;
 import org.eclipse.jface.operation.IRunnableWithProgress;
 import org.eclipse.jface.wizard.WizardDialog;
+import org.eclipse.m2e.internal.discovery.DiscoveryActivator;
 import org.eclipse.m2e.internal.discovery.operation.MavenDiscoveryInstallOperation;
 import org.eclipse.m2e.internal.discovery.operation.RestartInstallOperation;
 import org.eclipse.osgi.util.NLS;
 import org.eclipse.ui.PlatformUI;
 import org.eclipse.ui.statushandlers.StatusManager;
 
-/**
- * @author David Green
- */
-public abstract class DiscoveryUi {
 
-	public static final String ID_PLUGIN = "org.eclipse.equinox.p2.ui.discovery"; //$NON-NLS-1$
+/*
+ * This is used to replace the typical discovery install operation with our own which allows us to change 
+ * the restart policy and enable early startup for configuration updates.
+ * 
+ * Copied from org.eclipse.equinox.internal.p2.ui.discovery.DiscoveryUi
+ */
+public abstract class MavenDiscoveryUi {
 
-	private DiscoveryUi() {
+  private MavenDiscoveryUi() {
 		// don't allow clients to instantiate
 	}
 
@@ -50,7 +53,8 @@ public abstract class DiscoveryUi {
       IRunnableWithProgress runner = new MavenDiscoveryInstallOperation(descriptors);
 			context.run(true, true, runner);
 		} catch (InvocationTargetException e) {
-			IStatus status = new Status(IStatus.ERROR, DiscoveryUi.ID_PLUGIN, NLS.bind(Messages.ConnectorDiscoveryWizard_installProblems, new Object[] {e.getCause().getMessage()}), e.getCause());
+      IStatus status = new Status(IStatus.ERROR, DiscoveryActivator.PLUGIN_ID, NLS.bind(
+          Messages.ConnectorDiscoveryWizard_installProblems, new Object[] {e.getCause().getMessage()}), e.getCause());
 			StatusManager.getManager().handle(status, StatusManager.SHOW | StatusManager.BLOCK | StatusManager.LOG);
 			return false;
 		} catch (InterruptedException e) {
@@ -62,7 +66,7 @@ public abstract class DiscoveryUi {
 
   public static int openInstallWizard(Collection<IInstallableUnit> initialSelections,
       RestartInstallOperation operation, LoadMetadataRepositoryJob job) {
-    InstallWizard wizard = new InstallWizard(ProvisioningUI.getDefaultUI(), operation, initialSelections, job);
+    MavenDiscoveryInstallWizard wizard = new MavenDiscoveryInstallWizard(ProvisioningUI.getDefaultUI(), operation, initialSelections, job);
     WizardDialog dialog = new ProvisioningWizardDialog(ProvUI.getDefaultParentShell(), wizard);
     dialog.create();
     PlatformUI.getWorkbench().getHelpSystem().setHelp(dialog.getShell(), IProvHelpContextIds.INSTALL_WIZARD);
diff --git a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/wizards/MavenDiscoveryWizard.java b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/wizards/MavenDiscoveryWizard.java
index b937e0bf..ba29c815 100644
--- a/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/wizards/MavenDiscoveryWizard.java
+++ b/org.eclipse.m2e.discovery/src/org/eclipse/m2e/internal/discovery/wizards/MavenDiscoveryWizard.java
@@ -34,6 +34,6 @@ public class MavenDiscoveryWizard extends DiscoveryWizard {
 
   @Override
   public boolean performFinish() {
-    return DiscoveryUi.install(getCatalogPage().getInstallableConnectors(), getContainer());
+    return MavenDiscoveryUi.install(getCatalogPage().getInstallableConnectors(), getContainer());
   }
 }