From 54d4118d32873424ad71fb56de3da7bebb545b82 Mon Sep 17 00:00:00 2001 From: Everett Sochowski Date: Thu, 10 Apr 2014 13:16:31 +0200 Subject: [PATCH] Mostly working payments that is missing a check that both parties have the same currency accounts --- .../scala/code/api/1_2_1/OBPAPI1.2.1.scala | 149 ++++++++++-------- .../code/model/dataAccess/Connectors.scala | 4 +- src/test/scala/code/api/API121Test.scala | 47 ++++-- src/test/scala/code/api/ServerSetup.scala | 1 + 4 files changed, 123 insertions(+), 78 deletions(-) diff --git a/src/main/scala/code/api/1_2_1/OBPAPI1.2.1.scala b/src/main/scala/code/api/1_2_1/OBPAPI1.2.1.scala index 13f5da544..abf0ca862 100644 --- a/src/main/scala/code/api/1_2_1/OBPAPI1.2.1.scala +++ b/src/main/scala/code/api/1_2_1/OBPAPI1.2.1.scala @@ -874,54 +874,59 @@ def checkIfLocationPossible(lat:Double,lon:Double) : Box[Unit] = { }) import code.model.dataAccess.OBPEnvelope - def createTransaction(account : BankAccount, makeTransJs : MakeTransactionJson) : Box[OBPEnvelope] = { + def createTransaction(account : BankAccount, otherBankId : String, + otherAccountId : String, amount : String) : Box[OBPEnvelope] = { - val oldBalance = account.balance + val oldFromBalance = account.balance - //TODO: get Account from makeTransJs import code.model.dataAccess.Account import code.model.dataAccess.HostedBank + import java.text.SimpleDateFormat for { - otherBank <- HostedBank.find("permalink" -> makeTransJs.bank_id) ?~! "no other bank found" + otherBank <- HostedBank.find("permalink" -> otherBankId) ?~! "no other bank found" //yeah dumb, but blame the bad mongodb structure that attempts to use foreign keys - val otherAccs = Account.findAll(("permalink" -> makeTransJs.account_id)) - otherAcc <- Box(otherAccs.filter(_.bankId == otherBank.id.get.toString).headOption) ?~! s"no other acc found. ${otherAccs.size} searched for matching bank ${otherBank.id.get.toString} :: ${otherAccs.map(_.toString)}" - amt <- tryo {BigDecimal(makeTransJs.amount)} ?~! "amount not convertable to number"//TODO: this completely ignores currency + val otherAccs = Account.findAll(("permalink" -> otherAccountId)) + otherAcc <- Box(otherAccs.filter(_.bankPermalink == otherBank.permalink.get).headOption) ?~! s"no other acc found. ${otherAccs.size} searched for matching bank ${otherBank.id.get.toString} :: ${otherAccs.map(_.toString)}" + amt <- tryo {BigDecimal(amount)} ?~! "amount not convertable to number"//TODO: this completely ignores currency val transTime = now - val envjson = + val thisAccs = Account.findAll(("permalink" -> account.permalink)) + thisAcc <- Box(thisAccs.filter(_.bankPermalink == account.bankPermalink).headOption) ?~! s"no this acc found. ${thisAccs.size} searched for matching bank ${account.bankPermalink}?" + //mongodb/the lift mongo thing wants a literal Z in the timestamp, apparently + val envJsonDateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'") + val envJson = ("obp_transaction" -> ("this_account" -> - ("holder" -> account.name) ~ + ("holder" -> account.owners.headOption.map(_.name).getOrElse("")) ~ //TODO: this is rather fragile... ("number" -> account.number) ~ - ("kind" -> "") ~ + ("kind" -> thisAcc.kind.get) ~ ("bank" -> ("IBAN" -> account.iban.getOrElse("")) ~ ("national_identifier" -> account.nationalIdentifier) ~ - ("name" -> account.name))) ~ - ("other_account" -> - ("holder" -> otherAcc.holder.get) ~ - ("number" -> otherAcc.number.get) ~ - ("kind" -> otherAcc.kind.get) ~ - ("bank" -> - ("IBAN" -> "") ~ - ("national_identifier" -> otherBank.national_identifier.get) ~ - ("name" -> otherBank.name.get))) ~ - ("details" -> - ("type_en" -> "") ~ - ("type_de" -> "") ~ - ("posted" -> - ("$dt" -> dateFormat.format(transTime)) - ) ~ - ("completed" -> - ("$dt" -> dateFormat.format(transTime)) - ) ~ - ("new_balance" -> - ("currency" -> account.currency) ~ - ("amount" -> (oldBalance + amt).toString)) ~ - ("value" -> - ("currency" -> account.currency) ~ - ("amount" -> amt)))) - saved <- saveTransaction(envjson) + ("name" -> account.bankPermalink))) ~ + ("other_account" -> + ("holder" -> otherAcc.holder.get) ~ + ("number" -> otherAcc.number.get) ~ + ("kind" -> otherAcc.kind.get) ~ + ("bank" -> + ("IBAN" -> "") ~ + ("national_identifier" -> otherBank.national_identifier.get) ~ + ("name" -> otherBank.name.get))) ~ + ("details" -> + ("type_en" -> "") ~ + ("type_de" -> "") ~ + ("posted" -> + ("$dt" -> envJsonDateFormat.format(transTime)) + ) ~ + ("completed" -> + ("$dt" -> envJsonDateFormat.format(transTime)) + ) ~ + ("new_balance" -> + ("currency" -> account.currency) ~ + ("amount" -> (oldFromBalance + amt).toString)) ~ + ("value" -> + ("currency" -> account.currency) ~ + ("amount" -> amt.toString)))) + saved <- saveTransaction(envJson) } yield saved } @@ -940,20 +945,21 @@ def checkIfLocationPossible(lat:Double,lon:Double) : Box[Unit] = { import code.model.dataAccess.Account val accounts = Account.findAll(("number" -> accountNumber) ~ ("kind" -> accountKind) ~ ("holder" -> holder)) //Now get the one that actually belongs to the right bank - val wantedAccount = accounts.find(_.bankName == bankName) + val findFunc = (x : Account) => { + x.bankPermalink == bankName + } + val wantedAccount = accounts.find(findFunc) wantedAccount match { case Some(account) => { def updateAccountBalance() = { - val newest = OBPEnvelope.findAll(("obp_transaction.this_account.number" -> accountNumber) ~ - ("obp_transaction.this_account.kind" -> accountKind) ~ - ("obp_transaction.this_account.bank.name" -> bankName), - ("obp_transaction.details.completed" -> -1), Limit(1)).headOption - if (newest.isDefined) { - logger.debug("Updating current balance for " + bankName + "/" + accountNumber + "/" + accountKind) - account.balance(newest.get.obp_transaction.get.details.get.new_balance.get.amount.get).save - } else logger.warn("Could not update latest account balance") + logger.debug("Updating current balance for " + bankName + "/" + accountNumber + "/" + accountKind) + account.balance(e.obp_transaction.get.details.get.new_balance.get.amount.get).save + logger.debug("Saving new transaction") + val metadataCreated = e.createMetadataReference + if(metadataCreated.isDefined) e.save + else Failure("Server error, problem creating transaction metadata") } - account.lastUpdate(new Date).save + account.lastUpdate(new Date) updateAccountBalance() Full(e) } @@ -965,15 +971,6 @@ def checkIfLocationPossible(lat:Double,lon:Double) : Box[Unit] = { } case class MakeTransactionJson(bank_id : String, account_id : String, amount : String) - oauthServe(apiPrefix { - - //post transaction TODO: this is a work in progress/hackathon hack! most of this - //code needs to be moved into a connector function - case "foo" :: Nil JsonPost json -> _ => { - user => Full(errorJsonResponse("argh")) - } - }) - oauthServe(apiPrefix { @@ -986,14 +983,36 @@ def checkIfLocationPossible(lat:Double,lon:Double) : Box[Unit] = { Full(errorJsonResponse("nothing to see here!")) } else { for { - //u <- user ?~ "asdad" - account <- BankAccount(bankId, accountId) ?~ "asda2323d" - view <- View.fromUrl(viewId, account) ?~ "FFFFZ"//TODO: need to actually check something with this + u <- user ?~ "asdad" + fromAccount <- BankAccount(bankId, accountId) ?~ s"account $accountId not found at bank $bankId" + owner <- booleanToBox(u.ownerAccess(fromAccount), "user does not have access to owner view") + view <- View.fromUrl(viewId, fromAccount) ?~ s"view $viewId not found"//TODO: this isn't actually used, unlike for GET transactions makeTransJson <- tryo{json.extract[MakeTransactionJson]} ?~ {"wrong json format"} + toAccount <- { + BankAccount(makeTransJson.bank_id, makeTransJson.account_id) ?~! {"Transaction 'to' party with " + + s" account id ${makeTransJson.account_id} at bank ${makeTransJson.bank_id}" + + " not found on the open bank project"} + } + rawAmt <- tryo {BigDecimal(makeTransJson.amount)} ?~! "amount not convertable to number"//TODO: this completely ignores currency + isPositiveAmtToSend <- booleanToBox(rawAmt > BigDecimal("0"), "Can't send a payment with a value of 0 or less") } yield { - val created = createTransaction(account, makeTransJson) + val fromTransAmt = -rawAmt //from account balance should decrease + val toTransAmt = rawAmt //to account balance should increase + val createdFromTrans = createTransaction(fromAccount, makeTransJson.bank_id, + makeTransJson.account_id, fromTransAmt.toString) + // other party in the transaction for the "to" account is the "from" account + val createToTrans = createTransaction(toAccount, bankId, accountId, toTransAmt.toString) - created match { + /** + * WARNING!!! There is no check currently being done that the new transaction + update balance + * of the account receiving the money were properly saved. This payment implementation is for + * demo/sandbox/test purposes ONLY! + * + * I have not bothered to spend time doing anything about this. I see no point in trying to + * implement ACID transactions in mongodb when a real payment system will not use mongodb. + */ + + createdFromTrans match { case Full(c) => { val successJson : JValue = ("transaction_id" -> c.id.get.toString) successJsonResponse(successJson) @@ -1060,17 +1079,17 @@ def checkIfLocationPossible(lat:Double,lon:Double) : Box[Unit] = { }) oauthServe(apiPrefix { - //get transaction by id + //get transaction by id case "banks" :: bankId :: "accounts" :: accountId :: viewId :: "transactions" :: transactionId :: "transaction" :: Nil JsonGet json => { user => for { - account <- BankAccount(bankId, accountId) - view <- View.fromUrl(viewId, account) + account <- BankAccount(bankId, accountId) ?~! s"Bank account $accountId not found at bank $bankId" + view <- View.fromUrl(viewId, account) ?~! s"View $viewId not found for account" moderatedTransaction <- account.moderatedTransaction(transactionId, view, user) } yield { - val json = JSONFactory.createTransactionJSON(moderatedTransaction) - successJsonResponse(Extraction.decompose(json)) - } + val json = JSONFactory.createTransactionJSON(moderatedTransaction) + successJsonResponse(Extraction.decompose(json)) + } } }) diff --git a/src/main/scala/code/model/dataAccess/Connectors.scala b/src/main/scala/code/model/dataAccess/Connectors.scala index cea1fa9ec..a56f6cdc1 100644 --- a/src/main/scala/code/model/dataAccess/Connectors.scala +++ b/src/main/scala/code/model/dataAccess/Connectors.scala @@ -331,8 +331,8 @@ class MongoDBLocalStorage extends LocalStorage { private def getTransaction(id : String, bankPermalink : String, accountPermalink : String) : Box[Transaction] = { for{ - bank <- getHostedBank(bankPermalink) - account <- bank.getAccount(accountPermalink) + bank <- getHostedBank(bankPermalink) ?~! s"Transaction not found: bank $bankPermalink not found" + account <- bank.getAccount(accountPermalink) ?~! s"Transaction not found: account $accountPermalink not found" objectId <- tryo{new ObjectId(id)} ?~ {"Transaction "+id+" not found"} envelope <- OBPEnvelope.find(account.transactionsForAccount.put("_id").is(objectId).get) transaction <- createTransaction(envelope,account) diff --git a/src/test/scala/code/api/API121Test.scala b/src/test/scala/code/api/API121Test.scala index 8cc981bd7..fad6cdfb2 100644 --- a/src/test/scala/code/api/API121Test.scala +++ b/src/test/scala/code/api/API121Test.scala @@ -45,9 +45,11 @@ import _root_.net.liftweb.json.Serialization.write import _root_.net.liftweb.json.JsonAST.{JValue, JObject} import net.liftweb.json.NoTypeHints import net.liftweb.json.JsonDSL._ +import net.liftweb.json._ import net.liftweb.mapper.By import java.util.Date import code.model.TokenType._ +import scala.util.Random._ import code.model.{Consumer => OBPConsumer, Token => OBPToken, ViewUpdateData, View, ViewCreationJSON} import code.model.dataAccess.{APIUser, HostedAccount, ViewImpl, ViewPrivileges, Account, LocalStorage} import code.api.test.{ServerSetup, APIResponse} @@ -736,7 +738,8 @@ class API1_2_1Test extends ServerSetup{ val bankId = randomBank val acc1 = randomPrivateAccount(bankId) val acc2 = privateAccountThatsNot(bankId, acc1.id) - val view = randomViewPermalink(bankId, acc1) + + val view = "owner" def getFromAccount : BankAccount = { BankAccount(bankId, acc1.id).getOrElse(fail("couldn't get from account")) } @@ -749,37 +752,59 @@ class API1_2_1Test extends ServerSetup{ val toAccount = getToAccount - val beforeBalance = toAccount.balance + val beforeFromBalance = fromAccount.balance + val beforeToBalance = toAccount.balance - val amt = 12.50 + val amt = BigDecimal("12.50") val paymentJson = ("bank_id" -> toAccount.bankPermalink) ~ ("account_id" -> toAccount.permalink) ~ ("amount" -> amt.toString) + def postTransaction(bankId: String, accountId: String, viewId: String, consumerAndToken: Option[(Consumer, Token)]): APIResponse = { - val request = v1_2Request / "banks" / bankId / "accounts" / accountId / viewId / "transactions" <@ (consumerAndToken) - //makeGetRequest(request) - makePostRequest(request, paymentJson.toString) + val request = (v1_2Request / "banks" / bankId / "accounts" / accountId / viewId / "transactions").POST <@(consumerAndToken) + makePostRequest(request, compact(render(paymentJson))) } val postResult = postTransaction(fromAccount.bankPermalink, fromAccount.permalink, view, user1) + val transId : String = (postResult.body \ "transaction_id") match { case JString(i) => i case _ => "" } transId should not equal("") - + val reply = getTransaction( - toAccount.bankPermalink, toAccount.permalink, view, transId, user1) + fromAccount.bankPermalink, fromAccount.permalink, view, transId, user1) + Then("we should get a 200 ok code") reply.code should equal (200) val transJson = reply.body.extract[TransactionJSON] - val transAmt = transJson.details.value.amount - transAmt should equal(amt.toString) + val fromAccountTransAmt = transJson.details.value.amount + //the from account transaction should have a negative value + //since money left the account + fromAccountTransAmt should equal((-amt).toString) + + val expectedNewFromBalance = beforeFromBalance - amt - getToAccount.balance should equal(beforeBalance + BigDecimal(amt.toString)) + transJson.details.new_balance.amount should equal(expectedNewFromBalance.toString) + + + getFromAccount.balance should equal(expectedNewFromBalance) + + val toAccountTransactionsReq = getTransactions(toAccount.bankPermalink, toAccount.permalink, view, user1) + toAccountTransactionsReq.code should equal(200) + val toAccountTransactions = toAccountTransactionsReq.body.extract[TransactionsJSON] + val newestToAccountTransaction = toAccountTransactions.transactions(0) + + //here amt should be positive (unlike in the transaction in the "from" account") + newestToAccountTransaction.details.value.amount should equal(amt.toString) + + val expectedNewToBalance = beforeToBalance + amt + newestToAccountTransaction.details.new_balance.amount should equal(expectedNewToBalance.toString) + getToAccount.balance should equal(expectedNewToBalance) } } diff --git a/src/test/scala/code/api/ServerSetup.scala b/src/test/scala/code/api/ServerSetup.scala index fb826c2ab..3923b194e 100644 --- a/src/test/scala/code/api/ServerSetup.scala +++ b/src/test/scala/code/api/ServerSetup.scala @@ -203,6 +203,7 @@ trait ServerSetup extends FeatureSpec */ def makePostRequest(req: Req, json: String = ""): APIResponse = { req.addHeader("Content-Type", "application/json") + req.addHeader("Accept", "application/json") req.setBody(json) val jsonReq = (req).POST getAPIResponse(jsonReq)