feat: Adds a VaadinRequestInterceptor abstraction #17502

marcingrzejszczak · 2023-08-25T11:27:44Z

Description

VaadinFilter simulates an around aspect around processing of a request. Prerequisite for #17494

Related to #17436

Type of change

Bugfix
Feature

Checklist

I have read the contribution guide: https://vaadin.com/docs/latest/guide/contributing/overview/
I have added a description following the guideline.
The issue is created in the corresponding repository and I have referenced it.
I have added tests to ensure my change is effective and works as intended.
New and existing tests are passing locally with my change.
I have performed self-review and corrected misspellings.

Additional for `Feature` type of change

Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

marcingrzejszczak · 2023-08-25T14:44:08Z

Quick comment on the VaadinFilter (although now in separate PR): maybe it could also cover UI.access calls coming from separate threads 🤔 And then requests & al are optional or within some context parameter 🤷‍♂️

@mstahv can you elaborate on that?

mshabarov

Looks good to me in general.
Two concerns about naming of this new class and the fact that both servlet and service has these filters mutable.
If possible, I would preference to have immutable collection once it is initialised.
I will check how it can be done.

flow-server/src/main/java/com/vaadin/flow/server/VaadinFilter.java

flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java

github-actions · 2023-08-30T11:21:12Z

Test Results

1 007 files ±  0 1 007 suites ±0 1h 14m 21s ⏱️ + 13m 6s
6 420 tests +  2 6 379 ✔️ +  2 41 💤 ±0 0 ❌ ±0
6 677 runs +19 6 629 ✔️ +19 48 💤 ±0 0 ❌ ±0

Results for commit 3a85d40. ± Comparison against base commit cb63006.

♻️ This comment has been updated with latest results.

marcingrzejszczak · 2023-08-31T11:52:17Z

@mshabarov I've checked the Service init mechanism that you mentioned and decided to rewrite my proposal to use that mechanism. You can check the details here. I like it better because there is just 1 way of setting the interceptors common for spring and non-spring apps. PTAL

mstahv · 2023-08-31T12:45:46Z

Explaining my comment regarding the UI.access & requests:

Not every change in Vaadin UIs happen with an active http request
UIs can be modified by via other users request or by external threads as well
UIs are then actually updated by the next XHR or via constantly open web socket communication
For app developers it would/could be handy if they could hook into begin and end of a "UI access", which are typically wrapped in calls passed to UI.access calls, unless they already happen within a "UI thread". One could for example open and close user specific backend/database connection or do various profiling related tasks

I didn't look through the change proposal thoroughly, but by quickly skimming it through (mostly checkin names of new classes), this would help only with common XHR connections and user originated UI access 🤔

marcingrzejszczak · 2023-08-31T12:49:08Z

From my perspective we need to wrap all the entry points to the backend and everything going out of the backend. If we can control what goes into and out of the UI that would be perfect. Once we have such a list of entry and exit points I can instrument them (or at least try to :) )

mstahv · 2023-08-31T12:56:02Z

Yeah, I'd like to claim that it is not very rare that interesting things (regarding observability/profiling) happens in these other kind of UI access calls. It for sure does not happen in every app, but for example in dashboard like apps, a lot.

mshabarov · 2023-09-04T11:54:34Z

It makes sense to me what Matti said about having hooks for UI.access.

VaadinService has runPendingAccessTasks where we can hook and call request interceptors before and after calling UI access pending futures. VaadinRequestInterceptor can be extended with extra methods for UI.access.

For Vaadin Push connections (communications via WebSockets), the requestStart and requestEnd methods are already being called for every request from client, see PushHandler::callWithUi method.

mshabarov

My previous comments are fixed, awesome work! 👍
Three points I would like to fix and clarify before we could merge it:

