From 5cca6e61bc0377fede935efcb1ab06165f743771 Mon Sep 17 00:00:00 2001 From: Artur Iusupov Date: Wed, 23 Sep 2026 23:21:11 +0400 Subject: [PATCH] fix: make the OpenAPI document say what each operation answers Swagger is what a reviewer opens first, and it said 200 for every operation, although creations answer 201, sync requests 202 with Location, and ingest 201 or 200. It named no error at all, listed the demo chain simulator next to the API, and showed the validation getters isAmountValid, isDirectionValid, and isStatusValid as required request fields. The controllers declare their success codes, and one customizer adds the ProblemDetail errors every operation shares (400, 401, 403, 503) as application/problem+json with a ProblemDetail schema; the operation-specific errors stay in docs/api.md. The document covers /api only and responds in application/json, and the getters are ignored by Jackson, so they leave the schemas. --- CHANGELOG.md | 5 + README.md | 2 +- docs/testing.md | 1 + .../api/controller/AccountController.kt | 9 ++ .../api/controller/ObservedEventController.kt | 3 + .../api/controller/SyncController.kt | 14 +++ .../example/assetsync/api/dto/AccountDtos.kt | 2 + .../assetsync/api/dto/ObservedEventDtos.kt | 4 + .../assetsync/config/OpenApiConfiguration.kt | 49 ++++++++ src/main/resources/application.yml | 3 + .../OpenApiDocumentIntegrationTests.kt | 108 ++++++++++++++++++ 11 files changed, 199 insertions(+), 1 deletion(-) create mode 100644 src/test/kotlin/com/example/assetsync/integration/OpenApiDocumentIntegrationTests.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e1a554..1d7b3ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project are documented in this file. The format is b ## [Unreleased] +### Fixed + +- The OpenAPI document states the status each operation answers with, where it listed `200` for all of them: `201` for a new account or watched address, `201` or `200` for an ingested event, `202` with `Location` for a sync request. Every operation also lists the `ProblemDetail` errors all of them share, `400`, `401`, `403`, and `503`, under `application/problem+json`. +- The OpenAPI document covers `/api` only, so under `demo` it no longer lists the chain simulator. Its request schemas no longer name the validation getters `isAmountValid`, `isDirectionValid`, and `isStatusValid` as required fields. + ## [0.4.0] - 2026-09-23 ### Added diff --git a/README.md b/README.md index 8de26d0..7ec4af5 100644 --- a/README.md +++ b/README.md @@ -95,7 +95,7 @@ When the app is running: - Swagger UI: [http://localhost:18080/swagger-ui.html](http://localhost:18080/swagger-ui.html) - OpenAPI JSON: [http://localhost:18080/v3/api-docs](http://localhost:18080/v3/api-docs) -The OpenAPI document declares HTTP Basic, so **Authorize** in Swagger UI takes a username and password and sends them with every call. Outside `local` and `test` both pages and every API call require them. +The OpenAPI document declares HTTP Basic, so **Authorize** in Swagger UI takes a username and password and sends them with every call. Outside `local` and `test` both pages and every API call require them. The document covers the `/api` operations only, with the status each one answers with and the `ProblemDetail` errors all of them share (`400`, `401`, `403`, `503`); `docs/api.md` section 14 lists the rest. Current public endpoints: diff --git a/docs/testing.md b/docs/testing.md index 5a26ecb..ac525d0 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -67,6 +67,7 @@ Required cases: | Migrations | Full Liquibase changelog applies on an empty PostgreSQL database | | Accounts | `external_ref` uniqueness, valid statuses, nullable `external_ref` | | Watched addresses | FK to account, FK to chain config, unique `chain_id + address + asset` | +| OpenAPI document | each operation's success status (`201`, `202` with `Location`, `201` or `200` for ingest), the shared `ProblemDetail` errors, no simulator path under `demo`, and no validation getters in the request schemas | | Observed transactions | natural key uniqueness, invalid enum checks, non-negative checks | | First ingest | creates one observed transaction and one matching outbox event | | Duplicate ingest | creates no second transaction and no second outbox event | diff --git a/src/main/kotlin/com/example/assetsync/api/controller/AccountController.kt b/src/main/kotlin/com/example/assetsync/api/controller/AccountController.kt index ea233d5..9d351de 100644 --- a/src/main/kotlin/com/example/assetsync/api/controller/AccountController.kt +++ b/src/main/kotlin/com/example/assetsync/api/controller/AccountController.kt @@ -10,6 +10,9 @@ import com.example.assetsync.application.account.AccountApplicationService import com.example.assetsync.application.account.CreateAccountCommand import com.example.assetsync.application.account.RegisterWatchedAddressCommand import com.example.assetsync.application.account.WatchedAddressApplicationService +import io.swagger.v3.oas.annotations.headers.Header +import io.swagger.v3.oas.annotations.media.Schema +import io.swagger.v3.oas.annotations.responses.ApiResponse import jakarta.validation.Valid import java.net.URI import java.util.UUID @@ -31,6 +34,11 @@ class AccountController( ) { @PostMapping + @ApiResponse( + responseCode = "201", + description = "The account was created.", + headers = [Header(name = "Location", description = "The URL of the new account.", schema = Schema(type = "string", format = "uri"))], + ) fun createAccount( @Valid @RequestBody request: CreateAccountRequest, ): ResponseEntity { @@ -50,6 +58,7 @@ class AccountController( accountApplicationService.getAccount(accountId).toResponse() @PostMapping("/{accountId}/addresses") + @ApiResponse(responseCode = "201", description = "The watched address was registered.") fun registerWatchedAddress( @PathVariable accountId: UUID, @Valid @RequestBody request: RegisterWatchedAddressRequest, diff --git a/src/main/kotlin/com/example/assetsync/api/controller/ObservedEventController.kt b/src/main/kotlin/com/example/assetsync/api/controller/ObservedEventController.kt index 56ff3bd..f7d9bbe 100644 --- a/src/main/kotlin/com/example/assetsync/api/controller/ObservedEventController.kt +++ b/src/main/kotlin/com/example/assetsync/api/controller/ObservedEventController.kt @@ -6,6 +6,7 @@ import com.example.assetsync.api.dto.toCommand import com.example.assetsync.api.dto.toResponse import com.example.assetsync.application.transaction.ObservedEventApplicationService import com.example.assetsync.domain.model.TransitionOutcome +import io.swagger.v3.oas.annotations.responses.ApiResponse import jakarta.validation.Valid import java.security.Principal import org.springframework.http.HttpStatus @@ -27,6 +28,8 @@ class ObservedEventController( * nobody and record `rest:anonymous`. */ @PostMapping + @ApiResponse(responseCode = "201", description = "A new observed transaction was stored.") + @ApiResponse(responseCode = "200", description = "The transaction existed: its state was updated, or the event changed nothing.") fun ingestObservedEvent( @Valid @RequestBody request: IngestObservedEventRequest, principal: Principal?, diff --git a/src/main/kotlin/com/example/assetsync/api/controller/SyncController.kt b/src/main/kotlin/com/example/assetsync/api/controller/SyncController.kt index e870804..e94a56b 100644 --- a/src/main/kotlin/com/example/assetsync/api/controller/SyncController.kt +++ b/src/main/kotlin/com/example/assetsync/api/controller/SyncController.kt @@ -3,6 +3,9 @@ package com.example.assetsync.api.controller import com.example.assetsync.api.dto.SyncRunResponse import com.example.assetsync.api.dto.toResponse import com.example.assetsync.application.sync.SyncApplicationService +import io.swagger.v3.oas.annotations.headers.Header +import io.swagger.v3.oas.annotations.media.Schema +import io.swagger.v3.oas.annotations.responses.ApiResponse import java.net.URI import java.util.UUID import org.springframework.http.ResponseEntity @@ -19,6 +22,11 @@ class SyncController( ) { @PostMapping("/addresses/{addressId}/sync") + @ApiResponse( + responseCode = "202", + description = "The sync run is queued or already in flight; poll it at Location.", + headers = [Header(name = "Location", description = "The URL of the sync run.", schema = Schema(type = "string", format = "uri"))], + ) fun syncAddress( @PathVariable addressId: UUID, ): ResponseEntity { @@ -30,6 +38,11 @@ class SyncController( } @PostMapping("/accounts/{accountId}/sync") + @ApiResponse( + responseCode = "202", + description = "The sync run is queued or already in flight; poll it at Location.", + headers = [Header(name = "Location", description = "The URL of the sync run.", schema = Schema(type = "string", format = "uri"))], + ) fun syncAccount( @PathVariable accountId: UUID, ): ResponseEntity { @@ -49,3 +62,4 @@ class SyncController( private fun syncRunLocation(syncRunId: UUID): URI = URI.create("/api/v1/sync-runs/$syncRunId") } + diff --git a/src/main/kotlin/com/example/assetsync/api/dto/AccountDtos.kt b/src/main/kotlin/com/example/assetsync/api/dto/AccountDtos.kt index a9218f1..a73e7cc 100644 --- a/src/main/kotlin/com/example/assetsync/api/dto/AccountDtos.kt +++ b/src/main/kotlin/com/example/assetsync/api/dto/AccountDtos.kt @@ -3,6 +3,7 @@ package com.example.assetsync.api.dto import com.example.assetsync.application.account.Account import com.example.assetsync.application.account.WatchedAddress import com.example.assetsync.application.account.WatchedAddressStatus +import com.fasterxml.jackson.annotation.JsonIgnore import jakarta.validation.constraints.AssertTrue import jakarta.validation.constraints.NotBlank import jakarta.validation.constraints.Pattern @@ -47,6 +48,7 @@ data class UpdateWatchedAddressRequest( val status: String = "", ) { @get:AssertTrue(message = "status must be ACTIVE or DISABLED") + @get:JsonIgnore val isStatusValid: Boolean get() = status.isBlank() || WatchedAddressStatus.entries.any { it.name == status.trim() } diff --git a/src/main/kotlin/com/example/assetsync/api/dto/ObservedEventDtos.kt b/src/main/kotlin/com/example/assetsync/api/dto/ObservedEventDtos.kt index 3f83d18..086fccf 100644 --- a/src/main/kotlin/com/example/assetsync/api/dto/ObservedEventDtos.kt +++ b/src/main/kotlin/com/example/assetsync/api/dto/ObservedEventDtos.kt @@ -6,6 +6,7 @@ import com.example.assetsync.application.transaction.ObservedEventIngestionResul import com.example.assetsync.domain.model.Direction import com.example.assetsync.domain.model.TransactionStatus import com.example.assetsync.domain.policy.AmountPolicy +import com.fasterxml.jackson.annotation.JsonIgnore import jakarta.validation.constraints.AssertTrue import jakarta.validation.constraints.Min import jakarta.validation.constraints.NotBlank @@ -51,14 +52,17 @@ data class IngestObservedEventRequest( val status: String = "", ) { @get:AssertTrue(message = "amount must be a non-negative decimal string that fits numeric(38,18)") + @get:JsonIgnore val isAmountValid: Boolean get() = amount.isBlank() || parseAmountOrNull(amount) != null @get:AssertTrue(message = "direction must be INBOUND or OUTBOUND") + @get:JsonIgnore val isDirectionValid: Boolean get() = direction.isBlank() || Direction.entries.any { it.name == direction.trim() } @get:AssertTrue(message = "status must be SEEN, CONFIRMED, or REVERTED") + @get:JsonIgnore val isStatusValid: Boolean get() = status.isBlank() || TransactionStatus.entries.any { it.name == status.trim() } } diff --git a/src/main/kotlin/com/example/assetsync/config/OpenApiConfiguration.kt b/src/main/kotlin/com/example/assetsync/config/OpenApiConfiguration.kt index 6464f75..edcdd52 100644 --- a/src/main/kotlin/com/example/assetsync/config/OpenApiConfiguration.kt +++ b/src/main/kotlin/com/example/assetsync/config/OpenApiConfiguration.kt @@ -3,12 +3,29 @@ package com.example.assetsync.config import io.swagger.v3.oas.models.Components import io.swagger.v3.oas.models.OpenAPI import io.swagger.v3.oas.models.info.Info +import io.swagger.v3.oas.models.media.Content +import io.swagger.v3.oas.models.media.IntegerSchema +import io.swagger.v3.oas.models.media.MediaType +import io.swagger.v3.oas.models.media.ObjectSchema +import io.swagger.v3.oas.models.media.Schema +import io.swagger.v3.oas.models.media.StringSchema +import io.swagger.v3.oas.models.responses.ApiResponse import io.swagger.v3.oas.models.security.SecurityRequirement import io.swagger.v3.oas.models.security.SecurityScheme +import org.springdoc.core.customizers.OpenApiCustomizer import org.springframework.context.annotation.Bean import org.springframework.context.annotation.Configuration private const val BASIC_AUTH_SCHEME = "basicAuth" +private const val PROBLEM_DETAIL_SCHEMA = "ProblemDetail" + +/** The errors any API operation can answer with; the operation-specific ones are in docs/api.md. */ +private val COMMON_PROBLEMS = linkedMapOf( + "400" to "The request is invalid: malformed JSON, a field or parameter that breaks its rules, or failed validation.", + "401" to "HTTP Basic credentials are missing or wrong, in every profile except local and test.", + "403" to "The caller's role may not call this operation, in every profile except local and test.", + "503" to "The database could not serve the request.", +) @Configuration class OpenApiConfiguration { @@ -37,4 +54,36 @@ class OpenApiConfiguration { ), ) .addSecurityItem(SecurityRequirement().addList(BASIC_AUTH_SCHEME)) + + /** + * Adds the errors in [COMMON_PROBLEMS] to every operation as `application/problem+json`, and + * the ProblemDetail schema they share. The success codes come from each controller; the + * operation-specific errors (404, 409, 429, ...) stay in `docs/api.md` section 14 instead of + * being repeated here. + */ + @Bean + fun commonProblemResponses(): OpenApiCustomizer = + OpenApiCustomizer { openApi -> + openApi.components.addSchemas(PROBLEM_DETAIL_SCHEMA, problemDetailSchema()) + val content = Content().addMediaType( + org.springframework.http.MediaType.APPLICATION_PROBLEM_JSON_VALUE, + MediaType().schema(Schema().`$ref`(PROBLEM_DETAIL_SCHEMA)), + ) + openApi.paths.orEmpty().values.flatMap { it.readOperations() }.forEach { operation -> + COMMON_PROBLEMS.forEach { (status, description) -> + operation.responses.putIfAbsent(status, ApiResponse().description(description).content(content)) + } + } + } + + private fun problemDetailSchema(): Schema = + ObjectSchema() + .description("RFC 9457 problem detail. Some errors add domain properties, such as chainId.") + .addProperty("type", StringSchema().format("uri")) + .addProperty("title", StringSchema()) + .addProperty("status", IntegerSchema()) + .addProperty("detail", StringSchema()) + .addProperty("instance", StringSchema().format("uri")) + .addProperty("requestId", StringSchema()) + .additionalProperties(true) } diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index f60f051..6daa756 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -56,6 +56,9 @@ management: springdoc: # Deliberately enabled in every profile: non-local profiles put both endpoints behind HTTP Basic. + # The API only: the demo simulator stands in for the bridge and is not part of it. + paths-to-match: /api/** + default-produces-media-type: application/json api-docs: path: /v3/api-docs enabled: true diff --git a/src/test/kotlin/com/example/assetsync/integration/OpenApiDocumentIntegrationTests.kt b/src/test/kotlin/com/example/assetsync/integration/OpenApiDocumentIntegrationTests.kt new file mode 100644 index 0000000..d2555a0 --- /dev/null +++ b/src/test/kotlin/com/example/assetsync/integration/OpenApiDocumentIntegrationTests.kt @@ -0,0 +1,108 @@ +package com.example.assetsync.integration + +import com.example.assetsync.TestcontainersConfiguration +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.ObjectMapper +import java.util.Base64 +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import org.junit.jupiter.api.Test +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.context.annotation.Import +import org.springframework.http.HttpHeaders +import org.springframework.test.context.ActiveProfiles +import org.springframework.test.web.servlet.MockMvc +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status + +/** + * The OpenAPI document is what a reviewer opens first, so it must say what the API does: the + * success code each operation answers with, the ProblemDetail errors every operation shares, and + * only the fields a client sends. `demo` is the profile that also serves the chain simulator, + * which must stay out of the document. The configuration matches ChainSimulatorIntegrationTests, + * so both share one context. + */ +@ActiveProfiles("demo") +@Import(TestcontainersConfiguration::class) +@SpringBootTest( + properties = [ + "asset-sync.provider.base-url=http://localhost:1", + "asset-sync.outbox.scheduler.enabled=false", + "asset-sync.sync.recovery.enabled=false", + "asset-sync.sync.worker.enabled=false", + ], +) +@AutoConfigureMockMvc +class OpenApiDocumentIntegrationTests( + @Autowired private val mockMvc: MockMvc, + @Autowired private val objectMapper: ObjectMapper, +) { + + @Test + fun `each operation documents the status it answers with, and the simulator stays out`() { + val document = apiDocs() + val paths = document.path("paths") + + assertTrue(paths.fieldNames().asSequence().all { it.startsWith("/api/") }, "paths: ${paths.fieldNames().asSequence().toList()}") + assertEquals( + mapOf( + "POST /api/v1/accounts" to listOf("201"), + "GET /api/v1/accounts/{accountId}" to listOf("200"), + "POST /api/v1/accounts/{accountId}/addresses" to listOf("201"), + "GET /api/v1/accounts/{accountId}/addresses" to listOf("200"), + "PATCH /api/v1/addresses/{addressId}" to listOf("200"), + "POST /api/v1/addresses/{addressId}/sync" to listOf("202"), + "POST /api/v1/accounts/{accountId}/sync" to listOf("202"), + "GET /api/v1/sync-runs/{syncRunId}" to listOf("200"), + "POST /api/v1/observed-events" to listOf("200", "201"), + ), + operations(paths).mapValues { (_, operation) -> + operation.path("responses").fieldNames().asSequence().filter { it.startsWith("2") }.sorted().toList() + }, + ) + val createAccount = paths.path("/api/v1/accounts").path("post").path("responses").path("201") + assertTrue(createAccount.path("headers").has("Location"), createAccount.toString()) + assertTrue(createAccount.path("content").path("application/json").path("schema").path("\$ref").asText().endsWith("/AccountResponse"), createAccount.toString()) + val syncAddress = paths.path("/api/v1/addresses/{addressId}/sync").path("post").path("responses").path("202") + assertTrue(syncAddress.path("headers").has("Location"), syncAddress.toString()) + } + + @Test + fun `every operation shares the problem detail errors`() { + val document = apiDocs() + + assertEquals("object", document.path("components").path("schemas").path("ProblemDetail").path("type").asText()) + operations(document.path("paths")).forEach { (name, operation) -> + listOf("400", "401", "403", "503").forEach { status -> + val schema = operation.path("responses").path(status).path("content").path("application/problem+json").path("schema") + assertEquals("#/components/schemas/ProblemDetail", schema.path("\$ref").asText(), "$name $status") + } + } + } + + @Test + fun `request schemas list only the fields a client sends`() { + val schemas = apiDocs().path("components").path("schemas") + + listOf("IngestObservedEventRequest", "UpdateWatchedAddressRequest").forEach { name -> + val schema = schemas.path(name) + val fields = schema.path("properties").fieldNames().asSequence().toList() + schema.path("required").map { it.asText() } + assertTrue(fields.isNotEmpty() && fields.none { it.endsWith("Valid") }, "$name: $schema") + } + } + + private fun apiDocs(): JsonNode { + val credentials = Base64.getEncoder().encodeToString("demo-reader:demo-reader-pw".toByteArray()) + val body = mockMvc.perform(get("/v3/api-docs").header(HttpHeaders.AUTHORIZATION, "Basic $credentials")) + .andExpect(status().isOk) + .andReturn().response.contentAsString + return objectMapper.readTree(body) + } + + private fun operations(paths: JsonNode): Map = + paths.fields().asSequence().flatMap { (path, item) -> + item.fields().asSequence().map { (method, operation) -> "${method.uppercase()} $path" to operation } + }.toMap() +}