Skip Menu |
 

This queue is for tickets about the Moose CPAN distribution.

Report information
The Basics
Id: 73451
Status: open
Priority: 0/
Queue: Moose

People
Owner: Nobody in particular
Requestors: TRLORENZ [...] cpan.org
Cc:
AdminCc:

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



Subject: Newval arg may be wrong in a trigger fired during _call_all_triggers.
Download (untitled) / with headers
text/plain 1.6k
When a trigger is fired for an attribute during _call_all_triggers, if $attr->should_coerce is false, the trigger callback will receive as its newval arg whatever value was supplied to the constructor, even if the attribute value has since changed or been cleared. This looks to me to have been a bug since at least as far back as 2.02, when _call_all_triggers was introduced. ===== use Test::More; # _SortedAttributes IS NEEDED FOR THIS TEST SO THAT TRIGGERS # FIRE IN A PREDICTABLE ORDER DURING _call_all_triggers. # ("X" SORTS BEFORE "Y".) BEGIN { package _SortedAttributes; use Moose; extends 'Moose::Meta::Class'; around get_all_attributes => sub { my ($orig_method, $self_aka_class) = (shift, shift); my @attributes = $self_aka_class->$orig_method(@_); return sort({ $a->name cmp $b->name } @attributes); }; } { package MyClass; use Moose -metaclass => '_SortedAttributes'; our @NEWVALS; has X => ( is => 'rw', isa => 'Any', trigger => sub { shift->clear_Y }, ); has Y => ( is => 'rw', isa => 'Any', clearer => 'clear_Y', trigger => sub { my ($self, $new, $old) = (shift, @_); push(@NEWVALS, [defined($new) ? $new : 'UNDEF'], [exists($self->{Y}) ? defined($self->{Y}) ? $self-> {Y} : 'UNDEF' : 'NOVAL'], ); }, ); } TODO: { local $TODO = 'bug demo'; # X WILL FIRE, CLEARING Y; THEN Y WILL FIRE. my $obj = MyClass->new({X => 'X', Y => 'Y'}); is_deeply($MyClass::NEWVALS[0], $MyClass::NEWVALS[1], 'newval arg equals actual current value'); }
Download (untitled) / with headers
text/plain 1.9k
On Thu Dec 22 20:19:54 2011, TRLORENZ wrote: Show quoted text
> When a trigger is fired for an attribute during _call_all_triggers, if > $attr->should_coerce is false, the trigger callback will receive as
its Show quoted text
> newval arg whatever value was supplied to the constructor, even if the > attribute value has since changed or been cleared. > > This looks to me to have been a bug since at least as far back as
2.02, Show quoted text
> when _call_all_triggers was introduced. > > ===== > > use Test::More; > > # _SortedAttributes IS NEEDED FOR THIS TEST SO THAT TRIGGERS > # FIRE IN A PREDICTABLE ORDER DURING _call_all_triggers. > # ("X" SORTS BEFORE "Y".) > > BEGIN { package _SortedAttributes; > > use Moose; > extends 'Moose::Meta::Class'; > > around get_all_attributes => sub { > > my ($orig_method, $self_aka_class) = (shift, shift); > > my @attributes = $self_aka_class->$orig_method(@_); > > return sort({ $a->name cmp $b->name } @attributes); > }; > } > > { package MyClass; > > use Moose -metaclass => '_SortedAttributes'; > > our @NEWVALS; > > has X => ( > > is => 'rw', > isa => 'Any', > trigger => sub { shift->clear_Y }, > ); > > has Y => ( > > is => 'rw', > isa => 'Any', > clearer => 'clear_Y', > trigger => sub { > > my ($self, $new, $old) = (shift, @_); > > push(@NEWVALS, > > [defined($new) ? $new : 'UNDEF'], > > [exists($self->{Y}) ? defined($self->{Y}) ? $self-> > {Y} : 'UNDEF' : 'NOVAL'], > ); > }, > ); > } > > TODO: { > > local $TODO = 'bug demo'; > > # X WILL FIRE, CLEARING Y; THEN Y WILL FIRE. > > my $obj = MyClass->new({X => 'X', Y => 'Y'}); > > is_deeply($MyClass::NEWVALS[0], $MyClass::NEWVALS[1], 'newval arg > equals actual current value'); > }
I'm finding this a little confusing. The bug report text talks about coercion, but the example code doesn't use coercion. What am I missing?
Subject: RE: [rt.cpan.org #73451] Newval arg may be wrong in a trigger fired during _call_all_triggers.
Date: Mon, 20 Feb 2012 20:05:12 -0700
To: <bug-moose [...] rt.cpan.org>
From: Todd Lorenz <trlorenz [...] hotmail.com>
Download (untitled) / with headers
text/plain 3.8k
The bug only manifests when $attr->should_coerce is false (which it would be by default), and it only affects _call_all_triggers. I think it's really only a problem when a trigger clears or changes the value of another triggered attribute during init. (Which is maybe something people shouldn't have their triggers doing, anyway... but, eh, it's bound to happen.) I didn't encounter this bug in the wild; I spotted it while writing MooseX::OmniTrigger, going through Moose code. I've since realized there's another minor issue with _call_all_triggers, arising from that fact that it avoids providing an oldval altogether: ** When a trigger is fired during _call_all_triggers for any preexisting attribute of an object being reblessed, the trigger callback will receive nothing (an empty list) for its oldval arg, even if at the start of the rebless the attribute had a value. ** The newval bug is an easy fix. The oldval bug I think would require saving the attribute's current value at the beginning of fixup to provide as the oldval during _call_all_triggers, which means an additional little bit of attribute state. (In MooseX::OmniTrigger I used a fieldhash for this.) Note: _inline_triggers suffers the oldval bug as well (but no big deal so long as there's no _inline_rebless). I'll gladly supply a patch if anyone can confirm these are in fact bugs (by Moose standards, I mean; I've confirmed the behavior through testing). TRL Show quoted text
> Subject: [rt.cpan.org #73451] Newval arg may be wrong in a trigger fired during _call_all_triggers. > From: bug-Moose@rt.cpan.org > To: TRLORENZ@cpan.org > Date: Sun, 19 Feb 2012 18:16:54 -0500 > > <URL: https://rt.cpan.org/Ticket/Display.html?id=73451 > > > On Thu Dec 22 20:19:54 2011, TRLORENZ wrote:
> > When a trigger is fired for an attribute during _call_all_triggers, if > > $attr->should_coerce is false, the trigger callback will receive as
> its
> > newval arg whatever value was supplied to the constructor, even if the > > attribute value has since changed or been cleared. > > > > This looks to me to have been a bug since at least as far back as
> 2.02,
> > when _call_all_triggers was introduced. > > > > ===== > > > > use Test::More; > > > > # _SortedAttributes IS NEEDED FOR THIS TEST SO THAT TRIGGERS > > # FIRE IN A PREDICTABLE ORDER DURING _call_all_triggers. > > # ("X" SORTS BEFORE "Y".) > > > > BEGIN { package _SortedAttributes; > > > > use Moose; > > extends 'Moose::Meta::Class'; > > > > around get_all_attributes => sub { > > > > my ($orig_method, $self_aka_class) = (shift, shift); > > > > my @attributes = $self_aka_class->$orig_method(@_); > > > > return sort({ $a->name cmp $b->name } @attributes); > > }; > > } > > > > { package MyClass; > > > > use Moose -metaclass => '_SortedAttributes'; > > > > our @NEWVALS; > > > > has X => ( > > > > is => 'rw', > > isa => 'Any', > > trigger => sub { shift->clear_Y }, > > ); > > > > has Y => ( > > > > is => 'rw', > > isa => 'Any', > > clearer => 'clear_Y', > > trigger => sub { > > > > my ($self, $new, $old) = (shift, @_); > > > > push(@NEWVALS, > > > > [defined($new) ? $new : 'UNDEF'], > > > > [exists($self->{Y}) ? defined($self->{Y}) ? $self-> > > {Y} : 'UNDEF' : 'NOVAL'], > > ); > > }, > > ); > > } > > > > TODO: { > > > > local $TODO = 'bug demo'; > > > > # X WILL FIRE, CLEARING Y; THEN Y WILL FIRE. > > > > my $obj = MyClass->new({X => 'X', Y => 'Y'}); > > > > is_deeply($MyClass::NEWVALS[0], $MyClass::NEWVALS[1], 'newval arg > > equals actual current value'); > > }
> > I'm finding this a little confusing. The bug report text talks about > coercion, but the example code doesn't use coercion. What am I missing? >
Subject: RE: [rt.cpan.org #73451] Newval arg may be wrong in a trigger fired during _call_all_triggers.
Date: Mon, 20 Feb 2012 20:09:20 -0700
To: <bug-moose [...] rt.cpan.org>
From: Todd Lorenz <trlorenz [...] hotmail.com>
Download (untitled) / with headers
text/plain 4.3k
Geez, that was originally nicely formatted. RT noob question: What am I doing wrong, here?> Subject: RE: [rt.cpan.org #73451] Newval arg may be wrong in a trigger fired during _call_all_triggers. Show quoted text
> From: bug-Moose@rt.cpan.org > To: TRLORENZ@cpan.org > Date: Mon, 20 Feb 2012 22:05:26 -0500 > > <URL: https://rt.cpan.org/Ticket/Display.html?id=73451 > > > > The bug only manifests when $attr->should_coerce is false (which it would > be by default), and it only affects _call_all_triggers. I think it's really only a problem when a trigger clears or changes the > value of another triggered attribute during init. (Which is maybe something > people shouldn't have their triggers doing, anyway... but, eh, it's bound > to happen.) I didn't encounter this bug in the wild; I spotted it while writing > MooseX::OmniTrigger, going through Moose code. I've since realized there's > another minor issue with _call_all_triggers, arising from that fact that it > avoids providing an oldval altogether: ** When a trigger is fired during _call_all_triggers for any preexisting > attribute of an object being reblessed, the trigger callback will receive > nothing (an empty list) for its oldval arg, even if at the start of the > rebless the attribute had a value. ** The newval bug is an easy fix. The oldval bug I think would require saving > the attribute's current value at the beginning of fixup to provide as the > oldval during _call_all_triggers, which means an additional little bit of > attribute state. (In MooseX::OmniTrigger I used a fieldhash for this.) Note: _inline_triggers suffers the oldval bug as well (but no big deal so > long as there's no _inline_rebless). I'll gladly supply a patch if anyone can confirm these are in fact bugs (by > Moose standards, I mean; I've confirmed the behavior through testing). TRL
> > Subject: [rt.cpan.org #73451] Newval arg may be wrong in a trigger fired during _call_all_triggers.
> > From: bug-Moose@rt.cpan.org > > To: TRLORENZ@cpan.org > > Date: Sun, 19 Feb 2012 18:16:54 -0500 > > > > <URL: https://rt.cpan.org/Ticket/Display.html?id=73451 > > > > > On Thu Dec 22 20:19:54 2011, TRLORENZ wrote:
> > > When a trigger is fired for an attribute during _call_all_triggers, if > > > $attr->should_coerce is false, the trigger callback will receive as
> > its
> > > newval arg whatever value was supplied to the constructor, even if the > > > attribute value has since changed or been cleared. > > > > > > This looks to me to have been a bug since at least as far back as
> > 2.02,
> > > when _call_all_triggers was introduced. > > > > > > ===== > > > > > > use Test::More; > > > > > > # _SortedAttributes IS NEEDED FOR THIS TEST SO THAT TRIGGERS > > > # FIRE IN A PREDICTABLE ORDER DURING _call_all_triggers. > > > # ("X" SORTS BEFORE "Y".) > > > > > > BEGIN { package _SortedAttributes; > > > > > > use Moose; > > > extends 'Moose::Meta::Class'; > > > > > > around get_all_attributes => sub { > > > > > > my ($orig_method, $self_aka_class) = (shift, shift); > > > > > > my @attributes = $self_aka_class->$orig_method(@_); > > > > > > return sort({ $a->name cmp $b->name } @attributes); > > > }; > > > } > > > > > > { package MyClass; > > > > > > use Moose -metaclass => '_SortedAttributes'; > > > > > > our @NEWVALS; > > > > > > has X => ( > > > > > > is => 'rw', > > > isa => 'Any', > > > trigger => sub { shift->clear_Y }, > > > ); > > > > > > has Y => ( > > > > > > is => 'rw', > > > isa => 'Any', > > > clearer => 'clear_Y', > > > trigger => sub { > > > > > > my ($self, $new, $old) = (shift, @_); > > > > > > push(@NEWVALS, > > > > > > [defined($new) ? $new : 'UNDEF'], > > > > > > [exists($self->{Y}) ? defined($self->{Y}) ? $self-> > > > {Y} : 'UNDEF' : 'NOVAL'], > > > ); > > > }, > > > ); > > > } > > > > > > TODO: { > > > > > > local $TODO = 'bug demo'; > > > > > > # X WILL FIRE, CLEARING Y; THEN Y WILL FIRE. > > > > > > my $obj = MyClass->new({X => 'X', Y => 'Y'}); > > > > > > is_deeply($MyClass::NEWVALS[0], $MyClass::NEWVALS[1], 'newval arg > > > equals actual current value'); > > > }
> > > > I'm finding this a little confusing. The bug report text talks about > > coercion, but the example code doesn't use coercion. What am I missing? > >
>
Subject: RE: [rt.cpan.org #73451] Newval arg may be wrong in a trigger fired during _call_all_triggers.
Date: Mon, 20 Feb 2012 21:47:53 -0600 (CST)
To: via RT <bug-Moose [...] rt.cpan.org>
From: Dave Rolsky <autarch [...] urth.org>
Download (untitled) / with headers
text/plain 538b
On Mon, 20 Feb 2012, via RT wrote: Show quoted text
> Geez, that was originally nicely formatted. RT noob question: What am I doing wrong, here?
Not really sure. My experience is that it formats reasonably well when I use the web interface. It may not handle mail from some clients all that well. -dave /*============================================================ http://VegGuide.org http://blog.urth.org Your guide to all that's veg House Absolute(ly Pointless) ============================================================*/


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.