Wrapping UI.access calls with vaadin request interceptor to be able to add instrumentations for it (discussed it already with Matti T. in the comments above)
Cover ErrorHandler calls, so that the exceptions thrown in the UI update callbacks in the user's codes would be also wrapped and observable (see my comment below)
Maybe more global question: should we follow the same instrumentation list as we do for Vaadin Observability Kit, e.g. all Vaadin-provided request handlers, static file handler, data communicator and so on.
My vision is that we can do it iteratively, so we can apply this pull request, add instrumentation (another opened PR) for it and then extend VaadinRequestInterceptor step-by-step with extra methods for concrete instrumentations and egress points.
What do you think @heruan and @Legioth ?

mshabarov · 2023-09-04T13:08:00Z

flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java

@@ -1544,6 +1582,18 @@ private void handleExceptionDuringRequest(VaadinRequest request,
            vaadinSession.lock();
        }
        try {
+            try {
+                vaadinRequestInterceptors


handleExceptionDuringRequest doesn't actually handle all the errors during request processing.
It catches internal errors, but errors that might happen inside lambdas provided by developers (mainly UI updates, for instance, if you pass an UI update into beforeClientResponse) are processed by ErrorHandler, so that the exception is not thrown up to the invocation hierarchy, but handled by this error handler. See https://vaadin.com/docs/latest/advanced/custom-error-handler.

This might be a default error handler or a custom error handler, which, for example shows the notification with error message or changes component's styles.

Also, if you search in the project by string .getErrorHandler().error(, you will find a few entries in the core code that catch exceptions and delegate them to the error handler.

Thus, to cover also the exceptions handled by ErrorHandler objects, we may add a ErrorHandlerWrapper that takes ErrorHandler in VaadinSession::setErrorHandler and decorates it with calls to request interceptors.

vaadin-spring/src/test/java/com/vaadin/flow/spring/service/SpringVaadinServletServiceTest.java

...server/src/main/resources/META-INF/services/com.vaadin.flow.server.VaadinServiceInitListener

marcingrzejszczak · 2023-09-12T13:34:45Z

Wrapping UI.access calls with vaadin request interceptor to be able to add instrumentations for it (discussed it already with Matti T. in the comments above)

I was thinking that we could instrument com.vaadin.flow.server.VaadinService#runPendingAccessTasks. But we would have to instrument using a different interceptor. The VaadinRequestInterceptor is really related to http. Here we could have an interceptor around executing commands. Would that make any sense?

Cover ErrorHandler calls, so that the exceptions thrown in the UI update callbacks in the user's codes would be also wrapped and observable (see my comment below)

If we did this via the interceptor that I mentioned above we would also take care of the error handling?

Maybe more global question: should we follow the same instrumentation list as we do for Vaadin Observability Kit, e.g. all Vaadin-provided request handlers, static file handler, data communicator and so on.
My vision is that we can do it iteratively, so we can apply this pull request, add instrumentation (another opened PR) for it and then extend VaadinRequestInterceptor step-by-step with extra methods for concrete instrumentations and egress points.
What do you think @heruan and @Legioth ?

I am all for baby steps ;)

mshabarov · 2023-09-15T11:02:20Z

I was thinking that we could instrument com.vaadin.flow.server.VaadinService#runPendingAccessTasks. But we would have to instrument using a different interceptor. The VaadinRequestInterceptor is really related to http. Here we could have an interceptor around executing commands. Would that make any sense?

Yes, this makes sense to me to add a new aspect for pending access invocations. Let's do it in a separate pull request.

If we did this via the interceptor that I mentioned above we would also take care of the error handling?

UI.access and whatever server side code that can pass through the ErrorHandler are quite unrelated to each other.
UI.access refers to a PUSH functionality, i.e. asynchronous server-to-client UI updates, whereas ErrorHandler intercepts server-side exceptions, including (but not only limited to UI.access), exceptions in synchronous handlers, like "before client response" commands and RPC requests from clients.

From the other hand, when the ErrorHandler::error method is called, it doesn't know from what context it's called, and this is correct. This means that we cannot easily decide which interceptor to call, when this error handler is called: request or async interceptor. If we skip intercepting error handler :: error calls, UI update codes in command would be ignored by these interceptors and not instrumented, which will make user of this API upset a lot.

A simplest solution IMO is to make a decorator for error handler, whenever it is being added to the session here

flow/flow-server/src/main/java/com/vaadin/flow/server/VaadinSession.java

Line 429 in daecc25

public void setErrorHandler(ErrorHandler errorHandler) {

, put there a call to VaadinRequestInterceptor::handleException. This would intercept also errors for async updates. Yes, this might be a bit confusing for API users, but I see no better and easy solution at the moment.

I am all for baby steps ;)

We discussed the design internally, and our decision now is to focus on the basic request/response instrumentation, not trying to repeat all the features implemented with OpenTelemetry standard in Vaadin Observability Kits, because it would need a really deep code changes in some cases. This definitely better to do in a separate pull requests later. For most of the users even having a request/response/error instrumentation is already a big feature.

mshabarov · 2023-09-15T11:19:54Z

...server/src/main/java/com/vaadin/flow/server/VaadinRequestInterceptorServiceInitListener.java

+ * @author Marcin Grzejszczak
+ * @since 24.2
+ */
+public class VaadinRequestInterceptorServiceInitListener


By the way, why do we need this init listener?
Looks like doing this way, we make an extra loop forcing Flow to take a detour, whereas this can be "hardcoded" here

flow/flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java

Line 232 in 349cc11

event.getAddedRequestHandlers().forEach(handlers::add);

, the same way as for request handler.

This might lead to not being able to autowire the interceptors as a Spring bean, but I believe this would be fine, you can still create a VaadinRequestInterceptorServiceInitListener and say that it should be a AtComponent in Spring based apps.

I mean I think I was following what was being done now for RequestHandlers. I've actually copy pasted the whole code that the request handlers are following and did the same for interceptors.

My analysis:

A request interceptor List<RequestHandler> handlers = createRequestHandlers(); is initialized. These handlers do not require any collaborators (they have default constructors only, which will not be the case when doing instrumentations - you need tracers, registries etc.)

I'm doing exactly the same for the interceptors List<VaadinRequestInterceptor> requestInterceptors = createVaadinRequestInterceptors();

when VaadinService is initialized it calls instantiator.getServiceInitListeners().forEach(listener -> listener.serviceInit(event)); what will initialize the event with whatever the listeners want to add (e.g. custom interceptors etc.). The custom listeners would set interceptors such as the micrometer observation interceptor

I'm doing the same for interceptors - I read the entries from the event and add them to an unmodifiable list

For the compatibility with Spring I've added a second constructor for constructor injection

I don't understand what I should actually do then if not what I've already done

Let me checkout your fork and try to refactor it without making VaadinRequestInterceptorServiceInitListener. Will be back to you soon.

Alright, thanks for being patient :)
I checked out your fork ("vaadinFilter" branch) and made the following changes on top of it:

Removed changes in Instantiator, DefaultInstantiator and SpringInstantiator

Removed VaadinRequestInterceptorServiceInitListener

Removed META-INF/services file for VaadinRequestInterceptorServiceInitListener

Removed VaadinRequestInterceptorServiceInitListener from SpringBootAutoConfiguration.

Added an implementation for request interceptor impl to test-spring-boot test module:

@Component public class LoggerRequestInterceptor implements VaadinRequestInterceptor { private static final Logger logger = LoggerFactory.getLogger("RequestInterceptor"); @Override public void requestStart(VaadinRequest request, VaadinResponse response) { logger.info("Request start, path into = {}" , request.getPathInfo()); } @Override public void handleException(VaadinRequest request, VaadinResponse response, VaadinSession vaadinSession, Exception t) { } @Override public void requestEnd(VaadinRequest request, VaadinResponse response, VaadinSession session) { logger.info("Request end, path into = {}" , request.getPathInfo()); } }

Added initialiser to test-spring-boot test module:

@Component public class MyVaadinServiceInitListener implements VaadinServiceInitListener { private final List<VaadinRequestInterceptor> interceptors; public MyVaadinServiceInitListener(@Autowired List<VaadinRequestInterceptor> interceptors) { this.interceptors = interceptors; } @Override public void serviceInit(ServiceInitEvent event) { interceptors.forEach(event::addVaadinRequestInterceptor); } }

Changed some config in pom files to be able to quickly run the test app in test-spring-boot.

So now if you:

Build the project with
mvn clean install -DskipTests -pl flow-server,vaadin-spring,flow-tests/vaadin-spring-tests -am,

Navigate to test module
cd flow-tests/vaadin-spring-tests/test-spring-boot

And finally run it with
mvn clean spring-boot:run -nsu

And navigate to "localhost:8888",

You will see the interceptor in action, adding some log message to the server console.

Why I prefer to remove these codes:

Main reason is that I want to have a Micrometer implementation in a separate module, e.g. "flow-micrometer". Thus, the flow-server itself shouldn't add any interceptors by default, but flow-micrometer should - add the interceptors by adding the service init listener, like I showed above for MyVaadinServiceInitListener, as a Spring Bean.

Service Init Listener API is for external usage (in the Vaadin projects, not inside Flow). Internally, we can and should hard-code the handlers/interceptors, added by default. In our case, the interceptors are empty by default. This might however change in the future, who knows what interceptors we might want to add by default.
If we do it with adding a listener and adding SPI into meta-inf resources, it would mean a redundant loop in the code.

Instantiator and Lookup additions doesn't seem clear - Flow can spot Service Init Listener without them. Service Init Listener can be found as a Spring Bean as a AtComponent, so no idea why we need an extra code for it.

Not clear why SpringBootAutoConfiguration need a new bean (service init listener for interceptors). Again, you can create interceptors beans as I shown above in my example and autowire them to the listener class. All should work.

So, 2-4 seem to me an extra codes, interceptors API should work without it.
1 is proposed to be do in your second pull request.

Hope I haven't missed anything.

Here is IntelliJ IDEA patch that you can apply on your branch to see my codes (hope you are using IDEA :) )
remove_service_init_listener_and_add_a_usecase.patch

