Perf: Replace N+1 queries with optimized WHERE IN

lib/Controller/ApiController.php

@@ -1283,14 +1283,8 @@ public function getSubmissions(int $formId, ?string $query = null, ?int $limit =
 				// TRANSLATORS On Results when listing the single Responses to the form, this text is shown as heading of the Response.
 				$submission['userDisplayName'] = $this->l10n->t('Anonymous response');
 			} else {
-				$userEntity = $this->userManager->get($submission['userId']);
-
-				if ($userEntity instanceof IUser) {
-					$submission['userDisplayName'] = $userEntity->getDisplayName();
-				} else {
-					// Fallback, should not occur regularly.
-					$submission['userDisplayName'] = $submission['userId'];
-				}
+				// Fallback to the userId, should not occur regularly.
+				$submission['userDisplayName'] = $this->userManager->getDisplayName($submission['userId']) ?? $submission['userId'];
 			}
 			return $submission;
 		}, $submissions);
@@ -1352,14 +1346,8 @@ public function getSubmission(int $formId, int $submissionId): DataResponse|Data
 			// TRANSLATORS On Results when listing the single Responses to the form, this text is shown as heading of the Response.
 			$submission['userDisplayName'] = $this->l10n->t('Anonymous response');
 		} else {
-			$userEntity = $this->userManager->get($submission['userId']);
-
-			if ($userEntity instanceof IUser) {
-				$submission['userDisplayName'] = $userEntity->getDisplayName();
-			} else {
-				// Fallback, should not occur regularly.
-				$submission['userDisplayName'] = $submission['userId'];
-			}
+			// Fallback to the userId, should not occur regularly.
+			$submission['userDisplayName'] = $this->userManager->getDisplayName($submission['userId']) ?? $submission['userId'];
 		}
 
 		return new DataResponse($submission);

lib/Db/AnswerMapper.php

