You mean by using DispatchContext? What does it add in this context?

Jacques

Le 29/10/2014 20:44, Adrian Crum a écrit :
This commit is an ugly hack. Why are you defending it? Just fix it.

Adrian Crum
Sandglass Software
www.sandglass-software.com

On 10/29/2014 7:37 PM, Jacques Le Roux wrote:

Le 29/10/2014 19:22, Adrian Crum a écrit :
I'm not Jacques, but I'm pretty sure there is very little difference.
The disadvantage to hacks like this: Bypassing the service engine like
this will prevent any SECAs attached to the called service from being
invoked.

OOTB there are any SECAs attached to the countProductQuantityOrdered.
And I doubt there are in custom projects, they would be quite
convoluted. ProductCalculatedInfo entity is destined for stats and
reports. I don't see  it used in business workflows: after my change
this service is not used OOTB. I'm not even sure we need this service!

Jacques


Adrian Crum
Sandglass Software
www.sandglass-software.com

On 10/29/2014 5:50 PM, Jacopo Cappellato wrote:
Jacques,

what was the measured difference between the method call to the Java
method and the service call to the Java implementation?

Jacopo

On Oct 29, 2014, at 5:37 PM, [email protected] wrote:

Author: jleroux
Date: Wed Oct 29 16:37:40 2014
New Revision: 1635192

URL: http://svn.apache.org/r1635192
Log:
The storeOrder service, implemented by the
OrderService.createOrder() method, synchronously calls the
countProductQuantityOrdered service implemented in Minilang. OOTB,
the countProductQuantityOrdered service is only called by the
storeOrder service implementation. It's called inside a loop on
orderItems.

While intentionally load testing a custom project with JMeter on a
weak m1.small AWS machine, I noticed the overhead of the service
call for each order item iteration was significant on this slow
machine. With only 30 concurrent users the process was blocked by  a
timeout on the countProductQuantityOrdered service.

I then decided to transform the minilang code into an
OrderService.countProductQuantityOrdered() method that can be
directly called inside OrderService.createOrder() and also
implements the countProductQuantityOrdered service. Hence it avoids
the overhead of the  minilang service calls in loop while still
providing a countProductQuantityOrdered service for possible
external uses. For that I moved the definition from Product
component to Order component to avoid the hard coded dependency of
Order to Product. I then did not cross issues with the same load test.

I also added a test for the countProductQuantityOrdered service by
adding an order item of a virtual product (GZ-1006-1) in
SalesOrderTest.

Modified:
    ofbiz/trunk/applications/order/servicedef/services.xml
ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java

ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java

ofbiz/trunk/applications/product/servicedef/services.xml

Modified: ofbiz/trunk/applications/order/servicedef/services.xml
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff

==============================================================================

--- ofbiz/trunk/applications/order/servicedef/services.xml (original)
+++ ofbiz/trunk/applications/order/servicedef/services.xml Wed Oct
29 16:37:40 2014
@@ -1147,4 +1147,11 @@ under the License.
         <auto-attributes mode="IN"
entity-name="OrderItemGroupOrder" include="pk" optional="false"/>
     </service>

+    <service name="countProductQuantityOrdered" engine="java"
+        location="org.ofbiz.order.order.OrderServices"
invoke="countProductQuantityOrdered" auth="true">
+        <description>count Product Quantity Ordered</description>
+        <attribute name="productId" type="String" mode="IN"
optional="false"/>
+        <attribute name="quantity" type="BigDecimal" mode="IN"
optional="false"/>
+    </service>
+
</services>

Modified:
ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java

URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java?rev=1635192&r1=1635191&r2=1635192&view=diff

==============================================================================

---
ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java
(original)
+++
ofbiz/trunk/applications/order/src/org/ofbiz/order/order/OrderServices.java
Wed Oct 29 16:37:40 2014
@@ -280,15 +280,10 @@ public class OrderServices {
normalizedItemQuantities.put(currentProductId,
currentQuantity.add(orderItem.getBigDecimal("quantity")));
                 }

-                try {
-                    // count product ordered quantities
-                    // run this synchronously so it will run in the
same transaction
- dispatcher.runSync("countProductQuantityOrdered",
UtilMisc.<String, Object>toMap("productId", currentProductId,
"quantity", orderItem.getBigDecimal("quantity"), "userLogin",
userLogin));
-                } catch (GenericServiceException e1) {
-                    Debug.logError(e1, "Error calling
countProductQuantityOrdered service", module);
-                    return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,
- "OrderErrorCallingCountProductQuantityOrderedService",locale) +
e1.toString());
-                }
+                Map<String, Object> countContext = new
HashMap<String, Object>();
+                countContext.put("productId", currentProductId);
+                countContext.put("quantity",
orderItem.getBigDecimal("quantity"));
+                countProductQuantityOrdered(ctx, countContext);
             }
         }

@@ -1152,6 +1147,54 @@ public class OrderServices {

         return successResult;
     }
