Add database read/write suffix routing#407
Merged
Merged
Conversation
Document the signed-off plan for Hypervel-native database read/write suffix routing. The plan records why automatic routing remains the default, how explicit ::read and ::write suffixes are adapted for coroutine-safe pools, why ::direct stays intentionally unsupported, and how pooled URL parsing, db command merging, tests, and docs are updated.
Add a small value object for parsing database connection suffixes in one place. The helper exposes the requested name, base name, and optional read/write role, and rejects Laravel's ::direct suffix with the Hypervel-native guidance to use named connections plus migrations_connection instead. Add focused tests for plain names, read/write suffixes, and the explicit ::direct rejection path.
Teach Connection to carry an immutable read/write role marker from its config. This lets derived read connections report read-side query metadata without mutating shared connection state, while normal split connections continue to report the latest PDO side used. Fix connection detail reporting for derived read connections so query exceptions use the concrete derived config instead of an empty read config, and cover the sticky routing and pool reset behavior that keeps forced write reads from leaking across borrows.
Move read-side config derivation behind ConnectionFactory helpers so every resolver path uses the same rules. The factory now exposes URL parsing, detects non-null read config with the same gate used by normal connection creation, and derives read configs that strip nested read/write branches while preserving the base connection name and read role marker. Tests cover URL parsing, empty and null read config behavior, role marker placement, and the fact that write suffixes keep using the base config path.
Make database pool selection understand read/write suffixes without changing the normal pooled query path. A configured read side now gets a derived read pool keyed by name::read, write suffixes use the base pool, and unsplit read/write suffixes remain compatibility aliases of the base pool. Pool flushing now clears base and derived variants together. The pool constructor also parses URL-backed configs before pool setup, rejects derived in-memory SQLite read pools, and keeps the exact-key fast path for already-created pools. Tests cover base/write aliases, derived read pools, null read configs, URL-backed pooled configs, pool flushing, and the in-memory SQLite guard.
…ntext Make the pooled connection resolver parse requested connection suffixes and cache each requested name under its own coroutine context key. Explicit ::write borrows from the base pool and forces reads through the write connection for that borrow only. Explicit ::read can borrow the derived read pool when configured, and all mutable routing state is released through the existing coroutine defer path. Integration tests prove suffixes are isolated across concurrent coroutines and that using a suffixed connection in one coroutine does not affect plain connection reads in another.
Add read/write suffix parsing to non-pooled database manager resolution so direct manager usage matches pooled resolver behavior. The manager now builds requested suffix variants from the base config, derives read configs through the factory helpers, forces write reads for ::write, and rejects ::direct through the shared parser. Cleanup paths now handle base, read, and write variants deliberately: disconnect targets current cached variants while purge clears every variant and the related pools. Event resolution was also aligned with the container make() convention. Tests cover suffixed connections, unsplit aliases, explicit ::direct rejection, disconnect, purge, reconnect, and connection-established event dispatch.
Teach the foundation testing database resolver to cache and reset connections by requested read/write suffix. The resolver now applies the same base-name parsing as the runtime database manager, keeps the base testing configuration lookup, and reapplies forced-write routing after reset when the requested test connection is ::write. Tests cover distinct base/read/write resolver slots and verify write suffix state survives resolver reset for the active test connection.
Adapt the database console command's read/write config merge logic for nested connection configs. The command now handles list-style read/write configs, host arrays, empty nested configs, and strips nested read/write branches after selecting the requested side. It also uses the database URL parser and resolves config through the container make() path. Tests cover read and write list configs, host-array normalization, empty nested config fallback, stripped nested config branches, and URL-backed command configs.
Add end-to-end database coverage for explicit read/write suffixes. The integration tests cover automatic read/write routing, forced ::read and ::write behavior, unsplit connection aliases, query log and query exception read/write metadata, base-name transaction handling, and the deliberate omission of Laravel's ::direct suffix. This coverage keeps the public API parity additions tied to real database behavior rather than only unit-level config parsing.
Document explicit ::read and ::write connection suffixes, how they fit with Hypervel's automatic read/write routing, and how pooled read/write connections should be sized operationally. The Boost database guide now explains pool slot behavior, explicit suffix use cases, db command read/write config handling, and the Hypervel direct-connection model based on named connections plus migrations_connection. The database package README records the intentional difference from Laravel's ::direct suffix so future ports do not reintroduce a Laravel-specific external-pooler shortcut.
📝 WalkthroughWalkthroughAdds Laravel-compatible ChangesDatabase read/write connection routing
Sequence Diagram(s)sequenceDiagram
participant ConnectionResolver
participant ConnectionName
participant PoolFactory
participant DbPool
participant Connection
ConnectionResolver->>ConnectionName: parse(requestedName)
ConnectionResolver->>PoolFactory: getPool(requested)
PoolFactory->>ConnectionName: parse(name), check hasReadConfig
PoolFactory->>DbPool: resolve/create pool for base or read variant
DbPool-->>PoolFactory: PooledConnection
PoolFactory-->>ConnectionResolver: PooledConnection
ConnectionResolver->>Connection: useWriteConnectionWhenReading() if write
ConnectionResolver-->>ConnectionResolver: cache in coroutine Context
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds Laravel-style
Confidence Score: 5/5Safe to merge — all changed paths are well-tested, the reconnector fix is verified correct, and pool state is properly isolated per-coroutine. The implementation handles every major edge case: reconnect restores the correct side, pool release clears forced-write routing, in-memory SQLite read pools are guarded, unsplit connections fall back to base-connection aliases, and the Testbench resolver preserves suffix behaviour across test resets. The test suite covers unit and integration paths including coroutine isolation, reconnect, purge, and CLI config merging. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "fix(database): address read write routin..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/database/src/Pool/PoolFactory.php (1)
31-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLGTM overall — pool name resolution and flush logic are correct and well covered by tests.
One recommended cleanup:
getPoolName()here and the read-config derivation inDbPool::__construct(src/database/src/Pool/DbPool.php lines 49-67) both independently fetch the config service, build the samedatabase.connections.{base}key, and callConnectionFactory::hasReadConfig(). Both call sites are individually necessary (DbPool can be constructed directly, bypassing this factory, as the integration tests demonstrate), but the duplicated lookup logic could be consolidated into a shared helper (e.g., onConnectionFactoryorConnectionName) to avoid drift between the two implementations if the resolution logic changes later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/database/src/Pool/PoolFactory.php` around lines 31 - 121, The read-pool name resolution logic is duplicated between PoolFactory::getPoolName() and DbPool::__construct, which risks the two paths drifting apart. Extract the shared database.connections.{base} config lookup plus ConnectionFactory::hasReadConfig() decision into a common helper on ConnectionFactory or ConnectionName, then have both PoolFactory and DbPool call that helper so they use the same resolution rules.src/foundation/src/Testing/DatabaseConnectionResolver.php (1)
153-183: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent defensive guard between cached and freshly-resolved write connections.
The cached-hit branch checks
$connectionName->isWrite() && $connection instanceof Connectionbefore callinguseWriteConnectionWhenReading(), but the cache-miss branch (lines 179-181) calls it unconditionally. In practiceConnectionFactory::createConnection()'s declared return type guarantees aConnectioninstance here, so this isn't currently reachable as a bug, but the asymmetry is worth aligning for consistency and future-proofing against custom resolvers/mocks stored in the static cache.♻️ Suggested fix for consistency
- if ($connectionName->isWrite()) { + if ($connectionName->isWrite() && $connection instanceof Connection) { $connection->useWriteConnectionWhenReading(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/foundation/src/Testing/DatabaseConnectionResolver.php` around lines 153 - 183, The write-connection handling in DatabaseConnectionResolver::connection is inconsistent between the cached-hit and cache-miss paths. Update the cache-miss branch to use the same defensive guard as the cached branch before calling useWriteConnectionWhenReading(), keeping the logic aligned for both static::$connections hits and freshly resolved connections. This should be a small consistency fix within connection() that preserves current behavior while matching the existing instanceof Connection check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/database/src/Connectors/ConnectionFactory.php`:
- Around line 141-149: The merged read configuration from configForRead() is
returned without re-running parseConfig(), so a nested read.url can remain
unexpanded and keep the writer settings. Update
ConnectionFactory::configForRead() to first build the read config via
getReadConfig(), then pass the merged result back through parseConfig() before
adding the read/write type key. Add a regression test covering a config with
read => ['url' => ...] to verify the derived read connection uses the nested URL
correctly.
In `@tests/Integration/Database/ConnectionCoroutineSafetyTest.php`:
- Around line 34-49: The SQLite fixtures in setUpBeforeClass are being created
with sys_get_temp_dir(), which bypasses the framework-managed test temp root and
can collide in parallel runs. Update ConnectionCoroutineSafetyTest to create
static::$readPath and static::$writePath using the testbench/ParallelTesting
temp directory helper instead, and keep tearDownAfterClass unlinking those same
paths so the files stay isolated and disposable.
In `@tests/Integration/Database/DatabaseTransactionsTest.php`:
- Around line 18-33: The SQLite fixture setup in
DatabaseTransactionsTest::setUpBeforeClass and tearDownAfterClass is creating
files in sys_get_temp_dir(), bypassing the testbench/ParallelTesting temp area.
Update the fixture paths to use the framework-managed test temp directory helper
used by this test suite, then keep the existing touch/unlink lifecycle against
those new paths. Make sure the identifiers static::$readPath and
static::$writePath still point to isolated per-run files so parallel tests
remain safe.
---
Nitpick comments:
In `@src/database/src/Pool/PoolFactory.php`:
- Around line 31-121: The read-pool name resolution logic is duplicated between
PoolFactory::getPoolName() and DbPool::__construct, which risks the two paths
drifting apart. Extract the shared database.connections.{base} config lookup
plus ConnectionFactory::hasReadConfig() decision into a common helper on
ConnectionFactory or ConnectionName, then have both PoolFactory and DbPool call
that helper so they use the same resolution rules.
In `@src/foundation/src/Testing/DatabaseConnectionResolver.php`:
- Around line 153-183: The write-connection handling in
DatabaseConnectionResolver::connection is inconsistent between the cached-hit
and cache-miss paths. Update the cache-miss branch to use the same defensive
guard as the cached branch before calling useWriteConnectionWhenReading(),
keeping the logic aligned for both static::$connections hits and freshly
resolved connections. This should be a small consistency fix within connection()
that preserves current behavior while matching the existing instanceof
Connection check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 44fac691-259c-4c17-8106-e6e105cff8ff
📒 Files selected for processing (23)
docs/plans/2026-06-30-database-read-write-routing-and-pool-config.mdsrc/boost/docs/database.mdsrc/database/README.mdsrc/database/src/Connection.phpsrc/database/src/ConnectionName.phpsrc/database/src/ConnectionResolver.phpsrc/database/src/Connectors/ConnectionFactory.phpsrc/database/src/Console/DbCommand.phpsrc/database/src/DatabaseManager.phpsrc/database/src/Pool/DbPool.phpsrc/database/src/Pool/PoolFactory.phpsrc/foundation/src/Testing/DatabaseConnectionResolver.phptests/Database/ConnectionNameTest.phptests/Database/DatabaseConnectionFactoryTest.phptests/Database/DatabaseConnectionTest.phptests/Database/DatabaseDbCommandTest.phptests/Database/DatabaseManagerTest.phptests/Database/PoolFactoryTest.phptests/Foundation/Testing/DatabaseConnectionResolverTest.phptests/Integration/Database/ConnectionCoroutineSafetyTest.phptests/Integration/Database/DatabaseConnectionsTest.phptests/Integration/Database/DatabaseTransactionsTest.phptests/Integration/Database/PooledConnectionTest.php
Fix the manager-level reconnect path for non-pooled derived read connections by reconstructing the ::read suffix from the read-side config marker before reconnecting. This prevents a direct ::read connection from silently reconnecting through the base write-side connection after its PDO is lost. Parse merged read-side configuration inside ConnectionFactory::configForRead before adding the read marker so nested read.url values are fully resolved before pool validation and connection creation. This keeps derived read configs self-contained and lets the in-memory SQLite guard see parsed read URLs. Add focused regression coverage for non-pooled ::read and ::write reconnects, nested read.url parsing, and derived read pools backed by in-memory SQLite URLs. Convert touched SQLite fixture tests to ParallelTesting::tempDir so parallel workers do not share temp files, including the previously fixed-name pooled extension test database. Also fix the PHPStan TypedValue prompt issue encountered during composer fix by simplifying the preg_match_all control flow instead of suppressing the analysis error. Verification: composer fix.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/Integration/Database/PooledConnectionTest.php (1)
96-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting a shared temp-SQLite-directory helper.
The pattern
new Filesystem+ParallelTesting::tempDir(...)+ensureDirectoryExists+try { ... } finally { deleteDirectory }is now repeated across four test methods in this file. A small private helper (e.g.withTempSqliteDirectory(string $suffix, callable $callback)) would remove the duplication and reduce the chance of the pattern drifting across methods over time.Also applies to: 153-185, 187-228, 649-658
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Integration/Database/PooledConnectionTest.php` around lines 96 - 128, The temp SQLite setup/teardown pattern is duplicated across multiple tests in PooledConnectionTest, so extract it into a shared private helper such as withTempSqliteDirectory that wraps the Filesystem, ParallelTesting::tempDir, ensureDirectoryExists, and cleanup in one place. Update testPoolParsesUrlConfigurationBeforeCreatingConnection and the other repeated test methods to use that helper so the directory lifecycle logic stays consistent and easier to maintain.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/Integration/Database/PooledConnectionTest.php`:
- Around line 96-128: The temp SQLite setup/teardown pattern is duplicated
across multiple tests in PooledConnectionTest, so extract it into a shared
private helper such as withTempSqliteDirectory that wraps the Filesystem,
ParallelTesting::tempDir, ensureDirectoryExists, and cleanup in one place.
Update testPoolParsesUrlConfigurationBeforeCreatingConnection and the other
repeated test methods to use that helper so the directory lifecycle logic stays
consistent and easier to maintain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a073c5cd-aa2c-49ff-82df-f4dd03d5fae3
📒 Files selected for processing (9)
src/database/src/Connectors/ConnectionFactory.phpsrc/database/src/DatabaseManager.phpsrc/prompts/src/Concerns/TypedValue.phptests/Database/DatabaseConnectionFactoryTest.phptests/Database/DatabaseManagerTest.phptests/Integration/Database/ConnectionCoroutineSafetyTest.phptests/Integration/Database/DatabaseConnectionsTest.phptests/Integration/Database/DatabaseTransactionsTest.phptests/Integration/Database/PooledConnectionTest.php
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/Database/DatabaseConnectionFactoryTest.php
- src/database/src/Connectors/ConnectionFactory.php
- tests/Integration/Database/DatabaseTransactionsTest.php
- src/database/src/DatabaseManager.php
- tests/Integration/Database/ConnectionCoroutineSafetyTest.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
For more details, see: docs/plans/2026-06-30-database-read-write-routing-and-pool-config.md
This adds Hypervel-native support for Laravel-style explicit database read/write connection suffixes:
DB::connection('name::read')andDB::connection('name::write'). Normal application queries still use Hypervel's existing automatic read/write routing; the suffixes are for explicit inspection, diagnostics, and code paths that need a connection name for a specific side.The implementation intentionally does not port Laravel's
::directsuffix. Hypervel's external-pooler model uses normal named connections plusmigrations_connection, which keeps direct and pooled endpoints explicit and avoids mutating shared connection objects in a long-lived Swoole worker.Details
ConnectionNameparser for database connection suffixes, including an explicit::directrejection with Hypervel guidance.ConnectionFactory, including URL parsing through the database parser.name::readuses a separate read pool when the base connection has read config.name::writeuses the base pool and forces reads through the write connection for that borrow.::readand::writenames remain compatibility aliases of the base connection.CoroutineContext.DatabaseManagerresolution, disconnect, purge, and reconnect suffix-aware.db --read/db --writeconfig merging for list-style configs, host arrays, empty nested configs, and stripped nested branches.::directsuffix.Performance And Coroutine Safety
The hot query path keeps the existing automatic routing model. Suffix parsing and config derivation happen when resolving or creating connections and pools, not per query.
For pooled connections, suffix state is scoped to the requested coroutine context key. Forced write routing for
name::writeis applied only to the borrowed connection and is cleared by the existing pool reset path before the connection is reused.The read pool is created only when a configured read side exists.
name::writedoes not create another physical pool; it borrows from the base pool and uses the existing write-read routing flag.Testing
ConnectionName, factory read config derivation, connection metadata, pool keying, manager cleanup, Testbench resolver behavior, and db command config merging.composer fixsuccessfully before committing this branch.Summary by CodeRabbit
Summary by CodeRabbit
New Features
::read/::writeconnection suffix routing for database operations.--read/--writehandling for nested settings and host arrays.Bug Fixes
::directand derived in-memory SQLite read pools.Documentation