Title: Fix resolve() argument-count crash and add PHP 8 + Doctrine DBAL 4.x compatibility - #34
Open
marinhekman wants to merge 14 commits into
Open
Title: Fix resolve() argument-count crash and add PHP 8 + Doctrine DBAL 4.x compatibility#34marinhekman wants to merge 14 commits into
marinhekman wants to merge 14 commits into
Conversation
…lled react/promise (^3.3) requires exactly one argument
… across versions
1. src\Mock\MockedDBALConnection.php
◦ Simplified to a minimal final class MockedDBALConnection extends Connection {}.
◦ This avoids overriding DBAL methods whose signatures differ between DBAL 3 and 4 (quote, beginTransaction, commit, rollBack, lastInsertId, etc.).
2. src\Mock\MockedDriver.php
◦ Reworked with a runtime branch based on the installed DBAL Driver signature (connect parameter count).
◦ DBAL 4 branch implements modern signatures.
◦ DBAL 3 branch keeps legacy signatures.
◦ getDatabasePlatform(...) now returns the provided platform instead of throwing, so QueryBuilder can generate SQL.
3. src\SingleConnection.php
◦ Updated new MockedDriver() to new MockedDriver($this->platform)
This keeps behavior stable (mock still throws if real connection methods are used) but removes cross-version signature incompatibilities.
…teria() was removed in DBAL 4 — the class now just extends plain \InvalidArgumentException with no static factories.
Fix: replaced all 3 call sites in src\SingleConnection.php (lines ~311, ~338, ~407 — delete(), update(), createTable()) with new InvalidArgumentException('Empty criteria was used to build a query'), which works identically on both DBAL 3 and 4. Tests expecting InvalidArgumentException to be thrown should now pass again on PHP 8.
…?string return type, but Drift\DBAL\Driver\Exception::getSQLState() had none. PHP requires return types to match the interface once declared, so this fatal errors regardless of DBAL version — I just hadn't hit it yet. Added ?string return type to src\Driver\Exception.php; ?string is valid PHP 7.4+ syntax, so this is safe for both PHP 7 and 8.
…ated since DBAL 3.3, replaced by or()/and()). tests\ConnectionTest.php (line 231) still called orX(). Fix: replaced it with ->or(...), which exists in both DBAL 3 (3.9+, satisfies your ^3|^4 constraint) and DBAL 4 — no version branching needed since only this method name changed, not its signature.
…a concrete/instantiable class in DBAL 3). src\Driver\PostgreSQL\PostgreSQLDriver.php did new Exception('Connection closed') in two places (query() and insert()), which fatals on DBAL 4.
Fix: replaced both with new Doctrine\DBAL\ConnectionException('Connection closed'), a concrete class implementing that Exception interface, present in both DBAL 3 and 4. This keeps tests\ConnectionTest.php's ->otherwise(function (DBALException $exception) ...) type hint satisfied (React Promise v3 checks the callback's typehint against the actual rejection reason), so the rejection is still caught correctly on both PHP/DBAL versions.
…urned a bool ($a1['id'] > $a2['id']), deprecated since PHP 8.1 (spaceship required). Changed to $a1['id'] <=> $a2['id'].
…Fields dynamic property deprecations (PHP 8.2) come from vendor\react\mysql\src\Io\Parser.php, third-party code. so: • Upgrade react/mysql to a version that declares these properties (check for a newer release compatible with your constraints), Bumped composer.json: "react/mysql": "^0.5" → "react/mysql": "^0.6"
… to Doctrine\DBAL\Platforms\SQLitePlatform — a hard rename, no BC alias. Fix: in tests\SQLiteConnectionTest.php, replaced the static use SqlitePlatform import with a runtime class_exists() check that picks SQLitePlatform if available (DBAL 4), falling back to SqlitePlatform (DBAL 3). Verified testQueryBuilder now passes.
marinhekman
changed the title
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.
This PR contains two related but independent improvements found while running this library against react/promise ^3 and evaluating doctrine/dbal ^4 support:
Bugfix: ConnectionPool::releaseConnection() calls the global resolve() helper with zero arguments in both branches. Since react/promise ^3.x requires exactly one argument for resolve(), this throws ArgumentCountError: Too few arguments to function React\Promise\resolve(), 0 passed ... and exactly 1 expected every time a transaction is committed or rolled back.
PHPUnit / DBAL 4 & PHP 8.x compatibility fixes (cont'd):
• Fixed usort() comparator in ConnectionTest.php returning bool instead of int (PHP 8.1 deprecation)
• Upgraded react/mysql ^0.5 → ^0.6 to fix dynamic property deprecations on PHP 8.2+
• Fixed SQLiteConnectionTest to support both SqlitePlatform (DBAL 3) and SQLitePlatform (DBAL 4, renamed)