This is an automated email from the ASF dual-hosted git repository. jamesbognar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/juneau.git
commit 50c0b9356d3274007a7c621f5ba2e1fb1d0380b3 Author: James Bognar <[email protected]> AuthorDate: Wed Jul 22 14:26:23 2026 -0400 Fix RateLimitGuard test flakiness with injectable time source Make the InMemoryStorage/Bucket token-bucket clock injectable (a LongSupplier defaulting to System::nanoTime) so timing-sensitive tests can freeze the ~1s refill window. Production behavior is unchanged (Storage.inMemory()/builder still use the real clock; resolveKey() untouched). Wire a frozen clock into the 5 RateLimitGuard* test classes whose 200->429 assertions previously raced the refill window and could flake under heavy load. Co-authored-by: Cursor <[email protected]> --- .../guard/RateLimitGuard_AdvisoryHeaders_Test.java | 3 ++ .../guard/RateLimitGuard_ExemptPaths_Test.java | 7 ++++ .../guard/RateLimitGuard_KeyIsolation_Test.java | 5 +++ .../rest/server/guard/RateLimitGuard_Test.java | 3 ++ .../guard/RateLimitGuard_XForwardedFor_Test.java | 5 +++ .../juneau/rest/server/guard/RateLimitGuard.java | 47 +++++++++++++++++++--- 6 files changed, 65 insertions(+), 5 deletions(-) diff --git a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_AdvisoryHeaders_Test.java b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_AdvisoryHeaders_Test.java index 8281bcf8d4..83f5cf3f65 100644 --- a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_AdvisoryHeaders_Test.java +++ b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_AdvisoryHeaders_Test.java @@ -66,6 +66,9 @@ class RateLimitGuard_AdvisoryHeaders_Test extends TestBase { .burst(5) .keyBy(req -> "static-headers-rejection") .exemptPaths() + // Frozen clock: draining all 5 burst tokens then hitting 429 depends on landing inside the + // same 1-second refill window, which a real clock can't guarantee under CPU starvation/GC pauses. + .storage(new RateLimitGuard.InMemoryStorage(100_000, () -> 0L)) .build() ).build(); } diff --git a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_ExemptPaths_Test.java b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_ExemptPaths_Test.java index ddbae2eed4..e9cbc8a70a 100644 --- a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_ExemptPaths_Test.java +++ b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_ExemptPaths_Test.java @@ -38,6 +38,9 @@ class RateLimitGuard_ExemptPaths_Test extends TestBase { .permitsPerSecond(1) .burst(1) .keyBy(req -> "static-exempt") + // Frozen clock: the trailing 200/429 pair below depends on landing inside the same + // 1-second refill window, which a real clock can't guarantee under CPU starvation/GC pauses. + .storage(new RateLimitGuard.InMemoryStorage(100_000, () -> 0L)) .build() ).build(); } @@ -76,6 +79,8 @@ class RateLimitGuard_ExemptPaths_Test extends TestBase { .burst(1) .keyBy(req -> "static-custom-exempt") .exemptPaths("/special") + // Frozen clock: see rationale on class A above. + .storage(new RateLimitGuard.InMemoryStorage(100_000, () -> 0L)) .build() ).build(); } @@ -107,6 +112,8 @@ class RateLimitGuard_ExemptPaths_Test extends TestBase { .burst(1) .keyBy(req -> "static-no-exempt") .exemptPaths() + // Frozen clock: see rationale on class A above. + .storage(new RateLimitGuard.InMemoryStorage(100_000, () -> 0L)) .build() ).build(); } diff --git a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_KeyIsolation_Test.java b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_KeyIsolation_Test.java index ed3b87a763..d9390f0b48 100644 --- a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_KeyIsolation_Test.java +++ b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_KeyIsolation_Test.java @@ -37,6 +37,9 @@ class RateLimitGuard_KeyIsolation_Test extends TestBase { .burst(1) .keyBy(req -> req.getHeader("X-Tenant")) .exemptPaths() + // Frozen clock: the per-tenant 200/429 pairs below depend on landing inside the same + // 1-second refill window, which a real clock can't guarantee under CPU starvation/GC pauses. + .storage(new RateLimitGuard.InMemoryStorage(100_000, () -> 0L)) .build() ).build(); } @@ -78,6 +81,8 @@ class RateLimitGuard_KeyIsolation_Test extends TestBase { .burst(1) .keyBy(req -> null) .exemptPaths() + // Frozen clock: see rationale on class A above. + .storage(new RateLimitGuard.InMemoryStorage(100_000, () -> 0L)) .build() ).build(); } diff --git a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_Test.java b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_Test.java index d79370c6b7..428ab34bad 100644 --- a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_Test.java +++ b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_Test.java @@ -45,6 +45,9 @@ class RateLimitGuard_Test extends TestBase { .burst(3) .keyBy(req -> "static-burst-drain") .exemptPaths() + // Frozen clock: draining all 3 burst tokens then hitting 429 depends on landing inside the + // same 1-second refill window, which a real clock can't guarantee under CPU starvation/GC pauses. + .storage(new RateLimitGuard.InMemoryStorage(100_000, () -> 0L)) .build() ).build(); } diff --git a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_XForwardedFor_Test.java b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_XForwardedFor_Test.java index d32a4f23da..2afb2d5fdf 100644 --- a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_XForwardedFor_Test.java +++ b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/guard/RateLimitGuard_XForwardedFor_Test.java @@ -43,6 +43,9 @@ class RateLimitGuard_XForwardedFor_Test extends TestBase { .burst(1) .xForwardedForAware(true) .exemptPaths() + // Frozen clock: the 200/429 sequence below depends on landing inside the same 1-second + // refill window, which a real clock can't guarantee under CPU starvation/GC pauses. + .storage(new RateLimitGuard.InMemoryStorage(100_000, () -> 0L)) .build() ).build(); } @@ -83,6 +86,8 @@ class RateLimitGuard_XForwardedFor_Test extends TestBase { .permitsPerSecond(1) .burst(1) .exemptPaths() + // Frozen clock: see rationale on class A above. + .storage(new RateLimitGuard.InMemoryStorage(100_000, () -> 0L)) .build() ).build(); } diff --git a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/guard/RateLimitGuard.java b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/guard/RateLimitGuard.java index 8fe9a91456..e6e8c5e9ba 100644 --- a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/guard/RateLimitGuard.java +++ b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/guard/RateLimitGuard.java @@ -87,7 +87,10 @@ import org.apache.juneau.rest.server.*; * * <p> * Refill math uses {@link System#nanoTime()}. This is monotonic and safe across wall-clock jumps but does not - * map to a calendar instant, which slightly complicates debugging when the bucket is in an unexpected state. + * map to a calendar instant, which slightly complicates debugging when the bucket is in an unexpected state. The + * nanosecond source is pluggable on {@link InMemoryStorage} (package-private constructor overload) so tests can + * pin the refill window with a deterministic clock instead of racing real wall-clock time; production code always + * goes through the single-arg constructor, which defaults to {@code System::nanoTime}. * * <h5 class='section'>See Also:</h5><ul> * <li class='link'><a class="doclink" href="https://juneau.apache.org/docs/topics/RestServerRateLimitAndRequestId";>REST Server — Rate-Limiting and Request-Id Propagation</a> @@ -578,16 +581,35 @@ public class RateLimitGuard extends RestGuard { private final ConcurrentHashMap<String,Bucket> buckets = new ConcurrentHashMap<>(); private final int maxKeys; + private final LongSupplier nanoClock; InMemoryStorage(int maxKeys) { + this(maxKeys, System::nanoTime); + } + + /** + * Constructor allowing the nanosecond time source to be overridden. + * + * <p> + * Test-only seam. Production code should always use {@link #InMemoryStorage(int)}, which defaults to + * real {@link System#nanoTime()}. Tests that need to pin the token-bucket refill window regardless of + * machine speed can supply a deterministic {@link LongSupplier} here instead of racing {@link Thread#sleep} + * against real time. + * + * @param maxKeys The maximum number of keys held before LRU-style eviction kicks in. Must be {@code > 0}. + * @param nanoClock Supplies the current time in nanoseconds for refill math and idle-eviction bookkeeping. Must not be <jk>null</jk>. + */ + InMemoryStorage(int maxKeys, LongSupplier nanoClock) { + assertArgNotNull("nanoClock", nanoClock); if (maxKeys <= 0) throw new IllegalArgumentException("Argument 'maxKeys' must be > 0."); this.maxKeys = maxKeys; + this.nanoClock = nanoClock; } @Override public AcquireResult tryAcquire(String key, int capacity, double permitsPerSecond) { - var bucket = buckets.computeIfAbsent(key, k -> new Bucket(capacity)); + var bucket = buckets.computeIfAbsent(key, k -> new Bucket(capacity, nanoClock)); var result = bucket.tryAcquire(capacity, permitsPerSecond); if (buckets.size() > maxKeys) evictOldest(); @@ -596,7 +618,7 @@ public class RateLimitGuard extends RestGuard { @Override public void evict(Duration ttl) { - var threshold = System.nanoTime() - ttl.toNanos(); + var threshold = nanoClock.getAsLong() - ttl.toNanos(); buckets.entrySet().removeIf(e -> e.getValue().lastTouchedNanos() < threshold); } @@ -642,15 +664,30 @@ public class RateLimitGuard extends RestGuard { private double tokens; private long lastNanos; private long lastWallMillis; + private final LongSupplier nanoClock; Bucket(int capacity) { + this(capacity, System::nanoTime); + } + + /** + * Constructor allowing the nanosecond time source to be overridden. + * + * <p> + * Test-only seam — see {@link InMemoryStorage#InMemoryStorage(int, LongSupplier)}. + * + * @param capacity The initial (full) token count. + * @param nanoClock Supplies the current time in nanoseconds for refill math. Must not be <jk>null</jk>. + */ + Bucket(int capacity, LongSupplier nanoClock) { + this.nanoClock = nanoClock; this.tokens = capacity; - this.lastNanos = System.nanoTime(); + this.lastNanos = nanoClock.getAsLong(); this.lastWallMillis = System.currentTimeMillis(); } synchronized Storage.AcquireResult tryAcquire(int capacity, double permitsPerSecond) { - var now = System.nanoTime(); + var now = nanoClock.getAsLong(); var elapsedSeconds = (now - lastNanos) / 1_000_000_000.0; tokens = Math.min(capacity, tokens + elapsedSeconds * permitsPerSecond); lastNanos = now;
