Skip Menu |

This queue is for tickets about the Class-Trigger CPAN distribution.

Report information
The Basics
Id: 37299
Status: open
Priority: 0/
Queue: Class-Trigger

People
Owner: Nobody in particular
Requestors: ryu1ro [...] gmail.com
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: inherit
Date: Wed, 02 Jul 2008 00:43:32 +0900
To: bug-Class-Trigger [...] rt.cpan.org
From: Hiroyuki Kobayashi <ryu1ro [...] gmail.com>
package Foo; use Class::Trigger qw(init); sub init { my $class = shift; my $self = bless {},$class; $self->call_trigger('init'); return $self; } __PACKAGE__->add_trigger(init => sub { warn "foo";} ); package Bar; use base 'Foo'; __PACKAGE__->add_trigger(init => sub { warn "bar";} ); package Baz; use base 'Bar'; __PACKAGE__->add_trigger(init => sub { warn "baz";} ); package main; my $foo = Foo->init; my $bar = Bar->init; my $baz = Baz->init; 0.12 results foo at test.pl line 11. foo at test.pl line 11. bar at test.pl line 16. foo at test.pl line 11. bar at test.pl line 16. baz at test.pl line 21. 0.13 results foo at test.pl line 11. foo at test.pl line 11. bar at test.pl line 16. bar at test.pl line 16. baz at test.pl line 21.
Subject: inherited triggers / cache is broken
This bit me also. The problem is caused by the addition of the class triggers cache in 0.13. __fetch_all_triggers() added this section: + return __cached_triggers($obj, $when) + if $Fetch_All_Triggers_Cache{$class}{$when_key}; But the problem is that __fetch_all_triggers calls itself recursively farther down, ignoring the return value: __fetch_all_triggers($c, $when, $list, $order, 1) unless $c eq $class; Instead, it depends on __fetch_all_triggers updating $list and $order, which simply does not happen if the cache is hit. I fixed this by added a $nocache arg to __fetch_all_triggers. Patch, including test case, is attached.
From 54da8028048dc8d731003e36fd315a9b50fb7d04 Mon Sep 17 00:00:00 2001 From: Michael Schout <mschout@gkg.net> Date: Wed, 7 Oct 2009 12:06:16 -0500 Subject: [PATCH] properly handle lookup of inherited triggers in __fetch_all_triggers(). The addition of the cache caused this to break lookups of inherited triggers. --- lib/Class/Trigger.pm | 11 +++++++---- t/09_inherit.t | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 t/09_inherit.t diff --git a/lib/Class/Trigger.pm b/lib/Class/Trigger.pm index 5d1db0f..44f1685 100644 --- a/lib/Class/Trigger.pm +++ b/lib/Class/Trigger.pm @@ -87,13 +87,16 @@ sub call_trigger { } sub __fetch_all_triggers { - my ($obj, $when, $list, $order) = @_; + my ($obj, $when, $list, $order, $nocache) = @_; + $nocache = 0 unless defined $nocache; my $class = ref $obj || $obj; my $return; my $when_key = defined $when ? $when : ''; - return __cached_triggers($obj, $when) - if $Fetch_All_Triggers_Cache{$class}{$when_key}; + unless ($nocache) { + return __cached_triggers($obj, $when) + if $Fetch_All_Triggers_Cache{$class}{$when_key}; + } unless ($list) { # Absence of the $list parameter conditions the creation of @@ -112,7 +115,7 @@ sub __fetch_all_triggers { next if $list->{$c}; if (UNIVERSAL::can($c, 'call_trigger')) { $list->{$c} = []; - __fetch_all_triggers($c, $when, $list, $order) + __fetch_all_triggers($c, $when, $list, $order, 1) unless $c eq $class; if (defined $when && $Triggers{$c}{$when}) { push @$order, $c; diff --git a/t/09_inherit.t b/t/09_inherit.t new file mode 100644 index 0000000..5df3779 --- /dev/null +++ b/t/09_inherit.t @@ -0,0 +1,40 @@ +use strict; +use Test::More tests => 3; + +my @data; + +package Foo; +use Class::Trigger qw(init); + +sub init { + my $class = shift; + my $self = bless {},$class; + $self->call_trigger('init'); + return $self; +} + +__PACKAGE__->add_trigger(init => sub { push @data, 'foo' }); + +package Bar; +use base 'Foo'; + +__PACKAGE__->add_trigger(init => sub { push @data, 'bar' }); + +package Baz; +use base 'Bar'; + +__PACKAGE__->add_trigger(init => sub { push @data, 'baz' }); + +package main; + +Foo->init; +is join(':', @data), 'foo'; +@data = (); + +Bar->init; +is join(':', @data), 'foo:bar'; +@data = (); + +Baz->init; +is join(':', @data), 'foo:bar:baz'; +@data = (); -- 1.6.0.4
and just to clarify, my patch does not disable the cache. it only disables the cache on the recursive __fetch_all_triggers() calls. So triggers are still cached after they are computed the first time.
Subject: Re: [rt.cpan.org #37299] inherited triggers / cache is broken
Date: Wed, 7 Oct 2009 21:54:42 -0700
To: bug-Class-Trigger [...] rt.cpan.org
From: Tatsuhiko Miyagawa <miyagawa [...] gmail.com>
Applied and shipped 0.13_01 to CPAN. Thanks! On Wed, Oct 7, 2009 at 10:11 AM, MSCHOUT via RT <bug-Class-Trigger@rt.cpan.org> wrote: Show quoted text
>       Queue: Class-Trigger >  Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=37299 > > > This bit me also. > > The problem is caused by the addition of the class triggers cache in > 0.13.  __fetch_all_triggers() added this section: > > +    return __cached_triggers($obj, $when) > +        if $Fetch_All_Triggers_Cache{$class}{$when_key}; > > But the problem is that __fetch_all_triggers calls itself recursively > farther down, ignoring the return value: > >            __fetch_all_triggers($c, $when, $list, $order, 1) >                unless $c eq $class; > > Instead, it depends on __fetch_all_triggers updating $list and $order, > which simply does not happen if the cache is hit. > > I fixed this by added a $nocache arg to __fetch_all_triggers.  Patch, > including test case, is attached. >
-- Tatsuhiko Miyagawa