From f69b83ce6c928fcd3e95122887d8450e0a02f9f3 Mon Sep 17 00:00:00 2001 From: John Blum Date: Tue, 23 Oct 2018 20:05:13 -0700 Subject: [PATCH] Default Idle Timeout to 30 minutes. Allow construction with a null Idle Timeout, which may signify disabling expiration timeouts. Implement the SessionExpirationTimeoutAware interface. Return Optional from determineExpirationTimeout(:Session). Edit Javadoc. Resolve gh-5. --- .../IdleTimeoutSessionExpirationPolicy.java | 82 ++++++++---- ...meoutSessionExpirationPolicyUnitTests.java | 118 +++++++++++++----- 2 files changed, 140 insertions(+), 60 deletions(-) diff --git a/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/expiration/support/IdleTimeoutSessionExpirationPolicy.java b/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/expiration/support/IdleTimeoutSessionExpirationPolicy.java index a1a363d..5d8a23c 100644 --- a/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/expiration/support/IdleTimeoutSessionExpirationPolicy.java +++ b/spring-session-data-geode/src/main/java/org/springframework/session/data/gemfire/expiration/support/IdleTimeoutSessionExpirationPolicy.java @@ -17,62 +17,90 @@ package org.springframework.session.data.gemfire.expiration.support; import java.time.Duration; +import java.util.Optional; import org.springframework.lang.NonNull; +import org.springframework.lang.Nullable; import org.springframework.session.Session; import org.springframework.session.data.gemfire.expiration.SessionExpirationPolicy; -import org.springframework.util.Assert; +import org.springframework.session.data.gemfire.expiration.config.SessionExpirationTimeoutAware; /** - * An implementation of the {@link SessionExpirationPolicy} interface that specifies an expiration policy - * based on inactive, idle {@link Session Sessions} exceeding a predefined time period for expiration. + * An implementation of the {@link SessionExpirationPolicy} interface that specifies an expiration policy for + * {@link Session Sessions} that have been idle, or inactive for a predefined {@link Duration duration of time}. * * @author John Blum * @see java.time.Duration + * @see java.util.Optional * @see org.springframework.session.Session * @see org.springframework.session.data.gemfire.expiration.SessionExpirationPolicy + * @see org.springframework.session.data.gemfire.expiration.config.SessionExpirationTimeoutAware * @since 2.1.0 */ @SuppressWarnings("unused") -public class IdleTimeoutSessionExpirationPolicy implements SessionExpirationPolicy { +public class IdleTimeoutSessionExpirationPolicy implements SessionExpirationPolicy, SessionExpirationTimeoutAware { - private final Duration idleExpirationTimeout; + protected static final Duration DEFAULT_IDLE_TIMEOUT = Duration.ofMinutes(30L); + + private Duration idleTimeout; /** - * Constructs a new instance of {@link IdleTimeoutSessionExpirationPolicy} initialized with - * the given {@link Duration expiration timeout}. + * Constructs a new {@link IdleTimeoutSessionExpirationPolicy} initialized with + * the {@link IdleTimeoutSessionExpirationPolicy#DEFAULT_IDLE_TIMEOUT}. * - * @param idleExpirationTimeout {@link Duration} specifying the length of time until the {@link Session} expires. - * @throws IllegalArgumentException if {@link Duration} is {@literal null}. - * @see java.time.Duration + * @see org.springframework.session.data.gemfire.expiration.support.IdleTimeoutSessionExpirationPolicy + * #DEFAULT_IDLE_TIMEOUT */ - public IdleTimeoutSessionExpirationPolicy(@NonNull Duration idleExpirationTimeout) { - - Assert.notNull(idleExpirationTimeout, "Idle expiration timeout is required"); - - this.idleExpirationTimeout = idleExpirationTimeout; - + public IdleTimeoutSessionExpirationPolicy() { + this(DEFAULT_IDLE_TIMEOUT); } /** - * Return the configured {@link Duration idle expiration timeout}. + * Constructs a new {@link IdleTimeoutSessionExpirationPolicy} initialized with + * the given {@link Duration idle timeout}. * - * @return the configured {@link Duration idle expiration timeout}. + * @param idleTimeout {@link Duration length of time} until an idle, or inactive {@link Session} should expire; + * Maybe {@literal null} to suggest the {@link Session} should not expire. * @see java.time.Duration */ - protected Duration getIdleExpirationTimeout() { - return this.idleExpirationTimeout; + public IdleTimeoutSessionExpirationPolicy(@Nullable Duration idleTimeout) { + this.idleTimeout = idleTimeout; } - @NonNull @Override - public Duration expireAfter(@NonNull Session session) { + /** + * Configures the expiration {@link Duration idle timeout}. + * + * @param idleTimeout {@link Duration length of time} until an idle, or inactive {@link Session} should expire; + * Maybe {@literal null} to suggest the {@link Session} should not expire. + * @see java.time.Duration + */ + @Override + public void setExpirationTimeout(@Nullable Duration idleTimeout) { + this.idleTimeout = idleTimeout; + } - long currentTimeMinusLastAccessTime = - Math.max(System.currentTimeMillis() - session.getLastAccessedTime().toEpochMilli(), 0); + /** + * Return an {@link Optional optionally} configured expiration {@link Duration idle timeout}. + * + * @return the {@link Optional optionally} configured expiration {@link Duration idle timeout}. + * @see java.time.Duration + * @see java.util.Optional + */ + protected Optional getIdleTimeout() { + return Optional.ofNullable(this.idleTimeout); + } - Duration expirationDuration = - getIdleExpirationTimeout().minus(Duration.ofMillis(currentTimeMinusLastAccessTime)); + @Override @SuppressWarnings("all") + public Optional determineExpirationTimeout(@NonNull Session session) { - return expirationDuration.isNegative() ? Duration.ZERO : expirationDuration; + return getIdleTimeout() + .map(idleTimeout -> idleTimeout.minus(computeIdleTime(session))); + } + + private Duration computeIdleTime(@NonNull Session session) { + + long idleTime = Math.max(System.currentTimeMillis() - session.getLastAccessedTime().toEpochMilli(), 0L); + + return Duration.ofMillis(idleTime); } } diff --git a/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/expiration/support/IdleTimeoutSessionExpirationPolicyUnitTests.java b/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/expiration/support/IdleTimeoutSessionExpirationPolicyUnitTests.java index 606c678..fd73c21 100644 --- a/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/expiration/support/IdleTimeoutSessionExpirationPolicyUnitTests.java +++ b/spring-session-data-geode/src/test/java/org/springframework/session/data/gemfire/expiration/support/IdleTimeoutSessionExpirationPolicyUnitTests.java @@ -22,6 +22,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.springframework.session.data.gemfire.expiration.support.IdleTimeoutSessionExpirationPolicy.DEFAULT_IDLE_TIMEOUT; import java.time.Duration; import java.time.Instant; @@ -34,6 +35,8 @@ import org.springframework.session.Session; * Unit tests for {@link IdleTimeoutSessionExpirationPolicy}. * * @author John Blum + * @see java.time.Duration + * @see java.time.Instant * @see org.junit.Test * @see org.mockito.Mockito * @see org.springframework.session.Session @@ -43,72 +46,121 @@ import org.springframework.session.Session; public class IdleTimeoutSessionExpirationPolicyUnitTests { @Test - public void constructIdleTimeoutSessionExpirationPolicy() { + public void constructDefaultIdleTimeoutExpirationPolicy() { - Duration idleExpirationTimeout = Duration.ofSeconds(60); + IdleTimeoutSessionExpirationPolicy sessionExpirationPolicy = new IdleTimeoutSessionExpirationPolicy(); + + assertThat(sessionExpirationPolicy).isNotNull(); + assertThat(sessionExpirationPolicy.getIdleTimeout().orElse(null)).isEqualTo(DEFAULT_IDLE_TIMEOUT); + } + + @Test + public void constructNewIdleTimeoutSessionExpirationPolicyWithIdleTimeout() { + + Duration idleExpirationTimeout = Duration.ofMinutes(60L); IdleTimeoutSessionExpirationPolicy sessionExpirationPolicy = new IdleTimeoutSessionExpirationPolicy(idleExpirationTimeout); assertThat(sessionExpirationPolicy).isNotNull(); - assertThat(sessionExpirationPolicy.getIdleExpirationTimeout()).isEqualTo(idleExpirationTimeout); - } - - @Test(expected = IllegalArgumentException.class) - public void constructIdleTimeoutSessionExpirationPolicyWithNullDuration() { - - try { - new IdleTimeoutSessionExpirationPolicy(null); - } - catch (IllegalArgumentException expected) { - - assertThat(expected).hasMessage("Idle expiration timeout is required"); - assertThat(expected).hasNoCause(); - - throw expected; - } + assertThat(sessionExpirationPolicy.getIdleTimeout().orElse(null)).isEqualTo(idleExpirationTimeout); } @Test - public void expireAfterReturnsFutureDuration() { + public void constructNewIdleTimeoutSessionExpirationPolicyWithNullIdleTimeout() { - Duration expirationTimeout = Duration.ofSeconds(30); + IdleTimeoutSessionExpirationPolicy sessionExpirationPolicy = new IdleTimeoutSessionExpirationPolicy(null); + + assertThat(sessionExpirationPolicy).isNotNull(); + assertThat(sessionExpirationPolicy.getIdleTimeout().orElse(null)).isNull(); + } + + @Test + public void determineExpirationTimeoutReturnsExpiredDuration() { + + Duration idleTimeout = Duration.ofSeconds(60L); IdleTimeoutSessionExpirationPolicy sessionExpirationPolicy = - new IdleTimeoutSessionExpirationPolicy(expirationTimeout); + new IdleTimeoutSessionExpirationPolicy(idleTimeout); + + assertThat(sessionExpirationPolicy.getIdleTimeout().orElse(null)).isEqualTo(idleTimeout); Session mockSession = mock(Session.class); when(mockSession.getLastAccessedTime()) - .thenReturn(Instant.ofEpochMilli(System.currentTimeMillis() - Duration.ofSeconds(15).toMillis())); + .thenReturn(Instant.ofEpochMilli(System.currentTimeMillis() - Duration.ofSeconds(61L).toMillis())); - Duration expireAfter = sessionExpirationPolicy.expireAfter(mockSession); + Duration expirationTimeout = sessionExpirationPolicy.determineExpirationTimeout(mockSession).orElse(null); - assertThat(expireAfter).isNotNull(); - assertThat(expireAfter.getSeconds()).isLessThanOrEqualTo(15); + assertThat(expirationTimeout).isNotNull(); + assertThat(expirationTimeout).isLessThan(Duration.ZERO); - verify(mockSession, times(1)).getLastAccessedTime(); verify(mockSession, never()).getCreationTime(); + verify(mockSession, times(1)).getLastAccessedTime(); } @Test - public void expireAfterReturnsZero() { + public void determineExpirationTimeoutReturnsNonExpiredDuration() { - Duration expirationTimeout = Duration.ofSeconds(30); + Duration idleTimeout = Duration.ofSeconds(60L); IdleTimeoutSessionExpirationPolicy sessionExpirationPolicy = - new IdleTimeoutSessionExpirationPolicy(expirationTimeout); + new IdleTimeoutSessionExpirationPolicy(idleTimeout); + + assertThat(sessionExpirationPolicy.getIdleTimeout().orElse(null)).isEqualTo(idleTimeout); Session mockSession = mock(Session.class); when(mockSession.getLastAccessedTime()) - .thenReturn(Instant.ofEpochMilli(System.currentTimeMillis() - Duration.ofSeconds(60).toMillis())); + .thenReturn(Instant.ofEpochMilli(System.currentTimeMillis() - Duration.ofSeconds(30L).toMillis())); - Duration expireAfter = sessionExpirationPolicy.expireAfter(mockSession); + Duration expirationTimeout = sessionExpirationPolicy.determineExpirationTimeout(mockSession).orElse(null); - assertThat(expireAfter).isEqualTo(Duration.ZERO); + assertThat(expirationTimeout).isNotNull(); + assertThat(expirationTimeout).isGreaterThan(Duration.ZERO); - verify(mockSession, times(1)).getLastAccessedTime(); verify(mockSession, never()).getCreationTime(); + verify(mockSession, times(1)).getLastAccessedTime(); + } + + @Test + public void determineExpirationTimeoutWithNoIdleTimeoutConfiguredReturnsNoDuration() { + + IdleTimeoutSessionExpirationPolicy sessionExpirationPolicy = new IdleTimeoutSessionExpirationPolicy(null); + + assertThat(sessionExpirationPolicy.getIdleTimeout().orElse(null)).isNull(); + + Session mockSession = mock(Session.class); + + Duration expirationTimeout = sessionExpirationPolicy.determineExpirationTimeout(mockSession).orElse(null); + + assertThat(expirationTimeout).isNull(); + + verify(mockSession, never()).getCreationTime(); + verify(mockSession, never()).getLastAccessedTime(); + } + + @Test + public void expirationTimeoutIsNotGreaterThanIdleTimeout() { + + Duration idleTimeout = Duration.ofSeconds(60L); + + IdleTimeoutSessionExpirationPolicy sessionExpirationPolicy = + new IdleTimeoutSessionExpirationPolicy(idleTimeout); + + assertThat(sessionExpirationPolicy.getIdleTimeout().orElse(null)).isEqualTo(idleTimeout); + + Session mockSession = mock(Session.class); + + when(mockSession.getLastAccessedTime()) + .thenReturn(Instant.ofEpochMilli(System.currentTimeMillis() + Duration.ofSeconds(60L).toMillis())); + + Duration expirationTimeout = sessionExpirationPolicy.determineExpirationTimeout(mockSession).orElse(null); + + assertThat(expirationTimeout).isNotNull(); + assertThat(expirationTimeout).isEqualTo(idleTimeout); + + verify(mockSession, never()).getCreationTime(); + verify(mockSession, times(1)).getLastAccessedTime(); } }