From 2fc2dab2302afb6e558c7a0e0c172291e1fca217 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 6 Sep 2017 11:43:14 -0400 Subject: [PATCH] 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 ec6c17b8e7..0000000000 --- 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 b32f7b42c1..3b833490f9 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 a3aa6635ac..efc92a315a 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 5c06d7b665..48958d75ad 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();