Details
diff --git a/api/src/main/java/com/ning/billing/config/PaymentConfig.java b/api/src/main/java/com/ning/billing/config/PaymentConfig.java
index f934906..7e8d5cc 100644
--- a/api/src/main/java/com/ning/billing/config/PaymentConfig.java
+++ b/api/src/main/java/com/ning/billing/config/PaymentConfig.java
@@ -29,22 +29,21 @@ public interface PaymentConfig extends NotificationConfig, KillbillConfig {
@Default("noop")
public String getDefaultPaymentProvider();
-
@Config("killbill.payment.retry.days")
@Default("8,8,8")
public List<Integer> getPaymentRetryDays();
@Config("killbill.payment.failure.retry.start.sec")
@Default("300")
- public int getPaymentFailureRetryStart();
+ public int getPluginFailureRetryStart();
@Config("killbill.payment.failure.retry.multiplier")
@Default("2")
- public int getPaymentFailureRetryMultiplier();
+ public int getPluginFailureRetryMultiplier();
@Config("killbill.payment.failure.retry.max.attempts")
@Default("8")
- public int getPaymentFailureRetryMaxAttempts();
+ public int getPluginFailureRetryMaxAttempts();
@Override
@Config("killbill.payment.engine.notifications.sleep")
diff --git a/payment/src/main/java/com/ning/billing/payment/core/PaymentProcessor.java b/payment/src/main/java/com/ning/billing/payment/core/PaymentProcessor.java
index 229850f..73151f8 100644
--- a/payment/src/main/java/com/ning/billing/payment/core/PaymentProcessor.java
+++ b/payment/src/main/java/com/ning/billing/payment/core/PaymentProcessor.java
@@ -37,7 +37,6 @@ import com.ning.billing.ErrorCode;
import com.ning.billing.account.api.Account;
import com.ning.billing.account.api.AccountApiException;
import com.ning.billing.account.api.AccountUserApi;
-import com.ning.billing.config.PaymentConfig;
import com.ning.billing.invoice.api.Invoice;
import com.ning.billing.invoice.api.InvoicePaymentApi;
import com.ning.billing.payment.api.DefaultPayment;
@@ -70,7 +69,6 @@ public class PaymentProcessor extends ProcessorBase {
private final InvoicePaymentApi invoicePaymentApi;
private final FailedPaymentRetryServiceScheduler failedPaymentRetryService;
private final PluginFailureRetryServiceScheduler pluginFailureRetryService;
- private final PaymentConfig config;
private final PaymentDao paymentDao;
private final CallContextFactory factory;
private final Clock clock;
@@ -87,7 +85,6 @@ public class PaymentProcessor extends ProcessorBase {
final FailedPaymentRetryServiceScheduler failedPaymentRetryService,
final PluginFailureRetryServiceScheduler pluginFailureRetryService,
final PaymentDao paymentDao,
- final PaymentConfig config,
final Bus eventBus,
final Clock clock,
final GlobalLocker locker,
@@ -99,7 +96,6 @@ public class PaymentProcessor extends ProcessorBase {
this.pluginFailureRetryService = pluginFailureRetryService;
this.paymentDao = paymentDao;
this.clock = clock;
- this.config = config;
this.factory = factory;
this.paymentPluginDispatcher = new PluginDispatcher<Payment>(executor);
this.voidPluginDispatcher = new PluginDispatcher<Void>(executor);
@@ -340,6 +336,7 @@ public class PaymentProcessor extends ProcessorBase {
//
paymentStatus = isInstantPayment ? PaymentStatus.PAYMENT_FAILURE_ABORTED : scheduleRetryOnPluginFailure(paymentInput.getId());
// STEPH message might need truncation to fit??
+
paymentDao.updateStatusForPaymentWithAttempt(paymentInput.getId(), paymentStatus, e.getMessage(), attemptInput.getId(), context);
throw new PaymentApiException(ErrorCode.PAYMENT_CREATE_PAYMENT, account.getId(), e.getMessage());
@@ -354,8 +351,7 @@ public class PaymentProcessor extends ProcessorBase {
private PaymentStatus scheduleRetryOnPluginFailure(UUID paymentId) {
List<PaymentAttemptModelDao> allAttempts = paymentDao.getAttemptsForPayment(paymentId);
- // STEPH unknown only?
- final int retryAttempt = getNumberAttemptsInState(paymentId, allAttempts, PaymentStatus.UNKNOWN);
+ final int retryAttempt = getNumberAttemptsInState(paymentId, allAttempts, PaymentStatus.UNKNOWN, PaymentStatus.PLUGIN_FAILURE);
final boolean isScheduledForRetry = pluginFailureRetryService.scheduleRetry(paymentId, retryAttempt);
return isScheduledForRetry ? PaymentStatus.PLUGIN_FAILURE : PaymentStatus.PLUGIN_FAILURE_ABORTED;
}
diff --git a/payment/src/main/java/com/ning/billing/payment/glue/PaymentModule.java b/payment/src/main/java/com/ning/billing/payment/glue/PaymentModule.java
index eee9302..c559720 100644
--- a/payment/src/main/java/com/ning/billing/payment/glue/PaymentModule.java
+++ b/payment/src/main/java/com/ning/billing/payment/glue/PaymentModule.java
@@ -51,7 +51,7 @@ public class PaymentModule extends AbstractModule {
public final static String PLUGIN_EXECUTOR_NAMED = "PluginExecutor";
- private final Properties props;
+ protected final Properties props;
public PaymentModule() {
this.props = System.getProperties();
diff --git a/payment/src/main/java/com/ning/billing/payment/retry/PluginFailureRetryService.java b/payment/src/main/java/com/ning/billing/payment/retry/PluginFailureRetryService.java
index f333170..d3e9c8a 100644
--- a/payment/src/main/java/com/ning/billing/payment/retry/PluginFailureRetryService.java
+++ b/payment/src/main/java/com/ning/billing/payment/retry/PluginFailureRetryService.java
@@ -92,13 +92,16 @@ public class PluginFailureRetryService extends BaseRetryService implements Retry
return scheduleRetryFromTransaction(paymentId, nextRetryDate, transactionalDao);
}
- private DateTime getNextRetryDate(int retryAttempt) {
- if (retryAttempt > config.getPaymentFailureRetryMaxAttempts()) {
+ private DateTime getNextRetryDate(final int retryAttempt) {
+
+
+ if (retryAttempt > config.getPluginFailureRetryMaxAttempts()) {
return null;
}
- int nbSec = config.getPaymentFailureRetryStart();
- while (--retryAttempt > 0) {
- nbSec = nbSec * config.getPaymentFailureRetryMultiplier();
+ int nbSec = config.getPluginFailureRetryStart();
+ int remainingAttempts = retryAttempt;
+ while (--remainingAttempts > 0) {
+ nbSec = nbSec * config.getPluginFailureRetryMultiplier();
}
return clock.getUTCNow().plusSeconds(nbSec);
}
diff --git a/payment/src/test/java/com/ning/billing/payment/glue/PaymentTestModuleWithMocks.java b/payment/src/test/java/com/ning/billing/payment/glue/PaymentTestModuleWithMocks.java
index c2fdc8c..15d0cf6 100644
--- a/payment/src/test/java/com/ning/billing/payment/glue/PaymentTestModuleWithMocks.java
+++ b/payment/src/test/java/com/ning/billing/payment/glue/PaymentTestModuleWithMocks.java
@@ -16,13 +16,16 @@
package com.ning.billing.payment.glue;
+import static org.testng.Assert.assertNotNull;
+
+import java.io.IOException;
+import java.net.URL;
+import java.util.Properties;
+
import org.apache.commons.collections.MapUtils;
import com.google.common.collect.ImmutableMap;
-import com.google.inject.Provider;
import com.ning.billing.config.PaymentConfig;
-import com.ning.billing.junction.api.BillingApi;
-import com.ning.billing.mock.BrainDeadProxyFactory;
import com.ning.billing.mock.glue.MockInvoiceModule;
import com.ning.billing.mock.glue.MockNotificationQueueModule;
import com.ning.billing.mock.glue.TestDbiModule;
@@ -34,7 +37,7 @@ import com.ning.billing.util.globallocker.GlobalLocker;
import com.ning.billing.util.globallocker.MockGlobalLocker;
import com.ning.billing.util.glue.BusModule;
import com.ning.billing.util.glue.BusModule.BusType;
-import com.ning.billing.util.glue.GlobalLockerModule;
+
import com.ning.billing.util.glue.TagStoreModule;
public class PaymentTestModuleWithMocks extends PaymentModule {
@@ -48,7 +51,17 @@ public class PaymentTestModuleWithMocks extends PaymentModule {
}
*/
+ private void loadSystemPropertiesFromClasspath(final String resource) {
+ final URL url = PaymentTestModuleWithMocks.class.getResource(resource);
+ assertNotNull(url);
+ try {
+ props.load(url.openStream());
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
public PaymentTestModuleWithMocks() {
super(MapUtils.toProperties(ImmutableMap.of("killbill.payment.provider.default", "my-mock",
"killbill.payment.engine.events.off", "false")));
@@ -66,6 +79,7 @@ public class PaymentTestModuleWithMocks extends PaymentModule {
@Override
protected void configure() {
+ loadSystemPropertiesFromClasspath("/payment.properties");
super.configure();
install(new BusModule(BusType.MEMORY));
install(new MockNotificationQueueModule());
diff --git a/payment/src/test/java/com/ning/billing/payment/TestRetryService.java b/payment/src/test/java/com/ning/billing/payment/TestRetryService.java
index d083c9c..a32330d 100644
--- a/payment/src/test/java/com/ning/billing/payment/TestRetryService.java
+++ b/payment/src/test/java/com/ning/billing/payment/TestRetryService.java
@@ -63,6 +63,7 @@ import com.ning.billing.payment.glue.PaymentTestModuleWithMocks;
import com.ning.billing.payment.provider.MockPaymentProviderPlugin;
import com.ning.billing.payment.provider.PaymentProviderPluginRegistry;
import com.ning.billing.payment.retry.FailedPaymentRetryService;
+import com.ning.billing.payment.retry.PluginFailureRetryService;
import com.ning.billing.util.bus.Bus;
import com.ning.billing.util.clock.ClockMock;
import com.ning.billing.util.glue.CallContextModule;
@@ -83,6 +84,8 @@ public class TestRetryService {
private PaymentProviderPluginRegistry registry;
@Inject
private FailedPaymentRetryService retryService;
+ @Inject
+ private PluginFailureRetryService pluginRetryService;
@Inject
private ClockMock clock;
@@ -98,6 +101,9 @@ public class TestRetryService {
@BeforeMethod(groups = "fast")
public void setUp() throws Exception {
+ pluginRetryService.initialize(DefaultPaymentService.SERVICE_NAME);
+ pluginRetryService.start();
+
retryService.initialize(DefaultPaymentService.SERVICE_NAME);
retryService.start();
eventBus.start();
@@ -113,6 +119,7 @@ public class TestRetryService {
@AfterMethod(groups = "fast")
public void tearDown() throws Exception {
retryService.stop();
+ pluginRetryService.stop();
eventBus.stop();
}
@@ -127,22 +134,38 @@ public class TestRetryService {
@Test(groups = "fast")
+ public void testFailedPluginWithOneSuccessfulRetry() throws Exception {
+ testSchedulesRetryInternal(1, FailureType.PLUGIN_EXCEPTION);
+ }
+
+ @Test(groups = "fast")
+ public void testFailedPpluginWithLastRetrySuccess() throws Exception {
+ testSchedulesRetryInternal(paymentConfig.getPluginFailureRetryMaxAttempts(), FailureType.PLUGIN_EXCEPTION);
+ }
+
+ @Test(groups = "fast")
+ public void testAbortedPlugin() throws Exception {
+ testSchedulesRetryInternal(paymentConfig.getPluginFailureRetryMaxAttempts() + 1, FailureType.PLUGIN_EXCEPTION);
+ }
+
+
+ @Test(groups = "fast")
public void testFailedPaymentWithOneSuccessfulRetry() throws Exception {
- testSchedulesRetryInternal(1);
+ testSchedulesRetryInternal(1, FailureType.PAYMENT_FAILURE);
}
@Test(groups = "fast")
public void testFailedPaymentWithLastRetrySuccess() throws Exception {
- testSchedulesRetryInternal(paymentConfig.getPaymentRetryDays().size());
+ testSchedulesRetryInternal(paymentConfig.getPaymentRetryDays().size(), FailureType.PAYMENT_FAILURE);
}
@Test(groups = "fast")
public void testAbortedPayment() throws Exception {
- testSchedulesRetryInternal(paymentConfig.getPaymentRetryDays().size() + 1);
+ testSchedulesRetryInternal(paymentConfig.getPaymentRetryDays().size() + 1, FailureType.PAYMENT_FAILURE);
}
- private void testSchedulesRetryInternal(int maxTries) throws Exception {
+ private void testSchedulesRetryInternal(int maxTries, final FailureType failureType) throws Exception {
final Account account = testHelper.createTestCreditCardAccount();
final Invoice invoice = testHelper.createTestInvoice(account, clock.getUTCNow(), Currency.USD);
@@ -162,8 +185,7 @@ public class TestRetryService {
amount,
new BigDecimal("1.0"),
Currency.USD));
-
- mockPaymentProviderPlugin.makeNextPaymentFailWithError();
+ setPaymentFailure(failureType);
boolean failed = false;
try {
paymentProcessor.createPayment(account.getExternalKey(), invoice.getId(), amount, context, false);
@@ -172,20 +194,15 @@ public class TestRetryService {
}
assertTrue(failed);
-
- //int maxTries = paymentAborted ? paymentConfig.getPaymentRetryDays().size() + 1 : 1;
-
for (int curFailure = 0; curFailure < maxTries; curFailure++) {
if (curFailure < maxTries - 1) {
- mockPaymentProviderPlugin.makeNextPaymentFailWithError();
+ setPaymentFailure(failureType);
}
- if (curFailure < paymentConfig.getPaymentRetryDays().size()) {
+ if (curFailure < getMaxRetrySizeForFailureType(failureType)) {
- int nbDays = paymentConfig.getPaymentRetryDays().get(curFailure);
- clock.addDays(nbDays + 1);
-
+ moveClockForFailureType(failureType, curFailure);
try {
await().atMost(3, SECONDS).until(new Callable<Boolean>() {
@Override
@@ -201,12 +218,11 @@ public class TestRetryService {
}
}
}
-
-
Payment payment = getPaymentForInvoice(invoice.getId());
List<PaymentAttempt> attempts = payment.getAttempts();
- int expectedAttempts = maxTries < paymentConfig.getPaymentRetryDays().size() ? maxTries + 1 : paymentConfig.getPaymentRetryDays().size() + 1;
+ int expectedAttempts = maxTries < getMaxRetrySizeForFailureType(failureType) ?
+ maxTries + 1 : getMaxRetrySizeForFailureType(failureType) + 1;
assertEquals(attempts.size(), expectedAttempts);
Collections.sort(attempts, new Comparator<PaymentAttempt>() {
@Override
@@ -218,14 +234,54 @@ public class TestRetryService {
for (int i = 0; i < attempts.size(); i++) {
PaymentAttempt cur = attempts.get(i);
if (i < attempts.size() - 1) {
- assertEquals(cur.getPaymentStatus(), PaymentStatus.PAYMENT_FAILURE);
- } else if (maxTries <= paymentConfig.getPaymentRetryDays().size()) {
+ if (failureType == FailureType.PAYMENT_FAILURE) {
+ assertEquals(cur.getPaymentStatus(), PaymentStatus.PAYMENT_FAILURE);
+ } else {
+ assertEquals(cur.getPaymentStatus(), PaymentStatus.PLUGIN_FAILURE);
+ }
+ } else if (maxTries <= getMaxRetrySizeForFailureType(failureType)) {
assertEquals(cur.getPaymentStatus(), PaymentStatus.SUCCESS);
assertEquals(payment.getPaymentStatus(), PaymentStatus.SUCCESS);
} else {
- assertEquals(cur.getPaymentStatus(), PaymentStatus.PAYMENT_FAILURE_ABORTED);
- assertEquals(payment.getPaymentStatus(), PaymentStatus.PAYMENT_FAILURE_ABORTED);
+ if (failureType == FailureType.PAYMENT_FAILURE) {
+ assertEquals(cur.getPaymentStatus(), PaymentStatus.PAYMENT_FAILURE_ABORTED);
+ assertEquals(payment.getPaymentStatus(), PaymentStatus.PAYMENT_FAILURE_ABORTED);
+ } else {
+ assertEquals(cur.getPaymentStatus(), PaymentStatus.PLUGIN_FAILURE_ABORTED);
+ assertEquals(payment.getPaymentStatus(), PaymentStatus.PLUGIN_FAILURE_ABORTED);
+ }
}
}
}
+
+ private enum FailureType {
+ PLUGIN_EXCEPTION,
+ PAYMENT_FAILURE
+ }
+
+ private void setPaymentFailure(FailureType failureType) {
+ if (failureType == FailureType.PAYMENT_FAILURE) {
+ mockPaymentProviderPlugin.makeNextPaymentFailWithError();
+ } else if (failureType == FailureType.PLUGIN_EXCEPTION) {
+ mockPaymentProviderPlugin.makeNextPaymentFailWithException();
+ }
+ }
+
+ private void moveClockForFailureType(FailureType failureType, int curFailure) {
+ if (failureType == FailureType.PAYMENT_FAILURE) {
+ int nbDays = paymentConfig.getPaymentRetryDays().get(curFailure);
+ clock.addDays(nbDays + 1);
+ } else {
+ clock.addDays(1);
+ }
+ }
+
+ private int getMaxRetrySizeForFailureType(FailureType failureType) {
+ if (failureType == FailureType.PAYMENT_FAILURE) {
+ return paymentConfig.getPaymentRetryDays().size();
+ } else {
+ return paymentConfig.getPluginFailureRetryMaxAttempts();
+ }
+ }
+
}
diff --git a/payment/src/test/resources/payment.properties b/payment/src/test/resources/payment.properties
new file mode 100644
index 0000000..8a11d90
--- /dev/null
+++ b/payment/src/test/resources/payment.properties
@@ -0,0 +1,4 @@
+killbill.payment.failure.retry.start.sec=3600
+killbill.payment.failure.retry.multiplier=1
+killbill.payment.failure.retry.max.attempts=3
+
