Resource docs#2825
Merged
Merged
Conversation
…endpoints visible under newer-version prefixes Companion to commit 9c9b5fe (the NMB fix that restored cascade reachability at runtime when `api_disabled_versions` skips a middle version). That commit pins the *runtime* side of the cascade contract via ResourceDocMiddlewareEnableDisablePropsTest. This commit pins the *documentation* side: when an operator skips v2.0.0 in `api_enabled_versions`, the v2.0.0-origin endpoints stay reachable via newer prefixes — they must therefore also still appear in the corresponding `/resource-docs/.../obp` responses, or callers can't discover what's reachable. The NMB-reported endpoint `Add Entitlement for User` was exactly this case. Two scenarios added to SwaggerDocsTest: 1. `GET /obp/v6.0.0/resource-docs/v6.0.0/obp` with the NMB-style `api_enabled_versions` (skips v2.0.0) — the response must still include the v2.0.0-origin `addEntitlement` doc. 2. The same response surfaces a non-trivial number (>5) of v2.0.0-origin endpoints — a regression that drops most of the cascade fails even if one or two stay. If `getResourceDocsList` ever starts filtering by `versionIsAllowed(rd.implementedInApiVersion)`, or the `OBPAPI{version}.allResourceDocs` cascade chain breaks, these tests fail and force a re-think before the change ships. SwaggerDocsTest now has 14 scenarios; was 12.
…ired in migration doc + regression-guard test Companion to PR OpenBankProject#2814 (commit d19af2b) which commented out 5 non-OBP Lift API standards. The disable is structural — `ClassScanUtils.getSubTypeObjects` finds no `ScannedApis` in those packages, so Lift's dispatch never sees them and `Boot.scala` needs no change. The flip side is that a partial uncomment of any retired-standard file would silently re-register that standard at the next startup. Two changes: 1. **`LIFT_HTTP4S_MIGRATION.md`** — replace the five "Lift" rows in the Open-banking standards table with "Retired" (strike-through) entries, add a paragraph explaining the comment-out pattern + the deletion candidate timeline, and forward-reference the new regression-guard test. Section heading drops "(large, deferred indefinitely)" — only the BG v1.3 / UKOpenBanking / sandbox rows remain in-scope. 2. **`RetiredApiStandardsTest`** — scenario `"ScannedApis registry must not contain any object from a retired-standard package"`. Walks the live `ScannedApis.versionMapScannedApis` map and asserts no entry's fully-qualified class name starts with one of: code.api.BahrainOBF. code.api.AUOpenBanking. code.api.STET. code.api.Polish. code.api.MxOF. If anyone partially uncomments a retired standard, this test fails with the exact class name they brought back, and forces a re-think before the change ships. Uses `V400ServerSetup` because the scan needs a real classpath view (same hierarchy as `ApiVersionUtilsTest`, which counts scanned versions 26 → 20). Currently passes — confirms the empirical "zero scanned classes in the retired packages" invariant after PR OpenBankProject#2814.
…tHelper
The bare `POST /my/logins/direct` route has been served by
`code.api.DirectLoginRoutes` (wired into `Http4sApp.baseServices`) since
`LiftRules.statelessDispatch.append(DirectLogin)` was removed from
`Boot.scala`. That left the `dlServe` self-registration block inside
`DirectLogin` as dead code — `super.serve(...)` only does anything when
the object is registered with `LiftRules`, which it no longer is.
Removed:
- the `dlServe` helper (its only job was wrapping a handler in
`RestHelper.serve`),
- the `dlServe { case Req("my" :: "logins" :: "direct" :: Nil, _, PostRequest) }`
registration block,
- `extends RestHelper` (now only `MdcLoggable`) — nothing outside the dead
block used a RestHelper member; there were no `.extract`/`Formats` call
sites, so no local `Formats` replacement was needed (unlike the
OAuth2/GatewayLogin/DAuth de-RestHelper cleanups),
- the now-unused `RestHelper` and `NewStyle.HttpCode` imports.
Kept (still have live callers): `createTokenCommonPart`, `getAllParameters`,
`validator`, `validatorFuture`, `validatorFutureWithParams`,
`getUserFromDirectLoginHeaderFuture`, `getUser`, `getConsumer`,
`grantEntitlementsToUseDynamicEndpointsInSpacesInDirectLogin`, and the
token/consumer/user lookup helpers used by the auth flows.
No behavioural change: the removed block was never reachable after the
Boot dispatch removal. obp-api compiles; no test referenced the removed
members.
…ridge)
Phase 2 of the dynamic-* migration. The operator-created dynamic-entity
data plane (`/obp/dynamic-entity/{entityName | my/.. | public/.. |
community/..}` and their `banks/BANK_ID/..` variants) is now served by a
native `code.api.dynamic.entity.Http4sDynamicEntity` route instead of
falling through to `Http4sLiftWebBridge` → `OBPAPIDynamicEntity`.
What it does:
- Reuses the framework-agnostic `EntityName` / `PublicEntityName` /
`CommunityEntityName` extractors (they already take `List[String]` and
consult `DynamicEntityHelper.definitionsMap`, rebuilt per request).
- Mirrors the Lift `genericEndpoint` / `publicEndpoint` / `communityEndpoint`
partial functions verbatim — same `authenticatedAccess` / `anonymousAccess`,
`getBank`, role checks (`hasEntitlement` with the same canGet/Create/Update/
Delete roles + the `personalRequiresRole` shortcut), before/after auth
interceptors, and `NewStyle.invokeDynamicConnector` calls. Only the Lift
plumbing changes: matching on the http4s path, query params from the URI,
and the `(JValue, HttpCode)` return replaced by `EndpointHelpers`
(200 for GET/PUT/DELETE, 201 for POST). Public/Community are matched
before the generic extractor to preserve Lift's registration precedence.
Wiring:
- `gate(ApiVersion.dynamic-entity, Http4sDynamicEntity.routes)` added to
`Http4sApp.baseServices` just before the Lift bridge — gated by the same
api_disabled_versions / api_enabled_versions machinery as the versioned
routes. A non-match yields OptionT.none so unrelated paths fall through
unchanged.
- `OBPAPIDynamicEntity` stays registered on Lift as a dormant fallback (this
route wins by ordering); its Lift registration is removed in the
bridge-removal PR.
Out of scope: dynamic-*endpoint* (runtime Scala codegen → Lift `OBPEndpoint`)
remains on the bridge — a separate workstream. Admin CRUD
(createDynamicEntity, …) was already native http4s in the versioned files.
Verified: v6 DynamicEntityAccessFlagsTest + v4 DynamicEntityTest (38 tests)
pass against the in-process http4s server, covering personal/public/community
access flags, full CRUD, and the 404 fall-through for a system-level entity's
absent `my` endpoint. v4 DynamicIntegrationTest / DynamicResourceDocTest /
DynamicendPointsTest also green (dynamic-endpoint still served by the bridge).
…nt remains Phase 3 Companion to ee05e70. Reflects that the dynamic-entity data plane is now served by `Http4sDynamicEntity` and off the Lift bridge: - Progress table: add "Dynamic-entity data plane" (done) and "Dynamic-endpoint data plane" (todo — Phase 3, runtime codegen still on the bridge) rows. - Bridge-traffic audit section: note that the `/obp/dynamic-entity/...` half of the `real_work` bucket no longer reaches the bridge (test runs show only legitimate 404 fall-throughs remain, e.g. a `my/` request against a system-level entity); the remaining `real_work` is `/obp/dynamic-endpoint/...`.
…on guard
CLAUDE.md flagged a "~40% flaky" race in MakerCheckerTransactionRequestTest:
in the multi-challenge path, the request-scoped proxy connection sometimes
failed to propagate (via TransmittableThreadLocal) to a read Future's worker
thread, so the read landed on a fresh pool connection and saw 0 uncommitted
challenge rows. Two empirical sweeps + an in-JVM stress run all show the
race no longer reproduces on current develop:
- 8 instrumented runs (logged `currentProxy.get() == null` and row counts at
every challenge write/read): all PASS, `proxyNull=false` on every read,
across many worker threads, multi-user path included.
- 12 clean runs (no instrumentation, separate JVMs): all PASS.
- 1 stress run (20-iteration loop of multi-challenge creates in one warm JVM,
hammering the exact write→read surface): all 20 iterations PASS, both
challenges read back into the 201 response every time.
40 consecutive multi-challenge create→read cycles with zero failures. At the
historical ~40% per-cycle rate, P ≈ 1.3×10⁻⁹ — the rate definitively no
longer holds. Almost certainly fixed by the RequestScopeConnection hardening
already on develop:
1. `currentProxy.childValue(parent) → null` (overridden in
`RequestScopeConnection.scala`). Its own comment describes exactly the
failing symptom: "workers stuck with a stale proxy then read 0 rows for
the current request's freshly-written data, since the underlying real
connection was closed by the original request's WBT."
2. The stale-proxy `isClosed()` guard added in
`RequestAwareConnectionManager.newConnection`, which now falls back to a
fresh vendor connection rather than returning a stale (already-closed)
proxy.
This commit locks the resolution in and updates the stale TODO:
- **MakerCheckerTransactionRequestTest** — add a new scenario,
"Stress: repeated multi-challenge creates must always read back both
challenges (RequestScopeConnection regression guard)". Reuses the existing
multi-user setup (REQUIRED_CHALLENGE_ANSWERS=2, two-user view access,
maker-checker enforcement) and fires 20 INITIATED creates in one warm JVM,
asserting both per-user challenges round-trip into each 201 response. No
money moves (INITIATED only), so it's safe to loop. Iteration is recorded
via `withClue` so a regression points at the offending iteration.
- **CLAUDE.md** — strike through the "Flaky MakerCheckerTransactionRequestTest"
bullet under "Other TODOs", explain it's resolved by the RequestScopeConnection
hardening, link the regression guard, and keep the historical diagnosis +
original fix directions in case a regression resurfaces.
No core connector / payment-path code is changed — the proper fix already
landed in `RequestScopeConnection`; this commit adds the safety net.
…ric bridge The dynamic-endpoint data plane (operator-created endpoints under `/obp/dynamic-endpoint/...`) is now served by a native http4s service — `code.api.dynamic.endpoint.Http4sDynamicEndpoint` — that dispatches `OBPAPIDynamicEndpoint` directly inside a scoped `S.init`, rather than letting the request fall through to the generic `Http4sLiftWebBridge`. The runtime Scala codegen (`DynamicEndpoints.compileScalaCode[OBPEndpoint]`) is unchanged: compiled user-supplied bodies still read `request.body`, `request.json`, `request.path.partPath` from a Lift `Req`. Phase 3a keeps that shape and stands a thin shim in front of it; Phase 3b (rewriting the codegen to emit `HttpRoutes[IO]` and drop the synthetic-`Req` coupling) is deferred. How it works: 1. `Http4sDynamicEndpoint.routes` matches `/obp/dynamic-endpoint/*` and reads the request body once. 2. Builds a synthetic Lift `Req` (`Http4sLiftWebBridge.buildLiftReq` — same `Http4sLiftRequest` shim the bridge already uses; full `HTTPRequest` surface satisfied). 3. Inside `S.init(Full(liftReq), session)` for a Lift stateless session, dispatches via `OBPAPIDynamicEndpoint` itself — `RestHelper` IS a `PartialFunction[Req, () => Box[LiftResponse]]`, the already-fully-wrapped shape (apiPrefix + oauthServe + wrappedWithAuthCheck), so URL matching, auth, role and `ResourceDoc`/`operationId` plumbing all run through the same code paths production has always used. No need to invoke the raw `OBPEndpoint` PFs or reimplement `apiPrefix`'s prefix-strip — which is essential, because `wrappedWithAuthCheck`'s `isUrlMatchesResourceDocUrl` (APIUtil.scala:1897) expects the prefix already stripped. 4. The resulting `Box[LiftResponse]` is mapped to a `LiftResponse` with the same `Full` / `Failure` / `ParamFailure` / `Empty` handling as `Http4sLiftWebBridge.runLiftDispatch`. `JsonResponseException` and `net.liftweb.http.rest.ContinuationException` (thrown by Future-based for-comprehension bodies via the OBPReturnType implicit) are caught at both depths and routed through the bridge's `resolveContinuation` reflective shim. 5. `Http4sLiftWebBridge.liftResponseToHttp4s` converts the final `LiftResponse` to `Response[IO]` — handles `InMemoryResponse`, `StreamingResponse`, `OutputStreamResponse`, `BasicResponse`. 6. Unmatched `/obp/dynamic-endpoint/...` paths return 404 with the same `InvalidUri` framing the bridge produces — **authoritative** for the prefix; never falls through to the generic bridge. To enable the reuse, three private helpers in `Http4sLiftWebBridge` had their `private` visibility relaxed to package-visible: `buildLiftReq`, `liftResponseToHttp4s`, `resolveContinuation`. Their bodies are unchanged. `OBPAPIDynamicEndpoint` stays registered on `LiftRules.statelessDispatch` (`APIUtil.scala:2878`) as a dormant fallback — the new http4s service wins by ordering in `Http4sApp.baseServices`. Removed in the bridge-removal PR. Audit impact: after this PR, `/obp/dynamic-endpoint/...` requests stop reaching `Http4sLiftWebBridge`. The bridge's `real_work` audit bucket reduces to the open-banking standards (BG v1.3, UK OB, sandbox); all OBP-native data-plane traffic is now off the generic bridge. Wiring: `gate(ApiVersion.dynamic-endpoint, Http4sDynamicEndpoint.routes)` added to `Http4sApp.baseServices` immediately after the dynamic-entity route and before `Http4sLiftWebBridge.routes`. Verified: 13 suites / 52 tests green — v4 DynamicendPointsTest, DynamicIntegrationTest, DynamicResourceDocTest, DynamicEndpointHelperTest, DynamicMessageDocTest, code.util.DynamicUtilTest, plus v6 DynamicEntityAccessFlagsTest as a regression sanity check (dynamic-entity is unaffected — this commit only adds a separate route for /dynamic-endpoint/). Out of scope (deferred): - Phase 3b — rewrite codegen to emit `HttpRoutes[IO]` directly. - Unregistering `OBPAPIDynamicEndpoint` from `LiftRules` — bridge-removal PR.
…sions Shard 1 CI on origin/develop (commits ee05e70..ca4237e) reports 6 failing v4 tests, all 500 where 400/403/444 was expected: ForceErrorValidationTest (5 fails) — "dynamic entity endpoints Force-Error" JsonSchemaValidationTest (1 fail) — dynamic entity JSON-schema validation All on the dynamic-entity data plane. Regression introduced by the Phase 2 commit (ee05e70) `feat(dynamic-entity): native http4s data-plane routes`: - Pre-Phase-2 path: dynamic-entity → Lift bridge. `Http4sLiftWebBridge.runLiftDispatch` has explicit `case JsonResponseException(jsonResponse) => jsonResponse` → the embedded synthesized response (Force-Error 400/403/444, JSON-schema 400, etc.) is sent verbatim. - Post-Phase-2 path: dynamic-entity → `Http4sDynamicEntity` → `EndpointHelpers` → on Future failure, `ErrorResponseConverter.toHttp4sResponse(err, cc)`. That converter only knew `APIFailureNewStyle` and the OBP-prefixed-message form; `JsonResponseException` fell to `unknownErrorToResponse` → 500. `JsonResponseException` (defined `OBPRestHelper.scala:228`) is thrown from `APIUtil.anonymousAccess` (line 3517) and from the after-authenticate interceptor chain in `NewStyle.scala:959` — it is *the* mechanism used by the interceptor framework (Force-Error / JSON schema validation / auth-type validation) to short-circuit with a fully-formed `JsonResponse` from inside a `Future`. Any http4s endpoint whose body crosses one of those interceptors needs this case to translate the carried response to http4s. This is a defensive improvement at the right level: it fixes all http4s endpoints, not just dynamic-entity. Reuses `Http4sLiftWebBridge.liftResponseToHttp4s` (already package-visible from the Phase 3a commit) so the byte-shape of the response matches the bridge's exactly. No production logic moves — only the exception case is added. Verified locally: 19 suites / 111 tests green — ForceErrorValidationTest, JsonSchemaValidationTest, AuthenticationTypeValidationTest (same interceptor mechanism), v4 DynamicendPointsTest, DynamicIntegrationTest, DynamicResourceDocTest, DynamicEndpointHelperTest, DynamicMessageDocTest, code.util.DynamicUtilTest, v6 DynamicEntityAccessFlagsTest, v4 MakerCheckerTransactionRequestTest. Diagnostic that pinned the root cause (kept for the PR description, not in code): `io-compute-*` threads logging `unknownErrorToResponse says: 500 returned` followed by `code.api.JsonResponseException: null` for each of the 5 ForceError scenarios.
…resource-docs Lift cleanup Adopts upstream PR OpenBankProject#2815 (Hongwei) into the local fork's develop. His PR was opened against `aa21d891d` on upstream — before all of my recent work landed on `origin/develop` — so we did the dynamic-entity Phase 2 work in parallel. His implementation supersedes mine because: 1. **Transaction atomicity.** He extracted `withBusinessDBTransaction` from `ResourceDocMiddleware` into `RequestScopeConnection`, and his `Http4sDynamicEntity` wraps POST/PUT/DELETE in it. My Phase 2 missed this — every DB op auto-committed, no request-scoped atomicity. Real bug, undetected by tests because each handler does a single DB op. 2. **Cleaner Lift unregistration.** `OBPAPIDynamicEntity.routes = Nil` + `LiftRules.statelessDispatch.append` commented out + the Lift OBPEndpoint handlers in `APIMethodsDynamicEntity` commented (with a `/* DISABLED … */` block per the repo's convention). My version kept the Lift registration as a dormant fallback, which is safer during migration but messier as an end state. 3. **Characterization tests.** `DynamicEntityFilterAndBankAccessTest` (275 lines) locks in GET-all `req.params` filtering (PARAM_LOCALE exclusion) + bank-level public/community access — both edge cases I hadn't covered. Written *before* his migration as a regression gate. 4. **Resource-docs cleanup.** Three follow-up commits (aabebc0, 35988b0, ca461f6) comment out the `ResourceDocs140..600` Lift dispatch handlers and restore minimal stubs for the entries `Http4sResourceDocs` still references. Independent of dynamic-entity but on the same Lift-removal track. Conflict resolution (3 files, all "take theirs" with one Phase-3a exception): - `Http4sDynamicEntity.scala` (add/add) — accepted his 358-line file verbatim (vs my 488-line version). - `ErrorResponseConverter.scala` — accepted his `JsonResponseException` case (extracts `(message, code)` → `OBPErrorResponse` envelope). My `1e1047da1` `liftResponseToHttp4s` pass-through approach is superseded; his envelope-extraction works for every test that asserts on `(code, message)` (which is all of them today). Worth a follow-up to swap to `liftResponseToHttp4s` if a future test asserts on Lift-side response headers, but not blocking. - `Http4sApp.scala` — kept his `dynamicEntityRoutes` declaration (`Http4sDynamicEntity.wrappedRoutesDynamicEntity` — his entry-point name) **and** my `dynamicEndpointRoutes` declaration (Phase-3a dynamic-endpoint adapter, unrelated to his PR). De-duplicated the `.orElse(dynamicEntityRoutes.run(req))` wire-in that auto-merge inserted twice — kept it once at his position (right after v121). Surviving from my fork (not affected by his PR): - `8dfdae9ae` — DirectLogin cleanup (dead `dlServe` block removal). - `ca4237e73` — MakerChecker TTL race resolution + stress regression guard. - `97c74e09e` — dynamic-endpoint Phase 3a (scoped Lift-Req shim). - `4c262e420` — migration-doc update for dynamic-entity (still accurate — the doc says Phase 2 is done; the *implementation* is now his). Effectively superseded by his (no functional loss): - `ee05e701a` — my Phase 2 Http4sDynamicEntity (his version replaces it). - `1e1047da1` — my JsonResponseException case in ErrorResponseConverter (his version replaces it). Verified: 23 suites / 151 tests green — ForceErrorValidationTest (the 5-fail regression — now green), JsonSchemaValidationTest (the 1-fail regression — now green), AuthenticationTypeValidationTest (same interceptor mechanism), v6 DynamicEntityFilterAndBankAccessTest (Hongwei's new tests), v6 DynamicEntityAccessFlagsTest, v4 DynamicendPointsTest, DynamicIntegrationTest, DynamicResourceDocTest, DynamicEndpointHelperTest, DynamicMessageDocTest, code.util.DynamicUtilTest, v4 MakerCheckerTransactionRequestTest (incl. my stress regression guard). Once Hongwei's PR is also merged into upstream and origin/develop pulls that in, the resulting state matches this merge exactly — this lets the fork stay green while the upstream PR completes its review.
…oints to native http4s # Conflicts: # obp-api/src/main/scala/code/api/util/http4s/Http4sApp.scala
- drop 45-name migrated-endpoints list and TODO/Phase-Progress section (status → MIGRATION.md) - condense CI Performance Profile to shard map + 1-line perf note - add two gotchas (JVM 64KB <init> limit, isStatisticallyTooPermissive) - drop stale V7ResourceDocsAggregationTest 'intentionally failing' note (bug fixed)
- collapse per-version drift tables to a script-pointer + drift-category summary - merge Migration Order + Progress into one per-version table; auth stack to status rows - fold in TODO items moved out of CLAUDE.md (OBP-Trading, CI speed-up, disabled tests, MakerChecker resolved) - drop redundant ASCII server chain, 'Why http4s', duplicated migration-mapping prose - keep all open TODOs, decision gates, risks, bridge-audit playbook, done criteria
# Conflicts: # obp-api/src/main/scala/code/api/dynamic/endpoint/Http4sDynamicEndpoint.scala # obp-api/src/main/scala/code/api/util/http4s/Http4sApp.scala # obp-api/src/main/scala/code/api/util/http4s/Http4sLiftWebBridge.scala
… sandbox classloader test issue - dynamic-endpoint data plane: todo Phase 3 → done (fully-native http4s, off-bridge) - bridge-audit: no known real_work remains; OIDC is now the last code blocker - record DynamicResourceDocTest sandbox specifyStreamHandler failures (upstream-inherited, not the merge)
# Conflicts: # CLAUDE.md # LIFT_HTTP4S_MIGRATION.md
…he docs list The resource-docs / swagger / openapi endpoints are served natively by Http4sResourceDocs, but their self-describing ResourceDoc entries were lost when serving moved Lift -> http4s: the Lift `localResourceDocs += ...` registrations were commented out and Http4sResourceDocs registered none, so `localResourceDocs` was empty. Since `getResourceDocsList` only appends `localResourceDocs` for the obp standard, "Get Resource Docs" and friends disappeared from the listing shown by API Explorer. Repopulate `localResourceDocs` with the four endpoints (getResourceDocsObp, getBankLevelDynamicResourceDocsObp, getResourceDocsSwagger, getResourceDocsOpenAPI31), restored verbatim from the pre-comment source so summaries/descriptions/tags/roles/error-lists match the originals. These are documentation-only entries (partialFunction = null, no http4sPartialFunction) because routing is handled upstream by Http4sResourceDocs. The duplicate getResourceDocsObpV400 registration removed in efb9753 is intentionally not reintroduced. Update the stale Http4s700 comment to reflect the registration. Verified: ResourceDocsTest (63) and V7ResourceDocsAggregationTest (14) pass.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.