From 3e221a6907d60d9bf448521327f7285dcaceb838 Mon Sep 17 00:00:00 2001 From: Krish Date: Wed, 23 Sep 2020 18:10:00 +0100 Subject: [PATCH] PP-6988 Add RefundExpungeService - Added Refund expunge service which parity checks and expunges eligible refunds from connector --- README.md | 3 +- .../connector/app/config/ExpungeConfig.java | 8 ++ .../expunge/service/RefundExpungeService.java | 121 +++++++++++++++++ src/main/resources/config/config.yaml | 1 + .../service/RefundExpungeServiceTest.java | 124 ++++++++++++++++++ ...config-with-smartpay-timeout-override.yaml | 1 + ...config-with-worldpay-timeout-override.yaml | 1 + .../config/client-factory-test-config.yaml | 1 + src/test/resources/config/test-config.yaml | 1 + src/test/resources/config/test-it-config.yaml | 1 + 10 files changed, 261 insertions(+), 1 deletion(-) create mode 100644 src/main/java/uk/gov/pay/connector/expunge/service/RefundExpungeService.java create mode 100644 src/test/java/uk/gov/pay/connector/expunge/service/RefundExpungeServiceTest.java diff --git a/README.md b/README.md index b78f41066c..492b3d964a 100644 --- a/README.md +++ b/README.md @@ -47,11 +47,12 @@ The GOV.UK Pay Connector in Java (Dropwizard) |---------|---------|---------| | `EXPUNGE_NO_OF_CHARGES_OR_REFUNDS_PER_TASK_RUN` | 25000 | Number of charges or refunds to expunge each time expunge resource endpoint is invoked | | `EXPUNGE_EXCLUDE_CHARGES_OR_REFUNDS_PARITY_CHECKED_WITHIN_DAYS` | 7 | Exclude charges or refunds from expunging if parity checked within the configured days | -| `EXPUNGE_HISTORIC_CHARGE_OR_REFUND_EXCEPTIONS_OLDER_THAN_DAYS` | 90 | Number of days after which charges or refunds in a certain state (ex: CAPTURE_SUBMITTED for charge) can be expunged, even when not in expungeable state | +| `EXPUNGE_HISTORIC_CHARGE_EXCEPTIONS_OLDER_THAN_DAYS` | 90 | Number of days after which charges in a certain state (ex: CAPTURE_SUBMITTED for charge) can be expunged, even when not in expungeable state | | `EXPUNGE_CHARGES_ENABLED` | false | Set to true to enable expunging charges (in expungeable state) | | `EXPUNGE_CHARGES_OLDER_THAN_DAYS` | 7 | Expunge charges older than 7 days (or as configured) based on created date | | `EXPUNGE_REFUNDS_ENABLED` | false | Set to true to enable expunging refunds in terminal state | | `EXPUNGE_REFUNDS_OLDER_THAN_DAYS` | 7 | Expunge refunds older than 7 days (or as configured) based on created date | +| `EXPUNGE_HISTORIC_REFUND_EXCEPTIONS_OLDER_THAN_DAYS` | 90 | Number of days after which refunds in a certain state (ex: REFUND_SUBMITTED) can be expunged, even when not in terminal state | ### Background captures diff --git a/src/main/java/uk/gov/pay/connector/app/config/ExpungeConfig.java b/src/main/java/uk/gov/pay/connector/app/config/ExpungeConfig.java index 44de4c505c..537e541295 100644 --- a/src/main/java/uk/gov/pay/connector/app/config/ExpungeConfig.java +++ b/src/main/java/uk/gov/pay/connector/app/config/ExpungeConfig.java @@ -16,6 +16,10 @@ public class ExpungeConfig extends Configuration { @NotNull private int minimumAgeForHistoricChargeExceptions; + @Valid + @NotNull + private int minimumAgeForHistoricRefundExceptions; + @Valid @NotNull @Min(1) @@ -46,6 +50,10 @@ public int getExcludeChargesOrRefundsParityCheckedWithInDays() { return excludeChargesOrRefundsParityCheckedWithInDays; } + public int getMinimumAgeForHistoricRefundExceptions() { + return minimumAgeForHistoricRefundExceptions; + } + public boolean isExpungeChargesEnabled() { return expungeChargesEnabled; } diff --git a/src/main/java/uk/gov/pay/connector/expunge/service/RefundExpungeService.java b/src/main/java/uk/gov/pay/connector/expunge/service/RefundExpungeService.java new file mode 100644 index 0000000000..ce1d8a9937 --- /dev/null +++ b/src/main/java/uk/gov/pay/connector/expunge/service/RefundExpungeService.java @@ -0,0 +1,121 @@ +package uk.gov.pay.connector.expunge.service; + +import com.google.inject.persist.Transactional; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.MDC; +import uk.gov.pay.connector.app.ConnectorConfiguration; +import uk.gov.pay.connector.app.config.ExpungeConfig; +import uk.gov.pay.connector.refund.dao.RefundDao; +import uk.gov.pay.connector.refund.model.domain.RefundEntity; +import uk.gov.pay.connector.refund.model.domain.RefundStatus; +import uk.gov.pay.connector.refund.service.RefundService; +import uk.gov.pay.connector.tasks.service.ParityCheckService; + +import javax.inject.Inject; +import javax.persistence.OptimisticLockException; +import java.time.ZonedDateTime; +import java.time.temporal.ChronoUnit; +import java.util.stream.IntStream; + +import static java.lang.String.format; +import static java.time.ZoneOffset.UTC; +import static net.logstash.logback.argument.StructuredArguments.kv; +import static uk.gov.pay.connector.charge.model.domain.ParityCheckStatus.SKIPPED; +import static uk.gov.pay.connector.filters.RestClientLoggingFilter.HEADER_REQUEST_ID; +import static uk.gov.pay.connector.refund.model.domain.RefundStatus.REFUNDED; +import static uk.gov.pay.connector.refund.model.domain.RefundStatus.REFUND_ERROR; +import static uk.gov.pay.connector.refund.model.domain.RefundStatus.REFUND_SUBMITTED; +import static uk.gov.pay.logging.LoggingKeys.REFUND_EXTERNAL_ID; + +public class RefundExpungeService { + + private final Logger logger = LoggerFactory.getLogger(getClass()); + private final ExpungeConfig expungeConfig; + private final ParityCheckService parityCheckService; + private final RefundService refundService; + private final RefundDao refundDao; + + @Inject + public RefundExpungeService(ConnectorConfiguration connectorConfiguration, + ParityCheckService parityCheckService, + RefundService refundService, RefundDao refundDao) { + expungeConfig = connectorConfiguration.getExpungeConfig(); + this.parityCheckService = parityCheckService; + this.refundService = refundService; + this.refundDao = refundDao; + } + + public void expunge(Integer noOfRefundsToExpungeQueryParam) { + if (!expungeConfig.isExpungeRefundsEnabled()) { + logger.info("Refunds expunging feature is disabled. No refunds have been expunged"); + } else { + int noOfRefundsToExpunge = getNumberOfRefundsToExpunge(noOfRefundsToExpungeQueryParam); + int minimumAgeOfRefundInDays = expungeConfig.getMinimumAgeOfRefundInDays(); + int excludeRefundsParityCheckedWithInDays = expungeConfig.getExcludeChargesOrRefundsParityCheckedWithInDays(); + + IntStream.range(0, noOfRefundsToExpunge).forEach(number -> { + refundDao.findRefundToExpunge(minimumAgeOfRefundInDays, excludeRefundsParityCheckedWithInDays) + .ifPresent(refundEntity -> { + MDC.put(REFUND_EXTERNAL_ID, refundEntity.getExternalId()); + logger.info(format("Attempting to expunge refund %s", refundEntity.getExternalId())); + try { + parityCheckAndExpunge(refundEntity); + } catch (OptimisticLockException error) { + logger.info("Expunging process conflicted with an already running process, exit"); + MDC.remove(HEADER_REQUEST_ID); + throw error; + } + MDC.remove(REFUND_EXTERNAL_ID); + }); + }); + } + } + + private int getNumberOfRefundsToExpunge(Integer noOfRefundsToExpungeQueryParam) { + if (noOfRefundsToExpungeQueryParam != null && noOfRefundsToExpungeQueryParam > 0) { + return noOfRefundsToExpungeQueryParam; + } + return expungeConfig.getNumberOfChargesOrRefundsToExpunge(); + } + + private void parityCheckAndExpunge(RefundEntity refundEntity) { + boolean hasRefundBeenParityCheckedBefore = refundEntity.getParityCheckDate() != null; + + if (isInExpungeableState(refundEntity)) { + boolean matchesWithLedger = parityCheckService.parityCheckRefundForExpunger(refundEntity); + + if (matchesWithLedger) { + expungeRefund(refundEntity); + logger.info("Refund expunged from connector {}", kv(REFUND_EXTERNAL_ID, refundEntity.getExternalId())); + } else if (hasRefundBeenParityCheckedBefore) { + logger.error("Refund cannot be expunged because parity check with ledger repeatedly failed", + kv(REFUND_EXTERNAL_ID, refundEntity.getExternalId())); + } else { + logger.info("Refund cannot be expunged because parity check with ledger failed", + kv(REFUND_EXTERNAL_ID, refundEntity.getExternalId())); + } + } else { + refundService.updateRefundParityStatus(refundEntity.getExternalId(), SKIPPED); + logger.info("Refund is not in expungeable state", + kv(REFUND_EXTERNAL_ID, refundEntity.getExternalId())); + } + } + + private boolean isInExpungeableState(RefundEntity refundEntity) { + long ageInDays = ChronoUnit.DAYS.between(refundEntity.getCreatedDate(), ZonedDateTime.now(UTC)); + boolean isRefundHistoric = ageInDays > expungeConfig.getMinimumAgeForHistoricRefundExceptions(); + + RefundStatus refundStatus = refundEntity.getStatus(); + if (isRefundHistoric && REFUND_SUBMITTED.equals(refundStatus)) { + return true; + } + + return REFUNDED.equals(refundStatus) || REFUND_ERROR.equals(refundStatus); + } + + @Transactional + public void expungeRefund(RefundEntity refundEntity) { + refundDao.expungeRefund(refundEntity.getExternalId()); + } +} diff --git a/src/main/resources/config/config.yaml b/src/main/resources/config/config.yaml index e3091fa4c1..b0726bd1ac 100644 --- a/src/main/resources/config/config.yaml +++ b/src/main/resources/config/config.yaml @@ -249,6 +249,7 @@ expungeConfig: minimumAgeForHistoricChargeExceptions: ${EXPUNGE_HISTORIC_CHARGE_EXCEPTIONS_OLDER_THAN_DAYS:-90} expungeRefundsEnabled: ${EXPUNGE_REFUNDS_ENABLED:-false} minimumAgeOfRefundInDays: ${EXPUNGE_REFUNDS_OLDER_THAN_DAYS:-7} + minimumAgeForHistoricRefundExceptions: ${EXPUNGE_HISTORIC_REFUND_EXCEPTIONS_OLDER_THAN_DAYS:-90} authorisation3dsConfig: maximumNumberOfTimesToAllowUserToAttempt3ds: ${MAXIMUM_NO_USER_3DS_ATTEMPTS:-1} diff --git a/src/test/java/uk/gov/pay/connector/expunge/service/RefundExpungeServiceTest.java b/src/test/java/uk/gov/pay/connector/expunge/service/RefundExpungeServiceTest.java new file mode 100644 index 0000000000..685fbb21cf --- /dev/null +++ b/src/test/java/uk/gov/pay/connector/expunge/service/RefundExpungeServiceTest.java @@ -0,0 +1,124 @@ +package uk.gov.pay.connector.expunge.service; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import uk.gov.pay.connector.app.ConnectorConfiguration; +import uk.gov.pay.connector.app.config.ExpungeConfig; +import uk.gov.pay.connector.model.domain.RefundEntityFixture; +import uk.gov.pay.connector.refund.dao.RefundDao; +import uk.gov.pay.connector.refund.model.domain.RefundEntity; +import uk.gov.pay.connector.refund.service.RefundService; +import uk.gov.pay.connector.tasks.service.ParityCheckService; + +import java.time.ZonedDateTime; +import java.util.Optional; + +import static java.time.ZoneOffset.UTC; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; +import static uk.gov.pay.connector.charge.model.domain.ParityCheckStatus.SKIPPED; +import static uk.gov.pay.connector.refund.model.domain.RefundStatus.REFUNDED; +import static uk.gov.pay.connector.refund.model.domain.RefundStatus.REFUND_SUBMITTED; + +@RunWith(MockitoJUnitRunner.class) +public class RefundExpungeServiceTest { + + private RefundExpungeService refundExpungeService; + private int minimumAgeOfRefundInDays = 3; + private int defaultNumberOfRefundsToExpunge = 10; + private int defaultExcludeRefundsParityCheckedWithInDays = 10; + + @Mock + private ExpungeConfig mockExpungeConfig; + @Mock + private RefundDao mockRefundDao; + @Mock + private RefundService mockRefundService; + @Mock + private ConnectorConfiguration mockConnectorConfiguration; + @Mock + private ParityCheckService mockParityCheckService; + + @Before + public void setUp() { + when(mockExpungeConfig.isExpungeRefundsEnabled()).thenReturn(true); + when(mockExpungeConfig.getNumberOfChargesOrRefundsToExpunge()).thenReturn(defaultNumberOfRefundsToExpunge); + when(mockExpungeConfig.getMinimumAgeOfRefundInDays()).thenReturn(minimumAgeOfRefundInDays); + when(mockExpungeConfig.getExcludeChargesOrRefundsParityCheckedWithInDays()).thenReturn(defaultExcludeRefundsParityCheckedWithInDays); + when(mockExpungeConfig.getMinimumAgeForHistoricRefundExceptions()).thenReturn(10); + + when(mockConnectorConfiguration.getExpungeConfig()).thenReturn(mockExpungeConfig); + + refundExpungeService = new RefundExpungeService(mockConnectorConfiguration, mockParityCheckService, + mockRefundService, mockRefundDao); + } + + @Test + public void expunge_shouldExpungeNoOfRefundsAsPerConfiguration() { + RefundEntity refundEntity = RefundEntityFixture.aValidRefundEntity() + .withStatus(REFUNDED).build(); + when(mockParityCheckService.parityCheckRefundForExpunger(any())).thenReturn(true); + when(mockRefundDao.findRefundToExpunge(minimumAgeOfRefundInDays, defaultExcludeRefundsParityCheckedWithInDays)) + .thenReturn(Optional.of(refundEntity)); + refundExpungeService.expunge(null); + + verify(mockRefundDao, times(defaultNumberOfRefundsToExpunge)).expungeRefund(any()); + verify(mockRefundDao, times(defaultNumberOfRefundsToExpunge)).findRefundToExpunge(minimumAgeOfRefundInDays, + defaultExcludeRefundsParityCheckedWithInDays); + } + + @Test + public void expunge_shouldExpungeHistoricRefundInNonTerminalState() { + RefundEntity refundEntity = RefundEntityFixture.aValidRefundEntity() + .withCreatedDate(ZonedDateTime.now(UTC).minusDays(20)) + .withStatus(REFUND_SUBMITTED).build(); + when(mockRefundDao.findRefundToExpunge(minimumAgeOfRefundInDays, defaultExcludeRefundsParityCheckedWithInDays)) + .thenReturn(Optional.of(refundEntity)); + when(mockParityCheckService.parityCheckRefundForExpunger(refundEntity)).thenReturn(true); + + refundExpungeService.expunge(1); + + verify(mockRefundDao).expungeRefund(refundEntity.getExternalId()); + } + + @Test + public void expunge_shouldNotExpungeRefundsIfFeatureIsNotEnabled() { + when(mockExpungeConfig.isExpungeRefundsEnabled()).thenReturn(false); + + refundExpungeService.expunge(null); + verifyNoInteractions(mockRefundDao); + } + + @Test + public void expunge_shouldNotExpungeRefundInNonTerminalState() { + RefundEntity refundEntity = RefundEntityFixture.aValidRefundEntity() + .withStatus(REFUND_SUBMITTED).build(); + when(mockRefundDao.findRefundToExpunge(minimumAgeOfRefundInDays, defaultExcludeRefundsParityCheckedWithInDays)) + .thenReturn(Optional.of(refundEntity)); + + refundExpungeService.expunge(1); + + verify(mockRefundService).updateRefundParityStatus(refundEntity.getExternalId(), SKIPPED); + verify(mockRefundDao, never()).expungeRefund(any()); + } + + @Test + public void expunge_shouldNotExpungeRefundIfParityCheckFailed() { + RefundEntity refundEntity = RefundEntityFixture.aValidRefundEntity() + .withStatus(REFUNDED).build(); + when(mockRefundDao.findRefundToExpunge(minimumAgeOfRefundInDays, defaultExcludeRefundsParityCheckedWithInDays)) + .thenReturn(Optional.of(refundEntity)); + when(mockParityCheckService.parityCheckRefundForExpunger(refundEntity)).thenReturn(false); + + refundExpungeService.expunge(1); + + verify(mockRefundDao, never()).expungeRefund(any()); + } +} diff --git a/src/test/resources/config/client-factory-test-config-with-smartpay-timeout-override.yaml b/src/test/resources/config/client-factory-test-config-with-smartpay-timeout-override.yaml index 1770d3eb70..9fe4f80b7d 100644 --- a/src/test/resources/config/client-factory-test-config-with-smartpay-timeout-override.yaml +++ b/src/test/resources/config/client-factory-test-config-with-smartpay-timeout-override.yaml @@ -194,6 +194,7 @@ expungeConfig: minimumAgeForHistoricChargeExceptions: ${EXPUNGE_HISTORIC_CHARGE_EXCEPTIONS_OLDER_THAN_DAYS:-90} expungeRefundsEnabled: ${EXPUNGE_REFUNDS_ENABLED:-false} minimumAgeOfRefundInDays: ${EXPUNGE_REFUNDS_OLDER_THAN_DAYS:-7} + minimumAgeForHistoricRefundExceptions: ${EXPUNGE_HISTORIC_REFUND_EXCEPTIONS_OLDER_THAN_DAYS:-90} authorisation3dsConfig: maximumNumberOfTimesToAllowUserToAttempt3ds: ${MAXIMUM_NO_USER_3DS_ATTEMPTS:-1} diff --git a/src/test/resources/config/client-factory-test-config-with-worldpay-timeout-override.yaml b/src/test/resources/config/client-factory-test-config-with-worldpay-timeout-override.yaml index e031abd4b5..1df6418ef5 100644 --- a/src/test/resources/config/client-factory-test-config-with-worldpay-timeout-override.yaml +++ b/src/test/resources/config/client-factory-test-config-with-worldpay-timeout-override.yaml @@ -193,6 +193,7 @@ expungeConfig: minimumAgeForHistoricChargeExceptions: ${EXPUNGE_HISTORIC_CHARGE_EXCEPTIONS_OLDER_THAN_DAYS:-90} expungeRefundsEnabled: ${EXPUNGE_REFUNDS_ENABLED:-false} minimumAgeOfRefundInDays: ${EXPUNGE_REFUNDS_OLDER_THAN_DAYS:-7} + minimumAgeForHistoricRefundExceptions: ${EXPUNGE_HISTORIC_REFUND_EXCEPTIONS_OLDER_THAN_DAYS:-90} authorisation3dsConfig: maximumNumberOfTimesToAllowUserToAttempt3ds: ${MAXIMUM_NO_USER_3DS_ATTEMPTS:-1} diff --git a/src/test/resources/config/client-factory-test-config.yaml b/src/test/resources/config/client-factory-test-config.yaml index a679408198..be14d64436 100644 --- a/src/test/resources/config/client-factory-test-config.yaml +++ b/src/test/resources/config/client-factory-test-config.yaml @@ -183,6 +183,7 @@ expungeConfig: minimumAgeForHistoricChargeExceptions: ${EXPUNGE_HISTORIC_CHARGE_EXCEPTIONS_OLDER_THAN_DAYS:-90} expungeRefundsEnabled: ${EXPUNGE_REFUNDS_ENABLED:-false} minimumAgeOfRefundInDays: ${EXPUNGE_REFUNDS_OLDER_THAN_DAYS:-7} + minimumAgeForHistoricRefundExceptions: ${EXPUNGE_HISTORIC_REFUND_EXCEPTIONS_OLDER_THAN_DAYS:-90} authorisation3dsConfig: maximumNumberOfTimesToAllowUserToAttempt3ds: ${MAXIMUM_NO_USER_3DS_ATTEMPTS:-1} diff --git a/src/test/resources/config/test-config.yaml b/src/test/resources/config/test-config.yaml index 15d1af07c5..81931a99bf 100644 --- a/src/test/resources/config/test-config.yaml +++ b/src/test/resources/config/test-config.yaml @@ -189,6 +189,7 @@ expungeConfig: minimumAgeForHistoricChargeExceptions: ${EXPUNGE_HISTORIC_CHARGE_EXCEPTIONS_OLDER_THAN_DAYS:-90} expungeRefundsEnabled: ${EXPUNGE_REFUNDS_ENABLED:-false} minimumAgeOfRefundInDays: ${EXPUNGE_REFUNDS_OLDER_THAN_DAYS:-7} + minimumAgeForHistoricRefundExceptions: ${EXPUNGE_HISTORIC_REFUND_EXCEPTIONS_OLDER_THAN_DAYS:-90} authorisation3dsConfig: maximumNumberOfTimesToAllowUserToAttempt3ds: ${MAXIMUM_NO_USER_3DS_ATTEMPTS:-1} diff --git a/src/test/resources/config/test-it-config.yaml b/src/test/resources/config/test-it-config.yaml index 2448e41538..98ac12c6d8 100644 --- a/src/test/resources/config/test-it-config.yaml +++ b/src/test/resources/config/test-it-config.yaml @@ -188,6 +188,7 @@ expungeConfig: minimumAgeForHistoricChargeExceptions: ${EXPUNGE_HISTORIC_CHARGE_EXCEPTIONS_OLDER_THAN_DAYS:-90} expungeRefundsEnabled: ${EXPUNGE_REFUNDS_ENABLED:-true} minimumAgeOfRefundInDays: ${EXPUNGE_REFUNDS_OLDER_THAN_DAYS:-90} + minimumAgeForHistoricRefundExceptions: ${EXPUNGE_HISTORIC_REFUND_EXCEPTIONS_OLDER_THAN_DAYS:-90} authorisation3dsConfig: maximumNumberOfTimesToAllowUserToAttempt3ds: ${MAXIMUM_NO_USER_3DS_ATTEMPTS:-1}