From 486a8ac38af256d0bd1bbf4b5dda36cb95dc0f7b Mon Sep 17 00:00:00 2001 From: Jonathan Miller Date: Sat, 6 Jun 2026 06:03:50 -0500 Subject: [PATCH] fix: resolve 5 open bugs (#92, #94, #95, #100, #101) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - #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) --- .../sql/updates/mysql/01.01.00.sql | 1 + .../src/Controller/PostsController.php | 14 ++- .../src/Helper/QueueProcessor.php | 102 ++++++++++++++++-- .../com_mokojoomcross/src/Model/PostModel.php | 10 ++ 4 files changed, 117 insertions(+), 10 deletions(-) diff --git a/src/packages/com_mokojoomcross/sql/updates/mysql/01.01.00.sql b/src/packages/com_mokojoomcross/sql/updates/mysql/01.01.00.sql index 4e51ffbd..074b66a1 100644 --- a/src/packages/com_mokojoomcross/sql/updates/mysql/01.01.00.sql +++ b/src/packages/com_mokojoomcross/sql/updates/mysql/01.01.00.sql @@ -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, diff --git a/src/packages/com_mokojoomcross/src/Controller/PostsController.php b/src/packages/com_mokojoomcross/src/Controller/PostsController.php index 6adcff92..379dafc5 100644 --- a/src/packages/com_mokojoomcross/src/Controller/PostsController.php +++ b/src/packages/com_mokojoomcross/src/Controller/PostsController.php @@ -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(); diff --git a/src/packages/com_mokojoomcross/src/Helper/QueueProcessor.php b/src/packages/com_mokojoomcross/src/Helper/QueueProcessor.php index db06b938..2fc4053c 100644 --- a/src/packages/com_mokojoomcross/src/Helper/QueueProcessor.php +++ b/src/packages/com_mokojoomcross/src/Helper/QueueProcessor.php @@ -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(); } diff --git a/src/packages/com_mokojoomcross/src/Model/PostModel.php b/src/packages/com_mokojoomcross/src/Model/PostModel.php index fe5ad1e8..ef98a999 100644 --- a/src/packages/com_mokojoomcross/src/Model/PostModel.php +++ b/src/packages/com_mokojoomcross/src/Model/PostModel.php @@ -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;