Re: Incorrect use of eca for create/updateShipment

[Jacques Le Roux]

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, ki...@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)"<j...@apache.org>
To:     ki...@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