Skip to content

Commit e7a7b4a

Browse files
icewind1991icewind1991
committed
perf: switch places that always use the first getById result to getFirstNodeById
Signed-off-by: Robin Appelman <robin@icewind.nl>
1 parent 4d110c1 commit e7a7b4a

File tree

50 files changed

+316
-313
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+316
-313
lines changed

‎apps/dav/lib/BulkUpload/BulkUploadPlugin.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
9191

9292
$node = $this->userFolder->newFile($headers['x-file-path'], $content);
9393
$node->touch($mtime);
94-
$node = $this->userFolder->getById($node->getId())[0];
94+
$node = $this->userFolder->getFirstNodeById($node->getId());
9595

9696
$writtenFiles[$headers['x-file-path']] = [
9797
"error" => false,

‎apps/dav/lib/Connector/Sabre/FilesReportPlugin.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -424,22 +424,22 @@ public function findNodesByFileIds(Node $rootNode, array $fileIds): array {
424424
}
425425
$folder = $this->userFolder;
426426
if (trim($rootNode->getPath(), '/') !== '') {
427+
/** @var Folder $folder */
427428
$folder = $folder->get($rootNode->getPath());
428429
}
429430

430431
$results = [];
431432
foreach ($fileIds as $fileId) {
432-
$entry = $folder->getById($fileId);
433+
$entry = $folder->getFirstNodeById($fileId);
433434
if ($entry) {
434-
$entry = current($entry);
435435
$results[] = $this->wrapNode($entry);
436436
}
437437
}
438438

439439
return $results;
440440
}
441441

