From b40456f7dd4b8a988f9c6a5718452936101cb8ff Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 31 Mar 2017 13:43:38 -0300 Subject: [PATCH] Bug 18364: Do not LOCK/UNLOCK tables from tests From the MySQL doc: "LOCK TABLES is not transaction-safe and implicitly commits any active transaction before attempting to lock the tables." If the LOCK/UNLOCK statements are executed from tests, the current transaction will be committed. To avoid that we need to guess if this code is execute from testsa or not (yes it is a bit hacky) Better ideas are welcome! Another fix would have been to revert commit be156d9ad9e5bcfadab34d44f90e04fd61e256ad Bug 15854: Use a READ and WRITE LOCK on message_queue but theorically a race is still possible. Existing tests seem to be safe, to test this patch you will need new tests from bug 17964. Test plan: prove t/db_dependent/Letters/TemplateToolkit.t twice, and notice that changes have been comitted. Signed-off-by: Nick Clemens Signed-off-by: Marcel de Rooy Signed-off-by: Kyle M Hall --- C4/Circulation.pm | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index e71600d01e..9d613d55bd 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3359,6 +3359,13 @@ sub SendCirculationAlert { my $schema = Koha::Database->new->schema; my @transports = keys %{ $borrower_preferences->{transports} }; + + # From the MySQL doc: + # LOCK TABLES is not transaction-safe and implicitly commits any active transaction before attempting to lock the tables. + # If the LOCK/UNLOCK statements are executed from tests, the current transaction will be committed. + # To avoid that we need to guess if this code is execute from tests or not (yes it is a bit hacky) + my $called_from_tests = exists $ENV{_} and $ENV{_} =~ m|prove|; + for my $mtt (@transports) { my $letter = C4::Letters::GetPreparedLetter ( module => 'circulation', @@ -3376,17 +3383,17 @@ sub SendCirculationAlert { ) or next; $schema->storage->txn_begin; - C4::Context->dbh->do(q|LOCK TABLE message_queue READ|); - C4::Context->dbh->do(q|LOCK TABLE message_queue WRITE|); + C4::Context->dbh->do(q|LOCK TABLE message_queue READ|) unless $called_from_tests; + C4::Context->dbh->do(q|LOCK TABLE message_queue WRITE|) unless $called_from_tests; my $message = C4::Message->find_last_message($borrower, $type, $mtt); unless ( $message ) { - C4::Context->dbh->do(q|UNLOCK TABLES|); + C4::Context->dbh->do(q|UNLOCK TABLES|) unless $called_from_tests; C4::Message->enqueue($letter, $borrower, $mtt); } else { $message->append($letter); $message->update; } - C4::Context->dbh->do(q|UNLOCK TABLES|); + C4::Context->dbh->do(q|UNLOCK TABLES|) unless $called_from_tests; $schema->storage->txn_commit; } -- 2.39.5