killbill-uncached

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 08688dc..ede05e3 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
@@ -112,11 +112,7 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
                     return undefinedPaymentTransaction;
                 }
             });
-        } catch (final PaymentPluginApiException ignored) {
-            // TODO STEPH: In the case of a payment call, PaymentPluginApiException means that plugin knows payment transaction WAS NOT EVEN attempted; what does it mean for the getPaymentInfo?
-            // The code below assumes the same, but that needs to be discussed
-            paymentTransactionInfoPlugin = null;
-        } catch (final RuntimeException e) {
+        } catch (final Exception e) {
             paymentTransactionInfoPlugin = undefinedPaymentTransaction;
         }
 
@@ -168,7 +164,7 @@ public class IncompletePaymentTransactionTask extends CompletionTaskBase<Payment
     }
 
     // Keep the existing currentTransactionStatus if we can't obtain a better answer from the plugin; if not, return the newTransactionStatus
-    private TransactionStatus computeNewTransactionStatusFromPaymentTransactionInfoPlugin(@Nullable PaymentTransactionInfoPlugin input, final TransactionStatus currentTransactionStatus) {
+    private TransactionStatus computeNewTransactionStatusFromPaymentTransactionInfoPlugin(PaymentTransactionInfoPlugin input, final TransactionStatus currentTransactionStatus) {
         final TransactionStatus newTransactionStatus = PaymentTransactionInfoPluginConverter.toTransactionStatus(input);
         return (newTransactionStatus != TransactionStatus.UNKNOWN) ? newTransactionStatus : currentTransactionStatus;
     }

diff --git a/payment/src/main/java/org/killbill/billing/payment/core/PaymentTransactionInfoPluginConverter.java b/payment/src/main/java/org/killbill/billing/payment/core/PaymentTransactionInfoPluginConverter.java
index 91accab..bcdc703 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/PaymentTransactionInfoPluginConverter.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/PaymentTransactionInfoPluginConverter.java
@@ -29,15 +29,7 @@ import org.killbill.billing.payment.plugin.api.PaymentTransactionInfoPlugin;
 public class PaymentTransactionInfoPluginConverter {
 
 
-    public static TransactionStatus toTransactionStatus(@Nullable final PaymentTransactionInfoPlugin paymentTransactionInfoPlugin) {
-        //
-        // A null paymentTransactionInfoPlugin means that plugin threw a PluginApiException to indicate it knew for sure
-        // the transaction did not occur, so we can safely transition into PLUGIN_FAILURE
-        //
-        if (paymentTransactionInfoPlugin == null) {
-            return TransactionStatus.PLUGIN_FAILURE;
-        }
-
+    public static TransactionStatus toTransactionStatus(final PaymentTransactionInfoPlugin paymentTransactionInfoPlugin) {
         switch (paymentTransactionInfoPlugin.getStatus()) {
             case PROCESSED:
                 return TransactionStatus.SUCCESS;
@@ -48,6 +40,10 @@ public class PaymentTransactionInfoPluginConverter {
             case ERROR:
                 return TransactionStatus.PAYMENT_FAILURE;
             //
+            // The plugin is trying to tell us that it knows for sure that payment transaction did not happen (connection failure,..)
+            case CANCELED:
+                return TransactionStatus.PLUGIN_FAILURE;
+            //
             // This will be picked up by Janitor to figure out what really happened and correct the state if needed
             // Note that the default case includes the null status
             //
@@ -57,11 +53,7 @@ public class PaymentTransactionInfoPluginConverter {
         }
     }
 
-    public static OperationResult toOperationResult(@Nullable final PaymentTransactionInfoPlugin paymentTransactionInfoPlugin) {
-        if (paymentTransactionInfoPlugin == null || paymentTransactionInfoPlugin.getStatus() == null) {
-            return OperationResult.EXCEPTION;
-        }
-
+    public static OperationResult toOperationResult(final PaymentTransactionInfoPlugin paymentTransactionInfoPlugin) {
         switch (paymentTransactionInfoPlugin.getStatus()) {
             case PROCESSED:
                 return OperationResult.SUCCESS;
@@ -69,8 +61,8 @@ public class PaymentTransactionInfoPluginConverter {
                 return OperationResult.PENDING;
             case ERROR:
                 return OperationResult.FAILURE;
-            // Might change later if janitor fixes the state
             case UNDEFINED:
+            case CANCELED:
             default:
                 return OperationResult.EXCEPTION;
         }

diff --git a/payment/src/main/java/org/killbill/billing/payment/core/ProcessorBase.java b/payment/src/main/java/org/killbill/billing/payment/core/ProcessorBase.java
index a411e2c..77a1265 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/ProcessorBase.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/ProcessorBase.java
@@ -174,8 +174,7 @@ public abstract class ProcessorBase {
         final PaymentTransactionModelDao transactionAlreadyExists = Iterables.tryFind(transactions, new Predicate<PaymentTransactionModelDao>() {
             @Override
             public boolean apply(final PaymentTransactionModelDao input) {
-                return input.getTransactionStatus() == TransactionStatus.SUCCESS ||
-                       input.getTransactionStatus() == TransactionStatus.UNKNOWN;
+                return input.getTransactionStatus() == TransactionStatus.SUCCESS;
             }
         }).orNull();
         if (transactionAlreadyExists != null) {

diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentOperation.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentOperation.java
index d0fdaf4..6781ecd 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentOperation.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/payments/PaymentOperation.java
@@ -74,30 +74,24 @@ public abstract class PaymentOperation extends OperationCallbackBase<PaymentTran
             } else {
                 return doSimpleOperationCallback();
             }
-        } catch (final PaymentApiException e) {
-            throw new OperationException(e, OperationResult.EXCEPTION);
+        } catch (final Exception e) {
+            throw convertToUnknownTransactionStatusAndErroredPaymentState(e);
         }
     }
 
     @Override
     protected OperationException rewrapExecutionException(final PaymentStateContext paymentStateContext, final ExecutionException e) {
         //
-        // There are 2 main cases to distinguish:
-        // - PaymentPluginApiException (hidden in the PaymentApiException) -> plugin is telling us transaction was not even attempted => TransactionStatus.PLUGIN_FAILURE
-        // - All other exceptions => TransactionStatus.UNKNOWN (Candidates to be fixed by Janitor)
+        // Any case of exception (checked or runtime) should lead to a TransactionStatus.UNKNOWN (and a XXX_ERRORED payment state).
+        // In order to reach that state we create PaymentTransactionInfoPlugin with an PaymentPluginStatus.UNDEFINED status (and an OperationResult.EXCEPTION).
         //
-        if (e.getCause() instanceof PaymentApiException && ((PaymentApiException)e.getCause()).getCode() == ErrorCode.PAYMENT_PLUGIN_EXCEPTION.getCode()) {
-            // We keep the PaymentTransactionInfoPlugin unset in the context to mark that specific case
-            return new OperationException(MoreObjects.firstNonNull(e.getCause(), e), OperationResult.EXCEPTION);
+        final Throwable originalExceptionOrCause = MoreObjects.firstNonNull(e.getCause(), e);
+        if (originalExceptionOrCause instanceof LockFailedException) {
+            logger.warn("Failed to lock account {}", paymentStateContext.getAccount().getExternalKey());
         } else {
-            if (e.getCause() instanceof LockFailedException) {
-                logger.warn("Failed to lock account {}", paymentStateContext.getAccount().getExternalKey());
-            } else {
-                logger.warn("Payment plugin call threw an exception for account {}", paymentStateContext.getAccount().getExternalKey(), e);
-            }
-            convertToUnknownTransactionStatusAndErroredPaymentState(e);
-            return new OperationException(MoreObjects.firstNonNull(e.getCause(), e), OperationResult.EXCEPTION);
+            logger.warn("Payment plugin call threw an exception for account {}", paymentStateContext.getAccount().getExternalKey(), originalExceptionOrCause);
         }
+        return convertToUnknownTransactionStatusAndErroredPaymentState(originalExceptionOrCause);
     }
 
     @Override
@@ -107,7 +101,7 @@ public abstract class PaymentOperation extends OperationCallbackBase<PaymentTran
     }
 
     //
-    // In the case we don't know exactly what happen (Timeout or PluginApiException):
+    // In case of exceptions, timeouts, we don't really know what happened:
     // - Return an OperationResult.EXCEPTION to transition Payment State to Errored (see PaymentTransactionInfoPluginConverter#toOperationResult)
     // - Construct a PaymentTransactionInfoPlugin whose PaymentPluginStatus = UNDEFINED to end up with a paymentTransactionStatus = UNKNOWN and have a chance to
     //   be fixed by Janitor.

diff --git a/payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentRoutingPluginApi.java b/payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentRoutingPluginApi.java
index e1b5ddf..30d5115 100644
--- a/payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentRoutingPluginApi.java
+++ b/payment/src/main/java/org/killbill/billing/payment/invoice/InvoicePaymentRoutingPluginApi.java
@@ -385,7 +385,8 @@ public final class InvoicePaymentRoutingPluginApi implements PaymentRoutingPlugi
     private DateTime getNextRetryDateForPluginFailure(final List<PaymentTransactionModelDao> purchasedTransactions) {
 
         DateTime result = null;
-        final int attemptsInState = getNumberAttemptsInState(purchasedTransactions, TransactionStatus.PLUGIN_FAILURE);
+        // Not clear whether or not we should really include TransactionStatus.UNKNOWN
+        final int attemptsInState = getNumberAttemptsInState(purchasedTransactions, TransactionStatus.PLUGIN_FAILURE, TransactionStatus.UNKNOWN);
         final int retryAttempt = (attemptsInState - 1) >= 0 ? (attemptsInState - 1) : 0;
 
         if (retryAttempt < paymentConfig.getPluginFailureRetryMaxAttempts()) {

diff --git a/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentEnteringStateCallback.java b/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentEnteringStateCallback.java
index c4f53e6..e146a6e 100644
--- a/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentEnteringStateCallback.java
+++ b/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentEnteringStateCallback.java
@@ -105,18 +105,6 @@ public class TestPaymentEnteringStateCallback extends PaymentTestSuiteWithEmbedd
         Assert.assertEquals(paymentTransaction.getGatewayErrorMsg(), paymentInfoPlugin.getGatewayError());
     }
 
-    @Test(groups = "slow")
-    public void testEnterStateWithOperationException() throws Exception {
-        daoHelper.createNewPaymentTransaction();
-        // Simulate a bug in the plugin - i.e. nothing was returned
-        paymentStateContext.setPaymentTransactionInfoPlugin(null);
-        operationResult = OperationResult.EXCEPTION;
-
-        callback.enteringState(state, operationCallback, operationResult, leavingStateCallback);
-
-        Assert.assertEquals(paymentDao.getPaymentTransaction(paymentStateContext.getPaymentTransactionModelDao().getId(), internalCallContext).getTransactionStatus(), TransactionStatus.PLUGIN_FAILURE);
-    }
-
     private static final class PaymentEnteringStateTestCallback extends PaymentEnteringStateCallback {
 
         private PaymentEnteringStateTestCallback(final PaymentAutomatonDAOHelper daoHelper, final PaymentStateContext paymentStateContext) throws PaymentApiException {

diff --git a/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentOperation.java b/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentOperation.java
index 9f6eb58..586e759 100644
--- a/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentOperation.java
+++ b/payment/src/test/java/org/killbill/billing/payment/core/sm/TestPaymentOperation.java
@@ -76,7 +76,8 @@ public class TestPaymentOperation extends PaymentTestSuiteNoDB {
             Assert.assertEquals(e.getOperationResult(), OperationResult.EXCEPTION);
         }
 
-        Assert.assertNull(paymentStateContext.getPaymentTransactionInfoPlugin());
+        Assert.assertNotNull(paymentStateContext.getPaymentTransactionInfoPlugin());
+        Assert.assertEquals(paymentStateContext.getPaymentTransactionInfoPlugin().getStatus(), PaymentPluginStatus.UNDEFINED);
     }
 
     @Test(groups = "fast")

diff --git a/pom.xml b/pom.xml
index ffe30ae..837f44f 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.14</version>
+        <version>0.15</version>
     </parent>
     <artifactId>killbill</artifactId>
     <version>0.15.0-SNAPSHOT</version>