Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
569d99e
refactor: drive Http4s700RoutesTest in-process (no TCP), add CORS sce…
constantine2nd Apr 21, 2026
2b121a6
Merge remote-tracking branch 'upstream/develop' into develop
constantine2nd May 5, 2026
c27ef2b
docs: update CLAUDE.md after upstream/develop merge
constantine2nd May 5, 2026
16e2935
docs: add MIGRATION.md with full Lift→http4s plan; reference it from …
constantine2nd May 5, 2026
efbb403
docs: Add Lift → http4s Migration
constantine2nd May 6, 2026
e1c5793
refactor: Remove getCards and getCardsForBank from Http4s700.scala — …
constantine2nd May 6, 2026
63af52a
fix: resource-docs aggregation in getResourceDocsObpV700 — delegate t…
constantine2nd May 6, 2026
296654e
feat: migrate OBP v1.2.1 endpoints to http4s (Http4s121.scala)
constantine2nd May 7, 2026
cc1c5d0
feat: migrate v1.3.0 to http4s — Http4s130.scala
constantine2nd May 7, 2026
14813ee
feat: migrate v1.4.0 to http4s — Http4s140.scala
constantine2nd May 8, 2026
521823f
feat: migrate v2.0.0 to http4s — Http4s200.scala
constantine2nd May 8, 2026
106b0a0
fix: two regressions in Http4s200 / ResourceDocMiddleware
constantine2nd May 8, 2026
01efd7f
fix: use anonymousAccess in ResourceDocMiddleware to preserve Failure…
constantine2nd May 8, 2026
e1214aa
fix: parse exception JSON in ResourceDocMiddleware to recover 400 for…
constantine2nd May 8, 2026
a8c9a19
feat: migrate v2.1.0 to http4s — Http4s210.scala
constantine2nd May 9, 2026
822462b
feat: migrate v2.2.0 to http4s — Http4s220.scala
constantine2nd May 9, 2026
7162d60
docs: add migration tips for IO-based 400 account lookup and middlewa…
constantine2nd May 9, 2026
84b5d35
feat: migrate v3.0.0 to http4s — Http4s300.scala
constantine2nd May 11, 2026
028250c
fix: return 401 for auth failures on New Style endpoints in ResourceD…
constantine2nd May 11, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 53 additions & 11 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,27 @@

## Working Style
- Never blame pre-existing issues or other commits. No excuses, no finger-pointing — diagnose and resolve.
- Never add `Co-Authored-By` trailers to commit messages.
- **Goal is full http4s migration** — eliminate Lift Web and all deprecated libraries entirely. Treat Lift code as temporary scaffolding to be removed, not maintained. When fixing bugs or adding features, always prefer the http4s path.
- **Versioning is tech-agnostic** — API version numbers reflect API signature changes (new/changed fields, new behaviour), never the underlying framework. A framework migration (Lift → http4s) happens in-place at the existing version; it does not justify a version bump.

## Architecture (Onboarding)

v7.0.0 is a Lift Web → http4s migration. Not a replacement for v6.0.0 yet — 27 of 633 endpoints migrated.
> **Migration plan**: see [`LIFT_HTTP4S_MIGRATION.md`](LIFT_HTTP4S_MIGRATION.md) for the full in-place Lift → http4s strategy, file order, auth stack workstream, and progress tracker.

**Request priority chain** (Http4sServer): `corsHandler` (OPTIONS) → StatusPage → Http4s500 → Http4s700 → Http4sBGv2 → Http4sLiftWebBridge (Lift fallback). Unhandled `/obp/v7.0.0/*` paths fall through silently to Lift — they do not 404.
The goal is a full http4s migration — replace Lift Web across all version files and remove it entirely. **API versions are tech-agnostic**: a version bump means a changed/new API signature, never a framework change. Framework migration happens in-place inside the existing version file. v7.0.0 currently serves 45 endpoints; most arrived there for historical reasons and stay as-is.

**Key files**: `Http4s700.scala` (endpoints), `Http4sSupport.scala` (EndpointHelpers + recordMetric), `ResourceDocMiddleware.scala` (auth, entity resolution, transaction wrapper), `RequestScopeConnection.scala` (DB transaction propagation to Futures).
**Request priority chain** (Http4sServer): `corsHandler` (OPTIONS) → StatusPage → Http4s500 → Http4s700 → Http4sBGv2 → Http4s200 → Http4s140 → Http4s130 → Http4s121 → Http4sLiftWebBridge (Lift fallback). Unhandled `/obp/v7.0.0/*` paths fall through silently to Lift — they do not 404.

