killbill-aplcache
Changes
payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentAttemptTask.java 85(+35 -50)
payment/src/main/java/org/killbill/billing/payment/core/sm/control/CompletionControlOperation.java 28(+23 -5)
Details
diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationBase.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationBase.java
index 0fdb3c7..7697a94 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationBase.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestIntegrationBase.java
@@ -86,6 +86,7 @@ import org.killbill.billing.overdue.caching.OverdueConfigCache;
import org.killbill.billing.overdue.listener.OverdueListener;
import org.killbill.billing.overdue.wrapper.OverdueWrapper;
import org.killbill.billing.overdue.wrapper.OverdueWrapperFactory;
+import org.killbill.billing.payment.api.AdminPaymentApi;
import org.killbill.billing.payment.api.Payment;
import org.killbill.billing.payment.api.PaymentApi;
import org.killbill.billing.payment.api.PaymentApiException;
@@ -208,6 +209,9 @@ public class TestIntegrationBase extends BeatrixTestSuiteWithEmbeddedDB {
protected PaymentApi paymentApi;
@Inject
+ protected AdminPaymentApi adminPaymentApi;
+
+ @Inject
protected EntitlementApi entitlementApi;
@Inject
diff --git a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestInvoicePayment.java b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestInvoicePayment.java
index 6a86dfb..9af4e66 100644
--- a/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestInvoicePayment.java
+++ b/beatrix/src/test/java/org/killbill/billing/beatrix/integration/TestInvoicePayment.java
@@ -786,6 +786,111 @@ public class TestInvoicePayment extends TestIntegrationBase {
}
@Test(groups = "slow")
+ public void testWithSuccessfulPaymentFixedToFailure() throws Exception {
+ // Verify integration with Overdue in that particular test
+ final String configXml = "<overdueConfig>" +
+ " <accountOverdueStates>" +
+ " <initialReevaluationInterval>" +
+ " <unit>DAYS</unit><number>1</number>" +
+ " </initialReevaluationInterval>" +
+ " <state name=\"OD1\">" +
+ " <condition>" +
+ " <timeSinceEarliestUnpaidInvoiceEqualsOrExceeds>" +
+ " <unit>DAYS</unit><number>1</number>" +
+ " </timeSinceEarliestUnpaidInvoiceEqualsOrExceeds>" +
+ " </condition>" +
+ " <externalMessage>Reached OD1</externalMessage>" +
+ " <blockChanges>true</blockChanges>" +
+ " <disableEntitlementAndChangesBlocked>false</disableEntitlementAndChangesBlocked>" +
+ " </state>" +
+ " </accountOverdueStates>" +
+ "</overdueConfig>";
+ final InputStream is = new ByteArrayInputStream(configXml.getBytes());
+ final DefaultOverdueConfig config = XMLLoader.getObjectFromStreamNoValidation(is, DefaultOverdueConfig.class);
+ overdueConfigCache.loadDefaultOverdueConfig(config);
+
+ clock.setDay(new LocalDate(2012, 4, 1));
+
+ final AccountData accountData = getAccountData(1);
+ final Account account = createAccountWithNonOsgiPaymentMethod(accountData);
+ accountChecker.checkAccount(account.getId(), accountData, callContext);
+
+ final DefaultEntitlement baseEntitlement = createBaseEntitlementAndCheckForCompletion(account.getId(), "bundleKey", "Shotgun", ProductCategory.BASE, BillingPeriod.MONTHLY, NextEvent.CREATE, NextEvent.BLOCK, NextEvent.INVOICE);
+
+ addDaysAndCheckForCompletion(30, NextEvent.PHASE, NextEvent.INVOICE, NextEvent.PAYMENT, NextEvent.INVOICE_PAYMENT);
+
+ checkODState(OverdueWrapper.CLEAR_STATE_NAME, account.getId());
+
+ invoiceChecker.checkChargedThroughDate(baseEntitlement.getId(), new LocalDate(2012, 6, 1), callContext);
+
+ final List<Invoice> invoices = invoiceUserApi.getInvoicesByAccount(account.getId(), false, callContext);
+ assertEquals(invoices.size(), 2);
+
+ final Invoice invoice1 = invoices.get(0).getInvoiceItems().get(0).getInvoiceItemType() == InvoiceItemType.RECURRING ?
+ invoices.get(0) : invoices.get(1);
+ assertTrue(invoice1.getBalance().compareTo(BigDecimal.ZERO) == 0);
+ assertTrue(invoice1.getPaidAmount().compareTo(new BigDecimal("249.95")) == 0);
+ assertTrue(invoice1.getChargedAmount().compareTo(new BigDecimal("249.95")) == 0);
+ assertEquals(invoice1.getPayments().size(), 1);
+ assertEquals(invoice1.getPayments().get(0).getAmount().compareTo(new BigDecimal("249.95")), 0);
+ assertEquals(invoice1.getPayments().get(0).getCurrency(), Currency.USD);
+ assertTrue(invoice1.getPayments().get(0).isSuccess());
+ assertNotNull(invoice1.getPayments().get(0).getPaymentId());
+
+ final BigDecimal accountBalance1 = invoiceUserApi.getAccountBalance(account.getId(), callContext);
+ assertTrue(accountBalance1.compareTo(BigDecimal.ZERO) == 0);
+
+ final List<Payment> payments = paymentApi.getAccountPayments(account.getId(), false, true, ImmutableList.<PluginProperty>of(), callContext);
+ assertEquals(payments.size(), 1);
+ assertEquals(payments.get(0).getPurchasedAmount().compareTo(new BigDecimal("249.95")), 0);
+ assertEquals(payments.get(0).getTransactions().size(), 1);
+ assertEquals(payments.get(0).getTransactions().get(0).getAmount().compareTo(new BigDecimal("249.95")), 0);
+ assertEquals(payments.get(0).getTransactions().get(0).getCurrency(), Currency.USD);
+ assertEquals(payments.get(0).getTransactions().get(0).getProcessedAmount().compareTo(new BigDecimal("249.95")), 0);
+ assertEquals(payments.get(0).getTransactions().get(0).getProcessedCurrency(), Currency.USD);
+ assertEquals(payments.get(0).getTransactions().get(0).getTransactionStatus(), TransactionStatus.SUCCESS);
+ assertEquals(payments.get(0).getPaymentAttempts().size(), 1);
+ assertEquals(payments.get(0).getPaymentAttempts().get(0).getPluginName(), InvoicePaymentControlPluginApi.PLUGIN_NAME);
+ assertEquals(payments.get(0).getPaymentAttempts().get(0).getStateName(), "SUCCESS");
+
+ // Transition the payment to failure
+ busHandler.pushExpectedEvents(NextEvent.PAYMENT_ERROR, NextEvent.INVOICE_PAYMENT_ERROR);
+ adminPaymentApi.fixPaymentTransactionState(payments.get(0), payments.get(0).getTransactions().get(0), TransactionStatus.PAYMENT_FAILURE, null, null, ImmutableList.<PluginProperty>of(), callContext);
+ assertListenerStatus();
+
+ final Invoice invoice2 = invoiceUserApi.getInvoice(invoice1.getId(), callContext);
+ assertTrue(invoice2.getBalance().compareTo(new BigDecimal("249.95")) == 0);
+ assertTrue(invoice2.getPaidAmount().compareTo(BigDecimal.ZERO) == 0);
+ assertTrue(invoice2.getChargedAmount().compareTo(new BigDecimal("249.95")) == 0);
+ assertEquals(invoice2.getPayments().size(), 1);
+ assertEquals(invoice2.getPayments().get(0).getAmount().compareTo(BigDecimal.ZERO), 0);
+ assertEquals(invoice2.getPayments().get(0).getCurrency(), Currency.USD);
+ assertFalse(invoice2.getPayments().get(0).isSuccess());
+ assertNotNull(invoice2.getPayments().get(0).getPaymentId());
+
+ final BigDecimal accountBalance2 = invoiceUserApi.getAccountBalance(account.getId(), callContext);
+ assertTrue(accountBalance2.compareTo(new BigDecimal("249.95")) == 0);
+
+ final List<Payment> payments2 = paymentApi.getAccountPayments(account.getId(), false, true, ImmutableList.<PluginProperty>of(), callContext);
+ assertEquals(payments2.size(), 1);
+ assertEquals(payments2.get(0).getPurchasedAmount().compareTo(BigDecimal.ZERO), 0);
+ assertEquals(payments2.get(0).getTransactions().size(), 1);
+ assertEquals(payments2.get(0).getTransactions().get(0).getAmount().compareTo(new BigDecimal("249.95")), 0);
+ assertEquals(payments2.get(0).getTransactions().get(0).getCurrency(), Currency.USD);
+ assertEquals(payments2.get(0).getTransactions().get(0).getProcessedAmount().compareTo(new BigDecimal("249.95")), 0);
+ assertEquals(payments2.get(0).getTransactions().get(0).getProcessedCurrency(), Currency.USD);
+ assertEquals(payments2.get(0).getTransactions().get(0).getTransactionStatus(), TransactionStatus.PAYMENT_FAILURE);
+ assertEquals(payments2.get(0).getPaymentAttempts().size(), 1);
+ assertEquals(payments2.get(0).getPaymentAttempts().get(0).getPluginName(), InvoicePaymentControlPluginApi.PLUGIN_NAME);
+ // Note that because fixPaymentTransactionState is considered an API call, not retry will be attempted
+ assertEquals(payments2.get(0).getPaymentAttempts().get(0).getStateName(), "ABORTED");
+
+ // Verify account transitions to OD1
+ addDaysAndCheckForCompletion(2, NextEvent.BLOCK);
+ checkODState("OD1", account.getId());
+ }
+
+ @Test(groups = "slow")
public void testWithIncompletePaymentAttempt() throws Exception {
// 2012-05-01T00:03:42.000Z
clock.setTime(new DateTime(2012, 5, 1, 0, 3, 42, 0));
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/dao/DefaultInvoiceDao.java b/invoice/src/main/java/org/killbill/billing/invoice/dao/DefaultInvoiceDao.java
index d0ca343..c452e53 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/dao/DefaultInvoiceDao.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/dao/DefaultInvoiceDao.java
@@ -763,7 +763,7 @@ public class DefaultInvoiceDao extends EntityDaoBase<InvoiceModelDao, Invoice, I
if (existingAttempt == null) {
transactional.create(invoicePayment, context);
- } else if (!existingAttempt.getSuccess()) {
+ } else {
transactional.updateAttempt(existingAttempt.getRecordId(),
invoicePayment.getPaymentId().toString(),
invoicePayment.getPaymentDate().toDate(),
diff --git a/payment/src/main/java/org/killbill/billing/payment/api/DefaultAdminPaymentApi.java b/payment/src/main/java/org/killbill/billing/payment/api/DefaultAdminPaymentApi.java
index 18f1feb..9ebad50 100644
--- a/payment/src/main/java/org/killbill/billing/payment/api/DefaultAdminPaymentApi.java
+++ b/payment/src/main/java/org/killbill/billing/payment/api/DefaultAdminPaymentApi.java
@@ -17,30 +17,40 @@
package org.killbill.billing.payment.api;
+import java.util.List;
+
import javax.annotation.Nullable;
import javax.inject.Inject;
import org.killbill.billing.callcontext.InternalCallContext;
import org.killbill.billing.payment.core.PaymentTransactionInfoPluginConverter;
+import org.killbill.billing.payment.core.janitor.IncompletePaymentAttemptTask;
import org.killbill.billing.payment.core.sm.PaymentStateMachineHelper;
+import org.killbill.billing.payment.dao.PaymentAttemptModelDao;
import org.killbill.billing.payment.dao.PaymentDao;
import org.killbill.billing.util.callcontext.CallContext;
import org.killbill.billing.util.callcontext.InternalCallContextFactory;
import org.killbill.billing.util.config.definition.PaymentConfig;
+import com.google.common.base.Predicate;
+import com.google.common.collect.Iterables;
+
public class DefaultAdminPaymentApi extends DefaultApiBase implements AdminPaymentApi {
private final PaymentStateMachineHelper paymentSMHelper;
+ private final IncompletePaymentAttemptTask incompletePaymentAttemptTask;
private final PaymentDao paymentDao;
private final InternalCallContextFactory internalCallContextFactory;
@Inject
public DefaultAdminPaymentApi(final PaymentConfig paymentConfig,
final PaymentStateMachineHelper paymentSMHelper,
+ final IncompletePaymentAttemptTask incompletePaymentAttemptTask,
final PaymentDao paymentDao,
final InternalCallContextFactory internalCallContextFactory) {
super(paymentConfig, internalCallContextFactory);
this.paymentSMHelper = paymentSMHelper;
+ this.incompletePaymentAttemptTask = incompletePaymentAttemptTask;
this.paymentDao = paymentDao;
this.internalCallContextFactory = internalCallContextFactory;
}
@@ -111,5 +121,20 @@ public class DefaultAdminPaymentApi extends DefaultApiBase implements AdminPayme
paymentTransaction.getGatewayErrorCode(),
paymentTransaction.getGatewayErrorMsg(),
internalCallContext);
+
+ // If there is a payment attempt associated with that transaction, we need to update it as well
+ final List<PaymentAttemptModelDao> paymentAttemptsModelDao = paymentDao.getPaymentAttemptByTransactionExternalKey(paymentTransaction.getExternalKey(), internalCallContext);
+ final PaymentAttemptModelDao paymentAttemptModelDao = Iterables.<PaymentAttemptModelDao>tryFind(paymentAttemptsModelDao,
+ new Predicate<PaymentAttemptModelDao>() {
+ @Override
+ public boolean apply(final PaymentAttemptModelDao input) {
+ return paymentTransaction.getId().equals(input.getTransactionId());
+ }
+ }).orNull();
+ if (paymentAttemptModelDao != null) {
+ // We can re-use the logic from IncompletePaymentAttemptTask as it is doing very similar work (i.e. run the completion part of
+ // the state machine to call the plugins and update the attempt in the right terminal state)
+ incompletePaymentAttemptTask.doIteration(paymentAttemptModelDao);
+ }
}
}
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentAttemptTask.java b/payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentAttemptTask.java
index 2aa31ed..568ec94 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentAttemptTask.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentAttemptTask.java
@@ -117,17 +117,7 @@ public class IncompletePaymentAttemptTask extends CompletionTaskBase<PaymentAtte
log.warn("Found {} transactions for paymentAttempt {}", filteredTransactions.size(), attempt.getId());
}
final PaymentTransactionModelDao transaction = filteredTransactions.isEmpty() ? null : filteredTransactions.get(0);
-
-
- // In those 3 cases (null transaction, PLUGIN_FAILURE and PAYMENT_FAILURE), we are taking a *shortcut* but this is incorrect; ideally we should call back the priorCall
- // control plugins to decide what to do:
- // * For null transaction and PLUGIN_FAILURE something went wrong before we could even make the payment, so possibly we should inform the control plugin
- // and retry
- // * For PAYMENT_FAILURE, the payment went through but was denied by the gateway, and so this is a different case where a control plugin may want to retry
- //
- if (transaction == null ||
- transaction.getTransactionStatus() == TransactionStatus.PLUGIN_FAILURE ||
- transaction.getTransactionStatus() == TransactionStatus.PAYMENT_FAILURE) {
+ if (transaction == null) {
log.info("Moving attemptId='{}' to ABORTED", attempt.getId());
paymentDao.updatePaymentAttempt(attempt.getId(), attempt.getTransactionId(), "ABORTED", internalCallContext);
return;
@@ -139,45 +129,40 @@ public class IncompletePaymentAttemptTask extends CompletionTaskBase<PaymentAtte
return;
}
- // On SUCCESS, PENDING state we complete the payment control state machine, allowing to call the control plugin onSuccessCall API.
- if (transaction.getTransactionStatus() == TransactionStatus.SUCCESS ||
- transaction.getTransactionStatus() == TransactionStatus.PENDING) {
-
- try {
- log.info("Moving attemptId='{}' to SUCCESS", attempt.getId());
-
- final Account account = accountInternalApi.getAccountById(attempt.getAccountId(), tenantContext);
- final boolean isApiPayment = true; // unclear
- final PaymentStateControlContext paymentStateContext = new PaymentStateControlContext(attempt.toPaymentControlPluginNames(),
- isApiPayment,
- null,
- transaction.getPaymentId(),
- attempt.getPaymentExternalKey(),
- transaction.getId(),
- transaction.getTransactionExternalKey(),
- transaction.getTransactionType(),
- account,
- attempt.getPaymentMethodId(),
- transaction.getAmount(),
- transaction.getCurrency(),
- PluginPropertySerializer.deserialize(attempt.getPluginProperties()),
- internalCallContext,
- callContext);
-
- paymentStateContext.setAttemptId(attempt.getId()); // Normally set by leavingState Callback
- paymentStateContext.setPaymentTransactionModelDao(transaction); // Normally set by raw state machine
- //
- // Will rerun the state machine with special callbacks to only make the executePluginOnSuccessCalls call
- // to the PaymentControlPluginApi plugin and transition the state.
- //
- pluginControlledPaymentAutomatonRunner.completeRun(paymentStateContext);
- } catch (final AccountApiException e) {
- log.warn("Error completing paymentAttemptId='{}'", attempt.getId(), e);
- } catch (final PluginPropertySerializerException e) {
- log.warn("Error completing paymentAttemptId='{}'", attempt.getId(), e);
- } catch (final PaymentApiException e) {
- log.warn("Error completing paymentAttemptId='{}'", attempt.getId(), e);
- }
+ try {
+ log.info("Completing attemptId='{}'", attempt.getId());
+
+ final Account account = accountInternalApi.getAccountById(attempt.getAccountId(), tenantContext);
+ final boolean isApiPayment = true; // unclear
+ final PaymentStateControlContext paymentStateContext = new PaymentStateControlContext(attempt.toPaymentControlPluginNames(),
+ isApiPayment,
+ null,
+ transaction.getPaymentId(),
+ attempt.getPaymentExternalKey(),
+ transaction.getId(),
+ transaction.getTransactionExternalKey(),
+ transaction.getTransactionType(),
+ account,
+ attempt.getPaymentMethodId(),
+ transaction.getAmount(),
+ transaction.getCurrency(),
+ PluginPropertySerializer.deserialize(attempt.getPluginProperties()),
+ internalCallContext,
+ callContext);
+
+ paymentStateContext.setAttemptId(attempt.getId()); // Normally set by leavingState Callback
+ paymentStateContext.setPaymentTransactionModelDao(transaction); // Normally set by raw state machine
+ //
+ // Will rerun the state machine with special callbacks to only make the executePluginOnSuccessCalls / executePluginOnFailureCalls calls
+ // to the PaymentControlPluginApi plugin and transition the state.
+ //
+ pluginControlledPaymentAutomatonRunner.completeRun(paymentStateContext);
+ } catch (final AccountApiException e) {
+ log.warn("Error completing paymentAttemptId='{}'", attempt.getId(), e);
+ } catch (final PluginPropertySerializerException e) {
+ log.warn("Error completing paymentAttemptId='{}'", attempt.getId(), e);
+ } catch (final PaymentApiException e) {
+ log.warn("Error completing paymentAttemptId='{}'", attempt.getId(), e);
}
}
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/control/CompletionControlOperation.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/control/CompletionControlOperation.java
index fd8eed7..17b2af3 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/control/CompletionControlOperation.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/control/CompletionControlOperation.java
@@ -22,19 +22,22 @@ import java.util.List;
import org.killbill.automaton.OperationException;
import org.killbill.automaton.OperationResult;
import org.killbill.billing.control.plugin.api.PaymentApiType;
+import org.killbill.billing.control.plugin.api.PaymentControlContext;
import org.killbill.billing.payment.api.Payment;
import org.killbill.billing.payment.api.PaymentApiException;
+import org.killbill.billing.payment.api.PluginProperty;
+import org.killbill.billing.payment.api.TransactionStatus;
import org.killbill.billing.payment.core.PaymentProcessor;
import org.killbill.billing.payment.core.ProcessorBase.DispatcherCallback;
import org.killbill.billing.payment.core.sm.control.ControlPluginRunner.DefaultPaymentControlContext;
import org.killbill.billing.payment.dao.PaymentTransactionModelDao;
import org.killbill.billing.payment.dispatcher.PluginDispatcher;
import org.killbill.billing.payment.dispatcher.PluginDispatcher.PluginDispatcherReturnType;
-import org.killbill.billing.control.plugin.api.PaymentControlContext;
import org.killbill.billing.util.config.definition.PaymentConfig;
import org.killbill.commons.locker.GlobalLocker;
import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
//
// Used from AttemptCompletionTask to resume an incomplete payment that went through control API.
@@ -54,11 +57,11 @@ public class CompletionControlOperation extends OperationControlCallback {
@Override
public OperationResult doOperationCallback() throws OperationException {
-
final List<String> controlPluginNameList = paymentStateControlContext.getPaymentControlPluginNames();
final String controlPluginNames = JOINER.join(controlPluginNameList);
return dispatchWithAccountLockAndTimeout(controlPluginNames, new DispatcherCallback<PluginDispatcherReturnType<OperationResult>, OperationException>() {
+
@Override
public PluginDispatcherReturnType<OperationResult> doOperation() throws OperationException {
final PaymentTransactionModelDao transaction = paymentStateContext.getPaymentTransactionModelDao();
@@ -78,15 +81,30 @@ public class CompletionControlOperation extends OperationControlCallback {
transaction.getProcessedCurrency(),
paymentStateControlContext.isApiPayment(),
paymentStateContext.getCallContext());
+ try {
+ final Payment result = doCallSpecificOperationCallback();
+ ((PaymentStateControlContext) paymentStateContext).setResult(result);
- executePluginOnSuccessCalls(paymentStateControlContext.getPaymentControlPluginNames(), updatedPaymentControlContext);
- return PluginDispatcher.createPluginDispatcherReturnType(OperationResult.SUCCESS);
+ final boolean success = transaction.getTransactionStatus() == TransactionStatus.SUCCESS || transaction.getTransactionStatus() == TransactionStatus.PENDING;
+ if (success) {
+ executePluginOnSuccessCalls(paymentStateControlContext.getPaymentControlPluginNames(), updatedPaymentControlContext);
+ return PluginDispatcher.createPluginDispatcherReturnType(OperationResult.SUCCESS);
+ } else {
+ throw new OperationException(null, executePluginOnFailureCallsAndSetRetryDate(updatedPaymentControlContext));
+ }
+ } catch (final PaymentApiException e) {
+ // Wrap PaymentApiException, and throw a new OperationException with an ABORTED/FAILURE state based on the retry result.
+ throw new OperationException(e, executePluginOnFailureCallsAndSetRetryDate(updatedPaymentControlContext));
+ } catch (final RuntimeException e) {
+ // Attempts to set the retry date in context if needed.
+ throw new OperationException(e, executePluginOnFailureCallsAndSetRetryDate(updatedPaymentControlContext));
+ }
}
});
}
@Override
protected Payment doCallSpecificOperationCallback() throws PaymentApiException {
- return null;
+ return paymentProcessor.getPayment(paymentStateContext.getPaymentId(), false, false, ImmutableList.<PluginProperty>of(), paymentStateContext.getCallContext(), paymentStateContext.getInternalCallContext());
}
}
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/control/OperationControlCallback.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/control/OperationControlCallback.java
index 4f05018..b8fc059 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/control/OperationControlCallback.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/control/OperationControlCallback.java
@@ -212,7 +212,7 @@ public abstract class OperationControlCallback extends OperationCallbackBase<Pay
adjustStateContextPluginProperties(paymentStateContext, result.getAdjustedPluginProperties());
}
- private OperationResult executePluginOnFailureCallsAndSetRetryDate(final PaymentControlContext paymentControlContext) {
+ protected OperationResult executePluginOnFailureCallsAndSetRetryDate(final PaymentControlContext paymentControlContext) {
final DateTime retryDate = executePluginOnFailureCalls(paymentStateControlContext.getPaymentControlPluginNames(), paymentControlContext);
if (retryDate != null) {
((PaymentStateControlContext) paymentStateContext).setRetryDate(retryDate);
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/PluginControlPaymentAutomatonRunner.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/PluginControlPaymentAutomatonRunner.java
index aa05dfa..d4b2b39 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/PluginControlPaymentAutomatonRunner.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/PluginControlPaymentAutomatonRunner.java
@@ -274,14 +274,17 @@ public class PluginControlPaymentAutomatonRunner extends PaymentAutomatonRunner
} catch (final MissingEntryException e) {
throw new PaymentApiException(e.getCause(), ErrorCode.PAYMENT_INTERNAL_ERROR, Objects.firstNonNull(e.getMessage(), ""));
} catch (final OperationException e) {
- if (e.getCause() == null) {
- throw new PaymentApiException(e, ErrorCode.PAYMENT_INTERNAL_ERROR, Objects.firstNonNull(e.getMessage(), ""));
- } else if (e.getCause() instanceof PaymentApiException) {
+ if (e.getCause() instanceof PaymentApiException) {
throw (PaymentApiException) e.getCause();
- } else {
- throw new PaymentApiException(e.getCause(), ErrorCode.PAYMENT_INTERNAL_ERROR, Objects.firstNonNull(e.getMessage(), ""));
+ // If the control plugin tries to pass us back a PaymentApiException we throw it
+ } else if (e.getCause() instanceof PaymentControlApiException && e.getCause().getCause() instanceof PaymentApiException) {
+ throw (PaymentApiException) e.getCause().getCause();
+ } else if (e.getCause() != null || paymentStateContext.getResult() == null) {
+ throw new PaymentApiException(e.getCause(), ErrorCode.PAYMENT_INTERNAL_ERROR, MoreObjects.firstNonNull(e.getMessage(), ""));
}
}
+ // If the result is set (and cause is null), that means we created a Payment but the associated transaction status is 'XXX_FAILURE',
+ // we don't throw, and return the failed Payment instead to be consistent with what happens when we don't go through control api.
return paymentStateContext.getResult();
}