Skip Menu |
 

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

Report information
The Basics
Id: 16515
Status: new
Priority: 0/
Queue: Class-DBI

People
Owner: Nobody in particular
Requestors: d.rowles [...] outcometechnologies.com
Cc:
AdminCc:

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

Attachments


Subject: Deep recursion on _flesh
Download (untitled) / with headers
text/plain 901b
Included in the attached patch is a test case that causes a "deep recursion on _flesh" error when run against Class::DBI v3.0.12. The deep recursion is being caused by a "before_create" trigger which attempts to assign a default value for the "id" field if it doesn't already have one (this is useful for using UUIDs as your primary keys). The problem is caused by the "unless $this->id" statement. This calls "$this->get('id')", which notices that "$this->_attribute_exists('id')" returns false, and attempts to call "$this->_flesh" to load that column from the database. Of course, calling "$this->_flesh" calls "$this->id" again, causing the deep recursion. The patch also includes a fix, which disables the "_flesh" method when being called from within a "before_create" and "deflate_for_create" trigger. This is not a particularly elegant way of solving this problem - although it is effective!
Download recursion.diff
text/x-diff 1.9k
diff -urN Class-DBI-v3.0.12.old/lib/Class/DBI.pm Class-DBI-v3.0.12.new/lib/Class/DBI.pm --- Class-DBI-v3.0.12.old/lib/Class/DBI.pm 2005-11-04 09:22:56.000000000 +0000 +++ Class-DBI-v3.0.12.new/lib/Class/DBI.pm 2005-12-15 16:50:59.000000000 +0000 @@ -532,6 +532,7 @@ my $class = ref $proto || $proto; my $self = $class->_init($data); + $self->{__are_in_insert_method} = 1; $self->call_trigger('before_create'); $self->call_trigger('deflate_for_create'); @@ -544,6 +545,7 @@ $self->_attrs($col); } $self->_insert_row($real); + delete $self->{__are_in_insert_method}; my @primary_columns = $class->primary_columns; $self->_attribute_store( @@ -818,6 +820,10 @@ sub _flesh { my ($self, @groups) = @_; + if(ref($self) && $self->{__are_in_insert_method}) { + $self->call_trigger('select'); + return 1; + } my @real = grep $_ ne "TEMP", @groups; if (my @want = grep !$self->_attribute_exists($_), $self->__grouper->columns_in(@real)) { diff -urN Class-DBI-v3.0.12.old/t/26-deep-recursion.t Class-DBI-v3.0.12.new/t/26-deep-recursion.t --- Class-DBI-v3.0.12.old/t/26-deep-recursion.t 1970-01-01 01:00:00.000000000 +0100 +++ Class-DBI-v3.0.12.new/t/26-deep-recursion.t 2005-12-15 16:38:49.000000000 +0000 @@ -0,0 +1,29 @@ +use strict; +use Test::More; + +#---------------------------------------------------------------------- +# Test lazy loading +#---------------------------------------------------------------------- + +BEGIN { + eval "use DBD::SQLite"; + plan $@ ? (skip_all => 'needs DBD::SQLite for testing') : (tests => 4); +} + +INIT { + use lib 't/testlib'; + use Actor; +} + + +Actor->add_trigger(before_create => sub { + my $this = shift; + $this->set_id(123) unless $this->id; + $this->set_salary(123) unless $this->salary; +}); + +ok(my $a1 = Actor->create({ name => "Bob Bobbage" }), + "Create an actor"); +ok($a1->id, "has an id"); +is($a1->name, "Bob Bobbage", "actor name"); +is($a1->salary, 123, "salary ok");
[guest - Thu Dec 15 11:54:22 2005]: Show quoted text
> Included in the attached patch is a test case that causes a "deep > recursion on _flesh" error when run against Class::DBI v3.0.12. The > deep recursion is being caused by a "before_create" trigger which > attempts to assign a default value for the "id" field if it doesn't > already have one (this is useful for using UUIDs as your primary > keys).
At one level, this is a known issue, and is documented not to work. You're meant to use the attribute methods at this point rather than direct mutators. That said, this is a problem, and I'm certainly open to patches to fix it. Show quoted text
> The patch also includes a fix, which disables the "_flesh" method when > being called from within a "before_create" and "deflate_for_create" > trigger. This is not a particularly elegant way of solving this > problem - although it is effective!
I'd like to ponder this one for a while to see if there's a better solution. Have you discussed this approach on the mailing list? Thanks, Tony


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.