Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Revert "[stable30] fix: Handle copy of folders containing live photos"" #49840

Open
wants to merge 2 commits into
base: stable30
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion apps/dav/lib/Connector/Sabre/Directory.php
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,13 @@
throw new InvalidPath($ex->getMessage());
}

return $this->fileView->copy($sourcePath, $destinationPath);
$copyOkay = $this->fileView->copy($sourcePath, $destinationPath);

if (!$copyOkay) {

Check notice

Code scanning / Psalm

RiskyTruthyFalsyComparison Note

Operand of type bool|mixed contains type mixed, which can be falsy and truthy. This can cause possibly unexpected behavior. Use strict comparison instead.
throw new \Sabre\DAV\Exception\Forbidden('Copy did not proceed');
}

return true;
} catch (StorageNotAvailableException $e) {
throw new ServiceUnavailable($e->getMessage(), $e->getCode(), $e);
} catch (ForbiddenException $ex) {
Expand Down
184 changes: 117 additions & 67 deletions apps/files/lib/Listener/SyncLivePhotosListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,23 @@

namespace OCA\Files\Listener;

use Exception;
use OC\Files\Node\NonExistingFile;
use OC\Files\Node\NonExistingFolder;
use OC\Files\View;
use OC\FilesMetadata\Model\FilesMetadata;
use OCA\Files\Service\LivePhotosService;
use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventListener;
use OCP\Exceptions\AbortedEventException;
use OCP\Files\Cache\CacheEntryRemovedEvent;
use OCP\Files\Events\Node\AbstractNodesEvent;
use OCP\Files\Events\Node\BeforeNodeCopiedEvent;
use OCP\Files\Events\Node\BeforeNodeDeletedEvent;
use OCP\Files\Events\Node\BeforeNodeRenamedEvent;
use OCP\Files\Events\Node\NodeCopiedEvent;
use OCP\Files\File;
use OCP\Files\Folder;
use OCP\Files\IRootFolder;
use OCP\Files\Node;
use OCP\Files\NotFoundException;
use OCP\FilesMetadata\IFilesMetadataManager;
Expand All @@ -37,6 +42,8 @@
private ?Folder $userFolder,
private IFilesMetadataManager $filesMetadataManager,
private LivePhotosService $livePhotosService,
private IRootFolder $rootFolder,
private View $view,
) {
}

Expand All @@ -45,61 +52,47 @@
return;
}

$peerFileId = null;

if ($event instanceof BeforeNodeRenamedEvent) {
$peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getSource()->getId());
} elseif ($event instanceof BeforeNodeDeletedEvent) {
$peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getNode()->getId());
} elseif ($event instanceof CacheEntryRemovedEvent) {
$peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getFileId());
} elseif ($event instanceof BeforeNodeCopiedEvent || $event instanceof NodeCopiedEvent) {
$peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getSource()->getId());
}
if ($event instanceof BeforeNodeCopiedEvent || $event instanceof NodeCopiedEvent) {
$this->handleCopyRecursive($event, $event->getSource(), $event->getTarget());
} else {
$peerFileId = null;

if ($event instanceof BeforeNodeRenamedEvent) {
$peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getSource()->getId());
} elseif ($event instanceof BeforeNodeDeletedEvent) {
$peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getNode()->getId());
} elseif ($event instanceof CacheEntryRemovedEvent) {
$peerFileId = $this->livePhotosService->getLivePhotoPeerId($event->getFileId());
}

if ($peerFileId === null) {
return; // Not a live photo.
}
if ($peerFileId === null) {
return; // Not a live photo.
}

// Check the user's folder.
$peerFile = $this->userFolder->getFirstNodeById($peerFileId);
// Check the user's folder.
$peerFile = $this->userFolder->getFirstNodeById($peerFileId);

if ($peerFile === null) {
return; // Peer file not found.
}
if ($peerFile === null) {
return; // Peer file not found.
}

if ($event instanceof BeforeNodeRenamedEvent) {
$this->handleMove($event, $peerFile, false);
} elseif ($event instanceof BeforeNodeDeletedEvent) {
$this->handleDeletion($event, $peerFile);
} elseif ($event instanceof CacheEntryRemovedEvent) {
$peerFile->delete();
} elseif ($event instanceof BeforeNodeCopiedEvent) {
$this->handleMove($event, $peerFile, true);
} elseif ($event instanceof NodeCopiedEvent) {
$this->handleCopy($event, $peerFile);
if ($event instanceof BeforeNodeRenamedEvent) {
$this->runMoveOrCopyChecks($event->getSource(), $event->getTarget(), $peerFile);
$this->handleMove($event->getSource(), $event->getTarget(), $peerFile);
} elseif ($event instanceof BeforeNodeDeletedEvent) {
$this->handleDeletion($event, $peerFile);
} elseif ($event instanceof CacheEntryRemovedEvent) {
$peerFile->delete();
}
}
}

