search

vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
618 stars 167 forks source link

Binder throws NPE when field was bound twice #18702

Closed davidmllr closed 8 months ago

davidmllr commented 9 months ago

Description of the bug

After upgrading from Vaadin 23 to Vaadin 24 our application suddenly starts to throw an NPE when interacting with a field that was bound twice (meaning Binder::bind has been called multiple times).

This is the exception

java.lang.NullPointerException: Cannot invoke "com.vaadin.flow.data.binder.BindingValidationStatus.getBinding()" because "s" is null
    at com.vaadin.flow.data.binder.BinderValidationStatus.lambda$notifyBindingValidationStatusHandlers$1(BinderValidationStatus.java:213) ~[flow-data-24.3.5.jar:24.3.5]
    at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184) ~[na:na]
    at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179) ~[na:na]
    at java.base/java.util.Collections$2.tryAdvance(Collections.java:5073) ~[na:na]
    at java.base/java.util.Collections$2.forEachRemaining(Collections.java:5081) ~[na:na]
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[na:na]
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[na:na]
    at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151) ~[na:na]
    at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174) ~[na:na]
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[na:na]
    at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) ~[na:na]
    at com.vaadin.flow.data.binder.BinderValidationStatus.notifyBindingValidationStatusHandlers(BinderValidationStatus.java:213) ~[flow-data-24.3.5.jar:24.3.5]
    at com.vaadin.flow.data.binder.BinderValidationStatus.notifyBindingValidationStatusHandlers(BinderValidationStatus.java:197) ~[flow-data-24.3.5.jar:24.3.5]
    at com.vaadin.flow.data.binder.Binder.handleBinderValidationStatus(Binder.java:3032) ~[flow-data-24.3.5.jar:24.3.5]
    at com.vaadin.flow.data.binder.Binder.validate(Binder.java:2641) ~[flow-data-24.3.5.jar:24.3.5]
    at com.vaadin.flow.data.binder.Binder.handleFieldValueChange(Binder.java:1839) ~[flow-data-24.3.5.jar:24.3.5]
    at com.vaadin.flow.data.binder.Binder$BindingImpl.handleFieldValueChange(Binder.java:1435) ~[flow-data-24.3.5.jar:24.3.5]
    at com.vaadin.flow.data.binder.Binder$BindingImpl.lambda$new$f9b94f89$1(Binder.java:1251) ~[flow-data-24.3.5.jar:24.3.5]
    at com.vaadin.flow.component.internal.AbstractFieldSupport.lambda$addValueChangeListener$828eca10$1(AbstractFieldSupport.java:98) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.component.ComponentEventBus.fireEventForListener(ComponentEventBus.java:239) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.component.ComponentEventBus.fireEvent(ComponentEventBus.java:228) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.component.Component.fireEvent(Component.java:411) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.component.ComponentUtil.fireEvent(ComponentUtil.java:416) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.component.internal.AbstractFieldSupport.setValue(AbstractFieldSupport.java:209) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.component.internal.AbstractFieldSupport.setModelValue(AbstractFieldSupport.java:169) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.component.AbstractField.setModelValue(AbstractField.java:225) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.component.AbstractSinglePropertyField.doSetModelValue(AbstractSinglePropertyField.java:360) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.component.AbstractSinglePropertyField.handlePropertyChange(AbstractSinglePropertyField.java:352) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.component.AbstractSinglePropertyField$1.propertyChange(AbstractSinglePropertyField.java:329) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.internal.nodefeature.ElementPropertyMap.lambda$fireEvent$2(ElementPropertyMap.java:465) ~[flow-server-24.3.5.jar:24.3.5]
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) ~[na:na]
    at com.vaadin.flow.internal.nodefeature.ElementPropertyMap.fireEvent(ElementPropertyMap.java:465) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.internal.nodefeature.ElementPropertyMap$PutResult.run(ElementPropertyMap.java:170) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.server.communication.ServerRpcHandler.lambda$handleInvocations$1(ServerRpcHandler.java:436) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.server.communication.ServerRpcHandler.runMapSyncTask(ServerRpcHandler.java:452) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.server.communication.ServerRpcHandler.lambda$handleInvocations$3(ServerRpcHandler.java:446) ~[flow-server-24.3.5.jar:24.3.5]
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1596) ~[na:na]
    at com.vaadin.flow.server.communication.ServerRpcHandler.handleInvocations(ServerRpcHandler.java:446) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.server.communication.ServerRpcHandler.handleRpc(ServerRpcHandler.java:324) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.server.communication.UidlRequestHandler.synchronizedHandleRequest(UidlRequestHandler.java:114) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.server.SynchronizedRequestHandler.handleRequest(SynchronizedRequestHandler.java:40) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.server.VaadinService.handleRequest(VaadinService.java:1574) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.server.VaadinServlet.service(VaadinServlet.java:398) ~[flow-server-24.3.5.jar:24.3.5]
    at com.vaadin.flow.spring.SpringServlet.service(SpringServlet.java:106) ~[vaadin-spring-24.3.5.jar:na]
    at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:658) ~[tomcat-embed-core-10.1.18.jar:6.0]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:205) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.ApplicationDispatcher.invoke(ApplicationDispatcher.java:642) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.ApplicationDispatcher.processRequest(ApplicationDispatcher.java:408) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.ApplicationDispatcher.doForward(ApplicationDispatcher.java:313) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.ApplicationDispatcher.forward(ApplicationDispatcher.java:277) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.springframework.web.servlet.mvc.ServletForwardingController.handleRequestInternal(ServletForwardingController.java:141) ~[spring-webmvc-6.1.3.jar:6.1.3]
    at org.springframework.web.servlet.mvc.AbstractController.handleRequest(AbstractController.java:178) ~[spring-webmvc-6.1.3.jar:6.1.3]
    at org.springframework.web.servlet.mvc.SimpleControllerHandlerAdapter.handle(SimpleControllerHandlerAdapter.java:51) ~[spring-webmvc-6.1.3.jar:6.1.3]
    at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1089) ~[spring-webmvc-6.1.3.jar:6.1.3]
    at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:979) ~[spring-webmvc-6.1.3.jar:6.1.3]
    at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1014) ~[spring-webmvc-6.1.3.jar:6.1.3]
    at org.springframework.web.servlet.FrameworkServlet.doPost(FrameworkServlet.java:914) ~[spring-webmvc-6.1.3.jar:6.1.3]
    at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:590) ~[tomcat-embed-core-10.1.18.jar:6.0]
    at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:885) ~[spring-webmvc-6.1.3.jar:6.1.3]
    at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:658) ~[tomcat-embed-core-10.1.18.jar:6.0]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:205) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:51) ~[tomcat-embed-websocket-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100) ~[spring-web-6.1.3.jar:6.1.3]
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116) ~[spring-web-6.1.3.jar:6.1.3]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93) ~[spring-web-6.1.3.jar:6.1.3]
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116) ~[spring-web-6.1.3.jar:6.1.3]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201) ~[spring-web-6.1.3.jar:6.1.3]
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116) ~[spring-web-6.1.3.jar:6.1.3]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:174) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:149) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:167) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:90) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:482) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:115) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:93) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:340) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:391) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:63) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:896) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1744) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:52) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1191) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61) ~[tomcat-embed-core-10.1.18.jar:10.1.18]
    at java.base/java.lang.Thread.run(Thread.java:1583) ~[na:na]

