Skip to content

Commit 9522dde

Browse files
CarlSchwanCarlSchwan
committed
feat: Add Folder::getOrCreateFolder api
Allow to remove some boilerplate and also this new function is type safe. Signed-off-by: Carl Schwan <[email protected]>
1 parent 38fd84a commit 9522dde

File tree

8 files changed

+153
-42
lines changed

8 files changed

+153
-42
lines changed

‎lib/private/Files/Node/Folder.php‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use OC\User\LazyUser;
1717
use OCP\Files\Cache\ICacheEntry;
1818
use OCP\Files\FileInfo;
19+
use OCP\Files\Folder as IFolder;
1920
use OCP\Files\Mount\IMountPoint;
2021
use OCP\Files\Node as INode;
2122
use OCP\Files\NotFoundException;
@@ -26,8 +27,9 @@
2627
use OCP\Files\Search\ISearchOrder;
2728
use OCP\Files\Search\ISearchQuery;
2829
use OCP\IUserManager;
30+
use Override;
2931

30-
class Folder extends Node implements \OCP\Files\Folder {
32+
class Folder extends Node implements IFolder {
3133

3234
private ?IUserManager $userManager = null;
3335

@@ -480,4 +482,28 @@ private function recreateIfNeeded(): void {
480482
$this->wasDeleted = false;
481483
}
482484
}
485+
486+
#[Override]
487+
public function getOrCreateFolder(string $path, int $maxRetries = 5): IFolder {
488+
$i = 0;
489+
while (true) {
490+
$path = $i === 0 ? $path : $path . ' (' . $i . ')';
491+
try {
492+
$folder = $this->get($path);
493+
if ($folder instanceof IFolder) {
494+
return $folder;
495+
}
496+
} catch (NotFoundException) {
497+
$folder = dirname($path) === '.' ? $this : $this->get(dirname($path));
498+
if (!($folder instanceof Folder)) {
499+
throw new NotPermittedException("Unable to create folder $path. Parent is not a directory.");
500+
}
501+
return $folder->newFolder(basename($path));
502+
}
503+
$i++;
504+
if ($i === $maxRetries) {
505+
throw new NotPermittedException('Unable to load or create folder.');
506+
}
507+
}
508+
}
483509
}

‎lib/private/Files/Node/LazyFolder.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use OCP\Files\IRootFolder;
1515
use OCP\Files\Mount\IMountPoint;
1616
use OCP\Files\NotPermittedException;
17+
use Override;
1718

1819
/**
1920
* Class LazyFolder
@@ -138,6 +139,11 @@ public function get($path) {
138139
return $this->getRootFolder()->get($this->getFullPath($path));
139140
}
140141

142+
#[Override]
143+
public function getOrCreateFolder(string $path, int $maxRetries = 5): Folder {
144+
return $this->getRootFolder()->getOrCreateFolder($this->getFullPath($path), $maxRetries);
145+
}
146+
141147
/**
142148
* @inheritDoc
143149
*/

‎lib/private/Files/Template/TemplateManager.php‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -355,14 +355,7 @@ public function initializeTemplateDirectory(?string $path = null, ?string $userI
355355
}
356356
}
357357

358-
try {
359-
/** @var Folder $folder */
360-
$folder = $userFolder->get($userTemplatePath);
361-
} catch (NotFoundException $e) {
362-
/** @var Folder $folder */
363-
$folder = $userFolder->get(dirname($userTemplatePath));
364-
$folder = $folder->newFolder(basename($userTemplatePath));
365-
}
358+
$folder = $userFolder->getOrCreateFolder($userTemplatePath);
366359

367360
$folderIsEmpty = count($folder->getDirectoryListing()) === 0;
368361

‎lib/public/Files/Folder.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,15 @@ public function getDirectoryListing();
6565
*/
6666
public function get($path);
6767