**Migrated endpoints** (27): root, getBanks, getCards, getCardsForBank, getResourceDocsObpV700, getBank, getCurrentUser, getCoreAccountById, getPrivateAccountByIdFull, getExplicitCounterpartyById, deleteEntitlement, addEntitlement, getFeatures, getScannedApiVersions, getConnectors, getProviders, getUsers, getCustomersAtOneBank, getCustomerByCustomerId, getAccountsAtBank, getUserByUserId, getCacheConfig, getCacheInfo, getDatabasePoolInfo, getStoredProcedureConnectorHealth, getMigrations, getCacheNamespaces.
**Key files**: `Http4s700.scala` (v7.0.0 endpoints), `Http4s200.scala` (v2.0.0 endpoints — 37 own + path-rewriting bridge to Http4s140), `Http4s140.scala` (v1.4.0 endpoints — 11 own + path-rewriting bridge to Http4s130), `Http4s130.scala` (v1.3.0 endpoints — 3 own + path-rewriting bridge to Http4s121), `Http4s121.scala` (v1.2.1 endpoints — all 323 API1_2_1Test scenarios), `Http4sSupport.scala` (EndpointHelpers + recordMetric), `ResourceDocMiddleware.scala` (auth, entity resolution, transaction wrapper), `IdempotencyMiddleware.scala` (Redis-backed idempotency, opt-in via `Idempotency-Key` header, nested inside ResourceDocMiddleware), `RequestScopeConnection.scala` (DB transaction propagation to Futures).

**Tests**: `Http4s700RoutesTest` (93 scenarios, port 8087). `makeHttpRequest` returns `(Int, JValue, Map[String, String])`. `makeHttpRequestWithBody(method, path, body, headers)` for POST/PUT.
**Migrated endpoints** (45): root, getBanks, getCards, getCardsForBank, getResourceDocsObpV700, getBank, getCurrentUser, getCoreAccountById, getPrivateAccountByIdFull, getExplicitCounterpartyById, deleteEntitlement, addEntitlement, getAccountAccessTrace, getFeatures, getScannedApiVersions, getConnectors, getErrorMessages, getProviders, getUsers, getUserByUserId, getCustomersAtOneBank, getCustomerByCustomerId, getAccountsAtBank, createTradingOffer, getTradingOffer, getTradingOffers, cancelTradingOffer, createMarketOrder, getMarketOrder, cancelMarketOrder, createMarketMatch, getMarketTrade, requestSettlement, requestWithdrawal, getCacheConfig, getCacheInfo, getDatabasePoolInfo, getStoredProcedureConnectorHealth, getMigrations, getCacheNamespaces, createOrganisation, getOrganisations, getOrganisation, updateOrganisation, deleteOrganisation.

## Migrating a v6.0.0 Endpoint to v7.0.0
**Tests**: `Http4s700RoutesTest` (111 scenarios, port 8087). `makeHttpRequest` returns `(Int, JValue, Map[String, String])`. `makeHttpRequestWithBody(method, path, body, headers)` for POST/PUT.

## Migrating a Lift Endpoint to http4s

Rules apply regardless of which version file the endpoint lives in. Use v7.0.0 only when the API signature is new or changed; otherwise migrate in-place in the original version file.

### Rule 1 — ResourceDoc registration
```scala
Expand Down Expand Up @@ -71,7 +78,7 @@ EndpointHelpers.withBankAccount(req) { (user, account, cc) => ... } /
EndpointHelpers.withView(req) { (user, account, view, cc) => ... } // + VIEW_ID
EndpointHelpers.withCounterparty(req) { (user, account, view, cp, cc) => ... } // + COUNTERPARTY_ID
```
**POST → 201**: `executeFutureWithBodyCreated[B,A]` / `withUserAndBodyCreated[B,A]` / `withUserAndBankAndBodyCreated[B,A]`
**POST → 201**: `executeFutureWithBodyCreated[B,A]` / `withUserAndBodyCreated[B,A]` / `withUserAndBankAndBodyCreated[B,A]` / `withViewCreated[A]` (when view context is needed)
**PUT → 200**: `executeFutureWithBody[B,A]` / `withUserAndBody[B,A]` / `withUserAndBankAndBody[B,A]`
**DELETE → 204**: `executeDelete` / `withUserDelete` / `withUserAndBankDelete`

