From bd2101e7bb34d2b39dd41a577e2909c026e68c9b Mon Sep 17 00:00:00 2001 From: Varun Patil Date: Sun, 15 Oct 2023 12:46:35 -0700 Subject: [PATCH] refactor: PHP 8 syntax migration Signed-off-by: Varun Patil --- .php-cs-fixer.php | 2 +- lib/ClustersBackend/AlbumsBackend.php | 5 +- lib/ClustersBackend/Backend.php | 9 +- .../FaceRecognitionBackend.php | 6 +- lib/ClustersBackend/PeopleBackendUtils.php | 17 ++- lib/ClustersBackend/PlacesBackend.php | 3 +- lib/ClustersBackend/RecognizeBackend.php | 6 +- lib/ClustersBackend/TagsBackend.php | 2 +- lib/Command/Index.php | 12 +- lib/Command/MigrateGoogleTakeout.php | 26 ++--- lib/Command/PlacesSetup.php | 1 + lib/Controller/AdminController.php | 24 ++-- lib/Controller/ArchiveController.php | 6 +- lib/Controller/ClustersController.php | 6 +- lib/Controller/DownloadController.php | 14 +-- lib/Controller/FoldersController.php | 2 +- lib/Controller/ImageController.php | 28 ++--- lib/Controller/OtherController.php | 8 +- lib/Controller/PageController.php | 33 +++--- lib/Controller/PublicAlbumController.php | 2 +- lib/Controller/PublicController.php | 14 +-- lib/Controller/ShareController.php | 67 ++++++------ lib/Controller/VideoController.php | 18 +-- lib/Db/AlbumsQuery.php | 30 +++-- lib/Db/FsManager.php | 7 +- lib/Db/TimelineQueryCTE.php | 7 +- lib/Db/TimelineQueryMap.php | 13 ++- lib/Db/TimelineRoot.php | 21 ++-- lib/Db/TimelineWrite.php | 8 +- lib/Db/TimelineWriteOrphans.php | 9 +- lib/Db/TimelineWritePlaces.php | 14 +-- lib/Exceptions.php | 2 +- lib/Exif.php | 78 ++++++++++--- lib/Service/BinExt.php | 12 +- lib/Service/FileRobotMagick.php | 2 +- lib/Service/Index.php | 3 + lib/Settings/Admin.php | 6 + lib/Settings/AdminSection.php | 17 ++- lib/Util.php | 103 ++++++++---------- lib/UtilController.php | 48 ++++---- 40 files changed, 381 insertions(+), 310 deletions(-) diff --git a/.php-cs-fixer.php b/.php-cs-fixer.php index fb0988a7..e20d7eb3 100644 --- a/.php-cs-fixer.php +++ b/.php-cs-fixer.php @@ -26,7 +26,7 @@ $config '@PhpCsFixer' => true, '@PhpCsFixer:risky' => true, 'general_phpdoc_annotation_remove' => ['annotations' => ['expectedDeprecation']], // one should use PHPUnit built-in method instead - 'phpdoc_to_comment' => ['ignored_tags' => ['psalm-suppress', 'template-implements']], + 'phpdoc_to_comment' => ['ignored_tags' => ['psalm-suppress', 'template-implements', 'var']], 'trailing_comma_in_multiline' => ['elements' => ['arrays', 'parameters', 'arguments']], 'modernize_strpos' => true, 'no_alias_functions' => true, diff --git a/lib/ClustersBackend/AlbumsBackend.php b/lib/ClustersBackend/AlbumsBackend.php index 562023a5..dafa813d 100644 --- a/lib/ClustersBackend/AlbumsBackend.php +++ b/lib/ClustersBackend/AlbumsBackend.php @@ -27,6 +27,7 @@ use OCA\Memories\Db\AlbumsQuery; use OCA\Memories\Db\TimelineQuery; use OCA\Memories\Exceptions; use OCA\Memories\Util; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IRequest; class AlbumsBackend extends Backend @@ -57,7 +58,7 @@ class AlbumsBackend extends Backend return explode('/', $name)[1]; } - public function transformDayQuery(&$query, bool $aggregate): void + public function transformDayQuery(IQueryBuilder &$query, bool $aggregate): void { $albumId = (string) $this->request->getParam(self::clusterType()); @@ -102,7 +103,7 @@ class AlbumsBackend extends Backend // Add display names for users $userManager = \OC::$server->get(\OCP\IUserManager::class); - array_walk($list, static function (&$item) use ($userManager) { + array_walk($list, static function (array &$item) use ($userManager) { $user = $userManager->get($item['user']); $item['user_display'] = $user ? $user->getDisplayName() : null; }); diff --git a/lib/ClustersBackend/Backend.php b/lib/ClustersBackend/Backend.php index 75d1f758..63ae203f 100644 --- a/lib/ClustersBackend/Backend.php +++ b/lib/ClustersBackend/Backend.php @@ -24,6 +24,7 @@ declare(strict_types=1); namespace OCA\Memories\ClustersBackend; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\Files\SimpleFS\ISimpleFile; abstract class Backend { @@ -49,7 +50,7 @@ abstract class Backend * @param IQueryBuilder $query Query builder * @param bool $aggregate Whether this is an aggregate query */ - abstract public function transformDayQuery(&$query, bool $aggregate): void; + abstract public function transformDayQuery(IQueryBuilder &$query, bool $aggregate): void; /** * Apply post-query transformations for the given photo object. @@ -110,12 +111,12 @@ abstract class Backend /** * Perform any post processing and get the blob from the preview file. * - * @param \OCP\Files\SimpleFS\ISimpleFile $file Preview file - * @param array $photo Photo object + * @param ISimpleFile $file Preview file + * @param array $photo Photo object * * @return array [Blob, mimetype] of data */ - public function getPreviewBlob($file, $photo): array + public function getPreviewBlob(ISimpleFile $file, array $photo): array { return [$file->getContent(), $file->getMimeType()]; } diff --git a/lib/ClustersBackend/FaceRecognitionBackend.php b/lib/ClustersBackend/FaceRecognitionBackend.php index 8dd0202f..5ba0fade 100644 --- a/lib/ClustersBackend/FaceRecognitionBackend.php +++ b/lib/ClustersBackend/FaceRecognitionBackend.php @@ -25,6 +25,8 @@ namespace OCA\Memories\ClustersBackend; use OCA\Memories\Db\TimelineQuery; use OCA\Memories\Util; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\Files\SimpleFS\ISimpleFile; use OCP\IConfig; use OCP\IRequest; @@ -54,7 +56,7 @@ class FaceRecognitionBackend extends Backend && Util::facerecognitionIsEnabled(); } - public function transformDayQuery(&$query, bool $aggregate): void + public function transformDayQuery(IQueryBuilder &$query, bool $aggregate): void { $personStr = (string) $this->request->getParam('facerecognition'); @@ -196,7 +198,7 @@ class FaceRecognitionBackend extends Backend $this->sortByScores($photos); } - public function getPreviewBlob($file, $photo): array + public function getPreviewBlob(ISimpleFile $file, array $photo): array { return $this->cropFace($file, $photo, 1.8); } diff --git a/lib/ClustersBackend/PeopleBackendUtils.php b/lib/ClustersBackend/PeopleBackendUtils.php index 224ad39a..4f130a28 100644 --- a/lib/ClustersBackend/PeopleBackendUtils.php +++ b/lib/ClustersBackend/PeopleBackendUtils.php @@ -23,6 +23,8 @@ declare(strict_types=1); namespace OCA\Memories\ClustersBackend; +use OCP\Files\SimpleFS\ISimpleFile; + trait PeopleBackendUtils { /** @@ -83,9 +85,9 @@ trait PeopleBackendUtils * - width: width of the face in the image (percentage) * - height: height of the face in the image (percentage) * - * @param \OCP\Files\SimpleFS\ISimpleFile $file Actual file containing the image - * @param array $photo The face object - * @param float $padding The padding to add around the face + * @param ISimpleFile $file Actual file containing the image + * @param array $photo The face object + * @param float $padding The padding to add around the face * * @return string[] [Blob, mimetype] of resulting image * @@ -93,7 +95,7 @@ trait PeopleBackendUtils * * @psalm-return list{string, string} */ - private function cropFace($file, array $photo, float $padding): array + private function cropFace(ISimpleFile $file, array $photo, float $padding): array { $img = new \OCP\Image(); $img->loadFromData($file->getContent()); @@ -124,11 +126,8 @@ trait PeopleBackendUtils $img->scaleDownToFit(512, 512); // Get blob and mimetype - $data = $img->data(); - $mime = $img->mimeType(); - if (null === $data || null === $mime) { - throw new \Exception('Could not get image data'); - } + $data = $img->data() ?: throw new \Exception('Could not get image data'); + $mime = $img->mimeType() ?: throw new \Exception('Could not get image mimetype'); return [$data, $mime]; } diff --git a/lib/ClustersBackend/PlacesBackend.php b/lib/ClustersBackend/PlacesBackend.php index 1517e523..6e1da72d 100644 --- a/lib/ClustersBackend/PlacesBackend.php +++ b/lib/ClustersBackend/PlacesBackend.php @@ -25,6 +25,7 @@ namespace OCA\Memories\ClustersBackend; use OCA\Memories\Db\TimelineQuery; use OCA\Memories\Util; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IRequest; class PlacesBackend extends Backend @@ -49,7 +50,7 @@ class PlacesBackend extends Backend return Util::placesGISType() > 0; } - public function transformDayQuery(&$query, bool $aggregate): void + public function transformDayQuery(IQueryBuilder &$query, bool $aggregate): void { $locationId = (int) $this->request->getParam('places'); diff --git a/lib/ClustersBackend/RecognizeBackend.php b/lib/ClustersBackend/RecognizeBackend.php index f96c308a..615cabae 100644 --- a/lib/ClustersBackend/RecognizeBackend.php +++ b/lib/ClustersBackend/RecognizeBackend.php @@ -25,6 +25,8 @@ namespace OCA\Memories\ClustersBackend; use OCA\Memories\Db\TimelineQuery; use OCA\Memories\Util; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\Files\SimpleFS\ISimpleFile; use OCP\IRequest; class RecognizeBackend extends Backend @@ -51,7 +53,7 @@ class RecognizeBackend extends Backend return Util::recognizeIsEnabled(); } - public function transformDayQuery(&$query, bool $aggregate): void + public function transformDayQuery(IQueryBuilder &$query, bool $aggregate): void { // Check if Recognize is enabled if (!$this->isEnabled()) { @@ -232,7 +234,7 @@ class RecognizeBackend extends Backend $this->sortByScores($photos); } - public function getPreviewBlob($file, $photo): array + public function getPreviewBlob(ISimpleFile $file, array $photo): array { return $this->cropFace($file, $photo, 1.5); } diff --git a/lib/ClustersBackend/TagsBackend.php b/lib/ClustersBackend/TagsBackend.php index 07613486..0410b42e 100644 --- a/lib/ClustersBackend/TagsBackend.php +++ b/lib/ClustersBackend/TagsBackend.php @@ -50,7 +50,7 @@ class TagsBackend extends Backend return Util::tagsIsEnabled(); } - public function transformDayQuery(&$query, bool $aggregate): void + public function transformDayQuery(IQueryBuilder &$query, bool $aggregate): void { $tagName = (string) $this->request->getParam('tags'); diff --git a/lib/Command/Index.php b/lib/Command/Index.php index a2afa1d9..d9d71399 100644 --- a/lib/Command/Index.php +++ b/lib/Command/Index.php @@ -57,11 +57,13 @@ class IndexOpts class Index extends Command { - // IO + /** @psalm-suppress PropertyNotSetInConstructor */ private InputInterface $input; + + /** @psalm-suppress PropertyNotSetInConstructor */ private OutputInterface $output; - // Command options + /** @psalm-suppress PropertyNotSetInConstructor */ private IndexOpts $opts; public function __construct( @@ -184,9 +186,9 @@ class Index extends Command /** * Run function for all users (or selected user if set). * - * @param mixed $closure + * @param \Closure(IUser $user): void $closure */ - private function runForUsers($closure): void + private function runForUsers(\Closure $closure): void { if ($uid = $this->opts->user) { if ($user = $this->userManager->get($uid)) { @@ -203,7 +205,7 @@ class Index extends Command $this->output->writeln("Group {$gid} not found\n"); } } else { - $this->userManager->callForSeenUsers(static fn (IUser $user) => $closure($user)); + $this->userManager->callForSeenUsers($closure); } } } diff --git a/lib/Command/MigrateGoogleTakeout.php b/lib/Command/MigrateGoogleTakeout.php index 76a7cc7c..775ace21 100644 --- a/lib/Command/MigrateGoogleTakeout.php +++ b/lib/Command/MigrateGoogleTakeout.php @@ -44,7 +44,10 @@ class MigrateGoogleTakeout extends Command protected const MIGRATOR_VERSION = 1; protected const MIGRATED_KEY = 'memoriesMigratorVersion'; + /** @psalm-suppress PropertyNotSetInConstructor */ protected OutputInterface $output; + + /** @psalm-suppress PropertyNotSetInConstructor */ protected InputInterface $input; // Stats @@ -124,18 +127,22 @@ class MigrateGoogleTakeout extends Command protected function migrateUser(IUser $user): void { - $this->output->writeln("Migrating user {$user->getUID()}"); + $uid = $user->getUID(); + $this->output->writeln("Migrating user {$uid}"); // Get user's root folder \OC_Util::tearDownFS(); - \OC_Util::setupFS($user->getUID()); - $folder = $this->rootFolder->getUserFolder($user->getUID()); + \OC_Util::setupFS($uid); + $folder = $this->rootFolder->getUserFolder($uid); // Check if we need to migrate a specific folder if ($path = $this->input->getOption('folder')) { try { $folder = $folder->get($path); - } catch (\Exception $e) { + if (!$folder instanceof Folder) { + throw new \Exception(); + } + } catch (\Exception) { $this->output->writeln("Folder {$path} does not exist"); return; @@ -282,15 +289,10 @@ class MigrateGoogleTakeout extends Command ++$this->nProcessed; } - /** - * @return (float|mixed|string)[] - * - * @psalm-return array - */ protected function takeoutToExiftoolJson(array $json): array { // Helper to get a value from nested JSON - $get = static function (string $source) use ($json) { + $get = static function (string $source) use ($json): mixed { $keys = array_reverse(explode('.', $source)); while (\count($keys) > 0) { $key = array_pop($keys); @@ -336,8 +338,6 @@ class MigrateGoogleTakeout extends Command $txf['GPSAltitude'] = $get('geoData.altitude'); // Remove all null values - return array_filter($txf, static function ($value) { - return null !== $value; - }); + return array_filter($txf, static fn (mixed $value) => null !== $value); } } diff --git a/lib/Command/PlacesSetup.php b/lib/Command/PlacesSetup.php index 82446522..af36744f 100644 --- a/lib/Command/PlacesSetup.php +++ b/lib/Command/PlacesSetup.php @@ -31,6 +31,7 @@ use Symfony\Component\Console\Output\OutputInterface; class PlacesSetup extends Command { + /** @psalm-suppress PropertyNotSetInConstructor */ protected OutputInterface $output; public function __construct(protected Places $places) diff --git a/lib/Controller/AdminController.php b/lib/Controller/AdminController.php index 0902dbf4..c1f59d18 100644 --- a/lib/Controller/AdminController.php +++ b/lib/Controller/AdminController.php @@ -49,10 +49,8 @@ class AdminController extends GenericApiController /** * @AdminRequired - * - * @param mixed $value */ - public function setSystemConfig(string $key, $value): Http\Response + public function setSystemConfig(string $key, mixed $value): Http\Response { return Util::guardEx(function () use ($key, $value) { // Make sure not running in read-only mode @@ -94,7 +92,7 @@ class AdminController extends GenericApiController $exiftoolNoLocal = Util::getSystemConfig('memories.exiftool_no_local'); $status['exiftool'] = $this->getExecutableStatus( static fn () => BinExt::getExiftoolPBin(), - static fn ($p) => BinExt::testExiftool(), + static fn () => BinExt::testExiftool(), !$exiftoolNoLocal, !$exiftoolNoLocal, ); @@ -103,7 +101,7 @@ class AdminController extends GenericApiController /** @psalm-suppress ForbiddenCode */ $status['perl'] = $this->getExecutableStatus( trim(shell_exec('which perl') ?: '/bin/perl'), - static fn ($p) => BinExt::testSystemPerl($p), + static fn (string $p) => BinExt::testSystemPerl($p), ); // Check number of indexed files @@ -220,14 +218,14 @@ class AdminController extends GenericApiController /** * Get the status of an executable. * - * @param \Closure|string $path Path to the executable - * @param ?\Closure $testFunction Function to test the executable - * @param bool $testIfFile Test if the path is a file - * @param bool $testIfExecutable Test if the path is executable + * @param (\Closure():string)|string $path Path to the executable + * @param null|(\Closure(string):string) $testFunction Function to test the executable + * @param bool $testIfFile Test if the path is a file + * @param bool $testIfExecutable Test if the path is executable */ private function getExecutableStatus( - $path, - ?\Closure $testFunction = null, + \Closure|string $path, + null|\Closure $testFunction = null, bool $testIfFile = true, bool $testIfExecutable = true, ): string { @@ -239,10 +237,6 @@ class AdminController extends GenericApiController } } - if (!\is_string($path)) { - return 'not_found'; - } - if ($testIfFile && !is_file($path)) { return 'not_found'; } diff --git a/lib/Controller/ArchiveController.php b/lib/Controller/ArchiveController.php index a82cc640..a002a407 100644 --- a/lib/Controller/ArchiveController.php +++ b/lib/Controller/ArchiveController.php @@ -36,7 +36,7 @@ class ArchiveController extends GenericApiController * * Move one file to the archive folder * - * @param string fileid + * @param string $id File ID to archive / unarchive */ public function archive(string $id): Http\Response { @@ -73,6 +73,7 @@ class ArchiveController extends GenericApiController $parent = $file->getParent(); $isArchived = false; while (true) { + /** @psalm-suppress DocblockTypeContradiction */ if (null === $parent) { throw new \Exception('Cannot get correct parent of file'); } @@ -108,6 +109,9 @@ class ArchiveController extends GenericApiController // Get path of current file relative to the parent folder $relativeFilePath = $parent->getRelativePath($file->getPath()); + if (!$relativeFilePath) { + throw new \Exception('Cannot get relative path of file'); + } // Check if we want to archive or unarchive $body = $this->request->getParams(); diff --git a/lib/Controller/ClustersController.php b/lib/Controller/ClustersController.php index f4bf246b..4407af22 100644 --- a/lib/Controller/ClustersController.php +++ b/lib/Controller/ClustersController.php @@ -32,7 +32,11 @@ use OCP\AppFramework\Http\JSONResponse; class ClustersController extends GenericApiController { - /** Current backend for this instance */ + /** + * Current backend for this instance. + * + * @psalm-suppress PropertyNotSetInConstructor + */ protected ClustersBackend\Backend $backend; /** diff --git a/lib/Controller/DownloadController.php b/lib/Controller/DownloadController.php index a35aa46f..95650953 100644 --- a/lib/Controller/DownloadController.php +++ b/lib/Controller/DownloadController.php @@ -42,17 +42,11 @@ class DownloadController extends GenericApiController * * Request to download one or more files * - * @param mixed $files + * @param int[] $files List of file IDs */ - public function request($files): Http\Response + public function request(array $files): Http\Response { return Util::guardEx(static function () use ($files) { - // Get ids from body - if (null === $files || !\is_array($files)) { - throw Exceptions::MissingParameter('files'); - } - - // Return id $handle = self::createHandle('memories', $files); return new JSONResponse(['handle' => $handle]); @@ -65,7 +59,7 @@ class DownloadController extends GenericApiController * The calling controller must have the UseSession annotation. * * @param string $name Name of zip file - * @param int[] $files + * @param int[] $files List of file IDs */ public static function createHandle(string $name, array $files): string { @@ -282,7 +276,7 @@ class DownloadController extends GenericApiController break; } - /** @var bool|resource */ + /** @var false|resource */ $handle = false; /** @var ?\OCP\Files\File */ diff --git a/lib/Controller/FoldersController.php b/lib/Controller/FoldersController.php index 6d0358f7..b5bf0bc0 100644 --- a/lib/Controller/FoldersController.php +++ b/lib/Controller/FoldersController.php @@ -21,7 +21,7 @@ class FoldersController extends GenericApiController return Util::guardEx(function () use ($folder) { try { $node = Util::getUserFolder()->get($folder); - } catch (\OCP\Files\NotFoundException $e) { + } catch (\OCP\Files\NotFoundException) { throw Exceptions::NotFound('Folder not found'); } diff --git a/lib/Controller/ImageController.php b/lib/Controller/ImageController.php index 02244bc6..36c858a1 100644 --- a/lib/Controller/ImageController.php +++ b/lib/Controller/ImageController.php @@ -81,10 +81,12 @@ class ImageController extends GenericApiController return Util::guardExDirect(function (Http\IOutput $out) { // read body to array $body = file_get_contents('php://input'); + + /** @var array */ $files = json_decode($body, true); // Filter files with valid parameters - $files = array_filter($files, static function ($file) { + $files = array_filter($files, static function (array $file) { return isset($file['reqid'], $file['fileid'], $file['x'], $file['y'], $file['a']) && (int) $file['fileid'] > 0 && (int) $file['x'] > 0 @@ -92,14 +94,14 @@ class ImageController extends GenericApiController }); // Sort files by size, ascending - usort($files, static function ($a, $b) { + usort($files, static function (array $a, array $b) { $aArea = (int) $a['x'] * (int) $a['y']; $bArea = (int) $b['x'] * (int) $b['y']; return $aArea <=> $bArea; }); - /** @var \OCP\IPreview $previewManager */ + /** @var \OCP\IPreview */ $previewManager = \OC::$server->get(\OCP\IPreview::class); // For checking max previews @@ -202,8 +204,7 @@ class ImageController extends GenericApiController $info['basename'] = $file->getName(); // Allow these ony for logged in users - $user = $this->userSession->getUser(); - if (null !== $user) { + if ($user = $this->userSession->getUser()) { // Get the path of the file relative to current user // "/admin/files/Photos/Camera/20230821_135017.jpg" => "/Photos/..." $parts = explode('/', $file->getPath()); @@ -242,9 +243,6 @@ class ImageController extends GenericApiController * @NoAdminRequired * * Set the exif data for a file. - * - * @param int fileid - * @param array raw exif data */ public function setExif(int $id, array $raw): Http\Response { @@ -398,7 +396,9 @@ class ImageController extends GenericApiController * @param string $blob Blob of image data in any format * @param string $mimetype Mimetype of image data * - * @return array [blob, mimetype] + * @return string[] [blob, mimetype] + * + * @psalm-return list{string, string} */ private function getImageJPEG($blob, $mimetype): array { @@ -435,6 +435,8 @@ class ImageController extends GenericApiController /** * Get the tags for a file. + * + * @return string[] */ private function getTags(int $fileId): array { @@ -447,12 +449,10 @@ class ImageController extends GenericApiController $objectMapper = \OC::$server->get(\OCP\SystemTag\ISystemTagObjectMapper::class); $tagIds = $objectMapper->getTagIdsForObjects([$fileId], 'files')[(string) $fileId]; - // Get the tag names and filter out the ones that are not user visible - $tagManager = \OC::$server->get(\OCP\SystemTag\ISystemTagManager::class); - - /** @var \OCP\SystemTag\ISystemTag[] */ - $tags = $tagManager->getTagsByIds($tagIds); + // Get all matching tag objects + $tags = \OC::$server->get(\OCP\SystemTag\ISystemTagManager::class)->getTagsByIds($tagIds); + // Filter out the tags that are not user visible $visible = array_filter($tags, static fn ($t) => $t->isUserVisible()); // Get the tag names diff --git a/lib/Controller/OtherController.php b/lib/Controller/OtherController.php index 3f5328f5..1e6c6e6a 100644 --- a/lib/Controller/OtherController.php +++ b/lib/Controller/OtherController.php @@ -40,7 +40,7 @@ class OtherController extends GenericApiController * @param string key the identifier to change * @param string value the value to set * - * @return JSONResponse an empty JSONResponse with respective http status code + * @return Http\Response empty JSONResponse with respective http status code */ public function setUserConfig(string $key, string $value): Http\Response { @@ -77,7 +77,7 @@ class OtherController extends GenericApiController } // helper function to get user config values - $getAppConfig = function ($key, $default) use ($uid) { + $getAppConfig = function (string $key, mixed $default) use ($uid): mixed { return $this->config->getUserValue($uid, Application::APPNAME, $key, $default); }; @@ -124,7 +124,7 @@ class OtherController extends GenericApiController * * @NoCSRFRequired */ - public function describeApi(): JSONResponse + public function describeApi(): Http\Response { return Util::guardEx(static function () { $appManager = \OC::$server->get(\OCP\App\IAppManager::class); @@ -138,7 +138,7 @@ class OtherController extends GenericApiController try { $info['uid'] = Util::getUID(); - } catch (\Exception $e) { + } catch (\Exception) { $info['uid'] = null; } diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 282eb50d..c449494a 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -10,6 +10,7 @@ use OCA\Memories\Service\BinExt; use OCA\Memories\Util; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\ContentSecurityPolicy; +use OCP\AppFramework\Http\Response; use OCP\AppFramework\Http\Template\PublicTemplateResponse; use OCP\AppFramework\Http\TemplateResponse; use OCP\EventDispatcher\IEventDispatcher; @@ -29,7 +30,7 @@ class PageController extends Controller * * @NoCSRFRequired */ - public function main() + public function main(): Response { // Check native version if available $nativeVer = Util::callerNativeVersion(); @@ -59,11 +60,11 @@ class PageController extends Controller } /** Get the common content security policy */ - public static function getCSP() + public static function getCSP(): ContentSecurityPolicy { // Image domains MUST be added to the connect domain list // because of the service worker fetch() call - $addImageDomain = static function ($url) use (&$policy): void { + $addImageDomain = static function (string $url) use (&$policy): void { $policy->addAllowedImageDomain($url); $policy->addAllowedConnectDomain($url); }; @@ -114,7 +115,7 @@ class PageController extends Controller * * @NoCSRFRequired */ - public function folder() + public function folder(): Response { return $this->main(); } @@ -124,7 +125,7 @@ class PageController extends Controller * * @NoCSRFRequired */ - public function favorites() + public function favorites(): Response { return $this->main(); } @@ -134,7 +135,7 @@ class PageController extends Controller * * @NoCSRFRequired */ - public function albums() + public function albums(): Response { return $this->main(); } @@ -144,7 +145,7 @@ class PageController extends Controller * * @NoCSRFRequired */ - public function videos() + public function videos(): Response { return $this->main(); } @@ -154,7 +155,7 @@ class PageController extends Controller * * @NoCSRFRequired */ - public function archive() + public function archive(): Response { return $this->main(); } @@ -164,7 +165,7 @@ class PageController extends Controller * * @NoCSRFRequired */ - public function thisday() + public function thisday(): Response { return $this->main(); } @@ -174,7 +175,7 @@ class PageController extends Controller * * @NoCSRFRequired */ - public function recognize() + public function recognize(): Response { return $this->main(); } @@ -184,7 +185,7 @@ class PageController extends Controller * * @NoCSRFRequired */ - public function facerecognition() + public function facerecognition(): Response { return $this->main(); } @@ -194,7 +195,7 @@ class PageController extends Controller * * @NoCSRFRequired */ - public function places() + public function places(): Response { return $this->main(); } @@ -204,7 +205,7 @@ class PageController extends Controller * * @NoCSRFRequired */ - public function tags() + public function tags(): Response { return $this->main(); } @@ -214,7 +215,7 @@ class PageController extends Controller * * @NoCSRFRequired */ - public function map() + public function map(): Response { return $this->main(); } @@ -224,7 +225,7 @@ class PageController extends Controller * * @NoCSRFRequired */ - public function explore() + public function explore(): Response { return $this->main(); } @@ -234,7 +235,7 @@ class PageController extends Controller * * @NoCSRFRequired */ - public function nxsetup() + public function nxsetup(): Response { return $this->main(); } diff --git a/lib/Controller/PublicAlbumController.php b/lib/Controller/PublicAlbumController.php index fe85eccc..805cd071 100644 --- a/lib/Controller/PublicAlbumController.php +++ b/lib/Controller/PublicAlbumController.php @@ -45,7 +45,7 @@ class PublicAlbumController extends Controller * * @NoCSRFRequired */ - public function showShare(string $token) + public function showShare(string $token): Response { // Validate token exists $album = $this->albumsQuery->getAlbumByLink($token); diff --git a/lib/Controller/PublicController.php b/lib/Controller/PublicController.php index 846a5157..422224d9 100644 --- a/lib/Controller/PublicController.php +++ b/lib/Controller/PublicController.php @@ -25,6 +25,7 @@ use OCP\Share\IShare; class PublicController extends AuthPublicShareController { + /** @psalm-suppress PropertyNotSetInConstructor */ protected IShare $share; public function __construct( @@ -65,7 +66,7 @@ class PublicController extends AuthPublicShareController $this->share = $this->shareManager->getShareByToken($this->getToken()); return true; - } catch (\Exception $e) { + } catch (\Exception) { return false; } } @@ -80,7 +81,7 @@ class PublicController extends AuthPublicShareController // Check whether share exists try { $share = $this->shareManager->getShareByToken($this->getToken()); - } catch (\Exception $e) { + } catch (\Exception) { throw new NotFoundException(); } @@ -144,6 +145,7 @@ class PublicController extends AuthPublicShareController protected function isPasswordProtected(): bool { + /** @psalm-suppress RedundantConditionGivenDocblockType */ return null !== $this->share->getPassword(); } @@ -215,11 +217,7 @@ class PublicController extends AuthPublicShareController */ private function getSingleItemInitialState(\OCP\Files\File $file): array { - $data = $this->tq->getSingleItem($file->getId()); - if (null === $data) { - throw new NotFoundException(); - } - - return $data; + return $this->tq->getSingleItem($file->getId()) + ?? throw new NotFoundException(); } } diff --git a/lib/Controller/ShareController.php b/lib/Controller/ShareController.php index 18f3eb1d..d54b5397 100644 --- a/lib/Controller/ShareController.php +++ b/lib/Controller/ShareController.php @@ -27,6 +27,8 @@ use OCA\Memories\Exceptions; use OCA\Memories\Util; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; +use OCP\Share\IManager; +use OCP\Share\IShare; class ShareController extends GenericApiController { @@ -34,19 +36,16 @@ class ShareController extends GenericApiController * @NoAdminRequired * * Get the tokens of a node shared using an external link - * - * @param mixed $id - * @param mixed $path */ - public function links($id, $path): Http\Response + public function links(?int $id, ?string $path): Http\Response { return Util::guardEx(function () use ($id, $path) { $file = $this->getNodeByIdOrPath($id, $path); - /** @var \OCP\Share\IManager $shareManager */ - $shareManager = \OC::$server->get(\OCP\Share\IManager::class); + $shares = \OC::$server->get(IManager::class) + ->getSharesBy(Util::getUID(), IShare::TYPE_LINK, $file, true, 50, 0) + ; - $shares = $shareManager->getSharesBy(Util::getUID(), \OCP\Share\IShare::TYPE_LINK, $file, true, 50, 0); if (empty($shares)) { throw Exceptions::NotFound('external links'); } @@ -61,24 +60,21 @@ class ShareController extends GenericApiController * @NoAdminRequired * * Share a node using an external link - * - * @param mixed $id - * @param mixed $path */ - public function createNode($id, $path): Http\Response + public function createNode(?int $id, ?string $path): Http\Response { return Util::guardEx(function () use ($id, $path) { $file = $this->getNodeByIdOrPath($id, $path); - $shareManager = \OC::$server->get(\OCP\Share\IManager::class); + $manager = \OC::$server->get(IManager::class); - $share = $shareManager->newShare(); - $share->setNode($file); - $share->setShareType(\OCP\Share\IShare::TYPE_LINK); - $share->setSharedBy($this->userSession->getUser()->getUID()); - $share->setPermissions(\OCP\Constants::PERMISSION_READ); - - $share = $shareManager->createShare($share); + $share = $manager->createShare( + $manager->newShare() + ->setNode($file) + ->setShareType(\OCP\Share\IShare::TYPE_LINK) + ->setSharedBy(Util::getUID()) + ->setPermissions(\OCP\Constants::PERMISSION_READ), + ); return new JSONResponse($this->makeShareResponse($share), Http::STATUS_OK); }); @@ -94,22 +90,21 @@ class ShareController extends GenericApiController return Util::guardEx(static function () use ($id) { $uid = Util::getUID(); - /** @var \OCP\Share\IManager $shareManager */ - $shareManager = \OC::$server->get(\OCP\Share\IManager::class); + $manager = \OC::$server->get(\OCP\Share\IManager::class); - $share = $shareManager->getShareById($id); + $share = $manager->getShareById($id); if ($share->getSharedBy() !== $uid) { throw Exceptions::Forbidden('You are not the owner of this share'); } - $shareManager->deleteShare($share); + $manager->deleteShare($share); return new JSONResponse([], Http::STATUS_OK); }); } - private function getNodeByIdOrPath($id, $path): \OCP\Files\Node + private function getNodeByIdOrPath(?int $id, ?string $path): \OCP\Files\Node { $uid = Util::getUID(); @@ -120,7 +115,7 @@ class ShareController extends GenericApiController } elseif ($path) { $file = Util::getUserFolder($uid)->get($path); } - } catch (\OCP\Files\NotFoundException $e) { + } catch (\OCP\Files\NotFoundException) { throw Exceptions::NotFoundFile($path ?? $id); } @@ -131,24 +126,26 @@ class ShareController extends GenericApiController return $file; } - private function makeShareResponse(\OCP\Share\IShare $share): array + private function makeShareResponse(IShare $share): array { - /** @var \OCP\IURLGenerator $urlGenerator */ - $urlGenerator = \OC::$server->get(\OCP\IURLGenerator::class); + $token = $share->getToken(); + $url = \OC::$server->get(\OCP\IURLGenerator::class) + ->linkToRouteAbsolute('memories.Public.showShare', ['token' => $token]) + ; - $tok = $share->getToken(); - $exp = $share->getExpirationDate(); - $url = $urlGenerator->linkToRouteAbsolute('memories.Public.showShare', [ - 'token' => $tok, - ]); + /** + * @psalm-suppress RedundantConditionGivenDocblockType + * @psalm-suppress DocblockTypeContradiction + */ + $expiration = $share->getExpirationDate()?->getTimestamp(); return [ 'id' => $share->getFullId(), 'label' => $share->getLabel(), - 'token' => $tok, + 'token' => $token, 'url' => $url, 'hasPassword' => $share->getPassword() ? true : false, - 'expiration' => $exp ? $exp->getTimestamp() : null, + 'expiration' => $expiration, 'editable' => $share->getPermissions() & \OCP\Constants::PERMISSION_UPDATE, ]; } diff --git a/lib/Controller/VideoController.php b/lib/Controller/VideoController.php index 47390cd3..f7f052e5 100644 --- a/lib/Controller/VideoController.php +++ b/lib/Controller/VideoController.php @@ -78,7 +78,7 @@ class VideoController extends GenericApiController } // Request and check data was received - return Util::guardExDirect(function ($out) use ($client, $path, $profile) { + return Util::guardExDirect(function (Http\IOutput $out) use ($client, $path, $profile) { try { $status = $this->getUpstream($client, $path, $profile); if (409 === $status || -1 === $status) { @@ -261,7 +261,7 @@ class VideoController extends GenericApiController // Make sure query params are repeated // For example, in folder sharing, we need the params on every request $url = BinExt::getGoVodUrl($client, $path, $profile); - if ($params = $_SERVER['QUERY_STRING']) { + if (\array_key_exists('QUERY_STRING', $_SERVER) && !empty($params = $_SERVER['QUERY_STRING'])) { $url .= "?{$params}"; } @@ -275,7 +275,7 @@ class VideoController extends GenericApiController // Stream the response to the browser without reading it into memory $headersWritten = false; - curl_setopt($ch, CURLOPT_WRITEFUNCTION, static function ($curl, $data) use (&$headersWritten, $profile) { + curl_setopt($ch, CURLOPT_WRITEFUNCTION, static function (mixed $curl, mixed $data) use (&$headersWritten, $profile) { $returnCode = (int) curl_getinfo($curl, CURLINFO_HTTP_CODE); if (200 === $returnCode) { @@ -318,9 +318,9 @@ class VideoController extends GenericApiController /** * POST to go-vod to create a temporary file. * - * @param mixed $blob + * @return mixed The response from upstream */ - private static function postFile(string $client, $blob) + private static function postFile(string $client, mixed $blob): mixed { try { return self::postFileInternal($client, $blob); @@ -333,7 +333,7 @@ class VideoController extends GenericApiController } } - private static function postFileInternal(string $client, $blob) + private static function postFileInternal(string $client, mixed $blob): mixed { $url = BinExt::getGoVodUrl($client, '/create', 'ignore'); @@ -352,7 +352,7 @@ class VideoController extends GenericApiController throw new \Exception("Could not create temporary file ({$returnCode})"); } - return json_decode($response, true); + return json_decode((string) $response, true); } /** @@ -366,6 +366,8 @@ class VideoController extends GenericApiController // Get file paths for all live photos $liveFiles = array_map(fn ($r) => $this->rootFolder->getById((int) $r['fileid']), $liveRecords); $liveFiles = array_filter($liveFiles, static fn ($files) => \count($files) > 0 && $files[0] instanceof File); + + /** @var File[] (checked above) */ $liveFiles = array_map(static fn ($files) => $files[0], $liveFiles); // Should be filtered enough by now @@ -380,7 +382,7 @@ class VideoController extends GenericApiController // Remove extension so the filename itself counts in the path if (str_contains($filename, '.')) { - $filename = substr($filename, 0, strrpos($filename, '.')); + $filename = substr($filename, 0, strrpos($filename, '.') ?: null); } // Get components with the filename as lowercase diff --git a/lib/Db/AlbumsQuery.php b/lib/Db/AlbumsQuery.php index d6cfa248..e3dbbd8f 100644 --- a/lib/Db/AlbumsQuery.php +++ b/lib/Db/AlbumsQuery.php @@ -91,9 +91,9 @@ class AlbumsQuery /** * Check if an album has a file. * - * @return ?string owner of file + * @return false|string owner of file if found */ - public function hasFile(int $albumId, int $fileId): ?string + public function hasFile(int $albumId, int $fileId): false|string { $query = $this->connection->getQueryBuilder(); $query->select('owner')->from('photos_albums_files')->where( @@ -103,15 +103,15 @@ class AlbumsQuery ), ); - return $query->executeQuery()->fetchOne() ?: null; + return $query->executeQuery()->fetchOne(); } /** * Check if a file belongs to a user through an album. * - * @return bool|string owner of file + * @return false|string owner of file if found */ - public function userHasFile(string $uid, int $fileId) + public function userHasFile(string $uid, int $fileId): false|string { $query = $this->connection->getQueryBuilder(); $query->select('paf.owner')->from('photos_albums_files', 'paf')->where( @@ -173,6 +173,7 @@ class AlbumsQuery } // Check if user is owner + /** @psalm-suppress PossiblyUndefinedVariable */ if ($albumUid === $uid) { return $album; } @@ -264,15 +265,21 @@ class AlbumsQuery return $result; } - /** Get list of collaborator ids including user id and groups */ + /** + * Get the various collaborator IDs that a user has. + * This includes the groups the user is in and the user itself. + * + * @return string[] List of collaborator IDs + */ private function getSelfCollaborators(string $uid) { + // Get the user in question + $user = \OC::$server->get(\OCP\IUserManager::class)->get($uid) + ?: throw new \Exception('User not found'); // Get groups for the user - $groupManager = \OC::$server->get(\OCP\IGroupManager::class); - $user = \OC::$server->get(\OCP\IUserManager::class)->get($uid); - $groups = $groupManager->getUserGroupIds($user); + $groups = \OC::$server->get(\OCP\IGroupManager::class)->getUserGroupIds($user); - // And albums shared with user + // Add the user itself as a collaborator $groups[] = $uid; return $groups; @@ -284,8 +291,7 @@ class AlbumsQuery private function collaboratorsTable(): string { // https://github.com/nextcloud/photos/commit/20e3e61ad577014e5f092a292c90a8476f630355 - $appManager = \OC::$server->get(\OCP\App\IAppManager::class); - $photosVersion = $appManager->getAppVersion('photos'); + $photosVersion = \OC::$server->get(\OCP\App\IAppManager::class)->getAppVersion('photos'); if (version_compare($photosVersion, '2.0.1', '>=')) { return 'photos_albums_collabs'; } diff --git a/lib/Db/FsManager.php b/lib/Db/FsManager.php index 926d2333..582c0e82 100644 --- a/lib/Db/FsManager.php +++ b/lib/Db/FsManager.php @@ -151,6 +151,8 @@ class FsManager * * @param Folder $root root folder * @param string $key cache key + * + * @return string[] List of paths */ public function getNoMediaFolders(Folder $root, string $key): array { @@ -303,7 +305,7 @@ class FsManager } // Check if share is password protected - if (($password = $share->getPassword()) !== null) { + if (!empty($password = $share->getPassword())) { $session = \OC::$server->get(\OCP\ISession::class); // https://github.com/nextcloud/server/blob/0447b53bda9fe95ea0cbed765aa332584605d652/lib/public/AppFramework/PublicShareController.php#L119 @@ -318,7 +320,7 @@ class FsManager return $share; } - public function getShareNode() + public function getShareNode(): ?Node { $share = $this->getShareObject(); if (null === $share) { @@ -400,6 +402,7 @@ class FsManager Util::forcePermissions($file, $perm); } + /** @var File */ return $file; } diff --git a/lib/Db/TimelineQueryCTE.php b/lib/Db/TimelineQueryCTE.php index cad103e4..2e618d2a 100644 --- a/lib/Db/TimelineQueryCTE.php +++ b/lib/Db/TimelineQueryCTE.php @@ -78,7 +78,9 @@ trait TimelineQueryCTE return self::bundleCTEs([self::CTE_FOLDERS_ALL($hidden), $cte]); } - /** CTE to get all archive folders recursively in the given top folders */ + /** + * CTE to get all archive folders recursively in the given top folders. + */ protected static function CTE_FOLDERS_ARCHIVE(): string { $cte = "*PREFIX*cte_folders(fileid) AS ( @@ -102,6 +104,9 @@ trait TimelineQueryCTE return self::bundleCTEs([self::CTE_FOLDERS_ALL(true), $cte]); } + /** + * @param string[] $ctes The CTEs to bundle + */ protected static function bundleCTEs(array $ctes): string { return 'WITH RECURSIVE '.implode(',', $ctes); diff --git a/lib/Db/TimelineQueryMap.php b/lib/Db/TimelineQueryMap.php index 9204d2b8..190264f5 100644 --- a/lib/Db/TimelineQueryMap.php +++ b/lib/Db/TimelineQueryMap.php @@ -33,10 +33,8 @@ trait TimelineQueryMap ); } - public function getMapClusters( - float $gridLen, - string $bounds, - ): array { + public function getMapClusters(float $gridLen, string $bounds): array + { $query = $this->connection->getQueryBuilder(); // Get the average location of each cluster @@ -84,6 +82,11 @@ trait TimelineQueryMap return $clusters; } + /** + * Gets previews for a list of map clusters. + * + * @param int[] $clusterIds + */ public function getMapClusterPreviews(array $clusterIds): array { $query = $this->connection->getQueryBuilder(); @@ -131,6 +134,8 @@ trait TimelineQueryMap /** * Gets the suggested initial coordinates for the map. * Uses the coordinates of the newest photo (by date). + * + * @psalm-return array{lat: float, lon: float}|null */ public function getMapInitialPosition(): ?array { diff --git a/lib/Db/TimelineRoot.php b/lib/Db/TimelineRoot.php index bfa3de66..2ed5ff62 100644 --- a/lib/Db/TimelineRoot.php +++ b/lib/Db/TimelineRoot.php @@ -8,7 +8,10 @@ use OCP\Files\FileInfo; class TimelineRoot { + /** @var array */ protected array $folders = []; + + /** @var array */ protected array $folderPaths = []; /** @@ -29,7 +32,7 @@ class TimelineRoot } // Add top level folder - $this->setFolder($info->getId(), $info, $path); + $this->setFolder($info->getId() ?? 0, $info, $path); } /** @@ -89,28 +92,18 @@ class TimelineRoot } } - public function getFolderPath(int $id) - { - return $this->folderPaths[$id]; - } - - /** - * @return int[] - */ + /** @return int[] */ public function getIds(): array { return array_keys($this->folderPaths); } - /** - * @return null|int - */ - public function getOneId() + public function getOneId(): ?int { return array_key_first($this->folders); } - public function getFolder(int $id) + public function getFolder(int $id): ?FileInfo { return $this->folders[$id]; } diff --git a/lib/Db/TimelineWrite.php b/lib/Db/TimelineWrite.php index 64eea5d3..5647d0ef 100644 --- a/lib/Db/TimelineWrite.php +++ b/lib/Db/TimelineWrite.php @@ -138,7 +138,7 @@ class TimelineWrite $buid = $prevRow ? $prevRow['buid'] : ''; if (empty($buid)) { $imageUniqueId = \array_key_exists('ImageUniqueID', $exif) ? $exif['ImageUniqueID'] : null; - $buid = Exif::getBUID($file->getName(), $imageUniqueId, $file->getSize()); + $buid = Exif::getBUID($file->getName(), $imageUniqueId, (int) $file->getSize()); } // Get exif json @@ -185,7 +185,7 @@ class TimelineWrite /** * Remove a file from the exif database. */ - public function deleteFile(File &$file): void + public function deleteFile(File $file): void { // Get full record $query = $this->connection->getQueryBuilder(); @@ -261,7 +261,7 @@ class TimelineWrite */ private function getCurrentRow(int $fileId): ?array { - $fetch = function (string $table) use ($fileId) { + $fetch = function (string $table) use ($fileId): false|null|array { $query = $this->connection->getQueryBuilder(); return $query->select('*') @@ -277,6 +277,8 @@ class TimelineWrite /** * Convert EXIF data to filtered JSON string. + * + * @param array $exif EXIF data */ private function getExifJson(array $exif): string { diff --git a/lib/Db/TimelineWriteOrphans.php b/lib/Db/TimelineWriteOrphans.php index 2554663a..dde93eb9 100644 --- a/lib/Db/TimelineWriteOrphans.php +++ b/lib/Db/TimelineWriteOrphans.php @@ -47,9 +47,9 @@ trait TimelineWriteOrphans /** * Orphan and run an update on all files. * - * @param array $fields list of fields to select - * @param int $txnSize number of rows to process in a single transaction - * @param \Closure $callback will be passed each row + * @param array $fields list of fields to select + * @param int $txnSize number of rows to process in a single transaction + * @param \Closure(array): void $callback will be passed each row */ public function orphanAndRun(array $fields, int $txnSize, \Closure $callback): void { @@ -74,6 +74,9 @@ trait TimelineWriteOrphans /** * Get a list of orphaned files. + * + * @param int $count max number of rows to return + * @param string[] $fields list of fields to select */ protected function getSomeOrphans(int $count, array $fields): array { diff --git a/lib/Db/TimelineWritePlaces.php b/lib/Db/TimelineWritePlaces.php index 4a6109cd..5dd7c28c 100644 --- a/lib/Db/TimelineWritePlaces.php +++ b/lib/Db/TimelineWritePlaces.php @@ -18,13 +18,13 @@ trait TimelineWritePlaces /** * Add places data for a file. * - * @param int $fileId The file ID - * @param null|float $lat The latitude of the file - * @param null|float $lon The longitude of the file + * @param int $fileId The file ID + * @param ?float $lat The latitude of the file + * @param ?float $lon The longitude of the file * - * @return array The list of osm_id of the places + * @return int[] The list of osm_id of the places */ - public function updatePlacesData(int $fileId, $lat, $lon): array + public function updatePlacesData(int $fileId, ?float $lat, ?float $lon): array { // Get GIS type $gisType = \OCA\Memories\Util::placesGISType(); @@ -74,7 +74,7 @@ trait TimelineWritePlaces $this->connection->commit(); // Return list of osm_id - return array_map(static fn ($row) => $row['osm_id'], $rows); + return array_map(static fn ($row) => (int) $row['osm_id'], $rows); } /** @@ -155,7 +155,7 @@ trait TimelineWritePlaces /** * Read coordinates from array and round to 6 decimal places. * - * Modifies the array to remove invalid coordinates. + * Modifies the EXIF array to remove invalid coordinates. * * @return (null|float)[] * diff --git a/lib/Exceptions.php b/lib/Exceptions.php index 4dec524d..06b102fb 100644 --- a/lib/Exceptions.php +++ b/lib/Exceptions.php @@ -44,7 +44,7 @@ class Exceptions ], Http::STATUS_NOT_FOUND)); } - public static function NotFoundFile($identifier): HttpResponseException + public static function NotFoundFile(null|int|string $identifier): HttpResponseException { return new HttpResponseException(new DataResponse([ 'message' => "File not found ({$identifier})", diff --git a/lib/Exif.php b/lib/Exif.php index e040ecc0..fa4a5438 100644 --- a/lib/Exif.php +++ b/lib/Exif.php @@ -15,21 +15,31 @@ class Exif private const EXIFTOOL_ARGS = ['-api', 'QuickTimeUTC=1', '-n', '-json']; /** Opened instance of exiftool when running in command mode */ + /** @var null|resource */ private static $staticProc; + + /** @var null|resource[] */ private static $staticPipes; + + /** Disable uisage of static process */ private static bool $noStaticProc = false; public static function closeStaticExiftoolProc(): void { try { - if (self::$staticProc) { + // Close I/O pipes + if (self::$staticPipes) { fclose(self::$staticPipes[0]); fclose(self::$staticPipes[1]); fclose(self::$staticPipes[2]); + self::$staticPipes = null; + } + + // Close process + if (self::$staticProc) { proc_terminate(self::$staticProc); proc_close(self::$staticProc); self::$staticProc = null; - self::$staticPipes = null; } } catch (\Exception $ex) { } @@ -50,10 +60,13 @@ class Exif if (!self::$staticProc) { self::initializeStaticExiftoolProc(); usleep(500000); // wait if error + + /** @psalm-suppress NullArgument */ if (!proc_get_status(self::$staticProc)['running']) { error_log('WARN: Failed to create stay_open exiftool process'); self::$noStaticProc = true; self::$staticProc = null; + self::$staticPipes = null; } return; @@ -61,12 +74,15 @@ class Exif if (!proc_get_status(self::$staticProc)['running']) { self::$staticProc = null; + self::$staticPipes = null; self::ensureStaticExiftoolProc(); } } /** * Get exif data as a JSON object from a Nextcloud file. + * + * @return array */ public static function getExifFromFile(File $file): array { @@ -108,6 +124,8 @@ class Exif /** * Get exif data as a JSON object from a local file path. + * + * @return array */ public static function getExifFromLocalPath(string $path): array { @@ -122,6 +140,8 @@ class Exif /** * Parse date from exif format and throw error if invalid. + * + * @param array $exif */ public static function parseExifDate(array $exif): \DateTime { @@ -140,9 +160,12 @@ class Exif // Get timezone from exif try { - $exifTz = $exif['OffsetTimeOriginal'] ?? $exif['OffsetTime'] ?? $exif['LocationTZID'] ?? null; - $exifTz = new \DateTimeZone($exifTz); - } catch (\Error $e) { + $tzStr = $exif['OffsetTimeOriginal'] + ?: $exif['OffsetTime'] + ?: $exif['LocationTZID'] + ?: throw new \Exception(); + $exifTz = new \DateTimeZone((string) $tzStr); + } catch (\Exception) { $exifTz = null; } @@ -200,13 +223,15 @@ class Exif /** * Get the date taken from either the file or exif data if available. + * + * @param array $exif */ public static function getDateTaken(File $file, array $exif): \DateTime { try { return self::parseExifDate($exif); - } catch (\Exception $ex) { - } catch (\ValueError $ex) { + } catch (\Exception) { + } catch (\ValueError) { } // Fall back to modification time @@ -217,7 +242,7 @@ class Exif try { $dt->setTimezone(new \DateTimeZone($tz)); - } catch (\Exception $e) { + } catch (\Exception) { throw new \Error("FATAL: system timezone is invalid (TZ): {$tz}"); } @@ -235,9 +260,13 @@ class Exif /** * Get image dimensions from Exif data. * - * @return array [width, height] + * @param array $exif + * + * @return int[] + * + * @psalm-return list{int, int} */ - public static function getDimensions(array $exif) + public static function getDimensions(array $exif): array { $width = $exif['ImageWidth'] ?? 0; $height = $exif['ImageHeight'] ?? 0; @@ -274,7 +303,7 @@ class Exif * @param mixed $imageUniqueID EXIF field * @param int $size the file size in bytes (fallback) */ - public static function getBUID(string $basename, $imageUniqueID, int $size): string + public static function getBUID(string $basename, mixed $imageUniqueID, int $size): string { $sfx = "size={$size}"; if (null !== $imageUniqueID && \strlen((string) $imageUniqueID) >= 4) { @@ -295,8 +324,8 @@ class Exif /** * Set exif data using raw json. * - * @param string $path to local file - * @param array $data exif data + * @param string $path to local file + * @param array $data exif data * * @throws \Exception on failure */ @@ -330,6 +359,11 @@ class Exif } } + /** + * Set exif data using a raw array. + * + * @param array $data exif data + */ public static function setFileExif(File $file, array $data): void { // Get path to local file so we can skip reading @@ -389,6 +423,7 @@ class Exif private static function initializeStaticExiftoolProc(): void { self::closeStaticExiftoolProc(); + self::$staticPipes = []; self::$staticProc = proc_open(array_merge(self::getExiftool(), ['-stay_open', 'true', '-@', '-']), [ 0 => ['pipe', 'r'], 1 => ['pipe', 'w'], @@ -429,19 +464,34 @@ class Exif private static function getExifFromLocalPathWithStaticProc(string $path): array { + // This function should not be called if there is no static process + if (!self::$staticPipes) { + throw new \Error('[BUG] No static pipes found'); + } + + // Create arguments for exiftool $args = implode("\n", self::EXIFTOOL_ARGS); fwrite(self::$staticPipes[0], "{$path}\n{$args}\n-execute\n"); fflush(self::$staticPipes[0]); + // The output of exiftool's stay_open process ends with this token $readyToken = "\n{ready}\n"; try { $buf = self::readOrTimeout(self::$staticPipes[1], self::EXIFTOOL_TIMEOUT, $readyToken); + + // The output buffer should always contain the ready token + // (this is the point of readOrTimeout) $tokPos = strrpos($buf, $readyToken); + if (false === $tokPos) { + throw new \Error('[BUG] No ready token found in output buffer'); + } + + // Slice everything before the ready token $buf = substr($buf, 0, $tokPos); return self::processStdout($buf); - } catch (\Exception $ex) { + } catch (\Exception) { error_log("ERROR: Exiftool may have crashed, restarting process [{$path}]"); self::restartStaticExiftoolProc(); diff --git a/lib/Service/BinExt.php b/lib/Service/BinExt.php index ce6d295a..efe6c6a3 100644 --- a/lib/Service/BinExt.php +++ b/lib/Service/BinExt.php @@ -126,10 +126,8 @@ class BinExt /** * Detect the exiftool binary to use. - * - * @return false|string */ - public static function detectExiftool() + public static function detectExiftool(): false|string { if (!empty($path = Util::getSystemConfig('memories.exiftool'))) { return $path; @@ -371,10 +369,8 @@ class BinExt /** * Detect the go-vod binary to use. - * - * @return false|string */ - public static function detectGoVod() + public static function detectGoVod(): false|string { $goVodPath = Util::getSystemConfig('memories.vod.path'); @@ -445,7 +441,7 @@ class BinExt return explode(' ', $matches[0])[2]; } - public static function testSystemPerl(string $path): ?string + public static function testSystemPerl(string $path): string { /** @psalm-suppress ForbiddenCode */ if (($out = shell_exec("{$path} -e 'print \"OK\";'")) !== 'OK') { @@ -453,6 +449,6 @@ class BinExt } /** @psalm-suppress ForbiddenCode */ - return shell_exec("{$path} -e 'print $^V;'") ?: null; + return shell_exec("{$path} -e 'print $^V;'") ?: 'unknown version'; } } diff --git a/lib/Service/FileRobotMagick.php b/lib/Service/FileRobotMagick.php index 378f2ca8..62684e47 100644 --- a/lib/Service/FileRobotMagick.php +++ b/lib/Service/FileRobotMagick.php @@ -49,7 +49,7 @@ class FileRobotImageState /** 0 to 200 */ public ?float $warmth = null; - /** Order of filters */ + /** @var string[] Order of filters */ public array $finetuneOrder = []; /** Crop X coordinate */ diff --git a/lib/Service/Index.php b/lib/Service/Index.php index bc528eb8..e278c213 100644 --- a/lib/Service/Index.php +++ b/lib/Service/Index.php @@ -46,9 +46,12 @@ class Index /** * Callback to check if the process should continue. * This is called before every file is indexed. + * + * @var null|\Closure(): bool */ public ?\Closure $continueCheck = null; + /** @var string[] */ private static ?array $mimeList = null; public function __construct( diff --git a/lib/Settings/Admin.php b/lib/Settings/Admin.php index 80a4e702..07a4596a 100644 --- a/lib/Settings/Admin.php +++ b/lib/Settings/Admin.php @@ -23,11 +23,17 @@ class Admin implements ISettings return new TemplateResponse('memories', 'main', PageController::getMainParams()); } + /** + * @return string + */ public function getSection() { return Application::APPNAME; } + /** + * @return int + */ public function getPriority() { return 50; diff --git a/lib/Settings/AdminSection.php b/lib/Settings/AdminSection.php index 82d68688..6c0039ea 100644 --- a/lib/Settings/AdminSection.php +++ b/lib/Settings/AdminSection.php @@ -21,27 +21,36 @@ class AdminSection implements IIconSection */ public function getForm() { - $parameters = [ - ]; - - return new TemplateResponse('memories', 'admin', $parameters); + return new TemplateResponse('memories', 'admin', []); } + /** + * @return string + */ public function getID() { return 'memories'; } + /** + * @return string + */ public function getName() { return $this->l->t('Memories'); } + /** + * @return int + */ public function getPriority() { return 75; } + /** + * @return string + */ public function getIcon() { return $this->urlGenerator->imagePath('memories', 'app-dark.svg'); diff --git a/lib/Util.php b/lib/Util.php index 3e1c69b0..7e1415d7 100644 --- a/lib/Util.php +++ b/lib/Util.php @@ -27,11 +27,11 @@ class Util */ public static function getArch(): ?string { - $uname = php_uname('m'); - if (false !== stripos($uname, 'aarch64') || false !== stripos($uname, 'arm64')) { + $uname = strtolower(php_uname('m') ?: 'unknown'); + if (str_contains($uname, 'aarch64') || str_contains($uname, 'arm64')) { return 'aarch64'; } - if (false !== stripos($uname, 'x86_64') || false !== stripos($uname, 'amd64')) { + if (str_contains($uname, 'x86_64') || str_contains($uname, 'amd64')) { return 'amd64'; } @@ -46,13 +46,12 @@ class Util public static function getLibc(): ?string { /** @psalm-suppress ForbiddenCode */ - if ($ldd = shell_exec('ldd --version 2>&1')) { - if (false !== stripos($ldd, 'musl')) { - return 'musl'; - } - if (false !== stripos($ldd, 'glibc')) { - return 'glibc'; - } + $ldd = strtolower(shell_exec('ldd --version 2>&1') ?: 'unknown'); + if (str_contains($ldd, 'musl')) { + return 'musl'; + } + if (str_contains($ldd, 'glibc')) { + return 'glibc'; } return null; @@ -79,9 +78,7 @@ class Util */ public static function tagsIsEnabled(): bool { - $appManager = \OC::$server->get(IAppManager::class); - - return $appManager->isEnabledForUser('systemtags'); + return \OC::$server->get(IAppManager::class)->isEnabledForUser('systemtags'); } /** @@ -130,16 +127,14 @@ class Util } try { - $uid = self::getUID(); - } catch (\Exception $e) { - return false; + return 'true' === \OC::$server->get(IConfig::class) + ->getUserValue(self::getUID(), 'facerecognition', 'enabled', 'false') + ; + } catch (\Exception) { + // not logged in } - $enabled = \OC::$server->get(IConfig::class) - ->getUserValue($uid, 'facerecognition', 'enabled', 'false') - ; - - return 'true' === $enabled; + return false; } /** @@ -163,9 +158,7 @@ class Util */ public static function previewGeneratorIsEnabled(): bool { - $appManager = \OC::$server->get(IAppManager::class); - - return $appManager->isEnabledForUser('previewgenerator'); + return \OC::$server->get(IAppManager::class)->isEnabledForUser('previewgenerator'); } /** @@ -196,11 +189,11 @@ class Util * Force a fileinfo value on a node. * This is a hack to avoid subclassing everything. * - * @param mixed $node File to patch - * @param mixed $key Key to set - * @param mixed $value Value to set + * @param Node $node File to patch + * @param string $key Key to set + * @param mixed $value Value to set */ - public static function forceFileInfo(Node &$node, $key, $value): void + public static function forceFileInfo(Node &$node, string $key, mixed $value): void { /** @var \OC\Files\Node\Node */ $node = $node; @@ -212,8 +205,8 @@ class Util /** * Force permissions on a node. * - * @param mixed $node File to patch - * @param mixed $permissions Permissions to set + * @param Node $node File to patch + * @param int $permissions Permissions to set */ public static function forcePermissions(Node &$node, int $permissions): void { @@ -248,10 +241,10 @@ class Util /** * Add OG metadata to a page for a node. * - * @param $node Node to get metadata from - * @param $title Title of the page - * @param $url URL of the page - * @param $previewArgs Preview arguments (e.g. token) + * @param Node $node Node to get metadata from + * @param string $title Title of the page + * @param string $url URL of the page + * @param array $previewArgs Preview arguments (e.g. token) */ public static function addOgMetadata(Node $node, string $title, string $url, array $previewArgs): void { @@ -291,8 +284,6 @@ class Util /** * Get a random image or video from a given folder. - * - * @param $folder Folder to search */ public static function getAnyMedia(\OCP\Files\Folder $folder): ?Node { @@ -335,36 +326,40 @@ class Util /** * Get list of timeline paths as array. + * + * @return string[] List of paths */ public static function getTimelinePaths(string $uid): array { - $config = \OC::$server->get(IConfig::class); - $paths = $config->getUserValue($uid, Application::APPNAME, 'timelinePath', null) - ?? self::getSystemConfig('memories.timeline.default_path'); + $paths = \OC::$server->get(IConfig::class) + ->getUserValue($uid, Application::APPNAME, 'timelinePath', null) + ?: self::getSystemConfig('memories.timeline.default_path'); - return array_map(static fn ($p) => self::sanitizePath(trim($p)), explode(';', $paths)); + return array_map( + static fn ($p) => self::sanitizePath(trim($p)), + explode(';', $paths), + ); } /** * Sanitize a path to keep only ASCII characters and special characters. + * Blank will be returned on error. */ - public static function sanitizePath(string $path): ?string + public static function sanitizePath(string $path): string { $path = str_replace("\0", '', $path); // remove null characters - return mb_ereg_replace('\/\/+', '/', $path) ?: null; // remove extra slashes + return mb_ereg_replace('\/\/+', '/', $path) ?: ''; // remove extra slashes } /** * Convert SQL UTC date to timestamp. - * - * @param mixed $sqlDate */ - public static function sqlUtcToTimestamp($sqlDate): int + public static function sqlUtcToTimestamp(string $sqlDate): int { try { return (new \DateTime($sqlDate, new \DateTimeZone('UTC')))->getTimestamp(); - } catch (\Throwable $e) { + } catch (\Throwable) { return 0; } } @@ -372,9 +367,9 @@ class Util /** * Explode a string into fixed number of components. * - * @param string $delimiter Delimiter - * @param string $string String to explode - * @param int $count Number of components + * @param non-empty-string $delimiter Delimiter + * @param string $string String to explode + * @param int $count Number of components * * @return string[] Array of components */ @@ -386,10 +381,10 @@ class Util /** * Get a system config key with the correct default. * - * @param string $key System config key - * @param null|mixed $default Default value + * @param string $key System config key + * @param mixed $default Default value */ - public static function getSystemConfig(string $key, $default = null) + public static function getSystemConfig(string $key, mixed $default = null): mixed { $config = \OC::$server->get(\OCP\IConfig::class); @@ -404,11 +399,9 @@ class Util /** * Set a system config key. * - * @param mixed $value - * * @throws \InvalidArgumentException */ - public static function setSystemConfig(string $key, $value): void + public static function setSystemConfig(string $key, mixed $value): void { $config = \OC::$server->get(\OCP\IConfig::class); diff --git a/lib/UtilController.php b/lib/UtilController.php index da4e2838..253ca3c1 100644 --- a/lib/UtilController.php +++ b/lib/UtilController.php @@ -12,12 +12,12 @@ trait UtilController /** * Run a function and catch exceptions to return HTTP response. * - * @param mixed $function + * @param \Closure(): Http\Response $closure */ - public static function guardEx($function): Http\Response + public static function guardEx(\Closure $closure): Http\Response { try { - return $function(); + return $closure(); } catch (\OCA\Memories\HttpResponseException $e) { return $e->response; } catch (\Exception $e) { @@ -29,23 +29,25 @@ trait UtilController /** * Return a callback response with guarded exceptions. + * + * @param \Closure(Http\IOutput): void $closure */ public static function guardExDirect(\Closure $closure): Http\Response { /** @psalm-suppress MissingTemplateParam */ return new class($closure) extends Http\Response implements Http\ICallbackResponse { - private \Closure $_closure; - - public function __construct(\Closure $closure) + /** + * @param \Closure(Http\IOutput): void $closure + */ + public function __construct(private \Closure $closure) { parent::__construct(); - $this->_closure = $closure; } public function callback(Http\IOutput $output) { try { - ($this->_closure)($output); + ($this->closure)($output); } catch (\OCA\Memories\HttpResponseException $e) { $res = $e->response; $output->setHttpResponseCode($res->getStatus()); @@ -73,12 +75,8 @@ trait UtilController */ public static function getUser(): \OCP\IUser { - $user = \OC::$server->get(\OCP\IUserSession::class)->getUser(); - if (null === $user) { - throw Exceptions::NotLoggedIn(); - } - - return $user; + return \OC::$server->get(\OCP\IUserSession::class)->getUser() + ?? throw Exceptions::NotLoggedIn(); } /** @@ -102,17 +100,15 @@ trait UtilController /** * Get a user's home folder. * - * @param null|string $uid User ID, or null for current user + * @param null|string $uid User ID, or null for the user * * @throws \OCA\Memories\HttpResponseException if the user is not logged in */ public static function getUserFolder(?string $uid = null): \OCP\Files\Folder { - if (null === $uid) { - $uid = self::getUID(); - } - - return \OC::$server->get(\OCP\Files\IRootFolder::class)->getUserFolder($uid); + return \OC::$server->get(\OCP\Files\IRootFolder::class) + ->getUserFolder($uid ?? self::getUID()) + ; } /** @@ -124,14 +120,12 @@ trait UtilController $config = \OC::$server->get(\OCP\IConfig::class); $default = $config->getSystemValue('default_language', 'en'); - // Get UID of the user try { - $uid = self::getUID(); - } catch (\Exception $e) { - return 'en'; + // Get language of the user + return $config->getUserValue(self::getUID(), 'core', 'lang', $default); + } catch (\Exception) { + // Fallback to server language + return $default; } - - // Get language of the user - return $config->getUserValue($uid, 'core', 'lang', $default); } }