refactor: use generic controller for most clusters

Signed-off-by: Varun Patil <varunpatil@ucla.edu>

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#list', 'url' => '/api/tags', 'verb' => 'GET'],
-        ['name' => 'Tags#preview', 'url' => '/api/tags/preview/{tag}', '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#list', 'url' => '/api/places', 'verb' => 'GET'],
-        ['name' => 'Places#preview', 'url' => '/api/places/preview/{id}', 'verb' => 'GET'],
+        ['name' => 'Places#preview', 'url' => '/api/places/preview/{name}', 'verb' => 'GET'],
 
         ['name' => 'Map#clusters', 'url' => '/api/map/clusters', 'verb' => 'GET'],
 

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 OCA\Memories\HttpResponseException;
-use OCP\AppFramework\Http\JSONResponse;
 
-class AlbumsController extends GenericApiController
+class AlbumsController extends GenericClusterController
 {
-    /**
+    protected function appName(): string
-     * @NoAdminRequired
-     *
-     * Get list of albums with counts of images
-     */
-    public function albums(int $t = 0): Http\Response
     {
-        $user = $this->userSession->getUser();
+        return 'Albums';
-        if (null === $user) {
+    }
-            return Errors::NotLoggedIn();
-        }
 
-        if (!$this->albumsIsEnabled()) {
+    protected function isEnabled(): bool
-            return Errors::NotEnabled('Albums');
+    {
-        }
+        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 array_values($list);
-
-        return new JSONResponse($list, Http::STATUS_OK);
     }
 
-    /**
+    protected function getFileIds(string $name, ?int $limit = null): array
-     * @NoAdminRequired
-     *
-     * @UseSession
-     *
-     * Download an album as a zip file
-     */
-    public function download(string $name = ''): Http\Response
     {
-        $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']);
+        $list = $this->timelineQuery->getAlbumFiles((int) $album['album_id'], $limit) ?? [];
-        if (empty($files)) {
-            return Errors::NotFound("zero files in album {$name}");
-        }
 
-        // Get download handle
+        return array_map(fn ($item) => (int) $item['fileid'], $list);
-        $albumName = explode('/', $name)[1];
-        $handle = \OCA\Memories\Controller\DownloadController::createHandle($albumName, $files);
-
-        return new JSONResponse(['handle' => $handle], Http::STATUS_OK);
     }
 }

lib/Controller/GenericClusterController.php

@@ -0,0 +1,183 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2022 Varun Patil <radialapps@gmail.com>
+ * @author Varun Patil <radialapps@gmail.com>
+ * @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 <http://www.gnu.org/licenses/>.
+ */
+
+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;
+    }
+}

lib/Controller/PlacesController.php

@@ -23,76 +23,27 @@ declare(strict_types=1);
 
 namespace OCA\Memories\Controller;
 
-use OCA\Memories\Errors;
+class PlacesController extends GenericClusterController
-use OCP\AppFramework\Http;
-use OCP\AppFramework\Http\JSONResponse;
-
-class PlacesController extends GenericApiController
 {
-    /**
+    protected function appName(): string
-     * @NoAdminRequired
-     *
-     * Get list of places with counts of images
-     */
-    public function places(): Http\Response
     {
-        $user = $this->userSession->getUser();
+        return 'Places';
-        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);
     }
 
-    /**
+    protected function isEnabled(): bool
-     * @NoAdminRequired
-     *
-     * @NoCSRFRequired
-     *
-     * Get preview for a location
-     */
-    public function preview(int $id): Http\Response
     {
-        $user = $this->userSession->getUser();
+        return $this->placesIsEnabled();
-        if (null === $user) {
+    }
-            return Errors::NotLoggedIn();
-        }
 
-        // Check tags enabled for this user
+    protected function getClusters(): array
-        if (!$this->placesIsEnabled()) {
+    {
-            return Errors::NotEnabled('places');
+        return $this->timelineQuery->getPlaces($this->root);
-        }
+    }
 
-        // If this isn't the timeline folder then things aren't going to work
+    protected function getFileIds(string $name, ?int $limit = null): array
-        $root = $this->getRequestRoot();
+    {
-        if ($root->isEmpty()) {
+        $list = $this->timelineQuery->getPlaceFiles((int) $name, $this->root, $limit) ?? [];
-            return Errors::NoRequestRoot();
-        }
 
-        // Run actual query
+        return array_map(fn ($item) => (int) $item['fileid'], $list);
-        $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));
     }
 }

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);
+    }
 }

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 = [];
+        if (null !== $limit) {
-        $result = $query->executeQuery();
+            $query->setMaxResults($limit);
-        while ($row = $result->fetch()) {
-            $fileIds[] = (int) $row['file_id'];
         }
 
-        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 */

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
+        // MAX number of files
-        $query->setMaxResults(8);
+        if (null !== $limit) {
+            $query->setMaxResults($limit);
+        }
 
         // FETCH tag previews
         $cursor = $this->executeQueryWithCTEs($query);

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
+        // MAX number of files
-        $query->setMaxResults(8);
+        if (null !== $limit) {
+            $query->setMaxResults($limit);
+        }
 
         // FETCH tag previews
         $cursor = $this->executeQueryWithCTEs($query);

lib/HttpResponseException.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace OCA\Memories;
+
+use OCP\AppFramework\Http;
+
+class HttpResponseException extends \Exception
+{
+    public Http\Response $response;
+
+    public function __construct(Http\Response $response)
+    {
+        $this->response = $response;
+    }
+}