flow icon indicating copy to clipboard operation
flow copied to clipboard

Add access method to Component that delegates Command to be run via right UI

Open TatuLund opened this issue 4 months ago • 0 comments

Describe your motivation

Using UI.access(..) for asynchronous tasks right way in Vaadin requires some ceremony, which might not be obvious. Also matching this with the architecture is not obvious.

It is desirable that in the framework only few very specific methods are thread safe and they are named consistently. Also you would like to avoid setting thread locals in background thread and have UI code there. It would thus be preferable that the Component has uiAccess delegate method, that runs UI.access(..) in correct UI, so that it is possible in to write methods in your Route that can be safely called from the background thread to Push updates to UI.

Describe the solution you'd like

Below is a draft idea how to use proposed access method in View component.

@Route("route)
public class MyRoute extends VerticalLayout() implements AfterNavigationObserver {

   Grid<Data> grid = new Grid<>();

   public MyRoute(MyPresenter presenter) {
      presenter.setView(this);
      ... configure Grid ...
   }
  
    @Override
    public void afterNavigation(AfterNavigationEvent event) {
       presenter.backGroundtask();
    } 

    public void setDataAsync(List<Data> data) {
         uiAccess(() -> {
              grid.setItems(data);
         });
    });
}

Presenter

@RouteScoped
@Component
public class MyPresenter() {

    private MyRoute view;
    private CompletableFuture<Data> future;

    ... autowire backend and executor ...

    public void setView(MyRoute view) {
       this.view = view;
    }

    public void backgroundTask() {
        future = CompletableFuture.supplyAsync(this::fetchData, executor);
        future.thenAccept(data -> {
           view.setDataAsync(data);
        });
    }

    private List<Data> fetchData() {
        ... fetches data from backend ...
    }

    public void cancel() {
       if (future != null) {
          future.cancel(true);
          future = null;
       }
    }
}

Describe alternatives you've considered

This pattern comes close (works 99% of the time), but have some caveats as in some corner cases getUI() is not thread safe (mainly when your Route happens to be extending Composite).

getUI().ifPresent(ui -> ui.access(() -> {
    ...
});

Passing UI reference to thread is commonly used by many developers, but has caveats, especially if set as thread local. This is prone to security risks (e.g. updating wrong UI by accident).

TatuLund avatar Oct 02 '24 06:10 TatuLund