Skip to content

Commit 8fb8626

Browse files
KocKoc
committed
Perf: Replace N+1 queries with optimized WHERE IN (try fix pipeline)
Signed-off-by: Kostiantyn Miakshyn <molodchick@gmail.com>
1 parent e5c24eb commit 8fb8626

9 files changed

Lines changed: 61 additions & 19 deletions

‎appinfo/info.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
<screenshot>https://raw.githubusercontent.com/nextcloud/forms/main/screenshots/forms3.png</screenshot>
6262

6363
<dependencies>
64-
<nextcloud min-version="33" max-version="36" />
64+
<nextcloud min-version="33" max-version="35" />
6565
</dependencies>
6666

6767
<background-jobs>

‎tests/Unit/BackgroundJob/CleanupUploadedFilesJobTest.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,16 @@
1313
use OCA\Forms\Db\FormMapper;
1414
use OCA\Forms\Db\UploadedFile;
1515
use OCA\Forms\Db\UploadedFileMapper;
16+
use OCA\Forms\Tests\Unit\UserFolderMockTrait;
1617
use OCP\AppFramework\Utility\ITimeFactory;
1718
use OCP\Files\IRootFolder;
1819
use PHPUnit\Framework\MockObject\MockObject;
1920
use Psr\Log\LoggerInterface;
2021
use Test\TestCase;
2122