Expand All @@ -80,6 +87,15 @@ EndpointHelpers.withCounterparty(req) { (user, account, view, cp, cc) => ... } /

## Tricky Parts (Gotchas)

**Conditional role check (403)**: `NewStyle.function.hasEntitlement` uses `booleanToFuture` with default `failCode = 400`, which gives 400 instead of 403 when the role is missing. For conditional checks (e.g. only needed when creating for another user), keep ResourceDoc roles `None` and call `booleanToFuture` directly:
```scala
_ <- if (userIdAccountOwner == loggedInUserId) Future.successful(Full(()))
else code.util.Helper.booleanToFuture(
s"$UserHasMissingRoles $canCreateAccount", failCode = 403, cc = Some(cc)) {
APIUtil.hasEntitlement(bankId, loggedInUserId, canCreateAccount)
}
```

**View permissions**: `view.canGetCounterparty` (MappedBoolean) always returns `false` for system views. Use `view.allowed_actions.exists(_ == CAN_GET_COUNTERPARTY)` instead.

**BankExtended**: `privateAccountsFuture`, `privateAccounts`, `publicAccounts` are on `code.model.BankExtended`, not `commons.Bank`. Wrap: `code.model.BankExtended(bank).privateAccountsFuture(...)`.
Expand Down Expand Up @@ -115,6 +131,30 @@ EndpointHelpers.withCounterparty(req) { (user, account, view, cp, cc) => ... } /

**Creating test data**: use provider directly — e.g. `CustomerX.customerProvider.vend.addCustomer(...)`. Do not call v6 endpoints via HTTP in v7 tests.

**`NewStyle.function.getBankAccount` returns 404**: The `unboxFullOrFail` inside hardcodes code 404. When your endpoint must return 400 for a missing account (e.g. v1.2.1 tests), bypass it: use `Connector.connector.vend.checkBankAccountExists(bankId, accountId, cc)` then `Future { unboxFullOrFail(rawBox, cc, msg) }` — the default code is 400.

**Middleware URL template bypass** (non-standard uppercase vars): `validateAccount` checks `pathParams.get("ACCOUNT_ID")` and `validateView` checks `pathParams.get("VIEW_ID")` by exact key. Any other all-caps segment (e.g. `BANK_ACCOUNT_ID`, `CUSTOM_VIEW_ID`, `GRANT_VIEW_ID`, `NEW_ACCOUNT_ID`, `VIEW_ACCOUNT_ID`, `UPD_VIEW_ID`) is still matched as a template variable (wildcard) but skips the 404/403 validation. Use this when your handler does inline validation returning 400 but middleware would return 404 or 403 first.

For IO-based handlers that bypass `ACCOUNT_ID`, look up the account inline and return 400 for missing accounts (matching Lift behaviour):
```scala
// ResourceDoc URL: "/banks/BANK_ID/accounts/VIEW_ACCOUNT_ID/views"
case req @ POST -> `prefixPath` / "banks" / _ / "accounts" / accountIdStr / "views" =>
implicit val cc: CallContext = req.callContext
val io = for {
bank <- IO.fromOption(cc.bank)(new RuntimeException(BankNotFound))
rawBox <- IO.fromFuture(IO(Connector.connector.vend.checkBankAccountExists(bank.bankId, AccountId(accountIdStr), Some(cc)).map(_._1)))
account <- IO(unboxFullOrFail(rawBox, Some(cc), BankAccountNotFound)) // default emptyBoxErrorCode=400
...
} yield result
```
`checkBankAccountExists` returns `OBPReturnType[Box[BankAccount]]` = `Future[(Box[BankAccount], Option[CC])]`. Extract the `Box` with `.map(_._1)`. `unboxFullOrFail` with default `emptyBoxErrorCode=400` throws a JSON-encoded 400 exception that `ErrorResponseConverter` parses correctly.

**Auth failure status code — Old Style vs New Style**: `ResourceDocMiddleware.authenticate` returns 400 for auth failures (locked user, invalid DAuth JWT) on Old Style endpoints (v1.2.1, v1.3.0, v1.4.0, v2.0.0) and 401 on New Style endpoints (v2.1.0+). The version check is in the `case Left(e)` branch: `oldStyleShortVersions.contains(resourceDoc.implementedInApiVersion.apiShortVersion)`. `anonymousAccess` always converts Failure boxes to `Exception(json_of_APIFailureNewStyle)` with `failCode=401` via `fullBoxOrException`. The Old Style 400 is a deliberate override for backward compatibility.

