configurable sessions expiration (#1501)

* unit test + interface

* Renaming and moving expiry related methods to implementation

---------

Co-authored-by: dotasek <david.otasek@smilecdr.com>

org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/services/PassiveExpiringSessionCache.java

@@ -0,0 +1,127 @@
+package org.hl7.fhir.validation.cli.services;
+
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.commons.collections4.map.PassiveExpiringMap;
+import org.hl7.fhir.validation.ValidationEngine;
+
+/**
+ * SessionCache for storing and retrieving ValidationEngine instances, so callers do not have to re-instantiate a new
+ * instance for each validation request.
+ */
+public class PassiveExpiringSessionCache implements SessionCache {
+
+  protected static final long TIME_TO_LIVE = 60;
+  protected static final TimeUnit TIME_UNIT = TimeUnit.MINUTES;
+  protected boolean resetExpirationAfterFetch = false;
+
+  private final PassiveExpiringMap<String, ValidationEngine> cachedSessions;
+
+  public PassiveExpiringSessionCache() {
+    cachedSessions = new PassiveExpiringMap<>(TIME_TO_LIVE, TIME_UNIT);
+  }
+
+  /**
+   * @param sessionLength the constant amount of time an entry is available before it expires. A negative value results
+   *                      in entries that NEVER expire. A zero value results in entries that ALWAYS expire.
+   * @param sessionLengthUnit the unit of time for the timeToLive parameter, must not be null
+   */
+  public PassiveExpiringSessionCache(long sessionLength, TimeUnit sessionLengthUnit) {
+    cachedSessions = new PassiveExpiringMap<>(sessionLength, sessionLengthUnit);
+  }
+
+  /**
+   * Stores the initialized {@link ValidationEngine} in the cache. Returns the session id that will be associated with
+   * this instance.
+   * @param validationEngine {@link ValidationEngine}
+   * @return The {@link String} id associated with the stored instance.
+   */
+  public String cacheSession(ValidationEngine validationEngine) {
+    String generatedId = generateID();
+    cachedSessions.put(generatedId, validationEngine);
+    return generatedId;
+  }
+
+  /**
+   * Stores the initialized {@link ValidationEngine} in the cache with the passed in id as the key. If a null key is
+   * passed in, a new key is generated and returned.
+   * @param sessionId The {@link String} key to associate with this stored {@link ValidationEngine}
+   * @param validationEngine The {@link ValidationEngine} instance to cache.
+   * @return The {@link String} id that will be associated with the stored {@link ValidationEngine}
+   */
+  public String cacheSession(String sessionId, ValidationEngine validationEngine) {
+    if(sessionId == null) {
+      sessionId = cacheSession(validationEngine);
+    } else {
+      cachedSessions.put(sessionId, validationEngine);
+    }
+    return sessionId;
+  }
+
+  /**
+   * Sets whether or not a cached Session entry's expiration time is reset after session fetches are performed.
+   * @param resetExpirationAfterFetch If true, when sessions are fetched, their expiry time will be reset to sessionLength
+   * @return The {@link SessionCache} with the explicit expiration policy
+   */
+  public PassiveExpiringSessionCache setResetExpirationAfterFetch(boolean resetExpirationAfterFetch) {
+    this.resetExpirationAfterFetch = resetExpirationAfterFetch;
+    return this;
+  }
+
+  /**
+   * When called, this actively checks the cache for expired entries and removes
+   * them.
+   */
+  protected void removeExpiredSessions() {
+    /*
+    The PassiveExpiringMap will remove entries when accessing the mapped value
+    for a key, OR when invoking methods that involve accessing the entire map
+    contents. So, we call keySet below to force removal of all expired entries.
+    * */
+    cachedSessions.keySet();
+  }
+
+  /**
+   * Checks if the passed in {@link String} id exists in the set of stored session ids.
+   *
+   * As an optimization, when this is called, any expired sessions are also removed.
+   *
+   * @param sessionId The {@link String} id to search for.
+   * @return {@link Boolean#TRUE} if such id exists.
+   */
+  public boolean sessionExists(String sessionId) {
+    removeExpiredSessions();
+    return cachedSessions.containsKey(sessionId);
+  }
+
+  /**
+   * Returns the stored {@link ValidationEngine} associated with the passed in session id, if one such instance exists.
+   * @param sessionId The {@link String} session id.
+   * @return The {@link ValidationEngine} associated with the passed in id, or null if none exists.
+   */
+  public ValidationEngine fetchSessionValidatorEngine(String sessionId) {
+    ValidationEngine engine = cachedSessions.get(sessionId);
+    if (this.resetExpirationAfterFetch) {
+      cachedSessions.put(sessionId, engine);
+    }   
+    return engine;
+  }
+
+  /**
+   * Returns the set of stored session ids.
+   * @return {@link Set} of session ids.
+   */
+  public Set<String> getSessionIds() {
+    return cachedSessions.keySet();
+  }
+
+  /**
+   * Session ids generated internally are UUID {@link String}.
+   * @return A new {@link String} session id.
+   */
+  private String generateID() {
+    return UUID.randomUUID().toString();
+  }
+}

org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/services/SessionCache.java

@@ -1,35 +1,10 @@
 package org.hl7.fhir.validation.cli.services;
 
 import java.util.Set;
-import java.util.UUID;
-import java.util.concurrent.TimeUnit;
 
-import org.apache.commons.collections4.map.PassiveExpiringMap;
 import org.hl7.fhir.validation.ValidationEngine;
 
-/**
- * SessionCache for storing and retrieving ValidationEngine instances, so callers do not have to re-instantiate a new
- * instance for each validation request.
- */
-public class SessionCache {
-
-  protected static final long TIME_TO_LIVE = 60;
-  protected static final TimeUnit TIME_UNIT = TimeUnit.MINUTES;
-
-  private final PassiveExpiringMap<String, ValidationEngine> cachedSessions;
-
-  public SessionCache() {
-    cachedSessions = new PassiveExpiringMap<>(TIME_TO_LIVE, TIME_UNIT);
-  }
-
-  /**
-   * @param sessionLength the constant amount of time an entry is available before it expires. A negative value results
-   *                      in entries that NEVER expire. A zero value results in entries that ALWAYS expire.
-   * @param sessionLengthUnit the unit of time for the timeToLive parameter, must not be null
-   */
-  public SessionCache(long sessionLength, TimeUnit sessionLengthUnit) {
-    cachedSessions = new PassiveExpiringMap<>(sessionLength, sessionLengthUnit);
-  }
+public interface SessionCache {
 
   /**
    * Stores the initialized {@link ValidationEngine} in the cache. Returns the session id that will be associated with
@@ -37,11 +12,7 @@ public class SessionCache {
    * @param validationEngine {@link ValidationEngine}
    * @return The {@link String} id associated with the stored instance.
    */
-  public String cacheSession(ValidationEngine validationEngine) {
-    String generatedId = generateID();
-    cachedSessions.put(generatedId, validationEngine);
-    return generatedId;
-  }
+  String cacheSession(ValidationEngine validationEngine);
 
   /**
    * Stores the initialized {@link ValidationEngine} in the cache with the passed in id as the key. If a null key is
@@ -50,59 +21,29 @@ public class SessionCache {
    * @param validationEngine The {@link ValidationEngine} instance to cache.
    * @return The {@link String} id that will be associated with the stored {@link ValidationEngine}
    */
-  public String cacheSession(String sessionId, ValidationEngine validationEngine) {
-    if(sessionId == null) {
-      sessionId = cacheSession(validationEngine);
-    } else {
-      cachedSessions.put(sessionId, validationEngine);
-    }
-    return sessionId;
-  }
+  String cacheSession(String sessionId, ValidationEngine validationEngine);
+
+
 
-  /**
-   * When called, this actively checks the cache for expired entries and removes
-   * them.
-   */
-  public void removeExpiredSessions() {
-    /*
-    The PassiveExpiringMap will remove entries when accessing the mapped value
-    for a key, OR when invoking methods that involve accessing the entire map
-    contents. So, we call keySet below to force removal of all expired entries.
-    * */
-    cachedSessions.keySet();
-  }
 
   /**
    * Checks if the passed in {@link String} id exists in the set of stored session id.
    * @param sessionId The {@link String} id to search for.
    * @return {@link Boolean#TRUE} if such id exists.
    */
-  public boolean sessionExists(String sessionId) {
-    return cachedSessions.containsKey(sessionId);
-  }
+  boolean sessionExists(String sessionId);
 
   /**
    * Returns the stored {@link ValidationEngine} associated with the passed in session id, if one such instance exists.
    * @param sessionId The {@link String} session id.
    * @return The {@link ValidationEngine} associated with the passed in id, or null if none exists.
    */
-  public ValidationEngine fetchSessionValidatorEngine(String sessionId) {
-    return cachedSessions.get(sessionId);
-  }
+  ValidationEngine fetchSessionValidatorEngine(String sessionId);
 
   /**
    * Returns the set of stored session ids.
    * @return {@link Set} of session ids.
    */
-  public Set<String> getSessionIds() {
-    return cachedSessions.keySet();
-  }
-
-  /**
-   * Session ids generated internally are UUID {@link String}.
-   * @return A new {@link String} session id.
-   */
-  private String generateID() {
-    return UUID.randomUUID().toString();
-  }
-}
+  Set<String> getSessionIds();
+    
+}

org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/cli/services/ValidationService.java

@@ -75,11 +75,11 @@ public class ValidationService {
   private String runDate;
 
   public ValidationService() {
-    sessionCache = new SessionCache();
+    sessionCache = new PassiveExpiringSessionCache();
     runDate = new SimpleDateFormat("hh:mm:ss", new Locale("en", "US")).format(new Date());
   }
 
-  protected ValidationService(SessionCache cache) {
+  public ValidationService(SessionCache cache) {
     this.sessionCache = cache;
   }
 
@@ -438,7 +438,7 @@ public class ValidationService {
 
   public String initializeValidator(CliContext cliContext, String definitions, TimeTracker tt, String sessionId) throws Exception {
     tt.milestone();
-    sessionCache.removeExpiredSessions();
+
     if (!sessionCache.sessionExists(sessionId)) {
       if (sessionId != null) {
         System.out.println("No such cached session exists for session id " + sessionId + ", re-instantiating validator.");

org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/cli/services/PassiveExpiringSessionCacheTest.java

@@ -9,13 +9,13 @@ import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 
-class SessionCacheTest {
+class PassiveExpiringSessionCacheTest {
 
   @Test
   @DisplayName("test session expiration works")
   void expiredSession() throws IOException, InterruptedException {
     final long EXPIRE_TIME = 5L;
-    SessionCache cache = new SessionCache(EXPIRE_TIME, TimeUnit.SECONDS);
+    SessionCache cache = new PassiveExpiringSessionCache(EXPIRE_TIME, TimeUnit.SECONDS);
     ValidationEngine testEngine = new ValidationEngine.ValidationEngineBuilder().fromNothing();
     String sessionId = cache.cacheSession(testEngine);
     TimeUnit.SECONDS.sleep(EXPIRE_TIME + 1L);
@@ -26,7 +26,7 @@ class SessionCacheTest {
   @DisplayName("test session caching works")
   void cachedSession() throws IOException {
     final long EXPIRE_TIME = 5L;
-    SessionCache cache = new SessionCache(EXPIRE_TIME, TimeUnit.SECONDS);
+    SessionCache cache = new PassiveExpiringSessionCache(EXPIRE_TIME, TimeUnit.SECONDS);
     ValidationEngine testEngine = new ValidationEngine.ValidationEngineBuilder().fromNothing();
     String sessionId = cache.cacheSession(testEngine);
     Assertions.assertEquals(testEngine, cache.fetchSessionValidatorEngine(sessionId));
@@ -35,7 +35,7 @@ class SessionCacheTest {
   @Test
   @DisplayName("test session exists")
   void sessionExists() throws IOException {
-    SessionCache cache = new SessionCache();
+    SessionCache cache = new PassiveExpiringSessionCache();
     ValidationEngine testEngine = new ValidationEngine.ValidationEngineBuilder().fromNothing();
     String sessionId = cache.cacheSession(testEngine);
     Assertions.assertTrue(cache.sessionExists(sessionId));
@@ -45,15 +45,15 @@ class SessionCacheTest {
   @Test
   @DisplayName("test null session test id returns false")
   void testNullSessionExists() {
-    SessionCache cache = new SessionCache();
+    SessionCache cache = new PassiveExpiringSessionCache();
     Assertions.assertFalse(cache.sessionExists(null));
   }
 
   @Test
-  @DisplayName("test that explicit removeExiredSessions works")
+  @DisplayName("test that explicit removeExpiredSessions works")
   void testRemoveExpiredSessions() throws InterruptedException, IOException {
     final long EXPIRE_TIME = 5L;
-    SessionCache cache = new SessionCache(EXPIRE_TIME, TimeUnit.SECONDS);
+    PassiveExpiringSessionCache cache = new PassiveExpiringSessionCache(EXPIRE_TIME, TimeUnit.SECONDS);
     ValidationEngine testEngine = new ValidationEngine.ValidationEngineBuilder().fromNothing();
     String sessionId = cache.cacheSession(testEngine);
     Assertions.assertTrue(cache.sessionExists(sessionId));
@@ -61,4 +61,18 @@ class SessionCacheTest {
     cache.removeExpiredSessions();
     Assertions.assertTrue(cache.getSessionIds().isEmpty());
   }
+
+  @Test
+  @DisplayName("test that explicitly configured expiration reset works")
+  void testConfigureDuration() throws InterruptedException, IOException {
+    final long EXPIRE_TIME = 15L;
+    PassiveExpiringSessionCache cache = new PassiveExpiringSessionCache(EXPIRE_TIME, TimeUnit.SECONDS).setResetExpirationAfterFetch(true);
+    ValidationEngine testEngine = new ValidationEngine.ValidationEngineBuilder().fromNothing();
+    String sessionId = cache.cacheSession(testEngine);
+    TimeUnit.SECONDS.sleep(10L);
+    Assertions.assertTrue(cache.sessionExists(sessionId));
+    cache.fetchSessionValidatorEngine(sessionId);
+    TimeUnit.SECONDS.sleep(10L);
+    Assertions.assertTrue(cache.sessionExists(sessionId));
+  }
 }

org.hl7.fhir.validation/src/test/java/org/hl7/fhir/validation/cli/services/ValidationServiceTest.java

@@ -55,7 +55,7 @@ class ValidationServiceTest  {
   @Test
   void validateSources() throws Exception {
     TestingUtilities.injectCorePackageLoader();
-    SessionCache sessionCache = Mockito.spy(new SessionCache());
+    SessionCache sessionCache = Mockito.spy(new PassiveExpiringSessionCache());
     ValidationService myService = new ValidationService(sessionCache);
 
     String resource = IOUtils.toString(getFileFromResourceAsStream("detected_issues.json"), StandardCharsets.UTF_8);