From fa9205a11ef706c37894daf55a97cd239b4fb778 Mon Sep 17 00:00:00 2001 From: Varun Patil Date: Thu, 23 Mar 2023 17:56:41 -0700 Subject: [PATCH] More refactor Signed-off-by: Varun Patil --- lib/AppInfo/Application.php | 11 ++- lib/ClustersBackend/AlbumsBackend.php | 11 ++- lib/ClustersBackend/Backend.php | 64 ------------- lib/Controller/ClustersController.php | 19 +--- lib/Controller/DaysController.php | 23 ++++- lib/Controller/GenericApiController.php | 2 - lib/Controller/GenericApiControllerParams.php | 54 ----------- lib/Controller/MapController.php | 10 +- lib/Controller/OtherController.php | 4 +- lib/Controller/TagsController.php | 2 +- lib/Db/AlbumsQuery.php | 2 +- lib/Db/TimelineQueryDays.php | 3 +- lib/Manager/ClustersBackendManager.php | 93 +++++++++++++++++++ 13 files changed, 142 insertions(+), 156 deletions(-) delete mode 100644 lib/Controller/GenericApiControllerParams.php create mode 100644 lib/Manager/ClustersBackendManager.php diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index d38cb9d4..bf6f40a1 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -26,6 +26,7 @@ namespace OCA\Memories\AppInfo; use OCA\Memories\ClustersBackend; use OCA\Memories\Listeners\PostDeleteListener; use OCA\Memories\Listeners\PostWriteListener; +use OCA\Memories\Manager\ClustersBackendManager; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootContext; use OCP\AppFramework\Bootstrap\IBootstrap; @@ -75,11 +76,11 @@ class Application extends App implements IBootstrap $context->registerEventListener(NodeDeletedEvent::class, PostDeleteListener::class); // Register clusters backends - ClustersBackend\Backend::register('albums', ClustersBackend\AlbumsBackend::class); - ClustersBackend\Backend::register('tags', ClustersBackend\TagsBackend::class); - ClustersBackend\Backend::register('places', ClustersBackend\PlacesBackend::class); - ClustersBackend\Backend::register('recognize', ClustersBackend\RecognizeBackend::class); - ClustersBackend\Backend::register('facerecognition', ClustersBackend\FaceRecognitionBackend::class); + ClustersBackendManager::register('albums', ClustersBackend\AlbumsBackend::class); + ClustersBackendManager::register('tags', ClustersBackend\TagsBackend::class); + ClustersBackendManager::register('places', ClustersBackend\PlacesBackend::class); + ClustersBackendManager::register('recognize', ClustersBackend\RecognizeBackend::class); + ClustersBackendManager::register('facerecognition', ClustersBackend\FaceRecognitionBackend::class); } public function boot(IBootContext $context): void diff --git a/lib/ClustersBackend/AlbumsBackend.php b/lib/ClustersBackend/AlbumsBackend.php index 009261ef..91d2acc7 100644 --- a/lib/ClustersBackend/AlbumsBackend.php +++ b/lib/ClustersBackend/AlbumsBackend.php @@ -60,10 +60,8 @@ class AlbumsBackend extends Backend { $albumId = (string) $this->request->getParam('albums'); - $uid = Util::isLoggedIn() ? Util::getUID() : ''; - // Get album object - $album = $this->albumsQuery->getIfAllowed($uid, $albumId); + $album = $this->albumsQuery->getIfAllowed($this->getUID(), $albumId); // Check permission if (null === $album) { @@ -110,7 +108,7 @@ class AlbumsBackend extends Backend public function getPhotos(string $name, ?int $limit = null): array { // Get album - $album = $this->albumsQuery->getIfAllowed(Util::getUID(), $name); + $album = $this->albumsQuery->getIfAllowed($this->getUID(), $name); if (null === $album) { throw Exceptions::NotFound("album {$name}"); } @@ -120,4 +118,9 @@ class AlbumsBackend extends Backend return $this->albumsQuery->getAlbumPhotos($id, $limit) ?? []; } + + private function getUID(): string + { + return Util::isLoggedIn() ? Util::getUID() : '---'; + } } diff --git a/lib/ClustersBackend/Backend.php b/lib/ClustersBackend/Backend.php index 03f3301f..89c90f3f 100644 --- a/lib/ClustersBackend/Backend.php +++ b/lib/ClustersBackend/Backend.php @@ -24,13 +24,9 @@ declare(strict_types=1); namespace OCA\Memories\ClustersBackend; use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\IRequest; abstract class Backend { - /** Mapping of backend name to className */ - public static array $backends = []; - /** * A human-readable name for the app. * Used for error messages. @@ -71,66 +67,6 @@ abstract class Backend */ abstract public function getPhotos(string $name, ?int $limit = null): array; - /** - * Get a cluster backend. - * - * @param string $name Name of the backend - * - * @throws \Exception If the backend is not registered - */ - public static function get(string $name): self - { - if (!\array_key_exists($name, self::$backends)) { - throw new \Exception("Invalid clusters backend '{$name}'"); - } - - return \OC::$server->get(self::$backends[$name]); - } - - /** - * Apply all query transformations for the given request. - */ - public static function getTransforms(IRequest $request): array - { - $transforms = []; - foreach (array_keys(self::$backends) as $backendName) { - if ($request->getParam($backendName)) { - $backend = self::get($backendName); - if ($backend->isEnabled()) { - $transforms[] = [$backend, 'transformDayQuery']; - } - } - } - - return $transforms; - } - - /** - * Apply all post-query transformations for the given day object. - */ - public static function applyDayPostTransforms(IRequest $request, array &$row): void - { - foreach (array_keys(self::$backends) as $backendName) { - if ($request->getParam($backendName)) { - $backend = self::get($backendName); - if ($backend->isEnabled()) { - $backend->transformDayPost($row); - } - } - } - } - - /** - * Register a new backend. - * - * @param mixed $name - * @param mixed $className - */ - public static function register($name, $className): void - { - self::$backends[$name] = $className; - } - /** * Human readable name for the cluster. */ diff --git a/lib/Controller/ClustersController.php b/lib/Controller/ClustersController.php index c9a74ffa..62ffb25f 100644 --- a/lib/Controller/ClustersController.php +++ b/lib/Controller/ClustersController.php @@ -25,6 +25,7 @@ namespace OCA\Memories\Controller; use OCA\Memories\ClustersBackend\Backend; use OCA\Memories\Exceptions; +use OCA\Memories\Manager\ClustersBackendManager; use OCA\Memories\Util; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataDisplayResponse; @@ -114,7 +115,7 @@ class ClustersController extends GenericApiController throw Exceptions::NotLoggedIn(); } - $this->backend = Backend::get($backend); + $this->backend = ClustersBackendManager::get($backend); if (!$this->backend->isEnabled()) { throw Exceptions::NotEnabled($this->backend->appName()); @@ -130,23 +131,13 @@ class ClustersController extends GenericApiController $previewManager = \OC::$server->get(\OCP\IPreview::class); // Try to get a preview - $userFolder = Util::getUserFolder(); foreach ($photos as $img) { - // Get the file - $files = $userFolder->getById($this->backend->getFileId($img)); - if (0 === \count($files)) { - continue; - } - - // Check read permission - if (!$files[0]->isReadable()) { - continue; - } - // Get preview image try { $quality = $this->backend->getPreviewQuality(); - $file = $previewManager->getPreview($files[0], $quality, $quality, false); + + $file = $this->fs->getUserFile($this->backend->getFileId($img)); + $file = $previewManager->getPreview($file, $quality, $quality, false); [$blob, $mimetype] = $this->backend->getPreviewBlob($file, $img); diff --git a/lib/Controller/DaysController.php b/lib/Controller/DaysController.php index 9c5304ce..561ab7bc 100644 --- a/lib/Controller/DaysController.php +++ b/lib/Controller/DaysController.php @@ -24,6 +24,7 @@ declare(strict_types=1); namespace OCA\Memories\Controller; use OCA\Memories\Exceptions; +use OCA\Memories\Manager\ClustersBackendManager; use OCA\Memories\Util; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; @@ -145,7 +146,7 @@ class DaysController extends GenericApiController $transforms = []; // Add clustering transforms - $transforms = array_merge($transforms, \OCA\Memories\ClustersBackend\Backend::getTransforms($this->request)); + $transforms = array_merge($transforms, ClustersBackendManager::getTransforms($this->request)); // Other transforms not allowed for public shares if (!Util::isLoggedIn()) { @@ -259,4 +260,24 @@ class DaysController extends GenericApiController return $dayIds; } + + private function isRecursive() + { + return null === $this->request->getParam('folder') || $this->request->getParam('recursive'); + } + + private function isArchive() + { + return null !== $this->request->getParam('archive'); + } + + private function isMonthView() + { + return null !== $this->request->getParam('monthView'); + } + + private function isReverse() + { + return null !== $this->request->getParam('reverse'); + } } diff --git a/lib/Controller/GenericApiController.php b/lib/Controller/GenericApiController.php index 58028956..db67d3b3 100644 --- a/lib/Controller/GenericApiController.php +++ b/lib/Controller/GenericApiController.php @@ -37,8 +37,6 @@ use Psr\Log\LoggerInterface; abstract class GenericApiController extends Controller { - use GenericApiControllerParams; - protected IConfig $config; protected IUserSession $userSession; protected IRootFolder $rootFolder; diff --git a/lib/Controller/GenericApiControllerParams.php b/lib/Controller/GenericApiControllerParams.php deleted file mode 100644 index b7b70cd1..00000000 --- a/lib/Controller/GenericApiControllerParams.php +++ /dev/null @@ -1,54 +0,0 @@ - - * @author Varun Patil - * @license AGPL-3.0-or-later - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -namespace OCA\Memories\Controller; - -trait GenericApiControllerParams -{ - /** - * current request. - * - * @var \OCP\IRequest - */ - protected $request; - - protected function isRecursive() - { - return null === $this->request->getParam('folder') || $this->request->getParam('recursive'); - } - - protected function isArchive() - { - return null !== $this->request->getParam('archive'); - } - - protected function isMonthView() - { - return null !== $this->request->getParam('monthView'); - } - - protected function isReverse() - { - return null !== $this->request->getParam('reverse'); - } -} diff --git a/lib/Controller/MapController.php b/lib/Controller/MapController.php index 999efeed..7660b942 100644 --- a/lib/Controller/MapController.php +++ b/lib/Controller/MapController.php @@ -33,21 +33,19 @@ class MapController extends GenericApiController /** * @NoAdminRequired */ - public function clusters(): Http\Response + public function clusters(string $bounds, string $zoom): Http\Response { - return Util::guardEx(function () { + return Util::guardEx(function () use ($bounds, $zoom) { // Make sure we have bounds and zoom level // Zoom level is used to determine the grid length - $bounds = $this->request->getParam('bounds'); - $zoomLevel = $this->request->getParam('zoom'); - if (!$bounds || !$zoomLevel || !is_numeric($zoomLevel)) { + if (!$bounds || !$zoom || !is_numeric($zoom)) { throw Exceptions::MissingParameter('bounds or zoom'); } // A tweakable parameter to determine the number of boxes in the map // Note: these parameters need to be changed in MapSplitMatter.vue as well $clusterDensity = 1; - $gridLen = 180.0 / (2 ** $zoomLevel * $clusterDensity); + $gridLen = 180.0 / (2 ** $zoom * $clusterDensity); $clusters = $this->timelineQuery->getMapClusters($gridLen, $bounds); diff --git a/lib/Controller/OtherController.php b/lib/Controller/OtherController.php index 5b284271..0ee5f9ea 100644 --- a/lib/Controller/OtherController.php +++ b/lib/Controller/OtherController.php @@ -45,14 +45,12 @@ class OtherController extends GenericApiController public function setUserConfig(string $key, string $value): Http\Response { return Util::guardEx(function () use ($key, $value) { - $uid = Util::getUID(); - // Make sure not running in read-only mode if ($this->config->getSystemValue('memories.readonly', false)) { throw Exceptions::Forbidden('Cannot change settings in readonly mode'); } - $this->config->setUserValue($uid, Application::APPNAME, $key, $value); + $this->config->setUserValue(Util::getUID(), Application::APPNAME, $key, $value); return new JSONResponse([], Http::STATUS_OK); }); diff --git a/lib/Controller/TagsController.php b/lib/Controller/TagsController.php index 37e9116a..e1d34976 100644 --- a/lib/Controller/TagsController.php +++ b/lib/Controller/TagsController.php @@ -47,7 +47,7 @@ class TagsController extends GenericApiController $file = $this->fs->getUserFile($id); // Check the user is allowed to edit the file - if (!$file->isUpdateable() || !($file->getPermissions() & \OCP\Constants::PERMISSION_UPDATE)) { + if (!$file->isUpdateable()) { throw Exceptions::ForbiddenFileUpdate($file->getName()); } diff --git a/lib/Db/AlbumsQuery.php b/lib/Db/AlbumsQuery.php index e9b5c29d..e04067d5 100644 --- a/lib/Db/AlbumsQuery.php +++ b/lib/Db/AlbumsQuery.php @@ -124,7 +124,7 @@ class AlbumsQuery * @param string $uid UID of CURRENT user * @param string $albumId $user/$name where $user is the OWNER of the album */ - public function getIfAllowed(string $uid, string $albumId) + public function getIfAllowed(string $uid, string $albumId): ?array { $album = null; diff --git a/lib/Db/TimelineQueryDays.php b/lib/Db/TimelineQueryDays.php index a97d592d..39680d00 100644 --- a/lib/Db/TimelineQueryDays.php +++ b/lib/Db/TimelineQueryDays.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace OCA\Memories\Db; +use OCA\Memories\Manager\ClustersBackendManager; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; @@ -274,7 +275,7 @@ trait TimelineQueryDays } // All cluster transformations - \OCA\Memories\ClustersBackend\Backend::applyDayPostTransforms($this->request, $row); + ClustersBackendManager::applyDayPostTransforms($this->request, $row); // We don't need these fields unset($row['datetaken']); diff --git a/lib/Manager/ClustersBackendManager.php b/lib/Manager/ClustersBackendManager.php new file mode 100644 index 00000000..343b47ca --- /dev/null +++ b/lib/Manager/ClustersBackendManager.php @@ -0,0 +1,93 @@ + + * @author Varun Patil + * @license AGPL-3.0-or-later + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +namespace OCA\Memories\Manager; + +use OCA\Memories\ClustersBackend\Backend; +use OCP\IRequest; + +class ClustersBackendManager +{ + /** Mapping of backend name to className */ + public static array $backends = []; + + /** + * Get a cluster backend. + * + * @param string $name Name of the backend + * + * @throws \Exception If the backend is not registered + */ + public static function get(string $name): Backend + { + if (!\array_key_exists($name, self::$backends)) { + throw new \Exception("Invalid clusters backend '{$name}'"); + } + + return \OC::$server->get(self::$backends[$name]); + } + + /** + * Register a new backend. + * + * @param mixed $name + * @param mixed $className + */ + public static function register($name, $className): void + { + self::$backends[$name] = $className; + } + + /** + * Apply all query transformations for the given request. + */ + public static function getTransforms(IRequest $request): array + { + $transforms = []; + foreach (array_keys(self::$backends) as $backendName) { + if ($request->getParam($backendName)) { + $backend = self::get($backendName); + if ($backend->isEnabled()) { + $transforms[] = [$backend, 'transformDayQuery']; + } + } + } + + return $transforms; + } + + /** + * Apply all post-query transformations for the given day object. + */ + public static function applyDayPostTransforms(IRequest $request, array &$row): void + { + foreach (array_keys(self::$backends) as $backendName) { + if ($request->getParam($backendName)) { + $backend = self::get($backendName); + if ($backend->isEnabled()) { + $backend->transformDayPost($row); + } + } + } + } +}