Improve database extensibility and testing lifecycle correctness - #575
Merged
Conversation
Move read/write routing policy into the protocol-neutral connection so native and HTTP drivers can honor the same transaction, forced-write, and sticky-read behavior as PDO drivers. Keep resource selection in the concrete driver and retain existing PDO cursor timing. Add an opt-in streaming execution boundary that defers execution until iteration, reports success only after exhaustion, and preserves cleanup and cancellation. Restore the exact retrieved role across consumer yields, use the existing retry policy only before a value is yielded, and share query-exception construction without changing public execution signatures. Cover deferred hooks, retry and failure paths, abandonment, nested query roles, exception overrides, routing, and pool reset. Full framework composer fix passed; the final connection test rerun passed with 122 tests and 553 assertions.
Carry custom binding-slot types through query builder factories and cloning while preserving existing runtime binding APIs. Use native static returns where factories already preserve the concrete builder; keep subquery returns broad enough for a join to create a parent query builder. Centralize ordinary Blueprint column creation behind a typed factory. Preserve base-typed heterogeneous storage and specialized foreign-ID definitions, allowing custom column modifiers without pretending every stored definition has the same subtype. Extend runtime tests and PHPStan fixtures for nested queries, pagination clones, custom slots, inherited column helpers, and mixed definition storage. Full framework composer fix and the final source/type analysis passed.
Let schema builders own migration-repository creation and bulk testing truncation while retaining the existing repository and Foundation testing lifecycle. Keep the default relational migration table and normalize numeric-string batch aggregates to the declared integer return type. Apply table filters and prefix handling before bulk delegation. Route default row-existence checks to the writer so stale replicas cannot leave primary rows behind. Restore the exact event dispatcher in finally blocks when transaction setup, rollback, discovery, or truncation fails. Add schema, migration, failure-identity, and SQLite read/write regression coverage, update the Schema facade and native testing documentation, and retain existing seeding and transaction behavior. Full framework composer fix passed, including parallel, Testbench, and dogfood suites.
Preserve primary URL components as literal strings after one percent-decoding pass instead of JSON-decoding credentials and host names. Keep the native integer port and the existing null-host convention used by SQLite and role-only URLs. Continue converting query options to native configuration types, preserving that separate public contract. This prevents numeric, boolean-looking, and JSON-looking credentials from changing before database and other service factories receive them. Add strict parser cases for literal and encoded credentials, missing versus empty values, hosts, ports, and typed query options, plus a config-first database resolver regression. Full framework composer fix passed, covering shared parser consumers.
Retain complete read/write endpoint records when resolving a custom config-first driver through an explicit read alias. Reuse the existing name-before-driver resolver lookup instead of introducing another registry. Keep the selected read projection local to pool options and SQLite classification. PDO drivers still receive the same projected record, while custom drivers receive complete configuration and the role marker through creation and reconnect. Parse URL configuration before deciding whether a derived read pool exists. Cover direct and pooled custom-driver reconnects, read-side pool overrides, normalized timeouts, and URL-only read records while preserving existing PDO and SQLite guards. Full framework composer fix passed.
Add an extension-only database CLI manager and immutable launch configuration so custom drivers can use the existing db command without opening a database connection. Keep built-in command helpers and subclass overrides active, and launch through one common process path. Resolve selected role URLs before final host-list normalization, handle empty lists through the existing diagnostic, and preserve zero-valued credentials. Retain the nullable environment helper contract while supplying a normalized environment to the process. Test extension precedence, built-in helper results, role and URL selection, hostless extensions, and actual process arguments through isolated launch tests. Full framework composer fix passed; the final isolated launch rerun passed with 2 tests and 22 assertions.
Document protocol-neutral routing, streaming completion and retry boundaries, custom exception construction, migration and truncation schema hooks, and typed query and Blueprint extension contracts. Explain complete endpoint configuration and explicit read-alias ownership separately from pool-option projection, and show boot-time registration of custom command-line clients. State that streamed duration includes consumer work between yields and contributes to cumulative duration thresholds. Keep these additive capabilities in their canonical feature documentation. Checked the examples and contracts against the reviewed implementation; full framework checks passed.
Keep the porting guide focused on existing Laravel usage that requires adaptation or a compatibility check, rather than cataloguing opt-in framework additions. Remove the duplicated conditional-provider section; its canonical explanation remains in the provider documentation. Replace repeated Redis tag-mode details with the storage-compatibility warning and a link to the cache documentation. Document the concrete return contract required by custom query builder factory overrides and link to the database extension guide. Verified that the canonical provider section and Redis tag-mode anchor remain present.
Record the owner-approved requirement to group configuration settings under Laravel-style section comment blocks with concise user-facing explanations. Place the rule alongside the existing configuration conventions so newly written and ported configuration files follow the same familiar structure. This instruction-only change is separate from framework implementation and porting-guide updates.
Add StreamClosedException for drivers whose active response stream is explicitly closed while iteration is suspended. Pass it through both runStreaming exception boundaries unchanged, keeping deliberate termination distinct from a failed query and from ordinary exhaustion. Do not increment the connection error count, retry the query, emit query events, or record successful execution for this signal. Leave buffered execution and PDO cursor behavior unchanged. Verify exception identity and resource cleanup before and after the first yielded value, with no query log or duration accounting. Document the additive driver contract alongside the streaming extension API.
Rename the existing protected timeout-specific guard to ensureCanEmbedQuery so query builder extensions can validate statement-level options at the established attachment boundaries. Keep the timeout check, exception message, and all four call sites unchanged in behavior. Validation remains attachment-time only, with no recursive query traversal or additional compilation work. Document the extension contract and parent-call requirement. Add focused tests for override dispatch, rejection before outer-query mutation, and retained-child behavior across parsed, scalar, exists, and union subqueries. Validation: 440 query builder tests with 1706 assertions; full source and fixture static analysis; focused formatting checks.
Accept mixed having operands consistently with ordinary where predicates, retaining the existing operator resolution and driver binding preparation. Exclude inline expressions from value-between bindings and normalize array operands through the existing scalar-value hook so one placeholder receives one binding. Preserve expression and driver-owned objects through day and month predicates instead of coercing them to the integer one. Materialize iterable range bounds once after DatePeriod resolution so generators and keyed collections can be compiled repeatedly without retaining an exhausted or non-indexable source. Add focused regressions for overloads, nested predicates, expression binding counts, scalar coercion, object identity, date formatting, iterable consumption, and real SQLite execution. Verified the 461-test query-builder corpus, full production and fixture static analysis, formatting, and whitespace checks.
Retain the root query builder class when constructing a join inside another join. Grouped ON predicates and closure subqueries can then reconstruct a parent using the builder constructor, while preserving the immediate connection, grammar, processor, and existing factory methods. Accept mixed value operands in joinWhere, leftJoinWhere, rightJoinWhere, and straightJoinWhere, matching their existing delegation to value-based predicates. Keep ordinary column-comparison join signatures and all argument names and defaults unchanged. Add focused regressions for nested grouped conditions, closure subqueries, root subclass and dependency preservation, exact binding order, and boolean, integer, null, and expression operands across the four helpers. The full focused query-builder corpus, source and type-fixture analysis, and formatting checks pass.
Pass model attributes as an explicit one-row batch through the existing Eloquent insert dispatch. Drivers that retain array-valued attributes must not have those values mistaken for separate rows or their inner keys compiled as column names. Keep query-builder row and batch interpretation unchanged, including named and sparse batches. Preserve the incrementing insert-and-ID path, empty-attribute handling, binary preparation, unique IDs, timestamps, and model events without introducing driver detection or model-context state. Add real builder and grammar regression coverage for array-first, associative-array-only, empty-array, and scalar attributes, with exact SQL and binding assertions and unchanged model state. Retain the existing lifecycle and custom builder dispatch assertions, and verify empty non-incrementing models do not issue an insert. Verification: the complete model test file, formatting, source and type-fixture static analysis, full parallel framework tests, Testbench contract tests, package-mode tests, and diff checks pass. Peer review signed off both whole files.
Preserve the inner query's forced-write route on the executed count statement. Transfer it after SQL compilation so before-query callbacks that select the writer remain effective, without changing the original query or connection routing policy. Attach the compiled subquery and its complete binding list through fromRaw. These bindings belong to the outer FROM clause; retaining their original clause slots allowed aggregation to discard bindings for ordering preserved by a driver's pagination clone. Keep the existing unprefixed aggregate_table alias, timeout transfer, ordinary count branch, and public mergeBindings API unchanged. Add split in-memory SQLite routing regressions and a neutral ordering-preserving builder fixture. Cover callback routing and binding changes, exact binding order and ownership, table prefixes, and original-builder state. Verified the database suite, source and type-fixture analysis, and formatting.
Apply pending before-query callbacks on a local clone at the public pagination-count boundary, before choosing the count shape, pruning page clauses, or transferring statement options. Previously callback-added grouping could return the first group's count, callback projections could leave excess bindings, and callback timeouts could decorate only an inner query. Keep preparation inside the existing fetch-mode scope so callback-supplied fetch modes cannot change the count result shape. Preserve the original page builder and its callbacks, run callbacks once per count, and avoid an extra clone when none are pending. Transfer the derived count's writer route alongside its timeout now that callbacks have already completed, removing the obsolete compilation-order comment. Add focused SQLite and grammar regressions for grouping, projection and pagination pruning, statement timeout and routing, callback ownership, exception identity, and fetch-scope restoration. Existing database, one-of-many, and query integration coverage remains unchanged. Verified formatting, static analysis, and the affected suites.
Integrate the current framework baseline while preserving the independently useful database extension points and protocol-neutral execution behavior on this branch. Carry the existing read-extension configuration and URL-derived role handling into DatabasePool and PoolManager. Update the corresponding regression tests to the new borrow, options, and lifecycle APIs without weakening their endpoint-selection assertions. Preserve the shared query-exception factory and streaming boundary while incorporating normalized binding masking and unique-constraint context. Combine the improved schema type documentation with the generic column-factory return annotations, and retain both sets of query-builder type fixtures. Verify the merged query, model, schema, routing, and connection behavior with the complete formatter, source and type analysis, parallel test, Testbench, and dogfood checks. The complete merge result and adaptations received peer review.
Register the existing in-memory migration-state cleanup feature as soon as the migration concern initializes. Deferring registration until after application setup allowed a later service skip or setup exception to bypass registration entirely. DatabaseTruncation intentionally retains its migrated PDO between test methods. Without the class cleanup registration, an aborted setup could therefore leave a previous class schema available to an unrelated test class using the same connection name. Keep the existing in-memory detection, per-method retention, and file-backed behavior unchanged. No new lifecycle hooks, state flags, or unconditional database resets are introduced. Add regression coverage for both skipped and failed setup, proving that per-method teardown retains the database but class teardown releases the migration state. Both cases fail before the fix and pass afterward. Reproduce the original interaction on the unmodified baseline and verify the correction through the full framework and Testbench checks and peer review.
Pass the model attributes as an explicit row list to insertOrIgnoreReturning. The query builder otherwise treats an array-valued first attribute as a batch, causing mixed attributes to fail and all-array attributes to compile as different rows. Keep conflict targets, returning columns, binary preparation, model events, and key assignment unchanged. The correction applies to both incrementing and non-incrementing models without changing the public query-builder insert API. Update the existing call expectations and add regression coverage through the real model and query builders for both key modes, all-array attributes including an empty value, and ordinary scalar attributes. Verify SQL, bindings, conflict targets, returned keys, and model lifecycle flags. Model/query tests, SQLite integration tests, formatting, and source and type analysis pass.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
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
Hypervel already separates its driver-neutral database connection from PDO. This PR carries that separation through the parts of the framework that custom drivers need: query execution, schema operations, testing, and the database CLI.
The goal is to let a driver extend the framework's normal behavior without replacing whole classes or pretending to provide a PDO connection. The same work also fixes several query-builder and testing problems that affect the built-in drivers.
Database extension points
Database CLI
DatabaseCliManager::extend()lets a driver provide the executable, arguments, and environment for thedbcommand. Arguments remain separate process arguments, not a shell command string. Built-in drivers continue through the command's existing helpers, including subclass overrides.The command now applies a selected read/write record's URL before launching the client and consistently chooses the first configured host. It also preserves valid zero-valued credentials and options instead of treating them as absent.
Configuration URL parsing preserves literal strings and decodes encoded components without treating a plus sign as a space.
Query and model fixes
insert()API is unchanged.Testing lifecycle fixes
Documentation
The database guide covers the new extension contracts, streaming ownership, connection roles, custom CLI clients, and builder types. The database-testing guide explains transaction requirements and driver-defined truncation.
The Laravel porting guide calls out the builder factories that now require
staticreturns. It also removes the conditional-provider feature description and links to the Redis tag documentation instead of repeating it. Conditional providers remain documented in the provider guide. Contributor guidance now requires Laravel-style configuration section comments.Validation
Formatting, source and type-fixture analysis, the full parallel test suite, Testbench package-mode tests, and the dogfood package checks pass.
Regression tests cover streaming completion and failure, read/write routing, custom-driver configuration, CLI launching, query bindings and pagination, model inserts, schema extensions, and cleanup after skipped or failed setup. Tests that need unconfigured external services retain their normal opt-in behavior.
Summary by CodeRabbit