From 70a3dbff24e7e39b8be9861a6fcb0cd05eb52bb2 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 26 Nov 2019 10:44:09 +0000 Subject: [PATCH] WebSession creation does not block Closes gh-24027 --- .../session/InMemoryWebSessionStore.java | 10 +++++++-- .../session/InMemoryWebSessionStoreTests.java | 11 ++++++++++ .../annotation/ModelInitializerTests.java | 21 +++++++++++-------- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java b/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java index cb23bb313d50..09eb47560e0f 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java +++ b/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,6 +29,7 @@ import java.util.concurrent.locks.ReentrantLock; import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; import org.springframework.util.Assert; import org.springframework.util.IdGenerator; @@ -111,9 +112,14 @@ public Map getSessions() { @Override public Mono createWebSession() { + + // Opportunity to clean expired sessions Instant now = this.clock.instant(); this.expiredSessionChecker.checkIfNecessary(now); - return Mono.fromSupplier(() -> new InMemoryWebSession(now)); + + return Mono.fromSupplier(() -> new InMemoryWebSession(now)) + .subscribeOn(Schedulers.boundedElastic()) + .cast(WebSession.class); } @Override diff --git a/spring-web/src/test/java/org/springframework/web/server/session/InMemoryWebSessionStoreTests.java b/spring-web/src/test/java/org/springframework/web/server/session/InMemoryWebSessionStoreTests.java index d2ab0de61d95..998f22dccc11 100644 --- a/spring-web/src/test/java/org/springframework/web/server/session/InMemoryWebSessionStoreTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/session/InMemoryWebSessionStoreTests.java @@ -22,7 +22,10 @@ import java.util.Map; import java.util.stream.IntStream; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; import org.springframework.beans.DirectFieldAccessor; import org.springframework.web.server.WebSession; @@ -56,6 +59,14 @@ public void startsSessionImplicitly() { assertThat(session.isStarted()).isTrue(); } + @Disabled // TODO: remove if/when Blockhound is enabled + @Test // gh-24027 + public void createSessionDoesNotBlock() { + Mono.defer(() -> this.store.createWebSession()) + .subscribeOn(Schedulers.parallel()) + .block(); + } + @Test public void retrieveExpiredSession() { WebSession session = this.store.createWebSession().block(); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelInitializerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelInitializerTests.java index a8a5ef0d0fa7..0a388a1ca568 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelInitializerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelInitializerTests.java @@ -64,6 +64,9 @@ */ public class ModelInitializerTests { + private static final Duration TIMEOUT = Duration.ofMillis(5000); + + private ModelInitializer modelInitializer; private final ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path")); @@ -93,7 +96,7 @@ public void initBinderMethod() { Method method = ResolvableMethod.on(TestController.class).annotPresent(GetMapping.class).resolveMethod(); HandlerMethod handlerMethod = new HandlerMethod(controller, method); - this.modelInitializer.initModel(handlerMethod, context, this.exchange).block(Duration.ofMillis(5000)); + this.modelInitializer.initModel(handlerMethod, context, this.exchange).block(TIMEOUT); WebExchangeDataBinder binder = context.createDataBinder(this.exchange, "name"); assertThat(binder.getValidators()).isEqualTo(Collections.singletonList(validator)); @@ -107,7 +110,7 @@ public void modelAttributeMethods() { Method method = ResolvableMethod.on(TestController.class).annotPresent(GetMapping.class).resolveMethod(); HandlerMethod handlerMethod = new HandlerMethod(controller, method); - this.modelInitializer.initModel(handlerMethod, context, this.exchange).block(Duration.ofMillis(5000)); + this.modelInitializer.initModel(handlerMethod, context, this.exchange).block(TIMEOUT); Map model = context.getModel().asMap(); assertThat(model.size()).isEqualTo(5); @@ -116,7 +119,7 @@ public void modelAttributeMethods() { assertThat(((TestBean) value).getName()).isEqualTo("Bean"); value = model.get("monoBean"); - assertThat(((Mono) value).block(Duration.ofMillis(5000)).getName()).isEqualTo("Mono Bean"); + assertThat(((Mono) value).block(TIMEOUT).getName()).isEqualTo("Mono Bean"); value = model.get("singleBean"); assertThat(((Single) value).toBlocking().value().getName()).isEqualTo("Single Bean"); @@ -135,7 +138,7 @@ public void saveModelAttributeToSession() { Method method = ResolvableMethod.on(TestController.class).annotPresent(GetMapping.class).resolveMethod(); HandlerMethod handlerMethod = new HandlerMethod(controller, method); - this.modelInitializer.initModel(handlerMethod, context, this.exchange).block(Duration.ofMillis(5000)); + this.modelInitializer.initModel(handlerMethod, context, this.exchange).block(TIMEOUT); WebSession session = this.exchange.getSession().block(Duration.ZERO); assertThat(session).isNotNull(); @@ -148,7 +151,7 @@ public void saveModelAttributeToSession() { @Test public void retrieveModelAttributeFromSession() { - WebSession session = this.exchange.getSession().block(Duration.ZERO); + WebSession session = this.exchange.getSession().block(TIMEOUT); assertThat(session).isNotNull(); TestBean testBean = new TestBean("Session Bean"); @@ -159,7 +162,7 @@ public void retrieveModelAttributeFromSession() { Method method = ResolvableMethod.on(TestController.class).annotPresent(GetMapping.class).resolveMethod(); HandlerMethod handlerMethod = new HandlerMethod(controller, method); - this.modelInitializer.initModel(handlerMethod, context, this.exchange).block(Duration.ofMillis(5000)); + this.modelInitializer.initModel(handlerMethod, context, this.exchange).block(TIMEOUT); context.saveModel(); assertThat(session.getAttributes().size()).isEqualTo(1); @@ -174,13 +177,13 @@ public void requiredSessionAttributeMissing() { Method method = ResolvableMethod.on(TestController.class).annotPresent(PostMapping.class).resolveMethod(); HandlerMethod handlerMethod = new HandlerMethod(controller, method); assertThatIllegalArgumentException().isThrownBy(() -> - this.modelInitializer.initModel(handlerMethod, context, this.exchange).block(Duration.ofMillis(5000))) + this.modelInitializer.initModel(handlerMethod, context, this.exchange).block(TIMEOUT)) .withMessage("Required attribute 'missing-bean' is missing."); } @Test public void clearModelAttributeFromSession() { - WebSession session = this.exchange.getSession().block(Duration.ZERO); + WebSession session = this.exchange.getSession().block(TIMEOUT); assertThat(session).isNotNull(); TestBean testBean = new TestBean("Session Bean"); @@ -191,7 +194,7 @@ public void clearModelAttributeFromSession() { Method method = ResolvableMethod.on(TestController.class).annotPresent(GetMapping.class).resolveMethod(); HandlerMethod handlerMethod = new HandlerMethod(controller, method); - this.modelInitializer.initModel(handlerMethod, context, this.exchange).block(Duration.ofMillis(5000)); + this.modelInitializer.initModel(handlerMethod, context, this.exchange).block(TIMEOUT); context.getSessionStatus().setComplete(); context.saveModel();