Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/main/8700 improve slow query #3870

Merged
merged 9 commits into from
May 30, 2024
2 changes: 1 addition & 1 deletion api/v1/issues/IssueController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
4 changes: 2 additions & 2 deletions classes/doi/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down
66 changes: 30 additions & 36 deletions classes/issue/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -133,7 +136,7 @@ public function getCount(Collector $query): int
{
return $query
->getQueryBuilder()
->count();
->getCountForPagination();
}

/**
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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);
}

/**
Expand Down
69 changes: 38 additions & 31 deletions classes/plugins/PubObjectsExportPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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'),
];
}

Expand All @@ -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;
}
Expand All @@ -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'),
];
}

Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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',
Expand All @@ -594,7 +588,7 @@ protected function _getObjectAdditionalSettings()
*/
public function callbackParseCronTab($hookName, $args)
{
$taskFilesPath = & $args[0];
$taskFilesPath = &$args[0];

$scheduledTasksPath = "{$this->getPluginPath()}/scheduledTasks.xml";

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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));
}
}
2 changes: 1 addition & 1 deletion classes/services/StatsIssueService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

/**
Expand Down
8 changes: 3 additions & 5 deletions classes/services/queryBuilders/GalleyQueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand All @@ -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]]
*/
Expand Down
2 changes: 1 addition & 1 deletion classes/services/queryBuilders/StatsIssueQueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public function getIssueIds(): Builder
{
return $this->_getObject()
->select([StatisticsHelper::STATISTICS_DIMENSION_ISSUE_ID])
->distinct();
->groupBy(StatisticsHelper::STATISTICS_DIMENSION_ISSUE_ID);
}

/**
Expand Down
Loading