Basically I agree with Jacques. I am in favour of keeping ECAs, as I believe them (seca, eeca and meca) to be a useful and powerful extension mechanism.
However I think they are misused and overused in OOTB code. It is a while since I looked closely at them, so I can't give examples, but I have seen places where I couldn't understand why they were being used in preference to changing the triggering service. Which means either I didn't understand that part of the system properly, or they shouldn't have been implemented as ecas. It looks like Kiran has found and fixed one of the eca mis-uses. An update of an entity being triggered by a create of the same entity does not sound sensible. Surely the entity should be created correctly the first time, and not rely on triggered updates to reach a desired state. The risk of timing and similar bugs is high, for what advantage? An example of where I think a seca often makes sense: when a status change in one entity should trigger an async change elsewhere (not in the same entity). Cheers, Anne. On 5 November 2011 08:59, Jacques Le Roux <[email protected]> wrote: > I think that if we want to discuss this seriously we need to have 1st a > clear and complete workflow of ECA use in OFBiz. > > IMO, the Event Driven Architecture (EDA) > http://en.wikipedia.org/wiki/Event-driven_architecture, is well adapted to > complete SOA > (Service Oriented Architecture). But one Criticism of Event Driven > Programming > (http://en.wikipedia.org/wiki/Event-driven_programming#Criticism_and_best_practice) > apply: it can lead programmers to create > difficult to extend and, especially, excessively complex application code. > So it's rather a matter of use and abuse. > > In other words, I'd be ok to remove abuse but not use. In some cases it's > very convenient... > > Jacques > > J. Eckard wrote: >> >> I spend a great deal of time reading through existing OFBiz codebases to >> get a handle on process implementations, an experience >> that feels much more tedious and frustrating than it should. >> >> One of the things that contributes to the difficulty is the practice of >> using EECAs and / or SECAs to orchestrate a basic process >> with smaller, specialized services. >> >> I was hesitant to bring this up because I don't have any concrete >> suggestions or guidelines for improvements - its more of a >> nagging feeling I get when I see ECAs that are not very generic or used >> outside of the orchestration, or ECAs that perform >> crucial tasks that you would never want to disable or remove. >> >> Joe >> >> On Nov 3, 2011, at 2:12 PM, Adrian Crum wrote: >> >>> Actually, what I recommended was a discussion on using/removing ECAs in >>> general - not this specific case. >>> >>> -Adrian >>> >>> On 11/3/2011 5:15 PM, [email protected] wrote: >>>> >>>> Hello Friends, >>>> >>>> In case of createShipment, during commit, eca rules are fired. These >>>> invoke updateShipment(i.e: even before commit on createShipment is >>>> complete). Update Shipment is called multiple times (You can view the >>>> log >>>> during quickShipOrder/quickDropShipOrder). Also, these rules are fired >>>> in >>>> incorrect order. These rules are updating the same shipment that is >>>> being >>>> committed by the original method. I believe this is incorrect. >>>> >>>> I have verified the functionality of quickShipOrder/quickDropShipOrder >>>> after the changes. Let me know if there are any testcases that I need to >>>> run. Please take a look at email thread for more details. Let me know if >>>> you have questions and concerns. If we integrate the change sooner, we >>>> can >>>> avoid merge conflicts. >>>> >>>> PS: Thanks Adrian and Anil for your vote of confidence. As per your >>>> recommendation, I am posting this to dev mailing list. >>>> >>>> Regards, >>>> Kiran Gawde >>>> >>>> Senior Software Architect >>>> Object Edge Inc >>>> (925) 943 5558 x108 >>>> >>>> "There are two kind of people: Those who do the work and those who take >>>> the credit. Try to be in the first group because there is less >>>> competition >>>> there." >>>> "Never give up on what you really want to do. The person with big dreams >>>> is more powerful than one with all the facts". >>>> >>>> >>>> >>>> >>>> From: "Adrian Crum (Commented) (JIRA)"<[email protected]> >>>> To: [email protected] >>>> Date: 11/03/2011 02:04 AM >>>> Subject: [jira] [Commented] (OFBIZ-4501) Incorrect use of eca for >>>> create/updateShipment >>>> >>>> >>>> >>>> >>>> [ >>>> >>>> https://issues.apache.org/jira/browse/OFBIZ-4501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13142972#comment-13142972 >>>> ] >>>> >>>> Adrian Crum commented on OFBIZ-4501: >>>> ------------------------------------ >>>> >>>> Kiran, >>>> >>>> Thank you for working on this. I agree that the overuse of ECAs causes >>>> problems and makes the services difficult to troubleshoot. But removing >>>> them is going to be a tough sell because many in the community see it as >>>> a >>>> feature - they see it as a crude form of a workflow implementation. I >>>> have >>>> added my vote to this issue because I believe a lot of the ECAs should >>>> go >>>> away. It might help your cause to start a discussion on the dev mailing >>>> list and see if you can rally some more support for ECA removal. >>>> >>>> >>>>> Incorrect use of eca for create/updateShipment >>>>> ---------------------------------------------- >>>>> >>>>> Key: OFBIZ-4501 >>>>> URL: https://issues.apache.org/jira/browse/OFBIZ-4501 >>>>> Project: OFBiz >>>>> Issue Type: Bug >>>>> Components: product >>>>> Affects Versions: Release Branch 11.04, SVN trunk >>>>> Reporter: Kiran Gawde >>>>> Attachments: >>>> >>>> OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, >>>> OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, >>>> OFBIZ-4501-ShipmentServiceXml.patch, >>>> OFBIZ-4501-ShipmentServiceXml.patch, >>>> OFBIZ-4501-ShipmentServiceXml.patch >>>>> >>>>> createShipment service doesn't populate the facility and order info >>>>> into >>>> >>>> shipment. Instead it is handled by eca rules. This is wrong. ECA rules >>>> should be used to update other objects or cause other actions and not >>>> update the object that is being committed. This makes it difficult to >>>> traverse the code. Can also cause bugs that are difficult troubleshoot. >>>> e.g: In this case, facilities are populated in shipment by method >>>> setShipmentSettingsFromPrimaryOrder, but eca rule checking for >>>> originFacilityId gets executed before it is populated. Following eca >>>> rules >>>> should be removed and instead the code should be added to >>>> create/updateshipment methods. >>>>> >>>>> <!-- if new originFacilityId or destinationFacilityId, get settings >>>> >>>> from facilities --> >>>>> >>>>> <eca service="createShipment" event="commit"> >>>>> <condition field-name="originFacilityId" >>>> >>>> operator="is-not-empty"/> >>>>> >>>>> <action service="setShipmentSettingsFromFacilities" >>>> >>>> mode="sync"/> >>>>> >>>>> </eca> >>>>> <eca service="createShipment" event="commit"> >>>>> <condition field-name="destinationFacilityId" >>>> >>>> operator="is-not-empty"/> >>>>> >>>>> <action service="setShipmentSettingsFromFacilities" >>>> >>>> mode="sync"/> >>>>> >>>>> </eca> >>>>> <eca service="updateShipment" event="commit"> >>>>> <condition-field field-name="originFacilityId" >>>> >>>> operator="not-equals" to-field-name="oldOriginFacilityId"/> >>>>> >>>>> <condition field-name="originFacilityId" >>>> >>>> operator="is-not-empty"/> >>>>> >>>>> <action service="setShipmentSettingsFromFacilities" >>>> >>>> mode="sync"/> >>>>> >>>>> </eca> >>>>> <eca service="updateShipment" event="commit"> >>>>> <condition-field field-name="destinationFacilityId" >>>> >>>> operator="not-equals" to-field-name="oldDestinationFacilityId"/> >>>>> >>>>> <condition field-name="destinationFacilityId" >>>> >>>> operator="is-not-empty"/> >>>>> >>>>> <action service="setShipmentSettingsFromFacilities" >>>> >>>> mode="sync"/> >>>>> >>>>> </eca> >>>>> <!-- if new primaryOrderId, get settings from order --> >>>>> <eca service="createShipment" event="commit"> >>>>> <condition field-name="primaryOrderId" operator="is-not-empty"/> >>>>> <action service="setShipmentSettingsFromPrimaryOrder" >>>> >>>> mode="sync"/> >>>>> >>>>> </eca> >>>>> <eca service="updateShipment" event="commit"> >>>>> <condition-field field-name="primaryOrderId" >>>> >>>> operator="not-equals" to-field-name="oldPrimaryOrderId"/> >>>>> >>>>> <condition field-name="primaryOrderId" operator="is-not-empty"/> >>>>> <action service="setShipmentSettingsFromPrimaryOrder" >>>> >>>> mode="sync"/> >>>>> >>>>> </eca> >>>> >>>> -- >>>> This message is automatically generated by JIRA. >>>> If you think it was sent incorrectly, please contact your JIRA >>>> administrators: >>>> https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa >>>> For more information on JIRA, see: >>>> http://www.atlassian.com/software/jira > -- Coherent Software Australia Pty Ltd PO Box 2773 Cheltenham Vic 3192 Phone: (03) 9585 6788 Fax: (03) 9585 1086 Web: http://www.cohsoft.com.au/ Email: [email protected] Bonsai ERP, the all-inclusive ERP system http://www.bonsaierp.com.au/
