From 3a36aa014524e02d56e49e73bd12b8cacadc5f1c Mon Sep 17 00:00:00 2001 From: simonredfern Date: Tue, 26 May 2026 16:37:14 +0200 Subject: [PATCH 1/2] Update .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index bae954efd8..7aed6a8dd6 100644 --- a/.gitignore +++ b/.gitignore @@ -49,3 +49,4 @@ obp-http4s-runner/src/main/resources/git.properties test-results untracked_files/ dependency-reduced-pom.xml +_DO_NOT_COMMIT_/ From 24dec0797b80510f49e06362fe0fd71465baccd5 Mon Sep 17 00:00:00 2001 From: simonredfern Date: Mon, 1 Jun 2026 21:12:49 +0200 Subject: [PATCH 2/2] jsonContentType --- CLAUDE.md | 4 ++-- LIFT_HTTP4S_MIGRATION.md | 6 ++--- .../code/api/util/http4s/Http4sSupport.scala | 22 ++++++++++++------- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a8ece6c322..1f4559e535 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -296,7 +296,7 @@ At the integration level both frameworks are similarly server/DB-bound (~0.32– The 6 integration suites (pre-merge timings; Http4s700RoutesTest is currently 102 scenarios): - `obp-api/src/test/scala/code/api/http4sbridge/Http4sLiftBridgePropertyTest.scala` — 51 tests, 31.9s - `obp-api/src/test/scala/code/api/v7_0_0/Http4s700RoutesTest.scala` — 102 tests (was 75 pre-merge, 23.8s) -- `obp-api/src/test/scala/code/api/v7_0_0/V7ResourceDocsAggregationTest.scala` — intentionally failing until resource-docs aggregation bug is fixed +- `obp-api/src/test/scala/code/api/v7_0_0/V7ResourceDocsAggregationTest.scala` — was intentionally failing; aggregation bug fixed in `efb97531e` (2026-05-19) and the suite now passes. Kept in place as a regression guard — the `>= 500 docs`, version-mix, dedup, and `specifiedUrl` assertions still encode the contract. - `obp-api/src/test/scala/code/api/http4sbridge/Http4sServerIntegrationTest.scala` — 16 tests, 5.0s - `obp-api/src/test/scala/code/api/v5_0_0/Http4s500SystemViewsTest.scala` — 13 tests, 4.4s - `obp-api/src/test/scala/code/api/v7_0_0/Http4s700TransactionTest.scala` — 5 tests, 1.9s @@ -362,5 +362,5 @@ Architectural note from the v6 migration: around the 140-endpoint mark `Implemen - **OBP-Trading**: trading endpoints (createTradingOffer, getTradingOffer, getTradingOffers, cancelTradingOffer, createMarketOrder, getMarketOrder, cancelMarketOrder, createMarketMatch, getMarketTrade, requestSettlement, requestWithdrawal) are now in `Http4s700.scala`. 5 payment-auth endpoints remain commented out (notifyDeposit, createPaymentAuth, capturePaymentAuth, releasePaymentAuth, getPaymentAuth) — see `ideas/CAPTURE_RELEASE_TRANSACTION_REQUEST_TYPES.md`. - **CI speed-up** (not done): two-tier fast gate + full suite; surefire parallel forks. - **Disabled tests to fix**: `Http4s500RoutesTest` (@Ignore, in-process issue), `RootAndBanksTest` (@Ignore), `V500ContractParityTest` (@Ignore), `CardTest` (fully commented out). `v5_0_0`: 13 skipped tests (setup cost paid, no value). -- **`V7ResourceDocsAggregationTest`**: intentionally failing — encodes the fix for the resource-docs aggregation bug (v7 endpoint returns only ~10 own docs instead of 500+ aggregated). Fix the bug to make this suite pass. +- **`V7ResourceDocsAggregationTest`**: fixed in `efb97531e` (2026-05-19) — *"fix(resource-docs): correct v7 aggregation specifiedUrl and remove shadowed v7 handler"*. Two root causes: (1) `ResourceDocs1_4_0` registered the same `(GET, /resource-docs/API_VERSION/obp)` doc twice (getResourceDocsObp + getResourceDocsObpV400), so v7 aggregation surfaced a duplicate; (2) `getAllResourceDocsObpCached` froze the first caller's `specifiedUrl` for dynamic-endpoint docs (`case Some(_) => it`), poisoning every later request. Now serves as a regression guard — `>= 500 resource_docs`, mixed-version discovery (OBPv1.2.1 through OBPv7.0.0), dedup, and per-request `specifiedUrl` recompute are all still asserted. Live server returns ~949 docs. - **Flaky `MakerCheckerTransactionRequestTest` — TTL/proxy connection race in v4 createTransactionRequest** (pre-existing, predates the auth-stack migration). Scenario *"Multiple challenges with maker-checker: different users answer their own challenges"* (`MakerCheckerTransactionRequestTest.scala:246`) fails ~40% of local runs and was observed once in CI shard1. Diagnosed root cause: inside one HTTP request, `LocalMappedConnector.createTransactionRequestv210` writes N rows to `MappedExpectedChallengeAnswer` via the request-scoped proxy connection (auto-commit=false, request-end commit) and then reads them back via `getChallengesByTransactionRequestId`. When `RequestScopeConnection.currentProxy` (a `TransmittableThreadLocal`) fails to propagate to the read `Future`'s worker thread, `RequestAwareConnectionManager.newConnection` returns `null` → falls back to a fresh pool connection (autocommit=true) that cannot see the proxy connection's uncommitted writes → read returns 0 rows. Diagnostic confirmed: in failing runs, `createChallengesC2` is called with the correct 2 userIds, but `MappedExpectedChallengeAnswer.findAll()` (no WHERE clause) returns 0 rows — i.e. the entire table is empty from the read connection's view. Only the multi-user path (`REQUIRED_CHALLENGE_ANSWERS > 1`) hits this because it adds an extra synchronous `Views.views.vend.permissions(...)` inside `getAccountAttributesByAccount.map` that shifts the Future-scheduling timing. The other 3 scenarios in the file always pass because they take the default `REQUIRED_CHALLENGE_ANSWERS=1` shortcut. **Fix direction:** every DB-touching `Future { ... }` inside the connector chain needs to go through `RequestScopeConnection.fromFuture` (which atomically sets+submits+clears the TTL inside `IO.defer`) instead of being raw Scala `Future { ... }` chained via `flatMap`. Alternatively: stop relying on TTL and pass the proxy connection explicitly down the connector call-chain (bigger change, but eliminates the race class entirely). diff --git a/LIFT_HTTP4S_MIGRATION.md b/LIFT_HTTP4S_MIGRATION.md index cf4653b990..0a94a5505a 100644 --- a/LIFT_HTTP4S_MIGRATION.md +++ b/LIFT_HTTP4S_MIGRATION.md @@ -159,9 +159,9 @@ GET /obp/*/message-docs/CONNECTOR/swagger2.0 → absorbs APIMethods310.ge The wildcard prefix means all resource-doc requests are intercepted regardless of which version prefix the client uses. This workstream is **independent of the per-version migration order** — it can land at any time and immediately removes all resource-docs traffic from the Lift bridge. -### Prerequisite: fix the aggregation bug +### ~~Prerequisite~~ Prerequisite (done): aggregation bug fix -`V7ResourceDocsAggregationTest` is intentionally failing. The current `getResourceDocsObpV700` has a broken branch for `requestedApiVersion == v7.0.0` that manually iterates `allResourceDocs` (~45 own docs) instead of calling `getResourceDocsList`, which aggregates all 500+. Fix this first — it is the same defect the centralized service must not repeat. +~~`V7ResourceDocsAggregationTest` is intentionally failing.~~ **Fixed in `efb97531e` (2026-05-19)** — *"fix(resource-docs): correct v7 aggregation specifiedUrl and remove shadowed v7 handler"*. Two root causes addressed: (1) `ResourceDocs1_4_0` registered the same `(GET, /resource-docs/API_VERSION/obp)` doc twice, so v7 aggregation surfaced a duplicate; (2) `getAllResourceDocsObpCached` cached `specifiedUrl` per dynamic-endpoint doc with `case Some(_) => it`, so the first caller froze the URL and every later request inherited it. `getResourceDocsObpV700` now calls `getResourceDocsList`, which aggregates the full cascade (~949 docs on a live server). The centralized service must preserve this contract — `V7ResourceDocsAggregationTest` now acts as the regression guard. ### `openapi.yaml` @@ -173,7 +173,7 @@ Currently served via a raw Lift `serve { case Req(..., "openapi.yaml", ...) }` b ### Steps -1. Fix aggregation bug in `getResourceDocsObpV700` → make `V7ResourceDocsAggregationTest` pass. +1. ~~Fix aggregation bug in `getResourceDocsObpV700` → make `V7ResourceDocsAggregationTest` pass.~~ **Done** in `efb97531e` (2026-05-19). See the Prerequisite section above. 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~~ — **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. diff --git a/obp-api/src/main/scala/code/api/util/http4s/Http4sSupport.scala b/obp-api/src/main/scala/code/api/util/http4s/Http4sSupport.scala index 01faf73c89..c2455e3d91 100644 --- a/obp-api/src/main/scala/code/api/util/http4s/Http4sSupport.scala +++ b/obp-api/src/main/scala/code/api/util/http4s/Http4sSupport.scala @@ -10,6 +10,7 @@ import net.liftweb.common.{Box, Empty, Full} import net.liftweb.http.provider.HTTPParam import org.http4s._ import org.http4s.dsl.io._ +import org.http4s.headers.`Content-Type` import org.typelevel.ci.CIString import org.typelevel.vault.Key @@ -104,9 +105,14 @@ object Http4sRequestAttributes { import net.liftweb.json.JsonAST.prettyRender import net.liftweb.json.{Extraction, Formats} + // OBP responses are always JSON. Passing a raw String to http4s' Ok/Created uses the + // default EntityEncoder[String], which sets `Content-Type: text/plain`. Override it so + // clients (and the strict application/json check in the frontend) see the real type. + private val jsonContentType = `Content-Type`(MediaType.application.json) + private def toJsonOk[A](result: A)(implicit formats: Formats): IO[Response[IO]] = { val jsonString = prettyRender(Extraction.decompose(result)) - Ok(jsonString) + Ok(jsonString, jsonContentType) } /** @@ -227,7 +233,7 @@ object Http4sRequestAttributes { RequestScopeConnection.fromFuture(f(body, cc)).attempt.flatMap { case Right(result) => val jsonString = prettyRender(Extraction.decompose(result)) - Created(jsonString).flatTap(recordMetric(result, _)) + Created(jsonString, jsonContentType).flatTap(recordMetric(result, _)) case Left(err) => ErrorResponseConverter.toHttp4sResponse(err, cc).flatTap(recordMetric(err.getMessage, _)) } } @@ -269,7 +275,7 @@ object Http4sRequestAttributes { io.attempt.flatMap { case Right(result) => val jsonString = prettyRender(Extraction.decompose(result)) - Created(jsonString).flatTap(recordMetric(result, _)) + Created(jsonString, jsonContentType).flatTap(recordMetric(result, _)) case Left(err) => ErrorResponseConverter.toHttp4sResponse(err, cc).flatTap(recordMetric(err.getMessage, _)) } } @@ -313,7 +319,7 @@ object Http4sRequestAttributes { io.attempt.flatMap { case Right(result) => val jsonString = prettyRender(Extraction.decompose(result)) - Created(jsonString).flatTap(recordMetric(result, _)) + Created(jsonString, jsonContentType).flatTap(recordMetric(result, _)) case Left(err) => ErrorResponseConverter.toHttp4sResponse(err, cc).flatTap(recordMetric(err.getMessage, _)) } } @@ -351,7 +357,7 @@ object Http4sRequestAttributes { io.attempt.flatMap { case Right(result) => val jsonString = prettyRender(Extraction.decompose(result)) - Created(jsonString).flatTap(recordMetric(result, _)) + Created(jsonString, jsonContentType).flatTap(recordMetric(result, _)) case Left(err) => ErrorResponseConverter.toHttp4sResponse(err, cc).flatTap(recordMetric(err.getMessage, _)) } } @@ -374,7 +380,7 @@ object Http4sRequestAttributes { io.attempt.flatMap { case Right(result) => val jsonString = prettyRender(Extraction.decompose(result)) - Created(jsonString).flatTap(recordMetric(result, _)) + Created(jsonString, jsonContentType).flatTap(recordMetric(result, _)) case Left(err) => ErrorResponseConverter.toHttp4sResponse(err, cc).flatTap(recordMetric(err.getMessage, _)) } } @@ -439,7 +445,7 @@ object Http4sRequestAttributes { RequestScopeConnection.fromFuture(f).attempt.flatMap { case Right(result) => val jsonString = prettyRender(Extraction.decompose(result)) - Created(jsonString).flatTap(recordMetric(result, _)) + Created(jsonString, jsonContentType).flatTap(recordMetric(result, _)) case Left(err) => ErrorResponseConverter.toHttp4sResponse(err, cc).flatTap(recordMetric(err.getMessage, _)) } } @@ -460,7 +466,7 @@ object Http4sRequestAttributes { case Right((result, code)) => val jsonString = prettyRender(Extraction.decompose(result)) val status = Status.fromInt(code).getOrElse(Status.Ok) - IO.pure(Response[IO](status).withEntity(jsonString)).flatTap(recordMetric(result, _)) + IO.pure(Response[IO](status).withEntity(jsonString).withContentType(jsonContentType)).flatTap(recordMetric(result, _)) case Left(err) => ErrorResponseConverter.toHttp4sResponse(err, cc).flatTap(recordMetric(err.getMessage, _)) } }