Skip to content

Commit

Permalink
Split the CourseIntegrityCheck.pm module into two parts.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@drgrice1
drgrice1 committed Dec 19, 2024
1 parent f36d1c5 commit 475de2b
Show file tree
Hide file tree
Showing 10 changed files with 560 additions and 1,341 deletions.
1 change: 0 additions & 1 deletion bin/update-OPL-statistics.pl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ BEGIN
use String::ShellQuote;

use DBI;
use WeBWorK::Utils::CourseIntegrityCheck;
use WeBWorK::Utils::CourseManagement qw/listCourses/;

my $time = time();
Expand Down
25 changes: 8 additions & 17 deletions bin/upgrade_admin_db.pl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down
104 changes: 52 additions & 52 deletions lib/WeBWorK/ContentGenerator/CourseAdmin.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand All @@ -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',
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand All @@ -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',
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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')
Expand All @@ -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,
Expand All @@ -2712,45 +2714,43 @@ 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;

for my $key (keys %fieldInfo) {
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('')));
Expand Down
4 changes: 0 additions & 4 deletions lib/WeBWorK/DB/Record/Depths.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

5 changes: 1 addition & 4 deletions lib/WeBWorK/DB/Record/Setting.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand All @@ -35,10 +33,9 @@ BEGIN {
__PACKAGE__->_initial_records(
{
name => "db_version",
value => 3.1415926 # $WeBWorK::Utils::DBUpgrade::THIS_DB_VERSION
value => 3.1415926
},
);
}

1;

Loading

0 comments on commit 475de2b

Please sign in to comment.