Bug #38024 for Workflow: all conditions with 'test' attirbute appears to be cached as 'evaluate'

Tue Jul 29 16:05:14 2008 robert.stockdale [...] gmail.com - Ticket created

Subject:

all conditions with 'test' attirbute appears to be cached as 'evaluate'

It looks like conditions specified with a 'test' attribute instead of a 'name' attribute are cached with the name 'evaluate' regardless of the test being performed so if more than one of these conditions exists at a particular state the cached value will always be used. This is a major problem when using autorun states where one condition is the negation of another. Unfortunately I have not yet been able to verify this because of the other bug I just submitted.

Tue Jul 29 16:44:45 2008 jonasbn [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #38024] all conditions with 'test' attirbute appears to be cached as 'evaluate'
Date:	Tue, 29 Jul 2008 22:44:09 +0200
To:	bug-Workflow [...] rt.cpan.org
From:	Jonas Brømsø Nielsen <jonasbn [...] gmail.com>

Hi Robert, I have fixed the issue you previously raised (RT #38023), do you want me to create a preliminary release for you to use in pursuing the issues you describe below? I will forward this ticket information to our mailing list, to see if anybody else has experienced or want to out some effort into investigating the problem you describe and I will have a look myself. Thanks for the bug reports. jonasbn On 29/07/2008, at 22.05, Robert Stockdale via RT wrote: Show quoted text

> Tue Jul 29 16:05:14 2008: Request 38024 was acted upon. > Transaction: Ticket created by stocks29 > Queue: Workflow > Subject: all conditions with 'test' attirbute appears to be > cached as > 'evaluate' > Broken in: 0.32_2 > Severity: Important > Owner: Nobody > Requestors: robert.stockdale@gmail.com > Status: new > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=38024 > > > > It looks like conditions specified with a 'test' attribute instead > of a > 'name' attribute are cached with the name 'evaluate' regardless of the > test being performed so if more than one of these conditions exists > at a > particular state the cached value will always be used. > > This is a major problem when using autorun states where one > condition is > the negation of another. > > Unfortunately I have not yet been able to verify this because of the > other bug I just submitted.

Tue Jul 29 16:44:49 2008 The RT System itself - Status changed from 'new' to 'open'

Tue Jul 29 22:50:59 2008 robert.stockdale [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #38024] all conditions with 'test' attirbute appears to be cached as 'evaluate'
Date:	Tue, 29 Jul 2008 22:50:45 -0400
To:	bug-Workflow [...] rt.cpan.org
From:	"Robert Stockdale" <robert.stockdale [...] gmail.com>

Hi Jonas, A preliminary release would be great. On Tue, Jul 29, 2008 at 4:44 PM, Jonas B. Nielsen via RT < bug-Workflow@rt.cpan.org> wrote: Show quoted text

> <URL: http://rt.cpan.org/Ticket/Display.html?id=38024 > > > Hi Robert, > > I have fixed the issue you previously raised (RT #38023), do you want > me to create a preliminary release for you to use in pursuing the > issues you describe below? > > I will forward this ticket information to our mailing list, to see if > anybody else has experienced or want to out some effort into > investigating the problem you describe and I will have a look myself. > > Thanks for the bug reports. > > jonasbn > > On 29/07/2008, at 22.05, Robert Stockdale via RT wrote: >

> > Tue Jul 29 16:05:14 2008: Request 38024 was acted upon. > > Transaction: Ticket created by stocks29 > > Queue: Workflow > > Subject: all conditions with 'test' attirbute appears to be > > cached as > > 'evaluate' > > Broken in: 0.32_2 > > Severity: Important > > Owner: Nobody > > Requestors: robert.stockdale@gmail.com > > Status: new > > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=38024 > > > > > > > It looks like conditions specified with a 'test' attribute instead > > of a > > 'name' attribute are cached with the name 'evaluate' regardless of the > > test being performed so if more than one of these conditions exists > > at a > > particular state the cached value will always be used. > > > > This is a major problem when using autorun states where one > > condition is > > the negation of another. > > > > Unfortunately I have not yet been able to verify this because of the > > other bug I just submitted.

> > >

-- Robert Stockdale robert.stockdale@gmail.com

Sat Aug 16 15:33:23 2008 jonasbn [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #38024] all conditions with 'test' attirbute appears to be cached as 'evaluate'
Date:	Sat, 16 Aug 2008 21:32:29 +0200
To:	bug-Workflow [...] rt.cpan.org
From:	Jonas Brømsø Nielsen <jonasbn [...] gmail.com>

Hi Robert, A new release (0.32_3) have just made it to the CPAN so check your local mirror. Looking forward to your feedback, jonasbn On 30/07/2008, at 04.51, Robert Stockdale via RT wrote: Show quoted text

> Queue: Workflow > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=38024 > > > Hi Jonas, > > A preliminary release would be great. > > On Tue, Jul 29, 2008 at 4:44 PM, Jonas B. Nielsen via RT < > bug-Workflow@rt.cpan.org> wrote: >

>> <URL: http://rt.cpan.org/Ticket/Display.html?id=38024 > >> >> Hi Robert, >> >> I have fixed the issue you previously raised (RT #38023), do you want >> me to create a preliminary release for you to use in pursuing the >> issues you describe below? >> >> I will forward this ticket information to our mailing list, to see if >> anybody else has experienced or want to out some effort into >> investigating the problem you describe and I will have a look myself. >> >> Thanks for the bug reports. >> >> jonasbn >> >> On 29/07/2008, at 22.05, Robert Stockdale via RT wrote: >>

>>> Tue Jul 29 16:05:14 2008: Request 38024 was acted upon. >>> Transaction: Ticket created by stocks29 >>> Queue: Workflow >>> Subject: all conditions with 'test' attirbute appears to be >>> cached as >>> 'evaluate' >>> Broken in: 0.32_2 >>> Severity: Important >>> Owner: Nobody >>> Requestors: robert.stockdale@gmail.com >>> Status: new >>> Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=38024 > >>> >>> >>> It looks like conditions specified with a 'test' attribute instead >>> of a >>> 'name' attribute are cached with the name 'evaluate' regardless of >>> the >>> test being performed so if more than one of these conditions exists >>> at a >>> particular state the cached value will always be used. >>> >>> This is a major problem when using autorun states where one >>> condition is >>> the negation of another. >>> >>> Unfortunately I have not yet been able to verify this because of the >>> other bug I just submitted.

>> >> >>

> > > -- > Robert Stockdale > robert.stockdale@gmail.com > > Hi Jonas, > > A preliminary release would be great. > > On Tue, Jul 29, 2008 at 4:44 PM, Jonas B. Nielsen via RT <bug-Workflow@rt.cpan.org

> > wrote:

> <URL: http://rt.cpan.org/Ticket/Display.html?id=38024 > > > Hi Robert, > > I have fixed the issue you previously raised (RT #38023), do you want > me to create a preliminary release for you to use in pursuing the > issues you describe below? > > I will forward this ticket information to our mailing list, to see if > anybody else has experienced or want to out some effort into > investigating the problem you describe and I will have a look myself. > > Thanks for the bug reports. > > jonasbn > > On 29/07/2008, at 22.05, Robert Stockdale via RT wrote: >

> > Tue Jul 29 16:05:14 2008: Request 38024 was acted upon. > > Transaction: Ticket created by stocks29 > > Queue: Workflow > > Subject: all conditions with 'test' attirbute appears to be > > cached as > > 'evaluate' > > Broken in: 0.32_2 > > Severity: Important > > Owner: Nobody > > Requestors: robert.stockdale@gmail.com > > Status: new > > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=38024 > > > > > > > It looks like conditions specified with a 'test' attribute instead > > of a > > 'name' attribute are cached with the name 'evaluate' regardless of

> the

> > test being performed so if more than one of these conditions exists > > at a > > particular state the cached value will always be used. > > > > This is a major problem when using autorun states where one > > condition is > > the negation of another. > > > > Unfortunately I have not yet been able to verify this because of the > > other bug I just submitted.

> > > > > > -- > Robert Stockdale > robert.stockdale@gmail.com

Mon Sep 29 15:48:39 2008 robert.stockdale [...] gmail.com - Correspondence added

From:

robert.stockdale [...] gmail.com

Hi Jonas, Sorry for the delay. I was able to confirm this bug. The name is being explicitly set to 'evaluate' in Workflow::State::_create_condition_objects for all conditions that specify 'test' as an attribute in the xml configuration. This, coupled along with caching of condition results means that only the first 'evaluate' condition will actually be evaluated and each subsequent call will use the cached result. Attached is a patch that will resolve the issue. Essentially I am just taking the value of the 'test' attribute and naming the condition that, with one small exception. Perl seems to have some issue with keys that start with '!' so I prefixed it with an underscore. I tested this change and it is working in my dev environment. An alternative option would be to allow the user to specify the 'name' attribute in the xml just like with any other condition. I did not exercise this option. -Bob

347c347 < name => 'evaluate', --- > name => "_$condition_info->{test}",

Tue Nov 18 14:33:58 2008 STOCKS [...] cpan.org - Correspondence added

I have attached an updated patch file. It can be applied directly to State.pm. Also, I wrote some automated tests but they require modifications to a few files. What is the best way to get these updates to you? Creating individual patches for each file seemed a little tedious.

diff --git a/workflow/lib/Workflow/State.pm b/workflow/lib/Workflow/State.pm index bc1333a..af2f5da 100644 --- a/workflow/lib/Workflow/State.pm +++ b/workflow/lib/Workflow/State.pm @@ -12,7 +12,8 @@ use Workflow::Factory qw( FACTORY ); $Workflow::State::VERSION = '1.13'; my @FIELDS = qw( state description type ); -__PACKAGE__->mk_accessors( @FIELDS ); +my @INTERNAL = qw( _test_condition_count ); +__PACKAGE__->mk_accessors( @FIELDS, @INTERNAL ); my ( $log ); @@ -261,6 +262,7 @@ sub init { # Note this is the workflow type. $self->type( $config->{type} ); $self->description( $config->{description} ); + if ( $config->{autorun} ) { $self->autorun( $config->{autorun} ); } @@ -343,8 +345,11 @@ sub _create_condition_objects { # Special case: a 'test' denotes our 'evaluate' condition if ( $condition_info->{test} ) { + my $state = $self->state(); + my $action = $action_config->{name}; + my $count = $self->_get_next_condition_count(); push @condition_objects, Workflow::Condition::Evaluate->new({ - name => 'evaluate', + name => "_$state\_$action\_condition\_$count", class => 'Workflow::Condition::Evaluate', test => $condition_info->{test}, }); @@ -377,6 +382,18 @@ sub _contains_action_check { } } +sub _get_next_condition_count { + my ($self) = @_; + + # Initialize if not set. + my $count + = defined $self->_test_condition_count() + ? $self->_test_condition_count() + 1 + : 1; + + return $self->_test_condition_count($count); +} + 1; __END__

Wed Dec 17 09:15:17 2008 jonasbn [...] cpan.org - Taken

Wed Dec 17 09:16:06 2008 jonasbn [...] cpan.org - Correspondence added

This has been fixed and is currently under evaluation in release 0.32_6, which is a beta release available on CPAN. jonasbn

Wed Dec 17 09:16:07 2008 jonasbn [...] cpan.org - Status changed from 'open' to 'resolved'

Wed Dec 17 09:16:35 2008 jonasbn [...] cpan.org - Fixed in 0.32_6 added

Sat Oct 27 09:16:01 2012 jonasbn [...] cpan.org - Fixed in 1.32 added

Bug #38024 for Workflow: all conditions with 'test' attirbute appears to be cached as 'evaluate'

Preferred bug tracker