From 3b066cac479d0e23b28bd8de626656d36eeb035b Mon Sep 17 00:00:00 2001
From: Simon Barner <barner@fortiss.org>
Date: Wed, 18 Oct 2023 08:51:59 +0200
Subject: [PATCH] Review (one RED, rest GREEN with some TODOs linking to
issues)
Issue-Ref: 4310
Issue-Url: https://git.fortiss.org/af3/af3/-/issues/4310
Signed-off-by: Simon Barner <barner@fortiss.org>
---
.../tooling/ext/quality/ui/view/fx/.ratings | 2 +-
.../ui/view/fx/ModelQualityFXController.java | 19 ++---
.../html/developer/.ratings | 2 +-
.../org/fortiss/tooling/ext/quality/.ratings | 6 +-
.../ext/quality/GraphMetricsProvider.java | 30 ++++---
.../HierarchicElementMetricProvider.java | 34 ++++----
.../fortiss/tooling/ext/quality/data/.ratings | 8 +-
.../ext/quality/data/DataRootElement.java | 10 +--
.../tooling/ext/quality/data/MetricData.java | 6 +-
.../tooling/ext/quality/data/MetricKey.java | 33 ++++----
.../ext/quality/data/MetricTreeNode.java | 6 +-
.../tooling/ext/quality/service/.ratings | 4 +-
.../quality/service/IModelQualityService.java | 16 ++--
.../quality/service/ModelQualityService.java | 84 +++++++++----------
.../tooling/ext/quality/storage/.ratings | 4 +-
.../ext/quality/storage/CSVFileWriter.java | 70 ++++++++++------
16 files changed, 172 insertions(+), 162 deletions(-)
diff --git a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/.ratings b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/.ratings
index ed265042f..060cb0ae4 100644
--- a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/.ratings
+++ b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/.ratings
@@ -1,3 +1,3 @@
IModelQualityViewPart.java 708f8089645df12098ea67190805cce343045d2e GREEN
-ModelQualityFXController.java 64c5fb2ca8cca77982b5a20695efcf2b1442045e RED
+ModelQualityFXController.java 91a82ea532fda6ef6b6db0430cece98016add60f RED
ModelQualityFXViewPart.java 9cfc7f60a86ea5b915c726b16712f8be7dec2c5f GREEN
diff --git a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXController.java b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXController.java
index 64c5fb2ca..91a82ea53 100644
--- a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXController.java
+++ b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXController.java
@@ -125,7 +125,7 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
/**
* The last {@link EObject} which the user has clicked. Might be null when no or an invalid
- * Object is selected.
+ * object is selected.
*/
private EObject lastSelectedElement;
@@ -167,8 +167,8 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
return;
}
- IModelQualityService mqs = IModelQualityService.getInstance();
- var rootDataElement = mqs.getMetricData().getDataRootElementMap().get(currentRootElement);
+ MetricData metricData = IModelQualityService.getInstance().getMetricData();
+ var rootDataElement = metricData.getDataRootElementMap().get(currentRootElement);
if(rootDataElement == null) {
bottomText.setText(
@@ -328,7 +328,8 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
private SpiderChartViewer buildSpiderChart() {
// Calculate the intersection of the keys, i.e. a set of keys which have a non-null value in
// all elements of the lastViewedElements list
- // TODO (SB): Add check that lastViewedElements is non empty, and add a corresonding comment
+ // TODO (SB): Add check that lastViewedElements is non empty, and add a corresponding
+ // comment
// to the .get(0) statement
Set<MetricKey> keysIntersection = lastViewedElements.get(0).getValue();
for(var p : lastViewedElements) {
@@ -395,9 +396,7 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
// Configure legend
LegendStyle legendStyle = new LegendStyle(false, 5, BLACK_VERDANA_12PT);
chartStyle.setLegendStyle(legendStyle);
-
chartStyle.setAxisSegments(4);
-
chartStyle.setBackgroundFillStyle(new FillStyle(LIGHTGRAY));
chartStyle.setBackgroundLineStyle(new LineStyle(DARKGRAY));
@@ -414,8 +413,8 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
return;
}
- IModelQualityService mqs = IModelQualityService.getInstance();
- var rootDataElement = mqs.getMetricData().getDataRootElementMap().get(updatedElement);
+ MetricData metricData = IModelQualityService.getInstance().getMetricData();
+ var rootDataElement = metricData.getDataRootElementMap().get(updatedElement);
if(rootDataElement != null) {
bottomText.setText("Metrics last updated: " +
timeFormat.format(rootDataElement.getLastRefresh()) +
@@ -440,8 +439,8 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
// Create set of keys which store values in the ndoe
Set<MetricKey> keys = new HashSet<>();
- keys.addAll(node.getStoredDoubles().keySet());
- keys.addAll(node.getStoredIntegers().keySet());
+ keys.addAll(node.getDoubleMetrics().keySet());
+ keys.addAll(node.getIntegerMetrics().keySet());
// Check if the node is already part of the list, and skip adding it if it is
if(lastViewedElements.stream().allMatch(p -> p.getKey() != node)) {
diff --git a/org.fortiss.tooling.ext.quality/html/developer/.ratings b/org.fortiss.tooling.ext.quality/html/developer/.ratings
index b6a7b0b9a..424f339e6 100644
--- a/org.fortiss.tooling.ext.quality/html/developer/.ratings
+++ b/org.fortiss.tooling.ext.quality/html/developer/.ratings
@@ -1 +1 @@
-documentation.html b66c2bfb495047a8e4b680b02231ab2fdb34cec3 YELLOW
+documentation.html b66c2bfb495047a8e4b680b02231ab2fdb34cec3 GREEN
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/.ratings b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/.ratings
index f93b691d3..44fde8c40 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/.ratings
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/.ratings
@@ -1,5 +1,5 @@
-GraphMetricsProvider.java d18cff37dcb103a447cead64e1c2b00866996b3c YELLOW
-HierarchicElementMetricProvider.java e76e846d76c6ba8782a20d9c9f9c6d57eaaa416e YELLOW
+GraphMetricsProvider.java ce70c838b887df0bdf776f462b23cf594c700822 GREEN
+HierarchicElementMetricProvider.java 966b3aeef92475034fc35e68ba5344688988b88a GREEN
IMetricProvider.java 99fc8993b0e65b2f8757978eeb0481d912f5608c GREEN
IMetricUpdateListener.java c24dc7c0f282623bbf1eefac1fbbb6752c97ddf0 GREEN
-ModelQualityActivator.java 6d5f0794aa48670cf45fb405bd7641bce6c1fec4 YELLOW
+ModelQualityActivator.java 6d5f0794aa48670cf45fb405bd7641bce6c1fec4 GREEN
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/GraphMetricsProvider.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/GraphMetricsProvider.java
index d18cff37d..ce70c838b 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/GraphMetricsProvider.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/GraphMetricsProvider.java
@@ -15,6 +15,8 @@
+--------------------------------------------------------------------------*/
package org.fortiss.tooling.ext.quality;
+import static org.fortiss.tooling.kernel.utils.EcoreUtils.pickInstanceOf;
+
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
@@ -25,8 +27,6 @@ import java.util.Queue;
import java.util.Set;
import java.util.Stack;
-import static org.fortiss.tooling.kernel.utils.EcoreUtils.pickInstanceOf;
-
import org.eclipse.emf.common.util.EList;
import org.fortiss.tooling.base.model.base.EntryConnectorBase;
import org.fortiss.tooling.base.model.base.ExitConnectorBase;
@@ -35,6 +35,7 @@ import org.fortiss.tooling.base.model.element.IConnector;
import org.fortiss.tooling.base.model.element.IHierarchicElement;
import org.fortiss.tooling.ext.quality.data.MetricData;
import org.fortiss.tooling.ext.quality.data.MetricKey;
+import org.fortiss.tooling.ext.quality.data.MetricTreeNode;
/**
* Supplies methods to compute certain graph metrics.
@@ -42,7 +43,7 @@ import org.fortiss.tooling.ext.quality.data.MetricKey;
* @author groh
*/
-// TODO (#4332) Restructure GraphMetricsProvider in quality Plugin
+// TODO (#4332) Restructure GraphMetricsProvider
public class GraphMetricsProvider {
/**
@@ -171,9 +172,11 @@ public class GraphMetricsProvider {
/**
* Calculates the clustering coefficient based on the neighborhood.
- * https://en.wikipedia.org/wiki/Clustering_coefficient
- * Clustering coefficient C for a node V is counting the triangles divided by the "number of neighbors of
- * V" times "number of neighbors of V - 1" which results in the potential number of maximum links between the nodes
+ * See, e.g., https://en.wikipedia.org/wiki/Clustering_coefficient
+ * Clustering coefficient C for a node V is counting the triangles divided by the "number of
+ * neighbors of "V" times "number of neighbors of V - 1" which results in the potential number
+ * of maximum links between the nodes
+ *
* @param element
* of which the coefficient shall be calculated
* @return value of the coefficient
@@ -202,7 +205,7 @@ public class GraphMetricsProvider {
/**
* Calculates and saves the betweenness centrality of all elements contained inside the provided
- * scopeElement. Described for example in https://en.wikipedia.org/wiki/Betweenness_centrality
+ * scopeElement. See, e.g., https://en.wikipedia.org/wiki/Betweenness_centrality
*
* @param scopeElement
* the scope of this calculation
@@ -218,9 +221,9 @@ public class GraphMetricsProvider {
// Abort if no elements are found
if(scopeElement.getContainedElements().isEmpty()) {
- return;
+ return;
}
-
+
Set<IHierarchicElement> graphNodes = getLocalGraphView(scopeElement, recursively);
// Initialize the betweenness values for all nodes to zero
Map<IHierarchicElement, Double> betweenness = new HashMap<>();
@@ -277,8 +280,8 @@ public class GraphMetricsProvider {
MetricKey key = recursively ? MetricKey.BETWEENESS_CENTRALITY_RECURSIVELY
: MetricKey.BETWEENESS_CENTRALITY;
for(IHierarchicElement child : scopeElement.getContainedElements()) {
- metricData.getTreeNodeLookupTable().get(child).getStoredDoubles().put(key,
- betweenness.get(child));
+ MetricTreeNode node = metricData.getTreeNodeLookupTable().get(child);
+ node.getDoubleMetrics().put(key, betweenness.get(child));
}
}
@@ -316,7 +319,8 @@ public class GraphMetricsProvider {
stack.push(v);
// Iterate over all connectors going out from the element
- for(ExitConnectorBase exitConnectors : pickInstanceOf(ExitConnectorBase.class, v.getConnectors())) {
+ for(ExitConnectorBase exitConnectors : pickInstanceOf(ExitConnectorBase.class,
+ v.getConnectors())) {
// Iterate over all connections
for(IConnection outgoingConnection : exitConnectors.getOutgoing()) {
@@ -353,7 +357,7 @@ public class GraphMetricsProvider {
if(targetDistance < 0) {
queue.add(targetElement);
distance.put(targetElement, vDistance + 1);
- }else if(targetDistance == vDistance + 1) {
+ } else if(targetDistance == vDistance + 1) {
dependency.put(targetElement,
dependency.get(targetElement) + dependency.get(v));
predecessor.get(targetElement).add(v);
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/HierarchicElementMetricProvider.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/HierarchicElementMetricProvider.java
index e76e846d7..966b3aeef 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/HierarchicElementMetricProvider.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/HierarchicElementMetricProvider.java
@@ -19,7 +19,6 @@ import static org.fortiss.tooling.kernel.utils.EcoreUtils.pickInstanceOf;
import org.fortiss.tooling.base.model.base.EntryConnectorBase;
import org.fortiss.tooling.base.model.base.ExitConnectorBase;
-
import org.fortiss.tooling.base.model.element.IHierarchicElement;
import org.fortiss.tooling.ext.quality.data.MetricKey;
import org.fortiss.tooling.ext.quality.data.MetricTreeNode;
@@ -36,33 +35,32 @@ public class HierarchicElementMetricProvider implements IMetricProvider<IHierarc
@Override
public void collectMetrics(MetricTreeNode node, IHierarchicElement currentElement) {
- var intMetrics = node.getStoredIntegers();
+ var intMetrics = node.getIntegerMetrics();
// Only collect these metrics if the name is null, in which case they have not been
// collected before
// This is used in some cases to combine metrics from two different IHierarchicElement
if(node.getName() == null) {
// Metrics are always set to default value, as these metric are combined later
- String comment;
if(currentElement instanceof INamedCommentedElement) {
intMetrics.put(MetricKey.NUMBER_OF_TOTAL_COMMENTABLE_ELEMENTS, 1);
- comment =((INamedCommentedElement)currentElement).getComment();
- if(comment != "") {
+ INamedCommentedElement nce = (INamedCommentedElement)currentElement;
+ String comment = nce.getComment();
+ if(comment != null && comment != "") {
intMetrics.put(MetricKey.NUMBER_OF_TOTAL_COMMENTED_ELEMENTS, 1);
}
- node.setName(((INamedCommentedElement)currentElement).getName());
- }else {
+ node.setName(nce.getName());
+ } else {
intMetrics.put(MetricKey.NUMBER_OF_TOTAL_COMMENTABLE_ELEMENTS, 0);
- comment = null;
intMetrics.put(MetricKey.NUMBER_OF_TOTAL_COMMENTED_ELEMENTS, 0);
node.setName("No Name existant");
}
-
+
if(currentElement instanceof IIdLabeled) {
- intMetrics.put(MetricKey.UNIQUE_ID,((IIdLabeled)currentElement).getId());
- }else {
+ intMetrics.put(MetricKey.UNIQUE_ID, ((IIdLabeled)currentElement).getId());
+ } else {
intMetrics.put(MetricKey.UNIQUE_ID, -1);
- }
+ }
}
// connector metrics
@@ -71,10 +69,10 @@ public class HierarchicElementMetricProvider implements IMetricProvider<IHierarc
intMetrics.put(MetricKey.NUMBER_OF_CONTAINED_ELEMENTS,
currentElement.getContainedElements().size());
intMetrics.put(MetricKey.NUMBER_OF_CONNECTIONS, currentElement.getConnections().size());
-
+
// depth metrics
intMetrics.put(MetricKey.NUMBER_OF_TOTAL_CONNECTORS, connectors.size());
-
+
var entry_connectors = pickInstanceOf(EntryConnectorBase.class, connectors);
var exit_connectors = pickInstanceOf(ExitConnectorBase.class, connectors);
intMetrics.put(MetricKey.NUMBER_OF_TOTAL_ENTRY_CONNECTORS, entry_connectors.size());
@@ -82,12 +80,12 @@ public class HierarchicElementMetricProvider implements IMetricProvider<IHierarc
intMetrics.put(MetricKey.NUMBER_OF_TOTAL_ELEMENTS, 1);
- if (currentElement.getContainedElements().isEmpty()) {
+ if(currentElement.getContainedElements().isEmpty()) {
intMetrics.put(MetricKey.NUMBER_OF_TOTAL_LEAF_ELEMENTS, 0);
- }else {
+ } else {
intMetrics.put(MetricKey.NUMBER_OF_TOTAL_LEAF_ELEMENTS, 1);
}
- //graph metrics
- }
+ // graph metrics
+ }
}
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/.ratings b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/.ratings
index 03a1517ed..a66b72971 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/.ratings
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/.ratings
@@ -1,4 +1,4 @@
-DataRootElement.java e598763b5199c4810da191623a07a9edeb154a84 YELLOW
-MetricData.java 03fd1cf0477a42f1f50c899c180035498d21faa3 YELLOW
-MetricKey.java 81ba35f8537c4cb9c0f487a2bfafdb65f9d6c3c0 YELLOW
-MetricTreeNode.java 8eb3c6761a3af482215d48e1bada53415d4ba8e4 YELLOW
+DataRootElement.java 7b655236c155e3a4473301aa8c3ecfe769cf071c GREEN
+MetricData.java b3c1a395cca29ced5845b789d76ee2c965ec66f6 GREEN
+MetricKey.java d78026f0c68aeeff0cc758bbd76fbb84da65afd6 GREEN
+MetricTreeNode.java 88ddbdfc62fb8efee71aa3ee44257d1a1a582fe6 GREEN
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/DataRootElement.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/DataRootElement.java
index e598763b5..7b655236c 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/DataRootElement.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/DataRootElement.java
@@ -30,13 +30,13 @@ public class DataRootElement {
/** The last time a metric tree was refreshed. */
private LocalDateTime lastRefresh;
- /** the last time the metric tree was refreshed */
+ /** The last time the metric tree was refreshed. */
private ITopLevelElement topLevelElement;
- /** Root {@link MetricTreeNode} for the metric tree containg all data */
+ /** Root {@link MetricTreeNode} for the metric tree containing all data. */
private MetricTreeNode rootNode;
- /**Constructor. */
+ /** Constructor. */
public DataRootElement(LocalDateTime lastRefresh, ITopLevelElement topLevelElement,
MetricTreeNode rootNode) {
super();
@@ -65,12 +65,12 @@ public class DataRootElement {
this.topLevelElement = topLevelElement;
}
- /** Getter for {@link #MetricTreeNode}. */
+ /** Getter for {@link #rootNode}. */
public MetricTreeNode getRootNode() {
return rootNode;
}
- /** Setter for {@link #MetricTreeNode}. */
+ /** Setter for {@link #rootNode}. */
public void setRootNode(MetricTreeNode metricTreeNode) {
this.rootNode = metricTreeNode;
}
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricData.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricData.java
index 03fd1cf04..b3c1a395c 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricData.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricData.java
@@ -22,7 +22,7 @@ import org.eclipse.emf.ecore.EObject;
import org.fortiss.tooling.kernel.model.IProjectRootElement;
/**
- * Class storing the metrics computed for the all suitable elemnets.
+ * Class storing the metrics computed for the all suitable elements.
*
* @author groh
*/
@@ -34,7 +34,7 @@ public class MetricData {
/** A map to lookup the corresponding root tree node for a project. */
private Map<IProjectRootElement, DataRootElement> dataRootElementMap;
- /** Constructor.*/
+ /** Constructor. */
public MetricData() {
dataRootElementMap = new HashMap<>();
treeNodeLookupTable = new HashMap<>();
@@ -45,7 +45,7 @@ public class MetricData {
return treeNodeLookupTable;
}
- /** Getter for {@link #DataRootElementMap}. */
+ /** Getter for {@link #dataRootElementMap}. */
public Map<IProjectRootElement, DataRootElement> getDataRootElementMap() {
return dataRootElementMap;
}
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricKey.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricKey.java
index 81ba35f85..d78026f0c 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricKey.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricKey.java
@@ -21,6 +21,7 @@ import java.util.List;
/**
* Enum class representing a key which can be used to store a respective metric.
* Metrics provided by the HierarchicElementProvider.
+ *
* @author groh
*/
public enum MetricKey {
@@ -34,7 +35,7 @@ public enum MetricKey {
NUMBER_OF_CONTAINED_ELEMENTS(false),
/** Key for the metric counting channels. */
NUMBER_OF_CONNECTIONS(false),
- // TODO (#4333):
+ // TODO (#4333):
/** Safety level of the element. */
SAFETY_LEVEL(false, true),
/** How deeply nested the corresponding element is. */
@@ -73,9 +74,14 @@ public enum MetricKey {
NUMBER_OF_TOTAL_COMMENTABLE_ELEMENTS(true),
/** Key for the metric counting the total amount of elements which are commented. */
NUMBER_OF_TOTAL_COMMENTED_ELEMENTS(true),
- /** Key for the metric counting the total amount of elements which do not contain other elements. */
+ /**
+ * Key for the metric counting the total amount of elements which do not contain other elements.
+ */
NUMBER_OF_TOTAL_LEAF_ELEMENTS(true),
- /** Key for the metric counting of the total amount of constant values in all code specifications. */
+ /**
+ * Key for the metric counting of the total amount of constant values in all code
+ * specifications.
+ */
NUMBER_OF_TOTAL_CODE_CONSTANTS(true),
/** Cyclomatic complexity number as sum over all components. */
NUMBER_OF_TOTAL_CODE_CYCLOMATIC_COMPLEXITY(true),
@@ -89,25 +95,24 @@ public enum MetricKey {
/** Getter for {@link #collectorKeys}. */
public static List<MetricKey> getCollectorKeys() {
- if (collectorKeys == null) {
- collectorKeys = new ArrayList<>();
- for (MetricKey key : MetricKey.values()) {
- if (key.isCollectorKey) {
- collectorKeys.add(key);
- }
- }
- }
- return collectorKeys;
+ if(collectorKeys == null) {
+ collectorKeys = new ArrayList<>();
+ for(MetricKey key : MetricKey.values()) {
+ if(key.isCollectorKey) {
+ collectorKeys.add(key);
+ }
+ }
+ }
+ return collectorKeys;
}
/** Used for populating the {@link MetricKey#getCollectorKeys()} method. */
private boolean isCollectorKey;
- // TODO (SB): In MetricKeyNode, there are int, double, and String metrics. However, here are
+ // TODO (4336): In MetricTreeNode, there are int, double, and String metrics. However, here are
// only doubles and strings. What about marking the type of a MetricKey with a
// Class<? extends Number> field? In the constructor, you can check that only the admissible
// types Integer, Double, String are passed.
- // TODO (#4333)
/** Determines if the corresponding value to this key is a string or not */
private boolean isStringValue;
/** Determines if the corresponding value to this key is a integer or not */
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricTreeNode.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricTreeNode.java
index 8eb3c6761..88ddbdfc6 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricTreeNode.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/data/MetricTreeNode.java
@@ -69,17 +69,17 @@ public class MetricTreeNode {
}
/** Getter for {@link #integerMetrics}. */
- public Map<MetricKey, Integer> getStoredIntegers() {
+ public Map<MetricKey, Integer> getIntegerMetrics() {
return integerMetrics;
}
/** Getter for {@link #doubleMetrics}. */
- public Map<MetricKey, Double> getStoredDoubles() {
+ public Map<MetricKey, Double> getDoubleMetrics() {
return doubleMetrics;
}
/** Getter for {@link #stringMetrics}. */
- public Map<MetricKey, String> getStoredStrings() {
+ public Map<MetricKey, String> getStringMetrics() {
return stringMetrics;
}
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/.ratings b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/.ratings
index 4f07e9c17..179fd635e 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/.ratings
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/.ratings
@@ -1,2 +1,2 @@
-IModelQualityService.java e474e51bec8ce03209b69a40f3433a070bd73daa YELLOW
-ModelQualityService.java 4a620f5c7732069d78b0e37e051ff99f4732642e YELLOW
+IModelQualityService.java db237db72cf1a0905fa82b1dfd799f830d092a10 GREEN
+ModelQualityService.java dc65c5cc1f06971db96d62aed4831b137929c789 GREEN
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/IModelQualityService.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/IModelQualityService.java
index e474e51be..db237db72 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/IModelQualityService.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/IModelQualityService.java
@@ -19,7 +19,6 @@ import org.eclipse.emf.ecore.EObject;
import org.fortiss.tooling.ext.quality.IMetricProvider;
import org.fortiss.tooling.ext.quality.IMetricUpdateListener;
import org.fortiss.tooling.ext.quality.data.MetricData;
-import org.fortiss.tooling.ext.quality.storage.ModelQualityStorageManager;
import org.fortiss.tooling.kernel.extension.data.ITopLevelElement;
/**
@@ -32,7 +31,7 @@ public interface IModelQualityService {
static IModelQualityService getInstance() {
return ModelQualityService.getInstance();
}
-
+
/** Starting the service. */
void startService();
@@ -44,8 +43,8 @@ public interface IModelQualityService {
void scheduleMetricCollection(ITopLevelElement top);
/** Returns the {@link MetricData} for the Service. */
- MetricData getMetricData();
-
+ MetricData getMetricData();
+
/**
* Registers the provided {@link IMetricUpdateListener} to be called when the metrics are
* updated.
@@ -54,12 +53,7 @@ public interface IModelQualityService {
* to be registered
*/
void registerMetricUpdateListener(IMetricUpdateListener listener);
-
+
/** Creating the quality folder to store the metrics in. */
- static void createQualityFolder() {
- if(!ModelQualityStorageManager.MODEL_QUALITY_PROJECT_DIR.exists()) {
- ModelQualityStorageManager.MODEL_QUALITY_PROJECT_DIR.mkdirs();
- }
- }
-
+ void createQualityFolder();
}
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/ModelQualityService.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/ModelQualityService.java
index 4a620f5c7..dc65c5cc1 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/ModelQualityService.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/service/ModelQualityService.java
@@ -16,13 +16,14 @@
package org.fortiss.tooling.ext.quality.service;
import static java.util.Collections.emptyList;
+import static java.util.Collections.sort;
import static java.util.stream.Collectors.groupingBy;
+import static org.fortiss.tooling.kernel.utils.KernelModelElementUtils.getRootElements;
import java.io.IOException;
import java.security.NoSuchAlgorithmException;
import java.time.LocalDateTime;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -47,6 +48,7 @@ import org.fortiss.tooling.ext.quality.data.MetricData;
import org.fortiss.tooling.ext.quality.data.MetricKey;
import org.fortiss.tooling.ext.quality.data.MetricTreeNode;
import org.fortiss.tooling.ext.quality.storage.CSVFileWriter;
+import org.fortiss.tooling.ext.quality.storage.ModelQualityStorageManager;
import org.fortiss.tooling.kernel.extension.data.IConstraintViolation;
import org.fortiss.tooling.kernel.extension.data.IConstraintViolation.ESeverity;
import org.fortiss.tooling.kernel.extension.data.ITopLevelElement;
@@ -59,7 +61,6 @@ import org.fortiss.tooling.kernel.service.IKernelIntrospectionSystemService;
import org.fortiss.tooling.kernel.service.IPersistencyService;
import org.fortiss.tooling.kernel.service.base.EObjectAwareServiceBase;
import org.fortiss.tooling.kernel.service.listener.IPersistencyServiceListener;
-import org.fortiss.tooling.kernel.utils.EcoreUtils;
/**
* This class implements the {@link IModelQualityService}.
@@ -67,7 +68,8 @@ import org.fortiss.tooling.kernel.utils.EcoreUtils;
* @author blaschke
* @author groh
*/
-/* package */ class ModelQualityService extends EObjectAwareServiceBase<IMetricProvider<? extends EObject>>
+/* package */ class ModelQualityService
+ extends EObjectAwareServiceBase<IMetricProvider<? extends EObject>>
implements IIntrospectiveKernelService, IModelQualityService, IPersistencyServiceListener {
/** The singleton instance. */
@@ -78,7 +80,7 @@ import org.fortiss.tooling.kernel.utils.EcoreUtils;
return INSTANCE;
}
- /** Instance of the MetricDataManager. */
+ /** Collected metrics. */
private MetricData metricDataContainer = new MetricData();
/** List of {@link IMetricUpdateListener} which are called when the metrics are updated */
@@ -109,7 +111,7 @@ import org.fortiss.tooling.kernel.utils.EcoreUtils;
e.printStackTrace();
} catch(IOException e) {
e.printStackTrace();
- } catch (CoreException e) {
+ } catch(CoreException e) {
e.printStackTrace();
}
} while(true);
@@ -153,7 +155,7 @@ import org.fortiss.tooling.kernel.utils.EcoreUtils;
"\n\nThis service allows to track the evolution of model metrics." +
"\n The service determines the current metrics of a model and stores" +
"\n them in a CSV file for later analysis of the modeling history." +
- "\n\nThe service extension point is 'not existant'.";
+ "\n\nThe service extension point is 'not existent'.";
}
/**
@@ -163,35 +165,19 @@ import org.fortiss.tooling.kernel.utils.EcoreUtils;
* the {@link ITopLevelElement} on which the analysis should be performed
* @throws IOException
* @throws NoSuchAlgorithmException
- * @throws CoreException
+ * @throws CoreException
*/
private void performMetricCollection(ITopLevelElement topLevelElement)
throws NoSuchAlgorithmException, IOException, CoreException {
- // TODO (#4333): Is the problem that metrics a duplicated when following (non-containment)
- // EReferences?
- // Is is only a problem when IProjectRoot elements (as for the allocation tables) are
- // referenced, or a general problem? A more general solution would be to adapt the model
- // traversal strategy.
- // Sort root elements, as some metrics are saved into trees from other root elements
- // To avoid these problems, the elements have to be processed in a certain order
-
- // TODO (#4333): In case the model traversal strategy cannot be changed, the implementation
- // below has to be generalized such that no knowledge about the AF3 metamodel is injected
- // into the kernel.
-
- // Currently the only element affected by this is the allocation table, which is
- // why push it to the end of the list
- List<IProjectRootElement> rootElements = new ArrayList<>();
- rootElements.addAll(EcoreUtils.getChildrenWithType(topLevelElement.getRootModelElement(),
- IProjectRootElement.class));
+ List<IProjectRootElement> rootElements =
+ getRootElements(topLevelElement.getRootModelElement(), IProjectRootElement.class);
+ // TODO(#XXX): Remove below workaround that makes assumptions on the overall structure of
+ // the model, and its implementation.
String allocationTableClassName =
"org.fortiss.af3.allocation.model.impl.AllocationTableCollectionImpl";
-
- // Sort with custom comparator which pushes the object back if it matches the above class
- // name
- Collections.sort(rootElements, (r1, r2) -> {
+ sort(rootElements, (r1, r2) -> {
if(allocationTableClassName.equals(r1.getClass().getName())) {
return 1;
} else if(allocationTableClassName.equals(r2.getClass().getName())) {
@@ -227,7 +213,7 @@ import org.fortiss.tooling.kernel.utils.EcoreUtils;
if(node != null) {
// Collect violations and store them in the error and warning metric
List<IConstraintViolation<? extends EObject>> violations = entry.getValue();
- var integers = node.getStoredIntegers();
+ var integers = node.getIntegerMetrics();
integers.put(MetricKey.CONSTRAINT_VIOLATIONS_ERROR, (int)violations.stream()
.filter(v -> v.getSeverity() == ESeverity.ERROR).count());
integers.put(MetricKey.CONSTRAINT_VIOLATIONS_WARNING, (int)violations.stream()
@@ -260,10 +246,8 @@ import org.fortiss.tooling.kernel.utils.EcoreUtils;
private void recursivlyCollectMetrics(MetricTreeNode node, IHierarchicElement currentElement,
int recursionLevel) {
- var metricData = metricDataContainer;
-
collectMetrics(node, currentElement);
- node.getStoredIntegers().put(MetricKey.NESTING_LEVEL, recursionLevel);
+ node.getIntegerMetrics().put(MetricKey.NESTING_LEVEL, recursionLevel);
// Check if any of the specifications is an IHierarchicElement
// This is for example the case for AF3 StateAutomatons
@@ -278,7 +262,7 @@ import org.fortiss.tooling.kernel.utils.EcoreUtils;
specificationElement.getContainedElements();
if(containedElements.size() == 1) {
// Add reference from the element to this, so the lookups works as expected
- metricData.getTreeNodeLookupTable().put(specificationElement, node);
+ metricDataContainer.getTreeNodeLookupTable().put(specificationElement, node);
// There is exactly one contained element. Skip the specification element to get a
// more useful tree structure
recursivlyCollectMetrics(node, containedElements.get(0), recursionLevel);
@@ -286,19 +270,19 @@ import org.fortiss.tooling.kernel.utils.EcoreUtils;
recursivlyCollectMetrics(node, specificationElement, recursionLevel);
}
// Add reference from the element to this, so the lookups works as expected
- metricData.getTreeNodeLookupTable().put(currentElement, node);
+ metricDataContainer.getTreeNodeLookupTable().put(currentElement, node);
} else {
// Iterate over all children and merge values
- var nodeDoubles = node.getStoredDoubles();
- var nodeIntegers = node.getStoredIntegers();
+ var nodeDoubles = node.getDoubleMetrics();
+ var nodeIntegers = node.getIntegerMetrics();
for(IHierarchicElement containedElement : currentElement.getContainedElements()) {
MetricTreeNode child = new MetricTreeNode();
node.getChildren().add(child);
recursivlyCollectMetrics(child, containedElement, recursionLevel + 1);
for(MetricKey key : MetricKey.getCollectorKeys()) {
- var childDoubles = child.getStoredDoubles();
- var childIntegers = child.getStoredIntegers();
+ var childDoubles = child.getDoubleMetrics();
+ var childIntegers = child.getIntegerMetrics();
if(childDoubles.containsKey(key)) {
nodeDoubles.merge(key, childDoubles.get(key), Double::sum);
}
@@ -308,12 +292,14 @@ import org.fortiss.tooling.kernel.utils.EcoreUtils;
}
}
- metricData.getTreeNodeLookupTable().put(currentElement, node);
-
+ metricDataContainer.getTreeNodeLookupTable().put(currentElement, node);
+
// TODO (#4332) Restructure GraphMetricsProvider in quality Plugin
- GraphMetricsProvider.calculateBetweennessCentrality(currentElement, metricData, false);
- GraphMetricsProvider.calculateBetweennessCentrality(currentElement, metricData, true);
- node.getStoredDoubles().put(MetricKey.CLUSTERING_COEFFICIENT,
+ GraphMetricsProvider.calculateBetweennessCentrality(currentElement, metricDataContainer,
+ false);
+ GraphMetricsProvider.calculateBetweennessCentrality(currentElement, metricDataContainer,
+ true);
+ node.getDoubleMetrics().put(MetricKey.CLUSTERING_COEFFICIENT,
GraphMetricsProvider.calculateClusteringCoefficent(currentElement));
}
}
@@ -333,7 +319,7 @@ import org.fortiss.tooling.kernel.utils.EcoreUtils;
List<IMetricProvider<? extends EObject>> providers = getAllMetricProviders(object);
for(IMetricProvider<? extends EObject> provider : providers) {
- //TODO (#4331)
+ // TODO (#4331)
((IMetricProvider<EObject>)provider).collectMetrics(node, object);
}
}
@@ -416,10 +402,18 @@ import org.fortiss.tooling.kernel.utils.EcoreUtils;
public void topLevelElementContentChanged(ITopLevelElement element) {
scheduleMetricCollection(element);
}
-
+
/** {@inheritDoc} */
@Override
public MetricData getMetricData() {
return metricDataContainer;
}
+
+ /** {@inheritDoc} */
+ @Override
+ public void createQualityFolder() {
+ if(!ModelQualityStorageManager.MODEL_QUALITY_PROJECT_DIR.exists()) {
+ ModelQualityStorageManager.MODEL_QUALITY_PROJECT_DIR.mkdirs();
+ }
+ }
}
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/.ratings b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/.ratings
index 000e059c8..d2dd8e585 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/.ratings
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/.ratings
@@ -1,2 +1,2 @@
-CSVFileWriter.java 89ed8b8b05dbbe55c30bd416e9255e3896ca980c YELLOW
-ModelQualityStorageManager.java c5a6783f51f596663bf836f98abf2b23130fe180 YELLOW
+CSVFileWriter.java ba986ec60099bf94472f9e80cba10c01c85bc73f GREEN
+ModelQualityStorageManager.java c5a6783f51f596663bf836f98abf2b23130fe180 GREEN
diff --git a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/CSVFileWriter.java b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/CSVFileWriter.java
index 89ed8b8b0..ba986ec60 100644
--- a/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/CSVFileWriter.java
+++ b/org.fortiss.tooling.ext.quality/src/org/fortiss/tooling/ext/quality/storage/CSVFileWriter.java
@@ -15,6 +15,9 @@
+--------------------------------------------------------------------------*/
package org.fortiss.tooling.ext.quality.storage;
+import static java.util.stream.Collectors.toList;
+import static org.apache.commons.io.FilenameUtils.getBaseName;
+
import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.File;
@@ -34,8 +37,7 @@ import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;
-import org.apache.commons.io.FilenameUtils;
-import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.runtime.NullProgressMonitor;
import org.fortiss.tooling.ext.quality.data.DataRootElement;
import org.fortiss.tooling.ext.quality.data.MetricKey;
import org.fortiss.tooling.ext.quality.service.IModelQualityService;
@@ -49,10 +51,15 @@ import org.fortiss.tooling.kernel.model.IProjectRootElement;
*/
public class CSVFileWriter {
+ /** Suffix of the file in which the data is written. */
+ private static final String CSV = ".csv";
+
/**
- * Method to write the data into CSV files. For each project one file is built. If it is already existant,
- * it uses to the existing keys in the file as dimensions, to help that the log file always remains consistent.
- * Newly added MetricKeys are not added in the next exportations if it was not present and build.
+ * Method to write the data into CSV files. For each project, one file is built. If it already
+ * exists, it uses to the existing keys in the file as dimensions, to help that the log file
+ * always remains consistent.
+ *
+ * TODO (#4334) Newly added {@link MetricKey}s are not added in the next export
*
* @param map
* to save into CSV file
@@ -60,31 +67,40 @@ public class CSVFileWriter {
* @throws NoSuchAlgorithmException
*
*/
-
- /** Suffix of the file in which the data is written. */
- private static final String CSV = ".csv";
-
public static void metricExtractionToCSV(Map<IProjectRootElement, DataRootElement> map)
- throws NoSuchAlgorithmException, IOException, CoreException {
+ throws NoSuchAlgorithmException, IOException {
// check if there is any data to export
if(map == null || map.isEmpty()) {
return;
}
-
- IModelQualityService.createQualityFolder();
- // path to CSV file
+ IModelQualityService.getInstance().createQualityFolder();
+
+ // Map is not empty due to check above
DataRootElement firstValue = map.entrySet().iterator().next().getValue();
ITopLevelElement topLevelElement = firstValue.getTopLevelElement();
String projectName = topLevelElement.getSaveableName();
- var fileBaseName = FilenameUtils.getBaseName(projectName).toString();
- Path path = new File(ModelQualityStorageManager.MODEL_QUALITY_PROJECT_DIR, fileBaseName.concat(CSV)).toPath();
-
- //save before using the saved file to produce the hash
- //FIXME (KB): produces unstoppable exceptions if I use it (IllegalStateExceptions)
- //topLevelElement.doSave(new NullProgressMonitor());
+ var fileBaseName = getBaseName(projectName).toString();
+ // TODO (#4333)
+ Path path = new File(ModelQualityStorageManager.MODEL_QUALITY_PROJECT_DIR,
+ fileBaseName.concat(CSV)).toPath();
+
+ // Metric extraction is triggered when model files are saved. Therefore, the topLevelElement
+ // should never be dirty here. In order to be on the safe side, conditionally save model.
+ // save before using the saved file to produce the hash
+ if(topLevelElement.isDirty()) {
+ try {
+ topLevelElement.doSave(new NullProgressMonitor());
+ // Avoid duplicated metric extraction (the save operation will trigger another one).
+ return;
+ } catch(Exception e) {
+ e.printStackTrace();
+ }
+ return;
+ }
+ // TODO (#4333)
Path modelFile = new File(ModelQualityStorageManager.MODEL_PROJECT_DIR,
topLevelElement.getSaveableName()).toPath();
String projectHash = computeGitObjectHash(modelFile);
@@ -141,7 +157,7 @@ public class CSVFileWriter {
// Collect all MetricKeys, convert them to a string and append them to the allKeys
// list
allKeys.addAll(Arrays.stream(MetricKey.values()).map(MetricKey::toString)
- .map(String::toLowerCase).collect(Collectors.toList()));
+ .map(String::toLowerCase).collect(toList()));
// Write first line
writer.write(String.join(",", allKeys));
@@ -160,12 +176,12 @@ public class CSVFileWriter {
String rootName = rootElement.getClass().getSimpleName();
var rootNode = dataRootElement.getRootNode();
rootNode.traverseTree(node -> {
- var integers = node.getStoredIntegers();
- var doubles = node.getStoredDoubles();
- var strings = node.getStoredStrings();
+ var integers = node.getIntegerMetrics();
+ var doubles = node.getDoubleMetrics();
+ var strings = node.getStringMetrics();
// collect IDs of children
String children_ids = node.getChildren().stream()
- .map(n -> n.getStoredIntegers().get(MetricKey.UNIQUE_ID))
+ .map(n -> n.getIntegerMetrics().get(MetricKey.UNIQUE_ID))
.map(id -> id.toString()).reduce((a, b) -> a + " " + b).orElse("");
// These 6 data entries are just concatenated in front
@@ -176,8 +192,7 @@ public class CSVFileWriter {
.map(String::toUpperCase).map(MetricKey::valueOf).map(k -> {
if(k.isStringValue()) {
return strings.get(k);
- }
- if(k.isDoubleValue()) {
+ } else if(k.isDoubleValue()) {
return doubles.get(k);
}
return integers.get(k);
@@ -218,7 +233,8 @@ public class CSVFileWriter {
// Construct pre-image (input to hash function) according to Git specification
String fileContents = inputBuilder.toString();
- // fileContexts.length() does not return the correct length on some platforms (e.g., Linux)
+ // fileContexts.length() does not return the correct length on some platforms (e.g.,
+ // Linux)
int n = fileContents.getBytes().length;
String preImage = "blob " + n + "\0" + fileContents;
--
GitLab