diff --git a/docs/adr/0014-pan-redaction-payment-method-aggregate-and-idempotency-key.md b/docs/adr/0014-pan-redaction-payment-method-aggregate-and-idempotency-key.md index 5389e68..faff93a 100644 --- a/docs/adr/0014-pan-redaction-payment-method-aggregate-and-idempotency-key.md +++ b/docs/adr/0014-pan-redaction-payment-method-aggregate-and-idempotency-key.md @@ -31,3 +31,7 @@ Two smaller decisions follow from the same logic. A **blank header is treated as ## Consequences No new infrastructure and no new dependency: tokenization is a UUID and a `last_four`, dedupe is one table and one SHA-256. Cost is zero. The honest limits are recorded so they are not mistaken for finished work: the `vault_ref` is a placeholder, not a real external vault; tokens are not yet reused across a customer's orders (every checkout mints a new one), which is fine for a simulator but is the obvious next deepening if a real provider is wired in Phase 15; and the idempotency table grows unbounded until a retention sweep is added. **PragProg reversibility** — both features are additive and the header is optional, so nothing here forecloses a later move to a real tokenization provider or a managed idempotency layer. + +## Update — idempotency retention sweep (#73, 2026-06-05) + +The "grows unbounded" limitation above is now closed. `IdempotencyRetention` runs a scheduled sweep (`@EnableScheduling`, fixed delay `shopsphere.ordering.idempotency.sweep-interval`, default `PT1H`) that issues `DELETE FROM ordering.idempotency_keys WHERE created_at < now() − ttl`, where the TTL is `shopsphere.ordering.idempotency.ttl` (default `PT24H`). Flyway `V13` adds an index on `created_at` so the periodic delete finds expired rows without a full scan. The 24h default is a wide margin over the seconds-to-minutes a legitimate client retry takes, so the sweep never races a live dedupe: by the time a claim is eligible for deletion, no in-flight request could still reference it, and a retry arriving after expiry correctly places a fresh order. **XP YAGNI / PragProg** — this is the minimum that bounds the table (one scheduled `DELETE`, one index, two config knobs), not a partitioning or external-TTL scheme; the cutoff and cadence are properties, so tightening them later is a config change, not a redeploy of logic. The sweep is driven through a package-private `sweep()` method so the integration test can run it deterministically (backdated key → swept; fresh key survives), and the Phase-14 `IdempotencyKeyIT` dedup scenarios stay green because the default TTL is far larger than any test's lifetime. diff --git a/docs/modulith/components.puml b/docs/modulith/components.puml index 8acff6d..925b8b1 100644 --- a/docs/modulith/components.puml +++ b/docs/modulith/components.puml @@ -16,12 +16,12 @@ Container_Boundary("ShopSphere.ShopSphere_boundary", "ShopSphere", $tags="") { Component(ShopSphere.ShopSphere.Ordering, "Ordering", $techn="Module", $descr="", $tags="", $link="") } +Rel(ShopSphere.ShopSphere.Ordering, ShopSphere.ShopSphere.Common, "depends on", $techn="", $tags="", $link="") Rel(ShopSphere.ShopSphere.Ordering, ShopSphere.ShopSphere.Catalog, "uses", $techn="", $tags="", $link="") Rel(ShopSphere.ShopSphere.Ordering, ShopSphere.ShopSphere.Payment, "uses", $techn="", $tags="", $link="") -Rel(ShopSphere.ShopSphere.Payment, ShopSphere.ShopSphere.Common, "depends on", $techn="", $tags="", $link="") Rel(ShopSphere.ShopSphere.Catalog, ShopSphere.ShopSphere.Common, "depends on", $techn="", $tags="", $link="") +Rel(ShopSphere.ShopSphere.Payment, ShopSphere.ShopSphere.Common, "depends on", $techn="", $tags="", $link="") Rel(ShopSphere.ShopSphere.Ordering, ShopSphere.ShopSphere.Identity, "depends on", $techn="", $tags="", $link="") -Rel(ShopSphere.ShopSphere.Ordering, ShopSphere.ShopSphere.Common, "depends on", $techn="", $tags="", $link="") SHOW_LEGEND(true) @enduml \ No newline at end of file diff --git a/docs/modulith/module-ordering.puml b/docs/modulith/module-ordering.puml index d37d337..bd113fa 100644 --- a/docs/modulith/module-ordering.puml +++ b/docs/modulith/module-ordering.puml @@ -16,12 +16,12 @@ Container_Boundary("ShopSphere.ShopSphere_boundary", "ShopSphere", $tags="") { Component(ShopSphere.ShopSphere.Ordering, "Ordering", $techn="Module", $descr="", $tags="", $link="") } +Rel(ShopSphere.ShopSphere.Ordering, ShopSphere.ShopSphere.Common, "depends on", $techn="", $tags="", $link="") Rel(ShopSphere.ShopSphere.Ordering, ShopSphere.ShopSphere.Catalog, "uses", $techn="", $tags="", $link="") Rel(ShopSphere.ShopSphere.Ordering, ShopSphere.ShopSphere.Payment, "uses", $techn="", $tags="", $link="") -Rel(ShopSphere.ShopSphere.Payment, ShopSphere.ShopSphere.Common, "depends on", $techn="", $tags="", $link="") Rel(ShopSphere.ShopSphere.Catalog, ShopSphere.ShopSphere.Common, "depends on", $techn="", $tags="", $link="") +Rel(ShopSphere.ShopSphere.Payment, ShopSphere.ShopSphere.Common, "depends on", $techn="", $tags="", $link="") Rel(ShopSphere.ShopSphere.Ordering, ShopSphere.ShopSphere.Identity, "depends on", $techn="", $tags="", $link="") -Rel(ShopSphere.ShopSphere.Ordering, ShopSphere.ShopSphere.Common, "depends on", $techn="", $tags="", $link="") SHOW_LEGEND(true) @enduml \ No newline at end of file diff --git a/src/main/java/com/shopsphere/ordering/IdempotencyKeys.java b/src/main/java/com/shopsphere/ordering/IdempotencyKeys.java index ac1c364..d7ae4f4 100644 --- a/src/main/java/com/shopsphere/ordering/IdempotencyKeys.java +++ b/src/main/java/com/shopsphere/ordering/IdempotencyKeys.java @@ -3,6 +3,8 @@ import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.stereotype.Component; +import java.sql.Timestamp; +import java.time.Instant; import java.util.Optional; import java.util.UUID; @@ -41,6 +43,17 @@ Optional find(UUID customerId, String key) { customerId, key).stream().findFirst(); } + /** + * Deletes every claim created strictly before {@code cutoff} and returns how many rows went. + * Used by the retention sweep ({@link IdempotencyRetention}) to keep the table from growing + * unbounded; the cutoff only needs to predate any legitimate client-retry window. + */ + int deleteOlderThan(Instant cutoff) { + return jdbc.update( + "DELETE FROM ordering.idempotency_keys WHERE created_at < ?", + Timestamp.from(cutoff)); + } + record PriorRequest(String requestHash, UUID orderId) { } } diff --git a/src/main/java/com/shopsphere/ordering/IdempotencyRetention.java b/src/main/java/com/shopsphere/ordering/IdempotencyRetention.java new file mode 100644 index 0000000..55f6426 --- /dev/null +++ b/src/main/java/com/shopsphere/ordering/IdempotencyRetention.java @@ -0,0 +1,58 @@ +package com.shopsphere.ordering; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.stereotype.Component; + +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; + +/** + * Bounds the growth of {@code ordering.idempotency_keys}. Every keyed {@code POST /api/v1/orders} + * inserts a row and the placement path never removes it, so without this sweep the table and its + * primary-key index grow forever — the honest limitation deferred in ADR-0014 (issue #73). + *

