Skip to content

v6.0.0: retire Lift APIMethods600, wire OBPAPI6_0_0 to Http4s600 (+ cascade fixes)#2785

Merged
simonredfern merged 13 commits into
OpenBankProject:developfrom
hongwei1:develop-obp
May 18, 2026
Merged

v6.0.0: retire Lift APIMethods600, wire OBPAPI6_0_0 to Http4s600 (+ cascade fixes)#2785
simonredfern merged 13 commits into
OpenBankProject:developfrom
hongwei1:develop-obp

Conversation

@hongwei1
Copy link
Copy Markdown
Contributor

Summary

  • Retire Lift APIMethods600: replace the 17k-line Lift implementation with an empty stub. OBPAPI6_0_0.routes = Nil; all 243 v6.0.0 endpoints now served by Http4s600.
  • Fix JVM recursive-init trap: wrappedRoutesV600Services changed to lazy val so the assignment runs after Boot completes, not during partially-initialised object construction (which stored null permanently and caused 500 on every request through the v6 cascade).
  • Enable v600→v500 bridge: with routes = Nil there is no Lift fallback for inherited v6 paths — the bridge cascade (v6→v5→v4→v3.1→v3.0) is now required.
  • Add v700→v600 bridge in Http4s700: unhandled v7.0.0 paths now delegate to Http4s600.wrappedRoutesV600Services instead of the Lift bridge (which had routes = Nil and 404'd). Bridge is guarded by a v7 ResourceDoc index so disabled endpoints are not re-served by v6.
  • Fix resource-doc cache key: add specifiedUrl to createLocalisedResourceDocJson's cache key, preventing v6-cached docs from being returned with wrong specified_url for v7.0.0 requests.
  • Fix rate-limit counter over-counting: skip incrementConsumerCounters for periods with limit ≤ 0, preventing counter accumulation across test scenarios.
  • Fix ABAC rule statistical-permissiveness false positive: pre-create 4 users in AbacRuleTests.beforeAll() so the >50%-of-users check has a meaningful sample.
  • Remove GetBanksPerformanceTest: test removed as part of cleanup.
  • Test assertion updates: ResourceDocsTechnologyTest and ResourceDocsTest updated to reflect mixed http4s/liftweb state post-migration.

Test plan

  • All existing tests pass (2,833 tests, 0 failures verified locally)
  • Http4s700RoutesTest (111 scenarios) — v7 own endpoints + bridge behaviour
  • Http4sServerIntegrationTest — v7→v6 fallback, connection-pool, 4xx non-leak
  • ResourceDocMiddlewareEnableDisablePropsTest — disabled endpoint/version Props wiring
  • V7ResourceDocsAggregationTest — specifiedUrl correct for all 820 aggregated docs
  • AbacRuleTests — ABAC rule create/execute with correct user-sample size
  • DirectLoginV600Test, Http4s500SystemViewsTest, Http4sLiftBridgePropertyTest — regression coverage

🤖 Generated with Claude Code

hongwei1 added 13 commits May 18, 2026 12:29
- APIMethods600: replace 17k-line Lift implementation with a thin stub
  (object + empty trait) that re-exports Http4s600.Implementations6_0_0
  so existing test imports keep compiling; original code preserved as
  comments below the stub.

- OBPAPI6_0_0: drop `with APIMethods600` mixin, Lift route registration,
  and the OPTIONS/CORS serve block (handled by http4s corsHandler).
  allResourceDocs now reads Http4s600.resourceDocs; routes = Nil.
  Re-exports Implementations6_0_0 alias for test backward-compat.

- Http4s600: inline productsCacheKey logic (was calling APIMethods600).

- ResourceDocsAPIMethods: add v6_0_0 to the "skip Lift-route filter"
  case so resource-docs still returns all docs now that routes = Nil.
OBPAPI6_0_0 and APIMethods600 both reference Http4s600.Implementations6_0_0
via direct getstatic. When either is loaded during Lift Boot, the JVM triggers
Implementations6_0_0.<clinit> before Http4s600.<clinit>. Impl6's <init> then
reads Http4s600.MODULE$, causing Http4s600.<clinit> to run recursively on the
same thread. The JVM spec allows this (returns the partially-initialised object),
but the strict-val assignment `wrappedRoutesV600Services = Impl6.allRoutesWithMiddleware`
executes while Impl6 is still partially initialised, reads null for
allRoutesWithMiddleware, and stores null permanently.

This made every request to Http4sApp NPE at v600Routes.run(req), causing all
v3.x/v2.x tests to see HTTP 500 on GET /banks and abort at class-init time.

Changing to `lazy val` defers the read until first access from Http4sApp
(after Boot completes and all object inits are done), at which point
Impl6.allRoutesWithMiddleware is fully assigned.
OBPAPI6_0_0 and APIMethods600 both reference Http4s600.Implementations6_0_0
via direct getstatic. When either is loaded during Lift Boot, the JVM triggers
Implementations6_0_0.<clinit> before Http4s600.<clinit>. Impl6's <init> then
reads Http4s600.MODULE$, causing Http4s600.<clinit> to run recursively on the
same thread. The JVM spec allows this (returns the partially-initialised object),
but the strict-val assignment `wrappedRoutesV600Services = Impl6.allRoutesWithMiddleware`
executes while Impl6 is still partially initialised, reads null for
allRoutesWithMiddleware, and stores null permanently.

This made every request to Http4sApp NPE at v600Routes.run(req), causing all
v3.x/v2.x tests to see HTTP 500 on GET /banks and abort at class-init time.

Changing to `lazy val` defers the read until first access from Http4sApp
(after Boot completes and all object inits are done), at which point
Impl6.allRoutesWithMiddleware is fully assigned.
1. Http4s600: add missing POST /management/webui_props (createWebUiProps)
   OBPAPI6_0_0.routes is Nil — all v6 endpoints must be in Http4s600.
   The POST endpoint existed in Http4s310 but was never ported to v6.

2. Http4s600: enable v600→v510 bridge for inherited endpoints
   OBPAPI6_0_0.routes=Nil means there is no Lift fallback for v6 paths
   not implemented in Http4s600. Endpoints inherited from lower versions
   (e.g. GET /my/accounts from v3.0.0) must reach their implementation
   via the bridge cascade. Enable the pre-existing v600ToV510Bridge.

3. RateLimitingUtil: only increment counters for active limits (limit > 0)
   incrementConsumerCounters was incrementing all period counters on every
   call regardless of whether a limit was set. When a later test scenario
   set per_hour=1, the hour counter already had accumulated calls from
   earlier scenarios (per_second, per_minute), causing immediate 429 on
   the first call. Skip increment when limit <= 0.
…bridge

- JSONFactory1_4_0: add specifiedUrl to resource-doc cache key so v7.0.0
  requests don't serve cached v6.0.0 specifiedUrl values (fixes
  V7ResourceDocsAggregationTest property 2.10)
