From b79306d1587cd69690fd6db42e8ba5ce8ee0c86f Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Mon, 24 Aug 2026 03:39:50 +0530 Subject: [PATCH 1/2] feat: CoverageStore contract tests and configurable eviction Add a reusable CoverageStoreContract JUnit base that any store can extend, run InMemoryCoverageStore against it, and let the in-memory store choose oldest-first or reject-when-full eviction via reqover.mvc.snapshot-eviction / reqover.webflux.snapshot-eviction. Document both next to max-snapshots in the integration guide. Fixes #14 Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> --- docs/17_integration_guide.md | 12 +- .../reqover/core/InMemoryCoverageStore.java | 43 +++++- .../reqover/core/SnapshotEvictionPolicy.java | 29 ++++ .../reqover/core/CoverageStoreContract.java | 136 ++++++++++++++++++ .../core/InMemoryCoverageStoreTest.java | 46 +++--- .../spring/mvc/ReqoverMvcConfiguration.java | 3 +- .../spring/mvc/ReqoverMvcProperties.java | 21 ++- .../mvc/ReqoverMvcAutoConfigurationTest.java | 16 +++ .../webflux/ReqoverWebFluxConfiguration.java | 3 +- .../webflux/ReqoverWebFluxProperties.java | 21 ++- 10 files changed, 299 insertions(+), 31 deletions(-) create mode 100644 reqover-core/src/main/java/io/reqover/core/SnapshotEvictionPolicy.java create mode 100644 reqover-core/src/test/java/io/reqover/core/CoverageStoreContract.java diff --git a/docs/17_integration_guide.md b/docs/17_integration_guide.md index 883a1d2..2b81aa8 100644 --- a/docs/17_integration_guide.md +++ b/docs/17_integration_guide.md @@ -161,10 +161,12 @@ Every property Reqover reads, with the default that applies when you leave it ou | `reqover.mvc.enabled` | `true` | Whether request attribution is installed at all. `false` keeps the MVC adapter out of the context | | `reqover.mvc.include-path-patterns` | `/**` | Ant path patterns the interceptor attributes. Defaults to everything | | `reqover.mvc.exclude-path-patterns` | `/reqover`, `/reqover/**`, `/error` | Paths excluded from attribution. Setting this **replaces** the default list | -| `reqover.mvc.max-snapshots` | `10000` | How many finished requests the default in-memory store retains before evicting the oldest. Ignored when you supply your own `CoverageStore` bean | +| `reqover.mvc.max-snapshots` | `10000` | How many finished requests the default in-memory store retains before applying the eviction policy. Ignored when you supply your own `CoverageStore` bean | +| `reqover.mvc.snapshot-eviction` | `oldest-first` | What happens at the bound: `oldest-first` drops the oldest snapshot (default), `reject-when-full` keeps the existing window and ignores new flushes. Ignored when you supply your own `CoverageStore` bean | | `reqover.webflux.enabled` | `true` | Whether the WebFlux adapter is installed. `false` also skips enabling Reactor's automatic context propagation | | `reqover.webflux.exclude-path-prefixes` | `/reqover` | Paths excluded from attribution, matched as **prefixes** (not Ant patterns). Setting this replaces the default list | | `reqover.webflux.max-snapshots` | `10000` | Same as `reqover.mvc.max-snapshots`, for reactive applications | +| `reqover.webflux.snapshot-eviction` | `oldest-first` | Same as `reqover.mvc.snapshot-eviction`, for reactive applications | | `reqover.report.endpoint.enabled` | **`false`** | Whether the built-in HTTP report endpoint is registered. Off by default — see [step 3](#3-decide-how-you-read-the-report) | | `reqover.report.endpoint.path` | `/reqover/report` | Base path for the endpoint. JSON is served here, and the HTML report at the same path with `.html` appended | | `reqover.report.export.json-path` | *unset* | Where to write the JSON report when the application context closes. Unset or blank means no JSON export | @@ -418,14 +420,20 @@ The most common failure is **"the report is empty"**, and the cause is usually ` ### Adjusting retention -Records live in memory only, with a default cap of 10,000 entries; beyond that the oldest are dropped. In `0.2.0` this is a property — no bean needed: +Records live in memory only, with a default cap of 10,000 entries. Beyond that the store either drops the oldest snapshot (`oldest-first`, the default) or keeps the existing window and ignores new flushes (`reject-when-full`). In `0.2.0` both the bound and the policy are properties — no bean needed: ```properties reqover.mvc.max-snapshots=50000 +reqover.mvc.snapshot-eviction=oldest-first +# Keep the first N for a long QA session instead of rolling: +# reqover.mvc.snapshot-eviction=reject-when-full # WebFlux: # reqover.webflux.max-snapshots=50000 +# reqover.webflux.snapshot-eviction=reject-when-full ``` +A second `CoverageStore` implementation can pin the same behaviour with the abstract JUnit contract in `reqover-core` tests (`CoverageStoreContract`): extend it, return your store from `newStore()`, and run the suite. + ### Replacing the store `CoverageStore` is the SPI for where records go. Define a bean of that type and the adapters back off — both contribute their store with `@ConditionalOnMissingBean(CoverageStore.class)` — so you can write snapshots to disk, to a database, or drop them under a sampling rule: diff --git a/reqover-core/src/main/java/io/reqover/core/InMemoryCoverageStore.java b/reqover-core/src/main/java/io/reqover/core/InMemoryCoverageStore.java index 37bf593..df8ca8b 100644 --- a/reqover-core/src/main/java/io/reqover/core/InMemoryCoverageStore.java +++ b/reqover-core/src/main/java/io/reqover/core/InMemoryCoverageStore.java @@ -1,16 +1,20 @@ package io.reqover.core; import java.util.List; +import java.util.Objects; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicInteger; /** * Thread-safe {@link CoverageStore} that retains snapshots in heap. * - *

