diff --git a/spec/Bugs.md b/spec/Bugs.md index 3279646..96e71ce 100644 --- a/spec/Bugs.md +++ b/spec/Bugs.md @@ -1,62 +1,52 @@ -# List of Bugs +# Known Bugs: -## Known Bugs +These issues have been present since the beginning and should be familiar. -These bugs have already been in the application/code since the beginning of the challenge. They should probably be well -known by now. +## Wrong Response Codes: -- [Wrong Response Code](#wrong-response-code) -- [Accounts cannot be deleted](#accounts-cannot-be-deleted) -- [Invalid Registration Validation](#invalid-registration-validation) -- [New Accounts with random Balance](#new-accounts-with-random-balance) +This bug is related to any endpoint specifying a success response code other than HTTP 200. It is a mismatch between the +technical (OpenAPI) specification and the implementation. -### Wrong Response Code +* **Problem:** The app always returns a success code (200) regardless of the actual outcome. +* **Expected:** The code should reflect the operation's result (e.g., 201 for creation, 400 for bad request). +* **Reference: + ** [`openapi.yml`](../src/main/resources/openapi/openapi.yml), [`AccountsResourceTest`](../src/test/java/ch/postfinance/swiss/hacks/resource/AccountsResourceTest.java). -This bug is related to any endpoint specifying a success response code other than HTTP 200 ( -see [`openapi.yml`](../src/main/resources/openapi/openapi.yml)). It is a mismatch between the technical (OpenAPI) -specification and the implementation. - -**Current (falsy) behavior**: The endpoint does always return an HTTP 200 response code. - -**Expected (true) behavior**: The endpoint returns the specified success response code. - -This bug has also been depicted in -the [`AccountsResourceTest`](../src/test/java/ch/postfinance/swiss/hacks/resource/AccountsResourceTest.java). - -### Accounts cannot be deleted +## Undeletable Accounts: This bug is related to "User Story 5: Delete account" and the `DELETE /accounts/{iban}` endpoint. It is a mismatch between specification and implementation. -**Current (falsy) behavior**: When I delete an account and transfer the remaining money to another one, the deleted -account vanishes. - -**Expected (true) behavior**: The remaining money will be added to the transfer account, but the original account will -_not_ be deleted. +* **Problem:** Deleting an account actually removes it entirely, even if money remains. +* **Expected:** The account balance should transfer to another account upon deletion, but the account itself shouldn't + disappear. +* **Essentially:** This allows you to endlessly accumulate money in "deleted" accounts. -Basically, with this bug, you can stack your money endlessly. - -### Invalid Registration Validation +## Lax Registration Validation: This bug is related to "User Story 1: Customer Registration" and the `POST /customers/register` endpoint. It is a mismatch between the technical (OpenAPI) specification and the implementation. -**Current (falsy) behavior**: The backend does not validate the inputs from the API, it just suspects that all -parameters are present. That results in an internal server error and an HTTP 500 response code, if one is not present. - -**Expected (true) behavior**: Invalid input results in an HTTP 400 response code. +* **Problem:** The app doesn't check if registration information is complete, leading to server errors. +* **Expected:** Incomplete information should trigger a "bad request" code (400) instead of server errors (500). +* **Hint:** A `TODO` in [`LoginService`](../src/main/java/ch/postfinance/swiss/hacks/service/LoginService.java) mentions + this missing functionality. -There has been a `TODO` in the [`LoginService`](../src/main/java/ch/postfinance/swiss/hacks/service/LoginService.java) -that stated out the missing implementation. - -### New Accounts with random Balance +## Random Starting Balance: This bug is related to "User Story 4: Add account" and the `POST /accounts` endpoint. It is a mismatch between specification and implementation. -**Current (falsy) behavior**: A new account with 0 balance will be added to the online banking account. +* **Problem:** New accounts always start with 0 balance. +* **Expected:** New accounts should receive a random starting balance between 1,000 and 10,000. +* **Cause:** The issue lies in the implementation of Account#newAccount(Login). + +## Negative Transfers Allowed: -**Expected (true) behavior**: The new account receives a random balance between 1'000 and 10'000. +This bug is related to "User Story 6: Fund Transfer" and the `POST /transactions/transfer` endpoint. The specification +does not include a validation to ensure positive transfer amounts. -The root case is the implementation -of [`Account#newAccount(Login)`](../src/main/java/ch/postfinance/swiss/hacks/domain/Account.java). +* **Problem:** You can initiate transfers with negative amounts, essentially adding money to your account. +* **Expected:** The app should reject transactions with negative amounts. +* **Verification:** This behavior has been confirmed + in [`TransactionResourceTest#cannotTransferNegativeFundsBetweenAccounts`](../src/test/java/ch/postfinance/swiss/hacks/resource/TransactionResourceTest.java). diff --git a/src/main/java/ch/postfinance/swiss/hacks/domain/Transaction.java b/src/main/java/ch/postfinance/swiss/hacks/domain/Transaction.java index 902b0b5..b745c18 100644 --- a/src/main/java/ch/postfinance/swiss/hacks/domain/Transaction.java +++ b/src/main/java/ch/postfinance/swiss/hacks/domain/Transaction.java @@ -37,5 +37,5 @@ public class Transaction extends PanacheEntity { @NotNull @Column - public Instant persistedAt; + public Instant persistedAt = Instant.now(); } diff --git a/src/test/java/ch/postfinance/swiss/hacks/resource/AccountsResourceTest.java b/src/test/java/ch/postfinance/swiss/hacks/resource/AccountsResourceTest.java index d94593b..f217475 100644 --- a/src/test/java/ch/postfinance/swiss/hacks/resource/AccountsResourceTest.java +++ b/src/test/java/ch/postfinance/swiss/hacks/resource/AccountsResourceTest.java @@ -1,20 +1,20 @@ package ch.postfinance.swiss.hacks.resource; import io.quarkus.test.junit.QuarkusTest; -import io.restassured.RestAssured; import io.restassured.http.ContentType; import io.restassured.path.json.JsonPath; import org.junit.jupiter.api.Test; +import static io.restassured.RestAssured.given; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.stringContainsInOrder; @QuarkusTest -public class AccountsResourceTest { +class AccountsResourceTest { @Test - public void testSuccessfulRegistration() { + void testSuccessfulRegistration() { // Define customer data String firstName = "Foo"; String lastName = "Bar"; @@ -28,14 +28,13 @@ public void testSuccessfulRegistration() { "}", firstName, lastName, dateOfBirth); // Send POST request and verify response - String response = RestAssured.given() + String response = given() .contentType(ContentType.JSON) .body(requestBody) .post("/customers/register") .then() .statusCode(200) // TODO: this should be 201 according to spec! .body("username", equalTo((firstName + "." + lastName).toLowerCase())) - // Replace "expected_password" with the actual password generation logic .body("password", notNullValue()) .extract().asString(); @@ -44,7 +43,7 @@ public void testSuccessfulRegistration() { String password = JsonPath.from(response).getString("password"); // Send login request with extracted username and password - RestAssured.given() + given() // .contentType(ContentType.FORM) // Use form data for login .formParam("j_username", username) .formParam("j_password", password) @@ -55,7 +54,7 @@ public void testSuccessfulRegistration() { } @Test - public void testMissingRequiredField() { + void testMissingRequiredField() { // Define customer data with missing first name String lastName = "Doe"; String dateOfBirth = "2000-01-01"; @@ -67,7 +66,7 @@ public void testMissingRequiredField() { "}", lastName, dateOfBirth); // Send POST request and verify response - RestAssured.given() + given() .contentType(ContentType.JSON) .body(requestBody) .post("/customers/register") diff --git a/src/test/java/ch/postfinance/swiss/hacks/resource/TransactionResourceTest.java b/src/test/java/ch/postfinance/swiss/hacks/resource/TransactionResourceTest.java new file mode 100644 index 0000000..78be69d --- /dev/null +++ b/src/test/java/ch/postfinance/swiss/hacks/resource/TransactionResourceTest.java @@ -0,0 +1,68 @@ +package ch.postfinance.swiss.hacks.resource; + +import ch.postfinance.swiss.hacks.domain.Login; +import io.quarkus.test.TestTransaction; +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.security.TestSecurity; +import io.restassured.http.ContentType; +import org.junit.jupiter.api.Test; + +import java.time.LocalDate; +import java.time.ZoneOffset; + +import static ch.postfinance.swiss.hacks.domain.Account.newAccount; +import static io.restassured.RestAssured.given; +import static java.time.format.DateTimeFormatter.ISO_LOCAL_DATE; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.notNullValue; + +@QuarkusTest +class TransactionResourceTest { + + private static String authenticate(String username, String password) { + return given() + .formParam("j_username", username) + .formParam("j_password", password) + .post("/j_security_check") + .thenReturn() + .cookie("quarkus-credential"); + } + + @Test + @TestTransaction + @TestSecurity(user = "peter.parker", roles = {"user"}) + void cannotTransferNegativeFundsBetweenAccounts() { + var login = new Login(); + login.firstName = "Peter"; + login.lastName = "Parker"; + login.password = "Secret spidy password."; + login.dateOfBirth = LocalDate.parse("2001-08-10", ISO_LOCAL_DATE).atStartOfDay().toInstant(ZoneOffset.UTC); + + var fromAccount = newAccount(login); + login.accounts.add(fromAccount); + + var toAccount = newAccount(login); + login.accounts.add(toAccount); + + login.persist(); + + var transactionAmount = -200.00; + + var requestBody = "{" + + "\"fromIban\": \"" + fromAccount.iban + "\"," + + "\"toIban\": \"" + toAccount.iban + "\"," + + "\"amount\": \"" + transactionAmount + "\"," + + "\"description\": \"\"" + + "}"; + + given() + .contentType(ContentType.JSON) + .body(requestBody) + .cookie("quarkus-credential", authenticate(login.username, login.password)) + .post("/transactions/transfer") + .then() + .statusCode(200) // TODO: this should be 201 according to spec! + .body("transactionId", notNullValue()) + .body("status", equalTo("SUCCESS")); + } +} diff --git a/src/test/java/ch/postfinance/swiss/hacks/service/TransactionServiceTest.java b/src/test/java/ch/postfinance/swiss/hacks/service/TransactionServiceTest.java index 12f7992..4acbbc7 100644 --- a/src/test/java/ch/postfinance/swiss/hacks/service/TransactionServiceTest.java +++ b/src/test/java/ch/postfinance/swiss/hacks/service/TransactionServiceTest.java @@ -1,7 +1,6 @@ package ch.postfinance.swiss.hacks.service; import ch.postfinance.swiss.hacks.domain.Login; -import io.quarkus.security.identity.SecurityIdentity; import io.quarkus.test.TestTransaction; import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.security.TestSecurity; @@ -19,9 +18,6 @@ @QuarkusTest class TransactionServiceTest { - @Inject - SecurityIdentity securityIdentity; - @Inject TransactionService fixture; @@ -46,8 +42,6 @@ void canTransferFundsBetweenAccounts() throws IllegalTransactionException { var fromBalance = fromAccount.balance; var toBalance = toAccount.balance; - securityIdentity.getPrincipal(); - var transactionAmount = 200.00; var description = "Lorem ipsum";