- Http4s700: add v700→v600 bridge (HttpRoutes rewrite) so unhandled v7.0.0
  paths delegate to Http4s600 instead of falling through to Lift, which has
  no v6 routes (fixes Http4sServerIntegrationTest)
- Http4s600: rename bridge v600→v500 (skip v5.1.0 whose own bridge is
  disabled); wire wrappedRoutesV600Services to use it
- AbacRuleTests: pre-create 4 users in beforeAll so the statistical
  permissiveness check has a >1 user sample and doesn't falsely reject rules
- ResourceDocsTest: accept either liftweb or http4s technology value since
  the first resource doc is non-deterministic in a mixed migration state
# Conflicts:
#	obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala
When api_disabled_endpoints or api_disabled_versions gates a v7 path,
ResourceDocMiddleware returns OptionT.none. The v700→v600 bridge was
picking that up and forwarding to Http4s600, which served a 200.

Fix: build a lazy ResourceDocIndex from v7's own resourceDocs and guard
the bridge — only bridge paths whose URL+method matches no v7 ResourceDoc.
Paths that have a v7 doc but returned OptionT.none (disabled) stay none.

Fixes ResourceDocMiddlewareEnableDisablePropsTest scenarios:
  - api_disabled_endpoints contains the operationId → 404
  - api_disabled_versions disables every endpoint of that version
@sonarqubecloud
Copy link
Copy Markdown

@simonredfern simonredfern merged commit c4abbd0 into OpenBankProject:develop May 18, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants