diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 6f79420b11338..0a5711081b093 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -611,7 +611,7 @@ public function createShare( ?IShareAttributes $attributes = null, ): MockObject { $share = $this->createMock(IShare::class); - $share->method('getId')->willReturn($id); + $share->method('getId')->willReturn((string)$id); $share->method('getShareType')->willReturn($shareType); $share->method('getSharedWith')->willReturn($sharedWith); $share->method('getSharedBy')->willReturn($sharedBy); @@ -1014,11 +1014,11 @@ public static function dataGetShares(): array { 'sharedBy' => 'initiator', 'owner' => 'currentUser', 'node' => $file1, - 'id' => 4, + 'id' => '4', ]; $file1UserShareOwnerExpected = [ - 'id' => 4, + 'id' => '4', 'share_type' => IShare::TYPE_USER, ]; @@ -1028,11 +1028,11 @@ public static function dataGetShares(): array { 'sharedBy' => 'currentUser', 'owner' => 'owner', 'node' => $file1, - 'id' => 8, + 'id' => '8', ]; $file1UserShareInitiatorExpected = [ - 'id' => 8, + 'id' => '8', 'share_type' => IShare::TYPE_USER, ]; @@ -1042,11 +1042,11 @@ public static function dataGetShares(): array { 'sharedBy' => 'initiator', 'owner' => 'owner', 'node' => $file1, - 'id' => 15, + 'id' => '15', ]; $file1UserShareRecipientExpected = [ - 'id' => 15, + 'id' => '15', 'share_type' => IShare::TYPE_USER, ]; @@ -1056,11 +1056,11 @@ public static function dataGetShares(): array { 'sharedBy' => 'initiator', 'owner' => 'owner', 'node' => $file1, - 'id' => 16, + 'id' => '16', ]; $file1UserShareOtherExpected = [ - 'id' => 16, + 'id' => '16', 'share_type' => IShare::TYPE_USER, ]; @@ -1070,11 +1070,11 @@ public static function dataGetShares(): array { 'sharedBy' => 'initiator', 'owner' => 'currentUser', 'node' => $file1, - 'id' => 23, + 'id' => '23', ]; $file1GroupShareOwnerExpected = [ - 'id' => 23, + 'id' => '23', 'share_type' => IShare::TYPE_GROUP, ]; @@ -1084,11 +1084,11 @@ public static function dataGetShares(): array { 'sharedBy' => 'initiator', 'owner' => 'owner', 'node' => $file1, - 'id' => 42, + 'id' => '42', ]; $file1GroupShareRecipientExpected = [ - 'id' => 42, + 'id' => '42', 'share_type' => IShare::TYPE_GROUP, ]; @@ -1098,7 +1098,7 @@ public static function dataGetShares(): array { 'sharedBy' => 'initiator', 'owner' => 'owner', 'node' => $file1, - 'id' => 108, + 'id' => '108', ]; $file1LinkShareOwner = [ @@ -1107,11 +1107,11 @@ public static function dataGetShares(): array { 'sharedBy' => 'initiator', 'owner' => 'currentUser', 'node' => $file1, - 'id' => 415, + 'id' => '415', ]; $file1LinkShareOwnerExpected = [ - 'id' => 415, + 'id' => '415', 'share_type' => IShare::TYPE_LINK, ]; @@ -1121,11 +1121,11 @@ public static function dataGetShares(): array { 'sharedBy' => 'initiator', 'owner' => 'currentUser', 'node' => $file1, - 'id' => 416, + 'id' => '416', ]; $file1EmailShareOwnerExpected = [ - 'id' => 416, + 'id' => '416', 'share_type' => IShare::TYPE_EMAIL, ]; @@ -1135,11 +1135,11 @@ public static function dataGetShares(): array { 'sharedBy' => 'initiator', 'owner' => 'currentUser', 'node' => $file1, - 'id' => 423, + 'id' => '423', ]; $file1CircleShareOwnerExpected = [ - 'id' => 423, + 'id' => '423', 'share_type' => IShare::TYPE_CIRCLE, ]; @@ -1149,11 +1149,11 @@ public static function dataGetShares(): array { 'sharedBy' => 'initiator', 'owner' => 'currentUser', 'node' => $file1, - 'id' => 442, + 'id' => '442', ]; $file1RoomShareOwnerExpected = [ - 'id' => 442, + 'id' => '442', 'share_type' => IShare::TYPE_ROOM, ]; @@ -1164,11 +1164,11 @@ public static function dataGetShares(): array { 'owner' => 'currentUser', 'expirationDate' => new \DateTime('2000-01-01T01:02:03'), 'node' => $file1, - 'id' => 815, + 'id' => '815', ]; $file1RemoteShareOwnerExpected = [ - 'id' => 815, + 'id' => '815', 'share_type' => IShare::TYPE_REMOTE, ]; @@ -1179,11 +1179,11 @@ public static function dataGetShares(): array { 'owner' => 'currentUser', 'expirationDate' => new \DateTime('2000-01-01T01:02:03'), 'node' => $file1, - 'id' => 816, + 'id' => '816', ]; $file1RemoteGroupShareOwnerExpected = [ - 'id' => 816, + 'id' => '816', 'share_type' => IShare::TYPE_REMOTE_GROUP, ]; @@ -1193,11 +1193,11 @@ public static function dataGetShares(): array { 'sharedBy' => 'initiator', 'owner' => 'currentUser', 'node' => $file2, - 'id' => 823, + 'id' => '823', ]; $file2UserShareOwnerExpected = [ - 'id' => 823, + 'id' => '823', 'share_type' => IShare::TYPE_USER, ]; @@ -1540,7 +1540,7 @@ function (array $shareParams): IShare { ->setSharedBy($shareParams['sharedBy']) ->setShareOwner($shareParams['owner']) ->setPermissions(Constants::PERMISSION_READ) - ->setId($shareParams['id']); + ->setId((string)$shareParams['id']); if (isset($shareParams['sharedWith'])) { $share->setSharedWith($shareParams['sharedWith']); } @@ -5338,8 +5338,8 @@ public function testFormatShareWithFederatedShare(bool $isKnownServer, bool $isT 1, IShare::TYPE_REMOTE, 'recipient@remoteserver.com', // shared with - 'sender@testserver.com', // shared by - 'shareOwner', // share owner + 'sender@testserver.com', // shared by + 'shareOwner', // share owner $node, Constants::PERMISSION_READ, time(), diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 8c34578e3b13f..77ad37e7f9b9a 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -4147,14 +4147,6 @@ - - - node]]> - - - - - diff --git a/lib/private/Share20/Share.php b/lib/private/Share20/Share.php index 571efc8c4bef6..eff1e2379e3f6 100644 --- a/lib/private/Share20/Share.php +++ b/lib/private/Share20/Share.php @@ -1,5 +1,7 @@ id !== null) { throw new IllegalIDChangeException('Not allowed to assign a new internal id to a share'); } @@ -103,31 +68,18 @@ public function setId($id) { return $this; } - /** - * @inheritdoc - */ - public function getId() { + public function getId(): ?string { return $this->id; } - /** - * @inheritdoc - */ - public function getFullId() { + public function getFullId(): string { if ($this->providerId === null || $this->id === null) { - throw new \UnexpectedValueException; + throw new \UnexpectedValueException('Provider ID and ID must be set before getting full ID'); } return $this->providerId . ':' . $this->id; } - /** - * @inheritdoc - */ - public function setProviderId($id) { - if (!is_string($id)) { - throw new \InvalidArgumentException('String expected.'); - } - + public function setProviderId(string $id): static { if ($this->providerId !== null) { throw new IllegalIDChangeException('Not allowed to assign a new provider id to a share'); } @@ -136,56 +88,44 @@ public function setProviderId($id) { return $this; } - /** - * @inheritdoc - */ - public function setNode(Node $node) { + public function setNode(Node $node): static { $this->fileId = null; $this->nodeType = null; $this->node = $node; return $this; } - /** - * @inheritdoc - */ - public function getNode() { - if ($this->node === null) { - if ($this->shareOwner === null || $this->fileId === null) { - throw new NotFoundException(); - } - - // for federated shares the owner can be a remote user, in this - // case we use the initiator - if ($this->userManager->userExists($this->shareOwner)) { - $userFolder = $this->rootFolder->getUserFolder($this->shareOwner); - } else { - $userFolder = $this->rootFolder->getUserFolder($this->sharedBy); - } - - $node = $userFolder->getFirstNodeById($this->fileId); - if (!$node) { - throw new NotFoundException('Node for share not found, fileid: ' . $this->fileId); - } - - $this->node = $node; + public function getNode(): Node { + if ($this->node !== null) { + return $this->node; } + if ($this->shareOwner === null || $this->fileId === null) { + throw new NotFoundException('Share owner and file ID must be set'); + } + + // For federated shares the owner can be a remote user, in this case we use the initiator + $owner = $this->userManager->userExists($this->shareOwner) + ? $this->shareOwner + : $this->sharedBy; + + $userFolder = $this->rootFolder->getUserFolder($owner); + $node = $userFolder->getFirstNodeById($this->fileId); + + if (!$node) { + throw new NotFoundException('Node for share not found, fileid: ' . $this->fileId); + } + + $this->node = $node; return $this->node; } - /** - * @inheritdoc - */ - public function setNodeId($fileId) { + public function setNodeId(int $fileId): static { $this->node = null; $this->fileId = $fileId; return $this; } - /** - * @inheritdoc - */ public function getNodeId(): int { if ($this->fileId === null) { $this->fileId = $this->getNode()->getId(); @@ -193,339 +133,198 @@ public function getNodeId(): int { if ($this->fileId === null) { throw new NotFoundException('Share source not found'); - } else { - return $this->fileId; } + + return $this->fileId; } - /** - * @inheritdoc - */ - public function setNodeType($type) { + public function setNodeType(string $type): static { if ($type !== 'file' && $type !== 'folder') { - throw new \InvalidArgumentException(); + throw new \InvalidArgumentException('Node type must be "file" or "folder"'); } $this->nodeType = $type; return $this; } - /** - * @inheritdoc - */ - public function getNodeType() { - if ($this->nodeType === null) { - if ($this->getNodeCacheEntry()) { - $info = $this->getNodeCacheEntry(); - $this->nodeType = $info->getMimeType() === FileInfo::MIMETYPE_FOLDER ? 'folder' : 'file'; - } else { - $node = $this->getNode(); - $this->nodeType = $node instanceof File ? 'file' : 'folder'; - } + public function getNodeType(): string { + if ($this->nodeType !== null) { + return $this->nodeType; + } + + if ($cacheEntry = $this->getNodeCacheEntry()) { + $this->nodeType = $cacheEntry->getMimeType() === FileInfo::MIMETYPE_FOLDER ? 'folder' : 'file'; + } else { + $node = $this->getNode(); + $this->nodeType = $node instanceof File ? 'file' : 'folder'; } return $this->nodeType; } - /** - * @inheritdoc - */ - public function setShareType($shareType) { + public function setShareType(int $shareType): static { $this->shareType = $shareType; return $this; } - /** - * @inheritdoc - */ - public function getShareType() { + public function getShareType(): ?int { return $this->shareType; } - /** - * @inheritdoc - */ - public function setSharedWith($sharedWith) { - if (!is_string($sharedWith)) { - throw new \InvalidArgumentException(); - } + public function setSharedWith(string $sharedWith): static { $this->sharedWith = $sharedWith; return $this; } - /** - * @inheritdoc - */ - public function getSharedWith() { + public function getSharedWith(): ?string { return $this->sharedWith; } - /** - * @inheritdoc - */ - public function setSharedWithDisplayName($displayName) { - if (!is_string($displayName)) { - throw new \InvalidArgumentException(); - } + public function setSharedWithDisplayName(string $displayName): static { $this->sharedWithDisplayName = $displayName; return $this; } - /** - * @inheritdoc - */ - public function getSharedWithDisplayName() { + public function getSharedWithDisplayName(): ?string { return $this->sharedWithDisplayName; } - /** - * @inheritdoc - */ - public function setSharedWithAvatar($src) { - if (!is_string($src)) { - throw new \InvalidArgumentException(); - } + public function setSharedWithAvatar(string $src): static { $this->sharedWithAvatar = $src; return $this; } - /** - * @inheritdoc - */ - public function getSharedWithAvatar() { + public function getSharedWithAvatar(): ?string { return $this->sharedWithAvatar; } - /** - * @inheritdoc - */ - public function setPermissions($permissions) { - //TODO checks - + public function setPermissions(int $permissions): static { $this->permissions = $permissions; return $this; } - /** - * @inheritdoc - */ - public function getPermissions() { + public function getPermissions(): ?int { return $this->permissions; } - /** - * @inheritdoc - */ public function newAttributes(): IAttributes { return new ShareAttributes(); } - /** - * @inheritdoc - */ - public function setAttributes(?IAttributes $attributes) { + public function setAttributes(?IAttributes $attributes): static { $this->attributes = $attributes; return $this; } - /** - * @inheritdoc - */ public function getAttributes(): ?IAttributes { return $this->attributes; } - /** - * @inheritdoc - */ - public function setStatus(int $status): IShare { + public function setStatus(int $status): static { $this->status = $status; return $this; } - /** - * @inheritdoc - */ - public function getStatus(): int { + public function getStatus(): ?int { return $this->status; } - /** - * @inheritdoc - */ - public function setNote($note) { + public function setNote(string $note): static { $this->note = $note; return $this; } - /** - * @inheritdoc - */ - public function getNote() { - if (is_string($this->note)) { - return $this->note; - } - return ''; + public function getNote(): string { + return $this->note; } - /** - * @inheritdoc - */ - public function setLabel($label) { + public function setLabel(string $label): static { $this->label = $label; return $this; } - /** - * @inheritdoc - */ - public function getLabel() { + public function getLabel(): string { return $this->label; } - /** - * @inheritdoc - */ - public function setExpirationDate($expireDate) { - //TODO checks - + public function setExpirationDate(?\DateTimeInterface $expireDate): static { $this->expireDate = $expireDate; return $this; } - /** - * @inheritdoc - */ - public function getExpirationDate() { + public function getExpirationDate(): ?\DateTimeInterface { return $this->expireDate; } - /** - * @inheritdoc - */ - public function setNoExpirationDate(bool $noExpirationDate) { + public function setNoExpirationDate(bool $noExpirationDate): static { $this->noExpirationDate = $noExpirationDate; return $this; } - /** - * @inheritdoc - */ public function getNoExpirationDate(): bool { return $this->noExpirationDate; } - /** - * @inheritdoc - */ - public function isExpired() { - return $this->getExpirationDate() !== null - && $this->getExpirationDate() <= new \DateTime(); + public function isExpired(): bool { + $expirationDate = $this->getExpirationDate(); + return $expirationDate !== null && $expirationDate <= new \DateTime(); } - /** - * @inheritdoc - */ - public function setSharedBy($sharedBy) { - if (!is_string($sharedBy)) { - throw new \InvalidArgumentException(); - } - //TODO checks + public function setSharedBy(string $sharedBy): static { $this->sharedBy = $sharedBy; - return $this; } - /** - * @inheritdoc - */ - public function getSharedBy() { - //TODO check if set + public function getSharedBy(): ?string { return $this->sharedBy; } - /** - * @inheritdoc - */ - public function setShareOwner($shareOwner) { - if (!is_string($shareOwner)) { - throw new \InvalidArgumentException(); - } - //TODO checks - + public function setShareOwner(string $shareOwner): static { $this->shareOwner = $shareOwner; return $this; } - /** - * @inheritdoc - */ - public function getShareOwner() { - //TODO check if set + public function getShareOwner(): ?string { return $this->shareOwner; } - /** - * @inheritdoc - */ - public function setPassword($password) { + public function setPassword(?string $password): static { $this->password = $password; return $this; } - /** - * @inheritdoc - */ - public function getPassword() { + public function getPassword(): ?string { return $this->password; } - /** - * @inheritdoc - */ - public function setPasswordExpirationTime(?\DateTimeInterface $passwordExpirationTime = null): IShare { + public function setPasswordExpirationTime(?\DateTimeInterface $passwordExpirationTime = null): static { $this->passwordExpirationTime = $passwordExpirationTime; return $this; } - /** - * @inheritdoc - */ public function getPasswordExpirationTime(): ?\DateTimeInterface { return $this->passwordExpirationTime; } - /** - * @inheritdoc - */ - public function setSendPasswordByTalk(bool $sendPasswordByTalk) { + public function setSendPasswordByTalk(bool $sendPasswordByTalk): static { $this->sendPasswordByTalk = $sendPasswordByTalk; return $this; } - /** - * @inheritdoc - */ public function getSendPasswordByTalk(): bool { return $this->sendPasswordByTalk; } - /** - * @inheritdoc - */ - public function setToken($token) { + public function setToken(?string $token): static { $this->token = $token; return $this; } - /** - * @inheritdoc - */ - public function getToken() { + public function getToken(): ?string { return $this->token; } - public function setParent(int $parent): self { + public function setParent(int $parent): static { $this->parent = $parent; return $this; } @@ -534,66 +333,43 @@ public function getParent(): ?int { return $this->parent; } - /** - * @inheritdoc - */ - public function setTarget($target) { + public function setTarget(string $target): static { $this->target = $target; return $this; } - /** - * @inheritdoc - */ - public function getTarget() { + public function getTarget(): ?string { return $this->target; } - /** - * @inheritdoc - */ - public function setShareTime(\DateTime $shareTime) { + public function setShareTime(\DateTimeInterface $shareTime): static { $this->shareTime = $shareTime; return $this; } - /** - * @inheritdoc - */ - public function getShareTime() { + public function getShareTime(): ?\DateTimeInterface { return $this->shareTime; } - /** - * @inheritdoc - */ - public function setMailSend($mailSend) { + public function setMailSend(bool $mailSend): static { $this->mailSend = $mailSend; return $this; } - /** - * @inheritdoc - */ - public function getMailSend() { + public function getMailSend(): ?bool { return $this->mailSend; } - /** - * @inheritdoc - */ - public function setNodeCacheEntry(ICacheEntry $entry) { + public function setNodeCacheEntry(ICacheEntry $entry): static { $this->nodeCacheEntry = $entry; + return $this; } - /** - * @inheritdoc - */ - public function getNodeCacheEntry() { + public function getNodeCacheEntry(): ?ICacheEntry { return $this->nodeCacheEntry; } - public function setHideDownload(bool $hide): IShare { + public function setHideDownload(bool $hide): static { $this->hideDownload = $hide; return $this; } @@ -602,7 +378,7 @@ public function getHideDownload(): bool { return $this->hideDownload; } - public function setReminderSent(bool $reminderSent): IShare { + public function setReminderSent(bool $reminderSent): static { $this->reminderSent = $reminderSent; return $this; } @@ -615,13 +391,10 @@ public function canSeeContent(): bool { $shareManager = Server::get(IManager::class); $allowViewWithoutDownload = $shareManager->allowViewWithoutDownload(); - // If the share manager allows viewing without download, we can always see the content. if ($allowViewWithoutDownload) { return true; } - // No "allow preview" header set, so we must check if - // the share has not explicitly disabled download permissions $attributes = $this->getAttributes(); if ($attributes?->getAttribute('permissions', 'download') === false) { return false; diff --git a/lib/public/Share/IShare.php b/lib/public/Share/IShare.php index d3c0873a3afea..065f9d5ae38d5 100644 --- a/lib/public/Share/IShare.php +++ b/lib/public/Share/IShare.php @@ -1,5 +1,7 @@ :. @@ -146,46 +141,44 @@ public function getId(); * @since 9.0.0 * @throws \UnexpectedValueException If the fullId could not be constructed */ - public function getFullId(); + public function getFullId(): string; /** * Set the provider id of the share * It is only allowed to set the provider id of a share once. * Attempts to override the provider id will result in an IllegalIDChangeException * - * @param string $id - * @return \OCP\Share\IShare + * @return IShare The modified object * @throws IllegalIDChangeException - * @throws \InvalidArgumentException * @since 9.1.0 */ - public function setProviderId($id); + public function setProviderId(string $id): IShare; /** * Set the node of the file/folder that is shared * * @param Node $node - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 9.0.0 */ - public function setNode(Node $node); + public function setNode(Node $node): IShare; /** * Get the node of the file/folder that is shared * - * @return File|Folder + * @return Node * @since 9.0.0 * @throws NotFoundException */ - public function getNode(); + public function getNode(): Node; /** * Set file id for lazy evaluation of the node * @param int $fileId - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 9.0.0 */ - public function setNodeId($fileId); + public function setNodeId(int $fileId): IShare; /** * Get the fileid of the node of this share @@ -199,87 +192,86 @@ public function getNodeId(): int; * Set the type of node (file/folder) * * @param string $type - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 9.0.0 */ - public function setNodeType($type); + public function setNodeType(string $type): IShare; /** * Get the type of node (file/folder) * * @return string * @since 9.0.0 - * @throws NotFoundException */ - public function getNodeType(); + public function getNodeType(): string; /** * Set the shareType * * @param int $shareType - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 9.0.0 */ - public function setShareType($shareType); + public function setShareType(int $shareType): IShare; /** * Get the shareType * - * @return int + * @return int|null * @since 9.0.0 */ - public function getShareType(); + public function getShareType(): ?int; /** * Set the receiver of this share. * * @param string $sharedWith - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 9.0.0 */ - public function setSharedWith($sharedWith); + public function setSharedWith(string $sharedWith): IShare; /** * Get the receiver of this share. * - * @return string + * @return string|null * @since 9.0.0 */ - public function getSharedWith(); + public function getSharedWith(): ?string; /** * Set the display name of the receiver of this share. * * @param string $displayName - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 14.0.0 */ - public function setSharedWithDisplayName($displayName); + public function setSharedWithDisplayName(string $displayName): IShare; /** * Get the display name of the receiver of this share. * - * @return string + * @return string|null * @since 14.0.0 */ - public function getSharedWithDisplayName(); + public function getSharedWithDisplayName(): ?string; /** * Set the avatar of the receiver of this share. * * @param string $src - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 14.0.0 */ - public function setSharedWithAvatar($src); + public function setSharedWithAvatar(string $src): IShare; /** * Get the avatar of the receiver of this share. * - * @return string + * @return string|null * @since 14.0.0 */ - public function getSharedWithAvatar(); + public function getSharedWithAvatar(): ?string; /** * Set the permissions. @@ -289,16 +281,16 @@ public function getSharedWithAvatar(); * @return IShare The modified object * @since 9.0.0 */ - public function setPermissions($permissions); + public function setPermissions(int $permissions): IShare; /** * Get the share permissions * See \OCP\Constants::PERMISSION_* * - * @return int + * @return int|null * @since 9.0.0 */ - public function getPermissions(); + public function getPermissions(): ?int; /** * Create share attributes object @@ -311,17 +303,17 @@ public function newAttributes(): IAttributes; /** * Set share attributes * - * @param ?IAttributes $attributes + * @param IAttributes|null $attributes * @since 25.0.0 * @return IShare The modified object */ - public function setAttributes(?IAttributes $attributes); + public function setAttributes(?IAttributes $attributes): IShare; /** * Get share attributes * * @since 25.0.0 - * @return ?IAttributes + * @return IAttributes|null */ public function getAttributes(): ?IAttributes; @@ -339,19 +331,19 @@ public function setStatus(int $status): IShare; * Get the accepted status * See self::STATUS_* * - * @return int + * @return int|null * @since 18.0.0 */ - public function getStatus(): int; + public function getStatus(): ?int; /** * Attach a note to a share * * @param string $note - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 14.0.0 */ - public function setNote($note); + public function setNote(string $note): IShare; /** * Get note attached to a share @@ -359,35 +351,33 @@ public function setNote($note); * @return string * @since 14.0.0 */ - public function getNote(); - + public function getNote(): string; /** * Set the expiration date * - * @param \DateTime|null $expireDate - * @return \OCP\Share\IShare The modified object + * @param \DateTimeInterface|null $expireDate + * @return IShare The modified object * @since 9.0.0 */ - public function setExpirationDate(?\DateTime $expireDate); + public function setExpirationDate(?\DateTimeInterface $expireDate): IShare; /** * Get the expiration date * - * @return \DateTime|null + * @return \DateTimeInterface|null * @since 9.0.0 */ - public function getExpirationDate(); + public function getExpirationDate(): ?\DateTimeInterface; /** * Set overwrite flag for falsy expiry date values * * @param bool $noExpirationDate - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 30.0.0 */ - public function setNoExpirationDate(bool $noExpirationDate); - + public function setNoExpirationDate(bool $noExpirationDate): IShare; /** * Get value of overwrite falsy expiry date flag @@ -395,24 +385,24 @@ public function setNoExpirationDate(bool $noExpirationDate); * @return bool * @since 30.0.0 */ - public function getNoExpirationDate(); + public function getNoExpirationDate(): bool; /** * Is the share expired ? * - * @return boolean + * @return bool * @since 18.0.0 */ - public function isExpired(); + public function isExpired(): bool; /** * set a label for a share, some shares, e.g. public links can have a label * * @param string $label - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 15.0.0 */ - public function setLabel($label); + public function setLabel(string $label): IShare; /** * get label for the share, some shares, e.g. public links can have a label @@ -420,41 +410,41 @@ public function setLabel($label); * @return string * @since 15.0.0 */ - public function getLabel(); + public function getLabel(): string; /** * Set the sharer of the path. * * @param string $sharedBy - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 9.0.0 */ - public function setSharedBy($sharedBy); + public function setSharedBy(string $sharedBy): IShare; /** * Get share sharer * - * @return string + * @return string|null * @since 9.0.0 */ - public function getSharedBy(); + public function getSharedBy(): ?string; /** * Set the original share owner (who owns the path that is shared) * * @param string $shareOwner - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 9.0.0 */ - public function setShareOwner($shareOwner); + public function setShareOwner(string $shareOwner): IShare; /** * Get the original share owner (who owns the path that is shared) * - * @return string + * @return string|null * @since 9.0.0 */ - public function getShareOwner(); + public function getShareOwner(): ?string; /** * Set the password for this share. @@ -462,10 +452,10 @@ public function getShareOwner(); * or updated the password will be hashed. * * @param string|null $password - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 9.0.0 */ - public function setPassword($password); + public function setPassword(?string $password): IShare; /** * Get the password of this share. @@ -475,18 +465,21 @@ public function setPassword($password); * @return string|null * @since 9.0.0 */ - public function getPassword(); + public function getPassword(): ?string; /** * Set the password's expiration time of this share. * - * @return self The modified object + * @param \DateTimeInterface|null $passwordExpirationTime + * @return IShare The modified object * @since 24.0.0 */ public function setPasswordExpirationTime(?\DateTimeInterface $passwordExpirationTime = null): IShare; /** * Get the password's expiration time of this share. + * + * @return \DateTimeInterface|null * @since 24.0.0 */ public function getPasswordExpirationTime(): ?\DateTimeInterface; @@ -496,10 +489,10 @@ public function getPasswordExpirationTime(): ?\DateTimeInterface; * password using Nextcloud Talk. * * @param bool $sendPasswordByTalk - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 14.0.0 */ - public function setSendPasswordByTalk(bool $sendPasswordByTalk); + public function setSendPasswordByTalk(bool $sendPasswordByTalk): IShare; /** * Get if the recipient can start a conversation with the owner to get the @@ -516,30 +509,33 @@ public function getSendPasswordByTalk(): bool; /** * Set the public link token. * - * @param string $token - * @return \OCP\Share\IShare The modified object + * @param string|null $token + * @return IShare The modified object * @since 9.0.0 */ - public function setToken($token); + public function setToken(?string $token): IShare; /** * Get the public link token. * - * @return string + * @return string|null * @since 9.0.0 */ - public function getToken(); + public function getToken(): ?string; /** * Set the parent of this share * + * @param int $parent + * @return IShare The modified object * @since 9.0.0 */ - public function setParent(int $parent): self; + public function setParent(int $parent): IShare; /** * Get the parent of this share. * + * @return int|null * @since 9.0.0 */ public function getParent(): ?int; @@ -548,69 +544,69 @@ public function getParent(): ?int; * Set the target path of this share relative to the recipients user folder. * * @param string $target - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 9.0.0 */ - public function setTarget($target); + public function setTarget(string $target): IShare; /** * Get the target path of this share relative to the recipients user folder. * - * @return string + * @return string|null * @since 9.0.0 */ - public function getTarget(); + public function getTarget(): ?string; /** * Set the time this share was created * - * @param \DateTime $shareTime - * @return \OCP\Share\IShare The modified object + * @param \DateTimeInterface $shareTime + * @return IShare The modified object * @since 9.0.0 */ - public function setShareTime(\DateTime $shareTime); + public function setShareTime(\DateTimeInterface $shareTime): IShare; /** * Get the timestamp this share was created * - * @return \DateTime + * @return \DateTimeInterface|null * @since 9.0.0 */ - public function getShareTime(); + public function getShareTime(): ?\DateTimeInterface; /** * Set if the recipient should be informed by mail about the share. * * @param bool $mailSend - * @return \OCP\Share\IShare The modified object + * @return IShare The modified object * @since 9.0.0 */ - public function setMailSend($mailSend); + public function setMailSend(bool $mailSend): IShare; /** * Get if the recipient should be informed by mail about the share. * - * @return bool + * @return bool|null * @since 9.0.0 */ - public function getMailSend(); + public function getMailSend(): ?bool; /** * Set the cache entry for the shared node * * @param ICacheEntry $entry - * @return void + * @return IShare The modified object * @since 11.0.0 */ - public function setNodeCacheEntry(ICacheEntry $entry); + public function setNodeCacheEntry(ICacheEntry $entry): IShare; /** * Get the cache entry for the shared node * - * @return null|ICacheEntry + * @return ICacheEntry|null * @since 11.0.0 */ - public function getNodeCacheEntry(); + public function getNodeCacheEntry(): ?ICacheEntry; /** * Sets a shares hide download state @@ -618,7 +614,7 @@ public function getNodeCacheEntry(); * hide download buttons etc. * * @param bool $hide - * @return IShare + * @return IShare The modified object * @since 15.0.0 */ public function setHideDownload(bool $hide): IShare; @@ -636,7 +632,8 @@ public function getHideDownload(): bool; /** * Sets a flag that stores whether a reminder via email has been sent * - * @return self The modified object + * @param bool $reminderSent + * @return IShare The modified object * @since 31.0.0 */ public function setReminderSent(bool $reminderSent): IShare; @@ -653,6 +650,9 @@ public function getReminderSent(): bool; * Check if the current user can see this share files contents. * This will check the download permissions as well as the global * admin setting to allow viewing files without downloading. + * + * @return bool + * @since 15.0.0 */ public function canSeeContent(): bool; } diff --git a/tests/lib/Share20/DefaultShareProviderTest.php b/tests/lib/Share20/DefaultShareProviderTest.php index 2ceb77aeb5008..9c33f8921797f 100644 --- a/tests/lib/Share20/DefaultShareProviderTest.php +++ b/tests/lib/Share20/DefaultShareProviderTest.php @@ -376,7 +376,7 @@ public function testGetShareByIdUserGroupShare(): void { $group0->method('getDisplayName')->willReturn('g0-displayname'); $node = $this->createMock(Folder::class); - $node->method('getId')->willReturn(42); + $node->method('getId')->willReturn('42'); $node->method('getName')->willReturn('myTarget'); $this->rootFolder->method('getUserFolder')->with('user0')->willReturnSelf(); @@ -1429,7 +1429,7 @@ public function testGetSharesNode(): void { $this->assertEquals(1, $qb->executeStatement()); $file = $this->createMock(File::class); - $file->method('getId')->willReturn(42); + $file->method('getId')->willReturn('42'); $this->rootFolder->method('getUserFolder')->with('shareOwner')->willReturnSelf(); $this->rootFolder->method('getFirstNodeById')->with(42)->willReturn($file); @@ -1479,7 +1479,7 @@ public function testGetSharesReshare(): void { $id2 = $qb->getLastInsertId(); $file = $this->createMock(File::class); - $file->method('getId')->willReturn(42); + $file->method('getId')->willReturn('42'); $this->rootFolder->method('getUserFolder')->with('shareOwner')->willReturnSelf(); $this->rootFolder->method('getFirstNodeById')->with(42)->willReturn($file); @@ -1868,7 +1868,7 @@ function ($userId) use ($users) { ); $file1 = $this->createMock(File::class); - $file1->method('getId')->willReturn(42); + $file1->method('getId')->willReturn('42'); $file2 = $this->createMock(File::class); $file2->method('getId')->willReturn(43); @@ -1925,7 +1925,7 @@ function ($userId) use ($users) { ); $file1 = $this->createMock(File::class); - $file1->method('getId')->willReturn(42); + $file1->method('getId')->willReturn('42'); $file2 = $this->createMock(File::class); $file2->method('getId')->willReturn(43); @@ -1991,7 +1991,7 @@ function ($userId) use ($users) { ); $file1 = $this->createMock(File::class); - $file1->method('getId')->willReturn(42); + $file1->method('getId')->willReturn('42'); $file2 = $this->createMock(File::class); $file2->method('getId')->willReturn(43); @@ -2062,7 +2062,7 @@ function ($groupId) use ($groups) { ); $file1 = $this->createMock(File::class); - $file1->method('getId')->willReturn(42); + $file1->method('getId')->willReturn('42'); $file2 = $this->createMock(File::class); $file2->method('getId')->willReturn(43); @@ -2141,7 +2141,7 @@ function ($groupId) use ($groups) { ); $file1 = $this->createMock(File::class); - $file1->method('getId')->willReturn(42); + $file1->method('getId')->willReturn('42'); $file2 = $this->createMock(File::class); $file2->method('getId')->willReturn(43); @@ -2221,7 +2221,7 @@ public function testMoveUserShare(): void { ]); $file = $this->createMock(File::class); - $file->method('getId')->willReturn(42); + $file->method('getId')->willReturn('42'); $this->rootFolder->method('getUserFolder')->with('user1')->willReturnSelf(); $this->rootFolder->method('getFirstNodeById')->willReturn($file); @@ -2257,7 +2257,7 @@ public function testMoveGroupShare(): void { ]); $folder = $this->createMock(Folder::class); - $folder->method('getId')->willReturn(42); + $folder->method('getId')->willReturn('42'); $this->rootFolder->method('getUserFolder')->with('user1')->willReturnSelf(); $this->rootFolder->method('getFirstNodeById')->willReturn($folder); diff --git a/tests/lib/Share20/ShareTest.php b/tests/lib/Share20/ShareTest.php index f15fbb860dbe4..d021f1b51e3dd 100644 --- a/tests/lib/Share20/ShareTest.php +++ b/tests/lib/Share20/ShareTest.php @@ -1,5 +1,7 @@ share = new Share($this->rootFolder, $this->userManager); } - - public function testSetIdInvalid(): void { - $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage('String expected.'); + public function testSetIdInvalidType(): void { + $this->expectException(\TypeError::class); $this->share->setId(1.2); } - public function testSetIdInt(): void { + public function testSetIdIntType(): void { + $this->expectException(\TypeError::class); + $this->share->setId(42); - $this->assertEquals('42', $this->share->getId()); } - public function testSetIdString(): void { $this->share->setId('foo'); $this->assertEquals('foo', $this->share->getId()); } + public function testSetIdTrimsWhitespace(): void { + $this->share->setId(' foo '); + $this->assertEquals('foo', $this->share->getId()); + } public function testSetIdOnce(): void { $this->expectException(IllegalIDChangeException::class); @@ -62,21 +70,27 @@ public function testSetIdOnce(): void { $this->share->setId('bar'); } + public function testGetIdBeforeSet(): void { + $this->assertNull($this->share->getId()); + } - public function testSetProviderIdInt(): void { - $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage('String expected.'); + public function testSetProviderIdInvalidType(): void { + $this->expectException(\TypeError::class); $this->share->setProviderId(42); } - public function testSetProviderIdString(): void { $this->share->setProviderId('foo'); $this->share->setId('bar'); $this->assertEquals('foo:bar', $this->share->getFullId()); } + public function testSetProviderIdTrimsWhitespace(): void { + $this->share->setProviderId(' foo '); + $this->share->setId('bar'); + $this->assertEquals('foo:bar', $this->share->getFullId()); + } public function testSetProviderIdOnce(): void { $this->expectException(IllegalIDChangeException::class); @@ -85,4 +99,285 @@ public function testSetProviderIdOnce(): void { $this->share->setProviderId('foo'); $this->share->setProviderId('bar'); } + + public function testGetFullIdWithoutProviderId(): void { + $this->expectException(\UnexpectedValueException::class); + + $this->share->setId('bar'); + $this->share->getFullId(); + } + + public function testGetFullIdWithoutId(): void { + $this->expectException(\UnexpectedValueException::class); + + $this->share->setProviderId('foo'); + $this->share->getFullId(); + } + + public function testSetAndGetNode(): void { + $node = $this->createMock(File::class); + + $this->share->setNode($node); + $this->assertSame($node, $this->share->getNode()); + } + + public function testSetNodeResetsFileIdAndType(): void { + $node = $this->createMock(File::class); + + $this->share->setNodeId(42); + $this->share->setNodeType('file'); + $this->share->setNode($node); + + // After setting node, the internal fileId and nodeType should be reset + $this->assertSame($node, $this->share->getNode()); + } + + public function testGetNodeWithoutSet(): void { + $this->expectException(NotFoundException::class); + $this->expectExceptionMessage('Share owner and file ID must be set'); + + $this->share->getNode(); + } + + public function testSetAndGetNodeId(): void { + $this->share->setNodeId(123); + + $node = $this->createMock(File::class); + $node->method('getId')->willReturn(123); + + $this->share->setNode($node); + $this->assertEquals(123, $this->share->getNodeId()); + } + + public function testSetNodeTypeFile(): void { + $this->share->setNodeType('file'); + + $file = $this->createMock(File::class); + $this->share->setNode($file); + + $this->assertEquals('file', $this->share->getNodeType()); + } + + public function testSetNodeTypeFolder(): void { + $this->share->setNodeType('folder'); + + $folder = $this->createMock(Folder::class); + $this->share->setNode($folder); + + $this->assertEquals('folder', $this->share->getNodeType()); + } + + public function testSetNodeTypeInvalid(): void { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Node type must be "file" or "folder"'); + + $this->share->setNodeType('invalid'); + } + + public function testGetNodeTypeFromNode(): void { + $file = $this->createMock(File::class); + $this->share->setNode($file); + + $this->assertEquals('file', $this->share->getNodeType()); + } + + public function testSetAndGetShareType(): void { + $this->share->setShareType(IShare::TYPE_USER); + $this->assertEquals(IShare::TYPE_USER, $this->share->getShareType()); + } + + public function testGetShareTypeBeforeSet(): void { + $this->assertNull($this->share->getShareType()); + } + + public function testSetAndGetSharedWith(): void { + $this->share->setSharedWith('user1'); + $this->assertEquals('user1', $this->share->getSharedWith()); + } + + public function testSetAndGetSharedWithDisplayName(): void { + $this->share->setSharedWithDisplayName('User One'); + $this->assertEquals('User One', $this->share->getSharedWithDisplayName()); + } + + public function testSetAndGetSharedWithAvatar(): void { + $this->share->setSharedWithAvatar('avatar.jpg'); + $this->assertEquals('avatar.jpg', $this->share->getSharedWithAvatar()); + } + + public function testSetAndGetPermissions(): void { + $this->share->setPermissions(19); + $this->assertEquals(19, $this->share->getPermissions()); + } + + public function testSetAndGetStatus(): void { + $this->share->setStatus(IShare::STATUS_ACCEPTED); + $this->assertEquals(IShare::STATUS_ACCEPTED, $this->share->getStatus()); + } + + public function testSetAndGetNote(): void { + $this->share->setNote('Test note'); + $this->assertEquals('Test note', $this->share->getNote()); + } + + public function testGetNoteDefault(): void { + $this->assertEquals('', $this->share->getNote()); + } + + public function testSetAndGetLabel(): void { + $this->share->setLabel('Public link label'); + $this->assertEquals('Public link label', $this->share->getLabel()); + } + + public function testGetLabelDefault(): void { + $this->assertEquals('', $this->share->getLabel()); + } + + public function testSetAndGetExpirationDate(): void { + $date = new \DateTime('2026-12-31'); + $this->share->setExpirationDate($date); + $this->assertEquals($date, $this->share->getExpirationDate()); + } + + public function testSetExpirationDateNull(): void { + $this->share->setExpirationDate(null); + $this->assertNull($this->share->getExpirationDate()); + } + + public function testIsExpiredFalse(): void { + $date = new \DateTime('+1 day'); + $this->share->setExpirationDate($date); + $this->assertFalse($this->share->isExpired()); + } + + public function testIsExpiredTrue(): void { + $date = new \DateTime('-1 day'); + $this->share->setExpirationDate($date); + $this->assertTrue($this->share->isExpired()); + } + + public function testIsExpiredNoDate(): void { + $this->assertFalse($this->share->isExpired()); + } + + public function testSetAndGetNoExpirationDate(): void { + $this->share->setNoExpirationDate(true); + $this->assertTrue($this->share->getNoExpirationDate()); + + $this->share->setNoExpirationDate(false); + $this->assertFalse($this->share->getNoExpirationDate()); + } + + public function testSetAndGetSharedBy(): void { + $this->share->setSharedBy('user2'); + $this->assertEquals('user2', $this->share->getSharedBy()); + } + + public function testSetAndGetShareOwner(): void { + $this->share->setShareOwner('owner'); + $this->assertEquals('owner', $this->share->getShareOwner()); + } + + public function testSetAndGetPassword(): void { + $this->share->setPassword('secret'); + $this->assertEquals('secret', $this->share->getPassword()); + } + + public function testSetPasswordNull(): void { + $this->share->setPassword(null); + $this->assertNull($this->share->getPassword()); + } + + public function testSetAndGetPasswordExpirationTime(): void { + $date = new \DateTime('2026-06-01'); + $this->share->setPasswordExpirationTime($date); + $this->assertEquals($date, $this->share->getPasswordExpirationTime()); + } + + public function testSetAndGetSendPasswordByTalk(): void { + $this->share->setSendPasswordByTalk(true); + $this->assertTrue($this->share->getSendPasswordByTalk()); + + $this->share->setSendPasswordByTalk(false); + $this->assertFalse($this->share->getSendPasswordByTalk()); + } + + public function testGetSendPasswordByTalkDefault(): void { + $this->assertFalse($this->share->getSendPasswordByTalk()); + } + + public function testSetTokenNull(): void { + $this->share->setToken(null); + $this->assertNull($this->share->getToken()); + } + + public function testSetAndGetToken(): void { + $this->share->setToken('abc123'); + $this->assertEquals('abc123', $this->share->getToken()); + } + + public function testSetAndGetParent(): void { + $this->share->setParent(42); + $this->assertEquals(42, $this->share->getParent()); + } + + public function testGetParentBeforeSet(): void { + $this->assertNull($this->share->getParent()); + } + + public function testSetAndGetTarget(): void { + $this->share->setTarget('/shared/folder'); + $this->assertEquals('/shared/folder', $this->share->getTarget()); + } + + public function testSetAndGetShareTime(): void { + $date = new \DateTime('2026-01-01'); + $this->share->setShareTime($date); + $this->assertEquals($date, $this->share->getShareTime()); + } + + public function testSetAndGetMailSend(): void { + $this->share->setMailSend(true); + $this->assertTrue($this->share->getMailSend()); + + $this->share->setMailSend(false); + $this->assertFalse($this->share->getMailSend()); + } + + public function testSetAndGetHideDownload(): void { + $this->share->setHideDownload(true); + $this->assertTrue($this->share->getHideDownload()); + + $this->share->setHideDownload(false); + $this->assertFalse($this->share->getHideDownload()); + } + + public function testGetHideDownloadDefault(): void { + $this->assertFalse($this->share->getHideDownload()); + } + + public function testSetAndGetReminderSent(): void { + $this->share->setReminderSent(true); + $this->assertTrue($this->share->getReminderSent()); + + $this->share->setReminderSent(false); + $this->assertFalse($this->share->getReminderSent()); + } + + public function testGetReminderSentDefault(): void { + $this->assertFalse($this->share->getReminderSent()); + } + + public function testFluentInterface(): void { + $result = $this->share + ->setId('id1') + ->setProviderId('provider1') + ->setShareType(IShare::TYPE_USER) + ->setSharedWith('user1') + ->setPermissions(31); + + $this->assertInstanceOf(Share::class, $result); + $this->assertEquals('id1', $this->share->getId()); + $this->assertEquals('provider1:id1', $this->share->getFullId()); + } }