/**
* During rename events, which also include move operations,
* we rename the peer file using the same name.
* The event listener being singleton, we can store the current state
* of pending renames inside the 'pendingRenames' property,
* to prevent infinite recursive.
*/
private function handleMove(AbstractNodesEvent $event, Node $peerFile, bool $prepForCopyOnly = false): void {
if (!($event instanceof BeforeNodeCopiedEvent) &&
!($event instanceof BeforeNodeRenamedEvent)) {
return;
}

$sourceFile = $event->getSource();
$targetFile = $event->getTarget();
private function runMoveOrCopyChecks(Node $sourceFile, Node $targetFile, Node $peerFile): void {
$targetParent = $targetFile->getParent();
$sourceExtension = $sourceFile->getExtension();
$peerFileExtension = $peerFile->getExtension();
$targetName = $targetFile->getName();
$peerTargetName = substr($targetName, 0, -strlen($sourceExtension)) . $peerFileExtension;

if (!str_ends_with($targetName, "." . $sourceExtension)) {
throw new AbortedEventException('Cannot change the extension of a Live Photo');
Expand All @@ -111,15 +104,31 @@
} catch (NotFoundException) {
}

$peerTargetName = substr($targetName, 0, -strlen($sourceExtension)) . $peerFileExtension;
try {
$targetParent->get($peerTargetName);
throw new AbortedEventException('A file already exist at destination path of the Live Photo');
} catch (NotFoundException) {
if (!($targetParent instanceof NonExistingFolder)) {
try {
$targetParent->get($peerTargetName);
throw new AbortedEventException('A file already exist at destination path of the Live Photo');
} catch (NotFoundException) {
}
}
}

/**
* During rename events, which also include move operations,
* we rename the peer file using the same name.
* The event listener being singleton, we can store the current state
* of pending renames inside the 'pendingRenames' property,
* to prevent infinite recursive.
*/
private function handleMove(Node $sourceFile, Node $targetFile, Node $peerFile): void {
$targetParent = $targetFile->getParent();
$sourceExtension = $sourceFile->getExtension();
$peerFileExtension = $peerFile->getExtension();
$targetName = $targetFile->getName();
$peerTargetName = substr($targetName, 0, -strlen($sourceExtension)) . $peerFileExtension;

// in case the rename was initiated from this listener, we stop right now
if ($prepForCopyOnly || in_array($peerFile->getId(), $this->pendingRenames)) {
if (in_array($peerFile->getId(), $this->pendingRenames)) {
return;
}

Expand All @@ -130,39 +139,37 @@
throw new AbortedEventException($ex->getMessage());
}

array_diff($this->pendingRenames, [$sourceFile->getId()]);
$this->pendingRenames = array_diff($this->pendingRenames, [$sourceFile->getId()]);
}


/**
* handle copy, we already know if it is doable from BeforeNodeCopiedEvent, so we just copy the linked file
*
* @param NodeCopiedEvent $event
* @param Node $peerFile
*/
private function handleCopy(NodeCopiedEvent $event, Node $peerFile): void {
$sourceFile = $event->getSource();
private function handleCopy(File $sourceFile, File $targetFile, File $peerFile): void {
$sourceExtension = $sourceFile->getExtension();
$peerFileExtension = $peerFile->getExtension();
$targetFile = $event->getTarget();
$targetParent = $targetFile->getParent();
$targetName = $targetFile->getName();
$peerTargetName = substr($targetName, 0, -strlen($sourceExtension)) . $peerFileExtension;

/**
* let's use freshly set variable.
* we copy the file and get its id. We already have the id of the current copy
* We have everything to update metadata and keep the link between the 2 copies.
*/
$newPeerFile = $peerFile->copy($targetParent->getPath() . '/' . $peerTargetName);

if ($targetParent->nodeExists($peerTargetName)) {
// If the copy was a folder copy, then the peer file already exists.
$targetPeerFile = $targetParent->get($peerTargetName);
} else {
// If the copy was a file copy, then we need to create the peer file.
$targetPeerFile = $peerFile->copy($targetParent->getPath() . '/' . $peerTargetName);
}

/** @var FilesMetadata $targetMetadata */
$targetMetadata = $this->filesMetadataManager->getMetadata($targetFile->getId(), true);
$targetMetadata->setStorageId($targetFile->getStorage()->getCache()->getNumericStorageId());
$targetMetadata->setString('files-live-photo', (string)$newPeerFile->getId());
$targetMetadata->setString('files-live-photo', (string)$targetPeerFile->getId());
$this->filesMetadataManager->saveMetadata($targetMetadata);
/** @var FilesMetadata $peerMetadata */
$peerMetadata = $this->filesMetadataManager->getMetadata($newPeerFile->getId(), true);
$peerMetadata->setStorageId($newPeerFile->getStorage()->getCache()->getNumericStorageId());
$peerMetadata = $this->filesMetadataManager->getMetadata($targetPeerFile->getId(), true);
$peerMetadata->setStorageId($targetPeerFile->getStorage()->getCache()->getNumericStorageId());
$peerMetadata->setString('files-live-photo', (string)$targetFile->getId());
$this->filesMetadataManager->saveMetadata($peerMetadata);
}
Expand Down Expand Up @@ -193,4 +200,47 @@
}
return;
}

/*
* Recursively get all the peer ids of a live photo.
* Needed when coping a folder.
*
* @param BeforeNodeCopiedEvent|NodeCopiedEvent $event
*/
private function handleCopyRecursive(Event $event, Node $sourceNode, Node $targetNode): void {
if ($sourceNode instanceof Folder && $targetNode instanceof Folder) {
foreach ($sourceNode->getDirectoryListing() as $sourceChild) {
if ($event instanceof BeforeNodeCopiedEvent) {
if ($sourceChild instanceof Folder) {
$targetChild = new NonExistingFolder($this->rootFolder, $this->view, $targetNode->getPath() . '/' . $sourceChild->getName(), null, $targetNode);
} else {
$targetChild = new NonExistingFile($this->rootFolder, $this->view, $targetNode->getPath() . '/' . $sourceChild->getName(), null, $targetNode);
}
} elseif ($event instanceof NodeCopiedEvent) {
$targetChild = $targetNode->get($sourceChild->getName());
} else {
throw new Exception('Event is type is not supported');
}

$this->handleCopyRecursive($event, $sourceChild, $targetChild);
}
} elseif ($sourceNode instanceof File && $targetNode instanceof File) {
$peerFileId = $this->livePhotosService->getLivePhotoPeerId($sourceNode->getId());
if ($peerFileId === null) {
return;
}
$peerFile = $this->userFolder->getFirstNodeById($peerFileId);

Check notice

Code scanning / Psalm

PossiblyNullReference Note

Cannot call method getFirstNodeById on possibly null value
if ($peerFile === null) {
return;
}

if ($event instanceof BeforeNodeCopiedEvent) {
$this->runMoveOrCopyChecks($sourceNode, $targetNode, $peerFile);
} elseif ($event instanceof NodeCopiedEvent) {
$this->handleCopy($sourceNode, $targetNode, $peerFile);

Check notice

Code scanning / Psalm

ArgumentTypeCoercion Note

Argument 3 of OCA\Files\Listener\SyncLivePhotosListener::handleCopy expects OCP\Files\File, but parent type OCP\Files\Node provided
}
} else {
throw new Exception('Source and target type are not matching');
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

use Exception;
use OC\Files\Node\NonExistingFile;
use OC\Files\Node\NonExistingFolder;
use OCA\Files_Versions\Versions\IVersionBackend;
use OCA\Files_Versions\Versions\IVersionManager;
use OCA\Files_Versions\Versions\IVersionsImporterBackend;
Expand Down Expand Up @@ -130,7 +131,7 @@ private function handleMoveOrCopy(Event $event, IUser $user, File $source, File
}

private function getNodeStorage(Node $node): IStorage {
if ($node instanceof NonExistingFile) {
if ($node instanceof NonExistingFile || $node instanceof NonExistingFolder) {
return $node->getParent()->getStorage();
} else {
return $node->getStorage();
Expand Down
13 changes: 13 additions & 0 deletions cypress.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,19 @@ export default defineConfig({

on('task', { removeDirectory })

// This allows to store global data (e.g. the name of a snapshot)
// because Cypress.env() and other options are local to the current spec file.
const data = {}
on('task', {
setVariable({ key, value }) {
data[key] = value
return null
},
getVariable({ key }) {
return data[key] ?? null
},
})

// Disable spell checking to prevent rendering differences
on('before:browser:launch', (browser, launchOptions) => {
if (browser.family === 'chromium' && browser.name !== 'electron') {
Expand Down
2 changes: 1 addition & 1 deletion cypress/e2e/files/FilesUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export const createFolder = (folderName: string) => {

// TODO: replace by proper data-cy selectors
cy.get('[data-cy-upload-picker] .action-item__menutoggle').first().click()
cy.contains('.upload-picker__menu-entry button', 'New folder').click()
cy.get('[data-cy-upload-picker-menu-entry="newFolder"] button').click()
cy.get('[data-cy-files-new-node-dialog]').should('be.visible')
cy.get('[data-cy-files-new-node-dialog-input]').type(`{selectall}${folderName}`)
cy.get('[data-cy-files-new-node-dialog-submit]').click()
Expand Down
Loading
Loading