From 8848d7e87bd75b2b4b63529f55fc4cfad597864a Mon Sep 17 00:00:00 2001 From: yessjun Date: Sat, 8 Aug 2026 22:35:25 +0900 Subject: [PATCH] test: compare contract names in the design contract gate The gate compared path+method sets only, so a design-contract operationId or schema name could drift from the generated spec and still pass; 20 had drifted that way. Verified by pointing the gate at a renamed copy of the design contract and watching each axis fail. --- README.md | 3 +- .../pickle/contract/ContractDriftTest.java | 99 +++++++++++++++++-- 2 files changed, 94 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 64b49cb4..54e1ea50 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,8 @@ mvn test -Dtest=ContractDriftTest -Dcontract.update=true 갱신하지 않으면 `ContractDriftTest`가 빌드를 실패시킵니다. 환경 변수 `PICKLE_CONTRACT_MASTER`에 수기로 쓴 설계 명세 YAML 경로를 주면 설계 표면과 구현 표면의 -집합 대조까지 추가로 수행합니다. +집합 대조에 더해, 두 문서가 함께 가진 오퍼레이션의 `operationId`와 설계 명세가 붙인 +스키마명이 생성본과 같은지까지 대조합니다. 실행 중인 서버도 같은 스펙을 제공합니다: https://pickle.pusan.ac.kr/api/v1/openapi diff --git a/src/test/java/kr/ac/pusan/pickle/contract/ContractDriftTest.java b/src/test/java/kr/ac/pusan/pickle/contract/ContractDriftTest.java index 1812486b..aa46c394 100644 --- a/src/test/java/kr/ac/pusan/pickle/contract/ContractDriftTest.java +++ b/src/test/java/kr/ac/pusan/pickle/contract/ContractDriftTest.java @@ -10,10 +10,13 @@ import com.fasterxml.jackson.dataformat.yaml.YAMLMapper; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Iterator; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.TreeMap; import java.util.TreeSet; import kr.ac.pusan.pickle.support.EmbeddedPostgresConfig; import org.junit.jupiter.api.Assumptions; @@ -44,6 +47,14 @@ * parallel development against a frozen design contract; it must be empty * once the matching endpoints ship.

* + *

4. Design-contract names (optional) — the design contract states + * that it reuses the generated names verbatim, so for every operation the two + * documents share, their {@code operationId} must be equal, and every schema + * the design contract names must exist under the generated name. Check 3 alone + * passes on a name mismatch because it compares path+method only; 20 operation + * ids had drifted that way unnoticed before the 2026-08-08 alignment, which is + * why this axis is a gate rather than a manual comparison step.

+ * *

Limitation: checks 2 and 3 compare METHOD+path sets only. * Parameters, schema shapes and error codes are covered by check 1 (the * published snapshot is byte-stable) and by contract review.

@@ -257,13 +268,7 @@ void runtimeExposesExactlyTheImplementedSet() throws Exception { @Test void designContractSurfaceMatchesImplementedPlusPlanned() throws Exception { - String master = System.getenv("PICKLE_CONTRACT_MASTER"); - Assumptions.assumeTrue(master != null && !master.isBlank(), - "PICKLE_CONTRACT_MASTER not set — design-contract comparison skipped"); - - Path masterPath = Path.of(master); - assertThat(masterPath).as("design contract at $PICKLE_CONTRACT_MASTER").exists(); - JsonNode contract = new YAMLMapper().readTree(Files.readString(masterPath)); + JsonNode contract = designContractOrSkip(); // doesNotContainAnyElementsOf rejects an empty iterable with an // IllegalArgumentException, so guard the all-shipped state (PLANNED empty). @@ -281,6 +286,48 @@ void designContractSurfaceMatchesImplementedPlusPlanned() throws Exception { .isEqualTo(contractSurface); } + @Test + void designContractNamesMatchGeneratedNames() throws Exception { + JsonNode contract = designContractOrSkip(); + JsonNode runtime = fetchRuntimeSpec(); + + Map generatedIds = operationIdsOf(runtime, SERVER_PREFIX); + List drifted = new ArrayList<>(); + for (Map.Entry operation : operationIdsOf(contract, "").entrySet()) { + String generated = generatedIds.get(operation.getKey()); + if (generated != null && !generated.equals(operation.getValue())) { + drifted.add(operation.getKey() + " — design contract " + operation.getValue() + + ", generated " + generated); + } + } + assertThat(drifted) + .as("operationId drift, listed per operation the two specs share") + .isEmpty(); + + // Containment runs one way: the generated spec names every DTO the + // runtime exposes, while the design contract names only the subset it + // documents. An unimplemented design operation may carry schemas the + // runtime cannot know yet, so the axis holds only while PLANNED is empty. + if (PLANNED.isEmpty()) { + Set unknownSchemas = schemaNamesOf(contract); + unknownSchemas.removeAll(schemaNamesOf(runtime)); + assertThat(unknownSchemas) + .as("design contract schema names absent from the generated spec") + .isEmpty(); + } + } + + /** Reads the design contract, or skips the test when it was not pointed at. */ + private static JsonNode designContractOrSkip() throws Exception { + String master = System.getenv("PICKLE_CONTRACT_MASTER"); + Assumptions.assumeTrue(master != null && !master.isBlank(), + "PICKLE_CONTRACT_MASTER not set — design-contract comparison skipped"); + + Path masterPath = Path.of(master); + assertThat(masterPath).as("design contract at $PICKLE_CONTRACT_MASTER").exists(); + return new YAMLMapper().readTree(Files.readString(masterPath)); + } + /** * Published-spec convention: path keys are server-relative and the prefix * lives in {@code servers[0].url} — matching how typed clients (the console @@ -326,6 +373,44 @@ private static String toCanonicalYaml(JsonNode spec) throws Exception { return yaml.writeValueAsString(tree); } + /** + * Maps each "METHOD path" to its declared {@code operationId}, normalizing + * away {@code stripPrefix}. An operation without one maps to a placeholder + * so a missing id reads as drift instead of silently matching. + */ + private static Map operationIdsOf(JsonNode spec, String stripPrefix) { + Map operationIds = new TreeMap<>(); + JsonNode paths = spec.path("paths"); + for (Iterator> it = paths.properties().iterator(); it.hasNext(); ) { + Map.Entry entry = it.next(); + String path = entry.getKey(); + if (!stripPrefix.isEmpty() && path.startsWith(stripPrefix)) { + path = path.substring(stripPrefix.length()); + } + for (Iterator> ops = entry.getValue().properties().iterator(); + ops.hasNext(); ) { + Map.Entry operation = ops.next(); + String method = operation.getKey(); + if (!HTTP_METHODS.contains(method.toLowerCase(Locale.ROOT))) { + continue; + } + JsonNode operationId = operation.getValue().path("operationId"); + operationIds.put(method.toUpperCase(Locale.ROOT) + " " + path, + operationId.isTextual() ? operationId.asText() : "(no operationId)"); + } + } + return operationIds; + } + + /** Names declared under {@code components.schemas}. */ + private static Set schemaNamesOf(JsonNode spec) { + Set names = new TreeSet<>(); + for (Iterator it = spec.path("components").path("schemas").fieldNames(); it.hasNext(); ) { + names.add(it.next()); + } + return names; + } + /** Extracts "METHOD path" pairs, normalizing away {@code stripPrefix}. */ private static Set endpointsOf(JsonNode spec, String stripPrefix) { Set endpoints = new TreeSet<>();