Looks awesome! I mean we have the same outcome by having less code I'm all for it :D So how do we proceed now?

I cannot push into your branch, because I have no rights.
If you can apply my patch locally, could you commit it and push (only changes in flow-server and vaadin-spring, others are use case codes and local workarounds) it?
Also, tests I think need some tweaks, if you have a chance please do, otherwise I can help you with it.

mshabarov · 2023-09-15T11:24:21Z

vaadin-spring/src/main/java/com/vaadin/flow/spring/SpringBootAutoConfiguration.java

@@ -119,6 +119,13 @@ public ServletRegistrationBean<SpringServlet> servletRegistrationBean(
        return registration;
    }

+    @Bean
+    public VaadinRequestInterceptorServiceInitListener vaadinRequestInterceptorServiceInitListener(
+            ObjectProvider<VaadinRequestInterceptor> interceptors) {


On a second look this seems to me as an overhead.
We don't have a similar API/ability to add request handler and whatever objects developers can add in ServiceInitListeners.

Plus, projects which already have a service init listener can just add one line to their listener to add the interceptors.

See my another comment https://github.com/vaadin/flow/pull/17502/files#r1327153517.

So, if you have no big objections, I would propose to remove this from autoconfiguration for simplicity.

We don't have a similar API/ability to add request handler and whatever objects developers can add in ServiceInitListeners.

I mean you do, ServiceInitListener works via SPI and it can append things to the init event. I thought that it's done exactly for this purpose (https://vaadin.com/docs/latest/advanced/service-init-listener).

I can do whatever you want but I don't know how you would like this to work :) How will you want to add the request interceptors to the VaadinService?

Here is my vision of design so far:

flow-server creates an empty list of interceptors by default and makes this protected, so developers can change it if they want. In the future we might want to add some interceptors by default (for whatever purpose). flow-server collects interceptors from ServiceInitEvent and makes an immutable collection that is used for further http request interception. (this is already done and look awesome).

flow-micrometer module in the Flow repo adds a VaadinRequestInterceptorServiceInitListener as a Spring'sAtComponent and injects there all needed Micrometer Interceptors. No SPI registration is needed. If this module is in the classpath of the project, this listener should be picked and executed by Flow.

Vaadin plain projects can register their own service init listeners with SPI and Vaadin Spring-based projects can register this listener as a component and autowire interceptors.

No changes to Instantiator and SpringBootAutoConfiguration is needed for the above points to work.

Your solution so far seems to make it possible to only add the interceptors beans in the projects (without care about making own service init listener).
I would strongly recommend to not do it within this pull request.
It should be a separate feature and consistent with other kind of data in the service init event.
So far I would state that the request interceptors, as request handlers, should be always added through a service init listener defined in the project.

marcingrzejszczak · 2023-09-15T14:09:29Z

Yes, this makes sense to me to add a new aspect for pending access invocations. Let's do it in a separate pull request.

While waiting for feedback I started playing around with this concept and I got sth like this out of it (check the CommandInterceptor)
https://github.com/marcingrzejszczak/flow/compare/vaadinFilter...marcingrzejszczak:flow:vaadinCommandInterceptor?expand=1 , the only problem I had was that when I added that to an application not a single time did my interceptor got called (but maybe I did sth wrong).

What I'm currently thinking about is that we already have instrumentation coming form the Spring Web instrumentation (https://github.com/spring-projects/spring-boot/blob/v3.1.3/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/observation/web/servlet/WebMvcObservationAutoConfiguration.java#L82-L96) of the incoming requests. So if we have a Vaadin application that is non-Spring definitely this new, server side instrumentation ( makes a lot of sense. In the Spring world we need to have IMO one or the other unless I'm missing sth.

mshabarov · 2023-09-20T07:22:20Z

While waiting for feedback I started playing around with this concept and I got sth like this out of it (check the CommandInterceptor)

Great! Thanks for the enthusiasm 👍 🥇
I propose to make a separate PR for it, it would be much more easier.

mshabarov · 2023-09-20T07:24:41Z

@marcingrzejszczak So I propose to apply my last patch #17502 (comment), adapt the tests for it and then I propose to merge this PR, because it's going to be endless otherwise :)

Then we can consider making a new module for instrumentation and then continue with command interceptor.

marcingrzejszczak · 2023-09-25T09:26:34Z

I've just applied the patch, let's see how the build goes...

mshabarov · 2023-09-25T09:41:33Z

Please run mvn formatter:format to let the format checker step pass.

mshabarov

Implementation looks good to me.
A few comments about leftover codes and imports, and one comment about test.

mshabarov · 2023-09-25T09:42:56Z

flow-server/src/main/java/com/vaadin/flow/di/DefaultInstantiator.java

+import java.util.ServiceLoader;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;


Please revert these changes. This class doesn't have a meaningful additions now.

mshabarov · 2023-09-25T09:43:21Z

flow-server/src/main/java/com/vaadin/flow/di/Instantiator.java

@@ -31,6 +27,10 @@
 import com.vaadin.flow.server.communication.IndexHtmlRequestListener;
 import com.vaadin.flow.server.communication.UidlWriter;

+import java.io.Serializable;
+import java.util.ServiceLoader;
+import java.util.stream.Stream;


Please revert these changes. This class doesn't have a meaningful additions now.

mshabarov · 2023-09-25T09:45:02Z

flow-server/src/main/java/com/vaadin/flow/server/VaadinService.java

+import com.vaadin.flow.router.RouteData;
+import com.vaadin.flow.router.Router;
+import com.vaadin.flow.server.HandlerHelper.RequestType;
+import com.vaadin.flow.server.communication.*;


Please don't use the wildcard import, but specify the exact classes, like it was before.

mshabarov · 2023-09-25T09:48:34Z

flow-tests/vaadin-spring-tests/pom.xml

-                        </execution>
-                    </executions>
-                </plugin>
+<!--                <plugin>-->


Revert this. It is a leftover from a local run.

mshabarov · 2023-09-25T09:54:32Z

flow-tests/vaadin-spring-tests/test-spring-boot/pom.xml

+            <groupId>com.vaadin</groupId>
+            <artifactId>vaadin-dev-server</artifactId>
+            <version>${project.version}</version>
+        </dependency>


Same. Please revert this. It is a left over from local test.

mshabarov · 2023-09-25T09:54:38Z

flow-tests/vaadin-spring-tests/test-spring-boot/pom.xml

@@ -112,7 +118,7 @@
                    <execution>
                        <goals>
                            <goal>prepare-frontend</goal>
-                            <goal>build-frontend</goal>
+<!--                            <goal>build-frontend</goal>-->


Same. Please revert this. It is a left over from local test.

mshabarov · 2023-09-25T09:57:03Z

vaadin-spring/src/main/java/com/vaadin/flow/spring/SpringBootAutoConfiguration.java

-
-import jakarta.servlet.MultipartConfigElement;
+import java.util.HashMap;
+import java.util.Map;


Revert imports change, no functionality is added into this class.

mshabarov · 2023-09-25T10:33:44Z

vaadin-spring/src/test/java/com/vaadin/flow/spring/service/SpringVaadinServletServiceTest.java

@@ -89,4 +95,21 @@ public void uiInitListenerAsSpringBean_listenerIsAutoregisteredAsUIInitiLietnerI
        Assert.assertSame(ui, listener.events.get(0).getUI());
    }

