From ebfce33733556fbf57a190118e18c8ab9a56d48a Mon Sep 17 00:00:00 2001 From: mgeiss Date: Fri, 6 Oct 2017 17:10:38 +0200 Subject: [PATCH] added more upfront verifications --- .../io/mifos/teller/TestTellerManagement.java | 23 ++++ .../io/mifos/teller/TestTellerOperation.java | 7 ++ .../PortfolioTransactionHandler.java | 11 +- .../service/TellerManagementService.java | 56 +++++---- .../service/helper/PortfolioService.java | 1 - .../rest/TellerOperationRestController.java | 118 ++++++++++++------ 6 files changed, 153 insertions(+), 63 deletions(-) diff --git a/component-test/src/main/java/io/mifos/teller/TestTellerManagement.java b/component-test/src/main/java/io/mifos/teller/TestTellerManagement.java index 8180d7b..80eae70 100644 --- a/component-test/src/main/java/io/mifos/teller/TestTellerManagement.java +++ b/component-test/src/main/java/io/mifos/teller/TestTellerManagement.java @@ -21,6 +21,7 @@ import io.mifos.teller.api.v1.client.TellerNotFoundException; import io.mifos.teller.api.v1.client.TellerValidationException; import io.mifos.teller.api.v1.domain.Teller; +import io.mifos.teller.api.v1.domain.TellerBalanceSheet; import io.mifos.teller.api.v1.domain.TellerManagementCommand; import io.mifos.teller.util.TellerGenerator; import org.apache.commons.lang3.RandomStringUtils; @@ -596,6 +597,28 @@ public void shouldNotDeleteTeller() throws Exception { Assert.assertTrue(super.eventRecorder.wait(EventConstants.DELETE_TELLER, teller.getCode())); } + @Test + public void shouldReturnZeroBalanceTellerNeverOpened() throws Exception { + final String officeIdentifier = RandomStringUtils.randomAlphabetic(32); + final Teller teller = TellerGenerator.createRandomTeller(); + teller.setCashdrawLimit(BigDecimal.valueOf(10000.00D)); + + Mockito.doAnswer(invocation -> true) + .when(super.organizationServiceSpy).officeExists(Matchers.eq(officeIdentifier)); + + Mockito.doAnswer(invocation -> Optional.of(new Account())) + .when(super.accountingServiceSpy).findAccount(Matchers.eq(teller.getTellerAccountIdentifier())); + + Mockito.doAnswer(invocation -> Optional.of(new Account())) + .when(super.accountingServiceSpy).findAccount(Matchers.eq(teller.getVaultAccountIdentifier())); + + super.testSubject.create(officeIdentifier, teller); + Assert.assertTrue(super.eventRecorder.wait(EventConstants.POST_TELLER, teller.getCode())); + + final TellerBalanceSheet tellerBalanceSheet = super.testSubject.getBalance(officeIdentifier, teller.getCode()); + Assert.assertTrue(BigDecimal.ZERO.compareTo(tellerBalanceSheet.getBalance()) == 0); + } + private void compareTeller(final Teller expected, final Teller actual) { Assert.assertEquals(expected.getCode(), actual.getCode()); Assert.assertEquals(expected.getTellerAccountIdentifier(), actual.getTellerAccountIdentifier()); diff --git a/component-test/src/main/java/io/mifos/teller/TestTellerOperation.java b/component-test/src/main/java/io/mifos/teller/TestTellerOperation.java index 02242a5..fdff73d 100644 --- a/component-test/src/main/java/io/mifos/teller/TestTellerOperation.java +++ b/component-test/src/main/java/io/mifos/teller/TestTellerOperation.java @@ -140,6 +140,8 @@ public void shouldOpenAccount() throws Exception { .when(super.depositAccountManagementServiceSpy).getCharges(Matchers.eq(tellerTransaction)); Mockito.doAnswer(invocation -> Collections.emptyList()) .when(super.depositAccountManagementServiceSpy).fetchProductInstances(tellerTransaction.getCustomerIdentifier()); + Mockito.doAnswer(invocation -> new ProductDefinition()) + .when(super.depositAccountManagementServiceSpy).findProductDefinition(tellerTransaction.getProductIdentifier()); super.testSubject.post(teller.getCode(), tellerTransaction); } @@ -415,6 +417,7 @@ public void shouldNotWithdrawExceedsCashDrawLimit() throws Exception { tellerTransaction.setAmount(BigDecimal.valueOf(15000L)); final Account account = new Account(); + account.setState(Account.State.OPEN.name()); account.setBalance(20000.00D); Mockito.doAnswer(invocation -> Optional.of(account)) .when(super.accountingServiceSpy).findAccount(tellerTransaction.getCustomerAccountIdentifier()); @@ -527,6 +530,8 @@ public void shouldNotReopenAccountClosed() throws Exception { .when(super.depositAccountManagementServiceSpy).getCharges(Matchers.eq(openAccountTransaction)); Mockito.doAnswer(invocation -> Collections.emptyList()) .when(super.depositAccountManagementServiceSpy).fetchProductInstances(openAccountTransaction.getCustomerIdentifier()); + Mockito.doAnswer(invocation -> new ProductDefinition()) + .when(super.depositAccountManagementServiceSpy).findProductDefinition(openAccountTransaction.getProductIdentifier()); final TellerTransactionCosts openingCosts = super.testSubject.post(teller.getCode(), openAccountTransaction); super.testSubject.confirm(teller.getCode(), openingCosts.getTellerTransactionIdentifier(), "CONFIRM", "excluded"); @@ -623,6 +628,7 @@ public void shouldProcessCheque() throws Exception { Mockito .doAnswer(invocation -> { final Account mockedAccount = new Account(); + mockedAccount.setBalance(2000.00D); mockedAccount.setState(Account.State.OPEN.name()); return Optional.of(mockedAccount); }) @@ -687,6 +693,7 @@ public void shouldNotProcessChequeAlreadyUsed() throws Exception { Mockito .doAnswer(invocation -> { final Account mockedAccount = new Account(); + mockedAccount.setBalance(1000.00D); mockedAccount.setState(Account.State.OPEN.name()); return Optional.of(mockedAccount); }) diff --git a/service/src/main/java/io/mifos/teller/service/internal/processor/PortfolioTransactionHandler.java b/service/src/main/java/io/mifos/teller/service/internal/processor/PortfolioTransactionHandler.java index 4d62d11..81c53d8 100644 --- a/service/src/main/java/io/mifos/teller/service/internal/processor/PortfolioTransactionHandler.java +++ b/service/src/main/java/io/mifos/teller/service/internal/processor/PortfolioTransactionHandler.java @@ -30,6 +30,8 @@ import java.math.BigDecimal; import java.util.List; import java.util.Optional; +import java.util.function.Function; +import java.util.stream.Collectors; @Component public class PortfolioTransactionHandler { @@ -59,9 +61,12 @@ public TellerTransactionCosts getTellerTransactionCosts(final TellerTransaction tellerTransactionCosts.setCharges(charges); tellerTransactionCosts.setTellerTransactionIdentifier(tellerTransaction.getIdentifier()); tellerTransactionCosts.setTotalAmount( - BigDecimal.valueOf( - charges.stream().mapToDouble(value -> value.getAmount().doubleValue()).sum() - ) + charges + .stream() + .map(Charge::getAmount) + .collect(Collectors.toList()) + .stream() + .reduce(BigDecimal.ZERO, BigDecimal::add) ); return tellerTransactionCosts; diff --git a/service/src/main/java/io/mifos/teller/service/internal/service/TellerManagementService.java b/service/src/main/java/io/mifos/teller/service/internal/service/TellerManagementService.java index 5335fae..6603b1d 100644 --- a/service/src/main/java/io/mifos/teller/service/internal/service/TellerManagementService.java +++ b/service/src/main/java/io/mifos/teller/service/internal/service/TellerManagementService.java @@ -17,7 +17,9 @@ import io.mifos.accounting.api.v1.domain.AccountEntry; import io.mifos.accounting.api.v1.domain.AccountEntryPage; +import io.mifos.accounting.api.v1.domain.TransactionType; import io.mifos.core.lang.DateConverter; +import io.mifos.teller.ServiceConstants; import io.mifos.teller.api.v1.domain.Teller; import io.mifos.teller.api.v1.domain.TellerBalanceSheet; import io.mifos.teller.api.v1.domain.TellerEntry; @@ -69,28 +71,38 @@ public TellerBalanceSheet getBalance(final String tellerCode) { final Optional optionalTellerEntity = this.tellerRepository.findByIdentifier(tellerCode); optionalTellerEntity.ifPresent(tellerEntity -> { - final String accountIdentifier = tellerEntity.getTellerAccountIdentifier(); - final LocalDate startDate = tellerEntity.getLastOpenedOn().toLocalDate(); - final LocalDate endDate = LocalDate.now(Clock.systemUTC()); - final String dateRange = - DateConverter.toIsoString(startDate) + ".." + DateConverter.toIsoString(endDate); - - final List tellerEntries = this.fetchTellerEntries(accountIdentifier, dateRange, 0); - tellerBalanceSheet.setEntries(tellerEntries); - tellerBalanceSheet.setDay(startDate.format(DateTimeFormatter.BASIC_ISO_DATE)); - final double sumDebits = tellerEntries - .stream() - .filter(tellerEntry -> tellerEntry.getType().equals(AccountEntry.Type.DEBIT.name())) - .mapToDouble(tellerEntry -> tellerEntry.getAmount().doubleValue()) - .sum(); - - final double sumCredits = tellerEntries - .stream() - .filter(tellerEntry -> tellerEntry.getType().equals(AccountEntry.Type.CREDIT.name())) - .mapToDouble(tellerEntry -> tellerEntry.getAmount().doubleValue()) - .sum(); - - tellerBalanceSheet.setBalance(BigDecimal.valueOf(sumDebits - sumCredits)); + if (tellerEntity.getLastOpenedOn() != null) { + final String accountIdentifier = tellerEntity.getTellerAccountIdentifier(); + final LocalDate startDate = tellerEntity.getLastOpenedOn().toLocalDate(); + final LocalDate endDate = LocalDate.now(Clock.systemUTC()); + final String dateRange = + DateConverter.toIsoString(startDate) + ".." + DateConverter.toIsoString(endDate); + + final List tellerEntries = this.fetchTellerEntries(accountIdentifier, dateRange, 0); + tellerBalanceSheet.setEntries(tellerEntries); + tellerBalanceSheet.setDay(startDate.format(DateTimeFormatter.BASIC_ISO_DATE)); + final BigDecimal sumDebits = tellerEntries + .stream() + .filter(tellerEntry -> tellerEntry.getType().equals(AccountEntry.Type.DEBIT.name()) + && !tellerEntry.getMessage().equals(ServiceConstants.TX_CHEQUE)) + .map(TellerEntry::getAmount) + .collect(Collectors.toList()) + .stream() + .reduce(BigDecimal.ZERO, BigDecimal::add); + + final BigDecimal sumCredits = tellerEntries + .stream() + .filter(tellerEntry -> tellerEntry.getType().equals(AccountEntry.Type.CREDIT.name()) + && !tellerEntry.getMessage().equals(ServiceConstants.TX_CHEQUE)) + .map(TellerEntry::getAmount) + .collect(Collectors.toList()) + .stream() + .reduce(BigDecimal.ZERO, BigDecimal::add); + + tellerBalanceSheet.setBalance(sumDebits.subtract(sumCredits)); + } else { + tellerBalanceSheet.setBalance(BigDecimal.ZERO); + } }); return tellerBalanceSheet; } diff --git a/service/src/main/java/io/mifos/teller/service/internal/service/helper/PortfolioService.java b/service/src/main/java/io/mifos/teller/service/internal/service/helper/PortfolioService.java index cc0df1e..ee70810 100644 --- a/service/src/main/java/io/mifos/teller/service/internal/service/helper/PortfolioService.java +++ b/service/src/main/java/io/mifos/teller/service/internal/service/helper/PortfolioService.java @@ -61,7 +61,6 @@ public List getCharges(final String productIdentifier, final String case final List charges = new ArrayList<>(); try { - //TODO switch CostComponent to PlannedPayment once avail final Payment payment = this.portfolioManager.getCostComponentsForAction( productIdentifier, caseIdentifier, Action.ACCEPT_PAYMENT.name(), Sets.newHashSet(), paymentSize); diff --git a/service/src/main/java/io/mifos/teller/service/rest/TellerOperationRestController.java b/service/src/main/java/io/mifos/teller/service/rest/TellerOperationRestController.java index da793ce..18f35cd 100644 --- a/service/src/main/java/io/mifos/teller/service/rest/TellerOperationRestController.java +++ b/service/src/main/java/io/mifos/teller/service/rest/TellerOperationRestController.java @@ -22,6 +22,7 @@ import io.mifos.core.command.gateway.CommandGateway; import io.mifos.core.lang.DateConverter; import io.mifos.core.lang.ServiceException; +import io.mifos.deposit.api.v1.definition.domain.ProductDefinition; import io.mifos.teller.ServiceConstants; import io.mifos.teller.api.v1.PermittableGroupIds; import io.mifos.teller.api.v1.domain.MICR; @@ -39,6 +40,7 @@ import io.mifos.teller.service.internal.service.TellerOperationService; import io.mifos.teller.service.internal.service.helper.AccountingService; import io.mifos.teller.service.internal.service.helper.ChequeService; +import io.mifos.teller.service.internal.service.helper.DepositAccountManagementService; import io.mifos.teller.service.internal.service.helper.OrganizationService; import io.mifos.teller.service.internal.util.MICRParser; import org.slf4j.Logger; @@ -59,7 +61,6 @@ import java.time.Clock; import java.time.LocalDate; import java.util.List; -import java.util.Optional; @RestController @RequestMapping("/teller/{tellerCode}") @@ -73,6 +74,7 @@ public class TellerOperationRestController { private final ChequeService chequeService; private final TellerTransactionProcessor tellerTransactionProcessor; private final OrganizationService organizationService; + private final DepositAccountManagementService depositAccountManagementService; @Autowired public TellerOperationRestController(@Qualifier(ServiceConstants.LOGGER_NAME) final Logger logger, @@ -82,7 +84,8 @@ public TellerOperationRestController(@Qualifier(ServiceConstants.LOGGER_NAME) fi final AccountingService accountingService, final ChequeService chequeService, final TellerTransactionProcessor tellerTransactionProcessor, - final OrganizationService organizationService) { + final OrganizationService organizationService, + final DepositAccountManagementService depositAccountManagementService) { this.logger = logger; this.commandGateway = commandGateway; this.tellerOperationService = tellerOperationService; @@ -91,6 +94,7 @@ public TellerOperationRestController(@Qualifier(ServiceConstants.LOGGER_NAME) fi this.chequeService = chequeService; this.tellerTransactionProcessor = tellerTransactionProcessor; this.organizationService = organizationService; + this.depositAccountManagementService = depositAccountManagementService; } @Permittable(value = AcceptedTokenType.TENANT, groupId = PermittableGroupIds.TELLER_OPERATION) @@ -168,36 +172,13 @@ ResponseEntity post(@PathVariable("tellerCode") final St @RequestBody @Valid final TellerTransaction tellerTransaction) { final Teller teller = this.verifyTeller(tellerCode); - this.verifyEmployee(teller); - if (!teller.getState().equals(Teller.State.ACTIVE.name())) { throw ServiceException.conflict("Teller {0} ist not active.", tellerCode); } - final String transactionType = tellerTransaction.getTransactionType(); - - if (transactionType.equals(ServiceConstants.TX_CASH_WITHDRAWAL) - || transactionType.equals(ServiceConstants.TX_CLOSE_ACCOUNT)) { - if (tellerTransaction.getAmount().compareTo(teller.getCashdrawLimit()) > 0) { - throw ServiceException.conflict("Amount exceeds cash drawl limit."); - } - } - + this.verifyEmployee(teller); this.verifyAccounts(tellerTransaction); - - if (transactionType.equals(ServiceConstants.TX_CHEQUE)) { - final LocalDate dateIssued = DateConverter.dateFromIsoString(tellerTransaction.getCheque().getDateIssued()); - final LocalDate sixMonth = LocalDate.now(Clock.systemUTC()).minusMonths(6); - if (dateIssued.isBefore(sixMonth)) { - throw ServiceException.conflict("Cheque is older than 6 months."); - } - - final MICR micr = tellerTransaction.getCheque().getMicr(); - final String chequeIdentifier = MICRParser.toIdentifier(micr); - if (this.chequeService.chequeExists(chequeIdentifier)) { - throw ServiceException.conflict("Cheque {0} already used.", chequeIdentifier); - } - } + this.verifyTellerTransaction(teller, tellerTransaction); try { return ResponseEntity.ok( @@ -223,6 +204,10 @@ ResponseEntity confirm(@PathVariable("tellerCode") final String tellerCode @RequestParam(value = "charges", required = false, defaultValue = "excluded") final String charges) { final Teller teller = this.verifyTeller(tellerCode); + if (!teller.getState().equals(Teller.State.ACTIVE.name())) { + throw ServiceException.conflict("Teller {0} ist not active.", tellerCode); + } + this.verifyEmployee(teller); switch (command.toUpperCase()) { @@ -266,12 +251,10 @@ ResponseEntity> fetch(@PathVariable("tellerCode") final } private Teller verifyTeller(final String tellerCode) { - final Optional optionalTeller = this.tellerManagementService.findByIdentifier(tellerCode); - if (!optionalTeller.isPresent()) { - throw ServiceException.notFound("Teller {0} not found.", tellerCode); - } else { - return optionalTeller.get(); - } + final Teller teller = this.tellerManagementService.findByIdentifier(tellerCode) + .orElseThrow(() -> ServiceException.notFound("Teller {0} not found.", tellerCode)); + + return teller; } private void verifyEmployee(final Teller teller) { @@ -297,8 +280,8 @@ private void verifyAccounts(final TellerTransaction tellerTransaction) { } private void verifyAccount(final String accountIdentifier) { - final Account account = this.accountingService.findAccount(accountIdentifier).orElseThrow( - () -> ServiceException.conflict("Account {0} not found.", accountIdentifier)); + final Account account = this.accountingService.findAccount(accountIdentifier) + .orElseThrow(() -> ServiceException.conflict("Account {0} not found.", accountIdentifier)); if (!account.getState().equals(Account.State.OPEN.name())) { throw ServiceException.conflict("Account {0} is not open.", accountIdentifier); @@ -318,8 +301,9 @@ private void verifyWithdrawalTransaction(final String tellerTransactionIdentifie || transactionType.equals(ServiceConstants.TX_CASH_WITHDRAWAL) || transactionType.equals(ServiceConstants.TX_CLOSE_ACCOUNT)) { - final Account account = this.accountingService.findAccount(tellerTransaction.getCustomerAccountIdentifier()).orElseThrow( - () -> ServiceException.notFound("Customer account {0} not found.", tellerTransaction.getCustomerAccountIdentifier())); + final Account account = this.accountingService.findAccount(tellerTransaction.getCustomerAccountIdentifier()) + .orElseThrow(() -> ServiceException.notFound("Customer account {0} not found.", + tellerTransaction.getCustomerAccountIdentifier())); final BigDecimal currentBalance = BigDecimal.valueOf(account.getBalance()); @@ -340,4 +324,64 @@ private void verifyWithdrawalTransaction(final String tellerTransactionIdentifie } } } + + private void verifyTellerTransaction(final Teller teller, final TellerTransaction tellerTransaction) { + final String transactionType = tellerTransaction.getTransactionType(); + final BigDecimal transactionAmount = tellerTransaction.getAmount(); + + if (transactionType.equals(ServiceConstants.TX_CASH_WITHDRAWAL) + || transactionType.equals(ServiceConstants.TX_CLOSE_ACCOUNT)) { + if (transactionAmount.compareTo(teller.getCashdrawLimit()) > 0) { + throw ServiceException.conflict("Amount exceeds cash drawl limit."); + } + } + + if (transactionType.equals(ServiceConstants.TX_OPEN_ACCOUNT)) { + final ProductDefinition productDefinition = + this.depositAccountManagementService.findProductDefinition(tellerTransaction.getProductIdentifier()); + if (productDefinition.getMinimumBalance() != null && productDefinition.getMinimumBalance() > 0.00D) { + final BigDecimal minimumBalance = BigDecimal.valueOf(productDefinition.getMinimumBalance()); + if (transactionAmount.compareTo(minimumBalance) < 0) { + throw ServiceException.conflict( + "Amount {0} must be equals or greater than minimum balance {1}.", + transactionAmount, minimumBalance); + } + + final TellerTransactionCosts tellerTransactionCosts = + this.tellerTransactionProcessor.getCosts(tellerTransaction); + final BigDecimal costs = tellerTransactionCosts.getTotalAmount(); + final BigDecimal amountExcludingCosts = transactionAmount.subtract(costs != null ? costs : BigDecimal.ZERO); + if (amountExcludingCosts.compareTo(minimumBalance) < 0) { + throw ServiceException.conflict( + "Amount {0} excluding costs {1} must be equals or greater than minimum balance {2}.", + transactionAmount, costs, minimumBalance); + } + } + } + + this.verifyChequeTransaction(tellerTransaction); + } + + private void verifyChequeTransaction(final TellerTransaction tellerTransaction) { + if (tellerTransaction.getTransactionType().equals(ServiceConstants.TX_CHEQUE)) { + final LocalDate dateIssued = DateConverter.dateFromIsoString(tellerTransaction.getCheque().getDateIssued()); + final LocalDate sixMonth = LocalDate.now(Clock.systemUTC()).minusMonths(6); + if (dateIssued.isBefore(sixMonth)) { + throw ServiceException.conflict("Cheque is older than 6 months."); + } + + final MICR micr = tellerTransaction.getCheque().getMicr(); + final String chequeIdentifier = MICRParser.toIdentifier(micr); + if (this.chequeService.chequeExists(chequeIdentifier)) { + throw ServiceException.conflict("Cheque {0} already used.", chequeIdentifier); + } + + this.accountingService.findAccount(micr.getAccountNumber()).ifPresent(account -> { + final BigDecimal balance = BigDecimal.valueOf(account.getBalance()); + if (tellerTransaction.getAmount().compareTo(balance) > 0) { + throw ServiceException.conflict("Cheque not covered."); + } + }); + } + } }