ofbiz-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jacques Le Roux" <jacques.le.r...@les7arts.com>
Subject Re: Incorrect use of eca for create/updateShipment
Date Fri, 04 Nov 2011 21:59:21 GMT
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, kiran@objectedge.com 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)"<jira@apache.org>
>>> To:     kiran@objectedge.com
>>> 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

Mime
View raw message