Refactor/migrate dynamic entity to http4s#2815
Merged
simonredfern merged 4 commits intoMay 28, 2026
Merged
Conversation
…p4sResourceDocs All Lift dispatch has been retired; Http4sResourceDocs now handles every /resource-docs/* URL. However Http4sResourceDocs still delegates business logic to ImplementationsResourceDocs (defined in ResourceDocsAPIMethods trait), which requires a concrete OBPRestHelper instance to instantiate it. Restore two minimal stub objects: - ResourceDocs140: provides the default ImplementationsResourceDocs (includeTechnologyInResponse = false) - ResourceDocs300 / ResourceDocs300.ResourceDocs600: provides the v6.0.0 variant (includeTechnologyInResponse = true, so the technology field appears in v6.0.0 resource-doc responses) Both stubs have empty routes and are not registered in LiftRules.statelessDispatch. All other per-version objects (ResourceDocs200 through ResourceDocs510) remain commented out — Http4sResourceDocs uses a single ImplDefault instance for every non-v6 prefix. Also restores the corresponding import in Http4sResourceDocs.scala and comments out the now-stale Boot.scala wildcard import of nested objects (ResourceDocs310/400/500/510/600) that no longer exist.
…ceDocsAPIMethods All resource-docs / swagger / openapi traffic is now served by Http4sResourceDocs (wired into Http4sApp.baseServices). The Lift OBPEndpoint handlers inside ImplementationsResourceDocs are dead code. Comment out (not delete) the following Lift-specific sections: - cleanApiVersionString — dead code, never referenced - getResourceDocsDescription — only used by the commented-out localResourceDocs registrations - resourceDocsRequireRole — only used by Lift handlers - lazy val getResourceDocsObp / getResourceDocsObpV400 (OBPEndpoint) - private def getApiLevelResourceDocs (Lift handler helper) - def getBankLevelDynamicResourceDocsObp (OBPEndpoint) - def getResourceDocsSwagger : OBPEndpoint (public Lift handler — the private overload getResourceDocsSwagger(String, List[...]) is kept because convertResourceDocsToSwaggerJvalueAndSetCache calls it) - def getResourceDocsOpenAPI31 : OBPEndpoint - All four localResourceDocs += ResourceDoc(...) registration blocks Business-logic methods remain untouched: getResourceDocsList, getStaticResourceDocsObpCached, getAllResourceDocsObpCached, getResourceDocsObpDynamicCached, convertResourceDocsToSwaggerJvalueAndSetCache, convertResourceDocsToOpenAPI31JvalueAndSetCache, convertResourceDocsToOpenAPI31YAMLAndSetCache, resourceDocsToResourceDocJson, getSpecialInstructions, resourceDocsJsonToJsonResponse — all still called by Http4sResourceDocs. Also update ResourceDocsTest / SwaggerDocsTest: replace nameOf(ImplementationsResourceDocs.xxx) macro calls that referenced now-commented members with equivalent string literals.
…native http4s Serve /obp/dynamic-entity/* with a new native http4s service (Http4sDynamicEntity) instead of the Lift OBPAPIDynamicEntity dispatch; remove dynamic-entity from LiftRules.statelessDispatch. Wired into Http4sApp.baseServices ahead of the Lift bridge. - Http4sDynamicEntity: generic (GET/POST/PUT/DELETE), public (anonymous GET) and community (authenticated GET) handlers ported from APIMethodsDynamicEntity, including the before/after authenticate interceptors (auth-type / query-param / header-key, and Force-Error / JSON-schema validation). Query-param filtering uses http4s multiParams. - Extract withBusinessDBTransaction from ResourceDocMiddleware into RequestScopeConnection so the new service (which bypasses ResourceDocMiddleware, since the dynamic-entity set is runtime-mutable) reuses the same request-scoped commit/rollback/close; middleware delegates. - ErrorResponseConverter handles JsonResponseException (raised by the auth/session chain for Force-Error / JSON-schema and by NewStyle), mapping to the embedded response code (mirroring Lift's OBPRestHelper) instead of returning 500. - Comment out the Lift dispatch/handlers (OBPAPIDynamicEntity routes -> Nil; the three APIMethodsDynamicEntity handlers) per the revert-and-comment convention; objects kept as accessors for resource-docs aggregation. ResourceDocsAPIMethods: dynamic-entity skips the Lift-route-class filter. - Tests: add DynamicEntityFilterAndBankAccessTest (query-param filtering + bank-level public/community characterization); fix a nameOf() reference in ForceErrorValidationTest. DynamicEndpoint (proxy + runtime-compiled resource docs) is a separate task, untouched.
|
constantine2nd
added a commit
to constantine2nd/OBP-API
that referenced
this pull request
May 28, 2026
…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.
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.