2223
class CleanupUploadedFilesJobTest extends TestCase {
24+
use UserFolderMockTrait;
25+
2326
private IRootFolder|MockObject $rootFolder;
2427
private CleanupUploadedFilesJob $cleanupUploadedFilesJob;
2528
private FormMapper|MockObject $formMapper;
@@ -60,9 +63,10 @@ public function testHandle() {
6063
->method('findUploadedEarlierThan')
6164
->willReturn([$uploadedFile]);
6265

66+
$userFolder = $this->createUserFolderMock();
6367
$this->rootFolder->expects($this->atLeastOnce())
6468
->method('getUserFolder')
65-
->willReturn($this->rootFolder);
69+
->willReturn($userFolder);
6670

6771
$this->cleanupUploadedFilesJob->run([]);
6872
}

‎tests/Unit/Controller/ApiControllerTest.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ function is_uploaded_file(string|bool|null $filename) {
4747
use OCA\Forms\Service\ConfirmationEmailService;
4848
use OCA\Forms\Service\FormsService;
4949
use OCA\Forms\Service\SubmissionService;
50+
use OCA\Forms\Tests\Unit\UserFolderMockTrait;
5051
use OCP\AppFramework\Db\DoesNotExistException;
5152
use OCP\AppFramework\Http;
5253
use OCP\AppFramework\Http\DataDownloadResponse;
@@ -71,6 +72,8 @@ function is_uploaded_file(string|bool|null $filename) {
7172
use Test\TestCase;
7273

7374
class ApiControllerTest extends TestCase {
75+
use UserFolderMockTrait;
76+
7477
private ApiController $apiController;
7578
private AnswerMapper|MockObject $answerMapper;
7679
private FormMapper|MockObject $formMapper;
@@ -714,7 +717,7 @@ public function testUploadFiles() {
714717

715718
\OCA\Forms\Controller\is_uploaded_file(true);
716719

717-
$userFolder = $this->createMock(Folder::class);
720+
$userFolder = $this->createUserFolderMock();
718721
$userFolder->expects($this->once())
719722
->method('nodeExists')
720723
->willReturn(true);
@@ -848,7 +851,7 @@ public function testNewSubmission_answers() {
848851
->method('add')
849852
->with(SyncSubmissionsWithLinkedFileJob::class, ['form_id' => 1]);
850853

851-
$userFolder = $this->createMock(Folder::class);
854+
$userFolder = $this->createUserFolderMock();
852855
$userFolder->expects($this->once())
853856
->method('nodeExists')
854857
->willReturn(true);

‎tests/Unit/Controller/ShareApiControllerTest.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use OCA\Forms\Service\ConfigService;
2222
use OCA\Forms\Service\FormsService;
2323
use OCA\Forms\Service\UploadedFilesShareService;
24+
use OCA\Forms\Tests\Unit\UserFolderMockTrait;
2425
use OCP\AppFramework\Db\DoesNotExistException;
2526
use OCP\AppFramework\Db\IMapperException;
2627
use OCP\AppFramework\Http;
@@ -49,6 +50,7 @@ interface MapperException extends Throwable, IMapperException {
4950
};
5051

5152
class ShareApiControllerTest extends TestCase {
53+
use UserFolderMockTrait;
5254

5355
private ShareApiController $shareApiController;
5456
private FormMapper|MockObject $formMapper;
@@ -545,7 +547,7 @@ public function testDeleteShare_cleansUpFileShare(): void {
545547

546548
// Mock the uploaded files folder lookup
547549
$folder = $this->createMock(Folder::class);
548-
$userFolder = $this->createMock(Folder::class);
550+
$userFolder = $this->createUserFolderMock();
549551
$userFolder->expects($this->once())
550552
->method('get')
551553
->willReturn($folder);
@@ -837,7 +839,7 @@ public function testUpdateShare(array $share, string $formOwner, array $keyValue
837839
->with('otherUser')
838840
->willReturn($this->createMock(IUser::class));
839841

840-
$userFolder = $this->createMock(Folder::class);
842+
$userFolder = $this->createUserFolderMock();
841843

842844
$file = $this->createMock(File::class);
843845
$file->expects($this->any())

‎tests/Unit/Helper/FilePathHelperTest.php‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use OCA\Forms\Constants;
1111
use OCA\Forms\Db\Form;
1212
use OCA\Forms\Helper\FilePathHelper;
13+
use OCA\Forms\Tests\Unit\UserFolderMockTrait;
1314
use OCP\Files\Folder;
1415
use OCP\Files\IFilenameValidator;
1516
use OCP\Files\IRootFolder;
@@ -18,6 +19,8 @@
1819
use Test\TestCase;
1920

2021
class FilePathHelperTest extends TestCase {
22+
use UserFolderMockTrait;
23+
2124
private FilePathHelper $filePathHelper;
2225
private IFilenameValidator|MockObject $filenameValidator;
2326
private IRootFolder|MockObject $rootFolder;
@@ -86,7 +89,7 @@ public function testGetUploadedFilePathWithoutQuestionName() {
8689
}
8790

8891
public function testGetFormsFolderReturnsFolder() {
89-
$userFolder = $this->createMock(Folder::class);
92+
$userFolder = $this->createUserFolderMock();
9093
$formsFolder = $this->createMock(Folder::class);
9194

9295
$this->rootFolder->expects($this->once())
@@ -104,7 +107,7 @@ public function testGetFormsFolderReturnsFolder() {
104107
}
105108

106109
public function testGetFormsFolderReturnsNullWhenNotFound() {
107-
$userFolder = $this->createMock(Folder::class);
110+
$userFolder = $this->createUserFolderMock();
108111

109112
$this->rootFolder->expects($this->once())
110113
->method('getUserFolder')
@@ -121,7 +124,7 @@ public function testGetFormsFolderReturnsNullWhenNotFound() {
121124
}
122125

123126
public function testGetFormsFolderReturnsNullWhenNotFolder() {
124-
$userFolder = $this->createMock(Folder::class);
127+
$userFolder = $this->createUserFolderMock();
125128
$notAFolder = $this->createMock(\OCP\Files\File::class);
126129

127130
$this->rootFolder->expects($this->once())
@@ -155,7 +158,7 @@ public function testGetAllFormFoldersById() {
155158
]);
156159

157160
// Mock getFormsFolder to return our formsFolder
158-
$userFolder = $this->createMock(Folder::class);
161+
$userFolder = $this->createUserFolderMock();
159162
$this->rootFolder->expects($this->once())
160163
->method('getUserFolder')
161164
->with('user1')
@@ -174,7 +177,7 @@ public function testGetAllFormFoldersById() {
174177
}
175178

176179
public function testGetAllFormFoldersByIdReturnsEmptyWhenFormsFolderNull() {
177-
$userFolder = $this->createMock(Folder::class);
180+
$userFolder = $this->createUserFolderMock();
178181
$this->rootFolder->expects($this->once())
179182
->method('getUserFolder')
180183
->with('user1')
@@ -201,7 +204,7 @@ public function testGetSubmissionFolder() {
201204
$formFolder->method('getName')->willReturn('42 - Form Title');
202205
$formsFolder->method('getDirectoryListing')->willReturn([$formFolder]);
203206

204-
$userFolder = $this->createMock(Folder::class);
207+
$userFolder = $this->createUserFolderMock();
205208
$this->rootFolder->expects($this->once())
206209
->method('getUserFolder')
207210
->with('user1')
@@ -232,7 +235,7 @@ public function testGetSubmissionFolderReturnsNullWhenNotFound() {
232235
$formFolder->method('getName')->willReturn('42 - Form Title');
233236
$formsFolder->method('getDirectoryListing')->willReturn([$formFolder]);
234237

235-
$userFolder = $this->createMock(Folder::class);
238+
$userFolder = $this->createUserFolderMock();
236239
$this->rootFolder->expects($this->once())
237240
->method('getUserFolder')
238241
->with('user1')
@@ -264,7 +267,7 @@ public function testGetSubmissionFolderReturnsNullWhenNotFolder() {
264267
$formFolder->method('getName')->willReturn('42 - Form Title');
265268
$formsFolder->method('getDirectoryListing')->willReturn([$formFolder]);
266269

267-
$userFolder = $this->createMock(Folder::class);
270+
$userFolder = $this->createUserFolderMock();
268271
$this->rootFolder->expects($this->once())
269272
->method('getUserFolder')
270273
->with('user1')

‎tests/Unit/Service/FormsServiceTest.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ function microtime(bool|float $asFloat = false) {
4646
use OCA\Forms\Service\ConfigService;
4747
use OCA\Forms\Service\ConfirmationEmailService;
4848
use OCA\Forms\Service\FormsService;
49+
use OCA\Forms\Tests\Unit\UserFolderMockTrait;
4950
use OCP\AppFramework\Db\DoesNotExistException;
5051
use OCP\EventDispatcher\IEventDispatcher;
51-
use OCP\Files\Folder;
5252
use OCP\Files\IFilenameValidator;
5353
use OCP\Files\IRootFolder;
5454
use OCP\Files\NotFoundException;
@@ -66,6 +66,7 @@ function microtime(bool|float $asFloat = false) {
6666
use Test\TestCase;
6767

6868
class FormsServiceTest extends TestCase {
69+
use UserFolderMockTrait;
6970

7071
private FormsService $formsService;
7172
private ActivityManager|MockObject $activityManager;
@@ -1652,7 +1653,7 @@ public function testGetFilePathThrowsAnException() {
16521653
$form->setFileId(100);
16531654
$form->setOwnerId('user1');
16541655

1655-
$folder = $this->createMock(Folder::class);
1656+
$folder = $this->createUserFolderMock();
16561657
$folder->expects($this->once())
16571658
->method('getById')
16581659
->with(100)

‎tests/Unit/Service/SubmissionServiceTest.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use OCA\Forms\Db\UploadedFileMapper;
2323
use OCA\Forms\Service\FormsService;
2424
use OCA\Forms\Service\SubmissionService;
25+
use OCA\Forms\Tests\Unit\UserFolderMockTrait;
2526
use OCP\AppFramework\Db\DoesNotExistException;
2627
use OCP\Config\IUserConfig;
2728
use OCP\Files\File;
@@ -43,6 +44,7 @@
4344
use Test\TestCase;
4445

4546
class SubmissionServiceTest extends TestCase {
47+
use UserFolderMockTrait;
4648

4749
private SubmissionService $submissionService;
4850
private FormMapper|MockObject $formMapper;
@@ -325,7 +327,7 @@ public function testWriteFileToCloud(string $formTitle, string $path, string $pa
325327
$pathNode = $folderNode;
326328
}
327329

328-
$userFolder = $this->createMock(Folder::class);
330+
$userFolder = $this->createUserFolderMock();
329331
$userFolder->expects($this->once())
330332
->method('get')
331333
->with($path)

‎tests/Unit/Service/UploadedFilesShareServiceTest.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use OCA\Forms\Db\ShareMapper;
1616
use OCA\Forms\Helper\FilePathHelper;
1717
use OCA\Forms\Service\UploadedFilesShareService;
18+
use OCA\Forms\Tests\Unit\UserFolderMockTrait;
1819
use OCP\Files\Folder;
1920
use OCP\Files\IFilenameValidator;
2021
use OCP\Files\IRootFolder;
@@ -26,6 +27,8 @@
2627
use Test\TestCase;
2728

2829
class UploadedFilesShareServiceTest extends TestCase {
30+
use UserFolderMockTrait;
31+
2932
private UploadedFilesShareService $service;
3033
private ShareMapper|MockObject $shareMapper;
3134
private IRootFolder|MockObject $rootFolder;
@@ -75,7 +78,7 @@ public function testRemoveAllForFormCleansUpResultsShares(): void {
7578
->willReturn([$resultsShare, $submitOnlyShare]);
7679

7780
$folder = $this->createMock(Folder::class);
78-
$userFolder = $this->createMock(Folder::class);
81+
$userFolder = $this->createUserFolderMock();
7982
$userFolder->expects($this->once())
8083
->method('get')
8184
->with('Forms/2 - test')
@@ -125,7 +128,7 @@ public function testRemoveForCollaboratorIgnoresMissingUploadedFilesFolder(): vo
125128
$share->setShareWith('bob');
126129
$share->setPermissions([Constants::PERMISSION_RESULTS]);
127130

128-
$userFolder = $this->createMock(Folder::class);
131+
$userFolder = $this->createUserFolderMock();
129132
$userFolder->method('get')->willThrowException(new NotFoundException());
130133
$this->rootFolder->method('getUserFolder')->with('alice')->willReturn($userFolder);
131134
$this->shareManager->expects($this->never())->method('getSharesBy');

‎tests/Unit/UserFolderMockTrait.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\Forms\Tests\Unit;
10+
11+
use OCP\Files\Folder;
12+
use OCP\Files\IUserFolder;
13+
use PHPUnit\Framework\MockObject\MockObject;
14+
15+
/**
16+
* IRootFolder::getUserFolder() is typed as IUserFolder since Nextcloud 36,
17+
* older versions expect a Folder. Mock whichever interface exists.
18+
*/
19+
trait UserFolderMockTrait {
20+
private function createUserFolderMock(): Folder&MockObject {
21+
$className = interface_exists(IUserFolder::class) ? IUserFolder::class : Folder::class;
22+
return $this->createMock($className);
23+
}
24+
}

0 commit comments

Comments
 (0)