tmf: Bug 494689: fix failing name conflict handling in trace import

This patch also adds test cases for RENAME_ALL and OVERWRITE_ALL in
the corresponding SWTBot test class.

It also fixes the test cases when overwriting existing resources
without confirmation.

Change-Id: I7b4a7779091d824502ea15a18573f162304d143b
Signed-off-by: Bernd Hufmann <Bernd.Hufmann@ericsson.com>
Reviewed-on: https://git.eclipse.org/r/73759
Reviewed-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
Tested-by: Jean-Christian Kouame <jean-christian.kouame@ericsson.com>
Reviewed-by: Hudson CI
Reviewed-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
Tested-by: Marc-Andre Laperle <marc-andre.laperle@ericsson.com>

diff --git a/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/AbstractImportAndReadSmokeTest.java b/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/AbstractImportAndReadSmokeTest.java
index 4a6a63e3901154ff5c990deff812d2ad91545a17..204fef13e81d7dab7484dad7443b88341114f49d 100644 (file)
--- a/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/AbstractImportAndReadSmokeTest.java
+++ b/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/AbstractImportAndReadSmokeTest.java
@@ -37,6 +37,7 @@ import org.eclipse.swtbot.swt.finder.widgets.SWTBotText;
 import org.eclipse.swtbot.swt.finder.widgets.SWTBotToolbarButton;
 import org.eclipse.swtbot.swt.finder.widgets.SWTBotTree;
 import org.eclipse.swtbot.swt.finder.widgets.SWTBotTreeItem;
+import org.eclipse.tracecompass.internal.tmf.ui.project.wizards.importtrace.ImportConfirmation;
 import org.eclipse.tracecompass.internal.tmf.ui.views.statistics.TmfStatisticsViewImpl;
 import org.eclipse.tracecompass.testtraces.ctf.CtfTestTrace;
 import org.eclipse.tracecompass.tmf.core.signal.TmfSelectionRangeUpdatedSignal;
