killbill-memoizeit

diff --git a/payment/src/main/java/org/killbill/billing/payment/control/InvoicePaymentControlPluginApi.java b/payment/src/main/java/org/killbill/billing/payment/control/InvoicePaymentControlPluginApi.java
index 8d1ee9b..dbfe997 100644
--- a/payment/src/main/java/org/killbill/billing/payment/control/InvoicePaymentControlPluginApi.java
+++ b/payment/src/main/java/org/killbill/billing/payment/control/InvoicePaymentControlPluginApi.java
@@ -262,7 +262,9 @@ public final class InvoicePaymentControlPluginApi implements PaymentControlPlugi
         if (payment == null) {
             throw new PaymentControlApiException();
         }
-        // STEPH this check for invoice item but we also need to check that refundAmount is less or equal to paymentAmount - all refund.
+        // This will calculate the upper bound on the refund amount based on the invoice items associated with that payment.
+        // Note that we are not checking that other (partial) refund occurred, but if the refund ends up being greater than waht is allowed
+        // the call to the gateway would fail; it would need noce to validate on our side though...
         final BigDecimal amountToBeRefunded = computeRefundAmount(payment.getId(), paymentControlPluginContext.getAmount(), idWithAmount, internalContext);
         final boolean isAborted = amountToBeRefunded.compareTo(BigDecimal.ZERO) == 0;
 
@@ -303,7 +305,6 @@ public final class InvoicePaymentControlPluginApi implements PaymentControlPlugi
             return specifiedRefundAmount;
         }
 
-        // If we have
         final List<InvoiceItem> items;
         try {
             items = invoiceApi.getInvoiceForPaymentId(paymentId, context).getInvoiceItems();

diff --git a/payment/src/main/java/org/killbill/billing/payment/core/PaymentMethodProcessor.java b/payment/src/main/java/org/killbill/billing/payment/core/PaymentMethodProcessor.java
index 484b60c..69ff4a7 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/PaymentMethodProcessor.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/PaymentMethodProcessor.java
@@ -112,7 +112,6 @@ public class PaymentMethodProcessor extends ProcessorBase {
                         }
                     } catch (final PaymentPluginApiException e) {
                         log.warn("Error adding payment method " + pm.getId() + " for plugin " + paymentPluginServiceName, e);
-                        // STEPH all errors should also take a pluginName
                         throw new PaymentApiException(ErrorCode.PAYMENT_ADD_PAYMENT_METHOD, account.getId(), e.getErrorMessage());
                     } catch (final AccountApiException e) {
                         throw new PaymentApiException(e);

diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentOperation.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentOperation.java
index be3da35..786171c 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentOperation.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentOperation.java
@@ -89,8 +89,6 @@ public abstract class PaymentOperation extends OperationCallbackBase implements 
             logger.error(String.format(format), e);
             return new OperationException(realException, OperationResult.FAILURE);
         } else /* if (e instanceof RuntimeException) */ {
-
-     // STEPH: should we ever return an OperationResult.EXCEPTION at this layer -- since there is transtion back to init and there cannot be retried?
             logger.warn("Plugin call threw an exception for account {}", paymentStateContext.getAccount().getExternalKey(), e);
             return new OperationException(realException, OperationResult.EXCEPTION);
         }

diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/PluginControlledPaymentAutomatonRunner.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/PluginControlledPaymentAutomatonRunner.java
index d980903..ec4280c 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/PluginControlledPaymentAutomatonRunner.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/PluginControlledPaymentAutomatonRunner.java
@@ -110,6 +110,7 @@ public class PluginControlledPaymentAutomatonRunner extends PaymentAutomatonRunn
             throw new PaymentApiException(e.getCause(), ErrorCode.PAYMENT_INTERNAL_ERROR, Objects.firstNonNull(e.getMessage(), ""));
         } catch (OperationException e) {
             if (e.getCause() == null) {
+                // Unclear if we should check whether there is a result that was set and return that result.
                 throw new PaymentApiException(e, ErrorCode.PAYMENT_INTERNAL_ERROR, Objects.firstNonNull(e.getMessage(), ""));
             } else if (e.getCause() instanceof PaymentApiException) {
                 throw (PaymentApiException) e.getCause();

diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/RetryLeavingStateCallback.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/RetryLeavingStateCallback.java
index 0369883..4a4b2ed 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/RetryLeavingStateCallback.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/RetryLeavingStateCallback.java
@@ -77,9 +77,7 @@ public class RetryLeavingStateCallback implements LeavingStateCallback {
 
                 retryablePaymentAutomatonRunner.paymentDao.insertPaymentAttemptWithProperties(attempt, stateContext.internalCallContext);
                 stateContext.setAttemptId(attempt.getId());
-
             } catch (PluginPropertySerializerException e) {
-                // STEPH
                 throw new OperationException(e);
             }
 

diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/RetryOperationCallback.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/RetryOperationCallback.java
index d71cdc3..3718fd5 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/RetryOperationCallback.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/RetryOperationCallback.java
@@ -138,12 +138,7 @@ public abstract class RetryOperationCallback extends OperationCallbackBase imple
                         onCompletion(retryablePaymentStateContext.getPluginName(), updatedPaymentControlContext);
                         return OperationResult.SUCCESS;
                     } else {
-                        // Return an ABORTED/FAILURE state based on the retry result.
-                        //return getOperationResultAndSetContext(retryablePaymentStateContext, paymentControlContext);
-
-                        // STEPH Do we actually want the purchase call to fail with an exception ?
                         throw new OperationException(null, getOperationResultAndSetContext(retryablePaymentStateContext, paymentControlContext));
-
                     }
                 } catch (PaymentApiException e) {
                     // Wrap PaymentApiException, and throw a new OperationException with an ABORTED/FAILURE state based on the retry result.

diff --git a/payment/src/main/java/org/killbill/billing/payment/retry/DefaultRetryService.java b/payment/src/main/java/org/killbill/billing/payment/retry/DefaultRetryService.java
index d120a29..f08f43a 100644
--- a/payment/src/main/java/org/killbill/billing/payment/retry/DefaultRetryService.java
+++ b/payment/src/main/java/org/killbill/billing/payment/retry/DefaultRetryService.java
@@ -43,10 +43,6 @@ public class DefaultRetryService extends BaseRetryService implements RetryServic
         return QUEUE_NAME;
     }
 
