You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by ad...@apache.org on 2012/10/23 13:40:52 UTC
svn commit: r1401254 -
/ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
Author: adrianc
Date: Tue Oct 23 11:40:51 2012
New Revision: 1401254
URL: http://svn.apache.org/viewvc?rev=1401254&view=rev
Log:
Commented out a bad ECA in the Example component that was causing exceptions to be thrown in the updateExample service.
Modified:
ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
Modified: ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
URL: http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/example/servicedef/secas.xml?rev=1401254&r1=1401253&r2=1401254&view=diff
==============================================================================
--- ofbiz/trunk/specialpurpose/example/servicedef/secas.xml (original)
+++ ofbiz/trunk/specialpurpose/example/servicedef/secas.xml Tue Oct 23 11:40:51 2012
@@ -23,8 +23,13 @@ under the License.
<eca service="createExample" event="return">
<action service="createExampleStatus" mode="sync"/>
</eca>
+ <!-- FIXME: This ECA assumes the updateExample service was called with a statusId parameter.
+ The ECAs in this file demonstrate a bad design pattern. The action services should be
+ invoked within the called service, not invoked by ECAs. -->
+ <!--
<eca service="updateExample" event="return">
<condition-field field-name="statusId" operator="not-equals" to-field-name="oldStatusId"/>
<action service="createExampleStatus" mode="sync"/>
</eca>
+ -->
</service-eca>
Re: svn commit: r1401254 - /ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
Posted by Adrian Crum <ad...@sandglass-software.com>.
I changed the comment. There is no condition to check for not empty.
-Adrian
On 10/24/2012 2:30 AM, Scott Gray wrote:
> Yeah that could be done but conditions exist to avoid the additional overhead of executing every possible ECA service.
>
> The quality of the pattern aside, my concern is more about the confusion to developers. To randomly declare a bad practice in the example app for something that is heavily used throughout the system would leave me scratching my head as to how exactly I'm supposed to trigger a SECA on a specific status change.
>
> Regards
> Scott
>
> On 24/10/2012, at 11:09 AM, Adrian Crum wrote:
>
>> It could be fixed by removing the ECA condition and have the createExampleStatus service do a better job of checking status changes.
>>
>> I agree the pattern is used heavily throughout the codebase, but that doesn't make it a good pattern.
>>
>> -Adrian
>>
>> On 10/23/2012 10:23 PM, Scott Gray wrote:
>>> Hi Adrian,
>>>
>>> This type of pattern is pretty heavily used throughout the codebase isn't it? While updating a status history table might not be the best use of it, it does still serve as an example of that pattern. Couldn't it be fixed by including an is-not-empty condition?
>>>
>>> Thanks
>>> Scott
>>>
>>> On 24/10/2012, at 12:40 AM, adrianc@apache.org wrote:
>>>
>>>> Author: adrianc
>>>> Date: Tue Oct 23 11:40:51 2012
>>>> New Revision: 1401254
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1401254&view=rev
>>>> Log:
>>>> Commented out a bad ECA in the Example component that was causing exceptions to be thrown in the updateExample service.
>>>>
>>>> Modified:
>>>> ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
>>>>
>>>> Modified: ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/example/servicedef/secas.xml?rev=1401254&r1=1401253&r2=1401254&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/specialpurpose/example/servicedef/secas.xml (original)
>>>> +++ ofbiz/trunk/specialpurpose/example/servicedef/secas.xml Tue Oct 23 11:40:51 2012
>>>> @@ -23,8 +23,13 @@ under the License.
>>>> <eca service="createExample" event="return">
>>>> <action service="createExampleStatus" mode="sync"/>
>>>> </eca>
>>>> + <!-- FIXME: This ECA assumes the updateExample service was called with a statusId parameter.
>>>> + The ECAs in this file demonstrate a bad design pattern. The action services should be
>>>> + invoked within the called service, not invoked by ECAs. -->
>>>> + <!--
>>>> <eca service="updateExample" event="return">
>>>> <condition-field field-name="statusId" operator="not-equals" to-field-name="oldStatusId"/>
>>>> <action service="createExampleStatus" mode="sync"/>
>>>> </eca>
>>>> + -->
>>>> </service-eca>
>>>>
>>>>
Re: svn commit: r1401254 - /ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
Posted by Scott Gray <sc...@hotwaxmedia.com>.
Yeah that could be done but conditions exist to avoid the additional overhead of executing every possible ECA service.
The quality of the pattern aside, my concern is more about the confusion to developers. To randomly declare a bad practice in the example app for something that is heavily used throughout the system would leave me scratching my head as to how exactly I'm supposed to trigger a SECA on a specific status change.
Regards
Scott
On 24/10/2012, at 11:09 AM, Adrian Crum wrote:
> It could be fixed by removing the ECA condition and have the createExampleStatus service do a better job of checking status changes.
>
> I agree the pattern is used heavily throughout the codebase, but that doesn't make it a good pattern.
>
> -Adrian
>
> On 10/23/2012 10:23 PM, Scott Gray wrote:
>> Hi Adrian,
>>
>> This type of pattern is pretty heavily used throughout the codebase isn't it? While updating a status history table might not be the best use of it, it does still serve as an example of that pattern. Couldn't it be fixed by including an is-not-empty condition?
>>
>> Thanks
>> Scott
>>
>> On 24/10/2012, at 12:40 AM, adrianc@apache.org wrote:
>>
>>> Author: adrianc
>>> Date: Tue Oct 23 11:40:51 2012
>>> New Revision: 1401254
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1401254&view=rev
>>> Log:
>>> Commented out a bad ECA in the Example component that was causing exceptions to be thrown in the updateExample service.
>>>
>>> Modified:
>>> ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
>>>
>>> Modified: ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/example/servicedef/secas.xml?rev=1401254&r1=1401253&r2=1401254&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/specialpurpose/example/servicedef/secas.xml (original)
>>> +++ ofbiz/trunk/specialpurpose/example/servicedef/secas.xml Tue Oct 23 11:40:51 2012
>>> @@ -23,8 +23,13 @@ under the License.
>>> <eca service="createExample" event="return">
>>> <action service="createExampleStatus" mode="sync"/>
>>> </eca>
>>> + <!-- FIXME: This ECA assumes the updateExample service was called with a statusId parameter.
>>> + The ECAs in this file demonstrate a bad design pattern. The action services should be
>>> + invoked within the called service, not invoked by ECAs. -->
>>> + <!--
>>> <eca service="updateExample" event="return">
>>> <condition-field field-name="statusId" operator="not-equals" to-field-name="oldStatusId"/>
>>> <action service="createExampleStatus" mode="sync"/>
>>> </eca>
>>> + -->
>>> </service-eca>
>>>
>>>
>
Re: svn commit: r1401254 - /ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
Posted by Adrian Crum <ad...@sandglass-software.com>.
It could be fixed by removing the ECA condition and have the
createExampleStatus service do a better job of checking status changes.
I agree the pattern is used heavily throughout the codebase, but that
doesn't make it a good pattern.
-Adrian
On 10/23/2012 10:23 PM, Scott Gray wrote:
> Hi Adrian,
>
> This type of pattern is pretty heavily used throughout the codebase isn't it? While updating a status history table might not be the best use of it, it does still serve as an example of that pattern. Couldn't it be fixed by including an is-not-empty condition?
>
> Thanks
> Scott
>
> On 24/10/2012, at 12:40 AM, adrianc@apache.org wrote:
>
>> Author: adrianc
>> Date: Tue Oct 23 11:40:51 2012
>> New Revision: 1401254
>>
>> URL: http://svn.apache.org/viewvc?rev=1401254&view=rev
>> Log:
>> Commented out a bad ECA in the Example component that was causing exceptions to be thrown in the updateExample service.
>>
>> Modified:
>> ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
>>
>> Modified: ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/example/servicedef/secas.xml?rev=1401254&r1=1401253&r2=1401254&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/specialpurpose/example/servicedef/secas.xml (original)
>> +++ ofbiz/trunk/specialpurpose/example/servicedef/secas.xml Tue Oct 23 11:40:51 2012
>> @@ -23,8 +23,13 @@ under the License.
>> <eca service="createExample" event="return">
>> <action service="createExampleStatus" mode="sync"/>
>> </eca>
>> + <!-- FIXME: This ECA assumes the updateExample service was called with a statusId parameter.
>> + The ECAs in this file demonstrate a bad design pattern. The action services should be
>> + invoked within the called service, not invoked by ECAs. -->
>> + <!--
>> <eca service="updateExample" event="return">
>> <condition-field field-name="statusId" operator="not-equals" to-field-name="oldStatusId"/>
>> <action service="createExampleStatus" mode="sync"/>
>> </eca>
>> + -->
>> </service-eca>
>>
>>
Re: svn commit: r1401254 - /ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
Posted by Scott Gray <sc...@hotwaxmedia.com>.
Hi Adrian,
This type of pattern is pretty heavily used throughout the codebase isn't it? While updating a status history table might not be the best use of it, it does still serve as an example of that pattern. Couldn't it be fixed by including an is-not-empty condition?
Thanks
Scott
On 24/10/2012, at 12:40 AM, adrianc@apache.org wrote:
> Author: adrianc
> Date: Tue Oct 23 11:40:51 2012
> New Revision: 1401254
>
> URL: http://svn.apache.org/viewvc?rev=1401254&view=rev
> Log:
> Commented out a bad ECA in the Example component that was causing exceptions to be thrown in the updateExample service.
>
> Modified:
> ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
>
> Modified: ofbiz/trunk/specialpurpose/example/servicedef/secas.xml
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/example/servicedef/secas.xml?rev=1401254&r1=1401253&r2=1401254&view=diff
> ==============================================================================
> --- ofbiz/trunk/specialpurpose/example/servicedef/secas.xml (original)
> +++ ofbiz/trunk/specialpurpose/example/servicedef/secas.xml Tue Oct 23 11:40:51 2012
> @@ -23,8 +23,13 @@ under the License.
> <eca service="createExample" event="return">
> <action service="createExampleStatus" mode="sync"/>
> </eca>
> + <!-- FIXME: This ECA assumes the updateExample service was called with a statusId parameter.
> + The ECAs in this file demonstrate a bad design pattern. The action services should be
> + invoked within the called service, not invoked by ECAs. -->
> + <!--
> <eca service="updateExample" event="return">
> <condition-field field-name="statusId" operator="not-equals" to-field-name="oldStatusId"/>
> <action service="createExampleStatus" mode="sync"/>
> </eca>
> + -->
> </service-eca>
>
>