@@ -, +, @@ --- C4/Members.pm | 180 +++++++++++++++++++++++--------- installer/data/mysql/updatedatabase.pl | 101 ++++++++++++++++++ kohaversion.pl | 2 +- 3 files changed, 230 insertions(+), 53 deletions(-) --- a/C4/Members.pm +++ a/C4/Members.pm @@ -881,72 +881,148 @@ sub changepassword { =head2 fixup_cardnumber -Warning: The caller is responsible for locking the members table in write -mode, to avoid database corruption. +Previously, there was a gap in time between the insertion of the +card number into borrowers and the generation. This gap meant that +the same card number could be calculated by two different processes +resulting in two different members being added with the same card +number. -=cut +By serializing the cardnumber requests, at worst we may skip a +cardnumber, rather than duplicate one. No locking required, +because the INSERT into cardnumber_sequence is atomic! -use vars qw( @weightings ); -my @weightings = ( 8, 4, 6, 3, 5, 2, 1 ); +The reason no duplicates happen is because the sequence of +cardnumbers is calculated from the previous one, and recalculating +would generate exactly the same value. So even if two processes +are calculating the same cardnumber for a given UID record, it +will result in the same value. + +=cut sub fixup_cardnumber { my ($cardnumber) = @_; - my $autonumber_members = C4::Context->boolean_preference('autoMemberNum') || 0; + my @weightings = ( 8, 4, 6, 3, 5, 2, 1 ); + my ( + $autonumber_members, + $checkdigit, + $dbh, + $sth, + $my_UID, + $current_UID, + $previous_UID, + $previous_cardnumber, + $current_cardnumber, + $sum, + $i, + $temp1, + $temp2, + $rem, + $new_cardnumber, + ); # Find out whether member numbers should be generated # automatically. Should be either "1" or something else. # Defaults to "0", which is interpreted as "no". + $autonumber_members = C4::Context->boolean_preference('autoMemberNum') || 0; - # if ($cardnumber !~ /\S/ && $autonumber_members) { + # if we don't auto-generate, then there is nothing to fixup. + # So return what was passed. ($autonumber_members) or return $cardnumber; - my $checkdigit = C4::Context->preference('checkdigit'); - my $dbh = C4::Context->dbh; - if ( $checkdigit and $checkdigit eq 'katipo' ) { - - # if checkdigit is selected, calculate katipo-style cardnumber. - # otherwise, just use the max() - # purpose: generate checksum'd member numbers. - # We'll assume we just got the max value of digits 2-8 of member #'s - # from the database and our job is to increment that by one, - # determine the 1st and 9th digits and return the full string. - my $sth = $dbh->prepare( - "select max(substring(borrowers.cardnumber,2,7)) as new_num from borrowers" - ); - $sth->execute; - my $data = $sth->fetchrow_hashref; - $cardnumber = $data->{new_num}; - if ( !$cardnumber ) { # If DB has no values, - $cardnumber = 1000000; # start at 1000000 - } else { - $cardnumber += 1; - } - - my $sum = 0; - for ( my $i = 0 ; $i < 8 ; $i += 1 ) { - # read weightings, left to right, 1 char at a time - my $temp1 = $weightings[$i]; - # sequence left to right, 1 char at a time - my $temp2 = substr( $cardnumber, $i, 1 ); - - # mult each char 1-7 by its corresponding weighting - $sum += $temp1 * $temp2; + # determine which kind of auto-generate we are doing + $checkdigit = C4::Context->preference('checkdigit'); + + # grab a database handle. + $dbh = C4::Context->dbh; + + # first we need to show our intent to generate a cardnumber. + $sth = $dbh->prepare(" +INSERT INTO cardnumber_sequence + (cardnumber,checkdigit) VALUES + ('',?); + "); + $sth->execute($checkdigit); + $my_UID = $dbh->last_insert_id(undef,undef,'cardnumber_sequence','UID'); + + # next we need to find the first ungenerated cardnumber. + # if we don't find any, another process filled them for us! + # if we find some, then we need to start generating. + $sth = $dbh->prepare(" +SELECT MIN(UID) AS UID FROM cardnumber_sequence +WHERE cardnumber='' AND checkdigit=?; + "); + $sth->execute($checkdigit); + $current_UID = $sth->fetchrow(); + + # if there is a current_UID, we need to calculate + while ($current_UID) { + + # so find the previous UID + $sth = $dbh->prepare(" +SELECT MAX(UID) AS UID FROM cardnumber_sequence +WHERE UID < ? AND checkdigit=?; + "); + $sth->execute($current_UID,$checkdigit); + $previous_UID = $sth->fetchrow(); + + # grab the previous cardnumber + $sth = $dbh->prepare(" +SELECT cardnumber FROM cardnumber_sequence +WHERE UID = ?; + "); + $sth->execute($previous_UID); + $previous_cardnumber = $sth->fetchrow(); + + # now calculate the current cardnumber. + if ($checkdigit && $checkdigit eq 'katipo') { + $current_cardnumber = substr($previous_cardnumber,1,7); + $current_cardnumber = $current_cardnumber+1; + $sum = 0; + foreach $i (1..7) { + # read weightings, left to right, 1 char at a time + $temp1 = $weightings[$i-1]; + + # sequence left to right, 1 char at a time + $temp2 = substr( $current_cardnumber, $i-1, 1 ); + + # mult each char 1-7 by its corresponding weighting + $sum += $temp1 * $temp2; + } + $rem = ( $sum % 11 ); + $rem = 'X' if $rem == 10; + $current_cardnumber = "V$current_cardnumber$rem"; + } + else { + $current_cardnumber = $previous_cardnumber+1; } - my $rem = ( $sum % 11 ); - $rem = 'X' if $rem == 10; - - return "V$cardnumber$rem"; - } else { - - my $sth = $dbh->prepare( - 'SELECT MAX( CAST( cardnumber AS SIGNED ) ) FROM borrowers WHERE cardnumber REGEXP "^-?[0-9]+$"' - ); - $sth->execute; - my ($result) = $sth->fetchrow; - return $result + 1; - } - return $cardnumber; # just here as a fallback/reminder + # update the current_UID record with the generated cardnumber. + $sth = $dbh->prepare(" +UPDATE cardnumber_sequence SET cardnumber=? WHERE UID=?; + "); + $sth->execute($current_cardnumber,$current_UID); + + # we need to find the next ungenerated cardnumber. + # if we don't find any, we are done generating everything. + $sth = $dbh->prepare(" +SELECT MIN(UID) AS UID FROM cardnumber_sequence +WHERE cardnumber='' and checkdigit=?; + "); + $sth->execute($checkdigit); + $current_UID = $sth->fetchrow(); + + } + + # need already know our UID ($my_UID) + # let's grab the cardnumber. + $sth = $dbh->prepare(" +SELECT cardnumber FROM cardnumber_sequence +WHERE UID=?; + "); + $sth->execute($my_UID); + $new_cardnumber = $sth->fetchrow(); + + return $new_cardnumber; } =head2 GetGuarantees --- a/installer/data/mysql/updatedatabase.pl +++ a/installer/data/mysql/updatedatabase.pl @@ -7010,6 +7010,107 @@ CREATE TABLE IF NOT EXISTS borrower_files ( SetVersion($DBversion); } +$DBversion = "3.13.00.XXX"; +if ( CheckVersion($DBversion) ) { + + # Create new table used to serialize cardnumber requests. + $dbh->do(" +CREATE TABLE IF NOT EXISTS cardnumber_sequence ( + UID INT(16) NOT NULL AUTO_INCREMENT, + cardnumber VARCHAR(16) NOT NULL, + checkdigit TEXT NOT NULL, + PRIMARY KEY (UID) +) ENGINE=InnoDB DEFAULT CHARSET=utf8; + "); + + my ( + $dbh, + $sth, + @cardnumbers_katipo, + @cardnumbers_none, + @cardnumbers, + $cardnumber, + $data, + ); + + # Add 2 records for the case that borrowers is empty. + $dbh = C4::Context->dbh; + $sth = $dbh->prepare(" +INSERT INTO cardnumber_sequence + (cardnumber,checkdigit) VALUES + ('0','none'); + "); + $sth->execute(); + + $sth = $dbh->prepare(" +INSERT INTO cardnumber_sequence + (cardnumber,checkdigit) VALUES + ('V09999992','katipo'); + "); + $sth->execute(); + + # Convert existing borrowers records into records that + # were supposedly generated. + + # grab checkdigit = katipo records + $sth = $dbh->prepare(" +SELECT cardnumber FROM borrowers +WHERE SUBSTR(cardnumber,1,1) + NOT IN ('0','1','2','3','4','5','6','7','8','9') +ORDER BY cardnumber; + "); + $sth->execute(); + $data = $sth->fetchrow_hashref(); + $cardnumber = undef; + $cardnumber = $data->{'cardnumber'} if $data->{'cardnumber'}; + while ($data->{'cardnumber'}) { + push @cardnumbers_katipo,$data->{'cardnumber'}; + $data = $sth->fetchrow_hashref(); + $cardnumber = undef; + $cardnumber = $data->{'cardnumber'} if $data->{'cardnumber'}; + } + + # grab checkdigit = none records + $sth = $dbh->prepare(" +SELECT cardnumber FROM borrowers +WHERE SUBSTR(cardnumber,1,1) + IN ('0','1','2','3','4','5','6','7','8','9') +ORDER BY CAST(cardnumber AS SIGNED); + "); + $sth->execute(); + $data = $sth->fetchrow_hashref(); + $cardnumber = undef; + $cardnumber = $data->{'cardnumber'} if $data->{'cardnumber'}; + while ($data->{'cardnumber'}) { + push @cardnumbers_none,$data->{'cardnumber'}; + $data = $sth->fetchrow_hashref(); + $cardnumber = undef; + $cardnumber = $data->{'cardnumber'} if $data->{'cardnumber'}; + } + + # add the card numbers in. + foreach $cardnumber (@cardnumbers_none) { + $sth = $dbh->prepare(" +INSERT INTO cardnumber_sequence + (cardnumber,checkdigit) VALUES + (?,'none'); + "); + $sth->execute($cardnumber); + } + + foreach $cardnumber (@cardnumbers_katipo) { + $sth = $dbh->prepare(" +INSERT INTO cardnumber_sequence + (cardnumber,checkdigit) VALUES + (?,'katipo'); + "); + $sth->execute($cardnumber); + } + + print "Upgrade to $DBversion done (Bug 10454: Duplicate card numbers may be generated)\n"; + SetVersion($DBversion); +} + =head1 FUNCTIONS =head2 TableExists($table) --- a/kohaversion.pl +++ a/kohaversion.pl @@ -16,7 +16,7 @@ the kohaversion is divided in 4 parts : use strict; sub kohaversion { - our $VERSION = '3.13.00.008'; + our $VERSION = '3.13.00.XXX'; # version needs to be set this way # so that it can be picked up by Makefile.PL # during install --