killbill-memoizeit

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")