From fffc59779714d6a2147494cff688ccd4a34d4f49 Mon Sep 17 00:00:00 2001 From: Varun Patil Date: Wed, 22 Mar 2023 12:54:03 -0700 Subject: [PATCH] refactor: use generic controller for most clusters Signed-off-by: Varun Patil --- appinfo/routes.php | 10 +- lib/Controller/AlbumsController.php | 79 ++++----- lib/Controller/GenericClusterController.php | 183 ++++++++++++++++++++ lib/Controller/PlacesController.php | 77 ++------ lib/Controller/TagsController.php | 95 +++------- lib/Db/TimelineQueryAlbums.php | 16 +- lib/Db/TimelineQueryPlaces.php | 8 +- lib/Db/TimelineQueryTags.php | 8 +- lib/HttpResponseException.php | 15 ++ 9 files changed, 291 insertions(+), 200 deletions(-) create mode 100644 lib/Controller/GenericClusterController.php create mode 100644 lib/HttpResponseException.php diff --git a/appinfo/routes.php b/appinfo/routes.php index 02425e10..05a75d7e 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -52,11 +52,11 @@ return [ ['name' => 'Days#day', 'url' => '/api/days/{id}', 'verb' => 'GET'], ['name' => 'Days#dayPost', 'url' => '/api/days', 'verb' => 'POST'], - ['name' => 'Albums#albums', 'url' => '/api/albums', 'verb' => 'GET'], + ['name' => 'Albums#list', 'url' => '/api/albums', 'verb' => 'GET'], ['name' => 'Albums#download', 'url' => '/api/albums/download', 'verb' => 'POST'], - ['name' => 'Tags#tags', 'url' => '/api/tags', 'verb' => 'GET'], - ['name' => 'Tags#preview', 'url' => '/api/tags/preview/{tag}', 'verb' => 'GET'], + ['name' => 'Tags#list', 'url' => '/api/tags', 'verb' => 'GET'], + ['name' => 'Tags#preview', 'url' => '/api/tags/preview/{name}', 'verb' => 'GET'], ['name' => 'Tags#set', 'url' => '/api/tags/set/{id}', 'verb' => 'PATCH'], ['name' => 'People#recognizePeople', 'url' => '/api/recognize/people', 'verb' => 'GET'], @@ -64,8 +64,8 @@ return [ ['name' => 'People#facerecognitionPeople', 'url' => '/api/facerecognition/people', 'verb' => 'GET'], ['name' => 'People#facerecognitionPeoplePreview', 'url' => '/api/facerecognition/people/preview/{id}', 'verb' => 'GET'], - ['name' => 'Places#places', 'url' => '/api/places', 'verb' => 'GET'], - ['name' => 'Places#preview', 'url' => '/api/places/preview/{id}', 'verb' => 'GET'], + ['name' => 'Places#list', 'url' => '/api/places', 'verb' => 'GET'], + ['name' => 'Places#preview', 'url' => '/api/places/preview/{name}', 'verb' => 'GET'], ['name' => 'Map#clusters', 'url' => '/api/map/clusters', 'verb' => 'GET'], diff --git a/lib/Controller/AlbumsController.php b/lib/Controller/AlbumsController.php index 8a3e676f..0a8b5c2b 100644 --- a/lib/Controller/AlbumsController.php +++ b/lib/Controller/AlbumsController.php @@ -24,34 +24,40 @@ declare(strict_types=1); namespace OCA\Memories\Controller; use OCA\Memories\Errors; -use OCP\AppFramework\Http; -use OCP\AppFramework\Http\JSONResponse; +use OCA\Memories\HttpResponseException; -class AlbumsController extends GenericApiController +class AlbumsController extends GenericClusterController { - /** - * @NoAdminRequired - * - * Get list of albums with counts of images - */ - public function albums(int $t = 0): Http\Response + protected function appName(): string { - $user = $this->userSession->getUser(); - if (null === $user) { - return Errors::NotLoggedIn(); - } + return 'Albums'; + } - if (!$this->albumsIsEnabled()) { - return Errors::NotEnabled('Albums'); - } + protected function isEnabled(): bool + { + return $this->albumsIsEnabled(); + } + protected function useTimelineRoot(): bool + { + return false; + } + + protected function clusterName(string $name) + { + return explode('/', $name)[1]; + } + + protected function getClusters(): array + { // Run actual query $list = []; + $t = (int) $this->request->getParam('t', 0); if ($t & 1) { // personal - $list = array_merge($list, $this->timelineQuery->getAlbums($user->getUID())); + $list = array_merge($list, $this->timelineQuery->getAlbums($this->getUID())); } if ($t & 2) { // shared - $list = array_merge($list, $this->timelineQuery->getAlbums($user->getUID(), true)); + $list = array_merge($list, $this->timelineQuery->getAlbums($this->getUID(), true)); } // Remove elements with duplicate album_id @@ -66,45 +72,20 @@ class AlbumsController extends GenericApiController }); // Convert $list to sequential array - $list = array_values($list); - - return new JSONResponse($list, Http::STATUS_OK); + return array_values($list); } - /** - * @NoAdminRequired - * - * @UseSession - * - * Download an album as a zip file - */ - public function download(string $name = ''): Http\Response + protected function getFileIds(string $name, ?int $limit = null): array { - $user = $this->userSession->getUser(); - if (null === $user) { - return Errors::NotLoggedIn(); - } - - if (!$this->albumsIsEnabled()) { - return Errors::NotEnabled('Albums'); - } - // Get album - $album = $this->timelineQuery->getAlbumIfAllowed($user->getUID(), $name); + $album = $this->timelineQuery->getAlbumIfAllowed($this->getUID(), $name); if (null === $album) { - return Errors::NotFound("album {$name}"); + throw new HttpResponseException(Errors::NotFound("album {$name}")); } // Get files - $files = $this->timelineQuery->getAlbumFiles((int) $album['album_id']); - if (empty($files)) { - return Errors::NotFound("zero files in album {$name}"); - } + $list = $this->timelineQuery->getAlbumFiles((int) $album['album_id'], $limit) ?? []; - // Get download handle - $albumName = explode('/', $name)[1]; - $handle = \OCA\Memories\Controller\DownloadController::createHandle($albumName, $files); - - return new JSONResponse(['handle' => $handle], Http::STATUS_OK); + return array_map(fn ($item) => (int) $item['fileid'], $list); } } diff --git a/lib/Controller/GenericClusterController.php b/lib/Controller/GenericClusterController.php new file mode 100644 index 00000000..78976908 --- /dev/null +++ b/lib/Controller/GenericClusterController.php @@ -0,0 +1,183 @@ + + * @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; + +use OCA\Memories\Db\TimelineRoot; +use OCA\Memories\Errors; +use OCA\Memories\HttpResponseException; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\JSONResponse; + +abstract class GenericClusterController extends GenericApiController +{ + protected ?TimelineRoot $root; + + /** + * @NoAdminRequired + * + * @NoCSRFRequired + * + * Get list of clusters + */ + public function list(): Http\Response + { + try { + $this->init(); + + // Get cluster list that will directly be returned as JSON + $list = $this->getClusters(); + + return new JSONResponse($list, Http::STATUS_OK); + } catch (HttpResponseException $e) { + return $e->response; + } catch (\Exception $e) { + return Errors::Generic($e); + } + } + + /** + * @NoAdminRequired + * + * @NoCSRFRequired + * + * Get preview for a cluster + */ + public function preview(string $name): Http\Response + { + try { + $this->init(); + + // Get list of some files in this cluster + $fileIds = $this->getFileIds($name, 8); + + // If no files found then return 404 + if (0 === \count($fileIds)) { + return new JSONResponse([], Http::STATUS_NOT_FOUND); + } + + // Shuffle the list so we don't always get the same preview + shuffle($fileIds); + + // Get preview from image list + return $this->getPreviewFromImageList($fileIds); + } catch (HttpResponseException $e) { + return $e->response; + } catch (\Exception $e) { + return Errors::Generic($e); + } + } + + /** + * @NoAdminRequired + * + * @UseSession + * + * Download a cluster as a zip file + */ + public function download(string $name): Http\Response + { + try { + $this->init(); + + // Get list of all files in this cluster + $fileIds = $this->getFileIds($name); + + // Get download handle + $filename = $this->clusterName($name); + $handle = \OCA\Memories\Controller\DownloadController::createHandle($filename, $fileIds); + + return new JSONResponse(['handle' => $handle], Http::STATUS_OK); + } catch (HttpResponseException $e) { + return $e->response; + } catch (\Exception $e) { + return Errors::Generic($e); + } + } + + /** + * A human-readable name for the app. + * Used for error messages. + */ + abstract protected function appName(): string; + + /** + * Whether the app is enabled for the current user. + */ + abstract protected function isEnabled(): bool; + + /** + * Get the cluster list for the current user. + */ + abstract protected function getClusters(): array; + + /** + * Get a list of fileids for the given cluster for preview generation. + * + * @param string $name Identifier for the cluster + * @param int $limit Maximum number of fileids to return + * + * @return int[] + */ + abstract protected function getFileIds(string $name, ?int $limit = null): array; + + /** + * Initialize and check if the app is enabled. + * Gets the root node if required. + */ + protected function init(): void + { + $user = $this->userSession->getUser(); + if (null === $user) { + throw new HttpResponseException(Errors::NotLoggedIn()); + } + + if (!$this->isEnabled()) { + throw new HttpResponseException(Errors::NotEnabled($this->appName())); + } + + $this->root = null; + if ($this->useTimelineRoot()) { + $this->root = $this->getRequestRoot(); + if ($this->root->isEmpty()) { + throw new HttpResponseException(Errors::NoRequestRoot()); + } + } + } + + /** + * Should the timeline root be queried? + */ + protected function useTimelineRoot(): bool + { + return true; + } + + /** + * Human readable name for the cluster. + */ + protected function clusterName(string $name) + { + return $name; + } +} diff --git a/lib/Controller/PlacesController.php b/lib/Controller/PlacesController.php index 9cb55bae..6856649f 100644 --- a/lib/Controller/PlacesController.php +++ b/lib/Controller/PlacesController.php @@ -23,76 +23,27 @@ declare(strict_types=1); namespace OCA\Memories\Controller; -use OCA\Memories\Errors; -use OCP\AppFramework\Http; -use OCP\AppFramework\Http\JSONResponse; - -class PlacesController extends GenericApiController +class PlacesController extends GenericClusterController { - /** - * @NoAdminRequired - * - * Get list of places with counts of images - */ - public function places(): Http\Response + protected function appName(): string { - $user = $this->userSession->getUser(); - if (null === $user) { - return Errors::NotLoggedIn(); - } - - // Check tags enabled for this user - if (!$this->placesIsEnabled()) { - return Errors::NotEnabled('places'); - } - - // If this isn't the timeline folder then things aren't going to work - $root = $this->getRequestRoot(); - if ($root->isEmpty()) { - return Errors::NoRequestRoot(); - } - - // Run actual query - $list = $this->timelineQuery->getPlaces($root); - - return new JSONResponse($list, Http::STATUS_OK); + return 'Places'; } - /** - * @NoAdminRequired - * - * @NoCSRFRequired - * - * Get preview for a location - */ - public function preview(int $id): Http\Response + protected function isEnabled(): bool { - $user = $this->userSession->getUser(); - if (null === $user) { - return Errors::NotLoggedIn(); - } + return $this->placesIsEnabled(); + } - // Check tags enabled for this user - if (!$this->placesIsEnabled()) { - return Errors::NotEnabled('places'); - } + protected function getClusters(): array + { + return $this->timelineQuery->getPlaces($this->root); + } - // If this isn't the timeline folder then things aren't going to work - $root = $this->getRequestRoot(); - if ($root->isEmpty()) { - return Errors::NoRequestRoot(); - } + protected function getFileIds(string $name, ?int $limit = null): array + { + $list = $this->timelineQuery->getPlaceFiles((int) $name, $this->root, $limit) ?? []; - // Run actual query - $list = $this->timelineQuery->getPlacePreviews($id, $root); - if (null === $list || 0 === \count($list)) { - return Errors::NotFound('previews'); - } - shuffle($list); - - // Get preview from image list - return $this->getPreviewFromImageList(array_map(function ($item) { - return (int) $item['fileid']; - }, $list)); + return array_map(fn ($item) => (int) $item['fileid'], $list); } } diff --git a/lib/Controller/TagsController.php b/lib/Controller/TagsController.php index c69cd79f..1a8e9aac 100644 --- a/lib/Controller/TagsController.php +++ b/lib/Controller/TagsController.php @@ -27,77 +27,8 @@ use OCA\Memories\Errors; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; -class TagsController extends GenericApiController +class TagsController extends GenericClusterController { - /** - * @NoAdminRequired - * - * Get list of tags with counts of images - */ - public function tags(): Http\Response - { - $user = $this->userSession->getUser(); - if (null === $user) { - return Errors::NotLoggedIn(); - } - - // Check tags enabled for this user - if (!$this->tagsIsEnabled()) { - return Errors::NotEnabled('Tags'); - } - - // If this isn't the timeline folder then things aren't going to work - $root = $this->getRequestRoot(); - if ($root->isEmpty()) { - return Errors::NoRequestRoot(); - } - - // Run actual query - $list = $this->timelineQuery->getTags( - $root, - ); - - return new JSONResponse($list, Http::STATUS_OK); - } - - /** - * @NoAdminRequired - * - * @NoCSRFRequired - * - * Get preview for a tag - */ - public function preview(string $tag): Http\Response - { - $user = $this->userSession->getUser(); - if (null === $user) { - return Errors::NotLoggedIn(); - } - - // Check tags enabled for this user - if (!$this->tagsIsEnabled()) { - return Errors::NotEnabled('Tags'); - } - - // If this isn't the timeline folder then things aren't going to work - $root = $this->getRequestRoot(); - if ($root->isEmpty()) { - return Errors::NoRequestRoot(); - } - - // Run actual query - $list = $this->timelineQuery->getTagPreviews($tag, $root); - if (null === $list || 0 === \count($list)) { - return new JSONResponse([], Http::STATUS_NOT_FOUND); - } - shuffle($list); - - // Get preview from image list - return $this->getPreviewFromImageList(array_map(function ($item) { - return (int) $item['fileid']; - }, $list)); - } - /** * @NoAdminRequired * @@ -107,7 +38,7 @@ class TagsController extends GenericApiController { // Check tags enabled for this user if (!$this->tagsIsEnabled()) { - return Errors::NotEnabled('Tags'); + return Errors::NotEnabled($this->appName()); } // Check the user is allowed to edit the file @@ -130,4 +61,26 @@ class TagsController extends GenericApiController return new JSONResponse([], Http::STATUS_OK); } + + protected function appName(): string + { + return 'Tags'; + } + + protected function isEnabled(): bool + { + return $this->tagsIsEnabled(); + } + + protected function getClusters(): array + { + return $this->timelineQuery->getTags($this->root); + } + + protected function getFileIds(string $name, ?int $limit = null): array + { + $list = $this->timelineQuery->getTagFiles($name, $this->root, $limit) ?? []; + + return array_map(fn ($item) => (int) $item['fileid'], $list); + } } diff --git a/lib/Db/TimelineQueryAlbums.php b/lib/Db/TimelineQueryAlbums.php index b092dae4..03d68768 100644 --- a/lib/Db/TimelineQueryAlbums.php +++ b/lib/Db/TimelineQueryAlbums.php @@ -258,7 +258,7 @@ trait TimelineQueryAlbums /** * Get full list of fileIds in album. */ - public function getAlbumFiles(int $albumId) + public function getAlbumFiles(int $albumId, ?int $limit) { $query = $this->connection->getQueryBuilder(); $query->select('file_id')->from('photos_albums_files', 'paf')->where( @@ -266,13 +266,17 @@ trait TimelineQueryAlbums ); $query->innerJoin('paf', 'filecache', 'fc', $query->expr()->eq('fc.fileid', 'paf.file_id')); - $fileIds = []; - $result = $query->executeQuery(); - while ($row = $result->fetch()) { - $fileIds[] = (int) $row['file_id']; + if (null !== $limit) { + $query->setMaxResults($limit); } - return $fileIds; + $result = $query->executeQuery()->fetchAll(); + + foreach ($result as &$row) { + $row['fileid'] = (int) $row['file_id']; + } + + return $result; } /** Get list of collaborator ids including user id and groups */ diff --git a/lib/Db/TimelineQueryPlaces.php b/lib/Db/TimelineQueryPlaces.php index 801b17cf..b9fd7aa0 100644 --- a/lib/Db/TimelineQueryPlaces.php +++ b/lib/Db/TimelineQueryPlaces.php @@ -54,7 +54,7 @@ trait TimelineQueryPlaces return $places; } - public function getPlacePreviews(int $id, TimelineRoot &$root) + public function getPlaceFiles(int $id, TimelineRoot $root, ?int $limit) { $query = $this->connection->getQueryBuilder(); @@ -69,8 +69,10 @@ trait TimelineQueryPlaces // WHERE these photos are in the user's requested folder recursively $query = $this->joinFilecache($query, $root, true, false); - // MAX 8 - $query->setMaxResults(8); + // MAX number of files + if (null !== $limit) { + $query->setMaxResults($limit); + } // FETCH tag previews $cursor = $this->executeQueryWithCTEs($query); diff --git a/lib/Db/TimelineQueryTags.php b/lib/Db/TimelineQueryTags.php index a6ae73b2..9714d261 100644 --- a/lib/Db/TimelineQueryTags.php +++ b/lib/Db/TimelineQueryTags.php @@ -77,7 +77,7 @@ trait TimelineQueryTags return $tags; } - public function getTagPreviews(string $tagName, TimelineRoot &$root) + public function getTagFiles(string $tagName, TimelineRoot $root, ?int $limit) { $query = $this->connection->getQueryBuilder(); $tagId = $this->getSystemTagId($query, $tagName); @@ -100,8 +100,10 @@ trait TimelineQueryTags // WHERE these photos are in the user's requested folder recursively $query = $this->joinFilecache($query, $root, true, false); - // MAX 8 - $query->setMaxResults(8); + // MAX number of files + if (null !== $limit) { + $query->setMaxResults($limit); + } // FETCH tag previews $cursor = $this->executeQueryWithCTEs($query); diff --git a/lib/HttpResponseException.php b/lib/HttpResponseException.php new file mode 100644 index 00000000..420d4587 --- /dev/null +++ b/lib/HttpResponseException.php @@ -0,0 +1,15 @@ +response = $response; + } +}