[java-opensaml] 01/01: OSJ-358 - Turn Revocation/ReplayCache into interfaces/implementations

Scott Cantor cantor.2 at osu.edu
Fri Aug 5 18:34:27 UTC 2022


This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch dev/OSJ-358
in repository java-opensaml.

View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=3cdd69c3c7cb3866bb985620f60451b2a690575d

commit 3cdd69c3c7cb3866bb985620f60451b2a690575d
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Fri Aug 5 14:33:52 2022 -0400

    OSJ-358 - Turn Revocation/ReplayCache into interfaces/implementations
    
    https://shibboleth.atlassian.net/browse/OSJ-358
---
 .../java/org/opensaml/storage/ReplayCache.java     | 129 +---------
 .../java/org/opensaml/storage/RevocationCache.java | 262 ++-------------------
 .../storage/impl/StorageServiceReplayCache.java    |  25 +-
 .../impl/StorageServiceRevocationCache.java        | 113 ++-------
 ...est.java => StorageServiceReplayCacheTest.java} |  14 +-
 ...java => StorageServiceRevocationCacheTest.java} |  19 +-
 6 files changed, 78 insertions(+), 484 deletions(-)

diff --git a/opensaml-storage-api/src/main/java/org/opensaml/storage/ReplayCache.java b/opensaml-storage-api/src/main/java/org/opensaml/storage/ReplayCache.java
index fece6681a..a4a8d3143 100644
--- a/opensaml-storage-api/src/main/java/org/opensaml/storage/ReplayCache.java
+++ b/opensaml-storage-api/src/main/java/org/opensaml/storage/ReplayCache.java
@@ -17,144 +17,29 @@
 
 package org.opensaml.storage;
 
-import java.io.IOException;
-import java.security.NoSuchAlgorithmException;
 import java.time.Instant;
 
 import javax.annotation.Nonnull;
+import javax.annotation.concurrent.ThreadSafe;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
-import net.shibboleth.utilities.java.support.annotation.constraint.ThreadSafeAfterInit;
-import net.shibboleth.utilities.java.support.codec.StringDigester;
-import net.shibboleth.utilities.java.support.codec.StringDigester.OutputFormat;
-import net.shibboleth.utilities.java.support.component.AbstractIdentifiableInitializableComponent;
-import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
-import net.shibboleth.utilities.java.support.logic.Constraint;
 
 /**
- * Tracks non-replayable values in order to detect replays of the values, commonly used to track message identifiers.
- * 
- * <p>This class is thread-safe and uses a synchronized method to prevent race conditions within the underlying
- * store (lacking an atomic "check and insert" operation).</p>
+ * Interface to a component that checks for replay of a value.
  */
