TMF: Validate new trace type before setting it

Also change the return type of TraceTypeHelper#validate to return an IStatus
instead. This patch fixes bug #431646

Change-Id: I893ade44645f92dfe231cb603e16e6fbf17209eb
Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
Reviewed-on: https://git.eclipse.org/r/24208
Tested-by: Hudson CI
Reviewed-by: Patrick Tasse <patrick.tasse@gmail.com>
IP-Clean: Patrick Tasse <patrick.tasse@gmail.com>

4 files changed, 28 insertions, 63 deletions

diff --git a/lttng/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/project/model/TmfTraceType.java b/lttng/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/project/model/TmfTraceType.java
index b62b58e441..1949e39d42 100644
--- a/lttng/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/project/model/TmfTraceType.java
+++ b/lttng/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/project/model/TmfTraceType.java
@@ -411,19 +411,21 @@ public final class TmfTraceType {
                         trace = (ITmfTrace) ce.createExecutableExtension(TmfTraceType.EXPERIMENT_TYPE_ATTR);
                         elementType = TraceElementType.EXPERIMENT;
                     }
-                    if (trace != null) {
-                        // Deregister trace as signal handler because it is only
-                        // used for validation
-                        TmfSignalManager.deregister(trace);
+                    if (trace == null) {
+                        break;
                     }
+                    // Deregister trace as signal handler because it is only
+                    // used for validation
+                    TmfSignalManager.deregister(trace);
+
+                    final String dirString = ce.getAttribute(TmfTraceType.IS_DIR_ATTR);
+                    boolean isDir = Boolean.parseBoolean(dirString);
+
+                    TraceTypeHelper tt = new TraceTypeHelper(typeId, category, attribute, trace, isDir, elementType);
+                    fTraceTypes.put(typeId, tt);
                 } catch (CoreException e) {
                 }
 
-                final String dirString = ce.getAttribute(TmfTraceType.IS_DIR_ATTR);
-                boolean isDir = Boolean.parseBoolean(dirString);
-
-                TraceTypeHelper tt = new TraceTypeHelper(typeId, category, attribute, trace, isDir, elementType);
-                fTraceTypes.put(typeId, tt);
             }
         }
     }
