diff --git a/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/AbstractConcurrentSessionOperationsIntegrationTests.java b/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/AbstractConcurrentSessionOperationsIntegrationTests.java new file mode 100644 index 0000000..2e4a93d --- /dev/null +++ b/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/AbstractConcurrentSessionOperationsIntegrationTests.java @@ -0,0 +1,170 @@ +/* + * Copyright 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.session.data.gemfire; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.springframework.session.data.gemfire.AbstractGemFireOperationsSessionRepository.GemFireSession; + +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.Test; + +import edu.umd.cs.mtc.MultithreadedTestCase; +import edu.umd.cs.mtc.TestFramework; + +import org.springframework.session.Session; +import org.springframework.session.SessionRepository; + +/** + * The {@link AbstractConcurrentSessionOperationsIntegrationTests} class is an abstract base class encapsulating + * functionality common to all concurrent {@link Session} operation and access based integration tests. + * + * @author John Blum + * @see org.junit.Test + * @see org.mockito.Mockito + * @see org.springframework.session.Session + * @see org.springframework.session.SessionRepository + * @see edu.umd.cs.mtc.MultithreadedTestCase + * @see edu.umd.cs.mtc.TestFramework + * @since 2.1.0 + */ +public abstract class AbstractConcurrentSessionOperationsIntegrationTests extends AbstractGemFireIntegrationTests { + + @Test + public void concurrentSessionOperationsAreCorrect() throws Throwable { + TestFramework.runOnce(new ConcurrentSessionOperationsTestCase(this)); + } + + protected static class AbstractConcurrentSessionOperationsTestCase extends MultithreadedTestCase { + + private final AbstractConcurrentSessionOperationsIntegrationTests testInstance; + + protected AbstractConcurrentSessionOperationsTestCase( + AbstractConcurrentSessionOperationsIntegrationTests testInstance) { + + assertThat(testInstance).as("Test class instance must not be null").isNotNull(); + + this.testInstance = testInstance; + } + + protected AbstractConcurrentSessionOperationsIntegrationTests getTestInstance() { + return this.testInstance; + } + + @SuppressWarnings("unchecked") + protected > T getSessionRepository() { + return (T) getTestInstance().getSessionRepository(); + } + + protected Session findById(String id) { + return getTestInstance().get(id); + } + + protected Session newSession() { + return getTestInstance().createSession(); + } + + protected T save(T session) { + return getTestInstance().save(session); + } + } + + @SuppressWarnings("unused") + public static class ConcurrentSessionOperationsTestCase extends AbstractConcurrentSessionOperationsTestCase { + + private final AtomicReference sessionId = new AtomicReference<>(null); + + public ConcurrentSessionOperationsTestCase(AbstractConcurrentSessionOperationsIntegrationTests testInstance) { + super(testInstance); + } + + public void thread1() { + + Thread.currentThread().setName("User Session One"); + + assertTick(0); + + Session session = newSession(); + + assertThat(session).isNotNull(); + assertThat(session.getId()).isNotEmpty(); + assertThat(session.isExpired()).isFalse(); + assertThat(session.getAttributeNames()).isEmpty(); + + session.setAttribute("attributeOne", "one"); + session.setAttribute("attributeTwo", "two"); + + save(session); + + this.sessionId.set(session.getId()); + + waitForTick(2); + assertTick(2); + + // Save Session with no changes, no delta + assertThat(session instanceof GemFireSession && ((GemFireSession) session).isDirty()).isFalse(); + + save(session); + } + + public void thread2() { + + Thread.currentThread().setName("User Session Two"); + + waitForTick(1); + assertTick(1); + + Session session = findById(this.sessionId.get()); + + assertThat(session).isNotNull(); + assertThat(session.getId()).isEqualTo(this.sessionId.get()); + assertThat(session.isExpired()).isFalse(); + assertThat(session.getAttributeNames()).containsOnly("attributeOne", "attributeTwo"); + assertThat(session.getAttribute("attributeOne")).isEqualTo("one"); + assertThat(session.getAttribute("attributeTwo")).isEqualTo("two"); + + session.setAttribute("attributeThree", "three"); + + assertThat(session.getAttributeNames()).containsOnly("attributeOne", "attributeTwo", "attributeThree"); + assertThat(session.getAttribute("attributeThree")).isEqualTo("three"); + + save(session); + } + + @Override + public void finish() { + + super.finish(); + + Session session = findById(this.sessionId.get()); + + assertThat(session).isNotNull(); + assertThat(session.getId()).isEqualTo(this.sessionId.get()); + assertThat(session.isExpired()).isFalse(); + assertThat(session.getAttributeNames()).containsOnly("attributeOne", "attributeTwo", "attributeThree"); + assertThat(session.getAttribute("attributeOne")).isEqualTo("one"); + assertThat(session.getAttribute("attributeTwo")).isEqualTo("two"); + assertThat(session.getAttribute("attributeThree")).isEqualTo("three"); + + verify(this.getSessionRepository(), times(2)) + .doSave(eq(session)); + } + } +} diff --git a/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/AbstractGemFireIntegrationTests.java b/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/AbstractGemFireIntegrationTests.java index 93b4731..87adb06 100644 --- a/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/AbstractGemFireIntegrationTests.java +++ b/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/AbstractGemFireIntegrationTests.java @@ -31,6 +31,7 @@ import java.util.Optional; import java.util.stream.Collectors; import org.junit.Before; +import org.mockito.Mockito; import org.apache.geode.cache.DataPolicy; import org.apache.geode.cache.ExpirationAction; @@ -96,9 +97,13 @@ public abstract class AbstractGemFireIntegrationTests extends ForkingClientServe @Before public void setup() { - this.sessionRepository = this.sessionRepository != null - ? this.sessionRepository - : this.gemfireSessionRepository; + this.sessionRepository = this.gemfireSessionRepository != null + ? this.gemfireSessionRepository + : this.sessionRepository; + + this.sessionRepository = Optional.ofNullable(this.sessionRepository) + .map(Mockito::spy) + .orElse(null); } protected static String buildClassPathContainingJarFiles(String... jarFilenames) { diff --git a/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/ConcurrentSessionOperationsUsingClientCachingProxyRegionIntegrationTests.java b/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/ConcurrentSessionOperationsUsingClientCachingProxyRegionIntegrationTests.java new file mode 100644 index 0000000..012c8e7 --- /dev/null +++ b/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/ConcurrentSessionOperationsUsingClientCachingProxyRegionIntegrationTests.java @@ -0,0 +1,150 @@ +/* + * Copyright 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.session.data.gemfire; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.IOException; +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import edu.umd.cs.mtc.TestFramework; + +import org.apache.geode.cache.client.ClientRegionShortcut; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.data.gemfire.config.annotation.CacheServerApplication; +import org.springframework.data.gemfire.config.annotation.ClientCacheApplication; +import org.springframework.session.Session; +import org.springframework.session.data.gemfire.config.annotation.web.http.EnableGemFireHttpSession; +import org.springframework.session.data.gemfire.config.annotation.web.http.GemFireHttpSessionConfiguration; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringRunner; + +/** + * The ConcurrentSessionOperationsUsingClientCachingProxyRegionIntegrationTests class... + * + * @author John Blum + * @since 1.0.0 + */ +@RunWith(SpringRunner.class) +@ContextConfiguration( + classes = ConcurrentSessionOperationsUsingClientCachingProxyRegionIntegrationTests.GemFireClientConfiguration.class +) +public class ConcurrentSessionOperationsUsingClientCachingProxyRegionIntegrationTests + extends AbstractConcurrentSessionOperationsIntegrationTests { + + @Test + public void concurrentCachedSessionAccessIsCorrect() throws Throwable { + TestFramework.runOnce(new ConcurrentCachedSessionAccessTestCase(this)); + } + + @SuppressWarnings("unused") + public static class ConcurrentCachedSessionAccessTestCase extends AbstractConcurrentSessionOperationsTestCase { + + private final AtomicReference sessionId = new AtomicReference<>(null); + + public ConcurrentCachedSessionAccessTestCase( + ConcurrentSessionOperationsUsingClientCachingProxyRegionIntegrationTests testInstance) { + + super(testInstance); + } + + public void thread1() { + + Thread.currentThread().setName("User Session One"); + + assertTick(0); + + Session session = newSession(); + + assertThat(session).isNotNull(); + assertThat(session.getId()).isNotEmpty(); + assertThat(session.isExpired()).isFalse(); + assertThat(session.getAttributeNames()).isEmpty(); + + save(session); + + this.sessionId.set(session.getId()); + + waitForTick(2); + assertTick(2); + + // modify the Session without saving + session.setAttribute("attributeOne", "one"); + session.setAttribute("attributeTwo", "two"); + } + + public void thread2() { + + Thread.currentThread().setName("User Session Two"); + + waitForTick(1); + assertTick(1); + + Session session = findById(this.sessionId.get()); + + assertThat(session).isNotNull(); + assertThat(session.getId()).isEqualTo(this.sessionId.get()); + assertThat(session.isExpired()).isFalse(); + assertThat(session.getAttributeNames()).isEmpty(); + + waitForTick(3); + assertTick(3); + + assertThat(session.getAttributeNames()).containsOnly("attributeOne", "attributeTwo"); + assertThat(session.getAttribute("attributeOne")).isEqualTo("one"); + assertThat(session.getAttribute("attributeTwo")).isEqualTo("two"); + } + } + + @BeforeClass + public static void startGemFireServer() throws IOException { + startGemFireServer(GemFireServerConfiguration.class); + } + + // Tests fail when copyOnRead is set to true. + //@ClientCacheApplication(copyOnRead = true, logLevel = "error", subscriptionEnabled = true) + @ClientCacheApplication(logLevel = "error", subscriptionEnabled = true) + @EnableGemFireHttpSession( + clientRegionShortcut = ClientRegionShortcut.CACHING_PROXY, + poolName = "DEFAULT", + sessionSerializerBeanName = GemFireHttpSessionConfiguration.SESSION_DATA_SERIALIZER_BEAN_NAME + ) + static class GemFireClientConfiguration { } + + @CacheServerApplication( + name = "ConcurrentSessionOperationsUsingClientCachingProxyRegionIntegrationTests", + logLevel = "error" + ) + @EnableGemFireHttpSession( + sessionSerializerBeanName = GemFireHttpSessionConfiguration.SESSION_DATA_SERIALIZER_BEAN_NAME + ) + static class GemFireServerConfiguration { + + public static void main(String[] args) { + + AnnotationConfigApplicationContext applicationContext = + new AnnotationConfigApplicationContext(GemFireServerConfiguration.class); + + applicationContext.registerShutdownHook(); + } + } +} diff --git a/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/ConcurrentSessionOperationsUsingClientLocalRegionIntegrationTests.java b/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/ConcurrentSessionOperationsUsingClientLocalRegionIntegrationTests.java new file mode 100644 index 0000000..ae86afe --- /dev/null +++ b/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/ConcurrentSessionOperationsUsingClientLocalRegionIntegrationTests.java @@ -0,0 +1,120 @@ +/* + * Copyright 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.session.data.gemfire; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import edu.umd.cs.mtc.TestFramework; + +import org.apache.geode.cache.client.ClientRegionShortcut; + +import org.springframework.data.gemfire.config.annotation.ClientCacheApplication; +import org.springframework.session.Session; +import org.springframework.session.data.gemfire.config.annotation.web.http.EnableGemFireHttpSession; +import org.springframework.session.data.gemfire.config.annotation.web.http.GemFireHttpSessionConfiguration; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringRunner; + +/** + * The ConcurrentSessionOperationsUsingClientLocalRegionIntegrationTests class... + * + * @author John Blum + * @since 1.0.0 + */ +@RunWith(SpringRunner.class) +@ContextConfiguration +public class ConcurrentSessionOperationsUsingClientLocalRegionIntegrationTests + extends AbstractConcurrentSessionOperationsIntegrationTests { + + @Test + public void concurrentLocalSessionAccessIsCorrect() throws Throwable { + TestFramework.runOnce(new ConcurrentLocalSessionAccessTestCase(this)); + } + + @SuppressWarnings("unused") + public static class ConcurrentLocalSessionAccessTestCase extends AbstractConcurrentSessionOperationsTestCase { + + private final AtomicReference sessionId = new AtomicReference<>(null); + + public ConcurrentLocalSessionAccessTestCase( + ConcurrentSessionOperationsUsingClientLocalRegionIntegrationTests testInstance) { + + super(testInstance); + } + + public void thread1() { + + Thread.currentThread().setName("User Session One"); + + assertTick(0); + + Session session = newSession(); + + assertThat(session).isNotNull(); + assertThat(session.getId()).isNotEmpty(); + assertThat(session.isExpired()).isFalse(); + assertThat(session.getAttributeNames()).isEmpty(); + + save(session); + + this.sessionId.set(session.getId()); + + waitForTick(2); + assertTick(2); + + // modify the Session without saving + session.setAttribute("attributeOne", "one"); + session.setAttribute("attributeTwo", "two"); + } + + public void thread2() { + + Thread.currentThread().setName("User Session Two"); + + waitForTick(1); + assertTick(1); + + Session session = findById(this.sessionId.get()); + + assertThat(session).isNotNull(); + assertThat(session.getId()).isEqualTo(this.sessionId.get()); + assertThat(session.isExpired()).isFalse(); + assertThat(session.getAttributeNames()).isEmpty(); + + waitForTick(3); + assertTick(3); + + assertThat(session.getAttributeNames()).containsOnly("attributeOne", "attributeTwo"); + assertThat(session.getAttribute("attributeOne")).isEqualTo("one"); + assertThat(session.getAttribute("attributeTwo")).isEqualTo("two"); + } + } + + @ClientCacheApplication(logLevel = "error") + @EnableGemFireHttpSession( + clientRegionShortcut = ClientRegionShortcut.LOCAL, + poolName = "DEFAULT", + sessionSerializerBeanName = GemFireHttpSessionConfiguration.SESSION_DATA_SERIALIZER_BEAN_NAME + ) + static class TestConfiguration { } + +} diff --git a/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/ConcurrentSessionOperationsUsingClientProxyRegionIntegrationTests.java b/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/ConcurrentSessionOperationsUsingClientProxyRegionIntegrationTests.java new file mode 100644 index 0000000..d82f738 --- /dev/null +++ b/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/ConcurrentSessionOperationsUsingClientProxyRegionIntegrationTests.java @@ -0,0 +1,73 @@ +/* + * Copyright 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.session.data.gemfire; + +import java.io.IOException; + +import org.junit.BeforeClass; +import org.junit.runner.RunWith; + +import org.apache.geode.cache.client.ClientRegionShortcut; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.data.gemfire.config.annotation.CacheServerApplication; +import org.springframework.data.gemfire.config.annotation.ClientCacheApplication; +import org.springframework.session.data.gemfire.config.annotation.web.http.EnableGemFireHttpSession; +import org.springframework.session.data.gemfire.config.annotation.web.http.GemFireHttpSessionConfiguration; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringRunner; + +/** + * The ConcurrentSessionOperationsUsingClientProxyRegionIntegrationTests class... + * + * @author John Blum + * @since 1.0.0 + */ +@RunWith(SpringRunner.class) +@ContextConfiguration( + classes = ConcurrentSessionOperationsUsingClientProxyRegionIntegrationTests.GemFireClientConfiguration.class +) +public class ConcurrentSessionOperationsUsingClientProxyRegionIntegrationTests + extends AbstractConcurrentSessionOperationsIntegrationTests { + + @BeforeClass + public static void startGemFireServer() throws IOException { + startGemFireServer(GemFireServerConfiguration.class); + } + + @ClientCacheApplication(logLevel = "error", subscriptionEnabled = true) + @EnableGemFireHttpSession(clientRegionShortcut = ClientRegionShortcut.PROXY, poolName = "DEFAULT") + static class GemFireClientConfiguration { } + + @CacheServerApplication( + name = "ConcurrentSessionOperationsUsingClientProxyRegionIntegrationTests", + logLevel = "error" + ) + @EnableGemFireHttpSession( + sessionSerializerBeanName = GemFireHttpSessionConfiguration.SESSION_DATA_SERIALIZER_BEAN_NAME + ) + static class GemFireServerConfiguration { + + public static void main(String[] args) { + + AnnotationConfigApplicationContext applicationContext = + new AnnotationConfigApplicationContext(GemFireServerConfiguration.class); + + applicationContext.registerShutdownHook(); + } + } +} diff --git a/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/MultiThreadedClientServerHttpSessionAttributesDeltaIntegrationTests.java b/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/MultiThreadedClientServerHttpSessionAttributesDeltaIntegrationTests.java index cd12fb1..32652d8 100644 --- a/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/MultiThreadedClientServerHttpSessionAttributesDeltaIntegrationTests.java +++ b/spring-session-data-geode/src/integration-test/java/org/springframework/session/data/gemfire/MultiThreadedClientServerHttpSessionAttributesDeltaIntegrationTests.java @@ -57,17 +57,17 @@ public class MultiThreadedClientServerHttpSessionAttributesDeltaIntegrationTests @Test public void multiThreadedSessionOperationsAreCorrect() throws Throwable { - TestFramework.runOnce(new MultiThreadedSessionAccessTestCase(this)); + TestFramework.runOnce(new MultiThreadedSessionOperationsTestCase(this)); } @SuppressWarnings("unused") - public static class MultiThreadedSessionAccessTestCase extends MultithreadedTestCase { + public static class MultiThreadedSessionOperationsTestCase extends MultithreadedTestCase { private final AtomicReference sessionId = new AtomicReference<>(null); private final MultiThreadedClientServerHttpSessionAttributesDeltaIntegrationTests testInstance; - public MultiThreadedSessionAccessTestCase( + public MultiThreadedSessionOperationsTestCase( MultiThreadedClientServerHttpSessionAttributesDeltaIntegrationTests testInstance) { this.testInstance = testInstance; @@ -205,7 +205,7 @@ public class MultiThreadedClientServerHttpSessionAttributesDeltaIntegrationTests @Override public void finish() { - Session session = this.testInstance.get(this.sessionId.get()); + Session session = findById(this.sessionId.get()); assertThat(session).isNotNull(); assertThat(session.getId()).isEqualTo(this.sessionId.get()); diff --git a/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/AbstractGemFireOperationsSessionRepository.java b/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/AbstractGemFireOperationsSessionRepository.java index 6d1f9b2..874546c 100644 --- a/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/AbstractGemFireOperationsSessionRepository.java +++ b/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/AbstractGemFireOperationsSessionRepository.java @@ -246,6 +246,17 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi .orElse(0); } + /** + * Gets a reference to the {@link GemfireOperations template} used to perform data access operations + * and other interactions on the cache {@link Region} backing this {@link SessionRepository}. + * + * @return a reference to the {@link GemfireOperations template} used to interact with GemFire/Geode. + * @see org.springframework.data.gemfire.GemfireOperations + */ + public GemfireOperations getTemplate() { + return this.template; + } + /** * Sets a condition indicating whether the DataSerialization framework has been configured. * @@ -264,17 +275,6 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi return usingDataSerialization.get(); } - /** - * Gets a reference to the {@link GemfireOperations template} used to perform data access operations - * and other interactions on the cache {@link Region} backing this {@link SessionRepository}. - * - * @return a reference to the {@link GemfireOperations template} used to interact with GemFire/Geode. - * @see org.springframework.data.gemfire.GemfireOperations - */ - public GemfireOperations getTemplate() { - return this.template; - } - /** * Callback method during Spring bean initialization that will capture the fully-qualified name * of the cache {@link Region} used to manage {@link Session} state and register this {@link SessionRepository} @@ -619,10 +619,11 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi "{ @type = %1$s, id = %2$s, creationTime = %3$s, lastAccessedTime = %4$s, maxInactiveInterval = %5$s, principalName = %6$s }"; private transient boolean delta = false; + private transient boolean dirty = false; private Duration maxInactiveInterval = DEFAULT_MAX_INACTIVE_INTERVAL; - private Instant creationTime; + private final Instant creationTime; private Instant lastAccessedTime; private transient final SpelExpressionParser parser = new SpelExpressionParser(); @@ -636,14 +637,16 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi } protected GemFireSession(String id) { + this.id = validateId(id); this.creationTime = Instant.now(); + this.dirty = true; this.lastAccessedTime = this.creationTime; } protected GemFireSession(Session session) { - Assert.notNull(session, "The Session to copy cannot be null"); + Assert.notNull(session, "The Session to copy must not be null"); this.id = session.getId(); this.creationTime = session.getCreationTime(); @@ -652,27 +655,31 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi this.sessionAttributes.from(session); } - public static GemFireSession copy(Session session) { - return (isUsingDataSerialization() ? new DeltaCapableGemFireSession(session) : new GemFireSession(session)); - } - public static GemFireSession create() { return create(DEFAULT_MAX_INACTIVE_INTERVAL); } public static GemFireSession create(Duration maxInactiveInterval) { - GemFireSession session = - (isUsingDataSerialization() ? new DeltaCapableGemFireSession() : new GemFireSession()); + GemFireSession session = isUsingDataSerialization() + ? new DeltaCapableGemFireSession() + : new GemFireSession(); session.setMaxInactiveInterval(maxInactiveInterval); return session; } + public static GemFireSession copy(Session session) { + + return isUsingDataSerialization() + ? new DeltaCapableGemFireSession(session) + : new GemFireSession(session); + } + @SuppressWarnings("unchecked") public static T from(Session session) { - return (T) (session instanceof GemFireSession ? (GemFireSession) session : copy(session)); + return (T) (session instanceof GemFireSession ? session : copy(session)); } /** @@ -686,10 +693,19 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi } private static String validateId(String id) { - return Optional.ofNullable(id).filter(StringUtils::hasText) + + return Optional.ofNullable(id) + .filter(StringUtils::hasText) .orElseThrow(() -> newIllegalArgumentException("ID is required")); } + /** + * Constructs a new {@link GemFireSessionAttributes} object to store and manage Session attributes. + * + * @param lock {@link Object} used as the mutex for concurrent access and Thread-safety. + * @return the new {@link GemFireSessionAttributes}. + * @see GemFireSessionAttributes + */ @SuppressWarnings("unchecked") protected T newSessionAttributes(Object lock) { return (T) new GemFireSessionAttributes(lock); @@ -700,6 +716,7 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi this.id = generateId(); + markDirty(); triggerDelta(); return getId(); @@ -710,11 +727,10 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi } public synchronized boolean hasDelta() { - return (this.delta || this.sessionAttributes.hasDelta()); + return this.delta || getAttributes().hasDelta(); } - @SuppressWarnings("unused") - protected void triggerDelta() { + protected synchronized void triggerDelta() { triggerDelta(true); } @@ -722,6 +738,23 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi this.delta |= condition; } + public synchronized void commit() { + this.dirty = false; + getAttributes().commit(); + } + + protected synchronized boolean isDirty() { + return this.dirty || getAttributes().isDirty(); + } + + protected synchronized void markDirty() { + markDirty(true); + } + + protected synchronized void markDirty(boolean dirty) { + this.dirty |= dirty; + } + synchronized void setId(String id) { this.id = validateId(id); } @@ -730,24 +763,24 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi return this.id; } - public void setAttribute(String attributeName, Object attributeValue) { - this.sessionAttributes.setAttribute(attributeName, attributeValue); - } - - public void removeAttribute(String attributeName) { - this.sessionAttributes.removeAttribute(attributeName); - } - - public T getAttribute(String attributeName) { - return this.sessionAttributes.getAttribute(attributeName); + public T getAttributes() { + return this.sessionAttributes; } public Set getAttributeNames() { - return this.sessionAttributes.getAttributeNames(); + return getAttributes().getAttributeNames(); } - public T getAttributes() { - return this.sessionAttributes; + public void setAttribute(String attributeName, Object attributeValue) { + getAttributes().setAttribute(attributeName, attributeValue); + } + + public void removeAttribute(String attributeName) { + getAttributes().removeAttribute(attributeName); + } + + public T getAttribute(String attributeName) { + return getAttributes().getAttribute(attributeName); } public synchronized Instant getCreationTime() { @@ -760,12 +793,12 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi Duration maxInactiveInterval = getMaxInactiveInterval(); - return (isExpirationEnabled(maxInactiveInterval) - && Instant.now().minus(maxInactiveInterval).isAfter(lastAccessedTime)); + return isExpirationEnabled(maxInactiveInterval) + && Instant.now().minus(maxInactiveInterval).isAfter(lastAccessedTime); } private boolean isExpirationDisabled(Duration duration) { - return (duration == null || duration.isNegative() || duration.isZero()); + return duration == null || duration.isNegative() || duration.isZero(); } private boolean isExpirationEnabled(Duration duration) { @@ -773,7 +806,12 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi } public synchronized void setLastAccessedTime(Instant lastAccessedTime) { - triggerDelta(!ObjectUtils.nullSafeEquals(this.lastAccessedTime, lastAccessedTime)); + + boolean changed = !ObjectUtils.nullSafeEquals(this.lastAccessedTime, lastAccessedTime); + + markDirty(changed); + triggerDelta(changed); + this.lastAccessedTime = lastAccessedTime; } @@ -782,7 +820,12 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi } public synchronized void setMaxInactiveInterval(Duration maxInactiveIntervalInSeconds) { - triggerDelta(!ObjectUtils.nullSafeEquals(this.maxInactiveInterval, maxInactiveIntervalInSeconds)); + + boolean changed = !ObjectUtils.nullSafeEquals(this.maxInactiveInterval, maxInactiveIntervalInSeconds); + + markDirty(changed); + triggerDelta(changed); + this.maxInactiveInterval = maxInactiveIntervalInSeconds; } @@ -814,6 +857,7 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi } @SuppressWarnings("all") + @Override public int compareTo(Session session) { return getCreationTime().compareTo(session.getCreationTime()); } @@ -847,8 +891,8 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi @Override public synchronized String toString() { - return String.format(GEMFIRE_SESSION_TO_STRING, getClass().getName(), getId(), - getCreationTime(), getLastAccessedTime(), getMaxInactiveInterval(), getPrincipalName()); + return String.format(GEMFIRE_SESSION_TO_STRING, getClass().getName(), getId(), getCreationTime(), + getLastAccessedTime(), getMaxInactiveInterval(), getPrincipalName()); } } @@ -976,6 +1020,8 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi @SuppressWarnings("serial") public static class GemFireSessionAttributes extends AbstractMap { + private volatile transient boolean dirty = false; + private transient final Map sessionAttributes = new HashMap<>(); private transient final Object lock; @@ -1004,14 +1050,26 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi synchronized (getLock()) { return attributeValue != null - ? this.sessionAttributes.put(attributeName, attributeValue) + ? doSetAttribute(attributeName, attributeValue) : removeAttribute(attributeName); } } + private Object doSetAttribute(String attributeName, Object attributeValue) { + + Object previousAttributeValue = this.sessionAttributes.put(attributeName, attributeValue); + + this.dirty |= !attributeValue.equals(previousAttributeValue); + + return previousAttributeValue; + } + public Object removeAttribute(String attributeName) { synchronized (getLock()) { + + this.dirty |= this.sessionAttributes.containsKey(attributeName); + return this.sessionAttributes.remove(attributeName); } } @@ -1052,6 +1110,10 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi public void clearDelta() { } + public void commit() { + this.dirty = false; + } + public void from(Session session) { synchronized (getLock()) { @@ -1079,6 +1141,10 @@ public abstract class AbstractGemFireOperationsSessionRepository extends CacheLi return false; } + public boolean isDirty() { + return this.dirty; + } + @Override public String toString() { return this.sessionAttributes.toString(); diff --git a/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/GemFireOperationsSessionRepository.java b/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/GemFireOperationsSessionRepository.java index 025da22..2065b0a 100644 --- a/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/GemFireOperationsSessionRepository.java +++ b/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/GemFireOperationsSessionRepository.java @@ -19,10 +19,13 @@ package org.springframework.session.data.gemfire; import java.time.Duration; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import org.apache.geode.cache.query.SelectResults; import org.springframework.data.gemfire.GemfireOperations; +import org.springframework.lang.NonNull; +import org.springframework.lang.Nullable; import org.springframework.session.Session; import org.springframework.session.SessionRepository; @@ -109,6 +112,7 @@ public class GemFireOperationsSessionRepository extends AbstractGemFireOperation * @see org.springframework.session.Session * @see #getMaxInactiveIntervalInSeconds() */ + @NonNull public Session createSession() { return GemFireSession.create(getMaxInactiveInterval()); } @@ -124,6 +128,7 @@ public class GemFireOperationsSessionRepository extends AbstractGemFireOperation * @see org.springframework.session.Session * @see #deleteById(String) */ + @Nullable public Session findById(String sessionId) { Session storedSession = getTemplate().get(sessionId); @@ -138,14 +143,35 @@ public class GemFireOperationsSessionRepository extends AbstractGemFireOperation } /** - * Saves the specified {@link Session} to GemFire. + * Saves the specified {@link Session} to Apache Geode or Pivotal GemFire. + * + * Warning, the save method should never be called asynchronously and concurrently, from a separate Thread, + * while the caller continues to modify the given {@link Session} from the forking Thread + * or data loss can occur! There is a reason why this method is blocking! * * @param session the {@link Session} to save. * @see org.springframework.data.gemfire.GemfireOperations#put(Object, Object) * @see org.springframework.session.Session */ - public void save(Session session) { + public void save(@Nullable Session session) { + + Optional.ofNullable(session) + .filter(this::isDirty) + .ifPresent(this::doSave); + } + + private boolean isDirty(@NonNull Session session) { + return !(session instanceof GemFireSession) || ((GemFireSession) session).isDirty(); + } + + /*private*/ void doSave(@NonNull Session session) { + + // Save Session As GemFireSession getTemplate().put(session.getId(), GemFireSession.from(session)); + + if (session instanceof GemFireSession) { + ((GemFireSession) session).commit(); + } } /** diff --git a/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/AbstractGemFireOperationsSessionRepositoryTests.java b/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/AbstractGemFireOperationsSessionRepositoryTests.java index 0c37cff..585796e 100644 --- a/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/AbstractGemFireOperationsSessionRepositoryTests.java +++ b/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/AbstractGemFireOperationsSessionRepositoryTests.java @@ -36,6 +36,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; import static org.springframework.data.gemfire.util.CollectionUtils.asSet; import static org.springframework.session.data.gemfire.AbstractGemFireOperationsSessionRepository.DeltaCapableGemFireSession; import static org.springframework.session.data.gemfire.AbstractGemFireOperationsSessionRepository.DeltaCapableGemFireSessionAttributes; @@ -49,9 +50,11 @@ import java.time.Duration; import java.time.Instant; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.UUID; import java.util.concurrent.TimeUnit; import org.junit.Before; @@ -92,7 +95,6 @@ import org.apache.commons.logging.Log; * Unit tests for {@link AbstractGemFireOperationsSessionRepository}. * * @author John Blum - * @since 1.1.0 * @see org.junit.Test * @see org.junit.runner.RunWith * @see org.mockito.Mock @@ -101,12 +103,12 @@ import org.apache.commons.logging.Log; * @see org.mockito.Spy * @see org.springframework.data.gemfire.GemfireOperations * @see org.springframework.session.Session - * @see org.springframework.session.Session * @see org.springframework.session.data.gemfire.AbstractGemFireOperationsSessionRepository * @see org.springframework.session.data.gemfire.config.annotation.web.http.GemFireHttpSessionConfiguration * @see org.apache.geode.cache.Region * @see edu.umd.cs.mtc.MultithreadedTestCase * @see edu.umd.cs.mtc.TestFramework + * @since 1.1.0 */ @RunWith(MockitoJUnitRunner.class) public class AbstractGemFireOperationsSessionRepositoryTests { @@ -163,6 +165,25 @@ public class AbstractGemFireOperationsSessionRepositoryTests { return mock(RegionAttributes.class, name); } + private Session mockSession() { + + String sessionId = UUID.randomUUID().toString(); + + Instant now = Instant.now(); + + Duration maxInactiveInterval = Duration.ofSeconds(MAX_INACTIVE_INTERVAL_IN_SECONDS); + + Session mockSession = mock(Session.class, withSettings().name(sessionId).lenient()); + + when(mockSession.getId()).thenReturn(sessionId); + when(mockSession.getAttributeNames()).thenReturn(Collections.emptySet()); + when(mockSession.getCreationTime()).thenReturn(now); + when(mockSession.getLastAccessedTime()).thenReturn(now); + when(mockSession.getMaxInactiveInterval()).thenReturn(maxInactiveInterval); + + return mockSession; + } + protected Session mockSession(String sessionId, long creationAndLastAccessedTime, long maxInactiveIntervalInSeconds) { @@ -1118,6 +1139,21 @@ public class AbstractGemFireOperationsSessionRepositoryTests { assertThat(session.getAttributeNames()).isEmpty(); } + @Test(expected = IllegalArgumentException.class) + public void constructGemFireSessionWithNullId() { + + try { + new GemFireSession((String) null); + } + catch (IllegalArgumentException expected) { + + assertThat(expected).hasMessage("ID is required"); + assertThat(expected).hasNoCause(); + + throw expected; + } + } + @Test(expected = IllegalArgumentException.class) public void constructGemFireSessionWithUnspecifiedId() { @@ -1177,7 +1213,7 @@ public class AbstractGemFireOperationsSessionRepositoryTests { } catch (IllegalArgumentException expected) { - assertThat(expected).hasMessage("The Session to copy cannot be null"); + assertThat(expected).hasMessage("The Session to copy must not be null"); assertThat(expected).hasNoCause(); throw expected; @@ -1293,7 +1329,7 @@ public class AbstractGemFireOperationsSessionRepositoryTests { } @Test - public void isExpiredIsFalseWhenMaxInactiveIntervalIsNegative() { + public void isExpiredReturnsFalseWhenMaxInactiveIntervalIsNegative() { Duration expectedMaxInactiveIntervalInSeconds = Duration.ofSeconds(-1); @@ -1305,7 +1341,7 @@ public class AbstractGemFireOperationsSessionRepositoryTests { } @Test - public void isExpiredIsFalseWhenMaxInactiveIntervalIsZero() { + public void isExpiredReturnsFalseWhenMaxInactiveIntervalIsZero() { Duration expectedMaxInactiveIntervalInSeconds = Duration.ZERO; @@ -1317,7 +1353,7 @@ public class AbstractGemFireOperationsSessionRepositoryTests { } @Test - public void isExpiredIsFalseWhenSessionIsActive() { + public void isExpiredReturnsFalseWhenSessionIsActive() { long expectedMaxInactiveIntervalInSeconds = TimeUnit.HOURS.toSeconds(2); @@ -1336,7 +1372,7 @@ public class AbstractGemFireOperationsSessionRepositoryTests { } @Test - public void isExpiredIsTrueWhenSessionIsInactive() { + public void isExpiredReturnsTrueWhenSessionIsInactive() { int expectedMaxInactiveIntervalInSeconds = 60; @@ -1380,12 +1416,12 @@ public class AbstractGemFireOperationsSessionRepositoryTests { } @Test - public void hasDeltaWhenNoSessionChangesIsFalse() { + public void hasDeltaReturnsFalseWhenNoSessionChanges() { assertThat(new AbstractGemFireOperationsSessionRepository.GemFireSession().hasDelta()).isFalse(); } @Test - public void hasDeltaWhenSessionAttributesChangeIsTrue() { + public void hasDeltaReturnsTrueWhenSessionAttributesChange() { GemFireSession session = new DeltaCapableGemFireSession(); @@ -1397,7 +1433,7 @@ public class AbstractGemFireOperationsSessionRepositoryTests { } @Test - public void hasDeltaWhenSessionLastAccessedTimeIsUpdatedIsTrue() { + public void hasDeltaReturnsTrueWhenSessionLastAccessedTimeIsUpdated() { Instant expectedLastAccessTime = Instant.ofEpochMilli(1L); @@ -1419,7 +1455,7 @@ public class AbstractGemFireOperationsSessionRepositoryTests { } @Test - public void hasDeltaWhenSessionMaxInactiveIntervalInSecondsIsUpdatedIsTrue() { + public void hasDeltaReturnsTrueWhenSessionMaxInactiveIntervalInSecondsIsUpdated() { Duration expectedMaxInactiveIntervalInSeconds = Duration.ofSeconds(300L); @@ -1490,7 +1526,188 @@ public class AbstractGemFireOperationsSessionRepositoryTests { } @Test - public void sessionComparisons() { + public void isDirtyReturnsFalseWithExistingSession() { + + GemFireSession session = GemFireSession.from(this.mockSession); + + assertThat(session).isNotNull(); + assertThat(session.isDirty()).isFalse(); + } + + @Test + public void isDirtyReturnsTrueWithNewGemFireSession() { + + GemFireSession session = GemFireSession.create(); + + assertThat(session).isNotNull(); + assertThat(session.isDirty()).isTrue(); + } + + @Test + public void isDirtyReturnsTrueWhenSessionAttributeIsAdded() { + + GemFireSession session = GemFireSession.from(this.mockSession); + + assertThat(session).isNotNull(); + assertThat(session.getAttributeNames()).isEmpty(); + assertThat(session.isDirty()).isFalse(); + + session.setAttribute("attributeOne", "one"); + + assertThat(session.getAttributeNames()).containsExactly("attributeOne"); + assertThat(session.getAttribute("attributeOne")).isEqualTo("one"); + assertThat(session.isDirty()).isTrue(); + + session.commit(); + + assertThat(session.isDirty()); + } + + @Test + public void isDirtyReturnsTrueWhenSessionAttributeIsRemoved() { + + GemFireSession session = GemFireSession.create(); + + assertThat(session).isNotNull(); + + session.setAttribute("attributeOne", "one"); + session.commit(); + + assertThat(session.getAttributeNames()).containsExactly("attributeOne"); + assertThat(session.getAttribute("attributeOne")).isEqualTo("one"); + assertThat(session.isDirty()).isFalse(); + + session.removeAttribute("attributeOne"); + + assertThat(session.getAttributeNames()).isEmpty(); + assertThat(session.getAttribute("attributeOne")).isNull(); + assertThat(session.isDirty()).isTrue(); + + session.commit(); + + assertThat(session.isDirty()); + } + + @Test + public void isDirtyReturnsTrueWhenSessionAttributeIsUpdated() { + + GemFireSession session = GemFireSession.create(); + + assertThat(session).isNotNull(); + + session.setAttribute("attributeOne", "one"); + session.commit(); + + assertThat(session.getAttributeNames()).containsExactly("attributeOne"); + assertThat(session.getAttribute("attributeOne")).isEqualTo("one"); + assertThat(session.isDirty()).isFalse(); + + session.setAttribute("attributeOne", "two"); + + assertThat(session.getAttributeNames()).containsExactly("attributeOne"); + assertThat(session.getAttribute("attributeOne")).isEqualTo("two"); + assertThat(session.isDirty()).isTrue(); + + session.commit(); + + assertThat(session.isDirty()); + } + + @Test + public void isDirtyReturnsTrueWhenSessionIdChanges() { + + GemFireSession session = GemFireSession.from(mockSession()); + + assertThat(session).isNotNull(); + assertThat(session.getId()).isNotEmpty(); + assertThat(session.isDirty()).isFalse(); + + String currentSessionId = session.getId(); + + assertThat(currentSessionId).isNotEmpty(); + assertThat(session.changeSessionId()).isNotEqualTo(currentSessionId); + assertThat(session.isDirty()).isTrue(); + + session.commit(); + + assertThat(session.isDirty()).isFalse(); + } + + @Test + public void isDirtyReturnsTrueWhenSessionLastAccessedTimeChanges() { + + GemFireSession session = GemFireSession.from(mockSession()); + + assertThat(session).isNotNull(); + assertThat(session.isDirty()).isFalse(); + + Instant lastAccessedTime = session.getLastAccessedTime(); + + assertThat(lastAccessedTime).isNotNull(); + + session.setLastAccessedTime(lastAccessedTime.plus(Duration.ofSeconds(5))); + + assertThat(session.getLastAccessedTime()).isAfter(lastAccessedTime); + assertThat(session.isDirty()).isTrue(); + + session.commit(); + + assertThat(session.isDirty()).isFalse(); + } + + @Test + public void isDirtyReturnsTrueWhenSessionMaxInactiveIntervalChanges() { + + GemFireSession session = GemFireSession.from(mockSession()); + + assertThat(session).isNotNull(); + assertThat(session.isDirty()).isFalse(); + + Duration maxInactiveInterval = session.getMaxInactiveInterval(); + + assertThat(maxInactiveInterval).isNotNull(); + + session.setMaxInactiveInterval(maxInactiveInterval.plus(Duration.ofSeconds(5))); + + assertThat(session.getMaxInactiveInterval()).isGreaterThan(maxInactiveInterval); + assertThat(session.isDirty()).isTrue(); + + session.commit(); + + assertThat(session.isDirty()).isFalse(); + } + + @Test + public void isDirtyReturnsTrueWhenSessionIsDirtyAndAttributesAreNotModifiedOnSubsequentUpdate() { + + GemFireSession session = GemFireSession.from(mockSession()); + + assertThat(session).isNotNull(); + assertThat(session.isDirty()).isFalse(); + + String previousSessionId = session.getId(); + + Instant newLastAccessedTime = session.getLastAccessedTime().plusSeconds(5L); + + Duration newMaxLastInactiveInterval = session.getMaxInactiveInterval().plusSeconds(5L); + + assertThat(session.changeSessionId()).isNotEqualTo(previousSessionId); + + session.setAttribute("attributeOne", "testOne"); + session.setLastAccessedTime(newLastAccessedTime); + session.setMaxInactiveInterval(newMaxLastInactiveInterval); + + assertThat(session.isDirty()).isTrue(); + + session.setAttribute("attributeOne", "testOne"); + session.setLastAccessedTime(newLastAccessedTime); + session.setMaxInactiveInterval(newMaxLastInactiveInterval); + + assertThat(session.isDirty()).isTrue(); + } + + @Test + public void sessionCompareTo() { Instant twoHoursAgo = Instant.now().minusMillis(TimeUnit.HOURS.toMillis(2)); @@ -1537,23 +1754,56 @@ public class AbstractGemFireOperationsSessionRepositoryTests { assertThat(session.hashCode()).isNotEqualTo("1".hashCode()); } + @Test @SuppressWarnings("unchecked") + public void sessionToStringContainsId() { + + Session mockSession = mockSession(); + + GemFireSession session = GemFireSession.from(mockSession); + + assertThat(session).isNotNull(); + assertThat(session.getId()).isEqualTo(mockSession.getId()); + assertThat(session.toString()).startsWith(String.format("{ @type = %1$s, id = %2$s", + session.getClass().getName(), session.getId())); + } + + @Test + public void sessionAttributesFromMap() { + + Map source = new HashMap<>(); + + source.put("attrOne", "testOne"); + source.put("attrTwo", "testTwo"); + + GemFireSessionAttributes target = new GemFireSessionAttributes(); + + assertThat(target.getAttributeNames()).isEmpty(); + + target.from(source); + + assertThat(target.getAttributeNames().size()).isEqualTo(2); + assertThat(target.getAttributeNames()).containsOnly("attrOne", "attrTwo"); + assertThat(target.getAttribute("attrOne")).isEqualTo("testOne"); + assertThat(target.getAttribute("attrTwo")).isEqualTo("testTwo"); + } + @Test public void sessionAttributesFromSession() { Session mockSession = mock(Session.class); - given(mockSession.getAttributeNames()).willReturn(asSet("attrOne", "attrTwo")); - given(mockSession.getAttribute(eq("attrOne"))).willReturn("testOne"); - given(mockSession.getAttribute(eq("attrTwo"))).willReturn("testTwo"); + when(mockSession.getAttributeNames()).thenReturn(asSet("attrOne", "attrTwo")); + when(mockSession.getAttribute(eq("attrOne"))).thenReturn("testOne"); + when(mockSession.getAttribute(eq("attrTwo"))).thenReturn("testTwo"); GemFireSessionAttributes sessionAttributes = new GemFireSessionAttributes(); - assertThat(sessionAttributes.getAttributeNames().isEmpty()).isTrue(); + assertThat(sessionAttributes.getAttributeNames()).isEmpty(); sessionAttributes.from(mockSession); assertThat(sessionAttributes.getAttributeNames().size()).isEqualTo(2); - assertThat(sessionAttributes.getAttributeNames().containsAll(asSet("attrOne", "attrTwo"))).isTrue(); + assertThat(sessionAttributes.getAttributeNames()).containsOnly("attrOne", "attrTwo"); assertThat(sessionAttributes.getAttribute("attrOne")).isEqualTo("testOne"); assertThat(sessionAttributes.getAttribute("attrTwo")).isEqualTo("testTwo"); @@ -1565,31 +1815,180 @@ public class AbstractGemFireOperationsSessionRepositoryTests { @Test public void sessionAttributesFromSessionAttributes() { - AbstractGemFireOperationsSessionRepository.GemFireSessionAttributes source = - new AbstractGemFireOperationsSessionRepository.GemFireSessionAttributes(); + GemFireSessionAttributes source = new GemFireSessionAttributes(); source.setAttribute("attrOne", "testOne"); source.setAttribute("attrTwo", "testTwo"); GemFireSessionAttributes target = new GemFireSessionAttributes(); - assertThat(target.getAttributeNames().isEmpty()).isTrue(); + assertThat(target.getAttributeNames()).isEmpty(); target.from(source); assertThat(target.getAttributeNames().size()).isEqualTo(2); - assertThat(target.getAttributeNames().containsAll(asSet("attrOne", "attrTwo"))).isTrue(); + assertThat(target.getAttributeNames()).containsOnly("attrOne", "attrTwo"); assertThat(target.getAttribute("attrOne")).isEqualTo("testOne"); assertThat(target.getAttribute("attrTwo")).isEqualTo("testTwo"); } @Test - public void sessionAttributesHasDeltaIsFalse() { - assertThat(new AbstractGemFireOperationsSessionRepository.GemFireSessionAttributes().hasDelta()).isFalse(); + public void sessionAttributesIsDirtyReturnsTrueOnSetWhenAdd() { + + GemFireSessionAttributes sessionAttributes = new GemFireSessionAttributes(); + + assertThat(sessionAttributes.getAttributeNames()).isEmpty(); + assertThat(sessionAttributes.isDirty()).isFalse(); + + sessionAttributes.setAttribute("attributeOne", "testOne"); + + // Set attribute to the same value again to make sure it does not clear the dirty bit + sessionAttributes.setAttribute("attributeOne", "testOne"); + + assertThat(sessionAttributes.getAttributeNames()).containsExactly("attributeOne"); + assertThat(sessionAttributes.getAttribute("attributeOne")).isEqualTo("testOne"); + assertThat(sessionAttributes.isDirty()).isTrue(); + + sessionAttributes.commit(); + + assertThat(sessionAttributes.isDirty()).isFalse(); } @Test - public void sessionAttributesHasDeltaIsTrue() { + public void sessionAttributesIsDirtyReturnsTrueOnSetWhenModified() { + + GemFireSessionAttributes sessionAttributes = new GemFireSessionAttributes(); + + sessionAttributes.from(Collections.singletonMap("attributeOne", "testOne")); + sessionAttributes.commit(); + + assertThat(sessionAttributes.getAttributeNames()).containsExactly("attributeOne"); + assertThat(sessionAttributes.getAttribute("attributeOne")).isEqualTo("testOne"); + assertThat(sessionAttributes.isDirty()).isFalse(); + + sessionAttributes.setAttribute("attributeOne", "testTwo"); + + assertThat(sessionAttributes.getAttributeNames()).containsExactly("attributeOne"); + assertThat(sessionAttributes.getAttribute("attributeOne")).isEqualTo("testTwo"); + assertThat(sessionAttributes.isDirty()).isTrue(); + + sessionAttributes.commit(); + + assertThat(sessionAttributes.isDirty()).isFalse(); + } + + @Test + public void sessionAttributesIsDirtyReturnsFalseOnSetWhenNotModified() { + + GemFireSessionAttributes sessionAttributes = new GemFireSessionAttributes(); + + sessionAttributes.from(Collections.singletonMap("attributeOne", "testOne")); + sessionAttributes.commit(); + + assertThat(sessionAttributes.getAttributeNames()).containsExactly("attributeOne"); + assertThat(sessionAttributes.getAttribute("attributeOne")).isEqualTo("testOne"); + assertThat(sessionAttributes.isDirty()).isFalse(); + + sessionAttributes.setAttribute("attributeOne", "testOne"); + + assertThat(sessionAttributes.getAttributeNames()).containsExactly("attributeOne"); + assertThat(sessionAttributes.getAttribute("attributeOne")).isEqualTo("testOne"); + assertThat(sessionAttributes.isDirty()).isFalse(); + + sessionAttributes.commit(); + + assertThat(sessionAttributes.isDirty()).isFalse(); + } + + @Test + public void sessionAttributesIsDirtyReturnsTrueOnSetWhenRemoved() { + + GemFireSessionAttributes sessionAttributes = new GemFireSessionAttributes(); + + sessionAttributes.from(Collections.singletonMap("attributeOne", "testOne")); + sessionAttributes.commit(); + + assertThat(sessionAttributes.getAttributeNames()).containsExactly("attributeOne"); + assertThat(sessionAttributes.getAttribute("attributeOne")).isEqualTo("testOne"); + assertThat(sessionAttributes.isDirty()).isFalse(); + + sessionAttributes.setAttribute("attributeOne", null); + + assertThat(sessionAttributes.getAttributeNames()).isEmpty(); + assertThat(sessionAttributes.getAttribute("attributeOne")).isNull(); + assertThat(sessionAttributes.isDirty()).isTrue(); + + sessionAttributes.commit(); + + assertThat(sessionAttributes.isDirty()).isFalse(); + } + + @Test + public void sessionAttributesIsDirtyReturnsTrueOnRemove() { + + GemFireSessionAttributes sessionAttributes = new GemFireSessionAttributes(); + + sessionAttributes.from(Collections.singletonMap("attributeOne", "testOne")); + sessionAttributes.commit(); + + assertThat(sessionAttributes.getAttributeNames()).containsExactly("attributeOne"); + assertThat(sessionAttributes.getAttribute("attributeOne")).isEqualTo("testOne"); + assertThat(sessionAttributes.isDirty()).isFalse(); + + sessionAttributes.removeAttribute("attributeOne"); + + // Remove attribute again to make sure it does not clear the dirty bit + sessionAttributes.removeAttribute("attributeOne"); + + assertThat(sessionAttributes.getAttributeNames()).isEmpty(); + assertThat(sessionAttributes.getAttribute("attributeOne")).isNull(); + assertThat(sessionAttributes.isDirty()).isTrue(); + + sessionAttributes.commit(); + + assertThat(sessionAttributes.isDirty()).isFalse(); + } + + @Test + public void sessionAttributesIsDirtyReturnsFalseOnRemoveForNonExistingAttribute() { + + GemFireSessionAttributes sessionAttributes = new GemFireSessionAttributes(); + + sessionAttributes.from(Collections.singletonMap("attributeOne", "testOne")); + sessionAttributes.commit(); + + assertThat(sessionAttributes.getAttributeNames()).containsExactly("attributeOne"); + assertThat(sessionAttributes.getAttribute("attributeOne")).isEqualTo("testOne"); + assertThat(sessionAttributes.isDirty()).isFalse(); + + sessionAttributes.removeAttribute("nonExistingAttribute"); + + assertThat(sessionAttributes.getAttributeNames()).containsExactly("attributeOne"); + assertThat(sessionAttributes.getAttribute("attributeOne")).isEqualTo("testOne"); + assertThat(sessionAttributes.isDirty()).isFalse(); + } + + @Test + public void sessionAttributesIsDirtyReturnsFalseOnRemoveForNullAttribute() { + + GemFireSessionAttributes sessionAttributes = new GemFireSessionAttributes(); + + assertThat(sessionAttributes.getAttributeNames()).isEmpty(); + assertThat(sessionAttributes.isDirty()).isFalse(); + + sessionAttributes.removeAttribute(null); + + assertThat(sessionAttributes.getAttributeNames()).isEmpty(); + assertThat(sessionAttributes.isDirty()).isFalse(); + } + + @Test + public void sessionAttributesHasDeltaReturnsFalse() { + assertThat(new GemFireSessionAttributes().hasDelta()).isFalse(); + } + + @Test + public void sessionAttributesHasDeltaReturnsTrue() { GemFireSessionAttributes sessionAttributes = new DeltaCapableGemFireSessionAttributes(); @@ -1900,6 +2299,6 @@ public class AbstractGemFireOperationsSessionRepositoryTests { } } - static class Tombstone { - } + static class Tombstone { } + } diff --git a/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/GemFireOperationsSessionRepositoryTests.java b/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/GemFireOperationsSessionRepositoryTests.java index 8c7b035..0db795a 100644 --- a/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/GemFireOperationsSessionRepositoryTests.java +++ b/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/GemFireOperationsSessionRepositoryTests.java @@ -25,16 +25,23 @@ import static org.mockito.ArgumentMatchers.same; import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.willAnswer; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; +import static org.springframework.session.FindByIndexNameSessionRepository.PRINCIPAL_NAME_INDEX_NAME; +import static org.springframework.session.data.gemfire.AbstractGemFireOperationsSessionRepository.GemFireSession; import java.time.Duration; import java.time.Instant; import java.util.Arrays; import java.util.Collections; import java.util.Map; +import java.util.UUID; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -42,6 +49,7 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @@ -53,7 +61,6 @@ import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationEventPublisher; import org.springframework.data.gemfire.GemfireAccessor; import org.springframework.data.gemfire.GemfireOperations; -import org.springframework.session.FindByIndexNameSessionRepository; import org.springframework.session.Session; import org.springframework.session.data.gemfire.support.GemFireUtils; import org.springframework.session.events.AbstractSessionEvent; @@ -63,7 +70,9 @@ import org.springframework.session.events.SessionDeletedEvent; * Unit tests for {@link GemFireOperationsSessionRepository}. * * @author John Blum - * @since 1.1.0 + * @see java.time.Duration + * @see java.time.Instant + * @see java.util.UUID * @see org.junit.Test * @see org.junit.runner.RunWith * @see org.mockito.Mock @@ -78,6 +87,7 @@ import org.springframework.session.events.SessionDeletedEvent; * @see org.springframework.session.Session * @see org.springframework.session.events.AbstractSessionEvent * @see org.springframework.session.data.gemfire.GemFireOperationsSessionRepository + * @since 1.1.0 */ @RunWith(MockitoJUnitRunner.class) public class GemFireOperationsSessionRepositoryTests { @@ -116,8 +126,28 @@ public class GemFireOperationsSessionRepositoryTests { assertThat(this.sessionRepository.getMaxInactiveIntervalInSeconds()).isEqualTo(MAX_INACTIVE_INTERVAL_IN_SECONDS); } + private Session mockSession() { + + String sessionId = UUID.randomUUID().toString(); + + Instant now = Instant.now(); + + Duration maxInactiveInterval = Duration.ofSeconds(MAX_INACTIVE_INTERVAL_IN_SECONDS); + + Session mockSession = mock(Session.class, withSettings().name(sessionId).lenient()); + + when(mockSession.getId()).thenReturn(sessionId); + when(mockSession.getAttributeNames()).thenReturn(Collections.emptySet()); + when(mockSession.getCreationTime()).thenReturn(now); + when(mockSession.getLastAccessedTime()).thenReturn(now); + when(mockSession.getMaxInactiveInterval()).thenReturn(maxInactiveInterval); + + return mockSession; + } + @After public void tearDown() { + verify(this.mockAttributesMutator, times(1)).addCacheListener(same(this.sessionRepository)); verify(this.mockRegion, times(1)).getFullPath(); verify(this.mockTemplate, times(1)).getRegion(); @@ -125,7 +155,7 @@ public class GemFireOperationsSessionRepositoryTests { @Test @SuppressWarnings("unchecked") - public void findByIndexNameValueFindsMatchingSession() { + public void findByIndexNameAndIndexValueFindsMatchingSession() { Session mockSession = mock(Session.class, "MockSession"); @@ -138,8 +168,9 @@ public class GemFireOperationsSessionRepositoryTests { String indexName = "vip"; String indexValue = "rwinch"; - String expectedQql = String.format(GemFireOperationsSessionRepository.FIND_SESSIONS_BY_INDEX_NAME_INDEX_VALUE_QUERY, - this.sessionRepository.getFullyQualifiedRegionName(), indexName); + String expectedQql = + String.format(GemFireOperationsSessionRepository.FIND_SESSIONS_BY_INDEX_NAME_INDEX_VALUE_QUERY, + this.sessionRepository.getFullyQualifiedRegionName(), indexName); given(this.mockTemplate.find(eq(expectedQql), eq(indexValue))).willReturn(mockSelectResults); @@ -173,13 +204,14 @@ public class GemFireOperationsSessionRepositoryTests { String principalName = "jblum"; - String expectedOql = String.format(GemFireOperationsSessionRepository.FIND_SESSIONS_BY_PRINCIPAL_NAME_QUERY, - this.sessionRepository.getFullyQualifiedRegionName()); + String expectedOql = + String.format(GemFireOperationsSessionRepository.FIND_SESSIONS_BY_PRINCIPAL_NAME_QUERY, + this.sessionRepository.getFullyQualifiedRegionName()); given(this.mockTemplate.find(eq(expectedOql), eq(principalName))).willReturn(mockSelectResults); - Map sessions = this.sessionRepository.findByIndexNameAndIndexValue( - FindByIndexNameSessionRepository.PRINCIPAL_NAME_INDEX_NAME, principalName); + Map sessions = + this.sessionRepository.findByIndexNameAndIndexValue(PRINCIPAL_NAME_INDEX_NAME, principalName); assertThat(sessions).isNotNull(); assertThat(sessions.size()).isEqualTo(3); @@ -204,13 +236,14 @@ public class GemFireOperationsSessionRepositoryTests { String principalName = "jblum"; - String expectedOql = String.format(GemFireOperationsSessionRepository.FIND_SESSIONS_BY_PRINCIPAL_NAME_QUERY, - this.sessionRepository.getFullyQualifiedRegionName()); + String expectedOql = + String.format(GemFireOperationsSessionRepository.FIND_SESSIONS_BY_PRINCIPAL_NAME_QUERY, + this.sessionRepository.getFullyQualifiedRegionName()); given(this.mockTemplate.find(eq(expectedOql), eq(principalName))).willReturn(mockSelectResults); - Map sessions = this.sessionRepository.findByIndexNameAndIndexValue( - FindByIndexNameSessionRepository.PRINCIPAL_NAME_INDEX_NAME, principalName); + Map sessions = + this.sessionRepository.findByIndexNameAndIndexValue(PRINCIPAL_NAME_INDEX_NAME, principalName); assertThat(sessions).isNotNull(); assertThat(sessions.isEmpty()).isTrue(); @@ -219,29 +252,33 @@ public class GemFireOperationsSessionRepositoryTests { verify(mockSelectResults, times(1)).asList(); } - @Test - public void prepareQueryReturnsPrincipalNameOql() { - - String actualQql = - this.sessionRepository.prepareQuery(FindByIndexNameSessionRepository.PRINCIPAL_NAME_INDEX_NAME); - - String expectedOql = String.format(GemFireOperationsSessionRepository.FIND_SESSIONS_BY_PRINCIPAL_NAME_QUERY, - this.sessionRepository.getFullyQualifiedRegionName()); - - assertThat(actualQql).isEqualTo(expectedOql); - } - @Test public void prepareQueryReturnsIndexNameValueOql() { String attributeName = "testAttributeName"; + String actualOql = this.sessionRepository.prepareQuery(attributeName); - String expectedOql = String.format(GemFireOperationsSessionRepository.FIND_SESSIONS_BY_INDEX_NAME_INDEX_VALUE_QUERY, - this.sessionRepository.getFullyQualifiedRegionName(), attributeName); + + String expectedOql = + String.format(GemFireOperationsSessionRepository.FIND_SESSIONS_BY_INDEX_NAME_INDEX_VALUE_QUERY, + this.sessionRepository.getFullyQualifiedRegionName(), attributeName); assertThat(actualOql).isEqualTo(expectedOql); } + @Test + public void prepareQueryReturnsPrincipalNameOql() { + + String actualQql = + this.sessionRepository.prepareQuery(PRINCIPAL_NAME_INDEX_NAME); + + String expectedOql = + String.format(GemFireOperationsSessionRepository.FIND_SESSIONS_BY_PRINCIPAL_NAME_QUERY, + this.sessionRepository.getFullyQualifiedRegionName()); + + assertThat(actualQql).isEqualTo(expectedOql); + } + @Test public void createProperlyInitializedSession() { @@ -316,6 +353,7 @@ public class GemFireOperationsSessionRepositoryTests { Session actualSession = this.sessionRepository.findById(expectedId); + assertThat(actualSession).isNotNull(); assertThat(actualSession).isNotSameAs(mockSession); assertThat(actualSession.getId()).isEqualTo(expectedId); assertThat(actualSession.getCreationTime()).isEqualTo(expectedCreationTime); @@ -357,8 +395,8 @@ public class GemFireOperationsSessionRepositoryTests { given(mockSession.getMaxInactiveInterval()).willReturn(expectedMaxInactiveInterval); given(mockSession.getAttributeNames()).willReturn(Collections.emptySet()); - given(this.mockTemplate.put(eq(expectedSessionId), - isA(AbstractGemFireOperationsSessionRepository.GemFireSession.class))).willAnswer(invocation -> { + given(this.mockTemplate.put(eq(expectedSessionId), isA(GemFireSession.class))) + .willAnswer(invocation -> { Session session = invocation.getArgument(1); @@ -380,7 +418,52 @@ public class GemFireOperationsSessionRepositoryTests { verify(mockSession, times(1)).getMaxInactiveInterval(); verify(mockSession, times(1)).getAttributeNames(); verify(this.mockTemplate, times(1)).put(eq(expectedSessionId), - isA(AbstractGemFireOperationsSessionRepository.GemFireSession.class)); + isA(GemFireSession.class)); + } + + @Test + public void saveStoresAndCommitsGemFireSession() { + + GemFireSession session = spy(GemFireSession.create()); + + assertThat(session).isNotNull(); + assertThat(session.isDirty()).isTrue(); + + this.sessionRepository.save(session); + + InOrder orderVerifier = inOrder(session); + + orderVerifier.verify(session, times(2)).isDirty(); + orderVerifier.verify(session, times(1)).getId(); + orderVerifier.verify(session, times(1)).commit(); + + verify(this.mockTemplate, times(1)).put(eq(session.getId()), eq(session)); + } + + @Test + @SuppressWarnings("unchecked") + public void saveDoesNotStoreNonDirtyGemFireSessions() { + + GemFireSession session = spy(GemFireSession.from(mockSession())); + + assertThat(session).isNotNull(); + assertThat(session.hasDelta()).isFalse(); + assertThat(session.isDirty()).isFalse(); + + this.sessionRepository.save(session); + + verify(session, times(2)).isDirty(); + verify(session, never()).getId(); + verify(session, never()).commit(); + verify(this.mockTemplate, never()).put(any(), any(GemFireSession.class)); + } + + @Test + public void saveIsNullSafe() { + + this.sessionRepository.save(null); + + verify(this.mockTemplate, never()).put(any(), any()); } @Test @@ -406,6 +489,7 @@ public class GemFireOperationsSessionRepositoryTests { assertThat(sessionEvent.getSessionId()).isEqualTo(expectedSessionId); return null; + }).given(this.mockApplicationEventPublisher).publishEvent(isA(SessionDeletedEvent.class)); this.sessionRepository.deleteById(expectedSessionId); @@ -460,7 +544,6 @@ public class GemFireOperationsSessionRepositoryTests { .publishEvent(isA(SessionDeletedEvent.class)); } - protected abstract class GemfireOperationsAccessor extends GemfireAccessor implements GemfireOperations { + protected abstract class GemfireOperationsAccessor extends GemfireAccessor implements GemfireOperations { } - } }