tmf: Clean up tmf.ui.project.model
authorAlexandre Montplaisir <alexmonthy@efficios.com>
Tue, 23 Feb 2016 23:27:50 +0000 (18:27 -0500)
committerAlexandre Montplaisir <alexmonthy@efficios.com>
Wed, 2 Mar 2016 19:08:59 +0000 (14:08 -0500)
Cleanup the ITmfProjectModelElement hierarchy:

* Make all fields in the abstract class private, replace accesses with
  the getter methods.
  This also has the advantage of giving us the right type, for elements
  that override for example the getResource() return type.

* Make refreshChildren() abstract, the great majority of the
  implementations should handle it. It's easy to "forget" to override
  a method, while you can't forget an abstract method.

* Remove addChild() and removeChild() from the interface. Ideally
  these should be only called by the owners themselves.

* The "parent.addChild(this)" pattern is quite bad. It muddles the
  ownership, and calls "this" in a constructor, which is bad practice.
  Have the parents add the child to themselves after they create it.

* Implement hasChildren() as a default interface method.

* Have getChildren() return an ImmutableList copy of the original
  list. Callers shouldn't be modifying it!

Change-Id: Ie2e8129bef447ecbdce9add0e8cf9bd28ba36783
Signed-off-by: Alexandre Montplaisir <alexmonthy@efficios.com>
Reviewed-on: https://git.eclipse.org/r/67387
Reviewed-by: Patrick Tasse <patrick.tasse@gmail.com>
Tested-by: Patrick Tasse <patrick.tasse@gmail.com>
Reviewed-by: Hudson CI
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/project/model/ITmfProjectModelElement.java
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/project/model/TmfAnalysisElement.java
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/project/model/TmfAnalysisOutputElement.java
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/project/model/TmfCommonProjectElement.java
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/project/model/TmfExperimentElement.java
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/project/model/TmfExperimentFolder.java
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/project/model/TmfProjectElement.java
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/project/model/TmfProjectModelElement.java
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/project/model/TmfTraceElement.java
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/project/model/TmfTraceFolder.java

index 6025e812f191fc52e1ca40b018dcd915dde4d738..991bf0aa430f50aed8b6bcba1ffee6d45316eaf5 100644 (file)
@@ -32,56 +32,65 @@ public interface ITmfProjectModelElement {
 
     /**
      * Returns the name of the project model element.
+     *
      * @return the name of the project element.
      */
     String getName();
+
     /**
      * Returns the resource associated with the project model element.
+     *
      * @return the model resource.
      */
     IResource getResource();
+
     /**
      * Returns the path of the project model resource.
+     *
      * @return the resource path.
      */
     IPath getPath();
+
     /**
      * Returns the URI (location) of the resource.
+     *
      * @return the resource URI.
      */
     URI getLocation();
+
     /**
      * Returns the project model element.
+     *
      * @return the project model element.
      */
     TmfProjectElement getProject();
+
     /**
      * Returns the parent of this model element.
+     *
      * @return the parent of this model element.
      */
     ITmfProjectModelElement getParent();
-    /**
-     * Returns whether this model element has children or not.
-     * @return <code>true</code> if this model has children else <code>false</code>
-     */
-    boolean hasChildren();
+
     /**
      * Returns a list of children model elements.
+     *
      * @return a list of children model elements.
      */
     List<ITmfProjectModelElement> getChildren();
-    /**
-     * Method to add a child to the model element.
-     * @param child A child element to add.
-     */
-    void addChild(ITmfProjectModelElement child);
-    /**
-     * Method to remove a child from the model element.
-     * @param child A child element to remove
-     */
-    void removeChild(ITmfProjectModelElement child);
+
     /**
      * Method to request to refresh the project.
      */
     void refresh();
+
+    /**
+     * Returns whether this model element has children or not.
+     *
+     * @return <code>true</code> if this model element has children else
+     *         <code>false</code>
+     */
+    default boolean hasChildren() {
+        return !getChildren().isEmpty();
+    }
 }
