Details
diff --git a/api/src/main/java/com/ning/billing/ErrorCode.java b/api/src/main/java/com/ning/billing/ErrorCode.java
index dd03756..8ea1ee2 100644
--- a/api/src/main/java/com/ning/billing/ErrorCode.java
+++ b/api/src/main/java/com/ning/billing/ErrorCode.java
@@ -248,7 +248,8 @@ public enum ErrorCode {
PAYMENT_NO_SUCH_REFUND(7023, "Refund %s does not exist"),
PAYMENT_NO_SUCH_SUCCESS_PAYMENT(7024, "Payment %s did not succeed"),
PAYMENT_REFUND_AMOUNT_TOO_LARGE(7025, "Refund amount if larger than payment"),
- PAYMENT_BAD_ACCOUNT(7026, "Account %s has payments left in an unknwon state"),
+ PAYMENT_REFUND_AMOUNT_NEGATIVE_OR_NULL(7026, "Refund amount needs to be strictly positive"),
+ PAYMENT_BAD_ACCOUNT(7027, "Account %s has payments left in an unknwon state"),
PAYMENT_PLUGIN_TIMEOUT(7100, "Plugin timeout for account %s and invoice %s"),
PAYMENT_PLUGIN_ACCOUNT_INIT(7101, "Account initialization for account %s and plugin % s failed: %s"),
diff --git a/invoice/src/main/java/com/ning/billing/invoice/api/invoice/DefaultInvoicePaymentApi.java b/invoice/src/main/java/com/ning/billing/invoice/api/invoice/DefaultInvoicePaymentApi.java
index dfb40b4..513dae0 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/api/invoice/DefaultInvoicePaymentApi.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/api/invoice/DefaultInvoicePaymentApi.java
@@ -24,6 +24,7 @@ import java.util.UUID;
import org.joda.time.DateTime;
import com.google.inject.Inject;
+import com.ning.billing.ErrorCode;
import com.ning.billing.catalog.api.Currency;
import com.ning.billing.invoice.api.Invoice;
import com.ning.billing.invoice.api.InvoiceApiException;
@@ -112,6 +113,9 @@ public class DefaultInvoicePaymentApi implements InvoicePaymentApi {
@Override
public InvoicePayment createRefund(final UUID paymentId, final BigDecimal amount, final boolean isInvoiceAdjusted,
final UUID paymentCookieId, final CallContext context) throws InvoiceApiException {
+ if (amount.compareTo(BigDecimal.ZERO) <= 0) {
+ throw new InvoiceApiException(ErrorCode.PAYMENT_REFUND_AMOUNT_NEGATIVE_OR_NULL);
+ }
return dao.createRefund(paymentId, amount, isInvoiceAdjusted, paymentCookieId, context);
}
}
diff --git a/invoice/src/main/java/com/ning/billing/invoice/dao/DefaultInvoiceDao.java b/invoice/src/main/java/com/ning/billing/invoice/dao/DefaultInvoiceDao.java
index 86d7bef..2dec344 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/dao/DefaultInvoiceDao.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/dao/DefaultInvoiceDao.java
@@ -305,13 +305,7 @@ public class DefaultInvoiceDao implements InvoiceDao {
throw new InvoiceApiException(ErrorCode.INVOICE_PAYMENT_BY_ATTEMPT_NOT_FOUND, paymentId);
}
final BigDecimal maxRefundAmount = payment.getAmount() == null ? BigDecimal.ZERO : payment.getAmount();
- final BigDecimal requestedAmount = amount == null ? maxRefundAmount : amount;
- if (requestedAmount.compareTo(BigDecimal.ZERO) >= 0) {
- throw new InvoiceApiException(ErrorCode.REFUND_AMOUNT_IS_POSITIVE);
- }
-
- // Now that we checked signs, let's work with positive numbers, this makes things simpler
- final BigDecimal requestedPositiveAmount = requestedAmount.negate();
+ final BigDecimal requestedPositiveAmount = amount == null ? maxRefundAmount : amount;
if (requestedPositiveAmount.compareTo(maxRefundAmount) > 0) {
throw new InvoiceApiException(ErrorCode.REFUND_AMOUNT_TOO_HIGH, requestedPositiveAmount, maxRefundAmount);
}
diff --git a/invoice/src/test/java/com/ning/billing/invoice/dao/TestInvoiceDao.java b/invoice/src/test/java/com/ning/billing/invoice/dao/TestInvoiceDao.java
index fb7bf9c..6f3594c 100644
--- a/invoice/src/test/java/com/ning/billing/invoice/dao/TestInvoiceDao.java
+++ b/invoice/src/test/java/com/ning/billing/invoice/dao/TestInvoiceDao.java
@@ -561,7 +561,7 @@ public class TestInvoiceDao extends InvoiceDaoTestBase {
final DateTime endDate = startDate.plusMonths(1);
final BigDecimal rate1 = new BigDecimal("20.0");
- final BigDecimal refund1 = new BigDecimal("-7.00");
+ final BigDecimal refund1 = new BigDecimal("7.00");
final BigDecimal rate2 = new BigDecimal("10.0");
// Recurring item
@@ -590,28 +590,28 @@ public class TestInvoiceDao extends InvoiceDaoTestBase {
@Test(groups = "slow")
public void testAccountBalanceWithSmallRefundAndCBANoAdj() throws InvoiceApiException {
- BigDecimal refundAmount = new BigDecimal("-7.00");
+ BigDecimal refundAmount = new BigDecimal("7.00");
BigDecimal expectedBalance = new BigDecimal("-3.00");
testAccountBalanceWithRefundAndCBAInternal(false, refundAmount, expectedBalance);
}
@Test(groups = "slow")
public void testAccountBalanceWithSmallRefundAndCBAWithAdj() throws InvoiceApiException {
- BigDecimal refundAmount = new BigDecimal("-7.00");
+ BigDecimal refundAmount = new BigDecimal("7.00");
BigDecimal expectedBalance = new BigDecimal("-3.00");
testAccountBalanceWithRefundAndCBAInternal(true, refundAmount, expectedBalance);
}
@Test(groups = "slow")
public void testAccountBalanceWithLargeRefundAndCBANoAdj() throws InvoiceApiException {
- BigDecimal refundAmount = new BigDecimal("-20.00");
+ BigDecimal refundAmount = new BigDecimal("20.00");
BigDecimal expectedBalance = new BigDecimal("10.00");
testAccountBalanceWithRefundAndCBAInternal(false, refundAmount, expectedBalance);
}
@Test(groups = "slow")
public void testAccountBalanceWithLargeRefundAndCBAWithAdj() throws InvoiceApiException {
- BigDecimal refundAmount = new BigDecimal("-20.00");
+ BigDecimal refundAmount = new BigDecimal("20.00");
BigDecimal expectedBalance = BigDecimal.ZERO;
testAccountBalanceWithRefundAndCBAInternal(true, refundAmount, expectedBalance);
}
diff --git a/payment/src/main/java/com/ning/billing/payment/api/DefaultPaymentApi.java b/payment/src/main/java/com/ning/billing/payment/api/DefaultPaymentApi.java
index 4968a24..1bcedf9 100644
--- a/payment/src/main/java/com/ning/billing/payment/api/DefaultPaymentApi.java
+++ b/payment/src/main/java/com/ning/billing/payment/api/DefaultPaymentApi.java
@@ -88,6 +88,9 @@ public class DefaultPaymentApi implements PaymentApi {
public Refund createRefund(Account account, UUID paymentId,
BigDecimal refundAmount, boolean isAdjusted, CallContext context)
throws PaymentApiException {
+ if (refundAmount == null || refundAmount.compareTo(BigDecimal.ZERO) <= 0) {
+ throw new PaymentApiException(ErrorCode.PAYMENT_REFUND_AMOUNT_NEGATIVE_OR_NULL);
+ }
return refundProcessor.createRefund(account, paymentId, refundAmount, isAdjusted, context);
}
diff --git a/payment/src/main/java/com/ning/billing/payment/core/ProcessorBase.java b/payment/src/main/java/com/ning/billing/payment/core/ProcessorBase.java
index 9d31f67..33c1431 100644
--- a/payment/src/main/java/com/ning/billing/payment/core/ProcessorBase.java
+++ b/payment/src/main/java/com/ning/billing/payment/core/ProcessorBase.java
@@ -66,6 +66,15 @@ public abstract class ProcessorBase {
this.executor = executor;
}
+ protected PaymentPluginApi getPaymentProviderPlugin(final UUID paymentMethodId) throws PaymentApiException {
+ final PaymentMethodModelDao methodDao = paymentDao.getPaymentMethod(paymentMethodId);
+ if (methodDao == null) {
+ log.error("PaymentMethod dpes not exist", paymentMethodId);
+ throw new PaymentApiException(ErrorCode.PAYMENT_NO_SUCH_PAYMENT_METHOD, paymentMethodId);
+ }
+ return pluginRegistry.getPlugin(methodDao.getPluginName());
+ }
+
protected PaymentPluginApi getPaymentProviderPlugin(final String accountKey)
throws AccountApiException, PaymentApiException {
@@ -79,20 +88,11 @@ public abstract class ProcessorBase {
}
protected PaymentPluginApi getPaymentProviderPlugin(final Account account) throws PaymentApiException {
- String paymentProviderName = null;
- if (account != null) {
- final UUID paymentMethodId = account.getPaymentMethodId();
- if (paymentMethodId == null) {
- throw new PaymentApiException(ErrorCode.PAYMENT_NO_DEFAULT_PAYMENT_METHOD, account.getId());
- }
- final PaymentMethodModelDao methodDao = paymentDao.getPaymentMethod(paymentMethodId);
- if (methodDao == null) {
- log.error("Account {} has a non existent default payment method {}!!!", account.getId(), paymentMethodId);
- throw new PaymentApiException(ErrorCode.PAYMENT_NO_DEFAULT_PAYMENT_METHOD, account.getId());
- }
- paymentProviderName = methodDao.getPluginName();
+ final UUID paymentMethodId = account.getPaymentMethodId();
+ if (paymentMethodId == null) {
+ throw new PaymentApiException(ErrorCode.PAYMENT_NO_DEFAULT_PAYMENT_METHOD, account.getId());
}
- return pluginRegistry.getPlugin(paymentProviderName);
+ return getPaymentProviderPlugin(paymentMethodId);
}
protected void postPaymentEvent(final BusEvent ev, final UUID accountId) {
diff --git a/payment/src/main/java/com/ning/billing/payment/core/RefundProcessor.java b/payment/src/main/java/com/ning/billing/payment/core/RefundProcessor.java
index a7768c2..6640ddb 100644
--- a/payment/src/main/java/com/ning/billing/payment/core/RefundProcessor.java
+++ b/payment/src/main/java/com/ning/billing/payment/core/RefundProcessor.java
@@ -44,6 +44,7 @@ import com.ning.billing.payment.api.PaymentStatus;
import com.ning.billing.payment.api.Refund;
import com.ning.billing.payment.dao.PaymentAttemptModelDao;
import com.ning.billing.payment.dao.PaymentDao;
+import com.ning.billing.payment.dao.PaymentModelDao;
import com.ning.billing.payment.dao.RefundModelDao;
import com.ning.billing.payment.dao.RefundModelDao.RefundStatus;
import com.ning.billing.payment.plugin.api.PaymentPluginApi;
@@ -89,11 +90,11 @@ public class RefundProcessor extends ProcessorBase {
public Refund doOperation() throws PaymentApiException {
try {
- final PaymentAttemptModelDao successfulAttempt = getPaymentAttempt(paymentId);
- if (successfulAttempt == null) {
+ final PaymentModelDao payment = paymentDao.getPayment(paymentId);
+ if (payment == null) {
throw new PaymentApiException(ErrorCode.PAYMENT_NO_SUCH_SUCCESS_PAYMENT, paymentId);
}
- if (successfulAttempt.getRequestedAmount().compareTo(refundAmount) < 0) {
+ if (payment.getAmount().compareTo(refundAmount) < 0) {
throw new PaymentApiException(ErrorCode.PAYMENT_REFUND_AMOUNT_TOO_LARGE);
}
@@ -102,7 +103,9 @@ public class RefundProcessor extends ProcessorBase {
RefundModelDao refundInfo = null;
List<RefundModelDao> existingRefunds = paymentDao.getRefundsForPayment(paymentId);
for (RefundModelDao cur : existingRefunds) {
- if (cur.getAmount().compareTo(refundAmount) == 0) {
+
+ final BigDecimal existingPositiveAmount = cur.getAmount().negate();
+ if (existingPositiveAmount.compareTo(refundAmount) == 0) {
if (cur.getRefundStatus() == RefundStatus.CREATED) {
if (refundInfo == null) {
refundInfo = cur;
@@ -117,12 +120,14 @@ public class RefundProcessor extends ProcessorBase {
paymentDao.insertRefund(refundInfo, context);
}
- final PaymentPluginApi plugin = getPaymentProviderPlugin(account);
+ final PaymentPluginApi plugin = getPaymentProviderPlugin(payment.getPaymentMethodId());
int nbExistingRefunds = plugin.getNbRefundForPaymentAmount(account, paymentId, refundAmount);
+ log.info(String.format("STEPH debug : found %d pluginRefunds for paymentId %s and amount %s", nbExistingRefunds, paymentId, refundAmount));
+
if (nbExistingRefunds > foundPluginCompletedRefunds) {
log.info("Found existing plugin refund for paymentId {}, skip plugin", paymentId);
} else {
- // If there is no such existng refund we create it
+ // If there is no such existing refund we create it
plugin.processRefund(account, paymentId, refundAmount);
}
paymentDao.updateRefundStatus(refundInfo.getId(), RefundStatus.PLUGIN_COMPLETED, context);
diff --git a/server/src/test/java/com/ning/billing/jaxrs/TestPayment.java b/server/src/test/java/com/ning/billing/jaxrs/TestPayment.java
index be1ec0d..090b700 100644
--- a/server/src/test/java/com/ning/billing/jaxrs/TestPayment.java
+++ b/server/src/test/java/com/ning/billing/jaxrs/TestPayment.java
@@ -79,7 +79,7 @@ public class TestPayment extends TestJaxrsBase {
// Issue the refund
- RefundJson refundJson = new RefundJson(null, paymentId, paymentAmount.negate(), false);
+ RefundJson refundJson = new RefundJson(null, paymentId, paymentAmount, false);
baseJson = mapper.writeValueAsString(refundJson);
response = doPost(uri, baseJson, DEFAULT_EMPTY_QUERY, DEFAULT_HTTP_TIMEOUT_SEC);
assertEquals(response.getStatusCode(), Status.CREATED.getStatusCode());