The store keeps at most {@code maxSnapshots} entries; once the bound is - * reached, the oldest snapshots are evicted so a long-running application does - * not grow memory without limit. Eviction is best-effort under heavy - * concurrency, but the size stays close to the configured bound. + *

The store keeps at most {@code maxSnapshots} entries. What happens once + * that bound is reached is controlled by {@link SnapshotEvictionPolicy}: + * {@link SnapshotEvictionPolicy#OLDEST_FIRST} drops the oldest snapshot + * (the historical default), while {@link SnapshotEvictionPolicy#REJECT_WHEN_FULL} + * keeps the existing window and ignores further flushes. Eviction is + * best-effort under heavy concurrency, but the size stays close to the + * configured bound. */ public final class InMemoryCoverageStore implements CoverageStore { /** Default retention bound, sized for local development and demo traffic. */ @@ -19,16 +23,22 @@ public final class InMemoryCoverageStore implements CoverageStore { private final ConcurrentLinkedQueue completed = new ConcurrentLinkedQueue<>(); private final AtomicInteger size = new AtomicInteger(); private final int maxSnapshots; + private final SnapshotEvictionPolicy evictionPolicy; public InMemoryCoverageStore() { - this(DEFAULT_MAX_SNAPSHOTS); + this(DEFAULT_MAX_SNAPSHOTS, SnapshotEvictionPolicy.OLDEST_FIRST); } public InMemoryCoverageStore(int maxSnapshots) { + this(maxSnapshots, SnapshotEvictionPolicy.OLDEST_FIRST); + } + + public InMemoryCoverageStore(int maxSnapshots, SnapshotEvictionPolicy evictionPolicy) { if (maxSnapshots <= 0) { throw new IllegalArgumentException("maxSnapshots must be positive: " + maxSnapshots); } this.maxSnapshots = maxSnapshots; + this.evictionPolicy = Objects.requireNonNull(evictionPolicy, "evictionPolicy"); } /** The retention bound this store was built with. */ @@ -36,9 +46,30 @@ public int maxSnapshots() { return maxSnapshots; } + /** Policy applied when {@link #maxSnapshots()} is reached. */ + public SnapshotEvictionPolicy evictionPolicy() { + return evictionPolicy; + } + @Override public void flush(CoverageBucket bucket) { - completed.add(bucket.snapshot()); + CoverageBucketSnapshot snapshot = bucket.snapshot(); + if (evictionPolicy == SnapshotEvictionPolicy.REJECT_WHEN_FULL) { + // Reserve a slot first so concurrent flushes cannot overshoot the bound. + int current = size.get(); + while (current < maxSnapshots) { + if (size.compareAndSet(current, current + 1)) { + completed.add(snapshot); + return; + } + current = size.get(); + } + // Store is full — drop the new snapshot (already taken so the bucket + // cannot mutate what was retained earlier). + return; + } + + completed.add(snapshot); if (size.incrementAndGet() > maxSnapshots && completed.poll() != null) { size.decrementAndGet(); } diff --git a/reqover-core/src/main/java/io/reqover/core/SnapshotEvictionPolicy.java b/reqover-core/src/main/java/io/reqover/core/SnapshotEvictionPolicy.java new file mode 100644 index 0000000..43fe1c6 --- /dev/null +++ b/reqover-core/src/main/java/io/reqover/core/SnapshotEvictionPolicy.java @@ -0,0 +1,29 @@ +package io.reqover.core; + +/** + * How {@link InMemoryCoverageStore} behaves once {@code maxSnapshots} is reached. + */ +public enum SnapshotEvictionPolicy { + /** Drop the oldest retained snapshot to make room for the new one (default). */ + OLDEST_FIRST, + /** Leave the store unchanged and ignore the newly flushed bucket. */ + REJECT_WHEN_FULL; + + /** + * Parses a configuration token such as {@code oldest-first} or + * {@code reject-when-full}. Unknown values throw. + */ + public static SnapshotEvictionPolicy fromProperty(String value) { + if (value == null || value.isBlank()) { + return OLDEST_FIRST; + } + String normalized = value.trim().toLowerCase().replace('_', '-'); + return switch (normalized) { + case "oldest-first", "oldestfirst", "oldest" -> OLDEST_FIRST; + case "reject-when-full", "rejectwhenfull", "reject" -> REJECT_WHEN_FULL; + default -> throw new IllegalArgumentException( + "Unknown snapshot eviction policy: " + value + + " (expected oldest-first or reject-when-full)"); + }; + } +} diff --git a/reqover-core/src/test/java/io/reqover/core/CoverageStoreContract.java b/reqover-core/src/test/java/io/reqover/core/CoverageStoreContract.java new file mode 100644 index 0000000..39af6ad --- /dev/null +++ b/reqover-core/src/test/java/io/reqover/core/CoverageStoreContract.java @@ -0,0 +1,136 @@ +package io.reqover.core; + +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Reusable contract tests for {@link CoverageStore} implementations. + * + *

Subclass this and implement {@link #newStore()} to pin a second store + * against the same behaviour the in-memory implementation is held to. + */ +public abstract class CoverageStoreContract { + + /** Fresh empty store under test. */ + protected abstract CoverageStore newStore(); + + protected CoverageBucket bucket(String unitId) { + return new CoverageBucket(UnitInfo.httpRequest(unitId, "GET", "/orders/{id}")); + } + + @Test + void flushedBucketAppearsInSnapshots() { + CoverageStore store = newStore(); + CoverageBucket bucket = bucket("req-1"); + bucket.record(10, 3); + + store.flush(bucket); + + assertEquals(1, store.snapshots().size()); + assertTrue(store.snapshots().get(0).hasHit(10, 3)); + assertEquals("req-1", store.snapshots().get(0).unitInfo().unitId()); + } + + @Test + void snapshotsReturnsCopySafeToIterateWhileAnotherThreadFlushes() throws Exception { + CoverageStore store = newStore(); + store.flush(bucket("seed")); + + List view = store.snapshots(); + assertEquals(1, view.size()); + + int readers = 4; + int flushes = 40; + ExecutorService pool = Executors.newFixedThreadPool(readers + 1); + CyclicBarrier start = new CyclicBarrier(readers + 1); + CountDownLatch done = new CountDownLatch(readers + 1); + AtomicReference failure = new AtomicReference<>(); + + Future writer = pool.submit(() -> { + try { + start.await(5, TimeUnit.SECONDS); + for (int i = 0; i < flushes; i++) { + store.flush(bucket("w-" + i)); + } + } catch (Throwable t) { + failure.compareAndSet(null, t); + } finally { + done.countDown(); + } + }); + + List> readerFutures = new ArrayList<>(); + for (int r = 0; r < readers; r++) { + readerFutures.add(pool.submit(() -> { + try { + start.await(5, TimeUnit.SECONDS); + for (int i = 0; i < 200; i++) { + int sum = 0; + for (CoverageBucketSnapshot snapshot : view) { + sum += snapshot.unitInfo().unitId().length(); + } + assertTrue(sum > 0); + // Fresh snapshots() must not be the same list instance. + assertNotSame(view, store.snapshots()); + } + } catch (Throwable t) { + failure.compareAndSet(null, t); + } finally { + done.countDown(); + } + })); + } + + assertTrue(done.await(30, TimeUnit.SECONDS), "contract threads timed out"); + writer.get(5, TimeUnit.SECONDS); + for (Future future : readerFutures) { + future.get(5, TimeUnit.SECONDS); + } + pool.shutdownNow(); + if (failure.get() != null) { + throw new AssertionError("contract concurrency failure", failure.get()); + } + assertEquals(1, view.size(), "snapshot list returned earlier must stay stable"); + } + + @Test + void clearEmptiesTheStore() { + CoverageStore store = newStore(); + store.flush(bucket("a")); + store.flush(bucket("b")); + assertFalse(store.snapshots().isEmpty()); + + store.clear(); + + assertTrue(store.snapshots().isEmpty()); + } + + @Test + void hitsAfterFlushDoNotMutateStoredSnapshot() { + CoverageStore store = newStore(); + CoverageBucket bucket = bucket("req-live"); + bucket.record(1, 1); + store.flush(bucket); + + bucket.record(2, 2); + + CoverageBucketSnapshot stored = store.snapshots().get(0); + assertTrue(stored.hasHit(1, 1)); + assertFalse(stored.hasHit(2, 2), "post-flush hits must not mutate the retained snapshot"); + assertTrue(bucket.hasHit(2, 2), "the live bucket may keep receiving hits"); + } +} diff --git a/reqover-core/src/test/java/io/reqover/core/InMemoryCoverageStoreTest.java b/reqover-core/src/test/java/io/reqover/core/InMemoryCoverageStoreTest.java index 448c7a4..59aff1d 100644 --- a/reqover-core/src/test/java/io/reqover/core/InMemoryCoverageStoreTest.java +++ b/reqover-core/src/test/java/io/reqover/core/InMemoryCoverageStoreTest.java @@ -3,19 +3,12 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertThrows; -class InMemoryCoverageStoreTest { - @Test - void flushesSnapshots() { - InMemoryCoverageStore store = new InMemoryCoverageStore(); - CoverageBucket bucket = new CoverageBucket(UnitInfo.httpRequest("req-1", "GET", "/orders/{id}")); - bucket.record(10, 3); - - store.flush(bucket); - - assertEquals(1, store.snapshots().size()); - assertTrue(store.snapshots().get(0).hasHit(10, 3)); +class InMemoryCoverageStoreTest extends CoverageStoreContract { + @Override + protected CoverageStore newStore() { + return new InMemoryCoverageStore(); } @Test @@ -29,14 +22,33 @@ void evictsOldestSnapshotsBeyondCapacity() { assertEquals(2, store.snapshots().size()); assertEquals("req-2", store.snapshots().get(0).unitInfo().unitId()); assertEquals("req-3", store.snapshots().get(1).unitInfo().unitId()); + assertEquals(SnapshotEvictionPolicy.OLDEST_FIRST, store.evictionPolicy()); + } + + @Test + void rejectWhenFullKeepsExistingWindow() { + InMemoryCoverageStore store = + new InMemoryCoverageStore(2, SnapshotEvictionPolicy.REJECT_WHEN_FULL); + + store.flush(new CoverageBucket(UnitInfo.httpRequest("req-1", "GET", "/orders/{id}"))); + store.flush(new CoverageBucket(UnitInfo.httpRequest("req-2", "GET", "/orders/{id}"))); + store.flush(new CoverageBucket(UnitInfo.httpRequest("req-3", "GET", "/orders/{id}"))); + + assertEquals(2, store.snapshots().size()); + assertEquals("req-1", store.snapshots().get(0).unitInfo().unitId()); + assertEquals("req-2", store.snapshots().get(1).unitInfo().unitId()); + assertEquals(SnapshotEvictionPolicy.REJECT_WHEN_FULL, store.evictionPolicy()); } @Test void rejectsNonPositiveCapacity() { - org.junit.jupiter.api.Assertions.assertThrows( - IllegalArgumentException.class, - () -> new InMemoryCoverageStore(0) - ); + assertThrows(IllegalArgumentException.class, () -> new InMemoryCoverageStore(0)); } -} + @Test + void parsesEvictionPolicyTokens() { + assertEquals(SnapshotEvictionPolicy.OLDEST_FIRST, SnapshotEvictionPolicy.fromProperty("oldest-first")); + assertEquals(SnapshotEvictionPolicy.REJECT_WHEN_FULL, SnapshotEvictionPolicy.fromProperty("reject-when-full")); + assertThrows(IllegalArgumentException.class, () -> SnapshotEvictionPolicy.fromProperty("sample")); + } +} diff --git a/reqover-spring-mvc/src/main/java/io/reqover/spring/mvc/ReqoverMvcConfiguration.java b/reqover-spring-mvc/src/main/java/io/reqover/spring/mvc/ReqoverMvcConfiguration.java index 8ce438c..2e82cd2 100644 --- a/reqover-spring-mvc/src/main/java/io/reqover/spring/mvc/ReqoverMvcConfiguration.java +++ b/reqover-spring-mvc/src/main/java/io/reqover/spring/mvc/ReqoverMvcConfiguration.java @@ -20,7 +20,7 @@ public class ReqoverMvcConfiguration { @Bean @ConditionalOnMissingBean(CoverageStore.class) public CoverageStore reqoverCoverageStore(ReqoverMvcProperties properties) { - return new InMemoryCoverageStore(properties.getMaxSnapshots()); + return new InMemoryCoverageStore(properties.getMaxSnapshots(), properties.getSnapshotEviction()); } @Bean @@ -53,3 +53,4 @@ public void addInterceptors(InterceptorRegistry registry) { }; } } + diff --git a/reqover-spring-mvc/src/main/java/io/reqover/spring/mvc/ReqoverMvcProperties.java b/reqover-spring-mvc/src/main/java/io/reqover/spring/mvc/ReqoverMvcProperties.java index 15ccd0d..0fa1b9c 100644 --- a/reqover-spring-mvc/src/main/java/io/reqover/spring/mvc/ReqoverMvcProperties.java +++ b/reqover-spring-mvc/src/main/java/io/reqover/spring/mvc/ReqoverMvcProperties.java @@ -1,6 +1,7 @@ package io.reqover.spring.mvc; import io.reqover.core.InMemoryCoverageStore; +import io.reqover.core.SnapshotEvictionPolicy; import org.springframework.boot.context.properties.ConfigurationProperties; import java.util.ArrayList; @@ -16,6 +17,7 @@ public class ReqoverMvcProperties { private List excludePathPatterns = new ArrayList<>(List.of("/reqover", "/reqover/**", "/error")); private int maxSnapshots = InMemoryCoverageStore.DEFAULT_MAX_SNAPSHOTS; + private SnapshotEvictionPolicy snapshotEviction = SnapshotEvictionPolicy.OLDEST_FIRST; /** Whether request attribution is installed at all. */ public boolean isEnabled() { @@ -49,8 +51,8 @@ public void setExcludePathPatterns(List excludePathPatterns) { /** * How many finished requests the default in-memory store retains before - * evicting the oldest. Ignored when the application supplies its own - * {@link io.reqover.core.CoverageStore} bean. + * applying {@link #getSnapshotEviction()}. Ignored when the application + * supplies its own {@link io.reqover.core.CoverageStore} bean. */ public int getMaxSnapshots() { return maxSnapshots; @@ -59,4 +61,19 @@ public int getMaxSnapshots() { public void setMaxSnapshots(int maxSnapshots) { this.maxSnapshots = maxSnapshots; } + + /** + * What the default in-memory store does once {@link #getMaxSnapshots()} is + * reached: drop the oldest snapshot, or reject new ones. Ignored when the + * application supplies its own {@link io.reqover.core.CoverageStore} bean. + */ + public SnapshotEvictionPolicy getSnapshotEviction() { + return snapshotEviction; + } + + public void setSnapshotEviction(SnapshotEvictionPolicy snapshotEviction) { + this.snapshotEviction = snapshotEviction == null + ? SnapshotEvictionPolicy.OLDEST_FIRST + : snapshotEviction; + } } diff --git a/reqover-spring-mvc/src/test/java/io/reqover/spring/mvc/ReqoverMvcAutoConfigurationTest.java b/reqover-spring-mvc/src/test/java/io/reqover/spring/mvc/ReqoverMvcAutoConfigurationTest.java index 9d0fb62..6b648b9 100644 --- a/reqover-spring-mvc/src/test/java/io/reqover/spring/mvc/ReqoverMvcAutoConfigurationTest.java +++ b/reqover-spring-mvc/src/test/java/io/reqover/spring/mvc/ReqoverMvcAutoConfigurationTest.java @@ -50,6 +50,22 @@ void sizesTheDefaultStoreFromTheConfiguredBound() { }); } + @Test + void configuresSnapshotEvictionOnTheDefaultStore() { + contextRunner + .withPropertyValues( + "reqover.mvc.max-snapshots=3", + "reqover.mvc.snapshot-eviction=reject-when-full" + ) + .run(context -> { + InMemoryCoverageStore store = (InMemoryCoverageStore) context.getBean(CoverageStore.class); + assertEquals(3, store.maxSnapshots()); + assertEquals( + io.reqover.core.SnapshotEvictionPolicy.REJECT_WHEN_FULL, + store.evictionPolicy()); + }); + } + @Test void backsOffWhenTheApplicationSuppliesItsOwnStore() { contextRunner diff --git a/reqover-spring-webflux/src/main/java/io/reqover/spring/webflux/ReqoverWebFluxConfiguration.java b/reqover-spring-webflux/src/main/java/io/reqover/spring/webflux/ReqoverWebFluxConfiguration.java index 87dad65..77d2899 100644 --- a/reqover-spring-webflux/src/main/java/io/reqover/spring/webflux/ReqoverWebFluxConfiguration.java +++ b/reqover-spring-webflux/src/main/java/io/reqover/spring/webflux/ReqoverWebFluxConfiguration.java @@ -39,7 +39,7 @@ public void afterPropertiesSet() { @Bean @ConditionalOnMissingBean(CoverageStore.class) public CoverageStore reqoverCoverageStore(ReqoverWebFluxProperties properties) { - return new InMemoryCoverageStore(properties.getMaxSnapshots()); + return new InMemoryCoverageStore(properties.getMaxSnapshots(), properties.getSnapshotEviction()); } @Bean @@ -58,3 +58,4 @@ public ReqoverWebFilter reqoverWebFilter( return new ReqoverWebFilter(coverageStore, requestIdGenerator, properties.getExcludePathPrefixes()); } } + diff --git a/reqover-spring-webflux/src/main/java/io/reqover/spring/webflux/ReqoverWebFluxProperties.java b/reqover-spring-webflux/src/main/java/io/reqover/spring/webflux/ReqoverWebFluxProperties.java index 817dabb..19b6732 100644 --- a/reqover-spring-webflux/src/main/java/io/reqover/spring/webflux/ReqoverWebFluxProperties.java +++ b/reqover-spring-webflux/src/main/java/io/reqover/spring/webflux/ReqoverWebFluxProperties.java @@ -1,6 +1,7 @@ package io.reqover.spring.webflux; import io.reqover.core.InMemoryCoverageStore; +import io.reqover.core.SnapshotEvictionPolicy; import org.springframework.boot.context.properties.ConfigurationProperties; import java.util.ArrayList; @@ -14,6 +15,7 @@ public class ReqoverWebFluxProperties { private boolean enabled = true; private List excludePathPrefixes = new ArrayList<>(List.of("/reqover")); private int maxSnapshots = InMemoryCoverageStore.DEFAULT_MAX_SNAPSHOTS; + private SnapshotEvictionPolicy snapshotEviction = SnapshotEvictionPolicy.OLDEST_FIRST; /** * Whether the adapter is installed. Turning it off also skips enabling @@ -43,8 +45,8 @@ public void setExcludePathPrefixes(List excludePathPrefixes) { /** * How many finished requests the default in-memory store retains before - * evicting the oldest. Ignored when the application supplies its own - * {@link io.reqover.core.CoverageStore} bean. + * applying {@link #getSnapshotEviction()}. Ignored when the application + * supplies its own {@link io.reqover.core.CoverageStore} bean. */ public int getMaxSnapshots() { return maxSnapshots; @@ -53,4 +55,19 @@ public int getMaxSnapshots() { public void setMaxSnapshots(int maxSnapshots) { this.maxSnapshots = maxSnapshots; } + + /** + * What the default in-memory store does once {@link #getMaxSnapshots()} is + * reached: drop the oldest snapshot, or reject new ones. Ignored when the + * application supplies its own {@link io.reqover.core.CoverageStore} bean. + */ + public SnapshotEvictionPolicy getSnapshotEviction() { + return snapshotEviction; + } + + public void setSnapshotEviction(SnapshotEvictionPolicy snapshotEviction) { + this.snapshotEviction = snapshotEviction == null + ? SnapshotEvictionPolicy.OLDEST_FIRST + : snapshotEviction; + } } From 276d7df2ac6e1e0aa22805a99b242eba3cb16178 Mon Sep 17 00:00:00 2001 From: lsmin3388 Date: Wed, 2 Sep 2026 15:35:11 +0900 Subject: [PATCH 2/2] refactor(core): drop the unreachable eviction-policy parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spring's relaxed binding already turns `oldest-first` and `reject-when-full` into the enum constants, so `fromProperty` had no caller in main and was a second parser that could drift from the first. It also lower-cased with the default locale, which turns `OLDEST_FIRST` into `oldest-fırst` under tr_TR and throws. While finishing the review of #15: the Korean integration guide gets the same two property rows and the retention paragraph, the changelog records the feature under Unreleased, the English guide stops attributing the policy to 0.2.0, and the two configuration classes lose a trailing blank line. --- CHANGELOG.md | 11 +++++++++ docs/17_integration_guide.ko.md | 12 ++++++++-- docs/17_integration_guide.md | 2 +- .../reqover/core/SnapshotEvictionPolicy.java | 24 ++++--------------- .../core/InMemoryCoverageStoreTest.java | 7 ------ .../spring/mvc/ReqoverMvcConfiguration.java | 1 - .../webflux/ReqoverWebFluxConfiguration.java | 1 - 7 files changed, 27 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b74498..446487d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,17 @@ All notable changes to Reqover are documented in this file. ### Added +- **A configurable eviction policy for the in-memory store.** + `reqover.mvc.snapshot-eviction` and `reqover.webflux.snapshot-eviction` + choose what happens at `max-snapshots`: `oldest-first` (the default, and the + only behaviour until now) or `reject-when-full`, which keeps the first N + snapshots and ignores later flushes. The slot is reserved with a CAS, so + concurrent flushes cannot overshoot the bound. +- **`CoverageStoreContract`**, an abstract JUnit suite in the `reqover-core` + tests that a second `CoverageStore` implementation extends to pin the SPI's + invariants — in particular that `snapshots()` stays stable while another + thread is flushing. (#14; contributed in #15 by @VedantMadane, the first + change to Reqover from outside the team) - **Compatibility policy** ([docs/20_versioning_and_compatibility.md](docs/20_versioning_and_compatibility.md)): what a version number promises, what counts as public API, how the report schema evolves, and what happens when a release is broken. Maven Central is diff --git a/docs/17_integration_guide.ko.md b/docs/17_integration_guide.ko.md index 679cff0..8fa2bbf 100644 --- a/docs/17_integration_guide.ko.md +++ b/docs/17_integration_guide.ko.md @@ -161,10 +161,12 @@ Reqover가 읽는 속성 전부와, 지정하지 않았을 때 적용되는 기 | `reqover.mvc.enabled` | `true` | 요청 추적을 아예 붙일지 여부. `false`면 MVC 어댑터가 컨텍스트에 올라오지 않습니다 | | `reqover.mvc.include-path-patterns` | `/**` | 인터셉터가 추적할 Ant 경로 패턴. 기본은 전체입니다 | | `reqover.mvc.exclude-path-patterns` | `/reqover`, `/reqover/**`, `/error` | 추적에서 제외할 경로. 이 속성을 지정하면 기본 목록을 **대체**합니다 | -| `reqover.mvc.max-snapshots` | `10000` | 기본 인메모리 저장소가 오래된 것을 지우기 전까지 보관하는 완료 요청 수. 내 `CoverageStore` 빈을 넣으면 무시됩니다 | +| `reqover.mvc.max-snapshots` | `10000` | 기본 인메모리 저장소가 보관 정책을 적용하기 전까지 보관하는 완료 요청 수. 내 `CoverageStore` 빈을 넣으면 무시됩니다 | +| `reqover.mvc.snapshot-eviction` | `oldest-first` | 상한에 닿았을 때의 동작: `oldest-first`는 가장 오래된 스냅샷을 지우고(기본), `reject-when-full`은 기존 창을 그대로 두고 새로 들어오는 것을 버립니다. 내 `CoverageStore` 빈을 넣으면 무시됩니다 | | `reqover.webflux.enabled` | `true` | WebFlux 어댑터를 붙일지 여부. `false`면 Reactor 컨텍스트 자동 전달을 켜는 것도 건너뜁니다 | | `reqover.webflux.exclude-path-prefixes` | `/reqover` | 추적에서 제외할 경로. Ant 패턴이 아니라 **앞부분 일치**로 비교합니다. 지정하면 기본 목록을 대체합니다 | | `reqover.webflux.max-snapshots` | `10000` | `reqover.mvc.max-snapshots`와 같고, 리액티브 애플리케이션용입니다 | +| `reqover.webflux.snapshot-eviction` | `oldest-first` | `reqover.mvc.snapshot-eviction`과 같고, 리액티브 애플리케이션용입니다 | | `reqover.report.endpoint.enabled` | **`false`** | 내장 HTTP 리포트 엔드포인트를 등록할지 여부. 기본은 꺼짐 — [3단계](#3-리포트를-어떻게-볼지-정하기) 참고 | | `reqover.report.endpoint.path` | `/reqover/report` | 엔드포인트의 기준 경로. 이 경로로 JSON이, 같은 경로에 `.html`을 붙인 경로로 HTML 리포트가 나갑니다 | | `reqover.report.export.json-path` | *지정 안 함* | 애플리케이션 컨텍스트가 닫힐 때 JSON 리포트를 쓸 경로. 비워두면 JSON을 내보내지 않습니다 | @@ -418,14 +420,20 @@ WebFlux라면 하나 더 — 한 API의 기록 안에 **서로 다른 스레드 ### 보관 개수 조정 -기록은 메모리에만 남고 기본 상한이 10,000건입니다. 넘으면 오래된 것부터 지워집니다. `0.2.0`부터는 속성으로 조정합니다 — 빈을 만들 필요가 없습니다. +기록은 메모리에만 남고 기본 상한이 10,000건입니다. 상한을 넘으면 가장 오래된 스냅샷을 지우거나(`oldest-first`, 기본), 기존 창을 그대로 두고 새로 들어오는 것을 버립니다(`reject-when-full`). 상한과 정책 모두 속성으로 조정합니다 — 빈을 만들 필요가 없습니다. ```properties reqover.mvc.max-snapshots=50000 +reqover.mvc.snapshot-eviction=oldest-first +# 긴 QA 세션에서 처음 N건을 남기고 싶다면: +# reqover.mvc.snapshot-eviction=reject-when-full # WebFlux라면 # reqover.webflux.max-snapshots=50000 +# reqover.webflux.snapshot-eviction=reject-when-full ``` +`CoverageStore`를 직접 구현한다면 `reqover-core` 테스트에 있는 추상 JUnit 계약(`CoverageStoreContract`)으로 같은 동작을 보장할 수 있습니다. 상속해서 `newStore()`가 내 저장소를 돌려주게 하고 테스트를 돌리면 됩니다. + ### 저장소 교체하기 기록이 어디로 갈지는 `CoverageStore`가 SPI입니다. 이 타입의 빈을 정의하면 어댑터가 물러납니다 — 두 어댑터 모두 자기 저장소 빈에 `@ConditionalOnMissingBean(CoverageStore.class)`를 걸어 두었습니다. 그래서 스냅샷을 디스크나 데이터베이스에 쓰거나, 샘플링 규칙으로 버리는 구현을 넣을 수 있습니다. diff --git a/docs/17_integration_guide.md b/docs/17_integration_guide.md index 4d1df57..ac09eb8 100644 --- a/docs/17_integration_guide.md +++ b/docs/17_integration_guide.md @@ -420,7 +420,7 @@ The most common failure is **"the report is empty"**, and the cause is usually ` ### Adjusting retention -Records live in memory only, with a default cap of 10,000 entries. Beyond that the store either drops the oldest snapshot (`oldest-first`, the default) or keeps the existing window and ignores new flushes (`reject-when-full`). In `0.2.0` both the bound and the policy are properties — no bean needed: +Records live in memory only, with a default cap of 10,000 entries. Beyond that the store either drops the oldest snapshot (`oldest-first`, the default) or keeps the existing window and ignores new flushes (`reject-when-full`). Both the bound and the policy are properties — no bean needed: ```properties reqover.mvc.max-snapshots=50000 diff --git a/reqover-core/src/main/java/io/reqover/core/SnapshotEvictionPolicy.java b/reqover-core/src/main/java/io/reqover/core/SnapshotEvictionPolicy.java index 43fe1c6..950aa34 100644 --- a/reqover-core/src/main/java/io/reqover/core/SnapshotEvictionPolicy.java +++ b/reqover-core/src/main/java/io/reqover/core/SnapshotEvictionPolicy.java @@ -2,28 +2,14 @@ /** * How {@link InMemoryCoverageStore} behaves once {@code maxSnapshots} is reached. + * + *

The Spring adapters bind {@code reqover.mvc.snapshot-eviction} and + * {@code reqover.webflux.snapshot-eviction} to these constants directly + * ({@code oldest-first}, {@code reject-when-full}); there is no separate parser. */ public enum SnapshotEvictionPolicy { /** Drop the oldest retained snapshot to make room for the new one (default). */ OLDEST_FIRST, /** Leave the store unchanged and ignore the newly flushed bucket. */ - REJECT_WHEN_FULL; - - /** - * Parses a configuration token such as {@code oldest-first} or - * {@code reject-when-full}. Unknown values throw. - */ - public static SnapshotEvictionPolicy fromProperty(String value) { - if (value == null || value.isBlank()) { - return OLDEST_FIRST; - } - String normalized = value.trim().toLowerCase().replace('_', '-'); - return switch (normalized) { - case "oldest-first", "oldestfirst", "oldest" -> OLDEST_FIRST; - case "reject-when-full", "rejectwhenfull", "reject" -> REJECT_WHEN_FULL; - default -> throw new IllegalArgumentException( - "Unknown snapshot eviction policy: " + value - + " (expected oldest-first or reject-when-full)"); - }; - } + REJECT_WHEN_FULL } diff --git a/reqover-core/src/test/java/io/reqover/core/InMemoryCoverageStoreTest.java b/reqover-core/src/test/java/io/reqover/core/InMemoryCoverageStoreTest.java index 59aff1d..55e7089 100644 --- a/reqover-core/src/test/java/io/reqover/core/InMemoryCoverageStoreTest.java +++ b/reqover-core/src/test/java/io/reqover/core/InMemoryCoverageStoreTest.java @@ -44,11 +44,4 @@ void rejectWhenFullKeepsExistingWindow() { void rejectsNonPositiveCapacity() { assertThrows(IllegalArgumentException.class, () -> new InMemoryCoverageStore(0)); } - - @Test - void parsesEvictionPolicyTokens() { - assertEquals(SnapshotEvictionPolicy.OLDEST_FIRST, SnapshotEvictionPolicy.fromProperty("oldest-first")); - assertEquals(SnapshotEvictionPolicy.REJECT_WHEN_FULL, SnapshotEvictionPolicy.fromProperty("reject-when-full")); - assertThrows(IllegalArgumentException.class, () -> SnapshotEvictionPolicy.fromProperty("sample")); - } } diff --git a/reqover-spring-mvc/src/main/java/io/reqover/spring/mvc/ReqoverMvcConfiguration.java b/reqover-spring-mvc/src/main/java/io/reqover/spring/mvc/ReqoverMvcConfiguration.java index 2e82cd2..ef6499a 100644 --- a/reqover-spring-mvc/src/main/java/io/reqover/spring/mvc/ReqoverMvcConfiguration.java +++ b/reqover-spring-mvc/src/main/java/io/reqover/spring/mvc/ReqoverMvcConfiguration.java @@ -53,4 +53,3 @@ public void addInterceptors(InterceptorRegistry registry) { }; } } - diff --git a/reqover-spring-webflux/src/main/java/io/reqover/spring/webflux/ReqoverWebFluxConfiguration.java b/reqover-spring-webflux/src/main/java/io/reqover/spring/webflux/ReqoverWebFluxConfiguration.java index 77d2899..29032b3 100644 --- a/reqover-spring-webflux/src/main/java/io/reqover/spring/webflux/ReqoverWebFluxConfiguration.java +++ b/reqover-spring-webflux/src/main/java/io/reqover/spring/webflux/ReqoverWebFluxConfiguration.java @@ -58,4 +58,3 @@ public ReqoverWebFilter reqoverWebFilter( return new ReqoverWebFilter(coverageStore, requestIdGenerator, properties.getExcludePathPrefixes()); } } -