- at ThreadSafeAfterInit
-public class ReplayCache extends AbstractIdentifiableInitializableComponent {
-
-    /** Logger. */
-    private final Logger log = LoggerFactory.getLogger(ReplayCache.class);
-
-    /** Backing storage for the replay cache. */
-    @NonnullAfterInit private StorageService storage;
-
-    /** Digester if key is too long. */
-    @NonnullAfterInit private StringDigester digester;
-    
-    /** Flag controlling behavior on storage failure. */
-    private boolean strict;
-    
-    /**
-     * Get the backing store for the cache.
-     * 
-     * @return the backing store.
-     */
-    @NonnullAfterInit public StorageService getStorage() {
-        return storage;
-    }
-    
-    /**
-     * Set the backing store for the cache.
-     * 
-     * @param storageService backing store to use
-     */
-    public void setStorage(@Nonnull final StorageService storageService) {
-        checkSetterPreconditions();
-        
-        storage = Constraint.isNotNull(storageService, "StorageService cannot be null");
-        final StorageCapabilities caps = storage.getCapabilities();
-        if (caps instanceof StorageCapabilitiesEx) {
-            Constraint.isTrue(((StorageCapabilitiesEx) caps).isServerSide(), "StorageService cannot be client-side");
-        }
-    }
-    
-    /**
-     * Get the strictness flag.
-     * 
-     * @return true iff we should treat storage failures as a replay
-     */
-    public boolean isStrict() {
-        return strict;
-    }
-
-    /**
-     * Set the strictness flag.
-     * 
-     * @param flag true iff we should treat storage failures as a replay
-     */
-    public void setStrict(final boolean flag) {
-        checkSetterPreconditions();
-        
-        strict = flag;
-    }
-
-
-    /** {@inheritDoc} */
-    @Override
-    public void doInitialize() throws ComponentInitializationException {
-        if (storage == null) {
-            throw new ComponentInitializationException("StorageService cannot be null");
-        }
-
-        try {
-            digester = new StringDigester("SHA", OutputFormat.HEX_LOWER);
-        } catch (final NoSuchAlgorithmException e) {
-            throw new ComponentInitializationException(e);
-        }
-    }
+ at ThreadSafe
+public interface ReplayCache {
 
     /**
      * Returns true iff the check value is not found in the cache, and stores it.
      * 
      * @param context   a context label to subdivide the cache
-     * @param s         value to check
+     * @param key       key to check
      * @param expires   time for disposal of value from cache
      * 
      * @return true iff the check value is not found in the cache
      */
-    public synchronized boolean check(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String s,
-            @Nonnull final Instant expires) {
-
-        final String key;
-        
-        final StorageCapabilities caps = storage.getCapabilities();
-        if (context.length() > caps.getContextSize()) {
-            log.error("Context '{}' too long for StorageService (limit {})", context, caps.getContextSize());
-            return false;
-        } else if (s.length() > caps.getKeySize()) {
-            key = digester.apply(s);
-        } else {
-            key = s;
-        }
-
-        try {
-            final StorageRecord<?> entry = storage.read(context, key);
-            if (entry == null) {
-                log.debug("Value '{}' was not a replay, adding to cache with expiration time {}", s, expires);
-                storage.create(context, key, "x", expires.toEpochMilli());
-                return true;
-            }
-            
-            log.debug("Replay of value '{}' detected in cache, expires at {}", s,
-                    Instant.ofEpochMilli(entry.getExpiration()));
-            return false;
-            
-        } catch (final IOException e) {
-            log.error("Exception reading/writing to storage service, returning {}", strict ? "failure" : "success", e);
-            return !strict;
-        }
-    }
+    boolean check(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String key,
+            @Nonnull final Instant expires);
 
 }
\ No newline at end of file
diff --git a/opensaml-storage-api/src/main/java/org/opensaml/storage/RevocationCache.java b/opensaml-storage-api/src/main/java/org/opensaml/storage/RevocationCache.java
index c8846cbca..43b1f1d2d 100644
--- a/opensaml-storage-api/src/main/java/org/opensaml/storage/RevocationCache.java
+++ b/opensaml-storage-api/src/main/java/org/opensaml/storage/RevocationCache.java
@@ -19,122 +19,21 @@ package org.opensaml.storage;
 
 import java.io.IOException;
 import java.time.Duration;
-import java.time.Instant;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
 
-import org.apache.commons.codec.digest.DigestUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
-import net.shibboleth.utilities.java.support.annotation.constraint.Positive;
-import net.shibboleth.utilities.java.support.annotation.constraint.ThreadSafeAfterInit;
-import net.shibboleth.utilities.java.support.component.AbstractIdentifiableInitializableComponent;
-import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
-import net.shibboleth.utilities.java.support.logic.Constraint;
 
 /**
- * Stores and checks for revocation entries.
- * 
- * <p>
- * This class is thread-safe and uses a synchronized method to prevent race conditions within the underlying store
- * (lacking an atomic "check and insert" operation).
- * </p>
+ * Interface to a cache that tracks revoked information.
  * 
- * @since 4.2.0
+ * <p>Revocation may include specific information for storage and retrieval,
+ * or simply a tracking of revoked status.</p>
  */
- at ThreadSafeAfterInit
-public class RevocationCache extends AbstractIdentifiableInitializableComponent {
-
-    /** Logger. */
-    @Nonnull private final Logger log = LoggerFactory.getLogger(RevocationCache.class);
-
-    /** Backing storage for the replay cache. */
-    @NonnullAfterInit private StorageService storage;
-
-    /** Flag controlling behavior on storage failure. */
-    private boolean strict;
-
-    /** Default lifetime of revocation entry. Default value: 6 hours */
-    @Nonnull @Positive private Duration expires;
-
-    /**
-     * Constructor.
-     */
-    public RevocationCache() {
-        expires = Duration.ofHours(6);
-    }
-
-    /**
-     * Set the default revocation entry expiration.
-     * 
-     * @param entryExpiration lifetime of an revocation entry in milliseconds
-     */
-    public void setEntryExpiration(@Positive final Duration entryExpiration) {
-        checkSetterPreconditions();
-        
-        Constraint.isTrue(entryExpiration != null && !entryExpiration.isNegative() && !entryExpiration.isZero(),
-                "Revocation cache default entry expiration must be greater than 0");
-        expires = entryExpiration;
-    }
-
-    /**
-     * Get the backing store for the cache.
-     * 
-     * @return the backing store.
-     */
-    @NonnullAfterInit public StorageService getStorage() {
-        return storage;
-    }
-
-    /**
-     * Set the backing store for the cache.
-     * 
-     * @param storageService backing store to use
-     */
-    public void setStorage(@Nonnull final StorageService storageService) {
-        checkSetterPreconditions();
-
-        storage = Constraint.isNotNull(storageService, "StorageService cannot be null");
-        final StorageCapabilities caps = storage.getCapabilities();
-        if (caps instanceof StorageCapabilitiesEx) {
-            Constraint.isTrue(((StorageCapabilitiesEx) caps).isServerSide(), "StorageService cannot be client-side");
-        }
-    }
-
-    /**
-     * Get the strictness flag.
-     * 
-     * @return true iff we should treat storage failures as a revocation
-     */
-    public boolean isStrict() {
-        return strict;
-    }
-
-    /**
-     * Set the strictness flag.
-     * 
-     * @param flag true iff we should treat storage failures as a revocation
-     */
-    public void setStrict(final boolean flag) {
-        checkSetterPreconditions();
-
-        strict = flag;
-    }
-
-    /** {@inheritDoc} */
-    @Override
-    public void doInitialize() throws ComponentInitializationException {
-        super.doInitialize();
-        
-        if (storage == null) {
-            throw new ComponentInitializationException("StorageService cannot be null");
-        }
-    }
-    
+ at ThreadSafe
+public interface RevocationCache {
 
     /**
      * Invokes {@link #revoke(String, String, Duration)} with a default expiration parameter.
@@ -144,9 +43,9 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
      * 
      * @return true if key has successfully been listed as revoked in the cache
      */
-    public synchronized boolean revoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String key) {
-        return revoke(context, key, expires);
-    }    
+    default boolean revoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final  String key) {
+        return revoke(context, key, "y");
+    }
 
     /**
      * Invokes {@link #revoke(String, String, String, Duration)} with a placeholder value parameter.
@@ -159,12 +58,11 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
      * 
      * @since 4.3.0
      */
-    public synchronized boolean revoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String key,
+    default boolean revoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String key,
             @Nonnull final Duration exp) {
         return revoke(context, key, "y", exp);
     }
 
