[java-opensaml] branch main updated: IDP-2069 - Null Handling Task

Scott Cantor cantor.2 at osu.edu
Thu Mar 9 18:23:41 UTC 2023


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

scantor pushed a commit to branch main
in repository java-opensaml.

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

The following commit(s) were added to refs/heads/main by this push:
     new ad182fc5c IDP-2069 - Null Handling Task
ad182fc5c is described below

commit ad182fc5c660cf2a2a52f46cb2281bd0a4c619a6
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Mar 9 13:23:37 2023 -0500

    IDP-2069 - Null Handling Task
    
    https://shibboleth.atlassian.net/browse/IDP-2069
    
    Clean opensaml-storage-api.
---
 .../storage/AbstractMapBackedStorageService.java   |  7 ++-
 .../opensaml/storage/AbstractStorageService.java   | 72 +++++++++++++++++-----
 .../java/org/opensaml/storage/StorageRecord.java   |  4 +-
 .../storage/annotation/AnnotationSupport.java      | 40 +++++++-----
 .../org/opensaml/storage/annotation/Context.java   |  4 +-
 .../opensaml/storage/annotation/Expiration.java    |  4 +-
 .../java/org/opensaml/storage/annotation/Key.java  |  4 +-
 .../org/opensaml/storage/annotation/Value.java     |  4 +-
 8 files changed, 99 insertions(+), 40 deletions(-)

diff --git a/opensaml-storage-api/src/main/java/org/opensaml/storage/AbstractMapBackedStorageService.java b/opensaml-storage-api/src/main/java/org/opensaml/storage/AbstractMapBackedStorageService.java
index 97eb7c612..2f30265fa 100644
--- a/opensaml-storage-api/src/main/java/org/opensaml/storage/AbstractMapBackedStorageService.java
+++ b/opensaml-storage-api/src/main/java/org/opensaml/storage/AbstractMapBackedStorageService.java
@@ -33,9 +33,9 @@ import net.shibboleth.shared.annotation.constraint.NonnullElements;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.annotation.constraint.Positive;
 import net.shibboleth.shared.collection.Pair;
+import net.shibboleth.shared.primitive.LoggerFactory;
 
 import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Partial implementation of {@link StorageService} that stores data in-memory with no persistence
@@ -149,8 +149,8 @@ public abstract class AbstractMapBackedStorageService extends AbstractStorageSer
 
     /** {@inheritDoc} */
     @Override
-    public boolean deleteWithVersion(final long version, final String context, final String key) throws IOException,
-            VersionMismatchException {
+    public boolean deleteWithVersion(final long version, @Nonnull final String context, @Nonnull final String key)
+            throws IOException, VersionMismatchException {
         return deleteImpl(version, context, key);
     }
     
@@ -476,6 +476,7 @@ public abstract class AbstractMapBackedStorageService extends AbstractStorageSer
         
         return dataMap.entrySet().removeIf(new Predicate<Entry<String, MutableStorageRecord<?>>>() {
                 public boolean test(@Nullable final Entry<String, MutableStorageRecord<?>> entry) {
+                    assert entry != null;
                     final Long exp = entry.getValue().getExpiration();
                     return exp != null && exp <= expiration;
                 }
diff --git a/opensaml-storage-api/src/main/java/org/opensaml/storage/AbstractStorageService.java b/opensaml-storage-api/src/main/java/org/opensaml/storage/AbstractStorageService.java
index 6a9e1ac69..ec22e916c 100644
--- a/opensaml-storage-api/src/main/java/org/opensaml/storage/AbstractStorageService.java
+++ b/opensaml-storage-api/src/main/java/org/opensaml/storage/AbstractStorageService.java
@@ -48,13 +48,13 @@ public abstract class AbstractStorageService extends AbstractIdentifiableInitial
     @Nonnull private Duration cleanupInterval;
 
     /** Timer used to schedule cleanup tasks. */
-    private Timer cleanupTaskTimer;
+    @Nullable private Timer cleanupTaskTimer;
 
     /** Timer used to schedule cleanup tasks if no external one set. */
-    private Timer internalTaskTimer;
+    @Nullable private Timer internalTaskTimer;
 
     /** Task that cleans up expired records. */
-    private TimerTask cleanupTask;
+    @Nullable private TimerTask cleanupTask;
 
     /** Configurable context size limit. */
     @Positive private int contextSize;
@@ -178,6 +178,7 @@ public abstract class AbstractStorageService extends AbstractIdentifiableInitial
             } else {
                 internalTaskTimer = cleanupTaskTimer;
             }
+            assert internalTaskTimer != null;
             internalTaskTimer.schedule(cleanupTask, cleanupInterval.toMillis(), cleanupInterval.toMillis());
         }
     }
