Skip to content

Commit fffec38

Browse files
Merge Hongwei's PR OpenBankProject#2815 — adopt his dynamic-entity + 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.
2 parents 1e1047d + 76f7589 commit fffec38

15 files changed

Lines changed: 1365 additions & 1161 deletions

obp-api/src/main/scala/bootstrap/liftweb/Boot.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import code.accountattribute.MappedAccountAttribute
3737
import code.accountholders.MapperAccountHolders
3838
import code.actorsystem.ObpActorSystem
3939
import code.api.Constant._
40-
import code.api.ResourceDocs1_4_0.ResourceDocs300.{ResourceDocs310, ResourceDocs400, ResourceDocs500, ResourceDocs510, ResourceDocs600}
40+
//import code.api.ResourceDocs1_4_0.ResourceDocs300.{ResourceDocs310, ResourceDocs400, ResourceDocs500, ResourceDocs510, ResourceDocs600}
4141
import code.api.ResourceDocs1_4_0._
4242
import code.api._
4343
import code.api.attributedefinition.AttributeDefinition

obp-api/src/main/scala/code/api/ResourceDocs1_4_0/ResourceDocs140.scala

Lines changed: 159 additions & 124 deletions
Large diffs are not rendered by default.

obp-api/src/main/scala/code/api/ResourceDocs1_4_0/ResourceDocsAPIMethods.scala

Lines changed: 592 additions & 591 deletions
Large diffs are not rendered by default.

obp-api/src/main/scala/code/api/dynamic/entity/APIMethodsDynamicEntity.scala

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,11 @@ trait APIMethodsDynamicEntity {
7070
}
7171
}
7272

73+
/* DISABLED — DynamicEntity runtime CRUD migrated to code.api.dynamic.entity.Http4sDynamicEntity
74+
(wired into Http4sApp.baseServices). These Lift OBPEndpoint handlers are no longer registered
75+
(OBPAPIDynamicEntity.routes = Nil and dynamic-entity removed from LiftRules.statelessDispatch).
76+
Retained, commented out, for historical reference per the repo's revert-and-comment convention.
77+
7378
lazy val genericEndpoint: OBPEndpoint = {
7479
case EntityName(bankId, entityName, id, isPersonalEntity) JsonGet req => { cc =>
7580
val listName = StringHelpers.snakify(entityName).replaceFirst("[-_]*$", "_list")
@@ -513,6 +518,8 @@ trait APIMethodsDynamicEntity {
513518
}
514519
}
515520
}
521+
*/
522+
// end DISABLED handlers (migrated to Http4sDynamicEntity)
516523
}
517524
}
518525

obp-api/src/main/scala/code/api/dynamic/entity/Http4sDynamicEntity.scala

Lines changed: 216 additions & 346 deletions
Large diffs are not rendered by default.

obp-api/src/main/scala/code/api/dynamic/entity/OBPAPIDynamicEntity.scala

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -50,26 +50,29 @@ object OBPAPIDynamicEntity extends OBPRestHelper with MdcLoggable with Versioned
5050
// if old version ResourceDoc objects have the same name endpoint with new version, omit old version ResourceDoc.
5151
def allResourceDocs = collectResourceDocs(ImplementationsDynamicEntity.resourceDocs)
5252

53-
val routes : List[OBPEndpoint] = List(ImplementationsDynamicEntity.publicEndpoint, ImplementationsDynamicEntity.communityEndpoint, ImplementationsDynamicEntity.genericEndpoint)
53+
// Runtime CRUD migrated to code.api.dynamic.entity.Http4sDynamicEntity (wired into
54+
// Http4sApp.baseServices). routes reduced to Nil — the Lift OBPEndpoint handlers are no
55+
// longer registered with Lift. This object is retained only as an accessor for
56+
// allResourceDocs / routes referenced by ResourceDocsAPIMethods.getResourceDocsList.
57+
val routes : List[OBPEndpoint] = Nil
58+
// val routes : List[OBPEndpoint] = List(ImplementationsDynamicEntity.publicEndpoint, ImplementationsDynamicEntity.communityEndpoint, ImplementationsDynamicEntity.genericEndpoint)
59+
60+
// routes.map(endpoint => oauthServe(apiPrefix{endpoint}, None)) // no Lift dispatch registration — served by Http4sDynamicEntity
5461

55-
routes.map(endpoint => oauthServe(apiPrefix{endpoint}, None))
56-
5762
logger.info(s"version $version has been run! There are ${routes.length} routes.")
58-
// specified response for OPTIONS request.
59-
private val corsResponse: Box[LiftResponse] = Full{
60-
val corsHeaders = List(
61-
"Access-Control-Allow-Origin" -> "*",
62-
"Access-Control-Allow-Methods" -> "GET, POST, OPTIONS, PUT, PATCH, DELETE",
63-
"Access-Control-Allow-Headers" -> "*",
64-
"Access-Control-Allow-Credentials" -> "true",
65-
"Access-Control-Max-Age" -> "1728000" //Tell client that this pre-flight info is valid for 20 days
66-
)
67-
PlainTextResponse("", corsHeaders, HttpStatus.SC_NO_CONTENT)
68-
}
69-
/*
70-
* process OPTIONS http request, just return no content and status is 204
71-
*/
72-
this.serve({
73-
case req if req.requestType.method == "OPTIONS" => corsResponse
74-
})
63+
64+
// OPTIONS / CORS is handled by Http4sApp.corsHandler — the Lift OPTIONS serve below is disabled.
65+
// private val corsResponse: Box[LiftResponse] = Full{
66+
// val corsHeaders = List(
67+
// "Access-Control-Allow-Origin" -> "*",
68+
// "Access-Control-Allow-Methods" -> "GET, POST, OPTIONS, PUT, PATCH, DELETE",
69+
// "Access-Control-Allow-Headers" -> "*",
70+
// "Access-Control-Allow-Credentials" -> "true",
71+
// "Access-Control-Max-Age" -> "1728000" //Tell client that this pre-flight info is valid for 20 days
72+
// )
73+
// PlainTextResponse("", corsHeaders, HttpStatus.SC_NO_CONTENT)
74+
// }
75+
// this.serve({
76+
// case req if req.requestType.method == "OPTIONS" => corsResponse
77+
// })
7578
}

