From 4e884cdaf285ed1759a67f12a84c41e67ba01b10 Mon Sep 17 00:00:00 2001 From: Andreas Kasparek Date: Mon, 23 Aug 2021 16:21:31 +0200 Subject: [PATCH] Always set time-to-live within entry processor Closes gh-1899 --- ...elcast4IndexedSessionRepositoryITests.java | 50 +++++++ ...zelcast4IndexedSessionRepositoryTests.java | 25 +++- .../Hazelcast4IndexedSessionRepository.java | 13 +- ...Hazelcast4SessionUpdateEntryProcessor.java | 11 +- ...zelcast4IndexedSessionRepositoryTests.java | 4 +- ...lcast4SessionUpdateEntryProcessorTest.java | 126 ++++++++++++++++++ 6 files changed, 206 insertions(+), 23 deletions(-) create mode 100644 spring-session-hazelcast/hazelcast4/src/test/java/org/springframework/session/hazelcast/Hazelcast4SessionUpdateEntryProcessorTest.java diff --git a/spring-session-hazelcast/hazelcast4/src/integration-test/java/org/springframework/session/hazelcast/AbstractHazelcast4IndexedSessionRepositoryITests.java b/spring-session-hazelcast/hazelcast4/src/integration-test/java/org/springframework/session/hazelcast/AbstractHazelcast4IndexedSessionRepositoryITests.java index 482b0b38..f38d7647 100644 --- a/spring-session-hazelcast/hazelcast4/src/integration-test/java/org/springframework/session/hazelcast/AbstractHazelcast4IndexedSessionRepositoryITests.java +++ b/spring-session-hazelcast/hazelcast4/src/integration-test/java/org/springframework/session/hazelcast/AbstractHazelcast4IndexedSessionRepositoryITests.java @@ -16,6 +16,9 @@ package org.springframework.session.hazelcast; +import java.time.Duration; +import java.time.Instant; + import com.hazelcast.core.HazelcastInstance; import com.hazelcast.map.IMap; import org.junit.jupiter.api.Test; @@ -198,4 +201,51 @@ abstract class AbstractHazelcast4IndexedSessionRepositoryITests { this.repository.deleteById(sessionId); } + @Test + void createAndUpdateSessionWhileKeepingOriginalTimeToLiveConfiguredOnRepository() { + final Duration defaultSessionTimeout = Duration.ofSeconds(1800); + + final IMap hazelcastMap = this.hazelcastInstance + .getMap(Hazelcast4IndexedSessionRepository.DEFAULT_SESSION_MAP_NAME); + + HazelcastSession session = this.repository.createSession(); + String sessionId = session.getId(); + this.repository.save(session); + + assertThat(session.getMaxInactiveInterval()).isEqualTo(defaultSessionTimeout); + assertThat(hazelcastMap.getEntryView(sessionId).getTtl()).isEqualTo(defaultSessionTimeout.toMillis()); + + session = this.repository.findById(sessionId); + session.setLastAccessedTime(Instant.now()); + this.repository.save(session); + + session = this.repository.findById(sessionId); + assertThat(session.getMaxInactiveInterval()).isEqualTo(defaultSessionTimeout); + assertThat(hazelcastMap.getEntryView(sessionId).getTtl()).isEqualTo(defaultSessionTimeout.toMillis()); + } + + @Test + void createAndUpdateSessionWhileKeepingTimeToLiveSetOnSession() { + final Duration individualSessionTimeout = Duration.ofSeconds(23); + + final IMap hazelcastMap = this.hazelcastInstance + .getMap(Hazelcast4IndexedSessionRepository.DEFAULT_SESSION_MAP_NAME); + + HazelcastSession session = this.repository.createSession(); + session.setMaxInactiveInterval(individualSessionTimeout); + String sessionId = session.getId(); + this.repository.save(session); + + assertThat(session.getMaxInactiveInterval()).isEqualTo(individualSessionTimeout); + assertThat(hazelcastMap.getEntryView(sessionId).getTtl()).isEqualTo(individualSessionTimeout.toMillis()); + + session = this.repository.findById(sessionId); + session.setAttribute("attribute", "value"); + this.repository.save(session); + + session = this.repository.findById(sessionId); + assertThat(session.getMaxInactiveInterval()).isEqualTo(individualSessionTimeout); + assertThat(hazelcastMap.getEntryView(sessionId).getTtl()).isEqualTo(individualSessionTimeout.toMillis()); + } + } diff --git a/spring-session-hazelcast/hazelcast4/src/integration-test/java/org/springframework/session/hazelcast/SessionEventHazelcast4IndexedSessionRepositoryTests.java b/spring-session-hazelcast/hazelcast4/src/integration-test/java/org/springframework/session/hazelcast/SessionEventHazelcast4IndexedSessionRepositoryTests.java index 087ab814..b761a3b0 100644 --- a/spring-session-hazelcast/hazelcast4/src/integration-test/java/org/springframework/session/hazelcast/SessionEventHazelcast4IndexedSessionRepositoryTests.java +++ b/spring-session-hazelcast/hazelcast4/src/integration-test/java/org/springframework/session/hazelcast/SessionEventHazelcast4IndexedSessionRepositoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2020 the original author or authors. + * Copyright 2014-2021 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. @@ -199,6 +199,29 @@ class SessionEventHazelcast4IndexedSessionRepositoryTests { assertThat(this.repository.findById(sessionToUpdate.getId())).isNull(); } + @Test // gh-1899 + void updateSessionAndExpireAfterOriginalTimeToLiveTest() throws InterruptedException { + S sessionToSave = this.repository.createSession(); + this.repository.save(sessionToSave); + + assertThat(this.registry.receivedEvent(sessionToSave.getId())).isTrue(); + assertThat(this.registry.getEvent(sessionToSave.getId())) + .isInstanceOf(SessionCreatedEvent.class); + this.registry.clear(); + + S sessionToUpdate = this.repository.findById(sessionToSave.getId()); + sessionToUpdate.setLastAccessedTime(Instant.now()); + this.repository.save(sessionToUpdate); + + assertThat(this.registry.receivedEvent(sessionToUpdate.getId())).isTrue(); + assertThat(this.registry.getEvent(sessionToUpdate.getId())) + .isInstanceOf(SessionExpiredEvent.class); + // Assert this after the expired event was received because it would otherwise do + // its own expiration check and explicitly delete the session from Hazelcast + // regardless of the TTL of the IMap entry. + assertThat(this.repository.findById(sessionToUpdate.getId())).isNull(); + } + @Configuration @EnableHazelcastHttpSession(maxInactiveIntervalInSeconds = MAX_INACTIVE_INTERVAL_IN_SECONDS) static class HazelcastSessionConfig { diff --git a/spring-session-hazelcast/hazelcast4/src/main/java/org/springframework/session/hazelcast/Hazelcast4IndexedSessionRepository.java b/spring-session-hazelcast/hazelcast4/src/main/java/org/springframework/session/hazelcast/Hazelcast4IndexedSessionRepository.java index ddb4d13f..8ca681d6 100644 --- a/spring-session-hazelcast/hazelcast4/src/main/java/org/springframework/session/hazelcast/Hazelcast4IndexedSessionRepository.java +++ b/spring-session-hazelcast/hazelcast4/src/main/java/org/springframework/session/hazelcast/Hazelcast4IndexedSessionRepository.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2020 the original author or authors. + * Copyright 2014-2021 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. @@ -54,7 +54,6 @@ import org.springframework.session.events.SessionCreatedEvent; import org.springframework.session.events.SessionDeletedEvent; import org.springframework.session.events.SessionExpiredEvent; import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; /** * A {@link org.springframework.session.SessionRepository} implementation using Hazelcast @@ -128,8 +127,6 @@ public class Hazelcast4IndexedSessionRepository private static final String SPRING_SECURITY_CONTEXT = "SPRING_SECURITY_CONTEXT"; - private static final boolean SUPPORTS_SET_TTL = ClassUtils.hasAtLeastOneMethodWithName(IMap.class, "setTtl"); - private static final Log logger = LogFactory.getLog(Hazelcast4IndexedSessionRepository.class); private final HazelcastInstance hazelcastInstance; @@ -262,9 +259,6 @@ public class Hazelcast4IndexedSessionRepository entryProcessor.setLastAccessedTime(session.getLastAccessedTime()); } if (session.maxInactiveIntervalChanged) { - if (SUPPORTS_SET_TTL) { - updateTtl(session); - } entryProcessor.setMaxInactiveInterval(session.getMaxInactiveInterval()); } if (!session.delta.isEmpty()) { @@ -275,10 +269,6 @@ public class Hazelcast4IndexedSessionRepository session.clearChangeFlags(); } - private void updateTtl(HazelcastSession session) { - this.sessions.setTtl(session.getId(), session.getMaxInactiveInterval().getSeconds(), TimeUnit.SECONDS); - } - @Override public HazelcastSession findById(String id) { MapSession saved = this.sessions.get(id); @@ -416,6 +406,7 @@ public class Hazelcast4IndexedSessionRepository @Override public void setMaxInactiveInterval(Duration interval) { + Assert.notNull(interval, "interval must not be null"); this.delegate.setMaxInactiveInterval(interval); this.maxInactiveIntervalChanged = true; flushImmediateIfNecessary(); diff --git a/spring-session-hazelcast/hazelcast4/src/main/java/org/springframework/session/hazelcast/Hazelcast4SessionUpdateEntryProcessor.java b/spring-session-hazelcast/hazelcast4/src/main/java/org/springframework/session/hazelcast/Hazelcast4SessionUpdateEntryProcessor.java index 947d9be8..eb29a74e 100644 --- a/spring-session-hazelcast/hazelcast4/src/main/java/org/springframework/session/hazelcast/Hazelcast4SessionUpdateEntryProcessor.java +++ b/spring-session-hazelcast/hazelcast4/src/main/java/org/springframework/session/hazelcast/Hazelcast4SessionUpdateEntryProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2020 the original author or authors. + * Copyright 2014-2021 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. @@ -63,13 +63,8 @@ public class Hazelcast4SessionUpdateEntryProcessor implements EntryProcessor) entry).setValue(value, this.maxInactiveInterval.getSeconds(), - TimeUnit.SECONDS); - } - else { - entry.setValue(value); - } + ((ExtendedMapEntry) entry).setValue(value, value.getMaxInactiveInterval().getSeconds(), + TimeUnit.SECONDS); return Boolean.TRUE; } diff --git a/spring-session-hazelcast/hazelcast4/src/test/java/org/springframework/session/hazelcast/Hazelcast4IndexedSessionRepositoryTests.java b/spring-session-hazelcast/hazelcast4/src/test/java/org/springframework/session/hazelcast/Hazelcast4IndexedSessionRepositoryTests.java index c1337ae9..8cc271a0 100644 --- a/spring-session-hazelcast/hazelcast4/src/test/java/org/springframework/session/hazelcast/Hazelcast4IndexedSessionRepositoryTests.java +++ b/spring-session-hazelcast/hazelcast4/src/test/java/org/springframework/session/hazelcast/Hazelcast4IndexedSessionRepositoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2020 the original author or authors. + * Copyright 2014-2021 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. @@ -46,7 +46,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isA; @@ -254,7 +253,6 @@ class Hazelcast4IndexedSessionRepositoryTests { session.setMaxInactiveInterval(Duration.ofSeconds(1)); verify(this.sessions, times(1)).set(eq(sessionId), eq(session.getDelegate()), isA(Long.class), eq(TimeUnit.SECONDS)); - verify(this.sessions).setTtl(eq(sessionId), anyLong(), any()); verify(this.sessions, times(1)).executeOnKey(eq(sessionId), any(EntryProcessor.class)); this.repository.save(session); diff --git a/spring-session-hazelcast/hazelcast4/src/test/java/org/springframework/session/hazelcast/Hazelcast4SessionUpdateEntryProcessorTest.java b/spring-session-hazelcast/hazelcast4/src/test/java/org/springframework/session/hazelcast/Hazelcast4SessionUpdateEntryProcessorTest.java new file mode 100644 index 00000000..eec0b19b --- /dev/null +++ b/spring-session-hazelcast/hazelcast4/src/test/java/org/springframework/session/hazelcast/Hazelcast4SessionUpdateEntryProcessorTest.java @@ -0,0 +1,126 @@ +/* + * Copyright 2014-2021 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 + * + * https://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.session.hazelcast; + +import java.time.Duration; +import java.time.Instant; +import java.util.HashMap; +import java.util.concurrent.TimeUnit; + +import com.hazelcast.map.ExtendedMapEntry; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.session.MapSession; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +class Hazelcast4SessionUpdateEntryProcessorTest { + + private Hazelcast4SessionUpdateEntryProcessor processor; + + @BeforeEach + void setUp() { + this.processor = new Hazelcast4SessionUpdateEntryProcessor(); + } + + @Test + void shouldReturnFalseIfNoSessionExistsInHazelcastMapEntry() { + @SuppressWarnings("unchecked") + ExtendedMapEntry mapEntry = mock(ExtendedMapEntry.class); + + Object result = this.processor.process(mapEntry); + + assertThat(result).isEqualTo(Boolean.FALSE); + } + + @Test + void shouldUpdateMaxInactiveIntervalOnSessionAndSetMapEntryValueWithNewTimeToLive() { + Duration newMaxInactiveInterval = Duration.ofSeconds(123L); + MapSession mapSession = new MapSession(); + @SuppressWarnings("unchecked") + ExtendedMapEntry mapEntry = mock(ExtendedMapEntry.class); + given(mapEntry.getValue()).willReturn(mapSession); + + this.processor.setMaxInactiveInterval(newMaxInactiveInterval); + Object result = this.processor.process(mapEntry); + + assertThat(result).isEqualTo(Boolean.TRUE); + assertThat(mapSession.getMaxInactiveInterval()).isEqualTo(newMaxInactiveInterval); + verify(mapEntry).setValue(mapSession, newMaxInactiveInterval.getSeconds(), TimeUnit.SECONDS); + } + + @Test + void shouldSetMapEntryValueWithOldTimeToLiveIfNoChangeToMaxInactiveIntervalIsRegistered() { + Duration maxInactiveInterval = Duration.ofSeconds(123L); + MapSession mapSession = new MapSession(); + mapSession.setMaxInactiveInterval(maxInactiveInterval); + @SuppressWarnings("unchecked") + ExtendedMapEntry mapEntry = mock(ExtendedMapEntry.class); + given(mapEntry.getValue()).willReturn(mapSession); + + Object result = this.processor.process(mapEntry); + + assertThat(result).isEqualTo(Boolean.TRUE); + assertThat(mapSession.getMaxInactiveInterval()).isEqualTo(maxInactiveInterval); + verify(mapEntry).setValue(mapSession, maxInactiveInterval.getSeconds(), TimeUnit.SECONDS); + } + + @Test + void shouldUpdateLastAccessTimeOnSessionAndSetMapEntryValueWithOldTimeToLive() { + Instant lastAccessTime = Instant.ofEpochSecond(1234L); + MapSession mapSession = new MapSession(); + @SuppressWarnings("unchecked") + ExtendedMapEntry mapEntry = mock(ExtendedMapEntry.class); + given(mapEntry.getValue()).willReturn(mapSession); + + this.processor.setLastAccessedTime(lastAccessTime); + Object result = this.processor.process(mapEntry); + + assertThat(result).isEqualTo(Boolean.TRUE); + assertThat(mapSession.getLastAccessedTime()).isEqualTo(lastAccessTime); + verify(mapEntry).setValue(mapSession, mapSession.getMaxInactiveInterval().getSeconds(), TimeUnit.SECONDS); + } + + @Test + void shouldUpdateSessionAttributesFromDeltaAndSetMapEntryValueWithOldTimeToLive() { + MapSession mapSession = new MapSession(); + mapSession.setAttribute("changed", "oldValue"); + mapSession.setAttribute("removed", "existingValue"); + @SuppressWarnings("unchecked") + ExtendedMapEntry mapEntry = mock(ExtendedMapEntry.class); + given(mapEntry.getValue()).willReturn(mapSession); + + HashMap delta = new HashMap<>(); + delta.put("added", "addedValue"); + delta.put("changed", "newValue"); + delta.put("removed", null); + this.processor.setDelta(delta); + + Object result = this.processor.process(mapEntry); + + assertThat(result).isEqualTo(Boolean.TRUE); + assertThat((String) mapSession.getAttribute("added")).isEqualTo("addedValue"); + assertThat((String) mapSession.getAttribute("changed")).isEqualTo("newValue"); + assertThat((String) mapSession.getAttribute("removed")).isNull(); + verify(mapEntry).setValue(mapSession, mapSession.getMaxInactiveInterval().getSeconds(), TimeUnit.SECONDS); + } + +}