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.
openwebwork · Dec 1, 2022 · 6e381b1 · 6e381b1
1 parent 073e9f5
commit 6e381b1
Show file tree

Hide file tree

Showing 2 changed files with 96 additions and 17 deletions.
diff --git a/lib/WeBWorK/ContentGenerator/Instructor.pm b/lib/WeBWorK/ContentGenerator/Instructor.pm
@@ -74,11 +74,10 @@ 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 @globalProblems = grep { defined $_ } $db->getAllGlobalProblems($setID);
+
+	my @result = $self->assignMultipleProblemsToUser($userID, @globalProblems);
+	push @results, @result if @result and not $set_assigned;
 
 	return @results;
 }
@@ -155,6 +154,42 @@ 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 @records;
+	for my $globalProblem (@globalProblems) {
+		my $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->addUserMultipleProblems(@records) };
+	if ($@) {
+		if ($@ =~ m/user problems existed/) {
+			return
+				'some problem in the set '
+				. $globalProblems[0]->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
@@ -318,30 +353,28 @@ sub assignSetToAllUsers {
 	debug("$setID: (done with that)");
 
 	debug("$setID: getting problem list");
-	my @GlobalProblems = $db->getAllGlobalProblems($setID);
+	my @globalProblems = $db->getAllGlobalProblems($setID);
 	debug("$setID: (done with that)");
 
 	my @results;
 
-	foreach my $User (@userRecords) {
-		next unless $self->r->ce->status_abbrev_has_behavior($User->status, "include_in_assignment");
-		my $UserSet = $db->newUserSet;
-		my $userID  = $User->user_id;
-		$UserSet->user_id($userID);
-		$UserSet->set_id($setID);
+	foreach my $user (@userRecords) {
+		next unless $self->r->ce->status_abbrev_has_behavior($user->status, "include_in_assignment");
+		my $userSet = $db->newUserSet;
+		my $userID  = $user->user_id;
+		$userSet->user_id($userID);
+		$userSet->set_id($setID);
 		debug("$setID: adding UserSet for $userID");
-		eval { $db->addUserSet($UserSet) };
+		eval { $db->addUserSet($userSet) };
 		if ($@) {
 			next if $@ =~ m/user set exists/;
 			die $@;
 		}
 		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
@@ -2051,6 +2051,52 @@ sub addUserProblem {
 	}
 }
 
+# Used to add multiple problems to a SINGLE set with less DB calls.
+# Not for use by versioned sets
+sub addUserMultipleProblems {
+	my ($self, @problemList) = @_;
+
+	my $firstUserProblem = $problemList[0];
+
+	my ($nv_set_id, $versionNum) = grok_vsetID($firstUserProblem->set_id);
+	croak "addUserMultipleProblems 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 = ($firstUserProblem);
+	$self->checkArgs(\@tmp, qw/VREC:problem_user/);
+
+	# Just needed once
+	croak 'addMultipleUserProblems: user set ', $firstUserProblem->set_id, ' for user ', $firstUserProblem->user_id,
+		' not found'
+		unless $self->{set_user}->exists($firstUserProblem->user_id, $firstUserProblem->set_id);
+
+	my @sawRecordExists;
+	my @otherErrors;
+
+	for my $userProblem (@problemList) {
+		eval { $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");
+		}
+	}
+	if (@sawRecordExists || @otherErrors) {
+		croak join(
+			'',
+			@sawRecordExists
+			? join('',
+				'addUserMultipleProblems: the following user problems existed before ',
+				join(', ', @sawRecordExists), "\n")
+			: '',
+			@otherErrors
+		);
+	}
+}
+
 # versioned_ok is an optional argument which lets us slip versioned setIDs through checkArgs.
 sub putUserProblem {
 	my $V = $_[2] ? "V" : "";