This commit breaks promotions.
1. Create a new sales order, customer DemoCustomer. 2. Add GZ-8544, qty 1 to the order. 3. Promo items do not appear in order. 4. Log shows an exception being thrown: 2014-01-14 12:15:56,030 (http-bio-0.0.0.0-8443-exec-1) [ ProductPromoWorker.java:385:ERROR] ---- runtime exception report -------------------------------------------------- Error running promotions, will ignore: java.lang.ArithmeticException: Non-terminating decimal expansion; no exact representable decimal result. Exception: java.lang.ArithmeticException Message: Non-terminating decimal expansion; no exact representable decimal result. ---- stack trace --------------------------------------------------------------- java.lang.ArithmeticException: Non-terminating decimal expansion; no exact representable decimal result. java.math.BigDecimal.divide(BigDecimal.java:1616) org.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo.getUsageWeight(ShoppingCart.java:4418) org.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo.compareTo(ShoppingCart.java:4423) org.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo.compareTo(ShoppingCart.java:4388) java.util.ComparableTimSort.countRunAndMakeAscending(ComparableTimSort.java:290) java.util.ComparableTimSort.sort(ComparableTimSort.java:157) java.util.ComparableTimSort.sort(ComparableTimSort.java:146) java.util.Arrays.sort(Arrays.java:472) java.util.Collections.sort(Collections.java:155) org.ofbiz.order.shoppingcart.product.ProductPromoWorker.doPromotions(ProductPromoWorker.java:334) org.ofbiz.order.shoppingcart.product.ProductPromoWorker.doPromotions(ProductPromoWorker.java:293) org.ofbiz.order.shoppingcart.ShoppingCartItem.setQuantity(ShoppingCartItem.java:1061) org.ofbiz.order.shoppingcart.ShoppingCartItem.makeItem(ShoppingCartItem.java:502) org.ofbiz.order.shoppingcart.ShoppingCartItem.makeItem(ShoppingCartItem.java:334) org.ofbiz.order.shoppingcart.ShoppingCart.addOrIncreaseItem(ShoppingCart.java:590) org.ofbiz.order.shoppingcart.ShoppingCartHelper.addToCart(ShoppingCartHelper.java:245) org.ofbiz.order.shoppingcart.ShoppingCartEvents.addToCart(ShoppingCartEvents.java:638) sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) java.lang.reflect.Method.invoke(Method.java:606) org.ofbiz.webapp.event.JavaEventHandler.invoke(JavaEventHandler.java:93) org.ofbiz.webapp.event.JavaEventHandler.invoke(JavaEventHandler.java:79) org.ofbiz.webapp.control.RequestHandler.runEvent(RequestHandler.java:749) org.ofbiz.webapp.control.RequestHandler.doRequest(RequestHandler.java:469) org.ofbiz.webapp.control.ControlServlet.doGet(ControlServlet.java:219) org.ofbiz.webapp.control.ControlServlet.doPost(ControlServlet.java:91) javax.servlet.http.HttpServlet.service(HttpServlet.java:641) javax.servlet.http.HttpServlet.service(HttpServlet.java:722) org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:305) org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:210) org.ofbiz.webapp.control.ContextFilter.doFilter(ContextFilter.java:314) org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:243) org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:210) org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:222) org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:123) org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:502) org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:171) org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:100) org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:118) org.apache.catalina.valves.AccessLogValve.invoke(AccessLogValve.java:953) org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:409) org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1044) org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:607) org.apache.tomcat.util.net.JIoEndpoint$SocketProcessor.run(JIoEndpoint.java:315) java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) java.lang.Thread.run(Thread.java:744) -------------------------------------------------------------------------------- -Adrian Quoting [hidden email]: > Author: jacopoc > Date: Mon Dec 30 15:53:07 2013 > New Revision: 1554265 > > URL: http://svn.apache.org/r1554265 > Log: > This is a refactoring of the product promotion engine in order to > overcome some limitations that prevented it to select and apply the > best set of promotions under special conditions. > > Example: Consider two promotions: > * consider two products: Product A, with price $20, and Product B, > with price $40 > * Promotion 1: 20% discount on all the products in a category > containing Product A and Product B > * Promotion 2: 40% discount on Product A > > When Product A and Product B are both in the cart: > * Expected behavior: on Product A the Promotion 2 should be applied > i.e. 40% discount, and on Product B Promotion 1 should be applied > i.e. 20% discount. > ** Summary > Product Price Discount Subtotal > A $20 $8 (40% discount) $12 > B $40 $8 (20% discount) $32 > Total Adjustment: $16 > > * OFBiz behavior (before this fix): Promotion 1 is applied to > Product A and Product B; this happens because the total discount of > Promotion 1 is greater than the total discount of Promotion 2 and > OFBiz applies promotions sorted by discount (desc) > ** Summary > Product Price Discount Subtotal > A $20 $4 (20% discount) $16 > B $40 $8 (20% discount) $32 > Total Adjustment: $12 > > The new solution fixes this issue and similar ones. > > Here are some details about the new algorithm. > > Overview of the flow: > 1) run the promotions one by one in a test run > 2) collect the ProductPromoUse information > 3) sort them by weight (i.e. the ratio between the discount and the > value of the products discounted) > 4) execute the ProductPromoUse in the given order > > In order to understand this solution, and specifically the changes > to ProductPromoWorker.java, there is an important concept to consider: > one Promotion can generate more than one ProductPromoUseInfo objects. > For example if I have 2 units of WG-1111 in the cart (in one cart > item) and I have the promotion “20% discount on WG-1111 and GZ-1000” > then the system will create TWO ProductPromoUseInfo objects both > associated to the same promotion one for each of the 2 units > discounted. > Similarly if I had two lines: 2 units of WG-1111 and 1 unit of > GZ-1000 I would get 3 ProductPromoUseInfo objects 2 objects for > WG-1111 and 1 object for GZ-1000 > > We can sort these ProductPromoUseInfo objects based on their weight > (i.e. the ratio between the discount and the value of the products > discounted) in desc order > and now we have a sorted list of ProductPromoUseInfo objects ready > to be executed > However we only want to execute each of them once and for this > reason we set (in memory, not in the DB) the useLimitPerOrder to 1 > in the first ProductPromoUseInfo of a given promotion and then to 2 > if the same ProductPromoUseInfo is associated to the same promotion > etc... > in this way the end result is that the system will generate, as we > desire, ONE ProductPromoUseInfo only for each of the > ProductPromoUseInfo in the list. > > Here is an example: > we have 2 promotions: > PROMO A > PROMO B > > After test run: > > ProductPromoUseInfo - PROMO A - #1 - weight 0.3 > ProductPromoUseInfo - PROMO A - #2 - weight 0.3 > ProductPromoUseInfo - PROMO B - #1 - weight 0.4 > > After sorting: > > ProductPromoUseInfo - PROMO B - #1 - weight 0.4 > ProductPromoUseInfo - PROMO A - #1 - weight 0.3 > ProductPromoUseInfo - PROMO A - #2 - weight 0.3 > > Based on this we create a list (sortedExplodedProductPromoList) of > ProductPromo: > > PROMO B - with useLimitPerOrder=1 > PROMO A - with useLimitPerOrder=1 > PROMO A - with useLimitPerOrder=2 > > When we apply these to the cart we get the following results: > > PROMO B - with useLimitPerOrder=1 APPLIED > PROMO A - with useLimitPerOrder=1 APPLIED > PROMO A - with useLimitPerOrder=2 NOT APPLIED (because PROMO B used the item) > > > Modified: > > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java > > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java > > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java > > Modified: > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java > URL: > http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java?rev=1554265&r1=1554264&r2=1554265&view=diff > ============================================================================== > --- > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java > (original) > +++ > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java Mon Dec 30 15:53:07 > 2013 > @@ -2714,7 +2714,7 @@ public class ShoppingCart implements Ite > } > itemsTotal = itemsTotal.add(cartItem.getItemSubTotal()); > } > - return itemsTotal; > + return itemsTotal.add(this.getOrderOtherAdjustmentTotal()); > } > > /** > @@ -3142,12 +3142,12 @@ public class ShoppingCart implements Ite > return new HashMap<GenericPK, > String>(this.desiredAlternateGiftByAction); > } > > - public void addProductPromoUse(String productPromoId, String > productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal > quantityLeftInActions) { > + public void addProductPromoUse(String productPromoId, String > productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal > quantityLeftInActions, Map<ShoppingCartItem,BigDecimal> > usageInfoMap) { > if (UtilValidate.isNotEmpty(productPromoCodeId) && > !this.productPromoCodes.contains(productPromoCodeId)) { > throw new IllegalStateException("Cannot add a use to a > promo code use for a code that has not been entered."); > } > if (Debug.verboseOn()) Debug.logVerbose("Used promotion [" > + productPromoId + "] with code [" + productPromoCodeId + "] for > total discount [" + totalDiscountAmount + "] and quantity left in > actions [" + quantityLeftInActions + "]", module); > - this.productPromoUseInfoList.add(new > ProductPromoUseInfo(productPromoId, productPromoCodeId, > totalDiscountAmount, quantityLeftInActions)); > + this.productPromoUseInfoList.add(new > ProductPromoUseInfo(productPromoId, productPromoCodeId, > totalDiscountAmount, quantityLeftInActions, usageInfoMap)); > } > > public void removeProductPromoUse(String productPromoId) { > @@ -4385,23 +4385,43 @@ public class ShoppingCart implements Ite > } > } > > - public static class ProductPromoUseInfo implements Serializable { > + public static class ProductPromoUseInfo implements > Serializable, Comparable<ProductPromoUseInfo> { > public String productPromoId = null; > public String productPromoCodeId = null; > public BigDecimal totalDiscountAmount = BigDecimal.ZERO; > public BigDecimal quantityLeftInActions = BigDecimal.ZERO; > + private Map<ShoppingCartItem,BigDecimal> usageInfoMap = null; > > - public ProductPromoUseInfo(String productPromoId, String > productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal > quantityLeftInActions) { > + public ProductPromoUseInfo(String productPromoId, String > productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal > quantityLeftInActions, Map<ShoppingCartItem,BigDecimal> > usageInfoMap) { > this.productPromoId = productPromoId; > this.productPromoCodeId = productPromoCodeId; > this.totalDiscountAmount = totalDiscountAmount; > this.quantityLeftInActions = quantityLeftInActions; > + this.usageInfoMap = usageInfoMap; > } > > public String getProductPromoId() { return this.productPromoId; } > public String getProductPromoCodeId() { return > this.productPromoCodeId; } > public BigDecimal getTotalDiscountAmount() { return > this.totalDiscountAmount; } > public BigDecimal getQuantityLeftInActions() { return > this.quantityLeftInActions; } > + public Map<ShoppingCartItem,BigDecimal> getUsageInfoMap() { > return this.usageInfoMap; } > + public BigDecimal getUsageWeight() { > + Iterator<ShoppingCartItem> lineItems = > this.usageInfoMap.keySet().iterator(); > + BigDecimal totalAmount = BigDecimal.ZERO; > + while (lineItems.hasNext()) { > + ShoppingCartItem lineItem = lineItems.next(); > + totalAmount = > totalAmount.add(lineItem.getBasePrice().multiply(usageInfoMap.get(lineItem))); > + } > + if (totalAmount.compareTo(BigDecimal.ZERO) == 0) { > + return BigDecimal.ZERO; > + } else { > + return > getTotalDiscountAmount().negate().divide(totalAmount); > + } > + } > + > + public int compareTo(ProductPromoUseInfo other) { > + return other.getUsageWeight().compareTo(getUsageWeight()); > + } > } > > public static class CartShipInfo implements Serializable { > > Modified: > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java > URL: > http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java?rev=1554265&r1=1554264&r2=1554265&view=diff > ============================================================================== > --- > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java > (original) > +++ > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java Mon Dec 30 15:53:07 > 2013 > @@ -21,6 +21,7 @@ package org.ofbiz.order.shoppingcart; > import java.math.BigDecimal; > import java.math.MathContext; > import java.sql.Timestamp; > +import java.util.HashMap; > import java.util.Iterator; > import java.util.List; > import java.util.Locale; > @@ -629,7 +630,7 @@ public class ShoppingCartServices { > cart.addProductPromoCode(productPromoCode, dispatcher); > } > for (GenericValue productPromoUse: orh.getProductPromoUse()) { > - > cart.addProductPromoUse(productPromoUse.getString("productPromoId"), > productPromoUse.getString("productPromoCodeId"), > productPromoUse.getBigDecimal("totalDiscountAmount"), > productPromoUse.getBigDecimal("quantityLeftInActions")); > + > cart.addProductPromoUse(productPromoUse.getString("productPromoId"), > productPromoUse.getString("productPromoCodeId"), > productPromoUse.getBigDecimal("totalDiscountAmount"), > productPromoUse.getBigDecimal("quantityLeftInActions"), new > HashMap<ShoppingCartItem, BigDecimal>()); > } > } > > > Modified: > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java > URL: > http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java?rev=1554265&r1=1554264&r2=1554265&view=diff > ============================================================================== > --- > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java > (original) > +++ > ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java Mon Dec 30 15:53:07 > 2013 > @@ -22,6 +22,8 @@ import java.math.BigDecimal; > import java.math.MathContext; > import java.sql.Timestamp; > import java.util.ArrayList; > +import java.util.Collections; > +import java.util.HashMap; > import java.util.Iterator; > import java.util.List; > import java.util.Locale; > @@ -51,6 +53,7 @@ import org.ofbiz.entity.condition.Entity > import org.ofbiz.entity.util.EntityUtil; > import org.ofbiz.order.shoppingcart.CartItemModifyException; > import org.ofbiz.order.shoppingcart.ShoppingCart; > +import org.ofbiz.order.shoppingcart.ShoppingCart.ProductPromoUseInfo; > import org.ofbiz.order.shoppingcart.ShoppingCartEvents; > import org.ofbiz.order.shoppingcart.ShoppingCartItem; > import org.ofbiz.product.product.ProductContentWrapper; > @@ -318,44 +321,62 @@ public class ProductPromoWorker { > // do a calculate only run through the promotions, then > order by descending totalDiscountAmount for each promotion > // NOTE: on this run, with isolatedTestRun passed as > false it should not apply any adjustments > // or track which cart items are used for which > promotions, but it will track ProductPromoUseInfo and > - // useLimits; we are basicly just trying to run each > promo "independently" to see how much each is worth > + // useLimits; we are basically just trying to run each > promo "independently" to see how much each is worth > runProductPromos(productPromoList, cart, delegator, > dispatcher, nowTimestamp, true); > > - // NOTE: after that first pass we could remove any that > have a 0 totalDiscountAmount from the run list, but we won't because > by the time they are run the cart may have changed enough to get > them to go; also, certain actions like free shipping should always > be run even though we won't know what the totalDiscountAmount is at > the time the promotion is run > - // each ProductPromoUseInfo on the shopping cart will > contain it's total value, so add up all totals for each promoId and > put them in a List of Maps > - // create a List of Maps with productPromo and > totalDiscountAmount, use the Map sorter to sort them descending by > totalDiscountAmount > - > - // before sorting split into two lists and sort each > list; one list for promos that have a order total condition, and the > other list for all promos that don't; then we'll always run the ones > that have no condition on the order total first > - List<Map<Object, Object>> productPromoDiscountMapList = > FastList.newInstance(); > - List<Map<Object, Object>> > productPromoDiscountMapListOrderTotal = FastList.newInstance(); > + // NOTE: we can easily recognize the promos for the > order total: they are the ones with usage set to 0 > + Iterator<ProductPromoUseInfo> promoUses = > cart.getProductPromoUseInfoIter(); > + List<ProductPromoUseInfo> sortedPromoUses = new > ArrayList<ProductPromoUseInfo>(); > + while (promoUses.hasNext()) { > + ProductPromoUseInfo promoUse = promoUses.next(); > + sortedPromoUses.add(promoUse); > + } > + Collections.sort(sortedPromoUses); > + List<GenericValue> sortedExplodedProductPromoList = new > ArrayList<GenericValue>(sortedPromoUses.size()); > + Map<String, Long> usesPerPromo = new HashMap<String, Long>(); > + int indexOfFirstOrderTotalPromo = -1; > + for (ProductPromoUseInfo promoUse: sortedPromoUses) { > + GenericValue productPromo = > delegator.findOne("ProductPromo", UtilMisc.toMap("productPromoId", > promoUse.getProductPromoId()), true); > + GenericValue newProductPromo = > (GenericValue)productPromo.clone(); > + if > (!usesPerPromo.containsKey(promoUse.getProductPromoId())) { > + usesPerPromo.put(promoUse.getProductPromoId(), 0l); > + } > + long uses = usesPerPromo.get(promoUse.getProductPromoId()); > + uses = uses + 1; > + long useLimitPerOrder = > (newProductPromo.get("useLimitPerOrder") != null? > newProductPromo.getLong("useLimitPerOrder"): -1); > + if (useLimitPerOrder == -1 || uses < useLimitPerOrder) { > + newProductPromo.set("useLimitPerOrder", uses); > + } > + usesPerPromo.put(promoUse.getProductPromoId(), uses); > + sortedExplodedProductPromoList.add(newProductPromo); > + if (indexOfFirstOrderTotalPromo == -1 && > BigDecimal.ZERO.equals(promoUse.getUsageWeight())) { > + indexOfFirstOrderTotalPromo = > sortedExplodedProductPromoList.size() - 1; > + } > + } > + if (indexOfFirstOrderTotalPromo == -1) { > + indexOfFirstOrderTotalPromo = > sortedExplodedProductPromoList.size() - 1; > + } > + > for (GenericValue productPromo : productPromoList) { > Map<Object, Object> productPromoDiscountMap = > UtilGenerics.checkMap(UtilMisc.toMap("productPromo", productPromo, > "totalDiscountAmount", > cart.getProductPromoUseTotalDiscount(productPromo.getString("productPromoId")))); > if (hasOrderTotalCondition(productPromo, delegator)) { > - > productPromoDiscountMapListOrderTotal.add(productPromoDiscountMap); > + if > (!usesPerPromo.containsKey(productPromo.getString("productPromoId"))) > { > + sortedExplodedProductPromoList.add(productPromo); > + } > } else { > - > productPromoDiscountMapList.add(productPromoDiscountMap); > + if > (!usesPerPromo.containsKey(productPromo.getString("productPromoId"))) > { > + if (indexOfFirstOrderTotalPromo != -1) { > + > sortedExplodedProductPromoList.add(indexOfFirstOrderTotalPromo, > productPromo); > + } else { > + sortedExplodedProductPromoList.add(0, > productPromo); > + } > + } > } > } > > - > - // sort the Map List, do it ascending because the > discount amounts will be negative, so the lowest number is really > the highest discount > - productPromoDiscountMapList = > UtilMisc.sortMaps(productPromoDiscountMapList, > UtilMisc.toList("+totalDiscountAmount")); > - productPromoDiscountMapListOrderTotal = > UtilMisc.sortMaps(productPromoDiscountMapListOrderTotal, > UtilMisc.toList("+totalDiscountAmount")); > - > - > productPromoDiscountMapList.addAll(productPromoDiscountMapListOrderTotal); > - > - List<GenericValue> sortedProductPromoList = new > ArrayList<GenericValue>(productPromoDiscountMapList.size()); > - Iterator<Map<Object, Object>> > productPromoDiscountMapIter = productPromoDiscountMapList.iterator(); > - while (productPromoDiscountMapIter.hasNext()) { > - Map<Object, Object> productPromoDiscountMap = > UtilGenerics.checkMap(productPromoDiscountMapIter.next()); > - GenericValue productPromo = (GenericValue) > productPromoDiscountMap.get("productPromo"); > - sortedProductPromoList.add(productPromo); > - if (Debug.verboseOn()) Debug.logVerbose("Sorted > Promo [" + productPromo.getString("productPromoId") + "] with total > discount: " + productPromoDiscountMap.get("totalDiscountAmount"), > module); > - } > - > // okay, all ready, do the real run, clearing the > temporary result first... > cart.clearAllPromotionInformation(); > - runProductPromos(sortedProductPromoList, cart, > delegator, dispatcher, nowTimestamp, false); > + runProductPromos(sortedExplodedProductPromoList, cart, > delegator, dispatcher, nowTimestamp, false); > } catch (NumberFormatException e) { > Debug.logError(e, "Number not formatted correctly in > promotion rules, not completed...", module); > } catch (GenericEntityException e) { > @@ -436,7 +457,7 @@ public class ProductPromoWorker { > GenericValue productPromoCode = > productPromoCodeIter.next(); > String productPromoCodeId = > productPromoCode.getString("productPromoCodeId"); > Long codeUseLimit = > getProductPromoCodeUseLimit(productPromoCode, partyId, delegator); > - if (runProductPromoRules(cart, > cartChanged, useLimit, true, productPromoCodeId, codeUseLimit, > maxUseLimit, productPromo, productPromoRules, dispatcher, delegator, > nowTimestamp)) { > + if (runProductPromoRules(cart, > useLimit, true, productPromoCodeId, codeUseLimit, maxUseLimit, > productPromo, productPromoRules, dispatcher, delegator, > nowTimestamp)) { > cartChanged = true; > } > > @@ -448,7 +469,7 @@ public class ProductPromoWorker { > } > } else { > try { > - if (runProductPromoRules(cart, > cartChanged, useLimit, false, null, null, maxUseLimit, productPromo, > productPromoRules, dispatcher, delegator, nowTimestamp)) { > + if (runProductPromoRules(cart, > useLimit, false, null, null, maxUseLimit, productPromo, > productPromoRules, dispatcher, delegator, nowTimestamp)) { > cartChanged = true; > } > } catch (RuntimeException e) { > @@ -735,8 +756,10 @@ public class ProductPromoWorker { > return promoDescBuf.toString(); > } > > - protected static boolean runProductPromoRules(ShoppingCart > cart, boolean cartChanged, Long useLimit, boolean requireCode, > String productPromoCodeId, Long codeUseLimit, long maxUseLimit, > + protected static boolean runProductPromoRules(ShoppingCart > cart, Long useLimit, boolean requireCode, String productPromoCodeId, > Long codeUseLimit, long maxUseLimit, > GenericValue productPromo, List<GenericValue> > productPromoRules, LocalDispatcher dispatcher, Delegator delegator, > Timestamp nowTimestamp) throws GenericEntityException, > UseLimitException { > + boolean cartChanged = false; > + Map<ShoppingCartItem,BigDecimal> usageInfoMap = > prepareProductUsageInfoMap(cart); > String productPromoId = productPromo.getString("productPromoId"); > while ((useLimit == null || useLimit.longValue() > > cart.getProductPromoUseCount(productPromoId)) && > (!requireCode || > UtilValidate.isNotEmpty(productPromoCodeId)) && > @@ -755,17 +778,17 @@ public class ProductPromoWorker { > // loop through conditions for rule, if any false, > set allConditionsTrue to false > List<GenericValue> productPromoConds = > delegator.findByAnd("ProductPromoCond", > UtilMisc.toMap("productPromoId", > productPromo.get("productPromoId")), > UtilMisc.toList("productPromoCondSeqId"), true); > productPromoConds = > EntityUtil.filterByAnd(productPromoConds, > UtilMisc.toMap("productPromoRuleId", > productPromoRule.get("productPromoRuleId"))); > - // using the other method to consolodate cache > entries because the same cache is used elsewhere: List > productPromoConds = productPromoRule.getRelated("ProductPromoCond", > null, UtilMisc.toList("productPromoCondSeqId"), true); > + // using the other method to consolidate cache > entries because the same cache is used elsewhere: List > productPromoConds = productPromoRule.getRelated("ProductPromoCond", > null, UtilMisc.toList("productPromoCondSeqId"), true); > if (Debug.verboseOn()) Debug.logVerbose("Checking " > + productPromoConds.size() + " conditions for rule " + > productPromoRule, module); > > Iterator<GenericValue> productPromoCondIter = > UtilMisc.toIterator(productPromoConds); > while (productPromoCondIter != null && > productPromoCondIter.hasNext()) { > GenericValue productPromoCond = > productPromoCondIter.next(); > > - boolean condResult = > checkCondition(productPromoCond, cart, delegator, dispatcher, > nowTimestamp); > + boolean conditionSatisfied = > checkCondition(productPromoCond, cart, delegator, dispatcher, > nowTimestamp); > > // any false condition will cause it to NOT > perform the action > - if (condResult == false) { > + if (!conditionSatisfied) { > performActions = false; > break; > } > @@ -797,13 +820,16 @@ public class ProductPromoWorker { > } > > if (promoUsed) { > - > cart.addProductPromoUse(productPromo.getString("productPromoId"), > productPromoCodeId, totalDiscountAmount, quantityLeftInActions); > + // Get product use information from the cart > + Map<ShoppingCartItem,BigDecimal> newUsageInfoMap = > prepareProductUsageInfoMap(cart); > + Map<ShoppingCartItem,BigDecimal> deltaUsageInfoMap > = prepareDeltaProductUsageInfoMap(usageInfoMap, newUsageInfoMap); > + usageInfoMap = newUsageInfoMap; > + > cart.addProductPromoUse(productPromo.getString("productPromoId"), > productPromoCodeId, totalDiscountAmount, quantityLeftInActions, > deltaUsageInfoMap); > } else { > // the promotion was not used, don't try again > until we finish a full pass and come back to see the promo > conditions are now satisfied based on changes to the cart > break; > } > > - > if (cart.getProductPromoUseCount(productPromoId) > > maxUseLimit) { > throw new UseLimitException("ERROR: While > calculating promotions the promotion [" + productPromoId + "] action > was applied more than " + maxUseLimit + " times, so the calculation > has been ended. This should generally never happen unless you have > bad rule definitions."); > } > @@ -812,6 +838,34 @@ public class ProductPromoWorker { > return cartChanged; > } > > + private static Map<ShoppingCartItem,BigDecimal> > prepareProductUsageInfoMap(ShoppingCart cart) { > + Map<ShoppingCartItem,BigDecimal> usageInfoMap = new > HashMap<ShoppingCartItem, BigDecimal>(); > + List<ShoppingCartItem> lineOrderedByBasePriceList = > cart.getLineListOrderedByBasePrice(false); > + for (ShoppingCartItem cartItem : lineOrderedByBasePriceList) { > + BigDecimal used = cartItem.getPromoQuantityUsed(); > + if (used.compareTo(BigDecimal.ZERO) != 0) { > + usageInfoMap.put(cartItem, used); > + } > + } > + return usageInfoMap; > + } > + > + 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); > + if (newUsed.compareTo(oldUsed) > 0) { > + deltaUsageInfoMap.put(cartLine, > newUsed.add(oldUsed.negate())); > + } else { > + deltaUsageInfoMap.remove(cartLine); > + } > + } > + return deltaUsageInfoMap; > + } > + > protected static boolean checkCondition(GenericValue > productPromoCond, ShoppingCart cart, Delegator delegator, > LocalDispatcher dispatcher, Timestamp nowTimestamp) throws > GenericEntityException { > String condValue = productPromoCond.getString("condValue"); > String otherValue = productPromoCond.getString("otherValue"); > @@ -1772,8 +1826,8 @@ public class ProductPromoWorker { > actionResultInfo.ranAction = false; > } > > + // in action, if doesn't have enough quantity to use the > promo at all, remove candidate promo uses and increment > promoQuantityUsed; this should go for all actions, if any action > runs we confirm > if (actionResultInfo.ranAction) { > - // in action, if doesn't have enough quantity to use > the promo at all, remove candidate promo uses and increment > promoQuantityUsed; this should go for all actions, if any action > runs we confirm > > cart.confirmPromoRuleUse(productPromoAction.getString("productPromoId"), > productPromoAction.getString("productPromoRuleId")); > } else { > > cart.resetPromoRuleUse(productPromoAction.getString("productPromoId"), > productPromoAction.getString("productPromoRuleId")); > > > |
Thanks Adrian,
I am looking into this. Jacopo On Jan 14, 2014, at 6:24 PM, [hidden email] wrote: > This commit breaks promotions. > > 1. Create a new sales order, customer DemoCustomer. > 2. Add GZ-8544, qty 1 to the order. > 3. Promo items do not appear in order. > 4. Log shows an exception being thrown: > > 2014-01-14 12:15:56,030 (http-bio-0.0.0.0-8443-exec-1) [ ProductPromoWorker.java:385:ERROR] > ---- runtime exception report -------------------------------------------------- > Error running promotions, will ignore: java.lang.ArithmeticException: Non-terminating decimal expansion; no exact representable decimal result. > Exception: java.lang.ArithmeticException > Message: Non-terminating decimal expansion; no exact representable decimal result. > ---- stack trace --------------------------------------------------------------- > java.lang.ArithmeticException: Non-terminating decimal expansion; no exact representable decimal result. > java.math.BigDecimal.divide(BigDecimal.java:1616) > org.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo.getUsageWeight(ShoppingCart.java:4418) > org.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo.compareTo(ShoppingCart.java:4423) > org.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo.compareTo(ShoppingCart.java:4388) > java.util.ComparableTimSort.countRunAndMakeAscending(ComparableTimSort.java:290) > java.util.ComparableTimSort.sort(ComparableTimSort.java:157) > java.util.ComparableTimSort.sort(ComparableTimSort.java:146) > java.util.Arrays.sort(Arrays.java:472) > java.util.Collections.sort(Collections.java:155) > org.ofbiz.order.shoppingcart.product.ProductPromoWorker.doPromotions(ProductPromoWorker.java:334) > org.ofbiz.order.shoppingcart.product.ProductPromoWorker.doPromotions(ProductPromoWorker.java:293) > org.ofbiz.order.shoppingcart.ShoppingCartItem.setQuantity(ShoppingCartItem.java:1061) > org.ofbiz.order.shoppingcart.ShoppingCartItem.makeItem(ShoppingCartItem.java:502) > org.ofbiz.order.shoppingcart.ShoppingCartItem.makeItem(ShoppingCartItem.java:334) > org.ofbiz.order.shoppingcart.ShoppingCart.addOrIncreaseItem(ShoppingCart.java:590) > org.ofbiz.order.shoppingcart.ShoppingCartHelper.addToCart(ShoppingCartHelper.java:245) > org.ofbiz.order.shoppingcart.ShoppingCartEvents.addToCart(ShoppingCartEvents.java:638) > sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) > sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > java.lang.reflect.Method.invoke(Method.java:606) > org.ofbiz.webapp.event.JavaEventHandler.invoke(JavaEventHandler.java:93) > org.ofbiz.webapp.event.JavaEventHandler.invoke(JavaEventHandler.java:79) > org.ofbiz.webapp.control.RequestHandler.runEvent(RequestHandler.java:749) > org.ofbiz.webapp.control.RequestHandler.doRequest(RequestHandler.java:469) > org.ofbiz.webapp.control.ControlServlet.doGet(ControlServlet.java:219) > org.ofbiz.webapp.control.ControlServlet.doPost(ControlServlet.java:91) > javax.servlet.http.HttpServlet.service(HttpServlet.java:641) > javax.servlet.http.HttpServlet.service(HttpServlet.java:722) > org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:305) > org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:210) > org.ofbiz.webapp.control.ContextFilter.doFilter(ContextFilter.java:314) > org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:243) > org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:210) > org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:222) > org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:123) > org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:502) > org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:171) > org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:100) > org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:118) > org.apache.catalina.valves.AccessLogValve.invoke(AccessLogValve.java:953) > org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:409) > org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1044) > org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:607) > org.apache.tomcat.util.net.JIoEndpoint$SocketProcessor.run(JIoEndpoint.java:315) > java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) > java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) > java.lang.Thread.run(Thread.java:744) > -------------------------------------------------------------------------------- > > > -Adrian > > > > Quoting [hidden email]: > >> Author: jacopoc >> Date: Mon Dec 30 15:53:07 2013 >> New Revision: 1554265 >> >> URL: http://svn.apache.org/r1554265 >> Log: >> This is a refactoring of the product promotion engine in order to overcome some limitations that prevented it to select and apply the best set of promotions under special conditions. >> >> Example: Consider two promotions: >> * consider two products: Product A, with price $20, and Product B, with price $40 >> * Promotion 1: 20% discount on all the products in a category containing Product A and Product B >> * Promotion 2: 40% discount on Product A >> >> When Product A and Product B are both in the cart: >> * Expected behavior: on Product A the Promotion 2 should be applied i.e. 40% discount, and on Product B Promotion 1 should be applied i.e. 20% discount. >> ** Summary >> Product Price Discount Subtotal >> A $20 $8 (40% discount) $12 >> B $40 $8 (20% discount) $32 >> Total Adjustment: $16 >> >> * OFBiz behavior (before this fix): Promotion 1 is applied to Product A and Product B; this happens because the total discount of Promotion 1 is greater than the total discount of Promotion 2 and OFBiz applies promotions sorted by discount (desc) >> ** Summary >> Product Price Discount Subtotal >> A $20 $4 (20% discount) $16 >> B $40 $8 (20% discount) $32 >> Total Adjustment: $12 >> >> The new solution fixes this issue and similar ones. >> >> Here are some details about the new algorithm. >> >> Overview of the flow: >> 1) run the promotions one by one in a test run >> 2) collect the ProductPromoUse information >> 3) sort them by weight (i.e. the ratio between the discount and the value of the products discounted) >> 4) execute the ProductPromoUse in the given order >> >> In order to understand this solution, and specifically the changes to ProductPromoWorker.java, there is an important concept to consider: >> one Promotion can generate more than one ProductPromoUseInfo objects. >> For example if I have 2 units of WG-1111 in the cart (in one cart item) and I have the promotion “20% discount on WG-1111 and GZ-1000” then the system will create TWO ProductPromoUseInfo objects both associated to the same promotion one for each of the 2 units discounted. >> Similarly if I had two lines: 2 units of WG-1111 and 1 unit of GZ-1000 I would get 3 ProductPromoUseInfo objects 2 objects for WG-1111 and 1 object for GZ-1000 >> >> We can sort these ProductPromoUseInfo objects based on their weight (i.e. the ratio between the discount and the value of the products discounted) in desc order >> and now we have a sorted list of ProductPromoUseInfo objects ready to be executed >> However we only want to execute each of them once and for this reason we set (in memory, not in the DB) the useLimitPerOrder to 1 in the first ProductPromoUseInfo of a given promotion and then to 2 if the same ProductPromoUseInfo is associated to the same promotion etc... >> in this way the end result is that the system will generate, as we desire, ONE ProductPromoUseInfo only for each of the ProductPromoUseInfo in the list. >> >> Here is an example: >> we have 2 promotions: >> PROMO A >> PROMO B >> >> After test run: >> >> ProductPromoUseInfo - PROMO A - #1 - weight 0.3 >> ProductPromoUseInfo - PROMO A - #2 - weight 0.3 >> ProductPromoUseInfo - PROMO B - #1 - weight 0.4 >> >> After sorting: >> >> ProductPromoUseInfo - PROMO B - #1 - weight 0.4 >> ProductPromoUseInfo - PROMO A - #1 - weight 0.3 >> ProductPromoUseInfo - PROMO A - #2 - weight 0.3 >> >> Based on this we create a list (sortedExplodedProductPromoList) of ProductPromo: >> >> PROMO B - with useLimitPerOrder=1 >> PROMO A - with useLimitPerOrder=1 >> PROMO A - with useLimitPerOrder=2 >> >> When we apply these to the cart we get the following results: >> >> PROMO B - with useLimitPerOrder=1 APPLIED >> PROMO A - with useLimitPerOrder=1 APPLIED >> PROMO A - with useLimitPerOrder=2 NOT APPLIED (because PROMO B used the item) >> >> >> Modified: >> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java >> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java >> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java >> >> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java?rev=1554265&r1=1554264&r2=1554265&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java (original) >> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java Mon Dec 30 15:53:07 2013 >> @@ -2714,7 +2714,7 @@ public class ShoppingCart implements Ite >> } >> itemsTotal = itemsTotal.add(cartItem.getItemSubTotal()); >> } >> - return itemsTotal; >> + return itemsTotal.add(this.getOrderOtherAdjustmentTotal()); >> } >> >> /** >> @@ -3142,12 +3142,12 @@ public class ShoppingCart implements Ite >> return new HashMap<GenericPK, String>(this.desiredAlternateGiftByAction); >> } >> >> - public void addProductPromoUse(String productPromoId, String productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal quantityLeftInActions) { >> + public void addProductPromoUse(String productPromoId, String productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal quantityLeftInActions, Map<ShoppingCartItem,BigDecimal> usageInfoMap) { >> if (UtilValidate.isNotEmpty(productPromoCodeId) && !this.productPromoCodes.contains(productPromoCodeId)) { >> throw new IllegalStateException("Cannot add a use to a promo code use for a code that has not been entered."); >> } >> if (Debug.verboseOn()) Debug.logVerbose("Used promotion [" + productPromoId + "] with code [" + productPromoCodeId + "] for total discount [" + totalDiscountAmount + "] and quantity left in actions [" + quantityLeftInActions + "]", module); >> - this.productPromoUseInfoList.add(new ProductPromoUseInfo(productPromoId, productPromoCodeId, totalDiscountAmount, quantityLeftInActions)); >> + this.productPromoUseInfoList.add(new ProductPromoUseInfo(productPromoId, productPromoCodeId, totalDiscountAmount, quantityLeftInActions, usageInfoMap)); >> } >> >> public void removeProductPromoUse(String productPromoId) { >> @@ -4385,23 +4385,43 @@ public class ShoppingCart implements Ite >> } >> } >> >> - public static class ProductPromoUseInfo implements Serializable { >> + public static class ProductPromoUseInfo implements Serializable, Comparable<ProductPromoUseInfo> { >> public String productPromoId = null; >> public String productPromoCodeId = null; >> public BigDecimal totalDiscountAmount = BigDecimal.ZERO; >> public BigDecimal quantityLeftInActions = BigDecimal.ZERO; >> + private Map<ShoppingCartItem,BigDecimal> usageInfoMap = null; >> >> - public ProductPromoUseInfo(String productPromoId, String productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal quantityLeftInActions) { >> + public ProductPromoUseInfo(String productPromoId, String productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal quantityLeftInActions, Map<ShoppingCartItem,BigDecimal> usageInfoMap) { >> this.productPromoId = productPromoId; >> this.productPromoCodeId = productPromoCodeId; >> this.totalDiscountAmount = totalDiscountAmount; >> this.quantityLeftInActions = quantityLeftInActions; >> + this.usageInfoMap = usageInfoMap; >> } >> >> public String getProductPromoId() { return this.productPromoId; } >> public String getProductPromoCodeId() { return this.productPromoCodeId; } >> public BigDecimal getTotalDiscountAmount() { return this.totalDiscountAmount; } >> public BigDecimal getQuantityLeftInActions() { return this.quantityLeftInActions; } >> + public Map<ShoppingCartItem,BigDecimal> getUsageInfoMap() { return this.usageInfoMap; } >> + public BigDecimal getUsageWeight() { >> + Iterator<ShoppingCartItem> lineItems = this.usageInfoMap.keySet().iterator(); >> + BigDecimal totalAmount = BigDecimal.ZERO; >> + while (lineItems.hasNext()) { >> + ShoppingCartItem lineItem = lineItems.next(); >> + totalAmount = totalAmount.add(lineItem.getBasePrice().multiply(usageInfoMap.get(lineItem))); >> + } >> + if (totalAmount.compareTo(BigDecimal.ZERO) == 0) { >> + return BigDecimal.ZERO; >> + } else { >> + return getTotalDiscountAmount().negate().divide(totalAmount); >> + } >> + } >> + >> + public int compareTo(ProductPromoUseInfo other) { >> + return other.getUsageWeight().compareTo(getUsageWeight()); >> + } >> } >> >> public static class CartShipInfo implements Serializable { >> >> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java?rev=1554265&r1=1554264&r2=1554265&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java (original) >> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java Mon Dec 30 15:53:07 2013 >> @@ -21,6 +21,7 @@ package org.ofbiz.order.shoppingcart; >> import java.math.BigDecimal; >> import java.math.MathContext; >> import java.sql.Timestamp; >> +import java.util.HashMap; >> import java.util.Iterator; >> import java.util.List; >> import java.util.Locale; >> @@ -629,7 +630,7 @@ public class ShoppingCartServices { >> cart.addProductPromoCode(productPromoCode, dispatcher); >> } >> for (GenericValue productPromoUse: orh.getProductPromoUse()) { >> - cart.addProductPromoUse(productPromoUse.getString("productPromoId"), productPromoUse.getString("productPromoCodeId"), productPromoUse.getBigDecimal("totalDiscountAmount"), productPromoUse.getBigDecimal("quantityLeftInActions")); >> + cart.addProductPromoUse(productPromoUse.getString("productPromoId"), productPromoUse.getString("productPromoCodeId"), productPromoUse.getBigDecimal("totalDiscountAmount"), productPromoUse.getBigDecimal("quantityLeftInActions"), new HashMap<ShoppingCartItem, BigDecimal>()); >> } >> } >> >> >> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java >> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java?rev=1554265&r1=1554264&r2=1554265&view=diff >> ============================================================================== >> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java (original) >> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java Mon Dec 30 15:53:07 2013 >> @@ -22,6 +22,8 @@ import java.math.BigDecimal; >> import java.math.MathContext; >> import java.sql.Timestamp; >> import java.util.ArrayList; >> +import java.util.Collections; >> +import java.util.HashMap; >> import java.util.Iterator; >> import java.util.List; >> import java.util.Locale; >> @@ -51,6 +53,7 @@ import org.ofbiz.entity.condition.Entity >> import org.ofbiz.entity.util.EntityUtil; >> import org.ofbiz.order.shoppingcart.CartItemModifyException; >> import org.ofbiz.order.shoppingcart.ShoppingCart; >> +import org.ofbiz.order.shoppingcart.ShoppingCart.ProductPromoUseInfo; >> import org.ofbiz.order.shoppingcart.ShoppingCartEvents; >> import org.ofbiz.order.shoppingcart.ShoppingCartItem; >> import org.ofbiz.product.product.ProductContentWrapper; >> @@ -318,44 +321,62 @@ public class ProductPromoWorker { >> // do a calculate only run through the promotions, then order by descending totalDiscountAmount for each promotion >> // NOTE: on this run, with isolatedTestRun passed as false it should not apply any adjustments >> // or track which cart items are used for which promotions, but it will track ProductPromoUseInfo and >> - // useLimits; we are basicly just trying to run each promo "independently" to see how much each is worth >> + // useLimits; we are basically just trying to run each promo "independently" to see how much each is worth >> runProductPromos(productPromoList, cart, delegator, dispatcher, nowTimestamp, true); >> >> - // NOTE: after that first pass we could remove any that have a 0 totalDiscountAmount from the run list, but we won't because by the time they are run the cart may have changed enough to get them to go; also, certain actions like free shipping should always be run even though we won't know what the totalDiscountAmount is at the time the promotion is run >> - // each ProductPromoUseInfo on the shopping cart will contain it's total value, so add up all totals for each promoId and put them in a List of Maps >> - // create a List of Maps with productPromo and totalDiscountAmount, use the Map sorter to sort them descending by totalDiscountAmount >> - >> - // before sorting split into two lists and sort each list; one list for promos that have a order total condition, and the other list for all promos that don't; then we'll always run the ones that have no condition on the order total first >> - List<Map<Object, Object>> productPromoDiscountMapList = FastList.newInstance(); >> - List<Map<Object, Object>> productPromoDiscountMapListOrderTotal = FastList.newInstance(); >> + // NOTE: we can easily recognize the promos for the order total: they are the ones with usage set to 0 >> + Iterator<ProductPromoUseInfo> promoUses = cart.getProductPromoUseInfoIter(); >> + List<ProductPromoUseInfo> sortedPromoUses = new ArrayList<ProductPromoUseInfo>(); >> + while (promoUses.hasNext()) { >> + ProductPromoUseInfo promoUse = promoUses.next(); >> + sortedPromoUses.add(promoUse); >> + } >> + Collections.sort(sortedPromoUses); >> + List<GenericValue> sortedExplodedProductPromoList = new ArrayList<GenericValue>(sortedPromoUses.size()); >> + Map<String, Long> usesPerPromo = new HashMap<String, Long>(); >> + int indexOfFirstOrderTotalPromo = -1; >> + for (ProductPromoUseInfo promoUse: sortedPromoUses) { >> + GenericValue productPromo = delegator.findOne("ProductPromo", UtilMisc.toMap("productPromoId", promoUse.getProductPromoId()), true); >> + GenericValue newProductPromo = (GenericValue)productPromo.clone(); >> + if (!usesPerPromo.containsKey(promoUse.getProductPromoId())) { >> + usesPerPromo.put(promoUse.getProductPromoId(), 0l); >> + } >> + long uses = usesPerPromo.get(promoUse.getProductPromoId()); >> + uses = uses + 1; >> + long useLimitPerOrder = (newProductPromo.get("useLimitPerOrder") != null? newProductPromo.getLong("useLimitPerOrder"): -1); >> + if (useLimitPerOrder == -1 || uses < useLimitPerOrder) { >> + newProductPromo.set("useLimitPerOrder", uses); >> + } >> + usesPerPromo.put(promoUse.getProductPromoId(), uses); >> + sortedExplodedProductPromoList.add(newProductPromo); >> + if (indexOfFirstOrderTotalPromo == -1 && BigDecimal.ZERO.equals(promoUse.getUsageWeight())) { >> + indexOfFirstOrderTotalPromo = sortedExplodedProductPromoList.size() - 1; >> + } >> + } >> + if (indexOfFirstOrderTotalPromo == -1) { >> + indexOfFirstOrderTotalPromo = sortedExplodedProductPromoList.size() - 1; >> + } >> + >> for (GenericValue productPromo : productPromoList) { >> Map<Object, Object> productPromoDiscountMap = UtilGenerics.checkMap(UtilMisc.toMap("productPromo", productPromo, "totalDiscountAmount", cart.getProductPromoUseTotalDiscount(productPromo.getString("productPromoId")))); >> if (hasOrderTotalCondition(productPromo, delegator)) { >> - productPromoDiscountMapListOrderTotal.add(productPromoDiscountMap); >> + if (!usesPerPromo.containsKey(productPromo.getString("productPromoId"))) { >> + sortedExplodedProductPromoList.add(productPromo); >> + } >> } else { >> - productPromoDiscountMapList.add(productPromoDiscountMap); >> + if (!usesPerPromo.containsKey(productPromo.getString("productPromoId"))) { >> + if (indexOfFirstOrderTotalPromo != -1) { >> + sortedExplodedProductPromoList.add(indexOfFirstOrderTotalPromo, productPromo); >> + } else { >> + sortedExplodedProductPromoList.add(0, productPromo); >> + } >> + } >> } >> } >> >> - >> - // sort the Map List, do it ascending because the discount amounts will be negative, so the lowest number is really the highest discount >> - productPromoDiscountMapList = UtilMisc.sortMaps(productPromoDiscountMapList, UtilMisc.toList("+totalDiscountAmount")); >> - productPromoDiscountMapListOrderTotal = UtilMisc.sortMaps(productPromoDiscountMapListOrderTotal, UtilMisc.toList("+totalDiscountAmount")); >> - >> - productPromoDiscountMapList.addAll(productPromoDiscountMapListOrderTotal); >> - >> - List<GenericValue> sortedProductPromoList = new ArrayList<GenericValue>(productPromoDiscountMapList.size()); >> - Iterator<Map<Object, Object>> productPromoDiscountMapIter = productPromoDiscountMapList.iterator(); >> - while (productPromoDiscountMapIter.hasNext()) { >> - Map<Object, Object> productPromoDiscountMap = UtilGenerics.checkMap(productPromoDiscountMapIter.next()); >> - GenericValue productPromo = (GenericValue) productPromoDiscountMap.get("productPromo"); >> - sortedProductPromoList.add(productPromo); >> - if (Debug.verboseOn()) Debug.logVerbose("Sorted Promo [" + productPromo.getString("productPromoId") + "] with total discount: " + productPromoDiscountMap.get("totalDiscountAmount"), module); >> - } >> - >> // okay, all ready, do the real run, clearing the temporary result first... >> cart.clearAllPromotionInformation(); >> - runProductPromos(sortedProductPromoList, cart, delegator, dispatcher, nowTimestamp, false); >> + runProductPromos(sortedExplodedProductPromoList, cart, delegator, dispatcher, nowTimestamp, false); >> } catch (NumberFormatException e) { >> Debug.logError(e, "Number not formatted correctly in promotion rules, not completed...", module); >> } catch (GenericEntityException e) { >> @@ -436,7 +457,7 @@ public class ProductPromoWorker { >> GenericValue productPromoCode = productPromoCodeIter.next(); >> String productPromoCodeId = productPromoCode.getString("productPromoCodeId"); >> Long codeUseLimit = getProductPromoCodeUseLimit(productPromoCode, partyId, delegator); >> - if (runProductPromoRules(cart, cartChanged, useLimit, true, productPromoCodeId, codeUseLimit, maxUseLimit, productPromo, productPromoRules, dispatcher, delegator, nowTimestamp)) { >> + if (runProductPromoRules(cart, useLimit, true, productPromoCodeId, codeUseLimit, maxUseLimit, productPromo, productPromoRules, dispatcher, delegator, nowTimestamp)) { >> cartChanged = true; >> } >> >> @@ -448,7 +469,7 @@ public class ProductPromoWorker { >> } >> } else { >> try { >> - if (runProductPromoRules(cart, cartChanged, useLimit, false, null, null, maxUseLimit, productPromo, productPromoRules, dispatcher, delegator, nowTimestamp)) { >> + if (runProductPromoRules(cart, useLimit, false, null, null, maxUseLimit, productPromo, productPromoRules, dispatcher, delegator, nowTimestamp)) { >> cartChanged = true; >> } >> } catch (RuntimeException e) { >> @@ -735,8 +756,10 @@ public class ProductPromoWorker { >> return promoDescBuf.toString(); >> } >> >> - protected static boolean runProductPromoRules(ShoppingCart cart, boolean cartChanged, Long useLimit, boolean requireCode, String productPromoCodeId, Long codeUseLimit, long maxUseLimit, >> + protected static boolean runProductPromoRules(ShoppingCart cart, Long useLimit, boolean requireCode, String productPromoCodeId, Long codeUseLimit, long maxUseLimit, >> GenericValue productPromo, List<GenericValue> productPromoRules, LocalDispatcher dispatcher, Delegator delegator, Timestamp nowTimestamp) throws GenericEntityException, UseLimitException { >> + boolean cartChanged = false; >> + Map<ShoppingCartItem,BigDecimal> usageInfoMap = prepareProductUsageInfoMap(cart); >> String productPromoId = productPromo.getString("productPromoId"); >> while ((useLimit == null || useLimit.longValue() > cart.getProductPromoUseCount(productPromoId)) && >> (!requireCode || UtilValidate.isNotEmpty(productPromoCodeId)) && >> @@ -755,17 +778,17 @@ public class ProductPromoWorker { >> // loop through conditions for rule, if any false, set allConditionsTrue to false >> List<GenericValue> productPromoConds = delegator.findByAnd("ProductPromoCond", UtilMisc.toMap("productPromoId", productPromo.get("productPromoId")), UtilMisc.toList("productPromoCondSeqId"), true); >> productPromoConds = EntityUtil.filterByAnd(productPromoConds, UtilMisc.toMap("productPromoRuleId", productPromoRule.get("productPromoRuleId"))); >> - // using the other method to consolodate cache entries because the same cache is used elsewhere: List productPromoConds = productPromoRule.getRelated("ProductPromoCond", null, UtilMisc.toList("productPromoCondSeqId"), true); >> + // using the other method to consolidate cache entries because the same cache is used elsewhere: List productPromoConds = productPromoRule.getRelated("ProductPromoCond", null, UtilMisc.toList("productPromoCondSeqId"), true); >> if (Debug.verboseOn()) Debug.logVerbose("Checking " + productPromoConds.size() + " conditions for rule " + productPromoRule, module); >> >> Iterator<GenericValue> productPromoCondIter = UtilMisc.toIterator(productPromoConds); >> while (productPromoCondIter != null && productPromoCondIter.hasNext()) { >> GenericValue productPromoCond = productPromoCondIter.next(); >> >> - boolean condResult = checkCondition(productPromoCond, cart, delegator, dispatcher, nowTimestamp); >> + boolean conditionSatisfied = checkCondition(productPromoCond, cart, delegator, dispatcher, nowTimestamp); >> >> // any false condition will cause it to NOT perform the action >> - if (condResult == false) { >> + if (!conditionSatisfied) { >> performActions = false; >> break; >> } >> @@ -797,13 +820,16 @@ public class ProductPromoWorker { >> } >> >> if (promoUsed) { >> - cart.addProductPromoUse(productPromo.getString("productPromoId"), productPromoCodeId, totalDiscountAmount, quantityLeftInActions); >> + // Get product use information from the cart >> + Map<ShoppingCartItem,BigDecimal> newUsageInfoMap = prepareProductUsageInfoMap(cart); >> + Map<ShoppingCartItem,BigDecimal> deltaUsageInfoMap = prepareDeltaProductUsageInfoMap(usageInfoMap, newUsageInfoMap); >> + usageInfoMap = newUsageInfoMap; >> + cart.addProductPromoUse(productPromo.getString("productPromoId"), productPromoCodeId, totalDiscountAmount, quantityLeftInActions, deltaUsageInfoMap); >> } else { >> // the promotion was not used, don't try again until we finish a full pass and come back to see the promo conditions are now satisfied based on changes to the cart >> break; >> } >> >> - >> if (cart.getProductPromoUseCount(productPromoId) > maxUseLimit) { >> throw new UseLimitException("ERROR: While calculating promotions the promotion [" + productPromoId + "] action was applied more than " + maxUseLimit + " times, so the calculation has been ended. This should generally never happen unless you have bad rule definitions."); >> } >> @@ -812,6 +838,34 @@ public class ProductPromoWorker { >> return cartChanged; >> } >> >> + private static Map<ShoppingCartItem,BigDecimal> prepareProductUsageInfoMap(ShoppingCart cart) { >> + Map<ShoppingCartItem,BigDecimal> usageInfoMap = new HashMap<ShoppingCartItem, BigDecimal>(); >> + List<ShoppingCartItem> lineOrderedByBasePriceList = cart.getLineListOrderedByBasePrice(false); >> + for (ShoppingCartItem cartItem : lineOrderedByBasePriceList) { >> + BigDecimal used = cartItem.getPromoQuantityUsed(); >> + if (used.compareTo(BigDecimal.ZERO) != 0) { >> + usageInfoMap.put(cartItem, used); >> + } >> + } >> + return usageInfoMap; >> + } >> + >> + 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); >> + if (newUsed.compareTo(oldUsed) > 0) { >> + deltaUsageInfoMap.put(cartLine, newUsed.add(oldUsed.negate())); >> + } else { >> + deltaUsageInfoMap.remove(cartLine); >> + } >> + } >> + return deltaUsageInfoMap; >> + } >> + >> protected static boolean checkCondition(GenericValue productPromoCond, ShoppingCart cart, Delegator delegator, LocalDispatcher dispatcher, Timestamp nowTimestamp) throws GenericEntityException { >> String condValue = productPromoCond.getString("condValue"); >> String otherValue = productPromoCond.getString("otherValue"); >> @@ -1772,8 +1826,8 @@ public class ProductPromoWorker { >> actionResultInfo.ranAction = false; >> } >> >> + // in action, if doesn't have enough quantity to use the promo at all, remove candidate promo uses and increment promoQuantityUsed; this should go for all actions, if any action runs we confirm >> if (actionResultInfo.ranAction) { >> - // in action, if doesn't have enough quantity to use the promo at all, remove candidate promo uses and increment promoQuantityUsed; this should go for all actions, if any action runs we confirm >> cart.confirmPromoRuleUse(productPromoAction.getString("productPromoId"), productPromoAction.getString("productPromoRuleId")); >> } else { >> cart.resetPromoRuleUse(productPromoAction.getString("productPromoId"), productPromoAction.getString("productPromoRuleId")); >> >> >> > > > |
Fixed at rev. 1558145
Thanks for the report. Jacopo On Jan 14, 2014, at 6:36 PM, Jacopo Cappellato <[hidden email]> wrote: > Thanks Adrian, > > I am looking into this. > > Jacopo > > On Jan 14, 2014, at 6:24 PM, [hidden email] wrote: > >> This commit breaks promotions. >> >> 1. Create a new sales order, customer DemoCustomer. >> 2. Add GZ-8544, qty 1 to the order. >> 3. Promo items do not appear in order. >> 4. Log shows an exception being thrown: >> >> 2014-01-14 12:15:56,030 (http-bio-0.0.0.0-8443-exec-1) [ ProductPromoWorker.java:385:ERROR] >> ---- runtime exception report -------------------------------------------------- >> Error running promotions, will ignore: java.lang.ArithmeticException: Non-terminating decimal expansion; no exact representable decimal result. >> Exception: java.lang.ArithmeticException >> Message: Non-terminating decimal expansion; no exact representable decimal result. >> ---- stack trace --------------------------------------------------------------- >> java.lang.ArithmeticException: Non-terminating decimal expansion; no exact representable decimal result. >> java.math.BigDecimal.divide(BigDecimal.java:1616) >> org.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo.getUsageWeight(ShoppingCart.java:4418) >> org.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo.compareTo(ShoppingCart.java:4423) >> org.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo.compareTo(ShoppingCart.java:4388) >> java.util.ComparableTimSort.countRunAndMakeAscending(ComparableTimSort.java:290) >> java.util.ComparableTimSort.sort(ComparableTimSort.java:157) >> java.util.ComparableTimSort.sort(ComparableTimSort.java:146) >> java.util.Arrays.sort(Arrays.java:472) >> java.util.Collections.sort(Collections.java:155) >> org.ofbiz.order.shoppingcart.product.ProductPromoWorker.doPromotions(ProductPromoWorker.java:334) >> org.ofbiz.order.shoppingcart.product.ProductPromoWorker.doPromotions(ProductPromoWorker.java:293) >> org.ofbiz.order.shoppingcart.ShoppingCartItem.setQuantity(ShoppingCartItem.java:1061) >> org.ofbiz.order.shoppingcart.ShoppingCartItem.makeItem(ShoppingCartItem.java:502) >> org.ofbiz.order.shoppingcart.ShoppingCartItem.makeItem(ShoppingCartItem.java:334) >> org.ofbiz.order.shoppingcart.ShoppingCart.addOrIncreaseItem(ShoppingCart.java:590) >> org.ofbiz.order.shoppingcart.ShoppingCartHelper.addToCart(ShoppingCartHelper.java:245) >> org.ofbiz.order.shoppingcart.ShoppingCartEvents.addToCart(ShoppingCartEvents.java:638) >> sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) >> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) >> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) >> java.lang.reflect.Method.invoke(Method.java:606) >> org.ofbiz.webapp.event.JavaEventHandler.invoke(JavaEventHandler.java:93) >> org.ofbiz.webapp.event.JavaEventHandler.invoke(JavaEventHandler.java:79) >> org.ofbiz.webapp.control.RequestHandler.runEvent(RequestHandler.java:749) >> org.ofbiz.webapp.control.RequestHandler.doRequest(RequestHandler.java:469) >> org.ofbiz.webapp.control.ControlServlet.doGet(ControlServlet.java:219) >> org.ofbiz.webapp.control.ControlServlet.doPost(ControlServlet.java:91) >> javax.servlet.http.HttpServlet.service(HttpServlet.java:641) >> javax.servlet.http.HttpServlet.service(HttpServlet.java:722) >> org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:305) >> org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:210) >> org.ofbiz.webapp.control.ContextFilter.doFilter(ContextFilter.java:314) >> org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:243) >> org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:210) >> org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:222) >> org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:123) >> org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:502) >> org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:171) >> org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:100) >> org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:118) >> org.apache.catalina.valves.AccessLogValve.invoke(AccessLogValve.java:953) >> org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:409) >> org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1044) >> org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:607) >> org.apache.tomcat.util.net.JIoEndpoint$SocketProcessor.run(JIoEndpoint.java:315) >> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) >> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) >> java.lang.Thread.run(Thread.java:744) >> -------------------------------------------------------------------------------- >> >> >> -Adrian >> >> >> >> Quoting [hidden email]: >> >>> Author: jacopoc >>> Date: Mon Dec 30 15:53:07 2013 >>> New Revision: 1554265 >>> >>> URL: http://svn.apache.org/r1554265 >>> Log: >>> This is a refactoring of the product promotion engine in order to overcome some limitations that prevented it to select and apply the best set of promotions under special conditions. >>> >>> Example: Consider two promotions: >>> * consider two products: Product A, with price $20, and Product B, with price $40 >>> * Promotion 1: 20% discount on all the products in a category containing Product A and Product B >>> * Promotion 2: 40% discount on Product A >>> >>> When Product A and Product B are both in the cart: >>> * Expected behavior: on Product A the Promotion 2 should be applied i.e. 40% discount, and on Product B Promotion 1 should be applied i.e. 20% discount. >>> ** Summary >>> Product Price Discount Subtotal >>> A $20 $8 (40% discount) $12 >>> B $40 $8 (20% discount) $32 >>> Total Adjustment: $16 >>> >>> * OFBiz behavior (before this fix): Promotion 1 is applied to Product A and Product B; this happens because the total discount of Promotion 1 is greater than the total discount of Promotion 2 and OFBiz applies promotions sorted by discount (desc) >>> ** Summary >>> Product Price Discount Subtotal >>> A $20 $4 (20% discount) $16 >>> B $40 $8 (20% discount) $32 >>> Total Adjustment: $12 >>> >>> The new solution fixes this issue and similar ones. >>> >>> Here are some details about the new algorithm. >>> >>> Overview of the flow: >>> 1) run the promotions one by one in a test run >>> 2) collect the ProductPromoUse information >>> 3) sort them by weight (i.e. the ratio between the discount and the value of the products discounted) >>> 4) execute the ProductPromoUse in the given order >>> >>> In order to understand this solution, and specifically the changes to ProductPromoWorker.java, there is an important concept to consider: >>> one Promotion can generate more than one ProductPromoUseInfo objects. >>> For example if I have 2 units of WG-1111 in the cart (in one cart item) and I have the promotion “20% discount on WG-1111 and GZ-1000” then the system will create TWO ProductPromoUseInfo objects both associated to the same promotion one for each of the 2 units discounted. >>> Similarly if I had two lines: 2 units of WG-1111 and 1 unit of GZ-1000 I would get 3 ProductPromoUseInfo objects 2 objects for WG-1111 and 1 object for GZ-1000 >>> >>> We can sort these ProductPromoUseInfo objects based on their weight (i.e. the ratio between the discount and the value of the products discounted) in desc order >>> and now we have a sorted list of ProductPromoUseInfo objects ready to be executed >>> However we only want to execute each of them once and for this reason we set (in memory, not in the DB) the useLimitPerOrder to 1 in the first ProductPromoUseInfo of a given promotion and then to 2 if the same ProductPromoUseInfo is associated to the same promotion etc... >>> in this way the end result is that the system will generate, as we desire, ONE ProductPromoUseInfo only for each of the ProductPromoUseInfo in the list. >>> >>> Here is an example: >>> we have 2 promotions: >>> PROMO A >>> PROMO B >>> >>> After test run: >>> >>> ProductPromoUseInfo - PROMO A - #1 - weight 0.3 >>> ProductPromoUseInfo - PROMO A - #2 - weight 0.3 >>> ProductPromoUseInfo - PROMO B - #1 - weight 0.4 >>> >>> After sorting: >>> >>> ProductPromoUseInfo - PROMO B - #1 - weight 0.4 >>> ProductPromoUseInfo - PROMO A - #1 - weight 0.3 >>> ProductPromoUseInfo - PROMO A - #2 - weight 0.3 >>> >>> Based on this we create a list (sortedExplodedProductPromoList) of ProductPromo: >>> >>> PROMO B - with useLimitPerOrder=1 >>> PROMO A - with useLimitPerOrder=1 >>> PROMO A - with useLimitPerOrder=2 >>> >>> When we apply these to the cart we get the following results: >>> >>> PROMO B - with useLimitPerOrder=1 APPLIED >>> PROMO A - with useLimitPerOrder=1 APPLIED >>> PROMO A - with useLimitPerOrder=2 NOT APPLIED (because PROMO B used the item) >>> >>> >>> Modified: >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java >>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java >>> >>> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java >>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java?rev=1554265&r1=1554264&r2=1554265&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java (original) >>> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java Mon Dec 30 15:53:07 2013 >>> @@ -2714,7 +2714,7 @@ public class ShoppingCart implements Ite >>> } >>> itemsTotal = itemsTotal.add(cartItem.getItemSubTotal()); >>> } >>> - return itemsTotal; >>> + return itemsTotal.add(this.getOrderOtherAdjustmentTotal()); >>> } >>> >>> /** >>> @@ -3142,12 +3142,12 @@ public class ShoppingCart implements Ite >>> return new HashMap<GenericPK, String>(this.desiredAlternateGiftByAction); >>> } >>> >>> - public void addProductPromoUse(String productPromoId, String productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal quantityLeftInActions) { >>> + public void addProductPromoUse(String productPromoId, String productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal quantityLeftInActions, Map<ShoppingCartItem,BigDecimal> usageInfoMap) { >>> if (UtilValidate.isNotEmpty(productPromoCodeId) && !this.productPromoCodes.contains(productPromoCodeId)) { >>> throw new IllegalStateException("Cannot add a use to a promo code use for a code that has not been entered."); >>> } >>> if (Debug.verboseOn()) Debug.logVerbose("Used promotion [" + productPromoId + "] with code [" + productPromoCodeId + "] for total discount [" + totalDiscountAmount + "] and quantity left in actions [" + quantityLeftInActions + "]", module); >>> - this.productPromoUseInfoList.add(new ProductPromoUseInfo(productPromoId, productPromoCodeId, totalDiscountAmount, quantityLeftInActions)); >>> + this.productPromoUseInfoList.add(new ProductPromoUseInfo(productPromoId, productPromoCodeId, totalDiscountAmount, quantityLeftInActions, usageInfoMap)); >>> } >>> >>> public void removeProductPromoUse(String productPromoId) { >>> @@ -4385,23 +4385,43 @@ public class ShoppingCart implements Ite >>> } >>> } >>> >>> - public static class ProductPromoUseInfo implements Serializable { >>> + public static class ProductPromoUseInfo implements Serializable, Comparable<ProductPromoUseInfo> { >>> public String productPromoId = null; >>> public String productPromoCodeId = null; >>> public BigDecimal totalDiscountAmount = BigDecimal.ZERO; >>> public BigDecimal quantityLeftInActions = BigDecimal.ZERO; >>> + private Map<ShoppingCartItem,BigDecimal> usageInfoMap = null; >>> >>> - public ProductPromoUseInfo(String productPromoId, String productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal quantityLeftInActions) { >>> + public ProductPromoUseInfo(String productPromoId, String productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal quantityLeftInActions, Map<ShoppingCartItem,BigDecimal> usageInfoMap) { >>> this.productPromoId = productPromoId; >>> this.productPromoCodeId = productPromoCodeId; >>> this.totalDiscountAmount = totalDiscountAmount; >>> this.quantityLeftInActions = quantityLeftInActions; >>> + this.usageInfoMap = usageInfoMap; >>> } >>> >>> public String getProductPromoId() { return this.productPromoId; } >>> public String getProductPromoCodeId() { return this.productPromoCodeId; } >>> public BigDecimal getTotalDiscountAmount() { return this.totalDiscountAmount; } >>> public BigDecimal getQuantityLeftInActions() { return this.quantityLeftInActions; } >>> + public Map<ShoppingCartItem,BigDecimal> getUsageInfoMap() { return this.usageInfoMap; } >>> + public BigDecimal getUsageWeight() { >>> + Iterator<ShoppingCartItem> lineItems = this.usageInfoMap.keySet().iterator(); >>> + BigDecimal totalAmount = BigDecimal.ZERO; >>> + while (lineItems.hasNext()) { >>> + ShoppingCartItem lineItem = lineItems.next(); >>> + totalAmount = totalAmount.add(lineItem.getBasePrice().multiply(usageInfoMap.get(lineItem))); >>> + } >>> + if (totalAmount.compareTo(BigDecimal.ZERO) == 0) { >>> + return BigDecimal.ZERO; >>> + } else { >>> + return getTotalDiscountAmount().negate().divide(totalAmount); >>> + } >>> + } >>> + >>> + public int compareTo(ProductPromoUseInfo other) { >>> + return other.getUsageWeight().compareTo(getUsageWeight()); >>> + } >>> } >>> >>> public static class CartShipInfo implements Serializable { >>> >>> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java >>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java?rev=1554265&r1=1554264&r2=1554265&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java (original) >>> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java Mon Dec 30 15:53:07 2013 >>> @@ -21,6 +21,7 @@ package org.ofbiz.order.shoppingcart; >>> import java.math.BigDecimal; >>> import java.math.MathContext; >>> import java.sql.Timestamp; >>> +import java.util.HashMap; >>> import java.util.Iterator; >>> import java.util.List; >>> import java.util.Locale; >>> @@ -629,7 +630,7 @@ public class ShoppingCartServices { >>> cart.addProductPromoCode(productPromoCode, dispatcher); >>> } >>> for (GenericValue productPromoUse: orh.getProductPromoUse()) { >>> - cart.addProductPromoUse(productPromoUse.getString("productPromoId"), productPromoUse.getString("productPromoCodeId"), productPromoUse.getBigDecimal("totalDiscountAmount"), productPromoUse.getBigDecimal("quantityLeftInActions")); >>> + cart.addProductPromoUse(productPromoUse.getString("productPromoId"), productPromoUse.getString("productPromoCodeId"), productPromoUse.getBigDecimal("totalDiscountAmount"), productPromoUse.getBigDecimal("quantityLeftInActions"), new HashMap<ShoppingCartItem, BigDecimal>()); >>> } >>> } >>> >>> >>> Modified: ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java >>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java?rev=1554265&r1=1554264&r2=1554265&view=diff >>> ============================================================================== >>> --- ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java (original) >>> +++ ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java Mon Dec 30 15:53:07 2013 >>> @@ -22,6 +22,8 @@ import java.math.BigDecimal; >>> import java.math.MathContext; >>> import java.sql.Timestamp; >>> import java.util.ArrayList; >>> +import java.util.Collections; >>> +import java.util.HashMap; >>> import java.util.Iterator; >>> import java.util.List; >>> import java.util.Locale; >>> @@ -51,6 +53,7 @@ import org.ofbiz.entity.condition.Entity >>> import org.ofbiz.entity.util.EntityUtil; >>> import org.ofbiz.order.shoppingcart.CartItemModifyException; >>> import org.ofbiz.order.shoppingcart.ShoppingCart; >>> +import org.ofbiz.order.shoppingcart.ShoppingCart.ProductPromoUseInfo; >>> import org.ofbiz.order.shoppingcart.ShoppingCartEvents; >>> import org.ofbiz.order.shoppingcart.ShoppingCartItem; >>> import org.ofbiz.product.product.ProductContentWrapper; >>> @@ -318,44 +321,62 @@ public class ProductPromoWorker { >>> // do a calculate only run through the promotions, then order by descending totalDiscountAmount for each promotion >>> // NOTE: on this run, with isolatedTestRun passed as false it should not apply any adjustments >>> // or track which cart items are used for which promotions, but it will track ProductPromoUseInfo and >>> - // useLimits; we are basicly just trying to run each promo "independently" to see how much each is worth >>> + // useLimits; we are basically just trying to run each promo "independently" to see how much each is worth >>> runProductPromos(productPromoList, cart, delegator, dispatcher, nowTimestamp, true); >>> >>> - // NOTE: after that first pass we could remove any that have a 0 totalDiscountAmount from the run list, but we won't because by the time they are run the cart may have changed enough to get them to go; also, certain actions like free shipping should always be run even though we won't know what the totalDiscountAmount is at the time the promotion is run >>> - // each ProductPromoUseInfo on the shopping cart will contain it's total value, so add up all totals for each promoId and put them in a List of Maps >>> - // create a List of Maps with productPromo and totalDiscountAmount, use the Map sorter to sort them descending by totalDiscountAmount >>> - >>> - // before sorting split into two lists and sort each list; one list for promos that have a order total condition, and the other list for all promos that don't; then we'll always run the ones that have no condition on the order total first >>> - List<Map<Object, Object>> productPromoDiscountMapList = FastList.newInstance(); >>> - List<Map<Object, Object>> productPromoDiscountMapListOrderTotal = FastList.newInstance(); >>> + // NOTE: we can easily recognize the promos for the order total: they are the ones with usage set to 0 >>> + Iterator<ProductPromoUseInfo> promoUses = cart.getProductPromoUseInfoIter(); >>> + List<ProductPromoUseInfo> sortedPromoUses = new ArrayList<ProductPromoUseInfo>(); >>> + while (promoUses.hasNext()) { >>> + ProductPromoUseInfo promoUse = promoUses.next(); >>> + sortedPromoUses.add(promoUse); >>> + } >>> + Collections.sort(sortedPromoUses); >>> + List<GenericValue> sortedExplodedProductPromoList = new ArrayList<GenericValue>(sortedPromoUses.size()); >>> + Map<String, Long> usesPerPromo = new HashMap<String, Long>(); >>> + int indexOfFirstOrderTotalPromo = -1; >>> + for (ProductPromoUseInfo promoUse: sortedPromoUses) { >>> + GenericValue productPromo = delegator.findOne("ProductPromo", UtilMisc.toMap("productPromoId", promoUse.getProductPromoId()), true); >>> + GenericValue newProductPromo = (GenericValue)productPromo.clone(); >>> + if (!usesPerPromo.containsKey(promoUse.getProductPromoId())) { >>> + usesPerPromo.put(promoUse.getProductPromoId(), 0l); >>> + } >>> + long uses = usesPerPromo.get(promoUse.getProductPromoId()); >>> + uses = uses + 1; >>> + long useLimitPerOrder = (newProductPromo.get("useLimitPerOrder") != null? newProductPromo.getLong("useLimitPerOrder"): -1); >>> + if (useLimitPerOrder == -1 || uses < useLimitPerOrder) { >>> + newProductPromo.set("useLimitPerOrder", uses); >>> + } >>> + usesPerPromo.put(promoUse.getProductPromoId(), uses); >>> + sortedExplodedProductPromoList.add(newProductPromo); >>> + if (indexOfFirstOrderTotalPromo == -1 && BigDecimal.ZERO.equals(promoUse.getUsageWeight())) { >>> + indexOfFirstOrderTotalPromo = sortedExplodedProductPromoList.size() - 1; >>> + } >>> + } >>> + if (indexOfFirstOrderTotalPromo == -1) { >>> + indexOfFirstOrderTotalPromo = sortedExplodedProductPromoList.size() - 1; >>> + } >>> + >>> for (GenericValue productPromo : productPromoList) { >>> Map<Object, Object> productPromoDiscountMap = UtilGenerics.checkMap(UtilMisc.toMap("productPromo", productPromo, "totalDiscountAmount", cart.getProductPromoUseTotalDiscount(productPromo.getString("productPromoId")))); >>> if (hasOrderTotalCondition(productPromo, delegator)) { >>> - productPromoDiscountMapListOrderTotal.add(productPromoDiscountMap); >>> + if (!usesPerPromo.containsKey(productPromo.getString("productPromoId"))) { >>> + sortedExplodedProductPromoList.add(productPromo); >>> + } >>> } else { >>> - productPromoDiscountMapList.add(productPromoDiscountMap); >>> + if (!usesPerPromo.containsKey(productPromo.getString("productPromoId"))) { >>> + if (indexOfFirstOrderTotalPromo != -1) { >>> + sortedExplodedProductPromoList.add(indexOfFirstOrderTotalPromo, productPromo); >>> + } else { >>> + sortedExplodedProductPromoList.add(0, productPromo); >>> + } >>> + } >>> } >>> } >>> >>> - >>> - // sort the Map List, do it ascending because the discount amounts will be negative, so the lowest number is really the highest discount >>> - productPromoDiscountMapList = UtilMisc.sortMaps(productPromoDiscountMapList, UtilMisc.toList("+totalDiscountAmount")); >>> - productPromoDiscountMapListOrderTotal = UtilMisc.sortMaps(productPromoDiscountMapListOrderTotal, UtilMisc.toList("+totalDiscountAmount")); >>> - >>> - productPromoDiscountMapList.addAll(productPromoDiscountMapListOrderTotal); >>> - >>> - List<GenericValue> sortedProductPromoList = new ArrayList<GenericValue>(productPromoDiscountMapList.size()); >>> - Iterator<Map<Object, Object>> productPromoDiscountMapIter = productPromoDiscountMapList.iterator(); >>> - while (productPromoDiscountMapIter.hasNext()) { >>> - Map<Object, Object> productPromoDiscountMap = UtilGenerics.checkMap(productPromoDiscountMapIter.next()); >>> - GenericValue productPromo = (GenericValue) productPromoDiscountMap.get("productPromo"); >>> - sortedProductPromoList.add(productPromo); >>> - if (Debug.verboseOn()) Debug.logVerbose("Sorted Promo [" + productPromo.getString("productPromoId") + "] with total discount: " + productPromoDiscountMap.get("totalDiscountAmount"), module); >>> - } >>> - >>> // okay, all ready, do the real run, clearing the temporary result first... >>> cart.clearAllPromotionInformation(); >>> - runProductPromos(sortedProductPromoList, cart, delegator, dispatcher, nowTimestamp, false); >>> + runProductPromos(sortedExplodedProductPromoList, cart, delegator, dispatcher, nowTimestamp, false); >>> } catch (NumberFormatException e) { >>> Debug.logError(e, "Number not formatted correctly in promotion rules, not completed...", module); >>> } catch (GenericEntityException e) { >>> @@ -436,7 +457,7 @@ public class ProductPromoWorker { >>> GenericValue productPromoCode = productPromoCodeIter.next(); >>> String productPromoCodeId = productPromoCode.getString("productPromoCodeId"); >>> Long codeUseLimit = getProductPromoCodeUseLimit(productPromoCode, partyId, delegator); >>> - if (runProductPromoRules(cart, cartChanged, useLimit, true, productPromoCodeId, codeUseLimit, maxUseLimit, productPromo, productPromoRules, dispatcher, delegator, nowTimestamp)) { >>> + if (runProductPromoRules(cart, useLimit, true, productPromoCodeId, codeUseLimit, maxUseLimit, productPromo, productPromoRules, dispatcher, delegator, nowTimestamp)) { >>> cartChanged = true; >>> } >>> >>> @@ -448,7 +469,7 @@ public class ProductPromoWorker { >>> } >>> } else { >>> try { >>> - if (runProductPromoRules(cart, cartChanged, useLimit, false, null, null, maxUseLimit, productPromo, productPromoRules, dispatcher, delegator, nowTimestamp)) { >>> + if (runProductPromoRules(cart, useLimit, false, null, null, maxUseLimit, productPromo, productPromoRules, dispatcher, delegator, nowTimestamp)) { >>> cartChanged = true; >>> } >>> } catch (RuntimeException e) { >>> @@ -735,8 +756,10 @@ public class ProductPromoWorker { >>> return promoDescBuf.toString(); >>> } >>> >>> - protected static boolean runProductPromoRules(ShoppingCart cart, boolean cartChanged, Long useLimit, boolean requireCode, String productPromoCodeId, Long codeUseLimit, long maxUseLimit, >>> + protected static boolean runProductPromoRules(ShoppingCart cart, Long useLimit, boolean requireCode, String productPromoCodeId, Long codeUseLimit, long maxUseLimit, >>> GenericValue productPromo, List<GenericValue> productPromoRules, LocalDispatcher dispatcher, Delegator delegator, Timestamp nowTimestamp) throws GenericEntityException, UseLimitException { >>> + boolean cartChanged = false; >>> + Map<ShoppingCartItem,BigDecimal> usageInfoMap = prepareProductUsageInfoMap(cart); >>> String productPromoId = productPromo.getString("productPromoId"); >>> while ((useLimit == null || useLimit.longValue() > cart.getProductPromoUseCount(productPromoId)) && >>> (!requireCode || UtilValidate.isNotEmpty(productPromoCodeId)) && >>> @@ -755,17 +778,17 @@ public class ProductPromoWorker { >>> // loop through conditions for rule, if any false, set allConditionsTrue to false >>> List<GenericValue> productPromoConds = delegator.findByAnd("ProductPromoCond", UtilMisc.toMap("productPromoId", productPromo.get("productPromoId")), UtilMisc.toList("productPromoCondSeqId"), true); >>> productPromoConds = EntityUtil.filterByAnd(productPromoConds, UtilMisc.toMap("productPromoRuleId", productPromoRule.get("productPromoRuleId"))); >>> - // using the other method to consolodate cache entries because the same cache is used elsewhere: List productPromoConds = productPromoRule.getRelated("ProductPromoCond", null, UtilMisc.toList("productPromoCondSeqId"), true); >>> + // using the other method to consolidate cache entries because the same cache is used elsewhere: List productPromoConds = productPromoRule.getRelated("ProductPromoCond", null, UtilMisc.toList("productPromoCondSeqId"), true); >>> if (Debug.verboseOn()) Debug.logVerbose("Checking " + productPromoConds.size() + " conditions for rule " + productPromoRule, module); >>> >>> Iterator<GenericValue> productPromoCondIter = UtilMisc.toIterator(productPromoConds); >>> while (productPromoCondIter != null && productPromoCondIter.hasNext()) { >>> GenericValue productPromoCond = productPromoCondIter.next(); >>> >>> - boolean condResult = checkCondition(productPromoCond, cart, delegator, dispatcher, nowTimestamp); >>> + boolean conditionSatisfied = checkCondition(productPromoCond, cart, delegator, dispatcher, nowTimestamp); >>> >>> // any false condition will cause it to NOT perform the action >>> - if (condResult == false) { >>> + if (!conditionSatisfied) { >>> performActions = false; >>> break; >>> } >>> @@ -797,13 +820,16 @@ public class ProductPromoWorker { >>> } >>> >>> if (promoUsed) { >>> - cart.addProductPromoUse(productPromo.getString("productPromoId"), productPromoCodeId, totalDiscountAmount, quantityLeftInActions); >>> + // Get product use information from the cart >>> + Map<ShoppingCartItem,BigDecimal> newUsageInfoMap = prepareProductUsageInfoMap(cart); >>> + Map<ShoppingCartItem,BigDecimal> deltaUsageInfoMap = prepareDeltaProductUsageInfoMap(usageInfoMap, newUsageInfoMap); >>> + usageInfoMap = newUsageInfoMap; >>> + cart.addProductPromoUse(productPromo.getString("productPromoId"), productPromoCodeId, totalDiscountAmount, quantityLeftInActions, deltaUsageInfoMap); >>> } else { >>> // the promotion was not used, don't try again until we finish a full pass and come back to see the promo conditions are now satisfied based on changes to the cart >>> break; >>> } >>> >>> - >>> if (cart.getProductPromoUseCount(productPromoId) > maxUseLimit) { >>> throw new UseLimitException("ERROR: While calculating promotions the promotion [" + productPromoId + "] action was applied more than " + maxUseLimit + " times, so the calculation has been ended. This should generally never happen unless you have bad rule definitions."); >>> } >>> @@ -812,6 +838,34 @@ public class ProductPromoWorker { >>> return cartChanged; >>> } >>> >>> + private static Map<ShoppingCartItem,BigDecimal> prepareProductUsageInfoMap(ShoppingCart cart) { >>> + Map<ShoppingCartItem,BigDecimal> usageInfoMap = new HashMap<ShoppingCartItem, BigDecimal>(); >>> + List<ShoppingCartItem> lineOrderedByBasePriceList = cart.getLineListOrderedByBasePrice(false); >>> + for (ShoppingCartItem cartItem : lineOrderedByBasePriceList) { >>> + BigDecimal used = cartItem.getPromoQuantityUsed(); >>> + if (used.compareTo(BigDecimal.ZERO) != 0) { >>> + usageInfoMap.put(cartItem, used); >>> + } >>> + } >>> + return usageInfoMap; >>> + } >>> + >>> + 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); >>> + if (newUsed.compareTo(oldUsed) > 0) { >>> + deltaUsageInfoMap.put(cartLine, newUsed.add(oldUsed.negate())); >>> + } else { >>> + deltaUsageInfoMap.remove(cartLine); >>> + } >>> + } >>> + return deltaUsageInfoMap; >>> + } >>> + >>> protected static boolean checkCondition(GenericValue productPromoCond, ShoppingCart cart, Delegator delegator, LocalDispatcher dispatcher, Timestamp nowTimestamp) throws GenericEntityException { >>> String condValue = productPromoCond.getString("condValue"); >>> String otherValue = productPromoCond.getString("otherValue"); >>> @@ -1772,8 +1826,8 @@ public class ProductPromoWorker { >>> actionResultInfo.ranAction = false; >>> } >>> >>> + // in action, if doesn't have enough quantity to use the promo at all, remove candidate promo uses and increment promoQuantityUsed; this should go for all actions, if any action runs we confirm >>> if (actionResultInfo.ranAction) { >>> - // in action, if doesn't have enough quantity to use the promo at all, remove candidate promo uses and increment promoQuantityUsed; this should go for all actions, if any action runs we confirm >>> cart.confirmPromoRuleUse(productPromoAction.getString("productPromoId"), productPromoAction.getString("productPromoRuleId")); >>> } else { >>> cart.resetPromoRuleUse(productPromoAction.getString("productPromoId"), productPromoAction.getString("productPromoRuleId")); >>> >>> >>> >> >> >> > |
Thanks Jacopo!
-Adrian Quoting Jacopo Cappellato <[hidden email]>: > Fixed at rev. 1558145 > Thanks for the report. > > Jacopo > > On Jan 14, 2014, at 6:36 PM, Jacopo Cappellato > <[hidden email]> wrote: > >> Thanks Adrian, >> >> I am looking into this. >> >> Jacopo >> >> On Jan 14, 2014, at 6:24 PM, [hidden email] wrote: >> >>> This commit breaks promotions. >>> >>> 1. Create a new sales order, customer DemoCustomer. >>> 2. Add GZ-8544, qty 1 to the order. >>> 3. Promo items do not appear in order. >>> 4. Log shows an exception being thrown: >>> >>> 2014-01-14 12:15:56,030 (http-bio-0.0.0.0-8443-exec-1) [ >>> ProductPromoWorker.java:385:ERROR] >>> ---- runtime exception report >>> -------------------------------------------------- >>> Error running promotions, will ignore: >>> java.lang.ArithmeticException: Non-terminating decimal expansion; >>> no exact representable decimal result. >>> Exception: java.lang.ArithmeticException >>> Message: Non-terminating decimal expansion; no exact representable >>> decimal result. >>> ---- stack trace >>> --------------------------------------------------------------- >>> java.lang.ArithmeticException: Non-terminating decimal expansion; >>> no exact representable decimal result. >>> java.math.BigDecimal.divide(BigDecimal.java:1616) >>> org.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo.getUsageWeight(ShoppingCart.java:4418) >>> org.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo.compareTo(ShoppingCart.java:4423) >>> org.ofbiz.order.shoppingcart.ShoppingCart$ProductPromoUseInfo.compareTo(ShoppingCart.java:4388) >>> java.util.ComparableTimSort.countRunAndMakeAscending(ComparableTimSort.java:290) >>> java.util.ComparableTimSort.sort(ComparableTimSort.java:157) >>> java.util.ComparableTimSort.sort(ComparableTimSort.java:146) >>> java.util.Arrays.sort(Arrays.java:472) >>> java.util.Collections.sort(Collections.java:155) >>> org.ofbiz.order.shoppingcart.product.ProductPromoWorker.doPromotions(ProductPromoWorker.java:334) >>> org.ofbiz.order.shoppingcart.product.ProductPromoWorker.doPromotions(ProductPromoWorker.java:293) >>> org.ofbiz.order.shoppingcart.ShoppingCartItem.setQuantity(ShoppingCartItem.java:1061) >>> org.ofbiz.order.shoppingcart.ShoppingCartItem.makeItem(ShoppingCartItem.java:502) >>> org.ofbiz.order.shoppingcart.ShoppingCartItem.makeItem(ShoppingCartItem.java:334) >>> org.ofbiz.order.shoppingcart.ShoppingCart.addOrIncreaseItem(ShoppingCart.java:590) >>> org.ofbiz.order.shoppingcart.ShoppingCartHelper.addToCart(ShoppingCartHelper.java:245) >>> org.ofbiz.order.shoppingcart.ShoppingCartEvents.addToCart(ShoppingCartEvents.java:638) >>> sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) >>> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) >>> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) >>> java.lang.reflect.Method.invoke(Method.java:606) >>> org.ofbiz.webapp.event.JavaEventHandler.invoke(JavaEventHandler.java:93) >>> org.ofbiz.webapp.event.JavaEventHandler.invoke(JavaEventHandler.java:79) >>> org.ofbiz.webapp.control.RequestHandler.runEvent(RequestHandler.java:749) >>> org.ofbiz.webapp.control.RequestHandler.doRequest(RequestHandler.java:469) >>> org.ofbiz.webapp.control.ControlServlet.doGet(ControlServlet.java:219) >>> org.ofbiz.webapp.control.ControlServlet.doPost(ControlServlet.java:91) >>> javax.servlet.http.HttpServlet.service(HttpServlet.java:641) >>> javax.servlet.http.HttpServlet.service(HttpServlet.java:722) >>> org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:305) >>> org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:210) >>> org.ofbiz.webapp.control.ContextFilter.doFilter(ContextFilter.java:314) >>> org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:243) >>> org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:210) >>> org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:222) >>> org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:123) >>> org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:502) >>> org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:171) >>> org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:100) >>> org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:118) >>> org.apache.catalina.valves.AccessLogValve.invoke(AccessLogValve.java:953) >>> org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:409) >>> org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1044) >>> org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:607) >>> org.apache.tomcat.util.net.JIoEndpoint$SocketProcessor.run(JIoEndpoint.java:315) >>> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) >>> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) >>> java.lang.Thread.run(Thread.java:744) >>> -------------------------------------------------------------------------------- >>> >>> >>> -Adrian >>> >>> >>> >>> Quoting [hidden email]: >>> >>>> Author: jacopoc >>>> Date: Mon Dec 30 15:53:07 2013 >>>> New Revision: 1554265 >>>> >>>> URL: http://svn.apache.org/r1554265 >>>> Log: >>>> This is a refactoring of the product promotion engine in order to >>>> overcome some limitations that prevented it to select and apply >>>> the best set of promotions under special conditions. >>>> >>>> Example: Consider two promotions: >>>> * consider two products: Product A, with price $20, and Product >>>> B, with price $40 >>>> * Promotion 1: 20% discount on all the products in a category >>>> containing Product A and Product B >>>> * Promotion 2: 40% discount on Product A >>>> >>>> When Product A and Product B are both in the cart: >>>> * Expected behavior: on Product A the Promotion 2 should be >>>> applied i.e. 40% discount, and on Product B Promotion 1 should be >>>> applied i.e. 20% discount. >>>> ** Summary >>>> Product Price Discount Subtotal >>>> A $20 $8 (40% discount) $12 >>>> B $40 $8 (20% discount) $32 >>>> Total Adjustment: $16 >>>> >>>> * OFBiz behavior (before this fix): Promotion 1 is applied to >>>> Product A and Product B; this happens because the total discount >>>> of Promotion 1 is greater than the total discount of Promotion 2 >>>> and OFBiz applies promotions sorted by discount (desc) >>>> ** Summary >>>> Product Price Discount Subtotal >>>> A $20 $4 (20% discount) $16 >>>> B $40 $8 (20% discount) $32 >>>> Total Adjustment: $12 >>>> >>>> The new solution fixes this issue and similar ones. >>>> >>>> Here are some details about the new algorithm. >>>> >>>> Overview of the flow: >>>> 1) run the promotions one by one in a test run >>>> 2) collect the ProductPromoUse information >>>> 3) sort them by weight (i.e. the ratio between the discount and >>>> the value of the products discounted) >>>> 4) execute the ProductPromoUse in the given order >>>> >>>> In order to understand this solution, and specifically the >>>> changes to ProductPromoWorker.java, there is an important concept >>>> to consider: >>>> one Promotion can generate more than one ProductPromoUseInfo objects. >>>> For example if I have 2 units of WG-1111 in the cart (in one cart >>>> item) and I have the promotion “20% discount on WG-1111 and >>>> GZ-1000” then the system will create TWO ProductPromoUseInfo >>>> objects both associated to the same promotion one for each of the >>>> 2 units discounted. >>>> Similarly if I had two lines: 2 units of WG-1111 and 1 unit of >>>> GZ-1000 I would get 3 ProductPromoUseInfo objects 2 objects for >>>> WG-1111 and 1 object for GZ-1000 >>>> >>>> We can sort these ProductPromoUseInfo objects based on their >>>> weight (i.e. the ratio between the discount and the value of the >>>> products discounted) in desc order >>>> and now we have a sorted list of ProductPromoUseInfo objects >>>> ready to be executed >>>> However we only want to execute each of them once and for this >>>> reason we set (in memory, not in the DB) the useLimitPerOrder to >>>> 1 in the first ProductPromoUseInfo of a given promotion and then >>>> to 2 if the same ProductPromoUseInfo is associated to the same >>>> promotion etc... >>>> in this way the end result is that the system will generate, as >>>> we desire, ONE ProductPromoUseInfo only for each of the >>>> ProductPromoUseInfo in the list. >>>> >>>> Here is an example: >>>> we have 2 promotions: >>>> PROMO A >>>> PROMO B >>>> >>>> After test run: >>>> >>>> ProductPromoUseInfo - PROMO A - #1 - weight 0.3 >>>> ProductPromoUseInfo - PROMO A - #2 - weight 0.3 >>>> ProductPromoUseInfo - PROMO B - #1 - weight 0.4 >>>> >>>> After sorting: >>>> >>>> ProductPromoUseInfo - PROMO B - #1 - weight 0.4 >>>> ProductPromoUseInfo - PROMO A - #1 - weight 0.3 >>>> ProductPromoUseInfo - PROMO A - #2 - weight 0.3 >>>> >>>> Based on this we create a list (sortedExplodedProductPromoList) >>>> of ProductPromo: >>>> >>>> PROMO B - with useLimitPerOrder=1 >>>> PROMO A - with useLimitPerOrder=1 >>>> PROMO A - with useLimitPerOrder=2 >>>> >>>> When we apply these to the cart we get the following results: >>>> >>>> PROMO B - with useLimitPerOrder=1 APPLIED >>>> PROMO A - with useLimitPerOrder=1 APPLIED >>>> PROMO A - with useLimitPerOrder=2 NOT APPLIED (because PROMO B >>>> used the item) >>>> >>>> >>>> Modified: >>>> >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java >>>> >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java >>>> >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java >>>> >>>> Modified: >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java >>>> URL: >>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java?rev=1554265&r1=1554264&r2=1554265&view=diff >>>> ============================================================================== >>>> --- >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java >>>> (original) >>>> +++ >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCart.java Mon Dec 30 15:53:07 >>>> 2013 >>>> @@ -2714,7 +2714,7 @@ public class ShoppingCart implements Ite >>>> } >>>> itemsTotal = itemsTotal.add(cartItem.getItemSubTotal()); >>>> } >>>> - return itemsTotal; >>>> + return itemsTotal.add(this.getOrderOtherAdjustmentTotal()); >>>> } >>>> >>>> /** >>>> @@ -3142,12 +3142,12 @@ public class ShoppingCart implements Ite >>>> return new HashMap<GenericPK, >>>> String>(this.desiredAlternateGiftByAction); >>>> } >>>> >>>> - public void addProductPromoUse(String productPromoId, String >>>> productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal >>>> quantityLeftInActions) { >>>> + public void addProductPromoUse(String productPromoId, String >>>> productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal >>>> quantityLeftInActions, Map<ShoppingCartItem,BigDecimal> >>>> usageInfoMap) { >>>> if (UtilValidate.isNotEmpty(productPromoCodeId) && >>>> !this.productPromoCodes.contains(productPromoCodeId)) { >>>> throw new IllegalStateException("Cannot add a use to a >>>> promo code use for a code that has not been entered."); >>>> } >>>> if (Debug.verboseOn()) Debug.logVerbose("Used promotion [" >>>> + productPromoId + "] with code [" + productPromoCodeId + "] for >>>> total discount [" + totalDiscountAmount + "] and quantity left in >>>> actions [" + quantityLeftInActions + "]", module); >>>> - this.productPromoUseInfoList.add(new >>>> ProductPromoUseInfo(productPromoId, productPromoCodeId, >>>> totalDiscountAmount, quantityLeftInActions)); >>>> + this.productPromoUseInfoList.add(new >>>> ProductPromoUseInfo(productPromoId, productPromoCodeId, >>>> totalDiscountAmount, quantityLeftInActions, usageInfoMap)); >>>> } >>>> >>>> public void removeProductPromoUse(String productPromoId) { >>>> @@ -4385,23 +4385,43 @@ public class ShoppingCart implements Ite >>>> } >>>> } >>>> >>>> - public static class ProductPromoUseInfo implements Serializable { >>>> + public static class ProductPromoUseInfo implements >>>> Serializable, Comparable<ProductPromoUseInfo> { >>>> public String productPromoId = null; >>>> public String productPromoCodeId = null; >>>> public BigDecimal totalDiscountAmount = BigDecimal.ZERO; >>>> public BigDecimal quantityLeftInActions = BigDecimal.ZERO; >>>> + private Map<ShoppingCartItem,BigDecimal> usageInfoMap = null; >>>> >>>> - public ProductPromoUseInfo(String productPromoId, String >>>> productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal >>>> quantityLeftInActions) { >>>> + public ProductPromoUseInfo(String productPromoId, String >>>> productPromoCodeId, BigDecimal totalDiscountAmount, BigDecimal >>>> quantityLeftInActions, Map<ShoppingCartItem,BigDecimal> >>>> usageInfoMap) { >>>> this.productPromoId = productPromoId; >>>> this.productPromoCodeId = productPromoCodeId; >>>> this.totalDiscountAmount = totalDiscountAmount; >>>> this.quantityLeftInActions = quantityLeftInActions; >>>> + this.usageInfoMap = usageInfoMap; >>>> } >>>> >>>> public String getProductPromoId() { return this.productPromoId; } >>>> public String getProductPromoCodeId() { return >>>> this.productPromoCodeId; } >>>> public BigDecimal getTotalDiscountAmount() { return >>>> this.totalDiscountAmount; } >>>> public BigDecimal getQuantityLeftInActions() { return >>>> this.quantityLeftInActions; } >>>> + public Map<ShoppingCartItem,BigDecimal> >>>> getUsageInfoMap() { return this.usageInfoMap; } >>>> + public BigDecimal getUsageWeight() { >>>> + Iterator<ShoppingCartItem> lineItems = >>>> this.usageInfoMap.keySet().iterator(); >>>> + BigDecimal totalAmount = BigDecimal.ZERO; >>>> + while (lineItems.hasNext()) { >>>> + ShoppingCartItem lineItem = lineItems.next(); >>>> + totalAmount = >>>> totalAmount.add(lineItem.getBasePrice().multiply(usageInfoMap.get(lineItem))); >>>> + } >>>> + if (totalAmount.compareTo(BigDecimal.ZERO) == 0) { >>>> + return BigDecimal.ZERO; >>>> + } else { >>>> + return >>>> getTotalDiscountAmount().negate().divide(totalAmount); >>>> + } >>>> + } >>>> + >>>> + public int compareTo(ProductPromoUseInfo other) { >>>> + return other.getUsageWeight().compareTo(getUsageWeight()); >>>> + } >>>> } >>>> >>>> public static class CartShipInfo implements Serializable { >>>> >>>> Modified: >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java >>>> URL: >>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java?rev=1554265&r1=1554264&r2=1554265&view=diff >>>> ============================================================================== >>>> --- >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java >>>> (original) >>>> +++ >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/ShoppingCartServices.java Mon Dec 30 15:53:07 >>>> 2013 >>>> @@ -21,6 +21,7 @@ package org.ofbiz.order.shoppingcart; >>>> import java.math.BigDecimal; >>>> import java.math.MathContext; >>>> import java.sql.Timestamp; >>>> +import java.util.HashMap; >>>> import java.util.Iterator; >>>> import java.util.List; >>>> import java.util.Locale; >>>> @@ -629,7 +630,7 @@ public class ShoppingCartServices { >>>> cart.addProductPromoCode(productPromoCode, dispatcher); >>>> } >>>> for (GenericValue productPromoUse: orh.getProductPromoUse()) { >>>> - >>>> cart.addProductPromoUse(productPromoUse.getString("productPromoId"), >>>> productPromoUse.getString("productPromoCodeId"), >>>> productPromoUse.getBigDecimal("totalDiscountAmount"), >>>> productPromoUse.getBigDecimal("quantityLeftInActions")); >>>> + >>>> cart.addProductPromoUse(productPromoUse.getString("productPromoId"), >>>> productPromoUse.getString("productPromoCodeId"), >>>> productPromoUse.getBigDecimal("totalDiscountAmount"), >>>> productPromoUse.getBigDecimal("quantityLeftInActions"), new >>>> HashMap<ShoppingCartItem, BigDecimal>()); >>>> } >>>> } >>>> >>>> >>>> Modified: >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java >>>> URL: >>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java?rev=1554265&r1=1554264&r2=1554265&view=diff >>>> ============================================================================== >>>> --- >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java >>>> (original) >>>> +++ >>>> ofbiz/trunk/applications/order/src/org/ofbiz/order/shoppingcart/product/ProductPromoWorker.java Mon Dec 30 15:53:07 >>>> 2013 >>>> @@ -22,6 +22,8 @@ import java.math.BigDecimal; >>>> import java.math.MathContext; >>>> import java.sql.Timestamp; >>>> import java.util.ArrayList; >>>> +import java.util.Collections; >>>> +import java.util.HashMap; >>>> import java.util.Iterator; >>>> import java.util.List; >>>> import java.util.Locale; >>>> @@ -51,6 +53,7 @@ import org.ofbiz.entity.condition.Entity >>>> import org.ofbiz.entity.util.EntityUtil; >>>> import org.ofbiz.order.shoppingcart.CartItemModifyException; >>>> import org.ofbiz.order.shoppingcart.ShoppingCart; >>>> +import org.ofbiz.order.shoppingcart.ShoppingCart.ProductPromoUseInfo; >>>> import org.ofbiz.order.shoppingcart.ShoppingCartEvents; >>>> import org.ofbiz.order.shoppingcart.ShoppingCartItem; >>>> import org.ofbiz.product.product.ProductContentWrapper; >>>> @@ -318,44 +321,62 @@ public class ProductPromoWorker { >>>> // do a calculate only run through the promotions, >>>> then order by descending totalDiscountAmount for each promotion >>>> // NOTE: on this run, with isolatedTestRun passed as >>>> false it should not apply any adjustments >>>> // or track which cart items are used for which >>>> promotions, but it will track ProductPromoUseInfo and >>>> - // useLimits; we are basicly just trying to run >>>> each promo "independently" to see how much each is worth >>>> + // useLimits; we are basically just trying to run >>>> each promo "independently" to see how much each is worth >>>> runProductPromos(productPromoList, cart, delegator, >>>> dispatcher, nowTimestamp, true); >>>> >>>> - // NOTE: after that first pass we could remove any >>>> that have a 0 totalDiscountAmount from the run list, but we won't >>>> because by the time they are run the cart may have changed enough >>>> to get them to go; also, certain actions like free shipping >>>> should always be run even though we won't know what the >>>> totalDiscountAmount is at the time the promotion is run >>>> - // each ProductPromoUseInfo on the shopping cart >>>> will contain it's total value, so add up all totals for each >>>> promoId and put them in a List of Maps >>>> - // create a List of Maps with productPromo and >>>> totalDiscountAmount, use the Map sorter to sort them descending >>>> by totalDiscountAmount >>>> - >>>> - // before sorting split into two lists and sort each >>>> list; one list for promos that have a order total condition, and >>>> the other list for all promos that don't; then we'll always run >>>> the ones that have no condition on the order total first >>>> - List<Map<Object, Object>> >>>> productPromoDiscountMapList = FastList.newInstance(); >>>> - List<Map<Object, Object>> >>>> productPromoDiscountMapListOrderTotal = FastList.newInstance(); >>>> + // NOTE: we can easily recognize the promos for the >>>> order total: they are the ones with usage set to 0 >>>> + Iterator<ProductPromoUseInfo> promoUses = >>>> cart.getProductPromoUseInfoIter(); >>>> + List<ProductPromoUseInfo> sortedPromoUses = new >>>> ArrayList<ProductPromoUseInfo>(); >>>> + while (promoUses.hasNext()) { >>>> + ProductPromoUseInfo promoUse = promoUses.next(); >>>> + sortedPromoUses.add(promoUse); >>>> + } >>>> + Collections.sort(sortedPromoUses); >>>> + List<GenericValue> sortedExplodedProductPromoList = >>>> new ArrayList<GenericValue>(sortedPromoUses.size()); >>>> + Map<String, Long> usesPerPromo = new HashMap<String, Long>(); >>>> + int indexOfFirstOrderTotalPromo = -1; >>>> + for (ProductPromoUseInfo promoUse: sortedPromoUses) { >>>> + GenericValue productPromo = >>>> delegator.findOne("ProductPromo", >>>> UtilMisc.toMap("productPromoId", promoUse.getProductPromoId()), >>>> true); >>>> + GenericValue newProductPromo = >>>> (GenericValue)productPromo.clone(); >>>> + if >>>> (!usesPerPromo.containsKey(promoUse.getProductPromoId())) { >>>> + usesPerPromo.put(promoUse.getProductPromoId(), 0l); >>>> + } >>>> + long uses = >>>> usesPerPromo.get(promoUse.getProductPromoId()); >>>> + uses = uses + 1; >>>> + long useLimitPerOrder = >>>> (newProductPromo.get("useLimitPerOrder") != null? >>>> newProductPromo.getLong("useLimitPerOrder"): -1); >>>> + if (useLimitPerOrder == -1 || uses < useLimitPerOrder) { >>>> + newProductPromo.set("useLimitPerOrder", uses); >>>> + } >>>> + usesPerPromo.put(promoUse.getProductPromoId(), uses); >>>> + sortedExplodedProductPromoList.add(newProductPromo); >>>> + if (indexOfFirstOrderTotalPromo == -1 && >>>> BigDecimal.ZERO.equals(promoUse.getUsageWeight())) { >>>> + indexOfFirstOrderTotalPromo = >>>> sortedExplodedProductPromoList.size() - 1; >>>> + } >>>> + } >>>> + if (indexOfFirstOrderTotalPromo == -1) { >>>> + indexOfFirstOrderTotalPromo = >>>> sortedExplodedProductPromoList.size() - 1; >>>> + } >>>> + >>>> for (GenericValue productPromo : productPromoList) { >>>> Map<Object, Object> productPromoDiscountMap = >>>> UtilGenerics.checkMap(UtilMisc.toMap("productPromo", >>>> productPromo, "totalDiscountAmount", >>>> cart.getProductPromoUseTotalDiscount(productPromo.getString("productPromoId")))); >>>> if (hasOrderTotalCondition(productPromo, delegator)) { >>>> - >>>> productPromoDiscountMapListOrderTotal.add(productPromoDiscountMap); >>>> + if >>>> (!usesPerPromo.containsKey(productPromo.getString("productPromoId"))) >>>> { >>>> + sortedExplodedProductPromoList.add(productPromo); >>>> + } >>>> } else { >>>> - >>>> productPromoDiscountMapList.add(productPromoDiscountMap); >>>> + if >>>> (!usesPerPromo.containsKey(productPromo.getString("productPromoId"))) >>>> { >>>> + if (indexOfFirstOrderTotalPromo != -1) { >>>> + >>>> sortedExplodedProductPromoList.add(indexOfFirstOrderTotalPromo, >>>> productPromo); >>>> + } else { >>>> + >>>> sortedExplodedProductPromoList.add(0, productPromo); >>>> + } >>>> + } >>>> } >>>> } >>>> >>>> - >>>> - // sort the Map List, do it ascending because the >>>> discount amounts will be negative, so the lowest number is really >>>> the highest discount >>>> - productPromoDiscountMapList = >>>> UtilMisc.sortMaps(productPromoDiscountMapList, >>>> UtilMisc.toList("+totalDiscountAmount")); >>>> - productPromoDiscountMapListOrderTotal = >>>> UtilMisc.sortMaps(productPromoDiscountMapListOrderTotal, >>>> UtilMisc.toList("+totalDiscountAmount")); >>>> - >>>> - >>>> productPromoDiscountMapList.addAll(productPromoDiscountMapListOrderTotal); >>>> - >>>> - List<GenericValue> sortedProductPromoList = new >>>> ArrayList<GenericValue>(productPromoDiscountMapList.size()); >>>> - Iterator<Map<Object, Object>> >>>> productPromoDiscountMapIter = >>>> productPromoDiscountMapList.iterator(); >>>> - while (productPromoDiscountMapIter.hasNext()) { >>>> - Map<Object, Object> productPromoDiscountMap = >>>> UtilGenerics.checkMap(productPromoDiscountMapIter.next()); >>>> - GenericValue productPromo = (GenericValue) >>>> productPromoDiscountMap.get("productPromo"); >>>> - sortedProductPromoList.add(productPromo); >>>> - if (Debug.verboseOn()) Debug.logVerbose("Sorted >>>> Promo [" + productPromo.getString("productPromoId") + "] with >>>> total discount: " + >>>> productPromoDiscountMap.get("totalDiscountAmount"), module); >>>> - } >>>> - >>>> // okay, all ready, do the real run, clearing the >>>> temporary result first... >>>> cart.clearAllPromotionInformation(); >>>> - runProductPromos(sortedProductPromoList, cart, >>>> delegator, dispatcher, nowTimestamp, false); >>>> + runProductPromos(sortedExplodedProductPromoList, >>>> cart, delegator, dispatcher, nowTimestamp, false); >>>> } catch (NumberFormatException e) { >>>> Debug.logError(e, "Number not formatted correctly in >>>> promotion rules, not completed...", module); >>>> } catch (GenericEntityException e) { >>>> @@ -436,7 +457,7 @@ public class ProductPromoWorker { >>>> GenericValue productPromoCode >>>> = productPromoCodeIter.next(); >>>> String productPromoCodeId = >>>> productPromoCode.getString("productPromoCodeId"); >>>> Long codeUseLimit = >>>> getProductPromoCodeUseLimit(productPromoCode, partyId, delegator); >>>> - if >>>> (runProductPromoRules(cart, cartChanged, useLimit, true, >>>> productPromoCodeId, codeUseLimit, maxUseLimit, productPromo, >>>> productPromoRules, dispatcher, delegator, nowTimestamp)) { >>>> + if >>>> (runProductPromoRules(cart, useLimit, true, productPromoCodeId, >>>> codeUseLimit, maxUseLimit, productPromo, productPromoRules, >>>> dispatcher, delegator, nowTimestamp)) { >>>> cartChanged = true; >>>> } >>>> >>>> @@ -448,7 +469,7 @@ public class ProductPromoWorker { >>>> } >>>> } else { >>>> try { >>>> - if (runProductPromoRules(cart, >>>> cartChanged, useLimit, false, null, null, maxUseLimit, >>>> productPromo, productPromoRules, dispatcher, delegator, >>>> nowTimestamp)) { >>>> + if (runProductPromoRules(cart, >>>> useLimit, false, null, null, maxUseLimit, productPromo, >>>> productPromoRules, dispatcher, delegator, nowTimestamp)) { >>>> cartChanged = true; >>>> } >>>> } catch (RuntimeException e) { >>>> @@ -735,8 +756,10 @@ public class ProductPromoWorker { >>>> return promoDescBuf.toString(); >>>> } >>>> >>>> - protected static boolean runProductPromoRules(ShoppingCart >>>> cart, boolean cartChanged, Long useLimit, boolean requireCode, >>>> String productPromoCodeId, Long codeUseLimit, long maxUseLimit, >>>> + protected static boolean runProductPromoRules(ShoppingCart >>>> cart, Long useLimit, boolean requireCode, String >>>> productPromoCodeId, Long codeUseLimit, long maxUseLimit, >>>> GenericValue productPromo, List<GenericValue> >>>> productPromoRules, LocalDispatcher dispatcher, Delegator >>>> delegator, Timestamp nowTimestamp) throws GenericEntityException, >>>> UseLimitException { >>>> + boolean cartChanged = false; >>>> + Map<ShoppingCartItem,BigDecimal> usageInfoMap = >>>> prepareProductUsageInfoMap(cart); >>>> String productPromoId = productPromo.getString("productPromoId"); >>>> while ((useLimit == null || useLimit.longValue() > >>>> cart.getProductPromoUseCount(productPromoId)) && >>>> (!requireCode || >>>> UtilValidate.isNotEmpty(productPromoCodeId)) && >>>> @@ -755,17 +778,17 @@ public class ProductPromoWorker { >>>> // loop through conditions for rule, if any false, >>>> set allConditionsTrue to false >>>> List<GenericValue> productPromoConds = >>>> delegator.findByAnd("ProductPromoCond", >>>> UtilMisc.toMap("productPromoId", >>>> productPromo.get("productPromoId")), >>>> UtilMisc.toList("productPromoCondSeqId"), true); >>>> productPromoConds = >>>> EntityUtil.filterByAnd(productPromoConds, >>>> UtilMisc.toMap("productPromoRuleId", >>>> productPromoRule.get("productPromoRuleId"))); >>>> - // using the other method to consolodate cache >>>> entries because the same cache is used elsewhere: List >>>> productPromoConds = >>>> productPromoRule.getRelated("ProductPromoCond", null, >>>> UtilMisc.toList("productPromoCondSeqId"), true); >>>> + // using the other method to consolidate cache >>>> entries because the same cache is used elsewhere: List >>>> productPromoConds = >>>> productPromoRule.getRelated("ProductPromoCond", null, >>>> UtilMisc.toList("productPromoCondSeqId"), true); >>>> if (Debug.verboseOn()) Debug.logVerbose("Checking >>>> " + productPromoConds.size() + " conditions for rule " + >>>> productPromoRule, module); >>>> >>>> Iterator<GenericValue> productPromoCondIter = >>>> UtilMisc.toIterator(productPromoConds); >>>> while (productPromoCondIter != null && >>>> productPromoCondIter.hasNext()) { >>>> GenericValue productPromoCond = >>>> productPromoCondIter.next(); >>>> >>>> - boolean condResult = >>>> checkCondition(productPromoCond, cart, delegator, dispatcher, >>>> nowTimestamp); >>>> + boolean conditionSatisfied = >>>> checkCondition(productPromoCond, cart, delegator, dispatcher, >>>> nowTimestamp); >>>> >>>> // any false condition will cause it to NOT >>>> perform the action >>>> - if (condResult == false) { >>>> + if (!conditionSatisfied) { >>>> performActions = false; >>>> break; >>>> } >>>> @@ -797,13 +820,16 @@ public class ProductPromoWorker { >>>> } >>>> >>>> if (promoUsed) { >>>> - >>>> cart.addProductPromoUse(productPromo.getString("productPromoId"), >>>> productPromoCodeId, totalDiscountAmount, quantityLeftInActions); >>>> + // Get product use information from the cart >>>> + Map<ShoppingCartItem,BigDecimal> newUsageInfoMap >>>> = prepareProductUsageInfoMap(cart); >>>> + Map<ShoppingCartItem,BigDecimal> >>>> deltaUsageInfoMap = prepareDeltaProductUsageInfoMap(usageInfoMap, >>>> newUsageInfoMap); >>>> + usageInfoMap = newUsageInfoMap; >>>> + >>>> cart.addProductPromoUse(productPromo.getString("productPromoId"), >>>> productPromoCodeId, totalDiscountAmount, quantityLeftInActions, >>>> deltaUsageInfoMap); >>>> } else { >>>> // the promotion was not used, don't try again >>>> until we finish a full pass and come back to see the promo >>>> conditions are now satisfied based on changes to the cart >>>> break; >>>> } >>>> >>>> - >>>> if (cart.getProductPromoUseCount(productPromoId) > >>>> maxUseLimit) { >>>> throw new UseLimitException("ERROR: While >>>> calculating promotions the promotion [" + productPromoId + "] >>>> action was applied more than " + maxUseLimit + " times, so the >>>> calculation has been ended. This should generally never happen >>>> unless you have bad rule definitions."); >>>> } >>>> @@ -812,6 +838,34 @@ public class ProductPromoWorker { >>>> return cartChanged; >>>> } >>>> >>>> + private static Map<ShoppingCartItem,BigDecimal> >>>> prepareProductUsageInfoMap(ShoppingCart cart) { >>>> + Map<ShoppingCartItem,BigDecimal> usageInfoMap = new >>>> HashMap<ShoppingCartItem, BigDecimal>(); >>>> + List<ShoppingCartItem> lineOrderedByBasePriceList = >>>> cart.getLineListOrderedByBasePrice(false); >>>> + for (ShoppingCartItem cartItem : lineOrderedByBasePriceList) { >>>> + BigDecimal used = cartItem.getPromoQuantityUsed(); >>>> + if (used.compareTo(BigDecimal.ZERO) != 0) { >>>> + usageInfoMap.put(cartItem, used); >>>> + } >>>> + } >>>> + return usageInfoMap; >>>> + } >>>> + >>>> + 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); >>>> + if (newUsed.compareTo(oldUsed) > 0) { >>>> + deltaUsageInfoMap.put(cartLine, >>>> newUsed.add(oldUsed.negate())); >>>> + } else { >>>> + deltaUsageInfoMap.remove(cartLine); >>>> + } >>>> + } >>>> + return deltaUsageInfoMap; >>>> + } >>>> + >>>> protected static boolean checkCondition(GenericValue >>>> productPromoCond, ShoppingCart cart, Delegator delegator, >>>> LocalDispatcher dispatcher, Timestamp nowTimestamp) throws >>>> GenericEntityException { >>>> String condValue = productPromoCond.getString("condValue"); >>>> String otherValue = productPromoCond.getString("otherValue"); >>>> @@ -1772,8 +1826,8 @@ public class ProductPromoWorker { >>>> actionResultInfo.ranAction = false; >>>> } >>>> >>>> + // in action, if doesn't have enough quantity to use the >>>> promo at all, remove candidate promo uses and increment >>>> promoQuantityUsed; this should go for all actions, if any action >>>> runs we confirm >>>> if (actionResultInfo.ranAction) { >>>> - // in action, if doesn't have enough quantity to use >>>> the promo at all, remove candidate promo uses and increment >>>> promoQuantityUsed; this should go for all actions, if any action >>>> runs we confirm >>>> >>>> cart.confirmPromoRuleUse(productPromoAction.getString("productPromoId"), >>>> productPromoAction.getString("productPromoRuleId")); >>>> } else { >>>> >>>> cart.resetPromoRuleUse(productPromoAction.getString("productPromoId"), >>>> productPromoAction.getString("productPromoRuleId")); >>>> >>>> >>>> >>> >>> >>> >> > > |
Free forum by Nabble | Edit this page |