Add typing to SimpleFS

- Fix putContent sometimes return a bool and sometimes nothing

Signed-off-by: Carl Schwan <carl@carlschwan.eu>

lib/private/Files/SimpleFS/NewSimpleFile.php

@@ -34,15 +34,12 @@ use OCP\Files\NotPermittedException;
 use OCP\Files\SimpleFS\ISimpleFile;
 
 class NewSimpleFile implements ISimpleFile {
-	private $parentFolder;
-	private $name;
-	/** @var File|null */
-	private $file = null;
+	private Folder $parentFolder;
+	private string $name;
+	private ?File $file = null;
 
 	/**
 	 * File constructor.
-	 *
-	 * @param File $file
 	 */
 	public function __construct(Folder $parentFolder, string $name) {
 		$this->parentFolder = $parentFolder;
@@ -51,19 +48,15 @@ class NewSimpleFile implements ISimpleFile {
 
 	/**
 	 * Get the name
-	 *
-	 * @return string
 	 */
-	public function getName() {
+	public function getName(): string {
 		return $this->name;
 	}
 
 	/**
 	 * Get the size in bytes
-	 *
-	 * @return int
 	 */
-	public function getSize() {
+	public function getSize(): int {
 		if ($this->file) {
 			return $this->file->getSize();
 		} else {
@@ -73,10 +66,8 @@ class NewSimpleFile implements ISimpleFile {
 
 	/**
 	 * Get the ETag
-	 *
-	 * @return string
 	 */
-	public function getETag() {
+	public function getETag(): string {
 		if ($this->file) {
 			return $this->file->getEtag();
 		} else {
@@ -86,10 +77,8 @@ class NewSimpleFile implements ISimpleFile {
 
 	/**
 	 * Get the last modification time
-	 *
-	 * @return int
 	 */
-	public function getMTime() {
+	public function getMTime(): int {
 		if ($this->file) {
 			return $this->file->getMTime();
 		} else {
@@ -100,11 +89,10 @@ class NewSimpleFile implements ISimpleFile {
 	/**
 	 * Get the content
 	 *
-	 * @return string
 	 * @throws NotFoundException
 	 * @throws NotPermittedException
 	 */
-	public function getContent() {
+	public function getContent(): string {
 		if ($this->file) {
 			$result = $this->file->getContent();
 
@@ -125,7 +113,7 @@ class NewSimpleFile implements ISimpleFile {
 	 * @throws NotPermittedException
 	 * @throws NotFoundException
 	 */
-	public function putContent($data) {
+	public function putContent($data): void {
 		try {
 			if ($this->file) {
 				$this->file->putContent($data);
@@ -147,7 +135,7 @@ class NewSimpleFile implements ISimpleFile {
 	 *
 	 * @throws NotFoundException
 	 */
-	private function checkFile() {
+	private function checkFile(): void {
 		$cur = $this->file;
 
 		while ($cur->stat() === false) {
@@ -171,7 +159,7 @@ class NewSimpleFile implements ISimpleFile {
 	 *
 	 * @throws NotPermittedException
 	 */
-	public function delete() {
+	public function delete(): void {
 		if ($this->file) {
 			$this->file->delete();
 		}
@@ -182,7 +170,7 @@ class NewSimpleFile implements ISimpleFile {
 	 *
 	 * @return string
 	 */
-	public function getMimeType() {
+	public function getMimeType(): string {
 		if ($this->file) {
 			return $this->file->getMimeType();
 		} else {

lib/private/Files/SimpleFS/SimpleFile.php

@@ -30,52 +30,37 @@ use OCP\Files\NotPermittedException;
 use OCP\Files\SimpleFS\ISimpleFile;
 
 class SimpleFile implements ISimpleFile {
+	private File $file;
 
-	/** @var File $file */
-	private $file;
-
-	/**
-	 * File constructor.
-	 *
-	 * @param File $file
-	 */
 	public function __construct(File $file) {
 		$this->file = $file;
 	}
 
 	/**
 	 * Get the name
-	 *
-	 * @return string
 	 */
-	public function getName() {
+	public function getName(): string {
 		return $this->file->getName();
 	}
 
 	/**
 	 * Get the size in bytes
-	 *
-	 * @return int
 	 */
-	public function getSize() {
+	public function getSize(): int {
 		return $this->file->getSize();
 	}
 
 	/**
 	 * Get the ETag
-	 *
-	 * @return string
 	 */
-	public function getETag() {
+	public function getETag(): string {
 		return $this->file->getEtag();
 	}
 
 	/**
 	 * Get the last modification time
-	 *
-	 * @return int
 	 */
-	public function getMTime() {
+	public function getMTime(): int {
 		return $this->file->getMTime();
 	}
 
@@ -84,9 +69,8 @@ class SimpleFile implements ISimpleFile {
 	 *
 	 * @throws NotPermittedException
 	 * @throws NotFoundException
-	 * @return string
 	 */
-	public function getContent() {
+	public function getContent(): string {
 		$result = $this->file->getContent();
 
 		if ($result === false) {
@@ -103,9 +87,9 @@ class SimpleFile implements ISimpleFile {
 	 * @throws NotPermittedException
 	 * @throws NotFoundException
 	 */
-	public function putContent($data) {
+	public function putContent($data): void {
 		try {
-			return $this->file->putContent($data);
+			$this->file->putContent($data);
 		} catch (NotFoundException $e) {
 			$this->checkFile();
 		}
@@ -121,7 +105,7 @@ class SimpleFile implements ISimpleFile {
 	 *
 	 * @throws NotFoundException
 	 */
-	private function checkFile() {
+	private function checkFile(): void {
 		$cur = $this->file;
 
 		while ($cur->stat() === false) {
@@ -145,16 +129,14 @@ class SimpleFile implements ISimpleFile {
 	 *
 	 * @throws NotPermittedException
 	 */
-	public function delete() {
+	public function delete(): void {
 		$this->file->delete();
 	}
 
 	/**
 	 * Get the MimeType
-	 *
-	 * @return string
 	 */
-	public function getMimeType() {
+	public function getMimeType(): string {
 		return $this->file->getMimeType();
 	}
 
@@ -179,7 +161,7 @@ class SimpleFile implements ISimpleFile {
 	/**
 	 * Open the file as stream for writing, resulting resource can be operated as stream like the result from php's own fopen
 	 *
-	 * @return resource
+	 * @return resource|false
 	 * @throws \OCP\Files\NotPermittedException
 	 * @since 14.0.0
 	 */

lib/private/Files/SimpleFS/SimpleFolder.php

@@ -29,6 +29,7 @@ use OCP\Files\Folder;
 use OCP\Files\Node;
 use OCP\Files\NotFoundException;
 use OCP\Files\SimpleFS\ISimpleFolder;
+use OCP\Files\SimpleFS\ISimpleFile;
 
 class SimpleFolder implements ISimpleFolder {
 
@@ -44,11 +45,11 @@ class SimpleFolder implements ISimpleFolder {
 		$this->folder = $folder;
 	}
 
-	public function getName() {
+	public function getName(): string {
 		return $this->folder->getName();
 	}
 
-	public function getDirectoryListing() {
+	public function getDirectoryListing(): array {
 		$listing = $this->folder->getDirectoryListing();
 
 		$fileListing = array_map(function (Node $file) {
@@ -63,15 +64,15 @@ class SimpleFolder implements ISimpleFolder {
 		return array_values($fileListing);
 	}
 
-	public function delete() {
+	public function delete(): void {
 		$this->folder->delete();
 	}
 
-	public function fileExists($name) {
+	public function fileExists(string $name): bool {
 		return $this->folder->nodeExists($name);
 	}
 
-	public function getFile($name) {
+	public function getFile(string $name): ISimpleFile {
 		$file = $this->folder->get($name);
 
 		if (!($file instanceof File)) {
@@ -81,7 +82,7 @@ class SimpleFolder implements ISimpleFolder {
 		return new SimpleFile($file);
 	}
 
-	public function newFile($name, $content = null) {
+	public function newFile(string $name, $content = null): ISimpleFile {
 		if ($content === null) {
 			// delay creating the file until it's written to
 			return new NewSimpleFile($this->folder, $name);

lib/public/Files/SimpleFS/ISimpleFile.php

@@ -41,44 +41,39 @@ interface ISimpleFile {
 	/**
 	 * Get the name
 	 *
-	 * @return string
 	 * @since 11.0.0
 	 */
-	public function getName();
+	public function getName(): string;
 
 	/**
 	 * Get the size in bytes
 	 *
-	 * @return int
 	 * @since 11.0.0
 	 */
-	public function getSize();
+	public function getSize(): int;
 
 	/**
 	 * Get the ETag
 	 *
-	 * @return string
 	 * @since 11.0.0
 	 */
-	public function getETag();
+	public function getETag(): string;
 
 	/**
 	 * Get the last modification time
 	 *
-	 * @return int
 	 * @since 11.0.0
 	 */
-	public function getMTime();
+	public function getMTime(): int;
 
 	/**
 	 * Get the content
 	 *
 	 * @throws NotPermittedException
 	 * @throws NotFoundException
-	 * @return string
 	 * @since 11.0.0
 	 */
-	public function getContent();
+	public function getContent(): string;
 
 	/**
 	 * Overwrite the file
@@ -88,7 +83,7 @@ interface ISimpleFile {
 	 * @throws NotFoundException
 	 * @since 11.0.0
 	 */
-	public function putContent($data);
+	public function putContent($data): void;
 
 	/**
 	 * Delete the file
@@ -96,15 +91,14 @@ interface ISimpleFile {
 	 * @throws NotPermittedException
 	 * @since 11.0.0
 	 */
-	public function delete();
+	public function delete(): void;
 
 	/**
 	 * Get the MimeType
 	 *
-	 * @return string
 	 * @since 11.0.0
 	 */
-	public function getMimeType();
+	public function getMimeType(): string;
 
 	/**
 	 * @since 24.0.0

lib/public/Files/SimpleFS/ISimpleFolder.php

@@ -38,7 +38,7 @@ interface ISimpleFolder {
 	 * @return ISimpleFile[]
 	 * @since 11.0.0
 	 */
-	public function getDirectoryListing();
+	public function getDirectoryListing(): array;
 
 	/**
 	 * Check if a file with $name exists
@@ -47,28 +47,24 @@ interface ISimpleFolder {
 	 * @return bool
 	 * @since 11.0.0
 	 */
-	public function fileExists($name);
+	public function fileExists(string $name): bool;
 
 	/**
 	 * Get the file named $name from the folder
 	 *
-	 * @param string $name
-	 * @return ISimpleFile
 	 * @throws NotFoundException
 	 * @since 11.0.0
 	 */
-	public function getFile($name);
+	public function getFile(string $name): ISimpleFile;
 
 	/**
 	 * Creates a new file with $name in the folder
 	 *
-	 * @param string $name
 	 * @param string|resource|null $content @since 19.0.0
-	 * @return ISimpleFile
 	 * @throws NotPermittedException
 	 * @since 11.0.0
 	 */
-	public function newFile($name, $content = null);
+	public function newFile(string $name, $content = null): ISimpleFile;
 
 	/**
 	 * Remove the folder and all the files in it
@@ -76,13 +72,12 @@ interface ISimpleFolder {
 	 * @throws NotPermittedException
 	 * @since 11.0.0
 	 */
-	public function delete();
+	public function delete(): void;
 
 	/**
 	 * Get the folder name
 	 *
-	 * @return string
 	 * @since 11.0.0
 	 */
-	public function getName();
+	public function getName(): string;
 }

lib/public/Files/SimpleFS/ISimpleRoot.php

@@ -35,8 +35,6 @@ interface ISimpleRoot {
 	/**
 	 * Get the folder with name $name
 	 *
-	 * @param string $name
-	 * @return ISimpleFolder
 	 * @throws NotFoundException
 	 * @throws \RuntimeException
 	 * @since 11.0.0
@@ -56,8 +54,6 @@ interface ISimpleRoot {
 	/**
 	 * Create a new folder named $name
 	 *
-	 * @param string $name
-	 * @return ISimpleFolder
 	 * @throws NotPermittedException
 	 * @throws \RuntimeException
 	 * @since 11.0.0

lib/public/Files/SimpleFS/InMemoryFile.php

@@ -36,17 +36,13 @@ use OCP\Files\NotPermittedException;
 class InMemoryFile implements ISimpleFile {
 	/**
 	 * Holds the file name.
-	 *
-	 * @var string
 	 */
-	private $name;
+	private string $name;
 
 	/**
 	 * Holds the file contents.
-	 *
-	 * @var string
 	 */
-	private $contents;
+	private string $contents;
 
 	/**
 	 * InMemoryFile constructor.
@@ -64,7 +60,7 @@ class InMemoryFile implements ISimpleFile {
 	 * @inheritdoc
 	 * @since 16.0.0
 	 */
-	public function getName() {
+	public function getName(): string {
 		return $this->name;
 	}
 
@@ -72,7 +68,7 @@ class InMemoryFile implements ISimpleFile {
 	 * @inheritdoc
 	 * @since 16.0.0
 	 */
-	public function getSize() {
+	public function getSize(): int {
 		return strlen($this->contents);
 	}
 
@@ -80,7 +76,7 @@ class InMemoryFile implements ISimpleFile {
 	 * @inheritdoc
 	 * @since 16.0.0
 	 */
-	public function getETag() {
+	public function getETag(): string {
 		return '';
 	}
 
@@ -88,7 +84,7 @@ class InMemoryFile implements ISimpleFile {
 	 * @inheritdoc
 	 * @since 16.0.0
 	 */
-	public function getMTime() {
+	public function getMTime(): int {
 		return time();
 	}
 
@@ -96,7 +92,7 @@ class InMemoryFile implements ISimpleFile {
 	 * @inheritdoc
 	 * @since 16.0.0
 	 */
-	public function getContent() {
+	public function getContent(): string {
 		return $this->contents;
 	}
 
@@ -104,7 +100,7 @@ class InMemoryFile implements ISimpleFile {
 	 * @inheritdoc
 	 * @since 16.0.0
 	 */
-	public function putContent($data) {
+	public function putContent($data): void {
 		$this->contents = $data;
 	}
 
@@ -113,7 +109,7 @@ class InMemoryFile implements ISimpleFile {
 	 *
 	 * @since 16.0.0
 	 */
-	public function delete() {
+	public function delete(): void {
 		// unimplemented for in memory files
 	}
 
@@ -121,7 +117,7 @@ class InMemoryFile implements ISimpleFile {
 	 * @inheritdoc
 	 * @since 16.0.0
 	 */
-	public function getMimeType() {
+	public function getMimeType(): string {
 		$fileInfo = new \finfo(FILEINFO_MIME_TYPE);
 		return $fileInfo->buffer($this->contents);
 	}

tests/lib/Avatar/UserAvatarTest.php

@@ -11,7 +11,6 @@ namespace Test\Avatar;
 use OC\Files\SimpleFS\SimpleFolder;
 use OC\User\User;
 use OCP\Files\File;
-use OCP\Files\Folder;
 use OCP\Files\NotFoundException;
 use OCP\Files\SimpleFS\ISimpleFile;
 use OCP\IConfig;
@@ -19,7 +18,7 @@ use OCP\IL10N;
 use Psr\Log\LoggerInterface;
 
 class UserAvatarTest extends \Test\TestCase {
-	/** @var Folder | \PHPUnit\Framework\MockObject\MockObject */
+	/** @var SimpleFolder | \PHPUnit\Framework\MockObject\MockObject */
 	private $folder;
 
 	/** @var \OC\Avatar\UserAvatar */
@@ -91,12 +90,13 @@ class UserAvatarTest extends \Test\TestCase {
 			->willReturnMap([
 				['avatar.jpg', true],
 				['avatar.128.jpg', true],
+				['generated', false],
 			]);
 
 		$expected = new \OC_Image();
 		$expected->loadFromFile(\OC::$SERVERROOT . '/tests/data/testavatar.png');
 
-		$file = $this->createMock(File::class);
+		$file = $this->createMock(ISimpleFile::class);
 		$file->method('getContent')->willReturn($expected->data());
 		$this->folder->method('getFile')->with('avatar.128.jpg')->willReturn($file);
 
@@ -107,12 +107,13 @@ class UserAvatarTest extends \Test\TestCase {
 		$this->folder->method('fileExists')
 			->willReturnMap([
 				['avatar.jpg', true],
+				['generated', false],
 			]);
 
 		$expected = new \OC_Image();
 		$expected->loadFromFile(\OC::$SERVERROOT . '/tests/data/testavatar.png');
 
-		$file = $this->createMock(File::class);
+		$file = $this->createMock(ISimpleFile::class);
 		$file->method('getContent')->willReturn($expected->data());
 		$this->folder->method('getFile')->with('avatar.jpg')->willReturn($file);
 
@@ -122,8 +123,10 @@ class UserAvatarTest extends \Test\TestCase {
 	public function testGetAvatarNoSizeMatch() {
 		$this->folder->method('fileExists')
 			->willReturnMap([
+				['avatar.jpg', false],
 				['avatar.png', true],
 				['avatar.32.png', false],
+				['generated', false],
 			]);
 
 		$expected = new \OC_Image();
@@ -132,7 +135,7 @@ class UserAvatarTest extends \Test\TestCase {
 		$expected2->loadFromFile(\OC::$SERVERROOT . '/tests/data/testavatar.png');
 		$expected2->resize(32);
 
-		$file = $this->createMock(File::class);
+		$file = $this->createMock(ISimpleFile::class);
 		$file->method('getContent')->willReturn($expected->data());
 
 		$this->folder->method('getFile')
@@ -146,7 +149,7 @@ class UserAvatarTest extends \Test\TestCase {
 				}
 			);
 
-		$newFile = $this->createMock(File::class);
+		$newFile = $this->createMock(ISimpleFile::class);
 		$newFile->expects($this->once())
 			->method('putContent')
 			->with($expected2->data());
@@ -202,12 +205,12 @@ class UserAvatarTest extends \Test\TestCase {
 		$this->folder->method('getDirectoryListing')
 			->willReturn([$avatarFileJPG, $avatarFilePNG, $resizedAvatarFile]);
 
-		$generated = $this->createMock(File::class);
+		$generated = $this->createMock(ISimpleFile::class);
 		$this->folder->method('getFile')
 			->with('generated')
 			->willReturn($generated);
 
-		$newFile = $this->createMock(File::class);
+		$newFile = $this->createMock(ISimpleFile::class);
 		$this->folder->expects($this->once())
 			->method('newFile')
 			->with('avatar.png')

tests/lib/Preview/GeneratorTest.php

@@ -95,6 +95,7 @@ class GeneratorTest extends \Test\TestCase {
 		$maxPreview = $this->createMock(ISimpleFile::class);
 		$maxPreview->method('getName')
 			->willReturn('1000-1000-max.png');
+		$maxPreview->method('getSize')->willReturn(1000);
 		$maxPreview->method('getMimeType')
 			->willReturn('image/png');
 
@@ -102,6 +103,7 @@ class GeneratorTest extends \Test\TestCase {
 			->willReturn([$maxPreview]);
 
 		$previewFile = $this->createMock(ISimpleFile::class);
+		$previewFile->method('getSize')->willReturn(1000);
 
 		$previewFolder->method('getFile')
 			->with($this->equalTo('256-256.png'))
@@ -203,8 +205,10 @@ class GeneratorTest extends \Test\TestCase {
 		$maxPreview = $this->createMock(ISimpleFile::class);
 		$maxPreview->method('getName')->willReturn('2048-2048-max.png');
 		$maxPreview->method('getMimeType')->willReturn('image/png');
+		$maxPreview->method('getSize')->willReturn(1000);
 
 		$previewFile = $this->createMock(ISimpleFile::class);
+		$previewFile->method('getSize')->willReturn(1000);
 
 		$previewFolder->method('getDirectoryListing')
 			->willReturn([]);
@@ -313,6 +317,7 @@ class GeneratorTest extends \Test\TestCase {
 		$maxPreview = $this->createMock(ISimpleFile::class);
 		$maxPreview->method('getName')
 			->willReturn('2048-2048-max.png');
+		$maxPreview->method('getSize')->willReturn(1000);
 		$maxPreview->method('getMimeType')
 			->willReturn('image/png');
 
@@ -320,6 +325,7 @@ class GeneratorTest extends \Test\TestCase {
 			->willReturn([$maxPreview]);
 
 		$preview = $this->createMock(ISimpleFile::class);
+		$preview->method('getSize')->willReturn(1000);
 		$previewFolder->method('getFile')
 			->with($this->equalTo('1024-512-crop.png'))
 			->willReturn($preview);
@@ -471,6 +477,7 @@ class GeneratorTest extends \Test\TestCase {
 			->willReturn($maxX . '-' . $maxY . '-max.png');
 		$maxPreview->method('getMimeType')
 			->willReturn('image/png');
+		$maxPreview->method('getSize')->willReturn(1000);
 
 		$previewFolder->method('getDirectoryListing')
 			->willReturn([$maxPreview]);
@@ -490,6 +497,7 @@ class GeneratorTest extends \Test\TestCase {
 			->willReturn($image);
 
 		$preview = $this->createMock(ISimpleFile::class);
+		$preview->method('getSize')->willReturn(1000);
 		$previewFolder->method('newFile')
 			->with($this->equalTo($filename))
 			->willReturn($preview);