vaadin / flow

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

Opening grid editor fails when a non-edited ComponentRenderer column returns null for that row #19629

Open guttormvik2 opened 4 days ago

guttormvik2 commented 4 days ago

Describe the bug

I have a ComponentRenderer column that returns either a VaadinIcon.CLOCK.create(), a Div with some text, or null. When I click on a row (which also opens the editor) where this has value, everything is fine. When I click on a row where this is null, I get

13:43:55,264 ERROR [com.ptsmc.vaadin.CustomErrorHandler] (default task-26) Logged error: java.lang.NullPointerException: Cannot invoke "com.vaadin.flow.component.Component.getElement()" because "recreatedComponent" is null
    at deployment.ptsmc.ear//com.vaadin.flow.data.provider.AbstractComponentDataGenerator.refreshData(AbstractComponentDataGenerator.java:51)
    at deployment.ptsmc.ear//com.vaadin.flow.data.provider.CompositeDataGenerator.lambda$refreshData$2(CompositeDataGenerator.java:62)
    at java.base/java.lang.Iterable.forEach(Iterable.java:75)
    at deployment.ptsmc.ear//com.vaadin.flow.data.provider.CompositeDataGenerator.refreshData(CompositeDataGenerator.java:62)
    at deployment.ptsmc.ear//com.vaadin.flow.data.provider.CompositeDataGenerator.lambda$refreshData$2(CompositeDataGenerator.java:62)
    at java.base/java.lang.Iterable.forEach(Iterable.java:75)
    at deployment.ptsmc.ear//com.vaadin.flow.data.provider.CompositeDataGenerator.refreshData(CompositeDataGenerator.java:62)
    at deployment.ptsmc.ear//com.vaadin.flow.data.provider.DataCommunicator.refresh(DataCommunicator.java:405)
    at deployment.ptsmc.ear//com.vaadin.flow.component.grid.Grid$AbstractGridExtension.refresh(Grid.java:1213)
    at deployment.ptsmc.ear//com.vaadin.flow.component.grid.editor.EditorImpl.requestEditItem(EditorImpl.java:152)
    at deployment.ptsmc.ear//com.vaadin.flow.component.grid.editor.EditorImpl.lambda$editItem$2d5ea2ce$1(EditorImpl.java:137)
    at deployment.ptsmc.ear//com.vaadin.flow.internal.StateTree.lambda$runExecutionsBeforeClientResponse$2(StateTree.java:397)
    at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
    at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
    at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
    at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
    at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
    at deployment.ptsmc.ear//com.vaadin.flow.internal.StateTree.runExecutionsBeforeClientResponse(StateTree.java:392)
    at deployment.ptsmc.ear//com.vaadin.flow.server.communication.UidlWriter.encodeChanges(UidlWriter.java:394)
    at deployment.ptsmc.ear//com.vaadin.flow.server.communication.UidlWriter.createUidl(UidlWriter.java:170)
    at deployment.ptsmc.ear//com.vaadin.flow.server.communication.UidlRequestHandler.createUidl(UidlRequestHandler.java:155)
    at deployment.ptsmc.ear//com.vaadin.flow.server.communication.UidlRequestHandler.writeUidl(UidlRequestHandler.java:145)
    at deployment.ptsmc.ear//com.vaadin.flow.server.communication.UidlRequestHandler.synchronizedHandleRequest(UidlRequestHandler.java:115)
    at deployment.ptsmc.ear//com.vaadin.flow.server.SynchronizedRequestHandler.handleRequest(SynchronizedRequestHandler.java:40)
    at deployment.ptsmc.ear//com.vaadin.flow.server.VaadinService.handleRequest(VaadinService.java:1574)
    at deployment.ptsmc.ear//com.vaadin.flow.server.VaadinServlet.service(VaadinServlet.java:398)
    at jakarta.servlet.api@6.0.0//jakarta.servlet.http.HttpServlet.service(HttpServlet.java:614)
    ...

When I change the ComponentRenderer to return an empty Div instead of null, it works.

Expected-behavior

No response

Reproduction

See description at top. I assume this is simply missing null-test and that the stacktrace is enough? If not I can try to produce an example

System Info

Windows 10, Vaadin 24.3.12, Firefox 127.0

TatuLund commented 4 days ago

Originally it was requirement that ComponentRenderer must not return null, but it was lifted by this PR: https://github.com/vaadin/flow-components/pull/4361/files

However it seems not to be complete.

Similar fix is needed in here: https://github.com/vaadin/flow/blob/main/flow-data/src/main/java/com/vaadin/flow/data/provider/AbstractComponentDataGenerator.java#L49

Same approach could be used,

if (recreatedComponent == null) {
   recreatedComponent = new Text("");
}

And if the fix is added in AbstractComponentDataGenerator, I would assume the fix in ComponentDataGenerator becomes redundant.

You seem to have found a corner case where flow is not going via ComponentDataGenerator ...