aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVincent Petry <vincent@nextcloud.com>2022-05-25 09:55:22 +0200
committerCarl Schwan <carl@carlschwan.eu>2022-07-28 16:53:22 +0200
commit03b1791cca3e0334637aa232d1f7c11850793646 (patch)
tree33db9b8db60a592089aaa7a4d6c04f4db4c1860f
parent9493f86de34e76e37c13f87aab3123a3efbfdd84 (diff)
downloadnextcloud-server-03b1791cca3e0334637aa232d1f7c11850793646.tar.gz
nextcloud-server-03b1791cca3e0334637aa232d1f7c11850793646.zip
Fix share attribute related tests + code style
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
-rw-r--r--apps/dav/lib/DAV/ViewOnlyPlugin.php1
-rw-r--r--apps/files_sharing/lib/Controller/ShareAPIController.php43
-rw-r--r--apps/files_sharing/tests/ApiTest.php11
-rw-r--r--apps/files_sharing/tests/ApplicationTest.php19
-rw-r--r--apps/files_sharing/tests/Controller/ShareAPIControllerTest.php34
-rw-r--r--apps/files_sharing/tests/MountProviderTest.php7
-rw-r--r--lib/private/Share20/Manager.php15
-rw-r--r--lib/private/Share20/Share.php4
-rw-r--r--lib/public/Share/IAttributes.php8
-rw-r--r--lib/public/Share/IShare.php8
-rw-r--r--tests/lib/Share20/ManagerTest.php3
11 files changed, 87 insertions, 66 deletions
diff --git a/apps/dav/lib/DAV/ViewOnlyPlugin.php b/apps/dav/lib/DAV/ViewOnlyPlugin.php
index b6cd85a69a0..cd322572dcc 100644
--- a/apps/dav/lib/DAV/ViewOnlyPlugin.php
+++ b/apps/dav/lib/DAV/ViewOnlyPlugin.php
@@ -48,6 +48,7 @@ class ViewOnlyPlugin extends ServerPlugin {
*/
public function __construct(ILogger $logger) {
$this->logger = $logger;
+ $this->server = null;
}
/**
diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php
index f5c64c5d0d0..6b43e0f9dcf 100644
--- a/apps/files_sharing/lib/Controller/ShareAPIController.php
+++ b/apps/files_sharing/lib/Controller/ShareAPIController.php
@@ -45,7 +45,6 @@ declare(strict_types=1);
namespace OCA\Files_Sharing\Controller;
use OC\Files\FileInfo;
-use OCA\DAV\DAV\ViewOnlyPlugin;
use OCA\Files_Sharing\Exceptions\SharingRightsException;
use OCA\Files_Sharing\External\Storage;
use OCA\Files\Helper;
@@ -442,6 +441,7 @@ class ShareAPIController extends OCSController {
* @param string $sendPasswordByTalk
* @param string $expireDate
* @param string $label
+ * @param string $attributes
*
* @return DataResponse
* @throws NotFoundException
@@ -462,7 +462,8 @@ class ShareAPIController extends OCSController {
string $sendPasswordByTalk = null,
string $expireDate = '',
string $note = '',
- string $label = ''
+ string $label = '',
+ string $attributes = null
): DataResponse {
$share = $this->shareManager->newShare();
@@ -680,7 +681,7 @@ class ShareAPIController extends OCSController {
$share->setNote($note);
}
- $share = $this->setShareAttributes($share, $this->request->getParam('attributes', null));
+ $share = $this->setShareAttributes($share, $attributes);
try {
$share = $this->shareManager->createShare($share);
@@ -1043,6 +1044,7 @@ class ShareAPIController extends OCSController {
* @param string $note
* @param string $label
* @param string $hideDownload
+ * @param string $attributes
* @return DataResponse
* @throws LockedException
* @throws NotFoundException
@@ -1059,7 +1061,8 @@ class ShareAPIController extends OCSController {
string $expireDate = null,
string $note = null,
string $label = null,
- string $hideDownload = null
+ string $hideDownload = null,
+ string $attributes = null
): DataResponse {
try {
$share = $this->getShareById($id);
@@ -1077,8 +1080,6 @@ class ShareAPIController extends OCSController {
throw new OCSForbiddenException('You are not allowed to edit incoming shares');
}
- $shareAttributes = $this->request->getParam('attributes', null);
-
if (
$permissions === null &&
$password === null &&
@@ -1088,7 +1089,7 @@ class ShareAPIController extends OCSController {
$note === null &&
$label === null &&
$hideDownload === null &&
- $shareAttributes === null
+ $attributes === null
) {
throw new OCSBadRequestException($this->l->t('Wrong or no update parameter given'));
}
@@ -1227,7 +1228,7 @@ class ShareAPIController extends OCSController {
}
}
- $share = $this->setShareAttributes($share, $shareAttributes);
+ $share = $this->setShareAttributes($share, $attributes);
try {
$share = $this->shareManager->updateShare($share);
@@ -1848,18 +1849,24 @@ class ShareAPIController extends OCSController {
/**
* @param IShare $share
- * @param string[][]|null $formattedShareAttributes
+ * @param string|null $attributesString
* @return IShare modified share
*/
- private function setShareAttributes(IShare $share, $formattedShareAttributes) {
- $newShareAttributes = $this->shareManager->newShare()->newAttributes();
- if ($formattedShareAttributes !== null) {
- foreach ($formattedShareAttributes as $formattedAttr) {
- $newShareAttributes->setAttribute(
- $formattedAttr['scope'],
- $formattedAttr['key'],
- (bool) \json_decode($formattedAttr['enabled'])
- );
+ private function setShareAttributes(IShare $share, ?string $attributesString) {
+ $newShareAttributes = null;
+ if ($attributesString !== null) {
+ $newShareAttributes = $this->shareManager->newShare()->newAttributes();
+ $formattedShareAttributes = \json_decode($attributesString, true);
+ if (is_array($formattedShareAttributes)) {
+ foreach ($formattedShareAttributes as $formattedAttr) {
+ $newShareAttributes->setAttribute(
+ $formattedAttr['scope'],
+ $formattedAttr['key'],
+ is_string($formattedAttr['enabled']) ? (bool) \json_decode($formattedAttr['enabled']) : $formattedAttr['enabled']
+ );
+ }
+ } else {
+ throw new OCSBadRequestException('Invalid share attributes provided: \"' . $attributesString . '\"');
}
}
$share->setAttributes($newShareAttributes);
diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php
index 50869b7a9f8..98d0c171681 100644
--- a/apps/files_sharing/tests/ApiTest.php
+++ b/apps/files_sharing/tests/ApiTest.php
@@ -950,9 +950,13 @@ class ApiTest extends TestCase {
->setShareType(IShare::TYPE_USER)
->setPermissions(19)
->setAttributes($this->shareManager->newShare()->newAttributes());
+
+ $this->assertNotNull($share1->getAttributes());
$share1 = $this->shareManager->createShare($share1);
+ $this->assertNull($share1->getAttributes());
$this->assertEquals(19, $share1->getPermissions());
- $this->assertEquals(null, $share1->getAttributes());
+ // attributes get cleared when empty
+ $this->assertNull($share1->getAttributes());
$share2 = $this->shareManager->newShare();
$share2->setNode($node1)
@@ -964,7 +968,10 @@ class ApiTest extends TestCase {
// update permissions
$ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1);
- $ocs->updateShare($share1->getId(), 1);
+ $ocs->updateShare(
+ $share1->getId(), 1, null, null, null, null, null, null, null,
+ '[{"scope": "app1", "key": "attr1", "enabled": true}]'
+ );
$ocs->cleanup();
$share1 = $this->shareManager->getShareById('ocinternal:' . $share1->getId());
diff --git a/apps/files_sharing/tests/ApplicationTest.php b/apps/files_sharing/tests/ApplicationTest.php
index 3f3164b233e..ee04996ac15 100644
--- a/apps/files_sharing/tests/ApplicationTest.php
+++ b/apps/files_sharing/tests/ApplicationTest.php
@@ -65,8 +65,7 @@ class ApplicationTest extends TestCase {
$this->application = new Application([]);
- // FIXME: how to mock this one ??
- $symfonyDispatcher = $this->createMock(SymfonyDispatcher::class);
+ $symfonyDispatcher = new SymfonyDispatcher();
$this->eventDispatcher = new EventDispatcher(
$symfonyDispatcher,
$this->createMock(IServerContainer::class),
@@ -133,9 +132,9 @@ class ApplicationTest extends TestCase {
*/
public function testCheckDirectCanBeDownloaded($path, $userFolder, $run) {
$user = $this->createMock(IUser::class);
- $user->method("getUID")->willReturn("test");
- $this->userSession->method("getUser")->willReturn($user);
- $this->userSession->method("isLoggedIn")->willReturn(true);
+ $user->method('getUID')->willReturn('test');
+ $this->userSession->method('getUser')->willReturn($user);
+ $this->userSession->method('isLoggedIn')->willReturn(true);
$this->rootFolder->method('getUserFolder')->willReturn($userFolder);
// Simulate direct download of file
@@ -210,11 +209,11 @@ class ApplicationTest extends TestCase {
*/
public function testCheckZipCanBeDownloaded($dir, $files, $userFolder, $run) {
$user = $this->createMock(IUser::class);
- $user->method("getUID")->willReturn("test");
- $this->userSession->method("getUser")->willReturn($user);
- $this->userSession->method("isLoggedIn")->willReturn(true);
+ $user->method('getUID')->willReturn('test');
+ $this->userSession->method('getUser')->willReturn($user);
+ $this->userSession->method('isLoggedIn')->willReturn(true);
- $this->rootFolder->method('getUserFolder')->with("test")->willReturn($userFolder);
+ $this->rootFolder->method('getUserFolder')->with('test')->willReturn($userFolder);
// Simulate zip download of folder folder
$event = new GenericEvent(null, ['dir' => $dir, 'files' => $files, 'run' => true]);
@@ -225,7 +224,7 @@ class ApplicationTest extends TestCase {
}
public function testCheckFileUserNotFound() {
- $this->userSession->method("isLoggedIn")->willReturn(false);
+ $this->userSession->method('isLoggedIn')->willReturn(false);
// Simulate zip download of folder folder
$event = new GenericEvent(null, ['dir' => '/test', 'files' => ['test.txt'], 'run' => true]);
diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
index f6568415727..a6a81bd672c 100644
--- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
+++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
@@ -58,6 +58,7 @@ use OCP\IUser;
use OCP\IUserManager;
use OCP\Lock\LockedException;
use OCP\Share\Exceptions\GenericShareException;
+use OCP\Share\IAttributes as IShareAttributes;
use OCP\Share\IManager;
use OCP\Share\IShare;
use Test\TestCase;
@@ -125,10 +126,6 @@ class ShareAPIControllerTest extends TestCase {
$this->shareManager
->expects($this->any())
->method('shareProviderExists')->willReturn(true);
- $this->shareManager
- ->expects($this->any())
- ->method('newShare')
- ->willReturn($this->newShare());
$this->groupManager = $this->createMock(IGroupManager::class);
$this->userManager = $this->createMock(IUserManager::class);
$this->request = $this->createMock(IRequest::class);
@@ -201,11 +198,9 @@ class ShareAPIControllerTest extends TestCase {
private function mockShareAttributes() {
$formattedShareAttributes = [
[
- [
- 'scope' => 'permissions',
- 'key' => 'download',
- 'enabled' => true
- ]
+ 'scope' => 'permissions',
+ 'key' => 'download',
+ 'enabled' => true
]
];
@@ -641,6 +636,7 @@ class ShareAPIControllerTest extends TestCase {
'can_edit' => false,
'can_delete' => false,
'status' => [],
+ 'attributes' => null,
];
$data[] = [$share, $expected];
@@ -692,6 +688,7 @@ class ShareAPIControllerTest extends TestCase {
'hide_download' => 0,
'can_edit' => false,
'can_delete' => false,
+ 'attributes' => null,
];
$data[] = [$share, $expected];
@@ -749,6 +746,7 @@ class ShareAPIControllerTest extends TestCase {
'hide_download' => 0,
'can_edit' => false,
'can_delete' => false,
+ 'attributes' => null,
];
$data[] = [$share, $expected];
@@ -3808,6 +3806,7 @@ class ShareAPIControllerTest extends TestCase {
'can_edit' => false,
'can_delete' => false,
'status' => [],
+ 'attributes' => '[{"scope":"permissions","key":"download","enabled":true}]',
], $share, [], false
];
// User backend up
@@ -3845,6 +3844,7 @@ class ShareAPIControllerTest extends TestCase {
'can_edit' => false,
'can_delete' => false,
'status' => [],
+ 'attributes' => '[{"scope":"permissions","key":"download","enabled":true}]',
], $share, [
['owner', $owner],
['initiator', $initiator],
@@ -3898,6 +3898,7 @@ class ShareAPIControllerTest extends TestCase {
'can_edit' => false,
'can_delete' => false,
'status' => [],
+ 'attributes' => null,
], $share, [], false
];
@@ -3947,6 +3948,7 @@ class ShareAPIControllerTest extends TestCase {
'can_edit' => true,
'can_delete' => true,
'status' => [],
+ 'attributes' => null,
], $share, [], false
];
@@ -3996,6 +3998,7 @@ class ShareAPIControllerTest extends TestCase {
'hide_download' => 0,
'can_edit' => false,
'can_delete' => false,
+ 'attributes' => null,
], $share, [], false
];
@@ -4042,6 +4045,7 @@ class ShareAPIControllerTest extends TestCase {
'hide_download' => 0,
'can_edit' => false,
'can_delete' => false,
+ 'attributes' => null,
], $share, [], false
];
@@ -4095,6 +4099,7 @@ class ShareAPIControllerTest extends TestCase {
'hide_download' => 0,
'can_edit' => false,
'can_delete' => false,
+ 'attributes' => null,
], $share, [], false
];
@@ -4148,6 +4153,7 @@ class ShareAPIControllerTest extends TestCase {
'hide_download' => 0,
'can_edit' => false,
'can_delete' => false,
+ 'attributes' => null,
], $share, [], false
];
@@ -4195,6 +4201,7 @@ class ShareAPIControllerTest extends TestCase {
'hide_download' => 0,
'can_edit' => false,
'can_delete' => false,
+ 'attributes' => null,
], $share, [], false
];
@@ -4242,6 +4249,7 @@ class ShareAPIControllerTest extends TestCase {
'hide_download' => 0,
'can_edit' => false,
'can_delete' => false,
+ 'attributes' => null,
], $share, [], false
];
@@ -4292,6 +4300,7 @@ class ShareAPIControllerTest extends TestCase {
'hide_download' => 0,
'can_edit' => false,
'can_delete' => false,
+ 'attributes' => null,
], $share, [], false
];
@@ -4339,6 +4348,7 @@ class ShareAPIControllerTest extends TestCase {
'hide_download' => 0,
'can_edit' => false,
'can_delete' => false,
+ 'attributes' => null,
], $share, [], false
];
@@ -4386,6 +4396,7 @@ class ShareAPIControllerTest extends TestCase {
'hide_download' => 0,
'can_edit' => false,
'can_delete' => false,
+ 'attributes' => null,
], $share, [], false
];
@@ -4450,6 +4461,7 @@ class ShareAPIControllerTest extends TestCase {
'can_edit' => false,
'can_delete' => false,
'password_expiration_time' => null,
+ 'attributes' => null,
], $share, [], false
];
@@ -4500,6 +4512,7 @@ class ShareAPIControllerTest extends TestCase {
'can_edit' => false,
'can_delete' => false,
'password_expiration_time' => null,
+ 'attributes' => null,
], $share, [], false
];
@@ -4549,6 +4562,7 @@ class ShareAPIControllerTest extends TestCase {
'can_edit' => true,
'can_delete' => true,
'status' => [],
+ 'attributes' => null,
], $share, [], false
];
@@ -4700,6 +4714,7 @@ class ShareAPIControllerTest extends TestCase {
'label' => '',
'can_edit' => false,
'can_delete' => false,
+ 'attributes' => null,
], $share, false, []
];
@@ -4746,6 +4761,7 @@ class ShareAPIControllerTest extends TestCase {
'label' => '',
'can_edit' => false,
'can_delete' => false,
+ 'attributes' => null,
], $share, true, [
'share_with_displayname' => 'recipientRoomName'
]
diff --git a/apps/files_sharing/tests/MountProviderTest.php b/apps/files_sharing/tests/MountProviderTest.php
index 740c7c89eb7..37e7e3d9d03 100644
--- a/apps/files_sharing/tests/MountProviderTest.php
+++ b/apps/files_sharing/tests/MountProviderTest.php
@@ -39,6 +39,7 @@ use OCP\IConfig;
use OCP\ILogger;
use OCP\IUser;
use OCP\IUserManager;
+use OCP\Share\IAttributes as IShareAttributes;
use OCP\Share\IManager;
use OCP\Share\IShare;
@@ -102,7 +103,7 @@ class MountProviderTest extends \Test\TestCase {
return $shareAttributes;
}
- private function makeMockShare($id, $nodeId, $owner = 'user2', $target = null, $permissions = 31, $attributes) {
+ private function makeMockShare($id, $nodeId, $owner = 'user2', $target = null, $permissions = 31, $attributes = null) {
$share = $this->createMock(IShare::class);
$share->expects($this->any())
->method('getPermissions')
@@ -371,10 +372,10 @@ class MountProviderTest extends \Test\TestCase {
$userManager = $this->createMock(IUserManager::class);
$userShares = array_map(function ($shareSpec) {
- return $this->makeMockShare($shareSpec[0], $shareSpec[1], $shareSpec[2], $shareSpec[3], $shareSpec[4]);
+ return $this->makeMockShare($shareSpec[0], $shareSpec[1], $shareSpec[2], $shareSpec[3], $shareSpec[4], $shareSpec[5]);
}, $userShares);
$groupShares = array_map(function ($shareSpec) {
- return $this->makeMockShare($shareSpec[0], $shareSpec[1], $shareSpec[2], $shareSpec[3], $shareSpec[4]);
+ return $this->makeMockShare($shareSpec[0], $shareSpec[1], $shareSpec[2], $shareSpec[3], $shareSpec[4], $shareSpec[5]);
}, $groupShares);
$this->user->expects($this->any())
diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
index d0120a299be..a46126b7ac4 100644
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@@ -70,7 +70,6 @@ use OCP\Share;
use OCP\Share\Exceptions\AlreadySharedException;
use OCP\Share\Exceptions\GenericShareException;
use OCP\Share\Exceptions\ShareNotFound;
-use OCP\Share\IAttributes;
use OCP\Share\IManager;
use OCP\Share\IProviderFactory;
use OCP\Share\IShare;
@@ -1094,7 +1093,7 @@ class Manager implements IManager {
'shareWith' => $share->getSharedWith(),
'uidOwner' => $share->getSharedBy(),
'permissions' => $share->getPermissions(),
- 'attributes' => $share->getAttributes(),
+ 'attributes' => $share->getAttributes() !== null ? $share->getAttributes()->toArray() : null,
'path' => $userFolder->getRelativePath($share->getNode()->getPath()),
]);
}
@@ -2089,16 +2088,4 @@ class Manager implements IManager {
yield from $provider->getAllShares();
}
}
-
- /**
- * @param IAttributes|null $perms
- * @return string
- */
- private function hashAttributes($perms) {
- if ($perms === null || empty($perms->toArray())) {
- return "";
- }
-
- return \md5(\json_encode($perms->toArray()));
- }
}
diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php
index 1aeb79d4d31..9ba5db5252e 100644
--- a/lib/private/Share20/Share.php
+++ b/lib/private/Share20/Share.php
@@ -345,7 +345,7 @@ class Share implements IShare {
/**
* @inheritdoc
*/
- public function setAttributes(IAttributes $attributes) {
+ public function setAttributes(?IAttributes $attributes) {
$this->attributes = $attributes;
return $this;
}
@@ -353,7 +353,7 @@ class Share implements IShare {
/**
* @inheritdoc
*/
- public function getAttributes() {
+ public function getAttributes(): ?IAttributes {
return $this->attributes;
}
diff --git a/lib/public/Share/IAttributes.php b/lib/public/Share/IAttributes.php
index 9f2556e4005..6e4cee08b12 100644
--- a/lib/public/Share/IAttributes.php
+++ b/lib/public/Share/IAttributes.php
@@ -24,7 +24,7 @@ namespace OCP\Share;
* Interface IAttributes
*
* @package OCP\Share
- * @since 10.2.0
+ * @since 25.0.0
*/
interface IAttributes {
@@ -35,7 +35,7 @@ interface IAttributes {
* @param string $key key
* @param bool $enabled enabled
* @return IAttributes The modified object
- * @since 10.2.0
+ * @since 25.0.0
*/
public function setAttribute($scope, $key, $enabled);
@@ -46,7 +46,7 @@ interface IAttributes {
* @param string $scope scope
* @param string $key key
* @return bool|null
- * @since 10.2.0
+ * @since 25.0.0
*/
public function getAttribute($scope, $key);
@@ -62,7 +62,7 @@ interface IAttributes {
* ]
*
* @return array formatted IAttributes
- * @since 10.2.0
+ * @since 25.0.0
*/
public function toArray();
}
diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php
index d81f263b464..f86e1ec542d 100644
--- a/lib/public/Share/IShare.php
+++ b/lib/public/Share/IShare.php
@@ -325,19 +325,19 @@ interface IShare {
/**
* Set share attributes
*
- * @param IAttributes $attributes
+ * @param ?IAttributes $attributes
* @since 25.0.0
* @return IShare The modified object
*/
- public function setAttributes(IAttributes $attributes);
+ public function setAttributes(?IAttributes $attributes);
/**
* Get share attributes
*
* @since 25.0.0
- * @return IAttributes
+ * @return ?IAttributes
*/
- public function getAttributes();
+ public function getAttributes(): ?IAttributes;
/**
* Set the accepted status
diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php
index 797a5ebf683..ab296172a3c 100644
--- a/tests/lib/Share20/ManagerTest.php
+++ b/tests/lib/Share20/ManagerTest.php
@@ -3132,6 +3132,8 @@ class ManagerTest extends \Test\TestCase {
$manager->expects($this->once())->method('getShareById')->with('foo:42')->willReturn($originalShare);
$share = $this->manager->newShare();
+ $attrs = $this->manager->newShare()->newAttributes();
+ $attrs->setAttribute('app1', 'perm1', true);
$share->setProviderId('foo')
->setId('42')
->setShareType(IShare::TYPE_USER)
@@ -3164,6 +3166,7 @@ class ManagerTest extends \Test\TestCase {
'uidOwner' => 'sharer',
'permissions' => 31,
'path' => '/myPath',
+ 'attributes' => $attrs->toArray(),
]);
$manager->updateShare($share);