flow
flow copied to clipboard
vaadin
Add flag to tell if DetachEvent happened because of resynchronization
Describe your motivation
In the case of resynchronization, the UI is detached and reattached. This causes the detach events to fire. In use cases where expensive resources are acquired on attach and released on detach, it would be useful to know that the detach happens because of resynchronization, meaning that it can be expected to see reattaching happen soon (and releasing the resources isn't necessary).
Describe the solution you'd like
The DetachEvent could contain a flag (such as resynchronization) that tells if the detach happened due to resynchronization or not.
Describe alternatives you've considered
If this information would be available elsewhere, such as the current Request, that could work too.
Additional context
I don't think "resynchronization" is the correct semantic in this case since the same pattern with detaching and immediately re-attaching a whole component tree does also happen with @PreserveOnRefresh.
Maybe "temporary" or "intermediate" would be more suitable?
There is actually some sort of API for this. When resynchronization is happening, the same UI instance is re-attached, hence UI is not closing. So you can use UI#isClosing() as a flag, i.e. if it is true then detach is not due resynchronization or refresh (in case @PreserceOnRefresh). Do we need additional convenience API or just document this better?
The trick of looking at ui.isClosing() should indeed work in the particular case of the UI which isn't detached for other reasons. The same approach would not work for arbitrary components since they are also detached e.g. when navigating between views. On the other hand, support for arbitrary components might not be super relevant since these types of resources are typically tied to the UI life cycle.
On the other hand, support for arbitrary components might not be super relevant since these types of resources are typically tied to the UI life cycle.
For us it's relevant because even though our resources are tied to the UI life cycle as well, they often only provide an API for releasing them permanently, which means they cannot be reacquired in onAttach afterwards.
We thought this should work as long as we don't reattach a component to the UI after it has been removed, but due to resynchronization, these reattachments can happen out of the blue.
In this sense, some Flag on the detach event would help us the understand whether this is "the final" detach, so we can decide whether to release these resources or not.
@mperktold Did you look into using ui.isClosing() as a way of distinguishing between the different detach reasons?
I haven't actually tried it, but as you said yourself, it doesn't work for components other than the UI itself, since they can be detached when navigating to another view. It's also not possible to somehow propagate the distinction from the UI down to the component tree, since the components are detached bottom up (which makes sense).
We did find another workaround for the NPEs, namely to remove the problematic components from the DOM tree on detach. If the detach is due to resynchronization, these components will only be detached but not reattached, because they won't belong to the DOM anymore. And if the detach is because of some navigation, removing these components won't have any noticable effect.
It's also not possible to somehow propagate the distinction from the UI down to the component tree
@mperktold There is no need, you can get UI via event.
I add here just example code to demonstrate that use of the proposed method is really simple
protected void onDetach(DetachEvent event) {
if (event.getUI().getChildren().count() == 0
&& !event.getUI().isClosing()) {
// Detaching due resync or PreserveOnRefresh
System.out.println("DETACH ON REFRESH");
}
}
But it's the same for normal navigation to other views: the given component will be detached, whereas the UI will not be closing.
You can't differentiate those cases using this pattern, and I would need to differentiate, because for normal navigation (or generally, the "last detach" for this component instance), I need to cleanup resources, whereas for resync I don't.
The only difference between the two scenarios is that for resynchronization, the UI will be detached as well, whereas for normal navigation it won't. This fact doesn't help us, however, since UI will be detached last, so inside onDetach of our component, we don't know if the UI will be detached as well or not.
@mperktold Thanks for spotting silly mistake, I corrected the code and tested it works with preserve on refresh.
Checking the number of children of the UI is an interesting idea.
However, it never returns 0 for me, neither for resync, nor for normal navigation, nor for closing the tab/refresh.
However, it never returns 0 for me, neither for resync,
I did not test the resync case, with PreserveOnRefresh it works
nor for normal navigation, nor for closing the tab/refresh.
In these cases it should not, that was exactly the point.
It seems that when resync is done UI is detached after component is detached, but when preserve on refresh is done, it is not done.
protected void onDetach(DetachEvent event) {
event.getUI().addDetachListener(e -> {
if (!e.getUI().isClosing()) {
System.out.println("DETACH ON RESYNC");
}
});
if (event.getUI().getChildren().count() == 0
&& !event.getUI().isClosing()) {
System.out.println("DETACH ON REFRESH");
}
}
To be fair, I tested resync by faking a resync request in my debugger, i.e. adding resynchronize: true to the JSON payload in a breakpoint.
I don't think the behavior would be different for a "real" resync request, though.
There is method in UI internals to trigger server initiated resync, which we use in our integration testing
Button button = new Button("Resync", e -> getUI()
.ifPresent(ui -> ui.getInternals().incrementServerId()));
If there would be the flag in AttachEvent as well, it would provide alternative approach to show customized error message for the user as described here: https://github.com/vaadin/flow/issues/19804
... protected void onDetach(DetachEvent event) { event.getUI().addDetachListener(e -> { if (!e.getUI().isClosing()) { System.out.println("DETACH ON RESYNC"); } }); ...
With this the recognization of a resync is delayed:
I switched between some views and every time onDetach has been called. This is OK.
Unfortunately the message "DETACH ON RESYNC" comes later, when pressing the button to enforce resync (as you wrote above).
In my application I need to decide immediately in the method onDetach whether to clean up and close resources or to wait (because it's just a resync).
Have a look here: https://stackoverflow.com/questions/78942037/howto-recognize-vaadin-24-resynchronization-in-ondetach-method
@TatuLund : Maybe you could introduce a boolean resync attribute at the DetachEvent and set it in ServerRpcHandler.handleRpc when rpcRequest.isResynchronize() is true?
@fabst2w I read the SO question and based on that the use case is roughly the same I have for example in BeanTable component.
I am using onAttach pared with onDetach instead of constructor
https://github.com/TatuLund/bean-table/blob/v24/src/main/java/org/vaadin/tatu/BeanTable.java#L1226
and
https://github.com/TatuLund/bean-table/blob/v24/src/main/java/org/vaadin/tatu/BeanTable.java#L1236
This way I do not need to care whether detach or attach happens due resync or preserve on refresh. It will work on both cases.
@TatuLund Thanks for your code example. I understand the approach. At the same time, I believe that the effort for a complete and nice refactoring of all components in the application is quite high, which is why I'm shying away from it (right now). Fortunately, the situation has only occurred very rarely so far (primarily with home office users with a poor WLAN connection, approximately once a week), which is why I now implemented this workaround: the system remembers that the components are detached when a 'onDetach' is performed. And with a subsequent 'onAttach', the system displays a message to the user and asks them to refresh the page.