ofbiz-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anne <a...@cohsoft.com.au>
Subject Re: Incorrect use of eca for create/updateShipment
Date Mon, 07 Nov 2011 02:54:51 GMT
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 <jacques.le.roux@les7arts.com> 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, 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
>



-- 
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: sales@cohsoft.com.au

Bonsai ERP, the all-inclusive ERP system
http://www.bonsaierp.com.au/

Mime
View raw message