Validation of excluded properties

Question

Validation of excluded properties

Opened this issue 6 years ago · 7 comments

Hi,
my question might be better asked on SO but from my experience FXForm2-related questions on SO do not get a lot of feedback, so I'm asking here.

I would like to add or remove an ExcludeFilter to/from my form depending on the state of a property. This is my sample code:

import com.dooapp.fxform.FXForm;
import com.dooapp.fxform.annotation.Accessor;
import com.dooapp.fxform.filter.ExcludeFilter;

import org.hibernate.validator.constraints.NotBlank;

import javafx.application.Application;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.IntegerProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleIntegerProperty;
import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;
import javafx.scene.Scene;
import javafx.scene.layout.Pane;
import javafx.stage.Stage;

public class FilteredForm extends Application {

    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage primaryStage) {
        MyBean bean = new MyBean();
        FXForm<MyBean> form = new FXForm<>();
        form.setSource(bean);

        Pane root = new Pane(form);
        Scene scene = new Scene(root, 800, 600);
        primaryStage.setScene(scene);
        primaryStage.show();

        ExcludeFilter commentExclusion = new ExcludeFilter("comment");
        bean.commentEnabled.addListener((obs, oldValue, newValue) -> {
            if (newValue) {
                form.getFilters().remove(commentExclusion);
            } else {
                form.addFilters(commentExclusion);
            }
        });
    }

    @Accessor(value = Accessor.AccessType.METHOD)
    public static class MyBean {

        private BooleanProperty commentEnabled = new SimpleBooleanProperty(this, "commentEnabled", true);
        private IntegerProperty priority = new SimpleIntegerProperty(this, "priority", 1);
        private StringProperty comment = new SimpleStringProperty(this, "comment", "");

        public boolean isCommentEnabled() {
            return commentEnabled.get();
        }

        public BooleanProperty commentEnabledProperty() {
            return commentEnabled;
        }

        public int getPriority() {
            return priority.get();
        }

        public void setPriority(int priority) {
            this.priority.set(priority);
        }

        public IntegerProperty priorityProperty() {
            return priority;
        }

        @NotBlank
        public String getComment() {
            return comment.get();
        }

        public void setComment(String comment) {
            this.comment.set(comment);
        }

        public StringProperty commentProperty() {
            return comment;
        }
    }

}

When I run this application, the commentEnabled check box will be selected and the comment text field is shown. When I unselect, the comment text field is gone. When I select again, I see the following null pointer exception:

