From 0b87bd81c945bec201bc0bd1cb9f2e097e396db9 Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Thu, 21 Nov 2024 17:23:36 +0000 Subject: [PATCH 1/3] Prevent duplicate entries using unique index Addresses #89 Using a generated column to make a hash of cron task uniqueness lets us add a unique index to the table. Changing the $wpdb->insert() call to use replace() means that an existing record will be updated rather than causing an error. Need to determine if this is really the right way to define uniqueness. Potential for improving lookup performance using a generated hash column for args as well as thats more reasonable to add an index for. --- inc/class-job.php | 2 +- inc/namespace.php | 6 +++++ inc/upgrade/namespace.php | 49 +++++++++++++++++++++++++++++++++++++++ plugin.php | 2 +- 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/inc/class-job.php b/inc/class-job.php index f6087b5..052d06b 100644 --- a/inc/class-job.php +++ b/inc/class-job.php @@ -64,7 +64,7 @@ public function save() { ]; $result = $wpdb->update( $this->get_table(), $data, $where, $this->row_format( $data ), $this->row_format( $where ) ); } else { - $result = $wpdb->insert( $this->get_table(), $data, $this->row_format( $data ) ); + $result = $wpdb->replace( $this->get_table(), $data, $this->row_format( $data ) ); $this->id = $wpdb->insert_id; } diff --git a/inc/namespace.php b/inc/namespace.php index 3288229..a2b60d8 100644 --- a/inc/namespace.php +++ b/inc/namespace.php @@ -90,8 +90,14 @@ function create_tables() { `interval` int unsigned DEFAULT NULL, `status` varchar(255) NOT NULL DEFAULT 'waiting', `schedule` varchar(255) DEFAULT NULL, + `hash` binary(32) GENERATED ALWAYS AS ( + UNHEX(SHA2(CONCAT_WS( + '-', `site`, `hook`, `args`, `nextrun`, `schedule` + ), 256)) + ) STORED, PRIMARY KEY (`id`), + UNIQUE INDEX `uniq` (`hash`), KEY `status` (`status`), KEY `site` (`site`), KEY `hook` (`hook`) diff --git a/inc/upgrade/namespace.php b/inc/upgrade/namespace.php index 853e97e..3ff5278 100644 --- a/inc/upgrade/namespace.php +++ b/inc/upgrade/namespace.php @@ -8,6 +8,7 @@ use const HM\Cavalcade\Plugin\DATABASE_VERSION; use HM\Cavalcade\Plugin as Cavalcade; use HM\Cavalcade\Plugin\Job; +use WP_CLI; /** * Update the Cavalcade database version if required. @@ -37,6 +38,10 @@ function upgrade_database() { upgrade_database_4(); } + if ( $database_version < 5 ) { + upgrade_database_5(); + } + update_site_option( 'cavalcade_db_version', DATABASE_VERSION ); Job::flush_query_cache(); @@ -100,3 +105,47 @@ function upgrade_database_4() { $wpdb->query( $query ); } + +/** + * Upgrade Cavalcade database tables to version 5. + * + * Add unique index to prevent duplicate entries being created. + */ +function upgrade_database_5() { + global $wpdb; + + $queries = [ + // Add generated stored hash column. + "ALTER TABLE `{$wpdb->base_prefix}cavalcade_jobs` + ADD COLUMN `hash` BINARY(32) GENERATED ALWAYS AS ( + UNHEX(SHA2(CONCAT_WS( + '-', `site`, `hook`, `args`, `nextrun`, `schedule` + ), 256)) + ) STORED;", + // Remove full group by mode requirement if set. + "SET @sql_mode_tmp = (SELECT @@sql_mode);", + "SET sql_mode = (SELECT REPLACE(@@sql_mode,'ONLY_FULL_GROUP_BY',''));", + // Remove any current duplicates. + "DELETE t1 FROM `{$wpdb->base_prefix}cavalcade_jobs` t1 + INNER JOIN ( + SELECT MIN(`id`) as `id`, `hash` + FROM `{$wpdb->base_prefix}cavalcade_jobs` + GROUP BY `hash` + HAVING COUNT(*) > 1 + ) t2 + ON t1.`hash` = t2.`hash` + AND t1.id != t2.id;", + // Add a unique index on the hash column. + "ALTER TABLE `{$wpdb->base_prefix}cavalcade_jobs` ADD UNIQUE INDEX `uniq` (`hash`);", + // Restore the SQL mode. + "SET sql_mode = (SELECT @sql_mode_tmp);", + ]; + + foreach ( $queries as $query ) { + $wpdb->query( $query ); + + if ( defined( 'WP_CLI' ) && WP_CLI && ! empty( $wpdb->last_error ) ) { + WP_CLI::error( $wpdb->last_error, false ); + } + } +} diff --git a/plugin.php b/plugin.php index 7ebf88c..258f4e6 100644 --- a/plugin.php +++ b/plugin.php @@ -12,7 +12,7 @@ namespace HM\Cavalcade\Plugin; const DATE_FORMAT = 'Y-m-d H:i:s'; -const DATABASE_VERSION = 4; +const DATABASE_VERSION = 5; require __DIR__ . '/inc/namespace.php'; require __DIR__ . '/inc/class-job.php'; From 3b8ad66535668f8b6c4f3e17d53d7af9de6ef0c6 Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Fri, 22 Nov 2024 12:00:07 +0000 Subject: [PATCH 2/3] Add status to the hash column --- inc/namespace.php | 2 +- inc/upgrade/namespace.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/inc/namespace.php b/inc/namespace.php index a2b60d8..5fd9b63 100644 --- a/inc/namespace.php +++ b/inc/namespace.php @@ -92,7 +92,7 @@ function create_tables() { `schedule` varchar(255) DEFAULT NULL, `hash` binary(32) GENERATED ALWAYS AS ( UNHEX(SHA2(CONCAT_WS( - '-', `site`, `hook`, `args`, `nextrun`, `schedule` + '-', `site`, `hook`, `args`, `nextrun`, `status`, `schedule` ), 256)) ) STORED, diff --git a/inc/upgrade/namespace.php b/inc/upgrade/namespace.php index 3ff5278..b2ffedb 100644 --- a/inc/upgrade/namespace.php +++ b/inc/upgrade/namespace.php @@ -119,7 +119,7 @@ function upgrade_database_5() { "ALTER TABLE `{$wpdb->base_prefix}cavalcade_jobs` ADD COLUMN `hash` BINARY(32) GENERATED ALWAYS AS ( UNHEX(SHA2(CONCAT_WS( - '-', `site`, `hook`, `args`, `nextrun`, `schedule` + '-', `site`, `hook`, `args`, `nextrun`, `status`, `schedule` ), 256)) ) STORED;", // Remove full group by mode requirement if set. From 6c124526f6d63bad2183b78c9cef1d09c28da810 Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Fri, 22 Nov 2024 12:06:35 +0000 Subject: [PATCH 3/3] Try handling duplicate error on insert --- inc/class-job.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/inc/class-job.php b/inc/class-job.php index 052d06b..58f259e 100644 --- a/inc/class-job.php +++ b/inc/class-job.php @@ -64,8 +64,14 @@ public function save() { ]; $result = $wpdb->update( $this->get_table(), $data, $where, $this->row_format( $data ), $this->row_format( $where ) ); } else { - $result = $wpdb->replace( $this->get_table(), $data, $this->row_format( $data ) ); - $this->id = $wpdb->insert_id; + $result = $wpdb->insert( $this->get_table(), $data, $this->row_format( $data ) ); + + // Swallow duplicate insert errors. + if ( ! $result && strpos( $wpdb->last_error, '[1062]' ) !== false ) { + $result = true; + } else { + $this->id = $wpdb->insert_id; + } } self::flush_query_cache();