You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by mb...@apache.org on 2017/10/07 09:59:47 UTC
svn commit: r1811405 - in
/ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice:
InvoiceServices.java InvoiceWorker.java
Author: mbrohl
Date: Sat Oct 7 09:59:47 2017
New Revision: 1811405
URL: http://svn.apache.org/viewvc?rev=1811405&view=rev
Log:
Improved: Fixing defects reported by FindBugs, package
org.apache.ofbiz.accounting.invoice.
(OFBIZ-9541)
Thanks Karsten Tymann for reporting and providing the patch.
Modified:
ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java
ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceWorker.java
Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java?rev=1811405&r1=1811404&r2=1811405&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java Sat Oct 7 09:59:47 2017
@@ -32,6 +32,7 @@ import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Set;
import org.apache.commons.collections4.CollectionUtils;
@@ -104,7 +105,7 @@ import org.apache.ofbiz.service.ServiceU
*/
public class InvoiceServices {
- public static String module = InvoiceServices.class.getName();
+ public static final String module = InvoiceServices.class.getName();
// set some BigDecimal properties
private static final BigDecimal ZERO = BigDecimal.ZERO;
@@ -386,11 +387,14 @@ public class InvoiceServices {
orderItem = itemIssuance.getRelatedOne("OrderItem", false);
} else if ((orderItem == null) && (shipmentReceipt != null)) {
orderItem = shipmentReceipt.getRelatedOne("OrderItem", false);
- } else if ((orderItem == null) && (itemIssuance == null) && (shipmentReceipt == null)) {
+ }
+
+ if (orderItem == null) {
Debug.logError("Cannot create invoice when orderItem, itemIssuance, and shipmentReceipt are all null", module);
return ServiceUtil.returnError(UtilProperties.getMessage(resource,
"AccountingIllegalValuesPassedToCreateInvoiceService", locale));
}
+
GenericValue product = null;
if (orderItem.get("productId") != null) {
product = orderItem.getRelatedOne("Product", false);
@@ -763,9 +767,10 @@ public class InvoiceServices {
// next do the shipping adjustments. Note that we do not want to add these to the invoiceSubTotal or orderSubTotal for pro-rating tax later, as that would cause
// numerator/denominator problems when the shipping is not pro-rated but rather charged all on the first invoice
- for (GenericValue adj : shipAdjustments.keySet()) {
- BigDecimal adjAlreadyInvoicedAmount = shipAdjustments.get(adj);
-
+ for (Map.Entry<GenericValue, BigDecimal> set : shipAdjustments.entrySet()) {
+ BigDecimal adjAlreadyInvoicedAmount = set.getValue();
+ GenericValue adj = set.getKey();
+
if ("N".equalsIgnoreCase(prorateShipping)) {
// Set the divisor and multiplier to 1 to avoid prorating
@@ -918,8 +923,8 @@ public class InvoiceServices {
BigDecimal appliedFraction = amountTotal.divide(amountTotal, 12, ROUNDING);
GenericValue invoice = null;
boolean isReturn = false;
- List<String> billFromVendorInvoiceRoles = new ArrayList<String>();
- List<GenericValue> invoiceItems = new ArrayList<GenericValue>();
+ List<String> billFromVendorInvoiceRoles;
+ List<GenericValue> invoiceItems;
try {
List<EntityExpr> invoiceRoleConds = UtilMisc.toList(
EntityCondition.makeCondition("invoiceId", EntityOperator.EQUALS, salesInvoiceId),
@@ -960,8 +965,7 @@ public class InvoiceServices {
// Determine commissions for various parties.
for (GenericValue invoiceItem : invoiceItems) {
BigDecimal amount = ZERO;
- BigDecimal quantity = ZERO;
- quantity = invoiceItem.getBigDecimal("quantity");
+ BigDecimal quantity = invoiceItem.getBigDecimal("quantity");
amount = invoiceItem.getBigDecimal("amount");
amount = isReturn ? amount.negate() : amount;
String productId = invoiceItem.getString("productId");
@@ -1034,12 +1038,12 @@ public class InvoiceServices {
createInvoiceMap.put("currencyUomId", invoice.getString("currencyUomId"));
createInvoiceMap.put("userLogin", userLogin);
// store the invoice first
- Map<String, Object> createInvoiceResult = null;
+ Map<String, Object> createInvoiceResult;
try {
createInvoiceResult = dispatcher.runSync("createInvoice", createInvoiceMap);
} catch (GenericServiceException e) {
return ServiceUtil.returnError(UtilProperties.getMessage(resource,
- "AccountingInvoiceCommissionError", locale), null, null, createInvoiceResult);
+ "AccountingInvoiceCommissionError", locale), null, null, null);
}
String invoiceId = (String) createInvoiceResult.get("invoiceId");
// create the bill-from (or pay-to) contact mech as the primary PAYMENT_LOCATION of the party from the store
@@ -1154,7 +1158,7 @@ public class InvoiceServices {
LocalDispatcher dispatcher = dctx.getDispatcher();
String shipmentId = (String) context.get("shipmentId");
Locale locale = (Locale) context.get("locale");
- List<String> invoicesCreated = new LinkedList<String>();
+ List<String> invoicesCreated;
Map<String, Object> response = ServiceUtil.returnSuccess();
GenericValue orderShipment = null;
String invoicePerShipment = null;
@@ -1222,7 +1226,7 @@ public class InvoiceServices {
return ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingShipmentNotFound", locale));
}
- List<GenericValue> itemIssuances = new LinkedList<GenericValue>();
+ List<GenericValue> itemIssuances;
try {
itemIssuances = EntityQuery.use(delegator).select("orderId", "shipmentId")
.from("ItemIssuance").where("shipmentId", shipmentId).orderBy("orderId").distinct().queryList();
@@ -1275,7 +1279,7 @@ public class InvoiceServices {
// For In-Process invoice, move the status to ready and capture the payment
for (GenericValue invoice : ordersWithInProcessInvoice.values()) {
String invoiceId = invoice.getString("invoiceId");
- Map<String, Object> setInvoiceStatusResult = new HashMap<String, Object>();
+ Map<String, Object> setInvoiceStatusResult;
try {
setInvoiceStatusResult = dispatcher.runSync("setInvoiceStatus", UtilMisc.<String, Object>toMap("invoiceId", invoiceId, "statusId", "INVOICE_READY", "userLogin", userLogin));
} catch (GenericServiceException e) {
@@ -1451,11 +1455,10 @@ public class InvoiceServices {
}
// make sure we aren't billing items already invoiced i.e. items billed as digital (FINDIG)
- Set<String> orders = shippedOrderItems.keySet();
- for (String orderId : orders) {
-
+ for (Entry<String, List<GenericValue>> order : shippedOrderItems.entrySet()) {
+ String orderId = order.getKey();
// we'll only use this list to figure out which ones to send
- List<GenericValue> billItems = shippedOrderItems.get(orderId);
+ List<GenericValue> billItems = order.getValue();
// a new list to be used to pass to the create invoice service
List<GenericValue> toBillItems = new LinkedList<GenericValue>();
@@ -1533,6 +1536,9 @@ public class InvoiceServices {
billAvail = ZERO;
} else {
// now have been billed
+ if(issueQty == null){
+ issueQty = ZERO;
+ }
billAvail = billAvail.subtract(issueQty).setScale(DECIMALS, ROUNDING);
}
@@ -2612,10 +2618,6 @@ public class InvoiceServices {
// Payment.....
BigDecimal paymentApplyAvailable = ZERO;
// amount available on the payment reduced by the already applied amounts
- BigDecimal amountAppliedMax = ZERO;
- // the maximum that can be applied taking payment,invoice,invoiceitem,billing account in concideration
- // if maxApplied is missing, this value can be used,
- // Payment this should be checked after the invoice checking because it is possible the currency is changed
GenericValue payment = null;
String currencyUomId = null;
if (paymentId == null || paymentId.equals("")) {
@@ -2629,6 +2631,7 @@ public class InvoiceServices {
if (payment == null) {
errorMessageList.add(UtilProperties.getMessage(resource,
"AccountingPaymentRecordNotFound", UtilMisc.toMap("paymentId", paymentId), locale));
+ return ServiceUtil.returnError(errorMessageList);
}
paymentApplyAvailable = payment.getBigDecimal("amount").subtract(PaymentWorker.getPaymentApplied(payment)).setScale(DECIMALS,ROUNDING);
@@ -2643,12 +2646,6 @@ public class InvoiceServices {
currencyUomId = payment.getString("currencyUomId");
- // if the amount to apply is 0 give it amount the payment still need
- // to apply
- if (amountApplied.signum() == 0) {
- amountAppliedMax = paymentApplyAvailable;
- }
-
}
// the "TO" Payment.....
@@ -2663,6 +2660,7 @@ public class InvoiceServices {
if (toPayment == null) {
errorMessageList.add(UtilProperties.getMessage(resource,
"AccountingPaymentRecordNotFound", UtilMisc.toMap("paymentId", toPaymentId), locale));
+ return ServiceUtil.returnError(errorMessageList);
}
toPaymentApplyAvailable = toPayment.getBigDecimal("amount").subtract(PaymentWorker.getPaymentApplied(toPayment)).setScale(DECIMALS,ROUNDING);
@@ -2675,11 +2673,6 @@ public class InvoiceServices {
"AccountingPaymentConfirmed", UtilMisc.toMap("paymentId", paymentId), locale));
}
- // if the amount to apply is less then required by the payment reduce it
- if (amountAppliedMax.compareTo(toPaymentApplyAvailable) > 0) {
- amountAppliedMax = toPaymentApplyAvailable;
- }
-
if (paymentApplicationId == null) {
// only check for new application records, update on existing records is checked in the paymentApplication section
if (toPaymentApplyAvailable.signum() == 0) {
@@ -2738,6 +2731,7 @@ public class InvoiceServices {
if (billingAccount == null) {
errorMessageList.add(UtilProperties.getMessage(resource,
"AccountingBillingAccountNotFound", UtilMisc.toMap("billingAccountId", billingAccountId), locale));
+ return ServiceUtil.returnError(errorMessageList);
}
// check the currency
if (billingAccount.get("accountCurrencyUomId") != null && currencyUomId != null &&
@@ -2791,20 +2785,12 @@ public class InvoiceServices {
}
}
paymentApplyAvailable = payment.getBigDecimal("actualCurrencyAmount").subtract(PaymentWorker.getPaymentApplied(payment)).setScale(DECIMALS,ROUNDING);
- if (amountApplied.signum() == 0) {
- amountAppliedMax = paymentApplyAvailable;
- }
}
// check if the invoice already covered by payments
BigDecimal invoiceTotal = InvoiceWorker.getInvoiceTotal(invoice);
invoiceApplyAvailable = InvoiceWorker.getInvoiceNotApplied(invoice);
- // adjust the amountAppliedMax value if required....
- if (invoiceApplyAvailable.compareTo(amountAppliedMax) < 0) {
- amountAppliedMax = invoiceApplyAvailable;
- }
-
if (invoiceTotal.signum() == 0) {
errorMessageList.add(UtilProperties.getMessage(resource,
"AccountingInvoiceTotalZero", UtilMisc.toMap("invoiceId", invoiceId), locale));
@@ -2986,7 +2972,7 @@ public class InvoiceServices {
UtilMisc.<String, Object>toMap("tooMuch", newInvoiceApplyAvailable.negate(),
"invoiceId", invoiceId), locale));
}
- } else if (invoiceItemSeqId != null && paymentApplication.get("invoiceItemSeqId") == null) {
+ } else if (paymentApplication.get("invoiceItemSeqId") == null) {
// check if the item number changed from a null value to
// a real Item number
newInvoiceItemApplyAvailable = invoiceItemApplyAvailable.subtract(amountApplied).setScale(DECIMALS, ROUNDING);
@@ -3268,22 +3254,16 @@ public class InvoiceServices {
// if no paymentApplicationId supplied create a new record with the data
// supplied...
- if (paymentApplicationId == null && amountApplied != null) {
- paymentApplication.set("paymentApplicationId", paymentApplicationId);
- paymentApplication.set("invoiceId", invoiceId);
- paymentApplication.set("invoiceItemSeqId", invoiceItemSeqId);
- paymentApplication.set("paymentId", paymentId);
- paymentApplication.set("toPaymentId", toPaymentId);
- paymentApplication.set("amountApplied", amountApplied);
- paymentApplication.set("billingAccountId", billingAccountId);
- paymentApplication.set("taxAuthGeoId", taxAuthGeoId);
- return storePaymentApplication(delegator, paymentApplication,locale);
- }
+ paymentApplication.set("paymentApplicationId", paymentApplicationId);
+ paymentApplication.set("invoiceId", invoiceId);
+ paymentApplication.set("invoiceItemSeqId", invoiceItemSeqId);
+ paymentApplication.set("paymentId", paymentId);
+ paymentApplication.set("toPaymentId", toPaymentId);
+ paymentApplication.set("amountApplied", amountApplied);
+ paymentApplication.set("billingAccountId", billingAccountId);
+ paymentApplication.set("taxAuthGeoId", taxAuthGeoId);
+ return storePaymentApplication(delegator, paymentApplication,locale);
- // should never come here...
- errorMessageList.add(UtilProperties.getMessage(resource, "AccountingPaymentApplicationParameterUnsuitable", locale));
- errorMessageList.add(UtilProperties.getMessage(resource, "AccountingPaymentApplicationParameterListUnsuitable", UtilMisc.toMap("invoiceId", invoiceId, "invoiceItemSeqId", invoiceItemSeqId, "paymentId", paymentId, "toPaymentId", toPaymentId, "paymentApplicationId", paymentApplicationId, "amountApplied", amountApplied), locale));
- return ServiceUtil.returnError(errorMessageList);
}
public static Map<String, Object> calculateInvoicedAdjustmentTotal(DispatchContext dctx, Map<String, Object> context) {
@@ -3445,22 +3425,23 @@ public class InvoiceServices {
LocalDispatcher dispatcher = dctx.getDispatcher();
GenericValue userLogin = (GenericValue) context.get("userLogin");
ByteBuffer fileBytes = (ByteBuffer) context.get("uploadedFile");
+
+ if (fileBytes == null) {
+ return ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingUploadedFileDataNotFound", locale));
+ }
+
String organizationPartyId = (String) context.get("organizationPartyId");
String encoding = System.getProperty("file.encoding");
String csvString = Charset.forName(encoding).decode(fileBytes).toString();
final BufferedReader csvReader = new BufferedReader(new StringReader(csvString));
CSVFormat fmt = CSVFormat.DEFAULT.withHeader();
- List<String> errMsgs = new LinkedList<String>();
- List<String> newErrMsgs = new LinkedList<String>();
+ List<String> errMsgs = new LinkedList<>();
+ List<String> newErrMsgs;
String lastInvoiceId = null;
String currentInvoiceId = null;
String newInvoiceId = null;
int invoicesCreated = 0;
- if (fileBytes == null) {
- return ServiceUtil.returnError(UtilProperties.getMessage(resource, "AccountingUploadedFileDataNotFound", locale));
- }
-
try {
for (final CSVRecord rec : fmt.parse(csvReader)) {
currentInvoiceId = rec.get("invoiceId");
@@ -3487,8 +3468,8 @@ public class InvoiceServices {
}
// invoice validation
- try {
- newErrMsgs = new LinkedList<String>();
+ newErrMsgs = new LinkedList<>();
+ try {
if (UtilValidate.isEmpty(invoice.get("partyIdFrom"))) {
newErrMsgs.add("Line number " + rec.getRecordNumber() + ": Mandatory Party Id From and Party Id From Trans missing for invoice: " + currentInvoiceId);
} else if (EntityQuery.use(delegator).from("Party").where("partyId", invoice.get("partyIdFrom")).queryOne() == null) {
@@ -3553,8 +3534,8 @@ public class InvoiceServices {
invoiceItem.put("productId", rec.get("productIdTrans"));
}
// invoice item validation
+ newErrMsgs = new LinkedList<>();
try {
- newErrMsgs = new LinkedList<String>();
if (UtilValidate.isEmpty(invoiceItem.get("invoiceItemSeqId"))) {
newErrMsgs.add("Line number " + rec.getRecordNumber() + ": Mandatory item sequence Id missing for invoice: " + currentInvoiceId);
}
@@ -3564,6 +3545,7 @@ public class InvoiceServices {
newErrMsgs.add("Line number " + rec.getRecordNumber() + ": InvoiceItem Item type id: " + invoiceItem.get("invoiceItemTypeId") + " not found for invoice: " + currentInvoiceId + " Item seqId:" + invoiceItem.get("invoiceItemSeqId"));
}
if (UtilValidate.isEmpty(invoiceItem.get("productId")) && UtilValidate.isEmpty(invoiceItem.get("description"))) {
+ newErrMsgs.add("Line number " + rec.getRecordNumber() + ": no Product Id given, no description given");
}
if (UtilValidate.isNotEmpty(invoiceItem.get("productId")) && EntityQuery.use(delegator).from("Product").where("productId", invoiceItem.get("productId")).queryOne() == null) {
newErrMsgs.add("Line number " + rec.getRecordNumber() + ": Product Id: " + invoiceItem.get("productId") + " not found for invoice: " + currentInvoiceId + " Item seqId:" + invoiceItem.get("invoiceItemSeqId"));
Modified: ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceWorker.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceWorker.java?rev=1811405&r1=1811404&r2=1811405&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceWorker.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceWorker.java Sat Oct 7 09:59:47 2017
@@ -161,8 +161,7 @@ public final class InvoiceWorker {
*/
public static BigDecimal getInvoiceTotal(GenericValue invoice, Boolean actualCurrency) {
BigDecimal invoiceTotal = ZERO;
- BigDecimal invoiceTaxTotal = ZERO;
- invoiceTaxTotal = InvoiceWorker.getInvoiceTaxTotal(invoice);
+ BigDecimal invoiceTaxTotal = InvoiceWorker.getInvoiceTaxTotal(invoice);
List<GenericValue> invoiceItems = null;
try {