killbill-memoizeit

Code review integration for 2a9a4b4c918ff5bb3aa491c8eb64c7ff22d6d8fd

7/14/2015 8:05:22 PM

Details

diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/control/DefaultControlCompleted.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/control/DefaultControlCompleted.java
index 3e42fee..f3daf43 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/control/DefaultControlCompleted.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/control/DefaultControlCompleted.java
@@ -40,11 +40,13 @@ public class DefaultControlCompleted implements EnteringStateCallback {
     private PluginRoutingPaymentAutomatonRunner retryablePaymentAutomatonRunner;
     private final PaymentStateControlContext paymentStateContext;
     private final RetryServiceScheduler retryServiceScheduler;
+    private final State retriedState;
 
     public DefaultControlCompleted(final PluginRoutingPaymentAutomatonRunner retryablePaymentAutomatonRunner, final PaymentStateControlContext paymentStateContext,
-                                   final RetryServiceScheduler retryServiceScheduler) {
+                                   final State retriedState, final RetryServiceScheduler retryServiceScheduler) {
         this.retryablePaymentAutomatonRunner = retryablePaymentAutomatonRunner;
         this.paymentStateContext = paymentStateContext;
+        this.retriedState = retriedState;
         this.retryServiceScheduler = retryServiceScheduler;
     }
 
@@ -56,7 +58,7 @@ public class DefaultControlCompleted implements EnteringStateCallback {
                                    null;
         retryablePaymentAutomatonRunner.getPaymentDao().updatePaymentAttempt(attempt.getId(), transactionId, state.getName(), paymentStateContext.getInternalCallContext());
 
-        if ("RETRIED".equals(state.getName()) && !isUnknownTransaction()) {
+        if (retriedState.getName().equals(state.getName()) && !isUnknownTransaction()) {
             retryServiceScheduler.scheduleRetry(ObjectType.PAYMENT_ATTEMPT, attempt.getId(), attempt.getId(), attempt.getTenantRecordId(),
                                                 paymentStateContext.getPaymentControlPluginNames(), paymentStateContext.getRetryDate());
         }
@@ -77,8 +79,7 @@ public class DefaultControlCompleted implements EnteringStateCallback {
                     return input.getTransactionStatus() == TransactionStatus.UNKNOWN &&
                            // Not strictly required
                            // (Note, we don't match on AttemptId as it is risky, the row on disk would match the first attempt, not necessarily the current one)
-                           input.getAccountRecordId() == paymentStateContext.getInternalCallContext().getAccountRecordId() &&
-                           input.getAmount() == paymentStateContext.getAmount();
+                           input.getAccountRecordId().equals(paymentStateContext.getInternalCallContext().getAccountRecordId());
                 }
             });
         }
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentLeavingStateCallback.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentLeavingStateCallback.java
index d898745..6fb17d7 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentLeavingStateCallback.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentLeavingStateCallback.java
@@ -65,21 +65,23 @@ public abstract class PaymentLeavingStateCallback implements LeavingStateCallbac
             if (paymentStateContext.getTransactionId() != null) {
                 final PaymentTransactionModelDao transactionModelDao = daoHelper.getPaymentDao().getPaymentTransaction(paymentStateContext.getTransactionId(), paymentStateContext.getInternalCallContext());
                 existingPaymentTransactions = ImmutableList.of(transactionModelDao);
-            } else {
+            } else if (paymentStateContext.getPaymentTransactionExternalKey() != null) {
                 existingPaymentTransactions = daoHelper.getPaymentDao().getPaymentTransactionsByExternalKey(paymentStateContext.getPaymentTransactionExternalKey(), paymentStateContext.getInternalCallContext());
+            } else {
+                existingPaymentTransactions = ImmutableList.of();
             }
 
             // Validate some constraints on the unicity of that paymentTransactionExternalKey
             validateUniqueTransactionExternalKey(existingPaymentTransactions);
 