**`ResourceDoc` description and `needsAuthentication`**: The `ResourceDoc` constructor removes `AuthenticatedUserIsRequired` from `errorResponseBodies` when `description.contains(authenticationIsOptional) && rolesIsEmpty`. `needsAuthentication = errorResponseBodies.contains($AuthenticatedUserIsRequired) || roles.nonEmpty`. If the description embeds `${userAuthenticationMessage(false)}` (which includes `authenticationIsOptional`) and roles are empty, the error is silently removed → `needsAuthentication=false` → anonymous access → unauthenticated requests reach the handler. Fix: remove `${userAuthenticationMessage(false)}` from the description when `AuthenticatedUserIsRequired` must remain in the error list.

**v1.2.1 test framework sends filter params as HTTP headers**: `makeGetRequest(req, params)` puts `params` into `extra_headers`, not the URL query string. This means `obp_limit`, `obp_sort_direction`, `obp_from_date`, etc. arrive as request headers. Do NOT use `createHttpParamsByUrl(req.uri.renderString)` — it only scans the URL for non-prefixed names. Instead: `req.headers.headers.toList.map(h => HTTPParam(h.name.toString, h.value))`, then pass to `createQueriesByHttpParamsFuture`.

**CI**: Tests run with `mvn test -DwildcardSuites="..."`. `hikari.maximumPoolSize=20` required in test props for concurrent tests (`withRequestTransaction` holds 1 connection per request; rate-limit queries need a 2nd → pool of 10 exhausts at 5 concurrent requests).

## CI Performance Profile
Expand Down Expand Up @@ -147,9 +187,10 @@ Compile times are consistent across all three shards — Zinc cache restores cor

At the integration level both frameworks are similarly server/DB-bound (~0.32–0.45 s/test). The real http4s gain is the **unit/pure tier** — tests that don't need a running server are 54× faster. As more logic moves into pure functions (request parsing, response building, auth checks) these unit tests replace integration tests and the savings compound.

The 5 integration suites (160 tests, 66.9s total):
The 6 integration suites (pre-merge timings; Http4s700RoutesTest has grown to 111 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` — 75 tests, 23.8s
- `obp-api/src/test/scala/code/api/v7_0_0/Http4s700RoutesTest.scala` — 111 tests (was 75, 23.8s pre-merge)
- `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/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
Expand All @@ -170,7 +211,7 @@ The 12 pure-unit suites (172 tests, 1.3s total):

### Known bottlenecks

**`API1_2_1Test`** (Lift v1) — 143s for 323 tests, 36% of shard2's entire test time. Larger than the full http4s v7 budget. The first test in the suite (`"base line URL works"`) takes 0.97s — Lift's lazy init cost. Moving this suite to its own shard would reduce pipeline wall-clock by ~90s.
**`API1_2_1Test`** (now http4s-backed via `Http4s121`) — was 143s for 323 tests on the Lift path; expected to improve as Lift bridge overhead is eliminated. The suite is in shard 3 (`code.api.v1_2_1` prefix).

**`Http4sLiftBridgePropertyTest`** — 31.9s for 51 tests. Property 7 ("Session and Context Adapter Correctness") accounts for 13.4s of that: three ScalaCheck properties exercise concurrent requests through the Lift/http4s bridge, hitting real lock contention between Lift's session manager and the http4s fiber scheduler. Property 7.4 alone is 8.54s. These are the most meaningful slow tests — they exercise a genuine concurrency boundary.

Expand Down Expand Up @@ -215,6 +256,7 @@ Body helpers and DELETE 204 helpers ready. Velocity: 6–8 endpoints/day.
Dynamic entities, ABAC rules, mandate workflows, polymorphic bodies. ~45–60 min each.

### Other TODOs
- **OBP-Trading** (at `/home/marko/Tesobe/GitHub/constantine2nd/OBP-Trading`): pending team decision — port trading endpoints into `Http4s700.scala` or keep as a separate service that OBP-API proxies to. Connectors (`ObpApiUserConnector`, `ObpPaymentsConnector`) are currently in-memory stubs.
- **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.
103 changes: 0 additions & 103 deletions LIFT_HTTP4S_COEXISTENCE.md

This file was deleted.

Loading
Loading