killbill-memoizeit
Changes
payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentTransactionTask.java 50(+31 -19)
payment/src/main/java/org/killbill/billing/payment/core/PaymentTransactionInfoPluginConverter.java 26(+12 -14)
payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentOperation.java 57(+22 -35)
payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentRoutingPluginApi.java 3(+2 -1)
payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentEnteringStateCallback.java 12(+0 -12)
pom.xml 2(+1 -1)
Details
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentTransactionTask.java b/payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentTransactionTask.java
index 4edf27f..ede05e3 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentTransactionTask.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentTransactionTask.java
@@ -31,16 +31,18 @@ import org.killbill.billing.osgi.api.OSGIServiceRegistration;
import org.killbill.billing.payment.api.PluginProperty;
import org.killbill.billing.payment.api.TransactionStatus;
import org.killbill.billing.payment.core.PaymentTransactionInfoPluginConverter;
+import org.killbill.billing.payment.core.sm.PaymentControlStateMachineHelper;
import org.killbill.billing.payment.core.sm.PaymentStateMachineHelper;
import org.killbill.billing.payment.core.sm.PluginRoutingPaymentAutomatonRunner;
-import org.killbill.billing.payment.core.sm.PaymentControlStateMachineHelper;
import org.killbill.billing.payment.dao.PaymentDao;
import org.killbill.billing.payment.dao.PaymentMethodModelDao;
import org.killbill.billing.payment.dao.PaymentModelDao;
import org.killbill.billing.payment.dao.PaymentTransactionModelDao;
import org.killbill.billing.payment.plugin.api.PaymentPluginApi;
import org.killbill.billing.payment.plugin.api.PaymentPluginApiException;
+import org.killbill.billing.payment.plugin.api.PaymentPluginStatus;
import org.killbill.billing.payment.plugin.api.PaymentTransactionInfoPlugin;
+import org.killbill.billing.payment.provider.DefaultNoOpPaymentInfoPlugin;
import org.killbill.billing.util.callcontext.CallContext;
import org.killbill.billing.util.callcontext.InternalCallContextFactory;
import org.killbill.billing.util.config.PaymentConfig;
@@ -48,6 +50,7 @@ import org.killbill.clock.Clock;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
+import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
@@ -59,7 +62,6 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
.build();
private final static int MAX_ITEMS_PER_LOOP = 100;
-
public IncompletePaymentTransactionTask(final Janitor janitor, final InternalCallContextFactory internalCallContextFactory, final PaymentConfig paymentConfig,
final PaymentDao paymentDao, final Clock clock,
final PaymentStateMachineHelper paymentStateMachineHelper, final PaymentControlStateMachineHelper retrySMHelper, final AccountInternalApi accountInternalApi,
@@ -71,7 +73,7 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
public List<PaymentTransactionModelDao> getItemsForIteration() {
final List<PaymentTransactionModelDao> result = paymentDao.getByTransactionStatusAcrossTenants(TRANSACTION_STATUSES_TO_CONSIDER, getCreatedDateBefore(), getCreatedDateAfter(), MAX_ITEMS_PER_LOOP);
if (!result.isEmpty()) {
- log.info("Janitor IncompletePaymentTransactionTask start run: found {} errored/unknown payments", result.size());
+ log.info("Janitor IncompletePaymentTransactionTask start run: found {} pending/unknown payments", result.size());
}
return result;
}
@@ -83,10 +85,19 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
final CallContext callContext = createCallContext("IncompletePaymentTransactionTask", internalTenantContext);
final PaymentModelDao payment = paymentDao.getPayment(paymentTransaction.getPaymentId(), internalTenantContext);
-
final PaymentMethodModelDao paymentMethod = paymentDao.getPaymentMethod(payment.getPaymentMethodId(), internalTenantContext);
final PaymentPluginApi paymentPluginApi = getPaymentPluginApi(payment, paymentMethod.getPluginName());
+
+ final PaymentTransactionInfoPlugin undefinedPaymentTransaction = new DefaultNoOpPaymentInfoPlugin(payment.getId(),
+ paymentTransaction.getId(),
+ paymentTransaction.getTransactionType(),
+ paymentTransaction.getAmount(),
+ paymentTransaction.getCurrency(),
+ paymentTransaction.getCreatedDate(),
+ paymentTransaction.getCreatedDate(),
+ PaymentPluginStatus.UNDEFINED,
+ null);
PaymentTransactionInfoPlugin paymentTransactionInfoPlugin;
try {
final List<PaymentTransactionInfoPlugin> result = paymentPluginApi.getPaymentInfo(payment.getAccountId(), payment.getId(), ImmutableList.<PluginProperty>of(), callContext);
@@ -95,10 +106,14 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
public boolean apply(final PaymentTransactionInfoPlugin input) {
return input.getKbTransactionPaymentId().equals(paymentTransaction.getId());
}
- }).orNull();
- } catch (final PaymentPluginApiException ignored) {
- // The paymentTransactionInfoPlugin will be null and handled below as if the plugin did not know about that transaction
- paymentTransactionInfoPlugin = null;
+ }).or(new Supplier<PaymentTransactionInfoPlugin>() {
+ @Override
+ public PaymentTransactionInfoPlugin get() {
+ return undefinedPaymentTransaction;
+ }
+ });
+ } catch (final Exception e) {
+ paymentTransactionInfoPlugin = undefinedPaymentTransaction;
}
//
@@ -120,9 +135,10 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
case PLUGIN_FAILURE:
case UNKNOWN:
default:
- // In this case we don't try to update the current paymentState (since we could not get anything interesting from the plugin)
- newPaymentState = payment.getStateName();
- break;
+ log.info("Janitor IncompletePaymentTransactionTask repairing payment {}, transaction {}, bail early...",
+ new Object[]{payment.getId(), paymentTransaction.getId(), paymentTransaction.getTransactionStatus(), transactionStatus});
+ // We can't get anything interesting from the plugin...
+ return;
}
// Recompute new lastSuccessPaymentState. This is important to be able to allow new operations on the state machine (for e.g an AUTH_SUCCESS would now allow a CAPTURE operation)
@@ -138,7 +154,8 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
final String gatewayErrorCode = paymentTransactionInfoPlugin != null ? paymentTransactionInfoPlugin.getGatewayErrorCode() : paymentTransaction.getGatewayErrorCode();
final String gatewayError = paymentTransactionInfoPlugin != null ? paymentTransactionInfoPlugin.getGatewayError() : paymentTransaction.getGatewayErrorMsg();
- log.info("Janitor IncompletePaymentTransactionTask repairing payment {}, transaction {}", payment.getId(), paymentTransaction.getId());
+ log.info("Janitor IncompletePaymentTransactionTask repairing payment {}, transaction {}, transitioning transactionStatus from {} -> {}",
+ new Object[]{payment.getId(), paymentTransaction.getId(), paymentTransaction.getTransactionStatus(), transactionStatus});
final InternalCallContext internalCallContext = internalCallContextFactory.createInternalCallContext(payment.getAccountId(), callContext);
paymentDao.updatePaymentAndTransactionOnCompletion(payment.getAccountId(), payment.getId(), paymentTransaction.getTransactionType(), newPaymentState, lastSuccessPaymentState,
@@ -147,13 +164,9 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
}
// Keep the existing currentTransactionStatus if we can't obtain a better answer from the plugin; if not, return the newTransactionStatus
- private TransactionStatus computeNewTransactionStatusFromPaymentTransactionInfoPlugin(@Nullable PaymentTransactionInfoPlugin input, final TransactionStatus currentTransactionStatus) {
+ private TransactionStatus computeNewTransactionStatusFromPaymentTransactionInfoPlugin(PaymentTransactionInfoPlugin input, final TransactionStatus currentTransactionStatus) {
final TransactionStatus newTransactionStatus = PaymentTransactionInfoPluginConverter.toTransactionStatus(input);
- if (newTransactionStatus == TransactionStatus.PLUGIN_FAILURE || newTransactionStatus == TransactionStatus.UNKNOWN) {
- return currentTransactionStatus;
- } else {
- return newTransactionStatus;
- }
+ return (newTransactionStatus != TransactionStatus.UNKNOWN) ? newTransactionStatus : currentTransactionStatus;
}
private boolean isPendingOrFinalTransactionStatus(final TransactionStatus transactionStatus) {
@@ -173,7 +186,6 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
return clock.getUTCNow().minusMillis((int) delayBeforeNowMs);
}
-
private DateTime getCreatedDateAfter() {
final long delayBeforeNowMs = paymentConfig.getIncompleteTransactionsTimeSpanGiveup().getMillis();
return clock.getUTCNow().minusMillis((int) delayBeforeNowMs);
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/PaymentTransactionInfoPluginConverter.java b/payment/src/main/java/org/killbill/billing/payment/core/PaymentTransactionInfoPluginConverter.java
index d60d540..bcdc703 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/PaymentTransactionInfoPluginConverter.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/PaymentTransactionInfoPluginConverter.java
@@ -28,34 +28,32 @@ import org.killbill.billing.payment.plugin.api.PaymentTransactionInfoPlugin;
//
public class PaymentTransactionInfoPluginConverter {
- public static TransactionStatus toTransactionStatus(@Nullable final PaymentTransactionInfoPlugin paymentTransactionInfoPlugin) {
- //
- // paymentTransactionInfoPlugin when we got an exception from the plugin, or if the plugin behaves badly
- // and decides to return null; in all cases this is seen as a PLUGIN_FAILURE
- //
- if (paymentTransactionInfoPlugin == null || paymentTransactionInfoPlugin.getStatus() == null) {
- return TransactionStatus.PLUGIN_FAILURE;
- }
+ public static TransactionStatus toTransactionStatus(final PaymentTransactionInfoPlugin paymentTransactionInfoPlugin) {
switch (paymentTransactionInfoPlugin.getStatus()) {
case PROCESSED:
return TransactionStatus.SUCCESS;
case PENDING:
return TransactionStatus.PENDING;
+ // The naming is a bit inconsistent, but ERROR on the plugin side means PAYMENT_FAILURE (that is a case where transaction went through but did not
+ // return successfully (e.g: CC denied, ...)
case ERROR:
return TransactionStatus.PAYMENT_FAILURE;
+ //
+ // The plugin is trying to tell us that it knows for sure that payment transaction did not happen (connection failure,..)
+ case CANCELED:
+ return TransactionStatus.PLUGIN_FAILURE;
+ //
// This will be picked up by Janitor to figure out what really happened and correct the state if needed
+ // Note that the default case includes the null status
+ //
case UNDEFINED:
default:
return TransactionStatus.UNKNOWN;
}
}
- public static OperationResult toOperationResult(@Nullable final PaymentTransactionInfoPlugin paymentTransactionInfoPlugin) {
- if (paymentTransactionInfoPlugin == null || paymentTransactionInfoPlugin.getStatus() == null) {
- return OperationResult.EXCEPTION;
- }
-
+ public static OperationResult toOperationResult(final PaymentTransactionInfoPlugin paymentTransactionInfoPlugin) {
switch (paymentTransactionInfoPlugin.getStatus()) {
case PROCESSED:
return OperationResult.SUCCESS;
@@ -63,8 +61,8 @@ public class PaymentTransactionInfoPluginConverter {
return OperationResult.PENDING;
case ERROR:
return OperationResult.FAILURE;
- // Might change later if Janitor fixes the state
case UNDEFINED:
+ case CANCELED:
default:
return OperationResult.EXCEPTION;
}
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/ProcessorBase.java b/payment/src/main/java/org/killbill/billing/payment/core/ProcessorBase.java
index a411e2c..77a1265 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/ProcessorBase.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/ProcessorBase.java
@@ -174,8 +174,7 @@ public abstract class ProcessorBase {
final PaymentTransactionModelDao transactionAlreadyExists = Iterables.tryFind(transactions, new Predicate<PaymentTransactionModelDao>() {
@Override
public boolean apply(final PaymentTransactionModelDao input) {
- return input.getTransactionStatus() == TransactionStatus.SUCCESS ||
- input.getTransactionStatus() == TransactionStatus.UNKNOWN;
+ return input.getTransactionStatus() == TransactionStatus.SUCCESS;
}
}).orNull();
if (transactionAlreadyExists != null) {
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentOperation.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentOperation.java
index 9f989f3..6781ecd 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentOperation.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentOperation.java
@@ -45,7 +45,7 @@ import org.killbill.billing.payment.provider.DefaultNoOpPaymentInfoPlugin;
import org.killbill.commons.locker.GlobalLocker;
import org.killbill.commons.locker.LockFailedException;
-import com.google.common.base.Objects;
+import com.google.common.base.MoreObjects;
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
@@ -74,25 +74,24 @@ public abstract class PaymentOperation extends OperationCallbackBase<PaymentTran
} else {
return doSimpleOperationCallback();
}
- } catch (final PaymentApiException e) {
- throw new OperationException(e, OperationResult.EXCEPTION);
+ } catch (final Exception e) {
+ throw convertToUnknownTransactionStatusAndErroredPaymentState(e);
}
}
@Override
protected OperationException rewrapExecutionException(final PaymentStateContext paymentStateContext, final ExecutionException e) {
- final Throwable realException = Objects.firstNonNull(e.getCause(), e);
- if (e.getCause() instanceof PaymentApiException) {
- logger.warn("Unsuccessful plugin call for account {}", paymentStateContext.getAccount().getExternalKey(), realException);
- return convertToUnknownTransactionStatusAndErroredPaymentState(realException);
- } else if (e.getCause() instanceof LockFailedException) {
- final String format = String.format("Failed to lock account %s", paymentStateContext.getAccount().getExternalKey());
- logger.error(String.format(format));
- return new OperationException(realException, OperationResult.EXCEPTION);
- } else /* if (e instanceof RuntimeException) */ {
- logger.warn("Plugin call threw an exception for account {}", paymentStateContext.getAccount().getExternalKey(), e);
- return new OperationException(realException, OperationResult.EXCEPTION);
+ //
+ // Any case of exception (checked or runtime) should lead to a TransactionStatus.UNKNOWN (and a XXX_ERRORED payment state).
+ // In order to reach that state we create PaymentTransactionInfoPlugin with an PaymentPluginStatus.UNDEFINED status (and an OperationResult.EXCEPTION).
+ //
+ final Throwable originalExceptionOrCause = MoreObjects.firstNonNull(e.getCause(), e);
+ if (originalExceptionOrCause instanceof LockFailedException) {
+ logger.warn("Failed to lock account {}", paymentStateContext.getAccount().getExternalKey());
+ } else {
+ logger.warn("Payment plugin call threw an exception for account {}", paymentStateContext.getAccount().getExternalKey(), originalExceptionOrCause);
}
+ return convertToUnknownTransactionStatusAndErroredPaymentState(originalExceptionOrCause);
}
@Override
@@ -102,7 +101,7 @@ public abstract class PaymentOperation extends OperationCallbackBase<PaymentTran
}
//
- // In the case we don't know exactly what happen (Timeout or PluginApiException):
+ // In case of exceptions, timeouts, we don't really know what happened:
// - Return an OperationResult.EXCEPTION to transition Payment State to Errored (see PaymentTransactionInfoPluginConverter#toOperationResult)
// - Construct a PaymentTransactionInfoPlugin whose PaymentPluginStatus = UNDEFINED to end up with a paymentTransactionStatus = UNKNOWN and have a chance to
// be fixed by Janitor.
@@ -181,8 +180,14 @@ public abstract class PaymentOperation extends OperationCallbackBase<PaymentTran
//
if (paymentStateContext.getOverridePluginOperationResult() == null) {
final PaymentTransactionInfoPlugin paymentInfoPlugin = doCallSpecificOperationCallback();
- // Throws if plugin is ot correctly implemented (e.g returns null result, values,..)
- sanityOnPaymentInfoPlugin(paymentInfoPlugin);
+ //
+ // We catch null paymentInfoPlugin and throw a RuntimeException to end up in an UNKNOWN transactionStatus
+ // That way we can use the null paymentInfoPlugin when a PaymentPluginApiException is thrown and correctly
+ // make the transition to PLUGIN_FAILURE
+ //
+ if (paymentInfoPlugin == null) {
+ throw new IllegalStateException("Payment plugin returned a null result");
+ }
paymentStateContext.setPaymentTransactionInfoPlugin(paymentInfoPlugin);
return PaymentTransactionInfoPluginConverter.toOperationResult(paymentStateContext.getPaymentTransactionInfoPlugin());
@@ -217,22 +222,4 @@ public abstract class PaymentOperation extends OperationCallbackBase<PaymentTran
return PaymentPluginStatus.UNDEFINED;
}
}
-
- private void sanityOnPaymentInfoPlugin(final PaymentTransactionInfoPlugin paymentInfoPlugin) throws PaymentApiException {
- if (paymentInfoPlugin == null) {
- throw new PaymentApiException(ErrorCode.PAYMENT_PLUGIN_EXCEPTION, "Payment plugin returned a null result");
- }
- /*
- TODO this breaks our tests so test would have to be fixed
- if (paymentTransactionInfoPlugin.getKbTransactionPaymentId() == null || !paymentTransactionInfoPlugin.getKbTransactionPaymentId().equals(paymentStateContext.getTransactionId())) {
- throw new PaymentApiException(ErrorCode.PAYMENT_PLUGIN_EXCEPTION, "Payment plugin returned invalid kbTransactionId");
- }
- if (paymentTransactionInfoPlugin.getKbPaymentId() == null || !paymentTransactionInfoPlugin.getKbPaymentId().equals(paymentStateContext.getPaymentId())) {
- throw new PaymentApiException(ErrorCode.PAYMENT_PLUGIN_EXCEPTION, "Payment plugin returned invalid kbPaymentId");
- }
- if (paymentTransactionInfoPlugin.getTransactionType() == null || !paymentTransactionInfoPlugin.getKbPaymentId().equals(paymentStateContext.getTransactionType())) {
- throw new PaymentApiException(ErrorCode.PAYMENT_PLUGIN_EXCEPTION, "Payment plugin returned invalid transaction type");
- }
- */
- }
}
diff --git a/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java b/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java
index c1f1203..3b41115 100644
--- a/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java
+++ b/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java
@@ -59,6 +59,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.base.Function;
+import com.google.common.base.Functions;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
import com.google.common.collect.Collections2;
@@ -161,12 +162,8 @@ public class DefaultPaymentDao implements PaymentDao {
@Override
public List<PaymentTransactionModelDao> inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception {
final TransactionSqlDao transactional = entitySqlDaoWrapperFactory.become(TransactionSqlDao.class);
- final Collection<String> allTransactionStatus = ImmutableList.copyOf(Iterables.transform(transactionStatuses, new Function<TransactionStatus, String>() {
- @Override
- public String apply(final TransactionStatus input) {
- return input.toString();
- }
- }));
+
+ final Collection<String> allTransactionStatus = ImmutableList.copyOf(Iterables.transform(transactionStatuses, Functions.toStringFunction()));
return transactional.getByTransactionStatusPriorDateAcrossTenants(allTransactionStatus, createdBeforeDate.toDate(), createdAfterDate.toDate(), limit);
}
});
diff --git a/payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentRoutingPluginApi.java b/payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentRoutingPluginApi.java
index e1b5ddf..30d5115 100644
--- a/payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentRoutingPluginApi.java
+++ b/payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentRoutingPluginApi.java
@@ -385,7 +385,8 @@ public final class InvoicePaymentRoutingPluginApi implements PaymentRoutingPlugi
private DateTime getNextRetryDateForPluginFailure(final List<PaymentTransactionModelDao> purchasedTransactions) {
DateTime result = null;
- final int attemptsInState = getNumberAttemptsInState(purchasedTransactions, TransactionStatus.PLUGIN_FAILURE);
+ // Not clear whether or not we should really include TransactionStatus.UNKNOWN
+ final int attemptsInState = getNumberAttemptsInState(purchasedTransactions, TransactionStatus.PLUGIN_FAILURE, TransactionStatus.UNKNOWN);
final int retryAttempt = (attemptsInState - 1) >= 0 ? (attemptsInState - 1) : 0;
if (retryAttempt < paymentConfig.getPluginFailureRetryMaxAttempts()) {
diff --git a/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentEnteringStateCallback.java b/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentEnteringStateCallback.java
index c4f53e6..e146a6e 100644
--- a/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentEnteringStateCallback.java
+++ b/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentEnteringStateCallback.java
@@ -105,18 +105,6 @@ public class TestPaymentEnteringStateCallback extends PaymentTestSuiteWithEmbedd
Assert.assertEquals(paymentTransaction.getGatewayErrorMsg(), paymentInfoPlugin.getGatewayError());
}
- @Test(groups = "slow")
- public void testEnterStateWithOperationException() throws Exception {
- daoHelper.createNewPaymentTransaction();
- // Simulate a bug in the plugin - i.e. nothing was returned
- paymentStateContext.setPaymentTransactionInfoPlugin(null);
- operationResult = OperationResult.EXCEPTION;
-
- callback.enteringState(state, operationCallback, operationResult, leavingStateCallback);
-
- Assert.assertEquals(paymentDao.getPaymentTransaction(paymentStateContext.getPaymentTransactionModelDao().getId(), internalCallContext).getTransactionStatus(), TransactionStatus.PLUGIN_FAILURE);
- }
-
private static final class PaymentEnteringStateTestCallback extends PaymentEnteringStateCallback {
private PaymentEnteringStateTestCallback(final PaymentAutomatonDAOHelper daoHelper, final PaymentStateContext paymentStateContext) throws PaymentApiException {
diff --git a/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentOperation.java b/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentOperation.java
index 9f6eb58..586e759 100644
--- a/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentOperation.java
+++ b/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentOperation.java
@@ -76,7 +76,8 @@ public class TestPaymentOperation extends PaymentTestSuiteNoDB {
Assert.assertEquals(e.getOperationResult(), OperationResult.EXCEPTION);
}
- Assert.assertNull(paymentStateContext.getPaymentTransactionInfoPlugin());
+ Assert.assertNotNull(paymentStateContext.getPaymentTransactionInfoPlugin());
+ Assert.assertEquals(paymentStateContext.getPaymentTransactionInfoPlugin().getStatus(), PaymentPluginStatus.UNDEFINED);
}
@Test(groups = "fast")
diff --git a/payment/src/test/java/org/killbill/billing/payment/TestJanitor.java b/payment/src/test/java/org/killbill/billing/payment/TestJanitor.java
index 986106b..bc68a36 100644
--- a/payment/src/test/java/org/killbill/billing/payment/TestJanitor.java
+++ b/payment/src/test/java/org/killbill/billing/payment/TestJanitor.java
@@ -21,6 +21,7 @@ import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import java.util.UUID;
import org.joda.time.LocalDate;
@@ -38,10 +39,13 @@ import org.killbill.billing.payment.api.TransactionStatus;
import org.killbill.billing.payment.api.TransactionType;
import org.killbill.billing.payment.core.janitor.Janitor;
import org.killbill.billing.payment.dao.PaymentAttemptModelDao;
+import org.killbill.billing.payment.dao.PaymentTransactionModelDao;
import org.killbill.billing.payment.invoice.InvoicePaymentRoutingPluginApi;
import org.killbill.billing.payment.provider.MockPaymentProviderPlugin;
import org.killbill.billing.platform.api.KillbillConfigSource;
import org.killbill.bus.api.PersistentBus.EventBusException;
+import org.skife.jdbi.v2.Handle;
+import org.skife.jdbi.v2.tweak.HandleCallback;
import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.AfterMethod;
@@ -72,6 +76,8 @@ public class TestJanitor extends PaymentTestSuiteWithEmbeddedDB {
@Inject
private Janitor janitor;
+ private MockPaymentProviderPlugin mockPaymentProviderPlugin;
+
private Account account;
@Override
@@ -86,6 +92,7 @@ public class TestJanitor extends PaymentTestSuiteWithEmbeddedDB {
@BeforeClass(groups = "slow")
protected void beforeClass() throws Exception {
super.beforeClass();
+ mockPaymentProviderPlugin = (MockPaymentProviderPlugin) registry.getServiceForName(MockPaymentProviderPlugin.PLUGIN_NAME);
janitor.start();
}
@@ -97,6 +104,7 @@ public class TestJanitor extends PaymentTestSuiteWithEmbeddedDB {
@BeforeMethod(groups = "slow")
public void beforeMethod() throws Exception {
super.beforeMethod();
+ mockPaymentProviderPlugin.clear();
account = testHelper.createTestAccount("bobo@gmail.com", true);
}
@@ -151,7 +159,6 @@ public class TestJanitor extends PaymentTestSuiteWithEmbeddedDB {
Thread.sleep(1500);
} catch (InterruptedException e) {
}
- ;
final PaymentAttemptModelDao attempt3 = paymentDao.getPaymentAttempt(attempt.getId(), internalCallContext);
assertEquals(attempt3.getStateName(), "SUCCESS");
@@ -214,7 +221,6 @@ public class TestJanitor extends PaymentTestSuiteWithEmbeddedDB {
Thread.sleep(1500);
} catch (InterruptedException e) {
}
- ;
final PaymentAttemptModelDao attempt3 = paymentDao.getPaymentAttempt(refundAttempt.getId(), internalCallContext);
assertEquals(attempt3.getStateName(), "SUCCESS");
@@ -244,13 +250,83 @@ public class TestJanitor extends PaymentTestSuiteWithEmbeddedDB {
final Payment updatedPayment = paymentApi.getPayment(payment.getId(), false, ImmutableList.<PluginProperty>of(), callContext);
assertEquals(updatedPayment.getTransactions().get(0).getTransactionStatus(), TransactionStatus.SUCCESS);
+ }
+
+ @Test(groups = "slow")
+ public void testUnknownEntriesWithFailures() throws PaymentApiException, EventBusException {
+
+ final BigDecimal requestedAmount = BigDecimal.TEN;
+ final String paymentExternalKey = "minus";
+ final String transactionExternalKey = "plus";
+
+ // Make sure the state as seen by the plugin will be in PaymentPluginStatus.ERROR, which will be returned later to Janitor
+ mockPaymentProviderPlugin.makeNextPaymentFailWithError();
+ final Payment payment = paymentApi.createAuthorization(account, account.getPaymentMethodId(), null, requestedAmount, account.getCurrency(), paymentExternalKey,
+ transactionExternalKey, ImmutableList.<PluginProperty>of(), callContext);
+
+ // Artificially move the transaction status to UNKNOWN
+ final String paymentStateName = paymentSMHelper.getErroredStateForTransaction(TransactionType.AUTHORIZE).toString();
+ paymentDao.updatePaymentAndTransactionOnCompletion(account.getId(), payment.getId(), TransactionType.AUTHORIZE, paymentStateName, paymentStateName,
+ payment.getTransactions().get(0).getId(), TransactionStatus.UNKNOWN, requestedAmount, account.getCurrency(),
+ "foo", "bar", internalCallContext);
+
+ final List<PaymentTransactionModelDao> paymentTransactionHistoryBeforeJanitor = getPaymentTransactionHistory(transactionExternalKey);
+ Assert.assertEquals(paymentTransactionHistoryBeforeJanitor.size(), 3);
+
+ clock.addDays(1);
+ try {
+ Thread.sleep(1500);
+ } catch (InterruptedException e) {
+ }
+
+ // Proves the Janitor ran (and updated the transaction)
+ final List<PaymentTransactionModelDao> paymentTransactionHistoryAfterJanitor = getPaymentTransactionHistory(transactionExternalKey);
+ Assert.assertEquals(paymentTransactionHistoryAfterJanitor.size(), 4);
+ Assert.assertEquals(paymentTransactionHistoryAfterJanitor.get(3).getTransactionStatus(), TransactionStatus.PAYMENT_FAILURE);
+ final Payment updatedPayment = paymentApi.getPayment(payment.getId(), false, ImmutableList.<PluginProperty>of(), callContext);
+ // Janitor should have moved us to PAYMENT_FAILURE
+ assertEquals(updatedPayment.getTransactions().get(0).getTransactionStatus(), TransactionStatus.PAYMENT_FAILURE);
}
+ @Test(groups = "slow")
+ public void testUnknownEntriesWithExceptions() throws PaymentApiException, EventBusException {
+
+ final BigDecimal requestedAmount = BigDecimal.TEN;
+ final String paymentExternalKey = "minus";
+ final String transactionExternalKey = "plus";
+
+ // Make sure the state as seen by the plugin will be in PaymentPluginStatus.ERROR, which will be returned later to Janitor
+ mockPaymentProviderPlugin.makeNextPaymentFailWithException();
+ try {
+ paymentApi.createAuthorization(account, account.getPaymentMethodId(), null, requestedAmount, account.getCurrency(), paymentExternalKey,
+ transactionExternalKey, ImmutableList.<PluginProperty>of(), callContext);
+ } catch (PaymentApiException ignore) {
+ }
+ final Payment payment = paymentApi.getPaymentByExternalKey(paymentExternalKey, false, ImmutableList.<PluginProperty>of(), callContext);
+ // Artificially move the transaction status to UNKNOWN
+ final String paymentStateName = paymentSMHelper.getErroredStateForTransaction(TransactionType.AUTHORIZE).toString();
+ paymentDao.updatePaymentAndTransactionOnCompletion(account.getId(), payment.getId(), TransactionType.AUTHORIZE, paymentStateName, paymentStateName,
+ payment.getTransactions().get(0).getId(), TransactionStatus.UNKNOWN, requestedAmount, account.getCurrency(),
+ "foo", "bar", internalCallContext);
+
+ final List<PaymentTransactionModelDao> paymentTransactionHistoryBeforeJanitor = getPaymentTransactionHistory(transactionExternalKey);
+ Assert.assertEquals(paymentTransactionHistoryBeforeJanitor.size(), 3);
+
+ clock.addDays(1);
+ try {
+ Thread.sleep(1500);
+ } catch (InterruptedException e) {
+ }
+
+ // Nothing new happened
+ final List<PaymentTransactionModelDao> paymentTransactionHistoryAfterJanitor = getPaymentTransactionHistory(transactionExternalKey);
+ Assert.assertEquals(paymentTransactionHistoryAfterJanitor.size(), 3);
+ }
@Test(groups = "slow")
- public void testPendingEntries() throws PaymentApiException, InvoiceApiException, EventBusException {
+ public void testPendingEntries() throws PaymentApiException, EventBusException {
final BigDecimal requestedAmount = BigDecimal.TEN;
final String paymentExternalKey = "jhj44";
@@ -280,5 +356,34 @@ public class TestJanitor extends PaymentTestSuiteWithEmbeddedDB {
result.add(new PluginProperty(InvoicePaymentRoutingPluginApi.PROP_IPCD_INVOICE_ID, invoice.getId().toString(), false));
return result;
}
+
+ // I wish we had a simplest way to query our history rows..
+ private List<PaymentTransactionModelDao> getPaymentTransactionHistory(final String transactionExternalKey) {
+ return dbi.withHandle(new HandleCallback<List<PaymentTransactionModelDao>>() {
+ @Override
+ public List<PaymentTransactionModelDao> withHandle(final Handle handle) throws Exception {
+ final List<Map<String, Object>> queryResult = handle.select("select * from payment_transaction_history where transaction_external_key = ? order by record_id asc",
+ transactionExternalKey);
+ final List<PaymentTransactionModelDao> result = new ArrayList<PaymentTransactionModelDao>(queryResult.size());
+ for (final Map<String, Object> row : queryResult) {
+ final PaymentTransactionModelDao transactionModelDao = new PaymentTransactionModelDao(UUID.fromString((String) row.get("id")),
+ null,
+ (String) row.get("transaction_external_key"),
+ null,
+ null,
+ UUID.fromString((String) row.get("payment_id")),
+ TransactionType.valueOf((String) row.get("transaction_type")),
+ null,
+ TransactionStatus.valueOf((String) row.get("transaction_status")),
+ (BigDecimal) row.get("amount"),
+ Currency.valueOf((String) row.get("currency")),
+ (String) row.get("gateway_error_code"),
+ (String) row.get("gateway_error_msg"));
+ result.add(transactionModelDao);
+ }
+ return result;
+ }
+ });
+ }
}
pom.xml 2(+1 -1)
diff --git a/pom.xml b/pom.xml
index ffe30ae..837f44f 100644
--- a/pom.xml
+++ b/pom.xml
@@ -21,7 +21,7 @@
<parent>
<artifactId>killbill-oss-parent</artifactId>
<groupId>org.kill-bill.billing</groupId>
- <version>0.14</version>
+ <version>0.15</version>
</parent>
<artifactId>killbill</artifactId>
<version>0.15.0-SNAPSHOT</version>
diff --git a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestAdmin.java b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestAdmin.java
new file mode 100644
index 0000000..5953157
--- /dev/null
+++ b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestAdmin.java
@@ -0,0 +1,118 @@
+/*
+ * Copyright 2014-2015 Groupon, Inc
+ * Copyright 2014-2015 The Billing Project, LLC
+ *
+ * The Billing Project licenses this file to you under the Apache License, version 2.0
+ * (the "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.killbill.billing.jaxrs;
+
+import java.math.BigDecimal;
+import java.util.UUID;
+
+import org.killbill.billing.client.KillBillClientException;
+import org.killbill.billing.client.KillBillHttpClient;
+import org.killbill.billing.client.model.Account;
+import org.killbill.billing.client.model.Payment;
+import org.killbill.billing.client.model.PaymentTransaction;
+import org.killbill.billing.jaxrs.json.AdminPaymentJson;
+import org.killbill.billing.payment.api.TransactionStatus;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.Multimap;
+
+public class TestAdmin extends TestJaxrsBase {
+
+ @Test(groups = "slow")
+ public void testAdminPaymentEndpoint() throws Exception {
+ final Account account = createAccountWithDefaultPaymentMethod();
+
+ final String paymentExternalKey = "extkey";
+
+ // Create Authorization
+ final String authTransactionExternalKey = UUID.randomUUID().toString();
+ final PaymentTransaction authTransaction = new PaymentTransaction();
+ authTransaction.setAmount(BigDecimal.TEN);
+ authTransaction.setCurrency(account.getCurrency());
+ authTransaction.setPaymentExternalKey(paymentExternalKey);
+ authTransaction.setTransactionExternalKey(authTransactionExternalKey);
+ authTransaction.setTransactionType("AUTHORIZE");
+ final Payment authPayment = killBillClient.createPayment(account.getAccountId(), account.getPaymentMethodId(), authTransaction, createdBy, reason, comment);
+
+ // First fix transactionStatus and paymentSstate (but not lastSuccessPaymentState
+ // Note that state is not consistent between TransactionStatus and lastSuccessPaymentState but we don't care.
+ fixPaymentState(authPayment, null, "AUTH_FAILED", TransactionStatus.PAYMENT_FAILURE);
+
+ final Payment updatedPayment1 = killBillClient.getPayment(authPayment.getPaymentId());
+ Assert.assertEquals(updatedPayment1.getTransactions().size(), 1);
+ final PaymentTransaction authTransaction1 = updatedPayment1.getTransactions().get(0);
+ Assert.assertEquals(authTransaction1.getStatus(), TransactionStatus.PAYMENT_FAILURE.toString());
+
+ // Capture should succeed because lastSuccessPaymentState was left untouched
+ doCapture(updatedPayment1, false);
+
+ fixPaymentState(authPayment, "AUTH_FAILED", "AUTH_FAILED", TransactionStatus.PAYMENT_FAILURE);
+
+ final Payment updatedPayment2 = killBillClient.getPayment(authPayment.getPaymentId());
+ Assert.assertEquals(updatedPayment2.getTransactions().size(), 2);
+ final PaymentTransaction authTransaction2 = updatedPayment2.getTransactions().get(0);
+ Assert.assertEquals(authTransaction2.getStatus(), TransactionStatus.PAYMENT_FAILURE.toString());
+
+ final PaymentTransaction captureTransaction2 = updatedPayment2.getTransactions().get(1);
+ Assert.assertEquals(captureTransaction2.getStatus(), TransactionStatus.SUCCESS.toString());
+
+ // Capture should now failed because lastSuccessPaymentState was moved to AUTH_FAILED
+ doCapture(updatedPayment2, true);
+ }
+
+ private void doCapture(final Payment payment, final boolean expectException) throws KillBillClientException {
+ // Payment object does not export state, this is purely internal, so to verify that we indeed changed to Failed, we can attempt
+ // a capture, which should fail
+ final String capture1TransactionExternalKey = UUID.randomUUID().toString();
+ final PaymentTransaction captureTransaction = new PaymentTransaction();
+ captureTransaction.setPaymentId(payment.getPaymentId());
+ captureTransaction.setAmount(BigDecimal.ONE);
+ captureTransaction.setCurrency(payment.getCurrency());
+ captureTransaction.setPaymentExternalKey(payment.getPaymentExternalKey());
+ captureTransaction.setTransactionExternalKey(capture1TransactionExternalKey);
+ try {
+ killBillClient.captureAuthorization(captureTransaction, createdBy, reason, comment);
+ if (expectException) {
+ Assert.fail("Capture should not succeed, after auth was moved to a PAYMENT_FAILURE");
+ }
+ } catch (final KillBillClientException mabeExpected) {
+ if (!expectException) {
+ throw mabeExpected;
+ }
+ }
+
+ }
+
+
+ private void fixPaymentState(final Payment payment, final String lastSuccessPaymentState, final String currentPaymentStateName, final TransactionStatus transactionStatus) throws KillBillClientException {
+ //
+ // We do not expose the endpoint in the client API on purpose since this should only be accessed using special permission ADMIN_CAN_FIX_DATA
+ // for when there is a need to fix payment state.
+ //
+ final String uri = "/1.0/kb/admin/payments/" + payment.getPaymentId().toString() + "/transactions/" + payment.getTransactions().get(0).getTransactionId().toString();
+
+ final AdminPaymentJson body = new AdminPaymentJson(lastSuccessPaymentState, currentPaymentStateName, transactionStatus.toString());
+ final Multimap result = HashMultimap.create();
+ result.put(KillBillHttpClient.AUDIT_OPTION_CREATED_BY, createdBy);
+ result.put(KillBillHttpClient.AUDIT_OPTION_REASON, reason);
+ result.put(KillBillHttpClient.AUDIT_OPTION_COMMENT, comment);
+ killBillHttpClient.doPut(uri, body, result);
+ }
+}