]> gerrit.simantics Code Review - simantics/district.git/commitdiff
More rigorous content validation for element selection dialog 47/4047/1
authorReino Ruusu <reino.ruusu@semantum.fi>
Fri, 27 Mar 2020 11:35:10 +0000 (13:35 +0200)
committerReino Ruusu <reino.ruusu@semantum.fi>
Fri, 27 Mar 2020 11:37:30 +0000 (13:37 +0200)
gitlab #84

Change-Id: Iadb875ea9db3db8350095e21c39ecfad6cc6af36

org.simantics.district.selection.ui/src/org/simantics/district/selection/ui/parts/EditSelectorDialog.java

index a269d5efe3da4cb483f81492e766aacd465929a5..414eb33998e7ada921f6a63de19b9bcc2879d290 100644 (file)
@@ -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;
                        }