More refactor

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

lib/ClustersBackend/AlbumsBackend.php

@@ -26,7 +26,6 @@ namespace OCA\Memories\ClustersBackend;
 use OCA\Memories\Db\AlbumsQuery;
 use OCA\Memories\Exceptions;
 use OCA\Memories\Util;
-use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IRequest;
 
 class AlbumsBackend extends Backend
@@ -57,7 +56,7 @@ class AlbumsBackend extends Backend
         return explode('/', $name)[1];
     }
 
-    public function transformDays(IQueryBuilder &$query, bool $aggregate): void
+    public function transformDayQuery(&$query, bool $aggregate): void
     {
         $albumId = (string) $this->request->getParam('albums');
 

lib/ClustersBackend/Backend.php

@@ -44,13 +44,16 @@ abstract class Backend
 
     /**
      * Apply query transformations for days query.
+     *
+     * @param IQueryBuilder $query     Query builder
+     * @param bool          $aggregate Whether this is an aggregate query
      */
-    abstract public function transformDays(IQueryBuilder &$query, bool $aggregate): void;
+    abstract public function transformDayQuery(&$query, bool $aggregate): void;
 
     /**
-     * Apply post-query transformations for the given day object.
+     * Apply post-query transformations for the given photo object.
      */
-    public function transformDayPhoto(array &$row): void
+    public function transformDayPost(array &$row): void
     {
     }
 
@@ -94,7 +97,7 @@ abstract class Backend
             if ($request->getParam($backendName)) {
                 $backend = self::get($backendName);
                 if ($backend->isEnabled()) {
-                    $transforms[] = [$backend, 'transformDays'];
+                    $transforms[] = [$backend, 'transformDayQuery'];
                 }
             }
         }
@@ -111,7 +114,7 @@ abstract class Backend
             if ($request->getParam($backendName)) {
                 $backend = self::get($backendName);
                 if ($backend->isEnabled()) {
-                    $backend->transformDayPhoto($row);
+                    $backend->transformDayPost($row);
                 }
             }
         }
@@ -159,7 +162,7 @@ abstract class Backend
      * @param \OCP\Files\SimpleFS\ISimpleFile $file  Preview file
      * @param array                           $photo Photo object
      *
-     * @return [Blob, mimetype] of data
+     * @return array [Blob, mimetype] of data
      */
     public function getPreviewBlob($file, $photo): array
     {

lib/ClustersBackend/FaceRecognitionBackend.php

@@ -25,7 +25,6 @@ namespace OCA\Memories\ClustersBackend;
 
 use OCA\Memories\Db\TimelineQuery;
 use OCA\Memories\Util;
-use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IConfig;
 use OCP\IRequest;
 
@@ -58,7 +57,7 @@ class FaceRecognitionBackend extends Backend
                && Util::facerecognitionIsEnabled();
     }
 
-    public function transformDays(IQueryBuilder &$query, bool $aggregate): void
+    public function transformDayQuery(&$query, bool $aggregate): void
     {
         $personStr = (string) $this->request->getParam('facerecognition');
 
@@ -78,12 +77,7 @@ class FaceRecognitionBackend extends Backend
         ));
 
         // Join with faces
-        $query->innerJoin(
+        $query->innerJoin('fri', 'facerecog_faces', 'frf', $query->expr()->eq('frf.image', 'fri.id'));
-            'fri',
-            'facerecog_faces',
-            'frf',
-            $query->expr()->eq('frf.image', 'fri.id')
-        );
 
         // Join with persons
         $nameField = is_numeric($personName) ? 'frp.id' : 'frp.name';
@@ -106,7 +100,7 @@ class FaceRecognitionBackend extends Backend
         }
     }
 
