Clarify behavior of WebSession#save()

+ minor update to the InMemoryWebSession to match the defined behavior.

Issue: SPR-17051

Author: rstoyanchev

spring-web/src/main/java/org/springframework/web/server/WebSession.java

@@ -116,13 +116,19 @@ default <T> T getAttributeOrDefault(String name, T defaultValue) {
 	Mono<Void> invalidate();
 
 	/**
-	 * Save the session persisting attributes (e.g. if stored remotely) and also
-	 * sending the session id to the client if the session is new.
-	 * <p>Note that a session must be started explicitly via {@link #start()} or
-	 * implicitly by adding attributes or otherwise this method has no effect.
+	 * Save the session through the {@code WebSessionStore} as follows:
+	 * <ul>
+	 * <li>If the session is new (i.e. created but never persisted), it must have
+	 * been started explicitly via {@link #start()} or implicitly by adding
+	 * attributes, or otherwise this method should have no effect.
+	 * <li>If the session was retrieved through the {@code WebSessionStore},
+	 * the implementation for this method must check whether the session was
+	 * {@link #invalidate() invalidated} and if so return an error.
+	 * </ul>
+	 * <p>Note that this method is not intended for direct use by applications.
+	 * Instead it is automatically invoked just before the response is
+	 * committed is committed.
 	 * @return {@code Mono} to indicate completion with success or error
-	 * <p>Typically this method should be automatically invoked just before the
-	 * response is committed so applications don't have to by default.
 	 */
 	Mono<Void> save();
 

spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java

@@ -240,17 +240,35 @@ public Mono<Void> invalidate() {
 
 		@Override
 		public Mono<Void> save() {
+
+			checkMaxSessionsLimit();
+
+			// Implicitly started session..
+			if (!getAttributes().isEmpty()) {
+				this.state.compareAndSet(State.NEW, State.STARTED);
+			}
+
+			if (isStarted()) {
+				// Save
+				InMemoryWebSessionStore.this.sessions.put(this.getId(), this);
+
+				// Unless it was invalidated
+				if (this.state.get().equals(State.EXPIRED)) {
+					InMemoryWebSessionStore.this.sessions.remove(this.getId());
+					return Mono.error(new IllegalStateException("Session was invalidated"));
+				}
+			}
+
+			return Mono.empty();
+		}
+
+		private void checkMaxSessionsLimit() {
 			if (sessions.size() >= maxSessions) {
 				expiredSessionChecker.removeExpiredSessions(clock.instant());
 				if (sessions.size() >= maxSessions) {
-					return Mono.error(new IllegalStateException("Max sessions limit reached: " + sessions.size()));
+					throw new IllegalStateException("Max sessions limit reached: " + sessions.size());
 				}
 			}
-			if (!getAttributes().isEmpty()) {
-				this.state.compareAndSet(State.NEW, State.STARTED);
-			}
-			InMemoryWebSessionStore.this.sessions.put(this.getId(), this);
-			return Mono.empty();
 		}
 
 		@Override

spring-web/src/test/java/org/springframework/web/server/session/InMemoryWebSessionStoreTests.java

@@ -83,6 +83,7 @@ public void lastAccessTimeIsUpdatedOnRetrieve() {
 		assertNotNull(session1);
 		String id = session1.getId();
 		Instant time1 = session1.getLastAccessTime();
+		session1.start();
 		session1.save().block();
 
 		// Fast-forward a few seconds
@@ -95,6 +96,34 @@ public void lastAccessTimeIsUpdatedOnRetrieve() {
 		assertTrue(time1.isBefore(time2));
 	}
 
+	@Test // SPR-17051
+	public void sessionInvalidatedBeforeSave() {
+		// Request 1 creates session
+		WebSession session1 = this.store.createWebSession().block();
+		assertNotNull(session1);
+		String id = session1.getId();
+		session1.start();
+		session1.save().block();
+
+		// Request 2 retrieves session
+		WebSession session2 = this.store.retrieveSession(id).block();
+		assertNotNull(session2);
+		assertSame(session1, session2);
+
+		// Request 3 retrieves and invalidates
+		WebSession session3 = this.store.retrieveSession(id).block();
+		assertNotNull(session3);
+		assertSame(session1, session3);
+		session3.invalidate().block();
+
+		// Request 2 saves session after invalidated
+		session2.save().block();
+
+		// Session should not be present
+		WebSession session4 = this.store.retrieveSession(id).block();
+		assertNull(session4);
+	}
+
 	@Test
 	public void expirationCheckPeriod() {