From ed02b5a850e745eb014b8fe49cb9f36c68d11e4a Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 12 Oct 2026 10:48:29 +1100 Subject: [PATCH 01/10] Bring TableGateway and RowGateway to Mago compliance The gateway dependencies stay nullable so subclasses can still fill them in before initialize(). Each read now goes through a private guard that throws a catalogued exception when the value is unset: RuntimeException::forUninitializedProperty() for featureSet, sql and resultSetPrototype, and the existing forMissingAdapter(), forMissingTable(), forMissingSqlObject() and forMissingPrimaryKeyColumn() elsewhere. A statement that returns no result now throws forMissingStatementResult() instead of a TypeError, and RowGateway::save() throws forMissingSavedRow() when the saved row cannot be read back. Saving a row whose composite primary key changed in only one column reselected with null for the unchanged columns. It now reselects by the new key. getRawState('table') on Delete, Insert and Update, and getRawState(Select::TABLE), get conditional return types so the gateways read a typed table. ResultInterface declares its Iterator template, which also clears four Adapter and ResultSet findings. RowGatewayFeature checks that each primary key column from metadata is a string. Every gateway class and interface is tagged @api. AbstractTableGateway, AbstractRowGateway, both FeatureSets and EventFeature join the too-many-methods exclusions. Twelve gateway findings stay baselined: - class-level cyclomatic-complexity and kan-defect on the two abstract gateways and TableGateway\Feature\FeatureSet, and cyclomatic-complexity on RowGatewayFeature; - no-boolean-flag-parameter on populate(), a public signature; - five less-specific-nested-argument-type findings where row data reaches the Sql builders: populate() accepts any array per RowPrototypeInterface, so the values cannot be narrowed to ColumnValue without validating them. --- analyzer-baseline.toml | 654 +----------------- lint-baseline.toml | 108 +-- mago.toml | 9 + src/Adapter/Driver/ResultInterface.php | 3 + src/RowGateway/AbstractRowGateway.php | 261 ++++--- .../Exception/InvalidArgumentException.php | 3 + src/RowGateway/Exception/RuntimeException.php | 17 + src/RowGateway/Feature/FeatureSet.php | 3 - src/RowGateway/RowGateway.php | 14 +- src/RowGateway/RowGatewayInterface.php | 3 + src/Sql/Delete.php | 3 + src/Sql/Insert.php | 12 +- src/Sql/Select.php | 2 +- src/Sql/Update.php | 3 + src/TableGateway/AbstractTableGateway.php | 363 ++++++---- .../Exception/InvalidArgumentException.php | 3 + .../Exception/RuntimeException.php | 19 + .../EventFeature/TableGatewayEvent.php | 2 + src/TableGateway/Feature/MetadataFeature.php | 2 + .../Feature/RowGatewayFeature.php | 24 +- src/TableGateway/Feature/SequenceFeature.php | 8 +- src/TableGateway/TableGateway.php | 9 +- src/TableGateway/TableGatewayInterface.php | 15 +- .../RowGateway/AbstractRowGatewayTest.php | 82 +++ .../Exception/RuntimeExceptionTest.php | 12 + test/unit/RowGateway/RowGatewayTest.php | 2 + .../TableGateway/AbstractTableGatewayTest.php | 337 ++++++++- .../Exception/RuntimeExceptionTest.php | 12 + .../Feature/RowGatewayFeatureTest.php | 57 ++ .../TestAsset/HookRecordingTableGateway.php | 61 ++ 30 files changed, 1089 insertions(+), 1014 deletions(-) create mode 100644 test/unit/TableGateway/TestAsset/HookRecordingTableGateway.php diff --git a/analyzer-baseline.toml b/analyzer-baseline.toml index 39ab6cfa8..a1e8fd1af 100644 --- a/analyzer-baseline.toml +++ b/analyzer-baseline.toml @@ -492,18 +492,6 @@ code = "incompatible-return-type" message = 'Return type `mixed` of `PhpDb\Adapter\Driver\Pdo\Result::next()` is incompatible with parent return type `void` of `Iterator::next()`' count = 1 -[[issues]] -file = "src/Adapter/Driver/Pdo/Result.php" -code = "less-specific-nested-argument-type" -message = 'Argument type mismatch for argument #1 of `PhpDb\ResultSet\AbstractResultSet::initialize`: expected `iterable`, but provided type `$this(PhpDb\Adapter\Driver\Pdo\Result)` is less specific.' -count = 1 - -[[issues]] -file = "src/Adapter/Driver/Pdo/Result.php" -code = "less-specific-nested-argument-type" -message = 'Argument type mismatch for argument #1 of `PhpDb\ResultSet\ResultSetInterface::initialize`: expected `iterable`, but provided type `$this(PhpDb\Adapter\Driver\Pdo\Result)` is less specific.' -count = 1 - [[issues]] file = "src/Adapter/Driver/Pdo/Result.php" code = "missing-constant-type" @@ -768,12 +756,6 @@ code = "missing-api-or-internal" message = 'Interface `PhpDb\Adapter\Driver\ResultInterface` is missing an `@api` or `@internal` annotation.' count = 1 -[[issues]] -file = "src/Adapter/Driver/ResultInterface.php" -code = "missing-template-parameter" -message = "Too few template arguments for `Iterator`: expected at least 2, but found 0." -count = 1 - [[issues]] file = "src/Adapter/Driver/StatementInterface.php" code = "imprecise-type" @@ -1428,12 +1410,6 @@ code = "mixed-assignment" message = "Assigning `mixed` type to a variable may lead to unexpected behavior." count = 2 -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "no-value" -message = 'Argument #1 passed to method `PhpDb\ResultSet\AbstractResultSet::initializeFromResult` has type `never`, meaning it cannot produce a value.' -count = 1 - [[issues]] file = "src/ResultSet/Exception/InvalidArgumentException.php" code = "class-must-be-final" @@ -1446,70 +1422,10 @@ code = "class-must-be-final" message = 'Class `PhpDb\ResultSet\Exception\RuntimeException` should be declared `final`.' count = 1 -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "imprecise-type" -message = "Type `array` in parameter `$array` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "imprecise-type" -message = "Type `array` in property `$data` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "imprecise-type" -message = "Type `array` in property `$primaryKeyColumn` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "imprecise-type" -message = "Type `array` in property `$primaryKeyData` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "incompatible-parameter-name" -message = 'Parameter #1 of `PhpDb\RowGateway\AbstractRowGateway::populate()` is named `$rowData` but parent `PhpDb\ResultSet\RowPrototypeInterface::populate()` names it `$data`' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "incompatible-parameter-type" -message = 'Parameter `$offset` of `PhpDb\RowGateway\AbstractRowGateway::offsetexists()` expects type `string` but parent `ArrayAccess::offsetexists()` expects type `mixed`' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "incompatible-parameter-type" -message = 'Parameter `$offset` of `PhpDb\RowGateway\AbstractRowGateway::offsetget()` expects type `string` but parent `ArrayAccess::offsetget()` expects type `mixed`' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "incompatible-parameter-type" -message = 'Parameter `$offset` of `PhpDb\RowGateway\AbstractRowGateway::offsetset()` expects type `string` but parent `ArrayAccess::offsetset()` expects type `mixed`' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "incompatible-parameter-type" -message = 'Parameter `$offset` of `PhpDb\RowGateway\AbstractRowGateway::offsetunset()` expects type `string` but parent `ArrayAccess::offsetunset()` expects type `mixed`' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "invalid-array-element-key" -message = "Invalid array key type." -count = 1 - [[issues]] file = "src/RowGateway/AbstractRowGateway.php" code = "less-specific-nested-argument-type" -message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Delete::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|PhpDb\Sql\Where|array|null|scalar>|string`, but provided type `array` is less specific.' +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Delete::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|PhpDb\Sql\Where|array|null|scalar>|string`, but provided type `array` is less specific.' count = 1 [[issues]] @@ -1521,7 +1437,7 @@ count = 1 [[issues]] file = "src/RowGateway/AbstractRowGateway.php" code = "less-specific-nested-argument-type" -message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Select::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|array|null|scalar>|string`, but provided type `array` is less specific.' +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Select::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|array|null|scalar>|string`, but provided type `array` is less specific.' count = 1 [[issues]] @@ -1533,169 +1449,7 @@ count = 1 [[issues]] file = "src/RowGateway/AbstractRowGateway.php" code = "less-specific-nested-argument-type" -message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Update::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|PhpDb\Sql\Where|array|null|scalar>|string`, but provided type `array` is less specific.' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "less-specific-return-statement" -message = 'Returned type `array` is less specific than the declared return type `array` for function `PhpDb\RowGateway\AbstractRowGateway::exchangeArray`.' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "missing-api-or-internal" -message = 'Abstract class `PhpDb\RowGateway\AbstractRowGateway` is missing an `@api` or `@internal` annotation.' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "missing-override-attribute" -message = 'Missing `#[Override]` attribute on overriding method `PhpDb\RowGateway\AbstractRowGateway::populate`.' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "missing-override-attribute" -message = 'Missing `#[Override]` attribute on overriding method `PhpDb\RowGateway\AbstractRowGateway::toArray`.' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "missing-template-parameter" -message = "Too few template arguments for `ArrayAccess`: expected at least 2, but found 0." -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\RowGateway\AbstractRowGateway::populate`: expected `array`, but found `mixed`.' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\RowGateway\Exception\RuntimeException::forMissingPrimaryKeyData`: expected `string`, but found `mixed`.' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "mixed-assignment" -message = "Assigning `mixed` type to a variable may lead to unexpected behavior." -count = 6 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "possible-method-access-on-null" -message = "Attempting to call a method on `null`." -count = 15 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "possibly-null-argument" -message = "Argument #1 of function `count` is possibly `null`, but parameter type `Countable|array` does not accept it." -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "possibly-null-array-access" -message = "Cannot perform array access on possibly `null` value." -count = 4 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "possibly-null-iterator" -message = "Expression being iterated (type `array|null`) might be `null` at runtime." -count = 5 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "possibly-undefined-int-array-index" -message = "Possibly undefined array key `int(0)` accessed on `non-empty-array`." -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "possibly-undefined-int-array-index" -message = "Possibly undefined array key `mixed` accessed on `array`." -count = 5 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "possibly-undefined-string-array-index" -message = "Possibly undefined array key `string` accessed on `array`." -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "redundant-comparison" -message = "Avoid direct comparison with boolean literals." -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "unhandled-thrown-type" -message = 'Potentially unhandled exception `PhpDb\RowGateway\Exception\RuntimeException` in `PhpDb\RowGateway\AbstractRowGateway::initialize`.' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "unhandled-thrown-type" -message = 'Potentially unhandled exception `PhpDb\RowGateway\Exception\RuntimeException` in `PhpDb\RowGateway\AbstractRowGateway::populate`.' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "unhandled-thrown-type" -message = 'Potentially unhandled exception `PhpDb\RowGateway\Exception\RuntimeException` in `PhpDb\RowGateway\AbstractRowGateway::save`.' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "unhandled-thrown-type" -message = 'Potentially unhandled exception `PhpDb\Sql\Exception\RuntimeException` in `PhpDb\RowGateway\AbstractRowGateway::delete`.' -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "unhandled-thrown-type" -message = 'Potentially unhandled exception `PhpDb\Sql\Exception\RuntimeException` in `PhpDb\RowGateway\AbstractRowGateway::save`.' -count = 1 - -[[issues]] -file = "src/RowGateway/Exception/InvalidArgumentException.php" -code = "class-must-be-final" -message = 'Class `PhpDb\RowGateway\Exception\InvalidArgumentException` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/RowGateway/Exception/RuntimeException.php" -code = "class-must-be-final" -message = 'Class `PhpDb\RowGateway\Exception\RuntimeException` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/RowGateway/Feature/FeatureSet.php" -code = "unused-property" -message = "Property `$magicSpecifications` is never used." -count = 1 - -[[issues]] -file = "src/RowGateway/RowGateway.php" -code = "class-must-be-final" -message = 'Class `PhpDb\RowGateway\RowGateway` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/RowGateway/RowGateway.php" -code = "imprecise-type" -message = "Type `array` in parameter `$primaryKeyColumn` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/RowGateway/RowGatewayInterface.php" -code = "missing-api-or-internal" -message = 'Interface `PhpDb\RowGateway\RowGatewayInterface` is missing an `@api` or `@internal` annotation.' +message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Update::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|PhpDb\Sql\Where|array|null|scalar>|string`, but provided type `array` is less specific.' count = 1 [[issues]] @@ -1865,405 +1619,3 @@ file = "src/Sql/Update.php" code = "missing-return-type" message = "Function `__clone` is missing a return type hint." count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "clone-inside-loop" -message = "Cloning variable `$tableObject` onto itself inside a loop might not have the intended effect." -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "impossible-condition" -message = "This condition (type `false`) will always evaluate to false." -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "imprecise-type" -message = "Type `array` in parameter `$arguments` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "imprecise-type" -message = "Type `array` in parameter `$joins` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "imprecise-type" -message = "Type `array` in parameter `$where` is imprecise, equivalent to `array`." -count = 3 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "imprecise-type" -message = "Type `array` in property `$columns` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "imprecise-type" -message = "Type `array` in property `$table` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "imprecise-type" -message = "Type `array` in return type of `getColumns` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "imprecise-type" -message = "Type `array` in return type of `getTable` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "invalid-array-access" -message = 'Cannot access array index on object `PhpDb\Sql\TableIdentifier` that does not implement `ArrayAccess`.' -count = 3 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "invalid-property-assignment-value" -message = "Invalid type for property `$lastInsertValue`: expected `int`, but got `false|int|null|string`." -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "invalid-return-statement" -message = 'Invalid return type for function `PhpDb\TableGateway\AbstractTableGateway::executeSelect`: expected `PhpDb\ResultSet\ResultSetInterface`, but found `PhpDb\ResultSet\ResultSetInterface|null`.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "invalid-return-statement" -message = 'Invalid return type for function `PhpDb\TableGateway\AbstractTableGateway::getFeatureSet`: expected `PhpDb\TableGateway\Feature\FeatureSet`, but found `PhpDb\TableGateway\Feature\FeatureSet|null`.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "invalid-return-statement" -message = 'Invalid return type for function `PhpDb\TableGateway\AbstractTableGateway::getResultSetPrototype`: expected `PhpDb\ResultSet\ResultSetInterface`, but found `PhpDb\ResultSet\ResultSetInterface|null`.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "invalid-return-statement" -message = 'Invalid return type for function `PhpDb\TableGateway\AbstractTableGateway::getSql`: expected `PhpDb\Sql\Sql`, but found `PhpDb\Sql\Sql|null`.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "invalid-return-statement" -message = 'Invalid return type for function `PhpDb\TableGateway\AbstractTableGateway::getTable`: expected `PhpDb\Sql\TableIdentifier|array|string`, but found `PhpDb\Sql\TableIdentifier|array|null|string`.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "less-specific-argument" -message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Insert::into`: expected `PhpDb\Sql\TableIdentifier|array|string`, but provided type `PhpDb\Sql\ExpressionInterface|PhpDb\Sql\Select|PhpDb\Sql\TableIdentifier|array|null|scalar` is less specific.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "less-specific-argument" -message = 'Argument type mismatch for argument #2 of `PhpDb\Sql\Sql::__construct`: expected `PhpDb\Sql\TableIdentifier|array|null|string`, but provided type `PhpDb\Sql\TableIdentifier|array|string` is less specific.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "less-specific-nested-argument-type" -message = 'Argument type mismatch for argument #1 of `PhpDb\ResultSet\ResultSetInterface::initialize`: expected `iterable`, but provided type `PhpDb\Adapter\Driver\ResultInterface|null` is less specific.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "less-specific-nested-argument-type" -message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Delete::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|PhpDb\Sql\Where|array|null|scalar>|string`, but provided type `PhpDb\Sql\Where|array|string` is less specific.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "less-specific-nested-argument-type" -message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Insert::values`: expected `PhpDb\Sql\Select|array`, but provided type `array` is less specific.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "less-specific-nested-argument-type" -message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Select::columns`: expected `array`, but provided type `non-empty-array` is less specific.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "less-specific-nested-argument-type" -message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Select::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|array|null|scalar>|string`, but provided type `PhpDb\Sql\Where|array|string` is less specific.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "less-specific-nested-argument-type" -message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Update::set`: expected `array`, but provided type `array` is less specific.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "less-specific-nested-argument-type" -message = 'Argument type mismatch for argument #1 of `PhpDb\Sql\Update::where`: expected `(closure(...mixed): mixed)|PhpDb\Sql\Predicate\PredicateInterface|PhpDb\Sql\Where|array|null|scalar>|string`, but provided type `(closure(...mixed=): mixed)|PhpDb\Sql\Where|array|string` is less specific.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "missing-api-or-internal" -message = 'Abstract class `PhpDb\TableGateway\AbstractTableGateway` is missing an `@api` or `@internal` annotation.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\Sql\Delete::from`: expected `PhpDb\Sql\TableIdentifier|array|string`, but found `mixed`.' -count = 2 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\Sql\Insert::into`: expected `PhpDb\Sql\TableIdentifier|array|string`, but found `mixed`.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\Sql\Update::join`: expected `PhpDb\Sql\TableIdentifier|non-empty-array|string`, but found `mixed`.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\Sql\Update::table`: expected `PhpDb\Sql\TableIdentifier|array|string`, but found `mixed`.' -count = 2 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #2 of `PhpDb\Sql\Update::join`: expected `string`, but found `mixed`.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "mixed-argument" -message = 'Invalid argument type for argument #3 of `PhpDb\Sql\Update::join`: expected `string`, but found `nonnull`.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "mixed-array-access" -message = "Unsafe array access on type `mixed`." -count = 6 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "mixed-assignment" -message = "Assigning `mixed` type to a variable may lead to unexpected behavior." -count = 9 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "mixed-assignment" -message = "Assigning `nonnull` type to a variable may lead to unexpected behavior." -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "mixed-clone" -message = "Cannot statically verify `clone` on a `mixed` type." -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "nullable-return-statement" -message = 'Function `PhpDb\TableGateway\AbstractTableGateway::executeSelect` is declared to return `PhpDb\ResultSet\ResultSetInterface` but possibly returns a nullable value (inferred as `PhpDb\ResultSet\ResultSetInterface|null`).' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "nullable-return-statement" -message = 'Function `PhpDb\TableGateway\AbstractTableGateway::getFeatureSet` is declared to return `PhpDb\TableGateway\Feature\FeatureSet` but possibly returns a nullable value (inferred as `PhpDb\TableGateway\Feature\FeatureSet|null`).' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "nullable-return-statement" -message = 'Function `PhpDb\TableGateway\AbstractTableGateway::getResultSetPrototype` is declared to return `PhpDb\ResultSet\ResultSetInterface` but possibly returns a nullable value (inferred as `PhpDb\ResultSet\ResultSetInterface|null`).' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "nullable-return-statement" -message = 'Function `PhpDb\TableGateway\AbstractTableGateway::getSql` is declared to return `PhpDb\Sql\Sql` but possibly returns a nullable value (inferred as `PhpDb\Sql\Sql|null`).' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "nullable-return-statement" -message = 'Function `PhpDb\TableGateway\AbstractTableGateway::getTable` is declared to return `PhpDb\Sql\TableIdentifier|array|string` but possibly returns a nullable value (inferred as `PhpDb\Sql\TableIdentifier|array|null|string`).' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "overly-wide-return-type" -message = "Declared return type `false|int|null|string` for `getlastinsertvalue` has unused branches: `false`, `string`, `null`." -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "possible-method-access-on-null" -message = "Attempting to call a method on `null`." -count = 28 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "possibly-invalid-clone" -message = 'Expression of type `PhpDb\ResultSet\ResultSetInterface|null` might not be a cloneable object.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "possibly-invalid-clone" -message = 'Expression of type `PhpDb\Sql\Sql|null` might not be a cloneable object.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "possibly-null-argument" -message = 'Argument #1 of method `PhpDb\ResultSet\ResultSetInterface::initialize` is possibly `null`, but parameter type `iterable` does not accept it.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "possibly-null-argument" -message = 'Argument #1 of method `PhpDb\Sql\Insert::into` is possibly `null`, but parameter type `PhpDb\Sql\TableIdentifier|array|string` does not accept it.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "possibly-undefined-int-array-index" -message = 'Possibly undefined array index accessed on `list`.' -count = 3 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "possibly-undefined-string-array-index" -message = '''Possibly undefined array key `string('table')` accessed on `array|string>`.''' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "possibly-undefined-string-array-index" -message = '''Possibly undefined array key `string('table')` accessed on `array`.''' -count = 2 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "redundant-comparison" -message = "Redundant `===` comparison: left-hand side is never identical to right-hand side." -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "unhandled-thrown-type" -message = 'Potentially unhandled exception `PhpDb\Sql\Exception\RuntimeException` in `PhpDb\TableGateway\AbstractTableGateway::executeDelete`.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "unhandled-thrown-type" -message = 'Potentially unhandled exception `PhpDb\Sql\Exception\RuntimeException` in `PhpDb\TableGateway\AbstractTableGateway::executeInsert`.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "unhandled-thrown-type" -message = 'Potentially unhandled exception `PhpDb\Sql\Exception\RuntimeException` in `PhpDb\TableGateway\AbstractTableGateway::executeSelect`.' -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "unhandled-thrown-type" -message = 'Potentially unhandled exception `PhpDb\Sql\Exception\RuntimeException` in `PhpDb\TableGateway\AbstractTableGateway::executeUpdate`.' -count = 1 - -[[issues]] -file = "src/TableGateway/Exception/InvalidArgumentException.php" -code = "class-must-be-final" -message = 'Class `PhpDb\TableGateway\Exception\InvalidArgumentException` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/TableGateway/Exception/RuntimeException.php" -code = "class-must-be-final" -message = 'Class `PhpDb\TableGateway\Exception\RuntimeException` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/TableGateway/Feature/EventFeature/TableGatewayEvent.php" -code = "imprecise-type" -message = "Type `array` in return type of `getParams` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/TableGateway/Feature/MetadataFeature.php" -code = "mixed-assignment" -message = "Assigning `mixed` type to a variable may lead to unexpected behavior." -count = 1 - -[[issues]] -file = "src/TableGateway/Feature/MetadataFeature.php" -code = "mixed-assignment" -message = "Assigning `nonnull` type to a variable may lead to unexpected behavior." -count = 1 - -[[issues]] -file = "src/TableGateway/Feature/RowGatewayFeature.php" -code = "mixed-assignment" -message = "Assigning `mixed` type to a variable may lead to unexpected behavior." -count = 1 - -[[issues]] -file = "src/TableGateway/TableGateway.php" -code = "class-must-be-final" -message = 'Class `PhpDb\TableGateway\TableGateway` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/TableGateway/TableGateway.php" -code = "imprecise-type" -message = "Type `array` in parameter `$table` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/TableGateway/TableGateway.php" -code = "less-specific-argument" -message = 'Argument type mismatch for argument #2 of `PhpDb\Sql\Sql::__construct`: expected `PhpDb\Sql\TableIdentifier|array|null|string`, but provided type `PhpDb\Sql\TableIdentifier|array|string` is less specific.' -count = 1 - -[[issues]] -file = "src/TableGateway/TableGatewayInterface.php" -code = "imprecise-type" -message = "Type `array` in parameter `$where` is imprecise, equivalent to `array`." -count = 3 - -[[issues]] -file = "src/TableGateway/TableGatewayInterface.php" -code = "imprecise-type" -message = "Type `array` in return type of `getTable` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/TableGateway/TableGatewayInterface.php" -code = "missing-api-or-internal" -message = 'Interface `PhpDb\TableGateway\TableGatewayInterface` is missing an `@api` or `@internal` annotation.' -count = 1 diff --git a/lint-baseline.toml b/lint-baseline.toml index 2b4cac963..e301389ec 100644 --- a/lint-baseline.toml +++ b/lint-baseline.toml @@ -258,12 +258,6 @@ code = "cyclomatic-complexity" message = "Class has high complexity." count = 1 -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "halstead" -message = "Method has a high halstead difficulty" -count = 1 - [[issues]] file = "src/RowGateway/AbstractRowGateway.php" code = "kan-defect" @@ -276,60 +270,6 @@ code = "no-boolean-flag-parameter" message = "Avoid boolean flag parameters." count = 1 -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "no-dead-store" -message = "Variable `$rowsAffected` is overwritten before its value is read." -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "no-else-clause" -message = "Avoid `else` clauses." -count = 4 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "no-isset" -message = "Use of the `isset` construct." -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "prefer-early-continue" -message = "Consider using early continue pattern to reduce nesting." -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "too-many-methods" -message = "Class has too many methods." -count = 1 - -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "yoda-conditions" -message = "Use Yoda condition style for safer comparisons" -count = 6 - -[[issues]] -file = "src/RowGateway/Feature/FeatureSet.php" -code = "too-many-methods" -message = "Class has too many methods." -count = 1 - -[[issues]] -file = "src/RowGateway/RowGateway.php" -code = "no-else-clause" -message = "Avoid `else` clauses." -count = 1 - -[[issues]] -file = "src/RowGateway/RowGateway.php" -code = "no-negated-ternary" -message = "Negated ternary condition." -count = 1 - [[issues]] file = "src/Sql/AbstractSql.php" code = "cyclomatic-complexity" @@ -396,48 +336,12 @@ code = "cyclomatic-complexity" message = "Class has high complexity." count = 1 -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "halstead" -message = "Method has a high halstead difficulty" -count = 1 - [[issues]] file = "src/TableGateway/AbstractTableGateway.php" code = "kan-defect" message = "Class has a high kan defect score." count = 1 -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "no-else-clause" -message = "Avoid `else` clauses." -count = 1 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "no-else-clause" -message = "Avoid `elseif` clauses." -count = 2 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "no-isset" -message = "Use of the `isset` construct." -count = 3 - -[[issues]] -file = "src/TableGateway/AbstractTableGateway.php" -code = "too-many-methods" -message = "Class has too many methods." -count = 1 - -[[issues]] -file = "src/TableGateway/Feature/EventFeature.php" -code = "too-many-methods" -message = "Class has too many methods." -count = 1 - [[issues]] file = "src/TableGateway/Feature/FeatureSet.php" code = "kan-defect" @@ -445,15 +349,9 @@ message = "Class has a high kan defect score." count = 1 [[issues]] -file = "src/TableGateway/Feature/FeatureSet.php" -code = "too-many-methods" -message = "Class has too many methods." -count = 1 - -[[issues]] -file = "src/TableGateway/TableGateway.php" -code = "no-shorthand-ternary" -message = "Use of the shorthand ternary operator." +file = "src/TableGateway/Feature/RowGatewayFeature.php" +code = "cyclomatic-complexity" +message = "Class has high complexity." count = 1 [[issues]] diff --git a/mago.toml b/mago.toml index 31246c5f0..eeb4fd26a 100644 --- a/mago.toml +++ b/mago.toml @@ -18,6 +18,10 @@ baseline = "lint-baseline.toml" # not apply. Predicate, AlterTable, CreateTable, Select, Insert and Update are fluent builders: # one public method per clause they can emit, so their method count is the size of their # public API. AbstractSql is their base and carries one protected hook per clause it renders. +# AbstractTableGateway and AbstractRowGateway expose one public method per gateway operation +# plus the ArrayAccess and magic-property contracts, the two FeatureSets one method per hook +# and magic dispatch they forward to features, and EventFeature one listener per gateway event, +# so their method count is the size of their public API as well. # "test/" repeats the phpdb-qa-tools default, which this key replaces. too-many-methods = { exclude = [ "test/", @@ -32,6 +36,11 @@ too-many-methods = { exclude = [ "src/Sql/Select.php", "src/Sql/Insert.php", "src/Sql/Update.php", + "src/TableGateway/AbstractTableGateway.php", + "src/TableGateway/Feature/EventFeature.php", + "src/TableGateway/Feature/FeatureSet.php", + "src/RowGateway/AbstractRowGateway.php", + "src/RowGateway/Feature/FeatureSet.php", ] } # Select holds one property per clause of a SELECT statement. diff --git a/src/Adapter/Driver/ResultInterface.php b/src/Adapter/Driver/ResultInterface.php index 75be8c068..5b6f26969 100644 --- a/src/Adapter/Driver/ResultInterface.php +++ b/src/Adapter/Driver/ResultInterface.php @@ -9,6 +9,9 @@ use PhpDb\Adapter\Exception; use PhpDb\ResultSet\ResultSetInterface; +/** + * @extends Iterator + */ interface ResultInterface extends Countable, Iterator { /** diff --git a/src/RowGateway/AbstractRowGateway.php b/src/RowGateway/AbstractRowGateway.php index b583378cc..c3181945e 100644 --- a/src/RowGateway/AbstractRowGateway.php +++ b/src/RowGateway/AbstractRowGateway.php @@ -7,23 +7,34 @@ use ArrayAccess; use Countable; use Override; +use PhpDb\Adapter\Driver\ResultInterface; +use PhpDb\Adapter\Driver\StatementInterface; +use PhpDb\Sql\Exception\RuntimeException as SqlRuntimeException; use PhpDb\Sql\Sql; use PhpDb\Sql\TableIdentifier; use ReturnTypeWillChange; use function array_key_exists; use function count; +use function is_array; +/** + * @api + * @implements ArrayAccess + */ abstract class AbstractRowGateway implements ArrayAccess, Countable, RowGatewayInterface { protected bool $isInitialized = false; protected TableIdentifier|string|null $table = null; + /** @var list|null */ protected ?array $primaryKeyColumn = null; + /** @var array|null */ protected ?array $primaryKeyData = null; + /** @var array */ protected array $data = []; protected ?Sql $sql = null; @@ -37,24 +48,27 @@ public function count(): int return count($this->data); } + /** + * @throws Exception\RuntimeException + * @throws SqlRuntimeException + */ #[Override] public function delete(): int { $this->initialize(); + $sql = $this->requireSql(); $where = []; - foreach ($this->primaryKeyColumn as $pkColumn) { + foreach ($this->requirePrimaryKeyColumn() as $pkColumn) { $where[$pkColumn] = $this->primaryKeyData[$pkColumn] ?? null; } // @todo determine if we need to do a select to ensure 1 row will be affected - $rowsAffected = 0; - $statement = $this->sql->prepareStatementForSqlObject($this->sql->delete()->where($where)); - $result = $statement->execute(); + $statement = $sql->prepareStatementForSqlObject($sql->delete()->where($where)); - $rowsAffected = $result->getAffectedRows(); - if ($rowsAffected === 1) { + $rowsAffected = $this->executeStatement($statement)->getAffectedRows(); + if (1 === $rowsAffected) { $this->primaryKeyData = null; } @@ -65,7 +79,11 @@ public function delete(): int * docs: Behaviour has changed - this no longer returns RowGatewayInterface but * instead an array of the old data as per original PHP spec. * - * @return array + * @param array $array + * + * @return array + * + * @throws Exception\RuntimeException */ public function exchangeArray(array $array): array { @@ -78,6 +96,8 @@ public function exchangeArray(array $array): array /** * initialize() + * + * @throws Exception\RuntimeException */ public function initialize(): void { @@ -85,26 +105,20 @@ public function initialize(): void return; } - if (! $this->featureSet instanceof Feature\FeatureSet) { - $this->featureSet = new Feature\FeatureSet(); - } + $this->featureSet ??= new Feature\FeatureSet(); + $featureSet = $this->featureSet; - $this->featureSet->setRowGateway($this); - $this->featureSet->apply('preInitialize', []); + $featureSet->setRowGateway($this); + $featureSet->apply('preInitialize', []); - if ($this->table === null) { + if (null === $this->table) { throw Exception\RuntimeException::forMissingTable(); } - if ($this->primaryKeyColumn === null) { - throw Exception\RuntimeException::forMissingPrimaryKeyColumn(); - } - - if ($this->sql === null) { - throw Exception\RuntimeException::forMissingSqlObject(); - } + $this->requirePrimaryKeyColumn(); + $this->requireSql(); - $this->featureSet->apply('postInitialize', []); + $featureSet->apply('postInitialize', []); $this->isInitialized = true; } @@ -130,13 +144,17 @@ public function offsetExists($offset): bool #[ReturnTypeWillChange] public function offsetGet($offset): mixed { - return $this->data[$offset]; + return $this->data[$offset] ?? null; } /** * Offset set * + * Returns the row for chaining, where ArrayAccess declares void. + * * @param string $offset + * + * @mago-expect analysis:incompatible-return-type */ #[Override] #[ReturnTypeWillChange] @@ -150,7 +168,11 @@ public function offsetSet($offset, mixed $value): static /** * Offset unset * + * Returns the row for chaining, where ArrayAccess declares void. + * * @param string $offset + * + * @mago-expect analysis:incompatible-return-type */ #[Override] #[ReturnTypeWillChange] @@ -163,16 +185,20 @@ public function offsetUnset($offset): static /** * Populate Data + * + * @param array $data + * + * @throws Exception\RuntimeException */ - public function populate(array $rowData, bool $rowExistsInDatabase = false): static + #[Override] + public function populate(array $data, bool $rowExistsInDatabase = false): static { $this->initialize(); - $this->data = $rowData; - if ($rowExistsInDatabase === true) { + $this->data = $data; + $this->primaryKeyData = null; + if ($rowExistsInDatabase) { $this->processPrimaryKeyData(); - } else { - $this->primaryKeyData = null; } return $this; @@ -180,72 +206,41 @@ public function populate(array $rowData, bool $rowExistsInDatabase = false): sta public function rowExistsInDatabase(): bool { - return $this->primaryKeyData !== null; + return null !== $this->primaryKeyData; } + /** + * @throws Exception\RuntimeException + * @throws SqlRuntimeException + * + * @mago-expect analysis:mixed-assignment + */ #[Override] public function save(): int { $this->initialize(); - $rowsAffected = 0; - - if ($this->rowExistsInDatabase()) { - $data = $this->data; - $where = []; - $isPkModified = false; + $sql = $this->requireSql(); - foreach ($this->primaryKeyColumn as $pkColumn) { - $where[$pkColumn] = $this->primaryKeyData[$pkColumn]; - if ($data[$pkColumn] === $this->primaryKeyData[$pkColumn]) { - unset($data[$pkColumn]); - } else { - $isPkModified = true; - } - } - - $statement = $this->sql->prepareStatementForSqlObject($this->sql->update()->set($data)->where($where)); - $result = $statement->execute(); - $rowsAffected = $result->getAffectedRows(); - unset($statement, $result); - - if ($isPkModified) { - foreach ($this->primaryKeyColumn as $pkColumn) { - if ($data[$pkColumn] !== $this->primaryKeyData[$pkColumn]) { - $where[$pkColumn] = $data[$pkColumn]; - } - } - } - } else { - $insert = $this->sql->insert(); - $insert->values($this->data); - - $statement = $this->sql->prepareStatementForSqlObject($insert); - $result = $statement->execute(); - if (($primaryKeyValue = $result->getGeneratedValue()) && count($this->primaryKeyColumn) === 1) { - $this->primaryKeyData = [$this->primaryKeyColumn[0] => $primaryKeyValue]; - } else { - $this->processPrimaryKeyData(); - } - $rowsAffected = $result->getAffectedRows(); - unset($statement, $result); + [$rowsAffected, $where] = $this->rowExistsInDatabase() + ? $this->updateRow($sql) + : $this->insertRow($sql); - $where = []; - foreach ($this->primaryKeyColumn as $pkColumn) { - $where[$pkColumn] = $this->primaryKeyData[$pkColumn]; - } + $statement = $sql->prepareStatementForSqlObject($sql->select()->where($where)); + $rowData = $this->executeStatement($statement)->current(); + if (! is_array($rowData)) { + throw Exception\RuntimeException::forMissingSavedRow(); } - $statement = $this->sql->prepareStatementForSqlObject($this->sql->select()->where($where)); - $result = $statement->execute(); - $rowData = $result->current(); - unset($statement, $result); - $this->populate($rowData, true); return $rowsAffected; } + /** + * @return array + */ + #[Override] public function toArray(): array { return $this->data; @@ -256,13 +251,115 @@ public function toArray(): array */ protected function processPrimaryKeyData(): void { - $this->primaryKeyData = []; - foreach ($this->primaryKeyColumn as $column) { - if (! isset($this->data[$column])) { - throw Exception\RuntimeException::forMissingPrimaryKeyData($column); + $primaryKeyData = []; + foreach ($this->requirePrimaryKeyColumn() as $column) { + $primaryKeyData[$column] = $this->data[$column] + ?? throw Exception\RuntimeException::forMissingPrimaryKeyData($column); + } + $this->primaryKeyData = $primaryKeyData; + } + + /** + * @throws Exception\RuntimeException + */ + private function executeStatement(StatementInterface $statement): ResultInterface + { + return $statement->execute() ?? throw Exception\RuntimeException::forMissingStatementResult(); + } + + /** + * Inserts the row and returns the affected row count with the WHERE that reselects it. + * + * @return array{int, array} + * + * @throws Exception\RuntimeException + * @throws SqlRuntimeException + */ + private function insertRow(Sql $sql): array + { + $primaryKeyColumn = $this->requirePrimaryKeyColumn(); + + $insert = $sql->insert(); + $insert->values($this->data); + + $result = $this->executeStatement($sql->prepareStatementForSqlObject($insert)); + $this->processInsertedPrimaryKey($primaryKeyColumn, $result->getGeneratedValue()); + + $where = []; + foreach ($primaryKeyColumn as $pkColumn) { + $where[$pkColumn] = $this->primaryKeyData[$pkColumn] ?? null; + } + + return [$result->getAffectedRows(), $where]; + } + + /** + * Takes a single-column primary key from the generated value, or every column from the row data. + * + * @param list $primaryKeyColumn + * + * @throws Exception\RuntimeException + */ + private function processInsertedPrimaryKey(array $primaryKeyColumn, string|int|false|null $generatedValue): void + { + if ($generatedValue && 1 === count($primaryKeyColumn)) { + $this->primaryKeyData = [$primaryKeyColumn[0] => $generatedValue]; + + return; + } + + $this->processPrimaryKeyData(); + } + + /** + * @return list + * + * @throws Exception\RuntimeException + */ + private function requirePrimaryKeyColumn(): array + { + return $this->primaryKeyColumn ?? throw Exception\RuntimeException::forMissingPrimaryKeyColumn(); + } + + /** @throws Exception\RuntimeException */ + private function requireSql(): Sql + { + return $this->sql ?? throw Exception\RuntimeException::forMissingSqlObject(); + } + + /** + * Updates the row and returns the affected row count with the WHERE that reselects it. + * + * @return array{int, array} + * + * @throws Exception\RuntimeException + * @throws SqlRuntimeException + */ + private function updateRow(Sql $sql): array + { + $primaryKeyColumn = $this->requirePrimaryKeyColumn(); + $primaryKeyData = $this->primaryKeyData ?? []; + $data = $this->data; + $where = []; + + foreach ($primaryKeyColumn as $pkColumn) { + $where[$pkColumn] = $primaryKeyData[$pkColumn] ?? null; + if (($data[$pkColumn] ?? null) === $where[$pkColumn]) { + unset($data[$pkColumn]); } - $this->primaryKeyData[$column] = $this->data[$column]; } + + $statement = $sql->prepareStatementForSqlObject($sql->update()->set($data)->where($where)); + $rowsAffected = $this->executeStatement($statement)->getAffectedRows(); + + foreach ($primaryKeyColumn as $pkColumn) { + if (! array_key_exists($pkColumn, $data)) { + continue; + } + $where[$pkColumn] = $data[$pkColumn]; + } + + return [$rowsAffected, $where]; } /** diff --git a/src/RowGateway/Exception/InvalidArgumentException.php b/src/RowGateway/Exception/InvalidArgumentException.php index 8028318e8..18bd53cc2 100644 --- a/src/RowGateway/Exception/InvalidArgumentException.php +++ b/src/RowGateway/Exception/InvalidArgumentException.php @@ -8,6 +8,9 @@ use function sprintf; +/** + * @api + */ class InvalidArgumentException extends Exception\InvalidArgumentException { final public const string INVALID_COLUMN = 'Not a valid column in this row: %s'; diff --git a/src/RowGateway/Exception/RuntimeException.php b/src/RowGateway/Exception/RuntimeException.php index 9a11226ee..acf35c825 100644 --- a/src/RowGateway/Exception/RuntimeException.php +++ b/src/RowGateway/Exception/RuntimeException.php @@ -8,6 +8,9 @@ use function sprintf; +/** + * @api + */ class RuntimeException extends Exception\RuntimeException { final public const string MISSING_PRIMARY_KEY_COLUMN = @@ -18,8 +21,12 @@ class RuntimeException extends Exception\RuntimeException 'While processing primary key data, a known key %s' . ' was not found in the data array'; + final public const string MISSING_SAVED_ROW = 'The saved row could not be read back from the database.'; + final public const string MISSING_SQL_OBJECT = 'This row object does not have a Sql object set.'; + final public const string MISSING_STATEMENT_RESULT = 'The statement did not return a result.'; + final public const string MISSING_TABLE = 'This row object does not have a valid table set.'; final public const string UNCALLABLE_METHOD = 'This method is not intended to be called on this object.'; @@ -34,11 +41,21 @@ public static function forMissingPrimaryKeyData(string $column): self return new self(sprintf(self::MISSING_PRIMARY_KEY_DATA, $column)); } + public static function forMissingSavedRow(): self + { + return new self(self::MISSING_SAVED_ROW); + } + public static function forMissingSqlObject(): self { return new self(self::MISSING_SQL_OBJECT); } + public static function forMissingStatementResult(): self + { + return new self(self::MISSING_STATEMENT_RESULT); + } + public static function forMissingTable(): self { return new self(self::MISSING_TABLE); diff --git a/src/RowGateway/Feature/FeatureSet.php b/src/RowGateway/Feature/FeatureSet.php index 1e567d230..b85ae794f 100644 --- a/src/RowGateway/Feature/FeatureSet.php +++ b/src/RowGateway/Feature/FeatureSet.php @@ -20,9 +20,6 @@ class FeatureSet /** @var FeatureInterface[] */ protected array $features = []; - /** @var array */ - protected array $magicSpecifications = []; - /** @param FeatureInterface[] $features */ public function __construct(array $features = []) { diff --git a/src/RowGateway/RowGateway.php b/src/RowGateway/RowGateway.php index f13bd498a..bc412ab54 100644 --- a/src/RowGateway/RowGateway.php +++ b/src/RowGateway/RowGateway.php @@ -10,12 +10,18 @@ use function is_string; +/** + * @api + */ class RowGateway extends AbstractRowGateway { /** * Constructor * + * @param string|list|null $primaryKeyColumn + * * @throws Exception\InvalidArgumentException + * @throws Exception\RuntimeException */ public function __construct( string|array|null $primaryKeyColumn, @@ -24,7 +30,7 @@ public function __construct( ) { // setup primary key if (is_string($primaryKeyColumn)) { - $primaryKeyColumn = '' !== $primaryKeyColumn ? (array) $primaryKeyColumn : null; + $primaryKeyColumn = '' === $primaryKeyColumn ? null : [$primaryKeyColumn]; } $this->primaryKeyColumn = $primaryKeyColumn; @@ -32,11 +38,7 @@ public function __construct( $this->table = $table; // set Sql object - if ($adapterOrSql instanceof Sql) { - $this->sql = $adapterOrSql; - } else { - $this->sql = new Sql($adapterOrSql, $this->table); - } + $this->sql = $adapterOrSql instanceof Sql ? $adapterOrSql : new Sql($adapterOrSql, $this->table); if ($this->sql->getTable() !== $this->table) { throw Exception\InvalidArgumentException::forSqlTableMismatch(); diff --git a/src/RowGateway/RowGatewayInterface.php b/src/RowGateway/RowGatewayInterface.php index e96b96528..1204881df 100644 --- a/src/RowGateway/RowGatewayInterface.php +++ b/src/RowGateway/RowGatewayInterface.php @@ -6,6 +6,9 @@ use PhpDb\ResultSet\RowPrototypeInterface; +/** + * @api + */ interface RowGatewayInterface extends RowPrototypeInterface { public function delete(): int; diff --git a/src/Sql/Delete.php b/src/Sql/Delete.php index b76a0fbd6..144409461 100644 --- a/src/Sql/Delete.php +++ b/src/Sql/Delete.php @@ -66,6 +66,9 @@ public function from(TableIdentifier|string|array $table): static return $this; } + /** + * @return ($key is 'table' ? TableIdentifier|string|array : mixed) + */ public function getRawState(?string $key = null): mixed { $rawState = [ diff --git a/src/Sql/Insert.php b/src/Sql/Insert.php index ab76640d4..eb6da4e72 100644 --- a/src/Sql/Insert.php +++ b/src/Sql/Insert.php @@ -83,11 +83,13 @@ public function columns(array $columns): static /** * Get raw state * - * @return TableIdentifier|string|array|list|array{ - * table: TableIdentifier|string|array, - * columns: list, - * values: list, - * } + * @return ($key is 'table' ? TableIdentifier|string|array : ( + * TableIdentifier|string|array|list|array{ + * table: TableIdentifier|string|array, + * columns: list, + * values: list, + * } + * )) */ public function getRawState(?string $key = null): TableIdentifier|string|array { diff --git a/src/Sql/Select.php b/src/Sql/Select.php index 54849fd3f..e8168b97e 100644 --- a/src/Sql/Select.php +++ b/src/Sql/Select.php @@ -244,7 +244,7 @@ public function from(array|string|TableIdentifier $table): static } /** - * @return ($key is self::COLUMNS ? ColumnList : mixed) + * @return ($key is self::TABLE ? TableReference|null : ($key is self::COLUMNS ? ColumnList : mixed)) */ public function getRawState(?string $key = null): mixed { diff --git a/src/Sql/Update.php b/src/Sql/Update.php index cd5f4fdfc..5066e61a4 100644 --- a/src/Sql/Update.php +++ b/src/Sql/Update.php @@ -84,6 +84,9 @@ public function __construct(string|TableIdentifier|array|null $table = null) } } + /** + * @return ($key is 'table' ? TableIdentifier|string|array : mixed) + */ public function getRawState(?string $key = null): mixed { $rawState = [ diff --git a/src/TableGateway/AbstractTableGateway.php b/src/TableGateway/AbstractTableGateway.php index 203982bf8..a0842496f 100644 --- a/src/TableGateway/AbstractTableGateway.php +++ b/src/TableGateway/AbstractTableGateway.php @@ -9,9 +9,12 @@ use PhpDb\Adapter\AdapterInterface; use PhpDb\ResultSet\ResultSet; use PhpDb\ResultSet\ResultSetInterface; +use PhpDb\Sql\AbstractSql; use PhpDb\Sql\Delete; +use PhpDb\Sql\Exception\RuntimeException as SqlRuntimeException; use PhpDb\Sql\Insert; use PhpDb\Sql\Join; +use PhpDb\Sql\Predicate\PredicateSet; use PhpDb\Sql\Select; use PhpDb\Sql\Sql; use PhpDb\Sql\TableIdentifier; @@ -19,18 +22,24 @@ use PhpDb\Sql\Where; use PhpDb\TableGateway\Feature\EventFeatureEventsInterface; -use function array_shift; +use function array_map; use function array_values; -use function count; use function end; +use function in_array; use function is_array; -use function is_object; -use function reset; /** - * @property AdapterInterface $adapter - * @property int $lastInsertValue - * @property TableIdentifier|string|array|null $table + * @api + * @psalm-import-type ColumnValue from AbstractSql + * @psalm-import-type ColumnList from Select + * @psalm-import-type JoinName from Join + * @psalm-import-type PredicateValue from PredicateSet + * @psalm-type GatewayTable = TableIdentifier|string|array + * @psalm-type GatewayWhere = Where|Closure|string|array + * @psalm-type GatewayJoin = array{name: JoinName, on: string, type?: string} + * @property AdapterInterface|null $adapter + * @property string|int|false|null $lastInsertValue + * @property GatewayTable|null $table */ abstract class AbstractTableGateway implements TableGatewayInterface { @@ -38,8 +47,10 @@ abstract class AbstractTableGateway implements TableGatewayInterface protected ?AdapterInterface $adapter = null; + /** @var GatewayTable|null */ protected TableIdentifier|string|array|null $table = null; + /** @var ColumnList */ protected array $columns = []; protected ?Feature\FeatureSet $featureSet = null; @@ -50,24 +61,70 @@ abstract class AbstractTableGateway implements TableGatewayInterface protected string|int|false|null $lastInsertValue = null; - /** @throws Exception\RuntimeException */ + /** + * @param GatewayTable|null $table + * + * @return GatewayTable|null + */ + private static function cloneTable(TableIdentifier|string|array|null $table): TableIdentifier|string|array|null + { + if ($table instanceof TableIdentifier) { + return clone $table; + } + + if (! is_array($table)) { + return $table; + } + + return array_map( + static fn(TableIdentifier|string $aliased): TableIdentifier|string => $aliased instanceof TableIdentifier + ? clone $aliased + : $aliased, + $table, + ); + } + + /** + * Returns the bare table of an aliased table array, or null when the table is not aliased. + * + * @param GatewayTable $table + */ + private static function unaliasTable(TableIdentifier|string|array $table): TableIdentifier|string|null + { + if (! is_array($table)) { + return null; + } + + return array_values($table)[0] ?? null; + } + + /** + * @param GatewayWhere $where + * + * @throws Exception\RuntimeException + * @throws SqlRuntimeException + */ #[Override] public function delete(Where|Closure|array|string $where): int { if (! $this->isInitialized) { $this->initialize(); } - $delete = $this->sql->delete(); + $delete = $this->requireSql()->delete(); if ($where instanceof Closure) { $where($delete); - } else { - $delete->where($where); + + return $this->executeDelete($delete); } + $delete->where($where); return $this->executeDelete($delete); } - /** @throws Exception\RuntimeException */ + /** + * @throws Exception\RuntimeException + * @throws SqlRuntimeException + */ public function deleteWith(Delete $delete): int { $this->initialize(); @@ -75,11 +132,13 @@ public function deleteWith(Delete $delete): int return $this->executeDelete($delete); } + /** @throws Exception\RuntimeException */ public function getAdapter(): AdapterInterface { - return $this->adapter; + return $this->adapter ?? throw Exception\RuntimeException::forMissingAdapter(); } + /** @return ColumnList */ public function getColumns(): array { return $this->columns; @@ -92,7 +151,7 @@ public function getFeatureSet(): Feature\FeatureSet $this->initialize(); } - return $this->featureSet; + return $this->requireFeatureSet(); } public function getLastInsertValue(): string|int|false|null @@ -107,7 +166,7 @@ public function getResultSetPrototype(): ResultSetInterface $this->initialize(); } - return $this->resultSetPrototype; + return $this->requireResultSetPrototype(); } /** @throws Exception\RuntimeException */ @@ -117,13 +176,18 @@ public function getSql(): Sql $this->initialize(); } - return $this->sql; + return $this->requireSql(); } + /** + * @return GatewayTable + * + * @throws Exception\RuntimeException + */ #[Override] public function getTable(): TableIdentifier|array|string { - return $this->table; + return $this->table ?? throw Exception\RuntimeException::forMissingTable(); } /** @@ -137,48 +201,45 @@ public function initialize(): void return; } - if (null === $this->featureSet) { - $this->featureSet = new Feature\FeatureSet(); - } - - $this->featureSet->setTableGateway($this); - $this->featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_INITIALIZE, []); - - if (null === $this->adapter) { - throw Exception\RuntimeException::forMissingAdapter(); - } + $this->featureSet ??= new Feature\FeatureSet(); + $featureSet = $this->featureSet; - if (null === $this->table) { - throw Exception\RuntimeException::forMissingTable(); - } + $featureSet->setTableGateway($this); + $featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_INITIALIZE, []); - if (null === $this->resultSetPrototype) { - $this->resultSetPrototype = new ResultSet(); - } + $adapter = $this->adapter ?? throw Exception\RuntimeException::forMissingAdapter(); + $table = $this->table ?? throw Exception\RuntimeException::forMissingTable(); - if (null === $this->sql) { - $this->sql = new Sql($this->adapter, $this->table); - } + $this->resultSetPrototype ??= new ResultSet(); + $this->sql ??= new Sql($adapter, $table); - $this->featureSet->apply(EventFeatureEventsInterface::EVENT_POST_INITIALIZE, []); + $featureSet->apply(EventFeatureEventsInterface::EVENT_POST_INITIALIZE, []); $this->isInitialized = true; } - /** @throws Exception\RuntimeException */ + /** + * @param array $set + * + * @throws Exception\RuntimeException + * @throws SqlRuntimeException + */ #[Override] public function insert(array $set): int { if (! $this->isInitialized) { $this->initialize(); } - $insert = $this->sql->insert(); + $insert = $this->requireSql()->insert(); $insert->values($set); return $this->executeInsert($insert); } - /** @throws Exception\RuntimeException */ + /** + * @throws Exception\RuntimeException + * @throws SqlRuntimeException + */ public function insertWith(Insert $insert): int { if (! $this->isInitialized) { @@ -193,7 +254,12 @@ public function isInitialized(): bool return $this->isInitialized; } - /** @throws Exception\RuntimeException */ + /** + * @param GatewayWhere|null $where + * + * @throws Exception\RuntimeException + * @throws SqlRuntimeException + */ #[Override] public function select(Where|Closure|string|array|null $where = null): ResultSetInterface { @@ -201,18 +267,25 @@ public function select(Where|Closure|string|array|null $where = null): ResultSet $this->initialize(); } - $select = $this->sql->select(); + $select = $this->requireSql()->select(); if ($where instanceof Closure) { $where($select); - } elseif (null !== $where) { + + return $this->selectWith($select); + } + + if (null !== $where) { $select->where($where); } return $this->selectWith($select); } - /** @throws Exception\RuntimeException */ + /** + * @throws Exception\RuntimeException + * @throws SqlRuntimeException + */ public function selectWith(Select $select): ResultSetInterface { if (! $this->isInitialized) { @@ -222,7 +295,14 @@ public function selectWith(Select $select): ResultSetInterface return $this->executeSelect($select); } - /** @throws Exception\RuntimeException */ + /** + * @param array $set + * @param GatewayWhere|null $where + * @param list|null $joins + * + * @throws Exception\RuntimeException + * @throws SqlRuntimeException + */ #[Override] public function update( array $set, @@ -232,24 +312,23 @@ public function update( if (! $this->isInitialized) { $this->initialize(); } - $sql = $this->sql; - $update = $sql->update(); + $update = $this->requireSql()->update(); $update->set($set); if (null !== $where) { $update->where($where); } - if ($joins) { - foreach ($joins as $join) { - $type = $join['type'] ?? Join::JOIN_INNER; - $update->join($join['name'], $join['on'], $type); - } + foreach ($joins ?? [] as $join) { + $update->join($join['name'], $join['on'], $join['type'] ?? Join::JOIN_INNER); } return $this->executeUpdate($update); } - /** @throws Exception\RuntimeException */ + /** + * @throws Exception\RuntimeException + * @throws SqlRuntimeException + */ public function updateWith(Update $update): int { if (! $this->isInitialized) { @@ -261,34 +340,35 @@ public function updateWith(Update $update): int /** * @throws Exception\RuntimeException + * @throws SqlRuntimeException * @todo add $columns support */ protected function executeDelete(Delete $delete): int { - $deleteState = $delete->getRawState(); - if ($deleteState['table'] !== $this->table) { + $table = $delete->getRawState('table'); + if ($table !== $this->table) { throw Exception\RuntimeException::forTableMismatch('Delete'); } + $featureSet = $this->requireFeatureSet(); + // pre delete update - $this->featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_DELETE, [$delete]); + $featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_DELETE, [$delete]); - $unaliasedTable = false; - if (is_array($deleteState['table'])) { - $tableData = array_values($deleteState['table']); - $unaliasedTable = array_shift($tableData); + $unaliasedTable = self::unaliasTable($table); + if (null !== $unaliasedTable) { $delete->from($unaliasedTable); } - $statement = $this->sql->prepareStatementForSqlObject($delete); - $result = $statement->execute(); + $statement = $this->requireSql()->prepareStatementForSqlObject($delete); + $result = $statement->execute() ?? throw Exception\RuntimeException::forMissingStatementResult('Delete'); // apply postDelete features - $this->featureSet->apply(EventFeatureEventsInterface::EVENT_POST_DELETE, [$statement, $result]); + $featureSet->apply(EventFeatureEventsInterface::EVENT_POST_DELETE, [$statement, $result]); // Reset original table information in Delete instance, if necessary - if ($unaliasedTable) { - $delete->from($deleteState['table']); + if (null !== $unaliasedTable) { + $delete->from($table); } return $result->getAffectedRows(); @@ -296,37 +376,39 @@ protected function executeDelete(Delete $delete): int /** * @throws Exception\RuntimeException + * @throws SqlRuntimeException * @todo add $columns support */ protected function executeInsert(Insert $insert): int { - $insertState = $insert->getRawState(); - if ($insertState['table'] !== $this->table) { + $table = $insert->getRawState('table'); + if ($table !== $this->table) { throw Exception\RuntimeException::forTableMismatch('Insert'); } + $featureSet = $this->requireFeatureSet(); + // apply preInsert features - $this->featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_INSERT, [$insert]); + $featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_INSERT, [$insert]); // Most RDBMS solutions do not allow using table aliases in INSERTs // See https://github.com/zendframework/zf2/issues/7311 - $unaliasedTable = false; - if (is_array($insertState['table'])) { - $tableData = array_values($insertState['table']); - $unaliasedTable = array_shift($tableData); + $unaliasedTable = self::unaliasTable($table); + if (null !== $unaliasedTable) { $insert->into($unaliasedTable); } - $statement = $this->sql->prepareStatementForSqlObject($insert); - $result = $statement->execute(); - $this->lastInsertValue = $this->adapter->getDriver()->getConnection()->getLastGeneratedValue(); + $statement = $this->requireSql()->prepareStatementForSqlObject($insert); + $result = $statement->execute() + ?? throw Exception\RuntimeException::forMissingStatementResult('Insert'); + $this->lastInsertValue = $this->getAdapter()->getDriver()->getConnection()->getLastGeneratedValue(); // apply postInsert features - $this->featureSet->apply(EventFeatureEventsInterface::EVENT_POST_INSERT, [$statement, $result]); + $featureSet->apply(EventFeatureEventsInterface::EVENT_POST_INSERT, [$statement, $result]); // Reset original table information in Insert instance, if necessary - if ($unaliasedTable) { - $insert->into($insertState['table']); + if (null !== $unaliasedTable) { + $insert->into($table); } return $result->getAffectedRows(); @@ -334,128 +416,149 @@ protected function executeInsert(Insert $insert): int /** * @throws Exception\RuntimeException + * @throws SqlRuntimeException */ protected function executeSelect(Select $select): ResultSetInterface { - $selectState = $select->getRawState(); - if ( - isset($selectState['table']) - && $selectState['table'] !== $this->table - && ( - is_array($selectState['table']) - && end($selectState['table']) !== $this->table - ) - ) { + $table = $select->getRawState(Select::TABLE); + if (is_array($table) && $table !== $this->table && end($table) !== $this->table) { throw Exception\RuntimeException::forTableMismatch('Select'); } - if ( - isset($selectState['columns']) - && [Select::SQL_STAR] === $selectState['columns'] - && [] !== $this->columns - ) { + if ([Select::SQL_STAR] === $select->getRawState(Select::COLUMNS) && [] !== $this->columns) { $select->columns($this->columns); } + $featureSet = $this->requireFeatureSet(); + // apply preSelect features - $this->featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_SELECT, [$select]); + $featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_SELECT, [$select]); // prepare and execute - $statement = $this->sql->prepareStatementForSqlObject($select); - $result = $statement->execute(); + $statement = $this->requireSql()->prepareStatementForSqlObject($select); + $result = $statement->execute() ?? throw Exception\RuntimeException::forMissingStatementResult('Select'); // build result set - $resultSet = clone $this->resultSetPrototype; + $resultSet = clone $this->requireResultSetPrototype(); $resultSet->initialize($result); // apply postSelect features - $this->featureSet->apply(EventFeatureEventsInterface::EVENT_POST_SELECT, [$statement, $result, $resultSet]); + $featureSet->apply(EventFeatureEventsInterface::EVENT_POST_SELECT, [$statement, $result, $resultSet]); return $resultSet; } /** * @throws Exception\RuntimeException + * @throws SqlRuntimeException * @todo add $columns support */ protected function executeUpdate(Update $update): int { - $updateState = $update->getRawState(); - if ($updateState['table'] !== $this->table) { + $table = $update->getRawState('table'); + if ($table !== $this->table) { throw Exception\RuntimeException::forTableMismatch('Update'); } + $featureSet = $this->requireFeatureSet(); + // apply preUpdate features - $this->featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_UPDATE, [$update]); + $featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_UPDATE, [$update]); - $unaliasedTable = false; - if (is_array($updateState['table'])) { - $tableData = array_values($updateState['table']); - $unaliasedTable = array_shift($tableData); + $unaliasedTable = self::unaliasTable($table); + if (null !== $unaliasedTable) { $update->table($unaliasedTable); } - $statement = $this->sql->prepareStatementForSqlObject($update); - $result = $statement->execute(); + $statement = $this->requireSql()->prepareStatementForSqlObject($update); + $result = $statement->execute() ?? throw Exception\RuntimeException::forMissingStatementResult('Update'); // apply postUpdate features - $this->featureSet->apply(EventFeatureEventsInterface::EVENT_POST_UPDATE, [$statement, $result]); + $featureSet->apply(EventFeatureEventsInterface::EVENT_POST_UPDATE, [$statement, $result]); // Reset original table information in Update instance, if necessary - if ($unaliasedTable) { - $update->table($updateState['table']); + if (null !== $unaliasedTable) { + $update->table($table); } return $result->getAffectedRows(); } + /** @throws Exception\RuntimeException */ + private function requireFeatureSet(): Feature\FeatureSet + { + return $this->featureSet ?? throw Exception\RuntimeException::forUninitializedProperty('featureSet'); + } + + /** @throws Exception\RuntimeException */ + private function requireResultSetPrototype(): ResultSetInterface + { + return ( + $this->resultSetPrototype + ?? throw Exception\RuntimeException::forUninitializedProperty('resultSetPrototype') + ); + } + + /** @throws Exception\RuntimeException */ + private function requireSql(): Sql + { + return $this->sql ?? throw Exception\RuntimeException::forUninitializedProperty('sql'); + } + /** + * @param array $arguments + * * @throws Exception\InvalidArgumentException + * @throws Exception\RuntimeException */ public function __call(string $method, array $arguments): mixed { - if ($this->featureSet->canCallMagicCall($method)) { - return $this->featureSet->callMagicCall($method, $arguments); + $featureSet = $this->requireFeatureSet(); + if ($featureSet->canCallMagicCall($method)) { + return $featureSet->callMagicCall($method, $arguments); } throw Exception\InvalidArgumentException::forInvalidMagicCall($method, static::class); } public function __clone(): void { - $this->resultSetPrototype = isset($this->resultSetPrototype) ? clone $this->resultSetPrototype : null; - $this->sql = clone $this->sql; - if (is_object($this->table)) { - $this->table = clone $this->table; - } elseif ( - is_array($this->table) - && count($this->table) === 1 - && is_object(reset($this->table)) - ) { - foreach ($this->table as &$tableObject) { - $tableObject = clone $tableObject; - } + if (null !== $this->resultSetPrototype) { + $this->resultSetPrototype = clone $this->resultSetPrototype; + } + + if (null !== $this->sql) { + $this->sql = clone $this->sql; } + + $this->table = self::cloneTable($this->table); } /** * @throws Exception\InvalidArgumentException + * @throws Exception\RuntimeException */ public function __get(string $property): mixed { - return match (true) { - 'lastInsertValue' === $property, 'adapter' === $property, 'table' === $property => $this->$property, - $this->featureSet->canCallMagicGet($property) => $this->featureSet->callMagicGet($property), - default => throw Exception\InvalidArgumentException::forInvalidMagicGet(static::class), - }; + if (in_array($property, ['lastInsertValue', 'adapter', 'table'], strict: true)) { + return $this->$property; + } + + $featureSet = $this->requireFeatureSet(); + if ($featureSet->canCallMagicGet($property)) { + return $featureSet->callMagicGet($property); + } + throw Exception\InvalidArgumentException::forInvalidMagicGet(static::class); } /** * @throws Exception\InvalidArgumentException + * @throws Exception\RuntimeException */ public function __set(string $property, mixed $value): void { - if ($this->featureSet->canCallMagicSet($property)) { - $this->featureSet->callMagicSet($property, $value); + $featureSet = $this->requireFeatureSet(); + if ($featureSet->canCallMagicSet($property)) { + $featureSet->callMagicSet($property, $value); return; } diff --git a/src/TableGateway/Exception/InvalidArgumentException.php b/src/TableGateway/Exception/InvalidArgumentException.php index 70ef23a92..744344839 100644 --- a/src/TableGateway/Exception/InvalidArgumentException.php +++ b/src/TableGateway/Exception/InvalidArgumentException.php @@ -8,6 +8,9 @@ use function sprintf; +/** + * @api + */ class InvalidArgumentException extends Exception\InvalidArgumentException { final public const string INVALID_MAGIC_CALL = 'Invalid method (%s) called, caught by %s::__call()'; diff --git a/src/TableGateway/Exception/RuntimeException.php b/src/TableGateway/Exception/RuntimeException.php index 8ee596b9e..58e5f4812 100644 --- a/src/TableGateway/Exception/RuntimeException.php +++ b/src/TableGateway/Exception/RuntimeException.php @@ -8,6 +8,9 @@ use function sprintf; +/** + * @api + */ class RuntimeException extends Exception\RuntimeException { final public const string MISSING_ADAPTER = 'This table does not have an Adapter setup'; @@ -35,6 +38,8 @@ class RuntimeException extends Exception\RuntimeException 'The table gateway must be initialized with a Sql instance' . ' before this feature is applied.'; + final public const string MISSING_STATEMENT_RESULT = 'The %s statement did not return a result.'; + final public const string MISSING_STATIC_ADAPTER = 'No database adapter was found in the static registry.'; final public const string MISSING_TABLE = 'This table object does not have a valid table set.'; @@ -51,6 +56,10 @@ class RuntimeException extends Exception\RuntimeException 'This feature %s expects the ResultSet' . ' to be an instance of %s'; + final public const string UNINITIALIZED_PROPERTY = + 'The table gateway has no %s;' + . ' initialize() has not been run.'; + final public const string UNNAMED_TABLE_IN_METADATA = 'The table gateway must reference a named table' . ' before metadata can be resolved.'; @@ -111,6 +120,11 @@ public static function forMissingSqlInstance(): self return new self(self::MISSING_SQL_INSTANCE); } + public static function forMissingStatementResult(string $statement): self + { + return new self(sprintf(self::MISSING_STATEMENT_RESULT, $statement)); + } + public static function forMissingStaticAdapter(): self { return new self(self::MISSING_STATIC_ADAPTER); @@ -141,6 +155,11 @@ public static function forUnexpectedResultSet(string $feature, string $expected) return new self(sprintf(self::UNEXPECTED_RESULT_SET, $feature, $expected)); } + public static function forUninitializedProperty(string $property): self + { + return new self(sprintf(self::UNINITIALIZED_PROPERTY, $property)); + } + public static function forUnnamedTableInMetadata(): self { return new self(self::UNNAMED_TABLE_IN_METADATA); diff --git a/src/TableGateway/Feature/EventFeature/TableGatewayEvent.php b/src/TableGateway/Feature/EventFeature/TableGatewayEvent.php index df747c1dd..5511abf16 100644 --- a/src/TableGateway/Feature/EventFeature/TableGatewayEvent.php +++ b/src/TableGateway/Feature/EventFeature/TableGatewayEvent.php @@ -40,6 +40,8 @@ public function getParam($name, $default = null): mixed /** * Get parameters passed to the event + * + * @return array|object */ #[Override] public function getParams(): array|object diff --git a/src/TableGateway/Feature/MetadataFeature.php b/src/TableGateway/Feature/MetadataFeature.php index ad0340d5a..289cf4f4f 100644 --- a/src/TableGateway/Feature/MetadataFeature.php +++ b/src/TableGateway/Feature/MetadataFeature.php @@ -34,6 +34,8 @@ public function __construct( /** * @throws Exception\RuntimeException + * + * @mago-expect analysis:mixed-assignment */ public function postInitialize(): void { diff --git a/src/TableGateway/Feature/RowGatewayFeature.php b/src/TableGateway/Feature/RowGatewayFeature.php index f0ca94bb4..caa6e99f0 100644 --- a/src/TableGateway/Feature/RowGatewayFeature.php +++ b/src/TableGateway/Feature/RowGatewayFeature.php @@ -5,6 +5,7 @@ namespace PhpDb\TableGateway\Feature; use PhpDb\ResultSet\RowPrototypeResultSet; +use PhpDb\RowGateway\Exception\RuntimeException as RowGatewayRuntimeException; use PhpDb\RowGateway\RowGateway; use PhpDb\RowGateway\RowGatewayInterface; use PhpDb\Sql\TableIdentifier; @@ -25,6 +26,9 @@ public function __construct(mixed ...$constructorArguments) /** * @throws Exception\RuntimeException + * @throws RowGatewayRuntimeException + * + * @mago-expect analysis:mixed-assignment */ public function postInitialize(): void { @@ -54,12 +58,12 @@ public function postInitialize(): void $resultSetPrototype->setRowPrototype(new RowGateway( $primaryKey, $table, - $this->tableGateway->adapter, + $this->tableGateway->getAdapter(), )); } /** - * @return string|array + * @return string|list * * @throws Exception\RuntimeException * @@ -82,10 +86,22 @@ private function primaryKeyFromMetadata(): string|array } $primaryKey = $metadataData['primaryKey'] ?? null; - if (! is_string($primaryKey) && ! is_array($primaryKey)) { + if (is_string($primaryKey)) { + return $primaryKey; + } + + if (! is_array($primaryKey)) { throw Exception\RuntimeException::forUnusablePrimaryKey(); } - return $primaryKey; + $columns = []; + foreach ($primaryKey as $column) { + if (! is_string($column)) { + throw Exception\RuntimeException::forUnusablePrimaryKey(); + } + $columns[] = $column; + } + + return $columns; } } diff --git a/src/TableGateway/Feature/SequenceFeature.php b/src/TableGateway/Feature/SequenceFeature.php index 23319806d..2329ef4c8 100644 --- a/src/TableGateway/Feature/SequenceFeature.php +++ b/src/TableGateway/Feature/SequenceFeature.php @@ -39,7 +39,7 @@ public function __construct(string $primaryKeyField, string $sequenceName) */ public function lastSequenceId(): int { - $platform = $this->tableGateway->adapter->getPlatform(); + $platform = $this->tableGateway->getAdapter()->getPlatform(); $platformName = $platform->getName(); // TODO(@simon-mundy): Remove string usage @@ -49,7 +49,7 @@ public function lastSequenceId(): int default => throw RuntimeException::forUnsupportedLastSequencePlatform(), }; - $statement = $this->tableGateway->adapter->createStatement(); + $statement = $this->tableGateway->getAdapter()->createStatement(); $statement->prepare($sql); $result = $statement->execute(); if (! $result instanceof ResultInterface) { @@ -75,7 +75,7 @@ public function lastSequenceId(): int */ public function nextSequenceId(): ?int { - $platform = $this->tableGateway->adapter->getPlatform(); + $platform = $this->tableGateway->getAdapter()->getPlatform(); $platformName = $platform->getName(); $sql = match ($platformName) { @@ -84,7 +84,7 @@ public function nextSequenceId(): ?int default => throw RuntimeException::forUnsupportedNextSequencePlatform(), }; - $statement = $this->tableGateway->adapter->createStatement(); + $statement = $this->tableGateway->getAdapter()->createStatement(); $statement->prepare($sql); $result = $statement->execute(); if (! $result instanceof ResultInterface) { diff --git a/src/TableGateway/TableGateway.php b/src/TableGateway/TableGateway.php index 3fe7b0ddc..7d04b9f19 100644 --- a/src/TableGateway/TableGateway.php +++ b/src/TableGateway/TableGateway.php @@ -12,10 +12,15 @@ use function is_array; +/** + * @api + * @psalm-import-type GatewayTable from AbstractTableGateway + */ class TableGateway extends AbstractTableGateway { /** - * @param Feature\FeatureSet|Feature\FeatureInterface|Feature\FeatureInterface[]|null $features + * @param GatewayTable $table + * @param Feature\FeatureSet|Feature\FeatureInterface|list|null $features * * @throws Exception\InvalidArgumentException * @throws Exception\RuntimeException @@ -39,7 +44,7 @@ public function __construct( $this->resultSetPrototype = $resultSetPrototype; - $this->sql = $sql ?: new Sql($this->adapter, $this->table); + $this->sql = $sql ?? new Sql($this->adapter, $this->table); if ($this->sql->getTable() !== $this->table) { throw Exception\InvalidArgumentException::forSqlTableMismatch(); diff --git a/src/TableGateway/TableGatewayInterface.php b/src/TableGateway/TableGatewayInterface.php index cacf807bc..49d7ce353 100644 --- a/src/TableGateway/TableGatewayInterface.php +++ b/src/TableGateway/TableGatewayInterface.php @@ -6,24 +6,35 @@ use Closure; use PhpDb\ResultSet\ResultSetInterface; +use PhpDb\Sql\AbstractSql; use PhpDb\Sql\TableIdentifier; use PhpDb\Sql\Where; +/** + * @api + * @psalm-import-type ColumnValue from AbstractSql + * @psalm-import-type GatewayTable from AbstractTableGateway + * @psalm-import-type GatewayWhere from AbstractTableGateway + */ interface TableGatewayInterface { + /** @param GatewayWhere $where */ public function delete(Where|Closure|array|string $where): int; + /** @return GatewayTable */ public function getTable(): TableIdentifier|string|array; /** - * @param array $set + * @param array $set */ public function insert(array $set): int; + /** @param GatewayWhere|null $where */ public function select(Where|Closure|string|array|null $where = null): ResultSetInterface; /** - * @param array $set + * @param array $set + * @param GatewayWhere $where */ public function update(array $set, Where|Closure|array|string $where): int; } diff --git a/test/unit/RowGateway/AbstractRowGatewayTest.php b/test/unit/RowGateway/AbstractRowGatewayTest.php index df35f323b..e39547ada 100644 --- a/test/unit/RowGateway/AbstractRowGatewayTest.php +++ b/test/unit/RowGateway/AbstractRowGatewayTest.php @@ -47,6 +47,12 @@ #[CoversMethod(RowGateway::class, 'count')] #[CoversMethod(RowGateway::class, 'toArray')] #[CoversMethod(RowGateway::class, 'exchangeArray')] +#[CoversMethod(AbstractRowGateway::class, 'executeStatement')] +#[CoversMethod(AbstractRowGateway::class, 'insertRow')] +#[CoversMethod(AbstractRowGateway::class, 'updateRow')] +#[CoversMethod(AbstractRowGateway::class, 'processInsertedPrimaryKey')] +#[CoversMethod(AbstractRowGateway::class, 'requirePrimaryKeyColumn')] +#[CoversMethod(AbstractRowGateway::class, 'requireSql')] final class AbstractRowGatewayTest extends TestCase { /** @var Adapter&MockObject */ @@ -74,6 +80,29 @@ public function delete(): void static::assertSame(1, $affectedRows); } + #[Test] + public function deleteThrowsWhenTheStatementReturnsNoResult(): void + { + $nullStatement = $this->getMockBuilder(StatementInterface::class)->getMock(); + $nullStatement->method('execute')->willReturn(null); + + $driver = $this->getMockBuilder(DriverInterface::class)->getMock(); + $driver->method('createStatement')->willReturn($nullStatement); + + $adapter = $this->getMockBuilder(Adapter::class) + ->onlyMethods([]) + ->setConstructorArgs([$driver, new TrustingSql92Platform()]) + ->getMock(); + + $rowGateway = new RowGateway('id', 'foo', $adapter); + $rowGateway->populate(['id' => 1], true); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage(RuntimeException::MISSING_STATEMENT_RESULT); + + $rowGateway->delete(); + } + #[Test] public function exchangeArray(): void { @@ -263,6 +292,14 @@ public function offsetGet(): void static::assertSame('test', $this->rowGateway['testColumn']); } + #[Test] + public function offsetGetReturnsNullForAnUnknownColumn(): void + { + $this->rowGateway->populate(['id' => 5], true); + + static::assertNull($this->rowGateway['unknown']); + } + #[Test] public function offsetSet(): void { @@ -366,6 +403,18 @@ public function saveInsertMultiKey(): void static::assertEquals(['one' => 'foo', 'two' => 'bar'], $refRowGatewayProp->getValue($this->rowGateway)); } + #[Test] + public function saveThrowsWhenTheSavedRowCannotBeReadBack(): void + { + $this->mockResult->expects($this->any())->method('current')->willReturn(false); + $this->rowGateway->populate(['id' => 6, 'name' => 'foo'], true); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage(RuntimeException::MISSING_SAVED_ROW); + + $this->rowGateway->save(); + } + #[Test] public function saveUpdate(): void { @@ -375,6 +424,39 @@ public function saveUpdate(): void static::assertSame(6, $this->rowGateway['id']); } + #[Test] + public function saveUpdateChangingOneColumnOfACompositeKeyReselectsByTheNewKey(): void + { + $selectMock = $this->getMockBuilder(Select::class) + ->onlyMethods(['where']) + ->getMock(); + $selectMock->expects($this->once()) + ->method('where') + ->with(static::equalTo(['one' => 1, 'two' => 3])) + ->willReturn($selectMock); + + $sqlMock = $this->getMockBuilder(Sql::class) + ->onlyMethods(['select']) + ->setConstructorArgs([$this->mockAdapter]) + ->getMock(); + $sqlMock->expects($this->any()) + ->method('select') + ->willReturn($selectMock); + + $this->setRowGatewayState(['primaryKeyColumn' => ['one', 'two'], 'sql' => $sqlMock]); + + $this->mockResult + ->expects($this->any()) + ->method('current') + ->willReturn(['one' => 1, 'two' => 3, 'name' => 'foo']); + + $this->rowGateway->populate(['one' => 1, 'two' => 2, 'name' => 'foo'], true); + $this->rowGateway->two = 3; + $this->rowGateway->save(); + + static::assertEquals(['one' => 1, 'two' => 3, 'name' => 'foo'], $this->rowGateway->toArray()); + } + #[Test] public function saveUpdateChangingPrimaryKey(): void { diff --git a/test/unit/RowGateway/Exception/RuntimeExceptionTest.php b/test/unit/RowGateway/Exception/RuntimeExceptionTest.php index e8408c070..8357a838d 100644 --- a/test/unit/RowGateway/Exception/RuntimeExceptionTest.php +++ b/test/unit/RowGateway/Exception/RuntimeExceptionTest.php @@ -17,7 +17,9 @@ #[Group('unit')] #[CoversMethod(RuntimeException::class, 'forMissingPrimaryKeyColumn')] #[CoversMethod(RuntimeException::class, 'forMissingPrimaryKeyData')] +#[CoversMethod(RuntimeException::class, 'forMissingSavedRow')] #[CoversMethod(RuntimeException::class, 'forMissingSqlObject')] +#[CoversMethod(RuntimeException::class, 'forMissingStatementResult')] #[CoversMethod(RuntimeException::class, 'forMissingTable')] #[CoversMethod(RuntimeException::class, 'forUncallableMethod')] final class RuntimeExceptionTest extends TestCase @@ -36,11 +38,21 @@ public static function namedConstructorProvider(): array ['id'], RuntimeException::MISSING_PRIMARY_KEY_DATA, ], + 'missing saved row' => [ + 'forMissingSavedRow', + [], + RuntimeException::MISSING_SAVED_ROW, + ], 'missing sql object' => [ 'forMissingSqlObject', [], RuntimeException::MISSING_SQL_OBJECT, ], + 'missing statement result' => [ + 'forMissingStatementResult', + [], + RuntimeException::MISSING_STATEMENT_RESULT, + ], 'missing table' => [ 'forMissingTable', [], diff --git a/test/unit/RowGateway/RowGatewayTest.php b/test/unit/RowGateway/RowGatewayTest.php index 048198c25..a75a6dc51 100644 --- a/test/unit/RowGateway/RowGatewayTest.php +++ b/test/unit/RowGateway/RowGatewayTest.php @@ -83,7 +83,9 @@ public function constructorWithStringPrimaryKey(): void $tableProp = new ReflectionProperty(RowGateway::class, 'table'); $sqlProp = new ReflectionProperty(RowGateway::class, 'sql'); + $pkProp = new ReflectionProperty(RowGateway::class, 'primaryKeyColumn'); + static::assertSame(['id'], $pkProp->getValue($rowGateway)); static::assertSame('foo', $tableProp->getValue($rowGateway)); static::assertInstanceOf(Sql::class, $sqlProp->getValue($rowGateway)); } diff --git a/test/unit/TableGateway/AbstractTableGatewayTest.php b/test/unit/TableGateway/AbstractTableGatewayTest.php index 568e31ba6..78b548350 100644 --- a/test/unit/TableGateway/AbstractTableGatewayTest.php +++ b/test/unit/TableGateway/AbstractTableGatewayTest.php @@ -21,9 +21,12 @@ use PhpDb\TableGateway\AbstractTableGateway; use PhpDb\TableGateway\Exception\InvalidArgumentException; use PhpDb\TableGateway\Exception\RuntimeException; +use PhpDb\TableGateway\Feature\EventFeatureEventsInterface; use PhpDb\TableGateway\Feature\FeatureSet; use PhpDbTest\TableGateway\Feature\TestAsset\TestTableGatewayFeature; +use PhpDbTest\TableGateway\TestAsset\HookRecordingTableGateway; use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\IgnoreDeprecations; use PHPUnit\Framework\Attributes\RequiresPhp; use PHPUnit\Framework\Attributes\Test; @@ -31,6 +34,7 @@ use PHPUnit\Framework\TestCase; use ReflectionClass; +use function count; use function sprintf; #[IgnoreDeprecations] @@ -60,6 +64,11 @@ #[CoversMethod(AbstractTableGateway::class, 'getColumns')] #[CoversMethod(AbstractTableGateway::class, 'getFeatureSet')] #[CoversMethod(AbstractTableGateway::class, 'initialize')] +#[CoversMethod(AbstractTableGateway::class, 'unaliasTable')] +#[CoversMethod(AbstractTableGateway::class, 'cloneTable')] +#[CoversMethod(AbstractTableGateway::class, 'requireFeatureSet')] +#[CoversMethod(AbstractTableGateway::class, 'requireResultSetPrototype')] +#[CoversMethod(AbstractTableGateway::class, 'requireSql')] final class AbstractTableGatewayTest extends TestCase { protected MockObject&Adapter $mockAdapter; @@ -75,6 +84,104 @@ final class AbstractTableGatewayTest extends TestCase protected MockObject&Update $mockUpdate; protected MockObject&Delete $mockDelete; + /** @return array}> */ + public static function executeHookProvider(): array + { + return [ + 'delete' => ['delete', ['id = 1']], + 'insert' => ['insert', [['foo' => 'bar']]], + 'select' => ['select', []], + 'update' => ['update', [['foo' => 'bar']]], + ]; + } + + /** @return array, list}> */ + public static function featureEventProvider(): array + { + return [ + 'delete' => [ + 'delete', + ['id = 1'], + [ + [EventFeatureEventsInterface::EVENT_PRE_DELETE, 1], + [EventFeatureEventsInterface::EVENT_POST_DELETE, 2], + ], + ], + 'insert' => [ + 'insert', + [['foo' => 'bar']], + [ + [EventFeatureEventsInterface::EVENT_PRE_INSERT, 1], + [EventFeatureEventsInterface::EVENT_POST_INSERT, 2], + ], + ], + 'select' => [ + 'select', + [], + [ + [EventFeatureEventsInterface::EVENT_PRE_SELECT, 1], + [EventFeatureEventsInterface::EVENT_POST_SELECT, 3], + ], + ], + 'update' => [ + 'update', + [['foo' => 'bar']], + [ + [EventFeatureEventsInterface::EVENT_PRE_UPDATE, 1], + [EventFeatureEventsInterface::EVENT_POST_UPDATE, 2], + ], + ], + ]; + } + + /** @return array}> */ + public static function initializingOperationProvider(): array + { + return [ + 'delete' => ['delete', ['id = 1']], + 'deleteWith' => ['deleteWith', [new Delete('foo')]], + 'getFeatureSet' => ['getFeatureSet', []], + 'getResultSetPrototype' => ['getResultSetPrototype', []], + 'getSql' => ['getSql', []], + 'insert' => ['insert', [['foo' => 'bar']]], + 'insertWith' => ['insertWith', [(new Insert('foo'))->values(['foo' => 'bar'])]], + 'select' => ['select', []], + 'selectWith' => ['selectWith', [new Select('foo')]], + 'update' => ['update', [['foo' => 'bar']]], + 'updateWith' => ['updateWith', [new Update('foo')]], + ]; + } + + /** @return array, string}> */ + public static function statementWithoutResultProvider(): array + { + return [ + 'delete' => ['delete', [['id' => 1]], 'Delete'], + 'insert' => ['insert', [['foo' => 'bar']], 'Insert'], + 'select' => ['select', [], 'Select'], + 'update' => ['update', [['foo' => 'bar']], 'Update'], + ]; + } + + /** @return array */ + public static function uninitializedPropertyProvider(): array + { + return [ + 'feature set' => ['featureSet', 'getFeatureSet'], + 'result set prototype' => ['resultSetPrototype', 'getResultSetPrototype'], + 'sql' => ['sql', 'getSql'], + ]; + } + + #[Test] + public function cloneCopiesTheSqlAndResultSetPrototype(): void + { + $cloned = clone $this->table; + + static::assertNotSame($this->table->getSql(), $cloned->getSql()); + static::assertNotSame($this->table->getResultSetPrototype(), $cloned->getResultSetPrototype()); + } + #[Test] public function cloneKeepsTheAdapter(): void { @@ -82,6 +189,19 @@ public function cloneKeepsTheAdapter(): void static::assertSame($this->mockAdapter, $cTable->getAdapter()); } + #[Test] + public function cloneOfAGatewayWithoutDependenciesKeepsThemUnset(): void + { + $gateway = $this->getMockBuilder(AbstractTableGateway::class)->onlyMethods([])->getMock(); + + $cloned = clone $gateway; + + $tgReflection = new ReflectionClass(AbstractTableGateway::class); + static::assertNull($tgReflection->getProperty('sql')->getValue($cloned)); + static::assertNull($tgReflection->getProperty('resultSetPrototype')->getValue($cloned)); + static::assertNull($tgReflection->getProperty('table')->getValue($cloned)); + } + #[Test] public function cloneWithAnAliasedTableIdentifier(): void { @@ -140,7 +260,7 @@ public function deleteWith(): void $delete->expects($this->any()) ->method('getRawState') - ->willReturn(['table' => 'foo']); + ->willReturnMap([['table', 'foo']]); $affectedRows = $this->table->deleteWith($delete); static::assertSame(5, $affectedRows); @@ -151,6 +271,8 @@ public function deleteWithClosure(): void { // The closure receives the Delete object created by $this->sql->delete() // We verify that the closure is called with a Delete instance + $this->mockDelete->expects($this->never())->method('where'); + $closureCalled = false; $affectedRows = $this->table->delete(static function ($delete) use (&$closureCalled) { $closureCalled = true; @@ -225,6 +347,17 @@ public function getAdapter(): void static::assertSame($this->mockAdapter, $this->table->getAdapter()); } + #[Test] + public function getAdapterThrowsWhenNoAdapterIsSet(): void + { + $gateway = $this->getMockBuilder(AbstractTableGateway::class)->onlyMethods([])->getMock(); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage(RuntimeException::MISSING_ADAPTER); + + $gateway->getAdapter(); + } + #[Test] public function getColumns(): void { @@ -266,6 +399,41 @@ public function getTable(): void static::assertSame('foo', $this->table->getTable()); } + #[Test] + public function getTableThrowsWhenNoTableIsSet(): void + { + $gateway = $this->getMockBuilder(AbstractTableGateway::class)->onlyMethods([])->getMock(); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage(RuntimeException::MISSING_TABLE); + + $gateway->getTable(); + } + + #[Test] + public function initializeAppliesItsFeatureEventsOnlyOnce(): void + { + $events = []; + $featureSet = $this->getMockBuilder(FeatureSet::class)->onlyMethods(['apply'])->getMock(); + $featureSet->method('apply') + ->willReturnCallback( + static function (string $event) use (&$events): void { + $events[] = $event; + }, + ); + + $tgReflection = new ReflectionClass(AbstractTableGateway::class); + $tgReflection->getProperty('featureSet')->setValue($this->table, $featureSet); + + $this->table->initialize(); + $this->table->initialize(); + + static::assertSame( + [EventFeatureEventsInterface::EVENT_PRE_INITIALIZE, EventFeatureEventsInterface::EVENT_POST_INITIALIZE], + $events, + ); + } + #[Test] public function initializeBuildsAResultSet(): void { @@ -553,6 +721,62 @@ public function magicSetThrowsForUnknownProperty(): void $this->table->invalidProperty = 'value'; } + /** + * @param list $arguments + * @param list $expectedEvents + */ + #[Test] + #[DataProvider('featureEventProvider')] + public function operationsApplyTheirPreAndPostFeatureEvents( + string $method, + array $arguments, + array $expectedEvents, + ): void { + $events = []; + $featureSet = $this->getMockBuilder(FeatureSet::class)->onlyMethods(['apply'])->getMock(); + $featureSet->method('apply') + ->willReturnCallback( + static function (string $event, array $args) use (&$events): void { + $events[] = [$event, count($args)]; + }, + ); + + $tgReflection = new ReflectionClass(AbstractTableGateway::class); + $tgReflection->getProperty('featureSet')->setValue($this->table, $featureSet); + $tgReflection->getProperty('isInitialized')->setValue($this->table, true); + + $this->table->{$method}(...$arguments); + + static::assertSame($expectedEvents, $events); + } + + /** @param list $arguments */ + #[Test] + #[DataProvider('initializingOperationProvider')] + public function operationsInitializeTheGatewayFirst(string $method, array $arguments): void + { + $gateway = $this->getMockBuilder(AbstractTableGateway::class)->onlyMethods([])->getMock(); + $tgReflection = new ReflectionClass(AbstractTableGateway::class); + $tgReflection->getProperty('table')->setValue($gateway, 'foo'); + $tgReflection->getProperty('adapter')->setValue($gateway, $this->mockAdapter); + + $gateway->{$method}(...$arguments); + + static::assertTrue($gateway->isInitialized()); + } + + /** @param list $arguments */ + #[Test] + #[DataProvider('executeHookProvider')] + public function operationsRunThroughTheOverridableExecuteHooks(string $method, array $arguments): void + { + $gateway = new HookRecordingTableGateway('foo', $this->mockAdapter); + + $gateway->{$method}(...$arguments); + + static::assertSame([$method], $gateway->executed); + } + #[Test] public function selectAppliesColumnsWhenStarSelected(): void { @@ -568,9 +792,9 @@ public function selectAppliesColumnsWhenStarSelected(): void $select->expects($this->any()) ->method('getRawState') - ->willReturn([ - 'table' => 'foo', - 'columns' => [Select::SQL_STAR], + ->willReturnMap([ + [Select::TABLE, 'foo'], + [Select::COLUMNS, [Select::SQL_STAR]], ]); $select->expects($this->once()) @@ -580,27 +804,56 @@ public function selectAppliesColumnsWhenStarSelected(): void $this->table->selectWith($select); } + #[Test] + public function selectInitializesTheResultSetWithTheStatementResult(): void + { + $resultSet = $this->table->select(); + + static::assertInstanceOf(ResultSet::class, $resultSet); + static::assertInstanceOf(ResultInterface::class, $resultSet->getDataSource()); + } + + #[Test] + public function selectKeepsExplicitlySelectedColumns(): void + { + $tgReflection = new ReflectionClass(AbstractTableGateway::class); + $tgReflection->getProperty('columns')->setValue($this->table, ['id', 'name', 'email']); + + $select = $this->getMockBuilder(Select::class) + ->onlyMethods(['getRawState', 'columns']) + ->setConstructorArgs(['foo']) + ->getMock(); + $select->method('getRawState') + ->willReturnMap([ + [Select::TABLE, 'foo'], + [Select::COLUMNS, ['id']], + ]); + $select->expects($this->never())->method('columns'); + + $this->table->selectWith($select); + } + #[Test] public function selectWithArrayTable(): void { // Case 1 $select1 = $this->getMockBuilder(Select::class)->onlyMethods(['getRawState'])->getMock(); - $select1->expects($this->once()) + $select1->expects($this->exactly(2)) ->method('getRawState') - ->willReturn([ - 'table' => 'foo', // Standard table name format, valid according to Select::from() - 'columns' => null, + ->willReturnMap([ + [Select::TABLE, 'foo'], // Standard table name format, valid according to Select::from() + [Select::COLUMNS, []], ]); $return = $this->table->selectWith($select1); static::assertInstanceOf(ResultSet::class, $return); // Case 2 $select1 = $this->getMockBuilder(Select::class)->onlyMethods(['getRawState'])->getMock(); - $select1->expects($this->once()) + $select1->expects($this->exactly(2)) ->method('getRawState') - ->willReturn([ - 'table' => ['f' => 'foo'], // Alias table name format, valid according to Select::from() - 'columns' => null, + ->willReturnMap([ + [Select::TABLE, ['f' => 'foo']], // Alias table name format, valid according to Select::from() + [Select::COLUMNS, []], ]); $return = $this->table->selectWith($select1); static::assertInstanceOf(ResultSet::class, $return); @@ -612,11 +865,13 @@ public function selectWithClosure(): void $mockSelect = $this->mockSelect; $mockSelect->expects($this->any()) ->method('getRawState') - ->willReturn([ - 'table' => $this->table->getTable(), - 'columns' => [], + ->willReturnMap([ + [Select::TABLE, $this->table->getTable()], + [Select::COLUMNS, []], ]); + $mockSelect->expects($this->never())->method('where'); + $closureCalled = false; $result = $this->table->select(static function ($select) use (&$closureCalled) { $closureCalled = true; @@ -643,9 +898,9 @@ public function selectWithWhereString(): void $mockSelect = $this->mockSelect; $mockSelect->expects($this->any()) ->method('getRawState') - ->willReturn([ - 'table' => $this->table->getTable(), - 'columns' => [], + ->willReturnMap([ + [Select::TABLE, $this->table->getTable()], + [Select::COLUMNS, []], ]); // assert select::from() is called @@ -656,6 +911,50 @@ public function selectWithWhereString(): void $this->table->select('foo'); } + #[Test] + #[DataProvider('uninitializedPropertyProvider')] + public function throwsWhenAnInitializedDependencyIsUnset(string $property, string $getter): void + { + $tgReflection = new ReflectionClass(AbstractTableGateway::class); + $tgReflection->getProperty('isInitialized')->setValue($this->table, true); + $tgReflection->getProperty($property)->setValue($this->table, null); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage(sprintf(RuntimeException::UNINITIALIZED_PROPERTY, $property)); + + $this->table->{$getter}(); + } + + /** @param list $arguments */ + #[Test] + #[DataProvider('statementWithoutResultProvider')] + public function throwsWhenTheStatementReturnsNoResult(string $method, array $arguments, string $statement): void + { + $nullStatement = $this->getMockBuilder(StatementInterface::class)->getMock(); + $nullStatement->method('execute')->willReturn(null); + + $driver = $this->getMockBuilder(DriverInterface::class)->getMock(); + $driver->method('createStatement')->willReturn($nullStatement); + + $platform = $this->getMockBuilder(PlatformInterface::class)->getMock(); + $platform->method('getSqlPlatformDecorator')->willReturn(new Sql\Platform\Platform($platform)); + + $adapter = $this->getMockBuilder(Adapter::class) + ->onlyMethods([]) + ->setConstructorArgs([$driver, $platform, $this->getMockBuilder(ResultSetInterface::class)->getMock()]) + ->getMock(); + + $gateway = $this->getMockBuilder(AbstractTableGateway::class)->onlyMethods([])->getMock(); + $tgReflection = new ReflectionClass(AbstractTableGateway::class); + $tgReflection->getProperty('table')->setValue($gateway, 'foo'); + $tgReflection->getProperty('adapter')->setValue($gateway, $adapter); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage(sprintf(RuntimeException::MISSING_STATEMENT_RESULT, $statement)); + + $gateway->{$method}(...$arguments); + } + #[Test] public function update(): void { @@ -680,7 +979,7 @@ public function updateWith(): void $update->expects($this->any()) ->method('getRawState') - ->willReturn(['table' => 'foo']); + ->willReturnMap([['table', 'foo']]); $affectedRows = $this->table->updateWith($update); static::assertSame(5, $affectedRows); diff --git a/test/unit/TableGateway/Exception/RuntimeExceptionTest.php b/test/unit/TableGateway/Exception/RuntimeExceptionTest.php index bb3b2384b..717bf266d 100644 --- a/test/unit/TableGateway/Exception/RuntimeExceptionTest.php +++ b/test/unit/TableGateway/Exception/RuntimeExceptionTest.php @@ -25,12 +25,14 @@ #[CoversMethod(RuntimeException::class, 'forMissingPrimaryKeyInMetadata')] #[CoversMethod(RuntimeException::class, 'forMissingSequenceResult')] #[CoversMethod(RuntimeException::class, 'forMissingSqlInstance')] +#[CoversMethod(RuntimeException::class, 'forMissingStatementResult')] #[CoversMethod(RuntimeException::class, 'forMissingStaticAdapter')] #[CoversMethod(RuntimeException::class, 'forMissingTable')] #[CoversMethod(RuntimeException::class, 'forNonArrayInsertData')] #[CoversMethod(RuntimeException::class, 'forNonArrayMetadata')] #[CoversMethod(RuntimeException::class, 'forTableMismatch')] #[CoversMethod(RuntimeException::class, 'forUnexpectedResultSet')] +#[CoversMethod(RuntimeException::class, 'forUninitializedProperty')] #[CoversMethod(RuntimeException::class, 'forUnnamedTableInMetadata')] #[CoversMethod(RuntimeException::class, 'forUnnamedTableInRowGateway')] #[CoversMethod(RuntimeException::class, 'forUnsupportedLastSequencePlatform')] @@ -82,6 +84,11 @@ public static function namedConstructorProvider(): array [], RuntimeException::MISSING_SQL_INSTANCE, ], + 'missing statement result' => [ + 'forMissingStatementResult', + ['Select'], + RuntimeException::MISSING_STATEMENT_RESULT, + ], 'missing static adapter' => [ 'forMissingStaticAdapter', [], @@ -112,6 +119,11 @@ public static function namedConstructorProvider(): array [RowGatewayFeature::class, RowPrototypeResultSet::class], RuntimeException::UNEXPECTED_RESULT_SET, ], + 'uninitialized property' => [ + 'forUninitializedProperty', + ['sql'], + RuntimeException::UNINITIALIZED_PROPERTY, + ], 'unnamed table in metadata' => [ 'forUnnamedTableInMetadata', [], diff --git a/test/unit/TableGateway/Feature/RowGatewayFeatureTest.php b/test/unit/TableGateway/Feature/RowGatewayFeatureTest.php index a122e05ce..5f2d7bc1b 100644 --- a/test/unit/TableGateway/Feature/RowGatewayFeatureTest.php +++ b/test/unit/TableGateway/Feature/RowGatewayFeatureTest.php @@ -153,6 +153,23 @@ public function postInitializeThrowsExceptionWhenNoMetadataAndNoPrimaryKey(): vo $feature->postInitialize(); } + #[Test] + public function postInitializeThrowsWhenAMetadataPrimaryKeyColumnIsNotAString(): void + { + $resultSet = $this->createInitialResultSet(); + + $feature = new RowGatewayFeature(); + $feature->setTableGateway($this->createTableGatewayMock( + $resultSet, + $this->createMetadataFeatureSet(['primaryKey' => ['id', 42]]), + )); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage(RuntimeException::UNUSABLE_PRIMARY_KEY); + + $feature->postInitialize(); + } + #[Test] public function postInitializeThrowsWhenMetadataIsNotAnArray(): void { @@ -249,6 +266,27 @@ public function postInitializeThrowsWhenTableIsNotNamed(): void $feature->postInitialize(); } + #[Test] + public function postInitializeWithACompositeMetadataPrimaryKey(): void + { + $resultSet = $this->createInitialResultSet(); + + $feature = new RowGatewayFeature(); + $feature->setTableGateway($this->createTableGatewayMock( + $resultSet, + $this->createMetadataFeatureSet(['primaryKey' => ['first' => 'one', 'second' => 'two']]), + )); + + $feature->postInitialize(); + + $rowPrototype = $resultSet->getRowPrototype(); + static::assertInstanceOf(RowGateway::class, $rowPrototype); + static::assertSame( + ['one', 'two'], + (new ReflectionProperty(RowGateway::class, 'primaryKeyColumn'))->getValue($rowPrototype), + ); + } + #[Test] public function postInitializeWithMetadataFeature(): void { @@ -321,6 +359,25 @@ private function createInitialResultSet(): RowPrototypeResultSet return new RowPrototypeResultSet($this->createMock(RowGatewayInterface::class)); } + /** + * @param array $metadata + */ + private function createMetadataFeatureSet(array $metadata): FeatureSet + { + $metadataFeature = $this->getMockBuilder(MetadataFeature::class) + ->disableOriginalConstructor() + ->getMock(); + + (new ReflectionProperty(MetadataFeature::class, 'sharedData'))->setValue($metadataFeature, [ + 'metadata' => $metadata, + ]); + + $featureSet = $this->createStub(FeatureSet::class); + $featureSet->method('getFeatureByClassName')->willReturn($metadataFeature); + + return $featureSet; + } + private function createTableGatewayMock( ResultSetInterface $resultSetPrototype, ?FeatureSet $featureSet = null, diff --git a/test/unit/TableGateway/TestAsset/HookRecordingTableGateway.php b/test/unit/TableGateway/TestAsset/HookRecordingTableGateway.php new file mode 100644 index 000000000..84392a066 --- /dev/null +++ b/test/unit/TableGateway/TestAsset/HookRecordingTableGateway.php @@ -0,0 +1,61 @@ + */ + public array $executed = []; + + public function __construct(string $table, AdapterInterface $adapter) + { + $this->table = $table; + $this->adapter = $adapter; + } + + #[Override] + protected function executeDelete(Delete $delete): int + { + $this->executed[] = 'delete'; + + return parent::executeDelete($delete); + } + + #[Override] + protected function executeInsert(Insert $insert): int + { + $this->executed[] = 'insert'; + + return parent::executeInsert($insert); + } + + #[Override] + protected function executeSelect(Select $select): ResultSetInterface + { + $this->executed[] = 'select'; + + return parent::executeSelect($select); + } + + #[Override] + protected function executeUpdate(Update $update): int + { + $this->executed[] = 'update'; + + return parent::executeUpdate($update); + } +} From e9d25c03e22f9779077929446ffa3f0de5f30bd5 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 12 Oct 2026 16:09:01 +1100 Subject: [PATCH 02/10] Reinstate RowGateway FeatureSet's $magicSpecifications and tag it @api The protected property is restored to match its TableGateway twin, so subclasses that read it keep working. The class was the only one in src tagged @final, which made the analyzer treat the protected property as private and report it unused. It is tagged @api instead, like the TableGateway FeatureSet and every other gateway class. --- src/RowGateway/Feature/FeatureSet.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/RowGateway/Feature/FeatureSet.php b/src/RowGateway/Feature/FeatureSet.php index b85ae794f..7c2cb9fe2 100644 --- a/src/RowGateway/Feature/FeatureSet.php +++ b/src/RowGateway/Feature/FeatureSet.php @@ -9,7 +9,7 @@ use function method_exists; /** - * @final + * @api */ class FeatureSet { @@ -20,6 +20,9 @@ class FeatureSet /** @var FeatureInterface[] */ protected array $features = []; + /** @var array */ + protected array $magicSpecifications = []; + /** @param FeatureInterface[] $features */ public function __construct(array $features = []) { From b233546ddc9b376dec4892b9fe44b1f704ca5970 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 12 Oct 2026 16:12:05 +1100 Subject: [PATCH 03/10] Stop swapping aliased tables during gateway execution executeDelete(), executeInsert() and executeUpdate() replaced an aliased table on the statement with the bare table before preparing it, then put the alias back afterwards. Delete, Insert and Update now drop the alias themselves when they render, through AbstractPreparableSql::unaliasTable(), so the swap no longer changes the SQL. It also left the caller's statement holding the bare table if preparing or executing threw, because the restore was not in a finally block. The swap and the private unaliasTable() helper are removed, so the gateway no longer modifies the statement it is given. The three *ShouldResetTableToUnaliasedTable tests, which asserted the swapped state, now assert that the statement keeps its alias. dataChangingCallOnAliasedTableRendersTheBareTable still covers the rendered SQL. --- src/TableGateway/AbstractTableGateway.php | 44 --------------------- test/unit/TableGateway/TableGatewayTest.php | 44 +++++++++------------ 2 files changed, 19 insertions(+), 69 deletions(-) diff --git a/src/TableGateway/AbstractTableGateway.php b/src/TableGateway/AbstractTableGateway.php index a0842496f..0a210d7c1 100644 --- a/src/TableGateway/AbstractTableGateway.php +++ b/src/TableGateway/AbstractTableGateway.php @@ -23,7 +23,6 @@ use PhpDb\TableGateway\Feature\EventFeatureEventsInterface; use function array_map; -use function array_values; use function end; use function in_array; use function is_array; @@ -84,20 +83,6 @@ private static function cloneTable(TableIdentifier|string|array|null $table): Ta ); } - /** - * Returns the bare table of an aliased table array, or null when the table is not aliased. - * - * @param GatewayTable $table - */ - private static function unaliasTable(TableIdentifier|string|array $table): TableIdentifier|string|null - { - if (! is_array($table)) { - return null; - } - - return array_values($table)[0] ?? null; - } - /** * @param GatewayWhere $where * @@ -355,22 +340,12 @@ protected function executeDelete(Delete $delete): int // pre delete update $featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_DELETE, [$delete]); - $unaliasedTable = self::unaliasTable($table); - if (null !== $unaliasedTable) { - $delete->from($unaliasedTable); - } - $statement = $this->requireSql()->prepareStatementForSqlObject($delete); $result = $statement->execute() ?? throw Exception\RuntimeException::forMissingStatementResult('Delete'); // apply postDelete features $featureSet->apply(EventFeatureEventsInterface::EVENT_POST_DELETE, [$statement, $result]); - // Reset original table information in Delete instance, if necessary - if (null !== $unaliasedTable) { - $delete->from($table); - } - return $result->getAffectedRows(); } @@ -393,10 +368,6 @@ protected function executeInsert(Insert $insert): int // Most RDBMS solutions do not allow using table aliases in INSERTs // See https://github.com/zendframework/zf2/issues/7311 - $unaliasedTable = self::unaliasTable($table); - if (null !== $unaliasedTable) { - $insert->into($unaliasedTable); - } $statement = $this->requireSql()->prepareStatementForSqlObject($insert); $result = $statement->execute() @@ -406,11 +377,6 @@ protected function executeInsert(Insert $insert): int // apply postInsert features $featureSet->apply(EventFeatureEventsInterface::EVENT_POST_INSERT, [$statement, $result]); - // Reset original table information in Insert instance, if necessary - if (null !== $unaliasedTable) { - $insert->into($table); - } - return $result->getAffectedRows(); } @@ -465,22 +431,12 @@ protected function executeUpdate(Update $update): int // apply preUpdate features $featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_UPDATE, [$update]); - $unaliasedTable = self::unaliasTable($table); - if (null !== $unaliasedTable) { - $update->table($unaliasedTable); - } - $statement = $this->requireSql()->prepareStatementForSqlObject($update); $result = $statement->execute() ?? throw Exception\RuntimeException::forMissingStatementResult('Update'); // apply postUpdate features $featureSet->apply(EventFeatureEventsInterface::EVENT_POST_UPDATE, [$statement, $result]); - // Reset original table information in Update instance, if necessary - if (null !== $unaliasedTable) { - $update->table($table); - } - return $result->getAffectedRows(); } diff --git a/test/unit/TableGateway/TableGatewayTest.php b/test/unit/TableGateway/TableGatewayTest.php index fc9187d29..b639aedea 100644 --- a/test/unit/TableGateway/TableGatewayTest.php +++ b/test/unit/TableGateway/TableGatewayTest.php @@ -41,16 +41,16 @@ final class TableGatewayTest extends TestCase /** * @psalm-return array{ - * 'identifier-alias': list{array{U: TableIdentifier}, TableIdentifier}, - * 'simple-alias': list{array{U: string}, string} + * 'identifier-alias': list{array{U: TableIdentifier}}, + * 'simple-alias': list{array{U: string}} * } */ public static function aliasedTables(): array { $identifier = new TableIdentifier('Users'); return [ - 'simple-alias' => [['U' => 'Users'], 'Users'], - 'identifier-alias' => [['U' => $identifier], $identifier], + 'simple-alias' => [['U' => 'Users']], + 'identifier-alias' => [['U' => $identifier]], ]; } @@ -225,14 +225,12 @@ static function () use (&$executed, &$sql, $result): ResultInterface { } /** - * @param AliasedTable $tableValue + * @param AliasedTable $tableValue */ #[Test] #[DataProvider('aliasedTables')] - public function deleteShouldResetTableToUnaliasedTable( - array $tableValue, - string|TableIdentifier $expected, - ): void { + public function deleteKeepsAliasedTableOnStatement(array $tableValue): void + { $delete = new Delete(); $delete->from($tableValue); @@ -248,10 +246,10 @@ public function deleteShouldResetTableToUnaliasedTable( ->method('execute') ->willReturn($result); - $statementExpectation = function (Delete $delete) use ($expected, $statement): MockObject&StatementInterface { + $statementExpectation = function (Delete $delete) use ($tableValue, $statement): MockObject&StatementInterface { $state = $delete->getRawState(); $this->assertIsArray($state); - $this->assertSame($expected, $state['table']); + $this->assertSame($tableValue, $state['table']); return $statement; }; @@ -292,15 +290,13 @@ public function deleteShouldResetTableToUnaliasedTable( } /** - * @param AliasedTable $tableValue + * @param AliasedTable $tableValue */ #[Test] #[DataProvider('aliasedTables')] #[Group('7311')] - public function insertShouldResetTableToUnaliasedTable( - array $tableValue, - string|TableIdentifier $expected, - ): void { + public function insertKeepsAliasedTableOnStatement(array $tableValue): void + { $insert = new Insert(); $insert->into($tableValue); @@ -316,10 +312,10 @@ public function insertShouldResetTableToUnaliasedTable( ->method('execute') ->willReturn($result); - $statementExpectation = function (Insert $insert) use ($expected, $statement): MockObject&StatementInterface { + $statementExpectation = function (Insert $insert) use ($tableValue, $statement): MockObject&StatementInterface { $state = $insert->getRawState(); $this->assertIsArray($state); - self::assertSame($expected, $state['table']); + self::assertSame($tableValue, $state['table']); return $statement; }; @@ -407,14 +403,12 @@ public function tableAsTableIdentifierObject(): void } /** - * @param AliasedTable $tableValue + * @param AliasedTable $tableValue */ #[Test] #[DataProvider('aliasedTables')] - public function updateShouldResetTableToUnaliasedTable( - array $tableValue, - string|TableIdentifier $expected, - ): void { + public function updateKeepsAliasedTableOnStatement(array $tableValue): void + { $update = new Update(); $update->table($tableValue); @@ -430,10 +424,10 @@ public function updateShouldResetTableToUnaliasedTable( ->method('execute') ->willReturn($result); - $statementExpectation = function (Update $update) use ($expected, $statement): MockObject&StatementInterface { + $statementExpectation = function (Update $update) use ($tableValue, $statement): MockObject&StatementInterface { $state = $update->getRawState(); $this->assertIsArray($state); - $this->assertSame($expected, $state['table']); + $this->assertSame($tableValue, $state['table']); return $statement; }; From 36a8cee730c7c0e6aeddcb8ec835a80242578532 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 12 Oct 2026 16:14:38 +1100 Subject: [PATCH 04/10] Define the gateway table and where types on TableGatewayInterface GatewayTable and GatewayWhere describe the TableGatewayInterface contract, so the interface defines them. AbstractTableGateway and TableGateway import them from the interface, and GatewayJoin, used only by AbstractTableGateway, stays there. --- src/TableGateway/AbstractTableGateway.php | 6 ++---- src/TableGateway/TableGateway.php | 2 +- src/TableGateway/TableGatewayInterface.php | 6 ++++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/TableGateway/AbstractTableGateway.php b/src/TableGateway/AbstractTableGateway.php index 0a210d7c1..6ef596cf9 100644 --- a/src/TableGateway/AbstractTableGateway.php +++ b/src/TableGateway/AbstractTableGateway.php @@ -14,7 +14,6 @@ use PhpDb\Sql\Exception\RuntimeException as SqlRuntimeException; use PhpDb\Sql\Insert; use PhpDb\Sql\Join; -use PhpDb\Sql\Predicate\PredicateSet; use PhpDb\Sql\Select; use PhpDb\Sql\Sql; use PhpDb\Sql\TableIdentifier; @@ -32,9 +31,8 @@ * @psalm-import-type ColumnValue from AbstractSql * @psalm-import-type ColumnList from Select * @psalm-import-type JoinName from Join - * @psalm-import-type PredicateValue from PredicateSet - * @psalm-type GatewayTable = TableIdentifier|string|array - * @psalm-type GatewayWhere = Where|Closure|string|array + * @psalm-import-type GatewayTable from TableGatewayInterface + * @psalm-import-type GatewayWhere from TableGatewayInterface * @psalm-type GatewayJoin = array{name: JoinName, on: string, type?: string} * @property AdapterInterface|null $adapter * @property string|int|false|null $lastInsertValue diff --git a/src/TableGateway/TableGateway.php b/src/TableGateway/TableGateway.php index 7d04b9f19..4fe4261ce 100644 --- a/src/TableGateway/TableGateway.php +++ b/src/TableGateway/TableGateway.php @@ -14,7 +14,7 @@ /** * @api - * @psalm-import-type GatewayTable from AbstractTableGateway + * @psalm-import-type GatewayTable from TableGatewayInterface */ class TableGateway extends AbstractTableGateway { diff --git a/src/TableGateway/TableGatewayInterface.php b/src/TableGateway/TableGatewayInterface.php index 49d7ce353..050ffcd35 100644 --- a/src/TableGateway/TableGatewayInterface.php +++ b/src/TableGateway/TableGatewayInterface.php @@ -7,14 +7,16 @@ use Closure; use PhpDb\ResultSet\ResultSetInterface; use PhpDb\Sql\AbstractSql; +use PhpDb\Sql\Predicate\PredicateSet; use PhpDb\Sql\TableIdentifier; use PhpDb\Sql\Where; /** * @api * @psalm-import-type ColumnValue from AbstractSql - * @psalm-import-type GatewayTable from AbstractTableGateway - * @psalm-import-type GatewayWhere from AbstractTableGateway + * @psalm-import-type PredicateValue from PredicateSet + * @psalm-type GatewayTable = TableIdentifier|string|array + * @psalm-type GatewayWhere = Where|Closure|string|array */ interface TableGatewayInterface { From c200dce3c49e3cde052a3ecb7861a8b40a510d02 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 12 Oct 2026 16:19:02 +1100 Subject: [PATCH 05/10] Drop the coverage target for the removed AbstractTableGateway::unaliasTable() PHPUnit reports a CoversMethod target that no longer exists as a warning, which fails the coverage run. --- test/unit/RowGateway/AbstractRowGatewayTest.php | 6 ------ test/unit/TableGateway/AbstractTableGatewayTest.php | 5 ----- 2 files changed, 11 deletions(-) diff --git a/test/unit/RowGateway/AbstractRowGatewayTest.php b/test/unit/RowGateway/AbstractRowGatewayTest.php index e39547ada..80a796bb8 100644 --- a/test/unit/RowGateway/AbstractRowGatewayTest.php +++ b/test/unit/RowGateway/AbstractRowGatewayTest.php @@ -47,12 +47,6 @@ #[CoversMethod(RowGateway::class, 'count')] #[CoversMethod(RowGateway::class, 'toArray')] #[CoversMethod(RowGateway::class, 'exchangeArray')] -#[CoversMethod(AbstractRowGateway::class, 'executeStatement')] -#[CoversMethod(AbstractRowGateway::class, 'insertRow')] -#[CoversMethod(AbstractRowGateway::class, 'updateRow')] -#[CoversMethod(AbstractRowGateway::class, 'processInsertedPrimaryKey')] -#[CoversMethod(AbstractRowGateway::class, 'requirePrimaryKeyColumn')] -#[CoversMethod(AbstractRowGateway::class, 'requireSql')] final class AbstractRowGatewayTest extends TestCase { /** @var Adapter&MockObject */ diff --git a/test/unit/TableGateway/AbstractTableGatewayTest.php b/test/unit/TableGateway/AbstractTableGatewayTest.php index 78b548350..6252c8e6f 100644 --- a/test/unit/TableGateway/AbstractTableGatewayTest.php +++ b/test/unit/TableGateway/AbstractTableGatewayTest.php @@ -64,11 +64,6 @@ #[CoversMethod(AbstractTableGateway::class, 'getColumns')] #[CoversMethod(AbstractTableGateway::class, 'getFeatureSet')] #[CoversMethod(AbstractTableGateway::class, 'initialize')] -#[CoversMethod(AbstractTableGateway::class, 'unaliasTable')] -#[CoversMethod(AbstractTableGateway::class, 'cloneTable')] -#[CoversMethod(AbstractTableGateway::class, 'requireFeatureSet')] -#[CoversMethod(AbstractTableGateway::class, 'requireResultSetPrototype')] -#[CoversMethod(AbstractTableGateway::class, 'requireSql')] final class AbstractTableGatewayTest extends TestCase { protected MockObject&Adapter $mockAdapter; From f2af46367dedc979b0041e1d9b7187af6acbd178 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Mon, 12 Oct 2026 16:21:09 +1100 Subject: [PATCH 06/10] Cover the abstract gateways by class rather than by method PHPUnit on PHP 8.4 rejects the private static unaliasTable() as a CoversMethod target, which failed the coverage run. CoversClass also attributes the private helpers without naming each one. --- .../RowGateway/AbstractRowGatewayTest.php | 19 ++----------- .../TableGateway/AbstractTableGatewayTest.php | 28 ++----------------- 2 files changed, 4 insertions(+), 43 deletions(-) diff --git a/test/unit/RowGateway/AbstractRowGatewayTest.php b/test/unit/RowGateway/AbstractRowGatewayTest.php index 80a796bb8..e76ef6046 100644 --- a/test/unit/RowGateway/AbstractRowGatewayTest.php +++ b/test/unit/RowGateway/AbstractRowGatewayTest.php @@ -18,7 +18,7 @@ use PhpDb\Sql\Select; use PhpDb\Sql\Sql; use PhpDbTest\TestAsset\TrustingSql92Platform; -use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\IgnoreDeprecations; use PHPUnit\Framework\Attributes\RequiresPhp; use PHPUnit\Framework\Attributes\Test; @@ -31,22 +31,7 @@ use function sprintf; #[IgnoreDeprecations] -#[CoversMethod(RowGateway::class, 'offsetSet')] -#[CoversMethod(RowGateway::class, '__set')] -#[CoversMethod(RowGateway::class, '__isset')] -#[CoversMethod(RowGateway::class, 'offsetExists')] -#[CoversMethod(RowGateway::class, '__unset')] -#[CoversMethod(RowGateway::class, 'offsetUnset')] -#[CoversMethod(RowGateway::class, 'offsetGet')] -#[CoversMethod(RowGateway::class, '__get')] -#[CoversMethod(RowGateway::class, 'save')] -#[CoversMethod(RowGateway::class, 'delete')] -#[CoversMethod(RowGateway::class, 'populate')] -#[CoversMethod(RowGateway::class, 'rowExistsInDatabase')] -#[CoversMethod(RowGateway::class, 'processPrimaryKeyData')] -#[CoversMethod(RowGateway::class, 'count')] -#[CoversMethod(RowGateway::class, 'toArray')] -#[CoversMethod(RowGateway::class, 'exchangeArray')] +#[CoversClass(AbstractRowGateway::class)] final class AbstractRowGatewayTest extends TestCase { /** @var Adapter&MockObject */ diff --git a/test/unit/TableGateway/AbstractTableGatewayTest.php b/test/unit/TableGateway/AbstractTableGatewayTest.php index 6252c8e6f..45fe362da 100644 --- a/test/unit/TableGateway/AbstractTableGatewayTest.php +++ b/test/unit/TableGateway/AbstractTableGatewayTest.php @@ -25,7 +25,7 @@ use PhpDb\TableGateway\Feature\FeatureSet; use PhpDbTest\TableGateway\Feature\TestAsset\TestTableGatewayFeature; use PhpDbTest\TableGateway\TestAsset\HookRecordingTableGateway; -use PHPUnit\Framework\Attributes\CoversMethod; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\IgnoreDeprecations; use PHPUnit\Framework\Attributes\RequiresPhp; @@ -39,31 +39,7 @@ #[IgnoreDeprecations] #[RequiresPhp('<= 8.6')] -#[CoversMethod(AbstractTableGateway::class, 'getTable')] -#[CoversMethod(AbstractTableGateway::class, 'getAdapter')] -#[CoversMethod(AbstractTableGateway::class, 'getSql')] -#[CoversMethod(AbstractTableGateway::class, 'getResultSetPrototype')] -#[CoversMethod(AbstractTableGateway::class, 'select')] -#[CoversMethod(AbstractTableGateway::class, 'selectWith')] -#[CoversMethod(AbstractTableGateway::class, 'executeSelect')] -#[CoversMethod(AbstractTableGateway::class, 'insert')] -#[CoversMethod(AbstractTableGateway::class, 'insertWith')] -#[CoversMethod(AbstractTableGateway::class, 'executeInsert')] -#[CoversMethod(AbstractTableGateway::class, 'update')] -#[CoversMethod(AbstractTableGateway::class, 'updateWith')] -#[CoversMethod(AbstractTableGateway::class, 'executeUpdate')] -#[CoversMethod(AbstractTableGateway::class, 'delete')] -#[CoversMethod(AbstractTableGateway::class, 'deleteWith')] -#[CoversMethod(AbstractTableGateway::class, 'executeDelete')] -#[CoversMethod(AbstractTableGateway::class, 'getLastInsertValue')] -#[CoversMethod(AbstractTableGateway::class, '__get')] -#[CoversMethod(AbstractTableGateway::class, '__set')] -#[CoversMethod(AbstractTableGateway::class, '__call')] -#[CoversMethod(AbstractTableGateway::class, '__clone')] -#[CoversMethod(AbstractTableGateway::class, 'isInitialized')] -#[CoversMethod(AbstractTableGateway::class, 'getColumns')] -#[CoversMethod(AbstractTableGateway::class, 'getFeatureSet')] -#[CoversMethod(AbstractTableGateway::class, 'initialize')] +#[CoversClass(AbstractTableGateway::class)] final class AbstractTableGatewayTest extends TestCase { protected MockObject&Adapter $mockAdapter; From 608d9bba812524b722e2cdc53a7849d0c390f88a Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 4 Nov 2026 14:50:39 +1100 Subject: [PATCH 07/10] Create gateway feature sets only when features are configured A table or row gateway with no features no longer creates an empty FeatureSet. Feature dispatch uses the nullsafe operator, so a plain gateway skips it entirely, and requireFeatureSet() is removed. - TableGateway's $features now defaults to null. - getFeatureSet() creates an empty FeatureSet on first use, already bound to the gateway, so features added through it work as before. - The magic methods report the invalid member when there is no feature set, instead of an uninitialised-property error. In a subclass, $this->featureSet is null until a feature set is configured. Constructing a gateway is about 37% faster and 87 bytes (table) or 123 bytes (row) smaller; operations drop two empty FeatureSet::apply() calls each. --- docs/book/table-gateway.md | 7 ++ src/RowGateway/AbstractRowGateway.php | 8 +- src/TableGateway/AbstractTableGateway.php | 55 +++------ src/TableGateway/TableGateway.php | 2 +- .../RowGateway/AbstractRowGatewayTest.php | 55 ++++++--- .../TableGateway/AbstractTableGatewayTest.php | 115 +++++++++++++++--- 6 files changed, 168 insertions(+), 74 deletions(-) diff --git a/docs/book/table-gateway.md b/docs/book/table-gateway.md index 0e94e6a02..555a509d4 100644 --- a/docs/book/table-gateway.md +++ b/docs/book/table-gateway.md @@ -153,6 +153,13 @@ constructor. The constructor can take features in 3 different forms: - as a `FeatureSet` instance - as an array of `Feature` instances +A gateway given no features does not create a `FeatureSet`, so its operations +skip feature dispatch entirely. `getFeatureSet()` creates an empty one, bound +to the gateway, the first time it is called; a feature added to it then works +like any other, except that it does not receive the initialization events, +which have already run. In a subclass, `$this->featureSet` is `null` until a +feature set is configured, so read it with `?->` or call `getFeatureSet()`. + There are a number of features built-in and shipped with laminas-db: ### GlobalAdapterFeature diff --git a/src/RowGateway/AbstractRowGateway.php b/src/RowGateway/AbstractRowGateway.php index c3181945e..aed7f3558 100644 --- a/src/RowGateway/AbstractRowGateway.php +++ b/src/RowGateway/AbstractRowGateway.php @@ -105,11 +105,7 @@ public function initialize(): void return; } - $this->featureSet ??= new Feature\FeatureSet(); - $featureSet = $this->featureSet; - - $featureSet->setRowGateway($this); - $featureSet->apply('preInitialize', []); + $this->featureSet?->setRowGateway($this)->apply('preInitialize', []); if (null === $this->table) { throw Exception\RuntimeException::forMissingTable(); @@ -118,7 +114,7 @@ public function initialize(): void $this->requirePrimaryKeyColumn(); $this->requireSql(); - $featureSet->apply('postInitialize', []); + $this->featureSet?->apply('postInitialize', []); $this->isInitialized = true; } diff --git a/src/TableGateway/AbstractTableGateway.php b/src/TableGateway/AbstractTableGateway.php index 6ef596cf9..2e97c8c21 100644 --- a/src/TableGateway/AbstractTableGateway.php +++ b/src/TableGateway/AbstractTableGateway.php @@ -134,7 +134,7 @@ public function getFeatureSet(): Feature\FeatureSet $this->initialize(); } - return $this->requireFeatureSet(); + return $this->featureSet ??= (new Feature\FeatureSet())->setTableGateway($this); } public function getLastInsertValue(): string|int|false|null @@ -184,11 +184,7 @@ public function initialize(): void return; } - $this->featureSet ??= new Feature\FeatureSet(); - $featureSet = $this->featureSet; - - $featureSet->setTableGateway($this); - $featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_INITIALIZE, []); + $this->featureSet?->setTableGateway($this)->apply(EventFeatureEventsInterface::EVENT_PRE_INITIALIZE, []); $adapter = $this->adapter ?? throw Exception\RuntimeException::forMissingAdapter(); $table = $this->table ?? throw Exception\RuntimeException::forMissingTable(); @@ -196,7 +192,7 @@ public function initialize(): void $this->resultSetPrototype ??= new ResultSet(); $this->sql ??= new Sql($adapter, $table); - $featureSet->apply(EventFeatureEventsInterface::EVENT_POST_INITIALIZE, []); + $this->featureSet?->apply(EventFeatureEventsInterface::EVENT_POST_INITIALIZE, []); $this->isInitialized = true; } @@ -333,16 +329,14 @@ protected function executeDelete(Delete $delete): int throw Exception\RuntimeException::forTableMismatch('Delete'); } - $featureSet = $this->requireFeatureSet(); - // pre delete update - $featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_DELETE, [$delete]); + $this->featureSet?->apply(EventFeatureEventsInterface::EVENT_PRE_DELETE, [$delete]); $statement = $this->requireSql()->prepareStatementForSqlObject($delete); $result = $statement->execute() ?? throw Exception\RuntimeException::forMissingStatementResult('Delete'); // apply postDelete features - $featureSet->apply(EventFeatureEventsInterface::EVENT_POST_DELETE, [$statement, $result]); + $this->featureSet?->apply(EventFeatureEventsInterface::EVENT_POST_DELETE, [$statement, $result]); return $result->getAffectedRows(); } @@ -359,10 +353,8 @@ protected function executeInsert(Insert $insert): int throw Exception\RuntimeException::forTableMismatch('Insert'); } - $featureSet = $this->requireFeatureSet(); - // apply preInsert features - $featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_INSERT, [$insert]); + $this->featureSet?->apply(EventFeatureEventsInterface::EVENT_PRE_INSERT, [$insert]); // Most RDBMS solutions do not allow using table aliases in INSERTs // See https://github.com/zendframework/zf2/issues/7311 @@ -373,7 +365,7 @@ protected function executeInsert(Insert $insert): int $this->lastInsertValue = $this->getAdapter()->getDriver()->getConnection()->getLastGeneratedValue(); // apply postInsert features - $featureSet->apply(EventFeatureEventsInterface::EVENT_POST_INSERT, [$statement, $result]); + $this->featureSet?->apply(EventFeatureEventsInterface::EVENT_POST_INSERT, [$statement, $result]); return $result->getAffectedRows(); } @@ -393,10 +385,8 @@ protected function executeSelect(Select $select): ResultSetInterface $select->columns($this->columns); } - $featureSet = $this->requireFeatureSet(); - // apply preSelect features - $featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_SELECT, [$select]); + $this->featureSet?->apply(EventFeatureEventsInterface::EVENT_PRE_SELECT, [$select]); // prepare and execute $statement = $this->requireSql()->prepareStatementForSqlObject($select); @@ -407,7 +397,7 @@ protected function executeSelect(Select $select): ResultSetInterface $resultSet->initialize($result); // apply postSelect features - $featureSet->apply(EventFeatureEventsInterface::EVENT_POST_SELECT, [$statement, $result, $resultSet]); + $this->featureSet?->apply(EventFeatureEventsInterface::EVENT_POST_SELECT, [$statement, $result, $resultSet]); return $resultSet; } @@ -424,26 +414,18 @@ protected function executeUpdate(Update $update): int throw Exception\RuntimeException::forTableMismatch('Update'); } - $featureSet = $this->requireFeatureSet(); - // apply preUpdate features - $featureSet->apply(EventFeatureEventsInterface::EVENT_PRE_UPDATE, [$update]); + $this->featureSet?->apply(EventFeatureEventsInterface::EVENT_PRE_UPDATE, [$update]); $statement = $this->requireSql()->prepareStatementForSqlObject($update); $result = $statement->execute() ?? throw Exception\RuntimeException::forMissingStatementResult('Update'); // apply postUpdate features - $featureSet->apply(EventFeatureEventsInterface::EVENT_POST_UPDATE, [$statement, $result]); + $this->featureSet?->apply(EventFeatureEventsInterface::EVENT_POST_UPDATE, [$statement, $result]); return $result->getAffectedRows(); } - /** @throws Exception\RuntimeException */ - private function requireFeatureSet(): Feature\FeatureSet - { - return $this->featureSet ?? throw Exception\RuntimeException::forUninitializedProperty('featureSet'); - } - /** @throws Exception\RuntimeException */ private function requireResultSetPrototype(): ResultSetInterface { @@ -463,12 +445,11 @@ private function requireSql(): Sql * @param array $arguments * * @throws Exception\InvalidArgumentException - * @throws Exception\RuntimeException */ public function __call(string $method, array $arguments): mixed { - $featureSet = $this->requireFeatureSet(); - if ($featureSet->canCallMagicCall($method)) { + $featureSet = $this->featureSet; + if (null !== $featureSet && $featureSet->canCallMagicCall($method)) { return $featureSet->callMagicCall($method, $arguments); } throw Exception\InvalidArgumentException::forInvalidMagicCall($method, static::class); @@ -489,7 +470,6 @@ public function __clone(): void /** * @throws Exception\InvalidArgumentException - * @throws Exception\RuntimeException */ public function __get(string $property): mixed { @@ -497,8 +477,8 @@ public function __get(string $property): mixed return $this->$property; } - $featureSet = $this->requireFeatureSet(); - if ($featureSet->canCallMagicGet($property)) { + $featureSet = $this->featureSet; + if (null !== $featureSet && $featureSet->canCallMagicGet($property)) { return $featureSet->callMagicGet($property); } throw Exception\InvalidArgumentException::forInvalidMagicGet(static::class); @@ -506,12 +486,11 @@ public function __get(string $property): mixed /** * @throws Exception\InvalidArgumentException - * @throws Exception\RuntimeException */ public function __set(string $property, mixed $value): void { - $featureSet = $this->requireFeatureSet(); - if ($featureSet->canCallMagicSet($property)) { + $featureSet = $this->featureSet; + if (null !== $featureSet && $featureSet->canCallMagicSet($property)) { $featureSet->callMagicSet($property, $value); return; diff --git a/src/TableGateway/TableGateway.php b/src/TableGateway/TableGateway.php index 4fe4261ce..7073a415c 100644 --- a/src/TableGateway/TableGateway.php +++ b/src/TableGateway/TableGateway.php @@ -28,7 +28,7 @@ class TableGateway extends AbstractTableGateway public function __construct( TableIdentifier|array|string $table, AdapterInterface $adapter, - Feature\FeatureSet|Feature\FeatureInterface|array|null $features = new Feature\FeatureSet(), + Feature\FeatureSet|Feature\FeatureInterface|array|null $features = null, ?ResultSetInterface $resultSetPrototype = new ResultSet(), ?Sql $sql = null, ) { diff --git a/test/unit/RowGateway/AbstractRowGatewayTest.php b/test/unit/RowGateway/AbstractRowGatewayTest.php index e76ef6046..fd2a46a79 100644 --- a/test/unit/RowGateway/AbstractRowGatewayTest.php +++ b/test/unit/RowGateway/AbstractRowGatewayTest.php @@ -100,27 +100,28 @@ public function exchangeArray(): void } #[Test] - public function initializeCreatesFeatureSetIfNotSet(): void + public function initializeBindsAndRunsAConfiguredFeatureSet(): void { $rowGateway = $this->getMockBuilder(AbstractRowGateway::class)->onlyMethods([])->getMock(); + $featureSet = $this->createMock(FeatureSet::class); + $featureSet->expects($this->once())->method('setRowGateway')->with($rowGateway)->willReturnSelf(); - $refRowGateway = new ReflectionObject($rowGateway); - - $tableProp = $refRowGateway->getProperty('table'); - $tableProp->setValue($rowGateway, 'foo'); - - $pkProp = $refRowGateway->getProperty('primaryKeyColumn'); - $pkProp->setValue($rowGateway, ['id']); - - $sqlProp = $refRowGateway->getProperty('sql'); - $sqlProp->setValue($rowGateway, new Sql($this->mockAdapter)); + $applied = []; + $featureSet->expects($this->exactly(2)) + ->method('apply') + ->willReturnCallback(static function (string $method) use (&$applied): void { + $applied[] = $method; + }); - $featureSetProp = $refRowGateway->getProperty('featureSet'); - static::assertNull($featureSetProp->getValue($rowGateway)); + $refRowGateway = new ReflectionObject($rowGateway); + $refRowGateway->getProperty('table')->setValue($rowGateway, 'foo'); + $refRowGateway->getProperty('primaryKeyColumn')->setValue($rowGateway, ['id']); + $refRowGateway->getProperty('sql')->setValue($rowGateway, new Sql($this->mockAdapter)); + $refRowGateway->getProperty('featureSet')->setValue($rowGateway, $featureSet); - $rowGateway->populate(['id' => 1, 'name' => 'test'], true); + $rowGateway->initialize(); - static::assertInstanceOf(FeatureSet::class, $featureSetProp->getValue($rowGateway)); + static::assertSame(['preInitialize', 'postInitialize'], $applied); } #[Test] @@ -140,6 +141,30 @@ public function initializeEarlyReturnWhenAlreadyInitialized(): void static::assertSame($originalFeatureSet, $featureSetProp->getValue($rowGateway)); } + #[Test] + public function initializeLeavesTheFeatureSetUnsetWhenNoneIsConfigured(): void + { + $rowGateway = $this->getMockBuilder(AbstractRowGateway::class)->onlyMethods([])->getMock(); + + $refRowGateway = new ReflectionObject($rowGateway); + + $tableProp = $refRowGateway->getProperty('table'); + $tableProp->setValue($rowGateway, 'foo'); + + $pkProp = $refRowGateway->getProperty('primaryKeyColumn'); + $pkProp->setValue($rowGateway, ['id']); + + $sqlProp = $refRowGateway->getProperty('sql'); + $sqlProp->setValue($rowGateway, new Sql($this->mockAdapter)); + + $featureSetProp = $refRowGateway->getProperty('featureSet'); + static::assertNull($featureSetProp->getValue($rowGateway)); + + $rowGateway->populate(['id' => 1, 'name' => 'test'], true); + + static::assertNull($featureSetProp->getValue($rowGateway)); + } + #[Test] public function initializeOnlyRunsOnce(): void { diff --git a/test/unit/TableGateway/AbstractTableGatewayTest.php b/test/unit/TableGateway/AbstractTableGatewayTest.php index 45fe362da..43a1ec8cd 100644 --- a/test/unit/TableGateway/AbstractTableGatewayTest.php +++ b/test/unit/TableGateway/AbstractTableGatewayTest.php @@ -42,19 +42,6 @@ #[CoversClass(AbstractTableGateway::class)] final class AbstractTableGatewayTest extends TestCase { - protected MockObject&Adapter $mockAdapter; - - protected PlatformInterface&MockObject $mockPlatform; - - protected ResultInterface&MockObject $mockResultSet; - protected MockObject&Sql\Sql $mockSql; - protected AbstractTableGateway&MockObject $table; - protected FeatureSet&MockObject $mockFeatureSet; - protected MockObject&Select $mockSelect; - protected MockObject&Insert $mockInsert; - protected MockObject&Update $mockUpdate; - protected MockObject&Delete $mockDelete; - /** @return array}> */ public static function executeHookProvider(): array { @@ -123,6 +110,33 @@ public static function initializingOperationProvider(): array ]; } + /** @return array */ + public static function magicAccessWithoutFeatureSetProvider(): array + { + return [ + 'call' => [ + static fn(AbstractTableGateway $gateway): mixed => $gateway->__call('invalidMethod', []), + static fn(string $class): string => sprintf( + InvalidArgumentException::INVALID_MAGIC_CALL, + 'invalidMethod', + $class, + ), + ], + 'get' => [ + static fn(AbstractTableGateway $gateway): mixed => $gateway->__get('invalidProperty'), + static fn(string $class): string => sprintf(InvalidArgumentException::INVALID_MAGIC_GET, $class), + ], + 'set' => [ + static function (AbstractTableGateway $gateway): mixed { + $gateway->__set('invalidProperty', 'value'); + + return null; + }, + static fn(string $class): string => sprintf(InvalidArgumentException::INVALID_MAGIC_SET, $class), + ], + ]; + } + /** @return array, string}> */ public static function statementWithoutResultProvider(): array { @@ -138,7 +152,6 @@ public static function statementWithoutResultProvider(): array public static function uninitializedPropertyProvider(): array { return [ - 'feature set' => ['featureSet', 'getFeatureSet'], 'result set prototype' => ['resultSetPrototype', 'getResultSetPrototype'], 'sql' => ['sql', 'getSql'], ]; @@ -345,6 +358,25 @@ public function getFeatureSet(): void static::assertSame($this->mockFeatureSet, $this->table->getFeatureSet()); } + #[Test] + public function getFeatureSetCreatesOneBoundToTheGatewayWhenNoneIsConfigured(): void + { + $gateway = $this->featurelessGateway(); + $feature = new TestTableGatewayFeature(); + + $gateway->getFeatureSet()->addFeature($feature); + + static::assertSame($gateway, $feature->receivedTableGateway()); + } + + #[Test] + public function getFeatureSetReturnsTheSameCreatedFeatureSetOnEveryCall(): void + { + $gateway = $this->featurelessGateway(); + + static::assertSame($gateway->getFeatureSet(), $gateway->getFeatureSet()); + } + #[Test] public function getLastInsertValue(): void { @@ -548,6 +580,23 @@ public function isInitialized(): void static::assertTrue($stub->isInitialized()); } + /** + * @param callable(AbstractTableGateway): mixed $access + * @param callable(string): string $expectedMessage + */ + #[Test] + #[DataProvider('magicAccessWithoutFeatureSetProvider')] + public function magicAccessWithoutAFeatureSetReportsTheInvalidMember( + callable $access, + callable $expectedMessage, + ): void { + $gateway = $this->getMockBuilder(AbstractTableGateway::class)->onlyMethods([])->getMock(); + + $this->expectExceptionObject(new InvalidArgumentException($expectedMessage($gateway::class))); + + $access($gateway); + } + #[Test] public function magicCallDelegatesToTheFeatureSet(): void { @@ -736,6 +785,21 @@ public function operationsInitializeTheGatewayFirst(string $method, array $argum static::assertTrue($gateway->isInitialized()); } + /** @param list $arguments */ + #[Test] + #[DataProvider('executeHookProvider')] + public function operationsLeaveTheFeatureSetUnsetWhenNoneIsConfigured(string $method, array $arguments): void + { + $gateway = $this->featurelessGateway(); + + $gateway->{$method}(...$arguments); + + static::assertNull( + (new ReflectionClass(AbstractTableGateway::class))->getProperty('featureSet') + ->getValue($gateway), + ); + } + /** @param list $arguments */ #[Test] #[DataProvider('executeHookProvider')] @@ -853,6 +917,19 @@ public function selectWithClosure(): void static::assertInstanceOf(ResultSet::class, $result); } + protected MockObject&Adapter $mockAdapter; + + protected PlatformInterface&MockObject $mockPlatform; + + protected ResultInterface&MockObject $mockResultSet; + protected MockObject&Sql\Sql $mockSql; + protected AbstractTableGateway&MockObject $table; + protected FeatureSet&MockObject $mockFeatureSet; + protected MockObject&Select $mockSelect; + protected MockObject&Insert $mockInsert; + protected MockObject&Update $mockUpdate; + protected MockObject&Delete $mockDelete; + #[Test] public function selectWithNoWhere(): void { @@ -1101,4 +1178,14 @@ protected function setUp(): void } } } + + private function featurelessGateway(): AbstractTableGateway + { + $gateway = $this->getMockBuilder(AbstractTableGateway::class)->onlyMethods([])->getMock(); + $tgReflection = new ReflectionClass(AbstractTableGateway::class); + $tgReflection->getProperty('table')->setValue($gateway, 'foo'); + $tgReflection->getProperty('adapter')->setValue($gateway, $this->mockAdapter); + + return $gateway; + } } From 4eeae508a234fe09f4a0c0282398184c96120b8b Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 4 Nov 2026 15:09:31 +1100 Subject: [PATCH 08/10] Clear the gateway test lint baseline entries Call __isset() and offsetExists() directly instead of through isset(), and build the constructor arguments before passing them, so the no-isset and no-assign-in-argument entries leave the lint baseline. --- lint-baseline.toml | 12 ------------ test/unit/RowGateway/AbstractRowGatewayTest.php | 8 ++++---- test/unit/TableGateway/TableGatewayTest.php | 11 ++++------- 3 files changed, 8 insertions(+), 23 deletions(-) diff --git a/lint-baseline.toml b/lint-baseline.toml index 50c1220bb..00099277f 100644 --- a/lint-baseline.toml +++ b/lint-baseline.toml @@ -468,12 +468,6 @@ code = "literal-named-argument" message = "Literal argument `true` should be passed as a named argument for clarity." count = 1 -[[issues]] -file = "test/unit/RowGateway/AbstractRowGatewayTest.php" -code = "no-isset" -message = "Use of the `isset` construct." -count = 4 - [[issues]] file = "test/unit/Sql/AbstractSqlFunctionalTestCase.php" code = "halstead" @@ -689,9 +683,3 @@ file = "test/unit/Sql/UpdateTest.php" code = "no-fully-qualified-global-class-like" message = "Fully-qualified class-like reference detected." count = 1 - -[[issues]] -file = "test/unit/TableGateway/TableGatewayTest.php" -code = "no-assign-in-argument" -message = "Avoid assignments in function call arguments." -count = 3 diff --git a/test/unit/RowGateway/AbstractRowGatewayTest.php b/test/unit/RowGateway/AbstractRowGatewayTest.php index fd2a46a79..388b18792 100644 --- a/test/unit/RowGateway/AbstractRowGatewayTest.php +++ b/test/unit/RowGateway/AbstractRowGatewayTest.php @@ -257,9 +257,9 @@ public function magicGetThrowsForUnknownColumn(): void #[Test] public function magicIssetReportsWhetherAColumnIsSet(): void { - static::assertFalse(isset($this->rowGateway->foo)); + static::assertFalse($this->rowGateway->__isset('foo')); $this->rowGateway->foo = 'bar'; - static::assertTrue(isset($this->rowGateway->foo)); + static::assertTrue($this->rowGateway->__isset('foo')); } #[Test] @@ -283,9 +283,9 @@ public function magicUnsetClearsTheColumnValue(): void #[Test] public function offsetExists(): void { - static::assertFalse(isset($this->rowGateway['foo'])); + static::assertFalse($this->rowGateway->offsetExists('foo')); $this->rowGateway['foo'] = 'bar'; - static::assertTrue(isset($this->rowGateway['foo'])); + static::assertTrue($this->rowGateway->offsetExists('foo')); } #[Test] diff --git a/test/unit/TableGateway/TableGatewayTest.php b/test/unit/TableGateway/TableGatewayTest.php index b639aedea..f01d21aa0 100644 --- a/test/unit/TableGateway/TableGatewayTest.php +++ b/test/unit/TableGateway/TableGatewayTest.php @@ -83,13 +83,10 @@ public function constructor(): void static::assertInstanceOf(Sql::class, $table->getSql()); // injecting all args - $table = new TableGateway( - 'foo', - $this->mockAdapter, - $featureSet = new Feature\FeatureSet(), - $resultSet = new ResultSet(), - $sql = new Sql($this->mockAdapter, 'foo'), - ); + $featureSet = new Feature\FeatureSet(); + $resultSet = new ResultSet(); + $sql = new Sql($this->mockAdapter, 'foo'); + $table = new TableGateway('foo', $this->mockAdapter, $featureSet, $resultSet, $sql); static::assertSame('foo', $table->getTable()); static::assertSame($this->mockAdapter, $table->getAdapter()); From 4bb3a7969fae89d301d31a835aa2c3c360f1af73 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 4 Nov 2026 15:43:23 +1100 Subject: [PATCH 09/10] Keep the table reference when cloning a table gateway __clone() copied a TableIdentifier table, including one inside an aliased array, but the cloned Sql object kept the original. The execute methods compare the two by identity, so a cloned gateway with a TableIdentifier table rejected every insert, update and delete, and select() too when the table was aliased. TableIdentifier is final readonly, so sharing it is safe: drop cloneTable() and keep the reference. --- src/TableGateway/AbstractTableGateway.php | 26 ----------- .../TableGateway/AbstractTableGatewayTest.php | 13 ++---- test/unit/TableGateway/TableGatewayTest.php | 43 +++++++++++++++++++ 3 files changed, 47 insertions(+), 35 deletions(-) diff --git a/src/TableGateway/AbstractTableGateway.php b/src/TableGateway/AbstractTableGateway.php index 2e97c8c21..d02f27a5e 100644 --- a/src/TableGateway/AbstractTableGateway.php +++ b/src/TableGateway/AbstractTableGateway.php @@ -21,7 +21,6 @@ use PhpDb\Sql\Where; use PhpDb\TableGateway\Feature\EventFeatureEventsInterface; -use function array_map; use function end; use function in_array; use function is_array; @@ -58,29 +57,6 @@ abstract class AbstractTableGateway implements TableGatewayInterface protected string|int|false|null $lastInsertValue = null; - /** - * @param GatewayTable|null $table - * - * @return GatewayTable|null - */ - private static function cloneTable(TableIdentifier|string|array|null $table): TableIdentifier|string|array|null - { - if ($table instanceof TableIdentifier) { - return clone $table; - } - - if (! is_array($table)) { - return $table; - } - - return array_map( - static fn(TableIdentifier|string $aliased): TableIdentifier|string => $aliased instanceof TableIdentifier - ? clone $aliased - : $aliased, - $table, - ); - } - /** * @param GatewayWhere $where * @@ -464,8 +440,6 @@ public function __clone(): void if (null !== $this->sql) { $this->sql = clone $this->sql; } - - $this->table = self::cloneTable($this->table); } /** diff --git a/test/unit/TableGateway/AbstractTableGatewayTest.php b/test/unit/TableGateway/AbstractTableGatewayTest.php index 43a1ec8cd..af3a0d52a 100644 --- a/test/unit/TableGateway/AbstractTableGatewayTest.php +++ b/test/unit/TableGateway/AbstractTableGatewayTest.php @@ -187,7 +187,7 @@ public function cloneOfAGatewayWithoutDependenciesKeepsThemUnset(): void } #[Test] - public function cloneWithAnAliasedTableIdentifier(): void + public function cloneSharesAnAliasedTableIdentifier(): void { $tableIdentifier = new Sql\TableIdentifier('bar', 'schema'); $aliasedTable = ['alias' => $tableIdentifier]; @@ -198,14 +198,11 @@ public function cloneWithAnAliasedTableIdentifier(): void $cloned = clone $this->table; - $clonedTable = $cloned->getTable(); - static::assertIsArray($clonedTable); - // The TableIdentifier inside the array should be cloned - static::assertNotSame($tableIdentifier, $clonedTable['alias']); + static::assertSame($aliasedTable, $cloned->getTable()); } #[Test] - public function cloneWithATableIdentifier(): void + public function cloneSharesATableIdentifier(): void { $tableIdentifier = new Sql\TableIdentifier('bar', 'schema'); @@ -215,9 +212,7 @@ public function cloneWithATableIdentifier(): void $cloned = clone $this->table; - // The table should be cloned, not the same instance - static::assertNotSame($tableIdentifier, $cloned->getTable()); - static::assertEquals($tableIdentifier->getTable(), $cloned->getTable()->getTable()); + static::assertSame($tableIdentifier, $cloned->getTable()); } #[Test] diff --git a/test/unit/TableGateway/TableGatewayTest.php b/test/unit/TableGateway/TableGatewayTest.php index f01d21aa0..218d128e4 100644 --- a/test/unit/TableGateway/TableGatewayTest.php +++ b/test/unit/TableGateway/TableGatewayTest.php @@ -54,6 +54,32 @@ public static function aliasedTables(): array ]; } + /** @return array, string, list}> */ + public static function clonedGatewayOperationProvider(): array + { + $tables = [ + 'table identifier' => new TableIdentifier('foo', 'bar'), + 'aliased table identifier' => ['f' => new TableIdentifier('foo', 'bar')], + 'aliased string' => ['f' => 'foo'], + 'string' => 'foo', + ]; + $operations = [ + 'insert' => [['a' => 1]], + 'update' => [['a' => 1]], + 'delete' => [['a' => 1]], + 'select' => [], + ]; + + $cases = []; + foreach ($tables as $tableLabel => $table) { + foreach ($operations as $method => $arguments) { + $cases["{$tableLabel}, {$method}"] = [$table, $method, $arguments]; + } + } + + return $cases; + } + /** @return array>, string}> */ public static function dataChangingCallOnAliasedTable(): array { @@ -64,6 +90,23 @@ public static function dataChangingCallOnAliasedTable(): array ]; } + /** + * @param string|TableIdentifier|array $table + * @param list $arguments + */ + #[Test] + #[DataProvider('clonedGatewayOperationProvider')] + public function clonedGatewayRunsOperationsOnItsTableLikeTheOriginal( + string|TableIdentifier|array $table, + string $method, + array $arguments, + ): void { + $gateway = new TableGateway($table, new Adapter($this->mockAdapter->getDriver(), new TrustingSql92Platform())); + $cloned = clone $gateway; + + static::assertEquals($gateway->{$method}(...$arguments), $cloned->{$method}(...$arguments)); + } + /** * Beside other tests checks for plain string table identifier */ From a478946b1570dbd85482216476f6d5c2bb562483 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 4 Nov 2026 16:34:00 +1100 Subject: [PATCH 10/10] Test saving a composite-key row that is missing a key column --- .../RowGateway/AbstractRowGatewayTest.php | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/unit/RowGateway/AbstractRowGatewayTest.php b/test/unit/RowGateway/AbstractRowGatewayTest.php index 388b18792..925cb2af2 100644 --- a/test/unit/RowGateway/AbstractRowGatewayTest.php +++ b/test/unit/RowGateway/AbstractRowGatewayTest.php @@ -493,6 +493,40 @@ public function saveUpdateChangingPrimaryKey(): void static::assertEquals(['id' => 7, 'name' => 'fooUpdated'], $this->rowGateway->toArray()); } + #[Test] + public function saveUpdateWithMissingCompositeKeyColumnReselectsByTheOldKeyValue(): void + { + $selectMock = $this->getMockBuilder(Select::class) + ->onlyMethods(['where']) + ->getMock(); + $selectMock->expects($this->once()) + ->method('where') + ->with(static::equalTo(['one' => 1, 'two' => 2])) + ->willReturn($selectMock); + + $sqlMock = $this->getMockBuilder(Sql::class) + ->onlyMethods(['select']) + ->setConstructorArgs([$this->mockAdapter]) + ->getMock(); + $sqlMock->expects($this->any()) + ->method('select') + ->willReturn($selectMock); + + $this->setRowGatewayState(['primaryKeyColumn' => ['one', 'two'], 'sql' => $sqlMock]); + + $this->mockResult + ->expects($this->any()) + ->method('current') + ->willReturn(['one' => 1, 'two' => 2, 'name' => 'bar']); + + $this->rowGateway->populate(['one' => 1, 'two' => 2, 'name' => 'foo'], true); + // offsetUnset() nulls the column instead of removing it, so drop the key from the data directly + $this->setRowGatewayState(['data' => ['one' => 1, 'name' => 'bar']]); + $this->rowGateway->save(); + + static::assertEquals(['one' => 1, 'two' => 2, 'name' => 'bar'], $this->rowGateway->toArray()); + } + #[Test] public function toArray(): void {