From bb4c8853a7a97d6cbd43af1e71653f3e3b561c89 Mon Sep 17 00:00:00 2001 From: Olli-Antti Kivilahti Date: Mon, 23 Oct 2017 15:29:41 +0300 Subject: [PATCH] Bug 19515 - Refactor REST API session mocking to reduce ridiculous amounts of duplication The amount of duplication in REST API tests is beyond anything. I must be hallucinating. This adds a t::lib::Mocks::mock_session() which mocks a session reverting to defaults, and checks CGI::Session->flush for errors. --- t/db_dependent/api/v1/accountlines.t | 8 +--- t/db_dependent/api/v1/auth.t | 7 +--- t/db_dependent/api/v1/availability.t | 7 +--- t/db_dependent/api/v1/biblios.t | 7 +--- t/db_dependent/api/v1/checkouts.t | 14 +------ t/db_dependent/api/v1/checkoutsexpanded.t | 7 +--- t/db_dependent/api/v1/checkoutshistory.t | 14 +------ t/db_dependent/api/v1/cities.t | 7 +--- t/db_dependent/api/v1/holds.t | 28 ++------------ t/db_dependent/api/v1/issuingrules.t | 7 +--- t/db_dependent/api/v1/items.t | 7 +--- t/db_dependent/api/v1/labels.t | 7 +--- t/db_dependent/api/v1/messagingpreferences.t | 7 +--- t/db_dependent/api/v1/notices.t | 7 +--- t/db_dependent/api/v1/ownerflag.t | 7 +--- t/db_dependent/api/v1/passwordrecovery.t | 7 +--- t/db_dependent/api/v1/patrons.t | 7 +--- t/db_dependent/api/v1/payments.t | 7 +--- t/db_dependent/api/v1/records.t | 7 +--- t/db_dependent/api/v1/reports.t | 7 +--- t/lib/Mocks.pm | 58 ++++++++++++++++++++++++++++ 21 files changed, 84 insertions(+), 150 deletions(-) diff --git a/t/db_dependent/api/v1/accountlines.t b/t/db_dependent/api/v1/accountlines.t index 120970f..82cbb05 100644 --- a/t/db_dependent/api/v1/accountlines.t +++ b/t/db_dependent/api/v1/accountlines.t @@ -300,12 +300,8 @@ sub create_user_and_session { } }); - my $session = C4::Auth::get_session(''); - $session->param('number', $user->{ borrowernumber }); - $session->param('id', $user->{ userid }); - $session->param('ip', '127.0.0.1'); - $session->param('lasttime', time()); - $session->flush; + # Create a session for the authorized user + my $session = t::lib::Mocks::mock_session({borrower => $user}); return ($user, $session); } diff --git a/t/db_dependent/api/v1/auth.t b/t/db_dependent/api/v1/auth.t index ca85f6b..c3c3178 100644 --- a/t/db_dependent/api/v1/auth.t +++ b/t/db_dependent/api/v1/auth.t @@ -237,12 +237,7 @@ sub create_user_and_session { ); # Create a session for the authorized user - my $session = C4::Auth::get_session(''); - $session->param( 'number', $user->{borrowernumber} ); - $session->param( 'id', $user->{userid} ); - $session->param( 'ip', '127.0.0.1' ); - $session->param( 'lasttime', time() ); - $session->flush; + my $session = t::lib::Mocks::mock_session({borrower => $user}); return ( $user->{borrowernumber}, $session->id ); } diff --git a/t/db_dependent/api/v1/availability.t b/t/db_dependent/api/v1/availability.t index 5b16883..443c9f8 100644 --- a/t/db_dependent/api/v1/availability.t +++ b/t/db_dependent/api/v1/availability.t @@ -392,12 +392,7 @@ sub create_user_and_session { ); # Create a session for the authorized user - my $session = C4::Auth::get_session(''); - $session->param( 'number', $user->{borrowernumber} ); - $session->param( 'id', $user->{userid} ); - $session->param( 'ip', '127.0.0.1' ); - $session->param( 'lasttime', time() ); - $session->flush; + my $session = t::lib::Mocks::mock_session({borrower => $user}); if ( $args->{authorized} ) { $dbh->do( " diff --git a/t/db_dependent/api/v1/biblios.t b/t/db_dependent/api/v1/biblios.t index 4b1c035..eeb6ea2 100644 --- a/t/db_dependent/api/v1/biblios.t +++ b/t/db_dependent/api/v1/biblios.t @@ -169,12 +169,7 @@ sub create_user_and_session { }, }); - my $session = C4::Auth::get_session(''); - $session->param('number', $user->{ borrowernumber }); - $session->param('id', $user->{ userid }); - $session->param('ip', '127.0.0.1'); - $session->param('lasttime', time()); - $session->flush; + my $session = t::lib::Mocks::mock_session({borrower => $user}); my $patron = Koha::Patrons->find($user->{borrowernumber}); if ( $flags ) { diff --git a/t/db_dependent/api/v1/checkouts.t b/t/db_dependent/api/v1/checkouts.t index e94d0ed..47dfc59 100644 --- a/t/db_dependent/api/v1/checkouts.t +++ b/t/db_dependent/api/v1/checkouts.t @@ -61,12 +61,7 @@ Koha::Auth::PermissionManager->grantPermission( 'circulate', 'circulate_remaining_permissions' ); -my $session = C4::Auth::get_session(''); -$session->param('number', $loggedinuser->{ borrowernumber }); -$session->param('id', $loggedinuser->{ userid }); -$session->param('ip', '127.0.0.1'); -$session->param('lasttime', time()); -$session->flush; +my $session = t::lib::Mocks::mock_session({borrower => $loggedinuser}); my $patron = $builder->build({ source => 'Borrower', value => { @@ -78,12 +73,7 @@ my $patron = $builder->build({ source => 'Borrower', } }); my $borrowernumber = $patron->{borrowernumber}; -my $patron_session = C4::Auth::get_session(''); -$patron_session->param('number', $borrowernumber); -$patron_session->param('id', $patron->{ userid }); -$patron_session->param('ip', '127.0.0.1'); -$patron_session->param('lasttime', time()); -$patron_session->flush; +my $patron_session = t::lib::Mocks::mock_session({borrower => $patron}); my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; my $module = new Test::MockModule('C4::Context'); diff --git a/t/db_dependent/api/v1/checkoutsexpanded.t b/t/db_dependent/api/v1/checkoutsexpanded.t index 59ea4fb..c8205f5 100644 --- a/t/db_dependent/api/v1/checkoutsexpanded.t +++ b/t/db_dependent/api/v1/checkoutsexpanded.t @@ -162,12 +162,7 @@ sub create_user_and_session { } }); - my $borrowersession = C4::Auth::get_session(''); - $borrowersession->param('number', $borrower->{ borrowernumber }); - $borrowersession->param('id', $borrower->{ userid }); - $borrowersession->param('ip', '127.0.0.1'); - $borrowersession->param('lasttime', time()); - $borrowersession->flush; + my $borrowersession = t::lib::Mocks::mock_session({borrower => $borrower}); return ($borrower->{borrowernumber}, $borrowersession->id); } diff --git a/t/db_dependent/api/v1/checkoutshistory.t b/t/db_dependent/api/v1/checkoutshistory.t index 72aaa28..835c213 100644 --- a/t/db_dependent/api/v1/checkoutshistory.t +++ b/t/db_dependent/api/v1/checkoutshistory.t @@ -58,12 +58,7 @@ Koha::Auth::PermissionManager->grantPermission( 'circulate', 'circulate_remaining_permissions' ); -my $session = C4::Auth::get_session(''); -$session->param('number', $loggedinuser->{ borrowernumber }); -$session->param('id', $loggedinuser->{ userid }); -$session->param('ip', '127.0.0.1'); -$session->param('lasttime', time()); -$session->flush; +my $session = t::lib::Mocks::mock_session({borrower => $loggedinuser}); my $nopermission = $builder->build({ source => 'Borrower', @@ -75,12 +70,7 @@ my $nopermission = $builder->build({ } }); -my $session_nopermission = C4::Auth::get_session(''); -$session_nopermission->param('number', $nopermission->{ borrowernumber }); -$session_nopermission->param('id', $nopermission->{ userid }); -$session_nopermission->param('ip', '127.0.0.1'); -$session_nopermission->param('lasttime', time()); -$session_nopermission->flush; +my $session_nopermission = t::lib::Mocks::mock_session({borrower => $nopermission}); my $borrower = $builder->build({ source => 'Borrower' }); my $borrowernumber = $borrower->{borrowernumber}; diff --git a/t/db_dependent/api/v1/cities.t b/t/db_dependent/api/v1/cities.t index cc7040e..5707aa0 100644 --- a/t/db_dependent/api/v1/cities.t +++ b/t/db_dependent/api/v1/cities.t @@ -360,12 +360,7 @@ sub create_user_and_session { ); # Create a session for the authorized user - my $session = C4::Auth::get_session(''); - $session->param( 'number', $user->{borrowernumber} ); - $session->param( 'id', $user->{userid} ); - $session->param( 'ip', '127.0.0.1' ); - $session->param( 'lasttime', time() ); - $session->flush; + my $session = t::lib::Mocks::mock_session({borrower => $user}); if ( $args->{authorized} ) { my $patron = Koha::Patrons->find($user->{borrowernumber}); diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t index 022323d..0b45032 100644 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -72,12 +72,7 @@ my $nopermission = $builder->build({ }); my $nopermission_patron = Koha::Patrons->find($nopermission->{borrowernumber}); -my $session_nopermission = C4::Auth::get_session(''); -$session_nopermission->param('number', $nopermission->{ borrowernumber }); -$session_nopermission->param('id', $nopermission->{ userid }); -$session_nopermission->param('ip', '127.0.0.1'); -$session_nopermission->param('lasttime', time()); -$session_nopermission->flush; +my $session_nopermission = t::lib::Mocks::mock_session({borrower => $nopermission_patron}); my $borrower = Koha::Patrons->find($builder->build({ source => 'Borrower', @@ -131,24 +126,9 @@ my $borrower3 = Koha::Patrons->find($builder->build({ my $borrowernumber3 = $borrower3->borrowernumber; # Get sessions -my $session = C4::Auth::get_session(''); -$session->param('number', $borrower->borrowernumber); -$session->param('id', $borrower->userid); -$session->param('ip', '127.0.0.1'); -$session->param('lasttime', time()); -$session->flush; -my $session2 = C4::Auth::get_session(''); -$session2->param('number', $borrower2->borrowernumber); -$session2->param('id', $borrower2->userid); -$session2->param('ip', '127.0.0.1'); -$session2->param('lasttime', time()); -$session2->flush; -my $session3 = C4::Auth::get_session(''); -$session3->param('number', $borrower3->borrowernumber); -$session3->param('id', $borrower3->userid); -$session3->param('ip', '127.0.0.1'); -$session3->param('lasttime', time()); -$session3->flush; +my $session = t::lib::Mocks::mock_session({borrower => $borrower}); +my $session2 = t::lib::Mocks::mock_session({borrower => $borrower2}); +my $session3 = t::lib::Mocks::mock_session({borrower => $borrower3}); my $biblionumber = create_biblio('RESTful Web APIs'); my $itemnumber = create_item($biblionumber, 'TEST000001'); diff --git a/t/db_dependent/api/v1/issuingrules.t b/t/db_dependent/api/v1/issuingrules.t index eeee70a..701d799 100644 --- a/t/db_dependent/api/v1/issuingrules.t +++ b/t/db_dependent/api/v1/issuingrules.t @@ -367,12 +367,7 @@ sub create_user_and_session { ); # Create a session for the authorized user - my $session = C4::Auth::get_session(''); - $session->param( 'number', $user->{borrowernumber} ); - $session->param( 'id', $user->{userid} ); - $session->param( 'ip', '127.0.0.1' ); - $session->param( 'lasttime', time() ); - $session->flush; + my $session = t::lib::Mocks::mock_session({borrower => $user}); if ( $args->{authorized} ) { my $patron = Koha::Patrons->find($user->{borrowernumber}); diff --git a/t/db_dependent/api/v1/items.t b/t/db_dependent/api/v1/items.t index 9b3d2e6..c437c54 100644 --- a/t/db_dependent/api/v1/items.t +++ b/t/db_dependent/api/v1/items.t @@ -110,12 +110,7 @@ sub create_session { my @sessions; foreach $borrower (@borrowers) { - my $session = C4::Auth::get_session(''); - $session->param('number', $borrower->{borrowernumber}); - $session->param('id', $borrower->{userid}); - $session->param('ip', '127.0.0.1'); - $session->param('lasttime', time()); - $session->flush; + my $session = t::lib::Mocks::mock_session({borrower => $borrower}); push @sessions, $session; } diff --git a/t/db_dependent/api/v1/labels.t b/t/db_dependent/api/v1/labels.t index 94427c1..97fbf89 100644 --- a/t/db_dependent/api/v1/labels.t +++ b/t/db_dependent/api/v1/labels.t @@ -283,12 +283,7 @@ sub create_user_and_session { } }); - my $borrowersession = C4::Auth::get_session(''); - $borrowersession->param('number', $borrower->{ borrowernumber }); - $borrowersession->param('id', $borrower->{ userid }); - $borrowersession->param('ip', '127.0.0.1'); - $borrowersession->param('lasttime', time()); - $borrowersession->flush; + my $borrowersession = t::lib::Mocks::mock_session({borrower => $borrower}); return ($borrower->{borrowernumber}, $borrowersession->id); } diff --git a/t/db_dependent/api/v1/messagingpreferences.t b/t/db_dependent/api/v1/messagingpreferences.t index d1538af..3a01d5b 100644 --- a/t/db_dependent/api/v1/messagingpreferences.t +++ b/t/db_dependent/api/v1/messagingpreferences.t @@ -292,12 +292,7 @@ sub create_user_and_session { } }); - my $session = C4::Auth::get_session(''); - $session->param('number', $borrower->{ borrowernumber }); - $session->param('id', $borrower->{ userid }); - $session->param('ip', '127.0.0.1'); - $session->param('lasttime', time()); - $session->flush; + my $session = t::lib::Mocks::mock_session({borrower => $borrower}); my $patron = Koha::Patrons->find($borrower->{borrowernumber}); return ($patron, $session); diff --git a/t/db_dependent/api/v1/notices.t b/t/db_dependent/api/v1/notices.t index 1178540..fd291fc 100644 --- a/t/db_dependent/api/v1/notices.t +++ b/t/db_dependent/api/v1/notices.t @@ -467,12 +467,7 @@ sub create_user_and_session { } }); - my $session = C4::Auth::get_session(''); - $session->param('number', $borrower->{ borrowernumber }); - $session->param('id', $borrower->{ userid }); - $session->param('ip', '127.0.0.1'); - $session->param('lasttime', time()); - $session->flush; + my $session = t::lib::Mocks::mock_session({borrower => $borrower}); my $patron = Koha::Patrons->find($borrower->{borrowernumber}); if ( $flags ) { Koha::Auth::PermissionManager->grantPermissions($patron, $flags); diff --git a/t/db_dependent/api/v1/ownerflag.t b/t/db_dependent/api/v1/ownerflag.t index 97e7ebb..2923c5f 100644 --- a/t/db_dependent/api/v1/ownerflag.t +++ b/t/db_dependent/api/v1/ownerflag.t @@ -120,12 +120,7 @@ sub create_user_and_session { lost => 0, } }); - my $session = C4::Auth::get_session(''); - $session->param('number', $patron->{ borrowernumber }); - $session->param('id', $patron->{ userid }); - $session->param('ip', '127.0.0.1'); - $session->param('lasttime', time()); - $session->flush; + my $session = t::lib::Mocks::mock_session({borrower => $patron}); return ($patron, $session); } diff --git a/t/db_dependent/api/v1/passwordrecovery.t b/t/db_dependent/api/v1/passwordrecovery.t index c38cd74..9c3e2a6 100644 --- a/t/db_dependent/api/v1/passwordrecovery.t +++ b/t/db_dependent/api/v1/passwordrecovery.t @@ -394,12 +394,7 @@ sub create_user_and_session { } }); - my $session = C4::Auth::get_session(''); - $session->param('number', $borrower->{ borrowernumber }); - $session->param('id', $borrower->{ userid }); - $session->param('ip', '127.0.0.1'); - $session->param('lasttime', time()); - $session->flush; + my $session = t::lib::Mocks::mock_session({borrower => $borrower}); my $patron = Koha::Patrons->find($borrower->{borrowernumber}); if ( $flags ) { Koha::Auth::PermissionManager->grantPermissions($patron, $flags); diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t index 1817904..e288931 100644 --- a/t/db_dependent/api/v1/patrons.t +++ b/t/db_dependent/api/v1/patrons.t @@ -660,12 +660,7 @@ sub create_user_and_session { ); # Create a session for the authorized user - my $session = C4::Auth::get_session(''); - $session->param( 'number', $user->{borrowernumber} ); - $session->param( 'id', $user->{userid} ); - $session->param( 'ip', '127.0.0.1' ); - $session->param( 'lasttime', time() ); - $session->flush; + my $session = t::lib::Mocks::mock_session({borrower => $user}); return ( $user->{borrowernumber}, $session->id ); } diff --git a/t/db_dependent/api/v1/payments.t b/t/db_dependent/api/v1/payments.t index 63c3888..1b368ac 100644 --- a/t/db_dependent/api/v1/payments.t +++ b/t/db_dependent/api/v1/payments.t @@ -238,12 +238,7 @@ sub create_user_and_session { } }); - my $borrowersession = C4::Auth::get_session(''); - $borrowersession->param('number', $borrower->{ borrowernumber }); - $borrowersession->param('id', $borrower->{ userid }); - $borrowersession->param('ip', '127.0.0.1'); - $borrowersession->param('lasttime', time()); - $borrowersession->flush; + my $borrowersession = t::lib::Mocks::mock_session({borrower => $borrower}); return ($borrower->{borrowernumber}, $borrowersession->id); } diff --git a/t/db_dependent/api/v1/records.t b/t/db_dependent/api/v1/records.t index 1aa9a03..2596cd5 100644 --- a/t/db_dependent/api/v1/records.t +++ b/t/db_dependent/api/v1/records.t @@ -383,12 +383,7 @@ sub create_user_and_session { } }); - my $session = C4::Auth::get_session(''); - $session->param('number', $borrower->{ borrowernumber }); - $session->param('id', $borrower->{ userid }); - $session->param('ip', '127.0.0.1'); - $session->param('lasttime', time()); - $session->flush; + my $session = t::lib::Mocks::mock_session({borrower => $borrower}); my $patron = Koha::Patrons->find($borrower->{borrowernumber}); if ( $flags ) { foreach my $flag (keys %$flags) { diff --git a/t/db_dependent/api/v1/reports.t b/t/db_dependent/api/v1/reports.t index 42b3e41..c2e9d07 100644 --- a/t/db_dependent/api/v1/reports.t +++ b/t/db_dependent/api/v1/reports.t @@ -160,12 +160,7 @@ sub create_user_and_session { } }); - my $session = C4::Auth::get_session(''); - $session->param('number', $borrower->{ borrowernumber }); - $session->param('id', $borrower->{ userid }); - $session->param('ip', '127.0.0.1'); - $session->param('lasttime', time()); - $session->flush; + my $session = t::lib::Mocks::mock_session({borrower => $borrower}); my $patron = Koha::Patrons->find($borrower->{borrowernumber}); if ( $flags ) { Koha::Auth::PermissionManager->grantPermissions($patron, $flags); diff --git a/t/lib/Mocks.pm b/t/lib/Mocks.pm index da20739..9894082 100644 --- a/t/lib/Mocks.pm +++ b/t/lib/Mocks.pm @@ -6,6 +6,9 @@ use C4::Context; use Koha::Schema; use Test::MockModule; +use Koha::Exceptions; +use Koha::Exceptions::File; + my %configs; sub mock_config { my $context = new Test::MockModule('C4::Context'); @@ -41,4 +44,59 @@ sub mock_preference { }); } +=head2 mock_session + + t::lib::Mocks::mock_session({ + borrower => { #One can give a borrower-hash or a Koha::Borrower-object to mint the session. + borrowernumber => 0420, + userid => 'OG Kush', + }, + params => { #Extra parameters given directly to the CGI::Session as param(), overwrites previous defaults, such as the values inferred from borrower + number => $borrowernumber, + id => $userid, + ip => '127.0.0.1', + lasttime => time(), + ... + }, + }); + +Mocks a CGI::Session used by C4::Auth. +Sets reasonable defaults for most values. + +@PARAM1 Hashref of parameters to be given to CGI::Session +@RETURNS CGI::Session, a new session object serialized to the backing store, ready to be authenticated with +@THROWS Koha::Exception::File, if flushing the session to a file failed + +=cut + +sub mock_session { + my ($args) = @_; + my $params = $args->{params}; + + my $session = C4::Auth::get_session(''); + if (my $borrower = $args->{borrower}) { + $borrower = Koha::Patrons->cast($borrower); + $session->param( 'number', $borrower->borrowernumber); + $session->param( 'id', $borrower->userid); + } + foreach my $k (keys %$params) { + $session->param( $k, $params->{$k}); + } + $session->param('ip', '127.0.0.1') unless $session->param('ip'); + $session->param('lasttime', time()) unless $session->param('lasttime'); + Koha::Exceptions::BadParameter->throw(error => "Mandatory CGI::Session parameter 'number' is not defined!") unless $session->param('number'); + $session->flush; + + if ($session->errstr()) { + if($session->errstr() =~ /couldn't open '(.+)'/) { + Koha::Exceptions::File->throw(error => "While trying to mock a CGI::Session, got the following CGI::Session error string '".$session->errstr()."'", path => $1); + } + else { + Koha::Exceptions::Exception->throw(error => "Getting a CGI::Session failed with an unexpected error '$session->errstr()' "); + } + } + + return $session; +} + 1; -- 2.7.4