442-
protected function wrapNode(\OCP\Files\File|\OCP\Files\Folder $node): File|Directory {
442+
protected function wrapNode(\OCP\Files\Node $node): File|Directory {
443443
if ($node instanceof \OCP\Files\File) {
444444
return new File($this->fileView, $node);
445445
} else {

‎apps/dav/lib/Controller/DirectController.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,17 +104,16 @@ public function __construct(string $appName,
104104
public function getUrl(int $fileId, int $expirationTime = 60 * 60 * 8): DataResponse {
105105
$userFolder = $this->rootFolder->getUserFolder($this->userId);
106106

107-
$files = $userFolder->getById($fileId);
107+
$file = $userFolder->getFirstNodeById($fileId);
108108

109-
if ($files === []) {
109+
if (!$file) {
110110
throw new OCSNotFoundException();
111111
}
112112

113113
if ($expirationTime <= 0 || $expirationTime > (60 * 60 * 24)) {
114114
throw new OCSBadRequestException('Expiration time should be greater than 0 and less than or equal to ' . (60 * 60 * 24));
115115
}
116116

117-
$file = array_shift($files);
118117
if (!($file instanceof File)) {
119118
throw new OCSBadRequestException('Direct download only works for files');
120119
}

‎apps/dav/lib/Direct/DirectFile.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,16 @@ public function getLastModified() {
108108
private function getFile() {
109109
if ($this->file === null) {
110110
$userFolder = $this->rootFolder->getUserFolder($this->direct->getUserId());
111-
$files = $userFolder->getById($this->direct->getFileId());
111+
$file = $userFolder->getFirstNodeById($this->direct->getFileId());
112112

113-
if ($files === []) {
113+
if (!$file) {
114114
throw new NotFound();
115115
}
116+
if (!$file instanceof File) {
117+
throw new Forbidden("direct download not allowed on directories");
118+
}
116119

117-
$this->file = array_shift($files);
120+
$this->file = $file;
118121
}
119122

120123
return $this->file;

‎apps/dav/lib/RootCollection.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,8 @@ public function __construct() {
134134
\OC::$server->getSystemTagObjectMapper(),
135135
\OC::$server->getUserSession(),
136136
$groupManager,
137-
$dispatcher
137+
$dispatcher,
138+
$rootFolder,
138139
);
139140
$systemTagInUseCollection = \OCP\Server::get(SystemTag\SystemTagsInUseCollection::class);
140141
$commentsCollection = new Comments\RootCollection(

‎apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
namespace OCA\DAV\SystemTag;
2828

2929
use OCP\EventDispatcher\IEventDispatcher;
30+
use OCP\Files\IRootFolder;
3031
use OCP\IGroupManager;
3132
use OCP\IUserSession;
3233
use OCP\SystemTag\ISystemTagManager;
@@ -42,6 +43,7 @@ public function __construct(
4243
IUserSession $userSession,
4344
IGroupManager $groupManager,
4445
IEventDispatcher $dispatcher,
46+
IRootFolder $rootFolder,
4547
) {
4648
$children = [
4749
new SystemTagsObjectTypeCollection(
@@ -50,9 +52,14 @@ public function __construct(
5052
$tagMapper,
5153
$userSession,
5254
$groupManager,
53-
function ($name) {
54-
$nodes = \OC::$server->getUserFolder()->getById((int)$name);
55-
return !empty($nodes);
55+
function (string $name) use ($rootFolder, $userSession): bool {
56+
$user = $userSession->getUser();
57+
if ($user) {
58+
$node = $rootFolder->getUserFolder($user->getUID())->getFirstNodeById((int)$name);
59+
return $node !== null;
60+
} else {
61+
return false;
62+
}
5663
}
5764
),
5865
];

‎apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -320,14 +320,14 @@ public function testFindNodesByFileIdsRoot(): void {
320320
->willReturn('/');
321321

322322
$this->userFolder->expects($this->exactly(2))
323-
->method('getById')
323+
->method('getFirstNodeById')
324324
->withConsecutive(
325325
['111'],
326326
['222'],
327327
)
328328
->willReturnOnConsecutiveCalls(
329-
[$filesNode1],
330-
[$filesNode2],
329+
$filesNode1,
330+
$filesNode2,
331331
);
332332

333333
/** @var \OCA\DAV\Connector\Sabre\Directory|MockObject $reportTargetNode */
@@ -373,14 +373,14 @@ public function testFindNodesByFileIdsSubDir(): void {
373373
->willReturn($subNode);
374374

375375
$subNode->expects($this->exactly(2))
376-
->method('getById')
376+
->method('getFirstNodeById')
377377
->withConsecutive(
378378
['111'],
379379
['222'],
380380
)
381381
->willReturnOnConsecutiveCalls(
382-
[$filesNode1],
383-
[$filesNode2],
382+
$filesNode1,
383+
$filesNode2,
384384
);
385385

386386
/** @var \OCA\DAV\Connector\Sabre\Directory|MockObject $reportTargetNode */

‎apps/dav/tests/unit/Controller/DirectControllerTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,9 @@ public function testGetUrlForFolder(): void {
110110

111111
$folder = $this->createMock(Folder::class);
112112

113-
$userFolder->method('getById')
113+
$userFolder->method('getFirstNodeById')
114114
->with(101)
115-
->willReturn([$folder]);
115+
->willReturn($folder);
116116

117117
$this->expectException(OCSBadRequestException::class);
118118
$this->controller->getUrl(101);
@@ -129,9 +129,9 @@ public function testGetUrlValid(): void {
129129
$this->timeFactory->method('getTime')
130130
->willReturn(42);
131131

132-
$userFolder->method('getById')
132+
$userFolder->method('getFirstNodeById')
133133
->with(101)
134-
->willReturn([$file]);
134+
->willReturn($file);
135135

136136
$userFolder->method('getRelativePath')
137137
->willReturn('/path');

‎apps/dav/tests/unit/Direct/DirectFileTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,9 @@ protected function setUp(): void {
7373
->willReturn($this->userFolder);
7474

7575
$this->file = $this->createMock(File::class);
76-
$this->userFolder->method('getById')
76+
$this->userFolder->method('getFirstNodeById')
7777
->with(42)
78-
->willReturn([$this->file]);
78+
->willReturn($this->file);
7979

8080
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
8181

‎apps/dav/tests/unit/SystemTag/SystemTagsObjectTypeCollectionTest.php‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,8 @@ protected function setUp(): void {
8484
$userFolder = $this->userFolder;
8585

8686
$closure = function ($name) use ($userFolder) {
87-
$nodes = $userFolder->getById(intval($name));
88-
return !empty($nodes);
87+
$node = $userFolder->getFirstNodeById(intval($name));
88+
return $node !== null;
8989
};
9090

9191
$this->node = new \OCA\DAV\SystemTag\SystemTagsObjectTypeCollection(
@@ -98,14 +98,14 @@ protected function setUp(): void {
9898
);
9999
}
100100

101-
101+
102102
public function testForbiddenCreateFile(): void {
103103
$this->expectException(\Sabre\DAV\Exception\Forbidden::class);
104104

105105
$this->node->createFile('555');
106106
}
107107

108-
108+
109109
public function testForbiddenCreateDirectory(): void {
110110
$this->expectException(\Sabre\DAV\Exception\Forbidden::class);
111111

@@ -114,27 +114,27 @@ public function testForbiddenCreateDirectory(): void {
114114

115115
public function testGetChild(): void {
116116
$this->userFolder->expects($this->once())
117-
->method('getById')
117+
->method('getFirstNodeById')
118118
->with('555')
119-
->willReturn([true]);
119+
->willReturn($this->createMock(\OCP\Files\Node::class));
120120
$childNode = $this->node->getChild('555');
121121

122122
$this->assertInstanceOf('\OCA\DAV\SystemTag\SystemTagsObjectMappingCollection', $childNode);
123123
$this->assertEquals('555', $childNode->getName());
124124
}
125125

126-
126+
127127
public function testGetChildWithoutAccess(): void {
128128
$this->expectException(\Sabre\DAV\Exception\NotFound::class);
129129

130130
$this->userFolder->expects($this->once())
131-
->method('getById')
131+
->method('getFirstNodeById')
132132
->with('555')
133-
->willReturn([]);
133+
->willReturn(null);
134134
$this->node->getChild('555');
135135
}
136136

137-
137+
138138
public function testGetChildren(): void {
139139
$this->expectException(\Sabre\DAV\Exception\MethodNotAllowed::class);
140140

@@ -143,28 +143,28 @@ public function testGetChildren(): void {
143143

144144
public function testChildExists(): void {
145145
$this->userFolder->expects($this->once())
146-
->method('getById')
146+
->method('getFirstNodeById')
147147
->with('123')
148-
->willReturn([true]);
148+
->willReturn($this->createMock(\OCP\Files\Node::class));
149149
$this->assertTrue($this->node->childExists('123'));
150150
}
151151

152152
public function testChildExistsWithoutAccess(): void {
153153
$this->userFolder->expects($this->once())
154-
->method('getById')
154+
->method('getFirstNodeById')
155155
->with('555')
156-
->willReturn([]);
156+
->willReturn(null);
157157
$this->assertFalse($this->node->childExists('555'));
158158
}
159159

160-
160+
161161
public function testDelete(): void {
162162
$this->expectException(\Sabre\DAV\Exception\Forbidden::class);
163163

164164
$this->node->delete();
165165
}
166166

167-
167+
168168
public function testSetName(): void {
169169
$this->expectException(\Sabre\DAV\Exception\Forbidden::class);
170170

0 commit comments

Comments
 (0)