From 56ef316884b320e028a37c8d5d05c91a6d174a9a Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 3 Nov 2026 14:46:10 +0100 Subject: [PATCH 1/4] fix: only act on peer-files of live-photos if they are in the same directory Signed-off-by: Robin Appelman Signed-off-by: Ferdinand Thiessen Assisted-by: ClaudeCode:claude-opus-5-5 --- .../lib/Listener/SyncLivePhotosListener.php | 54 +++++++++++++++---- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/apps/files/lib/Listener/SyncLivePhotosListener.php b/apps/files/lib/Listener/SyncLivePhotosListener.php index b6773e8c45239..ea56588dc34b1 100644 --- a/apps/files/lib/Listener/SyncLivePhotosListener.php +++ b/apps/files/lib/Listener/SyncLivePhotosListener.php @@ -57,22 +57,24 @@ public function handle(Event $event): void { if ($event instanceof BeforeNodeCopiedEvent || $event instanceof NodeCopiedEvent) { $this->handleCopyRecursive($event, $event->getSource(), $event->getTarget()); } else { - $peerFileId = null; - + $node = null; if ($event instanceof BeforeNodeRenamedEvent) { - $peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getSource()->getId()); + $node = $event->getSource(); } elseif ($event instanceof BeforeNodeDeletedEvent) { - $peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getNode()->getId()); + $node = $event->getNode(); } elseif ($event instanceof CacheEntryRemovedEvent) { - $peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getFileId()); + $this->handleCacheEntryRemovedEvent($event); + return; } + $peerFileId = $this->livePhotosService->getLivePhotoPeerId($node->getId()); + if ($peerFileId === null) { return; // Not a live photo. } // Check the user's folder. - $peerFile = $this->userFolder->getFirstNodeById($peerFileId); + $peerFile = $this->getPeerById($node, $peerFileId); if ($peerFile === null) { return; // Peer file not found. @@ -83,12 +85,46 @@ public function handle(Event $event): void { $this->handleMove($event->getSource(), $event->getTarget(), $peerFile); } elseif ($event instanceof BeforeNodeDeletedEvent) { $this->handleDeletion($event, $peerFile); - } elseif ($event instanceof CacheEntryRemovedEvent) { - $peerFile->delete(); } } } + /** + * Look for a peer file in the same directory as a file. + * + * If it's in a different folder, we consider the peer-relationship invalid + */ + private function getPeerById(Node $node, int $peerId): ?Node { + $parent = $node->getParent(); + foreach ($parent->getDirectoryListing() as $sibling) { + if ($sibling->getId() === $peerId) { + return $sibling; + } + } + return null; + } + + private function handleCacheEntryRemovedEvent(CacheEntryRemovedEvent $event): void { + $peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getFileId()); + if ($peerFileId === null) { + return; // Not a live photo. + } + + // Check the user's folder. + $peerFile = $this->userFolder->getFirstNodeById($peerFileId); + if ($peerFile === null) { + return; // Peer file not found. + } + + if ( + $event->getStorageId() !== $peerFile->getStorage()->getCache()->getNumericStorageId() + || dirname($event->getPath()) !== dirname($peerFile->getInternalPath()) + ) { + return; // Peer file is not in the same folder as the deleted file + } + $peerFile->delete(); + } + private function runMoveOrCopyChecks(Node $sourceFile, Node $targetFile, Node $peerFile): void { $targetParent = $targetFile->getParent(); $sourceExtension = $sourceFile->getExtension(); @@ -235,7 +271,7 @@ private function handleCopyRecursive(Event $event, Node $sourceNode, Node $targe if ($peerFileId === null) { return; } - $peerFile = $this->userFolder->getFirstNodeById($peerFileId); + $peerFile = $this->getPeerById($sourceNode, $peerFileId); if ($peerFile === null) { return; } From 84380063cdcfe37e02b790dba187d23a7ecee784 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 3 Nov 2026 14:46:31 +0100 Subject: [PATCH 2/4] fix: only count live photo peer relations that are mutual Signed-off-by: Robin Appelman Signed-off-by: Ferdinand Thiessen Assisted-by: ClaudeCode:claude-opus-5-5 --- apps/files/lib/Service/LivePhotosService.php | 15 ++++++ .../tests/Service/LivePhotosServiceTest.php | 46 +++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 apps/files/tests/Service/LivePhotosServiceTest.php diff --git a/apps/files/lib/Service/LivePhotosService.php b/apps/files/lib/Service/LivePhotosService.php index 3ac6601d5dce4..e4d5124a0d46e 100644 --- a/apps/files/lib/Service/LivePhotosService.php +++ b/apps/files/lib/Service/LivePhotosService.php @@ -21,6 +21,21 @@ public function __construct( * Get the associated live photo for a given file id */ public function getLivePhotoPeerId(int $fileId): ?int { + $peer = $this->getLivePhotoPeerIdUnchecked($fileId); + if ($peer === null) { + return null; + } + + $peerOfPeer = $this->getLivePhotoPeerIdUnchecked($peer); + if ($peerOfPeer === $fileId) { + // the peer relationship is only valid if it's mutual + return $peer; + } else { + return null; + } + } + + private function getLivePhotoPeerIdUnchecked(int $fileId): ?int { try { $metadata = $this->filesMetadataManager->getMetadata($fileId); } catch (FilesMetadataNotFoundException $ex) { diff --git a/apps/files/tests/Service/LivePhotosServiceTest.php b/apps/files/tests/Service/LivePhotosServiceTest.php new file mode 100644 index 0000000000000..158bb1a37f4d0 --- /dev/null +++ b/apps/files/tests/Service/LivePhotosServiceTest.php @@ -0,0 +1,46 @@ +createMock(IFilesMetadataManager::class); + $manager->method('getMetadata') + ->willReturnCallback(function (int $id) use ($peers) { + $metadata = new FilesMetadata($id); + if (isset($peers[$id])) { + $metadata->setString('files-live-photo', (string)$peers[$id]); + } + return $metadata; + }); + return $manager; + } + + public static function getPeerIdProvider(): array { + return [ + [[1 => 2, 2 => 1], 1, 2], + [[1 => 2], 1, null], + [[1 => 2], 2, null], + ]; + } + + /** + * @dataProvider getPeerIdProvider + */ + public function testGetPeerId(array $peers, int $fileId, ?int $expected): void { + $metadataManager = $this->getMetadataManager($peers); + $livePhotoService = new LivePhotosService($metadataManager); + $this->assertEquals($expected, $livePhotoService->getLivePhotoPeerId($fileId)); + } +} From 693972fb692bfac9ffff284494e811395d062a4d Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Tue, 3 Nov 2026 14:46:42 +0100 Subject: [PATCH 3/4] fix(live-photos): properly handle cross storage moves Signed-off-by: Ferdinand Thiessen Assisted-by: ClaudeCode:claude-opus-5-5 --- apps/files/lib/Listener/SyncLivePhotosListener.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/files/lib/Listener/SyncLivePhotosListener.php b/apps/files/lib/Listener/SyncLivePhotosListener.php index ea56588dc34b1..db58e3abfd73d 100644 --- a/apps/files/lib/Listener/SyncLivePhotosListener.php +++ b/apps/files/lib/Listener/SyncLivePhotosListener.php @@ -110,6 +110,10 @@ private function handleCacheEntryRemovedEvent(CacheEntryRemovedEvent $event): vo return; // Not a live photo. } + if (in_array($peerFileId, $this->pendingRenames, true)) { + return; // The file is moved to another storage along with its peer. + } + // Check the user's folder. $peerFile = $this->userFolder->getFirstNodeById($peerFileId); if ($peerFile === null) { From bcd5bcb2f88584621b29c4c706e92d6efad6b771 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Tue, 3 Nov 2026 14:47:11 +0100 Subject: [PATCH 4/4] test(files): add test coverage for live photos Signed-off-by: Ferdinand Thiessen Assisted-by: ClaudeCode:claude-opus-5-5 --- .../Listener/SyncLivePhotosListenerTest.php | 112 ++++++++++++++++++ .../features/bootstrap/MetadataContext.php | 19 +++ .../files_features/live-photos.feature | 91 ++++++++++++++ 3 files changed, 222 insertions(+) create mode 100644 apps/files/tests/Listener/SyncLivePhotosListenerTest.php create mode 100644 build/integration/files_features/live-photos.feature diff --git a/apps/files/tests/Listener/SyncLivePhotosListenerTest.php b/apps/files/tests/Listener/SyncLivePhotosListenerTest.php new file mode 100644 index 0000000000000..5a0097629d09b --- /dev/null +++ b/apps/files/tests/Listener/SyncLivePhotosListenerTest.php @@ -0,0 +1,112 @@ +userFolder = $this->createMock(Folder::class); + $this->livePhotosService = $this->createMock(LivePhotosService::class); + + $this->listener = new SyncLivePhotosListener( + $this->userFolder, + $this->createMock(IFilesMetadataManager::class), + $this->livePhotosService, + $this->createMock(IRootFolder::class), + $this->createMock(View::class), + ); + } + + private function createCacheEvent(int $fileId, int $storageId, string $path): CacheEntryRemovedEvent { + return new CacheEntryRemovedEvent($this->createMock(IStorage::class), $path, $fileId, $storageId); + } + + private function createPeer(int $storageId, string $internalPath): File&MockObject { + $cache = $this->createMock(ICache::class); + $cache->method('getNumericStorageId')->willReturn($storageId); + $storage = $this->createMock(IStorage::class); + $storage->method('getCache')->willReturn($cache); + + $peer = $this->createMock(File::class); + $peer->method('getStorage')->willReturn($storage); + $peer->method('getInternalPath')->willReturn($internalPath); + return $peer; + } + + public function testCacheEntryRemovedDeletesPeer(): void { + $this->livePhotosService->method('getLivePhotoPeerId') + ->with(1) + ->willReturn(11); + + $peer = $this->createPeer(10, 'files/photos/IMG.mov'); + $peer->expects($this->once())->method('delete'); + $this->userFolder->method('getFirstNodeById') + ->with(11) + ->willReturn($peer); + + $this->listener->handle($this->createCacheEvent(1, 10, 'files/photos/IMG.jpg')); + } + + public function testCacheEntryRemovedIgnoresNonLivePhoto(): void { + $this->livePhotosService->method('getLivePhotoPeerId') + ->willReturn(null); + $this->userFolder->expects($this->never())->method('getFirstNodeById'); + + $this->listener->handle($this->createCacheEvent(1, 10, 'files/photos/IMG.jpg')); + } + + public static function dataCacheEntryRemovedKeepsPeer(): array { + return [ + 'other folder' => [10, 'files/videos/IMG.mov'], + 'other folder with the same name' => [10, 'files/other/photos/IMG.mov'], + 'other storage' => [20, 'files/photos/IMG.mov'], + ]; + } + + /** + * @dataProvider dataCacheEntryRemovedKeepsPeer + */ + public function testCacheEntryRemovedKeepsPeer(int $peerStorageId, string $peerPath): void { + $this->livePhotosService->method('getLivePhotoPeerId') + ->willReturn(11); + + $peer = $this->createPeer($peerStorageId, $peerPath); + $peer->expects($this->never())->method('delete'); + $this->userFolder->method('getFirstNodeById')->willReturn($peer); + + $this->listener->handle($this->createCacheEvent(1, 10, 'files/photos/IMG.jpg')); + } + + public function testCacheEntryRemovedKeepsPeerThatIsBeingMoved(): void { + $this->livePhotosService->method('getLivePhotoPeerId') + ->willReturn(11); + $this->userFolder->expects($this->never())->method('getFirstNodeById'); + + self::invokePrivate($this->listener, 'pendingRenames', [[11]]); + $this->listener->handle($this->createCacheEvent(1, 10, 'files/IMG.jpg')); + } +} diff --git a/build/integration/features/bootstrap/MetadataContext.php b/build/integration/features/bootstrap/MetadataContext.php index 893c08a5467e9..f2f7469bcb5de 100644 --- a/build/integration/features/bootstrap/MetadataContext.php +++ b/build/integration/features/bootstrap/MetadataContext.php @@ -5,6 +5,7 @@ */ use Behat\Behat\Context\Context; +use Behat\Step\Given; use Behat\Step\Then; use Behat\Step\When; use PHPUnit\Framework\Assert; @@ -49,6 +50,24 @@ public function userSetsProp(string $user, string $metadataKey, string $metadata $client->request('PROPPATCH', $this->baseUrl . $davUrl, $body); } + #[Given('User :user links :fileName as live photo to :peerName')] + public function userLinksLivePhoto(string $user, string $fileName, string $peerName) { + $client = new SClient([ + 'baseUri' => $this->baseUrl, + 'userName' => $user, + 'password' => '123456', + 'authType' => SClient::AUTH_BASIC, + ]); + + $response = $client->propFind($this->baseUrl . $this->getDavUrl($user, $peerName), ['{http://owncloud.org/ns}fileid']); + $peerId = $response['{http://owncloud.org/ns}fileid']; + + $response = $client->propPatch($this->baseUrl . $this->getDavUrl($user, $fileName), [ + '{http://nextcloud.org/ns}metadata-files-live-photo' => (string)$peerId, + ]); + Assert::assertTrue($response); + } + #[When('User :user deletes the :metadataKey prop on :fileName')] public function userDeletesProp(string $user, string $metadataKey, string $fileName) { $client = new SClient([ diff --git a/build/integration/files_features/live-photos.feature b/build/integration/files_features/live-photos.feature new file mode 100644 index 0000000000000..d104dd1ab50fa --- /dev/null +++ b/build/integration/files_features/live-photos.feature @@ -0,0 +1,91 @@ +# SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors +# SPDX-License-Identifier: AGPL-3.0-only +Feature: live photos + Background: + Given using new dav path + And user "user0" exists + + Scenario: Deleting a live photo deletes its video + Given User "user0" uploads file with content "photo" to "/IMG.jpg" + And User "user0" uploads file with content "video" to "/IMG.mov" + And User "user0" links "/IMG.jpg" as live photo to "/IMG.mov" + And User "user0" links "/IMG.mov" as live photo to "/IMG.jpg" + When User "user0" deletes file "/IMG.jpg" + Then the HTTP status code should be "204" + And as "user0" the file "/IMG.mov" does not exist + + Scenario: Deleting a live photo keeps a linked video in another folder + Given User "user0" created a folder "/photos" + And User "user0" created a folder "/videos" + And User "user0" uploads file with content "photo" to "/photos/IMG.jpg" + And User "user0" uploads file with content "video" to "/videos/IMG.mov" + And User "user0" links "/photos/IMG.jpg" as live photo to "/videos/IMG.mov" + And User "user0" links "/videos/IMG.mov" as live photo to "/photos/IMG.jpg" + When User "user0" deletes file "/photos/IMG.jpg" + Then the HTTP status code should be "204" + And as "user0" the file "/videos/IMG.mov" exists + + Scenario: Renaming a live photo keeps a linked video in another folder + Given User "user0" created a folder "/photos" + And User "user0" created a folder "/videos" + And User "user0" uploads file with content "photo" to "/photos/IMG.jpg" + And User "user0" uploads file with content "video" to "/videos/IMG.mov" + And User "user0" links "/photos/IMG.jpg" as live photo to "/videos/IMG.mov" + And User "user0" links "/videos/IMG.mov" as live photo to "/photos/IMG.jpg" + When User "user0" moves file "/photos/IMG.jpg" to "/photos/renamed.jpg" + Then the HTTP status code should be "201" + And as "user0" the file "/videos/IMG.mov" exists + And as "user0" the file "/photos/renamed.mov" does not exist + + Scenario: Copying a live photo does not copy a linked video in another folder + Given User "user0" created a folder "/photos" + And User "user0" created a folder "/videos" + And User "user0" uploads file with content "photo" to "/photos/IMG.jpg" + And User "user0" uploads file with content "video" to "/videos/IMG.mov" + And User "user0" links "/photos/IMG.jpg" as live photo to "/videos/IMG.mov" + And User "user0" links "/videos/IMG.mov" as live photo to "/photos/IMG.jpg" + When User "user0" copies file "/photos/IMG.jpg" to "/copy.jpg" + Then the HTTP status code should be "201" + And as "user0" the file "/copy.mov" does not exist + + Scenario: Deleting a photo keeps a video that does not link back to it + Given User "user0" uploads file with content "photo" to "/IMG.jpg" + And User "user0" uploads file with content "video" to "/IMG.mov" + And User "user0" links "/IMG.jpg" as live photo to "/IMG.mov" + When User "user0" deletes file "/IMG.jpg" + Then the HTTP status code should be "204" + And as "user0" the file "/IMG.mov" exists + + Scenario: Deleting a video is allowed when the photo does not link back to it + Given User "user0" uploads file with content "photo" to "/IMG.jpg" + And User "user0" uploads file with content "video" to "/IMG.mov" + And User "user0" links "/IMG.mov" as live photo to "/IMG.jpg" + When User "user0" deletes file "/IMG.mov" + Then the HTTP status code should be "204" + And as "user0" the file "/IMG.jpg" exists + + Scenario: Moving a live photo into another storage moves its video along + Given User "user0" uploads file with content "photo" to "/IMG.jpg" + And User "user0" uploads file with content "video" to "/IMG.mov" + And User "user0" links "/IMG.jpg" as live photo to "/IMG.mov" + And User "user0" links "/IMG.mov" as live photo to "/IMG.jpg" + And User "user0" created a folder "/local_storage/live-photos" + When User "user0" moves file "/IMG.jpg" to "/local_storage/live-photos/IMG.jpg" + Then the HTTP status code should be "201" + And as "user0" the file "/local_storage/live-photos/IMG.jpg" exists + And as "user0" the file "/local_storage/live-photos/IMG.mov" exists + And as "user0" the file "/IMG.mov" does not exist + And User "user0" deletes folder "/local_storage/live-photos" + + Scenario: Moving a live photo video into another storage moves its photo along + Given User "user0" uploads file with content "photo" to "/IMG.jpg" + And User "user0" uploads file with content "video" to "/IMG.mov" + And User "user0" links "/IMG.jpg" as live photo to "/IMG.mov" + And User "user0" links "/IMG.mov" as live photo to "/IMG.jpg" + And User "user0" created a folder "/local_storage/live-photos" + When User "user0" moves file "/IMG.mov" to "/local_storage/live-photos/IMG.mov" + Then the HTTP status code should be "201" + And as "user0" the file "/local_storage/live-photos/IMG.mov" exists + And as "user0" the file "/local_storage/live-photos/IMG.jpg" exists + And as "user0" the file "/IMG.jpg" does not exist + And User "user0" deletes folder "/local_storage/live-photos"