killbill-memoizeit

payment: review ErrorCodes in PaymentProcessor This fixes

7/5/2016 2:43:16 PM

Details

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 3048539..f45ee2d 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
@@ -454,7 +454,7 @@ public class PaymentMethodProcessor extends ProcessorBase {
                     }
 
                     if (!paymentMethodModel.getAccountId().equals(account.getId())) {
-                        throw new PaymentApiException(ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID, paymentMethodId);
+                        throw new PaymentApiException(ErrorCode.PAYMENT_METHOD_DIFFERENT_ACCOUNT_ID, paymentMethodId);
                     }
 
                     try {
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/PaymentProcessor.java b/payment/src/main/java/org/killbill/billing/payment/core/PaymentProcessor.java
index 4df2541..43eb3bf 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/PaymentProcessor.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/PaymentProcessor.java
@@ -34,6 +34,7 @@ import javax.inject.Inject;
 import org.killbill.automaton.OperationResult;
 import org.killbill.billing.ErrorCode;
 import org.killbill.billing.account.api.Account;
+import org.killbill.billing.account.api.AccountApiException;
 import org.killbill.billing.account.api.AccountInternalApi;
 import org.killbill.billing.callcontext.InternalCallContext;
 import org.killbill.billing.callcontext.InternalTenantContext;
@@ -370,8 +371,7 @@ public class PaymentProcessor extends ProcessorBase {
 
             // Sanity: verify the payment belongs to the right account (in case it was looked-up by payment or transaction external key)
             if (!paymentModelDao.getAccountRecordId().equals(internalCallContext.getAccountRecordId())) {
-                // TODO 0.17.x New ErrorCode (it's not necessarily the transaction external key that matches)
-                throw new PaymentApiException(ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS, paymentStateContext.getPaymentTransactionExternalKey());
+                throw new PaymentApiException(ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID, paymentStateContext.getPaymentId());
             }
 
             if (paymentStateContext.getTransactionId() != null || paymentStateContext.getPaymentTransactionExternalKey() != null) {
@@ -455,7 +455,14 @@ public class PaymentProcessor extends ProcessorBase {
             // Sanity: don't share keys across accounts
             if (paymentTransactionModelDao.getTransactionExternalKey().equals(paymentStateContext.getPaymentTransactionExternalKey()) &&
                 !paymentTransactionModelDao.getAccountRecordId().equals(internalCallContext.getAccountRecordId())) {
-                throw new PaymentApiException(ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS, paymentStateContext.getPaymentTransactionExternalKey());
+                UUID accountId;
+                try {
+                    accountId = accountInternalApi.getAccountByRecordId(paymentModelDao.getAccountRecordId(), internalCallContext).getId();
+                } catch (final AccountApiException e) {
+                    log.warn("Unable to retrieve account", e);
+                    accountId = null;
+                }
+                throw new PaymentApiException(ErrorCode.PAYMENT_TRANSACTION_DIFFERENT_ACCOUNT_ID, accountId);
             }
 
             // UNKNOWN transactions are potential candidates, we'll invoke the Janitor first though
diff --git a/payment/src/test/java/org/killbill/billing/payment/api/TestPaymentApi.java b/payment/src/test/java/org/killbill/billing/payment/api/TestPaymentApi.java
index d455610..1b24f8b 100644
--- a/payment/src/test/java/org/killbill/billing/payment/api/TestPaymentApi.java
+++ b/payment/src/test/java/org/killbill/billing/payment/api/TestPaymentApi.java
@@ -1863,7 +1863,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             createPayment(account1, TransactionType.AUTHORIZE, null, pendingAuthorization.getExternalKey(), null, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID.getCode());
         }
 
         try {
@@ -1871,7 +1871,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             createPayment(account1, TransactionType.AUTHORIZE, null, pendingAuthorization.getExternalKey(), UUID.randomUUID().toString(), BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID.getCode());
         }
 
         try {
@@ -1879,7 +1879,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             createPayment(account1, TransactionType.AUTHORIZE, null, null, authKey, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID.getCode());
         }
 
         try {
@@ -1907,7 +1907,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             createPayment(account1, TransactionType.AUTHORIZE, null, pendingAuthorization.getExternalKey(), null, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID.getCode());
         }
 
         try {
@@ -1915,7 +1915,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             createPayment(account1, TransactionType.AUTHORIZE, null, pendingAuthorization.getExternalKey(), UUID.randomUUID().toString(), BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID.getCode());
         }
 
         try {
@@ -1923,7 +1923,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             createPayment(account1, TransactionType.AUTHORIZE, null, null, authKey, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID.getCode());
         }
 
         // Capture with a different transaction external key should go through
@@ -1946,7 +1946,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             createPayment(account1, TransactionType.AUTHORIZE, null, null, captureKey, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID.getCode());
         }
 
         // Second capture with the same transaction external key should go through (completion)
@@ -2010,7 +2010,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             createPayment(account1, TransactionType.AUTHORIZE, null, authorization.getExternalKey(), UUID.randomUUID().toString(), BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID.getCode());
         }
 
         try {
@@ -2018,7 +2018,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             createPayment(account1, TransactionType.AUTHORIZE, null, authorization.getExternalKey(), UUID.randomUUID().toString(), BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID.getCode());
         }
 
         try {
@@ -2026,7 +2026,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             createPayment(account1, TransactionType.AUTHORIZE, null, null, authKey, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID.getCode());
         }
 
         // Capture with a different transaction external key should go through
@@ -2057,7 +2057,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             createPayment(account1, TransactionType.AUTHORIZE, null, null, captureKey, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID.getCode());
         }
 
         // Second capture with a different transaction external key should go through
@@ -2083,7 +2083,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             createPayment(account1, TransactionType.AUTHORIZE, null, failedAuthorization1.getExternalKey(), null, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID.getCode());
         }
 
         try {
@@ -2091,7 +2091,7 @@ public class TestPaymentApi extends PaymentTestSuiteWithEmbeddedDB {
             createPayment(account1, TransactionType.AUTHORIZE, null, null, authKey, BigDecimal.TEN, PaymentPluginStatus.PROCESSED);
             Assert.fail();
         } catch (final PaymentApiException e) {
-            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_ACTIVE_TRANSACTION_KEY_EXISTS.getCode());
+            Assert.assertEquals(e.getCode(), ErrorCode.PAYMENT_DIFFERENT_ACCOUNT_ID.getCode());
         }
 
         // Different auth with the same payment external key should go through

pom.xml 2(+1 -1)

diff --git a/pom.xml b/pom.xml
index f68b817..ca27368 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.108</version>
+        <version>0.109</version>
     </parent>
     <artifactId>killbill</artifactId>
     <version>0.17.1-SNAPSHOT</version>