From 37979d019f1cf6f9c2a212dc2129b2d6b28f3741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Mili=C4=87?= Date: Wed, 20 May 2026 16:05:59 +0200 Subject: [PATCH 1/2] chore: retire legacy ImporterAPI bulk-transaction endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `POST /obp_transactions_saver/api/transactions` shared-secret endpoint was a pre-OBP-API mechanism for an external "transactions saver" cron job to bulk-insert bank transactions. It bypassed transaction-request validation entirely, used a LiftActor for sequencing, and predates the modern connector-driven and `/obp/vX.X.X/transaction-requests/...` flows that subsume it. Removed: - code.management.ImporterAPI — Lift dispatch handler + JSON case classes (ImporterTransaction, TransactionsToInsert, etc.) - code.management.ImporterAPIRoutes — its http4s replacement - code.management.TransactionInserter — LiftActor that did the duplicate-detect + insert work - code.management.ImporterTest — 8-scenario test suite - LocalMappedConnectorInternal: getMatchingTransactionCount, createImportedTransaction, updateAccountBalance, setBankAccountLastUpdated, and the getBankByNationalIdentifier / getAccountByNumber / bigDecimalFailureHandler private helpers that only existed to serve those methods (all explicitly commented as "used by transaction import api") - importer_secret prop from sample.props.template + test.default.props.template - importer_secret=change_me echo from both GitHub Actions workflows - ImporterAPIRoutes from Http4sApp.baseServices chain - stale ImporterAPI references in Boot.scala comments and LIFT_HTTP4S_MIGRATION.md If you have an external script POSTing to /obp_transactions_saver/api/transactions, migrate it to a connector (Kafka / rest_v*) or the /obp/vX.X.X/transaction-requests/... endpoints before deploying this commit. Net -959 lines. --- .github/workflows/build_container.yml | 1 - .github/workflows/build_pull_request.yml | 1 - LIFT_HTTP4S_MIGRATION.md | 6 +- .../resources/props/sample.props.template | 3 - .../props/test.default.props.template | 3 - .../main/scala/bootstrap/liftweb/Boot.scala | 12 +- .../code/api/util/http4s/Http4sApp.scala | 1 - .../LocalMappedConnectorInternal.scala | 115 ------ .../scala/code/management/ImporterAPI.scala | 201 ---------- .../code/management/ImporterAPIRoutes.scala | 129 ------ .../code/management/TransactionInserter.scala | 120 ------ .../scala/code/management/ImporterTest.scala | 375 ------------------ 12 files changed, 8 insertions(+), 959 deletions(-) delete mode 100644 obp-api/src/main/scala/code/management/ImporterAPI.scala delete mode 100644 obp-api/src/main/scala/code/management/ImporterAPIRoutes.scala delete mode 100644 obp-api/src/main/scala/code/management/TransactionInserter.scala delete mode 100644 obp-api/src/test/scala/code/management/ImporterTest.scala diff --git a/.github/workflows/build_container.yml b/.github/workflows/build_container.yml index 47e5166cae..91a43e2181 100644 --- a/.github/workflows/build_container.yml +++ b/.github/workflows/build_container.yml @@ -209,7 +209,6 @@ jobs: echo tests.port=8016 >> obp-api/src/main/resources/props/test.default.props echo End of minimum settings >> obp-api/src/main/resources/props/test.default.props echo payments_enabled=false >> obp-api/src/main/resources/props/test.default.props - echo importer_secret=change_me >> obp-api/src/main/resources/props/test.default.props echo messageQueue.updateBankAccountsTransaction=false >> obp-api/src/main/resources/props/test.default.props echo messageQueue.createBankAccounts=false >> obp-api/src/main/resources/props/test.default.props echo allow_sandbox_account_creation=true >> obp-api/src/main/resources/props/test.default.props diff --git a/.github/workflows/build_pull_request.yml b/.github/workflows/build_pull_request.yml index 198b0755f9..411bf39c40 100644 --- a/.github/workflows/build_pull_request.yml +++ b/.github/workflows/build_pull_request.yml @@ -212,7 +212,6 @@ jobs: echo tests.port=8016 >> obp-api/src/main/resources/props/test.default.props echo End of minimum settings >> obp-api/src/main/resources/props/test.default.props echo payments_enabled=false >> obp-api/src/main/resources/props/test.default.props - echo importer_secret=change_me >> obp-api/src/main/resources/props/test.default.props echo messageQueue.updateBankAccountsTransaction=false >> obp-api/src/main/resources/props/test.default.props echo messageQueue.createBankAccounts=false >> obp-api/src/main/resources/props/test.default.props echo allow_sandbox_account_creation=true >> obp-api/src/main/resources/props/test.default.props diff --git a/LIFT_HTTP4S_MIGRATION.md b/LIFT_HTTP4S_MIGRATION.md index 76b46f9343..644c10d614 100644 --- a/LIFT_HTTP4S_MIGRATION.md +++ b/LIFT_HTTP4S_MIGRATION.md @@ -225,7 +225,7 @@ Already partly described in the next major section, but counted here for complet | Endpoint | File | Notes | |---|---|---| | `aliveCheck` | `code/api/aliveCheck.scala` → `code/api/AliveCheckRoutes.scala` | **Done.** Native http4s route serves `GET /alive`; `LiftRules.statelessDispatch.append(aliveCheck)` removed from `Boot.scala`. | -| `ImporterAPI` | `code/management/ImporterAPI.scala` → `code/management/ImporterAPIRoutes.scala` | **Done.** Native http4s route serves `POST /obp_transactions_saver/api/transactions`; secret read from URL query, body parsed from `req.bodyText`, `TransactionInserter` LiftActor still invoked synchronously (wrapped in `IO.blocking`). `ImporterTest` (8 scenarios) green. | +| `ImporterAPI` | (deleted) | **Retired.** The legacy `POST /obp_transactions_saver/api/transactions` shared-secret bulk-insert endpoint, its `TransactionInserter` LiftActor, and the connector helpers it relied on (`createImportedTransaction`, `getMatchingTransactionCount`, `updateAccountBalance`, `setBankAccountLastUpdated`) have been removed entirely. Modern callers use connector-driven flows or the `/obp/vX.X.X/transaction-requests/...` endpoints. | | `OpenIdConnect` | (auth-stack table above) | OIDC callback, registered separately from OAuth2. | ### Open-banking standards (large, deferred indefinitely) @@ -254,7 +254,7 @@ Three forks for how this workstream resolves: Currently runs on startup and goes away once the Lift bridge is removable: -1. `LiftRules.statelessDispatch.append(...)` registrations: `DirectLogin`, `ImporterAPI`, `ResourceDocs140`–`ResourceDocs600`, `aliveCheck`. +1. `LiftRules.statelessDispatch.append(...)` registrations: `DirectLogin`, `ResourceDocs140`–`ResourceDocs600`, `aliveCheck`. 2. `LiftRules.dispatch.append(OpenIdConnect)`. 3. `LiftRules.addToPackages("code")` — Lift package scanner. 4. `LiftRules.exceptionHandler.prepend { ... }` — global exception handler. @@ -297,7 +297,7 @@ A second decision is *not* required for bridge removal, but is required for the 1. ~~**v4.0.0 bulk port**~~ — done (258/258, 100%). 2. ~~**DirectLogin**~~ — done. `code.api.DirectLoginRoutes` serves the bare `/my/logins/direct`; per-version paths served by their own `Http4sXxx`. `LiftRules.statelessDispatch.append(DirectLogin)` retired. -3. ~~**`aliveCheck`, `ImporterAPI`**~~ — done. `code.api.AliveCheckRoutes` serves `GET /alive`; `code.management.ImporterAPIRoutes` serves `POST /obp_transactions_saver/api/transactions`. Both Lift dispatches retired. +3. ~~**`aliveCheck`**~~ — done. `code.api.AliveCheckRoutes` serves `GET /alive`; Lift dispatch retired. **`ImporterAPI`** — retired entirely (no http4s replacement); the legacy shared-secret bulk-transaction-importer endpoint has been removed along with `TransactionInserter` and the connector helpers it relied on. 4. ~~**`Http4sResourceDocs` centralised service**~~ — done. `code.api.util.http4s.Http4sResourceDocs` serves `/obp/*/resource-docs/{API_VERSION}/{obp,swagger,openapi,openapi.yaml}`, `/obp/*/banks/{BANK_ID}/resource-docs/{API_VERSION}/obp`, and `/obp/*/message-docs/{CONNECTOR}/swagger2.0`. 10 `LiftRules.statelessDispatch.append(ResourceDocs140..600)` retired + `openapi.yaml` raw `serve { ... }` block removed. ResourceDocsTest (63) + SwaggerDocsTest (10) green. 5. **Auth stack: OAuth2 / GatewayLogin / DAuth** — done. All three turned out to be library-only token validators (no `serve` blocks, no `LiftRules` registration). Vestigial `extends RestHelper` mixins removed. 6. **OpenIdConnect** — the only remaining auth-stack work. Blocked on a portal-session decision (its success path calls `AuthUser.logUserIn` / `S.redirectTo`, which mutate Lift `SessionVar`s — see auth-stack table). OAuth 1.0a was removed entirely in commit `51820c75e`; no migration needed. diff --git a/obp-api/src/main/resources/props/sample.props.template b/obp-api/src/main/resources/props/sample.props.template index f4ccee3a3f..25a7b10509 100644 --- a/obp-api/src/main/resources/props/sample.props.template +++ b/obp-api/src/main/resources/props/sample.props.template @@ -463,9 +463,6 @@ connection.host=localhost connection.user=theusername connection.password=thepassword -## Secret key that allows access to the "add transactions" api. You should change this to your own secret key -importer_secret=change_me - ## Set this to true if you want to have the api send a message to the hbci project to refresh transactions for an account messageQueue.updateBankAccountsTransaction=false diff --git a/obp-api/src/main/resources/props/test.default.props.template b/obp-api/src/main/resources/props/test.default.props.template index 483bdfd656..78cf242755 100644 --- a/obp-api/src/main/resources/props/test.default.props.template +++ b/obp-api/src/main/resources/props/test.default.props.template @@ -96,9 +96,6 @@ payments_enabled=false #connection.user=user #connection.password=pw -#secret key that allows access to the "add transactions" api. You should change this to your own secret key -importer_secret=change_me - #set this to true if you want to have the api to send a message to the hbci project to refresh transactions for an account messageQueue.updateBankAccountsTransaction=false diff --git a/obp-api/src/main/scala/bootstrap/liftweb/Boot.scala b/obp-api/src/main/scala/bootstrap/liftweb/Boot.scala index bf2f7f50f3..398014cfd0 100644 --- a/obp-api/src/main/scala/bootstrap/liftweb/Boot.scala +++ b/obp-api/src/main/scala/bootstrap/liftweb/Boot.scala @@ -498,13 +498,11 @@ class Boot extends MdcLoggable { LiftRules.dispatch.append(OpenIdConnect) } } - // DirectLogin (POST /my/logins/direct), ImporterAPI (POST - // /obp_transactions_saver/api/transactions), and aliveCheck (GET /alive) - // are now served by their native http4s counterparts wired into - // Http4sApp.baseServices (DirectLoginRoutes / ImporterAPIRoutes / - // AliveCheckRoutes). The Lift dispatches were retired in the http4s - // migration; any prop gates (e.g. `allow_direct_login`) live with those - // routes. + // DirectLogin (POST /my/logins/direct) and aliveCheck (GET /alive) are now + // served by their native http4s counterparts wired into + // Http4sApp.baseServices (DirectLoginRoutes / AliveCheckRoutes). The Lift + // dispatches were retired in the http4s migration; any prop gates + // (e.g. `allow_direct_login`) live with those routes. diff --git a/obp-api/src/main/scala/code/api/util/http4s/Http4sApp.scala b/obp-api/src/main/scala/code/api/util/http4s/Http4sApp.scala index 83c0060a39..24bddefe92 100644 --- a/obp-api/src/main/scala/code/api/util/http4s/Http4sApp.scala +++ b/obp-api/src/main/scala/code/api/util/http4s/Http4sApp.scala @@ -123,7 +123,6 @@ object Http4sApp { .orElse(v121Routes.run(req)) .orElse(code.api.DirectLoginRoutes.routes.run(req)) .orElse(code.api.AliveCheckRoutes.routes.run(req)) - .orElse(code.management.ImporterAPIRoutes.routes.run(req)) .orElse(Http4sLiftWebBridge.routes.run(req)) } } diff --git a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala index 5313b89263..a77b2e4e31 100644 --- a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala +++ b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala @@ -19,7 +19,6 @@ import code.bankconnectors.ethereum.DecodeRawTx import code.branches.MappedBranch import code.fx.fx import code.fx.fx.TTL -import code.management.ImporterAPI.ImporterTransaction import code.model.dataAccess.{BankAccountRouting, MappedBank, MappedBankAccount} import code.model.toBankAccountExtended import code.transaction.MappedTransaction @@ -322,120 +321,6 @@ object LocalMappedConnectorInternal extends MdcLoggable { } - //transaction import api uses bank national identifiers to uniquely indentify banks, - //which is unfortunate as theoretically the national identifier is unique to a bank within - //one country - private def getBankByNationalIdentifier(nationalIdentifier: String): Box[Bank] = { - MappedBank.find(By(MappedBank.national_identifier, nationalIdentifier)) - } - - private def getAccountByNumber(bankId: BankId, number: String): Box[BankAccount] = { - MappedBankAccount.find( - By(MappedBankAccount.bank, bankId.value), - By(MappedBankAccount.accountNumber, number)) - } - - private val bigDecimalFailureHandler: PartialFunction[Throwable, Unit] = { - case ex: NumberFormatException => { - logger.warn(s"could not convert amount to a BigDecimal: $ex") - } - } - - //used by transaction import api call to check for duplicates - def getMatchingTransactionCount(bankNationalIdentifier: String, accountNumber: String, amount: String, completed: Date, otherAccountHolder: String): Box[Int] = { - //we need to convert from the legacy bankNationalIdentifier to BankId, and from the legacy accountNumber to AccountId - val count = for { - bankId <- getBankByNationalIdentifier(bankNationalIdentifier).map(_.bankId) - account <- getAccountByNumber(bankId, accountNumber) - amountAsBigDecimal <- tryo(bigDecimalFailureHandler)(BigDecimal(amount)) - } yield { - - val amountInSmallestCurrencyUnits = - Helper.convertToSmallestCurrencyUnits(amountAsBigDecimal, account.currency) - - MappedTransaction.count( - By(MappedTransaction.bank, bankId.value), - By(MappedTransaction.account, account.accountId.value), - By(MappedTransaction.amount, amountInSmallestCurrencyUnits), - By(MappedTransaction.tFinishDate, completed), - By(MappedTransaction.counterpartyAccountHolder, otherAccountHolder)) - } - - //icky - Full(count.map(_.toInt) getOrElse 0) - } - - - def createImportedTransaction(transaction: ImporterTransaction): Box[Transaction] = { - //we need to convert from the legacy bankNationalIdentifier to BankId, and from the legacy accountNumber to AccountId - val obpTransaction = transaction.obp_transaction - val thisAccount = obpTransaction.this_account - val nationalIdentifier = thisAccount.bank.national_identifier - val accountNumber = thisAccount.number - for { - bank <- getBankByNationalIdentifier(transaction.obp_transaction.this_account.bank.national_identifier) ?~! - s"No bank found with national identifier $nationalIdentifier" - bankId = bank.bankId - account <- getAccountByNumber(bankId, accountNumber) - details = obpTransaction.details - amountAsBigDecimal <- tryo(bigDecimalFailureHandler)(BigDecimal(details.value.amount)) - newBalanceAsBigDecimal <- tryo(bigDecimalFailureHandler)(BigDecimal(details.new_balance.amount)) - amountInSmallestCurrencyUnits = Helper.convertToSmallestCurrencyUnits(amountAsBigDecimal, account.currency) - newBalanceInSmallestCurrencyUnits = Helper.convertToSmallestCurrencyUnits(newBalanceAsBigDecimal, account.currency) - otherAccount = obpTransaction.other_account - mappedTransaction = MappedTransaction.create - .bank(bankId.value) - .account(account.accountId.value) - .transactionType(details.kind) - .amount(amountInSmallestCurrencyUnits) - .newAccountBalance(newBalanceInSmallestCurrencyUnits) - .currency(account.currency) - .tStartDate(details.posted.`$dt`) - .tFinishDate(details.completed.`$dt`) - .description(details.label) - .counterpartyAccountNumber(otherAccount.number) - .counterpartyAccountHolder(otherAccount.holder) - .counterpartyAccountKind(otherAccount.kind) - .counterpartyNationalId(otherAccount.bank.national_identifier) - .counterpartyBankName(otherAccount.bank.name) - .counterpartyIban(otherAccount.bank.IBAN) - .saveMe() - transaction <- mappedTransaction.toTransaction(account) - } yield transaction - } - - //used by the transaction import api - def updateAccountBalance(bankId: BankId, accountId: AccountId, newBalance: BigDecimal): Box[Boolean] = { - //this will be Full(true) if everything went well - val result = for { - (bank, _) <- Connector.connector.vend.getBankLegacy(bankId, None) - account <- Connector.connector.vend.getBankAccountLegacy(bankId, accountId, None).map(_._1).map(_.asInstanceOf[MappedBankAccount]) - } yield { - account.accountBalance(Helper.convertToSmallestCurrencyUnits(newBalance, account.currency)).save - setBankAccountLastUpdated(bank.nationalIdentifier, account.number, now).openOrThrowException(attemptedToOpenAnEmptyBox) - } - - Full(result.getOrElse(false)) - } - - def setBankAccountLastUpdated(bankNationalIdentifier: String, accountNumber: String, updateDate: Date): Box[Boolean] = { - val result = for { - bankId <- getBankByNationalIdentifier(bankNationalIdentifier).map(_.bankId) - account <- getAccountByNumber(bankId, accountNumber) - } yield { - val acc = MappedBankAccount.find( - By(MappedBankAccount.bank, bankId.value), - By(MappedBankAccount.theAccountId, account.accountId.value) - ) - acc match { - case Full(a) => a.accountLastUpdate(updateDate).save - case _ => logger.warn("can't set bank account.lastUpdated because the account was not found"); false - } - } - Full(result.getOrElse(false)) - } - - //creates a bank account for an existing bank, with the appropriate values set. Can fail if the bank doesn't exist def createSandboxBankAccount( bankId: BankId, diff --git a/obp-api/src/main/scala/code/management/ImporterAPI.scala b/obp-api/src/main/scala/code/management/ImporterAPI.scala deleted file mode 100644 index 0a65a03e01..0000000000 --- a/obp-api/src/main/scala/code/management/ImporterAPI.scala +++ /dev/null @@ -1,201 +0,0 @@ -package code.management - - -import java.util.Date -import code.api.util.ErrorMessages._ -import code.api.util.{APIUtil, CustomJsonFormats} -import code.api.util.APIUtil.DateWithMsExampleObject -import code.bankconnectors.{Connector, LocalMappedConnectorInternal} -import code.tesobe.ErrorMessage -import code.util.Helper.{MdcLoggable, ObpS} -import com.openbankproject.commons.model.Transaction -import com.openbankproject.commons.model.enums.AccountRoutingScheme -import net.liftweb.common.Full -import net.liftweb.http._ -import net.liftweb.http.js.JsExp -import net.liftweb.http.rest.RestHelper -import net.liftweb.json.Extraction -import net.liftweb.json.JsonAST.{JArray, JField, JObject, JString} -import net.liftweb.util.Helpers._ - -/** - * This is legacy code and does not handle edge cases very well and assumes certain things, e.g. - * that bank national identifier is unique (when in reality it should only be unique for a given - * country). So if it looks like it's doing things in a very weird way, that's because it is. - */ -object ImporterAPI extends RestHelper with MdcLoggable { - - case class TransactionsToInsert(l : List[ImporterTransaction]) - case class InsertedTransactions(l : List[Transaction]) - - //models the json format -> This model cannot change or it will break the api call! - //if you want to update/modernize the json format, you will need to create a new api call - //and leave these models untouched until the old api call is no longer used by any clients - case class ImporterTransaction(obp_transaction : ImporterOBPTransaction) - case class ImporterOBPTransaction(this_account : ImporterAccount, other_account : ImporterAccount, details : ImporterDetails) - case class ImporterAccount(holder : String, number : String, kind : String, bank : ImporterBank) - - /** - * it doesn't make sense that the IBAN attribute is on the Bank (as IBAN is in reality a property of an account), - * but this was how it was originally written and is also the - */ - case class ImporterBank(IBAN : String, national_identifier : String, name : String) - - case class ImporterDetails(kind : String, posted : ImporterDate, - completed : ImporterDate, new_balance : ImporterAmount, value : ImporterAmount, - label : String) - case class ImporterDate(`$dt` : Date) //format : "2012-01-04T18:06:22.000Z" (see tests) - case class ImporterAmount(currency : String, amount : String) - - implicit object ImporterDateOrdering extends Ordering[ImporterDate]{ - def compare(x: ImporterDate, y: ImporterDate): Int ={ - x.`$dt`.compareTo(y.`$dt`) - } - } - - def errorJsonResponse(message : String = "error", httpCode : Int = 400) : JsonResponse = - JsonResponse(Extraction.decompose(ErrorMessage(message)), APIUtil.getHeaders(), Nil, httpCode) - def successJsonResponse(json: JsExp, httpCode : Int = 200) : JsonResponse = - JsonResponse(json, APIUtil.getHeaders(), Nil, httpCode) - - /** - * Legacy format - * - * TODO: can we just get rid of this? is anything using it? - */ - def whenAddedJson(t : Transaction) : JObject = { - - def formatDate(date : Date) : String = { - CustomJsonFormats.losslessFormats.dateFormat.format(date) - } - - val thisBank = Connector.connector.vend.getBankLegacy(t.bankId, None).map(_._1) - val thisAcc = Connector.connector.vend.getBankAccountLegacy(t.bankId, t.accountId, None).map(_._1) - val thisAccJson = JObject(List(JField("holder", - JObject(List( - JField("holder", JString(thisAcc.map(_.accountHolder).getOrElse(""))), - JField("alias", JString("no"))))), - JField("number", JString(thisAcc.map(_.number).getOrElse(""))), - JField("kind", JString(thisAcc.map(_.accountType).getOrElse(""))), - JField("bank", JObject(List( JField("IBAN", JString(thisAcc.flatMap(_.accountRoutings.find(_.scheme == AccountRoutingScheme.IBAN.toString).map(_.address)).getOrElse(""))), - JField("national_identifier", JString(thisBank.map(_.nationalIdentifier).getOrElse(""))), - JField("name", JString(thisBank.map(_.fullName).getOrElse("")))))))) - - val otherAcc = t.otherAccount - val otherAccJson = JObject(List(JField("holder", - JObject(List( - JField("holder", JString(otherAcc.counterpartyName)), - JField("alias", JString("no"))))), - JField("number", JString(otherAcc.thisAccountId.value)), - JField("kind", JString(otherAcc.kind)), - JField("bank", JObject(List( JField("IBAN", JString(otherAcc.otherAccountRoutingAddress.getOrElse(""))), - JField("national_identifier", JString(otherAcc.nationalIdentifier)), - JField("name", JString(otherAcc.thisBankId.value))))))) - - val detailsJson = JObject(List( JField("type_en", JString(t.transactionType)), - JField("type", JString(t.transactionType)), - JField("posted", JString(formatDate(t.startDate))), - JField("completed", JString(formatDate(t.finishDate.getOrElse(DateWithMsExampleObject)))), - JField("other_data", JString("")), - JField("new_balance", JObject(List( JField("currency", JString(t.currency)), - JField("amount", JString(t.balance.toString))))), - JField("value", JObject(List( JField("currency", JString(t.currency)), - JField("amount", JString(t.amount.toString))))))) - - val transactionJson = { - JObject(List(JField("obp_transaction_uuid", JString(t.uuid)), - JField("this_account", thisAccJson), - JField("other_account", otherAccJson), - JField("details", detailsJson))) - } - - JObject( - List( - JField("obp_transaction", transactionJson) - ) - ) - } - - serve { - - case "obp_transactions_saver" :: "api" :: "transactions" :: Nil JsonPost req => { - - def savetransactions = { - def updateBankAccountBalance(insertedTransactions : List[Transaction]) = { - if(insertedTransactions.nonEmpty) { - //we assume here that all the Envelopes concern only one account - val mostRecentTransaction = insertedTransactions.maxBy(t => t.finishDate) - - LocalMappedConnectorInternal.updateAccountBalance( - mostRecentTransaction.bankId, - mostRecentTransaction.accountId, - mostRecentTransaction.balance).openOrThrowException(attemptedToOpenAnEmptyBox) - } - } - - val ipAddress = req._2.remoteAddr - - val rawTransactions = req._1.children - - logger.info("Received " + rawTransactions.size + - " json transactions to insert from ip address " + ipAddress) - - //importer api expects dates that also include milliseconds (lossless) - val losslessFormats = CustomJsonFormats.losslessFormats - val mf = implicitly[Manifest[ImporterTransaction]] - val importerTransactions = rawTransactions.flatMap(j => j.extractOpt[ImporterTransaction](losslessFormats, mf)) - - logger.info("Received " + importerTransactions.size + - " valid json transactions to insert from ip address " + ipAddress) - - if(importerTransactions.isEmpty) logger.warn("no transactions found to insert") - - val toInsert = TransactionsToInsert(importerTransactions) - - /** - * Using an actor to do insertions avoids concurrency issues with - * duplicate transactions by processing transaction batches one - * at a time. We'll have to monitor this to see if non-concurrent I/O - * is too inefficient. If it is, we could break it up into one actor - * per "Account". - */ - // TODO: this duration limit should be fixed - val createdEnvelopes = TransactionInserter !? (3.minutes, toInsert) - - createdEnvelopes match { - case Full(inserted : InsertedTransactions) => - val insertedTs = inserted.l - logger.info("inserted " + insertedTs.size + " transactions") - updateBankAccountBalance(insertedTs) - if (insertedTs.isEmpty && importerTransactions.nonEmpty) { - //refresh account lastUpdate in case transactions were posted but they were all duplicates (account was still "refreshed") - val mostRecentTransaction = importerTransactions.maxBy(t => t.obp_transaction.details.completed) - val account = mostRecentTransaction.obp_transaction.this_account - LocalMappedConnectorInternal.setBankAccountLastUpdated(account.bank.national_identifier, account.number, now).openOrThrowException(attemptedToOpenAnEmptyBox) - } - val jsonList = insertedTs.map(whenAddedJson) - successJsonResponse(JArray(jsonList)) - case _ => { - logger.warn("no envelopes inserted") - InternalServerErrorResponse() - } - } - } - - ObpS.param("secret") match { - case Full(s) => { - APIUtil.getPropsValue("importer_secret") match { - case Full(localS) => - if(localS == s) - savetransactions - else - errorJsonResponse("wrong secret", 401) - case _ => errorJsonResponse("importer_secret not set on the server.") - } - } - case _ => errorJsonResponse("secret missing") - } - - } - } -} diff --git a/obp-api/src/main/scala/code/management/ImporterAPIRoutes.scala b/obp-api/src/main/scala/code/management/ImporterAPIRoutes.scala deleted file mode 100644 index 63dadf8af4..0000000000 --- a/obp-api/src/main/scala/code/management/ImporterAPIRoutes.scala +++ /dev/null @@ -1,129 +0,0 @@ -package code.management - -import cats.effect.IO -import code.api.util.{APIUtil, CustomJsonFormats} -import code.api.util.ErrorMessages._ -import code.bankconnectors.LocalMappedConnectorInternal -import code.management.ImporterAPI._ -import code.tesobe.ErrorMessage -import code.util.Helper.MdcLoggable -import com.openbankproject.commons.model.Transaction -import net.liftweb.common.Full -import net.liftweb.json.Extraction -import net.liftweb.json.JsonAST.{JArray, JValue} -import net.liftweb.util.Helpers._ -import org.http4s.{Charset, HttpRoutes, MediaType, Request, Response, Status} -import org.http4s.dsl.io._ -import org.http4s.headers.`Content-Type` - -/** - * Native http4s route for the legacy `POST /obp_transactions_saver/api/transactions` endpoint. - * - * Mirrors `code.management.ImporterAPI`'s Lift `serve` block. Replaces - * `LiftRules.statelessDispatch.append(ImporterAPI)` in `Boot.scala`. - * - * Status-code parity (preserved verbatim from the Lift handler): - * - secret query param missing → 400 "secret missing" - * - secret wrong → 401 "wrong secret" - * - `importer_secret` prop not set on server → 400 "importer_secret not set on the server." - * - secret correct → 200 with JArray of inserted transactions - * - actor returns no envelopes → 500 - */ -object ImporterAPIRoutes extends MdcLoggable { - - private val jsonContentType: `Content-Type` = - `Content-Type`(MediaType.application.json, Charset.`UTF-8`) - - private def errorResponse(message: String, httpCode: Int): IO[Response[IO]] = { - val body = net.liftweb.json.compactRender( - Extraction.decompose(ErrorMessage(message))(CustomJsonFormats.formats)) - IO.pure( - Response[IO](Status.fromInt(httpCode).getOrElse(Status.BadRequest)) - .withEntity(body) - .withContentType(jsonContentType) - ) - } - - val routes: HttpRoutes[IO] = HttpRoutes.of[IO] { - case req @ POST -> Root / "obp_transactions_saver" / "api" / "transactions" => - req.uri.query.params.get("secret") match { - case None => errorResponse("secret missing", 400) - case Some(provided) => - APIUtil.getPropsValue("importer_secret") match { - case Full(expected) if expected == provided => saveTransactions(req) - case Full(_) => errorResponse("wrong secret", 401) - case _ => errorResponse("importer_secret not set on the server.", 400) - } - } - } - - private def saveTransactions(req: Request[IO]): IO[Response[IO]] = { - val ipAddress = req.remoteAddr.map(_.toUriString).getOrElse("") - req.bodyText.compile.string.flatMap { bodyText => - IO.blocking(processBody(bodyText, ipAddress)) - } - } - - // Synchronous: parses JSON, sends to TransactionInserter LiftActor via `!?` - // (blocking ask), then updates account balance / last-updated timestamps. - // Mirrors the Lift handler body in ImporterAPI.scala so behaviour - // (status codes, response shape, side-effects) is preserved verbatim. - private def processBody(bodyText: String, ipAddress: String): Response[IO] = { - val parsedJson: JValue = - scala.util.Try(net.liftweb.json.parse(bodyText)).getOrElse(JArray(Nil)) - val rawTransactions = parsedJson.children - - logger.info( - "Received " + rawTransactions.size + - " json transactions to insert from ip address " + ipAddress) - - val losslessFormats = CustomJsonFormats.losslessFormats - val mf = implicitly[Manifest[ImporterTransaction]] - val importerTransactions = - rawTransactions.flatMap(j => j.extractOpt[ImporterTransaction](losslessFormats, mf)) - - logger.info( - "Received " + importerTransactions.size + - " valid json transactions to insert from ip address " + ipAddress) - - if (importerTransactions.isEmpty) logger.warn("no transactions found to insert") - - val toInsert = TransactionsToInsert(importerTransactions) - val createdEnvelopes = TransactionInserter !? (3.minutes, toInsert) - - createdEnvelopes match { - case Full(inserted: InsertedTransactions) => - val insertedTs = inserted.l - logger.info("inserted " + insertedTs.size + " transactions") - updateBankAccountBalance(insertedTs) - if (insertedTs.isEmpty && importerTransactions.nonEmpty) { - // refresh account lastUpdate in case transactions were duplicates - val mostRecentTransaction = - importerTransactions.maxBy(t => t.obp_transaction.details.completed) - val account = mostRecentTransaction.obp_transaction.this_account - LocalMappedConnectorInternal - .setBankAccountLastUpdated(account.bank.national_identifier, account.number, now) - .openOrThrowException(attemptedToOpenAnEmptyBox) - } - val jsonList = insertedTs.map(whenAddedJson) - Response[IO](Status.Ok) - .withEntity(net.liftweb.json.compactRender(JArray(jsonList))) - .withContentType(jsonContentType) - case _ => - logger.warn("no envelopes inserted") - Response[IO](Status.InternalServerError) - } - } - - private def updateBankAccountBalance(insertedTransactions: List[Transaction]): Unit = { - if (insertedTransactions.nonEmpty) { - val mostRecentTransaction = insertedTransactions.maxBy(t => t.finishDate) - LocalMappedConnectorInternal - .updateAccountBalance( - mostRecentTransaction.bankId, - mostRecentTransaction.accountId, - mostRecentTransaction.balance) - .openOrThrowException(attemptedToOpenAnEmptyBox) - } - } -} diff --git a/obp-api/src/main/scala/code/management/TransactionInserter.scala b/obp-api/src/main/scala/code/management/TransactionInserter.scala deleted file mode 100644 index 7cdb6d54e2..0000000000 --- a/obp-api/src/main/scala/code/management/TransactionInserter.scala +++ /dev/null @@ -1,120 +0,0 @@ -package code.management - -import code.api.util.ErrorMessages._ -import code.bankconnectors.{Connector, LocalMappedConnectorInternal} -import code.management.ImporterAPI._ -import code.util.Helper.MdcLoggable -import com.openbankproject.commons.model.Transaction -import net.liftweb.actor.LiftActor -import net.liftweb.common._ -import net.liftweb.util.Helpers - -object TransactionInserter extends LiftActor with MdcLoggable { - - /** - * Determines whether two transactions to be imported are considered "identical" - * - * Currently this is considered true if the date cleared, the transaction amount, - * and the name of the other party are the same. - */ - def isIdentical(t1: ImporterTransaction, t2: ImporterTransaction) : Boolean = { - - val t1Details = t1.obp_transaction.details - val t1Completed = t1Details.completed - val t1Amount = t1Details.value.amount - val t1OtherAccHolder = t1.obp_transaction.other_account.holder - - val t2Details = t2.obp_transaction.details - val t2Completed = t2Details.completed - val t2Amount = t2Details.value.amount - val t2OtherAccHolder = t2.obp_transaction.other_account.holder - - t1Completed == t2Completed && - t1Amount == t2Amount && - t1OtherAccHolder == t2OtherAccHolder - } - - /** - * Inserts a list of identical transactions, ensuring that no duplicates are made. - * - * This is done by querying for all existing copies of this transaction, - * and comparing the number of results to the size of the list of transactions to insert. - * - * E.g. If this method receives 3 identical transactions, and only 1 copy exists - * in the database, then 2 more should be added. - * - * If this method receives 3 identical transactions, and 3 copies exist in the - * database, then 0 more should be added. - */ - def insertIdenticalTransactions(identicalTransactions : List[ImporterTransaction]) : List[Transaction] = { - if(identicalTransactions.size == 0){ - Nil - }else{ - //we don't want to be putting people's transaction info in the logs, so we use an id - val insertID = Helpers.randomString(10) - //logger.info("Starting insert operation, id: " + insertID) - - val toMatch = identicalTransactions(0) - - val existingMatches = LocalMappedConnectorInternal.getMatchingTransactionCount( - toMatch.obp_transaction.this_account.bank.national_identifier, - toMatch.obp_transaction.this_account.number, - toMatch.obp_transaction.details.value.amount, - toMatch.obp_transaction.details.completed.`$dt`, - toMatch.obp_transaction.other_account.holder).openOrThrowException(attemptedToOpenAnEmptyBox) - - //logger.info("Insert operation id " + insertID + " # of existing matches: " + existingMatches) - val numberToInsert = identicalTransactions.size - existingMatches - //if(numberToInsert > 0) - //logger.info("Insert operation id " + insertID + " copies being inserted: " + numberToInsert) - - val results = (1 to numberToInsert).map(_ => LocalMappedConnectorInternal.createImportedTransaction(toMatch)).toList - - results.foreach{ - case Failure(msg, _, _) => logger.warn(s"create transaction failed: $msg") - case Empty => logger.warn("create transaction failed") - case _ => Unit //do nothing - } - - results.flatten - } - } - - def messageHandler = { - case TransactionsToInsert(ts : List[ImporterTransaction]) => { - - /** - * Example: - * input : List(A,B,C,C,D,E,E,E,F) - * output: List(List(A), List(B), List(C,C), List(D), List(E,E,E), List(F)) - * - * This lets us run an insert function on each list of identical transactions that will - * avoid inserting duplicates. - */ - - def groupIdenticals(list : List[ImporterTransaction]) : List[List[ImporterTransaction]] = { - list match{ - case Nil => Nil - case h::Nil => List(list) - case h::t => { - //transactions that are identical to the head of the list - val matches = list.filter(isIdentical(h, _)) - List(matches) ++ groupIdenticals(list diff matches) - } - } - } - - val grouped = groupIdenticals(ts) - - val insertedTransactions = - grouped - .map(identicals => insertIdenticalTransactions(identicals)) - .flatten - - reply( - InsertedTransactions(insertedTransactions) - ) - } - } - -} diff --git a/obp-api/src/test/scala/code/management/ImporterTest.scala b/obp-api/src/test/scala/code/management/ImporterTest.scala deleted file mode 100644 index acad983da8..0000000000 --- a/obp-api/src/test/scala/code/management/ImporterTest.scala +++ /dev/null @@ -1,375 +0,0 @@ -package code.management - -import java.util.TimeZone - -import code.api.util.APIUtil -import code.api.util.ErrorMessages._ -import code.bankconnectors.Connector -import code.model._ -import code.setup.{APIResponse, DefaultConnectorTestSetup, ServerSetup} -import code.util.Helper.MdcLoggable -import com.openbankproject.commons.model.enums.AccountRoutingScheme -import com.openbankproject.commons.model.{AccountId, Transaction} -import net.liftweb.util.TimeHelpers._ - - -class ImporterTest extends ServerSetup with MdcLoggable with DefaultConnectorTestSetup { - - override def beforeEach() = { - super.beforeEach() - wipeTestData() - } - - override def afterEach() = { - super.afterEach() - wipeTestData() - } - - val secretKeyHttpParamName = "secret" - val secretKeyValue = APIUtil.getPropsValue("importer_secret").openOrThrowException("Prop importer_secret not specified.") - - val dummyKind = "Transfer" - - def defaultFixture() = fixture("an-account") - - def fixture(accId : String) = new Fixture(accId) - - class Fixture(accId : String) { - lazy val bank = createBank(APIUtil.defaultBankId) - lazy val accountCurrency = "EUR" - lazy val account = createAccount(bank.bankId, AccountId(accId), accountCurrency) - val originalBalance = account.balance.toString - - val t1Value = "12.34" - val t1NewBalance = "3434.22" - val t1StartDate = "2012-01-04T18:06:22.000Z" - val t1EndDate = "2012-01-05T18:52:13.000Z" - - val t2Value = "13.54" - val t2NewBalance = "3447.76" - val t2StartDate = "2012-01-04T18:06:22.000Z" - val t2EndDate = "2012-01-06T18:52:13.000Z" - - val dummyLabel = "this is a description" - - //import transaction json is just an array of 'tJson'. - val testJson = importJson(List( - tJson(t1Value, t1NewBalance, t1StartDate, t1EndDate), - tJson(t2Value, t2NewBalance, t2StartDate, t2EndDate))) - - def importJson(transactionJsons: List[String]) = { - s"""[${transactionJsons.mkString(",")} - ]""" - } - - def tJson(value : String, newBalance : String, startDateString : String, endDateString : String) : String = { - //the double dollar signs are single dollar signs that have been escaped - s"""{ - | "obp_transaction": { - | "this_account": { - | "holder": "${account.accountHolder}", - | "number": "${account.number}", - | "kind": "${account.accountType}", - | "bank": { - | "IBAN": "${account.accountRoutings.find(_.scheme == AccountRoutingScheme.IBAN.toString).map(_.address).getOrElse("")}", - | "national_identifier": "${account.nationalIdentifier}", - | "name": "${account.bankName}" - | } - | }, - | "other_account": { - | "holder": "Client 1", - | "number": "123567", - | "kind": "current", - | "bank": { - | "IBAN": "UK12222879", - | "national_identifier": "uk.10010010", - | "name": "HSBC" - | } - | }, - | "details": { - | "kind": "$dummyKind", - | "label": "$dummyLabel" - | "posted": { - | "$$dt": "$startDateString" - | }, - | "completed": { - | "$$dt": "$endDateString" - | }, - | "new_balance": { - | "currency": "EUR", - | "amount": "$newBalance" - | }, - | "value": { - | "currency": "EUR", - | "amount": "$value" - | } - | } - | } - |}""".stripMargin //stripMargin removes the whitespace before the pipes, and the pipes themselves - } - } - - feature("Importing transactions via an API call") { - - def addTransactions(data : String, secretKey : Option[String]) : APIResponse = { - - val baseReq = (baseRequest / "obp_transactions_saver" / "api" / "transactions").POST - - val req = secretKey match { - case Some(key) => - // the < key) - case None => - baseReq - } - - makePostRequest(req, data) - } - - def checkOkay(t : Transaction, value : String, newBalance : String, startDate : String, endDate : String, label : String) = { - t.amount.toString should equal(value) - t.balance.toString should equal(newBalance) - t.description should equal(Some(label)) - - //the import api uses a different degree of detail than the main api (extra SSS) - //to compare the import api date string values to the dates returned from the api - //we need to parse them - val importJsonDateFormat = { - val f = APIUtil.DateWithMsFormat - //setting the time zone is important! - f.setTimeZone(TimeZone.getTimeZone("UTC")) - f - } - - t.transactionType should equal(dummyKind) - - //compare time as a long to avoid issues comparing Dates, e.g. java.util.Date vs java.sql.Date - t.startDate.getTime should equal(importJsonDateFormat.parse(startDate).getTime) - t.finishDate.head.getTime should equal(importJsonDateFormat.parse(endDate).getTime) - } - - scenario("Attempting to import transactions without using a secret key") { - val f = defaultFixture() - - Given("An account with no transactions") - val tsBefore = Connector.connector.vend.getTransactionsLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - tsBefore.size should equal(0) - - When("We try to import transactions without using a secret key") - val response = addTransactions(f.testJson, None) - - Then("We should get a 400") - response.code should equal(400) - - And("No transactions should be added") - val tsAfter = Connector.connector.vend.getTransactionsLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - tsAfter.size should equal(0) - } - - scenario("Attempting to import transactions with the incorrect secret key") { - val f = defaultFixture() - - Given("An account with no transactions") - val tsBefore = Connector.connector.vend.getTransactionsLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - tsBefore.size should equal(0) - - When("We try to import transactions with the incorrect secret key") - val response = addTransactions(f.testJson, Some(secretKeyValue + "asdsadsad")) - - Then("We should get a 401") - response.code should equal(401) - - And("No transactions should be added") - val tsAfter = Connector.connector.vend.getTransactionsLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - tsAfter.size should equal(0) - } - - scenario("Attempting to import transactions with the correct secret key") { - val f = defaultFixture() - - Given("An account with no transactions") - val tsBefore = Connector.connector.vend.getTransactionsLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - tsBefore.size should equal(0) - - When("We try to import transactions with the correct secret key") - val response = addTransactions(f.testJson, Some(secretKeyValue)) - - Then("We should get a 200") //implementation returns 200 and not 201, so we'll leave it like that - response.code should equal(200) - - And("Transactions should be added") - val tsAfter = Connector.connector.vend.getTransactionsLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - tsAfter.size should equal(2) - - And("The transactions should have the correct parameters") - val t1 = tsAfter(0) - checkOkay(t1, f.t1Value, f.t1NewBalance, f.t1StartDate, f.t1EndDate, f.dummyLabel) - - val t2 = tsAfter(1) - checkOkay(t2, f.t2Value, f.t2NewBalance, f.t2StartDate, f.t2EndDate, f.dummyLabel) - - - And("The account should have its balance set to the 'new_balance' value of the most recently completed transaction") - val account = Connector.connector.vend.getBankAccountLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - account.balance.toString should equal(f.t2NewBalance) //t2 has a later completed date than t1 - - And("The account should have accountLastUpdate set to the current time") - val dt = (now.getTime - account.lastUpdate.getTime) - dt < 1000 should equal(true) - - } - - scenario("Attempting to add 'identical' transactions") { - val f = defaultFixture() - def checkTransactionOkay(t : Transaction) = checkOkay(t, f.t1Value, f.t1NewBalance, f.t1StartDate, f.t1EndDate, f.dummyLabel) - - Given("An account with no transactions") - val tsBefore = Connector.connector.vend.getTransactionsLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - tsBefore.size should equal(0) - - When("We try to import two identical transactions with the correct secret key") - val t1Json = f.tJson(f.t1Value, f.t1NewBalance, f.t1StartDate, f.t1EndDate) - val importJson = f.importJson(List.fill(2)(t1Json)) - val response = addTransactions(importJson, Some(secretKeyValue)) - - //it should NOT complain about identical "new balances" as sometimes this value is unknown, or computed on a daily basis - //and hence all transactions for the day will have the same end balance - Then("We should get a 200") //implementation returns 200 and not 201, so we'll leave it like that - response.code should equal(200) - - And("Transactions should be added") - val tsAfter = Connector.connector.vend.getTransactionsLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - tsAfter.size should equal(2) - - And("The transactions should have the correct parameters") - tsAfter.foreach(checkTransactionOkay) - - And("The account should have its balance set to the 'new_balance' value of the most recently completed transaction") - val account = Connector.connector.vend.getBankAccountLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - account.balance.toString should equal(f.t1NewBalance) - - And("The account should have accountLastUpdate set to the current time") - val dt = (now.getTime - account.lastUpdate.getTime) - // TODO Get rid of environment-dependent test - // dt < 1000 should equal(true) - } - - scenario("Adding transactions that have already been imported") { - val f = defaultFixture() - def checkTransactionOkay(t : Transaction) = checkOkay(t, f.t1Value, f.t1NewBalance, f.t1StartDate, f.t1EndDate, f.dummyLabel) - - val t1Json = f.tJson(f.t1Value, f.t1NewBalance, f.t1StartDate, f.t1EndDate) - val importJson = f.importJson(List.fill(2)(t1Json)) - - Given("Two 'identical' existing transactions") - addTransactions(importJson, Some(secretKeyValue)) - val tsBefore = Connector.connector.vend.getTransactionsLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - tsBefore.size should equal(2) - - tsBefore.foreach(checkTransactionOkay) - - //remember lastUpdate time - var account = Connector.connector.vend.getBankAccountLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - val oldTime = if(account.lastUpdate != null) account.lastUpdate.getTime else 0 - - When("We try to add those transactions again") - val response = addTransactions(importJson, Some(secretKeyValue)) - - Then("We should get a 200") //implementation returns 200 and not 201, so we'll leave it like that - response.code should equal(200) - - And("There should still only be two transactions") - val tsAfter = Connector.connector.vend.getTransactionsLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - tsAfter.size should equal(2) - - tsAfter.foreach(checkTransactionOkay) - - And("The account should have accountLastUpdate set to the current time (different from first insertion)") - account = Connector.connector.vend.getBankAccountLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - val dt = (account.lastUpdate.getTime - oldTime) - dt > 0 should equal(true) - } - - scenario("Adding 'identical' transactions, some of which have already been imported") { - val f = defaultFixture() - def checkTransactionOkay(t : Transaction) = checkOkay(t, f.t1Value, f.t1NewBalance, f.t1StartDate, f.t1EndDate, f.dummyLabel) - - val t1Json = f.tJson(f.t1Value, f.t1NewBalance, f.t1StartDate, f.t1EndDate) - val initialImportJson = f.importJson(List.fill(2)(t1Json)) - val secondImportJson = f.importJson(List.fill(5)(t1Json)) - - Given("Two 'identical' existing transactions") - addTransactions(initialImportJson, Some(secretKeyValue)) - val tsBefore = Connector.connector.vend.getTransactionsLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - tsBefore.size should equal(2) - - checkTransactionOkay(tsBefore(0)) - checkTransactionOkay(tsBefore(1)) - - When("We try to add 5 copies of the transaction") - val response = addTransactions(secondImportJson, Some(secretKeyValue)) - - Then("We should get a 200") //implementation returns 200 and not 201, so we'll leave it like that - response.code should equal(200) - - And("There should now be 5 transactions") - val tsAfter = Connector.connector.vend.getTransactionsLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - tsAfter.size should equal(5) - - tsAfter.foreach(checkTransactionOkay) - } - - //TODO: this test case is pretty messy and was done in bit of a rush - scenario("Adding 'identical' transactions when such transactions already exist, but for another account") { - val f1 = fixture("an-account") - val f2 = fixture("another-account") - def checkF1TransactionOkay(t : Transaction) = checkOkay(t, f1.t1Value, f1.t1NewBalance, f1.t1StartDate, f1.t1EndDate, f1.dummyLabel) - - def checkF2TransactionOkay(t : Transaction) = checkOkay(t, f2.t1Value, f2.t1NewBalance, f2.t1StartDate, f2.t1EndDate, f2.dummyLabel) - - val t1F1Json = f1.tJson(f1.t1Value, f1.t1NewBalance, f1.t1StartDate, f1.t1EndDate) - val t1F1ImportJson = f1.importJson(List.fill(2)(t1F1Json)) - val t1F2Json = f2.tJson(f1.t1Value, f1.t1NewBalance, f1.t1StartDate, f1.t1EndDate) - val t1F2ImportJson = f2.importJson(List.fill(2)(t1F2Json)) - - Given("Two 'identical' existing transactions at a different account") - addTransactions(t1F1ImportJson, Some(secretKeyValue)) - - val f1TsBefore = Connector.connector.vend.getTransactionsLegacy(f1.account.bankId, f1.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - f1TsBefore.size should equal(2) - f1TsBefore.foreach(checkF1TransactionOkay) - - When("We add these same 'identical' transactions to a different account") - addTransactions(t1F2ImportJson, Some(secretKeyValue)) - - Then("There should be two transactions for each account") - val f1TsAfter = Connector.connector.vend.getTransactionsLegacy(f1.account.bankId, f1.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - f1TsAfter.size should equal(2) - f1TsAfter.foreach(checkF1TransactionOkay) - - val f2Ts = Connector.connector.vend.getTransactionsLegacy(f2.account.bankId, f2.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - f2Ts.size should equal(2) - f2Ts.foreach(checkF2TransactionOkay) - - } - - scenario("Attempting to import transactions using an incorrect json format") { - val f = defaultFixture() - Given("An account with no transactions") - val tsBefore = Connector.connector.vend.getTransactionsLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - tsBefore.size should equal(0) - - When("We try to import transactions with the correct secret key") - val response = addTransactions("""{"some_gibberish" : "json"}""", Some(secretKeyValue)) - - Then("We should get a 500") //implementation returns 500, so we'll leave it like that - response.code should equal(200) - - And("No transactions should be added") - val tsAfter = Connector.connector.vend.getTransactionsLegacy(f.account.bankId, f.account.accountId, None).map(_._1).openOrThrowException(attemptedToOpenAnEmptyBox) - tsAfter.size should equal(0) - } - - } - -} From 3eda8849edc2505d70d369740aa8513f847ed00d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Mili=C4=87?= Date: Thu, 21 May 2026 09:08:55 +0200 Subject: [PATCH 2/2] scripts: add Lift vs http4s ResourceDoc parity checker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read-only script that parses ResourceDoc(...) blocks from each version's APIMethods*.scala (Lift, source of truth) and Http4s*.scala, matches by endpoint name, and reports field-level diffs (summary, description, exampleRequestBody, successResponseBody, errorResponseBodies, tags, URL, verb). Handles fully-commented Lift files (v5.0.0/v5.1.0/v6.0.0) by stripping leading // before parsing. Initial run flags 725/944 endpoints diverging from Lift — most notably v6.0.0 (240/241), where http4s ResourceDocs lose example bodies and trim descriptions. --- .../check_lift_http4s_resource_doc_parity.py | 482 ++++++++++++++++++ 1 file changed, 482 insertions(+) create mode 100755 scripts/check_lift_http4s_resource_doc_parity.py diff --git a/scripts/check_lift_http4s_resource_doc_parity.py b/scripts/check_lift_http4s_resource_doc_parity.py new file mode 100755 index 0000000000..b3d54cc62a --- /dev/null +++ b/scripts/check_lift_http4s_resource_doc_parity.py @@ -0,0 +1,482 @@ +#!/usr/bin/env python3 +""" +check_lift_http4s_resource_doc_parity.py + +For each API version, compare ResourceDoc declarations in the Lift APIMethods*.scala +file against those in the corresponding Http4s*.scala file. Report fields that +differ (summary, description, example request body, success response body, +errors, tags). + +Read-only. Produces a report to stdout. No files are modified. + +Usage: + python3 scripts/check_lift_http4s_resource_doc_parity.py # all versions + python3 scripts/check_lift_http4s_resource_doc_parity.py v6_0_0 # one version + python3 scripts/check_lift_http4s_resource_doc_parity.py v5_1_0 v6_0_0 + python3 scripts/check_lift_http4s_resource_doc_parity.py --field=successResponseBody v6_0_0 + +The "Lift file" can be either: + - an active APIMethodsXYZ.scala (live Lift code, as in v1..v4) + - a stubbed APIMethodsXYZ.scala where the original Lift code is preserved as + line comments (as in v5.0.0 / v5.1.0 / v6.0.0). The script strips leading + `// ` from such files before parsing. + +The 8th/9th positional ResourceDoc fields are exampleRequestBody/successResponseBody +(see APIUtil.scala:1589 case class ResourceDoc). When http4s has `EmptyBody` and +Lift has a populated case class, that surfaces in the report. +""" + +import argparse +import os +import re +import sys +from collections import OrderedDict +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +API_ROOT = REPO_ROOT / "obp-api" / "src" / "main" / "scala" / "code" / "api" + +# Positional fields in ResourceDoc(...) — see APIUtil.scala:1589. +POSITIONAL_FIELDS = [ + "partialFunction", # 0 + "implementedInApiVersion", # 1 + "partialFunctionName", # 2 — endpoint identifier + "requestVerb", # 3 + "requestUrl", # 4 + "summary", # 5 + "description", # 6 + "exampleRequestBody", # 7 + "successResponseBody", # 8 + "errorResponseBodies", # 9 + "tags", # 10 +] + +# Default fields included in the diff report. +DEFAULT_DIFF_FIELDS = [ + "requestVerb", + "requestUrl", + "summary", + "description", + "exampleRequestBody", + "successResponseBody", + "errorResponseBodies", + "tags", +] + +# Named-arg fields that can follow the positional list. +NAMED_ARG_FIELDS = { + "roles", + "http4sPartialFunction", + "isFeatured", + "specialInstructions", + "createdByBankId", + "authMode", +} + + +def uncomment(source: str) -> str: + """Strip leading `//` (with optional single space) from every line. + + Used for fully-commented Lift files (v5.0.0 / v5.1.0 / v6.0.0). + """ + out = [] + for line in source.splitlines(): + m = re.match(r"^(\s*)//\s?(.*)$", line) + if m: + out.append(m.group(1) + m.group(2)) + else: + out.append(line) + return "\n".join(out) + + +def strip_inline_comments(source: str) -> str: + """Remove `//` line and `/* */` block comments outside string literals. + + Run AFTER uncomment(). This stops trailing comments (like the `// TODO` + after `nameOf(getCurrentUser),`) from being split into the next positional + argument. + """ + out = [] + i = 0 + n = len(source) + while i < n: + skipped = _skip_string_or_comment(source, i) + if skipped is not None: + # If the skip was for a string, copy it through; if it was a + # comment, drop it. + c = source[i] + is_string = ( + source.startswith('"""', i) + or (c in ("s", "f") and source.startswith('"""', i + 1)) + or c == '"' + ) + if is_string: + out.append(source[i:skipped]) + # else: comment — discard, but preserve a single newline if the + # line comment ate one, so line numbering/structure stays sane. + elif c == "/" and i + 1 < n and source[i + 1] == "/": + # Line comment: emit the newline if present (skipped includes it). + if skipped <= n and source[skipped - 1:skipped] == "\n": + out.append("\n") + i = skipped + continue + out.append(source[i]) + i += 1 + return "".join(out) + + +def _skip_string_or_comment(source: str, i: int): + """If position i starts a string/comment, return the index after it. + Otherwise return None. Handles triple-quoted, interpolated triple-quoted, + single-quoted, line and block comments. + """ + n = len(source) + c = source[i] + # triple-quoted plain + if source.startswith('"""', i): + j = source.find('"""', i + 3) + return n if j == -1 else j + 3 + # interpolated triple-quote: s"""...""" or f"""...""" + if c in ("s", "f") and source.startswith('"""', i + 1): + j = source.find('"""', i + 4) + return n if j == -1 else j + 3 + # plain double-quoted string (handle escapes) + if c == '"': + j = i + 1 + while j < n: + if source[j] == "\\": + j += 2 + continue + if source[j] == '"': + return j + 1 + j += 1 + return n + # line comment + if c == "/" and i + 1 < n and source[i + 1] == "/": + j = source.find("\n", i) + return n if j == -1 else j + 1 + # block comment + if c == "/" and i + 1 < n and source[i + 1] == "*": + j = source.find("*/", i + 2) + return n if j == -1 else j + 2 + return None + + +def scan_matching_close(source: str, start: int, open_ch: str, close_ch: str): + """Return index of the matching close char (depth-aware, string-aware).""" + depth = 1 + i = start + n = len(source) + while i < n: + skipped = _skip_string_or_comment(source, i) + if skipped is not None: + i = skipped + continue + c = source[i] + if c == open_ch: + depth += 1 + elif c == close_ch: + depth -= 1 + if depth == 0: + return i + i += 1 + return None + + +def find_resourcedoc_blocks(source: str): + """Yield (start, end, body) for each `(static)?[rR]esourceDocs += ResourceDoc(...)`. + + If a block cannot be closed (e.g. because its inner contents are doubly + commented after a mass-comment pass — these are dead in both code paths + anyway), skip it and continue searching after the opening paren. + """ + pattern = re.compile( + r"(?:static)?[rR]esourceDocs\s*\+=\s*ResourceDoc\s*\(" + ) + i = 0 + while True: + m = pattern.search(source, i) + if not m: + return + body_start = m.end() + close = scan_matching_close(source, body_start, "(", ")") + if close is None: + # Couldn't find matching close — skip this match and advance. + i = body_start + continue + yield m.start(), close + 1, source[body_start:close] + i = close + 1 + + +def split_top_level_args(body: str): + """Split on commas at brace/paren depth 0, respecting strings/comments.""" + args = [] + depth = 0 + cur = 0 + i = 0 + n = len(body) + while i < n: + skipped = _skip_string_or_comment(body, i) + if skipped is not None: + i = skipped + continue + c = body[i] + if c in "([{": + depth += 1 + elif c in ")]}": + depth -= 1 + elif c == "," and depth == 0: + args.append(body[cur:i].strip()) + cur = i + 1 + i += 1 + tail = body[cur:].strip() + if tail: + args.append(tail) + return args + + +def split_named(arg: str): + """If `arg` is `name = value` at top-level, return (name, value). Else (None, arg).""" + m = re.match(r"^\s*([A-Za-z_][A-Za-z0-9_]*)\s*=\s*(.+)$", arg, re.S) + if m and m.group(1) in NAMED_ARG_FIELDS: + return m.group(1), m.group(2).strip() + return None, arg + + +def parse_resourcedoc(body: str): + raw = split_top_level_args(body) + positional = [] + named = OrderedDict() + for a in raw: + n, v = split_named(a) + if n is None: + positional.append(a) + else: + named[n] = v + out = OrderedDict() + for fname, val in zip(POSITIONAL_FIELDS, positional): + out[fname] = val + if len(positional) > len(POSITIONAL_FIELDS): + # 12th positional is roles (Option[List[ApiRole]]). + extra = positional[len(POSITIONAL_FIELDS):] + if "roles" not in named and extra: + named["roles"] = extra[0] + out.update(named) + return out + + +def endpoint_name(part_fn_name: str) -> str: + """`nameOf(getBanks)` -> `getBanks`. Literal `"root"` -> `root`.""" + s = (part_fn_name or "").strip() + m = re.match(r"^nameOf\s*\(\s*([A-Za-z_][A-Za-z0-9_]*)\s*\)\s*$", s) + if m: + return m.group(1) + if s.startswith('"') and s.endswith('"'): + return s[1:-1] + return s + + +def normalize(s: str) -> str: + """Compare semantically — strip whitespace and equivalent syntactic forms.""" + if s is None: + return None + t = s + # `s"""..."""` and `"""..."""` are equivalent when there's no `${...}` — + # we still keep `s` if there are interpolations because content differs. + # For comparison, drop a bare leading `s` before a triple quote. + t = re.sub(r"\bs(\"\"\")", r"\1", t) + # `.stripMargin` calls are formatting-only — same content with margins stripped + # also matches the inner content. Drop `.stripMargin` and the leading `|` markers + # so descriptions compare on their actual text. + t = re.sub(r"\.stripMargin\b", "", t) + t = re.sub(r"^\s*\|", "", t, flags=re.M) + # Collapse whitespace. + t = re.sub(r"\s+", "", t) + # `X :: Y :: Nil` -> `List(X,Y)`-style normalization is tricky; instead, treat + # both `::Nil` and `List(...)` ends the same way by stripping both. + return t + + +def normalize_list(s: str) -> str: + """Normalize list expressions like `X :: Y :: Nil` vs `List(X, Y)`.""" + if s is None: + return None + items = [] + # Try `List(...)` form + m = re.match(r"^\s*List\s*\(\s*(.*?)\s*\)\s*$", s, re.S) + if m: + items = [x.strip() for x in split_top_level_args(m.group(1))] + else: + # Try `X :: Y :: ... :: Nil` + # Replace `Nil` with empty and split on `::` + parts = [p.strip() for p in re.split(r"::", s)] + if parts and parts[-1] == "Nil": + items = [p for p in parts[:-1] if p] + else: + items = parts + return "[" + ",".join(re.sub(r"\s+", "", x) for x in items) + "]" + + +def short(s, max_len: int = 200): + if s is None: + return "(absent)" + one_line = re.sub(r"\s+", " ", s).strip() + if len(one_line) > max_len: + return one_line[:max_len] + f" …({len(one_line)} chars)" + return one_line + + +def find_pair_for_version(version_dir: Path): + api_methods = list(version_dir.glob("APIMethods*.scala")) + http4s = list(version_dir.glob("Http4s*.scala")) + if not api_methods or not http4s: + return None + # Prefer the canonical filename (APIMethods600 / Http4s600); reject decorators + # like APIMethodsCustom300 by preferring shorter names that start with the + # exact prefix. + def pick(files, prefix): + ranked = sorted( + files, + key=lambda p: ( + 0 if re.match(rf"^{prefix}\d+\.scala$", p.name) else 1, + len(p.name), + ), + ) + return ranked[0] + return pick(api_methods, "APIMethods"), pick(http4s, "Http4s") + + +def collect_resourcedocs(path: Path): + source = path.read_text(encoding="utf-8") + active = re.search(r"^\s*(?:static)?[rR]esourceDocs\s*\+=\s*ResourceDoc\s*\(", source, re.M) + commented = re.search(r"^\s*//\s*(?:static)?[rR]esourceDocs\s*\+=\s*ResourceDoc\s*\(", source, re.M) + if not active and commented: + source = uncomment(source) + source = strip_inline_comments(source) + docs = OrderedDict() + for _, _, body in find_resourcedoc_blocks(source): + args = parse_resourcedoc(body) + if "partialFunctionName" not in args: + continue + name = endpoint_name(args["partialFunctionName"]) + # Key by endpoint name only — URL/verb get compared as fields so + # intentional renames during migration show up as field diffs rather + # than "missing" entries. + docs.setdefault(name, args) + return docs + + +def main(): + ap = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + ap.add_argument("versions", nargs="*", help="Versions like v6_0_0 (omit = all auto-discovered)") + ap.add_argument("--field", action="append", default=None, + help=f"Restrict diff to these fields. Repeatable. Default: {','.join(DEFAULT_DIFF_FIELDS)}") + ap.add_argument("--list-only", action="store_true", + help="Just list endpoints; do not print diffs") + ap.add_argument("--verbose-bodies", action="store_true", + help="Print full lift/http4s field values (default: truncated to 200 chars)") + args = ap.parse_args() + + fields = args.field or DEFAULT_DIFF_FIELDS + truncate_len = 100000 if args.verbose_bodies else 200 + + versions = args.versions + if not versions: + versions = sorted( + d.name for d in API_ROOT.iterdir() + if d.is_dir() and re.match(r"^v\d+_\d+_\d+$", d.name) + ) + + total_endpoints = 0 + total_mismatches = 0 + only_in_lift = [] + only_in_http4s = [] + per_field_counts = {f: 0 for f in fields} + per_version_summary = [] + + for v in versions: + vdir = API_ROOT / v + pair = find_pair_for_version(vdir) + if not pair: + print(f"[{v}] no APIMethods/Http4s pair found — skipping", file=sys.stderr) + continue + lift_path, http_path = pair + lift_docs = collect_resourcedocs(lift_path) + http_docs = collect_resourcedocs(http_path) + + lift_keys = set(lift_docs.keys()) + http_keys = set(http_docs.keys()) + + print(f"\n=== {v} ===") + print(f" lift: {lift_path.relative_to(REPO_ROOT)} ({len(lift_docs)} docs)") + print(f" http4s: {http_path.relative_to(REPO_ROOT)} ({len(http_docs)} docs)") + + miss_http = sorted(lift_keys - http_keys) + miss_lift = sorted(http_keys - lift_keys) + if miss_http: + print(f" ⚠ {len(miss_http)} endpoint(s) in Lift but NOT in Http4s:") + for name in miss_http: + d = lift_docs[name] + verb = d.get("requestVerb", "").strip().strip('"') + url = d.get("requestUrl", "").strip().strip('"') + print(f" - {verb:6} {url} ({name})") + only_in_lift.extend((v, name) for name in miss_http) + if miss_lift: + print(f" ⚠ {len(miss_lift)} endpoint(s) in Http4s but NOT in Lift:") + for name in miss_lift: + d = http_docs[name] + verb = d.get("requestVerb", "").strip().strip('"') + url = d.get("requestUrl", "").strip().strip('"') + print(f" - {verb:6} {url} ({name})") + only_in_http4s.extend((v, name) for name in miss_lift) + + shared = sorted(lift_keys & http_keys) + version_mismatches = 0 + version_endpoints = len(shared) + total_endpoints += version_endpoints + + if not args.list_only: + for name in shared: + l = lift_docs[name] + h = http_docs[name] + diffs = [] + for f in fields: + lv = l.get(f) + hv = h.get(f) + if f in ("errorResponseBodies", "tags"): + eq = normalize_list(lv) == normalize_list(hv) + else: + eq = normalize(lv) == normalize(hv) + if not eq: + diffs.append((f, lv, hv)) + per_field_counts[f] += 1 + if diffs: + version_mismatches += 1 + total_mismatches += 1 + verb = h.get("requestVerb", l.get("requestVerb", "")).strip().strip('"') + url = h.get("requestUrl", l.get("requestUrl", "")).strip().strip('"') + print(f"\n ✗ {verb:6} {url} ({name})") + for fname, lv, hv in diffs: + print(f" [{fname}]") + print(f" lift: {short(lv, truncate_len)}") + print(f" http4s: {short(hv, truncate_len)}") + if version_mismatches == 0 and not miss_http and not miss_lift: + print(" ✓ all shared endpoints match") + per_version_summary.append((v, version_endpoints, version_mismatches, + len(miss_http), len(miss_lift))) + + print("\n=== SUMMARY ===") + print(f"{'version':10} {'shared':>8} {'mismatch':>10} {'only-lift':>10} {'only-http4s':>12}") + for v, sh, mm, ml, mh in per_version_summary: + print(f"{v:10} {sh:>8} {mm:>10} {ml:>10} {mh:>12}") + print(f"\nTotal endpoints compared: {total_endpoints}") + print(f"Total mismatches: {total_mismatches}") + print(f"Per-field counts:") + for f, c in per_field_counts.items(): + print(f" {f:25} {c}") + + return 0 if total_mismatches == 0 and not only_in_lift and not only_in_http4s else 1 + + +if __name__ == "__main__": + sys.exit(main())