Merge pull request #184 from phantomjinx/TEIIDDES-1755

Teiiddes 1755 and Move Refactoring Fixes
Teiid-Designer · Jun 12, 2013 · 8da0bc7 · 8da0bc7
2 parents 049e8e7 + 4f3c718
commit 8da0bc7
Show file tree

Hide file tree

Showing 6 changed files with 106 additions and 28 deletions.
diff --git a/...ins/org.teiid.designer.ui/src/org/teiid/designer/ui/explorer/ModelExplorerMoveAction.java b/...ins/org.teiid.designer.ui/src/org/teiid/designer/ui/explorer/ModelExplorerMoveAction.java
@@ -105,9 +105,6 @@ private IActionDelegate getMoveActionDelegate() {
     @Override
     protected boolean updateSelection(IStructuredSelection selection) {
         boolean bResult = false;
-
-        if ( selection.size() > 1 )
-            return false;
 
         List lstResourceObjects = SelectionUtilities.getSelectedIResourceObjects( selection );
         if ( lstResourceObjects.size() > 0 ) {

diff --git a/...rg.teiid.designer.ui/src/org/teiid/designer/ui/refactor/AbstractResourcesRefactoring.java b/...rg.teiid.designer.ui/src/org/teiid/designer/ui/refactor/AbstractResourcesRefactoring.java
@@ -9,17 +9,22 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IResource;
+import org.eclipse.core.resources.IResourceVisitor;
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
 import org.eclipse.ltk.core.refactoring.Change;
 import org.eclipse.ltk.core.refactoring.Refactoring;
 import org.eclipse.ltk.core.refactoring.RefactoringStatus;
 import org.eclipse.ltk.core.refactoring.TextFileChange;
+import org.teiid.designer.core.ModelerCore;
 import org.teiid.designer.core.util.KeyInValueHashMap;
 import org.teiid.designer.core.util.KeyInValueHashMap.KeyFromValueAdapter;
 import org.teiid.designer.ui.refactor.RefactorResourcesUtils.AbstractResourceCallback;
@@ -32,6 +37,8 @@ public abstract class AbstractResourcesRefactoring extends Refactoring {
 
     private final List<IResource> resources;
 
+    private Set<IResource> resourcesAndChildren;
+
     private Map<IResource, Collection<Change>> changes = new LinkedHashMap<IResource, Collection<Change>>();
 
     private final KeyInValueHashMap<IFile, VdbResourceChange> vdbChanges;
@@ -83,6 +90,36 @@ public List<IResource> getResources() {
         return resources;
     }
 
+    /**
+     * Returns all the resources as well as their children
+     *
+     * @param status to be populate if there are any errors with collecting the resources
+     *
+     * @return the resourcesAndChildren
+     */
+    public Set<IResource> getResourcesAndChildren(RefactoringStatus status) {
+        if (this.resourcesAndChildren == null) {
+            this.resourcesAndChildren = new HashSet<IResource>();
+
+            for (IResource resource : resources) {
+                try {
+                    resource.accept(new IResourceVisitor() {
+                        @Override
+                        public boolean visit(IResource visitedResource) {
+                            resourcesAndChildren.add(visitedResource);
+                            return true;
+                        }
+                    }, IResource.DEPTH_INFINITE, false);
+                } catch (Exception err) {
+                    ModelerCore.Util.log(IStatus.ERROR, err, err.getMessage());
+                    status.merge(RefactoringStatus.createFatalErrorStatus(err.getMessage()));
+                }
+            }
+        }
+
+        return this.resourcesAndChildren;
+    }
+
     /**
      * Provides a specific set of tests to test the given resource to ensure
      * that the operation can continue. This can then be used in both the

diff --git a/plugins/org.teiid.designer.ui/src/org/teiid/designer/ui/refactor/RefactorResourcesUtils.java b/plugins/org.teiid.designer.ui/src/org/teiid/designer/ui/refactor/RefactorResourcesUtils.java
@@ -434,20 +434,29 @@ public static TextFileChange calculateTextChanges(IFile file, Collection<PathPai
 
     /**
      * Finds the import locations in the given file and calculates the modified paths against the 
-     * given destination
+     * given destination. If an import location points to a resource in the given set then nothing
+     * should be done since that resource is also being moved to the destination and no change
+     * is necessary.
      *
-     * @param file
-     * @param destination
+     * @param file the file to be refactored
+     * @param destination the location the file is to be refactored to
+     * @param refactorResources the collection of all resources being refactored
      *
      * @return a set of path pairs representing the import locations
      *
      * @throws Exception
      */
-    public static Set<PathPair> calculateImportChanges(IFile file, String destination) throws Exception {
+    public static Set<PathPair> calculateImportChanges(IFile file, String destination, Set<IResource> refactorResources) throws Exception {
         File nativeFile = file.getRawLocation().makeAbsolute().toFile();
         if (nativeFile == null || ! nativeFile.exists())
             throw new Exception(getString("ResourcesRefactoring.fileNotFoundError", file.getFullPath())); //$NON-NLS-1$
 
+        Set<String> refactorResourcePaths = new HashSet<String>();
+        for (IResource resource : refactorResources) {
+            File nativeRes = resource.getRawLocation().makeAbsolute().toFile();
+            refactorResourcePaths.add(nativeRes.getCanonicalPath());
+        }
+
         Set<PathPair> importPairs = new HashSet<PathPair>();
         File parentFolder = nativeFile.getParentFile();
 
@@ -474,6 +483,8 @@ public static Set<PathPair> calculateImportChanges(IFile file, String destinatio
             // Find the absolute path of the model location based on the location of the file
             File absLocationFile = new File(parentFolder, relativeLocation);
             String absLocation = absLocationFile.getCanonicalPath();
+            if (refactorResourcePaths.contains(absLocation))
+                continue;
 
             // Use the new proposed location of the file to extrapolate the relative path of the
             // import location. This takes advantage of the getRelativePath function by adding

diff --git a/...iid.designer.ui/src/org/teiid/designer/ui/refactor/delete/DeleteResourcesRefactoring.java b/...iid.designer.ui/src/org/teiid/designer/ui/refactor/delete/DeleteResourcesRefactoring.java
@@ -13,7 +13,6 @@
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IResource;
-import org.eclipse.core.resources.IResourceVisitor;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
@@ -24,7 +23,6 @@
 import org.eclipse.ltk.core.refactoring.CompositeChange;
 import org.eclipse.ltk.core.refactoring.RefactoringStatus;
 import org.eclipse.ltk.core.refactoring.resource.DeleteResourceChange;
-import org.teiid.designer.core.ModelerCore;
 import org.teiid.designer.core.refactor.IRefactorModelHandler.RefactorType;
 import org.teiid.designer.core.refactor.RelatedResourceFinder.Relationship;
 import org.teiid.designer.core.workspace.ModelUtil;
@@ -59,19 +57,25 @@ public void indexFile(IResource resource, IFile relatedFile, RefactoringStatus s
 
             RefactorResourcesUtils.unloadModelResource(relatedFile);
 
-            if (! resourcesAndChildren.contains(relatedFile)) {
+            if (! getResourcesAndChildren(status).contains(relatedFile)) {
                 // file is NOT already being deleted
                 DeleteResourceChange change = new DeleteResourceChange(relatedFile.getFullPath(), true, isDeleteContents());
                 addChange(relatedFile, change);
             }
 
             RefactorResourcesUtils.calculateRelatedVdbResources(relatedFile, status, this);
         }
+
+        @Override
+        public void indexVdb(IResource resource, IFile vdbFile, RefactoringStatus status) {
+            if (! getResourcesAndChildren(status).contains(vdbFile)) {
+                // vdb is NOT already being deleted
+                super.indexVdb(resource, vdbFile, status);
+            }
+        }
     }
 
     private boolean deleteContents;
-
-    private Set<IResource> resourcesAndChildren = new HashSet<IResource>();
 
     /**
      * @param selectedResources
@@ -154,21 +158,6 @@ public RefactoringStatus checkInitialConditions(final IProgressMonitor progressM
             for (IResource resource : getResources()) {
                 checkResource(resource, progressMonitor, status);
                 if (! status.isOK()) break;
-
-                // Accumulate all the resources that will be deleted so that scheduleRemovedRelatedFile()
-                // does not add a delete change for a resource already being deleted
-                try {
-                    resource.accept(new IResourceVisitor() {
-                        @Override
-                        public boolean visit(IResource visitedResource) {
-                            resourcesAndChildren.add(visitedResource);
-                            return true;
-                        }
-                    }, IResource.DEPTH_INFINITE, false);
-                } catch (Exception err) {
-                    ModelerCore.Util.log(IStatus.ERROR, err, err.getMessage());
-                    status.merge(RefactoringStatus.createFatalErrorStatus(err.getMessage()));
-                }
 
                 // Check validity of related resources
                 IResourceCallback callback = new AbstractResourceCallback() {

diff --git a/...ins/org.teiid.designer.ui/src/org/teiid/designer/ui/refactor/move/MoveRefactorAction.java b/...ins/org.teiid.designer.ui/src/org/teiid/designer/ui/refactor/move/MoveRefactorAction.java
@@ -10,6 +10,7 @@
 import java.util.List;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IResource;
+import org.eclipse.core.runtime.IPath;
 import org.eclipse.jface.action.IAction;
 import org.eclipse.jface.viewers.ISelection;
 import org.eclipse.ltk.ui.refactoring.RefactoringWizard;
@@ -44,6 +45,25 @@ public void selectionChanged(IAction action, ISelection selection) {
                 return;
             }
         }
+
+        /*
+         * Check the resources being moved are in the same directory.
+         *
+         * This is a limitation of the move but avoids more difficult
+         * problems with keeping track of location changes with the
+         * resources being moved.
+        */
+        IPath parentDirectory = null;
+        for (IResource resource : resources) {
+            IPath path = resource.getFullPath();
+            path = path.uptoSegment(path.segmentCount() - 1);
+            if (parentDirectory == null) {
+                parentDirectory = path;
+            } else if (! parentDirectory.equals(path)) {
+                action.setEnabled(false);
+                return;
+            }
+        }
 
         super.selectionChanged(action, selection);
     }

diff --git a/...g.teiid.designer.ui/src/org/teiid/designer/ui/refactor/move/MoveResourcesRefactoring.java b/...g.teiid.designer.ui/src/org/teiid/designer/ui/refactor/move/MoveResourcesRefactoring.java
@@ -44,6 +44,8 @@ private class RelatedResourceCallback extends VdbResourceCallback {
 
         private final Set<PathPair> pathPairs;
 
+        private final Set<IResource> history = new HashSet<IResource>();
+
         /**
          * @param pathPairs
          */
@@ -60,6 +62,28 @@ public void checkValidFile(IFile relatedFile, RefactoringStatus status) {
         public void indexFile(IResource resource, IFile relatedFile, RefactoringStatus status) throws Exception {
             RefactorResourcesUtils.unloadModelResource(relatedFile);
 
+            if (history.contains(relatedFile)) {
+                /*
+                 * This file has been processed before implying that it is related
+                 * to more than one target resource.
+                 *
+                 * Since pathpairs are global to all resources there is
+                 * no point in processing the related file more than once.
+                 */
+                return;
+            }
+
+            // Add the related file to the history to avoid processing it again
+            history.add(relatedFile);
+
+            if (getResourcesAndChildren(status).contains(relatedFile)) {
+                /*
+                 * Changes are not applicable as the related file is moving
+                 * to the new destination as well
+                 */
+                return;
+            }
+
             IPath relatedFilePath = relatedFile.getRawLocation().makeAbsolute();
             IPath relatedParentPath = relatedFilePath.removeLastSegments(1);
 
@@ -181,7 +205,7 @@ public RefactoringStatus checkFinalConditions(IProgressMonitor progressMonitor)
                 // Find the resource's imports as they need to be updated due to the resource move
                 if (ModelUtil.isModelFile(resource)) {
                     IFile file = (IFile) resource;
-                    Set<PathPair> importPathPairs = RefactorResourcesUtils.calculateImportChanges(file, destinationPath);
+                    Set<PathPair> importPathPairs = RefactorResourcesUtils.calculateImportChanges(file, destinationPath, getResourcesAndChildren(status));
                     TextFileChange textFileChange = RefactorResourcesUtils.calculateTextChanges(file, importPathPairs);
                     if (addTextChange(file, textFileChange)) {
                         // Calculate the effect on any vdbs containing this modified related file