Skip to content

refactor(files): reduce duplication in node copy/move and tidy Node internals #60971

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.

Sign up for GitHub

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

Draft
joshtrichards wants to merge 4 commits into
base: master
Choose a base branch
from
+85 −161
Draft

refactor(files): reduce duplication in node copy/move and tidy Node internals #60971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d267a57
refactor(Node): copy/move early exits/code clarity
joshtrichards Jun 3, 2026
0abc730
docs(Node): add exceptions to interface copy()
joshtrichards Jun 3, 2026
5da2eba
chore(Node): drop remainder of unnecessary (duplicate) docblocks
joshtrichards Jun 3, 2026
48858c5
refactor(Node): eliminate duplicate copy/move logic via shared helper
joshtrichards Jun 3, 2026
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
243 changes: 82 additions & 161 deletions lib/private/Files/Node/Node.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,32 +28,19 @@

// FIXME: this class really should be abstract (+1)
class Node implements INode {
/**
* @var View $view
*/
protected $view;

protected IRootFolder $root;

/**
* @param View $view
* @param \OCP\Files\IRootFolder $root
* @param string $path
* @param FileInfo $fileInfo
*/
public function __construct(
IRootFolder $root,
$view,
protected IRootFolder $root,
protected View $view,
protected $path,
protected ?FileInfo $fileInfo = null,
protected ?INode $parent = null,
private bool $infoHasSubMountsIncluded = true,
) {
if (Filesystem::normalizePath($view->getRoot()) !== '/') {
throw new PreConditionNotMetException('The view passed to the node should not have any fake root set');
throw new PreConditionNotMetException(
'The view passed to the node should not have any fake root set'
);
}
$this->view = $view;
$this->root = $root;
}

/**
Expand All @@ -73,6 +60,8 @@ protected function createNonExistingNode($path) {
* @return FileInfo
* @throws InvalidPathException
* @throws NotFoundException
*
* @internal
*/
public function getFileInfo(bool $includeMountPoint = true) {
if (!$this->fileInfo) {
Expand Down Expand Up @@ -131,12 +120,6 @@ protected function checkPermissions($permissions) {
public function delete() {
}

/**
* @param int $mtime
* @throws InvalidPathException
* @throws NotFoundException
* @throws NotPermittedException
*/
#[\Override]
public function touch($mtime = null) {
if ($this->checkPermissions(Constants::PERMISSION_UPDATE)) {
Expand All @@ -163,123 +146,67 @@ public function getStorage() {
return $storage;
}

/**
* @return string
*/
#[\Override]
public function getPath() {
return $this->path;
}

/**
* @return string
*/
#[\Override]
public function getInternalPath() {
return $this->getFileInfo(false)->getInternalPath();
}

/**
* @return int
* @throws InvalidPathException
* @throws NotFoundException
*/
#[\Override]
public function getId() {
return $this->getFileInfo(false)->getId() ?? -1;
}

/**
* @return array
*/
#[\Override]
public function stat() {
return $this->view->stat($this->path);
}

/**
* @return int
* @throws InvalidPathException
* @throws NotFoundException
*/
#[\Override]
public function getMTime() {
return $this->getFileInfo()->getMTime();
}

/**
* @param bool $includeMounts
* @return int|float
* @throws InvalidPathException
* @throws NotFoundException
*/
#[\Override]
public function getSize($includeMounts = true): int|float {
return $this->getFileInfo()->getSize($includeMounts);
}

/**
* @return string
* @throws InvalidPathException
* @throws NotFoundException
*/
#[\Override]
public function getEtag() {
return $this->getFileInfo()->getEtag();
}

/**
* @return int
* @throws InvalidPathException
* @throws NotFoundException
*/
#[\Override]
public function getPermissions() {
return $this->getFileInfo(false)->getPermissions();
}

/**
* @return bool
* @throws InvalidPathException
* @throws NotFoundException
*/
#[\Override]
public function isReadable() {
return $this->getFileInfo(false)->isReadable();
}

/**
* @return bool
* @throws InvalidPathException
* @throws NotFoundException
*/
#[\Override]
public function isUpdateable() {
return $this->getFileInfo(false)->isUpdateable();
}

/**
* @return bool
* @throws InvalidPathException
* @throws NotFoundException
*/
#[\Override]
public function isDeletable() {
return $this->getFileInfo(false)->isDeletable();
}

/**
* @return bool
* @throws InvalidPathException
* @throws NotFoundException
*/
#[\Override]
public function isShareable() {
return $this->getFileInfo(false)->isShareable();
}

/**
* @return bool
* @throws InvalidPathException
* @throws NotFoundException
*/
Expand Down Expand Up @@ -320,9 +247,6 @@ public function getParent(): INode|IRootFolder {
return $this->parent;
}

/**
* @return string
*/
#[\Override]
public function getName() {
return basename($this->path);
Expand All @@ -341,6 +265,8 @@ protected function normalizePath($path) {
*
* @param string $path
* @return bool
*
* @internal
*/
public function isValidPath($path) {
return Filesystem::isValidPath($path);
Expand Down Expand Up @@ -395,106 +321,105 @@ public function getExtension(): string {
return $this->getFileInfo(false)->getExtension();
}

/**
* @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE
* @throws LockedException
*/
#[\Override]
public function lock($type) {
$this->view->lockFile($this->path, $type);
}

/**
* @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE
* @throws LockedException
*/
#[\Override]
public function changeLock($type) {
$this->view->changeLock($this->path, $type);
}

/**
* @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE
* @throws LockedException
*/
#[\Override]
public function unlock($type) {
$this->view->unlockFile($this->path, $type);
}

/**
* @param string $targetPath
* @return INode
* @throws InvalidPathException
* @throws NotFoundException
* @throws NotPermittedException if copy not allowed or failed
*/
#[\Override]
public function copy($targetPath) {
$targetPath = $this->normalizePath($targetPath);
$parent = $this->root->get(dirname($targetPath));
if ($parent instanceof Folder && $this->isValidPath($targetPath) && $parent->isCreatable()) {
$nonExisting = $this->createNonExistingNode($targetPath);
$this->sendHooks(['preCopy'], [$this, $nonExisting]);
$this->sendHooks(['preWrite'], [$nonExisting]);
if (!$this->view->copy($this->path, $targetPath)) {
throw new NotPermittedException('Could not copy ' . $this->path . ' to ' . $targetPath);
}
$targetNode = $this->root->get($targetPath);
$this->sendHooks(['postCopy'], [$this, $targetNode]);
$this->sendHooks(['postWrite'], [$targetNode]);
return $targetNode;
} else {
throw new NotPermittedException('No permission to copy to path ' . $targetPath);
[$targetPath, $targetPlaceholder] = $this->prepareTargetPath($targetPath);

$this->sendHooks(['preCopy'], [$this, $targetPlaceholder]);
$this->sendHooks(['preWrite'], [$targetPlaceholder]);

if (!$this->view->copy($this->path, $targetPath)) {
throw new NotPermittedException('Could not copy ' . $this->path . ' to ' . $targetPath);
}

$targetNode = $this->root->get($targetPath);
$this->sendHooks(['postCopy'], [$this, $targetNode]);
$this->sendHooks(['postWrite'], [$targetNode]);

return $targetNode;
}

/**
* @param string $targetPath
* @return INode
* @throws InvalidPathException
* @throws NotFoundException
* @throws NotPermittedException if move not allowed or failed
* @throws LockedException
*/
#[\Override]
public function move($targetPath) {
[$targetPath, $targetPlaceholder] = $this->prepareTargetPath($targetPath, true);

$this->sendHooks(['preRename'], [$this, $targetPlaceholder]);
$this->sendHooks(['preWrite'], [$targetPlaceholder]);

if (!$this->view->rename($this->path, $targetPath)) {
throw new NotPermittedException('Could not move ' . $this->path . ' to ' . $targetPath);
}

$this->updateCachedFileInfoAfterMove($targetPath);

$targetNode = $this->root->get($targetPath);
$this->sendHooks(['postRename'], [$this, $targetNode]);
$this->sendHooks(['postWrite'], [$targetNode]);
$this->path = $targetPath;

return $targetNode;
}

private function prepareTargetPath(string $targetPath, bool $allowRootMovable = false): array {
$targetPath = $this->normalizePath($targetPath);
$parent = $this->root->get(dirname($targetPath));
if (
($parent instanceof Folder)
&& $this->isValidPath($targetPath)
&& (
$parent->isCreatable()
|| (
$parent->getInternalPath() === ''
&& ($parent->getMountPoint() instanceof IMovableMount)
)
)
) {
$nonExisting = $this->createNonExistingNode($targetPath);
$this->sendHooks(['preRename'], [$this, $nonExisting]);
$this->sendHooks(['preWrite'], [$nonExisting]);
if (!$this->view->rename($this->path, $targetPath)) {
throw new NotPermittedException('Could not move ' . $this->path . ' to ' . $targetPath);
}
$targetParent = $this->root->get(dirname($targetPath));

$mountPoint = $this->getMountPoint();
if ($mountPoint) {
// update the cached fileinfo with the new (internal) path
/** @var \OC\Files\FileInfo $oldFileInfo */
$oldFileInfo = $this->getFileInfo();
$this->fileInfo = new \OC\Files\FileInfo($targetPath, $oldFileInfo->getStorage(), $mountPoint->getInternalPath($targetPath), $oldFileInfo->getData(), $mountPoint, $oldFileInfo->getOwner());
}
if (!($targetParent instanceof Folder)) {
throw new NotPermittedException('Target parent is not a folder: ' . dirname($targetPath));
}

$targetNode = $this->root->get($targetPath);
$this->sendHooks(['postRename'], [$this, $targetNode]);
$this->sendHooks(['postWrite'], [$targetNode]);
$this->path = $targetPath;
return $targetNode;
} else {
throw new NotPermittedException('No permission to move to path ' . $targetPath);
if (!$this->isValidPath($targetPath)) {
throw new NotPermittedException('Invalid target path: ' . $targetPath);
}

$isMovableRootMountTarget = $allowRootMovable
&& $targetParent->getInternalPath() === ''
&& $targetParent->getMountPoint() instanceof IMovableMount;

$canWriteToTargetParent = $targetParent->isCreatable() || $isMovableRootMountTarget;

if (!$canWriteToTargetParent) {
throw new NotPermittedException('No permission to write to path ' . $targetPath);
}

return [
$targetPath,
$this->createNonExistingNode($targetPath),
];
}

private function updateCachedFileInfoAfterMove(string $targetPath): void {
$mountPoint = $this->getMountPoint();
if (!$mountPoint) {
return;
}

// update the cached fileinfo with the new (internal) path
/** @var \OC\Files\FileInfo $oldFileInfo */
$oldFileInfo = $this->getFileInfo();
$this->fileInfo = new \OC\Files\FileInfo(
$targetPath,
$oldFileInfo->getStorage(),
$mountPoint->getInternalPath($targetPath),
$oldFileInfo->getData(),
$mountPoint,
$oldFileInfo->getOwner()
);
}

#[\Override]
Expand All @@ -517,10 +442,6 @@ public function getParentId(): int {
return $this->fileInfo->getParentId();
}

/**
* @inheritDoc
* @return array<string, int|string|bool|float|string[]|int[]>
*/
#[\Override]
public function getMetadata(): array {
return $this->fileInfo->getMetadata();
Expand Down
Loading
Loading