From b202c0c4cea2d40fe9c221e7289e855dd50f74d3 Mon Sep 17 00:00:00 2001 From: TallblokeUK Date: Thu, 29 Oct 2026 16:06:03 +0000 Subject: [PATCH 1/2] fix: upgrade a snippets table that missed an upgrade when a save fails on it, and report failed edits --- src/php/Core/DB.php | 25 +++++- src/php/snippet-ops.php | 16 +++- tests/unit/Core/DB_Test.php | 158 ++++++++++++++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 4 deletions(-) create mode 100644 tests/unit/Core/DB_Test.php diff --git a/src/php/Core/DB.php b/src/php/Core/DB.php index 5d90d3e6d..1e403cac6 100644 --- a/src/php/Core/DB.php +++ b/src/php/Core/DB.php @@ -98,13 +98,32 @@ public static function table_exists( string $table_name, bool $refresh = false ) /** * Create the snippet tables, or upgrade them if they already exist + * + * @return bool Whether every table was created or upgraded. */ - public function create_or_upgrade_tables() { + public function create_or_upgrade_tables(): bool { + $success = true; + if ( is_multisite() ) { - $this->create_table( $this->ms_table ); + $success = $this->create_table( $this->ms_table ); } - $this->create_table( $this->table ); + return $this->create_table( $this->table ) && $success; + } + + /** + * Upgrade a table that a write has just failed on because it lacks a column. + * + * A table that missed an upgrade fails every save that writes a column it does not have. + * + * @param string $table_name Table the write failed on. + * + * @return bool Whether the table was upgraded, so the write can be retried. + */ + public static function upgrade_after_unknown_column( string $table_name ): bool { + global $wpdb; + + return false !== stripos( (string) $wpdb->last_error, 'Unknown column' ) && self::create_table( $table_name ); } /** diff --git a/src/php/snippet-ops.php b/src/php/snippet-ops.php index 8f6dc441b..c96da8950 100644 --- a/src/php/snippet-ops.php +++ b/src/php/snippet-ops.php @@ -801,6 +801,11 @@ function save_snippet( $snippet ): ?Snippet { if ( 0 === $snippet->id ) { $data['created_by'] = $author_id; $result = $wpdb->insert( $table, $data, '%s' ); + + if ( false === $result && Core\DB::upgrade_after_unknown_column( $table ) ) { + $result = $wpdb->insert( $table, $data, '%s' ); + } + if ( false === $result ) { return null; } @@ -818,8 +823,17 @@ function save_snippet( $snippet ): ?Snippet { // Otherwise, update the snippet data. $existing = get_snippet( $snippet->id, $snippet->network ); + $result = $wpdb->update( $table, $data, [ 'id' => $snippet->id ], null, [ '%d' ] ); + + if ( false === $result && Core\DB::upgrade_after_unknown_column( $table ) ) { + $result = $wpdb->update( $table, $data, [ 'id' => $snippet->id ], null, [ '%d' ] ); + } + + if ( false === $result ) { + return null; + } + set_snippet_locked( $snippet->id, $snippet->locked, $snippet->network ); - $wpdb->update( $table, $data, [ 'id' => $snippet->id ], null, [ '%d' ] ); // The row has changed, so the cached list no longer describes it. It is // dropped before the snippet is read back, because everything below — diff --git a/tests/unit/Core/DB_Test.php b/tests/unit/Core/DB_Test.php new file mode 100644 index 000000000..7b7a27d98 --- /dev/null +++ b/tests/unit/Core/DB_Test.php @@ -0,0 +1,158 @@ +shadowed ) { + $wpdb->query( 'DROP TEMPORARY TABLE IF EXISTS `' . code_snippets()->db->table . '`' ); + $this->shadowed = false; + } + + remove_all_filters( 'query' ); + parent::tear_down(); + } + + /** + * Stand in a snippets table that missed the upgrade adding the authorship columns. + * + * A temporary table hides the real one of the same name for this connection only. + * + * @return void + */ + private function shadow_table_without_authorship(): void { + global $wpdb; + + $table = code_snippets()->db->table; + + $wpdb->query( + "CREATE TEMPORARY TABLE `$table` ( + id BIGINT(20) NOT NULL AUTO_INCREMENT, + name TINYTEXT NOT NULL, + description TEXT NOT NULL, + code LONGTEXT NOT NULL, + tags LONGTEXT NOT NULL, + scope VARCHAR(15) NOT NULL DEFAULT 'global', + condition_id BIGINT(20) NOT NULL DEFAULT 0, + priority SMALLINT NOT NULL DEFAULT 10, + active TINYINT(1) NOT NULL DEFAULT 0, + modified DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + revision BIGINT(20) NOT NULL DEFAULT 1, + cloud_id VARCHAR(255) NULL, + PRIMARY KEY (id) + )" + ); + + $this->shadowed = true; + DB::table_exists( $table, true ); + } + + /** + * The names of the columns the current snippets table has. + * + * @return string[] + */ + private function columns(): array { + global $wpdb; + + return $wpdb->get_col( 'SHOW COLUMNS FROM `' . code_snippets()->db->table . '`' ); + } + + /** + * A table that missed the authorship upgrade is upgraded by the first save, which then succeeds. + * + * @return void + */ + public function test_saving_upgrades_a_table_missing_a_column(): void { + $this->shadow_table_without_authorship(); + $this->assertNotContains( 'updated_by', $this->columns() ); + + $suppressed = $GLOBALS['wpdb']->suppress_errors(); + $created = save_snippet( + new Snippet( + [ + 'name' => 'Created on an old table', + 'code' => '// one', + ] + ) + ); + $GLOBALS['wpdb']->suppress_errors( $suppressed ); + + $this->assertNotNull( $created ); + $this->assertGreaterThan( 0, $created->id ); + $this->assertContains( 'created_by', $this->columns() ); + $this->assertContains( 'updated_by', $this->columns() ); + + $created->code = '// two'; + $this->assertNotNull( save_snippet( $created ) ); + $this->assertSame( '// two', get_snippet( $created->id )->code ); + } + + /** + * An edit that cannot be written is reported as failed rather than returning the unchanged snippet. + * + * @return void + */ + public function test_failed_update_returns_null(): void { + $snippet = save_snippet( + new Snippet( + [ + 'name' => 'Cannot be updated', + 'code' => '// before', + ] + ) + ); + $table = code_snippets()->db->table; + + add_filter( + 'query', + static fn( string $query ): string => 0 === strpos( $query, "UPDATE `$table`" ) ? 'UPDATE `cs_missing_table` SET id = 1' : $query + ); + + $snippet->code = '// after'; + + $suppressed = $GLOBALS['wpdb']->suppress_errors(); + $saved = save_snippet( $snippet ); + $GLOBALS['wpdb']->suppress_errors( $suppressed ); + + $this->assertNull( $saved ); + + remove_all_filters( 'query' ); + $this->assertSame( '// before', get_snippet( $snippet->id )->code ); + } + + /** + * Upgrading the tables reports success when they match the current schema. + * + * @return void + */ + public function test_create_or_upgrade_tables_reports_success(): void { + $this->assertTrue( code_snippets()->db->create_or_upgrade_tables() ); + } +} From 4e28e45d16a6711d26c103198136a80ab5db5945 Mon Sep 17 00:00:00 2001 From: TallblokeUK Date: Thu, 29 Oct 2026 16:06:13 +0000 Subject: [PATCH 2/2] fix: report a failed database table upgrade instead of always reporting success --- src/php/Settings/settings.php | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/php/Settings/settings.php b/src/php/Settings/settings.php index 069d1f9eb..423bfd53d 100644 --- a/src/php/Settings/settings.php +++ b/src/php/Settings/settings.php @@ -377,14 +377,26 @@ function process_settings_actions( array $input ): ?array { } if ( isset( $input['debug']['database_update'] ) ) { - code_snippets()->db->create_or_upgrade_tables(); - - add_settings_error( - OPTION_NAME, - 'database_update_done', - __( 'Successfully performed database table upgrade.', 'code-snippets' ), - 'updated' - ); + if ( code_snippets()->db->create_or_upgrade_tables() ) { + add_settings_error( + OPTION_NAME, + 'database_update_done', + __( 'Successfully performed database table upgrade.', 'code-snippets' ), + 'updated' + ); + } else { + global $wpdb; + + add_settings_error( + OPTION_NAME, + 'database_update_failed', + sprintf( + /* translators: %s: database error message */ + __( 'The database table upgrade did not complete: %s', 'code-snippets' ), + $wpdb->last_error + ) + ); + } } if ( isset( $input['debug']['reset_caches'] ) ) {