Merge pull request #233 from humhub/fix/230-slow-files-from-stream

Optimize sql query to get files from the stream
humhub · Sep 23, 2024 · 5313ad8 · 5313ad8
2 parents eef8f2a + c44209d
commit 5313ad8
Show file tree

Hide file tree

Showing 16 changed files with 65 additions and 57 deletions.
diff --git a/controllers/BaseController.php b/controllers/BaseController.php
@@ -45,15 +45,15 @@ public function beforeAction($action)
         if (!$this->getRootFolder()) {
             $this->_rootFolder = Folder::initRoot($this->contentContainer);
             $newRoot = true;
-        } elseif($this->getRootFolder()->content->isPrivate()) {
+        } elseif ($this->getRootFolder()->content->isPrivate()) {
             // Make sure older root folders are public by default.
             $this->getRootFolder()->content->visibility = Content::VISIBILITY_PUBLIC;
             $this->getRootFolder()->content->save();
         }
 
         if ($this->getAllPostedFilesFolder() == null) {
             $this->_allPostedFilesFolder = Folder::initPostedFilesFolder($this->contentContainer);
-        } elseif($this->getAllPostedFilesFolder()->content->isPrivate()) {
+        } elseif ($this->getAllPostedFilesFolder()->content->isPrivate()) {
             $this->getAllPostedFilesFolder()->content->visibility = Content::VISIBILITY_PUBLIC;
             $this->getAllPostedFilesFolder()->content->save();
         }

