From 17d3124dd85de474160106241c55970ad973e15c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 17 Aug 2022 13:55:05 +0200 Subject: [PATCH 1/2] Delay getting the node until we know that we use the data Signed-off-by: Joas Schilling --- lib/Chat/Parser/SystemMessage.php | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/lib/Chat/Parser/SystemMessage.php b/lib/Chat/Parser/SystemMessage.php index 51666142d04..9cd88747ff8 100644 --- a/lib/Chat/Parser/SystemMessage.php +++ b/lib/Chat/Parser/SystemMessage.php @@ -570,16 +570,12 @@ public function parseDeletedMessage(Message $chatMessage): void { */ protected function getFileFromShare(Participant $participant, string $shareId): array { $share = $this->shareProvider->getShareById($shareId); - $node = $share->getNode(); - $name = $node->getName(); - $size = $node->getSize(); - $path = $name; if (!$participant->isGuest()) { if ($share->getShareOwner() !== $participant->getAttendee()->getActorId()) { $userFolder = $this->rootFolder->getUserFolder($participant->getAttendee()->getActorId()); if ($userFolder instanceof Node) { - $userNodes = $userFolder->getById($node->getId()); + $userNodes = $userFolder->getById($share->getNodeId()); if (empty($userNodes)) { // FIXME This should be much more sensible, e.g. @@ -587,31 +583,40 @@ protected function getFileFromShare(Participant $participant, string $shareId): // 2. Once per request \OC_Util::tearDownFS(); \OC_Util::setupFS($participant->getAttendee()->getActorId()); - $userNodes = $userFolder->getById($node->getId()); + $userNodes = $userFolder->getById($share->getNodeId()); } if (empty($userNodes)) { throw new NotFoundException('File was not found'); } - /** @var Node $userNode */ - $userNode = reset($userNodes); - $fullPath = $userNode->getPath(); + /** @var Node $node */ + $node = reset($userNodes); + $fullPath = $node->getPath(); $pathSegments = explode('/', $fullPath, 4); - $name = $userNode->getName(); - $size = $userNode->getSize(); - $path = $pathSegments[3] ?? $path; + $name = $node->getName(); + $size = $node->getSize(); + $path = $pathSegments[3] ?? $name; } } else { + $node = $share->getNode(); + $name = $node->getName(); + $size = $node->getSize(); + $fullPath = $node->getPath(); $pathSegments = explode('/', $fullPath, 4); - $path = $pathSegments[3] ?? $path; + $path = $pathSegments[3] ?? $name; } $url = $this->url->linkToRouteAbsolute('files.viewcontroller.showFile', [ 'fileid' => $node->getId(), ]); } else { + $node = $share->getNode(); + $name = $node->getName(); + $size = $node->getSize(); + $path = $name; + $url = $this->url->linkToRouteAbsolute('files_sharing.sharecontroller.showShare', [ 'token' => $share->getToken(), ]); From 35d727bc8566005abae09f558b0b68777b546235 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 18 Aug 2022 13:25:10 +0200 Subject: [PATCH 2/2] Fix unit tests Signed-off-by: Joas Schilling --- tests/php/Chat/Parser/SystemMessageTest.php | 42 +++++++-------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/tests/php/Chat/Parser/SystemMessageTest.php b/tests/php/Chat/Parser/SystemMessageTest.php index 1ffa7d5d109..6a74d2d62ce 100644 --- a/tests/php/Chat/Parser/SystemMessageTest.php +++ b/tests/php/Chat/Parser/SystemMessageTest.php @@ -699,24 +699,10 @@ public function testGetFileFromShareForOwner() { } public function testGetFileFromShareForRecipient() { - $node = $this->createMock(Node::class); - $node->expects($this->exactly(3)) - ->method('getId') - ->willReturn('54'); - $node->expects($this->once()) - ->method('getName') - ->willReturn('name'); - $node->expects($this->atLeastOnce()) - ->method('getMimeType') - ->willReturn('application/octet-stream'); - $node->expects($this->once()) - ->method('getSize') - ->willReturn(65510); - $share = $this->createMock(IShare::class); - $share->expects($this->once()) - ->method('getNode') - ->willReturn($node); + $share->expects($this->any()) + ->method('getNodeId') + ->willReturn(54); $participant = $this->createMock(Participant::class); $participant->expects($this->once()) @@ -736,6 +722,9 @@ public function testGetFileFromShareForRecipient() { ->willReturn($share); $file = $this->createMock(Node::class); + $file->expects($this->any()) + ->method('getId') + ->willReturn('54'); $file->expects($this->once()) ->method('getName') ->willReturn('different'); @@ -745,6 +734,9 @@ public function testGetFileFromShareForRecipient() { $file->expects($this->once()) ->method('getSize') ->willReturn(65515); + $file->expects($this->atLeastOnce()) + ->method('getMimeType') + ->willReturn('application/octet-stream'); $userFolder = $this->createMock(Folder::class); $userFolder->expects($this->once()) @@ -759,7 +751,7 @@ public function testGetFileFromShareForRecipient() { $this->previewManager->expects($this->once()) ->method('isAvailable') - ->with($node) + ->with($file) ->willReturn(false); $this->url->expects($this->once()) @@ -783,18 +775,10 @@ public function testGetFileFromShareForRecipient() { } public function testGetFileFromShareForRecipientThrows() { - $node = $this->createMock(Node::class); - $node->expects($this->exactly(2)) - ->method('getId') - ->willReturn('54'); - $node->expects($this->once()) - ->method('getName') - ->willReturn('name'); - $share = $this->createMock(IShare::class); - $share->expects($this->once()) - ->method('getNode') - ->willReturn($node); + $share->expects($this->any()) + ->method('getNodeId') + ->willReturn(54); $participant = $this->createMock(Participant::class); $participant->expects($this->once())