+ * The sweep deletes any claim older than {@code shopsphere.ordering.idempotency.ttl}. That TTL only + * has to outlast the window in which a client might legitimately retry the same request (seconds to + * minutes), so the 24h default leaves a wide safety margin while still capping retention. Dropping + * an old claim is safe: a retry arriving after expiry simply places a fresh order, which is the + * correct behaviour once no in-flight request could still be referencing the key. + */ +@Component +class IdempotencyRetention { + + private static final Logger log = LoggerFactory.getLogger(IdempotencyRetention.class); + + private final IdempotencyKeys keys; + private final Duration ttl; + private final Clock clock; + + IdempotencyRetention(IdempotencyKeys keys, + @Value("${shopsphere.ordering.idempotency.ttl:PT24H}") Duration ttl, + Clock clock) { + this.keys = keys; + this.ttl = ttl; + this.clock = clock; + } + + /** + * Deletes idempotency claims older than the TTL and returns how many were removed. Package-private + * so it can be driven deterministically from a test; the schedule below only delegates here. + */ + int sweep() { + Instant cutoff = clock.instant().minus(ttl); + int removed = keys.deleteOlderThan(cutoff); + if (removed > 0) { + log.info("Idempotency retention sweep removed {} expired key(s) older than {}", removed, cutoff); + } + return removed; + } + + @Scheduled(fixedDelayString = "${shopsphere.ordering.idempotency.sweep-interval:PT1H}") + void scheduledSweep() { + sweep(); + } +} diff --git a/src/main/java/com/shopsphere/ordering/OrderingConfig.java b/src/main/java/com/shopsphere/ordering/OrderingConfig.java new file mode 100644 index 0000000..4f15223 --- /dev/null +++ b/src/main/java/com/shopsphere/ordering/OrderingConfig.java @@ -0,0 +1,13 @@ +package com.shopsphere.ordering; + +import org.springframework.context.annotation.Configuration; +import org.springframework.scheduling.annotation.EnableScheduling; + +/** + * Turns on Spring's scheduler so {@link IdempotencyRetention#scheduledSweep()} fires. Scoped to the + * Ordering module rather than the application class to keep the concern next to its only user. + */ +@Configuration +@EnableScheduling +class OrderingConfig { +} diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 501e7e3..9fbf171 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -63,3 +63,10 @@ shopsphere: ttl: PT15M refresh: ttl: P7D + ordering: + idempotency: + # Delete idempotency claims older than this. Must outlast any legitimate client-retry window; + # 24h is a wide margin over the seconds-to-minutes a real retry takes (#73, ADR-0014). + ttl: PT24H + # How often the retention sweep runs (fixed delay between completions). + sweep-interval: PT1H diff --git a/src/main/resources/db/migration/ordering/V13__idempotency_keys_created_at_index.sql b/src/main/resources/db/migration/ordering/V13__idempotency_keys_created_at_index.sql new file mode 100644 index 0000000..92ca37a --- /dev/null +++ b/src/main/resources/db/migration/ordering/V13__idempotency_keys_created_at_index.sql @@ -0,0 +1,5 @@ +-- Retention sweep (#73, deferred from ADR-0014) deletes idempotency claims by age: +-- DELETE FROM ordering.idempotency_keys WHERE created_at < . The primary key is on +-- (customer_id, idempotency_key), so that predicate would otherwise scan the whole table on every +-- sweep. This index lets the periodic DELETE find expired rows without a full scan. +CREATE INDEX idx_idempotency_keys_created_at ON ordering.idempotency_keys (created_at); diff --git a/src/test/java/com/shopsphere/ordering/IdempotencyRetentionIT.java b/src/test/java/com/shopsphere/ordering/IdempotencyRetentionIT.java new file mode 100644 index 0000000..b420806 --- /dev/null +++ b/src/test/java/com/shopsphere/ordering/IdempotencyRetentionIT.java @@ -0,0 +1,67 @@ +package com.shopsphere.ordering; + +import com.shopsphere.SharedContainers; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; + +import java.sql.Timestamp; +import java.time.Duration; +import java.time.Instant; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Retention sweep for {@code ordering.idempotency_keys} (#73, deferred from ADR-0014). A key older + * than the configured TTL is deleted by the sweep; a key inside the window survives untouched. + */ +@SpringBootTest +class IdempotencyRetentionIT { + + @DynamicPropertySource + static void containers(DynamicPropertyRegistry registry) { + SharedContainers.registerProperties(registry); + } + + @Autowired + IdempotencyRetention retention; + + @Autowired + JdbcTemplate jdbc; + + @Test + void sweepRemovesExpiredKeysButKeepsFreshOnes() { + UUID customer = UUID.randomUUID(); + String staleKey = "stale-" + UUID.randomUUID(); + String freshKey = "fresh-" + UUID.randomUUID(); + Instant now = Instant.now(); + + insertKey(customer, staleKey, now.minus(Duration.ofHours(48))); + insertKey(customer, freshKey, now); + + retention.sweep(); + + assertThat(rowExists(customer, staleKey)).isFalse(); + assertThat(rowExists(customer, freshKey)).isTrue(); + } + + private void insertKey(UUID customer, String key, Instant createdAt) { + jdbc.update( + "INSERT INTO ordering.idempotency_keys " + + "(customer_id, idempotency_key, request_hash, order_id, created_at) " + + "VALUES (?, ?, ?, ?, ?)", + customer, key, "hash", UUID.randomUUID(), Timestamp.from(createdAt)); + } + + private boolean rowExists(UUID customer, String key) { + Integer count = jdbc.queryForObject( + "SELECT count(*) FROM ordering.idempotency_keys " + + "WHERE customer_id = ? AND idempotency_key = ?", + Integer.class, customer, key); + return count != null && count > 0; + } +}