Bugzilla – Attachment 68390 Details for
Bug 19515
Refactor REST API session mocking to reduce code duplication
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 19515 - Refactor REST API session mocking to reduce ridiculous amounts of duplication
Bug-19515---Refactor-REST-API-session-mocking-to-r.patch (text/plain), 20.81 KB, created by
Olli-Antti Kivilahti
on 2017-10-23 12:30:33 UTC
(
hide
)
Description:
Bug 19515 - Refactor REST API session mocking to reduce ridiculous amounts of duplication
Filename:
MIME Type:
Creator:
Olli-Antti Kivilahti
Created:
2017-10-23 12:30:33 UTC
Size:
20.81 KB
patch
obsolete
>From bb4c8853a7a97d6cbd43af1e71653f3e3b561c89 Mon Sep 17 00:00:00 2001 >From: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> >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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=68390">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 19515
: 68390