feat: Implement a push transport mode utilizing SSE

[mstahv]

PoC for #23048

Mostly LLM generated, not throughly investitaged yet. Tested roughly locally and on a typical nginx proxied server.

[github-actions]

Format Checker Report

There are 4 files with format errors

• To see a complete report of formatting issues, download the differences artifact

• To fix the build, please run mvn spotless:apply in your branch and commit the changes.

• Optionally you might add the following line in your .git/hooks/pre-commit file:

  mvn spotless:apply
  

Here is the list of files with format issues in your PR:

flow-server/src/main/java/com/vaadin/flow/component/PushConfiguration.java
flow-server/src/main/java/com/vaadin/flow/server/communication/JavaScriptBootstrapHandler.java
flow-server/src/main/java/com/vaadin/flow/server/communication/SsePushConnection.java
flow-server/src/main/java/com/vaadin/flow/server/communication/SseRequestHandler.java

[github-actions]

Test Results

1 186 files   -   123  1 186 suites   - 123   1h 12m 51s ⏱️ - 2m 28s
8 007 tests  - 1 214  7 938 ✅  - 1 215  66 💤  - 2  3 ❌ +3 
8 457 runs   - 1 223  8 380 ✅  - 1 224  74 💤  - 2  3 ❌ +3 

For more details on these failures, see this check.

Results for commit e592b2d. ± Comparison against base commit d0ae97a.

This pull request removes 1214 tests.

com.vaadin.base.devserver.AbstractDevServerRunnerTest ‑ shouldPassEncodedUrlToDevServer
com.vaadin.base.devserver.AbstractDevServerRunnerTest ‑ updateServerStartupEnvironment_preferIpv4_LocalhostIpAddressAddedToProcessEnvironment
com.vaadin.base.devserver.AbstractDevServerRunnerTest ‑ updateServerStartupEnvironment_preferIpv6_LocalhostIpAddressAddedToProcessEnvironment
com.vaadin.base.devserver.BrowserLiveReloadAccessorImplTest ‑ getLiveReload_devMode_contextHasNoReloadInstance_instanceIsCreated
com.vaadin.base.devserver.BrowserLiveReloadAccessorImplTest ‑ getLiveReload_devMode_contextHasReloadInstance_instanceIsReturned
com.vaadin.base.devserver.BrowserLiveReloadAccessorImplTest ‑ getLiveReload_liveReloadDisabled_instanceIsCreated
com.vaadin.base.devserver.BrowserLiveReloadAccessorImplTest ‑ getLiveReload_productionMode_nullIsReturned
com.vaadin.base.devserver.DebugWindowConnectionLicenseCheckTest ‑ checkLicense_invalidLicense_sendLicenseCheckFailed
com.vaadin.base.devserver.DebugWindowConnectionLicenseCheckTest ‑ checkLicense_noLicenseKeys_sendLicenseCheckFailed
com.vaadin.base.devserver.DebugWindowConnectionLicenseCheckTest ‑ checkLicense_validLicense_sendLicenseOk
…

♻️ This comment has been updated with latest results.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[mshabarov]

@mstahv could you please fix the unit tests (minor issues with serializable) and resolve conflicts?

What do you think is remaining before merging this change? It's a PoC, so should we just review and agree on the concept or continue with finalizing it in the same PR? Do you want to complete it or shall we?

[Artur-]

I don't think this should ever be merged unless there is a comprehensive test set included. The current push implementations with Atmosphere (presumably) work correctly because a large test set was made for Vaadin 8 and both Vaadin and Atmosphere had a lot of fixes done. The feature was then ported to Vaadin 10+.

If you add a new push implementation that does not use Atmosphere you will likely have no support for lost messages due to the various disconnection/reconnection scenarious. Additionally reconnecting after a temporary network error (bad mobile reception) might or might not work in the browser. Additionally there are a lot of other cases. Testing the happy path is good but it is far from enough.

One way forward might be to first have some AI check what tests are still missing from Flow but exist in Vaadin 8. Then port those and also add SSE variants of all the tests. Then you would also need to add any relevant tests from Atmosphere to verify e.g. server side message caching works properly.

[mstahv]

This is a PoC ATM, but I do see potential in here. Not just getting rid of large badly maintained library, but e.g. request based security solutions (like what is default with Spring Security) probably works better than with "pure websockets".

More important than getting some unit tests working would be to understand what is expected to work regarding "push" and how are things tested currently. If somebody has good insights or knowledge on that, would be great. Reading from Artur's comment that our push test set in current Vaadin is not in par with Vaadin 8 does not sound good 😬

[mstahv]

Did we have some mechanism to build "feature snapshosts" from branches? Would make it easier to test in practical solutions/setups (I wouldn't be very convinced yet even if framework tests pass, especially as I have no idea how push is currently tested).

[mcollovati]

To better understand. what's the overall goal of this PR? Using SSE transport or getting rid of atmosphere in general?
If it is the first one, Atmosphere already supports SSE.

[mcollovati]

Did we have some mechanism to build "feature snapshosts" from branches?

You already used it by naming your branch feature/<something>. You can now use Flow artifacts with version 25.1.sse-SNAPSHOT

[mstahv]

To better understand. what's the overall goal of this PR? Using SSE transport or getting rid of atmosphere in general? If it is the first one, Atmosphere already supports SSE.

Getting rid of Atmosphere. Ultimatily: make push connection on by default. For that I'd want depedendency hit to be minimal/non-existent (or some smaller library that is actively maintained).