From 2c2c53f68056885f826b65232fbc31a5e92d83ad Mon Sep 17 00:00:00 2001 From: andres Date: Tue, 16 Jun 2026 14:04:35 +0200 Subject: [PATCH 1/6] Handling reaload case --- .../impl/CachingConfigServiceDecorator.java | 46 ++++++++++- .../CachingConfigServiceDecoratorTest.java | 82 +++++++++++++++++++ 2 files changed, 124 insertions(+), 4 deletions(-) diff --git a/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java b/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java index a37139193a..8a54adf72f 100644 --- a/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java +++ b/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java @@ -9,21 +9,39 @@ import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Ticker; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; import co.worklytics.psoxy.gateway.ConfigService; import co.worklytics.psoxy.gateway.SecretStore; import co.worklytics.psoxy.gateway.WritableConfigService; -import lombok.RequiredArgsConstructor; import lombok.SneakyThrows; +import lombok.extern.java.Log; +import org.jspecify.annotations.NonNull; +import java.util.logging.Level; -@RequiredArgsConstructor + +@Log public class CachingConfigServiceDecorator implements WritableConfigService, SecretStore { final ConfigService delegate; final Duration defaultTtl; + final Ticker ticker; + + public CachingConfigServiceDecorator(ConfigService delegate, Duration defaultTtl) { + this(delegate, defaultTtl, Ticker.systemTicker()); + } + + @VisibleForTesting + CachingConfigServiceDecorator(ConfigService delegate, Duration defaultTtl, Ticker ticker) { + this.delegate = delegate; + this.defaultTtl = defaultTtl; + this.ticker = ticker; + } private volatile LoadingCache cache; @@ -39,13 +57,33 @@ LoadingCache getCache() { if (this.cache == null) { this.cache = CacheBuilder.newBuilder() .maximumSize(100) - .expireAfterWrite(defaultTtl.getSeconds(), TimeUnit.SECONDS) + .ticker(ticker) + .refreshAfterWrite(defaultTtl.getSeconds(), TimeUnit.SECONDS) .recordStats() .build(new CacheLoader() { //req for java8-backwards compatibility @Override - public String load(ConfigProperty key) { + public String load(@NonNull ConfigProperty key) { return delegate.getConfigPropertyAsOptional(key).orElse(NEGATIVE_VALUE); } + + @Override + public ListenableFuture reload(@NonNull ConfigProperty key, @NonNull String oldValue) { + String newValue = delegate.getConfigPropertyAsOptional(key).orElse(NEGATIVE_VALUE); + // If the store returns empty but we previously had a valid value, + // treat it as a transient backend failure and retain the old value. + // Crucially, we return immediateFuture(oldValue) rather than a failed + // future: a failed future leaves the write-time unchanged so Guava + // would re-attempt the reload on every subsequent cache.get() call, + // hammering SSM for the entire outage window. Returning the old value + // marks the entry as freshly written, so the next retry waits a full TTL. + if (NEGATIVE_VALUE.equals(newValue) && !NEGATIVE_VALUE.equals(oldValue)) { + log.log(Level.WARNING, + "Transient failure reloading config property {0}; retaining cached value until next refresh cycle", + key.name()); + return Futures.immediateFuture(oldValue); + } + return Futures.immediateFuture(newValue); + } }); } } diff --git a/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecoratorTest.java b/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecoratorTest.java index 3f8c00e075..321e05edca 100644 --- a/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecoratorTest.java +++ b/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecoratorTest.java @@ -13,6 +13,9 @@ import java.util.Map; import java.util.NoSuchElementException; import java.util.Optional; +import java.util.concurrent.TimeUnit; + +import com.google.common.base.Ticker; import static org.junit.jupiter.api.Assertions.*; @@ -72,6 +75,39 @@ void putConfigProperty() { localHashMapConfigService.getConfigPropertyOrError(TestConfigProperties.EXAMPLE_PROPERTY)); } + @Test + void retainsStaleValueOnTransientReloadFailure() { + FakeTicker ticker = new FakeTicker(); + ToggleableConfigService delegate = new ToggleableConfigService(); + delegate.putConfigProperty(TestConfigProperties.EXAMPLE_PROPERTY, "valid_token"); + + CachingConfigServiceDecorator cache = + new CachingConfigServiceDecorator(delegate, Duration.ofMinutes(1), ticker); + + // initial load succeeds + assertEquals(Optional.of("valid_token"), + cache.getConfigPropertyAsOptional(TestConfigProperties.EXAMPLE_PROPERTY)); + assertEquals(1, delegate.getReads()); + + // simulate transient SSM failure and advance past TTL + delegate.setSimulateFailure(true); + ticker.advance(2, TimeUnit.MINUTES); + + // reload fails silently; old value is retained — caller sees no error + assertEquals(Optional.of("valid_token"), + cache.getConfigPropertyAsOptional(TestConfigProperties.EXAMPLE_PROPERTY)); + assertEquals(2, delegate.getReads()); // reload was attempted + + // SSM recovers; advance past TTL again + delegate.setSimulateFailure(false); + ticker.advance(2, TimeUnit.MINUTES); + + // next reload succeeds; value still valid + assertEquals(Optional.of("valid_token"), + cache.getConfigPropertyAsOptional(TestConfigProperties.EXAMPLE_PROPERTY)); + assertEquals(3, delegate.getReads()); + } + @Test void getConfigProperty_noCache() { assertTrue(config.getConfigPropertyAsOptional(TestConfigProperties.NO_CACHE).isEmpty()); @@ -91,6 +127,52 @@ void getConfigProperty_noCache() { } + static class FakeTicker extends Ticker { + private long nanos = 0; + + @Override + public long read() { + return nanos; + } + + void advance(long amount, TimeUnit unit) { + nanos += unit.toNanos(amount); + } + } + + static class ToggleableConfigService implements WritableConfigService { + private final Map map = new HashMap<>(); + + @Getter + private int reads = 0; + + private boolean simulateFailure = false; + + void setSimulateFailure(boolean simulateFailure) { + this.simulateFailure = simulateFailure; + } + + @Override + public void putConfigProperty(ConfigProperty property, String value) { + map.put(property, value); + } + + @Override + public String getConfigPropertyOrError(ConfigProperty property) { + return getConfigPropertyAsOptional(property) + .orElseThrow(() -> new NoSuchElementException("no value for " + property)); + } + + @Override + public Optional getConfigPropertyAsOptional(ConfigProperty property) { + reads++; + if (simulateFailure) { + return Optional.empty(); + } + return Optional.ofNullable(map.get(property)); + } + } + static class LocalHashMapConfigService implements WritableConfigService { Map map = new HashMap<>(); From 54d4e6d29d186effa41616f9903c08a6a50031b7 Mon Sep 17 00:00:00 2001 From: andres Date: Tue, 16 Jun 2026 14:36:41 +0200 Subject: [PATCH 2/6] Adding transient cases --- .../gateway/TransientConfigException.java | 19 ++++ .../impl/CachingConfigServiceDecorator.java | 45 ++++++--- .../CachingConfigServiceDecoratorTest.java | 94 +++++++++++++++++++ .../aws/ParameterStoreConfigService.java | 17 +++- .../psoxy/aws/SecretsManagerSecretStore.java | 23 +++-- 5 files changed, 173 insertions(+), 25 deletions(-) create mode 100644 java/core/src/main/java/co/worklytics/psoxy/gateway/TransientConfigException.java diff --git a/java/core/src/main/java/co/worklytics/psoxy/gateway/TransientConfigException.java b/java/core/src/main/java/co/worklytics/psoxy/gateway/TransientConfigException.java new file mode 100644 index 0000000000..a6bc3eab5d --- /dev/null +++ b/java/core/src/main/java/co/worklytics/psoxy/gateway/TransientConfigException.java @@ -0,0 +1,19 @@ +package co.worklytics.psoxy.gateway; + +/** + * Signals that a config/secret backend had a transient failure (credential rotation, network + * blip, service hiccup) and the value may still be accessible on the next attempt. + * + * Distinct from a missing value ({@code Optional.empty()} / {@code NEGATIVE_VALUE}): callers + * should NOT treat this as "property not configured" — they should retry or serve a cached value. + */ +public class TransientConfigException extends RuntimeException { + + public TransientConfigException(String message, Throwable cause) { + super(message, cause); + } + + public TransientConfigException(String message) { + super(message); + } +} diff --git a/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java b/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java index 8a54adf72f..9cb5e187cd 100644 --- a/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java +++ b/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java @@ -15,8 +15,10 @@ import com.google.common.cache.LoadingCache; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.UncheckedExecutionException; import co.worklytics.psoxy.gateway.ConfigService; import co.worklytics.psoxy.gateway.SecretStore; +import co.worklytics.psoxy.gateway.TransientConfigException; import co.worklytics.psoxy.gateway.WritableConfigService; import lombok.SneakyThrows; import lombok.extern.java.Log; @@ -68,21 +70,27 @@ public String load(@NonNull ConfigProperty key) { @Override public ListenableFuture reload(@NonNull ConfigProperty key, @NonNull String oldValue) { - String newValue = delegate.getConfigPropertyAsOptional(key).orElse(NEGATIVE_VALUE); - // If the store returns empty but we previously had a valid value, - // treat it as a transient backend failure and retain the old value. - // Crucially, we return immediateFuture(oldValue) rather than a failed - // future: a failed future leaves the write-time unchanged so Guava - // would re-attempt the reload on every subsequent cache.get() call, - // hammering SSM for the entire outage window. Returning the old value - // marks the entry as freshly written, so the next retry waits a full TTL. - if (NEGATIVE_VALUE.equals(newValue) && !NEGATIVE_VALUE.equals(oldValue)) { + try { + String newValue = delegate.getConfigPropertyAsOptional(key).orElse(NEGATIVE_VALUE); + // Fallback heuristic for backends that still swallow exceptions + // (e.g. GCP SecretManagerConfigService): if the value was valid + // before but now comes back empty, assume transient and retain. + if (NEGATIVE_VALUE.equals(newValue) && !NEGATIVE_VALUE.equals(oldValue)) { + log.log(Level.WARNING, + "Backend returned empty for config property {0} which was previously set; assuming transient failure and retaining cached value", + key.name()); + return Futures.immediateFuture(oldValue); + } + return Futures.immediateFuture(newValue); + } catch (TransientConfigException e) { + // Backend explicitly signalled a transient failure. + // Returning the old value resets the write-time so Guava waits a + // full TTL before retrying, rather than retrying on every request. log.log(Level.WARNING, "Transient failure reloading config property {0}; retaining cached value until next refresh cycle", key.name()); return Futures.immediateFuture(oldValue); } - return Futures.immediateFuture(newValue); } }); } @@ -123,8 +131,23 @@ public Optional getConfigPropertyAsOptional(ConfigProperty property) { } else { return Optional.of(value); } + } catch (UncheckedExecutionException e) { + // Guava wraps RuntimeExceptions from load() in UncheckedExecutionException. + // TransientConfigException is a RuntimeException, so it lands here. + Throwable cause = e.getCause(); + if (cause instanceof TransientConfigException) { + // load() threw a TransientConfigException — the cache stored nothing, so the + // next request will retry the backend immediately. Return empty so that + // optional-property callers degrade gracefully; required-property callers will + // reach getConfigPropertyOrError() which throws NoSuchElementException. + log.log(Level.WARNING, + "Transient backend failure for config property {0}; value not cached, next request will retry", + property.name()); + return Optional.empty(); + } + throw (cause instanceof RuntimeException) ? (RuntimeException) cause : e; } catch (ExecutionException e) { - //unwrap if possible, re-throw + // Guava wraps checked exceptions from load() in ExecutionException. if (e.getCause() == null) { throw e; } else { diff --git a/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecoratorTest.java b/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecoratorTest.java index 321e05edca..aade9f1db1 100644 --- a/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecoratorTest.java +++ b/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecoratorTest.java @@ -16,6 +16,7 @@ import java.util.concurrent.TimeUnit; import com.google.common.base.Ticker; +import co.worklytics.psoxy.gateway.TransientConfigException; import static org.junit.jupiter.api.Assertions.*; @@ -108,6 +109,63 @@ void retainsStaleValueOnTransientReloadFailure() { assertEquals(3, delegate.getReads()); } + @Test + void retainsStaleValueOnExplicitTransientException() { + FakeTicker ticker = new FakeTicker(); + ThrowingConfigService delegate = new ThrowingConfigService("valid_token"); + + CachingConfigServiceDecorator cache = + new CachingConfigServiceDecorator(delegate, Duration.ofMinutes(1), ticker); + + // initial load succeeds + assertEquals(Optional.of("valid_token"), + cache.getConfigPropertyAsOptional(TestConfigProperties.EXAMPLE_PROPERTY)); + assertEquals(1, delegate.getReads()); + + // backend starts throwing TransientConfigException + delegate.setThrowTransient(true); + ticker.advance(2, TimeUnit.MINUTES); + + // reload catches TransientConfigException; old value retained, no error to caller + assertEquals(Optional.of("valid_token"), + cache.getConfigPropertyAsOptional(TestConfigProperties.EXAMPLE_PROPERTY)); + assertEquals(2, delegate.getReads()); + + // backend recovers + delegate.setThrowTransient(false); + ticker.advance(2, TimeUnit.MINUTES); + + assertEquals(Optional.of("valid_token"), + cache.getConfigPropertyAsOptional(TestConfigProperties.EXAMPLE_PROPERTY)); + assertEquals(3, delegate.getReads()); + } + + @Test + void transientExceptionOnColdStartDoesNotCacheNegativeValue() { + FakeTicker ticker = new FakeTicker(); + ThrowingConfigService delegate = new ThrowingConfigService("valid_token"); + delegate.setThrowTransient(true); + + CachingConfigServiceDecorator cache = + new CachingConfigServiceDecorator(delegate, Duration.ofMinutes(1), ticker); + + // cold start with transient error: returns empty but does NOT cache NEGATIVE_VALUE + assertEquals(Optional.empty(), + cache.getConfigPropertyAsOptional(TestConfigProperties.EXAMPLE_PROPERTY)); + assertEquals(1, delegate.getReads()); + + // next request retries immediately (nothing cached) — still failing + assertEquals(Optional.empty(), + cache.getConfigPropertyAsOptional(TestConfigProperties.EXAMPLE_PROPERTY)); + assertEquals(2, delegate.getReads()); // retried, not served from cache + + // backend recovers (no TTL advance needed — nothing was cached) + delegate.setThrowTransient(false); + assertEquals(Optional.of("valid_token"), + cache.getConfigPropertyAsOptional(TestConfigProperties.EXAMPLE_PROPERTY)); + assertEquals(3, delegate.getReads()); + } + @Test void getConfigProperty_noCache() { assertTrue(config.getConfigPropertyAsOptional(TestConfigProperties.NO_CACHE).isEmpty()); @@ -140,6 +198,42 @@ void advance(long amount, TimeUnit unit) { } } + static class ThrowingConfigService implements WritableConfigService { + private String value; + private boolean throwTransient = false; + + @Getter + private int reads = 0; + + ThrowingConfigService(String value) { + this.value = value; + } + + void setThrowTransient(boolean throwTransient) { + this.throwTransient = throwTransient; + } + + @Override + public void putConfigProperty(ConfigProperty property, String newValue) { + this.value = newValue; + } + + @Override + public String getConfigPropertyOrError(ConfigProperty property) { + return getConfigPropertyAsOptional(property) + .orElseThrow(() -> new NoSuchElementException("no value for " + property)); + } + + @Override + public Optional getConfigPropertyAsOptional(ConfigProperty property) { + reads++; + if (throwTransient) { + throw new TransientConfigException("simulated transient failure"); + } + return Optional.ofNullable(value); + } + } + static class ToggleableConfigService implements WritableConfigService { private final Map map = new HashMap<>(); diff --git a/java/impl/aws/src/main/java/co/worklytics/psoxy/aws/ParameterStoreConfigService.java b/java/impl/aws/src/main/java/co/worklytics/psoxy/aws/ParameterStoreConfigService.java index f6ce84d7e9..eef806fb9f 100644 --- a/java/impl/aws/src/main/java/co/worklytics/psoxy/aws/ParameterStoreConfigService.java +++ b/java/impl/aws/src/main/java/co/worklytics/psoxy/aws/ParameterStoreConfigService.java @@ -3,6 +3,7 @@ import co.worklytics.psoxy.gateway.ConfigService; import co.worklytics.psoxy.gateway.LockService; import co.worklytics.psoxy.gateway.SecretStore; +import co.worklytics.psoxy.gateway.TransientConfigException; import co.worklytics.psoxy.gateway.impl.EnvVarsConfigService; import co.worklytics.psoxy.utils.DevLogUtils; import co.worklytics.psoxy.utils.RandomNumberGenerator; @@ -137,11 +138,17 @@ Optional getConfigPropertyAsOptional(ConfigProperty property, Function Optional getConfigPropertyAsOptional(ConfigProperty property, Function Optional getConfigPropertyAsOptional(ConfigProperty property, Function Date: Tue, 16 Jun 2026 18:29:07 +0200 Subject: [PATCH 3/6] More transient stuff --- .../gateway/impl/ApiDataRequestHandler.java | 8 ++++ .../impl/CachingConfigServiceDecorator.java | 45 +++++++++++++++---- .../CachingConfigServiceDecoratorTest.java | 24 +++++----- 3 files changed, 58 insertions(+), 19 deletions(-) diff --git a/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/ApiDataRequestHandler.java b/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/ApiDataRequestHandler.java index c1b638d294..8c25c44855 100644 --- a/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/ApiDataRequestHandler.java +++ b/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/ApiDataRequestHandler.java @@ -405,6 +405,14 @@ public HttpEventResponse handle(HttpEventRequest requestToProxy, log.log(Level.WARNING, "Confirm oauth scopes set in config.yaml match those granted in data source"); return builder.build(); + } catch (co.worklytics.psoxy.gateway.TransientConfigException e) { + // Config store was temporarily unreachable (e.g. credential rotation, AWS hiccup). + // The proxy already retried internally; this is not a misconfiguration. + builder.statusCode(HttpStatus.SC_SERVICE_UNAVAILABLE); + builder.header(ProcessedDataMetadataFields.ERROR.getHttpHeader(), + ErrorCauses.CONFIGURATION_FAILURE.name()); + log.log(Level.WARNING, "Transient config store failure after retries: " + e.getMessage(), e); + return builder.build(); } catch (java.util.NoSuchElementException e) { // missing config, such as ACCESS_TOKEN builder.statusCode(HttpStatus.SC_INTERNAL_SERVER_ERROR); diff --git a/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java b/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java index 9cb5e187cd..12709d8175 100644 --- a/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java +++ b/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java @@ -30,19 +30,29 @@ @Log public class CachingConfigServiceDecorator implements WritableConfigService, SecretStore { + static final int MAX_TRANSIENT_RETRIES = 3; + static final long DEFAULT_TRANSIENT_RETRY_DELAY_MS = 500L; + final ConfigService delegate; final Duration defaultTtl; final Ticker ticker; + final long transientRetryDelayMs; public CachingConfigServiceDecorator(ConfigService delegate, Duration defaultTtl) { - this(delegate, defaultTtl, Ticker.systemTicker()); + this(delegate, defaultTtl, Ticker.systemTicker(), DEFAULT_TRANSIENT_RETRY_DELAY_MS); } @VisibleForTesting CachingConfigServiceDecorator(ConfigService delegate, Duration defaultTtl, Ticker ticker) { + this(delegate, defaultTtl, ticker, DEFAULT_TRANSIENT_RETRY_DELAY_MS); + } + + @VisibleForTesting + CachingConfigServiceDecorator(ConfigService delegate, Duration defaultTtl, Ticker ticker, long transientRetryDelayMs) { this.delegate = delegate; this.defaultTtl = defaultTtl; this.ticker = ticker; + this.transientRetryDelayMs = transientRetryDelayMs; } private volatile LoadingCache cache; @@ -65,7 +75,27 @@ LoadingCache getCache() { .build(new CacheLoader() { //req for java8-backwards compatibility @Override public String load(@NonNull ConfigProperty key) { - return delegate.getConfigPropertyAsOptional(key).orElse(NEGATIVE_VALUE); + TransientConfigException lastException = null; + for (int attempt = 0; attempt < MAX_TRANSIENT_RETRIES; attempt++) { + if (attempt > 0) { + try { + if (transientRetryDelayMs > 0) Thread.sleep(transientRetryDelayMs); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new TransientConfigException("Config load for " + key.name() + " interrupted during retry", ie); + } + log.log(Level.WARNING, "Retrying config property {0}, attempt {1}/{2}", + new Object[]{key.name(), attempt + 1, MAX_TRANSIENT_RETRIES}); + } + try { + return delegate.getConfigPropertyAsOptional(key).orElse(NEGATIVE_VALUE); + } catch (TransientConfigException e) { + lastException = e; + log.log(Level.WARNING, "Transient failure on attempt {0}/{1} for config property {2}", + new Object[]{attempt + 1, MAX_TRANSIENT_RETRIES, key.name()}); + } + } + throw Objects.requireNonNull(lastException); } @Override @@ -136,14 +166,13 @@ public Optional getConfigPropertyAsOptional(ConfigProperty property) { // TransientConfigException is a RuntimeException, so it lands here. Throwable cause = e.getCause(); if (cause instanceof TransientConfigException) { - // load() threw a TransientConfigException — the cache stored nothing, so the - // next request will retry the backend immediately. Return empty so that - // optional-property callers degrade gracefully; required-property callers will - // reach getConfigPropertyOrError() which throws NoSuchElementException. + // load() retried MAX_TRANSIENT_RETRIES times and still failed. Nothing was + // cached, so the next request will retry immediately. Re-throw so callers can + // distinguish a transient store outage from a genuinely missing property. log.log(Level.WARNING, - "Transient backend failure for config property {0}; value not cached, next request will retry", + "Transient backend failure for config property {0}; all retries exhausted", property.name()); - return Optional.empty(); + throw (TransientConfigException) cause; } throw (cause instanceof RuntimeException) ? (RuntimeException) cause : e; } catch (ExecutionException e) { diff --git a/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecoratorTest.java b/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecoratorTest.java index aade9f1db1..705afcb8c8 100644 --- a/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecoratorTest.java +++ b/java/core/src/test/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecoratorTest.java @@ -146,24 +146,26 @@ void transientExceptionOnColdStartDoesNotCacheNegativeValue() { ThrowingConfigService delegate = new ThrowingConfigService("valid_token"); delegate.setThrowTransient(true); + // 0ms delay so the retry loop is fast in tests CachingConfigServiceDecorator cache = - new CachingConfigServiceDecorator(delegate, Duration.ofMinutes(1), ticker); + new CachingConfigServiceDecorator(delegate, Duration.ofMinutes(1), ticker, 0L); - // cold start with transient error: returns empty but does NOT cache NEGATIVE_VALUE - assertEquals(Optional.empty(), - cache.getConfigPropertyAsOptional(TestConfigProperties.EXAMPLE_PROPERTY)); - assertEquals(1, delegate.getReads()); + // cold start with transient error: retries MAX_TRANSIENT_RETRIES times then throws — + // nothing is cached as NEGATIVE_VALUE, so the next request will retry immediately + assertThrows(TransientConfigException.class, + () -> cache.getConfigPropertyAsOptional(TestConfigProperties.EXAMPLE_PROPERTY)); + assertEquals(CachingConfigServiceDecorator.MAX_TRANSIENT_RETRIES, delegate.getReads()); - // next request retries immediately (nothing cached) — still failing - assertEquals(Optional.empty(), - cache.getConfigPropertyAsOptional(TestConfigProperties.EXAMPLE_PROPERTY)); - assertEquals(2, delegate.getReads()); // retried, not served from cache + // second request: still failing, retries again from scratch (nothing was cached) + assertThrows(TransientConfigException.class, + () -> cache.getConfigPropertyAsOptional(TestConfigProperties.EXAMPLE_PROPERTY)); + assertEquals(CachingConfigServiceDecorator.MAX_TRANSIENT_RETRIES * 2, delegate.getReads()); - // backend recovers (no TTL advance needed — nothing was cached) + // backend recovers — no TTL advance needed since nothing was ever cached delegate.setThrowTransient(false); assertEquals(Optional.of("valid_token"), cache.getConfigPropertyAsOptional(TestConfigProperties.EXAMPLE_PROPERTY)); - assertEquals(3, delegate.getReads()); + assertEquals(CachingConfigServiceDecorator.MAX_TRANSIENT_RETRIES * 2 + 1, delegate.getReads()); } @Test From 5bf440e73f7f53ee94daeda5930a0283600b3b0b Mon Sep 17 00:00:00 2001 From: andres Date: Tue, 16 Jun 2026 20:06:25 +0200 Subject: [PATCH 4/6] Add missing AwsExceptionUtils (fixes CI compilation failure) Co-Authored-By: Claude Sonnet 4.6 --- .../co/worklytics/psoxy/aws/AwsExceptionUtils.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 java/impl/aws/src/main/java/co/worklytics/psoxy/aws/AwsExceptionUtils.java diff --git a/java/impl/aws/src/main/java/co/worklytics/psoxy/aws/AwsExceptionUtils.java b/java/impl/aws/src/main/java/co/worklytics/psoxy/aws/AwsExceptionUtils.java new file mode 100644 index 0000000000..a23e259934 --- /dev/null +++ b/java/impl/aws/src/main/java/co/worklytics/psoxy/aws/AwsExceptionUtils.java @@ -0,0 +1,14 @@ +package co.worklytics.psoxy.aws; + +import software.amazon.awssdk.awscore.exception.AwsServiceException; + +class AwsExceptionUtils { + + static boolean isAccessDenied(AwsServiceException e) { + if (e.awsErrorDetails() == null) { + return false; + } + String code = e.awsErrorDetails().errorCode(); + return code != null && (code.contains("AccessDenied") || code.contains("Forbidden")); + } +} From 42f2988c7b4b6ad4f4fafc09a292350feb1dad70 Mon Sep 17 00:00:00 2001 From: andres Date: Wed, 17 Jun 2026 13:02:52 +0200 Subject: [PATCH 5/6] Using right NonNull --- .../psoxy/gateway/impl/CachingConfigServiceDecorator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java b/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java index 12709d8175..66abb81786 100644 --- a/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java +++ b/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java @@ -20,9 +20,9 @@ import co.worklytics.psoxy.gateway.SecretStore; import co.worklytics.psoxy.gateway.TransientConfigException; import co.worklytics.psoxy.gateway.WritableConfigService; +import lombok.NonNull; import lombok.SneakyThrows; import lombok.extern.java.Log; -import org.jspecify.annotations.NonNull; import java.util.logging.Level; From 8a0e617f2cd9ea30de50f1f8ce8e8829225f4e49 Mon Sep 17 00:00:00 2001 From: andres Date: Wed, 17 Jun 2026 13:27:50 +0200 Subject: [PATCH 6/6] Feedback --- .../impl/CachingConfigServiceDecorator.java | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java b/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java index 66abb81786..961a9c84be 100644 --- a/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java +++ b/java/core/src/main/java/co/worklytics/psoxy/gateway/impl/CachingConfigServiceDecorator.java @@ -8,6 +8,7 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Ticker; import com.google.common.cache.CacheBuilder; @@ -77,25 +78,22 @@ LoadingCache getCache() { public String load(@NonNull ConfigProperty key) { TransientConfigException lastException = null; for (int attempt = 0; attempt < MAX_TRANSIENT_RETRIES; attempt++) { - if (attempt > 0) { - try { - if (transientRetryDelayMs > 0) Thread.sleep(transientRetryDelayMs); - } catch (InterruptedException ie) { - Thread.currentThread().interrupt(); - throw new TransientConfigException("Config load for " + key.name() + " interrupted during retry", ie); - } - log.log(Level.WARNING, "Retrying config property {0}, attempt {1}/{2}", - new Object[]{key.name(), attempt + 1, MAX_TRANSIENT_RETRIES}); - } try { return delegate.getConfigPropertyAsOptional(key).orElse(NEGATIVE_VALUE); } catch (TransientConfigException e) { lastException = e; - log.log(Level.WARNING, "Transient failure on attempt {0}/{1} for config property {2}", - new Object[]{attempt + 1, MAX_TRANSIENT_RETRIES, key.name()}); + log.log(Level.WARNING, String.format("Transient failure on attempt {0}/{1} for config property {2}", + attempt + 1, MAX_TRANSIENT_RETRIES, key.name())); + } + try { + if (transientRetryDelayMs > 0) + Thread.sleep(transientRetryDelayMs); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new TransientConfigException("Config load for " + key.name() + " interrupted during retry", ie); } } - throw Objects.requireNonNull(lastException); + throw lastException; } @Override