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 4c5ae83..c6507ae 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 @@ -106,10 +106,11 @@ import org.apache.commons.logging.LogFactory; public abstract class AbstractGemFireOperationsSessionRepository implements ApplicationEventPublisherAware, FindByIndexNameSessionRepository { - private static final boolean DEFAULT_REGISTER_INTEREST_ENABLED = false; private static final boolean DEFAULT_REGISTER_INTEREST_DURABILITY = false; + private static final boolean DEFAULT_REGISTER_INTEREST_ENABLED = false; private static final boolean DEFAULT_REGISTER_INTEREST_RECEIVE_VALUES = true; + // TODO - refactor and use non-static variable private static final AtomicBoolean usingDataSerialization = new AtomicBoolean(false); private static final InterestResultPolicy DEFAULT_REGISTER_INTEREST_RESULT_POLICY = InterestResultPolicy.NONE; @@ -279,6 +280,7 @@ public abstract class AbstractGemFireOperationsSessionRepository * used to store and manage {@link Session} data. * @see #getSessionsRegion() */ + // TODO - refactor and rename to SessionRegionName protected String getFullyQualifiedRegionName() { return getSessionsRegion().getFullPath(); } @@ -590,7 +592,7 @@ public abstract class AbstractGemFireOperationsSessionRepository @Override protected DeltaCapableGemFireSessionAttributes newSessionAttributes(Object lock) { - return new DeltaCapableGemFireSessionAttributes(); + return new DeltaCapableGemFireSessionAttributes(lock); } public synchronized void toDelta(DataOutput out) throws IOException { @@ -631,10 +633,11 @@ public abstract class AbstractGemFireOperationsSessionRepository * Factory method used to create a new instance of {@link GemFireSession} initialized with * the {@link #DEFAULT_MAX_INACTIVE_INTERVAL}. * + * @param {@link Class Sub-type} of {@link GemFireSessionAttributes}. * @return new {@link GemFireSession}. * @see #create(Duration) */ - public static GemFireSession create() { + public static GemFireSession create() { return create(DEFAULT_MAX_INACTIVE_INTERVAL); } @@ -642,6 +645,7 @@ public abstract class AbstractGemFireOperationsSessionRepository * Factory method used to create a new instance of {@link GemFireSession} initialized with * the given {@link Duration max inactive interval}. * + * @param {@link Class Sub-type} of {@link GemFireSessionAttributes}. * @param maxInactiveInterval {@link Duration} specifying the max inactive interval before * this {@link Session} will expire. * @return a new instance of {@link GemFireSession} initialized with @@ -649,7 +653,9 @@ public abstract class AbstractGemFireOperationsSessionRepository * @see #isUsingDataSerialization() * @see java.time.Duration */ - public static GemFireSession create(Duration maxInactiveInterval) { + @SuppressWarnings("unchecked") + // TODO - remove + public static GemFireSession create(Duration maxInactiveInterval) { GemFireSession session = isUsingDataSerialization() ? new DeltaCapableGemFireSession() @@ -679,15 +685,14 @@ public abstract class AbstractGemFireOperationsSessionRepository * Returns the given {@link Session} if the {@link Session} is a {@link GemFireSession} or return a copy * of the given {@link Session} as a {@link GemFireSession}. * - * @param {@link Class sub-type} of {@link GemFireSession}. * @param session {@link Session} to evaluate and possibly copy. * @return the given {@link Session} if the {@link Session} is a {@link GemFireSession} or return a copy * of the given {@link Session} as a {@link GemFireSession} * @see #copy(Session) */ @SuppressWarnings("unchecked") - public static T from(@NonNull Session session) { - return (T) (session instanceof GemFireSession ? session : copy(session)); + public static GemFireSession from(@NonNull Session session) { + return session instanceof GemFireSession ? (GemFireSession) session : copy(session); } private transient boolean delta = true; @@ -741,7 +746,7 @@ public abstract class AbstractGemFireOperationsSessionRepository */ protected GemFireSession(Session session) { - Assert.notNull(session, "The Session to copy must not be null"); + Assert.notNull(session, "Session is required"); this.id = session.getId(); this.creationTime = session.getCreationTime(); 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 5e07da1..17d199f 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 @@ -1820,6 +1820,20 @@ public class AbstractGemFireOperationsSessionRepositoryTests { assertThat(session.getAttributes()).isEmpty(); } + private void testConstructGemFireSessionWithInvalidId(String id) { + + try { + new GemFireSession(id); + } + catch (IllegalArgumentException expected) { + + assertThat(expected).hasMessage("ID is required"); + assertThat(expected).hasNoCause(); + + throw expected; + } + } + @Test(expected = IllegalArgumentException.class) public void constructGemFireSessionWithEmptyId() { testConstructGemFireSessionWithInvalidId(""); @@ -1835,20 +1849,6 @@ public class AbstractGemFireOperationsSessionRepositoryTests { testConstructGemFireSessionWithInvalidId(" "); } - private void testConstructGemFireSessionWithInvalidId(String id) { - - try { - new GemFireSession(id); - } - catch (IllegalArgumentException expected) { - - assertThat(expected).hasMessage("ID is required"); - assertThat(expected).hasNoCause(); - - throw expected; - } - } - @Test public void constructGemFireSessionWithSession() { @@ -1894,7 +1894,7 @@ public class AbstractGemFireOperationsSessionRepositoryTests { } catch (IllegalArgumentException expected) { - assertThat(expected).hasMessage("The Session to copy must not be null"); + assertThat(expected).hasMessage("Session is required"); assertThat(expected).hasNoCause(); throw expected; @@ -1919,7 +1919,7 @@ public class AbstractGemFireOperationsSessionRepositoryTests { } @Test - public void createNewGemFireSessionWithProvidedMaxInactiveInterval() { + public void createNewGemFireSessionWithSpecifiedMaxInactiveInterval() { Instant testCreationTime = Instant.now(); @@ -1938,14 +1938,14 @@ public class AbstractGemFireOperationsSessionRepositoryTests { } @Test(expected = IllegalArgumentException.class) - public void copyNullThrowsException() { + public void copyNullThrowsIllegalArgumentException() { try { GemFireSession.copy(null); } catch (IllegalArgumentException expected) { - assertThat(expected).hasMessage("The Session to copy must not be null"); + assertThat(expected).hasMessage("Session is required"); assertThat(expected).hasNoCause(); throw expected; @@ -2055,6 +2055,21 @@ public class AbstractGemFireOperationsSessionRepositoryTests { assertThat(fromGemFireSession).isSameAs(gemfireSession); } + @Test(expected = IllegalArgumentException.class) + public void fromNullSessionThrowsIllegalArgumentException() { + + try { + GemFireSession.from(null); + } + catch (IllegalArgumentException expected) { + + assertThat(expected).hasMessage("Session is required"); + assertThat(expected).hasNoCause(); + + throw expected; + } + } + @Test public void setGetAndRemoveAttribute() { @@ -3055,6 +3070,28 @@ public class AbstractGemFireOperationsSessionRepositoryTests { assertThat(entry.getValue()).isEqualTo("valueThree"); } + @Test + public void gemfireSessionIsLockForGemFireSessionAttributes() { + + GemFireSession session = new GemFireSession(); + + GemFireSessionAttributes sessionAttributes = session.newSessionAttributes(session); + + assertThat(sessionAttributes).isNotNull(); + assertThat(sessionAttributes.getLock()).isSameAs(session); + } + + @Test + public void deltaCapableGemFireSessionIsLockForDeltaCapableGemFirSessionAttributes() { + + DeltaCapableGemFireSession session = new DeltaCapableGemFireSession(); + + DeltaCapableGemFireSessionAttributes sessionAttributes = session.newSessionAttributes(session); + + assertThat(sessionAttributes).isNotNull(); + assertThat(sessionAttributes.getLock()).isSameAs(session); + } + @Test public void sessionWithAttributesAreThreadSafe() throws Throwable { TestFramework.runOnce(new ThreadSafeSessionTestCase());