+    @Test
+    public void requestInterceptorsAreRegisteredOnTheService()


I'm not sure that I understand how this is supposed to work.
How MyRequestInterceptor instances get injected into VaadinService? I see no service init listener here.

Would it be more clear if you:

Move LoggerRequestInterceptor and MyVaadinServiceInitListener classes from test-spring-boot module to here

Add MyVaadinServiceInitListener to the Spring context (maybe this isn't even needed if Spring test configuration can recognise the bean and add it automatically), so this listener would pass through the VaadinService and it's interceptors are collected and added to the list by new logic in the flow-server.

Pass any VaadinRequest to the VaadinServlet/VaadinService and check that the interceptor does changes in the request/response (like you already does in flow-server test).

Making this kind of test, we would repeat the use case of how these interceptors are added to Flow in reality.

Looks like this test fails with:

Error: Tests run: 3, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.172 s <<< FAILURE! -- in com.vaadin.flow.spring.service.SpringVaadinServletServiceTest Error: com.vaadin.flow.spring.service.SpringVaadinServletServiceTest.requestInterceptorsAreRegisteredOnTheService -- Time elapsed: 0.035 s <<< FAILURE! org.opentest4j.AssertionFailedError: There should be 1 filter ==> expected: <1> but was: <0> at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151) at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132) at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197) at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150) at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:560) at com.vaadin.flow.spring.service.SpringVaadinServletServiceTest.requestInterceptorsAreRegisteredOnTheService(SpringVaadinServletServiceTest.java:108)