-    
     /**
      * Invokes {@link #revoke(String, String, String, Duration)} with a default expiration parameter.
      * 
@@ -178,18 +76,16 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
      * 
      * @since 4.3.0
      */
-    public synchronized boolean revoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String key,
-            @Nonnull @NotEmpty final String value) {
-        return revoke(context, key, value, expires);
-    }
-    
+    boolean revoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String key,
+            @Nonnull @NotEmpty final String value);
+
     /**
      * Returns true if the value is successfully revoked.
      * 
      * <p>If the key has already been revoked, expiration is updated.</p>
      * 
      * @param context a context label to subdivide the cache
-     * @param s key to revoke
+     * @param key key to revoke
      * @param value value to insert into revocation record
      * @param exp entry expiration
      * 
@@ -197,107 +93,30 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
      * 
      * @since 4.3.0
      */
-    public synchronized boolean revoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String s,
-            @Nonnull @NotEmpty final String value, @Nonnull final Duration exp) {
-        checkComponentActive();
-        
-        final String key;
-        final StorageCapabilities caps = storage.getCapabilities();
-        if (context.length() > caps.getContextSize()) {
-            log.error("context {} too long for StorageService (limit {})", context, caps.getContextSize());
-            return false;
-        } else if (s.length() > caps.getKeySize()) {
-            key = DigestUtils.sha1Hex(s);
-        } else {
-            key = s;
-        }
-        try {
-            final StorageRecord<?> entry = storage.read(context, key);
-            if (entry == null) {
-                log.debug("Entry '{}' of context '{}' is not yet on list of revoked entries,"
-                        + " adding to cache with expiration time {}", key, context, expires);
-                storage.create(context, key, value, Instant.now().plus(exp).toEpochMilli());
-                return true;
-            }
-            
-            storage.updateExpiration(context, key, Instant.now().plus(exp).toEpochMilli());
-            log.debug("Entry '{}' of context '{}' was already revoked, updating expiration", key, context);
-            return true;
-        } catch (final IOException e) {
-            log.error("Exception reading/writing to storage service, returning {}", strict ? "failure" : "success", e);
-            return !strict;
-        }
-    }
-    
+    boolean revoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String key,
+            @Nonnull @NotEmpty final String value, @Nonnull final Duration exp);
+
     /**
      * Remove a revocation record.
      * 
      * @param context a context label to subdivide the cache
-     * @param s value to remove
+     * @param key value to remove
      * 
      * @return true iff a record was removed
      * 
      * @since 4.3.0
      */
