Restore backwards compatibility - introduce intent to collapse flag
Peter Rabbitson [Mon, 30 Nov 2009 09:07:49 +0000 (09:07 +0000)]
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

16 files changed:
lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/ResultSource.pm
t/19quotes.t
t/19quotes_newstyle.t
t/26dumper.t
t/76joins.t
t/76select.t
t/inflate/file_column.t
t/inflate/hri.t
t/prefetch/_internals.t [new file with mode: 0644]
t/prefetch/diamond.t
t/prefetch/multiple_hasmany.t
t/prefetch/multiple_hasmany_torture.t [new file with mode: 0644]
t/prefetch/standard.t
t/prefetch/with_limit.t
t/storage/debug.t

index ac349bb..37e1b4b 100644 (file)
@@ -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 );
         }
index cce1d77..aac050a 100644 (file)
@@ -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
index 8750d5a..398aa04 100644 (file)
@@ -2,7 +2,6 @@ use strict;
 use warnings;
 
 use Test::More;
-use IO::File;
 
 use lib qw(t/lib);
 use DBIC::SqlMakerTest;
index 3e7595a..ccf1445 100644 (file)
@@ -2,7 +2,6 @@ use strict;
 use warnings;
 
 use Test::More;
-use IO::File;
 
 use lib qw(t/lib);
 use DBIC::SqlMakerTest;
index e392df9..a238085 100644 (file)
@@ -1,6 +1,5 @@
 use strict;
 use Test::More;
-use IO::File;
 
 use Data::Dumper;
 $Data::Dumper::Sortkeys = 1;
index ba87a0a..0d5512e 100644 (file)
@@ -10,8 +10,6 @@ my $schema = DBICTest->init_schema();
 
 my $orig_debug = $schema->storage->debug;
 
-use IO::File;
-
 BEGIN {
     eval "use DBD::SQLite";
     plan $@
index 164f58e..e662266 100644 (file)
@@ -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;
index a9a75f0..639b12d 100644 (file)
@@ -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/;
 
index 79f96c6..034b080 100644 (file)
@@ -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 (file)
index 0000000..8ccea90
--- /dev/null
@@ -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;
index 9c87919..0de8009 100644 (file)
@@ -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;
index b9671c9..9c7bf38 100644 (file)
@@ -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 (file)
index 0000000..973df8b
--- /dev/null
@@ -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;
index 7980da3..66479b0 100644 (file)
@@ -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;
index 24625ad..b8c13a3 100644 (file)
@@ -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;
index bb55aba..480ad6e 100644 (file)
@@ -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;