Author: mbrohl
Date: Sat Dec 9 17:29:12 2017
New Revision: 1817636
URL: http://svn.apache.org/viewvc?rev=1817636&view=rev
Log:
Improved: Fixing defects reported by FindBugs, package
org.apache.ofbiz.order.shoppingcart.product.
(OFBIZ-9781)
Additionally removed the private method findAdjustment which was not
used anywhere in the code.
Thanks Julian Leichert for reporting and providing the patch.
Modified:
ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/product/ProductDisplayWorker.java
ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/product/ProductPromoWorker.java
Modified:
ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/product/ProductDisplayWorker.java
URL:
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/product/ProductDisplayWorker.java?rev=1817636&r1=1817635&r2=1817636&view=diff
==============================================================================
---
ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/product/ProductDisplayWorker.java
(original)
+++
ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/product/ProductDisplayWorker.java
Sat Dec 9 17:29:12 2017
@@ -325,6 +325,15 @@ public final class ProductDisplayWorker
}
@Override
+ public int hashCode() {
+ final int prime = 31;
+ int result = 1;
+ result = prime * result + (descending ? 1231 : 1237);
+ result = prime * result + ((orderByMap == null) ? 0 :
orderByMap.hashCode());
+ return result;
+ }
+
+ @Override
public boolean equals(java.lang.Object obj) {
if ((obj != null) && (obj instanceof ProductByMapComparator)) {
ProductByMapComparator that = (ProductByMapComparator) obj;
Modified:
ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/product/ProductPromoWorker.java
URL:
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/product/ProductPromoWorker.java?rev=1817636&r1=1817635&r2=1817636&view=diff
==============================================================================
---
ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/product/ProductPromoWorker.java
(original)
+++
ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/product/ProductPromoWorker.java
Sat Dec 9 17:29:12 2017
@@ -31,6 +31,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 javax.servlet.ServletRequest;
@@ -115,45 +116,43 @@ public final class ProductPromoWorker {
return productPromos;
}
- if (productStore != null) {
- Iterator<GenericValue> productStorePromoAppls =
UtilMisc.toIterator(EntityUtil.filterByDate(productStore.getRelated("ProductStorePromoAppl",
UtilMisc.toMap("productStoreId", productStoreId),
UtilMisc.toList("sequenceNum"), true), true));
- while (productStorePromoAppls != null &&
productStorePromoAppls.hasNext()) {
- GenericValue productStorePromoAppl =
productStorePromoAppls.next();
- if
(UtilValidate.isNotEmpty(productStorePromoAppl.getString("manualOnly")) &&
"Y".equals(productStorePromoAppl.getString("manualOnly"))) {
- // manual only promotions are not automatically
evaluated (they must be explicitly selected by the user)
- if (Debug.verboseOn()) Debug.logVerbose("Skipping
promotion with id [" + productStorePromoAppl.getString("productPromoId") + "]
because it is applied to the store with ID " + productStoreId + " as a manual
only promotion.", module);
- continue;
- }
- GenericValue productPromo =
productStorePromoAppl.getRelatedOne("ProductPromo", true);
- List<GenericValue> productPromoRules =
productPromo.getRelated("ProductPromoRule", null, null, true);
+ Iterator<GenericValue> productStorePromoAppls =
UtilMisc.toIterator(EntityUtil.filterByDate(productStore.getRelated("ProductStorePromoAppl",
UtilMisc.toMap("productStoreId", productStoreId),
UtilMisc.toList("sequenceNum"), true), true));
+ while (productStorePromoAppls != null &&
productStorePromoAppls.hasNext()) {
+ GenericValue productStorePromoAppl =
productStorePromoAppls.next();
+ if
(UtilValidate.isNotEmpty(productStorePromoAppl.getString("manualOnly")) &&
"Y".equals(productStorePromoAppl.getString("manualOnly"))) {
+ // manual only promotions are not automatically evaluated
(they must be explicitly selected by the user)
+ if (Debug.verboseOn()) Debug.logVerbose("Skipping
promotion with id [" + productStorePromoAppl.getString("productPromoId") + "]
because it is applied to the store with ID " + productStoreId + " as a manual
only promotion.", module);
+ continue;
+ }
+ GenericValue productPromo =
productStorePromoAppl.getRelatedOne("ProductPromo", true);
+ List<GenericValue> productPromoRules =
productPromo.getRelated("ProductPromoRule", null, null, true);
- if (productPromoRules != null) {
- Iterator<GenericValue> promoRulesItr =
productPromoRules.iterator();
+ if (productPromoRules != null) {
+ Iterator<GenericValue> promoRulesItr =
productPromoRules.iterator();
- while (condResult && promoRulesItr != null &&
promoRulesItr.hasNext()) {
- GenericValue promoRule = promoRulesItr.next();
- Iterator<GenericValue> productPromoConds =
UtilMisc.toIterator(promoRule.getRelated("ProductPromoCond", null,
UtilMisc.toList("productPromoCondSeqId"), true));
-
- while (condResult && productPromoConds != null &&
productPromoConds.hasNext()) {
- GenericValue productPromoCond =
productPromoConds.next();
-
- // evaluate the party related conditions; so
we don't show the promo if it doesn't apply.
- if
("PPIP_PARTY_ID".equals(productPromoCond.getString("inputParamEnumId"))) {
- condResult =
checkCondition(productPromoCond, cart, delegator, dispatcher, nowTimestamp);
- } else if
("PPIP_PARTY_GRP_MEM".equals(productPromoCond.getString("inputParamEnumId"))) {
- condResult =
checkCondition(productPromoCond, cart, delegator, dispatcher, nowTimestamp);
- } else if
("PPIP_PARTY_CLASS".equals(productPromoCond.getString("inputParamEnumId"))) {
- condResult =
checkCondition(productPromoCond, cart, delegator, dispatcher, nowTimestamp);
- } else if
("PPIP_ROLE_TYPE".equals(productPromoCond.getString("inputParamEnumId"))) {
- condResult =
checkCondition(productPromoCond, cart, delegator, dispatcher, nowTimestamp);
- }
+ while (condResult && promoRulesItr != null &&
promoRulesItr.hasNext()) {
+ GenericValue promoRule = promoRulesItr.next();
+ Iterator<GenericValue> productPromoConds =
UtilMisc.toIterator(promoRule.getRelated("ProductPromoCond", null,
UtilMisc.toList("productPromoCondSeqId"), true));
+
+ while (condResult && productPromoConds != null &&
productPromoConds.hasNext()) {
+ GenericValue productPromoCond =
productPromoConds.next();
+
+ // evaluate the party related conditions; so we
don't show the promo if it doesn't apply.
+ if
("PPIP_PARTY_ID".equals(productPromoCond.getString("inputParamEnumId"))) {
+ condResult = checkCondition(productPromoCond,
cart, delegator, dispatcher, nowTimestamp);
+ } else if
("PPIP_PARTY_GRP_MEM".equals(productPromoCond.getString("inputParamEnumId"))) {
+ condResult = checkCondition(productPromoCond,
cart, delegator, dispatcher, nowTimestamp);
+ } else if
("PPIP_PARTY_CLASS".equals(productPromoCond.getString("inputParamEnumId"))) {
+ condResult = checkCondition(productPromoCond,
cart, delegator, dispatcher, nowTimestamp);
+ } else if
("PPIP_ROLE_TYPE".equals(productPromoCond.getString("inputParamEnumId"))) {
+ condResult = checkCondition(productPromoCond,
cart, delegator, dispatcher, nowTimestamp);
}
}
- if (!condResult) productPromo = null;
}
- if (productPromo != null) productPromos.add(productPromo);
+ if (!condResult) productPromo = null;
}
+ if (productPromo != null) productPromos.add(productPromo);
}
} catch (GenericEntityException e) {
Debug.logError(e, module);
@@ -380,7 +379,7 @@ public final class ProductPromoWorker {
Debug.logError(e, "Number not formatted correctly in promotion
rules, not completed...", module);
} catch (GenericEntityException e) {
Debug.logError(e, "Error looking up promotion data while doing
promotions", module);
- } catch (Exception e) {
+ } catch (GeneralException e) {
Debug.logError(e, "Error running promotions, will ignore: " +
e.toString(), module);
}
}
@@ -505,9 +504,7 @@ public final class ProductPromoWorker {
Long useLimitPerOrder = productPromo.getLong("useLimitPerOrder");
if (useLimitPerOrder != null) {
- if (candidateUseLimit == null || candidateUseLimit.longValue() >
useLimitPerOrder.longValue()) {
- candidateUseLimit = useLimitPerOrder;
- }
+ candidateUseLimit = useLimitPerOrder;
}
Long useLimitPerCustomer = productPromo.getLong("useLimitPerCustomer");
@@ -565,9 +562,7 @@ public final class ProductPromoWorker {
productPromoCustomerUseSize =
EntityQuery.use(delegator).from("ProductPromoUseCheck").where(checkCondition).queryCount();
}
long perCustomerThisOrder = codeUseLimitPerCustomer.longValue() -
productPromoCustomerUseSize;
- if (codeUseLimit == null || codeUseLimit.longValue() >
perCustomerThisOrder) {
- codeUseLimit = Long.valueOf(perCustomerThisOrder);
- }
+ codeUseLimit = Long.valueOf(perCustomerThisOrder);
}
Long codeUseLimitPerCode = productPromoCode.getLong("useLimitPerCode");
@@ -877,15 +872,15 @@ public final class ProductPromoWorker {
private static Map<ShoppingCartItem,BigDecimal>
prepareDeltaProductUsageInfoMap(Map<ShoppingCartItem,BigDecimal> oldMap,
Map<ShoppingCartItem,BigDecimal> newMap) {
Map<ShoppingCartItem,BigDecimal> deltaUsageInfoMap = new
HashMap<ShoppingCartItem, BigDecimal>(newMap);
- Iterator<ShoppingCartItem> cartLines = oldMap.keySet().iterator();
- while (cartLines.hasNext()) {
- ShoppingCartItem cartLine = cartLines.next();
- BigDecimal oldUsed = oldMap.get(cartLine);
- BigDecimal newUsed = newMap.get(cartLine);
+
+ for (Entry<ShoppingCartItem, BigDecimal> entry : oldMap.entrySet()) {
+ ShoppingCartItem key = entry.getKey();
+ BigDecimal oldUsed = entry.getValue();
+ BigDecimal newUsed = entry.getValue();
if (newUsed.compareTo(oldUsed) > 0) {
- deltaUsageInfoMap.put(cartLine, newUsed.add(oldUsed.negate()));
+ deltaUsageInfoMap.put(key, newUsed.add(oldUsed.negate()));
} else {
- deltaUsageInfoMap.remove(cartLine);
+ deltaUsageInfoMap.remove(key);
}
}
return deltaUsageInfoMap;
@@ -899,8 +894,8 @@ public final class ProductPromoWorker {
String shippingMethod = "";
String carrierPartyId = "";
if (otherValue != null && otherValue.contains("@")) {
- carrierPartyId = otherValue.substring(0, otherValue.indexOf("@"));
- shippingMethod = otherValue.substring(otherValue.indexOf("@")+1);
+ carrierPartyId = otherValue.substring(0, otherValue.indexOf('@'));
+ shippingMethod = otherValue.substring(otherValue.indexOf('@') + 1);
otherValue = "";
}
String partyId = cart.getPartyId();
@@ -1462,11 +1457,9 @@ public final class ProductPromoWorker {
try {
// get the quantity in cart for inventory check
BigDecimal quantityAlreadyInCart = BigDecimal.ZERO;
- if (cart != null) {
- List<ShoppingCartItem> matchingItems =
cart.findAllCartItems(productId);
- for (ShoppingCartItem item : matchingItems) {
- quantityAlreadyInCart =
quantityAlreadyInCart.add(item.getQuantity());
- }
+ List<ShoppingCartItem> matchingItems =
cart.findAllCartItems(productId);
+ for (ShoppingCartItem item : matchingItems) {
+ quantityAlreadyInCart =
quantityAlreadyInCart.add(item.getQuantity());
}
Map<String, Object> invReqResult =
dispatcher.runSync("isStoreInventoryAvailable", UtilMisc.<String,
Object>toMap("productStoreId", productStoreId, "productId", productId,
"product", product, "quantity", quantity.add(quantityAlreadyInCart)));
if (ServiceUtil.isError(invReqResult)) {
@@ -1487,9 +1480,7 @@ public final class ProductPromoWorker {
// support multiple gift options if products are attached to
the action, or if the productId on the action is a virtual product
Set<String> productIds =
ProductPromoWorker.getPromoRuleActionProductIds(productPromoAction, delegator,
nowTimestamp);
- if (productIds != null) {
- optionProductIds.addAll(productIds);
- }
+ optionProductIds.addAll(productIds);
// make sure these optionProducts have inventory...
Iterator<String> optionProductIdIter =
optionProductIds.iterator();
@@ -1499,12 +1490,11 @@ public final class ProductPromoWorker {
try {
// get the quantity in cart for inventory check
BigDecimal quantityAlreadyInCart = BigDecimal.ZERO;
- if (cart != null) {
- List<ShoppingCartItem> matchingItems =
cart.findAllCartItems(optionProductId);
- for (ShoppingCartItem item : matchingItems) {
- quantityAlreadyInCart =
quantityAlreadyInCart.add(item.getQuantity());
- }
+ List<ShoppingCartItem> matchingItems =
cart.findAllCartItems(optionProductId);
+ for (ShoppingCartItem item : matchingItems) {
+ quantityAlreadyInCart =
quantityAlreadyInCart.add(item.getQuantity());
}
+
Map<String, Object> invReqResult =
dispatcher.runSync("isStoreInventoryAvailable", UtilMisc.<String,
Object>toMap("productStoreId", productStoreId, "productId", optionProductId,
"product", product, "quantity", quantity.add(quantityAlreadyInCart)));
if (ServiceUtil.isError(invReqResult)) {
Debug.logError("Error calling
isStoreInventoryAvailable service, result is: " + invReqResult, module);
@@ -1553,16 +1543,14 @@ public final class ProductPromoWorker {
ShoppingCartItem gwpItem = null;
try {
// just leave the prodCatalogId null, this line won't be
associated with a catalog
- String prodCatalogId = null;
- gwpItem = ShoppingCartItem.makeItem(null, product, null,
quantity, null, null, null, null, null, null, null, null, prodCatalogId, null,
null, null, dispatcher, cart, Boolean.FALSE, Boolean.TRUE, null, Boolean.FALSE,
Boolean.FALSE);
+ gwpItem = ShoppingCartItem.makeItem(null, product, null,
quantity, null, null, null, null, null, null, null, null, null, null, null,
null, dispatcher, cart, Boolean.FALSE, Boolean.TRUE, null, Boolean.FALSE,
Boolean.FALSE);
if (optionProductIds.size() > 0) {
gwpItem.setAlternativeOptionProductIds(optionProductIds);
} else {
gwpItem.setAlternativeOptionProductIds(null);
}
} catch (CartItemModifyException e) {
- int gwpItemIndex = cart.getItemIndex(gwpItem);
- cart.removeCartItem(gwpItemIndex, dispatcher);
+ Debug.logError(e.getMessage(), module);
throw e;
}
@@ -1948,19 +1936,6 @@ public final class ProductPromoWorker {
return null;
}
- private static Integer findAdjustment(GenericValue productPromoAction,
List<GenericValue> adjustments) {
- for (int i = 0; i < adjustments.size(); i++) {
- GenericValue checkOrderAdjustment = adjustments.get(i);
-
- if
(productPromoAction.getString("productPromoId").equals(checkOrderAdjustment.get("productPromoId"))
&&
-
productPromoAction.getString("productPromoRuleId").equals(checkOrderAdjustment.get("productPromoRuleId"))
&&
-
productPromoAction.getString("productPromoActionSeqId").equals(checkOrderAdjustment.get("productPromoActionSeqId")))
{
- return Integer.valueOf(i);
- }
- }
- return null;
- }
-
public static Set<String> getPromoRuleCondProductIds(GenericValue
productPromoCond, Delegator delegator, Timestamp nowTimestamp) throws
GenericEntityException {
// get a cached list for the whole promo and filter it as needed, this
for better efficiency in caching
List<GenericValue> productPromoCategoriesAll =
EntityQuery.use(delegator).from("ProductPromoCategory").where("productPromoId",
productPromoCond.get("productPromoId")).cache(true).queryList();
@@ -2089,12 +2064,6 @@ public final class ProductPromoWorker {
String andGroupId =
productPromoCategory.getString("andGroupId");
if ("_NA_".equals(andGroupId)) {
productCategoryIds.addAll(tempCatIdSet);
- } else {
- List<Set<String>> catIdSetList =
productCategoryGroupSetListMap.get(andGroupId);
- if (catIdSetList == null) {
- catIdSetList = new LinkedList<Set<String>>();
- }
- catIdSetList.add(tempCatIdSet);
}
}
}
@@ -2144,7 +2113,7 @@ public final class ProductPromoWorker {
firstProductIdSet.retainAll(productIdSet);
}
- if (firstProductIdSet.size() >= 0) {
+ if (!firstProductIdSet.isEmpty()) {
if (include) {
productIds.addAll(firstProductIdSet);
} else {
