From 5fb5ec8d095bfcf9bccbd6be506e509b72643fa5 Mon Sep 17 00:00:00 2001 From: Reino Ruusu Date: Mon, 14 Sep 2020 17:12:37 +0300 Subject: [PATCH] Tech type table content validation gitlab #96 Change-Id: Icbb92e366748d8de9205cd29487d03eaacb5e7a4 (cherry picked from commit 7d3e650a2d3891988e7d0ef83fcb3b283a226d09) --- .../fragment.e4xmi | 2 + .../ui/techtype/table/TechTypeTable.java | 111 ++++++++++++++- .../ui/techtype/table/TechTypeTableView.java | 36 +++-- .../table/ValidateTechTypeTableHandler.java | 42 ++++++ .../techtype/TechTypeValidationUtils.java | 132 ++++++++++++++++++ .../requests/PossibleTechTypeKeyName.java | 29 ++++ .../techtype/requests/TechTypeTableData.java | 2 +- .../requests/TechTypeTableKeyName.java | 14 +- 8 files changed, 339 insertions(+), 29 deletions(-) create mode 100644 org.simantics.district.network.ui/src/org/simantics/district/network/ui/techtype/table/ValidateTechTypeTableHandler.java create mode 100644 org.simantics.district.network/src/org/simantics/district/network/techtype/TechTypeValidationUtils.java create mode 100644 org.simantics.district.network/src/org/simantics/district/network/techtype/requests/PossibleTechTypeKeyName.java diff --git a/org.simantics.district.network.ui/fragment.e4xmi b/org.simantics.district.network.ui/fragment.e4xmi index 89afdc5d..1ea9c909 100644 --- a/org.simantics.district.network.ui/fragment.e4xmi +++ b/org.simantics.district.network.ui/fragment.e4xmi @@ -17,6 +17,7 @@ + @@ -50,5 +51,6 @@ + diff --git a/org.simantics.district.network.ui/src/org/simantics/district/network/ui/techtype/table/TechTypeTable.java b/org.simantics.district.network.ui/src/org/simantics/district/network/ui/techtype/table/TechTypeTable.java index 0c44bff0..8ee627a5 100644 --- a/org.simantics.district.network.ui/src/org/simantics/district/network/ui/techtype/table/TechTypeTable.java +++ b/org.simantics.district.network.ui/src/org/simantics/district/network/ui/techtype/table/TechTypeTable.java @@ -4,11 +4,14 @@ import java.io.IOException; import java.nio.charset.Charset; import java.nio.file.Files; import java.nio.file.Paths; +import java.util.Set; import java.util.stream.Collectors; import org.eclipse.jface.layout.GridDataFactory; import org.eclipse.jface.layout.GridLayoutFactory; import org.eclipse.nebula.widgets.nattable.NatTable; +import org.eclipse.nebula.widgets.nattable.config.AbstractRegistryConfiguration; +import org.eclipse.nebula.widgets.nattable.config.CellConfigAttributes; import org.eclipse.nebula.widgets.nattable.config.DefaultNatTableStyleConfiguration; import org.eclipse.nebula.widgets.nattable.config.IConfigRegistry; import org.eclipse.nebula.widgets.nattable.copy.command.CopyDataCommandHandler; @@ -31,9 +34,15 @@ import org.eclipse.nebula.widgets.nattable.hover.config.BodyHoverStylingBindings import org.eclipse.nebula.widgets.nattable.layer.DataLayer; import org.eclipse.nebula.widgets.nattable.layer.ILayer; import org.eclipse.nebula.widgets.nattable.layer.IUniqueIndexLayer; +import org.eclipse.nebula.widgets.nattable.layer.LabelStack; +import org.eclipse.nebula.widgets.nattable.layer.cell.IConfigLabelAccumulator; import org.eclipse.nebula.widgets.nattable.reorder.RowReorderLayer; import org.eclipse.nebula.widgets.nattable.selection.SelectionLayer; import org.eclipse.nebula.widgets.nattable.sort.SortHeaderLayer; +import org.eclipse.nebula.widgets.nattable.style.CellStyleAttributes; +import org.eclipse.nebula.widgets.nattable.style.DisplayMode; +import org.eclipse.nebula.widgets.nattable.style.Style; +import org.eclipse.nebula.widgets.nattable.util.GUIHelper; import org.eclipse.nebula.widgets.nattable.viewport.ViewportLayer; import org.eclipse.swt.SWT; import org.eclipse.swt.dnd.Clipboard; @@ -43,13 +52,20 @@ import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Text; import org.simantics.Simantics; import org.simantics.db.Resource; +import org.simantics.db.WriteGraph; +import org.simantics.db.common.request.WriteRequest; import org.simantics.db.exception.DatabaseException; +import org.simantics.db.layer0.request.PossibleActiveModel; +import org.simantics.district.network.techtype.requests.PossibleTechTypeKeyName; +import org.simantics.district.network.techtype.requests.PossibleTechTypeTable; import org.simantics.district.network.techtype.requests.WriteTechTypeTable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class TechTypeTable extends Composite { + private static final String INVALID_LABEL = "INVALID"; + private final static Logger LOGGER = LoggerFactory.getLogger(TechTypeTable.class); NatTable table; @@ -69,10 +85,14 @@ public class TechTypeTable extends Composite { private TechTypeTableSortModel sortModel; private Resource componentType; + private Resource tableResource; + + protected Set validationResult; - public TechTypeTable(Composite parent, int style, Resource componentType, String data) { + public TechTypeTable(Composite parent, int style, Resource componentType, Resource tableResource, String data) { super(parent, style); this.componentType = componentType; + this.tableResource = tableResource; defaultInitializeUI(data); } @@ -120,7 +140,7 @@ public class TechTypeTable extends Composite { // directly as body layer is also working. bodyDataProvider = new TechTypeTableDataProvider(data); bodyDataLayer = new DataLayer(bodyDataProvider); - + RowReorderLayer rowReorderLayer = new RowReorderLayer( columnHideShowLayer = new ColumnHideShowLayer(bodyDataLayer)); @@ -167,6 +187,21 @@ public class TechTypeTable extends Composite { table = new NatTable(parent, NatTable.DEFAULT_STYLE_OPTIONS | SWT.BORDER, gridLayer, false); GridDataFactory.fillDefaults().grab(true, true).applyTo(table); + + // Show entries labeled "INVALID" with red text + table.addConfiguration(new AbstractRegistryConfiguration() { + @Override + public void configureRegistry(IConfigRegistry configRegistry) { + Style cellStyle = new Style(); + cellStyle.setAttributeValue(CellStyleAttributes.BACKGROUND_COLOR, GUIHelper.COLOR_RED); + configRegistry.registerConfigAttribute( + CellConfigAttributes.CELL_STYLE, + cellStyle, + DisplayMode.NORMAL, + INVALID_LABEL + ); + } + }); // Register a CopyDataCommandHandler that also copies the headers and // uses the configured IDisplayConverters @@ -187,6 +222,18 @@ public class TechTypeTable extends Composite { table.configure(); } + private static String getKeyColumnName(Resource componentType) { + String keyName = null; + if (componentType != null) { + try { + keyName = Simantics.getSession().syncRequest(new PossibleTechTypeKeyName(componentType)); + } catch (DatabaseException e) { + LOGGER.error("Failed to read possible tech type key name for {}", componentType, e); + } + } + return keyName.startsWith("_") ? keyName.substring(1) : keyName; + } + @Override public void dispose() { cpb.dispose(); @@ -203,12 +250,22 @@ public class TechTypeTable extends Composite { } try { - Simantics.getSession().syncRequest(new WriteTechTypeTable(componentType, data)); + Simantics.getSession().syncRequest(new WriteRequest() { + @Override + public void perform(WriteGraph graph) throws DatabaseException { + graph.syncRequest(new WriteTechTypeTable(componentType, data)); + Resource model = graph.syncRequest(new PossibleActiveModel(Simantics.getProjectResource())); + if (model != null) { + tableResource = graph.syncRequest(new PossibleTechTypeTable(model, componentType)); + } + } + }); } catch (DatabaseException e) { LOGGER.error("Failed to write tech type table data to model", e); } setTechTypeData(data); + setValidationResult(null); } public void setTechTypeData(String data) { @@ -220,5 +277,51 @@ public class TechTypeTable extends Composite { this.componentType = componentType; } -} + /** + * Set results of a validation operation + * + * Invalid entries are designated by a string of the form "/". + * + * This method must be called in the SWT thread. + * + * @param result A set of strings representing invalid entries + */ + public void setValidationResult(Set result) { + if (result != null && result.isEmpty()) + result = null; + + this.validationResult = result; + if (result != null) { + String keyName = getKeyColumnName(componentType); + + bodyDataLayer.setConfigLabelAccumulator(new IConfigLabelAccumulator() { + @Override + public void accumulateConfigLabels(LabelStack configLabels, int columnPosition, int rowPosition) { + if (keyName != null) { + int keyColumn = bodyDataProvider.getVariableIndex(keyName); + if (keyColumn >= 0) { + String key = (String) bodyDataProvider.getDataValue(keyColumn, rowPosition); + String columnName = bodyDataProvider.getVariableName(columnPosition); + + if (validationResult.contains(key + "/" + columnName)) { + configLabels.addLabel(INVALID_LABEL); + } + } + } + } + }); + } else { + bodyDataLayer.setConfigLabelAccumulator(null); + } + + table.refresh(); + } + /** + * Get a resource representation of the currently open table, or null if + * table is not stored in the model. + */ + public Resource getCurrentTable() { + return tableResource; + } +} diff --git a/org.simantics.district.network.ui/src/org/simantics/district/network/ui/techtype/table/TechTypeTableView.java b/org.simantics.district.network.ui/src/org/simantics/district/network/ui/techtype/table/TechTypeTableView.java index 15b31784..6c25251c 100644 --- a/org.simantics.district.network.ui/src/org/simantics/district/network/ui/techtype/table/TechTypeTableView.java +++ b/org.simantics.district.network.ui/src/org/simantics/district/network/ui/techtype/table/TechTypeTableView.java @@ -21,8 +21,8 @@ import org.simantics.db.common.NamedResource; import org.simantics.db.common.procedure.adapter.TransientCacheListener; import org.simantics.db.exception.DatabaseException; import org.simantics.db.layer0.request.PossibleActiveModel; -import org.simantics.db.layer0.request.PossibleResource; import org.simantics.district.network.DistrictNetworkUtil; +import org.simantics.district.network.techtype.requests.PossibleTechTypeTable; import org.simantics.district.network.techtype.requests.PossibleTechTypeTableData; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -42,9 +42,20 @@ public class TechTypeTableView { MToolBar toolBar = MMenuFactory.INSTANCE.createToolBar(); toolBar.setToBeRendered(true); toolBar.getChildren().add(createImportCSVDataToolItem(app)); + toolBar.getChildren().add(createValidateTableToolItem(app)); part.setToolbar(toolBar); } + private MHandledToolItem createValidateTableToolItem(MApplication app) { + MHandledToolItem createHandledToolItem = MMenuFactory.INSTANCE.createHandledToolItem(); + // Command is contributed via fragment + MCommand command = app.getCommand("org.simantics.district.network.ui.command.validatetechtypetable"); + createHandledToolItem.setCommand(command); //$NON-NLS-1$ + createHandledToolItem.setLabel("Validate Tech Type Table"); + createHandledToolItem.setIconURI("platform:/plugin/com.famfamfam.silk/icons/accept.png"); //$NON-NLS-1$ + return createHandledToolItem; + } + private MHandledToolItem createImportCSVDataToolItem(MApplication app) { MHandledToolItem createHandledToolItem = MMenuFactory.INSTANCE.createHandledToolItem(); // Command is contributed via fragment @@ -65,10 +76,6 @@ public class TechTypeTableView { .filter(r -> r.getName().toLowerCase().contains("pipe")) .map(r -> r.getResource()) .findFirst().orElse(null); - - if (pipe == null) { - pipe = Simantics.getSession().syncRequest(new PossibleResource("http://DistrictComponents@C/dh_pipe@1")); - } } catch (DatabaseException e) { LOGGER.error("Failed to read district component types for active model", e); } @@ -76,15 +83,20 @@ public class TechTypeTableView { LOGGER.debug("Pipe component type is {}", pipe); String data = null; - try { - Resource model = Simantics.getSession().syncRequest(new PossibleActiveModel(Simantics.getProjectResource())); - if (model != null) - data = Simantics.getSession().syncRequest(new PossibleTechTypeTableData(model, pipe), TransientCacheListener.instance()); - } catch (DatabaseException e) { - LOGGER.error("Failed to read tech type table data for {}", pipe, e); + Resource tableResource = null; + if (pipe != null) { + try { + Resource model = Simantics.getSession().syncRequest(new PossibleActiveModel(Simantics.getProjectResource())); + if (model != null) { + tableResource = Simantics.getSession().syncRequest(new PossibleTechTypeTable(model, pipe), TransientCacheListener.instance()); + data = Simantics.getSession().syncRequest(new PossibleTechTypeTableData(model, pipe), TransientCacheListener.instance()); + } + } catch (DatabaseException e) { + LOGGER.error("Failed to read tech type table data for {}", pipe, e); + } } - table = new TechTypeTable(parent, 0, pipe, data); + table = new TechTypeTable(parent, 0, pipe, tableResource, data); } @PreDestroy diff --git a/org.simantics.district.network.ui/src/org/simantics/district/network/ui/techtype/table/ValidateTechTypeTableHandler.java b/org.simantics.district.network.ui/src/org/simantics/district/network/ui/techtype/table/ValidateTechTypeTableHandler.java new file mode 100644 index 00000000..8438fd7b --- /dev/null +++ b/org.simantics.district.network.ui/src/org/simantics/district/network/ui/techtype/table/ValidateTechTypeTableHandler.java @@ -0,0 +1,42 @@ +package org.simantics.district.network.ui.techtype.table; + +import java.util.Set; + +import javax.inject.Named; + +import org.eclipse.e4.core.di.annotations.CanExecute; +import org.eclipse.e4.core.di.annotations.Execute; +import org.eclipse.e4.ui.services.IServiceConstants; +import org.eclipse.jface.dialogs.MessageDialog; +import org.eclipse.swt.widgets.Shell; +import org.simantics.db.Resource; +import org.simantics.db.exception.DatabaseException; +import org.simantics.district.network.techtype.TechTypeValidationUtils; +import org.simantics.utils.ui.ExceptionUtils; + +public class ValidateTechTypeTableHandler { + @CanExecute + public boolean canExecute() { + return TechTypeTableView.table.getCurrentTable() != null; + } + + @Execute + public void execute(@Named(IServiceConstants.ACTIVE_SHELL) Shell s) { + Resource table = TechTypeTableView.table.getCurrentTable(); + if (table == null) + return; + + Set result; + try { + result = TechTypeValidationUtils.validateTechTypeTable(table); + } catch (DatabaseException e) { + ExceptionUtils.logAndShowError("Tech type table validation failed", e); + return; + } + + TechTypeTableView.table.setValidationResult(result); + + MessageDialog.openInformation(s, "Validation result", result.isEmpty() ? + "No invalid values found" : result.size() + " invalid values found"); + } +} diff --git a/org.simantics.district.network/src/org/simantics/district/network/techtype/TechTypeValidationUtils.java b/org.simantics.district.network/src/org/simantics/district/network/techtype/TechTypeValidationUtils.java new file mode 100644 index 00000000..a82f65a6 --- /dev/null +++ b/org.simantics.district.network/src/org/simantics/district/network/techtype/TechTypeValidationUtils.java @@ -0,0 +1,132 @@ +package org.simantics.district.network.techtype; + +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import org.simantics.Simantics; +import org.simantics.databoard.binding.NumberBinding; +import org.simantics.databoard.binding.error.BindingException; +import org.simantics.databoard.type.Datatype; +import org.simantics.databoard.type.NumberType; +import org.simantics.databoard.util.Range; +import org.simantics.databoard.util.RangeException; +import org.simantics.db.ReadGraph; +import org.simantics.db.Resource; +import org.simantics.db.common.procedure.adapter.TransientCacheListener; +import org.simantics.db.common.request.ResourceRead; +import org.simantics.db.common.request.UniqueRead; +import org.simantics.db.exception.DatabaseException; +import org.simantics.db.layer0.request.PropertyInfo; +import org.simantics.db.layer0.request.PropertyInfoRequest; +import org.simantics.district.network.ontology.DistrictNetworkResource; +import org.simantics.district.network.techtype.requests.TechTypeTableData; +import org.simantics.layer0.Layer0; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class TechTypeValidationUtils { + private static final Logger LOGGER = LoggerFactory.getLogger(TechTypeValidationUtils.class); + + /** + * Return a set of invalid tech type table entries. + * + * The tech type table values are validated against any limits that have been + * defined for associated component type properties. + * + * Invalid entries are designated by strings of the form "/". + * + * @param table A tech type table resource + * @return A set of labels for invalid values + * @throws DatabaseException + */ + public static Set validateTechTypeTable(Resource table) throws DatabaseException { + LOGGER.trace("Validating resource table {}", table); + + // Use a unique read - we don't want to pollute the cache with this + return Simantics.getSession().syncRequest(new UniqueRead>() { + @Override + public Set perform(ReadGraph graph) throws DatabaseException { + Resource type = graph.getPossibleObject(table, DistrictNetworkResource.getInstance(graph).TechType_TechTypeTable_HasComponentType); + if (type == null) + return Collections.emptySet(); + + Map props = graph.syncRequest(new PropertyInfoMapOfType(type), TransientCacheListener.instance()); + Set result = new HashSet<>(); + + Map> data = graph.syncRequest(new TechTypeTableData(table), TransientCacheListener.instance()); + for (String code : data.keySet()) { + LOGGER.trace(" type code {}", code); + Map values = data.get(code); + for (String propertyName : values.keySet()) { + PropertyInfo info = props.get(propertyName); + + // Allow property names to start with an underscore + if (info == null) + info = props.get("_" + propertyName); + + if (info == null) { + LOGGER.trace(" {} - no property", propertyName); + continue; + } + + Datatype dt = info.requiredDatatype; + if (dt == null || !(dt instanceof NumberType)) + continue; // Only check ranges of numerical properties + + String range = dt.metadata.get("range"); + if (range != null) { + Range rng; + try { + rng = Range.valueOf(range); + } catch (RangeException e1) { + LOGGER.error("Invalid range string {} for property {}", range, propertyName, e1); + continue; + } + + String valueString = values.get(propertyName).replace(",", "."); + try { + Double num = Double.valueOf(valueString); + NumberBinding binding = (NumberBinding)info.defaultBinding; + Number value = (Number) binding.create(num); + + if (!rng.contains(value)) { + LOGGER.trace(" {} - range violation {} / {}", propertyName, valueString, range); + result.add(code + "/" + propertyName); + } else { + LOGGER.trace(" {} - valid value {} / {}", propertyName, valueString, range); + } + } catch (NumberFormatException e) { + // Nothing to do here, treat non-numeric strings as missing values + LOGGER.trace(" {} - no value {} / {}", propertyName, valueString, range); + } catch (BindingException e) { + LOGGER.error("Binding error for property {}", propertyName, e); + } + } + } + } + + return result; + } + }); + } + + private static class PropertyInfoMapOfType extends ResourceRead> { + protected PropertyInfoMapOfType(Resource type) { + super(type); + } + + @Override + public Map perform(ReadGraph graph) throws DatabaseException { + Map result = new HashMap(); + for (Resource prop : graph.getObjects(resource, Layer0.getInstance(graph).DomainOf)) { + PropertyInfo info = graph.syncRequest(new PropertyInfoRequest(prop), TransientCacheListener.instance()); + result.put(info.name, info); + } + + return result; + } + } +} diff --git a/org.simantics.district.network/src/org/simantics/district/network/techtype/requests/PossibleTechTypeKeyName.java b/org.simantics.district.network/src/org/simantics/district/network/techtype/requests/PossibleTechTypeKeyName.java new file mode 100644 index 00000000..bd4b7216 --- /dev/null +++ b/org.simantics.district.network/src/org/simantics/district/network/techtype/requests/PossibleTechTypeKeyName.java @@ -0,0 +1,29 @@ +package org.simantics.district.network.techtype.requests; + +import org.simantics.db.ReadGraph; +import org.simantics.db.Resource; +import org.simantics.db.common.request.ResourceRead; +import org.simantics.db.exception.DatabaseException; +import org.simantics.district.network.ontology.DistrictNetworkResource; +import org.simantics.layer0.Layer0; + +public class PossibleTechTypeKeyName extends ResourceRead { + public PossibleTechTypeKeyName(Resource componentType) { + super(componentType); + } + + @Override + public String perform(ReadGraph graph) throws DatabaseException { + Layer0 L0 = Layer0.getInstance(graph); + DistrictNetworkResource DN = DistrictNetworkResource.getInstance(graph); + + for (Resource r : graph.getObjects(resource, L0.DomainOf)) { + Resource accessor = graph.getPossibleObject(r, L0.valueAccessor); + if (accessor.equals(DN.TechType_Functions_techTypeCodeValueAccessor)) { + return graph.getRelatedValue2(r, L0.HasName); + } + } + + return null; + } +} diff --git a/org.simantics.district.network/src/org/simantics/district/network/techtype/requests/TechTypeTableData.java b/org.simantics.district.network/src/org/simantics/district/network/techtype/requests/TechTypeTableData.java index 2ff575b7..4afe5bde 100644 --- a/org.simantics.district.network/src/org/simantics/district/network/techtype/requests/TechTypeTableData.java +++ b/org.simantics.district.network/src/org/simantics/district/network/techtype/requests/TechTypeTableData.java @@ -25,7 +25,7 @@ public class TechTypeTableData extends ResourceRead { @@ -15,19 +15,9 @@ public class TechTypeTableKeyName extends ResourceRead { @Override public String perform(ReadGraph graph) throws DatabaseException { - Layer0 L0 = Layer0.getInstance(graph); DistrictNetworkResource DN = DistrictNetworkResource.getInstance(graph); Resource type = graph.getPossibleObject(resource, DN.TechType_TechTypeTable_HasComponentType); - if (type != null) { - for (Resource r : graph.getObjects(type, L0.DomainOf)) { - Resource accessor = graph.getPossibleObject(r, L0.valueAccessor); - if (accessor.equals(DN.TechType_Functions_techTypeCodeValueAccessor)) { - return graph.getRelatedValue2(r, L0.HasName); - } - } - } - - return null; + return type != null ? graph.syncRequest(new PossibleTechTypeKeyName(type), TransientCacheListener.instance()) : null; } } \ No newline at end of file -- 2.47.1