From 1c024211316d96583c67e517f22b97a4f1075104 Mon Sep 17 00:00:00 2001 From: Yuriy Bakhtin Date: Fri, 20 Sep 2024 19:16:28 +0200 Subject: [PATCH 1/2] Optimize sql query to get files from the stream --- docs/CHANGELOG.md | 1 + models/File.php | 51 +++++++++++++++++++++++++++-------------------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 9d4a6ef..ee3cdce 100644 --- 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/models/File.php b/models/File.php index 609dbc9..0ab6102 100644 --- 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; @@ -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); } From c44209d97a44a84657d4e0d773e41c06f1813b11 Mon Sep 17 00:00:00 2001 From: yurabakhtin Date: Fri, 20 Sep 2024 17:16:49 +0000 Subject: [PATCH 2/2] Autocommit PHP CS Fixer --- controllers/BaseController.php | 4 ++-- controllers/BrowseController.php | 2 +- controllers/DeleteController.php | 2 +- controllers/EditController.php | 2 +- controllers/MoveController.php | 2 +- controllers/rest/FileController.php | 2 +- controllers/rest/ManageController.php | 8 ++++---- libs/ZipExtractor.php | 8 ++++---- migrations/m150720_174011_initial.php | 8 ++++---- migrations/m170830_122439_foreignkeys.php | 2 +- models/File.php | 12 ++++++------ models/Folder.php | 4 ++-- models/rows/AbstractFileSystemItemRow.php | 6 +++--- tests/codeception/_support/AcceptanceTester.php | 2 +- widgets/FileList.php | 6 +++--- 15 files changed, 35 insertions(+), 35 deletions(-) diff --git a/controllers/BaseController.php b/controllers/BaseController.php index 8c04c8a..7c3886e 100644 --- a/controllers/BaseController.php +++ b/controllers/BaseController.php @@ -45,7 +45,7 @@ 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(); @@ -53,7 +53,7 @@ public function beforeAction($action) 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 index f73e097..eb67ccc 100644 --- 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 index 67a7130..4e2f101 100644 --- 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 index 2f52f81..826e415 100644 --- 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 index 779d706..787dc39 100644 --- 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 index 9a92b3f..499e5e4 100644 --- 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 index a0f29b2..5e9fbc2 100644 --- 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/libs/ZipExtractor.php b/libs/ZipExtractor.php index 1a47d93..751aaca 100644 --- 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 index 11b6a52..aaa3f43 100644 --- 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 index 9dfda27..867b1fd 100644 --- 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 index 0ab6102..4cc9851 100644 --- a/models/File.php +++ b/models/File.php @@ -97,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]; } @@ -124,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(); } @@ -206,7 +206,7 @@ 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; } @@ -214,8 +214,8 @@ public function updateVisibility($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.'); diff --git a/models/Folder.php b/models/Folder.php index aa00c74..9700717 100644 --- 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 index 5b489ce..a5d0a97 100644 --- 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 index ca5f7a3..9449c03 100644 --- 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 index 803fefc..7d76bf9 100644 --- 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();