From 8c07ba4c80415d579779ddb0b1f344f07d15b169 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 29 Jan 2021 12:50:27 +0100 Subject: [PATCH] Bug 25078: Put db revs into different files to handle them better This patch suggests to stop using updatedatabase.pl to add new DB revs. Each DB rev will be in a separate pl files (installer/data/mysql/db_revs). The switch should ideally be done from 21.06.00.000. Each DBrev is executed in a try block and a transaction. If something went wrong, the whole DB rev is rolled back. Why do /var/log/koha/kohadev/updatedatabase_*.log (not -error) contain Status: 500 Content-type: text/html

Software error:

etc. Test plan: - git checkout c4b4db21d25 (master on 2021-07-08) - Set the version syspref to 21.0500000: > update systempreferences set value="21.0500000" where variable="version"; - Apply "Bug 25078: [DO NOT PUSH] DB revs for testing" (restart_all) - Read the different DBrevs created as examples - Make sure the different use cases are covered - execute the updatedatabase script (CLI) - Set the version syspref to 21.0500000 - Update the DB from the UI - Set the version syspref to 21.0500000 - execute the updatedatabase script with the --force parameter (for testing purpose) Signed-off-by: Martin Renvoize Signed-off-by: Kyle M Hall Signed-off-by: Jonathan Druart --- C4/Installer.pm | 235 +++++++++++++++++- installer/data/mysql/db_revs/210600000.pl | 8 + installer/data/mysql/db_revs/skeleton.pl | 13 + installer/data/mysql/updatedatabase.pl | 171 ++----------- installer/install.pl | 46 +--- .../prog/en/modules/installer/step3.tt | 40 +-- 6 files changed, 307 insertions(+), 206 deletions(-) create mode 100644 installer/data/mysql/db_revs/210600000.pl create mode 100644 installer/data/mysql/db_revs/skeleton.pl diff --git a/C4/Installer.pm b/C4/Installer.pm index 868a664554..4b62d5deac 100644 --- a/C4/Installer.pm +++ b/C4/Installer.pm @@ -19,10 +19,12 @@ package C4::Installer; use Modern::Perl; -use Encode; +use Try::Tiny; +use Encode qw( encode is_utf8 ); use DBIx::RunSQL; use YAML::XS; use C4::Context; +use Koha::Schema; use DBI; use Koha; @@ -30,7 +32,7 @@ use vars qw(@ISA @EXPORT); BEGIN { require Exporter; @ISA = qw( Exporter ); - push @EXPORT, qw( primary_key_exists unique_key_exists foreign_key_exists index_exists column_exists TableExists marc_framework_sql_list); + push @EXPORT, qw( primary_key_exists unique_key_exists foreign_key_exists index_exists column_exists TableExists marc_framework_sql_list TransformToNum CheckVersion sanitize_zero_date update get_db_entries ); }; =head1 NAME @@ -690,6 +692,235 @@ sub TableExists { # Could be renamed table_exists for consistency return 0; } +sub version_from_file { + my $file = shift; + return unless $file =~ m|(^\|/)(\d{2})(\d{2})(\d{2})(\d{3}).pl$|; + return sprintf "%s.%s.%s.%s", $2, $3, $4, $5; +} + +sub get_db_entries { + my $db_revs_dir = C4::Context->config('intranetdir') . '/installer/data/mysql/db_revs'; + opendir my $dh, $db_revs_dir or die "Cannot open $db_revs_dir dir ($!)"; + my @files = sort grep { m|\.pl$| && ! m|skeleton\.pl$| } readdir $dh; + my @need_update; + for my $file ( @files ) { + my $version = version_from_file( $file ); + + unless ( $version ) { + warn "Invalid db_rev found: " . $file; + next + } + + next unless CheckVersion( $version ); + + push @need_update, sprintf( "%s/%s", $db_revs_dir, $file ); + } + return \@need_update; +} + +sub update { + my ( $files, $params ) = @_; + + my $force = $params->{force} || 0; + + my $schema = Koha::Database->new->schema; + my ( @done, @errors ); + for my $file ( @$files ) { + + my $db_rev = do $file; + + my $error; + + try { + $schema->txn_do( + sub { + $db_rev->{up}->(); + } + ); + } catch { + $error = $_; + }; + + my $db_entry = { + bug_number => $db_rev->{bug_number}, + description => ( ref $db_rev->{description} eq 'CODE' ) + ? $db_rev->{description}->() + : $db_rev->{description}, + version => version_from_file($file), + time => POSIX::strftime( "%H:%M:%S", localtime ), + }; + $db_entry->{output} = output_version( { %$db_entry, done => !$error } ); + + if ( $error ) { + push @errors, { %$db_entry, error => $error }; + $force ? next : last ; + # We stop the update if an error occurred! + } + + SetVersion($db_entry->{version}); + push @done, $db_entry; + } + return { success => \@done, error => \@errors }; +} + +sub output_version { + my ( $db_entry ) = @_; + + my $descriptions = $db_entry->{description}; + my $DBversion = $db_entry->{version}; + my $bug_number = $db_entry->{bug_number}; + my $time = $db_entry->{time}; + my $done = $db_entry->{done} ? "done" : "failed"; + + unless ( ref($descriptions) ) { + $descriptions = [ $descriptions ]; + } + + my $first = 1; + my @output; + for my $description ( @$descriptions ) { + if ( @$descriptions > 1 ) { + if ( $first ) { + unless ( $bug_number ) { + push @output, sprintf "Upgrade to %s %s [%s]:", $DBversion, $done, $time; + } else { + push @output, sprintf "Upgrade to %s %s [%s]: Bug %5s", $DBversion, $done, $time, $bug_number; + } + } + push @output, sprintf "\t\t\t\t\t\t - %s", $description; + } else { + unless ( $bug_number ) { + push @output, sprintf "Upgrade to %s %s [%s]: %s", $DBversion, $done, $time, $description; + } else { + push @output, sprintf "Upgrade to %s %s [%s]: Bug %5s - %s", $DBversion, $done, $time, $bug_number, $description; + } + } + $first = 0; + } + return \@output; +} + +=head2 DropAllForeignKeys($table) + +Drop all foreign keys of the table $table + +=cut + +sub DropAllForeignKeys { + my ($table) = @_; + # get the table description + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare("SHOW CREATE TABLE $table"); + $sth->execute; + my $vsc_structure = $sth->fetchrow; + # split on CONSTRAINT keyword + my @fks = split /CONSTRAINT /,$vsc_structure; + # parse each entry + foreach (@fks) { + # isolate what is before FOREIGN KEY, if there is something, it's a foreign key to drop + $_ = /(.*) FOREIGN KEY.*/; + my $id = $1; + if ($id) { + # we have found 1 foreign, drop it + $dbh->do("ALTER TABLE $table DROP FOREIGN KEY $id"); + $id=""; + } + } +} + + +=head2 TransformToNum + +Transform the Koha version from a 4 parts string +to a number, with just 1 . + +=cut + +sub TransformToNum { + my $version = shift; + # remove the 3 last . to have a Perl number + $version =~ s/(.*\..*)\.(.*)\.(.*)/$1$2$3/; + # three X's at the end indicate that you are testing patch with dbrev + # change it into 999 + # prevents error on a < comparison between strings (should be: lt) + $version =~ s/XXX$/999/; + return $version; +} + +=head2 SetVersion + +set the DBversion in the systempreferences + +=cut + +sub SetVersion { + return if $_[0]=~ /XXX$/; + #you are testing a patch with a db revision; do not change version + my $kohaversion = TransformToNum($_[0]); + my $dbh = C4::Context->dbh; + if (C4::Context->preference('Version')) { + my $finish=$dbh->prepare("UPDATE systempreferences SET value=? WHERE variable='Version'"); + $finish->execute($kohaversion); + } else { + my $finish=$dbh->prepare("INSERT into systempreferences (variable,value,explanation) values ('Version',?,'The Koha database version. WARNING: Do not change this value manually, it is maintained by the webinstaller')"); + $finish->execute($kohaversion); + } + C4::Context::clear_syspref_cache(); # invalidate cached preferences +} + +sub NewVersion { + # void FIXME replace me +} + +=head2 CheckVersion + +Check whether a given update should be run when passed the proposed version +number. The update will always be run if the proposed version is greater +than the current database version and less than or equal to the version in +kohaversion.pl. The update is also run if the version contains XXX, though +this behavior will be changed following the adoption of non-linear updates +as implemented in bug 7167. + +=cut + +sub CheckVersion { + my ($proposed_version) = @_; + my $version_number = TransformToNum($proposed_version); + + # The following line should be deleted when bug 7167 is pushed + return 1 if ( $proposed_version =~ m/XXX/ ); + + if ( C4::Context->preference("Version") < $version_number + && $version_number <= TransformToNum( $Koha::VERSION ) ) + { + return 1; + } + + return 0; +} + +sub sanitize_zero_date { + my ( $table_name, $column_name ) = @_; + + my $dbh = C4::Context->dbh; + + my (undef, $datatype) = $dbh->selectrow_array(qq| + SHOW COLUMNS FROM $table_name WHERE Field = ?|, undef, $column_name); + + if ( $datatype eq 'date' ) { + $dbh->do(qq| + UPDATE $table_name + SET $column_name = NULL + WHERE CAST($column_name AS CHAR(10)) = '0000-00-00'; + |); + } else { + $dbh->do(qq| + UPDATE $table_name + SET $column_name = NULL + WHERE CAST($column_name AS CHAR(19)) = '0000-00-00 00:00:00'; + |); + } +} =head1 AUTHOR diff --git a/installer/data/mysql/db_revs/210600000.pl b/installer/data/mysql/db_revs/210600000.pl new file mode 100644 index 0000000000..fecc9f081f --- /dev/null +++ b/installer/data/mysql/db_revs/210600000.pl @@ -0,0 +1,8 @@ +use Modern::Perl; +use utf8; + +{ + bug_number => undef, + description => ["🎵 Run, rabbit run. 🎶", "Dig that hole, forget the sun,", "And when at last the work is done", "Don't sit down it's time to dig another one."], + up => sub {}, +} diff --git a/installer/data/mysql/db_revs/skeleton.pl b/installer/data/mysql/db_revs/skeleton.pl new file mode 100644 index 0000000000..76b4991ad2 --- /dev/null +++ b/installer/data/mysql/db_revs/skeleton.pl @@ -0,0 +1,13 @@ +use Modern::Perl; + +{ + bug_number => "BUG_NUMBER", + description => "A single line description", + # description => ["Multi", "lines", "description"], + # description => sub { return ["Your dynamic description"] }, + up => sub { + my $dbh = C4::Context->dbh; + # Do you stuffs here + $dbh->do(q{}); + }, +} diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 11f61583eb..e9f2a538c2 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -34,6 +34,7 @@ use feature 'say'; # CPAN modules use DBI; use Getopt::Long; +use Encode qw( encode_utf8 ); # Koha modules use C4::Context; use C4::Installer; @@ -59,10 +60,11 @@ my ( my $schema = Koha::Database->new()->schema(); -my $silent; +my ( $silent, $force ); GetOptions( - 's' =>\$silent - ); + 's' => \$silent, + 'force' => \$force, +); my $dbh = C4::Context->dbh; $|=1; # flushes output @@ -24281,9 +24283,17 @@ if( CheckVersion( $DBversion ) ) { NewVersion( $DBversion, "", "Koha 21.05.00 release" ); } -$DBversion = '21.06.00.000'; -if( CheckVersion( $DBversion ) ) { - NewVersion( $DBversion, "", ["🎵 Run, rabbit run. 🎶", "Dig that hole, forget the sun,", "And when at last the work is done", "Don't sit down it's time to dig another one."] ); +unless ( $ENV{HTTP_HOST} ) { # Is that correct? + my $files = C4::Installer::get_db_entries; + my $report = update( $files, { force => $force } ); + + for my $s ( @{ $report->{success} } ) { + say Encode::encode_utf8(join "\n", @{$s->{output}}); + } + for my $e ( @{ $report->{error} } ) { + say Encode::encode_utf8(join "\n", @{$e->{output}}); + say Encode::encode_utf8("ERROR - " . $e->{error}); + } } # SEE bug 13068 @@ -24304,153 +24314,4 @@ foreach my $file ( sort readdir $dirh ) { } } -=head1 FUNCTIONS - -=head2 DropAllForeignKeys($table) - -Drop all foreign keys of the table $table - -=cut - -sub DropAllForeignKeys { - my ($table) = @_; - # get the table description - my $sth = $dbh->prepare("SHOW CREATE TABLE $table"); - $sth->execute; - my $vsc_structure = $sth->fetchrow; - # split on CONSTRAINT keyword - my @fks = split /CONSTRAINT /,$vsc_structure; - # parse each entry - foreach (@fks) { - # isolate what is before FOREIGN KEY, if there is something, it's a foreign key to drop - $_ = /(.*) FOREIGN KEY.*/; - my $id = $1; - if ($id) { - # we have found 1 foreign, drop it - $dbh->do("ALTER TABLE $table DROP FOREIGN KEY $id"); - $id=""; - } - } -} - - -=head2 TransformToNum - -Transform the Koha version from a 4 parts string -to a number, with just 1 . - -=cut - -sub TransformToNum { - my $version = shift; - # remove the 3 last . to have a Perl number - $version =~ s/(.*\..*)\.(.*)\.(.*)/$1$2$3/; - # three X's at the end indicate that you are testing patch with dbrev - # change it into 999 - # prevents error on a < comparison between strings (should be: lt) - $version =~ s/XXX$/999/; - return $version; -} - -=head2 SetVersion - -set the DBversion in the systempreferences - -=cut - -sub SetVersion { - return if $_[0]=~ /XXX$/; - #you are testing a patch with a db revision; do not change version - my $kohaversion = TransformToNum($_[0]); - if (C4::Context->preference('Version')) { - my $finish=$dbh->prepare("UPDATE systempreferences SET value=? WHERE variable='Version'"); - $finish->execute($kohaversion); - } else { - my $finish=$dbh->prepare("INSERT into systempreferences (variable,value,explanation) values ('Version',?,'The Koha database version. WARNING: Do not change this value manually, it is maintained by the webinstaller')"); - $finish->execute($kohaversion); - } - C4::Context::clear_syspref_cache(); # invalidate cached preferences -} - -sub NewVersion { - my ( $DBversion, $bug_number, $descriptions ) = @_; - - SetVersion($DBversion); - - unless ( ref($descriptions) ) { - $descriptions = [ $descriptions ]; - } - my $first = 1; - my $time = POSIX::strftime("%H:%M:%S",localtime); - for my $description ( @$descriptions ) { - if ( @$descriptions > 1 ) { - if ( $first ) { - unless ( $bug_number ) { - say sprintf "Upgrade to %s done [%s]: %s", $DBversion, $time, $description; - } else { - say sprintf "Upgrade to %s done [%s]: Bug %5s - %s", $DBversion, $time, $bug_number, $description; - } - } else { - say sprintf "\t\t\t\t\t\t - %s", $description; - } - } else { - unless ( $bug_number ) { - say sprintf "Upgrade to %s done [%s]: %s", $DBversion, $time, $description; - } else { - say sprintf "Upgrade to %s done [%s]: Bug %5s - %s", $DBversion, $time, $bug_number, $description; - } - } - $first = 0; - } -} - -=head2 CheckVersion - -Check whether a given update should be run when passed the proposed version -number. The update will always be run if the proposed version is greater -than the current database version and less than or equal to the version in -kohaversion.pl. The update is also run if the version contains XXX, though -this behavior will be changed following the adoption of non-linear updates -as implemented in bug 7167. - -=cut - -sub CheckVersion { - my ($proposed_version) = @_; - my $version_number = TransformToNum($proposed_version); - - # The following line should be deleted when bug 7167 is pushed - return 1 if ( $proposed_version =~ m/XXX/ ); - - if ( C4::Context->preference("Version") < $version_number - && $version_number <= TransformToNum( $Koha::VERSION ) ) - { - return 1; - } - else { - return 0; - } -} - -sub sanitize_zero_date { - my ( $table_name, $column_name ) = @_; - - my (undef, $datatype) = $dbh->selectrow_array(qq| - SHOW COLUMNS FROM $table_name WHERE Field = ?|, undef, $column_name); - - if ( $datatype eq 'date' ) { - $dbh->do(qq| - UPDATE $table_name - SET $column_name = NULL - WHERE CAST($column_name AS CHAR(10)) = '0000-00-00'; - |); - } else { - $dbh->do(qq| - UPDATE $table_name - SET $column_name = NULL - WHERE CAST($column_name AS CHAR(19)) = '0000-00-00 00:00:00'; - |); - } -} - exit; diff --git a/installer/install.pl b/installer/install.pl index c87492bdeb..3547d6a564 100755 --- a/installer/install.pl +++ b/installer/install.pl @@ -393,41 +393,17 @@ elsif ( $step && $step == 3 ) { chk_log( $logdir, "updatedatabase-error_$filename_suffix" ) ); - my $cmd = C4::Context->config("intranetdir") - . "/installer/data/$info{dbms}/updatedatabase.pl >> $logfilepath 2>> $logfilepath_errors"; - - system($cmd ); - - my $fh; - open( $fh, "<:encoding(utf-8)", $logfilepath ) - or die "Cannot open log file $logfilepath: $!"; - my @report = <$fh>; - close $fh; - if (@report) { - $template->param( update_report => - [ map { { line => $_ =~ s/\t/  /gr } } split( /\n/, join( '', @report ) ) ] - ); - $template->param( has_update_succeeds => 1 ); - } - else { - eval { `rm $logfilepath` }; - } - open( $fh, "<:encoding(utf-8)", $logfilepath_errors ) - or die "Cannot open log file $logfilepath_errors: $!"; - @report = <$fh>; - close $fh; - if (@report) { - $template->param( update_errors => - [ map { { line => $_ } } split( /\n/, join( '', @report ) ) ] - ); - $template->param( has_update_errors => 1 ); - warn -"The following errors were returned while attempting to run the updatedatabase.pl script:\n"; - foreach my $line (@report) { warn "$line\n"; } - } - else { - eval { `rm $logfilepath_errors` }; - } + my $updatedatabase_path = C4::Context->config("intranetdir") + . "/installer/data/$info{dbms}/updatedatabase.pl"; + + my $db_entries = get_db_entries(); + my $report = update( $db_entries ); + + # FIXME restore log to logfilepath and logfilepath_errors + + $template->param( success => $report->{success}, error => $report->{error} ); + + #warn "The following errors were returned while attempting to run the updatedatabase.pl script:\n"; #FIXME restore this $template->param( $op => 1 ); } else { diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt index 789451f05c..04f9976c56 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt @@ -244,31 +244,43 @@ [% IF ( updatestructure ) %]

Updating database structure

- [% IF ( has_update_succeeds ) %] -

Update report :

+ [% IF success %] +

Update report:

    - [% FOREACH l IN update_report %] - [% SET line = l.line %] - [% IF line.match('^Upgrade to') %] -
  • [% line | $raw %]
    • - [% ELSE %] - [% line | $raw %]
    + [% FOR s IN success %] + [% FOR o IN s.output %] +
  • [% o | html %]
    • + [% IF s.output.size > 1 %] + [% IF loop.first %]
      [% ELSIF loop.last %]
    [% END %] + [% END %] [% END %] [% END %]
[% END %] - [% IF ( has_update_errors ) %] -

Update errors :

+ [% IF error %] +

Update error :

    - [% FOREACH update_error IN update_errors %] -
  • [% update_error.line | html %]
    • + [% FOR e IN error %] + [% FOR o IN e.output %] +
  • + [% o | html %] +
    + ERROR: [% e.error | html %] + + [% IF e.output.size > 1 %] + [% IF loop.first %]
      [% ELSIF loop.last %]
    [% END %] + [% END %] +
    • + [% END %] [% END %]
[% END %] - [% UNLESS ( has_update_errors ) %] + [% UNLESS error %]

Everything went okay. Update done.

+

Continue to log in to Koha

+ [% ELSE %] +

Try again

[% END %] -

Continue to log in to Koha

[% END # / IF updatestructure %] -- 2.39.5