vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
622 stars 166 forks source link

Provide a legal way to customize serviceUrl and pushUrl with Vaadin15+ bootstrap. #10776

Open xdenser opened 3 years ago

xdenser commented 3 years ago

Hi, again I have problems migrating from Vaadin14 bootstrap to Vaadin 15+. The reason to have special service/push URL is simple:

  1. I have more than 1 cluster node
  2. I do not want user to see cluster node path in address bar.
  3. My session handling also needs additional non-static parameters in service/push URLs

To override service url I was using custom BootstrapContext (which is not public by the way) in customized BootstrapHandler (Vaadin14 way). This also works for IndexHtmlRequestHandler in eager mode, but not in JavascriptBootstrapHandler - this one just takes url from browser location in Flow.ts.

As for push url - there is no hook to set it, so i have to override IndexHtmlRequestHandler #createAndInitUI() to adjust push url in eager mode. In Vaadin14 boostrap it was possible to use custom UI - and PushConfiguration is part of ui - so it was simple to override.

So non-eager mode is just not usable for me as I have no chance to adjust service URL. One possible solution would be to make authentication check and non-static url parameters initialization in customized JavascriptBootstrapHandler, but there I need sometimes to redirect to another page and this complicates things bacause it handles Ajax request - index page should support this somehow. I can also live with eager mode - but it is not default. I would prefer to use some default boostrap mode.

pleku commented 3 years ago

As for push url - there is no hook to set it

There is AppShellSettings -> getPushConfiguration -> setPushUrl already. It was in InitialPageSettings in v14 but apparently you we're not using it ?

xdenser commented 3 years ago

According to code it just maps to UI, and how AppShellSettings is supposed to be used? Push url has to be set at right moment - just before initial UI is sent to the client.

xdenser commented 3 years ago

Ah yes propbably it will just work in some MainView or AppLayout constructor: UI.getCurrent().getPushConfiguration().setPushUrl() , or via new AppshellSettings()... I did not try. Or did I and it was too late? I do not remember. Anyway it works in bootstrap handlers (IndexHtml ... or Javascript... depending on eagerServerLoad) just after createAndInitUI().

And for this to work whole chain of classes - VaadinServlet, VaadinServletService and finally already mentioned boostrap handlers have to be customized. But I need to customize bootstrap handler anyway because of special session handling, so this is not a problem for me.

xdenser commented 3 years ago

I think the right place would be AppShellConfigurator - because there we have @Push annotation and so it will be in the same place. But I think there is no way to configure push url there currently.

knoobie commented 3 years ago

@pleku Looks like the problem is that PushConfiguration has no simple setter for the pushURL - see: https://github.com/vaadin/flow/blob/995dd88bbe5dc0a40fc4cfaf179429b05b3581d9/flow-client/src/main/java/com/vaadin/client/communication/PushConfiguration.java#L96-L104 and getConfigurationMap() is private.

pleku commented 3 years ago

@knoobie That is the client side class, but I was looking at https://github.com/vaadin/flow/blob/master/flow-server/src/main/java/com/vaadin/flow/component/PushConfiguration.java#L150

pleku commented 3 years ago

But I think there is no way to configure push url there currently.

@xdenser https://vaadin.com/docs/latest/flow/advanced/modifying-the-bootstrap-page/#the-appshellconfiguratorconfigurepage-method

@Push
public class AppShell implements AppShellConfigurator {
    @Override
    void configurePage(AppShellSettings settings) {
        settings.getPushConfiguration().ifPresent(pushConfiguration -> pushConfiguration.setPushUrl("foobar"));
    }
}
xdenser commented 3 years ago

@pleku no this does not work for me, i.e. it is called but actually not used. At least in eagerServerLoad mode.

because in com.vaadin.flow.server.communication.IndexHtmlRequestHandler.synchronizedHandleRequest(VaadinSession, VaadinRequest, VaadinResponse) includeInitialUidl() is called before registry.modifyIndexHtml()

pleku commented 3 years ago

That sounds like a bug, but what is this mode ?

At least in eagerServerLoad mode.

xdenser commented 3 years ago

see com.vaadin.flow.function.DeploymentConfiguration.isEagerServerLoad()

pleku commented 3 years ago

Right, sorry I was not aware of the naming of that API. Like the javadocs in AppShellConfigurator state:

* A marker interface to configure the index.hml page when using client-side
 * bootstrapping.

The "eager mode" is the opposite of as this client side bootstrapping, so it should not even work that way. I think in that in the eager mode, you can use a implements PageConfigurator in your main-layout and configure the push configuration from there (unless that is broken). This is the v14 way for doing it and it this "eager mode" basically matches the v14 bootstrapping that is how we refer to it internally, need to probably change our terminology.

pleku commented 3 years ago

Provided that the old way of doing things (configuring stuff) still works for the bootstrapping, we should at least fix the javadocs to use the same naming consistently and consider if we should fail with clear error message if we detect that the application has the AppShellConfigurator but the bootstrapping mode is "non-eager".

xdenser commented 3 years ago

And what is then com.vaadin.flow.server.AbstractConfiguration.useV14Bootstrap()? this one switches then some other boostrap. see com.vaadin.flow.server.VaadinServletService.addBootstrapHandler(List)

xdenser commented 3 years ago

And in non-eager mode this Optional<PushConfiguratoin> is just empty, so it does not work either.

pleku commented 3 years ago

And in non-eager mode this Optional<PushConfiguration> is just empty, so it does not work either.

Just to be sure - did you mvn clean the project when made the change between the modes ? I do recall some issues when the setting has been cached and not thus applied.

While it is quite clear how to reproduce it I would still recommend using e.g. https://github.com/vaadin/skeleton-starter-flow to make a clean app that reproduces the issue, zip/link the project here and can be used to verify it. This is so that issue can be addressed faster.

xdenser commented 3 years ago

Here is example: non-eager.zip I did not find quickly how to disable default servlet - it just lives on /node1/ path. On root path there is "load balancing" servlet - serving to node2 and node3, in real world we have some reverse proxy instead of this servlet. Try to make it working w/o custom MyVaadinServlet - only with configuration. I would also prefer that initial uild request - http://localhost:8080/?v-r=init will come to corresponding node (the one from which index,html was served). I will make the same for eager mode next week.

xdenser commented 3 years ago

one addtional requirent - non static configuration, as you of course can use DeploymentConfiguration, but I need some bootstrap callbacks, as my serviceURL contains part of session id (i did no implement this in example) - we support multiple user sessions from the same browser, and this is hard to make with standard session handling.

xdenser commented 3 years ago

Here is eagerServerLoad variant. eager.zip