[java-identity-provider] 02/03: IDP-2378 Accessing metrics/updates always triggers four HTTPS requests

[Rod Widdowson]

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

rdw pushed a commit to branch main
in repository java-identity-provider.

View the commit online:
https://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=45f804fd26e22e2daca0564dddbcc6728c1e7078

commit 45f804fd26e22e2daca0564dddbcc6728c1e7078
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Sun Nov 9 13:50:00 2025 +0000

    IDP-2378 Accessing metrics/updates always triggers four HTTPS requests
    
    https://shibboleth.atlassian.net/browse/IDP-2378
    
    Add readwrite locking around cache.  Add tracing (because I cannot write time comparison)
---
 .../impl/InstallableComponentPropertyCache.java    | 61 ++++++++++++++++------
 1 file changed, 46 insertions(+), 15 deletions(-)

diff --git a/idp-admin-impl/src/main/java/net/shibboleth/idp/admin/impl/InstallableComponentPropertyCache.java b/idp-admin-impl/src/main/java/net/shibboleth/idp/admin/impl/InstallableComponentPropertyCache.java
index 158cefed2..1fca76c3e 100644
--- a/idp-admin-impl/src/main/java/net/shibboleth/idp/admin/impl/InstallableComponentPropertyCache.java
+++ b/idp-admin-impl/src/main/java/net/shibboleth/idp/admin/impl/InstallableComponentPropertyCache.java
@@ -21,6 +21,9 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -53,6 +56,8 @@ public class InstallableComponentPropertyCache extends AbstractIdentifiableIniti
      */
     @NonnullAfterInit private Duration cacheLife;
 
+    @Nonnull private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock(true);
+
     /** Set {@link #cacheLife}.
      * @param howLong what to set
      */
@@ -64,7 +69,9 @@ public class InstallableComponentPropertyCache extends AbstractIdentifiableIniti
 
     @Override
     protected void doInitialize() throws ComponentInitializationException {
-        Constraint.isNotNull(cacheLife, "Duration cache life was not set");
+        if (cacheLife == null) {
+            throw new ComponentInitializationException("Cache life was not specified was null");
+        }
     }
 
     /** Provide a shim on top of {@link InstallableComponentSupport#loadInfo(List, HttpClient, HttpClientSecurityParameters)}.
@@ -78,27 +85,51 @@ public class InstallableComponentPropertyCache extends AbstractIdentifiableIniti
                  @Nonnull final HttpClient client,
                  @Nullable final HttpClientSecurityParameters securityParameters) {
 
-        log.trace("Looking for {}", updateURLs);
+        log.trace("Looking for '{}'", updateURLs);
         if (updateURLs.isEmpty() || cacheLife.isZero()) {
             log.trace("No Key or zero cachelife");
             return InstallableComponentSupport.loadInfo(updateURLs, client, securityParameters);
         }
 
-        final Instant now = Instant.now();
-        final Pair<Instant, Properties> cachedValue = cache.get(updateURLs.get(0));
-
-        if (cachedValue != null) {
-            log.trace("Cache hit, expired {} (now {})", cachedValue.getFirst(), now);
-            // if it expires after now
-            if (cachedValue.getFirst().isAfter(now)) {
-                return cachedValue.getSecond();
+        final Lock readLock = readWriteLock.readLock();
+        try {
+            readLock.lock();
+
+            final Instant now = Instant.now();
+            Pair<Instant, Properties> cachedValue = cache.get(updateURLs.get(0));
+            if (cachedValue != null) {
+                log.trace("Cache hit: [expires: {} now: {}]", cachedValue.getFirst(), now);
+                // if it expires after now
+                if (cachedValue.getFirst().isAfter(now)) {
+                    return cachedValue.getSecond();
+                }
             }
+        } finally {
+            readLock.unlock();
         }
 
-        final Properties result = InstallableComponentSupport.loadInfo(updateURLs, client, securityParameters);
-        final Instant expires = now.plus(cacheLife);
-        log.trace("Caching , expires {}", expires);
-        cache.put(updateURLs.get(0), new Pair<>(expires, result));
-        return result;
+        log.trace("Cached entry not found or was out of date.");
+        final Lock writeLock = readWriteLock.readLock();
+        try {
+            writeLock.lock();
+            final Pair<Instant, Properties> cachedValue = cache.get(updateURLs.get(0));
+            final Instant now = Instant.now();
+            // lookup again time may have moved
+            if (cachedValue != null) {
+                log.trace("Cache hit: [expires: {} now: {}]", cachedValue.getFirst(), now);
+                // if it expires after now
+                if (cachedValue.getFirst().isAfter(now)) {
+                    return cachedValue.getSecond();
+                }
+            }
+            // ask the URL(s)
+            final Properties result = InstallableComponentSupport.loadInfo(updateURLs, client, securityParameters);
+            final Instant expires = now.plus(cacheLife);
+            log.trace("Caching entry with{} data [expires: {}]", result==null?"out":"", expires);
+            cache.put(updateURLs.get(0), new Pair<>(expires, result));
+            return result;
+        } finally {
+            writeLock.unlock();
+        }
     }
 }

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