From: Amiri Barksdale at Home <amiri@metalabel.com>
Date: Sun, 6 Jun 2010 03:10:20 +0000 (-0700)
Subject: Parser correct; test includes failure, shorter query tests; new test schema for bad... 
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=51b31bbe7d3359237fd2eb5e1f4aadb133578f25;p=dbsrgits%2FDBIx-Class-Historic.git

Parser correct; test includes failure, shorter query tests; new test schema for bad deps chain
---

diff --git a/lib/SQL/Translator/Parser/DBIx/Class.pm b/lib/SQL/Translator/Parser/DBIx/Class.pm
index d69e42c..93a39a0 100644
--- a/lib/SQL/Translator/Parser/DBIx/Class.pm
+++ b/lib/SQL/Translator/Parser/DBIx/Class.pm
@@ -17,6 +17,7 @@ use SQL::Translator::Utils qw(debug normalize_name);
 use Carp::Clan qw/^SQL::Translator|^DBIx::Class|^Try::Tiny/;
 use Scalar::Util 'weaken';
 use Try::Tiny;
+use Devel::Dwarn;
 use namespace::clean;
 
 use base qw(Exporter);
@@ -299,17 +300,26 @@ EOW
     }
 
     my %views;
+    my @views = map { $dbicschema->source($_) } keys %view_monikers;
+    my $view_dependencies;
 
-    my @view_sources =
-    sort {
-        keys %{ $a->deploy_depends_on || {} }
-        <=>
-        keys %{ $b->deploy_depends_on || {} }
-        ||
-        $a->source_name cmp $b->source_name
+    ### This is a loop instead of a map because
+    ### passing an object to the sub failed--gave
+    ### $view->name instead of the $view!
+
+    for my $view (@views) {
+        $view_dependencies->{ $view->name } =
+          _resolve_view_deps ( { view => $view }, \%view_monikers );
     }
-    map { $dbicschema->source($_) }
-    keys %view_monikers;
+
+    my @view_sources =
+      sort {
+        keys %{ $view_dependencies->{ $a->name }   || {} } <=>
+          keys %{ $view_dependencies->{ $b->name } || {} }
+          || $a->source_name cmp $b->source_name
+      }
+      map { $dbicschema->source($_) }
+      keys %view_monikers;
 
     foreach my $source (@view_sources)
     {
@@ -380,6 +390,33 @@ sub _resolve_deps {
   return $ret;
 }
 
+sub _resolve_view_deps {
+    my ( $view0, $monikers, $seen ) = @_;
+    my $view = $view0->{view};
+    my $ret  = {};
+    $seen ||= {};
+
+    # copy and bump all deps by one (so we can reconstruct the chain)
+    my %seen = map { $_ => $seen->{$_} + 1 } ( keys %$seen );
+    $seen{ $view->source_name } = 1;
+
+    for my $dep ( keys %{ $view->{deploy_depends_on} } ) {
+        if ( $seen->{$dep} ) {
+            return {};
+        }
+        my ($new_source_name) =
+          grep { $view->schema->source($_)->name eq $dep }
+          @{ [ $view->schema->sources ] };
+        my $subdeps = _resolve_view_deps(
+            { view => $view->schema->source($new_source_name) },
+            $monikers, \%seen, );
+        $ret->{$_} += $subdeps->{$_} for ( keys %$subdeps );
+
+        ++$ret->{$dep};
+    }
+    return $ret;
+}
+
 1;
 
 =head1 NAME
diff --git a/t/105view_deps.t b/t/105view_deps.t
index 223254d..66e409d 100644
--- a/t/105view_deps.t
+++ b/t/105view_deps.t
@@ -7,6 +7,7 @@ use Test::More;
 use Test::Exception;
 use lib qw(t/lib);
 use ViewDeps;
+use ViewDepsBad;
 
 BEGIN {
     use_ok('DBIx::Class::ResultSource::View');
@@ -14,7 +15,7 @@ BEGIN {
 
 #################### SANITY
 
-my $view = DBIx::Class::ResultSource::View->new( { name => 'Quux' } );
+my $view = DBIx::Class::ResultSource::View->new;
 
 isa_ok( $view, 'DBIx::Class::ResultSource', 'A new view' );
 isa_ok( $view, 'DBIx::Class', 'A new view also' );
@@ -27,38 +28,25 @@ my $schema
     = ViewDeps->connect( 'dbi:SQLite::memory:', { quote_char => '"', } );
 ok( $schema, 'Connected to ViewDeps schema OK' );
 
-my $deps_ref = {
-    map {
-        $schema->resultset($_)->result_source->name =>
-            $schema->resultset($_)->result_source->deploy_depends_on
-        }
-        grep {
-        $schema->resultset($_)
-            ->result_source->isa('DBIx::Class::ResultSource::View')
-        } @{ [ $schema->sources ] }
-};
-
-my @sorted_sources = sort {
-    keys %{ $deps_ref->{$a} || {} } <=> keys %{ $deps_ref->{$b} || {} }
-        || $a cmp $b
-    }
-    keys %$deps_ref;
-
 #################### DEPLOY
 
 $schema->deploy( { add_drop_table => 1 } );
 
 #################### DOES ORDERING WORK?
 
-my $tr = $schema->{sqlt};
+my $sqlt_object = $schema->{sqlt};
+
+my @keys = keys %{ $sqlt_object->{views} };
 
-my @keys = keys %{ $tr->{views} };
+my @sqlt_sources = sort {
+    $sqlt_object->{views}->{$a}->{order}
+        cmp $sqlt_object->{views}->{$b}->{order}
+} @keys;
 
-my @sqlt_sources
-    = sort { $tr->{views}->{$a}->{order} cmp $tr->{views}->{$b}->{order} }
-    @keys;
+my @expected
+    = qw/a_name_artists track_number_fives year_2010_cds ab_name_artists year_2010_cds_with_many_tracks aba_name_artists aba_name_artists_and_2010_cds_with_many_tracks/;
 
-is_deeply( \@sorted_sources, \@sqlt_sources,
+is_deeply( \@expected, \@sqlt_sources,
     "SQLT view order triumphantly matches our order." );
 
 #################### AND WHAT ABOUT USING THE SCHEMA?
@@ -69,4 +57,47 @@ lives_ok( sub { $schema->resultset($_)->next }, "Query on $_ succeeds" )
         ->result_source->isa('DBIx::Class::ResultSource::View')
     } @{ [ $schema->sources ] };
 
+#################### AND WHAT ABOUT A BAD DEPS CHAIN IN A VIEW?
+
+my $schema2
+    = ViewDepsBad->connect( 'dbi:SQLite::memory:', { quote_char => '"', } );
+ok( $schema2, 'Connected to ViewDepsBad schema OK' );
+
+#################### DEPLOY2
+
+$schema2->deploy( { add_drop_table => 1 } );
+
+#################### DOES ORDERING WORK 2?
+
+my $sqlt_object2 = $schema2->{sqlt};
+
+my @keys2 = keys %{ $sqlt_object->{views} };
+
+my @sqlt_sources2 = sort {
+    $sqlt_object->{views}->{$a}->{order}
+        cmp $sqlt_object->{views}->{$b}->{order}
+} @keys2;
+
+my @expected2
+    = qw/a_name_artists track_number_fives year_2010_cds ab_name_artists year_2010_cds_with_many_tracks aba_name_artists aba_name_artists_and_2010_cds_with_many_tracks/;
+
+is_deeply( \@expected2, \@sqlt_sources2,
+    "SQLT view order triumphantly matches our order." );
+
+#################### AND WHAT ABOUT USING THE SCHEMA2?
+
+lives_ok( sub { $schema2->resultset($_)->next }, "Query on $_ succeeds" )
+    for grep {
+    $schema2->resultset($_)
+        ->result_source->isa('DBIx::Class::ResultSource::View')
+    } grep { !/AbaNameArtistsAnd2010CDsWithManyTracks/ }
+    @{ [ $schema2->sources ] };
+
+dies_ok(
+    sub {
+        $schema2->resultset('AbaNameArtistsAnd2010CDsWithManyTracks')->next;
+    },
+    "Query on AbaNameArtistsAnd2010CDsWithManyTracks fails, because of incorrect deploy_depends_on in AbaNameArtists"
+);
+
 done_testing;
diff --git a/t/lib/ViewDepsBad.pm b/t/lib/ViewDepsBad.pm
new file mode 100644
index 0000000..9b5be12
--- /dev/null
+++ b/t/lib/ViewDepsBad.pm
@@ -0,0 +1,16 @@
+package    # hide from PAUSE
+    ViewDepsBad;
+## Used in 105view_deps.t
+
+use strict;
+use warnings;
+use base 'DBIx::Class::Schema';
+
+__PACKAGE__->load_namespaces;
+
+sub sqlt_deploy_hook {
+    my $self = shift;
+    $self->{sqlt} = shift;
+}
+
+1;
diff --git a/t/lib/ViewDepsBad/Result/ANameArtists.pm b/t/lib/ViewDepsBad/Result/ANameArtists.pm
new file mode 100644
index 0000000..8d16ae9
--- /dev/null
+++ b/t/lib/ViewDepsBad/Result/ANameArtists.pm
@@ -0,0 +1,25 @@
+package    # hide from PAUSE
+    ViewDepsBad::Result::ANameArtists;
+
+use strict;
+use warnings;
+use base 'DBIx::Class::Core';
+
+__PACKAGE__->table_class('DBIx::Class::ResultSource::View');
+__PACKAGE__->table('a_name_artists');
+__PACKAGE__->result_source_instance->view_definition(
+    "SELECT id,name FROM artist WHERE name like 'a%'"
+);
+
+__PACKAGE__->add_columns(
+    id   => { data_type => 'integer', is_auto_increment => 1 },
+    name => { data_type => 'text' },
+);
+
+__PACKAGE__->set_primary_key('id');
+
+__PACKAGE__->has_many( 'cds', 'ViewDeps::Result::CD',
+    { "foreign.artist" => "self.id" },
+);
+
+1;
diff --git a/t/lib/ViewDepsBad/Result/AbNameArtists.pm b/t/lib/ViewDepsBad/Result/AbNameArtists.pm
new file mode 100644
index 0000000..181d4ec
--- /dev/null
+++ b/t/lib/ViewDepsBad/Result/AbNameArtists.pm
@@ -0,0 +1,28 @@
+package    # hide from PAUSE
+    ViewDepsBad::Result::AbNameArtists;
+
+use strict;
+use warnings;
+use base 'DBIx::Class::Core';
+
+__PACKAGE__->table_class('DBIx::Class::ResultSource::View');
+__PACKAGE__->table('ab_name_artists');
+__PACKAGE__->result_source_instance->view_definition(
+    "SELECT id,name FROM a_name_artists WHERE name like 'ab%'"
+);
+__PACKAGE__->result_source_instance->deploy_depends_on(
+    ["ViewDepsBad::Result::ANameArtists"]
+);
+
+__PACKAGE__->add_columns(
+    id   => { data_type => 'integer', is_auto_increment => 1 },
+    name => { data_type => 'text' },
+);
+
+__PACKAGE__->set_primary_key('id');
+
+__PACKAGE__->has_many( 'cds', 'ViewDepsBad::Result::CD',
+    { "foreign.artist" => "self.id" },
+);
+
+1;
diff --git a/t/lib/ViewDepsBad/Result/AbaNameArtists.pm b/t/lib/ViewDepsBad/Result/AbaNameArtists.pm
new file mode 100644
index 0000000..715d56d
--- /dev/null
+++ b/t/lib/ViewDepsBad/Result/AbaNameArtists.pm
@@ -0,0 +1,26 @@
+package    # hide from PAUSE
+    ViewDepsBad::Result::AbaNameArtists;
+
+use strict;
+use warnings;
+use base 'DBIx::Class::Core';
+
+__PACKAGE__->table_class('DBIx::Class::ResultSource::View');
+__PACKAGE__->table('aba_name_artists');
+__PACKAGE__->result_source_instance->view_definition(
+    "SELECT id,name FROM ab_name_artists WHERE name like 'aba%'" );
+__PACKAGE__->result_source_instance->deploy_depends_on(
+    ["ViewDepsBad::Result::AbNameArtists", "ViewDepsBad::Result::AbaNameArtistsAnd2010CDsWithManyTracks"] );
+
+__PACKAGE__->add_columns(
+    id   => { data_type => 'integer', is_auto_increment => 1 },
+    name => { data_type => 'text' },
+);
+
+__PACKAGE__->set_primary_key('id');
+
+__PACKAGE__->has_many( 'cds', 'ViewDepsBad::Result::CD',
+    { "foreign.artist" => "self.id" },
+);
+
+1;
diff --git a/t/lib/ViewDepsBad/Result/AbaNameArtists.pm~ b/t/lib/ViewDepsBad/Result/AbaNameArtists.pm~
new file mode 100644
index 0000000..fc989f6
--- /dev/null
+++ b/t/lib/ViewDepsBad/Result/AbaNameArtists.pm~
@@ -0,0 +1,26 @@
+package    # hide from PAUSE
+    ViewDeps::Result::AbaNameArtists;
+
+use strict;
+use warnings;
+use base 'DBIx::Class::Core';
+
+__PACKAGE__->table_class('DBIx::Class::ResultSource::View');
+__PACKAGE__->table('aba_name_artists');
+__PACKAGE__->result_source_instance->view_definition(
+    "SELECT id,name FROM ab_name_artists WHERE name like 'aba%'" );
+__PACKAGE__->result_source_instance->deploy_depends_on(
+    ["ViewDeps::Result::AbNameArtists"] );
+
+__PACKAGE__->add_columns(
+    id   => { data_type => 'integer', is_auto_increment => 1 },
+    name => { data_type => 'text' },
+);
+
+__PACKAGE__->set_primary_key('id');
+
+__PACKAGE__->has_many( 'cds', 'ViewDeps::Result::CD',
+    { "foreign.artist" => "self.id" },
+);
+
+1;
diff --git a/t/lib/ViewDepsBad/Result/AbaNameArtistsAnd2010CDsWithManyTracks.pm b/t/lib/ViewDepsBad/Result/AbaNameArtistsAnd2010CDsWithManyTracks.pm
new file mode 100644
index 0000000..8751d57
--- /dev/null
+++ b/t/lib/ViewDepsBad/Result/AbaNameArtistsAnd2010CDsWithManyTracks.pm
@@ -0,0 +1,26 @@
+package    # hide from PAUSE
+    ViewDepsBad::Result::AbaNameArtistsAnd2010CDsWithManyTracks;
+
+use strict;
+use warnings;
+use base 'DBIx::Class::Core';
+
+__PACKAGE__->table_class('DBIx::Class::ResultSource::View');
+__PACKAGE__->table('aba_name_artists_and_2010_cds_with_many_tracks');
+__PACKAGE__->result_source_instance->view_definition(
+    "SELECT aba.id,aba.name,cd.title,cd.year,cd.number_tracks FROM aba_name_artists aba JOIN year_2010_cds_with_many_tracks cd on (aba.id = cd.artist)"
+);
+__PACKAGE__->result_source_instance->deploy_depends_on(
+    ["ViewDepsBad::Result::AbNameArtists","ViewDepsBad::Result::Year2010CDsWithManyTracks"] );
+
+__PACKAGE__->add_columns(
+    id            => { data_type => 'integer', is_auto_increment => 1 },
+    name          => { data_type => 'text' },
+    title         => { data_type => 'text' },
+    year          => { data_type => 'integer' },
+    number_tracks => { data_type => 'integer' },
+);
+
+__PACKAGE__->set_primary_key('id');
+
+1;
diff --git a/t/lib/ViewDepsBad/Result/Artist.pm b/t/lib/ViewDepsBad/Result/Artist.pm
new file mode 100644
index 0000000..6d7a0f5
--- /dev/null
+++ b/t/lib/ViewDepsBad/Result/Artist.pm
@@ -0,0 +1,21 @@
+package    # hide from PAUSE
+    ViewDepsBad::Result::Artist;
+
+use strict;
+use warnings;
+use base 'DBIx::Class::Core';
+
+__PACKAGE__->table('artist');
+
+__PACKAGE__->add_columns(
+    id   => { data_type => 'integer', is_auto_increment => 1 },
+    name => { data_type => 'text' },
+);
+
+__PACKAGE__->set_primary_key('id');
+
+__PACKAGE__->has_many( 'cds', 'ViewDepsBad::Result::CD',
+    { "foreign.artist" => "self.id" },
+);
+
+1;
diff --git a/t/lib/ViewDepsBad/Result/Artwork.pm b/t/lib/ViewDepsBad/Result/Artwork.pm
new file mode 100644
index 0000000..978e196
--- /dev/null
+++ b/t/lib/ViewDepsBad/Result/Artwork.pm
@@ -0,0 +1,22 @@
+package    # hide from PAUSE
+    ViewDepsBad::Result::Artwork;
+
+use strict;
+use warnings;
+use base 'DBIx::Class::Core';
+
+__PACKAGE__->table('artwork');
+
+__PACKAGE__->add_columns(
+    id            => { data_type => 'integer', is_auto_increment => 1 },
+    cd         => { data_type => 'integer' },
+    file          => { data_type => 'text' },
+);
+
+__PACKAGE__->set_primary_key('id');
+
+__PACKAGE__->belongs_to( 'cd', 'ViewDepsBad::Result::CD',
+    { "foreign.id" => "self.cd" },
+);
+
+1;
diff --git a/t/lib/ViewDepsBad/Result/CD.pm b/t/lib/ViewDepsBad/Result/CD.pm
new file mode 100644
index 0000000..ea40b84
--- /dev/null
+++ b/t/lib/ViewDepsBad/Result/CD.pm
@@ -0,0 +1,28 @@
+package    # hide from PAUSE
+    ViewDepsBad::Result::CD;
+
+use strict;
+use warnings;
+use base 'DBIx::Class::Core';
+
+__PACKAGE__->table('cd');
+
+__PACKAGE__->add_columns(
+    id            => { data_type => 'integer', is_auto_increment => 1 },
+    title         => { data_type => 'text' },
+    artist        => { data_type => 'integer', is_nullable       => 0 },
+    year          => { data_type => 'integer' },
+    number_tracks => { data_type => 'integer' },
+);
+
+__PACKAGE__->set_primary_key('id');
+
+__PACKAGE__->belongs_to( 'artist', 'ViewDepsBad::Result::Artist',
+    { "foreign.id" => "self.artist" },
+);
+
+__PACKAGE__->has_many( 'tracks', 'ViewDepsBad::Result::Track',
+    { "foreign.cd" => "self.id" },
+);
+
+1;
diff --git a/t/lib/ViewDepsBad/Result/Track.pm b/t/lib/ViewDepsBad/Result/Track.pm
new file mode 100644
index 0000000..0ff97f2
--- /dev/null
+++ b/t/lib/ViewDepsBad/Result/Track.pm
@@ -0,0 +1,23 @@
+package    # hide from PAUSE
+    ViewDepsBad::Result::Track;
+
+use strict;
+use warnings;
+use base 'DBIx::Class::Core';
+
+__PACKAGE__->table('track');
+
+__PACKAGE__->add_columns(
+    id           => { data_type => 'integer', is_auto_increment => 1 },
+    title        => { data_type => 'text' },
+    cd           => { data_type => 'integer' },
+    track_number => { data_type => 'integer' },
+);
+
+__PACKAGE__->set_primary_key('id');
+
+__PACKAGE__->belongs_to( 'cd', 'ViewDepsBad::Result::CD',
+    { "foreign.id" => "self.cd" },
+);
+
+1;
diff --git a/t/lib/ViewDepsBad/Result/TrackNumberFives.pm b/t/lib/ViewDepsBad/Result/TrackNumberFives.pm
new file mode 100644
index 0000000..ce09b80
--- /dev/null
+++ b/t/lib/ViewDepsBad/Result/TrackNumberFives.pm
@@ -0,0 +1,26 @@
+package    # hide from PAUSE
+    ViewDepsBad::Result::TrackNumberFives;
+
+use strict;
+use warnings;
+use base 'ViewDepsBad::Result::Track';
+
+__PACKAGE__->table_class('DBIx::Class::ResultSource::View');
+__PACKAGE__->table('track_number_fives');
+__PACKAGE__->result_source_instance->view_definition(
+    "SELECT id,title,cd,track_number FROM track WHERE track_number = '5'");
+
+__PACKAGE__->add_columns(
+    id           => { data_type => 'integer', is_auto_increment => 1 },
+    title        => { data_type => 'text' },
+    cd           => { data_type => 'integer' },
+    track_number => { data_type => 'integer' },
+);
+
+__PACKAGE__->set_primary_key('id');
+
+__PACKAGE__->belongs_to( 'cd', 'ViewDepsBad::Result::CD',
+    { "foreign.id" => "self.cd" },
+);
+
+1;
diff --git a/t/lib/ViewDepsBad/Result/Year2010CDs.pm b/t/lib/ViewDepsBad/Result/Year2010CDs.pm
new file mode 100644
index 0000000..8771ad9
--- /dev/null
+++ b/t/lib/ViewDepsBad/Result/Year2010CDs.pm
@@ -0,0 +1,31 @@
+package    # hide from PAUSE
+    ViewDepsBad::Result::Year2010CDs;
+
+use strict;
+use warnings;
+use base 'DBIx::Class::Core';
+
+__PACKAGE__->table_class('DBIx::Class::ResultSource::View');
+__PACKAGE__->table('year_2010_cds');
+__PACKAGE__->result_source_instance->view_definition(
+    "SELECT id,title,artist,year,number_tracks FROM cd WHERE year = '2010'");
+
+__PACKAGE__->add_columns(
+    id            => { data_type => 'integer', is_auto_increment => 1 },
+    title         => { data_type => 'text' },
+    artist        => { data_type => 'integer', is_nullable       => 0 },
+    year          => { data_type => 'integer' },
+    number_tracks => { data_type => 'integer' },
+);
+
+__PACKAGE__->set_primary_key('id');
+
+__PACKAGE__->belongs_to( 'artist', 'ViewDepsBad::Result::Artist',
+    { "foreign.id" => "self.artist" },
+);
+
+__PACKAGE__->has_many( 'tracks', 'ViewDepsBad::Result::Track',
+    { "foreign.cd" => "self.id" },
+);
+
+1;
diff --git a/t/lib/ViewDepsBad/Result/Year2010CDsWithManyTracks.pm b/t/lib/ViewDepsBad/Result/Year2010CDsWithManyTracks.pm
new file mode 100644
index 0000000..9a4900f
--- /dev/null
+++ b/t/lib/ViewDepsBad/Result/Year2010CDsWithManyTracks.pm
@@ -0,0 +1,36 @@
+package    # hide from PAUSE
+    ViewDepsBad::Result::Year2010CDsWithManyTracks;
+
+use strict;
+use warnings;
+use base 'ViewDepsBad::Result::Year2010CDs';
+
+__PACKAGE__->table_class('DBIx::Class::ResultSource::View');
+__PACKAGE__->table('year_2010_cds_with_many_tracks');
+__PACKAGE__->result_source_instance->view_definition(
+    "SELECT cd.id,cd.title,cd.artist,cd.year,cd.number_tracks,art.file FROM year_2010_cds cd JOIN artwork art on art.cd = cd.id WHERE cd.number_tracks > 10"
+);
+
+__PACKAGE__->result_source_instance->deploy_depends_on(
+    ["ViewDepsBad::Result::Year2010CDs"] );
+
+__PACKAGE__->add_columns(
+    id            => { data_type => 'integer', is_auto_increment => 1 },
+    title         => { data_type => 'text' },
+    artist        => { data_type => 'integer', is_nullable       => 0 },
+    year          => { data_type => 'integer' },
+    number_tracks => { data_type => 'integer' },
+    file       => { data_type => 'integer' },
+);
+
+__PACKAGE__->set_primary_key('id');
+
+__PACKAGE__->belongs_to( 'artist', 'ViewDepsBad::Result::Artist',
+    { "foreign.id" => "self.artist" },
+);
+
+__PACKAGE__->has_many( 'tracks', 'ViewDepsBad::Result::Track',
+    { "foreign.cd" => "self.id" },
+);
+
+1;