Honestly, after the changes you've made I don't know how this is supposed to work but I'm not a Vaadin expert ;) I'm trying to make the tests work now. Will ping you once I can't figure things out

Actually I've made the test pass, let me see if everything else is working fine and that formatting is not broken and I will push the changes! Your comments were very helpful.

Ok, now this test clearly shows that MyRequestInterceptor is autowired to the listener component and gets used by VaadinService.

LoggerRequestInterceptor class is not needed I think. I suppose it is added to the list and invoked, but not used anyhow in tests.

So let's remove LoggerRequestInterceptor and I'm ready to approve the PR.

marcingrzejszczak · 2023-09-25T11:38:20Z

Ok, hopefully everything is fine now. I do have problems with the wildcard imports and formatting because I can't seem to align with the intellij idea formatting style for this project. Anyways, hopefully that's resolved now

mshabarov · 2023-09-25T12:32:06Z

vaadin-spring/src/test/java/com/vaadin/flow/spring/service/SpringVaadinServletServiceTest.java

@@ -89,4 +95,21 @@ public void uiInitListenerAsSpringBean_listenerIsAutoregisteredAsUIInitiLietnerI
        Assert.assertSame(ui, listener.events.get(0).getUI());
    }

+    @Test
+    public void requestInterceptorsAreRegisteredOnTheService()


