Merge pull request #3870 from jonasraoni/feature/main/8700-improve-sl…

…ow-query Feature/main/8700 improve slow query
pkp · May 30, 2024 · 04237af · 04237af
2 parents 8659fcd + a58a921
commit 04237af
Show file tree

Hide file tree

Showing 26 changed files with 357 additions and 407 deletions.
diff --git a/api/v1/issues/IssueController.php b/api/v1/issues/IssueController.php
@@ -212,7 +212,7 @@ public function getMany(Request $illuminateRequest): JsonResponse
 
         return response()->json([
             'items' => Repo::issue()->getSchemaMap()->summarizeMany($issues, $context)->values(),
-            'itemsMax' => $collector->limit(null)->offset(null)->getCount(),
+            'itemsMax' => $collector->getCount(),
         ], Response::HTTP_OK);
     }
 

diff --git a/classes/doi/Repository.php b/classes/doi/Repository.php
@@ -306,8 +306,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/issue/DAO.php b/classes/issue/DAO.php
@@ -16,6 +16,9 @@
 namespace APP\issue;
 
 use APP\facades\Repo;
+use APP\plugins\PubObjectsExportPlugin;
+use Illuminate\Database\Query\Builder;
+use Illuminate\Database\Query\JoinClause;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\LazyCollection;
@@ -133,7 +136,7 @@ public function getCount(Collector $query): int
     {
         return $query
             ->getQueryBuilder()
-            ->count();
+            ->getCountForPagination();
     }
 
     /**
@@ -266,10 +269,9 @@ public function resequenceCustomIssueOrders(int $contextId)
      */
     public function customIssueOrderingExists(int $contextId): bool
     {
-        $resultCount = DB::table('custom_issue_orders', 'o')
+        return DB::table('custom_issue_orders', 'o')
             ->where('o.journal_id', '=', $contextId)
-            ->count();
-        return $resultCount != 0;
+            ->getCountForPagination() > 0;
     }
 
     /**
@@ -432,38 +434,30 @@ public function deleteAllPubIds($contextId, $pubIdType)
      */
     public function getExportable($contextId, $pubIdType = null, $pubIdSettingName = null, $pubIdSettingValue = null, $rangeInfo = null)
     {
-        $params = [];
-        if ($pubIdSettingName) {
-            $params[] = $pubIdSettingName;
-        }
-        $params[] = (int) $contextId;
-        if ($pubIdType) {
-            $params[] = 'pub-id::' . $pubIdType;
-        }
-
-        import('classes.plugins.PubObjectsExportPlugin'); // Constants
-        if ($pubIdSettingName && $pubIdSettingValue && $pubIdSettingValue != EXPORT_STATUS_NOT_DEPOSITED) {
-            $params[] = $pubIdSettingValue;
-        }
-
-        $result = $this->deprecatedDao->retrieveRange(
-            $sql = 'SELECT i.*
-			FROM issues i
-				LEFT JOIN custom_issue_orders o ON (o.issue_id = i.issue_id)
-				' . ($pubIdType != null ? ' LEFT JOIN issue_settings ist ON (i.issue_id = ist.issue_id)' : '')
-                . ($pubIdSettingName != null ? ' LEFT JOIN issue_settings iss ON (i.issue_id = iss.issue_id AND iss.setting_name = ?)' : '') . '
-			WHERE
-				i.published = 1  AND i.journal_id = ?
-				' . ($pubIdType != null ? ' AND ist.setting_name = ? AND ist.setting_value IS NOT NULL' : '')
-                . (($pubIdSettingName != null && $pubIdSettingValue != null && $pubIdSettingValue == EXPORT_STATUS_NOT_DEPOSITED) ? ' AND iss.setting_value IS NULL' : '')
-                . (($pubIdSettingName != null && $pubIdSettingValue != null && $pubIdSettingValue != EXPORT_STATUS_NOT_DEPOSITED) ? ' AND iss.setting_value = ?' : '')
-                . (($pubIdSettingName != null && is_null($pubIdSettingValue)) ? ' AND (iss.setting_value IS NULL OR iss.setting_value = \'\')' : '')
-                . ' ORDER BY i.date_published DESC',
-            $params,
-            $rangeInfo
-        );
-
-        return new DAOResultFactory($result, $this, 'fromRow', [], $sql, $params, $rangeInfo);
+        $q = DB::table('issues', 'i')
+            ->leftJoin('custom_issue_orders AS o', 'o.issue_id', '=', 'i.issue_id')
+            ->when($pubIdType != null, fn (Builder $q) => $q->leftJoin('issue_settings AS ist', 'i.issue_id', '=', 'ist.issue_id'))
+            ->when($pubIdSettingName, fn (Builder $q) => $q->leftJoin('issue_settings AS iss', fn (JoinClause $j) => $j->on('i.issue_id', '=', 'iss.issue_id')->where('iss.setting_name', '=', $pubIdSettingName)))
+            ->where('i.published', '=', 1)
+            ->where('i.journal_id', '=', $contextId)
+            ->when($pubIdType != null, fn (Builder $q) => $q->where('ist.setting_name', '=', "pub-id::{$pubIdType}")->whereNotNull('ist.setting_value'))
+            ->when(
+                $pubIdSettingName,
+                fn (Builder $q) => $q->when(
+                    $pubIdSettingValue === null,
+                    fn (Builder $q) => $q->whereRaw("COALESCE(iss.setting_value, '') = ''"),
+                    fn (Builder $q) => $q->when(
+                        $pubIdSettingValue != PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED,
+                        fn (Builder $q) => $q->where('iss.setting_value', '=', $pubIdSettingValue),
+                        fn (Builder $q) => $q->whereNull('iss.setting_value')
+                    )
+                )
+            )
+            ->orderByDesc('i.date_published')
+            ->select('i.*');
+
+        $result = $this->deprecatedDao->retrieveRange($q, [], $rangeInfo);
+        return new DAOResultFactory($result, $this, 'fromRow', [], $q, [], $rangeInfo);
     }
 
     /**

diff --git a/classes/plugins/PubObjectsExportPlugin.php b/classes/plugins/PubObjectsExportPlugin.php
@@ -43,25 +43,19 @@
 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
 {
+    // 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;
@@ -169,7 +163,7 @@ public function display($args, $request)
                     }
                     $pluginSetting = $this->getSetting($context->getId(), $fieldName);
                     if (empty($pluginSetting)) {
-                        $configurationErrors[] = EXPORT_CONFIG_ERROR_SETTINGS;
+                        $configurationErrors[] = PubObjectsExportPlugin::EXPORT_CONFIG_ERROR_SETTINGS;
                         break;
                     }
                 }
@@ -252,7 +246,7 @@ public function executeExportAction($request, $objects, $filter, $tab, $objectsF
     {
         $context = $request->getContext();
         $path = ['plugin', $this->getName()];
-        if ($this->_checkForExportAction(EXPORT_ACTION_EXPORT)) {
+        if ($this->_checkForExportAction(PubObjectsExportPlugin::EXPORT_ACTION_EXPORT)) {
             assert($filter != null);
 
             $onlyValidateExport = ($request->getUserVar('onlyValidateExport')) ? true : false;
@@ -288,7 +282,7 @@ public function executeExportAction($request, $objects, $filter, $tab, $objectsF
                 $fileManager->downloadByPath($exportFileName);
                 $fileManager->deleteByPath($exportFileName);
             }
-        } elseif ($this->_checkForExportAction(EXPORT_ACTION_DEPOSIT)) {
+        } elseif ($this->_checkForExportAction(PubObjectsExportPlugin::EXPORT_ACTION_DEPOSIT)) {
             assert($filter != null);
             // Get the XML
             $exportXml = $this->exportXML($objects, $filter, $context, $noValidation);
@@ -325,7 +319,7 @@ public function executeExportAction($request, $objects, $filter, $tab, $objectsF
                 // redirect back to the right tab
                 $request->redirect(null, null, null, $path, null, $tab);
             }
-        } elseif ($this->_checkForExportAction(EXPORT_ACTION_MARKREGISTERED)) {
+        } elseif ($this->_checkForExportAction(PubObjectsExportPlugin::EXPORT_ACTION_MARKREGISTERED)) {
             $this->markRegistered($context, $objects);
             if ($shouldRedirect) {
                 // redirect back to the right tab
@@ -409,10 +403,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'),
         ];
     }
 
@@ -438,9 +432,9 @@ public function getStatusActions($pubObject)
      */
     public function getExportActions($context)
     {
-        $actions = [EXPORT_ACTION_EXPORT, EXPORT_ACTION_MARKREGISTERED];
+        $actions = [PubObjectsExportPlugin::EXPORT_ACTION_EXPORT, PubObjectsExportPlugin::EXPORT_ACTION_MARKREGISTERED];
         if ($this->getSetting($context->getId(), 'username') && $this->getSetting($context->getId(), 'password')) {
-            array_unshift($actions, EXPORT_ACTION_DEPOSIT);
+            array_unshift($actions, PubObjectsExportPlugin::EXPORT_ACTION_DEPOSIT);
         }
         return $actions;
     }
