Skip Menu |

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

Report information
The Basics
Id: 21199
Status: open
Priority: 0/
Queue: Class-DBI

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

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



Subject: Construct throws away data if object is in LiveObjects index
Date: Sun, 27 Aug 2006 13:01:28 -0700 (PDT)
To: bugs-Class-DBI [...] rt.cpan.org
From: "Ryan Tate" <ryantate [...] ryantate.com>
If a row is retrieved via SQL, then turned into an object via CDBI's "construct" method, any columns retrieved for that row will be thrown away if the object is in the LiveObject index, since in that case the indexed version of the object is used with no attempt to add in new columns retrieved from the database. If the original object had only the id column, then only that column will be present in the "construc"ed object. A great way to fix this problem is to patch _init: sub _init { my $class = shift; my $data = shift || {}; my $key = $class->_live_object_key($data); my $obj; if ($obj = $Live_Objects{$key}) { $obj->_attribute_store(%$data); } else { $obj = $class->_fresh_init($key => $data); } return $obj; }
Subject: Re: [rt.cpan.org #21199] Construct throws away data if object is in LiveObjects index
Date: Mon, 28 Aug 2006 10:07:12 +0100
To: "ryantate [...] ryantate.com via RT" <bug-Class-DBI [...] rt.cpan.org>
From: Tony Bowden <tony [...] kasei.com>
On Sun, Aug 27, 2006 at 08:01:51PM -0400, ryantate@ryantate.com via RT wrote: Show quoted text
> If the original object had only the id column, then only that column will be present in the "construc"ed object.
Thanks for the report. You wouldn't happen to have a test case would you? I can probably put one together easier enough but it would be quicker if you already had one lying around! Thanks, Tony
Subject: Re: [rt.cpan.org #21199] Construct throws away data if object is in LiveObjects index
Date: Mon, 28 Aug 2006 09:01:09 -0700 (PDT)
To: "Tony Bowden via RT" <bug-Class-DBI [...] rt.cpan.org>
From: "Ryan Tate" <ryantate [...] ryantate.com>
I have enough code to put one together, shouldn't take more than ~24 hours... Show quoted text
-----Original Message----- From: Tony Bowden via RT <bug-Class-DBI@rt.cpan.org> Sent: Mon, August 28, 2006 2:07 am To: ryantate@ryantate.com Subject: Re: [rt.cpan.org #21199] Construct throws away data if object is in LiveObjects index <URL: http://rt.cpan.org/Ticket/Display.html?id=21199 > On Sun, Aug 27, 2006 at 08:01:51PM -0400, ryantate@ryantate.com via RT wrote:
> If the original object had only the id column, then only that column will be present in the "construc"ed object.
Thanks for the report. You wouldn't happen to have a test case would you? I can probably put one together easier enough but it would be quicker if you already had one lying around! Thanks, Tony
Subject: Re: [rt.cpan.org #21199] Construct throws away data if object is in LiveObjects index
Date: Tue, 29 Aug 2006 23:07:59 -0700 (PDT)
To: "Tony Bowden via RT" <bug-Class-DBI [...] rt.cpan.org>
From: "Ryan Tate" <ryantate [...] ryantate.com>
On Mon, August 28, 2006 2:07 am, Tony Bowden via RT <bug-Class-DBI@rt.cpan.org> said: Show quoted text
> You wouldn't happen to have a test case would > you?
A test case follows. In fact, two test cases follow. The second addresses a concern, apparently historical based on some list discussion, about what this amendment to ->construct will do to changed columns that have not yet been saved back to the database. There is a feeling the code should check for changed-but-unsaved columns and recognize cases where fresh data from the database sent to ->construct differs from in-memory data added to the object but not yet saved in the database. There was some discussion on whether the proper thing to do in such a case is to die or to preserve the unsaved data silently. I have come around to preserving unsaved data silently, since this is what Class::DBI does now -- ->construct ignored any new data from the database differing from the in-memory data (since it ignores *everything* from the database when there is in-memory data). It is more important to preseve existing behavior than to be "correct," and in this case there is debate over what's correct anyway. However, I do think CDBI should emit a warning in these cases. My second test, then, checks to make sure that if you change an object column, do not save the new data to the database, then try and ->construct that object with new, conflicting data for that column, CDBI will preservbe the changed, unsaved column value. This passes at the moment since this is how CDBI works now. Finally, my test is probably a little more complicated than strictly neccesary because it mirrors the real-world scenario where this emerged for me, which was fastest for me to code. Here are the tests: use strict; use warnings; package Project; use base 'Class::DBI::Test::SQLite'; __PACKAGE__->set_table('project'); __PACKAGE__->columns(Primary => qw(id)); __PACKAGE__->columns(Essential => qw(title size latest_event_id)); __PACKAGE__->has_a(latest_event_id => 'Event'); __PACKAGE__->has_many(events => ['Project::Event' => 'event_id']); sub create_sql{ return q{ id INT PRIMARY KEY, title VARCHAR(255), size INT, latest_event_id INT } } __PACKAGE__->set_sql('events_with_project_events', q{ SELECT event.id, event.title, project_event.occured_on FROM event, project_event WHERE event.id=project_event.event_id AND project_event.project_id=? }); sub events_with_project_events{ my $self = shift; my $sth = $self->sql_events_with_project_events; $sth->execute($self->id); my @events_with_project_events; while (my $row = $sth->fetchrow_arrayref) { my $i = 0; my %event = map { $_ => $row->[ $i++ ] } qw(id title); my %project_event = map { $_ => $row->[ $i++ ] } qw(occured_on); my $event = Event->construct(\%event); my $project_event = Project::Event->construct(\%project_event); push @events_with_project_events, [$event, $project_event]; } $sth->finish; return @events_with_project_events; } package Event; use base 'Class::DBI::Test::SQLite'; __PACKAGE__->set_table('event'); __PACKAGE__->autoupdate(0); __PACKAGE__->columns(Primary => qw(id)); __PACKAGE__->columns(Essential => qw(title)); __PACKAGE__->has_many(projects => ['Project::Event' => 'project_id']); sub create_sql{ return q{ id INT PRIMARY KEY, title VARCHAR(255) } } package Project::Event; use base 'Class::DBI::Test::SQLite'; __PACKAGE__->set_table('project_event'); __PACKAGE__->columns(Primary => qw(project_id event_id)); __PACKAGE__->columns(Essential => qw(occured_on)); sub create_sql{ return q{ project_id INT, event_id INT, occured_on INT, to_occur_on INT } } package main; use Test::More; plan tests => 2; Event->insert({id => 1, title => 'Conception'}); Event->insert({id => 2, title => 'Completion'}); Project->insert({id => 1, title => 'Cafe Julep', size => '1000', latest_event_id => 2}); Project::Event->insert({project_id => 1, event_id => 1, occured_on => 1156915321}); Project::Event->insert({project_id => 1, event_id => 2, occured_on => 1156915322}); my $project = Project->retrieve(1); my ($event_with_project_event) = grep { $_->[0]->id == 2 } $project->events_with_project_events; my ($event, $project_event) = @$event_with_project_event; ok(($event->_attr('title'))[0] eq 'Completion', 'construct can add data to objects in LiveObject index'); $event->title('Finished'); $project->events_with_project_events; ok(($event->_attr('title'))[0] eq 'Finished', 'construct preserves value of changed but unsaved columns of objects in LiveObject index');
Subject: Re: [rt.cpan.org #21199] Construct throws away data if object is in LiveObjects index
Date: Sun, 3 Sep 2006 22:46:51 +0100
To: "ryantate [...] ryantate.com via RT" <bug-Class-DBI [...] rt.cpan.org>
From: Tony Bowden <tony [...] kasei.com>
On Wed, Aug 30, 2006 at 02:08:16AM -0400, ryantate@ryantate.com via RT wrote: Show quoted text
> A test case follows.
Thanks. I'm just back from YAPC::Europe, so I'll have a look at this in more detail once I've caught up. Tony
From: jon [...] burdge.org
I'm hitting this problem as well. I created a test case based on the existing .t files in the distribution and have attached it here. I tried to copy what you had done in another tests and make a local version of *Ima::DBI::st:execute which would fail if the database was accessed, but this didn't seem to work as I expected and I'm not very familiar with Ima::DBI so I went with something simpler.
use strict; use Test::More; use Data::Dumper; BEGIN { eval "use DBD::SQLite"; plan $@ ? (skip_all => 'needs DBD::SQLite for testing') : (tests => 9); } use lib 't/testlib'; use Film; use Director; # finish Film configuration ok( Film->columns(Essential => qw/Title Director/), "set essential film columns" ); ok(Film->has_a('director' => 'Director'), "Link Director table"); Film->create_test_film; # finish Directory configuration ok( Director->columns(Essential => qw/Name Birthday IsInsane/), "set essential director columns" ); ok( Director->insert( { Name => 'Peter Jackson', Birthday => -300000000, IsInsane => 1 } ), 'insert Director' ); # verify that retrieving a director populates essential fields { ok my $pj = Director->retrieve(Name => 'Peter Jackson'), "found pj"; ok defined $pj->{birthday}, "essential birthday column is loaded"; } # verify that retrieving a director populates essential fields after the # stub director object has already been created by retrieving the movie ok my $btaste = Film->retrieve('Bad Taste'), "We have Bad Taste"; ok my $pj = Director->retrieve(Name => 'Peter Jackson'), "found pj again"; ok defined $pj->{birthday}, "essential birthday column is loaded";