Exception in thread "JavaFX Application Thread" java.lang.NullPointerException
	at com.dooapp.fxform.controller.PropertyEditorController.updateView(PropertyEditorController.java:112)
	at com.dooapp.fxform.controller.PropertyEditorController.access$300(PropertyEditorController.java:38)
	at com.dooapp.fxform.controller.PropertyEditorController$2.changed(PropertyEditorController.java:97)
	at com.sun.javafx.binding.ExpressionHelper$Generic.fireValueChangedEvent(ExpressionHelper.java:361)
	at com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:81)
	at javafx.beans.property.BooleanPropertyBase.fireValueChangedEvent(BooleanPropertyBase.java:103)
	at javafx.beans.property.BooleanPropertyBase.markInvalid(BooleanPropertyBase.java:110)
	at javafx.beans.property.BooleanPropertyBase.set(BooleanPropertyBase.java:144)
	at javafx.beans.property.BooleanProperty.setValue(BooleanProperty.java:81)
	at javafx.beans.property.BooleanPropertyBase.setValue(BooleanPropertyBase.java:49)
	at com.dooapp.fxform.model.impl.PropertyMethodElement.setValue(PropertyMethodElement.java:35)
	at com.dooapp.fxform.controller.PropertyEditorController$1.changed(PropertyEditorController.java:81)
	at com.sun.javafx.binding.ExpressionHelper$SingleChange.fireValueChangedEvent(ExpressionHelper.java:182)
	at com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:81)
	at javafx.beans.property.BooleanPropertyBase.fireValueChangedEvent(BooleanPropertyBase.java:103)
	at javafx.beans.property.BooleanPropertyBase.markInvalid(BooleanPropertyBase.java:110)
	at javafx.beans.property.BooleanPropertyBase.set(BooleanPropertyBase.java:144)
	at javafx.scene.control.CheckBox.setSelected(CheckBox.java:156)
	at javafx.scene.control.CheckBox.fire(CheckBox.java:236)
	at com.sun.javafx.scene.control.behavior.ButtonBehavior.mouseReleased(ButtonBehavior.java:182)
	at com.sun.javafx.scene.control.skin.BehaviorSkinBase$1.handle(BehaviorSkinBase.java:96)
	at com.sun.javafx.scene.control.skin.BehaviorSkinBase$1.handle(BehaviorSkinBase.java:89)
	at com.sun.javafx.event.CompositeEventHandler$NormalEventHandlerRecord.handleBubblingEvent(CompositeEventHandler.java:218)
	at com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:80)
	at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:238)
	at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:191)
	at com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
	at com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:54)
	at javafx.event.Event.fireEvent(Event.java:198)
	at javafx.scene.Scene$MouseHandler.process(Scene.java:3757)
	at javafx.scene.Scene$MouseHandler.access$1500(Scene.java:3485)
	at javafx.scene.Scene.impl_processMouseEvent(Scene.java:1762)
	at javafx.scene.Scene$ScenePeerListener.mouseEvent(Scene.java:2494)
	at com.sun.javafx.tk.quantum.GlassViewEventHandler$MouseEventNotification.run(GlassViewEventHandler.java:394)
	at com.sun.javafx.tk.quantum.GlassViewEventHandler$MouseEventNotification.run(GlassViewEventHandler.java:295)
	at java.security.AccessController.doPrivileged(Native Method)
	at com.sun.javafx.tk.quantum.GlassViewEventHandler.lambda$handleMouseEvent$353(GlassViewEventHandler.java:432)
	at com.sun.javafx.tk.quantum.QuantumToolkit.runWithoutRenderLock(QuantumToolkit.java:389)
	at com.sun.javafx.tk.quantum.GlassViewEventHandler.handleMouseEvent(GlassViewEventHandler.java:431)
	at com.sun.glass.ui.View.handleMouseEvent(View.java:555)
	at com.sun.glass.ui.View.notifyMouse(View.java:937)
	at com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
	at com.sun.glass.ui.win.WinApplication.lambda$null$147(WinApplication.java:177)
	at java.lang.Thread.run(Thread.java:748)

I can ignore the exception, but there is another issue (or feature?): the comment property is validated although it is not shown in the form. If I leave the comment field empty (and the check box unselected), the form's list of constraint violations contains the 'not blank' violation from the comment property, and form.isValid() returns false. This is because the validation takes place on the bean, not on the form, I assume, but I find this slightly confusing as the form itself looks valid to the user when its internal state is not.

I think it is possible to filter the constraint violations accordingly such that the form internal validation's state would match the state presented in the GUI, but I'm wondering if this would be the correct approach. So my questions are:

Is the existing behaviour the intended behaviour?
Is using a (dynamic) exclusion filter the right approach for this use case? (the NPE might suggest otherwise)
Could filters be taken into account by FXForm when validating form elements? This might require FXForm to trigger a re-validation of all properties when one property changes as we do not know which property had a new filter attached.

I'd appreciate any feedback or hints to things I have overlooked or misunderstood. Thank you.

Answer 1 · 2018-12-14T11:36:54.000Z

The exception you get is due to the fact to you are modifying the form structure (by modifying the filters) while the form is currently being updated (since a form property is modified).
We could try to better handle this case in the form, but it might lead to unexpected behavior. I would rather suggest to make sure that the form is modified after any property value changes have been processed, by using a Platform.runLater call :

bean.commentEnabled.addListener((obs, oldValue, newValue) -> {
        if (newValue) {
            Platform.runLater(() -> form.getFilters().remove(commentExclusion));
        } else {
            Platform.runLater(() -> form.addFilters(commentExclusion));
        }
});

