Merge pull request #58286 from nextcloud/fix/groupfolders-phpstan

Fix a bunch of typing issues to make PHPStan level 10 happy on groupfolders

apps/dav/lib/Connector/Sabre/File.php

@@ -236,7 +236,13 @@ class File extends Node implements IFile {
 					// because we have no clue about the cause we can only throw back a 500/Internal Server Error
 					throw new Exception($this->l10n->t('Could not write file contents'));
 				}
-				[$count, $result] = Files::streamCopy($data, $target, true);
+				$count = stream_copy_to_stream($data, $target);
+				if ($count === false) {
+					$result = false;
+					$count = 0;
+				} else {
+					$result = true;
+				}
 				fclose($target);
 			}
 			if ($result === false && $expected !== null) {

apps/dav/lib/Connector/Sabre/FilesPlugin.php

@@ -166,11 +166,6 @@ class FilesPlugin extends ServerPlugin {
 			return;
 		}
 
-		// Ensure source exists
-		$sourceNodeFileInfo = $sourceNode->getFileInfo();
-		if ($sourceNodeFileInfo === null) {
-			throw new NotFound($source . ' does not exist');
-		}
 		// Ensure the target name is valid
 		try {
 			[$targetPath, $targetName] = \Sabre\Uri\split($target);

apps/dav/lib/Connector/Sabre/Node.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@@ -20,42 +22,48 @@ use OCP\Files\IRootFolder;
 use OCP\Files\NotFoundException;
 use OCP\Files\Storage\ISharedStorage;
 use OCP\Files\StorageNotAvailableException;
+use OCP\IUser;
+use OCP\Lock\ILockingProvider;
+use OCP\Lock\LockedException;
+use OCP\PreConditionNotMetException;
 use OCP\Server;
 use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IManager;
+use RuntimeException;
+use Sabre\DAV\Exception;
+use Sabre\DAV\Exception\Forbidden;
+use Sabre\DAV\INode;
 
-abstract class Node implements \Sabre\DAV\INode {
+abstract class Node implements INode {
 	/**
 	 * The path to the current node
-	 *
-	 * @var string
 	 */
-	protected $path;
+	protected string $path;
 
 	protected FileInfo $info;
 
-	/**
-	 * @var IManager
-	 */
-	protected $shareManager;
+	protected IManager $shareManager;
 
 	protected \OCP\Files\Node $node;
 
 	/**
 	 * Sets up the node, expects a full path name
+	 * @throws PreConditionNotMetException
 	 */
 	public function __construct(
 		protected View $fileView,
 		FileInfo $info,
 		?IManager $shareManager = null,
 	) {
-		$this->path = $this->fileView->getRelativePath($info->getPath());
-		$this->info = $info;
-		if ($shareManager) {
-			$this->shareManager = $shareManager;
-		} else {
-			$this->shareManager = Server::get(\OCP\Share\IManager::class);
+		$relativePath = $this->fileView->getRelativePath($info->getPath());
+		if ($relativePath === null) {
+			throw new RuntimeException('Failed to get relative path for ' . $info->getPath());
 		}
+
+		$this->path = $relativePath;
+		$this->info = $info;
+		$this->shareManager = $shareManager instanceof IManager ? $shareManager : Server::get(IManager::class);
+
 		if ($info instanceof Folder || $info instanceof File) {
 			$this->node = $info;
 		} else {
@@ -70,11 +78,16 @@ abstract class Node implements \Sabre\DAV\INode {
 		}
 	}
 
+	/**
+	 * @throws Exception
+	 * @throws PreConditionNotMetException
+	 */
 	protected function refreshInfo(): void {
 		$info = $this->fileView->getFileInfo($this->path);
 		if ($info === false) {
-			throw new \Sabre\DAV\Exception('Failed to get fileinfo for ' . $this->path);
+			throw new Exception('Failed to get fileinfo for ' . $this->path);
 		}
+
 		$this->info = $info;
 		$root = Server::get(IRootFolder::class);
 		$rootView = Server::get(View::class);
@@ -87,19 +100,15 @@ abstract class Node implements \Sabre\DAV\INode {
 
 	/**
 	 *  Returns the name of the node
-	 *
-	 * @return string
 	 */
-	public function getName() {
+	public function getName(): string {
 		return $this->info->getName();
 	}
 
 	/**
 	 * Returns the full path
-	 *
-	 * @return string
 	 */
-	public function getPath() {
+	public function getPath(): string {
 		return $this->path;
 	}
 
@@ -107,25 +116,30 @@ abstract class Node implements \Sabre\DAV\INode {
 	 * Renames the node
 	 *
 	 * @param string $name The new name
-	 * @throws \Sabre\DAV\Exception\BadRequest
-	 * @throws \Sabre\DAV\Exception\Forbidden
+	 * @throws Exception
+	 * @throws Forbidden
+	 * @throws InvalidPath
+	 * @throws PreConditionNotMetException
+	 * @throws LockedException
 	 */
-	public function setName($name) {
+	public function setName($name): void {
 		// rename is only allowed if the delete privilege is granted
 		// (basically rename is a copy with delete of the original node)
-		if (!($this->info->isDeletable() || ($this->info->getMountPoint() instanceof MoveableMount && $this->info->getInternalPath() === ''))) {
-			throw new \Sabre\DAV\Exception\Forbidden();
+		if (!$this->info->isDeletable() && !($this->info->getMountPoint() instanceof MoveableMount && $this->info->getInternalPath() === '')) {
+			throw new Forbidden();
 		}
 
+		/** @var string $parentPath */
 		[$parentPath,] = \Sabre\Uri\split($this->path);
+		/** @var string $newName */
 		[, $newName] = \Sabre\Uri\split($name);
 		$newPath = $parentPath . '/' . $newName;
 
 		// verify path of the target
 		$this->verifyPath($newPath);
 
-		if (!$this->fileView->rename($this->path, $newPath)) {
-			throw new \Sabre\DAV\Exception('Failed to rename ' . $this->path . ' to ' . $newPath);
+		if ($this->fileView->rename($this->path, $newPath) === false) {
+			throw new Exception('Failed to rename ' . $this->path . ' to ' . $newPath);
 		}
 
 		$this->path = $newPath;
@@ -138,12 +152,8 @@ abstract class Node implements \Sabre\DAV\INode {
 	 *
 	 * @return int timestamp as integer
 	 */
-	public function getLastModified() {
-		$timestamp = $this->info->getMtime();
-		if (!empty($timestamp)) {
-			return (int)$timestamp;
-		}
-		return $timestamp;
+	public function getLastModified(): int {
+		return $this->info->getMtime();
 	}
 
 	/**
@@ -151,7 +161,7 @@ abstract class Node implements \Sabre\DAV\INode {
 	 *  in the second parameter or to now if the second param is empty.
 	 *  Even if the modification time is set to a custom value the access time is set to now.
 	 */
-	public function touch($mtime) {
+	public function touch(string $mtime): void {
 		$mtime = $this->sanitizeMtime($mtime);
 		$this->fileView->touch($this->path, $mtime);
 		$this->refreshInfo();
@@ -165,37 +175,29 @@ abstract class Node implements \Sabre\DAV\INode {
 	 * arbitrary string, but MUST be surrounded by double-quotes.
 	 *
 	 * Return null if the ETag can not effectively be determined
-	 *
-	 * @return string
 	 */
-	public function getETag() {
+	public function getETag(): string {
 		return '"' . $this->info->getEtag() . '"';
 	}
 
 	/**
 	 * Sets the ETag
 	 *
-	 * @param string $etag
-	 *
 	 * @return int file id of updated file or -1 on failure
 	 */
-	public function setETag($etag) {
+	public function setETag(string $etag): int {
 		return $this->fileView->putFileInfo($this->path, ['etag' => $etag]);
 	}
 
-	public function setCreationTime(int $time) {
+	public function setCreationTime(int $time): int {
 		return $this->fileView->putFileInfo($this->path, ['creation_time' => $time]);
 	}
 
-	public function setUploadTime(int $time) {
-		return $this->fileView->putFileInfo($this->path, ['upload_time' => $time]);
-	}
-
 	/**
 	 * Returns the size of the node, in bytes
 	 *
+	 * @psalm-suppress UnusedPsalmSuppress psalm:strict actually thinks there is no mismatch, idk lol
 	 * @psalm-suppress ImplementedReturnTypeMismatch \Sabre\DAV\IFile::getSize signature does not support 32bit
-	 * @return int|float
 	 */
 	public function getSize(): int|float {
 		return $this->info->getSize();
@@ -203,28 +205,21 @@ abstract class Node implements \Sabre\DAV\INode {
 
 	/**
 	 * Returns the cache's file id
-	 *
-	 * @return int
 	 */
-	public function getId() {
+	public function getId(): ?int {
 		return $this->info->getId();
 	}
 
-	/**
-	 * @return string|null
-	 */
-	public function getFileId() {
-		if ($id = $this->info->getId()) {
+	public function getFileId(): ?string {
+		$id = $this->info->getId();
+		if ($id !== null) {
 			return DavUtil::getDavFileId($id);
 		}
 
 		return null;
 	}
 
-	/**
-	 * @return integer
-	 */
-	public function getInternalFileId() {
+	public function getInternalFileId(): ?int {
 		return $this->info->getId();
 	}
 
@@ -232,30 +227,24 @@ abstract class Node implements \Sabre\DAV\INode {
 		return $this->info->getInternalPath();
 	}
 
-	/**
-	 * @param string $user
-	 * @return int
-	 */
-	public function getSharePermissions($user) {
+	public function getSharePermissions(?string $user): int {
 		// check of we access a federated share
 		if ($user !== null) {
 			try {
-				$share = $this->shareManager->getShareByToken($user);
-				return $share->getPermissions();
-			} catch (ShareNotFound $e) {
+				return $this->shareManager->getShareByToken($user)->getPermissions();
+			} catch (ShareNotFound) {
 				// ignore
 			}
 		}
 
 		try {
 			$storage = $this->info->getStorage();
-		} catch (StorageNotAvailableException $e) {
+		} catch (StorageNotAvailableException) {
 			$storage = null;
 		}
 
 		if ($storage && $storage->instanceOfStorage(ISharedStorage::class)) {
-			/** @var ISharedStorage $storage */
-			$permissions = (int)$storage->getShare()->getPermissions();
+			$permissions = $storage->getShare()->getPermissions();
 		} else {
 			$permissions = $this->info->getPermissions();
 		}
@@ -266,6 +255,10 @@ abstract class Node implements \Sabre\DAV\INode {
 		 */
 		$mountpoint = $this->info->getMountPoint();
 		if (!($mountpoint instanceof MoveableMount)) {
+			/**
+			 * @psalm-suppress UnnecessaryVarAnnotation Rector doesn't trust the return type annotation
+			 * @var string $mountpointpath
+			 */
 			$mountpointpath = $mountpoint->getMountPoint();
 			if (str_ends_with($mountpointpath, '/')) {
 				$mountpointpath = substr($mountpointpath, 0, -1);
@@ -286,25 +279,21 @@ abstract class Node implements \Sabre\DAV\INode {
 		return $permissions;
 	}
 
-	/**
-	 * @return array
-	 */
 	public function getShareAttributes(): array {
 		try {
 			$storage = $this->node->getStorage();
-		} catch (NotFoundException $e) {
+		} catch (NotFoundException) {
 			return [];
 		}
 
 		$attributes = [];
 		if ($storage->instanceOfStorage(ISharedStorage::class)) {
-			/** @var ISharedStorage $storage */
 			$attributes = $storage->getShare()->getAttributes();
 			if ($attributes === null) {
 				return [];
-			} else {
-				return $attributes->toArray();
 			}
+
+			return $attributes->toArray();
 		}
 
 		return $attributes;
@@ -318,63 +307,66 @@ abstract class Node implements \Sabre\DAV\INode {
 		}
 
 		if ($storage->instanceOfStorage(ISharedStorage::class)) {
-			/** @var ISharedStorage $storage */
 			$share = $storage->getShare();
 			if ($user === $share->getShareOwner()) {
 				// Note is only for recipient not the owner
 				return null;
 			}
+
 			return $share->getNote();
 		}
 
 		return null;
 	}
 
-	/**
-	 * @return string
-	 */
-	public function getDavPermissions() {
+	public function getDavPermissions(): string {
 		return DavUtil::getDavPermissions($this->info);
 	}
 
-	public function getOwner() {
+	public function getOwner(): ?IUser {
 		return $this->info->getOwner();
 	}
 
+	/**
+	 * @throws InvalidPath
+	 */
 	protected function verifyPath(?string $path = null): void {
 		try {
-			$path = $path ?? $this->info->getPath();
+			$path ??= $this->info->getPath();
 			$this->fileView->verifyPath(
 				dirname($path),
 				basename($path),
 			);
-		} catch (InvalidPathException $ex) {
-			throw new InvalidPath($ex->getMessage());
+		} catch (InvalidPathException $invalidPathException) {
+			throw new InvalidPath($invalidPathException->getMessage(), false, $invalidPathException);
 		}
 	}
 
 	/**
-	 * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE
+	 * @param ILockingProvider::LOCK_* $type
+	 * @throws LockedException
 	 */
-	public function acquireLock($type) {
+	public function acquireLock($type): void {
 		$this->fileView->lockFile($this->path, $type);
 	}
 
 	/**
-	 * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE
+	 * @param ILockingProvider::LOCK_* $type
+	 * @throws LockedException
 	 */
-	public function releaseLock($type) {
+	public function releaseLock($type): void {
 		$this->fileView->unlockFile($this->path, $type);
 	}
 
 	/**
-	 * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE
+	 * @param ILockingProvider::LOCK_* $type
+	 * @throws LockedException
 	 */
-	public function changeLock($type) {
+	public function changeLock($type): void {
 		$this->fileView->changeLock($this->path, $type);
 	}
 
-	public function getFileInfo() {
+	public function getFileInfo(): FileInfo {
 		return $this->info;
 	}
 

apps/dav/lib/Files/FilesHome.php

@@ -32,12 +32,12 @@ class FilesHome extends Directory {
 		throw new Forbidden('Permission denied to delete home folder');
 	}
 
-	public function getName() {
+	public function getName(): string {
 		[,$name] = \Sabre\Uri\split($this->principalInfo['uri']);
 		return $name;
 	}
 
-	public function setName($name) {
+	public function setName($name): void {
 		throw new Forbidden('Permission denied to rename this folder');
 	}
 }

apps/dav/tests/unit/Connector/Sabre/CommentsPropertiesPluginTest.php

@@ -61,14 +61,14 @@ class CommentsPropertiesPluginTest extends \Test\TestCase {
 
 	public static function baseUriProvider(): array {
 		return [
-			['owncloud/remote.php/webdav/', '4567', 'owncloud/remote.php/dav/comments/files/4567'],
-			['owncloud/remote.php/files/', '4567', 'owncloud/remote.php/dav/comments/files/4567'],
-			['owncloud/wicked.php/files/', '4567', null]
+			['owncloud/remote.php/webdav/', 4567, 'owncloud/remote.php/dav/comments/files/4567'],
+			['owncloud/remote.php/files/', 4567, 'owncloud/remote.php/dav/comments/files/4567'],
+			['owncloud/wicked.php/files/', 4567, null]
 		];
 	}
 
 	#[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'baseUriProvider')]
-	public function testGetCommentsLink(string $baseUri, string $fid, ?string $expectedHref): void {
+	public function testGetCommentsLink(string $baseUri, int $fid, ?string $expectedHref): void {
 		$node = $this->createMock(File::class);
 		$node->expects($this->any())
 			->method('getId')
@@ -94,7 +94,7 @@ class CommentsPropertiesPluginTest extends \Test\TestCase {
 		$node = $this->createMock(File::class);
 		$node->expects($this->any())
 			->method('getId')
-			->willReturn('4567');
+			->willReturn(4567);
 
 		if ($user !== null) {
 			$user = $this->createMock($user);

apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php

@@ -230,6 +230,10 @@ class DirectoryTest extends \Test\TestCase {
 		$info->expects($this->any())
 			->method('isReadable')
 			->willReturn(false);
+		$this->view
+			->method('getRelativePath')
+			->with(null)
+			->willReturn('');
 
 		$dir = new Directory($this->view, $info);
 		$dir->getChildren();
@@ -242,6 +246,10 @@ class DirectoryTest extends \Test\TestCase {
 		$this->info->expects($this->any())
 			->method('isReadable')
 			->willReturn(false);
+		$this->view
+			->method('getRelativePath')
+			->with('/admin/files/folder')
+			->willReturn('');
 
 		$dir = new Directory($this->view, $this->info);
 		$dir->getChild('test');
@@ -254,6 +262,10 @@ class DirectoryTest extends \Test\TestCase {
 		$this->view->expects($this->once())
 			->method('getFileInfo')
 			->willThrowException(new StorageNotAvailableException());
+		$this->view
+			->method('getRelativePath')
+			->with('/admin/files/folder')
+			->willReturn('');
 
 		$dir = new Directory($this->view, $this->info);
 		$dir->getChild('.');
@@ -268,6 +280,10 @@ class DirectoryTest extends \Test\TestCase {
 			->willThrowException(new InvalidPathException());
 		$this->view->expects($this->never())
 			->method('getFileInfo');
+		$this->view
+			->method('getRelativePath')
+			->with('/admin/files/folder')
+			->willReturn('');
 
 		$dir = new Directory($this->view, $this->info);
 		$dir->getChild('.');
@@ -376,6 +392,11 @@ class DirectoryTest extends \Test\TestCase {
 	}
 
 	public function testGetNodeForPathFailsWithNoReadPermissionsForPath(): void {
+		$this->view
+			->method('getRelativePath')
+			->with('/admin/files/')
+			->willReturn('');
+
 		$directoryNode = $this->createMock(Folder::class);
 		$pathNode = $this->createMock(Folder::class);
 		$storage = $this->createMock(IStorage::class);
@@ -396,7 +417,7 @@ class DirectoryTest extends \Test\TestCase {
 					2 => false,
 				};
 			});
-		$directoryNode->expects($this->once())
+		$directoryNode
 			->method('getPath')
 			->willReturn('/admin/files/');
 		$directoryNode->expects($this->once())

apps/dav/tests/unit/Connector/Sabre/FileTest.php

@@ -673,6 +673,10 @@ class FileTest extends TestCase {
 		/** @var View&MockObject */
 		$view = $this->getMockBuilder(View::class)
 			->getMock();
+		$view
+			->method('getRelativePath')
+			->with('/test.txt')
+			->willReturn('');
 
 		$view->expects($this->once())
 			->method('unlink')
@@ -697,6 +701,10 @@ class FileTest extends TestCase {
 		/** @var View&MockObject */
 		$view = $this->getMockBuilder(View::class)
 			->getMock();
+		$view
+			->method('getRelativePath')
+			->with('/test.txt')
+			->willReturn('');
 
 		$info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, [
 			'permissions' => 0,
@@ -717,6 +725,10 @@ class FileTest extends TestCase {
 		/** @var View&MockObject */
 		$view = $this->getMockBuilder(View::class)
 			->getMock();
+		$view
+			->method('getRelativePath')
+			->with('/test.txt')
+			->willReturn('');
 
 		// but fails
 		$view->expects($this->once())
@@ -742,6 +754,10 @@ class FileTest extends TestCase {
 		/** @var View&MockObject */
 		$view = $this->getMockBuilder(View::class)
 			->getMock();
+		$view
+			->method('getRelativePath')
+			->with('/test.txt')
+			->willReturn('');
 
 		// but fails
 		$view->expects($this->once())

apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php

@@ -101,7 +101,7 @@ class FilesPluginTest extends TestCase {
 			->willReturn('00000123instanceid');
 		$node->expects($this->any())
 			->method('getInternalFileId')
-			->willReturn('123');
+			->willReturn(123);
 		$node->expects($this->any())
 			->method('getEtag')
 			->willReturn('"abc"');
@@ -455,7 +455,7 @@ class FilesPluginTest extends TestCase {
 		$node->expects($this->once())
 			->method('setEtag')
 			->with('newetag')
-			->willReturn(true);
+			->willReturn(123);
 
 		$node->expects($this->once())
 			->method('setCreationTime')
@@ -562,35 +562,11 @@ class FilesPluginTest extends TestCase {
 		$this->plugin->checkMove('FolderA/test.txt', 'test.txt');
 	}
 
-	public function testMoveSrcNotExist(): void {
-		$this->expectException(\Sabre\DAV\Exception\NotFound::class);
-		$this->expectExceptionMessage('FolderA/test.txt does not exist');
-
-		$node = $this->createMock(Node::class);
-		$node->expects($this->atLeastOnce())
-			->method('getFileInfo')
-			->willReturn(null);
-
-		$this->tree->expects($this->atLeastOnce())
-			->method('getNodeForPath')
-			->willReturn($node);
-
-		$this->plugin->checkMove('FolderA/test.txt', 'test.txt');
-	}
-
 	public function testMoveDestinationInvalid(): void {
 		$this->expectException(InvalidPath::class);
 		$this->expectExceptionMessage('Mocked exception');
 
-		$fileInfoFolderATestTXT = $this->createMock(FileInfo::class);
-		$fileInfoFolderATestTXT->expects(self::any())
-			->method('isDeletable')
-			->willReturn(true);
-
 		$node = $this->createMock(Node::class);
-		$node->expects($this->atLeastOnce())
-			->method('getFileInfo')
-			->willReturn($fileInfoFolderATestTXT);
 
 		$this->tree->expects($this->atLeastOnce())
 			->method('getNodeForPath')
@@ -604,31 +580,11 @@ class FilesPluginTest extends TestCase {
 		$this->plugin->checkMove('FolderA/test.txt', 'invalid\\path.txt');
 	}
 
-	public function testCopySrcNotExist(): void {
-		$this->expectException(\Sabre\DAV\Exception\NotFound::class);
-		$this->expectExceptionMessage('FolderA/test.txt does not exist');
-
-		$node = $this->createMock(Node::class);
-		$node->expects($this->atLeastOnce())
-			->method('getFileInfo')
-			->willReturn(null);
-
-		$this->tree->expects($this->atLeastOnce())
-			->method('getNodeForPath')
-			->willReturn($node);
-
-		$this->plugin->checkCopy('FolderA/test.txt', 'test.txt');
-	}
-
 	public function testCopyDestinationInvalid(): void {
 		$this->expectException(InvalidPath::class);
 		$this->expectExceptionMessage('Mocked exception');
 
-		$fileInfoFolderATestTXT = $this->createMock(FileInfo::class);
 		$node = $this->createMock(Node::class);
-		$node->expects($this->atLeastOnce())
-			->method('getFileInfo')
-			->willReturn($fileInfoFolderATestTXT);
 
 		$this->tree->expects($this->atLeastOnce())
 			->method('getNodeForPath')

apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php

@@ -57,6 +57,10 @@ class FilesReportPluginTest extends \Test\TestCase {
 
 		$this->tree = $this->createMock(Tree::class);
 		$this->view = $this->createMock(View::class);
+		$this->view
+			->method('getRelativePath')
+			->with(null)
+			->willReturn('');
 
 		$this->server = $this->getMockBuilder(Server::class)
 			->setConstructorArgs([$this->tree])
@@ -315,14 +319,14 @@ class FilesReportPluginTest extends \Test\TestCase {
 
 		$node1->expects($this->once())
 			->method('getInternalFileId')
-			->willReturn('111');
+			->willReturn(111);
 		$node1->expects($this->any())
 			->method('getPath')
 			->willReturn('/node1');
 		$node1->method('getFileInfo')->willReturn($fileInfo);
 		$node2->expects($this->once())
 			->method('getInternalFileId')
-			->willReturn('222');
+			->willReturn(222);
 		$node2->expects($this->once())
 			->method('getSize')
 			->willReturn(1024);

apps/dav/tests/unit/Connector/Sabre/NodeTest.php

@@ -94,6 +94,10 @@ class NodeTest extends \Test\TestCase {
 		$info->method('getStorage')
 			->willReturn($storage);
 		$view = $this->createMock(View::class);
+		$view
+			->method('getRelativePath')
+			->with(null)
+			->willReturn('');
 
 		$node = new  File($view, $info);
 		$this->assertEquals($expected, $node->getDavPermissions());
@@ -169,6 +173,10 @@ class NodeTest extends \Test\TestCase {
 		$info->method('getPermissions')->willReturn($permissions);
 
 		$view = $this->createMock(View::class);
+		$view
+			->method('getRelativePath')
+			->with(null)
+			->willReturn('');
 
 		$node = new File($view, $info);
 		$this->invokePrivate($node, 'shareManager', [$shareManager]);
@@ -204,6 +212,10 @@ class NodeTest extends \Test\TestCase {
 
 		/** @var View&MockObject $view */
 		$view = $this->createMock(View::class);
+		$view
+			->method('getRelativePath')
+			->with(null)
+			->willReturn('');
 
 		$node = new File($view, $info);
 		$this->invokePrivate($node, 'shareManager', [$shareManager]);
@@ -225,6 +237,10 @@ class NodeTest extends \Test\TestCase {
 
 		/** @var View&MockObject */
 		$view = $this->createMock(View::class);
+		$view
+			->method('getRelativePath')
+			->with(null)
+			->willReturn('');
 
 		$node = new File($view, $info);
 		$this->invokePrivate($node, 'shareManager', [$shareManager]);
@@ -243,6 +259,10 @@ class NodeTest extends \Test\TestCase {
 		$view = $this->getMockBuilder(View::class)
 			->disableOriginalConstructor()
 			->getMock();
+		$view
+			->method('getRelativePath')
+			->with(null)
+			->willReturn('');
 		$info = $this->getMockBuilder(FileInfo::class)
 			->disableOriginalConstructor()
 			->getMock();
@@ -263,6 +283,11 @@ class NodeTest extends \Test\TestCase {
 		$this->expectException(\InvalidArgumentException::class);
 
 		$view = $this->createMock(View::class);
+		$view
+			->method('getRelativePath')
+			->with(null)
+			->willReturn('');
+
 		$info = $this->createMock(FileInfo::class);
 
 		$node = new File($view, $info);

apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php

@@ -63,6 +63,10 @@ class ObjectTreeTest extends \Test\TestCase {
 			->method('getFileInfo')
 			->with($targetParent === '' ? '.' : $targetParent)
 			->willReturn($info);
+		$view
+			->method('getRelativePath')
+			->with(null)
+			->willReturn('');
 
 		$rootDir = new Directory($view, $info);
 		$objectTree = $this->getMockBuilder(ObjectTree::class)
@@ -104,6 +108,10 @@ class ObjectTreeTest extends \Test\TestCase {
 			->method('getFileInfo')
 			->with($targetParent === '' ? '.' : $targetParent)
 			->willReturn($info);
+		$view
+			->method('getRelativePath')
+			->with(null)
+			->willReturn('');
 
 		$rootDir = new Directory($view, $info);
 		$objectTree = $this->getMockBuilder(ObjectTree::class)
@@ -141,6 +149,10 @@ class ObjectTreeTest extends \Test\TestCase {
 		$view->method('getFileInfo')
 			->with($fileInfoQueryPath)
 			->willReturn($fileInfo);
+		$view
+			->method('getRelativePath')
+			->with(null)
+			->willReturn('');
 
 		$tree = new ObjectTree();
 		$tree->init($rootNode, $view, $mountManager);

apps/files_sharing/lib/SharedStorage.php

@@ -85,12 +85,6 @@ class SharedStorage extends Jail implements LegacyISharedStorage, ISharedStorage
 
 	private static int $initDepth = 0;
 
-	/**
-	 * @psalm-suppress NonInvariantDocblockPropertyType
-	 * @var ?Storage $storage
-	 */
-	protected $storage;
-
 	public function __construct(array $parameters) {
 		$this->ownerView = $parameters['ownerView'];
 		$this->logger = Server::get(LoggerInterface::class);

apps/files_versions/lib/Storage.php

@@ -433,7 +433,10 @@ class Storage {
 						$target = $storage2->fopen($internalPath2, 'w');
 						$result = $target !== false;
 						if ($result) {
-							[, $result] = Files::streamCopy($source, $target, true);
+							$result = stream_copy_to_stream($source, $target);
+							if ($result !== false) {
+								$result = true;
+							}
 						}
 						// explicit check as S3 library closes streams already
 						if (is_resource($target)) {

apps/files_versions/lib/Versions/IMetadataVersion.php

@@ -16,7 +16,7 @@ interface IMetadataVersion {
 	/**
 	 * retrieves the all the metadata
 	 *
-	 * @return string[]
+	 * @return array<string, string>
 	 * @since 29.0.0
 	 */
 	public function getMetadata(): array;

apps/settings/lib/Service/AuthorizedGroupService.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2021 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -13,9 +15,9 @@ use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
 use OCP\DB\Exception;
 use OCP\IGroup;
+use Throwable;
 
-class AuthorizedGroupService {
-
+readonly class AuthorizedGroupService {
 	public function __construct(
 		private AuthorizedGroupMapper $mapper,
 	) {
@@ -23,6 +25,7 @@ class AuthorizedGroupService {
 
 	/**
 	 * @return AuthorizedGroup[]
+	 * @throws Exception
 	 */
 	public function findAll(): array {
 		return $this->mapper->findAll();
@@ -30,43 +33,40 @@ class AuthorizedGroupService {
 
 	/**
 	 * Find AuthorizedGroup by id.
-	 *
-	 * @param int $id
+	 * @throws DoesNotExistException
+	 * @throws Exception
+	 * @throws MultipleObjectsReturnedException
 	 */
 	public function find(int $id): ?AuthorizedGroup {
 		return $this->mapper->find($id);
 	}
 
 	/**
-	 * @param $e
 	 * @throws NotFoundException
+	 * @throws Throwable
 	 */
-	private function handleException(\Exception $e): void {
+	private function handleException(Throwable $e): void {
 		if ($e instanceof DoesNotExistException
 			|| $e instanceof MultipleObjectsReturnedException) {
 			throw new NotFoundException('AuthorizedGroup not found');
-		} else {
-			throw $e;
 		}
+
+		throw $e;
 	}
 
 	/**
 	 * Create a new AuthorizedGroup
 	 *
-	 * @param string $groupId
-	 * @param string $class
-	 * @return AuthorizedGroup
 	 * @throws Exception
 	 * @throws ConflictException
+	 * @throws MultipleObjectsReturnedException
 	 */
 	public function create(string $groupId, string $class): AuthorizedGroup {
 		// Check if the group is already assigned to this class
 		try {
-			$existing = $this->mapper->findByGroupIdAndClass($groupId, $class);
-			if ($existing) {
-				throw new ConflictException('Group is already assigned to this class');
-			}
-		} catch (DoesNotExistException $e) {
+			$this->mapper->findByGroupIdAndClass($groupId, $class);
+			throw new ConflictException('Group is already assigned to this class');
+		} catch (DoesNotExistException) {
 			// This is expected when no duplicate exists, continue with creation
 		}
 
@@ -78,30 +78,37 @@ class AuthorizedGroupService {
 
 	/**
 	 * @throws NotFoundException
+	 * @throws Throwable
 	 */
 	public function delete(int $id): void {
 		try {
 			$authorizedGroup = $this->mapper->find($id);
 			$this->mapper->delete($authorizedGroup);
-		} catch (\Exception $e) {
-			$this->handleException($e);
+		} catch (\Exception $exception) {
+			$this->handleException($exception);
 		}
 	}
 
+	/**
+	 * @return list<AuthorizedGroup>
+	 */
 	public function findExistingGroupsForClass(string $class): array {
 		try {
-			$authorizedGroup = $this->mapper->findExistingGroupsForClass($class);
-			return $authorizedGroup;
-		} catch (\Exception $e) {
+			return $this->mapper->findExistingGroupsForClass($class);
+		} catch (\Exception) {
 			return [];
 		}
 	}
 
+	/**
+	 * @throws Throwable
+	 * @throws NotFoundException
+	 */
 	public function removeAuthorizationAssociatedTo(IGroup $group): void {
 		try {
 			$this->mapper->removeGroup($group->getGID());
-		} catch (\Exception $e) {
-			$this->handleException($e);
+		} catch (\Exception $exception) {
+			$this->handleException($exception);
 		}
 	}
 }

build/psalm-baseline.xml

@@ -557,12 +557,6 @@
     </ParamNameMismatch>
   </file>
   <file src="apps/dav/lib/Connector/Sabre/File.php">
-    <DeprecatedClass>
-      <code><![CDATA[Files::streamCopy($data, $target, true)]]></code>
-    </DeprecatedClass>
-    <DeprecatedMethod>
-      <code><![CDATA[Files::streamCopy($data, $target, true)]]></code>
-    </DeprecatedMethod>
     <InternalMethod>
       <code><![CDATA[file_exists]]></code>
       <code><![CDATA[file_exists]]></code>
@@ -616,20 +610,11 @@
       <code><![CDATA[lockFile]]></code>
       <code><![CDATA[putFileInfo]]></code>
       <code><![CDATA[putFileInfo]]></code>
-      <code><![CDATA[putFileInfo]]></code>
       <code><![CDATA[rename]]></code>
       <code><![CDATA[touch]]></code>
       <code><![CDATA[unlockFile]]></code>
       <code><![CDATA[verifyPath]]></code>
     </InternalMethod>
-    <InvalidNullableReturnType>
-      <code><![CDATA[int]]></code>
-      <code><![CDATA[integer]]></code>
-    </InvalidNullableReturnType>
-    <NullableReturnStatement>
-      <code><![CDATA[$this->info->getId()]]></code>
-      <code><![CDATA[$this->info->getId()]]></code>
-    </NullableReturnStatement>
   </file>
   <file src="apps/dav/lib/Connector/Sabre/ObjectTree.php">
     <InternalMethod>
@@ -1991,14 +1976,12 @@
   </file>
   <file src="apps/files_versions/lib/Storage.php">
     <DeprecatedClass>
-      <code><![CDATA[Files::streamCopy($source, $target, true)]]></code>
       <code><![CDATA[\OC_Util::setupFS($uid)]]></code>
     </DeprecatedClass>
     <DeprecatedInterface>
       <code><![CDATA[$bus]]></code>
     </DeprecatedInterface>
     <DeprecatedMethod>
-      <code><![CDATA[Files::streamCopy($source, $target, true)]]></code>
       <code><![CDATA[dispatch]]></code>
     </DeprecatedMethod>
     <InternalClass>

build/rector-strict.php

@@ -12,6 +12,13 @@ return (require __DIR__ . '/rector-shared.php')
 		$nextcloudDir . '/build/rector-strict.php',
 		$nextcloudDir . '/core/BackgroundJobs/ExpirePreviewsJob.php',
 		$nextcloudDir . '/lib/public/IContainer.php',
+		$nextcloudDir . '/apps/dav/lib/Connector/Sabre/Node.php',
+		$nextcloudDir . '/apps/files_versions/lib/Versions/IMetadataVersion.php',
+		$nextcloudDir . '/lib/private/Settings/AuthorizedGroup.php',
+		$nextcloudDir . '/lib/private/Settings/AuthorizedGroupMapper.php',
+		$nextcloudDir . '/apps/settings/lib/Service/AuthorizedGroupService.php',
+		$nextcloudDir . '/lib/private/Files/Storage/Storage.php',
+		$nextcloudDir . '/lib/private/Files/Storage/Wrapper/Wrapper.php',
 	])
 	->withPreparedSets(
 		deadCode: true,

lib/private/Files/Storage/Common.php

@@ -212,7 +212,10 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage,
 		} else {
 			$sourceStream = $this->fopen($source, 'r');
 			$targetStream = $this->fopen($target, 'w');
-			[, $result] = Files::streamCopy($sourceStream, $targetStream, true);
+			$result = stream_copy_to_stream($sourceStream, $targetStream);
+			if ($result !== false) {
+				$result = true;
+			}
 			if (!$result) {
 				Server::get(LoggerInterface::class)->warning("Failed to write data while copying $source to $target");
 			}
@@ -743,8 +746,8 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage,
 			throw new GenericFileException("Failed to open $path for writing");
 		}
 		try {
-			[$count, $result] = Files::streamCopy($stream, $target, true);
-			if (!$result) {
+			$count = stream_copy_to_stream($stream, $target);
+			if ($count === false) {
 				throw new GenericFileException('Failed to copy stream');
 			}
 		} finally {

lib/private/Files/Storage/LocalTempFileTrait.php

@@ -7,7 +7,6 @@
  */
 namespace OC\Files\Storage;
 
-use OCP\Files;
 use OCP\ITempManager;
 use OCP\Server;
 
@@ -49,8 +48,12 @@ trait LocalTempFileTrait {
 		}
 		$tmpFile = Server::get(ITempManager::class)->getTemporaryFile($extension);
 		$target = fopen($tmpFile, 'w');
-		Files::streamCopy($source, $target);
+		$result = stream_copy_to_stream($source, $target);
 		fclose($target);
+		if ($result === false) {
+			return false;
+		}
+
 		return $tmpFile;
 	}
 }

lib/private/Files/Storage/Storage.php

@@ -35,6 +35,9 @@ interface Storage extends IStorage, ILockingStorage {
 
 	public function getStorageCache(): \OC\Files\Cache\Storage;
 
+	/**
+	 * @return ?array<string, mixed>
+	 */
 	public function getMetaData(string $path): ?array;
 
 	/**
@@ -49,6 +52,8 @@ interface Storage extends IStorage, ILockingStorage {
 	 * - etag
 	 * - storage_mtime
 	 * - permissions
+	 *
+	 * @return \Traversable<array<string, mixed>>
 	 */
 	public function getDirectoryContent(string $directory): \Traversable;
 }

lib/private/Files/Storage/Wrapper/Encryption.php

@@ -693,7 +693,10 @@ class Encryption extends Wrapper {
 				if ($source === false || $target === false) {
 					$result = false;
 				} else {
-					[, $result] = Files::streamCopy($source, $target, true);
+					$result = stream_copy_to_stream($source, $target);
+					if ($result !== false) {
+						$result = true;
+					}
 				}
 			} finally {
 				if ($source !== false) {
@@ -715,7 +718,7 @@ class Encryption extends Wrapper {
 				$this->getCache()->remove($targetInternalPath);
 			}
 		}
-		return (bool)$result;
+		return $result;
 	}
 
 	public function getLocalFile(string $path): string|false {
@@ -915,7 +918,13 @@ class Encryption extends Wrapper {
 		if ($target === false) {
 			throw new GenericFileException("Failed to open $path for writing");
 		}
-		[$count, $result] = Files::streamCopy($stream, $target, true);
+		$count = stream_copy_to_stream($stream, $target);
+		if ($count === false) {
+			$result = false;
+			$count = 0;
+		} else {
+			$result = true;
+		}
 		fclose($stream);
 		fclose($target);
 

lib/private/Files/Storage/Wrapper/Jail.php

@@ -11,15 +11,17 @@ use OC\Files\Cache\Wrapper\CacheJail;
 use OC\Files\Cache\Wrapper\JailPropagator;
 use OC\Files\Cache\Wrapper\JailWatcher;
 use OC\Files\Filesystem;
-use OCP\Files;
+use OC\Files\Storage\FailedStorage;
 use OCP\Files\Cache\ICache;
 use OCP\Files\Cache\IPropagator;
 use OCP\Files\Cache\IWatcher;
+use OCP\Files\GenericFileException;
 use OCP\Files\Storage\IStorage;
 use OCP\Files\Storage\IWriteStreamStorage;
 use OCP\IDBConnection;
 use OCP\Lock\ILockingProvider;
 use OCP\Server;
+use Psr\Log\LoggerInterface;
 
 /**
  * Jail to a subdirectory of the wrapped storage
@@ -51,6 +53,12 @@ class Jail extends Wrapper {
 	 * This is separate from Wrapper::getWrapperStorage so we can get the jailed storage consistently even if the jail is inside another wrapper
 	 */
 	public function getUnjailedStorage(): IStorage {
+		if ($this->storage === null) {
+			$message = 'storage jail ' . get_class($this) . " doesn't have a wrapped storage set";
+			Server::get(LoggerInterface::class)->error($message);
+			$this->storage = new FailedStorage(['exception' => new \Exception($message)]);
+		}
+
 		return $this->storage;
 	}
 
@@ -258,9 +266,13 @@ class Jail extends Wrapper {
 			return $storage->writeStream($this->getUnjailedPath($path), $stream, $size);
 		} else {
 			$target = $this->fopen($path, 'w');
-			$count = Files::streamCopy($stream, $target);
+			$count = stream_copy_to_stream($stream, $target);
 			fclose($stream);
 			fclose($target);
+			if ($count === false) {
+				throw new GenericFileException('Failed to copy stream.');
+			}
+
 			return $count;
 		}
 	}

lib/private/Files/Storage/Wrapper/Wrapper.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@@ -9,12 +11,12 @@ namespace OC\Files\Storage\Wrapper;
 
 use OC\Files\Storage\FailedStorage;
 use OC\Files\Storage\Storage;
-use OCP\Files;
 use OCP\Files\Cache\ICache;
 use OCP\Files\Cache\IPropagator;
 use OCP\Files\Cache\IScanner;
 use OCP\Files\Cache\IUpdater;
 use OCP\Files\Cache\IWatcher;
+use OCP\Files\GenericFileException;
 use OCP\Files\Storage\ILockingStorage;
 use OCP\Files\Storage\IStorage;
 use OCP\Files\Storage\IWriteStreamStorage;
@@ -24,31 +26,32 @@ use Override;
 use Psr\Log\LoggerInterface;
 
 class Wrapper implements Storage, ILockingStorage, IWriteStreamStorage {
-	/**
-	 * @var Storage $storage
-	 */
-	protected $storage;
+	protected ?Storage $storage = null;
 
-	public $cache;
-	public $scanner;
-	public $watcher;
-	public $propagator;
-	public $updater;
+	public ?ICache $cache = null;
+
+	public ?IScanner $scanner = null;
+
+	public ?IWatcher $watcher = null;
+
+	public ?IPropagator $propagator = null;
+
+	public ?IUpdater $updater = null;
 
 	/**
-	 * @param array $parameters
+	 * @param array{storage: Storage} $parameters
 	 */
 	public function __construct(array $parameters) {
 		$this->storage = $parameters['storage'];
 	}
 
 	public function getWrapperStorage(): Storage {
-		if (!$this->storage) {
-			$message = 'storage wrapper ' . get_class($this) . " doesn't have a wrapped storage set";
-			$logger = Server::get(LoggerInterface::class);
-			$logger->error($message);
+		if (!$this->storage instanceof Storage) {
+			$message = 'storage wrapper ' . static::class . " doesn't have a wrapped storage set";
+			Server::get(LoggerInterface::class)->error($message);
 			$this->storage = new FailedStorage(['exception' => new \Exception($message)]);
 		}
+
 		return $this->storage;
 	}
 
@@ -169,16 +172,18 @@ class Wrapper implements Storage, ILockingStorage, IWriteStreamStorage {
 	}
 
 	public function getCache(string $path = '', ?IStorage $storage = null): ICache {
-		if (!$storage) {
+		if (!$storage instanceof IStorage) {
 			$storage = $this;
 		}
+
 		return $this->getWrapperStorage()->getCache($path, $storage);
 	}
 
 	public function getScanner(string $path = '', ?IStorage $storage = null): IScanner {
-		if (!$storage) {
+		if (!$storage instanceof IStorage) {
 			$storage = $this;
 		}
+
 		return $this->getWrapperStorage()->getScanner($path, $storage);
 	}
 
@@ -187,23 +192,26 @@ class Wrapper implements Storage, ILockingStorage, IWriteStreamStorage {
 	}
 
 	public function getWatcher(string $path = '', ?IStorage $storage = null): IWatcher {
-		if (!$storage) {
+		if (!$storage instanceof IStorage) {
 			$storage = $this;
 		}
+
 		return $this->getWrapperStorage()->getWatcher($path, $storage);
 	}
 
 	public function getPropagator(?IStorage $storage = null): IPropagator {
-		if (!$storage) {
+		if (!$storage instanceof IStorage) {
 			$storage = $this;
 		}
+
 		return $this->getWrapperStorage()->getPropagator($storage);
 	}
 
 	public function getUpdater(?IStorage $storage = null): IUpdater {
-		if (!$storage) {
+		if (!$storage instanceof IStorage) {
 			$storage = $this;
 		}
+
 		return $this->getWrapperStorage()->getUpdater($storage);
 	}
 
@@ -224,10 +232,6 @@ class Wrapper implements Storage, ILockingStorage, IWriteStreamStorage {
 	}
 
 	public function instanceOfStorage(string $class): bool {
-		if (ltrim($class, '\\') === 'OC\Files\Storage\Shared') {
-			// FIXME Temporary fix to keep existing checks working
-			$class = '\OCA\Files_Sharing\SharedStorage';
-		}
 		return is_a($this, $class) || $this->getWrapperStorage()->instanceOfStorage($class);
 	}
 
@@ -242,11 +246,14 @@ class Wrapper implements Storage, ILockingStorage, IWriteStreamStorage {
 			if ($storage instanceof $class) {
 				break;
 			}
+
 			$storage = $storage->getWrapperStorage();
 		}
+
 		if (!($storage instanceof $class)) {
 			return null;
 		}
+
 		return $storage;
 	}
 
@@ -261,6 +268,7 @@ class Wrapper implements Storage, ILockingStorage, IWriteStreamStorage {
 
 	#[Override]
 	public function getDirectDownload(string $path): array|false {
+		/** @psalm-suppress DeprecatedMethod */
 		return $this->getWrapperStorage()->getDirectDownload($path);
 	}
 
@@ -302,20 +310,23 @@ class Wrapper implements Storage, ILockingStorage, IWriteStreamStorage {
 	}
 
 	public function acquireLock(string $path, int $type, ILockingProvider $provider): void {
-		if ($this->getWrapperStorage()->instanceOfStorage('\OCP\Files\Storage\ILockingStorage')) {
-			$this->getWrapperStorage()->acquireLock($path, $type, $provider);
+		$storage = $this->getWrapperStorage();
+		if ($storage->instanceOfStorage(ILockingStorage::class)) {
+			$storage->acquireLock($path, $type, $provider);
 		}
 	}
 
 	public function releaseLock(string $path, int $type, ILockingProvider $provider): void {
-		if ($this->getWrapperStorage()->instanceOfStorage('\OCP\Files\Storage\ILockingStorage')) {
-			$this->getWrapperStorage()->releaseLock($path, $type, $provider);
+		$storage = $this->getWrapperStorage();
+		if ($storage->instanceOfStorage(ILockingStorage::class)) {
+			$storage->releaseLock($path, $type, $provider);
 		}
 	}
 
 	public function changeLock(string $path, int $type, ILockingProvider $provider): void {
-		if ($this->getWrapperStorage()->instanceOfStorage('\OCP\Files\Storage\ILockingStorage')) {
-			$this->getWrapperStorage()->changeLock($path, $type, $provider);
+		$storage = $this->getWrapperStorage();
+		if ($storage->instanceOfStorage(ILockingStorage::class)) {
+			$storage->changeLock($path, $type, $provider);
 		}
 	}
 
@@ -328,13 +339,21 @@ class Wrapper implements Storage, ILockingStorage, IWriteStreamStorage {
 		if ($storage->instanceOfStorage(IWriteStreamStorage::class)) {
 			/** @var IWriteStreamStorage $storage */
 			return $storage->writeStream($path, $stream, $size);
-		} else {
-			$target = $this->fopen($path, 'w');
-			$count = Files::streamCopy($stream, $target);
-			fclose($stream);
-			fclose($target);
-			return $count;
 		}
+
+		$target = $this->fopen($path, 'w');
+		if ($target === false) {
+			throw new GenericFileException('Failed to open ' . $path);
+		}
+
+		$count = stream_copy_to_stream($stream, $target);
+		fclose($stream);
+		fclose($target);
+		if ($count === false) {
+			throw new GenericFileException('Failed to copy stream.');
+		}
+
+		return $count;
 	}
 
 	public function getDirectoryContent(string $directory): \Traversable {
@@ -346,9 +365,11 @@ class Wrapper implements Storage, ILockingStorage, IWriteStreamStorage {
 		if ($wrapped === $storage) {
 			return true;
 		}
+
 		if ($wrapped instanceof Wrapper) {
 			return $wrapped->isWrapperOf($storage);
 		}
+
 		return false;
 	}
 

lib/private/Files/View.php

@@ -639,7 +639,10 @@ class View {
 				[$storage, $internalPath] = $this->resolvePath($path);
 				$target = $storage->fopen($internalPath, 'w');
 				if ($target) {
-					[, $result] = Files::streamCopy($data, $target, true);
+					$result = stream_copy_to_stream($data, $target);
+					if ($result !== false) {
+						$result = true;
+					}
 					fclose($target);
 					fclose($data);
 

lib/private/Group/Manager.php

@@ -391,15 +391,6 @@ class Manager extends PublicEmitter implements IGroupManager {
 		}, $this->getUserGroups($user));
 	}
 
-	/**
-	 * get a list of all display names in a group
-	 *
-	 * @param string $gid
-	 * @param string $search
-	 * @param int $limit
-	 * @param int $offset
-	 * @return array an array of display names (value) and user ids (key)
-	 */
 	public function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0) {
 		$group = $this->get($gid);
 		if (is_null($group)) {

lib/private/Settings/AuthorizedGroup.php

@@ -8,21 +8,25 @@ declare(strict_types=1);
  */
 namespace OC\Settings;
 
+use JsonSerializable;
 use OCP\AppFramework\Db\Entity;
 
 /**
  * @method setGroupId(string $groupId)
  * @method setClass(string $class)
- * @method getGroupId(): string
- * @method getClass(): string
+ * @method string getGroupId()
+ * @method string getClass()
  */
-class AuthorizedGroup extends Entity implements \JsonSerializable {
-	/** @var string $group_id */
-	protected $groupId;
+class AuthorizedGroup extends Entity implements JsonSerializable {
+	public $id;
 
-	/** @var string $class */
-	protected $class;
+	protected ?string $groupId = null;
 
+	protected ?string $class = null;
+
+	/**
+	 * @return array<string, mixed>
+	 */
 	public function jsonSerialize(): array {
 		return [
 			'id' => $this->getId(),

lib/private/Settings/AuthorizedGroupMapper.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2021 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -7,7 +9,6 @@
 namespace OC\Settings;
 
 use OCP\AppFramework\Db\DoesNotExistException;
-use OCP\AppFramework\Db\Entity;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
 use OCP\AppFramework\Db\QBMapper;
 use OCP\DB\Exception;
@@ -32,48 +33,40 @@ class AuthorizedGroupMapper extends QBMapper {
 	public function findAllClassesForUser(IUser $user): array {
 		$qb = $this->db->getQueryBuilder();
 
-		/** @var IGroupManager $groupManager */
 		$groupManager = Server::get(IGroupManager::class);
 		$groups = $groupManager->getUserGroups($user);
 		if (count($groups) === 0) {
 			return [];
 		}
 
-		$result = $qb->select('class')
+		/** @var list<string> $rows */
+		$rows = $qb->select('class')
 			->from($this->getTableName(), 'auth')
-			->where($qb->expr()->in('group_id', array_map(function (IGroup $group) use ($qb) {
-				return $qb->createNamedParameter($group->getGID());
-			}, $groups), IQueryBuilder::PARAM_STR))
-			->executeQuery();
+			->where($qb->expr()->in('group_id', array_map(static fn (IGroup $group) => $qb->createNamedParameter($group->getGID()), $groups), IQueryBuilder::PARAM_STR))
+			->executeQuery()
+			->fetchFirstColumn();
 
-		$classes = [];
-		while ($row = $result->fetch()) {
-			$classes[] = $row['class'];
-		}
-		$result->closeCursor();
-		return $classes;
+		return $rows;
 	}
 
 	/**
 	 * @throws DoesNotExistException
 	 * @throws MultipleObjectsReturnedException
-	 * @throws \OCP\DB\Exception
+	 * @throws Exception
 	 */
 	public function find(int $id): AuthorizedGroup {
 		$queryBuilder = $this->db->getQueryBuilder();
 		$queryBuilder->select('*')
 			->from($this->getTableName())
 			->where($queryBuilder->expr()->eq('id', $queryBuilder->createNamedParameter($id)));
-		/** @var AuthorizedGroup $authorizedGroup */
-		$authorizedGroup = $this->findEntity($queryBuilder);
-		return $authorizedGroup;
+		return $this->findEntity($queryBuilder);
 	}
 
 	/**
 	 * Get all the authorizations stored in the database.
 	 *
 	 * @return AuthorizedGroup[]
-	 * @throws \OCP\DB\Exception
+	 * @throws Exception
 	 */
 	public function findAll(): array {
 		$qb = $this->db->getQueryBuilder();
@@ -81,7 +74,12 @@ class AuthorizedGroupMapper extends QBMapper {
 		return $this->findEntities($qb);
 	}
 
-	public function findByGroupIdAndClass(string $groupId, string $class) {
+	/**
+	 * @throws DoesNotExistException
+	 * @throws Exception
+	 * @throws MultipleObjectsReturnedException
+	 */
+	public function findByGroupIdAndClass(string $groupId, string $class): AuthorizedGroup {
 		$qb = $this->db->getQueryBuilder();
 		$qb->select('*')
 			->from($this->getTableName())
@@ -91,8 +89,8 @@ class AuthorizedGroupMapper extends QBMapper {
 	}
 
 	/**
-	 * @return Entity[]
-	 * @throws \OCP\DB\Exception
+	 * @return list<AuthorizedGroup>
+	 * @throws Exception
 	 */
 	public function findExistingGroupsForClass(string $class): array {
 		$qb = $this->db->getQueryBuilder();
@@ -105,7 +103,7 @@ class AuthorizedGroupMapper extends QBMapper {
 	/**
 	 * @throws Exception
 	 */
-	public function removeGroup(string $gid) {
+	public function removeGroup(string $gid): void {
 		$qb = $this->db->getQueryBuilder();
 		$qb->delete($this->getTableName())
 			->where($qb->expr()->eq('group_id', $qb->createNamedParameter($gid)))

lib/public/Files.php

@@ -10,8 +10,6 @@
 
 namespace OCP;
 
-use OCP\Files\IMimeTypeDetector;
-
 /**
  * This class provides access to the internal filesystem abstraction layer. Use
  * this class exclusively if you want to access files
@@ -60,70 +58,4 @@ class Files {
 
 		return !file_exists($dir);
 	}
-
-	/**
-	 * Get the mimetype form a local file
-	 * @param string $path
-	 * @return string
-	 *                does NOT work for ownClouds filesystem, use OC_FileSystem::getMimeType instead
-	 * @since 5.0.0
-	 * @deprecated 14.0.0
-	 */
-	public static function getMimeType($path) {
-		return Server::get(IMimeTypeDetector::class)->detect($path);
-	}
-
-	/**
-	 * Search for files by mimetype
-	 * @param string $mimetype
-	 * @return array
-	 * @since 6.0.0
-	 * @deprecated 14.0.0
-	 */
-	public static function searchByMime($mimetype) {
-		return \OC\Files\Filesystem::searchByMime($mimetype);
-	}
-
-	/**
-	 * Copy the contents of one stream to another
-	 *
-	 * @template T of null|true
-	 * @param resource $source
-	 * @param resource $target
-	 * @param T $includeResult
-	 * @return int|array
-	 * @psalm-return (T is true ? array{0: int, 1: bool} : int)
-	 * @since 5.0.0
-	 * @since 32.0.0 added $includeResult parameter
-	 * @deprecated 14.0.0
-	 */
-	public static function streamCopy($source, $target, ?bool $includeResult = null) {
-		if (!$source || !$target) {
-			return $includeResult ? [0, false] : 0;
-		}
-
-		$bufSize = 8192;
-		$count = 0;
-		$result = true;
-		while (!feof($source)) {
-			$buf = fread($source, $bufSize);
-			if ($buf === false) {
-				$result = false;
-				break;
-			}
-
-			$bytesWritten = fwrite($target, $buf);
-			if ($bytesWritten !== false) {
-				$count += $bytesWritten;
-			}
-
-			if ($bytesWritten === false
-				|| ($bytesWritten < $bufSize && $bytesWritten < strlen($buf))
-			) {
-				$result = false;
-				break;
-			}
-		}
-		return $includeResult ? [$count, $result] : $count;
-	}
 }

lib/public/Files/Storage/IStorage.php

@@ -81,7 +81,7 @@ interface IStorage {
 	 * see https://www.php.net/manual/en/function.stat.php
 	 * only the following keys are required in the result: size and mtime
 	 *
-	 * @return array|false
+	 * @return array<int|string, mixed>|false
 	 * @since 9.0.0
 	 */
 	public function stat(string $path);

lib/public/IGroupManager.php

@@ -101,7 +101,7 @@ interface IGroupManager {
 	 * @param string $search
 	 * @param int $limit
 	 * @param int $offset
-	 * @return array an array of display names (value) and user ids (key)
+	 * @return array<string, string> ['user id' => 'display name']
 	 * @since 8.0.0
 	 */
 	public function displayNamesInGroup($gid, $search = '', $limit = -1, $offset = 0);

psalm-strict.xml

@@ -18,6 +18,13 @@
 	<projectFiles>
 		<file name="core/BackgroundJobs/ExpirePreviewsJob.php"/>
 		<file name="lib/public/IContainer.php"/>
+		<file name="apps/dav/lib/Connector/Sabre/Node.php"/>
+		<file name="apps/files_versions/lib/Versions/IMetadataVersion.php"/>
+		<file name="lib/private/Settings/AuthorizedGroup.php"/>
+		<file name="lib/private/Settings/AuthorizedGroupMapper.php"/>
+		<file name="apps/settings/lib/Service/AuthorizedGroupService.php"/>
+		<file name="lib/private/Files/Storage/Storage.php"/>
+		<file name="lib/private/Files/Storage/Wrapper/Wrapper.php"/>
 		<ignoreFiles>
 			<directory name="apps/**/composer"/>
 			<directory name="apps/**/tests"/>
@@ -27,6 +34,29 @@
 		</ignoreFiles>
 	</projectFiles>
 	<extraFiles>
+		<directory name="apps/dav/lib"/>
+		<directory name="apps/settings/lib"/>
 		<directory name="3rdparty"/>
 	</extraFiles>
+	<stubs>
+		<!-- Psalm does not find methods in here through <extraFiles/> 🤷‍♀️ -->
+		<file name="3rdparty/sabre/uri/lib/functions.php"/>
+	</stubs>
+	<issueHandlers>
+		<InternalClass>
+			<errorLevel type="suppress">
+				<directory name="."/>
+			</errorLevel>
+		</InternalClass>
+		<InternalMethod>
+			<errorLevel type="suppress">
+				<directory name="."/>
+			</errorLevel>
+		</InternalMethod>
+		<InternalProperty>
+			<errorLevel type="suppress">
+				<directory name="."/>
+			</errorLevel>
+		</InternalProperty>
+	</issueHandlers>
 </psalm>

tests/lib/Archive/TestBase.php

@@ -96,7 +96,8 @@ abstract class TestBase extends \Test\TestCase {
 		$this->instance = $this->getNew();
 		$fh = $this->instance->getStream('lorem.txt', 'w');
 		$source = fopen($dir . '/lorem.txt', 'r');
-		Files::streamCopy($source, $fh);
+		$result = stream_copy_to_stream($source, $fh);
+		$this->assertNotFalse($result);
 		fclose($source);
 		fclose($fh);
 		$this->assertTrue($this->instance->fileExists('lorem.txt'));

tests/lib/Encryption/EncryptionWrapperTest.php

@@ -15,7 +15,6 @@ use OC\Memcache\ArrayCache;
 use OCA\Files_Trashbin\Storage;
 use OCP\Files\Mount\IMountPoint;
 use OCP\Files\Storage\IDisableEncryptionStorage;
-use OCP\Files\Storage\IStorage;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
 
@@ -45,17 +44,13 @@ class EncryptionWrapperTest extends TestCase {
 
 	#[\PHPUnit\Framework\Attributes\DataProvider('provideWrapStorage')]
 	public function testWrapStorage($expectedWrapped, $wrappedStorages): void {
-		$storage = $this->getMockBuilder(IStorage::class)
+		$storage = $this->getMockBuilder(Storage::class)
 			->disableOriginalConstructor()
 			->getMock();
 
-		foreach ($wrappedStorages as $wrapper) {
-			$storage->expects($this->any())
-				->method('instanceOfStorage')
-				->willReturnMap([
-					[$wrapper, true],
-				]);
-		}
+		$storage->expects($this->any())
+			->method('instanceOfStorage')
+			->willReturnCallback(fn (string $storage): bool => in_array($storage, $wrappedStorages, true));
 
 		$mount = $this->getMockBuilder(IMountPoint::class)
 			->disableOriginalConstructor()

tests/lib/Files/Storage/Wrapper/EncryptionTest.php

@@ -958,7 +958,7 @@ class EncryptionTest extends Storage {
 		$wrapper = $this->getMockBuilder(Encryption::class)
 			->setConstructorArgs(
 				[
-					['mountPoint' => '', 'mount' => $mount, 'storage' => ''],
+					['mountPoint' => '', 'mount' => $mount, 'storage' => null],
 					$encryptionManager,
 					$util,
 					$this->logger,

tests/lib/Files/Storage/Wrapper/QuotaTest.php

@@ -115,9 +115,8 @@ class QuotaTest extends \Test\Files\Storage\Storage {
 		$instance = $this->getLimitedStorage(16);
 		$inputStream = fopen('data://text/plain,foobarqwerty', 'r');
 		$outputStream = $instance->fopen('files/foo', 'w+');
-		[$count, $result] = Files::streamCopy($inputStream, $outputStream, true);
+		$count = stream_copy_to_stream($inputStream, $outputStream);
 		$this->assertEquals(12, $count);
-		$this->assertTrue($result);
 		fclose($inputStream);
 		fclose($outputStream);
 	}
@@ -126,9 +125,8 @@ class QuotaTest extends \Test\Files\Storage\Storage {
 		$instance = $this->getLimitedStorage(9);
 		$inputStream = fopen('data://text/plain,foobarqwerty', 'r');
 		$outputStream = $instance->fopen('files/foo', 'w+');
-		[$count, $result] = Files::streamCopy($inputStream, $outputStream, true);
-		$this->assertEquals(9, $count);
-		$this->assertFalse($result);
+		$count = stream_copy_to_stream($inputStream, $outputStream);
+		$this->assertFalse($count);
 		fclose($inputStream);
 		fclose($outputStream);
 	}

tests/lib/FilesTest.php

@@ -39,36 +39,4 @@ class FilesTest extends TestCase {
 		Files::rmdirr($baseDir);
 		$this->assertFalse(file_exists($baseDir));
 	}
-
-	#[\PHPUnit\Framework\Attributes\DataProvider('streamCopyDataProvider')]
-	public function testStreamCopy($expectedCount, $expectedResult, $source, $target): void {
-		if (is_string($source)) {
-			$source = fopen($source, 'r');
-		}
-		if (is_string($target)) {
-			$target = fopen($target, 'w');
-		}
-
-		[$count, $result] = Files::streamCopy($source, $target, true);
-
-		if (is_resource($source)) {
-			fclose($source);
-		}
-		if (is_resource($target)) {
-			fclose($target);
-		}
-
-		$this->assertSame($expectedCount, $count);
-		$this->assertSame($expectedResult, $result);
-	}
-
-
-	public static function streamCopyDataProvider(): array {
-		return [
-			[0, false, false, false],
-			[0, false, \OC::$SERVERROOT . '/tests/data/lorem.txt', false],
-			[filesize(\OC::$SERVERROOT . '/tests/data/lorem.txt'), true, \OC::$SERVERROOT . '/tests/data/lorem.txt', \OC::$SERVERROOT . '/tests/data/lorem-copy.txt'],
-			[3670, true, \OC::$SERVERROOT . '/tests/data/testimage.png', \OC::$SERVERROOT . '/tests/data/testimage-copy.png'],
-		];
-	}
 }