@@ -188,6 +189,7 @@ public abstract class AbstractStorageService extends AbstractIdentifiableInitial
             cleanupTask.cancel();
             cleanupTask = null;
             if (cleanupTaskTimer == null) {
+                assert internalTaskTimer != null;
                 internalTaskTimer.cancel();
             }
             internalTaskTimer = null;
@@ -225,13 +227,26 @@ public abstract class AbstractStorageService extends AbstractIdentifiableInitial
 
     /** {@inheritDoc} */
     @Override public boolean create(@Nonnull final Object value) throws IOException {
-        return create(AnnotationSupport.getContext(value), AnnotationSupport.getKey(value),
-                AnnotationSupport.getValue(value), AnnotationSupport.getExpiration(value));
+        
+        final String context = AnnotationSupport.getContext(value);
+        final String key = AnnotationSupport.getKey(value);
+        final String val = AnnotationSupport.getValue(value);
+        if (context == null || key == null || val == null) {
+            throw new IOException("Context, key, and value must be non-null");
+        }
+        
+        return create(context, key, val, AnnotationSupport.getExpiration(value));
     }
 
     /** {@inheritDoc} */
     @Override @Nullable public Object read(@Nonnull final Object value) throws IOException {
-        final StorageRecord<?> record = read(AnnotationSupport.getContext(value), AnnotationSupport.getKey(value));
+        final String context = AnnotationSupport.getContext(value);
+        final String key = AnnotationSupport.getKey(value);
+        if (context == null || key == null) {
+            throw new IOException("Context and key must be non-null");
+        }
+        
+        final StorageRecord<?> record = read(context, key);
         if (record != null) {
             AnnotationSupport.setValue(value, record.getValue());
             AnnotationSupport.setExpiration(value, record.getExpiration());
@@ -262,32 +277,61 @@ public abstract class AbstractStorageService extends AbstractIdentifiableInitial
 
     /** {@inheritDoc} */
     @Override public boolean update(@Nonnull final Object value) throws IOException {
-        return update(AnnotationSupport.getContext(value), AnnotationSupport.getKey(value),
-                AnnotationSupport.getValue(value), AnnotationSupport.getExpiration(value));
+        final String context = AnnotationSupport.getContext(value);
+        final String key = AnnotationSupport.getKey(value);
+        final String val = AnnotationSupport.getValue(value);
+        if (context == null || key == null || val == null) {
+            throw new IOException("Context, key, and value must be non-null");
+        }
+        
+        return update(context, key, val, AnnotationSupport.getExpiration(value));
     }
 
     /** {@inheritDoc} */
     @Override @Nullable public Long updateWithVersion(@Positive final long version, @Nonnull final Object value)
             throws IOException, VersionMismatchException {
-        return updateWithVersion(version, AnnotationSupport.getContext(value), AnnotationSupport.getKey(value),
-                AnnotationSupport.getValue(value), AnnotationSupport.getExpiration(value));
+        final String context = AnnotationSupport.getContext(value);
+        final String key = AnnotationSupport.getKey(value);
+        final String val = AnnotationSupport.getValue(value);
+        if (context == null || key == null || val == null) {
+            throw new IOException("Context, key, and value must be non-null");
+        }
+        
+        return updateWithVersion(version, context, key, val, AnnotationSupport.getExpiration(value));
     }
 
     /** {@inheritDoc} */
     @Override public boolean updateExpiration(@Nonnull final Object value) throws IOException {
-        return updateExpiration(AnnotationSupport.getContext(value), AnnotationSupport.getKey(value),
-                AnnotationSupport.getExpiration(value));
+        final String context = AnnotationSupport.getContext(value);
+        final String key = AnnotationSupport.getKey(value);
+        if (context == null || key == null) {
+            throw new IOException("Context and key must be non-null");
+        }
+
+        return updateExpiration(context, key, AnnotationSupport.getExpiration(value));
     }
 
     /** {@inheritDoc} */
     @Override public boolean delete(@Nonnull final Object value) throws IOException {
-        return delete(AnnotationSupport.getContext(value), AnnotationSupport.getKey(value));
+        final String context = AnnotationSupport.getContext(value);
+        final String key = AnnotationSupport.getKey(value);
+        if (context == null || key == null) {
+            throw new IOException("Context and key must be non-null");
+        }
+
+        return delete(context, key);
     }
 
     /** {@inheritDoc} */
     @Override public boolean deleteWithVersion(@Positive final long version, @Nonnull final Object value)
             throws IOException, VersionMismatchException {
-        return deleteWithVersion(version, AnnotationSupport.getContext(value), AnnotationSupport.getKey(value));
+        final String context = AnnotationSupport.getContext(value);
+        final String key = AnnotationSupport.getKey(value);
+        if (context == null || key == null) {
+            throw new IOException("Context and key must be non-null");
+        }
+        
+        return deleteWithVersion(version, context, key);
     }
 
 }
\ No newline at end of file
diff --git a/opensaml-storage-api/src/main/java/org/opensaml/storage/StorageRecord.java b/opensaml-storage-api/src/main/java/org/opensaml/storage/StorageRecord.java
index e2e223895..fd5d743ca 100644
--- a/opensaml-storage-api/src/main/java/org/opensaml/storage/StorageRecord.java
+++ b/opensaml-storage-api/src/main/java/org/opensaml/storage/StorageRecord.java
@@ -38,10 +38,10 @@ public class StorageRecord<Type> {
     private long version;
     
     /** Value field. */
-    private String value;
+    @Nonnull @NotEmpty private String value;
     
     /** Expiration field. */
-    private Long expiration;
+    @Nullable private Long expiration;
     
     /**
      * Constructor.
diff --git a/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/AnnotationSupport.java b/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/AnnotationSupport.java
index daf7f8957..30f8d9659 100644
--- a/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/AnnotationSupport.java
+++ b/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/AnnotationSupport.java
@@ -54,13 +54,15 @@ public final class AnnotationSupport {
      * @throws IllegalArgumentException if the target object doesn't declare a {@link Context} annotation
      * @throws RuntimeException if the field cannot be read on the target object
      */
-    @Nonnull @NotEmpty public static String getContext(@Nonnull final Object target) {
+    @Nullable @NotEmpty public static String getContext(@Nonnull final Object target) {
         final Context ctxField = getAnnotation(target, Context.class);
         final Object value = getFieldValue(target, ctxField.value());
         if (value instanceof String) {
             return (String) value;
+        } else if (value != null) {
+            return value.toString();
         }
-        return value.toString();
+        return null;
     }
 
     /**
@@ -90,13 +92,15 @@ public final class AnnotationSupport {
      * @throws IllegalArgumentException if the target object doesn't declare a {@link Key} annotation
      * @throws RuntimeException if the field cannot be read on the target object
      */
-    @Nonnull @NotEmpty public static String getKey(@Nonnull final Object target) {
+    @Nullable @NotEmpty public static String getKey(@Nonnull final Object target) {
         final Key keyField = getAnnotation(target, Key.class);
         final Object value = getFieldValue(target, keyField.value());
         if (value instanceof String) {
             return (String) value;
+        } else if (value != null) {
+            return value.toString();
         }
-        return value.toString();
+        return null;
     }
 
     /**
@@ -126,13 +130,15 @@ public final class AnnotationSupport {
      * @throws IllegalArgumentException if the target object doesn't declare a {@link Value} annotation
      * @throws RuntimeException if the field cannot be read on the target object
      */
-    @Nonnull @NotEmpty public static String getValue(@Nonnull final Object target) {
+    @Nullable @NotEmpty public static String getValue(@Nonnull final Object target) {
         final Value valueField = getAnnotation(target, Value.class);
         final Object value = getFieldValue(target, valueField.value());
         if (value instanceof String) {
             return (String) value;
+        } else if (value != null) {
+            return value.toString();
         }
-        return value.toString();
+        return null;
     }
 
     /**
@@ -206,17 +212,17 @@ public final class AnnotationSupport {
         final Expiration expField = getAnnotation(target, Expiration.class);
         if (expiration == null) {
             setFieldValue(target, expField.value(), null);
-        }
-        
-        final Class<?> type = getField(target, expField.value()).getType();
-        if (Long.class.isAssignableFrom(type)) {
-            setFieldValue(target, expField.value(), expiration);
-        } else if (Date.class.isAssignableFrom(type)) {
-            setFieldValue(target, expField.value(), new Date(expiration));
-        } else if (Instant.class.isAssignableFrom(type)) {
-            setFieldValue(target, expField.value(), Instant.ofEpochMilli(expiration));
         } else {
-            throw new RuntimeException(type + " is an unsupported data type for an expiration field.");
+            final Class<?> type = getField(target, expField.value()).getType();
+            if (Long.class.isAssignableFrom(type)) {
+                setFieldValue(target, expField.value(), expiration);
+            } else if (Date.class.isAssignableFrom(type)) {
+                setFieldValue(target, expField.value(), new Date(expiration));
+            } else if (Instant.class.isAssignableFrom(type)) {
+                setFieldValue(target, expField.value(), Instant.ofEpochMilli(expiration));
+            } else {
+                throw new RuntimeException(type + " is an unsupported data type for an expiration field.");
+            }
         }
     }
 
@@ -235,7 +241,7 @@ public final class AnnotationSupport {
         final Class<?> targetClass = target.getClass();
         final T keyField = targetClass.getAnnotation(annotationType);
         if (keyField == null) {
-            throw new IllegalArgumentException("Key annotation not found on " + target);
+            throw new IllegalArgumentException("Annotation not found on " + target);
         }
         return keyField;
     }
diff --git a/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Context.java b/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Context.java
index 7f46460a2..18e621ebb 100644
--- a/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Context.java
+++ b/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Context.java
@@ -22,6 +22,8 @@ import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
+import javax.annotation.Nonnull;
+
 /**
  * Identifies the field of a class that serves as the context of a {@link org.opensaml.storage.StorageService} record.
  */
@@ -34,5 +36,5 @@ public @interface Context {
      * 
      * @return value of this annotation
      */
-    public String value();
+    @Nonnull public String value();
 }
\ No newline at end of file
diff --git a/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Expiration.java b/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Expiration.java
index 14d553987..016d8315a 100644
--- a/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Expiration.java
+++ b/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Expiration.java
@@ -22,6 +22,8 @@ import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
+import javax.annotation.Nonnull;
+
 /**
  * Identifies the field of a class that serves as the expiration of a
  * {@link org.opensaml.storage.StorageService} record.
@@ -35,5 +37,5 @@ public @interface Expiration {
      * 
      * @return value of this annotation
      */
-    public String value();
+    @Nonnull public String value();
 }
\ No newline at end of file
diff --git a/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Key.java b/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Key.java
index 10b2e1ffe..65436c39d 100644
--- a/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Key.java
+++ b/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Key.java
@@ -22,6 +22,8 @@ import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
+import javax.annotation.Nonnull;
+
 /**
  * Identifies the field of a class that serves as the key of a
  * {@link org.opensaml.storage.StorageService} record.
@@ -35,5 +37,5 @@ public @interface Key {
      * 
      * @return value of this annotation
      */
-    public String value();
+    @Nonnull public String value();
 }
\ No newline at end of file
diff --git a/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Value.java b/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Value.java
index 809c6803e..6f8ca28a6 100644
--- a/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Value.java
+++ b/opensaml-storage-api/src/main/java/org/opensaml/storage/annotation/Value.java
@@ -22,6 +22,8 @@ import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
+import javax.annotation.Nonnull;
+
 /**
  * Identifies the field of a class that serves as the value of a
  * {@link org.opensaml.storage.StorageService} record.
@@ -35,5 +37,5 @@ public @interface Value {
      * 
      * @return value of this annotation
      */
-    public String value();
+    @Nonnull public String value();
 }
\ No newline at end of file

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


More information about the commits mailing list