-    @Override
-    public void retry(final UUID paymentId, final Iterable<PluginProperty> properties, final InternalCallContext context) {
-
-    }
 
     @Override
     public void retryPaymentTransaction(final UUID attemptId, final String pluginName, final InternalCallContext context) {

diff --git a/payment/src/main/java/org/killbill/billing/payment/retry/RetryService.java b/payment/src/main/java/org/killbill/billing/payment/retry/RetryService.java
index 934fd03..9864e0a 100644
--- a/payment/src/main/java/org/killbill/billing/payment/retry/RetryService.java
+++ b/payment/src/main/java/org/killbill/billing/payment/retry/RetryService.java
@@ -35,8 +35,5 @@ public interface RetryService {
 
     public String getQueueName();
 
-    // STEPH_RETRY API disappear
-    public void retry(UUID paymentId, final Iterable<PluginProperty> properties, final InternalCallContext context);
-
     public void retryPaymentTransaction(final UUID attemptId, String pluginName, final InternalCallContext context);
 }

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 f6aae85..bd13896 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
@@ -66,27 +66,6 @@ public class TestPluginOperation extends PaymentTestSuiteNoDB {
         testLocking(true);
     }
 
-    // STEPH the test now fails because the logic has been changed; we don't check in the dispatchWithAccountLockAndTimeout
-    // method to see whether account should be locked or not. Instead we either (dispatch AND lock) OR
-    // ! (dispatch AND lock)
-    @Test(groups = "fast", enabled = false)
-    public void testWithoutAccountLock() throws Exception {
-        testLocking(false);
-    }
-
-    @Test(groups = "fast")
-    public void testOperationTimeout() throws Exception {
-        final CallbackTest callback = new CallbackTest(Integer.MAX_VALUE);
-        final PaymentOperation pluginOperation = getPluginOperation(false, 1);
-
-        try {
-            pluginOperation.dispatchWithAccountLockAndTimeout(callback);
-            Assert.fail();
-        } catch (final OperationException e) {
-            Assert.assertEquals(e.getOperationResult(), OperationResult.EXCEPTION);
-            Assert.assertTrue(e.getCause() instanceof TimeoutException);
-        }
-    }
 
     @Test(groups = "fast")
     public void testOperationThrowsPaymentApiException() throws Exception {

diff --git a/payment/src/test/java/org/killbill/billing/payment/provider/MockPaymentProviderPlugin.java b/payment/src/test/java/org/killbill/billing/payment/provider/MockPaymentProviderPlugin.java
index 0b29446..28f90ed 100644
--- a/payment/src/test/java/org/killbill/billing/payment/provider/MockPaymentProviderPlugin.java
+++ b/payment/src/test/java/org/killbill/billing/payment/provider/MockPaymentProviderPlugin.java
@@ -244,18 +244,6 @@ public class MockPaymentProviderPlugin implements NoOpPaymentPluginApi {
 
     @Override
     public Pagination<PaymentTransactionInfoPlugin> searchPayments(final String searchKey, final Long offset, final Long limit, final Iterable<PluginProperty> properties, final TenantContext tenantContext) throws PaymentPluginApiException {
-/*
-        final ImmutableList<PaymentTransactionInfoPlugin> results = ImmutableList.<PaymentTransactionInfoPlugin>copyOf(Iterables.<PaymentTransactionInfoPlugin>filter(payments.values(), new Predicate<PaymentTransactionInfoPlugin>() {
-            @Override
-            public boolean apply(final PaymentTransactionInfoPlugin input) {
-                return (input.getKbPaymentId() != null && input.getKbPaymentId().toString().equals(searchKey)) ||
-                       (input.getFirstPaymentReferenceId() != null && input.getFirstPaymentReferenceId().contains(searchKey)) ||
-                       (input.getSecondPaymentReferenceId() != null && input.getSecondPaymentReferenceId().contains(searchKey));
-            }
-        }));
-        return DefaultPagination.<PaymentTransactionInfoPlugin>build(offset, limit, results);
-        */
-        // STEPH
         throw new IllegalStateException("Not implemented");
     }
 

diff --git a/payment/src/test/java/org/killbill/billing/payment/provider/TestExternalPaymentProviderPlugin.java b/payment/src/test/java/org/killbill/billing/payment/provider/TestExternalPaymentProviderPlugin.java
index 65d9791..3241ea5 100644
--- a/payment/src/test/java/org/killbill/billing/payment/provider/TestExternalPaymentProviderPlugin.java
+++ b/payment/src/test/java/org/killbill/billing/payment/provider/TestExternalPaymentProviderPlugin.java
@@ -64,7 +64,7 @@ public class TestExternalPaymentProviderPlugin extends PaymentTestSuiteNoDB {
         Assert.assertEquals(paymentInfoPlugin.getStatus(), PaymentPluginStatus.PROCESSED);
 
         final List<PaymentTransactionInfoPlugin> retrievedPaymentTransactionInfoPlugin = plugin.getPaymentInfo(accountId, paymentId, properties, callContext);
-        // STEPH getPaymentInfo mock is not implemented (yet)
+        // getPaymentInfo mock is not implemented (yet)
         //Assert.assertEquals(retrievedPaymentTransactionInfoPlugin.get(0).getStatus(), PaymentPluginStatus.PROCESSED);
     }
 }