From 9b82084f21b3af2506f69163bc8823731a892438 Mon Sep 17 00:00:00 2001 From: MK13 Date: Sun, 5 May 2019 02:01:12 +0200 Subject: [PATCH] Introduce 'deleted' flag to RecurringTransaction Hard-deleting recurring transaction cannot be done reliable because a transaction may have a foreign key reference to a recurring transaction of this transaction has been created from a recurring transaction. Thus hard-deleting a recurring transaction may lead to data inconsistencies. --- .../dba/RecurringTransactionRepository.java | 4 +++- .../financer/model/RecurringTransaction.java | 9 +++++++++ .../service/RecurringTransactionService.java | 12 +++++++---- .../database/hsqldb/V1_0_0__init.sql | 1 + .../database/postgres/V1_0_0__init.sql | 1 + ...ervice_deleteRecurringTransactionTest.java | 2 +- ...getAllDueToday_DAILY_NEXT_WORKDAYTest.java | 9 +++++---- ...tAllDueToday_MONTHLY_NEXT_WORKDAYTest.java | 20 ++++++++++++++----- ...DueToday_MONTHLY_PREVIOUS_WORKDAYTest.java | 2 +- ...e_getAllDueToday_MONTHLY_SAME_DAYTest.java | 3 ++- 10 files changed, 46 insertions(+), 17 deletions(-) diff --git a/src/main/java/de/financer/dba/RecurringTransactionRepository.java b/src/main/java/de/financer/dba/RecurringTransactionRepository.java index e55bf38..5226f92 100644 --- a/src/main/java/de/financer/dba/RecurringTransactionRepository.java +++ b/src/main/java/de/financer/dba/RecurringTransactionRepository.java @@ -12,5 +12,7 @@ import java.time.LocalDate; public interface RecurringTransactionRepository extends CrudRepository { Iterable findRecurringTransactionsByFromAccountOrToAccount(Account fromAccount, Account toAccount); - Iterable findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(LocalDate lastOccurrence); + Iterable findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(LocalDate lastOccurrence); + + Iterable findByDeletedFalse(); } diff --git a/src/main/java/de/financer/model/RecurringTransaction.java b/src/main/java/de/financer/model/RecurringTransaction.java index 1a9d69e..3df99c8 100644 --- a/src/main/java/de/financer/model/RecurringTransaction.java +++ b/src/main/java/de/financer/model/RecurringTransaction.java @@ -20,6 +20,7 @@ public class RecurringTransaction { private LocalDate lastOccurrence; @Enumerated(EnumType.STRING) private HolidayWeekendType holidayWeekendType; + private boolean deleted; public Long getId() { return id; @@ -88,4 +89,12 @@ public class RecurringTransaction { public void setIntervalType(IntervalType intervalType) { this.intervalType = intervalType; } + + public boolean isDeleted() { + return deleted; + } + + public void setDeleted(boolean deleted) { + this.deleted = deleted; + } } diff --git a/src/main/java/de/financer/service/RecurringTransactionService.java b/src/main/java/de/financer/service/RecurringTransactionService.java index 777f015..4af4ce9 100644 --- a/src/main/java/de/financer/service/RecurringTransactionService.java +++ b/src/main/java/de/financer/service/RecurringTransactionService.java @@ -44,12 +44,12 @@ public class RecurringTransactionService { private TransactionService transactionService; public Iterable getAll() { - return this.recurringTransactionRepository.findAll(); + return this.recurringTransactionRepository.findByDeletedFalse(); } public Iterable getAllActive() { return this.recurringTransactionRepository - .findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(LocalDate.now()); + .findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(LocalDate.now()); } public Iterable getAllForAccount(String accountKey) { @@ -79,7 +79,7 @@ public class RecurringTransactionService { // Visible for unit tests /* package */ Iterable getAllDueToday(LocalDate now) { final Iterable allRecurringTransactions = this.recurringTransactionRepository - .findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(now); + .findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(now); //@formatter:off return IterableUtils.toList(allRecurringTransactions).stream() @@ -411,7 +411,11 @@ public class RecurringTransactionService { } try { - this.recurringTransactionRepository.deleteById(Long.valueOf(recurringTransactionId)); + RecurringTransaction recurringTransaction = optionalRecurringTransaction.get(); + + recurringTransaction.setDeleted(true); + + this.recurringTransactionRepository.save(recurringTransaction); } catch (Exception e) { LOGGER.error("Could not delete recurring transaction!", e); diff --git a/src/main/resources/database/hsqldb/V1_0_0__init.sql b/src/main/resources/database/hsqldb/V1_0_0__init.sql index 0737352..b1a900b 100644 --- a/src/main/resources/database/hsqldb/V1_0_0__init.sql +++ b/src/main/resources/database/hsqldb/V1_0_0__init.sql @@ -24,6 +24,7 @@ CREATE TABLE recurring_transaction ( first_occurrence DATE NOT NULL, last_occurrence DATE, holiday_weekend_type VARCHAR(255) NOT NULL, + deleted BOOLEAN DEFAULT FALSE NOT NULL, CONSTRAINT fk_recurring_transaction_from_account FOREIGN KEY (from_account_id) REFERENCES account (id), CONSTRAINT fk_recurring_transaction_to_account FOREIGN KEY (to_account_id) REFERENCES account (id) diff --git a/src/main/resources/database/postgres/V1_0_0__init.sql b/src/main/resources/database/postgres/V1_0_0__init.sql index 52158af..5d6221e 100644 --- a/src/main/resources/database/postgres/V1_0_0__init.sql +++ b/src/main/resources/database/postgres/V1_0_0__init.sql @@ -24,6 +24,7 @@ CREATE TABLE recurring_transaction ( first_occurrence DATE NOT NULL, last_occurrence DATE, holiday_weekend_type VARCHAR(255) NOT NULL, + deleted BOOLEAN DEFAULT 'TRUE' NOT NULL, CONSTRAINT fk_recurring_transaction_from_account FOREIGN KEY (from_account_id) REFERENCES account (id), CONSTRAINT fk_recurring_transaction_to_account FOREIGN KEY (to_account_id) REFERENCES account (id) diff --git a/src/test/java/de/financer/service/RecurringTransactionService_deleteRecurringTransactionTest.java b/src/test/java/de/financer/service/RecurringTransactionService_deleteRecurringTransactionTest.java index 635d1a9..8a44a1c 100644 --- a/src/test/java/de/financer/service/RecurringTransactionService_deleteRecurringTransactionTest.java +++ b/src/test/java/de/financer/service/RecurringTransactionService_deleteRecurringTransactionTest.java @@ -71,7 +71,7 @@ public class RecurringTransactionService_deleteRecurringTransactionTest { public void test_deleteRecurringTransaction_UNKNOWN_ERROR() { // Arrange Mockito.when(this.recurringTransactionRepository.findById(Mockito.anyLong())).thenReturn(Optional.of(new RecurringTransaction())); - Mockito.doThrow(new NullPointerException()).when(this.recurringTransactionRepository).deleteById(Mockito.anyLong()); + Mockito.doThrow(new NullPointerException()).when(this.recurringTransactionRepository).save(Mockito.any()); // Act final ResponseReason response = this.classUnderTest.deleteRecurringTransaction("123"); diff --git a/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_DAILY_NEXT_WORKDAYTest.java b/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_DAILY_NEXT_WORKDAYTest.java index d01b7f6..0471644 100644 --- a/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_DAILY_NEXT_WORKDAYTest.java +++ b/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_DAILY_NEXT_WORKDAYTest.java @@ -54,7 +54,7 @@ public class RecurringTransactionService_getAllDueToday_DAILY_NEXT_WORKDAYTest { // Arrange // Implicitly: ruleService.isHoliday().return(false) and ruleService.isWeekend().return(false) Mockito.when(this.recurringTransactionRepository - .findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) + .findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) .thenReturn(Collections.singletonList(createRecurringTransaction(-3))); final LocalDate now = LocalDate.now(); @@ -74,7 +74,7 @@ public class RecurringTransactionService_getAllDueToday_DAILY_NEXT_WORKDAYTest { // Arrange // Implicitly: ruleService.isWeekend().return(false) Mockito.when(this.recurringTransactionRepository - .findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) + .findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) .thenReturn(Collections.singletonList(createRecurringTransaction(0))); // Today is a holiday, but yesterday was not Mockito.when(this.ruleService.isHoliday(Mockito.any())).thenReturn(Boolean.TRUE, Boolean.FALSE); @@ -96,7 +96,7 @@ public class RecurringTransactionService_getAllDueToday_DAILY_NEXT_WORKDAYTest { // Arrange // Implicitly: ruleService.isHoliday().return(false) Mockito.when(this.recurringTransactionRepository - .findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) + .findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) .thenReturn(Collections.singletonList(createRecurringTransaction(0))); // Today is a weekend day, but yesterday was not Mockito.when(this.ruleService.isWeekend(Mockito.any())).thenReturn(Boolean.TRUE, Boolean.FALSE); @@ -118,7 +118,7 @@ public class RecurringTransactionService_getAllDueToday_DAILY_NEXT_WORKDAYTest { // Arrange // Implicitly: ruleService.isHoliday().return(false) and ruleService.isWeekend().return(false) Mockito.when(this.recurringTransactionRepository - .findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) + .findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) .thenReturn(Collections.singletonList(createRecurringTransaction(1))); final LocalDate now = LocalDate.now(); @@ -136,6 +136,7 @@ public class RecurringTransactionService_getAllDueToday_DAILY_NEXT_WORKDAYTest { recurringTransaction.setHolidayWeekendType(HolidayWeekendType.NEXT_WORKDAY); recurringTransaction.setIntervalType(IntervalType.DAILY); + recurringTransaction.setDeleted(false); return recurringTransaction; } diff --git a/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_MONTHLY_NEXT_WORKDAYTest.java b/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_MONTHLY_NEXT_WORKDAYTest.java index 420c171..db5f517 100644 --- a/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_MONTHLY_NEXT_WORKDAYTest.java +++ b/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_MONTHLY_NEXT_WORKDAYTest.java @@ -7,6 +7,7 @@ import de.financer.model.RecurringTransaction; import org.apache.commons.collections4.IterableUtils; import org.junit.Assert; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; @@ -43,7 +44,7 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_NEXT_WORKDAYTest public void test_getAllDueToday_duePast_holiday() { // Arrange Mockito.when(this.recurringTransactionRepository - .findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) + .findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) .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); @@ -81,7 +82,7 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_NEXT_WORKDAYTest final LocalDate monday = now.minusDays(now.getDayOfWeek().getValue() - 1); // The transaction occurs on a friday Mockito.when(this.recurringTransactionRepository - .findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) + .findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) .thenReturn(Collections.singletonList(createRecurringTransaction(-(now.getDayOfWeek().getValue() + 2)))); // First False for the dueToday check, 2x True for actual weekend, second False for Friday Mockito.when(this.ruleService.isWeekend(Mockito.any())) @@ -103,14 +104,20 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_NEXT_WORKDAYTest * (monday) */ @Test + @Ignore + // This test does not work as expected: if go back to the last sunday and then again one month back, we do + // not necessarily end up on on a date that causes the transaction to be due on monday + // e.g. 01.04.19 -> monday, 31.03.19 -> sunday, minus one month -> 28.02.19 + // whereas the resulting 28.02.19 would be the first occurrence of the transaction. The next due dates would + // be 28.03.19 and 28.04.19 and not the 01.04.19 as expected public void test_getAllDueToday_duePast_weekend_sunday() { // Arrange final LocalDate now = LocalDate.now(); final LocalDate monday = now.minusDays(now.getDayOfWeek().getValue() - 1); // The transaction occurs on a sunday Mockito.when(this.recurringTransactionRepository - .findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) - .thenReturn(Collections.singletonList(createRecurringTransaction(-now.getDayOfWeek().getValue()))); + .findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) + .thenReturn(Collections.singletonList(createRecurringTransaction(-(now.getDayOfWeek().getValue())))); // First False for the dueToday check, 2x True for actual weekend, second False for Friday Mockito.when(this.ruleService.isWeekend(Mockito.any())) .thenReturn(Boolean.FALSE, Boolean.TRUE, Boolean.TRUE, Boolean.FALSE); @@ -128,13 +135,15 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_NEXT_WORKDAYTest * today (monday) */ @Test + @Ignore + // Same as with the _sunday test -> does not work as expected public void test_getAllDueToday_duePast_weekend_saturday() { // Arrange final LocalDate now = LocalDate.now(); final LocalDate monday = now.minusDays(now.getDayOfWeek().getValue() - 1); // The transaction occurs on a saturday Mockito.when(this.recurringTransactionRepository - .findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) + .findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) .thenReturn(Collections.singletonList(createRecurringTransaction(-(now.getDayOfWeek().getValue() + 1)))); // First False for the dueToday check, 2x True for actual weekend, second False for Friday Mockito.when(this.ruleService.isWeekend(Mockito.any())) @@ -154,6 +163,7 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_NEXT_WORKDAYTest recurringTransaction.setHolidayWeekendType(HolidayWeekendType.NEXT_WORKDAY); recurringTransaction.setIntervalType(IntervalType.MONTHLY); + recurringTransaction.setDeleted(false); return recurringTransaction; } diff --git a/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_MONTHLY_PREVIOUS_WORKDAYTest.java b/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_MONTHLY_PREVIOUS_WORKDAYTest.java index 113aef3..2a17604 100644 --- a/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_MONTHLY_PREVIOUS_WORKDAYTest.java +++ b/src/test/java/de/financer/service/RecurringTransactionService_getAllDueToday_MONTHLY_PREVIOUS_WORKDAYTest.java @@ -43,7 +43,7 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_PREVIOUS_WORKDAY public void test_getAllDueToday_dueFuture_holiday() { // Arrange Mockito.when(this.recurringTransactionRepository - .findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) + .findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) .thenReturn(Collections.singletonList(createRecurringTransaction(1))); // Today is not a holiday but tomorrow is Mockito.when(this.ruleService.isHoliday(Mockito.any())).thenReturn(Boolean.FALSE, Boolean.TRUE); 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 index 8d5cf19..e04ef6a 100644 --- 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 @@ -43,7 +43,7 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_SAME_DAYTest { public void test_getAllDueToday_duePast_holiday() { // Arrange Mockito.when(this.recurringTransactionRepository - .findByLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) + .findByDeletedFalseAndLastOccurrenceIsNullOrLastOccurrenceGreaterThanEqual(Mockito.any())) .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); @@ -63,6 +63,7 @@ public class RecurringTransactionService_getAllDueToday_MONTHLY_SAME_DAYTest { recurringTransaction.setHolidayWeekendType(HolidayWeekendType.SAME_DAY); recurringTransaction.setIntervalType(IntervalType.MONTHLY); + recurringTransaction.setDeleted(false); return recurringTransaction; }