-    public function transformDayPhoto(array &$row): void
+    public function transformDayPost(array &$row): void
     {
         // Differentiate Recognize queries from Face Recognition
         if (!isset($row) || !isset($row['face_width']) || !isset($row['image_width'])) {
@@ -168,13 +162,10 @@ class FaceRecognitionBackend extends Backend
         $query->innerJoin('fri', 'memories', 'm', $query->expr()->eq('m.fileid', 'fri.file'));
 
         $query->innerJoin('frf', 'facerecog_persons', 'frp', $query->expr()->eq('frp.id', 'frf.person'));
-        if (is_numeric($name)) {
+
-            // WHERE faces are from id persons (a cluster).
+        // WHERE faces are from id persons (or a cluster).
-            $query->where($query->expr()->eq('frp.id', $query->createNamedParameter($name)));
+        $nameField = is_numeric($name) ? 'frp.id' : 'frp.name';
-        } else {
+        $query->where($query->expr()->eq($nameField, $query->createNamedParameter($name)));
-            // WHERE faces are from name on persons.
-            $query->where($query->expr()->eq('frp.name', $query->createNamedParameter($name)));
-        }
 
         // WHERE these photos are in the user's requested folder recursively
         $query = $this->tq->joinFilecache($query);

lib/ClustersBackend/PlacesBackend.php

@@ -25,7 +25,6 @@ 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 +48,7 @@ class PlacesBackend extends Backend
         return Util::placesGISType() > 0;
     }
 
-    public function transformDays(IQueryBuilder &$query, bool $aggregate): void
+    public function transformDayQuery(&$query, bool $aggregate): void
     {
         $locationId = (int) $this->request->getParam('places');
 

lib/ClustersBackend/RecognizeBackend.php

@@ -25,7 +25,6 @@ namespace OCA\Memories\ClustersBackend;
 
 use OCA\Memories\Db\TimelineQuery;
 use OCA\Memories\Util;
-use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IRequest;
 
 class RecognizeBackend extends Backend
@@ -51,7 +50,7 @@ class RecognizeBackend extends Backend
         return Util::recognizeIsEnabled();
     }
 
-    public function transformDays(IQueryBuilder &$query, bool $aggregate): void
+    public function transformDayQuery(&$query, bool $aggregate): void
     {
         $faceStr = (string) $this->request->getParam('recognize');
 
@@ -103,7 +102,7 @@ class RecognizeBackend extends Backend
         ));
     }
 
