From 0570f3151b2c8a6a942c75f49f8d14c8dc178345 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 29 Oct 2026 03:57:24 +0000 Subject: [PATCH 1/7] Preserve inherited queue attributes behind null child properties Queueable declares nullable routing properties on the child class. Those defaults must not hide Connection, Queue or Delay attributes inherited from a parent. Ignore null properties when deciding whether a child property overrides an attribute, while retaining non-null overrides and cached reflection metadata. Port laravel/framework#61826 from 1e3185debc2949e73d6d7fe3146bce09a8f127a6, reconciled against the recorded 13.x target 5d7797528723107cd4f495326838a7d7d67fbb20. Include the upstream parent/child dispatch regression with exact connection, queue and delay assertions. Validation: the regression fails before the correction. The complete dispatcher test file, affected queue attribute consumers, formatting and full source, type-fixture and skeleton analysis pass. --- .../src/Traits/ReadsClassAttributes.php | 1 + tests/Bus/BusDispatcherTest.php | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/support/src/Traits/ReadsClassAttributes.php b/src/support/src/Traits/ReadsClassAttributes.php index a29e59c8bf..d7e23c1597 100644 --- a/src/support/src/Traits/ReadsClassAttributes.php +++ b/src/support/src/Traits/ReadsClassAttributes.php @@ -81,6 +81,7 @@ protected function propertyOverridesAttribute(object $target, ReflectionClass $r return $propertyReflection->isPublic() && $propertyReflection->isInitialized($target) + && $target->{$property} !== null && $propertyReflection->getDeclaringClass()->isSubclassOf($attributeDeclaringClass->getName()); } } diff --git a/tests/Bus/BusDispatcherTest.php b/tests/Bus/BusDispatcherTest.php index c3e6adb7a8..a12add6310 100644 --- a/tests/Bus/BusDispatcherTest.php +++ b/tests/Bus/BusDispatcherTest.php @@ -10,6 +10,7 @@ use Hypervel\Container\Container; use Hypervel\Contracts\Queue\Queue; use Hypervel\Contracts\Queue\ShouldQueue; +use Hypervel\Queue\Attributes\Connection; use Hypervel\Queue\Attributes\Delay; use Hypervel\Queue\Attributes\Queue as QueueAttribute; use Hypervel\Queue\InteractsWithQueue; @@ -62,6 +63,26 @@ public function testCommandsThatShouldQueueIsQueuedUsingCustomQueueAndDelay(): v $dispatcher->dispatch(new BusDispatcherTestSpecificQueueAndDelayCommand); } + public function testCommandsAreQueuedUsingAttributesInheritedFromParentClass(): void + { + $container = new Container; + $container->instance('queue.routes', new QueueRoutes); + Container::setInstance($container); + $usedConnection = null; + $dispatcher = new Dispatcher($container, function (?string $connection) use (&$usedConnection): Queue { + $usedConnection = $connection; + + $mock = m::mock(Queue::class); + $mock->expects('later')->with(10, m::type(BusDispatcherTestQueueableChildCommand::class), '', 'foo'); + + return $mock; + }); + + $dispatcher->dispatch(new BusDispatcherTestQueueableChildCommand); + + $this->assertSame('redis', $usedConnection); + } + public function testCommandsThatShouldQueueIsQueuedUsingQueueAndDelayAttributes(): void { $container = new Container; @@ -359,6 +380,18 @@ class BusDispatcherTestSpecificQueueAndDelayAttributeWithPropertyCommand impleme use Queueable; } +#[Connection('redis')] +#[QueueAttribute('foo')] +#[Delay(10)] +abstract class BusDispatcherTestParentCommandWithAttributes implements ShouldQueue +{ +} + +class BusDispatcherTestQueueableChildCommand extends BusDispatcherTestParentCommandWithAttributes +{ + use Queueable; +} + class BusDispatcherQueueable implements ShouldQueue { use Queueable; From f98d3353f00edd5027fd4b7324194adf3f42163e Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 29 Oct 2026 03:57:42 +0000 Subject: [PATCH 2/7] Preserve Eloquent cast updates and invalidate stale accessor values JSON-path writes now merge pending object mutations before editing the stored value, use the correct encryption path for encrypted class casts, and clear stale accessor values afterward. Normal assignments also clear the accessor cache. Keep strict JSON handling and per-model cache ownership. Reconcile laravel/framework#61834 (09991edb59), #61847 (297e6acacb), #61962 (f618f7911d) and #62035 (bff5d2a24e) together against 13.x target 5d7797528723107cd4f495326838a7d7d67fbb20. These changes share the same attribute write paths and regression files; include every applicable upstream test and assertion. Correct related stale-cache paths through enum casts, custom class casts and both mutator forms. Invalidate after successful writes because callbacks can read and repopulate old values. Cache setter inputs only for attributes with getters, preventing a write-only input object from overwriting subsequent mutations to a class-cast object. Paired accessor and setter object retention remains intact. Encrypted class-cast dirty checks now use Crypt for decryption and previous keys, matching the casts themselves. A separately configured model encrypter continues to serve primitive encrypted casts. This prevents MAC failures and incorrect key-rotation decisions when the two encrypters differ. Validation: upstream and additional regressions reproduce the defects before correction. Complete model and encrypted-cast test files, affected Eloquent and casting integration tests on isolated SQLite, formatting and full source, type-fixture and skeleton analysis pass. Server-specific createOrFirst cases retain their existing driver requirements. --- .../src/Eloquent/Concerns/HasAttributes.php | 57 +++-- tests/Database/DatabaseEloquentModelTest.php | 217 ++++++++++++++++++ .../EloquentModelEncryptedCastingTest.php | 88 +++++++ 3 files changed, 349 insertions(+), 13 deletions(-) diff --git a/src/database/src/Eloquent/Concerns/HasAttributes.php b/src/database/src/Eloquent/Concerns/HasAttributes.php index 562cb8a028..b1d568fcca 100644 --- a/src/database/src/Eloquent/Concerns/HasAttributes.php +++ b/src/database/src/Eloquent/Concerns/HasAttributes.php @@ -1108,6 +1108,8 @@ public function setAttribute(string|int $key, mixed $value): mixed $this->attributes[$key] = $value; + unset($this->attributeCastCache[$key]); + return $this; } @@ -1149,7 +1151,12 @@ protected function setMutatedAttributeValue(string $key, mixed $value): mixed { $this->mergeAttributesFromCachedCasts(); - return $this->{'set' . StrCache::studly($key) . 'Attribute'}($value); + $result = $this->{'set' . StrCache::studly($key) . 'Attribute'}($value); + + // The setter may read and cache this attribute before replacing its stored value. + unset($this->classCastCache[$key], $this->attributeCastCache[$key]); + + return $result; } /** @@ -1173,7 +1180,10 @@ protected function setAttributeMarkedMutatedAttributeValue(string $key, mixed $v ) ); - if ($attribute->withCaching || (is_object($value) && $attribute->withObjectCaching)) { + unset($this->classCastCache[$key]); + + // Setter-only inputs must not overwrite later changes to objects returned by class casts. + if ($attribute->get && ($attribute->withCaching || (is_object($value) && $attribute->withObjectCaching))) { $this->attributeCastCache[$key] = $value; } else { unset($this->attributeCastCache[$key]); @@ -1198,20 +1208,26 @@ public function fillJsonAttribute(string $key, mixed $value): static { [$key, $path] = explode('->', $key, 2); + $this->mergeAttributeFromCachedCasts($key); + $value = $this->castAttributeAsJson($key, $this->getArrayAttributeWithValue( $path, $key, $value )); - $this->attributes[$key] = $this->isEncryptedCastable($key) - ? $this->castAttributeAsEncryptedString($key, $value) - : $value; + $this->attributes[$key] = match (true) { + $this->isEncryptedCastable($key) => $this->castAttributeAsEncryptedString($key, $value), + $this->isEncryptedClassCastable($key) => Crypt::encryptString($value), + default => $value, + }; if ($this->isClassCastable($key)) { unset($this->classCastCache[$key]); } + unset($this->attributeCastCache[$key]); + return $this; } @@ -1232,6 +1248,9 @@ protected function setClassCastableAttribute(string $key, mixed $value): void )) ); + // A custom caster may read the accessor while preparing the new stored value. + unset($this->attributeCastCache[$key]); + if ($caster instanceof CastsInboundAttributes || ! is_object($value) || ($caster->withoutObjectCaching ?? false)) { @@ -1260,6 +1279,8 @@ protected function setEnumCastableAttribute(string $key, mixed $value): void $this->getEnumCaseFromValue($enumClass, $value) ); } + + unset($this->attributeCastCache[$key]); } /** @@ -1309,11 +1330,11 @@ protected function getArrayAttributeByKey(string $key): array return []; } - return $this->fromJson( - $this->isEncryptedCastable($key) - ? $this->fromEncryptedString($this->attributes[$key]) - : $this->attributes[$key] - ); + return $this->fromJson(match (true) { + $this->isEncryptedCastable($key) => $this->fromEncryptedString($this->attributes[$key]), + $this->isEncryptedClassCastable($key) => Crypt::decryptString($this->attributes[$key]), + default => $this->attributes[$key], + }); } /** @@ -1390,7 +1411,7 @@ protected function castAttributeAsEncryptedString(string $key, #[SensitiveParame * Set the encrypter instance that will be used to encrypt attributes. * * Boot-only. The encrypter persists in a static property for the worker - * lifetime and is used by every encrypted attribute across all coroutines. + * lifetime and is used by primitive encrypted casts across all coroutines. */ public static function encryptUsing(?EncrypterContract $encrypter): void { @@ -1675,6 +1696,15 @@ protected function isEncryptedCastable(string $key): bool return $this->castMetadataCache['encryptedCastable'][$key] ??= $this->hasCast($key, ['encrypted', 'encrypted:array', 'encrypted:collection', 'encrypted:json', 'encrypted:object']); } + /** + * Determine whether a value is an encrypted class castable for inbound manipulation. + */ + protected function isEncryptedClassCastable(string $key): bool + { + return $this->isClassCastable($key) + && Str::startsWith($this->getCasts()[$key], [AsEncryptedArrayObject::class, AsEncryptedCollection::class]); + } + /** * Determine if the given key is cast using a custom class. * @@ -2365,8 +2395,9 @@ public function originalIsEquivalent(string $key): bool return $current === $original; } if ($this->isClassCastable($key) && $original !== null && Str::startsWith($this->getCasts()[$key], [AsEncryptedArrayObject::class, AsEncryptedCollection::class])) { - if (empty(static::currentEncrypter()->getPreviousKeys())) { - return $this->fromEncryptedString($attribute) === $this->fromEncryptedString($original); + // These casts use Crypt, independently of the model's custom encrypter. + if (empty(Crypt::getPreviousKeys())) { + return Crypt::decryptString($attribute) === Crypt::decryptString($original); } return false; diff --git a/tests/Database/DatabaseEloquentModelTest.php b/tests/Database/DatabaseEloquentModelTest.php index d8be4103a2..fdaa846d7a 100755 --- a/tests/Database/DatabaseEloquentModelTest.php +++ b/tests/Database/DatabaseEloquentModelTest.php @@ -1865,6 +1865,212 @@ public function testFillingJSONAttributes(): void ); } + public function testFillingJSONAttributesKeepsUnsavedChangesOnCastObjects(): void + { + $model = new CastingStub; + $model->setRawAttributes([ + 'asarrayobjectAttribute' => '{"foo":"bar"}', + 'ascollectionAttribute' => '{"foo":"bar"}', + 'asFluentAttribute' => '{"foo":"bar"}', + ]); + + $model->asarrayobjectAttribute['foo'] = 'baz'; + $model->ascollectionAttribute->put('foo', 'baz'); + $model->asFluentAttribute->foo = 'baz'; + + $model->forceFill([ + 'asarrayobjectAttribute->bar' => 'qux', + 'ascollectionAttribute->bar' => 'qux', + 'asFluentAttribute->bar' => 'qux', + ]); + + $this->assertSame(['foo' => 'baz', 'bar' => 'qux'], $model->asarrayobjectAttribute->getArrayCopy()); + $this->assertSame(['foo' => 'baz', 'bar' => 'qux'], $model->ascollectionAttribute->all()); + $this->assertSame(['foo' => 'baz', 'bar' => 'qux'], $model->asFluentAttribute->toArray()); + } + + public function testFillingJSONAttributesRefreshesObjectsCachedByAttributeAccessors(): void + { + $model = new class extends Model { + /** + * Get the settings attribute. + */ + protected function settings(): Attribute + { + return Attribute::make( + get: fn (string $value): Fluent => new Fluent(json_decode($value, true)), + set: fn (mixed $value): string|false => json_encode($value), + ); + } + }; + + $model->setRawAttributes(['settings' => '{"theme":"light"}'], true); + + $this->assertSame('light', $model->settings->theme); + + $model->forceFill(['settings->theme' => 'dark']); + + $this->assertSame('dark', $model->settings->theme); + $this->assertSame(['settings' => '{"theme":"dark"}'], $model->getDirty()); + } + + public function testSettingAttributesRefreshesObjectsCachedByGetOnlyAttributeAccessors(): void + { + $model = new class extends Model { + /** + * Get the settings attribute. + */ + protected function settings(): Attribute + { + return Attribute::get(fn (string $value): Fluent => new Fluent(json_decode($value, true))); + } + }; + + $model->setRawAttributes(['settings' => '{"theme":"light"}'], true); + + $this->assertSame('light', $model->settings->theme); + + $model->settings = '{"theme":"dark"}'; + + $this->assertSame('dark', $model->settings->theme); + } + + public function testSettingEnumAttributesRefreshesCachedGetOnlyAccessors(): void + { + $model = new class extends Model { + protected array $casts = ['status' => StringStatus::class]; + + /** + * Get the status attribute. + */ + protected function status(): Attribute + { + return Attribute::get(fn (string $value): Fluent => new Fluent(['value' => $value])); + } + }; + $model->setRawAttributes(['status' => 'draft'], true); + + $this->assertSame('draft', $model->status->value); + + $model->status = StringStatus::Done; + + $this->assertSame('done', $model->status->value); + $this->assertSame(['status' => 'done'], $model->getDirty()); + } + + public function testClassCastThatReadsAnAccessorDoesNotLeaveItsCachedValueStale(): void + { + $model = new class extends Model { + protected array $casts = ['settings' => MergingSettingsCast::class]; + + /** + * Get the settings attribute. + */ + protected function settings(): Attribute + { + return Attribute::get(fn (string $value): Fluent => new Fluent(json_decode($value, true))); + } + }; + $model->setRawAttributes(['settings' => '{"theme":"light","locale":"en"}'], true); + + $this->assertSame('light', $model->settings->theme); + + $model->settings = ['theme' => 'dark']; + + $this->assertSame(['theme' => 'dark', 'locale' => 'en'], $model->settings->toArray()); + $this->assertSame(['settings' => '{"theme":"dark","locale":"en"}'], $model->getDirty()); + } + + public function testLegacySetterThatReadsAnAccessorDoesNotLeaveItsCachedValueStale(): void + { + $model = new class extends Model { + /** + * Get the settings attribute. + */ + protected function settings(): Attribute + { + return Attribute::get(fn (string $value): Fluent => new Fluent(json_decode($value, true))); + } + + /** + * Merge the given settings with the existing values. + */ + public function setSettingsAttribute(array $value): void + { + $this->attributes['settings'] = json_encode(array_replace($this->settings->toArray(), $value)); + } + }; + $model->setRawAttributes(['settings' => '{"theme":"light","locale":"en"}'], true); + + $this->assertSame('light', $model->settings->theme); + + $model->settings = ['theme' => 'dark']; + + $this->assertSame(['theme' => 'dark', 'locale' => 'en'], $model->settings->toArray()); + $this->assertSame(['settings' => '{"theme":"dark","locale":"en"}'], $model->getDirty()); + } + + #[TestWith([false])] + #[TestWith([true])] + public function testMutatorsReplaceCachedClassCastValues(bool $legacy): void + { + if ($legacy) { + $model = new class extends Model { + protected array $casts = ['settings' => AsArrayObject::class]; + + /** + * Set the settings attribute. + */ + public function setSettingsAttribute(array $value): void + { + $this->attributes['settings'] = json_encode($value); + } + }; + } else { + $model = new class extends Model { + protected array $casts = ['settings' => AsArrayObject::class]; + + /** + * Set the settings attribute. + */ + protected function settings(): Attribute + { + return Attribute::set(fn (array $value): string|false => json_encode($value)); + } + }; + } + + $model->setRawAttributes(['settings' => '{"theme":"light"}'], true); + + $this->assertSame('light', $model->settings['theme']); + + $model->settings = ['theme' => 'dark']; + + $this->assertSame('dark', $model->settings['theme']); + $this->assertSame(['settings' => '{"theme":"dark"}'], $model->getDirty()); + } + + public function testSetOnlyMutatorDoesNotOverwriteChangesToClassCastObjects(): void + { + $model = new class extends Model { + protected array $casts = ['settings' => AsArrayObject::class]; + + /** + * Set the settings attribute. + */ + protected function settings(): Attribute + { + return Attribute::set(fn (ArrayObject $value): string|false => json_encode($value)); + } + }; + $model->setRawAttributes(['settings' => '{"theme":"light"}'], true); + + $model->settings = new ArrayObject(['theme' => 'dark']); + $model->settings['locale'] = 'fr'; + + $this->assertSame(['settings' => '{"theme":"dark","locale":"fr"}'], $model->getDirty()); + } + public function testUnguardAllowsAnythingToBeSet(): void { $model = new ModelStub; @@ -5425,6 +5631,17 @@ protected function serializeDate(DateTimeInterface $date): string } } +class MergingSettingsCast implements CastsInboundAttributes +{ + /** + * Merge incoming settings with the model's current settings. + */ + public function set(Model $model, string $key, mixed $value, array $attributes): string|false + { + return json_encode(array_replace($model->{$key}->toArray(), $value)); + } +} + class EnumCastingStub extends Model { protected array $casts = ['enumAttribute' => StringStatus::class]; diff --git a/tests/Integration/Database/EloquentModelEncryptedCastingTest.php b/tests/Integration/Database/EloquentModelEncryptedCastingTest.php index e50b719ddd..e04353ef56 100644 --- a/tests/Integration/Database/EloquentModelEncryptedCastingTest.php +++ b/tests/Integration/Database/EloquentModelEncryptedCastingTest.php @@ -18,6 +18,7 @@ use Hypervel\Support\Facades\Schema; use Hypervel\Support\Fluent; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\TestWith; use stdClass; class EloquentModelEncryptedCastingTest extends DatabaseTestCase @@ -344,6 +345,66 @@ public function testAsEncryptedArrayObject(): void $this->assertNull($subject->fresh()->secret_array); } + public function testAsEncryptedArrayObjectJsonAttributeIsCastable(): void + { + $this->encrypter->allows('encryptString') + ->with('{"key1":"value1"}') + ->andReturn('encrypted-secret-array-string-1'); + $this->encrypter->allows('decryptString') + ->with('encrypted-secret-array-string-1') + ->andReturn('{"key1":"value1"}'); + $this->encrypter->allows('encryptString') + ->with('{"key1":"value1","key2":"value2"}') + ->andReturn('encrypted-secret-array-string-2'); + $this->encrypter->allows('decryptString') + ->with('encrypted-secret-array-string-2') + ->andReturn('{"key1":"value1","key2":"value2"}'); + + $subject = new EncryptedCast; + + $subject->mergeCasts(['secret_array' => AsEncryptedArrayObject::class]); + + $subject->secret_array = ['key1' => 'value1']; + $subject->fill(['secret_array->key2' => 'value2']); + $subject->save(); + + $this->assertSame(['key1' => 'value1', 'key2' => 'value2'], $subject->secret_array->getArrayCopy()); + $this->assertDatabaseHas('encrypted_casts', [ + 'id' => $subject->id, + 'secret_array' => 'encrypted-secret-array-string-2', + ]); + } + + public function testAsEncryptedCollectionJsonAttributeIsCastable(): void + { + $this->encrypter->allows('encryptString') + ->with('{"key1":"value1"}') + ->andReturn('encrypted-secret-collection-string-1'); + $this->encrypter->allows('decryptString') + ->with('encrypted-secret-collection-string-1') + ->andReturn('{"key1":"value1"}'); + $this->encrypter->allows('encryptString') + ->with('{"key1":"value1","key2":"value2"}') + ->andReturn('encrypted-secret-collection-string-2'); + $this->encrypter->allows('decryptString') + ->with('encrypted-secret-collection-string-2') + ->andReturn('{"key1":"value1","key2":"value2"}'); + + $subject = new EncryptedCast; + + $subject->mergeCasts(['secret_collection' => AsEncryptedCollection::class]); + + $subject->secret_collection = new Collection(['key1' => 'value1']); + $subject->fill(['secret_collection->key2' => 'value2']); + $subject->save(); + + $this->assertSame(['key1' => 'value1', 'key2' => 'value2'], $subject->secret_collection->all()); + $this->assertDatabaseHas('encrypted_casts', [ + 'id' => $subject->id, + 'secret_collection' => 'encrypted-secret-collection-string-2', + ]); + } + /** * @param class-string $model */ @@ -465,6 +526,33 @@ public static function encryptedJsonAttributes(): array ]; } + #[TestWith([false])] + #[TestWith([true])] + public function testEncryptedClassCastDirtyCheckingUsesItsOwnEncrypter(bool $rotatingKey): void + { + $encrypter = new RealEncrypter(str_repeat('a', 16)); + $encrypter->previousKeys($rotatingKey ? [str_repeat('c', 16)] : []); + Crypt::swap($encrypter); + Model::encryptUsing(new RealEncrypter(str_repeat('b', 16))); + + $subject = new EncryptedCast; + $subject->mergeCasts(['secret_array' => AsEncryptedArrayObject::class]); + $subject->secret_array = ['key1' => 'value1']; + $subject->save(); + + $subject->secret_array = ['key1' => 'value1']; + + $this->assertSame($rotatingKey, $subject->isDirty('secret_array')); + + $subject->fill(['secret_array->key2' => 'value2']); + $subject->save(); + + $this->assertSame( + '{"key1":"value1","key2":"value2"}', + $encrypter->decryptString($subject->fresh()->getRawOriginal('secret_array')), + ); + } + public function testCustomEncrypterCanBeSpecified(): void { $customEncrypter = $this->mock(Encrypter::class); From d50c1ab9a80b2a406891ae55e9e510a737b109ac Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:59:57 +0000 Subject: [PATCH 3/7] Test cached routes served from a subdirectory Cover root and nested route matching when the application is served below a URL prefix, including repeated trailing slashes. Preserve all request server metadata and assertions from laravel/framework#61794 (916931af14). The compiled matcher already trims the request path without rewriting REQUEST_URI, so no source change is needed. These cases protect base-URL detection and the existing route-cache behavior. Validation: the complete CompiledRouteCollectionTest passes, along with formatting and framework static analysis. --- .../Routing/CompiledRouteCollectionTest.php | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/Integration/Routing/CompiledRouteCollectionTest.php b/tests/Integration/Routing/CompiledRouteCollectionTest.php index da0e59e61e..e7f8d9313f 100644 --- a/tests/Integration/Routing/CompiledRouteCollectionTest.php +++ b/tests/Integration/Routing/CompiledRouteCollectionTest.php @@ -807,6 +807,53 @@ public function testTrailingSlashIsTrimmedWhenMatchingCachedRoutes(): void $this->assertSame('foo', $this->collection()->match($request)->getName()); } + public function testMatchingRootUriWhenServedFromSubdirectory(): void + { + $this->routeCollection->add( + $this->newRoute('GET', '/', ['uses' => 'FooController@index', 'as' => 'foo']) + ); + + $request = Request::create('http://example.com/app/', 'GET', [], [], [], [ + 'SCRIPT_NAME' => '/app/index.php', + 'PHP_SELF' => '/app/index.php', + 'SCRIPT_FILENAME' => '/var/www/public/index.php', + ]); + + $this->assertSame('foo', $this->collection()->match($request)->getName()); + } + + public function testMatchingRootUriWithRepeatedSlashes(): void + { + $this->routeCollection->add( + $this->newRoute('GET', '/', ['uses' => 'FooController@index', 'as' => 'foo']) + ); + + $this->assertSame('foo', $this->collection()->match(Request::create('http://example.com//'))->getName()); + + $request = Request::create('http://example.com/app//', 'GET', [], [], [], [ + 'SCRIPT_NAME' => '/app/index.php', + 'PHP_SELF' => '/app/index.php', + 'SCRIPT_FILENAME' => '/var/www/public/index.php', + ]); + + $this->assertSame('foo', $this->collection()->match($request)->getName()); + } + + public function testMatchingUriWhenServedFromSubdirectory(): void + { + $this->routeCollection->add( + $this->newRoute('GET', 'foo/bar', ['uses' => 'FooController@index', 'as' => 'foo']) + ); + + $request = Request::create('http://example.com/app/foo/bar/', 'GET', [], [], [], [ + 'SCRIPT_NAME' => '/app/index.php', + 'PHP_SELF' => '/app/index.php', + 'SCRIPT_FILENAME' => '/var/www/public/index.php', + ]); + + $this->assertSame('foo', $this->collection()->match($request)->getName()); + } + public function testTrailingSlashWithQueryStringIsTrimmedWhenMatchingCachedRoutes(): void { $this->routeCollection->add( From b3727a9b3beaf1367d1c181d1da6f68935d14158 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 29 Oct 2026 08:00:27 +0000 Subject: [PATCH 4/7] Preserve selected rows and bindings during keyset pagination Integrate laravel/framework#61837, #61838, #61841 and #62131 from revisions 2c3294632c, 71cf667d43, a76d678087 and 6d0f491630. Reject invalid row-value operators, quote literal cursor columns correctly, retain union select and join bindings, and remove union page bounds from grouped pagination counts. Correct related pagination defects at the query-builder boundary. Each union member resolves its own selected columns and expressions, nested unions receive the page conditions, and bindings are rebuilt in SQL order. Bounded selects and union groups keep their original row selection while paging filters the resulting rows. Reverse navigation changes the page ordering rather than the ordering that chooses bounded rows. Counts retain that ordering only when it affects membership. Track the conditions and bindings owned by paging until scopes and query callbacks finish. This preserves normal Eloquent scope removal, eager loading, model cursor casts, direct page-builder identity, later ordering and limit changes, and caller-updated bindings. Clone shared union members and retire consumed markers so reusing a builder cannot apply old page filters again. Ordinary queries avoid normalized condition copies and unrelated nested-group clones. Complete embedded callbacks before validating statement options or capturing bindings. Make property resets respect nullable and non-nullable types, and document the custom-driver hooks for embedding, row selection and aggregate ordering. Source comments explain the reasons for these differences. Port all applicable upstream assertions and add regressions for compound aliases, nested and bounded unions, forward and backward iteration, quoted output aliases, current bindings, builder reuse and custom-driver isolation. Validation: formatting and full source, type-fixture and skeleton analysis pass; database unit tests and affected SQLite integration tests pass, as do cursor, binary and query-builder tests on isolated PostgreSQL, MySQL and MariaDB. The dependent custom-driver implementation and its analytical paging tests were also verified against this branch. --- src/database/src/Concerns/BuildsQueries.php | 86 +-- src/database/src/Eloquent/Builder.php | 8 +- src/database/src/Query/Builder.php | 514 +++++++++++++++++- src/docs/database.md | 4 +- .../DatabaseQueryBuilderEmbeddingTest.php | 6 +- .../DatabaseQueryBuilderPaginationTest.php | 86 +++ tests/Database/DatabaseQueryBuilderTest.php | 260 ++++++++- .../Database/EloquentCursorPaginateTest.php | 115 ++++ .../Integration/Database/QueryBuilderTest.php | 9 + 9 files changed, 984 insertions(+), 104 deletions(-) diff --git a/src/database/src/Concerns/BuildsQueries.php b/src/database/src/Concerns/BuildsQueries.php index 9a052ab9da..cdeb5e8f71 100644 --- a/src/database/src/Concerns/BuildsQueries.php +++ b/src/database/src/Concerns/BuildsQueries.php @@ -10,7 +10,6 @@ use Hypervel\Database\Eloquent\Model; use Hypervel\Database\MultipleRecordsFoundException; use Hypervel\Database\Query\Builder as QueryBuilder; -use Hypervel\Database\Query\Expression; use Hypervel\Database\RecordNotFoundException; use Hypervel\Database\RecordsNotFoundException; use Hypervel\Pagination\Cursor; @@ -506,81 +505,26 @@ protected function paginateUsingCursor( : CursorPaginator::resolveCurrentCursor($cursorName, $cursor); } - $orders = $this->ensureOrderForCursorPagination(! is_null($cursor) && $cursor->pointsToPreviousItems()); + $query = $this instanceof Builder ? $this->getQuery() : $this; - if (! is_null($cursor)) { - // Reset the union bindings so we can add the cursor where in the correct position... - $this->setBindings([], 'union'); - - $addCursorConditions = function (QueryBuilder|Builder $builder, $previousColumn, $originalColumn, $i) use (&$addCursorConditions, $cursor, $orders) { - // Unions are base queries; the other builder family only appears in nested constraints without unions. - $unionBuilders = $builder instanceof self ? $builder->getUnionBuilders() : new Collection; - - if (! is_null($previousColumn)) { - $originalColumn ??= $this->getOriginalColumnNameForCursorPagination($this, $previousColumn); - - $builder->where( - Str::contains($originalColumn, ['(', ')']) ? new Expression($originalColumn) : $originalColumn, - '=', - $this->getCursorParameterBinding($cursor, $previousColumn) - ); - - $unionBuilders->each(function ($unionBuilder) use ($previousColumn, $cursor) { - $unionBuilder->where( - $this->getOriginalColumnNameForCursorPagination($unionBuilder, $previousColumn), - '=', - $this->getCursorParameterBinding($cursor, $previousColumn) - ); - - $this->addBinding($unionBuilder->getRawBindings()['where'], 'union'); - }); - } + if ($cursor !== null) { + $query->prepareCursorOrdering(); + } - $builder->where(function (QueryBuilder|Builder $secondBuilder) use ($addCursorConditions, $cursor, $orders, $i, $unionBuilders) { - ['column' => $column, 'direction' => $direction] = $orders[$i]; - - $originalColumn = $this->getOriginalColumnNameForCursorPagination($this, $column); - - $secondBuilder->where( - Str::contains($originalColumn, ['(', ')']) ? new Expression($originalColumn) : $originalColumn, - $direction === 'asc' ? '>' : '<', - $this->getCursorParameterBinding($cursor, $column) - ); - - if ($i < $orders->count() - 1) { - $secondBuilder->orWhere(function (QueryBuilder|Builder $thirdBuilder) use ($addCursorConditions, $column, $originalColumn, $i) { - $addCursorConditions($thirdBuilder, $column, $originalColumn, $i + 1); - }); - } - - $unionBuilders->each(function ($unionBuilder) use ($column, $direction, $cursor, $i, $orders, $addCursorConditions) { - $unionWheres = $unionBuilder->getRawBindings()['where']; - - $originalColumn = $this->getOriginalColumnNameForCursorPagination($unionBuilder, $column); - $unionBuilder->where(function ($unionBuilder) use ($column, $direction, $cursor, $i, $orders, $addCursorConditions, $originalColumn, $unionWheres) { - $unionBuilder->where( - $originalColumn, - $direction === 'asc' ? '>' : '<', - $this->getCursorParameterBinding($cursor, $column) - ); - - if ($i < $orders->count() - 1) { - $unionBuilder->orWhere(function (QueryBuilder|Builder $fourthBuilder) use ($addCursorConditions, $column, $originalColumn, $i) { - $addCursorConditions($fourthBuilder, $column, $originalColumn, $i + 1); - }); - } - - $this->addBinding($unionWheres, 'union'); - $this->addBinding($unionBuilder->getRawBindings()['where'], 'union'); - }); - }); - }); - }; + $orders = $this->ensureOrderForCursorPagination(! is_null($cursor) && $cursor->pointsToPreviousItems()); - $addCursorConditions($this, null, null, 0); + if (! is_null($cursor)) { + // Resolve model casts before a bounded query is replaced by its derived table. + $bindings = $orders->mapWithKeys(fn (array $order): array => [ + $order['column'] => $this->getCursorParameterBinding($cursor, $order['column']), + ]); + + // The base builder owns placement after scopes and callbacks; applying scopes + // here would freeze them before callers can remove or change them. + $query->addCursorPaginationConditions($orders->all(), $bindings->all()); } - $this->limit($perPage + 1); + $query->limit($perPage + 1); return $this->cursorPaginator($this->get($columns), $perPage, $cursor, [ 'path' => Paginator::resolveCurrentPath(), diff --git a/src/database/src/Eloquent/Builder.php b/src/database/src/Eloquent/Builder.php index 52821cc823..d847a5d920 100644 --- a/src/database/src/Eloquent/Builder.php +++ b/src/database/src/Eloquent/Builder.php @@ -1175,8 +1175,12 @@ protected function ensureOrderForCursorPagination(bool $shouldReverse = false): }; if ($shouldReverse) { - $this->query->orders = (new BaseCollection($this->query->orders))->map($reverseDirection)->toArray(); - $this->query->unionOrders = (new BaseCollection($this->query->unionOrders))->map($reverseDirection)->toArray(); + if (! empty($this->query->unionOrders)) { + // Keep the first SELECT's limit ordering; reverse only the union's page ordering. + $this->query->unionOrders = (new BaseCollection($this->query->unionOrders))->map($reverseDirection)->toArray(); + } else { + $this->query->orders = (new BaseCollection($this->query->orders))->map($reverseDirection)->toArray(); + } } $orders = ! empty($this->query->unionOrders) ? $this->query->unionOrders : $this->query->orders; diff --git a/src/database/src/Query/Builder.php b/src/database/src/Query/Builder.php index ba6973fe99..a67622ed1d 100644 --- a/src/database/src/Query/Builder.php +++ b/src/database/src/Query/Builder.php @@ -32,6 +32,7 @@ use Hypervel\Support\Arr; use Hypervel\Support\Collection; use Hypervel\Support\LazyCollection; +use Hypervel\Support\Str; use Hypervel\Support\StrCache; use Hypervel\Support\Stringable; use Hypervel\Support\Traits\ForwardsCalls; @@ -40,6 +41,7 @@ use JsonException; use LogicException; use PDO; +use ReflectionProperty; use RuntimeException; use SortDirection; use stdClass; @@ -206,6 +208,18 @@ class Builder implements BuilderContract */ public array $beforeQueryCallbacks = []; + /** + * Indicates whether paging conditions need preparation after callbacks and scopes. + */ + protected bool $hasKeysetConditions = false; + + /** + * The SELECT ordering before paging changed the traversal direction. + * + * @var null|array{orders: array, bindings: array, retainedBindings: int, pageBindingCount: int} + */ + protected ?array $keysetOrdering = null; + /** * The callbacks that should be invoked after retrieving data from the database. * @@ -395,7 +409,7 @@ protected function parseSub(mixed $query): array } if ($query instanceof self) { - $this->ensureCanEmbedQuery($query); + $this->prepareQueryForEmbedding($query); $query = $this->prependDatabaseNameIfCrossDatabaseQuery($query); @@ -1849,7 +1863,7 @@ protected function whereSub(ExpressionContract|string $column, string $operator, $query = $callback instanceof self ? $callback : $callback->toBase(); } - $this->ensureCanEmbedQuery($query); + $this->prepareQueryForEmbedding($query); $this->wheres[] = [ 'type' => $type, 'column' => $column, 'operator' => $operator, 'query' => $query, 'boolean' => $boolean, @@ -1926,7 +1940,7 @@ public function orWhereNotExists(Closure|self|EloquentBuilder $callback): static */ public function addWhereExistsQuery(self $query, string $boolean = 'and', bool $not = false): static { - $this->ensureCanEmbedQuery($query); + $this->prepareQueryForEmbedding($query); $type = $not ? 'NotExists' : 'Exists'; @@ -1938,7 +1952,7 @@ public function addWhereExistsQuery(self $query, string $boolean = 'and', bool $ } /** - * Adds a where condition using row values. + * Add a where condition using row values. * * @throws InvalidArgumentException */ @@ -1948,6 +1962,10 @@ public function whereRowValues(array $columns, string $operator, array $values, throw new InvalidArgumentException('The number of columns must match the number of values'); } + if ($this->invalidOperator($operator)) { + throw new InvalidArgumentException('Invalid operator passed to whereRowValues method.'); + } + $type = 'RowValues'; $this->wheres[] = ['type' => $type, 'columns' => $columns, 'operator' => $operator, 'values' => $values, 'boolean' => $boolean]; @@ -2757,7 +2775,16 @@ public function forPageBeforeId(int $perPage = 15, mixed $lastId = 0, string $co $lastId = BinaryParameter::fromStream($lastId); } - $this->orders = $this->removeExistingOrdersFor($column); + $this->rememberKeysetOrdering(); + $bindingOffset = count($this->bindings['where']); + $orderColumn = $this->unions ? $this->getKeysetOutputColumn($column) : $column; + + if ($this->unions) { + $this->unionOrders = $this->removeOrdersFor($this->unionOrders ?? [], $this->bindings['unionOrder'], $orderColumn); + } else { + $this->orders = $this->removeExistingOrdersFor($column); + $this->keysetOrdering['retainedBindings'] = count($this->bindings['order']); + } if (is_null($lastId)) { $this->whereNotNull($column); @@ -2765,8 +2792,14 @@ public function forPageBeforeId(int $perPage = 15, mixed $lastId = 0, string $co $this->where($column, '<', $lastId); } - return $this->orderBy($column, SortDirection::Descending) - ->limit($perPage); + $this->markKeysetCondition($bindingOffset, $column); + $this->orderBy($orderColumn, SortDirection::Descending)->limit($perPage); + + if (! $this->unions) { + $this->orders[array_key_last($this->orders)]['keyset'] = true; + } + + return $this; } /** @@ -2780,7 +2813,16 @@ public function forPageAfterId(int $perPage = 15, mixed $lastId = 0, string $col $lastId = BinaryParameter::fromStream($lastId); } - $this->orders = $this->removeExistingOrdersFor($column); + $this->rememberKeysetOrdering(); + $bindingOffset = count($this->bindings['where']); + $orderColumn = $this->unions ? $this->getKeysetOutputColumn($column) : $column; + + if ($this->unions) { + $this->unionOrders = $this->removeOrdersFor($this->unionOrders ?? [], $this->bindings['unionOrder'], $orderColumn); + } else { + $this->orders = $this->removeExistingOrdersFor($column); + $this->keysetOrdering['retainedBindings'] = count($this->bindings['order']); + } if (is_null($lastId)) { $this->whereNotNull($column); @@ -2788,8 +2830,14 @@ public function forPageAfterId(int $perPage = 15, mixed $lastId = 0, string $col $this->where($column, '>', $lastId); } - return $this->orderBy($column, SortDirection::Ascending) - ->limit($perPage); + $this->markKeysetCondition($bindingOffset, $column); + $this->orderBy($orderColumn, SortDirection::Ascending)->limit($perPage); + + if (! $this->unions) { + $this->orders[array_key_last($this->orders)]['keyset'] = true; + } + + return $this; } /** @@ -2799,13 +2847,18 @@ public function forPageAfterId(int $perPage = 15, mixed $lastId = 0, string $col */ public function reorder(Closure|self|EloquentBuilder|Relation|ExpressionContract|string|null $column = null, SortDirection|string $direction = SortDirection::Ascending): static { + $this->keysetOrdering = null; $this->orders = null; $this->unionOrders = null; $this->bindings['order'] = []; $this->bindings['unionOrder'] = []; if ($column) { - return $this->orderBy($column, $direction); + $this->orderBy($column, $direction); + } + + if ($this->hasKeysetConditions) { + $this->prepareCursorOrdering(); } return $this; @@ -2824,7 +2877,15 @@ public function reorderDesc(Closure|self|EloquentBuilder|Relation|ExpressionCont */ protected function removeExistingOrdersFor(string $column): array { - return (new Collection($this->orders)) + return $this->removeOrdersFor($this->orders ?? [], $this->bindings['order'], $column); + } + + /** + * Remove matching orders from the ordering and bindings owned by a SELECT or union. + */ + protected function removeOrdersFor(array $orders, array &$bindings, string $column): array + { + return (new Collection($orders)) ->reject(fn ($order) => isset($order['column']) && $order['column'] === $column) ->values() ->all(); @@ -2847,7 +2908,7 @@ public function union(Closure|self|EloquentBuilder $query, bool $all = false): s $query = $query->toBase(); } - $this->ensureCanEmbedQuery($query); + $this->prepareQueryForEmbedding($query); $this->unions[] = ['query' => $query, 'all' => $all]; @@ -2931,6 +2992,12 @@ public function applyBeforeQueryCallbacks(): void } $this->beforeQueryCallbacks = []; + + // Scopes and callbacks may add row-limiting clauses. Place paging predicates only + // after those clauses exist, without changing Eloquent's scope lifecycle. + if ($this->hasKeysetConditions) { + $this->completeKeysetPagination(); + } } /** @@ -3225,8 +3292,12 @@ protected function ensureOrderForCursorPagination(bool $shouldReverse = false): }; if ($shouldReverse) { - $this->orders = (new Collection($this->orders))->map($reverseDirection)->toArray(); - $this->unionOrders = (new Collection($this->unionOrders))->map($reverseDirection)->toArray(); + if (! empty($this->unionOrders)) { + // Keep the first SELECT's limit ordering; reverse only the union's page ordering. + $this->unionOrders = (new Collection($this->unionOrders))->map($reverseDirection)->toArray(); + } else { + $this->orders = (new Collection($this->orders))->map($reverseDirection)->toArray(); + } } $orders = ! empty($this->unionOrders) ? $this->unionOrders : $this->orders; @@ -3236,6 +3307,369 @@ protected function ensureOrderForCursorPagination(bool $shouldReverse = false): ->values(); } + /** + * Preserve the SELECT's ordering before paging changes its traversal order. + */ + protected function rememberKeysetOrdering(): void + { + if ($this->unions) { + return; + } + + if ($this->keysetOrdering === null) { + $this->keysetOrdering = [ + 'orders' => $this->orders ?? [], + 'bindings' => $this->bindings['order'], + 'retainedBindings' => count($this->bindings['order']), + 'pageBindingCount' => 0, + ]; + } else { + foreach ($this->orders ?? [] as $order) { + if (! isset($order['keyset']) && ! isset($order['keysetOriginal'])) { + $this->keysetOrdering['orders'][] = $order; + } + } + + array_push($this->keysetOrdering['bindings'], ...array_slice( + $this->bindings['order'], + $this->keysetOrdering['retainedBindings'] + )); + $this->keysetOrdering['retainedBindings'] = count($this->bindings['order']); + } + + foreach ($this->orders ?? [] as $index => $order) { + if (! isset($order['keyset'])) { + $this->orders[$index]['keysetOriginal'] = true; + } + } + } + + /** + * Retain the SELECT ordering for cursor pagination's traversal direction. + */ + public function prepareCursorOrdering(): void + { + $this->rememberKeysetOrdering(); + + if (! $this->unions) { + $this->keysetOrdering['pageBindingCount'] = count($this->bindings['order']); + foreach ($this->orders ?? [] as $index => $order) { + $this->orders[$index]['keyset'] = true; + } + } + } + + /** + * Record the paging predicate and the bindings it owns. + */ + protected function markKeysetCondition(int $offset, ?string $column = null): void + { + $index = array_key_last($this->wheres); + $this->wheres[$index]['keyset'] = [ + 'offset' => $offset, + 'count' => count($this->bindings['where']) - $offset, + ]; + + if ($column !== null) { + $this->wheres[$index]['keysetColumn'] = $column; + } + + $this->hasKeysetConditions = true; + } + + /** + * Add cursor pagination's internal predicates with their owned columns and bindings. + */ + public function addCursorPaginationConditions(array $orders, array $bindings): static + { + $offset = count($this->bindings['where']); + $this->addCursorComparisons($this, $orders, $bindings, 0); + $this->markKeysetCondition($offset); + + if (! $this->unions) { + foreach ($this->orders ?? [] as $index => $order) { + $this->orders[$index]['keyset'] = true; + } + } + + return $this; + } + + /** + * Add the ordered comparisons for a compound cursor. + */ + protected function addCursorComparisons(self $owner, array $orders, array $bindings, int $index): void + { + ['column' => $parameter, 'direction' => $direction] = $orders[$index]; + $column = $this->getOriginalColumnNameForCursorPagination($owner, $parameter); + + if ($column !== $parameter && Str::contains($column, ['(', ')'])) { + $column = new Expression($column); + } + + $this->where(function (self $query) use ($owner, $orders, $bindings, $index, $parameter, $direction, $column): void { + $query->where($column, $direction === 'asc' ? '>' : '<', $bindings[$parameter]); + $query->wheres[array_key_last($query->wheres)]['keysetColumn'] = $parameter; + + if ($index < count($orders) - 1) { + $query->orWhere(function (self $query) use ($owner, $orders, $bindings, $index, $parameter, $column): void { + $query->where($column, '=', $bindings[$parameter]); + $query->wheres[array_key_last($query->wheres)]['keysetColumn'] = $parameter; + $query->addCursorComparisons($owner, $orders, $bindings, $index + 1); + }); + } + }); + } + + /** + * Determine whether paging must filter outside this SELECT's row limit. + */ + protected function requiresKeysetIsolation(bool $limitSelectsRows): bool + { + return $limitSelectsRows && ($this->limit !== null || $this->offset !== null); + } + + /** + * Place paging predicates outside bounded SELECTs and in every union member. + */ + protected function completeKeysetPagination(): void + { + $this->hasKeysetConditions = false; + $limitSelectsRows = (bool) $this->unions; + $isolate = $this->requiresKeysetIsolation($limitSelectsRows); + + if ($isolate) { + $this->rememberKeysetOrdering(); + } + $conditions = []; + // Ordinary pages only retire their markers; they do not need normalized + // condition copies. Retaining markers would replay them if a union is added later. + $this->wheres = $this->collectKeysetConditions($this->wheres, $conditions, $isolate, $isolate || $limitSelectsRows); + + if ($isolate) { + // Binding offsets are recorded when predicates are added, so scope grouping + // cannot make us remove a user's or a scope's bindings by mistake. + $ranges = array_column($conditions, 'range'); + usort($ranges, static fn (array $left, array $right): int => $right['offset'] <=> $left['offset']); + foreach ($ranges as $range) { + array_splice($this->bindings['where'], $range['offset'], $range['count']); + } + + $pageOrders = array_values(array_filter($this->orders ?? [], static fn (array $order): bool => isset($order['keyset']))); + foreach ($pageOrders as $index => $order) { + if (isset($order['column']) && is_string($order['column'])) { + $pageOrders[$index]['column'] = $this->getKeysetOutputColumn($order['column']); + } + } + $pageBindings = array_slice($this->bindings['order'], 0, $this->keysetOrdering['pageBindingCount'] ?? 0); + $this->isolateKeysetSelect($limitSelectsRows); + $this->appendKeysetConditions($conditions); + + if (! $limitSelectsRows) { + foreach ($pageOrders as $order) { + $this->orders[] = $order; + } + $this->bindings['order'] = $pageBindings; + } + } + + if ($this->unions) { + $this->bindings['union'] = []; + + foreach ($this->unions as $index => $union) { + // Union members are shared by shallow builder clones. Paging one clone + // must not add its cursor to another query built from the same member. + $member = clone $union['query']; + $member->applyBeforeQueryCallbacks(); + $member = $member->applyKeysetConditions($conditions); + $this->unions[$index]['query'] = $member; + $this->addBinding($member->getBindings(), 'union'); + } + } + + $this->keysetOrdering = null; + foreach ($this->orders ?? [] as $index => $order) { + unset($this->orders[$index]['keyset'], $this->orders[$index]['keysetOriginal']); + } + } + + /** + * Collect owned predicates without changing nested groups shared with another clone. + */ + protected function collectKeysetConditions(array $wheres, array &$conditions, bool $remove, bool $collect = true): array + { + foreach ($wheres as $index => $where) { + if (isset($where['keyset'])) { + $range = $where['keyset']; + unset($where['keyset']); + if ($collect) { + $conditions[] = [ + 'where' => $this->normalizeKeysetColumns($where), + 'bindings' => array_slice($this->bindings['where'], $range['offset'], $range['count']), + 'range' => $range, + ]; + } + + if ($remove) { + unset($wheres[$index]); + } else { + $wheres[$index] = $where; + } + } elseif ($where['type'] === 'Nested') { + $nestedWheres = $this->collectKeysetConditions($where['query']->wheres, $conditions, $remove, $collect); + if ($remove && $nestedWheres === []) { + unset($wheres[$index]); + } elseif ($nestedWheres !== $where['query']->wheres) { + // Clear consumed markers without changing a group shared with another + // clone. Otherwise a later page call would replay old union predicates. + $where['query'] = clone $where['query']; + $where['query']->wheres = $nestedWheres; + $wheres[$index] = $where; + } + } + } + + return $remove ? array_values($wheres) : $wheres; + } + + /** + * Resolve paging columns to the output names shared by the union's SELECTs. + */ + protected function normalizeKeysetColumns(array $where): array + { + if (isset($where['keysetColumn'])) { + $where['keysetColumn'] = $this->getKeysetOutputColumn($where['keysetColumn']); + } + + if ($where['type'] === 'Nested') { + $where['query'] = clone $where['query']; + $where['query']->wheres = array_map($this->normalizeKeysetColumns(...), $where['query']->wheres); + } + + return $where; + } + + /** + * Get the selected output name for a paging column. + */ + protected function getKeysetOutputColumn(string $parameter): string + { + foreach ($this->getColumns() as $column) { + if (($position = strripos($column, ' as ')) !== false) { + $original = substr($column, 0, $position); + $alias = substr($column, $position + 4); + $quotes = $this->grammar->wrap(''); + if (str_starts_with($alias, $quotes[0]) && str_ends_with($alias, $quotes[-1])) { + $alias = str_replace($quotes[-1] . $quotes[-1], $quotes[-1], substr($alias, 1, -1)); + } + if ($parameter === $alias || $parameter === $original) { + return $alias; + } + } + } + + return Str::afterLast($parameter, '.'); + } + + /** + * Apply inherited paging predicates to this member and its nested unions. + */ + protected function applyKeysetConditions(array $conditions): static + { + if ($this->unions && ($this->unionLimit !== null || $this->unionOffset !== null)) { + return $this->newQuery()->fromSub($this, 'cursor_table')->appendKeysetConditions($conditions); + } + + if ($this->requiresKeysetIsolation(true)) { + $this->isolateKeysetSelect(true); + } + + $this->appendKeysetConditions($conditions); + + if ($this->unions) { + $this->bindings['union'] = []; + foreach ($this->unions as $index => $union) { + $member = clone $union['query']; + $member->applyBeforeQueryCallbacks(); + $member = $member->applyKeysetConditions($conditions); + $this->unions[$index]['query'] = $member; + $this->addBinding($member->getBindings(), 'union'); + } + } + + return $this; + } + + /** + * Append paging predicates using this SELECT's own source columns and expressions. + */ + protected function appendKeysetConditions(array $conditions): static + { + foreach ($conditions as $condition) { + $this->wheres[] = $this->rebindKeysetColumns($condition['where']); + $this->addBinding($condition['bindings'], 'where'); + } + + return $this; + } + + /** + * Resolve output names against this SELECT without rewriting user predicates. + */ + protected function rebindKeysetColumns(array $where): array + { + if (isset($where['keysetColumn'])) { + $parameter = $where['keysetColumn']; + $column = $this->getOriginalColumnNameForCursorPagination($this, $parameter); + $where['column'] = $column !== $parameter && Str::contains($column, ['(', ')']) + ? new Expression($column) + : $column; + unset($where['keysetColumn']); + } + + if ($where['type'] === 'Nested') { + $where['query'] = clone $where['query']; + $where['query']->wheres = array_map($this->rebindKeysetColumns(...), $where['query']->wheres); + } + + return $where; + } + + /** + * Move the completed SELECT into a derived table while retaining the page's clauses. + */ + protected function isolateKeysetSelect(bool $limitSelectsRows): void + { + $aggregate = $this->aggregate; + $inner = $this->cloneWithout(['aggregate', 'unions', 'unionOrders', 'unionLimit', 'unionOffset', 'timeout']) + ->cloneWithoutBindings(['union', 'unionOrder']); + $inner->columns ??= ['*']; + $limit = $this->limit; + $offset = $this->offset; + + if (! $limitSelectsRows) { + $inner->limit = $inner->offset = null; + $inner->orders = $this->keysetOrdering['orders'] ?? $inner->orders; + $inner->bindings['order'] = $this->keysetOrdering['bindings'] ?? $inner->bindings['order']; + } + + $this->resetProperties([ + 'aggregate', 'columns', 'distinct', 'from', 'indexHint', 'joins', 'wheres', 'groups', 'havings', + 'orders', 'limit', 'groupLimit', 'offset', 'lock', + ]); + foreach (['select', 'from', 'join', 'where', 'groupBy', 'having', 'order'] as $binding) { + $this->bindings[$binding] = []; + } + $this->columns = ['*']; + $this->aggregate = $aggregate; + $this->fromSub($inner, 'cursor_table'); + + if (! $limitSelectsRows) { + $this->limit = $limit; + $this->offset = $offset; + } + } + /** * Get the count of the total records for the paginator. * @@ -3312,8 +3746,12 @@ protected function runPaginationCountQuery(array $columns = ['*']): array */ protected function cloneForPaginationCount(): static { - return $this->cloneWithout(['orders', 'limit', 'offset']) - ->cloneWithoutBindings(['order']); + // A union's page bounds do not belong to its first SELECT. Removing that + // SELECT's own bounds would change the rows whose total we are counting. + return $this->cloneWithout($this->unions + ? ['unionOrders', 'unionLimit', 'unionOffset'] + : ['orders', 'limit', 'offset']) + ->cloneWithoutBindings([$this->unions ? 'unionOrder' : 'order']); } /** @@ -3661,7 +4099,7 @@ protected function setAggregate(string $function, array $columns): static { $this->aggregate = ['function' => $function, 'columns' => $columns]; - if (empty($this->groups)) { + if (! $this->shouldPreserveAggregateOrdering()) { $this->orders = null; $this->bindings['order'] = []; @@ -3670,6 +4108,16 @@ protected function setAggregate(string $function, array $columns): static return $this; } + /** + * Determine whether ordering selects rows retained by the aggregate's inner query. + */ + protected function shouldPreserveAggregateOrdering(): bool + { + // An unbounded SELECT needs no sort for a count; a bounded union member + // needs its ordering to retain the same rows, particularly with UNION DISTINCT. + return (bool) $this->groups || ($this->unions && ($this->limit !== null || $this->offset !== null)); + } + /** * Execute the given callback while selecting the given columns. * @@ -4381,12 +4829,15 @@ protected function isQueryable(mixed $value): bool } /** - * Ensure the query can be embedded in another statement. + * Prepare the query for embedding in another statement. * * @throws InvalidArgumentException */ - protected function ensureCanEmbedQuery(self $query): void + protected function prepareQueryForEmbedding(self $query): void { + // Embedded SQL and bindings must reflect the same completed query callbacks. + $query->applyBeforeQueryCallbacks(); + if ($query->timeout !== null) { throw new InvalidArgumentException( 'An embedded query cannot define its own timeout. Apply the timeout to the outer query instead.' @@ -4407,11 +4858,24 @@ public function clone(): static */ public function cloneWithout(array $properties): static { - return tap($this->clone(), function ($clone) use ($properties) { - foreach ($properties as $property) { - $clone->{$property} = is_array($clone->{$property}) ? [] : null; - } - }); + return tap($this->clone(), static fn (self $clone) => $clone->resetProperties($properties)); + } + + /** + * Clear query properties while retaining defaults required by non-nullable types. + */ + protected function resetProperties(array $properties): void + { + foreach ($properties as $property) { + $reflection = new ReflectionProperty($this, $property); + $this->{$property} = ($reflection->getType()?->allowsNull() ?? true) + ? null + : $reflection->getDefaultValue(); + } + + if (in_array('wheres', $properties, true)) { + $this->hasKeysetConditions = false; + } } /** diff --git a/src/docs/database.md b/src/docs/database.md index 973204b0bd..7549a46857 100644 --- a/src/docs/database.md +++ b/src/docs/database.md @@ -528,7 +528,9 @@ Both execution methods construct database errors through the protected `newQuery Custom drivers used with database cache locks must identify unique-constraint violations through `isUniqueConstraintError` so lock contention can be handled without hiding other database failures. -Query builders with statement-level options may override the protected `Query\Builder::ensureCanEmbedQuery` method to reject options that belong on the outer statement. It runs when attaching a subquery, scalar or exists predicate, or union member; call the parent to preserve the built-in rejection of embedded timeouts. +Query builders with statement-level options may override the protected `Query\Builder::prepareQueryForEmbedding` method to reject options that belong on the outer statement. It runs when attaching a subquery, scalar or exists predicate, or union member; call the parent first to apply the embedded query's callbacks and preserve the built-in rejection of embedded timeouts. + +Builders with additional row-limiting SELECT clauses should override `requiresKeysetIsolation` and `isolateKeysetSelect`. The query builder prepares paging conditions after scopes and query callbacks have run, keeping them outside any SELECT whose clauses decide which rows survive. Keep custom SELECT clauses inside the derived table and statement-level options on the outer query. Cursor pagination, ID-based chunking, and direct ID-based page methods share this preparation, including when used through Eloquent. Override `shouldPreserveAggregateOrdering` when ordering determines which rows a custom clause retains for an aggregate. Custom Eloquent builders may override the public `ensureCanCreateOrFirst(): void` method to reject helpers that depend on unique-constraint recovery. Both `firstOrCreate` and `createOrFirst` call it before reading or writing, including their relationship forms; `updateOrCreate` and `incrementOrCreate` reach it through those helpers. Helpers delegate to each other, so validation may run more than once per operation; keep overrides side-effect-free. Relationships use the related model's builder, even when the parent uses another driver. The default method imposes no restriction, and ordinary `create`, `save`, and `firstOrNew` are unchanged. diff --git a/tests/Database/DatabaseQueryBuilderEmbeddingTest.php b/tests/Database/DatabaseQueryBuilderEmbeddingTest.php index c346e2355f..96e2b5f254 100644 --- a/tests/Database/DatabaseQueryBuilderEmbeddingTest.php +++ b/tests/Database/DatabaseQueryBuilderEmbeddingTest.php @@ -92,11 +92,11 @@ class EmbeddingAwareQueryBuilder extends Builder public ?InvalidArgumentException $embeddingFailure = null; /** - * Extend embedding validation while retaining the framework timeout rule. + * Extend embedding preparation while retaining callbacks and the timeout rule. */ - protected function ensureCanEmbedQuery(Builder $query): void + protected function prepareQueryForEmbedding(Builder $query): void { - parent::ensureCanEmbedQuery($query); + parent::prepareQueryForEmbedding($query); $this->validatedQueries[] = $query; diff --git a/tests/Database/DatabaseQueryBuilderPaginationTest.php b/tests/Database/DatabaseQueryBuilderPaginationTest.php index cab6c59c89..fbaedae06e 100644 --- a/tests/Database/DatabaseQueryBuilderPaginationTest.php +++ b/tests/Database/DatabaseQueryBuilderPaginationTest.php @@ -6,6 +6,9 @@ use Hypervel\Database\BinaryParameter; use Hypervel\Database\Connection; +use Hypervel\Database\ConnectionResolverInterface; +use Hypervel\Database\Eloquent\Builder as EloquentBuilder; +use Hypervel\Database\Eloquent\Model; use Hypervel\Database\Query\Builder; use Hypervel\Database\Query\Grammars\Grammar; use Hypervel\Database\Query\Grammars\MySqlGrammar; @@ -20,6 +23,89 @@ class DatabaseQueryBuilderPaginationTest extends TestCase { + #[DataProvider('keysetIterators')] + public function testKeysetIterationVisitsEveryUnionRowOnce(string $method, bool $lazy, bool $descending): void + { + $connection = new SQLiteConnection(new PDO('sqlite::memory:')); + $connection->unprepared('create table posts (id integer)'); + $connection->unprepared('create table archives (archive_id integer)'); + $connection->unprepared('insert into posts values (1), (2), (3)'); + $connection->unprepared('insert into archives values (4), (5), (6)'); + $member = $connection->table('archives')->select('archive_id as value'); + $query = $connection->table('posts')->selectRaw('id as "value"')->unionAll($member); + $actual = []; + + if ($lazy) { + $actual = $query->{$method}(2, 'id', 'value')->take(8)->pluck('value')->all(); + } else { + $this->assertTrue($query->{$method}(2, function (Collection $rows) use (&$actual): bool { + array_push($actual, ...$rows->pluck('value')->all()); + + return count($actual) < 8; + }, 'id', 'value')); + } + + $this->assertSame($descending ? [6, 5, 4, 3, 2, 1] : [1, 2, 3, 4, 5, 6], $actual); + $this->assertSame([], $member->wheres); + $this->assertSame([], $query->getBindings()); + } + + /** + * Exercise both shared iteration paths and traversal directions. + */ + public static function keysetIterators(): iterable + { + yield 'chunks ascending' => ['chunkById', false, false]; + yield 'chunks descending' => ['chunkByIdDesc', false, true]; + yield 'lazy ascending' => ['lazyById', true, false]; + yield 'lazy descending' => ['lazyByIdDesc', true, true]; + } + + public function testDirectKeysetPagesPreserveScopesLimitsAndCurrentBindings(): void + { + $connection = new SQLiteConnection(new PDO('sqlite::memory:')); + $connection->unprepared('create table posts (id integer)'); + $connection->unprepared('create table archives (id integer)'); + $connection->unprepared('insert into posts values (1), (2), (3)'); + $connection->unprepared('insert into archives values (4), (5), (6)'); + $model = new class extends Model { + protected ?string $table = 'posts'; + }; + $model::setConnectionResolver(m::mock(ConnectionResolverInterface::class, ['connection' => $connection])); + $query = (new EloquentBuilder($connection->query()))->setModel($model) + ->withGlobalScope('visible', static fn (EloquentBuilder $query): EloquentBuilder => $query->where('posts.id', '<', 3)) + ->select('id')->orderByDesc('id')->limit(2) + ->unionAll($connection->table('archives')->select('id'))->orderBy('id'); + + $scoped = (clone $query)->forPageAfterId(2, 1)->limit(1); + $this->assertSame([2], $scoped->get()->modelKeys()); + $unscoped = (clone $query)->forPageAfterId(2, 2)->withoutGlobalScope('visible')->limit(1); + $this->assertSame([3], $unscoped->get()->modelKeys()); + $this->assertSame(2, (clone $query)->withoutGlobalScopes()->forPageAfterId(2, 2)->count()); + $this->assertSame(4, (clone $query)->withoutGlobalScopes()->forPageAfterId(10, 2)->count()); + $this->assertSame(4, (clone $query)->withoutGlobalScopes()->forPageAfterId(2, 2) + ->getQuery()->groupBy('id')->getCountForPagination()); + + $base = (clone $query)->withoutGlobalScopes()->getQuery(); + $this->assertSame($base, $base->forPageAfterId(3, 1)); + $this->assertSame($base, $base->forPageBeforeId(3, 5)); + $base->setBindings([2, 5], 'where'); + $this->assertSame([4, 3], $base->get()->pluck('id')->all()); + $sql = $base->toSql(); + $bindings = $base->getBindings(); + $this->assertSame($sql, $base->toSql()); + $this->assertSame($bindings, $base->getBindings()); + + $base->forPageAfterId(2, 3); + $this->assertSame([4], $base->get()->pluck('id')->all()); + $this->assertSame([2, 5, 3], $base->getRawBindings()['union']); + + $reused = $connection->table('posts')->forPageAfterId(1, 2); + $this->assertSame([3], $reused->get()->pluck('id')->all()); + $reused->unionAll($connection->table('posts')->where('id', 1))->forPageAfterId(2, 0); + $this->assertSame([1, 3], $reused->get()->pluck('id')->all()); + } + public function testBinaryStringsCanBePagedUsingAnExplicitCursorAlias(): void { $connection = new SQLiteConnection(new PDO('sqlite::memory:')); diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index e6914ebb24..96f1cc2249 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -3296,6 +3296,35 @@ public function testGetCountForPaginationWithUnionLimitAndOffset(): void $this->assertEquals(1, $count); } + public function testGetCountForPaginationWithGroupedUnionLimitAndOffset(): void + { + $builder = $this->getBuilder(); + $builder->from('posts')->select('category')->groupBy('category')->unionAll($this->getBuilder()->from('videos')->select('category'))->limit(15)->offset(1); + + $builder->getConnection()->expects('select')->with('select count(*) as "aggregate" from ((select "category" from "posts" group by "category") union all (select "category" from "videos")) as "aggregate_table"', [], true, [])->andReturn([['aggregate' => 1]]); + $builder->getProcessor()->expects('processSelect')->andReturnUsing(function ($builder, $results) { + return $results; + }); + + $count = $builder->getCountForPagination(); + $this->assertEquals(1, $count); + } + + public function testGetCountForPaginationWithGroupedUnionOrders(): void + { + $builder = $this->getBuilder(); + $builder->from('posts')->select('category')->groupBy('category')->unionAll($this->getBuilder()->from('videos')->select('category'))->orderBy('category')->orderByRaw('field(category, ?)', ['news']); + + $builder->getConnection()->expects('select')->with('select count(*) as "aggregate" from ((select "category" from "posts" group by "category") union all (select "category" from "videos")) as "aggregate_table"', [], true, [])->andReturn([['aggregate' => 1]]); + $builder->getProcessor()->expects('processSelect')->andReturnUsing(function ($builder, $results) { + return $results; + }); + + $count = $builder->getCountForPagination(); + $this->assertEquals(1, $count); + $this->assertSame(['news'], $builder->getRawBindings()['unionOrder']); + } + public function testGroupLimitCompilationWithAndWithoutOffset(): void { $builder = $this->getSQLiteBuilder() @@ -3576,6 +3605,27 @@ public function testUnionRejectsItsOwnTimeoutBeforeEmbeddingIt(): void ); } + public function testEmbeddedQueryRejectsTimeoutAddedByCallback(): void + { + $outer = $this->getMySqlBuilder()->from('users')->where('tenant_id', 7); + $sql = $outer->toSql(); + $embedded = $this->getMySqlBuilder()->from('members')->select('id') + ->beforeQuery(fn (Builder $query) => $query->timeout(2)); + + try { + $outer->whereIn('id', $embedded); + $this->fail('Expected the embedded query timeout to be rejected.'); + } catch (InvalidArgumentException $exception) { + $this->assertSame( + 'An embedded query cannot define its own timeout. Apply the timeout to the outer query instead.', + $exception->getMessage() + ); + } + + $this->assertSame($sql, $outer->toSql()); + $this->assertSame([7], $outer->getBindings()); + } + public function testExplainRejectsQueryTimeoutWithoutMutatingTheBuilder(): void { $builder = $this->getMySqlBuilder()->from('users')->where('id', 1)->timeout(2); @@ -6168,6 +6218,41 @@ public function testPreservedAreAppliedByExists(): void $builder->exists(); } + #[DataProvider('embeddedQueryCallbacksProvider')] + public function testEmbeddedQueryCallbacksRunBeforeBindingsAreCaptured(string $kind, string $sql): void + { + $calls = 0; + $embedded = $this->getBuilder()->select('id')->from('members')->beforeQuery(function (Builder $query) use (&$calls): void { + ++$calls; + $query->where('active', 1); + }); + $outer = $this->getBuilder()->select('id')->from('users')->where('tenant_id', 7); + + match ($kind) { + 'union' => $outer->unionAll($embedded), + 'exists' => $outer->whereExists($embedded), + 'scalar' => $outer->where('id', '=', $embedded), + 'parsed' => $outer->whereIn('id', $embedded), + }; + + $this->assertSame($sql, $outer->toSql()); + $this->assertSame([7, 1], $outer->getBindings()); + $this->assertSame(1, $calls); + } + + /** + * Provide the supported query embedding forms. + */ + public static function embeddedQueryCallbacksProvider(): array + { + return [ + ['union', '(select "id" from "users" where "tenant_id" = ?) union all (select "id" from "members" where "active" = ?)'], + ['exists', 'select "id" from "users" where "tenant_id" = ? and exists (select "id" from "members" where "active" = ?)'], + ['scalar', 'select "id" from "users" where "tenant_id" = ? and "id" = (select "id" from "members" where "active" = ?)'], + ['parsed', 'select "id" from "users" where "tenant_id" = ? and "id" in (select "id" from "members" where "active" = ?)'], + ]; + } + public function testPostgresInsertGetId(): void { $builder = $this->getPostgresBuilder(); @@ -6822,7 +6907,7 @@ public function testCustomBindingSlotsSurviveFactoriesAndPaginationCloning(): vo $clone = (new ReflectionMethod($builder, 'cloneForPaginationCount'))->invoke($builder); $this->assertInstanceOf(DatabaseQueryBuilderWithCustomBindings::class, $clone); $this->assertSame(['tenant', true], $clone->getBindings()); - $this->assertSame([], $clone->orders); + $this->assertNull($clone->orders); $this->assertNull($clone->limit); $this->assertNull($clone->offset); @@ -7494,6 +7579,59 @@ public function testCursorPaginateMultipleOrderColumns(): void ]), $result); } + public function testCursorPaginateWrapsOrderColumnsContainingParentheses(): void + { + $perPage = 16; + $columns = ['*']; + $cursorName = 'cursor-name'; + $column = 'id) or 1=1 or (id'; + $cursor = new Cursor([$column => 'bar', 'id' => 'foo']); + $builder = $this->getMockQueryBuilder(['get']); + $builder->from('foobar')->orderBy($column)->orderBy('id'); + + $path = 'http://foo.bar?cursor=' . $cursor->encode(); + + $results = collect([['id' => 1], ['id' => 2]]); + + $builder->expects('get')->andReturnUsing(function () use ($builder, $results): Collection { + $this->assertSame( + 'select * from "foobar" where ("id) or 1=1 or (id" > ? or ("id) or 1=1 or (id" = ? and ("id" > ?))) order by "id) or 1=1 or (id" asc, "id" asc limit 17', + $builder->toSql() + ); + $this->assertEquals(['bar', 'bar', 'foo'], $builder->bindings['where']); + + return $results; + }); + + Paginator::currentPathResolver(function () use ($path): string { + return $path; + }); + + $builder->cursorPaginate($perPage, $columns, $cursorName, $cursor); + } + + public function testCustomKeysetIsolationKeepsSelectOrderingAndPageOrderingSeparate(): void + { + $connection = $this->getConnection(); + $builder = new class($connection, new Grammar($connection), new Processor) extends Builder { + /** + * Require paging outside a custom SELECT clause. + */ + protected function requiresKeysetIsolation(bool $limitSelectsRows): bool + { + return true; + } + }; + $builder->from('events')->orderBy('id')->orderByRaw('id + ?', [7]) + ->forPageBeforeId(2, 5); + + $this->assertSame( + 'select * from (select * from "events" order by "id" asc, id + ?) as "cursor_table" where "id" < ? order by "id" desc limit 2', + $builder->toSql() + ); + $this->assertSame([7, 5], $builder->getBindings()); + } + public function testCursorPaginateWithDefaultArguments(): void { $perPage = 15; @@ -7794,6 +7932,72 @@ public function testCursorPaginateWithUnionWheres(): void ]), $result); } + public function testCursorPaginateFiltersNestedUnionsAndRebuildsTheirBindings(): void + { + $builder = $this->getMockQueryBuilder(['get']); + $builder->select('id')->from('videos')->where('active', 1) + ->unionAll($this->getBuilder()->select('id')->from('news')->where('active', 2) + ->unionAll($this->getBuilder()->select('id')->from('archives')->where('active', 3))) + ->orderBy('id'); + + $builder->expects('get')->andReturnUsing(function () use ($builder): Collection { + $this->assertSame( + '(select "id" from "videos" where "active" = ? and ("id" > ?)) union all ((select "id" from "news" where "active" = ? and ("id" > ?)) union all (select "id" from "archives" where "active" = ? and ("id" > ?))) order by "id" asc limit 11', + $builder->toSql() + ); + $this->assertSame([1, 2, 2, 2, 3, 2], $builder->getBindings()); + + return new Collection; + }); + + $builder->cursorPaginate(10, cursor: new Cursor(['id' => 2])); + } + + public function testCursorPaginateWithUnionSelectAndJoinBindings(): void + { + $timestamp = now()->toDateTimeString(); + + $perPage = 16; + $columns = ['test']; + $cursorName = 'cursor-name'; + $cursor = new Cursor(['created_at' => $timestamp]); + $builder = $this->getMockQueryBuilder(['get']); + $builder->select('id', 'created_at')->selectRaw('? as type', ['video'])->from('videos'); + $builder->union($this->getBuilder()->select('id', 'created_at')->selectRaw('? as type', ['news'])->from('news')->join('authors', function (JoinClause $join): void { + $join->on('authors.id', '=', 'news.author_id')->where('authors.active', true); + })); + $builder->orderBy('created_at'); + + $path = 'http://foo.bar?cursor=' . $cursor->encode(); + + $results = collect([ + ['id' => 1, 'created_at' => now(), 'type' => 'video'], + ['id' => 2, 'created_at' => now(), 'type' => 'news'], + ]); + + $builder->expects('get')->andReturnUsing(function () use ($builder, $results, $timestamp): Collection { + $this->assertSame( + '(select "id", "created_at", ? as type from "videos" where ("created_at" > ?)) union (select "id", "created_at", ? as type from "news" inner join "authors" on "authors"."id" = "news"."author_id" and "authors"."active" = ? where ("created_at" > ?)) order by "created_at" asc limit 17', + $builder->toSql() + ); + $this->assertEquals(['video', $timestamp, 'news', true, $timestamp], $builder->getBindings()); + + return $results; + }); + + Paginator::currentPathResolver(function () use ($path): string { + return $path; + }); + + $result = $builder->cursorPaginate($perPage, $columns, $cursorName, $cursor); + + $this->assertEquals(new CursorPaginator($results, $perPage, $cursor, [ + 'path' => $path, + 'cursorName' => $cursorName, + 'parameters' => ['created_at'], + ]), $result); + } + public function testCursorPaginateWithMultipleUnionsAndMultipleWheres(): void { $ts = now()->toDateTimeString(); @@ -7865,7 +8069,7 @@ public function testCursorPaginateWithUnionMultipleWheresMultipleOrders(): void $builder->expects('get')->andReturnUsing(function () use ($builder, $results, $ts) { $this->assertSame( - '(select "id", "start_time" as "created_at", "type" from "videos" where "extra" = ? and ("id" > ? or ("id" = ? and ("start_time" < ? or ("start_time" = ? and ("type" > ?)))))) union (select "id", "created_at", "type" from "news" where "extra" = ? and ("id" > ? or ("id" = ? and ("start_time" < ? or ("start_time" = ? and ("type" > ?)))))) union (select "id", "created_at", "type" from "podcasts" where "extra" = ? and ("id" > ? or ("id" = ? and ("start_time" < ? or ("start_time" = ? and ("type" > ?)))))) order by "id" asc, "created_at" desc, "type" asc limit 17', + '(select "id", "start_time" as "created_at", "type" from "videos" where "extra" = ? and ("id" > ? or ("id" = ? and ("start_time" < ? or ("start_time" = ? and ("type" > ?)))))) union (select "id", "created_at", "type" from "news" where "extra" = ? and ("id" > ? or ("id" = ? and ("created_at" < ? or ("created_at" = ? and ("type" > ?)))))) union (select "id", "created_at", "type" from "podcasts" where "extra" = ? and ("id" > ? or ("id" = ? and ("created_at" < ? or ("created_at" = ? and ("type" > ?)))))) order by "id" asc, "created_at" desc, "type" asc limit 17', $builder->toSql() ); $this->assertEquals(['first', 1, 1, $ts, $ts, 'news'], $builder->bindings['where']); @@ -8065,6 +8269,26 @@ public function testCursorPaginateWithUnionWheresAndAliassedOrderColumns(): void ]), $result); } + public function testCursorPaginateWithUnionOrderExpressions(): void + { + $builder = $this->getMockQueryBuilder(['get']); + $builder->from('videos')->select('id')->selectRaw('(score * 2) as ranked'); + $builder->union($this->getBuilder()->from('news')->select('id')->selectRaw('(score * 3) as ranked')); + $builder->orderBy('ranked')->orderBy('id'); + + $builder->expects('get')->andReturnUsing(function () use ($builder): Collection { + $this->assertSame( + '(select "id", (score * 2) as ranked from "videos" where ((score * 2) > ? or ((score * 2) = ? and ("id" > ?)))) union (select "id", (score * 3) as ranked from "news" where ((score * 3) > ? or ((score * 3) = ? and ("id" > ?)))) order by "ranked" asc, "id" asc limit 17', + $builder->toSql() + ); + $this->assertSame([10, 10, 1, 10, 10, 1], $builder->getBindings()); + + return new Collection; + }); + + $builder->cursorPaginate(16, cursor: new Cursor(['ranked' => 10, 'id' => 1])); + } + public function testWhereExpression(): void { $builder = $this->getBuilder(); @@ -8104,6 +8328,14 @@ public function testWhereRowValuesArityMismatch(): void $builder->select('*')->from('orders')->whereRowValues(['last_update'], '<', [1, 2]); } + public function testWhereRowValuesInvalidOperator(): void + { + $this->expectExceptionObject(new InvalidArgumentException('Invalid operator passed to whereRowValues method.')); + + $builder = $this->getBuilder(); + $builder->select('*')->from('orders')->whereRowValues(['last_update', 'order_number'], '< (1, 2) or 1=1 or (1, 2) <', [1, 2]); + } + public function testWhereJsonContainsMySql(): void { $builder = $this->getMySqlBuilder(); @@ -8642,6 +8874,30 @@ public function testCloneWithoutBindings(): void $this->assertSame([], $clone->getBindings()); } + public function testCloneWithoutClearsNullablePropertiesAndResetsNonNullableProperties(): void + { + $connection = $this->getConnection(); + $builder = new class($connection, new Grammar($connection), new Processor) extends Builder { + public ?int $limit = 10; + }; + $builder->from('users')->select('id')->distinct()->where('active', true)->groupLimit(2, 'team_id'); + $builder->aggregate = ['function' => 'count', 'columns' => ['*']]; + + $clone = $builder->cloneWithout(['columns', 'aggregate', 'groupLimit', 'limit', 'distinct', 'wheres']) + ->cloneWithoutBindings(['where']); + + $this->assertSame('select * from "users"', $clone->toSql()); + $this->assertSame([], $clone->getBindings()); + $this->assertNull($clone->limit); + $this->assertNull($clone->columns); + $this->assertNull($clone->aggregate); + $this->assertNull($clone->groupLimit); + $this->assertFalse($clone->distinct); + $this->assertSame([], $clone->wheres); + $this->assertSame(10, $builder->limit); + $this->assertSame(['id'], $builder->columns); + } + public function testWhereVectorSimilarToOnPostgres(): void { $builder = $this->getPostgresBuilder(); diff --git a/tests/Integration/Database/EloquentCursorPaginateTest.php b/tests/Integration/Database/EloquentCursorPaginateTest.php index b884195aaf..f6a2967523 100644 --- a/tests/Integration/Database/EloquentCursorPaginateTest.php +++ b/tests/Integration/Database/EloquentCursorPaginateTest.php @@ -5,16 +5,19 @@ namespace Hypervel\Tests\Integration\Database; use Hypervel\Database\BinaryParameter; +use Hypervel\Database\Eloquent\Builder; use Hypervel\Database\Eloquent\Casts\AsBinary; use Hypervel\Database\Eloquent\Model; use Hypervel\Database\Eloquent\Relations\BelongsToMany; use Hypervel\Database\Eloquent\Relations\HasManyThrough; use Hypervel\Database\Eloquent\Relations\Pivot; +use Hypervel\Database\Query\Builder as QueryBuilder; use Hypervel\Database\Query\Expression; use Hypervel\Database\Schema\Blueprint; use Hypervel\Pagination\Cursor; use Hypervel\Support\Facades\DB; use Hypervel\Support\Facades\Schema; +use PHPUnit\Framework\Attributes\DataProvider; class EloquentCursorPaginateTest extends DatabaseTestCase { @@ -88,6 +91,118 @@ public function testPaginationWithUnion(): void $this->assertSame(['user_id'], $result->getOptions()['parameters']); } + #[DataProvider('boundedUnionProvider')] + public function testPaginationPreservesBoundedUnionMembers(string $shape, array $firstIds, array $secondIds): void + { + for ($id = 1; $id <= 7; ++$id) { + TestPost::create(['id' => $id]); + } + + $calls = 0; + $query = function () use ($shape, &$calls): QueryBuilder { + $bounded = DB::table('test_posts')->where('id', '<', 7)->orderByDesc('id')->limit(3); + $ordinary = DB::table('test_posts')->select('id as identifier')->where('id', 7); + + $query = match ($shape) { + 'root' => $bounded->unionAll($ordinary), + 'member' => $ordinary->unionAll($bounded->select('id as identifier')), + 'group' => $ordinary->unionAll( + DB::table('test_posts')->select('id as identifier')->where('id', '<=', 3) + ->unionAll(DB::table('test_posts')->select('id as identifier')->whereBetween('id', [4, 6])) + ->orderByDesc('identifier')->offset(1)->limit(3) + ), + }; + + return $query->orderByDesc('identifier')->beforeQuery(function () use (&$calls): void { + ++$calls; + }); + }; + + $first = $query()->cursorPaginate(2, ['id as identifier']); + $this->assertSame($firstIds, $first->pluck('identifier')->all()); + $this->assertSame(1, $calls); + + $second = $query()->cursorPaginate(2, ['id as identifier'], cursor: $first->nextCursor()); + $this->assertSame($secondIds, $second->pluck('identifier')->all()); + $this->assertSame(2, $calls); + + $previous = $query()->cursorPaginate(2, ['id as identifier'], cursor: $second->previousCursor()); + $this->assertSame($firstIds, $previous->pluck('identifier')->all()); + $this->assertSame(3, $calls); + + $after = $query()->cursorPaginate(2, ['id as identifier'], cursor: new Cursor(['identifier' => $secondIds[1]])); + $this->assertSame([], $after->items()); + $this->assertSame(4, $calls); + } + + /** + * Provide bounded primary selects, members and complete union groups. + */ + public static function boundedUnionProvider(): array + { + return [ + ['root', [7, 6], [5, 4]], + ['member', [7, 6], [5, 4]], + ['group', [7, 5], [4, 3]], + ]; + } + + public function testBoundedEloquentUnionKeepsScopesAndEagerLoads(): void + { + TestUser::fillAndInsert([['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4]]); + TestPost::fillAndInsert([['user_id' => 1], ['user_id' => 3], ['user_id' => 4]]); + + $scopeCalls = 0; + $query = function () use (&$scopeCalls): Builder { + return TestUser::with('posts') + ->withGlobalScope('visible', function (Builder $query) use (&$scopeCalls): void { + ++$scopeCalls; + $query->where('test_users.id', '>=', 2); + }) + ->orderByDesc('id')->limit(2) + ->unionAll(TestUser::where('id', 1)) + ->orderByDesc('id'); + }; + + $first = $query()->cursorPaginate(2); + $this->assertSame([4, 3], $first->getCollection()->modelKeys()); + $this->assertSame(1, $scopeCalls); + + $second = $query()->cursorPaginate(2, cursor: $first->nextCursor()); + $this->assertSame([1], $second->getCollection()->modelKeys()); + $this->assertSame(2, $scopeCalls); + $this->assertTrue($second->items()[0]->relationLoaded('posts')); + $this->assertSame([1], $second->items()[0]->posts->pluck('user_id')->all()); + + $previous = $query()->cursorPaginate(2, cursor: $second->previousCursor()); + $this->assertSame([4, 3], $previous->getCollection()->modelKeys()); + $this->assertSame(3, $scopeCalls); + } + + public function testBoundedUnionMemberKeepsItsImplicitProjection(): void + { + TestPost::fillAndInsert([['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4]]); + $query = fn (): QueryBuilder => DB::table('test_posts')->where('id', '<=', 2)->orderByDesc('id')->limit(1) + ->unionAll(DB::table('test_posts', 'archive')->where('id', '>', 2)->orderByDesc('id')->limit(1)) + ->orderByDesc('id'); + + $first = $query()->cursorPaginate(1, ['test_posts.*']); + $this->assertSame([4], $first->pluck('id')->all()); + + $second = $query()->cursorPaginate(1, ['test_posts.*'], cursor: $first->nextCursor()); + $this->assertSame([2], $second->pluck('id')->all()); + } + + public function testOrdinaryCursorPaginationStillOwnsItsLimit(): void + { + TestPost::fillAndInsert([['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4]]); + + $page = DB::table('test_posts')->orderBy('id')->limit(1) + ->cursorPaginate(3, cursor: new Cursor(['id' => 1])); + + $this->assertSame([2, 3, 4], $page->pluck('id')->all()); + } + public function testPaginationWithDistinct(): void { for ($i = 1; $i <= 3; ++$i) { diff --git a/tests/Integration/Database/QueryBuilderTest.php b/tests/Integration/Database/QueryBuilderTest.php index a7c1be6307..1e6c7e0d50 100644 --- a/tests/Integration/Database/QueryBuilderTest.php +++ b/tests/Integration/Database/QueryBuilderTest.php @@ -658,6 +658,15 @@ public function testPaginateWithSpecificColumns(): void ]); } + public function testGetCountForPaginationWithGroupedUnionAndLimit(): void + { + $query = DB::table('posts')->select('title')->groupBy('title') + ->unionAll(DB::table('posts')->select('title')->where('title', 'Foo Post')) + ->limit(1); + + $this->assertSame(3, $query->getCountForPagination()); + } + public function testChunkMap(): void { DB::enableQueryLog(); From 196ddd0d66715b389f1c4b271f7a2d3e0c041bac Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 29 Oct 2026 10:32:34 +0000 Subject: [PATCH 5/7] Separate pagination changes from the attribute fixes Remove the coupled union pagination, count, query preparation and embedding changes from this branch so they can be completed together in a dedicated change. Restore the previous query-builder implementation, corresponding tests and driver documentation. Keep the independent Laravel #61837 and #61838 fixes: quote literal cursor columns containing parentheses and reject invalid row-value operators. Their complete regression tests remain in this branch. Validated the restored query-builder, embedding, pagination and integration test files, the Database suite, formatting and full static analysis. --- src/database/src/Concerns/BuildsQueries.php | 86 ++- src/database/src/Eloquent/Builder.php | 8 +- src/database/src/Query/Builder.php | 508 +----------------- src/docs/database.md | 4 +- .../DatabaseQueryBuilderEmbeddingTest.php | 6 +- .../DatabaseQueryBuilderPaginationTest.php | 86 --- tests/Database/DatabaseQueryBuilderTest.php | 221 +------- .../Database/EloquentCursorPaginateTest.php | 115 ---- .../Integration/Database/QueryBuilderTest.php | 9 - 9 files changed, 103 insertions(+), 940 deletions(-) diff --git a/src/database/src/Concerns/BuildsQueries.php b/src/database/src/Concerns/BuildsQueries.php index cdeb5e8f71..639c8b35ee 100644 --- a/src/database/src/Concerns/BuildsQueries.php +++ b/src/database/src/Concerns/BuildsQueries.php @@ -10,6 +10,7 @@ use Hypervel\Database\Eloquent\Model; use Hypervel\Database\MultipleRecordsFoundException; use Hypervel\Database\Query\Builder as QueryBuilder; +use Hypervel\Database\Query\Expression; use Hypervel\Database\RecordNotFoundException; use Hypervel\Database\RecordsNotFoundException; use Hypervel\Pagination\Cursor; @@ -505,26 +506,81 @@ protected function paginateUsingCursor( : CursorPaginator::resolveCurrentCursor($cursorName, $cursor); } - $query = $this instanceof Builder ? $this->getQuery() : $this; - - if ($cursor !== null) { - $query->prepareCursorOrdering(); - } - $orders = $this->ensureOrderForCursorPagination(! is_null($cursor) && $cursor->pointsToPreviousItems()); if (! is_null($cursor)) { - // Resolve model casts before a bounded query is replaced by its derived table. - $bindings = $orders->mapWithKeys(fn (array $order): array => [ - $order['column'] => $this->getCursorParameterBinding($cursor, $order['column']), - ]); - - // The base builder owns placement after scopes and callbacks; applying scopes - // here would freeze them before callers can remove or change them. - $query->addCursorPaginationConditions($orders->all(), $bindings->all()); + // Reset the union bindings so we can add the cursor where in the correct position... + $this->setBindings([], 'union'); + + $addCursorConditions = function (QueryBuilder|Builder $builder, $previousColumn, $originalColumn, $i) use (&$addCursorConditions, $cursor, $orders) { + // Unions are base queries; the other builder family only appears in nested constraints without unions. + $unionBuilders = $builder instanceof self ? $builder->getUnionBuilders() : new Collection; + + if (! is_null($previousColumn)) { + $originalColumn ??= $this->getOriginalColumnNameForCursorPagination($this, $previousColumn); + + $builder->where( + $originalColumn !== $previousColumn && Str::contains($originalColumn, ['(', ')']) ? new Expression($originalColumn) : $originalColumn, + '=', + $this->getCursorParameterBinding($cursor, $previousColumn) + ); + + $unionBuilders->each(function ($unionBuilder) use ($previousColumn, $cursor) { + $unionBuilder->where( + $this->getOriginalColumnNameForCursorPagination($unionBuilder, $previousColumn), + '=', + $this->getCursorParameterBinding($cursor, $previousColumn) + ); + + $this->addBinding($unionBuilder->getRawBindings()['where'], 'union'); + }); + } + + $builder->where(function (QueryBuilder|Builder $secondBuilder) use ($addCursorConditions, $cursor, $orders, $i, $unionBuilders) { + ['column' => $column, 'direction' => $direction] = $orders[$i]; + + $originalColumn = $this->getOriginalColumnNameForCursorPagination($this, $column); + + $secondBuilder->where( + $originalColumn !== $column && Str::contains($originalColumn, ['(', ')']) ? new Expression($originalColumn) : $originalColumn, + $direction === 'asc' ? '>' : '<', + $this->getCursorParameterBinding($cursor, $column) + ); + + if ($i < $orders->count() - 1) { + $secondBuilder->orWhere(function (QueryBuilder|Builder $thirdBuilder) use ($addCursorConditions, $column, $originalColumn, $i) { + $addCursorConditions($thirdBuilder, $column, $originalColumn, $i + 1); + }); + } + + $unionBuilders->each(function ($unionBuilder) use ($column, $direction, $cursor, $i, $orders, $addCursorConditions) { + $unionWheres = $unionBuilder->getRawBindings()['where']; + + $originalColumn = $this->getOriginalColumnNameForCursorPagination($unionBuilder, $column); + $unionBuilder->where(function ($unionBuilder) use ($column, $direction, $cursor, $i, $orders, $addCursorConditions, $originalColumn, $unionWheres) { + $unionBuilder->where( + $originalColumn, + $direction === 'asc' ? '>' : '<', + $this->getCursorParameterBinding($cursor, $column) + ); + + if ($i < $orders->count() - 1) { + $unionBuilder->orWhere(function (QueryBuilder|Builder $fourthBuilder) use ($addCursorConditions, $column, $originalColumn, $i) { + $addCursorConditions($fourthBuilder, $column, $originalColumn, $i + 1); + }); + } + + $this->addBinding($unionWheres, 'union'); + $this->addBinding($unionBuilder->getRawBindings()['where'], 'union'); + }); + }); + }); + }; + + $addCursorConditions($this, null, null, 0); } - $query->limit($perPage + 1); + $this->limit($perPage + 1); return $this->cursorPaginator($this->get($columns), $perPage, $cursor, [ 'path' => Paginator::resolveCurrentPath(), diff --git a/src/database/src/Eloquent/Builder.php b/src/database/src/Eloquent/Builder.php index d847a5d920..52821cc823 100644 --- a/src/database/src/Eloquent/Builder.php +++ b/src/database/src/Eloquent/Builder.php @@ -1175,12 +1175,8 @@ protected function ensureOrderForCursorPagination(bool $shouldReverse = false): }; if ($shouldReverse) { - if (! empty($this->query->unionOrders)) { - // Keep the first SELECT's limit ordering; reverse only the union's page ordering. - $this->query->unionOrders = (new BaseCollection($this->query->unionOrders))->map($reverseDirection)->toArray(); - } else { - $this->query->orders = (new BaseCollection($this->query->orders))->map($reverseDirection)->toArray(); - } + $this->query->orders = (new BaseCollection($this->query->orders))->map($reverseDirection)->toArray(); + $this->query->unionOrders = (new BaseCollection($this->query->unionOrders))->map($reverseDirection)->toArray(); } $orders = ! empty($this->query->unionOrders) ? $this->query->unionOrders : $this->query->orders; diff --git a/src/database/src/Query/Builder.php b/src/database/src/Query/Builder.php index a67622ed1d..398db7d6a6 100644 --- a/src/database/src/Query/Builder.php +++ b/src/database/src/Query/Builder.php @@ -32,7 +32,6 @@ use Hypervel\Support\Arr; use Hypervel\Support\Collection; use Hypervel\Support\LazyCollection; -use Hypervel\Support\Str; use Hypervel\Support\StrCache; use Hypervel\Support\Stringable; use Hypervel\Support\Traits\ForwardsCalls; @@ -41,7 +40,6 @@ use JsonException; use LogicException; use PDO; -use ReflectionProperty; use RuntimeException; use SortDirection; use stdClass; @@ -208,18 +206,6 @@ class Builder implements BuilderContract */ public array $beforeQueryCallbacks = []; - /** - * Indicates whether paging conditions need preparation after callbacks and scopes. - */ - protected bool $hasKeysetConditions = false; - - /** - * The SELECT ordering before paging changed the traversal direction. - * - * @var null|array{orders: array, bindings: array, retainedBindings: int, pageBindingCount: int} - */ - protected ?array $keysetOrdering = null; - /** * The callbacks that should be invoked after retrieving data from the database. * @@ -409,7 +395,7 @@ protected function parseSub(mixed $query): array } if ($query instanceof self) { - $this->prepareQueryForEmbedding($query); + $this->ensureCanEmbedQuery($query); $query = $this->prependDatabaseNameIfCrossDatabaseQuery($query); @@ -1863,7 +1849,7 @@ protected function whereSub(ExpressionContract|string $column, string $operator, $query = $callback instanceof self ? $callback : $callback->toBase(); } - $this->prepareQueryForEmbedding($query); + $this->ensureCanEmbedQuery($query); $this->wheres[] = [ 'type' => $type, 'column' => $column, 'operator' => $operator, 'query' => $query, 'boolean' => $boolean, @@ -1940,7 +1926,7 @@ public function orWhereNotExists(Closure|self|EloquentBuilder $callback): static */ public function addWhereExistsQuery(self $query, string $boolean = 'and', bool $not = false): static { - $this->prepareQueryForEmbedding($query); + $this->ensureCanEmbedQuery($query); $type = $not ? 'NotExists' : 'Exists'; @@ -2775,16 +2761,7 @@ public function forPageBeforeId(int $perPage = 15, mixed $lastId = 0, string $co $lastId = BinaryParameter::fromStream($lastId); } - $this->rememberKeysetOrdering(); - $bindingOffset = count($this->bindings['where']); - $orderColumn = $this->unions ? $this->getKeysetOutputColumn($column) : $column; - - if ($this->unions) { - $this->unionOrders = $this->removeOrdersFor($this->unionOrders ?? [], $this->bindings['unionOrder'], $orderColumn); - } else { - $this->orders = $this->removeExistingOrdersFor($column); - $this->keysetOrdering['retainedBindings'] = count($this->bindings['order']); - } + $this->orders = $this->removeExistingOrdersFor($column); if (is_null($lastId)) { $this->whereNotNull($column); @@ -2792,14 +2769,8 @@ public function forPageBeforeId(int $perPage = 15, mixed $lastId = 0, string $co $this->where($column, '<', $lastId); } - $this->markKeysetCondition($bindingOffset, $column); - $this->orderBy($orderColumn, SortDirection::Descending)->limit($perPage); - - if (! $this->unions) { - $this->orders[array_key_last($this->orders)]['keyset'] = true; - } - - return $this; + return $this->orderBy($column, SortDirection::Descending) + ->limit($perPage); } /** @@ -2813,16 +2784,7 @@ public function forPageAfterId(int $perPage = 15, mixed $lastId = 0, string $col $lastId = BinaryParameter::fromStream($lastId); } - $this->rememberKeysetOrdering(); - $bindingOffset = count($this->bindings['where']); - $orderColumn = $this->unions ? $this->getKeysetOutputColumn($column) : $column; - - if ($this->unions) { - $this->unionOrders = $this->removeOrdersFor($this->unionOrders ?? [], $this->bindings['unionOrder'], $orderColumn); - } else { - $this->orders = $this->removeExistingOrdersFor($column); - $this->keysetOrdering['retainedBindings'] = count($this->bindings['order']); - } + $this->orders = $this->removeExistingOrdersFor($column); if (is_null($lastId)) { $this->whereNotNull($column); @@ -2830,14 +2792,8 @@ public function forPageAfterId(int $perPage = 15, mixed $lastId = 0, string $col $this->where($column, '>', $lastId); } - $this->markKeysetCondition($bindingOffset, $column); - $this->orderBy($orderColumn, SortDirection::Ascending)->limit($perPage); - - if (! $this->unions) { - $this->orders[array_key_last($this->orders)]['keyset'] = true; - } - - return $this; + return $this->orderBy($column, SortDirection::Ascending) + ->limit($perPage); } /** @@ -2847,18 +2803,13 @@ public function forPageAfterId(int $perPage = 15, mixed $lastId = 0, string $col */ public function reorder(Closure|self|EloquentBuilder|Relation|ExpressionContract|string|null $column = null, SortDirection|string $direction = SortDirection::Ascending): static { - $this->keysetOrdering = null; $this->orders = null; $this->unionOrders = null; $this->bindings['order'] = []; $this->bindings['unionOrder'] = []; if ($column) { - $this->orderBy($column, $direction); - } - - if ($this->hasKeysetConditions) { - $this->prepareCursorOrdering(); + return $this->orderBy($column, $direction); } return $this; @@ -2877,15 +2828,7 @@ public function reorderDesc(Closure|self|EloquentBuilder|Relation|ExpressionCont */ protected function removeExistingOrdersFor(string $column): array { - return $this->removeOrdersFor($this->orders ?? [], $this->bindings['order'], $column); - } - - /** - * Remove matching orders from the ordering and bindings owned by a SELECT or union. - */ - protected function removeOrdersFor(array $orders, array &$bindings, string $column): array - { - return (new Collection($orders)) + return (new Collection($this->orders)) ->reject(fn ($order) => isset($order['column']) && $order['column'] === $column) ->values() ->all(); @@ -2908,7 +2851,7 @@ public function union(Closure|self|EloquentBuilder $query, bool $all = false): s $query = $query->toBase(); } - $this->prepareQueryForEmbedding($query); + $this->ensureCanEmbedQuery($query); $this->unions[] = ['query' => $query, 'all' => $all]; @@ -2992,12 +2935,6 @@ public function applyBeforeQueryCallbacks(): void } $this->beforeQueryCallbacks = []; - - // Scopes and callbacks may add row-limiting clauses. Place paging predicates only - // after those clauses exist, without changing Eloquent's scope lifecycle. - if ($this->hasKeysetConditions) { - $this->completeKeysetPagination(); - } } /** @@ -3292,12 +3229,8 @@ protected function ensureOrderForCursorPagination(bool $shouldReverse = false): }; if ($shouldReverse) { - if (! empty($this->unionOrders)) { - // Keep the first SELECT's limit ordering; reverse only the union's page ordering. - $this->unionOrders = (new Collection($this->unionOrders))->map($reverseDirection)->toArray(); - } else { - $this->orders = (new Collection($this->orders))->map($reverseDirection)->toArray(); - } + $this->orders = (new Collection($this->orders))->map($reverseDirection)->toArray(); + $this->unionOrders = (new Collection($this->unionOrders))->map($reverseDirection)->toArray(); } $orders = ! empty($this->unionOrders) ? $this->unionOrders : $this->orders; @@ -3307,369 +3240,6 @@ protected function ensureOrderForCursorPagination(bool $shouldReverse = false): ->values(); } - /** - * Preserve the SELECT's ordering before paging changes its traversal order. - */ - protected function rememberKeysetOrdering(): void - { - if ($this->unions) { - return; - } - - if ($this->keysetOrdering === null) { - $this->keysetOrdering = [ - 'orders' => $this->orders ?? [], - 'bindings' => $this->bindings['order'], - 'retainedBindings' => count($this->bindings['order']), - 'pageBindingCount' => 0, - ]; - } else { - foreach ($this->orders ?? [] as $order) { - if (! isset($order['keyset']) && ! isset($order['keysetOriginal'])) { - $this->keysetOrdering['orders'][] = $order; - } - } - - array_push($this->keysetOrdering['bindings'], ...array_slice( - $this->bindings['order'], - $this->keysetOrdering['retainedBindings'] - )); - $this->keysetOrdering['retainedBindings'] = count($this->bindings['order']); - } - - foreach ($this->orders ?? [] as $index => $order) { - if (! isset($order['keyset'])) { - $this->orders[$index]['keysetOriginal'] = true; - } - } - } - - /** - * Retain the SELECT ordering for cursor pagination's traversal direction. - */ - public function prepareCursorOrdering(): void - { - $this->rememberKeysetOrdering(); - - if (! $this->unions) { - $this->keysetOrdering['pageBindingCount'] = count($this->bindings['order']); - foreach ($this->orders ?? [] as $index => $order) { - $this->orders[$index]['keyset'] = true; - } - } - } - - /** - * Record the paging predicate and the bindings it owns. - */ - protected function markKeysetCondition(int $offset, ?string $column = null): void - { - $index = array_key_last($this->wheres); - $this->wheres[$index]['keyset'] = [ - 'offset' => $offset, - 'count' => count($this->bindings['where']) - $offset, - ]; - - if ($column !== null) { - $this->wheres[$index]['keysetColumn'] = $column; - } - - $this->hasKeysetConditions = true; - } - - /** - * Add cursor pagination's internal predicates with their owned columns and bindings. - */ - public function addCursorPaginationConditions(array $orders, array $bindings): static - { - $offset = count($this->bindings['where']); - $this->addCursorComparisons($this, $orders, $bindings, 0); - $this->markKeysetCondition($offset); - - if (! $this->unions) { - foreach ($this->orders ?? [] as $index => $order) { - $this->orders[$index]['keyset'] = true; - } - } - - return $this; - } - - /** - * Add the ordered comparisons for a compound cursor. - */ - protected function addCursorComparisons(self $owner, array $orders, array $bindings, int $index): void - { - ['column' => $parameter, 'direction' => $direction] = $orders[$index]; - $column = $this->getOriginalColumnNameForCursorPagination($owner, $parameter); - - if ($column !== $parameter && Str::contains($column, ['(', ')'])) { - $column = new Expression($column); - } - - $this->where(function (self $query) use ($owner, $orders, $bindings, $index, $parameter, $direction, $column): void { - $query->where($column, $direction === 'asc' ? '>' : '<', $bindings[$parameter]); - $query->wheres[array_key_last($query->wheres)]['keysetColumn'] = $parameter; - - if ($index < count($orders) - 1) { - $query->orWhere(function (self $query) use ($owner, $orders, $bindings, $index, $parameter, $column): void { - $query->where($column, '=', $bindings[$parameter]); - $query->wheres[array_key_last($query->wheres)]['keysetColumn'] = $parameter; - $query->addCursorComparisons($owner, $orders, $bindings, $index + 1); - }); - } - }); - } - - /** - * Determine whether paging must filter outside this SELECT's row limit. - */ - protected function requiresKeysetIsolation(bool $limitSelectsRows): bool - { - return $limitSelectsRows && ($this->limit !== null || $this->offset !== null); - } - - /** - * Place paging predicates outside bounded SELECTs and in every union member. - */ - protected function completeKeysetPagination(): void - { - $this->hasKeysetConditions = false; - $limitSelectsRows = (bool) $this->unions; - $isolate = $this->requiresKeysetIsolation($limitSelectsRows); - - if ($isolate) { - $this->rememberKeysetOrdering(); - } - $conditions = []; - // Ordinary pages only retire their markers; they do not need normalized - // condition copies. Retaining markers would replay them if a union is added later. - $this->wheres = $this->collectKeysetConditions($this->wheres, $conditions, $isolate, $isolate || $limitSelectsRows); - - if ($isolate) { - // Binding offsets are recorded when predicates are added, so scope grouping - // cannot make us remove a user's or a scope's bindings by mistake. - $ranges = array_column($conditions, 'range'); - usort($ranges, static fn (array $left, array $right): int => $right['offset'] <=> $left['offset']); - foreach ($ranges as $range) { - array_splice($this->bindings['where'], $range['offset'], $range['count']); - } - - $pageOrders = array_values(array_filter($this->orders ?? [], static fn (array $order): bool => isset($order['keyset']))); - foreach ($pageOrders as $index => $order) { - if (isset($order['column']) && is_string($order['column'])) { - $pageOrders[$index]['column'] = $this->getKeysetOutputColumn($order['column']); - } - } - $pageBindings = array_slice($this->bindings['order'], 0, $this->keysetOrdering['pageBindingCount'] ?? 0); - $this->isolateKeysetSelect($limitSelectsRows); - $this->appendKeysetConditions($conditions); - - if (! $limitSelectsRows) { - foreach ($pageOrders as $order) { - $this->orders[] = $order; - } - $this->bindings['order'] = $pageBindings; - } - } - - if ($this->unions) { - $this->bindings['union'] = []; - - foreach ($this->unions as $index => $union) { - // Union members are shared by shallow builder clones. Paging one clone - // must not add its cursor to another query built from the same member. - $member = clone $union['query']; - $member->applyBeforeQueryCallbacks(); - $member = $member->applyKeysetConditions($conditions); - $this->unions[$index]['query'] = $member; - $this->addBinding($member->getBindings(), 'union'); - } - } - - $this->keysetOrdering = null; - foreach ($this->orders ?? [] as $index => $order) { - unset($this->orders[$index]['keyset'], $this->orders[$index]['keysetOriginal']); - } - } - - /** - * Collect owned predicates without changing nested groups shared with another clone. - */ - protected function collectKeysetConditions(array $wheres, array &$conditions, bool $remove, bool $collect = true): array - { - foreach ($wheres as $index => $where) { - if (isset($where['keyset'])) { - $range = $where['keyset']; - unset($where['keyset']); - if ($collect) { - $conditions[] = [ - 'where' => $this->normalizeKeysetColumns($where), - 'bindings' => array_slice($this->bindings['where'], $range['offset'], $range['count']), - 'range' => $range, - ]; - } - - if ($remove) { - unset($wheres[$index]); - } else { - $wheres[$index] = $where; - } - } elseif ($where['type'] === 'Nested') { - $nestedWheres = $this->collectKeysetConditions($where['query']->wheres, $conditions, $remove, $collect); - if ($remove && $nestedWheres === []) { - unset($wheres[$index]); - } elseif ($nestedWheres !== $where['query']->wheres) { - // Clear consumed markers without changing a group shared with another - // clone. Otherwise a later page call would replay old union predicates. - $where['query'] = clone $where['query']; - $where['query']->wheres = $nestedWheres; - $wheres[$index] = $where; - } - } - } - - return $remove ? array_values($wheres) : $wheres; - } - - /** - * Resolve paging columns to the output names shared by the union's SELECTs. - */ - protected function normalizeKeysetColumns(array $where): array - { - if (isset($where['keysetColumn'])) { - $where['keysetColumn'] = $this->getKeysetOutputColumn($where['keysetColumn']); - } - - if ($where['type'] === 'Nested') { - $where['query'] = clone $where['query']; - $where['query']->wheres = array_map($this->normalizeKeysetColumns(...), $where['query']->wheres); - } - - return $where; - } - - /** - * Get the selected output name for a paging column. - */ - protected function getKeysetOutputColumn(string $parameter): string - { - foreach ($this->getColumns() as $column) { - if (($position = strripos($column, ' as ')) !== false) { - $original = substr($column, 0, $position); - $alias = substr($column, $position + 4); - $quotes = $this->grammar->wrap(''); - if (str_starts_with($alias, $quotes[0]) && str_ends_with($alias, $quotes[-1])) { - $alias = str_replace($quotes[-1] . $quotes[-1], $quotes[-1], substr($alias, 1, -1)); - } - if ($parameter === $alias || $parameter === $original) { - return $alias; - } - } - } - - return Str::afterLast($parameter, '.'); - } - - /** - * Apply inherited paging predicates to this member and its nested unions. - */ - protected function applyKeysetConditions(array $conditions): static - { - if ($this->unions && ($this->unionLimit !== null || $this->unionOffset !== null)) { - return $this->newQuery()->fromSub($this, 'cursor_table')->appendKeysetConditions($conditions); - } - - if ($this->requiresKeysetIsolation(true)) { - $this->isolateKeysetSelect(true); - } - - $this->appendKeysetConditions($conditions); - - if ($this->unions) { - $this->bindings['union'] = []; - foreach ($this->unions as $index => $union) { - $member = clone $union['query']; - $member->applyBeforeQueryCallbacks(); - $member = $member->applyKeysetConditions($conditions); - $this->unions[$index]['query'] = $member; - $this->addBinding($member->getBindings(), 'union'); - } - } - - return $this; - } - - /** - * Append paging predicates using this SELECT's own source columns and expressions. - */ - protected function appendKeysetConditions(array $conditions): static - { - foreach ($conditions as $condition) { - $this->wheres[] = $this->rebindKeysetColumns($condition['where']); - $this->addBinding($condition['bindings'], 'where'); - } - - return $this; - } - - /** - * Resolve output names against this SELECT without rewriting user predicates. - */ - protected function rebindKeysetColumns(array $where): array - { - if (isset($where['keysetColumn'])) { - $parameter = $where['keysetColumn']; - $column = $this->getOriginalColumnNameForCursorPagination($this, $parameter); - $where['column'] = $column !== $parameter && Str::contains($column, ['(', ')']) - ? new Expression($column) - : $column; - unset($where['keysetColumn']); - } - - if ($where['type'] === 'Nested') { - $where['query'] = clone $where['query']; - $where['query']->wheres = array_map($this->rebindKeysetColumns(...), $where['query']->wheres); - } - - return $where; - } - - /** - * Move the completed SELECT into a derived table while retaining the page's clauses. - */ - protected function isolateKeysetSelect(bool $limitSelectsRows): void - { - $aggregate = $this->aggregate; - $inner = $this->cloneWithout(['aggregate', 'unions', 'unionOrders', 'unionLimit', 'unionOffset', 'timeout']) - ->cloneWithoutBindings(['union', 'unionOrder']); - $inner->columns ??= ['*']; - $limit = $this->limit; - $offset = $this->offset; - - if (! $limitSelectsRows) { - $inner->limit = $inner->offset = null; - $inner->orders = $this->keysetOrdering['orders'] ?? $inner->orders; - $inner->bindings['order'] = $this->keysetOrdering['bindings'] ?? $inner->bindings['order']; - } - - $this->resetProperties([ - 'aggregate', 'columns', 'distinct', 'from', 'indexHint', 'joins', 'wheres', 'groups', 'havings', - 'orders', 'limit', 'groupLimit', 'offset', 'lock', - ]); - foreach (['select', 'from', 'join', 'where', 'groupBy', 'having', 'order'] as $binding) { - $this->bindings[$binding] = []; - } - $this->columns = ['*']; - $this->aggregate = $aggregate; - $this->fromSub($inner, 'cursor_table'); - - if (! $limitSelectsRows) { - $this->limit = $limit; - $this->offset = $offset; - } - } - /** * Get the count of the total records for the paginator. * @@ -3746,12 +3316,8 @@ protected function runPaginationCountQuery(array $columns = ['*']): array */ protected function cloneForPaginationCount(): static { - // A union's page bounds do not belong to its first SELECT. Removing that - // SELECT's own bounds would change the rows whose total we are counting. - return $this->cloneWithout($this->unions - ? ['unionOrders', 'unionLimit', 'unionOffset'] - : ['orders', 'limit', 'offset']) - ->cloneWithoutBindings([$this->unions ? 'unionOrder' : 'order']); + return $this->cloneWithout(['orders', 'limit', 'offset']) + ->cloneWithoutBindings(['order']); } /** @@ -4099,7 +3665,7 @@ protected function setAggregate(string $function, array $columns): static { $this->aggregate = ['function' => $function, 'columns' => $columns]; - if (! $this->shouldPreserveAggregateOrdering()) { + if (empty($this->groups)) { $this->orders = null; $this->bindings['order'] = []; @@ -4108,16 +3674,6 @@ protected function setAggregate(string $function, array $columns): static return $this; } - /** - * Determine whether ordering selects rows retained by the aggregate's inner query. - */ - protected function shouldPreserveAggregateOrdering(): bool - { - // An unbounded SELECT needs no sort for a count; a bounded union member - // needs its ordering to retain the same rows, particularly with UNION DISTINCT. - return (bool) $this->groups || ($this->unions && ($this->limit !== null || $this->offset !== null)); - } - /** * Execute the given callback while selecting the given columns. * @@ -4829,15 +4385,12 @@ protected function isQueryable(mixed $value): bool } /** - * Prepare the query for embedding in another statement. + * Ensure the query can be embedded in another statement. * * @throws InvalidArgumentException */ - protected function prepareQueryForEmbedding(self $query): void + protected function ensureCanEmbedQuery(self $query): void { - // Embedded SQL and bindings must reflect the same completed query callbacks. - $query->applyBeforeQueryCallbacks(); - if ($query->timeout !== null) { throw new InvalidArgumentException( 'An embedded query cannot define its own timeout. Apply the timeout to the outer query instead.' @@ -4858,24 +4411,11 @@ public function clone(): static */ public function cloneWithout(array $properties): static { - return tap($this->clone(), static fn (self $clone) => $clone->resetProperties($properties)); - } - - /** - * Clear query properties while retaining defaults required by non-nullable types. - */ - protected function resetProperties(array $properties): void - { - foreach ($properties as $property) { - $reflection = new ReflectionProperty($this, $property); - $this->{$property} = ($reflection->getType()?->allowsNull() ?? true) - ? null - : $reflection->getDefaultValue(); - } - - if (in_array('wheres', $properties, true)) { - $this->hasKeysetConditions = false; - } + return tap($this->clone(), function ($clone) use ($properties) { + foreach ($properties as $property) { + $clone->{$property} = is_array($clone->{$property}) ? [] : null; + } + }); } /** diff --git a/src/docs/database.md b/src/docs/database.md index 7549a46857..973204b0bd 100644 --- a/src/docs/database.md +++ b/src/docs/database.md @@ -528,9 +528,7 @@ Both execution methods construct database errors through the protected `newQuery Custom drivers used with database cache locks must identify unique-constraint violations through `isUniqueConstraintError` so lock contention can be handled without hiding other database failures. -Query builders with statement-level options may override the protected `Query\Builder::prepareQueryForEmbedding` method to reject options that belong on the outer statement. It runs when attaching a subquery, scalar or exists predicate, or union member; call the parent first to apply the embedded query's callbacks and preserve the built-in rejection of embedded timeouts. - -Builders with additional row-limiting SELECT clauses should override `requiresKeysetIsolation` and `isolateKeysetSelect`. The query builder prepares paging conditions after scopes and query callbacks have run, keeping them outside any SELECT whose clauses decide which rows survive. Keep custom SELECT clauses inside the derived table and statement-level options on the outer query. Cursor pagination, ID-based chunking, and direct ID-based page methods share this preparation, including when used through Eloquent. Override `shouldPreserveAggregateOrdering` when ordering determines which rows a custom clause retains for an aggregate. +Query builders with statement-level options may override the protected `Query\Builder::ensureCanEmbedQuery` method to reject options that belong on the outer statement. It runs when attaching a subquery, scalar or exists predicate, or union member; call the parent to preserve the built-in rejection of embedded timeouts. Custom Eloquent builders may override the public `ensureCanCreateOrFirst(): void` method to reject helpers that depend on unique-constraint recovery. Both `firstOrCreate` and `createOrFirst` call it before reading or writing, including their relationship forms; `updateOrCreate` and `incrementOrCreate` reach it through those helpers. Helpers delegate to each other, so validation may run more than once per operation; keep overrides side-effect-free. Relationships use the related model's builder, even when the parent uses another driver. The default method imposes no restriction, and ordinary `create`, `save`, and `firstOrNew` are unchanged. diff --git a/tests/Database/DatabaseQueryBuilderEmbeddingTest.php b/tests/Database/DatabaseQueryBuilderEmbeddingTest.php index 96e2b5f254..c346e2355f 100644 --- a/tests/Database/DatabaseQueryBuilderEmbeddingTest.php +++ b/tests/Database/DatabaseQueryBuilderEmbeddingTest.php @@ -92,11 +92,11 @@ class EmbeddingAwareQueryBuilder extends Builder public ?InvalidArgumentException $embeddingFailure = null; /** - * Extend embedding preparation while retaining callbacks and the timeout rule. + * Extend embedding validation while retaining the framework timeout rule. */ - protected function prepareQueryForEmbedding(Builder $query): void + protected function ensureCanEmbedQuery(Builder $query): void { - parent::prepareQueryForEmbedding($query); + parent::ensureCanEmbedQuery($query); $this->validatedQueries[] = $query; diff --git a/tests/Database/DatabaseQueryBuilderPaginationTest.php b/tests/Database/DatabaseQueryBuilderPaginationTest.php index fbaedae06e..cab6c59c89 100644 --- a/tests/Database/DatabaseQueryBuilderPaginationTest.php +++ b/tests/Database/DatabaseQueryBuilderPaginationTest.php @@ -6,9 +6,6 @@ use Hypervel\Database\BinaryParameter; use Hypervel\Database\Connection; -use Hypervel\Database\ConnectionResolverInterface; -use Hypervel\Database\Eloquent\Builder as EloquentBuilder; -use Hypervel\Database\Eloquent\Model; use Hypervel\Database\Query\Builder; use Hypervel\Database\Query\Grammars\Grammar; use Hypervel\Database\Query\Grammars\MySqlGrammar; @@ -23,89 +20,6 @@ class DatabaseQueryBuilderPaginationTest extends TestCase { - #[DataProvider('keysetIterators')] - public function testKeysetIterationVisitsEveryUnionRowOnce(string $method, bool $lazy, bool $descending): void - { - $connection = new SQLiteConnection(new PDO('sqlite::memory:')); - $connection->unprepared('create table posts (id integer)'); - $connection->unprepared('create table archives (archive_id integer)'); - $connection->unprepared('insert into posts values (1), (2), (3)'); - $connection->unprepared('insert into archives values (4), (5), (6)'); - $member = $connection->table('archives')->select('archive_id as value'); - $query = $connection->table('posts')->selectRaw('id as "value"')->unionAll($member); - $actual = []; - - if ($lazy) { - $actual = $query->{$method}(2, 'id', 'value')->take(8)->pluck('value')->all(); - } else { - $this->assertTrue($query->{$method}(2, function (Collection $rows) use (&$actual): bool { - array_push($actual, ...$rows->pluck('value')->all()); - - return count($actual) < 8; - }, 'id', 'value')); - } - - $this->assertSame($descending ? [6, 5, 4, 3, 2, 1] : [1, 2, 3, 4, 5, 6], $actual); - $this->assertSame([], $member->wheres); - $this->assertSame([], $query->getBindings()); - } - - /** - * Exercise both shared iteration paths and traversal directions. - */ - public static function keysetIterators(): iterable - { - yield 'chunks ascending' => ['chunkById', false, false]; - yield 'chunks descending' => ['chunkByIdDesc', false, true]; - yield 'lazy ascending' => ['lazyById', true, false]; - yield 'lazy descending' => ['lazyByIdDesc', true, true]; - } - - public function testDirectKeysetPagesPreserveScopesLimitsAndCurrentBindings(): void - { - $connection = new SQLiteConnection(new PDO('sqlite::memory:')); - $connection->unprepared('create table posts (id integer)'); - $connection->unprepared('create table archives (id integer)'); - $connection->unprepared('insert into posts values (1), (2), (3)'); - $connection->unprepared('insert into archives values (4), (5), (6)'); - $model = new class extends Model { - protected ?string $table = 'posts'; - }; - $model::setConnectionResolver(m::mock(ConnectionResolverInterface::class, ['connection' => $connection])); - $query = (new EloquentBuilder($connection->query()))->setModel($model) - ->withGlobalScope('visible', static fn (EloquentBuilder $query): EloquentBuilder => $query->where('posts.id', '<', 3)) - ->select('id')->orderByDesc('id')->limit(2) - ->unionAll($connection->table('archives')->select('id'))->orderBy('id'); - - $scoped = (clone $query)->forPageAfterId(2, 1)->limit(1); - $this->assertSame([2], $scoped->get()->modelKeys()); - $unscoped = (clone $query)->forPageAfterId(2, 2)->withoutGlobalScope('visible')->limit(1); - $this->assertSame([3], $unscoped->get()->modelKeys()); - $this->assertSame(2, (clone $query)->withoutGlobalScopes()->forPageAfterId(2, 2)->count()); - $this->assertSame(4, (clone $query)->withoutGlobalScopes()->forPageAfterId(10, 2)->count()); - $this->assertSame(4, (clone $query)->withoutGlobalScopes()->forPageAfterId(2, 2) - ->getQuery()->groupBy('id')->getCountForPagination()); - - $base = (clone $query)->withoutGlobalScopes()->getQuery(); - $this->assertSame($base, $base->forPageAfterId(3, 1)); - $this->assertSame($base, $base->forPageBeforeId(3, 5)); - $base->setBindings([2, 5], 'where'); - $this->assertSame([4, 3], $base->get()->pluck('id')->all()); - $sql = $base->toSql(); - $bindings = $base->getBindings(); - $this->assertSame($sql, $base->toSql()); - $this->assertSame($bindings, $base->getBindings()); - - $base->forPageAfterId(2, 3); - $this->assertSame([4], $base->get()->pluck('id')->all()); - $this->assertSame([2, 5, 3], $base->getRawBindings()['union']); - - $reused = $connection->table('posts')->forPageAfterId(1, 2); - $this->assertSame([3], $reused->get()->pluck('id')->all()); - $reused->unionAll($connection->table('posts')->where('id', 1))->forPageAfterId(2, 0); - $this->assertSame([1, 3], $reused->get()->pluck('id')->all()); - } - public function testBinaryStringsCanBePagedUsingAnExplicitCursorAlias(): void { $connection = new SQLiteConnection(new PDO('sqlite::memory:')); diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index 96f1cc2249..a3f9717f7a 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -3296,35 +3296,6 @@ public function testGetCountForPaginationWithUnionLimitAndOffset(): void $this->assertEquals(1, $count); } - public function testGetCountForPaginationWithGroupedUnionLimitAndOffset(): void - { - $builder = $this->getBuilder(); - $builder->from('posts')->select('category')->groupBy('category')->unionAll($this->getBuilder()->from('videos')->select('category'))->limit(15)->offset(1); - - $builder->getConnection()->expects('select')->with('select count(*) as "aggregate" from ((select "category" from "posts" group by "category") union all (select "category" from "videos")) as "aggregate_table"', [], true, [])->andReturn([['aggregate' => 1]]); - $builder->getProcessor()->expects('processSelect')->andReturnUsing(function ($builder, $results) { - return $results; - }); - - $count = $builder->getCountForPagination(); - $this->assertEquals(1, $count); - } - - public function testGetCountForPaginationWithGroupedUnionOrders(): void - { - $builder = $this->getBuilder(); - $builder->from('posts')->select('category')->groupBy('category')->unionAll($this->getBuilder()->from('videos')->select('category'))->orderBy('category')->orderByRaw('field(category, ?)', ['news']); - - $builder->getConnection()->expects('select')->with('select count(*) as "aggregate" from ((select "category" from "posts" group by "category") union all (select "category" from "videos")) as "aggregate_table"', [], true, [])->andReturn([['aggregate' => 1]]); - $builder->getProcessor()->expects('processSelect')->andReturnUsing(function ($builder, $results) { - return $results; - }); - - $count = $builder->getCountForPagination(); - $this->assertEquals(1, $count); - $this->assertSame(['news'], $builder->getRawBindings()['unionOrder']); - } - public function testGroupLimitCompilationWithAndWithoutOffset(): void { $builder = $this->getSQLiteBuilder() @@ -3605,27 +3576,6 @@ public function testUnionRejectsItsOwnTimeoutBeforeEmbeddingIt(): void ); } - public function testEmbeddedQueryRejectsTimeoutAddedByCallback(): void - { - $outer = $this->getMySqlBuilder()->from('users')->where('tenant_id', 7); - $sql = $outer->toSql(); - $embedded = $this->getMySqlBuilder()->from('members')->select('id') - ->beforeQuery(fn (Builder $query) => $query->timeout(2)); - - try { - $outer->whereIn('id', $embedded); - $this->fail('Expected the embedded query timeout to be rejected.'); - } catch (InvalidArgumentException $exception) { - $this->assertSame( - 'An embedded query cannot define its own timeout. Apply the timeout to the outer query instead.', - $exception->getMessage() - ); - } - - $this->assertSame($sql, $outer->toSql()); - $this->assertSame([7], $outer->getBindings()); - } - public function testExplainRejectsQueryTimeoutWithoutMutatingTheBuilder(): void { $builder = $this->getMySqlBuilder()->from('users')->where('id', 1)->timeout(2); @@ -6218,41 +6168,6 @@ public function testPreservedAreAppliedByExists(): void $builder->exists(); } - #[DataProvider('embeddedQueryCallbacksProvider')] - public function testEmbeddedQueryCallbacksRunBeforeBindingsAreCaptured(string $kind, string $sql): void - { - $calls = 0; - $embedded = $this->getBuilder()->select('id')->from('members')->beforeQuery(function (Builder $query) use (&$calls): void { - ++$calls; - $query->where('active', 1); - }); - $outer = $this->getBuilder()->select('id')->from('users')->where('tenant_id', 7); - - match ($kind) { - 'union' => $outer->unionAll($embedded), - 'exists' => $outer->whereExists($embedded), - 'scalar' => $outer->where('id', '=', $embedded), - 'parsed' => $outer->whereIn('id', $embedded), - }; - - $this->assertSame($sql, $outer->toSql()); - $this->assertSame([7, 1], $outer->getBindings()); - $this->assertSame(1, $calls); - } - - /** - * Provide the supported query embedding forms. - */ - public static function embeddedQueryCallbacksProvider(): array - { - return [ - ['union', '(select "id" from "users" where "tenant_id" = ?) union all (select "id" from "members" where "active" = ?)'], - ['exists', 'select "id" from "users" where "tenant_id" = ? and exists (select "id" from "members" where "active" = ?)'], - ['scalar', 'select "id" from "users" where "tenant_id" = ? and "id" = (select "id" from "members" where "active" = ?)'], - ['parsed', 'select "id" from "users" where "tenant_id" = ? and "id" in (select "id" from "members" where "active" = ?)'], - ]; - } - public function testPostgresInsertGetId(): void { $builder = $this->getPostgresBuilder(); @@ -6907,7 +6822,7 @@ public function testCustomBindingSlotsSurviveFactoriesAndPaginationCloning(): vo $clone = (new ReflectionMethod($builder, 'cloneForPaginationCount'))->invoke($builder); $this->assertInstanceOf(DatabaseQueryBuilderWithCustomBindings::class, $clone); $this->assertSame(['tenant', true], $clone->getBindings()); - $this->assertNull($clone->orders); + $this->assertSame([], $clone->orders); $this->assertNull($clone->limit); $this->assertNull($clone->offset); @@ -7610,28 +7525,6 @@ public function testCursorPaginateWrapsOrderColumnsContainingParentheses(): void $builder->cursorPaginate($perPage, $columns, $cursorName, $cursor); } - public function testCustomKeysetIsolationKeepsSelectOrderingAndPageOrderingSeparate(): void - { - $connection = $this->getConnection(); - $builder = new class($connection, new Grammar($connection), new Processor) extends Builder { - /** - * Require paging outside a custom SELECT clause. - */ - protected function requiresKeysetIsolation(bool $limitSelectsRows): bool - { - return true; - } - }; - $builder->from('events')->orderBy('id')->orderByRaw('id + ?', [7]) - ->forPageBeforeId(2, 5); - - $this->assertSame( - 'select * from (select * from "events" order by "id" asc, id + ?) as "cursor_table" where "id" < ? order by "id" desc limit 2', - $builder->toSql() - ); - $this->assertSame([7, 5], $builder->getBindings()); - } - public function testCursorPaginateWithDefaultArguments(): void { $perPage = 15; @@ -7932,72 +7825,6 @@ public function testCursorPaginateWithUnionWheres(): void ]), $result); } - public function testCursorPaginateFiltersNestedUnionsAndRebuildsTheirBindings(): void - { - $builder = $this->getMockQueryBuilder(['get']); - $builder->select('id')->from('videos')->where('active', 1) - ->unionAll($this->getBuilder()->select('id')->from('news')->where('active', 2) - ->unionAll($this->getBuilder()->select('id')->from('archives')->where('active', 3))) - ->orderBy('id'); - - $builder->expects('get')->andReturnUsing(function () use ($builder): Collection { - $this->assertSame( - '(select "id" from "videos" where "active" = ? and ("id" > ?)) union all ((select "id" from "news" where "active" = ? and ("id" > ?)) union all (select "id" from "archives" where "active" = ? and ("id" > ?))) order by "id" asc limit 11', - $builder->toSql() - ); - $this->assertSame([1, 2, 2, 2, 3, 2], $builder->getBindings()); - - return new Collection; - }); - - $builder->cursorPaginate(10, cursor: new Cursor(['id' => 2])); - } - - public function testCursorPaginateWithUnionSelectAndJoinBindings(): void - { - $timestamp = now()->toDateTimeString(); - - $perPage = 16; - $columns = ['test']; - $cursorName = 'cursor-name'; - $cursor = new Cursor(['created_at' => $timestamp]); - $builder = $this->getMockQueryBuilder(['get']); - $builder->select('id', 'created_at')->selectRaw('? as type', ['video'])->from('videos'); - $builder->union($this->getBuilder()->select('id', 'created_at')->selectRaw('? as type', ['news'])->from('news')->join('authors', function (JoinClause $join): void { - $join->on('authors.id', '=', 'news.author_id')->where('authors.active', true); - })); - $builder->orderBy('created_at'); - - $path = 'http://foo.bar?cursor=' . $cursor->encode(); - - $results = collect([ - ['id' => 1, 'created_at' => now(), 'type' => 'video'], - ['id' => 2, 'created_at' => now(), 'type' => 'news'], - ]); - - $builder->expects('get')->andReturnUsing(function () use ($builder, $results, $timestamp): Collection { - $this->assertSame( - '(select "id", "created_at", ? as type from "videos" where ("created_at" > ?)) union (select "id", "created_at", ? as type from "news" inner join "authors" on "authors"."id" = "news"."author_id" and "authors"."active" = ? where ("created_at" > ?)) order by "created_at" asc limit 17', - $builder->toSql() - ); - $this->assertEquals(['video', $timestamp, 'news', true, $timestamp], $builder->getBindings()); - - return $results; - }); - - Paginator::currentPathResolver(function () use ($path): string { - return $path; - }); - - $result = $builder->cursorPaginate($perPage, $columns, $cursorName, $cursor); - - $this->assertEquals(new CursorPaginator($results, $perPage, $cursor, [ - 'path' => $path, - 'cursorName' => $cursorName, - 'parameters' => ['created_at'], - ]), $result); - } - public function testCursorPaginateWithMultipleUnionsAndMultipleWheres(): void { $ts = now()->toDateTimeString(); @@ -8069,7 +7896,7 @@ public function testCursorPaginateWithUnionMultipleWheresMultipleOrders(): void $builder->expects('get')->andReturnUsing(function () use ($builder, $results, $ts) { $this->assertSame( - '(select "id", "start_time" as "created_at", "type" from "videos" where "extra" = ? and ("id" > ? or ("id" = ? and ("start_time" < ? or ("start_time" = ? and ("type" > ?)))))) union (select "id", "created_at", "type" from "news" where "extra" = ? and ("id" > ? or ("id" = ? and ("created_at" < ? or ("created_at" = ? and ("type" > ?)))))) union (select "id", "created_at", "type" from "podcasts" where "extra" = ? and ("id" > ? or ("id" = ? and ("created_at" < ? or ("created_at" = ? and ("type" > ?)))))) order by "id" asc, "created_at" desc, "type" asc limit 17', + '(select "id", "start_time" as "created_at", "type" from "videos" where "extra" = ? and ("id" > ? or ("id" = ? and ("start_time" < ? or ("start_time" = ? and ("type" > ?)))))) union (select "id", "created_at", "type" from "news" where "extra" = ? and ("id" > ? or ("id" = ? and ("start_time" < ? or ("start_time" = ? and ("type" > ?)))))) union (select "id", "created_at", "type" from "podcasts" where "extra" = ? and ("id" > ? or ("id" = ? and ("start_time" < ? or ("start_time" = ? and ("type" > ?)))))) order by "id" asc, "created_at" desc, "type" asc limit 17', $builder->toSql() ); $this->assertEquals(['first', 1, 1, $ts, $ts, 'news'], $builder->bindings['where']); @@ -8269,26 +8096,6 @@ public function testCursorPaginateWithUnionWheresAndAliassedOrderColumns(): void ]), $result); } - public function testCursorPaginateWithUnionOrderExpressions(): void - { - $builder = $this->getMockQueryBuilder(['get']); - $builder->from('videos')->select('id')->selectRaw('(score * 2) as ranked'); - $builder->union($this->getBuilder()->from('news')->select('id')->selectRaw('(score * 3) as ranked')); - $builder->orderBy('ranked')->orderBy('id'); - - $builder->expects('get')->andReturnUsing(function () use ($builder): Collection { - $this->assertSame( - '(select "id", (score * 2) as ranked from "videos" where ((score * 2) > ? or ((score * 2) = ? and ("id" > ?)))) union (select "id", (score * 3) as ranked from "news" where ((score * 3) > ? or ((score * 3) = ? and ("id" > ?)))) order by "ranked" asc, "id" asc limit 17', - $builder->toSql() - ); - $this->assertSame([10, 10, 1, 10, 10, 1], $builder->getBindings()); - - return new Collection; - }); - - $builder->cursorPaginate(16, cursor: new Cursor(['ranked' => 10, 'id' => 1])); - } - public function testWhereExpression(): void { $builder = $this->getBuilder(); @@ -8874,30 +8681,6 @@ public function testCloneWithoutBindings(): void $this->assertSame([], $clone->getBindings()); } - public function testCloneWithoutClearsNullablePropertiesAndResetsNonNullableProperties(): void - { - $connection = $this->getConnection(); - $builder = new class($connection, new Grammar($connection), new Processor) extends Builder { - public ?int $limit = 10; - }; - $builder->from('users')->select('id')->distinct()->where('active', true)->groupLimit(2, 'team_id'); - $builder->aggregate = ['function' => 'count', 'columns' => ['*']]; - - $clone = $builder->cloneWithout(['columns', 'aggregate', 'groupLimit', 'limit', 'distinct', 'wheres']) - ->cloneWithoutBindings(['where']); - - $this->assertSame('select * from "users"', $clone->toSql()); - $this->assertSame([], $clone->getBindings()); - $this->assertNull($clone->limit); - $this->assertNull($clone->columns); - $this->assertNull($clone->aggregate); - $this->assertNull($clone->groupLimit); - $this->assertFalse($clone->distinct); - $this->assertSame([], $clone->wheres); - $this->assertSame(10, $builder->limit); - $this->assertSame(['id'], $builder->columns); - } - public function testWhereVectorSimilarToOnPostgres(): void { $builder = $this->getPostgresBuilder(); diff --git a/tests/Integration/Database/EloquentCursorPaginateTest.php b/tests/Integration/Database/EloquentCursorPaginateTest.php index f6a2967523..b884195aaf 100644 --- a/tests/Integration/Database/EloquentCursorPaginateTest.php +++ b/tests/Integration/Database/EloquentCursorPaginateTest.php @@ -5,19 +5,16 @@ namespace Hypervel\Tests\Integration\Database; use Hypervel\Database\BinaryParameter; -use Hypervel\Database\Eloquent\Builder; use Hypervel\Database\Eloquent\Casts\AsBinary; use Hypervel\Database\Eloquent\Model; use Hypervel\Database\Eloquent\Relations\BelongsToMany; use Hypervel\Database\Eloquent\Relations\HasManyThrough; use Hypervel\Database\Eloquent\Relations\Pivot; -use Hypervel\Database\Query\Builder as QueryBuilder; use Hypervel\Database\Query\Expression; use Hypervel\Database\Schema\Blueprint; use Hypervel\Pagination\Cursor; use Hypervel\Support\Facades\DB; use Hypervel\Support\Facades\Schema; -use PHPUnit\Framework\Attributes\DataProvider; class EloquentCursorPaginateTest extends DatabaseTestCase { @@ -91,118 +88,6 @@ public function testPaginationWithUnion(): void $this->assertSame(['user_id'], $result->getOptions()['parameters']); } - #[DataProvider('boundedUnionProvider')] - public function testPaginationPreservesBoundedUnionMembers(string $shape, array $firstIds, array $secondIds): void - { - for ($id = 1; $id <= 7; ++$id) { - TestPost::create(['id' => $id]); - } - - $calls = 0; - $query = function () use ($shape, &$calls): QueryBuilder { - $bounded = DB::table('test_posts')->where('id', '<', 7)->orderByDesc('id')->limit(3); - $ordinary = DB::table('test_posts')->select('id as identifier')->where('id', 7); - - $query = match ($shape) { - 'root' => $bounded->unionAll($ordinary), - 'member' => $ordinary->unionAll($bounded->select('id as identifier')), - 'group' => $ordinary->unionAll( - DB::table('test_posts')->select('id as identifier')->where('id', '<=', 3) - ->unionAll(DB::table('test_posts')->select('id as identifier')->whereBetween('id', [4, 6])) - ->orderByDesc('identifier')->offset(1)->limit(3) - ), - }; - - return $query->orderByDesc('identifier')->beforeQuery(function () use (&$calls): void { - ++$calls; - }); - }; - - $first = $query()->cursorPaginate(2, ['id as identifier']); - $this->assertSame($firstIds, $first->pluck('identifier')->all()); - $this->assertSame(1, $calls); - - $second = $query()->cursorPaginate(2, ['id as identifier'], cursor: $first->nextCursor()); - $this->assertSame($secondIds, $second->pluck('identifier')->all()); - $this->assertSame(2, $calls); - - $previous = $query()->cursorPaginate(2, ['id as identifier'], cursor: $second->previousCursor()); - $this->assertSame($firstIds, $previous->pluck('identifier')->all()); - $this->assertSame(3, $calls); - - $after = $query()->cursorPaginate(2, ['id as identifier'], cursor: new Cursor(['identifier' => $secondIds[1]])); - $this->assertSame([], $after->items()); - $this->assertSame(4, $calls); - } - - /** - * Provide bounded primary selects, members and complete union groups. - */ - public static function boundedUnionProvider(): array - { - return [ - ['root', [7, 6], [5, 4]], - ['member', [7, 6], [5, 4]], - ['group', [7, 5], [4, 3]], - ]; - } - - public function testBoundedEloquentUnionKeepsScopesAndEagerLoads(): void - { - TestUser::fillAndInsert([['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4]]); - TestPost::fillAndInsert([['user_id' => 1], ['user_id' => 3], ['user_id' => 4]]); - - $scopeCalls = 0; - $query = function () use (&$scopeCalls): Builder { - return TestUser::with('posts') - ->withGlobalScope('visible', function (Builder $query) use (&$scopeCalls): void { - ++$scopeCalls; - $query->where('test_users.id', '>=', 2); - }) - ->orderByDesc('id')->limit(2) - ->unionAll(TestUser::where('id', 1)) - ->orderByDesc('id'); - }; - - $first = $query()->cursorPaginate(2); - $this->assertSame([4, 3], $first->getCollection()->modelKeys()); - $this->assertSame(1, $scopeCalls); - - $second = $query()->cursorPaginate(2, cursor: $first->nextCursor()); - $this->assertSame([1], $second->getCollection()->modelKeys()); - $this->assertSame(2, $scopeCalls); - $this->assertTrue($second->items()[0]->relationLoaded('posts')); - $this->assertSame([1], $second->items()[0]->posts->pluck('user_id')->all()); - - $previous = $query()->cursorPaginate(2, cursor: $second->previousCursor()); - $this->assertSame([4, 3], $previous->getCollection()->modelKeys()); - $this->assertSame(3, $scopeCalls); - } - - public function testBoundedUnionMemberKeepsItsImplicitProjection(): void - { - TestPost::fillAndInsert([['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4]]); - $query = fn (): QueryBuilder => DB::table('test_posts')->where('id', '<=', 2)->orderByDesc('id')->limit(1) - ->unionAll(DB::table('test_posts', 'archive')->where('id', '>', 2)->orderByDesc('id')->limit(1)) - ->orderByDesc('id'); - - $first = $query()->cursorPaginate(1, ['test_posts.*']); - $this->assertSame([4], $first->pluck('id')->all()); - - $second = $query()->cursorPaginate(1, ['test_posts.*'], cursor: $first->nextCursor()); - $this->assertSame([2], $second->pluck('id')->all()); - } - - public function testOrdinaryCursorPaginationStillOwnsItsLimit(): void - { - TestPost::fillAndInsert([['id' => 1], ['id' => 2], ['id' => 3], ['id' => 4]]); - - $page = DB::table('test_posts')->orderBy('id')->limit(1) - ->cursorPaginate(3, cursor: new Cursor(['id' => 1])); - - $this->assertSame([2, 3, 4], $page->pluck('id')->all()); - } - public function testPaginationWithDistinct(): void { for ($i = 1; $i <= 3; ++$i) { diff --git a/tests/Integration/Database/QueryBuilderTest.php b/tests/Integration/Database/QueryBuilderTest.php index 1e6c7e0d50..a7c1be6307 100644 --- a/tests/Integration/Database/QueryBuilderTest.php +++ b/tests/Integration/Database/QueryBuilderTest.php @@ -658,15 +658,6 @@ public function testPaginateWithSpecificColumns(): void ]); } - public function testGetCountForPaginationWithGroupedUnionAndLimit(): void - { - $query = DB::table('posts')->select('title')->groupBy('title') - ->unionAll(DB::table('posts')->select('title')->where('title', 'Foo Post')) - ->limit(1); - - $this->assertSame(3, $query->getCountForPagination()); - } - public function testChunkMap(): void { DB::enableQueryLog(); From bd90ac5295bcc5f6cc9df0ca7672e4754314afc0 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 29 Oct 2026 10:32:50 +0000 Subject: [PATCH 6/7] Preserve cached cast edits when setters replace stored attributes Clearing every class-cast cache entry after a mutator loses edits made to the cached object inside that mutator. Keeping every entry instead allows an old object to overwrite replacement values. Track the values each cached object read or last wrote, and compare only its backing columns during writeback. Explicit replacements take precedence for their columns while pending object edits to other columns survive. Discard the obsolete object after merging the remaining writes. Keep the record on the model instance and clear it wherever the object cache is cleared; no additional caster calls are needed. Evaluate caster and Attribute callbacks before merging the current attributes so their other writes and encrypted-cast synchronizations are retained. Clear caches before synchronizing raw replacement attributes, preventing objects from the previous row from overwriting the replacement. Regression coverage includes both setter styles, encrypted and plain casts, held object references, partial backing-column replacements, reads and edits within setters, raw-row replacement, and callback side effects. Formatting, full static analysis, the Database suite and affected Eloquent integration tests pass. --- .../src/Eloquent/Concerns/HasAttributes.php | 95 +++++---- src/database/src/Eloquent/Model.php | 4 +- tests/Database/DatabaseEloquentModelTest.php | 180 +++++++++++++++++- 3 files changed, 232 insertions(+), 47 deletions(-) diff --git a/src/database/src/Eloquent/Concerns/HasAttributes.php b/src/database/src/Eloquent/Concerns/HasAttributes.php index b1d568fcca..5bf3078f0a 100644 --- a/src/database/src/Eloquent/Concerns/HasAttributes.php +++ b/src/database/src/Eloquent/Concerns/HasAttributes.php @@ -104,6 +104,13 @@ trait HasAttributes */ protected array $classCastCache = []; + /** + * The stored attributes each cached class cast was created from or last wrote. + * + * @var array> + */ + protected array $classCastStoredValues = []; + /** * The attributes that have been cast using "Attribute" return type mutators. */ @@ -903,9 +910,12 @@ protected function getClassCastableAttributeValue(string $key, mixed $value): mi if ($caster instanceof CastsInboundAttributes || ! is_object($value) || $objectCachingDisabled) { - unset($this->classCastCache[$key]); + unset($this->classCastCache[$key], $this->classCastStoredValues[$key]); } else { $this->classCastCache[$key] = $value; + // The first writeback identifies the cast's backing columns. Until then, + // retain the values it read without invoking its setter a second time. + $this->classCastStoredValues[$key] = $this->attributes; } return $value; @@ -1153,8 +1163,9 @@ protected function setMutatedAttributeValue(string $key, mixed $value): mixed $result = $this->{'set' . StrCache::studly($key) . 'Attribute'}($value); - // The setter may read and cache this attribute before replacing its stored value. - unset($this->classCastCache[$key], $this->attributeCastCache[$key]); + // A legacy setter can refresh stored data after reading a cached accessor. + // Class casts reconcile their own writes so in-place mutations survive. + unset($this->attributeCastCache[$key]); return $result; } @@ -1172,15 +1183,10 @@ protected function setAttributeMarkedMutatedAttributeValue(string $key, mixed $v $this->attributes[$key] = $value; }; - $this->attributes = array_merge( - $this->attributes, - $this->normalizeCastClassResponse( - $key, - $callback($value, $this->attributes) - ) - ); - - unset($this->classCastCache[$key]); + // Read attributes after the callback so its other writes and cast + // synchronizations are not replaced by an earlier argument snapshot. + $attributes = $this->normalizeCastClassResponse($key, $callback($value, $this->attributes)); + $this->attributes = array_merge($this->attributes, $attributes); // Setter-only inputs must not overwrite later changes to objects returned by class casts. if ($attribute->get && ($attribute->withCaching || (is_object($value) && $attribute->withObjectCaching))) { @@ -1223,7 +1229,7 @@ public function fillJsonAttribute(string $key, mixed $value): static }; if ($this->isClassCastable($key)) { - unset($this->classCastCache[$key]); + unset($this->classCastCache[$key], $this->classCastStoredValues[$key]); } unset($this->attributeCastCache[$key]); @@ -1238,15 +1244,9 @@ protected function setClassCastableAttribute(string $key, mixed $value): void { $caster = $this->resolveCasterClass($key); - $this->attributes = array_replace( - $this->attributes, - $this->normalizeCastClassResponse($key, $caster->set( - $this, - $key, - $value, - $this->attributes - )) - ); + // A caster can update other attributes while producing its stored values. + $attributes = $this->normalizeCastClassResponse($key, $caster->set($this, $key, $value, $this->attributes)); + $this->attributes = array_replace($this->attributes, $attributes); // A custom caster may read the accessor while preparing the new stored value. unset($this->attributeCastCache[$key]); @@ -1254,9 +1254,10 @@ protected function setClassCastableAttribute(string $key, mixed $value): void if ($caster instanceof CastsInboundAttributes || ! is_object($value) || ($caster->withoutObjectCaching ?? false)) { - unset($this->classCastCache[$key]); + unset($this->classCastCache[$key], $this->classCastStoredValues[$key]); } else { $this->classCastCache[$key] = $value; + $this->classCastStoredValues[$key] = $attributes; } } @@ -1899,12 +1900,29 @@ protected function mergeAttributeFromClassCasts(string $key): void $value = $this->classCastCache[$key]; $caster = $this->resolveCasterClass($key); - $this->attributes = array_merge( - $this->attributes, - $caster instanceof CastsInboundAttributes - ? [$key => $value] - : $this->normalizeCastClassResponse($key, $caster->set($this, $key, $value, $this->attributes)) - ); + $attributes = $caster instanceof CastsInboundAttributes + ? [$key => $value] + : $this->normalizeCastClassResponse($key, $caster->set($this, $key, $value, $this->attributes)); + + $replaced = false; + + // Replaced backing values take precedence over the cached object's edits. + // Keep its edits to other columns; unrelated attributes can change independently, + // including encrypted casts that are written back during reads. + foreach ($attributes as $column => $attribute) { + if (($this->attributes[$column] ?? null) !== ($this->classCastStoredValues[$key][$column] ?? null)) { + unset($attributes[$column]); + $replaced = true; + } + } + + $this->attributes = array_merge($this->attributes, $attributes); + + if ($replaced) { + unset($this->classCastCache[$key], $this->classCastStoredValues[$key]); + } else { + $this->classCastStoredValues[$key] = $attributes; + } } /** @@ -1937,13 +1955,9 @@ protected function mergeAttributeFromAttributeCasts(string $key): void $this->attributes[$key] = $value; }; - $this->attributes = array_merge( - $this->attributes, - $this->normalizeCastClassResponse( - $key, - $callback($value, $this->attributes) - ) - ); + // Preserve writes performed by the setter before merging its returned values. + $attributes = $this->normalizeCastClassResponse($key, $callback($value, $this->attributes)); + $this->attributes = array_merge($this->attributes, $attributes); } /** @@ -2035,13 +2049,15 @@ public function setRawAttributes(array $attributes, bool $sync = false): static { $this->attributes = $attributes; + // Synchronizing original values must not write back objects from the old row. + $this->classCastCache = []; + $this->classCastStoredValues = []; + $this->attributeCastCache = []; + if ($sync) { $this->syncOriginal(); } - $this->classCastCache = []; - $this->attributeCastCache = []; - return $this; } @@ -2212,6 +2228,7 @@ public function discardChanges(): static [$this->attributes, $this->changes, $this->previous] = [$this->original, [], []]; $this->classCastCache = []; + $this->classCastStoredValues = []; $this->attributeCastCache = []; return $this; diff --git a/src/database/src/Eloquent/Model.php b/src/database/src/Eloquent/Model.php index b5ae0d7c90..172573cc6d 100644 --- a/src/database/src/Eloquent/Model.php +++ b/src/database/src/Eloquent/Model.php @@ -3086,7 +3086,8 @@ public function offsetUnset($offset): void $this->attributes[$offset], $this->relations[$offset], $this->attributeCastCache[$offset], - $this->classCastCache[$offset] + $this->classCastCache[$offset], + $this->classCastStoredValues[$offset] ); } @@ -3173,6 +3174,7 @@ public function __sleep(): array $this->mergeAttributesFromCachedCasts(); $this->classCastCache = []; + $this->classCastStoredValues = []; $this->attributeCastCache = []; $this->flushCastCaches(); $this->relationAutoloadCallback = null; diff --git a/tests/Database/DatabaseEloquentModelTest.php b/tests/Database/DatabaseEloquentModelTest.php index fdaa846d7a..ceb0c70767 100755 --- a/tests/Database/DatabaseEloquentModelTest.php +++ b/tests/Database/DatabaseEloquentModelTest.php @@ -61,10 +61,12 @@ use Hypervel\Database\Query\Grammars\PostgresGrammar; use Hypervel\Database\Query\Processors\Processor; use Hypervel\Database\SQLiteConnection; +use Hypervel\Encryption\Encrypter as ConcreteEncrypter; use Hypervel\Events\Dispatcher as EventDispatcher; use Hypervel\Support\CarbonImmutable; use Hypervel\Support\ClassInvoker; use Hypervel\Support\Collection as BaseCollection; +use Hypervel\Support\Facades\Crypt; use Hypervel\Support\Fluent; use Hypervel\Support\HtmlString; use Hypervel\Support\InteractsWithTime; @@ -2010,10 +2012,14 @@ public function setSettingsAttribute(array $value): void $this->assertSame(['settings' => '{"theme":"dark","locale":"en"}'], $model->getDirty()); } - #[TestWith([false])] - #[TestWith([true])] - public function testMutatorsReplaceCachedClassCastValues(bool $legacy): void + #[TestWith([false, false])] + #[TestWith([true, false])] + #[TestWith([false, true])] + #[TestWith([true, true])] + public function testMutatorsReplaceCachedClassCastValues(bool $legacy, bool $encrypted): void { + Crypt::swap(new ConcreteEncrypter(str_repeat('a', 32), 'AES-256-CBC')); + if ($legacy) { $model = new class extends Model { protected array $casts = ['settings' => AsArrayObject::class]; @@ -2023,7 +2029,11 @@ public function testMutatorsReplaceCachedClassCastValues(bool $legacy): void */ public function setSettingsAttribute(array $value): void { - $this->attributes['settings'] = json_encode($value); + $this->attributes['settings'] = $this->getCasts()['settings'] === AsEncryptedArrayObject::class + ? Crypt::encryptString(json_encode($value)) + : json_encode($value); + $this->attributes['selected_theme'] = $this->settings['theme']; + $this->settings['locale'] = 'fr'; } }; } else { @@ -2035,19 +2045,126 @@ public function setSettingsAttribute(array $value): void */ protected function settings(): Attribute { - return Attribute::set(fn (array $value): string|false => json_encode($value)); + return Attribute::set(fn (array $value): string|false => $this->getCasts()['settings'] === AsEncryptedArrayObject::class + ? Crypt::encryptString(json_encode($value)) + : json_encode($value)); } }; } - $model->setRawAttributes(['settings' => '{"theme":"light"}'], true); + $model->mergeCasts(['settings' => $encrypted ? AsEncryptedArrayObject::class : AsArrayObject::class]); + $model->setRawAttributes(['settings' => $encrypted ? Crypt::encryptString('{"theme":"light"}') : '{"theme":"light"}'], true); $this->assertSame('light', $model->settings['theme']); $model->settings = ['theme' => 'dark']; $this->assertSame('dark', $model->settings['theme']); - $this->assertSame(['settings' => '{"theme":"dark"}'], $model->getDirty()); + if ($legacy) { + $this->assertSame('dark', $model->selected_theme); + $this->assertSame('fr', $model->settings['locale']); + } + + $stored = $model->getDirty()['settings']; + $this->assertSame( + $legacy ? '{"theme":"dark","locale":"fr"}' : '{"theme":"dark"}', + $encrypted ? Crypt::decryptString($stored) : $stored + ); + } + + #[TestWith([LegacySettingsMutationModel::class, false, false])] + #[TestWith([LegacySettingsMutationModel::class, true, false])] + #[TestWith([LegacySettingsMutationModel::class, false, true])] + #[TestWith([LegacySettingsMutationModel::class, true, true])] + #[TestWith([AttributeSettingsMutationModel::class, false, false])] + #[TestWith([AttributeSettingsMutationModel::class, true, false])] + #[TestWith([AttributeSettingsMutationModel::class, false, true])] + #[TestWith([AttributeSettingsMutationModel::class, true, true])] + public function testMutatorsKeepInPlaceClassCastEdits(string $modelClass, bool $encrypted, bool $readFirst): void + { + Crypt::swap(new ConcreteEncrypter(str_repeat('a', 32), 'AES-256-CBC')); + $model = new $modelClass; + $model->mergeCasts(['settings' => $encrypted ? AsEncryptedArrayObject::class : AsArrayObject::class]); + $model->setRawAttributes(['settings' => $encrypted ? Crypt::encryptString('{"theme":"light"}') : '{"theme":"light"}'], true); + $held = $readFirst ? $model->settings : null; + + $model->settings = ['theme' => 'dark']; + + $this->assertSame('dark', $model->settings['theme']); + $this->assertSame(7, $model->settings_updated_by); + if ($readFirst) { + $this->assertSame($held, $model->settings); + $held['locale'] = 'fr'; + } + + $stored = $model->getDirty()['settings']; + $this->assertSame( + $readFirst ? '{"theme":"dark","locale":"fr"}' : '{"theme":"dark"}', + $encrypted ? Crypt::decryptString($stored) : $stored + ); + } + + public function testReplacingOneBackingColumnKeepsOtherClassCastEdits(): void + { + $model = (new ModelStub)->mergeCasts(['address' => Address::class]); + $model->setRawAttributes(['address_line_one' => 'First', 'address_line_two' => 'Second'], true); + $address = $model->address; + $address->lineTwo = 'Unsaved'; + $model->address_line_one = 'Replacement'; + + $this->assertSame('Replacement', $model->address->lineOne); + $this->assertSame('Unsaved', $model->address->lineTwo); + $this->assertSame(['address_line_one' => 'Replacement', 'address_line_two' => 'Unsaved'], $model->getDirty()); + } + + public function testRawAttributeReplacementDoesNotWriteBackCachedObjects(): void + { + $model = (new ModelStub)->mergeCasts(['settings' => AsArrayObject::class]); + $model->setRawAttributes(['settings' => '{"theme":"light"}'], true); + $model->settings['theme'] = 'unsaved'; + + $model->setRawAttributes(['settings' => '{"theme":"dark"}'], true); + + $this->assertSame('dark', $model->settings['theme']); + $this->assertSame('{"theme":"dark"}', $model->getRawOriginal('settings')); + $this->assertSame([], $model->getDirty()); + } + + public function testCasterWritesToOtherAttributesSurviveAssignmentAndWriteback(): void + { + $model = (new ModelStub)->mergeCasts(['settings' => AuditedSettingsCast::class]); + $settings = new Fluent(['theme' => 'dark']); + $model->settings = $settings; + + $this->assertSame('dark', $model->settings_theme); + $settings->theme = 'light'; + $this->assertSame(['settings_theme' => 'light', 'settings' => '{"theme":"light"}'], $model->getAttributes()); + } + + public function testAttributeSetterWritesToOtherAttributesSurviveAssignmentAndWriteback(): void + { + $model = new class extends Model { + /** + * Get and set the settings attribute. + */ + protected function settings(): Attribute + { + return Attribute::make( + get: fn (string $value): Fluent => new Fluent(json_decode($value, true)), + set: function (Fluent $value): string|false { + $this->attributes['settings_theme'] = $value->theme; + + return json_encode($value); + }, + ); + } + }; + $settings = new Fluent(['theme' => 'dark']); + $model->settings = $settings; + + $this->assertSame('dark', $model->settings_theme); + $settings->theme = 'light'; + $this->assertSame(['settings_theme' => 'light', 'settings' => '{"theme":"light"}'], $model->getAttributes()); } public function testSetOnlyMutatorDoesNotOverwriteChangesToClassCastObjects(): void @@ -5631,6 +5748,55 @@ protected function serializeDate(DateTimeInterface $date): string } } +class LegacySettingsMutationModel extends Model +{ + /** + * Update the existing settings and record their author. + */ + public function setSettingsAttribute(array $value): void + { + $this->settings['theme'] = $value['theme']; + $this->attributes['settings_updated_by'] = 7; + } +} + +class AttributeSettingsMutationModel extends Model +{ + /** + * Update the existing settings and record their author. + */ + protected function settings(): Attribute + { + return Attribute::set(function (array $value): array { + $this->settings['theme'] = $value['theme']; + $this->attributes['settings_updated_by'] = 7; + + return []; + }); + } +} + +class AuditedSettingsCast implements CastsAttributes +{ + /** + * Return the stored settings. + */ + public function get(Model $model, string $key, mixed $value, array $attributes): Fluent + { + return new Fluent(json_decode($value, true)); + } + + /** + * Store the settings and update their indexed theme. + */ + public function set(Model $model, string $key, mixed $value, array $attributes): string|false + { + $model->settings_theme = $value->theme; + + return json_encode($value); + } +} + class MergingSettingsCast implements CastsInboundAttributes { /** From 2bff3de9ce78d3507ddb33ea19471cf51a12639d Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 29 Oct 2026 11:02:09 +0000 Subject: [PATCH 7/7] Preserve raw writes and partial responses in cached accessors A cached Attribute value could overwrite a raw column updated by another setter. Apply the same backing-value comparison used for class casts through one shared writeback helper. Explicit replacements win while pending edits to other columns survive. Validate cached null and get-only values against their own raw input without invoking a setter or inferring dependencies on other attributes. Preserve normal same-key invalidation and caching flags, and clear the new per-model records with their cache entries. Keep baseline values for columns omitted by partial setter responses, including when an object is first assigned. Update only entries whose values changed so unchanged reads do not copy the shared snapshot. Regression coverage includes sibling setter writes, cached nulls, paired multi-column accessors and partial responses after reads and assignments. Formatting, full static analysis, affected database suites and focused model and casting checks pass. --- .../src/Eloquent/Concerns/HasAttributes.php | 92 +++++++++---- src/database/src/Eloquent/Model.php | 2 + tests/Database/DatabaseEloquentModelTest.php | 127 +++++++++++++++++- 3 files changed, 191 insertions(+), 30 deletions(-) diff --git a/src/database/src/Eloquent/Concerns/HasAttributes.php b/src/database/src/Eloquent/Concerns/HasAttributes.php index 5bf3078f0a..97a67be3de 100644 --- a/src/database/src/Eloquent/Concerns/HasAttributes.php +++ b/src/database/src/Eloquent/Concerns/HasAttributes.php @@ -116,6 +116,13 @@ trait HasAttributes */ protected array $attributeCastCache = []; + /** + * The stored attributes each cached accessor was created from or last wrote. + * + * @var array> + */ + protected array $attributeCastStoredValues = []; + /** * The cached merged casts array for this instance. */ @@ -738,8 +745,9 @@ protected function mutateAttributeMarkedAttribute(string $key, mixed $value): mi if ($attribute->withCaching || (is_object($value) && $attribute->withObjectCaching)) { $this->attributeCastCache[$key] = $value; + $this->attributeCastStoredValues[$key] = $this->attributes; } else { - unset($this->attributeCastCache[$key]); + unset($this->attributeCastCache[$key], $this->attributeCastStoredValues[$key]); } return $value; @@ -1118,7 +1126,7 @@ public function setAttribute(string|int $key, mixed $value): mixed $this->attributes[$key] = $value; - unset($this->attributeCastCache[$key]); + unset($this->attributeCastCache[$key], $this->attributeCastStoredValues[$key]); return $this; } @@ -1165,7 +1173,7 @@ protected function setMutatedAttributeValue(string $key, mixed $value): mixed // A legacy setter can refresh stored data after reading a cached accessor. // Class casts reconcile their own writes so in-place mutations survive. - unset($this->attributeCastCache[$key]); + unset($this->attributeCastCache[$key], $this->attributeCastStoredValues[$key]); return $result; } @@ -1191,8 +1199,9 @@ protected function setAttributeMarkedMutatedAttributeValue(string $key, mixed $v // Setter-only inputs must not overwrite later changes to objects returned by class casts. if ($attribute->get && ($attribute->withCaching || (is_object($value) && $attribute->withObjectCaching))) { $this->attributeCastCache[$key] = $value; + $this->attributeCastStoredValues[$key] = $this->attributes; } else { - unset($this->attributeCastCache[$key]); + unset($this->attributeCastCache[$key], $this->attributeCastStoredValues[$key]); } return $this; @@ -1232,7 +1241,7 @@ public function fillJsonAttribute(string $key, mixed $value): static unset($this->classCastCache[$key], $this->classCastStoredValues[$key]); } - unset($this->attributeCastCache[$key]); + unset($this->attributeCastCache[$key], $this->attributeCastStoredValues[$key]); return $this; } @@ -1249,7 +1258,7 @@ protected function setClassCastableAttribute(string $key, mixed $value): void $this->attributes = array_replace($this->attributes, $attributes); // A custom caster may read the accessor while preparing the new stored value. - unset($this->attributeCastCache[$key]); + unset($this->attributeCastCache[$key], $this->attributeCastStoredValues[$key]); if ($caster instanceof CastsInboundAttributes || ! is_object($value) @@ -1257,7 +1266,7 @@ protected function setClassCastableAttribute(string $key, mixed $value): void unset($this->classCastCache[$key], $this->classCastStoredValues[$key]); } else { $this->classCastCache[$key] = $value; - $this->classCastStoredValues[$key] = $attributes; + $this->classCastStoredValues[$key] = $this->attributes; } } @@ -1281,7 +1290,7 @@ protected function setEnumCastableAttribute(string $key, mixed $value): void ); } - unset($this->attributeCastCache[$key]); + unset($this->attributeCastCache[$key], $this->attributeCastStoredValues[$key]); } /** @@ -1904,24 +1913,8 @@ protected function mergeAttributeFromClassCasts(string $key): void ? [$key => $value] : $this->normalizeCastClassResponse($key, $caster->set($this, $key, $value, $this->attributes)); - $replaced = false; - - // Replaced backing values take precedence over the cached object's edits. - // Keep its edits to other columns; unrelated attributes can change independently, - // including encrypted casts that are written back during reads. - foreach ($attributes as $column => $attribute) { - if (($this->attributes[$column] ?? null) !== ($this->classCastStoredValues[$key][$column] ?? null)) { - unset($attributes[$column]); - $replaced = true; - } - } - - $this->attributes = array_merge($this->attributes, $attributes); - - if ($replaced) { + if ($this->mergeCachedCastAttributes($attributes, $this->classCastStoredValues[$key])) { unset($this->classCastCache[$key], $this->classCastStoredValues[$key]); - } else { - $this->classCastStoredValues[$key] = $attributes; } } @@ -1940,14 +1933,20 @@ protected function mergeAttributesFromAttributeCasts(): void */ protected function mergeAttributeFromAttributeCasts(string $key): void { - if (! isset($this->attributeCastCache[$key])) { + if (! array_key_exists($key, $this->attributeCastCache)) { return; } $value = $this->attributeCastCache[$key]; - $attribute = $this->{StrCache::camel($key)}(); + $attribute = $value === null ? null : $this->{StrCache::camel($key)}(); + + // Cached nulls and get-only accessors have no writeback. Refresh them when + // their raw input changes without invoking a setter or guessing dependencies. + if ($attribute === null || ($attribute->get && ! $attribute->set)) { + if (($this->attributes[$key] ?? null) !== ($this->attributeCastStoredValues[$key][$key] ?? null)) { + unset($this->attributeCastCache[$key], $this->attributeCastStoredValues[$key]); + } - if ($attribute->get && ! $attribute->set) { return; } @@ -1957,7 +1956,42 @@ protected function mergeAttributeFromAttributeCasts(string $key): void // Preserve writes performed by the setter before merging its returned values. $attributes = $this->normalizeCastClassResponse($key, $callback($value, $this->attributes)); + + if ($this->mergeCachedCastAttributes($attributes, $this->attributeCastStoredValues[$key])) { + unset($this->attributeCastCache[$key], $this->attributeCastStoredValues[$key]); + } + } + + /** + * Merge cached cast values, advance their baseline, and indicate whether stored replacements took precedence. + */ + protected function mergeCachedCastAttributes(array $attributes, array &$storedValues): bool + { + $replaced = false; + + // Replaced backing values take precedence over cached edits. Keep edits + // to other columns; unrelated attributes can change independently, + // including encrypted casts that are written back during reads. + foreach ($attributes as $column => $attribute) { + if (($this->attributes[$column] ?? null) !== ($storedValues[$column] ?? null)) { + unset($attributes[$column]); + $replaced = true; + } + } + $this->attributes = array_merge($this->attributes, $attributes); + + if (! $replaced) { + // Partial responses retain omitted columns. Skip unchanged values so + // read-only writeback does not copy the shared attribute snapshot. + foreach ($attributes as $column => $attribute) { + if (($storedValues[$column] ?? null) !== $attribute) { + $storedValues[$column] = $attribute; + } + } + } + + return $replaced; } /** @@ -2053,6 +2087,7 @@ public function setRawAttributes(array $attributes, bool $sync = false): static $this->classCastCache = []; $this->classCastStoredValues = []; $this->attributeCastCache = []; + $this->attributeCastStoredValues = []; if ($sync) { $this->syncOriginal(); @@ -2230,6 +2265,7 @@ public function discardChanges(): static $this->classCastCache = []; $this->classCastStoredValues = []; $this->attributeCastCache = []; + $this->attributeCastStoredValues = []; return $this; } diff --git a/src/database/src/Eloquent/Model.php b/src/database/src/Eloquent/Model.php index 172573cc6d..8ebf1d37b5 100644 --- a/src/database/src/Eloquent/Model.php +++ b/src/database/src/Eloquent/Model.php @@ -3086,6 +3086,7 @@ public function offsetUnset($offset): void $this->attributes[$offset], $this->relations[$offset], $this->attributeCastCache[$offset], + $this->attributeCastStoredValues[$offset], $this->classCastCache[$offset], $this->classCastStoredValues[$offset] ); @@ -3176,6 +3177,7 @@ public function __sleep(): array $this->classCastCache = []; $this->classCastStoredValues = []; $this->attributeCastCache = []; + $this->attributeCastStoredValues = []; $this->flushCastCaches(); $this->relationAutoloadCallback = null; $this->relationAutoloadContext = null; diff --git a/tests/Database/DatabaseEloquentModelTest.php b/tests/Database/DatabaseEloquentModelTest.php index ceb0c70767..f9394dfc4e 100755 --- a/tests/Database/DatabaseEloquentModelTest.php +++ b/tests/Database/DatabaseEloquentModelTest.php @@ -2104,9 +2104,25 @@ public function testMutatorsKeepInPlaceClassCastEdits(string $modelClass, bool $ ); } - public function testReplacingOneBackingColumnKeepsOtherClassCastEdits(): void + #[TestWith([false])] + #[TestWith([true])] + public function testReplacingOneBackingColumnKeepsOtherCastEdits(bool $attribute): void { - $model = (new ModelStub)->mergeCasts(['address' => Address::class]); + $model = $attribute ? new class extends Model { + /** + * Get and set the address columns. + */ + protected function address(): Attribute + { + return Attribute::make( + get: static fn (mixed $value, array $attributes): Address => new Address($attributes['address_line_one'], $attributes['address_line_two']), + set: static fn (Address $value): array => [ + 'address_line_one' => $value->lineOne, + 'address_line_two' => $value->lineTwo, + ], + ); + } + } : (new ModelStub)->mergeCasts(['address' => Address::class]); $model->setRawAttributes(['address_line_one' => 'First', 'address_line_two' => 'Second'], true); $address = $model->address; $address->lineTwo = 'Unsaved'; @@ -2117,6 +2133,91 @@ public function testReplacingOneBackingColumnKeepsOtherClassCastEdits(): void $this->assertSame(['address_line_one' => 'Replacement', 'address_line_two' => 'Unsaved'], $model->getDirty()); } + #[TestWith([false, false])] + #[TestWith([false, true])] + #[TestWith([true, false])] + #[TestWith([true, true])] + public function testPartialCastWritebackKeepsTheBaselineForLaterEdits(bool $attribute, bool $assign): void + { + $model = $attribute ? new class extends Model { + /** + * Get the address and write only its changed columns. + */ + protected function address(): Attribute + { + return Attribute::make( + get: static fn (mixed $value, array $attributes): Address => new Address($attributes['address_line_one'], $attributes['address_line_two']), + set: static fn (Address $value, array $attributes): array => array_diff_assoc([ + 'address_line_one' => $value->lineOne, + 'address_line_two' => $value->lineTwo, + ], $attributes), + ); + } + } : (new ModelStub)->mergeCasts(['address' => PartialAddressCast::class]); + $model->setRawAttributes(['address_line_one' => 'First', 'address_line_two' => 'Second'], true); + if ($assign) { + $model->address = new Address('First', 'Second'); + } + $address = $model->address; + + $this->assertSame([], $model->getDirty()); + + $address->lineOne = 'Changed'; + $this->assertSame(['address_line_one' => 'Changed'], $model->getDirty()); + + $address->lineTwo = 'Later'; + $this->assertSame(['address_line_one' => 'Changed', 'address_line_two' => 'Later'], $model->getDirty()); + $this->assertSame($address, $model->address); + } + + #[TestWith([false, 'old'])] + #[TestWith([true, 'old'])] + #[TestWith([false, null])] + #[TestWith([true, null])] + public function testCachedAccessorsRefreshAfterAnotherSetterWritesTheirRawValue(bool $getOnly, ?string $initial): void + { + $model = new class extends Model { + public bool $getOnly = false; + + public int $setterCalls = 0; + + /** + * Get the slug, optionally allowing explicit updates. + */ + protected function slug(): Attribute + { + return Attribute::make( + get: static fn (?string $value): ?string => $value === null ? null : '/' . $value, + set: $this->getOnly ? null : function (string $value): string { + ++$this->setterCalls; + + return ltrim($value, '/'); + }, + )->shouldCache(); + } + + /** + * Set the name and its stored slug. + */ + public function setNameAttribute(string $value): void + { + $this->attributes['name'] = $value; + $this->attributes['slug'] = strtolower($value); + } + }; + $model->getOnly = $getOnly; + $model->setRawAttributes(['name' => 'Old', 'slug' => $initial], true); + $this->assertSame($initial === null ? null : '/old', $model->slug); + + $model->name = 'New'; + + if ($getOnly || $initial === null) { + $this->assertSame(0, $model->setterCalls); + } + $this->assertSame('/new', $model->slug); + $this->assertSame(['name' => 'New', 'slug' => 'new'], $model->getDirty()); + } + public function testRawAttributeReplacementDoesNotWriteBackCachedObjects(): void { $model = (new ModelStub)->mergeCasts(['settings' => AsArrayObject::class]); @@ -5776,6 +5877,28 @@ protected function settings(): Attribute } } +class PartialAddressCast implements CastsAttributes +{ + /** + * Return the address from its backing columns. + */ + public function get(Model $model, string $key, mixed $value, array $attributes): Address + { + return new Address($attributes['address_line_one'], $attributes['address_line_two']); + } + + /** + * Return only the address columns changed by this write. + */ + public function set(Model $model, string $key, mixed $value, array $attributes): array + { + return array_diff_assoc([ + 'address_line_one' => $value->lineOne, + 'address_line_two' => $value->lineTwo, + ], $attributes); + } +} + class AuditedSettingsCast implements CastsAttributes { /**