@@ -488,7 +490,7 @@ public final class TmfTraceType {
     public boolean validate(String traceTypeName, String fileName) {
         if (traceTypeName != null && !traceTypeName.isEmpty()) {
             final TraceTypeHelper traceTypeHelper = fTraceTypes.get(traceTypeName);
-            if (!traceTypeHelper.validate(fileName)) {
+            if (!traceTypeHelper.validate(fileName).isOK()) {
                 return false;
             }
         }
@@ -604,7 +606,7 @@ public final class TmfTraceType {
         final Iterable<TraceTypeHelper> traceTypeHelpers = getTraceTypeHelpers();
         for (TraceTypeHelper traceTypeHelper : traceTypeHelpers) {
             if (traceTypeHelper.isDirectoryTraceType() &&
-                    traceTypeHelper.validate(path)) {
+                    traceTypeHelper.validate(path).isOK()) {
                 return true;
             }
         }
diff --git a/lttng/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/project/model/TraceTypeHelper.java b/lttng/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/project/model/TraceTypeHelper.java
index 49881bff34..b647038809 100644
--- a/lttng/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/project/model/TraceTypeHelper.java
+++ b/lttng/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/project/model/TraceTypeHelper.java
@@ -15,6 +15,7 @@
 package org.eclipse.linuxtools.tmf.core.project.model;
 
 import org.eclipse.core.runtime.IStatus;
+import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.linuxtools.tmf.core.project.model.TmfTraceType.TraceElementType;
 import org.eclipse.linuxtools.tmf.core.trace.ITmfTrace;
 import org.eclipse.linuxtools.tmf.core.trace.TraceValidationStatus;
@@ -32,6 +33,7 @@ public class TraceTypeHelper {
     private final String fCategoryName;
     private final String fCanonicalName;
     private final TraceElementType fElementType;
+    @NonNull
     private final ITmfTrace fTrace;
     private final boolean fIsDirectory;
 
@@ -53,7 +55,7 @@ public class TraceTypeHelper {
      * @param elementType
      *            True if this helper is for an experiment type
      */
-    public TraceTypeHelper(String canonicalName, String categoryName, String name, ITmfTrace trace, boolean isDir, TraceElementType elementType) {
+    public TraceTypeHelper(String canonicalName, String categoryName, String name, @NonNull ITmfTrace trace, boolean isDir, TraceElementType elementType) {
         fName = name;
         fCategoryName = categoryName;
         fCanonicalName = canonicalName;
@@ -96,12 +98,8 @@ public class TraceTypeHelper {
      *            the trace to validate
      * @return whether it passes the validation
      */
-    public boolean validate(String path) {
-        boolean valid = false;
-        if (fTrace != null) {
-            valid = standardValidate(path);
-        }
-        return valid;
+    public IStatus validate(String path) {
+        return fTrace.validate(null, path);
     }
 
     /**
@@ -114,13 +112,11 @@ public class TraceTypeHelper {
      */
     public int validateWithConfidence(String path) {
         int result = -1;
-        if (fTrace != null) {
-            IStatus status = fTrace.validate(null, path);
-            if (status.isOK()) {
-                result = 0;
-                if (status instanceof TraceValidationStatus) {
-                    result = ((TraceValidationStatus) status).getConfidence();
-                }
+        IStatus status = fTrace.validate(null, path);
+        if (status.isOK()) {
+            result = 0;
+            if (status instanceof TraceValidationStatus) {
+                result = ((TraceValidationStatus) status).getConfidence();
             }
         }
         return result;
@@ -144,11 +140,6 @@ public class TraceTypeHelper {
         return fElementType == TraceElementType.EXPERIMENT;
     }
 
-    private boolean standardValidate(String path) {
-        final boolean valid = fTrace.validate(null, path).isOK();
-        return valid;
-    }
-
     /**
      * Get the class associated with this trace type
      *
diff --git a/lttng/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/internal/tmf/ui/project/handlers/SelectTraceTypeHandler.java b/lttng/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/internal/tmf/ui/project/handlers/SelectTraceTypeHandler.java
index 95e4e7545c..b8990b5575 100644
--- a/lttng/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/internal/tmf/ui/project/handlers/SelectTraceTypeHandler.java
+++ b/lttng/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/internal/tmf/ui/project/handlers/SelectTraceTypeHandler.java
@@ -23,7 +23,6 @@ import java.util.Set;
 import org.eclipse.core.commands.AbstractHandler;
 import org.eclipse.core.commands.ExecutionEvent;
 import org.eclipse.core.commands.ExecutionException;
-import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IResource;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IStatus;
@@ -34,10 +33,8 @@ import org.eclipse.jface.viewers.ISelection;
 import org.eclipse.jface.viewers.ISelectionProvider;
 import org.eclipse.jface.viewers.TreeSelection;
 import org.eclipse.linuxtools.internal.tmf.ui.Activator;
-import org.eclipse.linuxtools.tmf.core.TmfCommonConstants;
 import org.eclipse.linuxtools.tmf.core.project.model.TmfTraceType;
 import org.eclipse.linuxtools.tmf.core.project.model.TraceTypeHelper;
-import org.eclipse.linuxtools.tmf.core.trace.ITmfTrace;
 import org.eclipse.linuxtools.tmf.ui.project.model.TmfCommonProjectElement;
 import org.eclipse.linuxtools.tmf.ui.project.model.TmfExperimentElement;
 import org.eclipse.linuxtools.tmf.ui.project.model.TmfExperimentFolder;
@@ -178,18 +175,15 @@ public class SelectTraceTypeHandler extends AbstractHandler {
     private static IStatus propagateProperties(TmfCommonProjectElement element, String traceType)
             throws CoreException {
 
-        IResource resource = element.getResource();
-        String svTraceType = resource.getPersistentProperty(TmfCommonConstants.TRACETYPE);
-        TraceTypeHelper svTraceTypeHelper = TmfTraceType.getInstance().getTraceType(svTraceType);
-
         TraceTypeHelper traceTypeHelper = TmfTraceType.getInstance().getTraceType(traceType);
-        TmfTraceTypeUIUtils.setTraceType(resource, traceTypeHelper);
-        final IStatus validateTraceType = validateTraceType(element);
+        final IStatus validateTraceType = traceTypeHelper.validate(element.getLocation().getPath());
         if (!validateTraceType.isOK()) {
-            TmfTraceTypeUIUtils.setTraceType(resource, svTraceTypeHelper);
             return validateTraceType;
         }
 
+        IResource resource = element.getResource();
+        TmfTraceTypeUIUtils.setTraceType(resource, traceTypeHelper);
+
         TmfExperimentFolder experimentFolder = element.getProject().getExperimentsFolder();
         for (final TmfExperimentElement experiment : experimentFolder.getExperiments()) {
             for (final TmfTraceElement child : experiment.getTraces()) {
@@ -203,26 +197,4 @@ public class SelectTraceTypeHandler extends AbstractHandler {
         return Status.OK_STATUS;
     }
 
-    private static IStatus validateTraceType(TmfCommonProjectElement trace) {
-        IProject project = trace.getProject().getResource();
-        ITmfTrace tmfTrace = null;
-        IStatus validate = null;
-        try {
-            tmfTrace = trace.instantiateTrace();
-            if (tmfTrace != null) {
-                validate = tmfTrace.validate(project, trace.getLocation().getPath());
-            } else {
-                validate =  new Status(IStatus.ERROR, trace.getName(), "File does not exist : " + trace.getLocation().getPath()); //$NON-NLS-1$
-            }
-        } finally {
-            if (tmfTrace != null) {
-                tmfTrace.dispose();
-            }
-        }
-        if (validate == null) {
-            validate = new Status(IStatus.ERROR, "unknown", "unknown"); //$NON-NLS-1$ //$NON-NLS-2$
-        }
-        return validate;
-    }
-
 }
diff --git a/lttng/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/ImportTraceWizardPage.java b/lttng/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/ImportTraceWizardPage.java
index 34fac0b495..a0b41f076c 100644
--- a/lttng/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/ImportTraceWizardPage.java
+++ b/lttng/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/ImportTraceWizardPage.java
@@ -828,7 +828,7 @@ public class ImportTraceWizardPage extends WizardResourceImportPage {
                     throw new TmfTraceImportException(Messages.ImportTraceWizard_TraceTypeNotFound);
                 }
 
-                if (!traceTypeHelper.validate(path)) {
+                if (!traceTypeHelper.validate(path).isOK()) {
                     // Trace type exist but doesn't validate for given trace.
                     return;
                 }