Ok, now this test clearly shows that MyRequestInterceptor is autowired to the listener component and gets used by VaadinService.

LoggerRequestInterceptor class is not needed I think. I suppose it is added to the list and invoked, but not used anyhow in tests.

So let's remove LoggerRequestInterceptor and I'm ready to approve the PR.

VaadinFilter simulates an around aspect around processing of a request related to vaadingh-17436

* renamed VaadinFilter to VaadinRequestInterceptor * Vaadin uses Lookup to init the VaadinService ** Added VaadinRequestInterceptors to ServiceInitEvent ** Added a VaadinRequestInterceptorServiceInitListener that will add interceptors to the init event ** The listener works with ServiceLocator based DI and Spring based DI (has both arg and non-arg constructor) ** That way we have 1 way of initializing the interceptors * Added the VaadinRequestInterceptorServiceInitListener to META-INF for automated locator discovery * Added the VaadinRequestInterceptorServiceInitListener as a bean for Spring Boot auto configuration * Extended the Instantiator mechanism to find all instances of a given class ** Implemented the method in Spring and non-Spring implementations

Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>

knoobie · 2023-09-25T12:55:51Z

Before it is forgotten, can we get the 24.2 label? ;)

mshabarov · 2023-09-25T13:35:40Z

Before it is forgotten, can we get the 24.2 label? ;)

