Validate accounts don't allow a negative balance

lmirabal · lmirabal · commit b15318b1358f · 2020-12-30T13:02:00.000Z
Use result type to express error case when withdrawing funds.

This is one example where driving the solution outside-in makes sense,
since the UI being the ultimate consumer should define what details the
domain needs to provide.
diff --git a/domain/build.gradle b/domain/build.gradle
@@ -3,6 +3,8 @@ plugins {
 }
 
 dependencies {
+    api 'dev.forkhandles:result4k:1.6.0.0'
+
     testFixturesImplementation 'org.jetbrains.kotlin:kotlin-test-junit5'
     testFixturesImplementation 'org.junit.jupiter:junit-jupiter-api:5.7.0'
     testFixturesImplementation 'com.natpryce:hamkrest:1.8.0.1'
diff --git a/domain/src/main/kotlin/lmirabal/bank/Bank.kt b/domain/src/main/kotlin/lmirabal/bank/Bank.kt
@@ -1,12 +1,14 @@
 package lmirabal.bank
 
+import dev.forkhandles.result4k.Result
 import lmirabal.bank.model.Amount
 import lmirabal.bank.model.BankAccount
 import lmirabal.bank.model.BankAccountId
+import lmirabal.bank.model.NotEnoughFunds
 
 interface Bank {
     fun createAccount(): BankAccount
     fun listAccounts(): List<BankAccount>
     fun deposit(id: BankAccountId, amount: Amount): BankAccount
-    fun withdraw(id: BankAccountId, amount: Amount): BankAccount
+    fun withdraw(id: BankAccountId, amount: Amount): Result<BankAccount, NotEnoughFunds>
 }
diff --git a/domain/src/main/kotlin/lmirabal/bank/BankService.kt b/domain/src/main/kotlin/lmirabal/bank/BankService.kt
@@ -1,9 +1,12 @@
 package lmirabal.bank
 
+import dev.forkhandles.result4k.Result
+import dev.forkhandles.result4k.peek
 import lmirabal.bank.data.BankAccountRepository
 import lmirabal.bank.model.Amount
 import lmirabal.bank.model.BankAccount
 import lmirabal.bank.model.BankAccountId
+import lmirabal.bank.model.NotEnoughFunds
 
 class BankService(
     private val accountRepository: BankAccountRepository,
@@ -22,9 +25,9 @@ class BankService(
             .also { updatedAccount -> accountRepository.update(updatedAccount) }
     }
 
-    override fun withdraw(id: BankAccountId, amount: Amount): BankAccount {
+    override fun withdraw(id: BankAccountId, amount: Amount): Result<BankAccount, NotEnoughFunds> {
         val account = accountRepository.list().first { it.id == id }
         return account.withdraw(amount)
-            .also { updatedAccount -> accountRepository.update(updatedAccount) }
+            .peek { updatedAccount -> accountRepository.update(updatedAccount) }
     }
 }
diff --git a/domain/src/main/kotlin/lmirabal/bank/model/Amount.kt b/domain/src/main/kotlin/lmirabal/bank/model/Amount.kt
@@ -1,6 +1,6 @@
 package lmirabal.bank.model
 
-data class Amount(val minorUnits: Long) {
+data class Amount(val minorUnits: Long) : Comparable<Amount> {
     init {
         require(minorUnits >= 0)
     }
@@ -13,6 +13,10 @@ data class Amount(val minorUnits: Long) {
         return Amount(minorUnits - other.minorUnits)
     }
 
+    override fun compareTo(other: Amount): Int {
+        return minorUnits.compareTo(other.minorUnits)
+    }
+
     companion object {
         val ZERO = Amount(0)
     }
diff --git a/domain/src/main/kotlin/lmirabal/bank/model/BankAccount.kt b/domain/src/main/kotlin/lmirabal/bank/model/BankAccount.kt
@@ -1,14 +1,18 @@
 package lmirabal.bank.model
 
+import dev.forkhandles.result4k.Failure
+import dev.forkhandles.result4k.Result
+import dev.forkhandles.result4k.Success
 import java.util.UUID
 
 data class BankAccount(val id: BankAccountId, val balance: Amount) {
     fun deposit(amount: Amount): BankAccount {
         return copy(balance = balance + amount)
     }
 
-    fun withdraw(amount: Amount): BankAccount {
-        return copy(balance = balance - amount)
+    fun withdraw(amount: Amount): Result<BankAccount, NotEnoughFunds> {
+        return if (amount <= balance) Success(copy(balance = balance - amount))
+        else Failure(NotEnoughFunds(id, balance, amount - balance))
     }
 }
 
@@ -18,3 +22,5 @@ data class BankAccountId(val value: UUID) {
         fun random() = BankAccountId(UUID.randomUUID())
     }
 }
+
+data class NotEnoughFunds(val id: BankAccountId, val balance: Amount, val additionalFundsRequired: Amount)
diff --git a/domain/src/test/kotlin/lmirabal/bank/model/AmountTest.kt b/domain/src/test/kotlin/lmirabal/bank/model/AmountTest.kt
@@ -2,8 +2,13 @@ package lmirabal.bank.model
 
 import com.natpryce.hamkrest.assertion.assertThat
 import com.natpryce.hamkrest.equalTo
+import com.natpryce.hamkrest.greaterThan
+import com.natpryce.hamkrest.greaterThanOrEqualTo
+import com.natpryce.hamkrest.lessThan
+import com.natpryce.hamkrest.lessThanOrEqualTo
 import com.natpryce.hamkrest.throws
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertAll
 
 class AmountTest {
     @Test
@@ -24,4 +29,14 @@ class AmountTest {
 
         assertThat(result, equalTo(Amount(20)))
     }
+
+    @Test
+    fun `can be compared`() {
+        assertAll(
+            { assertThat(Amount(10), lessThan(Amount(20))) },
+            { assertThat(Amount(20), lessThanOrEqualTo(Amount(20))) },
+            { assertThat(Amount(20), greaterThan(Amount(10))) },
+            { assertThat(Amount(20), greaterThanOrEqualTo(Amount(20))) },
+        )
+    }
 }
diff --git a/domain/src/test/kotlin/lmirabal/bank/model/BankAccountTest.kt b/domain/src/test/kotlin/lmirabal/bank/model/BankAccountTest.kt
@@ -2,6 +2,9 @@ package lmirabal.bank.model
 
 import com.natpryce.hamkrest.assertion.assertThat
 import com.natpryce.hamkrest.equalTo
+import com.natpryce.hamkrest.present
+import dev.forkhandles.result4k.failureOrNull
+import dev.forkhandles.result4k.valueOrNull
 import org.junit.jupiter.api.Test
 
 class BankAccountTest {
@@ -20,8 +23,21 @@ class BankAccountTest {
         val id = BankAccountId.random()
         val account = BankAccount(id, Amount(300))
 
-        val updatedAccount = account.withdraw(Amount(200))
+        val result = account.withdraw(Amount(200))
 
-        assertThat(updatedAccount, equalTo(BankAccount(id, Amount(100))))
+        assertThat(result.valueOrNull(), present(equalTo(BankAccount(id, Amount(100)))))
+    }
+
+    @Test
+    fun `cannot withdraw more than balance`() {
+        val id = BankAccountId.random()
+        val account = BankAccount(id, Amount(300))
+
+        val result = account.withdraw(Amount(500))
+
+        assertThat(
+            result.failureOrNull(),
+            present(equalTo(NotEnoughFunds(id, Amount(300), Amount(200))))
+        )
     }
 }
diff --git a/domain/src/testFixtures/kotlin/lmirabal/bank/BankTest.kt b/domain/src/testFixtures/kotlin/lmirabal/bank/BankTest.kt
@@ -2,7 +2,11 @@ package lmirabal.bank
 
 import com.natpryce.hamkrest.assertion.assertThat
 import com.natpryce.hamkrest.equalTo
+import com.natpryce.hamkrest.present
+import dev.forkhandles.result4k.failureOrNull
+import dev.forkhandles.result4k.valueOrNull
 import lmirabal.bank.model.Amount
+import lmirabal.bank.model.NotEnoughFunds
 import org.junit.jupiter.api.Test
 
 abstract class BankTest {
@@ -35,6 +39,19 @@ abstract class BankTest {
         bank.deposit(bankAccount.id, Amount(300))
         val updatedAccount = bank.withdraw(bankAccount.id, Amount(200))
 
-        assertThat(updatedAccount.balance, equalTo(Amount(100)))
+        assertThat(updatedAccount.valueOrNull()?.balance, present(equalTo(Amount(100))))
+    }
+
+    @Test
+    fun `cannot withdraw from account more than balance`() {
+        val bankAccount = bank.createAccount()
+
+        bank.deposit(bankAccount.id, Amount(300))
+        val updatedAccount = bank.withdraw(bankAccount.id, Amount(500))
+
+        assertThat(
+            updatedAccount.failureOrNull(),
+            present(equalTo(NotEnoughFunds(bankAccount.id, Amount(300), Amount(200))))
+        )
     }
 }
diff --git a/http/src/main/kotlin/lmirabal/bank/http/BankHttp.kt b/http/src/main/kotlin/lmirabal/bank/http/BankHttp.kt
@@ -1,16 +1,20 @@
 package lmirabal.bank.http
 
+import dev.forkhandles.result4k.map
+import dev.forkhandles.result4k.recover
 import lmirabal.bank.Bank
 import lmirabal.bank.BankService
 import lmirabal.bank.data.InMemoryBankAccountRepository
 import lmirabal.bank.model.Amount
 import lmirabal.bank.model.BankAccount
 import lmirabal.bank.model.BankAccountId
+import lmirabal.bank.model.NotEnoughFunds
 import org.http4k.core.Body
 import org.http4k.core.HttpHandler
 import org.http4k.core.Method.GET
 import org.http4k.core.Method.POST
 import org.http4k.core.Response
+import org.http4k.core.Status.Companion.BAD_REQUEST
 import org.http4k.core.Status.Companion.OK
 import org.http4k.core.with
 import org.http4k.format.Jackson.auto
@@ -28,6 +32,7 @@ internal val bankAccountLens = Body.auto<BankAccount>().toLens()
 internal val bankAccountListLens = Body.auto<List<BankAccount>>().toLens()
 internal val accountIdLens = Path.map({ BankAccountId(UUID.fromString(it)) }, { it.value.toString() }).of("id")
 internal val amountLens = Body.auto<Amount>().toLens()
+internal val notEnoughFundsLens = Body.auto<NotEnoughFunds>().toLens()
 
 internal fun bankHttp(bank: Bank): HttpHandler {
     return routes(
@@ -46,7 +51,9 @@ internal fun bankHttp(bank: Bank): HttpHandler {
                 val accountId = accountIdLens(request)
                 val amount = amountLens(request)
 
-                Response(OK).with(bankAccountLens of bank.withdraw(accountId, amount))
+                bank.withdraw(accountId, amount)
+                    .map { updatedAccount -> Response(OK).with(bankAccountLens of updatedAccount) }
+                    .recover { failure: NotEnoughFunds -> Response(BAD_REQUEST).with(notEnoughFundsLens of failure) }
             }
         ),
     )
diff --git a/http/src/main/kotlin/lmirabal/bank/http/BankHttpClient.kt b/http/src/main/kotlin/lmirabal/bank/http/BankHttpClient.kt
@@ -1,13 +1,19 @@
 package lmirabal.bank.http
 
+import dev.forkhandles.result4k.Failure
+import dev.forkhandles.result4k.Result
+import dev.forkhandles.result4k.Success
 import lmirabal.bank.Bank
 import lmirabal.bank.model.Amount
 import lmirabal.bank.model.BankAccount
 import lmirabal.bank.model.BankAccountId
+import lmirabal.bank.model.NotEnoughFunds
 import org.http4k.core.HttpHandler
 import org.http4k.core.Method.GET
 import org.http4k.core.Method.POST
 import org.http4k.core.Request
+import org.http4k.core.Response
+import org.http4k.core.Status
 import org.http4k.core.with
 
 class BankHttpClient(val http: HttpHandler) : Bank {
@@ -22,18 +28,23 @@ class BankHttpClient(val http: HttpHandler) : Bank {
     }
 
     override fun deposit(id: BankAccountId, amount: Amount): BankAccount {
-        return changeBalanceAction(BANK_ACCOUNT_DEPOSIT_PATH, id, amount)
+        val response = changeBalanceAction(BANK_ACCOUNT_DEPOSIT_PATH, id, amount)
+        return bankAccountLens(response)
     }
 
-    override fun withdraw(id: BankAccountId, amount: Amount): BankAccount {
-        return changeBalanceAction(BANK_ACCOUNT_WITHDRAWAL_PATH, id, amount)
+    override fun withdraw(id: BankAccountId, amount: Amount): Result<BankAccount, NotEnoughFunds> {
+        val response = changeBalanceAction(BANK_ACCOUNT_WITHDRAWAL_PATH, id, amount)
+        return when (response.status) {
+            Status.OK -> Success(bankAccountLens(response))
+            Status.BAD_REQUEST -> Failure(notEnoughFundsLens(response))
+            else -> throw Exception("Not expected: $response")
+        }
     }
 
-    private fun changeBalanceAction(actionPath: String, id: BankAccountId, amount: Amount): BankAccount {
-        val response = http(
+    private fun changeBalanceAction(actionPath: String, id: BankAccountId, amount: Amount): Response {
+        return http(
             Request(POST, BANK_ACCOUNTS_BASE_URL + actionPath)
                 .with(accountIdLens of id, amountLens of amount)
         )
-        return bankAccountLens(response)
     }
 }
diff --git a/web/src/main/kotlin/lmirabal/bank/web/BankWeb.kt b/web/src/main/kotlin/lmirabal/bank/web/BankWeb.kt
@@ -1,15 +1,17 @@
 package lmirabal.bank.web
 
+import dev.forkhandles.result4k.map
+import dev.forkhandles.result4k.recover
 import lmirabal.bank.Bank
 import lmirabal.bank.model.Amount
 import lmirabal.bank.model.BankAccount
 import lmirabal.bank.model.BankAccountId
+import lmirabal.bank.model.NotEnoughFunds
 import org.http4k.core.Body
 import org.http4k.core.ContentType.Companion.TEXT_HTML
 import org.http4k.core.HttpHandler
 import org.http4k.core.Method.GET
 import org.http4k.core.Method.POST
-import org.http4k.core.Request
 import org.http4k.core.Response
 import org.http4k.core.Status
 import org.http4k.core.Status.Companion.SEE_OTHER
@@ -33,8 +35,8 @@ fun bankWeb(bank: Bank): HttpHandler {
             POST to CreateAccount(bank),
             GET to ListAccounts(bank)
         ),
-        "/{id}/deposit" bind POST to ChangeBalance(bank) { id, amount -> deposit(id, amount) },
-        "/{id}/withdraw" bind POST to ChangeBalance(bank) { id, amount -> withdraw(id, amount) },
+        "/{id}/deposit" bind POST to DepositAmount(bank),
+        "/{id}/withdraw" bind POST to WithdrawAmount(bank),
     )
 }
 
@@ -58,29 +60,46 @@ object ListAccounts {
 
 }
 
-class ChangeBalance(private val bank: Bank, private val action: Bank.(BankAccountId, Amount) -> BankAccount) :
-    HttpHandler {
+object DepositAmount {
     private val amountField = FormField.long().map { majorUnits -> majorUnits.toAmount() }.required("amount")
     private val formBody = Body.webForm(Validator.Feedback, amountField).toLens()
 
-    override fun invoke(request: Request): Response {
+    operator fun invoke(bank: Bank): HttpHandler = { request ->
         val id = request.path("id") ?: throw Exception("Bank account id must be present")
         val form = formBody(request)
         val amount = amountField(form)
-        action(bank, BankAccountId(UUID.fromString(id)), amount)
-        return Response(SEE_OTHER).header("location", "/")
+        bank.deposit(BankAccountId(UUID.fromString(id)), amount)
+        Response(SEE_OTHER).header("location", "/")
     }
-
-    private fun Long.toAmount() = Amount(
-        BigDecimal(this)
-            .movePointRight(2)
-            .longValueExact()
-    )
 }
 
-fun Amount.format(): String {
-    return BigDecimal(minorUnits).movePointLeft(2).longValueExact().toString()
+object WithdrawAmount {
+    private val amountField = FormField.long().map { majorUnits -> majorUnits.toAmount() }.required("amount")
+    private val formBody = Body.webForm(Validator.Feedback, amountField).toLens()
+
+    private val templates = HandlebarsTemplates().CachingClasspath()
+    private val viewLens = Body.viewModel(templates, TEXT_HTML).toLens()
+
+    operator fun invoke(bank: Bank): HttpHandler = { request ->
+        val id = request.path("id") ?: throw Exception("Bank account id must be present")
+        val form = formBody(request)
+        val amount = amountField(form)
+
+        bank.withdraw(BankAccountId(UUID.fromString(id)), amount)
+            .map { Response(SEE_OTHER).header("location", "/") }
+            .recover { failure: NotEnoughFunds -> Response(Status.OK).with(viewLens of failure.toView()) }
+    }
 }
 
+fun Amount.format(): String = BigDecimal(minorUnits).movePointLeft(2).longValueExact().toString()
+
+private fun Long.toAmount() = Amount(
+    BigDecimal(this).movePointRight(2).longValueExact()
+)
+
 data class BankAccountView(val id: String, val balance: String)
 data class BankAccountListView(val accounts: List<BankAccountView>) : ViewModel
+data class NotEnoughFundsView(val id: String, val balance: String, val additionalFundsRequired: String) : ViewModel
+
+fun NotEnoughFunds.toView() =
+    NotEnoughFundsView(id.value.toString(), balance.format(), additionalFundsRequired.format())
diff --git a/web/src/main/resources/lmirabal/bank/web/NotEnoughFundsView.hbs b/web/src/main/resources/lmirabal/bank/web/NotEnoughFundsView.hbs
diff --git a/web/src/test/kotlin/lmirabal/bank/web/BankWebTest.kt b/web/src/test/kotlin/lmirabal/bank/web/BankWebTest.kt

Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,8 @@ plugins {`
`3`	`3`	`}`
`4`	`4`
`5`	`5`	`dependencies {`
	`6`	`+ api 'dev.forkhandles:result4k:1.6.0.0'`
	`7`	`+`
`6`	`8`	`testFixturesImplementation 'org.jetbrains.kotlin:kotlin-test-junit5'`
`7`	`9`	`testFixturesImplementation 'org.junit.jupiter:junit-jupiter-api:5.7.0'`
`8`	`10`	`testFixturesImplementation 'com.natpryce:hamkrest:1.8.0.1'`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`package lmirabal.bank.model`
`2`	`2`
`3`		`-data class Amount(val minorUnits: Long) {`
	`3`	`+data class Amount(val minorUnits: Long) : Comparable<Amount> {`
`4`	`4`	`init {`
`5`	`5`	`require(minorUnits >= 0)`
`6`	`6`	`}`
`@@ -13,6 +13,10 @@ data class Amount(val minorUnits: Long) {`
`13`	`13`	`return Amount(minorUnits - other.minorUnits)`
`14`	`14`	`}`
`15`	`15`
	`16`	`+ override fun compareTo(other: Amount): Int {`
	`17`	`+ return minorUnits.compareTo(other.minorUnits)`
	`18`	`+ }`
	`19`	`+`
`16`	`20`	`companion object {`
`17`	`21`	`val ZERO = Amount(0)`
`18`	`22`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,14 +1,18 @@`
`1`	`1`	`package lmirabal.bank.model`
`2`	`2`
	`3`	`+import dev.forkhandles.result4k.Failure`
	`4`	`+import dev.forkhandles.result4k.Result`
	`5`	`+import dev.forkhandles.result4k.Success`
`3`	`6`	`import java.util.UUID`
`4`	`7`
`5`	`8`	`data class BankAccount(val id: BankAccountId, val balance: Amount) {`
`6`	`9`	`fun deposit(amount: Amount): BankAccount {`
`7`	`10`	`return copy(balance = balance + amount)`
`8`	`11`	`}`
`9`	`12`
`10`		`- fun withdraw(amount: Amount): BankAccount {`
`11`		`- return copy(balance = balance - amount)`
	`13`	`+ fun withdraw(amount: Amount): Result<BankAccount, NotEnoughFunds> {`
	`14`	`+ return if (amount <= balance) Success(copy(balance = balance - amount))`
	`15`	`+ else Failure(NotEnoughFunds(id, balance, amount - balance))`
`12`	`16`	`}`
`13`	`17`	`}`
`14`	`18`
`@@ -18,3 +22,5 @@ data class BankAccountId(val value: UUID) {`
`18`	`22`	`fun random() = BankAccountId(UUID.randomUUID())`
`19`	`23`	`}`
`20`	`24`	`}`
	`25`	`+`
	`26`	`+data class NotEnoughFunds(val id: BankAccountId, val balance: Amount, val additionalFundsRequired: Amount)`