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

    <h1>Software error:</h1>
etc.

Test plan:
- git checkout c4b4db21d2 (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 <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Jonathan Druart 2021-01-29 12:50:27 +01:00
parent 64ccbdb8f2
commit 8c07ba4c80
6 changed files with 305 additions and 204 deletions

View File

@ -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

View File

@ -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 {},
}

View File

@ -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{});
},
}

View File

@ -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;

View File

@ -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";
my $updatedatabase_path = C4::Context->config("intranetdir")
. "/installer/data/$info{dbms}/updatedatabase.pl";
system($cmd );
my $db_entries = get_db_entries();
my $report = update( $db_entries );
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/&emsp;&emsp;/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` };
}
# 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 {

View File

@ -244,31 +244,43 @@
[% IF ( updatestructure ) %]
<h2>Updating database structure</h2>
[% IF ( has_update_succeeds ) %]
<p>Update report :</p>
[% IF success %]
<p>Update report:</p>
<ul>
[% FOREACH l IN update_report %]
[% SET line = l.line %]
[% IF line.match('^Upgrade to') %]
<li>[% line | $raw %]</li>
[% ELSE %]
[% line | $raw %]<br/>
[% FOR s IN success %]
[% FOR o IN s.output %]
<li>[% o | html %]</li>
[% IF s.output.size > 1 %]
[% IF loop.first %]<ul>[% ELSIF loop.last %]</ul>[% END %]
[% END %]
[% END %]
[% END %]
</ul>
[% END %]
[% IF ( has_update_errors ) %]
<p>Update errors :</p>
[% IF error %]
<p>Update error :</p>
<ul>
[% FOREACH update_error IN update_errors %]
<li class="update_error">[% update_error.line | html %]</li>
[% FOR e IN error %]
[% FOR o IN e.output %]
<li class="update_error">
[% o | html %]
<br/>
ERROR: [% e.error | html %]
[% IF e.output.size > 1 %]
[% IF loop.first %]<ul>[% ELSIF loop.last %]</ul>[% END %]
[% END %]
</li>
[% END %]
[% END %]
</ul>
[% END %]
[% UNLESS ( has_update_errors ) %]
[% UNLESS error %]
<p>Everything went okay. Update done.</p>
<p><a href="install.pl?step=3&amp;op=finished" class="btn btn-primary">Continue to log in to Koha</a></p>
[% ELSE %]
<p><a href="install.pl?step=3&amp;op=updatestructure" class="btn btn-primary">Try again</a></p>
[% END %]
<p><a href="install.pl?step=3&amp;op=finished" class="btn btn-primary">Continue to log in to Koha</a></p>
[% END # / IF updatestructure %]
</div> <!-- / #installer-step3 -->
</div> <!-- / .row -->