-    public synchronized boolean unrevoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String s) {
-        checkComponentActive();
-        
-        final String key;
-        final StorageCapabilities caps = storage.getCapabilities();
-        if (context.length() > caps.getContextSize()) {
-            log.error("context {} too long for StorageService (limit {})", context, caps.getContextSize());
-            return false;
-        } else if (s.length() > caps.getKeySize()) {
-            key = DigestUtils.sha1Hex(s);
-        } else {
-            key = s;
-        }
-
-        try {
-            return storage.delete(context, key);
-        } catch (final IOException e) {
-            log.error("Exception writing to storage service", e);
-            return false;
-        }
-    }
+    boolean unrevoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String key);
 
     /**
      * Returns true iff the value has been revoked.
      * 
      * @param context a context label to subdivide the cache
-     * @param s value to check
+     * @param key value to check
      * 
      * @return true iff the check value is found in the cache
      */
-    public synchronized boolean isRevoked(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String s) {
-        checkComponentActive();
-
-        final String key;
-        final StorageCapabilities caps = storage.getCapabilities();
-        if (context.length() > caps.getContextSize()) {
-            log.error("context {} too long for StorageService (limit {})", context, caps.getContextSize());
-            return true;
-        } else if (s.length() > caps.getKeySize()) {
-            key = DigestUtils.sha1Hex(s);
-        } else {
-            key = s;
-        }
-
-        try {
-            final StorageRecord<?> entry = storage.read(context, key);
-            if (entry == null) {
-                log.debug("Entry '{}' is not revoked", key);
-                return false;
-            }
-            
-            log.debug("Entry '{}' is revoked", s);
-            return true;
-        } catch (final IOException e) {
-            log.error("Exception reading  storage service, indicating {}",
-                    strict ? "revoked" : "not revoked", e);
-            return strict;
-        }
-    }
+    boolean isRevoked(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String key);
 
     /**
      * Attempts to read back a revocation record for a given context and key.
@@ -306,7 +125,7 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
      * rather than simple presence/absence as a signal.</p>
      * 
      * @param context revocation context
-     * @param s revocation key
+     * @param key revocation key
      * 
      * @return the matching record, if found, or null if absent
      * 
@@ -314,38 +133,7 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
      * 
      * @since 4.3.0
      */
