From: Peter Rabbitson <ribasushi@cpan.org>
Date: Mon, 28 Mar 2016 16:56:46 +0000 (+0200)
Subject: More tests and tighter code with better error reporting in collapser maker
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=9ceb04c6a5a6de6e11009e1f58e387a77c618284;p=dbsrgits%2FDBIx-Class.git

More tests and tighter code with better error reporting in collapser maker

Additionally RowParser::Util was scanned with Devel::Core to ensure I don't
have any remaining holes similar to 1fd3505d. The coverage of said file is
not at 100%, due to bugs in Devel::Cover itself, notably handling of
$x = $y || $z ( https://github.com/pjcj/Devel--Cover/issues/154 )
---

diff --git a/lib/DBIx/Class/ResultSource/RowParser/Util.pm b/lib/DBIx/Class/ResultSource/RowParser/Util.pm
index d82f7b4..09b8ec4 100644
--- a/lib/DBIx/Class/ResultSource/RowParser/Util.pm
+++ b/lib/DBIx/Class/ResultSource/RowParser/Util.pm
@@ -4,8 +4,7 @@ package # hide from the pauses
 use strict;
 use warnings;
 
-use List::Util 'first';
-use DBIx::Class::_Util 'perlstring';
+use DBIx::Class::_Util qw( perlstring dump_value );
 
 use constant HAS_DOR => ( "$]" < 5.010 ? 0 : 1 );
 
@@ -126,7 +125,7 @@ sub assemble_collapsing_parser {
     my @path_parts = map { sprintf
       "( ( defined \$cur_row_data->[%d] ) && (join qq(\xFF), '', %s, '') )",
       $_->[0],  # checking just first is enough - one ID defined, all defined
-      ( join ', ', map { ++$variant_idcols->{$_} and " \$cur_row_ids{$_} " } @$_ ),
+      ( join ', ', map { $variant_idcols->{$_} = 1; " \$cur_row_ids{$_} " } @$_ ),
     } @variants;
 
     my $virtual_column_idx = (scalar keys %{$args->{val_index}} ) + 1;
@@ -144,7 +143,10 @@ sub assemble_collapsing_parser {
     };
   }
   else {
-    die('Unexpected collapse map contents');
+    DBIx::Class::Exception->throw(
+     'Unexpected collapse map contents: ' . dump_value $args->{collapse_map},
+      1,
+    )
   }
 
   my ($data_assemblers, $stats) = __visit_infmap_collapse ($args);
@@ -169,7 +171,7 @@ sub assemble_collapsing_parser {
     )
   ;
 
-  my $parser_src = sprintf (<<'EOS', $row_id_defs, $top_node_key_assembler||'', $top_node_key, join( "\n", @{$data_assemblers||[]} ) );
+  my $parser_src = sprintf (<<'EOS', $row_id_defs, $top_node_key_assembler||'', $top_node_key, join( "\n", @$data_assemblers ) );
 ### BEGIN LITERAL STRING EVAL
   my $rows_pos = 0;
   my ($result_pos, @collapse_idx, $cur_row_data, %%cur_row_ids );
@@ -322,12 +324,17 @@ sub __visit_infmap_collapse {
 
     if (
       $relinfo->{-is_optional}
-        and
-      defined ( my $first_distinct_child_idcol = first
+    ) {
+
+      my ($first_distinct_child_idcol) = grep
         { ! $known_present_ids->{$_} }
         @{$relinfo->{-identifying_columns}}
-      )
-    ) {
+      ;
+
+      DBIx::Class::Exception->throw(
+        "An optional node *without* a distinct identifying set shouldn't be possible: " . dump_value $args->{collapse_map},
+        1,
+      ) unless defined $first_distinct_child_idcol;
 
       if ($args->{prune_null_branches}) {
 
diff --git a/t/resultset/inflate_result_api.t b/t/resultset/inflate_result_api.t
index 6f5b0a8..e09bad1 100644
--- a/t/resultset/inflate_result_api.t
+++ b/t/resultset/inflate_result_api.t
@@ -39,6 +39,7 @@ $schema->resultset('CD')->create({
       title => 'Oxygene',
       year => 1976,
       artist => { name => 'JMJ' },
+      artwork => {},
       tracks => [
         { title => 'o2', position => 2},  # the position should not be needed here, bug in MC
       ],
@@ -469,6 +470,49 @@ INFTYPE: for ('', '(native inflator)') {
     ],
     "Expected output of collapsing 1:M with empty root selection $native_inflator",
   );
+
+  cmp_structures (
+    rs_contents( $schema->resultset ('CD')->search_rs (
+      {
+        'tracks.title' => 'e2',
+        'cds.title' => 'Oxygene',
+      },
+      {
+        collapse => 1,
+        join => [
+          'tracks',
+          { single_track => { cd => 'mandatory_artwork' } },
+          { artist => { cds => 'mandatory_artwork'} },
+        ],
+        columns => {
+          cdid                                      => 'cdid',
+          'single_track.cd.mandatory_artwork.cd_id' => 'mandatory_artwork.cd_id',
+          'artist.cds.mandatory_artwork.cd_id'      => 'mandatory_artwork_2.cd_id',
+        },
+      },
+    )),
+    [[
+      { cdid => 3 },
+      {
+        single_track => [
+          undef,
+          { cd => [
+            undef,
+            { mandatory_artwork => [ { cd_id => 2 } ] }
+          ] }
+        ],
+        artist => [
+          undef,
+          { cds => [
+            [
+              undef,
+              { mandatory_artwork => [ { cd_id => 2 } ] }
+            ]
+          ] },
+        ],
+      }
+    ]],
+  );
 }
 
 sub null_branch {