Hi Pierre, cmssite, ecommerce and webpos are specialpurpose components that can rely on the artifacts (services, entities, labels, screens and *classes*) defined in the application component (including the "marketing" component to which TrackingCodeEvents belongs).
With that said, there is a chance that you are misunderstanding the purpose of Events vs services. Events should be used for web applications (i.e. they are http aware) while services should be used to implement re-usable business logic that can be used in a wider scope (i.e. not limited to web applications, like batch jobs, integrations with external systems etc...). Untangling the components' dependencies (or merging mutual dependent parts into one component, if not possible otherwise) is an important task but it requires proper (and not so obvious) design and tools; the recent move to Gradle is a small step in the right direction, in my opinion, but a lot more design work is required to clear the dependencies in an organized and consistent way. And at this stage I don't think that taking shortcuts, like introducing new settings to disable the calls to the service of another component, as was proposed in your and Jacques' contribution, is the right way to go because it will make the system more difficult to configure and manage to our users. And in general, by converting an event to a service, we indeed remove a compile time dependency but the runtime dependency is still there: and this is risky to adopters that may think that there is no dependency and may disable a component that is instead required by the system. Kind regards, Jacopo On Mon, Aug 22, 2016 at 9:16 PM, Pierre Smits <pierre.sm...@gmail.com> wrote: > Hi Jacopo, > > I see that most functions in TrackingCodeEvents.java are used in cmssite, > ecommerce and webpos. Should we not convert all to services? > > Best regards, > > Pierre Smits > > ORRTIZ.COM <http://www.orrtiz.com> > OFBiz based solutions & services > > OFBiz Extensions Marketplace > http://oem.ofbizci.net/oci-2/ > > On Mon, Aug 22, 2016 at 4:00 PM, Jacopo Cappellato <jacopo.cappellato@ > hotwaxsystems.com> wrote: > > > Hi Pierre, > > > > since Jacques responded to my mail, acknowledging that there are issues > and > > that he is going to fix them, I will let him complete the work before > > digging into the code details. > > Anyway, they are pretty basic issues and I am concerned that you and > > Jacques couldn't identify them with your testing and review process: this > > is why I was asking about them. > > > > Kind regards, > > > > Jacopo > > > > > > On Mon, Aug 22, 2016 at 3:21 PM, Pierre Smits <pierre.sm...@gmail.com> > > wrote: > > > > > Hi Jacopo, > > > > > > Can you explain why it seems 'completely' wrong to you? > > > > > > Best regards, > > > > > > > > > Pierre Smits > > > > > > ORRTIZ.COM <http://www.orrtiz.com> > > > OFBiz based solutions & services > > > > > > OFBiz Extensions Marketplace > > > http://oem.ofbizci.net/oci-2/ > > > > > > On Mon, Aug 22, 2016 at 12:43 PM, Jacopo Cappellato < > > > jacopo.cappell...@hotwaxsystems.com> wrote: > > > > > > > This contribution seems completely wrong to me. Pierre and Jacques, > > have > > > > you performed proper tests and reviews before committing it? > > > > > > > > Jacopo > > > > > > > > > > > > On Mon, Aug 22, 2016 at 11:58 AM, <jler...@apache.org> wrote: > > > > > > > > > Author: jleroux > > > > > Date: Mon Aug 22 09:58:35 2016 > > > > > New Revision: 1757130 > > > > > > > > > > URL: http://svn.apache.org/viewvc?rev=1757130&view=rev > > > > > Log: > > > > > A modified patch from Pierre Smits for "remove build dependency of > > > Order > > > > > on Marketing" https://issues.apache.org/jira/browse/OFBIZ-7966 > > > > > > > > > > Currently there is a build dependency from order - > > CheckOutEvents.java > > > on > > > > > marketing - TrackingCodeEvents.java > > > > > The createOrder function (in CheckOutEvents.java) calls the > > > > > makeTrackingCodeOrders function in TrackingCodeEvents.java > > > > > > > > > > jleroux: I merged parts of the 2 patches, the 2nd was good but > > missing > > > > the > > > > > makeTrackingCodeOrders service definition. I also fixed the warning > > > about > > > > > trackingCodeOrdersList creation not being generic > > > > > > > > > > Modified: > > > > > ofbiz/trunk/applications/marketing/servicedef/services.xml > > > > > ofbiz/trunk/applications/marketing/src/main/java/org/ > > > > > apache/ofbiz/marketing/tracking/TrackingCodeEvents.java > > > > > ofbiz/trunk/applications/order/data/ > OrderSystemPropertyData.xml > > > > > ofbiz/trunk/applications/order/src/main/java/org/ > > > apache/ofbiz/order/ > > > > > shoppingcart/CheckOutEvents.java > > > > > ofbiz/trunk/applications/order/src/main/java/org/ > > > apache/ofbiz/order/ > > > > > shoppingcart/CheckOutHelper.java > > > > > > > > > > Modified: ofbiz/trunk/applications/marketing/servicedef/services. > xml > > > > > URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/ > > > > > marketing/servicedef/services.xml?rev=1757130&r1=1757129&r2= > > > > > 1757130&view=diff > > > > > ============================================================ > > > > > ================== > > > > > --- ofbiz/trunk/applications/marketing/servicedef/services.xml > > > > (original) > > > > > +++ ofbiz/trunk/applications/marketing/servicedef/services.xml Mon > > Aug > > > > 22 > > > > > 09:58:35 2016 > > > > > @@ -419,6 +419,12 @@ under the License. > > > > > <attribute type="String" mode="IN" name="returnId" > > > > > optional="false"/> > > > > > </service> > > > > > > > > > > + <service name="makeTrackingCodeOrders" engine="java" > > > > > location="org.apache.ofbiz.marketing.tracking.TrackingCodeEvents" > > > > invoke="makeTrackingCodeOrders" > > > > > auth="true"> > > > > > + <description>Makes a list of TrackingCodeOrder entities to > > be > > > > > attached to the current order</description> > > > > > + <attribute name="request" mode="IN" > > type="javax.servlet.http. > > > > > HttpServletRequest"/> > > > > > + <attribute name="trackingCodeOrders" type="List" > mode="OUT" > > > > > optional="false"/> > > > > > + </service> > > > > > + > > > > > <!-- marketing permission service --> > > > > > <service name="marketingPermissionService" engine="simple" > > > > > location="component://common/minilang/permission/ > > > > CommonPermissionServices.xml" > > > > > invoke="genericBasePermissionCheck"> > > > > > > > > > > Modified: ofbiz/trunk/applications/marketing/src/main/java/org/ > > > > > apache/ofbiz/marketing/tracking/TrackingCodeEvents.java > > > > > URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/ > > > > > marketing/src/main/java/org/apache/ofbiz/marketing/ > > > > > tracking/TrackingCodeEvents.java?rev=1757130&r1=1757129& > > > > > r2=1757130&view=diff > > > > > ============================================================ > > > > > ================== > > > > > --- ofbiz/trunk/applications/marketing/src/main/java/org/ > > > > > apache/ofbiz/marketing/tracking/TrackingCodeEvents.java (original) > > > > > +++ ofbiz/trunk/applications/marketing/src/main/java/org/ > > > > > apache/ofbiz/marketing/tracking/TrackingCodeEvents.java Mon Aug 22 > > > > > 09:58:35 2016 > > > > > @@ -31,14 +31,14 @@ import org.apache.ofbiz.base.util.Debug; > > > > > import org.apache.ofbiz.base.util.UtilDateTime; > > > > > import org.apache.ofbiz.base.util.UtilMisc; > > > > > import org.apache.ofbiz.base.util.UtilValidate; > > > > > -import org.apache.ofbiz.webapp.stats.VisitHandler; > > > > > -import org.apache.ofbiz.webapp.website.WebSiteWorker; > > > > > import org.apache.ofbiz.entity.Delegator; > > > > > import org.apache.ofbiz.entity.GenericEntityException; > > > > > import org.apache.ofbiz.entity.GenericValue; > > > > > import org.apache.ofbiz.entity.util.EntityQuery; > > > > > import org.apache.ofbiz.entity.util.EntityUtilProperties; > > > > > import org.apache.ofbiz.product.category.CategoryWorker; > > > > > +import org.apache.ofbiz.webapp.stats.VisitHandler; > > > > > +import org.apache.ofbiz.webapp.website.WebSiteWorker; > > > > > > > > > > /** > > > > > * Events used for maintaining TrackingCode related information > > > > > @@ -285,7 +285,7 @@ public class TrackingCodeEvents { > > > > > String prodCatalogId = trackingCode.getString(" > > > prodCatalogId"); > > > > > if (UtilValidate.isNotEmpty(prodCatalogId)) { > > > > > session.setAttribute("CURRENT_CATALOG_ID", > > > prodCatalogId); > > > > > - CategoryWorker.setTrail(request, new LinkedList()); > > > > > + CategoryWorker.setTrail(request, new > > > LinkedList<String>()); > > > > > } > > > > > > > > > > // if forward/redirect is needed, do a > response.sendRedirect > > > and > > > > > return null to tell the control servlet to not do any other > > > > requests/views > > > > > > > > > > Modified: ofbiz/trunk/applications/order/data/ > > > > OrderSystemPropertyData.xml > > > > > URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/order/ > > data/ > > > > > OrderSystemPropertyData.xml?rev=1757130&r1=1757129&r2= > > > 1757130&view=diff > > > > > ============================================================ > > > > > ================== > > > > > --- ofbiz/trunk/applications/order/data/ > OrderSystemPropertyData.xml > > > > > (original) > > > > > +++ ofbiz/trunk/applications/order/data/ > OrderSystemPropertyData.xml > > > Mon > > > > > Aug 22 09:58:35 2016 > > > > > @@ -50,4 +50,12 @@ order.properties setting is: order.item. > > > > > description="Allow comment on Order Item. Choices are: Y or > N." > > > > > /> > > > > > > > > > > +<!-- > > > > > +# > > > > > +marketing.tracking.enable=Y > > > > > +--> > > > > > + <SystemProperty systemResourceId="order" > > > > systemPropertyId="marketing.tracking.enable" > > > > > systemPropertyValue="Y" > > > > > + description="Allow tracking in marketing component (optional). > > > > > Choices are: Y or N." > > > > > + /> > > > > > + > > > > > </entity-engine-xml> > > > > > \ No newline at end of file > > > > > > > > > > Modified: ofbiz/trunk/applications/order/src/main/java/org/ > > > > > apache/ofbiz/order/shoppingcart/CheckOutEvents.java > > > > > URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/ > > > > > order/src/main/java/org/apache/ofbiz/order/ > > > shoppingcart/CheckOutEvents. > > > > > java?rev=1757130&r1=1757129&r2=1757130&view=diff > > > > > ============================================================ > > > > > ================== > > > > > --- ofbiz/trunk/applications/order/src/main/java/org/ > > > apache/ofbiz/order/ > > > > > shoppingcart/CheckOutEvents.java (original) > > > > > +++ ofbiz/trunk/applications/order/src/main/java/org/ > > > apache/ofbiz/order/ > > > > > shoppingcart/CheckOutEvents.java Mon Aug 22 09:58:35 2016 > > > > > @@ -42,7 +42,7 @@ import org.apache.ofbiz.entity.Delegator > > > > > import org.apache.ofbiz.entity.GenericEntityException; > > > > > import org.apache.ofbiz.entity.GenericValue; > > > > > import org.apache.ofbiz.entity.util.EntityQuery; > > > > > -import org.apache.ofbiz.marketing.tracking.TrackingCodeEvents; > > > > > +import org.apache.ofbiz.entity.util.EntityUtilProperties; > > > > > import org.apache.ofbiz.order.order.OrderReadHelper; > > > > > import org.apache.ofbiz.party.party.PartyWorker; > > > > > import org.apache.ofbiz.product.store.ProductStoreWorker; > > > > > @@ -53,6 +53,7 @@ import org.apache.ofbiz.service.ServiceU > > > > > import org.apache.ofbiz.webapp.stats.VisitHandler; > > > > > import org.apache.ofbiz.webapp.website.WebSiteWorker; > > > > > > > > > > + > > > > > /** > > > > > * Events used for processing checkout and orders. > > > > > */ > > > > > @@ -438,15 +439,28 @@ public class CheckOutEvents { > > > > > session.removeAttribute("_QUICK_REORDER_PRODUCTS_"); > > > > > > > > > > boolean areOrderItemsExploded = > explodeOrderItems(delegator, > > > > > cart); > > > > > - > > > > > - //get the TrackingCodeOrder List > > > > > - List<GenericValue> trackingCodeOrders = > TrackingCodeEvents. > > > > > makeTrackingCodeOrders(request); > > > > > + > > > > > + //get the TrackingCodeOrder List > > > > > + String trackingEnabled = EntityUtilProperties. > > > > > getPropertyValue("order","marketing.tracking.enable", delegator); > > > > > + Map<String, Object> trackingCodeOrders = new > HashMap<String, > > > > > Object>(); > > > > > + if (trackingEnabled != null && trackingEnabled == "Y") { > > > > > + //get the TrackingCodeOrder List > > > > > + Map<String, Object> inMap = new HashMap<String, > > Object>(); > > > > > + inMap.put("request", request); > > > > > + try { > > > > > + trackingCodeOrders = dispatcher.runSync(" > > > > > makeTrackingCodeOrder",inMap); > > > > > + } catch (GenericServiceException e) { > > > > > + Debug.logError(e, module); > > > > > + } > > > > > + } > > > > > + > > > > > String distributorId = (String) session.getAttribute("_ > > > > > DISTRIBUTOR_ID_"); > > > > > String affiliateId = (String) session.getAttribute("_ > > > > > AFFILIATE_ID_"); > > > > > String visitId = VisitHandler.getVisitId(session); > > > > > String webSiteId = WebSiteWorker.getWebSiteId(request); > > > > > + List<GenericValue> trackingCodeOrdersList = > > > > > UtilGenerics.checkList(new ArrayList<GenericValue>()); > > > > > > > > > > - callResult = checkOutHelper.createOrder(userLogin, > > > > > distributorId, affiliateId, trackingCodeOrders, > > areOrderItemsExploded, > > > > > visitId, webSiteId); > > > > > + callResult = checkOutHelper.createOrder(userLogin, > > > > > distributorId, affiliateId, trackingCodeOrdersList, > > > > areOrderItemsExploded, > > > > > visitId, webSiteId); > > > > > if (callResult != null) { > > > > > ServiceUtil.getMessages(request, callResult, null); > > > > > if (ServiceUtil.isError(callResult)) { > > > > > > > > > > Modified: ofbiz/trunk/applications/order/src/main/java/org/ > > > > > apache/ofbiz/order/shoppingcart/CheckOutHelper.java > > > > > URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/ > > > > > order/src/main/java/org/apache/ofbiz/order/ > > > shoppingcart/CheckOutHelper. > > > > > java?rev=1757130&r1=1757129&r2=1757130&view=diff > > > > > ============================================================ > > > > > ================== > > > > > --- ofbiz/trunk/applications/order/src/main/java/org/ > > > apache/ofbiz/order/ > > > > > shoppingcart/CheckOutHelper.java (original) > > > > > +++ ofbiz/trunk/applications/order/src/main/java/org/ > > > apache/ofbiz/order/ > > > > > shoppingcart/CheckOutHelper.java Mon Aug 22 09:58:35 2016 > > > > > @@ -559,7 +559,7 @@ public class CheckOutHelper { > > > > > > > > > > // Create order event - uses createOrder service for > processing > > > > > public Map<String, Object> createOrder(GenericValue userLogin, > > > > String > > > > > distributorId, String affiliateId, > > > > > - List<GenericValue> trackingCodeOrders, boolean > > > > > areOrderItemsExploded, String visitId, String webSiteId) { > > > > > + List<GenericValue> trackingCodeOrdersList, boolean > > > > > areOrderItemsExploded, String visitId, String webSiteId) { > > > > > if (this.cart == null) { > > > > > return null; > > > > > } > > > > > @@ -575,7 +575,7 @@ public class CheckOutHelper { > > > > > Map<String, Object> context = this.cart.makeCartMap(this. > > > > dispatcher, > > > > > areOrderItemsExploded); > > > > > > > > > > //get the TrackingCodeOrder List > > > > > - context.put("trackingCodeOrders", trackingCodeOrders); > > > > > + context.put("trackingCodeOrders", > trackingCodeOrdersList); > > > > > > > > > > if (distributorId != null) context.put("distributorId", > > > > > distributorId); > > > > > if (affiliateId != null) context.put("affiliateId", > > > > affiliateId); > > > > > > > > > > > > > > > > > > > > > > > > >
