From 413a9f4fb10686b3610094bf8ad18b31fc8e88e3 Mon Sep 17 00:00:00 2001 From: Christian Hartmann Date: Thu, 25 Jun 2026 00:11:20 +0200 Subject: [PATCH 1/4] chore: reduce database queries on exporting responses Signed-off-by: Christian Hartmann --- lib/Db/AnswerMapper.php | 15 ++++++ lib/Db/OptionMapper.php | 16 ++++++ lib/Service/SubmissionService.php | 87 +++++++++++++++++++------------ 3 files changed, 86 insertions(+), 32 deletions(-) diff --git a/lib/Db/AnswerMapper.php b/lib/Db/AnswerMapper.php index 2420f6b34..66e9c9464 100644 --- a/lib/Db/AnswerMapper.php +++ b/lib/Db/AnswerMapper.php @@ -41,6 +41,21 @@ public function findBySubmission(int $submissionId): array { return $this->findEntities($qb); } + /** + * @param int $formId + * @return Answer[] + */ + public function findByForm(int $formId): array { + $qb = $this->db->getQueryBuilder(); + + $qb->select('a.*') + ->from($this->getTableName(), 'a') + ->join('a', 'forms_v2_submissions', 's', $qb->expr()->eq('a.submission_id', 's.id')) + ->where($qb->expr()->eq('s.form_id', $qb->createNamedParameter($formId, IQueryBuilder::PARAM_INT))); + + return $this->findEntities($qb); + } + /** * @param int $submissionId */ diff --git a/lib/Db/OptionMapper.php b/lib/Db/OptionMapper.php index 2e0140dbe..7b6160ceb 100644 --- a/lib/Db/OptionMapper.php +++ b/lib/Db/OptionMapper.php @@ -46,6 +46,22 @@ public function findByQuestion(int|float $questionId, ?string $optionType = null return $this->findEntities($qb); } + /** + * @param int $formId + * @return Option[] + */ + public function findByForm(int $formId): array { + $qb = $this->db->getQueryBuilder(); + + $qb->select('o.*') + ->from($this->getTableName(), 'o') + ->join('o', 'forms_v2_questions', 'q', $qb->expr()->eq('o.question_id', 'q.id')) + ->where($qb->expr()->eq('q.form_id', $qb->createNamedParameter($formId))) + ->orderBy('o.order'); + + return $this->findEntities($qb); + } + public function deleteByQuestion(int $questionId): void { $qb = $this->db->getQueryBuilder(); diff --git a/lib/Service/SubmissionService.php b/lib/Service/SubmissionService.php index 21b524b75..e2de8adca 100644 --- a/lib/Service/SubmissionService.php +++ b/lib/Service/SubmissionService.php @@ -230,11 +230,29 @@ public function getSubmissionsData(Form $form, string $fileFormat, ?File $file = $questions = $this->questionMapper->findByForm($form->getId()); $defaultTimeZone = $this->config->getSystemValueString('default_timezone', 'UTC'); + $userTimezone = $this->currentUser + ? $this->config->getUserValue($this->currentUser->getUID(), 'core', 'timezone', $defaultTimeZone) + : $this->config->getUserValue($form->getOwnerId(), 'core', 'timezone', $defaultTimeZone); + + $allAnswers = $this->answerMapper->findByForm($form->getId()); + $answersBySubmission = []; + foreach ($allAnswers as $answer) { + $answersBySubmission[$answer->getSubmissionId()][] = $answer; + } - if (!$this->currentUser) { - $userTimezone = $this->config->getUserValue($form->getOwnerId(), 'core', 'timezone', $defaultTimeZone); - } else { - $userTimezone = $this->config->getUserValue($this->currentUser->getUID(), 'core', 'timezone', $defaultTimeZone); + $userIds = array_map(fn ($submission) => $submission->getUserId(), $submissionEntities); + $userDisplayNames = []; + foreach (array_unique($userIds) as $userId) { + $user = $this->userManager->get($userId); + if ($user !== null) { + $userDisplayNames[$user->getUID()] = $user->getDisplayName(); + } + } + + $allOptions = $this->optionMapper->findByForm($form->getId()); + $optionsByQuestion = []; + foreach ($allOptions as $option) { + $optionsByQuestion[$option->getQuestionId()][] = $option; } // Process initial header @@ -253,45 +271,50 @@ public function getSubmissionsData(Form $form, string $fileFormat, ?File $file = $optionPerOptionId = []; foreach ($questions as $question) { + $questionId = $question->getId(); + $questionPerQuestionId[$questionId] = $question; + $questionOptions = $optionsByQuestion[$questionId] ?? []; + + foreach ($questionOptions as $option) { + $optionPerOptionId[$option->getId()] = $option; + } + if ($question->getType() === Constants::ANSWER_TYPE_GRID) { $gridCellType = $question->getExtraSettings()['questionType']; - $options = $this->optionMapper->findByQuestion($question->getId()); + $gridRowsPerQuestionId[$questionId] = []; + $gridColumnsPerQuestionId[$questionId] = []; - foreach ($options as $option) { - $optionPerOptionId[$option->getId()] = $option; + foreach ($questionOptions as $option) { if ($option->getOptionType() === Option::OPTION_TYPE_ROW) { - $gridRowsPerQuestionId[$question->getId()][] = $option->getId(); + $gridRowsPerQuestionId[$questionId][] = $option->getId(); } if ($option->getOptionType() === Option::OPTION_TYPE_COLUMN) { - $gridColumnsPerQuestionId[$question->getId()][] = $option->getId(); + $gridColumnsPerQuestionId[$questionId][] = $option->getId(); } } - foreach ($gridRowsPerQuestionId[$question->getId()] as $rowId) { + foreach ($gridRowsPerQuestionId[$questionId] as $rowId) { if ($gridCellType === Constants::ANSWER_GRID_TYPE_CHECKBOX || $gridCellType === Constants::ANSWER_GRID_TYPE_RADIO) { $header[] = $question->getText() . ' (' . $optionPerOptionId[$rowId]->getText() . ')'; } if ($gridCellType === Constants::ANSWER_GRID_TYPE_NUMBER) { - foreach ($gridColumnsPerQuestionId[$question->getId()] as $columnId) { + foreach ($gridColumnsPerQuestionId[$questionId] as $columnId) { $header[] = $question->getText() . ' (' . $optionPerOptionId[$rowId]->getText() . ' - ' . $optionPerOptionId[$columnId]->getText() . ')'; } } } } elseif ($question->getType() === Constants::ANSWER_TYPE_RANKING) { - $options = $this->optionMapper->findByQuestion($question->getId()); - foreach ($options as $option) { - $optionPerOptionId[$option->getId()] = $option; - $rankingOptionsPerQuestionId[$question->getId()][] = $option->getId(); + $rankingOptionsPerQuestionId[$questionId] = []; + foreach ($questionOptions as $option) { + $rankingOptionsPerQuestionId[$questionId][] = $option->getId(); } - foreach ($rankingOptionsPerQuestionId[$question->getId()] as $optionId) { + foreach ($rankingOptionsPerQuestionId[$questionId] as $optionId) { $header[] = $question->getText() . ' (' . $optionPerOptionId[$optionId]->getText() . ')'; } } else { $header[] = $question->getText(); } - - $questionPerQuestionId[$question->getId()] = $question; } // Init dataset @@ -300,27 +323,21 @@ public function getSubmissionsData(Form $form, string $fileFormat, ?File $file = // Process each answers foreach ($submissionEntities as $submission) { $row = []; + $userId = $submission->getUserId(); // User - $user = $this->userManager->get($submission->getUserId()); - if ($user === 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[] = $userDisplayNames[$userId] ?? $this->l10n->t('Anonymous user'); // 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()), + $submissionAnswers = $answersBySubmission[$submission->getId()] ?? []; + $answers = array_reduce($submissionAnswers, function (array $carry, Answer $answer) use ($questionPerQuestionId, $gridRowsPerQuestionId, $gridColumnsPerQuestionId, $rankingOptionsPerQuestionId, $optionPerOptionId) { $questionId = $answer->getQuestionId(); - $questionType = isset($questionPerQuestionId[$questionId]) ? $questionPerQuestionId[$questionId]->getType() : null; + $questionType = $questionPerQuestionId[$questionId]->getType() ?? null; if ($questionType === Constants::ANSWER_TYPE_FILE) { if (array_key_exists($questionId, $carry)) { @@ -332,12 +349,18 @@ function (array $carry, Answer $answer) use ($questionPerQuestionId, $gridRowsPe ]; } } elseif ($questionType === Constants::ANSWER_TYPE_GRID) { - $gridCellType = isset($questionPerQuestionId[$questionId]) ? $questionPerQuestionId[$questionId]->getExtraSettings()['questionType'] : null; + $gridCellType = $questionPerQuestionId[$questionId]->getExtraSettings()['questionType'] ?? null; $answerText = json_decode($answer->getText(), true); $columns = []; foreach ($gridRowsPerQuestionId[$questionId] as $row) { if (empty($answerText[$row])) { - $columns[] = ''; + if ($gridCellType === Constants::ANSWER_GRID_TYPE_NUMBER) { + foreach ($gridColumnsPerQuestionId[$questionId] as $column) { + $columns[] = ''; + } + } else { + $columns[] = ''; + } continue; } From 5ea3a9764447cabbb9e9008283df200c19b2b483 Mon Sep 17 00:00:00 2001 From: Christian Hartmann Date: Fri, 26 Jun 2026 20:41:28 +0200 Subject: [PATCH 2/4] fix tests Signed-off-by: Christian Hartmann --- tests/Unit/Service/SubmissionServiceTest.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/Unit/Service/SubmissionServiceTest.php b/tests/Unit/Service/SubmissionServiceTest.php index 08266087b..c08302561 100644 --- a/tests/Unit/Service/SubmissionServiceTest.php +++ b/tests/Unit/Service/SubmissionServiceTest.php @@ -770,16 +770,16 @@ private function setUpCsvTest(array $questions, array $submissions, string $csvT ['unknown', null] ]); - $this->answerMapper->expects($this->any()) - ->method('findBySubmission') - // Return AnswerObjects for corresponding submission - ->will($this->returnCallback(function (int $submissionId) use ($submissions) { - $matchingSubmission = array_filter($submissions, fn ($submission) => $submission['id'] === $submissionId); - - $answerEntities = array_map(fn ($answer) => Answer::fromParams($answer), current($matchingSubmission)['answers']); - - return $answerEntities; - })); + $this->answerMapper->method('findByForm') + ->with(5) + ->willReturnCallback(function () use ($submissions) { + $allAnswers = []; + foreach ($submissions as $submission) { + $answers = array_map(fn ($answer) => Answer::fromParams($answer), $submission['answers']); + $allAnswers = array_merge($allAnswers, $answers); + } + return $allAnswers; + }); // Prepend BOM-Sequence as Writer does and remove formatting-artefacts of dataProvider. $dataExpectation = chr(239) . chr(187) . chr(191) . ltrim((string)preg_replace('/\t+/', '', $csvText)); From cbb29eaaf131f2790687b2ecad0c8bca89e8849c Mon Sep 17 00:00:00 2001 From: Christian Hartmann Date: Fri, 26 Jun 2026 20:52:30 +0200 Subject: [PATCH 3/4] revert Signed-off-by: Christian Hartmann --- tests/Unit/Service/SubmissionServiceTest.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/Unit/Service/SubmissionServiceTest.php b/tests/Unit/Service/SubmissionServiceTest.php index c08302561..08266087b 100644 --- a/tests/Unit/Service/SubmissionServiceTest.php +++ b/tests/Unit/Service/SubmissionServiceTest.php @@ -770,16 +770,16 @@ private function setUpCsvTest(array $questions, array $submissions, string $csvT ['unknown', null] ]); - $this->answerMapper->method('findByForm') - ->with(5) - ->willReturnCallback(function () use ($submissions) { - $allAnswers = []; - foreach ($submissions as $submission) { - $answers = array_map(fn ($answer) => Answer::fromParams($answer), $submission['answers']); - $allAnswers = array_merge($allAnswers, $answers); - } - return $allAnswers; - }); + $this->answerMapper->expects($this->any()) + ->method('findBySubmission') + // Return AnswerObjects for corresponding submission + ->will($this->returnCallback(function (int $submissionId) use ($submissions) { + $matchingSubmission = array_filter($submissions, fn ($submission) => $submission['id'] === $submissionId); + + $answerEntities = array_map(fn ($answer) => Answer::fromParams($answer), current($matchingSubmission)['answers']); + + return $answerEntities; + })); // Prepend BOM-Sequence as Writer does and remove formatting-artefacts of dataProvider. $dataExpectation = chr(239) . chr(187) . chr(191) . ltrim((string)preg_replace('/\t+/', '', $csvText)); From 09fc66357951ddc6c680db19fff248146c15ac42 Mon Sep 17 00:00:00 2001 From: Christian Hartmann Date: Fri, 26 Jun 2026 20:58:03 +0200 Subject: [PATCH 4/4] fix tests Signed-off-by: Christian Hartmann --- tests/Unit/Service/SubmissionServiceTest.php | 21 ++++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/tests/Unit/Service/SubmissionServiceTest.php b/tests/Unit/Service/SubmissionServiceTest.php index 08266087b..9a18c30a0 100644 --- a/tests/Unit/Service/SubmissionServiceTest.php +++ b/tests/Unit/Service/SubmissionServiceTest.php @@ -755,6 +755,16 @@ private function setUpCsvTest(array $questions, array $submissions, string $csvT ->with('currentUser', 'core', 'timezone', 'Europe/Berlin') ->willReturn('Europe/Berlin'); + $allAnswers = []; + foreach ($submissions as $submission) { + if (isset($submission['answers'])) { + foreach ($submission['answers'] as $answer) { + $allAnswers[] = Answer::fromParams($answer); + } + } + } + $this->answerMapper->method('findByForm')->with(5)->willReturn($allAnswers); + $user = $this->createMock(IUser::class); $user->expects($this->any()) ->method('getUID') @@ -770,17 +780,6 @@ private function setUpCsvTest(array $questions, array $submissions, string $csvT ['unknown', null] ]); - $this->answerMapper->expects($this->any()) - ->method('findBySubmission') - // Return AnswerObjects for corresponding submission - ->will($this->returnCallback(function (int $submissionId) use ($submissions) { - $matchingSubmission = array_filter($submissions, fn ($submission) => $submission['id'] === $submissionId); - - $answerEntities = array_map(fn ($answer) => Answer::fromParams($answer), current($matchingSubmission)['answers']); - - return $answerEntities; - })); - // Prepend BOM-Sequence as Writer does and remove formatting-artefacts of dataProvider. $dataExpectation = chr(239) . chr(187) . chr(191) . ltrim((string)preg_replace('/\t+/', '', $csvText));