-    @Nullable @NotEmpty public synchronized String getRevocationRecord(@Nonnull @NotEmpty final String context,
-            @Nonnull @NotEmpty final String s) throws IOException {
-        checkComponentActive();
-
-        final String key;
-        final StorageCapabilities caps = storage.getCapabilities();
-        if (context.length() > caps.getContextSize()) {
-            log.error("context {} too long for StorageService (limit {})", context, caps.getContextSize());
-            throw new IOException("Context exceeded storage service limit.");
-        } else if (s.length() > caps.getKeySize()) {
-            key = DigestUtils.sha1Hex(s);
-        } else {
-            key = s;
-        }
+    @Nullable String getRevocationRecord(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String key)
+            throws IOException;
 
-        try {
-            final StorageRecord<?> entry = storage.read(context, key);
-            if (entry == null) {
-                log.debug("Entry '{}' is not revoked", key);
-                return null;
-            }
-        
-            log.debug("Entry '{}' is revoked", s);
-            return entry.getValue();
-        } catch (final IOException e) {
-            if (strict) {
-                throw e;
-            }
-            
-            log.error("Exception reading from storage service, non-strict so treating as non-revoked", e);
-            return null;
-        }
-    }
-    
-}
+}
\ No newline at end of file
diff --git a/opensaml-storage-api/src/main/java/org/opensaml/storage/ReplayCache.java b/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/StorageServiceReplayCache.java
similarity index 89%
copy from opensaml-storage-api/src/main/java/org/opensaml/storage/ReplayCache.java
copy to opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/StorageServiceReplayCache.java
index fece6681a..83def11c6 100644
--- a/opensaml-storage-api/src/main/java/org/opensaml/storage/ReplayCache.java
+++ b/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/StorageServiceReplayCache.java
@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-package org.opensaml.storage;
+package org.opensaml.storage.impl;
 
 import java.io.IOException;
 import java.security.NoSuchAlgorithmException;
@@ -23,6 +23,11 @@ import java.time.Instant;
 
 import javax.annotation.Nonnull;
 
+import org.opensaml.storage.ReplayCache;
+import org.opensaml.storage.StorageCapabilities;
+import org.opensaml.storage.StorageCapabilitiesEx;
+import org.opensaml.storage.StorageRecord;
+import org.opensaml.storage.StorageService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -36,16 +41,18 @@ import net.shibboleth.utilities.java.support.component.ComponentInitializationEx
 import net.shibboleth.utilities.java.support.logic.Constraint;
 
 /**
- * Tracks non-replayable values in order to detect replays of the values, commonly used to track message identifiers.
+ * {@link ReplayCache} implementation backed by a {@link StorageService}.
  * 
  * <p>This class is thread-safe and uses a synchronized method to prevent race conditions within the underlying
  * store (lacking an atomic "check and insert" operation).</p>
+ * 
+ * @since 5.0.0
  */
 @ThreadSafeAfterInit
