From 167ddc7cfcc7d96da94454bb8cf2405e7161ee82 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Tue, 5 Sep 2017 22:04:14 -0500 Subject: [PATCH 1/6] Polish HeaderWebSessionIdResolver Use constant for default header name and make getHeaderName private. Also switch HeaderWebSessionIdResolverTests to unit tests rather than testing with DefaultWebSessionManager. Issue: SPR-15917 --- .../session/HeaderWebSessionIdResolver.java | 24 ++- .../HeaderWebSessionIdResolverTests.java | 197 +++++++----------- 2 files changed, 93 insertions(+), 128 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/server/session/HeaderWebSessionIdResolver.java b/spring-web/src/main/java/org/springframework/web/server/session/HeaderWebSessionIdResolver.java index 3845e293b54..9ee5cffd614 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/HeaderWebSessionIdResolver.java +++ b/spring-web/src/main/java/org/springframework/web/server/session/HeaderWebSessionIdResolver.java @@ -26,18 +26,23 @@ import org.springframework.web.server.ServerWebExchange; * Request and response header-based {@link WebSessionIdResolver}. * * @author Greg Turnquist + * @author Rob Winch * @since 5.0 */ public class HeaderWebSessionIdResolver implements WebSessionIdResolver { - private String headerName = "SESSION"; + /** + * The default header name + */ + public static final String DEFAULT_HEADER_NAME = "SESSION"; + private String headerName = DEFAULT_HEADER_NAME; /** * Set the name of the session header to use for the session id. * The name is used to extract the session id from the request headers as * well to set the session id on the response headers. - *

By default set to {@literal "SESSION"}. + *

By default set to {@code DEFAULT_HEADER_NAME} * @param headerName the header name */ public void setHeaderName(String headerName) { @@ -45,14 +50,6 @@ public class HeaderWebSessionIdResolver implements WebSessionIdResolver { this.headerName = headerName; } - /** - * Return the configured header name. - */ - public String getHeaderName() { - return this.headerName; - } - - @Override public List resolveSessionIds(ServerWebExchange exchange) { HttpHeaders headers = exchange.getRequest().getHeaders(); @@ -70,4 +67,11 @@ public class HeaderWebSessionIdResolver implements WebSessionIdResolver { this.setSessionId(exchange, ""); } + /** + * Return the configured header name. + * @return the configured header name + */ + private String getHeaderName() { + return this.headerName; + } } diff --git a/spring-web/src/test/java/org/springframework/web/server/session/HeaderWebSessionIdResolverTests.java b/spring-web/src/test/java/org/springframework/web/server/session/HeaderWebSessionIdResolverTests.java index d0d4a7117a0..ad31462a56e 100644 --- a/spring-web/src/test/java/org/springframework/web/server/session/HeaderWebSessionIdResolverTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/session/HeaderWebSessionIdResolverTests.java @@ -15,170 +15,131 @@ */ package org.springframework.web.server.session; -import java.time.Clock; -import java.time.Duration; -import java.time.Instant; -import java.time.ZoneId; -import java.util.UUID; - import org.junit.Before; import org.junit.Test; -import reactor.core.publisher.Mono; - -import org.springframework.http.codec.ServerCodecConfigurer; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; -import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse; import org.springframework.web.server.ServerWebExchange; -import org.springframework.web.server.WebSession; -import org.springframework.web.server.adapter.DefaultServerWebExchange; -import org.springframework.web.server.i18n.AcceptHeaderLocaleContextResolver; -import static org.hamcrest.collection.IsCollectionWithSize.hasSize; -import static org.hamcrest.core.Is.is; -import static org.hamcrest.core.IsCollectionContaining.hasItem; +import java.util.Arrays; +import java.util.List; + import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** * Tests using {@link HeaderWebSessionIdResolver}. * * @author Greg Turnquist + * @author Rob Winch */ public class HeaderWebSessionIdResolverTests { - - private static final Clock CLOCK = Clock.system(ZoneId.of("GMT")); - - private HeaderWebSessionIdResolver idResolver; - private DefaultWebSessionManager manager; - private ServerWebExchange exchange; - @Before public void setUp() { this.idResolver = new HeaderWebSessionIdResolver(); - this.manager = new DefaultWebSessionManager(); - this.manager.setSessionIdResolver(this.idResolver); + this.exchange = MockServerHttpRequest.get("/path").toExchange(); + } - MockServerHttpRequest request = MockServerHttpRequest.get("/path").build(); - MockServerHttpResponse response = new MockServerHttpResponse(); + @Test + public void expireWhenValidThenSetsEmptyHeader() { + this.idResolver.expireSession(this.exchange); - this.exchange = new DefaultServerWebExchange(request, response, this.manager, - ServerCodecConfigurer.create(), new AcceptHeaderLocaleContextResolver()); + assertEquals(Arrays.asList(""), + this.exchange.getResponse().getHeaders().get(HeaderWebSessionIdResolver.DEFAULT_HEADER_NAME)); } @Test - public void getSessionWithoutStarting() throws Exception { - WebSession session = this.manager.getSession(this.exchange).block(); - session.save(); + public void expireWhenMultipleInvocationThenSetsSingleEmptyHeader() { + this.idResolver.expireSession(this.exchange); + + this.idResolver.expireSession(this.exchange); - assertFalse(session.isStarted()); - assertFalse(session.isExpired()); - assertNull(this.manager.getSessionStore().retrieveSession(session.getId()).block()); + assertEquals(Arrays.asList(""), + this.exchange.getResponse().getHeaders().get(HeaderWebSessionIdResolver.DEFAULT_HEADER_NAME)); } @Test - public void startSessionExplicitly() throws Exception { - WebSession session = this.manager.getSession(this.exchange).block(); - session.start(); - session.save().block(); - - assertThat(this.exchange.getResponse().getHeaders().containsKey("SESSION"), is(true)); - assertThat(this.exchange.getResponse().getHeaders().get("SESSION"), hasSize(1)); - assertThat(this.exchange.getResponse().getHeaders().get("SESSION"), hasItem(session.getId())); + public void expireWhenAfterSetSessionIdThenSetsEmptyHeader() { + this.idResolver.setSessionId(this.exchange, "123"); + + this.idResolver.expireSession(this.exchange); + + assertEquals(Arrays.asList(""), + this.exchange.getResponse().getHeaders().get(HeaderWebSessionIdResolver.DEFAULT_HEADER_NAME)); } @Test - public void startSessionImplicitly() throws Exception { - WebSession session = this.manager.getSession(this.exchange).block(); - session.getAttributes().put("foo", "bar"); - session.save(); + public void setSessionIdWhenValidThenSetsHeader() { + String id = "123"; + + this.idResolver.setSessionId(this.exchange, id); - assertNotNull(this.exchange.getResponse().getHeaders().get("SESSION")); + assertEquals(Arrays.asList(id), + this.exchange.getResponse().getHeaders().get(HeaderWebSessionIdResolver.DEFAULT_HEADER_NAME)); } @Test - public void existingSession() throws Exception { - UUID sessionId = UUID.randomUUID(); - DefaultWebSession existing = createDefaultWebSession(sessionId); - this.manager.getSessionStore().storeSession(existing); - - this.exchange = this.exchange.mutate() - .request(this.exchange.getRequest().mutate() - .header("SESSION", sessionId.toString()) - .build()) - .build(); - - WebSession actual = this.manager.getSession(this.exchange).block(); - assertNotNull(actual); - assertEquals(existing.getId(), actual.getId()); + public void setSessionIdWhenMultipleThenSetsSingleHeader() { + String id = "123"; + this.idResolver.setSessionId(this.exchange, "overriddenByNextInvocation"); + + this.idResolver.setSessionId(this.exchange, id); + + assertEquals(Arrays.asList(id), + this.exchange.getResponse().getHeaders().get(HeaderWebSessionIdResolver.DEFAULT_HEADER_NAME)); } @Test - public void existingSessionIsExpired() throws Exception { - UUID sessionId = UUID.randomUUID(); - DefaultWebSession existing = createDefaultWebSession(sessionId); - existing.start(); - Instant lastAccessTime = Instant.now(CLOCK).minus(Duration.ofMinutes(31)); - existing = new DefaultWebSession(existing, lastAccessTime, s -> Mono.empty()); - this.manager.getSessionStore().storeSession(existing); - - this.exchange = this.exchange.mutate() - .request(this.exchange.getRequest().mutate() - .header("SESSION", sessionId.toString()) - .build()) - .build(); - - WebSession actual = this.manager.getSession(this.exchange).block(); - assertNotSame(existing, actual); + public void setSessionIdWhenCustomHeaderNameThenSetsHeader() { + String headerName = "x-auth"; + String id = "123"; + this.idResolver.setHeaderName(headerName); + + this.idResolver.setSessionId(this.exchange, id); + + assertEquals(Arrays.asList(id), + this.exchange.getResponse().getHeaders().get(headerName)); } - @Test - public void multipleSessionIds() throws Exception { - UUID sessionId = UUID.randomUUID(); - DefaultWebSession existing = createDefaultWebSession(sessionId); - this.manager.getSessionStore().storeSession(existing); - this.manager.getSessionStore().storeSession(createDefaultWebSession(UUID.randomUUID())); - this.manager.getSessionStore().storeSession(createDefaultWebSession(UUID.randomUUID())); - - this.exchange = this.exchange.mutate() - .request(this.exchange.getRequest().mutate() - .header("SESSION", sessionId.toString()) - .build()) - .build(); - - WebSession actual = this.manager.getSession(this.exchange).block(); - assertNotNull(actual); - assertEquals(existing.getId(), actual.getId()); + @Test(expected = IllegalArgumentException.class) + public void setSessionIdWhenNullIdThenIllegalArgumentException() { + String id = null; + + this.idResolver.setSessionId(this.exchange, id); } @Test - public void alternateHeaderName() throws Exception { - this.idResolver.setHeaderName("alternateHeaderName"); - - UUID sessionId = UUID.randomUUID(); - DefaultWebSession existing = createDefaultWebSession(sessionId); - this.manager.getSessionStore().storeSession(existing); - - this.exchange = this.exchange.mutate() - .request(this.exchange.getRequest().mutate() - .header("alternateHeaderName", sessionId.toString()) - .build()) - .build(); - - WebSession actual = this.manager.getSession(this.exchange).block(); - assertNotNull(actual); - assertEquals(existing.getId(), actual.getId()); + public void resolveSessionIdsWhenNoIdsThenEmpty() { + List ids = this.idResolver.resolveSessionIds(this.exchange); + + assertTrue(ids.isEmpty()); } - private DefaultWebSession createDefaultWebSession(UUID sessionId) { - return new DefaultWebSession(() -> sessionId, CLOCK, (s, session) -> Mono.empty(), s -> Mono.empty()); + @Test + public void resolveSessionIdsWhenIdThenIdFound() { + String id = "123"; + this.exchange = MockServerHttpRequest.get("/path") + .header(HeaderWebSessionIdResolver.DEFAULT_HEADER_NAME, id) + .toExchange(); + + List ids = this.idResolver.resolveSessionIds(this.exchange); + + assertEquals(Arrays.asList(id), ids); } + @Test + public void resolveSessionIdsWhenMultipleIdsThenIdsFound() { + String id1 = "123"; + String id2 = "abc"; + this.exchange = MockServerHttpRequest.get("/path") + .header(HeaderWebSessionIdResolver.DEFAULT_HEADER_NAME, id1, id2) + .toExchange(); + + List ids = this.idResolver.resolveSessionIds(this.exchange); + + assertEquals(Arrays.asList(id1, id2), ids); + } } From b7280472d611f642da1ca56e6371355815ece4b2 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Wed, 23 Aug 2017 13:58:13 -0500 Subject: [PATCH 2/6] Improve WebSession related tests Add missing DefaultWebSessionManagerTests .block(). Previously session.save() was invoked, but we did not ensure it was completed. This commit makes it block on session.save() Fix existingSessionIsExpired. This test is actually broken and is testing a new session is created because the session id returned by the idResolver does not match the existing WebSession. This commit ensures that the id of the WebSession found by idResolver matches the existing WebSession. DefaultWebSessionManagerTests use Mockito. To ensure we test with independence from InMemoryWebSessionStore we use Mockito for the DefaultWebessionManager collaborators. Add test for response.setComplete(). We want to ensure that when the response is completed, it saves the WebSession and writes it to the response using idResolver --- .../DefaultWebSessionManagerTests.java | 114 ++++++++---------- 1 file changed, 53 insertions(+), 61 deletions(-) diff --git a/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java b/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java index d05cd7fd042..7d7501e32a2 100644 --- a/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java @@ -19,17 +19,17 @@ import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.time.ZoneId; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.List; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; import reactor.core.publisher.Mono; import org.springframework.http.codec.ServerCodecConfigurer; -import org.springframework.lang.Nullable; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse; import org.springframework.util.IdGenerator; @@ -43,13 +43,18 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; /** * Unit tests for {@link DefaultWebSessionManager}. * @author Rossen Stoyanchev + * @author Rob Winch */ +@RunWith(MockitoJUnitRunner.class) public class DefaultWebSessionManagerTests { private static final Clock CLOCK = Clock.system(ZoneId.of("GMT")); @@ -59,16 +64,19 @@ public class DefaultWebSessionManagerTests { private DefaultWebSessionManager manager; - private TestWebSessionIdResolver idResolver; - private ServerWebExchange exchange; + @Mock + private WebSessionIdResolver idResolver; + + @Mock + private WebSessionStore store; @Before public void setUp() throws Exception { this.manager = new DefaultWebSessionManager(); - this.idResolver = new TestWebSessionIdResolver(); this.manager.setSessionIdResolver(this.idResolver); + this.manager.setSessionStore(this.store); MockServerHttpRequest request = MockServerHttpRequest.get("/path").build(); MockServerHttpResponse response = new MockServerHttpResponse(); @@ -79,45 +87,60 @@ public class DefaultWebSessionManagerTests { @Test public void getSessionWithoutStarting() throws Exception { - this.idResolver.setIdsToResolve(Collections.emptyList()); + when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.emptyList()); WebSession session = this.manager.getSession(this.exchange).block(); - session.save(); + session.save().block(); assertFalse(session.isStarted()); assertFalse(session.isExpired()); - assertNull(this.idResolver.getSavedId()); - assertNull(this.manager.getSessionStore().retrieveSession(session.getId()).block()); + verify(this.store, never()).storeSession(any()); + verify(this.idResolver, never()).setSessionId(any(), any()); } @Test public void startSessionExplicitly() throws Exception { - this.idResolver.setIdsToResolve(Collections.emptyList()); + when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.emptyList()); + when(this.store.storeSession(any())).thenReturn(Mono.empty()); WebSession session = this.manager.getSession(this.exchange).block(); session.start(); - session.save(); + session.save().block(); String id = session.getId(); - assertNotNull(this.idResolver.getSavedId()); - assertEquals(id, this.idResolver.getSavedId()); - assertSame(session, this.manager.getSessionStore().retrieveSession(id).block()); + verify(this.store).storeSession(any()); + verify(this.idResolver).setSessionId(any(), eq(id)); } @Test public void startSessionImplicitly() throws Exception { - this.idResolver.setIdsToResolve(Collections.emptyList()); + when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.emptyList()); + when(this.store.storeSession(any())).thenReturn(Mono.empty()); WebSession session = this.manager.getSession(this.exchange).block(); session.getAttributes().put("foo", "bar"); - session.save(); + session.save().block(); - assertNotNull(this.idResolver.getSavedId()); + verify(this.idResolver).setSessionId(any(), any()); + verify(this.store).storeSession(any()); + } + + @Test + public void exchangeWhenResponseSetCompleteThenSavesAndSetsId() throws Exception { + when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.emptyList()); + when(this.store.storeSession(any())).thenReturn(Mono.empty()); + WebSession session = this.manager.getSession(this.exchange).block(); + String id = session.getId(); + session.getAttributes().put("foo", "bar"); + this.exchange.getResponse().setComplete().block(); + + verify(this.idResolver).setSessionId(any(), eq(id)); + verify(this.store).storeSession(any()); } @Test public void existingSession() throws Exception { DefaultWebSession existing = createDefaultWebSession(); String id = existing.getId(); - this.manager.getSessionStore().storeSession(existing); - this.idResolver.setIdsToResolve(Collections.singletonList(id)); + when(this.store.retrieveSession(id)).thenReturn(Mono.just(existing)); + when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.singletonList(id)); WebSession actual = this.manager.getSession(this.exchange).block(); assertNotNull(actual); @@ -130,19 +153,23 @@ public class DefaultWebSessionManagerTests { existing.start(); Instant lastAccessTime = Instant.now(CLOCK).minus(Duration.ofMinutes(31)); existing = new DefaultWebSession(existing, lastAccessTime, s -> Mono.empty()); - this.manager.getSessionStore().storeSession(existing); - this.idResolver.setIdsToResolve(Collections.singletonList("1")); + when(this.store.retrieveSession(existing.getId())).thenReturn(Mono.just(existing)); + when(this.store.removeSession(existing.getId())).thenReturn(Mono.empty()); + when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.singletonList(existing.getId())); WebSession actual = this.manager.getSession(this.exchange).block(); assertNotSame(existing, actual); + verify(this.store).removeSession(existing.getId()); + verify(this.idResolver).expireSession(any()); } @Test public void multipleSessionIds() throws Exception { DefaultWebSession existing = createDefaultWebSession(); String id = existing.getId(); - this.manager.getSessionStore().storeSession(existing); - this.idResolver.setIdsToResolve(Arrays.asList("neither-this", "nor-that", id)); + when(this.store.retrieveSession(any())).thenReturn(Mono.empty()); + when(this.store.retrieveSession(id)).thenReturn(Mono.just(existing)); + when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Arrays.asList("neither-this", "nor-that", id)); WebSession actual = this.manager.getSession(this.exchange).block(); assertNotNull(actual); @@ -152,39 +179,4 @@ public class DefaultWebSessionManagerTests { private DefaultWebSession createDefaultWebSession() { return new DefaultWebSession(idGenerator, CLOCK, (s, session) -> Mono.empty(), s -> Mono.empty()); } - - - private static class TestWebSessionIdResolver implements WebSessionIdResolver { - - private List idsToResolve = new ArrayList<>(); - - @Nullable - private String id = null; - - - public void setIdsToResolve(List idsToResolve) { - this.idsToResolve = idsToResolve; - } - - @Nullable - public String getSavedId() { - return this.id; - } - - @Override - public List resolveSessionIds(ServerWebExchange exchange) { - return this.idsToResolve; - } - - @Override - public void setSessionId(ServerWebExchange exchange, String sessionId) { - this.id = sessionId; - } - - @Override - public void expireSession(ServerWebExchange exchange) { - this.id = null; - } - } - } From 86912475af6cd86c2e09c8aeabd8697708be1ab8 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Tue, 22 Aug 2017 20:36:04 -0500 Subject: [PATCH 3/6] Refactor WebSessionStore - Add WebSessionStore.createWebSession. - Remove remove WebSessionStore.changeSessionId - Add WebSessionStore updateLastAccessTime which allows updating the WebSession lastAccessTime without exposing a method on WebSession in an implementation independent way. - Remove WebSessionStore.storeSession. This method is not necessary since the WebSession that is returned allows saving the WebSession. Additionally, it is error prone since the wrong type might be passed into it. Issue: SPR-15875, 15876 --- .../session/DefaultWebSessionManager.java | 26 +++----- .../session/InMemoryWebSessionStore.java | 66 +++++++++++++++---- .../web/server/session/WebSessionStore.java | 28 ++++---- .../DefaultWebSessionManagerTests.java | 5 ++ .../session/WebSessionIntegrationTests.java | 2 +- 5 files changed, 81 insertions(+), 46 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java b/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java index e9502090701..1d1ce7f58ae 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java +++ b/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java @@ -24,8 +24,6 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import org.springframework.util.Assert; -import org.springframework.util.IdGenerator; -import org.springframework.util.JdkIdGenerator; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebSession; @@ -36,13 +34,11 @@ import org.springframework.web.server.WebSession; * {@link WebSessionStore} * * @author Rossen Stoyanchev + * @author Rob Winch * @since 5.0 */ public class DefaultWebSessionManager implements WebSessionManager { - private static final IdGenerator idGenerator = new JdkIdGenerator(); - - private WebSessionIdResolver sessionIdResolver = new CookieWebSessionIdResolver(); private WebSessionStore sessionStore = new InMemoryWebSessionStore(); @@ -111,22 +107,20 @@ public class DefaultWebSessionManager implements WebSessionManager { return Mono.defer(() -> retrieveSession(exchange) .flatMap(session -> removeSessionIfExpired(exchange, session)) - .map(session -> { - Instant lastAccessTime = Instant.now(getClock()); - return new DefaultWebSession(session, lastAccessTime, s -> saveSession(exchange, s)); - }) + .flatMap(this.getSessionStore()::updateLastAccessTime) .switchIfEmpty(createSession(exchange)) + .cast(DefaultWebSession.class) + .map(session -> new DefaultWebSession(session, session.getLastAccessTime(), s -> saveSession(exchange, s))) .doOnNext(session -> exchange.getResponse().beforeCommit(session::save))); } - private Mono retrieveSession(ServerWebExchange exchange) { + private Mono retrieveSession(ServerWebExchange exchange) { return Flux.fromIterable(getSessionIdResolver().resolveSessionIds(exchange)) .concatMap(this.sessionStore::retrieveSession) - .cast(DefaultWebSession.class) .next(); } - private Mono removeSessionIfExpired(ServerWebExchange exchange, DefaultWebSession session) { + private Mono removeSessionIfExpired(ServerWebExchange exchange, WebSession session) { if (session.isExpired()) { this.sessionIdResolver.expireSession(exchange); return this.sessionStore.removeSession(session.getId()).then(Mono.empty()); @@ -162,11 +156,7 @@ public class DefaultWebSessionManager implements WebSessionManager { return ids.isEmpty() || !session.getId().equals(ids.get(0)); } - private Mono createSession(ServerWebExchange exchange) { - return Mono.fromSupplier(() -> - new DefaultWebSession(idGenerator, getClock(), - (oldId, session) -> this.sessionStore.changeSessionId(oldId, session), - session -> saveSession(exchange, session))); + private Mono createSession(ServerWebExchange exchange) { + return this.sessionStore.createWebSession(); } - } 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 dd193faf5ef..bd2a7a97ba6 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 @@ -15,9 +15,15 @@ */ package org.springframework.web.server.session; +import java.time.Clock; +import java.time.Instant; +import java.time.ZoneId; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import org.springframework.util.Assert; +import org.springframework.util.IdGenerator; +import org.springframework.util.JdkIdGenerator; import reactor.core.publisher.Mono; import org.springframework.web.server.WebSession; @@ -26,18 +32,17 @@ import org.springframework.web.server.WebSession; * Simple Map-based storage for {@link WebSession} instances. * * @author Rossen Stoyanchev + * @author Rob Winch * @since 5.0 */ public class InMemoryWebSessionStore implements WebSessionStore { - private final Map sessions = new ConcurrentHashMap<>(); + private static final IdGenerator idGenerator = new JdkIdGenerator(); + private Clock clock = Clock.system(ZoneId.of("GMT")); + + private final Map sessions = new ConcurrentHashMap<>(); - @Override - public Mono storeSession(WebSession session) { - this.sessions.put(session.getId(), session); - return Mono.empty(); - } @Override public Mono retrieveSession(String id) { @@ -45,16 +50,55 @@ public class InMemoryWebSessionStore implements WebSessionStore { } @Override - public Mono changeSessionId(String oldId, WebSession session) { + public Mono removeSession(String id) { + this.sessions.remove(id); + return Mono.empty(); + } + + public Mono createWebSession() { + return Mono.fromSupplier(() -> + new DefaultWebSession(idGenerator, getClock(), + (oldId, session) -> this.changeSessionId(oldId, session), + this::storeSession)); + } + + public Mono updateLastAccessTime(WebSession webSession) { + return Mono.fromSupplier(() -> { + DefaultWebSession session = (DefaultWebSession) webSession; + Instant lastAccessTime = Instant.now(getClock()); + return new DefaultWebSession(session, lastAccessTime); + }); + } + + /** + * Configure the {@link Clock} to use to set lastAccessTime on every created + * session and to calculate if it is expired. + *

This may be useful to align to different timezone or to set the clock + * back in a test, e.g. {@code Clock.offset(clock, Duration.ofMinutes(-31))} + * in order to simulate session expiration. + *

By default this is {@code Clock.system(ZoneId.of("GMT"))}. + * @param clock the clock to use + */ + public void setClock(Clock clock) { + Assert.notNull(clock, "'clock' is required."); + this.clock = clock; + } + + /** + * Return the configured clock for session lastAccessTime calculations. + */ + public Clock getClock() { + return this.clock; + } + + private Mono changeSessionId(String oldId, WebSession session) { this.sessions.remove(oldId); this.sessions.put(session.getId(), session); return Mono.empty(); } - @Override - public Mono removeSession(String id) { - this.sessions.remove(id); + private Mono storeSession(WebSession session) { + this.sessions.put(session.getId(), session); return Mono.empty(); } - } diff --git a/spring-web/src/main/java/org/springframework/web/server/session/WebSessionStore.java b/spring-web/src/main/java/org/springframework/web/server/session/WebSessionStore.java index b52cafe5469..26c064153b4 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/WebSessionStore.java +++ b/spring-web/src/main/java/org/springframework/web/server/session/WebSessionStore.java @@ -19,20 +19,22 @@ import reactor.core.publisher.Mono; import org.springframework.web.server.WebSession; +import java.time.Instant; + /** * Strategy for {@link WebSession} persistence. * * @author Rossen Stoyanchev + * @author Rob Winch * @since 5.0 */ public interface WebSessionStore { /** - * Store the given WebSession. - * @param session the session to store - * @return a completion notification (success or error) + * Creates the WebSession that can be stored by this WebSessionStore. + * @return the session */ - Mono storeSession(WebSession session); + Mono createWebSession(); /** * Return the WebSession for the given id. @@ -41,18 +43,6 @@ public interface WebSessionStore { */ Mono retrieveSession(String sessionId); - /** - * Update WebSession data storage to reflect a change in session id. - *

Note that the same can be achieved via a combination of - * {@link #removeSession} + {@link #storeSession}. The purpose of this method - * is to allow a more efficient replacement of the session id mapping - * without replacing and storing the session with all of its data. - * @param oldId the previous session id - * @param session the session reflecting the changed session id - * @return completion notification (success or error) - */ - Mono changeSessionId(String oldId, WebSession session); - /** * Remove the WebSession for the specified id. * @param sessionId the id of the session to remove @@ -60,4 +50,10 @@ public interface WebSessionStore { */ Mono removeSession(String sessionId); + /** + * Update the last accessed time to now. + * @param webSession the session to update + * @return the session with the updated last access time + */ + Mono updateLastAccessTime(WebSession webSession); } diff --git a/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java b/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java index 7d7501e32a2..290c8dac531 100644 --- a/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java @@ -74,6 +74,9 @@ public class DefaultWebSessionManagerTests { @Before public void setUp() throws Exception { + when(this.store.createWebSession()).thenReturn(Mono.just(createDefaultWebSession())); + when(this.store.updateLastAccessTime(any())).thenAnswer( invocation -> Mono.just(invocation.getArgument(0))); + this.manager = new DefaultWebSessionManager(); this.manager.setSessionIdResolver(this.idResolver); this.manager.setSessionStore(this.store); @@ -106,6 +109,7 @@ public class DefaultWebSessionManagerTests { session.save().block(); String id = session.getId(); + verify(this.store).createWebSession(); verify(this.store).storeSession(any()); verify(this.idResolver).setSessionId(any(), eq(id)); } @@ -118,6 +122,7 @@ public class DefaultWebSessionManagerTests { session.getAttributes().put("foo", "bar"); session.save().block(); + verify(this.store).createWebSession(); verify(this.idResolver).setSessionId(any(), any()); verify(this.store).storeSession(any()); } diff --git a/spring-web/src/test/java/org/springframework/web/server/session/WebSessionIntegrationTests.java b/spring-web/src/test/java/org/springframework/web/server/session/WebSessionIntegrationTests.java index 705044447f8..be203770259 100644 --- a/spring-web/src/test/java/org/springframework/web/server/session/WebSessionIntegrationTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/session/WebSessionIntegrationTests.java @@ -115,7 +115,7 @@ public class WebSessionIntegrationTests extends AbstractHttpHandlerIntegrationTe assertNotNull(session); Instant lastAccessTime = Clock.offset(this.sessionManager.getClock(), Duration.ofMinutes(-31)).instant(); session = new DefaultWebSession(session, lastAccessTime); - store.storeSession(session); + session.save().block(); // Third request: expired session, new session created request = RequestEntity.get(createUri()).header("Cookie", "SESSION=" + id).build(); From 8ad14ae95c973b5eb1feb05e8cb52a48513cca72 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 24 Aug 2017 12:51:38 -0500 Subject: [PATCH 4/6] DefaultWebSessionManager decoupled from DefaultWebSession DefaultWebSessionManager no longer requires the WebSessionStore to use DefaultWebSession. Removed explicit start() in save(). This seemed unnecessary since at that point isStarted is guaranteed to return true. The status can be updated through the copy constructor. DefaultWebSessionTests added. Issue: SPR-15875 --- .../web/server/session/DefaultWebSession.java | 27 +----- .../session/DefaultWebSessionManager.java | 53 ++--------- .../DefaultWebSessionManagerTests.java | 95 ++++++++----------- .../session/DefaultWebSessionTests.java | 74 +++++++++++++++ .../session/WebSessionIntegrationTests.java | 4 +- 5 files changed, 125 insertions(+), 128 deletions(-) create mode 100644 spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionTests.java diff --git a/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSession.java b/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSession.java index 152c602c0ac..74c4925cd53 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSession.java +++ b/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSession.java @@ -86,28 +86,9 @@ class DefaultWebSession implements WebSession { } /** - * Constructor to refresh an existing session for a new request. - * @param existingSession the session to recreate - * @param lastAccessTime the last access time - * @param saveOperation save operation for the current request - */ - DefaultWebSession(DefaultWebSession existingSession, Instant lastAccessTime, - Function> saveOperation) { - - this.id = existingSession.id; - this.idGenerator = existingSession.idGenerator; - this.attributes = existingSession.attributes; - this.clock = existingSession.clock; - this.changeIdOperation = existingSession.changeIdOperation; - this.saveOperation = saveOperation; - this.creationTime = existingSession.creationTime; - this.lastAccessTime = lastAccessTime; - this.maxIdleTime = existingSession.maxIdleTime; - this.state = existingSession.state; - } - - /** - * For testing purposes. + * Constructor for creating a new session with an updated last access time. + * @param existingSession the existing session to copy + * @param lastAccessTime the new last access time */ DefaultWebSession(DefaultWebSession existingSession, Instant lastAccessTime) { this.id = existingSession.id; @@ -119,7 +100,7 @@ class DefaultWebSession implements WebSession { this.creationTime = existingSession.creationTime; this.lastAccessTime = lastAccessTime; this.maxIdleTime = existingSession.maxIdleTime; - this.state = existingSession.state; + this.state = existingSession.isStarted() ? State.STARTED : State.NEW; } diff --git a/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java b/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java index 1d1ce7f58ae..4a1cc17b580 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java +++ b/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java @@ -15,9 +15,6 @@ */ package org.springframework.web.server.session; -import java.time.Clock; -import java.time.Instant; -import java.time.ZoneId; import java.util.List; import reactor.core.publisher.Flux; @@ -43,9 +40,6 @@ public class DefaultWebSessionManager implements WebSessionManager { private WebSessionStore sessionStore = new InMemoryWebSessionStore(); - private Clock clock = Clock.system(ZoneId.of("GMT")); - - /** * Configure the id resolution strategy. *

By default an instance of {@link CookieWebSessionIdResolver}. @@ -80,38 +74,14 @@ public class DefaultWebSessionManager implements WebSessionManager { return this.sessionStore; } - /** - * Configure the {@link Clock} to use to set lastAccessTime on every created - * session and to calculate if it is expired. - *

This may be useful to align to different timezone or to set the clock - * back in a test, e.g. {@code Clock.offset(clock, Duration.ofMinutes(-31))} - * in order to simulate session expiration. - *

By default this is {@code Clock.system(ZoneId.of("GMT"))}. - * @param clock the clock to use - */ - public void setClock(Clock clock) { - Assert.notNull(clock, "'clock' is required."); - this.clock = clock; - } - - /** - * Return the configured clock for session lastAccessTime calculations. - */ - public Clock getClock() { - return this.clock; - } - - @Override public Mono getSession(ServerWebExchange exchange) { return Mono.defer(() -> retrieveSession(exchange) .flatMap(session -> removeSessionIfExpired(exchange, session)) .flatMap(this.getSessionStore()::updateLastAccessTime) - .switchIfEmpty(createSession(exchange)) - .cast(DefaultWebSession.class) - .map(session -> new DefaultWebSession(session, session.getLastAccessTime(), s -> saveSession(exchange, s))) - .doOnNext(session -> exchange.getResponse().beforeCommit(session::save))); + .switchIfEmpty(this.sessionStore.createWebSession()) + .doOnNext(session -> exchange.getResponse().beforeCommit(() -> save(exchange, session)))); } private Mono retrieveSession(ServerWebExchange exchange) { @@ -128,35 +98,28 @@ public class DefaultWebSessionManager implements WebSessionManager { return Mono.just(session); } - private Mono saveSession(ServerWebExchange exchange, WebSession session) { + private Mono save(ServerWebExchange exchange, WebSession session) { if (session.isExpired()) { return Mono.error(new IllegalStateException( "Sessions are checked for expiration and have their " + - "lastAccessTime updated when first accessed during request processing. " + - "However this session is expired meaning that maxIdleTime elapsed " + - "before the call to session.save().")); + "lastAccessTime updated when first accessed during request processing. " + + "However this session is expired meaning that maxIdleTime elapsed " + + "before the call to session.save().")); } if (!session.isStarted()) { return Mono.empty(); } - // Force explicit start - session.start(); - if (hasNewSessionId(exchange, session)) { - this.sessionIdResolver.setSessionId(exchange, session.getId()); + DefaultWebSessionManager.this.sessionIdResolver.setSessionId(exchange, session.getId()); } - return this.sessionStore.storeSession(session); + return session.save(); } private boolean hasNewSessionId(ServerWebExchange exchange, WebSession session) { List ids = getSessionIdResolver().resolveSessionIds(exchange); return ids.isEmpty() || !session.getId().equals(ids.get(0)); } - - private Mono createSession(ServerWebExchange exchange) { - return this.sessionStore.createWebSession(); - } } diff --git a/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java b/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java index 290c8dac531..3bfe7ccf5a0 100644 --- a/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionManagerTests.java @@ -15,10 +15,6 @@ */ package org.springframework.web.server.session; -import java.time.Clock; -import java.time.Duration; -import java.time.Instant; -import java.time.ZoneId; import java.util.Arrays; import java.util.Collections; @@ -32,8 +28,6 @@ import reactor.core.publisher.Mono; import org.springframework.http.codec.ServerCodecConfigurer; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse; -import org.springframework.util.IdGenerator; -import org.springframework.util.JdkIdGenerator; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebSession; import org.springframework.web.server.adapter.DefaultServerWebExchange; @@ -42,11 +36,11 @@ import org.springframework.web.server.i18n.AcceptHeaderLocaleContextResolver; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNotSame; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; /** @@ -57,11 +51,6 @@ import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class DefaultWebSessionManagerTests { - private static final Clock CLOCK = Clock.system(ZoneId.of("GMT")); - - private static final IdGenerator idGenerator = new JdkIdGenerator(); - - private DefaultWebSessionManager manager; private ServerWebExchange exchange; @@ -72,10 +61,23 @@ public class DefaultWebSessionManagerTests { @Mock private WebSessionStore store; + @Mock + private WebSession createSession; + + @Mock + private WebSession retrieveSession; + + @Mock + private WebSession updateSession; + @Before public void setUp() throws Exception { - when(this.store.createWebSession()).thenReturn(Mono.just(createDefaultWebSession())); - when(this.store.updateLastAccessTime(any())).thenAnswer( invocation -> Mono.just(invocation.getArgument(0))); + when(this.store.createWebSession()).thenReturn(Mono.just(this.createSession)); + when(this.store.updateLastAccessTime(any())).thenReturn(Mono.just(this.updateSession)); + when(this.store.retrieveSession(any())).thenReturn(Mono.just(this.retrieveSession)); + when(this.createSession.save()).thenReturn(Mono.empty()); + when(this.updateSession.getId()).thenReturn("update-session-id"); + when(this.retrieveSession.getId()).thenReturn("retrieve-session-id"); this.manager = new DefaultWebSessionManager(); this.manager.setSessionIdResolver(this.idResolver); @@ -87,90 +89,71 @@ public class DefaultWebSessionManagerTests { ServerCodecConfigurer.create(), new AcceptHeaderLocaleContextResolver()); } - @Test - public void getSessionWithoutStarting() throws Exception { + public void getSessionSaveWhenCreatedAndNotStartedThenNotSaved() throws Exception { when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.emptyList()); WebSession session = this.manager.getSession(this.exchange).block(); - session.save().block(); + this.exchange.getResponse().setComplete().block(); assertFalse(session.isStarted()); assertFalse(session.isExpired()); - verify(this.store, never()).storeSession(any()); + verifyZeroInteractions(this.retrieveSession, this.updateSession); + verify(this.createSession, never()).save(); verify(this.idResolver, never()).setSessionId(any(), any()); } @Test - public void startSessionExplicitly() throws Exception { + public void getSessionSaveWhenCreatedAndStartedThenSavesAndSetsId() throws Exception { when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.emptyList()); - when(this.store.storeSession(any())).thenReturn(Mono.empty()); WebSession session = this.manager.getSession(this.exchange).block(); - session.start(); - session.save().block(); + when(this.createSession.isStarted()).thenReturn(true); + this.exchange.getResponse().setComplete().block(); String id = session.getId(); verify(this.store).createWebSession(); - verify(this.store).storeSession(any()); + verify(this.createSession).save(); verify(this.idResolver).setSessionId(any(), eq(id)); } - @Test - public void startSessionImplicitly() throws Exception { - when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.emptyList()); - when(this.store.storeSession(any())).thenReturn(Mono.empty()); - WebSession session = this.manager.getSession(this.exchange).block(); - session.getAttributes().put("foo", "bar"); - session.save().block(); - - verify(this.store).createWebSession(); - verify(this.idResolver).setSessionId(any(), any()); - verify(this.store).storeSession(any()); - } - @Test public void exchangeWhenResponseSetCompleteThenSavesAndSetsId() throws Exception { when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.emptyList()); - when(this.store.storeSession(any())).thenReturn(Mono.empty()); + String id = this.createSession.getId(); WebSession session = this.manager.getSession(this.exchange).block(); - String id = session.getId(); - session.getAttributes().put("foo", "bar"); + when(this.createSession.isStarted()).thenReturn(true); this.exchange.getResponse().setComplete().block(); verify(this.idResolver).setSessionId(any(), eq(id)); - verify(this.store).storeSession(any()); + verify(this.createSession).save(); } @Test public void existingSession() throws Exception { - DefaultWebSession existing = createDefaultWebSession(); - String id = existing.getId(); - when(this.store.retrieveSession(id)).thenReturn(Mono.just(existing)); + String id = this.updateSession.getId(); + when(this.store.retrieveSession(id)).thenReturn(Mono.just(this.updateSession)); when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.singletonList(id)); WebSession actual = this.manager.getSession(this.exchange).block(); assertNotNull(actual); - assertEquals(existing.getId(), actual.getId()); + assertEquals(id, actual.getId()); } @Test public void existingSessionIsExpired() throws Exception { - DefaultWebSession existing = createDefaultWebSession(); - existing.start(); - Instant lastAccessTime = Instant.now(CLOCK).minus(Duration.ofMinutes(31)); - existing = new DefaultWebSession(existing, lastAccessTime, s -> Mono.empty()); - when(this.store.retrieveSession(existing.getId())).thenReturn(Mono.just(existing)); - when(this.store.removeSession(existing.getId())).thenReturn(Mono.empty()); - when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.singletonList(existing.getId())); + String id = this.retrieveSession.getId(); + when(this.retrieveSession.isExpired()).thenReturn(true); + when(this.idResolver.resolveSessionIds(this.exchange)).thenReturn(Collections.singletonList(id)); + when(this.store.removeSession(any())).thenReturn(Mono.empty()); WebSession actual = this.manager.getSession(this.exchange).block(); - assertNotSame(existing, actual); - verify(this.store).removeSession(existing.getId()); + assertEquals(this.createSession.getId(), actual.getId()); + verify(this.store).removeSession(id); verify(this.idResolver).expireSession(any()); } @Test public void multipleSessionIds() throws Exception { - DefaultWebSession existing = createDefaultWebSession(); + WebSession existing = this.updateSession; String id = existing.getId(); when(this.store.retrieveSession(any())).thenReturn(Mono.empty()); when(this.store.retrieveSession(id)).thenReturn(Mono.just(existing)); @@ -180,8 +163,4 @@ public class DefaultWebSessionManagerTests { assertNotNull(actual); assertEquals(existing.getId(), actual.getId()); } - - private DefaultWebSession createDefaultWebSession() { - return new DefaultWebSession(idGenerator, CLOCK, (s, session) -> Mono.empty(), s -> Mono.empty()); - } } diff --git a/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionTests.java b/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionTests.java new file mode 100644 index 00000000000..a3aa6635ac2 --- /dev/null +++ b/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionTests.java @@ -0,0 +1,74 @@ +/* + * Copyright 2002-2017 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.web.server.session; + +import org.junit.Test; +import org.springframework.util.IdGenerator; +import org.springframework.util.JdkIdGenerator; +import reactor.core.publisher.Mono; + +import java.time.Clock; +import java.time.ZoneId; + +import static org.junit.Assert.assertTrue; + +/** + * @author Rob Winch + * @since 5.0 + */ +public class DefaultWebSessionTests { + private static final Clock CLOCK = Clock.system(ZoneId.of("GMT")); + + private static final IdGenerator idGenerator = new JdkIdGenerator(); + + @Test + public void constructorWhenImplicitStartCopiedThenCopyIsStarted() { + DefaultWebSession original = createDefaultWebSession(); + original.getAttributes().put("foo", "bar"); + + DefaultWebSession copy = new DefaultWebSession(original, CLOCK.instant()); + + assertTrue(copy.isStarted()); + } + + @Test + public void constructorWhenExplicitStartCopiedThenCopyIsStarted() { + DefaultWebSession original = createDefaultWebSession(); + original.start(); + + DefaultWebSession copy = new DefaultWebSession(original, CLOCK.instant()); + + assertTrue(copy.isStarted()); + } + + @Test + public void startsSessionExplicitly() { + DefaultWebSession session = createDefaultWebSession(); + session.start(); + assertTrue(session.isStarted()); + } + + @Test + public void startsSessionImplicitly() { + DefaultWebSession session = createDefaultWebSession(); + session.getAttributes().put("foo", "bar"); + assertTrue(session.isStarted()); + } + + private DefaultWebSession createDefaultWebSession() { + return new DefaultWebSession(idGenerator, CLOCK, (s, session) -> Mono.empty(), s -> Mono.empty()); + } +} \ No newline at end of file diff --git a/spring-web/src/test/java/org/springframework/web/server/session/WebSessionIntegrationTests.java b/spring-web/src/test/java/org/springframework/web/server/session/WebSessionIntegrationTests.java index be203770259..5c06d7b665f 100644 --- a/spring-web/src/test/java/org/springframework/web/server/session/WebSessionIntegrationTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/session/WebSessionIntegrationTests.java @@ -110,10 +110,10 @@ public class WebSessionIntegrationTests extends AbstractHttpHandlerIntegrationTe assertEquals(2, this.handler.getSessionRequestCount()); // Now set the clock of the session back by 31 minutes - WebSessionStore store = this.sessionManager.getSessionStore(); + InMemoryWebSessionStore store = (InMemoryWebSessionStore) this.sessionManager.getSessionStore(); DefaultWebSession session = (DefaultWebSession) store.retrieveSession(id).block(); assertNotNull(session); - Instant lastAccessTime = Clock.offset(this.sessionManager.getClock(), Duration.ofMinutes(-31)).instant(); + Instant lastAccessTime = Clock.offset(store.getClock(), Duration.ofMinutes(-31)).instant(); session = new DefaultWebSession(session, lastAccessTime); session.save().block(); From c7d54c8b525ff8f6dbaf667745010a3247b2e272 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 6 Sep 2017 11:01:22 -0400 Subject: [PATCH 5/6] Polish --- .../web/server/session/DefaultWebSession.java | 5 ++- .../session/DefaultWebSessionManager.java | 2 + .../session/HeaderWebSessionIdResolver.java | 22 +++++----- .../session/InMemoryWebSessionStore.java | 44 ++++++++++--------- .../web/server/session/WebSessionStore.java | 12 ++--- 5 files changed, 47 insertions(+), 38 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSession.java b/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSession.java index 74c4925cd53..ec6c17b8e7a 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSession.java +++ b/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSession.java @@ -60,7 +60,7 @@ class DefaultWebSession implements WebSession { /** - * Constructor for creating a brand, new session. + * Constructor for creating a new session instance. * @param idGenerator the session id generator * @param clock for access to current time */ @@ -86,7 +86,8 @@ class DefaultWebSession implements WebSession { } /** - * Constructor for creating a new session with an updated last access time. + * Copy constructor to re-create a session at the start of a new request + * refreshing the last access time of the session. * @param existingSession the existing session to copy * @param lastAccessTime the new last access time */ diff --git a/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java b/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java index 4a1cc17b580..fa501205541 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java +++ b/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSessionManager.java @@ -40,6 +40,7 @@ public class DefaultWebSessionManager implements WebSessionManager { private WebSessionStore sessionStore = new InMemoryWebSessionStore(); + /** * Configure the id resolution strategy. *

By default an instance of {@link CookieWebSessionIdResolver}. @@ -74,6 +75,7 @@ public class DefaultWebSessionManager implements WebSessionManager { return this.sessionStore; } + @Override public Mono getSession(ServerWebExchange exchange) { return Mono.defer(() -> diff --git a/spring-web/src/main/java/org/springframework/web/server/session/HeaderWebSessionIdResolver.java b/spring-web/src/main/java/org/springframework/web/server/session/HeaderWebSessionIdResolver.java index 9ee5cffd614..54dafb43cee 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/HeaderWebSessionIdResolver.java +++ b/spring-web/src/main/java/org/springframework/web/server/session/HeaderWebSessionIdResolver.java @@ -31,13 +31,13 @@ import org.springframework.web.server.ServerWebExchange; */ public class HeaderWebSessionIdResolver implements WebSessionIdResolver { - /** - * The default header name - */ + /** Default value for {@link #setHeaderName(String)}. */ public static final String DEFAULT_HEADER_NAME = "SESSION"; + private String headerName = DEFAULT_HEADER_NAME; + /** * Set the name of the session header to use for the session id. * The name is used to extract the session id from the request headers as @@ -50,6 +50,15 @@ public class HeaderWebSessionIdResolver implements WebSessionIdResolver { this.headerName = headerName; } + /** + * Return the configured header name. + * @return the configured header name + */ + public String getHeaderName() { + return this.headerName; + } + + @Override public List resolveSessionIds(ServerWebExchange exchange) { HttpHeaders headers = exchange.getRequest().getHeaders(); @@ -67,11 +76,4 @@ public class HeaderWebSessionIdResolver implements WebSessionIdResolver { this.setSessionId(exchange, ""); } - /** - * Return the configured header name. - * @return the configured header name - */ - private String getHeaderName() { - return this.headerName; - } } 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 bd2a7a97ba6..b32f7b42c1f 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 @@ -39,11 +39,34 @@ public class InMemoryWebSessionStore implements WebSessionStore { private static final IdGenerator idGenerator = new JdkIdGenerator(); + private Clock clock = Clock.system(ZoneId.of("GMT")); private final Map sessions = new ConcurrentHashMap<>(); + /** + * Configure the {@link Clock} to use to set lastAccessTime on every created + * session and to calculate if it is expired. + *

This may be useful to align to different timezone or to set the clock + * back in a test, e.g. {@code Clock.offset(clock, Duration.ofMinutes(-31))} + * in order to simulate session expiration. + *

By default this is {@code Clock.system(ZoneId.of("GMT"))}. + * @param clock the clock to use + */ + public void setClock(Clock clock) { + Assert.notNull(clock, "'clock' is required."); + this.clock = clock; + } + + /** + * Return the configured clock for session lastAccessTime calculations. + */ + public Clock getClock() { + return this.clock; + } + + @Override public Mono retrieveSession(String id) { return (this.sessions.containsKey(id) ? Mono.just(this.sessions.get(id)) : Mono.empty()); @@ -70,27 +93,6 @@ public class InMemoryWebSessionStore implements WebSessionStore { }); } - /** - * Configure the {@link Clock} to use to set lastAccessTime on every created - * session and to calculate if it is expired. - *

This may be useful to align to different timezone or to set the clock - * back in a test, e.g. {@code Clock.offset(clock, Duration.ofMinutes(-31))} - * in order to simulate session expiration. - *

By default this is {@code Clock.system(ZoneId.of("GMT"))}. - * @param clock the clock to use - */ - public void setClock(Clock clock) { - Assert.notNull(clock, "'clock' is required."); - this.clock = clock; - } - - /** - * Return the configured clock for session lastAccessTime calculations. - */ - public Clock getClock() { - return this.clock; - } - private Mono changeSessionId(String oldId, WebSession session) { this.sessions.remove(oldId); this.sessions.put(session.getId(), session); diff --git a/spring-web/src/main/java/org/springframework/web/server/session/WebSessionStore.java b/spring-web/src/main/java/org/springframework/web/server/session/WebSessionStore.java index 26c064153b4..4ff61176b39 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/WebSessionStore.java +++ b/spring-web/src/main/java/org/springframework/web/server/session/WebSessionStore.java @@ -19,8 +19,6 @@ import reactor.core.publisher.Mono; import org.springframework.web.server.WebSession; -import java.time.Instant; - /** * Strategy for {@link WebSession} persistence. * @@ -31,8 +29,12 @@ import java.time.Instant; public interface WebSessionStore { /** - * Creates the WebSession that can be stored by this WebSessionStore. - * @return the session + * Create a new WebSession. + *

Note that this does nothing more than create a new instance. + * The session can later be started explicitly via {@link WebSession#start()} + * or implicitly by adding attributes -- and then persisted via + * {@link WebSession#save()}. + * @return the created session instance */ Mono createWebSession(); @@ -51,7 +53,7 @@ public interface WebSessionStore { Mono removeSession(String sessionId); /** - * Update the last accessed time to now. + * Update the last accessed timestamp to "now". * @param webSession the session to update * @return the session with the updated last access time */ From 2fc2dab2302afb6e558c7a0e0c172291e1fca217 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 6 Sep 2017 11:43:14 -0400 Subject: [PATCH 6/6] Fold DefaultWebSession within InMemoryWebSessionStore InMemoryWebSessionStore is very closely associated to DefaultWebSession passing it to it several fields and functions. Now that the store also creates the session, it makes sense to bring the latter in as an inner, nested class. Issue: SPR-15875, 15876 --- .../web/server/session/DefaultWebSession.java | 176 ------------------ .../session/InMemoryWebSessionStore.java | 120 ++++++++++-- ...java => InMemoryWebSessionStoreTests.java} | 39 ++-- .../session/WebSessionIntegrationTests.java | 6 +- 4 files changed, 130 insertions(+), 211 deletions(-) delete mode 100644 spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSession.java rename spring-web/src/test/java/org/springframework/web/server/session/{DefaultWebSessionTests.java => InMemoryWebSessionStoreTests.java} (58%) diff --git a/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSession.java b/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSession.java deleted file mode 100644 index ec6c17b8e7a..00000000000 --- a/spring-web/src/main/java/org/springframework/web/server/session/DefaultWebSession.java +++ /dev/null @@ -1,176 +0,0 @@ -/* - * Copyright 2002-2017 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.web.server.session; - -import java.time.Clock; -import java.time.Duration; -import java.time.Instant; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicReference; -import java.util.function.BiFunction; -import java.util.function.Function; - -import reactor.core.publisher.Mono; - -import org.springframework.util.Assert; -import org.springframework.util.IdGenerator; -import org.springframework.web.server.WebSession; - -/** - * Default implementation of {@link org.springframework.web.server.WebSession}. - * - * @author Rossen Stoyanchev - * @since 5.0 - */ -class DefaultWebSession implements WebSession { - - private final AtomicReference id; - - private final IdGenerator idGenerator; - - private final Map attributes; - - private final Clock clock; - - private final BiFunction> changeIdOperation; - - private final Function> saveOperation; - - private final Instant creationTime; - - private final Instant lastAccessTime; - - private volatile Duration maxIdleTime; - - private volatile State state; - - - /** - * Constructor for creating a new session instance. - * @param idGenerator the session id generator - * @param clock for access to current time - */ - DefaultWebSession(IdGenerator idGenerator, Clock clock, - BiFunction> changeIdOperation, - Function> saveOperation) { - - Assert.notNull(idGenerator, "'idGenerator' is required."); - Assert.notNull(clock, "'clock' is required."); - Assert.notNull(changeIdOperation, "'changeIdOperation' is required."); - Assert.notNull(saveOperation, "'saveOperation' is required."); - - this.id = new AtomicReference<>(String.valueOf(idGenerator.generateId())); - this.idGenerator = idGenerator; - this.clock = clock; - this.changeIdOperation = changeIdOperation; - this.saveOperation = saveOperation; - this.attributes = new ConcurrentHashMap<>(); - this.creationTime = Instant.now(clock); - this.lastAccessTime = this.creationTime; - this.maxIdleTime = Duration.ofMinutes(30); - this.state = State.NEW; - } - - /** - * Copy constructor to re-create a session at the start of a new request - * refreshing the last access time of the session. - * @param existingSession the existing session to copy - * @param lastAccessTime the new last access time - */ - DefaultWebSession(DefaultWebSession existingSession, Instant lastAccessTime) { - this.id = existingSession.id; - this.idGenerator = existingSession.idGenerator; - this.attributes = existingSession.attributes; - this.clock = existingSession.clock; - this.changeIdOperation = existingSession.changeIdOperation; - this.saveOperation = existingSession.saveOperation; - this.creationTime = existingSession.creationTime; - this.lastAccessTime = lastAccessTime; - this.maxIdleTime = existingSession.maxIdleTime; - this.state = existingSession.isStarted() ? State.STARTED : State.NEW; - } - - - @Override - public String getId() { - return this.id.get(); - } - - @Override - public Map getAttributes() { - return this.attributes; - } - - @Override - public Instant getCreationTime() { - return this.creationTime; - } - - @Override - public Instant getLastAccessTime() { - return this.lastAccessTime; - } - - /** - *

By default this is set to 30 minutes. - * @param maxIdleTime the max idle time - */ - @Override - public void setMaxIdleTime(Duration maxIdleTime) { - this.maxIdleTime = maxIdleTime; - } - - @Override - public Duration getMaxIdleTime() { - return this.maxIdleTime; - } - - - @Override - public void start() { - this.state = State.STARTED; - } - - @Override - public boolean isStarted() { - State value = this.state; - return (State.STARTED.equals(value) || (State.NEW.equals(value) && !getAttributes().isEmpty())); - } - - @Override - public Mono changeSessionId() { - String oldId = this.id.get(); - String newId = String.valueOf(this.idGenerator.generateId()); - this.id.set(newId); - return this.changeIdOperation.apply(oldId, this).doOnError(ex -> this.id.set(oldId)); - } - - @Override - public Mono save() { - return this.saveOperation.apply(this); - } - - @Override - public boolean isExpired() { - return (isStarted() && !this.maxIdleTime.isNegative() && - Instant.now(this.clock).minus(this.maxIdleTime).isAfter(this.lastAccessTime)); - } - - - private enum State { NEW, STARTED } - -} 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 b32f7b42c1f..3b833490f9e 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 @@ -16,16 +16,18 @@ package org.springframework.web.server.session; import java.time.Clock; +import java.time.Duration; import java.time.Instant; import java.time.ZoneId; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicReference; + +import reactor.core.publisher.Mono; import org.springframework.util.Assert; import org.springframework.util.IdGenerator; import org.springframework.util.JdkIdGenerator; -import reactor.core.publisher.Mono; - import org.springframework.web.server.WebSession; /** @@ -67,6 +69,11 @@ public class InMemoryWebSessionStore implements WebSessionStore { } + @Override + public Mono createWebSession() { + return Mono.fromSupplier(InMemoryWebSession::new); + } + @Override public Mono retrieveSession(String id) { return (this.sessions.containsKey(id) ? Mono.just(this.sessions.get(id)) : Mono.empty()); @@ -78,21 +85,16 @@ public class InMemoryWebSessionStore implements WebSessionStore { return Mono.empty(); } - public Mono createWebSession() { - return Mono.fromSupplier(() -> - new DefaultWebSession(idGenerator, getClock(), - (oldId, session) -> this.changeSessionId(oldId, session), - this::storeSession)); - } - public Mono updateLastAccessTime(WebSession webSession) { return Mono.fromSupplier(() -> { - DefaultWebSession session = (DefaultWebSession) webSession; + InMemoryWebSession session = (InMemoryWebSession) webSession; Instant lastAccessTime = Instant.now(getClock()); - return new DefaultWebSession(session, lastAccessTime); + return new InMemoryWebSession(session, lastAccessTime); }); } + /* Private methods for InMemoryWebSession */ + private Mono changeSessionId(String oldId, WebSession session) { this.sessions.remove(oldId); this.sessions.put(session.getId(), session); @@ -103,4 +105,100 @@ public class InMemoryWebSessionStore implements WebSessionStore { this.sessions.put(session.getId(), session); return Mono.empty(); } + + + private class InMemoryWebSession implements WebSession { + + private final AtomicReference id; + + private final Map attributes; + + private final Instant creationTime; + + private final Instant lastAccessTime; + + private volatile Duration maxIdleTime; + + private volatile boolean started; + + + InMemoryWebSession() { + this.id = new AtomicReference<>(String.valueOf(idGenerator.generateId())); + this.attributes = new ConcurrentHashMap<>(); + this.creationTime = Instant.now(getClock()); + this.lastAccessTime = this.creationTime; + this.maxIdleTime = Duration.ofMinutes(30); + } + + InMemoryWebSession(InMemoryWebSession existingSession, Instant lastAccessTime) { + this.id = existingSession.id; + this.attributes = existingSession.attributes; + this.creationTime = existingSession.creationTime; + this.lastAccessTime = lastAccessTime; + this.maxIdleTime = existingSession.maxIdleTime; + this.started = existingSession.isStarted(); // Use method (explicit or implicit start) + } + + + @Override + public String getId() { + return this.id.get(); + } + + @Override + public Map getAttributes() { + return this.attributes; + } + + @Override + public Instant getCreationTime() { + return this.creationTime; + } + + @Override + public Instant getLastAccessTime() { + return this.lastAccessTime; + } + + @Override + public void setMaxIdleTime(Duration maxIdleTime) { + this.maxIdleTime = maxIdleTime; + } + + @Override + public Duration getMaxIdleTime() { + return this.maxIdleTime; + } + + + @Override + public void start() { + this.started = true; + } + + @Override + public boolean isStarted() { + return this.started || !getAttributes().isEmpty(); + } + + @Override + public Mono changeSessionId() { + String oldId = this.id.get(); + String newId = String.valueOf(idGenerator.generateId()); + this.id.set(newId); + return InMemoryWebSessionStore.this.changeSessionId(oldId, this).doOnError(ex -> this.id.set(oldId)); + } + + @Override + public Mono save() { + return InMemoryWebSessionStore.this.storeSession(this); + } + + @Override + public boolean isExpired() { + return (isStarted() && !this.maxIdleTime.isNegative() && + Instant.now(getClock()).minus(this.maxIdleTime).isAfter(this.lastAccessTime)); + } + } + } diff --git a/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionTests.java b/spring-web/src/test/java/org/springframework/web/server/session/InMemoryWebSessionStoreTests.java similarity index 58% rename from spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionTests.java rename to spring-web/src/test/java/org/springframework/web/server/session/InMemoryWebSessionStoreTests.java index a3aa6635ac2..efc92a315ae 100644 --- a/spring-web/src/test/java/org/springframework/web/server/session/DefaultWebSessionTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/session/InMemoryWebSessionStoreTests.java @@ -16,59 +16,58 @@ package org.springframework.web.server.session; import org.junit.Test; -import org.springframework.util.IdGenerator; -import org.springframework.util.JdkIdGenerator; -import reactor.core.publisher.Mono; -import java.time.Clock; -import java.time.ZoneId; +import org.springframework.web.server.WebSession; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; /** + * Unit tests. * @author Rob Winch - * @since 5.0 */ -public class DefaultWebSessionTests { - private static final Clock CLOCK = Clock.system(ZoneId.of("GMT")); +public class InMemoryWebSessionStoreTests { + + private InMemoryWebSessionStore sessionStore = new InMemoryWebSessionStore(); - private static final IdGenerator idGenerator = new JdkIdGenerator(); @Test public void constructorWhenImplicitStartCopiedThenCopyIsStarted() { - DefaultWebSession original = createDefaultWebSession(); + WebSession original = this.sessionStore.createWebSession().block(); + assertNotNull(original); original.getAttributes().put("foo", "bar"); - DefaultWebSession copy = new DefaultWebSession(original, CLOCK.instant()); - + WebSession copy = this.sessionStore.updateLastAccessTime(original).block(); + assertNotNull(copy); assertTrue(copy.isStarted()); } @Test public void constructorWhenExplicitStartCopiedThenCopyIsStarted() { - DefaultWebSession original = createDefaultWebSession(); + WebSession original = this.sessionStore.createWebSession().block(); + assertNotNull(original); original.start(); - DefaultWebSession copy = new DefaultWebSession(original, CLOCK.instant()); - + WebSession copy = this.sessionStore.updateLastAccessTime(original).block(); + assertNotNull(copy); assertTrue(copy.isStarted()); } @Test public void startsSessionExplicitly() { - DefaultWebSession session = createDefaultWebSession(); + WebSession session = this.sessionStore.createWebSession().block(); + assertNotNull(session); session.start(); assertTrue(session.isStarted()); } @Test public void startsSessionImplicitly() { - DefaultWebSession session = createDefaultWebSession(); + WebSession session = this.sessionStore.createWebSession().block(); + assertNotNull(session); + session.start(); session.getAttributes().put("foo", "bar"); assertTrue(session.isStarted()); } - private DefaultWebSession createDefaultWebSession() { - return new DefaultWebSession(idGenerator, CLOCK, (s, session) -> Mono.empty(), s -> Mono.empty()); - } } \ No newline at end of file diff --git a/spring-web/src/test/java/org/springframework/web/server/session/WebSessionIntegrationTests.java b/spring-web/src/test/java/org/springframework/web/server/session/WebSessionIntegrationTests.java index 5c06d7b665f..48958d75ada 100644 --- a/spring-web/src/test/java/org/springframework/web/server/session/WebSessionIntegrationTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/session/WebSessionIntegrationTests.java @@ -111,11 +111,9 @@ public class WebSessionIntegrationTests extends AbstractHttpHandlerIntegrationTe // Now set the clock of the session back by 31 minutes InMemoryWebSessionStore store = (InMemoryWebSessionStore) this.sessionManager.getSessionStore(); - DefaultWebSession session = (DefaultWebSession) store.retrieveSession(id).block(); + WebSession session = store.retrieveSession(id).block(); assertNotNull(session); - Instant lastAccessTime = Clock.offset(store.getClock(), Duration.ofMinutes(-31)).instant(); - session = new DefaultWebSession(session, lastAccessTime); - session.save().block(); + store.setClock(Clock.offset(store.getClock(), Duration.ofMinutes(31))); // Third request: expired session, new session created request = RequestEntity.get(createUri()).header("Cookie", "SESSION=" + id).build();