Lift to Http4s migration#2774
Merged
Merged
Conversation
…narios to Http4sServerIntegrationTest
- Migrated endpoint count 27 → 45, full list updated with trading, market, settlement, organisation, and diagnostic endpoints - Test scenario count 93 → 111 in Http4s700RoutesTest - Add IdempotencyMiddleware to key files - CI profile: add V7ResourceDocsAggregationTest (intentionally failing), update Http4s700RoutesTest count, note timings are pre-merge - OBP-Trading TODO: replace stale pending-decision note with current state - Add V7ResourceDocsAggregationTest to known-failing tests list - Add working style rule: no Co-Authored-By trailers in commits
…they don't belong there. The Lift implementation in APIMethods130 already serves them correctly at /obp/v1.3.0/
…o getResourceDocsList for all versions and deduplicate by (verb, url)
All 323 API1_2_1Test scenarios now pass. Key fixes required: - ErrorResponseConverter: extract only failCode/failMsg from exception JSON to avoid parse failures on complex ccl field - NewStyle.moderatedOtherBankAccount: recover NoSuchElementException → 400 JSON so counterparty-not-found returns 400 instead of 500 - Http4sSupport: add withViewCreated helper (201 Created) - Http4s121: new file — 60+ v1.2.1 endpoints in http4s Http4s121 specifics: - POST counterparty alias/metadata endpoints use withViewCreated (201 not 200) - Descriptions no longer include userAuthenticationMessage(false) where AuthenticatedUserIsRequired must remain in the error list (401 fix) - View/account management endpoints use non-standard URL template vars (BANK_ACCOUNT_ID, CUSTOM_VIEW_ID, GRANT_VIEW_ID) to bypass middleware validateAccount (404) and validateView (403); handlers do inline validation returning 400 - createViewForBankAccount and updateViewForBankAccount capture bankId/accountId from route and call checkBankAccountExists inline (400 on missing account) - getTransactionsForBankAccount and getTransactionByIdForBankAccount rewritten with executeAndRespond + inline account/view validation; transaction query params extracted from request headers (test framework sends them as headers, not query string) via req.headers.headers → HTTPParam list - createQueriesByHttpParamsFuture used for obp_* param validation (400 on invalid)
3 own endpoints (root, getCards, getCardsForBank) serve natively; all inherited v1.2.1 endpoints are covered via a path-rewriting bridge that rewrites /obp/v1.3.0/… → /obp/v1.2.1/… and delegates to Http4s121.wrappedRoutesV121Services. PhysicalCardsTest: 2/2 pass.
11 own endpoints + path-rewriting bridge to Http4s130; fix addCustomer entitlement check to return 403 (not 400) when user has only one of the two required roles (canCreateCustomer + canCreateUserCustomerLink).
37 own endpoints + path-rewriting bridge to Http4s140 so all inherited v1.4.0/v1.3.0/v1.2.1 paths are served under the v2.0.0 prefix. Two ResourceDoc fixes discovered while running tests: - createUser: removed AuthenticatedUserIsRequired from error list — the endpoint is public; having it caused needsAuthentication=true and the middleware returned 401 for unauthenticated POST /users. - addEntitlement: removed roles from ResourceDoc — bank_id comes from the request body, not the URL, so the middleware's BANK_ID-based role check always used bank="" and rejected canCreateEntitlementAtOneBank holders. Handler already does the correct inline check; switched its booleanToFuture to failCode=403 to match the expected response code. All passing: AccountTest (4), CreateUserTest (3), CustomerTest (1), EntitlementTests (5), API1_2_1Test (323), Http4s700RoutesTest (110).
1. corePrivateAccountsAllBanks and corePrivateAccountsAtOneBank returned
CoreAccountsJSON(list) = {"accounts":[]} but the Lift path returned a
plain JSON array []. DirectLoginTest expects the plain array. Fix:
return List[CoreAccountJSON] directly.
2. authenticatedAccess returns Failure(UsernameHasBeenLocked) as a Box,
not as an exception. The old authenticate step passed the Failure user
through to the handler which returned 401. Old Lift code returned 400
for all unadorned Failure boxes. Fix: detect Failure in authenticate
when needsAuth=true and return 400; Empty box keeps 401.
… status codes authenticatedAccess wraps the returned box with fullBoxOrException, which converts any non-Full result (including Failure(UsernameHasBeenLocked)) into a thrown plain Exception(json) carrying failCode=401. The authenticate handler's `case Left(e: APIFailureNewStyle)` never matched; the catch-all returned 401 for all auth failures including locked users. anonymousAccess already runs all post-auth checks (locked/deleted user, consumer-disabled, rate-limiting) and returns the Failure box as a successful Future result. Switching to anonymousAccess lets the pattern match in authenticate see the raw Failure(UsernameHasBeenLocked) box and return 400, matching the old Lift behaviour for DirectLoginV600Test and DirectLoginTest.
… auth failures anonymousAccess converts all Failure boxes to a thrown plain Exception(json) via fullBoxOrException, hardcoding failCode=401. The previous case Left(_) catch-all was returning 401 for everything, including UsernameHasBeenLocked and DAuthJwtTokenIsNotValid which Lift Old Style returned 400 for (via errorJsonResponse default). Parse the thrown exception's JSON payload to recover the original failMsg, then return 400 — matching the Lift Old Style implicit behavior. Also remove the dead-code case Right((Failure,...)) branch: anonymousAccess never returns a Failure box since it always converts them to thrown exceptions first.
25 own endpoints ported from APIMethods210 (root, sandboxDataImport, getTransactionRequestTypesSupportedByBank, createTransactionRequest ×4, answerTransactionRequestChallenge, getTransactionRequests, getRoles, getEntitlementsByBankAndUser, getConsumer, getConsumers, enableDisableConsumers, addCardForBank, getUsers, createTransactionType, getAtm, getBranch, getProduct, getProducts, createCustomer, getCustomersForUser, getCustomersForCurrentUserAtBank, updateBranch, createBranch, updateConsumerRedirectUrl, getMetrics) plus a path-rewriting bridge to Http4s200 for inherited v2.0.0/v1.4.0/v1.3.0/ v1.2.1 paths. Also fixes body-parse error responses in HttpsSupport helpers: replace plain BadRequest(msg) with ErrorResponseConverter.createErrorResponse so invalid-JSON responses are properly formatted JSON objects instead of raw strings. All 79 v2.1.0 tests pass.
18 own endpoints (root, getViewsForBankAccount, createViewForBankAccount,
updateViewForBankAccount, getCurrentFxRate, getExplicitCounterpartiesForAccount,
getExplicitCounterpartyById, getMessageDocs, createBank, createBranch, createAtm,
createProduct, createFx, createAccount, config, getConnectorMetrics, createConsumer,
createCounterparty) + path-rewriting bridge to Http4s210.
All 27 v2.2.0 tests pass (AccountTest, API2_2_0Test, ExchangeRateTest,
CreateCounterpartyTest).
Notable patterns used:
- createAccount: ResourceDoc uses NEW_ACCOUNT_ID (non-standard) to bypass
middleware account-existence check; inline booleanToFuture(failCode=403)
for cross-user role enforcement
- createViewForBankAccount: ResourceDoc uses VIEW_ACCOUNT_ID to bypass
middleware 404; inline Connector.checkBankAccountExists → unboxFullOrFail
returns 400 for missing account (matching Lift behaviour)
- updateViewForBankAccount: ResourceDoc uses UPD_VIEW_ID to bypass middleware
VIEW_ID validation; handler's own startsWith("_") check returns 400 first
- ResourceDocMiddleware: fix case-None branch to attach basic CC to request so
endpoints at bare version paths (e.g. GET /obp/v2.2.0) can call req.callContext
…re bypass variants
47 own endpoints + path-rewriting bridge to Http4s220. All 86 v3.0.0 tests pass. Firehose endpoints use non-standard ResourceDoc URL template vars (FIREHOSE_BANK_ID / FIREHOSE_VIEW_ID) to bypass middleware bank/view resolution, ensuring prop check returns 400 and role check returns 403 in the correct order regardless of bank ID validity.
…ocMiddleware Old Style endpoints (v1.x, v2.0.0) keep 400 for auth failures (locked user, invalid DAuth JWT) matching Lift's errorJsonResponse default. New Style endpoints (v2.1.0+) now correctly return the parsed failCode from the exception (401), matching the Lift New Style contract. Fixes: DirectLoginTest locked-user scenario returning 400 at v3.0.0/users/current Fixes: dauthTest invalid DAuth JWT returning 400 at v3.0.0/users/current
|
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.