refactor: remove Lift Web framework — full http4s runtime + OIDC migration#2828
Open
hongwei1 wants to merge 68 commits into
Open
refactor: remove Lift Web framework — full http4s runtime + OIDC migration#2828hongwei1 wants to merge 68 commits into
hongwei1 wants to merge 68 commits into
Conversation
…overy off Lift
A1: add Http4sResourceDocAggregation (Lift-free cumulative per-version dedup);
repoint ResourceDocsAPIMethods doc source from OBPAPINxx to Http4sNxx;
force-init Implementations7_0_0 in Http4s700.allResourceDocs to fix
resource-docs aggregation (v7 now returns full aggregated catalog, 830 docs).
A2: drop `extends LiftRules.DispatchPF` from ScannedApis (discovery via ClassScanUtils).
A3: remove the 12+1 LiftRules.statelessDispatch.append calls in enableVersionIfAllowed.
Full suite green: 2998 tests, 0 failures.
…SON catch-all
B1: remove Http4sLiftWebBridge.routes from Http4sApp; replace with notFoundCatchAll
that returns {"code":404,"message":"..."} matching OBP error format.
B2: extract ensureStandardHeaders into Http4sStandardHeaders; add handleErrorWith
500 handler in httpApp; remove bridge traffic audit route.
B3: delete Http4sLiftWebBridge.scala and the 5 bridge-specific test files
(Http4sLiftBridgeTrafficTest, Http4sResponseConversionTest,
Http4sCallContextBuilderTest, Http4sResponseConversionPropertyTest,
Http4sRequestConversionPropertyTest).
Remove wrappedRoutesV500ServicesWithBridge from Http4s500 (used only by
the @ignore V500ContractParityTest, updated to use WithJsonNotFound variant).
Full suite green: 2902 tests, 0 failures.
Remove the Lift-typed partialFunction: OBPEndpoint first parameter from ResourceDoc across all 48 construction sites (45 main + 3 test files). All http4s call sites already passed null; dynamic-endpoint files passed dynamicEndpointStub which is now obsolete for this purpose. - ResourceDoc: remove partialFunction field; connectorMethods init → Nil - getAllowedEndpoints / getAllowedResourceDocs: deleted (no live callers) - getApiLinkTemplates: short-circuit to Nil (callers all commented out) - ResourceDocsAPIMethods case _: drop partialFunction.getClass filter - OBPRestHelper.registerRoutes: gut body to no-op - versionRoutesClasses local val: removed (unused after case _ change)
Remove dead Lift Web configuration that no longer has any effect since
all request dispatch is now handled by native http4s:
- LiftRules.addToPackages("code")
- LiftRules.liftRequest.append (H2 console gate, dev/test only)
- LiftRules.early.append (UTF-8 charset)
- LiftRules.explicitlyParsedSuffixes
- LiftRules.localeCalculator (S.findCookie/ObpS.param locale logic)
- LiftRules.supplementalHeaders (X-Frame-Options, already in Http4sStandardHeaders)
- S.addAround(DB.buildLoanWrapper) (no Lift requests to wrap)
- UsernameLockedChecker object + LiftSession.on{BeginServicing,SessionActivate,SessionPassivate}
(lockout is checked per-request in the http4s auth path)
- LiftRules.sessionInactivityTimeout
- Unused imports: LiftRules.DispatchPF, ObpS, SILENCE_IS_GOLDEN
Remove `routes: List[OBPEndpoint]` from VersionedOBPApis and ScannedApis traits, along with all override declarations and registerRoutes(routes,...) call sites across 27 aggregator files. The versionRoutes dispatch block in ResourceDocsAPIMethods (all branches returned Nil) is deleted. Dynamic endpoint/entity aggregators no longer import stale routes symbols from OBPAPI5_0_0.
…m APIMethods traits Delete aliveCheck.scala (replaced by AliveCheckRoutes) and AccountsAPI.scala (Boot dispatch commented out, entire body already commented). Remove extends RestHelper from DirectLogin (dlServe block deleted) and all APIMethods* companion objects + traits (self: RestHelper => self-type removed). Drop stale OBPEndpoint imports from aggregator files that no longer carry routes.
…ispatch - OBPRestHelper no longer extends RestHelper; removed apiPrefix, failIfBadJSON, failIfBadAuthorizationHeader, oauthServe, buildOAuthHandler, override serve, getEndpoints, RichStringList - registerRoutes simplified to a no-op stub (routes = Nil since C3c) - Boot: ConnectorEndpoints.registerConnectorEndpoints disabled — Lift dispatch path removed in Phase B, not yet migrated to http4s - ConnectorEndpoints: drop oauthServe import; make registerConnectorEndpoints a no-op matching Boot's disabled state - OBPAPIDynamicEndpoint / OBPAPIDynamicEntity: remove stale apiPrefix/registerRoutes imports
- Delete `type OBPEndpoint` from APIUtil; replace with inline
`PartialFunction[Req, CallContext => Box[JsonResponse]]` in the one
remaining live site (ConnectorEndpoints).
- Remove `dynamicEndpointStub` val (only set on commented-out dynamic
endpoint ResourceDocs; the http4s path never reads it).
- Delete `wrappedWithAuthCheck` method (~230 lines) — Lift-only auth
wrapper superseded by ResourceDocMiddleware.
- Retype `ApiRelation`, `InternalApiLink`, `CallerContext` case classes:
remove the `OBPEndpoint` fields (constructors are all commented-out at
call sites); keep `rel`/`requestUrl`/`caller: String` so the types and
their ArrayBuffer holders still compile.
- Delete `callEndpoint` helper (~45 lines) — Lift S.request / LAFuture
plumbing with no http4s equivalent and no live callers.
- Purge dead `/* DISABLED */` block + `filterDynamicObjects` helper from
APIMethodsDynamicEntity; drop the now-unused
`import net.liftweb.http.{JsonResponse, Req}`.
- Remove stale `import code.api.util.APIUtil.OBPEndpoint` from 12 version
aggregator files (OBPAPI1_3_0 … OBPAPI6_0_0, BG v1.3, BG v1.3 Alias).
Compilation clean after all changes.
…Methods300, DynamicEndpointHelper - AccountsHelper: remove dead getFilteredCoreAccounts/filterWithAccountType (only called from commented-out Lift code); drop net.liftweb.http.Req import - CustomAPIMethods300: remove extends RestHelper self-type; drop net.liftweb.http.rest.RestHelper import - DynamicEndpointHelper: replace extends RestHelper with implicit val formats; drop net.liftweb.http.rest.RestHelper import
…h own type Define APIUtil.HTTPParam (same shape as Lift's: name: String, values: List[String]) with a String-valued apply convenience constructor. Remove the lift-webkit HTTPParam import from 24 files; also drop dead S.request.headers fallback in getUserAndSessionContextFuture (Lift bridge removed in Phase B, always empty).
…) overload and S import The second overload was only reachable from the Lift dispatch path (removed in phase B). Cleans S, HTTPCookie, DirectLogin, Consumer, ResourceDoc, ObpS, and related dead imports.
…m I18NUtil Cookie-based locale selection was dead code in the http4s path (no Lift session). Simplified currentLocale() to only honour the PARAM_LOCALE query param, falling back to the configured default locale.
…am in CurrencyUtil and fx LiftRules is a Lift-Web type; replaced with standard getClass.getResourceAsStream so these files no longer depend on net.liftweb.http.
…ourceBundle in translatedErrorMessage
Removes LiftRules and TransientRequestMemoize from OBPRestHelper; translatedErrorMessage
now uses a plain Java ResourceBundle.getBundle("i18n.lift-core", locale) lookup, which
produces identical results without requiring a live Lift request scope.
…kie from search.scala searchProxy now returns JValue directly; searchProxyV300/StatsV300 return Box[JValue]; ESJsonResponse is a plain case class. Http4s200 updated to use the simplified API.
… ObpS to http4s stub OAuth.scala: dead unused import removed. Helper.scala: CGLIB proxy of Lift's S replaced with a plain Scala object whose methods return Empty/default, matching the actual runtime behaviour on the http4s path where the Lift S scope is never initialised.
Deletes connectorEndpoints lazy val, JsonAny object, and extends RestHelper — all dead since Phase B removed the Lift bridge. Removes JsonResponse/Req/RestHelper imports from net.liftweb.http.
… ResourceDocsAPIMethods Replace LiftRules.loadResourceAsString with getClass.getResourceAsStream; remove unused S, InMemoryResponse, PlainTextResponse imports.
…IUtil
- Remove bulk `net.liftweb.http._` import; keep narrow `JsonResponse` only
- Replace `LiftRules.getResource` with `getClass.getResourceAsStream`
- Replace `JsRaw("")` (4 sites) with `JNull`
- Replace `s.request` in GatewayLogin.validator call with `Empty` (S never
initialised in http4s path)
- Delete dead Lift-dispatch helpers: futureToResponse, futureToBoxedResponse,
getFilteredOrFullErrorMessage, scalaFutureToJsonResponse,
scalaFutureToBoxedJsonResponse, getRequestBody(Box[Req])
- Delete RestContinuation.async-based internal async utilities
- Stub S-dependent helpers to http4s-safe defaults (getCorrelationId,
getRemoteIpAddress, httpMethod, getUserAndSessionContextFuture etc.)
…ogin
- GatewayLogin: change validator/getAllParameters signature from Box[Req] to
Box[String] (Authorization header value); remove import net.liftweb.http._
- GatewayLogin.getUser: replace S.request with Empty (S never initialised
in http4s path)
- dauth: replace S.getRequestHeader with Empty; remove import net.liftweb.http._
- directlogin: replace S.request usages with safe defaults ("GET"/"POST");
getAllParameters returns MissingDirectLoginHeader error (Lift bridge removed);
remove import net.liftweb.http._
- APIUtil: update GatewayLogin.validator call to pass cc.authReqHeaderField
(the actual Authorization header value from CallContext)
…esponse Define a lightweight code.api.JsonResponse case class (body: JValue, headers, cookies, code) to replace Lift's net.liftweb.http.JsonResponse as the common error-carrier type. The http4s middleware (ErrorResponseConverter, JsonResponseExtractor) already read body+code so the switch is transparent. - OBPRestHelper: add JsonResponse + JsonResponse.apply(JValue,Int) definitions; remove import net.liftweb.http.JsonResponse - APIUtil: import code.api.JsonResponse; replace .toJsCmd with compactRender; remove .asInstanceOf[JsonResponse] casts - NewStyle: import code.api.JsonResponse - Http4s500: replace .toResponse.data with compactRender(body) - Boot: rename Lift's JsonResponse → LiftJsonResponse to resolve import ambiguity; update exceptionHandler + uriNotFound call sites - AuthUser: rename Lift's JsonResponse → LiftJsonResponse to resolve import ambiguity (AuthUser itself does not use JsonResponse directly)
Replace net.liftweb.http.provider.HTTPParam with the Lift-free drop-in replacement defined in APIUtil so test-compile passes after the net.liftweb.http removal in main sources.
…m Boot.scala
Lift no longer handles any HTTP requests (bridge removed in Phase B), so
the exceptionHandler and uriNotFound prepend blocks are dead code. Also
remove the two private helpers (showExceptionAtJson, sendExceptionEmail)
that were only called from those blocks.
Simplify the import from 'net.liftweb.http.{JsonResponse => ..., _}'
to 'import net.liftweb.http.LiftRules' — only LiftRules.unloadHooks,
LiftRules.context, and LiftRules.getResource remain active.
…BPRestHelper The implicit unwrapped Box[JsonResponse] into JsonResponse for the Lift dispatch path. Since dispatch is fully replaced by http4s and no Lift endpoint is served any more, this implicit is never triggered. Removing it eliminates the last live usage of Box[Failure/Empty/ParamFailure] inside OBPRestHelper and shrinks the class surface.
…n hooks
Replace LiftRules.unloadHooks.append with Runtime.getRuntime.addShutdownHook
so DB/Redis/gRPC cleanup fires on JVM termination regardless of whether
the Lift servlet lifecycle runs (it does not in the IOApp deployment).
Replace LiftRules.context.path (servlet context path) with '' since
http4s runs at root and the old servlet-context fragment was always
empty in OBP deployments.
Replace LiftRules.getResource('/') with getClass.getClassLoader.getResource
so the use_custom_webapp copy logic works without a servlet container.
Boot.scala no longer imports net.liftweb.http — only AuthUser.scala
(C5 scope) and commented-out legacy code retain that dependency.
S.request is always Empty in the http4s path (no Lift session), so the authorization and directLogin variables derived from it were always Empty. Replace with literal Empty to eliminate the S.request dependency in the active code path — portal redirect methods (logout, signup) still use S.request but are dead code in API-only mode.
….http imports Remove the three net.liftweb.http imports (S.fmapFunc, wildcard http, sitemap.Loc.*) from AuthUser.scala. All callers were dead portal code: - Remove _toForm overrides in MyFirstName/MyLastName/userName/MyPasswordNew/email (used fmapFunc — now gone, enabling import removal) - Remove S.notice/S.error side-effect calls from sendValidationEmail, validateUser, actionsAfterSignup (no-op in API-only http4s path) - Remove S.? i18n lookups; replace with literal fallback strings - Delete failedLoginRedirect SessionVar (no external callers since OAuthAuthorisation.scala setter was commented out) - Delete userLoginFailed (no callers anywhere in main sources) - Gut signup/loginMenuLocParams to empty/Nil stubs - Simplify logout to logoutCurrentUser + net.liftweb.http.S.redirectTo (one fully-qualified call, no import needed; keeps Nothing return type required by ProtoUser trait contract) Remaining net.liftweb.http reference in AuthUser is the single net.liftweb.http.S.redirectTo in logout — unavoidable without replacing MegaProtoUser/MetaMegaProtoUser (out of scope per C5 landmine note).
…ridge comments, rename test files - OBPRestHelper: Delete the `registerRoutes` stub, which has no active callers. - Http4sApp: Replace residual "Lift bridge" / "Lift fallback" phrasing within comments for `corsHandler`, `dynamicEndpoint`, `UK OB`, and `body-caching` with accurate descriptions (referring to the http4s version-cascade bridge and `notFoundCatchAll`; the actual Lift bridge has been removed). - Http4sServerIntegrationTest: Correct "Lift bridge" phrasing in scenario names to "http4s version-cascade"; remove references to non-existent documentation files. - Http4sLiftBridgePropertyTest → Http4sNativeRoutingPropertyTest: Remove redundant scenarios 7.1/7.2/7.4, which completely overlap with Properties 6.6/9.6; Replace all Lift bridge-related phrasing in the class name, Tags, and `feature`/`scenario` descriptions with http4s native routing / version-cascade terminology; assertions remain unchanged. - CLAUDE.md: Synchronize updates to two references pointing to old filenames/class names.
…ge removal The Lift fallback bridge (Http4sLiftWebBridge) was removed, but CLAUDE.md still described it as a live part of the routing chain in 6 places. Updated to reflect the real Http4sApp.baseServices chain, which terminates in notFoundCatchAll (JSON 404) with no Lift fallback: - Request priority chain: replaced the obsolete short chain with the actual route order; stated explicitly there is no Lift fallback. - system-views empty-segment gotcha: request falls through to notFoundCatchAll, not the Lift bridge. - bridge-cascade hijack note: rewritten as past tense; the Lift dedup safety net is gone, un-migrated overrides now cascade to older http4s handlers or 404. - API1_2_1Test perf note: now fully native http4s. - per-version completeness table: all versions show 0 active Lift handlers; noted v3.1.0 getMessageDocsSwagger / getObpConnectorLoopback are http4s-served. - v6.0.0 migration note: wired as v600Routes; dropped "ahead of the Lift bridge". Documentation only; no source changes, no behavior change.
…ders
68 single-line ignore(...) {} placeholders sit inside if (transactionRequests_enabled == false) { ignore } else { scenario } blocks (one gated on messageQueue.createBankAccounts). The prop defaults to false in tests, so these registered as IGNORED on every run while the real scenarios never executed - pure report noise. Comment them out, leaving the conditional structure intact (empty if-branch); the else-branch scenarios still run when the prop is enabled. The set of executed scenarios is unchanged and the full suite stays green (4/4 shards). Counts: v4 TransactionRequests 31, v2.1 TransactionRequests 20, v4 TransactionRequestAttributes 10, v4 MakerChecker 4, BankAccountCreationListener 2, API1_2_1 1.
The After column still showed ResourceDoc(null, ..., http4sPartialFunction = Some(root)). The null first-arg (OBPEndpoint partialFunction) was removed in f1d3544; align with the current signature (first param implementedInApiVersion), matching the CLAUDE.md Rule 1 fix.
…placeholders" This reverts commit 5e3752f.
…suite) ServerSetupWithTestData and ServerSetupWithTestDataAsync now skip creating the per-scenario 100 transactions + 200 transaction-requests unless the suite actually reads them (suitesNeedingTransactionData whitelist, matched by simple class name). The ~235 suites that never touch this data save ~300 DB writes per scenario. Full suite ALL SHARDS PASSED (5m39s vs 7m16s baseline) with per-test time down across the board. The only surefire flakies are pre-existing Redis-contention rate-limit/cache tests (4 shards share one Redis DB), unrelated to this change.
… flakiness Parallel test shards shared one Redis DB, so rate-limit counters and cache keys collided across shards and each scenario's FLUSHDB wiped other shards' state - causing intermittent '429 did not equal 200' and cache-test failures. Each shard now sets a distinct api_instance_id (OBP_API_INSTANCE_ID=shard_N), which flows through Constant.getGlobalCacheNamespacePrefix into every Redis key, isolating shards on the shared Redis. wipeTestData replaces FLUSHDB with Redis.deleteKeysByPattern(namespace + '*') so a shard only clears its own keys. (The earlier JedisPool DB-index approach was reverted: its 6-arg overload changed connection behaviour and broke ~20 suites; this key-namespace approach does not touch JedisPool.) Full suite ALL SHARDS PASSED (5m4s), ZERO 429s, all 4 Redis-contention suites green - down from 5 flaky to 1 unrelated pre-existing CounterpartyTest 'head of empty list'.
…nistic failure) CounterpartyTest reads otherAccountsJson.other_accounts.head (CounterpartyTest.scala:38); the other-accounts/counterparties are derived from beforeEach-created transactions' metadata. It was missing from suitesNeedingTransactionData, so after the on-demand-data change it received no transactions, other_accounts was empty, and .head threw 'head of empty list' deterministically — masked as flaky because scalatest did not halt the build. Adding it to the whitelist restores its transaction data. Full suite now truly green: ZERO surefire failures across all 4 shards (5m30s).
The Lift -> http4s migration is complete: every API version (v1.2.1 through
v7.0.0) is served by http4s, so the "http4s vs Lift" framing of the per-test
speed report is misleading -- there is no Lift HTTP code left. The "Lift vN"
labels described which API version a suite tests, not which framework runs it.
Rework test_speed_report.py to report two orthogonal, accurate views:
1. By execution model -- unit/pure (no server) vs integration (embedded
server). Integration is detected by scanning obp-api/src/test/scala for
test classes that extend ServerSetup, plus an explicit list of
self-starting http4s server suites. Degrades gracefully (counts suites as
integration) when sources are absent. This is the real migration KPI:
logic that used to need a running server is now pure unit-tested.
2. By API version -- API v1..v7, all served by http4s. Drops the "Lift vN"
labels.
Rename the report job step in both workflows accordingly. No test code is
changed; this only affects how the CI report classifies and labels suites.
The Lift -> http4s HTTP-layer migration is complete, so the two large migration-era property suites that verified "http4s behaves like Lift" are now redundant with regular coverage: - Http4sNativeRoutingPropertyTest (2255 lines): cross-cutting routing-layer properties (header injection, error JSON format, dispatch, cross-version exception format) are enforced by single shared middleware (Http4sStandardHeaders, ErrorResponseConverter) and already verified by Http4s700RoutesTest's "response headers" feature + Http4sServerIntegrationTest; its auth scenarios duplicate DirectLoginTest / gateWayloginTest. - AuthenticationPropertyTest (1682 lines): the lockout / login-attempt / provider-routing logic is covered concretely by AuthenticationRefactorTest (21 scenarios) plus DirectLoginTest / LockUserTest / VerifyExternalUserCredentialsTest. Http4sServerIntegrationTest is KEPT as the genuine end-to-end smoke layer. Also consolidate the duplicated v5.0.0 SystemViews suites: port the getSystemViewsIds 401/403/200 scenarios into the http4s-native Http4s500SystemViewsTest and remove the Lift-era SystemViewsTests (both exercised the same, now-http4s, handlers). Clean a stale suite reference in test_speed_report.py. Verified: full obp-api test-compile passes (no breakage from deletions) and Http4s500SystemViewsTest is green (16/16, including the 3 ported scenarios).
Follow-up cleanup after the migration-era property-suite removal:
- Http4s700RoutesTest: remove the empty feature("Http4s700 routing priority")
block (dead scaffolding -- a comment but no scenarios).
- V7ResourceDocsAggregationTest: the aggregation bug these scenarios were
written against is fixed and they now pass, so rename the misleading
"(EXPECTED TO FAIL)" / "(MAY FAIL)" scenario titles and reword the
"MUST FAIL on unfixed code" scaladoc/info lines to describe them as the
regression guards they now are. Assertions unchanged.
No behavior change; obp-api test-compile passes.
Optimize build_container.yml (push) and build_pull_request.yml (PR) to cut wall-clock from ~21min toward ~10min: - compile: add actions/cache for target/classes+test-classes+scala-2.12 (Zinc incremental analysis) and drop `clean`, so unchanged sources are not recompiled (full ~238s -> incremental ~80-120s on small diffs). Unique cache key per commit with a zinc- restore-keys prefix fallback to the latest cache. - test: split the 4-shard matrix into 8 rebalanced shards (v4_0_0 isolated as the bottleneck package; catch-all moved to shard 8) to lower the slowest shard from ~562s toward ~290s. - test: run 'mvn scalatest:test -pl obp-api -DfailIfNoTests=false' (goal-only) instead of 'mvn test', bypassing the compile/test-compile lifecycle phases (classes already supplied via the compiled-output artifact), matching run_tests_parallel.sh. - build_pull_request.yml: remove the 'if: github.repository == OpenBankProject' guard on the compile job so PR builds run on this fork instead of skipping the whole pipeline.
The HTTP-layer Lift -> http4s migration left 95 net.liftweb.http references in obp-api/src; this removes all of them, reaching ZERO. - AuthUser.scala: delete the dead `override def logout` (the last active net.liftweb.http call, S.redirectTo). It had zero callers -- Lift SiteMap / Menu / login UI is disabled (API-only mode) so Lift never invoked it; http4s logout uses the inherited AuthUser.logoutPath via GET /users/current/logout-link, which is unaffected. Dropping the override lets the inherited (unreachable) MegaProtoUser.logout apply. - 91 commented-out `//import net.liftweb.http...` lines across 80 files: dead import comments in the top-of-file import region, removed via an anchored line-precise pass that cannot touch any ResourceDoc/endpoint comment block. - 3 doc-strings (OBPRestHelper, DynamicCompileEndpoint, APIUtil): reworded to drop the net.liftweb.http reference while keeping the description. Verified: grep -rn "net.liftweb.http" obp-api/src -> 0; obp-api test-compile BUILD SUCCESS; ResourceDoc parity audit byte-identical before/after; DirectLoginTest + AuthenticationRefactorTest green.
The previous commit switched test shards to 'mvn scalatest:test' (goal-only), which skips the process-resources phase. The test.default.props generated by the Setup props step is therefore never copied into target/classes (the classpath), so ScalaTest's Props lookup fails, Constant init throws, suite discovery aborts: 0 tests run but the build reports SUCCESS (false green; shard Run-tests step was 15s vs the expected ~258s). Revert to mvn test: its process-resources phase places the props on the classpath, while the touch trick + cached target/ keep Zinc from recompiling so lifecycle overhead stays minimal. The 8-shard split and Zinc compile cache (the real speedups) are retained.
…eanup The dead AuthUser.logout override (the last vestigial Lift HTTP call) and the 91 commented-out import lines were removed, so OBP .scala source now has zero Lift HTTP references. Update the milestone note (drop the stale "one vestigial S.redirectTo survives / cleanup candidate" wording) and correct the remaining-Lift line to list the deliberately-kept non-web libraries (lift-mapper / lift-json / lift-common / lift-util) instead of only lift-mapper.
…eep props) mvn test spends ~208s/shard on the multi-module compile/testCompile lifecycle (Zinc full scan even when nothing recompiles + the obp-commons reactor). Switch to 'mvn process-resources scalatest:test -pl obp-commons,obp-api': process-resources still copies the dynamically generated test.default.props onto the classpath (fixing the false green that plain goal-only scalatest:test caused), while the compile/testCompile phases are skipped (classes come from the compiled-output artifact + touch trick). Incremental-compile (no-clean) cache verified safe: a deletion experiment showed scala-maven-plugin 4.8.1 removes orphan .class files on incremental rebuild (no residual, no jar leakage), so the Zinc cache is retained. Expected: slowest shard ~450s -> ~250s, wall-clock ~14min -> ~10min.
Experiment only (build_container.yml). Splits the 8-shard matrix into 12 to measure whether more shards reduce wall-clock. Hypothesis: NO, because the floor is compile (~350s, serial) + the two unsplittable big packages — code.api.v4_0_0 (~254s) and code.api.v1_2_1 (single API1_2_1Test suite, ~242s). v1_2_1 and v4 each get their own shard here; if wall-clock stays ~10min this proves shard count is no longer the bottleneck. Will revert to 8 if not faster (more shards also = more redis docker-pull flakiness).
This reverts commit b204432.
…ache experiments) Both experiments measured on real CI runs and rejected: - 12 shards (run 26859975836): wall-clock 630s vs 8-shard 622s -- NOT faster, even slightly slower. v4_0_0 (~254s) and v1_2_1 (single API1_2_1Test suite, ~281s) are unsplittable by package prefix, so each just becomes its own shard's floor; more shards only add redis docker-pull flakiness. - Zinc incremental cache (no-clean): a zero-source-change rebuild was still 333s vs a full 350s -- only ~17s saved, because compile is dominated by Maven startup, dependency resolution, packaging the ~297MB fat jar, and install, not Scala compilation. Not worth the stale-risk class of no-clean builds. Final config: 8 shards + goal-only (process-resources scalatest:test, ~208s/shard saved) + mvn clean install. ~10min wall-clock, the floor set by compile (~350s serial) + the largest unsplittable test shard.
…light shards The 10x stability run showed shard2 (v1_2_1+berlin+management+metrics) was the slowest at 314s while shards 7/8 sat at 87s/103s. v1_2_1 (single API1_2_1Test suite) alone is ~281s; the extra packages pushed it to 314s and set the critical path. Isolate v1_2_1 on its own shard; move berlin to shard 7 and management/metrics to shard 8 (both well under 281s). Expected slowest test shard 314s -> ~281s, wall-clock ~676s -> ~640s. Same total test count, no package dropped.
The 10x stability run showed code is fully stable (10/10 with 0 test failures) but 1/10 runs had a shard fail at Initialize containers -- the redis service image pull from docker.io timed out. Pin to redis:7-alpine (~30MB vs latest ~140MB): smaller image = shorter pull = smaller timeout window, plus a fixed version for reproducibility. OBP uses only basic redis commands (rate-limit counters, cache), fully supported by alpine.
hongwei1
changed the title
…ter)
compile was 315s, ~100s of which is maven-shade packaging the 297MB executable fat
jar -- which the test shards do not use (they only need target/classes). Make shade
skippable (root pom <shade.skip> property, default false so local/release builds are
unchanged; obp-api shade <skip>${shade.skip}</skip>), then:
- compile job: add -Dshade.skip=true -> only compiles classes (~315->~215s)
- new parallel jar job (needs compile, runs alongside test shards): downloads the
compiled classes and runs shade to build the fat jar -> ${sha} artifact, off the
test critical path
- docker: needs [test, jar], pulls the fat jar from the jar job
- build_pull_request.yml: compile -Dshade.skip=true, no jar job (PR only runs tests)
Expected: critical path 607 -> ~500s (compile 215 + slowest test shard 275 + report).
…100s faster)" This reverts commit b2cf02e.
…cess-path test
The Lift->http4s teardown removed S.addAround(DB.buildLoanWrapper), which gave each
request a single DB transaction. The OIDC callback runs synchronous Lift Mapper writes
inside IO.blocking and does not pass through ResourceDocMiddleware's
withBusinessDBTransaction (the request-scoped proxy/TTL is null on the blocking thread),
so its six provisioning writes (resource user, auth user, entitlements, consumer, OIDC
token, DirectLogin token) each auto-committed independently.
M1: wrap the provisioning block in DB.use(DefaultConnectionIdentifier) so all writes
share one connection and a thrown DB error rolls the whole set back -- faithfully
restoring the removed buildLoanWrapper semantics (it likewise committed on
logical-failure returns and only rolled back on a thrown exception). Token exchange and
JWKS validation stay outside the transaction so no connection is held during remote HTTP.
M2: add Http4sOpenIdConnectSuccessTest -- a self-contained success-path test that stands
up a local stub provider (token + JWKS endpoints), signs an RS256 id_token with Nimbus,
drives the callback in-process, and asserts 200 {token} plus that the resource user was
provisioned. This also exercises the M1 transaction wrapping.
Also fix a stale comment that referenced the removed Lift bridge.
…d connector-export endpoint
NEW-1 (test coverage hole): the 8-shard runner matches suites by FQN prefix
(-DwildcardSuites). The catch-all marks the parent package code.api as "covered" the
moment any child (code.api.v4_0_0, ...) is assigned, so it never appends bare code.api;
test classes sitting DIRECTLY in package code.api therefore run only if a shard filter
prefixes them explicitly. AliveCheckRoutesTest, Http4sOpenIdConnectRoutesTest and
Http4sOpenIdConnectSuccessTest matched no filter and ran in NO shard -- the OIDC tests
this branch added (incl. the success-path test) were never executed by CI. Add
code.api.AliveCheckRoutesTest and code.api.Http4sOpenIdConnect to shard 8's test_filter in
both build_pull_request.yml and build_container.yml. Verified locally that the prefix
discovers and runs all 6 OIDC scenarios.
NEW-2 (known gap): record the prop-gated connector.name.export.as.endpoints
(/connector/{methodName}) Lift endpoint -- removed with the Lift teardown, not ported to
http4s, no replacement -- as a known gap in LIFT_HTTP4S_MIGRATION.md so it isn't silently
lost.
Boot registered two separate Runtime.getRuntime.addShutdownHook threads -- DB/Redis close (early in boot) and gRPC server.stop() (in the grpc.server.enabled block). The JVM runs all shutdown hooks CONCURRENTLY with no ordering guarantee, so with gRPC enabled the two could race: a still-draining gRPC in-flight request touching the DB while the connection pool is being closed. Combine into a single ordered hook: stop gRPC first (drains in-flight), then close DB, then Redis -- each step guarded so one failure doesn't skip the rest. The gRPC server is now held in an Option (grpcServerOpt); when disabled the hook still closes DB/Redis. object ToSchemify (which hosts the gRPC start block) now extends MdcLoggable so the hook can log step failures. Only manifested with grpc.server.enabled=true (default false).
…ion-state caveat Security: exchangeAuthorizationCodeForTokens logged client_secret (inside `data`), the raw token-endpoint response, and id/access/refresh token values at debug level. Scrubbed all four sites to log only non-sensitive metadata (endpoint URL, success flag, tokenType/expiresIn/scope). Doc: portal-login removal left sessionState="" while openid_connect.check_session_state defaults to true (fail-closed), so a real callback state is rejected with 401 unless deployments set it false. This fail-closed default is intentional (locked by Http4sOpenIdConnectRoutesTest), so it is kept as-is; added a CONFIG CAVEAT comment in code + sample.props so operators know to set false. Verified: Http4sOpenIdConnectRoutesTest + Http4sOpenIdConnectSuccessTest + DirectLoginTest + gateWayloginTest green (22/22).
The 8-shard layout puts the catch-all on shard 8, but the log line still
hardcoded "Catch-all extras added to shard 4" (leftover from the old 4-shard
layout), so the run log misreported which shard absorbed the unassigned
packages (code.external, code.setup, com.openbankproject.commons.util). Use
${{ matrix.shard }} so it prints the actual shard. Log text only — no behavior
change; EXTRAS was already appended to the correct catch-all shard.
…s_parallel.sh gtimeout only exists on macOS (Homebrew coreutils); on Linux the binary is timeout, so every shard failed immediately with 'gtimeout: command not found' (rc 127) before mvn ran. Detect whichever binary is present.
|
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.
Remove Lift Web framework — full http4s runtime
Final teardown stage of the Lift → http4s migration. The runtime Lift Web framework is now entirely removed. Endpoint migration for every API version landed in earlier PRs; this PR removes the Lift scaffolding kept alive only as a fallback (the bridge) plus the last non-endpoint Lift surfaces (OpenID Connect callbacks, Boot lifecycle, the REST-helper base, portal-form code in AuthUser, aliveCheck).
After this PR there is no
net.liftweb.httpanywhere in source — 0 references (the final comment lines inAPIUtil.scalaand the last live call,AuthUser.logout'sS.redirectTo, were both removed). Lift libraries still used are non-web and out of scope:lift-json,lift-common(Box),lift-util,lift-mapper(ORM).This is an in-place migration — no API version bump (versioning is tech-agnostic; framework swaps don't change signatures).
Scope: 230 files, +2,151 / −12,408 (net −10k), 67 commits.
1. Lift Web teardown
Http4sLiftWebBridge.scalaDELETED (−583). Was the fallback handing unmatched routes to Lift dispatch. Unmatched/obp/*now terminates atnotFoundCatchAllinHttp4sApp(JSON 404). The chain is pure http4s.Boot.scala(−248). AllLiftRules.*removed: unload hooks →Runtime.getRuntime.addShutdownHook; charset/suffix/locale calculators dropped (API-only);supplementalHeaders(X-Frame-Options) →Http4sStandardHeaders;DB.buildLoanWrapper, exception handler,uriNotFound, OIDC dispatch removed.OBPRestHelper.scala(−562). No longer extends LiftRestHelper. Dropped thejsonResponseBoxToJsonResponseimplicit,failIfBadJSON, Lift JSON marshalling;JsonResponseis now a lightweight local carrier; localization via plainResourceBundle.getBundle.APIUtil.scala(−712). RemovedResourceDoc.partialFunction(LiftOBPEndpoint) andwrappedWithAuthCheck(auth now in http4s middleware);S.request-derived values replaced;HTTPParamre-implemented locally.AuthUser.scala(−219). Portal-form_toForm/S.notice/S.redirectTomachinery purged (including the finallogoutoverride — dead code, Lift SiteMap/Menu being disabled); now a data model only.getCurrentUserreturnsEmpty.aliveCheck.scalaDELETED → native http4sAliveCheckRoutes.scala(wired inHttp4sApp.baseServices).AccountsAPI.scalaDELETED — a commented-out, non-functional Lift stub.2. OpenID Connect → http4s
Http4sOpenIdConnect.scala(+391). Migrates the 3 callback routes (/auth/openid-connect/callback[-1|-2], GET+POST) from Liftserve{}dispatch. Wired inHttp4sAppafter DirectLogin, beforenotFoundCatchAll, gated byopenid_connect.enabled(default off).{"token": "..."}) instead of a Lift session redirect — stateless, suits mobile/SPA clients. Oldopenidconnect.scalafully commented out.client_secretand id/access/refresh tokens. Config caveat:openid_connect.check_session_statedefaultstrue(fail-closed); with portal-login gone there is no server-sidestateto compare, so live deployments must set itfalse(documented in code +sample.props).IO.blocking(provider token exchange + JWT validation + DB provisioning are blocking) — preserves prior behaviour but pins a compute thread for the call's duration. Covered byHttp4sOpenIdConnectRoutesTest(5 routing/failure scenarios) andHttp4sOpenIdConnectSuccessTest(success path).3. New http4s util surfaces
Http4sStandardHeaders.scala(NEW). Correlation-ID, no-cache headers,X-Frame-Options: DENY— extracted from the deleted bridge, applied to every response.Http4sResourceDocAggregation.scala(NEW +116). Lift-free resource-doc catalog: dedup by (requestUrl, verb) keeping the newest version; replaces the oldOBPAPINxx.allResourceDocsLift-dispatch ancestry.4. v7 duplicate-endpoint removal
19 duplicate POC endpoints removed from
Http4s700.scala(−750) → cascade to v6/v4 twins.scripts/remove_duplicate_v7_endpoints.pyis the removal tool.5. CI / test infra
build_pull_request.yml): isolatev1_2_1andv4; rebalance berlin/mgmt/metrics to light shards. A catch-all shard absorbs any package not explicitly assigned (verified at runtime:code.external,code.setup,com.openbankproject.commons.util).mvn test→mvn process-resources scalatest:test—process-resourceskeeps the test props on the classpath, so a red test actually fails the build (avoids the goal-only false-green where 0 tests run but BUILD SUCCESS).redis:7-alpine.test_speed_report.pyreframed "Lift vs http4s" → "unit/pure vs integration" (all http4s now).run_tests_parallel.sh(+330) — local mirror of the sharded CI runner.6. Test cleanup
Deleted migration scaffolding now dead: bridge-conversion suites (
Http4sLiftBridge*,Http4sRequest/ResponseConversion*,Http4sCallContextBuilderTest); the cross-cutting property suitesHttp4sNativeRoutingPropertyTest(−2255) (its header / error-format / dispatch properties are guaranteed by the shared middleware and already verified byHttp4s700RoutesTest) andAuthenticationPropertyTest(the lockout / login-attempt / provider logic is covered concretely byAuthenticationRefactorTest); v5 scaffolding (V500ContractParityTest,SystemViewsTests— itsgetSystemViewsIds401/403/200 scenarios ported intoHttp4s500SystemViewsTest,CardTest,Http4s500RoutesTest). AddedRetiredApiStandardsTest(guards retired endpoints stay retired),Http4sOpenIdConnectRoutesTest(5 scenarios) andHttp4sOpenIdConnectSuccessTest.7. Docs
LIFT_HTTP4S_MIGRATION.md,CLAUDE.md,FAQ.md,README.mdupdated to reflect the completed migration;todo_remove_duplicate_v7.0.0_endpoints.mddeleted.Reviewer notes
/obp/*path Lift used to serve now silently 404s atnotFoundCatchAll(the bridge previously caught these).IO.blockingis a known limitation; exceptions surface as JSON, not a hung thread.getCurrentUser → Empty: nothing in the runtime should rely on a Lift-session current user (portal is gone).LiftRules.unloadHooks(DB/connection cleanup).Verification
./run_tests_parallel.sh, default 4 shards): all shards green, ~5 min.grep -rn "net.liftweb.http" obp-api/src/main/scala→ 0 matches.