diff --git a/LIFT_HTTP4S_MIGRATION.md b/LIFT_HTTP4S_MIGRATION.md index d7d16dcccb..1e21552827 100644 --- a/LIFT_HTTP4S_MIGRATION.md +++ b/LIFT_HTTP4S_MIGRATION.md @@ -118,7 +118,7 @@ Bottom-up — each version depends on the one below it being done. | 5 | `APIMethods210` | 28 | **Done** — `Http4s210.scala`: 25 own endpoints + path-rewriting bridge to `Http4s200`; all 79 v2.1.0 tests pass | | 6 | `APIMethods220` | 19 | **Done** — `Http4s220.scala`: 18 own endpoints + path-rewriting bridge to `Http4s210`; all 27 v2.2.0 tests pass | | 7 | `APIMethods300` | 47 | **Done** — `Http4s300.scala`: 47 own endpoints + path-rewriting bridge to `Http4s220`; all 86 v3.0.0 tests pass | -| 8 | `APIMethods310` | 102 | **Done** — `Http4s310.scala` has all 100 functional endpoints (42 GET, 10 DELETE, 19 POST, 25 PUT, 1 GET-shaped revoke, 3 SCA aliases) + path-rewriting bridge to `Http4s300`; 181 v3.1.0 tests pass. `getMessageDocsSwagger` is shadowed in production by `Http4sResourceDocs.routes` but the Lift `lazy val` is intentionally kept — its deletion is caught by `FrozenClassTest` as a STABLE-API surface reduction. Both `getMessageDocsSwagger` and `getObpConnectorLoopback` retire together in the bridge-removal PR. | +| 8 | `APIMethods310` | 102 | **Done** — `Http4s310.scala` has all 100 functional endpoints (42 GET, 10 DELETE, 19 POST, 25 PUT, 1 GET-shaped revoke, 3 SCA aliases) + path-rewriting bridge to `Http4s300`; 181 v3.1.0 tests pass. The Lift `APIMethods310` trait is now a stub (live code commented out). `getObpConnectorLoopback` has a real native http4s route (always returns 400 NotImplemented). `getMessageDocsSwagger` has a `HttpRoutes.empty` stub in `Http4s310` — actual routing is owned by `Http4sResourceDocs`, the stub exists only so `nameOf(getMessageDocsSwagger)` compiles for `FrozenClassTest`. Both stubs deletable in the bridge-removal PR alongside a frozen-snapshot refresh. | | 9 | `APIMethods400` | 258 | **Done — 258 / 258 (100%)**. `Http4s400.scala` covers all 253 unique handlers (`lazy val NAME: HttpRoutes[IO]`) plus 8 ResourceDoc aliases for the transaction-request-type variants (ACCOUNT, ACCOUNT_OTP, SEPA, COUNTERPARTY, REFUND, FREE_FORM, SIMPLE, AGENT_CASH_WITHDRAWAL — handled by the shared `createTransactionRequest` wildcard handler; the `literalAllCapsSegments` set in `Http4sSupport.scala` dispatches the matcher to the per-type doc for swagger purposes). Adopts the **lazy val + helper-def init pattern** (Batches 1–19) introduced in v6 to dodge the JVM 64KB `` method-size limit. **Bridge-cascade hijack** historically threatened v4's overrides; resolved by migrating all 35/35 v4-over-older URL+verb overrides. | | 10 | `APIMethods500` | 10 | **Done** — `Http4s500.scala` (all v5.0.0 originals migrated) | | 11 | `APIMethods510` | 111 | **Done** — `Http4s510.scala`. v5.1.0's `createConsent` Lift handler is exposed in Http4s510 under the alias name `createConsentImplicit` (a single handler with `if scaMethod == "EMAIL" \|\| scaMethod == "SMS" \|\| scaMethod == "IMPLICIT"` guard covers all three SCA-method URLs). | @@ -159,7 +159,7 @@ Currently served via a raw Lift `serve { case Req(..., "openapi.yaml", ...) }` b 1. Fix aggregation bug in `getResourceDocsObpV700` → make `V7ResourceDocsAggregationTest` pass. 2. Extract shared handler logic into `Http4sResourceDocs` service; wire into `Http4sApp`. 3. Add `openapi.yaml` route to the same service. -4. Port `getMessageDocsSwagger` from `APIMethods310` into the same service (currently still served by the Lift bridge — see "Per-version Lift leftovers" below). +4. ~~Port `getMessageDocsSwagger` from `APIMethods310` into the same service~~ — **Done.** Now served by `Http4sResourceDocs.handleGetMessageDocsSwagger` via the wildcard `/obp/*/message-docs/{CONNECTOR}/swagger2.0` route matched before any per-version service. The `val getMessageDocsSwagger: HttpRoutes[IO] = HttpRoutes.empty` stub in `Http4s310.scala` exists only to satisfy the `FrozenClassTest` API-surface check. 5. Remove resource-docs from the per-version Lift objects (`ResourceDocs140`–`ResourceDocs600`) once the centralized service covers them. --- @@ -332,8 +332,8 @@ An `APIMethods{version}` file is marked **done** in the progress table when ever | Endpoint | Origin | Why on Lift | Retired by | |---|---|---|---| -| `getMessageDocsSwagger` (`GET /message-docs/CONNECTOR/swagger2.0`) | `APIMethods310` | The URL is already served by `Http4sResourceDocs.routes` (`handleGetMessageDocsSwagger`), so the Lift `lazy val` is shadowed dead code. **But** deleting it reduces v3.1.0's STABLE API surface, which `FrozenClassTest` correctly rejects (the frozen-API guard sees a `lazy val ... : OBPEndpoint` go missing from `Implementations3_1_0`). Refreshing the frozen snapshot via `FrozenClassUtil.main` is the documented way out, but doing so also requires touching `GetMessageDocsSwaggerTest`, which is below v7.0.0. Kept as-is until the bridge-removal PR retires it. | The bridge-removal PR. | -| `getObpConnectorLoopback` (`GET /connector/loopback`) | `APIMethods310` | Deprecated stub that unconditionally throws `IllegalStateException(NotImplemented)`; no functional behaviour | Either a 3-line native http4s route that throws the same exception or outright deletion, decided when the Lift bridge is removed | +| `getMessageDocsSwagger` (`GET /message-docs/CONNECTOR/swagger2.0`) | `Http4s310` (stub) + `Http4sResourceDocs` (real handler) | **Effectively done.** The real handler lives in `Http4sResourceDocs.handleGetMessageDocsSwagger`, matched by the wildcard `/obp/*/message-docs/{CONNECTOR}/swagger2.0` before any per-version service. `Http4s310.scala` keeps a stub `val getMessageDocsSwagger: HttpRoutes[IO] = HttpRoutes.empty` plus a `ResourceDoc` entry so the ResourceDoc surface stays consistent and the `FrozenClassTest` surface check keeps passing (`nameOf(getMessageDocsSwagger)` compiles). No bridge dispatch is involved. | The stub can be deleted as part of the bridge-removal PR alongside the frozen-snapshot refresh; until then the wiring above is correct in production. | +| `getObpConnectorLoopback` (`GET /connector/loopback`) | `Http4s310` | **Done.** Native http4s route in `Http4s310.scala` (~line 4875) — `booleanToFuture(NotImplemented, failCode = 400) { false }`, i.e. the route always returns 400 NotImplemented, mirroring Lift's original deprecated-stub behaviour. No bridge dispatch. | Deletable in the bridge-removal PR (or kept indefinitely as a documented deprecation stub). | | ~~`testResourceDoc`~~ | ~~`APIMethods140`~~ | Dev-mode-only `/dummy` stub deleted — returned a dummy `APIInfoJSON`, no production behaviour. Removed from `OBPAPI1_4_0.routes` and `Implementations1_4_0`. `FrozenClassTest` did not flag it because v1.4.0's `testResourceDoc` ResourceDoc was registered behind `if (Props.devMode)` — the frozen snapshot (captured in test mode) never contained it. | **Done.** | Track new leftovers here when later version files are migrated — the bridge-removal milestone in "Done Criteria" only requires the per-version files to be **done** in this table's sense (functional endpoints migrated, tests green). Leftovers folded into the Resource-docs or Auth-stack workstreams retire via those workstreams. @@ -344,6 +344,46 @@ Track new leftovers here when later version files are migrated — the bridge-re Things still on Lift that block the `Http4sLiftWebBridge` from being removed. Use this section as the master TODO for the "remove Lift Web" milestone. +### Bridge-traffic audit (data-driven prioritisation) + +Every request that reaches `Http4sLiftWebBridge.dispatch` is tallied in-memory by `Http4sLiftBridgeTraffic` so we can see exactly what still needs migrating before the bridge can be retired. + +- **First hit of any (method, path-bucket, status)** triple is logged at INFO: `[BRIDGE-AUDIT] first hit: METHOD /path/bucket STATUS (original path: /actual/path)`. Subsequent hits only increment an `AtomicLong`. +- **Snapshot endpoint** — `GET /admin/lift-bridge-traffic` returns the tally grouped into `real_work` (non-404) and `not_found` (404). 404 entries are typically test-probe traffic / stale URLs / dead links and are **not** migration work. Each group is sorted by hit count desc: + ```json + { + "unique_buckets": 5, + "total_hits": 248, + "summary": { + "real_work": {"unique_buckets": 3, "total_hits": 230}, + "not_found": {"unique_buckets": 2, "total_hits": 18} + }, + "real_work": [ + {"method": "GET", "bucket": "/auth/openid-connect/callback", "status": 200, "count": 99}, + {"method": "POST", "bucket": "/obp/dynamic-entity/FooBar", "status": 201, "count": 88}, + {"method": "GET", "bucket": "/obp/dynamic-entity/FooBar", "status": 200, "count": 43} + ], + "not_found": [ + {"method": "DELETE", "bucket": "/obp/v4.0.0/banks/{id}/accounts", "status": 404, "count": 16}, + {"method": "GET", "bucket": "/favicon.ico", "status": 404, "count": 2} + ] + } + ``` +- **Reset** — `POST /admin/lift-bridge-traffic/reset` clears the tally (handy for taking a baseline before a load test). +- **Path normalisation** collapses opaque IDs so the map doesn't fill up: UUIDs → `{uuid}`, all-digits → `{n}`, anything with a dot or 12+-char alnum mixed → `{id}`. API-version strings (`v6.0.0`, `v1_2_1`) are kept verbatim. Unit-tested in `Http4sLiftBridgeTrafficTest` (9 cases). + +Operator playbook for "is the bridge ready to retire?": +1. Reset the tally on a representative instance. +2. Let it run through a normal traffic window (e.g. 24h + the daily/weekly jobs). +3. Query `/admin/lift-bridge-traffic`: + - **`real_work[]` empty** → bridge can be retired (modulo any documented leftovers). + - **`real_work[]` non-empty** → those buckets are concrete migration targets. Each entry is a (method, URL pattern) that some live caller still needs Lift to serve. + - **`not_found[]`** is informational — useful for spotting stale callers or unused URL patterns, but not blocking bridge removal. + +First real audit data (shard 1 CI run on 2026-05-25, 515 tests): +- 20 `real_work` entries — all the `/obp/dynamic-entity/...` and `/obp/dynamic-endpoint/...` URLs. These are runtime-generated by Lift's dynamic dispatch when an admin creates a dynamic entity / endpoint at runtime; porting them is a workstream of its own (not endpoint-by-endpoint). +- 2 `not_found` entries — `DELETE /obp/v4.0.0/banks/{id}/accounts`, asserted as 404 by `DeleteBankCascadeTest` to verify the cascade actually wiped the bank. + ### Auth stack — every handler is its own `RestHelper` | Handler | File | Routes | Status | @@ -364,7 +404,7 @@ Already partly described in the next major section, but counted here for complet - `ResourceDocs140` … `ResourceDocs600` — six separate Lift files, each registered via `LiftRules.statelessDispatch.append` in `Boot.scala`. - `getResourceDocsObpV700` aggregation bug fix — landed (`V7ResourceDocsAggregationTest` passes). - `openapi.yaml` route — raw `Lift serve { ... }` block, no native http4s handler. -- `getMessageDocsSwagger` (v3.1.0) — folds into the centralised `Http4sResourceDocs` service when it ships. +- ~~`getMessageDocsSwagger` (v3.1.0) — folds into the centralised `Http4sResourceDocs` service when it ships.~~ **Done** — served by `Http4sResourceDocs.handleGetMessageDocsSwagger` via the wildcard `/obp/*/message-docs/{CONNECTOR}/swagger2.0` route. - One-PR opportunity: build `Http4sResourceDocs` above the Lift bridge in `Http4sApp`, intercept all `/obp/*/resource-docs/*` traffic, retire six Lift dispatch entries in a single change. ### Small singleton Lift endpoints @@ -540,7 +580,7 @@ Binds to `hostname` / `dev.port` from your props file (defaults: `127.0.0.1:8080 | `APIMethods210` | done — `Http4s210.scala` (25 own endpoints; path-rewriting bridge to Http4s200) | | `APIMethods220` | done — `Http4s220.scala` (18 own endpoints; path-rewriting bridge to Http4s210) | | `APIMethods300` | done — `Http4s300.scala` (47 own endpoints; path-rewriting bridge to Http4s220; all 86 v3.0.0 tests pass) | -| `APIMethods310` | done — `Http4s310.scala` (100 own endpoints + `updateCustomerAddress`; path-rewriting bridge to Http4s300; 2 endpoints still on Lift: `getMessageDocsSwagger` (shadowed by `Http4sResourceDocs.routes`, kept to satisfy `FrozenClassTest`) and `getObpConnectorLoopback`) | +| `APIMethods310` | done — `Http4s310.scala` (100 own endpoints + `updateCustomerAddress`; path-rewriting bridge to Http4s300). Two former Lift leftovers now both off-bridge: `getMessageDocsSwagger` served by `Http4sResourceDocs` (in-file stub kept only for `FrozenClassTest`), `getObpConnectorLoopback` served by a native http4s route that returns 400 NotImplemented. | | `APIMethods400` | **done — 258 / 258 (100%)**. `Http4s400.scala` covers all 253 unique handlers + 8 ResourceDoc aliases for transaction-request-type variants (served by the shared wildcard handler). | | `APIMethods500` | done — `Http4s500.scala` (all 10 v5.0.0 originals on http4s) | | `APIMethods510` | done — `Http4s510.scala` (all 111 v5.1.0 originals on http4s; `createConsent` exposed as `createConsentImplicit` with a guard covering EMAIL/SMS/IMPLICIT SCA methods) | diff --git a/obp-api/src/main/scala/code/api/util/http4s/Http4sApp.scala b/obp-api/src/main/scala/code/api/util/http4s/Http4sApp.scala index 24bddefe92..14092720a8 100644 --- a/obp-api/src/main/scala/code/api/util/http4s/Http4sApp.scala +++ b/obp-api/src/main/scala/code/api/util/http4s/Http4sApp.scala @@ -106,6 +106,12 @@ object Http4sApp { corsHandler.run(req) .orElse(AppsPage.routes.run(req)) .orElse(StatusPage.routes.run(req)) + // Bridge-retirement audit endpoint — exposes the in-memory tally of + // requests that have fallen through to Http4sLiftWebBridge so we can + // see exactly what still needs migrating before the bridge can be + // removed. Placed before the per-version routes so the admin path + // can't be shadowed by a version-prefixed handler. + .orElse(Http4sLiftBridgeTraffic.routes.run(req)) .orElse(Http4sResourceDocs.routes.run(req)) .orElse(v510Routes.run(req)) .orElse(v600Routes.run(req)) diff --git a/obp-api/src/main/scala/code/api/util/http4s/Http4sLiftWebBridge.scala b/obp-api/src/main/scala/code/api/util/http4s/Http4sLiftWebBridge.scala index 989ff3b57b..2abdcbaf08 100644 --- a/obp-api/src/main/scala/code/api/util/http4s/Http4sLiftWebBridge.scala +++ b/obp-api/src/main/scala/code/api/util/http4s/Http4sLiftWebBridge.scala @@ -11,15 +11,166 @@ import net.liftweb.common._ import net.liftweb.http._ import net.liftweb.http.provider._ import org.http4s._ +import org.http4s.dsl.io._ +import org.http4s.headers.`Content-Type` import org.typelevel.ci.CIString import java.io.{ByteArrayInputStream, ByteArrayOutputStream, InputStream} import java.time.format.DateTimeFormatter import java.time.{ZoneOffset, ZonedDateTime} import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.atomic.AtomicLong import java.util.{Locale, UUID} import scala.collection.JavaConverters._ +/** + * In-memory tally of which requests reach the Lift fallback bridge. + * + * Goal: data-driven prioritisation of remaining migration work. Every request + * that lands here means no http4s handler claimed it. Once an audit run shows + * which (method, path-bucket) pairs still hit the bridge, those buckets become + * the next migration targets. When the map is empty for a representative + * traffic window, the bridge can be retired. + * + * Path-bucket normalisation collapses common ID segments (long opaque tokens, + * UUIDs, numbers, version segments) so we don't fill the map with one entry + * per real-world ID value. The exact form of each bucket is logged the first + * time it is observed. + */ +object Http4sLiftBridgeTraffic extends MdcLoggable { + private val counts: ConcurrentHashMap[String, AtomicLong] = new ConcurrentHashMap() + private val UUID_RE = "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$".r + private val VERSION_RE = "^v\\d+(_\\d+){2}$|^v\\d+(\\.\\d+){2}$".r + private val NUMERIC_RE = "^\\d+$".r + + /** Buckets that look like opaque IDs: + * 1. Plain UUID → {uuid} + * 2. All-digits → {n} + * 3. Contains a `.` (e.g. `gh.29.uk`, `some.bank.io`, `127.0.0.1`) → {id} + * (API-version strings like `v6.0.0` are caught earlier and kept.) + * 4. Long-ish (≥ 12 chars) AND contains both letters & digits → {id} + * + * Keeps short path keywords like `openid-connect`, `callback-1`, + * `BANK_ID`, `accounts`, `views`. + */ + private def bucketSegment(seg: String): String = { + if (seg.isEmpty) return seg + if (VERSION_RE.findFirstIn(seg).isDefined) return seg + if (UUID_RE.findFirstIn(seg).isDefined) return "{uuid}" + if (NUMERIC_RE.findFirstIn(seg).isDefined) return "{n}" + if (seg.contains('.')) return "{id}" + val hasDigit = seg.exists(_.isDigit) + val hasLetter = seg.exists(_.isLetter) + if (seg.length >= 12 && hasDigit && hasLetter) return "{id}" + seg + } + + def bucket(path: String): String = + "/" + path.split('/').filter(_.nonEmpty).map(bucketSegment).mkString("/") + + /** Record one bridge dispatch with its outcome. Key is `METHOD BUCKET STATUS` + * so the snapshot can distinguish: + * - real Lift handler work (2xx/3xx/5xx) — actual migration targets + * - 404 fall-throughs — test code probing for non-existent endpoints, or + * stale callers; not migration work + */ + def observe(method: String, path: String, status: Int): Unit = { + val key = s"$method ${bucket(path)} $status" + val prev = counts.putIfAbsent(key, new AtomicLong(1L)) + if (prev == null) { + logger.info(s"[BRIDGE-AUDIT] first hit: $key (original path: $path)") + } else { + prev.incrementAndGet() + } + } + + /** Snapshot of (`METHOD BUCKET STATUS` → hit-count). */ + def snapshot(): Map[String, Long] = + counts.asScala.toMap.map { case (k, v) => k -> v.get() } + + /** Wipe the in-memory tally. Mostly for tests / a manual reset after a baseline. */ + def reset(): Unit = counts.clear() + + /** Split the key string `METHOD BUCKET STATUS` back into its parts. */ + private def splitKey(key: String): (String, String, Int) = { + // method is everything before the first space; status is the trailing int; + // bucket is the middle. + val firstSp = key.indexOf(' ') + val lastSp = key.lastIndexOf(' ') + if (firstSp <= 0 || lastSp <= firstSp) ("?", key, 0) + else { + val method = key.substring(0, firstSp) + val statusStr = key.substring(lastSp + 1) + val status = try statusStr.toInt catch { case _: Throwable => 0 } + val bucketStr = key.substring(firstSp + 1, lastSp) + (method, bucketStr, status) + } + } + + /** Admin route: `GET /admin/lift-bridge-traffic` returns the snapshot as JSON, + * grouped by `real_work` (non-404) vs `not_found` (404s — test probes / + * stale URLs / dead links). Entries inside each group are sorted by hit + * count desc. + * + * Wired into Http4sApp's baseServices ahead of the per-version routes so + * the admin path can't be shadowed. + */ + val routes: HttpRoutes[IO] = HttpRoutes.of[IO] { + case GET -> Root / "admin" / "lift-bridge-traffic" => + val rows = snapshot().toList.map { case (k, n) => + val (m, b, s) = splitKey(k) + (m, b, s, n) + } + val (notFound, realWork) = rows.partition { case (_, _, s, _) => s == 404 } + def renderGroup(group: List[(String, String, Int, Long)]): String = + group.sortBy { case (_, _, _, n) => -n }.map { case (m, b, s, n) => + s""" {"method": ${jsonString(m)}, "bucket": ${jsonString(b)}, "status": $s, "count": $n}""" + }.mkString(",\n") + val totalUnique = rows.length + val totalHits = rows.map(_._4).sum + val realHits = realWork.map(_._4).sum + val notFoundHits = notFound.map(_._4).sum + val body = + s"""{ + | "unique_buckets": $totalUnique, + | "total_hits": $totalHits, + | "summary": { + | "real_work": {"unique_buckets": ${realWork.length}, "total_hits": $realHits}, + | "not_found": {"unique_buckets": ${notFound.length}, "total_hits": $notFoundHits} + | }, + | "real_work": [ + |${renderGroup(realWork)} + | ], + | "not_found": [ + |${renderGroup(notFound)} + | ] + |} + |""".stripMargin + IO.pure(Response[IO]() + .withEntity(body.getBytes("UTF-8")) + .withHeaders(Headers(`Content-Type`(MediaType.application.json, Charset.`UTF-8`)))) + + case POST -> Root / "admin" / "lift-bridge-traffic" / "reset" => + reset() + IO.pure(Response[IO]() + .withEntity("""{"status":"reset"}""".getBytes("UTF-8")) + .withHeaders(Headers(`Content-Type`(MediaType.application.json, Charset.`UTF-8`)))) + } + + private def jsonString(s: String): String = { + val esc = s.flatMap { + case '"' => "\\\"" + case '\\' => "\\\\" + case '\n' => "\\n" + case '\r' => "\\r" + case '\t' => "\\t" + case c if c < 0x20 => f"\\u${c.toInt}%04x" + case c => c.toString + } + s""""$esc"""" + } +} + object Http4sLiftWebBridge extends MdcLoggable { type HttpF[A] = OptionT[IO, A] @@ -67,6 +218,7 @@ object Http4sLiftWebBridge extends MdcLoggable { val (effectiveReq, servedVersion) = rewriteIfV700(req) val uri = req.uri.renderString val method = req.method.name + val originalPath = req.uri.path.renderString logger.debug(s"Http4sLiftBridge dispatching: $method $uri, S.inStatefulScope_? = ${S.inStatefulScope_?}") val result = for { bodyBytes <- effectiveReq.body.compile.to(Array) @@ -99,11 +251,16 @@ object Http4sLiftWebBridge extends MdcLoggable { logger.debug(s"[BRIDGE] Http4sLiftBridge completed: $method $uri -> ${http4sResponse.status.code}") logger.debug(s"Http4sLiftBridge completed: $method $uri -> ${http4sResponse.status.code}") val baseResp = ensureStandardHeaders(req, http4sResponse) - servedVersion.fold(baseResp)(v => baseResp.putHeaders(Header.Raw(CIString("X-OBP-Version-Served"), v))) + val finalResp = servedVersion.fold(baseResp)(v => baseResp.putHeaders(Header.Raw(CIString("X-OBP-Version-Served"), v))) + // Tally with the response status now known. 404s tell us "test probes / + // stale URLs"; non-404s are real Lift work still owned by the bridge. + Http4sLiftBridgeTraffic.observe(method, originalPath, finalResp.status.code) + finalResp } result.handleErrorWith { e => logger.error(s"[BRIDGE] Uncaught exception in dispatch: $method $uri - ${e.getMessage}", e) val errorBody = s"""{"error":"Internal Server Error","message":"${e.getMessage}"}""" + Http4sLiftBridgeTraffic.observe(method, originalPath, 500) IO.pure(ensureStandardHeaders(req, Response[IO]( status = org.http4s.Status.InternalServerError ).withEntity(errorBody.getBytes("UTF-8")) diff --git a/obp-api/src/main/scala/code/api/util/http4s/Http4sResourceDocs.scala b/obp-api/src/main/scala/code/api/util/http4s/Http4sResourceDocs.scala index 8bde7e3dfd..1372b03613 100644 --- a/obp-api/src/main/scala/code/api/util/http4s/Http4sResourceDocs.scala +++ b/obp-api/src/main/scala/code/api/util/http4s/Http4sResourceDocs.scala @@ -690,13 +690,21 @@ object Http4sResourceDocs extends MdcLoggable { case req @ GET -> Root / "obp" / prefix / "resource-docs" / requestedApiVersionString / "swagger" => handleGetResourceDocsSwagger(req, prefix, requestedApiVersionString) - // OpenAPI 3.1 JSON was only registered by ResourceDocs600 (v6.0.0 prefix); guard the - // route so requests under other prefixes still 404-fall-through, matching old behaviour. - case req @ GET -> Root / "obp" / prefix / "resource-docs" / requestedApiVersionString / "openapi" if prefix == "v6.0.0" => + // OpenAPI 3.1 JSON and YAML — served for every URL prefix. + // + // Historically these routes were only registered by ResourceDocs600 (v6.0.0 + // prefix). With the centralised service, generating the OpenAPI spec only + // depends on the requested-API-version path segment (`requestedApiVersionString`), + // not on the URL prefix the client used to reach the service — so guarding + // on a single prefix added no value and surprised callers that hit e.g. + // `/obp/v5.1.0/resource-docs/v5.1.0/openapi` and got a Lift 404 fall-through. + // The handlers use `implForPrefix(prefix)` which falls back to `ImplDefault` + // for non-v6 prefixes; `isVersion4OrHigher` is hardcoded `true` inside the + // handlers because the OpenAPI converter always consumes the v4-shape input. + case req @ GET -> Root / "obp" / prefix / "resource-docs" / requestedApiVersionString / "openapi" => handleGetResourceDocsOpenAPI31(req, prefix, requestedApiVersionString) - // openapi.yaml was likewise only on v6.0.0. - case req @ GET -> Root / "obp" / prefix / "resource-docs" / requestedApiVersionString / "openapi.yaml" if prefix == "v6.0.0" => + case req @ GET -> Root / "obp" / prefix / "resource-docs" / requestedApiVersionString / "openapi.yaml" => handleGetResourceDocsOpenAPI31Yaml(req, prefix, requestedApiVersionString) case req @ GET -> Root / "obp" / prefix / "banks" / bankIdStr / "resource-docs" / requestedApiVersionString / "obp" => diff --git a/obp-api/src/main/scala/code/api/v1_2_1/Http4s121.scala b/obp-api/src/main/scala/code/api/v1_2_1/Http4s121.scala index bde8257772..3e7f990db1 100644 --- a/obp-api/src/main/scala/code/api/v1_2_1/Http4s121.scala +++ b/obp-api/src/main/scala/code/api/v1_2_1/Http4s121.scala @@ -1315,7 +1315,10 @@ object Http4s121 { resourceDocs += ResourceDoc( null, implementedInApiVersion, nameOf(addCounterpartyMoreInfo), "POST", "/banks/BANK_ID/accounts/ACCOUNT_ID/VIEW_ID/other_accounts/OTHER_ACCOUNT_ID/metadata/more_info", - "Add Counterparty More Info", "Add a description of the counter party from the perpestive of the account e.g. My dentist", + "Add Counterparty More Info", + // Intentional drift from Lift's APIMethods121.scala source-of-truth: + // typo fixes "counter party" → "counterparty" and "perpestive" → "perspective". + "Add a description of the counterparty from the perspective of the account e.g. My dentist", moreInfoJSON, successMessage, List( AuthenticatedUserIsRequired, @@ -1350,7 +1353,10 @@ object Http4s121 { resourceDocs += ResourceDoc( null, implementedInApiVersion, nameOf(updateCounterpartyMoreInfo), "PUT", "/banks/BANK_ID/accounts/ACCOUNT_ID/VIEW_ID/other_accounts/OTHER_ACCOUNT_ID/metadata/more_info", - "Update Counterparty More Info", "Update the more info description of the counter party from the perpestive of the account e.g. My dentist", + "Update Counterparty More Info", + // Intentional drift from Lift's APIMethods121.scala source-of-truth: + // typo fixes "counter party" → "counterparty" and "perpestive" → "perspective". + "Update the more info description of the counterparty from the perspective of the account e.g. My dentist", moreInfoJSON, successMessage, List(AuthenticatedUserIsRequired, BankAccountNotFound, InvalidJsonFormat, "the view does not allow metadata access", "the view does not allow updating more info", "More Info cannot be updated", UnknownError), List(apiTagCounterpartyMetaData, apiTagCounterparty), diff --git a/obp-api/src/test/scala/code/api/ResourceDocs1_4_0/SwaggerDocsTest.scala b/obp-api/src/test/scala/code/api/ResourceDocs1_4_0/SwaggerDocsTest.scala index 1412bf74c3..d015851682 100644 --- a/obp-api/src/test/scala/code/api/ResourceDocs1_4_0/SwaggerDocsTest.scala +++ b/obp-api/src/test/scala/code/api/ResourceDocs1_4_0/SwaggerDocsTest.scala @@ -230,6 +230,26 @@ class SwaggerDocsTest extends ResourceDocsV140ServerSetup with PropsReset with D responseGetOpenAPIYAML.body.toString.trim.nonEmpty should be (true) } + // The OpenAPI routes used to be gated on `prefix == "v6.0.0"` by the + // centralised Http4sResourceDocs service — replicating the historical Lift + // setup where only ResourceDocs600 registered them. The gate was removed + // because the spec content only depends on the API-version path segment, + // not on the URL prefix. Verify a non-v6 prefix is now served. + scenario("OpenAPI JSON - served for non-v6.0.0 URL prefix (v5.1.0)", ApiEndpoint1, VersionOfApi) { + setPropsValues("resource_docs_requires_role" -> "false") + val req = (ResourceDocsV5_1Request / "resource-docs" / "v5.1.0" / "openapi").GET < "false") + val req = (ResourceDocsV5_1Request / "resource-docs" / "v5.1.0" / "openapi.yaml").GET <