diff options
| -rw-r--r-- | apps/dav/lib/Files/Sharing/FilesDropPlugin.php | 71 | ||||
| -rw-r--r-- | apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php | 161 | ||||
| -rw-r--r-- | build/integration/filesdrop_features/filesdrop.feature | 41 |
3 files changed, 153 insertions, 120 deletions
diff --git a/apps/dav/lib/Files/Sharing/FilesDropPlugin.php b/apps/dav/lib/Files/Sharing/FilesDropPlugin.php index 0bb3e3274b1..9aee5283ea9 100644 --- a/apps/dav/lib/Files/Sharing/FilesDropPlugin.php +++ b/apps/dav/lib/Files/Sharing/FilesDropPlugin.php @@ -6,6 +6,7 @@ namespace OCA\DAV\Files\Sharing; use OCP\Files\Folder; +use OCP\Files\NotFoundException; use OCP\Share\IShare; use Sabre\DAV\Exception\MethodNotAllowed; use Sabre\DAV\ServerPlugin; @@ -131,47 +132,55 @@ class FilesDropPlugin extends ServerPlugin { } // Create the folders along the way - $folders = $this->getPathSegments(dirname($relativePath)); - foreach ($folders as $folder) { - if ($folder === '') { + $folder = $node; + $pathSegments = $this->getPathSegments(dirname($relativePath)); + foreach ($pathSegments as $pathSegment) { + if ($pathSegment === '') { continue; - } // skip empty parts - if (!$node->nodeExists($folder)) { - $node->newFolder($folder); + } + + try { + // get the current folder + $currentFolder = $folder->get($pathSegment); + // check target is a folder + if ($currentFolder instanceof Folder) { + $folder = $currentFolder; + } else { + // otherwise look in the parent folder if we already create an unique folder name + foreach ($folder->getDirectoryListing() as $child) { + // we look for folders which match "NAME (SUFFIX)" + if ($child instanceof Folder && str_starts_with($child->getName(), $pathSegment)) { + $suffix = substr($child->getName(), strlen($pathSegment)); + if (preg_match('/^ \(\d+\)$/', $suffix)) { + // we found the unique folder name and can use it + $folder = $child; + break; + } + } + } + // no folder found so we need to create a new unique folder name + if (!isset($child) || $child !== $folder) { + $folder = $folder->newFolder($folder->getNonExistingName($pathSegment)); + } + } + } catch (NotFoundException) { + // the folder does simply not exist so we create it + $folder = $folder->newFolder($pathSegment); } } // Finally handle conflicts on the end files - /** @var Folder */ - $folder = $node->get(dirname($relativePath)); - $uniqueName = $folder->getNonExistingName(basename(($relativePath))); - $path = '/files/' . $token . '/' . dirname($relativePath) . '/' . $uniqueName; - $url = $request->getBaseUrl() . str_replace('//', '/', $path); + $uniqueName = $folder->getNonExistingName(basename($relativePath)); + $relativePath = substr($folder->getPath(), strlen($node->getPath())); + $path = '/files/' . $token . '/' . $relativePath . '/' . $uniqueName; + $url = rtrim($request->getBaseUrl(), '/') . str_replace('//', '/', $path); $request->setUrl($url); } private function getPathSegments(string $path): array { // Normalize slashes and remove trailing slash - $path = rtrim(str_replace('\\', '/', $path), '/'); - - // Handle absolute paths starting with / - $isAbsolute = str_starts_with($path, '/'); - - $segments = explode('/', $path); - - // Add back the leading slash for the first segment if needed - $result = []; - $current = $isAbsolute ? '/' : ''; - - foreach ($segments as $segment) { - if ($segment === '') { - // skip empty parts - continue; - } - $current = rtrim($current, '/') . '/' . $segment; - $result[] = $current; - } + $path = trim(str_replace('\\', '/', $path), '/'); - return $result; + return explode('/', $path); } } diff --git a/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php b/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php index dc1e067dafd..16891ced3d0 100644 --- a/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php +++ b/apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php @@ -5,10 +5,12 @@ */ namespace OCA\DAV\Tests\Files\Sharing; -use OC\Files\View; use OCA\DAV\Files\Sharing\FilesDropPlugin; +use OCP\Files\Folder; +use OCP\Files\NotFoundException; use OCP\Share\IAttributes; use OCP\Share\IShare; +use PHPUnit\Framework\MockObject\MockObject; use Sabre\DAV\Exception\MethodNotAllowed; use Sabre\DAV\Server; use Sabre\HTTP\RequestInterface; @@ -17,29 +19,25 @@ use Test\TestCase; class FilesDropPluginTest extends TestCase { - /** @var View|\PHPUnit\Framework\MockObject\MockObject */ - private $view; + private FilesDropPlugin $plugin; - /** @var IShare|\PHPUnit\Framework\MockObject\MockObject */ - private $share; - - /** @var Server|\PHPUnit\Framework\MockObject\MockObject */ - private $server; - - /** @var FilesDropPlugin */ - private $plugin; - - /** @var RequestInterface|\PHPUnit\Framework\MockObject\MockObject */ - private $request; - - /** @var ResponseInterface|\PHPUnit\Framework\MockObject\MockObject */ - private $response; + private Folder&MockObject $node; + private IShare&MockObject $share; + private Server&MockObject $server; + private RequestInterface&MockObject $request; + private ResponseInterface&MockObject $response; protected function setUp(): void { parent::setUp(); - $this->view = $this->createMock(View::class); + $this->node = $this->createMock(Folder::class); + $this->node->method('getPath') + ->willReturn('/files/token'); + $this->share = $this->createMock(IShare::class); + $this->share->expects(self::any()) + ->method('getNode') + ->willReturn($this->node); $this->server = $this->createMock(Server::class); $this->plugin = new FilesDropPlugin(); @@ -56,28 +54,7 @@ class FilesDropPluginTest extends TestCase { ->willReturn('token'); } - public function testInitialize(): void { - $this->server->expects($this->at(0)) - ->method('on') - ->with( - $this->equalTo('beforeMethod:*'), - $this->equalTo([$this->plugin, 'beforeMethod']), - $this->equalTo(999) - ); - $this->server->expects($this->at(1)) - ->method('on') - ->with( - $this->equalTo('method:MKCOL'), - $this->equalTo([$this->plugin, 'onMkcol']), - ); - - $this->plugin->initialize($this->server); - } - public function testNotEnabled(): void { - $this->view->expects($this->never()) - ->method($this->anything()); - $this->request->expects($this->never()) ->method($this->anything()); @@ -86,7 +63,6 @@ class FilesDropPluginTest extends TestCase { public function testValid(): void { $this->plugin->enable(); - $this->plugin->setView($this->view); $this->plugin->setShare($this->share); $this->request->method('getMethod') @@ -98,9 +74,10 @@ class FilesDropPluginTest extends TestCase { $this->request->method('getBaseUrl') ->willReturn('https://example.com'); - $this->view->method('file_exists') - ->with('/file.txt') - ->willReturn(false); + $this->node->expects(self::once()) + ->method('getNonExistingName') + ->with('file.txt') + ->willReturn('file.txt'); $this->request->expects($this->once()) ->method('setUrl') @@ -111,7 +88,6 @@ class FilesDropPluginTest extends TestCase { public function testFileAlreadyExistsValid(): void { $this->plugin->enable(); - $this->plugin->setView($this->view); $this->plugin->setShare($this->share); $this->request->method('getMethod') @@ -123,14 +99,9 @@ class FilesDropPluginTest extends TestCase { $this->request->method('getBaseUrl') ->willReturn('https://example.com'); - $this->view->method('file_exists') - ->willReturnCallback(function ($path) { - if ($path === 'file.txt' || $path === '/file.txt') { - return true; - } else { - return false; - } - }); + $this->node->method('getNonExistingName') + ->with('file.txt') + ->willReturn('file (2).txt'); $this->request->expects($this->once()) ->method('setUrl') @@ -141,7 +112,6 @@ class FilesDropPluginTest extends TestCase { public function testNoMKCOLWithoutNickname(): void { $this->plugin->enable(); - $this->plugin->setView($this->view); $this->plugin->setShare($this->share); $this->request->method('getMethod') @@ -154,7 +124,6 @@ class FilesDropPluginTest extends TestCase { public function testMKCOLWithNickname(): void { $this->plugin->enable(); - $this->plugin->setView($this->view); $this->plugin->setShare($this->share); $this->request->method('getMethod') @@ -174,7 +143,6 @@ class FilesDropPluginTest extends TestCase { public function testSubdirPut(): void { $this->plugin->enable(); - $this->plugin->setView($this->view); $this->plugin->setShare($this->share); $this->request->method('getMethod') @@ -193,14 +161,30 @@ class FilesDropPluginTest extends TestCase { $this->request->method('getBaseUrl') ->willReturn('https://example.com'); - $this->view->method('file_exists') - ->willReturnCallback(function ($path) { - if ($path === 'file.txt' || $path === '/folder/file.txt') { - return true; - } else { - return false; - } - }); + $nodeName = $this->createMock(Folder::class); + $nodeFolder = $this->createMock(Folder::class); + $nodeFolder->expects(self::once()) + ->method('getPath') + ->willReturn('/files/token/nickname/folder'); + $nodeFolder->method('getNonExistingName') + ->with('file.txt') + ->willReturn('file.txt'); + $nodeName->expects(self::once()) + ->method('get') + ->with('folder') + ->willThrowException(new NotFoundException()); + $nodeName->expects(self::once()) + ->method('newFolder') + ->with('folder') + ->willReturn($nodeFolder); + + $this->node->expects(self::once()) + ->method('get') + ->willThrowException(new NotFoundException()); + $this->node->expects(self::once()) + ->method('newFolder') + ->with('nickname') + ->willReturn($nodeName); $this->request->expects($this->once()) ->method('setUrl') @@ -211,7 +195,6 @@ class FilesDropPluginTest extends TestCase { public function testRecursiveFolderCreation(): void { $this->plugin->enable(); - $this->plugin->setView($this->view); $this->plugin->setShare($this->share); $this->request->method('getMethod') @@ -227,40 +210,40 @@ class FilesDropPluginTest extends TestCase { ->willReturn('/files/token/folder/subfolder/file.txt'); $this->request->method('getBaseUrl') ->willReturn('https://example.com'); - $this->view->method('file_exists') - ->willReturn(false); - - $this->view->expects($this->exactly(4)) - ->method('file_exists') - ->withConsecutive( - ['/nickname'], - ['/nickname/folder'], - ['/nickname/folder/subfolder'], - ['/nickname/folder/subfolder/file.txt'] - ) - ->willReturnOnConsecutiveCalls( - false, - false, - false, - false, - ); - $this->view->expects($this->exactly(3)) - ->method('mkdir') - ->withConsecutive( - ['/nickname'], - ['/nickname/folder'], - ['/nickname/folder/subfolder'], - ); $this->request->expects($this->once()) ->method('setUrl') ->with($this->equalTo('https://example.com/files/token/nickname/folder/subfolder/file.txt')); + + $subfolder = $this->createMock(Folder::class); + $subfolder->expects(self::once()) + ->method('getNonExistingName') + ->with('file.txt') + ->willReturn('file.txt'); + $subfolder->expects(self::once()) + ->method('getPath') + ->willReturn('/files/token/nickname/folder/subfolder'); + + $folder = $this->createMock(Folder::class); + $folder->expects(self::once()) + ->method('get') + ->with('subfolder') + ->willReturn($subfolder); + + $nickname = $this->createMock(Folder::class); + $nickname->expects(self::once()) + ->method('get') + ->with('folder') + ->willReturn($folder); + + $this->node->method('get') + ->with('nickname') + ->willReturn($nickname); $this->plugin->beforeMethod($this->request, $this->response); } public function testOnMkcol(): void { $this->plugin->enable(); - $this->plugin->setView($this->view); $this->plugin->setShare($this->share); $this->response->expects($this->once()) diff --git a/build/integration/filesdrop_features/filesdrop.feature b/build/integration/filesdrop_features/filesdrop.feature index eacd19a4b13..e37a4d7138b 100644 --- a/build/integration/filesdrop_features/filesdrop.feature +++ b/build/integration/filesdrop_features/filesdrop.feature @@ -99,6 +99,47 @@ Feature: FilesDrop And Downloading file "/drop/Alice/folder/a.txt" Then Downloaded content should be "abc" + Scenario: File drop uploading folder with name of file + Given user "user0" exists + And As an "user0" + And user "user0" created a folder "/drop" + And as "user0" creating a share with + | path | drop | + | shareType | 4 | + | permissions | 4 | + | attributes | [{"scope":"fileRequest","key":"enabled","value":true}] | + | shareWith | | + When Dropping file "/folder" with "its a file" as "Alice" + Then the HTTP status code should be "201" + When Dropping file "/folder/a.txt" with "abc" as "Alice" + Then the HTTP status code should be "201" + When Downloading file "/drop/Alice/folder" + Then the HTTP status code should be "200" + And Downloaded content should be "its a file" + When Downloading file "/drop/Alice/folder (2)/a.txt" + Then Downloaded content should be "abc" + + Scenario: File drop uploading file with name of folder + Given user "user0" exists + And As an "user0" + And user "user0" created a folder "/drop" + And as "user0" creating a share with + | path | drop | + | shareType | 4 | + | permissions | 4 | + | attributes | [{"scope":"fileRequest","key":"enabled","value":true}] | + | shareWith | | + When Dropping file "/folder/a.txt" with "abc" as "Alice" + Then the HTTP status code should be "201" + When Dropping file "/folder" with "its a file" as "Alice" + Then the HTTP status code should be "201" + When Downloading file "/drop/Alice/folder/a.txt" + Then the HTTP status code should be "200" + And Downloaded content should be "abc" + When Downloading file "/drop/Alice/folder (2)" + Then the HTTP status code should be "200" + And Downloaded content should be "its a file" + Scenario: Put file same file multiple times via files drop Given user "user0" exists And As an "user0" |