Skip to content

Commit

Permalink
PP-13623 Set transaction ID for Worldpay refunds on creation
Browse files Browse the repository at this point in the history
For Worldpay, the gateway transaction ID (how Worldpay refers to
the refund) is always the same as the external ID of the refund.
Set the gateway transaction ID as soon as the refund entity is
created, rather than when we process the response to the refund
request to Worldpay, to avoid a possible race condition if we
receive a refund notification from Worldpay immediately.

To do this, introduce a RefundEntityFactory interface so the code
for different payment gateways can supply one with the desired
behaviour.

Co-authored-by: Alex Bishop <alexbishop1@users.noreply.github.com>
Co-authored-by: Hugo Jeffreys <hugo.jeffreys@digital.cabinet-office.gov.uk>
Co-authored-by: Idris Abdirizak <idris.abdirizak@digital.cabinet-office.gov.uk>
Co-authored-by: Marco Tranchino <marco.tranchino@digital.cabinet-office.gov.uk>
Co-authored-by: Sandor Arpa <SandorArpa@users.noreply.github.com>
  • Loading branch information
5 people committed Feb 11, 2025
1 parent 6ef2a3f commit 82ed499
Show file tree
Hide file tree
Showing 20 changed files with 225 additions and 56 deletions.
10 changes: 5 additions & 5 deletions .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -536,28 +536,28 @@
"filename": "src/test/java/uk/gov/pay/connector/gateway/worldpay/WorldpayPaymentProviderTest.java",
"hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8",
"is_verified": false,
"line_number": 145
"line_number": 146
},
{
"type": "Secret Keyword",
"filename": "src/test/java/uk/gov/pay/connector/gateway/worldpay/WorldpayPaymentProviderTest.java",
"hashed_secret": "fd9f851472586dc75f7a60ee311842ec64ecf527",
"is_verified": false,
"line_number": 148
"line_number": 149
},
{
"type": "Secret Keyword",
"filename": "src/test/java/uk/gov/pay/connector/gateway/worldpay/WorldpayPaymentProviderTest.java",
"hashed_secret": "9367c8305bd1afe815ffc0cc3ebf95af9cb6d157",
"is_verified": false,
"line_number": 151
"line_number": 152
},
{
"type": "Base64 High Entropy String",
"filename": "src/test/java/uk/gov/pay/connector/gateway/worldpay/WorldpayPaymentProviderTest.java",
"hashed_secret": "c6b43e7d8e68a66c283fd8760118b89592f6498d",
"is_verified": false,
"line_number": 1184
"line_number": 1188
}
],
"src/test/java/uk/gov/pay/connector/gateway/worldpay/wallets/WorldpayWalletAuthorisationHandlerTest.java": [
Expand Down Expand Up @@ -1101,5 +1101,5 @@
}
]
},
"generated_at": "2025-02-03T15:39:00Z"
"generated_at": "2025-02-11T16:17:13Z"
}
17 changes: 17 additions & 0 deletions src/main/java/uk/gov/pay/connector/app/ConnectorModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
import uk.gov.pay.connector.gatewayaccount.service.GatewayAccountServicesFactory;
import uk.gov.pay.connector.paymentprocessor.service.CardExecutorService;
import uk.gov.pay.connector.queue.statetransition.StateTransitionQueue;
import uk.gov.pay.connector.refund.service.DefaultRefundEntityFactory;
import uk.gov.pay.connector.refund.service.RefundEntityFactory;
import uk.gov.pay.connector.refund.service.WorldpayRefundEntityFactory;
import uk.gov.pay.connector.usernotification.govuknotify.NotifyClientFactory;
import uk.gov.pay.connector.util.CidrUtils;
import uk.gov.pay.connector.util.HashUtil;
Expand Down Expand Up @@ -185,6 +188,20 @@ public GatewayClient worldpayValidateCredentialsGatewayClient(GatewayClientFacto
return gatewayClientFactory.createGatewayClient(WORLDPAY, VALIDATE_CREDENTIALS, environment.metrics());
}

@Provides
@Singleton
@Named("DefaultRefundEntityFactory")
public RefundEntityFactory defaultRefundEntityFactory() {
return new DefaultRefundEntityFactory();
}

@Provides
@Singleton
@Named("WorldpayRefundEntityFactory")
public RefundEntityFactory worldpayRefundEntityFactory() {
return new WorldpayRefundEntityFactory();
}

