From 3439800676378547f89d162a7fab01986b70746a Mon Sep 17 00:00:00 2001 From: Jonas Raoni Soares da Silva Date: Thu, 6 Apr 2023 15:09:13 +0300 Subject: [PATCH 1/9] pkp/pkp#8700 Optimized DAOResultFactory instances --- classes/author/DAO.php | 69 ++++++++++--------- classes/issue/DAO.php | 28 ++++---- classes/submission/DAO.php | 54 ++++++++------- .../IndividualSubscriptionDAO.php | 25 ++++--- .../InstitutionalSubscriptionDAO.php | 32 +++++---- classes/subscription/SubscriptionTypeDAO.php | 7 +- 6 files changed, 117 insertions(+), 98 deletions(-) diff --git a/classes/author/DAO.php b/classes/author/DAO.php index d6a23a1115e..d6c7a0fa8c9 100644 --- a/classes/author/DAO.php +++ b/classes/author/DAO.php @@ -111,45 +111,48 @@ public function getAuthorsAlphabetizedByJournal($journalId = null, $initial = nu $initialSql .= ')'; } - $result = $this->deprecatedDao->retrieveRange( - $sql = '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 + $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 . ' + ' . ($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 journals j ON (ss.context_id = j.journal_id) - JOIN issues i ON (ppss.setting_name = ? AND ppss.setting_value = CAST(i.issue_id AS CHAR(20)) AND i.published = 1) - LEFT JOIN author_settings ac ON (ac.author_id = aa.author_id AND ac.setting_name = \'country\') - ' . $sqlJoinAuthorSettings . ' - WHERE j.enabled = 1 - ' . (isset($journalId) ? ' AND j.journal_id = ?' : '') - . $initialSql . ' - GROUP BY names - ) as t1 ON (t1.author_id = a.author_id) - ORDER BY author_family, author_given', + 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 journals j ON (ss.context_id = j.journal_id) + JOIN issues i ON (ppss.setting_name = ? AND ppss.setting_value = CAST(i.issue_id AS CHAR(20)) AND i.published = 1) + LEFT JOIN author_settings ac ON (ac.author_id = aa.author_id AND ac.setting_name = 'country') + {$sqlJoinAuthorSettings} + WHERE j.enabled = 1 + " . (isset($journalId) ? ' AND j.journal_id = ?' : '') . " + {$initialSql} + GROUP BY names + ) as t1 ON (t1.author_id = a.author_id)"; + + $result = $this->deprecatedDao->retrieveRange( + "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 + {$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/issue/DAO.php b/classes/issue/DAO.php index 7aec2af027e..edc94351b0c 100644 --- a/classes/issue/DAO.php +++ b/classes/issue/DAO.php @@ -446,24 +446,26 @@ public function getExportable($contextId, $pubIdType = null, $pubIdSettingName = $params[] = $pubIdSettingValue; } + $baseSql = ' + 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 = \'\')' : ''); + $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', + "SELECT i.* + {$baseSql} + ORDER BY i.date_published DESC", $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/submission/DAO.php b/classes/submission/DAO.php index 941ac8f6c29..7127bb5e5fb 100644 --- a/classes/submission/DAO.php +++ b/classes/submission/DAO.php @@ -76,31 +76,35 @@ public function getExportable( $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' - . ($issueId ? ' LEFT JOIN publication_settings psi ON p.publication_id = psi.publication_id AND psi.setting_name = \'issueId\' AND psi.locale = \'\'' : '') - . ($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 ?)' : '') - . ($issueId != null ? ' AND psi.setting_value = ?' : '') - . (($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'; + $baseSql = ' + 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' + . ($issueId ? ' LEFT JOIN publication_settings psi ON p.publication_id = psi.publication_id AND psi.setting_name = \'issueId\' AND psi.locale = \'\'' : '') + . ($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 ?)' : '') + . ($issueId != null ? ' AND psi.setting_value = ?' : '') + . (($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 = \'\')' : ''); - $rows = $this->deprecatedDao->retrieveRange($sql, $params, $rangeInfo); - return new DAOResultFactory($rows, $this, 'fromRow', [], $sql, $params, $rangeInfo); + $rows = $this->deprecatedDao->retrieveRange( + "SELECT s.* {$baseSql} + GROUP BY s.submission_id + ORDER BY MAX(p.date_published) DESC, s.submission_id DESC", + $params, + $rangeInfo + ); + return new DAOResultFactory($rows, $this, 'fromRow', [], "SELECT 0 {$baseSql}", $params, $rangeInfo); } } diff --git a/classes/subscription/IndividualSubscriptionDAO.php b/classes/subscription/IndividualSubscriptionDAO.php index 319463fd062..2d3d84a3603 100644 --- a/classes/subscription/IndividualSubscriptionDAO.php +++ b/classes/subscription/IndividualSubscriptionDAO.php @@ -392,21 +392,24 @@ public function getAll($rangeInfo = null) public function getByJournalId($journalId, $status = null, $searchField = null, $searchMatch = null, $search = null, $dateField = null, $dateFrom = null, $dateTo = null, $rangeInfo = null) { $params = array_merge($this->getFetchParameters(), [(int) $journalId]); + $baseSql = " + FROM subscriptions s + JOIN subscription_types st ON (s.type_id = st.type_id) + JOIN users u ON (s.user_id = u.user_id) + {$this->getFetchJoins()} + WHERE + st.institutional = 0 + AND s.journal_id = ? + " . parent::_generateSearchSQL($status, $searchField, $searchMatch, $search, $dateField, $dateFrom, $dateTo, $params); + $result = $this->retrieveRange( - $sql = 'SELECT s.*, ' . $this->getFetchColumns() . ' - FROM subscriptions s - JOIN subscription_types st ON (s.type_id = st.type_id) - JOIN users u ON (s.user_id = u.user_id) - ' . $this->getFetchJoins() . ' - WHERE - st.institutional = 0 - AND s.journal_id = ? ' . - parent::_generateSearchSQL($status, $searchField, $searchMatch, $search, $dateField, $dateFrom, $dateTo, $params) . ' ' . - 'ORDER BY u.user_id, s.subscription_id', + "SELECT s.*, {$this->getFetchColumns()} + {$baseSql} + ORDER BY u.user_id, s.subscription_id", $params, $rangeInfo ); - return new DAOResultFactory($result, $this, '_fromRow', [], $sql, $params, $rangeInfo); // Counted in subscription grid paging + return new DAOResultFactory($result, $this, '_fromRow', [], "SELECT 0 {$baseSql}", $params, $rangeInfo); // Counted in subscription grid paging } /** diff --git a/classes/subscription/InstitutionalSubscriptionDAO.php b/classes/subscription/InstitutionalSubscriptionDAO.php index 94139cd8436..3773c7925e9 100644 --- a/classes/subscription/InstitutionalSubscriptionDAO.php +++ b/classes/subscription/InstitutionalSubscriptionDAO.php @@ -437,25 +437,29 @@ public function getByJournalId($journalId, $status = null, $searchField = null, } } + $baseSql = " + FROM subscriptions s + JOIN subscription_types st ON (s.type_id = st.type_id) + JOIN users u ON (s.user_id = u.user_id) + JOIN institutional_subscriptions iss ON (s.subscription_id = iss.subscription_id) + {$institutionFetch} + {$ipRangeFetch} + {$this->getInstitutionNameFetchJoins()} + {$this->getFetchJoins()} + WHERE st.institutional = 1 AND s.journal_id = ? + {$searchSql}"; + $result = $this->retrieveRange( - $sql = 'SELECT DISTINCT s.*, iss.institution_id, iss.mailing_address, iss.domain, - ' . $this->getInstitutionNameFetchColumns() . ', - ' . $this->getFetchColumns() . ' - FROM subscriptions s - JOIN subscription_types st ON (s.type_id = st.type_id) - JOIN users u ON (s.user_id = u.user_id) - JOIN institutional_subscriptions iss ON (s.subscription_id = iss.subscription_id) - ' . $institutionFetch . ' - ' . $ipRangeFetch . ' - ' . $this->getInstitutionNameFetchJoins() . ' - ' . $this->getFetchJoins() . ' - WHERE st.institutional = 1 AND s.journal_id = ? - ' . $searchSql . ' ORDER BY institution_name ASC, s.subscription_id', + "SELECT DISTINCT s.*, iss.institution_id, iss.mailing_address, iss.domain, + {$this->getInstitutionNameFetchColumns()}, + {$this->getFetchColumns()} + {$baseSql} + ORDER BY institution_name ASC, s.subscription_id", $params, $rangeInfo ); - return new DAOResultFactory($result, $this, '_fromRow', [], $sql, $params, $rangeInfo); // Counted in subscription grid paging + return new DAOResultFactory($result, $this, '_fromRow', [], "SELECT 0 {$baseSql}", $params, $rangeInfo); // Counted in subscription grid paging } /** diff --git a/classes/subscription/SubscriptionTypeDAO.php b/classes/subscription/SubscriptionTypeDAO.php index 6d092e1b85d..6dd28be2fe6 100644 --- a/classes/subscription/SubscriptionTypeDAO.php +++ b/classes/subscription/SubscriptionTypeDAO.php @@ -320,12 +320,15 @@ public function deleteByJournal($journalId) */ public function getByJournalId($journalId, $rangeInfo = null) { + $baseSql = ' + FROM subscription_types + WHERE journal_id = ?'; $result = $this->retrieveRange( - $sql = 'SELECT * FROM subscription_types WHERE journal_id = ? ORDER BY seq', + "SELECT * {$baseSql} ORDER BY seq", $params = [(int) $journalId], $rangeInfo ); - return new DAOResultFactory($result, $this, '_fromRow', [], $sql, $params, $rangeInfo); // Counted in subscription type grid paging + return new DAOResultFactory($result, $this, '_fromRow', [], "SELECT 0 {$baseSql}", $params, $rangeInfo); // Counted in subscription type grid paging } /** From 1a43ad65fd1bc214e4e246240dbc677af7800ff3 Mon Sep 17 00:00:00 2001 From: Jonas Raoni Soares da Silva Date: Thu, 6 Apr 2023 17:24:24 +0300 Subject: [PATCH 2/9] pkp/pkp-lib#8700 Updated code to make use of the new safeCount() macro --- classes/doi/Repository.php | 4 ++-- classes/issue/DAO.php | 4 ++-- classes/services/StatsIssueService.php | 2 +- classes/services/queryBuilders/GalleyQueryBuilder.php | 4 +--- classes/services/queryBuilders/StatsIssueQueryBuilder.php | 2 +- 5 files changed, 7 insertions(+), 9 deletions(-) diff --git a/classes/doi/Repository.php b/classes/doi/Repository.php index 3085fbab63b..430e8072c42 100644 --- 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() + ->safeCount() > 0, default => false, }; diff --git a/classes/issue/DAO.php b/classes/issue/DAO.php index edc94351b0c..43b7cdff8fd 100644 --- a/classes/issue/DAO.php +++ b/classes/issue/DAO.php @@ -133,7 +133,7 @@ public function getCount(Collector $query): int { return $query ->getQueryBuilder() - ->count(); + ->safeCount(); } /** @@ -268,7 +268,7 @@ public function customIssueOrderingExists(int $contextId): bool { $resultCount = DB::table('custom_issue_orders', 'o') ->where('o.journal_id', '=', $contextId) - ->count(); + ->safeCount(); return $resultCount != 0; } diff --git a/classes/services/StatsIssueService.php b/classes/services/StatsIssueService.php index c85335b3e87..8bdf3932bf8 100644 --- a/classes/services/StatsIssueService.php +++ b/classes/services/StatsIssueService.php @@ -57,7 +57,7 @@ public function getCount(array $args): int Hook::call('StatsIssue::getCount::queryBuilder', [&$metricsQB, $args]); - return $metricsQB->getIssueIds()->get()->count(); + return $metricsQB->getIssueIds()->safeCount(); } /** diff --git a/classes/services/queryBuilders/GalleyQueryBuilder.php b/classes/services/queryBuilders/GalleyQueryBuilder.php index 51f8ea9a670..5e91f4097bf 100644 --- a/classes/services/queryBuilders/GalleyQueryBuilder.php +++ b/classes/services/queryBuilders/GalleyQueryBuilder.php @@ -56,9 +56,7 @@ public function getCount() { return $this ->getQuery() - ->select('g.galley_id') - ->get() - ->count(); + ->safeCount(); } /** diff --git a/classes/services/queryBuilders/StatsIssueQueryBuilder.php b/classes/services/queryBuilders/StatsIssueQueryBuilder.php index d6068ebc324..bd78dc1d376 100644 --- 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); } /** From c1a94230303b1ccd61c756be5d0af0f12f8c83b4 Mon Sep 17 00:00:00 2001 From: Jonas Raoni Soares da Silva Date: Thu, 23 May 2024 18:34:27 +0300 Subject: [PATCH 3/9] pkp/pkp-lib#8700 Dropped safeCount() workaround in favor of getCountForPagination() --- classes/doi/Repository.php | 2 +- classes/issue/DAO.php | 4 ++-- classes/services/StatsIssueService.php | 2 +- classes/services/queryBuilders/GalleyQueryBuilder.php | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/classes/doi/Repository.php b/classes/doi/Repository.php index 430e8072c42..d4147862e70 100644 --- a/classes/doi/Repository.php +++ b/classes/doi/Repository.php @@ -307,7 +307,7 @@ public function isAssigned(int $doiId, string $pubObjectType): bool ->getCollector() ->filterByDoiIds([$doiId]) ->getQueryBuilder() - ->safeCount() > 0, + ->getCountForPagination() > 0, default => false, }; diff --git a/classes/issue/DAO.php b/classes/issue/DAO.php index 43b7cdff8fd..d9bc99631c4 100644 --- a/classes/issue/DAO.php +++ b/classes/issue/DAO.php @@ -133,7 +133,7 @@ public function getCount(Collector $query): int { return $query ->getQueryBuilder() - ->safeCount(); + ->getCountForPagination(); } /** @@ -268,7 +268,7 @@ public function customIssueOrderingExists(int $contextId): bool { $resultCount = DB::table('custom_issue_orders', 'o') ->where('o.journal_id', '=', $contextId) - ->safeCount(); + ->getCountForPagination(); return $resultCount != 0; } diff --git a/classes/services/StatsIssueService.php b/classes/services/StatsIssueService.php index 8bdf3932bf8..825e5746972 100644 --- a/classes/services/StatsIssueService.php +++ b/classes/services/StatsIssueService.php @@ -57,7 +57,7 @@ public function getCount(array $args): int Hook::call('StatsIssue::getCount::queryBuilder', [&$metricsQB, $args]); - return $metricsQB->getIssueIds()->safeCount(); + return $metricsQB->getIssueIds()->getCountForPagination(); } /** diff --git a/classes/services/queryBuilders/GalleyQueryBuilder.php b/classes/services/queryBuilders/GalleyQueryBuilder.php index 5e91f4097bf..086b510cfad 100644 --- a/classes/services/queryBuilders/GalleyQueryBuilder.php +++ b/classes/services/queryBuilders/GalleyQueryBuilder.php @@ -56,11 +56,11 @@ public function getCount() { return $this ->getQuery() - ->safeCount(); + ->getCountForPagination(); } /** - * @copydoc PKP\services\queryBuilders\interfaces\EntityQueryBuilderInterface::getCount() + * @copydoc PKP\services\queryBuilders\interfaces\EntityQueryBuilderInterface::getIds() */ public function getIds() { @@ -72,7 +72,7 @@ public function getIds() } /** - * @copydoc PKP\services\queryBuilders\interfaces\EntityQueryBuilderInterface::getCount() + * @copydoc PKP\services\queryBuilders\interfaces\EntityQueryBuilderInterface::getQuery() */ public function getQuery() { From fb16d8a68621f7f0329e1ecf8661f196459488e0 Mon Sep 17 00:00:00 2001 From: Jonas Raoni Soares da Silva Date: Thu, 23 May 2024 18:34:55 +0300 Subject: [PATCH 4/9] pkp/pkp-lib#8700 Removed not needed optimizations --- api/v1/issues/IssueHandler.php | 2 +- .../grid/users/subscriberSelect/SubscriberSelectGridHandler.php | 2 +- pages/issue/IssueHandler.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/v1/issues/IssueHandler.php b/api/v1/issues/IssueHandler.php index 54bd4be9b14..096bfd85970 100644 --- a/api/v1/issues/IssueHandler.php +++ b/api/v1/issues/IssueHandler.php @@ -204,7 +204,7 @@ public function getMany($slimRequest, $response, $args) return $response->withJson([ 'items' => Repo::issue()->getSchemaMap()->summarizeMany($issues, $context)->values(), - 'itemsMax' => $collector->limit(null)->offset(null)->getCount(), + 'itemsMax' => $collector->getCount(), ], 200); } diff --git a/controllers/grid/users/subscriberSelect/SubscriberSelectGridHandler.php b/controllers/grid/users/subscriberSelect/SubscriberSelectGridHandler.php index e6394257fd1..bf7d57978df 100644 --- a/controllers/grid/users/subscriberSelect/SubscriberSelectGridHandler.php +++ b/controllers/grid/users/subscriberSelect/SubscriberSelectGridHandler.php @@ -137,7 +137,7 @@ protected function loadData($request, $filter) $users = $userCollector->getMany(); - $totalCount = $userCollector->limit(null)->offset(null)->getCount(); + $totalCount = $userCollector->getCount(); return new \PKP\core\VirtualArrayIterator(iterator_to_array($users, true), $totalCount, $rangeInfo->getPage(), $rangeInfo->getCount()); } diff --git a/pages/issue/IssueHandler.php b/pages/issue/IssueHandler.php index 03a2ba85f01..c24602f6c0a 100644 --- a/pages/issue/IssueHandler.php +++ b/pages/issue/IssueHandler.php @@ -165,7 +165,7 @@ public function archive($args, $request) ->filterByPublished(true); $issues = $collector->getMany()->toArray(); - $total = $collector->limit(null)->offset(null)->getCount(); + $total = $collector->getCount(); $showingStart = $offset + 1; $showingEnd = min($offset + $count, $offset + count($issues)); From dfac2e915363575a173c2714e35286338c397c6b Mon Sep 17 00:00:00 2001 From: Jonas Raoni Soares da Silva Date: Thu, 6 Apr 2023 17:25:03 +0300 Subject: [PATCH 5/9] pkp/pkp-lib#8700 Submodule update ##jonasraoni/feature/stable-3_4_0/8700-improve-slow-query## --- lib/pkp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pkp b/lib/pkp index 1f78b8f3530..c1bcdc8d515 160000 --- a/lib/pkp +++ b/lib/pkp @@ -1 +1 @@ -Subproject commit 1f78b8f3530869be44ceb4aec77239e375548b18 +Subproject commit c1bcdc8d515e7b711c9a0e16c4a1a51d3eaf1414 From d6a16bf19827a7fb2d03c635f62d19b7ba387a25 Mon Sep 17 00:00:00 2001 From: Jonas Raoni Soares da Silva Date: Tue, 28 May 2024 22:24:28 +0300 Subject: [PATCH 6/9] pkp/pkp-lib#8700 Namespaced PubObjectsExportPlugin constants --- classes/issue/DAO.php | 8 +-- classes/plugins/PubObjectsExportPlugin.php | 65 ++++++++++--------- classes/submission/DAO.php | 7 +- .../PubIdExportIssuesListGridCellProvider.php | 3 +- .../PubIdExportIssuesListGridHandler.php | 2 +- ...ortRepresentationsListGridCellProvider.php | 3 +- ...IdExportRepresentationsListGridHandler.php | 2 +- ...blishedSubmissionsListGridCellProvider.php | 3 +- ...ortPublishedSubmissionsListGridHandler.php | 3 +- .../importexport/doaj/DOAJExportPlugin.php | 8 +-- plugins/importexport/doaj/templates/index.tpl | 2 +- 11 files changed, 59 insertions(+), 47 deletions(-) diff --git a/classes/issue/DAO.php b/classes/issue/DAO.php index d9bc99631c4..6d6211bccba 100644 --- a/classes/issue/DAO.php +++ b/classes/issue/DAO.php @@ -16,6 +16,7 @@ namespace APP\issue; use APP\facades\Repo; +use APP\plugins\PubObjectsExportPlugin; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; use Illuminate\Support\LazyCollection; @@ -441,8 +442,7 @@ public function getExportable($contextId, $pubIdType = null, $pubIdSettingName = $params[] = 'pub-id::' . $pubIdType; } - import('classes.plugins.PubObjectsExportPlugin'); // Constants - if ($pubIdSettingName && $pubIdSettingValue && $pubIdSettingValue != EXPORT_STATUS_NOT_DEPOSITED) { + if ($pubIdSettingName && $pubIdSettingValue && $pubIdSettingValue != PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED) { $params[] = $pubIdSettingValue; } @@ -453,8 +453,8 @@ public function getExportable($contextId, $pubIdType = null, $pubIdSettingName = . ($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 && $pubIdSettingValue != null && $pubIdSettingValue == PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED) ? ' AND iss.setting_value IS NULL' : '') + . (($pubIdSettingName != null && $pubIdSettingValue != null && $pubIdSettingValue != PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED) ? ' AND iss.setting_value = ?' : '') . (($pubIdSettingName != null && is_null($pubIdSettingValue)) ? ' AND (iss.setting_value IS NULL OR iss.setting_value = \'\')' : ''); $result = $this->deprecatedDao->retrieveRange( diff --git a/classes/plugins/PubObjectsExportPlugin.php b/classes/plugins/PubObjectsExportPlugin.php index 08dcda82749..7a6a40c6f93 100644 --- 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); } } @@ -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/submission/DAO.php b/classes/submission/DAO.php index 7127bb5e5fb..0381628ab4b 100644 --- a/classes/submission/DAO.php +++ b/classes/submission/DAO.php @@ -13,6 +13,7 @@ namespace APP\submission; +use APP\plugins\PubObjectsExportPlugin; use PKP\db\DAOResultFactory; use PKP\db\DBResultRange; use PKP\identity\Identity; @@ -72,7 +73,7 @@ public function getExportable( if ($issueId) { $params[] = $issueId; } - if ($pubIdSettingName && $pubIdSettingValue && $pubIdSettingValue != EXPORT_STATUS_NOT_DEPOSITED) { + if ($pubIdSettingName && $pubIdSettingValue && $pubIdSettingValue != PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED) { $params[] = $pubIdSettingValue; } @@ -94,8 +95,8 @@ public function getExportable( . ($title != null ? ' AND (pst.setting_name = ? AND pst.setting_value LIKE ?)' : '') . ($author != null ? ' AND (asgs.setting_value LIKE ? OR asfs.setting_value LIKE ?)' : '') . ($issueId != null ? ' AND psi.setting_value = ?' : '') - . (($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 && $pubIdSettingValue != null && $pubIdSettingValue == PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED) ? ' AND pss.setting_value IS NULL' : '') + . (($pubIdSettingName != null && $pubIdSettingValue != null && $pubIdSettingValue != PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED) ? ' AND pss.setting_value = ?' : '') . (($pubIdSettingName != null && is_null($pubIdSettingValue)) ? ' AND (pss.setting_value IS NULL OR pss.setting_value = \'\')' : ''); $rows = $this->deprecatedDao->retrieveRange( diff --git a/controllers/grid/pubIds/PubIdExportIssuesListGridCellProvider.php b/controllers/grid/pubIds/PubIdExportIssuesListGridCellProvider.php index 46dc0036d35..da2303cbe8a 100644 --- a/controllers/grid/pubIds/PubIdExportIssuesListGridCellProvider.php +++ b/controllers/grid/pubIds/PubIdExportIssuesListGridCellProvider.php @@ -17,6 +17,7 @@ namespace APP\controllers\grid\pubIds; use APP\core\Application; +use APP\plugins\PubObjectsExportPlugin; use PKP\controllers\grid\DataObjectGridCellProvider; use PKP\controllers\grid\GridHandler; use PKP\core\PKPApplication; @@ -128,7 +129,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/PubIdExportIssuesListGridHandler.php b/controllers/grid/pubIds/PubIdExportIssuesListGridHandler.php index 555e2c226bb..95b216b0a10 100644 --- a/controllers/grid/pubIds/PubIdExportIssuesListGridHandler.php +++ b/controllers/grid/pubIds/PubIdExportIssuesListGridHandler.php @@ -237,7 +237,7 @@ protected function loadData($request, $filter) */ protected function getFilterValues($filter) { - 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/pubIds/PubIdExportRepresentationsListGridCellProvider.php b/controllers/grid/pubIds/PubIdExportRepresentationsListGridCellProvider.php index 8f3b0977776..51ac1519050 100644 --- a/controllers/grid/pubIds/PubIdExportRepresentationsListGridCellProvider.php +++ b/controllers/grid/pubIds/PubIdExportRepresentationsListGridCellProvider.php @@ -18,6 +18,7 @@ use APP\core\Application; use APP\facades\Repo; +use APP\plugins\PubObjectsExportPlugin; use PKP\controllers\grid\DataObjectGridCellProvider; use PKP\controllers\grid\GridHandler; use PKP\core\PKPApplication; @@ -158,7 +159,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 index 78be9d59612..fab175ecaf1 100644 --- a/controllers/grid/pubIds/PubIdExportRepresentationsListGridHandler.php +++ b/controllers/grid/pubIds/PubIdExportRepresentationsListGridHandler.php @@ -321,7 +321,7 @@ protected function getFilterValues($filter) } else { $issueId = 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 index 16107bd76c2..33a862b54ad 100644 --- a/controllers/grid/submissions/ExportPublishedSubmissionsListGridCellProvider.php +++ b/controllers/grid/submissions/ExportPublishedSubmissionsListGridCellProvider.php @@ -18,6 +18,7 @@ use APP\core\Application; use APP\facades\Repo; +use APP\plugins\PubObjectsExportPlugin; use APP\submission\Submission; use PKP\controllers\grid\DataObjectGridCellProvider; use PKP\controllers\grid\GridHandler; @@ -148,7 +149,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/submissions/ExportPublishedSubmissionsListGridHandler.php b/controllers/grid/submissions/ExportPublishedSubmissionsListGridHandler.php index 40dccc71c8b..3f817916d89 100644 --- a/controllers/grid/submissions/ExportPublishedSubmissionsListGridHandler.php +++ b/controllers/grid/submissions/ExportPublishedSubmissionsListGridHandler.php @@ -19,6 +19,7 @@ use APP\core\Application; use APP\facades\Repo; use APP\issue\Collector; +use APP\plugins\PubObjectsExportPlugin; use PKP\controllers\grid\feature\PagingFeature; use PKP\controllers\grid\feature\selectableItems\SelectableItemsFeature; use PKP\controllers\grid\GridColumn; @@ -299,7 +300,7 @@ protected function getFilterValues($filter) } else { $issueId = 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/plugins/importexport/doaj/DOAJExportPlugin.php b/plugins/importexport/doaj/DOAJExportPlugin.php index df1fdf8a3b2..a9d2f7815c6 100644 --- a/plugins/importexport/doaj/DOAJExportPlugin.php +++ b/plugins/importexport/doaj/DOAJExportPlugin.php @@ -89,9 +89,9 @@ public function getSubmissionFilter() */ 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(), 'apiKey')) { - array_unshift($actions, EXPORT_ACTION_DEPOSIT); + array_unshift($actions, PubObjectsExportPlugin::EXPORT_ACTION_DEPOSIT); } return $actions; } @@ -141,7 +141,7 @@ public function depositXML($objects, $context, $jsonString) return [['plugins.importexport.doaj.register.error.mdsError', $status . ' - ' . $response->getBody()]]; } // Deposit was received; set the status - $objects->setData($this->getDepositStatusSettingName(), EXPORT_STATUS_REGISTERED); + $objects->setData($this->getDepositStatusSettingName(), PubObjectsExportPlugin::EXPORT_STATUS_REGISTERED); $this->updateObject($objects); return true; } @@ -155,7 +155,7 @@ public function executeExportAction($request, $objects, $filter, $tab, $objectsF { $context = $request->getContext(); $path = ['plugin', $this->getName()]; - if ($request->getUserVar(EXPORT_ACTION_DEPOSIT)) { + if ($request->getUserVar(PubObjectsExportPlugin::EXPORT_ACTION_DEPOSIT)) { assert($filter != null); // Set filter for JSON $filter = 'article=>doaj-json'; diff --git a/plugins/importexport/doaj/templates/index.tpl b/plugins/importexport/doaj/templates/index.tpl index 427bbfb98e3..8d8f07b785e 100644 --- a/plugins/importexport/doaj/templates/index.tpl +++ b/plugins/importexport/doaj/templates/index.tpl @@ -37,7 +37,7 @@ {if !$allowExport}
{foreach from=$configurationErrors item=configurationError} - {if $configurationError == $smarty.const.EXPORT_CONFIG_ERROR_SETTINGS} + {if $configurationError == \APP\plugins\PubObjectsExportPlugin::EXPORT_CONFIG_ERROR_SETTINGS} {include file="controllers/notification/inPlaceNotificationContent.tpl" notificationId=doajConfigurationErrors notificationStyleClass="notifyWarning" notificationTitle="plugins.importexport.common.missingRequirements"|translate notificationContents="plugins.importexport.common.error.pluginNotConfigured"|translate} {/if} {/foreach} From c9882c383dbd867d9c46e1267df556189d9dd93b Mon Sep 17 00:00:00 2001 From: Jonas Raoni Soares da Silva Date: Wed, 29 May 2024 00:29:05 +0300 Subject: [PATCH 7/9] pkp/pkp-lib#8700 Updated code to make use of the Laravel Builder --- classes/issue/DAO.php | 64 +++--- classes/submission/DAO.php | 128 ++++++----- .../IndividualSubscriptionDAO.php | 73 ++++-- .../InstitutionalSubscriptionDAO.php | 155 +++++++------ classes/subscription/SubscriptionDAO.php | 215 ++++-------------- classes/subscription/SubscriptionTypeDAO.php | 15 +- 6 files changed, 287 insertions(+), 363 deletions(-) diff --git a/classes/issue/DAO.php b/classes/issue/DAO.php index 6d6211bccba..a422fc45d64 100644 --- a/classes/issue/DAO.php +++ b/classes/issue/DAO.php @@ -17,6 +17,8 @@ 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; @@ -267,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) - ->getCountForPagination(); - return $resultCount != 0; + ->getCountForPagination() > 0; } /** @@ -433,39 +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; - } - - if ($pubIdSettingName && $pubIdSettingValue && $pubIdSettingValue != PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED) { - $params[] = $pubIdSettingValue; - } - - $baseSql = ' - 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 == PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED) ? ' AND iss.setting_value IS NULL' : '') - . (($pubIdSettingName != null && $pubIdSettingValue != null && $pubIdSettingValue != PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED) ? ' AND iss.setting_value = ?' : '') - . (($pubIdSettingName != null && is_null($pubIdSettingValue)) ? ' AND (iss.setting_value IS NULL OR iss.setting_value = \'\')' : ''); - - $result = $this->deprecatedDao->retrieveRange( - "SELECT i.* - {$baseSql} - ORDER BY i.date_published DESC", - $params, - $rangeInfo - ); - - return new DAOResultFactory($result, $this, 'fromRow', [], "SELECT 0 {$baseSql}", $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/submission/DAO.php b/classes/submission/DAO.php index 0381628ab4b..f0faac7c21b 100644 --- a/classes/submission/DAO.php +++ b/classes/submission/DAO.php @@ -14,6 +14,9 @@ namespace APP\submission; use APP\plugins\PubObjectsExportPlugin; +use DB; +use Illuminate\Database\Query\Builder; +use Illuminate\Database\Query\JoinClause; use PKP\db\DAOResultFactory; use PKP\db\DBResultRange; use PKP\identity\Identity; @@ -43,69 +46,68 @@ public function deleteById(int $id) * * @return DAOResultFactory */ - 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 != PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED) { - $params[] = $pubIdSettingValue; - } - - $baseSql = ' - 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' - . ($issueId ? ' LEFT JOIN publication_settings psi ON p.publication_id = psi.publication_id AND psi.setting_name = \'issueId\' AND psi.locale = \'\'' : '') - . ($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 ?)' : '') - . ($issueId != null ? ' AND psi.setting_value = ?' : '') - . (($pubIdSettingName != null && $pubIdSettingValue != null && $pubIdSettingValue == PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED) ? ' AND pss.setting_value IS NULL' : '') - . (($pubIdSettingName != null && $pubIdSettingValue != null && $pubIdSettingValue != PubObjectsExportPlugin::EXPORT_STATUS_NOT_DEPOSITED) ? ' AND pss.setting_value = ?' : '') - . (($pubIdSettingName != null && is_null($pubIdSettingValue)) ? ' AND (pss.setting_value IS NULL OR pss.setting_value = \'\')' : ''); + 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( + $issueId, + fn (Builder $q) => $q->leftJoin( + 'publication_settings AS psi', + fn (JoinClause $j) => $j->on('p.publication_id', '=', 'psi.publication_id') + ->where('psi.setting_name', '=', 'issueId') + ->where('psi.locale', '=', '') + ) + ) + ->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($issueId != null, fn (Builder $q) => $q->where('psi.setting_value', '=', $issueId)) + ->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( - "SELECT s.* {$baseSql} - GROUP BY s.submission_id - ORDER BY MAX(p.date_published) DESC, s.submission_id DESC", - $params, - $rangeInfo - ); - return new DAOResultFactory($rows, $this, 'fromRow', [], "SELECT 0 {$baseSql}", $params, $rangeInfo); + $rows = $this->deprecatedDao->retrieveRange($q, [], $rangeInfo); + return new DAOResultFactory($rows, $this, 'fromRow', [], $q, [], $rangeInfo); } } diff --git a/classes/subscription/IndividualSubscriptionDAO.php b/classes/subscription/IndividualSubscriptionDAO.php index 2d3d84a3603..f48e736accd 100644 --- a/classes/subscription/IndividualSubscriptionDAO.php +++ b/classes/subscription/IndividualSubscriptionDAO.php @@ -18,9 +18,14 @@ namespace APP\subscription; +use APP\core\Application; +use DB; +use Illuminate\Database\Query\JoinClause; use PKP\core\Core; use PKP\db\DAOResultFactory; use PKP\db\DBResultRange; +use PKP\facades\Locale; +use PKP\identity\Identity; use PKP\plugins\Hook; class IndividualSubscriptionDAO extends SubscriptionDAO @@ -391,25 +396,55 @@ public function getAll($rangeInfo = null) */ public function getByJournalId($journalId, $status = null, $searchField = null, $searchMatch = null, $search = null, $dateField = null, $dateFrom = null, $dateTo = null, $rangeInfo = null) { - $params = array_merge($this->getFetchParameters(), [(int) $journalId]); - $baseSql = " - FROM subscriptions s - JOIN subscription_types st ON (s.type_id = st.type_id) - JOIN users u ON (s.user_id = u.user_id) - {$this->getFetchJoins()} - WHERE - st.institutional = 0 - AND s.journal_id = ? - " . parent::_generateSearchSQL($status, $searchField, $searchMatch, $search, $dateField, $dateFrom, $dateTo, $params); - - $result = $this->retrieveRange( - "SELECT s.*, {$this->getFetchColumns()} - {$baseSql} - ORDER BY u.user_id, s.subscription_id", - $params, - $rangeInfo - ); - return new DAOResultFactory($result, $this, '_fromRow', [], "SELECT 0 {$baseSql}", $params, $rangeInfo); // Counted in subscription grid paging + $locale = Locale::getLocale(); + // the users register for the site, thus + // the site primary locale should be the default locale + $site = Application::get()->getRequest()->getSite(); + $primaryLocale = $site->getPrimaryLocale(); + $q = DB::table('subscriptions', 's') + ->join('subscription_types AS st', 's.type_id', '=', 'st.type_id') + ->join('users AS u', 's.user_id', '=', 'u.user_id') + ->leftJoin( + 'user_settings AS ugl', + fn (JoinClause $j) => + $j->on('u.user_id', '=', 'ugl.user_id') + ->where('ugl.setting_name', '=', Identity::IDENTITY_SETTING_GIVENNAME) + ->where('ugl.locale', '=', $locale) + ) + ->leftJoin( + 'user_settings AS ugpl', + fn (JoinClause $j) => + $j->on('u.user_id', '=', 'ugpl.user_id') + ->where('ugpl.setting_name', '=', Identity::IDENTITY_SETTING_GIVENNAME) + ->where('ugpl.locale', '=', $primaryLocale) + ) + ->leftJoin( + 'user_settings AS ufl', + fn (JoinClause $j) => + $j->on('u.user_id', '=', 'ufl.user_id') + ->where('ufl.setting_name', '=', Identity::IDENTITY_SETTING_FAMILYNAME) + ->where('ufl.locale', '=', $locale) + ) + ->leftJoin( + 'user_settings AS ufpl', + fn (JoinClause $j) => + $j->on('u.user_id', '=', 'ufpl.user_id') + ->where('ufpl.setting_name', '=', Identity::IDENTITY_SETTING_FAMILYNAME) + ->where('ufpl.locale', '=', $primaryLocale) + ) + ->where('st.institutional', '=', 0) + ->where('s.journal_id', '=', $journalId) + ->orderBy('u.user_id') + ->orderBy('s.subscription_id') + ->select( + 's.*', + DB::raw('COALESCE(ugl.setting_value, ugpl.setting_value) AS user_given'), + DB::raw("CASE WHEN ugl.setting_value <> '' THEN ufl.setting_value ELSE ufpl.setting_value END AS user_family") + ); + $this->applySearchFilters($q, $status, $searchField, $searchMatch, $search, $dateField, $dateFrom, $dateTo, $params); + + $result = $this->retrieveRange($q, [], $rangeInfo); + return new DAOResultFactory($result, $this, '_fromRow', [], $q, [], $rangeInfo); // Counted in subscription grid paging } /** diff --git a/classes/subscription/InstitutionalSubscriptionDAO.php b/classes/subscription/InstitutionalSubscriptionDAO.php index 3773c7925e9..c2e17eb2fad 100644 --- a/classes/subscription/InstitutionalSubscriptionDAO.php +++ b/classes/subscription/InstitutionalSubscriptionDAO.php @@ -19,10 +19,13 @@ namespace APP\subscription; use APP\core\Application; +use Illuminate\Database\Query\JoinClause; +use Illuminate\Support\Facades\DB; use PKP\core\Core; use PKP\db\DAOResultFactory; use PKP\db\DBResultRange; use PKP\facades\Locale; +use PKP\identity\Identity; use PKP\plugins\Hook; class InstitutionalSubscriptionDAO extends SubscriptionDAO @@ -390,76 +393,100 @@ public function getAll($rangeInfo = null) */ public function getByJournalId($journalId, $status = null, $searchField = null, $searchMatch = null, $search = null, $dateField = null, $dateFrom = null, $dateTo = null, $rangeInfo = null) { - $params = array_merge($this->getInstitutionNameFetchParameters(), $this->getFetchParameters(), [(int) $journalId]); - $institutionFetch = $ipRangeFetch = ''; - $searchSql = $this->_generateSearchSQL($status, $searchField, $searchMatch, $search, $dateField, $dateFrom, $dateTo, $params); + $locale = Locale::getLocale(); + $request = Application::get()->getRequest(); + $journal = $request->getContext(); + $journalPrimaryLocale = $journal->getPrimaryLocale(); + $site = $request->getSite(); + $sitePrimaryLocale = $site->getPrimaryLocale(); + + $q = DB::table('subscriptions', 's') + ->join('subscription_types AS st', 's.type_id', '=', 'st.type_id') + ->join('users AS u', 's.user_id', '=', 'u.user_id') + ->join('institutional_subscriptions AS iss', 's.subscription_id', '=', 'iss.subscription_id') + ->leftJoin( + 'institution_settings AS isal', + fn (JoinClause $j) => + $j->on('isal.institution_id', '=', 'iss.institution_id') + ->where('isal.setting_name', '=', 'name') + ->where('isal.locale', '=', $locale) + ) + ->leftJoin( + 'institution_settings AS isapl', + fn (JoinClause $j) => + $j->on('isapl.institution_id', '=', 'iss.institution_id') + ->where('isapl.setting_name', '=', 'name') + ->where('isapl.locale', '=', $journalPrimaryLocale) + ) + ->leftJoin( + 'user_settings AS ugl', + fn (JoinClause $j) => + $j->on('u.user_id', '=', 'ugl.user_id') + ->where('ugl.setting_name', '=', Identity::IDENTITY_SETTING_GIVENNAME) + ->where('ugl.locale', '=', $locale) + ) + ->leftJoin( + 'user_settings AS ugpl', + fn (JoinClause $j) => + $j->on('u.user_id', '=', 'ugpl.user_id') + ->where('ugpl.setting_name', '=', Identity::IDENTITY_SETTING_GIVENNAME) + ->where('ugpl.locale', '=', $sitePrimaryLocale) + ) + ->leftJoin( + 'user_settings AS ufl', + fn (JoinClause $j) => + $j->on('u.user_id', '=', 'ufl.user_id') + ->where('ufl.setting_name', '=', Identity::IDENTITY_SETTING_FAMILYNAME) + ->where('ufl.locale', '=', $locale) + ) + ->leftJoin( + 'user_settings AS ufpl', + fn (JoinClause $j) => + $j->on('u.user_id', '=', 'ufpl.user_id') + ->where('ufpl.setting_name', '=', Identity::IDENTITY_SETTING_FAMILYNAME) + ->where('ufpl.locale', '=', $sitePrimaryLocale) + ) + ->where('st.institutional', '=', 1) + ->where('s.journal_id', '=', $journalId) + ->orderBy('institution_name') + ->orderBy('s.subscription_id') + ->select( + 's.*', + 'iss.institution_id', + 'iss.mailing_address', + 'iss.domain', + DB::raw('COALESCE(isal.setting_value, isapl.setting_value) AS institution_name'), + DB::raw('COALESCE(ugl.setting_value, ugpl.setting_value) AS user_given'), + DB::raw("CASE WHEN ugl.setting_value <> '' THEN ufl.setting_value ELSE ufpl.setting_value END AS user_family") + ) + ->distinct(); if (!empty($search)) { - switch ($searchField) { - case self::SUBSCRIPTION_INSTITUTION_NAME: - if ($searchMatch === 'is') { - $searchSql = ' AND LOWER(insl.setting_value) = LOWER(?)'; - } elseif ($searchMatch === 'contains') { - $searchSql = ' AND LOWER(insl.setting_value) LIKE LOWER(?)'; - $search = '%' . $search . '%'; - } else { // $searchMatch === 'startsWith' - $searchSql = ' AND LOWER(insl) LIKE LOWER(?)'; - $search = $search . '%'; - } - $institutionFetch = 'JOIN institution_settings insl ON (insl.institution_id = iss.institution_id AND insl.setting_name = \'name\')'; - $params[] = $search; - break; - case self::SUBSCRIPTION_DOMAIN: - if ($searchMatch === 'is') { - $searchSql = ' AND LOWER(iss.domain) = LOWER(?)'; - } elseif ($searchMatch === 'contains') { - $searchSql = ' AND LOWER(iss.domain) LIKE LOWER(?)'; - $search = '%' . $search . '%'; - } else { // $searchMatch === 'startsWith' - $searchSql = ' AND LOWER(iss.domain) LIKE LOWER(?)'; - $search = $search . '%'; - } - $params[] = $search; - break; - case self::SUBSCRIPTION_IP_RANGE: - if ($searchMatch === 'inip') { - $searchSql = ' AND LOWER(inip.ip_string) = LOWER(?)'; - } elseif ($searchMatch === 'contains') { - $searchSql = ' AND LOWER(inip.ip_string) LIKE LOWER(?)'; - $search = '%' . $search . '%'; - } else { // $searchMatch === 'startsWith' - $searchSql = ' AND LOWER(inip.ip_string) LIKE LOWER(?)'; - $search = $search . '%'; - } - $ipRangeFetch = ' JOIN institution_ip inip ON (inip.institution_id = iss.institution_id)'; - $params[] = $search; - break; + $field = match ($searchField) { + self::SUBSCRIPTION_INSTITUTION_NAME => 'insl.setting_value', + self::SUBSCRIPTION_DOMAIN => 'iss.domain', + self::SUBSCRIPTION_IP_RANGE => 'inip.ip_string', + default => null + }; + match ($searchField) { + self::SUBSCRIPTION_INSTITUTION_NAME => $q->join('institution_settings AS insl', fn (JoinClause $j) => $j->on('insl.institution_id', '=', 'iss.institution_id')->where('insl.setting_name', '=', 'name')), + self::SUBSCRIPTION_IP_RANGE => $q->join('institution_ip AS inip', 'inip.institution_id', '=', 'iss.institution_id'), + default => null + }; + if ($field) { + match ($searchMatch) { + 'is' => $q->whereRaw("LOWER({$field}) = LOWER(?)", [$search]), + 'contains' => $q->whereRaw("LOWER({$field}) LIKE LOWER(?)", ["%{$search}%"]), + //startsWith + default => $q->whereRaw("LOWER({$field}) = LOWER(?)", ["{$search}%"]) + }; } } - $baseSql = " - FROM subscriptions s - JOIN subscription_types st ON (s.type_id = st.type_id) - JOIN users u ON (s.user_id = u.user_id) - JOIN institutional_subscriptions iss ON (s.subscription_id = iss.subscription_id) - {$institutionFetch} - {$ipRangeFetch} - {$this->getInstitutionNameFetchJoins()} - {$this->getFetchJoins()} - WHERE st.institutional = 1 AND s.journal_id = ? - {$searchSql}"; - - $result = $this->retrieveRange( - "SELECT DISTINCT s.*, iss.institution_id, iss.mailing_address, iss.domain, - {$this->getInstitutionNameFetchColumns()}, - {$this->getFetchColumns()} - {$baseSql} - ORDER BY institution_name ASC, s.subscription_id", - $params, - $rangeInfo - ); + $this->applySearchFilters($q, $status, $searchField, $searchMatch, $search, $dateField, $dateFrom, $dateTo, $params); - return new DAOResultFactory($result, $this, '_fromRow', [], "SELECT 0 {$baseSql}", $params, $rangeInfo); // Counted in subscription grid paging + $result = $this->retrieveRange($q, [], $rangeInfo); + return new DAOResultFactory($result, $this, '_fromRow', [], $q, [], $rangeInfo); // Counted in subscription grid paging } /** diff --git a/classes/subscription/SubscriptionDAO.php b/classes/subscription/SubscriptionDAO.php index 57d91d5c91d..a5d20702c2f 100644 --- a/classes/subscription/SubscriptionDAO.php +++ b/classes/subscription/SubscriptionDAO.php @@ -18,12 +18,11 @@ namespace APP\subscription; -use APP\core\Application; use APP\facades\Repo; +use Illuminate\Database\Query\Builder; use PKP\db\DAORegistry; use PKP\db\DAOResultFactory; use PKP\db\DBResultRange; -use PKP\facades\Locale; use PKP\identity\Identity; use PKP\plugins\Hook; @@ -222,172 +221,44 @@ abstract public function renewSubscription($subscription); * @param null|mixed $dateFrom * @param null|mixed $dateTo * @param null|mixed $params - * - * @return string */ - protected function _generateSearchSQL($status = null, $searchField = null, $searchMatch = null, $search = null, $dateField = null, $dateFrom = null, $dateTo = null, &$params = null) + protected function applySearchFilters(Builder $query, $status = null, $searchField = null, $searchMatch = null, $search = null, $dateField = null, $dateFrom = null, $dateTo = null, &$params = null) { - $searchSql = ''; $userDao = Repo::user()->dao; if (!empty($search)) { - switch ($searchField) { - case Identity::IDENTITY_SETTING_GIVENNAME: - if ($searchMatch === 'is') { - $searchSql = ' AND LOWER(COALESCE(ugl.setting_value,ugpl.setting_value)) = LOWER(?)'; - } elseif ($searchMatch === 'contains') { - $searchSql = ' AND LOWER(COALESCE(ugl.setting_value,ugpl.setting_value)) LIKE LOWER(?)'; - $search = '%' . $search . '%'; - } else { // $searchMatch === 'startsWith' - $searchSql = ' AND LOWER(COALESCE(ugl,ugpl)) LIKE LOWER(?)'; - $search = $search . '%'; - } - $params[] = $search; - break; - case Identity::IDENTITY_SETTING_FAMILYNAME: - if ($searchMatch === 'is') { - $searchSql = ' AND LOWER(COALESCE(ufl.setting_value,ufpl.setting_value)) = LOWER(?)'; - } elseif ($searchMatch === 'contains') { - $searchSql = ' AND LOWER(COALESCE(ufl.setting_value,ufpl.setting_value)) LIKE LOWER(?)'; - $search = '%' . $search . '%'; - } else { // $searchMatch === 'startsWith' - $searchSql = ' AND LOWER(COALESCE(ufl.setting_value,ufpl.setting_value)) LIKE LOWER(?)'; - $search = $search . '%'; - } - $params[] = $search; - break; - case $userDao::USER_FIELD_USERNAME: - if ($searchMatch === 'is') { - $searchSql = ' AND LOWER(u.username) = LOWER(?)'; - } elseif ($searchMatch === 'contains') { - $searchSql = ' AND LOWER(u.username) LIKE LOWER(?)'; - $search = '%' . $search . '%'; - } else { // $searchMatch === 'startsWith' - $searchSql = ' AND LOWER(u.username) LIKE LOWER(?)'; - $search = $search . '%'; - } - $params[] = $search; - break; - case $userDao::USER_FIELD_EMAIL: - if ($searchMatch === 'is') { - $searchSql = ' AND LOWER(u.email) = LOWER(?)'; - } elseif ($searchMatch === 'contains') { - $searchSql = ' AND LOWER(u.email) LIKE LOWER(?)'; - $search = '%' . $search . '%'; - } else { // $searchMatch === 'startsWith' - $searchSql = ' AND LOWER(u.email) LIKE LOWER(?)'; - $search = $search . '%'; - } - $params[] = $search; - break; - case self::SUBSCRIPTION_MEMBERSHIP: - if ($searchMatch === 'is') { - $searchSql = ' AND LOWER(s.membership) = LOWER(?)'; - } elseif ($searchMatch === 'contains') { - $searchSql = ' AND LOWER(s.membership) LIKE LOWER(?)'; - $search = '%' . $search . '%'; - } else { // $searchMatch === 'startsWith' - $searchSql = ' AND LOWER(s.membership) LIKE LOWER(?)'; - $search = $search . '%'; - } - $params[] = $search; - break; - case self::SUBSCRIPTION_REFERENCE_NUMBER: - if ($searchMatch === 'is') { - $searchSql = ' AND LOWER(s.reference_number) = LOWER(?)'; - } elseif ($searchMatch === 'contains') { - $searchSql = ' AND LOWER(s.reference_number) LIKE LOWER(?)'; - $search = '%' . $search . '%'; - } else { // $searchMatch === 'startsWith' - $searchSql = ' AND LOWER(s.reference_number) LIKE LOWER(?)'; - $search = $search . '%'; - } - $params[] = $search; - break; - case self::SUBSCRIPTION_NOTES: - if ($searchMatch === 'is') { - $searchSql = ' AND LOWER(s.notes) = LOWER(?)'; - } elseif ($searchMatch === 'contains') { - $searchSql = ' AND LOWER(s.notes) LIKE LOWER(?)'; - $search = '%' . $search . '%'; - } else { // $searchMatch === 'startsWith' - $searchSql = ' AND LOWER(s.notes) LIKE LOWER(?)'; - $search = $search . '%'; - } - $params[] = $search; - break; + $field = match ($searchField) { + Identity::IDENTITY_SETTING_GIVENNAME => 'COALESCE(ugl.setting_value, ugpl.setting_value)', + Identity::IDENTITY_SETTING_FAMILYNAME => 'COALESCE(ufl.setting_value, ufpl.setting_value)', + $userDao::USER_FIELD_USERNAME => 'u.username', + $userDao::USER_FIELD_EMAIL => 'u.email', + static::SUBSCRIPTION_MEMBERSHIP => 's.membership', + static::SUBSCRIPTION_REFERENCE_NUMBER => 's.reference_number', + static::SUBSCRIPTION_NOTES => 's.notes', + default => null + }; + if ($field) { + match ($searchMatch) { + 'is' => $query->whereRaw("LOWER({$field}) = LOWER(?)", [$search]), + 'contains' => $query->whereRaw("LOWER({$field}) LIKE LOWER(?)", ["%{$search}%"]), + //startsWith + default => $query->whereRaw("LOWER({$field}) = LOWER(?)", ["{$search}%"]) + }; } } - if (!empty($dateFrom) || !empty($dateTo)) { - switch ($dateField) { - case Subscription::SUBSCRIPTION_DATE_START: - if (!empty($dateFrom)) { - $searchSql .= ' AND s.date_start >= ' . $this->datetimeToDB($dateFrom); - } - if (!empty($dateTo)) { - $searchSql .= ' AND s.date_start <= ' . $this->datetimeToDB($dateTo); - } - break; - case Subscription::SUBSCRIPTION_DATE_END: - if (!empty($dateFrom)) { - $searchSql .= ' AND s.date_end >= ' . $this->datetimeToDB($dateFrom); - } - if (!empty($dateTo)) { - $searchSql .= ' AND s.date_end <= ' . $this->datetimeToDB($dateTo); - } - break; + if ($dateField) { + $field = match ($dateField) { + Subscription::SUBSCRIPTION_DATE_START => 's.date_start', + Subscription::SUBSCRIPTION_DATE_END => 's.date_end', + default => null + }; + if ($field) { + $query->when(!empty($dateFrom), fn (Builder $q) => $q->where($field, '>=', $this->datetimeToDB($dateFrom))) + ->when(!empty($dateTo), fn (Builder $q) => $q->where($field, '<=', $this->datetimeToDB($dateTo))); } } - if (!empty($status)) { - $searchSql .= ' AND s.status = ' . (int) $status; - } - - return $searchSql; - } - - /** - * Return a list of extra parameters to bind to the user fetch queries. - * - * @return array - */ - public function getFetchParameters() - { - $locale = Locale::getLocale(); - // the users register for the site, thus - // the site primary locale should be the default locale - $site = Application::get()->getRequest()->getSite(); - $primaryLocale = $site->getPrimaryLocale(); - return [ - Identity::IDENTITY_SETTING_GIVENNAME, $locale, - Identity::IDENTITY_SETTING_GIVENNAME, $primaryLocale, - Identity::IDENTITY_SETTING_FAMILYNAME, $locale, - Identity::IDENTITY_SETTING_FAMILYNAME, $primaryLocale, - ]; - } - - /** - * Return a SQL snippet of extra columns to fetch during user fetch queries. - * - * @return string - */ - public function getFetchColumns() - { - return 'COALESCE(ugl.setting_value, ugpl.setting_value) AS user_given, - CASE WHEN ugl.setting_value <> \'\' THEN ufl.setting_value ELSE ufpl.setting_value END AS user_family'; - } - - /** - * Return a SQL snippet of extra joins to include during user fetch queries. - * - * @return string - */ - public function getFetchJoins() - { - return 'LEFT JOIN user_settings ugl ON (u.user_id = ugl.user_id AND ugl.setting_name = ? AND ugl.locale = ?) - LEFT JOIN user_settings ugpl ON (u.user_id = ugpl.user_id AND ugpl.setting_name = ? AND ugpl.locale = ?) - LEFT JOIN user_settings ufl ON (u.user_id = ufl.user_id AND ufl.setting_name = ? AND ufl.locale = ?) - LEFT JOIN user_settings ufpl ON (u.user_id = ufpl.user_id AND ufpl.setting_name = ? AND ufpl.locale = ?)'; + $query->when(!empty($status), fn (Builder $q) => $q->where('s.status', '=', $status)); } /** @@ -437,9 +308,9 @@ public function _insertObject($subscription) $this->update( sprintf( 'INSERT INTO subscriptions - (journal_id, user_id, type_id, date_start, date_end, status, membership, reference_number, notes) - VALUES - (?, ?, ?, %s, %s, ?, ?, ?, ?)', + (journal_id, user_id, type_id, date_start, date_end, status, membership, reference_number, notes) + VALUES + (?, ?, ?, %s, %s, ?, ?, ?, ?)', $dateStart !== null ? $this->dateToDB($dateStart) : 'null', $dateEnd !== null ? $this->datetimeToDB($dateEnd) : 'null' ), @@ -472,17 +343,17 @@ public function _updateObject($subscription) $this->update( sprintf( 'UPDATE subscriptions - SET - journal_id = ?, - user_id = ?, - type_id = ?, - date_start = %s, - date_end = %s, - status = ?, - membership = ?, - reference_number = ?, - notes = ? - WHERE subscription_id = ?', + SET + journal_id = ?, + user_id = ?, + type_id = ?, + date_start = %s, + date_end = %s, + status = ?, + membership = ?, + reference_number = ?, + notes = ? + WHERE subscription_id = ?', $dateStart !== null ? $this->dateToDB($dateStart) : 'null', $dateEnd !== null ? $this->datetimeToDB($dateEnd) : 'null' ), diff --git a/classes/subscription/SubscriptionTypeDAO.php b/classes/subscription/SubscriptionTypeDAO.php index 6dd28be2fe6..e9274135aba 100644 --- a/classes/subscription/SubscriptionTypeDAO.php +++ b/classes/subscription/SubscriptionTypeDAO.php @@ -320,15 +320,12 @@ public function deleteByJournal($journalId) */ public function getByJournalId($journalId, $rangeInfo = null) { - $baseSql = ' - FROM subscription_types - WHERE journal_id = ?'; - $result = $this->retrieveRange( - "SELECT * {$baseSql} ORDER BY seq", - $params = [(int) $journalId], - $rangeInfo - ); - return new DAOResultFactory($result, $this, '_fromRow', [], "SELECT 0 {$baseSql}", $params, $rangeInfo); // Counted in subscription type grid paging + $q = DB::table('subscription_types', 'st') + ->where('journal_id', '=', $journalId) + ->orderBy('st.seq') + ->select('st.*'); + $result = $this->retrieveRange($q, [], $rangeInfo); + return new DAOResultFactory($result, $this, '_fromRow', [], $q, [], $rangeInfo); // Counted in subscription type grid paging } /** From 4889ec4df16c53c3d77c53a316fed23b35ced022 Mon Sep 17 00:00:00 2001 From: Jonas Raoni Soares da Silva Date: Wed, 29 May 2024 00:30:10 +0300 Subject: [PATCH 8/9] pkp/pkp-lib#8700 Fixed namespace --- classes/submission/DAO.php | 2 +- classes/subscription/IndividualSubscriptionDAO.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/classes/submission/DAO.php b/classes/submission/DAO.php index f0faac7c21b..c1fc9cdea37 100644 --- a/classes/submission/DAO.php +++ b/classes/submission/DAO.php @@ -14,9 +14,9 @@ namespace APP\submission; use APP\plugins\PubObjectsExportPlugin; -use DB; 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; diff --git a/classes/subscription/IndividualSubscriptionDAO.php b/classes/subscription/IndividualSubscriptionDAO.php index f48e736accd..5b334aa3295 100644 --- a/classes/subscription/IndividualSubscriptionDAO.php +++ b/classes/subscription/IndividualSubscriptionDAO.php @@ -19,8 +19,8 @@ namespace APP\subscription; use APP\core\Application; -use DB; use Illuminate\Database\Query\JoinClause; +use Illuminate\Support\Facades\DB; use PKP\core\Core; use PKP\db\DAOResultFactory; use PKP\db\DBResultRange; From 04606dc9f8d395af0d206b1fccdb85fff0ec2fd4 Mon Sep 17 00:00:00 2001 From: Jonas Raoni Soares da Silva Date: Wed, 29 May 2024 18:48:17 +0300 Subject: [PATCH 9/9] pkp/pkp-lib#8700 Submodule update ##jonasraoni/feature-stable-3_4_0-8700-improve-slow-query## --- lib/pkp | 2 +- plugins/generic/googleScholar | 2 +- plugins/generic/orcidProfile | 2 +- plugins/generic/pdfJsViewer | 2 +- plugins/generic/webFeed | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/pkp b/lib/pkp index c1bcdc8d515..d9c1a6f7b72 160000 --- a/lib/pkp +++ b/lib/pkp @@ -1 +1 @@ -Subproject commit c1bcdc8d515e7b711c9a0e16c4a1a51d3eaf1414 +Subproject commit d9c1a6f7b726d88310b486d5a22ce6b501c10a49 diff --git a/plugins/generic/googleScholar b/plugins/generic/googleScholar index 523e4927606..2e9f7e3d3e2 160000 --- a/plugins/generic/googleScholar +++ b/plugins/generic/googleScholar @@ -1 +1 @@ -Subproject commit 523e49276066624fc3c63ae45ad7f935f19079a7 +Subproject commit 2e9f7e3d3e29496424b096c071b34b587a467f1b diff --git a/plugins/generic/orcidProfile b/plugins/generic/orcidProfile index d57a9d6c542..05d51c8b452 160000 --- a/plugins/generic/orcidProfile +++ b/plugins/generic/orcidProfile @@ -1 +1 @@ -Subproject commit d57a9d6c542199071f41cead6a277f60bb31cbb5 +Subproject commit 05d51c8b4520229bb9f38ed92050b039fec0cd37 diff --git a/plugins/generic/pdfJsViewer b/plugins/generic/pdfJsViewer index e22f0688e56..5102e8383d8 160000 --- a/plugins/generic/pdfJsViewer +++ b/plugins/generic/pdfJsViewer @@ -1 +1 @@ -Subproject commit e22f0688e568637a5c6bc8ab7f366428563695cf +Subproject commit 5102e8383d894c4ecd7ae5c9d132d4735f2d61ee diff --git a/plugins/generic/webFeed b/plugins/generic/webFeed index fb382b05c46..6c91be5860b 160000 --- a/plugins/generic/webFeed +++ b/plugins/generic/webFeed @@ -1 +1 @@ -Subproject commit fb382b05c460acdece9825253dfd275e8d768e14 +Subproject commit 6c91be5860b77c837c0ddf1ec0c70f61e4ebb947