diff --git a/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/expiration/support/SessionExpirationPolicyCustomExpiryAdapter.java b/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/expiration/support/SessionExpirationPolicyCustomExpiryAdapter.java index e0939c9..839896b 100644 --- a/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/expiration/support/SessionExpirationPolicyCustomExpiryAdapter.java +++ b/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/expiration/support/SessionExpirationPolicyCustomExpiryAdapter.java @@ -34,13 +34,14 @@ import org.springframework.util.Assert; /** * The {@link SessionExpirationPolicyCustomExpiryAdapter} class is an Apache Geode/Pivotal GemFire {@link CustomExpiry} * implementation wrapping and adapting an instance of the {@link SessionExpirationPolicy} strategy interface - * to plugin to GemFire/Geode's expiration logistics. + * to plugin to and affect Apache Geode/Pivotal GemFire's expiration behavior. * * @author John Blum * @see org.apache.geode.cache.CustomExpiry * @see org.apache.geode.cache.ExpirationAction * @see org.apache.geode.cache.ExpirationAttributes * @see org.apache.geode.cache.Region + * @see org.apache.geode.pdx.PdxInstance * @see org.springframework.session.Session * @see org.springframework.session.data.gemfire.expiration.SessionExpirationPolicy * @since 1.0.0 @@ -48,15 +49,18 @@ import org.springframework.util.Assert; @SuppressWarnings("unused") public class SessionExpirationPolicyCustomExpiryAdapter implements CustomExpiry { + protected static final SessionExpirationPolicy.ExpirationAction DEFAULT_EXPIRATION_ACTION = + SessionExpirationPolicy.ExpirationAction.INVALIDATE; + private final SessionExpirationPolicy sessionExpirationPolicy; /** - * Constructs a new instance of {@link SessionExpirationPolicyCustomExpiryAdapter} initialized with + * Constructs a new {@link SessionExpirationPolicyCustomExpiryAdapter} initialized with * the given, required {@link SessionExpirationPolicy}. * - * @param sessionExpirationPolicy {@link SessionExpirationPolicy} used to enforce the expiration policy - * on all {@link Session Sessions}. - * @throws IllegalArgumentException if the {@link SessionExpirationPolicy} is {@literal null}. + * @param sessionExpirationPolicy {@link SessionExpirationPolicy} used to determine the expiration policy + * for all {@link Session Sessions}. + * @throws IllegalArgumentException if {@link SessionExpirationPolicy} is {@literal null}. * @see org.springframework.session.data.gemfire.expiration.SessionExpirationPolicy */ public SessionExpirationPolicyCustomExpiryAdapter(@NonNull SessionExpirationPolicy sessionExpirationPolicy) { @@ -67,10 +71,10 @@ public class SessionExpirationPolicyCustomExpiryAdapter implements CustomExpiry< } /** - * Returns a reference to the {@link SessionExpirationPolicy} defining the expiration policies + * Returns the configured {@link SessionExpirationPolicy} defining the expiration policies * for all managed {@link Session Sessions}. * - * @return a reference to the {@link SessionExpirationPolicy}. + * @return the configured {@link SessionExpirationPolicy}. * @see org.springframework.session.data.gemfire.expiration.SessionExpirationPolicy */ protected SessionExpirationPolicy getSessionExpirationPolicy() { @@ -80,59 +84,61 @@ public class SessionExpirationPolicyCustomExpiryAdapter implements CustomExpiry< @Nullable @Override public ExpirationAttributes getExpiry(@Nullable Region.Entry regionEntry) { - return Optional.ofNullable(resolveSession(regionEntry)) - .map(this::newExpirationAttributes) + return resolveSession(regionEntry) + .flatMap(getSessionExpirationPolicy()::determineExpirationTimeout) + .map(expirationTimeout -> + newExpirationAttributes(expirationTimeout, getSessionExpirationPolicy().getExpirationAction())) .orElse(null); } /** - * Constructs {@link ExpirationAttributes} from the given {@link Session}. + * Constructs a new {@link ExpirationAttributes} initialized with the given {@link Duration expiration timeut} + * and default {@link ExpirationAction#INVALIDATE expirtion action}. * - * @param session {@link Session} used to construct the {@link ExpirationAttributes}. - * @return a new {@link ExpirationAttributes} constructed from the given {@link Session}. + * @param expirationTimeout {@link Duration} specifying the expiration timeout. + * @return the new {@link ExpirationAttributes}. * @see #newExpirationAttributes(Duration, SessionExpirationPolicy.ExpirationAction) * @see org.apache.geode.cache.ExpirationAttributes - * @see org.springframework.session.Session + * @see java.time.Duration */ - private ExpirationAttributes newExpirationAttributes(Session session) { - - SessionExpirationPolicy sessionExpirationPolicy = getSessionExpirationPolicy(); - Duration sessionExpirationDuration = sessionExpirationPolicy.expireAfter(session); - SessionExpirationPolicy.ExpirationAction sessionExpirationAction = sessionExpirationPolicy.getAction(); - - return newExpirationAttributes(sessionExpirationDuration, sessionExpirationAction); + @NonNull + protected ExpirationAttributes newExpirationAttributes(@NonNull Duration expirationTimeout) { + return newExpirationAttributes(expirationTimeout, DEFAULT_EXPIRATION_ACTION); } /** * Constructs a new {@link ExpirationAttributes} initialized with the given {@link Duration expiration timeout} * and {@link SessionExpirationPolicy.ExpirationAction} to take when the {@link Session} expires. * - * @param duration {@link Duration} specifying the expiration timeout. - * @param expirationAction {@link SessionExpirationPolicy.ExpirationAction} to take when - * the {@link Session} expires. + * @param expirationTimeout {@link Duration} specifying the expiration timeout. + * @param expirationAction {@link SessionExpirationPolicy.ExpirationAction} taken when the {@link Session} expires. * @return the new {@link ExpirationAttributes}. - * @see org.springframework.session.data.gemfire.expiration.SessionExpirationPolicy.ExpirationAction * @see #newExpirationAttributes(int, ExpirationAction) + * @see org.springframework.session.data.gemfire.expiration.SessionExpirationPolicy.ExpirationAction * @see org.apache.geode.cache.ExpirationAttributes * @see java.time.Duration */ - private ExpirationAttributes newExpirationAttributes(Duration duration, - SessionExpirationPolicy.ExpirationAction expirationAction) { + @NonNull + protected ExpirationAttributes newExpirationAttributes(@NonNull Duration expirationTimeout, + @Nullable SessionExpirationPolicy.ExpirationAction expirationAction) { - int expirationTimeout = (int) Math.min(Integer.MAX_VALUE, Math.max(duration.getSeconds(), 1)); + int expirationTimeoutInSeconds = + (int) Math.min(Integer.MAX_VALUE, Math.max(expirationTimeout.getSeconds(), 1)); - return newExpirationAttributes(expirationTimeout, resolveExpirationAction(expirationAction)); + return newExpirationAttributes(expirationTimeoutInSeconds, toGemFireExpirationAction(expirationAction)); } /** - * Constructs new {@link ExpirationAttributes} with the given {@link Integer expiration timeout} - * and {@link ExpirationAction} to take when the {@link Session} expires. + * Constructs a new {@link ExpirationAttributes} initialized with the given {@link Integer expiration timeout} + * in seconds and {@link ExpirationAction} taken when the {@link Session} expires. * - * @param expirationTimeInSeconds length of time in seconds until the {@link Session} expires. - * @param expirationAction {@link ExpirationAction} to take when the {@link Session} expires. + * @param expirationTimeInSeconds {@link Integer length of time} in seconds until the {@link Session} expires. + * @param expirationAction {@link ExpirationAction} taken when the {@link Session} expires. * @return the new {@link ExpirationAttributes}. + * @see org.apache.geode.cache.ExpirationAction * @see org.apache.geode.cache.ExpirationAttributes */ + @NonNull private ExpirationAttributes newExpirationAttributes(int expirationTimeInSeconds, ExpirationAction expirationAction) { @@ -140,20 +146,57 @@ public class SessionExpirationPolicyCustomExpiryAdapter implements CustomExpiry< } /** - * Resolves the {@link org.apache.geode.cache.ExpirationAction} from the given + * Resolves an {@link Optional} {@link Session} object from the {@link Region.Entry#getValue() Region Entry Value}. + * + * @param regionEntry {@link Region.Entry} from which to extract the {@link Session} value. + * @return an {@link Optional} {@link Session} object from the {@link Region.Entry#getValue() Region Entry Value}. + * @see org.apache.geode.cache.Region.Entry + * @see org.springframework.session.Session + * @see java.util.Optional + * @see #resolveSession(Object) + */ + private Optional resolveSession(@Nullable Region.Entry regionEntry) { + + return Optional.ofNullable(regionEntry) + .map(Region.Entry::getValue) + .flatMap(this::resolveSession); + } + + /** + * Resolves an {@link Optional} {@link Session} object from the given {@link Object} value. + * + * The {@link Object} may already be a {@link Session} or may possibly be a {@link PdxInstance} + * if Apache Geode/Pivotal GemFire PDX serialization is enabled. + * + * @param value {@link Object} to evaluate as a {@link Session}. + * @return an {@link Optional} {@link Session} from the given {@link Object}. + * @see org.springframework.session.Session + * @see org.apache.geode.pdx.PdxInstance + * @see java.util.Optional + * @see java.lang.Object + */ + private Optional resolveSession(@Nullable Object value) { + + return Optional.ofNullable(value instanceof Session ? (Session) value + : value instanceof PdxInstance ? (Session) ((PdxInstance) value).getObject() + : null); + } + + /** + * Converts the {@link org.apache.geode.cache.ExpirationAction} from the given * {@link SessionExpirationPolicy.ExpirationAction}. * * Defaults to {@link ExpirationAction#INVALIDATE} if {@link SessionExpirationPolicy.ExpirationAction} * is {@literal null}. * - * @param expirationAction {@link SessionExpirationPolicy.ExpirationAction} to convert into a + * @param expirationAction {@link SessionExpirationPolicy.ExpirationAction} to convert into an * {@link org.apache.geode.cache.ExpirationAction}. * @return an {@link org.apache.geode.cache.ExpirationAction} from the given - * {@link SessionExpirationPolicy.ExpirationAction}. + * {@link SessionExpirationPolicy.ExpirationAction}; defaults to {@link ExpirationAction#INVALIDATE}. * @see org.springframework.session.data.gemfire.expiration.SessionExpirationPolicy.ExpirationAction * @see org.apache.geode.cache.ExpirationAction */ - private ExpirationAction resolveExpirationAction(SessionExpirationPolicy.ExpirationAction expirationAction) { + private ExpirationAction toGemFireExpirationAction(SessionExpirationPolicy.ExpirationAction expirationAction) { switch (SessionExpirationPolicy.ExpirationAction.defaultIfNull(expirationAction)) { case DESTROY: @@ -162,38 +205,4 @@ public class SessionExpirationPolicyCustomExpiryAdapter implements CustomExpiry< return ExpirationAction.INVALIDATE; } } - - /** - * Resolves a {@link Session} object from the given {@link Region.Entry#getValue() Region Entry Value}. - * - * @param regionEntry {@link Region.Entry} from which to extract the {@link Session} value. - * @return a {@link Session} object from the given {@link Region.Entry#getValue()}. - * @see org.springframework.session.Session - * @see org.apache.geode.cache.Region.Entry - * @see #resolveSession(Object) - */ - @Nullable - private Session resolveSession(Region.Entry regionEntry) { - return resolveSession(Optional.ofNullable(regionEntry).map(Region.Entry::getValue).orElse(null)); - } - - /** - * Resolves a {@link Session} object from the given {@link Object} value. - * - * The {@link Object} may already be a {@link Session} or may possibly be a {@link PdxInstance} if GemFire/Geode - * PDX serialization is enabled. - * - * @param regionEntryValue {@link Object} to evaluate as a {@link Session}. - * @return a {@link Session} from the given {@link Object}. - * @see org.springframework.session.Session - * @see org.apache.geode.pdx.PdxInstance - * @see java.lang.Object - */ - @Nullable - private Session resolveSession(Object regionEntryValue) { - - return regionEntryValue instanceof Session ? (Session) regionEntryValue - : regionEntryValue instanceof PdxInstance ? (Session) ((PdxInstance) regionEntryValue).getObject() - : null; - } } diff --git a/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/expiration/support/SessionExpirationPolicyCustomExpiryAdapterUnitTests.java b/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/expiration/support/SessionExpirationPolicyCustomExpiryAdapterUnitTests.java index c1d1e25..9daecb3 100644 --- a/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/expiration/support/SessionExpirationPolicyCustomExpiryAdapterUnitTests.java +++ b/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/expiration/support/SessionExpirationPolicyCustomExpiryAdapterUnitTests.java @@ -20,12 +20,14 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; 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 java.time.Duration; +import java.util.Optional; import org.junit.Test; import org.junit.runner.RunWith; @@ -68,7 +70,7 @@ public class SessionExpirationPolicyCustomExpiryAdapterUnitTests { } @Test(expected = IllegalArgumentException.class) - public void constructNewSessionExpirationPolicyCustomExpiryAdapterWithNullSessionExpirationPolicy() { + public void constructsNewSessionExpirationPolicyCustomExpiryAdapterWithNullSessionExpirationPolicy() { try { new SessionExpirationPolicyCustomExpiryAdapter(null); @@ -84,19 +86,27 @@ public class SessionExpirationPolicyCustomExpiryAdapterUnitTests { @Test @SuppressWarnings("unchecked") - public void getExpiryHandlesInvalidRegionEntryValueType() { + public void getExpiryHandlesNoExpirationTimeout() { + + when(this.mockSessionExpirationPolicy.determineExpirationTimeout(any(Session.class))) + .thenReturn(Optional.empty()); + + Session mockSession = mock(Session.class); Region.Entry mockRegionEntry = mock(Region.Entry.class); - when(mockRegionEntry.getValue()).thenReturn("MockSession"); + when(mockRegionEntry.getValue()).thenReturn(mockSession); SessionExpirationPolicyCustomExpiryAdapter adapter = new SessionExpirationPolicyCustomExpiryAdapter(this.mockSessionExpirationPolicy); - assertThat(adapter.getExpiry(mockRegionEntry)).isNull(); + ExpirationAttributes expirationAttributes = adapter.getExpiry(mockRegionEntry); + + assertThat(expirationAttributes).isNull(); verify(mockRegionEntry, times(1)).getValue(); - verifyZeroInteractions(this.mockSessionExpirationPolicy); + verify(this.mockSessionExpirationPolicy, times(1)).determineExpirationTimeout(eq(mockSession)); + verify(this.mockSessionExpirationPolicy, never()).getExpirationAction(); } @Test @@ -150,19 +160,38 @@ public class SessionExpirationPolicyCustomExpiryAdapterUnitTests { @Test @SuppressWarnings("unchecked") - public void getExpiryReturnsExpirationAttributes() { - - Duration expirationTimeout = Duration.ofMinutes(30); - - SessionExpirationPolicy.ExpirationAction expirationAction = SessionExpirationPolicy.ExpirationAction.DESTROY; - - when(this.mockSessionExpirationPolicy.getAction()).thenReturn(expirationAction); - when(this.mockSessionExpirationPolicy.expireAfter(any(Session.class))).thenReturn(expirationTimeout); + public void getExpiryHandlesUnresolvableSession() { Region.Entry mockRegionEntry = mock(Region.Entry.class); + when(mockRegionEntry.getValue()).thenReturn("MockSession"); + + SessionExpirationPolicyCustomExpiryAdapter adapter = + new SessionExpirationPolicyCustomExpiryAdapter(this.mockSessionExpirationPolicy); + + assertThat(adapter.getExpiry(mockRegionEntry)).isNull(); + + verify(mockRegionEntry, times(1)).getValue(); + verifyZeroInteractions(this.mockSessionExpirationPolicy); + } + + @Test + @SuppressWarnings("unchecked") + public void getExpiryReturnsExpirationAttributes() { + + Duration expirationTimeout = Duration.ofMinutes(30L); + + SessionExpirationPolicy.ExpirationAction expirationAction = SessionExpirationPolicy.ExpirationAction.DESTROY; + + when(this.mockSessionExpirationPolicy.determineExpirationTimeout(any(Session.class))) + .thenReturn(Optional.of(expirationTimeout)); + + when(this.mockSessionExpirationPolicy.getExpirationAction()).thenReturn(expirationAction); + Session mockSession = mock(Session.class); + Region.Entry mockRegionEntry = mock(Region.Entry.class); + when(mockRegionEntry.getValue()).thenReturn(mockSession); SessionExpirationPolicyCustomExpiryAdapter adapter = @@ -175,18 +204,20 @@ public class SessionExpirationPolicyCustomExpiryAdapterUnitTests { assertThat(expirationAttributes.getTimeout()).isEqualTo(expirationTimeout.getSeconds()); verify(mockRegionEntry, times(1)).getValue(); - verify(this.mockSessionExpirationPolicy, times(1)).getAction(); - verify(this.mockSessionExpirationPolicy, times(1)).expireAfter(eq(mockSession)); + verify(this.mockSessionExpirationPolicy, times(1)).determineExpirationTimeout(eq(mockSession)); + verify(this.mockSessionExpirationPolicy, times(1)).getExpirationAction(); } @Test @SuppressWarnings("unchecked") - public void getExpiryWithPdxInstanceAndOverflowedExpirationTimeoutAndNullExpirationAction() { + public void getExpiryWithPdxInstanceAndOverflowedExpirationTimeoutAndNullExpirationActionReturnsExpirationAttributes() { Duration expirationTimeout = Duration.ofSeconds(987654321369L); - when(this.mockSessionExpirationPolicy.getAction()).thenReturn(null); - when(this.mockSessionExpirationPolicy.expireAfter(any(Session.class))).thenReturn(expirationTimeout); + when(this.mockSessionExpirationPolicy.determineExpirationTimeout(any(Session.class))) + .thenReturn(Optional.of(expirationTimeout)); + + when(this.mockSessionExpirationPolicy.getExpirationAction()).thenReturn(null); Session mockSession = mock(Session.class); @@ -208,20 +239,22 @@ public class SessionExpirationPolicyCustomExpiryAdapterUnitTests { verify(mockPdxInstance, times(1)).getObject(); verify(mockRegionEntry, times(1)).getValue(); - verify(this.mockSessionExpirationPolicy, times(1)).getAction(); - verify(this.mockSessionExpirationPolicy, times(1)).expireAfter(eq(mockSession)); + verify(this.mockSessionExpirationPolicy, times(1)).determineExpirationTimeout(eq(mockSession)); + verify(this.mockSessionExpirationPolicy, times(1)).getExpirationAction(); } @Test @SuppressWarnings("unchecked") - public void getExpiryWithUnderflowExpirationTimeout() { + public void getExpiryWithUnderflowExpirationTimeoutReturnsExpirationAttributes() { - Duration expirationTimeout = Duration.ofSeconds(-120); + Duration expirationTimeout = Duration.ofSeconds(-120L); SessionExpirationPolicy.ExpirationAction expirationAction = SessionExpirationPolicy.ExpirationAction.DESTROY; - when(this.mockSessionExpirationPolicy.getAction()).thenReturn(expirationAction); - when(this.mockSessionExpirationPolicy.expireAfter(any(Session.class))).thenReturn(expirationTimeout); + when(this.mockSessionExpirationPolicy.determineExpirationTimeout(any(Session.class))) + .thenReturn(Optional.of(expirationTimeout)); + + when(this.mockSessionExpirationPolicy.getExpirationAction()).thenReturn(expirationAction); Session mockSession = mock(Session.class); @@ -239,7 +272,7 @@ public class SessionExpirationPolicyCustomExpiryAdapterUnitTests { assertThat(expirationAttributes.getTimeout()).isEqualTo(1); verify(mockRegionEntry, times(1)).getValue(); - verify(this.mockSessionExpirationPolicy, times(1)).getAction(); - verify(this.mockSessionExpirationPolicy, times(1)).expireAfter(eq(mockSession)); + verify(this.mockSessionExpirationPolicy, times(1)).determineExpirationTimeout(eq(mockSession)); + verify(this.mockSessionExpirationPolicy, times(1)).getExpirationAction(); } }