From 1e294d9b9c8bba3573b7d2dd77281b356a6c6e84 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 22 Oct 2026 07:55:09 +0000 Subject: [PATCH 01/29] Clarify comments for upstream API and behavior differences Allow concise explanations beside intentional public API and behavior differences that a future upstream sync could revert or overwrite. Keep internal implementation comments governed by the existing usefulness rule and avoid annotations for mechanical porting changes. Remove the blanket prohibition on divergence comments and clarify that the three-location documentation requirement applies to intentionally omitted features. Rename that rule to reflect its scope without changing the README or porting-guide criteria. --- AGENTS.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c839ea1db..d98eeb61f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -194,7 +194,8 @@ Build complete, long-term solutions, not MVPs or local workarounds. A broad chan - **Use Laravel observer conventions** — place Eloquent observers in a top-level `Observers/` directory. Register model-specific observers with `#[ObservedBy(...)]`; use `observe()` only for dynamic registration or observers supplied automatically by a reusable concern. - **Use attributed local scopes** — define local Eloquent query scopes as protected methods marked with `#[Scope]`, rather than legacy public `scopeFoo()` methods. Use separate scope classes only for genuine global scopes. - **No class docblocks unless warranted** — only add a class-level docblock if something unusual or complex needs explanation: purpose, architectural role, usage patterns. Never write one that inventories the class — trait lists, method summaries, "registers X, configures Y" — that duplicates the members' own docblocks and goes stale. Method docblocks (title only, Laravel-style, imperative mood: "Return", not "Returns") are always added, except on test methods, whether identified by a test name prefix or PHPUnit’s `#[Test]` attribute. A body can accompany the title for complex methods that need further explanation. -- **Add comments only where they're genuinely useful** (required Laravel-style method title docblocks and config section comment blocks still apply) — a short WHY for logic that isn't obvious from reading the code, the reason behind a bug fix, or logic that's coupled to code in other files and hard to understand in isolation. Avoid unnecessary code comments; don't comment what the code does, and don't annotate framework divergences, routine casts, or type normalizations. When reconciling upstream changes, add missing explanations for non-obvious Hypervel behavior being retained, so future syncs can understand why it exists. +- **Add comments only where they're genuinely useful** (required Laravel-style method title docblocks and config section comment blocks still apply) — a short WHY for logic that isn't obvious from reading the code, the reason behind a bug fix, or logic that's coupled to code in other files and hard to understand in isolation. Avoid unnecessary code comments; don't comment what the code does, and don't annotate routine casts or type normalizations. When reconciling upstream changes, add missing explanations for non-obvious Hypervel behavior being retained, so future syncs can understand why it exists. +- **Explain intentional public API and behavior differences from upstream** — Where a future sync could revert or overwrite an intentional difference, add a concise comment beside the relevant code explaining why it differs. Do not annotate mechanical porting changes. Comments on internal implementation differences follow the general commenting rule. - **Don't make classes final by default** — keep classes open; add `final` only when it protects a real invariant or avoids a concrete framework/API problem, e.g. immutability, coroutine-safety, or a security guarantee. - **Place methods logically, not at the end** — group new methods with related ones (getters with getters, setters with setters). Two exceptions: preserve upstream order when merging ported code (see Porting rules), and `flushState()` has its own placement rule (see Static state and test cleanup). - **Only extract methods when justified** — extract only when the logic is complex enough to benefit from a name, it's likely to be reused, or two or more methods call it. Don't extract a simple one-liner with a single caller. @@ -847,7 +848,7 @@ When ported code adds a provider or listener, wire providers and aliases in both Keep everything else: behavioral descriptions, `@see` links, `@throws` annotations, warnings, contract explanations, usage notes. Modernize the title line to imperative form ("Returns" → "Return", "Retrieves" → "Retrieve") but do not remove or rewrite the body content beneath it. Translate non-English comments to English and fix grammar errors. -- **Record intentional Laravel differences where future ports will look** — When a Laravel feature is intentionally not ported because it does not fit Hypervel's Swoole/coroutine architecture, or because Hypervel has a better native equivalent, record it in three places so a future port cannot miss it: (1) the package README under `Differences From Laravel`, following the Package READMEs rules above and explaining what to use instead; (2) a concise source comment at the natural insertion point where the skipped method/class would otherwise sit; (3) a concise `REMOVED:` comment at the matching upstream test location when tests are skipped. This is a narrow exception to the "don't annotate divergences" rule: it applies only to intentionally omitted methods or features, never to ordinary ported-and-adapted code. Closed decisions only — real gaps still worth doing go in `docs/todo.md`. +- **Record intentionally omitted upstream features where future syncs will look** — When a Laravel feature is intentionally not ported because it does not fit Hypervel's Swoole/coroutine architecture, or because Hypervel has a better native equivalent, record it in three places so a future port cannot miss it: (1) the package README under `Differences From Laravel`, following the Package READMEs rules above and explaining what to use instead; (2) a concise source comment at the natural insertion point where the skipped method/class would otherwise sit; (3) a concise `REMOVED:` comment at the matching upstream test location when tests are skipped. This three-location requirement applies only to intentionally omitted methods or features, not ordinary ported-and-adapted code. Closed decisions only — real gaps still worth doing go in `docs/todo.md`. - **Replace framework names in code** — any occurrence of the word `laravel` or `hyperf` in ported code (string literals, comments, prefixes, identifiers, etc.) must be replaced with `hypervel`, preserving the original casing. For example: `laravel_reserved_` → `hypervel_reserved_`, `LaravelExcelExporter` → `HypervelExcelExporter`, `HYPERF_VERSION` → `HYPERVEL_VERSION`. This does not apply to namespaces (which have their own conversion rules) or to references that describe the upstream source (e.g., docblock `@see` links to Laravel/Hyperf source). - **Don't copy Laravel/Hyperf-specific framework details just to stay 1:1** — preserve public APIs and behavior while translating upstream-specific packages, providers, bootstrap mechanisms, and architecture to Hypervel equivalents. If no equivalent exists, reach consensus on the appropriate adaptation through a second-opinion loop with your peer agent before implementing. From ab73bca543800e21872d9ab523ab6aaa2aa5fa35 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 22 Oct 2026 08:58:39 +0000 Subject: [PATCH 02/29] Expose connection-owned skip-locked capability detection Add supportsSkipLocked to the concrete connection API without widening the driver-neutral interface. Reuse connection-local memoization and reset it when PDO resources are replaced or detached. Preserve configured server versions, MariaDB detection through both drivers, and MySQL, PostgreSQL and Vitess version thresholds. Have Database Queue consume the shared capability while preserving its protected getLockForPopping signature and invocation. Remove superseded internal connection helpers and regenerate the DB facade metadata. Retain existing queue caching coverage and extend capability tests for configured PostgreSQL versions and detached resources. Database and Queue suites and static analysis pass. --- src/database/src/Connection.php | 8 ++++ src/database/src/MySqlConnection.php | 12 ++--- src/database/src/PdoConnection.php | 22 ++++----- src/database/src/PostgresConnection.php | 8 ++-- src/queue/src/DatabaseQueue.php | 5 +- src/support/src/Facades/DB.php | 1 + tests/Database/DatabasePdoConnectionTest.php | 50 ++++++++++++-------- tests/Queue/QueueDatabaseQueueUnitTest.php | 9 ++-- 8 files changed, 63 insertions(+), 52 deletions(-) diff --git a/src/database/src/Connection.php b/src/database/src/Connection.php index 83d295716..56325c6a0 100755 --- a/src/database/src/Connection.php +++ b/src/database/src/Connection.php @@ -1837,6 +1837,14 @@ public function withoutTablePrefix(Closure $callback): mixed */ abstract public function getServerVersion(): string; + /** + * Determine whether the server supports skipping locked rows. + */ + public function supportsSkipLocked(): bool + { + return false; + } + /** * Register a connection resolver. * diff --git a/src/database/src/MySqlConnection.php b/src/database/src/MySqlConnection.php index a02d958c1..56ff11b86 100755 --- a/src/database/src/MySqlConnection.php +++ b/src/database/src/MySqlConnection.php @@ -185,25 +185,21 @@ public function getServerVersion(): string } /** - * Resolve the lock clause supported when popping queued jobs. + * Resolve whether the server supports skipping locked rows. */ - protected function resolveLockForPopping(): bool|string + protected function resolveSupportsSkipLocked(): bool { $version = (string) ($this->getConfig('version') ?? $this->getServerVersion()); $isMaria = str_contains($version, 'MariaDB') || $this->isMaria(); $version = $this->normalizeServerVersion($version); if (Str::contains($version, ['vitess', 'PlanetScale'])) { - return version_compare(Str::before($version, '-'), '19.0', '>=') - ? 'FOR UPDATE SKIP LOCKED' - : true; + return version_compare(Str::before($version, '-'), '19.0', '>='); } $minimumVersion = $isMaria ? '10.6.0' : '8.0.1'; - return version_compare($version, $minimumVersion, '>=') - ? 'FOR UPDATE SKIP LOCKED' - : true; + return version_compare($version, $minimumVersion, '>='); } /** diff --git a/src/database/src/PdoConnection.php b/src/database/src/PdoConnection.php index 39a20c926..d7991e299 100755 --- a/src/database/src/PdoConnection.php +++ b/src/database/src/PdoConnection.php @@ -42,9 +42,9 @@ class PdoConnection extends Connection protected int $fetchMode = PDO::FETCH_OBJ; /** - * The lock clause supported by the current physical connection generation. + * Whether the current physical connection generation supports skipping locked rows. */ - protected bool|string|null $lockForPopping = null; + protected ?bool $supportsSkipLocked = null; /** * The maximum number of statement bindings supported by the current physical connection generation. @@ -476,7 +476,7 @@ public function detachPdoResources(Closure $pdo, ?Closure $readPdo): void $this->pdo = $pdo; $this->readPdo = $readPdo; - $this->lockForPopping = null; + $this->supportsSkipLocked = null; $this->maxBindings = null; } @@ -692,7 +692,7 @@ protected function hasDriverResources(): bool public function setPdo(PDO|Closure|null $pdo): static { $this->transactions = 0; - $this->lockForPopping = null; + $this->supportsSkipLocked = null; $this->maxBindings = null; $this->pdo = $pdo; @@ -952,21 +952,19 @@ public function getServerVersion(): string } /** - * Get the lock clause supported when popping queued jobs. - * - * @internal + * Determine whether the server supports skipping locked rows. */ - public function lockForPopping(): bool|string + public function supportsSkipLocked(): bool { - return $this->lockForPopping ??= $this->resolveLockForPopping(); + return $this->supportsSkipLocked ??= $this->resolveSupportsSkipLocked(); } /** - * Resolve the lock clause supported when popping queued jobs. + * Resolve whether the server supports skipping locked rows. */ - protected function resolveLockForPopping(): bool|string + protected function resolveSupportsSkipLocked(): bool { - return true; + return false; } /** diff --git a/src/database/src/PostgresConnection.php b/src/database/src/PostgresConnection.php index 4858d670b..f4eef2c6f 100755 --- a/src/database/src/PostgresConnection.php +++ b/src/database/src/PostgresConnection.php @@ -140,15 +140,13 @@ protected function parseUniqueConstraintViolation(Exception $exception): array } /** - * Resolve the lock clause supported when popping queued jobs. + * Resolve whether the server supports skipping locked rows. */ - protected function resolveLockForPopping(): bool|string + protected function resolveSupportsSkipLocked(): bool { $version = (string) ($this->getConfig('version') ?? $this->getServerVersion()); - return version_compare($version, '9.5', '>=') - ? 'FOR UPDATE SKIP LOCKED' - : true; + return version_compare($version, '9.5', '>='); } /** diff --git a/src/queue/src/DatabaseQueue.php b/src/queue/src/DatabaseQueue.php index ae7b78121..fb3afb157 100644 --- a/src/queue/src/DatabaseQueue.php +++ b/src/queue/src/DatabaseQueue.php @@ -9,6 +9,7 @@ use Hypervel\Contracts\Queue\ClearableQueue; use Hypervel\Contracts\Queue\Job; use Hypervel\Contracts\Queue\Queue as QueueContract; +use Hypervel\Database\Connection; use Hypervel\Database\ConnectionInterface; use Hypervel\Database\ConnectionResolverInterface; use Hypervel\Database\DatabaseTransactionsManager; @@ -562,8 +563,8 @@ protected function getLockForPopping(): bool|string { $connection = $this->getDatabase(); - return $connection instanceof PdoConnection - ? $connection->lockForPopping() + return $connection instanceof Connection && $connection->supportsSkipLocked() + ? 'FOR UPDATE SKIP LOCKED' : true; } diff --git a/src/support/src/Facades/DB.php b/src/support/src/Facades/DB.php index b5ac8b6a1..5b4078afa 100644 --- a/src/support/src/Facades/DB.php +++ b/src/support/src/Facades/DB.php @@ -109,6 +109,7 @@ * @method static \Hypervel\Database\Connection setTablePrefix(string $prefix) * @method static \Hypervel\Database\Connection setTransactionManager(\Hypervel\Database\DatabaseTransactionsManager $manager) * @method static bool statement(string $query, array $bindings = []) + * @method static bool supportsSkipLocked() * @method static \Hypervel\Database\Query\Builder table(\Closure|\Hypervel\Database\Query\Builder|\Hypervel\Database\Eloquent\Builder<*>|\Hypervel\Database\Eloquent\Relations\Relation<*, *, *>|\Hypervel\Contracts\Database\Query\Expression|\UnitEnum|string $table, string|null $as = null) * @method static int|null threadCount() * @method static float totalQueryDuration() diff --git a/tests/Database/DatabasePdoConnectionTest.php b/tests/Database/DatabasePdoConnectionTest.php index 79c68e17b..b72303f03 100755 --- a/tests/Database/DatabasePdoConnectionTest.php +++ b/tests/Database/DatabasePdoConnectionTest.php @@ -480,7 +480,7 @@ public function testDatabaseCapabilities( string $connectionClass, string $serverVersion, ?string $configuredVersion, - bool|string $expectedLock, + bool $supportsSkipLocked, int $expectedMaxBindings, ): void { $pdo = new VersionedPDOStub($serverVersion); @@ -493,27 +493,28 @@ public function testDatabaseCapabilities( /** @var class-string $connectionClass */ $connection = new $connectionClass($pdo, 'test_db', '', $config); - $this->assertSame($expectedLock, $connection->lockForPopping()); + $this->assertSame($supportsSkipLocked, $connection->supportsSkipLocked()); $this->assertSame($expectedMaxBindings, $connection->maxBindings()); } public static function databaseCapabilityProvider(): array { return [ - 'unknown PDO connection' => [PdoConnection::class, '1.0', null, true, 999], - 'MySQL before SKIP LOCKED' => [MySqlConnection::class, '8.0.0', null, true, 65_535], - 'MySQL with SKIP LOCKED' => [MySqlConnection::class, '8.0.1', null, 'FOR UPDATE SKIP LOCKED', 65_535], - 'MariaDB through MySQL before SKIP LOCKED' => [MySqlConnection::class, '5.5.5-10.5.99-MariaDB', null, true, 65_535], - 'MariaDB through MySQL with SKIP LOCKED' => [MySqlConnection::class, '5.5.5-10.6.0-MariaDB', null, 'FOR UPDATE SKIP LOCKED', 65_535], - 'MariaDB connection' => [MariaDbConnection::class, '5.5.5-10.6.0-MariaDB', null, 'FOR UPDATE SKIP LOCKED', 65_535], - 'configured MySQL override' => [MySqlConnection::class, '8.0.0', '8.0.1', 'FOR UPDATE SKIP LOCKED', 65_535], - 'configured MariaDB marker' => [MySqlConnection::class, '8.0.0', '5.5.5-10.6.0-MariaDB', 'FOR UPDATE SKIP LOCKED', 65_535], - 'Vitess before SKIP LOCKED' => [MySqlConnection::class, '8.0.0', '18.0.0-vitess', true, 65_535], - 'Vitess with SKIP LOCKED' => [MySqlConnection::class, '8.0.0', '19.0.0-vitess', 'FOR UPDATE SKIP LOCKED', 65_535], - 'PlanetScale with SKIP LOCKED' => [MySqlConnection::class, '8.0.0', '19.0.0-PlanetScale', 'FOR UPDATE SKIP LOCKED', 65_535], - 'PostgreSQL before SKIP LOCKED' => [PostgresConnection::class, '9.4', null, true, 65_535], - 'PostgreSQL with SKIP LOCKED' => [PostgresConnection::class, '9.5', null, 'FOR UPDATE SKIP LOCKED', 65_535], - 'SQLite' => [SQLiteConnection::class, '', null, true, 32_766], + 'unknown PDO connection' => [PdoConnection::class, '1.0', null, false, 999], + 'MySQL before SKIP LOCKED' => [MySqlConnection::class, '8.0.0', null, false, 65_535], + 'MySQL with SKIP LOCKED' => [MySqlConnection::class, '8.0.1', null, true, 65_535], + 'MariaDB through MySQL before SKIP LOCKED' => [MySqlConnection::class, '5.5.5-10.5.99-MariaDB', null, false, 65_535], + 'MariaDB through MySQL with SKIP LOCKED' => [MySqlConnection::class, '5.5.5-10.6.0-MariaDB', null, true, 65_535], + 'MariaDB connection' => [MariaDbConnection::class, '5.5.5-10.6.0-MariaDB', null, true, 65_535], + 'configured MySQL override' => [MySqlConnection::class, '8.0.0', '8.0.1', true, 65_535], + 'configured MariaDB marker' => [MySqlConnection::class, '8.0.0', '5.5.5-10.6.0-MariaDB', true, 65_535], + 'Vitess before SKIP LOCKED' => [MySqlConnection::class, '8.0.0', '18.0.0-vitess', false, 65_535], + 'Vitess with SKIP LOCKED' => [MySqlConnection::class, '8.0.0', '19.0.0-vitess', true, 65_535], + 'PlanetScale with SKIP LOCKED' => [MySqlConnection::class, '8.0.0', '19.0.0-PlanetScale', true, 65_535], + 'PostgreSQL before SKIP LOCKED' => [PostgresConnection::class, '9.4', null, false, 65_535], + 'PostgreSQL with SKIP LOCKED' => [PostgresConnection::class, '9.5', null, true, 65_535], + 'configured PostgreSQL override' => [PostgresConnection::class, '9.4', '9.5', true, 65_535], + 'SQLite' => [SQLiteConnection::class, '', null, false, 32_766], ]; } @@ -524,9 +525,9 @@ public function testDatabaseCapabilitiesAreCachedUntilThePdoIsReplaced(): void public int $bindingResolutionCount = 0; - protected function resolveLockForPopping(): bool|string + protected function resolveSupportsSkipLocked(): bool { - return ++$this->lockResolutionCount === 1 ? true : 'FOR UPDATE SKIP LOCKED'; + return ++$this->lockResolutionCount > 1; } protected function resolveMaxBindings(): int @@ -535,8 +536,8 @@ protected function resolveMaxBindings(): int } }; - $this->assertTrue($connection->lockForPopping()); - $this->assertTrue($connection->lockForPopping()); + $this->assertFalse($connection->supportsSkipLocked()); + $this->assertFalse($connection->supportsSkipLocked()); $this->assertSame(999, $connection->maxBindings()); $this->assertSame(999, $connection->maxBindings()); $this->assertSame(1, $connection->lockResolutionCount); @@ -544,10 +545,17 @@ protected function resolveMaxBindings(): int $connection->setPdo(new PDOStub); - $this->assertSame('FOR UPDATE SKIP LOCKED', $connection->lockForPopping()); + $this->assertTrue($connection->supportsSkipLocked()); $this->assertSame(65_535, $connection->maxBindings()); $this->assertSame(2, $connection->lockResolutionCount); $this->assertSame(2, $connection->bindingResolutionCount); + + $connection->detachPdoResources(fn () => new PDOStub, null); + + $this->assertTrue($connection->supportsSkipLocked()); + $this->assertSame(65_535, $connection->maxBindings()); + $this->assertSame(3, $connection->lockResolutionCount); + $this->assertSame(3, $connection->bindingResolutionCount); } public function testEscapingUsesThePdoThatExecutedTheLastWrite(): void diff --git a/tests/Queue/QueueDatabaseQueueUnitTest.php b/tests/Queue/QueueDatabaseQueueUnitTest.php index d25f1ece1..3a2a2bea4 100644 --- a/tests/Queue/QueueDatabaseQueueUnitTest.php +++ b/tests/Queue/QueueDatabaseQueueUnitTest.php @@ -14,6 +14,7 @@ use Hypervel\Contracts\Events\Dispatcher as DispatcherContract; use Hypervel\Contracts\Queue\ShouldQueue; use Hypervel\Contracts\Queue\ShouldQueueAfterCommit; +use Hypervel\Database\Connection; use Hypervel\Database\ConnectionInterface; use Hypervel\Database\ConnectionResolverInterface; use Hypervel\Database\DatabaseTransactionsManager; @@ -89,13 +90,13 @@ public function testQueueNamesPreserveZeroAndDefaultEmptyString(): void $this->assertSame('Emails', $queue->getQueue(UnitQueueName::Emails)); } - public function testLockForPoppingUsesThePdoConnectionCapability(): void + public function testLockForPoppingUsesTheConnectionCapability(): void { $resolver = m::mock(ConnectionResolverInterface::class); - $connection = m::mock(PdoConnection::class); + $connection = m::mock(Connection::class); $resolver->shouldReceive('connection')->once()->with(null)->andReturn($connection); - $connection->shouldReceive('lockForPopping')->once()->andReturn('FOR UPDATE SKIP LOCKED'); + $connection->shouldReceive('supportsSkipLocked')->once()->andReturn(true); $queue = new TestDatabaseQueue( resolver: $resolver, @@ -108,7 +109,7 @@ public function testLockForPoppingUsesThePdoConnectionCapability(): void $this->assertSame('FOR UPDATE SKIP LOCKED', $queue->lockForPopping()); } - public function testLockForPoppingUsesAConservativeFallbackForNonPdoConnections(): void + public function testLockForPoppingUsesAConservativeFallbackForInterfaceOnlyConnections(): void { $resolver = m::mock(ConnectionResolverInterface::class); $resolver->shouldReceive('connection')->once()->with(null)->andReturn(m::mock(ConnectionInterface::class)); From a7cedcf3f41b1dcc112bc2ba9540db753d5e9c96 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 22 Oct 2026 08:58:39 +0000 Subject: [PATCH 03/29] Add fluent skip-locked query support Introduce skipLocked after lockForUpdate or sharedLock without changing either existing method signature. Keep lock intent on the builder and compile the appropriate update or shared syntax for MySQL, MariaDB and PostgreSQL. Unsupported servers retain their normal lock behavior. Reject combinations with custom string lock clauses in either setter order and document capability checks and queue-like claiming semantics. Keep custom string locks unchanged when the modifier is not used. Cover supported and fallback grammars, fluent behavior and invalid combinations. Verify real two-connection contention through parallel testing on MySQL, MariaDB, PostgreSQL and MariaDB using the MySQL driver. Formatting and static analysis pass. --- src/database/src/Query/Builder.php | 23 +++++ .../src/Query/Grammars/MySqlGrammar.php | 6 ++ .../src/Query/Grammars/PostgresGrammar.php | 6 +- src/docs/queries.md | 18 ++++ tests/Database/DatabaseQueryBuilderTest.php | 59 +++++++++++++ tests/Integration/Database/SkipLockedTest.php | 86 +++++++++++++++++++ 6 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 tests/Integration/Database/SkipLockedTest.php diff --git a/src/database/src/Query/Builder.php b/src/database/src/Query/Builder.php index 0c2662ee0..85aa9aadd 100644 --- a/src/database/src/Query/Builder.php +++ b/src/database/src/Query/Builder.php @@ -195,6 +195,11 @@ class Builder implements BuilderContract */ public string|bool|null $lock = null; + /** + * Whether to skip rows locked by another transaction. + */ + public bool $skipLocked = false; + /** * The query execution timeout in seconds. */ @@ -2860,6 +2865,10 @@ public function unionAll(Closure|self|EloquentBuilder $query): static */ public function lock(string|bool $value = true): static { + if ($this->skipLocked && is_string($value)) { + throw new InvalidArgumentException('Skip locked requires lockForUpdate() or sharedLock(), not a custom lock clause.'); + } + $this->lock = $value; $this->useWritePdo(); @@ -2882,6 +2891,20 @@ public function sharedLock(): static return $this->lock(false); } + /** + * Skip rows locked by another transaction when supported by the server. + */ + public function skipLocked(): static + { + if (! is_bool($this->lock)) { + throw new InvalidArgumentException('Skip locked requires lockForUpdate() or sharedLock(), not a custom lock clause.'); + } + + $this->skipLocked = true; + + return $this; + } + /** * Set a query execution timeout in seconds. * diff --git a/src/database/src/Query/Grammars/MySqlGrammar.php b/src/database/src/Query/Grammars/MySqlGrammar.php index a58f1de33..495e0b498 100755 --- a/src/database/src/Query/Grammars/MySqlGrammar.php +++ b/src/database/src/Query/Grammars/MySqlGrammar.php @@ -327,6 +327,12 @@ protected function compileUnionOffset(Builder $query, int $offset): string protected function compileLock(Builder $query, bool|string $value): string { if (! is_string($value)) { + if ($query->skipLocked && $this->connection->supportsSkipLocked()) { + // MySQL's legacy shared-lock syntax cannot take SKIP LOCKED; MariaDB requires it. + return ($value ? 'for update' : ($this->connection->isMaria() ? 'lock in share mode' : 'for share')) + . ' skip locked'; + } + return $value ? 'for update' : 'lock in share mode'; } diff --git a/src/database/src/Query/Grammars/PostgresGrammar.php b/src/database/src/Query/Grammars/PostgresGrammar.php index 92871e7bd..6120e656e 100755 --- a/src/database/src/Query/Grammars/PostgresGrammar.php +++ b/src/database/src/Query/Grammars/PostgresGrammar.php @@ -319,7 +319,11 @@ protected function compileHavingBitwise(array $having): string protected function compileLock(Builder $query, bool|string $value): string { if (! is_string($value)) { - return $value ? 'for update' : 'for share'; + $lock = $value ? 'for update' : 'for share'; + + return $query->skipLocked && $this->connection->supportsSkipLocked() + ? $lock . ' skip locked' + : $lock; } return $value; diff --git a/src/docs/queries.md b/src/docs/queries.md index d6aa2a407..5d5c481c3 100644 --- a/src/docs/queries.md +++ b/src/docs/queries.md @@ -1865,6 +1865,24 @@ DB::transaction(function () { }); ``` +When several workers read from a queue-like table, you may call `skipLocked` after `lockForUpdate` or `sharedLock` to skip rows locked by another transaction: + +```php +DB::transaction(function () { + $jobs = DB::table('jobs') + ->where('status', 'pending') + ->orderBy('id') + ->limit(100) + ->lockForUpdate() + ->skipLocked() + ->get(); + + // Process or claim the selected jobs within this transaction. +}); +``` + +Skipping locked rows does not give a complete view of the matching data, so use it for work claiming rather than general-purpose reads. The modifier cannot be combined with a custom string lock clause. On unsupported database versions, the query retains its normal locking behavior; SQLite does not acquire row locks. If your operation requires skipping locked rows, check `DB::supportsSkipLocked()` before choosing this approach. + ## Reusable Query Components diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index 247cc3ef2..a0955e450 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -6748,6 +6748,65 @@ public function testPostgresLock(): void $this->assertEquals(['baz'], $builder->getBindings()); } + #[DataProvider('skipLockedGrammarProvider')] + public function testSkipLockedUsesTheServerLockSyntax( + string $factory, + bool $supported, + ?bool $isMaria, + string $updateSql, + string $sharedSql, + ): void { + $builder = $this->{$factory}()->from('jobs'); + $connection = $builder->getConnection(); + $connection->shouldReceive('supportsSkipLocked')->andReturn($supported); + + if ($isMaria !== null) { + $connection->shouldReceive('isMaria')->andReturn($isMaria); + } + + $builder->lockForUpdate(); + $this->assertSame($builder, $builder->skipLocked()); + $this->assertSame($updateSql, $builder->toSql()); + $this->assertSame($sharedSql, $builder->sharedLock()->toSql()); + } + + public static function skipLockedGrammarProvider(): array + { + return [ + 'MySQL' => ['getMySqlBuilder', true, false, 'select * from `jobs` for update skip locked', 'select * from `jobs` for share skip locked'], + 'MariaDB through MySQL' => ['getMySqlBuilder', true, true, 'select * from `jobs` for update skip locked', 'select * from `jobs` lock in share mode skip locked'], + 'older MySQL' => ['getMySqlBuilder', false, null, 'select * from `jobs` for update', 'select * from `jobs` lock in share mode'], + 'PostgreSQL' => ['getPostgresBuilder', true, null, 'select * from "jobs" for update skip locked', 'select * from "jobs" for share skip locked'], + 'older PostgreSQL' => ['getPostgresBuilder', false, null, 'select * from "jobs" for update', 'select * from "jobs" for share'], + 'SQLite' => ['getSQLiteBuilder', false, null, 'select * from "jobs"', 'select * from "jobs"'], + 'base grammar' => ['getBuilder', false, null, 'select * from "jobs"', 'select * from "jobs"'], + ]; + } + + public function testSkipLockedRequiresAnExistingLock(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Skip locked requires lockForUpdate() or sharedLock(), not a custom lock clause.'); + + $this->getBuilder()->skipLocked(); + } + + public function testSkipLockedCannotFollowACustomLock(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Skip locked requires lockForUpdate() or sharedLock(), not a custom lock clause.'); + + $this->getBuilder()->lock('for update nowait')->skipLocked(); + } + + public function testCustomLockCannotFollowSkipLocked(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Skip locked requires lockForUpdate() or sharedLock(), not a custom lock clause.'); + + $this->getBuilder()->lockForUpdate()->skipLocked()->lock('for update nowait'); + } + public function testSelectWithLockUsesWritePdo(): void { $builder = $this->getMySqlBuilderWithProcessor(); diff --git a/tests/Integration/Database/SkipLockedTest.php b/tests/Integration/Database/SkipLockedTest.php new file mode 100644 index 000000000..d9894f268 --- /dev/null +++ b/tests/Integration/Database/SkipLockedTest.php @@ -0,0 +1,86 @@ +make('config'); + $default = $config->string('database.default'); + $connection = $config->array("database.connections.{$default}"); + $connection['lock_timeout'] = 1; + $connection['pool'] = [ + 'testing_enabled' => true, + 'min_retained_connections' => 1, + 'max_connections' => 1, + 'heartbeat_interval' => null, + ]; + + $config->set('database.connections.skip_locked', $connection); + } + + public function testBothLockModesSkipRowsHeldByAnotherConnection(): void + { + if (! DB::connection()->supportsSkipLocked()) { + $this->markTestSkipped('This server does not support skipping locked rows.'); + } + + $holderPool = new DatabasePool($this->app, 'skip_locked'); + $contenderPool = new DatabasePool($this->app, 'skip_locked'); + $holderLease = $holderPool->borrow(); + + try { + $contenderLease = $contenderPool->borrow(); + + try { + $holder = $holderLease->getConnection(); + $contender = $contenderLease->getConnection(); + $schema = $holder->getSchemaBuilder(); + $schema->create('skip_locked_rows', function (Blueprint $table): void { + $table->integer('id')->primary(); + }); + + try { + $holder->table('skip_locked_rows')->insert([['id' => 1], ['id' => 2]]); + + foreach (['lockForUpdate', 'sharedLock'] as $lockMethod) { + $holder->beginTransaction(); + + try { + $holder->table('skip_locked_rows')->where('id', 1)->lockForUpdate()->first(); + + $contender->transaction(function () use ($contender, $lockMethod): void { + $rows = $contender->table('skip_locked_rows') + ->orderBy('id')->{$lockMethod}()->skipLocked()->pluck('id'); + + $this->assertSame([2], $rows->all()); + }); + } finally { + $holder->rollBack(); + } + } + } finally { + $schema->drop('skip_locked_rows'); + } + } finally { + $contenderLease->release(); + } + } finally { + $holderLease->release(); + $holderPool->close(); + $contenderPool->close(); + } + } +} From bacd140a99ce269365d87a5f67238c426b3777bb Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 22 Oct 2026 08:59:10 +0000 Subject: [PATCH 04/29] Make queue rate-limit retry padding configurable Add a nonnegative releasePadding setter while preserving the existing three-second default. Apply padding to the limiter's calculated retry delay without adding storage access or replacing dynamic backoff. Preserve fixed releaseAfter precedence, including an explicit zero, and serialize the configured padding with middleware options. Document the distinction between fixed delay and padding. Exercise default and custom padding, subsecond rounding, long retry intervals, fixed overrides, negative input and serialization. Queue unit and rate-limiting integration tests, formatting and static analysis pass. --- src/docs/queues.md | 8 ++++ src/queue/src/Middleware/RateLimited.php | 24 +++++++++++- tests/Queue/RateLimitedTest.php | 50 +++++++++++++++++++++--- 3 files changed, 76 insertions(+), 6 deletions(-) diff --git a/src/docs/queues.md b/src/docs/queues.md index 450a19cec..43f18f3b5 100644 --- a/src/docs/queues.md +++ b/src/docs/queues.md @@ -827,6 +827,14 @@ public function middleware(): array } ``` +By default, the middleware adds three seconds to the rate limiter's calculated retry delay. You may adjust this padding without replacing the calculated delay: + +```php +return [(new RateLimited('backups'))->releasePadding(0)]; +``` + +Padding must be non-negative. A delay set with `releaseAfter` takes precedence over the calculated delay and padding. + If you do not want a job to be retried when it is rate limited, you may use the `dontRelease` method: ```php diff --git a/src/queue/src/Middleware/RateLimited.php b/src/queue/src/Middleware/RateLimited.php index 326eeeb36..a0c72d474 100644 --- a/src/queue/src/Middleware/RateLimited.php +++ b/src/queue/src/Middleware/RateLimited.php @@ -11,6 +11,7 @@ use Hypervel\RateLimiter\RateLimiter; use Hypervel\RateLimiter\Unlimited; use Hypervel\Support\Collection; +use InvalidArgumentException; use UnitEnum; use function Hypervel\Support\enum_value; @@ -37,6 +38,11 @@ class RateLimited */ public DateTimeInterface|int|null $releaseAfter = null; + /** + * The extra seconds added to the rate limiter's retry delay. + */ + protected int $releasePadding = 3; + /** * Indicates if the job should be released if the limit is exceeded. */ @@ -87,8 +93,9 @@ protected function handleJob(mixed $job, callable $next, array $limits, Limiter { foreach ($limiter->consumeMany(array_values($limits), $this->limiterName) as $result) { if ($result->denied()) { + // An explicit zero delay requests an immediate retry rather than the calculated delay. return $this->shouldRelease - ? $job->release($this->releaseAfter ?? $result->retryAfter() + 3) + ? $job->release($this->releaseAfter ?? $result->retryAfter() + $this->releasePadding) : false; } } @@ -106,6 +113,20 @@ public function releaseAfter(DateTimeInterface|int $releaseAfter): static return $this; } + /** + * Set the extra seconds added to the rate limiter's retry delay. + */ + public function releasePadding(int $seconds): static + { + if ($seconds < 0) { + throw new InvalidArgumentException('Release padding must not be negative.'); + } + + $this->releasePadding = $seconds; + + return $this; + } + /** * Do not release the job back to the queue if the limit is exceeded. */ @@ -137,6 +158,7 @@ public function __sleep(): array 'limiterName', 'storeName', 'releaseAfter', + 'releasePadding', 'shouldRelease', ]; } diff --git a/tests/Queue/RateLimitedTest.php b/tests/Queue/RateLimitedTest.php index 6649b1c8c..85369f827 100644 --- a/tests/Queue/RateLimitedTest.php +++ b/tests/Queue/RateLimitedTest.php @@ -11,8 +11,10 @@ use Hypervel\RateLimiter\LimitResult; use Hypervel\RateLimiter\RateLimiter; use Hypervel\Tests\TestCase; +use InvalidArgumentException; use Mockery as m; use Mockery\MockInterface; +use PHPUnit\Framework\Attributes\DataProvider; use stdClass; enum RateLimitedTestStringEnum: string @@ -107,14 +109,19 @@ public function testExplicitStoreOverridesTheNamedLimiterStoreAndSurvivesSeriali $this->assertSame('handled', $middleware->handle(new stdClass, fn (): string => 'handled')); } - public function testDeniedJobUsesDecisionRetryTimeAndThreeSecondBuffer(): void - { + #[DataProvider('retryDelayProvider')] + public function testDeniedJobUsesConfiguredRetryDelayAfterSerialization( + int $retryMicroseconds, + ?int $padding, + ?int $fixedDelay, + int $expectedDelay, + ): void { $policy = Limit::perMinute(1); $store = m::mock(Limiter::class); $store->shouldReceive('consumeMany') ->once() ->with([$policy], 'uploads') - ->andReturn([new LimitResult(false, 1, 0, 7_000_000, 7_000_000)]); + ->andReturn([new LimitResult(false, 1, 0, $retryMicroseconds, $retryMicroseconds)]); $manager = $this->mockRateLimiter(); $manager->shouldReceive('limiter')->andReturn(fn () => $policy); @@ -122,9 +129,42 @@ public function testDeniedJobUsesDecisionRetryTimeAndThreeSecondBuffer(): void $manager->shouldReceive('store')->with(null)->andReturn($store); $job = m::mock(); - $job->shouldReceive('release')->once()->with(10)->andReturnNull(); + $job->shouldReceive('release')->once()->with($expectedDelay)->andReturnNull(); + + $middleware = new RateLimited('uploads'); + + if ($padding !== null) { + $this->assertSame($middleware, $middleware->releasePadding($padding)); + } + + if ($fixedDelay !== null) { + $middleware->releaseAfter($fixedDelay); + } + + $middleware = unserialize(serialize($middleware)); + + $this->assertNull($middleware->handle($job, fn () => 'handled')); + } + + public static function retryDelayProvider(): array + { + return [ + 'default padding' => [7_000_000, null, null, 10], + 'custom padding' => [7_000_000, 5, null, 12], + 'zero padding preserves rounding' => [100_000, 0, null, 1], + 'zero padding preserves long delays' => [3_600_000_000, 0, null, 3600], + 'fixed delay overrides padding' => [7_000_000, 5, 60, 60], + 'fixed zero overrides padding' => [7_000_000, 5, 0, 0], + ]; + } + + public function testNegativeReleasePaddingIsRejected(): void + { + $this->mockRateLimiter(); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Release padding must not be negative.'); - $this->assertNull((new RateLimited('uploads'))->handle($job, fn () => 'handled')); + (new RateLimited('uploads'))->releasePadding(-1); } public function testGroupUsesTheFirstDeniedPolicyToReleaseTheJob(): void From 14f8a0608c9d139f86b6cb771e1034f7c4f4f1dc Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 22 Oct 2026 08:59:10 +0000 Subject: [PATCH 05/29] Enforce numeric exponent policy across validation paths Guard both field-comparison operands before arbitrary-precision parsing and retain each operand's attribute identity for custom exponent policies. Route compiled multiple_of checks through the same guarded validation entry point as delegated execution, keeping developer-supplied rule literals trusted. Treat malformed numeric input and rejected exponents as validation failures instead of application errors. Preserve protected calculation hooks and custom policies. When comparison message formatting encounters a rejected exponent, display the original trimmed value without expanding it or throwing again. Share the default exponent bound and extraction through Number for reuse by decimal casts. Add concise explanations of intentional behavior differences and regressions covering both execution paths, callbacks, malformed values and rendered messages. The Validation suite, formatting and both static-analysis passes are clean. --- src/support/src/Number.php | 19 +++++ .../src/Concerns/ReplacesAttributes.php | 10 ++- .../src/Concerns/ValidatesAttributes.php | 45 ++++++----- src/validation/src/PlanExecutor.php | 2 +- .../ValidationCompiledExecutionTest.php | 76 +++++++++++++++++++ 5 files changed, 133 insertions(+), 19 deletions(-) diff --git a/src/support/src/Number.php b/src/support/src/Number.php index 2f4bfb7a1..4d06ce671 100644 --- a/src/support/src/Number.php +++ b/src/support/src/Number.php @@ -16,6 +16,11 @@ class Number { use Macroable; + /** + * The largest scientific-notation exponent expanded by default. + */ + public const int MAX_EXPONENT = 1000; + /** * The default locale. */ @@ -64,6 +69,20 @@ public static function format(float|int $number, ?int $precision = null, ?int $m return $formatter->format($number); } + /** + * Get the exponent of a numeric string written in scientific notation. + * + * @internal + */ + public static function exponent(string $number): ?int + { + if (! is_numeric($number) || ($position = stripos($number, 'e')) === false) { + return null; + } + + return (int) substr($number, $position + 1); + } + /** * Parse the given string according to the specified format type. */ diff --git a/src/validation/src/Concerns/ReplacesAttributes.php b/src/validation/src/Concerns/ReplacesAttributes.php index 0e6e274e4..b5cccebd1 100644 --- a/src/validation/src/Concerns/ReplacesAttributes.php +++ b/src/validation/src/Concerns/ReplacesAttributes.php @@ -5,6 +5,7 @@ namespace Hypervel\Validation\Concerns; use Hypervel\Support\Arr; +use Hypervel\Support\Exceptions\MathException; use Hypervel\Support\Str; trait ReplacesAttributes @@ -453,7 +454,14 @@ protected function replaceGt(string $message, string $attribute, string $rule, a return str_replace(':value', $this->getDisplayableAttribute($parameters[0]), $message); } - return str_replace(':value', (string) $this->getSize($attribute, $value), $message); + try { + $size = $this->getSize($attribute, $value); + } catch (MathException) { + // Rejected exponents fail validation; rendering their message must not throw. + $size = $this->trim($value); + } + + return str_replace(':value', (string) $size, $message); } /** diff --git a/src/validation/src/Concerns/ValidatesAttributes.php b/src/validation/src/Concerns/ValidatesAttributes.php index 817bce46e..f07e128fe 100644 --- a/src/validation/src/Concerns/ValidatesAttributes.php +++ b/src/validation/src/Concerns/ValidatesAttributes.php @@ -24,6 +24,7 @@ use Hypervel\Support\Exceptions\MathException; use Hypervel\Support\Facades\Date; use Hypervel\Support\Json; +use Hypervel\Support\Number; use Hypervel\Support\Str; use Hypervel\Validation\FakeDnsGetRecordWrapper; use Hypervel\Validation\Rules\Exists; @@ -1349,7 +1350,9 @@ public function validateGt(string $attribute, mixed $value, mixed $parameters): if ($this->hasRule($attribute, $this->numericRules) && is_numeric($value) && is_numeric($comparedToValue)) { try { - return BigNumber::of((string) $this->trim($value))->isGreaterThan((string) $this->trim($comparedToValue)); + // Either field can contain scientific notation that expands beyond the allowed range. + return BigNumber::of((string) $this->ensureExponentWithinAllowedRange($attribute, $this->trim($value))) + ->isGreaterThan((string) $this->ensureExponentWithinAllowedRange($parameters[0], $this->trim($comparedToValue))); } catch (MathException|BrickMathException) { return false; } @@ -1393,7 +1396,9 @@ public function validateLt(string $attribute, mixed $value, mixed $parameters): if ($this->hasRule($attribute, $this->numericRules) && is_numeric($value) && is_numeric($comparedToValue)) { try { - return BigNumber::of((string) $this->trim($value))->isLessThan((string) $this->trim($comparedToValue)); + // Either field can contain scientific notation that expands beyond the allowed range. + return BigNumber::of((string) $this->ensureExponentWithinAllowedRange($attribute, $this->trim($value))) + ->isLessThan((string) $this->ensureExponentWithinAllowedRange($parameters[0], $this->trim($comparedToValue))); } catch (MathException|BrickMathException) { return false; } @@ -1437,7 +1442,9 @@ public function validateGte(string $attribute, mixed $value, mixed $parameters): if ($this->hasRule($attribute, $this->numericRules) && is_numeric($value) && is_numeric($comparedToValue)) { try { - return BigNumber::of((string) $this->trim($value))->isGreaterThanOrEqualTo((string) $this->trim($comparedToValue)); + // Either field can contain scientific notation that expands beyond the allowed range. + return BigNumber::of((string) $this->ensureExponentWithinAllowedRange($attribute, $this->trim($value))) + ->isGreaterThanOrEqualTo((string) $this->ensureExponentWithinAllowedRange($parameters[0], $this->trim($comparedToValue))); } catch (MathException|BrickMathException) { return false; } @@ -1481,7 +1488,9 @@ public function validateLte(string $attribute, mixed $value, mixed $parameters): if ($this->hasRule($attribute, $this->numericRules) && is_numeric($value) && is_numeric($comparedToValue)) { try { - return BigNumber::of((string) $this->trim($value))->isLessThanOrEqualTo((string) $this->trim($comparedToValue)); + // Either field can contain scientific notation that expands beyond the allowed range. + return BigNumber::of((string) $this->ensureExponentWithinAllowedRange($attribute, $this->trim($value))) + ->isLessThanOrEqualTo((string) $this->ensureExponentWithinAllowedRange($parameters[0], $this->trim($comparedToValue))); } catch (MathException|BrickMathException) { return false; } @@ -1919,20 +1928,24 @@ public function validateMissingWithAll(string $attribute, mixed $value, mixed $p * Validate the value of an attribute is a multiple of a given value. * * @param array $parameters - * - * @throws MathException */ public function validateMultipleOf(string $attribute, mixed $value, mixed $parameters): bool { $this->requireParameterCount(1, $parameters, 'multiple_of'); - return $this->isMultipleOf($value, $parameters[0]); + // Numeric input can still expand to an unsafe size before divisibility is checked. + try { + return $this->isMultipleOf( + $this->ensureExponentWithinAllowedRange($attribute, $value), + $parameters[0], + ); + } catch (MathException) { + return false; + } } /** * Determine if a value is an exact multiple of a divisor. - * - * @throws MathException */ protected function isMultipleOf(mixed $value, mixed $divisor): bool { @@ -1957,8 +1970,9 @@ protected function isMultipleOf(mixed $value, mixed $divisor): bool } return $numerator->remainder($denominator)->isZero(); - } catch (BrickMathException $e) { - throw new MathException('An error occurred while handling the multiple_of input values.', previous: $e); + } catch (BrickMathException) { + // Malformed numeric input fails validation rather than escaping as an application error. + return false; } } @@ -2786,16 +2800,13 @@ protected function ensureExponentWithinAllowedRange(string $attribute, mixed $va $stringValue = (string) $value; - if (! Str::contains($stringValue, 'e', ignoreCase: true)) { + if (($scale = Number::exponent($stringValue)) === null) { return $value; } - $scale = (int) (Str::contains($stringValue, 'e') - ? Str::after($stringValue, 'e') - : Str::after($stringValue, 'E')); - $withinRange = ( - $this->ensureExponentWithinAllowedRangeUsing ?? fn ($scale) => $scale <= 1000 && $scale >= -1000 + $this->ensureExponentWithinAllowedRangeUsing + ?? fn (int $scale) => $scale <= Number::MAX_EXPONENT && $scale >= -Number::MAX_EXPONENT )($scale, $attribute, $value); if (! $withinRange) { diff --git a/src/validation/src/PlanExecutor.php b/src/validation/src/PlanExecutor.php index a635677f1..625960343 100644 --- a/src/validation/src/PlanExecutor.php +++ b/src/validation/src/PlanExecutor.php @@ -355,7 +355,7 @@ protected function executeInline(InlineCheck $check, mixed $value, string $attri CheckType::DateBeforeOrEq => $this->inlineCompareDates($value, $check->param, '<='), CheckType::DateEquals => $this->inlineCompareDates($value, $check->param, '='), - CheckType::MultipleOf => $this->isMultipleOf($value, $check->param), + CheckType::MultipleOf => $this->validateMultipleOf($attribute, $value, [$check->param]), }; } diff --git a/tests/Validation/ValidationCompiledExecutionTest.php b/tests/Validation/ValidationCompiledExecutionTest.php index 3889df54c..2116d0fa1 100644 --- a/tests/Validation/ValidationCompiledExecutionTest.php +++ b/tests/Validation/ValidationCompiledExecutionTest.php @@ -175,6 +175,82 @@ function (int $scale, string $attribute, string $value) use (&$calls, &$argument } } + public function testFieldComparisonsEnforceEachOperandsExponentPolicy(): void + { + foreach ([Validator::class, DelegatedValidationValidator::class] as $validatorClass) { + foreach (['gt', 'gte', 'lt', 'lte'] as $rule) { + foreach (['value', 'other'] as $attribute) { + $data = ['value' => '1e1001', 'other' => '1E-1001']; + $validator = $this->makeValidator($data, ['value' => $rule . ':other'], validatorClass: $validatorClass); + $seen = []; + $validator->ensureExponentWithinAllowedRangeUsing( + function (int $scale, string $field, mixed $input) use (&$seen, $attribute): bool { + $seen[] = [$field, $input]; + + return $field !== $attribute; + }, + ); + + $this->assertFalse($validator->passes()); + $this->assertContains([$attribute, $data[$attribute]], $seen); + } + } + } + } + + public function testRejectedComparisonExponentsRemainPrintableInValidationMessages(): void + { + $translator = new Translator(new ArrayLoader, 'en'); + $translator->addLines(['validation.gt.numeric' => ':attribute must be greater than :value.'], 'en'); + + foreach ([Validator::class, DelegatedValidationValidator::class] as $validatorClass) { + $validator = new $validatorClass($translator, ['value' => 5, 'other' => '1e1001'], ['value' => 'gt:other']); + + $this->assertFalse($validator->passes()); + $this->assertSame('value must be greater than 1e1001.', $validator->messages()->first('value')); + } + } + + public function testNumericInputBeyondTheDefaultExponentRangeFailsValidation(): void + { + foreach ([Validator::class, DelegatedValidationValidator::class] as $validatorClass) { + foreach ([ + [['value' => '1e1001', 'other' => 1], 'gt:other'], + [['value' => '1e1001', 'other' => 1], 'gte:other'], + [['value' => '1e-1001', 'other' => 1], 'lt:other'], + [['value' => '1e-1001', 'other' => 1], 'lte:other'], + [['value' => 1, 'other' => '1e-1001'], 'gt:other'], + [['value' => 1, 'other' => '1e-1001'], 'gte:other'], + [['value' => 1, 'other' => '1e1001'], 'lt:other'], + [['value' => 1, 'other' => '1e1001'], 'lte:other'], + [['value' => '1e1001'], 'multiple_of:1'], + [['value' => '1e-1001'], 'multiple_of:1e-1001'], + [['value' => "5\x0C"], 'multiple_of:1'], + [['value' => "\0" . '5'], 'multiple_of:1'], + [['value' => NAN], 'multiple_of:1'], + ] as [$data, $rule]) { + $this->assertFalse($this->makeValidator($data, ['value' => $rule], validatorClass: $validatorClass)->passes()); + } + } + } + + public function testMultipleOfHonorsExponentOverridesWithoutRestrictingRuleLiterals(): void + { + foreach ([Validator::class, DelegatedValidationValidator::class] as $validatorClass) { + $validator = $this->makeValidator(['value' => '1e1001'], ['value' => 'multiple_of:1'], validatorClass: $validatorClass); + $arguments = []; + $validator->ensureExponentWithinAllowedRangeUsing(function (...$values) use (&$arguments): bool { + $arguments[] = $values; + + return true; + }); + + $this->assertTrue($validator->passes()); + $this->assertSame([[1001, 'value', '1e1001']], $arguments); + $this->assertTrue($this->makeValidator(['value' => 0], ['value' => 'multiple_of:1e1001'], validatorClass: $validatorClass)->passes()); + } + } + public function testMalformedAndNonFiniteNumericSizesFailWithoutWarnings(): void { $literalCases = [ From 444d2ac537f2c16aecf37d8507fa473829bab2db Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 22 Oct 2026 08:59:10 +0000 Subject: [PATCH 06/29] Bound decimal exponent expansion and preserve integer precision Reject scientific-notation exponents outside -1000 to 1000 before request or Eloquent decimal casting can expand them into excessive allocations. Retain existing math-exception boundaries, rational input support and unrestricted ordinary decimal-string length. Include integer inputs explicitly in Eloquent's decimal conversion signature so values above floating-point exactness are not rounded before parsing. Normalize NAN without PHP 8.5 coercion warnings before the existing parser rejects it. Document the scientific-notation bound and appropriate request validation. Cover positive and negative boundaries, long explicit decimals, fractions, integer exactness and non-finite rejection through request extraction and model reads and serialization. Affected unit and integration suites, formatting and static analysis pass. --- .../src/Eloquent/Concerns/HasAttributes.php | 14 ++++++-- src/docs/eloquent-mutators.md | 2 +- src/docs/validation.md | 4 ++- src/foundation/src/Http/Concerns/HasCasts.php | 12 ++++++- .../Http/FormRequestCastingTest.php | 34 +++++++++++++++++++ .../EloquentModelDecimalCastingTest.php | 34 +++++++++++++++++++ 6 files changed, 95 insertions(+), 5 deletions(-) diff --git a/src/database/src/Eloquent/Concerns/HasAttributes.php b/src/database/src/Eloquent/Concerns/HasAttributes.php index 073c4a5a7..5d13619fe 100644 --- a/src/database/src/Eloquent/Concerns/HasAttributes.php +++ b/src/database/src/Eloquent/Concerns/HasAttributes.php @@ -45,6 +45,7 @@ use Hypervel\Support\Facades\Crypt; use Hypervel\Support\Facades\Date; use Hypervel\Support\Facades\Hash; +use Hypervel\Support\Number; use Hypervel\Support\Str; use Hypervel\Support\StrCache; use InvalidArgumentException; @@ -1451,10 +1452,19 @@ public function fromFloat(mixed $value): float * * @throws MathException */ - protected function asDecimal(float|string $value, int $decimals): string + protected function asDecimal(float|int|string $value, int $decimals): string { + // Keep integers exact and avoid PHP 8.5's warning when stringifying NAN. + $value = is_float($value) && is_nan($value) ? 'NAN' : (string) $value; + $exponent = Number::exponent($value); + + // Bound scientific-notation expansion without restricting explicit decimal precision. + if ($exponent !== null && ($exponent > Number::MAX_EXPONENT || $exponent < -Number::MAX_EXPONENT)) { + throw new MathException('Unable to cast value to a decimal.'); + } + try { - return (string) BigDecimal::of((string) $value)->toScale($decimals, RoundingMode::HalfUp); + return (string) BigDecimal::of($value)->toScale($decimals, RoundingMode::HalfUp); } catch (BrickMathException $e) { throw new MathException('Unable to cast value to a decimal.', previous: $e); } diff --git a/src/docs/eloquent-mutators.md b/src/docs/eloquent-mutators.md index f61e397e9..984b94c33 100644 --- a/src/docs/eloquent-mutators.md +++ b/src/docs/eloquent-mutators.md @@ -255,7 +255,7 @@ The `casts` method should return an array where the key is the name of the attri -The `decimal` cast requires a non-negative integer precision, such as `decimal:2`. +The `decimal` cast requires a non-negative integer precision, such as `decimal:2`. Scientific-notation inputs must have an exponent between -1000 and 1000; larger exponents throw a `Hypervel\Support\Exceptions\MathException` to prevent excessive memory allocation. This does not limit the length of an ordinary decimal string. To demonstrate attribute casting, let's cast the `is_admin` attribute, which is stored in our database as an integer (`0` or `1`) to a boolean value: diff --git a/src/docs/validation.md b/src/docs/validation.md index c2384ade1..fb13eafef 100644 --- a/src/docs/validation.md +++ b/src/docs/validation.md @@ -771,7 +771,9 @@ Hypervel supports the following primitive casts: Primitive and enum casts preserve `null`. A custom caster receives a present `null` value so it may decide how nullable input should be handled. Missing paths are skipped. -Use validation rules that guarantee each submitted value can be cast. For example, validate JSON strings with the `json` rule, decimals with the `numeric` rule, and formatted dates with the matching `date_format` rule. An incompatible value causes casting to throw. +Use validation rules that guarantee each submitted value can be cast. For example, validate JSON strings with the `json` rule, decimals with the `numeric` rule and appropriate `min` and `max` bounds, and formatted dates with the matching `date_format` rule. An incompatible value causes casting to throw. + +Decimal casts accept scientific notation with exponents between -1000 and 1000. Values outside that range throw a `Hypervel\Support\Exceptions\MathException` to prevent excessive memory allocation, even if they passed the `numeric` rule. This does not limit the length of an ordinary decimal string. Date and time casts accept `DateTimeInterface` instances, Unix timestamps, and date strings. Numeric strings without an explicit format are treated as Unix timestamps. An explicit format takes precedence, so `datetime:!Y` parses the value `2026` as a year rather than a timestamp. diff --git a/src/foundation/src/Http/Concerns/HasCasts.php b/src/foundation/src/Http/Concerns/HasCasts.php index 8a71f6f82..ee9c69de3 100644 --- a/src/foundation/src/Http/Concerns/HasCasts.php +++ b/src/foundation/src/Http/Concerns/HasCasts.php @@ -18,6 +18,7 @@ use Hypervel\Support\Exceptions\MathException; use Hypervel\Support\Facades\Date; use Hypervel\Support\Json; +use Hypervel\Support\Number; use Hypervel\Validation\ValidationData; use InvalidArgumentException; use stdClass; @@ -443,8 +444,17 @@ protected function fromFloat(mixed $value): float */ protected function asDecimal(mixed $value, int $decimals): string { + // Avoid PHP 8.5's warning when stringifying NAN before Brick rejects it. + $value = is_float($value) && is_nan($value) ? 'NAN' : (string) $value; + $exponent = Number::exponent($value); + + // Validated numeric input can still contain exponents that exhaust worker memory. + if ($exponent !== null && ($exponent > Number::MAX_EXPONENT || $exponent < -Number::MAX_EXPONENT)) { + throw new MathException('Unable to cast value to a decimal.'); + } + try { - return (string) BigDecimal::of((string) $value)->toScale($decimals, RoundingMode::HalfUp); + return (string) BigDecimal::of($value)->toScale($decimals, RoundingMode::HalfUp); } catch (BrickMathException $exception) { throw new MathException('Unable to cast value to a decimal.', previous: $exception); } diff --git a/tests/Foundation/Http/FormRequestCastingTest.php b/tests/Foundation/Http/FormRequestCastingTest.php index 3bb5fa5a2..4d61fb881 100644 --- a/tests/Foundation/Http/FormRequestCastingTest.php +++ b/tests/Foundation/Http/FormRequestCastingTest.php @@ -284,6 +284,40 @@ public function testJsonCastsRejectInvalidInput(): void } } + #[DataProvider('decimalParsingProvider')] + public function testDecimalParsingPreservesPrecisionAndBoundsExponentExpansion(float|int|string $value, ?string $expected): void + { + $request = $this->validateRequest(ConfigurableDecimalRequest::class, [ + 'value' => $value, + 'declaration' => 'decimal:2', + ]); + + if ($expected === null) { + $this->expectException(MathException::class); + $this->expectExceptionMessageIs('Unable to cast value to a decimal.'); + $request->validated(); + + return; + } + + $this->assertSame($expected, $request->validated('value')); + $this->assertSame($expected, $request->safe()->all()['value']); + } + + public static function decimalParsingProvider(): array + { + return [ + 'positive exponent rejected' => ['1e1001', null], + 'negative exponent rejected' => ['1E-1001', null], + 'positive boundary accepted' => ['1e1000', '1' . str_repeat('0', 1000) . '.00'], + 'negative boundary accepted' => ['1e-1000', '0.00'], + 'explicit long decimal preserved' => [str_repeat('9', 1002) . '.125', str_repeat('9', 1002) . '.13'], + 'fraction preserved' => ['1/4', '0.25'], + 'integer precision preserved' => [9007199254740993, '9007199254740993.00'], + 'NAN rejected without warnings' => [NAN, null], + ]; + } + /** * Test invalid decimal input uses the framework math exception boundary. */ diff --git a/tests/Integration/Database/EloquentModelDecimalCastingTest.php b/tests/Integration/Database/EloquentModelDecimalCastingTest.php index 888524764..c724f1a86 100644 --- a/tests/Integration/Database/EloquentModelDecimalCastingTest.php +++ b/tests/Integration/Database/EloquentModelDecimalCastingTest.php @@ -41,6 +41,40 @@ public function testItHandlesExponent(): void $this->assertSame('123.45678900000000000000', $model->amount); } + #[DataProvider('decimalParsingProvider')] + public function testDecimalParsingPreservesPrecisionAndBoundsExponentExpansion(float|int|string $value, ?string $expected): void + { + $model = new class extends Model { + protected array $casts = ['amount' => 'decimal:2']; + }; + $model->amount = $value; + + if ($expected === null) { + $this->expectException(MathException::class); + $this->expectExceptionMessageIs('Unable to cast value to a decimal.'); + $model->toArray(); + + return; + } + + $this->assertSame($expected, $model->amount); + $this->assertSame($expected, $model->toArray()['amount']); + } + + public static function decimalParsingProvider(): array + { + return [ + 'positive exponent rejected' => ['1e1001', null], + 'negative exponent rejected' => ['1E-1001', null], + 'positive boundary accepted' => ['1e1000', '1' . str_repeat('0', 1000) . '.00'], + 'negative boundary accepted' => ['1e-1000', '0.00'], + 'explicit long decimal preserved' => [str_repeat('9', 1002) . '.125', str_repeat('9', 1002) . '.13'], + 'fraction preserved' => ['1/4', '0.25'], + 'integer precision preserved' => [9007199254740993, '9007199254740993.00'], + 'NAN rejected without warnings' => [NAN, null], + ]; + } + public function testItHandlesIntegersWithUnderscores(): void { $model = new class extends Model { From 619832b55754b8c7d61281a2b014faa84b6eecdd Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 22 Oct 2026 09:12:45 +0000 Subject: [PATCH 07/29] Document framework correctness improvements Record the database skip-locked capability and fluent query API, configurable queue retry padding, numeric validation guards, and bounded decimal casts in a framework-owned plan. Preserve engine compatibility, protected extension points, exact integer handling, and targeted verification requirements. Keep the plan independent of application domains and record that implementation is verified with merging remaining. --- ...0911-framework-correctness-improvements.md | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 docs/plans/2026-10-22-0911-framework-correctness-improvements.md diff --git a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md new file mode 100644 index 000000000..240035689 --- /dev/null +++ b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md @@ -0,0 +1,23 @@ +# Framework Correctness Improvements + +## Scope and status + +Independently useful database, queue, validation, and decimal-casting improvements. Keep this plan independent of private applications and packages; describe framework requirements without their names, paths, or domain details. Preserve Laravel signatures and protected extension points, except for explicitly approved behavior changes. + +The changes below are implemented and verified; merging remains. Record further framework changes here when they warrant planning, without duplicating consumer-specific implementation plans. + +## Design + +- **Database capability:** add `Connection::supportsSkipLocked(): bool`, false by default, rather than widening the driver-neutral `ConnectionInterface`. PDO implementations reuse the current engine/version decision and its connection-local memoization/reset behavior. Preserve configured version overrides and MariaDB detection through either driver: MariaDB 10.6+, MySQL 8.0.1+, Vitess/PlanetScale 19+, PostgreSQL 9.5+; SQLite remains false. Migrate Database Queue's internal capability use while preserving `getLockForPopping(): bool|string` and its invocation. Do not retain parallel engine/version logic; retire superseded `@internal` popping helpers after their callers migrate. +- **Query-builder API:** add fluent `lockForUpdate()->skipLocked()` without changing `lockForUpdate()` or `sharedLock()` signatures. The no-argument modifier requires an existing update/shared lock; reject custom-string combinations in either setter order. Carry intent structurally; grammars own engine SQL, including MySQL's `for share` versus MariaDB's `lock in share mode`. Unsupported versions retain their normal lock behavior; callers needing a nonblocking claim check the connection capability and choose compare-and-swap updates. No lock-mode class hierarchy or domain-specific framework claim service. +- **Queue middleware:** add `RateLimited::releasePadding(int $seconds): static`, nonnegative, with a default of three seconds preserving current behavior. `releaseAfter()` retains fixed-delay precedence. Include the property in the serialization whitelist. Callers can opt into `releasePadding(0)` without additional I/O or lookup for existing jobs. +- **Validation defect:** guard both numeric field operands of `gt/gte/lt/lte` with their respective attribute names. Guard `multiple_of` input through `validateMultipleOf()` in compiled and delegated execution, preserving the calculation hook's signature. Exponent rejection and numeric parse errors fail validation rather than escaping as math exceptions. Comparison-message formatting preserves the `getSize()` hook and displays the original trimmed value if its exponent is rejected, without expanding it or throwing again. Preserve the attribute-aware override callback; developer-supplied rule literals remain trusted. Components already require Brick Math 1.0; no further version migration is needed. +- **Decimal casts:** prevent scientific-notation expansion beyond ±1000 in request and Eloquent decimal casts, throwing their existing math exception. Share exponent extraction through internal `Number::exponent()` and the default `Number::MAX_EXPONENT` with validation, without limiting ordinary decimal-string length or changing rational parsing. Include `int` in Eloquent's decimal-cast input type so integer values retain their precision instead of passing through float conversion. Test both casts, exponent boundaries, unchanged explicit decimals/fractions and large integer precision with bounded inputs. + +## Verification + +- Reuse the existing database version/override/PDO-reset and queue-hook cases; add the additive builder grammar/unsupported-path checks and real multi-connection lock contention coverage. +- Test limiter default/fixed/zero-padding/long-delay behavior and serialization. +- Test exponent rejection on both comparison operands and `multiple_of`, callback overrides, and compiled/interpreted parity using safe inputs such as `1e1001`, not memory-exhaustion experiments. +- Run each affected test file immediately and finish owning-package checks. Run formatting, PHPStan, and targeted tests in that order; retain applicable upstream tests. +- No cache change is required: stack TTL caps, nullable unique-lock stores, and documented flexible-cache microcache behavior are already handled by current components. From 2960533d2a613a280000a204311dd7df4373a67d Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 22 Oct 2026 22:16:50 +0000 Subject: [PATCH 08/29] Restore cached SQLite databases at the physical test pool Restore the shared in-memory PDO through DatabasePool instead of replacing only a logical connection lease. Setup can release and reborrow sessions without losing the cached migrated schema or its dispatcher. Add testing-only restoration through DatabaseConnectionResolver, preserving unpooled and custom resolver paths. An already-restored PDO is a no-op; changing the shared PDO releases idle leases and rebuilds wrappers through the existing pool lifecycle. Cover eager refresh, truncation across application lifecycles, and resolver restoration. Document single-session in-memory pool behavior without adding production query overhead or changing Laravel testing hooks. --- src/database/src/Pool/DatabasePool.php | 14 +++++++ src/docs/database-testing.md | 2 + .../Testing/DatabaseConnectionResolver.php | 26 ++++++++++++ .../src/Testing/DatabaseTruncation.php | 7 ++++ .../src/Testing/RefreshDatabase.php | 7 ++++ .../DatabaseConnectionResolverTest.php | 33 +++++++++++++++ .../MigrateWithHypervelMigrationsTest.php | 1 + ...lMigrationsUsingDatabaseTruncationTest.php | 3 ++ .../Databases/PooledRefreshDatabaseTest.php | 41 +++++++++++++++++++ 9 files changed, 134 insertions(+) create mode 100644 tests/Testbench/Databases/PooledRefreshDatabaseTest.php diff --git a/src/database/src/Pool/DatabasePool.php b/src/database/src/Pool/DatabasePool.php index 903de2bc6..b0bf9f201 100644 --- a/src/database/src/Pool/DatabasePool.php +++ b/src/database/src/Pool/DatabasePool.php @@ -132,6 +132,20 @@ public function getSharedInMemorySqlitePdo(): ?PDO return $this->sharedInMemorySqlitePdo; } + /** + * Restore the shared in-memory database retained between tests. + * + * Tests only. Replaces the database used by every subsequent pool borrower. + */ + public function restoreSharedInMemorySqlitePdo(PDO $pdo): void + { + while ($connection = $this->popIdleConnection()) { + $this->destroyConnection($connection); + } + + $this->sharedInMemorySqlitePdo = $pdo; + } + /** * Record the query grammar date format of one of the pool's connections. * diff --git a/src/docs/database-testing.md b/src/docs/database-testing.md index baa045703..1e22f029d 100644 --- a/src/docs/database-testing.md +++ b/src/docs/database-testing.md @@ -65,6 +65,8 @@ If you would like to totally reset the database, you may use the `Hypervel\Found By default, database tests reuse one shared connection per connection name. For tests that issue queries concurrently, use a file-backed or server database and set `database.connections.{name}.pool.testing_enabled` to `true` in your test environment configuration. Each coroutine then borrows its own connection. Use `DatabaseTruncation` or `DatabaseMigrations` for these tests, since writes committed by child coroutines are not rolled back by the test coroutine's transaction. +An in-memory SQLite pool uses a single session, so its database queries run one at a time even when pooling is enabled. Use a file-backed or server database when your test needs simultaneous database sessions. + Transaction-based traits require a connection with real transaction support. `DatabaseTruncation` uses the connection's schema builder to reset the selected tables, allowing custom database drivers to provide their own reset behavior without replacing the native testing traits. Driver authors should follow the [schema-builder extension contracts](/docs/{{version}}/database#extending-database-connections). diff --git a/src/foundation/src/Testing/DatabaseConnectionResolver.php b/src/foundation/src/Testing/DatabaseConnectionResolver.php index 0899ad293..e180ac2f8 100644 --- a/src/foundation/src/Testing/DatabaseConnectionResolver.php +++ b/src/foundation/src/Testing/DatabaseConnectionResolver.php @@ -16,6 +16,7 @@ use Hypervel\Database\ConnectionResolver; use Hypervel\Database\Pool\DatabasePool; use Hypervel\Database\Pool\PooledConnection; +use PDO; use Throwable; use UnitEnum; @@ -295,6 +296,31 @@ public function connection(UnitEnum|string|null $name = null): ConnectionInterfa return $connection; } + /** + * Restore a pooled in-memory database, returning false when pooling is disabled. + * + * Tests only. Replaces the database shared by this application's pool borrowers. + */ + public function restorePooledInMemoryPdo(UnitEnum|string|null $name, PDO $pdo): bool + { + $connectionName = ConnectionName::parse($this->connectionName($name)); + + if (! $this->poolEnabled($connectionName)) { + return false; + } + + $pool = $this->poolManager->pool($connectionName->requested); + + if ($pool->getSharedInMemorySqlitePdo() === $pdo) { + return true; + } + + static::releaseIdleConnections(); + $pool->restoreSharedInMemorySqlitePdo($pdo); + + return true; + } + /** * Determine whether this test uses coroutine-owned pooled connections. */ diff --git a/src/foundation/src/Testing/DatabaseTruncation.php b/src/foundation/src/Testing/DatabaseTruncation.php index 6c92a945d..bc7afcd93 100644 --- a/src/foundation/src/Testing/DatabaseTruncation.php +++ b/src/foundation/src/Testing/DatabaseTruncation.php @@ -106,11 +106,18 @@ protected function restoreInMemoryDatabases(): void $database = $this->app->make('db'); $defaultConnection = $this->app->make('config')->string('database.default'); + $resolver = $this->app->bound('db.resolver') ? $this->app->make('db.resolver') : null; foreach ($this->connectionsToTruncate() as $name) { $connectionName = $name ?? $defaultConnection; if (isset(RefreshDatabaseState::$inMemoryConnections[$connectionName])) { + // A lease loses setPdo() on release; restore the pool's shared database instead. + if ($resolver instanceof DatabaseConnectionResolver + && $resolver->restorePooledInMemoryPdo($name, RefreshDatabaseState::$inMemoryConnections[$connectionName])) { + continue; + } + // The PDO outlives its original application; the dispatcher must not. $connection = $database->connection($name); diff --git a/src/foundation/src/Testing/RefreshDatabase.php b/src/foundation/src/Testing/RefreshDatabase.php index 5efba22eb..782c1e795 100644 --- a/src/foundation/src/Testing/RefreshDatabase.php +++ b/src/foundation/src/Testing/RefreshDatabase.php @@ -57,11 +57,18 @@ public function refreshDatabase(): void protected function restoreInMemoryDatabase(): void { $database = $this->app->make('db'); + $resolver = $this->app->bound('db.resolver') ? $this->app->make('db.resolver') : null; foreach ($this->connectionsToTransact() as $name) { $connectionName = $name ?? $this->getRefreshConnection(); if (isset(RefreshDatabaseState::$inMemoryConnections[$connectionName])) { + // A lease loses setPdo() on release; restore the pool's shared database instead. + if ($resolver instanceof DatabaseConnectionResolver + && $resolver->restorePooledInMemoryPdo($name, RefreshDatabaseState::$inMemoryConnections[$connectionName])) { + continue; + } + $connection = $database->connection($name); if (! $connection instanceof PdoConnection) { diff --git a/tests/Foundation/Testing/DatabaseConnectionResolverTest.php b/tests/Foundation/Testing/DatabaseConnectionResolverTest.php index 14f4a1165..e34b2fec0 100644 --- a/tests/Foundation/Testing/DatabaseConnectionResolverTest.php +++ b/tests/Foundation/Testing/DatabaseConnectionResolverTest.php @@ -5,21 +5,54 @@ namespace Hypervel\Tests\Foundation\Testing; use Hypervel\Container\Container; +use Hypervel\Contracts\Foundation\Application; use Hypervel\Database\Connection; use Hypervel\Database\DatabaseTransactionsManager; use Hypervel\Database\Pool\PooledConnection; use Hypervel\Database\Pool\PoolManager; use Hypervel\Foundation\Testing\DatabaseConnectionResolver; use Hypervel\Foundation\Testing\DatabaseTransactionsManager as TestingTransactionsManager; +use Hypervel\Testbench\Attributes\DefineEnvironment; use Hypervel\Testbench\Attributes\WithConfig; use Hypervel\Testbench\TestCase; use Mockery as m; +use PDO; use ReflectionProperty; use RuntimeException; #[WithConfig('database.default', 'testing')] class DatabaseConnectionResolverTest extends TestCase { + /** + * Use coroutine-owned SQLite sessions for restoration tests. + */ + protected function definePooledDatabase(Application $app): void + { + $app->make('config')->set('database.connections.testing.pool.testing_enabled', true); + } + + #[DefineEnvironment('definePooledDatabase')] + public function testRestoredPdoSurvivesIdleRelease(): void + { + $resolver = $this->app->make('db.resolver'); + $connection = $resolver->connection('testing::read'); + $connection->statement('create table previous_database (value integer)'); + $pool = $this->app->make(PoolManager::class)->pool('testing'); + $saved = new PDO('sqlite::memory:'); + $saved->exec('create table restored_database (value integer)'); + $saved->exec('insert into restored_database values (42)'); + + $this->assertTrue($resolver->restorePooledInMemoryPdo('testing::read', $saved)); + $this->assertSame($saved, $pool->getSharedInMemorySqlitePdo()); + $this->assertSame(0, $pool->getManagedCount()); + $this->assertSame(42, $connection->table('restored_database')->value('value')); + $this->assertSame($this->app->make('events'), $connection->getEventDispatcher()); + + $resolver->releaseIdleConnections(); + + $this->assertSame(42, $connection->table('restored_database')->value('value')); + } + public function testFlushDisconnectsCachedConnection(): void { $resolver = $this->app->make(DatabaseConnectionResolver::class); diff --git a/tests/Testbench/Databases/MigrateWithHypervelMigrationsTest.php b/tests/Testbench/Databases/MigrateWithHypervelMigrationsTest.php index d8e340b7b..5f80ef854 100644 --- a/tests/Testbench/Databases/MigrateWithHypervelMigrationsTest.php +++ b/tests/Testbench/Databases/MigrateWithHypervelMigrationsTest.php @@ -49,6 +49,7 @@ public function itStartsTheFirstUserTransactionInsideTheLazyTestTransaction(): v $connection->beginTransaction(); $this->assertSame(2, $connection->transactionLevel()); + $this->assertSame(0, $connection->table('users')->count()); $connection->rollBack(); diff --git a/tests/Testbench/Databases/MigrateWithHypervelMigrationsUsingDatabaseTruncationTest.php b/tests/Testbench/Databases/MigrateWithHypervelMigrationsUsingDatabaseTruncationTest.php index f02dfb408..b9e21c048 100644 --- a/tests/Testbench/Databases/MigrateWithHypervelMigrationsUsingDatabaseTruncationTest.php +++ b/tests/Testbench/Databases/MigrateWithHypervelMigrationsUsingDatabaseTruncationTest.php @@ -19,6 +19,7 @@ use function Hypervel\Testbench\workbench_path; #[WithConfig('database.default', 'testing')] +#[WithConfig('database.connections.testing.pool.testing_enabled', true)] class MigrateWithHypervelMigrationsUsingDatabaseTruncationTest extends TestCase { use DatabaseTruncation; @@ -61,6 +62,8 @@ public function itMigratesOnceAndThenTruncatesAcrossApplicationLifecycles(int $l $this->assertSame($lifecycle === 2, $this->truncated); $this->assertTrue(Schema::hasTable('users')); $this->assertTrue(Schema::hasTable('testbench_users')); + DB::releaseIdleConnections(); + $this->assertTrue(Schema::hasTable('testbench_users')); $this->assertSame(0, DB::connection()->transactionLevel()); $this->assertSame( 0, diff --git a/tests/Testbench/Databases/PooledRefreshDatabaseTest.php b/tests/Testbench/Databases/PooledRefreshDatabaseTest.php new file mode 100644 index 000000000..3f44c63d9 --- /dev/null +++ b/tests/Testbench/Databases/PooledRefreshDatabaseTest.php @@ -0,0 +1,41 @@ +exec('create table retained_records (value integer)'); + $saved->exec('insert into retained_records values (42)'); + RefreshDatabaseState::$inMemoryConnections['testing'] = $saved; + RefreshDatabaseState::$migrated = true; + } + + public function testRestoredDatabaseSurvivesTheSetupCoroutine(): void + { + $connection = DB::connection(); + $this->assertSame(1, $connection->transactionLevel()); + $this->assertSame(RefreshDatabaseState::$inMemoryConnections['testing'], $connection->getPdo()); + $this->assertSame(42, $connection->table('retained_records')->value('value')); + } +} From c75483995eab20d813c66ba1fc534d124c4ddca8 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 22 Oct 2026 22:17:01 +0000 Subject: [PATCH 09/29] Restore pooled databases before registering lazy refresh hooks Install the cached in-memory database before resolving the logical connection and registering lazy callbacks. A first query, cursor, or explicit session pin now uses the correct physical PDO while migration and transaction startup remain lazy. Retain the ordinary restoration path as an identity-preserving no-op for an already-restored pool. Add focused coverage for query-first and pinned first-use behavior alongside existing transaction-first tests. --- .../src/Testing/LazilyRefreshDatabase.php | 5 ++ .../PooledLazilyRefreshDatabaseTest.php | 59 +++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 tests/Testbench/Databases/PooledLazilyRefreshDatabaseTest.php diff --git a/src/foundation/src/Testing/LazilyRefreshDatabase.php b/src/foundation/src/Testing/LazilyRefreshDatabase.php index 52dc6d6af..25b6c9fbf 100644 --- a/src/foundation/src/Testing/LazilyRefreshDatabase.php +++ b/src/foundation/src/Testing/LazilyRefreshDatabase.php @@ -33,6 +33,11 @@ public function refreshDatabase(): void */ protected function setUpLazilyRefreshDatabaseInCoroutine(): void { + // Restore before registering callbacks: the first query already pins its pooled session. + if ($this->usingInMemoryDatabases()) { + $this->restoreInMemoryDatabase(); + } + $database = $this->app->make('db'); $callback = function (): void { diff --git a/tests/Testbench/Databases/PooledLazilyRefreshDatabaseTest.php b/tests/Testbench/Databases/PooledLazilyRefreshDatabaseTest.php new file mode 100644 index 000000000..a2cb11002 --- /dev/null +++ b/tests/Testbench/Databases/PooledLazilyRefreshDatabaseTest.php @@ -0,0 +1,59 @@ +exec('create table retained_records (value integer)'); + $saved->exec('insert into retained_records values (42)'); + RefreshDatabaseState::$inMemoryConnections['testing'] = $saved; + RefreshDatabaseState::$migrated = true; + } + + #[DataProvider('firstQueries')] + public function testRestoresTheDatabaseBeforeTheFirstQuery(string $scope): void + { + $connection = DB::connection(); + $this->assertSame(0, $connection->transactionLevel()); + $this->assertSame(RefreshDatabaseState::$inMemoryConnections['testing'], $connection->getPdo()); + $query = fn (): mixed => $connection->table('retained_records')->value('value'); + + $this->assertSame(42, match ($scope) { + 'pinned' => $connection->withPinnedSession($query), + 'transaction' => $connection->transaction($query), + default => $query(), + }); + $this->assertSame(1, $connection->transactionLevel()); + } + + /** + * Provide first-query scopes that must share the restored database. + */ + public static function firstQueries(): array + { + return ['ordinary query' => ['ordinary'], 'explicitly pinned query' => ['pinned'], 'transaction first' => ['transaction']]; + } +} From a856bd9782b1e8fdfba0d89e1f8f28e0bc476f5c Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 22 Oct 2026 22:17:01 +0000 Subject: [PATCH 10/29] Document pooled database restoration requirements Record physical-pool ownership of cached SQLite PDO restoration, testing resolver boundaries, and eager and lazy refresh coverage. Keep framework planning independent of downstream applications and retain the existing Laravel testing extension points. Mark the implemented correctness work as verified with merging remaining. Describe the single-session in-memory restriction and preserve the existing unpooled restoration path. --- ...-10-22-0911-framework-correctness-improvements.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md index 240035689..ea894d3bb 100644 --- a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md +++ b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md @@ -4,7 +4,7 @@ Independently useful database, queue, validation, and decimal-casting improvements. Keep this plan independent of private applications and packages; describe framework requirements without their names, paths, or domain details. Preserve Laravel signatures and protected extension points, except for explicitly approved behavior changes. -The changes below are implemented and verified; merging remains. Record further framework changes here when they warrant planning, without duplicating consumer-specific implementation plans. +The database capability, query-builder, queue middleware, validation, decimal-casting and pooled in-memory database restoration changes are implemented and verified; merging remains. Record further framework changes here when they warrant planning, without duplicating consumer-specific implementation plans. ## Design @@ -14,6 +14,15 @@ The changes below are implemented and verified; merging remains. Record further - **Validation defect:** guard both numeric field operands of `gt/gte/lt/lte` with their respective attribute names. Guard `multiple_of` input through `validateMultipleOf()` in compiled and delegated execution, preserving the calculation hook's signature. Exponent rejection and numeric parse errors fail validation rather than escaping as math exceptions. Comparison-message formatting preserves the `getSize()` hook and displays the original trimmed value if its exponent is rejected, without expanding it or throwing again. Preserve the attribute-aware override callback; developer-supplied rule literals remain trusted. Components already require Brick Math 1.0; no further version migration is needed. - **Decimal casts:** prevent scientific-notation expansion beyond ±1000 in request and Eloquent decimal casts, throwing their existing math exception. Share exponent extraction through internal `Number::exponent()` and the default `Number::MAX_EXPONENT` with validation, without limiting ordinary decimal-string length or changing rational parsing. Include `int` in Eloquent's decimal-cast input type so integer values retain their precision instead of passing through float conversion. Test both casts, exponent boundaries, unchanged explicit decimals/fractions and large integer precision with bounded inputs. +### Pooled in-memory database restoration + +- `RefreshDatabase` and `DatabaseTruncation` must restore cached SQLite databases at the physical pool owner. Setting the PDO only on a logical lease loses the restored schema when setup releases that lease; `LazilyRefreshDatabase` shares the affected refresh path. +- Add `DatabasePool::restoreSharedInMemorySqlitePdo(PDO $pdo): void`, marked `Tests only.` Drain idle wrappers through existing pool cleanup, then replace the shared PDO. Subsequent borrows use the current factory and dispatcher. Preserve normal connection/lease semantics and add no production hot-path work. +- Add testing `DatabaseConnectionResolver::restorePooledInMemoryPdo(UnitEnum|string|null $name, PDO $pdo): bool`, also marked `Tests only.` Reuse `poolEnabled()`; return false without resolving connections when disabled. Return true immediately when the named pool already holds the supplied PDO, before releasing any leases. Otherwise release idle leases, restore through that pool and return true. Preserve existing physical-pool name resolution. +- In `LazilyRefreshDatabase`, restore cached in-memory databases before resolving connections and registering lazy callbacks, using the existing `usingInMemoryDatabases()` condition. Migrations and transactions remain lazy. The callback's ordinary restoration becomes a no-op for an already-restored pool, so query-first, cursor-first and explicitly pinned first use retain normal session behavior. Explain this upstream difference concisely; do not change production query or foreign-key suppression pinning. +- Keep both traits' Laravel protected signatures and bare restoration bodies unchanged. Use the pooled operation only when a bound `db.resolver` is the testing resolver and the operation returns true. Custom resolvers and manager doubles retain the existing PDO/type-check/dispatcher path. Do not implicitly read test state during pool construction, add a registry, remigrate every test or substitute a file database for this fix. +- Document that in-memory SQLite pools use one session and serialize queries; tests requiring simultaneous database sessions still need a file-backed or server database. + ## Verification - Reuse the existing database version/override/PDO-reset and queue-hook cases; add the additive builder grammar/unsupported-path checks and real multi-connection lock contention coverage. @@ -21,3 +30,4 @@ The changes below are implemented and verified; merging remains. Record further - Test exponent rejection on both comparison operands and `multiple_of`, callback overrides, and compiled/interpreted parity using safe inputs such as `1e1001`, not memory-exhaustion experiments. - Run each affected test file immediately and finish owning-package checks. Run formatting, PHPStan, and targeted tests in that order; retain applicable upstream tests. - No cache change is required: stack TTL caps, nullable unique-lock stores, and documented flexible-cache microcache behavior are already handled by current components. +- For pooled SQLite restoration, cover setup-to-test coroutine handoff with a cached PDO seeded before eager refresh, and ordinary/pinned first queries with a cached PDO seeded before lazy hook registration. Testbench normally clears refresh state between applications; do not mistake remigration for restoration coverage. Retain the truncation cross-application regression, resolver idle release/reborrow checks, bare-path tests and existing transaction-first lazy coverage. Run the affected database/reset tests and the required Testbench package-mode suite. From 8be4d385acd9219bcb72babf05933d50e2ac5944 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Fri, 23 Oct 2026 05:22:14 +0000 Subject: [PATCH 11/29] Clarify conditional pooled database restoration naming Rename the testing resolver helper to restoreInMemoryPdoIfPooled so callers can distinguish an inapplicable pooled restore from a failed operation. False still means pooling is disabled and the normal restoration path should run. Update both database testing traits, the resolver regression test and the framework plan. Preserve the signature, restoration behavior and tests-only scope without adding a compatibility alias or another pooling lookup. --- .../plans/2026-10-22-0911-framework-correctness-improvements.md | 2 +- src/foundation/src/Testing/DatabaseConnectionResolver.php | 2 +- src/foundation/src/Testing/DatabaseTruncation.php | 2 +- src/foundation/src/Testing/RefreshDatabase.php | 2 +- tests/Foundation/Testing/DatabaseConnectionResolverTest.php | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md index 979d78725..62b6b5d12 100644 --- a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md +++ b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md @@ -18,7 +18,7 @@ The database capability, query-builder, queue middleware, validation, decimal-ca - `RefreshDatabase` and `DatabaseTruncation` must restore cached SQLite databases at the physical pool owner. Setting the PDO only on a logical lease loses the restored schema when setup releases that lease; `LazilyRefreshDatabase` shares the affected refresh path. - Add `DatabasePool::restoreSharedInMemorySqlitePdo(PDO $pdo): void`, marked `Tests only.` Drain idle wrappers through existing pool cleanup, then replace the shared PDO. Subsequent borrows use the current factory and dispatcher. Preserve normal connection/lease semantics and add no production hot-path work. -- Add testing `DatabaseConnectionResolver::restorePooledInMemoryPdo(UnitEnum|string|null $name, PDO $pdo): bool`, also marked `Tests only.` Reuse `poolEnabled()`; return false without resolving connections when disabled. Return true immediately when the named pool already holds the supplied PDO, before releasing any leases. Otherwise release idle leases, restore through that pool and return true. Preserve existing physical-pool name resolution. +- Add testing `DatabaseConnectionResolver::restoreInMemoryPdoIfPooled(UnitEnum|string|null $name, PDO $pdo): bool`, also marked `Tests only.` Reuse `poolEnabled()`; return false without resolving connections when disabled. Return true immediately when the named pool already holds the supplied PDO, before releasing any leases. Otherwise release idle leases, restore through that pool and return true. Preserve existing physical-pool name resolution. - In `LazilyRefreshDatabase`, restore cached in-memory databases before resolving connections and registering lazy callbacks, using the existing `usingInMemoryDatabases()` condition. Migrations and transactions remain lazy. The callback's ordinary restoration becomes a no-op for an already-restored pool, so query-first, cursor-first and explicitly pinned first use retain normal session behavior. Explain this upstream difference concisely; do not change production query or foreign-key suppression pinning. - Keep both traits' Laravel protected signatures and bare restoration bodies unchanged. Use the pooled operation only when a bound `db.resolver` is the testing resolver and the operation returns true. Custom resolvers and manager doubles retain the existing PDO/type-check/dispatcher path. Do not implicitly read test state during pool construction, add a registry, remigrate every test or substitute a file database for this fix. - Document that in-memory SQLite pools use one session and serialize queries; tests requiring simultaneous database sessions still need a file-backed or server database. diff --git a/src/foundation/src/Testing/DatabaseConnectionResolver.php b/src/foundation/src/Testing/DatabaseConnectionResolver.php index e180ac2f8..be0f28833 100644 --- a/src/foundation/src/Testing/DatabaseConnectionResolver.php +++ b/src/foundation/src/Testing/DatabaseConnectionResolver.php @@ -301,7 +301,7 @@ public function connection(UnitEnum|string|null $name = null): ConnectionInterfa * * Tests only. Replaces the database shared by this application's pool borrowers. */ - public function restorePooledInMemoryPdo(UnitEnum|string|null $name, PDO $pdo): bool + public function restoreInMemoryPdoIfPooled(UnitEnum|string|null $name, PDO $pdo): bool { $connectionName = ConnectionName::parse($this->connectionName($name)); diff --git a/src/foundation/src/Testing/DatabaseTruncation.php b/src/foundation/src/Testing/DatabaseTruncation.php index bc7afcd93..122e65a1d 100644 --- a/src/foundation/src/Testing/DatabaseTruncation.php +++ b/src/foundation/src/Testing/DatabaseTruncation.php @@ -114,7 +114,7 @@ protected function restoreInMemoryDatabases(): void if (isset(RefreshDatabaseState::$inMemoryConnections[$connectionName])) { // A lease loses setPdo() on release; restore the pool's shared database instead. if ($resolver instanceof DatabaseConnectionResolver - && $resolver->restorePooledInMemoryPdo($name, RefreshDatabaseState::$inMemoryConnections[$connectionName])) { + && $resolver->restoreInMemoryPdoIfPooled($name, RefreshDatabaseState::$inMemoryConnections[$connectionName])) { continue; } diff --git a/src/foundation/src/Testing/RefreshDatabase.php b/src/foundation/src/Testing/RefreshDatabase.php index 782c1e795..a4ea2dc42 100644 --- a/src/foundation/src/Testing/RefreshDatabase.php +++ b/src/foundation/src/Testing/RefreshDatabase.php @@ -65,7 +65,7 @@ protected function restoreInMemoryDatabase(): void if (isset(RefreshDatabaseState::$inMemoryConnections[$connectionName])) { // A lease loses setPdo() on release; restore the pool's shared database instead. if ($resolver instanceof DatabaseConnectionResolver - && $resolver->restorePooledInMemoryPdo($name, RefreshDatabaseState::$inMemoryConnections[$connectionName])) { + && $resolver->restoreInMemoryPdoIfPooled($name, RefreshDatabaseState::$inMemoryConnections[$connectionName])) { continue; } diff --git a/tests/Foundation/Testing/DatabaseConnectionResolverTest.php b/tests/Foundation/Testing/DatabaseConnectionResolverTest.php index e34b2fec0..9d8480ab0 100644 --- a/tests/Foundation/Testing/DatabaseConnectionResolverTest.php +++ b/tests/Foundation/Testing/DatabaseConnectionResolverTest.php @@ -42,7 +42,7 @@ public function testRestoredPdoSurvivesIdleRelease(): void $saved->exec('create table restored_database (value integer)'); $saved->exec('insert into restored_database values (42)'); - $this->assertTrue($resolver->restorePooledInMemoryPdo('testing::read', $saved)); + $this->assertTrue($resolver->restoreInMemoryPdoIfPooled('testing::read', $saved)); $this->assertSame($saved, $pool->getSharedInMemorySqlitePdo()); $this->assertSame(0, $pool->getManagedCount()); $this->assertSame(42, $connection->table('restored_database')->value('value')); From d969f23e8cb943ddc5999ae17f60e32c921be666 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Sat, 24 Oct 2026 20:59:24 +0000 Subject: [PATCH 12/29] Preserve exact numeric tokens when decoding JSON Add Json::decodeNumbersAsStrings for API payloads whose numbers must retain their original precision and notation. Validate the original document before quoting numeric tokens, skip existing strings with a linear scan, and use native decoding for values, object shapes and depth errors. Keep ordinary decoding unchanged. The scan avoids regular-expression limits on valid escaped strings. Cover exact decimals and large integers, exponent notation, escapes, malformed input and nesting boundaries. Formatting, full static analysis and the focused JSON tests pass. --- ...0911-framework-correctness-improvements.md | 8 ++- src/support/src/Json.php | 52 ++++++++++++++++ tests/Support/JsonTest.php | 60 +++++++++++++++++++ 3 files changed, 119 insertions(+), 1 deletion(-) diff --git a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md index 62b6b5d12..2e2131777 100644 --- a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md +++ b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md @@ -2,7 +2,7 @@ ## Scope and status -Independently useful database, queue, validation, and decimal-casting improvements. Keep this plan independent of private applications and packages; describe framework requirements without their names, paths, or domain details. Preserve Laravel signatures and protected extension points, except for explicitly approved behavior changes. +Independently useful database, queue, validation, decimal-casting and JSON-decoding improvements. Keep this plan independent of private applications and packages; describe framework requirements without their names, paths, or domain details. Preserve Laravel signatures and protected extension points, except for explicitly approved behavior changes. The database capability, query-builder, queue middleware, validation, decimal-casting and pooled in-memory database restoration changes are implemented and verified; merging remains. Record further framework changes here when they warrant planning, without duplicating consumer-specific implementation plans. @@ -14,6 +14,12 @@ The database capability, query-builder, queue middleware, validation, decimal-ca - **Validation defect:** guard both numeric field operands of `gt/gte/lt/lte` with their respective attribute names. Guard `multiple_of` input through `validateMultipleOf()` in compiled and delegated execution, preserving the calculation hook's signature. Exponent rejection and numeric parse errors fail validation rather than escaping as math exceptions. Comparison-message formatting preserves the `getSize()` hook and displays the original trimmed value if its exponent is rejected, without expanding it or throwing again. Preserve the attribute-aware override callback; developer-supplied rule literals remain trusted. Components already require Brick Math 1.0; no further version migration is needed. - **Decimal casts:** prevent scientific-notation expansion beyond ±1000 in request and Eloquent decimal casts, throwing their existing math exception. Share exponent extraction through internal `Number::exponent()` and the default `Number::MAX_EXPONENT` with validation, without limiting ordinary decimal-string length or changing rational parsing. Include `int` in Eloquent's decimal-cast input type so integer values retain their precision instead of passing through float conversion. Test both casts, exponent boundaries, unchanged explicit decimals/fractions and large integer precision with bounded inputs. +### Exact JSON numbers + +- Add `Json::decodeNumbersAsStrings(string $json, bool $assoc = true, int $depth = self::MAXIMUM_NESTING_DEPTH): mixed` for exact numeric API inputs. Return every numeric token as its original string, including integers, trailing zeros and exponent notation; preserve ordinary strings, booleans, null and native array/object shapes. +- Validate the original document through `Json::validate()` before quoting numeric tokens: quoting alone can turn malformed input into valid JSON. Use a linear walk that skips existing strings and escaped bytes, then delegate to `Json::decode()`. Retain native errors and the existing public depth convention. No new parser framework, number wrapper, flags parameter or changes to ordinary decoding. +- Cover exact values, escaped strings, associative/object results, malformed numbers and object keys, invalid UTF-8 and depth limits in the existing JSON tests. + ### Pooled in-memory database restoration - `RefreshDatabase` and `DatabaseTruncation` must restore cached SQLite databases at the physical pool owner. Setting the PDO only on a logical lease loses the restored schema when setup releases that lease; `LazilyRefreshDatabase` shares the affected refresh path. diff --git a/src/support/src/Json.php b/src/support/src/Json.php index e105262f2..c7131eaf8 100644 --- a/src/support/src/Json.php +++ b/src/support/src/Json.php @@ -42,6 +42,58 @@ public static function decode(string $json, bool $assoc = true, int $depth = sel return json_decode($json, $assoc, self::nativeDecodingDepth($depth), $flags | JSON_THROW_ON_ERROR); } + /** + * Decode JSON with every number preserved as its original string. + * + * @throws JsonException + */ + public static function decodeNumbersAsStrings(string $json, bool $assoc = true, int $depth = self::MAXIMUM_NESTING_DEPTH): mixed + { + // Quoting numbers before validation could make malformed JSON valid, such as {1:2}. + if (! self::validate($json, $depth)) { + throw new JsonException(json_last_error_msg(), json_last_error()); + } + + $length = strlen($json); + $offset = 0; + $start = 0; + $quoted = ''; + + while ($offset < $length) { + $offset += strcspn($json, '"-0123456789', $offset); + + if ($offset === $length) { + break; + } + + if ($json[$offset] === '"') { + ++$offset; + + while (true) { + $offset += strcspn($json, '"\\', $offset); + + if ($json[$offset] === '"') { + ++$offset; + + break; + } + + $offset += 2; + } + + continue; + } + + $numberLength = strspn($json, '0123456789+-.eE', $offset); + $quoted .= substr($json, $start, $offset - $start) + . '"' . substr($json, $offset, $numberLength) . '"'; + $offset += $numberLength; + $start = $offset; + } + + return self::decode($quoted . substr($json, $start), $assoc, $depth); + } + /** * Validate a JSON string. * diff --git a/tests/Support/JsonTest.php b/tests/Support/JsonTest.php index 4fea646dd..18faca18d 100644 --- a/tests/Support/JsonTest.php +++ b/tests/Support/JsonTest.php @@ -10,6 +10,7 @@ use Hypervel\Tests\TestCase; use JsonException; use PHPUnit\Framework\Attributes\DataProvider; +use stdClass; use ValueError; class JsonTest extends TestCase @@ -104,6 +105,63 @@ public function testDecodeThrowsOnInvalidJson(): void Json::decode('{invalid}'); } + #[DataProvider('jsonWithExactNumbers')] + public function testDecodeNumbersAsStringsPreservesNumberTokens(string $json, mixed $expected): void + { + $this->assertSame($expected, Json::decodeNumbersAsStrings($json)); + } + + /** + * Provide numeric tokens and strings whose contents must remain untouched. + */ + public static function jsonWithExactNumbers(): iterable + { + yield 'decimal scalar' => ['1.234567890123456789', '1.234567890123456789']; + yield 'lexical forms' => ['[1.10,-0,1E+2,1e9999]', ['1.10', '-0', '1E+2', '1e9999']]; + yield 'nested values' => [ + '{"123":{"rates":[123456789012345678901,0.000000000000000001]},"flags":[true,false,null],"empty":[]}', + [123 => ['rates' => ['123456789012345678901', '0.000000000000000001']], 'flags' => [true, false, null], 'empty' => []], + ]; + yield 'null scalar' => ['null', null]; + yield 'string scalar' => ['"123\u0034"', '1234']; + + $escaped = str_repeat('é123"\\', 2000); + + yield 'escaped strings' => [json_encode([$escaped, 1], JSON_THROW_ON_ERROR), [$escaped, '1']]; + } + + public function testDecodeNumbersAsStringsPreservesObjectShape(): void + { + $value = Json::decodeNumbersAsStrings('{"rate":1.10,"empty":{}}', false); + + $this->assertSame('1.10', $value->rate); + $this->assertInstanceOf(stdClass::class, $value->empty); + } + + #[DataProvider('invalidJsonWithNumbers')] + public function testDecodeNumbersAsStringsRejectsOriginalInvalidInput(string $json, int $depth): void + { + json_decode($json, true, $depth + 1); + $error = json_last_error(); + + $this->expectException(JsonException::class); + $this->expectExceptionCode($error); + + Json::decodeNumbersAsStrings($json, depth: $depth); + } + + /** + * Provide invalid documents that token quoting must not repair. + */ + public static function invalidJsonWithNumbers(): iterable + { + yield 'leading zero' => ['[01]', 8]; + yield 'missing fraction' => ['[1.]', 8]; + yield 'numeric key' => ['{1:2}', 8]; + yield 'invalid UTF-8' => ["[\"\xff\",1]", 8]; + yield 'too deep' => ['[[1]]', 1]; + } + public function testEncodeThrowsOnInvalidValue(): void { $this->expectException(JsonException::class); @@ -117,6 +175,7 @@ public function testDefaultMaximumNestingDepthRoundTrips(): void $json = Json::encode($value); $this->assertSame($value, Json::decode($json)); + $this->assertSame($value, Json::decodeNumbersAsStrings($json)); $this->assertTrue(Json::validate($json)); } @@ -126,6 +185,7 @@ public function testExplicitMaximumNestingDepthRoundTrips(): void $json = Json::encode($value, depth: 8); $this->assertSame($value, Json::decode($json, depth: 8)); + $this->assertSame($value, Json::decodeNumbersAsStrings($json, depth: 8)); $this->assertTrue(Json::validate($json, depth: 8)); } From c860506230bef321070b7fd08f6bd6d2df2d199a Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Wed, 28 Oct 2026 12:51:12 +0000 Subject: [PATCH 13/29] Accept model-specific Eloquent builders in attribute types Use wildcard model specializations for the builder attribute, resolved-class cache and resolver return annotation. This accepts custom builders constrained to their owning model without changing runtime construction or public signatures. Add a type regression covering the attribute with HasBuilder, a model subclass and a fluent terminal result. Full source and type analysis and the existing builder runtime tests pass. --- ...0911-framework-correctness-improvements.md | 5 ++ .../Attributes/UseEloquentBuilder.php | 3 +- src/database/src/Eloquent/Model.php | 4 +- .../Database/Eloquent/UseEloquentBuilder.php | 46 +++++++++++++++++++ 4 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 types/Database/Eloquent/UseEloquentBuilder.php diff --git a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md index 2e2131777..b4a6f16db 100644 --- a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md +++ b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md @@ -14,6 +14,11 @@ The database capability, query-builder, queue middleware, validation, decimal-ca - **Validation defect:** guard both numeric field operands of `gt/gte/lt/lte` with their respective attribute names. Guard `multiple_of` input through `validateMultipleOf()` in compiled and delegated execution, preserving the calculation hook's signature. Exponent rejection and numeric parse errors fail validation rather than escaping as math exceptions. Comparison-message formatting preserves the `getSize()` hook and displays the original trimmed value if its exponent is rejected, without expanding it or throwing again. Preserve the attribute-aware override callback; developer-supplied rule literals remain trusted. Components already require Brick Math 1.0; no further version migration is needed. - **Decimal casts:** prevent scientific-notation expansion beyond ±1000 in request and Eloquent decimal casts, throwing their existing math exception. Share exponent extraction through internal `Number::exponent()` and the default `Number::MAX_EXPONENT` with validation, without limiting ordinary decimal-string length or changing rational parsing. Include `int` in Eloquent's decimal-cast input type so integer values retain their precision instead of passing through float conversion. Test both casts, exponent boundaries, unchanged explicit decimals/fractions and large integer precision with bounded inputs. +### Model-specific Eloquent builder types + +- Let `UseEloquentBuilder` accept `class-string>`, matching the existing builder property and factory return type. The attribute accepts builders bound to a particular model; `Builder` incorrectly excludes them because the model parameter is invariant. Align the resolved-class cache and resolver return annotation with the same boundary. Runtime construction and public signatures remain unchanged. +- Cover the attribute with a model-bounded generic builder and `HasBuilder`, including a model subclass and a terminal query result in the type fixtures. + ### Exact JSON numbers - Add `Json::decodeNumbersAsStrings(string $json, bool $assoc = true, int $depth = self::MAXIMUM_NESTING_DEPTH): mixed` for exact numeric API inputs. Return every numeric token as its original string, including integers, trailing zeros and exponent notation; preserve ordinary strings, booleans, null and native array/object shapes. diff --git a/src/database/src/Eloquent/Attributes/UseEloquentBuilder.php b/src/database/src/Eloquent/Attributes/UseEloquentBuilder.php index e965a73ed..fa42cb719 100644 --- a/src/database/src/Eloquent/Attributes/UseEloquentBuilder.php +++ b/src/database/src/Eloquent/Attributes/UseEloquentBuilder.php @@ -6,7 +6,6 @@ use Attribute; use Hypervel\Database\Eloquent\Builder; -use Hypervel\Database\Eloquent\Model; /** * Declare the Eloquent builder class for a model using an attribute. @@ -26,7 +25,7 @@ class UseEloquentBuilder /** * Create a new attribute instance. * - * @param class-string> $builderClass + * @param class-string> $builderClass */ public function __construct( public string $builderClass, diff --git a/src/database/src/Eloquent/Model.php b/src/database/src/Eloquent/Model.php index dbc4984ec..96db809e8 100644 --- a/src/database/src/Eloquent/Model.php +++ b/src/database/src/Eloquent/Model.php @@ -288,7 +288,7 @@ abstract class Model implements Arrayable, ArrayAccess, CanBeEscapedWhenCastToSt /** * Cache of resolved custom builder classes per model. * - * @var array, class-string>|false> + * @var array, class-string>|false> */ protected static array $resolvedBuilderClasses = []; @@ -2109,7 +2109,7 @@ public function newEloquentBuilder(QueryBuilder $query): Builder /** * Resolve the custom Eloquent builder class from the model attributes. * - * @return class-string|false + * @return class-string>|false */ protected function resolveCustomBuilderClass(): string|false { diff --git a/types/Database/Eloquent/UseEloquentBuilder.php b/types/Database/Eloquent/UseEloquentBuilder.php new file mode 100644 index 000000000..4e74649b7 --- /dev/null +++ b/types/Database/Eloquent/UseEloquentBuilder.php @@ -0,0 +1,46 @@ +', Post::query()); + assertType('Hypervel\Types\UseEloquentBuilder\PostBuilder', FeaturedPost::query()); + assertType('Hypervel\Types\UseEloquentBuilder\FeaturedPost|null', FeaturedPost::query()->published()->first()); +} + +#[UseEloquentBuilder(PostBuilder::class)] +class Post extends Model +{ + /** @use HasBuilder> */ + use HasBuilder; +} + +class FeaturedPost extends Post +{ +} + +/** + * @template TModel of Post + * + * @extends Builder + */ +class PostBuilder extends Builder +{ + /** + * Limit results to published posts. + */ + public function published(): static + { + return $this->whereNotNull('published_at'); + } +} From 3945b5732f4e3fe6acd0fd6049f1104732ac8334 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Wed, 28 Oct 2026 16:53:03 +0000 Subject: [PATCH 14/29] Preserve the immutable date type after adding months Correct addMonths metadata to retain the calling CarbonImmutable subclass, including the optional anchor-day argument. Extend the type fixture and retain the existing runtime subclass coverage. Track removal of local modifier annotations after a released Carbon generator fix supplies precise inherited return types. --- .../2026-10-22-0911-framework-correctness-improvements.md | 5 +++++ docs/todo.md | 2 ++ src/support/src/CarbonImmutable.php | 1 + types/Support/CarbonImmutable.php | 1 + 4 files changed, 9 insertions(+) diff --git a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md index b4a6f16db..ba256ce71 100644 --- a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md +++ b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md @@ -19,6 +19,11 @@ The database capability, query-builder, queue middleware, validation, decimal-ca - Let `UseEloquentBuilder` accept `class-string>`, matching the existing builder property and factory return type. The attribute accepts builders bound to a particular model; `Builder` incorrectly excludes them because the model parameter is invariant. Align the resolved-class cache and resolver return annotation with the same boundary. Runtime construction and public signatures remain unchanged. - Cover the attribute with a model-bounded generic builder and `HasBuilder`, including a model subclass and a terminal query result in the type fixtures. +### Immutable date modifier types + +- Keep `CarbonImmutable::addMonths()` metadata typed as `static`, including its optional anchor-day argument. Carbon preserves subclasses at runtime; inherited metadata naming the base class incorrectly widens fluent results. Extend the existing modifier type fixture and reuse the runtime metadata/subclass test. No runtime or API behavior changes. +- The complete fix belongs in Carbon's PHPDoc generator, which emits concrete class names for fluent returns. Retain the existing local annotations until a corrected release is adopted, then remove the redundant annotations and their reflection test while keeping type regressions. Track that dependency update and cleanup in [the framework todo](../todo.md#framework-wide); do not duplicate Carbon's full generated method list here. + ### Exact JSON numbers - Add `Json::decodeNumbersAsStrings(string $json, bool $assoc = true, int $depth = self::MAXIMUM_NESTING_DEPTH): mixed` for exact numeric API inputs. Return every numeric token as its original string, including integers, trailing zeros and exponent notation; preserve ordinary strings, booleans, null and native array/object shapes. diff --git a/docs/todo.md b/docs/todo.md index 355d9ad90..8ea7c532f 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -21,6 +21,8 @@ ## Framework-wide +- Once a released `nesbot/carbon` version fixes generated fluent return annotations to preserve subclasses, raise the applicable Carbon minimum through Composer and remove the redundant `@method static` overrides from `Hypervel\Support\CarbonImmutable` (`src/support/src/CarbonImmutable.php`). Carbon's `phpdoc.php::compileDoc()` currently emits concrete `CarbonImmutable`/`CarbonInterface` return types for these methods even though runtime preserves the calling class. Retain the existing overrides until a fixed release is verified; do not expand them into a duplicate of Carbon's complete method list. Keep `types/Support/CarbonImmutable.php` as dependency regression coverage, verify further subclasses as well, and remove `SupportCarbonImmutableTest::testMagicModifierMetadataMatchesRuntimeStaticType()` if no local overrides remain. No upstream PR has been submitted yet. + - Investigate a small shared serializer contract with native PHP and igbinary implementations for storage and transport boundaries where configurable encoding provides a measured benefit. Select the implementation per store or subsystem, resolve it once during service construction, and keep the selection fixed during worker execution; an optional facade can expose application access. Benchmark representative cache values, sessions, and jobs for encode/decode time, payload size, and complete storage or transport cost, including any base64 encoding. Igbinary can reduce size and decode time for repeated data, but can encode small payloads more slowly; an `auto` option would express a documented preference among compatible installed serializers, not discover the fastest one or benchmark every call. Audit call sites rather than replacing every `serialize()` / `unserialize()` call: preserve native format contracts and internal key generation, reuse existing PhpRedis serialization and Lua packing, and preserve class restrictions (`igbinary_unserialize()` has no native `allowed_classes` equivalent). Queue JSON payloads and their native `O:` detection need explicit format and reader changes before supporting binary encoding. Define format identification and migration so persisted values remain readable across configuration changes and rolling deployments. Use Symfony Cache's `MarshallerInterface` / `DefaultMarshaller` as prior art rather than its object-normalizing Serializer component. GMP is not a general serializer; native PHP and igbinary already round-trip nested GMP values, while `gmp_export()` only exports an integer's magnitude and loses its sign. - Resolve the production dependency cycle between `hypervel/foundation` and `hypervel/testing`, which currently causes test-only classes, PHPUnit integration, and Mockery to propagate through `hypervel/support` into every package that depends on Support. Decide whether Foundation's testing namespace moves to `hypervel/testing` or Foundation changes its dependency to development-only while preserving split-package installation and public namespaces. - Design a connection-owned service identity and capability API for external backends that packages can query without repeated hot-path probes. Redis/Valkey and database connections already expose fragments of this information in different forms; prefer lazy detection cached for the current connection or pool generation, with invalidation on reconnect and purge, over an eager process-global startup registry that performs unused I/O or survives a backend change. Start with concrete consumers and capability checks rather than a universal version-comparison abstraction. diff --git a/src/support/src/CarbonImmutable.php b/src/support/src/CarbonImmutable.php index ddd46adf3..ce147e8d1 100644 --- a/src/support/src/CarbonImmutable.php +++ b/src/support/src/CarbonImmutable.php @@ -19,6 +19,7 @@ * @method static addMinute() * @method static addMinutes(int|float $value = 1) * @method static addMonth() + * @method static addMonths(int|float $value = 1, ?int $anchorDay = null) * @method static addSecond() * @method static addSeconds(int|float $value = 1) * @method static ceilSecond(float $precision = 1) diff --git a/types/Support/CarbonImmutable.php b/types/Support/CarbonImmutable.php index 5df73aaff..c643d165d 100644 --- a/types/Support/CarbonImmutable.php +++ b/types/Support/CarbonImmutable.php @@ -14,6 +14,7 @@ assertType(CarbonImmutable::class, $date->addMinute()); assertType(CarbonImmutable::class, $date->addMinutes()); assertType(CarbonImmutable::class, $date->addMinutes(1.5)); +assertType(CarbonImmutable::class, $date->addMonths(2, anchorDay: 31)); assertType(CarbonImmutable::class, $date->addSecond()); assertType(CarbonImmutable::class, $date->addSeconds()); assertType(CarbonImmutable::class, $date->addSeconds(1.5)); From 58fc2c3656f015f6dc69fbad0d7b266afb9251cb Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 29 Oct 2026 08:43:26 +0000 Subject: [PATCH 15/29] Preserve chained cancellation across database callbacks Recognize cancellation carried by callback and cleanup exceptions before query wrapping, failure reporting and retry decisions. Keep canceled transaction failures intact even when cleanup reports a deadlock. Stop commit callbacks on cancellation, finish rollback cleanup, and preserve cancellation priority through transaction notifications, recovery, disconnect and retained-connection termination. Retain native-only checks and existing public APIs. Extend existing query, callback, rollback and connection cleanup regressions. Validated with full formatting, static analysis, framework, Testbench and dogfood checks. --- ...0911-framework-correctness-improvements.md | 7 + .../src/Concerns/ManagesTransactions.php | 94 +++++++----- src/database/src/Connection.php | 29 ++-- src/database/src/ConnectionResolver.php | 2 +- .../src/DatabaseTransactionRecord.php | 16 ++- .../src/DatabaseTransactionsManager.php | 16 ++- tests/Database/ConnectionResolverTest.php | 11 +- tests/Database/DatabaseConnectionTest.php | 134 +++++++++++------- tests/Database/DatabasePdoConnectionTest.php | 18 ++- .../DatabaseTransactionsManagerTest.php | 19 ++- 10 files changed, 228 insertions(+), 118 deletions(-) diff --git a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md index ba256ce71..90cb290ed 100644 --- a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md +++ b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md @@ -39,6 +39,13 @@ The database capability, query-builder, queue middleware, validation, decimal-ca - Keep both traits' Laravel protected signatures and bare restoration bodies unchanged. Use the pooled operation only when a bound `db.resolver` is the testing resolver and the operation returns true. Custom resolvers and manager doubles retain the existing PDO/type-check/dispatcher path. Do not implicitly read test state during pool construction, add a registry, remigrate every test or substitute a file database for this fix. - Document that in-memory SQLite pools use one session and serialize queries; tests requiring simultaneous database sessions still need a file-backed or server database. +### Cancellation through database callbacks + +- Recognize cancellation carried by a callback or cleanup exception through `Coroutine::causedByCancellation()`, preserving the original exception and foreign-cancellation ownership. Query execution and streaming must not wrap cancellation, retry it, count it as a query error or dispatch `QueryFailed`. +- Stop commit callbacks on cancellation. Finish rollback callbacks while retaining cancellation ahead of ordinary failures. Apply the same rule through transaction manager/event notification and recovery boundaries, connection disconnect and retained-connection termination. Keep native-only PDO and join checks unchanged; do not add speculative cleanup machinery. +- Never retry a canceled transaction or wrap its failure as a deadlock, even when its cleanup exception names a deadlock. +- Extend existing cancellation tests with chained exceptions at query, transaction callback and cleanup boundaries. Preserve public signatures and ordinary-error behavior. Validate the framework and affected database consumers together. + ## Verification - Reuse the existing database version/override/PDO-reset and queue-hook cases; add the additive builder grammar/unsupported-path checks and real multi-connection lock contention coverage. diff --git a/src/database/src/Concerns/ManagesTransactions.php b/src/database/src/Concerns/ManagesTransactions.php index 5543abbdc..842d1295e 100644 --- a/src/database/src/Concerns/ManagesTransactions.php +++ b/src/database/src/Concerns/ManagesTransactions.php @@ -5,11 +5,11 @@ namespace Hypervel\Database\Concerns; use Closure; +use Hypervel\Coroutine\Coroutine; use Hypervel\Database\Connection; use Hypervel\Database\DeadlockException; use LogicException; use RuntimeException; -use Swoole\Coroutine\CanceledException; use Throwable; /** @@ -92,17 +92,21 @@ public function transaction(Closure $callback, int $attempts = 1): mixed $levelBeingCommitted, $this->transactions ); - } catch (CanceledException $cancellation) { - throw $cancellation; } catch (Throwable $throwable) { + if (Coroutine::causedByCancellation($throwable)) { + throw $throwable; + } + $exception = $throwable; } try { $this->fireConnectionEvent('committed'); - } catch (CanceledException $cancellation) { - throw $cancellation; } catch (Throwable $throwable) { + if (Coroutine::causedByCancellation($throwable)) { + throw $throwable; + } + $exception ??= $throwable; } @@ -124,9 +128,12 @@ public function transaction(Closure $callback, int $attempts = 1): mixed */ protected function handleTransactionException(Throwable $e, int $currentAttempt, int $maxAttempts): void { + // Cleanup may report a deadlock while carrying cancellation; do not retry or reclassify it. + $canceled = Coroutine::causedByCancellation($e); + // Roll back failed nested work before propagating it to the outer retry boundary. // If the server already discarded its savepoints, rollback disconnects the unusable session. - if ($this->causedByConcurrencyError($e) + if (! $canceled && $this->causedByConcurrencyError($e) && $this->transactions > 1) { $e = new DeadlockException( $e->getMessage(), @@ -142,19 +149,17 @@ protected function handleTransactionException(Throwable $e, int $currentAttempt, try { $this->rollBack(); - } catch (CanceledException $cancellation) { - if (! $e instanceof CanceledException) { - throw $cancellation; + } catch (Throwable $cleanup) { + if (! $canceled && Coroutine::causedByCancellation($cleanup)) { + throw $cleanup; } - $cleanedUp = false; - } catch (Throwable) { $cleanedUp = false; } // Only retry after all failed work is closed; a callback may leave manual transaction levels open. // Nested deadlocks already passed the driver's policy, which may not recognize our wrapper. - if ($cleanedUp + if (! $canceled && $cleanedUp && $this->transactions === 0 && ($e instanceof DeadlockException || $this->causedByConcurrencyError($e)) && $currentAttempt < $maxAttempts) { @@ -190,11 +195,11 @@ public function beginTransaction(): void } catch (Throwable $exception) { try { $this->rollBack($previousLevel); - } catch (CanceledException $cancellation) { - if (! $exception instanceof CanceledException) { - throw $cancellation; + } catch (Throwable $cleanup) { + if (Coroutine::causedByCancellation($cleanup) && ! Coroutine::causedByCancellation($exception)) { + throw $cleanup; } - } catch (Throwable) { + // Preserve the transaction publication failure. } @@ -263,17 +268,21 @@ public function commit(): void $levelBeingCommitted, $this->transactions ); - } catch (CanceledException $cancellation) { - throw $cancellation; } catch (Throwable $throwable) { + if (Coroutine::causedByCancellation($throwable)) { + throw $throwable; + } + $exception = $throwable; } try { $this->fireConnectionEvent('committed'); - } catch (CanceledException $cancellation) { - throw $cancellation; } catch (Throwable $throwable) { + if (Coroutine::causedByCancellation($throwable)) { + throw $throwable; + } + $exception ??= $throwable; } @@ -292,9 +301,11 @@ protected function handleCommitTransactionException(Throwable $e, int $currentAt if ($this->causedByLostConnection($e)) { try { $this->forgetLostConnection(); - } catch (CanceledException $cancellation) { - throw $cancellation; - } catch (Throwable) { + } catch (Throwable $cleanup) { + if (Coroutine::causedByCancellation($cleanup)) { + throw $cleanup; + } + // Preserve the physical commit failure. } @@ -305,17 +316,16 @@ protected function handleCommitTransactionException(Throwable $e, int $currentAt try { $this->rollBack(0); - } catch (CanceledException $cancellation) { - if (! $e instanceof CanceledException) { - throw $cancellation; + } catch (Throwable $cleanup) { + if (Coroutine::causedByCancellation($cleanup) && ! Coroutine::causedByCancellation($e)) { + throw $cleanup; } - $cleanedUp = false; - } catch (Throwable) { $cleanedUp = false; } if ($cleanedUp + && ! Coroutine::causedByCancellation($e) && $this->causedByConcurrencyError($e) && $currentAttempt < $maxAttempts) { return; @@ -352,11 +362,11 @@ public function rollBack(?int $toLevel = null): void // A missing or failed savepoint cannot safely preserve the enclosing transaction. try { $this->disconnect(); - } catch (CanceledException $cancellation) { - if (! $exception instanceof CanceledException) { - throw $cancellation; + } catch (Throwable $cleanup) { + if (Coroutine::causedByCancellation($cleanup) && ! Coroutine::causedByCancellation($exception)) { + throw $cleanup; } - } catch (Throwable) { + // Preserve the rollback failure after detaching the unusable connection. } } @@ -372,17 +382,21 @@ public function rollBack(?int $toLevel = null): void $this->getNameWithReadWriteType() ?? '', $this->transactions ); - } catch (CanceledException $cancellation) { - throw $cancellation; } catch (Throwable $throwable) { + if (Coroutine::causedByCancellation($throwable)) { + throw $throwable; + } + $exception = $throwable; } try { $this->fireConnectionEvent('rollingBack'); - } catch (CanceledException $cancellation) { - throw $cancellation; } catch (Throwable $throwable) { + if (Coroutine::causedByCancellation($throwable)) { + throw $throwable; + } + $exception ??= $throwable; } @@ -401,9 +415,11 @@ protected function handleRollBackException(Throwable $e): void if ($this->causedByLostConnection($e)) { try { $this->forgetLostConnection(); - } catch (CanceledException $cancellation) { - throw $cancellation; - } catch (Throwable) { + } catch (Throwable $cleanup) { + if (Coroutine::causedByCancellation($cleanup)) { + throw $cleanup; + } + // Preserve the physical rollback failure. } } diff --git a/src/database/src/Connection.php b/src/database/src/Connection.php index 2c19e0d64..fe15cbc7b 100755 --- a/src/database/src/Connection.php +++ b/src/database/src/Connection.php @@ -12,6 +12,7 @@ use Hypervel\Context\NonCopyableContext; use Hypervel\Contracts\Database\Query\Expression as ExpressionContract; use Hypervel\Contracts\Events\Dispatcher; +use Hypervel\Coroutine\Coroutine; use Hypervel\Database\Eloquent\Builder as EloquentBuilder; use Hypervel\Database\Eloquent\Relations\Relation; use Hypervel\Database\Events\QueryExecuted; @@ -584,9 +585,11 @@ protected function run(string $query, array $bindings, Closure $callback): mixed $callback ); } - } catch (CanceledException $exception) { - throw $exception; } catch (Throwable $exception) { + if (Coroutine::causedByCancellation($exception)) { + throw $exception; + } + $events = $this->events; if ($events?->hasListeners(QueryFailed::class)) { @@ -658,9 +661,13 @@ protected function runStreaming(string $query, array $bindings, Closure $callbac $this->latestReadWriteTypeRetrieved = $readWriteType; } } - } catch (CanceledException|StreamClosedException $exception) { + } catch (StreamClosedException $exception) { throw $exception; } catch (Exception $exception) { + if (Coroutine::causedByCancellation($exception)) { + throw $exception; + } + ++$this->errorCount; throw $this->newQueryException($query, $bindings, $exception); @@ -677,9 +684,13 @@ protected function runStreaming(string $query, array $bindings, Closure $callbac yield from $this->handleQueryException($exception, $query, $bindings, $execute); } - } catch (CanceledException|StreamClosedException $exception) { + } catch (StreamClosedException $exception) { throw $exception; } catch (Throwable $exception) { + if (Coroutine::causedByCancellation($exception)) { + throw $exception; + } + $events = $this->events; if ($events?->hasListeners(QueryFailed::class)) { @@ -720,9 +731,11 @@ protected function runQueryCallback(string $query, array $bindings, Closure $cal // If an exception occurs when attempting to run a query, we'll format the error // message to include the SQL and, unless masked, its bindings. This provides // more context for the developer than just the database's original error. - catch (CanceledException $exception) { - throw $exception; - } catch (Exception $e) { + catch (Exception $e) { + if (Coroutine::causedByCancellation($e)) { + throw $e; + } + ++$this->errorCount; throw $this->newQueryException($query, $bindings, $e); @@ -985,7 +998,7 @@ public function disconnect(): void $this->resetTransactionState(); } catch (Throwable $throwable) { if ($exception === null - || ($throwable instanceof CanceledException && ! $exception instanceof CanceledException) + || (Coroutine::causedByCancellation($throwable) && ! Coroutine::causedByCancellation($exception)) ) { $exception = $throwable; } diff --git a/src/database/src/ConnectionResolver.php b/src/database/src/ConnectionResolver.php index f9f8cd8fe..7e329580e 100755 --- a/src/database/src/ConnectionResolver.php +++ b/src/database/src/ConnectionResolver.php @@ -365,7 +365,7 @@ protected function terminateConnections(Closure $terminate): void $terminate($connection); } catch (Throwable $throwable) { if ($exception === null - || ($throwable instanceof CanceledException && ! $exception instanceof CanceledException) + || (Coroutine::causedByCancellation($throwable) && ! Coroutine::causedByCancellation($exception)) ) { $exception = $throwable; } diff --git a/src/database/src/DatabaseTransactionRecord.php b/src/database/src/DatabaseTransactionRecord.php index e0038b774..962af1256 100755 --- a/src/database/src/DatabaseTransactionRecord.php +++ b/src/database/src/DatabaseTransactionRecord.php @@ -4,7 +4,7 @@ namespace Hypervel\Database; -use Swoole\Coroutine\CanceledException; +use Hypervel\Coroutine\Coroutine; use Throwable; class DatabaseTransactionRecord @@ -75,9 +75,11 @@ public function executeCallbacks(): void foreach ($this->callbacks as $callback) { try { $callback(); - } catch (CanceledException $exception) { - throw $exception; } catch (Throwable $throwable) { + if (Coroutine::causedByCancellation($throwable)) { + throw $throwable; + } + $exception ??= $throwable; } } @@ -98,10 +100,12 @@ public function executeCallbacksForRollback(): void foreach ($this->callbacksForRollback as $callback) { try { $callback(); - } catch (CanceledException $canceledException) { - $cancellation ??= $canceledException; } catch (Throwable $throwable) { - $exception ??= $throwable; + if (Coroutine::causedByCancellation($throwable)) { + $cancellation ??= $throwable; + } else { + $exception ??= $throwable; + } } } diff --git a/src/database/src/DatabaseTransactionsManager.php b/src/database/src/DatabaseTransactionsManager.php index 3e96b4603..cc9997ad8 100755 --- a/src/database/src/DatabaseTransactionsManager.php +++ b/src/database/src/DatabaseTransactionsManager.php @@ -5,8 +5,8 @@ namespace Hypervel\Database; use Hypervel\Context\CoroutineContext; +use Hypervel\Coroutine\Coroutine; use Hypervel\Support\Collection; -use Swoole\Coroutine\CanceledException; use Throwable; /** @@ -240,9 +240,11 @@ protected function executeCommitCallbacks(Collection $transactions): void foreach ($transactions as $transaction) { try { $transaction->executeCallbacks(); - } catch (CanceledException $exception) { - throw $exception; } catch (Throwable $throwable) { + if (Coroutine::causedByCancellation($throwable)) { + throw $throwable; + } + $exception ??= $throwable; } } @@ -265,10 +267,12 @@ protected function executeRollbackCallbacks(Collection $transactions): void foreach ($transactions as $transaction) { try { $transaction->executeCallbacksForRollback(); - } catch (CanceledException $canceledException) { - $cancellation ??= $canceledException; } catch (Throwable $throwable) { - $exception ??= $throwable; + if (Coroutine::causedByCancellation($throwable)) { + $cancellation ??= $throwable; + } else { + $exception ??= $throwable; + } } } diff --git a/tests/Database/ConnectionResolverTest.php b/tests/Database/ConnectionResolverTest.php index fea12f4c3..62224d99c 100644 --- a/tests/Database/ConnectionResolverTest.php +++ b/tests/Database/ConnectionResolverTest.php @@ -18,8 +18,10 @@ use Hypervel\Tests\TestCase; use Mockery as m; use PDO; +use PHPUnit\Framework\Attributes\TestWith; use RuntimeException; use Swoole\Coroutine\CanceledException; +use Throwable; use function Hypervel\Coroutine\run; @@ -306,10 +308,15 @@ public function testTerminalReleaseExhaustsConnectionsAndPreservesTheFirstFailur $resolver->releaseConnections(); } - public function testTerminalReleasePrioritizesTheFirstCancellationAndStillExhaustsConnections(): void + #[TestWith([false])] + #[TestWith([true])] + public function testTerminalReleasePrioritizesTheFirstCancellationAndStillExhaustsConnections(bool $chained): void { $ordinaryFailure = new RuntimeException('First release failed.'); $firstCancellation = new CanceledException('Second release was canceled.'); + if ($chained) { + $firstCancellation = new RuntimeException('Release cleanup failed.', previous: $firstCancellation); + } $secondCancellation = new CanceledException('Third release was canceled.'); $poolManager = m::mock(PoolManager::class); $resolver = $this->makeResolver('first', $poolManager); @@ -335,7 +342,7 @@ public function testTerminalReleasePrioritizesTheFirstCancellationAndStillExhaus try { $resolver->releaseConnections(); $this->fail('Expected release cancellation to propagate.'); - } catch (CanceledException $throwable) { + } catch (Throwable $throwable) { $this->assertSame($firstCancellation, $throwable); } diff --git a/tests/Database/DatabaseConnectionTest.php b/tests/Database/DatabaseConnectionTest.php index 3c3eea5b0..678bd3f50 100755 --- a/tests/Database/DatabaseConnectionTest.php +++ b/tests/Database/DatabaseConnectionTest.php @@ -774,10 +774,10 @@ public function testRunMethodRetriesOnFailure(): void }]); } - public function testRunMethodPreservesQueryCancellationWithoutRetryOrLogging(): void + #[DataProvider('cancellations')] + public function testRunMethodPreservesQueryCancellationWithoutRetryOrLogging(Throwable $cancellation): void { $method = (new ReflectionClass(Connection::class))->getMethod('run'); - $cancellation = new CanceledException('query canceled'); $pdo = $this->createStub(PDOStub::class); $connection = $this->getMockConnection(['tryAgainIfCausedByLostConnection'], $pdo); $connection->expects($this->never())->method('tryAgainIfCausedByLostConnection'); @@ -788,7 +788,7 @@ public function testRunMethodPreservesQueryCancellationWithoutRetryOrLogging(): try { $method->invokeArgs($connection, ['select 1', [], fn () => throw $cancellation]); $this->fail('Expected the query cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -796,6 +796,15 @@ public function testRunMethodPreservesQueryCancellationWithoutRetryOrLogging(): $this->assertSame([], $connection->getQueryLog()); } + /** + * Supply direct cancellation and a cleanup exception carrying it. + */ + public static function cancellations(): iterable + { + yield 'direct' => [new CanceledException('Operation canceled.')]; + yield 'chained cleanup' => [new RuntimeException('Cleanup failed.', previous: new CanceledException('Operation canceled.'))]; + } + public function testRunDispatchesQueryFailedWithTheFinalLogicalFailure(): void { $method = (new ReflectionClass(Connection::class))->getMethod('run'); @@ -1233,7 +1242,8 @@ public function testStreamingConsumerExceptionsCleanUpWithoutQueryEvents(): void $this->assertSame([], $connection->getQueryLog()); } - public function testStreamingCancellationCleansUpWithoutWrappingOrEvents(): void + #[DataProvider('cancellations')] + public function testStreamingCancellationCleansUpWithoutWrappingOrEvents(Throwable $cancellation): void { $connection = new NeutralConnectionForTest; $connection->enableQueryLog(); @@ -1241,7 +1251,6 @@ public function testStreamingCancellationCleansUpWithoutWrappingOrEvents(): void $events->shouldNotReceive('hasListeners'); $events->shouldNotReceive('dispatch'); $connection->setEventDispatcher($events); - $cancellation = new CanceledException('stream canceled'); $cleaned = false; $thrown = null; @@ -1255,7 +1264,7 @@ public function testStreamingCancellationCleansUpWithoutWrappingOrEvents(): void $cleaned = true; } })); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $thrown = $exception; } @@ -1593,11 +1602,11 @@ public function testBeginPublicationFailureRollsBackAndPreservesTheOriginalFailu $this->assertFalse($connection->getPdo()->inTransaction()); } - public function testBeginPublicationCleanupPreservesCancellation(): void + #[DataProvider('cancellations')] + public function testBeginPublicationCleanupPreservesCancellation(Throwable $cancellation): void { $connection = $this->getSqliteTransactionConnection(); $publicationFailure = new RuntimeException('manager begin failure'); - $cancellation = new CanceledException('rollback cleanup canceled'); $manager = m::mock(DatabaseTransactionsManager::class); $manager->expects('begin')->with('default', 1)->andThrow($publicationFailure); $manager->expects('rollback')->with('default', 0)->andThrow($cancellation); @@ -1606,7 +1615,7 @@ public function testBeginPublicationCleanupPreservesCancellation(): void try { $connection->beginTransaction(); $this->fail('Expected rollback cleanup cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -1614,10 +1623,10 @@ public function testBeginPublicationCleanupPreservesCancellation(): void $this->assertFalse($connection->getPdo()->inTransaction()); } - public function testBeginPublicationCancellationRemainsPrimaryOverCleanupCancellation(): void + #[DataProvider('cancellations')] + public function testBeginPublicationCancellationRemainsPrimaryOverCleanupCancellation(Throwable $cancellation): void { $connection = $this->getSqliteTransactionConnection(); - $cancellation = new CanceledException('manager begin canceled'); $cleanupCancellation = new CanceledException('manager rollback canceled'); $manager = m::mock(DatabaseTransactionsManager::class); $manager->expects('begin')->with('default', 1)->andThrow($cancellation); @@ -1627,7 +1636,7 @@ public function testBeginPublicationCancellationRemainsPrimaryOverCleanupCancell try { $connection->beginTransaction(); $this->fail('Expected transaction publication cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -1759,11 +1768,11 @@ public function testManagedTransactionDoesNotRetryWhenRollbackCleanupFails(): vo $this->assertCount(0, $manager->getCommittedTransactions()); } - public function testManagedTransactionRollbackCleanupPreservesCancellation(): void + #[DataProvider('cancellations')] + public function testManagedTransactionRollbackCleanupPreservesCancellation(Throwable $cancellation): void { $connection = $this->getSqliteTransactionConnection(); $failure = new RuntimeException('transaction callback failure'); - $cancellation = new CanceledException('rollback cleanup canceled'); $manager = m::mock(DatabaseTransactionsManager::class); $manager->expects('begin')->with('default', 1); $manager->expects('rollback')->with('default', 0)->andThrow($cancellation); @@ -1772,7 +1781,7 @@ public function testManagedTransactionRollbackCleanupPreservesCancellation(): vo try { $connection->transaction(static fn (): never => throw $failure); $this->fail('Expected rollback cleanup cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -1780,10 +1789,10 @@ public function testManagedTransactionRollbackCleanupPreservesCancellation(): vo $this->assertFalse($connection->getPdo()->inTransaction()); } - public function testManagedTransactionCancellationRemainsPrimaryOverRollbackCancellation(): void + #[DataProvider('cancellations')] + public function testManagedTransactionCancellationRemainsPrimaryOverRollbackCancellation(Throwable $cancellation): void { $connection = $this->getSqliteTransactionConnection(); - $cancellation = new CanceledException('transaction callback canceled'); $cleanupCancellation = new CanceledException('rollback cleanup canceled'); $manager = m::mock(DatabaseTransactionsManager::class); $manager->expects('begin')->with('default', 1); @@ -1793,7 +1802,7 @@ public function testManagedTransactionCancellationRemainsPrimaryOverRollbackCanc try { $connection->transaction(static fn (): never => throw $cancellation); $this->fail('Expected transaction callback cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -1801,6 +1810,35 @@ public function testManagedTransactionCancellationRemainsPrimaryOverRollbackCanc $this->assertFalse($connection->getPdo()->inTransaction()); } + #[TestWith([false])] + #[TestWith([true])] + public function testCancellationIsNotRetriedOrWrappedWhenItsCleanupErrorMentionsDeadlock(bool $nested): void + { + $connection = $this->getSqliteTransactionConnection(); + $failure = new RuntimeException('Deadlock found when trying to get lock', previous: new CanceledException('Callback canceled.')); + $calls = 0; + if ($nested) { + $connection->beginTransaction(); + } + + try { + $connection->transaction(static function () use ($failure, &$calls): never { + ++$calls; + throw $failure; + }, attempts: 2); + $this->fail('Cancellation must stop the transaction without reclassification.'); + } catch (Throwable $exception) { + $this->assertSame($failure, $exception); + } finally { + if ($nested) { + $connection->rollBack(); + } + } + + $this->assertSame(1, $calls); + $this->assertSame(0, $connection->transactionLevel()); + } + public function testNestedDeadlockFailureRemainsPrimaryWhenRollbackCleanupFails(): void { $connection = $this->getSqliteTransactionConnection(); @@ -1844,14 +1882,14 @@ public function testNestedDeadlockFailureRemainsPrimaryWhenRollbackCleanupFails( $connection->rollBack(); } - public function testNestedDeadlockRollbackCleanupPreservesCancellation(): void + #[DataProvider('cancellations')] + public function testNestedDeadlockRollbackCleanupPreservesCancellation(Throwable $cancellation): void { $connection = new NeutralTransactionConnectionForTest( 'analytics', '', ['name' => 'analytics', 'driver' => 'http'] ); - $cancellation = new CanceledException('nested rollback cleanup canceled'); $manager = m::mock(DatabaseTransactionsManager::class); $manager->shouldReceive('begin')->twice(); $manager->expects('rollback')->with('analytics', 1)->andThrow($cancellation); @@ -1867,7 +1905,7 @@ public function testNestedDeadlockRollbackCleanupPreservesCancellation(): void try { $connection->transaction(static fn (): never => throw $failure); $this->fail('Expected nested rollback cleanup cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -1935,10 +1973,10 @@ public function testManagerCommitFailureStillDispatchesCommittedEventAfterPhysic $this->assertFalse($connection->getPdo()->inTransaction()); } - public function testManagedTransactionManagerCancellationSkipsCommittedEvent(): void + #[DataProvider('cancellations')] + public function testManagedTransactionManagerCancellationSkipsCommittedEvent(Throwable $cancellation): void { $connection = $this->getSqliteTransactionConnection(); - $cancellation = new CanceledException('manager commit canceled'); $manager = m::mock(DatabaseTransactionsManager::class); $manager->expects('begin')->with('default', 1); $manager->expects('commit')->with('default', 1, 0)->andThrow($cancellation); @@ -1952,7 +1990,7 @@ public function testManagedTransactionManagerCancellationSkipsCommittedEvent(): try { $connection->transaction(static fn (): null => null); $this->fail('Expected manager commit cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -1960,11 +1998,11 @@ public function testManagedTransactionManagerCancellationSkipsCommittedEvent(): $this->assertFalse($connection->getPdo()->inTransaction()); } - public function testManagedTransactionEventCancellationSupersedesManagerFailure(): void + #[DataProvider('cancellations')] + public function testManagedTransactionEventCancellationSupersedesManagerFailure(Throwable $cancellation): void { $connection = $this->getSqliteTransactionConnection(); $managerFailure = new RuntimeException('manager commit failure'); - $cancellation = new CanceledException('committed event canceled'); $manager = m::mock(DatabaseTransactionsManager::class); $manager->expects('begin')->with('default', 1); $manager->expects('commit')->with('default', 1, 0)->andThrow($managerFailure); @@ -1979,7 +2017,7 @@ public function testManagedTransactionEventCancellationSupersedesManagerFailure( try { $connection->transaction(static fn (): null => null); $this->fail('Expected committed event cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -1987,10 +2025,10 @@ public function testManagedTransactionEventCancellationSupersedesManagerFailure( $this->assertFalse($connection->getPdo()->inTransaction()); } - public function testExplicitCommitManagerCancellationSkipsCommittedEvent(): void + #[DataProvider('cancellations')] + public function testExplicitCommitManagerCancellationSkipsCommittedEvent(Throwable $cancellation): void { $connection = $this->getSqliteTransactionConnection(); - $cancellation = new CanceledException('manager commit canceled'); $manager = m::mock(DatabaseTransactionsManager::class); $manager->expects('begin')->with('default', 1); $manager->expects('commit')->with('default', 1, 0)->andThrow($cancellation); @@ -2004,7 +2042,7 @@ public function testExplicitCommitManagerCancellationSkipsCommittedEvent(): void try { $connection->commit(); $this->fail('Expected manager commit cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -2012,11 +2050,11 @@ public function testExplicitCommitManagerCancellationSkipsCommittedEvent(): void $this->assertFalse($connection->getPdo()->inTransaction()); } - public function testExplicitCommitEventCancellationSupersedesManagerFailure(): void + #[DataProvider('cancellations')] + public function testExplicitCommitEventCancellationSupersedesManagerFailure(Throwable $cancellation): void { $connection = $this->getSqliteTransactionConnection(); $managerFailure = new RuntimeException('manager commit failure'); - $cancellation = new CanceledException('committed event canceled'); $manager = m::mock(DatabaseTransactionsManager::class); $manager->expects('begin')->with('default', 1); $manager->expects('commit')->with('default', 1, 0)->andThrow($managerFailure); @@ -2031,7 +2069,7 @@ public function testExplicitCommitEventCancellationSupersedesManagerFailure(): v try { $connection->commit(); $this->fail('Expected committed event cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -2075,9 +2113,9 @@ public function testCommitFailureDoesNotRetryWhenRollbackCleanupFails(): void $this->assertCount(0, $manager->getCommittedTransactions()); } - public function testLostConnectionCommitCleanupPreservesCancellation(): void + #[DataProvider('cancellations')] + public function testLostConnectionCommitCleanupPreservesCancellation(Throwable $cancellation): void { - $cancellation = new CanceledException('lost connection cleanup canceled'); $pdo = $this->getMockBuilder(PDOStub::class) ->onlyMethods(['beginTransaction', 'commit']) ->getMock(); @@ -2094,7 +2132,7 @@ public function testLostConnectionCommitCleanupPreservesCancellation(): void try { $connection->transaction(static fn (): null => null); $this->fail('Expected lost connection cleanup cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -2102,9 +2140,9 @@ public function testLostConnectionCommitCleanupPreservesCancellation(): void $this->assertNull($connection->getRawPdo()); } - public function testCommitRollbackCleanupPreservesCancellationWithoutRetry(): void + #[DataProvider('cancellations')] + public function testCommitRollbackCleanupPreservesCancellationWithoutRetry(Throwable $cancellation): void { - $cancellation = new CanceledException('commit rollback cleanup canceled'); $pdo = $this->getMockBuilder(PDOStub::class) ->onlyMethods(['beginTransaction', 'commit', 'inTransaction', 'rollBack']) ->getMock(); @@ -2126,7 +2164,7 @@ public function testCommitRollbackCleanupPreservesCancellationWithoutRetry(): vo ++$callbackCalls; }, 2); $this->fail('Expected commit rollback cleanup cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -2193,10 +2231,10 @@ public function testManagerRollbackFailureStillDispatchesRolledBackEvent(): void $this->assertFalse($connection->getPdo()->inTransaction()); } - public function testExplicitRollbackManagerCancellationSkipsRolledBackEvent(): void + #[DataProvider('cancellations')] + public function testExplicitRollbackManagerCancellationSkipsRolledBackEvent(Throwable $cancellation): void { $connection = $this->getSqliteTransactionConnection(); - $cancellation = new CanceledException('manager rollback canceled'); $manager = m::mock(DatabaseTransactionsManager::class); $manager->expects('begin')->with('default', 1); $manager->expects('rollback')->with('default', 0)->andThrow($cancellation); @@ -2209,7 +2247,7 @@ public function testExplicitRollbackManagerCancellationSkipsRolledBackEvent(): v try { $connection->rollBack(); $this->fail('Expected manager rollback cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -2217,11 +2255,11 @@ public function testExplicitRollbackManagerCancellationSkipsRolledBackEvent(): v $this->assertFalse($connection->getPdo()->inTransaction()); } - public function testExplicitRollbackEventCancellationSupersedesManagerFailure(): void + #[DataProvider('cancellations')] + public function testExplicitRollbackEventCancellationSupersedesManagerFailure(Throwable $cancellation): void { $connection = $this->getSqliteTransactionConnection(); $managerFailure = new RuntimeException('manager rollback failure'); - $cancellation = new CanceledException('rolled back event canceled'); $manager = m::mock(DatabaseTransactionsManager::class); $manager->expects('begin')->with('default', 1); $manager->expects('rollback')->with('default', 0)->andThrow($managerFailure); @@ -2235,7 +2273,7 @@ public function testExplicitRollbackEventCancellationSupersedesManagerFailure(): try { $connection->rollBack(); $this->fail('Expected rolled back event cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -2267,9 +2305,9 @@ public function testRolledBackEventFailureOccursAfterManagerCleanup(): void $this->assertFalse($connection->getPdo()->inTransaction()); } - public function testLostConnectionRollbackCleanupPreservesCancellation(): void + #[DataProvider('cancellations')] + public function testLostConnectionRollbackCleanupPreservesCancellation(Throwable $cancellation): void { - $cancellation = new CanceledException('lost rollback cleanup canceled'); $pdo = $this->getMockBuilder(PDOStub::class) ->onlyMethods(['beginTransaction', 'inTransaction', 'rollBack']) ->getMock(); @@ -2288,7 +2326,7 @@ public function testLostConnectionRollbackCleanupPreservesCancellation(): void try { $connection->rollBack(); $this->fail('Expected lost rollback cleanup cancellation to be thrown.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } diff --git a/tests/Database/DatabasePdoConnectionTest.php b/tests/Database/DatabasePdoConnectionTest.php index b72303f03..f2b8225e8 100755 --- a/tests/Database/DatabasePdoConnectionTest.php +++ b/tests/Database/DatabasePdoConnectionTest.php @@ -1109,14 +1109,18 @@ public function testManagedTransactionRejectsManuallyEndingItsOwnTransaction(str }); } - #[TestWith([false])] - #[TestWith([true])] - public function testFailedSavepointCleanupPreservesErrorAndCancellationPriority(bool $cancel): void + #[TestWith([false, false])] + #[TestWith([true, false])] + #[TestWith([true, true])] + public function testFailedSavepointCleanupPreservesErrorAndCancellationPriority(bool $cancel, bool $callbackCancels): void { $failure = new PDOException('Savepoint rollback failed.'); $cleanupFailure = $cancel ? new CanceledException('Disconnect canceled.') : new RuntimeException('Disconnect failed.'); + if ($callbackCancels) { + $cleanupFailure = new RuntimeException('Rollback callback cleanup failed.', previous: $cleanupFailure); + } $pdo = $this->getMockBuilder(PDOStub::class) ->onlyMethods(['beginTransaction', 'exec', 'inTransaction', 'rollBack'])->getMock(); $pdo->expects($this->once())->method('beginTransaction'); @@ -1128,13 +1132,19 @@ public function testFailedSavepointCleanupPreservesErrorAndCancellationPriority( return 0; }); $pdo->expects($this->once())->method('inTransaction')->willReturn(true); - $pdo->expects($this->once())->method('rollBack')->willThrowException($cleanupFailure); + $pdo->expects($this->once())->method('rollBack')->willThrowException( + $callbackCancels ? new RuntimeException('Physical rollback failed.') : $cleanupFailure + ); $connection = $this->getMockConnection([], $pdo); $manager = new DatabaseTransactionsManager; $connection->setTransactionManager($manager); $connection->beginTransaction(); $connection->beginTransaction(); + if ($callbackCancels) { + $connection->afterRollBack(static fn (): never => throw $cleanupFailure); + } + try { $connection->rollBack(1); $this->fail('Rollback failure must propagate.'); diff --git a/tests/Database/DatabaseTransactionsManagerTest.php b/tests/Database/DatabaseTransactionsManagerTest.php index bd6455532..6e32aa9f4 100755 --- a/tests/Database/DatabaseTransactionsManagerTest.php +++ b/tests/Database/DatabaseTransactionsManagerTest.php @@ -10,6 +10,7 @@ use Hypervel\Database\DatabaseTransactionsManager; use Hypervel\Engine\Channel; use Hypervel\Tests\TestCase; +use PHPUnit\Framework\Attributes\TestWith; use RuntimeException; use Swoole\Coroutine\CanceledException; use Throwable; @@ -574,11 +575,16 @@ public function testRollbackExecutesCallbacksForCommittedSavepointsWhenOuterRoll $this->assertCount(0, $manager->getCommittedTransactions()); } - public function testRollbackCallbacksFinishTheBoundedDrainAndPreserveCancellation(): void + #[TestWith([false])] + #[TestWith([true])] + public function testRollbackCallbacksFinishTheBoundedDrainAndPreserveCancellation(bool $chained): void { $manager = new DatabaseTransactionsManager; $callbacks = []; $cancellation = new CanceledException; + if ($chained) { + $cancellation = new RuntimeException('Rollback cleanup failed.', previous: $cancellation); + } $manager->begin('default', 1); $manager->addCallbackForRollback(function () use (&$callbacks): void { @@ -603,7 +609,7 @@ public function testRollbackCallbacksFinishTheBoundedDrainAndPreserveCancellatio try { $manager->rollback('default', 0); $this->fail('Expected rollback cancellation to escape.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -719,11 +725,16 @@ public function testCommitCallbacksExhaustEveryRecordAndPreserveTheEarliestFailu $this->assertCount(0, $manager->getCommittedTransactions()); } - public function testCommitCancellationStopsTheCurrentAndRemainingTransactionRecords(): void + #[TestWith([false])] + #[TestWith([true])] + public function testCommitCancellationStopsTheCurrentAndRemainingTransactionRecords(bool $chained): void { $manager = new DatabaseTransactionsManager; $callbacks = []; $cancellation = new CanceledException; + if ($chained) { + $cancellation = new RuntimeException('Commit cleanup failed.', previous: $cancellation); + } $manager->begin('default', 1); $manager->addCallback(function () use (&$callbacks): void { @@ -747,7 +758,7 @@ public function testCommitCancellationStopsTheCurrentAndRemainingTransactionReco try { $manager->commit('default', 1, 0); $this->fail('Expected commit cancellation to escape.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } From 89189f235934d4a7d576737b2adcd3fcd7694478 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Thu, 29 Oct 2026 17:33:39 +0000 Subject: [PATCH 16/29] Clear Eloquent cast caches before synchronizing raw replacements Prevent cached class casts and Attribute objects from overwriting newly supplied raw attributes when setRawAttributes synchronizes originals. Clear both caches before syncOriginal while preserving unsynchronized replacement behavior and public signatures. Cover both cache paths with synchronized and unsynchronized regression cases. Update the framework plan and its completion status. Validated formatting, all static-analysis stages, 356 affected Eloquent tests and 116 permission cache and registrar tests on the combined working state. --- ...0911-framework-correctness-improvements.md | 7 +++- .../src/Eloquent/Concerns/HasAttributes.php | 7 ++-- .../DatabaseConcernsHasAttributesTest.php | 40 +++++++++++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md index 90cb290ed..3aee23438 100644 --- a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md +++ b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md @@ -4,7 +4,7 @@ Independently useful database, queue, validation, decimal-casting and JSON-decoding improvements. Keep this plan independent of private applications and packages; describe framework requirements without their names, paths, or domain details. Preserve Laravel signatures and protected extension points, except for explicitly approved behavior changes. -The database capability, query-builder, queue middleware, validation, decimal-casting and pooled in-memory database restoration changes are implemented and verified; merging remains. Record further framework changes here when they warrant planning, without duplicating consumer-specific implementation plans. +All changes below are implemented and verified; merging remains. Record further framework changes here when they warrant planning, without duplicating consumer-specific implementation plans. ## Design @@ -19,6 +19,11 @@ The database capability, query-builder, queue middleware, validation, decimal-ca - Let `UseEloquentBuilder` accept `class-string>`, matching the existing builder property and factory return type. The attribute accepts builders bound to a particular model; `Builder` incorrectly excludes them because the model parameter is invariant. Align the resolved-class cache and resolver return annotation with the same boundary. Runtime construction and public signatures remain unchanged. - Cover the attribute with a model-bounded generic builder and `HasBuilder`, including a model subclass and a terminal query result in the type fixtures. +### Raw attribute replacement + +- Clear both Eloquent cast-value caches before `setRawAttributes()` optionally calls `syncOriginal()`. Synchronization reads attributes through the cached casts; clearing afterwards lets stale objects overwrite the replacement values and marks those wrong values clean. Preserve the signature and unsynchronized replacement behavior, and explain the intentional ordering difference beside the code. +- Use one regression parameterized over synchronization, covering both `AsArrayObject` and an `Attribute` getter/setter object. Check replacement values, raw originals and dirty state without database setup. + ### Immutable date modifier types - Keep `CarbonImmutable::addMonths()` metadata typed as `static`, including its optional anchor-day argument. Carbon preserves subclasses at runtime; inherited metadata naming the base class incorrectly widens fluent results. Extend the existing modifier type fixture and reuse the runtime metadata/subclass test. No runtime or API behavior changes. diff --git a/src/database/src/Eloquent/Concerns/HasAttributes.php b/src/database/src/Eloquent/Concerns/HasAttributes.php index 28f3ae9f1..11f9201f3 100644 --- a/src/database/src/Eloquent/Concerns/HasAttributes.php +++ b/src/database/src/Eloquent/Concerns/HasAttributes.php @@ -2015,13 +2015,14 @@ public function setRawAttributes(array $attributes, bool $sync = false): static { $this->attributes = $attributes; + // Laravel clears these after syncing, which merges stale cast objects over the replacement values. + $this->classCastCache = []; + $this->attributeCastCache = []; + if ($sync) { $this->syncOriginal(); } - $this->classCastCache = []; - $this->attributeCastCache = []; - return $this; } diff --git a/tests/Database/DatabaseConcernsHasAttributesTest.php b/tests/Database/DatabaseConcernsHasAttributesTest.php index f48b6fe41..f4b2c2c93 100644 --- a/tests/Database/DatabaseConcernsHasAttributesTest.php +++ b/tests/Database/DatabaseConcernsHasAttributesTest.php @@ -4,12 +4,15 @@ namespace Hypervel\Tests\Database; +use ArrayObject; +use Hypervel\Database\Eloquent\Casts\AsArrayObject; use Hypervel\Database\Eloquent\Casts\Attribute; use Hypervel\Database\Eloquent\Concerns\HasAttributes; use Hypervel\Database\Eloquent\Model; use Hypervel\Support\Collection; use Hypervel\Tests\TestCase; use Mockery as m; +use PHPUnit\Framework\Attributes\TestWith; class DatabaseConcernsHasAttributesTest extends TestCase { @@ -66,6 +69,43 @@ public function testUnsettingCachedAttribute(): void $this->assertFalse($instance->cachedAttributeIsset('cacheableProperty')); } + + #[TestWith([false])] + #[TestWith([true])] + public function testReplacingRawAttributesDiscardsCachedCastValues(bool $sync): void + { + $instance = new HasAttributesWithCachedObjects; + $original = ['payload' => '{"days":30}', 'settings' => '{"days":30}']; + $replacement = ['payload' => '{"days":15}', 'settings' => '{"days":15}']; + $instance->setRawAttributes($original, true); + + $this->assertSame(30, $instance->getAttribute('payload')['days']); + $this->assertSame(30, $instance->getAttribute('settings')['days']); + + $instance->setRawAttributes($replacement, $sync); + + $this->assertSame($replacement, $instance->getAttributes()); + $this->assertSame(15, $instance->getAttribute('payload')['days']); + $this->assertSame(15, $instance->getAttribute('settings')['days']); + $this->assertSame($sync ? $replacement : $original, $instance->getRawOriginal()); + $this->assertSame(! $sync, $instance->isDirty()); + } +} + +class HasAttributesWithCachedObjects extends Model +{ + protected array $casts = ['payload' => AsArrayObject::class]; + + /** + * Get the cached settings object. + */ + protected function settings(): Attribute + { + return Attribute::make( + get: static fn (string $value): ArrayObject => new ArrayObject(json_decode($value, true, flags: JSON_THROW_ON_ERROR)), + set: static fn (ArrayObject $value): string => json_encode($value->getArrayCopy(), JSON_THROW_ON_ERROR), + ); + } } class HasAttributesWithoutConstructor From b605c54219f0318488e55f1a15eea983c1fb8e15 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Fri, 30 Oct 2026 04:08:59 +0000 Subject: [PATCH 17/29] Correct constrained relationship loader types Describe keyed eager constraints, aggregate Builder callbacks and class-keyed morph maps across models, collections and paginators. Include fresh() forwarding and callable eager array inputs while preserving native signatures and runtime behavior. Adapt morph type fixtures to supported inputs and verify callback inference, invokable constraints and fluent returns. Source, type-fixture and skeleton analysis pass; affected Eloquent tests and subscription consumer tests pass on the combined working state. --- ...0911-framework-correctness-improvements.md | 6 +++ src/database/src/Eloquent/Builder.php | 4 +- src/database/src/Eloquent/Collection.php | 20 +++---- src/database/src/Eloquent/Model.php | 36 ++++++------- .../src/AbstractCursorPaginator.php | 6 ++- src/pagination/src/AbstractPaginator.php | 6 ++- types/Database/Eloquent/Collection.php | 39 ++++++++------ types/Database/Eloquent/Model.php | 54 +++++++++++++++++++ types/Pagination/Paginator.php | 19 ++++++- 9 files changed, 138 insertions(+), 52 deletions(-) diff --git a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md index 3aee23438..ebf35a93e 100644 --- a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md +++ b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md @@ -19,6 +19,12 @@ All changes below are implemented and verified; merging remains. Record further - Let `UseEloquentBuilder` accept `class-string>`, matching the existing builder property and factory return type. The attribute accepts builders bound to a particular model; `Builder` incorrectly excludes them because the model parameter is invariant. Align the resolved-class cache and resolver return annotation with the same boundary. Runtime construction and public signatures remain unchanged. - Cover the attribute with a model-bounded generic builder and `HasBuilder`, including a model subclass and a terminal query result in the type fixtures. +### Constrained relationship-loading types + +- Correct model loading annotations to accept keyed constraints and nested relation specifications already supported at runtime, including model and collection `fresh()` and both paginator morph-loader families. Eager callbacks receive `Relation`; aggregate callbacks receive the related Eloquent `Builder`. Align the forwarding methods without changing native signatures or execution. +- Morph loaders take a class-keyed map whose values are relation specifications or strings. Adapt the existing collection morph type fixtures to that supported outer shape, preserving their inner cases. Keep aggregate input arrays broad; this correction does not add nested aggregate relationship support or change existing failure behavior. +- Extend existing model and collection type fixtures to verify constrained inputs, callback types and fluent returns. Keep the separate `Builder::with()` callback parameter's native `Closure` type; its array form supports callable constraints. Reuse existing aggregate callback runtime tests rather than adding duplicate cases. Run source and fixture analysis and affected database tests. + ### Raw attribute replacement - Clear both Eloquent cast-value caches before `setRawAttributes()` optionally calls `syncOriginal()`. Synchronization reads attributes through the cached casts; clearing afterwards lets stale objects overwrite the replacement values and marks those wrong values clean. Preserve the signature and unsynchronized replacement behavior, and explain the intentional ordering difference beside the code. diff --git a/src/database/src/Eloquent/Builder.php b/src/database/src/Eloquent/Builder.php index 52821cc82..16ce5a216 100644 --- a/src/database/src/Eloquent/Builder.php +++ b/src/database/src/Eloquent/Builder.php @@ -1676,7 +1676,7 @@ protected function createNestedWhere(array $whereSlice, string $boolean = 'and') /** * Specify relationships that should be eager loaded. * - * @param array): mixed)|string>|string $relations + * @param array): mixed)|string>|string $relations * @param (Closure(Relation<*,*,*>): mixed)|string|null $callback */ public function with(array|string $relations, Closure|string|null $callback = null): static @@ -1707,7 +1707,7 @@ public function without(mixed $relations): static /** * Set the relationships that should be eager loaded while removing any previously added eager loading specifications. * - * @param array): mixed)|string>|string $relations + * @param array): mixed)|string>|string $relations */ public function withOnly(array|string $relations): static { diff --git a/src/database/src/Eloquent/Collection.php b/src/database/src/Eloquent/Collection.php index c8c164a8e..a9cf0f196 100644 --- a/src/database/src/Eloquent/Collection.php +++ b/src/database/src/Eloquent/Collection.php @@ -127,7 +127,7 @@ public function load(array|string $relations): static /** * Load a set of aggregations over relationship's column onto the collection. * - * @param array): mixed)|string>|string $relations + * @param array): mixed)|string>|string $relations * * @throws MissingAttributeException */ @@ -189,7 +189,7 @@ public function loadAggregate(array|string $relations, Expression|string $column /** * Load a set of relationship counts onto the collection. * - * @param array): mixed)|string>|string $relations + * @param array): mixed)|string>|string $relations */ public function loadCount(array|string $relations): static { @@ -199,7 +199,7 @@ public function loadCount(array|string $relations): static /** * Load a set of relationship's max column values onto the collection. * - * @param array): mixed)|string>|string $relations + * @param array): mixed)|string>|string $relations */ public function loadMax(array|string $relations, Expression|string $column): static { @@ -209,7 +209,7 @@ public function loadMax(array|string $relations, Expression|string $column): sta /** * Load a set of relationship's min column values onto the collection. * - * @param array): mixed)|string>|string $relations + * @param array): mixed)|string>|string $relations */ public function loadMin(array|string $relations, Expression|string $column): static { @@ -219,7 +219,7 @@ public function loadMin(array|string $relations, Expression|string $column): sta /** * Load a set of relationship's column summations onto the collection. * - * @param array): mixed)|string>|string $relations + * @param array): mixed)|string>|string $relations */ public function loadSum(array|string $relations, Expression|string $column): static { @@ -229,7 +229,7 @@ public function loadSum(array|string $relations, Expression|string $column): sta /** * Load a set of relationship's average column values onto the collection. * - * @param array): mixed)|string>|string $relations + * @param array): mixed)|string>|string $relations */ public function loadAvg(array|string $relations, Expression|string $column): static { @@ -239,7 +239,7 @@ public function loadAvg(array|string $relations, Expression|string $column): sta /** * Load a set of related existences onto the collection. * - * @param array): mixed)|string>|string $relations + * @param array): mixed)|string>|string $relations */ public function loadExists(array|string $relations): static { @@ -347,7 +347,7 @@ protected function loadMissingRelation(self $models, array $path): void /** * Load a set of relationships onto the mixed relationship collection. * - * @param array): mixed)|string> $relations + * @param array): mixed)|string>|string> $relations */ public function loadMorph(string $relation, array $relations): static { @@ -362,7 +362,7 @@ public function loadMorph(string $relation, array $relations): static /** * Load a set of relationship counts onto the mixed relationship collection. * - * @param array): mixed)|string> $relations + * @param array): mixed)|string>|string> $relations */ public function loadMorphCount(string $relation, array $relations): static { @@ -485,7 +485,7 @@ public function mapWithKeys(callable $callback): BaseCollection /** * Reload a fresh model instance from the database for all the entities. * - * @param array|string $with + * @param array): mixed)|string>|string $with * * @throws MissingAttributeException */ diff --git a/src/database/src/Eloquent/Model.php b/src/database/src/Eloquent/Model.php index f681a5f93..00a81c0a7 100644 --- a/src/database/src/Eloquent/Model.php +++ b/src/database/src/Eloquent/Model.php @@ -972,7 +972,7 @@ public static function all(array|string $columns = ['*']): Collection /** * Begin querying a model with eager loading. * - * @param array|string $relations + * @param array): mixed)|string>|string $relations * @return Builder */ public static function with(array|string $relations): Builder @@ -985,7 +985,7 @@ public static function with(array|string $relations): Builder /** * Eager load relations on the model. * - * @param array|string $relations + * @param array): mixed)|string>|string $relations */ public function load(array|string $relations): static { @@ -1001,7 +1001,7 @@ public function load(array|string $relations): static /** * Eager load relationships on the polymorphic relation of a model. * - * @param array> $relations + * @param array): mixed)|string>|string> $relations */ public function loadMorph(string $relation, array $relations): static { @@ -1019,7 +1019,7 @@ public function loadMorph(string $relation, array $relations): static /** * Eager load relations on the model if they are not already eager loaded. * - * @param array|string $relations + * @param array): mixed)|string>|string $relations */ public function loadMissing(array|string $relations): static { @@ -1033,7 +1033,7 @@ public function loadMissing(array|string $relations): static /** * Eager load relation's column aggregations on the model. * - * @param array|string $relations + * @param array): mixed)|string>|string $relations */ public function loadAggregate(array|string $relations, Expression|string $column, ?string $function = null): static { @@ -1045,7 +1045,7 @@ public function loadAggregate(array|string $relations, Expression|string $column /** * Eager load relation counts on the model. * - * @param array|string $relations + * @param array): mixed)|string>|string $relations */ public function loadCount(array|string $relations): static { @@ -1057,7 +1057,7 @@ public function loadCount(array|string $relations): static /** * Eager load relation max column values on the model. * - * @param array|string $relations + * @param array): mixed)|string>|string $relations */ public function loadMax(array|string $relations, Expression|string $column): static { @@ -1067,7 +1067,7 @@ public function loadMax(array|string $relations, Expression|string $column): sta /** * Eager load relation min column values on the model. * - * @param array|string $relations + * @param array): mixed)|string>|string $relations */ public function loadMin(array|string $relations, Expression|string $column): static { @@ -1077,7 +1077,7 @@ public function loadMin(array|string $relations, Expression|string $column): sta /** * Eager load relation's column summations on the model. * - * @param array|string $relations + * @param array): mixed)|string>|string $relations */ public function loadSum(array|string $relations, Expression|string $column): static { @@ -1087,7 +1087,7 @@ public function loadSum(array|string $relations, Expression|string $column): sta /** * Eager load relation average column values on the model. * - * @param array|string $relations + * @param array): mixed)|string>|string $relations */ public function loadAvg(array|string $relations, Expression|string $column): static { @@ -1097,7 +1097,7 @@ public function loadAvg(array|string $relations, Expression|string $column): sta /** * Eager load related model existence values on the model. * - * @param array|string $relations + * @param array): mixed)|string>|string $relations */ public function loadExists(array|string $relations): static { @@ -1107,7 +1107,7 @@ public function loadExists(array|string $relations): static /** * Eager load relationship column aggregation on the polymorphic relation of a model. * - * @param array> $relations + * @param array): mixed)|string>|string> $relations */ public function loadMorphAggregate(string $relation, array $relations, Expression|string $column, ?string $function = null): static { @@ -1125,7 +1125,7 @@ public function loadMorphAggregate(string $relation, array $relations, Expressio /** * Eager load relationship counts on the polymorphic relation of a model. * - * @param array> $relations + * @param array): mixed)|string>|string> $relations */ public function loadMorphCount(string $relation, array $relations): static { @@ -1135,7 +1135,7 @@ public function loadMorphCount(string $relation, array $relations): static /** * Eager load relationship max column values on the polymorphic relation of a model. * - * @param array> $relations + * @param array): mixed)|string>|string> $relations */ public function loadMorphMax(string $relation, array $relations, Expression|string $column): static { @@ -1145,7 +1145,7 @@ public function loadMorphMax(string $relation, array $relations, Expression|stri /** * Eager load relationship min column values on the polymorphic relation of a model. * - * @param array> $relations + * @param array): mixed)|string>|string> $relations */ public function loadMorphMin(string $relation, array $relations, Expression|string $column): static { @@ -1155,7 +1155,7 @@ public function loadMorphMin(string $relation, array $relations, Expression|stri /** * Eager load relationship column summations on the polymorphic relation of a model. * - * @param array> $relations + * @param array): mixed)|string>|string> $relations */ public function loadMorphSum(string $relation, array $relations, Expression|string $column): static { @@ -1165,7 +1165,7 @@ public function loadMorphSum(string $relation, array $relations, Expression|stri /** * Eager load relationship average column values on the polymorphic relation of a model. * - * @param array> $relations + * @param array): mixed)|string>|string> $relations */ public function loadMorphAvg(string $relation, array $relations, Expression|string $column): static { @@ -2257,7 +2257,7 @@ public function jsonSerialize(): mixed /** * Reload a fresh model instance from the database. * - * @param array|string $with + * @param array): mixed)|string>|string $with */ public function fresh(array|string $with = []): ?static { diff --git a/src/pagination/src/AbstractCursorPaginator.php b/src/pagination/src/AbstractCursorPaginator.php index 420394512..81eba3c82 100644 --- a/src/pagination/src/AbstractCursorPaginator.php +++ b/src/pagination/src/AbstractCursorPaginator.php @@ -10,8 +10,10 @@ use Exception; use Hypervel\Contracts\Support\Htmlable; use Hypervel\Contracts\View\Factory; +use Hypervel\Database\Eloquent\Builder; use Hypervel\Database\Eloquent\Model; use Hypervel\Database\Eloquent\Relations\Pivot; +use Hypervel\Database\Eloquent\Relations\Relation; use Hypervel\Http\Resources\Json\JsonResource; use Hypervel\Support\Arr; use Hypervel\Support\Collection; @@ -360,7 +362,7 @@ protected function buildFragment(): string /** * Load a set of relationships onto the mixed relationship collection. * - * @param array> $relations + * @param array): mixed)|string>|string> $relations * @return $this */ public function loadMorph(string $relation, array $relations): static @@ -374,7 +376,7 @@ public function loadMorph(string $relation, array $relations): static /** * Load a set of relationship counts onto the mixed relationship collection. * - * @param array> $relations + * @param array): mixed)|string>|string> $relations * @return $this */ public function loadMorphCount(string $relation, array $relations): static diff --git a/src/pagination/src/AbstractPaginator.php b/src/pagination/src/AbstractPaginator.php index fb02d12f2..d6b15e0f8 100644 --- a/src/pagination/src/AbstractPaginator.php +++ b/src/pagination/src/AbstractPaginator.php @@ -9,6 +9,8 @@ use Hypervel\Contracts\Support\CanBeEscapedWhenCastToString; use Hypervel\Contracts\Support\Htmlable; use Hypervel\Contracts\View\Factory; +use Hypervel\Database\Eloquent\Builder; +use Hypervel\Database\Eloquent\Relations\Relation; use Hypervel\Support\Arr; use Hypervel\Support\Collection; use Hypervel\Support\Traits\ForwardsCalls; @@ -284,7 +286,7 @@ protected function buildFragment(): string /** * Load a set of relationships onto the mixed relationship collection. * - * @param array> $relations + * @param array): mixed)|string>|string> $relations * @return $this */ public function loadMorph(string $relation, array $relations): static @@ -298,7 +300,7 @@ public function loadMorph(string $relation, array $relations): static /** * Load a set of relationship counts onto the mixed relationship collection. * - * @param array> $relations + * @param array): mixed)|string>|string> $relations * @return $this */ public function loadMorphCount(string $relation, array $relations): static diff --git a/types/Database/Eloquent/Collection.php b/types/Database/Eloquent/Collection.php index 0af60e847..6e57ad253 100644 --- a/types/Database/Eloquent/Collection.php +++ b/types/Database/Eloquent/Collection.php @@ -36,42 +36,42 @@ assertType('Hypervel\Database\Eloquent\Collection', $collection->loadAggregate(['string' => ['foo' => fn ($q) => $q]], 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadAggregate(['string'], 'string', 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadAggregate(['string' => function ($query) { - // assertType('Hypervel\Database\Eloquent\Relations\Relation<*,*,*>', $query); + assertType('Hypervel\Database\Eloquent\Builder<*>', $query); }], 'string', 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadCount('string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadCount(['string'])); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadCount(['string' => ['foo' => fn ($q) => $q]])); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadCount(['string' => function ($query) { - // assertType('Hypervel\Database\Eloquent\Relations\Relation<*,*,*>', $query); + assertType('Hypervel\Database\Eloquent\Builder<*>', $query); }])); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMax('string', 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMax(['string'], 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMax(['string' => ['foo' => fn ($q) => $q]], 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMax(['string' => function ($query) { - // assertType('Hypervel\Database\Eloquent\Relations\Relation<*,*,*>', $query); + assertType('Hypervel\Database\Eloquent\Builder<*>', $query); }], 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMin('string', 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMin(['string'], 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMin(['string' => ['foo' => fn ($q) => $q]], 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMin(['string' => function ($query) { - // assertType('Hypervel\Database\Eloquent\Relations\Relation<*,*,*>', $query); + assertType('Hypervel\Database\Eloquent\Builder<*>', $query); }], 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadSum('string', 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadSum(['string'], 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadSum(['string' => ['foo' => fn ($q) => $q]], 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadSum(['string' => function ($query) { - // assertType('Hypervel\Database\Eloquent\Relations\Relation<*,*,*>', $query); + assertType('Hypervel\Database\Eloquent\Builder<*>', $query); }], 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadAvg('string', 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadAvg(['string'], 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadAvg(['string' => ['foo' => fn ($q) => $q]], 'string')); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadAvg(['string' => function ($query) { - // assertType('Hypervel\Database\Eloquent\Relations\Relation<*,*,*>', $query); + assertType('Hypervel\Database\Eloquent\Builder<*>', $query); }], 'string')); /** @@ -92,7 +92,7 @@ function assertEloquentCollectionAggregateExpressionTypes(Collection $collection assertType('Hypervel\Database\Eloquent\Collection', $collection->loadExists(['string'])); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadExists(['string' => ['foo' => fn ($q) => $q]])); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadExists(['string' => function ($query) { - // assertType('Hypervel\Database\Eloquent\Relations\Relation<*,*,*>', $query); + assertType('Hypervel\Database\Eloquent\Builder<*>', $query); }])); assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMissing('string')); @@ -102,17 +102,19 @@ function assertEloquentCollectionAggregateExpressionTypes(Collection $collection // assertType('Hypervel\Database\Eloquent\Relations\Relation<*,*,*>', $query); }])); -assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMorph('string', ['string'])); -assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMorph('string', ['string' => ['foo' => fn ($q) => $q]])); -assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMorph('string', ['string' => function ($query) { - // assertType('Hypervel\Database\Eloquent\Relations\Relation<*,*,*>', $query); -}])); +assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMorph('string', [User::class => ['string']])); +assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMorph('string', [User::class => ['string' => ['foo' => fn ($q) => $q]]])); +assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMorph('string', [User::class => ['string' => function ($query) { + assertType('Hypervel\Database\Eloquent\Relations\Relation<*, *, *>', $query); +}]])); +assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMorph('string', [User::class => 'string'])); -assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMorphCount('string', ['string'])); -assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMorphCount('string', ['string' => ['foo' => fn ($q) => $q]])); -assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMorphCount('string', ['string' => function ($query) { - // assertType('Hypervel\Database\Eloquent\Relations\Relation<*,*,*>', $query); -}])); +assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMorphCount('string', [User::class => ['string']])); +assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMorphCount('string', [User::class => ['foo' => fn ($q) => $q]])); +assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMorphCount('string', [User::class => ['string' => function ($query) { + assertType('Hypervel\Database\Eloquent\Builder<*>', $query); +}]])); +assertType('Hypervel\Database\Eloquent\Collection', $collection->loadMorphCount('string', [User::class => 'string'])); assertType('bool', $collection->contains(function ($user) { assertType('User', $user); @@ -179,6 +181,9 @@ function assertEloquentCollectionAggregateExpressionTypes(Collection $collection 'Hypervel\Database\Eloquent\Collection', $collection->fresh(['string']) ); +assertType('Hypervel\Database\Eloquent\Collection', $collection->fresh(['string' => function ($relation) { + assertType('Hypervel\Database\Eloquent\Relations\Relation<*, *, *>', $relation); +}])); assertType('Hypervel\Database\Eloquent\Collection', $collection->diff($collection)); assertType('Hypervel\Database\Eloquent\Collection', $collection->diff([new User])); diff --git a/types/Database/Eloquent/Model.php b/types/Database/Eloquent/Model.php index 93e61eb83..74c907793 100644 --- a/types/Database/Eloquent/Model.php +++ b/types/Database/Eloquent/Model.php @@ -6,9 +6,11 @@ use Hypervel\Contracts\Database\Query\Expression; use Hypervel\Database\Eloquent\Attributes\CollectedBy; +use Hypervel\Database\Eloquent\Builder; use Hypervel\Database\Eloquent\Collection; use Hypervel\Database\Eloquent\HasCollection; use Hypervel\Database\Eloquent\Model; +use Hypervel\Database\Eloquent\Relations\Relation; use Hypervel\Notifications\DatabaseNotification; use User; @@ -41,6 +43,39 @@ function test(User $user, Post $post, Comment $comment, Article $article, Databa assertType('Hypervel\Database\Eloquent\Builder', User::query()); assertType('Hypervel\Database\Eloquent\Builder', $user->newQuery()); + assertType('Hypervel\Database\Eloquent\Builder', User::with(['posts' => function ($relation) { + assertType('Hypervel\Database\Eloquent\Relations\Relation<*, *, *>', $relation); + }])); + assertType('User', $user->load(['posts' => function ($relation) { + assertType('Hypervel\Database\Eloquent\Relations\Relation<*, *, *>', $relation); + }])); + assertType('User', $user->loadMissing(['posts' => function ($relation) { + assertType('Hypervel\Database\Eloquent\Relations\Relation<*, *, *>', $relation); + }])); + assertType('User|null', $user->fresh(['posts' => function ($relation) { + assertType('Hypervel\Database\Eloquent\Relations\Relation<*, *, *>', $relation); + }])); + assertType('User', $user->load(['posts' => ['comments' => fn ($relation) => $relation]])); + assertType('User', $user->loadMorph('parentable', [Post::class => ['comments' => function ($relation) { + assertType('Hypervel\Database\Eloquent\Relations\Relation<*, *, *>', $relation); + }]])); + assertType('User', $user->loadMorph('parentable', [Post::class => 'comments'])); + + $constraint = new class { + /** + * Constrain eager loading through an invokable object. + * + * @param Relation<*, *, *> $relation + */ + public function __invoke(Relation $relation): void + { + $relation->where('active', true); + } + }; + assertType('Hypervel\Database\Eloquent\Builder', User::with(['posts' => $constraint])); + assertType('Hypervel\Database\Eloquent\Builder', $user->newQuery()->withOnly(['posts' => $constraint])); + assertType('User', $user->load(['posts' => $constraint])); + assertType('User', $user->loadMissing(['posts' => $constraint])); assertType('Hypervel\Database\Eloquent\Builder', $user->withTrashed()); assertType('Hypervel\Database\Eloquent\Builder', $user->onlyTrashed()); assertType('Hypervel\Database\Eloquent\Builder', $user->withoutTrashed()); @@ -65,6 +100,25 @@ function test(User $user, Post $post, Comment $comment, Article $article, Databa assertType('User', $user->loadMorphMin('parentable', [Post::class => ['comments']], $expression)); assertType('User', $user->loadMorphSum('parentable', [Post::class => ['comments']], $expression)); assertType('User', $user->loadMorphAvg('parentable', [Post::class => ['comments']], $expression)); + assertType('User', $user->loadCount(['posts' => function ($query) { + assertType('Hypervel\Database\Eloquent\Builder<*>', $query); + }])); + assertType('User', $user->loadMorphCount('parentable', [Post::class => ['comments' => function ($query) { + assertType('Hypervel\Database\Eloquent\Builder<*>', $query); + }]])); + $aggregate = static fn (Builder $query): Builder => $query->where('active', true); + assertType('User', $user->loadAggregate(['posts' => $aggregate], $expression, 'sum')); + assertType('User', $user->loadMax(['posts' => $aggregate], $expression)); + assertType('User', $user->loadMin(['posts' => $aggregate], $expression)); + assertType('User', $user->loadSum(['posts' => $aggregate], $expression)); + assertType('User', $user->loadAvg(['posts' => $aggregate], $expression)); + assertType('User', $user->loadExists(['posts' => $aggregate])); + assertType('User', $user->loadMorphAggregate('parentable', [Post::class => ['comments' => $aggregate]], $expression, 'sum')); + assertType('User', $user->loadMorphMax('parentable', [Post::class => ['comments' => $aggregate]], $expression)); + assertType('User', $user->loadMorphMin('parentable', [Post::class => ['comments' => $aggregate]], $expression)); + assertType('User', $user->loadMorphSum('parentable', [Post::class => ['comments' => $aggregate]], $expression)); + assertType('User', $user->loadMorphAvg('parentable', [Post::class => ['comments' => $aggregate]], $expression)); + assertType('User', $user->loadMorphCount('parentable', [Post::class => 'comments'])); assertType('bool', $user->restore()); assertType('User', $user->restoreOrCreate()); diff --git a/types/Pagination/Paginator.php b/types/Pagination/Paginator.php index a6a63dd89..be1ef8c48 100644 --- a/types/Pagination/Paginator.php +++ b/types/Pagination/Paginator.php @@ -2,6 +2,7 @@ declare(strict_types=1); +use Hypervel\Database\Eloquent\Collection as EloquentCollection; use Hypervel\Pagination\CursorPaginator; use Hypervel\Pagination\LengthAwarePaginator; use Hypervel\Pagination\Paginator; @@ -9,7 +10,7 @@ use function PHPStan\Testing\assertType; -$items = [new Post, new Post, new Post]; +$items = new EloquentCollection([new Post, new Post, new Post]); /** @var Paginator $paginator */ $paginator = new Paginator($items, 1, 1); @@ -19,6 +20,14 @@ assertType('string|null', $paginator->fragment()); assertType('Hypervel\Pagination\Paginator', $paginator->fragment('foo')); +assertType('Hypervel\Pagination\Paginator', $paginator->loadMorph('parentable', [Post::class => ['comments' => function ($relation) { + assertType('Hypervel\Database\Eloquent\Relations\Relation<*, *, *>', $relation); +}]])); +assertType('Hypervel\Pagination\Paginator', $paginator->loadMorphCount('parentable', [Post::class => ['comments' => function ($query) { + assertType('Hypervel\Database\Eloquent\Builder<*>', $query); +}]])); +assertType('Hypervel\Pagination\Paginator', $paginator->loadMorph('parentable', [Post::class => 'comments'])); +assertType('Hypervel\Pagination\Paginator', $paginator->loadMorphCount('parentable', [Post::class => 'comments'])); $paginator->each(function ($post) { assertType('Post', $post); @@ -57,6 +66,14 @@ assertType('string|null', $cursorPaginator->fragment()); assertType('Hypervel\Pagination\CursorPaginator', $cursorPaginator->fragment('foo')); +assertType('Hypervel\Pagination\CursorPaginator', $cursorPaginator->loadMorph('parentable', [Post::class => ['comments' => function ($relation) { + assertType('Hypervel\Database\Eloquent\Relations\Relation<*, *, *>', $relation); +}]])); +assertType('Hypervel\Pagination\CursorPaginator', $cursorPaginator->loadMorphCount('parentable', [Post::class => ['comments' => function ($query) { + assertType('Hypervel\Database\Eloquent\Builder<*>', $query); +}]])); +assertType('Hypervel\Pagination\CursorPaginator', $cursorPaginator->loadMorph('parentable', [Post::class => 'comments'])); +assertType('Hypervel\Pagination\CursorPaginator', $cursorPaginator->loadMorphCount('parentable', [Post::class => 'comments'])); $cursorPaginator->each(function ($post) { assertType('Post', $post); From a60fb5eb6475433d0cc122ca7a00429e4b465402 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:31:13 +0000 Subject: [PATCH 18/29] Honor write index hints and keep outbox claims bounded Compile explicit MySQL and MariaDB index hints on supported DELETE forms, preserving target aliases, table prefixes and join bindings. Keep unhinted SQL and ordered or limited unjoined deletes unchanged. Apply SQLite forced indexes to plain deletes and retain its existing selection subqueries for joined writes and writes with limits or offsets. Share the write-table hint compilation with UPDATE and explain the intentional Laravel difference beside the grammars. Document missing-index errors and the important limit that an ID-only delete or locking read can still choose a covering scan. Hints are not a universal lock-confinement guarantee; callers must preserve real ownership and eligibility predicates. Select only outbox IDs and original eligibility during InnoDB claims so the due index can satisfy the locking read without sorting and locking rows beyond the batch. Update the selected IDs, then read their message bodies through the primary key while retaining the lease predicate. Share that owned-row lookup with conditional claims and leave PostgreSQL's single-statement claim intact. Apply the primary-index gate to exact-ID claim, settlement and retry mutations. Build failed-release CASE updates through the query builder, inlining only internally generated integer deadlines and keeping external values bound. Allow three transaction attempts because concurrent scans of a nearly empty due index can deadlock when lease updates move entries into the shared end gap; rollback preserves single attempt accounting. Extend grammar and native tests for hinted deletes, joined aliases, missing indexes, lease ownership, bounded concurrent claims and gap-lock retries. Verify that both relays make progress without requiring full SKIP LOCKED batches, and that a rolled-back claim does not double-count. Validation: formatting and source/type/skeleton analysis; Database and Queue suites with affected tests rerun after corrections; native cases on MySQL 5.7, 8.4 and 9.5, MariaDB 10.11 and 12.3, PostgreSQL and SQLite; Redis dispatch/relay/worker delivery against MySQL. MariaDB 12.3 coverage includes its default snapshot isolation under REPEATABLE READ. --- ...0911-framework-correctness-improvements.md | 7 + .../src/Query/Grammars/MySqlGrammar.php | 32 ++- .../src/Query/Grammars/SQLiteGrammar.php | 23 ++- src/docs/queries.md | 16 +- .../src/Outbox/DatabaseOutboxRepository.php | 83 +++++--- tests/Database/DatabaseQueryBuilderTest.php | 54 +++++ ...dexHintTest.php => WriteIndexHintTest.php} | 75 +++++-- .../Queue/Fixtures/OutboxClaimTestCase.php | 193 +++++++++++++++++- 8 files changed, 430 insertions(+), 53 deletions(-) rename tests/Integration/Database/{UpdateIndexHintTest.php => WriteIndexHintTest.php} (57%) diff --git a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md index ebf35a93e..75d860b73 100644 --- a/docs/plans/2026-10-22-0911-framework-correctness-improvements.md +++ b/docs/plans/2026-10-22-0911-framework-correctness-improvements.md @@ -14,6 +14,13 @@ All changes below are implemented and verified; merging remains. Record further - **Validation defect:** guard both numeric field operands of `gt/gte/lt/lte` with their respective attribute names. Guard `multiple_of` input through `validateMultipleOf()` in compiled and delegated execution, preserving the calculation hook's signature. Exponent rejection and numeric parse errors fail validation rather than escaping as math exceptions. Comparison-message formatting preserves the `getSize()` hook and displays the original trimmed value if its exponent is rejected, without expanding it or throwing again. Preserve the attribute-aware override callback; developer-supplied rule literals remain trusted. Components already require Brick Math 1.0; no further version migration is needed. - **Decimal casts:** prevent scientific-notation expansion beyond ±1000 in request and Eloquent decimal casts, throwing their existing math exception. Share exponent extraction through internal `Number::exponent()` and the default `Number::MAX_EXPONENT` with validation, without limiting ordinary decimal-string length or changing rational parsing. Include `int` in Eloquent's decimal-cast input type so integer values retain their precision instead of passing through float conversion. Test both casts, exponent boundaries, unchanged explicit decimals/fractions and large integer precision with bounded inputs. +### Write index hints and outbox lock scope + +- Retain index hints on MySQL/MariaDB updates and forced indexes on plain SQLite updates. Honor MySQL/MariaDB DELETE hints through the native single-target multi-table form when an unjoined delete has no ordering or limit; honor hints on joined deletes as well. Derive the delete target before appending the table hint, preserving aliases and prefixes. Keep unhinted SQL and ordered/limited unjoined deletes unchanged. Plain SQLite deletes use `indexed by`; joined, limited and offset SQLite writes keep their existing selection subqueries. PostgreSQL remains unchanged. +- Explain this intentional difference from Laravel beside the grammar: ignoring requested indexes lets InnoDB scan and lock unrelated rows. A missing named index raises the native error. Hints do not guarantee lock confinement: an ID-only delete or locking read can still choose a covering scan. Retain real ownership/eligibility predicates; do not insert dummy predicates or introduce an exact-ID delete API. A caller requiring confined ID-only deletion needs its own driving-ID join. Document supported forms and limitations in query feature documentation; no README differences or porting-guide entry. +- Apply a shared native primary-index gate to outbox exact-ID mutations and claimed-row lookups while preserving lease and eligibility predicates. InnoDB claims select only IDs and original eligibility through the ordered due index, update those IDs, then read the owned message bodies within the transaction. This adds one bounded read and prevents payload selection from choosing a scan/filesort that locks beyond the batch. Allow three framework transaction attempts: concurrent due-index scans can share an end gap and deadlock when claim updates move their entries to future lease deadlines. The database-only closure can safely retry with its original token after rollback. Share the owned-row lookup with conditional claims; PostgreSQL retains its single-statement claim. Legitimate short SKIP LOCKED batches remain valid, but parallel relays must make progress when due rows outside each other's bounded batch are available. Cover native and conditional claim updates, accepted/deferred/failed settlement and operator batch retries. Express failed-release CASE updates through the builder using internally generated integer deadlines; keep external values bound and account for the three remaining non-ID bindings. +- Extend existing grammar and native index-hint tests for deletes, aliases, prefixes, joins, limits and missing indexes. Extend the existing native outbox fixture for mutation contention rather than adding another harness. Verify MySQL 5.7/8.4/9, MariaDB 10.11/12.3, PostgreSQL and SQLite behavior as applicable, including the conditional claim path on MySQL 5.7. Complete formatting, analysis, affected tests and peer review before committing. + ### Model-specific Eloquent builder types - Let `UseEloquentBuilder` accept `class-string>`, matching the existing builder property and factory return type. The attribute accepts builders bound to a particular model; `Builder` incorrectly excludes them because the model parameter is invariant. Align the resolved-class cache and resolver return annotation with the same boundary. Runtime construction and public signatures remain unchanged. diff --git a/src/database/src/Query/Grammars/MySqlGrammar.php b/src/database/src/Query/Grammars/MySqlGrammar.php index 495e0b498..5e88f00e3 100755 --- a/src/database/src/Query/Grammars/MySqlGrammar.php +++ b/src/database/src/Query/Grammars/MySqlGrammar.php @@ -467,7 +467,7 @@ protected function compileJsonUpdateColumn(string $key, array $values): string */ protected function compileUpdateWithoutJoins(Builder $query, string $table, string $columns, string $where): string { - $sql = parent::compileUpdateWithoutJoins($query, $this->compileUpdatedTable($query, $table), $columns, $where); + $sql = parent::compileUpdateWithoutJoins($query, $this->compileTableWithIndexHint($query, $table), $columns, $where); if (! empty($query->orders)) { $sql .= ' ' . $this->compileOrders($query, $query->orders); @@ -485,14 +485,16 @@ protected function compileUpdateWithoutJoins(Builder $query, string $table, stri */ protected function compileUpdateWithJoins(Builder $query, string $table, string $columns, string $where): string { - return parent::compileUpdateWithJoins($query, $this->compileUpdatedTable($query, $table), $columns, $where); + return parent::compileUpdateWithJoins($query, $this->compileTableWithIndexHint($query, $table), $columns, $where); } /** - * Compile the table an update statement changes, with the query's index hint. + * Compile a write target with the query's index hint. */ - protected function compileUpdatedTable(Builder $query, string $table): string + protected function compileTableWithIndexHint(Builder $query, string $table): string { + // Unlike Laravel, writes honor requested indexes: a scan can lock + // unrelated rows even when the predicate names only specific IDs. return $query->indexHint === null ? $table : $table . ' ' . $this->compileIndexHint($query, $query->indexHint); } @@ -533,6 +535,12 @@ public function prepareBindingsForUpdateWithJoins(array $bindings, array $values */ protected function compileDeleteWithoutJoins(Builder $query, string $table, string $where): string { + // MySQL and older MariaDB versions accept DELETE hints only in the + // multi-table form, which cannot preserve single-table ORDER BY/LIMIT. + if ($query->indexHint !== null && empty($query->orders) && ! isset($query->limit)) { + return $this->compileDeleteWithIndexHint($query, $table, $where); + } + $sql = parent::compileDeleteWithoutJoins($query, $table, $where); // When using MySQL, delete statements may contain order by statements and limits @@ -557,7 +565,9 @@ protected function compileDeleteWithoutJoins(Builder $query, string $table, stri */ protected function compileDeleteWithJoins(Builder $query, string $table, string $where): string { - $sql = parent::compileDeleteWithJoins($query, $table, $where); + $sql = $query->indexHint === null + ? parent::compileDeleteWithJoins($query, $table, $where) + : $this->compileDeleteWithIndexHint($query, $table, $where); if (! empty($query->orders)) { $sql .= ' ' . $this->compileOrders($query, $query->orders); @@ -570,6 +580,18 @@ protected function compileDeleteWithJoins(Builder $query, string $table, string return $sql; } + /** + * Compile a hinted delete with its target separate from the table reference. + */ + protected function compileDeleteWithIndexHint(Builder $query, string $table, string $where): string + { + $alias = Str::afterLast($table, ' as '); + $table = $this->compileTableWithIndexHint($query, $table); + $joins = isset($query->joins) ? ' ' . $this->compileJoins($query, $query->joins) : ''; + + return "delete {$alias} from {$table}{$joins} {$where}"; + } + /** * Compile a query to get the number of open connections for a database. */ diff --git a/src/database/src/Query/Grammars/SQLiteGrammar.php b/src/database/src/Query/Grammars/SQLiteGrammar.php index 7d849b531..14de09b82 100755 --- a/src/database/src/Query/Grammars/SQLiteGrammar.php +++ b/src/database/src/Query/Grammars/SQLiteGrammar.php @@ -224,15 +224,22 @@ public function compileUpdate(Builder $query, array $values): string /** * Compile an update statement without joins into SQL. - * - * A forced index applies to the updated table, as SQLite's "indexed by" - * clause allows. */ protected function compileUpdateWithoutJoins(Builder $query, string $table, string $columns, string $where): string { + return parent::compileUpdateWithoutJoins($query, $this->compileTableWithIndexHint($query, $table), $columns, $where); + } + + /** + * Compile a plain write target with its requested forced index. + */ + protected function compileTableWithIndexHint(Builder $query, string $table): string + { + // Unlike Laravel, plain writes honor forced indexes. Joined and + // limited writes already apply their hint in the selection subquery. $hint = $query->indexHint === null ? '' : $this->compileIndexHint($query, $query->indexHint); - return parent::compileUpdateWithoutJoins($query, $hint === '' ? $table : "{$table} {$hint}", $columns, $where); + return $hint === '' ? $table : "{$table} {$hint}"; } /** @@ -364,6 +371,14 @@ public function compileDelete(Builder $query): string return parent::compileDelete($query); } + /** + * Compile a plain delete with its requested forced index. + */ + protected function compileDeleteWithoutJoins(Builder $query, string $table, string $where): string + { + return parent::compileDeleteWithoutJoins($query, $this->compileTableWithIndexHint($query, $table), $where); + } + /** * Compile a delete statement with joins or limit into SQL. */ diff --git a/src/docs/queries.md b/src/docs/queries.md index 5d5c481c3..ca3bb85fa 100644 --- a/src/docs/queries.md +++ b/src/docs/queries.md @@ -403,7 +403,21 @@ DB::table('orders') ->update(['status' => 'shipped']); ``` -SQLite treats a forced index as a requirement rather than a preference: a query fails if the index does not exist, or if SQLite cannot use it for the query, such as a partial index whose condition the query's constraints do not imply. +MySQL and MariaDB also apply index hints to deletes without an `orderBy` or `limit`, and to joined deletes. Ordered or limited deletes without joins ignore the hint because the supported delete syntax cannot combine them. SQLite applies `forceIndex` to plain deletes; joined writes and writes with a limit or offset apply it to their selection subquery. + +For example, to delete expired orders on MySQL or MariaDB: + +```php +DB::table('orders') + ->forceIndex('primary') + ->whereIn('id', $orderIds) + ->where('status', 'expired') + ->delete(); +``` + +An index hint does not guarantee that a query locks only matching rows. MySQL and MariaDB can still scan when a delete or locking read uses only the primary key. Keep the query's actual ownership and eligibility conditions, and check its execution plan when lock scope matters. + +On MySQL, MariaDB, and SQLite, an applied hint naming an index that does not exist raises a database error. SQLite also fails when it cannot use a forced index for the query, such as a partial index whose condition the query's constraints do not imply. ## Raw Expressions diff --git a/src/queue/src/Outbox/DatabaseOutboxRepository.php b/src/queue/src/Outbox/DatabaseOutboxRepository.php index 231a9adcf..5864b3dbf 100644 --- a/src/queue/src/Outbox/DatabaseOutboxRepository.php +++ b/src/queue/src/Outbox/DatabaseOutboxRepository.php @@ -7,6 +7,7 @@ use Hypervel\Database\Connection; use Hypervel\Database\ConnectionInterface; use Hypervel\Database\ConnectionResolverInterface; +use Hypervel\Database\MySqlConnection; use Hypervel\Database\Query\Builder; use Hypervel\Database\Query\Expression; use Hypervel\Queue\Concerns\InsertsDatabaseRows; @@ -142,31 +143,27 @@ protected function claimOnPostgres(ConnectionInterface $connection, int $limit, */ protected function claimWithSkipLocked(ConnectionInterface $connection, int $limit, int $now, int $expiresAt, string $token): array { + // Moving claimed rows to a future deadline can deadlock on the due-index gap. return $connection->transaction(function () use ($connection, $limit, $now, $expiresAt, $token): array { - $rows = $this->query($connection) + // Keep the locking read covered by the due index. Fetching payloads + // here can choose a filesort that locks rows beyond the batch limit. + $candidates = $this->query($connection) ->where('available_at', '<=', $now) ->orderBy('available_at')->orderBy('id') ->limit($limit)->lockForUpdate()->skipLocked() - ->get()->all(); + ->pluck('available_at', 'id')->all(); - if ($rows === []) { + if ($candidates === []) { return []; } - $this->query($connection)->whereIn('id', array_column($rows, 'id'))->increment('attempts', 1, [ + $this->whereIds($this->query($connection), array_keys($candidates))->increment('attempts', 1, [ 'lease_token' => $token, 'available_at' => $expiresAt, ]); - foreach ($rows as $row) { - $row->eligible_at = $row->available_at; - $row->available_at = $expiresAt; - $row->lease_token = $token; - ++$row->attempts; - } - - return $rows; - }); + return $this->getClaimedRows($connection, $candidates, $token); + }, attempts: 3); } /** @@ -186,11 +183,22 @@ protected function claimWithConditionalUpdate(ConnectionInterface $connection, i } $ids = array_keys($candidates); - $this->query($connection) - ->whereIn('id', $ids)->where('available_at', '<=', $now) + $this->whereIds($this->query($connection), $ids)->where('available_at', '<=', $now) ->increment('attempts', 1, ['lease_token' => $token, 'available_at' => $expiresAt]); - $rows = $this->query($connection)->whereIn('id', $ids)->where('lease_token', $token)->get()->all(); + return $this->getClaimedRows($connection, $candidates, $token); + } + + /** + * Read the rows owned by this claim with their original eligibility. + * + * @param array $candidates + * @return list + */ + protected function getClaimedRows(ConnectionInterface $connection, array $candidates, string $token): array + { + $rows = $this->whereIds($this->query($connection), array_keys($candidates)) + ->where('lease_token', $token)->get()->all(); foreach ($rows as $row) { $row->eligible_at = $candidates[$row->id]; @@ -215,13 +223,15 @@ public function settle(string $database, array $messages, OutboxBatchResult $res $settled = 0; $maxBindings = $this->maxBindings($connection); + // Keep the lease predicate: besides fencing stale relays, it + // avoids an ID-only covering scan despite the primary-index hint. foreach (array_chunk($result->accepted, $maxBindings - 1) as $ids) { - $settled += $this->query($connection)->where('lease_token', $token)->whereIn('id', $ids)->delete(); + $settled += $this->whereIds($this->query($connection)->where('lease_token', $token), $ids)->delete(); } foreach (array_chunk($result->deferred, $maxBindings - 2) as $ids) { $grammar = $connection->getQueryGrammar(); - $settled += $this->query($connection)->where('lease_token', $token)->whereIn('id', $ids)->update([ + $settled += $this->whereIds($this->query($connection)->where('lease_token', $token), $ids)->update([ 'lease_token' => null, 'available_at' => new Expression($grammar->wrap('deliver_at')), 'attempts' => new Expression($grammar->wrap('attempts') . ' - 1'), @@ -247,23 +257,24 @@ protected function releaseFailed(ConnectionInterface $connection, string $token, $deadlines = array_map(static fn (int $delay): int => $now + random_int(max(1, (int) ceil($delay / 2)), $delay), self::RETRY_DELAYS); $error = mb_substr($error, 0, 4096); $grammar = $connection->getQueryGrammar(); - $table = $grammar->wrapTable($this->table); $attempts = $grammar->wrap('attempts'); $cases = []; - foreach (array_keys($deadlines) as $index) { - $cases[] = $index === count($deadlines) - 1 ? 'ELSE ?' : 'WHEN ' . $attempts . ' <= ' . ($index + 1) . ' THEN ?'; + // Only internally generated integers enter this expression; error + // text, lease tokens and IDs remain bound by the query builder. + foreach ($deadlines as $index => $deadline) { + $cases[] = $index === count($deadlines) - 1 ? "ELSE {$deadline}" : 'WHEN ' . $attempts . ' <= ' . ($index + 1) . " THEN {$deadline}"; } $case = 'CASE ' . implode(' ', $cases) . ' END'; $affected = 0; - foreach (array_chunk($ids, $this->maxBindings($connection) - count($deadlines) - 2) as $chunk) { - $parameters = implode(', ', array_fill(0, count($chunk), '?')); - $affected += $connection->update( - "UPDATE {$table} SET available_at = {$case}, lease_token = NULL, last_error = ? WHERE lease_token = ? AND id IN ({$parameters})", - [...$deadlines, $error, $token, ...$chunk], - ); + foreach (array_chunk($ids, $this->maxBindings($connection) - 3) as $chunk) { + $affected += $this->whereIds($this->query($connection)->where('lease_token', $token), $chunk)->update([ + 'available_at' => new Expression($case), + 'lease_token' => null, + 'last_error' => $error, + ]); } return $affected; @@ -355,7 +366,7 @@ public function retryAll(string $database): int break; } - $affected += $this->retryable($connection, $now)->whereIn('id', $ids)->update([ + $affected += $this->whereIds($this->retryable($connection, $now), $ids)->update([ 'available_at' => $now, 'lease_token' => null, 'last_error' => null, @@ -393,6 +404,22 @@ protected function withoutLiveLease(Builder $query, int $now): Builder }); } + /** + * Restrict a query to selected IDs using InnoDB's primary index. + * + * @param list $ids + */ + protected function whereIds(Builder $query, array $ids): Builder + { + // An InnoDB scan can deadlock after disjoint SKIP LOCKED claims, or + // prevent settlement after messages have already been published. + if ($query->getConnection() instanceof MySqlConnection) { + $query->forceIndex('PRIMARY'); + } + + return $query->whereIn('id', $ids); + } + /** * Query the configured table on the writer, including for operator mutations. */ diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index 85c01a15d..c1f9d508f 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -8622,6 +8622,37 @@ public function testIndexHintsOnMySqlUpdates(): void $this->assertSame(1, $builder->from('users')->forceIndex('primary')->where('id', 1)->update(['email' => 'foo'])); } + public function testIndexHintsOnMySqlDeletes(): void + { + foreach (['useIndex' => 'use', 'forceIndex' => 'force', 'ignoreIndex' => 'ignore'] as $method => $hint) { + $builder = $this->getMySqlBuilder(); + $builder->getConnection()->expects('delete')->with("delete `users` from `users` {$hint} index (test_index) where `id` = ?", [1])->andReturn(1); + $this->assertSame(1, $builder->from('users')->{$method}('test_index')->where('id', 1)->delete()); + } + + $builder = $this->getMySqlBuilder(prefix: 'prefix_'); + $builder->getConnection()->expects('delete')->with('delete `prefix_u` from `prefix_users` as `prefix_u` force index (primary) where `prefix_u`.`id` = ?', [1])->andReturn(1); + $this->assertSame(1, $builder->from('users as u')->forceIndex('primary')->where('u.id', 1)->delete()); + + $builder = $this->getMySqlBuilder(prefix: 'prefix_'); + $builder->getConnection()->expects('delete')->with('delete `prefix_u` from `prefix_users` as `prefix_u` force index (primary) inner join `prefix_orders` on `prefix_u`.`id` = `prefix_orders`.`user_id` and `prefix_orders`.`status` = ? where `prefix_u`.`id` = ?', ['paid', 1])->andReturn(1); + $this->assertSame(1, $builder->from('users as u')->forceIndex('primary')->join('orders', static function (JoinClause $join): void { + $join->on('u.id', '=', 'orders.user_id')->where('orders.status', 'paid'); + })->where('u.id', 1)->delete()); + + $builder = $this->getMySqlBuilder(); + $builder->getConnection()->expects('delete')->with('delete from `users` where `id` = ? order by `id` asc', [1])->andReturn(1); + $this->assertSame(1, $builder->from('users')->forceIndex('primary')->where('id', 1)->orderBy('id')->delete()); + + $builder = $this->getMySqlBuilder(); + $builder->getConnection()->expects('delete')->with('delete from `users` where `id` = ? limit 1', [1])->andReturn(1); + $this->assertSame(1, $builder->from('users')->forceIndex('primary')->where('id', 1)->limit(1)->delete()); + + $builder = $this->getMariaDbBuilder(); + $builder->getConnection()->expects('delete')->with('delete `users` from `users` force index (primary) where `id` = ?', [1])->andReturn(1); + $this->assertSame(1, $builder->from('users')->forceIndex('primary')->where('id', 1)->delete()); + } + public function testUseIndexSqlite(): void { $builder = $this->getSQLiteBuilder(); @@ -8667,6 +8698,29 @@ public function testIndexHintsOnSqliteUpdates(): void })->where('users.id', 1)->update(['email' => 'foo'])); } + public function testIndexHintsOnSqliteDeletes(): void + { + $builder = $this->getSQLiteBuilder(prefix: 'prefix_'); + $builder->getConnection()->expects('delete')->with('delete from "prefix_users" as "prefix_u" indexed by test_index where "prefix_u"."id" = ?', [1])->andReturn(1); + $this->assertSame(1, $builder->from('users as u')->forceIndex('test_index')->where('u.id', 1)->delete()); + + foreach (['useIndex', 'ignoreIndex'] as $method) { + $builder = $this->getSQLiteBuilder(); + $builder->getConnection()->expects('delete')->with('delete from "users" where "id" = ?', [1])->andReturn(1); + $this->assertSame(1, $builder->from('users')->{$method}('test_index')->where('id', 1)->delete()); + } + + $builder = $this->getSQLiteBuilder(); + $builder->getConnection()->expects('delete')->with('delete from "users" where "rowid" in (select "users"."rowid" from "users" indexed by test_index where "id" > ? order by "id" asc limit 3)', [1])->andReturn(1); + $this->assertSame(1, $builder->from('users')->forceIndex('test_index')->where('id', '>', 1)->orderBy('id')->limit(3)->delete()); + + $builder = $this->getSQLiteBuilder(); + $builder->getConnection()->expects('delete')->with('delete from "users" where "rowid" in (select "users"."rowid" from "users" indexed by test_index inner join "orders" on "users"."id" = "orders"."user_id" and "orders"."status" = ? where "users"."id" = ?)', ['paid', 1])->andReturn(1); + $this->assertSame(1, $builder->from('users')->forceIndex('test_index')->join('orders', static function (JoinClause $join): void { + $join->on('users.id', '=', 'orders.user_id')->where('orders.status', 'paid'); + })->where('users.id', 1)->delete()); + } + public function testClone(): void { $builder = $this->getBuilder(); diff --git a/tests/Integration/Database/UpdateIndexHintTest.php b/tests/Integration/Database/WriteIndexHintTest.php similarity index 57% rename from tests/Integration/Database/UpdateIndexHintTest.php rename to tests/Integration/Database/WriteIndexHintTest.php index 43059c3f0..67cbd1747 100644 --- a/tests/Integration/Database/UpdateIndexHintTest.php +++ b/tests/Integration/Database/WriteIndexHintTest.php @@ -12,12 +12,13 @@ use Hypervel\Engine\Channel; use Hypervel\Support\Facades\DB; use Hypervel\Support\Facades\Schema; +use PHPUnit\Framework\Attributes\TestWith; use function Hypervel\Coroutine\parallel; -class UpdateIndexHintTest extends DatabaseTestCase +class WriteIndexHintTest extends DatabaseTestCase { - private const string CONNECTION_NAME = 'update_index_hint_test'; + private const string CONNECTION_NAME = 'write_index_hint_test'; protected function defineEnvironment(ApplicationContract $app): void { @@ -68,35 +69,76 @@ public function testUpdatesTakeIndexHints(): void } } - public function testAnUpdateForcingAnIndexThatDoesNotExistFails(): void + public function testDeletesTakeIndexHints(): void + { + $hints = match ($this->driver) { + 'pgsql' => [null], + 'sqlite' => ['forceIndex'], + default => ['useIndex', 'forceIndex', 'ignoreIndex'], + }; + + foreach ($hints as $index => $hint) { + $first = $index * 2 + 1; + $query = DB::table('hinted_posts as target'); + $deleted = ($hint === null ? $query : $query->{$hint}('hinted_posts_title_index')) + ->whereIn('target.title', ["post {$first}", 'post ' . ($first + 1)])->delete(); + + $this->assertSame(2, $deleted); + $this->assertSame(0, DB::table('hinted_posts')->whereIn('id', [$first, $first + 1])->count()); + } + + $this->assertSame(30 - count($hints) * 2, DB::table('hinted_posts')->count()); + + DB::table('hinted_posts')->where('id', 28)->update(['votes' => 1]); + $joined = DB::table('hinted_posts as target') + ->join('hinted_posts as matches', 'target.id', '=', 'matches.id') + ->where('matches.votes', 0)->whereIn('target.id', [27, 28]); + if ($this->driver !== 'pgsql') { + $joined->forceIndex('hinted_posts_title_index'); + } + $this->assertSame(1, $joined->delete()); + $this->assertSame([28 => 1], DB::table('hinted_posts')->whereIn('id', [27, 28])->pluck('votes', 'id')->all()); + } + + #[TestWith(['update'])] + #[TestWith(['delete'])] + public function testAWriteForcingAnIndexThatDoesNotExistFails(string $operation): void { $this->skipIfDriver('pgsql'); try { - DB::table('hinted_posts')->forceIndex('hinted_posts_missing_index')->where('id', 1)->update(['votes' => 1]); + $query = DB::table('hinted_posts')->forceIndex('hinted_posts_missing_index')->where('id', 1); + $operation === 'update' ? $query->update(['votes' => 1]) : $query->delete(); - $this->fail('The update ran without its index hint.'); + $this->fail('The write ran without its index hint.'); } catch (QueryException $exception) { $this->assertStringContainsString('hinted_posts_missing_index', $exception->getMessage()); } $this->assertSame(0, (int) DB::table('hinted_posts')->sum('votes')); + $this->assertSame(30, DB::table('hinted_posts')->count()); } - public function testAnUpdateForcingThePrimaryKeyLocksOnlyTheRowsItNames(): void + #[TestWith(['update'])] + #[TestWith(['delete'])] + public function testAWriteForcingThePrimaryKeyLocksOnlyTheRowsItNames(string $operation): void { if (! in_array($this->driver, ['mysql', 'mariadb'], true)) { $this->markTestSkipped('InnoDB locks the rows a statement reads; other databases are not affected.'); } + if ($operation === 'delete') { + DB::table('hinted_posts')->where('id', 1)->update(['votes' => 1]); + } + $holderPool = new DatabasePool($this->app, self::CONNECTION_NAME); $contenderPool = new DatabasePool($this->app, self::CONNECTION_NAME); - $updated = new Channel(1); + $written = new Channel(1); $released = new Channel(1); try { [$count, $free] = parallel([ - function () use ($holderPool, $updated, $released): int { + function () use ($holderPool, $written, $released, $operation): int { /** @var PooledConnection $pooledConnection */ $pooledConnection = $holderPool->borrow(); $connection = $pooledConnection->getConnection(); @@ -104,8 +146,11 @@ function () use ($holderPool, $updated, $released): int { try { $connection->beginTransaction(); // Most of a small table, which both servers otherwise read whole, locking every row. - $count = $connection->table('hinted_posts')->forceIndex('primary')->whereIn('id', range(1, 20))->update(['votes' => 1]); - $updated->push(true); + $query = $connection->table('hinted_posts')->forceIndex('primary')->whereIn('id', range(1, 20)); + // Deletes retain their real eligibility condition. An ID-only + // delete can still choose a covering scan despite the hint. + $count = $operation === 'update' ? $query->update(['votes' => 1]) : $query->where('votes', 0)->delete(); + $written->push(true); $released->pop(5); $connection->commit(); @@ -118,8 +163,8 @@ function () use ($holderPool, $updated, $released): int { $pooledConnection->release(); } }, - function () use ($contenderPool, $updated, $released): ?int { - $updated->pop(5); + function () use ($contenderPool, $written, $released): ?int { + $written->pop(5); /** @var PooledConnection $pooledConnection */ $pooledConnection = $contenderPool->borrow(); @@ -138,8 +183,10 @@ function () use ($contenderPool, $updated, $released): ?int { $contenderPool->close(); } - $this->assertSame(20, $count); + $this->assertSame($operation === 'update' ? 20 : 19, $count); $this->assertSame(30, $free); - $this->assertSame(20, (int) DB::table('hinted_posts')->sum('votes')); + $this->assertSame($operation === 'update' ? 20 : 1, (int) DB::table('hinted_posts')->sum('votes')); + $this->assertSame($operation === 'update' ? 30 : 11, DB::table('hinted_posts')->count()); + $this->assertSame(1, DB::table('hinted_posts')->where('id', 1)->value('votes')); } } diff --git a/tests/Integration/Queue/Fixtures/OutboxClaimTestCase.php b/tests/Integration/Queue/Fixtures/OutboxClaimTestCase.php index 947de7e08..d0cc13523 100644 --- a/tests/Integration/Queue/Fixtures/OutboxClaimTestCase.php +++ b/tests/Integration/Queue/Fixtures/OutboxClaimTestCase.php @@ -20,6 +20,8 @@ use Hypervel\Testbench\Attributes\ResetRefreshDatabaseState; use Hypervel\Testbench\TestCase; use Mockery as m; +use PHPUnit\Framework\Attributes\TestWith; +use RuntimeException; use function Hypervel\Coroutine\parallel; @@ -39,6 +41,9 @@ protected function defineEnvironment(Application $app): void $config->set("database.connections.{$database}.pool.testing_enabled", true); $config->set("database.connections.{$database}.pool.max_connections", 4); $config->set("database.connections.{$database}.pool.heartbeat_interval", null); + if ($config->string("database.connections.{$database}.driver") !== 'sqlite') { + $config->set("database.connections.{$database}.lock_timeout", 1); + } } /** @@ -75,7 +80,7 @@ public function testClaimBoundsAndStatementCountOnTheLiveEngine(): void } $postgres = $connection->getDriverName() === 'pgsql'; - $this->assertCount($postgres ? 1 : ($skipLocked ? 2 : 3), $queries); + $this->assertCount($postgres ? 1 : 3, $queries); $this->assertSame($skipLocked && ! $postgres ? 1 : 0, $transactions); $this->assertSame([$rows[1]['uuid'], $rows[2]['uuid']], array_column($messages, 'uuid')); $this->assertSame([$now - 30, $now - 20], array_column($messages, 'eligibleAt')); @@ -221,6 +226,192 @@ function () use ($repository, $database, $ready, $release): array { $this->assertSame([$locked['uuid']], array_column($repository->claim($database, 1), 'uuid')); } + public function testConcurrentClaimsLeaveRowsForEachRelay(): void + { + $connection = DB::connection(); + if (! $connection->supportsSkipLocked() || $connection->getDriverName() === 'pgsql') { + $this->markTestSkipped('This regression needs the InnoDB locking-read claim path.'); + } + + Date::setTestNow('2040-01-01 12:00:00'); + $now = Date::now()->getTimestamp(); + $repository = $this->app->make(DatabaseOutboxRepository::class); + $database = $connection->getName(); + $repository->insert($connection, array_map( + fn (int $index): array => $this->row($now - ($index % 2 === 0 ? 20 : 10)), + range(0, 127), + )); + $ready = new Channel(2); + $release = new Channel(2); + + // Interleave the actual SELECT and UPDATE, after both relays + // have acquired their own candidate locks. + $this->app->make('events')->listen(QueryExecuted::class, function (QueryExecuted $event) use ($ready, $release): void { + if (str_contains($event->sql, 'job_outbox') && str_contains($event->sql, 'for update')) { + $this->assertTrue($ready->push(true, 5)); + $this->assertTrue($release->pop(5)); + } + }); + + try { + [$first, $second] = parallel([ + fn (): array => $repository->claim($database, 64), + fn (): array => $repository->claim($database, 64), + function () use ($ready, $release): void { + $this->assertTrue($ready->pop(5)); + $this->assertTrue($ready->pop(5)); + $release->push(true); + $release->push(true); + }, + ]); + $this->assertNotEmpty($first); + $this->assertNotEmpty($second); + $this->assertSame([], array_intersect(array_column($first, 'id'), array_column($second, 'id'))); + } finally { + $ready->close(); + $release->close(); + } + } + + public function testConcurrentClaimsRetryWhenMovingDueIndexEntries(): void + { + $connection = DB::connection(); + if (! $connection->supportsSkipLocked() || $connection->getDriverName() === 'pgsql') { + $this->markTestSkipped('This regression needs InnoDB next-key locking.'); + } + + Date::setTestNow('2040-01-01 12:00:00'); + $now = Date::now()->getTimestamp(); + $repository = $this->app->make(DatabaseOutboxRepository::class); + $database = $connection->getName(); + $repository->insert($connection, array_map(fn (): array => $this->row($now - 10), range(1, 4))); + $held = new Channel(1); + $unlock = new Channel(1); + $startSecond = new Channel(1); + $selected = new Channel(2); + $update = new Channel(2); + $reads = 0; + $this->app->make('events')->listen(QueryExecuted::class, function (QueryExecuted $event) use ($selected, $update, &$reads): void { + if (str_starts_with($event->sql, 'select ') && str_contains($event->sql, 'skip locked') && ++$reads <= 2) { + $this->assertTrue($selected->push(true, 5)); + $this->assertTrue($update->pop(5)); + } + }); + + try { + [, $first, $second] = parallel([ + function () use ($held, $unlock, $startSecond, $now): void { + DB::transaction(function () use ($held, $unlock, $now): void { + $index = DB::connection()->getSchemaBuilder()->qualifyIndexName('job_outbox_available_at_id_index'); + // Hold the secondary entries, so the first relay skips them + // before taking any locks on their primary records. + DB::table('job_outbox')->forceIndex($index)->where('available_at', $now - 10) + ->whereIn('id', [2, 4])->lockForUpdate()->get(); + $held->push(true); + $this->assertTrue($unlock->pop(5)); + }); + $startSecond->push(true); + }, + function () use ($held, $repository, $database): array { + $this->assertTrue($held->pop(5)); + + return $repository->claim($database, 100); + }, + function () use ($startSecond, $repository, $database): array { + $this->assertTrue($startSecond->pop(5)); + + return $repository->claim($database, 100); + }, + function () use ($selected, $unlock, $update): void { + $this->assertTrue($selected->pop(5)); + $unlock->push(true); + $this->assertTrue($selected->pop(5)); + $update->push(true); + $update->push(true); + }, + ]); + $messages = [...$first, ...$second]; + $ids = array_column($messages, 'id'); + sort($ids); + $this->assertSame([1, 2, 3, 4], $ids); + $this->assertSame([1, 1, 1, 1], array_column($messages, 'attempts')); + } finally { + $held->close(); + $unlock->close(); + $startSecond->close(); + $selected->close(); + $update->close(); + } + } + + #[TestWith(['accepted'])] + #[TestWith(['deferred'])] + #[TestWith(['failed'])] + #[TestWith(['retry'])] + public function testBatchMutationsDoNotWaitForAnotherRelaysRows(string $operation): void + { + $connection = DB::connection(); + if ($connection->getDriverName() === 'sqlite') { + $this->markTestSkipped('These mutation regressions require row locks.'); + } + + Date::setTestNow('2040-01-01 12:00:00'); + $now = Date::now()->getTimestamp(); + $repository = $this->app->make(DatabaseOutboxRepository::class); + $database = $connection->getName(); + $rows = []; + for ($index = 0; $index < 128; ++$index) { + $rows[] = [...$this->row($now - ($index % 2 === 0 ? 20 : 10)), 'deliver_at' => $now + 600]; + } + $repository->insert($connection, $rows); + $ready = new Channel(2); + $release = new Channel(2); + + try { + $messages = $repository->claim($database, 64); + $other = $repository->claim($database, 64); + $this->assertCount(64, $messages); + $this->assertCount(64, $other); + $ids = array_column($messages, 'id'); + if ($operation === 'retry') { + $connection->table('job_outbox')->whereIn('id', $ids)->update(['available_at' => $now - 1]); + } + + [, $affected] = parallel([ + function () use ($other, $ready, $release): void { + DB::transaction(function () use ($other, $ready, $release): void { + $query = DB::table('job_outbox')->whereIn('id', array_column($other, 'id')); + if (in_array(DB::connection()->getDriverName(), ['mysql', 'mariadb'], true)) { + $query->forceIndex('PRIMARY'); + } + $query->lockForUpdate()->get(); + $ready->push(true); + $this->assertTrue($release->pop(5)); + }); + }, + function () use ($repository, $database, $messages, $ids, $operation, $ready, $release): int { + try { + $this->assertTrue($ready->pop(5)); + + return match ($operation) { + 'accepted' => $repository->settle($database, $messages, new OutboxBatchResult($ids)), + 'deferred' => $repository->settle($database, $messages, new OutboxBatchResult(deferred: $ids)), + 'failed' => $repository->settle($database, $messages, new OutboxBatchResult(failure: new RuntimeException('Transport failed.'))), + 'retry' => $repository->retryAll($database), + }; + } finally { + $release->push(true); + } + }, + ]); + $this->assertSame(64, $affected); + $this->assertSame(64, DB::table('job_outbox')->where('lease_token', $other[0]->leaseToken)->count()); + } finally { + $ready->close(); + $release->close(); + } + } + /** * Create opaque stored bytes and exact epoch deadlines for claim tests. */ From 5e00ae6046d2640d851c9632491d14ec6ccf6f5e Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Sun, 1 Nov 2026 13:28:40 +0000 Subject: [PATCH 19/29] Explain the case-sensitive Vitess capability comparison Default Vitess advertises a MySQL version followed by a capitalized Vitess suffix. Keep that value on the MySQL comparison path; matching it case-insensitively would compare the MySQL version against the Vitess 19.0 threshold and disable skip-locked incorrectly. Document why release-number overrides use the separate branch so future cleanup or upstream synchronization preserves the distinction. This changes comments only. Verified against Vitess server-version source, the existing capability tests and Laravel queue detection; git diff --check passes. --- src/database/src/MySqlConnection.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/database/src/MySqlConnection.php b/src/database/src/MySqlConnection.php index 6613c3477..862412d17 100755 --- a/src/database/src/MySqlConnection.php +++ b/src/database/src/MySqlConnection.php @@ -309,6 +309,9 @@ protected function resolveSupportsSkipLocked(): bool $isMaria = str_contains($version, 'MariaDB') || $this->isMaria(); $version = $this->normalizeServerVersion($version); + // Keep this case-sensitive: default Vitess reports a MySQL version with a "-Vitess" + // suffix and must use the MySQL comparison below. Overrides such as "19.0.0-vitess" + // carry the Vitess release number required by this branch. if (Str::contains($version, ['vitess', 'PlanetScale'])) { return version_compare(Str::before($version, '-'), '19.0', '>='); } From ff0bed958ae2b90561143ef691e80c2774c86493 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 2 Nov 2026 07:46:08 +0000 Subject: [PATCH 20/29] Rebuild JWT signing configuration after boot-time settings settle Port tymondesigns/jwt-auth #2304 from 99e8f24cf96bfb6d847597c6a80713879a0a19c1. Defer configuration rebuilding until the provider is used so changing the algorithm before its keys succeeds. Keep configuration failures outside token-error handling, preserve parsed-key reuse and verification-only providers, and retain the upstream protected extension points. Merge the upstream setter regressions with existing coverage and check missing keys during both encoding and decoding. The affected test file, source/type/skeleton analysis and changed-file formatting pass. --- src/jwt/src/Providers/Lcobucci.php | 49 ++++++++++++++++----- src/jwt/src/Providers/Provider.php | 2 +- tests/Jwt/Providers/LcobucciTest.php | 65 +++++++++++++++++++++++----- 3 files changed, 93 insertions(+), 23 deletions(-) diff --git a/src/jwt/src/Providers/Lcobucci.php b/src/jwt/src/Providers/Lcobucci.php index 86268a70b..cf791e877 100644 --- a/src/jwt/src/Providers/Lcobucci.php +++ b/src/jwt/src/Providers/Lcobucci.php @@ -39,7 +39,7 @@ class Lcobucci extends Provider implements ProviderContract /** * The configuration that signs and verifies tokens. */ - protected Configuration $config; + protected ?Configuration $config; /** * Whether the provider has no private key, so it can only verify tokens. @@ -86,6 +86,8 @@ public function __construct( */ public function encode(array $payload): string { + $config = $this->config(); + if ($this->verifiesOnly) { throw new JwtException('Private key is not set.'); } @@ -94,7 +96,7 @@ public function encode(array $payload): string try { return $builder - ->getToken($this->config->signer(), $this->config->signingKey()) + ->getToken($config->signer(), $config->signingKey()) ->toString(); } catch (Exception $e) { throw new JwtException('Could not create token: ' . $e->getMessage(), $e->getCode(), $e); @@ -108,9 +110,11 @@ public function encode(array $payload): string */ public function decode(#[SensitiveParameter] string $token): array { + $config = $this->config(); + try { /** @var Plain */ - $token = $this->config->parser()->parse($token); + $token = $config->parser()->parse($token); } catch (Throwable $exception) { throw new TokenInvalidException( 'Could not decode token: ' . $exception->getMessage(), @@ -120,7 +124,7 @@ public function decode(#[SensitiveParameter] string $token): array } try { - $verified = $this->config->validator()->validate($token, ...$this->config->validationConstraints()); + $verified = $config->validator()->validate($token, ...$config->validationConstraints()); } catch (ConversionFailed) { // ECDSA verification rejects a signature of the wrong length before comparing it. $verified = false; @@ -141,7 +145,7 @@ public function decode(#[SensitiveParameter] string $token): array */ protected function getBuilderFromClaims(array $payload): Builder { - $builder = $this->config->builder(); + $builder = $this->config()->builder(); foreach ($payload as $key => $value) { switch ($key) { @@ -192,6 +196,27 @@ protected function getDateFromClaim(int|string|DateTimeInterface|DateInterval $v return DateTimeImmutable::createFromFormat('U', (string) $value); } + /** + * Get the configuration, rebuilding it if the algorithm or keys have changed. + */ + protected function config(): Configuration + { + return $this->config ?? $this->configure(); + } + + /** + * Discard the configuration so it and the signer are rebuilt on next use. + * + * @return $this + */ + protected function resetConfig(): static + { + // Algorithm and keys can be changed in either order before the provider is used. + $this->config = null; + + return $this; + } + /** * Build the configuration. */ @@ -218,24 +243,24 @@ protected function buildConfig(): Configuration * * @throws JwtException */ - protected function configure(?Configuration $config = null): void + protected function configure(?Configuration $config = null): Configuration { // buildConfig() reads the signer and the verification-only flag. $this->signer = $this->getSigner(); $this->verifiesOnly = $config === null && $this->isAsymmetric() && ! $this->getPrivateKey(); - $this->config = $config ?? $this->buildConfig(); + + return $this->config = $config ?? $this->buildConfig(); } /** - * Rebuild the signer and configuration after a setting changes. + * Discard the configuration after a setting changes. * - * A configuration given to the constructor is replaced by one built from the new settings. - * - * @throws JwtException + * A configuration given to the constructor is replaced by one built from the new settings + * on next use, when invalid settings also raise their configuration error. */ protected function onConfigurationChanged(): void { - $this->configure(); + $this->resetConfig(); } /** diff --git a/src/jwt/src/Providers/Provider.php b/src/jwt/src/Providers/Provider.php index f5bf2ec28..f9d62a8d5 100644 --- a/src/jwt/src/Providers/Provider.php +++ b/src/jwt/src/Providers/Provider.php @@ -161,7 +161,7 @@ abstract protected function isAsymmetric(): bool; * Hook fired after any of algo/secret/keys is mutated. * * Subclasses that cache derived state (signers, configuration objects) - * override this to rebuild it so the new values actually take effect. + * override this to invalidate it so the new values take effect on next use. */ protected function onConfigurationChanged(): void { diff --git a/tests/Jwt/Providers/LcobucciTest.php b/tests/Jwt/Providers/LcobucciTest.php index 25acf659d..dd5250a72 100644 --- a/tests/Jwt/Providers/LcobucciTest.php +++ b/tests/Jwt/Providers/LcobucciTest.php @@ -513,7 +513,7 @@ public function testShouldReturnTheKeys(): void $this->assertSame($keys, $provider->getKeys()); } - public function testSetAlgoTakesEffectOnEncoding(): void + public function testShouldSignWithTheNewAlgorithmAfterSettingTheAlgorithm(): void { $payload = ['sub' => 1, 'iat' => $this->testNowTimestamp]; @@ -523,10 +523,23 @@ public function testSetAlgoTakesEffectOnEncoding(): void $token = $provider->encode($payload); $header = json_decode(base64_decode(explode('.', $token)[0]), true); + $this->assertEquals(Provider::ALGO_HS512, $provider->getAlgo()); $this->assertEquals(Provider::ALGO_HS512, $header['alg']); + $this->assertEquals('1', $provider->decode($token)['sub']); } - public function testSetSecretTakesEffectOnSigning(): void + public function testShouldNotVerifyTokensSignedWithThePreviousSecretAfterSettingTheSecret(): void + { + $this->expectException(TokenInvalidException::class); + $this->expectExceptionMessageIs('Token Signature could not be verified.'); + + $provider = $this->getProvider($this->getRandomString(), Provider::ALGO_HS256); + $token = $provider->encode(['sub' => 1]); + + $provider->setSecret($this->getRandomString())->decode($token); + } + + public function testShouldSignWithTheNewSecretAfterSettingTheSecret(): void { $payload = ['sub' => 1, 'iat' => $this->testNowTimestamp]; @@ -548,6 +561,46 @@ public function testSetSecretTakesEffectOnSigning(): void $this->getProvider($originalSecret, Provider::ALGO_HS256)->decode($token); } + public function testCanSwitchFromASymmetricToAnAsymmetricAlgorithm(): void + { + $provider = $this->getProvider($this->getRandomString(), Provider::ALGO_HS256) + ->setAlgo(Provider::ALGO_RS256) + ->setKeys(['private' => $this->getDummyPrivateKey(), 'public' => $this->getDummyPublicKey()]); + + $token = $provider->encode(['sub' => 1]); + $header = json_decode(base64_decode(explode('.', $token)[0]), true); + + $this->assertEquals(Provider::ALGO_RS256, $header['alg']); + $this->assertEquals('1', $provider->decode($token)['sub']); + } + + #[DataProvider('incompleteAsymmetricConfigurationProvider')] + public function testShouldThrowAnExceptionWhenSwitchingToAnAsymmetricAlgorithmWithoutKeys(bool $decode): void + { + $provider = $this->getProvider($this->getRandomString(), Provider::ALGO_HS256) + ->setAlgo(Provider::ALGO_RS256); + + $this->expectException(JwtException::class); + $this->expectExceptionMessageIs('Public key is not set.'); + + if ($decode) { + $provider->decode('foo.bar.baz'); + } else { + $provider->encode(['sub' => 1]); + } + } + + /** + * Provide operations that require a complete signing configuration. + */ + public static function incompleteAsymmetricConfigurationProvider(): array + { + return [ + 'encode' => [false], + 'decode' => [true], + ]; + } + public function testShouldThrowAnExceptionWhenTheSecretHasBeenUpdatedAndAnOldTokenIsUsed(): void { $payload = ['sub' => '1', 'exp' => $this->testNowTimestamp + 3600, 'iat' => $this->testNowTimestamp, 'iss' => '/foo']; @@ -714,14 +767,6 @@ public static function credentialFailureProvider(): array '__construct', [0, 2], ], - 'new keys' => [ - static fn (): Lcobucci => (new Lcobucci('does_not_matter', Provider::ALGO_RS256, [ - 'private' => file_get_contents("{$keys}/id_rsa"), - 'public' => file_get_contents("{$keys}/id_rsa.pub"), - ]))->setKeys(['private' => file_get_contents("{$keys}/id_rsa_alt")]), - 'setKeys', - [0], - ], 'key file' => [ static fn (): Lcobucci => new Lcobucci('does_not_matter', Provider::ALGO_RS256, [ 'private' => "file://{$keys}/missing", From 16ec6f3a1d77972ff8cf8900cf666f0819e28d18 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 2 Nov 2026 07:46:08 +0000 Subject: [PATCH 21/29] Document AI stopping middleware and sub-agent approval limits Port laravel/docs #11415 and formatting commit 76d30af91d459b4333de30f00d9212b03b5abda3 from target b0d9ce8a6ab417d7ebef2423fb4682b7d9d8dfe1. Describe classification model selection, stopping runs and the sub-agent approval limitation, while preserving Hypervel's batched streaming updates and existing conversation behavior. Checked examples against the middleware, tool and classification implementations and verified internal anchors. --- src/docs/ai-sdk.md | 55 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/src/docs/ai-sdk.md b/src/docs/ai-sdk.md index d3acc9f82..9ac2ad957 100644 --- a/src/docs/ai-sdk.md +++ b/src/docs/ai-sdk.md @@ -264,7 +264,7 @@ Likewise, you must configure a default transcription model to use `Transcription #### OpenAI-Compatible Classification -To use `Classification` with an OpenAI-compatible provider, configure a default classification model. Requests are sent to the endpoint's `/decisions` route in OpenAI's format, so your endpoint must support it: +If your endpoint implements OpenAI's decisions endpoint, you may use [classification](#classification) with an OpenAI-compatible provider. The SDK sends questions to the endpoint's `/decisions` route and supports [image attachments](#classifying-images). Since the SDK does not know which models your endpoint supports, configure a default classification model or pass a model to the `classify` method: ```php 'local' => [ @@ -273,7 +273,7 @@ To use `Classification` with an OpenAI-compatible provider, configure a default 'key' => env('LOCAL_AI_API_KEY'), 'models' => [ 'classification' => [ - 'default' => 'local-classifier', + 'default' => env('LOCAL_AI_MODEL'), ], ], ], @@ -929,7 +929,7 @@ $response = (new SalesCoach)->prompt('Analyze this sales transcript...'); return $response['score']; ``` -When a run ends without structured output, such as when it pauses for [tool approval](#human-tool-approval) or is [stopped](#stopping-a-run) before its first step, the response's `structured` property is an empty array. +When a run ends without structured output, such as when it pauses for [tool approval](#human-tool-approval) or is [stopped](#stopping-runs) before its first step, the response's `structured` property is an empty array. #### Nested Objects @@ -2001,7 +2001,10 @@ class RefundsAgent implements Agent, CanActAsTool, HasTools If a sub-agent does not implement `CanActAsTool`, Hypervel will use the agent's class basename as the tool name and a generic description that asks the parent agent to pass a clear, self-contained task description. Each sub-agent invocation runs in isolation and does not receive the parent agent's conversation history. -When the parent agent is [streaming](#streaming), its sub-agents stream as well, except sub-agents with [structured output](#structured-output), which return their result once complete. As a sub-agent's text grows, the parent periodically emits `ToolResult` events containing the text produced so far. These events are marked as preliminary and are followed by the tool call's final result. Short responses may only produce the final result. You may skip preliminary events when iterating manually: +> [!WARNING] +> Sub-agents do not currently support tools that require [human approval](#human-tool-approval). If a sub-agent's tool requires approval, the sub-agent pauses. However, the parent agent receives a normal tool result and continues its run without notifying your application of the pending approval. Register tools that require approval on the top-level agent. + +When the parent agent is [streaming](#streaming), its sub-agents stream as well, unless they produce [structured output](#structured-output). For streaming sub-agents, the parent agent emits preliminary `ToolResult` events containing the text the sub-agent has produced so far. The tool call's final result follows these events, so you may skip preliminary events when iterating manually. Hypervel batches progress updates as the text grows, so short responses may only produce the final result: ```php use Hypervel\Ai\Streaming\Events\ToolResult; @@ -2081,7 +2084,7 @@ class LogPrompts } ``` -Middleware resolved from class names follows the normal container lifetime rules; shared middleware should be stateless. Middleware wraps each model step, not individual tool calls or child-agent runs. It can [end the run](#stopping-a-run) before the next model call, but does not interrupt a provider request that is already running. +Middleware resolved from class names follows the normal container lifetime rules; shared middleware should be stateless. Middleware wraps each model step, not individual tool calls or child-agent runs. It can [end the run](#stopping-runs) before the next model call, but does not interrupt a provider request that is already running. In addition to the `provider`, `model`, `instructions`, `messages`, and `tools` that are about to be sent, the step exposes the steps that have already completed, their combined usage, and the progress of the run: @@ -2148,7 +2151,7 @@ public function handle(PendingStep $step, Closure $next) } ``` -Middleware must return the result of `$next`, or its own `StepResponse` to answer the step without invoking the model, such as when serving a cached response. Returning any other value will throw a `LogicException`. +Middleware must return the result of `$next`, a `StepResponse`, or `StepResult::stop()`. Return a `StepResponse` to answer the step without invoking the model, such as when serving a cached response. Return `StepResult::stop()` to end the run. Any other return value causes a `LogicException`. You may also attach middleware when invoking an agent, without changing its class. The `withMiddleware` method appends middleware that runs before the agent's declared middleware, in the order supplied: @@ -2163,23 +2166,41 @@ $response = SalesCoach::make() You may pass middleware class names, instances, or closures. Middleware added this way remains on the agent instance for subsequent calls. A stream captures the added middleware when it is created, so adding more middleware afterwards does not change that stream. - -#### Stopping a Run + +#### Stopping Runs -The `Stop` middleware ends a run before its next model call. Its `when` and `unless` methods accept a boolean or a closure that receives the pending step: +To end an agent's run before its next model call, use the `Stop` middleware. For example, you may stop a run when a user presses a "stop" button or the run reaches an input-token budget. The response retains the completed steps and their combined usage: ```php +use Hypervel\Ai\Contracts\HasMiddleware; use Hypervel\Ai\Middleware\Stop; use Hypervel\Ai\PendingStep; +use Hypervel\Support\Facades\Cache; -$response = SalesCoach::make() - ->withMiddleware([ - Stop::when(fn (PendingStep $step) => $step->usage->totalTokens() > 50_000), - ]) - ->prompt('Review this sales call.'); +public function middleware(): array +{ + return [ + // Stop when the user has pressed the "stop" button... + Stop::when(fn (PendingStep $step) => Cache::has("chat:{$this->user->id}:stopped")), + + // Stop when the run has used at least 50,000 input tokens... + Stop::unless(fn (PendingStep $step) => $step->usage->inputTokens < 50_000), + ]; +} ``` -A stopped run returns the steps that completed before it stopped, and a remembered conversation keeps their messages. If no step completed, the response text is empty. A stopped stream ends with the `cancelled` finish reason. +Before each step, `when` stops the run if the condition is true, and `unless` stops the run if the condition is false. Conditions may be booleans or closures that receive the pending step. A stopped streaming run has a `cancelled` finish reason. A remembered conversation keeps the completed steps' messages. If no step completed, the response text is empty. + +Custom middleware may also end the run by returning `StepResult::stop()` instead of the result of `$next`: + +```php +use Hypervel\Ai\Gateway\StepResult; + +public function handle(PendingStep $step, Closure $next) +{ + return $this->cancelled($step) ? StepResult::stop() : $next($step); +} +``` Queued agents and queued AI event listeners require serializable middleware, captured values, and tool instances stored on the agent. Queued prompt listeners also serialize all tools resolved through `withTools`, including declared tools retained by its closure form. For tools that contain closures, such as `SimilaritySearch`, declare them in `tools` and omit `withTools` when using queued prompt listeners. @@ -3224,10 +3245,10 @@ $result = Classification::of('Inspect the product in this photo.', [ ->classify(provider: 'openai'); ``` -When images are given string keys, each image is labeled with its key, so your questions may refer to the images by name: +To refer to a specific image in a question, attach images with string keys. The SDK labels each image with its key: ```php -$result = Classification::of('Two product photos.', [ +$result = Classification::of('Inspect the products in these photos.', [ 'front' => Image::fromPath($front), 'back' => Image::fromPath($back), ]) From dfdf100b00335c386cff3d4c6c1259c0a32d68b0 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 2 Nov 2026 07:46:09 +0000 Subject: [PATCH 22/29] Explain MCP OAuth registration rate limiting Port laravel/docs #11416 from target b0d9ce8a6ab417d7ebef2423fb4682b7d9d8dfe1. Explain public registration, its database writes and the registration toggle. Document Hypervel's automatic Passport limiter instead of adding a duplicate application throttle group. Verified against Registrar::oauthRoutes and Passport configuration. --- src/docs/mcp.md | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/docs/mcp.md b/src/docs/mcp.md index be1466cf2..c56140e43 100644 --- a/src/docs/mcp.md +++ b/src/docs/mcp.md @@ -1858,12 +1858,11 @@ Mcp::web('/mcp/weather', WeatherExample::class) ]); ``` + #### New Passport Installation If your application is not already using Passport, follow Passport's [installation and deployment guide](/docs/{{version}}/passport#installation) to add Passport to your application. You should have an `OAuthenticatable` model, new authentication guard, and Passport keys before moving on. -Dynamic client registration applies `passport.middleware` and Passport's named `passport` rate limiter. You may customize that limiter using Passport's [rate limiting configuration](/docs/{{version}}/passport#rate-limiting). - Clients may register up to 10 redirect URIs, each at most 1,024 characters long. Dynamically registered clients are marked as third-party clients, so Passport's `firstParty()` method returns `false` even though they have no owner. This keeps them subject to consent when your application skips consent for its own clients. @@ -1928,6 +1927,17 @@ OAuth2.1 is the documented authentication mechanism in the Model Context Protoco If your application is already using [Sanctum](/docs/{{version}}/sanctum) then adding Passport may be cumbersome. In this instance, we recommend using Sanctum without Passport until you have a clear, necessary requirement to use an MCP client that only supports OAuth. + +#### Rate Limiting OAuth Routes + +As expected by the MCP specification and RFC 7591, the dynamic client registration route registered by `Mcp::oauthRoutes` is publicly accessible, allowing any MCP client to register itself without authentication. Each registration creates a new OAuth client in your database. Hypervel automatically applies `passport.middleware` and Passport's named `passport` rate limiter. Customize that limiter to fit your application's traffic using Passport's [rate limiting configuration](/docs/{{version}}/passport#rate-limiting). + +If your MCP clients do not need to register themselves dynamically, you may disable the client registration route entirely by passing `false` to the `registration` argument: + +```php +Mcp::oauthRoutes(registration: false); +``` + ### Sanctum From d0c78c724ef1ad9f4b5a6c3a402c4f220920ab10 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 2 Nov 2026 07:46:09 +0000 Subject: [PATCH 23/29] Warn against user-selected upload filenames Port laravel/docs #11422 from b0d9ce8a6ab417d7ebef2423fb4682b7d9d8dfe1. Explain why MIME validation does not remove embedded code and recommend generated storage names. Condition PHP execution risk on the web server's configuration. Compared the final upstream documentation and traced file storage and MIME validation. --- src/docs/filesystem.md | 3 +++ src/docs/validation.md | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/docs/filesystem.md b/src/docs/filesystem.md index c7255f289..338e2fe96 100644 --- a/src/docs/filesystem.md +++ b/src/docs/filesystem.md @@ -919,6 +919,9 @@ $path = Storage::putFileAs( ); ``` +> [!WARNING] +> Never derive the filename argument of `storeAs` or `putFileAs` from user-supplied input. An attacker could supply a name such as `shell.php`, causing the stored file to be executed if your web server permits PHP execution in that location. Use `hashName()` or a system-generated identifier for the stored filename, and persist the original name separately if you need to display it. + > [!WARNING] > Unprintable and invalid unicode characters will automatically be removed from file paths. Therefore, you may wish to sanitize your file paths before passing them to Hypervel's file storage methods. File paths are normalized using the `League\Flysystem\WhitespacePathNormalizer::normalizePath` method. diff --git a/src/docs/validation.md b/src/docs/validation.md index 53a54ccb9..d92f7465c 100644 --- a/src/docs/validation.md +++ b/src/docs/validation.md @@ -2540,6 +2540,9 @@ Even though you only need to specify the extensions, this rule actually validate This validation rule does not verify agreement between the MIME type and the extension the user assigned to the file. For example, the `mimes:png` validation rule would consider a file containing valid PNG content to be a valid PNG image, even if the file is named `photo.txt`. If you would like to validate the user-assigned extension of the file, you may use the [extensions](#rule-extensions) rule. +> [!WARNING] +> Passing image validation does not remove embedded code. A file that begins with GIF or JPEG magic bytes can pass `mimes:gif` or `mimes:jpg` validation while also containing a PHP payload. Always use a system-generated filename (e.g. `hashName()`) when storing uploaded files, and configure your web server to deny PHP execution inside storage directories. + #### min:_value_ From 2d71f9b8ce68528fcae7caa722a84d6218f4dfda Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 2 Nov 2026 07:46:09 +0000 Subject: [PATCH 24/29] Document initial Inertia page preloading and Link header limits Port inertiajs/docs #95 from f55db8196b7fa8723875e76d1e78c5c3e8077af1. Show initial component preloading for Vue, React and Svelte and explain limiting Link headers or increasing reverse-proxy buffers. Adapt examples to Hypervel middleware and omit Laravel-version and starter-kit assumptions. Verified the root-template page data and middleware APIs. --- src/docs/inertia.md | 46 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/src/docs/inertia.md b/src/docs/inertia.md index 33a61bcdf..cd719c5d0 100644 --- a/src/docs/inertia.md +++ b/src/docs/inertia.md @@ -236,6 +236,7 @@ Version labels refer to the Inertia client-side libraries. Features marked with - [Code Splitting](#code-splitting) - [Vite Plugin](#code-splitting-vite-plugin) - [Manual Vite](#code-splitting-manual-vite) + - [Preloading the Initial Page](#code-splitting-preloading-the-initial-page) - [DevTools](#devtools) - [Installation](#devtools-installation) - [The Timeline](#devtools-the-timeline) @@ -381,7 +382,7 @@ Next, create a `resources/views/app.blade.php` file. This root template will be ``` -For React applications, it's recommended to include the `@viteReactRefresh` directive before the `@vite` directive to enable Fast Refresh in development. Use `resources/js/app.jsx` in the `@vite` directive when following the React setup below, or `resources/js/app.tsx` for TypeScript. The entry point must match your Vite configuration. +For React applications, it's recommended to include the `@viteReactRefresh` directive before the `@vite` directive to enable Fast Refresh in development. You may also pass the current page component to the `@vite` directive to [preload the initial page](#code-splitting-preloading-the-initial-page). Use `resources/js/app.jsx` in the `@vite` directive when following the React setup below, or `resources/js/app.tsx` for TypeScript. The entry point must match your Vite configuration. The `` component renders a `
` element with an `id` of `app`. This element serves as the mounting point for your JavaScript application. You may customize the `id` by passing a different value to the component. @@ -12283,6 +12284,49 @@ resolve: name => { }, ``` + + +### Preloading the Initial Page + +On the first visit to your application, the browser must download and execute your main JavaScript bundle before it knows which page component to load. You may avoid this extra round trip by passing the current page component to the `@vite` directive in your root template, which preloads the page component and its dependencies alongside your main bundle. + +```blade tab=Vue +@vite(['resources/js/app.js', "resources/js/Pages/{$page['component']}.vue"]) +``` + +```blade tab=React +@vite(['resources/js/app.jsx', "resources/js/Pages/{$page['component']}.jsx"]) +``` + +```blade tab=Svelte +@vite(['resources/js/app.js', "resources/js/Pages/{$page['component']}.svelte"]) +``` + +The `$page` variable is available in your root template and contains the [page object](https://inertiajs.com/docs/v3/core-concepts/the-protocol#the-page-object). + + + +#### Link Headers + +Hypervel's `AddLinkHeadersForPreloadedAssets` middleware adds every preloaded asset to a `Link` response header, allowing the browser to start downloading them before parsing the HTML. Since the dependencies of the page component are included as well, this header may exceed your web server's response header buffer on larger pages, causing the initial page load to fail. Subsequent Inertia visits are not affected, as they return JSON without rendering the root template. + +You may limit the number of assets included in the header using the middleware's `using()` method: + +```php +use App\Http\Middleware\HandleInertiaRequests; +use Hypervel\Foundation\Configuration\Middleware; +use Hypervel\Http\Middleware\AddLinkHeadersForPreloadedAssets; + +->withMiddleware(function (Middleware $middleware) { + $middleware->web(append: [ + HandleInertiaRequests::class, + AddLinkHeadersForPreloadedAssets::using(limit: 10), + ]); +}) +``` + +Assets beyond the limit are still preloaded by the `` tags rendered by the `@vite` directive, so removing the middleware entirely has little impact. Alternatively, you may increase your web server's response header buffer, such as Nginx's `proxy_buffer_size` when proxying requests to Hypervel. + ## DevTools From 9fa4830632d1d35f744dd25c135f9443cee1c18a Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 2 Nov 2026 07:46:09 +0000 Subject: [PATCH 25/29] Track a targeted audit of cancellation through cleanup failures Record a separate audit of supported callback, fallback and I/O paths where cleanup failures can hide coroutine cancellation. Require focused evidence and regression coverage while retaining valid native checks and cancellation ownership boundaries. --- docs/todo.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/todo.md b/docs/todo.md index e352267f8..1fc9323f5 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -32,6 +32,7 @@ - Audit unmatched PHPStan inline ignores and global patterns with `reportUnmatchedIgnoredErrors` enabled — currently 196 unmatched inline ignores across 99 files plus 5 unmatched global patterns. Remove only suppressions that no longer match after tracing the underlying code; do not replace correct source with runtime branches or wider types merely to keep static analysis green. Decide as part of the work whether `phpstan.neon.dist` should then set `reportUnmatchedIgnoredErrors: true` permanently, since leaving it `false` lets the suppressions rot again. - Add PHPStan Eloquent extensions that preserve `Eloquent\Builder` for non-passthrough methods forwarded to `Query\Builder`, and expose model named scopes on Eloquent builders and relations. The query-builder mixin currently gives fluent calls the wrong builder type, while named scopes are treated as nonexistent methods; these gaps force scopes to split mutation from return and leave `HasDatabaseNotifications` with `method.notFound` suppressions. This will be the repository's first PHPStan extension, so use Larastan as prior art and wire the extensions into `phpstan.neon.dist` without maintaining duplicate query-method or scope lists in `@method` annotations. - Investigate static analysis that turns the coroutine-safety and worker-lifetime rules in `AGENTS.md` into automated checks, so mistakes no longer depend on agent steering and review alone. Decide which tool fits each check: PHPStan rules for detection with type information, Rector in dry-run mode, or another linter. Start with property writes outside the constructor on container-resolved classes (auto-singletons and singletons) that are not `Transient`, scoped, or execution-scoped, since per-request data stored there leaks to concurrent requests in the same worker. Intentional worker-lifetime state such as manager driver caches, registries, and resolved configuration needs an explicit way to opt out, so measure false positives across `src/` before choosing that mechanism. Then evaluate other patterns: static property writes outside boot code and `flushState()`; static caches keyed by request-derived values; request state captured in constructors of classes that are not execution-scoped; `Config::set()` during request handling; child coroutines started inside `DB::transaction()`, which borrow their own pooled connection and therefore run outside the transaction; and raw `sleep()`/`usleep()` in source instead of `Sleep`. Ship the useful rules so applications and private packages can enable the same checks. +- Audit remaining cancellation catches in callback, fallback and I/O paths for cleanup failures that hide cancellation in an exception chain. Trace supported execution paths, fix confirmed cases that retry or continue work after cancellation, and add focused regression tests. Preserve valid native-only checks, cleanup-error reporting and foreign-cancellation ownership; an exact exception check alone is not evidence of a defect. - Investigate a focused PHPStan rule for cancellation checks that miss cleanup exception chains. Detect bare `CanceledException` checks around callbacks or listeners, including `releaseIdleConnections()`, where `Coroutine::causedByCancellation()` is needed to prevent retries or continued work after cancellation. Use the cancellation regression cases to assess detection and false positives; preserve valid native-only checks, cleanup-error reporting and foreign-cancellation boundaries. Integrate useful checks with the coroutine-safety analysis above rather than attempting to infer every ownership decision automatically. - Audit typed input accessors across `InteractsWithData`, Support `DataObject`, and Hypervel Data. Define the accepted integer, float, and boolean forms for each public contract. Extract a neutral Support conversion primitive only if `InteractsWithData` and `DataObject` converge on exactly the same strict semantics, and decide separately whether Hypervel Data should adopt those semantics rather than changing its tested permissive casts as a side effect. From 2e555f383de6e61c84ae585988e942c9b02420e7 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 2 Nov 2026 08:37:13 +0000 Subject: [PATCH 26/29] Preserve chained cancellation through pool teardown and acquisition Drain all idle connections when cleanup wraps coroutine cancellation, and discard an unreturned borrow when maintenance is cancelled. Preserve cancellation precedence through failed logical-connection acquisition and cleanup without hiding genuine secondary cleanup errors. Extend direct-cancellation coverage with wrapped failures and exercise lazy lease reacquisition. The affected test files, targeted source analysis and formatting pass; the added cases fail before the fixes. --- src/connection-pool/src/ConnectionPool.php | 24 ++++++++------ src/database/src/ConnectionResolver.php | 9 +++--- src/database/src/Pool/ConnectionLease.php | 10 +++--- tests/ConnectionPool/ConnectionPoolTest.php | 14 ++++++--- tests/Database/ConnectionResolverTest.php | 14 ++++++--- .../Database/DatabaseConnectionLeaseTest.php | 31 +++++++++++++++++++ 6 files changed, 75 insertions(+), 27 deletions(-) diff --git a/src/connection-pool/src/ConnectionPool.php b/src/connection-pool/src/ConnectionPool.php index f271bde8d..e6defb891 100644 --- a/src/connection-pool/src/ConnectionPool.php +++ b/src/connection-pool/src/ConnectionPool.php @@ -105,16 +105,18 @@ public function borrow(): ConnectionContract } $this->idleMonitor?->start(); - } catch (CanceledException $cancellation) { - try { - $this->discard($connection); - } catch (CanceledException) { - } catch (Throwable $exception) { - $this->report($exception); + } catch (Throwable $exception) { + if (Coroutine::causedByCancellation($exception)) { + try { + $this->discard($connection); + } catch (CanceledException) { + } catch (Throwable $cleanupException) { + $this->report($cleanupException); + } + + throw $exception; } - throw $cancellation; - } catch (Throwable $exception) { $this->report($exception); } @@ -228,7 +230,11 @@ public function close(): void while ($connection = $this->popIdleConnection()) { try { $this->destroyConnection($connection); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { + if (! Coroutine::causedByCancellation($exception)) { + throw $exception; + } + $cancellation ??= $exception; } } diff --git a/src/database/src/ConnectionResolver.php b/src/database/src/ConnectionResolver.php index d785407c0..4ebaa6a63 100755 --- a/src/database/src/ConnectionResolver.php +++ b/src/database/src/ConnectionResolver.php @@ -11,7 +11,6 @@ use Hypervel\Database\Pool\ConnectionLease; use Hypervel\Database\Pool\PooledConnection; use Hypervel\Database\Pool\PoolManager; -use Swoole\Coroutine\CanceledException; use Throwable; use UnitEnum; @@ -358,11 +357,11 @@ protected function discardFailedConnection(ConnectionLease|PooledConnection $own { try { $owner->discard(); - } catch (CanceledException $cancellation) { - if (! $exception instanceof CanceledException) { - throw $cancellation; + } catch (Throwable $cleanupException) { + if (Coroutine::causedByCancellation($cleanupException) && ! Coroutine::causedByCancellation($exception)) { + throw $cleanupException; } - } catch (Throwable) { + // Preserve the connection setup failure. } } diff --git a/src/database/src/Pool/ConnectionLease.php b/src/database/src/Pool/ConnectionLease.php index 00a06e69b..e1ec16525 100644 --- a/src/database/src/Pool/ConnectionLease.php +++ b/src/database/src/Pool/ConnectionLease.php @@ -6,13 +6,13 @@ use Closure; use Hypervel\Context\NonCopyableContext; +use Hypervel\Coroutine\Coroutine; use Hypervel\Database\Connection; use Hypervel\Database\Connectors\ConnectionFactory; use Hypervel\Database\LeasableConnection; use Hypervel\Database\PdoConnection; use LogicException; use PDO; -use Swoole\Coroutine\CanceledException; use Throwable; /** @@ -229,11 +229,11 @@ protected function discardAfterFailure(Throwable $exception): void { try { $this->pooledConnection?->discard(); - } catch (CanceledException $cancellation) { - if (! $exception instanceof CanceledException) { - throw $cancellation; + } catch (Throwable $cleanupException) { + if (Coroutine::causedByCancellation($cleanupException) && ! Coroutine::causedByCancellation($exception)) { + throw $cleanupException; } - } catch (Throwable) { + // Preserve the acquisition or connection-listener failure. } } diff --git a/tests/ConnectionPool/ConnectionPoolTest.php b/tests/ConnectionPool/ConnectionPoolTest.php index 491e5e5f6..2e68fb854 100644 --- a/tests/ConnectionPool/ConnectionPoolTest.php +++ b/tests/ConnectionPool/ConnectionPoolTest.php @@ -131,9 +131,12 @@ public function testSnapshotsDistinguishCreationBorrowingAndCleanup(): void ], $pool->getStats()); } - public function testCloseDrainsEveryIdleConnectionBeforeRethrowingCancellation(): void + #[TestWith([false], 'direct cancellation')] + #[TestWith([true], 'cleanup failure carrying cancellation')] + public function testCloseDrainsEveryIdleConnectionBeforeRethrowingCancellation(bool $chained): void { $cancellation = new CanceledException('close canceled'); + $cancellation = $chained ? new RuntimeException('cleanup failed', previous: $cancellation) : $cancellation; $connections = [ new PoolConnectionStub(closeCallback: static fn (): bool => throw $cancellation), new PoolConnectionStub, @@ -152,7 +155,7 @@ static function () use (&$connections): Connection { try { $pool->close(); $this->fail('Closing an idle connection was expected to be canceled.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } @@ -833,9 +836,12 @@ public function testUsageTrackerFailureIsReportedWithoutLosingBorrow(): void $pool->release($connection); } - public function testUsageMaintenanceCancellationDiscardsTheUnreturnedBorrow(): void + #[TestWith([false], 'direct cancellation')] + #[TestWith([true], 'cleanup failure carrying cancellation')] + public function testUsageMaintenanceCancellationDiscardsTheUnreturnedBorrow(bool $chained): void { $cancellation = new CanceledException('maintenance canceled'); + $cancellation = $chained ? new RuntimeException('cleanup failed', previous: $cancellation) : $cancellation; $connections = [ new PoolConnectionStub, new PoolConnectionStub(closeCallback: static fn (): bool => throw $cancellation), @@ -865,7 +871,7 @@ public function shouldTrimExcessIdle(): bool try { $pool->borrow(); $this->fail('Usage maintenance was expected to be canceled.'); - } catch (CanceledException $exception) { + } catch (Throwable $exception) { $this->assertSame($cancellation, $exception); } diff --git a/tests/Database/ConnectionResolverTest.php b/tests/Database/ConnectionResolverTest.php index d46bd231c..ae0f07f6a 100644 --- a/tests/Database/ConnectionResolverTest.php +++ b/tests/Database/ConnectionResolverTest.php @@ -474,10 +474,13 @@ public function testDiscardFailureDoesNotReplaceTheSetupFailure(): void } } - public function testDiscardCancellationReplacesAnOrdinarySetupFailure(): void + #[TestWith([false], 'direct cancellation')] + #[TestWith([true], 'cleanup failure carrying cancellation')] + public function testDiscardCancellationReplacesAnOrdinarySetupFailure(bool $chained): void { $setupException = new RuntimeException('Connection retrieval failed.'); $discardCancellation = new CanceledException('Discard was canceled.'); + $discardCancellation = $chained ? new RuntimeException('Discard cleanup failed.', previous: $discardCancellation) : $discardCancellation; $poolManager = m::mock(PoolManager::class); $pool = m::mock(DatabasePool::class); $wrapper = m::mock(PooledConnection::class, ['usesSessionLeases' => false]); @@ -493,7 +496,7 @@ public function testDiscardCancellationReplacesAnOrdinarySetupFailure(): void try { $resolver->connection(); $this->fail('Expected discard cancellation to propagate.'); - } catch (CanceledException $throwable) { + } catch (Throwable $throwable) { $this->assertSame($discardCancellation, $throwable); } } @@ -522,9 +525,12 @@ public function testSetupCancellationRemainsPrimaryOverAnOrdinaryDiscardFailure( } } - public function testSetupCancellationRemainsPrimaryOverDiscardCancellation(): void + #[TestWith([false], 'direct cancellation')] + #[TestWith([true], 'setup failure carrying cancellation')] + public function testSetupCancellationRemainsPrimaryOverDiscardCancellation(bool $chained): void { $setupCancellation = new CanceledException('Connection setup was canceled.'); + $setupCancellation = $chained ? new RuntimeException('Connection setup cleanup failed.', previous: $setupCancellation) : $setupCancellation; $discardCancellation = new CanceledException('Discard was canceled.'); $poolManager = m::mock(PoolManager::class); $pool = m::mock(DatabasePool::class); @@ -541,7 +547,7 @@ public function testSetupCancellationRemainsPrimaryOverDiscardCancellation(): vo try { $resolver->connection(); $this->fail('Expected setup cancellation to propagate.'); - } catch (CanceledException $throwable) { + } catch (Throwable $throwable) { $this->assertSame($setupCancellation, $throwable); } } diff --git a/tests/Database/DatabaseConnectionLeaseTest.php b/tests/Database/DatabaseConnectionLeaseTest.php index 2602a8b74..5c3b18a9a 100644 --- a/tests/Database/DatabaseConnectionLeaseTest.php +++ b/tests/Database/DatabaseConnectionLeaseTest.php @@ -21,7 +21,9 @@ use Hypervel\Database\Events\ConnectionEstablished; use Hypervel\Database\Events\QueryExecuted; use Hypervel\Database\PdoConnection; +use Hypervel\Database\Pool\ConnectionLease; use Hypervel\Database\Pool\DatabasePool; +use Hypervel\Database\Pool\PooledConnection; use Hypervel\Database\Pool\PoolManager; use Hypervel\Database\PostgresConnection; use Hypervel\Database\Schema\Blueprint; @@ -56,6 +58,7 @@ use Hypervel\Tests\Grpc\Fixtures\ClientCallClientFactory; use Hypervel\Tests\Socialite\Fixtures\GenericTestProviderStub; use LogicException; +use Mockery as m; use PDO; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\RequiresOperatingSystem; @@ -1010,6 +1013,34 @@ public function testSessionConfiguratorsRunOnTheLogicalOwnerAfterEachBorrowerCha ], $configurator->applied); } + public function testAcquisitionCleanupPropagatesWrappedCancellation(): void + { + $setupFailure = new RuntimeException('Connection listener failed.'); + $cleanupFailure = new RuntimeException('Discard cleanup failed.', previous: new CanceledException('Discard canceled.')); + $physical = new SQLiteConnection(new PDO('sqlite::memory:'), ':memory:', '', [ + 'driver' => 'sqlite', + 'database' => ':memory:', + 'prefix' => '', + ]); + $pool = m::mock(DatabasePool::class); + $wrapper = m::mock(PooledConnection::class); + $pool->expects('borrow')->once()->andReturn($wrapper); + $wrapper->expects('attachLease')->twice(); + $wrapper->expects('getDriverConnection')->once()->andReturn($physical); + $wrapper->expects('dispatchConnectionEstablishedEvent')->once()->andThrow($setupFailure); + $wrapper->expects('discard')->once()->andThrow($cleanupFailure); + + $lease = new ConnectionLease($pool, $wrapper, $this->app->make('db.factory'), $physical); + $lease->detach(); + + try { + $lease->connection->getPdo(); + $this->fail('Expected cleanup cancellation to propagate.'); + } catch (Throwable $exception) { + $this->assertSame($cleanupFailure, $exception, $exception::class . ': ' . $exception->getMessage()); + } + } + #[DataProvider('connectionListenerFailures')] public function testConnectionListenerFailuresSettleTheLeaseOnce(bool $reacquire, bool $cancel): void { From 3fd0a4f1802652e61ce5cb514c430ddd4d03c1fb Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 2 Nov 2026 08:37:13 +0000 Subject: [PATCH 27/29] Reject SQLite test restoration while a pooled session is borrowed Prevent a restored in-memory SQLite pool from retaining a borrowed wrapper on its old PDO. Fail before draining or replacing state; callers can restore once their pinned session ends. The regression covers rejection without mutation and successful restoration after unpinning. Existing pooled refresh, lazy refresh and truncation tests pass. --- src/database/src/Pool/DatabasePool.php | 5 ++++ .../DatabaseConnectionResolverTest.php | 28 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/database/src/Pool/DatabasePool.php b/src/database/src/Pool/DatabasePool.php index b0bf9f201..3c28e41d9 100644 --- a/src/database/src/Pool/DatabasePool.php +++ b/src/database/src/Pool/DatabasePool.php @@ -17,6 +17,7 @@ use Hypervel\Database\SQLiteDatabase; use Hypervel\Support\Arr; use InvalidArgumentException; +use LogicException; use PDO; use Swoole\Coroutine\CanceledException; use Throwable; @@ -139,6 +140,10 @@ public function getSharedInMemorySqlitePdo(): ?PDO */ public function restoreSharedInMemorySqlitePdo(PDO $pdo): void { + if ($this->getBorrowedCount() > 0) { + throw new LogicException('Cannot restore the in-memory SQLite database while connections are borrowed.'); + } + while ($connection = $this->popIdleConnection()) { $this->destroyConnection($connection); } diff --git a/tests/Foundation/Testing/DatabaseConnectionResolverTest.php b/tests/Foundation/Testing/DatabaseConnectionResolverTest.php index 4cfe5f9dd..ecd6d5f2e 100644 --- a/tests/Foundation/Testing/DatabaseConnectionResolverTest.php +++ b/tests/Foundation/Testing/DatabaseConnectionResolverTest.php @@ -15,6 +15,7 @@ use Hypervel\Testbench\Attributes\DefineEnvironment; use Hypervel\Testbench\Attributes\WithConfig; use Hypervel\Testbench\TestCase; +use LogicException; use Mockery as m; use PDO; use ReflectionProperty; @@ -53,6 +54,33 @@ public function testRestoredPdoSurvivesIdleRelease(): void $this->assertSame(42, $connection->table('restored_database')->value('value')); } + #[DefineEnvironment('definePooledDatabase')] + public function testRestorationRejectsAPinnedSessionWithoutReplacingTheDatabase(): void + { + $resolver = $this->app->make('db.resolver'); + $connection = $resolver->connection('testing'); + $pool = $this->app->make(PoolManager::class)->pool('testing'); + $previous = $pool->getSharedInMemorySqlitePdo(); + $saved = new PDO('sqlite::memory:'); + + $connection->withPinnedSession(function () use ($resolver, $connection, $pool, $previous, $saved): void { + $this->assertSame($previous, $connection->getPdo()); + + try { + $resolver->restoreInMemoryPdoIfPooled('testing', $saved); + $this->fail('Expected restoration to reject the borrowed session.'); + } catch (LogicException $exception) { + $this->assertSame('Cannot restore the in-memory SQLite database while connections are borrowed.', $exception->getMessage()); + } + + $this->assertSame($previous, $pool->getSharedInMemorySqlitePdo()); + $this->assertSame($previous, $connection->getPdo()); + }); + + $this->assertTrue($resolver->restoreInMemoryPdoIfPooled('testing', $saved)); + $this->assertSame($saved, $connection->getPdo()); + } + public function testFlushDisconnectsCachedConnection(): void { $resolver = $this->app->make(DatabaseConnectionResolver::class); From 055b97e59a4f346b05643b4dbcf7929d9d49892d Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 2 Nov 2026 08:37:13 +0000 Subject: [PATCH 28/29] Clarify the required lock for skipLocked Use one accurate message for missing and custom lock clauses. Keep the existing guard and cover all three invalid call shapes with exact exception-message assertions. The query builder test file, targeted analysis and formatting pass. --- src/database/src/Query/Builder.php | 4 ++-- tests/Database/DatabaseQueryBuilderTest.php | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/database/src/Query/Builder.php b/src/database/src/Query/Builder.php index 8374e230a..1705f1b24 100644 --- a/src/database/src/Query/Builder.php +++ b/src/database/src/Query/Builder.php @@ -2981,7 +2981,7 @@ public function unionAll(Closure|self|EloquentBuilder $query): static public function lock(string|bool $value = true): static { if ($this->skipLocked && is_string($value)) { - throw new InvalidArgumentException('Skip locked requires lockForUpdate() or sharedLock(), not a custom lock clause.'); + throw new InvalidArgumentException('Skip locked requires lockForUpdate() or sharedLock().'); } $this->lock = $value; @@ -3012,7 +3012,7 @@ public function sharedLock(): static public function skipLocked(): static { if (! is_bool($this->lock)) { - throw new InvalidArgumentException('Skip locked requires lockForUpdate() or sharedLock(), not a custom lock clause.'); + throw new InvalidArgumentException('Skip locked requires lockForUpdate() or sharedLock().'); } $this->skipLocked = true; diff --git a/tests/Database/DatabaseQueryBuilderTest.php b/tests/Database/DatabaseQueryBuilderTest.php index 8d40ae15a..8ef047517 100755 --- a/tests/Database/DatabaseQueryBuilderTest.php +++ b/tests/Database/DatabaseQueryBuilderTest.php @@ -6947,7 +6947,7 @@ public static function skipLockedGrammarProvider(): array public function testSkipLockedRequiresAnExistingLock(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Skip locked requires lockForUpdate() or sharedLock(), not a custom lock clause.'); + $this->expectExceptionMessageIs('Skip locked requires lockForUpdate() or sharedLock().'); $this->getBuilder()->skipLocked(); } @@ -6955,7 +6955,7 @@ public function testSkipLockedRequiresAnExistingLock(): void public function testSkipLockedCannotFollowACustomLock(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Skip locked requires lockForUpdate() or sharedLock(), not a custom lock clause.'); + $this->expectExceptionMessageIs('Skip locked requires lockForUpdate() or sharedLock().'); $this->getBuilder()->lock('for update nowait')->skipLocked(); } @@ -6963,7 +6963,7 @@ public function testSkipLockedCannotFollowACustomLock(): void public function testCustomLockCannotFollowSkipLocked(): void { $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Skip locked requires lockForUpdate() or sharedLock(), not a custom lock clause.'); + $this->expectExceptionMessageIs('Skip locked requires lockForUpdate() or sharedLock().'); $this->getBuilder()->lockForUpdate()->skipLocked()->lock('for update nowait'); } From f32b84ac40ec2253394580edc7063ffbc874a675 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Mon, 2 Nov 2026 08:37:13 +0000 Subject: [PATCH 29/29] Describe decimal parsing test datasets Add method titles to the request and Eloquent decimal parsing providers. Both affected test files pass, with the Eloquent cases using in-memory SQLite. --- tests/Foundation/Http/FormRequestCastingTest.php | 3 +++ tests/Integration/Database/EloquentModelDecimalCastingTest.php | 3 +++ 2 files changed, 6 insertions(+) diff --git a/tests/Foundation/Http/FormRequestCastingTest.php b/tests/Foundation/Http/FormRequestCastingTest.php index 4d61fb881..205c1336e 100644 --- a/tests/Foundation/Http/FormRequestCastingTest.php +++ b/tests/Foundation/Http/FormRequestCastingTest.php @@ -304,6 +304,9 @@ public function testDecimalParsingPreservesPrecisionAndBoundsExponentExpansion(f $this->assertSame($expected, $request->safe()->all()['value']); } + /** + * Provide decimal inputs for precision and exponent-limit checks. + */ public static function decimalParsingProvider(): array { return [ diff --git a/tests/Integration/Database/EloquentModelDecimalCastingTest.php b/tests/Integration/Database/EloquentModelDecimalCastingTest.php index c724f1a86..71706b8b1 100644 --- a/tests/Integration/Database/EloquentModelDecimalCastingTest.php +++ b/tests/Integration/Database/EloquentModelDecimalCastingTest.php @@ -61,6 +61,9 @@ public function testDecimalParsingPreservesPrecisionAndBoundsExponentExpansion(f $this->assertSame($expected, $model->toArray()['amount']); } + /** + * Provide decimal inputs for precision and exponent-limit checks. + */ public static function decimalParsingProvider(): array { return [