-public class ReplayCache extends AbstractIdentifiableInitializableComponent {
+public class StorageServiceReplayCache extends AbstractIdentifiableInitializableComponent implements ReplayCache {
 
     /** Logger. */
-    private final Logger log = LoggerFactory.getLogger(ReplayCache.class);
+    @Nonnull private final Logger log = LoggerFactory.getLogger(StorageServiceReplayCache.class);
 
     /** Backing storage for the replay cache. */
     @NonnullAfterInit private StorageService storage;
@@ -115,15 +122,7 @@ public class ReplayCache extends AbstractIdentifiableInitializableComponent {
         }
     }
 
-    /**
-     * Returns true iff the check value is not found in the cache, and stores it.
-     * 
-     * @param context   a context label to subdivide the cache
-     * @param s         value to check
-     * @param expires   time for disposal of value from cache
-     * 
-     * @return true iff the check value is not found in the cache
-     */
+    /** {@inheritDoc} */
     public synchronized boolean check(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String s,
             @Nonnull final Instant expires) {
 
diff --git a/opensaml-storage-api/src/main/java/org/opensaml/storage/RevocationCache.java b/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/StorageServiceRevocationCache.java
similarity index 74%
copy from opensaml-storage-api/src/main/java/org/opensaml/storage/RevocationCache.java
copy to opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/StorageServiceRevocationCache.java
index c8846cbca..565bb45f1 100644
--- a/opensaml-storage-api/src/main/java/org/opensaml/storage/RevocationCache.java
+++ b/opensaml-storage-impl/src/main/java/org/opensaml/storage/impl/StorageServiceRevocationCache.java
@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-package org.opensaml.storage;
+package org.opensaml.storage.impl;
 
 import java.io.IOException;
 import java.time.Duration;
@@ -25,6 +25,11 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import org.apache.commons.codec.digest.DigestUtils;
+import org.opensaml.storage.RevocationCache;
+import org.opensaml.storage.StorageCapabilities;
+import org.opensaml.storage.StorageCapabilitiesEx;
+import org.opensaml.storage.StorageRecord;
+import org.opensaml.storage.StorageService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -37,20 +42,21 @@ import net.shibboleth.utilities.java.support.component.ComponentInitializationEx
 import net.shibboleth.utilities.java.support.logic.Constraint;
 
 /**
- * Stores and checks for revocation entries.
+ * Stores and checks for revocation entries via a {@link StorageService}.
  * 
  * <p>
  * This class is thread-safe and uses a synchronized method to prevent race conditions within the underlying store
  * (lacking an atomic "check and insert" operation).
  * </p>
  * 
- * @since 4.2.0
+ * @since 5.0.0
  */
 @ThreadSafeAfterInit
-public class RevocationCache extends AbstractIdentifiableInitializableComponent {
+public class StorageServiceRevocationCache extends AbstractIdentifiableInitializableComponent
+        implements RevocationCache {
 
     /** Logger. */
-    @Nonnull private final Logger log = LoggerFactory.getLogger(RevocationCache.class);
+    @Nonnull private final Logger log = LoggerFactory.getLogger(StorageServiceRevocationCache.class);
 
     /** Backing storage for the replay cache. */
     @NonnullAfterInit private StorageService storage;
@@ -64,7 +70,7 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
     /**
      * Constructor.
      */
-    public RevocationCache() {
+    public StorageServiceRevocationCache() {
         expires = Duration.ofHours(6);
     }
 
@@ -135,68 +141,13 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
         }
     }
     
-
-    /**
-     * Invokes {@link #revoke(String, String, Duration)} with a default expiration parameter.
-     * 
-     * @param context a context label to subdivide the cache
-     * @param key key to revoke
-     * 
-     * @return true if key has successfully been listed as revoked in the cache
-     */
-    public synchronized boolean revoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String key) {
-        return revoke(context, key, expires);
-    }    
-
-    /**
-     * Invokes {@link #revoke(String, String, String, Duration)} with a placeholder value parameter.
-     * 
-     * @param context a context label to subdivide the cache
-     * @param key key to revoke
-     * @param exp entry expiration
-     * 
-     * @return true if key has successfully been listed as revoked in the cache
-     * 
-     * @since 4.3.0
-     */
-    public synchronized boolean revoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String key,
-            @Nonnull final Duration exp) {
-        return revoke(context, key, "y", exp);
-    }
-
-    
-    /**
-     * Invokes {@link #revoke(String, String, String, Duration)} with a default expiration parameter.
-     * 
-     * <p>If the key has already been revoked, expiration is updated.</p>
-     * 
-     * @param context a context label to subdivide the cache
-     * @param key key to revoke
-     * @param value value to insert into revocation record
-     * 
-     * @return true if key has successfully been listed as revoked in the cache
-     * 
-     * @since 4.3.0
-     */
+    /** {@inheritDoc} */
     public synchronized boolean revoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String key,
             @Nonnull @NotEmpty final String value) {
         return revoke(context, key, value, expires);
     }
     
