Spring/spring-session-data-geode
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix bug in DeltaCapableGemFireSessionAttriutes.newSessionAttributes(lock:Object) not properly passing the lock to the new GemFireSessionAttributes instance.

Browse Source
This commit is contained in:
John Blum
2018-12-17 13:52:05 -08:00
parent 96632a0502
commit ad312984bc
2 changed files with 68 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/AbstractGemFireOperationsSessionRepository.java
View File

@@ -106,10 +106,11 @@ import org.apache.commons.logging.LogFactory;
public abstract class AbstractGemFireOperationsSessionRepository
implements ApplicationEventPublisherAware, FindByIndexNameSessionRepository<Session> {
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 <T> {@link Class Sub-type} of {@link GemFireSessionAttributes}.
* @return new {@link GemFireSession}.
* @see #create(Duration)
*/
public static GemFireSession create() {
public static <T extends GemFireSessionAttributes> GemFireSession<T> 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 <T> {@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 <T extends GemFireSessionAttributes> GemFireSession<T> 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 <T> {@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 extends GemFireSession> 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();

73
spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/AbstractGemFireOperationsSessionRepositoryTests.java
View File

@@ -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());