From: Amiri Barksdale at Home 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;