From 449e8fb216ce6b19f809fb70848c32747a7e85ae Mon Sep 17 00:00:00 2001 From: MK13 Date: Tue, 5 Mar 2019 23:13:26 +0100 Subject: [PATCH] Add implementation to create a recurring transaction - Therefore extracted the date format into the financer config - Simplify date testing, also for plain transaction service - Add test for duePast MONTHLY SAME_DAY --- src/main/java/de/financer/ResponseReason.java | 9 +- .../de/financer/config/FinancerConfig.java | 27 +++ .../service/RecurringTransactionService.java | 178 +++++++++++++++--- .../financer/service/TransactionService.java | 35 ++-- .../resources/config/application.properties | 5 +- ...e_getAllDueToday_MONTHLY_SAME_DAYTest.java | 68 +++++++ 6 files changed, 282 insertions(+), 40 deletions(-) create mode 100644 src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_MONTHLY_SAME_DAYTest.java diff --git a/src/main/java/de/financer/ResponseReason.java b/src/main/java/de/financer/ResponseReason.java index fd57146..e631e2b 100644 --- a/src/main/java/de/financer/ResponseReason.java +++ b/src/main/java/de/financer/ResponseReason.java @@ -15,7 +15,14 @@ public enum ResponseReason { MISSING_DATE(HttpStatus.INTERNAL_SERVER_ERROR), AMOUNT_ZERO(HttpStatus.INTERNAL_SERVER_ERROR), MISSING_AMOUNT(HttpStatus.INTERNAL_SERVER_ERROR), - INVALID_BOOKING_ACCOUNTS(HttpStatus.INTERNAL_SERVER_ERROR); + INVALID_BOOKING_ACCOUNTS(HttpStatus.INTERNAL_SERVER_ERROR), + MISSING_HOLIDAY_WEEKEND_TYPE(HttpStatus.INTERNAL_SERVER_ERROR), + INVALID_HOLIDAY_WEEKEND_TYPE(HttpStatus.INTERNAL_SERVER_ERROR), + MISSING_INTERVAL_TYPE(HttpStatus.INTERNAL_SERVER_ERROR), + INVALID_INTERVAL_TYPE(HttpStatus.INTERNAL_SERVER_ERROR), + MISSING_FIRST_OCCURRENCE(HttpStatus.INTERNAL_SERVER_ERROR), + INVALID_FIRST_OCCURRENCE_FORMAT(HttpStatus.INTERNAL_SERVER_ERROR), + INVALID_LAST_OCCURRENCE_FORMAT(HttpStatus.INTERNAL_SERVER_ERROR); private HttpStatus httpStatus; diff --git a/src/main/java/de/financer/config/FinancerConfig.java b/src/main/java/de/financer/config/FinancerConfig.java index a3a549a..3041b2e 100644 --- a/src/main/java/de/financer/config/FinancerConfig.java +++ b/src/main/java/de/financer/config/FinancerConfig.java @@ -4,6 +4,7 @@ import de.jollyday.HolidayCalendar; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.context.annotation.Configuration; +import java.time.format.DateTimeFormatter; import java.util.Arrays; import java.util.Optional; @@ -12,6 +13,7 @@ import java.util.Optional; public class FinancerConfig { private String countryCode; private String state; + private String dateFormat; /** * @return the raw country code, mostly an uppercase ISO 3166 2-letter code @@ -50,4 +52,29 @@ public class FinancerConfig { public void setCountryCode(String countryCode) { this.countryCode = countryCode; } + + /** + * @return the date format used in e.g. the + * {@link de.financer.service.TransactionService#createTransaction(String, String, Long, String, String) + * TransactionService#createTransaction} or + * {@link de.financer.service.RecurringTransactionService#createRecurringTransaction(String, String, Long, String, + * String, String, String, String) RecurringTransactionService#createRecurringTransaction} methods. Used to parse + * the client-supplied date string to proper {@link java.time.LocalDate LocalDate} objects + */ + public String getDateFormat() { + return dateFormat; + } + + public void setDateFormat(String dateFormat) { + try { + DateTimeFormatter.ofPattern(dateFormat); + } + catch (IllegalArgumentException e) { + // TODO log info about default dd.MM.yyyy + + dateFormat = "dd.MM.yyyy"; + } + + this.dateFormat = dateFormat; + } } diff --git a/src/main/java/de/financer/service/RecurringTransactionService.java b/src/main/java/de/financer/service/RecurringTransactionService.java index f03cf76..6bd4ce8 100644 --- a/src/main/java/de/financer/service/RecurringTransactionService.java +++ b/src/main/java/de/financer/service/RecurringTransactionService.java @@ -1,15 +1,21 @@ package de.financer.service; import de.financer.ResponseReason; +import de.financer.config.FinancerConfig; import de.financer.dba.RecurringTransactionRepository; import de.financer.model.Account; import de.financer.model.HolidayWeekendType; +import de.financer.model.IntervalType; import de.financer.model.RecurringTransaction; import org.apache.commons.collections4.IterableUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; import java.time.LocalDate; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeParseException; import java.util.Collections; import java.util.stream.Collectors; @@ -25,6 +31,9 @@ public class RecurringTransactionService { @Autowired private RuleService ruleService; + @Autowired + private FinancerConfig financerConfig; + public Iterable getAll() { return this.recurringTransactionRepository.findAll(); } @@ -42,8 +51,8 @@ public class RecurringTransactionService { /** * This method gets all recurring transactions that are due today. Whether a recurring transaction is due today - * depends on today's date and the configured {@link RecurringTransaction#getIntervalType() interval type} - * and {@link RecurringTransaction#getHolidayWeekendType() holiday weekend type}. + * depends on today's date and the configured {@link RecurringTransaction#getIntervalType() interval type} and + * {@link RecurringTransaction#getHolidayWeekendType() holiday weekend type}. * * @return all recurring transactions that are due today */ @@ -66,17 +75,16 @@ public class RecurringTransactionService { } /** - * This method checks whether the given {@link RecurringTransaction} is due today. - * A recurring transaction is due if the current {@link LocalDate date} is a multiple of the - * {@link RecurringTransaction#getFirstOccurrence() first occurrence} of the recurring transaction and the - * {@link RecurringTransaction#getIntervalType() interval type}. If today is a - * {@link RuleService#isHoliday(LocalDate) holiday} or a - * {@link RuleService#isWeekend(LocalDate) weekend day} the - * {@link HolidayWeekendType holiday weekend type} - * is taken into account to decide whether the recurring transaction should be deferred. + * This method checks whether the given {@link RecurringTransaction} is due today. A recurring transaction is due if + * the current {@link LocalDate date} is a multiple of the {@link RecurringTransaction#getFirstOccurrence() first + * occurrence} of the recurring transaction and the {@link RecurringTransaction#getIntervalType() interval type}. If + * today is a {@link RuleService#isHoliday(LocalDate) holiday} or a {@link RuleService#isWeekend(LocalDate) weekend + * day} the {@link HolidayWeekendType holiday weekend type} is taken into account to decide whether the recurring + * transaction should be deferred. * * @param recurringTransaction to check whether it is due today - * @param now today's date + * @param now today's date + * * @return true if the recurring transaction is due today, false otherwise */ private boolean checkRecurringTransactionDueToday(RecurringTransaction recurringTransaction, LocalDate now) { @@ -106,21 +114,26 @@ public class RecurringTransactionService { } /** - * This method checks whether the given {@link RecurringTransaction} was actually due in the close past - * but has been deferred to maybe today because the actual due day has been a holiday or weekend day and the - * {@link RecurringTransaction#getHolidayWeekendType() holiday weekend type} was - * {@link HolidayWeekendType#NEXT_WORKDAY}. Note that the recurring transaction may get deferred again if today - * again is a holiday or a weekend day. - * The period this method considers starts with today and ends with the last workday (no - * {@link RuleService#isHoliday(LocalDate) holiday}, not a {@link RuleService#isWeekend(LocalDate) weekend day}) - * whereas the end is exclusive, because if the recurring transaction would have been due at the last workday day - * it wouldn't has been deferred. + * This method checks whether the given {@link RecurringTransaction} was actually due in the close past but has been + * deferred to maybe today because the actual due day has been a holiday or weekend day and the {@link + * RecurringTransaction#getHolidayWeekendType() holiday weekend type} was {@link HolidayWeekendType#NEXT_WORKDAY}. + * Note that the recurring transaction may get deferred again if today again is a holiday or a weekend day. The + * period this method considers starts with today and ends with the last workday (no {@link + * RuleService#isHoliday(LocalDate) holiday}, not a {@link RuleService#isWeekend(LocalDate) weekend day}) whereas + * the end is exclusive, because if the recurring transaction would have been due at the last workday day it + * wouldn't has been deferred. * * @param recurringTransaction to check whether it is due today - * @param now today's date + * @param now today's date + * * @return true if the recurring transaction is due today, false otherwise */ private boolean checkRecurringTransactionDuePast(RecurringTransaction recurringTransaction, LocalDate now) { + // Recurring transactions with holiday weekend type SAME_DAY or PREVIOUS_WORKDAY can't be due in the past + if (!HolidayWeekendType.NEXT_WORKDAY.equals(recurringTransaction.getHolidayWeekendType())) { + return false; // early return + } + boolean weekend; boolean holiday; LocalDate yesterday = now; @@ -154,10 +167,129 @@ public class RecurringTransactionService { return due; } + @Transactional(propagation = Propagation.REQUIRED) public ResponseReason createRecurringTransaction(String fromAccountKey, String toAccountKey, Long amount, String description, String holidayWeekendType, String intervalType, String firstOccurrence, - String lastOccurrence) { - return null; + String lastOccurrence + ) { + final Account fromAccount = this.accountService.getAccountByKey(fromAccountKey); + final Account toAccount = this.accountService.getAccountByKey(toAccountKey); + ResponseReason response = validateParameters(fromAccount, toAccount, amount, holidayWeekendType, intervalType, + firstOccurrence, lastOccurrence); + + // If we detected an issue with the given parameters return the first error found to the caller + if (response != null) { + return response; // early return + } + + try { + final RecurringTransaction transaction = buildRecurringTransaction(fromAccount, toAccount, amount, + description, holidayWeekendType, intervalType, firstOccurrence, lastOccurrence); + + this.recurringTransactionRepository.save(transaction); + + response = ResponseReason.OK; + } catch (Exception e) { + // TODO log + + response = ResponseReason.UNKNOWN_ERROR; + } + + return response; + } + + /** + * This method builds the actual recurring transaction object with the given values. + * + * @param fromAccount the from account + * @param toAccount the to account + * @param amount the transaction amount + * @param holidayWeekendType the holiday weekend type + * @param intervalType the interval type + * @param firstOccurrence the first occurrence + * @param lastOccurrence the last occurrence, may be null + * + * @return the build {@link RecurringTransaction} instance + */ + private RecurringTransaction buildRecurringTransaction(Account fromAccount, Account toAccount, Long amount, + String description, String holidayWeekendType, + String intervalType, String firstOccurrence, + String lastOccurrence + ) { + final RecurringTransaction recurringTransaction = new RecurringTransaction(); + + recurringTransaction.setFromAccount(fromAccount); + recurringTransaction.setToAccount(toAccount); + recurringTransaction.setAmount(amount); + recurringTransaction.setDescription(description); + recurringTransaction.setHolidayWeekendType(HolidayWeekendType.valueOf(holidayWeekendType)); + recurringTransaction.setIntervalType(IntervalType.valueOf(intervalType)); + recurringTransaction.setFirstOccurrence(LocalDate + .parse(firstOccurrence, DateTimeFormatter.ofPattern(this.financerConfig.getDateFormat()))); + recurringTransaction.setLastOccurrence(LocalDate + .parse(lastOccurrence, DateTimeFormatter.ofPattern(this.financerConfig.getDateFormat()))); + + return recurringTransaction; + } + + /** + * This method checks whether the parameters for creating a transaction are valid. + * + * @param fromAccount the from account + * @param toAccount the to account + * @param amount the transaction amount + * @param holidayWeekendType the holiday weekend type + * @param intervalType the interval type + * @param firstOccurrence the first occurrence + * @param lastOccurrence the last occurrence, may be null + * + * @return the first error found or null if all parameters are valid + */ + private ResponseReason validateParameters(Account fromAccount, Account toAccount, Long amount, + String holidayWeekendType, String intervalType, String firstOccurrence, + String lastOccurrence + ) { + ResponseReason response = null; + + if (fromAccount == null && toAccount == null) { + response = ResponseReason.FROM_AND_TO_ACCOUNT_NOT_FOUND; + } else if (toAccount == null) { + response = ResponseReason.TO_ACCOUNT_NOT_FOUND; + } else if (fromAccount == null) { + response = ResponseReason.FROM_ACCOUNT_NOT_FOUND; + } else if (!this.ruleService.isValidBooking(fromAccount, toAccount)) { + response = ResponseReason.INVALID_BOOKING_ACCOUNTS; + } else if (amount == null) { + response = ResponseReason.MISSING_AMOUNT; + } else if (amount == 0l) { + response = ResponseReason.AMOUNT_ZERO; + } else if (holidayWeekendType == null) { + response = ResponseReason.MISSING_HOLIDAY_WEEKEND_TYPE; + } else if (!HolidayWeekendType.isValidType(holidayWeekendType)) { + response = ResponseReason.INVALID_HOLIDAY_WEEKEND_TYPE; + } else if (intervalType == null) { + response = ResponseReason.MISSING_INTERVAL_TYPE; + } else if (!IntervalType.isValidType(intervalType)) { + response = ResponseReason.INVALID_INTERVAL_TYPE; + } else if (firstOccurrence == null) { + response = ResponseReason.MISSING_FIRST_OCCURRENCE; + } + + try { + LocalDate.parse(firstOccurrence, DateTimeFormatter.ofPattern(this.financerConfig.getDateFormat())); + } catch (DateTimeParseException e) { + response = ResponseReason.INVALID_FIRST_OCCURRENCE_FORMAT; + } + + if (lastOccurrence != null) { + try { + LocalDate.parse(lastOccurrence, DateTimeFormatter.ofPattern(this.financerConfig.getDateFormat())); + } catch (DateTimeParseException e) { + response = ResponseReason.INVALID_LAST_OCCURRENCE_FORMAT; + } + } + + return response; } } diff --git a/src/main/java/de/financer/service/TransactionService.java b/src/main/java/de/financer/service/TransactionService.java index 7e3cdca..345c394 100644 --- a/src/main/java/de/financer/service/TransactionService.java +++ b/src/main/java/de/financer/service/TransactionService.java @@ -1,6 +1,7 @@ package de.financer.service; import de.financer.ResponseReason; +import de.financer.config.FinancerConfig; import de.financer.dba.TransactionRepository; import de.financer.model.Account; import de.financer.model.Transaction; @@ -16,8 +17,6 @@ import java.util.Collections; @Service public class TransactionService { - public static final String DATE_FORMAT = "dd.MM.yyyy"; - @Autowired private AccountService accountService; @@ -27,6 +26,9 @@ public class TransactionService { @Autowired private TransactionRepository transactionRepository; + @Autowired + private FinancerConfig financerConfig; + /** * @return all transactions, for all accounts and all time */ @@ -75,10 +77,6 @@ public class TransactionService { this.accountService.saveAccount(toAccount); response = ResponseReason.OK; - } catch (DateTimeParseException e) { - // TODO log - - response = ResponseReason.INVALID_DATE_FORMAT; } catch (Exception e) { // TODO log @@ -92,21 +90,20 @@ public class TransactionService { * This method builds the actual transaction object with the given values. * * @param fromAccount the from account - * @param toAccount the to account - * @param amount the transaction amount + * @param toAccount the to account + * @param amount the transaction amount * @param description the description of the transaction - * @param date the date of the transaction + * @param date the date of the transaction * @return the build {@link Transaction} instance - * @throws DateTimeParseException if the given date string cannot be parsed into a {@link java.time.LocalDate} instance */ - private Transaction buildTransaction(Account fromAccount, Account toAccount, Long amount, String description, String date) throws DateTimeParseException { + private Transaction buildTransaction(Account fromAccount, Account toAccount, Long amount, String description, String date) { final Transaction transaction = new Transaction(); transaction.setFromAccount(fromAccount); transaction.setToAccount(toAccount); transaction.setAmount(amount); transaction.setDescription(description); - transaction.setDate(LocalDate.parse(date, DateTimeFormatter.ofPattern(DATE_FORMAT))); + transaction.setDate(LocalDate.parse(date, DateTimeFormatter.ofPattern(this.financerConfig.getDateFormat()))); return transaction; } @@ -115,9 +112,9 @@ public class TransactionService { * This method checks whether the parameters for creating a transaction are valid. * * @param fromAccount the from account - * @param toAccount the to account - * @param amount the transaction amount - * @param date the transaction date + * @param toAccount the to account + * @param amount the transaction amount + * @param date the transaction date * @return the first error found or null if all parameters are valid */ private ResponseReason validateParameters(Account fromAccount, Account toAccount, Long amount, String date) { @@ -139,6 +136,14 @@ public class TransactionService { response = ResponseReason.MISSING_DATE; } + try { + LocalDate.parse(date, DateTimeFormatter.ofPattern(this.financerConfig.getDateFormat())); + } + catch (DateTimeParseException e) { + response = ResponseReason.INVALID_DATE_FORMAT; + } + + return response; } } diff --git a/src/main/resources/config/application.properties b/src/main/resources/config/application.properties index 00c8e39..9f05903 100644 --- a/src/main/resources/config/application.properties +++ b/src/main/resources/config/application.properties @@ -21,4 +21,7 @@ financer.countryCode=DE # The state used for holiday checks # For a complete list of the supported states see e.g. https://github.com/svendiedrichsen/jollyday/blob/master/src/main/resources/holidays/Holidays_de.xml -financer.state=sl \ No newline at end of file +financer.state=sl + +# The date format of the client-supplied date string, used to parse the string into a proper object +financer.dateFormat=dd.MM.yyyy \ No newline at end of file diff --git a/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_MONTHLY_SAME_DAYTest.java b/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_MONTHLY_SAME_DAYTest.java new file mode 100644 index 0000000..f787688 --- /dev/null +++ b/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_MONTHLY_SAME_DAYTest.java @@ -0,0 +1,68 @@ +package de.financer.service; + +import de.financer.dba.RecurringTransactionRepository; +import de.financer.model.HolidayWeekendType; +import de.financer.model.IntervalType; +import de.financer.model.RecurringTransaction; +import org.apache.commons.collections4.IterableUtils; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import java.time.LocalDate; +import java.time.Period; +import java.util.Collections; + +@RunWith(MockitoJUnitRunner.class) +public class RecurringTransactionService_getAllDueToday_MONTHLY_SAME_DAYTest { + @InjectMocks + private RecurringTransactionService classUnderTest; + + @Mock + private RecurringTransactionRepository recurringTransactionRepository; + + @Mock + private RuleService ruleService; + + @Before + public void setUp() { + Mockito.when(this.ruleService.getPeriodForInterval(IntervalType.MONTHLY)).thenReturn(Period.ofMonths(1)); + } + + /** + * This method tests whether a recurring transaction with firstOccurrence = one month and one day ago + * (and thus was actually due yesterday), intervalType = monthly and holidayWeekendType = same_day is not due today, + * if yesterday was a holiday but today is not + */ + @Test + public void test_getAllDueToday_duePast_holiday() { + // Arrange + Mockito.when(this.recurringTransactionRepository.findAll()) + .thenReturn(Collections.singletonList(createRecurringTransaction(-1))); + // Today is not a holiday but yesterday was + Mockito.when(this.ruleService.isHoliday(Mockito.any())).thenReturn(Boolean.FALSE, Boolean.TRUE); + final LocalDate now = LocalDate.now(); + + // Act + final Iterable recurringDueToday = this.classUnderTest.getAllDueToday(now); + + // Assert + Assert.assertEquals(0, IterableUtils.size(recurringDueToday)); + } + + private RecurringTransaction createRecurringTransaction(int days) { + final RecurringTransaction recurringTransaction = new RecurringTransaction(); + + recurringTransaction.setFirstOccurrence(LocalDate.now().plusDays(days).minusMonths(1)); + + recurringTransaction.setHolidayWeekendType(HolidayWeekendType.SAME_DAY); + recurringTransaction.setIntervalType(IntervalType.MONTHLY); + + return recurringTransaction; + } +}