From 739098f00cf61601ffdb1c365a6dee56172acf91 Mon Sep 17 00:00:00 2001 From: Reino Ruusu Date: Fri, 27 Mar 2020 13:35:10 +0200 Subject: [PATCH] More rigorous content validation for element selection dialog gitlab #84 Change-Id: Iadb875ea9db3db8350095e21c39ecfad6cc6af36 --- .../ui/parts/EditSelectorDialog.java | 119 ++++++++++++------ 1 file changed, 78 insertions(+), 41 deletions(-) diff --git a/org.simantics.district.selection.ui/src/org/simantics/district/selection/ui/parts/EditSelectorDialog.java b/org.simantics.district.selection.ui/src/org/simantics/district/selection/ui/parts/EditSelectorDialog.java index a269d5ef..414eb339 100644 --- a/org.simantics.district.selection.ui/src/org/simantics/district/selection/ui/parts/EditSelectorDialog.java +++ b/org.simantics.district.selection.ui/src/org/simantics/district/selection/ui/parts/EditSelectorDialog.java @@ -14,10 +14,8 @@ import java.util.function.Consumer; import javax.inject.Inject; -import org.eclipse.core.runtime.IStatus; -import org.eclipse.core.runtime.Status; import org.eclipse.jface.dialogs.Dialog; -import org.eclipse.jface.dialogs.ErrorDialog; +import org.eclipse.jface.dialogs.MessageDialog; import org.eclipse.jface.layout.GridDataFactory; import org.eclipse.jface.layout.GridLayoutFactory; import org.eclipse.jface.layout.RowDataFactory; @@ -143,10 +141,18 @@ public class EditSelectorDialog extends Dialog { private LocalResourceManager resourceManager; + static class ValidationException extends Exception { + private static final long serialVersionUID = 1L; + + public ValidationException(String message) { + super(message); + } + } + // Function type for updating condition objects with optional validation static interface Updater { // If 'validate' is true, a runtime exception may be thrown for invalid values - void update(boolean validate); + void update(boolean validate) throws ValidationException; } final static Updater NULL_UPDATE = validate -> {}; @@ -294,7 +300,13 @@ public class EditSelectorDialog extends Dialog { } private void updateDialog() { - updater.update(false); + try { + updater.update(false); + } catch (ValidationException e) { + // Should not happend with argument false + assert(false); + } + updater = updateConditionPanel(); content.layout(true, true); @@ -303,38 +315,53 @@ public class EditSelectorDialog extends Dialog { @Override protected void okPressed() { - generatorIndex = sourceField.getSelectionIndex(); - if (generatorIndex == 1) { - int selectionIndex = diagramField.getSelectionIndex(); - if (selectionIndex < 0) { - ErrorDialog.openError(getShell(), "Error", "Please select a diagram", new Status(IStatus.ERROR, "org.simantics.district.selection.ui", "No diagram selected")); - return; + try { + generatorIndex = sourceField.getSelectionIndex(); + if (generatorIndex == 1) { + int selectionIndex = diagramField.getSelectionIndex(); + if (selectionIndex < 0) { + diagramField.setFocus(); + throw new ValidationException("Please select a diagram"); + } + + diagram = diagrams.get(selectionIndex); } - diagram = diagrams.get(selectionIndex); - } - - name = nameField.getText(); - componentType = componentTypes.get(componentTypeField.getSelectionIndex()); - selectorIndex = selectorField.getSelectionIndex(); - int propertyIndex = propertyField.getSelectionIndex(); - propertyName = propertyIndex >= 0 ? propertyNames.get(propertyIndex) : propertyField.getText(); - - // Try to parse number of items - if (useNumberOfItems()) { - try { - numberOfItems = Integer.parseInt(nField.getText()); - } catch (RuntimeException e) { - nField.selectAll(); - nField.forceFocus(); - return; + name = nameField.getText(); + if (name.isEmpty()) { + nameField.setFocus(); + throw new ValidationException("Please enter a name"); } - } - - // To to update condition definitions - try { + + componentType = componentTypes.get(componentTypeField.getSelectionIndex()); + selectorIndex = selectorField.getSelectionIndex(); + int propertyIndex = propertyField.getSelectionIndex(); + propertyName = propertyIndex >= 0 ? propertyNames.get(propertyIndex) : propertyField.getText(); + if (propertyName.isEmpty()) { + propertyField.setFocus(); + throw new ValidationException("Please select a property"); + } + + // Try to parse number of items + if (useNumberOfItems()) { + try { + numberOfItems = Integer.parseInt(nField.getText()); + if (numberOfItems <= 0) { + nField.selectAll(); + nField.setFocus(); + throw new ValidationException("Number of elements must be positive"); + } + } catch (NumberFormatException e) { + nField.selectAll(); + nField.setFocus(); + throw new ValidationException("Please enter a valid number of elements"); + } + } + + // To to update condition definitions updater.update(true); - } catch (RuntimeException e) { + } catch (ValidationException e) { + MessageDialog.openError(this.getShell(), "Missing data", e.getMessage()); return; } @@ -586,7 +613,7 @@ public class EditSelectorDialog extends Dialog { Button notCheck = new Button(parent, SWT.CHECK); GridDataFactory.swtDefaults().align(SWT.BEGINNING, SWT.BEGINNING).applyTo(notCheck); notCheck.setText("not"); - notCheck.setSelection(condition.isInverse); + notCheck.setSelection(condition != null && condition.isInverse); Composite conditionComp = new Composite(parent, SWT.NONE); GridDataFactory.fillDefaults().applyTo(conditionComp); @@ -601,6 +628,8 @@ public class EditSelectorDialog extends Dialog { "Any of" ); + typeCombo.addSelectionListener(new ConditionTypeSelectionListener(typeCombo, consumer, condition)); + final Updater updater; if (condition instanceof PropertyCondition) { typeCombo.select(1); @@ -619,11 +648,9 @@ public class EditSelectorDialog extends Dialog { ROW_LAYOUT.applyTo(conditionComp); notCheck.setEnabled(false); typeCombo.select(0); - updater = validate -> {}; + return NULL_UPDATE; } - typeCombo.addSelectionListener(new ConditionTypeSelectionListener(typeCombo, consumer, condition)); - return validate -> { updater.update(validate); condition.isInverse = notCheck.getSelection(); @@ -695,7 +722,7 @@ public class EditSelectorDialog extends Dialog { addButton.addSelectionListener(new SelectionAdapter() { @Override public void widgetSelected(SelectionEvent e) { - cond.conditions.add(createPropertyCondition("property", null, null)); + cond.conditions.add(createPropertyCondition("", null, null)); updateDialog(); } }); @@ -756,6 +783,11 @@ public class EditSelectorDialog extends Dialog { // Register update return validate -> { int i = routeCombo.getSelectionIndex(); + if (validate && i < 0) { + routeCombo.forceFocus(); + throw new RuntimeException("Must select a route"); + } + condition.routeResource = i >= 0 ? routeResources[i] : null; }; } @@ -776,6 +808,11 @@ public class EditSelectorDialog extends Dialog { // Register update return validate -> { int i = regionCombo.getSelectionIndex(); + if (validate && i < 0) { + regionCombo.forceFocus(); + throw new ValidationException("Please select a region"); + } + condition.regionResource = i >= 0 ? regionResources[i] : null; }; } @@ -813,7 +850,7 @@ public class EditSelectorDialog extends Dialog { if (validate) { lowerLimitText.selectAll(); lowerLimitText.forceFocus(); - throw e; + throw new ValidationException("Please enter a valid lower limit"); } } @@ -824,7 +861,7 @@ public class EditSelectorDialog extends Dialog { if (validate) { upperLimitText.selectAll(); upperLimitText.forceFocus(); - throw e; + throw new ValidationException("Please enter a valid upper limit"); } } @@ -837,7 +874,7 @@ public class EditSelectorDialog extends Dialog { if (validate && name.isEmpty()) { propertyNameText.forceFocus(); - throw new RuntimeException(); + throw new ValidationException("Please select a property"); } else { condition.propertyName = name; } -- 2.45.2