From c96a36f3253234aa00e4063a724e358faa017d48 Mon Sep 17 00:00:00 2001 From: MK13 Date: Sun, 12 May 2019 00:43:28 +0200 Subject: [PATCH] Remove artificial restriction that account keys need to start with 'accounts.' Rework URL decoding into own util method because the previously used method is only available since Java 10 (current runtime is 1.9 however). Further adjust unit tests and test data to reflect the change. Also add a migration script for already installed instances. --- src/main/java/de/financer/ResponseReason.java | 1 - .../controller/AccountController.java | 11 +-- .../financer/controller/ControllerUtil.java | 22 +++++ .../RecurringTransactionController.java | 6 +- .../controller/TransactionController.java | 6 +- .../de/financer/service/AccountService.java | 11 +-- .../database/common/V3_0_0__accountRename.sql | 88 +++++++++++++++++++ ...teRecurringTransactionIntegrationTest.java | 4 +- .../AccountService_createAccountTest.java | 18 +--- .../AccountService_setAccountStatusTest.java | 18 +--- ...dRecurringTransactionReminderTaskTest.java | 6 +- .../integration/V999_99_00__testdata.sql | 8 +- 12 files changed, 136 insertions(+), 63 deletions(-) create mode 100644 src/main/java/de/financer/controller/ControllerUtil.java create mode 100644 src/main/resources/database/common/V3_0_0__accountRename.sql diff --git a/src/main/java/de/financer/ResponseReason.java b/src/main/java/de/financer/ResponseReason.java index d531686..8e87790 100644 --- a/src/main/java/de/financer/ResponseReason.java +++ b/src/main/java/de/financer/ResponseReason.java @@ -7,7 +7,6 @@ public enum ResponseReason { OK(HttpStatus.OK), UNKNOWN_ERROR(HttpStatus.INTERNAL_SERVER_ERROR), INVALID_ACCOUNT_TYPE(HttpStatus.INTERNAL_SERVER_ERROR), - INVALID_ACCOUNT_KEY(HttpStatus.INTERNAL_SERVER_ERROR), FROM_ACCOUNT_NOT_FOUND(HttpStatus.INTERNAL_SERVER_ERROR), TO_ACCOUNT_NOT_FOUND(HttpStatus.INTERNAL_SERVER_ERROR), FROM_AND_TO_ACCOUNT_NOT_FOUND(HttpStatus.INTERNAL_SERVER_ERROR), diff --git a/src/main/java/de/financer/controller/AccountController.java b/src/main/java/de/financer/controller/AccountController.java index 12e318f..9c37805 100644 --- a/src/main/java/de/financer/controller/AccountController.java +++ b/src/main/java/de/financer/controller/AccountController.java @@ -10,9 +10,6 @@ import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; -import java.net.URLDecoder; -import java.nio.charset.StandardCharsets; - @RestController @RequestMapping("accounts") public class AccountController { @@ -24,7 +21,7 @@ public class AccountController { @RequestMapping("getByKey") public Account getAccountByKey(String key) { - final String decoded = URLDecoder.decode(key, StandardCharsets.UTF_8); + final String decoded = ControllerUtil.urlDecode(key); if (LOGGER.isDebugEnabled()) { LOGGER.debug(String.format("/accounts/getAccountByKey got parameter: %s", decoded)); @@ -40,7 +37,7 @@ public class AccountController { @RequestMapping("createAccount") public ResponseEntity createAccount(String key, String type) { - final String decoded = URLDecoder.decode(key, StandardCharsets.UTF_8); + final String decoded = ControllerUtil.urlDecode(key); if (LOGGER.isDebugEnabled()) { LOGGER.debug(String.format("/accounts/createAccount got parameters: %s, %s", decoded, type)); @@ -57,7 +54,7 @@ public class AccountController { @RequestMapping("closeAccount") public ResponseEntity closeAccount(String key) { - final String decoded = URLDecoder.decode(key, StandardCharsets.UTF_8); + final String decoded = ControllerUtil.urlDecode(key); if (LOGGER.isDebugEnabled()) { LOGGER.debug(String.format("/accounts/closeAccount got parameters: %s", decoded)); @@ -74,7 +71,7 @@ public class AccountController { @RequestMapping("openAccount") public ResponseEntity openAccount(String key) { - final String decoded = URLDecoder.decode(key, StandardCharsets.UTF_8); + final String decoded = ControllerUtil.urlDecode(key); if (LOGGER.isDebugEnabled()) { LOGGER.debug(String.format("/accounts/openAccount got parameters: %s", decoded)); diff --git a/src/main/java/de/financer/controller/ControllerUtil.java b/src/main/java/de/financer/controller/ControllerUtil.java new file mode 100644 index 0000000..bf5711e --- /dev/null +++ b/src/main/java/de/financer/controller/ControllerUtil.java @@ -0,0 +1,22 @@ +package de.financer.controller; + +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; + +public class ControllerUtil { + /** + * This method decodes the given URL encoded string, e.g. replaces %20 with a space. + * + * @param toDecode the string to decode + * @return the decoded string in UTF-8 or, if UTF-8 is not available for whatever reason, the encoded string + */ + public static final String urlDecode(String toDecode) { + try { + return URLDecoder.decode(toDecode, StandardCharsets.UTF_8.name()); + } + catch (UnsupportedEncodingException e) { + return toDecode; + } + } +} diff --git a/src/main/java/de/financer/controller/RecurringTransactionController.java b/src/main/java/de/financer/controller/RecurringTransactionController.java index dc8ba26..623d6e1 100644 --- a/src/main/java/de/financer/controller/RecurringTransactionController.java +++ b/src/main/java/de/financer/controller/RecurringTransactionController.java @@ -35,7 +35,7 @@ public class RecurringTransactionController { @RequestMapping("getAllForAccount") public Iterable getAllForAccount(String accountKey) { - final String decoded = URLDecoder.decode(accountKey, StandardCharsets.UTF_8); + final String decoded = ControllerUtil.urlDecode(accountKey); if (LOGGER.isDebugEnabled()) { LOGGER.debug(String.format("/recurringTransactions/getAllForAccount got parameter: %s", decoded)); @@ -55,8 +55,8 @@ public class RecurringTransactionController { String intervalType, String firstOccurrence, String lastOccurrence ) { - final String decodedFrom = URLDecoder.decode(fromAccountKey, StandardCharsets.UTF_8); - final String decodedTo = URLDecoder.decode(toAccountKey, StandardCharsets.UTF_8); + final String decodedFrom = ControllerUtil.urlDecode(fromAccountKey); + final String decodedTo = ControllerUtil.urlDecode(toAccountKey); if (LOGGER.isDebugEnabled()) { LOGGER.debug(String diff --git a/src/main/java/de/financer/controller/TransactionController.java b/src/main/java/de/financer/controller/TransactionController.java index bce51d3..61ab109 100644 --- a/src/main/java/de/financer/controller/TransactionController.java +++ b/src/main/java/de/financer/controller/TransactionController.java @@ -28,7 +28,7 @@ public class TransactionController { @RequestMapping("getAllForAccount") public Iterable getAllForAccount(String accountKey) { - final String decoded = URLDecoder.decode(accountKey, StandardCharsets.UTF_8); + final String decoded = ControllerUtil.urlDecode(accountKey); if (LOGGER.isDebugEnabled()) { LOGGER.debug(String.format("/transactions/getAllForAccount got parameter: %s", decoded)); @@ -41,8 +41,8 @@ public class TransactionController { public ResponseEntity createTransaction(String fromAccountKey, String toAccountKey, Long amount, String date, String description ) { - final String decodedFrom = URLDecoder.decode(fromAccountKey, StandardCharsets.UTF_8); - final String decodedTo = URLDecoder.decode(toAccountKey, StandardCharsets.UTF_8); + final String decodedFrom = ControllerUtil.urlDecode(fromAccountKey); + final String decodedTo = ControllerUtil.urlDecode(toAccountKey); if (LOGGER.isDebugEnabled()) { LOGGER.debug(String diff --git a/src/main/java/de/financer/service/AccountService.java b/src/main/java/de/financer/service/AccountService.java index fba36b4..bcea422 100644 --- a/src/main/java/de/financer/service/AccountService.java +++ b/src/main/java/de/financer/service/AccountService.java @@ -55,10 +55,9 @@ public class AccountService { * This method creates new account with the given key and type. The account has status {@link AccountStatus#OPEN OPEN} * and a current balance of 0. * - * @param key the key of the new account. Must begin with account. + * @param key the key of the new account * @param type the type of the new account. Must be one of {@link AccountType}. * @return {@link ResponseReason#INVALID_ACCOUNT_TYPE} if the given type is not a valid {@link AccountType}, - * {@link ResponseReason#INVALID_ACCOUNT_KEY} if the given key does not conform to the format specification, * {@link ResponseReason#UNKNOWN_ERROR} if an unexpected error occurs and * {@link ResponseReason#OK} if the operation completed successfully. Never returns null. */ @@ -68,10 +67,6 @@ public class AccountService { return ResponseReason.INVALID_ACCOUNT_TYPE; } - if (!StringUtils.startsWith(key, "accounts.")) { - return ResponseReason.INVALID_ACCOUNT_KEY; - } - final Account account = new Account(); account.setKey(key); @@ -105,10 +100,6 @@ public class AccountService { // Visible for unit tests /* package */ ResponseReason setAccountStatus(String key, AccountStatus accountStatus) { - if (!StringUtils.startsWith(key, "accounts.")) { - return ResponseReason.INVALID_ACCOUNT_KEY; - } - final Account account = this.accountRepository.findByKey(key); if (account == null) { diff --git a/src/main/resources/database/common/V3_0_0__accountRename.sql b/src/main/resources/database/common/V3_0_0__accountRename.sql new file mode 100644 index 0000000..12652c7 --- /dev/null +++ b/src/main/resources/database/common/V3_0_0__accountRename.sql @@ -0,0 +1,88 @@ +-- Rename all accounts to proper names instead of the artificial 'accounts.' names +UPDATE account +SET "key" = 'Check account' +WHERE "key" = 'accounts.checkaccount'; + +UPDATE account +SET "key" = 'Income' +WHERE "key" = 'accounts.income' + +UPDATE account +SET "key" = 'Cash' +WHERE "key" = 'accounts.cash'; + +UPDATE account +SET "key" = 'Start' +WHERE "key" = 'accounts.start'; + +UPDATE account +SET "key" = 'Rent' +WHERE "key" = 'accounts.rent'; + +UPDATE account +SET "key" = 'FVS' +WHERE "key" = 'accounts.fvs'; + +UPDATE account +SET "key" = 'Car' +WHERE "key" = 'accounts.car'; + +UPDATE account +SET "key" = 'Gas' +WHERE "key" = 'accounts.gas'; + +UPDATE account +SET "key" = 'Alimony' +WHERE "key" = 'accounts.alimony'; + +UPDATE account +SET "key" = 'Electricity/Water' +WHERE "key" = 'accounts.electricitywater'; + +UPDATE account +SET "key" = 'Mobile' +WHERE "key" = 'accounts.mobile'; + +UPDATE account +SET "key" = 'Internet' +WHERE "key" = 'accounts.internet'; + +UPDATE account +SET "key" = 'Legal insurance' +WHERE "key" = 'accounts.legalinsurance'; + +UPDATE account +SET "key" = 'Netflix' +WHERE "key" = 'accounts.netflix'; + +UPDATE account +SET "key" = 'Hetzner' +WHERE "key" = 'accounts.hetzner'; + +UPDATE account +SET "key" = 'Fees' +WHERE "key" = 'accounts.fees'; + +UPDATE account +SET "key" = 'Food' +WHERE "key" = 'accounts.food'; + +UPDATE account +SET "key" = 'Food (external)' +WHERE "key" = 'accounts.foodexternal'; + +UPDATE account +SET "key" = 'Child' +WHERE "key" = 'accounts.child'; + +UPDATE account +SET "key" = 'Credit card' +WHERE "key" = 'accounts.creditcard'; + +UPDATE account +SET "key" = 'Student loan' +WHERE "key" = 'accounts.studentloan'; + +UPDATE account +SET "key" = 'Bed' +WHERE "key" = 'accounts.bed'; \ No newline at end of file diff --git a/src/test/java/de/financer/controller/integration/RecurringTransactionService_createRecurringTransactionIntegrationTest.java b/src/test/java/de/financer/controller/integration/RecurringTransactionService_createRecurringTransactionIntegrationTest.java index 6f8fc25..c64e6ae 100644 --- a/src/test/java/de/financer/controller/integration/RecurringTransactionService_createRecurringTransactionIntegrationTest.java +++ b/src/test/java/de/financer/controller/integration/RecurringTransactionService_createRecurringTransactionIntegrationTest.java @@ -36,8 +36,8 @@ public class RecurringTransactionService_createRecurringTransactionIntegrationTe @Test public void test_createRecurringTransaction() throws Exception { final MvcResult mvcRequest = this.mockMvc.perform(get("/recurringTransactions/createRecurringTransaction") - .param("fromAccountKey", "accounts.income") - .param("toAccountKey", "accounts.checkaccount") + .param("fromAccountKey", "Income") + .param("toAccountKey", "Check account") .param("amount", "250000") .param("description", "Monthly rent") .param("holidayWeekendType", "SAME_DAY") diff --git a/src/test/java/de/financer/service/AccountService_createAccountTest.java b/src/test/java/de/financer/service/AccountService_createAccountTest.java index 38daec8..ed4e007 100644 --- a/src/test/java/de/financer/service/AccountService_createAccountTest.java +++ b/src/test/java/de/financer/service/AccountService_createAccountTest.java @@ -32,25 +32,13 @@ public class AccountService_createAccountTest { Assert.assertEquals(ResponseReason.INVALID_ACCOUNT_TYPE, response); } - @Test - public void test_createAccount_INVALID_ACCOUNT_KEY() { - // Arrange - // Nothing to do - - // Act - ResponseReason response = this.classUnderTest.createAccount(null, "BANK"); - - // Assert - Assert.assertEquals(ResponseReason.INVALID_ACCOUNT_KEY, response); - } - @Test public void test_createAccount_UNKNOWN_ERROR() { // Arrange Mockito.doThrow(new NullPointerException()).when(this.accountRepository).save(Mockito.any(Account.class)); // Act - ResponseReason response = this.classUnderTest.createAccount("accounts.test", "BANK"); + ResponseReason response = this.classUnderTest.createAccount("Test", "BANK"); // Assert Assert.assertEquals(ResponseReason.UNKNOWN_ERROR, response); @@ -62,11 +50,11 @@ public class AccountService_createAccountTest { // Nothing to do // Act - ResponseReason response = this.classUnderTest.createAccount("accounts.test", "BANK"); + ResponseReason response = this.classUnderTest.createAccount("Test", "BANK"); // Assert Assert.assertEquals(ResponseReason.OK, response); Mockito.verify(this.accountRepository, Mockito.times(1)) - .save(ArgumentMatchers.argThat((acc) -> "accounts.test".equals(acc.getKey()))); + .save(ArgumentMatchers.argThat((acc) -> "Test".equals(acc.getKey()))); } } diff --git a/src/test/java/de/financer/service/AccountService_setAccountStatusTest.java b/src/test/java/de/financer/service/AccountService_setAccountStatusTest.java index 880fff8..6c0ce2c 100644 --- a/src/test/java/de/financer/service/AccountService_setAccountStatusTest.java +++ b/src/test/java/de/financer/service/AccountService_setAccountStatusTest.java @@ -21,25 +21,13 @@ public class AccountService_setAccountStatusTest { @Mock private AccountRepository accountRepository; - @Test - public void test_setAccountStatus_INVALID_ACCOUNT_KEY() { - // Arrange - // Nothing to do - - // Act - ResponseReason response = this.classUnderTest.setAccountStatus(null, AccountStatus.CLOSED); - - // Assert - Assert.assertEquals(ResponseReason.INVALID_ACCOUNT_KEY, response); - } - @Test public void test_setAccountStatus_ACCOUNT_NOT_FOUND() { // Arrange // Nothing to do // Act - ResponseReason response = this.classUnderTest.setAccountStatus("accounts.test", AccountStatus.CLOSED); + ResponseReason response = this.classUnderTest.setAccountStatus("Test", AccountStatus.CLOSED); // Assert Assert.assertEquals(ResponseReason.ACCOUNT_NOT_FOUND, response); @@ -52,7 +40,7 @@ public class AccountService_setAccountStatusTest { Mockito.doThrow(new NullPointerException()).when(this.accountRepository).save(Mockito.any(Account.class)); // Act - ResponseReason response = this.classUnderTest.setAccountStatus("accounts.test", AccountStatus.CLOSED); + ResponseReason response = this.classUnderTest.setAccountStatus("Test", AccountStatus.CLOSED); // Assert Assert.assertEquals(ResponseReason.UNKNOWN_ERROR, response); @@ -64,7 +52,7 @@ public class AccountService_setAccountStatusTest { Mockito.when(this.accountRepository.findByKey(Mockito.anyString())).thenReturn(new Account()); // Act - ResponseReason response = this.classUnderTest.setAccountStatus("accounts.test", AccountStatus.CLOSED); + ResponseReason response = this.classUnderTest.setAccountStatus("Test", AccountStatus.CLOSED); // Assert Assert.assertEquals(ResponseReason.OK, response); diff --git a/src/test/java/de/financer/task/SendRecurringTransactionReminderTaskTest.java b/src/test/java/de/financer/task/SendRecurringTransactionReminderTaskTest.java index 7af9de9..844178d 100644 --- a/src/test/java/de/financer/task/SendRecurringTransactionReminderTaskTest.java +++ b/src/test/java/de/financer/task/SendRecurringTransactionReminderTaskTest.java @@ -36,9 +36,9 @@ public class SendRecurringTransactionReminderTaskTest { public void test_sendReminder() { // Arrange final Collection recurringTransactions = Arrays.asList( - createRecurringTransaction("Test booking 1", "accounts.income", "accounts.bank", Long.valueOf(250000)), - createRecurringTransaction("Test booking 2", "accounts.bank", "accounts.rent", Long.valueOf(41500)), - createRecurringTransaction("Test booking 3", "accounts.bank", "accounts.cash", Long.valueOf(5000)) + createRecurringTransaction("Test booking 1", "Income", "accounts.bank", Long.valueOf(250000)), + createRecurringTransaction("Test booking 2", "Bank", "accounts.rent", Long.valueOf(41500)), + createRecurringTransaction("Test booking 3", "Bank", "accounts.cash", Long.valueOf(5000)) ); Mockito.when(this.recurringTransactionService.getAllDueToday()).thenReturn(recurringTransactions); diff --git a/src/test/resources/database/hsqldb/integration/V999_99_00__testdata.sql b/src/test/resources/database/hsqldb/integration/V999_99_00__testdata.sql index c55b95e..ef84e78 100644 --- a/src/test/resources/database/hsqldb/integration/V999_99_00__testdata.sql +++ b/src/test/resources/database/hsqldb/integration/V999_99_00__testdata.sql @@ -1,13 +1,13 @@ -- Accounts INSERT INTO account ("key", type, status, current_balance) -VALUES ('accounts.convenience', 'EXPENSE', 'OPEN', 0); +VALUES ('Convenience', 'EXPENSE', 'OPEN', 0); --Recurring transactions INSERT INTO recurring_transaction (from_account_id, to_account_id, description, amount, interval_type, first_occurrence, holiday_weekend_type) -VALUES ((SELECT ID FROM account WHERE "key" = 'accounts.income'), (SELECT ID FROM account WHERE "key" = 'accounts.checkaccount'), 'Pay', 250000, 'MONTHLY', '2019-01-15', 'NEXT_WORKDAY'); +VALUES ((SELECT ID FROM account WHERE "key" = 'Income'), (SELECT ID FROM account WHERE "key" = 'Check account'), 'Pay', 250000, 'MONTHLY', '2019-01-15', 'NEXT_WORKDAY'); INSERT INTO recurring_transaction (from_account_id, to_account_id, description, amount, interval_type, first_occurrence, holiday_weekend_type) -VALUES ((SELECT ID FROM account WHERE "key" = 'accounts.cash'), (SELECT ID FROM account WHERE "key" = 'accounts.convenience'), 'Pretzel', 170, 'DAILY', '2019-02-20', 'SAME_DAY'); +VALUES ((SELECT ID FROM account WHERE "key" = 'Cash'), (SELECT ID FROM account WHERE "key" = 'Convenience'), 'Pretzel', 170, 'DAILY', '2019-02-20', 'SAME_DAY'); INSERT INTO recurring_transaction (from_account_id, to_account_id, description, amount, interval_type, first_occurrence, last_occurrence, holiday_weekend_type) -VALUES ((SELECT ID FROM account WHERE "key" = 'accounts.cash'), (SELECT ID FROM account WHERE "key" = 'accounts.foodexternal'), 'McDonalds Happy Meal', 399, 'WEEKLY', '2019-02-20', '2019-03-20', 'SAME_DAY'); \ No newline at end of file +VALUES ((SELECT ID FROM account WHERE "key" = 'Cash'), (SELECT ID FROM account WHERE "key" = 'Food (external)'), 'McDonalds Happy Meal', 399, 'WEEKLY', '2019-02-20', '2019-03-20', 'SAME_DAY'); \ No newline at end of file