I'm afraid no, because Flow 24.2 beta is out today and we lost the release train for it. I propose to target this and further instrumentation feature to Vaadin 24.3.

sonarcloud · 2023-09-25T13:45:44Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

vaadin-bot · 2023-10-26T06:56:03Z

This ticket/PR has been released with Vaadin 24.3.0.alpha1 and is also targeting the upcoming stable 24.3.0 version.

mshabarov added the Contribution PRs coming from the community or external to the team label Aug 25, 2023

marcingrzejszczak force-pushed the vaadinFilter branch from 6a7486c to a327e3b Compare August 25, 2023 11:37

marcingrzejszczak mentioned this pull request Aug 25, 2023

feat: Micrometer Observation instrumentation #17494

Draft

9 tasks

marcingrzejszczak changed the title ~~Adds a VaadinFilter abstraction~~ feat: Adds a VaadinFilter abstraction Aug 25, 2023

marcingrzejszczak mentioned this pull request Aug 25, 2023

Micrometer Observation Support #17436

Open

mshabarov requested review from caalador and mshabarov August 28, 2023 11:07

caalador added this to Iteration Reviews in OLD Vaadin Flow ongoing work (Vaadin 10+) Aug 29, 2023

mshabarov requested changes Aug 30, 2023

View reviewed changes

vaadin-bot added the +1.0.0 label Aug 31, 2023

marcingrzejszczak force-pushed the vaadinFilter branch from e922f3e to 134785c Compare August 31, 2023 11:50

marcingrzejszczak force-pushed the vaadinFilter branch from 8340842 to e5ad270 Compare September 4, 2023 13:10

mshabarov requested changes Sep 4, 2023

View reviewed changes

marcingrzejszczak force-pushed the vaadinFilter branch from c2d8a0e to 0d6c5ec Compare September 12, 2023 13:14

marcingrzejszczak force-pushed the vaadinFilter branch from 0d6c5ec to 9052f7b Compare September 12, 2023 14:05

mshabarov reviewed Sep 15, 2023

View reviewed changes

marcingrzejszczak force-pushed the vaadinFilter branch from 9052f7b to 4c73948 Compare September 15, 2023 13:45

marcingrzejszczak force-pushed the vaadinFilter branch from 4c73948 to 697bdb1 Compare September 25, 2023 09:26