Your example might be helpful to other users, I have added it to the samples modules of FXForm2.

Regarding the validation of a filtered property, it's an interesting question. It should be possible to filter these properties from the form validation, I mark this ticket as improvement.

Answer 2 · 2018-12-14T13:20:19.000Z

Thanks for your reply and for considering this feature as an improvement. My use case would benefit from it.

I can confirm the NPE vanishes when you wrap the adding and removing of the filter in a Platform.runLater() call.

One more thing: when I start the application, leave the comment field empty and toggle the check box multiple times, the list of constraint violations grows by one with each checking of the box. That looks like unintended behaviour.

Answer 3 · 2019-01-02T16:49:05.000Z

After digging more on this, it appears that it looks more like a bug indeed.

The contraint violations of an excluded field should not appear in the form constraint violations (which is the case if you set the exclusion filter directly)
The bug is due to the fact that the constraint violations are not being updated when the filters are modified, the violations of the removed field should be cleaned.

I have added a unit test + fix, let me know if this fixes your case.

Answer 4 · 2019-01-02T16:52:44.000Z

Fix deployed in 8.3.0-SNAPSHOT

Answer 5 · 2019-01-10T08:29:42.000Z

I can confirm this fixes the issue I saw in the above example. Thanks a lot for fixing this bug and for your efforts in general.

Answer 6 · 2019-04-26T14:16:16.000Z

Sorry to bring this up again after such a long time, but I think I found another issues with these exclude filters.

Assume I use the same form as in my initial posting, but with a custom skin like this:

import java.net.URL;

import javax.validation.constraints.Min;

import com.dooapp.fxform.FXForm;
import com.dooapp.fxform.annotation.Accessor;
import com.dooapp.fxform.filter.ExcludeFilter;
import com.dooapp.fxform.view.control.ConstraintLabel;
import com.dooapp.fxform.view.property.DefaultPropertyProvider;
import com.dooapp.fxform.view.property.PropertyProvider;
import com.dooapp.fxform.view.skin.FXMLSkin;

import org.hibernate.validator.constraints.NotBlank;

import javafx.application.Application;
import javafx.application.Platform;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.IntegerProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleIntegerProperty;
import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;
import javafx.scene.Scene;
import javafx.scene.layout.Pane;
import javafx.stage.Stage;

public class FilteredForm extends Application {

    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage primaryStage) {
        MyBean bean = new MyBean();
        FXForm<MyBean> form = new FXForm<>();
        URL fxmlUrl = this.getClass().getResource("FilteredFormEditor.fxml");
        FilteredFormEditorController controller = new FilteredFormEditorController();
        form.setSkin(new FXMLSkin(form, fxmlUrl, controller));
        DefaultPropertyProvider propertyProvider = new DefaultPropertyProvider() {
            {
                map.put(ConstraintLabel.class, (PropertyProvider<ConstraintLabel>) ConstraintLabel::constraintProperty);
            }
        };
        form.setPropertyProvider(propertyProvider);
        form.setSource(bean);

        Pane root = new Pane(form);
        Scene scene = new Scene(root, 800, 600);
        primaryStage.setScene(scene);
        primaryStage.show();

        ExcludeFilter commentExclusion = new ExcludeFilter("comment");
        ExcludeFilter priorityExclusion = new ExcludeFilter("priority");

        bean.commentEnabled.addListener((obs, oldValue, newValue) -> {
            if (newValue) {
                Platform.runLater(() -> {
                    form.getFilters().remove(commentExclusion);
                    form.addFilters(priorityExclusion);
                });
            } else {
                Platform.runLater(() -> {
                    form.addFilters(commentExclusion);
                    form.getFilters().remove(priorityExclusion);
                });
            }
        });
    }

    @Accessor(value = Accessor.AccessType.METHOD)
    public static class MyBean {

        private BooleanProperty commentEnabled = new SimpleBooleanProperty(this, "commentEnabled", true);
        private IntegerProperty priority = new SimpleIntegerProperty(this, "priority", -1);
        private StringProperty comment = new SimpleStringProperty(this, "comment", "");

        public boolean isCommentEnabled() {
            return commentEnabled.get();
        }

        public BooleanProperty commentEnabledProperty() {
            return commentEnabled;
        }

        @Min(0)
        public int getPriority() {
            return priority.get();
        }

        public void setPriority(int priority) {
            this.priority.set(priority);
        }

        public IntegerProperty priorityProperty() {
            return priority;
        }

        @NotBlank
        public String getComment() {
            return comment.get();
        }

        public void setComment(String comment) {
            this.comment.set(comment);
        }

        public StringProperty commentProperty() {
            return comment;
        }
    }

}

