Merge pull request #686 from jonasraoni/feature/stable-3_4_0/8700-imp…

…rove-slow-query Feature/stable 3 4 0/8700 improve slow query
pkp · May 30, 2024 · 8ba99d5 · 8ba99d5
2 parents 2046034 + 40e11cc
commit 8ba99d5
Show file tree

Hide file tree

Showing 14 changed files with 148 additions and 134 deletions.
diff --git a/classes/author/DAO.php b/classes/author/DAO.php
@@ -106,44 +106,48 @@ public function getAuthorsAlphabetizedByServer($serverId = null, $initial = null
             $initialSql .= ')';
         }
 
+        $baseSql = '
+            FROM authors a
+            JOIN user_groups ug ON (a.user_group_id = ug.user_group_id)
+            JOIN publications p ON (p.publication_id = a.publication_id)
+            JOIN submissions s ON (s.current_publication_id = p.publication_id)
+            LEFT JOIN author_settings agl ON (a.author_id = agl.author_id AND agl.setting_name = ? AND agl.locale = ?)
+            LEFT JOIN author_settings agpl ON (a.author_id = agpl.author_id AND agpl.setting_name = ? AND agpl.locale = s.locale)
+            LEFT JOIN author_settings afl ON (a.author_id = afl.author_id AND afl.setting_name = ? AND afl.locale = ?)
+            LEFT JOIN author_settings afpl ON (a.author_id = afpl.author_id AND afpl.setting_name = ? AND afpl.locale = s.locale)
+            JOIN (
+                SELECT
+                MIN(aa.author_id) as author_id,
+                CONCAT(
+                ' . ($includeEmail ? 'aa.email, \' \', ' : '') . '
+                ac.setting_value,
+                \' \'
+                ' . $sqlColumnsAuthorSettings . '
+                ) as names
+                FROM authors aa
+                JOIN publications pp ON (pp.publication_id = aa.publication_id)
+                LEFT JOIN publication_settings ppss ON (ppss.publication_id = pp.publication_id)
+                JOIN submissions ss ON (ss.submission_id = pp.submission_id AND ss.current_publication_id = pp.publication_id AND ss.status = ' . PKPSubmission::STATUS_PUBLISHED . ')
+                JOIN servers j ON (ss.context_id = j.server_id)
+                LEFT JOIN author_settings ac ON (ac.author_id = aa.author_id AND ac.setting_name = \'country\')
+                ' . $sqlJoinAuthorSettings . '
+                WHERE j.enabled = 1
+                ' . (isset($serverId) ? ' AND j.server_id = ?' : '')
+                . $initialSql . '
+                GROUP BY names
+            ) as t1 ON (t1.author_id = a.author_id)';
+
         $result = $this->deprecatedDao->retrieveRange(
-            $sql = 'SELECT a.*, ug.show_title, s.locale,
+            "SELECT
+                a.*, ug.show_title, s.locale,
 				COALESCE(agl.setting_value, agpl.setting_value) AS author_given,
-				CASE WHEN agl.setting_value <> \'\' THEN afl.setting_value ELSE afpl.setting_value END AS author_family
-			FROM	authors a
-				JOIN user_groups ug ON (a.user_group_id = ug.user_group_id)
-				JOIN publications p ON (p.publication_id = a.publication_id)
-				JOIN submissions s ON (s.current_publication_id = p.publication_id)
-				LEFT JOIN author_settings agl ON (a.author_id = agl.author_id AND agl.setting_name = ? AND agl.locale = ?)
-				LEFT JOIN author_settings agpl ON (a.author_id = agpl.author_id AND agpl.setting_name = ? AND agpl.locale = s.locale)
-				LEFT JOIN author_settings afl ON (a.author_id = afl.author_id AND afl.setting_name = ? AND afl.locale = ?)
-				LEFT JOIN author_settings afpl ON (a.author_id = afpl.author_id AND afpl.setting_name = ? AND afpl.locale = s.locale)
-				JOIN (
-					SELECT
-					MIN(aa.author_id) as author_id,
-					CONCAT(
-					' . ($includeEmail ? 'aa.email, \' \', ' : '') . '
-					ac.setting_value,
-					\' \'
-					' . $sqlColumnsAuthorSettings . '
-					) as names
-					FROM authors aa
-					JOIN publications pp ON (pp.publication_id = aa.publication_id)
-					LEFT JOIN publication_settings ppss ON (ppss.publication_id = pp.publication_id)
-					JOIN submissions ss ON (ss.submission_id = pp.submission_id AND ss.current_publication_id = pp.publication_id AND ss.status = ' . PKPSubmission::STATUS_PUBLISHED . ')
-					JOIN servers j ON (ss.context_id = j.server_id)
-					LEFT JOIN author_settings ac ON (ac.author_id = aa.author_id AND ac.setting_name = \'country\')
-					' . $sqlJoinAuthorSettings . '
-					WHERE j.enabled = 1
-					' . (isset($serverId) ? ' AND j.server_id = ?' : '')
-                    . $initialSql . '
-					GROUP BY names
-				) as t1 ON (t1.author_id = a.author_id)
-				ORDER BY author_family, author_given',
+				CASE WHEN agl.setting_value <> '' THEN afl.setting_value ELSE afpl.setting_value END AS author_family
+			{$baseSql}
+            ORDER BY author_family, author_given",
             $params,
             $rangeInfo
         );
 