-            // Handle UNKNOWN cases, where we skip the whole state machine and let the getPayment logic refresh the state.
+            // Handle UNKNOWN cases, where we skip the whole state machine and let the getPayment (through Janitor) logic refresh the state.
             final PaymentTransactionModelDao unknownPaymentTransaction = getUnknownPaymentTransaction(existingPaymentTransactions);
             if (unknownPaymentTransaction != null) {
-                // Reset the attemptId on the existing paymentTransaction row since it it is not accurate
+                // Reset the attemptId on the existing paymentTransaction row since it is not accurate
                 unknownPaymentTransaction.setAttemptId(paymentStateContext.getAttemptId());
                 // Set the current paymentTransaction in the context (needed for the state machine logic)
                 paymentStateContext.setPaymentTransactionModelDao(unknownPaymentTransaction);
-                // Set special flag to bypass the state machine altogether (plugin will not be called, state will not be updated, no event will be sent)
+                // Set special flag to bypass the state machine altogether (plugin will not be called, state will not be updated, no event will be sent unless state is fixed)
                 paymentStateContext.setSkipOperationForUnknownTransaction(true);
                 return;
             }
@@ -127,12 +129,13 @@ public abstract class PaymentLeavingStateCallback implements LeavingStateCallbac
         if (Iterables.any(existingPaymentTransactions, new Predicate<PaymentTransactionModelDao>() {
             @Override
             public boolean apply(final PaymentTransactionModelDao input) {
-                       // An existing transaction in a SUCCESS state
+                // An existing transaction in a SUCCESS state
                 return input.getTransactionStatus() == TransactionStatus.SUCCESS ||
                        // Or, an existing transaction for a different payment (to do really well, we should also check on paymentExternalKey which is not available here)
                        (paymentStateContext.getPaymentId() != null && input.getPaymentId().compareTo(paymentStateContext.getPaymentId()) != 0) ||
                        // Or, an existing transaction for a different account.
-                       (input.getAccountRecordId() != paymentStateContext.getInternalCallContext().getAccountRecordId());
+                       (!input.getAccountRecordId().equals(paymentStateContext.getInternalCallContext().getAccountRecordId()));
+
             }
         })) {
             throw new PaymentApiException(ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS, paymentStateContext.getPaymentTransactionExternalKey());
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/PluginRoutingPaymentAutomatonRunner.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/PluginRoutingPaymentAutomatonRunner.java
index 7836409..c97f8db 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/PluginRoutingPaymentAutomatonRunner.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/PluginRoutingPaymentAutomatonRunner.java
@@ -113,7 +113,7 @@ public class PluginRoutingPaymentAutomatonRunner extends PaymentAutomatonRunner 
         try {
             final OperationCallback callback = createOperationCallback(transactionType, paymentStateContext);
             final LeavingStateCallback leavingStateCallback = new DefaultControlInitiated(this, paymentStateContext, paymentDao, paymentControlStateMachineHelper.getInitialState(), paymentControlStateMachineHelper.getRetriedState(), transactionType);
-            final EnteringStateCallback enteringStateCallback = new DefaultControlCompleted(this, paymentStateContext, retryServiceScheduler);
+            final EnteringStateCallback enteringStateCallback = new DefaultControlCompleted(this, paymentStateContext, paymentControlStateMachineHelper.getRetriedState(), retryServiceScheduler);
 
             state.runOperation(paymentControlStateMachineHelper.getOperation(), callback, enteringStateCallback, leavingStateCallback);
         } catch (final MissingEntryException e) {
@@ -135,7 +135,7 @@ public class PluginRoutingPaymentAutomatonRunner extends PaymentAutomatonRunner 
         try {
             final OperationCallback callback = new CompletionControlOperation(locker, paymentPluginDispatcher, paymentStateContext, paymentProcessor, paymentControlPluginRegistry);
             final LeavingStateCallback leavingStateCallback = new NoopControlInitiated();
-            final EnteringStateCallback enteringStateCallback = new DefaultControlCompleted(this, paymentStateContext, retryServiceScheduler);
+            final EnteringStateCallback enteringStateCallback = new DefaultControlCompleted(this, paymentStateContext, paymentControlStateMachineHelper.getRetriedState(), retryServiceScheduler);
 
             paymentControlStateMachineHelper.getInitialState().runOperation(paymentControlStateMachineHelper.getOperation(), callback, enteringStateCallback, leavingStateCallback);
         } catch (final MissingEntryException e) {