From 475de2bd37eb9c49a016f8edd3b11e42db326f92 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Sat, 19 Aug 2023 10:20:27 -0500 Subject: [PATCH 1/2] Split the CourseIntegrityCheck.pm module into two parts. The first is CourseDBIntegrityCheck.pm that checks the database integrity and handles a database upgrade. The second is the CourseDirectoryIntegrityCheck that checks the directory structure and handles fixing that if not correct. The database and directory integrity checks do completely separate things, and directory integrity check methods should not be part of the object that is used for the database integrity check. When that object is constructed it obtains a separate database connection that is unused for the directory check and should not be obtained if that is what is being done. So now the directory check and upgrade methods are simply methods exported from the `WeBWorK::Utils::CourseDirectoryIntegrityCheck` module. The unused `lib/WeBWorK/Utils/DBUpgrade.pm` file was deleted. --- bin/update-OPL-statistics.pl | 1 - bin/upgrade_admin_db.pl | 25 +- lib/WeBWorK/ContentGenerator/CourseAdmin.pm | 104 +-- lib/WeBWorK/DB/Record/Depths.pm | 4 - lib/WeBWorK/DB/Record/Setting.pm | 5 +- lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm | 325 ++++++++ .../Utils/CourseDirectoryIntegrityCheck.pm | 169 ++++ lib/WeBWorK/Utils/CourseIntegrityCheck.pm | 526 ------------- lib/WeBWorK/Utils/DBUpgrade.pm | 733 ------------------ .../CourseAdmin/upgrade_course_form.html.ep | 9 +- 10 files changed, 560 insertions(+), 1341 deletions(-) create mode 100644 lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm create mode 100644 lib/WeBWorK/Utils/CourseDirectoryIntegrityCheck.pm delete mode 100644 lib/WeBWorK/Utils/CourseIntegrityCheck.pm delete mode 100644 lib/WeBWorK/Utils/DBUpgrade.pm diff --git a/bin/update-OPL-statistics.pl b/bin/update-OPL-statistics.pl index c906887748..681caf2340 100755 --- a/bin/update-OPL-statistics.pl +++ b/bin/update-OPL-statistics.pl @@ -30,7 +30,6 @@ BEGIN use String::ShellQuote; use DBI; -use WeBWorK::Utils::CourseIntegrityCheck; use WeBWorK::Utils::CourseManagement qw/listCourses/; my $time = time(); diff --git a/bin/upgrade_admin_db.pl b/bin/upgrade_admin_db.pl index 15823bfebf..05805e99d0 100755 --- a/bin/upgrade_admin_db.pl +++ b/bin/upgrade_admin_db.pl @@ -24,40 +24,31 @@ BEGIN use lib "$ENV{WEBWORK_ROOT}/lib"; use WeBWorK::CourseEnvironment; - use WeBWorK::DB; -use WeBWorK::Utils::CourseIntegrityCheck; +use WeBWorK::Utils::CourseDBIntegrityCheck; -########################## -# update admin course -########################## +# Update admin course my $ce = WeBWorK::CourseEnvironment->new({ webwork_dir => $ENV{WEBWORK_ROOT} }); my $upgrade_courseID = $ce->{admin_course_id}; $ce = WeBWorK::CourseEnvironment->new({ webwork_dir => $ENV{WEBWORK_ROOT}, courseName => $upgrade_courseID, }); -#warn "do_upgrade_course: updating |$upgrade_courseID| from" , join("|",@upgrade_courseIDs); -############################################################################# -# Create integrity checker -############################################################################# +# Create integrity checker my @update_report; -my $CIchecker = new WeBWorK::Utils::CourseIntegrityCheck(ce => $ce); +my $CIchecker = new WeBWorK::Utils::CourseDBIntegrityCheck($ce); -############################################################################# # Add missing tables and missing fields to existing tables -############################################################################# - my ($tables_ok, $dbStatus) = $CIchecker->checkCourseTables($upgrade_courseID); my @schema_table_names = keys %$dbStatus; # update tables missing from database; my @tables_to_create = - grep { $dbStatus->{$_}->[0] == WeBWorK::Utils::CourseIntegrityCheck::ONLY_IN_A() } @schema_table_names; + grep { $dbStatus->{$_}->[0] == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_A() } @schema_table_names; my @tables_to_alter = - grep { $dbStatus->{$_}->[0] == WeBWorK::Utils::CourseIntegrityCheck::DIFFER_IN_A_AND_B() } @schema_table_names; + grep { $dbStatus->{$_}->[0] == WeBWorK::Utils::CourseDBIntegrityCheck::DIFFER_IN_A_AND_B() } @schema_table_names; push(@update_report, $CIchecker->updateCourseTables($upgrade_courseID, [@tables_to_create])); -foreach my $table_name (@tables_to_alter) -{ #warn "do_upgrade_course: adding new fields to table $table_name in course $upgrade_courseID"; + +for my $table_name (@tables_to_alter) { push(@update_report, $CIchecker->updateTableFields($upgrade_courseID, $table_name)); } diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index c87566a8f5..fdb4463636 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -30,11 +30,12 @@ use Time::localtime; use WeBWorK::CourseEnvironment; use WeBWorK::Debug; -use WeBWorK::Utils qw(cryptPassword trim_spaces); -use WeBWorK::Utils::CourseIntegrityCheck; +use WeBWorK::Utils qw(cryptPassword trim_spaces); use WeBWorK::Utils::CourseManagement qw(addCourse renameCourse retitleCourse deleteCourse listCourses archiveCourse unarchiveCourse initNonNativeTables); use WeBWorK::Utils::Logs qw(writeLog); +use WeBWorK::Utils::CourseDBIntegrityCheck; +use WeBWorK::Utils::CourseDirectoryIntegrityCheck qw(checkCourseDirectories updateCourseDirectories); use WeBWorK::DB; sub pre_header_initialize ($c) { @@ -513,7 +514,7 @@ sub rename_course_confirm ($c) { ) unless $c->param('rename_newCourseID_checkbox'); if ($ce2->{dbLayoutName}) { - my $CIchecker = WeBWorK::Utils::CourseIntegrityCheck->new(ce => $ce2); + my $CIchecker = WeBWorK::Utils::CourseDBIntegrityCheck->new($ce2); # Check database my ($tables_ok, $dbStatus) = $CIchecker->checkCourseTables($rename_oldCourseID); @@ -523,9 +524,9 @@ sub rename_course_confirm ($c) { if ($c->param('upgrade_course_tables')) { my @schema_table_names = keys %$dbStatus; my @tables_to_create = - grep { $dbStatus->{$_}->[0] == WeBWorK::Utils::CourseIntegrityCheck::ONLY_IN_A } @schema_table_names; + grep { $dbStatus->{$_}->[0] == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_A } @schema_table_names; my @tables_to_alter = - grep { $dbStatus->{$_}->[0] == WeBWorK::Utils::CourseIntegrityCheck::DIFFER_IN_A_AND_B } + grep { $dbStatus->{$_}->[0] == WeBWorK::Utils::CourseDBIntegrityCheck::DIFFER_IN_A_AND_B } @schema_table_names; push(@upgrade_report, $CIchecker->updateCourseTables($rename_oldCourseID, [@tables_to_create])); for my $table_name (@tables_to_alter) { @@ -536,7 +537,7 @@ sub rename_course_confirm ($c) { } # Check directories - my ($directories_ok, $directory_report) = $CIchecker->checkCourseDirectories($ce2); + my ($directories_ok, $directory_report) = checkCourseDirectories($ce2); return $c->include( 'ContentGenerator/CourseAdmin/rename_course_confirm', @@ -980,7 +981,7 @@ sub archive_course_confirm ($c) { my $ce2 = WeBWorK::CourseEnvironment->new({ courseName => $archive_courseID }); if ($ce2->{dbLayoutName}) { - my $CIchecker = WeBWorK::Utils::CourseIntegrityCheck->new(ce => $ce2); + my $CIchecker = WeBWorK::Utils::CourseDBIntegrityCheck->new($ce2); # Check database my ($tables_ok, $dbStatus) = $CIchecker->checkCourseTables($archive_courseID); @@ -990,9 +991,9 @@ sub archive_course_confirm ($c) { if ($c->param('upgrade_course_tables')) { my @schema_table_names = keys %$dbStatus; my @tables_to_create = - grep { $dbStatus->{$_}->[0] == WeBWorK::Utils::CourseIntegrityCheck::ONLY_IN_A } @schema_table_names; + grep { $dbStatus->{$_}->[0] == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_A } @schema_table_names; my @tables_to_alter = - grep { $dbStatus->{$_}->[0] == WeBWorK::Utils::CourseIntegrityCheck::DIFFER_IN_A_AND_B } + grep { $dbStatus->{$_}->[0] == WeBWorK::Utils::CourseDBIntegrityCheck::DIFFER_IN_A_AND_B } @schema_table_names; push(@upgrade_report, $CIchecker->updateCourseTables($archive_courseID, [@tables_to_create])); for my $table_name (@tables_to_alter) { @@ -1003,8 +1004,8 @@ sub archive_course_confirm ($c) { } # Update and check directories. - my $dir_update_messages = $c->param('upgrade_course_tables') ? $CIchecker->updateCourseDirectories : []; - my ($directories_ok, $directory_report) = $CIchecker->checkCourseDirectories($ce2); + my $dir_update_messages = $c->param('upgrade_course_tables') ? updateCourseDirectories($ce2) : []; + my ($directories_ok, $directory_report) = checkCourseDirectories($ce2); return $c->include( 'ContentGenerator/CourseAdmin/archive_course_confirm', @@ -1349,7 +1350,7 @@ sub upgrade_course_confirm ($c) { my $ce2 = WeBWorK::CourseEnvironment->new({ courseName => $upgrade_courseID }); # Create integrity checker - my $CIchecker = WeBWorK::Utils::CourseIntegrityCheck->new(ce => $ce2); + my $CIchecker = WeBWorK::Utils::CourseDBIntegrityCheck->new($ce2); # Report on database status my ($tables_ok, $dbStatus) = $CIchecker->checkCourseTables($upgrade_courseID); @@ -1428,7 +1429,7 @@ sub upgrade_course_confirm ($c) { } # Report on directory status - my ($directories_ok, $directory_report) = $CIchecker->checkCourseDirectories; + my ($directories_ok, $directory_report) = checkCourseDirectories($ce2); push(@$course_output, $c->tag('div', class => 'mb-2', $c->maketext('Directory structure:'))); push( @$course_output, @@ -1480,15 +1481,16 @@ sub do_upgrade_course ($c) { my $ce2 = WeBWorK::CourseEnvironment->new({ courseName => $upgrade_courseID }); # Create integrity checker - my $CIchecker = WeBWorK::Utils::CourseIntegrityCheck->new(ce => $ce2); + my $CIchecker = WeBWorK::Utils::CourseDBIntegrityCheck->new($ce2); # Add missing tables and missing fields to existing tables my ($tables_ok, $dbStatus) = $CIchecker->checkCourseTables($upgrade_courseID); my @schema_table_names = keys %$dbStatus; my @tables_to_create = - grep { $dbStatus->{$_}[0] == WeBWorK::Utils::CourseIntegrityCheck::ONLY_IN_A } @schema_table_names; + grep { $dbStatus->{$_}[0] == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_A } @schema_table_names; my @tables_to_alter = - grep { $dbStatus->{$_}[0] == WeBWorK::Utils::CourseIntegrityCheck::DIFFER_IN_A_AND_B } @schema_table_names; + grep { $dbStatus->{$_}[0] == WeBWorK::Utils::CourseDBIntegrityCheck::DIFFER_IN_A_AND_B } + @schema_table_names; my @upgrade_report; push( @@ -1540,8 +1542,8 @@ sub do_upgrade_course ($c) { } # Add missing directories and prepare report on directory status - my $dir_update_messages = $CIchecker->updateCourseDirectories; # Needs more error messages - my ($directories_ok, $directory_report) = $CIchecker->checkCourseDirectories; + my $dir_update_messages = updateCourseDirectories($ce2); # Needs more error messages + my ($directories_ok, $directory_report) = checkCourseDirectories($ce2); # Show status my $course_report = $c->c; @@ -2650,32 +2652,32 @@ sub do_registration ($c) { # Format a list of tables and fields in the database, and the status of each. sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) { my %table_status_message = ( - WeBWorK::Utils::CourseIntegrityCheck::SAME_IN_A_AND_B => + WeBWorK::Utils::CourseDBIntegrityCheck::SAME_IN_A_AND_B => $c->tag('span', class => 'text-success me-2', $c->maketext('Table is ok')), - WeBWorK::Utils::CourseIntegrityCheck::ONLY_IN_A => $c->tag( + WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_A => $c->tag( 'span', class => 'text-danger me-2', $c->maketext('Table defined in schema but missing in database') ), - WeBWorK::Utils::CourseIntegrityCheck::ONLY_IN_B => $c->tag( + WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_B => $c->tag( 'span', class => 'text-danger me-2', $c->maketext('Table defined in database but missing in schema') ), - WeBWorK::Utils::CourseIntegrityCheck::DIFFER_IN_A_AND_B => $c->tag( + WeBWorK::Utils::CourseDBIntegrityCheck::DIFFER_IN_A_AND_B => $c->tag( 'span', class => 'text-danger me-2', $c->maketext('Schema and database table definitions do not agree') ) ); my %field_status_message = ( - WeBWorK::Utils::CourseIntegrityCheck::SAME_IN_A_AND_B => + WeBWorK::Utils::CourseDBIntegrityCheck::SAME_IN_A_AND_B => $c->tag('span', class => 'text-success me-2', $c->maketext('Field is ok')), - WeBWorK::Utils::CourseIntegrityCheck::ONLY_IN_A => + WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_A => $c->tag('span', class => 'text-danger me-2', $c->maketext('Field missing in database')), - WeBWorK::Utils::CourseIntegrityCheck::ONLY_IN_B => + WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_B => $c->tag('span', class => 'text-danger me-2', $c->maketext('Field missing in schema')), - WeBWorK::Utils::CourseIntegrityCheck::DIFFER_IN_A_AND_B => $c->tag( + WeBWorK::Utils::CourseDBIntegrityCheck::DIFFER_IN_A_AND_B => $c->tag( 'span', class => 'text-danger me-2', $c->maketext('Schema and database field definitions do not agree') @@ -2695,9 +2697,9 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) { my $table_status = $dbStatus->{$table}[0]; push(@$table_report, $table . ': ', $table_status_message{$table_status}); - if ($table_status == WeBWorK::Utils::CourseIntegrityCheck::ONLY_IN_A) { + if ($table_status == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_A) { $all_tables_ok = 0; - } elsif ($table_status == WeBWorK::Utils::CourseIntegrityCheck::ONLY_IN_B) { + } elsif ($table_status == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_B) { $extra_database_tables = 1; push( @$table_report, @@ -2712,7 +2714,7 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) { ) ) ) if defined $courseID; - } elsif ($table_status == WeBWorK::Utils::CourseIntegrityCheck::DIFFER_IN_A_AND_B) { + } elsif ($table_status == WeBWorK::Utils::CourseDBIntegrityCheck::DIFFER_IN_A_AND_B) { my %fieldInfo = %{ $dbStatus->{$table}[1] }; my $fields_report = $c->c; @@ -2720,37 +2722,35 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) { my $field_status = $fieldInfo{$key}[0]; my $field_report = $c->c("$key: $field_status_message{$field_status}"); - if ($field_status == WeBWorK::Utils::CourseIntegrityCheck::ONLY_IN_B) { + if ($field_status == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_B) { if ($fieldInfo{$key}[1]) { $rebuild_table_indexes = 1; } else { $extra_database_fields = 1; } - if (defined $courseID) { - if ($fieldInfo{$key}[1]) { - push(@$field_report, $c->hidden_field("$courseID.$table.delete_fieldIDs" => $key)); - } else { - push( - @$field_report, + if ($fieldInfo{$key}[1]) { + push(@$field_report, $c->hidden_field("$courseID.$table.delete_fieldIDs" => $key)); + } else { + push( + @$field_report, + $c->tag( + 'span', + class => 'form-check d-inline-block', $c->tag( - 'span', - class => 'form-check d-inline-block', - $c->tag( - 'label', - class => 'form-check-label', - $c->c( - $c->check_box( - "$courseID.$table.delete_fieldIDs" => $key, - class => 'form-check-input' - ), - $c->maketext('Delete field when upgrading') - )->join('') - ) + 'label', + class => 'form-check-label', + $c->c( + $c->check_box( + "$courseID.$table.delete_fieldIDs" => $key, + class => 'form-check-input' + ), + $c->maketext('Delete field when upgrading') + )->join('') ) - ); - } + ) + ); } - } elsif ($field_status == WeBWorK::Utils::CourseIntegrityCheck::ONLY_IN_A) { + } elsif ($field_status == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_A) { $all_tables_ok = 0; } push(@$fields_report, $c->tag('li', $field_report->join(''))); diff --git a/lib/WeBWorK/DB/Record/Depths.pm b/lib/WeBWorK/DB/Record/Depths.pm index 91636a7e05..67e6b94ea9 100644 --- a/lib/WeBWorK/DB/Record/Depths.pm +++ b/lib/WeBWorK/DB/Record/Depths.pm @@ -25,15 +25,11 @@ WeBWorK::DB::Record::Depths - represent a record from the depths table. use strict; use warnings; -#use WeBWorK::Utils::DBUpgrade; - BEGIN { __PACKAGE__->_fields( md5 => { type => "CHAR(33) NOT NULL", key => 1 }, depth => { type => "SMALLINT" }, ); - } 1; - diff --git a/lib/WeBWorK/DB/Record/Setting.pm b/lib/WeBWorK/DB/Record/Setting.pm index c6098284cc..397fbff74b 100644 --- a/lib/WeBWorK/DB/Record/Setting.pm +++ b/lib/WeBWorK/DB/Record/Setting.pm @@ -25,8 +25,6 @@ WeBWorK::DB::Record::Setting - represent a record from the setting table. use strict; use warnings; -use WeBWorK::Utils::DBUpgrade; - BEGIN { __PACKAGE__->_fields( name => { type => "VARCHAR(240) NOT NULL", key => 1 }, @@ -35,10 +33,9 @@ BEGIN { __PACKAGE__->_initial_records( { name => "db_version", - value => 3.1415926 # $WeBWorK::Utils::DBUpgrade::THIS_DB_VERSION + value => 3.1415926 }, ); } 1; - diff --git a/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm b/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm new file mode 100644 index 0000000000..20f7760941 --- /dev/null +++ b/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm @@ -0,0 +1,325 @@ +################################################################################ +# WeBWorK Online Homework Delivery System +# Copyright © 2000-2024 The WeBWorK Project, https://github.com/openwebwork +# +# This program is free software; you can redistribute it and/or modify it under +# the terms of either: (a) the GNU General Public License as published by the +# Free Software Foundation; either version 2, or (at your option) any later +# version, or (b) the "Artistic License" which comes with this package. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See either the GNU General Public License or the +# Artistic License for more details. +################################################################################ + +package WeBWorK::Utils::CourseDBIntegrityCheck; + +=head1 NAME + +WeBWorK::Utils::CourseDBIntegrityCheck - Check that course database tables agree +with database schema. + +=cut + +use strict; +use warnings; + +use WeBWorK::Utils::CourseManagement qw(listCourses); + +# Developer note: This file should not format messages in html. Instead return an array of tuples. Each tuple should +# contain the message components, and the last element of the tuple should be 0 or 1 to indicate failure or success +# respectively. See the updateCourseTables and updateTableFields. + +# Constants describing the comparison of two hashes. +use constant { + ONLY_IN_A => 0, + ONLY_IN_B => 1, + DIFFER_IN_A_AND_B => 2, + SAME_IN_A_AND_B => 3 +}; + +sub new { + my ($invocant, $ce) = @_; + return bless { + dbh => DBI->connect( + $ce->{database_dsn}, + $ce->{database_username}, + $ce->{database_password}, + { + PrintError => 0, + RaiseError => 1 + } + ), + ce => $ce, + db => WeBWorK::DB->new($ce->{dbLayouts}{ $ce->{dbLayoutName} }) + }, + ref $invocant || $invocant; +} + +sub ce { return shift->{ce} } +sub db { return shift->{db} } +sub dbh { return shift->{dbh} } + +sub DESTROY { + my ($self) = @_; + $self->unlock_database if $self->{db_locked}; + return; +} + +=head2 checkCourseTables + +Usage: C<< $CIchecker->checkCourseTables($courseName); >> + +Checks the course tables in the mysql database and ensures that they are the +same as the ones specified by the databaseLayout + +=cut + +sub checkCourseTables { + my ($self, $courseName) = @_; + my $tables_ok = 1; + my %dbStatus; + + # Fetch schema from course environment and search database for corresponding tables. + my $db = $self->db; + my $ce = $self->{ce}; + + $self->lock_database; + + for my $table (sort keys %$db) { + next if $db->{$table}{params}{non_native}; + + # Exists means the table can be described + if ($db->{$table}->tableExists) { + my ($fields_ok, $fieldStatus) = $self->checkTableFields($courseName, $table); + if ($fields_ok) { + $dbStatus{$table} = [SAME_IN_A_AND_B]; + } else { + $dbStatus{$table} = [ DIFFER_IN_A_AND_B, $fieldStatus ]; + $tables_ok = 0; + } + } else { + $dbStatus{$table} = [ONLY_IN_A]; + $tables_ok = 0; + } + } + + # Fetch fetch corresponding tables in the database and search for corresponding schema entries. + # _ represents any single character in the MySQL like statement so we escape it + my $result = $self->dbh->selectall_arrayref("show tables like '${courseName}\\_%'"); + my @tableNames = map {@$_} @$result; # Drill down in the result to the table name level + + # Table names are of the form courseID_table (with an underscore). So if we have two courses mth101 and + # mth101_fall09 when we check the tables for mth101 we will inadvertantly pick up the tables for mth101_fall09. + # Thus we find all courseID's and exclude the extraneous tables. + my @courseIDs = listCourses($ce); + my @similarIDs; + for my $courseID (@courseIDs) { + next unless $courseID =~ /^${courseName}\_(.*)/; + push(@similarIDs, $courseID); + } + +OUTER_LOOP: + for my $table (sort @tableNames) { + # Double check that we only have our course tables and similar ones. + next unless $table =~ /^${courseName}\_(.*)/; + + for my $courseID (@similarIDs) { # Exclude tables with similar but wrong names. + next OUTER_LOOP if $table =~ /^${courseID}\_(.*)/; + } + + my $schema_name = $1; + unless (exists($db->{$schema_name})) { + $dbStatus{$schema_name} = [ONLY_IN_B]; + $tables_ok = 0; + } + } + + $self->unlock_database; + + return ($tables_ok, \%dbStatus); +} + +=head2 updateCourseTables + +Usage: C<< $CIchecker-> updateCourseTables($courseName, $table_names); >> + +Adds schema tables to the database that had been missing from the database. + +=cut + +sub updateCourseTables { + my ($self, $courseName, $schema_table_names, $delete_table_names) = @_; + my $db = $self->db; + + $self->lock_database; + + warn 'Pass reference to the array of table names to be updated.' unless ref($schema_table_names) eq 'ARRAY'; + + my @messages; + + # Add tables + for my $schema_table_name (sort @$schema_table_names) { + next if $db->{$schema_table_name}{params}{non_native}; + my $schema_obj = $db->{$schema_table_name}; + my $database_table_name = + exists $schema_obj->{params}{tableOverride} ? $schema_obj->{params}{tableOverride} : $schema_table_name; + + if ($schema_obj->can('create_table')) { + $schema_obj->create_table; + push(@messages, [ "Table $schema_table_name created as $database_table_name in database.", 1 ]); + } else { + push(@messages, [ "Skipping creation of '$schema_table_name' table: no create_table method", 0 ]); + } + } + + # Delete tables + for my $delete_table_name (@$delete_table_names) { + # There is no schema for these tables, so just prepend the course name that was stripped + # from the table when the database was checked in checkCourseTables and try that. + eval { $self->dbh->do("DROP TABLE `${courseName}_$delete_table_name`") }; + if ($@) { + push(@messages, [ "Unable to delete table '$delete_table_name' from database: $@", 0 ]); + } else { + push(@messages, [ "Table '$delete_table_name' deleted from database.", 1 ]); + } + } + + $self->unlock_database; + + return @messages; +} + +=head2 checkTableFields + +Usage: C<< $CIchecker->checkTableFields($courseName, $table); >> + +Checks the course tables in the mysql database and insures that they are the +same as the ones specified by the databaseLayout + +=cut + +sub checkTableFields { + my ($self, $courseName, $table) = @_; + my $fields_ok = 1; + my %fieldStatus; + + # Fetch schema from course environment and search database for corresponding tables. + my $db = $self->db; + my $table_name = + exists $db->{$table}{params}{tableOverride} ? $db->{$table}{params}{tableOverride} : $table; + warn "$table_name is a non native table" if $db->{$table}{params}{non_native}; + my @schema_field_names = $db->{$table}{record}->FIELDS; + my %schema_override_field_names; + for my $field (@schema_field_names) { + my $field_name = $db->{$table}{params}{fieldOverride}{$field} || $field; + $schema_override_field_names{$field_name} = $field; + if ($db->{$table}->tableFieldExists($field_name)) { + $fieldStatus{$field} = [SAME_IN_A_AND_B]; + } else { + $fieldStatus{$field} = [ONLY_IN_A]; + $fields_ok = 0; + } + } + + # Fetch corresponding tables in the database and search for corresponding schema entries. + # result is array: Field | Type | Null | Key | Default | Extra + my $result = $self->dbh->selectall_arrayref("SHOW COLUMNS FROM `$table_name`"); + my %database_fields = map { ${$_}[0] => $_ } @$result; # Construct a hash of field names to field data. + + for my $field_name (keys %database_fields) { + unless (exists($schema_override_field_names{$field_name})) { + $fields_ok = 0; + $fieldStatus{$field_name} = [ONLY_IN_B]; + push(@{ $fieldStatus{$field_name} }, 1) if $database_fields{$field_name}[3]; + } + } + + return ($fields_ok, \%fieldStatus); +} + +=head2 updateTableFields + +Usage: C<< $CIchecker->updateTableFields($courseName, $table); >> + +Checks the fields in the table in the mysql database and insures that they are +the same as the ones specified by the databaseLayout + +=cut + +sub updateTableFields { + my ($self, $courseName, $table, $delete_field_names) = @_; + my @messages; + + # Fetch schema from course environment and search database for corresponding tables. + my $db = $self->db; + my $table_name = exists $db->{$table}{params}{tableOverride} ? $db->{$table}{params}{tableOverride} : $table; + warn "$table_name is a non native table" if $db->{$table}{params}{non_native}; # skip non-native tables + my ($fields_ok, $fieldStatus) = $self->checkTableFields($courseName, $table); + + my $schema_obj = $db->{$table}; + + # Add fields + for my $field_name (keys %$fieldStatus) { + if ($fieldStatus->{$field_name}[0] == ONLY_IN_A) { + if ($schema_obj->can('add_column_field') && $schema_obj->add_column_field($field_name)) { + push(@messages, [ "Added column '$field_name' to table '$table'", 1 ]); + } + } + } + + # Rebuild indexes for the table if a previous key field column is going to be dropped. + if ($schema_obj->can('rebuild_indexes') + && (grep { $fieldStatus->{$_} && $fieldStatus->{$_}[1] } @$delete_field_names)) + { + my $result = eval { $schema_obj->rebuild_indexes }; + if ($@ || !$result) { + push(@messages, [ "There was an error rebuilding indexes for table '$table'", 0 ]); + } else { + push(@messages, [ "Rebuilt indexes for table '$table'", 1 ]); + } + } + + # Drop fields if listed in $delete_field_names. + for my $field_name (@$delete_field_names) { + if ($fieldStatus->{$field_name} && $fieldStatus->{$field_name}[0] == ONLY_IN_B) { + if ($schema_obj->can('drop_column_field') && $schema_obj->drop_column_field($field_name)) { + push(@messages, [ "Dropped column '$field_name' from table '$table'", 1 ]); + } + } + } + + return @messages; +} + +# Database locking utilities + +# Create a lock named 'webwork.dbugrade' that times out after 10 seconds. +sub lock_database { + my $self = shift; + my ($lock_status) = $self->dbh->selectrow_array("SELECT GET_LOCK('webwork.dbupgrade', 10)"); + if (!defined $lock_status) { + die "Couldn't obtain lock because a database error occurred.\n"; + } elsif (!$lock_status) { + die "Timed out while waiting for lock.\n"; + } + $self->{db_locked} = 1; + return; +} + +# Release the lock named 'webwork.dbugrade'. +sub unlock_database { + my $self = shift; + my ($lock_status) = $self->dbh->selectrow_array("SELECT RELEASE_LOCK('webwork.dbupgrade')"); + if ($lock_status) { + delete $self->{db_locked}; + } elsif (defined $lock_status) { + warn "Couldn't release lock because the lock is not held by this thread.\n"; + } else { + warn "Unable to release lock because a database error occurred.\n"; + } + return; +} + +1; diff --git a/lib/WeBWorK/Utils/CourseDirectoryIntegrityCheck.pm b/lib/WeBWorK/Utils/CourseDirectoryIntegrityCheck.pm new file mode 100644 index 0000000000..da0fad7f3b --- /dev/null +++ b/lib/WeBWorK/Utils/CourseDirectoryIntegrityCheck.pm @@ -0,0 +1,169 @@ +################################################################################ +# WeBWorK Online Homework Delivery System +# Copyright © 2000-2023 The WeBWorK Project, https://github.com/openwebwork +# +# This program is free software; you can redistribute it and/or modify it under +# the terms of either: (a) the GNU General Public License as published by the +# Free Software Foundation; either version 2, or (at your option) any later +# version, or (b) the "Artistic License" which comes with this package. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See either the GNU General Public License or the +# Artistic License for more details. +################################################################################ + +package WeBWorK::Utils::CourseDirectoryIntegrityCheck; +use parent Exporter; + +=head1 NAME + +WeBWorK::Utils::CourseDirectoryIntegrityCheck - Check that course directory +structure is correct. + +=cut + +use strict; +use warnings; + +use Mojo::File qw(path); + +our @EXPORT_OK = qw(checkCourseDirectories updateCourseDirectories); + +# Developer note: This file should not format messages in html. Instead return an array of tuples. Each tuple should +# contain the message components, and the last element of the tuple should be 0 or 1 to indicate failure or success +# respectively. See the the updateCourseDirectories method. + +=head2 checkCourseDirectories + +Usage: C<< checkCourseDirectories($ce) >> + +Checks the course directories to make sure they exist and have the correct +permissions. + +=cut + +sub checkCourseDirectories { + my $ce = shift; + + my @results; + my $directories_ok = 1; + + for my $dir (sort keys %{ $ce->{courseDirs} }) { + my $path = $ce->{courseDirs}{$dir}; + my $status = -e $path ? (-r $path ? 'r' : '-') . (-w _ ? 'w' : '-') . (-x _ ? 'x' : '-') : 'missing'; + + # All directories should be readable, writable and executable. + my $good = $status eq 'rwx'; + $directories_ok = 0 if !$good; + + push @results, [ $dir, $path, $good ]; + } + + return ($directories_ok, \@results); +} + +=head2 updateCourseDirectories + +Usage: C<< updateCourseDirectories($ce) >> + +Check to see if all course directories exist and have the correct permissions. + +If a directory does not exist, then it is copied from the model course if the +corresponding directory exists in the model course, and is created otherwise. + +If the permissions are not correct, then an attempt is made to correct the +permissions. The permissions are expected to match the course root directory. +If the permissions of the course root directory are not correct, then that will +need to be manually fixed. This method does not check that. + +=cut + +sub updateCourseDirectories { + my $ce = shift; + + my @messages; + + # Sort courseDirs by path. The important thing for the order is that a directory that is a subdirectory of + # another is listed after the directory containing it. + my @course_dirs = + grep { $_ ne 'root' } sort { $ce->{courseDirs}{$a} =~ /^$ce->{courseDirs}{$b}/ } keys %{ $ce->{courseDirs} }; + + # These are the directories in the model course that can be copied if not found in this course. + my %model_course_dirs = ( + templates => 'templates', + html => 'html', + achievements => 'templates/achievements', + achievement_notifications => 'templates/achievements/notifications', + email => 'templates/email', + achievements_html => 'html/achievements' + ); + + my $permissions = path($ce->{courseDirs}{root})->stat->mode & oct(777); + + for my $dir (@course_dirs) { + my $path = path($ce->{courseDirs}{$dir}); + next if -r $path && -w $path && -x $path; + + my $path_exists_initially = -e $path; + + # Create the directory if it doesn't exist. + if (!$path_exists_initially) { + eval { + $path->make_path({ mode => $permissions }); + push(@messages, [ "Created directory $path.", 1 ]); + }; + if ($@) { + push(@messages, [ "Failed to create directory $path.", 0 ]); + next; + } + } + + # Fix permissions if those are not correct. + if (($path->stat->mode & oct(777)) != $permissions) { + eval { + $path->chmod($permissions); + push(@messages, [ "Changed permissions for directory $path.", 1 ]); + }; + push(@messages, [ "Failed to change permissions for directory $path.", 0 ]) if $@; + } + + # If the path did not exist to begin with and there is a corresponding model course directory, + # then copy the contents of the model course directory. + if (!$path_exists_initially && $model_course_dirs{$dir}) { + my $modelCoursePath = "$ce->{webworkDirs}{courses}/modelCourse/$model_course_dirs{$dir}"; + if (!-r $modelCoursePath) { + push( + @messages, + [ + 'Your modelCourse in the "courses" directory is out of date or missing. Please update it from ' + . "the webwork2/courses.dist directory. Cannot find directory $modelCoursePath. The " + . "directory $path has been created, but may be missing the files it should contain.", + 0 + ] + ); + next; + } + + eval { + for (path($modelCoursePath)->list_tree({ dir => 1 })->each) { + my $destPath = $_ =~ s!$modelCoursePath!$path!r; + if (-l $_) { + symlink(readlink $_, $destPath); + } elsif (-d $_) { + path($destPath)->make_path({ mode => $permissions }); + } else { + $_->copy_to($destPath); + } + } + push(@messages, [ "Copied model course directory $modelCoursePath to $path.", 1 ]); + }; + push(@messages, [ "Failed to copy model course directory $modelCoursePath to $path: $@.", 0 ]) if $@; + } + + } + + return \@messages; +} + +1; diff --git a/lib/WeBWorK/Utils/CourseIntegrityCheck.pm b/lib/WeBWorK/Utils/CourseIntegrityCheck.pm deleted file mode 100644 index dc2ebbe56f..0000000000 --- a/lib/WeBWorK/Utils/CourseIntegrityCheck.pm +++ /dev/null @@ -1,526 +0,0 @@ -################################################################################ -# WeBWorK Online Homework Delivery System -# Copyright © 2000-2024 The WeBWorK Project, https://github.com/openwebwork -# -# This program is free software; you can redistribute it and/or modify it under -# the terms of either: (a) the GNU General Public License as published by the -# Free Software Foundation; either version 2, or (at your option) any later -# version, or (b) the "Artistic License" which comes with this package. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS -# FOR A PARTICULAR PURPOSE. See either the GNU General Public License or the -# Artistic License for more details. -################################################################################ - -package WeBWorK::Utils::CourseIntegrityCheck; - -=head1 NAME - -WeBWorK::Utils::CourseIntegrityCheck - check that course database tables agree -with database schema and that course directory structure is correct. - -=cut - -use strict; -use warnings; - -use Mojo::File qw(path); - -use WeBWorK::Debug; -use WeBWorK::Utils::CourseManagement qw/listCourses/; - -# Developer note: This file should not format messages in html. Instead return an array of tuples. Each tuple should -# contain the message components, and the last element of the tuple should be 0 or 1 to indicate failure or success -# respectively. See the updateCourseTables, updateTableFields, and updateCourseDirectories methods. - -use constant { # constants describing the comparison of two hashes. - ONLY_IN_A => 0, - ONLY_IN_B => 1, - DIFFER_IN_A_AND_B => 2, - SAME_IN_A_AND_B => 3 -}; -################################################################################ - -sub new { - my ($invocant, %options) = @_; - my $class = ref $invocant || $invocant; - my $self = bless {}, $class; - $self->init(%options); - return $self; -} - -sub init { - my ($self, %options) = @_; - - $self->{dbh} = DBI->connect( - $options{ce}{database_dsn}, - $options{ce}{database_username}, - $options{ce}{database_password}, - { - PrintError => 0, - RaiseError => 1, - }, - ); - - $self->{verbose_sub} = $options{verbose_sub} || \&debug; - $self->{confirm_sub} = $options{confirm_sub} || \&ask_permission_stdio; - $self->{ce} = $options{ce}; - my $dbLayoutName = $self->{ce}->{dbLayoutName}; - $self->{db} = WeBWorK::DB->new($self->{ce}{dbLayouts}->{$dbLayoutName}); - - return; -} - -sub ce { return shift->{ce} } -sub db { return shift->{db} } -sub dbh { return shift->{dbh} } -sub verbose { my ($self, @args) = @_; my $sub = $self->{verbose_sub}; return &$sub(@args) } -sub confirm { my ($self, @args) = @_; my $sub = $self->{confirm_sub}; return &$sub(@args) } - -sub DESTROY { - my ($self) = @_; - $self->unlock_database if $self->{db_locked}; - return; -} - -################################################################## - -=over - -=item $CIchecker->checkCourseTables($courseName); - -Checks the course tables in the mysql database and ensures that they are the -same as the ones specified by the databaseLayout - -=cut - -sub checkCourseTables { - my ($self, $courseName) = @_; - my $str = ''; - my $tables_ok = 1; - my %dbStatus = (); - - # Fetch schema from course environment and search database for corresponding tables. - my $db = $self->db; - my $ce = $self->{ce}; - $self->lock_database; - foreach my $table (sort keys %$db) { - next if $db->{$table}{params}{non_native}; # Skip non-native tables - my $table_name = - (exists $db->{$table}->{params}->{tableOverride}) ? $db->{$table}->{params}->{tableOverride} : $table; - my $database_table_exists = ($db->{$table}->tableExists) ? 1 : 0; - if ($database_table_exists) { # Exists means the table can be described - my ($fields_ok, $fieldStatus) = $self->checkTableFields($courseName, $table); - if ($fields_ok) { - $dbStatus{$table} = [ SAME_IN_A_AND_B() ]; - } else { - $dbStatus{$table} = [ DIFFER_IN_A_AND_B(), $fieldStatus ]; - $tables_ok = 0; - } - } else { - $tables_ok = 0; - $dbStatus{$table} = [ ONLY_IN_A(), ]; - } - } - - # Fetch fetch corresponding tables in the database and search for corresponding schema entries. - my $dbh = $self->dbh; - # _ represents any single character in the MySQL like statement so we escape it - my $tablePrefix = "${courseName}\\_"; - my $stmt = "show tables like '${tablePrefix}%'"; # mysql request - my $result = $dbh->selectall_arrayref($stmt); - my @tableNames = map {@$_} @$result; # Drill down in the result to the table name level - - # Table names are of the form courseID_table (with an underscore). So if we have two courses mth101 and - # mth101_fall09 when we check the tables for mth101 we will inadvertantly pick up the tables for mth101_fall09. - # Thus we find all courseID's and exclude the extraneous tables. - my @courseIDs = listCourses($ce); - my @similarIDs = (); - foreach my $courseID (@courseIDs) { - next unless $courseID =~ /^${courseName}\_(.*)/; - push(@similarIDs, $courseID); - } - -OUTER_LOOP: - foreach my $table (sort @tableNames) { - # Double check that we only have our course tables and similar ones. - next unless $table =~ /^${courseName}\_(.*)/; - - foreach my $courseID (@similarIDs) { # Exclude tables with similar but wrong names. - next OUTER_LOOP if $table =~ /^${courseID}\_(.*)/; - } - - my $schema_name = $1; - my $exists = exists($db->{$schema_name}); - $tables_ok = 0 unless exists($db->{$schema_name}); - $dbStatus{$schema_name} = [ONLY_IN_B] unless $exists; - } - $self->unlock_database; - return ($tables_ok, \%dbStatus); -} - -=item $CIchecker-> updateCourseTables($courseName, $table_names); - -Adds schema tables to the database that had been missing from the database. - -=cut - -sub updateCourseTables { - my ($self, $courseName, $schema_table_names, $delete_table_names) = @_; - my $db = $self->db; - $self->lock_database; - warn 'Pass reference to the array of table names to be updated.' unless ref($schema_table_names) eq 'ARRAY'; - - my @messages; - - # Add tables - for my $schema_table_name (sort @$schema_table_names) { - next if $db->{$schema_table_name}{params}{non_native}; # Skip non-native tables - my $schema_obj = $db->{$schema_table_name}; - my $database_table_name = - exists $schema_obj->{params}{tableOverride} ? $schema_obj->{params}{tableOverride} : $schema_table_name; - - if ($schema_obj->can('create_table')) { - $schema_obj->create_table; - push(@messages, [ "Table $schema_table_name created as $database_table_name in database.", 1 ]); - } else { - push(@messages, [ "Skipping creation of '$schema_table_name' table: no create_table method", 0 ]); - } - } - - # Delete tables - for my $delete_table_name (@$delete_table_names) { - # There is no schema for these tables, so just prepend the course name that was stripped - # from the table when the database was checked in checkCourseTables and try that. - eval { $self->dbh->do("DROP TABLE `${courseName}_$delete_table_name`") }; - if ($@) { - push(@messages, [ "Unable to delete table '$delete_table_name' from database: $@", 0 ]); - } else { - push(@messages, [ "Table '$delete_table_name' deleted from database.", 1 ]); - } - } - - $self->unlock_database; - return @messages; -} - -=item $CIchecker->checkTableFields($courseName, $table); - -Checks the course tables in the mysql database and insures that they are the -same as the ones specified by the databaseLayout - -=cut - -sub checkTableFields { - my ($self, $courseName, $table) = @_; - my $fields_ok = 1; - my %fieldStatus = (); - - # Fetch schema from course environment and search database for corresponding tables. - my $db = $self->db; - my $table_name = - (exists $db->{$table}->{params}->{tableOverride}) ? $db->{$table}->{params}->{tableOverride} : $table; - warn "$table_name is a non native table" if $db->{$table}{params}{non_native}; # skip non-native tables - my @schema_field_names = $db->{$table}->{record}->FIELDS; - my %schema_override_field_names = (); - foreach my $field (sort @schema_field_names) { - my $field_name = $db->{$table}->{params}->{fieldOverride}->{$field} || $field; - $schema_override_field_names{$field_name} = $field; - my $database_field_exists = $db->{$table}->tableFieldExists($field_name); - if ($database_field_exists) { - $fieldStatus{$field} = [SAME_IN_A_AND_B]; - } else { - $fields_ok = 0; - $fieldStatus{$field} = [ONLY_IN_A]; - } - - } - - # Fetch corresponding tables in the database and search for corresponding schema entries. - # result is array: Field | Type | Null | Key | Default | Extra - my $result = $self->dbh->selectall_arrayref("SHOW COLUMNS FROM `$table_name`"); - my %database_fields = map { ${$_}[0] => $_ } @$result; # Construct a hash of field names to field data. - - for my $field_name (keys %database_fields) { - unless (exists($schema_override_field_names{$field_name})) { - $fields_ok = 0; - $fieldStatus{$field_name} = [ONLY_IN_B]; - push(@{ $fieldStatus{$field_name} }, 1) if $database_fields{$field_name}[3]; - } - } - - return ($fields_ok, \%fieldStatus); -} - -=item $CIchecker->updateTableFields($courseName, $table); - -Checks the fields in the table in the mysql database and insures that they are -the same as the ones specified by the databaseLayout - -=cut - -sub updateTableFields { - my ($self, $courseName, $table, $delete_field_names) = @_; - my @messages; - - # Fetch schema from course environment and search database for corresponding tables. - my $db = $self->db; - my $table_name = exists $db->{$table}{params}{tableOverride} ? $db->{$table}{params}{tableOverride} : $table; - warn "$table_name is a non native table" if $db->{$table}{params}{non_native}; # skip non-native tables - my ($fields_ok, $fieldStatus) = $self->checkTableFields($courseName, $table); - - my $schema_obj = $db->{$table}; - - # Add fields - for my $field_name (keys %$fieldStatus) { - if ($fieldStatus->{$field_name}[0] == ONLY_IN_A) { - if ($schema_obj->can('add_column_field') && $schema_obj->add_column_field($field_name)) { - push(@messages, [ "Added column '$field_name' to table '$table'", 1 ]); - } - } - } - - # Rebuild indexes for the table if a previous key field column is going to be dropped. - if ($schema_obj->can('rebuild_indexes') - && (grep { $fieldStatus->{$_} && $fieldStatus->{$_}[1] } @$delete_field_names)) - { - my $result = eval { $schema_obj->rebuild_indexes }; - if ($@ || !$result) { - push(@messages, [ "There was an error rebuilding indexes for table '$table'", 0 ]); - } else { - push(@messages, [ "Rebuilt indexes for table '$table'", 1 ]); - } - } - - # Drop fields if listed in $delete_field_names. - for my $field_name (@$delete_field_names) { - if ($fieldStatus->{$field_name} && $fieldStatus->{$field_name}[0] == ONLY_IN_B) { - if ($schema_obj->can('drop_column_field') && $schema_obj->drop_column_field($field_name)) { - push(@messages, [ "Dropped column '$field_name' from table '$table'", 1 ]); - } - } - } - - return @messages; -} - -=item $CIchecker->checkCourseDirectories($courseName); - -Checks the course directories to make sure they exist and have the correct -permissions. - -=cut - -sub checkCourseDirectories { - my ($self) = @_; - my $ce = $self->{ce}; - - my @results; - my $directories_ok = 1; - - for my $dir (sort keys %{ $ce->{courseDirs} }) { - my $path = $ce->{courseDirs}{$dir}; - my $status = -e $path ? (-r $path ? 'r' : '-') . (-w _ ? 'w' : '-') . (-x _ ? 'x' : '-') : 'missing'; - - # All directories should be readable, writable and executable. - my $good = $status eq 'rwx'; - $directories_ok = 0 if !$good; - - push @results, [ $dir, $path, $good ]; - } - - return ($directories_ok, \@results); -} - -=item $CIchecker->updateCourseDirectories($courseName); - -Check to see if all course directories exist and have the correct permissions. - -If a directory does not exist, then it is copied from the model course if the -corresponding directory exists in the model course, and is created otherwise. - -If the permissions are not correct, then an attempt is made to correct the -permissions. The permissions are expected to match the course root directory. -If the permissions of the course root directory are not correct, then that will -need to be manually fixed. This method does not check that. - -=cut - -sub updateCourseDirectories { - my $self = shift; - my $ce = $self->{ce}; - - my @messages; - - # Sort courseDirs by path. The important thing for the order is that a directory that is a subdirectory of - # another is listed after the directory containing it. - my @course_dirs = - grep { $_ ne 'root' } sort { $ce->{courseDirs}{$a} =~ /^$ce->{courseDirs}{$b}/ } keys %{ $ce->{courseDirs} }; - - # These are the directories in the model course that can be copied if not found in this course. - my %model_course_dirs = ( - templates => 'templates', - html => 'html', - achievements => 'templates/achievements', - achievement_notifications => 'templates/achievements/notifications', - email => 'templates/email', - achievements_html => 'html/achievements' - ); - - my $permissions = path($ce->{courseDirs}{root})->stat->mode & 0777; - - for my $dir (@course_dirs) { - my $path = path($ce->{courseDirs}{$dir}); - next if -r $path && -w $path && -x $path; - - my $path_exists_initially = -e $path; - - # Create the directory if it doesn't exist. - if (!$path_exists_initially) { - eval { - $path->make_path({ mode => $permissions }); - push(@messages, [ "Created directory $path.", 1 ]); - }; - if ($@) { - push(@messages, [ "Failed to create directory $path.", 0 ]); - next; - } - } - - # Fix permissions if those are not correct. - if (($path->stat->mode & 0777) != $permissions) { - eval { - $path->chmod($permissions); - push(@messages, [ "Changed permissions for directory $path.", 1 ]); - }; - push(@messages, [ "Failed to change permissions for directory $path.", 0 ]) if $@; - } - - # If the path did not exist to begin with and there is a corresponding model course directory, - # then copy the contents of the model course directory. - if (!$path_exists_initially && $model_course_dirs{$dir}) { - my $modelCoursePath = "$ce->{webworkDirs}{courses}/modelCourse/$model_course_dirs{$dir}"; - if (!-r $modelCoursePath) { - push( - @messages, - [ - 'Your modelCourse in the "courses" directory is out of date or missing. Please update it from ' - . "the webwork2/courses.dist directory. Cannot find directory $modelCoursePath. The " - . "directory $path has been created, but may be missing the files it should contain.", - 0 - ] - ); - next; - } - - eval { - for (path($modelCoursePath)->list_tree({ dir => 1 })->each) { - my $destPath = $_ =~ s!$modelCoursePath!$path!r; - if (-l $_) { - symlink(readlink $_, $destPath); - } elsif (-d $_) { - path($destPath)->make_path({ mode => $permissions }); - } else { - $_->copy_to($destPath); - } - } - push(@messages, [ "Copied model course directory $modelCoursePath to $path.", 1 ]); - }; - push(@messages, [ "Failed to copy model course directory $modelCoursePath to $path: $@.", 0 ]) if $@; - } - - } - - return \@messages; -} - -############################################################################## -# Database utilities -- borrowed from DBUpgrade.pm ??use or modify??? --MEG -############################################################################## - -# Create a lock named 'webwork.dbugrade' that times out after 10 seconds. -sub lock_database { - my $self = shift; - my ($lock_status) = $self->dbh->selectrow_array("SELECT GET_LOCK('webwork.dbupgrade', 10)"); - if (!defined $lock_status) { - die "Couldn't obtain lock because a database error occurred.\n"; - } elsif (!$lock_status) { - die "Timed out while waiting for lock.\n"; - } - $self->{db_locked} = 1; - return; -} - -sub unlock_database { - my $self = shift; - my ($lock_status) = $self->dbh->selectrow_array("SELECT RELEASE_LOCK('webwork.dbupgrade')"); - if ($lock_status) { - delete $self->{db_locked}; - } elsif (defined $lock_status) { - warn "Couldn't release lock because the lock is not held by this thread.\n"; - } else { - warn "Unable to release lock because a database error occurred.\n"; - } - return; -} - -############################################################################## - -sub load_sql_table_list { - my $self = shift; - my $dbh = $self->dbh; - my $sql_tables_ref = $dbh->selectcol_arrayref("SHOW TABLES"); - $self->{sql_tables} = {}; - @{ $self->{sql_tables} }{@$sql_tables_ref} = (); - return; -} - -sub register_sql_table { - my $self = shift; - my $table = shift; - my $dbh = $self->dbh; - $self->{sql_tables}{$table} = (); - return; -} - -sub unregister_sql_table { - my $self = shift; - my $table = shift; - my $dbh = $self->dbh; - delete $self->{sql_tables}{$table}; - return; -} - -sub sql_table_exists { - my $self = shift; - my $table = shift; - my $dbh = $self->dbh; - return exists $self->{sql_tables}{$table}; -} - -################################################################################ - -sub ask_permission_stdio { - my ($prompt, $default) = @_; - - $default = 1 if not defined $default; - my $options = $default ? "[Y/n]" : "[y/N]"; - - while (1) { - print "$prompt $options "; - my $resp = ; - chomp $resp; - return $default if $resp eq ""; - return 1 if lc $resp eq "y"; - return 0 if lc $resp eq "n"; - $prompt = 'Please enter "y" or "n".'; - } - return 0; -} - -=back - -=cut - -1; diff --git a/lib/WeBWorK/Utils/DBUpgrade.pm b/lib/WeBWorK/Utils/DBUpgrade.pm deleted file mode 100644 index 49c36dd322..0000000000 --- a/lib/WeBWorK/Utils/DBUpgrade.pm +++ /dev/null @@ -1,733 +0,0 @@ -################################################################################ -# WeBWorK Online Homework Delivery System -# Copyright © 2000-2024 The WeBWorK Project, https://github.com/openwebwork -# -# This program is free software; you can redistribute it and/or modify it under -# the terms of either: (a) the GNU General Public License as published by the -# Free Software Foundation; either version 2, or (at your option) any later -# version, or (b) the "Artistic License" which comes with this package. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS -# FOR A PARTICULAR PURPOSE. See either the GNU General Public License or the -# Artistic License for more details. -################################################################################ - -package WeBWorK::Utils::DBUpgrade; - -=head1 NAME - -WeBWorK::Utils::DBUpgrade - upgrade WeBWorK SQL databases. - -=cut - -use strict; -use warnings; -use WeBWorK::Debug; -#use WeBWorK::Utils::CourseManagement qw/listCourses/; - -################################################################################ - -# dummy package variable to localize later -our $self; - -my $i = -1; -our @DB_VERSIONS; - -$DB_VERSIONS[ ++$i ]{desc} = "is the initial version of database, identical to database structure in WeBWorK 2.2.x."; - -$DB_VERSIONS[ ++$i ]{desc} = "adds dbupgrade table to facilitate automatic database upgrades."; -$DB_VERSIONS[$i]{global_code} = sub { - $self->dbh->do("CREATE TABLE `dbupgrade` (`name` VARCHAR(255) NOT NULL PRIMARY KEY, `value` TEXT)"); - $self->dbh->do("INSERT INTO `dbupgrade` (`name`, `value`) VALUES (?, ?)", {}, "db_version", 1); - $self->register_sql_table("dbupgrade"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "adds problems_per_page field to set and set_user tables of each course."; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - $self->dbh->do("ALTER TABLE `${course}_set` ADD COLUMN `problems_per_page` INT") - if $self->sql_table_exists("${course}_set"); - $self->dbh->do("ALTER TABLE `${course}_set_user` ADD COLUMN `problems_per_page` INT") - if $self->sql_table_exists("${course}_set_user"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "adds depths table to keep track of dvipng depth information."; -$DB_VERSIONS[$i]{global_code} = sub { - $self->dbh->do("CREATE TABLE depths (md5 CHAR(33) NOT NULL, depth SMALLINT, PRIMARY KEY (md5))"); - $self->register_sql_table("depths"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "changes type of key timestamp field to BIGINT"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - return unless $self->sql_table_exists("${course}_key"); - $self->dbh->do("ALTER TABLE `${course}_key` CHANGE COLUMN `timestamp` `timestamp` BIGINT"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "changes type of problem_user status field to FLOAT"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - return unless $self->sql_table_exists("${course}_problem_user"); - $self->dbh->do("UPDATE `${course}_problem_user` SET `status`=NULL WHERE `status`=''"); - $self->dbh->do("ALTER TABLE `${course}_problem_user` CHANGE COLUMN `status` `status` FLOAT"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "changes types of alphanumeric keyfields to TINYBLOB NOT NULL"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - $self->dbh->do("ALTER TABLE `${course}_user` CHANGE COLUMN `user_id` `user_id` TINYBLOB NOT NULL") - if $self->sql_table_exists("${course}_user"); - $self->dbh->do("ALTER TABLE `${course}_password` CHANGE COLUMN `user_id` `user_id` TINYBLOB NOT NULL") - if $self->sql_table_exists("${course}_password"); - $self->dbh->do("ALTER TABLE `${course}_permission` CHANGE COLUMN `user_id` `user_id` TINYBLOB NOT NULL") - if $self->sql_table_exists("${course}_permission"); - $self->dbh->do("ALTER TABLE `${course}_key` CHANGE COLUMN `user_id` `user_id` TINYBLOB NOT NULL") - if $self->sql_table_exists("${course}_key"); - $self->dbh->do("ALTER TABLE `${course}_set` CHANGE COLUMN `set_id` `set_id` TINYBLOB NOT NULL") - if $self->sql_table_exists("${course}_set"); - $self->dbh->do("ALTER TABLE `${course}_problem` CHANGE COLUMN `set_id` `set_id` TINYBLOB NOT NULL") - if $self->sql_table_exists("${course}_problem"); - $self->dbh->do("ALTER TABLE `${course}_set_user` CHANGE COLUMN `user_id` `user_id` TINYBLOB NOT NULL") - if $self->sql_table_exists("${course}_set_user"); - $self->dbh->do("ALTER TABLE `${course}_set_user` CHANGE COLUMN `set_id` `set_id` TINYBLOB NOT NULL") - if $self->sql_table_exists("${course}_set_user"); - $self->dbh->do("ALTER TABLE `${course}_problem_user` CHANGE COLUMN `user_id` `user_id` TINYBLOB NOT NULL") - if $self->sql_table_exists("${course}_problem_user"); - $self->dbh->do("ALTER TABLE `${course}_problem_user` CHANGE COLUMN `set_id` `set_id` TINYBLOB NOT NULL") - if $self->sql_table_exists("${course}_problem_user"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "fixes KEY length, adds UNIQUE KEY for user table"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - return unless $self->sql_table_eixsts("${course}_user"); - $self->dbh->do("ALTER TABLE `${course}_user` DROP KEY `user_id`"); - $self->dbh->do("ALTER TABLE `${course}_user` ADD UNIQUE KEY (`user_id`(255))"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "fixes KEY length, adds UNIQUE KEY for password table"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - return unless $self->sql_table_exists("${course}_password"); - $self->dbh->do("ALTER TABLE `${course}_password` DROP KEY `user_id`"); - $self->dbh->do("ALTER TABLE `${course}_password` ADD UNIQUE KEY (`user_id`(255))"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "fixes KEY length, adds UNIQUE KEY for permission table"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - return unless $self->sql_table_exists("${course}_permission"); - $self->dbh->do("ALTER TABLE `${course}_permission` DROP KEY `user_id`"); - $self->dbh->do("ALTER TABLE `${course}_permission` ADD UNIQUE KEY (`user_id`(255))"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "fixes KEY length, adds UNIQUE KEY for key table"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - return unless $self->sql_table_exists("${course}_key"); - $self->dbh->do("ALTER TABLE `${course}_key` DROP KEY `user_id`"); - $self->dbh->do("ALTER TABLE `${course}_key` ADD UNIQUE KEY (`user_id`(255))"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "fixes KEY length, adds UNIQUE KEY for set table"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - return unless $self->sql_table_exists("${course}_set"); - $self->dbh->do("ALTER TABLE `${course}_set` DROP KEY `set_id`"); - $self->dbh->do("ALTER TABLE `${course}_set` ADD UNIQUE KEY (`set_id`(255))"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "fixes KEY length, adds UNIQUE KEY for problem table"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - return unless $self->sql_table_exists("${course}_problem"); - $self->dbh->do("ALTER TABLE `${course}_problem` DROP KEY `set_id`"); - $self->dbh->do("ALTER TABLE `${course}_problem` ADD UNIQUE KEY (`set_id`(255), `problem_id`)"); - $self->dbh->do("ALTER TABLE `${course}_problem` DROP KEY `problem_id`"); - $self->dbh->do("ALTER TABLE `${course}_problem` ADD KEY (`problem_id`)"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "fixes KEY length, adds UNIQUE KEY for set_user table"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - return unless $self->sql_table_exists("${course}_set_user"); - $self->dbh->do("ALTER TABLE `${course}_set_user` DROP KEY `user_id`"); - $self->dbh->do("ALTER TABLE `${course}_set_user` ADD UNIQUE KEY (`user_id`(255), `set_id`(255))"); - $self->dbh->do("ALTER TABLE `${course}_set_user` DROP KEY `set_id`"); - $self->dbh->do("ALTER TABLE `${course}_set_user` ADD KEY (`set_id`(255))"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "fixes KEY length, adds UNIQUE KEY for problem_user table"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - return unless $self->sql_table_exists("${course}_problem_user"); - $self->dbh->do("ALTER TABLE `${course}_problem_user` DROP KEY `user_id`"); - $self->dbh->do( - "ALTER TABLE `${course}_problem_user` ADD UNIQUE KEY (`user_id`(255), `set_id`(255), `problem_id`)"); - $self->dbh->do("ALTER TABLE `${course}_problem_user` DROP KEY `set_id`"); - $self->dbh->do("ALTER TABLE `${course}_problem_user` ADD KEY (`set_id`(255), `problem_id`)"); - $self->dbh->do("ALTER TABLE `${course}_problem_user` DROP KEY `problem_id`"); - $self->dbh->do("ALTER TABLE `${course}_problem_user` ADD KEY (`problem_id`)"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "changes psvn index from PRIMARY KEY to UNIQUE KEY"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - return unless $self->sql_table_exists("${course}_set_user"); - $self->dbh->do("ALTER TABLE `${course}_set_user` ADD UNIQUE KEY (`psvn`)"); - $self->dbh->do("ALTER TABLE `${course}_set_user` DROP PRIMARY KEY"); -}; - -$DB_VERSIONS[ ++$i ]{desc} = "adds hide_score and hide_work fields to set and set_user"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - if ($self->sql_table_exists("${course}_set")) { - $self->dbh->do("ALTER TABLE `${course}_set` ADD COLUMN `hide_score` ENUM('0','1')"); - $self->dbh->do("ALTER TABLE `${course}_set` ADD COLUMN `hide_work` ENUM('0','1')"); - } - if ($self->sql_table_exists("${course}_set_user")) { - $self->dbh->do("ALTER TABLE `${course}_set_user` ADD COLUMN `hide_score` ENUM('0','1')"); - $self->dbh->do("ALTER TABLE `${course}_set_user` ADD COLUMN `hide_work` ENUM('0','1')"); - } -}; - -$DB_VERSIONS[ ++$i ]{desc} = - "updates hide_score and hide_work in set and set_user tables to allow more (and more descriptive) possible values"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - if ($self->sql_table_exists("${course}_set")) { - $self->dbh->do("ALTER TABLE `${course}_set` MODIFY COLUMN `hide_score` ENUM('0','1','2')"); - $self->dbh->do("ALTER TABLE `${course}_set` MODIFY COLUMN `hide_work` ENUM('0','1','2')"); - } - if ($self->sql_table_exists("${course}_set_user")) { - $self->dbh->do("ALTER TABLE `${course}_set_user` MODIFY COLUMN `hide_score` ENUM('0','1','2')"); - $self->dbh->do("ALTER TABLE `${course}_set_user` MODIFY COLUMN `hide_work` ENUM('0','1','2')"); - } -}; - -$DB_VERSIONS[ ++$i ]{desc} = "adds time_limit_cap field to set and set_user tables"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - if ($self->sql_table_exists("${course}_set")) { - $self->dbh->do("ALTER TABLE `${course}_set` ADD COLUMN `time_limit_cap` ENUM('0','1')"); - } - if ($self->sql_table_exists("${course}_set_user")) { - $self->dbh->do("ALTER TABLE `${course}_set_user` ADD COLUMN `time_limit_cap` ENUM('0','1')"); - } -}; - -$DB_VERSIONS[ ++$i ]{desc} = - "updates hide_score and hide_work in set and set_user tables to have more descriptive values, set default values"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - if ($self->sql_table_exists("${course}_set")) { - $self->dbh->do( - "ALTER TABLE `${course}_set` MODIFY COLUMN `hide_score` ENUM('N','Y','BeforeAnswerDate') DEFAULT 'N'"); - $self->dbh->do( - "ALTER TABLE `${course}_set` MODIFY COLUMN `hide_work` ENUM('N','Y','BeforeAnswerDate') DEFAULT 'N'"); - } - if ($self->sql_table_exists("${course}_set_user")) { - $self->dbh->do( - "ALTER TABLE `${course}_set_user` MODIFY COLUMN `hide_score` ENUM('N','Y','BeforeAnswerDate') DEFAULT 'N'"); - $self->dbh->do( - "ALTER TABLE `${course}_set_user` MODIFY COLUMN `hide_work` ENUM('N','Y','BeforeAnswerDate') DEFAULT 'N'"); - } -}; - -$DB_VERSIONS[ ++$i ]{desc} = - "adds locations, location_addresses, set_locations and set_locations_user tables to database, and add restrict_ip to set and set_user."; -$DB_VERSIONS[$i]{global_code} = sub { - $self->dbh->do( - "CREATE TABLE locations (location_id TINYBLOB NOT NULL, description TEXT, PRIMARY KEY (location_id(1000)))" - ); - $self->dbh->do( - "CREATE TABLE location_addresses (location_id TINYBLOB NOT NULL, ip_mask TINYBLOB NOT NULL, PRIMARY KEY (location_id(500),ip_mask(500)))" - ); -}; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - - $self->dbh->do( - "CREATE TABLE `${course}_set_locations` (set_id TINYBLOB NOT NULL, location_id TINYBLOB NOT NULL, PRIMARY KEY (set_id(500),location_id(500)))" - ); - $self->dbh->do( - "CREATE TABLE `${course}_set_locations_user` (set_id TINYBLOB NOT NULL, user_id TINYBLOB NOT NULL, location_id TINYBLOB NOT NULL, PRIMARY KEY (set_id(300),user_id(300),location_id(300)))" - ); - - if ($self->sql_table_exists("${course}_set")) { - $self->dbh->do( - "ALTER TABLE `${course}_set` ADD COLUMN `restrict_ip` enum('No','RestrictTo','DenyFrom') DEFAULT 'No'"); - } - if ($self->sql_table_exists("${course}_set_user")) { - $self->dbh->do( - "ALTER TABLE `${course}_set_user` ADD COLUMN `restrict_ip` enum('No','RestrictTo','DenyFrom')"); - } -}; - -$DB_VERSIONS[ ++$i ]{desc} = "updates defaults for hide_work and hide_score in set_user tables."; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - - if ($self->sql_table_exists("${course}_set_user")) { - $self->dbh->do( - "ALTER TABLE `${course}_set_user` MODIFY COLUMN `hide_score` ENUM('N','Y','BeforeAnswerDate')"); - $self->dbh->do( - "ALTER TABLE `${course}_set_user` MODIFY COLUMN `hide_work` ENUM('N','Y','BeforeAnswerDate')"); - } -}; - -$DB_VERSIONS[ ++$i ]{desc} = "adds relax_restrict_ip, hide_problem_score columns to set and set_user tables."; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - - if ($self->sql_table_exists("${course}_set")) { - $self->dbh->do( - "ALTER TABLE `${course}_set` ADD COLUMN `relax_restrict_ip` ENUM('No','AfterAnswerDate','AfterVersionAnswerDate') DEFAULT 'No'" - ); - $self->dbh->do("ALTER TABLE `${course}_set` ADD COLUMN `hide_score_by_problem` ENUM('N','Y') DEFAULT 'N'"); - } - if ($self->sql_table_exists("${course}_set_user")) { - $self->dbh->do( - "ALTER TABLE `${course}_set_user` ADD COLUMN `relax_restrict_ip` ENUM('No','AfterAnswerDate','AfterVersionAnswerDate')" - ); - $self->dbh->do("ALTER TABLE `${course}_set_user` ADD COLUMN `hide_score_by_problem` ENUM('N','Y')"); - } -}; - -$DB_VERSIONS[ ++$i ]{desc} = - "adds set and set_user fields to allow set-level proctor, updates permissions to allow finer-grained regulation of proctoring."; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - if ($self->sql_table_exists("${course}_permission")) { - $self->dbh->do("UPDATE `${course}_permission` SET `permission`=3 where `permission`=2"); - } - if ($self->sql_table_exists("${course}_set")) { - $self->dbh->do( - "ALTER TABLE `${course}_set` ADD COLUMN `restricted_login_proctor` ENUM('No','Yes') DEFAULT 'No'"); - } - if ($self->sql_table_exists("${course}_set_user")) { - $self->dbh->do("ALTER TABLE `${course}_set_user` ADD COLUMN `restricted_login_proctor` ENUM('No','Yes')"); - } -}; - -$DB_VERSIONS[ ++$i ]{desc} = "adds per-course setting table"; -$DB_VERSIONS[$i]{course_code} = sub { - my $course = shift; - $self->dbh->do("CREATE TABLE `${course}_setting` (`name` VARCHAR(255) NOT NULL PRIMARY KEY, `value` TEXT)"); - $self->register_sql_table("${course}_setting"); - $self->dbh->do("INSERT INTO `${course}_setting` (`name`, `value`) VALUES (?, ?)", {}, "db_version", $i); -}; -our $FIRST_COURSE_DB_VERSION = $i; - -our $THIS_DB_VERSION = $i; - -################################################################################ - -sub new { - my $invocant = shift; - my $class = ref $invocant || $invocant; - my $self = bless {}, $class; - $self->init(@_); - return $self; -} - -sub init { - my ($self, %options) = @_; - - $self->{dbh} = DBI->connect( - $options{ce}{database_dsn}, - $options{ce}{database_username}, - $options{ce}{database_password}, - { - PrintError => 0, - RaiseError => 1, - }, - ); - $self->{verbose_sub} = $options{verbose_sub} || \&debug; - $self->{confirm_sub} = $options{confirm_sub} || \&ask_permission_stdio; - $self->{ce} = $options{ce}; - $self->{course_db_versions} = {}; -} - -sub ce { return shift->{ce} } -sub dbh { return shift->{dbh} } -sub verbose { my $sub = shift->{verbose_sub}; return &$sub(@_) } -sub confirm { my $sub = shift->{confirm_sub}; return &$sub(@_) } - -sub DESTROY { - my ($self) = @_; - $self->unlock_database; - $self->SUPER::DESTROY if $self->can("SUPER::DESTROY"); -} - -################################################################################ - -sub lock_database { - my ($self) = @_; - - $self->verbose("Obtaining dbupgrade lock...\n"); - my ($lock_status) = $self->dbh->selectrow_array("SELECT GET_LOCK('dbupgrade', 10)"); - if (not defined $lock_status) { - die "Couldn't obtain lock because an error occurred.\n"; - } - if ($lock_status) { - $self->verbose("Got lock.\n"); - } else { - die "Timed out while waiting for lock.\n"; - } -} - -sub unlock_database { - my ($self) = @_; - - $self->verbose("Releasing dbupgrade lock...\n"); - my ($lock_status) = $self->dbh->selectrow_array("SELECT RELEASE_LOCK('dbupgrade')"); - if (not defined $lock_status) { - die "Couldn't release lock because the lock does not exist.\n"; - } - if ($lock_status) { - $self->verbose("Released lock.\n"); - } else { - die "Couldn't release lock because the lock is not held by this thread.\n"; - } -} - -################################################################################ - -sub load_sql_table_list { - my ($self) = @_; - my $sql_tables_ref = $self->dbh->selectcol_arrayref("SHOW TABLES"); - $self->{sql_tables} = {}; - @{ $self->{sql_tables} }{@$sql_tables_ref} = (); -} - -sub register_sql_table { - my ($self, $table) = @_; - $self->{sql_tables}{$table} = (); -} - -sub unregister_sql_table { - my ($self, $table) = @_; - delete $self->{sql_tables}{$table}; -} - -sub sql_table_exists { - my ($self, $table) = @_; - return exists $self->{sql_tables}{$table}; -} - -################################################################################ - -use constant DB_TABLE_MISSING => -1; -use constant DB_RECORD_MISSING => -2; - -sub get_db_version { - my ($self, $course) = @_; - my $table = defined $course ? "${course}_setting" : "dbupgrade"; - if ($self->sql_table_exists($table)) { - my $table_quoted = $self->dbh->quote_identifier($table); - my @record = $self->dbh->selectrow_array("SELECT `value` FROM $table_quoted WHERE `name`='db_version'"); - if (@record) { - return $record[0]; - } else { - return DB_RECORD_MISSING; - } - } else { - return DB_TABLE_MISSING; - } -} - -my $vers_value_should_be = "The value should always be a positive integer."; - -sub check_db_version_format { - my ($self, $db_version) = @_; - if (not defined $db_version) { - return "'db_version' has a NULL value. $vers_value_should_be"; - } elsif ($db_version !~ /^-?\d+$/) { - return "'db_version' is set to the non-numeric value '$db_version'. $vers_value_should_be"; - } elsif ($db_version < 0) { - return "'db_version' is set to the negative value '$db_version'. $vers_value_should_be"; - } elsif ($db_version == 0) { - return - "'db_version' is set to 0, which is reserved to indicate a pre-automatic-upgrade version. $vers_value_should_be"; - } else { - # db_version is a positive integer! yay! - return; - } -} - -sub set_db_version { - my ($self, $vers, $course) = @_; - my $table = defined $course ? "${course}_setting" : "dbupgrade"; - my $table_quoted = $self->dbh->quote_identifier($table); - $self->dbh->do("UPDATE $table_quoted SET `value`=? WHERE `name`='db_version'", {}, $vers); -} - -################################################################################ - -sub do_upgrade { - my ($self) = @_; - - $self->lock_database; - $self->load_sql_table_list; - - #### Get system's database version - - my $system_db_version = $self->get_db_version(); - - if ($system_db_version == DB_TABLE_MISSING) { - warn "No 'upgrade' table exists: assuming system database version is 0.\n"; - $system_db_version = 0; - } elsif ($system_db_version == DB_RECORD_MISSING) { - die "The 'dbupgrade' table exists in the database, but no 'db_version' record exists in it. Can't continue.\n"; - } elsif (my $error = $self->check_db_version_format($system_db_version)) { - die "$error Can't continue.\n"; - } elsif ($system_db_version > $THIS_DB_VERSION) { - die - "This database's system db_version value is $system_db_version, but the current database version is only $THIS_DB_VERSION. This database was probably used with a newer version of WeBWorK. Can't continue.\n"; - } - - $self->verbose("Initial system db_version is $system_db_version\n"); - - #### Get database version for each course - # If $system_db_version < $FIRST_COURSE_DB_VERSION, most courses will not have - # a db_version, but some might. (Say, if they were imported from a newer - # version of WeBWorK.) - - my @ww_courses = listCourses($self->ce); - $self->{ww_courses} = \@ww_courses; - - my $course_db_versions = $self->{course_db_versions}; - - foreach my $course (@ww_courses) { - my $course_db_version = $self->get_db_version($course); - - if ($system_db_version < $FIRST_COURSE_DB_VERSION) { - - if ($course_db_version == DB_TABLE_MISSING) { - # this is to be expected -- we assume the course is at the current system version - $self->verbose( - "Course '$course' has no db_version of it's own, assuming system db_version $system_db_version.\n"); - $course_db_versions->{$course} = $system_db_version; - } else { - # there is a settings table -- the course is probably from a later version of WW - warn "The course '$course' already contains a '${course}_setting' table." - . " Settings tables were introduced at db_version $FIRST_COURSE_DB_VERSION," - . " but the current system db_version is only $system_db_version." - . " We'll assume that this course is from a later version of WeBWorK" - . " and try to determine the course's version...\n"; - if ($course_db_version == DB_RECORD_MISSING) { - warn "There is no 'db_version' record in the course's settings table," - . " so we can't determine the version. This course will be excluded from upgrades." - . " If you know the version of this course," - . " add a 'db_version' record with the appropriate value to the '${course}_setting' table.\n"; - } elsif (my $error = check_db_version_format($course_db_version)) { - warn "$error\n"; - warn "There is a 'db_version' record in the course's settings table," - . " but it has an invalid value , so we can't determine the version." - . " This course will be excluded from upgrades." - . " If you know the version of this course," - . " update 'db_version' record in the '${course}_setting' table with the appropriate value.\n"; - } elsif ($course_db_version < $FIRST_COURSE_DB_VERSION) { - warn - "This course's version is $course_db_version, which is before per-course versioning was introduced." - . " Therefore, a course at version $course_db_version should have neither a '${course}_setting' table" - . " nor a 'db_version' record in that table. Regardless, we will assume the recorded version is correct.\n"; - $course_db_versions->{$course} = $system_db_version; - } else { - warn "This course's version is $course_db_version, which makes sense.\n"; - $course_db_versions->{$course} = $course_db_version; - } - } - - } else { - - if ($course_db_version == DB_TABLE_MISSING) { - warn "The course '$course' is missing a '${course}_setting' table, so we can't determine the version." - . " This course will be ignored." - . " If you know the version of this course, add a '${course}_setting' table" - . " and add a 'db_version' record with the appropriate value to the table.\n"; - } else { - # there is a settings table -- good - if ($course_db_version == DB_RECORD_MISSING) { - warn "The course '$course' is missing a 'db_version' record in its '${course}_setting' table," - . " so we can't determine the version. This course will be ignored." - . " If you know the version of this course," - . " add a 'db_version' record with the appropriate value to the '${course}_setting' table.\n"; - } elsif (my $error = check_db_version_format($course_db_version)) { - warn "$error\n"; - warn - "The course '$course' has an invalid value in the 'db_version' record in its '${course}_setting' table," - . " so we can't determine the version. This course will be ignored." - . " If you know the version of this course," - . " update the 'db_version' record in the '${course}_setting' table with the appropriate value.\n"; - } elsif ($course_db_version < $FIRST_COURSE_DB_VERSION) { - warn - "This course's version is $course_db_version, which is before per-course versioning was introduced." - . " Therefore, a course at version $course_db_version should have neither a '${course}_setting' table" - . " nor a 'db_version' record in that table. Regardless, we will assume the recorded version is correct.\n"; - $course_db_versions->{$course} = $course_db_version; - } else { - $self->verbose("Course '$course' has valid db_version $course_db_version.\n"); - $course_db_versions->{$course} = $system_db_version; - } - } - - } - } - - $self->verbose(map {"$_ => $$course_db_versions{$_}\n"} keys %$course_db_versions); - - #### Determine lowest version - - my $lowest_db_version = $system_db_version; - foreach my $v (values %$course_db_versions) { - $lowest_db_version = $v if $v < $lowest_db_version; - } - - $self->verbose("Lowest db_version is $lowest_db_version\n"); - - #### Do the upgrades - - # upgrade_to_version uses this - $self->{system_db_version} = $system_db_version; - - my $vers = $lowest_db_version; - while ($vers < $THIS_DB_VERSION) { - $vers++; - unless ($self->upgrade_to_version($vers)) { - print "\nUpgrading from version " . ($vers - 1) . " to $vers failed.\n\n"; - unless ($self->ask_permission("Ignore this error and go on to the next version?", 0)) { - exit 3; - } - } - } - - #### All done! - - print "\nDatabase is up-to-date at version $vers.\n"; -} - -################################################################################ - -use constant OK => 0; -use constant SKIPPED => 1; -use constant ERROR => 2; - -sub upgrade_to_version { - my ($self, $vers) = @_; - my %info = %{ $DB_VERSIONS[$vers] }; - - print "\nUpgrading database from version " . ($vers - 1) . " to $vers...\n"; - my $desc = $info{desc} || "has no description."; - print "(Version $vers $desc)\n"; - - if ($self->{system_db_version} < $vers and exists $info{global_code}) { - eval { - local $WeBWorK::Utils::DBUpgrade::self = $self; - $info{global_code}->(); - }; - if ($@) { - print "\nAn error occurred while running the system upgrade code for version $vers:\n"; - print "$@"; - return 0 unless $self->ask_permission("Ignore this error and keep going?", 0); - } - } - $self->set_db_version($vers); - - my $do_upgrade = 1; - foreach my $course (@{ $self->{ww_courses} }) { - if ($do_upgrade) { - my $result = $self->upgrade_course_to_version($course, $vers); - if ($result == ERROR) { - if ($self->ask_permission("Update course's stored db_version to $vers anyway?", 0)) { - set_db_version($vers, $course); - print "OK, updated course's stored db_version.\n"; - } else { - print "OK, not updating course's stored db_version.\n"; - } - if ($self->ask_permission("Upgrade the remaining courses to version $vers?", 0)) { - print "OK, going on to the next course...\n"; - } else { - print "OK, we'll skip upgrading the rest of the courses to version $vers.\n"; - if ($self->ask_permission( - "Update the stored db_version for the courses we're skipping, as if we had upgraded them?", - 1 - )) - { - $do_upgrade = 0; - } else { - return 0; - } - } - } elsif ($result == OK) { - $self->set_db_version($vers, $course); - } elsif ($result == SKIPPED) { - # do nothing - } - } else { - $self->set_db_version($vers, $course); - } - } - - return 1; -} - -sub upgrade_course_to_version { - my ($self, $course, $vers) = @_; - my $course_db_versions = $self->{course_db_versions}; - my %info = %{ $DB_VERSIONS[$vers] }; - - my $course_vers = $course_db_versions->{$course}; - #$self->verbose("course=$course course_vers=$course_vers vers=$vers\n"); - if (not defined $course_vers) { - $self->verbose("Course '$course' has a missing or invalid version -- skipping.\n"); - return SKIPPED; - } elsif ($course_vers == $vers) { - $self->verbose("Course '$course' is already at version $vers -- skipping.\n"); - return SKIPPED; - } elsif ($course_vers > $vers) { - $self->verbose("Course '$course' version $course_vers > target version $vers -- skipping.\n"); - return SKIPPED; - } elsif ($course_vers < $vers - 1) { - warn - "Course '$course' at version $course_vers, which is too old to upgrade to $vers. This should never happen. Not upgrading.\n"; - return SKIPPED; - } - - $self->verbose("Upgrading course '$course' to version $vers...\n"); - eval { - local $WeBWorK::Utils::DBUpgrade::self = $self; - $info{course_code}->($course); - }; - if ($@) { - print "\nAn error occurred while running the course upgrade code for version $vers on course $course:\n"; - print "$@"; - return ERROR; - } else { - return OK; - } -} - -################################################################################ - -sub ask_permission_stdio { - my ($prompt, $default) = @_; - - $default = 1 if not defined $default; - my $options = $default ? "[Y/n]" : "[y/N]"; - - while (1) { - print "$prompt $options "; - my $resp = ; - chomp $resp; - return $default if $resp eq ""; - return 1 if lc $resp eq "y"; - return 0 if lc $resp eq "n"; - $prompt = 'Please enter "y" or "n".'; - } -} - -1; - diff --git a/templates/ContentGenerator/CourseAdmin/upgrade_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/upgrade_course_form.html.ep index 2219923131..af93ccd66e 100644 --- a/templates/ContentGenerator/CourseAdmin/upgrade_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/upgrade_course_form.html.ep @@ -1,4 +1,5 @@ -% use WeBWorK::Utils::CourseIntegrityCheck; +% use WeBWorK::Utils::CourseDBIntegrityCheck; +% use WeBWorK::Utils::CourseDirectoryIntegrityCheck qw(checkCourseDirectories); % use WeBWorK::Utils::CourseManagement qw(listCourses); % use WeBWorK::CourseEnvironment; % @@ -27,9 +28,9 @@ % if ($@) { <%= maketext(q{Can't create course environment for [_1] because [_2]}, $courseID, $@) =%> % } - % my $CIchecker = WeBWorK::Utils::CourseIntegrityCheck->new(ce => $tempCE); - % my ($tables_ok) = $CIchecker->checkCourseTables($courseID); - % my ($directories_ok) = $CIchecker->checkCourseDirectories(); + % my $CDBIchecker = WeBWorK::Utils::CourseDBIntegrityCheck->new($tempCE); + % my ($tables_ok) = $CDBIchecker->checkCourseTables($courseID); + % my ($directories_ok) = checkCourseDirectories($tempCE); %
  • % if (!$tables_ok || !$directories_ok) { From 43af3479fa7e74ecdc13733824e56d9d9994cdd6 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Thu, 19 Dec 2024 10:04:22 -0600 Subject: [PATCH 2/2] Add the capability of changing database field types when upgrading the database. This is not done by default. There are check boxes that can be selected to do this when upgrading a course. Since this process can be risky there are ample warnings recommending that an archive be made before upgrading and changing field types. Also remove the `fieldOverride` usage. There are no field overrides anymore, and the current SQL::Abstract code no longer even supports it. --- bin/upgrade-database-to-utf8mb4.pl | 29 +++-- lib/WeBWorK/ContentGenerator/CourseAdmin.pm | 113 ++++++++++++++---- lib/WeBWorK/DB/Schema/NewSQL/Std.pm | 25 +++- lib/WeBWorK/DB/Schema/NewSQL/Versioned.pm | 2 - lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm | 37 +++++- lib/WeBWorK/Utils/CourseManagement.pm | 13 +- .../archive_course_confirm.html.ep | 20 +++- .../CourseAdmin/rename_course_confirm.html.ep | 20 +++- .../upgrade_course_confirm.html.ep | 9 ++ 9 files changed, 206 insertions(+), 62 deletions(-) diff --git a/bin/upgrade-database-to-utf8mb4.pl b/bin/upgrade-database-to-utf8mb4.pl index 4765ce28db..3751a2fd30 100755 --- a/bin/upgrade-database-to-utf8mb4.pl +++ b/bin/upgrade-database-to-utf8mb4.pl @@ -165,7 +165,7 @@ BEGIN my $dbuser = shell_quote($ce->{database_username}); my $dbpass = $ce->{database_password}; -$ENV{'MYSQL_PWD'} = $dbpass; +local $ENV{MYSQL_PWD} = $dbpass; if (!$no_backup) { # Backup the database @@ -212,7 +212,7 @@ BEGIN }, ); -my $db = new WeBWorK::DB($ce->{dbLayouts}{ $ce->{dbLayoutName} }); +my $db = WeBWorK::DB->new($ce->{dbLayouts}{ $ce->{dbLayoutName} }); my @table_types = sort(grep { !$db->{$_}{params}{non_native} } keys %$db); sub checkAndUpdateTableColumnTypes { @@ -222,29 +222,30 @@ sub checkAndUpdateTableColumnTypes { print "\tChecking '$table' (pass $pass)\n" if $verbose; my $schema_field_data = $db->{$table_type}{record}->FIELD_DATA; - for my $field (keys %$schema_field_data) { - my $field_name = $db->{$table_type}{params}{fieldOverride}{$field} || $field; - my @name_type = @{ + for my $field_name (keys %$schema_field_data) { + my @name_type = @{ $dbh->selectall_arrayref( "SELECT COLUMN_TYPE FROM INFORMATION_SCHEMA.COLUMNS " . "WHERE TABLE_SCHEMA='$dbname' AND TABLE_NAME='$table' AND COLUMN_NAME='$field_name';" ) }; - print("\t\tThe '$field_name' column is missing from '$table'.\n" - . "\t\tYou should upgrade the course via course administration to fix this.\n" - . "\t\tYou may need to run this script again after doing that.\n"), next - if !exists($name_type[0][0]); + if (!exists($name_type[0][0])) { + print("\t\tThe '$field_name' column is missing from '$table'.\n" + . "\t\tYou should upgrade the course via course administration to fix this.\n" + . "\t\tYou may need to run this script again after doing that.\n"); + next; + } my $data_type = $name_type[0][0]; next if !$data_type; $data_type =~ s/\(\d*\)$// if $data_type =~ /^(big|small)?int\(\d*\)$/; $data_type = lc($data_type); - my $schema_data_type = lc($schema_field_data->{$field}{type} =~ s/ .*$//r); + my $schema_data_type = lc($schema_field_data->{$field_name}{type} =~ s/ .*$//r); if ($data_type ne $schema_data_type) { print "\t\tUpdating data type for column '$field_name' in table '$table'\n" if $verbose; print "\t\t\t$data_type -> $schema_data_type\n" if $verbose; - eval { $dbh->do("ALTER TABLE `$table` MODIFY $field_name $schema_field_data->{$field}{type};"); }; + eval { $dbh->do("ALTER TABLE `$table` MODIFY $field_name $schema_field_data->{$field_name}{type};"); }; my $indent = $verbose ? "\t\t" : ""; die("${indent}Failed to modify '$field_name' in '$table' from '$data_type' to '$schema_data_type.\n" . "${indent}It is recommended that you restore a database backup. Make note of the\n" @@ -286,8 +287,10 @@ sub checkAndChangeTableCharacterSet { my $error = 0; for my $course (@courses) { - print("The course '$course' does not exist on the server\n"), next - if !grep($course eq $_, @server_courses); + if (!grep { $course eq $_ } @server_courses) { + print("The course '$course' does not exist on the server\n"); + next; + } print "Checking tables for '$course'\n" if $verbose; for my $table_type (@table_types) { diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index fdb4463636..63e14daf58 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -1339,7 +1339,7 @@ sub upgrade_course_confirm ($c) { my @upgrade_courseIDs = $c->param('upgrade_courseIDs'); - my ($extra_database_tables_exist, $extra_database_fields_exist) = (0, 0); + my ($extra_database_tables_exist, $extra_database_fields_exist, $incorrect_type_database_fields_exist) = (0, 0, 0); my $status_output = $c->c; @@ -1354,8 +1354,9 @@ sub upgrade_course_confirm ($c) { # Report on database status my ($tables_ok, $dbStatus) = $CIchecker->checkCourseTables($upgrade_courseID); - my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes, $db_report) = - $c->formatReportOnDatabaseTables($dbStatus, $upgrade_courseID); + my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes, + $incorrect_type_database_fields, $db_report) + = $c->formatReportOnDatabaseTables($dbStatus, $upgrade_courseID); my $course_output = $c->c; @@ -1428,6 +1429,24 @@ sub upgrade_course_confirm ($c) { ); } + if ($incorrect_type_database_fields) { + $incorrect_type_database_fields_exist = 1; + push( + @$course_output, + $c->tag( + 'p', + class => 'text-danger fw-bold', + $c->maketext( + 'There are database fields that do not have the same type as the field defined in the schema ' + . 'for at least one table. Check the checkbox by the field to change its type when ' + . 'upgrading the course. Warning: This can fail which may corrupt the table. If you have ' + . 'not archived this course, then do that now before upgrading if you want to change the ' + . 'type of any of these fields.' + ) + ) + ); + } + # Report on directory status my ($directories_ok, $directory_report) = checkCourseDirectories($ce2); push(@$course_output, $c->tag('div', class => 'mb-2', $c->maketext('Directory structure:'))); @@ -1464,10 +1483,11 @@ sub upgrade_course_confirm ($c) { return $c->include( 'ContentGenerator/CourseAdmin/upgrade_course_confirm', - upgrade_courseIDs => \@upgrade_courseIDs, - extra_database_tables_exist => $extra_database_tables_exist, - extra_database_fields_exist => $extra_database_fields_exist, - status_output => $status_output->join('') + upgrade_courseIDs => \@upgrade_courseIDs, + extra_database_tables_exist => $extra_database_tables_exist, + extra_database_fields_exist => $extra_database_fields_exist, + incorrect_type_database_fields_exist => $incorrect_type_database_fields_exist, + status_output => $status_output->join('') ); } @@ -1503,8 +1523,10 @@ sub do_upgrade_course ($c) { push( @upgrade_report, $CIchecker->updateTableFields( - $upgrade_courseID, $table_name, - [ ($c->param("$upgrade_courseID.$table_name.delete_fieldIDs")) ] + $upgrade_courseID, + $table_name, + [ ($c->param("$upgrade_courseID.$table_name.delete_fieldIDs")) ], + [ ($c->param("$upgrade_courseID.$table_name.fix_type_fieldIDs")) ], ) ); } @@ -1512,8 +1534,9 @@ sub do_upgrade_course ($c) { # Analyze database status and prepare status report ($tables_ok, $dbStatus) = $CIchecker->checkCourseTables($upgrade_courseID); - my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes, $db_report) = - $c->formatReportOnDatabaseTables($dbStatus); + my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes, + $incorrect_type_database_fields, $db_report) + = $c->formatReportOnDatabaseTables($dbStatus); # Prepend course name $db_report = $c->c($c->tag('div', class => 'mb-2', $c->maketext('Database:')), $db_report); @@ -1541,6 +1564,20 @@ sub do_upgrade_course ($c) { ); } + if ($incorrect_type_database_fields) { + push( + @$db_report, + $c->tag( + 'p', + class => 'text-danger fw-bold', + $c->maketext( + 'There are database fields that do not have the same type as the ' + . 'field defined in the schema for at least one table.' + ) + ) + ); + } + # Add missing directories and prepare report on directory status my $dir_update_messages = updateCourseDirectories($ce2); # Needs more error messages my ($directories_ok, $directory_report) = checkCourseDirectories($ce2); @@ -2684,10 +2721,11 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) { ) ); - my $all_tables_ok = 1; - my $extra_database_tables = 0; - my $extra_database_fields = 0; - my $rebuild_table_indexes = 0; + my $all_tables_ok = 1; + my $extra_database_tables = 0; + my $extra_database_fields = 0; + my $rebuild_table_indexes = 0; + my $incorrect_type_database_fields = 0; my $db_report = $c->c; @@ -2728,9 +2766,35 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) { } else { $extra_database_fields = 1; } - if ($fieldInfo{$key}[1]) { - push(@$field_report, $c->hidden_field("$courseID.$table.delete_fieldIDs" => $key)); - } else { + if (defined $courseID) { + if ($fieldInfo{$key}[1]) { + push(@$field_report, $c->hidden_field("$courseID.$table.delete_fieldIDs" => $key)); + } else { + push( + @$field_report, + $c->tag( + 'span', + class => 'form-check d-inline-block', + $c->tag( + 'label', + class => 'form-check-label', + $c->c( + $c->check_box( + "$courseID.$table.delete_fieldIDs" => $key, + class => 'form-check-input' + ), + $c->maketext('Delete field when upgrading') + )->join('') + ) + ) + ); + } + } + } elsif ($field_status == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_A) { + $all_tables_ok = 0; + } elsif ($field_status == WeBWorK::Utils::CourseDBIntegrityCheck::DIFFER_IN_A_AND_B) { + $incorrect_type_database_fields = 1; + if (defined $courseID) { push( @$field_report, $c->tag( @@ -2741,17 +2805,15 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) { class => 'form-check-label', $c->c( $c->check_box( - "$courseID.$table.delete_fieldIDs" => $key, - class => 'form-check-input' + "$courseID.$table.fix_type_fieldIDs" => $key, + class => 'form-check-input' ), - $c->maketext('Delete field when upgrading') + $c->maketext('Change type of field when upgrading') )->join('') ) ) ); } - } elsif ($field_status == WeBWorK::Utils::CourseDBIntegrityCheck::ONLY_IN_A) { - $all_tables_ok = 0; } push(@$fields_report, $c->tag('li', $field_report->join(''))); } @@ -2765,8 +2827,9 @@ sub formatReportOnDatabaseTables ($c, $dbStatus, $courseID = undef) { push(@$db_report, $c->tag('p', class => 'text-success', $c->maketext('Database tables are ok'))) if $all_tables_ok; return ( - $all_tables_ok, $extra_database_tables, $extra_database_fields, - $rebuild_table_indexes, $db_report->join('') + $all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes, + $incorrect_type_database_fields, + $db_report->join('') ); } diff --git a/lib/WeBWorK/DB/Schema/NewSQL/Std.pm b/lib/WeBWorK/DB/Schema/NewSQL/Std.pm index 497db424df..794713200c 100644 --- a/lib/WeBWorK/DB/Schema/NewSQL/Std.pm +++ b/lib/WeBWorK/DB/Schema/NewSQL/Std.pm @@ -368,6 +368,24 @@ sub _drop_column_field_stmt { return "Alter table `$sql_table_name` drop column `$sql_field_name` "; } +#################################################### +# Change the type of a column to the type defined in the schema +#################################################### + +sub change_column_field_type { + my ($self, $field_name) = @_; + return 0 unless defined $self->{record}->FIELD_DATA->{$field_name}; + eval { + $self->dbh->do('ALTER TABLE `' + . $self->sql_table_name + . '` MODIFY ' + . $self->sql_field_name($field_name) . ' ' + . $self->{record}->FIELD_DATA->{$field_name}{type} + . ';'); + }; + return $@ ? 0 : 1; +} + #################################################### # rebuild indexes for the table #################################################### @@ -375,9 +393,8 @@ sub _drop_column_field_stmt { sub rebuild_indexes { my ($self) = @_; - my $sql_table_name = $self->sql_table_name; - my $field_data = $self->field_data; - my %override_fields = reverse %{ $self->{params}{fieldOverride} }; + my $sql_table_name = $self->sql_table_name; + my $field_data = $self->field_data; # A key field column is going to be removed. The schema will not have the information for this column. So the # indexes need to be obtained from the database. Note that each element of the returned array is an array reference @@ -398,7 +415,7 @@ sub rebuild_indexes { my $column = (grep { $index->[4] eq $_->[0] } @$columns)[0]; if (defined $column && $column->[5] =~ m/AUTO_INCREMENT/i) { $self->dbh->do("ALTER TABLE `$sql_table_name` MODIFY `$column->[0]` $column->[1]"); - push @auto_increment_fields, $override_fields{ $column->[0] } // $column->[0]; + push @auto_increment_fields, $column->[0]; } $self->dbh->do("ALTER TABLE `$sql_table_name` DROP INDEX `$index->[2]`"); diff --git a/lib/WeBWorK/DB/Schema/NewSQL/Versioned.pm b/lib/WeBWorK/DB/Schema/NewSQL/Versioned.pm index 6f3e3f8a23..c91808dff0 100644 --- a/lib/WeBWorK/DB/Schema/NewSQL/Versioned.pm +++ b/lib/WeBWorK/DB/Schema/NewSQL/Versioned.pm @@ -97,8 +97,6 @@ sub where_user_id_eq_set_id_eq_problem_id_eq { # FIXME the rest of the places in this class that generate field lists (basically # anywhere that calls grok_*_from_vsetID_sql), should call this method instead. -# this method can handle if the set_id field has a fieldOverride set for it, and -# the other methods can't. sub sql_field_expression { my ($self, $field, $table) = @_; diff --git a/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm b/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm index 20f7760941..44d3e3b1e4 100644 --- a/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm +++ b/lib/WeBWorK/Utils/CourseDBIntegrityCheck.pm @@ -211,11 +211,9 @@ sub checkTableFields { exists $db->{$table}{params}{tableOverride} ? $db->{$table}{params}{tableOverride} : $table; warn "$table_name is a non native table" if $db->{$table}{params}{non_native}; my @schema_field_names = $db->{$table}{record}->FIELDS; - my %schema_override_field_names; + my %schema_field_names = map { $_ => 1 } @schema_field_names; for my $field (@schema_field_names) { - my $field_name = $db->{$table}{params}{fieldOverride}{$field} || $field; - $schema_override_field_names{$field_name} = $field; - if ($db->{$table}->tableFieldExists($field_name)) { + if ($db->{$table}->tableFieldExists($field)) { $fieldStatus{$field} = [SAME_IN_A_AND_B]; } else { $fieldStatus{$field} = [ONLY_IN_A]; @@ -229,10 +227,19 @@ sub checkTableFields { my %database_fields = map { ${$_}[0] => $_ } @$result; # Construct a hash of field names to field data. for my $field_name (keys %database_fields) { - unless (exists($schema_override_field_names{$field_name})) { + unless (exists($schema_field_names{$field_name})) { $fields_ok = 0; $fieldStatus{$field_name} = [ONLY_IN_B]; push(@{ $fieldStatus{$field_name} }, 1) if $database_fields{$field_name}[3]; + } else { + my $data_type = $database_fields{$field_name}[1]; + $data_type =~ s/\(\d*\)$// if $data_type =~ /^(big|small)?int\(\d*\)$/; + $data_type = lc($data_type); + my $schema_data_type = lc($db->{$table}{record}->FIELD_DATA->{$field_name}{type} =~ s/ .*$//r); + if ($data_type ne $schema_data_type) { + $fieldStatus{$field_name} = [DIFFER_IN_A_AND_B]; + $fields_ok = 0; + } } } @@ -249,7 +256,7 @@ the same as the ones specified by the databaseLayout =cut sub updateTableFields { - my ($self, $courseName, $table, $delete_field_names) = @_; + my ($self, $courseName, $table, $delete_field_names, $fix_type_field_names) = @_; my @messages; # Fetch schema from course environment and search database for corresponding tables. @@ -290,6 +297,24 @@ sub updateTableFields { } } + # Change types of fields list in $fix_type_field_names to the type defined in the schema. + for my $field_name (@$fix_type_field_names) { + if ($fieldStatus->{$field_name} && $fieldStatus->{$field_name}[0] == DIFFER_IN_A_AND_B) { + if ($schema_obj->can('change_column_field_type') && $schema_obj->change_column_field_type($field_name)) { + push(@messages, [ "Changed type of column '$field_name' from table '$table'", 1 ]); + } else { + push( + @messages, + [ + "Failed to changed type of column '$field_name' from table '$table'. " + . 'It is recommended that you delete this course and restore it from an archive.', + 0 + ] + ); + } + } + } + return @messages; } diff --git a/lib/WeBWorK/Utils/CourseManagement.pm b/lib/WeBWorK/Utils/CourseManagement.pm index 206fc98120..cb95b5eba6 100644 --- a/lib/WeBWorK/Utils/CourseManagement.pm +++ b/lib/WeBWorK/Utils/CourseManagement.pm @@ -1303,18 +1303,15 @@ sub initNonNativeTables { #fields in the schema. Its not a huge issue if the database table has extra columns. } else { my %fieldStatus; - my $fields_ok = 1; - my @schema_field_names = $db->{$table}->{record}->FIELDS; - my %schema_override_field_names = (); - foreach my $field (sort @schema_field_names) { - my $field_name = $db->{$table}->{params}->{fieldOverride}->{$field} || $field; - $schema_override_field_names{$field_name} = $field; + my $fields_ok = 1; + my @schema_field_names = $db->{$table}->{record}->FIELDS; + foreach my $field_name (sort @schema_field_names) { my $database_field_exists = $db->{$table}->tableFieldExists($field_name); #if the field doesn't exist then try to add it... if (!$database_field_exists) { $fields_ok = 0; - $fieldStatus{$field} = [ONLY_IN_A]; - warn "$field from $database_table_name (aka |$table|) is only in schema, " + $fieldStatus{$field_name} = [ONLY_IN_A]; + warn "$field_name from $database_table_name (aka |$table|) is only in schema, " . "not in database, so adding it ... "; if ($db->{$table}->can("add_column_field")) { if ($db->{$table}->add_column_field($field_name)) { diff --git a/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep b/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep index 9385d07be3..b70c2b4898 100644 --- a/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep +++ b/templates/ContentGenerator/CourseAdmin/archive_course_confirm.html.ep @@ -6,8 +6,14 @@

    <%= $_->[0] %>

    % } % } -% my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes, $db_report) = - % $c->formatReportOnDatabaseTables($dbStatus); +% my ( + % $all_tables_ok, + % $extra_database_tables, + % $extra_database_fields, + % $rebuild_table_indexes, + % $incorrect_type_database_fields, + % $db_report +% ) = $c->formatReportOnDatabaseTables($dbStatus); <%= $db_report %> % if ($extra_database_tables) {

    @@ -34,6 +40,16 @@ ) =%>

    % } +% if ($incorrect_type_database_fields) { +

    + <%= maketext( + 'There are database fields that do not have the same type as the field defined in the schema for ' + . 'at least one table. The types of these fields can be changed when upgrading the course. ' + . 'Make sure that you archive the course here before doing that, since changing the type of a ' + . 'field can corrupt the table.' + ) =%> +

    +% } % if (!$all_tables_ok) {

    <%= maketext('The course database must be upgraded before archiving this course.') =%> diff --git a/templates/ContentGenerator/CourseAdmin/rename_course_confirm.html.ep b/templates/ContentGenerator/CourseAdmin/rename_course_confirm.html.ep index e32033dc85..4c9322d704 100644 --- a/templates/ContentGenerator/CourseAdmin/rename_course_confirm.html.ep +++ b/templates/ContentGenerator/CourseAdmin/rename_course_confirm.html.ep @@ -6,8 +6,14 @@

    <%= $_->[0] %>

    % } % } -% my ($all_tables_ok, $extra_database_tables, $extra_database_fields, $rebuild_table_indexes, $db_report) = - % $c->formatReportOnDatabaseTables($dbStatus); +% my ( + % $all_tables_ok, + % $extra_database_tables, + % $extra_database_fields, + % $rebuild_table_indexes, + % $incorrect_type_database_fields, + % $db_report +% ) = $c->formatReportOnDatabaseTables($dbStatus); <%= $db_report =%> % if ($extra_database_tables) {

    @@ -34,6 +40,16 @@ ) =%>

    % } +% if ($incorrect_type_database_fields) { +

    + <%= maketext( + 'There are database fields that do not have the same type as the field defined in the schema for ' + . 'at least one table. The types of these fields can be changed when upgrading the course. ' + . 'Make sure that you archive the course before doing that, since changing the type of a ' + . 'field can corrupt the table.' + ) =%> +

    +% } % if (!$all_tables_ok) {

    <%= maketext('The course database must be upgraded before renaming this course.') =%> diff --git a/templates/ContentGenerator/CourseAdmin/upgrade_course_confirm.html.ep b/templates/ContentGenerator/CourseAdmin/upgrade_course_confirm.html.ep index 3d5e4ca7d0..fd1ed16e61 100644 --- a/templates/ContentGenerator/CourseAdmin/upgrade_course_confirm.html.ep +++ b/templates/ContentGenerator/CourseAdmin/upgrade_course_confirm.html.ep @@ -19,6 +19,15 @@ % } + % if ($incorrect_type_database_fields_exist) { +

    + +
    + % } <% end =%> %