From 49013dcf3abc43a3832ebe607af2fd978f31f2f9 Mon Sep 17 00:00:00 2001
From: Simon Barner <barner@fortiss.org>
Date: Mon, 16 Oct 2023 18:08:46 +0200
Subject: [PATCH] Update Review (1 RED). Note done yet.
Issue-Ref: 4310
Issue-Url: https://git.fortiss.org/af3/af3/-/issues/4310
Signed-off-by: Simon Barner <barner@fortiss.org>
---
.../META-INF/MANIFEST.MF | 11 ++----
.../fortiss/tooling/ext/quality/ui/.ratings | 2 +-
.../ui/ModelQualityExtractionMenu.java | 14 ++++---
.../tooling/ext/quality/ui/view/fx/.ratings | 4 +-
.../ui/view/fx/ModelQualityFXController.java | 37 +++++++++----------
.../ui/view/fx/ModelQualityFXViewPart.java | 4 --
6 files changed, 32 insertions(+), 40 deletions(-)
diff --git a/org.fortiss.tooling.ext.quality.ui/META-INF/MANIFEST.MF b/org.fortiss.tooling.ext.quality.ui/META-INF/MANIFEST.MF
index f86a41034..b0798135a 100644
--- a/org.fortiss.tooling.ext.quality.ui/META-INF/MANIFEST.MF
+++ b/org.fortiss.tooling.ext.quality.ui/META-INF/MANIFEST.MF
@@ -5,14 +5,9 @@ Bundle-SymbolicName: org.fortiss.tooling.ext.quality.ui;singleton:=true
Bundle-Version: 2.23.0.qualifier
Automatic-Module-Name: org.fortiss.tooling.ext.quality.ui
Bundle-RequiredExecutionEnvironment: JavaSE-11
-Require-Bundle: org.eclipse.ui.ide;visibility:=reexport,
- org.fortiss.tooling.base.ui;visibility:=reexport,
- org.fortiss.tooling.kernel,
- org.fortiss.tooling.kernel.ui,
- org.fortiss.tooling.ext.quality,
- org.fortiss.tooling.spiderchart.ui,
- org.eclipse.emf.ecore,
- org.fortiss.af3.project;bundle-version="2.23.0"
+Require-Bundle: org.fortiss.tooling.base.ui;visibility:=reexport,
+ org.fortiss.tooling.ext.quality;visibility:=reexport,
+ org.fortiss.tooling.spiderchart.ui;visibility:=reexport
Export-Package: org.fortiss.tooling.ext.quality.ui.view.fx
Bundle-Activator: org.fortiss.tooling.ext.quality.ui.ModelQualityUIActivator
Bundle-ActivationPolicy: lazy
diff --git a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/.ratings b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/.ratings
index 10f0daaec..cbf0a8d86 100644
--- a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/.ratings
+++ b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/.ratings
@@ -1,2 +1,2 @@
-ModelQualityExtractionMenu.java 3bdbf3d37715539648af8ba7feefbec9370c4dc4 YELLOW
+ModelQualityExtractionMenu.java d9282c7df44aa9461ec662beb557e74589f6b05f GREEN
ModelQualityUIActivator.java 2f47db82bc088072dbfe53f82d3ed417d77fd23c GREEN
diff --git a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/ModelQualityExtractionMenu.java b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/ModelQualityExtractionMenu.java
index 3bdbf3d37..d9282c7df 100644
--- a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/ModelQualityExtractionMenu.java
+++ b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/ModelQualityExtractionMenu.java
@@ -47,9 +47,12 @@ public class ModelQualityExtractionMenu implements IContextMenuContributor {
@Override
public List<IContributionItem> getContributedItems(EObject selection,
ContextMenuContextProvider contextProvider) {
+
if(!(selection.eContainer() != null)) {
return emptyList();
}
+
+ // A null eContainer indicates the model root
return asList(new IContributionItem[] {
new ActionContributionItem(new MetricExtractionAction(selection))});
}
@@ -68,19 +71,20 @@ public class ModelQualityExtractionMenu implements IContextMenuContributor {
/** Action for generating the set of . */
protected class MetricExtractionAction extends Action {
- /** {@link FileProject} on which the action will be triggered. */
- private final EObject fp;
+ /** Model on which the action will be triggered. */
+ private final EObject model;
/** Constructor. */
- public MetricExtractionAction(EObject fp) {
+ public MetricExtractionAction(EObject model) {
super(MENU_NAME, getActionIcon());
- this.fp = fp;
+ this.model = model;
}
/** {@inheritDoc} */
@Override
public void run() {
- ITopLevelElement top = IPersistencyService.getInstance().getTopLevelElementFor(this.fp);
+ ITopLevelElement top =
+ IPersistencyService.getInstance().getTopLevelElementFor(this.model);
IModelQualityService.getInstance().scheduleMetricCollection(top);
}
}
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 c0833d43f..ed265042f 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 67bb0a067c578ac9a9b30e5973587cc57374a782 YELLOW
-ModelQualityFXViewPart.java a1e82fe68a3fe7d813fa61d5148ffcd1592000f7 YELLOW
+ModelQualityFXController.java 64c5fb2ca8cca77982b5a20695efcf2b1442045e 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 67bb0a067..64c5fb2ca 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
@@ -15,10 +15,10 @@
+--------------------------------------------------------------------------*/
package org.fortiss.tooling.ext.quality.ui.view.fx;
+import static java.util.stream.Collectors.toList;
import static javafx.scene.paint.Color.BLUE;
import static javafx.scene.paint.Color.DARKGRAY;
import static javafx.scene.paint.Color.LIGHTGRAY;
-import static java.util.stream.Collectors.toList;
import static org.fortiss.tooling.common.ui.javafx.style.FontStyle.BLACK_VERDANA_10PT;
import static org.fortiss.tooling.common.ui.javafx.style.FontStyle.BLACK_VERDANA_12PT;
import static org.fortiss.tooling.common.ui.javafx.style.FontStyle.BLACK_VERDANA_14PT;
@@ -27,7 +27,6 @@ import static org.fortiss.tooling.common.ui.javafx.style.LineStyle.SOLID_BLACK_1
import static org.fortiss.tooling.kernel.ui.util.SelectionUtils.checkAndPickFirst;
import static org.fortiss.tooling.kernel.utils.EcoreUtils.getFirstParentWithType;
-
import java.text.DecimalFormat;
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
@@ -35,7 +34,6 @@ import java.util.HashSet;
import java.util.List;
import java.util.Set;
-
import org.eclipse.emf.ecore.EObject;
import org.eclipse.jface.viewers.ISelection;
import org.eclipse.ui.ISelectionListener;
@@ -153,8 +151,6 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
// Invalidate last selection, so if no new selection is found the charts are cleared
lastSelectedElement = null;
- IModelQualityService mqs = IModelQualityService.getInstance();
-
EObject selected = checkAndPickFirst(selection, EObject.class);
if(selected == null) {
@@ -170,6 +166,8 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
updateCharts("Selection does not have a project root element.");
return;
}
+
+ IModelQualityService mqs = IModelQualityService.getInstance();
var rootDataElement = mqs.getMetricData().getDataRootElementMap().get(currentRootElement);
if(rootDataElement == null) {
@@ -209,14 +207,10 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
private void updateCharts(String message) {
if(lastSelectedElement != null) {
- MetricData metricData =
- IModelQualityService.getInstance().getMetricData();
+ MetricData metricData = IModelQualityService.getInstance().getMetricData();
MetricTreeNode node = metricData.getTreeNodeLookupTable().get(lastSelectedElement);
if(node != null) {
-
- // updateChoiceBoxes(node);
-
if(!node.getChildren().isEmpty()) {
updatePieChart(node);
@@ -334,18 +328,21 @@ 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
+ // to the .get(0) statement
Set<MetricKey> keysIntersection = lastViewedElements.get(0).getValue();
for(var p : lastViewedElements) {
keysIntersection.retainAll(p.getValue());
}
// Create DataSeries for all elements
- List<Pair<MetricTreeNode, DataSeries>> dataSeriesList = lastViewedElements.stream()
- .map(pair -> new Pair<>(pair.getKey(),
- (pair.getKey().getName() == null) ? new DataSeries("No Name found") : new DataSeries(pair.getKey().getName())
- ))
- .collect(toList());
-
+ List<Pair<MetricTreeNode, DataSeries>> dataSeriesList =
+ lastViewedElements.stream().map(pair -> {
+ MetricTreeNode key = pair.getKey();
+ String name = key.getName();
+ return new Pair<>(key, (name == null) ? new DataSeries("No name found")
+ : new DataSeries(name));
+ }).collect(toList());
// Create SpiderChart and set basic styling
SpiderChart spiderChart = new SpiderChart();
@@ -384,6 +381,8 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
for(var pair : dataSeriesList) {
spiderChart.addData(pair.getValue());
// Assign different colors to different data series
+ // updateLastViewedElements() ensures that the are are at most three elements in
+ // updateLastViewedElements, from which dataSeriesList is derived above.
Color color = SPIDER_CHART_OVERLAY_COLORS[i];
LineStyle olive1pt = new LineStyle(color);
FillStyle oliveFill = new FillStyle(color, 0.25);
@@ -415,10 +414,8 @@ public class ModelQualityFXController extends CompositeFXControllerBase<SplitPan
return;
}
- // Store IModelQualityService.getInstance() (sic!) in a variable (e.g. "mqs") for
- // readability
- var rootDataElement = IModelQualityService.getInstance().getMetricData()
- .getDataRootElementMap().get(updatedElement);
+ IModelQualityService mqs = IModelQualityService.getInstance();
+ var rootDataElement = mqs.getMetricData().getDataRootElementMap().get(updatedElement);
if(rootDataElement != null) {
bottomText.setText("Metrics last updated: " +
timeFormat.format(rootDataElement.getLastRefresh()) +
diff --git a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXViewPart.java b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXViewPart.java
index a1e82fe68..9cfc7f60a 100644
--- a/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXViewPart.java
+++ b/org.fortiss.tooling.ext.quality.ui/src/org/fortiss/tooling/ext/quality/ui/view/fx/ModelQualityFXViewPart.java
@@ -27,9 +27,6 @@ import javafx.scene.Scene;
*/
public class ModelQualityFXViewPart extends AF3FXViewPart implements IModelQualityViewPart {
- // TODO (SB): Is there a reason to make the controller static? (also rename to viewController)
- // FIXME (KB): Naming convention and static taken from reuse library. So you want me to change the FX controller class to viewController?
- // Would result in inconsistent use otherwise. still change it and add the view controller as parameter for instancing a View Part?
/** The FX Controller for this view. */
private static final ModelQualityFXController VIEW_CONTROLLER = new ModelQualityFXController();
@@ -52,7 +49,6 @@ public class ModelQualityFXViewPart extends AF3FXViewPart implements IModelQuali
@Override
protected Scene createFxScene() {
Scene scene = super.createFxScene();
-
getSite().getWorkbenchWindow().getSelectionService().addSelectionListener(VIEW_CONTROLLER);
return scene;
--
GitLab