feat: CoverageStore contract test and configurable eviction

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

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)`를 걸어 두었습니다. 그래서 스냅샷을 디스크나 데이터베이스에 쓰거나, 샘플링 규칙으로 버리는 구현을 넣을 수 있습니다.

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`). 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:

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.
  *
- * <p>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.
+ * <p>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,26 +23,53 @@ public final class InMemoryCoverageStore implements CoverageStore {
     private final ConcurrentLinkedQueue<CoverageBucketSnapshot> 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. */
     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();
         }

reqover-core/src/main/java/io/reqover/core/SnapshotEvictionPolicy.java

@@ -0,0 +1,15 @@
+package io.reqover.core;
+
+/**
+ * How {@link InMemoryCoverageStore} behaves once {@code maxSnapshots} is reached.
+ *
+ * <p>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
+}

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.
+ *
+ * <p>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<CoverageBucketSnapshot> 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<Throwable> 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<Future<?>> 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");
+    }
+}

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,26 @@ 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));
     }
 }
-