@@ -149,9 +150,33 @@ public abstract class AbstractImportAndReadSmokeTest {
      * Finishes the wizard
      */
     protected void importFinish() {
+        importFinish(ImportConfirmation.CONTINUE);
+    }
+
+    /**
+     * Finishes the wizard
+     *
+     * @param confirmationMode
+     *            a confirmation value
+     *            Note: Only {@link ImportConfirmation#RENAME_ALL},
+     *            {@link ImportConfirmation#OVERWRITE_ALL},
+     *            {@link ImportConfirmation#CONTINUE} are supported
+     */
+    protected void importFinish(ImportConfirmation confirmationMode) {
         SWTBotShell shell = fBot.activeShell();
         final SWTBotButton finishButton = fBot.button("Finish");
         finishButton.click();
+        if (confirmationMode == ImportConfirmation.RENAME_ALL) {
+            fBot.waitUntil(Conditions.shellIsActive("Confirmation"));
+            SWTBotShell shell2 = fBot.activeShell();
+            SWTBotButton button = shell2.bot().button("Rename All");
+            button.click();
+        } else if (confirmationMode == ImportConfirmation.OVERWRITE_ALL) {
+            fBot.waitUntil(Conditions.shellIsActive("Confirmation"));
+            SWTBotShell shell2 = fBot.activeShell();
+            SWTBotButton button = shell2.bot().button("Overwrite All");
+            button.click();
+        }
         fBot.waitUntil(Conditions.shellCloses(shell));
         SWTBotUtils.waitForJobs();
     }

diff --git a/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/StandardImportAndReadSmokeTest.java b/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/StandardImportAndReadSmokeTest.java
index 7be0215c63749cc92bf7c24164e37385e7f596d9..da529b4703ce1dadd391362fc8ff679ddb8be4ce 100644 (file)
--- a/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/StandardImportAndReadSmokeTest.java
+++ b/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/StandardImportAndReadSmokeTest.java
@@ -57,6 +57,7 @@ import org.eclipse.swtbot.swt.finder.widgets.SWTBotText;
 import org.eclipse.swtbot.swt.finder.widgets.SWTBotTree;
 import org.eclipse.swtbot.swt.finder.widgets.SWTBotTreeItem;
 import org.eclipse.swtbot.swt.finder.widgets.TimeoutException;
+import org.eclipse.tracecompass.internal.tmf.ui.project.wizards.importtrace.ImportConfirmation;
 import org.eclipse.tracecompass.internal.tmf.ui.project.wizards.importtrace.ImportTraceWizard;
 import org.eclipse.tracecompass.internal.tmf.ui.project.wizards.importtrace.ImportTraceWizardPage;
 import org.eclipse.tracecompass.internal.tmf.ui.project.wizards.importtrace.Messages;
@@ -156,7 +157,7 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe
      */
     @Test
     public void testImportWithExperimentValidation() throws Exception {
-        testImport(ImportTraceWizardPage.OPTION_CREATE_EXPERIMENT, false, false, false);
+        testImport(ImportTraceWizardPage.OPTION_CREATE_EXPERIMENT, false, false, false, true, ImportConfirmation.CONTINUE);
     }
 
     /**
@@ -190,10 +191,34 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe
      */
     @Test
     public void testImportFromDirectoryOverwrite() throws Exception {
-        testImport(0, false, false);
+        testImport(0, false, false, true, false, ImportConfirmation.CONTINUE);
         testImport(ImportTraceWizardPage.OPTION_OVERWRITE_EXISTING_RESOURCES, false, false);
     }
 
+    /**
+     * Test import from directory, overwrite all
+     *
+     * @throws Exception
+     *             on error
+     */
+    @Test
+    public void testImportFromDirectoryOverwriteRenameAll() throws Exception {
+        testImport(0, false, false, true, false, ImportConfirmation.CONTINUE);
+        testImport(0, false, false, true, true, ImportConfirmation.RENAME_ALL);
+    }
+
+    /**
+     * Test import from directory, overwrite all
+     *
+     * @throws Exception
+     *             on error
+     */
+    @Test
+    public void testImportFromDirectoryOverwriteOverwriteAll() throws Exception {
+        testImport(0, false, false, true, false, ImportConfirmation.CONTINUE);
+        testImport(0, false, false, true, true, ImportConfirmation.OVERWRITE_ALL);
+    }
+
     /**
      * Test import from archive
      *
@@ -233,7 +258,7 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe
      */
     @Test
     public void testImportFromArchiveOverwrite() throws Exception {
-        testImport(0, false, true);
+        testImport(0, false, true, true, false, ImportConfirmation.CONTINUE);
         testImport(ImportTraceWizardPage.OPTION_OVERWRITE_EXISTING_RESOURCES, false, true);
     }
 
@@ -426,10 +451,10 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe
     }
 
     private void testImport(int options, boolean testViews, boolean fromArchive) throws Exception {
-        testImport(options, testViews, fromArchive, true);
+        testImport(options, testViews, fromArchive, true, true, ImportConfirmation.CONTINUE);
     }
 