@Provides
@Singleton
@Named("AllowedEpdqIpAddresses")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import uk.gov.pay.connector.gateway.model.response.GatewayResponse;
import uk.gov.pay.connector.gatewayaccount.model.GatewayAccountEntity;
import uk.gov.pay.connector.refund.model.domain.Refund;
import uk.gov.pay.connector.refund.service.RefundEntityFactory;
import uk.gov.pay.connector.wallets.applepay.ApplePayAuthorisationGatewayRequest;
import uk.gov.pay.connector.wallets.googlepay.GooglePayAuthorisationGatewayRequest;

Expand Down Expand Up @@ -67,4 +68,6 @@ default GatewayResponse<BaseAuthoriseResponse> authoriseGooglePay(GooglePayAutho
default void deleteStoredPaymentDetails(DeleteStoredPaymentDetailsGatewayRequest request) throws GatewayException {
throw new NotImplementedException("Delete Stored Payment Details is not implemented for this payment provider");
}

RefundEntityFactory getRefundEntityFactory();
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@
import uk.gov.pay.connector.gateway.util.ExternalRefundAvailabilityCalculator;
import uk.gov.pay.connector.gatewayaccount.model.GatewayAccountEntity;
import uk.gov.pay.connector.refund.model.domain.Refund;
import uk.gov.pay.connector.refund.service.RefundEntityFactory;
import uk.gov.pay.connector.wallets.applepay.ApplePayAuthorisationGatewayRequest;
import uk.gov.pay.connector.wallets.googlepay.GooglePayAuthorisationGatewayRequest;

import javax.inject.Inject;
import javax.inject.Named;
import java.util.List;
import java.util.Optional;

Expand All @@ -47,11 +50,14 @@ public class SandboxPaymentProvider implements PaymentProvider {

private final ExternalRefundAvailabilityCalculator externalRefundAvailabilityCalculator;
private final SandboxWalletAuthorisationHandler sandboxWalletAuthorisationHandler;
private final RefundEntityFactory refundEntityFactory;
private final SandboxGatewayResponseGenerator fullCardNumberSandboxResponseGenerator = new SandboxGatewayResponseGenerator(new SandboxFullCardNumbers());
private final SandboxGatewayResponseGenerator walletSandboxResponseGenerator = new SandboxGatewayResponseGenerator(new SandboxLast4DigitsCardNumbers());
private final SandboxGatewayResponseGenerator recurringSandboxResponseGenerator = new SandboxGatewayResponseGenerator(new SandboxFirst6AndLast4CardNumbers());

public SandboxPaymentProvider() {
@Inject
public SandboxPaymentProvider(@Named("DefaultRefundEntityFactory") RefundEntityFactory refundEntityFactory) {
this.refundEntityFactory = refundEntityFactory;
this.externalRefundAvailabilityCalculator = new DefaultExternalRefundAvailabilityCalculator();
this.sandboxWalletAuthorisationHandler = new SandboxWalletAuthorisationHandler(walletSandboxResponseGenerator);
}
Expand Down Expand Up @@ -187,4 +193,9 @@ public String toString() {
public void deleteStoredPaymentDetails(DeleteStoredPaymentDetailsGatewayRequest request) {
//No action needs to be taken in sandbox in response to a deleteStoredPaymentDetails task
}

@Override
public RefundEntityFactory getRefundEntityFactory() {
return refundEntityFactory;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,14 @@
import uk.gov.pay.connector.gatewayaccount.model.GatewayAccountEntity;
import uk.gov.pay.connector.gatewayaccountcredentials.model.GatewayAccountCredentialsEntity;
import uk.gov.pay.connector.refund.model.domain.Refund;
import uk.gov.pay.connector.refund.service.RefundEntityFactory;
import uk.gov.pay.connector.util.JsonObjectMapper;
import uk.gov.pay.connector.util.RandomIdGenerator;
import uk.gov.pay.connector.wallets.applepay.ApplePayAuthorisationGatewayRequest;
import uk.gov.pay.connector.wallets.googlepay.GooglePayAuthorisationGatewayRequest;

import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton;
import java.util.List;
import java.util.Optional;
Expand All @@ -78,13 +80,15 @@ public class StripePaymentProvider implements PaymentProvider {
private final StripeFailedPaymentFeeCollectionHandler stripeFailedPaymentFeeCollectionHandler;
private final StripeQueryPaymentStatusHandler stripeQueryPaymentStatusHandler;
private final StripeDisputeHandler stripeDisputeHandler;
private final RefundEntityFactory refundEntityFactory;
private final StripeSdkClient stripeSDKClient;

@Inject
public StripePaymentProvider(GatewayClientFactory gatewayClientFactory,
ConnectorConfiguration configuration,
JsonObjectMapper jsonObjectMapper,
Environment environment,
@Named("DefaultRefundEntityFactory") RefundEntityFactory refundEntityFactory,
StripeSdkClient stripeSDKClient) {
this.stripeGatewayConfig = configuration.getStripeConfig();
this.stripeSDKClient = stripeSDKClient;
Expand All @@ -98,6 +102,7 @@ public StripePaymentProvider(GatewayClientFactory gatewayClientFactory,
stripeFailedPaymentFeeCollectionHandler = new StripeFailedPaymentFeeCollectionHandler(client, stripeGatewayConfig, jsonObjectMapper);
stripeQueryPaymentStatusHandler = new StripeQueryPaymentStatusHandler(client, stripeGatewayConfig, jsonObjectMapper);
stripeDisputeHandler = new StripeDisputeHandler(client, stripeGatewayConfig, jsonObjectMapper);
this.refundEntityFactory = refundEntityFactory;
}

@Override
Expand Down Expand Up @@ -209,6 +214,11 @@ public void deleteStoredPaymentDetails(DeleteStoredPaymentDetailsGatewayRequest
}
}

@Override
public RefundEntityFactory getRefundEntityFactory() {
return refundEntityFactory;
}

public List<Fee> calculateAndTransferFeesForFailedPayments(ChargeEntity charge) throws GatewayException {
return stripeFailedPaymentFeeCollectionHandler.calculateAndTransferFees(charge);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import uk.gov.pay.connector.paymentprocessor.model.Exemption3ds;
import uk.gov.pay.connector.paymentprocessor.service.AuthorisationService;
import uk.gov.pay.connector.refund.model.domain.Refund;
import uk.gov.pay.connector.refund.service.RefundEntityFactory;
import uk.gov.pay.connector.wallets.applepay.ApplePayAuthorisationGatewayRequest;
import uk.gov.pay.connector.wallets.googlepay.GooglePayAuthorisationGatewayRequest;

Expand All @@ -66,14 +67,14 @@
import static uk.gov.pay.connector.gateway.PaymentGatewayName.WORLDPAY;
import static uk.gov.pay.connector.gateway.util.AuthUtil.getWorldpayAuthHeader;
import static uk.gov.pay.connector.gateway.util.AuthUtil.getWorldpayAuthHeaderForManagingRecurringAuthTokens;
import static uk.gov.pay.connector.gateway.worldpay.SendWorldpayExemptionRequest.DO_NOT_SEND_EXEMPTION_REQUEST;
import static uk.gov.pay.connector.gateway.worldpay.SendWorldpayExemptionRequest.SEND_CORPORATE_EXEMPTION_REQUEST;
import static uk.gov.pay.connector.gateway.worldpay.SendWorldpayExemptionRequest.SEND_EXEMPTION_ENGINE_REQUEST;
import static uk.gov.pay.connector.gateway.worldpay.WorldpayOrderRequestBuilder.aWorldpay3dsResponseAuthOrderRequestBuilder;
import static uk.gov.pay.connector.gateway.worldpay.WorldpayOrderRequestBuilder.aWorldpayCancelOrderRequestBuilder;
import static uk.gov.pay.connector.gateway.worldpay.WorldpayOrderRequestBuilder.aWorldpayDeleteTokenOrderRequestBuilder;
import static uk.gov.pay.connector.gateway.worldpay.WorldpayOrderRequestBuilder.aWorldpayInquiryRequestBuilder;
import static uk.gov.pay.connector.gateway.worldpay.WorldpayOrderStatusResponse.WORLDPAY_RECURRING_AUTH_TOKEN_PAYMENT_TOKEN_ID_KEY;
import static uk.gov.pay.connector.gateway.worldpay.SendWorldpayExemptionRequest.DO_NOT_SEND_EXEMPTION_REQUEST;
import static uk.gov.pay.connector.gateway.worldpay.SendWorldpayExemptionRequest.SEND_CORPORATE_EXEMPTION_REQUEST;
import static uk.gov.pay.connector.gateway.worldpay.SendWorldpayExemptionRequest.SEND_EXEMPTION_ENGINE_REQUEST;
import static uk.gov.pay.connector.paymentprocessor.model.Exemption3ds.EXEMPTION_HONOURED;
import static uk.gov.pay.connector.paymentprocessor.model.Exemption3ds.EXEMPTION_NOT_REQUESTED;
import static uk.gov.pay.connector.paymentprocessor.model.Exemption3ds.EXEMPTION_OUT_OF_SCOPE;
Expand Down Expand Up @@ -102,6 +103,7 @@ public class WorldpayPaymentProvider implements PaymentProvider, WorldpayGateway
private final WorldpayRefundHandler worldpayRefundHandler;
private final WorldpayWalletAuthorisationHandler worldpayWalletAuthorisationHandler;
private final WorldpayAuthoriseHandler worldpayAuthoriseHandler;
private final RefundEntityFactory refundEntityFactory;
private final Map<String, URI> gatewayUrlMap;
private final AuthorisationService authorisationService;
private final AuthorisationLogger authorisationLogger;
Expand All @@ -119,6 +121,7 @@ public WorldpayPaymentProvider(@Named("WorldpayGatewayUrlMap") Map<String, URI>
WorldpayAuthoriseHandler worldpayAuthoriseHandler,
WorldpayCaptureHandler worldpayCaptureHandler,
WorldpayRefundHandler worldpayRefundHandler,
@Named("WorldpayRefundEntityFactory") RefundEntityFactory refundEntityFactory,
AuthorisationService authorisationService,
AuthorisationLogger authorisationLogger,
ChargeDao chargeDao,
Expand All @@ -134,6 +137,7 @@ public WorldpayPaymentProvider(@Named("WorldpayGatewayUrlMap") Map<String, URI>
this.worldpayRefundHandler = worldpayRefundHandler;
this.worldpayWalletAuthorisationHandler = worldpayWalletAuthorisationHandler;
this.worldpayAuthoriseHandler = worldpayAuthoriseHandler;
this.refundEntityFactory = refundEntityFactory;
this.authorisationService = authorisationService;
this.authorisationLogger = authorisationLogger;
this.chargeDao = chargeDao;
Expand Down Expand Up @@ -424,7 +428,10 @@ public WorldpayAuthorisationRequestSummary generateAuthorisationRequestSummary(G
return new WorldpayAuthorisationRequestSummary(gatewayAccount, authCardDetails, isSetUpAgreement);
}


@Override
public RefundEntityFactory getRefundEntityFactory() {
return refundEntityFactory;
}

private GatewayOrder build3dsResponseAuthOrder(Auth3dsResponseGatewayRequest request) {
return aWorldpay3dsResponseAuthOrderRequestBuilder()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package uk.gov.pay.connector.refund.service;

import uk.gov.pay.connector.refund.model.domain.RefundEntity;

public class DefaultRefundEntityFactory implements RefundEntityFactory {

@Override
public RefundEntity create(long amount, String userExternalId, String refundUserEmail, String chargeExternalId) {
return new RefundEntity(amount, userExternalId, refundUserEmail, chargeExternalId);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package uk.gov.pay.connector.refund.service;

import uk.gov.pay.connector.refund.model.domain.RefundEntity;

public interface RefundEntityFactory {

RefundEntity create(long amount, String userExternalId, String refundUserEmail, String chargeExternalId);

}
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,11 @@ public ChargeRefundResponse submitRefund(GatewayAccountEntity gatewayAccountEnti
GatewayAccountCredentialsEntity gatewayAccountCredentialsEntity = gatewayAccountCredentialsService.findCredentialFromCharge(charge, gatewayAccountEntity)
.orElseThrow(() -> new GatewayAccountCredentialsNotFoundException("Unable to find gateway account credentials to use to refund charge."));

RefundEntity refundEntity = createRefund(charge, gatewayAccountEntity, refundRequest);
var paymentProvider = providers.byName(PaymentGatewayName.valueFrom(charge.getPaymentGatewayName()));

GatewayRefundResponse gatewayRefundResponse = providers
.byName(PaymentGatewayName.valueFrom(charge.getPaymentGatewayName()))
RefundEntity refundEntity = createRefund(charge, gatewayAccountEntity, refundRequest, paymentProvider.getRefundEntityFactory());

GatewayRefundResponse gatewayRefundResponse = paymentProvider
.refund(RefundGatewayRequest.valueOf(charge, refundEntity, gatewayAccountEntity, gatewayAccountCredentialsEntity));

RefundStatus refundStatus = determineRefundStatus(gatewayRefundResponse);
Expand All @@ -111,10 +112,10 @@ public ChargeRefundResponse submitRefund(GatewayAccountEntity gatewayAccountEnti

@Transactional
@SuppressWarnings("WeakerAccess")
public RefundEntity createRefund(Charge charge, GatewayAccountEntity gatewayAccountEntity, RefundRequest refundRequest) {
public RefundEntity createRefund(Charge charge, GatewayAccountEntity gatewayAccountEntity, RefundRequest refundRequest, RefundEntityFactory refundEntityFactory) {
List<Refund> refundList = findRefunds(charge);
long availableAmount = validateRefundAndGetAvailableAmount(charge, gatewayAccountEntity, refundRequest, refundList);
RefundEntity refundEntity = createRefundEntity(refundRequest, gatewayAccountEntity, charge);
RefundEntity refundEntity = createRefundEntity(refundRequest, gatewayAccountEntity, charge, refundEntityFactory);

logger.info("Card refund request sent - charge_external_id={}, status={}, amount={}, transaction_id={}, account_id={}, operation_type=Refund, amount_available_refund={}, amount_requested_refund={}, provider={}, provider_type={}, user_external_id={}",
charge.getExternalId(),
Expand Down Expand Up @@ -184,8 +185,8 @@ private RefundStatus determineRefundStatus(GatewayRefundResponse gatewayRefundRe

@Transactional
@SuppressWarnings("WeakerAccess")
public RefundEntity createRefundEntity(RefundRequest refundRequest, GatewayAccountEntity gatewayAccountEntity, Charge charge) {
RefundEntity refundEntity = new RefundEntity(refundRequest.getAmount(),
public RefundEntity createRefundEntity(RefundRequest refundRequest, GatewayAccountEntity gatewayAccountEntity, Charge charge, RefundEntityFactory refundEntityFactory) {
var refundEntity = refundEntityFactory.create(refundRequest.getAmount(),
refundRequest.getUserExternalId(), refundRequest.getUserEmail(), charge.getExternalId());
transitionRefundState(refundEntity, gatewayAccountEntity, RefundStatus.CREATED, charge);
refundDao.persist(refundEntity);
Expand Down Expand Up @@ -253,9 +254,8 @@ private void checkIfChargeIsRefundableOrTerminate(Charge reloadedCharge, Externa

/**
* <p>Worldpay -> Worldpay doesn't return reference. We use our externalId because that's what we sent in the
* request as our reference and it will be sent by Worldpay with the notification.</p>
* <p>ePDQ -> We construct PAYID/PAYIDSUB and use that as the reference. PAYID and PAYIDSUB will be sent with the
* notification.</p>
* request as our reference and it will be sent by Worldpay with the notification. We should have already set
* this when the refund was created (see {@link uk.gov.pay.connector.refund.service.WorldpayRefundEntityFactory)}).</p>
* if not successful (and the fact that we have got a proper response from Gateway, we have to assume
* no refund has not gone through and no reference returned(or needed) to be stored.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package uk.gov.pay.connector.refund.service;

import uk.gov.pay.connector.refund.model.domain.RefundEntity;

public class WorldpayRefundEntityFactory implements RefundEntityFactory {

@Override
public RefundEntity create(long amount, String userExternalId, String refundUserEmail, String chargeExternalId) {
var refundEntity = new RefundEntity(amount, userExternalId, refundUserEmail, chargeExternalId);
// We set the Worldpay’s gateway transaction ID to be the same as the refund external ID
refundEntity.setGatewayTransactionId(refundEntity.getExternalId());
return refundEntity;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import uk.gov.pay.connector.gatewayaccountcredentials.model.GatewayAccountCredentialsEntity;
import uk.gov.pay.connector.model.domain.RefundEntityFixture;
import uk.gov.pay.connector.paymentinstrument.service.PaymentInstrumentService;
import uk.gov.pay.connector.refund.service.RefundEntityFactory;
import uk.gov.service.payments.commons.model.CardExpiryDate;

import java.time.Instant;
Expand Down Expand Up @@ -60,7 +61,7 @@ public class SandboxPaymentProviderTest {
void setup() {
chargeService = mock(ChargeService.class);
paymentInstrumentService = mock(PaymentInstrumentService.class);
provider = new SandboxPaymentProvider();
provider = new SandboxPaymentProvider(mock(RefundEntityFactory.class));
credentialsEntity = aGatewayAccountCredentialsEntity()
.withCredentials(Map.of())
.withPaymentProvider(SANDBOX.getName())
Expand Down
Loading

0 comments on commit 82ed499

Please sign in to comment.