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 9e12621..8a93a68 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
@@ -98,49 +98,67 @@ public class IncompletePaymentAttemptTask extends CompletionTaskBase<PaymentAtte
final PaymentTransactionModelDao transaction = Iterables.tryFind(transactions, new Predicate<PaymentTransactionModelDao>() {
@Override
public boolean apply(final PaymentTransactionModelDao input) {
- return input.getAttemptId().equals(attempt.getId()) &&
- input.getTransactionStatus() == TransactionStatus.SUCCESS;
+ return input.getAttemptId().equals(attempt.getId());
}
}).orNull();
- if (transaction == null) {
+ // UNKNOWN transaction are handled by the Janitor IncompletePaymentTransactionTask and should eventually transition to something else,
+ // at which point the attempt can also be transition to a different state.
+ if (transaction.getTransactionStatus() == TransactionStatus.UNKNOWN) {
+ return;
+ }
+
+ // In those 3 case 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 byt 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) {
log.info("Janitor AttemptCompletionTask moving attempt " + attempt.getId() + " -> ABORTED");
paymentDao.updatePaymentAttempt(attempt.getId(), attempt.getTransactionId(), "ABORTED", internalCallContext);
return;
}
- try {
- log.info("Janitor AttemptCompletionTask completing attempt " + attempt.getId() + " -> SUCCESS");
-
- final Account account = accountInternalApi.getAccountById(attempt.getAccountId(), tenantContext);
- final boolean isApiPayment = true; // unclear
- final PaymentStateControlContext paymentStateContext = new PaymentStateControlContext(attempt.toPaymentControlPluginNames(),
- isApiPayment,
- transaction.getPaymentId(),
- attempt.getPaymentExternalKey(),
- 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("Janitor AttemptCompletionTask failed to complete payment attempt " + attempt.getId(), e);
- } catch (final PluginPropertySerializerException e) {
- log.warn("Janitor AttemptCompletionTask failed to complete payment attempt " + attempt.getId(), e);
- } catch (final PaymentApiException e) {
- log.warn("Janitor AttemptCompletionTask failed to complete payment attempt " + attempt.getId(), e);
+ // 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("Janitor AttemptCompletionTask completing attempt " + attempt.getId() + " -> SUCCESS");
+
+ final Account account = accountInternalApi.getAccountById(attempt.getAccountId(), tenantContext);
+ final boolean isApiPayment = true; // unclear
+ final PaymentStateControlContext paymentStateContext = new PaymentStateControlContext(attempt.toPaymentControlPluginNames(),
+ isApiPayment,
+ transaction.getPaymentId(),
+ attempt.getPaymentExternalKey(),
+ 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("Janitor AttemptCompletionTask failed to complete payment attempt " + attempt.getId(), e);
+ } catch (final PluginPropertySerializerException e) {
+ log.warn("Janitor AttemptCompletionTask failed to complete payment attempt " + attempt.getId(), e);
+ } catch (final PaymentApiException e) {
+ log.warn("Janitor AttemptCompletionTask failed to complete payment attempt " + attempt.getId(), e);
+ }
}
}
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 1c2f87c..de7730a 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
@@ -32,7 +32,12 @@ import org.killbill.automaton.OperationResult;
import org.killbill.billing.account.api.Account;
import org.killbill.billing.callcontext.DefaultCallContext;
import org.killbill.billing.catalog.api.Currency;
+import org.killbill.billing.control.plugin.api.OnFailurePaymentControlResult;
import org.killbill.billing.control.plugin.api.OnSuccessPaymentControlResult;
+import org.killbill.billing.control.plugin.api.PaymentControlApiException;
+import org.killbill.billing.control.plugin.api.PaymentControlContext;
+import org.killbill.billing.control.plugin.api.PaymentControlPluginApi;
+import org.killbill.billing.control.plugin.api.PriorPaymentControlResult;
import org.killbill.billing.osgi.api.OSGIServiceRegistration;
import org.killbill.billing.payment.api.Payment;
import org.killbill.billing.payment.api.PaymentApiException;
@@ -46,11 +51,6 @@ import org.killbill.billing.payment.core.sm.OperationCallbackBase;
import org.killbill.billing.payment.core.sm.PaymentStateContext;
import org.killbill.billing.payment.dispatcher.PluginDispatcher;
import org.killbill.billing.payment.dispatcher.PluginDispatcher.PluginDispatcherReturnType;
-import org.killbill.billing.control.plugin.api.OnFailurePaymentControlResult;
-import org.killbill.billing.control.plugin.api.PaymentControlApiException;
-import org.killbill.billing.control.plugin.api.PaymentControlContext;
-import org.killbill.billing.control.plugin.api.PaymentControlPluginApi;
-import org.killbill.billing.control.plugin.api.PriorPaymentControlResult;
import org.killbill.billing.payment.retry.DefaultPriorPaymentControlResult;
import org.killbill.billing.util.callcontext.CallContext;
import org.killbill.commons.locker.GlobalLocker;
@@ -229,8 +229,11 @@ public abstract class OperationControlCallback extends OperationCallbackBase<Pay
if (result.getAdjustedPluginProperties() != null) {
inputPluginProperties = result.getAdjustedPluginProperties();
}
+ // Exceptions form the control plugins are ignored (and logged) because the semantics on what to do are undefined.
} catch (final PaymentControlApiException e) {
logger.warn("Plugin " + pluginName + " failed to complete executePluginOnSuccessCalls call for " + paymentControlContext.getPaymentExternalKey(), e);
+ } catch (final RuntimeException e) {
+ logger.warn("Plugin " + pluginName + " failed to complete executePluginOnSuccessCalls call for " + paymentControlContext.getPaymentExternalKey(), e);
}
}
}
