From: Peter Rabbitson Date: Mon, 30 Nov 2009 09:07:49 +0000 (+0000) Subject: Restore backwards compatibility - introduce intent to collapse flag X-Git-Tag: v0.08240~38 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=9f6555d31bc48b6fa3792f74368fc1f17f77ea60;p=dbsrgits%2FDBIx-Class.git Restore backwards compatibility - introduce intent to collapse flag When no collapsing is requested HRI structures must always be hashrefs, not AoH as it is with a collapsed multi prefetch, roll the tests back to reflect this --- diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index ac349bb..37e1b4b 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -971,85 +971,6 @@ sub _construct_object { return @new; } -# _unflatten_result takes a row hashref which looks like this: -# $VAR1 = { -# 'cd.artist.artistid' => '1', -# 'cd.artist' => '1', -# 'cd_id' => '1', -# 'cd.genreid' => undef, -# 'cd.year' => '1999', -# 'cd.title' => 'Spoonful of bees', -# 'cd.single_track' => undef, -# 'cd.artist.name' => 'Caterwauler McCrae', -# 'cd.artist.rank' => '13', -# 'cd.artist.charfield' => undef, -# 'cd.cdid' => '1' -# }; - -# and generates the following structure: - -# $VAR1 = [ -# { -# 'cd_id' => '1' -# }, -# { -# 'cd' => [ -# { -# 'single_track' => undef, -# 'cdid' => '1', -# 'artist' => '1', -# 'title' => 'Spoonful of bees', -# 'year' => '1999', -# 'genreid' => undef -# }, -# { -# 'artist' => [ -# { -# 'artistid' => '1', -# 'charfield' => undef, -# 'name' => 'Caterwauler McCrae', -# 'rank' => '13' -# } -# ] -# } -# ] -# } -# ]; - -# It returns one row object which consists of an arrayref with two -# elements. The first contains the plain column data, the second -# contains the data of relationships. Those are row arrayrefs, themselves. - -# it's a recursive function. It needs to request the relationship_info -# to decide whether to put the data of a relationship in a hashref -# (i.e. belongs_to) or an arrayref (i.e. has_many). - -sub _unflatten_result { - my ( $self, $row ) = @_; - - my $columns = {}; - my $rels = {}; - - foreach my $column ( sort keys %$row ) { - if ( $column =~ /^(.*?)\.(.*)$/ ) { - $rels->{$1} ||= {}; - $rels->{$1}->{$2} = $row->{$column}; - } - else { - $columns->{$column} = $row->{$column}; - } - } - - foreach my $rel ( sort keys %$rels ) { - my $rel_info = $self->result_source->relationship_info($rel); - $rels->{$rel} = - $self->related_resultset($rel)->_unflatten_result( $rels->{$rel} ); - $rels->{$rel} = [ $rels->{$rel} ] - if ( $rel_info->{attrs}->{accessor} eq 'multi' ); - } - - return keys %$rels ? [ $columns, $rels ] : [$columns]; -} # two arguments: $as_proto is an arrayref of column names, # $row_ref is an arrayref of the data. If none of the row data @@ -1082,7 +1003,7 @@ sub _collapse_result { do { my $i = 0; my $row = { map { $_ => $row[ $i++ ] } @$as_proto }; - $row = $self->_unflatten_result($row); + $row = $self->result_source->_parse_row($row, $collapse); unless ( scalar @$rows ) { push( @$rows, $row ); } diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm index cce1d77..aac050a 100644 --- a/lib/DBIx/Class/ResultSource.pm +++ b/lib/DBIx/Class/ResultSource.pm @@ -1503,6 +1503,46 @@ sub _resolve_prefetch { } } +# Takes a hashref of $sth->fetchrow values keyed to the corresponding +# {as} dbic aliases, and splits it into a native columns hashref +# (as in $row->get_columns), followed by any non-native (prefetched) +# columns, presented in a nested structure resembling an HRI dump. +# The structure is constructed taking into account relationship metadata +# (single vs multi). +# The resulting arrayref resembles the arguments to ::Row::inflate_result +# For an example look at t/prefetch/_util.t +# +# The will collapse flag is for backwards compatibility only - if it is +# set, all relationship row-parts are returned as hashes, even if some +# of these relationships are has_many's +# +sub _parse_row { + my ( $self, $row, $will_collapse ) = @_; + + my ($me, $pref); + + foreach my $column ( keys %$row ) { + if ( $column =~ /^ ([^\.]+) \. (.*) $/x ) { + $pref->{$1}{$2} = $row->{$column}; + } + else { + $me->{$column} = $row->{$column}; + } + } + + foreach my $rel ( keys %{$pref||{}} ) { + my $rel_info = $self->relationship_info($rel); + + $pref->{$rel} = + $self->related_source($rel)->_parse_row( $pref->{$rel}, $will_collapse ); + + $pref->{$rel} = [ $pref->{$rel} ] + if ( $will_collapse && $rel_info->{attrs}{accessor} eq 'multi' ); + } + + return [ $me||{}, $pref||() ]; +} + =head2 related_source =over 4 diff --git a/t/19quotes.t b/t/19quotes.t index 8750d5a..398aa04 100644 --- a/t/19quotes.t +++ b/t/19quotes.t @@ -2,7 +2,6 @@ use strict; use warnings; use Test::More; -use IO::File; use lib qw(t/lib); use DBIC::SqlMakerTest; diff --git a/t/19quotes_newstyle.t b/t/19quotes_newstyle.t index 3e7595a..ccf1445 100644 --- a/t/19quotes_newstyle.t +++ b/t/19quotes_newstyle.t @@ -2,7 +2,6 @@ use strict; use warnings; use Test::More; -use IO::File; use lib qw(t/lib); use DBIC::SqlMakerTest; diff --git a/t/26dumper.t b/t/26dumper.t index e392df9..a238085 100644 --- a/t/26dumper.t +++ b/t/26dumper.t @@ -1,6 +1,5 @@ use strict; use Test::More; -use IO::File; use Data::Dumper; $Data::Dumper::Sortkeys = 1; diff --git a/t/76joins.t b/t/76joins.t index ba87a0a..0d5512e 100644 --- a/t/76joins.t +++ b/t/76joins.t @@ -10,8 +10,6 @@ my $schema = DBICTest->init_schema(); my $orig_debug = $schema->storage->debug; -use IO::File; - BEGIN { eval "use DBD::SQLite"; plan $@ diff --git a/t/76select.t b/t/76select.t index 164f58e..e662266 100644 --- a/t/76select.t +++ b/t/76select.t @@ -9,8 +9,6 @@ use DBIC::SqlMakerTest; my $schema = DBICTest->init_schema(); -plan tests => 23; - my $rs = $schema->resultset('CD')->search({}, { '+select' => \ 'COUNT(*)', @@ -170,14 +168,12 @@ is_deeply( { artist => 1, track_position => 2, - tracks => [ - { - trackid => 17, - title => 'Apiary', - }, - ], + tracks => { + trackid => 17, + title => 'Apiary', + }, }, 'columns/select/as fold properly on sub-searches', ); - +done_testing; diff --git a/t/inflate/file_column.t b/t/inflate/file_column.t index a9a75f0..639b12d 100644 --- a/t/inflate/file_column.t +++ b/t/inflate/file_column.t @@ -4,7 +4,6 @@ use warnings; use Test::More; use lib qw(t/lib); use DBICTest; -use IO::File; use File::Compare; use Path::Class qw/file/; diff --git a/t/inflate/hri.t b/t/inflate/hri.t index 79f96c6..034b080 100644 --- a/t/inflate/hri.t +++ b/t/inflate/hri.t @@ -28,7 +28,7 @@ my $schema = DBICTest->init_schema(); sub check_cols_of { my ($dbic_obj, $datahashref) = @_; - + foreach my $col (keys %$datahashref) { # plain column if (not ref ($datahashref->{$col}) ) { @@ -42,14 +42,14 @@ sub check_cols_of { elsif (ref ($datahashref->{$col}) eq 'ARRAY') { my @dbic_reltable = $dbic_obj->$col; my @hashref_reltable = @{$datahashref->{$col}}; - + is (scalar @hashref_reltable, scalar @dbic_reltable, 'number of related entries'); # for my $index (0..scalar @hashref_reltable) { for my $index (0..scalar @dbic_reltable) { my $dbic_reltable_obj = $dbic_reltable[$index]; my $hashref_reltable_entry = $hashref_reltable[$index]; - + check_cols_of($dbic_reltable_obj, $hashref_reltable_entry); } } @@ -110,8 +110,8 @@ for my $index (0 .. $#hashrefinf) { my $datahashref = $hashrefinf[$index]; is ($track->cd->artist->name, $datahashref->{name}, 'Brought back correct artist'); - for my $col (keys %{$datahashref->{cds}[0]{tracks}[0]}) { - is ($track->get_column ($col), $datahashref->{cds}[0]{tracks}[0]{$col}, "Correct track '$col'"); + for my $col (keys %{$datahashref->{cds}{tracks}}) { + is ($track->get_column ($col), $datahashref->{cds}{tracks}{$col}, "Correct track '$col'"); } } diff --git a/t/prefetch/_internals.t b/t/prefetch/_internals.t new file mode 100644 index 0000000..8ccea90 --- /dev/null +++ b/t/prefetch/_internals.t @@ -0,0 +1,102 @@ +use strict; +use warnings; + +use Test::More; +use lib qw(t/lib); +use DBICTest; + +my $schema = DBICTest->init_schema(no_deploy => 1); + + +my $irow = $schema->source ('Artwork')->_parse_row ( + { + 'cd_id' => '1', + + 'artwork_to_artist.artist_id' => '2', + 'artwork_to_artist.artwork_cd_id' => '1', + + 'cd.artist' => '1', + 'cd.cdid' => '1', + 'cd.title' => 'Spoonful of bees', + + 'cd.artist.artistid' => '1', + 'cd.artist.name' => 'Caterwauler McCrae', + }, + 'will collapse' +); + +is_deeply ( + $irow, + [ + { + 'cd_id' => '1' + }, + { + 'artwork_to_artist' => [ + [ + { + 'artist_id' => '2', + 'artwork_cd_id' => '1' + } + ] + ], + + 'cd' => [ + { + 'artist' => '1', + 'cdid' => '1', + 'title' => 'Spoonful of bees', + }, + { + 'artist' => [ + { + 'artistid' => '1', + 'name' => 'Caterwauler McCrae', + } + ] + } + ] + } + ], + '_parse_row works as expected with expected collapse', +); + +$irow = $schema->source ('Artist')->_parse_row ( + { + 'name' => 'Caterwauler McCrae', + 'cds.tracks.cd' => '3', + 'cds.tracks.title' => 'Fowlin', + 'cds.tracks.cd_single.title' => 'Awesome single', + } +); +is_deeply ( + $irow, + [ + { + 'name' => 'Caterwauler McCrae' + }, + { + 'cds' => [ + {}, + { + 'tracks' => [ + { + 'cd' => '3', + 'title' => 'Fowlin' + }, + { + 'cd_single' => [ + { + title => 'Awesome single', + }, + ], + }, + ] + } + ] + } + ], + '_parse_row works over missing joins without collapse', +); + +done_testing; diff --git a/t/prefetch/diamond.t b/t/prefetch/diamond.t index 9c87919..0de8009 100644 --- a/t/prefetch/diamond.t +++ b/t/prefetch/diamond.t @@ -96,8 +96,6 @@ foreach my $cd_path (keys %$cd_paths) { } } -plan tests => (scalar (keys %tests) * 3); - foreach my $name (keys %tests) { foreach my $artwork ($tests{$name}->all()) { is($artwork->id, 1, $name . ', correct artwork'); @@ -105,3 +103,5 @@ foreach my $name (keys %tests) { is($artwork->artwork_to_artist->first->artist->artistid, 2, $name . ', correct artist_id over A2A'); } } + +done_testing; diff --git a/t/prefetch/multiple_hasmany.t b/t/prefetch/multiple_hasmany.t index b9671c9..9c7bf38 100644 --- a/t/prefetch/multiple_hasmany.t +++ b/t/prefetch/multiple_hasmany.t @@ -5,7 +5,6 @@ use Test::More; use Test::Exception; use lib qw(t/lib); use DBICTest; -use IO::File; my $schema = DBICTest->init_schema(); my $sdebug = $schema->storage->debug; diff --git a/t/prefetch/multiple_hasmany_torture.t b/t/prefetch/multiple_hasmany_torture.t new file mode 100644 index 0000000..973df8b --- /dev/null +++ b/t/prefetch/multiple_hasmany_torture.t @@ -0,0 +1,303 @@ +use strict; +use warnings; + +use Test::More; +use Test::Exception; +use lib qw(t/lib); +use DBICTest; + +my $schema = DBICTest->init_schema(); + +my $mo_rs = $schema->resultset('Artist')->search( + { 'me.artistid' => 4 }, + { + prefetch => [ + { + cds => [ + { tracks => { cd_single => 'tracks' } }, + { cd_to_producer => 'producer' } + ] + }, + { artwork_to_artist => 'artwork' } + ], + + result_class => 'DBIx::Class::ResultClass::HashRefInflator', + } +); + + +$schema->resultset('Artist')->create( + { + name => 'mo', + rank => '1337', + cds => [ + { + title => 'Song of a Foo', + year => '1999', + tracks => [ + { + title => 'Foo Me Baby One More Time', + }, + { + title => 'Foo Me Baby One More Time II', + }, + { + title => 'Foo Me Baby One More Time III', + }, + { + title => 'Foo Me Baby One More Time IV', + cd_single => + { artist => 1, title => 'MO! Single', year => 2021, tracks => [ + { title => 'singled out' }, { title => 'still alone' }, + ] }, + } + ], + cd_to_producer => [ + { producer => { name => 'riba' } }, + { producer => { name => 'sushi' } }, + ] + }, + { + title => 'Song of a Foo II', + year => '2002', + tracks => [ + { + title => 'Quit Playing Games With My Heart', + }, + { + title => 'Bar Foo', + }, + { + title => 'Foo Bar', + cd_single => + { artist => 2, title => 'MO! Single', year => 2020, tracks => [ + { title => 'singled out' }, { title => 'still alone' }, + ] }, + } + ], + cd_to_producer => [ + { producer => { name => 'riba' } }, + { producer => { name => 'sushi' } }, + ], + } + ], + artwork_to_artist => + [ { artwork => { cd_id => 1 } }, { artwork => { cd_id => 2 } } ] + } +); + +my $mo = $mo_rs->next; + +is( @{$mo->{cds}}, 2, 'two CDs' ); + +is_deeply( + $mo, + { + 'cds' => [ + { + 'single_track' => undef, + 'tracks' => [ + { + 'small_dt' => undef, + 'cd' => '6', + 'position' => '1', + 'trackid' => '19', + 'title' => 'Foo Me Baby One More Time', + 'cd_single' => undef, + 'last_updated_on' => undef, + 'last_updated_at' => undef + }, + { + 'small_dt' => undef, + 'cd' => '6', + 'position' => '2', + 'trackid' => '20', + 'title' => 'Foo Me Baby One More Time II', + 'cd_single' => undef, + 'last_updated_on' => undef, + 'last_updated_at' => undef + }, + { + 'small_dt' => undef, + 'cd' => '6', + 'position' => '3', + 'trackid' => '21', + 'title' => 'Foo Me Baby One More Time III', + 'cd_single' => undef, + 'last_updated_on' => undef, + 'last_updated_at' => undef + }, + { + 'small_dt' => undef, + 'cd' => '6', + 'position' => '4', + 'trackid' => '22', + 'title' => 'Foo Me Baby One More Time IV', + 'last_updated_on' => undef, + 'last_updated_at' => undef, + 'cd_single' => { + 'single_track' => '22', + 'artist' => '1', + 'cdid' => '7', + 'title' => 'MO! Single', + 'genreid' => undef, + 'year' => '2021', + 'tracks' => [ + { + 'small_dt' => undef, + 'cd' => '7', + 'position' => '1', + 'title' => 'singled out', + 'trackid' => '23', + 'last_updated_at' => undef, + 'last_updated_on' => undef + }, + { + 'small_dt' => undef, + 'cd' => '7', + 'position' => '2', + 'title' => 'still alone', + 'trackid' => '24', + 'last_updated_at' => undef, + 'last_updated_on' => undef + }, + ], + }, + } + ], + 'artist' => '4', + 'cdid' => '6', + 'cd_to_producer' => [ + { + 'attribute' => undef, + 'cd' => '6', + 'producer' => { + 'name' => 'riba', + 'producerid' => '4' + } + }, + { + 'attribute' => undef, + 'cd' => '6', + 'producer' => { + 'name' => 'sushi', + 'producerid' => '5' + } + } + ], + 'title' => 'Song of a Foo', + 'genreid' => undef, + 'year' => '1999' + }, + { + 'single_track' => undef, + 'tracks' => [ + # FIXME + # although the positional ordering is correct, SQLite seems to return + # the rows randomly if an ORDER BY is not supplied. Of course ordering + # by right side of prefetch joins is not yet possible, thus we just hope + # that the order is stable + { + 'small_dt' => undef, + 'cd' => '8', + 'position' => '2', + 'trackid' => '26', + 'title' => 'Bar Foo', + 'cd_single' => undef, + 'last_updated_on' => undef, + 'last_updated_at' => undef + }, + { + 'small_dt' => undef, + 'cd' => '8', + 'position' => '1', + 'trackid' => '25', + 'title' => 'Quit Playing Games With My Heart', + 'last_updated_on' => undef, + 'last_updated_at' => undef, + 'cd_single' => undef, + }, + { + 'small_dt' => undef, + 'cd' => '8', + 'position' => '3', + 'trackid' => '27', + 'title' => 'Foo Bar', + 'last_updated_on' => undef, + 'last_updated_at' => undef, + 'cd_single' => { + 'single_track' => '27', + 'artist' => '2', + 'cdid' => '9', + 'title' => 'MO! Single', + 'genreid' => undef, + 'year' => '2020', + 'tracks' => [ + { + 'small_dt' => undef, + 'cd' => '9', + 'position' => '1', + 'title' => 'singled out', + 'trackid' => '28', + 'last_updated_at' => undef, + 'last_updated_on' => undef + }, + { + 'small_dt' => undef, + 'cd' => '9', + 'position' => '2', + 'title' => 'still alone', + 'trackid' => '29', + 'last_updated_at' => undef, + 'last_updated_on' => undef + }, + ], + + }, + }, + ], + 'artist' => '4', + 'cdid' => '8', + 'cd_to_producer' => [ + { + 'attribute' => undef, + 'cd' => '8', + 'producer' => { + 'name' => 'riba', + 'producerid' => '4' + } + }, + { + 'attribute' => undef, + 'cd' => '8', + 'producer' => { + 'name' => 'sushi', + 'producerid' => '5' + } + } + ], + 'title' => 'Song of a Foo II', + 'genreid' => undef, + 'year' => '2002' + } + ], + 'artistid' => '4', + 'charfield' => undef, + 'name' => 'mo', + 'artwork_to_artist' => [ + { + 'artwork' => { 'cd_id' => '1' }, + 'artist_id' => '4', + 'artwork_cd_id' => '1' + }, + { + 'artwork' => { 'cd_id' => '2' }, + 'artist_id' => '4', + 'artwork_cd_id' => '2' + } + ], + 'rank' => '1337' + } +); + +done_testing; diff --git a/t/prefetch/standard.t b/t/prefetch/standard.t index 7980da3..66479b0 100644 --- a/t/prefetch/standard.t +++ b/t/prefetch/standard.t @@ -5,7 +5,6 @@ use Test::More; use Test::Exception; use lib qw(t/lib); use DBICTest; -use IO::File; my $schema = DBICTest->init_schema(); my $orig_debug = $schema->storage->debug; diff --git a/t/prefetch/with_limit.t b/t/prefetch/with_limit.t index 24625ad..b8c13a3 100644 --- a/t/prefetch/with_limit.t +++ b/t/prefetch/with_limit.t @@ -8,8 +8,6 @@ use Test::Exception; use lib qw(t/lib); use DBICTest; -plan tests => 9; - my $schema = DBICTest->init_schema(); @@ -25,6 +23,8 @@ my $no_prefetch = $schema->resultset('Artist')->search( my $use_prefetch = $no_prefetch->search( {}, { + select => ['me.artistid', 'me.name'], + as => ['artistid', 'name'], prefetch => 'cds', order_by => { -desc => 'name' }, } @@ -89,3 +89,5 @@ is($artist->cds->count, 1, "count on search limiting prefetched has_many"); # try with double limit my $artist2 = $use_prefetch->search({'cds.title' => { '!=' => $artist_many_cds->cds->first->title } })->slice (0,0)->next; is($artist2->cds->count, 2, "count on search limiting prefetched has_many"); + +done_testing; diff --git a/t/storage/debug.t b/t/storage/debug.t index bb55aba..480ad6e 100644 --- a/t/storage/debug.t +++ b/t/storage/debug.t @@ -6,25 +6,19 @@ use lib qw(t/lib); use DBICTest; use DBIC::DebugObj; use DBIC::SqlMakerTest; +use Path::Class qw/file/; my $schema = DBICTest->init_schema(); -plan tests => 7; ok ( $schema->storage->debug(1), 'debug' ); -ok ( defined( - $schema->storage->debugfh( - IO::File->new('t/var/sql.log', 'w') - ) - ), - 'debugfh' - ); +$schema->storage->debugfh(file('t/var/sql.log')->openw); $schema->storage->debugfh->autoflush(1); my $rs = $schema->resultset('CD')->search({}); $rs->count(); -my $log = new IO::File('t/var/sql.log', 'r') or die($!); +my $log = file('t/var/sql.log')->openr; my $line = <$log>; $log->close(); ok($line =~ /^SELECT COUNT/, 'Log success'); @@ -33,7 +27,7 @@ $schema->storage->debugfh(undef); $ENV{'DBIC_TRACE'} = '=t/var/foo.log'; $rs = $schema->resultset('CD')->search({}); $rs->count(); -$log = new IO::File('t/var/foo.log', 'r') or die($!); +$log = file('t/var/foo.log')->openr; $line = <$log>; $log->close(); ok($line =~ /^SELECT COUNT/, 'Log success'); @@ -70,4 +64,4 @@ open(STDERR, '>&STDERRCOPY'); ); } -1; +done_testing;