From 50925c5b2678b78e36a5696ea302fd4b1e412110 Mon Sep 17 00:00:00 2001 From: John James Jacoby Date: Tue, 22 Sep 2026 00:08:28 -0500 Subject: [PATCH] Schema: preserve released CREATE TABLE override signature. --- CHANGELOG.md | 6 ++--- src/Database/Kern/Schema.php | 26 ++++++------------- src/Database/Kern/Table.php | 12 +++++++-- src/Database/Traits/Storage/Table/Alter.php | 2 +- .../Kern/Schema/SchemaForeignKeyTest.php | 19 +++++--------- .../Kern/Table/TableForeignKeyTest.php | 23 ++++++++++++++-- 6 files changed, 49 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eda2178d..e06f7733 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,9 +9,9 @@ Notable changes to BerlinDB are documented here. `get_filtered_columns()`, and `get_filtered_indexes()`; each filters the result of its released accessor, with a Query fallback for schemas exposing only `get_columns()`. `get_create_table_string()` uses a private - `get_create_table_array()` builder and accepts an optional foreign-key flag. - Subclasses overriding this 3.0 method must add `bool $with_foreign_keys = false` - to their signature. + `get_create_table_strings()` builder and retains its released zero-argument + signature. `Table::create()` appends enforced foreign keys in inline mode, so + existing Schema overrides remain compatible. Column compatibility is limited to APIs predating 3.0: `is_numeric()` retains its parameterless signature, with explicit type checks on `is_numeric_type()`, and `validate_datetime()`, `validate_decimal()`, and `validate_uuid()` remain diff --git a/src/Database/Kern/Schema.php b/src/Database/Kern/Schema.php index a6aa582e..8aa25a43 100644 --- a/src/Database/Kern/Schema.php +++ b/src/Database/Kern/Schema.php @@ -1620,28 +1620,24 @@ public function remove_index( $name = '' ) { * independently and in no guaranteed order, so a FK inside CREATE TABLE would * reference a table that may not exist yet (and MySQL would reject the whole * create), and two tables could never reference each other. Enforced keys are - * therefore added AFTER the tables exist, via Table::add_foreign_keys(). Pass - * $with_foreign_keys = true only when you control install order and want the - * constraints inline. + * therefore added AFTER the tables exist, via Table::add_foreign_keys(). + * Table::create() appends get_foreign_key_strings() in inline mode. * * @since 3.0.0 - * @since 3.1.0 Added the optional foreign-key flag. * - * @param bool $with_foreign_keys Include enforced foreign keys inline. * @return string SQL body string, or empty string if invalid or empty. */ - public function get_create_table_string( bool $with_foreign_keys = false ) { - return implode( ",\n", $this->get_create_table_array( $with_foreign_keys ) ); + public function get_create_table_string() { + return implode( ",\n", $this->get_create_table_strings() ); } /** * Build the validated CREATE TABLE body fragments. * * @since 3.1.0 - * @param bool $with_foreign_keys Include enforced foreign-key fragments. * @return string[] Non-empty SQL fragments, or an empty array if invalid. */ - private function get_create_table_array( bool $with_foreign_keys = false ): array { + private function get_create_table_strings(): array { // Bail if schema has validation errors. if ( ! $this->is_valid() ) { @@ -1654,11 +1650,6 @@ private function get_create_table_array( bool $with_foreign_keys = false ): arra $this->get_items_create_string( 'indexes' ), ); - // Foreign keys only when the caller controls installation order. - if ( true === $with_foreign_keys ) { - $strings = array_merge( $strings, $this->get_foreign_key_strings() ); - } - return array_values( array_filter( $strings ) ); } @@ -1666,10 +1657,9 @@ private function get_create_table_array( bool $with_foreign_keys = false ): arra * Return the FOREIGN KEY fragments for this schema's enforced relationships. * * One fragment per enforced, owning-side (belongs_to) relationship, each with - * its remote table resolved from the relationship's remote Query class. Shared - * by get_create_table_array() (emitted inside CREATE TABLE) and - * Table::add_foreign_keys() (emitted as ALTER TABLE ADD). Empty when nothing is - * enforced. Non-enforced relationships stay application-level and emit nothing. + * its remote table resolved from the relationship's remote Query class. Used + * by Table::create() in inline mode and Table::add_foreign_keys() in deferred + * mode. Non-enforced relationships emit nothing. * * @since 3.1.0 * diff --git a/src/Database/Kern/Table.php b/src/Database/Kern/Table.php index 91e7b54c..338e00b8 100644 --- a/src/Database/Kern/Table.php +++ b/src/Database/Kern/Table.php @@ -403,14 +403,22 @@ public function create() { * Get the "CREATE TABLE" string. Foreign keys are deferred to * add_foreign_keys() by default; emit them inline only when opted in. */ - $inline_foreign_keys = ( 'inline' === $this->foreign_keys ); - $create_table_string = $this->schema_object->get_create_table_string( $inline_foreign_keys ); + $create_table_string = $this->schema_object->get_create_table_string(); // Bail if no create string. if ( empty( $create_table_string ) ) { return false; } + // Append enforced constraints only when this table opts into inline keys. + if ( ( 'inline' === $this->foreign_keys ) && is_callable( array( $this->schema_object, 'get_foreign_key_strings' ) ) ) { + $foreign_key_strings = $this->schema_object->get_foreign_key_strings(); + + if ( ! empty( $foreign_key_strings ) ) { + $create_table_string .= ",\n" . implode( ",\n", $foreign_key_strings ); + } + } + // Required parts (TEMPORARY when this is a session-scoped table). $sql = array( $this->temporary diff --git a/src/Database/Traits/Storage/Table/Alter.php b/src/Database/Traits/Storage/Table/Alter.php index 00ea1d7f..c0f79a03 100644 --- a/src/Database/Traits/Storage/Table/Alter.php +++ b/src/Database/Traits/Storage/Table/Alter.php @@ -134,7 +134,7 @@ public function replace_index( Index $from, Index $to ) { * Add this schema's enforced foreign keys to the table, via ALTER TABLE. * * The deferred counterpart to emitting foreign keys inside CREATE TABLE (see - * Schema::get_create_table_string( true )): use this when the referenced tables were + * Table::create() with inline foreign keys): use this when referenced tables were * not guaranteed to exist at create time, or for two tables that reference each * other - create both first, then add the keys. Only enforced (enforce => true) * relationships emit anything; a schema with none is a no-op success. Each key diff --git a/tests/Database/Kern/Schema/SchemaForeignKeyTest.php b/tests/Database/Kern/Schema/SchemaForeignKeyTest.php index d70ac10b..c152878f 100644 --- a/tests/Database/Kern/Schema/SchemaForeignKeyTest.php +++ b/tests/Database/Kern/Schema/SchemaForeignKeyTest.php @@ -3,8 +3,8 @@ * Enforced foreign-key DDL emission (#205 / #193 Phase 5). * * An enforced belongs_to relationship now emits a FOREIGN KEY fragment - inside - * CREATE TABLE (Schema::get_create_table_string( true )) and as a reusable list - * (get_foreign_key_strings()), with the remote table resolved from the remote + * CREATE TABLE via Table::create() and as a reusable list + * (Schema::get_foreign_key_strings()), with the remote table resolved from the remote * Query class. These are integration tests: resolving the remote name needs the * remote table registered on $wpdb, which constructing a TestTable does. * @@ -148,21 +148,14 @@ public function test_create_table_string_omits_foreign_keys_by_default() { } /** - * CREATE TABLE includes the enforced foreign key when opted in. + * The released CREATE TABLE method remains a zero-argument extension point. * * @since 3.1.0 */ - public function test_create_table_string_includes_the_foreign_key_when_opted_in() { - $schema = $this->schema_with_relationship( true ); - $sql = $schema->get_create_table_string( true ); + public function test_create_table_string_keeps_its_released_signature() { + $method = new \ReflectionMethod( Schema::class, 'get_create_table_string' ); - $this->assertSame( - $schema->get_create_table_string() . ",\n" . implode( ",\n", $schema->get_foreign_key_strings() ), - $sql - ); - - $this->assertStringContainsString( 'FOREIGN KEY', $sql ); - $this->assertStringContainsString( '`widget_id`', $sql ); + $this->assertSame( 0, $method->getNumberOfParameters() ); } /** diff --git a/tests/Database/Kern/Table/TableForeignKeyTest.php b/tests/Database/Kern/Table/TableForeignKeyTest.php index b2fb3666..5b7962ac 100644 --- a/tests/Database/Kern/Table/TableForeignKeyTest.php +++ b/tests/Database/Kern/Table/TableForeignKeyTest.php @@ -110,6 +110,21 @@ class FkInlineChildTable extends FkChildTable { protected $foreign_keys = 'inline'; } +/** Released Schema override signature, still used by inline table creation. */ +class FkLegacySignatureChildSchema extends FkChildSchema { + public static $called = false; + + public function get_create_table_string() { + self::$called = true; + return parent::get_create_table_string(); + } +} + +/** Inline table that uses a Schema subclass with the released method signature. */ +class FkLegacySignatureInlineChildTable extends FkInlineChildTable { + protected $schema = FkLegacySignatureChildSchema::class; +} + /** * Runtime integration tests for Table::add_foreign_keys(). * @@ -195,8 +210,11 @@ public function test_enforced_foreign_key_is_deferred_then_added() { * @since 3.1.0 */ public function test_inline_creation_emits_foreign_keys(): void { - $table = new FkInlineChildTable(); - $sql = ''; + $table = new FkLegacySignatureInlineChildTable(); + $sql = ''; + + FkLegacySignatureChildSchema::$called = false; + $capture = static function ( $query ) use ( &$sql ) { if ( 0 === strpos( $query, 'CREATE ' ) ) { $sql = $query; @@ -210,6 +228,7 @@ public function test_inline_creation_emits_foreign_keys(): void { } finally { remove_filter( 'query', $capture ); } + $this->assertTrue( FkLegacySignatureChildSchema::$called ); $this->assertStringContainsString( 'FOREIGN KEY', $sql ); $this->assertStringContainsString( '`parent_id`', $sql ); }