killbill-memoizeit
Changes
payment/src/main/java/org/killbill/billing/payment/core/sm/control/OperationControlCallback.java 30(+6 -24)
payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentOperation.java 42(+11 -31)
Details
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 22b0d54..9e09c24 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
@@ -18,8 +18,6 @@
package org.killbill.billing.payment.core.sm.control;
import java.util.List;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeoutException;
import javax.annotation.Nullable;
@@ -47,12 +45,10 @@ import org.killbill.billing.payment.dispatcher.PluginDispatcher;
import org.killbill.billing.payment.dispatcher.PluginDispatcher.PluginDispatcherReturnType;
import org.killbill.billing.util.config.PaymentConfig;
import org.killbill.commons.locker.GlobalLocker;
-import org.killbill.commons.locker.LockFailedException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.base.Joiner;
-import com.google.common.base.MoreObjects;
public abstract class OperationControlCallback extends OperationCallbackBase<Payment, PaymentApiException> implements OperationCallback {
@@ -149,33 +145,19 @@ public abstract class OperationControlCallback extends OperationCallbackBase<Pay
throw new OperationException(e, executePluginOnFailureCallsAndSetRetryDate(paymentControlContext));
} catch (final RuntimeException e) {
// Attempts to set the retry date in context if needed.
- executePluginOnFailureCallsAndSetRetryDate(paymentControlContext);
- throw new OperationException(e, OperationResult.EXCEPTION);
+ throw new OperationException(e, executePluginOnFailureCallsAndSetRetryDate(paymentControlContext));
}
}
});
}
@Override
- protected OperationException unwrapExceptionFromDispatchedTask(final Exception e) {
- // If this is an ExecutionException we attempt to extract the cause first
- final Throwable originalExceptionOrCausePossiblyOperationException = e instanceof ExecutionException ? MoreObjects.firstNonNull(e.getCause(), e) : e;
-
- // Unwrap OperationException too (doOperationCallback wraps exceptions in OperationException)
- final Throwable originalExceptionOrCause = originalExceptionOrCausePossiblyOperationException instanceof OperationException ? MoreObjects.firstNonNull(originalExceptionOrCausePossiblyOperationException.getCause(), originalExceptionOrCausePossiblyOperationException) : originalExceptionOrCausePossiblyOperationException;
-
- if (originalExceptionOrCause instanceof OperationException) {
- return (OperationException) originalExceptionOrCause;
- } else if (originalExceptionOrCause instanceof LockFailedException) {
- logger.warn("Failed to lock accountId='{}'", paymentStateContext.getAccount().getId());
- } else if (originalExceptionOrCause instanceof TimeoutException) {
- logger.warn("Call TIMEOUT for accountId='{}'", paymentStateContext.getAccount().getId());
- } else if (originalExceptionOrCause instanceof InterruptedException) {
- logger.warn("Call was interrupted for accountId='{}'", paymentStateContext.getAccount().getId());
- } else {
- logger.warn("Operation failed for accountId='{}'", paymentStateContext.getAccount().getId(), e);
+ protected OperationException unwrapExceptionFromDispatchedTask(final PaymentApiException e) {
+ if (e.getCause() instanceof OperationException) {
+ return (OperationException) e.getCause();
}
- return new OperationException(originalExceptionOrCause, getOperationResultOnException(paymentStateContext));
+ logger.warn("Operation failed for accountId='{}'", paymentStateContext.getAccount().getId(), e);
+ return new OperationException(e, getOperationResultOnException(paymentStateContext));
}
private OperationResult getOperationResultOnException(final PaymentStateContext paymentStateContext) {
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/OperationCallbackBase.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/OperationCallbackBase.java
index 1f52802..50c95a3 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/OperationCallbackBase.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/OperationCallbackBase.java
@@ -18,14 +18,14 @@
package org.killbill.billing.payment.core.sm;
import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeoutException;
import org.killbill.automaton.OperationException;
import org.killbill.automaton.OperationResult;
import org.killbill.billing.account.api.Account;
+import org.killbill.billing.payment.api.PaymentApiException;
import org.killbill.billing.payment.core.ProcessorBase.CallableWithAccountLock;
import org.killbill.billing.payment.core.ProcessorBase.DispatcherCallback;
+import org.killbill.billing.payment.dispatcher.PaymentPluginDispatcher;
import org.killbill.billing.payment.dispatcher.PluginDispatcher;
import org.killbill.billing.payment.dispatcher.PluginDispatcher.PluginDispatcherReturnType;
import org.killbill.billing.util.config.PaymentConfig;
@@ -68,17 +68,10 @@ public abstract class OperationCallbackBase<CallbackOperationResult, CallbackOpe
paymentConfig,
callback);
logger.debug("Calling plugin(s) {}", pluginNames);
- final OperationResult operationResult = paymentPluginDispatcher.dispatchWithTimeout(task);
+ final OperationResult operationResult = PaymentPluginDispatcher.dispatchWithExceptionHandling(account, pluginNames, task, paymentPluginDispatcher);
logger.debug("Successful plugin(s) call of {} for account {} with result {}", pluginNames, account.getExternalKey(), operationResult);
return operationResult;
- } catch (final ExecutionException e) {
- throw unwrapExceptionFromDispatchedTask(e);
- } catch (final TimeoutException e) {
- logger.warn("TimeoutException while executing the plugin(s) {}", pluginNames);
- throw unwrapExceptionFromDispatchedTask(e);
- } catch (final InterruptedException e) {
- Thread.currentThread().interrupt();
- logger.warn("InterruptedException while executing the following plugin(s): {}", pluginNames);
+ } catch (final PaymentApiException e) {
throw unwrapExceptionFromDispatchedTask(e);
}
}
@@ -91,5 +84,5 @@ public abstract class OperationCallbackBase<CallbackOperationResult, CallbackOpe
//
protected abstract CallbackOperationResult doCallSpecificOperationCallback() throws CallbackOperationException;
- protected abstract OperationException unwrapExceptionFromDispatchedTask(final Exception e);
+ protected abstract OperationException unwrapExceptionFromDispatchedTask(final PaymentApiException e);
}
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 e269932..ba20e1a 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
@@ -19,8 +19,6 @@ package org.killbill.billing.payment.core.sm.payments;
import java.math.BigDecimal;
import java.util.Iterator;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeoutException;
import org.killbill.automaton.Operation.OperationCallback;
import org.killbill.automaton.OperationException;
@@ -44,11 +42,9 @@ import org.killbill.billing.payment.plugin.api.PaymentTransactionInfoPlugin;
import org.killbill.billing.payment.provider.DefaultNoOpPaymentInfoPlugin;
import org.killbill.billing.util.config.PaymentConfig;
import org.killbill.commons.locker.GlobalLocker;
-import org.killbill.commons.locker.LockFailedException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.google.common.base.MoreObjects;
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
@@ -86,35 +82,16 @@ public abstract class PaymentOperation extends OperationCallbackBase<PaymentTran
} else {
try {
return doSimpleOperationCallback();
- } catch (final Exception e) {
- // We need to unwrap OperationException (see doSimpleOperationCallback below)
- throw unwrapExceptionFromDispatchedTask(e);
- }
- }
+ } catch (final OperationException e) {
+ throw convertToUnknownTransactionStatusAndErroredPaymentState(e);
+ }
+ }
}
@Override
- protected OperationException unwrapExceptionFromDispatchedTask(final Exception e) {
- // If this is an ExecutionException we attempt to extract the cause first
- final Throwable originalExceptionOrCausePossiblyOperationException = e instanceof ExecutionException ? MoreObjects.firstNonNull(e.getCause(), e) : e;
-
- // Unwrap OperationException too (doOperationCallback wraps exceptions in OperationException)
- final Throwable originalExceptionOrCause = originalExceptionOrCausePossiblyOperationException instanceof OperationException ? MoreObjects.firstNonNull(originalExceptionOrCausePossiblyOperationException.getCause(), originalExceptionOrCausePossiblyOperationException) : originalExceptionOrCausePossiblyOperationException;
-
- //
- // 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).
- //
- if (originalExceptionOrCause instanceof LockFailedException) {
- logger.warn("Failed to lock accountExternalKey='{}'", paymentStateContext.getAccount().getExternalKey());
- } else if (originalExceptionOrCause instanceof TimeoutException) {
- logger.warn("Plugin call TIMEOUT for accountExternalKey='{}'", paymentStateContext.getAccount().getExternalKey());
- } else if (originalExceptionOrCause instanceof InterruptedException) {
- logger.warn("Plugin call was interrupted for accountExternalKey='{}'", paymentStateContext.getAccount().getExternalKey());
- } else {
- logger.warn("Payment plugin call threw an exception for accountExternalKey='{}'", paymentStateContext.getAccount().getExternalKey(), originalExceptionOrCause);
- }
- return convertToUnknownTransactionStatusAndErroredPaymentState(originalExceptionOrCause);
+ protected OperationException unwrapExceptionFromDispatchedTask(final PaymentApiException e) {
+ logger.warn("Payment plugin call threw an exception for accountExternalKey='{}'", paymentStateContext.getAccount().getExternalKey(), e);
+ return convertToUnknownTransactionStatusAndErroredPaymentState(e);
}
//
@@ -123,7 +100,7 @@ public abstract class PaymentOperation extends OperationCallbackBase<PaymentTran
// - Construct a PaymentTransactionInfoPlugin whose PaymentPluginStatus = UNDEFINED to end up with a paymentTransactionStatus = UNKNOWN and have a chance to
// be fixed by Janitor.
//
- private OperationException convertToUnknownTransactionStatusAndErroredPaymentState(final Throwable e) {
+ private OperationException convertToUnknownTransactionStatusAndErroredPaymentState(final Exception e) {
final PaymentTransactionInfoPlugin paymentInfoPlugin = new DefaultNoOpPaymentInfoPlugin(paymentStateContext.getPaymentId(),
paymentStateContext.getTransactionId(),
@@ -136,6 +113,9 @@ public abstract class PaymentOperation extends OperationCallbackBase<PaymentTran
null,
null);
paymentStateContext.setPaymentTransactionInfoPlugin(paymentInfoPlugin);
+ if (e instanceof OperationException) {
+ return (OperationException) e;
+ }
return new OperationException(e, OperationResult.EXCEPTION);
}
diff --git a/payment/src/main/java/org/killbill/billing/payment/dispatcher/PaymentPluginDispatcher.java b/payment/src/main/java/org/killbill/billing/payment/dispatcher/PaymentPluginDispatcher.java
index fec3c2e..7a4e1e2 100644
--- a/payment/src/main/java/org/killbill/billing/payment/dispatcher/PaymentPluginDispatcher.java
+++ b/payment/src/main/java/org/killbill/billing/payment/dispatcher/PaymentPluginDispatcher.java
@@ -64,7 +64,8 @@ public class PaymentPluginDispatcher {
log.warn(format);
throw new PaymentApiException(ErrorCode.PAYMENT_INTERNAL_ERROR, format);
} else {
- throw new PaymentApiException(e, ErrorCode.PAYMENT_INTERNAL_ERROR, MoreObjects.firstNonNull(e.getMessage(), ""));
+ // Unwraps the ExecutionException (e.getCause()), since it's a dispatch implementation detail
+ throw new PaymentApiException(e.getCause(), ErrorCode.PAYMENT_INTERNAL_ERROR, MoreObjects.firstNonNull(e.getMessage(), ""));
}
}
}
diff --git a/payment/src/test/java/org/killbill/billing/payment/core/sm/MockRetryAuthorizeOperationCallback.java b/payment/src/test/java/org/killbill/billing/payment/core/sm/MockRetryAuthorizeOperationCallback.java
index 2a3a677..07f878b 100644
--- a/payment/src/test/java/org/killbill/billing/payment/core/sm/MockRetryAuthorizeOperationCallback.java
+++ b/payment/src/test/java/org/killbill/billing/payment/core/sm/MockRetryAuthorizeOperationCallback.java
@@ -65,6 +65,8 @@ public class MockRetryAuthorizeOperationCallback extends AuthorizeControlOperati
if (exception != null) {
if (exception instanceof PaymentApiException) {
throw (PaymentApiException) exception;
+ } else if (exception instanceof RuntimeException) {
+ throw (RuntimeException) exception;
} else {
throw new RuntimeException(exception);
}
diff --git a/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPluginOperation.java b/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPluginOperation.java
index 41155e8..a1a514f 100644
--- a/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPluginOperation.java
+++ b/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPluginOperation.java
@@ -57,7 +57,7 @@ public class TestPluginOperation extends PaymentTestSuiteNoDB {
private static final String PLUGIN_NAME_PLACEHOLDER = "pluginName";
- private static final int TIMEOUT = 5;
+ private static final int TIMEOUT = 10;
private final GlobalLocker locker = new MemoryGlobalLocker();
private final Account account = Mockito.mock(Account.class);
@@ -97,7 +97,8 @@ public class TestPluginOperation extends PaymentTestSuiteNoDB {
Assert.fail();
} catch (final OperationException e) {
Assert.assertEquals(e.getOperationResult(), OperationResult.EXCEPTION);
- Assert.assertTrue(e.getCause() instanceof NullPointerException);
+ Assert.assertTrue(e.getCause() instanceof PaymentApiException);
+ Assert.assertTrue(e.getCause().getCause() instanceof NullPointerException);
}
}