-    public function transformDayPhoto(array &$row): void
+    public function transformDayPost(array &$row): void
     {
         // Differentiate Recognize queries from Face Recognition
         if (!isset($row) || !isset($row['face_w'])) {

lib/ClustersBackend/TagsBackend.php

@@ -49,7 +49,7 @@ class TagsBackend extends Backend
         return Util::tagsIsEnabled();
     }
 
-    public function transformDays(IQueryBuilder &$query, bool $aggregate): void
+    public function transformDayQuery(&$query, bool $aggregate): void
     {
         $tagName = (string) $this->request->getParam('tags');
 

lib/Controller/DownloadController.php

@@ -129,9 +129,6 @@ class DownloadController extends GenericApiController
     {
         return Util::guardEx(function () use ($fileid) {
             $file = $this->fs->getUserFile($fileid);
-            if (null === $file) {
-                return Exceptions::NotFoundFile($fileid);
-            }
 
             // Get the owner's root folder
             $owner = $file->getOwner()->getUID();
@@ -216,9 +213,6 @@ class DownloadController extends GenericApiController
             try {
                 // This checks permissions
                 $file = $this->fs->getUserFile($fileId);
-                if (null === $file) {
-                    throw new \Exception('File not found');
-                }
                 $name = $file->getName();
 
                 // Open file

lib/Controller/ImageController.php

@@ -56,10 +56,6 @@ class ImageController extends GenericApiController
             }
 
             $file = $this->fs->getUserFile($id);
-            if (!$file) {
-                throw Exceptions::NotFoundFile($id);
-            }
-
             $preview = \OC::$server->get(\OCP\IPreview::class)->getPreview($file, $x, $y, !$a, $mode);
             $response = new FileDisplayResponse($preview, Http::STATUS_OK, [
                 'Content-Type' => $preview->getMimeType(),
@@ -113,13 +109,9 @@ class ImageController extends GenericApiController
                     continue;
                 }
 
-                $file = $this->fs->getUserFile($fileid);
-                if (!$file) {
-                    continue;
-                }
-
                 try {
                     // Make sure max preview exists
+                    $file = $this->fs->getUserFile($fileid);
                     $fileId = (string) $file->getId();
                     $folder = $previewRoot->getFolder($fileId);
                     $hasMax = false;
@@ -179,9 +171,6 @@ class ImageController extends GenericApiController
     ): Http\Response {
         return Util::guardEx(function () use ($id, $basic, $current, $tags) {
             $file = $this->fs->getUserFile((int) $id);
-            if (!$file) {
-                throw Exceptions::NotFoundFile($id);
-            }
 
             // Get the image info
             $info = $this->timelineQuery->getInfoById($file->getId(), $basic);
@@ -223,9 +212,6 @@ class ImageController extends GenericApiController
     {
         return Util::guardEx(function () use ($id, $raw) {
             $file = $this->fs->getUserFile((int) $id);
-            if (!$file) {
-                throw Exceptions::NotFoundFile($id);
-            }
 
             // Check if user has permissions
             if (!$file->isUpdateable() || Util::isEncryptionEnabled()) {
@@ -262,9 +248,6 @@ class ImageController extends GenericApiController
     {
         return Util::guardEx(function () use ($id) {
             $file = $this->fs->getUserFile((int) $id);
-            if (!$file) {
-                throw Exceptions::NotFoundFile($id);
-            }
 
             // Check if valid image
             $mimetype = $file->getMimeType();

lib/Controller/TagsController.php

@@ -45,9 +45,6 @@ class TagsController extends GenericApiController
 
             // Check the user is allowed to edit the file
             $file = $this->fs->getUserFile($id);
-            if (null === $file) {
-                throw Exceptions::NotFoundFile($id);
-            }
 
             // Check the user is allowed to edit the file
             if (!$file->isUpdateable() || !($file->getPermissions() & \OCP\Constants::PERMISSION_UPDATE)) {

lib/Controller/VideoController.php

@@ -57,9 +57,6 @@ class VideoController extends GenericApiController
 
             // Get file
             $file = $this->fs->getUserFile($fileid);
-            if (!$file || !$file->isReadable()) {
-                throw Exceptions::NotFoundFile($fileid);
-            }
 
             // Local files only for now
             if (!$file->getStorage()->isLocal()) {
@@ -117,9 +114,6 @@ class VideoController extends GenericApiController
     ) {
         return Util::guardEx(function () use ($fileid, $liveid, $format, $transcode) {
             $file = $this->fs->getUserFile($fileid);
-            if (null === $file) {
-                throw Exceptions::NotFoundFile($fileid);
-            }
 
             // Check file liveid
             if (!$liveid) {

lib/Manager/FsManager.php

@@ -25,6 +25,7 @@ namespace OCA\Memories\Manager;
 
 use OCA\Memories\Db\AlbumsQuery;
 use OCA\Memories\Db\TimelineRoot;
+use OCA\Memories\Exceptions;
 use OCA\Memories\Exif;
 use OCA\Memories\Util;
 use OCP\Files\File;
@@ -121,8 +122,23 @@ class FsManager
 
     /**
      * Get a file with ID for the current user.
+     *
+     * @throws Exceptions\NotFoundFile
      */
-    public function getUserFile(int $fileId): ?File
+    public function getUserFile(int $fileId): File
+    {
+        $file = $this->getUserFileOrNull($fileId);
+        if (null === $file) {
+            throw Exceptions::NotFoundFile($fileId);
+        }
+
+        return $file;
+    }
+
+    /**
+     * Get a file with ID for the current user.
+     */
+    public function getUserFileOrNull(int $fileId): ?File
     {
         // Don't check self for share token
         if ($this->getShareToken()) {