-    private void testImport(int options, boolean testViews, boolean fromArchive, boolean defaultExperiment) throws Exception {
+    private void testImport(int options, boolean testViews, boolean fromArchive, boolean defaultExperiment, boolean clearTraces, ImportConfirmation confirmationMode) throws Exception {
         String expectedSourceLocation = null;
 
         @NonNull String experimentName;
@@ -464,14 +489,20 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe
         }
         checkFinishButton(true);
 
-        importFinish();
+        importFinish(confirmationMode);
 
         IPath expectedElementPath = new Path(TRACE_NAME);
         if ((options & ImportTraceWizardPage.OPTION_PRESERVE_FOLDER_STRUCTURE) != 0) {
             expectedElementPath = new Path(TRACE_FOLDER).append(expectedElementPath);
         }
 
-        checkOptions(options, expectedSourceLocation, expectedElementPath, experimentName);
+        if (confirmationMode == ImportConfirmation.RENAME_ALL) {
+            IPath expectedElementPathRenamed = new Path(TRACE_NAME + "(2)");
+            checkOptions(options, expectedSourceLocation, expectedElementPath, experimentName, expectedElementPathRenamed);
+        } else {
+            checkOptions(options, expectedSourceLocation, expectedElementPath, experimentName, null);
+        }
+
         TmfEventsEditor tmfEd = SWTBotUtils.openEditor(fBot, TRACE_PROJECT_NAME, expectedElementPath);
         if (testViews) {
             testViews(tmfEd);
@@ -480,7 +511,9 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe
         fBot.closeAllEditors();
 
         SWTBotUtils.clearExperimentFolder(fBot, TRACE_PROJECT_NAME);
-        SWTBotUtils.clearTracesFolder(fBot, TRACE_PROJECT_NAME);
+        if (clearTraces) {
+            SWTBotUtils.clearTracesFolder(fBot, TRACE_PROJECT_NAME);
+        }
     }
 
     private void testImportAndExtractArchives(int options, boolean testViews, boolean fromArchive) throws Exception {
@@ -731,10 +764,10 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe
     }
 
     private static void checkOptions(int optionFlags, String expectedSourceLocation, IPath expectedElementPath) throws CoreException {
-        checkOptions(optionFlags, expectedSourceLocation, expectedElementPath, null);
+        checkOptions(optionFlags, expectedSourceLocation, expectedElementPath, null, null);
     }
 
-    private static void checkOptions(int optionFlags, String expectedSourceLocation, IPath expectedElementPath, String experimentName) throws CoreException {
+    private static void checkOptions(int optionFlags, String expectedSourceLocation, IPath expectedElementPath, String experimentName, IPath expectedElementPathRenamed) throws CoreException {
         IProject project = getProjectResource();
         assertTrue(project.exists());
         TmfProjectElement tmfProject = TmfProjectRegistry.getProject(project, true);
@@ -759,6 +792,12 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe
         IPath expectedPath = Path.ROOT.append(new Path(TRACE_PROJECT_NAME)).append(TmfTracesFolder.TRACES_FOLDER_NAME).append(expectedElementPath);
         assertEquals(expectedPath, traceResource.getFullPath());
 
+        if (expectedElementPathRenamed != null) {
+            IPath expectedPathRenamed = Path.ROOT.append(new Path(TRACE_PROJECT_NAME)).append(TmfTracesFolder.TRACES_FOLDER_NAME).append(expectedElementPathRenamed);
+            IResource traceResourceRenamed = traces.get(1).getResource();
+            assertEquals(expectedPathRenamed, traceResourceRenamed.getFullPath());
+        }
+
         String sourceLocation = traceResource.getPersistentProperty(TmfCommonConstants.SOURCE_LOCATION);
         assertNotNull(sourceLocation);
         assertEquals(expectedSourceLocation, sourceLocation);

diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/ImportConflictHandler.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/ImportConflictHandler.java
index 1c2f783ff6f88e1b8646b589b98c62c2f7e40d42..687df41bfba9ba6fb2165cd30cad6811cba52a96 100644 (file)
--- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/ImportConflictHandler.java
+++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/ImportConflictHandler.java
@@ -15,6 +15,7 @@ import java.util.List;
 
 import org.eclipse.core.resources.IContainer;
 import org.eclipse.core.resources.IResource;
+import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.IProgressMonitor;
@@ -118,7 +119,7 @@ public class ImportConflictHandler {
     // ------------------------------------------------------------------------
     private ImportConfirmation checkForNameClash(IPath tracePath) throws InterruptedException {
         // handle rename
-        if (getExistingTrace(tracePath) != null) {
+        if (getExistingResource(tracePath) != null) {
             if ((fConfirmationMode == ImportConfirmation.RENAME_ALL) ||
                     (fConfirmationMode == ImportConfirmation.OVERWRITE_ALL) ||
                     (fConfirmationMode == ImportConfirmation.SKIP_ALL)) {
@@ -138,7 +139,7 @@ public class ImportConflictHandler {
 
     private int promptForOverwrite(IPath tracePath) {
         final MessageDialog dialog = new MessageDialog(fShell,
-                 null, null, NLS.bind(Messages.ImportTraceWizard_TraceAlreadyExists, tracePath.makeRelativeTo(fTraceFolderElement.getProject().getPath())),
+                Messages.ImportTraceWizard_MessageTitle, null, NLS.bind(Messages.ImportTraceWizard_TraceAlreadyExists, tracePath.makeRelativeTo(fTraceFolderElement.getProject().getPath())),
                 MessageDialog.QUESTION, new String[] {
                         ImportConfirmation.RENAME.getInName(),
                         ImportConfirmation.RENAME_ALL.getInName(),
@@ -164,18 +165,19 @@ public class ImportConflictHandler {
         return returnValue[0];
     }
 
-    private String rename(IPath tracePath) {
-        TmfTraceElement trace = getExistingTrace(tracePath);
-        if (trace == null) {
+    private static String rename(IPath tracePath) {
+        IResource existingResource = getExistingResource(tracePath);
+        if (existingResource == null) {
             return tracePath.lastSegment();
         }
 
         // Not using IFolder on purpose to leave the door open to import
         // directly into an IProject
-        IContainer folder = (IContainer) trace.getParent().getResource();
+        IContainer folder = existingResource.getParent();
+
         int i = 2;
         while (true) {
-            String name = trace.getName() + '(' + Integer.toString(i++) + ')';
+            String name = existingResource.getName() + '(' + Integer.toString(i++) + ')';
             IResource resource = folder.findMember(name);
             if (resource == null) {
                 return name;
@@ -184,12 +186,19 @@ public class ImportConflictHandler {
     }
 
     private void delete(IPath tracePath, IProgressMonitor monitor) throws CoreException {
-        TmfTraceElement trace = getExistingTrace(tracePath);
-        if (trace == null) {
+        IResource existingResource = getExistingResource(tracePath);
+        if (existingResource == null) {
+            return;
+        }
+        TmfTraceElement existingTraceElement = getExistingTrace(tracePath);
+        if (existingTraceElement != null) {
+            // Delete existing TmfTraceElement
+            existingTraceElement.delete(monitor);
             return;
         }
 
-        trace.delete(monitor);
+        // Delete resource existing in workspace
+        existingResource.delete(true, monitor);
     }
 
     private TmfTraceElement getExistingTrace(IPath tracePath) {
@@ -201,5 +210,10 @@ public class ImportConflictHandler {
         }
         return null;
     }
+
+    private static IResource getExistingResource(IPath tracePath) {
+        // Look for existing resource
+        return ResourcesPlugin.getWorkspace().getRoot().findMember(tracePath);
+    }
 }
 

diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/Messages.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/Messages.java
index 8e1995ef8ddf0c9fc4f46a79e7d70f3072dadc39..34128102e48f46d1b5d720b19ab79a98bbe36ea8 100644 (file)
--- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/Messages.java
+++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/Messages.java
@@ -83,6 +83,10 @@ public class Messages extends NLS {
      * The error message when a trace validation failed (import trace wizard).
      */
     public static String ImportTraceWizard_TraceValidationFailed;
+    /**
+     * The title of message dialog (import trace wizard).
+     */
+    public static String ImportTraceWizard_MessageTitle;
     /**
      * The error message when a trace already exists in project (import trace wizard).
      */

diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/messages.properties b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/messages.properties
index a1af39e147255e0c54889531cd188e7c8c44021f..760cc146e2ee314726200085501f95901d2cfd71 100644 (file)
--- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/messages.properties
+++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/messages.properties
@@ -26,6 +26,7 @@ ImportTraceWizard_CreateLinksInWorkspace=Create lin&ks in workspace
 ImportTraceWizard_PreserveFolderStructure=Preserve &folder structure
 ImportTraceWizard_CreateExperiment=Create experiment
 ImportTraceWizard_TraceValidationFailed=Validation failed for trace resource {0}
+ImportTraceWizard_MessageTitle=Confirmation
 ImportTraceWizard_TraceAlreadyExists=Trace with name "{0}" already exists in project. Do you want to rename, overwrite or skip?
 ImportTraceWizard_ImportConfigurationRename=Rename
 ImportTraceWizard_ImportConfigurationRenameAll=Rename All