diff --git a/controllers/BrowseController.php b/controllers/BrowseController.php
@@ -26,7 +26,7 @@ class BrowseController extends BaseController
     public function actionIndex()
     {
         $currentFolder = $this->getCurrentFolder();
-        if(!$currentFolder->content->canView()) {
+        if (!$currentFolder->content->canView()) {
             throw new HttpException(403);
         }
 

diff --git a/controllers/DeleteController.php b/controllers/DeleteController.php
@@ -32,7 +32,7 @@ public function actionIndex()
             foreach ($selectedItems as $itemId) {
                 $item = FileSystemItem::getItemById($itemId);
 
-                if(!$item->content->canEdit()) {
+                if (!$item->content->canEdit()) {
                     throw new HttpException(403);
                 }
 

diff --git a/controllers/EditController.php b/controllers/EditController.php
@@ -131,7 +131,7 @@ private function updateVisibility(SelectionForm $model, $visibility)
         foreach ($model->selection as $itemId) {
             $item = FileSystemItem::getItemById($itemId);
 
-            if(!$item->content->canEdit()) {
+            if (!$item->content->canEdit()) {
                 throw new HttpException(403);
             }
 

diff --git a/controllers/MoveController.php b/controllers/MoveController.php
@@ -49,7 +49,7 @@ public function actionIndex() //Make sure an $fid is given otherwise the root fo
             ]);
         }
 
-        if($model->save()) {
+        if ($model->save()) {
             $this->view->saved();
             return $this->htmlRedirect($model->destination->createUrl('/cfiles/browse'));
         } else {

diff --git a/controllers/rest/FileController.php b/controllers/rest/FileController.php
@@ -68,7 +68,7 @@ public function actionUpload($containerId)
         foreach ($files as $file) {
             $file = $targetDir->addUploadedFile($file);
 
-            if($file->hasErrors() || $file->baseFile->hasErrors()) {
+            if ($file->hasErrors() || $file->baseFile->hasErrors()) {
                 return $this->returnError(422, "File {$file->baseFile->name} could not be uploaded!", [
                     'errors' => array_merge($file->getErrors(), $file->baseFile->getErrors()),
                 ]);

diff --git a/controllers/rest/ManageController.php b/controllers/rest/ManageController.php
@@ -38,7 +38,7 @@ public function actionDelete($containerId)
         }
 
         foreach ($result as $item) {
-            if(! $item->delete()) {
+            if (! $item->delete()) {
                 Yii::error('Could not delete cFiles items.', 'api');
                 return $this->returnError(500, 'Internal error while deleting cFiles item!');
             }
@@ -81,7 +81,7 @@ public function actionMove($containerId)
             ]);
         }
 
-        if($model->load($params) && $model->save()) {
+        if ($model->load($params) && $model->save()) {
             return $this->returnSuccess('Items successfully moved.');
         }
 
@@ -115,7 +115,7 @@ public function actionMakePublic($containerId)
 
         foreach ($result as $item) {
             $item->updateVisibility(Content::VISIBILITY_PUBLIC);
-            if(! $item->content->save()) {
+            if (! $item->content->save()) {
                 Yii::error('Could not set public visibility for cFiles items.', 'api');
                 return $this->returnError(500, 'Internal error while setting public visibility for cFiles item!');
             }
@@ -144,7 +144,7 @@ public function actionMakePrivate($containerId)
 
         foreach ($result as $item) {
             $item->updateVisibility(Content::VISIBILITY_PRIVATE);
-            if(! $item->content->save()) {
+            if (! $item->content->save()) {
                 Yii::error('Could not set private visibility for cFiles items.', 'api');
                 return $this->returnError(500, 'Internal error while setting private visibility for cFiles item!');
             }

diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
@@ -7,6 +7,7 @@ Changelog
 - Enh #224: Unifying positions of button on modals for consistency and better UX
 - Enh #227: Use PHP CS Fixer
 - Fix: Add autofocus on file or folder edit (for HumHub 1.17 - see https://github.com/humhub/humhub/issues/7136)
+- Fix #230: Optimize sql query to get files from the stream
 
 0.16.6 - March 14, 2024
 -------------------------

diff --git a/libs/ZipExtractor.php b/libs/ZipExtractor.php
@@ -72,7 +72,7 @@ protected function generateModelsFromFilesystem(Folder $targetFolder, $folderPat
         // remove unwanted parent folder references from the scanned files
         $files = array_diff(scandir($folderPath), ['..','.']);
 
-        if(!$root) {
+        if (!$root) {
             $root = $targetFolder;
         }
 
@@ -81,9 +81,9 @@ protected function generateModelsFromFilesystem(Folder $targetFolder, $folderPat
             if (is_dir($filePath)) {
                 $folder = $targetFolder->findFolderByName($file);
 
-                if(!$folder) {
+                if (!$folder) {
                     $folder = $targetFolder->newFolder($file);
-                    if(!$folder->save()) {
+                    if (!$folder->save()) {
                         $root->addError('upload', Yii::t('CfilesModule.base', 'An error occurred while creating folder {folder}.', ['folder' => $file]));
                         continue;
                     }
@@ -92,7 +92,7 @@ protected function generateModelsFromFilesystem(Folder $targetFolder, $folderPat
                 $this->generateModelsFromFilesystem($folder, $filePath, $root);
             } else {
                 $result = $this->generateModelFromFile($targetFolder, $folderPath, $file);
-                if($result->hasErrors()) {
+                if ($result->hasErrors()) {
                     $root->addError('upload', Yii::t('CfilesModule.base', 'An error occurred while unpacking {filename}.', ['filename' => $file]));
                 }
             }

diff --git a/migrations/m150720_174011_initial.php b/migrations/m150720_174011_initial.php
@@ -7,16 +7,16 @@ class m150720_174011_initial extends Migration
 {
     public function up()
     {
-        $this->createTable('cfiles_file', array(
+        $this->createTable('cfiles_file', [
             'id' => 'pk',
             'parent_folder_id' => 'int(11) NULL',
-        ), '');
+        ], '');
 
-        $this->createTable('cfiles_folder', array(
+        $this->createTable('cfiles_folder', [
             'id' => 'pk',
             'parent_folder_id' => 'int(11) NULL',
             'title' => 'varchar(255) NOT NULL',
-        ), '');
+        ], '');
     }
 
     public function down()

diff --git a/migrations/m170830_122439_foreignkeys.php b/migrations/m170830_122439_foreignkeys.php
@@ -15,7 +15,7 @@ public function safeUp()
         try {
             $this->addForeignKey('fk_cfiles_file_parent_folder', 'cfiles_file', 'parent_folder_id', 'cfiles_folder', 'id', 'SET NULL');
             $this->addForeignKey('fk_cfiles_folder_parent_folder', 'cfiles_folder', 'parent_folder_id', 'cfiles_folder', 'id', 'SET NULL');
-        } catch(Exception $e) {
+        } catch (Exception $e) {
             Yii::error($e);
         }
     }

diff --git a/models/File.php b/models/File.php
@@ -10,6 +10,7 @@
 use humhub\modules\file\libs\FileHelper;
 use humhub\modules\file\models\File as BaseFile;
 use humhub\modules\file\models\FileUpload;
+use humhub\modules\post\models\Post;
 use humhub\modules\search\events\SearchAddEvent;
 use humhub\modules\topic\models\Topic;
 use humhub\modules\user\models\User;
@@ -96,7 +97,7 @@ public function rules()
             ['hidden', 'boolean'],
         ];
 
-        if($this->parentFolder && $this->parentFolder->content->isPublic()) {
+        if ($this->parentFolder && $this->parentFolder->content->isPublic()) {
             $rules[] = ['visibility', 'integer', 'min' => 0, 'max' => 1];
         }
 
@@ -123,11 +124,11 @@ public function getSearchAttributes()
             'description' => $this->description,
         ];
 
-        if($this->getCreator()) {
+        if ($this->getCreator()) {
             $attributes['creator'] = $this->getCreator()->getDisplayName();
         }
 
-        if($this->getEditor()) {
+        if ($this->getEditor()) {
             $attributes['editor'] = $this->getEditor()->getDisplayName();
         }
 
@@ -205,16 +206,16 @@ public function updateVisibility($visibility)
             return;
         }
 
-        if(!$this->parentFolder->content->isPrivate() || $visibility == Content::VISIBILITY_PRIVATE) {
+        if (!$this->parentFolder->content->isPrivate() || $visibility == Content::VISIBILITY_PRIVATE) {
             // For user profile files we use Content::VISIBILITY_OWNER isntead of private
             $this->content->visibility = $visibility;
         }
     }
 
     public function getVisibilityTitle()
     {
-        if(Yii::$app->getModule('friendship')->settings->get('enable') && $this->content->container instanceof User) {
-            if($this->content->container->isCurrentuser()) {
+        if (Yii::$app->getModule('friendship')->settings->get('enable') && $this->content->container instanceof User) {
+            if ($this->content->container->isCurrentuser()) {
                 $privateText =  Yii::t('CfilesModule.base', 'This file is only visible for you and your friends.');
             } else {
                 $privateText =  Yii::t('CfilesModule.base', 'This file is protected.');
@@ -323,7 +324,7 @@ public function getEditUrl()
     /**
      * Get the post related to the given file file.
      */
-    public static function getBasePost(\humhub\modules\file\models\File $file = null)
+    public static function getBasePost(BaseFile $file = null)
     {
         if ($file === null) {
             return null;
@@ -386,31 +387,37 @@ public function getFullPath($separator = '/')
      */
     public static function getPostedFiles($contentContainer, $filesOrder = ['file.updated_at' => SORT_ASC, 'file.title' => SORT_ASC])
     {
-        // Get Posted Files
-        $query = \humhub\modules\file\models\File::find();
-        // join comments to the file if available
-        $query->join('LEFT JOIN', 'comment', '(file.object_id=comment.id AND file.object_model=' . Yii::$app->db->quoteValue(Comment::className()) . ')');
-        // join parent post of comment or file
-        $query->join('LEFT JOIN', 'content', '(comment.object_model=content.object_model AND comment.object_id=content.object_id) OR (file.object_model=content.object_model AND file.object_id=content.object_id)');
+        // only accept Posts as the base content, so stuff from sumbmodules like files itsself or gallery will be excluded
 
-        $query->andWhere(['content.contentcontainer_id' => $contentContainer->contentContainerRecord->id]);
+        // Initialise sub queries to get files from Posts and Comments
+        $subQueries = [
+            Post::class => Content::find()
+                ->select('content.object_id')
+                ->where(['content.object_model' => Post::class]),
+            Comment::class => Content::find()
+                ->select('comment.id')
+                ->innerJoin('comment', 'comment.object_model = content.object_model AND comment.object_id = content.object_id')
+                ->where(['comment.object_model' => Post::class]),
+        ];
 
-        if(!$contentContainer->canAccessPrivateContent()) {
-            // Note this will cut comment images, but including the visibility of comments is pretty complex...
-            $query->andWhere(['content.visibility' => Content::VISIBILITY_PUBLIC]);
-        }
+        $query = BaseFile::find();
 
-        $query->andWhere(['content.state' => Content::STATE_PUBLISHED]);
+        foreach ($subQueries as $objectClass => $subQuery) {
+            // Filter Content records by container and visibility states
+            $subQuery->andWhere(['content.contentcontainer_id' => $contentContainer->contentContainerRecord->id])
+                ->andWhere(['content.state' => Content::STATE_PUBLISHED]);
+            if (!$contentContainer->canAccessPrivateContent()) {
+                // Note this will cut comment images, but including the visibility of comments is pretty complex...
+                $subQuery->andWhere(['content.visibility' => Content::VISIBILITY_PUBLIC]);
+            }
+
+            $query->orWhere([
+                'AND',
+                ['file.object_model' => $objectClass],
+                ['IN', 'file.object_id', $subQuery],
+            ]);
+        }
 
-        // only accept Posts as the base content, so stuff from sumbmodules like files itsself or gallery will be excluded
-        $query->andWhere(
-            ['or',
-                ['=', 'comment.object_model', \humhub\modules\post\models\Post::className()],
-                ['=', 'file.object_model', \humhub\modules\post\models\Post::className()],
-            ],
-        );
-
-        // Get Files from comments
         return $query->orderBy($filesOrder);
     }
 

diff --git a/models/Folder.php b/models/Folder.php
@@ -154,11 +154,11 @@ public function getSearchAttributes()
                 'description' => $this->description,
             ];
 
-            if($this->getCreator()) {
+            if ($this->getCreator()) {
                 $attributes['creator'] = $this->getCreator()->getDisplayName();
             }
 
-            if($this->getEditor()) {
+            if ($this->getEditor()) {
                 $attributes['editor'] = $this->getEditor()->getDisplayName();
             }
         }

diff --git a/models/rows/AbstractFileSystemItemRow.php b/models/rows/AbstractFileSystemItemRow.php
@@ -95,7 +95,7 @@ public static function translateOrder($sort = null, $order = 'ASC')
     {
         $result = static::DEFAULT_ORDER;
 
-        if($sort && array_key_exists($sort, static::ORDER_MAPPING)) {
+        if ($sort && array_key_exists($sort, static::ORDER_MAPPING)) {
             $result = static::ORDER_MAPPING[$sort] ? static::ORDER_MAPPING[$sort] . ' ' . $order : $result;
         }
 
@@ -108,11 +108,11 @@ public static function translateOrder($sort = null, $order = 'ASC')
      */
     public function isRenderColumn($column)
     {
-        if($column === self::COLUMN_SELECT && !$this->showSelect) {
+        if ($column === self::COLUMN_SELECT && !$this->showSelect) {
             return false;
         }
 
-        if(!$this->_columns) {
+        if (!$this->_columns) {
             $this->_columns = $this->getColumns();
         }
 

diff --git a/tests/codeception/_support/AcceptanceTester.php b/tests/codeception/_support/AcceptanceTester.php
@@ -129,7 +129,7 @@ public function createFolder($title = 'test', $description = 'test description',
         $this->fillField('Folder[title]', $title);
         $this->fillField('Folder[description]', $description);
 
-        if($isPublic) {
+        if ($isPublic) {
             $this->jsClick('input#folder-visibility');
         }
 

diff --git a/widgets/FileList.php b/widgets/FileList.php
@@ -83,11 +83,11 @@ public function initSort()
         $this->order = Yii::$app->request->get('order', $this->order);
 
         // Save sort settings if sorting was used and logged in user is given or try fetching user settings.
-        if(Yii::$app->request->get('sort') && !Yii::$app->user->isGuest) {
+        if (Yii::$app->request->get('sort') && !Yii::$app->user->isGuest) {
             $settings = $module->settings->user(Yii::$app->user->getIdentity());
             $settings->set('defaultSort', $this->sort);
             $settings->set('defaultOrder', $this->order);
-        } elseif(!Yii::$app->user->isGuest) {
+        } elseif (!Yii::$app->user->isGuest) {
             $settings = $module->settings->user(Yii::$app->user->getIdentity());
             $this->sort = $settings->get('defaultSort', $this->sort);
             $this->order = $settings->get('defaultOrder', $this->order);
@@ -115,7 +115,7 @@ public function getDefaultOrder()
      */
     public function run()
     {
-        if($this->folder->isAllPostedFiles()) {
+        if ($this->folder->isAllPostedFiles()) {
             $this->setPostedFilesRows();
         } else {
             $this->setSystemItemRows();