From a1bf319829c9d44991daa2fcf2593420f8445c5b Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 3 Oct 2017 19:20:18 -0300 Subject: [PATCH] Bug 19405: Prevent api/v1/holds.t to fail randomly DBD::mysql::st execute failed: Duplicate entry 'cEMggO40gdPLhcVXbpry8x0izO8lHr8NafFIBJwm0D1HgiXA57YR0a0VVxhQBzvn' for key 'userid' [for Statement "INSERT INTO `borrowers` ( `branchcode`, `categorycode`, `flags`, `surname`, `userid`) VALUES ( ?, ?, ?, ?, ? )" with ParamValues: 0='N2ElsY9', 1='Kk8G', 2=80, 3='Test Surname', 4='cEMggO40gdPLhcVXbpry8x0izO8lHr8NafFIBJwm0D1HgiXA57YR0a0VVxhQBzvnnbgezJqmxqwz'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832. DBIx::Class::Storage::DBI::_dbh_execute(): Duplicate entry 'cEMggO40gdPLhcVXbpry8x0izO8lHr8NafFIBJwm0D1HgiXA57YR0a0VVxhQBzvn' for key 'userid' at /kohadevbox/koha/Koha/Object.pm line 121 [18:52:19] t/db_dependent/api/v1/holds.t Reading the code I guess it happens if TestBuilder generates a userid with the size of borrowers.userid (75 chars). In that case the following lines are wrong: $borrower->userid($nopermission->{ userid }."z"); $borrower2->userid($nopermission->{ userid }."x"); $borrower3->userid($nopermission->{ userid }."y"); The 3 patrons will have the same userid. Signed-off-by: Jonathan Druart --- t/db_dependent/api/v1/holds.t | 89 ++++++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 40 deletions(-) diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t index d1f7978f67..05603be894 100644 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -65,49 +65,58 @@ $session_nopermission->param('ip', '127.0.0.1'); $session_nopermission->param('lasttime', time()); $session_nopermission->flush; -my $borrower = Koha::Patron->new; -$borrower->categorycode( $categorycode ); -$borrower->branchcode( $branchcode ); -$borrower->surname("Test Surname"); -$borrower->flags(80); #borrowers and reserveforothers flags -$borrower->userid($nopermission->{ userid }."z"); -$borrower->store; -my $borrowernumber = $borrower->borrowernumber; - -my $borrower2 = Koha::Patron->new; -$borrower2->categorycode( $categorycode ); -$borrower2->branchcode( $branchcode ); -$borrower2->surname("Test Surname 2"); -$borrower2->userid($nopermission->{ userid }."x"); -$borrower2->flags(16); # borrowers flag -$borrower2->store; -my $borrowernumber2 = $borrower2->borrowernumber; - -my $borrower3 = Koha::Patron->new; -$borrower3->categorycode( $categorycode ); -$borrower3->branchcode( $branchcode ); -$borrower3->surname("Test Surname 2"); -$borrower3->userid($nopermission->{ userid }."y"); -$borrower3->flags(64); # reserveforothers flag -$borrower3->store; -my $borrowernumber3 = $borrower3->borrowernumber; +my $patron_1 = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $categorycode, + branchcode => $branchcode, + surname => 'Test Surname', + flags => 80, #borrowers and reserveforothers flags + } + } +); + +my $patron_2 = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $categorycode, + branchcode => $branchcode, + surname => 'Test Surname 2', + flags => 16, # borrowers flag + } + } +); + +my $patron_3 = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $categorycode, + branchcode => $branchcode, + surname => 'Test Surname 3', + flags => 64, # reserveforothers flag + } + } +); # Get sessions my $session = C4::Auth::get_session(''); -$session->param('number', $borrower->borrowernumber); -$session->param('id', $borrower->userid); +$session->param('number', $patron_1->borrowernumber); +$session->param('id', $patron_1->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('number', $patron_2->borrowernumber); +$session2->param('id', $patron_2->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('number', $patron_3->borrowernumber); +$session3->param('id', $patron_3->userid); $session3->param('ip', '127.0.0.1'); $session3->param('lasttime', time()); $session3->flush; @@ -126,18 +135,18 @@ $dbh->do('DELETE FROM issuingrules'); VALUES (?, ?, ?, ?) }, {}, '*', '*', '*', 1); -my $reserve_id = C4::Reserves::AddReserve($branchcode, $borrowernumber, +my $reserve_id = C4::Reserves::AddReserve($branchcode, $patron_1->borrowernumber, $biblionumber, undef, 1, undef, undef, undef, '', $itemnumber); # Add another reserve to be able to change first reserve's rank -my $reserve_id2 = C4::Reserves::AddReserve($branchcode, $borrowernumber2, +my $reserve_id2 = C4::Reserves::AddReserve($branchcode, $patron_2->borrowernumber, $biblionumber, undef, 2, undef, undef, undef, '', $itemnumber); my $suspend_until = DateTime->now->add(days => 10)->ymd; my $expirationdate = DateTime->now->add(days => 10)->ymd; my $post_data = { - borrowernumber => int($borrowernumber), + borrowernumber => int($patron_1->borrowernumber), biblionumber => int($biblionumber), itemnumber => int($itemnumber), branchcode => $branchcode, @@ -164,11 +173,11 @@ subtest "Test endpoints without authentication" => sub { subtest "Test endpoints without permission" => sub { plan tests => 10; - $tx = $t->ua->build_tx(GET => "/api/v1/holds?borrowernumber=$borrowernumber"); + $tx = $t->ua->build_tx(GET => "/api/v1/holds?borrowernumber=" . $patron_1->borrowernumber); $tx->req->cookies({name => 'CGISESSID', value => $session_nopermission->id}); $t->request_ok($tx) # no permission ->status_is(403); - $tx = $t->ua->build_tx(GET => "/api/v1/holds?borrowernumber=$borrowernumber"); + $tx = $t->ua->build_tx(GET => "/api/v1/holds?borrowernumber=" . $patron_1->borrowernumber); $tx->req->cookies({name => 'CGISESSID', value => $session3->id}); $t->request_ok($tx) # reserveforothers permission ->status_is(403); @@ -261,13 +270,13 @@ subtest "Test endpoints with permission" => sub { ->status_is(404) ->json_has('/error'); - $tx = $t->ua->build_tx(GET => "/api/v1/holds?borrowernumber=".$borrower->borrowernumber); + $tx = $t->ua->build_tx(GET => "/api/v1/holds?borrowernumber=" . $patron_1->borrowernumber); $tx->req->cookies({name => 'CGISESSID', value => $session2->id}); # get with borrowers flag $t->request_ok($tx) ->status_is(200) ->json_is([]); - my $inexisting_borrowernumber = $borrowernumber2*2; + my $inexisting_borrowernumber = $patron_2->borrowernumber * 2; $tx = $t->ua->build_tx(GET => "/api/v1/holds?borrowernumber=$inexisting_borrowernumber"); $tx->req->cookies({name => 'CGISESSID', value => $session->id}); $t->request_ok($tx) @@ -286,7 +295,7 @@ subtest "Test endpoints with permission" => sub { ->json_has('/reserve_id'); $reserve_id = $t->tx->res->json->{reserve_id}; - $tx = $t->ua->build_tx(GET => "/api/v1/holds?borrowernumber=$borrowernumber"); + $tx = $t->ua->build_tx(GET => "/api/v1/holds?borrowernumber=" . $patron_1->borrowernumber); $tx->req->cookies({name => 'CGISESSID', value => $session->id}); $t->request_ok($tx) ->status_is(200) -- 2.11.0