+
+    public static Map<String, Object>
countProductQuantityOrdered(DispatchContext ctx, Map<String, Object>
context) {
+        Delegator delegator = ctx.getDelegator();
+        Locale locale = (Locale) context.get("locale");
+        List<GenericValue> productCalculatedInfoList = null;
+        GenericValue productCalculatedInfo = null;
+        String productId = (String) context.get("productId");
+        BigDecimal quantity = (BigDecimal) context.get("quantity");
+        try {
+            productCalculatedInfoList =
delegator.findByAnd("ProductCalculatedInfo",
UtilMisc.toMap("productId", productId), null, false);
+            if (UtilValidate.isEmpty(productCalculatedInfoList)) {
+                productCalculatedInfo =
delegator.makeValue("ProductCalculatedInfo");
+                productCalculatedInfo.set("productId", productId);
+ productCalculatedInfo.set("totalQuantityOrdered", quantity);
+                productCalculatedInfo.create();
+            } else {
+                productCalculatedInfo =
productCalculatedInfoList.get(0);
+                BigDecimal totalQuantityOrdered =
productCalculatedInfo.getBigDecimal("totalQuantityOrdered");
+                if (totalQuantityOrdered == null) {
+ productCalculatedInfo.set("totalQuantityOrdered", quantity);
+                } else {
+ productCalculatedInfo.set("totalQuantityOrdered",
totalQuantityOrdered.add(quantity));
+                }
+            }
+            productCalculatedInfo.store();
+        } catch (GenericEntityException e) {
+            Debug.logError(e, "Error calling
countProductQuantityOrdered service", module);
+            return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,
+ "OrderErrorCallingCountProductQuantityOrderedService",locale) +
e.toString());
+
+        }
+
+        String virtualProductId = null;
+        try {
+            GenericValue product = delegator.findOne("Product",
UtilMisc.toMap("productId", productId), true);
+            virtualProductId =
ProductWorker.getVariantVirtualId(product);
+        } catch (GenericEntityException e) {
+            Debug.logError(e, "Error calling
countProductQuantityOrdered service", module);
+            return
ServiceUtil.returnError(UtilProperties.getMessage(resource_error,
+ "OrderErrorCallingCountProductQuantityOrderedService",locale) +
e.toString());
+        }
+
+        if (UtilValidate.isNotEmpty(virtualProductId)) {
+            context.put("productId", virtualProductId);
+            countProductQuantityOrdered(ctx, context);
+        }
+        return ServiceUtil.returnSuccess();
+    }

     public static void reserveInventory(Delegator delegator,
LocalDispatcher dispatcher, GenericValue userLogin, Locale locale,
List<GenericValue> orderItemShipGroupInfo, List<String>
dropShipGroupIds, Map<String, GenericValue> itemValuesBySeqId,
String orderTypeId, String productStoreId, List<String>
resErrorMessages) throws GeneralException {
         boolean isImmediatelyFulfilled = false;
@@ -1729,7 +1772,7 @@ public class OrderServices {
                 if (UtilValidate.isNotEmpty(orderItemSeqId)) {
createOrderAdjContext.put("orderItemSeqId", orderItemSeqId);
                 } else {
- createOrderAdjContext.put("orderItemSeqId", "_NA_");
+ createOrderAdjContext.put("orderItemSeqId", "_NA_");
                 }
createOrderAdjContext.put("shipGroupSeqId", "_NA_");
                 createOrderAdjContext.put("description", "Tax
adjustment due to order change");
@@ -5787,4 +5830,4 @@ public class OrderServices {

         return ServiceUtil.returnSuccess();
     }
-}
\ No newline at end of file
+}

Modified:
ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java

URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java?rev=1635192&r1=1635191&r2=1635192&view=diff

==============================================================================

---
ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java
(original)
+++
ofbiz/trunk/applications/order/src/org/ofbiz/order/test/SalesOrderTest.java
Wed Oct 29 16:37:40 2014
@@ -114,8 +114,16 @@ public class SalesOrderTest extends OFBi
         orderItem.set("unitPrice", new BigDecimal("38.4"));
         orderItem.set("unitListPrice", new BigDecimal("48.0"));
         orderItem.set("statusId", "ITEM_CREATED");
-
         orderItems.add(orderItem);
+
+        orderItem = delegator.makeValue("OrderItem",
UtilMisc.toMap("orderItemSeqId", "00002", "orderItemTypeId",
"PRODUCT_ORDER_ITEM", "prodCatalogId", "DemoCatalog", "productId",
"GZ-1006-1", "quantity", BigDecimal.ONE, "selectedAmount",
BigDecimal.ZERO));
+        orderItem.set("isPromo", "N");
+        orderItem.set("isModifiedPrice", "N");
+        orderItem.set("unitPrice", new BigDecimal("1.99"));
+        orderItem.set("unitListPrice", new BigDecimal("5.99"));
+        orderItem.set("statusId", "ITEM_CREATED");
+        orderItems.add(orderItem);
+
         ctx.put("orderItems", orderItems);

         List<GenericValue> orderTerms = FastList.newInstance();

Modified: ofbiz/trunk/applications/product/servicedef/services.xml
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/servicedef/services.xml?rev=1635192&r1=1635191&r2=1635192&view=diff

==============================================================================

--- ofbiz/trunk/applications/product/servicedef/services.xml (original)
+++ ofbiz/trunk/applications/product/servicedef/services.xml Wed Oct
29 16:37:40 2014
@@ -175,13 +175,7 @@ under the License.
         <attribute name="productId" type="String" mode="IN"
optional="false"/>
         <attribute name="weight" type="Long" mode="IN"
optional="true"/>
     </service>
-    <service name="countProductQuantityOrdered" engine="simple"
-
location="component://product/script/org/ofbiz/product/product/ProductServices.xml"
invoke="countProductQuantityOrdered" auth="true">
-        <description>count Product Quantity Ordered</description>
-        <attribute name="productId" type="String" mode="IN"
optional="false"/>
-        <attribute name="quantity" type="BigDecimal" mode="IN"
optional="false"/>
-    </service>
-
+
     <service name="createProductReview" engine="simple"
location="component://product/script/org/ofbiz/product/product/ProductServices.xml"
invoke="createProductReview" auth="true">
         <description>Create a product review entity</description>





Reply via email to