From 3041ef16a199673db24812bd582ac575d7c3902a Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 10:32:07 +0600 Subject: [PATCH 01/32] fix: change() emits MODIFY COLUMN, not ADD COLUMN CHANGE COLUMN Blueprint::addColumnQuery() unconditionally prepended ADD COLUMN in edit mode and then also prepended CHANGE COLUMN when the change flag was set, producing invalid SQL. Make the two prefixes mutually exclusive: a changed column now emits MODIFY COLUMN; non-changed columns in edit mode still emit ADD COLUMN. Add SchemaTest with a failing-first test for both paths. Add has_cap() stub to FakeWpdb so Schema tests can boot. Remove the change() Known bug note and Limitations bullets from docs/schema.md and docs/usage.md. Assisted-By: AI --- docs/schema.md | 14 ++----------- docs/usage.md | 4 ---- src/Blueprint.php | 10 ++++----- src/Schema.php | 2 +- tests/SchemaTest.php | 49 ++++++++++++++++++++++++++++++++++++++++++++ tests/bootstrap.php | 30 +++++++++++++++++---------- 6 files changed, 75 insertions(+), 34 deletions(-) create mode 100644 tests/SchemaTest.php diff --git a/docs/schema.md b/docs/schema.md index 3708a4b..ff2581a 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -256,20 +256,15 @@ Schema::edit('orders', function ($table) { ### Modifying an existing column -Chain `change()` on a column definition inside `Schema::edit` to emit `CHANGE COLUMN` +Chain `change()` on a column definition inside `Schema::edit` to emit `MODIFY COLUMN` instead of `ADD COLUMN`: ```php Schema::edit('orders', function ($table) { - $table->varchar('reference', 128)->change(); // CHANGE COLUMN — widen the length + $table->varchar('reference', 128)->change(); // MODIFY COLUMN — widen the length }); ``` -> ⚠️ **Known bug:** `change()` currently emits malformed SQL (`ADD COLUMN CHANGE COLUMN …`) -> because `addColumnQuery()` unconditionally prepends `ADD COLUMN` in edit mode and then -> also prepends `CHANGE COLUMN` when the `change` flag is set. Do not rely on `change()` -> in production until this is fixed. - ### Drop helpers Only `dropColumn` and `renameColumn` produce complete SQL when called as a direct static @@ -335,11 +330,6 @@ Schema::withPrefix('custom_')->create('orders', function ($table) { To use the WordPress prefix, pass it explicitly: `Schema::withPrefix($wpdb->prefix)->create(...)`. -- **`change()` is broken — emits malformed SQL.** `addColumnQuery()` prepends - `ADD COLUMN` for every column in edit mode, then also prepends `CHANGE COLUMN` when the - `change` flag is set, producing `ADD COLUMN CHANGE COLUMN …`. Avoid `change()` in - production until this is fixed. - - **`dropTimestamps()`, `dropIndex()`, `dropUnique()`, `dropForeign()`, `dropPrimary()` must be used inside a `Schema::edit()` callback.** A direct static call (e.g. `Schema::dropTimestamps('orders')`) only sets the `ALTER TABLE` header and emits no diff --git a/docs/usage.md b/docs/usage.md index 8a8b408..cf46489 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -717,7 +717,3 @@ method relocation. defaults to `null`, not `''`; table names are used as-is unless you call `Schema::withPrefix()` explicitly. See [Schema builder reference](schema.md). -- **`change()` column modifier is broken: emits malformed `ADD COLUMN CHANGE COLUMN …` - SQL.** The `addColumnQuery()` method prepends `ADD COLUMN` in edit mode and also - prepends `CHANGE COLUMN` when the `change` flag is set, producing invalid SQL. - Avoid `change()` in production until this is fixed. diff --git a/src/Blueprint.php b/src/Blueprint.php index 1be96bf..4e0ea45 100644 --- a/src/Blueprint.php +++ b/src/Blueprint.php @@ -99,7 +99,7 @@ class Blueprint * @param string $prefix Table prefix * @param null|Closure $callback Closure to build the blueprint */ - public function __construct($table, $method, $prefix = '', Closure $callback = null) + public function __construct($table, $method, $prefix = '', ?Closure $callback = null) { $this->_prefix = $prefix; $this->table = "{$prefix}{$table}"; @@ -639,12 +639,10 @@ private function addColumnQuery() $query .= "\n, "; } - if ($this->method === 'edit') { - $query .= 'ADD COLUMN '; - } - if (isset($column['change'])) { - $query .= 'CHANGE COLUMN '; + $query .= 'MODIFY COLUMN '; + } elseif ($this->method === 'edit') { + $query .= 'ADD COLUMN '; } $query .= $column['name'] . ' ' . $column['type']; diff --git a/src/Schema.php b/src/Schema.php index 5e5aff0..ecc9409 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -46,7 +46,7 @@ public function __call($method, $parameters) return $this->build($blueprint); } - public function createBlueprint($schema, $method, Closure $callback = null) + public function createBlueprint($schema, $method, ?Closure $callback = null) { return new Blueprint( $schema, diff --git a/tests/SchemaTest.php b/tests/SchemaTest.php new file mode 100644 index 0000000..3437c1d --- /dev/null +++ b/tests/SchemaTest.php @@ -0,0 +1,49 @@ +varchar('reference', 128)->change(); + }); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('MODIFY COLUMN reference VARCHAR(128)', $sql); + $this->assertStringNotContainsString('ADD COLUMN', $sql); + } + + public function testNonChangedEditEmitsAddColumn(): void + { + Schema::edit('orders', function ($table) { + $table->varchar('note', 64); + }); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('ADD COLUMN note VARCHAR(64)', $sql); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 8c1236f..f7e202c 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -8,35 +8,34 @@ * so `Connection` can resolve a prefix and capture executed SQL without a real * database. */ - error_reporting(E_ALL & ~E_DEPRECATED); -if (!\defined('ABSPATH')) { - \define('ABSPATH', __DIR__ . '/'); +if (!defined('ABSPATH')) { + define('ABSPATH', __DIR__ . '/'); } -if (!\function_exists('esc_html')) { +if (!function_exists('esc_html')) { function esc_html($text) { return $text; } } -if (!\function_exists('wp_json_encode')) { +if (!function_exists('wp_json_encode')) { function wp_json_encode($data, $options = 0, $depth = 512) { return json_encode($data, $options, $depth); } } -if (!\function_exists('get_option')) { +if (!function_exists('get_option')) { function get_option($name, $default = false) { return $default; } } -if (!\function_exists('wp_timezone_string')) { +if (!function_exists('wp_timezone_string')) { function wp_timezone_string() { return 'UTC'; @@ -63,10 +62,14 @@ class FakeWpdb public $suppress_errors = false; - /** @var string[] */ + /** + * @var string[] + */ public $queries = []; - /** @var null|callable resolves a result set from the SQL string */ + /** + * @var null|callable resolves a result set from the SQL string + */ public $resolver; public function queueResult(array $rows) @@ -79,7 +82,7 @@ public function query($sql) $this->last_query = $sql; $this->queries[] = $sql; - if (\is_callable($this->resolver)) { + if (is_callable($this->resolver)) { $this->last_result = ($this->resolver)($sql); } @@ -88,7 +91,7 @@ public function query($sql) public function prepare($query, ...$args) { - if (\count($args) === 1 && \is_array($args[0])) { + if (count($args) === 1 && is_array($args[0])) { $args = $args[0]; } @@ -110,6 +113,11 @@ public function get_results($query) { return $this->last_result; } + + public function has_cap($cap) + { + return false; + } } $GLOBALS['wpdb'] = new FakeWpdb(); From 91845399f363a0bbf7b574b6a32494d93c6ca71a Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 10:39:25 +0600 Subject: [PATCH 02/32] fix: direct-call schema drop helpers emit full SQL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract DROP-clause builders and a shared applyDropClause() dispatcher so dropPrimary, dropTimestamps, dropIndex, dropUnique, and dropForeign emit a complete ALTER TABLE … DROP … statement when called directly (e.g. Schema::dropPrimary('orders')) instead of an empty header. Edit-mode aggregation via Schema::edit() is unchanged. Remove the now- invalid Limitations docs that documented these as edit-only helpers. Assisted-By: AI --- docs/schema.md | 12 ------ src/Blueprint.php | 99 ++++++++++++++++++++------------------------ tests/SchemaTest.php | 72 ++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 67 deletions(-) diff --git a/docs/schema.md b/docs/schema.md index ff2581a..d624073 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -267,12 +267,6 @@ Schema::edit('orders', function ($table) { ### Drop helpers -Only `dropColumn` and `renameColumn` produce complete SQL when called as a direct static -call (e.g. `Schema::dropColumn('orders', 'legacy_notes')`). The remaining helpers — -`dropTimestamps`, `dropIndex`, `dropUnique`, `dropForeign`, and `dropPrimary` — must be -used inside a `Schema::edit()` callback; a direct static call only emits the -`ALTER TABLE` header with no `DROP` clause. - | Method | Emitted SQL | |---|---| | `dropColumn($column)` | `DROP $column` | @@ -329,9 +323,3 @@ Schema::withPrefix('custom_')->create('orders', function ($table) { `null`, not `''`; no prefix is prepended unless you call `Schema::withPrefix()` first. To use the WordPress prefix, pass it explicitly: `Schema::withPrefix($wpdb->prefix)->create(...)`. - -- **`dropTimestamps()`, `dropIndex()`, `dropUnique()`, `dropForeign()`, `dropPrimary()` - must be used inside a `Schema::edit()` callback.** A direct static call (e.g. - `Schema::dropTimestamps('orders')`) only sets the `ALTER TABLE` header and emits no - `DROP` clause, producing a syntax error. Only `dropColumn()` and `renameColumn()` - produce complete SQL when called directly. diff --git a/src/Blueprint.php b/src/Blueprint.php index 4e0ea45..3186b77 100644 --- a/src/Blueprint.php +++ b/src/Blueprint.php @@ -499,83 +499,35 @@ public function cascade() public function dropForeign($keys) { - if ($this->method === 'dropForeign') { - $this->_sql = "ALTER TABLE `{$this->table}`"; - } else { - $sql = ''; - $idCount = \count($keys) - 1; - $i = 0; - if (\is_array($keys)) { - foreach ($keys as $key) { - if ($i == $idCount) { - $sql .= " DROP FOREIGN KEY `{$key}`"; - } else { - $sql .= " DROP FOREIGN KEY `{$key}`,"; - } - - $i++; - } - } else { - $sql .= " DROP FOREIGN KEY `{$keys}`"; - } - - $this->_edit['dropForeign'] = $sql; - } + $this->applyDropClause('dropForeign', 'dropForeign', $this->buildDropForeignClause($keys)); return $this; } public function dropIndex($indexes) { - if ($this->method === 'dropIndex') { - $this->_sql = "ALTER TABLE `{$this->table}`"; - } else { - $sql = ''; - $idCount = \count($indexes) - 1; - $i = 0; - if (\is_array($indexes)) { - foreach ($indexes as $index) { - if ($i == $idCount) { - $sql .= " DROP INDEX `{$index}`"; - } else { - $sql .= " DROP INDEX `{$index}`,"; - } - - $i++; - } - } else { - $sql .= " DROP INDEX `{$indexes}`"; - } - - $this->_edit['dropIndex'] = $sql; - } + $this->applyDropClause('dropIndex', 'dropIndex', $this->buildDropIndexClause($indexes)); return $this; } public function dropPrimary() { - if ($this->method === 'dropPrimary') { - $this->_sql = "ALTER TABLE `{$this->table}`"; - } else { - $this->_edit['dropPrimary'] = ' DROP PRIMARY KEY'; - } + $this->applyDropClause('dropPrimary', 'dropPrimary', ' DROP PRIMARY KEY'); return $this; } public function dropUnique($indexes) { - return $this->dropIndex($indexes); + $this->applyDropClause('dropUnique', 'dropIndex', $this->buildDropIndexClause($indexes)); + + return $this; } public function dropTimestamps() { - if ($this->method === 'dropTimestamps') { - $this->_sql = "ALTER TABLE `{$this->table}`"; - } else { - $this->_edit['dropTimestamps'] = ' DROP COLUMN created_at, DROP COLUMN updated_at'; - } + $this->applyDropClause('dropTimestamps', 'dropTimestamps', ' DROP COLUMN created_at, DROP COLUMN updated_at'); return $this; } @@ -596,6 +548,43 @@ public function length($length) return $this; } + private function buildDropIndexClause($indexes) + { + if (\is_array($indexes)) { + $clauses = []; + foreach ($indexes as $index) { + $clauses[] = " DROP INDEX `{$index}`"; + } + + return implode(',', $clauses); + } + + return " DROP INDEX `{$indexes}`"; + } + + private function buildDropForeignClause($keys) + { + if (\is_array($keys)) { + $clauses = []; + foreach ($keys as $key) { + $clauses[] = " DROP FOREIGN KEY `{$key}`"; + } + + return implode(',', $clauses); + } + + return " DROP FOREIGN KEY `{$keys}`"; + } + + private function applyDropClause($directMethod, $editKey, $clause) + { + if ($this->method === $directMethod) { + $this->_sql = "ALTER TABLE `{$this->table}`" . $clause; + } else { + $this->_edit[$editKey] = $clause; + } + } + private function getCollation() { $collate = null; diff --git a/tests/SchemaTest.php b/tests/SchemaTest.php index 3437c1d..f3a8096 100644 --- a/tests/SchemaTest.php +++ b/tests/SchemaTest.php @@ -11,6 +11,7 @@ * * These tests operate on the literal table name — no prefix is applied because * Connection::setPluginPrefix is intentionally not called here. + * */ final class SchemaTest extends TestCase { @@ -46,4 +47,75 @@ public function testNonChangedEditEmitsAddColumn(): void $this->assertStringContainsString('ADD COLUMN note VARCHAR(64)', $sql); } + + public function testDropPrimaryDirectCallEmitsFullSql(): void + { + Schema::dropPrimary('orders'); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('ALTER TABLE `orders`', $sql); + $this->assertStringContainsString('DROP PRIMARY KEY', $sql); + } + + public function testDropTimestampsDirectCallEmitsFullSql(): void + { + Schema::dropTimestamps('orders'); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('ALTER TABLE `orders`', $sql); + $this->assertStringContainsString('DROP COLUMN created_at, DROP COLUMN updated_at', $sql); + } + + public function testDropIndexDirectCallSingleNameEmitsFullSql(): void + { + Schema::dropIndex('orders', 'email_INDEX'); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('ALTER TABLE `orders`', $sql); + $this->assertStringContainsString('DROP INDEX `email_INDEX`', $sql); + } + + public function testDropIndexDirectCallArrayNamesEmitsFullSql(): void + { + Schema::dropIndex('orders', ['a', 'b']); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('DROP INDEX `a`', $sql); + $this->assertStringContainsString('DROP INDEX `b`', $sql); + } + + public function testDropUniqueDirectCallEmitsDropIndexSql(): void + { + Schema::dropUnique('orders', 'email_UNIQUE'); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('ALTER TABLE `orders`', $sql); + $this->assertStringContainsString('DROP INDEX `email_UNIQUE`', $sql); + } + + public function testDropForeignDirectCallEmitsFullSql(): void + { + Schema::dropForeign('orders', 'fk_user'); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('ALTER TABLE `orders`', $sql); + $this->assertStringContainsString('DROP FOREIGN KEY `fk_user`', $sql); + } + + public function testDropPrimaryEditModeStillWorks(): void + { + Schema::edit('orders', function ($table) { + $table->dropPrimary(); + }); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('DROP PRIMARY KEY', $sql); + } } From b37aaa7002057d7684986e17fbd4d9b5c14ed97f Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 10:44:59 +0600 Subject: [PATCH 03/32] feat: subscribable creating/created model events Assisted-By: AI --- docs/usage.md | 12 +-------- src/Concerns/HasEvents.php | 10 +++++++ tests/CreatingCreatedEventTest.php | 43 ++++++++++++++++++++++++++++++ tests/Fixtures/CreatingUser.php | 37 +++++++++++++++++++++++++ tests/bootstrap.php | 1 + 5 files changed, 92 insertions(+), 11 deletions(-) create mode 100644 tests/CreatingCreatedEventTest.php create mode 100644 tests/Fixtures/CreatingUser.php diff --git a/docs/usage.md b/docs/usage.md index cf46489..7285e43 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -565,17 +565,13 @@ class Contact extends Model ``` **Subscribable events** (registered via Closure using the methods below): -`retrieved`, `saving`, `saved`, `updating`, `updated`, `deleting`, `deleted`. +`retrieved`, `creating`, `created`, `saving`, `saved`, `updating`, `updated`, `deleting`, `deleted`. **Boot hooks** — override these protected static methods instead of registering a Closure: `booting()` (runs before `boot()`), `booted()` (runs after `boot()`). The `boot()` method itself is the standard entry point for registering event handlers. -> `creating`/`created` events fire internally but have no public registrar — -> they cannot be subscribed via `static::creating(...)` / `static::created(...)`. -> Use `saving`/`saved` instead, which fire on both insert and update. - > The `HasEvents` trait reserves the method names `boot`, `booting`, `booted`, > `fireEvent`, `fireCustomEvent`, `registerEvent` and the properties `$events`, > `$registeredEvents`, `$booted`. @@ -700,12 +696,6 @@ method relocation. timestamp may be wrong on update. Workaround: use separate `insert` + `update` calls where portability or correct timestamps are required. -- **`creating`/`created` events cannot be subscribed.** These event names fire - internally during `insert()` but `HasEvents` provides no registrar methods for - them. Calling `static::creating(fn ...)` or `static::created(fn ...)` in - `boot()` will throw a fatal error. Use `saving`/`saved` instead — they fire - on both insert and update. - - **Bulk `insert()` may return a bare array, not a `Collection`.** When the post-insert re-query that hydrates the inserted rows fails, the fallback path returns a plain PHP array of IDs rather than a `Collection`. Code that calls diff --git a/src/Concerns/HasEvents.php b/src/Concerns/HasEvents.php index 80cdf52..4594ae9 100644 --- a/src/Concerns/HasEvents.php +++ b/src/Concerns/HasEvents.php @@ -50,6 +50,16 @@ protected static function retrieved(Closure $callback) static::registerEvent('retrieved', $callback); } + protected static function creating(Closure $callback) + { + static::registerEvent('creating', $callback); + } + + protected static function created(Closure $callback) + { + static::registerEvent('created', $callback); + } + protected static function saving(Closure $callback) { static::registerEvent('saving', $callback); diff --git a/tests/CreatingCreatedEventTest.php b/tests/CreatingCreatedEventTest.php new file mode 100644 index 0000000..a3dd325 --- /dev/null +++ b/tests/CreatingCreatedEventTest.php @@ -0,0 +1,43 @@ +insert_id = 1; + + CreatingUser::insert(['name' => 'Ada']); + + $this->assertTrue(CreatingUser::$creatingCalled, 'creating handler should have run'); + $this->assertTrue(CreatingUser::$createdCalled, 'created handler should have run'); + } + + public function testCreatingReturningFalseAbortsInsert(): void + { + CreatingUser::$abortCreating = true; + + $result = CreatingUser::insert(['name' => 'Ada']); + + $this->assertSame([], $GLOBALS['wpdb']->queries, 'aborted insert must not execute any query'); + $this->assertFalse($result, 'aborted insert returns false'); + } +} diff --git a/tests/Fixtures/CreatingUser.php b/tests/Fixtures/CreatingUser.php new file mode 100644 index 0000000..b8ed830 --- /dev/null +++ b/tests/Fixtures/CreatingUser.php @@ -0,0 +1,37 @@ + Date: Tue, 30 Jun 2026 10:54:37 +0600 Subject: [PATCH 04/32] fix: bulk insert always returns a Collection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fallback path in QueryBuilder::bulkInsert() — triggered when the post-insert re-query hydrates nothing — returned a bare int[] instead of a Collection, breaking the documented contract. Wrap $ids in new Collection($ids) so callers receive a consistent Collection regardless of path. Element-type caveat: Collection elements are Model instances on the happy path (re-query succeeded) and ints (inserted IDs) on the fallback. foreach/count() are unaffected; is_array()/empty() now return false on the fallback. Docs: remove the Limitations bullet that documented the bug; update the bulk-insert code comment to note the element-type distinction. Assisted-By: AI --- docs/usage.md | 11 ++----- src/QueryBuilder.php | 2 +- tests/BulkInsertReturnTest.php | 53 ++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 9 deletions(-) create mode 100644 tests/BulkInsertReturnTest.php diff --git a/docs/usage.md b/docs/usage.md index 7285e43..59d065d 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -142,7 +142,9 @@ $contact = Contact::insert([ 'email' => 'ada@example.com', ]); -// Bulk insert (array of rows) — returns a Collection of created models +// Bulk insert (array of rows) — always returns a Collection. +// Element type: Model on success (re-query hydrated); int (inserted ID) on +// the rare fallback where the post-insert re-query yields no rows. $created = Contact::insert([ ['first_name' => 'Ada', 'email' => 'ada@x.com'], ['first_name' => 'Grace','email' => 'grace@x.com'], @@ -696,13 +698,6 @@ method relocation. timestamp may be wrong on update. Workaround: use separate `insert` + `update` calls where portability or correct timestamps are required. -- **Bulk `insert()` may return a bare array, not a `Collection`.** When the - post-insert re-query that hydrates the inserted rows fails, the fallback path - returns a plain PHP array of IDs rather than a `Collection`. Code that calls - Collection methods on the return value of `insert()` will break in that case. - Workaround: check `is_array()` on the result or use `Collection::make()` to - wrap it defensively. - - **Schema builder does not auto-apply the table prefix.** `Schema::$prefix` defaults to `null`, not `''`; table names are used as-is unless you call `Schema::withPrefix()` explicitly. See [Schema builder reference](schema.md). diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index 43fde14..093ab9c 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -1505,7 +1505,7 @@ function ($value) { return $allRows; } - return $ids; + return new Collection($ids); } return false; diff --git a/tests/BulkInsertReturnTest.php b/tests/BulkInsertReturnTest.php new file mode 100644 index 0000000..b900f48 --- /dev/null +++ b/tests/BulkInsertReturnTest.php @@ -0,0 +1,53 @@ +rows_affected = 2; + $GLOBALS['wpdb']->insert_id = 10; + // last_result defaults to [] — get() returns [], triggering the fallback path. + + $result = User::query()->insert([['name' => 'a'], ['name' => 'b']]); + + $this->assertInstanceOf(Collection::class, $result); + } + + public function testBulkInsertHappyPathReturnsCollection(): void + { + // Happy path: INSERT succeeds and re-query hydrates the rows into Models. + $GLOBALS['wpdb']->rows_affected = 2; + $GLOBALS['wpdb']->insert_id = 10; + $GLOBALS['wpdb']->queueResult([ + (object) ['id' => 10, 'name' => 'a'], + (object) ['id' => 11, 'name' => 'b'], + ]); + + $result = User::query()->insert([['name' => 'a'], ['name' => 'b']]); + + $this->assertInstanceOf(Collection::class, $result); + } +} From 3e5db3ab606534b69ba91a013446eae661afc552 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 10:59:02 +0600 Subject: [PATCH 05/32] fix: joins no longer double-prefix and qualify ON column Assisted-By: AI --- docs/usage.md | 14 -------------- src/QueryBuilder.php | 30 ++++++++++++++++-------------- tests/Query/GrammarTest.php | 15 ++++++++++++++- 3 files changed, 30 insertions(+), 29 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index 59d065d..cd11c5b 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -268,12 +268,6 @@ Contact::query() // also: rightJoin(), fullJoin(), crossJoin(), on(), orOn() ``` -> **Joins are currently unreliable.** The join implementation has known bugs: -> the joined table name is double-prefixed (`wp_wp_*`), ON-clause columns are not -> adjusted when an alias is present, and `prepareOn` reuses the mutated column -> value for the second-column lookup. Use raw SQL (`whereRaw` / `raw()`) until -> these are fixed. See [Limitations](#limitations--known-issues). - ### Limit / offset / pagination ```php @@ -668,14 +662,6 @@ method relocation. ## Limitations & known issues -- **Joins are broadly unreliable.** `join()` in `QueryBuilder` always prepends - `Connection::wpPrefix()` (plus the model prefix) onto the supplied table name, - causing a double prefix (`wp_wp_*`) unconditionally — not only when a plugin - prefix is set. Separately, `prepareOn()` reuses the mutated column value for the - second-column lookup, and ON-clause columns are not adjusted when an alias is - present. Workaround: write raw JOIN clauses via `raw()` and apply your own - prefix via `Connection::getPrefix()`. - - **`belongsToMany` is declared but non-functional.** The method exists but contains no pivot-table join logic. Calling it does not produce a many-to-many query through an intermediate table. Workaround: model the pivot diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index 093ab9c..f31cdae 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -705,18 +705,17 @@ public function orHaving(...$params) */ public function join($table, $firstColumn, $operator = null, $secondColumn = null, $type = 'INNER') { - $table = Connection::wpPrefix() . $this->_model->getPrefix() . $table; - $hasAlias = preg_split('/ as /i', $table); - if ($hasAlias && isset($hasAlias[1])) { - $table = $hasAlias[0]; - $alias = $hasAlias[1]; - } else { - $alias = $table; - } - - $on[] = $this->prepareOn($alias, $firstColumn, $operator, $secondColumn, 'AND'); + $parts = preg_split('/ as /i', $table); + $rawTable = $parts[0]; + $alias = isset($parts[1]) ? $parts[1] : null; + $prefixedTable = $this->_model->getPrefix() . $rawTable; + $reference = $alias !== null ? $alias : $prefixedTable; + $tableSql = $alias !== null ? $prefixedTable . ' as ' . $alias : $prefixedTable; + + $on[] = $this->prepareOn($reference, $firstColumn, $operator, $secondColumn, 'AND'); $this->joins[] = [ - 'table' => $table, + 'table' => $tableSql, + 'alias' => $reference, 'on' => $on, 'type' => $type, ]; @@ -801,7 +800,7 @@ public function on($firstColumn, $operator = null, $secondColumn = null, $bool = $joinIndex = 0; } - $table = $this->joins[$joinIndex]['table']; + $table = $this->joins[$joinIndex]['alias']; $this->joins[$joinIndex]['on'][] = $this->prepareOn($table, $firstColumn, $operator, $secondColumn, $bool); return $this; @@ -1393,11 +1392,14 @@ protected function prepareHaving($params, $bool = 'AND') protected function prepareOn($table, $column, $operator, $secondColumn, $bool = 'AND') { if (\is_null($operator) && \is_null($secondColumn)) { - $column = $this->_model->getTable() . '.' . $column; - $secondColumn = $table . '.' . $column; + $secondColumn = $column; $operator = '='; } + if (!\is_null($secondColumn) && strpos($secondColumn, '.') === false) { + $secondColumn = $table . '.' . $secondColumn; + } + return compact('column', 'operator', 'secondColumn', 'bool'); } diff --git a/tests/Query/GrammarTest.php b/tests/Query/GrammarTest.php index dc34f38..f72c60d 100644 --- a/tests/Query/GrammarTest.php +++ b/tests/Query/GrammarTest.php @@ -72,11 +72,24 @@ public function testNestedClosureWhereProducesParenthesizedGroup(): void public function testJoin(): void { $this->assertSame( - 'SELECT FROM wp_users INNER JOIN wp_wp_posts ON `wp_users`.`user_id` = id', + 'SELECT FROM wp_users INNER JOIN wp_posts ON `wp_users`.`user_id` = wp_posts.id', (new User())->join('posts', 'user_id', '=', 'id')->toSql() ); } + public function testJoinWithAlias(): void + { + $sql = (new User())->join('posts as p', 'user_id', '=', 'id')->toSql(); + $this->assertStringContainsString('INNER JOIN wp_posts as p ON', $sql); + $this->assertStringContainsString('= p.id', $sql); + } + + public function testJoinWithDottedColumnsUntouched(): void + { + $sql = (new User())->join('posts', 'posts.user_id', '=', 'users.id')->toSql(); + $this->assertStringContainsString('= users.id', $sql); + } + public function testGroupBy(): void { $this->assertSame( From 0e4b29afc2b33d066ce93750d3d3d458d3a5ce95 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 11:03:53 +0600 Subject: [PATCH 06/32] feat: add Schema::withWpPrefix() opt-in for WP-prefixed tables Adds a real static method Schema::withWpPrefix() that sets the Schema prefix to Connection::getPrefix() (wp_ + plugin prefix), giving authors a zero-BC way to create tables that match Model table names. Default behaviour (bare table name) is unchanged: Schema::create('orders') still emits CREATE TABLE IF NOT EXISTS orders. The BC guard test locks this. Docs: reframe the prefix Limitations bullet in usage.md and schema.md from "does not auto-apply" to "by design bare", and document the new method in the Table operations table with a code example. Assisted-By: AI --- docs/schema.md | 22 ++++++++++++++++------ docs/usage.md | 15 ++++++++++----- src/Schema.php | 8 ++++++++ tests/SchemaTest.php | 37 ++++++++++++++++++++++++++++++++++++- 4 files changed, 70 insertions(+), 12 deletions(-) diff --git a/docs/schema.md b/docs/schema.md index d624073..6d0c477 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -304,22 +304,32 @@ on `(new Schema())->create(...)` — both are equivalent thanks to `__callStatic | `Schema::drop($table)` | `DROP TABLE IF EXISTS`. | | `Schema::rename($table, $newName)` | `ALTER TABLE … RENAME TO`. Both names are used as-is unless `Schema::withPrefix()` is used (no prefix by default). | | `Schema::withPrefix($prefix)` | Sets the prefix applied for this call (there is no prefix by default). Returns a `Schema` instance; chain `.create()`, `.edit()`, etc. | +| `Schema::withWpPrefix()` | Sets the prefix to `Connection::getPrefix()` (WordPress prefix + plugin prefix), so the table matches what `Model`s use. Returns a `Schema` instance; chain `.create()`, `.edit()`, etc. | ```php -// Override prefix — table resolves as "custom_orders" -// (without withPrefix, the bare name "orders" is used — no prefix is applied automatically) +// Literal prefix — table resolves as "custom_orders" +// (without withPrefix/withWpPrefix, the bare name "orders" is used — no prefix is applied) Schema::withPrefix('custom_')->create('orders', function ($table) { $table->id(); $table->string('reference'); $table->timestamps(); }); + +// Match the Model prefix (e.g. wp_ or wp_myplugin_) — table resolves as "wp_orders" +Schema::withWpPrefix()->create('orders', function ($table) { + $table->id(); + $table->string('reference'); + $table->timestamps(); +}); ``` --- ## Limitations & known issues -- **Schema builder does not auto-apply the table prefix.** `Schema::$prefix` defaults to - `null`, not `''`; no prefix is prepended unless you call `Schema::withPrefix()` first. - To use the WordPress prefix, pass it explicitly: - `Schema::withPrefix($wpdb->prefix)->create(...)`. +- **Schema builder uses the literal table name by design.** `Schema::create()` passes the + name through as-is — no WordPress prefix is added automatically, so existing tables are + never relocated. Use `Schema::withPrefix('your_prefix_')` for a literal prefix, or + `Schema::withWpPrefix()` to match the prefix `Model`s use (`Connection::getPrefix()`). + The `$prefix` property intentionally defaults to `null` (not `''`) to preserve + bare-table behaviour for plugins that rely on it. diff --git a/docs/usage.md b/docs/usage.md index cd11c5b..3515777 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -611,8 +611,9 @@ Placeholders use `$wpdb` conventions (`%d`, `%s`, `%f`) with the bindings array. ## Schema builder Use `Schema` (a facade over `Blueprint`) to create, alter and drop tables. By -default the table name is used as-is — call `Schema::withPrefix($prefix)` to -apply a prefix. +design the table name is used as-is — call `Schema::withPrefix($prefix)` to +apply a literal prefix, or `Schema::withWpPrefix()` to use the same prefix +`Model`s use (`Connection::getPrefix()`). ```php use BitApps\WPDatabase\Schema; @@ -684,7 +685,11 @@ method relocation. timestamp may be wrong on update. Workaround: use separate `insert` + `update` calls where portability or correct timestamps are required. -- **Schema builder does not auto-apply the table prefix.** `Schema::$prefix` - defaults to `null`, not `''`; table names are used as-is unless you call - `Schema::withPrefix()` explicitly. See [Schema builder reference](schema.md). +- **Schema builder uses the literal table name by design.** `Schema::create()` passes + the name through as-is — no WordPress prefix is added automatically, so existing + tables are never relocated. Use `Schema::withPrefix('your_prefix_')` for a literal + prefix, or `Schema::withWpPrefix()` to match the prefix `Model`s use + (`Connection::getPrefix()`). The `$prefix` property intentionally defaults to `null` + (not `''`) to preserve bare-table behaviour for plugins that rely on it. + See [Schema builder reference](schema.md). diff --git a/src/Schema.php b/src/Schema.php index ecc9409..6cf2073 100644 --- a/src/Schema.php +++ b/src/Schema.php @@ -46,6 +46,14 @@ public function __call($method, $parameters) return $this->build($blueprint); } + public static function withWpPrefix() + { + $schema = new self(); + $schema->prefix = Connection::getPrefix(); + + return $schema; + } + public function createBlueprint($schema, $method, ?Closure $callback = null) { return new Blueprint( diff --git a/tests/SchemaTest.php b/tests/SchemaTest.php index f3a8096..af4756a 100644 --- a/tests/SchemaTest.php +++ b/tests/SchemaTest.php @@ -11,7 +11,6 @@ * * These tests operate on the literal table name — no prefix is applied because * Connection::setPluginPrefix is intentionally not called here. - * */ final class SchemaTest extends TestCase { @@ -118,4 +117,40 @@ public function testDropPrimaryEditModeStillWorks(): void $this->assertStringContainsString('DROP PRIMARY KEY', $sql); } + + // --- B1: prefix behaviour --- + + public function testBareDefaultNeverPrefixedWithWp(): void + { + Schema::create('orders', function ($table) { + $table->id(); + }); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('CREATE TABLE IF NOT EXISTS orders', $sql); + $this->assertStringNotContainsString('wp_orders', $sql); + } + + public function testWithWpPrefixPrependsWpPrefix(): void + { + Schema::withWpPrefix()->create('orders', function ($table) { + $table->id(); + }); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('CREATE TABLE IF NOT EXISTS wp_orders', $sql); + } + + public function testWithPrefixRegressionCustomPrefix(): void + { + Schema::withPrefix('custom_')->create('orders', function ($table) { + $table->id(); + }); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('CREATE TABLE IF NOT EXISTS custom_orders', $sql); + } } From ea54365da9ae8f9ec877449a1611626efa72b3cb Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 11:14:20 +0600 Subject: [PATCH 07/32] feat: opt-in soft-delete read scope with withTrashed/onlyTrashed Assisted-By: AI --- docs/schema.md | 7 ++- docs/usage.md | 21 ++++++--- src/Model.php | 2 + src/QueryBuilder.php | 67 ++++++++++++++++++++++++++- tests/Fixtures/ScopedSoftPost.php | 18 ++++++++ tests/SoftDeleteScopeTest.php | 75 +++++++++++++++++++++++++++++++ tests/bootstrap.php | 1 + 7 files changed, 181 insertions(+), 10 deletions(-) create mode 100644 tests/Fixtures/ScopedSoftPost.php create mode 100644 tests/SoftDeleteScopeTest.php diff --git a/docs/schema.md b/docs/schema.md index 6d0c477..0c4b801 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -212,8 +212,11 @@ Adds a single nullable `TIMESTAMP` column named `deleted_at`, defaulting to `NUL The model-side `$soft_deletes = true` property (see [Defining models](usage.md#defining-models)) instructs `delete()` to set `deleted_at` -rather than removing the row. **Reads are not filtered** — soft-deleted rows are returned -by `all()` and every query; append `->whereNull('deleted_at')` manually to exclude them. +rather than removing the row. Reads return **all rows by default** — including trashed +ones. To enable automatic filtering, also declare `public $soft_delete_scope = true;` on +the model: reads then exclude trashed rows automatically. Use `->withTrashed()` to +include them, or `->onlyTrashed()` to return only trashed rows. See +[Limitations](usage.md#limitations--known-issues) for the `refresh()` edge case. --- diff --git a/docs/usage.md b/docs/usage.md index 3515777..eaa2a40 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -123,7 +123,7 @@ class Contact extends Model | `$fillable` | Mass-assignment allow-list. Unset = allow all non-timestamp, non-PK attributes. | | `$casts` | Map of column → cast type. See [Attribute casting](#attribute-casting). | | `$timestamps` | Auto-set `created_at`/`updated_at` on insert/update (declared `true` in the base `Model`; set to `false` to disable). Requires the columns to exist — see [`timestamps()` in `docs/schema.md`](schema.md#timestamps). | -| `$soft_deletes` | Must be declared `true` on the model. `delete()` then sets `deleted_at` instead of removing the row. **Reads are not filtered** — soft-deleted rows are returned by `all()` and every query. See [Limitations](#limitations--known-issues) and [`softDeletes()` in `docs/schema.md`](schema.md#softdeletes). | +| `$soft_deletes` | Must be declared `true` on the model. `delete()` then sets `deleted_at` instead of removing the row. Reads return **all rows by default** — including trashed ones. To enable automatic filtering, also declare `public $soft_delete_scope = true;`; reads will then exclude trashed rows automatically. Use `->withTrashed()` to include them, or `->onlyTrashed()` to return only trashed rows. See [Limitations](#limitations--known-issues) and [`softDeletes()` in `docs/schema.md`](schema.md#softdeletes). | --- @@ -361,8 +361,10 @@ Contact::destroy([1, 2, 3]); // delete by primary keys `RuntimeException('SQL query is empty')` — a guard against wiping the table. Use a raw `TRUNCATE` if you really mean to empty it. - With `$soft_deletes = true`, `delete()` sets `deleted_at` instead of removing - the row. **Reads are not filtered** — soft-deleted rows are returned by `all()` - and every query; there is no automatic scope. See [Limitations](#limitations--known-issues). + the row. Reads return **all rows by default** — including trashed ones. Add + `public $soft_delete_scope = true;` to enable automatic filtering: reads exclude + trashed rows, `->withTrashed()` includes them, `->onlyTrashed()` returns only + trashed rows. See [Limitations](#limitations--known-issues). --- @@ -674,10 +676,15 @@ method relocation. **calling** model's table — the opposite of Laravel's convention. Ensure you supply both arguments explicitly to avoid confusion. -- **Soft delete is write-only.** `softDeletes()` adds a `deleted_at` column and - `delete()` sets it, but there is no global scope to filter soft-deleted rows - from queries. Every `get()` / `find()` returns soft-deleted rows alongside - live ones. Workaround: add `->whereNull('deleted_at')` to every read query. +- **Soft delete reads are unfiltered by default.** `softDeletes()` adds a + `deleted_at` column and `delete()` sets it. Reads return all rows — including + trashed ones — unless you also declare `public $soft_delete_scope = true;` on + the model. With the flag set, reads automatically exclude trashed rows; + `->withTrashed()` includes them and `->onlyTrashed()` returns only trashed rows. + **Edge case:** `refresh()` / `exists()` use a default (now scoped) read, so a + trashed opt-in model reloaded without `->withTrashed()` reports `exists() === false` + and a subsequent `save()` will INSERT rather than UPDATE. Follow-up planned to + thread soft-delete awareness into `refresh()`. - **`upsert` is MySQL-only.** It generates `INSERT … ON DUPLICATE KEY UPDATE`, which is not portable to other databases. Additionally, the generated SQL sets diff --git a/src/Model.php b/src/Model.php index 012e1da..a6f4d57 100644 --- a/src/Model.php +++ b/src/Model.php @@ -87,6 +87,8 @@ * @method static bool|string delete() * @method static string toSql() * @method static string prepare($sql = null) + * @method static QueryBuilder withTrashed() + * @method static QueryBuilder onlyTrashed() * * @mixin \BitApps\WPDatabase\QueryBuilder */ diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index f31cdae..c171637 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -77,6 +77,10 @@ class QueryBuilder private $_grammar; + private $_withTrashed = false; + + private $_onlyTrashed = false; + /** * Constructs QueryBuilder * @@ -240,13 +244,30 @@ public function getTable() /** * Returns the clause list (where/having) for the given type. * + * When the type is 'where' and the model opts into soft-delete scope, + * a deleted_at IS NULL (or IS NOT NULL for onlyTrashed) clause is injected + * on SELECT queries without mutating $this->where. + * * @param string $type * * @return array */ public function getClauseList($type) { - return $type === 'having' ? $this->having : $this->where; + if ($type === 'having') { + return $this->having; + } + + $where = $this->where; + if ($this->isSoftDeleteModel() && $this->_method === self::SELECT) { + if ($this->_onlyTrashed) { + $where[] = ['column' => 'deleted_at', 'operator' => 'IS NOT NULL']; + } elseif ($this->autoScopeEnabled() && !$this->_withTrashed) { + $where[] = ['column' => 'deleted_at', 'operator' => 'IS NULL']; + } + } + + return $where; } /** @@ -579,6 +600,30 @@ public function whereNotNull($column) return $this; } + /** + * Include soft-deleted rows in the result set. + * + * @return $this + */ + public function withTrashed() + { + $this->_withTrashed = true; + + return $this; + } + + /** + * Restrict results to only soft-deleted rows. + * + * @return $this + */ + public function onlyTrashed() + { + $this->_onlyTrashed = true; + + return $this; + } + /** * Set where clause with between condition * @@ -1438,6 +1483,26 @@ protected function getTimeZone() return $timezoneString; } + /** + * Returns true when the model declares soft-delete support. + * + * @return bool + */ + private function isSoftDeleteModel() + { + return property_exists($this->_model, 'soft_deletes') && $this->_model->soft_deletes; + } + + /** + * Returns true when the model opts into automatic soft-delete read scope. + * + * @return bool + */ + private function autoScopeEnabled() + { + return property_exists($this->_model, 'soft_delete_scope') && $this->_model->soft_delete_scope; + } + /** * Run bulk insert query * diff --git a/tests/Fixtures/ScopedSoftPost.php b/tests/Fixtures/ScopedSoftPost.php new file mode 100644 index 0000000..104d15d --- /dev/null +++ b/tests/Fixtures/ScopedSoftPost.php @@ -0,0 +1,18 @@ +toSql(); + $this->assertStringNotContainsString('deleted_at', $sql); + } + + // Non-soft-delete model untouched + public function testNonSoftDeleteModelIsNotFiltered(): void + { + $sql = User::query()->toSql(); + $this->assertStringNotContainsString('deleted_at', $sql); + } + + // Opt-in model: default query filters out trashed rows + public function testScopedModelDefaultQueryFiltersDeletedAt(): void + { + $sql = ScopedSoftPost::query()->toSql(); + $this->assertStringContainsString('deleted_at', $sql); + $this->assertStringContainsString('IS NULL', $sql); + } + + // withTrashed() removes the filter + public function testWithTrashedRemovesDeletedAtFilter(): void + { + $sql = ScopedSoftPost::query()->withTrashed()->toSql(); + $this->assertStringNotContainsString('deleted_at', $sql); + } + + // onlyTrashed() returns only trashed rows + public function testOnlyTrashedFiltersToTrashedRows(): void + { + $sql = ScopedSoftPost::query()->onlyTrashed()->toSql(); + $this->assertStringContainsString('deleted_at', $sql); + $this->assertStringContainsString('IS NOT NULL', $sql); + } + + // onlyTrashed() works on a non-scoped $soft_deletes model too + public function testOnlyTrashedWorksWithoutScopeFlag(): void + { + $sql = SoftPost::query()->onlyTrashed()->toSql(); + $this->assertStringContainsString('IS NOT NULL', $sql); + } + + // Aggregate carries the scope + public function testAggregateCarriesSoftDeleteScope(): void + { + ScopedSoftPost::query()->count(); + $sql = $GLOBALS['wpdb']->last_query; + $this->assertStringContainsString('deleted_at', $sql); + $this->assertStringContainsString('IS NULL', $sql); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 517bb5c..c270bc7 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -131,3 +131,4 @@ public function has_cap($cap) require __DIR__ . '/Fixtures/CastModel.php'; require __DIR__ . '/Fixtures/AccessorModel.php'; require __DIR__ . '/Fixtures/CreatingUser.php'; +require __DIR__ . '/Fixtures/ScopedSoftPost.php'; From e78915e0a08637e849f2de1c273370afa06e729c Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 11:24:22 +0600 Subject: [PATCH 08/32] fix: parenthesize user WHERE when injecting soft-delete scope Prevents AND/OR precedence leak when orWhere is combined with the auto-scope or onlyTrashed, which could expose trashed rows. Assisted-By: AI --- src/QueryBuilder.php | 33 +++++++++++++++++++++++++-------- tests/SoftDeleteScopeTest.php | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index c171637..bc41d9d 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -258,16 +258,33 @@ public function getClauseList($type) return $this->having; } - $where = $this->where; - if ($this->isSoftDeleteModel() && $this->_method === self::SELECT) { - if ($this->_onlyTrashed) { - $where[] = ['column' => 'deleted_at', 'operator' => 'IS NOT NULL']; - } elseif ($this->autoScopeEnabled() && !$this->_withTrashed) { - $where[] = ['column' => 'deleted_at', 'operator' => 'IS NULL']; - } + if (!$this->isSoftDeleteModel() || $this->_method !== self::SELECT) { + return $this->where; + } + + $scopeClause = null; + if ($this->_onlyTrashed) { + $scopeClause = ['column' => 'deleted_at', 'operator' => 'IS NOT NULL']; + } elseif ($this->autoScopeEnabled() && !$this->_withTrashed) { + $scopeClause = ['column' => 'deleted_at', 'operator' => 'IS NULL']; + } + + if ($scopeClause === null) { + return $this->where; } - return $where; + if (empty($this->where)) { + return [$scopeClause]; + } + + // Wrap user conditions to prevent AND/OR precedence issues with the injected scope + $nestedQuery = $this->newQuery(); + $nestedQuery->where = $this->where; + + return [ + ['query' => $nestedQuery], + $scopeClause, + ]; } /** diff --git a/tests/SoftDeleteScopeTest.php b/tests/SoftDeleteScopeTest.php index 4173d57..e73b86e 100644 --- a/tests/SoftDeleteScopeTest.php +++ b/tests/SoftDeleteScopeTest.php @@ -72,4 +72,36 @@ public function testAggregateCarriesSoftDeleteScope(): void $this->assertStringContainsString('deleted_at', $sql); $this->assertStringContainsString('IS NULL', $sql); } + + // scope + orWhere: user conditions must be parenthesized to prevent precedence leak + public function testScopedModelOrWhereGroupsUserConditions(): void + { + $sql = ScopedSoftPost::query() + ->where('status', 'active') + ->orWhere('status', 'pending') + ->toSql(); + + // scope clause must be outside the OR group + $this->assertStringContainsString('deleted_at', $sql); + $this->assertStringContainsString('IS NULL', $sql); + // user conditions must be parenthesized + $this->assertMatchesRegularExpression('/\(.*status.*OR.*status.*\)/s', $sql); + // scope must appear after the closing parenthesis (column may be backtick-quoted) + $this->assertMatchesRegularExpression('/\).*deleted_at.*IS\s+NULL/s', $sql); + } + + // onlyTrashed + orWhere: same grouping requirement + public function testOnlyTrashedOrWhereGroupsUserConditions(): void + { + $sql = ScopedSoftPost::query() + ->where('status', 'active') + ->orWhere('status', 'pending') + ->onlyTrashed() + ->toSql(); + + $this->assertStringContainsString('deleted_at', $sql); + $this->assertStringContainsString('IS NOT NULL', $sql); + $this->assertMatchesRegularExpression('/\(.*status.*OR.*status.*\)/s', $sql); + $this->assertMatchesRegularExpression('/\).*deleted_at.*IS\s+NOT\s+NULL/s', $sql); + } } From 9677449224aec788a5a5eded73ea1c92ab3c5283 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 12:10:26 +0600 Subject: [PATCH 09/32] fix: upsert manages updated_at; reorganise review-fix tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit upsert() on a $timestamps model now inserts both created_at and updated_at and, on a duplicate key, bumps updated_at (VALUES(updated_at)) while preserving the original created_at — replacing the prior logic that left updated_at unmanaged and emitted the `updated_at = VALUES(created_at)` swap. No timestamp magic when $timestamps is false. Splits the non-generic GeminiReviewFixesTest into topical suites — UpsertTest, AttributeCastTest, CollectionTest — and removes the old upsert-swap test that locked the previous mapping. Reframes the upsert docs and records the behavioural change in breaking-changes §3. Assisted-By: AI --- docs/breaking-changes.md | 6 +++ docs/usage.md | 18 +++----- src/QueryBuilder.php | 32 ++++++++----- tests/AttributeCastTest.php | 44 ++++++++++++++++++ tests/CollectionTest.php | 35 +++++++++++++++ tests/Fixtures/TimestampedRow.php | 18 ++++++++ tests/GeminiReviewFixesTest.php | 74 ------------------------------ tests/UpsertTest.php | 75 +++++++++++++++++++++++++++++++ tests/bootstrap.php | 1 + 9 files changed, 207 insertions(+), 96 deletions(-) create mode 100644 tests/AttributeCastTest.php create mode 100644 tests/CollectionTest.php create mode 100644 tests/Fixtures/TimestampedRow.php delete mode 100644 tests/GeminiReviewFixesTest.php create mode 100644 tests/UpsertTest.php diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index 2391de2..c180c7e 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -303,6 +303,12 @@ Not signature breaks, but observable runtime differences. - **Internal layout:** the three traits moved to `BitApps\WPDatabase\Concerns` and SELECT compilation extracted to `BitApps\WPDatabase\Query\Grammar`. Public classes are unchanged; only code importing those internals directly is affected. +- **`upsert()` now manages `updated_at`** for `$timestamps` models: it inserts both + `created_at` and `updated_at`, and on a duplicate key bumps + `updated_at = VALUES(updated_at)` while preserving `created_at` — replacing the + prior behavior that left `updated_at` untouched and mapped + `updated_at = VALUES(created_at)`. The generated SQL changes for upsert on + timestamped models. --- diff --git a/docs/usage.md b/docs/usage.md index eaa2a40..3deb44d 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -387,13 +387,11 @@ Contact::query()->upsert([ ``` > **MySQL only** — `upsert()` emits `INSERT … ON DUPLICATE KEY UPDATE` and will not -> work on other databases. -> -> **`updated_at` quirk** — when `updated_at` is in the conflict-update set, the -> generated SQL maps `updated_at = VALUES(created_at)` rather than -> `VALUES(updated_at)`. This is a known implementation detail; the timestamp will -> reflect the value written to `created_at` at insert time, not a separate -> `updated_at` value. See [Limitations](#limitations--known-issues). +> work on other databases. See [Limitations](#limitations--known-issues). + +On a model with `$timestamps = true`, `upsert()` sets both `created_at` and +`updated_at` on insert, and on a duplicate key it bumps `updated_at` +(`updated_at = VALUES(updated_at)`) while preserving the original `created_at`. --- @@ -687,10 +685,8 @@ method relocation. thread soft-delete awareness into `refresh()`. - **`upsert` is MySQL-only.** It generates `INSERT … ON DUPLICATE KEY UPDATE`, - which is not portable to other databases. Additionally, the generated SQL sets - `updated_at = VALUES(created_at)` instead of `VALUES(updated_at)`, so the - timestamp may be wrong on update. Workaround: use separate `insert` + `update` - calls where portability or correct timestamps are required. + which is not portable to other databases. Workaround: use separate `insert` + + `update` calls where cross-database portability is required. - **Schema builder uses the literal table name by design.** `Schema::create()` passes the name through as-is — no WordPress prefix is added automatically, so existing diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index bc41d9d..cafb12e 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -1280,10 +1280,15 @@ public function upsert(array $values, ?array $update = null) $this->bindings = []; $columns = array_keys($values[0]); sort($columns); - $createdAt = property_exists($this->_model, 'timestamps') && $this->_model->timestamps && !\in_array('created_at', $columns); - if ($createdAt) { + $manageTimestamps = property_exists($this->_model, 'timestamps') && $this->_model->timestamps; + $addCreatedAt = $manageTimestamps && !\in_array('created_at', $columns, true); + $addUpdatedAt = $manageTimestamps && !\in_array('updated_at', $columns, true); + if ($addCreatedAt) { $columns[] = 'created_at'; } + if ($addUpdatedAt) { + $columns[] = 'updated_at'; + } $sql = 'INSERT INTO ' . $this->table; $sql .= ' (' . implode(', ', $columns) . ')'; @@ -1291,8 +1296,14 @@ public function upsert(array $values, ?array $update = null) $insertAbleValues = []; foreach ($values as $row) { ksort($row); - if ($createdAt) { - $row['created_at'] = $this->currentTimestamp(); + if ($addCreatedAt || $addUpdatedAt) { + $now = $this->currentTimestamp(); + if ($addCreatedAt) { + $row['created_at'] = $now; + } + if ($addUpdatedAt) { + $row['updated_at'] = $now; + } } $rowValues = array_values($row); @@ -1316,15 +1327,14 @@ function ($value) { $sql .= empty($insertAbleValues) ? ' default values' : ' ' . implode(',', $insertAbleValues); $sql .= ' ON DUPLICATE KEY UPDATE '; - if (\in_array('created_at', $update, true)) { - $update = array_diff($update, ['created_at']); - $update[] = 'updated_at'; + if ($manageTimestamps) { + // Never overwrite the original creation time on update; always bump updated_at. + $update = array_diff($update, ['created_at']); + if (!\in_array('updated_at', $update, true)) { + $update[] = 'updated_at'; + } } $update = array_map(function ($column) { - if ($column === 'updated_at') { - return $column . ' = VALUES(created_at)'; - } - return $column . ' = VALUES(' . $column . ')'; }, $update); $sql .= implode(', ', $update); diff --git a/tests/AttributeCastTest.php b/tests/AttributeCastTest.php new file mode 100644 index 0000000..fdfa01d --- /dev/null +++ b/tests/AttributeCastTest.php @@ -0,0 +1,44 @@ + '1']); + + $this->assertSame(true, $model->flag); + } + + /** + * withCast() on the builder must stay chainable (return the QueryBuilder). + */ + public function testWithCastOnBuilderIsChainable(): void + { + $this->assertInstanceOf(QueryBuilder::class, User::query()->withCast(['flag' => 'bool'])); + } +} diff --git a/tests/CollectionTest.php b/tests/CollectionTest.php new file mode 100644 index 0000000..4228e2a --- /dev/null +++ b/tests/CollectionTest.php @@ -0,0 +1,35 @@ + 1])]); + + $this->assertSame(['L'], $collection->pluck('label')->all()); + } +} diff --git a/tests/Fixtures/TimestampedRow.php b/tests/Fixtures/TimestampedRow.php new file mode 100644 index 0000000..f10d535 --- /dev/null +++ b/tests/Fixtures/TimestampedRow.php @@ -0,0 +1,18 @@ +upsert(['first_name' => 'Ada', 'email' => 'a@x.com']); - - $sql = $GLOBALS['wpdb']->last_query; - - $this->assertStringContainsString('(email, first_name)', $sql, 'columns must be sorted to match the ksorted row values'); - $this->assertStringContainsString("('a@x.com', 'Ada')", $sql); - } - - /** upsert: an explicit `created_at` in the update list must be rewritten to `updated_at`. */ - public function testUpsertSwapsCreatedAtForUpdatedAtOnDuplicate(): void - { - User::query()->upsert( - ['email' => 'a@x.com', 'created_at' => '2020-01-01 00:00:00'], - ['email', 'created_at'] - ); - - $sql = $GLOBALS['wpdb']->last_query; - - $this->assertStringContainsString('updated_at = VALUES(created_at)', $sql); - $this->assertStringNotContainsString('created_at = VALUES(created_at)', $sql); - } - - /** The documented `boolean` cast must convert the value, like `bool`. */ - public function testBooleanCastConvertsValue(): void - { - $model = new CastModel(['flag' => '1']); - - $this->assertSame(true, $model->flag); - } - - /** Collection::pluck must resolve dynamic (accessor) attributes on models. */ - public function testPluckResolvesAccessorAttribute(): void - { - $collection = new Collection([new AccessorModel(['id' => 1])]); - - $this->assertSame(['L'], $collection->pluck('label')->all()); - } - - /** withCast() on the builder must stay chainable (return the QueryBuilder). */ - public function testWithCastOnBuilderIsChainable(): void - { - $this->assertInstanceOf(QueryBuilder::class, User::query()->withCast(['flag' => 'bool'])); - } -} diff --git a/tests/UpsertTest.php b/tests/UpsertTest.php new file mode 100644 index 0000000..e90140f --- /dev/null +++ b/tests/UpsertTest.php @@ -0,0 +1,75 @@ +upsert(['first_name' => 'Ada', 'email' => 'a@x.com']); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('(email, first_name)', $sql); + $this->assertStringContainsString("('a@x.com', 'Ada')", $sql); + } + + /** + * With timestamps enabled, upsert inserts both created_at and updated_at, and + * on duplicate bumps updated_at (VALUES(updated_at)) while preserving created_at + * (created_at is excluded from the ON DUPLICATE KEY UPDATE set). + */ + public function testUpsertManagesTimestampsOnDuplicate(): void + { + TimestampedRow::query()->upsert(['email' => 'a@x.com', 'name' => 'Ada']); + + $sql = $GLOBALS['wpdb']->last_query; + + // both timestamp columns are inserted + $this->assertStringContainsString('created_at', $sql); + $this->assertStringContainsString('updated_at', $sql); + // updated_at is bumped from its own inserted value, not created_at + $this->assertStringContainsString('updated_at = VALUES(updated_at)', $sql); + $this->assertStringNotContainsString('VALUES(created_at)', $sql); + // created_at is preserved on update (never in the update set) + $this->assertStringNotContainsString('created_at = VALUES(created_at)', $sql); + } + + /** + * With timestamps disabled, upsert applies no timestamp magic — columns map verbatim. + */ + public function testUpsertWithoutTimestampsHasNoMagic(): void + { + User::query()->upsert( + ['email' => 'a@x.com', 'created_at' => '2020-01-01 00:00:00'], + ['email', 'created_at'] + ); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('created_at = VALUES(created_at)', $sql); + $this->assertStringNotContainsString('updated_at', $sql); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index c270bc7..c23a194 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -132,3 +132,4 @@ public function has_cap($cap) require __DIR__ . '/Fixtures/AccessorModel.php'; require __DIR__ . '/Fixtures/CreatingUser.php'; require __DIR__ . '/Fixtures/ScopedSoftPost.php'; +require __DIR__ . '/Fixtures/TimestampedRow.php'; From 3cac80ce420b2e4db8baa322a18d71424e8dd04e Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 14:53:28 +0600 Subject: [PATCH 10/32] feat: add pivot-table many-to-many to belongsToMany (read-side) belongsToMany was a stub: it set a distinct relateAs tag but resolved identically to hasMany (no junction table). Extend it to a Laravel-style signature so it loads related rows through a pivot table. belongsToMany($model, $pivotTable = null, $foreignPivotKey = null, $relatedPivotKey = null, $parentKey = null, $relatedKey = null) The pivot branch JOINs the pivot table and carries the parent-link column back as an aliased select column, bucketing related rows on it; eager with() and lazy access both supported. withPivot() selects extra pivot columns, exposed flat as pivot_ attributes. Aggregates/whereHas over a pivot relation and withPivot() on a non-pivot relation fail loud. Zero-BC: each non-pivot path (hasMany, belongsTo/hasOne, legacy belongsToMany($m) with no pivot table) is byte-identical -- the pivot logic sits behind early relateAs guards above untouched bodies. PHP 7.4 compatible. Read-side only; attach/detach/sync deferred. Assisted-By: AI --- docs/breaking-changes.md | 34 +++++ docs/usage.md | 107 ++++++++++++-- src/Concerns/QueriesRelationships.php | 21 +++ src/Concerns/Relations.php | 148 ++++++++++++++++++- src/Model.php | 20 ++- tests/BelongsToManyPivotTest.php | 200 ++++++++++++++++++++++++++ tests/Fixtures/Member.php | 34 +++++ tests/Fixtures/Role.php | 14 ++ tests/bootstrap.php | 2 + 9 files changed, 563 insertions(+), 17 deletions(-) create mode 100644 tests/BelongsToManyPivotTest.php create mode 100644 tests/Fixtures/Member.php create mode 100644 tests/Fixtures/Role.php diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index c180c7e..481b6f9 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -309,6 +309,12 @@ Not signature breaks, but observable runtime differences. prior behavior that left `updated_at` untouched and mapped `updated_at = VALUES(created_at)`. The generated SQL changes for upsert on timestamped models. +- **`belongsToMany()` positional args 2 and 3 changed meaning** — from + `(foreignKey, localKey)` to `(pivotTable, foreignPivotKey)` (see §4.6). + `belongsToMany($model)` with no extra args is **byte-identical** to before + (legacy null-pivot path). Any call passing positional arg 2+ now takes the + pivot path, treating arg 2 as the pivot table name. Zero known callers across + consumers; flagged for completeness. --- @@ -410,6 +416,34 @@ User::query()->with('posts')->where('active', 1)->get(); - **QueryBuilder:** `static $TIME_ZONE` to set the timezone statically; `$select` / `$selectRaw` are now `public` (were `protected`). +### 4.6 Real pivot-table many-to-many on `belongsToMany` + +`belongsToMany` now resolves a true many-to-many relation through a pivot +(junction) table for **reads** — eager `with()` and lazy `$model->relation`: + +```php +public function roles() +{ + return $this->belongsToMany(Role::class, 'role_user', 'member_id', 'role_id'); +} + +Member::with('roles')->get(); // eager +$member->roles; // lazy +``` + +Full signature +`belongsToMany($model, $pivotTable = null, $foreignPivotKey = null, $relatedPivotKey = null, $parentKey = null, $relatedKey = null)`. +Omitted keys derive from the package FK convention (`members_id`, `roles_id`). +`withPivot([...])` selects extra pivot columns, exposed flat on each related +model as `pivot_` attributes (the parent link is always exposed as +`pivot_`). When `$pivotTable` is `null` the method keeps its +**legacy** behaviour (resolves like `hasMany`), so existing `belongsToMany($model)` +calls are unaffected. + +Out of scope (read-only): `attach`/`detach`/`sync`, and +`withCount`/`whereHas`/aggregates over a pivot relation (these throw +`RuntimeException`). See the usage doc's Limitations for the full list. + --- ## 5. Deprecations diff --git a/docs/usage.md b/docs/usage.md index 3deb44d..d02b6e7 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -472,13 +472,82 @@ class Deal extends Model > model. There is no separate reverse-direction implementation; the direction is > determined entirely by the keys you provide and which model calls the method. -Available: `hasOne()`, `hasMany()`, `belongsTo()`, `belongsToMany()` — each -takes `($model, $foreignKey = null, $localKey = null)`. +Available: `hasOne()`, `hasMany()`, `belongsTo()` — each takes +`($model, $foreignKey = null, $localKey = null)`. -> **`belongsToMany` is non-functional for pivot tables.** The method is -> declared but contains no pivot-table join logic. Calling it will not produce a -> many-to-many query through an intermediate table. See -> [Limitations](#limitations--known-issues). +### Many-to-many (`belongsToMany`) + +`belongsToMany` resolves a many-to-many relation through a pivot (junction) +table. Signature: + +```php +belongsToMany( + $model, + $pivotTable = null, // unprefixed pivot/junction table name + $foreignPivotKey = null, // parent's key column ON the pivot table + $relatedPivotKey = null, // related's key column ON the pivot table + $parentKey = null, // local key column on the parent table + $relatedKey = null // key column on the related table +) +``` + +When `$pivotTable` is `null` the method keeps its **legacy** behaviour (resolves +exactly like `hasMany` — the related table must carry the parent FK). Pass the +pivot table name (unprefixed; the package prefixes it like `join()` does) to get +real pivot behaviour. + +Omitted keys derive from the package's own foreign-key convention +(`{tableWithoutPrefix}_{primaryKey}`, e.g. `members_id` — note: plural, unlike +Laravel's singular default): + +| Argument | Default | Member (`members`) ↔ Role (`roles`) | +|---|---|---| +| `$foreignPivotKey` | parent `getForeignKey()` | `members_id` | +| `$relatedPivotKey` | related `getForeignKey()` | `roles_id` | +| `$parentKey` | parent `getPrimaryKey()` | `id` | +| `$relatedKey` | related `getPrimaryKey()` | `id` | + +```php +class Member extends Model +{ + protected $table = 'members'; + + public function roles() + { + // pivot table role_user(member_id, role_id) + return $this->belongsToMany(Role::class, 'role_user', 'member_id', 'role_id'); + } + + // Carry extra pivot columns; they surface flat as `pivot_` attributes + public function rolesWithAssignment() + { + return $this->belongsToMany(Role::class, 'role_user', 'member_id', 'role_id') + ->withPivot(['assigned_at']); + } +} +``` + +The link column rides along on every related model as the reserved attribute +`pivot_` (e.g. `pivot_member_id`), and each `withPivot()` column +as `pivot_` (e.g. `pivot_assigned_at`). These flat `pivot_*` attributes +appear in `toArray()`. + +```php +// Eager +foreach (Member::with('roles')->get() as $member) { + foreach ($member->roles as $role) { + echo $role->pivot_member_id; // the parent link + } +} + +// Lazy +$member = Member::query()->findOne(['id' => 1]); +foreach ($member->roles as $role) { /* ... */ } +``` + +Read-only only: `attach`/`detach`/`sync` and `withCount`/`whereHas`/aggregates +over a pivot relation are **not** supported (the latter throw). See +[Limitations](#limitations--known-issues). ### Eager loading @@ -663,10 +732,28 @@ method relocation. ## Limitations & known issues -- **`belongsToMany` is declared but non-functional.** The method exists but - contains no pivot-table join logic. Calling it does not produce a - many-to-many query through an intermediate table. Workaround: model the pivot - as an explicit intermediate model with `hasMany` on each side. +- **`belongsToMany` pivot relations are read-only and single-key.** Real + pivot-table many-to-many is supported for reads (eager `with()` + lazy + `$model->relation`), but with these gaps: + - No `withCount`/`whereHas`/aggregates on a pivot relation — they **throw** + `RuntimeException` (the pivot metadata has no single `foreignKey`/`localKey`). + - No `attach`/`detach`/`sync` (write side is out of scope). + - Single-column pivot/parent/related keys only — no composite keys. + - A non-empty model `$prefix` combined with a pivot table is unsupported (the + inherited `join()` prefixing quirk mis-prefixes the pivot table). The + default empty-`$prefix` case is correct. + - Duplicate pivot rows yield duplicate related models (no `DISTINCT`). + - Eager constraint closures may add `where`/`orderBy`/`limit` but **cannot** + narrow the selected columns — the pivot path always selects `related.*` so + the aliased pivot column can ride along. + - Pivot values surface as flat **reserved** `pivot_*` attributes (including the + link key `pivot_`) and appear in `toArray()`. A related + column literally named `pivot_*` would be overwritten. These attributes are + excluded from dirty-tracking on UPDATE, so re-saving a hydrated related model + is safe; a forced re-INSERT would attempt to write the non-existent columns. + - Null parent key: the eager path buckets a null parent key under null + (relation resolves to `null`); the lazy path renders `… IS NULL` and returns + pivot rows whose link column is NULL — a minor divergence. - **`belongsTo` and `hasOne` are the same alias; key naming is reversed from Laravel.** Both set the same `oneToOne` relation. The `$foreignKey` argument diff --git a/src/Concerns/QueriesRelationships.php b/src/Concerns/QueriesRelationships.php index 5e6366a..4b5ed88 100644 --- a/src/Concerns/QueriesRelationships.php +++ b/src/Concerns/QueriesRelationships.php @@ -7,8 +7,10 @@ namespace BitApps\WPDatabase\Concerns; +use BitApps\WPDatabase\Model; use BitApps\WPDatabase\QueryBuilder; use Closure; +use RuntimeException; if (!\defined('ABSPATH')) { exit; @@ -37,6 +39,21 @@ public function with($relation, $callback = null) return $this; } + /** + * Selects extra pivot-table columns for a pivot belongsToMany relation. + * They surface flat on each related model as `pivot_` attributes. + * + * @param array|string $columns + * + * @return $this + */ + public function withPivot($columns) + { + $this->_model->addPivotColumns(\is_array($columns) ? $columns : \func_get_args()); + + return $this; + } + /** * Adds a relation count sub query to this query. * @@ -284,6 +301,10 @@ private function resolveRelations($relation, $callback) */ private function correlate(QueryBuilder $relationalQuery) { + if ($relationalQuery->getModel()->getRelateAs() === Model::RELATE_AS_PIVOT) { + throw new RuntimeException('Relation aggregates and whereHas are not supported for pivot belongsToMany relations.'); + } + $relationKey = $relationalQuery->getModel()->getActiveRelationKey(); $relationalQuery->whereRaw( diff --git a/src/Concerns/Relations.php b/src/Concerns/Relations.php index a41f1d0..e5d11ed 100644 --- a/src/Concerns/Relations.php +++ b/src/Concerns/Relations.php @@ -9,6 +9,7 @@ use BitApps\WPDatabase\Model; use BitApps\WPDatabase\QueryBuilder; use Closure; +use RuntimeException; if (!\defined('ABSPATH')) { exit; @@ -81,13 +82,28 @@ public function newHasMany($model, $foreignKey = null, $localKey = null) return $model->newQuery(); } - public function belongsToMany($model, $foreignKey = null, $localKey = null) - { - $relationKeys = $this->getRelationKeys($foreignKey, $localKey); - $foreignKey = $relationKeys[0]; - $localKey = $relationKeys[1]; + public function belongsToMany( + $model, + $pivotTable = null, + $foreignPivotKey = null, + $relatedPivotKey = null, + $parentKey = null, + $relatedKey = null + ) { + if ($pivotTable === null) { + $relationKeys = $this->getRelationKeys($foreignPivotKey, $relatedPivotKey); + + return $this->newBelongsToMany($model, $relationKeys[0], $relationKeys[1]); + } - return $this->newBelongsToMany($model, $foreignKey, $localKey); + return $this->newBelongsToManyPivot( + $model, + $pivotTable, + $foreignPivotKey, + $relatedPivotKey, + $parentKey, + $relatedKey + ); } public function newBelongsToMany($model, $foreignKey = null, $localKey = null) @@ -102,6 +118,46 @@ public function newBelongsToMany($model, $foreignKey = null, $localKey = null) return $model->newQuery(); } + public function newBelongsToManyPivot( + $model, + $pivotTable, + $foreignPivotKey = null, + $relatedPivotKey = null, + $parentKey = null, + $relatedKey = null + ) { + $related = new $model(); + + $foreignPivotKey = $foreignPivotKey ?: $this->getForeignKey(); + $relatedPivotKey = $relatedPivotKey ?: $related->getForeignKey(); + $parentKey = $parentKey ?: $this->getPrimaryKey(); + $relatedKey = $relatedKey ?: $related->getPrimaryKey(); + + $related->setRelateAs(Model::RELATE_AS_PIVOT); + $related->_relationKeys[Model::RELATE_AS_PIVOT] = [ + 'pivotTable' => $pivotTable, + 'foreignPivotKey' => $foreignPivotKey, + 'relatedPivotKey' => $relatedPivotKey, + 'parentKey' => $parentKey, + 'relatedKey' => $relatedKey, + 'pivotColumns' => [], + ]; + + return $related->newQuery(); + } + + public function addPivotColumns(array $columns) + { + if (!isset($this->_relationKeys[Model::RELATE_AS_PIVOT])) { + throw new RuntimeException('withPivot() is only valid on a pivot belongsToMany relation.'); + } + + $this->_relationKeys[Model::RELATE_AS_PIVOT]['pivotColumns'] = array_merge( + $this->_relationKeys[Model::RELATE_AS_PIVOT]['pivotColumns'], + $columns + ); + } + /** * Returns list of query of relations * @@ -193,6 +249,12 @@ private function retrieveRelateData(QueryBuilder $query) if (\count($relations) > 0) { foreach ($relations as $relationName => $relationQuery) { + if ($relationQuery->getModel()->getRelateAs() === Model::RELATE_AS_PIVOT) { + $this->retrievePivotRelateData($relationName, $relationQuery, $query); + + continue; + } + $parentQuery = clone $query; $relationKey = $relationQuery->getModel()->getActiveRelationKey(); @@ -215,11 +277,73 @@ private function retrieveRelateData(QueryBuilder $query) } } + private function retrievePivotRelateData($relationName, QueryBuilder $relationQuery, QueryBuilder $query) + { + $bucketAlias = $this->applyPivotSelectAndJoin($relationQuery); + + $pivot = $relationQuery->getModel()->getActiveRelationKey(); + $pivotRef = $relationQuery->getModel()->getPrefix() . $pivot['pivotTable']; + $parentQuery = clone $query; + + $relationQuery->whereRaw( + $pivotRef . '.' . $pivot['foreignPivotKey'] + . ' IN ( SELECT * FROM (' + . $parentQuery->select($pivot['parentKey'])->prepare() + . ') AS subquery )' + ); + + $relatedModels = $relationQuery->get(); + + if ($relatedModels) { + foreach ($relatedModels as $relatedModel) { + $this->_relatedData[$relationName][$relatedModel->getAttribute($bucketAlias)][] = $relatedModel; + } + } + } + + /** + * Selects related.* plus the aliased pivot link column, joins the pivot + * table on the related key, and appends any withPivot() columns. Returns the + * bucket alias used to group related rows by their parent link. + * + * @return string + */ + private function applyPivotSelectAndJoin(QueryBuilder $relationQuery) + { + $model = $relationQuery->getModel(); + $pivot = $model->getActiveRelationKey(); + $pivotRef = $model->getPrefix() . $pivot['pivotTable']; + $alias = Model::PIVOT_ATTRIBUTE_PREFIX . $pivot['foreignPivotKey']; + + $relationQuery->select(['*']); + $relationQuery->join( + $pivot['pivotTable'], + $pivotRef . '.' . $pivot['relatedPivotKey'], + '=', + $relationQuery->getTable() . '.' . $pivot['relatedKey'] + ); + $relationQuery->selectRaw($pivotRef . '.' . $pivot['foreignPivotKey'] . ' as `' . $alias . '`'); + + foreach ($pivot['pivotColumns'] as $column) { + $relationQuery->selectRaw( + $pivotRef . '.' . $column . ' as `' . Model::PIVOT_ATTRIBUTE_PREFIX . $column . '`' + ); + } + + return $alias; + } + private function setRelatedData(Model $model) { $relations = $this->getRelations(); if (\count($relations) > 0) { foreach ($relations as $relationName => $relationQuery) { + if ($relationQuery->getModel()->getRelateAs() === Model::RELATE_AS_PIVOT) { + $this->setPivotRelatedData($model, $relationName, $relationQuery->getModel()); + + continue; + } + $relationKey = $relationQuery->getModel()->getActiveRelationKey(); $data = isset( @@ -236,4 +360,16 @@ private function setRelatedData(Model $model) } } } + + private function setPivotRelatedData(Model $model, $relationName, Model $relatedModel) + { + $pivot = $relatedModel->getActiveRelationKey(); + $key = $model->getAttribute($pivot['parentKey']); + $data = isset($this->_relatedData[$relationName][$key]) + ? $this->_relatedData[$relationName][$key] + : null; + + [$name, $alias] = $this->prepareRelationName($relationName); + $model->setAttribute(\is_null($alias) ? $name : $alias, $data); + } } diff --git a/src/Model.php b/src/Model.php index a6f4d57..7ab1cc8 100644 --- a/src/Model.php +++ b/src/Model.php @@ -73,6 +73,7 @@ * @method static Model|bool save() * @method static Model|bool upsert(array $values, ?array $update = null) * @method static QueryBuilder with(string|array $relation, ?Closure $callback = null) + * @method static QueryBuilder withPivot($columns) * @method static QueryBuilder withCount(string|array $relation) * @method static QueryBuilder withMin(string|array $relation) * @method static QueryBuilder withMax(string|array $relation) @@ -96,6 +97,10 @@ abstract class Model implements ArrayAccess, JsonSerializable { use Relations, HasEvents; + public const RELATE_AS_PIVOT = 'belongsToManyPivot'; + + public const PIVOT_ATTRIBUTE_PREFIX = 'pivot_'; + public $timestamps = true; protected $table; @@ -588,7 +593,20 @@ private function castToDate($value) private function processRelatedAttribute(QueryBuilder $attribute) { - $relation = $attribute->getModel()->getRelateAs(); + $relation = $attribute->getModel()->getRelateAs(); + + if ($relation === self::RELATE_AS_PIVOT) { + $this->applyPivotSelectAndJoin($attribute); + $pivot = $attribute->getModel()->getActiveRelationKey(); + $pivotRef = $attribute->getModel()->getPrefix() . $pivot['pivotTable']; + $attribute->where( + $pivotRef . '.' . $pivot['foreignPivotKey'], + $this->getAttribute($pivot['parentKey']) + ); + + return $attribute->get(); + } + $relationKey = $attribute->getModel()->getRelationalKeys()[$relation]; $attribute->where($relationKey['foreignKey'], $this->getAttribute($relationKey['localKey'])); if ($relation == 'oneToOne') { diff --git a/tests/BelongsToManyPivotTest.php b/tests/BelongsToManyPivotTest.php new file mode 100644 index 0000000..b762189 --- /dev/null +++ b/tests/BelongsToManyPivotTest.php @@ -0,0 +1,200 @@ + 100, 'pivot_member_id' => 1, 'pivot_assigned_at' => '2024-01-01'], + (object) ['id' => 101, 'pivot_member_id' => 1, 'pivot_assigned_at' => '2024-02-01'], + (object) ['id' => 102, 'pivot_member_id' => 2, 'pivot_assigned_at' => '2024-03-01'], + ]; + } + + return [(object) ['id' => 1], (object) ['id' => 2]]; + }; + } + + public function testEagerLoadGroupsRelatedRowsByParent(): void + { + $GLOBALS['wpdb']->resolver = $this->pivotResolver(); + + $members = Member::with('roles')->get(); + + $this->assertInstanceOf(Collection::class, $members); + $this->assertCount(2, $members); + $this->assertCount(2, $members[0]->roles, 'member 1 should have 2 roles'); + $this->assertCount(1, $members[1]->roles, 'member 2 should have 1 role'); + $this->assertInstanceOf(Role::class, $members[0]->roles[0]); + $this->assertSame(100, $members[0]->roles[0]->id); + $this->assertSame(101, $members[0]->roles[1]->id); + $this->assertSame(102, $members[1]->roles[0]->id); + } + + public function testEagerSqlShape(): void + { + $GLOBALS['wpdb']->resolver = $this->pivotResolver(); + + Member::with('roles')->get(); + $sql = $GLOBALS['wpdb']->queries[1]; + + $this->assertStringContainsString('SELECT `wp_roles`.*', $sql); + $this->assertStringContainsString('wp_role_user.member_id as `pivot_member_id`', $sql); + $this->assertStringContainsString('INNER JOIN wp_role_user', $sql); + $this->assertStringContainsString('wp_role_user.role_id = wp_roles.id', $sql); + // Inner fragment without a leading `WHERE ` boundary: the grammar emits `WHERE ` (double space). + $this->assertStringContainsString( + 'wp_role_user.member_id IN ( SELECT * FROM (SELECT `wp_members`.`id` FROM wp_members) AS subquery )', + $sql + ); + + // Exact pin (absorbs the double-space WHERE/ON grammar artifacts). + $expected = 'SELECT `wp_roles`.*, wp_role_user.member_id as `pivot_member_id`' + . ' FROM wp_roles INNER JOIN wp_role_user ON wp_role_user.role_id = wp_roles.id' + . ' WHERE wp_role_user.member_id IN ( SELECT * FROM (SELECT `wp_members`.`id` FROM wp_members) AS subquery )'; + $this->assertSame($expected, $sql); + } + + public function testLazyAccessEmitsSingleValuePredicate(): void + { + $GLOBALS['wpdb']->resolver = $this->pivotResolver(); + + $member = new Member(['id' => 1]); + $roles = $member->roles; + + $this->assertInstanceOf(Collection::class, $roles); + + $sql = $GLOBALS['wpdb']->last_query; + $this->assertStringNotContainsString('subquery', $sql, 'lazy access must not use the IN ( SELECT ) subquery form'); + + // Exact pin: single-value predicate, double-space WHERE/ON/`=` grammar artifacts. + $expected = 'SELECT `wp_roles`.*, wp_role_user.member_id as `pivot_member_id`' + . ' FROM wp_roles INNER JOIN wp_role_user ON wp_role_user.role_id = wp_roles.id' + . ' WHERE wp_role_user.member_id = 1'; + $this->assertSame($expected, $sql); + } + + public function testDefaultKeyDerivationUsesForeignKeyConvention(): void + { + $GLOBALS['wpdb']->resolver = function ($sql) { + if (strpos($sql, 'wp_roles') !== false) { + return [ + (object) ['id' => 100, 'pivot_members_id' => 1], + (object) ['id' => 101, 'pivot_members_id' => 1], + (object) ['id' => 102, 'pivot_members_id' => 2], + ]; + } + + return [(object) ['id' => 1], (object) ['id' => 2]]; + }; + + $members = Member::with('rolesDefaultKeys')->get(); + $sql = $GLOBALS['wpdb']->queries[1]; + + $this->assertStringContainsString('wp_role_user.members_id as `pivot_members_id`', $sql); + $this->assertStringContainsString('wp_role_user.roles_id = wp_roles.id', $sql); + $this->assertCount(2, $members[0]->rolesDefaultKeys); + $this->assertCount(1, $members[1]->rolesDefaultKeys); + } + + public function testWithPivotSelectsAndExposesExtraColumn(): void + { + $GLOBALS['wpdb']->resolver = $this->pivotResolver(); + + $members = Member::with('rolesWithPivot')->get(); + $sql = $GLOBALS['wpdb']->queries[1]; + + $this->assertStringContainsString('wp_role_user.assigned_at as `pivot_assigned_at`', $sql); + $this->assertSame('2024-01-01', $members[0]->rolesWithPivot[0]->pivot_assigned_at); + } + + public function testLegacyNullPivotPathIsUnchanged(): void + { + $query = (new Member())->legacyRoles(); + + $this->assertInstanceOf(QueryBuilder::class, $query); + $this->assertSame('belongsToMany', $query->getModel()->getRelateAs()); + $this->assertSame( + ['foreignKey' => 'members_id', 'localKey' => 'id'], + $query->getModel()->getActiveRelationKey() + ); + } + + public function testAggregatesOnPivotRelationThrow(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('pivot belongsToMany'); + + Member::withCount('roles')->get(); + } + + public function testBucketAliasLeaksAsReservedAttribute(): void + { + $GLOBALS['wpdb']->resolver = $this->pivotResolver(); + + $members = Member::with('roles')->get(); + + $this->assertSame(1, $members[0]->roles[0]->pivot_member_id); + $this->assertSame(2, $members[1]->roles[0]->pivot_member_id); + } + + public function testMultiplePivotRelationsResolveIndependently(): void + { + $GLOBALS['wpdb']->resolver = function ($sql) { + if (strpos($sql, 'assigned_at') !== false) { + return [ + (object) ['id' => 200, 'pivot_member_id' => 1, 'pivot_assigned_at' => '2024-01-01'], + (object) ['id' => 201, 'pivot_member_id' => 2, 'pivot_assigned_at' => '2024-02-01'], + ]; + } + + if (strpos($sql, 'wp_roles') !== false) { + return [ + (object) ['id' => 100, 'pivot_member_id' => 1], + (object) ['id' => 102, 'pivot_member_id' => 2], + ]; + } + + return [(object) ['id' => 1], (object) ['id' => 2]]; + }; + + $members = Member::with(['roles', 'rolesWithPivot'])->get(); + + $this->assertCount(1, $members[0]->roles); + $this->assertCount(1, $members[0]->rolesWithPivot); + $this->assertSame(100, $members[0]->roles[0]->id); + $this->assertSame(200, $members[0]->rolesWithPivot[0]->id); + $this->assertSame('2024-01-01', $members[0]->rolesWithPivot[0]->pivot_assigned_at); + $this->assertNull($members[0]->roles[0]->pivot_assigned_at, 'roles must not pick up the withPivot column'); + } + + public function testPivotConstantsAreExposedOnModel(): void + { + $this->assertSame('belongsToManyPivot', Model::RELATE_AS_PIVOT); + $this->assertSame('pivot_', Model::PIVOT_ATTRIBUTE_PREFIX); + } +} diff --git a/tests/Fixtures/Member.php b/tests/Fixtures/Member.php new file mode 100644 index 0000000..dc33e82 --- /dev/null +++ b/tests/Fixtures/Member.php @@ -0,0 +1,34 @@ +belongsToMany(Role::class, 'role_user', 'member_id', 'role_id'); + } + + public function rolesDefaultKeys() + { + return $this->belongsToMany(Role::class, 'role_user'); + } + + public function rolesWithPivot() + { + return $this->belongsToMany(Role::class, 'role_user', 'member_id', 'role_id')->withPivot(['assigned_at']); + } + + public function legacyRoles() + { + return $this->belongsToMany(Role::class); + } +} diff --git a/tests/Fixtures/Role.php b/tests/Fixtures/Role.php new file mode 100644 index 0000000..88ecdfb --- /dev/null +++ b/tests/Fixtures/Role.php @@ -0,0 +1,14 @@ + Date: Tue, 30 Jun 2026 15:28:37 +0600 Subject: [PATCH 11/32] fix: join and pivot tables keep wp_ prefix for custom-$prefix models 3e5db3a dropped the leading Connection::wpPrefix() from join() to stop default-prefix models double-prefixing. That broke custom-$prefix models (e.g. bit-crm): Model::getPrefix() returns the raw $prefix without wp_, so join/pivot tables lost the wp_ prefix and pointed at the wrong table. Add Model::getTablePrefix() -- the full prefix (wp_ plus plugin prefix) that mirrors how the constructor builds $this->table -- and route the constructor, join(), and the pivot select/join helper through it. getPrefix() is unchanged (public API). Default-prefix models stay byte-identical (getTablePrefix() === getPrefix()); custom-prefix joins and pivots regain wp_. Also simplify the pivot helper: applyPivotSelectAndJoin() returns [pivot, pivotRef, alias] so callers stop recomputing them. Assisted-By: AI --- src/Concerns/Relations.php | 18 +++++++-------- src/Model.php | 26 +++++++++++++-------- src/QueryBuilder.php | 2 +- tests/Fixtures/PrefixedModel.php | 18 +++++++++++++++ tests/TablePrefixTest.php | 39 ++++++++++++++++++++++++++++++++ tests/bootstrap.php | 1 + 6 files changed, 83 insertions(+), 21 deletions(-) create mode 100644 tests/Fixtures/PrefixedModel.php create mode 100644 tests/TablePrefixTest.php diff --git a/src/Concerns/Relations.php b/src/Concerns/Relations.php index e5d11ed..0ba2271 100644 --- a/src/Concerns/Relations.php +++ b/src/Concerns/Relations.php @@ -279,11 +279,8 @@ private function retrieveRelateData(QueryBuilder $query) private function retrievePivotRelateData($relationName, QueryBuilder $relationQuery, QueryBuilder $query) { - $bucketAlias = $this->applyPivotSelectAndJoin($relationQuery); - - $pivot = $relationQuery->getModel()->getActiveRelationKey(); - $pivotRef = $relationQuery->getModel()->getPrefix() . $pivot['pivotTable']; - $parentQuery = clone $query; + [$pivot, $pivotRef, $bucketAlias] = $this->applyPivotSelectAndJoin($relationQuery); + $parentQuery = clone $query; $relationQuery->whereRaw( $pivotRef . '.' . $pivot['foreignPivotKey'] @@ -303,16 +300,17 @@ private function retrievePivotRelateData($relationName, QueryBuilder $relationQu /** * Selects related.* plus the aliased pivot link column, joins the pivot - * table on the related key, and appends any withPivot() columns. Returns the - * bucket alias used to group related rows by their parent link. + * table on the related key, and appends any withPivot() columns. Returns + * [pivot metadata, prefixed pivot-table reference, bucket alias] so callers + * build their predicate without recomputing them. * - * @return string + * @return array */ private function applyPivotSelectAndJoin(QueryBuilder $relationQuery) { $model = $relationQuery->getModel(); $pivot = $model->getActiveRelationKey(); - $pivotRef = $model->getPrefix() . $pivot['pivotTable']; + $pivotRef = $model->getTablePrefix() . $pivot['pivotTable']; $alias = Model::PIVOT_ATTRIBUTE_PREFIX . $pivot['foreignPivotKey']; $relationQuery->select(['*']); @@ -330,7 +328,7 @@ private function applyPivotSelectAndJoin(QueryBuilder $relationQuery) ); } - return $alias; + return [$pivot, $pivotRef, $alias]; } private function setRelatedData(Model $model) diff --git a/src/Model.php b/src/Model.php index 7ab1cc8..b0c05f3 100644 --- a/src/Model.php +++ b/src/Model.php @@ -158,13 +158,7 @@ public function __construct($attributes = []) $this->_tableWithoutPrefix = $this->table; } - $dbPrefix = Connection::wpPrefix(); - - if ($this->prefix === '') { - $dbPrefix = Connection::getPrefix(); - } - - $this->table = $dbPrefix . $this->prefix . $this->_tableWithoutPrefix; + $this->table = $this->getTablePrefix() . $this->_tableWithoutPrefix; if (!isset($this->primaryKey)) { $this->primaryKey = 'id'; @@ -302,11 +296,25 @@ public function getTable() return $this->table; } + /** Query/schema default prefix; NOT the full table prefix — use getTablePrefix() for join/pivot table names (it keeps wp_ for custom $prefix). */ public function getPrefix() { return $this->prefix === '' ? Connection::getPrefix() : $this->prefix; } + /** + * Full prefix prepended to this model's table: wp_ plus the plugin prefix. + * Mirrors the table built in the constructor so joins and pivot tables + * resolve to the same physical table the model itself targets. Unlike + * getPrefix(), this never drops wp_ for custom-$prefix models. + * + * @return string + */ + public function getTablePrefix() + { + return $this->prefix === '' ? Connection::getPrefix() : Connection::wpPrefix() . $this->prefix; + } + public function getTableWithoutPrefix() { return $this->_tableWithoutPrefix; @@ -596,9 +604,7 @@ private function processRelatedAttribute(QueryBuilder $attribute) $relation = $attribute->getModel()->getRelateAs(); if ($relation === self::RELATE_AS_PIVOT) { - $this->applyPivotSelectAndJoin($attribute); - $pivot = $attribute->getModel()->getActiveRelationKey(); - $pivotRef = $attribute->getModel()->getPrefix() . $pivot['pivotTable']; + [$pivot, $pivotRef] = $this->applyPivotSelectAndJoin($attribute); $attribute->where( $pivotRef . '.' . $pivot['foreignPivotKey'], $this->getAttribute($pivot['parentKey']) diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index cafb12e..e319ca3 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -770,7 +770,7 @@ public function join($table, $firstColumn, $operator = null, $secondColumn = nul $parts = preg_split('/ as /i', $table); $rawTable = $parts[0]; $alias = isset($parts[1]) ? $parts[1] : null; - $prefixedTable = $this->_model->getPrefix() . $rawTable; + $prefixedTable = $this->_model->getTablePrefix() . $rawTable; $reference = $alias !== null ? $alias : $prefixedTable; $tableSql = $alias !== null ? $prefixedTable . ' as ' . $alias : $prefixedTable; diff --git a/tests/Fixtures/PrefixedModel.php b/tests/Fixtures/PrefixedModel.php new file mode 100644 index 0000000..056211f --- /dev/null +++ b/tests/Fixtures/PrefixedModel.php @@ -0,0 +1,18 @@ +. + */ +class PrefixedModel extends Model +{ + public $timestamps = false; + + protected $table = 'widgets'; + + protected $prefix = 'crm_'; +} diff --git a/tests/TablePrefixTest.php b/tests/TablePrefixTest.php new file mode 100644 index 0000000..40dfb8f --- /dev/null +++ b/tests/TablePrefixTest.php @@ -0,0 +1,39 @@ +..., + * not just ... — regression guard for the join double-prefix fix. + */ +final class TablePrefixTest extends TestCase +{ + protected function setUp(): void + { + $GLOBALS['wpdb'] = new FakeWpdb(); + } + + protected function tearDown(): void + { + $GLOBALS['wpdb'] = new FakeWpdb(); + } + + public function testCustomPrefixModelTableCarriesWpPrefix(): void + { + $this->assertSame('wp_crm_widgets', (new PrefixedModel())->getTable()); + } + + public function testJoinOnCustomPrefixModelKeepsWpPrefix(): void + { + $sql = (new PrefixedModel()) + ->join('gadgets', 'gadgets.widget_id', '=', 'widgets.id') + ->toSql(); + + $this->assertStringContainsString('JOIN wp_crm_gadgets', $sql); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 962d69e..8fb7b06 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -135,3 +135,4 @@ public function has_cap($cap) require __DIR__ . '/Fixtures/TimestampedRow.php'; require __DIR__ . '/Fixtures/Role.php'; require __DIR__ . '/Fixtures/Member.php'; +require __DIR__ . '/Fixtures/PrefixedModel.php'; From 7aa24f2ccb17cfcb36dd433f627729c0ab1ee4c5 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 15:51:28 +0600 Subject: [PATCH 12/32] fix: bulk insert and upsert JSON-encode array/object values save()/update() encode array and object attribute values via wp_json_encode(), but bulkInsert() (insert([[...],[...]])) and upsert() bound them raw -- an array became the literal "Array" and an object threw "could not be converted to string". Mirror the save() encoding in both bulk value builders so callers don't have to wp_json_encode() manually. Scalars are unchanged; repairs already-broken output (zero-BC). Assisted-By: AI --- src/QueryBuilder.php | 8 ++++++++ tests/BulkInsertReturnTest.php | 21 +++++++++++++++++++++ tests/UpsertTest.php | 13 +++++++++++++ 3 files changed, 42 insertions(+) diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index e319ca3..6791e12 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -1316,6 +1316,10 @@ function ($value) { return 'NULL'; } + if (\is_array($value) || \is_object($value)) { + $value = wp_json_encode($value); + } + $this->bindings[] = $value; return $this->getValueType($value); @@ -1569,6 +1573,10 @@ function ($value) { return 'NULL'; } + if (\is_array($value) || \is_object($value)) { + $value = wp_json_encode($value); + } + $this->bindings[] = $value; return $this->getValueType($value); diff --git a/tests/BulkInsertReturnTest.php b/tests/BulkInsertReturnTest.php index b900f48..359ff6c 100644 --- a/tests/BulkInsertReturnTest.php +++ b/tests/BulkInsertReturnTest.php @@ -50,4 +50,25 @@ public function testBulkInsertHappyPathReturnsCollection(): void $this->assertInstanceOf(Collection::class, $result); } + + /** + * Array/object values must be JSON-encoded in bulk insert, matching save()/ + * update() — so callers don't have to wp_json_encode() them manually. + */ + public function testBulkInsertEncodesArrayAndObjectValuesAsJson(): void + { + // rows_affected = 0 keeps the INSERT as last_query (no post-insert re-query). + $GLOBALS['wpdb']->rows_affected = 0; + + User::query()->insert([ + ['name' => 'a', 'meta' => ['x' => 1]], + ['name' => 'b', 'meta' => (object) ['y' => 2]], + ]); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('{"x":1}', $sql); + $this->assertStringContainsString('{"y":2}', $sql); + $this->assertStringNotContainsString('Array', $sql); + } } diff --git a/tests/UpsertTest.php b/tests/UpsertTest.php index e90140f..c019a15 100644 --- a/tests/UpsertTest.php +++ b/tests/UpsertTest.php @@ -72,4 +72,17 @@ public function testUpsertWithoutTimestampsHasNoMagic(): void $this->assertStringContainsString('created_at = VALUES(created_at)', $sql); $this->assertStringNotContainsString('updated_at', $sql); } + + /** + * Array/object values are JSON-encoded, matching bulk insert and save(). + */ + public function testUpsertEncodesArrayValuesAsJson(): void + { + User::query()->upsert(['email' => 'a@x.com', 'meta' => ['x' => 1]]); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('{"x":1}', $sql); + $this->assertStringNotContainsString('Array', $sql); + } } From 8b21d244e89befa11323c4236d79aafbbc7dc7a6 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 16:02:12 +0600 Subject: [PATCH 13/32] docs: document JSON-encoded insert/upsert values and getTablePrefix join fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Note that array/object values are JSON-encoded across save/update/insert/ bulk insert/upsert (usage + breaking-changes §3). - Note join() prepends the model's full table prefix; add getTablePrefix() to the additions list and a §3 behavioral note for the custom-$prefix fix. - Drop the now-stale pivot limitation claiming custom-$prefix is unsupported. Assisted-By: AI --- docs/breaking-changes.md | 13 ++++++++++++- docs/usage.md | 11 ++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index 481b6f9..752c86a 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -315,6 +315,16 @@ Not signature breaks, but observable runtime differences. (legacy null-pivot path). Any call passing positional arg 2+ now takes the pivot path, treating arg 2 as the pivot table name. Zero known callers across consumers; flagged for completeness. +- **Bulk `insert()` and `upsert()` now JSON-encode array/object values** via + `wp_json_encode`, matching `save()`/`update()`. Previously a multi-row + `insert([[...]])` or `upsert()` bound an array as the literal `"Array"` and an + object threw — both repaired. Scalar values are unchanged. +- **`join()` table prefix corrected for custom-`$prefix` models.** Join (and + pivot) tables now carry the model's **full** table prefix via the new + `Model::getTablePrefix()` (`wp_` plus the plugin prefix), matching the model's + own table. Default-`$prefix` models are unchanged (`getTablePrefix()` equals + `getPrefix()` there); custom-`$prefix` models that previously lost `wp_` on + joins now match their own table. --- @@ -409,7 +419,8 @@ User::query()->with('posts')->where('active', 1)->get(); `when()`, `toSql()`, `clone()`, `aggregate()`, `prepareColumnName()`, `withCast()` (chainable), and `__call()` forwarding to the bound model. - **Model:** `query()` (canonical static builder entry), `toArray()`, - `getPrefix()`, `withCast(array $casts)`, `bool`/`boolean` cast. + `getPrefix()`, `getTablePrefix()` (full table prefix — `wp_` + plugin prefix — + for join/pivot table names), `withCast(array $casts)`, `bool`/`boolean` cast. - **Connection:** `startTransaction()`, `commit()`, `rollback()`. - **Blueprint:** `unique($column = null)` — optional arg (backward compatible) for composite/explicit unique indexes. diff --git a/docs/usage.md b/docs/usage.md index d02b6e7..8779f7b 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -154,6 +154,10 @@ $created = Contact::insert([ `save()` inserts when the model is new and updates (dirty attributes only) when it already exists. On success it returns the model; on failure, `false`. +Array and object values are JSON-encoded (`wp_json_encode`) automatically across +`save()`, `update()`, single-row and bulk `insert()`, and `upsert()` — pass the +raw array/object, not a pre-encoded string. + --- ## Reading records @@ -268,6 +272,10 @@ Contact::query() // also: rightJoin(), fullJoin(), crossJoin(), on(), orOn() ``` +Pass **unprefixed** table names — `join()` prepends the model's full table prefix +(the same one the model's own table uses, including `wp_` for models with a custom +`$prefix`). Qualify the `ON` columns as `table.column`. + ### Limit / offset / pagination ```php @@ -739,9 +747,6 @@ method relocation. `RuntimeException` (the pivot metadata has no single `foreignKey`/`localKey`). - No `attach`/`detach`/`sync` (write side is out of scope). - Single-column pivot/parent/related keys only — no composite keys. - - A non-empty model `$prefix` combined with a pivot table is unsupported (the - inherited `join()` prefixing quirk mis-prefixes the pivot table). The - default empty-`$prefix` case is correct. - Duplicate pivot rows yield duplicate related models (no `DISTINCT`). - Eager constraint closures may add `where`/`orderBy`/`limit` but **cannot** narrow the selected columns — the pivot path always selects `related.*` so From a43e80ea2add60b440cc7d650d8fff13aafd4495 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 16:37:24 +0600 Subject: [PATCH 14/32] fix: select() handles `column AS alias` without back-ticking the alias prepareColumnName() wrapped the whole "title AS t" string as a single identifier (`table`.`title AS t`), so select(['col AS alias']) produced "Unknown column 'table.col AS alias'" errors. Split on ` AS ` (case-insensitive): qualify the column, keep the alias as a separate back-ticked identifier -> `table`.`col` AS `alias`. Plain/dotted columns and `*` are unchanged; raw expressions and function calls still need selectRaw(). Assisted-By: AI --- docs/breaking-changes.md | 12 ++++++++---- docs/usage.md | 6 ++++-- src/QueryBuilder.php | 4 ++++ tests/QueryFeaturesTest.php | 12 ++++++++++++ 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index 752c86a..d0f42f7 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -128,13 +128,17 @@ which wraps the name as `` `table`.`column` `` unless it already contains a `.`. ->select('COUNT(*) as total') // emitted: `COUNT(*) as total` ❌ invalid SQL ``` -**Why it breaks:** any raw SQL expression, function call, or alias passed to -`select()` is now quoted as a single identifier. +**Why it breaks:** a raw SQL expression or function call passed to `select()` is +quoted as a single identifier. -**Migration:** use `selectRaw()` for expressions; plain columns need no change: +A plain `column AS alias` **is** handled — `prepareColumnName()` qualifies the +column and keeps the alias separate, so `->select(['id', 'title AS t'])` emits +`` `table`.`id`, `table`.`title` AS `t` ``. Only expressions/function calls +(`COUNT(*)`, `SUM(amount) AS amt`, …) still need `selectRaw()`: ```php -->selectRaw('COUNT(*) as total') +->select(['title AS t']) // ✅ simple column alias +->selectRaw('COUNT(*) as total') // expressions / functions ->selectRaw('SUM(amount) as amt', $bindings) ``` diff --git a/docs/usage.md b/docs/usage.md index 8779f7b..2433bd9 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -198,13 +198,15 @@ inspect the SQL without executing. ```php Contact::select('id', 'email')->get(); Contact::select(['id', 'email'])->get(); +Contact::select(['id', 'title AS t'])->get(); // column alias — qualifies `title`, keeps AS `t` Contact::addSelect('phone')->get(); // add to existing select Contact::selectRaw('COUNT(*) as total')->get(); // raw expression (NOT select()) Contact::selectRaw('SUM(amount) as amt', [])->get(); ``` -> Pass raw SQL expressions to `selectRaw()`, not `select()` — `select()` -> back-tick-quotes its arguments as column identifiers. +> `select()` handles plain columns and `column AS alias`, back-tick-quoting them +> as identifiers. Pass raw SQL expressions or function calls (`COUNT(*)`, …) to +> `selectRaw()` instead. ### Where diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index 6791e12..0c6b45e 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -361,6 +361,10 @@ public function all($columns = ['*']) public function prepareColumnName(string $column) { + if (preg_match('/^(.+?)\s+as\s+(.+)$/i', $column, $matches)) { + return $this->prepareColumnName(trim($matches[1])) . ' AS `' . trim($matches[2], " `") . '`'; + } + if (strpos($column, '.') !== false) { return $column; } diff --git a/tests/QueryFeaturesTest.php b/tests/QueryFeaturesTest.php index d66d6a8..b0bb66a 100644 --- a/tests/QueryFeaturesTest.php +++ b/tests/QueryFeaturesTest.php @@ -24,6 +24,18 @@ protected function tearDown(): void $GLOBALS['wpdb'] = new FakeWpdb(); } + // --- Select -------------------------------------------------------------- + + public function testSelectColumnAliasQualifiesColumnNotWholeExpression(): void + { + $sql = (new User())->select(['id', 'name AS n'])->toSql(); + + // the column is qualified/back-ticked, the alias kept separate + $this->assertStringContainsString('`name` AS `n`', $sql); + // the whole "name AS n" must NOT be treated as one column name + $this->assertStringNotContainsString('`name AS n`', $sql); + } + // --- Joins --------------------------------------------------------------- public function testInnerJoinCompilesWithOnClause(): void From 08b8384f5a3ffb29685407537d2aeca8cef271f1 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 18:14:34 +0600 Subject: [PATCH 15/32] fix: eager-load key subquery ignores parent selectRaw with() builds `foreignKey IN ( SELECT * FROM () AS subquery )` from a clone of the parent query narrowed to the local key. select() resets the select list but not selectRaw, so a parent selectRaw('... as x') leaked a second column into the subquery -> MySQL "Operand should contain 1 column(s)". Strip selectRaw on the key-subquery clone via a shared prepareKeySubquery() helper, used by both the hasMany/belongsTo and pivot eager paths. Byte-identical when the parent has no selectRaw. Assisted-By: AI --- src/Concerns/Relations.php | 23 +++++++++++++++++++---- tests/EagerLoadIntegrationTest.php | 18 ++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/Concerns/Relations.php b/src/Concerns/Relations.php index 0ba2271..4012c42 100644 --- a/src/Concerns/Relations.php +++ b/src/Concerns/Relations.php @@ -243,6 +243,23 @@ private function getRelationKeys($foreignKey, $localKey) return [$foreignKey, $localKey]; } + /** + * Compiles the parent query as a single-column key subquery for a relation's + * IN (...) constraint. Strips the parent's selectRaw — that subquery must + * return exactly the key column, not the caller's extra raw expressions. + * + * @param string $keyColumn + * + * @return string + */ + private function prepareKeySubquery(QueryBuilder $query, $keyColumn): string + { + $keyQuery = clone $query; + $keyQuery->selectRaw = ['columns' => [], 'bindings' => []]; + + return $keyQuery->select($keyColumn)->prepare(); + } + private function retrieveRelateData(QueryBuilder $query) { $relations = $this->getRelations(); @@ -255,13 +272,12 @@ private function retrieveRelateData(QueryBuilder $query) continue; } - $parentQuery = clone $query; $relationKey = $relationQuery->getModel()->getActiveRelationKey(); $relationQuery->whereRaw( $relationKey['foreignKey'] . ' IN ( SELECT * FROM (' - . $parentQuery->select($relationKey['localKey'])->prepare() + . $this->prepareKeySubquery($query, $relationKey['localKey']) . ') AS subquery )' ); @@ -280,12 +296,11 @@ private function retrieveRelateData(QueryBuilder $query) private function retrievePivotRelateData($relationName, QueryBuilder $relationQuery, QueryBuilder $query) { [$pivot, $pivotRef, $bucketAlias] = $this->applyPivotSelectAndJoin($relationQuery); - $parentQuery = clone $query; $relationQuery->whereRaw( $pivotRef . '.' . $pivot['foreignPivotKey'] . ' IN ( SELECT * FROM (' - . $parentQuery->select($pivot['parentKey'])->prepare() + . $this->prepareKeySubquery($query, $pivot['parentKey']) . ') AS subquery )' ); diff --git a/tests/EagerLoadIntegrationTest.php b/tests/EagerLoadIntegrationTest.php index 16a0828..0c45a06 100644 --- a/tests/EagerLoadIntegrationTest.php +++ b/tests/EagerLoadIntegrationTest.php @@ -52,4 +52,22 @@ public function testStaticWithEagerLoadsAndGroupsRelatedRows(): void $this->assertCount(1, $second->posts, 'user 2 should have 1 post'); $this->assertInstanceOf(Post::class, $first->posts[0]); } + + /** + * The parent's selectRaw must NOT leak into the eager key subquery — that + * subquery feeds an IN (...) and must return exactly one column (the localKey), + * else MySQL raises "Operand should contain 1 column(s)". + */ + public function testEagerLoadKeySubqueryIgnoresParentSelectRaw(): void + { + User::select(['id'])->selectRaw('CONCAT("X", id) as cx')->with('posts')->get(); + + $postsSql = $GLOBALS['wpdb']->queries[1]; + + $this->assertStringNotContainsString('CONCAT', $postsSql); + $this->assertStringContainsString( + 'IN ( SELECT * FROM (SELECT `wp_users`.`id` FROM wp_users', + $postsSql + ); + } } From acc365227a9b0d7afd955abd4ce822d51a9d2619 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 19:02:38 +0600 Subject: [PATCH 16/32] fix: drop ORDER BY from eager-load key subquery (keep when LIMIT-bound) The `foreignKey IN ( SELECT key ... )` eager subquery cloned the parent and stripped selectRaw but kept ORDER BY. A parent ordered by a selectRaw alias (orderBy('dsc') after selectRaw('... as dsc')) then referenced a column absent from the narrowed subquery -> "Unknown column in order clause"; and ORDER BY is meaningless for set membership regardless. Move prepareKeySubquery onto QueryBuilder (so it can reset the protected orderBy) and drop ORDER BY unless a LIMIT pins which parent rows the set comes from. Byte-identical for parents without selectRaw/orderBy. Assisted-By: AI --- src/Concerns/Relations.php | 21 ++------------------- src/QueryBuilder.php | 22 ++++++++++++++++++++++ tests/EagerLoadIntegrationTest.php | 29 +++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 19 deletions(-) diff --git a/src/Concerns/Relations.php b/src/Concerns/Relations.php index 4012c42..71d2533 100644 --- a/src/Concerns/Relations.php +++ b/src/Concerns/Relations.php @@ -243,23 +243,6 @@ private function getRelationKeys($foreignKey, $localKey) return [$foreignKey, $localKey]; } - /** - * Compiles the parent query as a single-column key subquery for a relation's - * IN (...) constraint. Strips the parent's selectRaw — that subquery must - * return exactly the key column, not the caller's extra raw expressions. - * - * @param string $keyColumn - * - * @return string - */ - private function prepareKeySubquery(QueryBuilder $query, $keyColumn): string - { - $keyQuery = clone $query; - $keyQuery->selectRaw = ['columns' => [], 'bindings' => []]; - - return $keyQuery->select($keyColumn)->prepare(); - } - private function retrieveRelateData(QueryBuilder $query) { $relations = $this->getRelations(); @@ -277,7 +260,7 @@ private function retrieveRelateData(QueryBuilder $query) $relationQuery->whereRaw( $relationKey['foreignKey'] . ' IN ( SELECT * FROM (' - . $this->prepareKeySubquery($query, $relationKey['localKey']) + . $query->prepareKeySubquery($relationKey['localKey']) . ') AS subquery )' ); @@ -300,7 +283,7 @@ private function retrievePivotRelateData($relationName, QueryBuilder $relationQu $relationQuery->whereRaw( $pivotRef . '.' . $pivot['foreignPivotKey'] . ' IN ( SELECT * FROM (' - . $this->prepareKeySubquery($query, $pivot['parentKey']) + . $query->prepareKeySubquery($pivot['parentKey']) . ') AS subquery )' ); diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index 0c6b45e..9dac705 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -396,6 +396,28 @@ public function select($columns = ['*']) return $this; } + /** + * Compiles this query as a single-column key subquery for a relation's + * IN (...) constraint: only $keyColumn is projected. Strips selectRaw (extra + * columns would break the operand count) and, unless a LIMIT pins the set, + * ORDER BY (meaningless for set membership and may reference a stripped raw + * select alias). + * + * @param string $keyColumn + * + * @return string + */ + public function prepareKeySubquery($keyColumn): string + { + $clone = clone $this; + $clone->selectRaw = ['columns' => [], 'bindings' => []]; + if (!isset($clone->limit)) { + $clone->orderBy = []; + } + + return $clone->select($keyColumn)->prepare(); + } + /** * Adds column to select list * diff --git a/tests/EagerLoadIntegrationTest.php b/tests/EagerLoadIntegrationTest.php index 0c45a06..24f7bf2 100644 --- a/tests/EagerLoadIntegrationTest.php +++ b/tests/EagerLoadIntegrationTest.php @@ -70,4 +70,33 @@ public function testEagerLoadKeySubqueryIgnoresParentSelectRaw(): void $postsSql ); } + + /** + * ORDER BY is meaningless in a value-list IN ( SELECT key ... ) and may + * reference a stripped selectRaw alias — so it is dropped from the key + * subquery when no LIMIT pins the set. + */ + public function testEagerLoadKeySubqueryDropsParentOrderBy(): void + { + User::select(['id'])->selectRaw('CONCAT("X", id) as cx')->orderBy('cx', 'DESC')->with('posts')->get(); + + $postsSql = $GLOBALS['wpdb']->queries[1]; + + $this->assertStringNotContainsString('ORDER BY', $postsSql); + $this->assertStringNotContainsString('cx', $postsSql); + } + + /** + * When a LIMIT pins which parent rows the set comes from, ORDER BY is kept + * so the limited set stays deterministic. + */ + public function testEagerLoadKeySubqueryKeepsOrderByWhenLimited(): void + { + User::orderBy('id', 'DESC')->take(5)->with('posts')->get(); + + $postsSql = $GLOBALS['wpdb']->queries[1]; + + $this->assertStringContainsString('ORDER BY', $postsSql); + $this->assertStringContainsString('LIMIT 5', $postsSql); + } } From 234c80509425e71e33cb076690ac43028056ff39 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 19:25:55 +0600 Subject: [PATCH 17/32] test: edge-case characterization for where/select/join/aggregate/relations 41 tests locking verified current behavior (no source change): - WhereClauseEdgeTest: whereIn type-placeholders, implicit-IN array value, IS NULL, falsy values not dropped, operator/LIKE variants, whereBetween, nested-closure binding order, whereRaw/selectRaw/having binding order, soft-delete scope binding order through the paren-wrap. - SelectJoinAggregateEdgeTest: column-alias variants (dotted/lowercase/ backticked/spaced/multi-as), RIGHT/FULL/CROSS join, on()/orOn() chaining, custom-$prefix join ON qualification, count()=COUNT(pk), max/min empty=null, aggregate-runs-on-clone, asc() pk fallback. - RelationEagerEdgeTest: eager/whereHas/withCount constraint closures, multiple whereHas, withCount alias, select+withCount no dup, relation alias attach, parent filter in eager subquery, eager-on-first(). Assisted-By: AI --- tests/RelationEagerEdgeTest.php | 135 +++++++++++++++++++++ tests/SelectJoinAggregateEdgeTest.php | 152 +++++++++++++++++++++++ tests/WhereClauseEdgeTest.php | 166 ++++++++++++++++++++++++++ 3 files changed, 453 insertions(+) create mode 100644 tests/RelationEagerEdgeTest.php create mode 100644 tests/SelectJoinAggregateEdgeTest.php create mode 100644 tests/WhereClauseEdgeTest.php diff --git a/tests/RelationEagerEdgeTest.php b/tests/RelationEagerEdgeTest.php new file mode 100644 index 0000000..0e22a76 --- /dev/null +++ b/tests/RelationEagerEdgeTest.php @@ -0,0 +1,135 @@ + 10, 'user_id' => 1], + (object) ['id' => 11, 'user_id' => 1], + (object) ['id' => 12, 'user_id' => 2], + ]; + } + + return [(object) ['id' => 1], (object) ['id' => 2]]; + }; + } + + public function testWhereHasClosureConstrainsExistsSubquery(): void + { + $sql = User::whereHas('posts', static function ($q) { + $q->where('status', 'published'); + })->toSql(); + + $this->assertStringContainsString('exists(', $sql); + $this->assertStringContainsString('`wp_posts`.`status`', $sql); + $this->assertStringContainsString('`wp_users`.`id`=`wp_posts`.`user_id`', $sql); + } + + public function testMultipleWhereHasAreAnded(): void + { + $sql = User::whereHas('posts')->whereHas('posts')->toSql(); + + $this->assertSame(2, substr_count($sql, 'exists(')); + } + + public function testWithCountClosureNarrowsCountSubquery(): void + { + $sql = User::withCount(['posts' => static function ($q) { + $q->where('status', 'published'); + }])->toSql(); + + $this->assertStringContainsString('count(*)', $sql); + $this->assertStringContainsString('`wp_posts`.`status`', $sql); + $this->assertStringContainsString('as `posts_count`', $sql); + } + + public function testWithCountAliasNamesTheColumn(): void + { + $this->assertStringContainsString('as `c`', User::withCount('posts as c')->toSql()); + } + + public function testSelectThenWithCountDoesNotDuplicateBaseSelect(): void + { + $sql = User::select(['id'])->withCount('posts')->toSql(); + + $this->assertStringContainsString('`wp_users`.`id`, (SELECT count(*)', $sql); + $this->assertStringNotContainsString('`wp_users`.*', $sql); + } + + public function testEagerClosureConstraintFiltersTheRelationQuery(): void + { + $GLOBALS['wpdb']->resolver = $this->eagerResolver(); + + User::with(['posts' => static function ($q) { + $q->where('status', 'published'); + }])->get(); + + $postsSql = $GLOBALS['wpdb']->queries[1]; + + $this->assertStringContainsString('`wp_posts`.`status`', $postsSql); + $this->assertStringContainsString('IN ( SELECT * FROM (', $postsSql); + } + + public function testParentWherePropagatesIntoEagerSubquery(): void + { + $GLOBALS['wpdb']->resolver = $this->eagerResolver(); + + User::where('status', 'active')->with('posts')->get(); + + $postsSql = $GLOBALS['wpdb']->queries[1]; + + $this->assertStringContainsString('`wp_users`.`status`', $postsSql); + $this->assertStringContainsString('AS subquery', $postsSql); + } + + public function testEagerRelationAliasAttachesUnderAlias(): void + { + $GLOBALS['wpdb']->resolver = $this->eagerResolver(); + + $users = User::with('posts as recent')->get(); + + // eager-attached relations are plain arrays (lazy access returns a Collection). + $this->assertCount(2, $users[0]->recent); + $this->assertSame(10, $users[0]->recent[0]->id); + } + + public function testEagerLoadOnFirstAttachesRelation(): void + { + $GLOBALS['wpdb']->resolver = static function ($sql) { + if (strpos($sql, 'wp_posts') !== false) { + return [(object) ['id' => 10, 'user_id' => 1]]; + } + + return [(object) ['id' => 1]]; + }; + + $user = User::with('posts')->where('id', 1)->first(); + + $this->assertCount(1, $user->posts); + $this->assertSame(10, $user->posts[0]->id); + } +} diff --git a/tests/SelectJoinAggregateEdgeTest.php b/tests/SelectJoinAggregateEdgeTest.php new file mode 100644 index 0000000..089d97a --- /dev/null +++ b/tests/SelectJoinAggregateEdgeTest.php @@ -0,0 +1,152 @@ +assertStringContainsString('t.col AS `a`', (new User())->select(['t.col AS a'])->toSql()); + } + + public function testLowercaseAsNormalisesToUppercase(): void + { + $this->assertStringContainsString('`wp_users`.`col` AS `a`', (new User())->select(['col as a'])->toSql()); + } + + public function testAlreadyBacktickedAliasIsNotDoubleQuoted(): void + { + $this->assertStringContainsString('`wp_users`.`col` AS `a`', (new User())->select(['col AS `a`'])->toSql()); + } + + public function testAliasWithSpacesIsBacktickedWhole(): void + { + $this->assertStringContainsString('`wp_users`.`col` AS `the alias`', (new User())->select(['col AS the alias'])->toSql()); + } + + public function testMultipleAsSplitsOnFirst(): void + { + $this->assertStringContainsString('`wp_users`.`a` AS `b as c`', (new User())->select(['a as b as c'])->toSql()); + } + + public function testSelectThenSelectRawJoinedByComma(): void + { + $this->assertStringContainsString('`wp_users`.`id`, COUNT(*) as c', (new User())->select(['id'])->selectRaw('COUNT(*) as c')->toSql()); + } + + public function testEmptySelectEmitsQualifiedNothing(): void + { + $this->assertStringContainsString('SELECT FROM wp_users', (new User())->toSql()); + } + + public function testPrepareColumnNameStarStaysQualifiedStar(): void + { + $this->assertSame('`wp_users`.*', (new User())->prepareColumnName('*')); + } + + // --- Joins --------------------------------------------------------------- + + public function testRightFullCrossJoinKeywords(): void + { + $this->assertStringContainsString('RIGHT JOIN wp_posts', (new User())->rightJoin('posts', 'posts.user_id', '=', 'users.id')->toSql()); + $this->assertStringContainsString('FULL JOIN wp_posts', (new User())->fullJoin('posts', 'posts.user_id', '=', 'users.id')->toSql()); + $this->assertStringContainsString('CROSS JOIN wp_posts', (new User())->crossJoin('posts', 'posts.user_id', '=', 'users.id')->toSql()); + } + + public function testOnAndOrOnAppendToSameJoin(): void + { + $and = (new User())->join('posts', 'posts.user_id', '=', 'users.id')->on('posts.status', '=', 'users.state')->toSql(); + $this->assertStringContainsString('posts.user_id = users.id AND posts.status = users.state', $and); + + $or = (new User())->join('posts', 'posts.user_id', '=', 'users.id')->orOn('posts.status', '=', 'users.state')->toSql(); + $this->assertStringContainsString('posts.user_id = users.id OR posts.status = users.state', $or); + } + + public function testJoinOnCustomPrefixModelQualifiesBaseColumnWithFullPrefix(): void + { + $sql = (new PrefixedModel())->join('gadgets', 'gid', '=', 'wid')->toSql(); + + $this->assertStringContainsString('INNER JOIN wp_crm_gadgets', $sql); + $this->assertStringContainsString('`wp_crm_widgets`.`gid` = wp_crm_gadgets.wid', $sql); + } + + // --- Aggregates / ordering ---------------------------------------------- + + public function testCountCompilesCountOfQualifiedPrimaryKey(): void + { + $GLOBALS['wpdb']->resolver = static function () { + return [(object) ['COUNT' => '7']]; + }; + + $result = (new User())->count(); + + $this->assertSame(7, $result); + $this->assertStringContainsString('COUNT(`wp_users`.`id`) as COUNT', $GLOBALS['wpdb']->last_query); + } + + public function testMaxAndMinReturnNullOnEmptyResultSet(): void + { + $GLOBALS['wpdb']->resolver = static function () { + return []; + }; + + $this->assertNull((new User())->max('score')); + $this->assertNull((new User())->min('score')); + } + + public function testAggregateRunsOnCloneWithoutMutatingSelect(): void + { + $GLOBALS['wpdb']->resolver = static function () { + return [(object) ['MAX' => 9]]; + }; + + $query = (new User())->select(['id', 'name']); + $before = $query->toSql(); + $query->max('score'); + + $this->assertSame($before, $query->toSql()); + } + + public function testAggregateCarriesWhereClause(): void + { + $GLOBALS['wpdb']->resolver = static function () { + return [(object) ['COUNT' => '3']]; + }; + + (new User())->where('active', 1)->count(); + + $this->assertStringContainsString('WHERE', $GLOBALS['wpdb']->last_query); + $this->assertStringContainsString('`wp_users`.`active`', $GLOBALS['wpdb']->last_query); + } + + public function testAscFallsBackToPrimaryKey(): void + { + $this->assertStringContainsString('ORDER BY id ASC', (new User())->asc()->toSql()); + } + + public function testSkipWithoutTakeOmitsOffset(): void + { + $this->assertStringNotContainsString('OFFSET', (new User())->skip(20)->toSql()); + } +} diff --git a/tests/WhereClauseEdgeTest.php b/tests/WhereClauseEdgeTest.php new file mode 100644 index 0000000..9a7cdab --- /dev/null +++ b/tests/WhereClauseEdgeTest.php @@ -0,0 +1,166 @@ +toSql(); + + return $qb->getBindings(); + } + + public function testWhereInTypesPlaceholdersPerElement(): void + { + $qb = (new User())->whereIn('id', [1, 'a', 2.5]); + + $this->assertStringContainsString('`wp_users`.`id` IN (%d,%s,%f)', $qb->toSql()); + $this->assertSame([1, 'a', 2.5], $qb->getBindings()); + } + + public function testWhereInSingleElement(): void + { + $qb = (new User())->whereIn('id', [5]); + + $this->assertStringContainsString('`wp_users`.`id` IN (%d)', $qb->toSql()); + $this->assertSame([5], $qb->getBindings()); + } + + public function testWhereInAssocDropsKeys(): void + { + $qb = (new User())->whereIn('id', ['x' => 1, 'y' => 2]); + + $this->assertStringContainsString('IN (%d,%d)', $qb->toSql()); + $this->assertSame([1, 2], $qb->getBindings()); + } + + public function testWhereWithArrayValueIsImplicitIn(): void + { + $qb = (new User())->where('id', [1, 2, 3]); + + // the 2-arg array path produces "IN (" (two spaces) — distinct from whereIn's "IN (". + $this->assertStringContainsString('`wp_users`.`id` IN (%d,%d,%d)', $qb->toSql()); + $this->assertSame([1, 2, 3], $qb->getBindings()); + } + + public function testWhereNullEmitsIsNull(): void + { + $qb = (new User())->where('id', null); + + $this->assertStringContainsString('`wp_users`.`id` IS NULL', $qb->toSql()); + $this->assertSame([], $qb->getBindings()); + } + + public function testWhereNullAndNotNullHelpers(): void + { + $this->assertStringContainsString('`wp_users`.`deleted_at` IS NULL', (new User())->whereNull('deleted_at')->toSql()); + $this->assertStringContainsString('`wp_users`.`deleted_at` IS NOT NULL', (new User())->whereNotNull('deleted_at')->toSql()); + } + + public function testFalsyValuesAreNotDropped(): void + { + $this->assertSame([0], $this->bindings((new User())->where('id', 0))); + $this->assertSame([false], $this->bindings((new User())->where('active', false))); + $this->assertSame(['0'], $this->bindings((new User())->where('id', '0'))); + $this->assertSame([''], $this->bindings((new User())->where('id', ''))); + } + + public function testOperatorVariantsPassThrough(): void + { + $this->assertStringContainsString('`wp_users`.`id` != %d', (new User())->where('id', '!=', 5)->toSql()); + $this->assertStringContainsString('`wp_users`.`id` <> %d', (new User())->where('id', '<>', 5)->toSql()); + $this->assertStringContainsString('`wp_users`.`id` >= %d', (new User())->where('id', '>=', 5)->toSql()); + } + + public function testLikeUppercaseAndNotLike(): void + { + $this->assertStringContainsString('`wp_users`.`name` LIKE %s', (new User())->where('name', 'LIKE', '%a%')->toSql()); + $this->assertStringContainsString('`wp_users`.`name` NOT LIKE %s', (new User())->where('name', 'NOT LIKE', '%a%')->toSql()); + } + + public function testWhereBetweenAndOrWhereBetween(): void + { + $qb = (new User())->whereBetween('age', 18, 65); + $this->assertStringContainsString('(age BETWEEN %d AND %d)', $qb->toSql()); + $this->assertSame([18, 65], $qb->getBindings()); + + $qb2 = (new User())->where('id', 1)->orWhereBetween('age', 18, 65); + $this->assertStringContainsString('OR (age BETWEEN %d AND %d)', $qb2->toSql()); + $this->assertSame([1, 18, 65], $qb2->getBindings()); + } + + public function testNestedClosureFirstReordersBindingsToPlaceholderOrder(): void + { + $qb = (new User()) + ->where(static function ($q) { + $q->where('b', 2)->orWhere('c', 3); + }) + ->where('a', 1); + + $this->assertStringContainsString('( `wp_users`.`b` = %d OR `wp_users`.`c` = %d) AND `wp_users`.`a` = %d', $qb->toSql()); + $this->assertSame([2, 3, 1], $qb->getBindings()); + } + + public function testNestedClosureWithExplicitOrBool(): void + { + $qb = (new User()) + ->where('a', 1) + ->where(static function ($q) { + $q->where('b', 2); + }, 'OR'); + + $this->assertStringContainsString('`wp_users`.`a` = %d OR ( `wp_users`.`b` = %d)', $qb->toSql()); + $this->assertSame([1, 2], $qb->getBindings()); + } + + public function testWhereRawBindingOrder(): void + { + $this->assertSame([99, 5], $this->bindings((new User())->whereRaw('x = %d', [99])->where('id', 5))); + $this->assertSame([5, 99], $this->bindings((new User())->where('id', 5)->whereRaw('x = %d', [99]))); + $this->assertSame([5, 99], $this->bindings((new User())->where('id', 5)->orWhereRaw('x = %d', [99]))); + } + + public function testSelectRawThenWhereThenHavingBindingOrder(): void + { + $qb = (new User()) + ->selectRaw('%s as label', ['L']) + ->where('id', 5) + ->groupBy('status') + ->having('cnt', '>', 3); + + $this->assertSame(['L', 5, 3], $this->bindings($qb)); + } + + public function testSoftDeleteScopeKeepsUserBindingOrderThroughParenWrap(): void + { + $qb = ScopedSoftPost::query()->where('a', 1)->orWhere('b', 2); + + $sql = $qb->toSql(); + $this->assertStringContainsString('( `wp_scoped_soft_posts`.`a` = %d OR `wp_scoped_soft_posts`.`b` = %d)', $sql); + $this->assertStringContainsString('`deleted_at` IS NULL', $sql); + $this->assertSame([1, 2], $qb->getBindings()); + } +} From 42ba9b99d2d16bfb2923c26f5e216e6b533c5353 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 23:25:24 +0600 Subject: [PATCH 18/32] fix: repair insert/where/aggregate/soft-delete edge cases (zero-BC Phase 1) Pure repairs of crashing / invalid-SQL paths plus additive forceDelete/restore; byte-identical for every currently-working input (momus-reviewed). - insert(): route to bulk only for a true list-of-rows, so a single row whose first value is an array no longer crashes via ksort(). - where/whereIn: null + explicit operator -> IS [NOT] NULL; empty array -> the false constant `0 = 1` (was invalid `IN ()`); a nested IN element -> one JSON-encoded placeholder; object value -> wp_json_encode (was fatal on prepare). - empty save() (no dirty columns) skips the malformed `UPDATE ... SET` and returns the model (fires saving/saved). - insert([]) / empty bulk rows / upsert($v, []) no longer emit malformed SQL. - aggregate(fn, '*') -> COUNT(*) (was invalid COUNT(`t`.*)); count() unchanged. - forceDelete() (real DELETE bypassing the soft scope) and restore() on soft-delete models. - take()/skip() cast to int -> blocks limit/offset injection. - eager key subquery also strips the parent's groupBy/having. Assisted-By: AI --- src/Model.php | 2 + src/QueryBuilder.php | 184 ++++++++++++++++++++++++- tests/EagerKeySubqueryScopeFixTest.php | 43 ++++++ tests/QueryClauseEdgeFixTest.php | 110 +++++++++++++++ tests/SoftDeleteRestoreTest.php | 51 +++++++ tests/WriteEdgeFixTest.php | 111 +++++++++++++++ 6 files changed, 494 insertions(+), 7 deletions(-) create mode 100644 tests/EagerKeySubqueryScopeFixTest.php create mode 100644 tests/QueryClauseEdgeFixTest.php create mode 100644 tests/SoftDeleteRestoreTest.php create mode 100644 tests/WriteEdgeFixTest.php diff --git a/src/Model.php b/src/Model.php index b0c05f3..acd4fbe 100644 --- a/src/Model.php +++ b/src/Model.php @@ -86,6 +86,8 @@ * @method static mixed max($column) * @method static mixed min($column) * @method static bool|string delete() + * @method static string|bool forceDelete() + * @method static string|bool|Model restore() * @method static string toSql() * @method static string prepare($sql = null) * @method static QueryBuilder withTrashed() diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index 9dac705..b075542 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -81,6 +81,8 @@ class QueryBuilder private $_onlyTrashed = false; + private $_forceDelete = false; + /** * Constructs QueryBuilder * @@ -411,6 +413,8 @@ public function prepareKeySubquery($keyColumn): string { $clone = clone $this; $clone->selectRaw = ['columns' => [], 'bindings' => []]; + $clone->groupBy = []; + $clone->having = []; if (!isset($clone->limit)) { $clone->orderBy = []; } @@ -600,6 +604,14 @@ public function orWhereRaw($sql, $bindings = []) */ public function whereIn($column, $value) { + if (\is_array($value)) { + if ($value === []) { + return $this->whereRaw('0 = 1'); + } + + $value = $this->sanitizeInValues($value); + } + $this->where[] = [ 'column' => $column, 'value' => $value, @@ -1023,7 +1035,7 @@ public function prepareRaw() */ public function take($count) { - $this->limit = $count; + $this->limit = (int) $count; return $this; } @@ -1037,7 +1049,7 @@ public function take($count) */ public function skip($count) { - $this->offset = $count; + $this->offset = (int) $count; return $this; } @@ -1051,7 +1063,11 @@ public function skip($count) */ public function insert($attributes = []) { - if (\is_array(reset($attributes))) { + if (empty($attributes)) { + return false; + } + + if ($this->isListOfRows($attributes)) { return $this->bulkInsert($attributes); } @@ -1111,6 +1127,12 @@ public function save() $columns = $this->prepareAttributeForSaveOrUpdate($this->_model->exists()); $pk = $this->_model->getPrimaryKey(); if ($this->_model->exists()) { + if (empty($columns)) { + $this->_model->fireEvent('saved'); + + return $this->_model; + } + $isPkExistsInWhere = false; $pkValue = $this->_model->getAttribute($pk); @@ -1172,7 +1194,8 @@ public function aggregate($function, $column) $query = $this->clone(); $query->select = []; $query->selectRaw = ['columns' => [], 'bindings' => []]; - $result = $query->selectRaw($function . '(' . $query->prepareColumnName($column) . ') as ' . $function)->exec(); + $preparedColumn = $column === '*' ? '*' : $query->prepareColumnName($column); + $result = $query->selectRaw($function . '(' . $preparedColumn . ') as ' . $function)->exec(); return \is_array($result) && isset($result[0]->{$function}) ? $result[0]->{$function} : null; } @@ -1187,6 +1210,37 @@ public function delete() return $this->exec(); } + /** + * Permanently deletes the targeted rows of a soft-delete model, bypassing + * the soft-delete rewrite (emits a real DELETE). + * + * @return string|bool + */ + public function forceDelete() + { + if (!$this->isSoftDeleteModel()) { + throw new RuntimeException('forceDelete() is only available on soft-delete models.'); + } + + $this->_forceDelete = true; + + return $this->delete(); + } + + /** + * Restores soft-deleted rows by nulling deleted_at and persisting. + * + * @return string|bool|Model + */ + public function restore() + { + if (!$this->isSoftDeleteModel()) { + throw new RuntimeException('restore() is only available on soft-delete models.'); + } + + return $this->update(['deleted_at' => null]); + } + /** * Starts transaction * @@ -1299,7 +1353,7 @@ public function upsert(array $values, ?array $update = null) $values = [$values]; } - if (\is_null($update)) { + if (empty($update)) { $update = array_keys($values[0]); } @@ -1442,7 +1496,7 @@ protected function prepareConditional($params, $bool = 'AND', $type = 'where') $conditions['bool'] = $params[3]; } - return $conditions; + return $this->normalizeConditions($conditions, $type); } /** @@ -1540,6 +1594,118 @@ protected function getTimeZone() return $timezoneString; } + /** + * Coerces each IN-list element to a scalar so it maps to exactly one + * placeholder and one binding (a nested array/object is JSON-encoded, never + * flattened into multiple bindings). + * + * @param array $values + * + * @return array + */ + private function sanitizeInValues(array $values) + { + return array_map( + function ($value) { + if (\is_array($value) || \is_object($value)) { + return wp_json_encode($value); + } + + return $value; + }, + $values + ); + } + + /** + * True only when $attributes is a non-empty positional list whose every + * element is itself an array (a list of rows for bulk insert). An assoc row + * whose first value happens to be an array must take the single-row path. + * + * @param mixed $attributes + * + * @return bool + */ + private function isListOfRows($attributes) + { + if (!\is_array($attributes) || $attributes === []) { + return false; + } + + if (array_keys($attributes) !== range(0, \count($attributes) - 1)) { + return false; + } + + foreach ($attributes as $row) { + if (!\is_array($row)) { + return false; + } + } + + return true; + } + + /** + * Repairs where-clause values that would otherwise emit invalid SQL or fatal + * on prepare, independent of arity: an empty array becomes the false + * constant `0 = 1`; a null value with an explicit operator becomes + * IS [NOT] NULL; a non-empty array has its elements coerced to scalars; an + * object value is JSON-encoded. Having clauses are left untouched. + * + * @param array $conditions + * @param string $type + * + * @return array + */ + private function normalizeConditions(array $conditions, $type) + { + if ($type !== 'where' || !\array_key_exists('value', $conditions)) { + return $conditions; + } + + $value = $conditions['value']; + $bool = isset($conditions['bool']) ? $conditions['bool'] : 'AND'; + + if (\is_array($value)) { + if ($value === []) { + return ['bool' => $bool, 'raw' => '0 = 1', 'bindings' => []]; + } + + $conditions['value'] = $this->sanitizeInValues($value); + + return $conditions; + } + + if (\is_null($value)) { + if (isset($conditions['operator'])) { + unset($conditions['value']); + $conditions['operator'] = $this->nullOperator($conditions['operator']); + } + + return $conditions; + } + + if (\is_object($value)) { + $conditions['value'] = wp_json_encode($value); + } + + return $conditions; + } + + /** + * Maps a comparison operator to its null-safe form for a null value. + * + * @param string $operator + * + * @return string + */ + private function nullOperator($operator) + { + $negations = ['!=', '<>', 'NOT', 'IS NOT', 'IS NOT NULL']; + + return \in_array($operator, $negations, true) ? 'IS NOT NULL' : 'IS NULL'; + } + /** * Returns true when the model declares soft-delete support. * @@ -1570,6 +1736,10 @@ private function autoScopeEnabled() private function bulkInsert($attributes) { $firstRow = reset($attributes); + if (empty($firstRow)) { + return new Collection([]); + } + ksort($firstRow); $columns = array_keys($firstRow); $createdAt = property_exists($this->_model, 'timestamps') && $this->_model->timestamps; @@ -1758,7 +1928,7 @@ private function prepareDelete() return ''; } - if (property_exists($this->_model, 'soft_deletes') && $this->_model->soft_deletes) { + if (!$this->_forceDelete && property_exists($this->_model, 'soft_deletes') && $this->_model->soft_deletes) { $timestamp = $this->currentTimestamp(); array_unshift($this->bindings, $timestamp); diff --git a/tests/EagerKeySubqueryScopeFixTest.php b/tests/EagerKeySubqueryScopeFixTest.php new file mode 100644 index 0000000..45b54be --- /dev/null +++ b/tests/EagerKeySubqueryScopeFixTest.php @@ -0,0 +1,43 @@ +resolver = static function ($sql) { + if (strpos($sql, 'wp_posts') !== false) { + return [(object) ['id' => 10, 'user_id' => 1]]; + } + + return [(object) ['id' => 1]]; + }; + } + + protected function tearDown(): void + { + $GLOBALS['wpdb'] = new FakeWpdb(); + } + + public function testEagerKeySubqueryDropsParentGroupByAndHaving(): void + { + User::groupBy('status')->having('cnt', '>', 1)->with('posts')->get(); + + $postsSql = $GLOBALS['wpdb']->queries[1]; + + $this->assertStringNotContainsString('GROUP BY', $postsSql); + $this->assertStringNotContainsString('HAVING', $postsSql); + $this->assertStringNotContainsString('cnt', $postsSql); + } +} diff --git a/tests/QueryClauseEdgeFixTest.php b/tests/QueryClauseEdgeFixTest.php new file mode 100644 index 0000000..e475a79 --- /dev/null +++ b/tests/QueryClauseEdgeFixTest.php @@ -0,0 +1,110 @@ +where('id', '=', null); + + $this->assertStringContainsString('`wp_users`.`id` IS NULL', $qb->toSql()); + $qb->toSql(); + $this->assertSame([], $qb->getBindings()); + } + + public function testWhereWithNotEqualNullEmitsIsNotNull(): void + { + $this->assertStringContainsString( + '`wp_users`.`id` IS NOT NULL', + (new User())->where('id', '!=', null)->toSql() + ); + } + + // A3 --------------------------------------------------------------------- + + public function testWhereInEmptyArrayEmitsFalseConstant(): void + { + $qb = (new User())->whereIn('id', []); + + $sql = $qb->toSql(); + $this->assertStringContainsString('0 = 1', $sql); + $this->assertStringNotContainsString('IN ()', $sql); + $this->assertSame([], $qb->getBindings()); + } + + public function testWhereWithEmptyArrayValueEmitsFalseConstant(): void + { + $sql = (new User())->where('status', [])->toSql(); + + $this->assertStringContainsString('0 = 1', $sql); + $this->assertStringNotContainsString('IN ()', $sql); + } + + // A8 --------------------------------------------------------------------- + + public function testWhereInNestedArrayElementGetsOnePlaceholderEach(): void + { + $qb = (new User())->whereIn('id', [[1, 2], 3]); + + $this->assertStringContainsString('IN (%s,%d)', $qb->toSql()); + $this->assertSame(['[1,2]', 3], $qb->getBindings()); + } + + // B3 --------------------------------------------------------------------- + + public function testWhereWithObjectValueBindsJsonString(): void + { + $qb = (new User())->where('meta', (object) ['k' => 'v']); + + $this->assertStringContainsString('`wp_users`.`meta` = %s', $qb->toSql()); + $qb->toSql(); + $this->assertSame(['{"k":"v"}'], $qb->getBindings()); + } + + // E1 --------------------------------------------------------------------- + + public function testTakeCastsArgumentToIntBlockingInjection(): void + { + $sql = (new User())->take('5; DROP TABLE wp_users')->toSql(); + + $this->assertStringContainsString('LIMIT 5', $sql); + $this->assertStringNotContainsString('DROP', $sql); + } + + public function testTakeNumericIsByteIdentical(): void + { + $this->assertStringContainsString('LIMIT 10', (new User())->take(10)->toSql()); + } + + public function testSkipCastsArgumentToInt(): void + { + $sql = (new User())->take(10)->skip('20; DROP')->toSql(); + + $this->assertStringContainsString('OFFSET 20', $sql); + $this->assertStringNotContainsString('DROP', $sql); + } +} diff --git a/tests/SoftDeleteRestoreTest.php b/tests/SoftDeleteRestoreTest.php new file mode 100644 index 0000000..2277be8 --- /dev/null +++ b/tests/SoftDeleteRestoreTest.php @@ -0,0 +1,51 @@ +forceDelete(); + + $sql = $GLOBALS['wpdb']->last_query; + $this->assertStringContainsString('DELETE FROM wp_soft_posts', $sql); + $this->assertStringNotContainsString('deleted_at', $sql); + } + + public function testRestoreNullsDeletedAt(): void + { + SoftPost::where('id', 1)->restore(); + + $sql = $GLOBALS['wpdb']->last_query; + $this->assertMatchesRegularExpression('/UPDATE\s+wp_soft_posts\s+SET\s+deleted_at\s*=\s*NULL/i', $sql); + } + + public function testForceDeleteRejectsNonSoftDeleteModel(): void + { + $this->expectException(RuntimeException::class); + + User::where('id', 1)->forceDelete(); + } +} diff --git a/tests/WriteEdgeFixTest.php b/tests/WriteEdgeFixTest.php new file mode 100644 index 0000000..0ecdc10 --- /dev/null +++ b/tests/WriteEdgeFixTest.php @@ -0,0 +1,111 @@ +insert_id = 1; + + $result = User::query()->insert(['tags' => ['a'], 'name' => 'x']); + + $this->assertInstanceOf(User::class, $result); + + $sql = $GLOBALS['wpdb']->last_query; + $this->assertStringStartsWith('INSERT INTO wp_users', $sql); + $this->assertStringContainsString('["a"]', $sql, 'tags must be JSON-encoded'); + $this->assertStringNotContainsString('),', $sql, 'must be a single VALUES tuple, not bulk'); + } + + // A5 --------------------------------------------------------------------- + + public function testEmptySaveSkipsMalformedUpdateAndReturnsModel(): void + { + $GLOBALS['wpdb']->resolver = static function () { + return [(object) ['id' => 1, 'name' => 'Ada']]; + }; + + $user = User::query()->where('id', 1)->first(); + + $GLOBALS['wpdb']->queries = []; + $GLOBALS['wpdb']->last_query = ''; + + $result = $user->save(); + + $this->assertSame($user, $result); + $this->assertSame([], $GLOBALS['wpdb']->queries, 'no UPDATE … SET query may be emitted'); + } + + // A6 --------------------------------------------------------------------- + + public function testInsertEmptyArrayReturnsFalseWithoutQuery(): void + { + $result = User::query()->insert([]); + + $this->assertFalse($result); + $this->assertSame([], $GLOBALS['wpdb']->queries); + } + + public function testBulkInsertEmptyRowsReturnsEmptyCollection(): void + { + $result = User::query()->insert([[], []]); + + $this->assertInstanceOf(Collection::class, $result); + $this->assertCount(0, $result); + $this->assertSame([], $GLOBALS['wpdb']->queries); + } + + public function testUpsertEmptyUpdateListDefaultsToAllColumns(): void + { + User::query()->upsert(['email' => 'a@x.com'], []); + + $sql = $GLOBALS['wpdb']->last_query; + $this->assertStringContainsString('email = VALUES(email)', $sql); + $this->assertStringNotContainsString('UPDATE ;', $sql); + } + + // A7 --------------------------------------------------------------------- + + public function testAggregateStarUsesBareStar(): void + { + (new User())->aggregate('COUNT', '*'); + + $sql = $GLOBALS['wpdb']->last_query; + $this->assertStringContainsString('COUNT(*)', $sql); + $this->assertStringNotContainsString('COUNT(`wp_users`.*)', $sql); + } + + public function testCountAggregateStillQualifiesPrimaryKey(): void + { + $GLOBALS['wpdb']->resolver = static function () { + return [(object) ['COUNT' => '3']]; + }; + + (new User())->count(); + + $this->assertStringContainsString('COUNT(`wp_users`.`id`)', $GLOBALS['wpdb']->last_query); + } +} From 018290c21975f72570c59d554006e384676e40e0 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Tue, 30 Jun 2026 23:26:51 +0600 Subject: [PATCH 19/32] docs: forceDelete/restore + Phase 1 invalid-SQL repair notes Assisted-By: AI --- docs/breaking-changes.md | 15 +++++++++++++++ docs/usage.md | 8 ++++++++ 2 files changed, 23 insertions(+) diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index d0f42f7..2c9a9de 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -323,6 +323,18 @@ Not signature breaks, but observable runtime differences. `wp_json_encode`, matching `save()`/`update()`. Previously a multi-row `insert([[...]])` or `upsert()` bound an array as the literal `"Array"` and an object threw — both repaired. Scalar values are unchanged. +- **Invalid-SQL / crash repairs (output changes only for previously-broken + input; working inputs are byte-identical):** `whereIn('c', [])` / `where('c', + [])` now emit `0 = 1` (was invalid `IN ()`); `where('c', '=', null)` and other + operator+null forms emit `IS [NOT] NULL` (was a truncated, value-less clause); + a `where`/`whereIn` value that is an object or a nested array is `wp_json_encode`d + (was a fatal / binding mismatch); `aggregate(fn, '*')` emits `COUNT(*)` (was + invalid `COUNT(\`t\`.*)`); an empty `save()` (no changed columns) skips the + query and returns the model (was a malformed `UPDATE … SET`); `insert([])`, + empty bulk rows, and `upsert($v, [])` no longer emit malformed SQL; a single + `insert()` row whose first value is an array no longer crashes. +- **`take()` / `skip()` cast their argument to `int`** — blocks `LIMIT`/`OFFSET` + injection; numeric input is byte-identical. - **`join()` table prefix corrected for custom-`$prefix` models.** Join (and pivot) tables now carry the model's **full** table prefix via the new `Model::getTablePrefix()` (`wp_` plus the plugin prefix), matching the model's @@ -426,6 +438,9 @@ User::query()->with('posts')->where('active', 1)->get(); `getPrefix()`, `getTablePrefix()` (full table prefix — `wp_` + plugin prefix — for join/pivot table names), `withCast(array $casts)`, `bool`/`boolean` cast. - **Connection:** `startTransaction()`, `commit()`, `rollback()`. +- **Model (soft-delete):** `forceDelete()` (real `DELETE`, bypasses the soft + rewrite) and `restore()` (clears `deleted_at`). Both throw on a non-soft-delete + model. - **Blueprint:** `unique($column = null)` — optional arg (backward compatible) for composite/explicit unique indexes. - **QueryBuilder:** `static $TIME_ZONE` to set the timezone statically; `$select` diff --git a/docs/usage.md b/docs/usage.md index 2433bd9..bf77c85 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -375,6 +375,14 @@ Contact::destroy([1, 2, 3]); // delete by primary keys `public $soft_delete_scope = true;` to enable automatic filtering: reads exclude trashed rows, `->withTrashed()` includes them, `->onlyTrashed()` returns only trashed rows. See [Limitations](#limitations--known-issues). +- On a soft-delete model, `forceDelete()` emits a real `DELETE` (bypassing the + soft rewrite) and `restore()` clears `deleted_at`. Both throw on a model + without `$soft_deletes`. + +```php +Contact::where('id', 1)->forceDelete(); // real DELETE, even for soft-delete models +Contact::onlyTrashed()->where('id', 1)->restore(); // deleted_at = NULL +``` --- From 3810f3b3c29ba9827b95fcd0ab43d4909a347ecf Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Wed, 1 Jul 2026 00:17:57 +0600 Subject: [PATCH 20/32] fix: order/group injection guard, cast aliases, relation-dirty, ragged bulk, schema DDL (zero-BC Phase 2) Behavior-changing fixes verified safe against bit-pi/bit-crm/bit-social/bit-assist usage (consumers pass only identifiers to order/group by, don't use the cast aliases, etc.); byte-identical for currently-working inputs (momus-reviewed). - E2: orderBy()/groupBy() validate the column is a plain identifier (`^[A-Za-z0-9_.`]+$`) and throw otherwise -> blocks ORDER BY/GROUP BY injection. Valid identifiers emit byte-identical SQL; raw expressions still go through orderByRaw(). - B1: cast aliases integer/float/double/json/datetime now map onto the real casters (were silent no-ops). - C1: relation values (Collection / Model / list-of-Model) are excluded from the save/update column set -> lazy relation access no longer corrupts a later save(); scalar/JSON-array columns still persist. - C6: bulk insert aligns each row to the header columns by key (was silent misalignment for ragged rows); uniform rows unchanged. - B4: Schema edit-mode unique() emits `ADD UNIQUE INDEX`; renameColumn() in edit() emits `RENAME COLUMN`; decimal($p, $s) works (incl. scale 0). Assisted-By: AI --- src/Blueprint.php | 33 ++++++++----- src/Model.php | 67 ++++++++++++++++++++++--- src/QueryBuilder.php | 57 +++++++++++++++++++-- tests/CastAliasFixTest.php | 60 ++++++++++++++++++++++ tests/Fixtures/CastAliasModel.php | 22 +++++++++ tests/OrderGroupInjectionFixTest.php | 67 +++++++++++++++++++++++++ tests/RaggedBulkInsertFixTest.php | 43 ++++++++++++++++ tests/RelationDirtyFixTest.php | 74 ++++++++++++++++++++++++++++ tests/SchemaDdlFixTest.php | 55 +++++++++++++++++++++ tests/bootstrap.php | 1 + 10 files changed, 456 insertions(+), 23 deletions(-) create mode 100644 tests/CastAliasFixTest.php create mode 100644 tests/Fixtures/CastAliasModel.php create mode 100644 tests/OrderGroupInjectionFixTest.php create mode 100644 tests/RaggedBulkInsertFixTest.php create mode 100644 tests/RelationDirtyFixTest.php create mode 100644 tests/SchemaDdlFixTest.php diff --git a/src/Blueprint.php b/src/Blueprint.php index 3186b77..4a93645 100644 --- a/src/Blueprint.php +++ b/src/Blueprint.php @@ -43,8 +43,8 @@ * @method Blueprint float($name, $length = null) * @method Blueprint double($name, $length = null) * @method Blueprint double_precision($name, $length = null) - * @method Blueprint decimal($name, $length = null) - * @method Blueprint dec($name, $length = null) + * @method Blueprint decimal($name, $precision = null, $scale = null) + * @method Blueprint dec($name, $precision = null, $scale = null) * @method Blueprint date($name) * @method Blueprint datetime($name) * @method Blueprint timestamp($name) @@ -116,7 +116,9 @@ public function __call($method, $parameters) { $formattedMethodName = strtoupper(str_replace('_', ' ', $method)); if ($this->isValidType($formattedMethodName)) { - if (\count($parameters) > 2) { + // Only decimal/dec take a scale (name, precision, scale); others are name + length. + $maxParams = \in_array($method, ['decimal', 'dec'], true) ? 3 : 2; + if (\count($parameters) > $maxParams) { throw new Exception('Too many parameters'); } @@ -204,6 +206,7 @@ public function edit() { $queryToAdd[] = $this->addColumnQuery(); $queryToAdd[] = $this->dropColumnQuery(); + $queryToAdd[] = $this->renameColumnQuery(); $queryToAdd = $queryToAdd + $this->_edit; $queryToAdd[] = $this->addPrimaryKeyQuery(); $queryToAdd[] = $this->addUniqueIndexQuery(); @@ -237,11 +240,13 @@ public function rename($newName) return $this; } - public function addColumn($name, $type, $length = null) + public function addColumn($name, $type, $length = null, $scale = null) { if ($this->method === 'addColumn') { $this->_sql = "ALTER TABLE {$this->table} ADD {$name} {$type}"; - if ($length) { + if (!\is_null($scale)) { + $this->_sql .= "({$length}, {$scale})"; + } elseif ($length) { $this->_sql .= "({$length})"; } } else { @@ -250,7 +255,10 @@ public function addColumn($name, $type, $length = null) 'name' => $name, 'type' => $type, ]; - if (!\is_null($length)) { + if (!\is_null($scale)) { + $this->columns[$this->columnIndex]['precision'] = $length; + $this->columns[$this->columnIndex]['scale'] = $scale; + } elseif (!\is_null($length)) { $this->length($length); } } @@ -272,7 +280,7 @@ public function dropColumn($column) public function renameColumn($column, $newName) { if ($this->method === 'renameColumn') { - $this->_sql = "ALTER TABLE {$this->table} CHANGE {$column} {$newName}"; + $this->_sql = "ALTER TABLE {$this->table} RENAME COLUMN {$column} TO {$newName}"; } else { $this->columnsToRename[] = [ 'column' => $column, @@ -292,7 +300,7 @@ public function renameColumnQuery() $query .= "\n, "; } - $query .= "CHANGE {$column['column']} {$column['new_name']}"; + $query .= "RENAME COLUMN {$column['column']} TO {$column['new_name']}"; } } @@ -637,7 +645,7 @@ private function addColumnQuery() $query .= $column['name'] . ' ' . $column['type']; if (!empty($column['length'])) { $query .= '(' . $column['length'] . ')'; - } elseif (!empty($column['precision']) && !empty($column['scale'])) { + } elseif (isset($column['precision'], $column['scale'])) { $query .= '(' . $column['precision'] . ', ' . $column['scale'] . ')'; } else { $query .= ' '; @@ -735,12 +743,13 @@ private function addUniqueIndexQuery() return ''; } - $query = ''; + $addPrefix = $this->method === 'edit' ? ' ADD ' : ''; + $query = ''; foreach ($this->uniqueIndex as $key => $uniqueColumn) { if (\is_array($uniqueColumn)) { - $query .= "\nUNIQUE INDEX " . implode('_', $uniqueColumn) . '_UNIQUE (' . implode(',', $uniqueColumn) . '),'; + $query .= "\n" . $addPrefix . 'UNIQUE INDEX ' . implode('_', $uniqueColumn) . '_UNIQUE (' . implode(',', $uniqueColumn) . '),'; } else { - $query .= "\nUNIQUE INDEX {$uniqueColumn}_UNIQUE ({$uniqueColumn} ASC),"; + $query .= "\n" . $addPrefix . "UNIQUE INDEX {$uniqueColumn}_UNIQUE ({$uniqueColumn} ASC),"; } } diff --git a/src/Model.php b/src/Model.php index acd4fbe..961df86 100644 --- a/src/Model.php +++ b/src/Model.php @@ -385,7 +385,33 @@ public function isDirty() public function getDirtyAttributes() { - return $this->dirty; + if (!\is_array($this->dirty)) { + return $this->dirty; + } + + return array_filter($this->dirty, function ($value) { + return !$this->isRelationValue($value); + }); + } + + /** + * True when a value is a loaded relation (a Collection, a related Model, or + * a list whose first element is one) rather than a persistable column value. + * A plain JSON array of scalars is NOT a relation and is still written. + * + * @param mixed $value + * + * @return bool + */ + public function isRelationValue($value) + { + if ($value instanceof Collection || $value instanceof Model) { + return true; + } + + return \is_array($value) + && isset($value[0]) + && ($value[0] instanceof Model || $value[0] instanceof Collection); } public function getOriginal() @@ -547,15 +573,37 @@ private function castTo($column, $value) return $value; } - if ( - !isset($this->casts) - || (isset($this->casts) && !isset($this->casts[$column])) - || !method_exists($this, 'castTo' . ucfirst($this->casts[$column])) - ) { + if (!isset($this->casts) || !isset($this->casts[$column])) { + return $value; + } + + $caster = $this->resolveCastMethod($this->casts[$column]); + if (!method_exists($this, $caster)) { return $value; } - return \call_user_func([$this, 'castTo' . ucfirst($this->casts[$column])], $value); + return \call_user_func([$this, $caster], $value); + } + + /** + * Resolves a cast name to its caster method, mapping the documented aliases + * (integer/float/double/json/datetime) onto the existing casters. + * + * @param string $cast + * + * @return string + */ + private function resolveCastMethod($cast) + { + $aliases = [ + 'integer' => 'castToInt', + 'float' => 'castToFloat', + 'double' => 'castToFloat', + 'json' => 'castToArray', + 'datetime' => 'castToDate', + ]; + + return isset($aliases[$cast]) ? $aliases[$cast] : 'castTo' . ucfirst($cast); } private function castToObject($value) @@ -581,6 +629,11 @@ private function castToInt($value) return (int) $value; } + private function castToFloat($value) + { + return (float) $value; + } + private function castToString($value) { return (string) $value; diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index b075542..b3a56c9 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -762,7 +762,11 @@ public function paginate($pageNo = 0, $perPage = 10) */ public function groupBy($columns) { - $columns = \is_array($columns) ? $columns : \func_get_args(); + $columns = \is_array($columns) ? $columns : \func_get_args(); + foreach ($columns as $column) { + $this->assertSafeIdentifier($column); + } + $this->groupBy = array_merge($this->groupBy, $columns); return $this; @@ -929,6 +933,8 @@ public function orOn($firstColumn, $operator = null, $secondColumn = null) */ public function orderBy($column) { + $this->assertSafeIdentifier($column); + $this->orderBy[] = [ 'column' => $column, 'direction' => 'ASC', @@ -1706,6 +1712,23 @@ private function nullOperator($operator) return \in_array($operator, $negations, true) ? 'IS NOT NULL' : 'IS NULL'; } + /** + * Guards an ORDER BY / GROUP BY column against injection: only a plain, + * qualified (table.column) or back-ticked identifier is accepted. Raw + * expressions must go through orderByRaw(). Valid identifiers are not + * re-rendered, so the emitted SQL stays byte-identical. + * + * @param mixed $column + * + * @return void + */ + private function assertSafeIdentifier($column) + { + if (!\is_string($column) || !preg_match('/^[A-Za-z0-9_.`]+$/', $column)) { + throw new RuntimeException('Unsafe column passed to order/group by clause.'); + } + } + /** * Returns true when the model declares soft-delete support. * @@ -1754,13 +1777,18 @@ private function bulkInsert($attributes) $sql .= ' VALUES '; $values = []; foreach ($attributes as $row) { - ksort($row); if ($createdAt) { $row['created_at'] = $this->currentTimestamp(); } - $rowValues = array_values($row); - $values[] = ' (' + // Align each row to the header columns by key so rows with differing + // keys are not positionally misaligned (absent column => NULL). + $rowValues = []; + foreach ($columns as $column) { + $rowValues[] = isset($row[$column]) ? $row[$column] : null; + } + + $values[] = ' (' . implode( ', ', array_map( @@ -1828,6 +1856,8 @@ private function prepareAttributeForSaveOrUpdate($isUpdate = false) $columnsToPrepare = array_keys($this->_model->getAttributes()); } + $columnsToPrepare = $this->withoutRelationColumns($columnsToPrepare); + if (property_exists($this->_model, 'timestamps') && $this->_model->timestamps) { if (!$isUpdate) { $this->_model->setAttribute('created_at', $this->currentTimestamp()); @@ -1856,6 +1886,25 @@ private function prepareAttributeForSaveOrUpdate($isUpdate = false) return $columnsToPrepare; } + /** + * Drops columns whose current value is a loaded relation (Collection/Model) + * from a save/update write set, so lazily-read relations are never persisted + * to a non-existent column. Re-indexes to keep bindings positionally aligned. + * + * @param array $columns + * + * @return array + */ + private function withoutRelationColumns(array $columns) + { + $attributes = $this->_model->getAttributes(); + + return array_values(array_filter($columns, function ($column) use ($attributes) { + return !\array_key_exists($column, $attributes) + || !$this->_model->isRelationValue($attributes[$column]); + })); + } + /** * Prepares insert statement * diff --git a/tests/CastAliasFixTest.php b/tests/CastAliasFixTest.php new file mode 100644 index 0000000..d609fba --- /dev/null +++ b/tests/CastAliasFixTest.php @@ -0,0 +1,60 @@ + '42']); + + $this->assertSame(42, $model->n); + } + + public function testFloatAliasCastsToFloat(): void + { + $model = new CastAliasModel(['f' => '4.5']); + + $this->assertSame(4.5, $model->f); + } + + public function testDoubleAliasCastsToFloat(): void + { + $model = new CastAliasModel(['d' => '2.5']); + + $this->assertSame(2.5, $model->d); + } + + public function testJsonAliasDecodesToArray(): void + { + $model = new CastAliasModel(['data' => '{"x":1}']); + + $this->assertSame(['x' => 1], $model->data); + } + + public function testDatetimeAliasCastsToDate(): void + { + $model = new CastAliasModel(['at' => '2024-01-02 03:04:05']); + + $this->assertInstanceOf(DateTime::class, $model->at); + } +} diff --git a/tests/Fixtures/CastAliasModel.php b/tests/Fixtures/CastAliasModel.php new file mode 100644 index 0000000..b709f92 --- /dev/null +++ b/tests/Fixtures/CastAliasModel.php @@ -0,0 +1,22 @@ + 'integer', + 'f' => 'float', + 'd' => 'double', + 'data' => 'json', + 'at' => 'datetime', + ]; +} diff --git a/tests/OrderGroupInjectionFixTest.php b/tests/OrderGroupInjectionFixTest.php new file mode 100644 index 0000000..9d34158 --- /dev/null +++ b/tests/OrderGroupInjectionFixTest.php @@ -0,0 +1,67 @@ +expectException(RuntimeException::class); + + User::query()->orderBy('id; DROP TABLE x'); + } + + public function testGroupByRejectsInjectionPayload(): void + { + $this->expectException(RuntimeException::class); + + User::query()->groupBy('a); DROP'); + } + + public function testOrderByPlainColumnUnchanged(): void + { + $sql = User::query()->orderBy('id', 'DESC')->toSql(); + + $this->assertStringContainsString('ORDER BY id ASC', $sql); + } + + public function testOrderByQualifiedColumnUnchanged(): void + { + $sql = User::query()->orderBy('t.col')->toSql(); + + $this->assertStringContainsString('ORDER BY t.col ASC', $sql); + } + + public function testGroupByPlainColumnUnchanged(): void + { + $sql = User::query()->groupBy('contact_id')->toSql(); + + $this->assertStringContainsString('GROUP BY contact_id', $sql); + } + + public function testGroupByQualifiedColumnUnchanged(): void + { + $sql = User::query()->groupBy('wp_x.module')->toSql(); + + $this->assertStringContainsString('GROUP BY wp_x.module', $sql); + } +} diff --git a/tests/RaggedBulkInsertFixTest.php b/tests/RaggedBulkInsertFixTest.php new file mode 100644 index 0000000..421f471 --- /dev/null +++ b/tests/RaggedBulkInsertFixTest.php @@ -0,0 +1,43 @@ +rows_affected = 0; + + User::query()->insert([ + ['a' => 'x', 'b' => 'y'], + ['b' => 'z', 'c' => 'w'], + ]); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('(a, b)', $sql); + $this->assertStringContainsString("('x', 'y')", $sql); + $this->assertStringContainsString("(NULL, 'z')", $sql); + $this->assertStringNotContainsString("('z'", $sql); + } +} diff --git a/tests/RelationDirtyFixTest.php b/tests/RelationDirtyFixTest.php new file mode 100644 index 0000000..b34dbfe --- /dev/null +++ b/tests/RelationDirtyFixTest.php @@ -0,0 +1,74 @@ +resolver = static function ($sql) { + if (strpos($sql, 'wp_posts') !== false) { + return [(object) ['id' => 5, 'user_id' => 1, 'title' => 'p']]; + } + + return [(object) ['id' => 1, 'name' => 'Ada']]; + }; + + $user = User::query()->where('id', 1)->first(); + $posts = $user->posts; + + $this->assertInstanceOf(Collection::class, $posts); + $this->assertArrayNotHasKey('posts', $user->getDirtyAttributes()); + + $user->name = 'Grace'; + + $GLOBALS['wpdb']->queries = []; + $GLOBALS['wpdb']->last_query = ''; + + $user->save(); + + $sql = $GLOBALS['wpdb']->last_query; + $this->assertStringContainsString('UPDATE wp_users', $sql); + $this->assertStringContainsString('name =', $sql); + $this->assertStringNotContainsString('posts', $sql); + } + + public function testRealArrayColumnIsStillWritten(): void + { + $GLOBALS['wpdb']->resolver = static function () { + return [(object) ['id' => 1, 'name' => 'Ada']]; + }; + + $user = User::query()->where('id', 1)->first(); + $user->tags = ['a', 'b']; + + $GLOBALS['wpdb']->queries = []; + $GLOBALS['wpdb']->last_query = ''; + + $user->save(); + + $sql = $GLOBALS['wpdb']->last_query; + $this->assertStringContainsString('tags =', $sql); + $this->assertStringContainsString('["a","b"]', $sql); + } +} diff --git a/tests/SchemaDdlFixTest.php b/tests/SchemaDdlFixTest.php new file mode 100644 index 0000000..711f0f3 --- /dev/null +++ b/tests/SchemaDdlFixTest.php @@ -0,0 +1,55 @@ +string('email')->unique(); + }); + + $this->assertStringContainsString('ADD UNIQUE INDEX', $GLOBALS['wpdb']->last_query); + } + + public function testEditRenameColumnIsEmitted(): void + { + Schema::edit('orders', function ($table) { + $table->renameColumn('old_name', 'new_name'); + }); + + $this->assertStringContainsString( + 'RENAME COLUMN old_name TO new_name', + $GLOBALS['wpdb']->last_query + ); + } + + public function testDecimalEmitsPrecisionAndScale(): void + { + Schema::create('invoices', function ($table) { + $table->decimal('amount', 8, 2); + }); + + $this->assertStringContainsString('DECIMAL(8, 2)', $GLOBALS['wpdb']->last_query); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 8fb7b06..b012e3e 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -129,6 +129,7 @@ public function has_cap($cap) require __DIR__ . '/Fixtures/EventUser.php'; require __DIR__ . '/Fixtures/RetrieveUser.php'; require __DIR__ . '/Fixtures/CastModel.php'; +require __DIR__ . '/Fixtures/CastAliasModel.php'; require __DIR__ . '/Fixtures/AccessorModel.php'; require __DIR__ . '/Fixtures/CreatingUser.php'; require __DIR__ . '/Fixtures/ScopedSoftPost.php'; From c710ad00b60d42ce31ac856eaac9665e938c6869 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Wed, 1 Jul 2026 00:19:05 +0600 Subject: [PATCH 21/32] docs: Phase 2 behavioral notes (order/group validation, cast aliases, ragged bulk) Assisted-By: AI --- docs/breaking-changes.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index 2c9a9de..f3002f3 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -335,6 +335,15 @@ Not signature breaks, but observable runtime differences. `insert()` row whose first value is an array no longer crashes. - **`take()` / `skip()` cast their argument to `int`** — blocks `LIMIT`/`OFFSET` injection; numeric input is byte-identical. +- **`orderBy()` / `groupBy()` validate the column** as a plain identifier + (`^[A-Za-z0-9_.`]+$`) and throw `RuntimeException` otherwise — blocks + `ORDER BY`/`GROUP BY` injection. Plain/qualified identifiers emit byte-identical + SQL; pass raw expressions through `orderByRaw()`. +- **Cast aliases `integer`/`float`/`double`/`json`/`datetime` now work** (map onto + the existing casters) — they were previously silent no-ops returning the raw value. +- **Bulk `insert()` aligns ragged rows by column** — a row whose keys differ from + the first row no longer silently shifts values into the wrong columns (uniform + rows unchanged). - **`join()` table prefix corrected for custom-`$prefix` models.** Join (and pivot) tables now carry the model's **full** table prefix via the new `Model::getTablePrefix()` (`wp_` plus the plugin prefix), matching the model's From 65fff45ba22795acaf3ce52572166153d237af5a Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Wed, 1 Jul 2026 00:41:50 +0600 Subject: [PATCH 22/32] fix: eager empty relation resolves to [] without an N+1 re-query (C3) An eager-loaded parent with no related rows stored null, so accessing the relation fell through offsetExists() to a fresh lazy query (the N+1 the eager load exists to prevent) that returned an empty Collection. Store [] instead: offsetExists() is true, the resolved-empty value is returned with no extra query. Empty either way (count 0, falsy); the type now matches a non-empty eager relation (plain array). No existing test relied on the null. Assisted-By: AI --- docs/breaking-changes.md | 6 ++++++ src/Concerns/Relations.php | 6 ++++-- tests/EagerLoadIntegrationTest.php | 30 ++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index f3002f3..a1852e3 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -344,6 +344,12 @@ Not signature breaks, but observable runtime differences. - **Bulk `insert()` aligns ragged rows by column** — a row whose keys differ from the first row no longer silently shifts values into the wrong columns (uniform rows unchanged). +- **Eager-loaded empty relations resolve to `[]` without a re-query.** A parent + with no related rows previously stored `null`, so accessing the relation fired a + fresh lazy query (an N+1) that returned an empty `Collection`. It now holds `[]` + directly — no extra query. The value is empty either way (`count()` 0, falsy); + the type for an empty eager relation is now a plain array, matching a non-empty + eager relation. - **`join()` table prefix corrected for custom-`$prefix` models.** Join (and pivot) tables now carry the model's **full** table prefix via the new `Model::getTablePrefix()` (`wp_` plus the plugin prefix), matching the model's diff --git a/src/Concerns/Relations.php b/src/Concerns/Relations.php index 71d2533..f32e6aa 100644 --- a/src/Concerns/Relations.php +++ b/src/Concerns/Relations.php @@ -342,9 +342,11 @@ private function setRelatedData(Model $model) $relationKey = $relationQuery->getModel()->getActiveRelationKey(); + // empty relations store [] (not null) so accessing them returns the + // resolved-empty result instead of falling through to a lazy re-query (N+1). $data = isset( $this->_relatedData[$relationName][$model->getAttribute($relationKey['localKey'])] - ) ? $this->_relatedData[$relationName][$model->getAttribute($relationKey['localKey'])] : null; + ) ? $this->_relatedData[$relationName][$model->getAttribute($relationKey['localKey'])] : []; if ($relationQuery->getModel()->getRelateAs() === 'oneToOne' && is_countable($data) && \count($data)) { $data = $data[0]; @@ -363,7 +365,7 @@ private function setPivotRelatedData(Model $model, $relationName, Model $related $key = $model->getAttribute($pivot['parentKey']); $data = isset($this->_relatedData[$relationName][$key]) ? $this->_relatedData[$relationName][$key] - : null; + : []; [$name, $alias] = $this->prepareRelationName($relationName); $model->setAttribute(\is_null($alias) ? $name : $alias, $data); diff --git a/tests/EagerLoadIntegrationTest.php b/tests/EagerLoadIntegrationTest.php index 24f7bf2..ac99069 100644 --- a/tests/EagerLoadIntegrationTest.php +++ b/tests/EagerLoadIntegrationTest.php @@ -53,6 +53,36 @@ public function testStaticWithEagerLoadsAndGroupsRelatedRows(): void $this->assertInstanceOf(Post::class, $first->posts[0]); } + /** + * A parent with NO related rows must not trigger a fresh lazy query when the + * relation is accessed (the eager load already resolved it to empty) — the + * N+1 the eager load exists to prevent. + */ + public function testEmptyEagerRelationDoesNotReQueryOnAccess(): void + { + $GLOBALS['wpdb']->resolver = static function ($sql) { + if (strpos($sql, 'wp_posts') !== false) { + return [(object) ['id' => 10, 'user_id' => 1]]; + } + + return [(object) ['id' => 1], (object) ['id' => 2]]; + }; + + $users = User::with('posts')->get(); + $second = null; + foreach ($users as $u) { + if ((int) $u->id === 2) { + $second = $u; + } + } + + $GLOBALS['wpdb']->queries = []; + $posts = $second->posts; // user 2 has no posts + + $this->assertCount(0, $posts, 'empty eager relation resolves to empty'); + $this->assertCount(0, $GLOBALS['wpdb']->queries, 'no re-query (N+1) on accessing an empty eager relation'); + } + /** * The parent's selectRaw must NOT leak into the eager key subquery — that * subquery feeds an IN (...) and must return exactly one column (the localKey), From ede27bbe168fc8493ff4e9aa7c70eeb922074fc0 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Wed, 1 Jul 2026 11:26:14 +0600 Subject: [PATCH 23/32] fix: soft-delete reads exclude trashed rows by default A model with $soft_deletes = true now injects deleted_at IS NULL on every SELECT automatically; trashed rows no longer appear. Opt out with public $soft_delete_scope = false to restore the unfiltered read. refresh() reloads a row by its own primary key with withTrashed(), so a re-hydrated trashed model still reports exists() === true and the next save() UPDATEs instead of re-INSERTing a duplicate. Scope is injected on SELECT only; delete/restore/forceDelete/insert/ upsert/aggregate paths are unaffected. Assisted-By: AI --- src/Model.php | 5 ++++- src/QueryBuilder.php | 11 +++++++++-- tests/Fixtures/UnscopedSoftPost.php | 18 ++++++++++++++++++ tests/SoftDeleteScopeTest.php | 13 +++++++++++-- tests/bootstrap.php | 1 + 5 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 tests/Fixtures/UnscopedSoftPost.php diff --git a/src/Model.php b/src/Model.php index 961df86..897c1d4 100644 --- a/src/Model.php +++ b/src/Model.php @@ -238,7 +238,10 @@ public function refresh() return false; } - $result = $this->newQuery()->findOne([$this->primaryKey => $this->attributes[$this->primaryKey]]); + // withTrashed(): refresh reloads this row by its own PK, so it must find + // the row even when trashed — otherwise a hydrated soft-deleted model + // reports exists() === false and the next save() re-INSERTs a duplicate. + $result = $this->newQuery()->withTrashed()->findOne([$this->primaryKey => $this->attributes[$this->primaryKey]]); if (!$result) { $this->_isExists = false; diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index b3a56c9..8e0edad 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -1740,13 +1740,20 @@ private function isSoftDeleteModel() } /** - * Returns true when the model opts into automatic soft-delete read scope. + * Returns true when soft-delete read scope is active for the model. + * + * Soft-delete models exclude trashed rows by default; a model opts out by + * declaring public $soft_delete_scope = false. * * @return bool */ private function autoScopeEnabled() { - return property_exists($this->_model, 'soft_delete_scope') && $this->_model->soft_delete_scope; + if (property_exists($this->_model, 'soft_delete_scope')) { + return (bool) $this->_model->soft_delete_scope; + } + + return true; } /** diff --git a/tests/Fixtures/UnscopedSoftPost.php b/tests/Fixtures/UnscopedSoftPost.php new file mode 100644 index 0000000..9eb8140 --- /dev/null +++ b/tests/Fixtures/UnscopedSoftPost.php @@ -0,0 +1,18 @@ +toSql(); + $this->assertStringContainsString('deleted_at', $sql); + $this->assertStringContainsString('IS NULL', $sql); + } + + // Opt-out: $soft_delete_scope = false restores the unfiltered read + public function testSoftDeleteScopeFalseOptsOutOfFilter(): void + { + $sql = UnscopedSoftPost::query()->toSql(); $this->assertStringNotContainsString('deleted_at', $sql); } diff --git a/tests/bootstrap.php b/tests/bootstrap.php index b012e3e..e5c5697 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -133,6 +133,7 @@ public function has_cap($cap) require __DIR__ . '/Fixtures/AccessorModel.php'; require __DIR__ . '/Fixtures/CreatingUser.php'; require __DIR__ . '/Fixtures/ScopedSoftPost.php'; +require __DIR__ . '/Fixtures/UnscopedSoftPost.php'; require __DIR__ . '/Fixtures/TimestampedRow.php'; require __DIR__ . '/Fixtures/Role.php'; require __DIR__ . '/Fixtures/Member.php'; From 02ab5582262c0d7e120a8db7669fed6d9fa27d6c Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Wed, 1 Jul 2026 11:26:23 +0600 Subject: [PATCH 24/32] docs: add relations reference; soft-delete default-filter notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add docs/relations.md — a standalone relationship reference covering hasOne/belongsTo (the shared oneToOne alias and reversed-from-Laravel key naming), hasMany, belongsToMany pivot relations (signature, key defaults, withPivot, pivot_* attributes, read-only limits), eager/lazy loading, and relation aggregates. Linked from README and usage.md; the deep belongsToMany block and relation Limitations bullets in usage.md now point to it. Document the soft-delete default flip: usage.md (property table, deleting section, limitations) and breaking-changes.md (new 2.13 + summary row 13). Assisted-By: AI --- README.md | 2 + docs/breaking-changes.md | 30 +++++ docs/relations.md | 268 +++++++++++++++++++++++++++++++++++++++ docs/usage.md | 130 +++++-------------- 4 files changed, 333 insertions(+), 97 deletions(-) create mode 100644 docs/relations.md diff --git a/README.md b/README.md index c2e3f93..185f946 100644 --- a/README.md +++ b/README.md @@ -43,5 +43,7 @@ $active = Contact::where('is_active', 1) - **[Usage guide](docs/usage.md)** — models, query builder, relationships, casts, events, transactions, and more. +- **[Relationships](docs/relations.md)** — `hasOne`/`belongsTo`/`hasMany`/`belongsToMany`, + eager & lazy loading, relation aggregates, and limitations. - **[Schema builder](docs/schema.md)** — table creation, columns, indexes, and migrations. - **[Breaking changes](docs/breaking-changes.md)** — upgrade notes. diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index a1852e3..1cbc66c 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -29,6 +29,7 @@ API and runtime behavior change, it is a **major** version bump. | 10 | Relations | `addRelation()` signature `string` → `array`, void | Medium | | 11 | Blueprint | `binary()` removed | Low | | 12 | QueryBuilder | exception type/message changed in `exec()` | Low | +| 13 | Model | soft-delete reads exclude trashed by default (opt out with `$soft_delete_scope = false`) | Medium | --- @@ -285,6 +286,35 @@ specifically for this case, should be reviewed. --- +### 2.13 Soft-delete reads now exclude trashed rows by default + +A model with `public $soft_deletes = true;` previously returned **all** rows — +trashed and non-trashed alike — unless it also opted in with +`$soft_delete_scope = true`. Reads now inject `deleted_at IS NULL` **by default**, +so trashed rows no longer appear. + +```php +class Post extends Model +{ + public $soft_deletes = true; // reads now hide trashed rows automatically +} + +Post::all(); // excludes trashed rows +Post::withTrashed()->get(); // include trashed +Post::onlyTrashed()->get(); // only trashed +``` + +**Migration:** to keep the old unfiltered behavior, declare the opt-out flag: + +```php +public $soft_delete_scope = false; // reads return every row, including trashed +``` + +`refresh()` reloads a row by its own primary key with `withTrashed()`, so +re-hydrating a trashed model still reports `exists() === true`. + +--- + ## 3. Behavioral changes Not signature breaks, but observable runtime differences. diff --git a/docs/relations.md b/docs/relations.md new file mode 100644 index 0000000..4fa2a73 --- /dev/null +++ b/docs/relations.md @@ -0,0 +1,268 @@ +# wp-database — Relationships + +Full reference for defining and loading relationships. For the broader API see +the [Usage guide](usage.md). + +Relationships are declared as **methods** on a model; each method returns a query +for the related model. Load them eagerly with `with()` or lazily by accessing the +method name as a property (`$model->relation`). + +- [Key convention](#key-convention) +- [`hasOne` / `belongsTo` (one-to-one)](#hasone--belongsto-one-to-one) +- [`hasMany` (one-to-many)](#hasmany-one-to-many) +- [`belongsToMany` (many-to-many)](#belongstomany-many-to-many) +- [Eager loading](#eager-loading) +- [Lazy loading](#lazy-loading) +- [Relation aggregates & existence](#relation-aggregates--existence) +- [Limitations](#limitations) + +--- + +## Key convention + +> **`$foreignKey` is the column on the _related_ model's table; `$localKey` is +> the column on the _calling_ model's table.** This is the **reverse** of +> Laravel's naming — always pass both arguments explicitly when your columns +> differ from the ORM default (`{callerTable}_id` / `id`) to avoid confusion. + +Every relation method takes the same first three arguments: + +```php +relation($model, $foreignKey = null, $localKey = null) +``` + +Omitted keys derive from the package's foreign-key convention: `$foreignKey` +defaults to the caller's `getForeignKey()` (`{tableWithoutPrefix}_{primaryKey}`, +e.g. `contacts_id` — note: plural, unlike Laravel's singular default) and +`$localKey` defaults to the caller's primary key. + +--- + +## `hasOne` / `belongsTo` (one-to-one) + +**`hasOne()` is a direct alias of `belongsTo()`.** Both set the same `oneToOne` +relation type and return a single related model. There is no separate +reverse-direction implementation — direction is determined entirely by the keys +you pass and which model calls the method. + +```php +class Contact extends Model +{ + public function profile() + { + // foreignKey='contact_id' on the profiles (related) table + // localKey='id' on the contacts (this) table + // Predicate: WHERE profiles.contact_id IN (SELECT id FROM contacts) + return $this->hasOne(Profile::class, 'contact_id', 'id'); + } +} + +class Deal extends Model +{ + public function contact() + { + // Deal.contact_id references Contact.id + // foreignKey='id' on the contacts (related) table + // localKey='contact_id' on the deals (this) table + // Predicate: WHERE contacts.id IN (SELECT contact_id FROM deals) + return $this->belongsTo(Contact::class, 'id', 'contact_id'); + } +} +``` + +A `oneToOne` relation resolves to a single `Model` (or `[]` when there is no +match), not a `Collection`. + +--- + +## `hasMany` (one-to-many) + +Same key convention; resolves to a `Collection` of related models. + +```php +class Contact extends Model +{ + public function deals() + { + // foreignKey='contact_id' lives on the deals table + // localKey='id' lives on the contacts table + // Predicate: WHERE deals.contact_id IN (SELECT id FROM contacts) + return $this->hasMany(Deal::class, 'contact_id', 'id'); + } +} +``` + +--- + +## `belongsToMany` (many-to-many) + +Resolves a many-to-many relation through a pivot (junction) table. Signature: + +```php +belongsToMany( + $model, + $pivotTable = null, // unprefixed pivot/junction table name + $foreignPivotKey = null, // parent's key column ON the pivot table + $relatedPivotKey = null, // related's key column ON the pivot table + $parentKey = null, // local key column on the parent table + $relatedKey = null // key column on the related table +) +``` + +When `$pivotTable` is `null` the method keeps its **legacy** behaviour (resolves +exactly like `hasMany` — the related table must carry the parent FK). Pass the +pivot table name (unprefixed; the package prefixes it like `join()` does) to get +real pivot behaviour. + +Omitted keys derive from the package's foreign-key convention: + +| Argument | Default | Member (`members`) ↔ Role (`roles`) | +|---|---|---| +| `$foreignPivotKey` | parent `getForeignKey()` | `members_id` | +| `$relatedPivotKey` | related `getForeignKey()` | `roles_id` | +| `$parentKey` | parent `getPrimaryKey()` | `id` | +| `$relatedKey` | related `getPrimaryKey()` | `id` | + +```php +class Member extends Model +{ + protected $table = 'members'; + + public function roles() + { + // pivot table role_user(member_id, role_id) + return $this->belongsToMany(Role::class, 'role_user', 'member_id', 'role_id'); + } + + // Carry extra pivot columns; they surface flat as `pivot_` attributes + public function rolesWithAssignment() + { + return $this->belongsToMany(Role::class, 'role_user', 'member_id', 'role_id') + ->withPivot(['assigned_at']); + } +} +``` + +The link column rides along on every related model as the reserved attribute +`pivot_` (e.g. `pivot_member_id`), and each `withPivot()` column +as `pivot_` (e.g. `pivot_assigned_at`). These flat `pivot_*` attributes +appear in `toArray()`. + +```php +// Eager +foreach (Member::with('roles')->get() as $member) { + foreach ($member->roles as $role) { + echo $role->pivot_member_id; // the parent link + } +} + +// Lazy +$member = Member::query()->findOne(['id' => 1]); +foreach ($member->roles as $role) { /* ... */ } +``` + +Pivot relations are **read-only**: `attach`/`detach`/`sync` and +`withCount`/`whereHas`/aggregates over a pivot relation are **not** supported +(the aggregates throw a `RuntimeException`). See [Limitations](#limitations). + +--- + +## Eager loading + +Load a relation for every parent in one extra query (avoids the N+1 problem). + +```php +Contact::with('deals')->get(); // load deals for each contact +Contact::with(['deals', 'profile'])->get(); // multiple relations + +// Constrain the eager-loaded relation +Contact::with('deals', function ($q) { + $q->where('status', 'open'); +})->get(); + +// Alias the loaded relation key on the result model +Contact::with('deals as open_deals')->get(); + +// Access loaded relations +foreach (Contact::with('deals')->get() as $contact) { + foreach ($contact->deals as $deal) { /* ... */ } +} +``` + +A parent with no related rows resolves to an empty result **without** triggering +a fresh lazy query when the relation is later accessed — the eager load already +resolved it to empty. + +--- + +## Lazy loading + +Access the relation method name as a property to load it on demand: + +```php +$contact = Contact::query()->findOne(['id' => 1]); + +$contact->deals; // Collection, queried on first access +$contact->profile; // single Model or [] +``` + +--- + +## Relation aggregates & existence + +```php +Contact::withCount('deals')->get(); // adds `deals_count` +Contact::withSum('deals.amount')->get(); // adds `deals_sum` +Contact::withAvg('deals.amount')->get(); // adds `deals_avg` +Contact::withMin('deals.amount')->get(); // adds `deals_min` +Contact::withMax('deals.amount')->get(); // adds `deals_max` +Contact::withExists('deals')->get(); // adds bool-cast `deals_exists` + +// Filter by relation existence +Contact::whereHas('deals')->get(); +Contact::whereHas('deals', fn ($q) => $q->where('status', 'open'))->get(); + +// Filter by existence AND eager-load the same relation +Contact::withWhereHas('deals', fn ($q) => $q->where('status', 'open'))->get(); +``` + +Aggregate columns are aliased `_` by default +(e.g. `deals_count`, `deals_sum`). Pass `'relation as alias'` for a custom name: + +```php +Contact::withCount('deals as total_deals')->get(); // adds `total_deals` +Contact::withSum('deals.amount as revenue')->get(); // adds `revenue` +``` + +For `withMin`, `withMax`, `withAvg`, and `withSum` the column to aggregate must +be given as `'relation.column'`; passing just the relation name defaults to `*`, +which is meaningful only for `withCount` and `withExists`. + +--- + +## Limitations + +- **`belongsTo` and `hasOne` are the same alias; key naming is reversed from + Laravel.** Both set the `oneToOne` relation. `$foreignKey` is the column on the + **related** table and `$localKey` is the column on the **calling** model's + table — the opposite of Laravel. Supply both arguments explicitly. + +- **`belongsToMany` pivot relations are read-only and single-key.** Real + pivot-table many-to-many is supported for reads (eager `with()` + lazy + `$model->relation`), with these gaps: + - No `withCount`/`whereHas`/aggregates on a pivot relation — they **throw** + `RuntimeException` (the pivot metadata has no single `foreignKey`/`localKey`). + - No `attach`/`detach`/`sync` (write side is out of scope). + - Single-column pivot/parent/related keys only — no composite keys. + - Duplicate pivot rows yield duplicate related models (no `DISTINCT`). + - Eager constraint closures may add `where`/`orderBy`/`limit` but **cannot** + narrow the selected columns — the pivot path always selects `related.*` so + the aliased pivot column can ride along. + - Pivot values surface as flat **reserved** `pivot_*` attributes (including the + link key `pivot_`) and appear in `toArray()`. A related + column literally named `pivot_*` would be overwritten. These attributes are + excluded from dirty-tracking on UPDATE, so re-saving a hydrated related model + is safe; a forced re-INSERT would attempt to write the non-existent columns. + - Null parent key: the eager path buckets a null parent key under null + (relation resolves to `null`); the lazy path renders `… IS NULL` and returns + pivot rows whose link column is NULL — a minor divergence. diff --git a/docs/usage.md b/docs/usage.md index bf77c85..d3255ca 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -110,7 +110,7 @@ class Contact extends Model ]; // Soft deletes: delete() sets deleted_at instead of removing the row. - // NOTE: reads are NOT filtered — soft-deleted rows appear in all() and every query. + // Reads exclude trashed rows by default; opt out with $soft_delete_scope = false. public $soft_deletes = true; } ``` @@ -123,7 +123,7 @@ class Contact extends Model | `$fillable` | Mass-assignment allow-list. Unset = allow all non-timestamp, non-PK attributes. | | `$casts` | Map of column → cast type. See [Attribute casting](#attribute-casting). | | `$timestamps` | Auto-set `created_at`/`updated_at` on insert/update (declared `true` in the base `Model`; set to `false` to disable). Requires the columns to exist — see [`timestamps()` in `docs/schema.md`](schema.md#timestamps). | -| `$soft_deletes` | Must be declared `true` on the model. `delete()` then sets `deleted_at` instead of removing the row. Reads return **all rows by default** — including trashed ones. To enable automatic filtering, also declare `public $soft_delete_scope = true;`; reads will then exclude trashed rows automatically. Use `->withTrashed()` to include them, or `->onlyTrashed()` to return only trashed rows. See [Limitations](#limitations--known-issues) and [`softDeletes()` in `docs/schema.md`](schema.md#softdeletes). | +| `$soft_deletes` | Must be declared `true` on the model. `delete()` then sets `deleted_at` instead of removing the row. Reads **exclude trashed rows by default**. Use `->withTrashed()` to include them, or `->onlyTrashed()` to return only trashed rows. Declare `public $soft_delete_scope = false;` to opt out of the filter (reads return every row, including trashed). See [Limitations](#limitations--known-issues) and [`softDeletes()` in `docs/schema.md`](schema.md#softdeletes). | --- @@ -371,10 +371,11 @@ Contact::destroy([1, 2, 3]); // delete by primary keys `RuntimeException('SQL query is empty')` — a guard against wiping the table. Use a raw `TRUNCATE` if you really mean to empty it. - With `$soft_deletes = true`, `delete()` sets `deleted_at` instead of removing - the row. Reads return **all rows by default** — including trashed ones. Add - `public $soft_delete_scope = true;` to enable automatic filtering: reads exclude - trashed rows, `->withTrashed()` includes them, `->onlyTrashed()` returns only - trashed rows. See [Limitations](#limitations--known-issues). + the row, and reads **exclude trashed rows by default**. Use `->withTrashed()` + to include them, or `->onlyTrashed()` to return only trashed rows. To opt out + of the automatic filter (reads return every row, including trashed), declare + `public $soft_delete_scope = false;`. See + [Limitations](#limitations--known-issues). - On a soft-delete model, `forceDelete()` emits a real `DELETE` (bypassing the soft rewrite) and `restore()` clears `deleted_at`. Both throw on a model without `$soft_deletes`. @@ -445,6 +446,10 @@ Contact::query()->withCast(['is_active' => 'bool'])->get(); ## Relationships +> **Full reference:** [docs/relations.md](relations.md) — every relation type, +> eager/lazy loading, aggregates, and relation-specific limitations in one place. +> This section is a quick overview. + Define relationships as methods on the model. The relation method returns a query for the related model. @@ -495,35 +500,8 @@ Available: `hasOne()`, `hasMany()`, `belongsTo()` — each takes ### Many-to-many (`belongsToMany`) -`belongsToMany` resolves a many-to-many relation through a pivot (junction) -table. Signature: - -```php -belongsToMany( - $model, - $pivotTable = null, // unprefixed pivot/junction table name - $foreignPivotKey = null, // parent's key column ON the pivot table - $relatedPivotKey = null, // related's key column ON the pivot table - $parentKey = null, // local key column on the parent table - $relatedKey = null // key column on the related table -) -``` - -When `$pivotTable` is `null` the method keeps its **legacy** behaviour (resolves -exactly like `hasMany` — the related table must carry the parent FK). Pass the -pivot table name (unprefixed; the package prefixes it like `join()` does) to get -real pivot behaviour. - -Omitted keys derive from the package's own foreign-key convention -(`{tableWithoutPrefix}_{primaryKey}`, e.g. `members_id` — note: plural, unlike -Laravel's singular default): - -| Argument | Default | Member (`members`) ↔ Role (`roles`) | -|---|---|---| -| `$foreignPivotKey` | parent `getForeignKey()` | `members_id` | -| `$relatedPivotKey` | related `getForeignKey()` | `roles_id` | -| `$parentKey` | parent `getPrimaryKey()` | `id` | -| `$relatedKey` | related `getPrimaryKey()` | `id` | +Resolves a many-to-many relation through a pivot (junction) table. Pass the +unprefixed pivot table name plus the parent/related key columns on it: ```php class Member extends Model @@ -535,37 +513,16 @@ class Member extends Model // pivot table role_user(member_id, role_id) return $this->belongsToMany(Role::class, 'role_user', 'member_id', 'role_id'); } - - // Carry extra pivot columns; they surface flat as `pivot_` attributes - public function rolesWithAssignment() - { - return $this->belongsToMany(Role::class, 'role_user', 'member_id', 'role_id') - ->withPivot(['assigned_at']); - } } ``` -The link column rides along on every related model as the reserved attribute -`pivot_` (e.g. `pivot_member_id`), and each `withPivot()` column -as `pivot_` (e.g. `pivot_assigned_at`). These flat `pivot_*` attributes -appear in `toArray()`. - -```php -// Eager -foreach (Member::with('roles')->get() as $member) { - foreach ($member->roles as $role) { - echo $role->pivot_member_id; // the parent link - } -} - -// Lazy -$member = Member::query()->findOne(['id' => 1]); -foreach ($member->roles as $role) { /* ... */ } -``` +With no `$pivotTable` the call keeps its **legacy** `hasMany`-style behaviour. +Pivot values surface as flat reserved `pivot_*` attributes; add extra ones with +`->withPivot([...])`. Pivot relations are **read-only** — `attach`/`detach`/`sync` +and aggregates over a pivot relation are not supported (the latter throw). -Read-only only: `attach`/`detach`/`sync` and `withCount`/`whereHas`/aggregates -over a pivot relation are **not** supported (the latter throw). See -[Limitations](#limitations--known-issues). +See [docs/relations.md](relations.md#belongstomany-many-to-many) for the full +signature, key-defaults table, `withPivot`, eager/lazy access, and limitations. ### Eager loading @@ -750,41 +707,20 @@ method relocation. ## Limitations & known issues -- **`belongsToMany` pivot relations are read-only and single-key.** Real - pivot-table many-to-many is supported for reads (eager `with()` + lazy - `$model->relation`), but with these gaps: - - No `withCount`/`whereHas`/aggregates on a pivot relation — they **throw** - `RuntimeException` (the pivot metadata has no single `foreignKey`/`localKey`). - - No `attach`/`detach`/`sync` (write side is out of scope). - - Single-column pivot/parent/related keys only — no composite keys. - - Duplicate pivot rows yield duplicate related models (no `DISTINCT`). - - Eager constraint closures may add `where`/`orderBy`/`limit` but **cannot** - narrow the selected columns — the pivot path always selects `related.*` so - the aliased pivot column can ride along. - - Pivot values surface as flat **reserved** `pivot_*` attributes (including the - link key `pivot_`) and appear in `toArray()`. A related - column literally named `pivot_*` would be overwritten. These attributes are - excluded from dirty-tracking on UPDATE, so re-saving a hydrated related model - is safe; a forced re-INSERT would attempt to write the non-existent columns. - - Null parent key: the eager path buckets a null parent key under null - (relation resolves to `null`); the lazy path renders `… IS NULL` and returns - pivot rows whose link column is NULL — a minor divergence. - -- **`belongsTo` and `hasOne` are the same alias; key naming is reversed from - Laravel.** Both set the same `oneToOne` relation. The `$foreignKey` argument - is the column on the **related** table and `$localKey` is the column on the - **calling** model's table — the opposite of Laravel's convention. Ensure you - supply both arguments explicitly to avoid confusion. - -- **Soft delete reads are unfiltered by default.** `softDeletes()` adds a - `deleted_at` column and `delete()` sets it. Reads return all rows — including - trashed ones — unless you also declare `public $soft_delete_scope = true;` on - the model. With the flag set, reads automatically exclude trashed rows; - `->withTrashed()` includes them and `->onlyTrashed()` returns only trashed rows. - **Edge case:** `refresh()` / `exists()` use a default (now scoped) read, so a - trashed opt-in model reloaded without `->withTrashed()` reports `exists() === false` - and a subsequent `save()` will INSERT rather than UPDATE. Follow-up planned to - thread soft-delete awareness into `refresh()`. +- **Relation limitations** — `belongsToMany` pivot relations are read-only and + single-key (no `attach`/`detach`/`sync`, no aggregates over a pivot relation), + and `belongsTo`/`hasOne` share one `oneToOne` alias whose key naming is + **reversed from Laravel**. Full detail: + [docs/relations.md](relations.md#limitations). + +- **Soft delete reads exclude trashed rows by default.** `softDeletes()` adds a + `deleted_at` column and `delete()` sets it. Reads automatically filter out + trashed rows (`deleted_at IS NULL`); `->withTrashed()` includes them and + `->onlyTrashed()` returns only trashed rows. Declare + `public $soft_delete_scope = false;` on the model to opt out of the filter and + read every row, including trashed ones. `refresh()` reloads a row by its own + primary key with `withTrashed()`, so re-hydrating a trashed model still reports + `exists() === true` and a following `save()` correctly UPDATEs. - **`upsert` is MySQL-only.** It generates `INSERT … ON DUPLICATE KEY UPDATE`, which is not portable to other databases. Workaround: use separate `insert` + From 6b61445518b38d0a5080e9e93d58859ee985eabc Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Wed, 1 Jul 2026 11:26:36 +0600 Subject: [PATCH 25/32] test: add RED relation-resolution safety tests (impl pending) Failing tests pinning the desired behavior for relation-name resolution: with()/withCount()/whereHas() must reject a non-relation method name with a RuntimeException, and getActiveRelationKey() must fail loudly on an unknown relation tag rather than returning a silent null. These are the RED phase for the not-yet-implemented relation-safety fix (option alpha) and fail until that guard lands. Assisted-By: AI --- tests/Fixtures/RelationSentinel.php | 28 +++++++ tests/RelationResolutionSafetyTest.php | 101 +++++++++++++++++++++++++ tests/bootstrap.php | 1 + 3 files changed, 130 insertions(+) create mode 100644 tests/Fixtures/RelationSentinel.php create mode 100644 tests/RelationResolutionSafetyTest.php diff --git a/tests/Fixtures/RelationSentinel.php b/tests/Fixtures/RelationSentinel.php new file mode 100644 index 0000000..ad0a253 --- /dev/null +++ b/tests/Fixtures/RelationSentinel.php @@ -0,0 +1,28 @@ +hasMany(Post::class, 'sentinel_id', 'id'); + } + + /** NOT a relation — must be rejected, never used as one. */ + public function destroyTheWorld() + { + return 'not a query builder'; + } +} diff --git a/tests/RelationResolutionSafetyTest.php b/tests/RelationResolutionSafetyTest.php new file mode 100644 index 0000000..3a608e8 --- /dev/null +++ b/tests/RelationResolutionSafetyTest.php @@ -0,0 +1,101 @@ +assertInstanceOf( + RuntimeException::class, + $caught, + 'a non-relation method name must be rejected at resolution with a RuntimeException' + ); + } + + public function testWithRejectsNonRelationMethod(): void + { + // must be rejected at resolution (the with() call), not blow up later. + $this->assertRejectedAsRelation(static function () { + RelationSentinel::with('destroyTheWorld'); + }); + } + + public function testWithCountRejectsNonRelationMethod(): void + { + $this->assertRejectedAsRelation(static function () { + RelationSentinel::withCount('destroyTheWorld'); + }); + } + + public function testWhereHasRejectsNonRelationMethod(): void + { + $this->assertRejectedAsRelation(static function () { + RelationSentinel::whereHas('destroyTheWorld'); + }); + } + + public function testGenuineRelationStillResolves(): void + { + // zero-BC guard: a real relation method must keep resolving, never rejected. + try { + RelationSentinel::with('posts')->get(); + } catch (RuntimeException $e) { + $this->fail('a genuine relation must not be rejected: ' . $e->getMessage()); + } + + $this->addToAssertionCount(1); + } + + public function testGetActiveRelationKeyFailsLoudlyOnUnknownTag(): void + { + $model = new RelationSentinel(); + $model->setRelateAs('not_a_real_tag'); + + $threw = false; + + try { + @$model->getActiveRelationKey(); + } catch (Throwable $e) { + $threw = true; + } + + $this->assertTrue( + $threw, + 'getActiveRelationKey() must fail loudly on an unknown relation tag, not return a silent null' + ); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index e5c5697..e19a4b0 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -138,3 +138,4 @@ public function has_cap($cap) require __DIR__ . '/Fixtures/Role.php'; require __DIR__ . '/Fixtures/Member.php'; require __DIR__ . '/Fixtures/PrefixedModel.php'; +require __DIR__ . '/Fixtures/RelationSentinel.php'; From 51db655ae7a34467231a0b8b8919a7f84e837f58 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Wed, 1 Jul 2026 11:51:14 +0600 Subject: [PATCH 26/32] fix: reject non-relation names in with/withCount/whereHas (option alpha) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Relation resolution (prepareRelation, the single chokepoint for with/ withCount/whereHas/withWhereHas) previously called any method whose name matched a relation, gated only by method_exists — letting a framework Model method (e.g. refresh(), which runs a SELECT) or any consumer no-arg method run through a relation name, and failing messily on typos. Now: a name declared on the framework Model is rejected WITHOUT being called (declaring-class identity via ReflectionMethod, compared to Model::class so it holds under php-scoper; memoized per class::method); a consumer-declared method is called and its return validated as a relation query (QueryBuilder whose model has an active relation key) or rejected with a RuntimeException. A name that fails method_exists is still silently skipped (zero-BC for optional/typo'd relation lists). getActiveRelationKey() now throws on an unknown relation tag instead of returning a silent null. Turns the RED RelationResolutionSafetyTest green (216 tests, all pass). Assisted-By: AI --- docs/relations.md | 9 ++++ src/Concerns/Relations.php | 94 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 100 insertions(+), 3 deletions(-) diff --git a/docs/relations.md b/docs/relations.md index 4fa2a73..8348c04 100644 --- a/docs/relations.md +++ b/docs/relations.md @@ -242,6 +242,15 @@ which is meaningful only for `withCount` and `withExists`. ## Limitations +- **Relation names must not be untrusted input.** `with()`, `whereHas()`, + `withCount()` (and the other `with*` aggregates) resolve a relation by calling + the model method of that name. A name that is not a relation is rejected with a + `RuntimeException` ("Relation [x] is not defined on [Class]."), and framework + `Model` methods are rejected **without** being called — but a consumer model's + own no-arg method is invoked once before its non-relation return is discarded. + Pass only trusted, code-defined relation names (same contract as Eloquent), + never a raw request value. + - **`belongsTo` and `hasOne` are the same alias; key naming is reversed from Laravel.** Both set the `oneToOne` relation. `$foreignKey` is the column on the **related** table and `$localKey` is the column on the **calling** model's diff --git a/src/Concerns/Relations.php b/src/Concerns/Relations.php index f32e6aa..8ae2126 100644 --- a/src/Concerns/Relations.php +++ b/src/Concerns/Relations.php @@ -23,6 +23,9 @@ trait Relations private $_relationKeys = []; + /** Memoized framework-vs-consumer verdict per "class::method". */ + private static $relationMethodCache = []; + /** * Undocumented function. * @@ -185,7 +188,14 @@ public function getRelationalKeys() */ public function getActiveRelationKey() { - return $this->getRelationalKeys()[$this->getRelateAs()]; + $relateAs = $this->getRelateAs(); + $relationalKeys = $this->getRelationalKeys(); + + if (!isset($relationalKeys[$relateAs])) { + throw new RuntimeException('No relation keys for relation tag [' . $relateAs . '].'); + } + + return $relationalKeys[$relateAs]; } /** @@ -200,14 +210,14 @@ public function prepareRelation(array $relations): array $preparedRelation = []; foreach ($relations as $key => $value) { if (\is_int($key) && \is_string($value) && ($method = explode(' ', $value)[0]) && method_exists($this, $method)) { - $preparedRelation[$value] = $this->{$method}(); + $preparedRelation[$value] = $this->resolveRelationQuery($method); unset($relations[$key]); } } foreach ($relations as $key => $value) { if (\is_string($key) && ($method = explode(' ', $key)[0]) && method_exists($this, $method)) { - $preparedRelation[$key] = $this->{$method}(); + $preparedRelation[$key] = $this->resolveRelationQuery($method); if ($value instanceof Closure) { $value($preparedRelation[$key]); } @@ -217,6 +227,84 @@ public function prepareRelation(array $relations): array return $preparedRelation; } + /** + * Resolves an existing method name to its relation query, or fails loudly. + * + * The name has already passed method_exists() — a missing name is silently + * skipped by the caller (zero-BC for optional/typo'd relation lists). A method + * declared on the framework Model is rejected WITHOUT being called, so a + * side-effecting method (e.g. refresh()) can never run through a relation + * name; a consumer-declared method is called and its return validated. + * + * @param string $method + * + * @return QueryBuilder + */ + private function resolveRelationQuery($method) + { + if ($this->isFrameworkModelMethod($method)) { + throw new RuntimeException($this->undefinedRelationMessage($method)); + } + + $result = $this->{$method}(); + + if (!$this->isRelationQuery($result)) { + throw new RuntimeException($this->undefinedRelationMessage($method)); + } + + return $result; + } + + /** + * True when $method is declared on the framework Model itself (traits flatten + * into the using class, so relation/write helpers all report Model as their + * declaring class) rather than on a consumer subclass. Compares against + * Model::class so the check survives php-scoper's namespace prefixing. + * + * @param string $method + * + * @return bool + */ + private function isFrameworkModelMethod($method) + { + $cacheKey = \get_class($this) . '::' . $method; + if (isset(self::$relationMethodCache[$cacheKey])) { + return self::$relationMethodCache[$cacheKey]; + } + + $declaringClass = (new \ReflectionMethod($this, $method))->getDeclaringClass()->getName(); + + return self::$relationMethodCache[$cacheKey] = $declaringClass === Model::class; + } + + /** + * True when $result is a relation query: a QueryBuilder whose model carries + * an active relation-key entry. + * + * @param mixed $result + * + * @return bool + */ + private function isRelationQuery($result) + { + if (!$result instanceof QueryBuilder) { + return false; + } + + try { + $result->getModel()->getActiveRelationKey(); + } catch (RuntimeException $e) { + return false; + } + + return true; + } + + private function undefinedRelationMessage($method) + { + return 'Relation [' . $method . '] is not defined on [' . \get_class($this) . '].'; + } + public function prepareRelationName(string $relationName): array { $name = $relationName; From cf1055fc1505048a0fdedfccd05134fe3b149675 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Wed, 1 Jul 2026 13:02:39 +0600 Subject: [PATCH 27/32] docs: note non-obvious v2.0 migration gotchas (from bit-pi audit) Add the empirically-confirmed footguns a v1 consumer hits, under their existing sections: - 2.1 an is_array($result) "got rows?" guard now inverts (Collection when rows exist, [] when empty) -> silent data loss; use empty()/!empty(). - 2.2 $model->update([...])->save() fatals ("save() on false") on a 0-row update -> drop the trailing ->save(). - 2.4 a function/expression in select() only "survives" if it contains a '.', so CONCAT(url,...) breaks on dotless hosts (localhost); use selectRaw. - 2.9 a leftover no-arg ->withCount() throws ArgumentCountError, including when reached via eager-load relation resolution. Assisted-By: AI --- docs/breaking-changes.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index 1cbc66c..7422988 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -72,6 +72,12 @@ $users->all(); // underlying plain array $users->toArray(); // array of model arrays ``` +> **Silent-data-loss trap:** an `if (is_array($result)) { … }` "got rows?" guard +> now **inverts** — a non-empty read is a `Collection` (`is_array` → false) while +> a zero-row read is a real `[]` (`is_array` → true) — so the branch runs only +> when there is *nothing* to process, silently dropping data whenever rows exist. +> Replace such guards with `empty($result)` / `!empty($result)`. + --- ### 2.2 `QueryBuilder::update()` is no longer chainable and executes immediately @@ -94,6 +100,13 @@ attributes and executes. **Migration:** set conditions **before** `update()`; drop trailing `->save()`/`->exec()`. +> **Runtime fatal on a chained `->save()`:** `update()` returns the *result* +> (`Model|false` for an existing model, `int|false` for a fresh one), never the +> builder — so `$model->update([...])->save()` fatals (`Call to a member function +> save() on false`) whenever the UPDATE changes **0 rows** (e.g. an idempotent +> re-save where no value actually differs, which MySQL reports as 0 affected +> rows). Drop the trailing `->save()` — `update()` already persisted. + --- ### 2.3 `QueryBuilder::save()` return value changed @@ -143,6 +156,13 @@ column and keeps the alias separate, so `->select(['id', 'title AS t'])` emits ->selectRaw('SUM(amount) as amt', $bindings) ``` +> **Gotcha — an expression may "accidentally" survive:** `prepareColumnName()` +> passes a column through untouched only when it already contains a `.`. So +> `select(['CONCAT("https://example.com/…", col) as x'])` emits valid SQL *merely* +> because the URL contains a dot — the identical code breaks on a dotless host +> (`http://localhost/…`). Never rely on this; route any function/expression +> through `selectRaw()`. + --- ### 2.5 `delete()` with no `WHERE` clause no longer wipes the table @@ -243,6 +263,11 @@ User::withCount('posts')->get(); // adds posts_count sub-select **Migration:** for a plain row count use `->count()`; for relation counts use `withCount($relation)`. See §4.2 for the full aggregate family. +> **Runtime fatal:** a leftover no-arg `->withCount()` — including one buried in +> a relation method that is later eager-loaded via `with('rel')` (the relation +> resolver invokes the method to validate it) — now throws `ArgumentCountError`; +> the relation-name parameter is required. + --- ### 2.10 `addRelation()` signature changed From a37a8853eb6cb6ad2f3efa1794b4cf5f37e3e31c Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Wed, 1 Jul 2026 13:16:15 +0600 Subject: [PATCH 28/32] fix: save() treats a 0-row UPDATE as success, not failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit exec() returns rows-affected (0 for a no-op UPDATE) and false only on a real DB error or a cancelled pre-event. save()'s existing-model branch checked exec() for truthiness, so a valid UPDATE that changed no rows (an idempotent re-save where no value differs) returned false — misreporting success as failure. This was the root cause of `$model->update([...])->save()` fataling (false->save()) and made `if (!$model->save())` falsely error on no-op saves. Compare against false so any non-error result (including 0 rows) returns the Model and fires 'saved', consistent with the 'updated' event that already fired. Insert path and genuine-error path unchanged. Documented in breaking-changes.md §3 (+ §2.2 note updated). Adds SaveZeroRowUpdateFixTest (0-row returns Model, chained update()->save() no fatal, real error still false). Assisted-By: AI --- docs/breaking-changes.md | 17 +++++-- src/QueryBuilder.php | 3 +- tests/SaveZeroRowUpdateFixTest.php | 73 ++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 tests/SaveZeroRowUpdateFixTest.php diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index 7422988..12012a0 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -100,12 +100,14 @@ attributes and executes. **Migration:** set conditions **before** `update()`; drop trailing `->save()`/`->exec()`. -> **Runtime fatal on a chained `->save()`:** `update()` returns the *result* +> **Don't chain after `update()`:** `update()` returns the *result* > (`Model|false` for an existing model, `int|false` for a fresh one), never the -> builder — so `$model->update([...])->save()` fatals (`Call to a member function -> save() on false`) whenever the UPDATE changes **0 rows** (e.g. an idempotent -> re-save where no value actually differs, which MySQL reports as 0 affected -> rows). Drop the trailing `->save()` — `update()` already persisted. +> builder, so a trailing builder call breaks. On an **existing** model +> `$model->update([...])->save()` now works — the `->save()` lands on the returned +> Model and no-ops (redundant) — but on a **fresh** model the `int` return still +> fatals (`save()` on int). Drop the trailing `->save()`; `update()` already +> persisted. (Before the `save()` 0-row fix in §3, the existing-model chain also +> fataled on any no-op update.) --- @@ -354,6 +356,11 @@ Not signature breaks, but observable runtime differences. Bulk inserts now return all created models. - **NULL columns persist as SQL `NULL`** in insert/update/upsert, instead of being coerced to an empty string `''`. +- **`save()` treats a 0-row UPDATE as success.** A successful UPDATE that changes + no rows (an idempotent re-save where no value differs) now returns the Model + instead of `false` — `exec()` returns `false` only on a real DB error/cancel, + so `save()` no longer misreports a no-op update as a failure. The insert path + and the genuine-error path (returns `false`) are unchanged. - **`paginate()`** defaults `select` to `*` when empty and computes the count before applying limit/offset; pagination with explicit `select` columns and count no longer conflict. diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index 8e0edad..747e621 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -1154,7 +1154,8 @@ public function save() $this->update = $columns; - if ($this->exec()) { + // 0-row (no-op) UPDATE is success; exec() is false only on error/cancel. + if ($this->exec() !== false) { $this->_model->fireEvent('saved'); return $this->_model; diff --git a/tests/SaveZeroRowUpdateFixTest.php b/tests/SaveZeroRowUpdateFixTest.php new file mode 100644 index 0000000..1f5fc5f --- /dev/null +++ b/tests/SaveZeroRowUpdateFixTest.php @@ -0,0 +1,73 @@ +save()` fatals on `false->save()` and `if (!$model->save())` + * falsely reports an error. A genuine error must still return false. + */ +final class SaveZeroRowUpdateFixTest extends TestCase +{ + protected function setUp(): void + { + $GLOBALS['wpdb'] = new FakeWpdb(); + } + + protected function tearDown(): void + { + $GLOBALS['wpdb'] = new FakeWpdb(); + } + + private function existingUser(): User + { + $GLOBALS['wpdb']->resolver = static function () { + return [(object) ['id' => 1, 'name' => 'Ada']]; + }; + + return User::query()->where('id', 1)->first(); + } + + public function testZeroRowUpdateReturnsModelNotFalse(): void + { + $user = $this->existingUser(); + $user->name = 'Grace'; // dirty -> a real UPDATE is built + + $GLOBALS['wpdb']->rows_affected = 0; // matched the row but changed nothing + $GLOBALS['wpdb']->last_error = ''; + + $result = $user->save(); + + $this->assertSame($user, $result, 'a 0-row (no-op) UPDATE is success, must return the Model'); + } + + public function testChainedUpdateSaveDoesNotFatalOnZeroRowUpdate(): void + { + $user = $this->existingUser(); + + $GLOBALS['wpdb']->rows_affected = 0; + + // update() delegates to save() on an existing model; the trailing save() + // must land on a Model, not false. + $result = $user->update(['name' => 'Grace'])->save(); + + $this->assertSame($user, $result); + } + + public function testRealErrorStillReturnsFalse(): void + { + $user = $this->existingUser(); + $user->name = 'Grace'; + + $GLOBALS['wpdb']->rows_affected = 0; + $GLOBALS['wpdb']->last_error = 'ER_SOMETHING'; // exec() -> false + + $this->assertFalse($user->save(), 'a real DB error must still return false'); + } +} From 273fe83f15854cbf8d34d65f5122e501ff8d0fdd Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Wed, 1 Jul 2026 13:24:13 +0600 Subject: [PATCH 29/32] fix: save() insert decides success from exec(), keeps lastInsertId() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit save()'s insert branch decided success by lastInsertId() ($wpdb->insert_id), so a successful INSERT into a table with a manual/composite key (insert_id 0) returned false and never fired 'saved'. It also risked returning the Model on a FAILED insert when insert_id held a stale non-zero value from an earlier insert in the same request. Decide success from exec() (false only on real error/cancel), then still assign the auto-increment id to the primary key when present. A manual/ composite-key insert now returns the Model; a genuine error returns false before touching lastInsertId (no stale PK, no 'saved'). Documented in breaking-changes.md §3. Adds SaveInsertReturnFixTest (auto-increment sets PK; manual-PK returns Model; error returns false). Assisted-By: AI --- docs/breaking-changes.md | 11 +++--- src/QueryBuilder.php | 14 ++++--- tests/SaveInsertReturnFixTest.php | 64 +++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 10 deletions(-) create mode 100644 tests/SaveInsertReturnFixTest.php diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index 12012a0..36d3e90 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -356,11 +356,12 @@ Not signature breaks, but observable runtime differences. Bulk inserts now return all created models. - **NULL columns persist as SQL `NULL`** in insert/update/upsert, instead of being coerced to an empty string `''`. -- **`save()` treats a 0-row UPDATE as success.** A successful UPDATE that changes - no rows (an idempotent re-save where no value differs) now returns the Model - instead of `false` — `exec()` returns `false` only on a real DB error/cancel, - so `save()` no longer misreports a no-op update as a failure. The insert path - and the genuine-error path (returns `false`) are unchanged. +- **`save()` decides success from the query result, not the affected/returned id.** + A successful UPDATE that changes no rows (an idempotent re-save where no value + differs) and a successful INSERT into a table with a manual/composite key (no + auto-increment id) both now return the Model instead of `false` — `exec()` + returns `false` only on a real DB error/cancel. The auto-increment id is still + assigned to the primary key when present. Genuine errors still return `false`. - **`paginate()`** defaults `select` to `*` when empty and computes the count before applying limit/offset; pagination with explicit `select` columns and count no longer conflict. diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index 747e621..c1aad5d 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -1165,15 +1165,19 @@ public function save() } $this->insert = $columns; - $this->exec(); + if ($this->exec() === false) { + return false; + } + + // Set the PK from the auto-increment id when there is one; a table with a + // manual/composite key returns insert_id 0 yet the insert still succeeded. if ($insertId = $this->lastInsertId()) { $this->_model->setAttribute($pk, $insertId); - $this->_model->fireEvent('saved'); - - return $this->_model; } - return false; + $this->_model->fireEvent('saved'); + + return $this->_model; } /** diff --git a/tests/SaveInsertReturnFixTest.php b/tests/SaveInsertReturnFixTest.php new file mode 100644 index 0000000..3147735 --- /dev/null +++ b/tests/SaveInsertReturnFixTest.php @@ -0,0 +1,64 @@ +insert_id = 7; + $GLOBALS['wpdb']->rows_affected = 1; + + $user = new User(); + $user->name = 'Ada'; + $result = $user->save(); + + $this->assertSame($user, $result); + $this->assertEquals(7, $user->getAttribute('id'), 'auto-increment id must be set from lastInsertId()'); + } + + public function testManualPkInsertReturnsModelWhenNoAutoIncrementId(): void + { + $GLOBALS['wpdb']->insert_id = 0; // manual/composite key -> no auto id + $GLOBALS['wpdb']->rows_affected = 1; // insert succeeded + $GLOBALS['wpdb']->last_error = ''; + + $user = new User(); + $user->name = 'Ada'; + $result = $user->save(); + + $this->assertSame($user, $result, 'successful insert with no auto-increment id must return the Model'); + } + + public function testInsertErrorStillReturnsFalse(): void + { + $GLOBALS['wpdb']->insert_id = 0; + $GLOBALS['wpdb']->last_error = 'ER_DUP_ENTRY'; // exec() -> false + + $user = new User(); + $user->name = 'Ada'; + + $this->assertFalse($user->save(), 'a failed insert must return false'); + } +} From 4292e159d67c0ec5652279329af75ba6277b5d3b Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Wed, 1 Jul 2026 13:32:07 +0600 Subject: [PATCH 30/32] test: cover session-change edge cases; fix null-offset deprecation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the edge cases the session's fixes introduced but left unverified: - soft-delete: refresh() reloads a trashed row without the scope filter (withTrashed) so exists() stays true. - relation guard: a method returning a plain QueryBuilder with no active relation key is rejected; a framework Model method is rejected; the 4th entry point withWhereHas() rejects a non-relation too. - save() insert: a failed insert carrying a stale non-zero insert_id returns false and does not set the PK from it. - saved event fires on a 0-row UPDATE and a manual-PK INSERT, and never on a failed write (new SavedEventUser fixture). getActiveRelationKey() short-circuits a null relation tag before the isset() so it no longer trips PHP 8's "null as array offset" deprecation (surfaced by the plain-QueryBuilder rejection test). Behavior unchanged — both still throw. Assisted-By: AI --- src/Concerns/Relations.php | 3 +- tests/Fixtures/RelationSentinel.php | 6 +++ tests/Fixtures/SavedEventUser.php | 29 +++++++++++ tests/RelationResolutionSafetyTest.php | 26 ++++++++++ tests/SaveEventFiresTest.php | 69 ++++++++++++++++++++++++++ tests/SaveInsertReturnFixTest.php | 13 +++++ tests/SoftDeleteScopeTest.php | 18 +++++++ tests/bootstrap.php | 1 + 8 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 tests/Fixtures/SavedEventUser.php create mode 100644 tests/SaveEventFiresTest.php diff --git a/src/Concerns/Relations.php b/src/Concerns/Relations.php index 8ae2126..305d6a2 100644 --- a/src/Concerns/Relations.php +++ b/src/Concerns/Relations.php @@ -191,7 +191,8 @@ public function getActiveRelationKey() $relateAs = $this->getRelateAs(); $relationalKeys = $this->getRelationalKeys(); - if (!isset($relationalKeys[$relateAs])) { + // Short-circuit a null tag: isset($array[null]) is a deprecated null offset. + if ($relateAs === null || !isset($relationalKeys[$relateAs])) { throw new RuntimeException('No relation keys for relation tag [' . $relateAs . '].'); } diff --git a/tests/Fixtures/RelationSentinel.php b/tests/Fixtures/RelationSentinel.php index ad0a253..dc5ec3f 100644 --- a/tests/Fixtures/RelationSentinel.php +++ b/tests/Fixtures/RelationSentinel.php @@ -25,4 +25,10 @@ public function destroyTheWorld() { return 'not a query builder'; } + + /** Returns a QueryBuilder, but NOT a relation query (no active relation key). */ + public function plainQuery() + { + return self::query(); + } } diff --git a/tests/Fixtures/SavedEventUser.php b/tests/Fixtures/SavedEventUser.php new file mode 100644 index 0000000..7fde220 --- /dev/null +++ b/tests/Fixtures/SavedEventUser.php @@ -0,0 +1,29 @@ +assertRejectedAsRelation(static function () { + RelationSentinel::with('plainQuery'); + }); + } + + // A framework Model method (refresh() runs a SELECT) must be rejected — and, + // per option-α, never invoked. + public function testFrameworkModelMethodIsRejected(): void + { + $this->assertRejectedAsRelation(static function () { + RelationSentinel::with('refresh'); + }); + } + + // withWhereHas() is the 4th resolution entry point and must reject too. + public function testWithWhereHasRejectsNonRelationMethod(): void + { + $this->assertRejectedAsRelation(static function () { + RelationSentinel::withWhereHas('destroyTheWorld'); + }); + } } diff --git a/tests/SaveEventFiresTest.php b/tests/SaveEventFiresTest.php new file mode 100644 index 0000000..b961e7b --- /dev/null +++ b/tests/SaveEventFiresTest.php @@ -0,0 +1,69 @@ +resolver = static function () { + return [(object) ['id' => 1, 'name' => 'Ada']]; + }; + + $user = SavedEventUser::query()->where('id', 1)->first(); // existing + $user->name = 'Grace'; + + $GLOBALS['wpdb']->rows_affected = 0; // no-op UPDATE + SavedEventUser::$savedCount = 0; // ignore anything fired during load + + $user->save(); + + $this->assertSame(1, SavedEventUser::$savedCount, 'saved must fire on a 0-row UPDATE'); + } + + public function testSavedFiresOnManualPkInsert(): void + { + $GLOBALS['wpdb']->insert_id = 0; // no auto-increment id + $GLOBALS['wpdb']->rows_affected = 1; // insert succeeded + $GLOBALS['wpdb']->last_error = ''; + + $user = new SavedEventUser(); + $user->name = 'Ada'; + $user->save(); + + $this->assertSame(1, SavedEventUser::$savedCount, 'saved must fire on a successful manual-PK INSERT'); + } + + public function testSavedDoesNotFireOnFailedWrite(): void + { + $GLOBALS['wpdb']->last_error = 'ER_DUP_ENTRY'; // exec() -> false + + $user = new SavedEventUser(); + $user->name = 'Ada'; + $result = $user->save(); + + $this->assertFalse($result); + $this->assertSame(0, SavedEventUser::$savedCount, 'saved must NOT fire when the write fails'); + } +} diff --git a/tests/SaveInsertReturnFixTest.php b/tests/SaveInsertReturnFixTest.php index 3147735..82f0e41 100644 --- a/tests/SaveInsertReturnFixTest.php +++ b/tests/SaveInsertReturnFixTest.php @@ -61,4 +61,17 @@ public function testInsertErrorStillReturnsFalse(): void $this->assertFalse($user->save(), 'a failed insert must return false'); } + + public function testFailedInsertWithStaleInsertIdReturnsFalseAndDoesNotSetPk(): void + { + $GLOBALS['wpdb']->insert_id = 99; // stale id from an earlier insert + $GLOBALS['wpdb']->last_error = 'ER_DUP_ENTRY'; // this insert failed -> exec() false + + $user = new User(); + $user->name = 'Ada'; + $result = $user->save(); + + $this->assertFalse($result, 'a failed insert must return false even with a stale insert_id'); + $this->assertNotEquals(99, $user->getAttribute('id'), 'a failed insert must not set the PK from a stale insert_id'); + } } diff --git a/tests/SoftDeleteScopeTest.php b/tests/SoftDeleteScopeTest.php index 0e13578..8a956e0 100644 --- a/tests/SoftDeleteScopeTest.php +++ b/tests/SoftDeleteScopeTest.php @@ -113,4 +113,22 @@ public function testOnlyTrashedOrWhereGroupsUserConditions(): void $this->assertMatchesRegularExpression('/\(.*status.*OR.*status.*\)/s', $sql); $this->assertMatchesRegularExpression('/\).*deleted_at.*IS\s+NOT\s+NULL/s', $sql); } + + // refresh() reloads a row by PK with withTrashed(), so a trashed row is still + // found and exists() stays true (no re-INSERT on the next save()). + public function testRefreshReloadsTrashedRowWithoutScopeFilter(): void + { + $GLOBALS['wpdb']->resolver = static function () { + return [(object) ['id' => 1, 'deleted_at' => '2026-01-01 00:00:00']]; + }; + + $post = new SoftPost(1); // constructor triggers refresh() + + $this->assertTrue($post->exists(), 'refresh() must find the row even when trashed'); + $this->assertStringNotContainsString( + 'deleted_at', + $GLOBALS['wpdb']->last_query, + 'refresh() must not scope out trashed rows (withTrashed)' + ); + } } diff --git a/tests/bootstrap.php b/tests/bootstrap.php index e19a4b0..3d2d749 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -127,6 +127,7 @@ public function has_cap($cap) require __DIR__ . '/Fixtures/User.php'; require __DIR__ . '/Fixtures/SoftPost.php'; require __DIR__ . '/Fixtures/EventUser.php'; +require __DIR__ . '/Fixtures/SavedEventUser.php'; require __DIR__ . '/Fixtures/RetrieveUser.php'; require __DIR__ . '/Fixtures/CastModel.php'; require __DIR__ . '/Fixtures/CastAliasModel.php'; From 21c4fd696fb500625c468bfeb0c64683c881b5c8 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Wed, 1 Jul 2026 13:45:27 +0600 Subject: [PATCH 31/32] test: close remaining coverage gaps (memoization, base-class relation, events) Cover the three edges previously flagged as skipped: - relation guard memoizes the framework-vs-consumer verdict per class::method (reflection-asserted: refresh -> true, posts -> false, no collision). - a relation declared on an intermediate base class (leaf -> base -> Model) resolves, since its declaring class is the base, not the framework Model. - created fires on a successful INSERT and updated fires even on a 0-row UPDATE (SavedEventUser gains created/updated counters). New fixtures RelationBaseModel/RelationLeafModel. The memoization test guards ReflectionProperty::setAccessible() behind PHP_VERSION_ID (required on 7.4, a deprecated no-op on 8.1+) to keep the suite deprecation-clean. Assisted-By: AI --- tests/Fixtures/RelationBaseModel.php | 22 +++++++++++++ tests/Fixtures/RelationLeafModel.php | 11 +++++++ tests/Fixtures/SavedEventUser.php | 17 ++++++++++ tests/RelationResolutionSafetyTest.php | 44 ++++++++++++++++++++++++++ tests/SaveEventFiresTest.php | 39 ++++++++++++++++++++--- tests/bootstrap.php | 2 ++ 6 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 tests/Fixtures/RelationBaseModel.php create mode 100644 tests/Fixtures/RelationLeafModel.php diff --git a/tests/Fixtures/RelationBaseModel.php b/tests/Fixtures/RelationBaseModel.php new file mode 100644 index 0000000..99b1b87 --- /dev/null +++ b/tests/Fixtures/RelationBaseModel.php @@ -0,0 +1,22 @@ +hasMany(Post::class, 'base_id', 'id'); + } +} diff --git a/tests/Fixtures/RelationLeafModel.php b/tests/Fixtures/RelationLeafModel.php new file mode 100644 index 0000000..28d94d2 --- /dev/null +++ b/tests/Fixtures/RelationLeafModel.php @@ -0,0 +1,11 @@ + base -> Model) + // must resolve — its declaring class is the base, not the framework Model. + public function testRelationOnIntermediateBaseClassIsAllowed(): void + { + $threw = false; + + try { + RelationLeafModel::with('widgets'); + } catch (Throwable $e) { + $threw = true; + } + + $this->assertFalse($threw, 'a relation declared on an intermediate base class must resolve'); + } + + // The framework-vs-consumer verdict is memoized per "class::method"; the + // cache must hold the correct boolean for each and not collide across methods. + public function testFrameworkVerdictIsMemoizedPerClassMethod(): void + { + $cacheProp = new ReflectionProperty(Model::class, 'relationMethodCache'); + if (\PHP_VERSION_ID < 80100) { + $cacheProp->setAccessible(true); // required on 7.4; a deprecated no-op on 8.1+ + } + $cacheProp->setValue(null, []); + + try { + RelationSentinel::with('refresh'); // framework method -> rejected + } catch (RuntimeException $e) { + // expected + } + RelationSentinel::with('posts'); // consumer relation -> resolves + + $cache = $cacheProp->getValue(); + $prefix = RelationSentinel::class; + + $this->assertArrayHasKey($prefix . '::refresh', $cache); + $this->assertTrue($cache[$prefix . '::refresh'], 'refresh memoized as a framework method'); + $this->assertArrayHasKey($prefix . '::posts', $cache); + $this->assertFalse($cache[$prefix . '::posts'], 'posts memoized as a consumer method'); + } } diff --git a/tests/SaveEventFiresTest.php b/tests/SaveEventFiresTest.php index b961e7b..14401ce 100644 --- a/tests/SaveEventFiresTest.php +++ b/tests/SaveEventFiresTest.php @@ -15,14 +15,14 @@ final class SaveEventFiresTest extends TestCase { protected function setUp(): void { - $GLOBALS['wpdb'] = new FakeWpdb(); - SavedEventUser::$savedCount = 0; + $GLOBALS['wpdb'] = new FakeWpdb(); + SavedEventUser::resetCounters(); } protected function tearDown(): void { - $GLOBALS['wpdb'] = new FakeWpdb(); - SavedEventUser::$savedCount = 0; + $GLOBALS['wpdb'] = new FakeWpdb(); + SavedEventUser::resetCounters(); } public function testSavedFiresOnZeroRowUpdate(): void @@ -35,13 +35,42 @@ public function testSavedFiresOnZeroRowUpdate(): void $user->name = 'Grace'; $GLOBALS['wpdb']->rows_affected = 0; // no-op UPDATE - SavedEventUser::$savedCount = 0; // ignore anything fired during load + SavedEventUser::resetCounters(); // ignore anything fired during load $user->save(); $this->assertSame(1, SavedEventUser::$savedCount, 'saved must fire on a 0-row UPDATE'); } + public function testUpdatedFiresOnZeroRowUpdate(): void + { + $GLOBALS['wpdb']->resolver = static function () { + return [(object) ['id' => 1, 'name' => 'Ada']]; + }; + + $user = SavedEventUser::query()->where('id', 1)->first(); + $user->name = 'Grace'; + + $GLOBALS['wpdb']->rows_affected = 0; + SavedEventUser::resetCounters(); + + $user->save(); + + $this->assertSame(1, SavedEventUser::$updatedCount, 'updated must fire even on a 0-row UPDATE'); + } + + public function testCreatedFiresOnInsert(): void + { + $GLOBALS['wpdb']->insert_id = 5; + $GLOBALS['wpdb']->rows_affected = 1; + + $user = new SavedEventUser(); + $user->name = 'Ada'; + $user->save(); + + $this->assertSame(1, SavedEventUser::$createdCount, 'created must fire on a successful INSERT'); + } + public function testSavedFiresOnManualPkInsert(): void { $GLOBALS['wpdb']->insert_id = 0; // no auto-increment id diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 3d2d749..a84c415 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -140,3 +140,5 @@ public function has_cap($cap) require __DIR__ . '/Fixtures/Member.php'; require __DIR__ . '/Fixtures/PrefixedModel.php'; require __DIR__ . '/Fixtures/RelationSentinel.php'; +require __DIR__ . '/Fixtures/RelationBaseModel.php'; +require __DIR__ . '/Fixtures/RelationLeafModel.php'; From 6d8b1d3d183f39ada211caf7b608bde2ba9ecfc5 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Wed, 1 Jul 2026 14:13:50 +0600 Subject: [PATCH 32/32] build: require PHP >=8.2, add phpunit dev-dep + composer test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raise the minimum PHP from ^7.4 || ^8.0 to >=8.2 (breaking: the package no longer installs on PHP < 8.2). Add phpunit/phpunit ^11.5 as a dev dependency and a `composer test` script so tests run from the vendored PHPUnit instead of a gitignored phpunit.phar. Point the compat gate at 8.2- and pin config.platform.php to 8.2.0 for reproducible installs. Documented in breaking-changes.md §2.14 (+ summary row 14). Assisted-By: AI --- composer.json | 11 ++++++++--- docs/breaking-changes.md | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index 23eacd1..3dfd57f 100644 --- a/composer.json +++ b/composer.json @@ -29,7 +29,7 @@ ] }, "require": { - "php": "^7.4 || ^8.0" + "php": ">=8.2" }, "autoload": { "psr-4": { @@ -37,14 +37,16 @@ } }, "scripts": { + "test": "phpunit", "lint": "./vendor/bin/php-cs-fixer fix --config=.php-cs-fixer.php", - "compat": "./vendor/bin/phpcs -p ./src --standard=PHPCompatibilityWP --runtime-set testVersion 7.4-" + "compat": "./vendor/bin/phpcs -p ./src --standard=PHPCompatibilityWP --runtime-set testVersion 8.2-" }, "require-dev": { "friendsofphp/php-cs-fixer": "^3.10", "sirbrillig/phpcs-variable-analysis": "*", "dealerdirect/phpcodesniffer-composer-installer": "^0.7", - "phpcompatibility/phpcompatibility-wp": "*" + "phpcompatibility/phpcompatibility-wp": "*", + "phpunit/phpunit": "^11.5" }, "extra": { "branch-alias": { @@ -52,6 +54,9 @@ } }, "config": { + "platform": { + "php": "8.2.0" + }, "allow-plugins": { "dealerdirect/phpcodesniffer-composer-installer": true } diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index 36d3e90..b12df92 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -30,6 +30,7 @@ API and runtime behavior change, it is a **major** version bump. | 11 | Blueprint | `binary()` removed | Low | | 12 | QueryBuilder | exception type/message changed in `exec()` | Low | | 13 | Model | soft-delete reads exclude trashed by default (opt out with `$soft_delete_scope = false`) | Medium | +| 14 | Composer | minimum PHP raised to **8.2** (was 7.4) — package no longer installs on PHP < 8.2 | High | --- @@ -342,6 +343,21 @@ re-hydrating a trashed model still reports `exists() === true`. --- +### 2.14 Minimum PHP is now 8.2 + +`composer.json` `require.php` changed from `^7.4 || ^8.0` to `>=8.2`. The package +no longer installs on PHP 7.4, 8.0, or 8.1. + +**Why it breaks:** a plugin whose own `require.php` still allows < 8.2 can no +longer resolve this package version via Composer. + +**Migration:** raise the consuming plugin's minimum PHP to 8.2 (and its runtime) +before upgrading. Stay on the previous release if you must support older PHP. +The test suite runs on PHPUnit 11 (`composer test`); the compatibility gate now +targets `8.2-`. + +--- + ## 3. Behavioral changes Not signature breaks, but observable runtime differences.