From 12ce8c0ef08203e5de7669c1d9c93e3f567a3be6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 13 Mar 2025 12:09:38 +0100 Subject: [PATCH 1/9] fix(sharing): Publish activity for download by public link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even if dav endpoint is now used. Signed-off-by: Côme Chilliet --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../files_sharing/lib/AppInfo/Application.php | 5 + .../lib/Listener/BeforeNodeReadListener.php | 116 ++++++++++++++++++ 4 files changed, 123 insertions(+) create mode 100644 apps/files_sharing/lib/Listener/BeforeNodeReadListener.php diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php index b42d1555263..400df9c9771 100644 --- a/apps/files_sharing/composer/composer/autoload_classmap.php +++ b/apps/files_sharing/composer/composer/autoload_classmap.php @@ -59,6 +59,7 @@ return array( 'OCA\\Files_Sharing\\ISharedMountPoint' => $baseDir . '/../lib/ISharedMountPoint.php', 'OCA\\Files_Sharing\\ISharedStorage' => $baseDir . '/../lib/ISharedStorage.php', 'OCA\\Files_Sharing\\Listener\\BeforeDirectFileDownloadListener' => $baseDir . '/../lib/Listener/BeforeDirectFileDownloadListener.php', + 'OCA\\Files_Sharing\\Listener\\BeforeNodeReadListener' => $baseDir . '/../lib/Listener/BeforeNodeReadListener.php', 'OCA\\Files_Sharing\\Listener\\BeforeZipCreatedListener' => $baseDir . '/../lib/Listener/BeforeZipCreatedListener.php', 'OCA\\Files_Sharing\\Listener\\LoadAdditionalListener' => $baseDir . '/../lib/Listener/LoadAdditionalListener.php', 'OCA\\Files_Sharing\\Listener\\LoadPublicFileRequestAuthListener' => $baseDir . '/../lib/Listener/LoadPublicFileRequestAuthListener.php', diff --git a/apps/files_sharing/composer/composer/autoload_static.php b/apps/files_sharing/composer/composer/autoload_static.php index bb364256947..02bb6d08bb5 100644 --- a/apps/files_sharing/composer/composer/autoload_static.php +++ b/apps/files_sharing/composer/composer/autoload_static.php @@ -74,6 +74,7 @@ class ComposerStaticInitFiles_Sharing 'OCA\\Files_Sharing\\ISharedMountPoint' => __DIR__ . '/..' . '/../lib/ISharedMountPoint.php', 'OCA\\Files_Sharing\\ISharedStorage' => __DIR__ . '/..' . '/../lib/ISharedStorage.php', 'OCA\\Files_Sharing\\Listener\\BeforeDirectFileDownloadListener' => __DIR__ . '/..' . '/../lib/Listener/BeforeDirectFileDownloadListener.php', + 'OCA\\Files_Sharing\\Listener\\BeforeNodeReadListener' => __DIR__ . '/..' . '/../lib/Listener/BeforeNodeReadListener.php', 'OCA\\Files_Sharing\\Listener\\BeforeZipCreatedListener' => __DIR__ . '/..' . '/../lib/Listener/BeforeZipCreatedListener.php', 'OCA\\Files_Sharing\\Listener\\LoadAdditionalListener' => __DIR__ . '/..' . '/../lib/Listener/LoadAdditionalListener.php', 'OCA\\Files_Sharing\\Listener\\LoadPublicFileRequestAuthListener' => __DIR__ . '/..' . '/../lib/Listener/LoadPublicFileRequestAuthListener.php', diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index 6c0d5ca0781..512309d0c55 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -16,6 +16,7 @@ use OCA\Files_Sharing\External\Manager; use OCA\Files_Sharing\External\MountProvider as ExternalMountProvider; use OCA\Files_Sharing\Helper; use OCA\Files_Sharing\Listener\BeforeDirectFileDownloadListener; +use OCA\Files_Sharing\Listener\BeforeNodeReadListener; use OCA\Files_Sharing\Listener\BeforeZipCreatedListener; use OCA\Files_Sharing\Listener\LoadAdditionalListener; use OCA\Files_Sharing\Listener\LoadPublicFileRequestAuthListener; @@ -42,6 +43,7 @@ use OCP\Federation\ICloudIdManager; use OCP\Files\Config\IMountProviderCollection; use OCP\Files\Events\BeforeDirectFileDownloadEvent; use OCP\Files\Events\BeforeZipCreatedEvent; +use OCP\Files\Events\Node\BeforeNodeReadEvent; use OCP\Group\Events\GroupChangedEvent; use OCP\Group\Events\GroupDeletedEvent; use OCP\Group\Events\UserAddedEvent; @@ -94,6 +96,9 @@ class Application extends App implements IBootstrap { $context->registerEventListener(ShareCreatedEvent::class, UserShareAcceptanceListener::class); $context->registerEventListener(UserAddedEvent::class, UserAddedToGroupListener::class); + // Publish activity for public download + $context->registerEventListener(BeforeNodeReadEvent::class, BeforeNodeReadListener::class); + // Handle download events for view only checks $context->registerEventListener(BeforeZipCreatedEvent::class, BeforeZipCreatedListener::class); $context->registerEventListener(BeforeDirectFileDownloadEvent::class, BeforeDirectFileDownloadListener::class); diff --git a/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php b/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php new file mode 100644 index 00000000000..69f97e2a7c5 --- /dev/null +++ b/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php @@ -0,0 +1,116 @@ + + */ +class BeforeNodeReadListener implements IEventListener { + + public function __construct( + private IUserSession $userSession, + private IRootFolder $rootFolder, + protected \OCP\Activity\IManager $activityManager, + private IRequest $request, + ) { + } + + public function handle(Event $event): void { + if (!($event instanceof BeforeNodeReadEvent)) { + return; + } + + $node = $event->getNode(); + if (!($node instanceof File)) { + return; + } + + try { + $storage = $node->getStorage(); + } catch (NotFoundException) { + return; + } + + if (!$storage->instanceOfStorage(ISharedStorage::class)) { + return; + } + + /** @var ISharedStorage $storage */ + $share = $storage->getShare(); + + $this->singleFileDownloaded($share, $node); + } + + /** + * create activity if a single file was downloaded from a link share + */ + protected function singleFileDownloaded(IShare $share, File $node): void { + $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()) ?? ''; + + $parameters = [$userPath]; + + if ($share->getShareType() === IShare::TYPE_EMAIL) { + $subject = Downloads::SUBJECT_SHARED_FILE_BY_EMAIL_DOWNLOADED; + $parameters[] = $share->getSharedWith(); + } elseif ($share->getShareType() === IShare::TYPE_LINK) { + $subject = Downloads::SUBJECT_PUBLIC_SHARED_FILE_DOWNLOADED; + $remoteAddress = $this->request->getRemoteAddress(); + $dateTime = new \DateTime(); + $dateTime = $dateTime->format('Y-m-d H'); + $remoteAddressHash = md5($dateTime . '-' . $remoteAddress); + $parameters[] = $remoteAddressHash; + } else { + return; + } + + $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 + */ + protected function publishActivity( + string $subject, + array $parameters, + string $affectedUser, + int $fileId, + string $filePath, + ): void { + $event = $this->activityManager->generateEvent(); + $event->setApp('files_sharing') + ->setType('public_links') + ->setSubject($subject, $parameters) + ->setAffectedUser($affectedUser) + ->setObject('files', $fileId, $filePath); + $this->activityManager->publish($event); + } +} From a39bee57d95ac89ac248f7cb28f49c4ca9c749dc 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 2/9] 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'); From a3c531c31e4915a230b2d4f2374b2aeee1fb1724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 13 Mar 2025 16:37:16 +0100 Subject: [PATCH 3/9] fix: Avoid triggering several activities for Range request on the same file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoids flooding activities when someone browse a video in the web player. Signed-off-by: Côme Chilliet --- .../lib/Listener/BeforeNodeReadListener.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php b/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php index 087b76554f5..a4f87467ce0 100644 --- a/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php +++ b/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php @@ -22,7 +22,7 @@ use OCP\Files\Storage\ISharedStorage; use OCP\ICache; use OCP\ICacheFactory; use OCP\IRequest; -use OCP\IUserSession; +use OCP\ISession; use OCP\Share\IShare; /** @@ -32,7 +32,7 @@ class BeforeNodeReadListener implements IEventListener { private ICache $cache; public function __construct( - private IUserSession $userSession, + private ISession $session, private IRootFolder $rootFolder, private \OCP\Activity\IManager $activityManager, private IRequest $request, @@ -113,6 +113,14 @@ class BeforeNodeReadListener implements IEventListener { return; } + /* Avoid publishing several activities for one video playing */ + $cacheKey = $node->getId() . $node->getPath(); + if (($this->request->getHeader('range') !== '') && ($this->cache->get($cacheKey) == $this->session->getId())) { + /* This is a range request and an activity for the same file was published in the same session */ + return; + } + $this->cache->set($cacheKey, $this->session->getId(), 3600); + $this->singleFileDownloaded($share, $node); } From ec5ac0957a33b3e6e8d21bcf6335e5096ec9fe00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 13 Mar 2025 17:10:06 +0100 Subject: [PATCH 4/9] fix(files_sharing): Set higher priority for listeners that may abort the request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/files_sharing/lib/AppInfo/Application.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index db1a0e34c4d..4c47d3fc2c0 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -100,9 +100,9 @@ class Application extends App implements IBootstrap { $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); - $context->registerEventListener(BeforeDirectFileDownloadEvent::class, BeforeDirectFileDownloadListener::class); + // Handle download events for view only checks. Priority higher than 0 to run early. + $context->registerEventListener(BeforeZipCreatedEvent::class, BeforeZipCreatedListener::class, 5); + $context->registerEventListener(BeforeDirectFileDownloadEvent::class, BeforeDirectFileDownloadListener::class, 5); // File request auth $context->registerEventListener(BeforeTemplateRenderedEvent::class, LoadPublicFileRequestAuthListener::class); From a32875d4026b496f3876dac1e8a4f860a83fc3ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 13 Mar 2025 17:11:40 +0100 Subject: [PATCH 5/9] fix(files_sharing): Use session id as part of cache key to avoid concurrency issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If several people are watching and seeking the same video file we do not want the cache key to be the same or it would flood activity again. Signed-off-by: Côme Chilliet --- apps/files_sharing/lib/Listener/BeforeNodeReadListener.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php b/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php index a4f87467ce0..d19bc8dfae9 100644 --- a/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php +++ b/apps/files_sharing/lib/Listener/BeforeNodeReadListener.php @@ -114,12 +114,12 @@ class BeforeNodeReadListener implements IEventListener { } /* Avoid publishing several activities for one video playing */ - $cacheKey = $node->getId() . $node->getPath(); - if (($this->request->getHeader('range') !== '') && ($this->cache->get($cacheKey) == $this->session->getId())) { + $cacheKey = $node->getId() . $node->getPath() . $this->session->getId(); + if (($this->request->getHeader('range') !== '') && ($this->cache->get($cacheKey) === 'true')) { /* This is a range request and an activity for the same file was published in the same session */ return; } - $this->cache->set($cacheKey, $this->session->getId(), 3600); + $this->cache->set($cacheKey, 'true', 3600); $this->singleFileDownloaded($share, $node); } From a57f694e8d5b49123ca29f5dd17d1d1a59206adc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 18 Mar 2025 11:56:36 +0100 Subject: [PATCH 6/9] feat: add integration test for sharing activity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .github/workflows/integration-sqlite.yml | 9 +++++ .../sharing_features/sharing-activity.feature | 36 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 build/integration/sharing_features/sharing-activity.feature diff --git a/.github/workflows/integration-sqlite.yml b/.github/workflows/integration-sqlite.yml index ac5db51a759..ea4b46bb063 100644 --- a/.github/workflows/integration-sqlite.yml +++ b/.github/workflows/integration-sqlite.yml @@ -73,6 +73,7 @@ jobs: php-versions: ['8.1'] spreed-versions: ['main'] + activity-versions: ['master'] services: redis: @@ -104,6 +105,14 @@ jobs: path: apps/spreed ref: ${{ matrix.spreed-versions }} + - name: Checkout Activity app + if: ${{ matrix.test-suite == 'sharing_features' }} + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + repository: nextcloud/activity + path: apps/activity + ref: ${{ matrix.activity-versions }} + - name: Set up php ${{ matrix.php-versions }} uses: shivammathur/setup-php@9e72090525849c5e82e596468b86eb55e9cc5401 #v2.32.0 with: diff --git a/build/integration/sharing_features/sharing-activity.feature b/build/integration/sharing_features/sharing-activity.feature new file mode 100644 index 00000000000..67f2d9fd79b --- /dev/null +++ b/build/integration/sharing_features/sharing-activity.feature @@ -0,0 +1,36 @@ +# SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors +# SPDX-License-Identifier: AGPL-3.0-or-later +Feature: sharing + Background: + Given using api version "1" + Given using new dav path + Given invoking occ with "app:enable --force activity" + Given the command was successful + Given user "user0" exists + And Logging in using web as "user0" + And Sending a "POST" to "/apps/activity/settings" with requesttoken + | public_links_notification | 1 | + | public_links_upload_email | 1 | + | notify_setting_batchtime | 0 | + | activity_digest | 0 | + + Scenario: Creating a new mail share + Given dummy mail server is listening + And As an "user0" + When creating a share with + | path | welcome.txt | + | shareType | 4 | + | shareWith | dumy@test.com | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + And last share can be downloaded + + Scenario: Creating a new public share + Given user "user0" exists + And As an "user0" + When creating a share with + | path | welcome.txt | + | shareType | 3 | + Then the OCS status code should be "100" + And the HTTP status code should be "200" + And last link share can be downloaded From 140aba1f16a10a9823d628981a35472d55428114 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 18 Mar 2025 15:16:45 +0100 Subject: [PATCH 7/9] feat: Add context and test steps for activity in sharing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../features/bootstrap/Activity.php | 30 +++++++++++++++++++ .../features/bootstrap/BasicStructure.php | 4 +-- .../features/bootstrap/SharingContext.php | 1 + .../sharing_features/sharing-activity.feature | 14 +++++++-- 4 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 build/integration/features/bootstrap/Activity.php diff --git a/build/integration/features/bootstrap/Activity.php b/build/integration/features/bootstrap/Activity.php new file mode 100644 index 00000000000..466b2dde14e --- /dev/null +++ b/build/integration/features/bootstrap/Activity.php @@ -0,0 +1,30 @@ +sendRequestForJSON('GET', '/apps/activity/api/v2/activity'); + $this->theHTTPStatusCodeShouldBe('200'); + $data = json_decode($this->response->getBody()->getContents(), true); + $activities = $data['ocs']['data']; + $lastActivity = array_pop($activities); + foreach ($activity->getRowsHash() as $key => $value) { + Assert::assertEquals($value, $lastActivity[$key]); + } + } +} diff --git a/build/integration/features/bootstrap/BasicStructure.php b/build/integration/features/bootstrap/BasicStructure.php index 57d18757212..a8c232d6fe7 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -192,8 +192,8 @@ trait BasicStructure { $options = []; if ($this->currentUser === 'admin') { $options['auth'] = ['admin', 'admin']; - } elseif (strpos($this->currentUser, 'guest') !== 0) { - $options['auth'] = [$this->currentUser, self::TEST_PASSWORD]; + } elseif (strpos($this->currentUser, 'anonymous') !== 0) { + $options['auth'] = [$this->currentUser, $this->regularUser]; } if ($body instanceof TableNode) { $fd = $body->getRowsHash(); diff --git a/build/integration/features/bootstrap/SharingContext.php b/build/integration/features/bootstrap/SharingContext.php index d2f1a2446ae..8ef617adbfb 100644 --- a/build/integration/features/bootstrap/SharingContext.php +++ b/build/integration/features/bootstrap/SharingContext.php @@ -17,6 +17,7 @@ class SharingContext implements Context, SnippetAcceptingContext { use Trashbin; use AppConfiguration; use CommandLine; + use Activity; protected function resetAppConfigs() { $this->deleteServerConfig('core', 'shareapi_default_permissions'); diff --git a/build/integration/sharing_features/sharing-activity.feature b/build/integration/sharing_features/sharing-activity.feature index 67f2d9fd79b..5757b210134 100644 --- a/build/integration/sharing_features/sharing-activity.feature +++ b/build/integration/sharing_features/sharing-activity.feature @@ -14,7 +14,7 @@ Feature: sharing | notify_setting_batchtime | 0 | | activity_digest | 0 | - Scenario: Creating a new mail share + Scenario: Creating a new mail share and check activity Given dummy mail server is listening And As an "user0" When creating a share with @@ -24,8 +24,13 @@ Feature: sharing Then the OCS status code should be "100" And the HTTP status code should be "200" And last share can be downloaded + Then last activity should be + | app | files_sharing | + | type | public_links | + | object_type | files | + | object_name | /welcome.txt | - Scenario: Creating a new public share + Scenario: Creating a new public share and check activity Given user "user0" exists And As an "user0" When creating a share with @@ -34,3 +39,8 @@ Feature: sharing Then the OCS status code should be "100" And the HTTP status code should be "200" And last link share can be downloaded + Then last activity should be + | app | files_sharing | + | type | public_links | + | object_type | files | + | object_name | /welcome.txt | From c9938e54a69eec64c56140501f03e75820d51568 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 18 Mar 2025 15:47:29 +0100 Subject: [PATCH 8/9] fix(sharebymail): Fix activity rich object id type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes "Object for placeholder file is invalid, value 21 for key id is not a string" Exception spotted in tests logs. Signed-off-by: Côme Chilliet --- apps/sharebymail/lib/Activity.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/sharebymail/lib/Activity.php b/apps/sharebymail/lib/Activity.php index ae3407c1e2b..7e9389da060 100644 --- a/apps/sharebymail/lib/Activity.php +++ b/apps/sharebymail/lib/Activity.php @@ -233,12 +233,12 @@ class Activity implements IProvider { /** * @param int $id * @param string $path - * @return array + * @return array */ - protected function generateFileParameter($id, $path) { + protected function generateFileParameter($id, $path): array { return [ 'type' => 'file', - 'id' => $id, + 'id' => (string)$id, 'name' => basename($path), 'path' => trim($path, '/'), 'link' => $this->url->linkToRouteAbsolute('files.viewcontroller.showFile', ['fileid' => $id]), From da9b6e376d25619eb92fd3dca088d73e7cf445aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 18 Mar 2025 15:53:47 +0100 Subject: [PATCH 9/9] fix(tests): Sort activities by id to get the last one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- build/integration/features/bootstrap/Activity.php | 2 ++ build/integration/sharing_features/sharing-activity.feature | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/build/integration/features/bootstrap/Activity.php b/build/integration/features/bootstrap/Activity.php index 466b2dde14e..4172776304d 100644 --- a/build/integration/features/bootstrap/Activity.php +++ b/build/integration/features/bootstrap/Activity.php @@ -22,6 +22,8 @@ trait Activity { $this->theHTTPStatusCodeShouldBe('200'); $data = json_decode($this->response->getBody()->getContents(), true); $activities = $data['ocs']['data']; + /* Sort by id */ + uasort($activities, fn ($a, $b) => $a['activity_id'] <=> $b['activity_id']); $lastActivity = array_pop($activities); foreach ($activity->getRowsHash() as $key => $value) { Assert::assertEquals($value, $lastActivity[$key]); diff --git a/build/integration/sharing_features/sharing-activity.feature b/build/integration/sharing_features/sharing-activity.feature index 5757b210134..016b376488b 100644 --- a/build/integration/sharing_features/sharing-activity.feature +++ b/build/integration/sharing_features/sharing-activity.feature @@ -10,7 +10,7 @@ Feature: sharing And Logging in using web as "user0" And Sending a "POST" to "/apps/activity/settings" with requesttoken | public_links_notification | 1 | - | public_links_upload_email | 1 | + | public_links_upload_notification | 1 | | notify_setting_batchtime | 0 | | activity_digest | 0 |