The FXML looks like this:

<?xml version="1.0" encoding="UTF-8"?>

<?import com.dooapp.fxform.view.control.ConstraintLabel?>
<?import javafx.scene.control.CheckBox?>
<?import javafx.scene.control.Label?>
<?import javafx.scene.control.TextField?>
<?import javafx.scene.layout.StackPane?>
<?import javafx.scene.layout.VBox?>
<StackPane xmlns:fx="http://javafx.com/fxml/1" xmlns="http://javafx.com/javafx/8.0.172" fx:id="root">
    <VBox>
        <children>
            <VBox>
                <Label fx:id="commentEnabledLabel" id="commentEnabled-form-label"/>
                <CheckBox fx:id="commentEnabledCheckBox" id="commentEnabled-form-editor"/>
            </VBox>
            <VBox>
                <Label fx:id="priorityLabel" id="priority-form-label"/>
                <TextField fx:id="priorityTextField" id="priority-form-editor"/>
                <ConstraintLabel id="priority-form-constraint"/>
            </VBox>
            <VBox fx:id="commentBox">
                <Label fx:id="commentLabel" id="comment-form-label"/>
                <TextField fx:id="commentTextField" id="comment-form-editor"/>
                <ConstraintLabel id="comment-form-constraint"/>
            </VBox>
        </children>
    </VBox>
</StackPane>

And this is the controller class:

import java.net.URL;
import java.util.ResourceBundle;

import javafx.fxml.FXML;
import javafx.fxml.Initializable;
import javafx.scene.control.CheckBox;
import javafx.scene.control.Label;
import javafx.scene.control.TextField;
import javafx.scene.layout.StackPane;
import javafx.scene.layout.VBox;

public class FilteredFormEditorController implements Initializable {

    @FXML
    StackPane root;

    @FXML
    Label commentEnabledLabel;

    @FXML
    Label commentLabel;

    @FXML
    Label priorityLabel;

    @FXML
    CheckBox commentEnabledCheckBox;

    @FXML
    TextField commentTextField;

    @FXML
    TextField priorityTextField;

    @FXML
    VBox commentBox;

    @FXML
    public void initialize() {
        commentBox.visibleProperty().bind(commentEnabledCheckBox.selectedProperty());
    }

    @Override
    public void initialize(URL location, ResourceBundle resources) {
        initialize();
    }
}

When I run this application, I see a selected check box and two text fields with invalid inputs as the initial state. The constraint labels are shown as expected. And when I type '1' into the priority field and 'A' into the comment field, the constraint violations disappear as they should.

But when I deselect the check box and type '-1' into the priority field, no constraint label is shown. Likewise, when I select the check box again and clear the comment field, no constraint label is shown.
Vice versa, when I restart the application, first deselect the check box, and the type '1' into the priority field, the constraint label does not disappear.

Adding the exclude filters dynamically seems to mess up the logic for showing the constraint labels. Their styleclasses are updated correctly when I change the editor input from valid to invalid and vice versa, but I think their constraint property does not receive a change.

If I do not add and remove any exclude filters in this setup, the constraint labels are shown and hidden as expected.

Did I make a mistake in my form configuration or is this an issue with how FXForm handles the exclude filters internally?

Your support would be greatly appreciated. Thank you.

Answer 7 · 2020-03-06T16:46:39.000Z

Reopen the issue, didn't see oova reopened it before