OFBiz › OFBiz - Dev

Incorrect use of eca for create/updateShipment

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

8 messages Options

Kiran-2

Incorrect use of eca for create/updateShipment

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)" <[hidden email]>
To: [hidden email]
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.
> 
> <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>
> 
> <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

Adrian Crum-3

Re: Incorrect use of eca for create/updateShipment

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, [hidden email] 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)"<[hidden email]>
> To: [hidden email]
> 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.
>> 
>> <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>
>> 
>> <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
>
>
>
>

J. Eckard-2

Re: Incorrect use of eca for create/updateShipment

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, [hidden email] 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)"<[hidden email]>
>> To: [hidden email]
>> 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.
>>> 
>>> <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>
>>> 
>>> <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
>>
>>
>>
>>

Jacques Le Roux

Re: Incorrect use of eca for create/updateShipment

Administrator

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, [hidden email] 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)"<[hidden email]>
>>> To: [hidden email]
>>> 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.
>>>> 
>>>> <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>
>>>> 
>>>> <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

Anne Jessel

Re: Incorrect use of eca for create/updateShipment

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 <[hidden email]> 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, [hidden email] 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)"<[hidden email]>
>>>> To: [hidden email]
>>>> 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.
>>>>>
>>>>> 
>>>>>
>>>>> <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>
>>>>> 
>>>>> <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: [hidden email]

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

BJ Freeman

Re: Incorrect use of eca for create/updateShipment

In reply to this post by Jacques Le Roux

we actually have two levels.
you have the entity level, similar to the Triggered procedures used in
Databases where any action of the data is evaluated, even at creation
time. the Difference is that most validation is Done the thing is
Validation can be done at the Entity engine level, So most EECA are used
after the creation of the data.

The other level is business flow. the decision to use SOA and/or ECA is
a Design call and will very with the Designer background. I have brokend
it down that porcesses that over many business flows will not change are
SOA. what was one defined as hooks in C code would the the ECA

Jacques Le Roux sent the following on 11/4/2011 2:59 PM:

> 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, [hidden email] 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)"<[hidden email]>
>>>> To: [hidden email]
>>>> 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.
>>>>> 
>>>>> <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>
>>>>> 
>>>>> <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
>

Jacques Le Roux

Re: Incorrect use of eca for create/updateShipment

Administrator

Yes, I was speaking about the service level. I don't think anybody has a pb with the EECAs?

Jacques

From: "BJ Freeman" <[hidden email]>

> we actually have two levels.
> you have the entity level, similar to the Triggered procedures used in
> Databases where any action of the data is evaluated, even at creation
> time. the Difference is that most validation is Done the thing is
> Validation can be done at the Entity engine level, So most EECA are used
> after the creation of the data.
>
> The other level is business flow. the decision to use SOA and/or ECA is
> a Design call and will very with the Designer background. I have brokend
> it down that porcesses that over many business flows will not change are
> SOA. what was one defined as hooks in C code would the the ECA
>
> Jacques Le Roux sent the following on 11/4/2011 2:59 PM:
>> 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, [hidden email] 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)"<[hidden email]>
>>>>> To: [hidden email]
>>>>> 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.
>>>>>> 
>>>>>> <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>
>>>>>> 
>>>>>> <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
>>

Kiran-2

Re: Incorrect use of eca for create/updateShipment

So looks like everyone is in the agreement that: "We should continue use
ECA but we just need to use them properly".

Can someone evaluate my changes and integrate them? This is preventing me
from releasing other patches in this area. Thanks.
https://issues.apache.org/jira/browse/OFBIZ-4501

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: "Jacques Le Roux" <[hidden email]>
To: <[hidden email]>
Date: 11/08/2011 04:31 AM
Subject: Re: Incorrect use of eca for create/updateShipment

Yes, I was speaking about the service level. I don't think anybody has a
pb with the EECAs?

Jacques

From: "BJ Freeman" <[hidden email]>

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, [hidden email] 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)"<[hidden email]>
>>>>> To: [hidden email]
>>>>> 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.
>>>>>> 
>>>>>> <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>
>>>>>> 
>>>>>> <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
>>