From 882e0965d8ecbf0320905b794bbbdbc7cc3582f7 Mon Sep 17 00:00:00 2001 From: Alexander Hartmaier Date: Fri, 22 Oct 2010 14:47:10 +0200 Subject: [PATCH] Warn instead of throwing an exception if a key is neither a column, a relationship nor a many-to-many helper. also added a new api to the resultset api which is able to suppress the warnings when the second parameter is a hashref containing a key named unknown_params_ok with a true value. --- Changes | 2 + dist.ini | 1 + lib/DBIx/Class/ResultSet/RecursiveUpdate.pm | 76 ++++++++++----- t/lib/RunTests.pm | 101 ++++++++++++++------ 4 files changed, 126 insertions(+), 54 deletions(-) diff --git a/Changes b/Changes index f90c739..3de0d2c 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,8 @@ Revision history for DBIx::Class::ResultSet::RecursiveUpdate {{ $NEXT }} + - Warn instead of throwing an exception if a key is neither + a column, a relationship nor a many-to-many helper. 0.20 2010-10-19 09:25:33 Europe/Vienna - Support has_many relationships with multi-column primary keys diff --git a/dist.ini b/dist.ini index f6eaa7c..d98cf83 100644 --- a/dist.ini +++ b/dist.ini @@ -43,3 +43,4 @@ List::MoreUtils = 0.22 [Prereqs / TestRequires] Test::More = 0.88 +Test::Warn = 0.20 diff --git a/lib/DBIx/Class/ResultSet/RecursiveUpdate.pm b/lib/DBIx/Class/ResultSet/RecursiveUpdate.pm index ca0a771..6594748 100644 --- a/lib/DBIx/Class/ResultSet/RecursiveUpdate.pm +++ b/lib/DBIx/Class/ResultSet/RecursiveUpdate.pm @@ -2,17 +2,34 @@ use strict; use warnings; package DBIx::Class::ResultSet::RecursiveUpdate; + # ABSTRACT: like update_or_create - but recursive use base qw(DBIx::Class::ResultSet); sub recursive_update { - my ( $self, $updates, $fixed_fields ) = @_; + my ( $self, $updates, $attrs ) = @_; + + my $fixed_fields; + my $unknown_params_ok; + + # 0.21+ api + if ( defined $attrs && ref $attrs eq 'HASH' ) { + $fixed_fields = $attrs->{fixed_fields}; + $unknown_params_ok = $attrs->{unknown_params_ok}; + } + + # pre 0.21 api + elsif ( defined $attrs && ref $attrs eq 'ARRAY' ) { + $fixed_fields = $attrs; + } + return DBIx::Class::ResultSet::RecursiveUpdate::Functions::recursive_update( - resultset => $self, - updates => $updates, - fixed_fields => $fixed_fields + resultset => $self, + updates => $updates, + fixed_fields => $fixed_fields, + unknown_params_ok => $unknown_params_ok, ); } @@ -24,22 +41,25 @@ use List::MoreUtils qw/ any /; sub recursive_update { my %params = @_; my ( $self, $updates, $fixed_fields, $object, $resolved, - $if_not_submitted ) + $if_not_submitted, $unknown_params_ok ) = @params{ - qw/resultset updates fixed_fields object resolved if_not_submitted/}; + qw/resultset updates fixed_fields object resolved if_not_submitted unknown_params_ok/ + }; $resolved ||= {}; # warn 'entering: ' . $self->result_source->from(); carp 'fixed fields needs to be an array ref' - if $fixed_fields && ref($fixed_fields) ne 'ARRAY'; - my %fixed_fields; - %fixed_fields = map { $_ => 1 } @$fixed_fields if $fixed_fields; + if defined $fixed_fields && ref $fixed_fields ne 'ARRAY'; + if ( blessed($updates) && $updates->isa('DBIx::Class::Row') ) { return $updates; } if ( $updates->{id} ) { $object = $self->find( $updates->{id}, { key => 'primary' } ); } + + my %fixed_fields = map { $_ => 1 } @$fixed_fields + if $fixed_fields; my @missing = grep { !exists $updates->{$_} && !exists $fixed_fields{$_} } $self->result_source->primary_columns; @@ -122,11 +142,16 @@ sub recursive_update { } # unknown - # TODO: don't throw a warning instead of an exception to give users - # time to adapt to the new API - $self->throw_exception( + + # don't throw a warning instead of an exception to give users + # time to adapt to the new API + warn( "No such column, relationship, many-to-many helper accessor or generic accessor '$name'" - ); + ) unless $unknown_params_ok; + +#$self->throw_exception( +# "No such column, relationship, many-to-many helper accessor or generic accessor '$name'" +#); } # warn 'other: ' . Dumper( \%other_methods ); use Data::Dumper; @@ -528,8 +553,9 @@ __END__ title => "One Flew Over the Cuckoo's Nest" } ] - } - }); + }, + unknown_params_ok => 1, + ); # As ResultSet subclass: @@ -577,16 +603,18 @@ like in the case of: my $restricted_rs = $user_rs->search( { id => 1 } ); -then you need to inform recursive_update about additional predicate with a second argument: +then you need to inform recursive_update about the additional predicate with the fixed_fields attribute: - my $user = $restricted_rs->recursive_update( { - owned_dvds => [ - { - title => 'One Flew Over the Cuckoo's Nest' - } - ] - }, - [ 'id' ] + my $user = $restricted_rs->recursive_update( { + owned_dvds => [ + { + title => 'One Flew Over the Cuckoo's Nest' + } + ] + }, + { + fixed_fields => [ 'id' ], + } ); For a many_to_many (pseudo) relation you can supply a list of primary keys diff --git a/t/lib/RunTests.pm b/t/lib/RunTests.pm index d0fce5b..7121ed2 100644 --- a/t/lib/RunTests.pm +++ b/t/lib/RunTests.pm @@ -4,44 +4,86 @@ use Exporter 'import'; # gives you Exporter's import() method directly @EXPORT = qw(run_tests); use strict; use Test::More; +use Test::Warn; use DBIx::Class::ResultSet::RecursiveUpdate; sub run_tests { my $schema = shift; - plan tests => 47; + plan tests => 51; my $dvd_rs = $schema->resultset('Dvd'); my $user_rs = $schema->resultset('User'); - my $owner = $user_rs->next; - my $another_owner = $user_rs->next; - my $initial_user_count = $user_rs->count; - my $initial_dvd_count = $dvd_rs->count; + my $owner = $user_rs->next; + my $another_owner = $user_rs->next; + my $initial_user_count = $user_rs->count; + my $expected_user_count = $initial_user_count; + my $initial_dvd_count = $dvd_rs->count; my $updates; - $dvd_rs->search( { dvd_id => 1 } )->recursive_update( { - owner => { username => 'aaa' } + # pre 0.21 api + $dvd_rs->search( { dvd_id => 1 } )->recursive_update( { + owner => { username => 'aaa' } }, [ 'dvd_id' ] ); my $u = $user_rs->find( $dvd_rs->find( 1 )->owner->id ); - is( $u->username, 'aaa', 'fixed_fields' ); + is( $u->username, 'aaa', 'fixed_fields pre 0.21 api ok' ); + + # 0.21+ api + $dvd_rs->search( { dvd_id => 1 } )->recursive_update( { + owner => { username => 'bbb' } + }, + { + fixed_fields => [ 'dvd_id' ], + } + ); + + my $u = $user_rs->find( $dvd_rs->find( 1 )->owner->id ); + is( $u->username, 'bbb', 'fixed_fields 0.21+ api ok' ); # try to create with a not existing rel $updates = { - name => 'Test name for nonexisting rel', + name => 'Test for nonexisting rel', username => 'nonexisting_rel', password => 'whatever', nonexisting => { foo => 'bar' }, }; - eval { my $nonexisting_user = $user_rs->recursive_update($updates); }; - like( - $@, - qr/No such column, relationship, many-to-many helper accessor or generic accessor 'nonexisting'/, - 'nonexisting column, accessor, relationship fails' - ); + +# for future use when we switch from warn to throw_exception +# eval { $user_rs->recursive_update($updates); }; +# like( +# $@, +# qr/No such column, relationship, many-to-many helper accessor or generic accessor 'nonexisting'/, +# 'nonexisting column, accessor, relationship fails' +# ); + warning_is { + my $user = $user_rs->recursive_update($updates); + } + "No such column, relationship, many-to-many helper accessor or generic accessor 'nonexisting'", + 'nonexisting column, accessor, relationship warns'; + $expected_user_count++; + is( $user_rs->count, $expected_user_count, 'User created' ); + + # try to create with a not existing rel but suppressed warning + $updates = { + name => 'Test for nonexisting rel with suppressed warning', + username => 'suppressed_nonexisting_rel', + password => 'whatever', + nonexisting => { foo => 'bar' }, + }; + + warning_is { + my $user = + $user_rs->recursive_update( $updates, + { unknown_params_ok => 1 } ); + } + "", + "nonexisting column, accessor, relationship doesn't warn with unknown_params_ok"; + $expected_user_count++; + is( $user_rs->count, $expected_user_count, 'User created' ); # creating new record linked to some old record $updates = { @@ -53,8 +95,9 @@ sub run_tests { my $new_dvd = $dvd_rs->recursive_update($updates); is( $dvd_rs->count, $initial_dvd_count + 1, 'Dvd created' ); + is( $schema->resultset('User')->count, - $initial_user_count, "No new user created" ); + $expected_user_count, "No new user created" ); is( $new_dvd->name, 'Test name 2', 'Dvd name set' ); is( $new_dvd->owner->id, $another_owner->id, 'Owner set' ); is( $new_dvd->viewings->count, 1, 'Viewing created' ); @@ -81,12 +124,11 @@ sub run_tests { }; my $dvd = $dvd_rs->recursive_update($updates); + $expected_user_count++; is( $dvd_rs->count, $initial_dvd_count + 2, 'Dvd created' ); is( $schema->resultset('User')->count, - $initial_user_count + 1, - "One new user created" - ); + $expected_user_count, "One new user created" ); is( $dvd->name, 'Test name', 'Dvd name set' ); is_deeply( [ map { $_->id } $dvd->tags ], [ '2', '3' ], 'Tags set' ); is( $dvd->owner->id, $owner->id, 'Owner set' ); @@ -104,10 +146,10 @@ sub run_tests { ->find( { key1 => $onekey->id, key2 => 1 } ), 'Twokeys_belongsto created' ); - TODO: { +TODO: { local $TODO = 'value of fk from a multi relationship'; is( $dvd->twokeysfk, $onekey->id, 'twokeysfk in Dvd' ); - }; + } is( $dvd->name, 'Test name', 'Dvd name set' ); # changing existing records @@ -130,9 +172,7 @@ sub run_tests { is( $dvd_updated->dvd_id, $dvd->dvd_id, 'Pk from "id"' ); is( $schema->resultset('User')->count, - $initial_user_count + 1, - "No new user created" - ); + $expected_user_count, "No new user created" ); is( $dvd_updated->name, undef, 'Dvd name deleted' ); is( $dvd_updated->owner->id, $another_owner->id, 'Owner updated' ); is( $dvd_updated->current_borrower->name, @@ -174,10 +214,10 @@ sub run_tests { }; my $user = $user_rs->recursive_update($updates); + $expected_user_count++; + is( $schema->resultset('User')->count, - $initial_user_count + 2, - "New user created" - ); + $expected_user_count, "New user created" ); is( $dvd_rs->count, $initial_dvd_count + 4, 'Dvds created' ); my %owned_dvds = map { $_->name => $_ } $user->owned_dvds; is( scalar keys %owned_dvds, 2, 'Has many relations created' ); @@ -265,14 +305,15 @@ sub run_tests { # delete has_many where foreign cols aren't nullable my $rs_user_dvd = $user->owned_dvds; my @user_dvd_ids = map { $_->id } $rs_user_dvd->all; - is( $rs_user_dvd->count, 1, 'user owns 1 dvd'); + is( $rs_user_dvd->count, 1, 'user owns 1 dvd' ); $updates = { id => $user->id, owned_dvds => undef, }; $user = $user_rs->recursive_update($updates); - is( $user->owned_dvds->count, 0, 'user owns no dvds'); - is( $dvd_rs->search({ dvd_id => {-in => \@user_dvd_ids }})->count, 0, 'owned dvds deleted' ); + is( $user->owned_dvds->count, 0, 'user owns no dvds' ); + is( $dvd_rs->search( { dvd_id => { -in => \@user_dvd_ids } } )->count, + 0, 'owned dvds deleted' ); # $updates = { # name => 'Test name 1', -- 2.45.2