@@ -15,6 +15,7 @@
  * @extends QBMapper<Answer>
  */
 class AnswerMapper extends QBMapper {
+	private const CHUNK_SIZE = 1000;
 
 	/**
 	 * AnswerMapper constructor.
@@ -41,6 +42,28 @@ public function findBySubmission(int $submissionId): array {
 		return $this->findEntities($qb);
 	}
 
+	/**
+	 * @param list<int> $submissionIds
+	 * @return Answer[]
+	 */
+	public function findBySubmissions(array $submissionIds): array {
+		$answers = [];
+
+		foreach (array_chunk(array_unique($submissionIds), self::CHUNK_SIZE) as $submissionIdsChunk) {
+			$qb = $this->db->getQueryBuilder();
+
+			$qb->select('*')
+				->from($this->getTableName())
+				->where(
+					$qb->expr()->in('submission_id', $qb->createNamedParameter($submissionIdsChunk, IQueryBuilder::PARAM_INT_ARRAY))
+				);
+
+			$answers[] = $this->findEntities($qb);
+		}
+
+		return array_merge([], ...$answers);
+	}
+
 	/**
 	 * @param int $submissionId
 	 */

lib/Db/OptionMapper.php

@@ -10,12 +10,14 @@
 namespace OCA\Forms\Db;
 
 use OCP\AppFramework\Db\QBMapper;
+use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
 
 /**
  * @extends QBMapper<Option>
  */
 class OptionMapper extends QBMapper {
+	private const CHUNK_SIZE = 1000;
 
 	/**
 	 * OptionMapper constructor.
@@ -46,6 +48,27 @@ public function findByQuestion(int|float $questionId, ?string $optionType = null
 		return $this->findEntities($qb);
 	}
 
+	/**
+	 * @param list<int|float> $questionIds
+	 * @return Option[]
+	 */
+	public function findByQuestions(array $questionIds): array {
+		$options = [];
+		foreach (array_chunk(array_unique($questionIds), self::CHUNK_SIZE) as $questionIdsChunk) {
+			$qb = $this->db->getQueryBuilder();
+
+			$qb->select('*')
+				->from($this->getTableName())
+				->where($qb->expr()->in('question_id', $qb->createNamedParameter($questionIdsChunk, IQueryBuilder::PARAM_INT_ARRAY)))
+				->orderBy('order')
+				->addOrderBy('id');
+
+			$options[] = $this->findEntities($qb);
+		}
+
+		return array_merge([], ...$options);
+	}
+
 	public function deleteByQuestion(int $questionId): void {
 		$qb = $this->db->getQueryBuilder();
 

lib/Service/FormsService.php

@@ -11,6 +11,7 @@
 use OCA\Forms\Constants;
 use OCA\Forms\Db\Form;
 use OCA\Forms\Db\FormMapper;
+use OCA\Forms\Db\Option;
 use OCA\Forms\Db\OptionMapper;
 use OCA\Forms\Db\Question;
 use OCA\Forms\Db\QuestionMapper;
@@ -114,9 +115,19 @@ public function getQuestions(int $formId): array {
 		$questionList = [];
 		try {
 			$questionEntities = $this->questionMapper->findByForm($formId);
+
+			$questionIds = array_map(static fn (Question $question) => $question->getId(), $questionEntities);
+			$optionsByQuestion = [];
+			foreach ($this->optionMapper->findByQuestions($questionIds) as $optionEntity) {
+				$optionsByQuestion[$optionEntity->getQuestionId()][] = $optionEntity;
+			}
+
 			foreach ($questionEntities as $questionEntity) {
 				$question = $questionEntity->read();
-				$question['options'] = $this->getOptions($question['id']);
+				$question['options'] = array_map(
+					static fn (Option $option) => $option->read(),
+					$optionsByQuestion[$question['id']] ?? []
+				);
 				$question['accept'] = [];
 				if ($question['type'] === Constants::ANSWER_TYPE_FILE) {
 					if ($question['extraSettings']['allowedFileTypes'] ?? null) {

lib/Service/SubmissionService.php

@@ -17,6 +17,7 @@
 use OCA\Forms\Db\OptionMapper;
 use OCA\Forms\Db\Question;
 use OCA\Forms\Db\QuestionMapper;
+use OCA\Forms\Db\Submission;
 use OCA\Forms\Db\SubmissionMapper;
 use OCA\Forms\Db\UploadedFileMapper;
 use OCA\Forms\ResponseDefinitions;
@@ -110,9 +111,18 @@ public function getSubmissions(int $formId, ?string $userId = null, ?string $que
 		try {
 			$submissionEntities = $this->submissionMapper->findByForm($formId, $userId, $query, $limit, $offset);
 
+			$submissionIds = array_map(static fn (Submission $submission) => $submission->getId(), $submissionEntities);
+			$answersBySubmission = [];
+			foreach ($this->answerMapper->findBySubmissions($submissionIds) as $answer) {
+				$answersBySubmission[$answer->getSubmissionId()][] = $answer;
+			}
+
 			foreach ($submissionEntities as $submissionEntity) {
 				$submission = $submissionEntity->read();
-				$submission['answers'] = $this->getAnswers($submission['id']);
+				$submission['answers'] = array_map(
+					static fn (Answer $answer) => $answer->read(),
+					$answersBySubmission[$submission['id']] ?? []
+				);
 				$submissionList[] = $submission;
 			}
 		} catch (DoesNotExistException) {
@@ -221,6 +231,7 @@ public function getSubmissionsData(Form $form, string $fileFormat, ?File $file =
 			throw new \InvalidArgumentException('Invalid file format');
 		}
 
+		$submissionEntities = [];
 		try {
 			$submissionEntities = $this->submissionMapper->findByForm($form->getId());
 		} catch (DoesNotExistException) {
@@ -239,6 +250,26 @@ public function getSubmissionsData(Form $form, string $fileFormat, ?File $file =
 			$userTimezone = $this->userConfig->getValueString($this->currentUser->getUID(), 'core', 'timezone', $defaultTimeZone);
 		}
 
+		// Fetch all answers of the loaded submissions at once, grouped by submission
+		$submissionIds = array_map(static fn (Submission $submission) => $submission->getId(), $submissionEntities);
+		$answersBySubmission = [];
+		foreach ($this->answerMapper->findBySubmissions($submissionIds) as $answer) {
+			$answersBySubmission[$answer->getSubmissionId()][] = $answer;
+		}
+
+		// Resolve each submitting user's display name only once
+		$userDisplayNames = [];
+		foreach (array_unique(array_map(static fn (Submission $submission) => $submission->getUserId(), $submissionEntities)) as $userId) {
+			$userDisplayNames[$userId] = $this->userManager->getDisplayName($userId);
+		}
+
+		// Fetch all options of the form's questions at once, grouped by question
+		$questionIds = array_map(static fn (Question $question) => $question->getId(), $questions);
+		$optionsByQuestion = [];
+		foreach ($this->optionMapper->findByQuestions($questionIds) as $option) {
+			$optionsByQuestion[$option->getQuestionId()][] = $option;
+		}
+
 		// Process initial header
 		$header = [];
 		$header[] = ['id' => 'submission_id', 'title' => $this->l10n->t('Submission ID')];
@@ -258,7 +289,7 @@ public function getSubmissionsData(Form $form, string $fileFormat, ?File $file =
 		foreach ($questions as $question) {
 			if ($question->getType() === Constants::ANSWER_TYPE_GRID) {
 				$gridCellType = $question->getExtraSettings()['questionType'];
-				$options = $this->optionMapper->findByQuestion($question->getId());
+				$options = $optionsByQuestion[$question->getId()] ?? [];
 
 				foreach ($options as $option) {
 					$optionPerOptionId[$option->getId()] = $option;
@@ -282,7 +313,7 @@ public function getSubmissionsData(Form $form, string $fileFormat, ?File $file =
 					}
 				}
 			} elseif ($question->getType() === Constants::ANSWER_TYPE_RANKING) {
-				$options = $this->optionMapper->findByQuestion($question->getId());
+				$options = $optionsByQuestion[$question->getId()] ?? [];
 				foreach ($options as $option) {
 					$optionPerOptionId[$option->getId()] = $option;
 					$rankingOptionsPerQuestionId[$question->getId()][] = $option->getId();
@@ -307,22 +338,23 @@ public function getSubmissionsData(Form $form, string $fileFormat, ?File $file =
 			$row[] = $submission->getId();
 
 			// User
-			$user = $this->userManager->get($submission->getUserId());
-			if ($user === null) {
+			$userId = $submission->getUserId();
+			$displayName = $userDisplayNames[$userId] ?? null;
+			if ($displayName === null) {
 				// Give empty userId
 				$row[] = '';
 				// TRANSLATORS Shown on export if no Display-Name is available.
 				$row[] = $this->l10n->t('Anonymous user');
 			} else {
-				$row[] = $user->getUID();
-				$row[] = $user->getDisplayName();
+				$row[] = $userId;
+				$row[] = $displayName;
 			}
 
 			// Date
 			$row[] = date_format(date_timestamp_set(new DateTime(), $submission->getTimestamp())->setTimezone(new DateTimeZone($userTimezone)), 'c');
 
 			// Answers, make sure we keep the question order
-			$answers = array_reduce($this->answerMapper->findBySubmission($submission->getId()),
+			$answers = array_reduce($answersBySubmission[$submission->getId()] ?? [],
 				function (array $carry, Answer $answer) use ($questionPerQuestionId, $gridRowsPerQuestionId, $gridColumnsPerQuestionId, $rankingOptionsPerQuestionId, $optionPerOptionId) {
 					$questionId = $answer->getQuestionId();
 					$questionType = isset($questionPerQuestionId[$questionId]) ? $questionPerQuestionId[$questionId]->getType() : null;

tests/Unit/BackgroundJob/CleanupUploadedFilesJobTest.php

@@ -13,13 +13,16 @@
 use OCA\Forms\Db\FormMapper;
 use OCA\Forms\Db\UploadedFile;
 use OCA\Forms\Db\UploadedFileMapper;
+use OCA\Forms\Tests\Unit\UserFolderMockTrait;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Files\IRootFolder;
 use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
 class CleanupUploadedFilesJobTest extends TestCase {
+	use UserFolderMockTrait;
+
 	private IRootFolder|MockObject $rootFolder;
 	private CleanupUploadedFilesJob $cleanupUploadedFilesJob;
 	private FormMapper|MockObject $formMapper;
@@ -60,9 +63,10 @@ public function testHandle() {
 			->method('findUploadedEarlierThan')
 			->willReturn([$uploadedFile]);
 
+		$userFolder = $this->createUserFolderMock();
 		$this->rootFolder->expects($this->atLeastOnce())
 			->method('getUserFolder')
-			->willReturn($this->rootFolder);
+			->willReturn($userFolder);
 
 		$this->cleanupUploadedFilesJob->run([]);
 	}

tests/Unit/Controller/ApiControllerTest.php

@@ -47,6 +47,7 @@ function is_uploaded_file(string|bool|null $filename) {
 use OCA\Forms\Service\ConfirmationEmailService;
 use OCA\Forms\Service\FormsService;
 use OCA\Forms\Service\SubmissionService;
+use OCA\Forms\Tests\Unit\UserFolderMockTrait;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataDownloadResponse;
@@ -71,6 +72,8 @@ function is_uploaded_file(string|bool|null $filename) {
 use Test\TestCase;
 
 class ApiControllerTest extends TestCase {
+	use UserFolderMockTrait;
+
 	private ApiController $apiController;
 	private AnswerMapper|MockObject $answerMapper;
 	private FormMapper|MockObject $formMapper;
@@ -714,7 +717,7 @@ public function testUploadFiles() {
 
 		\OCA\Forms\Controller\is_uploaded_file(true);
 
-		$userFolder = $this->createMock(Folder::class);
+		$userFolder = $this->createUserFolderMock();
 		$userFolder->expects($this->once())
 			->method('nodeExists')
 			->willReturn(true);
@@ -848,7 +851,7 @@ public function testNewSubmission_answers() {
 			->method('add')
 			->with(SyncSubmissionsWithLinkedFileJob::class, ['form_id' => 1]);
 
-		$userFolder = $this->createMock(Folder::class);
+		$userFolder = $this->createUserFolderMock();
 		$userFolder->expects($this->once())
 			->method('nodeExists')
 			->willReturn(true);
@@ -1528,12 +1531,10 @@ public function testGetSubmission_success() {
 			->with(42)
 			->willReturn($submissionDataFromService); // Service returns an array
 
-		$user = $this->createMock(IUser::class);
-		$user->method('getDisplayName')->willReturn('jdoe');
 		$this->userManager->expects($this->once())
-			->method('get')
+			->method('getDisplayName')
 			->with('jdoe')
-			->willReturn($user);
+			->willReturn('jdoe');
 
 		$expectedSubmissionInResponse = $submissionDataFromService;
 		$expectedSubmissionInResponse['userDisplayName'] = 'jdoe';
@@ -1635,7 +1636,7 @@ public function testGetSubmission_userNotFound() {
 			->willReturn($submissionDataFromService); // Service returns an array
 
 		$this->userManager->expects($this->once())
-			->method('get')
+			->method('getDisplayName')
 			->with('nonExistentUser')
 			->willReturn(null);
 

tests/Unit/Controller/ShareApiControllerTest.php

@@ -21,6 +21,7 @@
 use OCA\Forms\Service\ConfigService;
 use OCA\Forms\Service\FormsService;
 use OCA\Forms\Service\UploadedFilesShareService;
+use OCA\Forms\Tests\Unit\UserFolderMockTrait;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\IMapperException;
 use OCP\AppFramework\Http;
@@ -49,6 +50,7 @@ interface MapperException extends Throwable, IMapperException {
 };
 
 class ShareApiControllerTest extends TestCase {
+	use UserFolderMockTrait;
 
 	private ShareApiController $shareApiController;
 	private FormMapper|MockObject $formMapper;
@@ -545,7 +547,7 @@ public function testDeleteShare_cleansUpFileShare(): void {
 
 		// Mock the uploaded files folder lookup
 		$folder = $this->createMock(Folder::class);
-		$userFolder = $this->createMock(Folder::class);
+		$userFolder = $this->createUserFolderMock();
 		$userFolder->expects($this->once())
 			->method('get')
 			->willReturn($folder);
@@ -837,7 +839,7 @@ public function testUpdateShare(array $share, string $formOwner, array $keyValue
 			->with('otherUser')
 			->willReturn($this->createMock(IUser::class));
 
-		$userFolder = $this->createMock(Folder::class);
+		$userFolder = $this->createUserFolderMock();
 
 		$file = $this->createMock(File::class);
 		$file->expects($this->any())