-        return new DAOResultFactory($result, $this, 'fromRow', [], $sql, $params, $rangeInfo);
+        return new DAOResultFactory($result, $this, 'fromRow', [], "SELECT 0 {$baseSql}", $params, $rangeInfo);
     }
 }
diff --git a/classes/doi/Repository.php b/classes/doi/Repository.php
@@ -160,8 +160,8 @@ public function isAssigned(int $doiId, string $pubObjectType): bool
             Repo::doi()::TYPE_REPRESENTATION => Repo::galley()
                 ->getCollector()
                 ->filterByDoiIds([$doiId])
-                ->getIds()
-                ->count(),
+                ->getQueryBuilder()
+                ->getCountForPagination() > 0,
             default => false,
         };
 

diff --git a/classes/plugins/PubObjectsExportPlugin.php b/classes/plugins/PubObjectsExportPlugin.php
@@ -43,23 +43,20 @@
 use PKP\submission\PKPSubmission;
 use PKP\user\User;
 
-// The statuses.
-define('EXPORT_STATUS_ANY', '');
-define('EXPORT_STATUS_NOT_DEPOSITED', 'notDeposited');
-define('EXPORT_STATUS_MARKEDREGISTERED', 'markedRegistered');
-define('EXPORT_STATUS_REGISTERED', 'registered');
-
-// The actions.
-define('EXPORT_ACTION_EXPORT', 'export');
-define('EXPORT_ACTION_MARKREGISTERED', 'markRegistered');
-define('EXPORT_ACTION_DEPOSIT', 'deposit');
-
-// Configuration errors.
-define('EXPORT_CONFIG_ERROR_SETTINGS', 0x02);
-
 abstract class PubObjectsExportPlugin extends ImportExportPlugin
 {
-    /** @var PubObjectCache */
+    // The statuses
+    public const EXPORT_STATUS_ANY = '';
+    public const EXPORT_STATUS_NOT_DEPOSITED = 'notDeposited';
+    public const EXPORT_STATUS_MARKEDREGISTERED = 'markedRegistered';
+    public const EXPORT_STATUS_REGISTERED = 'registered';
+    // The actions
+    public const EXPORT_ACTION_EXPORT = 'export';
+    public const EXPORT_ACTION_MARKREGISTERED = 'markRegistered';
+    public const EXPORT_ACTION_DEPOSIT = 'deposit';
+    // Configuration errors.
+    public const EXPORT_CONFIG_ERROR_SETTINGS = 2;    /** @var PubObjectCache */
+
     public $_cache;
 
     /**
@@ -354,10 +351,10 @@ public function getRepresentationFilter()
     public function getStatusNames()
     {
         return [
-            EXPORT_STATUS_ANY => __('plugins.importexport.common.status.any'),
-            EXPORT_STATUS_NOT_DEPOSITED => __('plugins.importexport.common.status.notDeposited'),
-            EXPORT_STATUS_MARKEDREGISTERED => __('plugins.importexport.common.status.markedRegistered'),
-            EXPORT_STATUS_REGISTERED => __('plugins.importexport.common.status.registered'),
+            PubObjectsExportPlugin::EXPORT_STATUS_ANY => __('plugins.importexport.common.status.any'),
+            PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED => __('plugins.importexport.common.status.notDeposited'),
+            PubObjectsExportPlugin::EXPORT_STATUS_MARKEDREGISTERED => __('plugins.importexport.common.status.markedRegistered'),
+            PubObjectsExportPlugin::EXPORT_STATUS_REGISTERED => __('plugins.importexport.common.status.registered'),
         ];
     }
 
@@ -458,7 +455,7 @@ public function exportXML($objects, $filter, $context, $noValidation = null, &$o
     public function markRegistered($context, $objects)
     {
         foreach ($objects as $object) {
-            $object->setData($this->getDepositStatusSettingName(), EXPORT_STATUS_MARKEDREGISTERED);
+            $object->setData($this->getDepositStatusSettingName(), PubObjectsExportPlugin::EXPORT_STATUS_MARKEDREGISTERED);
             $this->updateObject($object);
         }
     }
@@ -567,7 +564,7 @@ public function getUnregisteredPreprints($context)
             null,
             null,
             $this->getDepositStatusSettingName(),
-            EXPORT_STATUS_NOT_DEPOSITED,
+            PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED,
             null
         );
         return $preprints->toArray();
@@ -855,5 +852,18 @@ protected function _checkForExportAction(string $exportAction): bool
 }
 
 if (!PKP_STRICT_MODE) {
-    class_alias('\APP\plugins\PubObjectsExportPlugin', '\PubObjectExportsPlugin');
+    class_alias('\APP\plugins\PubObjectsExportPlugin', '\PubObjectsExportPlugin');
+
+    foreach ([
+        'EXPORT_STATUS_ANY',
+        'EXPORT_STATUS_NOT_DEPOSITED',
+        'EXPORT_STATUS_MARKEDREGISTERED',
+        'EXPORT_STATUS_REGISTERED',
+        'EXPORT_ACTION_EXPORT',
+        'EXPORT_ACTION_MARKREGISTERED',
+        'EXPORT_ACTION_DEPOSIT',
+        'EXPORT_CONFIG_ERROR_SETTINGS',
+    ] as $constantName) {
+        define($constantName, constant('\PubObjectsExportPlugin::' . $constantName));
+    }
 }
diff --git a/classes/services/queryBuilders/GalleyQueryBuilder.php b/classes/services/queryBuilders/GalleyQueryBuilder.php
@@ -60,13 +60,11 @@ public function getCount()
     {
         return $this
             ->getQuery()
-            ->select('g.galley_id')
-            ->get()
-            ->count();
+            ->getCountForPagination();
     }
 
     /**
-     * @copydoc PKP\services\queryBuilders\interfaces\EntityQueryBuilderInterface::getCount()
+     * @copydoc PKP\services\queryBuilders\interfaces\EntityQueryBuilderInterface::getIds()
      */
     public function getIds()
     {
@@ -78,7 +76,7 @@ public function getIds()
     }
 
     /**
-     * @copydoc PKP\services\queryBuilders\interfaces\EntityQueryBuilderInterface::getCount()
+     * @copydoc PKP\services\queryBuilders\interfaces\EntityQueryBuilderInterface::getQuery()
      */
     public function getQuery()
     {

diff --git a/classes/submission/DAO.php b/classes/submission/DAO.php
@@ -13,6 +13,10 @@
 
 namespace APP\submission;
 
+use APP\plugins\PubObjectsExportPlugin;
+use Illuminate\Database\Query\Builder;
+use Illuminate\Database\Query\JoinClause;
+use Illuminate\Support\Facades\DB;
 use PKP\db\DAOResultFactory;
 use PKP\db\DBResultRange;
 use PKP\identity\Identity;
@@ -32,73 +36,68 @@ public function deleteById(int $id)
     /**
      * Get all published submissions (eventually with a pubId assigned and) matching the specified settings.
      *
-     * @param ?string $pubIdType
-     * @param ?string $title
-     * @param ?string $author
-     * @param ?int $issueId
-     * @param ?string $pubIdSettingName
-     * @param ?string $pubIdSettingValue
+     * @param null|mixed $pubIdType
+     * @param null|mixed $title
+     * @param null|mixed $author
+     * @param null|mixed $issueId
+     * @param null|mixed $pubIdSettingName
+     * @param null|mixed $pubIdSettingValue
      * @param ?DBResultRange $rangeInfo
      *
      * @return DAOResultFactory<Submission>
      */
-    public function getExportable(
-        $contextId,
-        $pubIdType = null,
-        $title = null,
-        $author = null,
-        $issueId = null,
-        $pubIdSettingName = null,
-        $pubIdSettingValue = null,
-        $rangeInfo = null
-    ) {
-        $params = [];
-        if ($pubIdSettingName) {
-            $params[] = $pubIdSettingName;
-        }
-        $params[] = Submission::STATUS_PUBLISHED;
-        $params[] = $contextId;
-        if ($pubIdType) {
-            $params[] = 'pub-id::' . $pubIdType;
-        }
-        if ($title) {
-            $params[] = 'title';
-            $params[] = '%' . $title . '%';
-        }
-        if ($author) {
-            $params[] = $author;
-            $params[] = $author;
-        }
-        if ($issueId) {
-            $params[] = $issueId;
-        }
-        if ($pubIdSettingName && $pubIdSettingValue && $pubIdSettingValue != EXPORT_STATUS_NOT_DEPOSITED) {
-            $params[] = $pubIdSettingValue;
-        }
-
-        $sql = 'SELECT	s.*
-            FROM	submissions s
-                LEFT JOIN publications p ON s.current_publication_id = p.publication_id
-                LEFT JOIN publication_settings ps ON p.publication_id = ps.publication_id'
-                . ($pubIdType != null ? ' LEFT JOIN publication_settings pspidt ON (p.publication_id = pspidt.publication_id)' : '')
-                . ($title != null ? ' LEFT JOIN publication_settings pst ON (p.publication_id = pst.publication_id)' : '')
-                . ($author != null ? ' LEFT JOIN authors au ON (p.publication_id = au.publication_id)
-                        LEFT JOIN author_settings asgs ON (asgs.author_id = au.author_id AND asgs.setting_name = \'' . Identity::IDENTITY_SETTING_GIVENNAME . '\')
-                        LEFT JOIN author_settings asfs ON (asfs.author_id = au.author_id AND asfs.setting_name = \'' . Identity::IDENTITY_SETTING_FAMILYNAME . '\')
-                    ' : '')
-                . ($pubIdSettingName != null ? ' LEFT JOIN submission_settings pss ON (s.submission_id = pss.submission_id AND pss.setting_name = ?)' : '')
-            . ' WHERE	s.status = ?
-                AND s.context_id = ?'
-                . ($pubIdType != null ? ' AND pspidt.setting_name = ? AND pspidt.setting_value IS NOT NULL' : '')
-                . ($title != null ? ' AND (pst.setting_name = ? AND pst.setting_value LIKE ?)' : '')
-                . ($author != null ? ' AND (asgs.setting_value LIKE ? OR asfs.setting_value LIKE ?)' : '')
-                . (($pubIdSettingName != null && $pubIdSettingValue != null && $pubIdSettingValue == EXPORT_STATUS_NOT_DEPOSITED) ? ' AND pss.setting_value IS NULL' : '')
-                . (($pubIdSettingName != null && $pubIdSettingValue != null && $pubIdSettingValue != EXPORT_STATUS_NOT_DEPOSITED) ? ' AND pss.setting_value = ?' : '')
-                . (($pubIdSettingName != null && is_null($pubIdSettingValue)) ? ' AND (pss.setting_value IS NULL OR pss.setting_value = \'\')' : '')
-            . ' GROUP BY s.submission_id
-            ORDER BY MAX(p.date_published) DESC, s.submission_id DESC';
+    public function getExportable($contextId, $pubIdType = null, $title = null, $author = null, $issueId = null, $pubIdSettingName = null, $pubIdSettingValue = null, $rangeInfo = null)
+    {
+        $q = DB::table('submissions', 's')
+            ->leftJoin('publications AS p', 's.current_publication_id', '=', 'p.publication_id')
+            ->leftJoin('publication_settings AS ps', 'p.publication_id', '=', 'ps.publication_id')
+            ->when($pubIdType != null, fn (Builder $q) => $q->leftJoin('publication_settings AS pspidt', 'p.publication_id', '=', 'pspidt.publication_id'))
+            ->when($title != null, fn (Builder $q) => $q->leftJoin('publication_settings AS pst', 'p.publication_id', '=', 'pst.publication_id'))
+            ->when(
+                $author != null,
+                fn (Builder $q) => $q->leftJoin('authors AS au', 'p.publication_id', '=', 'au.publication_id')
+                    ->leftJoin(
+                        'author_settings AS asgs',
+                        fn (JoinClause $j) => $j->on('asgs.author_id', '=', 'au.author_id')
+                            ->where('asgs.setting_name', '=', Identity::IDENTITY_SETTING_GIVENNAME)
+                    )
+                    ->leftJoin(
+                        'author_settings AS asfs',
+                        fn (JoinClause $j) => $j->on('asfs.author_id', '=', 'au.author_id')
+                            ->where('asfs.setting_name', '=', Identity::IDENTITY_SETTING_FAMILYNAME)
+                    )
+            )
+            ->when(
+                $pubIdSettingName,
+                fn (Builder $q) => $q->leftJoin(
+                    'submission_settings AS pss',
+                    fn (JoinClause $j) => $j->on('s.submission_id', '=', 'pss.submission_id')
+                        ->where('pss.setting_name', '=', $pubIdSettingName)
+                )
+            )
+            ->where('s.status', '=', Submission::STATUS_PUBLISHED)
+            ->where('s.context_id', '=', $contextId)
+            ->when($pubIdType != null, fn (Builder $q) => $q->where('pspidt.setting_name', '=', "pub-id::{$pubIdType}")->whereNotNull('pspidt.setting_value'))
+            ->when($title != null, fn (Builder $q) => $q->where('pst.setting_name', '=', 'title')->where('pst.setting_value', 'LIKE', "%{$title}%"))
+            ->when($author != null, fn (Builder $q) => $q->where(fn (Builder $q) => $q->whereRaw("CONCAT(COALESCE(asgs.setting_value, ''), ' ', COALESCE(asfs.setting_value, ''))", 'LIKE', $author)))
+            ->when(
+                $pubIdSettingName,
+                fn (Builder $q) => $q->when(
+                    $pubIdSettingValue === null,
+                    fn (Builder $q) => $q->whereRaw("COALESCE(pss.setting_value, '') = ''"),
+                    fn (Builder $q) => $q->when(
+                        $pubIdSettingValue != PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED,
+                        fn (Builder $q) => $q->where('pss.setting_value', '=', $pubIdSettingValue),
+                        fn (Builder $q) => $q->whereNull('pss.setting_value')
+                    )
+                )
+            )
+            ->groupBy('s.submission_id')
+            ->orderByRaw('MAX(p.date_published) DESC')
+            ->orderByDesc('s.submission_id')
+            ->select('s.*');
 
-        $rows = $this->deprecatedDao->retrieveRange($sql, $params, $rangeInfo);
-        return new DAOResultFactory($rows, $this, 'fromRow', [], $sql, $params, $rangeInfo);
+        $rows = $this->deprecatedDao->retrieveRange($q, [], $rangeInfo);
+        return new DAOResultFactory($rows, $this, 'fromRow', [], $q, [], $rangeInfo);
     }
 }
diff --git a/controllers/grid/pubIds/PubIdExportRepresentationsListGridCellProvider.php b/controllers/grid/pubIds/PubIdExportRepresentationsListGridCellProvider.php
@@ -17,6 +17,7 @@
 namespace APP\controllers\grid\pubIds;
 
 use APP\facades\Repo;
+use APP\plugins\PubObjectsExportPlugin;
 use PKP\controllers\grid\DataObjectGridCellProvider;
 use PKP\controllers\grid\GridHandler;
 use PKP\linkAction\LinkAction;
@@ -134,7 +135,7 @@ public function getTemplateVarsFromRowColumn($row, $column)
                         $label = $statusNames[$status];
                     }
                 } else {
-                    $label = $statusNames[EXPORT_STATUS_NOT_DEPOSITED];
+                    $label = $statusNames[PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED];
                 }
                 return ['label' => $label];
         }

diff --git a/controllers/grid/pubIds/PubIdExportRepresentationsListGridHandler.php b/controllers/grid/pubIds/PubIdExportRepresentationsListGridHandler.php
@@ -285,7 +285,7 @@ protected function getFilterValues($filter)
         } else {
             $column = null;
         }
-        if (isset($filter['statusId']) && $filter['statusId'] != EXPORT_STATUS_ANY) {
+        if (isset($filter['statusId']) && $filter['statusId'] != PubObjectsExportPlugin::EXPORT_STATUS_ANY) {
             $statusId = $filter['statusId'];
         } else {
             $statusId = null;

diff --git a/controllers/grid/submissions/ExportPublishedSubmissionsListGridCellProvider.php b/controllers/grid/submissions/ExportPublishedSubmissionsListGridCellProvider.php
@@ -17,6 +17,7 @@
 namespace APP\controllers\grid\submissions;
 
 use APP\facades\Repo;
+use APP\plugins\PubObjectsExportPlugin;
 use PKP\controllers\grid\DataObjectGridCellProvider;
 use PKP\controllers\grid\GridHandler;
 use PKP\linkAction\LinkAction;
@@ -119,7 +120,7 @@ public function getTemplateVarsFromRowColumn($row, $column)
                         $label = $statusNames[$status];
                     }
                 } else {
-                    $label = $statusNames[EXPORT_STATUS_NOT_DEPOSITED];
+                    $label = $statusNames[PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED];
                 }
                 return ['label' => $label];
         }