Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
TatuLund opened this issue Oct 2, 2024 · 0 comments

Comments

@TatuLund
Copy link
Contributor

TatuLund commented Oct 2, 2024

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant