From: Peter Rabbitson Date: Sun, 10 Mar 2013 11:10:06 +0000 (+0100) Subject: Merge branch 'master' into topic/constructor_rewrite X-Git-Tag: v0.08242~19 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=459669a408682896779b908bc29d6264af8702e3;hp=7cd98754bc5a7287b44cc9638420bd03392e41cb;p=dbsrgits%2FDBIx-Class.git Merge branch 'master' into topic/constructor_rewrite --- diff --git a/.mailmap b/.mailmap index 02a82d5..25a41fa 100644 --- a/.mailmap +++ b/.mailmap @@ -12,6 +12,7 @@ Brendan Byrd Brian Phillips David Kamholz David Schmidt +David Schmidt Devin Austin Felix Antonius Wilhelm Ostmann Gerda Shank diff --git a/.travis.yml b/.travis.yml index fcf7acc..f51eeac 100644 --- a/.travis.yml +++ b/.travis.yml @@ -30,19 +30,15 @@ # # -# Smoke only specific branches to a) not overload the queue and b) not -# overspam the notification channels +# Smoke all branches except for blocked* and wip/* # -# Furthermore if the branch is ^topic/ - the custom compiled smokes will -# not run at all, again in order to conserve queue resources -# -# Additionally bleadperl tests do not run on master (but do run on smoke/*) +# Additionally master does not smoke with bleadperl +# ( implemented in maint/travis-ci_scripts/10_before_install.bash ) # branches: - only: - - master - - /^smoke\// - - /^topic\// + except: + - /^wip\// + - /^blocked/ notifications: irc: diff --git a/CONTRIBUTING b/CONTRIBUTING deleted file mode 100644 index 586ec4c..0000000 --- a/CONTRIBUTING +++ /dev/null @@ -1,19 +0,0 @@ -With DBIx::Class on git now, this is the process for contributing. - -1) Clone the DBIx::Class master branch - > git clone dbsrgits@git.shadowcat.co.uk:DBIx-Class.git -2) Do your work on your machine. This is git - everything is local! -3) When you think you're ready, push it back out to the origin as a - remote branch. See rebasing for what you should do before pushing. - > git push origin my-branch -4) Notify the other contributors that you're ready to have your branch - reviewed. -5) Another contributor will merge it back into master. See rebasing for - what you should do before merging. - -Rebasing: Please rebase before merging and pushing; we'd rather not have -commit messages that say, "Oops" and "typo", in master, and furthermore -fast-forward merges lead to a cleaner history. - -Yes, this does mean that DBIx::Class is moving to a formal code review process. -Yes, this does mean that you will never merge your own code to master. diff --git a/Changes b/Changes index 877fb10..c010a79 100644 --- a/Changes +++ b/Changes @@ -1,5 +1,33 @@ Revision history for DBIx::Class +0.08209 2013-03-01 12:56 (UTC) + * New Features / Changes + - Debugging aid - warn on invalid result objects created by what + seems like an invalid inheritance hierarchy + + * Fixes + - Fix another embarrassing regression preventing correct refining of + the search criteria on a prefetched relation (broken in 0.08205) + - Fix incorrect callsite reporting by DBIC::Carp + +0.08208 2013-02-20 09:56 (UTC) + * New Features / Changes + - A bunch of nonsensically named arguments to the SQL::Translator + parser have been marked as deprecated (while still fully + supported) + + * Fixes + - Fix duplicated selected columns when calling 'count' when a same + aggregate function is used more than once in a 'having' clause + (RT#83305) + - Prevent SQL::Translator::Producer::YAML from seeing the $dbh + in a potentially connected $schema instance (RT#75394) + + * Misc + - Fixup our distbuilding process to stop creating world-writable + tarball contents (implicitly fixes RT#83084) + - Added strict and warnings tests for all lib and test files + 0.08241-TRIAL (EXPERIMENTAL BETA RELEASE) 2013-02-20 11:97 (UTC) * New Features / Changes - Revert to passing the original (pre-0.08240) arguments to diff --git a/lib/DBIx/Class.pm b/lib/DBIx/Class.pm index adfcea4..c3be350 100644 --- a/lib/DBIx/Class.pm +++ b/lib/DBIx/Class.pm @@ -99,43 +99,67 @@ sub _attr_cache { DBIx::Class - Extensible and flexible object <-> relational mapper. -=head1 GETTING HELP/SUPPORT +=head1 WHERE TO START READING -The community can be found via: +See L for an overview of the exhaustive documentation. +To get the most out of DBIx::Class with the least confusion it is strongly +recommended to read (at the very least) the +L in the order presented there. + +=head1 HOW TO GET HELP + +Due to the complexity of its problem domain, DBIx::Class is a relatively +complex framework. After you start using DBIx::Class questions will inevitably +arise. If you are stuck with a problem or have doubts about a particular +approach do not hesitate to contact the community with your questions. The +list below is sorted by "fastest response time": =over =item * IRC: irc.perl.org#dbix-class =for html -(click for instant chatroom login) +(click for instant chatroom login) =item * Mailing list: L -=item * Twitter L +=item * RT Bug Tracker: L -=item * Web Site: L +=item * Twitter: L -=item * RT Bug Tracker: L +=item * Web Site: L =back -The project is maintained in a git repository, accessible from the following sources: +=head1 HOW TO CONTRIBUTE + +Contributions are always welcome, in all usable forms (we especially +welcome documentation improvements). The delivery methods include git- +or unified-diff formatted patches, GitHub pull requests, or plain bug +reports either via RT or the Mailing list. Contributors are generally +granted full access to the official repository after their first patch +passes successful review. + +=for comment +FIXME: Getty, frew and jnap need to get off their asses and finish the contrib section so we can link it here ;) + +This project is maintained in a git repository. The code and related tools are +accessible at the following locations: =over -=item * git: L +=item * Official repo: L -=item * gitweb: L +=item * Official gitweb: L -=item * github mirror: L +=item * GitHub mirror: L -=item * authorized committers: L +=item * Authorized committers: L -=item * Travis-CI log: L +=item * Travis-CI log: L =for html - +
↪ Stable branch CI status: =back @@ -273,11 +297,6 @@ The test suite is quite substantial, and several developer releases are generally made to CPAN before the branch for the next release is merged back to trunk for a major release. -=head1 WHERE TO GO NEXT - -L lists each task you might want help on, and -the modules where you will find documentation. - =head1 AUTHOR mst: Matt S. Trout @@ -343,6 +362,8 @@ clkao: CL Kao da5id: David Jack Olrik +davewood: David Schmidt + debolaz: Anders Nor Berle dew: Dan Thomas @@ -421,6 +442,8 @@ michaelr: Michael Reddick milki: Jonathan Chu +mithaldu: Christian Walde + mjemmeson: Michael Jemmeson mstratman: Mark A. Stratman diff --git a/lib/DBIx/Class/Admin/Types.pm b/lib/DBIx/Class/Admin/Types.pm index 23af292..c6f37c6 100644 --- a/lib/DBIx/Class/Admin/Types.pm +++ b/lib/DBIx/Class/Admin/Types.pm @@ -1,6 +1,10 @@ package # hide from PAUSE DBIx::Class::Admin::Types; +# Workaround for https://rt.cpan.org/Public/Bug/Display.html?id=83336 +use warnings; +use strict; + use MooseX::Types -declare => [qw( DBICConnectInfo DBICArrayRef diff --git a/lib/DBIx/Class/Carp.pm b/lib/DBIx/Class/Carp.pm index 443e6ca..d27df5d 100644 --- a/lib/DBIx/Class/Carp.pm +++ b/lib/DBIx/Class/Carp.pm @@ -30,8 +30,6 @@ sub __find_caller { my $fr_num = 1; # skip us and the calling carp* my @f; while (@f = caller($fr_num++)) { - last unless $f[0] =~ $skip_pattern; - if ( $f[0]->can('_skip_namespace_frames') and @@ -39,6 +37,8 @@ sub __find_caller { ) { $skip_pattern = qr/$skip_pattern|$extra_skip/; } + + last if $f[0] !~ $skip_pattern; } my ($ln, $calling) = @f # if empty - nothing matched - full stack @@ -69,8 +69,8 @@ sub import { my $into = caller; $skip_pattern = $skip_pattern - ? qr/ ^ $into $ | $skip_pattern /xo - : qr/ ^ $into $ /xo + ? qr/ ^ $into $ | $skip_pattern /x + : qr/ ^ $into $ /x ; no strict 'refs'; diff --git a/lib/DBIx/Class/Manual/Cookbook.pod b/lib/DBIx/Class/Manual/Cookbook.pod index 328c891..21d720e 100644 --- a/lib/DBIx/Class/Manual/Cookbook.pod +++ b/lib/DBIx/Class/Manual/Cookbook.pod @@ -604,7 +604,7 @@ C: # SELECT cd.*, artist.*, liner_notes.* FROM cd # JOIN artist ON cd.artist = artist.id # JOIN liner_notes ON cd.id = liner_notes.cd - # WHERE artist.name = 'Bob Marley' + # WHERE artist.name = 'Bob Marley' AND liner_notes.notes LIKE '%some text%' # ORDER BY artist.name =head2 Multi-step joins diff --git a/lib/DBIx/Class/Manual/DocMap.pod b/lib/DBIx/Class/Manual/DocMap.pod index c9bd1d8..83c9633 100644 --- a/lib/DBIx/Class/Manual/DocMap.pod +++ b/lib/DBIx/Class/Manual/DocMap.pod @@ -12,7 +12,7 @@ DBIx::Class::Manual::DocMap - What documentation do we have? =item L - Full example Schema. -=item L - How to use DBIx::Class if you know SQL (external, available on CPAN) +=item L - How to use DBIx::Class if you know SQL (external, available on CPAN) =item L - What do all those terms mean? @@ -22,50 +22,26 @@ DBIx::Class::Manual::DocMap - What documentation do we have? =item L - What to do if things go wrong (diagnostics of known error messages). -=item L - How to write your own DBIx::Class components. - =back -=head1 Setting up +=head1 Some essential reference documentation =over 4 -=item L - Overall schemas, and connection container. - -=item L - Source/Table definition functions. - -=item L - Simple relationships. - -=item L - Relationship details. - -=item L - Magically retrieve auto-incrementing fields. - -=item L - Set of standard components to load. - -=item L - Making objects out of your columns. - -=item L - Magically turn your datetime or timestamp columns into DateTime objects. +=item L - Representing a single result (row) from a DB query -=item L - Dealing with primary keys. - -=item L - Turns the resultsource into a table. - -=item L - Accessor grouping. - -=back - -=head1 Retrieving and creating data +=item L - Selecting and manipulating sets. -=over 4 +=item L - Perform operations on a single column of a ResultSet. -=item L - Selecting and manipulating sets. +=item L - Source/Table definition functions. -=item L - Perform operations on entire columns of a ResultSet. +=item L - Overall sourcess, and connection container. -=item L - Dealing with actual data. +=item L - Simple relationship declarations. -=item L - Basic Storage stuff. +=item L - Relationship declaration details. -=item L - Storage using L and L. +=item L - Making objects out of your column values. =back diff --git a/lib/DBIx/Class/Optional/Dependencies.pm b/lib/DBIx/Class/Optional/Dependencies.pm index faba747..6af4221 100644 --- a/lib/DBIx/Class/Optional/Dependencies.pm +++ b/lib/DBIx/Class/Optional/Dependencies.pm @@ -106,19 +106,6 @@ my $rdbms_firebird_odbc = { }; my $reqs = { - dist_upload => { - req => { - 'CPAN::Uploader' => '0.103001', - }, - }, - - dist_podinherit => { - req => { - 'Pod::Inherit' => '0.90', - 'Pod::Tree' => '0', - } - }, - replicated => { req => $replicated, pod => { @@ -197,7 +184,7 @@ my $reqs = { test_strictures => { req => { - 'Test::Strict' => '0.16', + 'Test::Strict' => '0.20', }, }, @@ -259,7 +246,6 @@ my $reqs = { test_cdbicompat => { req => { - 'Class::DBI' => 0, 'Class::DBI::Plugin::DeepAbstractSearch' => '0', %$datetime_basic, 'Time::Piece::MySQL' => '0', @@ -623,8 +609,23 @@ my $reqs = { }, }, + dist_dir => { + req => { + 'ExtUtils::MakeMaker' => '6.64', + 'Pod::Inherit' => '0.90', + 'Pod::Tree' => '0', + } + }, + + dist_upload => { + req => { + 'CPAN::Uploader' => '0.103001', + }, + }, + }; +our %req_availability_cache; sub req_list_for { my ($class, $group) = @_; @@ -639,7 +640,16 @@ sub req_list_for { } -our %req_availability_cache; +sub die_unless_req_ok_for { + my ($class, $group) = @_; + + Carp::croak "die_unless_req_ok_for() expects a requirement group name" + unless $group; + + $class->_check_deps($group)->{status} + or die sprintf( "Required modules missing, unable to continue: %s\n", $class->_check_deps($group)->{missing} ); +} + sub req_ok_for { my ($class, $group) = @_; @@ -871,6 +881,16 @@ The author is expected to prepend the necessary text to this message before returning the actual error seen by the user. EOD + '=head2 die_unless_req_ok_for', + '=over', + '=item Arguments: $group_name', + '=back', + <<'EOD', +Checks if L passes for the supplied C<$group_name>, and +in case of failure throws an exception including the information +from L. +EOD + '=head2 req_errorlist_for', '=over', '=item Arguments: $group_name', diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm index 5dc6454..c3156ab 100644 --- a/lib/DBIx/Class/ResultSet.pm +++ b/lib/DBIx/Class/ResultSet.pm @@ -5,7 +5,7 @@ use warnings; use base qw/DBIx::Class/; use DBIx::Class::Carp; use DBIx::Class::ResultSetColumn; -use Scalar::Util qw/blessed weaken/; +use Scalar::Util qw/blessed weaken reftype/; use Try::Tiny; use Data::Compare (); # no imports!!! guard against insane architecture @@ -25,6 +25,10 @@ use overload 'bool' => "_bool", fallback => 1; +# this is real - CDBICompat overrides it with insanity +# yes, prototype won't matter, but that's for now ;) +sub _bool () { 1 } + __PACKAGE__->mk_group_accessors('simple' => qw/_result_class result_source/); =head1 NAME @@ -393,11 +397,11 @@ sub search_rs { my $cache; my %safe = (alias => 1, cache => 1); if ( ! List::Util::first { !$safe{$_} } keys %$call_attrs and ( - ! defined $_[0] + ! defined $call_cond or - ref $_[0] eq 'HASH' && ! keys %{$_[0]} + ref $call_cond eq 'HASH' && ! keys %$call_cond or - ref $_[0] eq 'ARRAY' && ! @{$_[0]} + ref $call_cond eq 'ARRAY' && ! @$call_cond )) { $cache = $self->get_cache; } @@ -1682,9 +1686,6 @@ sub _count_subq_rs { ->get_column ('count'); } -sub _bool { - return 1; -} =head2 count_literal @@ -2361,15 +2362,29 @@ sub new_result { my ($merged_cond, $cols_from_relations) = $self->_merge_with_rscond($values); - my %new = ( + my $new = $self->result_class->new({ %$merged_cond, - @$cols_from_relations + ( @$cols_from_relations ? (-cols_from_relations => $cols_from_relations) - : (), + : () + ), -result_source => $self->result_source, # DO NOT REMOVE THIS, REQUIRED - ); + }); + + if ( + reftype($new) eq 'HASH' + and + ! keys %$new + and + blessed($new) + ) { + carp_unique (sprintf ( + "%s->new returned a blessed empty hashref - a strong indicator something is wrong with its inheritance chain", + $self->result_class, + )); + } - return $self->result_class->new(\%new); + $new; } # _merge_with_rscond diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm index daef684..8d4ff35 100644 --- a/lib/DBIx/Class/Storage/DBI.pm +++ b/lib/DBIx/Class/Storage/DBI.pm @@ -1178,7 +1178,9 @@ sub _describe_connection { SQL_TXN_ISOLATION_OPTION / ) { - my $v = $self->_dbh_get_info($inf); + # some drivers barf on things they do not know about instead + # of returning undef + my $v = try { $self->_dbh_get_info($inf) }; next unless defined $v; #my $key = sprintf( '%s(%s)', $inf, $DBI::Const::GetInfoType::GetInfoType{$inf} ); diff --git a/lib/DBIx/Class/Storage/DBI/Replicated/Types.pm b/lib/DBIx/Class/Storage/DBI/Replicated/Types.pm index 4e75aa2..fa926a9 100644 --- a/lib/DBIx/Class/Storage/DBI/Replicated/Types.pm +++ b/lib/DBIx/Class/Storage/DBI/Replicated/Types.pm @@ -4,6 +4,10 @@ package # hide from PAUSE # DBIx::Class::Storage::DBI::Replicated::Types - Types used internally by # L +# Workaround for https://rt.cpan.org/Public/Bug/Display.html?id=83336 +use warnings; +use strict; + use MooseX::Types -declare => [qw/BalancerClassNamePart Weight DBICSchema DBICStorageDBI/]; use MooseX::Types::Moose qw/ClassName Str Num/; diff --git a/lib/SQL/Translator/Parser/DBIx/Class.pm b/lib/SQL/Translator/Parser/DBIx/Class.pm index dc13790..527d5e5 100644 --- a/lib/SQL/Translator/Parser/DBIx/Class.pm +++ b/lib/SQL/Translator/Parser/DBIx/Class.pm @@ -36,16 +36,40 @@ use base qw(Exporter); sub parse { my ($tr, $data) = @_; my $args = $tr->parser_args; - my $dbicschema = $args->{'DBIx::Class::Schema'} || $args->{"DBIx::Schema"} ||$data; - $dbicschema ||= $args->{'package'}; - my $limit_sources = $args->{'sources'}; + + my $dbicschema = $data || $args->{dbic_schema}; + + for (qw(DBIx::Class::Schema DBIx::Schema package)) { + if (defined (my $s = delete $args->{$_} )) { + carp_unique("Supplying a schema via ... parser_args => { '$_' => \$schema } is deprecated. Please use parser_args => { dbic_schema => \$schema } instead"); + + # move it from the deprecated to the proper $args slot + unless ($dbicschema) { + $args->{dbic_schema} = $dbicschema = $s; + } + } + } DBIx::Class::Exception->throw('No DBIx::Class::Schema') unless ($dbicschema); + if (!ref $dbicschema) { eval "require $dbicschema" or DBIx::Class::Exception->throw("Can't load $dbicschema: $@"); } + if ( + ref $args->{dbic_schema} + and + $args->{dbic_schema}->storage + ) { + # we have a storage-holding $schema instance in $args + # we need to dissociate it from that $storage + # otherwise SQLT insanity may ensue due to how some + # serializing producers treat $args (crazy crazy shit) + local $args->{dbic_schema}{storage}; + $args->{dbic_schema} = $args->{dbic_schema}->clone; + } + my $schema = $tr->schema; my $table_no = 0; @@ -53,7 +77,7 @@ sub parse { unless ($schema->name); my @monikers = sort $dbicschema->sources; - if ($limit_sources) { + if (my $limit_sources = $args->{'sources'}) { my $ref = ref $limit_sources || ''; $dbicschema->throw_exception ("'sources' parameter must be an array or hash ref") unless( $ref eq 'ARRAY' || ref eq 'HASH' ); @@ -422,7 +446,7 @@ from a DBIx::Class::Schema instance my $trans = SQL::Translator->new ( parser => 'SQL::Translator::Parser::DBIx::Class', parser_args => { - package => $schema, + dbic_schema => $schema, add_fk_index => 0, sources => [qw/ Artist @@ -452,6 +476,27 @@ L. =head1 PARSER OPTIONS +=head2 dbic_schema + +The DBIx::Class schema (either an instance or a class name) to be parsed. +This argument is in fact optional - instead one can supply it later at +translation time as an argument to L. In +other words both of the following invocations are valid and will produce +conceptually identical output: + + my $yaml = SQL::Translator->new( + parser => 'SQL::Translator::Parser::DBIx::Class', + parser_args => { + dbic_schema => $schema, + }, + producer => 'SQL::Translator::Producer::YAML', + )->translate; + + my $yaml = SQL::Translator->new( + parser => 'SQL::Translator::Parser::DBIx::Class', + producer => 'SQL::Translator::Producer::YAML', + )->translate(data => $schema); + =head2 add_fk_index Create an index for each foreign key. diff --git a/maint/Makefile.PL.inc/12_authordeps.pl b/maint/Makefile.PL.inc/12_authordeps.pl index 7068fcb..3a8a2b7 100644 --- a/maint/Makefile.PL.inc/12_authordeps.pl +++ b/maint/Makefile.PL.inc/12_authordeps.pl @@ -36,7 +36,7 @@ EOW # exclude the rdbms_* groups which are for DBIC users $opt_testdeps = { - map { %{$reqs_for_group{$_}} } grep { !/^rdbms_/ } keys %reqs_for_group + map { %{$reqs_for_group{$_}} } grep { !/^rdbms_|^dist_/ } keys %reqs_for_group }; print "Including all optional deps\n"; diff --git a/maint/Makefile.PL.inc/50_redefine_makefile_flow.pl b/maint/Makefile.PL.inc/50_redefine_makefile_flow.pl index 3813e80..45a5218 100644 --- a/maint/Makefile.PL.inc/50_redefine_makefile_flow.pl +++ b/maint/Makefile.PL.inc/50_redefine_makefile_flow.pl @@ -7,7 +7,7 @@ return <<"EOM"; $snippet -create_distdir : clonedir_generate_files clonedir_post_generate_files fresh_manifest create_distdir_copy_manifested clonedir_cleanup_generated_files +create_distdir : check_create_distdir_prereqs clonedir_generate_files clonedir_post_generate_files fresh_manifest create_distdir_copy_manifested clonedir_cleanup_generated_files \t\$(NOECHO) \$(NOOP) clonedir_generate_files : @@ -19,6 +19,32 @@ clonedir_post_generate_files : clonedir_cleanup_generated_files : \t\$(NOECHO) \$(NOOP) +check_create_distdir_prereqs : +\t\$(NOECHO) @{[ + $mm_proto->oneliner("DBIx::Class::Optional::Dependencies->die_unless_req_ok_for(q(dist_dir))", [qw/-Ilib -MDBIx::Class::Optional::Dependencies/]) +]} + +EOM + } +} + +# M::I inserts its own default postamble, so we can't easily override upload +# but we can still hook postamble in EU::MM +{ + package MY; + + sub postamble { + my $snippet = shift->SUPER::postamble(@_); + return <<"EOM"; +$snippet + +upload :: check_create_distdir_prereqs check_upload_dist_prereqs + +check_upload_dist_prereqs : +\t\$(NOECHO) @{[ + $mm_proto->oneliner("DBIx::Class::Optional::Dependencies->die_unless_req_ok_for(q(dist_upload))", [qw/-Ilib -MDBIx::Class::Optional::Dependencies/]) +]} + EOM } } diff --git a/maint/Makefile.PL.inc/51_autohandle_MANIFEST.pl b/maint/Makefile.PL.inc/51_autohandle_MANIFEST.pl index f12ee30..938cab5 100644 --- a/maint/Makefile.PL.inc/51_autohandle_MANIFEST.pl +++ b/maint/Makefile.PL.inc/51_autohandle_MANIFEST.pl @@ -10,6 +10,13 @@ remove_manifest : realclean :: remove_manifest +manifest : check_manifest_is_lone_target + +check_manifest_is_lone_target : +\t\$(NOECHO) @{[ + $mm_proto->oneliner('q($(MAKECMDGOALS)) =~ /(\S*manifest\b)/ and q($(MAKECMDGOALS)) ne $1 and die qq(The DBIC build chain does not support mixing the $1 target with others\n)') +]} + EOM # keep the Makefile.PL eval happy diff --git a/maint/travis-ci_scripts/10_before_install.bash b/maint/travis-ci_scripts/10_before_install.bash index 7a5c5fe..7b6d98b 100755 --- a/maint/travis-ci_scripts/10_before_install.bash +++ b/maint/travis-ci_scripts/10_before_install.bash @@ -3,7 +3,6 @@ source maint/travis-ci_scripts/common.bash if [[ -n "$SHORT_CIRCUIT_SMOKE" ]] ; then return ; fi -# .travis.yml already restricts branches to master, topic/* and smoke/* # do some extra short-circuiting here # when smoking master do not attempt bleadperl (not release-critical) diff --git a/maint/travis-ci_scripts/30_before_script.bash b/maint/travis-ci_scripts/30_before_script.bash index 7be23f2..0dcbcca 100755 --- a/maint/travis-ci_scripts/30_before_script.bash +++ b/maint/travis-ci_scripts/30_before_script.bash @@ -70,13 +70,13 @@ else # parallel_installdeps_notest ExtUtils::MakeMaker parallel_installdeps_notest Carp - parallel_installdeps_notest Module::Build - parallel_installdeps_notest Module::Runtime ExtUtils::Depends File::Spec Data::Dumper - parallel_installdeps_notest Test::Exception Encode::Locale HTTP::Status HTTP::Daemon Try::Tiny - parallel_installdeps_notest Test::Fatal Test::Warn bareword::filehandles B::Hooks::EndOfScope + parallel_installdeps_notest Module::Build ExtUtils::Depends + parallel_installdeps_notest Module::Runtime File::Spec Data::Dumper + parallel_installdeps_notest Test::Exception Encode::Locale Test::Fatal + parallel_installdeps_notest Test::Warn bareword::filehandles B::Hooks::EndOfScope Test::Differences HTTP::Status parallel_installdeps_notest Test::Pod::Coverage Test::EOL Devel::GlobalDestruction Sub::Name MRO::Compat Class::XSAccessor URI::Escape HTML::Entities - parallel_installdeps_notest YAML LWP Moose Class::Trigger JSON::XS DBI - parallel_installdeps_notest Moo Class::Accessor::Grouped Module::Install JSON + parallel_installdeps_notest YAML LWP Moo Class::Trigger JSON::XS DBI DateTime::Format::Builder + parallel_installdeps_notest Moose Class::Accessor::Grouped Module::Install JSON Package::Variant if [[ -n "DBICTEST_FIREBIRD_DSN" ]] ; then # the official version is full of 5.10-isms, but works perfectly fine on 5.8 diff --git a/maint/travis-ci_scripts/50_after_success.bash b/maint/travis-ci_scripts/50_after_success.bash index e25d702..9ad53b8 100755 --- a/maint/travis-ci_scripts/50_after_success.bash +++ b/maint/travis-ci_scripts/50_after_success.bash @@ -3,4 +3,7 @@ source maint/travis-ci_scripts/common.bash if [[ -n "$SHORT_CIRCUIT_SMOKE" ]] ; then return ; fi -[[ "$CLEANTEST" = "true" ]] || run_or_err "Attempt to build a dist with all prereqs present" "make dist" +if [[ "$CLEANTEST" != "true" ]] ; then + parallel_installdeps_notest $(perl -Ilib -MDBIx::Class -e 'print join " ", keys %{DBIx::Class::Optional::Dependencies->req_list_for("dist_dir")}') + run_or_err "Attempt to build a dist with all prereqs present" "make dist" +fi diff --git a/t/99dbic_sqlt_parser.t b/t/99dbic_sqlt_parser.t index b98e7f2..33c33c2 100644 --- a/t/99dbic_sqlt_parser.t +++ b/t/99dbic_sqlt_parser.t @@ -2,6 +2,7 @@ use strict; use warnings; use Test::More; +use Test::Warn; use Test::Exception; use Scalar::Util (); @@ -21,11 +22,22 @@ BEGIN { my @schemas = ( create_schema ({ schema => $s }), - create_schema ({ args => { parser_args => { 'DBIx::Class::Schema' => $s } } }), - create_schema ({ args => { parser_args => { 'DBIx::Schema' => $s } } }), - create_schema ({ args => { parser_args => { package => $s } } }), + create_schema ({ args => { parser_args => { dbic_schema => $s } } }), ); + for my $parser_args_key (qw( + DBIx::Class::Schema + DBIx::Schema + package + )) { + warnings_exist { + push @schemas, create_schema({ + args => { parser_args => { $parser_args_key => $s } } + }); + } qr/\Qparser_args => {\E.+?is deprecated.+\Q@{[__FILE__]}/, + "deprecated crazy parser_arg '$parser_args_key' warned"; + } + Scalar::Util::weaken ($s); ok (!$s, 'Schema not leaked'); @@ -37,6 +49,45 @@ BEGIN { # make sure classname-style works lives_ok { isa_ok (create_schema ({ schema => 'DBICTest::Schema' }), 'SQL::Translator::Schema', 'SQLT schema object produced') }; +# make sure a connected instance passed via $args does not get the $dbh improperly serialized +SKIP: { + + # YAML is a build_requires dep of SQLT - it may or may not be here + eval { require YAML } or skip "Test requires YAML.pm", 1; + + lives_ok { + + my $s = DBICTest->init_schema(no_populate => 1); + ok ($s->storage->connected, '$schema instance connected'); + + # roundtrip through YAML + my $yaml_rt_schema = SQL::Translator->new( + parser => 'SQL::Translator::Parser::YAML' + )->translate( + data => SQL::Translator->new( + parser_args => { dbic_schema => $s }, + parser => 'SQL::Translator::Parser::DBIx::Class', + producer => 'SQL::Translator::Producer::YAML', + )->translate + ); + + isa_ok ( $yaml_rt_schema, 'SQL::Translator::Schema', 'SQLT schema object produced after YAML roundtrip'); + + ok ($s->storage->connected, '$schema instance still connected'); + } + + eval <<'EOE' or die $@; + END { + $^W = 1; # important, otherwise DBI won't trip the next fail() + $SIG{__WARN__} = sub { + fail "Unexpected global destruction warning" + if $_[0] =~ /is not a DBI/; + warn @_; + }; + } +EOE + +} my $schema = DBICTest->init_schema( no_deploy => 1 ); @@ -211,7 +262,6 @@ done_testing; sub create_schema { my $args = shift; - my $schema = $args->{schema}; my $additional_sqltargs = $args->{args} || {}; my $sqltargs = { @@ -224,7 +274,9 @@ sub create_schema { my $sqlt = SQL::Translator->new( $sqltargs ); $sqlt->parser('SQL::Translator::Parser::DBIx::Class'); - return $sqlt->translate({ data => $schema }) || die $sqlt->error; + return $sqlt->translate( + $args->{schema} ? ( data => $args->{schema} ) : () + ) || die $sqlt->error; } sub get_table { diff --git a/t/inflate/datetime.t b/t/inflate/datetime.t index 33be522..7062563 100644 --- a/t/inflate/datetime.t +++ b/t/inflate/datetime.t @@ -43,7 +43,8 @@ warnings_exist { 'using a DateTime object in ->search generates a warning'; { - local $TODO = "We can't do this yet before 0.09" if DBIx::Class->VERSION < 0.09; + local $TODO = "This stuff won't work without a -dt operator of some sort" + unless eval { require DBIx::Class::SQLMaker::DateOps }; is(eval { $row->id }, 1, 'DT in search'); diff --git a/t/lib/DBICTest.pm b/t/lib/DBICTest.pm index 0c1d3b2..589f82b 100644 --- a/t/lib/DBICTest.pm +++ b/t/lib/DBICTest.pm @@ -61,12 +61,25 @@ our ($global_lock_fh, $global_exclusive_lock); sub import { my $self = shift; - my $lockpath = DBICTest::RunMode->tmpdir->file('.dbictest_global.lock'); + my $tmpdir = DBICTest::RunMode->tmpdir; + my $lockpath = $tmpdir->file('.dbictest_global.lock'); { my $u = local_umask(0); # so that the file opens as 666, and any user can lock - sysopen ($global_lock_fh, $lockpath, O_RDWR|O_CREAT) - or die "Unable to open $lockpath: $!"; + sysopen ($global_lock_fh, $lockpath, O_RDWR|O_CREAT) or do { + my $err = $!; + + my @x_tests = map { (defined $_) ? ( $_ ? 1 : 0 ) : 'U' } map {(-e, -d, -f, -r, -w, -x, -o)} ($tmpdir, $lockpath); + + die sprintf <<"EOE", $lockpath, $err, scalar $>, scalar $), (stat($tmpdir))[4,5,2], @x_tests; +Unable to open %s: %s +Process EUID/EGID: %s / %s +TmpDir UID/GID: %s / %s +TmpDir StatMode: %o +TmpDir X-tests: -e:%s -d:%s -f:%s -r:%s -w:%s -x:%s -o:%s +TmpFile X-tests: -e:%s -d:%s -f:%s -r:%s -w:%s -x:%s -o:%s +EOE + }; } for (@_) { diff --git a/t/lib/DBICTest/BaseResultSet.pm b/t/lib/DBICTest/BaseResultSet.pm index 946219d..77d22f2 100644 --- a/t/lib/DBICTest/BaseResultSet.pm +++ b/t/lib/DBICTest/BaseResultSet.pm @@ -8,6 +8,7 @@ use warnings; use DBICTest::RunMode; use base 'DBIx::Class::ResultSet'; +__PACKAGE__->_skip_namespace_frames('^DBICTest'); sub all_hri { return [ shift->search ({}, { result_class => 'DBIx::Class::ResultClass::HashRefInflator' })->all ]; diff --git a/t/prefetch/join_type.t b/t/prefetch/join_type.t index 817b3b0..e58af4f 100644 --- a/t/prefetch/join_type.t +++ b/t/prefetch/join_type.t @@ -20,11 +20,16 @@ my $nulls = { # make sure null-prefetches do not screw with the final sql: for my $type (keys %$nulls) { -# is_same_sql_bind ( -# $cds->search({}, { prefetch => { artist => $nulls->{$type} } })->as_query, -# $cds->as_query, -# "same sql with null $type prefetch" -# ); + is_same_sql_bind ( + $cds->search({}, { prefetch => { artist => $nulls->{$type} } })->as_query, + '( SELECT me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track, + artist.artistid, artist.name, artist.rank, artist.charfield + FROM cd me + JOIN artist artist + ON artist.artistid = me.artist + )', [], + "same sql with null $type prefetch" + ); } # make sure left join is carried only starting from the first has_many diff --git a/t/prefetch/refined_search_on_relation.t b/t/prefetch/refined_search_on_relation.t new file mode 100644 index 0000000..8a7035c --- /dev/null +++ b/t/prefetch/refined_search_on_relation.t @@ -0,0 +1,56 @@ +use strict; +use warnings; + +use Test::More; +use lib qw(t/lib); +use DBICTest; + +my $schema = DBICTest->init_schema(); + +my $art = $schema->resultset('Artist')->find( + { 'me.artistid' => 1 }, + { prefetch => 'cds', order_by => { -desc => 'cds.year' } } +); + +is ( + $art->cds->search({ year => 1999 })->next->year, + 1999, + 'Found expected CD with year 1999 after refined search', +); + +is ( + $art->cds->count({ year => 1999 }), + 1, + 'Correct refined count', +); + +# this still should emit no queries: +{ + my $queries = 0; + my $orig_debug = $schema->storage->debug; + $schema->storage->debugcb(sub { $queries++; }); + $schema->storage->debug(1); + + my $cds = $art->cds; + is ( + $cds->count, + 3, + 'Correct prefetched count', + ); + + my @years = qw(2001 1999 1997); + while (my $cd = $cds->next) { + is ( + $cd->year, + (shift @years), + 'Correct prefetched cd year', + ); + } + + $schema->storage->debug($orig_debug); + $schema->storage->debugcb(undef); + + is ($queries, 0, 'No queries on prefetched operations'); +} + +done_testing; diff --git a/t/relationship/core.t b/t/relationship/core.t index 96c5066..af63a59 100644 --- a/t/relationship/core.t +++ b/t/relationship/core.t @@ -12,39 +12,23 @@ my $sdebug = $schema->storage->debug; # has_a test my $cd = $schema->resultset("CD")->find(4); -my ($artist) = ($INC{'DBICTest/HelperRels'} - ? $cd->artist - : $cd->search_related('artist')); +my ($artist) = $cd->search_related('artist'); is($artist->name, 'Random Boy Band', 'has_a search_related ok'); # has_many test with an order_by clause defined $artist = $schema->resultset("Artist")->find(1); -my @cds = ($INC{'DBICTest/HelperRels'} - ? $artist->cds - : $artist->search_related('cds')); +my @cds = $artist->search_related('cds'); is( $cds[1]->title, 'Spoonful of bees', 'has_many search_related with order_by ok' ); # search_related with additional abstract query -@cds = ($INC{'DBICTest/HelperRels'} - ? $artist->cds({ title => { like => '%of%' } }) - : $artist->search_related('cds', { title => { like => '%of%' } } ) - ); +@cds = $artist->search_related('cds', { title => { like => '%of%' } } ); is( $cds[1]->title, 'Forkful of bees', 'search_related with abstract query ok' ); # creating a related object -if ($INC{'DBICTest/HelperRels.pm'}) { - $artist->add_to_cds({ title => 'Big Flop', year => 2005 }); -} else { - my $big_flop = $artist->create_related( 'cds', { - title => 'Big Flop', - year => 2005, - } ); - - { - local $TODO = "Can't fix right now" if $DBIx::Class::VERSION < 0.09; - lives_ok { $big_flop->genre} "Don't throw exception when col is not loaded after insert"; - }; -} +$artist->create_related( 'cds', { + title => 'Big Flop', + year => 2005, +} ); my $big_flop_cd = ($artist->search_related('cds'))[3]; is( $big_flop_cd->title, 'Big Flop', 'create_related ok' ); diff --git a/t/resultset/update_delete.t b/t/resultset/update_delete.t index a5217ae..aea2ba7 100644 --- a/t/resultset/update_delete.t +++ b/t/resultset/update_delete.t @@ -263,29 +263,84 @@ $schema->storage->debug (1); 'Restricting prefetch left in, selector thrown out' ); - $rs->result_source->name('schema_qualified.cd'); - # this is expected to fail - we only want to collect the generated SQL - eval { $rs->delete }; + # switch artist and cd to fully qualified table names + # make sure nothing is stripped out + my $cd_rsrc = $schema->source('CD'); + $cd_rsrc->name('main.cd'); + $cd_rsrc->relationship_info($_)->{attrs}{cascade_delete} = 0 + for $cd_rsrc->relationships; + + my $art_rsrc = $schema->source('Artist'); + $art_rsrc->name(\'main.artist'); + $art_rsrc->relationship_info($_)->{attrs}{cascade_delete} = 0 + for $art_rsrc->relationships; + + $rs->delete; is_same_sql_bind ( $sql, \@bind, - 'DELETE FROM schema_qualified.cd WHERE ( year != ? )', + 'DELETE FROM main.cd WHERE ( year != ? )', ["'2010'"], - 'delete with fully qualified table name and subquery correct' + 'delete with fully qualified table name' + ); + + $rs->create({ title => 'foo', artist => 1, year => 2000 }); + $rs->delete_all; + is_same_sql_bind ( + $sql, + \@bind, + 'DELETE FROM main.cd WHERE ( cdid = ? )', + ["'1'"], + 'delete_all with fully qualified table name' + ); + + $rs->create({ cdid => 42, title => 'foo', artist => 2, year => 2000 }); + $rs->find(42)->delete; + is_same_sql_bind ( + $sql, + \@bind, + 'DELETE FROM main.cd WHERE ( cdid = ? )', + ["'42'"], + 'delete of object from table with fully qualified name' + ); + + $rs->create({ cdid => 42, title => 'foo', artist => 2, year => 2000 }); + $rs->find(42)->related_resultset('artist')->delete; + is_same_sql_bind ( + $sql, + \@bind, + 'DELETE FROM main.artist WHERE ( artistid IN ( SELECT me.artistid FROM main.artist me WHERE ( me.artistid = ? ) ) )', + ["'2'"], + 'delete of related object from scalarref fully qualified named table', + ); + + $schema->resultset('Artist')->find(3)->related_resultset('cds')->delete; + is_same_sql_bind ( + $sql, + \@bind, + 'DELETE FROM main.cd WHERE ( artist = ? )', + ["'3'"], + 'delete of related object from fully qualified named table', ); - # this is expected to fail - we only want to collect the generated SQL - eval { $rs->search({}, { prefetch => 'artist' })->delete }; + $schema->resultset('Artist')->find(3)->cds_unordered->delete; is_same_sql_bind ( $sql, \@bind, - 'DELETE FROM schema_qualified.cd WHERE ( cdid IN ( SELECT me.cdid FROM schema_qualified.cd me JOIN artist artist ON artist.artistid = me.artist WHERE ( me.year != ? ) ) )', + 'DELETE FROM main.cd WHERE ( artist = ? )', + ["'3'"], + 'delete of related object from fully qualified named table via relaccessor', + ); + + $rs->search({}, { prefetch => 'artist' })->delete; + is_same_sql_bind ( + $sql, + \@bind, + 'DELETE FROM main.cd WHERE ( cdid IN ( SELECT me.cdid FROM main.cd me JOIN main.artist artist ON artist.artistid = me.artist WHERE ( me.year != ? ) ) )', ["'2010'"], 'delete with fully qualified table name and subquery correct' ); - $rs->result_source->name('cd'); - # check that as_subselect_rs works ok # inner query is untouched, then a selector # and an IN condition @@ -300,14 +355,14 @@ $schema->storage->debug (1); $sql, \@bind, ' - DELETE FROM cd + DELETE FROM main.cd WHERE ( cdid IN ( SELECT me.cdid FROM ( SELECT me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track - FROM cd me - JOIN artist artist ON artist.artistid = me.artist + FROM main.cd me + JOIN main.artist artist ON artist.artistid = me.artist WHERE artist.name = ? AND me.cdid = ? ) me ) diff --git a/xt/strictures.t b/xt/strictures.t index 0ece5c9..0307cc2 100644 --- a/xt/strictures.t +++ b/xt/strictures.t @@ -23,19 +23,7 @@ find({ return if m{^(?: maint/Makefile.PL.inc/.+ # all the maint inc snippets are auto-strictured | - lib/DBIx/Class/Admin/Types.pm # MooseX::Types undetected - | - lib/DBIx/Class/Storage/DBI/Replicated/Types.pm # MooseX::Types undetected - | - lib/DBIx/Class/Storage/BlockRunner.pm # Moo undetected - | t/lib/DBICTest/Util/OverrideRequire.pm # no stictures by design (load order sensitive) - | - lib/DBIx/Class/Storage/DBI/Replicated/Replicant.pm # Moose::Role no longer detected (RT#83433) - | - lib/DBIx/Class/Storage/DBI/Replicated/WithDSN.pm # Moose::Role no longer detected (RT#83433) - | - lib/DBIx/Class/Storage/DBI/Replicated/Balancer.pm # Moose::Role no longer detected (RT#83433) )$}x; my $f = $_; diff --git a/xt/whitespace.t b/xt/whitespace.t index 111a0db..62405bb 100644 --- a/xt/whitespace.t +++ b/xt/whitespace.t @@ -20,7 +20,7 @@ unless ( DBIx::Class::Optional::Dependencies->req_ok_for ('test_whitespace') ) { { no warnings 'redefine'; my $is_pm = sub { - $_[0] !~ /\./ || $_[0] =~ /\.(?:pm|pod|skip|sql|json|proto)$/i || $_[0] =~ /::/; + $_[0] !~ /\./ || $_[0] =~ /\.(?:pm|pod|skip|bash|sql|json|proto)$/i || $_[0] =~ /::/; }; *Test::EOL::_is_perl_module = $is_pm;