From 7b7b92bd96e5e7c9cdc29cb78142f20b8c868fd3 Mon Sep 17 00:00:00 2001
From: Ferdinand Thiessen <opensource@fthiessen.de>
Date: Thu, 21 Dec 2023 14:34:59 +0100
Subject: [PATCH] fixup! use count for better performance

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
---
 lib/Db/SubmissionMapper.php                  | 12 ++++--
 lib/Service/SubmissionService.php            |  8 +---
 tests/Unit/Service/SubmissionServiceTest.php | 42 +++-----------------
 3 files changed, 16 insertions(+), 46 deletions(-)

diff --git a/lib/Db/SubmissionMapper.php b/lib/Db/SubmissionMapper.php
index ed5d0c03d..d5e5d2a76 100644
--- a/lib/Db/SubmissionMapper.php
+++ b/lib/Db/SubmissionMapper.php
@@ -110,16 +110,22 @@ public function findParticipantsByForm(int $formId): array {
 	}
 
 	/**
+	 * Count submissions by form and optionally also by userId
+	 * @param int $formId ID of the form to count submissions for
+	 * @param string|null $userId optionally limit submissions to the one of that user
 	 * @throws \Exception
 	 */
-	public function countSubmissions(int $formId): int {
+	public function countSubmissions(int $formId, ?string $userId = null): int {
 		$qb = $this->db->getQueryBuilder();
 
-		$qb->select($qb->func()->count('*', 'num_submissions'))
+		$query = $qb->select($qb->func()->count('*', 'num_submissions'))
 			->from($this->getTableName())
 			->where($qb->expr()->eq('form_id', $qb->createNamedParameter($formId, IQueryBuilder::PARAM_INT)));
+		if (!is_null($userId)) {
+			$query->andWhere($qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)));
+		}
 
-		$result = $qb->executeQuery();
+		$result = $query->executeQuery();
 		$row = $result->fetch();
 		$result->closeCursor();
 
diff --git a/lib/Service/SubmissionService.php b/lib/Service/SubmissionService.php
index 4cd91501a..538211bec 100644
--- a/lib/Service/SubmissionService.php
+++ b/lib/Service/SubmissionService.php
@@ -158,13 +158,7 @@ public function getSubmissions(int $formId): array {
 	 * Validate the new submission is unique
 	 */
 	public function isUniqueSubmission(Submission $newSubmission): bool {
-		$submissions = $this->submissionMapper->findByForm($newSubmission->getFormId());
-		foreach ($submissions as $submission) {
-			if ($submission->getUserId() === $newSubmission->getUserId() && $submission->getId() !== $newSubmission->getId()) {
-				return false;
-			}
-		}
-		return true;
+		return $this->submissionMapper->countSubmissions($newSubmission->getFormId(), $newSubmission->getUserId()) === 1;
 	}
 
 	/**
diff --git a/tests/Unit/Service/SubmissionServiceTest.php b/tests/Unit/Service/SubmissionServiceTest.php
index e69da8aa2..582dd28e5 100644
--- a/tests/Unit/Service/SubmissionServiceTest.php
+++ b/tests/Unit/Service/SubmissionServiceTest.php
@@ -131,9 +131,10 @@ public function setUp(): void {
 	/**
 	 * @dataProvider dataIsUniqueSubmission
 	 */
-	public function testIsUniqueSubmission(array $submissionData, array $otherSubmissionsData, bool $expected) {
-		$this->submissionMapper->method('findByForm')
-			->willReturn(array_map(fn ($data) => Submission::fromParams($data), $otherSubmissionsData));
+	public function testIsUniqueSubmission(array $submissionData, int $numberOfSubmissions, bool $expected) {
+		$this->submissionMapper->method('countSubmissions')
+			->with($submissionData['formId'], $submissionData['userId'])
+			->willReturn($numberOfSubmissions);
 
 		$submission = Submission::fromParams($submissionData);
 		$this->assertEquals($expected, $this->submissionService->isUniqueSubmission($submission));
@@ -141,33 +142,13 @@ public function testIsUniqueSubmission(array $submissionData, array $otherSubmis
 
 	public function dataIsUniqueSubmission() {
 		return [
-			[
-				'submissionData' => [
-					'id' => 3,
-					'userId' => 'user',
-					'formId' => 1,
-				],
-				'otherSubmissionData' => [
-					[
-						'id' => 1,
-						'userId' => 'other',
-						'formId' => 1,
-					],
-					[
-						'id' => 2,
-						'userId' => 'yetAnother',
-						'formId' => 1,
-					],
-				],
-				'expected' => true,
-			],
 			[
 				'submissionData' => [
 					'id' => 1,
 					'userId' => 'user',
 					'formId' => 1,
 				],
-				'otherSubmissionData' => [],
+				'numberOfSubmissions' => 1,
 				'expected' => true,
 			],
 			[
@@ -176,18 +157,7 @@ public function dataIsUniqueSubmission() {
 					'userId' => 'user',
 					'formId' => 1,
 				],
-				'otherSubmissionData' => [
-					[
-						'id' => 1,
-						'userId' => 'other',
-						'formId' => 1,
-					],
-					[
-						'id' => 2,
-						'formId' => 1,
-						'userId' => 'user', // conflict
-					],
-				],
+				'numberOfSubmissions' => 2,
 				'expected' => false,
 			],
 		];