Add flag to tell if DetachEvent happened because of resynchronization

[OlliTietavainenVaadin]

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

[Legioth]

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?

[TatuLund]

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?

[mshabarov]

Looks similar to #18414.

[Legioth]

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.

[mbrunenieks-proofit]

This suggested solution would work for #18414

[mperktold]

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.

[Legioth]

@mperktold Did you look into using ui.isClosing() as a way of distinguishing between the different detach reasons?

[mperktold]

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.

[TatuLund]

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");
        }
    }

[mperktold]

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.

[mperktold]

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.

[TatuLund]

@mperktold Thanks for spotting silly mistake, I corrected the code and tested it works with preserve on refresh.

[mperktold]

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.

[TatuLund]

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.

[TatuLund]

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");
        }
    }

[mperktold]

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.

[TatuLund]

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()));