killbill-memoizeit
Changes
payment/src/main/java/org/killbill/billing/payment/core/janitor/IncompletePaymentTransactionTask.java 32(+23 -9)
payment/src/test/java/org/killbill/billing/payment/core/janitor/TestIncompletePaymentTransactionTask.java 21(+9 -12)
payment/src/test/java/org/killbill/billing/payment/core/janitor/TestIncompletePaymentTransactionTaskWithDB.java 4(+2 -2)
pom.xml 2(+1 -1)
Details
diff --git a/payment/src/main/java/org/killbill/billing/payment/config/MultiTenantPaymentConfig.java b/payment/src/main/java/org/killbill/billing/payment/config/MultiTenantPaymentConfig.java
index 6e56c52..4998de8 100644
--- a/payment/src/main/java/org/killbill/billing/payment/config/MultiTenantPaymentConfig.java
+++ b/payment/src/main/java/org/killbill/billing/payment/config/MultiTenantPaymentConfig.java
@@ -84,17 +84,31 @@ public class MultiTenantPaymentConfig extends MultiTenantConfigBase implements P
}
@Override
- public List<TimeSpan> getIncompleteTransactionsRetries() {
- return staticConfig.getIncompleteTransactionsRetries();
+ public List<TimeSpan> getUnknownTransactionsRetries() {
+ return staticConfig.getUnknownTransactionsRetries();
}
@Override
- public List<TimeSpan> getIncompleteTransactionsRetries(@Param("dummy") final InternalTenantContext tenantContext) {
- final String result = getStringTenantConfig("getIncompleteTransactionsRetries", tenantContext);
+ public List<TimeSpan> getUnknownTransactionsRetries(@Param("dummy") final InternalTenantContext tenantContext) {
+ final String result = getStringTenantConfig("getUnknownTransactionsRetries", tenantContext);
if (result != null) {
- return convertToListTimeSpan(result, "getIncompleteTransactionsRetries");
+ return convertToListTimeSpan(result, "getUnknownTransactionsRetries");
}
- return getIncompleteTransactionsRetries();
+ return getUnknownTransactionsRetries();
+ }
+
+ @Override
+ public List<TimeSpan> getPendingTransactionsRetries() {
+ return staticConfig.getPendingTransactionsRetries();
+ }
+
+ @Override
+ public List<TimeSpan> getPendingTransactionsRetries(@Param("dummy") final InternalTenantContext tenantContext) {
+ final String result = getStringTenantConfig("getPendingTransactionsRetries", tenantContext);
+ if (result != null) {
+ return convertToListTimeSpan(result, "getPendingTransactionsRetries");
+ }
+ return getPendingTransactionsRetries();
}
@Override
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 b49b5f7..88fd895 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
@@ -104,11 +104,16 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
tryToProcessNotification(notificationKey, userToken, accountRecordId, tenantRecordId);
} catch (final LockFailedException e) {
log.warn("Error locking accountRecordId='{}', will attempt to retry later", accountRecordId, e);
- insertNewNotificationForUnresolvedTransactionIfNeeded(notificationKey.getUuidKey(), notificationKey.getAttemptNumber(), userToken, accountRecordId, tenantRecordId);
+
+ final InternalTenantContext internalTenantContext = internalCallContextFactory.createInternalTenantContext(tenantRecordId, accountRecordId);
+ final PaymentTransactionModelDao paymentTransaction = paymentDao.getPaymentTransaction(notificationKey.getUuidKey(), internalTenantContext);
+ if (TRANSACTION_STATUSES_TO_CONSIDER.contains(paymentTransaction.getTransactionStatus())) {
+ insertNewNotificationForUnresolvedTransactionIfNeeded(notificationKey.getUuidKey(), paymentTransaction.getTransactionStatus(), notificationKey.getAttemptNumber(), userToken, accountRecordId, tenantRecordId);
+ }
}
}
- public void tryToProcessNotification(final JanitorNotificationKey notificationKey, final UUID userToken, final Long accountRecordId, final long tenantRecordId) throws LockFailedException {
+ private void tryToProcessNotification(final JanitorNotificationKey notificationKey, final UUID userToken, final Long accountRecordId, final long tenantRecordId) throws LockFailedException {
final InternalTenantContext internalTenantContext = internalCallContextFactory.createInternalTenantContext(tenantRecordId, accountRecordId);
tryToDoJanitorOperationWithAccountLock(new JanitorIterationCallback() {
@Override
@@ -158,7 +163,7 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
if (!TRANSACTION_STATUSES_TO_CONSIDER.contains(event.getStatus())) {
return;
}
- insertNewNotificationForUnresolvedTransactionIfNeeded(event.getPaymentTransactionId(), 0, event.getUserToken(), event.getSearchKey1(), event.getSearchKey2());
+ insertNewNotificationForUnresolvedTransactionIfNeeded(event.getPaymentTransactionId(), event.getStatus(), 0, event.getUserToken(), event.getSearchKey1(), event.getSearchKey2());
}
public boolean updatePaymentAndTransactionIfNeededWithAccountLock(final PaymentModelDao payment, final PaymentTransactionModelDao paymentTransaction, final PaymentTransactionInfoPlugin paymentTransactionInfoPlugin, final InternalTenantContext internalTenantContext) {
@@ -211,7 +216,7 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
payment.getId(), paymentTransaction.getId(), paymentTransaction.getTransactionStatus(), transactionStatus);
}
// We can't get anything interesting from the plugin...
- insertNewNotificationForUnresolvedTransactionIfNeeded(paymentTransaction.getId(), attemptNumber, userToken, internalTenantContext.getAccountRecordId(), internalTenantContext.getTenantRecordId());
+ insertNewNotificationForUnresolvedTransactionIfNeeded(paymentTransaction.getId(), transactionStatus, attemptNumber, userToken, internalTenantContext.getAccountRecordId(), internalTenantContext.getTenantRecordId());
return false;
}
@@ -219,7 +224,7 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
if (transactionStatus == paymentTransaction.getTransactionStatus()) {
log.debug("Janitor IncompletePaymentTransactionTask repairing payment {}, transaction {}, transitioning transactionStatus from {} -> {}",
payment.getId(), paymentTransaction.getId(), paymentTransaction.getTransactionStatus(), transactionStatus);
- insertNewNotificationForUnresolvedTransactionIfNeeded(paymentTransaction.getId(), attemptNumber, userToken, internalTenantContext.getAccountRecordId(), internalTenantContext.getTenantRecordId());
+ insertNewNotificationForUnresolvedTransactionIfNeeded(paymentTransaction.getId(), transactionStatus, attemptNumber, userToken, internalTenantContext.getAccountRecordId(), internalTenantContext.getTenantRecordId());
return false;
}
@@ -266,9 +271,18 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
}
@VisibleForTesting
- DateTime getNextNotificationTime(final Integer attemptNumber, final InternalTenantContext tenantContext) {
+ DateTime getNextNotificationTime(final TransactionStatus transactionStatus, final Integer attemptNumber, final InternalTenantContext tenantContext) {
+
+ final List<TimeSpan> retries;
+ if (TransactionStatus.UNKNOWN.equals(transactionStatus)) {
+ retries = paymentConfig.getUnknownTransactionsRetries(tenantContext);
+ } else if (TransactionStatus.PENDING.equals(transactionStatus)) {
+ retries = paymentConfig.getPendingTransactionsRetries(tenantContext);
+ } else {
+ retries = ImmutableList.of();
+ log.warn("Unexpected transactionStatus='{}' from janitor, ignore...", transactionStatus);
+ }
- final List<TimeSpan> retries = paymentConfig.getIncompleteTransactionsRetries(tenantContext);
if (attemptNumber > retries.size()) {
return null;
}
@@ -276,7 +290,7 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
return clock.getUTCNow().plusMillis((int) nextDelay.getMillis());
}
- private void insertNewNotificationForUnresolvedTransactionIfNeeded(final UUID paymentTransactionId, @Nullable final Integer attemptNumber, @Nullable final UUID userToken, final Long accountRecordId, final Long tenantRecordId) {
+ private void insertNewNotificationForUnresolvedTransactionIfNeeded(final UUID paymentTransactionId, final TransactionStatus transactionStatus, @Nullable final Integer attemptNumber, @Nullable final UUID userToken, final Long accountRecordId, final Long tenantRecordId) {
// When we come from a GET path, we don't want to insert a new notification
if (attemptNumber == null) {
return;
@@ -287,7 +301,7 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
// Increment value before we insert
final Integer newAttemptNumber = attemptNumber.intValue() + 1;
final NotificationEvent key = new JanitorNotificationKey(paymentTransactionId, IncompletePaymentTransactionTask.class.toString(), newAttemptNumber);
- final DateTime notificationTime = getNextNotificationTime(newAttemptNumber, tenantContext);
+ final DateTime notificationTime = getNextNotificationTime(transactionStatus, newAttemptNumber, tenantContext);
// Will be null in the GET path or when we run out opf attempts..
if (notificationTime != null) {
try {
diff --git a/payment/src/test/java/org/killbill/billing/payment/core/janitor/TestIncompletePaymentTransactionTask.java b/payment/src/test/java/org/killbill/billing/payment/core/janitor/TestIncompletePaymentTransactionTask.java
index a3d260c..c8ffe3b 100644
--- a/payment/src/test/java/org/killbill/billing/payment/core/janitor/TestIncompletePaymentTransactionTask.java
+++ b/payment/src/test/java/org/killbill/billing/payment/core/janitor/TestIncompletePaymentTransactionTask.java
@@ -19,6 +19,7 @@ package org.killbill.billing.payment.core.janitor;
import org.joda.time.DateTime;
import org.killbill.billing.payment.PaymentTestSuiteNoDB;
+import org.killbill.billing.payment.api.TransactionStatus;
import org.testng.annotations.Test;
import com.google.inject.Inject;
@@ -36,31 +37,27 @@ public class TestIncompletePaymentTransactionTask extends PaymentTestSuiteNoDB {
public void testGetNextNotificationTime() {
final DateTime initTime = clock.getUTCNow();
- // Based on config "15s,1m,3m,1h,1d,1d,1d,1d,1d"
- for (int i = 1; i < 10; i++) {
- final DateTime nextTime = incompletePaymentTransactionTask.getNextNotificationTime(i, internalCallContext);
+ // Based on config "5m,1h,1d,1d,1d,1d,1d"
+ for (int i = 1; i < 8; i++) {
+ final DateTime nextTime = incompletePaymentTransactionTask.getNextNotificationTime(TransactionStatus.UNKNOWN, i, internalCallContext);
assertNotNull(nextTime);
assertTrue(nextTime.compareTo(initTime) > 0);
if (i == 0) {
- assertTrue(nextTime.compareTo(initTime.plusSeconds(3).plusSeconds(1)) < 0);
+ assertTrue(nextTime.compareTo(initTime.plusMinutes(5).plusSeconds(1)) < 0);
} else if (i == 1) {
- assertTrue(nextTime.compareTo(initTime.plusMinutes(1).plusSeconds(1)) < 0);
+ assertTrue(nextTime.compareTo(initTime.plusHours(1).plusSeconds(1)) < 0);
} else if (i == 2) {
- assertTrue(nextTime.compareTo(initTime.plusMinutes(3).plusSeconds(1)) < 0);
+ assertTrue(nextTime.compareTo(initTime.plusDays(1).plusSeconds(1)) < 0);
} else if (i == 3) {
- assertTrue(nextTime.compareTo(initTime.plusHours(1).plusSeconds(1)) < 0);
+ assertTrue(nextTime.compareTo(initTime.plusDays(1).plusSeconds(1)) < 0);
} else if (i == 4) {
assertTrue(nextTime.compareTo(initTime.plusDays(1).plusSeconds(1)) < 0);
} else if (i == 5) {
assertTrue(nextTime.compareTo(initTime.plusDays(1).plusSeconds(1)) < 0);
} else if (i == 6) {
assertTrue(nextTime.compareTo(initTime.plusDays(1).plusSeconds(1)) < 0);
- } else if (i == 7) {
- assertTrue(nextTime.compareTo(initTime.plusDays(1).plusSeconds(1)) < 0);
- } else if (i == 8) {
- assertTrue(nextTime.compareTo(initTime.plusDays(1).plusSeconds(1)) < 0);
}
}
- assertNull(incompletePaymentTransactionTask.getNextNotificationTime(10, internalCallContext));
+ assertNull(incompletePaymentTransactionTask.getNextNotificationTime(TransactionStatus.UNKNOWN, 8, internalCallContext));
}
}
diff --git a/payment/src/test/java/org/killbill/billing/payment/core/janitor/TestIncompletePaymentTransactionTaskWithDB.java b/payment/src/test/java/org/killbill/billing/payment/core/janitor/TestIncompletePaymentTransactionTaskWithDB.java
index 191ed5c..8dc40d7 100644
--- a/payment/src/test/java/org/killbill/billing/payment/core/janitor/TestIncompletePaymentTransactionTaskWithDB.java
+++ b/payment/src/test/java/org/killbill/billing/payment/core/janitor/TestIncompletePaymentTransactionTaskWithDB.java
@@ -95,8 +95,8 @@ public class TestIncompletePaymentTransactionTaskWithDB extends PaymentTestSuite
Assert.assertEquals(event.getUuidKey(), transactionId);
Assert.assertEquals((int) event.getAttemptNumber(), 2);
- // Based on config "15s,1m,3m,1h,1d,1d,1d,1d,1d"
- Assert.assertTrue(notificationEventWithMetadata.getEffectiveDate().compareTo(clock.getUTCNow().plusMinutes(1).plusSeconds(1)) < 0);
+ // Based on config "1h, 1d"
+ Assert.assertTrue(notificationEventWithMetadata.getEffectiveDate().compareTo(clock.getUTCNow().plusDays(1).plusSeconds(5)) < 0);
} catch (final LockFailedException e) {
Assert.fail();
} finally {
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 f6cd52a..03487f5 100644
--- a/payment/src/test/java/org/killbill/billing/payment/TestJanitor.java
+++ b/payment/src/test/java/org/killbill/billing/payment/TestJanitor.java
@@ -63,7 +63,6 @@ import org.skife.config.TimeSpan;
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;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.BeforeMethod;
@@ -403,9 +402,9 @@ public class TestJanitor extends PaymentTestSuiteWithEmbeddedDB {
"loup", "chat", internalCallContext);
testListener.assertListenerStatus();
- // Move clock for notification to be processed
+ // Move clock for notification to be processed ((default config is set for one hour)
testListener.pushExpectedEvent(NextEvent.PAYMENT);
- clock.addDeltaFromReality(5 * 60 * 1000);
+ clock.addDeltaFromReality(1000 * (3600 + 1));
assertNotificationsCompleted(internalCallContext, 5);
testListener.assertListenerStatus();
@@ -442,8 +441,8 @@ public class TestJanitor extends PaymentTestSuiteWithEmbeddedDB {
"loup", "chat", internalCallContext);
testListener.assertListenerStatus();
- // 15s,1m,3m,1h,1d,1d,1d,1d,1d
- for (final TimeSpan cur : paymentConfig.getIncompleteTransactionsRetries(internalCallContext)) {
+ // 1h, 1d
+ for (final TimeSpan cur : paymentConfig.getPendingTransactionsRetries(internalCallContext)) {
// Verify there is a notification to retry updating the value
assertEquals(getPendingNotificationCnt(internalCallContext), 1);
pom.xml 2(+1 -1)
diff --git a/pom.xml b/pom.xml
index 689f0a4..d5d52ae 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.140.20</version>
+ <version>0.140.21</version>
</parent>
<artifactId>killbill</artifactId>
<version>0.18.7-SNAPSHOT</version>
diff --git a/util/src/main/java/org/killbill/billing/util/config/definition/PaymentConfig.java b/util/src/main/java/org/killbill/billing/util/config/definition/PaymentConfig.java
index 97a018f..639429f 100644
--- a/util/src/main/java/org/killbill/billing/util/config/definition/PaymentConfig.java
+++ b/util/src/main/java/org/killbill/billing/util/config/definition/PaymentConfig.java
@@ -58,15 +58,26 @@ public interface PaymentConfig extends KillbillConfig {
@Description("Specify the multiplier to apply between in retry before retrying a payment that failed due to a plugin failure (gateway is down, transient error, ...)")
int getPluginFailureRetryMultiplier(@Param("dummy") final InternalTenantContext tenantContext);
- @Config("org.killbill.payment.janitor.transactions.retries")
- @Default("15s,1m,3m,1h,1d,1d,1d,1d,1d")
+ @Config("org.killbill.payment.janitor.unknown.retries")
+ @Default("5m,1h,1d,1d,1d,1d,1d")
@Description("Delay before which unresolved transactions should be retried")
- List<TimeSpan> getIncompleteTransactionsRetries();
+ List<TimeSpan> getUnknownTransactionsRetries();
- @Config("org.killbill.payment.janitor.transactions.retries")
- @Default("15s,1m,3m,1h,1d,1d,1d,1d,1d")
+ @Config("org.killbill.payment.janitor.unknown.retries")
+ @Default("5m,1h,1d,1d,1d,1d,1d")
@Description("Delay before which unresolved transactions should be retried")
- List<TimeSpan> getIncompleteTransactionsRetries(@Param("dummy") final InternalTenantContext tenantContext);
+ List<TimeSpan> getUnknownTransactionsRetries(@Param("dummy") final InternalTenantContext tenantContext);
+
+ @Config("org.killbill.payment.janitor.pending.retries")
+ @Default("1h, 1d")
+ @Description("Delay before which unresolved transactions should be retried")
+ List<TimeSpan> getPendingTransactionsRetries();
+
+ @Config("org.killbill.payment.janitor.pending.retries")
+ @Default("1h, 1d")
+ @Description("Delay before which unresolved transactions should be retried")
+ List<TimeSpan> getPendingTransactionsRetries(@Param("dummy") final InternalTenantContext tenantContext);
+
@Config("org.killbill.payment.failure.retry.max.attempts")
@Default("8")