From 038b164708068db76d8a82e6f967745f6df3e4ad Mon Sep 17 00:00:00 2001 From: Agustin Moyano Date: Thu, 5 Dec 2019 18:59:17 -0300 Subject: [PATCH] Bug 23571: Add flock to fines.pl This patch adds a lock file for fines.pl. To test: 1. Apply this patch 2. In two separate consoles run misc/cronjobs/fines.pl SUCCESS => The first one runs normally, but the second one fails with a message flock failed for the lock file. 3. Sign off. Sponsored-by: Orex Digital Signed-off-by: Hugo Agud --- misc/cronjobs/fines.pl | 182 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 118 insertions(+), 64 deletions(-) diff --git a/misc/cronjobs/fines.pl b/misc/cronjobs/fines.pl index f04da4b280..1e83e44d00 100755 --- a/misc/cronjobs/fines.pl +++ b/misc/cronjobs/fines.pl @@ -36,15 +36,19 @@ use C4::Overdues; use Getopt::Long; use Carp; use File::Spec; +use Fcntl qw(:flock); use Koha::Calendar; use Koha::DateUtils; use C4::Log; +use constant LOCK_FILENAME => 'fines..LCK'; + my $help; my $verbose; my $output_dir; my $log; +my $use_flock = 1; GetOptions( 'h|help' => \$help, @@ -74,6 +78,26 @@ if ($help) { cronlogaction(); +my ($lockfile, $LockFH); +foreach ( + '/var/lock/koha', + '/var/lock/koha_fines', + '/tmp/koha_fines' +) { + #we try three possibilities (we really want to lock :) + next if !$_; + ($LockFH, $lockfile) = _create_lockfile($_); + last if defined $LockFH; +} +if( !defined $LockFH ) { + print "WARNING: Could not create lock file $lockfile: $!\n"; + print "Verify if directories /var/lock/koha/". C4::Context->config('database').", /var/lock/koha_fines_" . C4::Context->config('database')." or /tmp/koha_fines_" . C4::Context->config('database')." exist and check file permissions\n"; + $use_flock = 0; # we disable file locking now and will continue + # without it + # note that this mimics old behavior (before we used + # the lockfile) +}; + my @borrower_fields = qw(cardnumber categorycode surname firstname email phone address citystate); my @item_fields = qw(itemnumber barcode date_due); @@ -98,78 +122,83 @@ if ($filename) { } my $counted = 0; my $overdues = Getoverdues(); -for my $overdue ( @{$overdues} ) { - next if $overdue->{itemlost}; - - if ( !defined $overdue->{borrowernumber} ) { - carp -"ERROR in Getoverdues : issues.borrowernumber IS NULL. Repair 'issues' table now! Skipping record.\n"; - next; - } - my $borrower = BorType( $overdue->{borrowernumber} ); - my $branchcode = - ( $control eq 'ItemHomeLibrary' ) ? $overdue->{homebranch} - : ( $control eq 'PatronLibrary' ) ? $borrower->{branchcode} - : $overdue->{branchcode}; - -# In final case, CircControl must be PickupLibrary. (branchcode comes from issues table here). - if ( !exists $is_holiday{$branchcode} ) { - $is_holiday{$branchcode} = set_holiday( $branchcode, $today ); - } +if(_flock($LockFH, LOCK_EX|LOCK_NB)) { + for my $overdue ( @{$overdues} ) { + next if $overdue->{itemlost}; + + if ( !defined $overdue->{borrowernumber} ) { + carp + "ERROR in Getoverdues : issues.borrowernumber IS NULL. Repair 'issues' table now! Skipping record.\n"; + next; + } + my $borrower = BorType( $overdue->{borrowernumber} ); + my $branchcode = + ( $control eq 'ItemHomeLibrary' ) ? $overdue->{homebranch} + : ( $control eq 'PatronLibrary' ) ? $borrower->{branchcode} + : $overdue->{branchcode}; + + # In final case, CircControl must be PickupLibrary. (branchcode comes from issues table here). + if ( !exists $is_holiday{$branchcode} ) { + $is_holiday{$branchcode} = set_holiday( $branchcode, $today ); + } - my $datedue = dt_from_string( $overdue->{date_due} ); - if ( DateTime->compare( $datedue, $today ) == 1 ) { - next; # not overdue - } - ++$counted; - - my ( $amount, $unitcounttotal, $unitcount ) = - CalcFine( $overdue, $borrower->{categorycode}, - $branchcode, $datedue, $today ); - - # Don't update the fine if today is a holiday. - # This ensures that dropbox mode will remove the correct amount of fine. - if ( $mode eq 'production' && !$is_holiday{$branchcode} ) { - if ( $amount && $amount > 0 ) { - UpdateFine( - { - issue_id => $overdue->{issue_id}, - itemnumber => $overdue->{itemnumber}, - borrowernumber => $overdue->{borrowernumber}, - amount => $amount, - due => output_pref($datedue), - } - ); + my $datedue = dt_from_string( $overdue->{date_due} ); + if ( DateTime->compare( $datedue, $today ) == 1 ) { + next; # not overdue + } + ++$counted; + + my ( $amount, $unitcounttotal, $unitcount ) = + CalcFine( $overdue, $borrower->{categorycode}, + $branchcode, $datedue, $today ); + + # Don't update the fine if today is a holiday. + # This ensures that dropbox mode will remove the correct amount of fine. + if ( $mode eq 'production' && !$is_holiday{$branchcode} ) { + if ( $amount && $amount > 0 ) { + UpdateFine( + { + issue_id => $overdue->{issue_id}, + itemnumber => $overdue->{itemnumber}, + borrowernumber => $overdue->{borrowernumber}, + amount => $amount, + due => output_pref($datedue), + } + ); + } + } + if ($filename) { + my @cells; + push @cells, + map { defined $borrower->{$_} ? $borrower->{$_} : q{} } + @borrower_fields; + push @cells, map { $overdue->{$_} } @item_fields; + push @cells, $unitcounttotal, $amount; + say {$fh} join $delim, @cells; } } - if ($filename) { - my @cells; - push @cells, - map { defined $borrower->{$_} ? $borrower->{$_} : q{} } - @borrower_fields; - push @cells, map { $overdue->{$_} } @item_fields; - push @cells, $unitcounttotal, $amount; - say {$fh} join $delim, @cells; + if ($filename){ + close $fh; } -} -if ($filename){ - close $fh; -} -if ($verbose) { - my $overdue_items = @{$overdues}; - print <<"EOM"; -Fines assessment -- $today + if ($verbose) { + my $overdue_items = @{$overdues}; + print <<"EOM"; + Fines assessment -- $today EOM - if ($filename) { - say "Saved to $filename"; - } - print <<"EOM"; -Number of Overdue Items: - counted $overdue_items - reported $counted + if ($filename) { + say "Saved to $filename"; + } + print <<"EOM"; + Number of Overdue Items: + counted $overdue_items + reported $counted EOM + } + _flock($LockFH, LOCK_UN); +} else { + print "Skipping fines assessment because flock failed on $lockfile: $!\n"; } sub set_holiday { @@ -196,3 +225,28 @@ sub get_filename { } return $name; } + +sub _create_lockfile { #returns undef on failure + my $dir= shift; + unless (-d $dir) { + eval { mkpath($dir, 0, oct(755)) }; + return if $@; + } + return if !open my $fh, q{>}, $dir.'/'.LOCK_FILENAME; + return ( $fh, $dir.'/'.LOCK_FILENAME ); +} + +sub _flock { + my ($fh, $op)= @_; + return 1 if !$use_flock; + + #check if flock is present; if not, you will have a fatal error + my $lock_acquired = eval { flock($fh, $op) }; + + # assuming that $fh and $op are fine(..), an undef $lock_acquired + # means no flock + $use_flock = defined($lock_acquired) ? 1 : 0; + print "Warning: flock could not be used!\n" if !$use_flock; + return 1 if !$use_flock; + return $lock_acquired; +} -- 2.11.0