fix: resolve 5 open bugs (#92, #94, #95, #100, #101)
Generic: Repo Health / Site Health (push) Has been cancelled
Generic: Repo Health / Access control (push) Has been cancelled
Generic: Repo Health / Scripts governance (push) Has been cancelled
Generic: Repo Health / Repository health (push) Has been cancelled
Generic: Repo Health / Report Issues (push) Has been cancelled

- #92: Replace MySQL-only GET_LOCK with db-agnostic locking — adds
  PostgreSQL pg_advisory_lock and timestamp-based fallback
- #94: retryFailed() now includes permanently_failed and cancelled
  statuses, not just failed
- #95: Validate scheduled_at datetime via Factory::getDate() in both
  PostModel::prepareTable() and PostsController::schedule()
- #100: Add clarifying comment to update SQL for duplicate table def
- #101: Replace fragile LIKE query with JSON_EXTRACT() for evergreen
  article detection

Authored-by: Moko Consulting
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jonathan Miller
2026-06-06 06:03:50 -05:00
parent 0ea55601d2
commit 486a8ac38a
4 changed files with 117 additions and 10 deletions
@@ -1,6 +1,7 @@
-- MokoJoomCross 01.01.00 — Category routing rules
-- Copyright (C) 2026 Moko Consulting. All rights reserved.
-- SPDX-License-Identifier: GPL-3.0-or-later
-- Note: also in install.mysql.sql for fresh installs; IF NOT EXISTS prevents conflicts
CREATE TABLE IF NOT EXISTS `#__mokojoomcross_category_rules` (
`id` int(10) unsigned NOT NULL AUTO_INCREMENT,
@@ -55,6 +55,18 @@ class PostsController extends AdminController
return;
}
try {
$scheduledDate = Factory::getDate($scheduledAt);
$scheduledAt = $scheduledDate->toSql();
} catch (\Throwable $e) {
$this->setRedirect(
Route::_('index.php?option=com_mokojoomcross&view=posts', false),
Text::_('COM_MOKOJOOMCROSS_SCHEDULE_INVALID_DATE'),
'error'
);
return;
}
$db = Factory::getDbo();
$now = Factory::getDate()->toSql();
@@ -123,7 +135,7 @@ class PostsController extends AdminController
->set($db->quoteName('retry_count') . ' = 0')
->set($db->quoteName('error_message') . ' = ' . $db->quote(''))
->set($db->quoteName('modified') . ' = ' . $db->quote(Factory::getDate()->toSql()))
->where($db->quoteName('status') . ' = ' . $db->quote('failed'));
->where($db->quoteName('status') . ' IN (' . $db->quote('failed') . ',' . $db->quote('permanently_failed') . ',' . $db->quote('cancelled') . ')');
$db->setQuery($query);
$db->execute();
@@ -320,7 +320,7 @@ class QueueProcessor
->select('c.id, c.attribs')
->from($db->quoteName('#__content', 'c'))
->where($db->quoteName('c.state') . ' = 1')
->where($db->quoteName('c.attribs') . ' LIKE ' . $db->quote('%"mokojoomcross_evergreen":"1"%'));
->where('JSON_EXTRACT(' . $db->quoteName('c.attribs') . ', ' . $db->quote('$.mokojoomcross_evergreen') . ') = ' . $db->quote('1'));
$db->setQuery($query);
$articles = $db->loadObjectList() ?: [];
@@ -733,27 +733,111 @@ class QueueProcessor
}
/**
* Acquire a MySQL advisory lock to prevent concurrent queue processing.
* Acquire a database lock to prevent concurrent queue processing.
*
* Uses GET_LOCK() which is atomic — no race condition possible.
* The 0 timeout means non-blocking (returns immediately if lock is held).
* MySQL automatically releases the lock if the connection drops.
* Uses MySQL GET_LOCK() or PostgreSQL pg_advisory_lock() when available,
* falling back to a timestamp-based check for other databases.
*/
private static function acquireLock(): bool
{
$db = Factory::getDbo();
$db->setQuery("SELECT GET_LOCK('mokojoomcross_queue', 0)");
return (int) $db->loadResult() === 1;
try {
$serverType = $db->getServerType();
if ($serverType === 'mysql' || $serverType === 'mariadb') {
$db->setQuery("SELECT GET_LOCK('mokojoomcross_queue', 0)");
return (int) $db->loadResult() === 1;
}
if ($serverType === 'postgresql') {
$db->setQuery("SELECT pg_try_advisory_lock(hashtext('mokojoomcross_queue'))");
return (bool) $db->loadResult();
}
} catch (\Throwable $e) {
// Fall through to timestamp-based lock
}
return self::acquireTimestampLock($db);
}
/**
* Release the MySQL advisory lock.
* Release the database lock.
*/
private static function releaseLock(): void
{
$db = Factory::getDbo();
$db->setQuery("SELECT RELEASE_LOCK('mokojoomcross_queue')");
try {
$serverType = $db->getServerType();
if ($serverType === 'mysql' || $serverType === 'mariadb') {
$db->setQuery("SELECT RELEASE_LOCK('mokojoomcross_queue')");
$db->execute();
return;
}
if ($serverType === 'postgresql') {
$db->setQuery("SELECT pg_advisory_unlock(hashtext('mokojoomcross_queue'))");
$db->execute();
return;
}
} catch (\Throwable $e) {
// Fall through to timestamp-based release
}
self::releaseTimestampLock($db);
}
/**
* Timestamp-based lock fallback for databases without advisory locks.
*
* Uses the component params to store a lock timestamp. Considers the lock
* stale after 120 seconds to prevent deadlocks from crashed processes.
*/
private static function acquireTimestampLock($db): bool
{
$params = ComponentHelper::getParams('com_mokojoomcross');
$lockTime = (int) $params->get('queue_lock_time', 0);
$now = time();
if ($lockTime > 0 && ($now - $lockTime) < 120) {
return false;
}
$params->set('queue_lock_time', $now);
$query = $db->getQuery(true)
->update($db->quoteName('#__extensions'))
->set($db->quoteName('params') . ' = ' . $db->quote($params->toString()))
->where($db->quoteName('element') . ' = ' . $db->quote('com_mokojoomcross'))
->where($db->quoteName('type') . ' = ' . $db->quote('component'));
$db->setQuery($query);
$db->execute();
return true;
}
/**
* Release the timestamp-based lock.
*/
private static function releaseTimestampLock($db): void
{
$params = ComponentHelper::getParams('com_mokojoomcross');
$params->set('queue_lock_time', 0);
$query = $db->getQuery(true)
->update($db->quoteName('#__extensions'))
->set($db->quoteName('params') . ' = ' . $db->quote($params->toString()))
->where($db->quoteName('element') . ' = ' . $db->quote('com_mokojoomcross'))
->where($db->quoteName('type') . ' = ' . $db->quote('component'));
$db->setQuery($query);
$db->execute();
}
@@ -53,6 +53,16 @@ class PostModel extends AdminModel
{
$now = Factory::getDate()->toSql();
// Validate scheduled_at datetime format
if (!empty($table->scheduled_at)) {
try {
$date = Factory::getDate($table->scheduled_at);
$table->scheduled_at = $date->toSql();
} catch (\Throwable $e) {
$table->scheduled_at = null;
}
}
if (empty($table->id)) {
$table->created = $now;
$table->modified = $now;