@@ -453,9 +447,9 @@ public function getExportActions($context)
     public function getExportActionNames()
     {
         return [
-            EXPORT_ACTION_DEPOSIT => __('plugins.importexport.common.action.register'),
-            EXPORT_ACTION_EXPORT => __('plugins.importexport.common.action.export'),
-            EXPORT_ACTION_MARKREGISTERED => __('plugins.importexport.common.action.markRegistered'),
+            PubObjectsExportPlugin::EXPORT_ACTION_DEPOSIT => __('plugins.importexport.common.action.register'),
+            PubObjectsExportPlugin::EXPORT_ACTION_EXPORT => __('plugins.importexport.common.action.export'),
+            PubObjectsExportPlugin::EXPORT_ACTION_MARKREGISTERED => __('plugins.importexport.common.action.markRegistered'),
         ];
     }
 
@@ -513,7 +507,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 +561,7 @@ public function getAdditionalFieldNames($hookName, $dao, &$additionalFields)
      */
     public function addToSchema($hookName, $params)
     {
-        $schema = & $params[0];
+        $schema = &$params[0];
         foreach ($this->_getObjectAdditionalSettings() as $fieldName) {
             $schema->properties->{$fieldName} = (object) [
                 'type' => 'string',
@@ -594,7 +588,7 @@ protected function _getObjectAdditionalSettings()
      */
     public function callbackParseCronTab($hookName, $args)
     {
-        $taskFilesPath = & $args[0];
+        $taskFilesPath = &$args[0];
 
         $scheduledTasksPath = "{$this->getPluginPath()}/scheduledTasks.xml";
 
@@ -623,7 +617,7 @@ public function getUnregisteredArticles($context)
             null,
             null,
             $this->getDepositStatusSettingName(),
-            EXPORT_STATUS_NOT_DEPOSITED,
+            PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED,
             null
         );
         return $articles->toArray();
@@ -932,4 +926,17 @@ protected function _checkForExportAction($exportAction)
 
 if (!PKP_STRICT_MODE) {
     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/StatsIssueService.php b/classes/services/StatsIssueService.php
@@ -59,7 +59,7 @@ public function getCount(array $args): int
 
         Hook::call('StatsIssue::getCount::queryBuilder', [&$metricsQB, $args]);
 
-        return $metricsQB->getIssueIds()->get()->count();
+        return $metricsQB->getIssueIds()->getCountForPagination();
     }
 
     /**

diff --git a/classes/services/queryBuilders/GalleyQueryBuilder.php b/classes/services/queryBuilders/GalleyQueryBuilder.php
@@ -56,13 +56,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()
     {
@@ -74,7 +72,7 @@ public function getIds()
     }
 
     /**
-     * @copydoc PKP\services\queryBuilders\interfaces\EntityQueryBuilderInterface::getCount()
+     * @copydoc PKP\services\queryBuilders\interfaces\EntityQueryBuilderInterface::getQuery()
      *
      * @hook Galley::getMany::queryObject [[&$q, $this]]
      */

diff --git a/classes/services/queryBuilders/StatsIssueQueryBuilder.php b/classes/services/queryBuilders/StatsIssueQueryBuilder.php
@@ -69,7 +69,7 @@ public function getIssueIds(): Builder
     {
         return $this->_getObject()
             ->select([StatisticsHelper::STATISTICS_DIMENSION_ISSUE_ID])
-            ->distinct();
+            ->groupBy(StatisticsHelper::STATISTICS_DIMENSION_ISSUE_ID);
     }
 
     /**