index 02820b687382ae1a1190676efe430490d5e21533..23af28ac52df780bd0e3ed99dd276530619d64e7 100644 (file)
@@ -73,15 +73,17 @@ public class TmfAnalysisElement extends TmfProjectModelElement implements ITmfSt
     protected TmfAnalysisElement(String name, IResource resource, ITmfProjectModelElement parent, @NonNull IAnalysisModuleHelper module) {
         super(name, resource, parent);
         fAnalysisHelper = module;
-        parent.addChild(this);
     }
 
     // ------------------------------------------------------------------------
     // TmfProjectModelElement
     // ------------------------------------------------------------------------
 
+    /**
+     * @since 2.0
+     */
     @Override
-    void refreshChildren() {
+    protected void refreshChildren() {
         fCanExecute = true;
 
         /* Refresh the outputs of this analysis */
@@ -92,8 +94,9 @@ public class TmfAnalysisElement extends TmfProjectModelElement implements ITmfSt
 
         /** Get base path for resource */
         IPath path = getProject().getTracesFolder().getPath();
-        if (fResource instanceof IFolder) {
-            path = ((IFolder) fResource).getFullPath();
+        IResource resource = getResource();
+        if (resource instanceof IFolder) {
+            path = ((IFolder) resource).getFullPath();
         }
 
         /*
@@ -124,6 +127,7 @@ public class TmfAnalysisElement extends TmfProjectModelElement implements ITmfSt
                 if (outputElement == null) {
                     IFolder newresource = ResourcesPlugin.getWorkspace().getRoot().getFolder(path.append(output.getName()));
                     outputElement = new TmfAnalysisOutputElement(output.getName(), newresource, this, output);
+                    addChild(outputElement);
                 }
                 outputElement.refreshChildren();
             }
index be720ee9b5db71e23176871d025044198b9fa4c2..58c74b1b9b2dd48e5c5a693d2aa53dbef3c2d89a 100644 (file)
@@ -45,7 +45,6 @@ public class TmfAnalysisOutputElement extends TmfProjectModelElement {
     protected TmfAnalysisOutputElement(String name, IResource resource, ITmfProjectModelElement parent, IAnalysisOutput output) {
         super(name, resource, parent);
         fOutput = output;
-        parent.addChild(this);
     }
 
     /**
@@ -82,4 +81,9 @@ public class TmfAnalysisOutputElement extends TmfProjectModelElement {
         }
     }
 
+    @Override
+    protected void refreshChildren() {
+        /* Nothing to do */
+    }
+
 }
index f6982820ba4faae96a6d82f651215b5b2079f9e6..b9870e0096b1701add705d505d4a45c371aa87e3 100644 (file)
@@ -87,7 +87,6 @@ public abstract class TmfCommonProjectElement extends TmfProjectModelElement {
      */
     public TmfCommonProjectElement(String name, IResource resource, TmfProjectModelElement parent) {
         super(name, resource, parent);
-        parent.addChild(this);
         refreshTraceType();
         TmfSignalManager.register(this);
     }
@@ -96,8 +95,11 @@ public abstract class TmfCommonProjectElement extends TmfProjectModelElement {
     // TmfProjectModelElement
     // ------------------------------------------------------------------------
 
+    /**
+     * @since 2.0
+     */
     @Override
-    void refreshChildren() {
+    protected void refreshChildren() {
 
         /* Refreshes the analysis under this trace */
         Map<String, TmfAnalysisElement> childrenMap = new HashMap<>();
@@ -122,7 +124,7 @@ public abstract class TmfCommonProjectElement extends TmfProjectModelElement {
         }
 
         /** Get the base path to put the resource to */
-        IPath path = fResource.getFullPath();
+        IPath path = getResource().getFullPath();
 
         /* Add all new analysis modules or refresh outputs of existing ones */
         for (IAnalysisModuleHelper module : TmfAnalysisManager.getAnalysisModules(traceClass).values()) {
@@ -136,6 +138,7 @@ public abstract class TmfCommonProjectElement extends TmfProjectModelElement {
                  */
                 IFolder newresource = ResourcesPlugin.getWorkspace().getRoot().getFolder(path.append(module.getId()));
                 analysis = new TmfAnalysisElement(module.getName(), newresource, this, module);
+                addChild(analysis);
             }
             analysis.refreshChildren();
         }
@@ -200,7 +203,7 @@ public abstract class TmfCommonProjectElement extends TmfProjectModelElement {
         while (!(parent instanceof TmfTracesFolder || parent instanceof TmfExperimentElement || parent instanceof TmfExperimentFolder)) {
             parent = parent.getParent();
         }
-        IPath path = fResource.getFullPath().makeRelativeTo(parent.getPath());
+        IPath path = getResource().getFullPath().makeRelativeTo(parent.getPath());
         return path.toString();
     }
 
@@ -286,7 +289,7 @@ public abstract class TmfCommonProjectElement extends TmfProjectModelElement {
      * @return the bookmarks file
      */
     public IFile getBookmarksFile() {
-        final IFolder folder = (IFolder) fResource;
+        final IFolder folder = (IFolder) getResource();
         IFile file = folder.getFile(getName() + '_');
         return file;
     }
@@ -540,7 +543,7 @@ public abstract class TmfCommonProjectElement extends TmfProjectModelElement {
         IFolder supplFolder = prepareTraceSupplementaryFolder(getSupplementaryFolderPath(), true);
 
         try {
-            fResource.setPersistentProperty(TmfCommonConstants.TRACE_SUPPLEMENTARY_FOLDER, supplFolder.getLocation().toOSString());
+            getResource().setPersistentProperty(TmfCommonConstants.TRACE_SUPPLEMENTARY_FOLDER, supplFolder.getLocation().toOSString());
         } catch (CoreException e) {
             Activator.getDefault().logError("Error setting persistant property " + TmfCommonConstants.TRACE_SUPPLEMENTARY_FOLDER, e); //$NON-NLS-1$
         }
index cf6832cdcc41b36866de0085676569ab6245098d..9c85d33600d2467038c043a6c2c01b8e36a09f36 100644 (file)
@@ -156,11 +156,14 @@ public class TmfExperimentElement extends TmfCommonProjectElement implements IPr
 
     @Override
     public IFolder getResource() {
-        return (IFolder) fResource;
+        return (IFolder) super.getResource();
     }
 
+    /**
+     * @since 2.0
+     */
     @Override
-    void refreshChildren() {
+    protected void refreshChildren() {
         IFolder folder = getResource();
 
         /* Update the trace children of this experiment */
@@ -179,6 +182,7 @@ public class TmfExperimentElement extends TmfCommonProjectElement implements IPr
                 childrenMap.remove(elementPath);
             } else {
                 element = new TmfTraceElement(name, resource, this);
+                addChild(element);
             }
         }
 
@@ -204,8 +208,9 @@ public class TmfExperimentElement extends TmfCommonProjectElement implements IPr
         }
         for (IAnalysisModuleHelper module : TmfAnalysisManager.getAnalysisModules().values()) {
             if (!analysisMap.containsKey(module.getId()) && module.appliesToExperiment() && (experiment.getAnalysisModule(module.getId()) != null)) {
-                IFolder newresource = ResourcesPlugin.getWorkspace().getRoot().getFolder(fResource.getFullPath().append(module.getId()));
+                IFolder newresource = ResourcesPlugin.getWorkspace().getRoot().getFolder(getResource().getFullPath().append(module.getId()));
                 TmfAnalysisElement analysis = new TmfAnalysisElement(module.getName(), newresource, this, module);
+                addChild(analysis);
                 analysis.refreshChildren();
                 analysisMap.put(module.getId(), analysis);
             }
index 6f28bf0de08db91520991fb3fd302da549c1af8e..4db58d274296871b9a95541a93d552acd262e588 100644 (file)
@@ -79,7 +79,6 @@ public class TmfExperimentFolder extends TmfProjectModelElement implements IProp
      */
     public TmfExperimentFolder(String name, IFolder folder, TmfProjectElement parent) {
         super(name, folder, parent);
-        parent.addChild(this);
     }
 
     // ------------------------------------------------------------------------
@@ -88,11 +87,14 @@ public class TmfExperimentFolder extends TmfProjectModelElement implements IProp
 
     @Override
     public IFolder getResource() {
-        return (IFolder) fResource;
+        return (IFolder) super.getResource();
     }
 
+    /**
+     * @since 2.0
+     */
     @Override
-    void refreshChildren() {
+    protected void refreshChildren() {
         IFolder folder = getResource();
 
         // Get the children from the model
@@ -112,6 +114,7 @@ public class TmfExperimentFolder extends TmfProjectModelElement implements IProp
                         childrenMap.remove(name);
                     } else {
                         element = new TmfExperimentElement(name, expFolder, this);
+                        addChild(element);
                     }
                     ((TmfExperimentElement) element).refreshChildren();
                 }
index 45cfcb4aea86d9f3d47332400a5e932e0a4928c1..96b360b96444248ffcbd0adc60044e12f37f4419 100644 (file)
@@ -37,13 +37,18 @@ public class TmfProjectElement extends TmfProjectModelElement {
     // ------------------------------------------------------------------------
     // Constructor
     // ------------------------------------------------------------------------
+
     /**
      * Constructor.
      *
      * Creates the TMF project model element.
-     * @param name The name of the project.
-     * @param project The project reference.
-     * @param parent The parent element
+     *
+     * @param name
+     *            The name of the project.
+     * @param project
+     *            The project reference.
+     * @param parent
+     *            The parent element
      */
     public TmfProjectElement(String name, IProject project, ITmfProjectModelElement parent) {
         super(name, project, parent);
@@ -55,7 +60,7 @@ public class TmfProjectElement extends TmfProjectModelElement {
 
     @Override
     public IProject getResource() {
-        return (IProject) fResource;
+        return (IProject) super.getResource();
     }
 
     @Override
@@ -95,8 +100,11 @@ public class TmfProjectElement extends TmfProjectModelElement {
     // TmfProjectModelElement
     // ------------------------------------------------------------------------
 
+    /**
+     * @since 2.0
+     */
     @Override
-    void refreshChildren() {
+    protected void refreshChildren() {
         IProject project = getResource();
 
         // Get the children from the model
@@ -115,6 +123,7 @@ public class TmfProjectElement extends TmfProjectModelElement {
                 childrenMap.remove(name);
             } else {
                 element = new TmfTracesFolder(TmfTracesFolder.TRACES_FOLDER_NAME, folder, this);
+                addChild(element);
             }
             ((TmfTracesFolder) element).refreshChildren();
         }
@@ -129,6 +138,7 @@ public class TmfProjectElement extends TmfProjectModelElement {
                 childrenMap.remove(name);
             } else {
                 element = new TmfExperimentFolder(TmfExperimentFolder.EXPER_FOLDER_NAME, folder, this);
+                addChild(element);
             }
             ((TmfExperimentFolder) element).refreshChildren();
         }
index 3a37b8a289a9429bef6bd950bf765f3f3c15a966..62a42ccae30285adce49c0398c3be0b9aa4283fc 100644 (file)
@@ -25,6 +25,7 @@ import org.eclipse.core.resources.IResource;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.NullProgressMonitor;
+import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.swt.widgets.Display;
 import org.eclipse.tracecompass.internal.tmf.ui.Activator;
 import org.eclipse.tracecompass.tmf.core.TmfCommonConstants;
@@ -37,6 +38,8 @@ import org.eclipse.ui.PlatformUI;
 import org.eclipse.ui.navigator.CommonNavigator;
 import org.eclipse.ui.navigator.CommonViewer;
 
+import com.google.common.collect.ImmutableList;
+
 /**
  * The implementation of the base TMF project model element. It provides default implementation
  * of the <code>ITmfProjectModelElement</code> interface.
@@ -51,34 +54,36 @@ public abstract class TmfProjectModelElement implements ITmfProjectModelElement
     // ------------------------------------------------------------------------
 
     private final String fName;
-    /**
-     * The project model element resource.
-     */
-    protected final IResource fResource;
-    /**
-     * The project model resource location (URI)
-     */
-    protected final URI fLocation;
-    /**
-     * The project model path of a resource.
-     */
-    protected final IPath fPath;
+
+    /** The project model element resource */
+    private final IResource fResource;
+
+    /** The project model resource location (URI) */
+    private final URI fLocation;
+
+    /** The project model path of a resource */
+    private final IPath fPath;
+
     private final ITmfProjectModelElement fParent;
-    /**
-     * The list of children elements.
-     */
-    protected final List<ITmfProjectModelElement> fChildren;
+
+    /** The list of children elements */
+    private final @NonNull List<ITmfProjectModelElement> fChildren;
 
     // ------------------------------------------------------------------------
     // Constructor
     // ------------------------------------------------------------------------
+
     /**
      * Constructor.
      *
      * Creates a base project model element.
-     * @param name The name of the element.
-     * @param resource The element resource.
-     * @param parent The parent model element.
+     *
+     * @param name
+     *            The name of the element.
+     * @param resource
+     *            The element resource.
+     * @param parent
+     *            The parent model element.
      */
     protected TmfProjectModelElement(String name, IResource resource, ITmfProjectModelElement parent) {
         fName = name;
@@ -123,24 +128,9 @@ public abstract class TmfProjectModelElement implements ITmfProjectModelElement
         return fParent;
     }
 
-    @Override
-    public boolean hasChildren() {
-        return fChildren.size() > 0;
-    }
-
     @Override
     public List<ITmfProjectModelElement> getChildren() {
-        return fChildren;
-    }
-
-    @Override
-    public void addChild(ITmfProjectModelElement child) {
-        fChildren.add(child);
-    }
-
-    @Override
-    public void removeChild(ITmfProjectModelElement child) {
-        fChildren.remove(child);
+        return ImmutableList.copyOf(fChildren);
     }
 
     @Override
@@ -211,9 +201,29 @@ public abstract class TmfProjectModelElement implements ITmfProjectModelElement
      * Refresh the children of this model element, adding new children and
      * removing dangling children as necessary. The remaining children should
      * also refresh their own children sub-tree.
+     *
+     * @since 2.0
      */
-    void refreshChildren() {
-        // Sub-classes may override this method as needed
+    protected abstract void refreshChildren();
+
+    /**
+     * Add a new child element to this element.
+     *
+     * @param child
+     *            The child to add
+     */
+    protected void addChild(ITmfProjectModelElement child) {
+        fChildren.add(child);
+    }
+
+    /**
+     * Remove an element from the current child elements.
+     *
+     * @param child
+     *            The child to remove
+     */
+    protected void removeChild(ITmfProjectModelElement child) {
+        fChildren.remove(child);
     }
 
     /**
index c8327a0c8fdaa319777a41229e1ca84fd35b51e5..91271eb04d6a949986f9fa11183722dbe7549992 100644 (file)
@@ -345,7 +345,7 @@ public class TmfTraceElement extends TmfCommonProjectElement implements IActionF
     @Override
     public IFile createBookmarksFile() throws CoreException {
         IFile file = getBookmarksFile();
-        if (fResource instanceof IFolder) {
+        if (getResource() instanceof IFolder) {
             return createBookmarksFile(getProject().getTracesFolder().getResource(), ITmfEventsEditorConstants.TRACE_EDITOR_INPUT_TYPE);
         }
         return file;
@@ -360,10 +360,11 @@ public class TmfTraceElement extends TmfCommonProjectElement implements IActionF
     @Override
     public IFile getBookmarksFile() {
         IFile file = null;
-        if (fResource instanceof IFile) {
-            file = (IFile) fResource;
-        } else if (fResource instanceof IFolder) {
-            final IFolder folder = (IFolder) fResource;
+        IResource resource = getResource();
+        if (resource instanceof IFile) {
+            file = (IFile) resource;
+        } else if (resource instanceof IFolder) {
+            final IFolder folder = (IFolder) resource;
             file = folder.getFile(getName() + '_');
         }
         return file;
@@ -665,7 +666,7 @@ public class TmfTraceElement extends TmfCommonProjectElement implements IActionF
             }
         });
 
-        IPath path = fResource.getLocation();
+        IPath path = getResource().getLocation();
         if (path != null) {
             if (getParent() instanceof TmfTraceFolder) {
                 TmfExperimentFolder experimentFolder = getProject().getExperimentsFolder();
@@ -693,7 +694,7 @@ public class TmfTraceElement extends TmfCommonProjectElement implements IActionF
         }
 
         // Finally, delete the trace
-        fResource.delete(true, progressMonitor);
+        getResource().delete(true, progressMonitor);
     }
 
 }
index e326c0ad363b4ab7c1d470b5ffeae8329bb0ad66..debdec07d7d74f6f55b376edb91c99500042f60e 100644 (file)
@@ -50,8 +50,7 @@ public class TmfTraceFolder extends TmfProjectModelElement implements IPropertyS
     private static final ReadOnlyTextPropertyDescriptor PATH_DESCRIPTOR = new ReadOnlyTextPropertyDescriptor(PATH, PATH);
     private static final ReadOnlyTextPropertyDescriptor LOCATION_DESCRIPTOR = new ReadOnlyTextPropertyDescriptor(LOCATION, LOCATION);
 
-    private static final IPropertyDescriptor[] DESCRIPTORS = { NAME_DESCRIPTOR, PATH_DESCRIPTOR,
-            LOCATION_DESCRIPTOR };
+    private static final IPropertyDescriptor[] DESCRIPTORS = { NAME_DESCRIPTOR, PATH_DESCRIPTOR, LOCATION_DESCRIPTOR };
 
     static {
         NAME_DESCRIPTOR.setCategory(INFO_CATEGORY);
@@ -64,27 +63,31 @@ public class TmfTraceFolder extends TmfProjectModelElement implements IPropertyS
     // ------------------------------------------------------------------------
 
     /**
-     * Constructor.
-     * Creates folder model element under the project.
-     * @param name The name of trace folder.
-     * @param resource The folder resource.
-     * @param parent The parent element (project).
+     * Constructor. Creates folder model element under the project.
+     *
+     * @param name
+     *            The name of trace folder.
+     * @param resource
+     *            The folder resource.
+     * @param parent
+     *            The parent element (project).
      */
     public TmfTraceFolder(String name, IFolder resource, TmfProjectElement parent) {
         super(name, resource, parent);
-        parent.addChild(this);
     }
 
     /**
-     * Constructor.
-     * Creates folder model element under another folder.
-     * @param name The name of trace folder.
-     * @param resource The folder resource.
-     * @param parent The parent element (folder).
+     * Constructor. Creates folder model element under another folder.
+     *
+     * @param name
+     *            The name of trace folder.
+     * @param resource
+     *            The folder resource.
+     * @param parent
+     *            The parent element (folder).
      */
     public TmfTraceFolder(String name, IFolder resource, TmfTraceFolder parent) {
         super(name, resource, parent);
-        parent.addChild(this);
     }
 
     // ------------------------------------------------------------------------
@@ -93,11 +96,14 @@ public class TmfTraceFolder extends TmfProjectModelElement implements IPropertyS
 
     @Override
     public IFolder getResource() {
-        return (IFolder) fResource;
+        return (IFolder) super.getResource();
     }
 
+    /**
+     * @since 2.0
+     */
     @Override
-    void refreshChildren() {
+    protected void refreshChildren() {
         IFolder folder = getResource();
 
         // Get the children from the model
@@ -119,8 +125,10 @@ public class TmfTraceFolder extends TmfProjectModelElement implements IPropertyS
                     } else {
                         element = new TmfTraceFolder(name, (IFolder) resource, this);
                     }
+                    addChild(element);
                 } else if (!isFolder && !(element instanceof TmfTraceElement)) {
                     element = new TmfTraceElement(name, resource, this);
+                    addChild(element);
                 } else {
                     childrenMap.remove(name);
                 }
This page took 0.038511 seconds and 5 git commands to generate.