Skip Menu |
 

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the Workflow CPAN distribution.

Report information
The Basics
Id: 38024
Status: resolved
Priority: 0/
Queue: Workflow

People
Owner: jonasbn [...] cpan.org
Requestors: robert.stockdale [...] gmail.com
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 0.32_2
Fixed in:
  • 0.32_6
  • 1.32



Subject: all conditions with 'test' attirbute appears to be cached as 'evaluate'
Download (untitled) / with headers
text/plain 467b
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.
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>
Download (untitled) / with headers
text/plain 1.3k
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.
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>
Download (untitled) / with headers
text/plain 1.6k
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
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>
Download (untitled) / with headers
text/plain 3.6k
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
From: robert.stockdale [...] gmail.com
Download (untitled) / with headers
text/plain 916b
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}",
Download (untitled) / with headers
text/plain 278b
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.
Download state.patch
text/x-diff 1.6k
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__
Download (untitled) / with headers
text/plain 124b
This has been fixed and is currently under evaluation in release 0.32_6, which is a beta release available on CPAN. jonasbn


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.