Closed idelpivnitskiy closed 1 week ago
just wanted to make sure that it is double checked that the new order is good and nothing ended up accidentally in the wrong spot of the filter chain. Doesn't look like it to me, just checking.
There are existing tests to validate most of the features and ordering:
HttpServerFilterOrderTest
to test the ordering of user-added filters;ServerEffectiveStrategyTest
to test OffloadingFilter
(it started to fail when I moved ClearAsyncContextHttpServiceFilter
);AbstractHttpServiceAsyncContextTest
to test ClearAsyncContextHttpServiceFilter
;HttpLifecycleObserverTest
to test observer;ConnectionCloseHeaderHandlingTest
to test KeepAliveServiceFilter
;HttpExceptionMapperServiceFilter
;HttpMessageDiscardWatchdogServiceFilterTest
(reenabled locally to test);
Motivation:
If users put any data into
AsyncContext
insideHttpLifecycleObserver
configured viaHttpServerBuilder.lifecycleObserver(...)
it won't be visible for filters and service becauseClearAsyncContextHttpServiceFilter
is appended afterHttpLifecycleObserverServiceFilter
insideapplyInternalFilters
.Modifications:
ClearAsyncContextHttpServiceFilter
from the beginning ofnoOffloadServiceFilters
toapplyInternalFilters
. However, this move discovered another bug in path that handlesnoOffloadServiceFilters.isEmpty()
case because it never addsOffloadingFilter
.listenForService
method to always make a copy of filters lists, then prepend/append internal filters to those copies in easy to read/understand way, and construct a single filtersStream
forbuildService
method.OffloadingFilter
to act as a regular filter factory instead of taking all furtherserviceFilters
as pre-built factory.ClearAsyncContextHttpServiceFilter
singleton instance.OptionalSslNegotiator
binder takes a raw service instead offilteredService
(user-defined filters were never applied for this path).AbstractHttpServiceAsyncContextTest
to testAsyncContext
propagation from lifecycle observer and non-offloading filters.AbstractHttpServiceAsyncContextTest
to testAsyncContext
initialized by early/late connection acceptor is not visible at request path.BlockingHttpServiceAsyncContextTest
andHttpServiceAsyncContextTest
to make sureAsyncContext
propagation for blocking/async aggregated services is also tested.ParameterizedTest
where possible.Results:
AsyncContext
initialized inside server's lifecycle observer is visible through filter chain and services.