Expected behavior

There is no exception. Instead, the last binding should be applied (I guess?).

Minimal reproducible example

The code below generates a view that contains only one text field. After inputting text in the field and blurring the element (e.g. by pressing TAB), the exception described above comes up.

@SpringBootApplication
@Theme(value = "my-app")
public class Application implements AppShellConfigurator {
    public static void main(String[] args) {
        SpringApplication.run(Application.class, args);
    }

    @Route(value = "/")
    public static class FormView extends Composite<VerticalLayout> {
        public FormView() {
            var textField = new TextField();
            var binder = new Binder<String>();
            binder.forField(textField).bind(ValueProvider.identity(), (item, value) -> {});
            binder.forField(textField).bind(str -> "bla", (item, value) -> out.println("nothing"));
            getContent().add(textField);
        }
    }
}

Versions

Flow: 24.3.5 Vaadin: 24.3.5 Java: Eclipse Adoptium 21.0.2 OS: aarch64 Mac OS X 13.4.1 Browser: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/121.0.0.0 Safari/537.36 Live reload: Java active (Spring Boot Devtools): Front end active

Legioth commented 9 months ago

Sounds to me like binding multiple times is a programming eror and a runtime exception is thus acceptable. The regression is on the other hand potentially problematic.

What use case do you have for binding multiple times? Would it make sense to explicitly require that this kind of situation is "acknowledged" in code by calling removeBinding before binding again?

davidmllr commented 9 months ago

Thanks for the quick response.

The NPE popped up while testing the upgrade to 24 and I had to look around a bit to find out where it was coming from. Turns out we accidentally had duplicated code in one of our views (containing the binding) which did not throw an exception before.

I think your suggestion of acknowledging the double binding beforehand is great because as a developer I don't really know upfront that using Binder::bind multiple times is discouraged.

mshabarov commented 8 months ago

Second binding should probably override the first one without any empty binding leftovers, or at least throw a runtime exception with meaningful message.

vaadin-bot commented 8 months ago

This ticket/PR has been released with Vaadin 24.3.6.

vaadin-bot commented 8 months ago

This ticket/PR has been released with Vaadin 24.2.11.