From 71e7804d2c5b7576a86cfe2234cff34020742d77 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Tue, 11 Oct 2016 11:52:12 +0200 Subject: [PATCH] Bug 17425: Make Koha::Object raise exceptions This patch makes Koha::Object raise exceptions in the following situations: - When a non existent accessor is called - When a non existent property is tried to be updated using ->set On implementing this change, we introduce Koha::Exceptions::Object class to contain all Koha::Object-specific exception definitions. Unit tests for this change are introduced in t/db_dependent/Koha/Objects.t To test: - Apply the patches on master - Run: $ prove t/db_dependent/Koha/Objects.t => SUCCESS: Tests return green - Sign off Note: A followup introduces the dependency for Try::Tiny. It needs to be present for running the tests. Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi Signed-off-by: Kyle M Hall --- Koha/Exceptions/Object.pm | 20 ++++++++++++++++++++ Koha/Object.pm | 7 +++---- t/db_dependent/Koha/Objects.t | 30 +++++++++++++++++++++++++++--- 3 files changed, 50 insertions(+), 7 deletions(-) create mode 100644 Koha/Exceptions/Object.pm diff --git a/Koha/Exceptions/Object.pm b/Koha/Exceptions/Object.pm new file mode 100644 index 0000000000..e08b5a5cd0 --- /dev/null +++ b/Koha/Exceptions/Object.pm @@ -0,0 +1,20 @@ +package Koha::Exceptions::Object; + +use Modern::Perl; + +use Exception::Class ( + + 'Koha::Exceptions::Object' => { + description => 'Something went wrong!', + }, + 'Koha::Exceptions::Object::MethodNotFound' => { + isa => 'Koha::Exceptions::Object', + description => "Invalid method", + }, + 'Koha::Exceptions::Object::PropertyNotFound' => { + isa => 'Koha::Exceptions::Object', + description => "Invalid property", + } +); + +1; diff --git a/Koha/Object.pm b/Koha/Object.pm index c6bfede2c5..cb2b9ef4a4 100644 --- a/Koha/Object.pm +++ b/Koha/Object.pm @@ -23,6 +23,7 @@ use Modern::Perl; use Carp; use Koha::Database; +use Koha::Exceptions::Object; =head1 NAME @@ -170,8 +171,7 @@ sub set { foreach my $p ( keys %$properties ) { unless ( grep {/^$p$/} @columns ) { - carp("No property $p!"); - return 0; + Koha::Exceptions::Object::PropertyNotFound->throw( "No property $p for " . ref($self) ); } } @@ -252,8 +252,7 @@ sub AUTOLOAD { my $r = eval { $self->_result->$method(@_) }; if ( $@ ) { - carp "No method $method found for " . ref($self) . " " . $@; - return + Koha::Exceptions::Object::MethodNotFound->throw( "No method $method for " . ref($self) ); } return $r; } diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t index 87f4a94491..de7b327e0a 100644 --- a/t/db_dependent/Koha/Objects.t +++ b/t/db_dependent/Koha/Objects.t @@ -31,8 +31,11 @@ use Koha::Database; use t::lib::TestBuilder; +use Try::Tiny; + my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; +my $builder = t::lib::TestBuilder->new; is( ref(Koha::Authority::Types->find('')), 'Koha::Authority::Type', 'Koha::Objects->find should work if the primary key is an empty string' ); @@ -42,7 +45,7 @@ is( $borrowernumber_exists, 1, 'Koha::Objects->columns should return the table c subtest 'update' => sub { plan tests => 2; - my $builder = t::lib::TestBuilder->new; + $builder->build( { source => 'City', value => { city_country => 'UK' } } ); $builder->build( { source => 'City', value => { city_country => 'UK' } } ); $builder->build( { source => 'City', value => { city_country => 'UK' } } ); @@ -62,7 +65,7 @@ subtest 'pager' => sub { subtest 'reset' => sub { plan tests => 1; - my $builder = t::lib::TestBuilder->new; + my $patrons = Koha::Patrons->search; my $first_borrowernumber = $patrons->next->borrowernumber; my $second_borrowernumber = $patrons->next->borrowernumber; @@ -71,7 +74,7 @@ subtest 'reset' => sub { subtest 'delete' => sub { plan tests => 2; - my $builder = t::lib::TestBuilder->new; + my $patron_1 = $builder->build({source => 'Borrower'}); my $patron_2 = $builder->build({source => 'Borrower'}); is( Koha::Patrons->search({ -or => { borrowernumber => [ $patron_1->{borrowernumber}, $patron_2->{borrowernumber}]}})->delete, 2, ''); @@ -111,5 +114,26 @@ subtest 'search_related' => sub { is( $libraries[1]->branchcode, $patron_2->{branchcode}, 'Koha::Objects->search_related should work as expected' ); }; +subtest 'Exceptions' => sub { + plan tests => 2; + + my $patron_borrowernumber = $builder->build({ source => 'Borrower' })->{ borrowernumber }; + my $patron = Koha::Patrons->find( $patron_borrowernumber ); + + try { + $patron->blah('blah'); + } catch { + ok( $_->isa('Koha::Exceptions::Object::MethodNotFound'), + 'Calling a non-existent method should raise a Koha::Exceptions::Object::MethodNotFound exception' ); + }; + + try { + $patron->set({ blah => 'blah' }); + } catch { + ok( $_->isa('Koha::Exceptions::Object::PropertyNotFound'), + 'Setting a non-existent property should raise a Koha::Exceptions::Object::PropertyNotFound exception' ); + }; +}; + $schema->storage->txn_rollback; 1; -- 2.39.5