From 97dbbe4768398324b8398dc252f1ee7176e410f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 13 Mar 2025 15:07:53 +0100 Subject: [PATCH] fix: Fix download activity for folders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove duplicate activity publishing from share controller download, listen to BeforeZipCreatedEvent to publish activity for folders, and cache folders activity to avoid sending activity for each file in the folder. Signed-off-by: Côme Chilliet --- .../files_sharing/lib/AppInfo/Application.php | 1 + .../lib/Controller/ShareController.php | 103 +----------------- .../lib/Listener/BeforeNodeReadListener.php | 79 ++++++++++++-- 3 files changed, 78 insertions(+), 105 deletions(-) diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index 512309d0c55..db1a0e34c4d 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -98,6 +98,7 @@ class Application extends App implements IBootstrap { // Publish activity for public download $context->registerEventListener(BeforeNodeReadEvent::class, BeforeNodeReadListener::class); + $context->registerEventListener(BeforeZipCreatedEvent::class, BeforeNodeReadListener::class); // Handle download events for view only checks $context->registerEventListener(BeforeZipCreatedEvent::class, BeforeZipCreatedListener::class); diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index cfd9628410e..ad8023ba6bb 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -9,7 +9,6 @@ namespace OCA\Files_Sharing\Controller; use OC\Security\CSP\ContentSecurityPolicy; use OCA\DAV\Connector\Sabre\PublicAuth; use OCA\FederatedFileSharing\FederatedShareProvider; -use OCA\Files_Sharing\Activity\Providers\Downloads; use OCA\Files_Sharing\Event\BeforeTemplateRenderedEvent; use OCA\Files_Sharing\Event\ShareLinkAccessedEvent; use OCP\Accounts\IAccountManager; @@ -28,7 +27,6 @@ use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\IRootFolder; -use OCP\Files\Node; use OCP\Files\NotFoundException; use OCP\HintException; use OCP\IConfig; @@ -368,15 +366,9 @@ class ShareController extends AuthPublicShareController { throw new NotFoundException(); } - // Single file share - if ($share->getNode() instanceof File) { - // Single file download - $this->singleFileDownloaded($share, $share->getNode()); - } - // Directory share - else { - /** @var Folder $node */ - $node = $share->getNode(); + $node = $share->getNode(); + if ($node instanceof Folder) { + // Directory share // Try to get the path if ($path !== '') { @@ -391,22 +383,10 @@ class ShareController extends AuthPublicShareController { if ($node instanceof Folder) { if ($files === null || $files === '') { - // The folder is downloaded - $this->singleFileDownloaded($share, $share->getNode()); - } else { - $fileList = json_decode($files); - // in case we get only a single file - if (!is_array($fileList)) { - $fileList = [$fileList]; - } - foreach ($fileList as $file) { - $subNode = $node->get($file); - $this->singleFileDownloaded($share, $subNode); + if ($share->getHideDownload()) { + throw new NotFoundException('Downloading a folder'); } } - } else { - // Single file download - $this->singleFileDownloaded($share, $share->getNode()); } } @@ -419,77 +399,4 @@ class ShareController extends AuthPublicShareController { } return new RedirectResponse($this->urlGenerator->getAbsoluteURL($davUrl)); } - - /** - * create activity if a single file was downloaded from a link share - * - * @param Share\IShare $share - * @throws NotFoundException when trying to download a folder of a "hide download" share - */ - protected function singleFileDownloaded(IShare $share, Node $node) { - if ($share->getHideDownload() && $node instanceof Folder) { - throw new NotFoundException('Downloading a folder'); - } - - $fileId = $node->getId(); - - $userFolder = $this->rootFolder->getUserFolder($share->getSharedBy()); - $userNode = $userFolder->getFirstNodeById($fileId); - $ownerFolder = $this->rootFolder->getUserFolder($share->getShareOwner()); - $userPath = $userFolder->getRelativePath($userNode->getPath()); - $ownerPath = $ownerFolder->getRelativePath($node->getPath()); - $remoteAddress = $this->request->getRemoteAddress(); - $dateTime = new \DateTime(); - $dateTime = $dateTime->format('Y-m-d H'); - $remoteAddressHash = md5($dateTime . '-' . $remoteAddress); - - $parameters = [$userPath]; - - if ($share->getShareType() === IShare::TYPE_EMAIL) { - if ($node instanceof File) { - $subject = Downloads::SUBJECT_SHARED_FILE_BY_EMAIL_DOWNLOADED; - } else { - $subject = Downloads::SUBJECT_SHARED_FOLDER_BY_EMAIL_DOWNLOADED; - } - $parameters[] = $share->getSharedWith(); - } else { - if ($node instanceof File) { - $subject = Downloads::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED; - $parameters[] = $remoteAddressHash; - } else { - $subject = Downloads::SUBJECT_PUBLIC_SHARED_FOLDER_DOWNLOADED; - $parameters[] = $remoteAddressHash; - } - } - - $this->publishActivity($subject, $parameters, $share->getSharedBy(), $fileId, $userPath); - - if ($share->getShareOwner() !== $share->getSharedBy()) { - $parameters[0] = $ownerPath; - $this->publishActivity($subject, $parameters, $share->getShareOwner(), $fileId, $ownerPath); - } - } - - /** - * publish activity - * - * @param string $subject - * @param array $parameters - * @param string $affectedUser - * @param int $fileId - * @param string $filePath - */ - protected function publishActivity($subject, - array $parameters, - $affectedUser, - $fileId, - $filePath) { - $event = $this->activityManager->generateEvent(); - $event->setApp('files_sharing') - ->setType('public_links') - ->setSubject($subject, $parameters) - ->setAffectedUser($affectedUser) - ->setObject('files', $fileId, $filePath); - $this->activityManager->publish($event); - } } diff --git a/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php b/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php index 69f97e2a7c5..087b76554f5 100644 --- a/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php +++ b/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php @@ -12,32 +12,79 @@ namespace OCA\Files_Sharing\Listener; use OCA\Files_Sharing\Activity\Providers\Downloads; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; +use OCP\Files\Events\BeforeZipCreatedEvent; use OCP\Files\Events\Node\BeforeNodeReadEvent; use OCP\Files\File; +use OCP\Files\Folder; use OCP\Files\IRootFolder; +use OCP\Files\NotFoundException; use OCP\Files\Storage\ISharedStorage; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\IRequest; use OCP\IUserSession; use OCP\Share\IShare; /** - * @template-implements IEventListener + * @template-implements IEventListener */ class BeforeNodeReadListener implements IEventListener { + private ICache $cache; public function __construct( private IUserSession $userSession, private IRootFolder $rootFolder, - protected \OCP\Activity\IManager $activityManager, + private \OCP\Activity\IManager $activityManager, private IRequest $request, + ICacheFactory $cacheFactory, ) { + $this->cache = $cacheFactory->createDistributed('files_sharing_activity_events'); } public function handle(Event $event): void { - if (!($event instanceof BeforeNodeReadEvent)) { + if ($event instanceof BeforeZipCreatedEvent) { + $this->handleBeforeZipCreatedEvent($event); + } elseif ($event instanceof BeforeNodeReadEvent) { + $this->handleBeforeNodeReadEvent($event); + } + } + + public function handleBeforeZipCreatedEvent(BeforeZipCreatedEvent $event): void { + $files = $event->getFiles(); + if (count($files) !== 0) { + /* No need to do anything, activity will be triggered for each file in the zip by the BeforeNodeReadEvent */ + return; + } + + $node = $event->getFolder(); + if (!($node instanceof Folder)) { + return; + } + + try { + $storage = $node->getStorage(); + } catch (NotFoundException) { + return; + } + + if (!$storage->instanceOfStorage(ISharedStorage::class)) { return; } + /** @var ISharedStorage $storage */ + $share = $storage->getShare(); + + if (!in_array($share->getShareType(), [IShare::TYPE_EMAIL, IShare::TYPE_LINK])) { + return; + } + + /* Cache that that folder download activity was published */ + $this->cache->set($this->request->getId(), $node->getPath(), 3600); + + $this->singleFileDownloaded($share, $node); + } + + public function handleBeforeNodeReadEvent(BeforeNodeReadEvent $event): void { $node = $event->getNode(); if (!($node instanceof File)) { return; @@ -56,13 +103,23 @@ class BeforeNodeReadListener implements IEventListener { /** @var ISharedStorage $storage */ $share = $storage->getShare(); + if (!in_array($share->getShareType(), [IShare::TYPE_EMAIL, IShare::TYPE_LINK])) { + return; + } + + $path = $this->cache->get($this->request->getId()); + if (is_string($path) && str_starts_with($node->getPath(), $path)) { + /* An activity was published for a containing folder already */ + return; + } + $this->singleFileDownloaded($share, $node); } /** - * create activity if a single file was downloaded from a link share + * create activity if a single file or folder was downloaded from a link share */ - protected function singleFileDownloaded(IShare $share, File $node): void { + protected function singleFileDownloaded(IShare $share, File|Folder $node): void { $fileId = $node->getId(); $userFolder = $this->rootFolder->getUserFolder($share->getSharedBy()); @@ -74,10 +131,18 @@ class BeforeNodeReadListener implements IEventListener { $parameters = [$userPath]; if ($share->getShareType() === IShare::TYPE_EMAIL) { - $subject = Downloads::SUBJECT_SHARED_FILE_BY_EMAIL_DOWNLOADED; + if ($node instanceof File) { + $subject = Downloads::SUBJECT_SHARED_FILE_BY_EMAIL_DOWNLOADED; + } else { + $subject = Downloads::SUBJECT_SHARED_FOLDER_BY_EMAIL_DOWNLOADED; + } $parameters[] = $share->getSharedWith(); } elseif ($share->getShareType() === IShare::TYPE_LINK) { - $subject = Downloads::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED; + if ($node instanceof File) { + $subject = Downloads::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED; + } else { + $subject = Downloads::SUBJECT_PUBLIC_SHARED_FOLDER_DOWNLOADED; + } $remoteAddress = $this->request->getRemoteAddress(); $dateTime = new \DateTime(); $dateTime = $dateTime->format('Y-m-d H');