vaadin-bot added +0.1.0 and removed +1.0.0 labels Sep 25, 2023

mshabarov requested changes Sep 25, 2023

View reviewed changes

marcingrzejszczak and others added 10 commits September 25, 2023 14:39

Adds a VaadinFilter abstraction

8d5347c

VaadinFilter simulates an around aspect around processing of a request related to vaadingh-17436

Polish

82f2253

Apply suggestions from code review

1fedec7

Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>

Applied changes from @mshabarov

0990647

Ran formatting plugin

bb80d3e

Fixed request interceptor test

aeb3ea2

Applied changes following the review

7f239f0

Fixed invalid imports

a702101

Removed LoggerRequestInterceptor

3a85d40

marcingrzejszczak force-pushed the vaadinFilter branch from 7c224e2 to 3a85d40 Compare September 25, 2023 12:39

mshabarov approved these changes Sep 25, 2023

View reviewed changes

mshabarov changed the title ~~feat: Adds a VaadinFilter abstraction~~ feat: Adds a VaadinRequestInterceptor abstraction Sep 25, 2023

mshabarov merged commit 18e2851 into vaadin:main Sep 26, 2023
28 of 30 checks passed

OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Iteration Reviews to Done - pending release Sep 26, 2023

marcingrzejszczak deleted the vaadinFilter branch September 26, 2023 08:17

vaadin-bot added prerelease version for Vaadin 24.3.0 Released with Vaadin 24.3.0.alpha1 labels Oct 26, 2023

feat: Adds a VaadinRequestInterceptor abstraction #17502

feat: Adds a VaadinRequestInterceptor abstraction #17502

Conversation

marcingrzejszczak commented Aug 25, 2023

Description

Type of change

Checklist

Additional for Feature type of change

marcingrzejszczak commented Aug 25, 2023

mshabarov left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 30, 2023 • edited

Test Results

marcingrzejszczak commented Aug 31, 2023

mstahv commented Aug 31, 2023

marcingrzejszczak commented Aug 31, 2023

mstahv commented Aug 31, 2023

mshabarov commented Sep 4, 2023 • edited

mshabarov left a comment • edited

Choose a reason for hiding this comment

mshabarov Sep 4, 2023 • edited

Choose a reason for hiding this comment

marcingrzejszczak commented Sep 12, 2023

mshabarov commented Sep 15, 2023 • edited

mshabarov Sep 15, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcingrzejszczak Sep 15, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mshabarov Sep 19, 2023 • edited

Choose a reason for hiding this comment

marcingrzejszczak commented Sep 15, 2023

mshabarov commented Sep 20, 2023

mshabarov commented Sep 20, 2023 • edited

marcingrzejszczak commented Sep 25, 2023

mshabarov commented Sep 25, 2023

mshabarov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mshabarov Sep 25, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mshabarov Sep 25, 2023 • edited

Choose a reason for hiding this comment

marcingrzejszczak commented Sep 25, 2023

mshabarov Sep 25, 2023 • edited

Choose a reason for hiding this comment

knoobie commented Sep 25, 2023

mshabarov commented Sep 25, 2023 • edited

sonarcloud bot commented Sep 25, 2023

vaadin-bot commented Oct 26, 2023

Additional for `Feature` type of change

github-actions bot commented Aug 30, 2023 •

edited

mshabarov commented Sep 4, 2023 •

edited

mshabarov left a comment •

edited

mshabarov Sep 4, 2023 •

edited

mshabarov commented Sep 15, 2023 •

edited

mshabarov Sep 15, 2023 •

edited

marcingrzejszczak Sep 15, 2023 •

edited

mshabarov Sep 19, 2023 •

edited

mshabarov commented Sep 20, 2023 •

edited

mshabarov Sep 25, 2023 •

edited

mshabarov Sep 25, 2023 •

edited

mshabarov Sep 25, 2023 •

edited

mshabarov commented Sep 25, 2023 •

edited