-    /**
-     * Returns true if the value is successfully revoked.
-     * 
-     * <p>If the key has already been revoked, expiration is updated.</p>
-     * 
-     * @param context a context label to subdivide the cache
-     * @param s key to revoke
-     * @param value value to insert into revocation record
-     * @param exp entry expiration
-     * 
-     * @return true if key has successfully been listed as revoked in the cache
-     * 
-     * @since 4.3.0
-     */
+    /** {@inheritDoc} */
     public synchronized boolean revoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String s,
             @Nonnull @NotEmpty final String value, @Nonnull final Duration exp) {
         checkComponentActive();
@@ -229,16 +180,7 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
         }
     }
     
-    /**
-     * Remove a revocation record.
-     * 
-     * @param context a context label to subdivide the cache
-     * @param s value to remove
-     * 
-     * @return true iff a record was removed
-     * 
-     * @since 4.3.0
-     */
+    /** {@inheritDoc} */
     public synchronized boolean unrevoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String s) {
         checkComponentActive();
         
@@ -261,14 +203,7 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
         }
     }
 
-    /**
-     * Returns true iff the value has been revoked.
-     * 
-     * @param context a context label to subdivide the cache
-     * @param s value to check
-     * 
-     * @return true iff the check value is found in the cache
-     */
+    /** {@inheritDoc} */
     public synchronized boolean isRevoked(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String s) {
         checkComponentActive();
 
@@ -299,21 +234,7 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
         }
     }
 
