Allow simpler extension of the SpringServlet / structuring of SpringBootAutoConfiguration

[pgould-taskize]

Describe your motivation

We have certain ThreadLocal values that need to be set before access tasks are run.

The com.vaadin.flow.server.VaadinService provides a hook to allow overriding the accessSession, and in order to do this, one needs to extend the VaadinServlet class and override the createServletService(), which used to be advertised as a
https://vaadin.com/docs/v14/flow/advanced/tutorial-application-lifecycle.

    /**
     * Implementation for {@link VaadinSession#access(Command)}. This method is
     * implemented here instead of in {@link VaadinSession} to enable overriding
     * the implementation without using a custom subclass of VaadinSession.
     *
     * @param session
     *            the vaadin session to access
     * @param command
     *            the command to run with the session locked
     * @return a future that can be used to check for task completion and to
     *         cancel the task
     * @see VaadinSession#access(Command)
     */
    public Future<Void> accessSession(VaadinSession session, Command command) 
        ...
    }

This documentation has been removed in the V24 documentation, yet no other strategy for achieving the same process has been given, although it seems that this is still possible but requires requires cut + pasting of thecom.vaadin.flow.spring.SpringBootAutoConfiguration#servletRegistrationBean.

Describe the solution you'd like

Could the auto-configuration be modified in such a way that cut and pasting the entire servletRegistrationBean is not necessary, for example, the only piece of code that I'd like to have to supply is the springServletBean definition in the following configuration:

@Configuration
public class OverriddenServletConfiguration {

    @Autowired
    private WebApplicationContext context;

    @Bean
    public SpringServlet springServletBean(VaadinConfigurationProperties configurationProperties) {
        String mapping = configurationProperties.getUrlMapping();
        boolean rootMapping = RootMappedCondition.isRootMapping(mapping);
        return new OverriddenSpringServlet(context, rootMapping);
    }

    @Bean
    public ServletRegistrationBean<SpringServlet> servletRegistrationBean(
            ObjectProvider<MultipartConfigElement> multipartConfig,
            VaadinConfigurationProperties configurationProperties,
            SpringServlet springServlet) {
        String mapping = configurationProperties.getUrlMapping();
        Map<String, String> initParameters = new HashMap<>();
        boolean rootMapping = RootMappedCondition.isRootMapping(mapping);

        if (rootMapping) {
            mapping = VaadinServletConfiguration.VAADIN_SERVLET_MAPPING;
            initParameters.put(
                    VaadinServlet.INTERNAL_VAADIN_SERVLET_VITE_DEV_MODE_FRONTEND_PATH,
                    "");
        }

        String pushUrl = rootMapping ? "" : mapping.replace("/*", "");
        pushUrl += "/" + Constants.PUSH_MAPPING;

        initParameters.put(ApplicationConfig.JSR356_MAPPING_PATH, pushUrl);

        ServletRegistrationBean<SpringServlet> registration = new ServletRegistrationBean<>(springServlet, mapping);
        registration.setInitParameters(initParameters);
        registration
                .setAsyncSupported(configurationProperties.isAsyncSupported());
        registration.setName(
                ClassUtils.getShortNameAsProperty(SpringServlet.class));
        // Setup multi part form processing for non root servlet mapping to be
        // able to process Hilla login out of the box
        if (!rootMapping) {
            multipartConfig.ifAvailable(registration::setMultipartConfig);
        }
        registration.setLoadOnStartup(
                configurationProperties.isLoadOnStartup() ? 1 : -1);
        return registration;
    }

}

Describe alternatives you've considered

Any other hooks to execute code prior to access tasks being run and cleaned up afterwards would be perfectly acceptable.

Additional context

Is there anything else you can add about the proposal?

[TatuLund]

A simpler approach to this would be to implement AbstractView which will be extended by Route classes and implement access delegate method there which also takes care of handling thread locals before and after ui.access(..).

public abstract class AbstractView extends Div {

    private UI ui;
    Future<Void> future;

    protected void onAttach(AttachEvent attachEvent) {
        super.onAttach(attachEvent);
        ui = attachEvent.getUI();
    }

    protected Future<Void> uiAccess(Command command) {
        if (ui == null) {
            throw new IllegalStateException("Component not attached");
        }
        // Set thread locals
        try {
            future = ui.access(command);
            return future;
        } finally {
            // Remove thread locals
        }
    }

    protected void onDetach(DetachEvent detachEvent) {
        super.onDetach(detachEvent);
        future.cancel(true);
        ui = null;
    }
}

[pgould-taskize]

We can't do something like you suggested above, as the access call is coming from a flush requested by the DataCommunicator.

The data that we need available at the point that access is called is a cross cutting concern which our DataProviders should have no knowledge of. Whilst we could start doing things along the lines of having a custom CallbackDataProvider which all our other data providers extend to set this context, who is to say that there aren't other parts of the platform that will do similar if we use them.

However, the hook to do what we want to do is there, and documented in the Javadocs - so I don't understand the reluctance to make the facility easier to use. If we're not expected to use it, could I suggest that it be deprecated or the documentation updated to reflect this?

com.taskize.repository.tenant.rules.TenantBusinessRulesImpl.getUserOrganisations(TenantBusinessRulesImpl.java:474)
  at com.taskize.ui.bubble.bubblelist.BubbleListService.getParticipantQuerySpecs(BubbleListService.java:59)
  at com.taskize.ui.bubble.bubblelist.BubbleListService.countForPrincipalAndFilter(BubbleListService.java:46)
  at com.taskize.ui.bubble.bubblelist.BubbleListDataProviderCallbacks.count(BubbleListDataProviderCallbacks.java:52)
  at com.vaadin.flow.data.provider.CallbackDataProvider.sizeInBackEnd(CallbackDataProvider.java:142)
  at com.vaadin.flow.data.provider.AbstractBackEndDataProvider.size(AbstractBackEndDataProvider.java:66)
  at com.vaadin.flow.data.provider.DataCommunicator.getDataProviderSize(DataCommunicator.java:940)
  at com.vaadin.flow.data.provider.DataCommunicator.flush(DataCommunicator.java:1193)
  at com.vaadin.flow.data.provider.DataCommunicator.lambda$requestFlush$7258256f$1(DataCommunicator.java:1138)
  at 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 com.vaadin.flow.internal.StateTree.runExecutionsBeforeClientResponse(StateTree.java:392)
  at com.vaadin.flow.server.communication.UidlWriter.encodeChanges(UidlWriter.java:394)
  at com.vaadin.flow.server.communication.UidlWriter.createUidl(UidlWriter.java:170)
  at com.vaadin.flow.server.communication.UidlWriter.createUidl(UidlWriter.java:215)
  at com.vaadin.flow.server.communication.AtmospherePushConnection.push(AtmospherePushConnection.java:207)
  ... 15 common frames omitted