bug 337565 - Fixes to exclude refactoring

2 files changed, 55 insertions, 47 deletions

diff --git a/org.eclipse.m2e.refactoring/src/org/eclipse/m2e/refactoring/exclude/ExcludeArtifactRefactoring.java b/org.eclipse.m2e.refactoring/src/org/eclipse/m2e/refactoring/exclude/ExcludeArtifactRefactoring.java
index 0d61efd7..27c3aae8 100644
--- a/org.eclipse.m2e.refactoring/src/org/eclipse/m2e/refactoring/exclude/ExcludeArtifactRefactoring.java
+++ b/org.eclipse.m2e.refactoring/src/org/eclipse/m2e/refactoring/exclude/ExcludeArtifactRefactoring.java
@@ -53,6 +53,7 @@ import org.sonatype.aether.util.artifact.JavaScopes;
 
 
 public class ExcludeArtifactRefactoring extends Refactoring {
+  private static final String PLUGIN_ID = "org.eclipse.m2e.refactoring";
 
   private ArtifactKey[] keys;
 
@@ -62,8 +63,6 @@ public class ExcludeArtifactRefactoring extends Refactoring {
 
   private List<MavenProject> hierarchy;
 
-  private Set<ArtifactKey> locatedKeys;
-
   public ExcludeArtifactRefactoring(IFile pomFile, ArtifactKey[] keys) {
     this.pomFile = pomFile;
     this.keys = keys;
@@ -122,7 +121,7 @@ public class ExcludeArtifactRefactoring extends Refactoring {
    */
   public RefactoringStatus checkFinalConditions(IProgressMonitor pm) throws CoreException, OperationCanceledException {
     changes = new ArrayList<Change>();
-    locatedKeys = new HashSet<ArtifactKey>();
+    Set<ArtifactKey> locatedKeys = new HashSet<ArtifactKey>();
     List<IStatus> statuses = new ArrayList<IStatus>();
     SubMonitor monitor = SubMonitor.convert(pm, getHierarchy().size());
 
@@ -130,8 +129,7 @@ public class ExcludeArtifactRefactoring extends Refactoring {
     // Exclusion point
     Visitor visitor = locate(exclusionPoint, monitor.newChild(1));
     for(Entry<Dependency, Set<ArtifactKey>> entry : visitor.getSourceMap().entrySet()) {
-      exclusionPoint.getOriginalModel().getDependencies();
-      locatedKeys.addAll(entry.getValue());
+        locatedKeys.addAll(entry.getValue());
       if(contains(entry.getValue(), entry.getKey())) {
         exclusionOp.add(new RemoveDependencyOperation(entry.getKey()));
       } else {
@@ -159,9 +157,6 @@ public class ExcludeArtifactRefactoring extends Refactoring {
           }
         }
       }
-      if(!visitor.getStatus().isOK()) {
-        statuses.add(visitor.getStatus());
-      }
     }
 
     // Above exclusion - Add dep to exclusionPoint
@@ -169,29 +164,31 @@ public class ExcludeArtifactRefactoring extends Refactoring {
       visitor = locate(project, monitor.newChild(1));
       for(Entry<Dependency, Set<ArtifactKey>> entry : locate(project, monitor.newChild(1)).getSourceMap().entrySet()) {
         locatedKeys.addAll(entry.getValue());
-        if(contains(entry.getValue(), entry.getKey())) {
+        Dependency dependency = entry.getKey();
+        if(contains(entry.getValue(), dependency)) {
           if(project.getFile() != null) {
-            changes.add(PomHelper.createChange(getFile(project), new RemoveDependencyOperation(entry.getKey()),
+            statuses.add(new Status(IStatus.INFO, PLUGIN_ID, NLS.bind("Dependency {0} will be removed from {1}",
+                toString(dependency), getMavenProjectFacade(project).getPom().getFullPath())));
+            changes.add(PomHelper.createChange(getFile(project), new RemoveDependencyOperation(dependency),
                 "Remove dependency {0}"));
           }
         } else {
-          exclusionOp.add(new AddDependencyOperation(entry.getKey()));
+          exclusionOp.add(new AddDependencyOperation(dependency));
           for(ArtifactKey key : entry.getValue()) {
-            exclusionOp.add(new AddExclusionOperation(entry.getKey(), key));
+            exclusionOp.add(new AddExclusionOperation(dependency, key));
           }
         }
       }
-      if(!visitor.getStatus().isOK()) {
-        statuses.add(visitor.getStatus());
-      }
     }
-    changes.add(PomHelper.createChange(getFile(exclusionPoint),
-        new CompoundOperation(exclusionOp.toArray(new Operation[exclusionOp.size()])), getName()));
+    if(!exclusionOp.isEmpty()) {
+      changes.add(PomHelper.createChange(getFile(exclusionPoint),
+          new CompoundOperation(exclusionOp.toArray(new Operation[exclusionOp.size()])), getName()));
+    }
 
     if(statuses.size() == 1) {
       return RefactoringStatus.create(statuses.get(0));
     } else if(statuses.size() > 1) {
-      return RefactoringStatus.create(new MultiStatus("org.eclipse.m2e.refactoring", 0, statuses
+      return RefactoringStatus.create(new MultiStatus(PLUGIN_ID, 0, statuses
           .toArray(new IStatus[statuses.size()]), "Errors occurred creating refactoring", null));
     } else if(locatedKeys.isEmpty()) {
       return RefactoringStatus.createFatalErrorStatus(Messages.AbstractPomHeirarchyRefactoring_noTargets);
@@ -279,9 +276,11 @@ public class ExcludeArtifactRefactoring extends Refactoring {
     return ancestors;
   }
 
-  private class Visitor implements DependencyVisitor {
-    private List<IStatus> statuses = new ArrayList<IStatus>();
+  private static String toString(Dependency dependency) {
+    return dependency.getArtifactId() + ":" + dependency.getArtifactId() + ":" + dependency.getVersion();
+  }
 
+  private class Visitor implements DependencyVisitor {
     private List<Dependency> dependencies;
 
     private Map<Dependency, Set<ArtifactKey>> sourceMap = new HashMap<Dependency, Set<ArtifactKey>>();
@@ -290,14 +289,6 @@ public class ExcludeArtifactRefactoring extends Refactoring {
       dependencies = project.getOriginalModel().getDependencies();
     }
 
-    IStatus getStatus() {
-      if(statuses.isEmpty()) {
-        return Status.OK_STATUS;
-      }
-      return new MultiStatus("org.eclipse.m2e.refactoring", 0, statuses.toArray(new IStatus[statuses.size()]),
-          "Errors occurred", null);
-    }
-
     Map<Dependency, Set<ArtifactKey>> getSourceMap() {
       return sourceMap;
     }
diff --git a/org.eclipse.m2e.refactoring/src/org/eclipse/m2e/refactoring/exclude/ExcludeWizardPage.java b/org.eclipse.m2e.refactoring/src/org/eclipse/m2e/refactoring/exclude/ExcludeWizardPage.java
index ea41f986..28458ba3 100644
--- a/org.eclipse.m2e.refactoring/src/org/eclipse/m2e/refactoring/exclude/ExcludeWizardPage.java
+++ b/org.eclipse.m2e.refactoring/src/org/eclipse/m2e/refactoring/exclude/ExcludeWizardPage.java
@@ -27,6 +27,7 @@ import org.eclipse.swt.layout.GridLayout;
 import org.eclipse.swt.widgets.Button;
 import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Label;
 import org.eclipse.ui.ISharedImages;
 import org.eclipse.ui.PlatformUI;
 
@@ -41,7 +42,7 @@ public class ExcludeWizardPage extends UserInputWizardPage implements SelectionL
 
   private IMavenProjectFacade facade;
 
-  private CLabel label;
+  private CLabel status;
 
   protected ExcludeWizardPage(IMavenProjectFacade facade) {
     super("Place to exclude");
@@ -56,26 +57,34 @@ public class ExcludeWizardPage extends UserInputWizardPage implements SelectionL
     setControl(composite);
     composite.setLayout(new GridLayout(1, false));
 
+    Label label = new Label(composite, SWT.NONE);
+    label.setLayoutData(new GridData(SWT.LEFT, SWT.TOP, true, false));
+    label.setText("Choose pom to place exclusion");
+
     currentPom = new Button(composite, SWT.RADIO);
-    currentPom.setLayoutData(new GridData(SWT.LEFT, SWT.CENTER, false, false));
-    currentPom.setText("Selected pom");
+    GridData gd_currentPom = new GridData(SWT.LEFT, SWT.CENTER, false, false, 1, 1);
+    gd_currentPom.horizontalIndent = 15;
+    currentPom.setLayoutData(gd_currentPom);
+    currentPom.setText(facade.getArtifactKey().toString());
     currentPom.setSelection(true);
     currentPom.addSelectionListener(this);
 
     hierarchy = new Button(composite, SWT.RADIO);
-    hierarchy.setLayoutData(new GridData(SWT.LEFT, SWT.TOP, false, false));
+    GridData gd_hierarchy = new GridData(SWT.LEFT, SWT.TOP, false, false);
+    gd_hierarchy.horizontalIndent = 15;
+    hierarchy.setLayoutData(gd_hierarchy);
     hierarchy.setText("Choose from project hierarchy");
     hierarchy.addSelectionListener(this);
 
     pomHierarchy = new PomHierarchyComposite(composite, SWT.BORDER);
     GridData gd_pomHierarchy = new GridData(SWT.FILL, SWT.FILL, true, true);
-    gd_pomHierarchy.horizontalIndent = 15;
+    gd_pomHierarchy.horizontalIndent = 35;
     pomHierarchy.setLayoutData(gd_pomHierarchy);
     pomHierarchy.setEnabled(false);
     pomHierarchy.addSelectionChangedListener(this);
 
-    label = new CLabel(composite, SWT.NONE);
-    label.setLayoutData(new GridData(SWT.LEFT, SWT.CENTER, true, false));
+    status = new CLabel(composite, SWT.NONE);
+    status.setLayoutData(new GridData(SWT.FILL, SWT.BOTTOM, true, false));
 
     Display.getCurrent().asyncExec(new Runnable() {
       public void run() {
@@ -106,11 +115,11 @@ public class ExcludeWizardPage extends UserInputWizardPage implements SelectionL
 
   private void setStatus(String msg) {
     if(msg == null) {
-      label.setImage(null);
-      label.setText("");
+      status.setImage(null);
+      status.setText("");
     } else {
-      label.setText(msg);
-      label.setImage(PlatformUI.getWorkbench().getSharedImages().getImage(ISharedImages.IMG_OBJS_ERROR_TSK));
+      status.setText(msg);
+      status.setImage(PlatformUI.getWorkbench().getSharedImages().getImage(ISharedImages.IMG_OBJS_ERROR_TSK));
     }
   }
 
@@ -118,19 +127,27 @@ public class ExcludeWizardPage extends UserInputWizardPage implements SelectionL
     ExcludeArtifactRefactoring refactoring = (ExcludeArtifactRefactoring) getRefactoring();
     if (hierarchy.getSelection()) {
       MavenProject project = fromSelection(pomHierarchy.getSelection());
-      setPageComplete(project != null && project.getFile() != null);
-      if(project != null && project.getFile() == null) {
-        setStatus("Changes must occur within the workspace");
-      } else {
-        setStatus(null);
-        refactoring.setExclusionPoint(fromSelection(pomHierarchy.getSelection()));
-      }
+      updateStatusBar(project);
+      refactoring.setExclusionPoint(project);
     } else {
-      setStatus(null);
+      updateStatusBar(facade.getMavenProject());
       refactoring.setExclusionPoint(facade.getMavenProject());
     }
   }
 
+  private void updateStatusBar(MavenProject project) {
+    if(project == null) {
+      setStatus("Select a workspace pom");
+      setPageComplete(false);
+    } else if(project.getFile() == null) {
+      setStatus("Changes must occur within the workspace");
+      setPageComplete(false);
+    } else {
+      setStatus(null);
+      setPageComplete(true);
+    }
+  }
+
   /* (non-Javadoc)
    * @see org.eclipse.jface.viewers.ISelectionChangedListener#selectionChanged(org.eclipse.jface.viewers.SelectionChangedEvent)
    */