-    /**
-     * Attempts to read back a revocation record for a given context and key.
-     * 
-     * <p>This alternative approach allows revocation records to include richer data,
-     * rather than simple presence/absence as a signal.</p>
-     * 
-     * @param context revocation context
-     * @param s revocation key
-     * 
-     * @return the matching record, if found, or null if absent
-     * 
-     * @throws IOException raised if an error occurs leading to an indeterminate result
-     * 
-     * @since 4.3.0
-     */
+    /** {@inheritDoc} */
     @Nullable @NotEmpty public synchronized String getRevocationRecord(@Nonnull @NotEmpty final String context,
             @Nonnull @NotEmpty final String s) throws IOException {
         checkComponentActive();
diff --git a/opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/ReplayCacheTest.java b/opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/StorageServiceReplayCacheTest.java
similarity index 93%
rename from opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/ReplayCacheTest.java
rename to opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/StorageServiceReplayCacheTest.java
index 416ce4ba9..4827561b5 100644
--- a/opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/ReplayCacheTest.java
+++ b/opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/StorageServiceReplayCacheTest.java
@@ -19,7 +19,6 @@ package org.opensaml.storage.impl;
 
 import java.time.Instant;
 
-import org.opensaml.storage.ReplayCache;
 import org.opensaml.storage.impl.client.ClientStorageService;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.Test;
@@ -27,9 +26,9 @@ import org.testng.annotations.BeforeMethod;
 import org.testng.Assert;
 
 /**
- * Tests for {@link ReplayCache}
+ * Tests for {@link StorageServiceReplayCache}.
  */
-public class ReplayCacheTest {
+public class StorageServiceReplayCacheTest {
 
     private String context;
     
@@ -39,7 +38,7 @@ public class ReplayCacheTest {
 
     private MemoryStorageService storageService;
     
-    private ReplayCache replayCache;
+    private StorageServiceReplayCache replayCache;
 
     @BeforeMethod
     protected void setUp() throws Exception {
@@ -51,7 +50,7 @@ public class ReplayCacheTest {
         storageService.setId("test");
         storageService.initialize();
         
-        replayCache = new ReplayCache();
+        replayCache = new StorageServiceReplayCache();
         replayCache.setStorage(storageService);
         replayCache.initialize();
     }
@@ -65,9 +64,12 @@ public class ReplayCacheTest {
         storageService = null;
     }
     
+    /**
+     * Test init methods.
+     */
     @Test
     public void testInit() {
-        replayCache = new ReplayCache();
+        replayCache = new StorageServiceReplayCache();
         try {
             replayCache.setStorage(null);
             Assert.fail("Null StorageService should have caused constraint violation");
diff --git a/opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/RevocationCacheTest.java b/opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/StorageServiceRevocationCacheTest.java
similarity index 92%
rename from opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/RevocationCacheTest.java
rename to opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/StorageServiceRevocationCacheTest.java
index b423af25a..efc97baa3 100644
--- a/opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/RevocationCacheTest.java
+++ b/opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/StorageServiceRevocationCacheTest.java
@@ -32,16 +32,15 @@ import net.shibboleth.utilities.java.support.component.ComponentInitializationEx
 import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
 
 import org.testng.annotations.BeforeMethod;
-import org.opensaml.storage.RevocationCache;
 
 /**
- * Tests for {@link RevocationCache}
+ * Tests for {@link StorageServiceRevocationCache}.
  */
-public class RevocationCacheTest {
+public class StorageServiceRevocationCacheTest {
     
     private MemoryStorageService storageService;
     
-    private RevocationCache revocationCache;
+    private StorageServiceRevocationCache revocationCache;
 
     @BeforeMethod
     protected void setUp() throws ComponentInitializationException {
@@ -51,7 +50,7 @@ public class RevocationCacheTest {
         storageService.setCleanupInterval(Duration.ZERO);
         storageService.initialize();
         
-        revocationCache = new RevocationCache();
+        revocationCache = new StorageServiceRevocationCache();
         revocationCache.setId("test");
         revocationCache.setEntryExpiration(Duration.ofMillis(500));
         revocationCache.setStorage(storageService);
@@ -66,7 +65,7 @@ public class RevocationCacheTest {
     
     @Test
     public void testInit() {
-        revocationCache = new RevocationCache();
+        revocationCache = new StorageServiceRevocationCache();
         try {
             revocationCache.setStorage(null);
             fail("Null StorageService should have caused constraint violation");
@@ -85,7 +84,7 @@ public class RevocationCacheTest {
     @Test
     public void testStrictSetter() throws ComponentInitializationException {
         assertFalse(revocationCache.isStrict());
-        revocationCache = new RevocationCache();
+        revocationCache = new StorageServiceRevocationCache();
         revocationCache.setId("test");
         revocationCache.setStorage(storageService);
         revocationCache.setStrict(true);
@@ -96,7 +95,7 @@ public class RevocationCacheTest {
     @Test (expectedExceptions = ConstraintViolationException.class)
     public void testExpirationSetter() {
         //Must be positive
-        revocationCache = new RevocationCache();
+        revocationCache = new StorageServiceRevocationCache();
         revocationCache.setEntryExpiration(Duration.ZERO);
     }
     
@@ -120,7 +119,7 @@ public class RevocationCacheTest {
         storageService.setCleanupInterval(Duration.ZERO);
         storageService.initialize();
         
-        revocationCache = new RevocationCache();
+        revocationCache = new StorageServiceRevocationCache();
         revocationCache.setId("test");
         revocationCache.setStorage(storageService);
         revocationCache.initialize();
@@ -140,7 +139,7 @@ public class RevocationCacheTest {
         storageService.setCleanupInterval(Duration.ZERO);
         storageService.setKeySize(50);
         storageService.initialize();
-        revocationCache = new RevocationCache();
+        revocationCache = new StorageServiceRevocationCache();
         revocationCache.setId("test");
         revocationCache.setStorage(storageService);
         revocationCache.initialize();

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list