From ec03bfbc84e9c3eca4532ee4498db90048aee8f3 Mon Sep 17 00:00:00 2001 From: Nathan Wallach Date: Thu, 1 Dec 2022 17:49:15 +0200 Subject: [PATCH] Add code to allow adding multiple records to the problem_user table (for a single set for a single user) as a batch, which allows reducing the number of test calls made to the database. It would be better to be able to add all records in a single database call, but at present the underlying database abstraction layer does not seem to have any support for inserting multiple records in one call. --- lib/WeBWorK/ContentGenerator/Instructor.pm | 48 ++++++++++++++++++---- lib/WeBWorK/DB.pm | 47 +++++++++++++++++++++ 2 files changed, 87 insertions(+), 8 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/Instructor.pm b/lib/WeBWorK/ContentGenerator/Instructor.pm index ca6bafb89b..40a964053d 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor.pm @@ -75,10 +75,9 @@ sub assignSetToUser { } my @GlobalProblems = grep { defined $_ } $db->getAllGlobalProblems($setID); - foreach my $GlobalProblem (@GlobalProblems) { - my @result = $self->assignProblemToUser($userID, $GlobalProblem); - push @results, @result if @result and not $set_assigned; - } + + my @result = $self->assignMultipleProblemsToUser($userID, @GlobalProblems); + push @results, @result if @result and not $set_assigned; return @results; } @@ -155,6 +154,41 @@ sub unassignSetFromUser { $db->deleteUserSet($userID, $setID); } +=item assignMultipleProblemsToUser($userID, @GlobalProblems) + +Assigns a list of problems to the given user. +If some problem is already assigned to the user, an error string is returned. + +=cut + +sub assignMultipleProblemsToUser { + my ($self, $userID, @GlobalProblems) = @_; + my $db = $self->{db}; + + my $GlobalProblem; + my $UserProblem; + my @records; + foreach $GlobalProblem (@GlobalProblems) { + $UserProblem = $db->newUserProblem; + $UserProblem->user_id($userID); + $UserProblem->set_id($GlobalProblem->set_id); + $UserProblem->problem_id($GlobalProblem->problem_id); + initializeUserProblem($UserProblem, undef); # No $seed + push(@records, $UserProblem); + } + + eval { $db->addUserMultipleProblem(@records) }; + if ($@) { + if ($@ =~ m/user problems existed/) { + return "some problem in the set " . $GlobalProblem->set_id . " were already assigned to user $userID.\n $@"; + } else { + die $@; + } + } + + return (); +} + =item assignProblemToUser($userID, $GlobalProblem, $seed) Assigns the given problem to the given user. If the problem is already assigned @@ -338,10 +372,8 @@ sub assignSetToAllUsers { debug("$setID: (done with that)"); debug("$setID: adding UserProblems for $userID"); - foreach my $GlobalProblem (@GlobalProblems) { - my @result = $self->assignProblemToUser($userID, $GlobalProblem); - push @results, @result if @result; - } + my @result = $self->assignMultipleProblemsToUser($userID, @GlobalProblems); + push @results, @result if @result; debug("$setID: (done with that)"); } diff --git a/lib/WeBWorK/DB.pm b/lib/WeBWorK/DB.pm index 0748010363..cd9ef28f88 100644 --- a/lib/WeBWorK/DB.pm +++ b/lib/WeBWorK/DB.pm @@ -2051,6 +2051,53 @@ sub addUserProblem { } } +# Adds multiple problems to a user with less database calls. +sub addUserMultipleProblem { + # Intended to be used to add multiple problems to a SINGLE set with less DB calls. + # Not for use by versioned sets + my ($self, @ProblemList) = @_; + + my $UserProblem = $ProblemList[0]; + + my ($nv_set_id, $versionNum) = grok_vsetID($UserProblem->set_id); + croak "addUserMultipleProblem does not support versioned sets" + if defined($versionNum); + + # Possible improvement, if a version set was sent, make calls to addUserProblem for each one. + + # Do a single checkArgs call, all records in the list were made in the same manner + my @tmp = ($UserProblem); + $self->checkArgs(\@tmp, qw/VREC:problem_user/); + + # Just needed once + croak "addUserProblem: user set ", $UserProblem->set_id, " for user ", $UserProblem->user_id, " not found" + unless $self->{set_user}->exists($UserProblem->user_id, $UserProblem->set_id); + + my @sawRecordExists; + my @otherErrors; + my @ToReturn; + my $result1; + + foreach $UserProblem (@ProblemList) { + eval { $result1 = $self->{problem_user}->add($UserProblem); }; + if (my $ex = caught WeBWorK::DB::Ex::RecordExists) { + push(@sawRecordExists, $UserProblem->problem_id); + } elsif ($@) { + push(@otherErrors, "for problem_id $UserProblem->problem_id) saw error: $@\n"); + } + } + + my $msg1 = (@sawRecordExists == 0) ? "" : join("", + "addUserMultipleProblems: the following user problems existed before ", + join(", ", @sawRecordExists), "\n"); + if (@otherErrors) { + die join("", $msg1, @otherErrors); + } elsif (@sawRecordExists) { + croak $msg1; + } + return @ToReturn; +} + # versioned_ok is an optional argument which lets us slip versioned setIDs through checkArgs. sub putUserProblem { my $V = $_[2] ? "V" : "";