diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index b747406e4df..35586647588 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -16,6 +16,7 @@ use OC\Files\Utils\PathHelper; use OC\User\LazyUser; use OCP\Files\Cache\ICacheEntry; use OCP\Files\FileInfo; +use OCP\Files\Folder as IFolder; use OCP\Files\Mount\IMountPoint; use OCP\Files\Node as INode; use OCP\Files\NotFoundException; @@ -26,8 +27,9 @@ use OCP\Files\Search\ISearchOperator; use OCP\Files\Search\ISearchOrder; use OCP\Files\Search\ISearchQuery; use OCP\IUserManager; +use Override; -class Folder extends Node implements \OCP\Files\Folder { +class Folder extends Node implements IFolder { private ?IUserManager $userManager = null; @@ -480,4 +482,28 @@ class Folder extends Node implements \OCP\Files\Folder { $this->wasDeleted = false; } } + + #[Override] + public function getOrCreateFolder(string $path, int $maxRetries = 5): IFolder { + $i = 0; + while (true) { + $path = $i === 0 ? $path : $path . ' (' . $i . ')'; + try { + $folder = $this->get($path); + if ($folder instanceof IFolder) { + return $folder; + } + } catch (NotFoundException) { + $folder = dirname($path) === '.' ? $this : $this->get(dirname($path)); + if (!($folder instanceof Folder)) { + throw new NotPermittedException("Unable to create folder $path. Parent is not a directory."); + } + return $folder->newFolder(basename($path)); + } + $i++; + if ($i === $maxRetries) { + throw new NotPermittedException('Unable to load or create folder.'); + } + } + } } diff --git a/lib/private/Files/Node/LazyFolder.php b/lib/private/Files/Node/LazyFolder.php index ceadc4cad1e..c4188b80ee5 100644 --- a/lib/private/Files/Node/LazyFolder.php +++ b/lib/private/Files/Node/LazyFolder.php @@ -14,6 +14,7 @@ use OCP\Files\Folder; use OCP\Files\IRootFolder; use OCP\Files\Mount\IMountPoint; use OCP\Files\NotPermittedException; +use Override; /** * Class LazyFolder @@ -138,6 +139,11 @@ class LazyFolder implements Folder { return $this->getRootFolder()->get($this->getFullPath($path)); } + #[Override] + public function getOrCreateFolder(string $path, int $maxRetries = 5): Folder { + return $this->getRootFolder()->getOrCreateFolder($this->getFullPath($path), $maxRetries); + } + /** * @inheritDoc */ diff --git a/lib/private/Files/Template/TemplateManager.php b/lib/private/Files/Template/TemplateManager.php index 4d66ed8f9c8..155be9303b1 100644 --- a/lib/private/Files/Template/TemplateManager.php +++ b/lib/private/Files/Template/TemplateManager.php @@ -355,14 +355,7 @@ class TemplateManager implements ITemplateManager { } } - try { - /** @var Folder $folder */ - $folder = $userFolder->get($userTemplatePath); - } catch (NotFoundException $e) { - /** @var Folder $folder */ - $folder = $userFolder->get(dirname($userTemplatePath)); - $folder = $folder->newFolder(basename($userTemplatePath)); - } + $folder = $userFolder->getOrCreateFolder($userTemplatePath); $folderIsEmpty = count($folder->getDirectoryListing()) === 0; diff --git a/lib/public/Files/Folder.php b/lib/public/Files/Folder.php index a35d2d78bc9..163d362ecb4 100644 --- a/lib/public/Files/Folder.php +++ b/lib/public/Files/Folder.php @@ -65,6 +65,15 @@ interface Folder extends Node { */ public function get($path); + /** + * Get or create new folder if the folder does not already exist. + * + * @param string $path relative path of the file or folder + * @throw \OCP\Files\NotPermittedException + * @since 33.0.0 + */ + public function getOrCreateFolder(string $path, int $maxRetries = 5): Folder; + /** * Check if a file or folder exists in the folder * diff --git a/tests/lib/Files/Node/FileTest.php b/tests/lib/Files/Node/FileTest.php index 027f48498ea..36a410887cc 100644 --- a/tests/lib/Files/Node/FileTest.php +++ b/tests/lib/Files/Node/FileTest.php @@ -9,9 +9,14 @@ namespace Test\Files\Node; use OC\Files\Node\File; +use OC\Files\Node\NonExistingFile; use OC\Files\Node\Root; +use OC\Files\View; use OCP\Constants; +use OCP\Files\IRootFolder; use OCP\Files\NotPermittedException; +use OCP\Files\Storage\IStorage; +use PHPUnit\Framework\MockObject\MockObject; /** * Class FileTest @@ -21,7 +26,7 @@ use OCP\Files\NotPermittedException; */ #[\PHPUnit\Framework\Attributes\Group('DB')] class FileTest extends NodeTestCase { - protected function createTestNode($root, $view, $path, array $data = [], $internalPath = '', $storage = null) { + protected function createTestNode(IRootFolder $root, View&MockObject $view, string $path, array $data = [], string $internalPath = '', ?IStorage $storage = null): File { if ($data || $internalPath || $storage) { return new File($root, $view, $path, $this->getFileInfo($data, $internalPath, $storage)); } else { @@ -29,15 +34,15 @@ class FileTest extends NodeTestCase { } } - protected function getNodeClass() { - return '\OC\Files\Node\File'; + protected function getNodeClass(): string { + return File::class; } - protected function getNonExistingNodeClass() { - return '\OC\Files\Node\NonExistingFile'; + protected function getNonExistingNodeClass(): string { + return NonExistingFile::class; } - protected function getViewDeleteMethod() { + protected function getViewDeleteMethod(): string { return 'unlink'; } diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index 8ba0745d4e1..9801f05d610 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/tests/lib/Files/Node/FolderTest.php @@ -17,6 +17,7 @@ use OC\Files\Mount\MountPoint; use OC\Files\Node\File; use OC\Files\Node\Folder; use OC\Files\Node\Node; +use OC\Files\Node\NonExistingFolder; use OC\Files\Node\Root; use OC\Files\Search\SearchBinaryOperator; use OC\Files\Search\SearchComparison; @@ -37,6 +38,7 @@ use OCP\Files\Search\ISearchBinaryOperator; use OCP\Files\Search\ISearchComparison; use OCP\Files\Search\ISearchOrder; use OCP\Files\Storage\IStorage; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; /** @@ -47,7 +49,7 @@ use PHPUnit\Framework\MockObject\MockObject; */ #[\PHPUnit\Framework\Attributes\Group('DB')] class FolderTest extends NodeTestCase { - protected function createTestNode($root, $view, $path, array $data = [], $internalPath = '', $storage = null) { + protected function createTestNode(IRootFolder $root, View&MockObject $view, string $path, array $data = [], string $internalPath = '', ?IStorage $storage = null): Folder { $view->expects($this->any()) ->method('getRoot') ->willReturn(''); @@ -58,23 +60,20 @@ class FolderTest extends NodeTestCase { } } - protected function getNodeClass() { - return '\OC\Files\Node\Folder'; + protected function getNodeClass(): string { + return Folder::class; } - protected function getNonExistingNodeClass() { - return '\OC\Files\Node\NonExistingFolder'; + protected function getNonExistingNodeClass(): string { + return NonExistingFolder::class; } - protected function getViewDeleteMethod() { + protected function getViewDeleteMethod(): string { return 'rmdir'; } public function testGetDirectoryContent(): void { $manager = $this->createMock(Manager::class); - /** - * @var View|\PHPUnit\Framework\MockObject\MockObject $view - */ $root = $this->getMockBuilder(Root::class) ->setConstructorArgs([$manager, $this->view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) ->getMock(); @@ -299,7 +298,6 @@ class FolderTest extends NodeTestCase { ->getMock(); $root->method('getUser') ->willReturn($this->user); - /** @var Storage\IStorage&MockObject $storage */ $storage = $this->createMock(IStorage::class); $storage->method('getId')->willReturn('test::1'); $cache = new Cache($storage); @@ -349,7 +347,6 @@ class FolderTest extends NodeTestCase { $root->expects($this->any()) ->method('getUser') ->willReturn($this->user); - /** @var \PHPUnit\Framework\MockObject\MockObject|Storage $storage */ $storage = $this->createMock(IStorage::class); $storage->method('getId')->willReturn('test::2'); $cache = new Cache($storage); @@ -1041,4 +1038,87 @@ class FolderTest extends NodeTestCase { }, $result); $this->assertEquals($expectedPaths, $ids); } + + public static function dataGetOrCreateFolder(): \Generator { + yield 'Create new folder' => [0]; + yield 'Get existing folder' => [1]; + yield 'Create new folder while a file with the same name already exists' => [2]; + } + + #[DataProvider('dataGetOrCreateFolder')] + public function testGetOrCreateFolder(int $case): void { + $folderName = 'asd'; + + $view = $this->getRootViewMock(); + $manager = $this->createMock(Manager::class); + $root = $this->getMockBuilder(Root::class) + ->setConstructorArgs([$manager, $view, $this->user, $this->userMountCache, $this->logger, $this->userManager, $this->eventDispatcher, $this->cacheFactory, $this->appConfig]) + ->getMock(); + $root->expects($this->any()) + ->method('getUser') + ->willReturn($this->user); + + $view->method('getFileInfo') + ->willReturnCallback(function (string $path) use ($folderName) { + if ($path === '/bar/foo' || $path === '/bar/foo/' . $folderName) { + return $this->getFileInfo(['permissions' => Constants::PERMISSION_ALL]); + } + $this->fail('Trying to get ' . $path); + }); + + $view->method('mkdir') + ->willReturn(true); + + $view->method('touch') + ->with('/bar/foo/asd') + ->willReturn(true); + + $node = new Folder($root, $view, '/bar/foo'); + + switch ($case) { + case 0: + $child = new Folder($root, $view, '/bar/foo/' . $folderName, null, $node); + + $root->expects($this->any()) + ->method('get') + ->willReturnCallback(function (string $path) use ($root, $view, $folderName) { + if ($path === '/bar/foo/') { + return new Folder($root, $view, '/bar/foo/'); + } elseif ($path === '/bar/foo/' . $folderName) { + throw new NotFoundException(); + } + $this->fail('Trying to get ' . $path); + }); + + break; // do nothing + case 1: + $child = new Folder($root, $view, '/bar/foo/' . $folderName, null, $node); + + $root->expects($this->any()) + ->method('get') + ->with('/bar/foo/' . $folderName) + ->willReturn($child); + $node->newFolder($folderName); + break; + case 2: + $child = new Folder($root, $view, '/bar/foo/' . $folderName . ' (1)', null, $node); + $root->expects($this->any()) + ->method('get') + ->willReturnCallback(function (string $path) use ($root, $view, $folderName) { + if ($path === '/bar/foo/') { + return new Folder($root, $view, '/bar/foo/'); + } elseif ($path === '/bar/foo/' . $folderName) { + return new File($root, $view, '/bar/foo/asd'); + } elseif ($path === '/bar/foo/' . $folderName . ' (1)') { + throw new NotFoundException(); + } + $this->fail('Trying to get ' . $path); + }); + $node->newFile($folderName); + break; + } + + $result = $node->getOrCreateFolder($folderName); + $this->assertEquals($child, $result); + } } diff --git a/tests/lib/Files/Node/NodeTestCase.php b/tests/lib/Files/Node/NodeTestCase.php index 90f7c2c0f65..56a20c3d714 100644 --- a/tests/lib/Files/Node/NodeTestCase.php +++ b/tests/lib/Files/Node/NodeTestCase.php @@ -92,30 +92,24 @@ abstract class NodeTestCase extends \Test\TestCase { return $view; } + abstract protected function createTestNode(IRootFolder $root, View&MockObject $view, string $path, array $data = [], string $internalPath = '', ?IStorage $storage = null): Node; + /** - * @param IRootFolder $root - * @param View $view - * @param string $path - * @return Node + * @return class-string */ - abstract protected function createTestNode($root, $view, $path, array $data = [], $internalPath = '', $storage = null); + abstract protected function getNodeClass(): string; + + /** + * @return class-string + */ + abstract protected function getNonExistingNodeClass(): string; /** * @return string */ - abstract protected function getNodeClass(); + abstract protected function getViewDeleteMethod(): string; - /** - * @return string - */ - abstract protected function getNonExistingNodeClass(); - - /** - * @return string - */ - abstract protected function getViewDeleteMethod(); - - protected function getMockStorage() { + protected function getMockStorage(): IStorage&MockObject { $storage = $this->getMockBuilder(IStorage::class) ->disableOriginalConstructor() ->getMock(); @@ -125,7 +119,7 @@ abstract class NodeTestCase extends \Test\TestCase { return $storage; } - protected function getFileInfo($data, $internalPath = '', $storage = null) { + protected function getFileInfo($data, $internalPath = '', ?IStorage $storage = null) { $mount = $this->createMock(IMountPoint::class); $mount->method('getStorage') ->willReturn($storage); diff --git a/tests/lib/Files/Template/TemplateManagerTest.php b/tests/lib/Files/Template/TemplateManagerTest.php index f0c5acaec90..7b3f0beff42 100644 --- a/tests/lib/Files/Template/TemplateManagerTest.php +++ b/tests/lib/Files/Template/TemplateManagerTest.php @@ -23,8 +23,6 @@ use OCP\IDBConnection; use OCP\IL10N; use OCP\IPreview; use OCP\IServerContainer; -use OCP\IUserManager; -use OCP\IUserSession; use OCP\IUser; use OCP\L10N\IFactory; use Psr\Log\NullLogger;