obp-api/src/main/scala/code/api/util/APIUtil.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2875,7 +2875,10 @@ object APIUtil extends MdcLoggable with CustomJsonFormats{
28752875
case ApiVersion.v5_1_0 => LiftRules.statelessDispatch.append(v5_1_0.OBPAPI5_1_0)
28762876
case ApiVersion.v6_0_0 => LiftRules.statelessDispatch.append(v6_0_0.OBPAPI6_0_0)
28772877
case ApiVersion.`dynamic-endpoint` => LiftRules.statelessDispatch.append(OBPAPIDynamicEndpoint)
2878-
case ApiVersion.`dynamic-entity` => LiftRules.statelessDispatch.append(OBPAPIDynamicEntity)
2878+
// dynamic-entity endpoints migrated to Http4sDynamicEntity (wired into Http4sApp.baseServices).
2879+
// Keep the case label with an empty body so ApiVersion.`dynamic-entity` does NOT fall through
2880+
// to the ScannedApiVersion branch below (which would re-append it via ScannedApis).
2881+
case ApiVersion.`dynamic-entity` => // LiftRules.statelessDispatch.append(OBPAPIDynamicEntity)
28792882
case version: ScannedApiVersion =>
28802883
ScannedApis.versionMapScannedApis.get(version).foreach(api => LiftRules.statelessDispatch.append(api))
28812884
case _ => logger.info(s"There is no ${version.toString}")

obp-api/src/main/scala/code/api/util/http4s/ErrorResponseConverter.scala

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package code.api.util.http4s
22

33
import cats.effect._
44
import code.api.APIFailureNewStyle
5+
import code.api.JsonResponseException
6+
import code.api.util.APIUtil.JsonResponseExtractor
57
import code.api.util.ErrorMessages._
68
import code.api.util.CallContext
79
import net.liftweb.common.{Failure => LiftFailure}
@@ -73,30 +75,40 @@ object ErrorResponseConverter {
7375

7476
/**
7577
* Convert any error to http4s Response[IO].
76-
*
77-
* `JsonResponseException` carries a fully-formed Lift `JsonResponse` and is the
78-
* mechanism `APIUtil.anonymousAccess` (line 3517) and the after-authenticate
79-
* interceptor chain (Force-Error, JSON-schema validation, etc. — `APIUtil.scala:5064`
80-
* `afterAuthenticateInterceptors`) use to short-circuit a request with a synthesized
81-
* response from inside a `Future`. The Lift bridge handles it with
82-
* `case JsonResponseException(jsonResponse) => jsonResponse` in
83-
* `Http4sLiftWebBridge.runLiftDispatch`; without an equivalent case here,
84-
* EndpointHelpers-based handlers fall through to `unknownErrorToResponse` and
85-
* return 500 where production has always returned the synthesized status (e.g.
86-
* 400/403/444 for Force-Error scenarios). Reuse `liftResponseToHttp4s` so the
87-
* conversion matches the bridge byte-for-byte.
8878
*/
8979
def toHttp4sResponse(error: Throwable, callContext: CallContext): IO[Response[IO]] = {
9080
error match {
9181
case e: APIFailureNewStyle => apiFailureToResponse(e, callContext)
92-
case e: code.api.JsonResponseException => Http4sLiftWebBridge.liftResponseToHttp4s(e.jsonResponse)
82+
case JsonResponseException(jsonResponse) =>
83+
// Force-Error / JSON-schema validation (APIUtil.afterAuthenticateInterceptResult, applied
84+
// inside the auth/session-context chain) and dynamic-resource-doc permission errors
85+
// (NewStyle) are raised as JsonResponseException carrying a Lift JsonResponse. Lift's
86+
// OBPRestHelper catches these and returns the embedded response; mirror that here using
87+
// the JsonResponse's own status code (no OBP-prefix remapping).
88+
jsonResponse match {
89+
case JsonResponseExtractor(message, code) => jsonErrorResponse(code, message, callContext)
90+
case _ => unknownErrorToResponse(error, callContext)
91+
}
9392
case _ =>
9493
tryExtractApiFailureFromExceptionMessage(error) match {
9594
case Some(apiFailure) => apiFailureToResponse(apiFailure, callContext)
9695
case None => unknownErrorToResponse(error, callContext)
9796
}
9897
}
9998
}
99+
100+
/** Build a JSON error response using the supplied status code verbatim (used for
101+
* JsonResponseException, whose embedded JsonResponse already carries the final code). */
102+
private def jsonErrorResponse(code: Int, message: String, callContext: CallContext): IO[Response[IO]] = {
103+
val errorJson = OBPErrorResponse(code, message)
104+
val status = org.http4s.Status.fromInt(code).getOrElse(org.http4s.Status.BadRequest)
105+
IO.pure(
106+
Response[IO](status)
107+
.withEntity(toJsonString(errorJson))
108+
.withContentType(jsonContentType)
109+
.putHeaders(org.http4s.Header.Raw(CIString("Correlation-Id"), callContext.correlationId))
110+
)
111+
}
100112

101113
/** Old-style versions keep raw 400 codes — they never promote to 403/401/etc.
102114
* Mirrors the same set used in ResourceDocMiddleware.authenticate.

obp-api/src/main/scala/code/api/util/http4s/Http4sApp.scala

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,9 @@ object Http4sApp {
7474
private val v510Routes: HttpRoutes[IO] = gate(ApiVersion.v5_1_0, code.api.v5_1_0.Http4s510.wrappedRoutesV510Services)
7575
private val v600Routes: HttpRoutes[IO] = gate(ApiVersion.v6_0_0, code.api.v6_0_0.Http4s600.wrappedRoutesV600Services)
7676
private val v700Routes: HttpRoutes[IO] = gate(ApiVersion.v7_0_0, code.api.v7_0_0.Http4s700.wrappedRoutesV700Services)
77-
// Dynamic-entity data plane (operator-created entities under /obp/dynamic-entity/*).
78-
// Native http4s replacement for the Lift OBPAPIDynamicEntity dispatch; gated by the
79-
// same api_disabled_versions / api_enabled_versions machinery as the versioned routes.
80-
private val dynamicEntityRoutes: HttpRoutes[IO] = gate(ApiVersion.`dynamic-entity`, code.api.dynamic.entity.Http4sDynamicEntity.routes)
77+
// DynamicEntity runtime CRUD (/obp/dynamic-entity/*) — native http4s, replaces the Lift
78+
// OBPAPIDynamicEntity dispatch. Hongwei's PR #2815 implementation.
79+
private val dynamicEntityRoutes: HttpRoutes[IO] = gate(ApiVersion.`dynamic-entity`, code.api.dynamic.entity.Http4sDynamicEntity.wrappedRoutesDynamicEntity)
8180
// Dynamic-endpoint data plane (operator-created endpoints under /obp/dynamic-endpoint/*).
8281
// Phase-3a adapter: scoped Lift-Req shim that dispatches OBPAPIDynamicEndpoint directly,
8382
// keeping the runtime Scala codegen (which still emits Lift OBPEndpoint) unchanged.
@@ -135,9 +134,9 @@ object Http4sApp {
135134
.orElse(v140Routes.run(req))
136135
.orElse(v130Routes.run(req))
137136
.orElse(v121Routes.run(req))
137+
.orElse(dynamicEntityRoutes.run(req))
138138
.orElse(code.api.DirectLoginRoutes.routes.run(req))
139139
.orElse(code.api.AliveCheckRoutes.routes.run(req))
140-
.orElse(dynamicEntityRoutes.run(req))
141140
.orElse(dynamicEndpointRoutes.run(req))
142141
.orElse(Http4sLiftWebBridge.routes.run(req))
143142
}

obp-api/src/main/scala/code/api/util/http4s/RequestScopeConnection.scala

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
package code.api.util.http4s
22

3-
import cats.effect.{IO, IOLocal}
3+
import cats.effect.{Deferred, IO, IOLocal, Outcome}
44
import cats.effect.unsafe.IORuntime
55
import com.alibaba.ttl.TransmittableThreadLocal
66
import net.liftweb.common.{Box, Full}
77
import net.liftweb.db.ConnectionManager
88
import net.liftweb.util.ConnectionIdentifier
9+
import org.http4s.Response
910

11+
import code.api.util.APIUtil
1012
import code.util.Helper.MdcLoggable
1113
import java.lang.reflect.{InvocationHandler, Method, Proxy => JProxy}
1214
import java.sql.Connection
@@ -173,6 +175,51 @@ object RequestScopeConnection extends MdcLoggable {
173175
}
174176
}
175177

178+
/**
179+
* Wrap an http4s route IO in a request-scoped DB transaction.
180+
*
181+
* Installs a once-only lazy connection-acquisition scope (requestLazyAcquire): no real
182+
* connection is borrowed until the FIRST fromFuture call that touches the DB. The first
183+
* fiber to complete the inner Deferred wins; concurrent losers discard their connection
184+
* and share the winner's proxy, so all fibers use one underlying Connection / one
185+
* transaction. On success: commit then close. On error/cancel: rollback then close.
186+
* If no DB call was made: nothing to commit or close (pool unaffected).
187+
*
188+
* GET/HEAD must NOT be wrapped (they run on auto-commit vendor connections). Used by
189+
* ResourceDocMiddleware (v6/v7) and by services that build their own request scope
190+
* without the middleware (e.g. Http4sDynamicEntity).
191+
*/
192+
def withBusinessDBTransaction(io: IO[Response[IO]]): IO[Response[IO]] =
193+
Deferred[IO, (Connection, Connection)].flatMap { deferred =>
194+
// acquireOnce: idempotent across concurrent callers via the Deferred.
195+
val acquireOnce: IO[Connection] = for {
196+
realConn <- IO.blocking(APIUtil.vendor.HikariDatasource.ds.getConnection())
197+
_ <- IO.blocking { realConn.setAutoCommit(false) }
198+
proxy = makeProxy(realConn)
199+
ok <- deferred.complete((realConn, proxy))
200+
_ <- if (!ok) IO.blocking { try { realConn.close() } catch { case _: Exception => () } }
201+
else IO.unit
202+
p <- deferred.get.map(_._2)
203+
} yield p
204+
205+
requestLazyAcquire.set(Some(acquireOnce)).bracket(_ =>
206+
io.guaranteeCase { outcome =>
207+
deferred.tryGet.flatMap {
208+
case None => IO.unit // no DB calls — pool unaffected
209+
case Some((realConn, _)) =>
210+
requestProxyLocal.set(None) *>
211+
(outcome match {
212+
case Outcome.Succeeded(_) =>
213+
IO.blocking { realConn.commit() }
214+
case _ =>
215+
IO.blocking { try { realConn.rollback() } catch { case _: Exception => () } }
216+
}) *>
217+
IO.blocking { try { realConn.close() } catch { case _: Exception => () } }
218+
}
219+
}
220+
)(_ => requestLazyAcquire.set(None))
221+
}
222+
176223
/**
177224
* Returns the proxy for the current fiber, acquiring it lazily on first call.
178225
*

0 commit comments

Comments
 (0)