68+
/**
69+
* Get or create new folder if the folder does not already exist.
70+
*
71+
* @param string $path relative path of the file or folder
72+
* @throw \OCP\Files\NotPermittedException
73+
* @since 33.0.0
74+
*/
75+
public function getOrCreateFolder(string $path, int $maxRetries = 5): Folder;
76+
6877
/**
6978
* Check if a file or folder exists in the folder
7079
*

‎tests/lib/Files/Node/FileTest.php‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,14 @@
99
namespace Test\Files\Node;
1010

1111
use OC\Files\Node\File;
12+
use OC\Files\Node\NonExistingFile;
1213
use OC\Files\Node\Root;
14+
use OC\Files\View;
1315
use OCP\Constants;
16+
use OCP\Files\IRootFolder;
1417
use OCP\Files\NotPermittedException;
18+
use OCP\Files\Storage\IStorage;
19+
use PHPUnit\Framework\MockObject\MockObject;
1520

1621
/**
1722
* Class FileTest
@@ -21,23 +26,23 @@
2126
*/
2227
#[\PHPUnit\Framework\Attributes\Group('DB')]
2328
class FileTest extends NodeTestCase {
24-
protected function createTestNode($root, $view, $path, array $data = [], $internalPath = '', $storage = null) {
29+
protected function createTestNode(IRootFolder $root, View&MockObject $view, string $path, array $data = [], string $internalPath = '', ?IStorage $storage = null): File {
2530
if ($data || $internalPath || $storage) {
2631
return new File($root, $view, $path, $this->getFileInfo($data, $internalPath, $storage));
2732
} else {
2833
return new File($root, $view, $path);
2934
}
3035
}
3136

32-
protected function getNodeClass() {
33-
return '\OC\Files\Node\File';
37+
protected function getNodeClass(): string {
38+
return File::class;
3439
}
3540

36-
protected function getNonExistingNodeClass() {
37-
return '\OC\Files\Node\NonExistingFile';
41+
protected function getNonExistingNodeClass(): string {
42+
return NonExistingFile::class;
3843
}
3944

40-
protected function getViewDeleteMethod() {
45+
protected function getViewDeleteMethod(): string {
4146
return 'unlink';
4247
}
4348

‎tests/lib/Files/Node/FolderTest.php‎

Lines changed: 91 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use OC\Files\Node\File;
1818
use OC\Files\Node\Folder;
1919
use OC\Files\Node\Node;
20+
use OC\Files\Node\NonExistingFolder;
2021
use OC\Files\Node\Root;
2122
use OC\Files\Search\SearchBinaryOperator;
2223
use OC\Files\Search\SearchComparison;
@@ -37,6 +38,7 @@
3738
use OCP\Files\Search\ISearchComparison;
3839
use OCP\Files\Search\ISearchOrder;
3940
use OCP\Files\Storage\IStorage;
41+
use PHPUnit\Framework\Attributes\DataProvider;
4042
use PHPUnit\Framework\MockObject\MockObject;
4143

4244
/**
@@ -47,7 +49,7 @@
4749
*/
4850
#[\PHPUnit\Framework\Attributes\Group('DB')]
4951
class FolderTest extends NodeTestCase {
50-
protected function createTestNode($root, $view, $path, array $data = [], $internalPath = '', $storage = null) {
52+
protected function createTestNode(IRootFolder $root, View&MockObject $view, string $path, array $data = [], string $internalPath = '', ?IStorage $storage = null): Folder {
5153
$view->expects($this->any())
5254
->method('getRoot')
5355
->willReturn('');
@@ -58,23 +60,20 @@ protected function createTestNode($root, $view, $path, array $data = [], $intern
5860
}
5961
}
6062

61-
protected function getNodeClass() {
62-
return '\OC\Files\Node\Folder';
63+
protected function getNodeClass(): string {
64+
return Folder::class;
6365
}
6466

65-
protected function getNonExistingNodeClass() {
66-
return '\OC\Files\Node\NonExistingFolder';
67+
protected function getNonExistingNodeClass(): string {
68+
return NonExistingFolder::class;
6769
}
6870

69-
protected function getViewDeleteMethod() {
71+
protected function getViewDeleteMethod(): string {
7072
return 'rmdir';
7173
}
7274

7375
public function testGetDirectoryContent(): void {
7476
$manager = $this->createMock(Manager::class);
75-
/**
76-
* @var View|\PHPUnit\Framework\MockObject\MockObject $view
77-
*/
7877
$root = $this->getMockBuilder(Root::class)
7978
->setConstructorArgs([$manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig])
8079
->getMock();
@@ -299,7 +298,6 @@ public function testSearch(): void {
299298
->getMock();
300299
$root->method('getUser')
301300
->willReturn($this->user);
302-
/** @var Storage\IStorage&MockObject $storage */
303301
$storage = $this->createMock(IStorage::class);
304302
$storage->method('getId')->willReturn('test::1');
305303
$cache = new Cache($storage);
@@ -349,7 +347,6 @@ public function testSearchInRoot(): void {
349347
$root->expects($this->any())
350348
->method('getUser')
351349
->willReturn($this->user);
352-
/** @var \PHPUnit\Framework\MockObject\MockObject|Storage $storage */
353350
$storage = $this->createMock(IStorage::class);
354351
$storage->method('getId')->willReturn('test::2');
355352
$cache = new Cache($storage);
@@ -1041,4 +1038,87 @@ public function testSearchSubStoragesLimitOffset(int $offset, int $limit, array
10411038
}, $result);
10421039
$this->assertEquals($expectedPaths, $ids);
10431040
}
1041+
1042+
public static function dataGetOrCreateFolder(): \Generator {
1043+
yield 'Create new folder' => [0];
1044+
yield 'Get existing folder' => [1];
1045+
yield 'Create new folder while a file with the same name already exists' => [2];
1046+
}
1047+
1048+
#[DataProvider('dataGetOrCreateFolder')]
1049+
public function testGetOrCreateFolder(int $case): void {
1050+
$folderName = 'asd';
1051+
1052+
$view = $this->getRootViewMock();
1053+
$manager = $this->createMock(Manager::class);
1054+
$root = $this->getMockBuilder(Root::class)
1055+
->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig])
1056+
->getMock();
1057+
$root->expects($this->any())
1058+
->method('getUser')
1059+
->willReturn($this->user);
1060+
1061+
$view->method('getFileInfo')
1062+
->willReturnCallback(function (string $path) use ($folderName) {
1063+
if ($path === '/bar/foo' || $path === '/bar/foo/' . $folderName) {
1064+
return $this->getFileInfo(['permissions' => Constants::PERMISSION_ALL]);
1065+
}
1066+
$this->fail('Trying to get ' . $path);
1067+
});
1068+
1069+
$view->method('mkdir')
1070+
->willReturn(true);
1071+
1072+
$view->method('touch')
1073+
->with('/bar/foo/asd')
1074+
->willReturn(true);
1075+
1076+
$node = new Folder($root, $view, '/bar/foo');
1077+
1078+
switch ($case) {
1079+
case 0:
1080+
$child = new Folder($root, $view, '/bar/foo/' . $folderName, null, $node);
1081+
1082+
$root->expects($this->any())
1083+
->method('get')
1084+
->willReturnCallback(function (string $path) use ($root, $view, $folderName) {
1085+
if ($path === '/bar/foo/') {
1086+
return new Folder($root, $view, '/bar/foo/');
1087+
} elseif ($path === '/bar/foo/' . $folderName) {
1088+
throw new NotFoundException();
1089+
}
1090+
$this->fail('Trying to get ' . $path);
1091+
});
1092+
1093+
break; // do nothing
1094+
case 1:
1095+
$child = new Folder($root, $view, '/bar/foo/' . $folderName, null, $node);
1096+
1097+
$root->expects($this->any())
1098+
->method('get')
1099+
->with('/bar/foo/' . $folderName)
1100+
->willReturn($child);
1101+
$node->newFolder($folderName);
1102+
break;
1103+
case 2:
1104+
$child = new Folder($root, $view, '/bar/foo/' . $folderName . ' (1)', null, $node);
1105+
$root->expects($this->any())
1106+
->method('get')
1107+
->willReturnCallback(function (string $path) use ($root, $view, $folderName) {
1108+
if ($path === '/bar/foo/') {
1109+
return new Folder($root, $view, '/bar/foo/');
1110+
} elseif ($path === '/bar/foo/' . $folderName) {
1111+
return new File($root, $view, '/bar/foo/asd');
1112+
} elseif ($path === '/bar/foo/' . $folderName . ' (1)') {
1113+
throw new NotFoundException();
1114+
}
1115+
$this->fail('Trying to get ' . $path);
1116+
});
1117+
$node->newFile($folderName);
1118+
break;
1119+
}
1120+
1121+
$result = $node->getOrCreateFolder($folderName);
1122+
$this->assertEquals($child, $result);
1123+
}
10441124
}

‎tests/lib/Files/Node/NodeTestCase.php‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -92,30 +92,24 @@ protected function getRootViewMock() {
9292
return $view;
9393
}
9494

95-
/**
96-
* @param IRootFolder $root
97-
* @param View $view
98-
* @param string $path
99-
* @return Node
100-
*/
101-
abstract protected function createTestNode($root, $view, $path, array $data = [], $internalPath = '', $storage = null);
95+
abstract protected function createTestNode(IRootFolder $root, View&MockObject $view, string $path, array $data = [], string $internalPath = '', ?IStorage $storage = null): Node;
10296

10397
/**
104-
* @return string
98+
* @return class-string<Node>
10599
*/
106-
abstract protected function getNodeClass();
100+
abstract protected function getNodeClass(): string;
107101

108102
/**
109-
* @return string
103+
* @return class-string<Node>
110104
*/
111-
abstract protected function getNonExistingNodeClass();
105+
abstract protected function getNonExistingNodeClass(): string;
112106

113107
/**
114108
* @return string
115109
*/
116-
abstract protected function getViewDeleteMethod();
110+
abstract protected function getViewDeleteMethod(): string;
117111

118-
protected function getMockStorage() {
112+
protected function getMockStorage(): IStorage&MockObject {
119113
$storage = $this->getMockBuilder(IStorage::class)
120114
->disableOriginalConstructor()
121115
->getMock();
@@ -125,7 +119,7 @@ protected function getMockStorage() {
125119
return $storage;
126120
}
127121

128-
protected function getFileInfo($data, $internalPath = '', $storage = null) {
122+
protected function getFileInfo($data, $internalPath = '', ?IStorage $storage = null) {
129123
$mount = $this->createMock(IMountPoint::class);
130124
$mount->method('getStorage')
131125
->willReturn($storage);

‎tests/lib/Files/Template/TemplateManagerTest.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323
use OCP\IL10N;
2424
use OCP\IPreview;
2525
use OCP\IServerContainer;
26-
use OCP\IUserManager;
27-
use OCP\IUserSession;
2826
use OCP\IUser;
2927
use OCP\L10N\IFactory;
3028
use Psr\Log\NullLogger;

0 commit comments

Comments
 (0)