From 3e27bbe08b955fb550b11bbec1b5cc08095e1a26 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 30 Sep 2026 10:36:43 +1000 Subject: [PATCH 01/17] Return object rows rather than discarding them AbstractResultSet::current() ended with `is_array($data) ? $data : null` while declaring `array|object|null`. The guard exists to turn `false`, meaning the results are exhausted, into `null`, and it took objects with it. Pdo\Result::isBuffered() is always false, so a PDO result never takes the `buffer === -1` path that returns the row untouched, and the guard saw every row. Under FETCH_OBJ and FETCH_LAZY every row became null, with no exception and no warning. FETCH_BOUND is unchanged on purpose: that mode carries its row in variables bound by reference and returns only true, so null is correct. Five further defects followed from the same fact, that a row need not be an array: - initialize() measured the first element of an array rather than the array, so an array of objects raised an uncaught TypeError from count(). - initialize() called getIterator() once, but that method returns Traversable and may hand back another aggregate. It now walks the chain and rejects one that never bottoms out in an Iterator. - The iteration methods reached the data source directly, so calling them before initialize() raised an Error. They now report an uninitialised result set. - HydratingResultSet::toArray() handed non-objects to the hydrator. The @throws tag promised an exception that was never raised; it is now. - ResultSet::current() and RowPrototypeResultSet::current() declared return types that excluded objects the parent can return. ResultSetFetchModeTest drives a real SQLite result through the PDO driver across every entry in Result::VALID_FETCH_MODES: nine modes return rows, FETCH_BOUND returns null while its bound columns fill, and three throw. A single FETCH_OBJ case would have missed FETCH_LAZY entirely. The three that throw are adapter defects raised separately as #198 and #199. src/ResultSet goes from 90 baselined findings to 36. The remainder are the mixed family that follows from a row being mixed, complexity metrics, array shapes no truthful annotation can narrow, and a Mago stub that treats DOMNodeList and its kin as Traversable but not Iterator. No public signature is narrowed and nothing is sealed, so the change is binary compatible for downstream consumers. --- analyzer-baseline.toml | 198 +++--------------- lint-baseline.toml | 68 +----- src/ResultSet/AbstractResultSet.php | 138 +++++++++--- .../ArrayObjectResultSetInterface.php | 2 + .../Exception/InvalidArgumentException.php | 19 +- src/ResultSet/Exception/RuntimeException.php | 16 ++ src/ResultSet/HydratingResultSet.php | 29 ++- src/ResultSet/HydratingResultSetInterface.php | 2 + src/ResultSet/ResultSet.php | 22 +- src/ResultSet/ResultSetInterface.php | 11 +- src/ResultSet/RowPrototypeInterface.php | 4 +- src/ResultSet/RowPrototypeResultSet.php | 7 +- .../RowPrototypeResultSetInterface.php | 2 + test/unit/ResultSet/AbstractResultSetTest.php | 24 +++ .../InvalidArgumentExceptionTest.php | 48 +++++ .../Exception/RuntimeExceptionTest.php | 32 +++ .../unit/ResultSet/HydratingResultSetTest.php | 13 ++ .../unit/ResultSet/ResultSetFetchModeTest.php | 149 +++++++++++++ .../ResultSet/ResultSetIntegrationTest.php | 24 ++- 19 files changed, 517 insertions(+), 291 deletions(-) create mode 100644 test/unit/ResultSet/Exception/InvalidArgumentExceptionTest.php create mode 100644 test/unit/ResultSet/ResultSetFetchModeTest.php diff --git a/analyzer-baseline.toml b/analyzer-baseline.toml index e17bd353..06c340ad 100644 --- a/analyzer-baseline.toml +++ b/analyzer-baseline.toml @@ -540,6 +540,18 @@ 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" @@ -1500,24 +1512,12 @@ code = "missing-constant-type" message = "Class constant `DEFAULT_SCHEMA` is missing a type hint." count = 1 -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "imprecise-type" -message = "Type `array` in property `$buffer` is imprecise, equivalent to `array`." -count = 1 - [[issues]] file = "src/ResultSet/AbstractResultSet.php" code = "imprecise-type" message = "Type `array` in return type of `current` is imprecise, equivalent to `array`." count = 1 -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "imprecise-type" -message = "Type `iterable` in parameter `$dataSource` is imprecise, equivalent to `iterable`." -count = 1 - [[issues]] file = "src/ResultSet/AbstractResultSet.php" code = "incompatible-return-type" @@ -1526,26 +1526,20 @@ count = 1 [[issues]] file = "src/ResultSet/AbstractResultSet.php" -code = "invalid-property-assignment-value" -message = 'Invalid type for property `$dataSource`: expected `Iterator|IteratorAggregate|PhpDb\Adapter\Driver\ResultInterface|null`, but got `iterable`.' -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "invalid-type-cast" -message = "Casting `mixed` to `array`." +code = "invalid-return-statement" +message = 'Invalid return type for function `PhpDb\ResultSet\AbstractResultSet::dataSource`: expected `Iterator`, but found `Iterator|IteratorAggregate|PhpDb\Adapter\Driver\ResultInterface`.' count = 1 [[issues]] file = "src/ResultSet/AbstractResultSet.php" -code = "missing-api-or-internal" -message = 'Abstract class `PhpDb\ResultSet\AbstractResultSet` is missing an `@api` or `@internal` annotation.' +code = "invalid-return-statement" +message = 'Invalid return type for function `PhpDb\ResultSet\AbstractResultSet::resolveIterator`: expected `Iterator`, but found `DOMNodeList|DatePeriod|Ds\Collection|Generator|Iterator|PDOStatement`.' count = 1 [[issues]] file = "src/ResultSet/AbstractResultSet.php" -code = "mixed-argument" -message = "Invalid argument type for argument #1 of `count`: expected `Countable|array`, but found `mixed`." +code = "invalid-type-cast" +message = "Casting `mixed` to `array`." count = 1 [[issues]] @@ -1556,57 +1550,27 @@ count = 3 [[issues]] file = "src/ResultSet/AbstractResultSet.php" -code = "mixed-return-statement" -message = 'Could not infer a precise return type for function `PhpDb\ResultSet\AbstractResultSet::current`. Saw type `mixed`.' +code = "mixed-property-type-coercion" +message = "A value with a less specific type `mixed` is being assigned to property `$fieldCount` (int|null)." count = 1 [[issues]] file = "src/ResultSet/AbstractResultSet.php" code = "mixed-return-statement" -message = 'Could not infer a precise return type for function `PhpDb\ResultSet\AbstractResultSet::current`. Saw type `nonnull`.' +message = 'Could not infer a precise return type for function `PhpDb\ResultSet\AbstractResultSet::current`. Saw type `mixed`.' count = 1 [[issues]] file = "src/ResultSet/AbstractResultSet.php" code = "mixed-return-statement" -message = 'Could not infer a precise return type for function `PhpDb\ResultSet\AbstractResultSet::valid`. Saw type `mixed`.' -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "non-existent-method" -message = "Method `current` does not exist on type `IteratorAggregate`." -count = 3 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "non-existent-method" -message = "Method `key` does not exist on type `IteratorAggregate`." -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "non-existent-method" -message = "Method `next` does not exist on type `IteratorAggregate`." +message = 'Could not infer a precise return type for function `PhpDb\ResultSet\AbstractResultSet::current`. Saw type `nonnull`.' count = 1 [[issues]] file = "src/ResultSet/AbstractResultSet.php" code = "non-existent-method" message = "Method `rewind` does not exist on type `IteratorAggregate`." -count = 3 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "non-existent-method" -message = "Method `valid` does not exist on type `IteratorAggregate`." -count = 2 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "possible-method-access-on-null" -message = "Attempting to call a method on `null`." -count = 6 +count = 1 [[issues]] file = "src/ResultSet/AbstractResultSet.php" @@ -1614,30 +1578,12 @@ code = "possibly-invalid-argument" message = 'Possible argument type mismatch for argument #1 of `count`: expected `Countable|array`, but possibly received `Iterator|IteratorAggregate|PhpDb\Adapter\Driver\ResultInterface`.' count = 1 -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "property-type-coercion" -message = 'A value of a less specific type `Traversable` is being assigned to property `$dataSource` (Iterator|IteratorAggregate|PhpDb\Adapter\Driver\ResultInterface|null).' -count = 1 - -[[issues]] -file = "src/ResultSet/ArrayObjectResultSetInterface.php" -code = "missing-api-or-internal" -message = 'Interface `PhpDb\ResultSet\ArrayObjectResultSetInterface` is missing an `@api` or `@internal` annotation.' -count = 1 - [[issues]] file = "src/ResultSet/ArrayResultSet.php" code = "class-must-be-final" message = 'Class `PhpDb\ResultSet\ArrayResultSet` should be declared `final`.' count = 1 -[[issues]] -file = "src/ResultSet/ArrayResultSet.php" -code = "imprecise-type" -message = "Type `array` in return type of `toArray` is imprecise, equivalent to `array`." -count = 1 - [[issues]] file = "src/ResultSet/Exception/InvalidArgumentException.php" code = "class-must-be-final" @@ -1656,12 +1602,6 @@ code = "class-must-be-final" message = 'Class `PhpDb\ResultSet\HydratingResultSet` should be declared `final`.' count = 1 -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "imprecise-type" -message = "Type `array` in return type of `toArray` is imprecise, equivalent to `array`." -count = 1 - [[issues]] file = "src/ResultSet/HydratingResultSet.php" code = "mixed-assignment" @@ -1674,36 +1614,12 @@ code = "mixed-return-statement" message = 'Could not infer a precise return type for function `PhpDb\ResultSet\HydratingResultSet::current`. Saw type `nonnull`.' count = 1 -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "non-existent-method" -message = "Method `current` does not exist on type `IteratorAggregate`." -count = 1 - [[issues]] file = "src/ResultSet/HydratingResultSet.php" code = "overly-wide-return-type" message = "Declared return type `null|object` for `getobjectprototype` has unused branches: `null`." count = 1 -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "possible-method-access-on-null" -message = "Attempting to call a method on `null`." -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "possibly-null-argument" -message = 'Argument #1 of method `Laminas\Hydrator\ExtractionInterface::extract` is possibly `null`, but parameter type `object` does not accept it.' -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSetInterface.php" -code = "missing-api-or-internal" -message = 'Interface `PhpDb\ResultSet\HydratingResultSetInterface` is missing an `@api` or `@internal` annotation.' -count = 1 - [[issues]] file = "src/ResultSet/ResultSet.php" code = "class-must-be-final" @@ -1716,24 +1632,6 @@ code = "imprecise-type" message = "Type `array` in return type of `current` is imprecise, equivalent to `array`." count = 1 -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "imprecise-type" -message = "Type `array` in return type of `toArray` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "invalid-return-statement" -message = 'Invalid return type for function `PhpDb\ResultSet\ResultSet::getReturnType`: expected `enum(PhpDb\ResultSet\ResultSetReturnType)`, but found `enum(PhpDb\ResultSet\ResultSetReturnType)|string`.' -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "less-specific-return-statement" -message = 'Returned type `array|null|object` is less specific than the declared return type `ArrayObject|array|null` for function `PhpDb\ResultSet\ResultSet::current`.' -count = 1 - [[issues]] file = "src/ResultSet/ResultSet.php" code = "missing-constant-type" @@ -1746,30 +1644,6 @@ code = "missing-constant-type" message = "Class constant `TYPE_ARRAY` is missing a type hint." count = 1 -[[issues]] -file = "src/ResultSet/ResultSetInterface.php" -code = "imprecise-type" -message = "Type `array` in return type of `toArray` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSetInterface.php" -code = "imprecise-type" -message = "Type `iterable` in parameter `$dataSource` is imprecise, equivalent to `iterable`." -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSetInterface.php" -code = "missing-api-or-internal" -message = 'Interface `PhpDb\ResultSet\ResultSetInterface` is missing an `@api` or `@internal` annotation.' -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSetInterface.php" -code = "missing-template-parameter" -message = "Too few template arguments for `Iterator`: expected at least 2, but found 0." -count = 1 - [[issues]] file = "src/ResultSet/RowPrototypeInterface.php" code = "imprecise-type" @@ -1782,12 +1656,6 @@ code = "imprecise-type" message = "Type `array` in return type of `toArray` is imprecise, equivalent to `array`." count = 1 -[[issues]] -file = "src/ResultSet/RowPrototypeInterface.php" -code = "missing-api-or-internal" -message = 'Interface `PhpDb\ResultSet\RowPrototypeInterface` is missing an `@api` or `@internal` annotation.' -count = 1 - [[issues]] file = "src/ResultSet/RowPrototypeResultSet.php" code = "class-must-be-final" @@ -1800,24 +1668,6 @@ code = "imprecise-type" message = "Type `array` in return type of `current` is imprecise, equivalent to `array`." count = 1 -[[issues]] -file = "src/ResultSet/RowPrototypeResultSet.php" -code = "imprecise-type" -message = "Type `array` in return type of `toArray` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/ResultSet/RowPrototypeResultSet.php" -code = "less-specific-return-statement" -message = 'Returned type `null|object` is less specific than the declared return type `PhpDb\ResultSet\RowPrototypeInterface|array|null` for function `PhpDb\ResultSet\RowPrototypeResultSet::current`.' -count = 1 - -[[issues]] -file = "src/ResultSet/RowPrototypeResultSetInterface.php" -code = "missing-api-or-internal" -message = 'Interface `PhpDb\ResultSet\RowPrototypeResultSetInterface` is missing an `@api` or `@internal` annotation.' -count = 1 - [[issues]] file = "src/RowGateway/AbstractRowGateway.php" code = "imprecise-type" @@ -5259,7 +5109,7 @@ count = 1 [[issues]] file = "src/TableGateway/AbstractTableGateway.php" code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\ResultSet\ResultSetInterface::initialize`: expected `iterable`, but found `mixed`.' +message = 'Invalid argument type for argument #1 of `PhpDb\ResultSet\ResultSetInterface::initialize`: expected `iterable`, but found `mixed`.' count = 1 [[issues]] diff --git a/lint-baseline.toml b/lint-baseline.toml index a167d2d1..9ff1f0d7 100644 --- a/lint-baseline.toml +++ b/lint-baseline.toml @@ -260,88 +260,28 @@ count = 2 [[issues]] file = "src/ResultSet/AbstractResultSet.php" -code = "no-else-clause" -message = "Avoid `else` clauses." +code = "kan-defect" +message = "Class has a high kan defect score." count = 1 [[issues]] file = "src/ResultSet/AbstractResultSet.php" -code = "no-else-clause" -message = "Avoid `elseif` clauses." -count = 3 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "no-isset" -message = "Use of the `isset` construct." -count = 2 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "no-redundant-else" -message = "The `if` branch always terminates; the trailing branches can be extracted." +code = "too-many-methods" +message = "Class has too many methods." count = 1 -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "yoda-conditions" -message = "Use Yoda condition style for safer comparisons" -count = 5 - [[issues]] file = "src/ResultSet/ArrayObjectResultSetInterface.php" code = "prefer-self-return-type" message = "Return type `ArrayObjectResultSetInterface` refers to the enclosing class; use `self` instead." count = 1 -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "halstead" -message = "Method has a high halstead difficulty" -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "no-else-clause" -message = "Avoid `elseif` clauses." -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "no-isset" -message = "Use of the `isset` construct." -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "yoda-conditions" -message = "Use Yoda condition style for safer comparisons" -count = 1 - [[issues]] file = "src/ResultSet/HydratingResultSetInterface.php" code = "prefer-self-return-type" message = "Return type `HydratingResultSetInterface` refers to the enclosing class; use `self` instead." count = 1 -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "yoda-conditions" -message = "Use Yoda condition style for safer comparisons" -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSetInterface.php" -code = "prefer-self-return-type" -message = "Return type `ResultSetInterface` refers to the enclosing class; use `self` instead." -count = 1 - -[[issues]] -file = "src/ResultSet/RowPrototypeInterface.php" -code = "prefer-self-return-type" -message = "Return type `RowPrototypeInterface` refers to the enclosing class; use `self` instead." -count = 1 - [[issues]] file = "src/ResultSet/RowPrototypeResultSetInterface.php" code = "prefer-self-return-type" diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index b4ddc698..571908ed 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -14,20 +14,31 @@ use PhpDb\ResultSet\Exception\InvalidArgumentException; use PhpDb\ResultSet\Exception\RuntimeException; use ReturnTypeWillChange; +use Traversable; +use function array_key_exists; use function count; use function current; use function is_array; +use function is_object; use function reset; +/** + * @api + */ abstract class AbstractResultSet implements ResultSetInterface { + /** How deeply initialize() will unwrap a chain of IteratorAggregate. */ + private const int MAX_ITERATOR_DEPTH = 8; + /** * if -1, datasource is already buffered * if -2, implicitly disabling buffering in ResultSet * if false, explicitly disabled * if null, default state - nothing, but can buffer until iteration started * if array, already buffering + * + * @var int|array|bool|null */ protected int|array|bool|null $buffer = null; @@ -39,14 +50,50 @@ abstract class AbstractResultSet implements ResultSetInterface protected int $position = 0; + /** + * Resolve an IteratorAggregate chain down to the Iterator it wraps. + * + * IteratorAggregate::getIterator() is declared to return Traversable, so it may + * hand back another aggregate. Anything that never bottoms out in an Iterator is + * rejected rather than stored, since the result set can only iterate an Iterator. + * + * @throws InvalidArgumentException + * @throws Exception If the data source raises one while handing over its iterator. + */ + private static function resolveIterator(Traversable $dataSource): Iterator + { + $depth = 0; + while ($dataSource instanceof IteratorAggregate) { + if (++$depth > self::MAX_ITERATOR_DEPTH) { + throw InvalidArgumentException::forUnresolvableIterator(self::MAX_ITERATOR_DEPTH); + } + + $dataSource = $dataSource->getIterator(); + } + + if ($dataSource instanceof Iterator) { + return $dataSource; + } + + // Userland cannot implement Traversable without Iterator or IteratorAggregate, and the + // internal classes that do (PDOStatement, DOMNodeList, DatePeriod) are all aggregates + // the loop above has already unwrapped. Kept so the return type cannot be violated. + // @codeCoverageIgnoreStart + throw InvalidArgumentException::forNonIteratorDataSource($dataSource::class); + + // @codeCoverageIgnoreEnd + } + /** * @throws RuntimeException */ public function buffer(): ResultSetInterface { - if ($this->buffer === -2) { + if (-2 === $this->buffer) { throw RuntimeException::forUnbufferedIteration(); - } elseif ($this->buffer === null) { + } + + if (null === $this->buffer) { $this->buffer = []; if ($this->dataSource instanceof ResultInterface) { $this->dataSource->rewind(); @@ -63,7 +110,7 @@ public function buffer(): ResultSetInterface #[ReturnTypeWillChange] public function count(): ?int { - if ($this->count !== null) { + if (null !== $this->count) { return $this->count; } @@ -76,27 +123,35 @@ public function count(): ?int /** * Iterator: get current item + * + * @throws RuntimeException */ #[Override] public function current(): array|object|null { if (-1 === $this->buffer) { // datasource was an array when the resultset was initialized - return $this->dataSource->current(); + return $this->dataSource()->current(); } - if ($this->buffer === null) { + if (null === $this->buffer) { $this->buffer = -2; // implicitly disable buffering from here on - } elseif (is_array($this->buffer) && isset($this->buffer[$this->position])) { + } + + if ( + is_array($this->buffer) + && array_key_exists($this->position, $this->buffer) + && null !== $this->buffer[$this->position] + ) { return $this->buffer[$this->position]; } - $data = $this->dataSource->current(); + $data = $this->dataSource()->current(); if (is_array($this->buffer)) { $this->buffer[$this->position] = $data; } - return is_array($data) ? $data : null; + return is_array($data) || is_object($data) ? $data : null; } /** @@ -109,6 +164,8 @@ public function getDataSource(): ResultInterface|IteratorAggregate|Iterator|null /** * Retrieve count of fields in individual rows of the result set + * + * @throws RuntimeException */ #[Override] public function getFieldCount(): int @@ -117,11 +174,12 @@ public function getFieldCount(): int return $this->fieldCount; } - $dataSource = $this->getDataSource(); - if (null === $dataSource) { + if (null === $this->dataSource) { return 0; } + $dataSource = $this->dataSource(); + $dataSource->rewind(); if (! $dataSource->valid()) { $this->fieldCount = 0; @@ -166,21 +224,19 @@ public function initialize(iterable $dataSource): ResultSetInterface return $this; } - if (is_array($dataSource)) { - // its safe to get numbers from an array - $first = current($dataSource); - reset($dataSource); - $this->fieldCount = $first === false ? 0 : count($first); - $this->dataSource = new ArrayIterator($dataSource); - $this->buffer = -1; // array's are a natural buffer - } elseif ($dataSource instanceof IteratorAggregate) { - /** @phpstan-ignore assign.propertyType */ - $this->dataSource = $dataSource->getIterator(); - } else { - /** @phpstan-ignore assign.propertyType */ - $this->dataSource = $dataSource; + if ($dataSource instanceof Traversable) { + $this->dataSource = self::resolveIterator($dataSource); + + return $this; } + // the array is safe to measure, but its first row can be any shape at all + $first = current($dataSource); + $this->fieldCount = is_array($first) || $first instanceof Countable ? count($first) : 0; + reset($dataSource); + $this->dataSource = new ArrayIterator($dataSource); + $this->buffer = -1; // array's are a natural buffer + return $this; } @@ -200,16 +256,18 @@ public function key(): int /** * Iterator: move pointer to next item + * + * @throws RuntimeException */ #[Override] public function next(): void { - if ($this->buffer === null) { + if (null === $this->buffer) { $this->buffer = -2; // implicitly disable buffering from here on } - if (! is_array($this->buffer) || $this->position === $this->dataSource->key()) { - $this->dataSource->next(); + if (! is_array($this->buffer) || $this->position === $this->dataSource()->key()) { + $this->dataSource()->next(); } $this->position++; @@ -217,12 +275,14 @@ public function next(): void /** * Iterator: rewind + * + * @throws RuntimeException */ #[Override] public function rewind(): void { if (! is_array($this->buffer)) { - $this->dataSource->rewind(); + $this->dataSource()->rewind(); } $this->position = 0; @@ -230,14 +290,34 @@ public function rewind(): void /** * Iterator: is pointer valid? + * + * @throws RuntimeException */ #[Override] public function valid(): bool { - if (is_array($this->buffer) && isset($this->buffer[$this->position])) { + if ( + is_array($this->buffer) + && array_key_exists($this->position, $this->buffer) + && null !== $this->buffer[$this->position] + ) { return true; } - return $this->dataSource->valid(); + return $this->dataSource()->valid(); + } + + /** + * The data source, once initialize() has supplied one. + * + * @throws RuntimeException + */ + protected function dataSource(): Iterator + { + if (! $this->dataSource instanceof Iterator) { + throw RuntimeException::forUninitialisedDataSource(); + } + + return $this->dataSource; } } diff --git a/src/ResultSet/ArrayObjectResultSetInterface.php b/src/ResultSet/ArrayObjectResultSetInterface.php index f73b82ce..95f9f6e1 100644 --- a/src/ResultSet/ArrayObjectResultSetInterface.php +++ b/src/ResultSet/ArrayObjectResultSetInterface.php @@ -8,6 +8,8 @@ /** * Capability interface for a ResultSet whose rows clone an ArrayObject prototype. + * + * @api */ interface ArrayObjectResultSetInterface { diff --git a/src/ResultSet/Exception/InvalidArgumentException.php b/src/ResultSet/Exception/InvalidArgumentException.php index eea805db..3757942e 100644 --- a/src/ResultSet/Exception/InvalidArgumentException.php +++ b/src/ResultSet/Exception/InvalidArgumentException.php @@ -6,4 +6,21 @@ use PhpDb\Exception; -class InvalidArgumentException extends Exception\InvalidArgumentException {} +use function sprintf; + +class InvalidArgumentException extends Exception\InvalidArgumentException +{ + final public const string UNRESOLVABLE_ITERATOR = 'The data source nests IteratorAggregate more than %d levels deep and cannot be resolved to an Iterator'; + + final public const string NON_ITERATOR_DATA_SOURCE = 'The data source resolved to "%s", which is not an Iterator and cannot be traversed by a result set'; + + public static function forNonIteratorDataSource(string $type): self + { + return new self(sprintf(self::NON_ITERATOR_DATA_SOURCE, $type)); + } + + public static function forUnresolvableIterator(int $maxDepth): self + { + return new self(sprintf(self::UNRESOLVABLE_ITERATOR, $maxDepth)); + } +} diff --git a/src/ResultSet/Exception/RuntimeException.php b/src/ResultSet/Exception/RuntimeException.php index 4b972d66..20a063eb 100644 --- a/src/ResultSet/Exception/RuntimeException.php +++ b/src/ResultSet/Exception/RuntimeException.php @@ -6,12 +6,28 @@ use PhpDb\Exception; +use function sprintf; + class RuntimeException extends Exception\RuntimeException { final public const string UNBUFFERED_ITERATION = 'Buffering must be enabled before iteration is started'; + final public const string UNINITIALISED_DATA_SOURCE = 'The result set has no data source; call initialize() first'; + public static function forUnbufferedIteration(): self { return new self(self::UNBUFFERED_ITERATION); } + + final public const string UNHYDRATABLE_ROW = 'Cannot extract a row of type "%s"; the hydrator requires an object'; + + public static function forUnhydratableRow(string $type): self + { + return new self(sprintf(self::UNHYDRATABLE_ROW, $type)); + } + + public static function forUninitialisedDataSource(): self + { + return new self(self::UNINITIALISED_DATA_SOURCE); + } } diff --git a/src/ResultSet/HydratingResultSet.php b/src/ResultSet/HydratingResultSet.php index 1653e380..16725c1c 100644 --- a/src/ResultSet/HydratingResultSet.php +++ b/src/ResultSet/HydratingResultSet.php @@ -8,8 +8,12 @@ use Laminas\Hydrator\ArraySerializableHydrator; use Laminas\Hydrator\HydratorInterface; use Override; +use PhpDb\ResultSet\Exception\RuntimeException; +use function array_key_exists; +use function get_debug_type; use function is_array; +use function is_object; class HydratingResultSet extends AbstractResultSet implements HydratingResultSetInterface { @@ -20,16 +24,25 @@ public function __construct( /** * Iterator: get current item + * + * @throws RuntimeException */ #[Override] public function current(): ?object { - if ($this->buffer === null) { + if (null === $this->buffer) { $this->buffer = -2; // implicitly disable buffering from here on - } elseif (is_array($this->buffer) && isset($this->buffer[$this->position])) { + } + + if ( + is_array($this->buffer) + && array_key_exists($this->position, $this->buffer) + && null !== $this->buffer[$this->position] + ) { return $this->buffer[$this->position]; } - $data = $this->dataSource->current(); + + $data = $this->dataSource()->current(); $current = is_array($data) ? $this->getHydrator()->hydrate($data, clone $this->getRowPrototype()) : null; if (is_array($this->buffer)) { @@ -77,8 +90,9 @@ public function setObjectPrototype(object $objectPrototype): ResultSetInterface /** {@inheritDoc} */ #[Override] - public function setRowPrototype(object $rowPrototype): ResultSetInterface&HydratingResultSetInterface - { + public function setRowPrototype( + object $rowPrototype, + ): ResultSetInterface&HydratingResultSetInterface { $this->rowPrototype = $rowPrototype; return $this; } @@ -93,8 +107,13 @@ public function toArray(): array { $return = []; foreach ($this as $row) { + if (! is_object($row)) { + throw RuntimeException::forUnhydratableRow(get_debug_type($row)); + } + $return[] = $this->getHydrator()->extract($row); } + return $return; } } diff --git a/src/ResultSet/HydratingResultSetInterface.php b/src/ResultSet/HydratingResultSetInterface.php index 011a90b5..936cec16 100644 --- a/src/ResultSet/HydratingResultSetInterface.php +++ b/src/ResultSet/HydratingResultSetInterface.php @@ -6,6 +6,8 @@ /** * Capability interface for a ResultSet whose rows are hydrated onto an arbitrary object prototype. + * + * @api */ interface HydratingResultSetInterface { diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index f49d0bef..ddf510ff 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -16,27 +16,27 @@ class ResultSet extends AbstractResultSet implements ArrayObjectResultSetInterfa public const TYPE_ARRAYOBJECT = 'arrayobject'; public const TYPE_ARRAY = 'array'; + private ResultSetReturnType $returnType; + public function __construct( - private ResultSetReturnType|string $returnType = ResultSetReturnType::ArrayObject, + ResultSetReturnType|string $returnType = ResultSetReturnType::ArrayObject, private ArrayObject $rowPrototype = new ArrayObject( [], ArrayObject::ARRAY_AS_PROPS, ), ) { - if (is_string($this->returnType)) { - $this->returnType = ResultSetReturnType::from($this->returnType); - } + $this->returnType = is_string($returnType) ? ResultSetReturnType::from($returnType) : $returnType; } /** * Iterator: get current item */ #[Override] - public function current(): array|ArrayObject|null + public function current(): array|object|null { $data = parent::current(); - if ($this->returnType === ResultSetReturnType::ArrayObject && is_array($data)) { + if (ResultSetReturnType::ArrayObject === $this->returnType && is_array($data)) { $ao = clone $this->getRowPrototype(); $ao->exchangeArray($data); @@ -74,15 +74,17 @@ public function getRowPrototype(): ArrayObject * * @deprecated use setRowPrototype() */ - public function setArrayObjectPrototype(ArrayObject $arrayObjectPrototype): ResultSetInterface&ArrayObjectResultSetInterface - { + public function setArrayObjectPrototype( + ArrayObject $arrayObjectPrototype, + ): ResultSetInterface&ArrayObjectResultSetInterface { return $this->setRowPrototype($arrayObjectPrototype); } /** {@inheritDoc} */ #[Override] - public function setRowPrototype(ArrayObject $rowPrototype): ResultSetInterface&ArrayObjectResultSetInterface - { + public function setRowPrototype( + ArrayObject $rowPrototype, + ): ResultSetInterface&ArrayObjectResultSetInterface { $this->rowPrototype = $rowPrototype; return $this; diff --git a/src/ResultSet/ResultSetInterface.php b/src/ResultSet/ResultSetInterface.php index dc0a3525..d06e4ed1 100644 --- a/src/ResultSet/ResultSetInterface.php +++ b/src/ResultSet/ResultSetInterface.php @@ -7,6 +7,11 @@ use Countable; use Iterator; +/** + * @api + * + * @extends Iterator + */ interface ResultSetInterface extends Iterator, Countable { /** @@ -18,11 +23,15 @@ public function getFieldCount(): int; /** * Can be anything iterable|array + * + * @param iterable $dataSource */ - public function initialize(iterable $dataSource): ResultSetInterface; + public function initialize(iterable $dataSource): self; /** * Get all rows as an array + * + * @return list */ public function toArray(): array; } diff --git a/src/ResultSet/RowPrototypeInterface.php b/src/ResultSet/RowPrototypeInterface.php index 44b3ece1..5d3c1db2 100644 --- a/src/ResultSet/RowPrototypeInterface.php +++ b/src/ResultSet/RowPrototypeInterface.php @@ -10,13 +10,15 @@ * Row prototypes are cloned (but do not have to be) for each row and populated via populate(). * This interface allows custom row objects (like RowGateway) to be used * as prototypes without depending on ArrayObject. + * + * @api */ interface RowPrototypeInterface { /** * Populate the prototype with row data. Mutating vs. returning a new instance is up to the implementation. */ - public function populate(array $data): RowPrototypeInterface; + public function populate(array $data): self; /** * Current data as an array and match current RowGateway implementations. diff --git a/src/ResultSet/RowPrototypeResultSet.php b/src/ResultSet/RowPrototypeResultSet.php index 898a66bb..bc5b4cff 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -18,7 +18,7 @@ public function __construct( * Iterator: get current item */ #[Override] - public function current(): array|RowPrototypeInterface|null + public function current(): array|object|null { $data = parent::current(); @@ -38,8 +38,9 @@ public function getRowPrototype(): RowPrototypeInterface /** {@inheritDoc} */ #[Override] - public function setRowPrototype(RowPrototypeInterface $rowPrototype): ResultSetInterface&RowPrototypeResultSetInterface - { + public function setRowPrototype( + RowPrototypeInterface $rowPrototype, + ): ResultSetInterface&RowPrototypeResultSetInterface { $this->rowPrototype = $rowPrototype; return $this; diff --git a/src/ResultSet/RowPrototypeResultSetInterface.php b/src/ResultSet/RowPrototypeResultSetInterface.php index ce10aa20..2b1f316d 100644 --- a/src/ResultSet/RowPrototypeResultSetInterface.php +++ b/src/ResultSet/RowPrototypeResultSetInterface.php @@ -6,6 +6,8 @@ /** * Capability interface for a ResultSet whose rows clone a RowPrototypeInterface prototype. + * + * @api */ interface RowPrototypeResultSetInterface { diff --git a/test/unit/ResultSet/AbstractResultSetTest.php b/test/unit/ResultSet/AbstractResultSetTest.php index 83bd3b7c..a0312072 100644 --- a/test/unit/ResultSet/AbstractResultSetTest.php +++ b/test/unit/ResultSet/AbstractResultSetTest.php @@ -25,6 +25,8 @@ use function assert; #[CoversMethod(AbstractResultSet::class, 'initialize')] +#[CoversMethod(AbstractResultSet::class, 'resolveIterator')] +#[CoversMethod(AbstractResultSet::class, 'dataSource')] #[CoversMethod(AbstractResultSet::class, 'buffer')] #[CoversMethod(AbstractResultSet::class, 'isBuffered')] #[CoversMethod(AbstractResultSet::class, 'getDataSource')] @@ -153,6 +155,17 @@ public function current(): void static::assertEquals(['id' => 1, 'name' => 'one'], $resultSet->current()); } + #[Test] + public function currentReportsAnUninitialisedDataSourceRatherThanFailingOnNull(): void + { + $resultSet = $this->createResultSetMock(); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage(RuntimeException::UNINITIALISED_DATA_SOURCE); + + $resultSet->current(); + } + #[Test] public function currentReturnsBufferedDataOnSecondPass(): void { @@ -545,6 +558,17 @@ public function validAnswersFromTheBufferAfterTheDataSourceIsExhausted(): void static::assertTrue($resultSet->valid()); } + #[Test] + public function validReportsAnUninitialisedDataSourceRatherThanFailingOnNull(): void + { + $resultSet = $this->createResultSetMock(); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage(RuntimeException::UNINITIALISED_DATA_SOURCE); + + $resultSet->valid(); + } + #[Test] public function validReturnsFalseAfterLastElement(): void { diff --git a/test/unit/ResultSet/Exception/InvalidArgumentExceptionTest.php b/test/unit/ResultSet/Exception/InvalidArgumentExceptionTest.php new file mode 100644 index 00000000..c330e5b2 --- /dev/null +++ b/test/unit/ResultSet/Exception/InvalidArgumentExceptionTest.php @@ -0,0 +1,48 @@ +getMessage(), + ); + } + + #[Test] + public function forNonIteratorDataSourceReturnsTheComponentExceptionType(): void + { + self::assertInstanceOf( + ExceptionInterface::class, + InvalidArgumentException::forNonIteratorDataSource('DatePeriod'), + ); + } + + #[Test] + public function forUnresolvableIteratorNamesTheDepthItGaveUpAt(): void + { + self::assertStringContainsString('8', InvalidArgumentException::forUnresolvableIterator(8)->getMessage()); + } + + #[Test] + public function forUnresolvableIteratorReturnsTheComponentExceptionType(): void + { + self::assertInstanceOf(ExceptionInterface::class, InvalidArgumentException::forUnresolvableIterator(8)); + } +} diff --git a/test/unit/ResultSet/Exception/RuntimeExceptionTest.php b/test/unit/ResultSet/Exception/RuntimeExceptionTest.php index bab9cbdb..b8d89c08 100644 --- a/test/unit/ResultSet/Exception/RuntimeExceptionTest.php +++ b/test/unit/ResultSet/Exception/RuntimeExceptionTest.php @@ -13,6 +13,8 @@ #[Group('unit')] #[CoversMethod(RuntimeException::class, 'forUnbufferedIteration')] +#[CoversMethod(RuntimeException::class, 'forUninitialisedDataSource')] +#[CoversMethod(RuntimeException::class, 'forUnhydratableRow')] final class RuntimeExceptionTest extends TestCase { #[Test] @@ -29,4 +31,34 @@ public function forUnbufferedIterationReturnsTheComponentExceptionType(): void { self::assertInstanceOf(ExceptionInterface::class, RuntimeException::forUnbufferedIteration()); } + + #[Test] + public function forUnhydratableRowNamesTheOffendingType(): void + { + self::assertStringContainsString( + 'bool', + RuntimeException::forUnhydratableRow('bool')->getMessage(), + ); + } + + #[Test] + public function forUnhydratableRowReturnsTheComponentExceptionType(): void + { + self::assertInstanceOf(ExceptionInterface::class, RuntimeException::forUnhydratableRow('bool')); + } + + #[Test] + public function forUninitialisedDataSourceRendersItsTemplate(): void + { + self::assertSame( + RuntimeException::UNINITIALISED_DATA_SOURCE, + RuntimeException::forUninitialisedDataSource()->getMessage(), + ); + } + + #[Test] + public function forUninitialisedDataSourceReturnsTheComponentExceptionType(): void + { + self::assertInstanceOf(ExceptionInterface::class, RuntimeException::forUninitialisedDataSource()); + } } diff --git a/test/unit/ResultSet/HydratingResultSetTest.php b/test/unit/ResultSet/HydratingResultSetTest.php index d8e79255..35d4bcca 100644 --- a/test/unit/ResultSet/HydratingResultSetTest.php +++ b/test/unit/ResultSet/HydratingResultSetTest.php @@ -207,6 +207,19 @@ public function toArray(): void static::assertIsArray($obj); } + #[Test] + public function toArrayReportsARowTheHydratorCannotExtract(): void + { + $hydratingRs = new HydratingResultSet(); + // Scalars never hydrate into an object, so current() yields null for each row. + $hydratingRs->initialize(new ArrayIterator([1, 2])); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage('null'); + + $hydratingRs->toArray(); + } + #[Test] public function toArrayUsesHydratorExtract(): void { diff --git a/test/unit/ResultSet/ResultSetFetchModeTest.php b/test/unit/ResultSet/ResultSetFetchModeTest.php new file mode 100644 index 00000000..9b78eb3d --- /dev/null +++ b/test/unit/ResultSet/ResultSetFetchModeTest.php @@ -0,0 +1,149 @@ + + */ + public static function rowYieldingModeProvider(): array + { + return [ + 'FETCH_LAZY hands back a PDORow' => [PDO::FETCH_LAZY, PDORow::class], + 'FETCH_ASSOC fills the ArrayObject prototype' => [PDO::FETCH_ASSOC, ArrayObject::class], + 'FETCH_NUM fills the ArrayObject prototype' => [PDO::FETCH_NUM, ArrayObject::class], + 'FETCH_BOTH fills the ArrayObject prototype' => [PDO::FETCH_BOTH, ArrayObject::class], + 'FETCH_OBJ hands back a stdClass' => [PDO::FETCH_OBJ, stdClass::class], + 'FETCH_NAMED fills the ArrayObject prototype' => [PDO::FETCH_NAMED, ArrayObject::class], + 'FETCH_KEY_PAIR fills the ArrayObject prototype' => [PDO::FETCH_KEY_PAIR, ArrayObject::class], + 'FETCH_PROPS_LATE is a flag, so PDO falls back' => [PDO::FETCH_PROPS_LATE, ArrayObject::class], + 'FETCH_CLASSTYPE is a flag, so PDO falls back' => [PDO::FETCH_CLASSTYPE, ArrayObject::class], + ]; + } + + /** + * Modes Result::setFetchMode() accepts but cannot actually drive, because it takes + * only an int and PDO needs more. Documented here so the gap stays visible. + * + * @return array + */ + public static function unusableModeProvider(): array + { + return [ + 'FETCH_CLASS needs a class name PDO is never given' => [PDO::FETCH_CLASS], + 'FETCH_INTO needs a target object PDO is never given' => [PDO::FETCH_INTO], + 'FETCH_FUNC is rejected by PDO outside fetchAll()' => [PDO::FETCH_FUNC], + ]; + } + + /** + * FETCH_BOUND carries its row in variables bound by reference, so there is nothing + * for the result set to hand back. Null is the correct answer here, not a loss. + */ + #[Test] + public function boundFetchModeYieldsNullAndPopulatesTheBoundColumns(): void + { + $result = new Result(); + $result->initialize($this->pdo->query('SELECT id, name FROM t'), null); + $result->setFetchMode(PDO::FETCH_BOUND); + + $id = null; + $resource = $result->getResource(); + $resource->bindColumn(1, $id); + + $resultSet = new ResultSet(); + $resultSet->initialize($result); + + $seen = []; + foreach ($resultSet as $row) { + static::assertNull($row); + $seen[] = (int) $id; + } + + static::assertSame([1, 2], $seen); + } + + #[Test] + #[DataProvider('rowYieldingModeProvider')] + public function everyRowYieldingFetchModeReturnsBothRows(int $fetchMode): void + { + static::assertCount(2, $this->rowsFor($fetchMode)); + } + + #[Test] + #[DataProvider('rowYieldingModeProvider')] + public function everyRowYieldingFetchModeReturnsItsRows(int $fetchMode, string $expectedType): void + { + $rows = $this->rowsFor($fetchMode); + + static::assertContainsOnlyInstancesOf($expectedType, $rows); + } + + #[Test] + #[DataProvider('unusableModeProvider')] + public function fetchModesNeedingExtraArgumentsCannotBeDriven(int $fetchMode): void + { + $this->expectException(Throwable::class); + + $this->rowsFor($fetchMode); + } + + #[Test] + public function objectRowsKeepTheirColumnValues(): void + { + $rows = $this->rowsFor(PDO::FETCH_OBJ); + + static::assertSame([1, 2], array_map(static fn(stdClass $row): int => (int) $row->id, $rows)); + } + + protected function setUp(): void + { + $this->pdo = new PDO('sqlite::memory:'); + $this->pdo->exec('CREATE TABLE t (id INTEGER, name TEXT)'); + $this->pdo->exec('INSERT INTO t VALUES (1, "one"), (2, "two")'); + } + + /** @return list */ + private function rowsFor(int $fetchMode): array + { + $result = new Result(); + $result->initialize($this->pdo->query('SELECT id, name FROM t'), null); + $result->setFetchMode($fetchMode); + + $resultSet = new ResultSet(); + $resultSet->initialize($result); + + return iterator_to_array($resultSet, preserve_keys: false); + } +} diff --git a/test/unit/ResultSet/ResultSetIntegrationTest.php b/test/unit/ResultSet/ResultSetIntegrationTest.php index b1bfd67b..15189c73 100644 --- a/test/unit/ResultSet/ResultSetIntegrationTest.php +++ b/test/unit/ResultSet/ResultSetIntegrationTest.php @@ -9,6 +9,7 @@ use Override; use PhpDb\Adapter\Driver\ResultInterface; use PhpDb\ResultSet\AbstractResultSet; +use PhpDb\ResultSet\Exception\InvalidArgumentException; use PhpDb\ResultSet\Exception\RuntimeException; use PhpDb\ResultSet\ResultSet; use PhpDb\ResultSet\ResultSetReturnType; @@ -28,6 +29,7 @@ use function var_export; #[CoversMethod(AbstractResultSet::class, 'current')] +#[CoversMethod(AbstractResultSet::class, 'resolveIterator')] #[CoversMethod(AbstractResultSet::class, 'buffer')] #[CoversMethod(ResultSet::class, 'current')] #[CoversMethod(ResultSet::class, 'getReturnType')] @@ -97,13 +99,14 @@ public function canProvideArrayAsDataSource(): void #[Test] public function canProvideIteratorAggregateAsDataSource(): void { + $iterator = new ArrayIterator([['id' => 1, 'name' => 'one']]); $iteratorAggregate = $this->getMockBuilder('IteratorAggregate') ->onlyMethods(['getIterator']) ->getMock(); - $iteratorAggregate->expects($this->any())->method('getIterator')->willReturn($iteratorAggregate); - // Initialize with IteratorAggregate and verify its iterator is used + $iteratorAggregate->expects($this->any())->method('getIterator')->willReturn($iterator); + // Initialize with IteratorAggregate and verify the iterator it wraps is stored $this->resultSet->initialize($iteratorAggregate); - static::assertSame($iteratorAggregate->getIterator(), $this->resultSet->getDataSource()); + static::assertSame($iterator, $this->resultSet->getDataSource()); } /** @@ -282,6 +285,21 @@ public function invalidDataSourceRaisesException(mixed $dataSource): void $this->resultSet->initialize($dataSource); } + /** + * @throws \Exception + */ + #[Test] + public function rejectsAnIteratorAggregateThatNeverResolvesToAnIterator(): void + { + $iteratorAggregate = $this->getMockBuilder('IteratorAggregate') + ->onlyMethods(['getIterator']) + ->getMock(); + $iteratorAggregate->expects($this->any())->method('getIterator')->willReturn($iteratorAggregate); + + self::expectException(InvalidArgumentException::class); + $this->resultSet->initialize($iteratorAggregate); + } + #[Test] public function returnTypeIsObjectByDefault(): void { From c087edb1ad507940536acd662d5a86b8b6cc5265 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 30 Sep 2026 10:58:06 +1000 Subject: [PATCH 02/17] Fill row prototypes from object rows instead of widening current() The previous commit widened ResultSet::current() to array|object|null and RowPrototypeResultSet::current() likewise, so that object rows could pass through untouched. That made both declarations weaker than the contract each class actually offers: a ResultSet hands back ArrayObject, and a RowPrototypeResultSet hands back its prototype. Both keep their original narrow return types. A row that already satisfies the declared type is handed back untouched, preserving instance identity. Any other object row is reduced to an array and used to fill a clone of the prototype, so FETCH_OBJ now yields an ArrayObject rather than a stdClass. HydratingResultSet did the same thing behind a ?object declaration: it tested is_array() and returned null for everything else, so object rows were still discarded there. It now hydrates them on the same terms. AbstractResultSet::rowToArray() performs the reduction. A row carrying its values as elements rather than properties, ArrayObject being the common case, is read through its iterator. Everything else goes through get_object_vars(), which sees only public properties from outside the row's scope; a plain cast would instead yield mangled keys for anything private. A row that exposes nothing either way is rejected rather than emptied. PDORow is the case that matters: FETCH_LAZY resolves columns through __get(), so both get_object_vars() and an array cast return an empty array. Converting it would drop every column, which is a quieter version of the defect this branch set out to fix, so FETCH_LAZY now raises RuntimeException::forUnconvertibleRow() through the three result sets that fill a prototype. It continues to work on AbstractResultSet, which promises nothing about the row's type. HydratingResultSet::current() gained enough work to cross the halstead threshold, so the conversion and hydration move into hydrateRow(). src/ResultSet is at 100% line coverage and 35 baselined findings. --- analyzer-baseline.toml | 6 -- src/ResultSet/AbstractResultSet.php | 34 ++++++++++++ src/ResultSet/Exception/RuntimeException.php | 7 +++ src/ResultSet/HydratingResultSet.php | 24 +++++++- src/ResultSet/ResultSet.php | 14 ++++- src/ResultSet/RowPrototypeResultSet.php | 14 ++++- .../Exception/RuntimeExceptionTest.php | 16 ++++++ .../unit/ResultSet/HydratingResultSetTest.php | 31 +++++++++++ .../unit/ResultSet/ResultSetFetchModeTest.php | 24 ++++++-- .../ResultSet/RowPrototypeResultSetTest.php | 55 +++++++++++++++++++ 10 files changed, 210 insertions(+), 15 deletions(-) diff --git a/analyzer-baseline.toml b/analyzer-baseline.toml index 06c340ad..052f7885 100644 --- a/analyzer-baseline.toml +++ b/analyzer-baseline.toml @@ -1602,12 +1602,6 @@ code = "class-must-be-final" message = 'Class `PhpDb\ResultSet\HydratingResultSet` should be declared `final`.' count = 1 -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "mixed-assignment" -message = "Assigning `mixed` type to a variable may lead to unexpected behavior." -count = 1 - [[issues]] file = "src/ResultSet/HydratingResultSet.php" code = "mixed-return-statement" diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index 571908ed..e3954d6c 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -19,8 +19,11 @@ use function array_key_exists; use function count; use function current; +use function get_debug_type; +use function get_object_vars; use function is_array; use function is_object; +use function iterator_to_array; use function reset; /** @@ -320,4 +323,35 @@ protected function dataSource(): Iterator return $this->dataSource; } + + /** + * Reduce an object row to the array a prototype can be filled from. + * + * A row that carries its values as elements rather than properties, ArrayObject + * being the common case, is read through its iterator. Everything else is reduced + * with get_object_vars(), which sees only public properties from out here; a plain + * cast would instead yield mangled keys for anything private. + * + * A row that exposes nothing either way cannot fill a prototype at all. PDORow, + * which FETCH_LAZY yields, is the case that matters: it resolves columns through + * __get() and reduces to an empty array, so converting it would quietly drop every + * column. Such a row is rejected rather than emptied. + * + * @return array + * + * @throws RuntimeException + */ + protected function rowToArray(object $row): array + { + if ($row instanceof Traversable) { + return iterator_to_array($row); + } + + $data = get_object_vars($row); + if ([] === $data) { + throw RuntimeException::forUnconvertibleRow(get_debug_type($row)); + } + + return $data; + } } diff --git a/src/ResultSet/Exception/RuntimeException.php b/src/ResultSet/Exception/RuntimeException.php index 20a063eb..c23f36ea 100644 --- a/src/ResultSet/Exception/RuntimeException.php +++ b/src/ResultSet/Exception/RuntimeException.php @@ -21,6 +21,13 @@ public static function forUnbufferedIteration(): self final public const string UNHYDRATABLE_ROW = 'Cannot extract a row of type "%s"; the hydrator requires an object'; + final public const string UNCONVERTIBLE_ROW = 'A row of type "%s" exposes no properties and cannot fill a row prototype; select a fetch mode that yields arrays'; + + public static function forUnconvertibleRow(string $type): self + { + return new self(sprintf(self::UNCONVERTIBLE_ROW, $type)); + } + public static function forUnhydratableRow(string $type): self { return new self(sprintf(self::UNHYDRATABLE_ROW, $type)); diff --git a/src/ResultSet/HydratingResultSet.php b/src/ResultSet/HydratingResultSet.php index 16725c1c..ffd29230 100644 --- a/src/ResultSet/HydratingResultSet.php +++ b/src/ResultSet/HydratingResultSet.php @@ -42,8 +42,7 @@ public function current(): ?object return $this->buffer[$this->position]; } - $data = $this->dataSource()->current(); - $current = is_array($data) ? $this->getHydrator()->hydrate($data, clone $this->getRowPrototype()) : null; + $current = $this->hydrateRow($this->dataSource()->current()); if (is_array($this->buffer)) { $this->buffer[$this->position] = $current; @@ -116,4 +115,25 @@ public function toArray(): array return $return; } + + /** + * Hydrate one row onto a clone of the prototype. + * + * A row that is neither an array nor reducible to one carries nothing to hydrate, + * which is how FETCH_BOUND arrives. + * + * @throws RuntimeException + */ + private function hydrateRow(mixed $data): ?object + { + if (is_object($data)) { + $data = $this->rowToArray($data); + } + + if (! is_array($data)) { + return null; + } + + return $this->getHydrator()->hydrate($data, clone $this->getRowPrototype()); + } } diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index ddf510ff..2f17e34c 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -8,6 +8,7 @@ use Override; use function is_array; +use function is_object; use function is_string; class ResultSet extends AbstractResultSet implements ArrayObjectResultSetInterface @@ -30,12 +31,23 @@ public function __construct( /** * Iterator: get current item + * + * @throws Exception\RuntimeException If a row cannot fill the prototype. */ #[Override] - public function current(): array|object|null + public function current(): array|ArrayObject|null { $data = parent::current(); + if ($data instanceof ArrayObject) { + // already the shape this result set promises, so hand it back untouched + return $data; + } + + if (is_object($data)) { + $data = $this->rowToArray($data); + } + if (ResultSetReturnType::ArrayObject === $this->returnType && is_array($data)) { $ao = clone $this->getRowPrototype(); $ao->exchangeArray($data); diff --git a/src/ResultSet/RowPrototypeResultSet.php b/src/ResultSet/RowPrototypeResultSet.php index bc5b4cff..d24219ae 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -7,6 +7,7 @@ use Override; use function is_array; +use function is_object; class RowPrototypeResultSet extends AbstractResultSet implements RowPrototypeResultSetInterface { @@ -16,12 +17,23 @@ public function __construct( /** * Iterator: get current item + * + * @throws Exception\RuntimeException If a row cannot fill the prototype. */ #[Override] - public function current(): array|object|null + public function current(): array|RowPrototypeInterface|null { $data = parent::current(); + if ($data instanceof RowPrototypeInterface) { + // already the shape this result set promises, so hand it back untouched + return $data; + } + + if (is_object($data)) { + $data = $this->rowToArray($data); + } + if (is_array($data)) { return (clone $this->getRowPrototype())->populate($data); } diff --git a/test/unit/ResultSet/Exception/RuntimeExceptionTest.php b/test/unit/ResultSet/Exception/RuntimeExceptionTest.php index b8d89c08..fe9ef92a 100644 --- a/test/unit/ResultSet/Exception/RuntimeExceptionTest.php +++ b/test/unit/ResultSet/Exception/RuntimeExceptionTest.php @@ -15,6 +15,7 @@ #[CoversMethod(RuntimeException::class, 'forUnbufferedIteration')] #[CoversMethod(RuntimeException::class, 'forUninitialisedDataSource')] #[CoversMethod(RuntimeException::class, 'forUnhydratableRow')] +#[CoversMethod(RuntimeException::class, 'forUnconvertibleRow')] final class RuntimeExceptionTest extends TestCase { #[Test] @@ -32,6 +33,21 @@ public function forUnbufferedIterationReturnsTheComponentExceptionType(): void self::assertInstanceOf(ExceptionInterface::class, RuntimeException::forUnbufferedIteration()); } + #[Test] + public function forUnconvertibleRowNamesTheOffendingType(): void + { + self::assertStringContainsString( + 'PDORow', + RuntimeException::forUnconvertibleRow('PDORow')->getMessage(), + ); + } + + #[Test] + public function forUnconvertibleRowReturnsTheComponentExceptionType(): void + { + self::assertInstanceOf(ExceptionInterface::class, RuntimeException::forUnconvertibleRow('PDORow')); + } + #[Test] public function forUnhydratableRowNamesTheOffendingType(): void { diff --git a/test/unit/ResultSet/HydratingResultSetTest.php b/test/unit/ResultSet/HydratingResultSetTest.php index 35d4bcca..29d1c598 100644 --- a/test/unit/ResultSet/HydratingResultSetTest.php +++ b/test/unit/ResultSet/HydratingResultSetTest.php @@ -10,6 +10,7 @@ use Laminas\Hydrator\ArraySerializableHydrator; use Laminas\Hydrator\ClassMethodsHydrator; use Override; +use PhpDb\ResultSet\AbstractResultSet; use PhpDb\ResultSet\Exception\RuntimeException; use PhpDb\ResultSet\HydratingResultSet; use PHPUnit\Framework\Attributes\CoversMethod; @@ -18,11 +19,13 @@ use PHPUnit\Framework\TestCase; use stdClass; +#[CoversMethod(AbstractResultSet::class, 'rowToArray')] #[CoversMethod(HydratingResultSet::class, 'setObjectPrototype')] #[CoversMethod(HydratingResultSet::class, 'getObjectPrototype')] #[CoversMethod(HydratingResultSet::class, 'setHydrator')] #[CoversMethod(HydratingResultSet::class, 'getHydrator')] #[CoversMethod(HydratingResultSet::class, 'current')] +#[CoversMethod(HydratingResultSet::class, 'hydrateRow')] #[CoversMethod(HydratingResultSet::class, 'toArray')] #[CoversMethod(HydratingResultSet::class, '__construct')] #[CoversMethod(HydratingResultSet::class, 'setRowPrototype')] @@ -85,6 +88,34 @@ public function currentHasData(): void static::assertInstanceOf('ArrayObject', $obj); } + #[Test] + public function currentHydratesAnObjectRowRatherThanDiscardingIt(): void + { + $row = new stdClass(); + $row->id = 1; + $row->name = 'one'; + + $resultSet = new HydratingResultSet(new ArraySerializableHydrator(), new ArrayObject()); + $resultSet->initialize(new ArrayIterator([$row])); + + $current = $resultSet->current(); + + static::assertInstanceOf(ArrayObject::class, $current); + static::assertSame(['id' => 1, 'name' => 'one'], $current->getArrayCopy()); + } + + #[Test] + public function currentRejectsARowThatExposesNothing(): void + { + $resultSet = new HydratingResultSet(new ArraySerializableHydrator(), new ArrayObject()); + $resultSet->initialize(new ArrayIterator([new stdClass()])); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage('exposes no properties'); + + $resultSet->current(); + } + #[Test] public function currentWithBufferReturnsBufferedObject(): void { diff --git a/test/unit/ResultSet/ResultSetFetchModeTest.php b/test/unit/ResultSet/ResultSetFetchModeTest.php index 9b78eb3d..8fba1fe5 100644 --- a/test/unit/ResultSet/ResultSetFetchModeTest.php +++ b/test/unit/ResultSet/ResultSetFetchModeTest.php @@ -6,15 +6,15 @@ use ArrayObject; use PDO; -use PDORow; use PhpDb\Adapter\Driver\Pdo\Result; +use PhpDb\ResultSet\AbstractResultSet; +use PhpDb\ResultSet\Exception\RuntimeException; use PhpDb\ResultSet\ResultSet; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -use stdClass; use Throwable; use function array_map; @@ -26,6 +26,7 @@ * every entry in Result::VALID_FETCH_MODES, so that a mode cannot start discarding rows * without a test noticing. */ +#[CoversMethod(AbstractResultSet::class, 'rowToArray')] #[CoversMethod(ResultSet::class, 'current')] #[Group('unit')] final class ResultSetFetchModeTest extends TestCase @@ -40,11 +41,10 @@ final class ResultSetFetchModeTest extends TestCase public static function rowYieldingModeProvider(): array { return [ - 'FETCH_LAZY hands back a PDORow' => [PDO::FETCH_LAZY, PDORow::class], 'FETCH_ASSOC fills the ArrayObject prototype' => [PDO::FETCH_ASSOC, ArrayObject::class], 'FETCH_NUM fills the ArrayObject prototype' => [PDO::FETCH_NUM, ArrayObject::class], 'FETCH_BOTH fills the ArrayObject prototype' => [PDO::FETCH_BOTH, ArrayObject::class], - 'FETCH_OBJ hands back a stdClass' => [PDO::FETCH_OBJ, stdClass::class], + 'FETCH_OBJ fills the ArrayObject prototype' => [PDO::FETCH_OBJ, ArrayObject::class], 'FETCH_NAMED fills the ArrayObject prototype' => [PDO::FETCH_NAMED, ArrayObject::class], 'FETCH_KEY_PAIR fills the ArrayObject prototype' => [PDO::FETCH_KEY_PAIR, ArrayObject::class], 'FETCH_PROPS_LATE is a flag, so PDO falls back' => [PDO::FETCH_PROPS_LATE, ArrayObject::class], @@ -119,12 +119,26 @@ public function fetchModesNeedingExtraArgumentsCannotBeDriven(int $fetchMode): v $this->rowsFor($fetchMode); } + /** + * FETCH_LAZY yields a PDORow, which resolves its columns through __get() and so + * reduces to an empty array. There is no way to fill a prototype from it, and + * emptying it silently is what this component used to do. + */ + #[Test] + public function lazyFetchModeIsRejectedRatherThanEmptied(): void + { + self::expectException(RuntimeException::class); + self::expectExceptionMessage('exposes no properties'); + + $this->rowsFor(PDO::FETCH_LAZY); + } + #[Test] public function objectRowsKeepTheirColumnValues(): void { $rows = $this->rowsFor(PDO::FETCH_OBJ); - static::assertSame([1, 2], array_map(static fn(stdClass $row): int => (int) $row->id, $rows)); + static::assertSame([1, 2], array_map(static fn(ArrayObject $row): int => (int) $row['id'], $rows)); } protected function setUp(): void diff --git a/test/unit/ResultSet/RowPrototypeResultSetTest.php b/test/unit/ResultSet/RowPrototypeResultSetTest.php index 4a588c78..2fd98387 100644 --- a/test/unit/ResultSet/RowPrototypeResultSetTest.php +++ b/test/unit/ResultSet/RowPrototypeResultSetTest.php @@ -5,18 +5,73 @@ namespace PhpDbTest\ResultSet; use ArrayIterator; +use ArrayObject; +use PhpDb\ResultSet\AbstractResultSet; +use PhpDb\ResultSet\Exception\RuntimeException; use PhpDb\ResultSet\RowPrototypeInterface; use PhpDb\ResultSet\RowPrototypeResultSet; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; +use stdClass; +#[CoversMethod(AbstractResultSet::class, 'rowToArray')] #[CoversMethod(RowPrototypeResultSet::class, 'current')] #[CoversMethod(RowPrototypeResultSet::class, 'toArray')] #[Group('unit')] final class RowPrototypeResultSetTest extends TestCase { + #[Test] + public function currentPopulatesThePrototypeFromAnObjectRow(): void + { + $resultSet = new RowPrototypeResultSet($this->createRowPrototype()); + $row = new stdClass(); + $row->id = 1; + $row->name = 'one'; + $resultSet->initialize(new ArrayIterator([$row])); + + $current = $resultSet->current(); + + static::assertInstanceOf(RowPrototypeInterface::class, $current); + static::assertSame(['id' => 1, 'name' => 'one'], $current->toArray()); + } + + #[Test] + public function currentPopulatesThePrototypeFromARowCarryingItsValuesAsElements(): void + { + $resultSet = new RowPrototypeResultSet($this->createRowPrototype()); + $resultSet->initialize(new ArrayIterator([new ArrayObject(['id' => 1, 'name' => 'one'])])); + + $current = $resultSet->current(); + + static::assertInstanceOf(RowPrototypeInterface::class, $current); + static::assertSame(['id' => 1, 'name' => 'one'], $current->toArray()); + } + + #[Test] + public function currentRejectsARowThatExposesNothing(): void + { + $resultSet = new RowPrototypeResultSet($this->createRowPrototype()); + $resultSet->initialize(new ArrayIterator([new stdClass()])); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage('exposes no properties'); + + $resultSet->current(); + } + + #[Test] + public function currentReturnsARowThatIsAlreadyAPrototypeUntouched(): void + { + $prototype = $this->createRowPrototype(); + $row = $this->createRowPrototype()->populate(['id' => 1]); + $resultSet = new RowPrototypeResultSet($prototype); + $resultSet->initialize(new ArrayIterator([$row])); + + static::assertSame($row, $resultSet->current()); + } + #[Test] public function currentReturnsDataUnchangedWhenNotArray(): void { From 1ecfa9bd0ab1312ab4e85912fe1dabf9e02522a8 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 30 Sep 2026 10:59:44 +1000 Subject: [PATCH 03/17] Drop the dead array arm from RowPrototypeResultSet::current() Every array row fills the prototype and every object row is either already a prototype or reduced to an array that does, so the method could only ever return a prototype or null. The declared `array` arm was unreachable. This is the one return type in the component that narrows against 0.6.x. A subclass overriding current() with the wider declaration would no longer satisfy covariance. --- analyzer-baseline.toml | 6 ------ src/ResultSet/RowPrototypeResultSet.php | 6 ++++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/analyzer-baseline.toml b/analyzer-baseline.toml index 052f7885..6ee1f4c5 100644 --- a/analyzer-baseline.toml +++ b/analyzer-baseline.toml @@ -1656,12 +1656,6 @@ code = "class-must-be-final" message = 'Class `PhpDb\ResultSet\RowPrototypeResultSet` should be declared `final`.' count = 1 -[[issues]] -file = "src/ResultSet/RowPrototypeResultSet.php" -code = "imprecise-type" -message = "Type `array` in return type of `current` is imprecise, equivalent to `array`." -count = 1 - [[issues]] file = "src/RowGateway/AbstractRowGateway.php" code = "imprecise-type" diff --git a/src/ResultSet/RowPrototypeResultSet.php b/src/ResultSet/RowPrototypeResultSet.php index d24219ae..5fd541a9 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -21,7 +21,7 @@ public function __construct( * @throws Exception\RuntimeException If a row cannot fill the prototype. */ #[Override] - public function current(): array|RowPrototypeInterface|null + public function current(): ?RowPrototypeInterface { $data = parent::current(); @@ -38,7 +38,9 @@ public function current(): array|RowPrototypeInterface|null return (clone $this->getRowPrototype())->populate($data); } - return $data; + // every array fills the prototype and every object is reduced to one, so + // anything reaching here is the exhausted-or-unusable row the parent nulls + return null; } /** {@inheritDoc} */ From cd59a9aaaded71e2638b7856910e0e64100b8e6a Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 30 Sep 2026 11:36:11 +1000 Subject: [PATCH 04/17] Narrow ArrayResultSet::current() to ?array and drop added commentary ArrayResultSet had no current() override, so it inherited array|object|null from the base class and could hand back an object despite being the result set that exists to yield arrays. It now declares ?array and reduces an object row through rowToArray(), on the same terms as the three prototype-filling result sets. Five cases cover the method: an array row, an object row, a row carrying its values as elements, a row that exposes nothing, and an exhausted data source. Also removes the inline comments and docblock prose added earlier in this branch. The @throws and @return tags stay, since mago analyze reports unhandled-thrown-type and imprecise-type without them. --- src/ResultSet/AbstractResultSet.php | 18 ------- src/ResultSet/ArrayResultSet.php | 21 ++++++++ src/ResultSet/HydratingResultSet.php | 3 -- src/ResultSet/ResultSet.php | 1 - src/ResultSet/RowPrototypeResultSet.php | 3 -- test/unit/ResultSet/ArrayResultSetTest.php | 59 ++++++++++++++++++++++ 6 files changed, 80 insertions(+), 25 deletions(-) diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index e3954d6c..5f616302 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -56,10 +56,6 @@ abstract class AbstractResultSet implements ResultSetInterface /** * Resolve an IteratorAggregate chain down to the Iterator it wraps. * - * IteratorAggregate::getIterator() is declared to return Traversable, so it may - * hand back another aggregate. Anything that never bottoms out in an Iterator is - * rejected rather than stored, since the result set can only iterate an Iterator. - * * @throws InvalidArgumentException * @throws Exception If the data source raises one while handing over its iterator. */ @@ -78,9 +74,6 @@ private static function resolveIterator(Traversable $dataSource): Iterator return $dataSource; } - // Userland cannot implement Traversable without Iterator or IteratorAggregate, and the - // internal classes that do (PDOStatement, DOMNodeList, DatePeriod) are all aggregates - // the loop above has already unwrapped. Kept so the return type cannot be violated. // @codeCoverageIgnoreStart throw InvalidArgumentException::forNonIteratorDataSource($dataSource::class); @@ -233,7 +226,6 @@ public function initialize(iterable $dataSource): ResultSetInterface return $this; } - // the array is safe to measure, but its first row can be any shape at all $first = current($dataSource); $this->fieldCount = is_array($first) || $first instanceof Countable ? count($first) : 0; reset($dataSource); @@ -327,16 +319,6 @@ protected function dataSource(): Iterator /** * Reduce an object row to the array a prototype can be filled from. * - * A row that carries its values as elements rather than properties, ArrayObject - * being the common case, is read through its iterator. Everything else is reduced - * with get_object_vars(), which sees only public properties from out here; a plain - * cast would instead yield mangled keys for anything private. - * - * A row that exposes nothing either way cannot fill a prototype at all. PDORow, - * which FETCH_LAZY yields, is the case that matters: it resolves columns through - * __get() and reduces to an empty array, so converting it would quietly drop every - * column. Such a row is rejected rather than emptied. - * * @return array * * @throws RuntimeException diff --git a/src/ResultSet/ArrayResultSet.php b/src/ResultSet/ArrayResultSet.php index 2f93eb4e..e1131452 100644 --- a/src/ResultSet/ArrayResultSet.php +++ b/src/ResultSet/ArrayResultSet.php @@ -6,8 +6,29 @@ use Override; +use function is_object; + class ArrayResultSet extends AbstractResultSet { + /** + * Iterator: get current item + * + * @return array|null + * + * @throws Exception\RuntimeException + */ + #[Override] + public function current(): ?array + { + $data = parent::current(); + + if (is_object($data)) { + return $this->rowToArray($data); + } + + return $data; + } + /** {@inheritDoc} */ #[Override] public function toArray(): array diff --git a/src/ResultSet/HydratingResultSet.php b/src/ResultSet/HydratingResultSet.php index ffd29230..66a17c80 100644 --- a/src/ResultSet/HydratingResultSet.php +++ b/src/ResultSet/HydratingResultSet.php @@ -119,9 +119,6 @@ public function toArray(): array /** * Hydrate one row onto a clone of the prototype. * - * A row that is neither an array nor reducible to one carries nothing to hydrate, - * which is how FETCH_BOUND arrives. - * * @throws RuntimeException */ private function hydrateRow(mixed $data): ?object diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index 2f17e34c..60b85aaa 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -40,7 +40,6 @@ public function current(): array|ArrayObject|null $data = parent::current(); if ($data instanceof ArrayObject) { - // already the shape this result set promises, so hand it back untouched return $data; } diff --git a/src/ResultSet/RowPrototypeResultSet.php b/src/ResultSet/RowPrototypeResultSet.php index 5fd541a9..e34e6d21 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -26,7 +26,6 @@ public function current(): ?RowPrototypeInterface $data = parent::current(); if ($data instanceof RowPrototypeInterface) { - // already the shape this result set promises, so hand it back untouched return $data; } @@ -38,8 +37,6 @@ public function current(): ?RowPrototypeInterface return (clone $this->getRowPrototype())->populate($data); } - // every array fills the prototype and every object is reduced to one, so - // anything reaching here is the exhausted-or-unusable row the parent nulls return null; } diff --git a/test/unit/ResultSet/ArrayResultSetTest.php b/test/unit/ResultSet/ArrayResultSetTest.php index 75b46a9c..6cd53564 100644 --- a/test/unit/ResultSet/ArrayResultSetTest.php +++ b/test/unit/ResultSet/ArrayResultSetTest.php @@ -4,16 +4,75 @@ namespace PhpDbTest\ResultSet; +use ArrayIterator; +use ArrayObject; +use PhpDb\ResultSet\AbstractResultSet; use PhpDb\ResultSet\ArrayResultSet; +use PhpDb\ResultSet\Exception\RuntimeException; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; +use stdClass; +#[CoversMethod(AbstractResultSet::class, 'rowToArray')] +#[CoversMethod(ArrayResultSet::class, 'current')] #[CoversMethod(ArrayResultSet::class, 'toArray')] #[Group('unit')] final class ArrayResultSetTest extends TestCase { + #[Test] + public function currentReducesAnObjectRowToAnArray(): void + { + $row = new stdClass(); + $row->id = 1; + $row->name = 'one'; + + $resultSet = new ArrayResultSet(); + $resultSet->initialize(new ArrayIterator([$row])); + + static::assertSame(['id' => 1, 'name' => 'one'], $resultSet->current()); + } + + #[Test] + public function currentReducesARowCarryingItsValuesAsElements(): void + { + $resultSet = new ArrayResultSet(); + $resultSet->initialize(new ArrayIterator([new ArrayObject(['id' => 1, 'name' => 'one'])])); + + static::assertSame(['id' => 1, 'name' => 'one'], $resultSet->current()); + } + + #[Test] + public function currentRejectsARowThatExposesNothing(): void + { + $resultSet = new ArrayResultSet(); + $resultSet->initialize(new ArrayIterator([new stdClass()])); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage('exposes no properties'); + + $resultSet->current(); + } + + #[Test] + public function currentReturnsAnArrayRowUntouched(): void + { + $resultSet = new ArrayResultSet(); + $resultSet->initialize([['id' => 1, 'name' => 'one']]); + + static::assertSame(['id' => 1, 'name' => 'one'], $resultSet->current()); + } + + #[Test] + public function currentReturnsNullOnceTheRowsAreExhausted(): void + { + $resultSet = new ArrayResultSet(); + $resultSet->initialize(new ArrayIterator([])); + + static::assertNull($resultSet->current()); + } + #[Test] public function toArrayReturnsRowsAsProvided(): void { From 6c556bb790b0f7f84eeb5bbd66c0c7d9b58b70c4 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 30 Sep 2026 11:52:42 +1000 Subject: [PATCH 05/17] Honour ResultSetReturnType::Prototype in ResultSet::current() The case was added in f3942077 with no consumer. Nothing read it, so a caller selecting it silently got Array behaviour: the constructor accepted it, getReturnType() reported it, current() checked only for ArrayObject and let everything else fall through to the raw row. Every observable signal said it worked. Prototype is the successor name for ArrayObject. It follows the rename already in flight elsewhere in the class, where TYPE_ARRAYOBJECT gives way to the enum and setArrayObjectPrototype() to setRowPrototype(); for ResultSet the ArrayObject and the row prototype are the same object. Both cases now fill the prototype. The decision is resolved once in the constructor rather than on each current() call, since the return type cannot change after construction. Both properties are readonly so that a later setter cannot leave the derived flag stale. Per-row cost drops to a bool read, below the enum comparison it replaces. The match is exhaustive. A fourth case with no decision here raises UnhandledMatchError at construction, before any row is touched, rather than falling through current() unnoticed. returnTypeProvider() covers every case and asserts its own completeness against ResultSetReturnType::cases(), so the same omission fails the suite too. --- src/ResultSet/ResultSet.php | 18 +++++++-- .../ResultSet/ResultSetIntegrationTest.php | 37 ++++++++++++++++++- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index 60b85aaa..29437afa 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -17,7 +17,15 @@ class ResultSet extends AbstractResultSet implements ArrayObjectResultSetInterfa public const TYPE_ARRAYOBJECT = 'arrayobject'; public const TYPE_ARRAY = 'array'; - private ResultSetReturnType $returnType; + private readonly ResultSetReturnType $returnType; + + /** + * Whether the selected return type fills the row prototype, resolved once. + * + * The match is exhaustive so that a return type added without a decision here + * fails at construction rather than falling through current() unnoticed. + */ + private readonly bool $fillsRowPrototype; public function __construct( ResultSetReturnType|string $returnType = ResultSetReturnType::ArrayObject, @@ -26,7 +34,11 @@ public function __construct( ArrayObject::ARRAY_AS_PROPS, ), ) { - $this->returnType = is_string($returnType) ? ResultSetReturnType::from($returnType) : $returnType; + $this->returnType = is_string($returnType) ? ResultSetReturnType::from($returnType) : $returnType; + $this->fillsRowPrototype = match ($this->returnType) { + ResultSetReturnType::ArrayObject, ResultSetReturnType::Prototype => true, + ResultSetReturnType::Array => false, + }; } /** @@ -47,7 +59,7 @@ public function current(): array|ArrayObject|null $data = $this->rowToArray($data); } - if (ResultSetReturnType::ArrayObject === $this->returnType && is_array($data)) { + if ($this->fillsRowPrototype && is_array($data)) { $ao = clone $this->getRowPrototype(); $ao->exchangeArray($data); diff --git a/test/unit/ResultSet/ResultSetIntegrationTest.php b/test/unit/ResultSet/ResultSetIntegrationTest.php index 15189c73..6bc39324 100644 --- a/test/unit/ResultSet/ResultSetIntegrationTest.php +++ b/test/unit/ResultSet/ResultSetIntegrationTest.php @@ -24,6 +24,8 @@ use stdClass; use TypeError; +use function count; +use function get_debug_type; use function is_array; use function random_int; use function var_export; @@ -44,7 +46,6 @@ final class ResultSetIntegrationTest extends TestCase { protected ResultSet $resultSet; - /** @psalm-return array */ public static function invalidReturnTypes(): array { return [ @@ -57,6 +58,30 @@ public static function invalidReturnTypes(): array ]; } + /** @psalm-return array */ + /** + * Every case in ResultSetReturnType and the type a row arrives as, so a case + * cannot be added without deciding what current() does with it. + * + * @return array + */ + public static function returnTypeProvider(): array + { + $cases = [ + 'ArrayObject fills the row prototype' => [ResultSetReturnType::ArrayObject, ArrayObject::class], + 'Prototype fills the row prototype' => [ResultSetReturnType::Prototype, ArrayObject::class], + 'Array hands back the raw row' => [ResultSetReturnType::Array, 'array'], + ]; + + self::assertSame( + count(ResultSetReturnType::cases()), + count($cases), + 'every ResultSetReturnType case needs a row type asserted here', + ); + + return $cases; + } + /** * @throws Exception * @throws \Exception @@ -217,6 +242,16 @@ public function dataSourceIsNullByDefault(): void static::assertNull($this->resultSet->getDataSource()); } + #[Test] + #[DataProvider('returnTypeProvider')] + public function everyReturnTypeYieldsItsRowType(ResultSetReturnType $returnType, string $expected): void + { + $resultSet = new ResultSet($returnType); + $resultSet->initialize([['id' => 1, 'name' => 'one']]); + + static::assertSame($expected, get_debug_type($resultSet->current())); + } + #[Test] public function fieldCountIsZeroWithNoDataSourcePresent(): void { From e135d64f3419c8a3fb6b74f440129b43c017ea82 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 30 Sep 2026 14:26:34 +1000 Subject: [PATCH 06/17] Refuse rows a result set cannot read - current() moves to each concrete set; mapRow() shapes the row - buffering moves behind RowBuffer, with __clone to unshare it - ObjectResultSet serves FETCH_OBJ and FETCH_LAZY rows - rows are typed through a TRow template - the data source narrows to ?Iterator --- analyzer-baseline.toml | 130 +---------- src/Exception/ValueError.php | 9 + src/ResultSet/AbstractResultSet.php | 206 +++++++++++------- src/ResultSet/ArrayResultSet.php | 27 ++- src/ResultSet/Exception/RuntimeException.php | 16 -- src/ResultSet/Exception/ValueError.php | 45 ++++ src/ResultSet/HydratingResultSet.php | 59 ++--- src/ResultSet/ObjectResultSet.php | 92 ++++++++ src/ResultSet/ResultSet.php | 62 ++++-- src/ResultSet/ResultSetInterface.php | 4 +- src/ResultSet/RowBuffer.php | 132 +++++++++++ src/ResultSet/RowBufferState.php | 31 +++ src/ResultSet/RowPrototypeInterface.php | 4 + src/ResultSet/RowPrototypeResultSet.php | 50 +++-- .../AbstractResultSetIntegrationTest.php | 12 +- test/unit/ResultSet/AbstractResultSetTest.php | 45 +++- test/unit/ResultSet/ArrayResultSetTest.php | 31 +-- .../Exception/RuntimeExceptionTest.php | 32 --- .../ResultSet/Exception/ValueErrorTest.php | 107 +++++++++ .../unit/ResultSet/HydratingResultSetTest.php | 23 +- test/unit/ResultSet/ObjectResultSetTest.php | 137 ++++++++++++ .../unit/ResultSet/ResultSetFetchModeTest.php | 136 ++++++++---- .../ResultSet/ResultSetIntegrationTest.php | 4 +- test/unit/ResultSet/RowBufferTest.php | 172 +++++++++++++++ .../ResultSet/RowPrototypeResultSetTest.php | 50 +++-- .../TestAsset/PassThroughResultSet.php | 44 ++++ 26 files changed, 1210 insertions(+), 450 deletions(-) create mode 100644 src/Exception/ValueError.php create mode 100644 src/ResultSet/Exception/ValueError.php create mode 100644 src/ResultSet/ObjectResultSet.php create mode 100644 src/ResultSet/RowBuffer.php create mode 100644 src/ResultSet/RowBufferState.php create mode 100644 test/unit/ResultSet/Exception/ValueErrorTest.php create mode 100644 test/unit/ResultSet/ObjectResultSetTest.php create mode 100644 test/unit/ResultSet/RowBufferTest.php create mode 100644 test/unit/ResultSet/TestAsset/PassThroughResultSet.php diff --git a/analyzer-baseline.toml b/analyzer-baseline.toml index 6ee1f4c5..3d105079 100644 --- a/analyzer-baseline.toml +++ b/analyzer-baseline.toml @@ -1317,7 +1317,7 @@ count = 1 [[issues]] file = "src/Container/AbstractAdapterInterfaceFactory.php" code = "possibly-null-argument" -message = 'Argument #3 of method `PhpDb\Adapter\Adapter::__construct` is possibly `null`, but parameter type `PhpDb\ResultSet\ResultSetInterface` does not accept it.' +message = 'Argument #3 of method `PhpDb\Adapter\Adapter::__construct` is possibly `null`, but parameter type `PhpDb\ResultSet\ResultSetInterface` does not accept it.' count = 1 [[issues]] @@ -1419,7 +1419,7 @@ count = 1 [[issues]] file = "src/Container/AdapterInterfaceFactory.php" code = "possibly-null-argument" -message = 'Argument #3 of method `PhpDb\Adapter\Adapter::__construct` is possibly `null`, but parameter type `PhpDb\ResultSet\ResultSetInterface` does not accept it.' +message = 'Argument #3 of method `PhpDb\Adapter\Adapter::__construct` is possibly `null`, but parameter type `PhpDb\ResultSet\ResultSetInterface` does not accept it.' count = 1 [[issues]] @@ -1512,24 +1512,12 @@ code = "missing-constant-type" message = "Class constant `DEFAULT_SCHEMA` is missing a type hint." count = 1 -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "imprecise-type" -message = "Type `array` in return type of `current` is imprecise, equivalent to `array`." -count = 1 - [[issues]] file = "src/ResultSet/AbstractResultSet.php" code = "incompatible-return-type" message = 'Return type `int|null` of `PhpDb\ResultSet\AbstractResultSet::count()` is incompatible with parent return type `int` of `Countable::count()`' count = 1 -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "invalid-return-statement" -message = 'Invalid return type for function `PhpDb\ResultSet\AbstractResultSet::dataSource`: expected `Iterator`, but found `Iterator|IteratorAggregate|PhpDb\Adapter\Driver\ResultInterface`.' -count = 1 - [[issues]] file = "src/ResultSet/AbstractResultSet.php" code = "invalid-return-statement" @@ -1546,7 +1534,7 @@ count = 1 file = "src/ResultSet/AbstractResultSet.php" code = "mixed-assignment" message = "Assigning `mixed` type to a variable may lead to unexpected behavior." -count = 3 +count = 2 [[issues]] file = "src/ResultSet/AbstractResultSet.php" @@ -1554,36 +1542,6 @@ code = "mixed-property-type-coercion" message = "A value with a less specific type `mixed` is being assigned to property `$fieldCount` (int|null)." count = 1 -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "mixed-return-statement" -message = 'Could not infer a precise return type for function `PhpDb\ResultSet\AbstractResultSet::current`. Saw type `mixed`.' -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "mixed-return-statement" -message = 'Could not infer a precise return type for function `PhpDb\ResultSet\AbstractResultSet::current`. Saw type `nonnull`.' -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "non-existent-method" -message = "Method `rewind` does not exist on type `IteratorAggregate`." -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "possibly-invalid-argument" -message = 'Possible argument type mismatch for argument #1 of `count`: expected `Countable|array`, but possibly received `Iterator|IteratorAggregate|PhpDb\Adapter\Driver\ResultInterface`.' -count = 1 - -[[issues]] -file = "src/ResultSet/ArrayResultSet.php" -code = "class-must-be-final" -message = 'Class `PhpDb\ResultSet\ArrayResultSet` should be declared `final`.' -count = 1 - [[issues]] file = "src/ResultSet/Exception/InvalidArgumentException.php" code = "class-must-be-final" @@ -1596,78 +1554,12 @@ code = "class-must-be-final" message = 'Class `PhpDb\ResultSet\Exception\RuntimeException` should be declared `final`.' count = 1 -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "class-must-be-final" -message = 'Class `PhpDb\ResultSet\HydratingResultSet` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "mixed-return-statement" -message = 'Could not infer a precise return type for function `PhpDb\ResultSet\HydratingResultSet::current`. Saw type `nonnull`.' -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "overly-wide-return-type" -message = "Declared return type `null|object` for `getobjectprototype` has unused branches: `null`." -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "class-must-be-final" -message = 'Class `PhpDb\ResultSet\ResultSet` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "imprecise-type" -message = "Type `array` in return type of `current` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "missing-constant-type" -message = "Class constant `TYPE_ARRAYOBJECT` is missing a type hint." -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "missing-constant-type" -message = "Class constant `TYPE_ARRAY` is missing a type hint." -count = 1 - -[[issues]] -file = "src/ResultSet/RowPrototypeInterface.php" -code = "imprecise-type" -message = "Type `array` in parameter `$data` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/ResultSet/RowPrototypeInterface.php" -code = "imprecise-type" -message = "Type `array` in return type of `toArray` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/ResultSet/RowPrototypeResultSet.php" -code = "class-must-be-final" -message = 'Class `PhpDb\ResultSet\RowPrototypeResultSet` 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 parameter `$rowData` is imprecise, equivalent to `array`." -count = 1 - [[issues]] file = "src/RowGateway/AbstractRowGateway.php" code = "imprecise-type" @@ -1686,12 +1578,6 @@ code = "imprecise-type" message = "Type `array` in property `$primaryKeyData` is imprecise, equivalent to `array`." count = 1 -[[issues]] -file = "src/RowGateway/AbstractRowGateway.php" -code = "imprecise-type" -message = "Type `array` in return type of `toArray` is imprecise, equivalent to `array`." -count = 1 - [[issues]] file = "src/RowGateway/AbstractRowGateway.php" code = "incompatible-parameter-name" @@ -5061,7 +4947,7 @@ 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`.' +message = 'Invalid return type for function `PhpDb\TableGateway\AbstractTableGateway::executeSelect`: expected `PhpDb\ResultSet\ResultSetInterface`, but found `PhpDb\ResultSet\ResultSetInterface|null`.' count = 1 [[issues]] @@ -5073,7 +4959,7 @@ 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`.' +message = 'Invalid return type for function `PhpDb\TableGateway\AbstractTableGateway::getResultSetPrototype`: expected `PhpDb\ResultSet\ResultSetInterface`, but found `PhpDb\ResultSet\ResultSetInterface|null`.' count = 1 [[issues]] @@ -5193,7 +5079,7 @@ 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`).' +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]] @@ -5205,7 +5091,7 @@ 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`).' +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]] @@ -5235,7 +5121,7 @@ count = 25 [[issues]] file = "src/TableGateway/AbstractTableGateway.php" code = "possibly-invalid-clone" -message = 'Expression of type `PhpDb\ResultSet\ResultSetInterface|null` might not be a cloneable object.' +message = 'Expression of type `PhpDb\ResultSet\ResultSetInterface|null` might not be a cloneable object.' count = 1 [[issues]] diff --git a/src/Exception/ValueError.php b/src/Exception/ValueError.php new file mode 100644 index 00000000..699dbce5 --- /dev/null +++ b/src/Exception/ValueError.php @@ -0,0 +1,9 @@ + */ abstract class AbstractResultSet implements ResultSetInterface { @@ -35,19 +37,19 @@ abstract class AbstractResultSet implements ResultSetInterface private const int MAX_ITERATOR_DEPTH = 8; /** - * if -1, datasource is already buffered - * if -2, implicitly disabling buffering in ResultSet - * if false, explicitly disabled - * if null, default state - nothing, but can buffer until iteration started - * if array, already buffering + * Created on first use, so that a subclass constructor need not call this one. * - * @var int|array|bool|null + * @var RowBuffer|null */ - protected int|array|bool|null $buffer = null; + private ?RowBuffer $buffer = null; protected ?int $count = null; - protected Iterator|IteratorAggregate|ResultInterface|null $dataSource = null; + /** + * Always an Iterator once initialize() has run: an IteratorAggregate is resolved + * before it is stored, and a ResultInterface is an Iterator in its own right. + */ + protected ?Iterator $dataSource = null; protected ?int $fieldCount = null; @@ -85,15 +87,11 @@ private static function resolveIterator(Traversable $dataSource): Iterator */ public function buffer(): ResultSetInterface { - if (-2 === $this->buffer) { - throw RuntimeException::forUnbufferedIteration(); - } + $pending = $this->rowBuffer()->isPending(); + $this->rowBuffer()->store(); - if (null === $this->buffer) { - $this->buffer = []; - if ($this->dataSource instanceof ResultInterface) { - $this->dataSource->rewind(); - } + if ($pending && $this->dataSource instanceof ResultInterface) { + $this->dataSource->rewind(); } return $this; @@ -117,43 +115,10 @@ public function count(): ?int return $this->count; } - /** - * Iterator: get current item - * - * @throws RuntimeException - */ - #[Override] - public function current(): array|object|null - { - if (-1 === $this->buffer) { - // datasource was an array when the resultset was initialized - return $this->dataSource()->current(); - } - - if (null === $this->buffer) { - $this->buffer = -2; // implicitly disable buffering from here on - } - - if ( - is_array($this->buffer) - && array_key_exists($this->position, $this->buffer) - && null !== $this->buffer[$this->position] - ) { - return $this->buffer[$this->position]; - } - - $data = $this->dataSource()->current(); - if (is_array($this->buffer)) { - $this->buffer[$this->position] = $data; - } - - return is_array($data) || is_object($data) ? $data : null; - } - /** * Get the data source used to create the result set */ - public function getDataSource(): ResultInterface|IteratorAggregate|Iterator|null + public function getDataSource(): ?Iterator { return $this->dataSource; } @@ -201,20 +166,17 @@ public function getFieldCount(): int #[Override] public function initialize(iterable $dataSource): ResultSetInterface { - // reset buffering - if (is_array($this->buffer)) { - $this->buffer = []; - } + $this->rowBuffer()->clear(); if ($dataSource instanceof ResultInterface) { $this->fieldCount = $dataSource->getFieldCount(); $this->dataSource = $dataSource; if ($dataSource->isBuffered()) { - $this->buffer = -1; + $this->rowBuffer()->useDataSource(); } - if (is_array($this->buffer)) { - $this->dataSource->rewind(); + if ($this->rowBuffer()->isStoring()) { + $dataSource->rewind(); } return $this; @@ -230,14 +192,14 @@ public function initialize(iterable $dataSource): ResultSetInterface $this->fieldCount = is_array($first) || $first instanceof Countable ? count($first) : 0; reset($dataSource); $this->dataSource = new ArrayIterator($dataSource); - $this->buffer = -1; // array's are a natural buffer + $this->rowBuffer()->useDataSource(); // an array is its own buffer return $this; } public function isBuffered(): bool { - return $this->buffer === -1 || is_array($this->buffer); + return $this->rowBuffer()->isBuffered(); } /** @@ -257,11 +219,9 @@ public function key(): int #[Override] public function next(): void { - if (null === $this->buffer) { - $this->buffer = -2; // implicitly disable buffering from here on - } + $this->rowBuffer()->startIteration(); - if (! is_array($this->buffer) || $this->position === $this->dataSource()->key()) { + if (! $this->rowBuffer()->isStoring() || $this->position === $this->dataSource()->key()) { $this->dataSource()->next(); } @@ -276,7 +236,7 @@ public function next(): void #[Override] public function rewind(): void { - if (! is_array($this->buffer)) { + if (! $this->rowBuffer()->isStoring()) { $this->dataSource()->rewind(); } @@ -291,17 +251,61 @@ public function rewind(): void #[Override] public function valid(): bool { - if ( - is_array($this->buffer) - && array_key_exists($this->position, $this->buffer) - && null !== $this->buffer[$this->position] - ) { + if ($this->rowBuffer()->has($this->position)) { return true; } return $this->dataSource()->valid(); } + /** + * Shape one raw data source row into the type this result set yields. + * + * Implementations narrow the return type to whatever their current() declares. + * + * @return TRow + */ + abstract protected function mapRow(mixed $row): mixed; + + /** + * The row at the current position, served from the buffer when one is held there. + * + * Concrete result sets call this from current() and shape the row in mapRow(), so + * that buffering lives here and row typing lives with the set that declares it. + * + * @return TRow|null + * + * @throws RuntimeException + */ + protected function currentRow(): mixed + { + $this->rowBuffer()->startIteration(); + + if ($this->rowBuffer()->has($this->position)) { + return $this->rowBuffer()->get($this->position); + } + + $dataSource = $this->dataSource(); + + /** + * The data source is asked for its row first, because a ResultInterface only + * knows whether it is still valid once it has tried to fetch one. + * + * @var mixed $row + */ + $row = $dataSource->current(); + + if (! $dataSource->valid()) { + return null; + } + + $row = $this->mapRow($row); + + $this->rowBuffer()->put($this->position, $row); + + return $row; + } + /** * The data source, once initialize() has supplied one. * @@ -309,7 +313,7 @@ public function valid(): bool */ protected function dataSource(): Iterator { - if (! $this->dataSource instanceof Iterator) { + if (null === $this->dataSource) { throw RuntimeException::forUninitialisedDataSource(); } @@ -317,23 +321,59 @@ protected function dataSource(): Iterator } /** - * Reduce an object row to the array a prototype can be filled from. + * The row data a prototype can be filled from. + * + * Only the types a result set may safely read are accepted; anything else is the + * caller's own object, which this component refuses to reshape. * * @return array * - * @throws RuntimeException + * @throws ValueError If the row is neither an array nor an ArrayObject. */ - protected function rowToArray(object $row): array + protected function getArrayData(mixed $row): array { - if ($row instanceof Traversable) { - return iterator_to_array($row); + if (is_array($row)) { + return $row; + } + + if ($row instanceof ArrayObject) { + return $row->getArrayCopy(); } - $data = get_object_vars($row); - if ([] === $data) { - throw RuntimeException::forUnconvertibleRow(get_debug_type($row)); + throw $this->unsupportedRowError($row); + } + + /** + * The rows this result set holds on to. + * + * @return RowBuffer + */ + protected function rowBuffer(): RowBuffer + { + return $this->buffer ??= new RowBuffer(); + } + + /** + * The error describing a row this result set cannot read. + * + * Overridden by a result set that accepts more than row data, so that the message + * names everything it would have taken. + */ + protected function unsupportedRowError(mixed $row): ValueError + { + return ValueError::forRowThatIsNotArrayData(get_debug_type($row), static::class); + } + + /** + * A result set is routinely cloned from a prototype, so the rows one holds must + * never be shared with the next. + */ + public function __clone(): void + { + if (null === $this->buffer) { + return; } - return $data; + $this->buffer = clone $this->buffer; } } diff --git a/src/ResultSet/ArrayResultSet.php b/src/ResultSet/ArrayResultSet.php index e1131452..d87af4d0 100644 --- a/src/ResultSet/ArrayResultSet.php +++ b/src/ResultSet/ArrayResultSet.php @@ -6,8 +6,11 @@ use Override; -use function is_object; - +/** + * @api + * + * @extends AbstractResultSet> + */ class ArrayResultSet extends AbstractResultSet { /** @@ -16,17 +19,12 @@ class ArrayResultSet extends AbstractResultSet * @return array|null * * @throws Exception\RuntimeException + * @throws Exception\ValueError If a row is not row data. */ #[Override] public function current(): ?array { - $data = parent::current(); - - if (is_object($data)) { - return $this->rowToArray($data); - } - - return $data; + return $this->currentRow(); } /** {@inheritDoc} */ @@ -40,4 +38,15 @@ public function toArray(): array return $return; } + + /** + * @return array + * + * @throws Exception\ValueError If the row is not row data. + */ + #[Override] + protected function mapRow(mixed $row): array + { + return $this->getArrayData($row); + } } diff --git a/src/ResultSet/Exception/RuntimeException.php b/src/ResultSet/Exception/RuntimeException.php index c23f36ea..57b6553c 100644 --- a/src/ResultSet/Exception/RuntimeException.php +++ b/src/ResultSet/Exception/RuntimeException.php @@ -6,8 +6,6 @@ use PhpDb\Exception; -use function sprintf; - class RuntimeException extends Exception\RuntimeException { final public const string UNBUFFERED_ITERATION = 'Buffering must be enabled before iteration is started'; @@ -19,20 +17,6 @@ public static function forUnbufferedIteration(): self return new self(self::UNBUFFERED_ITERATION); } - final public const string UNHYDRATABLE_ROW = 'Cannot extract a row of type "%s"; the hydrator requires an object'; - - final public const string UNCONVERTIBLE_ROW = 'A row of type "%s" exposes no properties and cannot fill a row prototype; select a fetch mode that yields arrays'; - - public static function forUnconvertibleRow(string $type): self - { - return new self(sprintf(self::UNCONVERTIBLE_ROW, $type)); - } - - public static function forUnhydratableRow(string $type): self - { - return new self(sprintf(self::UNHYDRATABLE_ROW, $type)); - } - public static function forUninitialisedDataSource(): self { return new self(self::UNINITIALISED_DATA_SOURCE); diff --git a/src/ResultSet/Exception/ValueError.php b/src/ResultSet/Exception/ValueError.php new file mode 100644 index 00000000..c9313dd7 --- /dev/null +++ b/src/ResultSet/Exception/ValueError.php @@ -0,0 +1,45 @@ + + */ class HydratingResultSet extends AbstractResultSet implements HydratingResultSetInterface { public function __construct( @@ -26,29 +26,12 @@ public function __construct( * Iterator: get current item * * @throws RuntimeException + * @throws Exception\ValueError If a row is not row data. */ #[Override] public function current(): ?object { - if (null === $this->buffer) { - $this->buffer = -2; // implicitly disable buffering from here on - } - - if ( - is_array($this->buffer) - && array_key_exists($this->position, $this->buffer) - && null !== $this->buffer[$this->position] - ) { - return $this->buffer[$this->position]; - } - - $current = $this->hydrateRow($this->dataSource()->current()); - - if (is_array($this->buffer)) { - $this->buffer[$this->position] = $current; - } - - return $current; + return $this->currentRow(); } /** @@ -60,7 +43,7 @@ public function getHydrator(): HydratorInterface } /** @deprecated use getRowPrototype() */ - public function getObjectPrototype(): ?object + public function getObjectPrototype(): object { return $this->getRowPrototype(); } @@ -99,17 +82,15 @@ public function setRowPrototype( /** * Cast result set to array of arrays * - * @throws Exception\RuntimeException If any row is not castable to an array. + * @throws Exception\ValueError If any row is not row data. */ #[Override] public function toArray(): array { $return = []; - foreach ($this as $row) { - if (! is_object($row)) { - throw RuntimeException::forUnhydratableRow(get_debug_type($row)); - } + /** @var object $row Every row this set yields is hydrated, or mapRow() throws. */ + foreach ($this as $row) { $return[] = $this->getHydrator()->extract($row); } @@ -119,18 +100,14 @@ public function toArray(): array /** * Hydrate one row onto a clone of the prototype. * - * @throws RuntimeException + * @throws Exception\ValueError If the row is not row data. */ - private function hydrateRow(mixed $data): ?object + #[Override] + protected function mapRow(mixed $row): object { - if (is_object($data)) { - $data = $this->rowToArray($data); - } - - if (! is_array($data)) { - return null; - } - - return $this->getHydrator()->hydrate($data, clone $this->getRowPrototype()); + return $this->getHydrator()->hydrate( + $this->getArrayData($row), + clone $this->getRowPrototype(), + ); } } diff --git a/src/ResultSet/ObjectResultSet.php b/src/ResultSet/ObjectResultSet.php new file mode 100644 index 00000000..7cfc8c39 --- /dev/null +++ b/src/ResultSet/ObjectResultSet.php @@ -0,0 +1,92 @@ + + */ +class ObjectResultSet extends AbstractResultSet +{ + /** + * Iterator: get current item + * + * @throws Exception\RuntimeException + * @throws Exception\ValueError If a row is not an object. + */ + #[Override] + public function current(): ?object + { + return $this->currentRow(); + } + + /** + * Cast result set to array of arrays + * + * @throws Exception\ValueError If a row exposes nothing to cast. + */ + #[Override] + public function toArray(): array + { + $return = []; + + /** @var object $row Every row this set yields is an object, or mapRow() throws. */ + foreach ($this as $row) { + $return[] = $this->rowToArray($row); + } + + return $return; + } + + /** + * @throws Exception\ValueError If the row is not an object. + */ + #[Override] + protected function mapRow(mixed $row): object + { + if (is_object($row)) { + return $row; + } + + throw Exception\ValueError::forRowThatIsNotAnObject(get_debug_type($row), static::class); + } + + /** + * Read an object row's values for toArray(), which owes its caller arrays. + * + * @return array + * + * @throws Exception\ValueError If the row exposes nothing to read. + */ + private function rowToArray(object $row): array + { + if ($row instanceof Traversable) { + return iterator_to_array($row); + } + + $data = get_object_vars($row); + + if ([] === $data) { + throw Exception\ValueError::forUnreadableRow(get_debug_type($row), static::class); + } + + return $data; + } +} diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index 29437afa..7ed76b21 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -7,15 +7,18 @@ use ArrayObject; use Override; -use function is_array; -use function is_object; use function is_string; +/** + * @api + * + * @extends AbstractResultSet|ArrayObject> + */ class ResultSet extends AbstractResultSet implements ArrayObjectResultSetInterface { /** @deprecated use ResultSetReturnType */ - public const TYPE_ARRAYOBJECT = 'arrayobject'; - public const TYPE_ARRAY = 'array'; + public const string TYPE_ARRAYOBJECT = 'arrayobject'; + public const string TYPE_ARRAY = 'array'; private readonly ResultSetReturnType $returnType; @@ -44,29 +47,15 @@ public function __construct( /** * Iterator: get current item * - * @throws Exception\RuntimeException If a row cannot fill the prototype. + * @return array|ArrayObject|null + * + * @throws Exception\RuntimeException + * @throws Exception\ValueError If a row is not row data. */ #[Override] public function current(): array|ArrayObject|null { - $data = parent::current(); - - if ($data instanceof ArrayObject) { - return $data; - } - - if (is_object($data)) { - $data = $this->rowToArray($data); - } - - if ($this->fillsRowPrototype && is_array($data)) { - $ao = clone $this->getRowPrototype(); - $ao->exchangeArray($data); - - return $ao; - } - - return $data; + return $this->currentRow(); } /** @@ -124,4 +113,31 @@ public function toArray(): array return $return; } + + /** + * An ArrayObject row is the caller's own object and is passed through untouched; + * row data fills the prototype when the return type calls for it. + * + * @return array|ArrayObject + * + * @throws Exception\ValueError If the row is neither an ArrayObject nor row data. + */ + #[Override] + protected function mapRow(mixed $row): array|ArrayObject + { + if ($row instanceof ArrayObject) { + return $row; + } + + $data = $this->getArrayData($row); + + if (! $this->fillsRowPrototype) { + return $data; + } + + $ao = clone $this->getRowPrototype(); + $ao->exchangeArray($data); + + return $ao; + } } diff --git a/src/ResultSet/ResultSetInterface.php b/src/ResultSet/ResultSetInterface.php index d06e4ed1..f5facdd9 100644 --- a/src/ResultSet/ResultSetInterface.php +++ b/src/ResultSet/ResultSetInterface.php @@ -10,7 +10,9 @@ /** * @api * - * @extends Iterator + * @template TRow + * + * @extends Iterator */ interface ResultSetInterface extends Iterator, Countable { diff --git a/src/ResultSet/RowBuffer.php b/src/ResultSet/RowBuffer.php new file mode 100644 index 00000000..aa11a84a --- /dev/null +++ b/src/ResultSet/RowBuffer.php @@ -0,0 +1,132 @@ + */ + private array $rows = []; + + private RowBufferState $state = RowBufferState::Pending; + + /** + * Forget every row held, leaving the state alone. + */ + public function clear(): void + { + $this->rows = []; + } + + /** + * The row held for a position, if one is held there. + * + * @return TRow|null + */ + public function get(int $position): mixed + { + return $this->rows[$position] ?? null; + } + + /** + * Whether a row is held for a position. + */ + public function has(int $position): bool + { + return RowBufferState::Storing === $this->state && array_key_exists($position, $this->rows); + } + + /** + * Whether rows can be read more than once, however that came about. + */ + public function isBuffered(): bool + { + return RowBufferState::Storing === $this->state || RowBufferState::Passthrough === $this->state; + } + + /** + * Whether buffering has still to be settled either way. + */ + public function isPending(): bool + { + return RowBufferState::Pending === $this->state; + } + + /** + * Whether rows are being held here as they are read. + */ + public function isStoring(): bool + { + return RowBufferState::Storing === $this->state; + } + + /** + * Hold a row for a position, if rows are being held at all. + * + * @param TRow $row + */ + public function put(int $position, mixed $row): void + { + if (! $this->isStoring()) { + return; + } + + $this->rows[$position] = $row; + } + + /** + * Settle an undecided buffer, because iteration has begun and buffer() is too late. + */ + public function startIteration(): void + { + if (! $this->isPending()) { + return; + } + + $this->state = RowBufferState::Disabled; + } + + /** + * Begin holding rows. + * + * @throws RuntimeException If iteration has already settled the buffer. + */ + public function store(): void + { + if (RowBufferState::Disabled === $this->state) { + throw RuntimeException::forUnbufferedIteration(); + } + + if ($this->isBuffered()) { + return; + } + + $this->state = RowBufferState::Storing; + $this->rows = []; + } + + /** + * Defer to a data source that can already be read more than once. + */ + public function useDataSource(): void + { + $this->state = RowBufferState::Passthrough; + $this->rows = []; + } +} diff --git a/src/ResultSet/RowBufferState.php b/src/ResultSet/RowBufferState.php new file mode 100644 index 00000000..fc55e2de --- /dev/null +++ b/src/ResultSet/RowBufferState.php @@ -0,0 +1,31 @@ + $data */ public function populate(array $data): self; /** * Current data as an array and match current RowGateway implementations. + * + * @return array */ public function toArray(): array; } diff --git a/src/ResultSet/RowPrototypeResultSet.php b/src/ResultSet/RowPrototypeResultSet.php index e34e6d21..1b957efa 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -6,9 +6,13 @@ use Override; -use function is_array; -use function is_object; +use function get_debug_type; +/** + * @api + * + * @extends AbstractResultSet + */ class RowPrototypeResultSet extends AbstractResultSet implements RowPrototypeResultSetInterface { public function __construct( @@ -18,26 +22,13 @@ public function __construct( /** * Iterator: get current item * - * @throws Exception\RuntimeException If a row cannot fill the prototype. + * @throws Exception\RuntimeException + * @throws Exception\ValueError If a row is not row data. */ #[Override] public function current(): ?RowPrototypeInterface { - $data = parent::current(); - - if ($data instanceof RowPrototypeInterface) { - return $data; - } - - if (is_object($data)) { - $data = $this->rowToArray($data); - } - - if (is_array($data)) { - return (clone $this->getRowPrototype())->populate($data); - } - - return null; + return $this->currentRow(); } /** {@inheritDoc} */ @@ -68,4 +59,27 @@ public function toArray(): array return $return; } + + /** + * A row that already satisfies the prototype interface is the caller's own object + * and is passed through untouched; row data populates a clone of the prototype. + * + * @throws Exception\ValueError If the row is neither a RowPrototypeInterface nor row data. + */ + #[Override] + protected function mapRow(mixed $row): RowPrototypeInterface + { + if ($row instanceof RowPrototypeInterface) { + return $row; + } + + return (clone $this->getRowPrototype())->populate($this->getArrayData($row)); + } + + /** {@inheritDoc} */ + #[Override] + protected function unsupportedRowError(mixed $row): Exception\ValueError + { + return Exception\ValueError::forRowThatIsNotArrayDataOrPrototype(get_debug_type($row), static::class); + } } diff --git a/test/unit/ResultSet/AbstractResultSetIntegrationTest.php b/test/unit/ResultSet/AbstractResultSetIntegrationTest.php index a7cd3d59..5a9b80b1 100644 --- a/test/unit/ResultSet/AbstractResultSetIntegrationTest.php +++ b/test/unit/ResultSet/AbstractResultSetIntegrationTest.php @@ -7,16 +7,16 @@ use Override; use PhpDb\Adapter\Driver\ResultInterface; use PhpDb\ResultSet\AbstractResultSet; +use PhpDbTest\ResultSet\TestAsset\PassThroughResultSet; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\Exception; -use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -#[CoversMethod(AbstractResultSet::class, 'current')] +#[CoversMethod(AbstractResultSet::class, 'currentRow')] final class AbstractResultSetIntegrationTest extends TestCase { - protected MockObject|AbstractResultSet $resultSet; + protected AbstractResultSet $resultSet; /** * @throws \Exception @@ -26,6 +26,7 @@ public function currentCallsDataSourceCurrentAsManyTimesWithoutBuffer(): void { $result = $this->getMockBuilder(ResultInterface::class)->getMock(); $this->resultSet->initialize($result); + $result->method('valid')->willReturn(true); $result->expects($this->exactly(3))->method('current')->willReturn(['foo' => 'bar']); // Call current() multiple times and verify data source is called each time $value1 = $this->resultSet->current(); @@ -43,6 +44,7 @@ public function currentCallsDataSourceCurrentOnceWithBuffer(): void $result = $this->getMockBuilder(ResultInterface::class)->getMock(); $this->resultSet->buffer(); $this->resultSet->initialize($result); + $result->method('valid')->willReturn(true); $result->expects($this->once())->method('current')->willReturn(['foo' => 'bar']); // Call current() multiple times and verify data source is called only once due to buffering $value1 = $this->resultSet->current(); @@ -60,8 +62,6 @@ public function currentCallsDataSourceCurrentOnceWithBuffer(): void #[Override] protected function setUp(): void { - $this->resultSet = $this->getMockBuilder(AbstractResultSet::class) - ->onlyMethods(['toArray']) - ->getMock(); + $this->resultSet = new PassThroughResultSet(); } } diff --git a/test/unit/ResultSet/AbstractResultSetTest.php b/test/unit/ResultSet/AbstractResultSetTest.php index a0312072..a8c2e76c 100644 --- a/test/unit/ResultSet/AbstractResultSetTest.php +++ b/test/unit/ResultSet/AbstractResultSetTest.php @@ -15,10 +15,10 @@ use PhpDb\Adapter\Driver\ResultInterface; use PhpDb\ResultSet\AbstractResultSet; use PhpDb\ResultSet\Exception\RuntimeException; +use PhpDbTest\ResultSet\TestAsset\PassThroughResultSet; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; -use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use TypeError; @@ -33,13 +33,15 @@ #[CoversMethod(AbstractResultSet::class, 'getFieldCount')] #[CoversMethod(AbstractResultSet::class, 'next')] #[CoversMethod(AbstractResultSet::class, 'key')] -#[CoversMethod(AbstractResultSet::class, 'current')] +#[CoversMethod(AbstractResultSet::class, '__clone')] +#[CoversMethod(AbstractResultSet::class, 'rowBuffer')] +#[CoversMethod(AbstractResultSet::class, 'currentRow')] #[CoversMethod(AbstractResultSet::class, 'valid')] #[CoversMethod(AbstractResultSet::class, 'rewind')] #[CoversMethod(AbstractResultSet::class, 'count')] final class AbstractResultSetTest extends TestCase { - protected MockObject|AbstractResultSet $resultSet; + protected AbstractResultSet $resultSet; /** * @throws Exception @@ -100,6 +102,31 @@ public function bufferIterations(): void static::assertSame(3, $data['id']); } + /** + * Sets up the fixture, for example, opens a network connection. + * This method is called before a test is executed. + */ + /** + * A table gateway clones its result set prototype for every select, so two result + * sets cloned from one prototype must not hand each other's rows back. + * + * @throws Exception + */ + #[Test] + public function cloningAResultSetDoesNotShareTheRowsItHolds(): void + { + $prototype = new PassThroughResultSet(); + $prototype->initialize(new ArrayIterator([['id' => 1]])); + $prototype->buffer(); + $prototype->current(); + + $clone = clone $prototype; + $clone->initialize(new ArrayIterator([['id' => 2]])); + + static::assertSame(['id' => 2], $clone->current()); + static::assertSame(['id' => 1], $prototype->current()); + } + #[Test] public function countReturnsCachedResult(): void { @@ -611,21 +638,15 @@ public function getIterator(): ArrayIterator static::assertTrue($resultSet->valid()); } - /** - * Sets up the fixture, for example, opens a network connection. - * This method is called before a test is executed. - */ #[Override] protected function setUp(): void { $this->resultSet = $this->createResultSetMock(); } - private function createResultSetMock(): MockObject|AbstractResultSet + private function createResultSetMock(): AbstractResultSet { - return $this->getMockBuilder(AbstractResultSet::class) - ->onlyMethods(['toArray']) - ->getMock(); + return new PassThroughResultSet(); } /** @@ -634,7 +655,7 @@ private function createResultSetMock(): MockObject|AbstractResultSet * * @throws Exception */ - private function drainIntoBuffer(): MockObject|AbstractResultSet + private function drainIntoBuffer(): AbstractResultSet { $resultSet = $this->createResultSetMock(); $resultSet->initialize(new ArrayIterator([ diff --git a/test/unit/ResultSet/ArrayResultSetTest.php b/test/unit/ResultSet/ArrayResultSetTest.php index 6cd53564..544c0ec4 100644 --- a/test/unit/ResultSet/ArrayResultSetTest.php +++ b/test/unit/ResultSet/ArrayResultSetTest.php @@ -8,39 +8,44 @@ use ArrayObject; use PhpDb\ResultSet\AbstractResultSet; use PhpDb\ResultSet\ArrayResultSet; -use PhpDb\ResultSet\Exception\RuntimeException; +use PhpDb\ResultSet\Exception\ValueError; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; use stdClass; -#[CoversMethod(AbstractResultSet::class, 'rowToArray')] +#[CoversMethod(AbstractResultSet::class, 'getArrayData')] +#[CoversMethod(AbstractResultSet::class, 'unsupportedRowError')] #[CoversMethod(ArrayResultSet::class, 'current')] +#[CoversMethod(ArrayResultSet::class, 'mapRow')] #[CoversMethod(ArrayResultSet::class, 'toArray')] #[Group('unit')] final class ArrayResultSetTest extends TestCase { #[Test] - public function currentReducesAnObjectRowToAnArray(): void + public function currentReducesARowCarryingItsValuesAsElements(): void { - $row = new stdClass(); - $row->id = 1; - $row->name = 'one'; - $resultSet = new ArrayResultSet(); - $resultSet->initialize(new ArrayIterator([$row])); + $resultSet->initialize(new ArrayIterator([new ArrayObject(['id' => 1, 'name' => 'one'])])); static::assertSame(['id' => 1, 'name' => 'one'], $resultSet->current()); } #[Test] - public function currentReducesARowCarryingItsValuesAsElements(): void + public function currentRejectsAnObjectRowRatherThanReducingIt(): void { + $row = new stdClass(); + $row->id = 1; + $row->name = 'one'; + $resultSet = new ArrayResultSet(); - $resultSet->initialize(new ArrayIterator([new ArrayObject(['id' => 1, 'name' => 'one'])])); + $resultSet->initialize(new ArrayIterator([$row])); - static::assertSame(['id' => 1, 'name' => 'one'], $resultSet->current()); + self::expectException(ValueError::class); + self::expectExceptionMessage('A row of type "stdClass"'); + + $resultSet->current(); } #[Test] @@ -49,8 +54,8 @@ public function currentRejectsARowThatExposesNothing(): void $resultSet = new ArrayResultSet(); $resultSet->initialize(new ArrayIterator([new stdClass()])); - self::expectException(RuntimeException::class); - self::expectExceptionMessage('exposes no properties'); + self::expectException(ValueError::class); + self::expectExceptionMessage('will not transform a row it did not create'); $resultSet->current(); } diff --git a/test/unit/ResultSet/Exception/RuntimeExceptionTest.php b/test/unit/ResultSet/Exception/RuntimeExceptionTest.php index fe9ef92a..a2f1ab3b 100644 --- a/test/unit/ResultSet/Exception/RuntimeExceptionTest.php +++ b/test/unit/ResultSet/Exception/RuntimeExceptionTest.php @@ -14,8 +14,6 @@ #[Group('unit')] #[CoversMethod(RuntimeException::class, 'forUnbufferedIteration')] #[CoversMethod(RuntimeException::class, 'forUninitialisedDataSource')] -#[CoversMethod(RuntimeException::class, 'forUnhydratableRow')] -#[CoversMethod(RuntimeException::class, 'forUnconvertibleRow')] final class RuntimeExceptionTest extends TestCase { #[Test] @@ -33,36 +31,6 @@ public function forUnbufferedIterationReturnsTheComponentExceptionType(): void self::assertInstanceOf(ExceptionInterface::class, RuntimeException::forUnbufferedIteration()); } - #[Test] - public function forUnconvertibleRowNamesTheOffendingType(): void - { - self::assertStringContainsString( - 'PDORow', - RuntimeException::forUnconvertibleRow('PDORow')->getMessage(), - ); - } - - #[Test] - public function forUnconvertibleRowReturnsTheComponentExceptionType(): void - { - self::assertInstanceOf(ExceptionInterface::class, RuntimeException::forUnconvertibleRow('PDORow')); - } - - #[Test] - public function forUnhydratableRowNamesTheOffendingType(): void - { - self::assertStringContainsString( - 'bool', - RuntimeException::forUnhydratableRow('bool')->getMessage(), - ); - } - - #[Test] - public function forUnhydratableRowReturnsTheComponentExceptionType(): void - { - self::assertInstanceOf(ExceptionInterface::class, RuntimeException::forUnhydratableRow('bool')); - } - #[Test] public function forUninitialisedDataSourceRendersItsTemplate(): void { diff --git a/test/unit/ResultSet/Exception/ValueErrorTest.php b/test/unit/ResultSet/Exception/ValueErrorTest.php new file mode 100644 index 00000000..b1280b11 --- /dev/null +++ b/test/unit/ResultSet/Exception/ValueErrorTest.php @@ -0,0 +1,107 @@ +getMessage(), + ); + } + + #[Test] + public function forRowThatIsNotArrayDataNamesTheAcceptedTypes(): void + { + self::assertStringContainsString( + 'which accepts an array or ArrayObject', + ValueError::forRowThatIsNotArrayData('PDORow', ResultSet::class)->getMessage(), + ); + } + + #[Test] + public function forRowThatIsNotArrayDataOrPrototypeNamesTheAcceptedTypes(): void + { + self::assertStringContainsString( + 'which accepts an array, ArrayObject or RowPrototypeInterface', + ValueError::forRowThatIsNotArrayDataOrPrototype('stdClass', RowPrototypeResultSet::class) + ->getMessage(), + ); + } + + #[Test] + public function forUnreadableRowNamesTheOffendingType(): void + { + self::assertStringContainsString( + 'A row of type "PDORow"', + ValueError::forUnreadableRow('PDORow', ResultSet::class)->getMessage(), + ); + } + + #[Test] + public function forUnreadableRowReturnsTheComponentExceptionType(): void + { + self::assertInstanceOf( + ExceptionInterface::class, + ValueError::forUnreadableRow('PDORow', ResultSet::class), + ); + } + + #[Test] + public function forUnsupportedRowNamesTheOffendingType(): void + { + self::assertStringContainsString( + 'A row of type "PDORow"', + ValueError::forRowThatIsNotArrayData('PDORow', ResultSet::class)->getMessage(), + ); + } + + #[Test] + public function forUnsupportedRowNamesTheResultSetThatRefusedIt(): void + { + self::assertStringContainsString( + ResultSet::class, + ValueError::forRowThatIsNotArrayData('PDORow', ResultSet::class)->getMessage(), + ); + } + + #[Test] + public function forUnsupportedRowRemainsCatchableAsAValueError(): void + { + self::assertInstanceOf( + GlobalValueError::class, + ValueError::forRowThatIsNotArrayData('bool', ResultSet::class), + ); + } + + #[Test] + public function forUnsupportedRowReturnsTheComponentExceptionType(): void + { + self::assertInstanceOf( + ExceptionInterface::class, + ValueError::forRowThatIsNotArrayData('bool', ResultSet::class), + ); + } +} diff --git a/test/unit/ResultSet/HydratingResultSetTest.php b/test/unit/ResultSet/HydratingResultSetTest.php index 29d1c598..3ac93c71 100644 --- a/test/unit/ResultSet/HydratingResultSetTest.php +++ b/test/unit/ResultSet/HydratingResultSetTest.php @@ -12,6 +12,7 @@ use Override; use PhpDb\ResultSet\AbstractResultSet; use PhpDb\ResultSet\Exception\RuntimeException; +use PhpDb\ResultSet\Exception\ValueError; use PhpDb\ResultSet\HydratingResultSet; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; @@ -19,13 +20,14 @@ use PHPUnit\Framework\TestCase; use stdClass; -#[CoversMethod(AbstractResultSet::class, 'rowToArray')] +#[CoversMethod(AbstractResultSet::class, 'getArrayData')] +#[CoversMethod(AbstractResultSet::class, 'unsupportedRowError')] #[CoversMethod(HydratingResultSet::class, 'setObjectPrototype')] #[CoversMethod(HydratingResultSet::class, 'getObjectPrototype')] #[CoversMethod(HydratingResultSet::class, 'setHydrator')] #[CoversMethod(HydratingResultSet::class, 'getHydrator')] #[CoversMethod(HydratingResultSet::class, 'current')] -#[CoversMethod(HydratingResultSet::class, 'hydrateRow')] +#[CoversMethod(HydratingResultSet::class, 'mapRow')] #[CoversMethod(HydratingResultSet::class, 'toArray')] #[CoversMethod(HydratingResultSet::class, '__construct')] #[CoversMethod(HydratingResultSet::class, 'setRowPrototype')] @@ -89,7 +91,7 @@ public function currentHasData(): void } #[Test] - public function currentHydratesAnObjectRowRatherThanDiscardingIt(): void + public function currentRejectsAnObjectRowRatherThanHydratingIt(): void { $row = new stdClass(); $row->id = 1; @@ -98,10 +100,10 @@ public function currentHydratesAnObjectRowRatherThanDiscardingIt(): void $resultSet = new HydratingResultSet(new ArraySerializableHydrator(), new ArrayObject()); $resultSet->initialize(new ArrayIterator([$row])); - $current = $resultSet->current(); + self::expectException(ValueError::class); + self::expectExceptionMessage('A row of type "stdClass"'); - static::assertInstanceOf(ArrayObject::class, $current); - static::assertSame(['id' => 1, 'name' => 'one'], $current->getArrayCopy()); + $resultSet->current(); } #[Test] @@ -110,8 +112,8 @@ public function currentRejectsARowThatExposesNothing(): void $resultSet = new HydratingResultSet(new ArraySerializableHydrator(), new ArrayObject()); $resultSet->initialize(new ArrayIterator([new stdClass()])); - self::expectException(RuntimeException::class); - self::expectExceptionMessage('exposes no properties'); + self::expectException(ValueError::class); + self::expectExceptionMessage('will not transform a row it did not create'); $resultSet->current(); } @@ -242,11 +244,10 @@ public function toArray(): void public function toArrayReportsARowTheHydratorCannotExtract(): void { $hydratingRs = new HydratingResultSet(); - // Scalars never hydrate into an object, so current() yields null for each row. $hydratingRs->initialize(new ArrayIterator([1, 2])); - self::expectException(RuntimeException::class); - self::expectExceptionMessage('null'); + self::expectException(ValueError::class); + self::expectExceptionMessage('A row of type "int"'); $hydratingRs->toArray(); } diff --git a/test/unit/ResultSet/ObjectResultSetTest.php b/test/unit/ResultSet/ObjectResultSetTest.php new file mode 100644 index 00000000..774a06e4 --- /dev/null +++ b/test/unit/ResultSet/ObjectResultSetTest.php @@ -0,0 +1,137 @@ + + */ + public static function nonObjectRowProvider(): array + { + return [ + 'an array row means the fetch mode yields arrays' => [['id' => 1], 'array'], + 'FETCH_BOUND yields a success flag' => [true, 'bool'], + 'a null row is not a row' => [null, 'null'], + ]; + } + + #[Test] + #[DataProvider('nonObjectRowProvider')] + public function currentRejectsARowThatIsNotAnObject(mixed $row, string $rowType): void + { + $resultSet = new ObjectResultSet(); + $resultSet->initialize(new ArrayIterator([$row])); + + self::expectException(ValueError::class); + self::expectExceptionMessage(sprintf('A row of type "%s"', $rowType)); + + $resultSet->current(); + } + + #[Test] + public function currentRepeatsTheSameInstanceWhileBuffered(): void + { + $row = new stdClass(); + $row->id = 1; + $resultSet = new ObjectResultSet(); + $resultSet->initialize(new ArrayIterator([$row])); + $resultSet->buffer(); + + $first = $resultSet->current(); + $resultSet->rewind(); + + static::assertSame($first, $resultSet->current()); + } + + #[Test] + public function currentReturnsNullOnceTheRowsAreExhausted(): void + { + $resultSet = new ObjectResultSet(); + $resultSet->initialize(new ArrayIterator([])); + + static::assertNull($resultSet->current()); + } + + #[Test] + public function currentReturnsTheVeryObjectTheDataSourceGave(): void + { + $row = new stdClass(); + $row->id = 1; + $row->name = 'one'; + + $resultSet = new ObjectResultSet(); + $resultSet->initialize(new ArrayIterator([$row])); + + static::assertSame($row, $resultSet->current()); + } + + #[Test] + public function currentSaysWhichTypesItAccepts(): void + { + $resultSet = new ObjectResultSet(); + $resultSet->initialize(new ArrayIterator([['id' => 1]])); + + self::expectException(ValueError::class); + self::expectExceptionMessage('which accepts an object'); + + $resultSet->current(); + } + + #[Test] + public function toArrayReadsARowCarryingItsValuesAsElements(): void + { + $resultSet = new ObjectResultSet(); + $resultSet->initialize(new ArrayIterator([new ArrayObject(['id' => 1, 'name' => 'one'])])); + + static::assertSame([['id' => 1, 'name' => 'one']], $resultSet->toArray()); + } + + #[Test] + public function toArrayReadsThePublicPropertiesOfEachRow(): void + { + $first = new stdClass(); + $first->id = 1; + $second = new stdClass(); + $second->id = 2; + + $resultSet = new ObjectResultSet(); + $resultSet->initialize(new ArrayIterator([$first, $second])); + + static::assertSame([['id' => 1], ['id' => 2]], $resultSet->toArray()); + } + + #[Test] + public function toArrayRejectsARowThatExposesNothing(): void + { + $resultSet = new ObjectResultSet(); + $resultSet->initialize(new ArrayIterator([new stdClass()])); + + self::expectException(ValueError::class); + self::expectExceptionMessage('exposes no values'); + + $resultSet->toArray(); + } +} diff --git a/test/unit/ResultSet/ResultSetFetchModeTest.php b/test/unit/ResultSet/ResultSetFetchModeTest.php index 8fba1fe5..061a2936 100644 --- a/test/unit/ResultSet/ResultSetFetchModeTest.php +++ b/test/unit/ResultSet/ResultSetFetchModeTest.php @@ -6,49 +6,85 @@ use ArrayObject; use PDO; +use PDORow; use PhpDb\Adapter\Driver\Pdo\Result; use PhpDb\ResultSet\AbstractResultSet; -use PhpDb\ResultSet\Exception\RuntimeException; +use PhpDb\ResultSet\Exception\ValueError; +use PhpDb\ResultSet\ObjectResultSet; use PhpDb\ResultSet\ResultSet; +use PhpDb\ResultSet\ResultSetInterface; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; +use stdClass; use Throwable; use function array_map; use function iterator_to_array; +use function sprintf; /** * The fetch mode is chosen by the caller, so a row reaching the result set need not be - * an array. These cases drive a real SQLite result through the PDO driver and cover - * every entry in Result::VALID_FETCH_MODES, so that a mode cannot start discarding rows - * without a test noticing. + * an array. A result set fills its prototype from row data and refuses anything else + * rather than reshaping an object the caller asked PDO for. These cases drive a real + * SQLite result through the PDO driver and cover every entry in + * Result::VALID_FETCH_MODES, so that a mode cannot change sides without a test noticing. */ -#[CoversMethod(AbstractResultSet::class, 'rowToArray')] +#[CoversMethod(AbstractResultSet::class, 'getArrayData')] +#[CoversMethod(AbstractResultSet::class, 'unsupportedRowError')] #[CoversMethod(ResultSet::class, 'current')] +#[CoversMethod(ResultSet::class, 'mapRow')] +#[CoversMethod(ObjectResultSet::class, 'mapRow')] #[Group('unit')] final class ResultSetFetchModeTest extends TestCase { private PDO $pdo; /** - * Every mode that yields rows, and the type each row arrives as. + * Modes whose rows are objects, which ObjectResultSet passes through untouched. * * @return array */ + public static function objectYieldingModeProvider(): array + { + return [ + 'FETCH_OBJ yields a stdClass' => [PDO::FETCH_OBJ, stdClass::class], + 'FETCH_LAZY yields a PDORow' => [PDO::FETCH_LAZY, PDORow::class], + ]; + } + + /** + * Modes whose rows are not row data. The result set will not transform them, so each + * names the type the caller is left holding. + * + * @return array + */ + public static function refusedModeProvider(): array + { + return [ + 'FETCH_OBJ yields a stdClass' => [PDO::FETCH_OBJ, 'stdClass'], + 'FETCH_LAZY yields a PDORow' => [PDO::FETCH_LAZY, 'PDORow'], + 'FETCH_BOUND yields true and binds by reference' => [PDO::FETCH_BOUND, 'bool'], + ]; + } + + /** + * Every mode that yields row data, all of which fill the ArrayObject prototype. + * + * @return array + */ public static function rowYieldingModeProvider(): array { return [ - 'FETCH_ASSOC fills the ArrayObject prototype' => [PDO::FETCH_ASSOC, ArrayObject::class], - 'FETCH_NUM fills the ArrayObject prototype' => [PDO::FETCH_NUM, ArrayObject::class], - 'FETCH_BOTH fills the ArrayObject prototype' => [PDO::FETCH_BOTH, ArrayObject::class], - 'FETCH_OBJ fills the ArrayObject prototype' => [PDO::FETCH_OBJ, ArrayObject::class], - 'FETCH_NAMED fills the ArrayObject prototype' => [PDO::FETCH_NAMED, ArrayObject::class], - 'FETCH_KEY_PAIR fills the ArrayObject prototype' => [PDO::FETCH_KEY_PAIR, ArrayObject::class], - 'FETCH_PROPS_LATE is a flag, so PDO falls back' => [PDO::FETCH_PROPS_LATE, ArrayObject::class], - 'FETCH_CLASSTYPE is a flag, so PDO falls back' => [PDO::FETCH_CLASSTYPE, ArrayObject::class], + 'FETCH_ASSOC' => [PDO::FETCH_ASSOC], + 'FETCH_NUM' => [PDO::FETCH_NUM], + 'FETCH_BOTH' => [PDO::FETCH_BOTH], + 'FETCH_NAMED' => [PDO::FETCH_NAMED], + 'FETCH_KEY_PAIR' => [PDO::FETCH_KEY_PAIR], + 'FETCH_PROPS_LATE is a flag, so PDO falls back' => [PDO::FETCH_PROPS_LATE], + 'FETCH_CLASSTYPE is a flag, so PDO falls back' => [PDO::FETCH_CLASSTYPE], ]; } @@ -68,11 +104,11 @@ public static function unusableModeProvider(): array } /** - * FETCH_BOUND carries its row in variables bound by reference, so there is nothing - * for the result set to hand back. Null is the correct answer here, not a loss. + * FETCH_BOUND carries its row in variables bound by reference and yields only a + * success flag, so the columns still arrive even though the row itself is refused. */ #[Test] - public function boundFetchModeYieldsNullAndPopulatesTheBoundColumns(): void + public function boundFetchModeStillPopulatesTheBoundColumns(): void { $result = new Result(); $result->initialize($this->pdo->query('SELECT id, name FROM t'), null); @@ -85,29 +121,27 @@ public function boundFetchModeYieldsNullAndPopulatesTheBoundColumns(): void $resultSet = new ResultSet(); $resultSet->initialize($result); - $seen = []; - foreach ($resultSet as $row) { - static::assertNull($row); - $seen[] = (int) $id; - } + self::expectException(ValueError::class); - static::assertSame([1, 2], $seen); + try { + iterator_to_array($resultSet, preserve_keys: false); + } finally { + static::assertSame(1, (int) $id); + } } #[Test] #[DataProvider('rowYieldingModeProvider')] - public function everyRowYieldingFetchModeReturnsBothRows(int $fetchMode): void + public function everyRowYieldingFetchModeFillsThePrototype(int $fetchMode): void { - static::assertCount(2, $this->rowsFor($fetchMode)); + static::assertContainsOnlyInstancesOf(ArrayObject::class, $this->rowsFor($fetchMode)); } #[Test] #[DataProvider('rowYieldingModeProvider')] - public function everyRowYieldingFetchModeReturnsItsRows(int $fetchMode, string $expectedType): void + public function everyRowYieldingFetchModeReturnsBothRows(int $fetchMode): void { - $rows = $this->rowsFor($fetchMode); - - static::assertContainsOnlyInstancesOf($expectedType, $rows); + static::assertCount(2, $this->rowsFor($fetchMode)); } #[Test] @@ -119,26 +153,40 @@ public function fetchModesNeedingExtraArgumentsCannotBeDriven(int $fetchMode): v $this->rowsFor($fetchMode); } - /** - * FETCH_LAZY yields a PDORow, which resolves its columns through __get() and so - * reduces to an empty array. There is no way to fill a prototype from it, and - * emptying it silently is what this component used to do. - */ #[Test] - public function lazyFetchModeIsRejectedRatherThanEmptied(): void - { - self::expectException(RuntimeException::class); - self::expectExceptionMessage('exposes no properties'); + #[DataProvider('refusedModeProvider')] + public function fetchModesYieldingSomethingOtherThanRowDataAreRefused( + int $fetchMode, + string $rowType, + ): void { + self::expectException(ValueError::class); + self::expectExceptionMessage(sprintf('A row of type "%s"', $rowType)); - $this->rowsFor(PDO::FETCH_LAZY); + $this->rowsFor($fetchMode); } #[Test] - public function objectRowsKeepTheirColumnValues(): void + public function objectResultSetKeepsTheColumnValuesOfAnObjectRow(): void { - $rows = $this->rowsFor(PDO::FETCH_OBJ); + $rows = $this->rowsFor(PDO::FETCH_OBJ, new ObjectResultSet()); + + static::assertSame([1, 2], array_map(static fn(object $row): int => (int) $row->id, $rows)); + } - static::assertSame([1, 2], array_map(static fn(ArrayObject $row): int => (int) $row['id'], $rows)); + /** + * The modes ResultSet refuses are the ones ObjectResultSet exists for; it yields the + * caller's own objects rather than reshaping them. + */ + #[Test] + #[DataProvider('objectYieldingModeProvider')] + public function objectYieldingFetchModesAreServedByObjectResultSet( + int $fetchMode, + string $rowType, + ): void { + $rows = $this->rowsFor($fetchMode, new ObjectResultSet()); + + static::assertCount(2, $rows); + static::assertContainsOnlyInstancesOf($rowType, $rows); } protected function setUp(): void @@ -149,13 +197,13 @@ protected function setUp(): void } /** @return list */ - private function rowsFor(int $fetchMode): array + private function rowsFor(int $fetchMode, ?ResultSetInterface $resultSet = null): array { $result = new Result(); $result->initialize($this->pdo->query('SELECT id, name FROM t'), null); $result->setFetchMode($fetchMode); - $resultSet = new ResultSet(); + $resultSet ??= new ResultSet(); $resultSet->initialize($result); return iterator_to_array($resultSet, preserve_keys: false); diff --git a/test/unit/ResultSet/ResultSetIntegrationTest.php b/test/unit/ResultSet/ResultSetIntegrationTest.php index 6bc39324..df486068 100644 --- a/test/unit/ResultSet/ResultSetIntegrationTest.php +++ b/test/unit/ResultSet/ResultSetIntegrationTest.php @@ -30,10 +30,11 @@ use function random_int; use function var_export; -#[CoversMethod(AbstractResultSet::class, 'current')] +#[CoversMethod(AbstractResultSet::class, 'currentRow')] #[CoversMethod(AbstractResultSet::class, 'resolveIterator')] #[CoversMethod(AbstractResultSet::class, 'buffer')] #[CoversMethod(ResultSet::class, 'current')] +#[CoversMethod(ResultSet::class, 'mapRow')] #[CoversMethod(ResultSet::class, 'getReturnType')] #[CoversMethod(ResultSet::class, '__construct')] #[CoversMethod(ResultSet::class, 'getArrayObjectPrototype')] @@ -224,6 +225,7 @@ public function currentReturnsNullForNonExistingValues(): void public function currentWithBufferingCallsDataSourceCurrentOnce(): void { $mockResult = $this->getMockBuilder(ResultInterface::class)->getMock(); + $mockResult->method('valid')->willReturn(true); $mockResult->expects($this->once())->method('current')->willReturn(['foo' => 'bar']); $this->resultSet->initialize($mockResult); diff --git a/test/unit/ResultSet/RowBufferTest.php b/test/unit/ResultSet/RowBufferTest.php new file mode 100644 index 00000000..ee53d10c --- /dev/null +++ b/test/unit/ResultSet/RowBufferTest.php @@ -0,0 +1,172 @@ +isPending()); + static::assertFalse($buffer->isStoring()); + static::assertFalse($buffer->isBuffered()); + } + + #[Test] + public function clearForgetsTheRowsItHeld(): void + { + $buffer = new RowBuffer(); + $buffer->store(); + $buffer->put(0, ['id' => 1]); + + $buffer->clear(); + + static::assertFalse($buffer->has(0)); + } + + #[Test] + public function clearLeavesTheStateAlone(): void + { + $buffer = new RowBuffer(); + $buffer->store(); + + $buffer->clear(); + + static::assertTrue($buffer->isStoring()); + } + + #[Test] + public function getReturnsNullForAPositionItHoldsNothingFor(): void + { + $buffer = new RowBuffer(); + $buffer->store(); + + static::assertNull($buffer->get(3)); + } + + #[Test] + public function hasIsFalseWhileNotStoringEvenAfterAPut(): void + { + $buffer = new RowBuffer(); + $buffer->useDataSource(); + $buffer->put(0, ['id' => 1]); + + static::assertFalse($buffer->has(0)); + } + + #[Test] + public function iterationCannotBeginAndThenBeBuffered(): void + { + $buffer = new RowBuffer(); + $buffer->startIteration(); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage(RuntimeException::UNBUFFERED_ITERATION); + + $buffer->store(); + } + + #[Test] + public function iterationLeavesAStoringBufferStoring(): void + { + $buffer = new RowBuffer(); + $buffer->store(); + + $buffer->startIteration(); + + static::assertTrue($buffer->isStoring()); + } + + #[Test] + public function storeHoldsTheRowsPutIntoIt(): void + { + $buffer = new RowBuffer(); + $buffer->store(); + + $buffer->put(0, ['id' => 1]); + + static::assertTrue($buffer->has(0)); + static::assertSame(['id' => 1], $buffer->get(0)); + } + + #[Test] + public function storeLeavesAPassthroughBufferDeferringToItsDataSource(): void + { + $buffer = new RowBuffer(); + $buffer->useDataSource(); + + $buffer->store(); + + static::assertFalse($buffer->isStoring()); + static::assertTrue($buffer->isBuffered()); + } + + #[Test] + public function storeMakesTheBufferHoldRows(): void + { + $buffer = new RowBuffer(); + + $buffer->store(); + + static::assertTrue($buffer->isStoring()); + static::assertTrue($buffer->isBuffered()); + } + + #[Test] + public function storingTwiceKeepsTheRowsAlreadyHeld(): void + { + $buffer = new RowBuffer(); + $buffer->store(); + $buffer->put(0, ['id' => 1]); + + $buffer->store(); + + static::assertSame(['id' => 1], $buffer->get(0)); + } + + #[Test] + public function useDataSourceCountsAsBufferedWithoutStoring(): void + { + $buffer = new RowBuffer(); + + $buffer->useDataSource(); + + static::assertTrue($buffer->isBuffered()); + static::assertFalse($buffer->isStoring()); + static::assertFalse($buffer->isPending()); + } + + #[Test] + public function useDataSourceDropsTheRowsItWasHolding(): void + { + $buffer = new RowBuffer(); + $buffer->store(); + $buffer->put(0, ['id' => 1]); + + $buffer->useDataSource(); + + static::assertNull($buffer->get(0)); + } +} diff --git a/test/unit/ResultSet/RowPrototypeResultSetTest.php b/test/unit/ResultSet/RowPrototypeResultSetTest.php index 2fd98387..ebbf39b3 100644 --- a/test/unit/ResultSet/RowPrototypeResultSetTest.php +++ b/test/unit/ResultSet/RowPrototypeResultSetTest.php @@ -7,7 +7,7 @@ use ArrayIterator; use ArrayObject; use PhpDb\ResultSet\AbstractResultSet; -use PhpDb\ResultSet\Exception\RuntimeException; +use PhpDb\ResultSet\Exception\ValueError; use PhpDb\ResultSet\RowPrototypeInterface; use PhpDb\ResultSet\RowPrototypeResultSet; use PHPUnit\Framework\Attributes\CoversMethod; @@ -16,20 +16,19 @@ use PHPUnit\Framework\TestCase; use stdClass; -#[CoversMethod(AbstractResultSet::class, 'rowToArray')] +#[CoversMethod(AbstractResultSet::class, 'getArrayData')] +#[CoversMethod(RowPrototypeResultSet::class, 'unsupportedRowError')] #[CoversMethod(RowPrototypeResultSet::class, 'current')] +#[CoversMethod(RowPrototypeResultSet::class, 'mapRow')] #[CoversMethod(RowPrototypeResultSet::class, 'toArray')] #[Group('unit')] final class RowPrototypeResultSetTest extends TestCase { #[Test] - public function currentPopulatesThePrototypeFromAnObjectRow(): void + public function currentPopulatesThePrototypeFromARowCarryingItsValuesAsElements(): void { $resultSet = new RowPrototypeResultSet($this->createRowPrototype()); - $row = new stdClass(); - $row->id = 1; - $row->name = 'one'; - $resultSet->initialize(new ArrayIterator([$row])); + $resultSet->initialize(new ArrayIterator([new ArrayObject(['id' => 1, 'name' => 'one'])])); $current = $resultSet->current(); @@ -38,15 +37,31 @@ public function currentPopulatesThePrototypeFromAnObjectRow(): void } #[Test] - public function currentPopulatesThePrototypeFromARowCarryingItsValuesAsElements(): void + public function currentRejectsAnObjectRowThatIsNotAPrototype(): void { $resultSet = new RowPrototypeResultSet($this->createRowPrototype()); - $resultSet->initialize(new ArrayIterator([new ArrayObject(['id' => 1, 'name' => 'one'])])); + $row = new stdClass(); + $row->id = 1; + $row->name = 'one'; + $resultSet->initialize(new ArrayIterator([$row])); - $current = $resultSet->current(); + self::expectException(ValueError::class); + self::expectExceptionMessage('A row of type "stdClass"'); - static::assertInstanceOf(RowPrototypeInterface::class, $current); - static::assertSame(['id' => 1, 'name' => 'one'], $current->toArray()); + $resultSet->current(); + } + + #[Test] + public function currentRejectsANullRowRatherThanYieldingIt(): void + { + $prototype = $this->createRowPrototype(); + $resultSet = new RowPrototypeResultSet($prototype); + $resultSet->initialize(new ArrayIterator([null])); + + self::expectException(ValueError::class); + self::expectExceptionMessage('A row of type "null"'); + + $resultSet->current(); } #[Test] @@ -55,8 +70,8 @@ public function currentRejectsARowThatExposesNothing(): void $resultSet = new RowPrototypeResultSet($this->createRowPrototype()); $resultSet->initialize(new ArrayIterator([new stdClass()])); - self::expectException(RuntimeException::class); - self::expectExceptionMessage('exposes no properties'); + self::expectException(ValueError::class); + self::expectExceptionMessage('an array, ArrayObject or RowPrototypeInterface'); $resultSet->current(); } @@ -73,11 +88,10 @@ public function currentReturnsARowThatIsAlreadyAPrototypeUntouched(): void } #[Test] - public function currentReturnsDataUnchangedWhenNotArray(): void + public function currentReturnsNullOnceTheRowsAreExhausted(): void { - $prototype = $this->createRowPrototype(); - $resultSet = new RowPrototypeResultSet($prototype); - $resultSet->initialize(new ArrayIterator([null])); + $resultSet = new RowPrototypeResultSet($this->createRowPrototype()); + $resultSet->initialize(new ArrayIterator([])); static::assertNull($resultSet->current()); } diff --git a/test/unit/ResultSet/TestAsset/PassThroughResultSet.php b/test/unit/ResultSet/TestAsset/PassThroughResultSet.php new file mode 100644 index 00000000..67fc8f44 --- /dev/null +++ b/test/unit/ResultSet/TestAsset/PassThroughResultSet.php @@ -0,0 +1,44 @@ +currentRow(); + } + + /** {@inheritDoc} */ + #[Override] + public function toArray(): array + { + $return = []; + foreach ($this as $row) { + $return[] = $row; + } + + return $return; + } + + #[Override] + protected function mapRow(mixed $row): mixed + { + return $row; + } +} From 10e33754e4b29c71bcf6ea4d9e31a4cf48da7262 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 30 Sep 2026 14:26:40 +1000 Subject: [PATCH 07/17] Document ObjectResultSet and the row types each set accepts --- docs/book/result-set/intro.md | 46 +++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/docs/book/result-set/intro.md b/docs/book/result-set/intro.md index 9b1fb1d2..a4e0b263 100644 --- a/docs/book/result-set/intro.md +++ b/docs/book/result-set/intro.md @@ -145,6 +145,52 @@ For more information, see the documentation to get a better sense of the different strategies that can be employed in order to populate a target object. +## ObjectResultSet + +`PhpDb\ResultSet\ObjectResultSet` is for fetch modes that yield objects, such as +`PDO::FETCH_OBJ` and `PDO::FETCH_LAZY`. Rows leave it as the very instances the driver +produced: nothing is cloned, hydrated or reshaped. + +```php title="Using ObjectResultSet with PDO::FETCH_OBJ" +use PDO; +use PhpDb\ResultSet\ObjectResultSet; + +$result = $statement->execute(); +$result->setFetchMode(PDO::FETCH_OBJ); + +$resultSet = new ObjectResultSet(); +$resultSet->initialize($result); + +foreach ($resultSet as $user) { + printf("%s %s\n", $user->first_name, $user->last_name); +} +``` + +`toArray()` reads each row's values, either by traversing it or by reading its public +properties. A row that exposes neither — a `PDORow` from `PDO::FETCH_LAZY`, for +instance — throws rather than yielding an empty array, so iterate such a result set +instead of calling `toArray()` on it. + +## Choosing a Result Set + +A result set fills its rows from row data, meaning an `array` or an `ArrayObject`. It +will not transform a row the driver handed it into some other shape, so the fetch mode +and the result set have to agree; where they do not, the row is refused with a +`PhpDb\ResultSet\Exception\ValueError` naming both types. + +| Result set | Rows arrive as | Rows leave as | +|---|---|---| +| `ResultSet` | row data, or an `ArrayObject` | a filled `ArrayObject` prototype, an array, or that same `ArrayObject` | +| `ArrayResultSet` | row data | an array | +| `ObjectResultSet` | any object | that same object | +| `HydratingResultSet` | row data | the hydrated row prototype | +| `RowPrototypeResultSet` | row data, or a `RowPrototypeInterface` | a populated `RowPrototypeInterface` | + +With PDO that means `FETCH_ASSOC`, `FETCH_NUM`, `FETCH_BOTH`, `FETCH_NAMED` and +`FETCH_KEY_PAIR` suit the row-data sets, while `FETCH_OBJ` and `FETCH_LAZY` need +`ObjectResultSet`. `FETCH_BOUND` yields only a success flag and binds its columns by +reference, so no result set accepts its rows; read the bound variables instead. + ## Data Source Types The `initialize()` method accepts arrays, `Iterator`, `IteratorAggregate`, From 329f9775c5cb480c98c871558011dc9fbb8de6bf Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 30 Sep 2026 14:45:02 +1000 Subject: [PATCH 08/17] Exclude prefer-self-return-type where self cannot be used PHP rejects `self` inside an intersection type, so the capability interfaces cannot take the rule's advice. Drops the three stale baseline entries. --- lint-baseline.toml | 18 ------------------ mago.toml | 8 ++++++++ 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/lint-baseline.toml b/lint-baseline.toml index 9ff1f0d7..de4db269 100644 --- a/lint-baseline.toml +++ b/lint-baseline.toml @@ -270,24 +270,6 @@ code = "too-many-methods" message = "Class has too many methods." count = 1 -[[issues]] -file = "src/ResultSet/ArrayObjectResultSetInterface.php" -code = "prefer-self-return-type" -message = "Return type `ArrayObjectResultSetInterface` refers to the enclosing class; use `self` instead." -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSetInterface.php" -code = "prefer-self-return-type" -message = "Return type `HydratingResultSetInterface` refers to the enclosing class; use `self` instead." -count = 1 - -[[issues]] -file = "src/ResultSet/RowPrototypeResultSetInterface.php" -code = "prefer-self-return-type" -message = "Return type `RowPrototypeResultSetInterface` refers to the enclosing class; use `self` instead." -count = 1 - [[issues]] file = "src/RowGateway/AbstractRowGateway.php" code = "cyclomatic-complexity" diff --git a/mago.toml b/mago.toml index 1ddf6a0f..d91e430c 100644 --- a/mago.toml +++ b/mago.toml @@ -18,6 +18,14 @@ baseline = "lint-baseline.toml" # not apply. "test/" repeats the phpdb-qa-tools default, which this key replaces. too-many-methods = { exclude = ["test/", "src/Sql/Exception/", "src/TableGateway/Exception/", "src/Adapter/Exception/"] } +# The capability interfaces return `ResultSetInterface&`, and PHP rejects +# `self` as part of an intersection type, so this rule's advice cannot be taken there. +prefer-self-return-type = { exclude = [ + "src/ResultSet/ArrayObjectResultSetInterface.php", + "src/ResultSet/HydratingResultSetInterface.php", + "src/ResultSet/RowPrototypeResultSetInterface.php", +] } + [analyzer] baseline = "analyzer-baseline.toml" class-initializers = [ From 84134c5eef7bb7c6b5af1a7ebfa3159199cbc457 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 2 Oct 2026 15:43:18 +1000 Subject: [PATCH 09/17] Return null for an exhausted row without asking the data source valid() The mysqli Result closes its statement once a fetch finds no row, so the valid() that followed current() fetched again from a closed statement and threw on an empty result. A null or false row now ends the read on its own, which also restores the 0.6.x handling of a null row inside array data. --- src/ResultSet/AbstractResultSet.php | 19 +++++----- test/unit/ResultSet/AbstractResultSetTest.php | 28 ++++++++++++++ test/unit/ResultSet/ObjectResultSetTest.php | 1 - .../ResultSet/ResultSetIntegrationTest.php | 37 ++++++++++--------- .../ResultSet/RowPrototypeResultSetTest.php | 27 +++++++------- 5 files changed, 71 insertions(+), 41 deletions(-) diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index cbbc308a..e788dd6c 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -279,29 +279,28 @@ abstract protected function mapRow(mixed $row): mixed; */ protected function currentRow(): mixed { - $this->rowBuffer()->startIteration(); + $buffer = $this->rowBuffer(); + $buffer->startIteration(); - if ($this->rowBuffer()->has($this->position)) { - return $this->rowBuffer()->get($this->position); + if ($buffer->has($this->position)) { + return $buffer->get($this->position); } - $dataSource = $this->dataSource(); - /** - * The data source is asked for its row first, because a ResultInterface only - * knows whether it is still valid once it has tried to fetch one. + * A driver Result signals the end of its rows with null or false, and asking it + * valid() afterwards may fetch again from a statement it has already closed. * * @var mixed $row */ - $row = $dataSource->current(); + $row = $this->dataSource()->current(); - if (! $dataSource->valid()) { + if (null === $row || false === $row) { return null; } $row = $this->mapRow($row); - $this->rowBuffer()->put($this->position, $row); + $buffer->put($this->position, $row); return $row; } diff --git a/test/unit/ResultSet/AbstractResultSetTest.php b/test/unit/ResultSet/AbstractResultSetTest.php index a8c2e76c..0d6be72d 100644 --- a/test/unit/ResultSet/AbstractResultSetTest.php +++ b/test/unit/ResultSet/AbstractResultSetTest.php @@ -182,6 +182,25 @@ public function current(): void static::assertEquals(['id' => 1, 'name' => 'one'], $resultSet->current()); } + /** + * The mysqli Result closes its statement once a fetch finds no row, so a valid() + * asked after that fetches again from a closed statement. + * + * @throws Exception + */ + #[Test] + public function currentDoesNotAskAnExhaustedDriverResultWhetherItIsValid(): void + { + $result = $this->createMock(ResultInterface::class); + $result->method('current')->willReturn(null); + $result->expects(self::never())->method('valid'); + + $resultSet = $this->createResultSetMock(); + $resultSet->initialize($result); + + static::assertNull($resultSet->current()); + } + #[Test] public function currentReportsAnUninitialisedDataSourceRatherThanFailingOnNull(): void { @@ -218,6 +237,15 @@ public function currentReturnsBufferedDataOnSecondPass(): void static::assertEquals($firstPass, $secondPass); } + #[Test] + public function currentReturnsNullForAFalseRowFromTheDataSource(): void + { + $resultSet = $this->createResultSetMock(); + $resultSet->initialize(new ArrayIterator([false])); + + static::assertNull($resultSet->current()); + } + /** * @throws Exception */ diff --git a/test/unit/ResultSet/ObjectResultSetTest.php b/test/unit/ResultSet/ObjectResultSetTest.php index 774a06e4..87886761 100644 --- a/test/unit/ResultSet/ObjectResultSetTest.php +++ b/test/unit/ResultSet/ObjectResultSetTest.php @@ -34,7 +34,6 @@ public static function nonObjectRowProvider(): array return [ 'an array row means the fetch mode yields arrays' => [['id' => 1], 'array'], 'FETCH_BOUND yields a success flag' => [true, 'bool'], - 'a null row is not a row' => [null, 'null'], ]; } diff --git a/test/unit/ResultSet/ResultSetIntegrationTest.php b/test/unit/ResultSet/ResultSetIntegrationTest.php index df486068..0342a4e5 100644 --- a/test/unit/ResultSet/ResultSetIntegrationTest.php +++ b/test/unit/ResultSet/ResultSetIntegrationTest.php @@ -11,6 +11,7 @@ use PhpDb\ResultSet\AbstractResultSet; use PhpDb\ResultSet\Exception\InvalidArgumentException; use PhpDb\ResultSet\Exception\RuntimeException; +use PhpDb\ResultSet\Exception\ValueError; use PhpDb\ResultSet\ResultSet; use PhpDb\ResultSet\ResultSetReturnType; use PHPUnit\Framework\Attributes\CoversMethod; @@ -177,6 +178,25 @@ public function currentClonesRowPrototypeOnEachCall(): void static::assertNotSame($first, $second); } + /** + * @throws Exception + * @throws \Exception + */ + #[Test] + public function currentRejectsARowThatIsNotArrayData(): void + { + $mockResult = $this->createMock(ResultInterface::class); + $mockResult->method('current')->willReturn('Not an Array'); + + $this->resultSet->initialize($mockResult); + $this->resultSet->buffer(); + + self::expectException(ValueError::class); + self::expectExceptionMessage('A row of type "string"'); + + $this->resultSet->current(); + } + #[Test] public function currentReturnsArrayObjectWhenReturnTypeIsArrayObject(): void { @@ -201,23 +221,6 @@ public function currentReturnsArrayWhenReturnTypeIsArray(): void static::assertSame(1, $current['id']); } - /** - * @throws Exception - * @throws \Exception - */ - #[Test] - public function currentReturnsNullForNonExistingValues(): void - { - $mockResult = $this->createMock(ResultInterface::class); - $mockResult->expects($this->once())->method('current')->willReturn('Not an Array'); - - $this->resultSet->initialize($mockResult); - $this->resultSet->buffer(); - - // Verify current() returns null when data source returns non-array value - static::assertNull($this->resultSet->current()); - } - /** * @throws \Exception */ diff --git a/test/unit/ResultSet/RowPrototypeResultSetTest.php b/test/unit/ResultSet/RowPrototypeResultSetTest.php index ebbf39b3..5144e842 100644 --- a/test/unit/ResultSet/RowPrototypeResultSetTest.php +++ b/test/unit/ResultSet/RowPrototypeResultSetTest.php @@ -51,19 +51,6 @@ public function currentRejectsAnObjectRowThatIsNotAPrototype(): void $resultSet->current(); } - #[Test] - public function currentRejectsANullRowRatherThanYieldingIt(): void - { - $prototype = $this->createRowPrototype(); - $resultSet = new RowPrototypeResultSet($prototype); - $resultSet->initialize(new ArrayIterator([null])); - - self::expectException(ValueError::class); - self::expectExceptionMessage('A row of type "null"'); - - $resultSet->current(); - } - #[Test] public function currentRejectsARowThatExposesNothing(): void { @@ -112,6 +99,20 @@ public function currentReturnsPopulatedCloneOfPrototypeForArrayRow(): void static::assertSame(['id' => 1, 'name' => 'one'], $current->toArray()); } + #[Test] + public function iterationYieldsNullForANullRowAndCarriesOn(): void + { + $resultSet = new RowPrototypeResultSet($this->createRowPrototype()); + $resultSet->initialize([['id' => 1], null, ['id' => 3]]); + + $rows = []; + foreach ($resultSet as $row) { + $rows[] = $row?->toArray(); + } + + static::assertSame([['id' => 1], null, ['id' => 3]], $rows); + } + #[Test] public function toArrayConvertsPrototypeRowsToArrays(): void { From df2c82a8b78c4838e3920881f6035a17875ea9d6 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 2 Oct 2026 16:13:01 +1000 Subject: [PATCH 10/17] Read rows straight from the data source once buffering is settled Iterating a ResultSet cost about 25 method calls per row against 7 in 0.6.x, nearly all of it RowBuffer bookkeeping that had nothing to do once buffering was settled. - RowBuffer folds back into AbstractResultSet as a RowBufferState and an array of held rows; rowBuffer() and __clone() go - $readsDirectly and $storesRows, derived from the state by setBufferState(), give valid(), current() and next() a direct branch for each common state - mapRow() reads plain arrays first and clones the row prototype property rather than calling its getter, as laminas-db did --- analyzer-baseline.toml | 4 +- src/ResultSet/AbstractResultSet.php | 205 +++++++++++++----- src/ResultSet/HydratingResultSet.php | 11 +- src/ResultSet/ResultSet.php | 17 +- src/ResultSet/RowBuffer.php | 132 ----------- src/ResultSet/RowBufferState.php | 2 +- src/ResultSet/RowPrototypeResultSet.php | 7 +- test/unit/ResultSet/AbstractResultSetTest.php | 108 ++++++++- test/unit/ResultSet/RowBufferTest.php | 172 --------------- 9 files changed, 282 insertions(+), 376 deletions(-) delete mode 100644 src/ResultSet/RowBuffer.php delete mode 100644 test/unit/ResultSet/RowBufferTest.php diff --git a/analyzer-baseline.toml b/analyzer-baseline.toml index 3d105079..6a0f82f4 100644 --- a/analyzer-baseline.toml +++ b/analyzer-baseline.toml @@ -1538,8 +1538,8 @@ count = 2 [[issues]] file = "src/ResultSet/AbstractResultSet.php" -code = "mixed-property-type-coercion" -message = "A value with a less specific type `mixed` is being assigned to property `$fieldCount` (int|null)." +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]] diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index e788dd6c..491ddeab 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -36,12 +36,34 @@ abstract class AbstractResultSet implements ResultSetInterface /** How deeply initialize() will unwrap a chain of IteratorAggregate. */ private const int MAX_ITERATOR_DEPTH = 8; + private RowBufferState $bufferState = RowBufferState::Pending; + + /** + * Whether rows are read straight from the data source, with no buffer work per row. + * + * True for Passthrough and Disabled, which are only ever entered once a data source + * is in place, so valid(), current() and next() can go straight to it. Derived from + * $bufferState and written only by setBufferState(), so the two cannot disagree. + */ + private bool $readsDirectly = false; + /** - * Created on first use, so that a subclass constructor need not call this one. + * Whether rows are held as they are read, with a data source in place to read them from. * - * @var RowBuffer|null + * Derived from $bufferState and the data source by setBufferState(), which + * initialize() calls again whenever it replaces the data source. */ - private ?RowBuffer $buffer = null; + private bool $storesRows = false; + + /** + * Rows held for later passes, keyed by position, already shaped by mapRow(). + * + * Only filled while Storing, and emptied by initialize() before the state can + * leave Storing, so a row found here never needs the state checked as well. + * + * @var array + */ + private array $bufferedRows = []; protected ?int $count = null; @@ -87,10 +109,17 @@ private static function resolveIterator(Traversable $dataSource): Iterator */ public function buffer(): ResultSetInterface { - $pending = $this->rowBuffer()->isPending(); - $this->rowBuffer()->store(); + if (RowBufferState::Disabled === $this->bufferState) { + throw RuntimeException::forUnbufferedIteration(); + } + + if (RowBufferState::Pending !== $this->bufferState) { + return $this; + } + + $this->setBufferState(RowBufferState::Storing); - if ($pending && $this->dataSource instanceof ResultInterface) { + if ($this->dataSource instanceof ResultInterface) { $this->dataSource->rewind(); } @@ -166,24 +195,17 @@ public function getFieldCount(): int #[Override] public function initialize(iterable $dataSource): ResultSetInterface { - $this->rowBuffer()->clear(); + $this->bufferedRows = []; if ($dataSource instanceof ResultInterface) { - $this->fieldCount = $dataSource->getFieldCount(); - $this->dataSource = $dataSource; - if ($dataSource->isBuffered()) { - $this->rowBuffer()->useDataSource(); - } - - if ($this->rowBuffer()->isStoring()) { - $dataSource->rewind(); - } + $this->initializeFromResult($dataSource); return $this; } if ($dataSource instanceof Traversable) { $this->dataSource = self::resolveIterator($dataSource); + $this->setBufferState($this->bufferState); return $this; } @@ -192,14 +214,15 @@ public function initialize(iterable $dataSource): ResultSetInterface $this->fieldCount = is_array($first) || $first instanceof Countable ? count($first) : 0; reset($dataSource); $this->dataSource = new ArrayIterator($dataSource); - $this->rowBuffer()->useDataSource(); // an array is its own buffer + // An array is its own buffer + $this->setBufferState(RowBufferState::Passthrough); return $this; } public function isBuffered(): bool { - return $this->rowBuffer()->isBuffered(); + return RowBufferState::Storing === $this->bufferState || RowBufferState::Passthrough === $this->bufferState; } /** @@ -219,10 +242,34 @@ public function key(): int #[Override] public function next(): void { - $this->rowBuffer()->startIteration(); + if ($this->readsDirectly) { + /** @var Iterator $dataSource */ + $dataSource = $this->dataSource; + $dataSource->next(); + $this->position++; + return; + } + + if ($this->storesRows) { + /** @var Iterator $dataSource */ + $dataSource = $this->dataSource; + if ($this->position === $dataSource->key()) { + $dataSource->next(); + } + + $this->position++; + return; + } - if (! $this->rowBuffer()->isStoring() || $this->position === $this->dataSource()->key()) { - $this->dataSource()->next(); + $dataSource = $this->dataSource ?? throw RuntimeException::forUninitialisedDataSource(); + + // While Storing, a pass over held rows leaves the data source where it stopped + if (RowBufferState::Storing !== $this->bufferState || $this->position === $dataSource->key()) { + $dataSource->next(); + } + + if (RowBufferState::Pending === $this->bufferState) { + $this->setBufferState(RowBufferState::Disabled); } $this->position++; @@ -236,7 +283,7 @@ public function next(): void #[Override] public function rewind(): void { - if (! $this->rowBuffer()->isStoring()) { + if (RowBufferState::Storing !== $this->bufferState) { $this->dataSource()->rewind(); } @@ -251,11 +298,17 @@ public function rewind(): void #[Override] public function valid(): bool { - if ($this->rowBuffer()->has($this->position)) { + if ($this->readsDirectly) { + /** @var Iterator $dataSource */ + $dataSource = $this->dataSource; + return $dataSource->valid(); + } + + if (null !== ($this->bufferedRows[$this->position] ?? null)) { return true; } - return $this->dataSource()->valid(); + return ($this->dataSource ?? throw RuntimeException::forUninitialisedDataSource())->valid(); } /** @@ -273,36 +326,75 @@ abstract protected function mapRow(mixed $row): mixed; * Concrete result sets call this from current() and shape the row in mapRow(), so * that buffering lives here and row typing lives with the set that declares it. * + * Every row of every pass comes through here, so the buffered path stays inline + * rather than being split out to meet the complexity limit. + * + * A driver Result signals the end of its rows with null or false, and asking it + * valid() afterwards may fetch again from a statement it has already closed, so + * either value ends the read on its own. + * * @return TRow|null * * @throws RuntimeException + * + * @mago-expect lint:halstead */ protected function currentRow(): mixed { - $buffer = $this->rowBuffer(); - $buffer->startIteration(); + if ($this->readsDirectly) { + /** @var Iterator $dataSource */ + $dataSource = $this->dataSource; - if ($buffer->has($this->position)) { - return $buffer->get($this->position); + /** @var mixed $row */ + $row = $dataSource->current(); + + return null === $row || false === $row ? null : $this->mapRow($row); } - /** - * A driver Result signals the end of its rows with null or false, and asking it - * valid() afterwards may fetch again from a statement it has already closed. - * - * @var mixed $row - */ - $row = $this->dataSource()->current(); + if ($this->storesRows) { + /** @var Iterator $dataSource */ + $dataSource = $this->dataSource; + + $held = $this->bufferedRows[$this->position] ?? null; + + if (null !== $held) { + return $held; + } + + /** @var mixed $row */ + $row = $dataSource->current(); + + if (null === $row || false === $row) { + return null; + } + + return $this->bufferedRows[$this->position] = $this->mapRow($row); + } + + $dataSource = $this->dataSource ?? throw RuntimeException::forUninitialisedDataSource(); + + if (RowBufferState::Pending === $this->bufferState) { + $this->setBufferState(RowBufferState::Disabled); + } + + $held = $this->bufferedRows[$this->position] ?? null; + + if (null !== $held) { + return $held; + } + + /** @var mixed $row */ + $row = $dataSource->current(); if (null === $row || false === $row) { return null; } - $row = $this->mapRow($row); - - $buffer->put($this->position, $row); + if (RowBufferState::Storing !== $this->bufferState) { + return $this->mapRow($row); + } - return $row; + return $this->bufferedRows[$this->position] = $this->mapRow($row); } /** @@ -342,16 +434,6 @@ protected function getArrayData(mixed $row): array throw $this->unsupportedRowError($row); } - /** - * The rows this result set holds on to. - * - * @return RowBuffer - */ - protected function rowBuffer(): RowBuffer - { - return $this->buffer ??= new RowBuffer(); - } - /** * The error describing a row this result set cannot read. * @@ -363,16 +445,27 @@ protected function unsupportedRowError(mixed $row): ValueError return ValueError::forRowThatIsNotArrayData(get_debug_type($row), static::class); } - /** - * A result set is routinely cloned from a prototype, so the rows one holds must - * never be shared with the next. - */ - public function __clone(): void + private function initializeFromResult(ResultInterface $result): void { - if (null === $this->buffer) { + $this->fieldCount = $result->getFieldCount(); + $this->dataSource = $result; + + if ($result->isBuffered()) { + $this->setBufferState(RowBufferState::Passthrough); return; } - $this->buffer = clone $this->buffer; + $this->setBufferState($this->bufferState); + + if (RowBufferState::Storing === $this->bufferState) { + $result->rewind(); + } + } + + private function setBufferState(RowBufferState $state): void + { + $this->bufferState = $state; + $this->readsDirectly = RowBufferState::Passthrough === $state || RowBufferState::Disabled === $state; + $this->storesRows = RowBufferState::Storing === $state && null !== $this->dataSource; } } diff --git a/src/ResultSet/HydratingResultSet.php b/src/ResultSet/HydratingResultSet.php index e56015b5..14ad8619 100644 --- a/src/ResultSet/HydratingResultSet.php +++ b/src/ResultSet/HydratingResultSet.php @@ -10,6 +10,8 @@ use Override; use PhpDb\ResultSet\Exception\RuntimeException; +use function is_array; + /** * @api * @@ -101,13 +103,16 @@ public function toArray(): array * Hydrate one row onto a clone of the prototype. * * @throws Exception\ValueError If the row is not row data. + * @throws \Laminas\Hydrator\Exception\RuntimeException If the hydrator cannot fill the prototype. */ #[Override] protected function mapRow(mixed $row): object { - return $this->getHydrator()->hydrate( - $this->getArrayData($row), - clone $this->getRowPrototype(), + $this->rowPrototype ??= new ArrayObject(); + + return ($this->hydrator ??= new ArraySerializableHydrator())->hydrate( + is_array($row) ? $row : $this->getArrayData($row), + clone $this->rowPrototype, ); } } diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index 7ed76b21..caf1f475 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -7,6 +7,7 @@ use ArrayObject; use Override; +use function is_array; use function is_string; /** @@ -125,18 +126,20 @@ public function toArray(): array #[Override] protected function mapRow(mixed $row): array|ArrayObject { - if ($row instanceof ArrayObject) { - return $row; - } + if (! is_array($row)) { + if ($row instanceof ArrayObject) { + return $row; + } - $data = $this->getArrayData($row); + $row = $this->getArrayData($row); + } if (! $this->fillsRowPrototype) { - return $data; + return $row; } - $ao = clone $this->getRowPrototype(); - $ao->exchangeArray($data); + $ao = clone $this->rowPrototype; + $ao->exchangeArray($row); return $ao; } diff --git a/src/ResultSet/RowBuffer.php b/src/ResultSet/RowBuffer.php deleted file mode 100644 index aa11a84a..00000000 --- a/src/ResultSet/RowBuffer.php +++ /dev/null @@ -1,132 +0,0 @@ - */ - private array $rows = []; - - private RowBufferState $state = RowBufferState::Pending; - - /** - * Forget every row held, leaving the state alone. - */ - public function clear(): void - { - $this->rows = []; - } - - /** - * The row held for a position, if one is held there. - * - * @return TRow|null - */ - public function get(int $position): mixed - { - return $this->rows[$position] ?? null; - } - - /** - * Whether a row is held for a position. - */ - public function has(int $position): bool - { - return RowBufferState::Storing === $this->state && array_key_exists($position, $this->rows); - } - - /** - * Whether rows can be read more than once, however that came about. - */ - public function isBuffered(): bool - { - return RowBufferState::Storing === $this->state || RowBufferState::Passthrough === $this->state; - } - - /** - * Whether buffering has still to be settled either way. - */ - public function isPending(): bool - { - return RowBufferState::Pending === $this->state; - } - - /** - * Whether rows are being held here as they are read. - */ - public function isStoring(): bool - { - return RowBufferState::Storing === $this->state; - } - - /** - * Hold a row for a position, if rows are being held at all. - * - * @param TRow $row - */ - public function put(int $position, mixed $row): void - { - if (! $this->isStoring()) { - return; - } - - $this->rows[$position] = $row; - } - - /** - * Settle an undecided buffer, because iteration has begun and buffer() is too late. - */ - public function startIteration(): void - { - if (! $this->isPending()) { - return; - } - - $this->state = RowBufferState::Disabled; - } - - /** - * Begin holding rows. - * - * @throws RuntimeException If iteration has already settled the buffer. - */ - public function store(): void - { - if (RowBufferState::Disabled === $this->state) { - throw RuntimeException::forUnbufferedIteration(); - } - - if ($this->isBuffered()) { - return; - } - - $this->state = RowBufferState::Storing; - $this->rows = []; - } - - /** - * Defer to a data source that can already be read more than once. - */ - public function useDataSource(): void - { - $this->state = RowBufferState::Passthrough; - $this->rows = []; - } -} diff --git a/src/ResultSet/RowBufferState.php b/src/ResultSet/RowBufferState.php index fc55e2de..66e3926c 100644 --- a/src/ResultSet/RowBufferState.php +++ b/src/ResultSet/RowBufferState.php @@ -5,7 +5,7 @@ namespace PhpDb\ResultSet; /** - * The states a RowBuffer can be in. + * The buffering states a result set can be in. * * @internal */ diff --git a/src/ResultSet/RowPrototypeResultSet.php b/src/ResultSet/RowPrototypeResultSet.php index 1b957efa..d898fe26 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -7,6 +7,7 @@ use Override; use function get_debug_type; +use function is_array; /** * @api @@ -69,11 +70,15 @@ public function toArray(): array #[Override] protected function mapRow(mixed $row): RowPrototypeInterface { + if (is_array($row)) { + return (clone $this->rowPrototype)->populate($row); + } + if ($row instanceof RowPrototypeInterface) { return $row; } - return (clone $this->getRowPrototype())->populate($this->getArrayData($row)); + return (clone $this->rowPrototype)->populate($this->getArrayData($row)); } /** {@inheritDoc} */ diff --git a/test/unit/ResultSet/AbstractResultSetTest.php b/test/unit/ResultSet/AbstractResultSetTest.php index 0d6be72d..045191bf 100644 --- a/test/unit/ResultSet/AbstractResultSetTest.php +++ b/test/unit/ResultSet/AbstractResultSetTest.php @@ -23,6 +23,7 @@ use TypeError; use function assert; +use function iterator_to_array; #[CoversMethod(AbstractResultSet::class, 'initialize')] #[CoversMethod(AbstractResultSet::class, 'resolveIterator')] @@ -33,9 +34,9 @@ #[CoversMethod(AbstractResultSet::class, 'getFieldCount')] #[CoversMethod(AbstractResultSet::class, 'next')] #[CoversMethod(AbstractResultSet::class, 'key')] -#[CoversMethod(AbstractResultSet::class, '__clone')] -#[CoversMethod(AbstractResultSet::class, 'rowBuffer')] #[CoversMethod(AbstractResultSet::class, 'currentRow')] +#[CoversMethod(AbstractResultSet::class, 'initializeFromResult')] +#[CoversMethod(AbstractResultSet::class, 'setBufferState')] #[CoversMethod(AbstractResultSet::class, 'valid')] #[CoversMethod(AbstractResultSet::class, 'rewind')] #[CoversMethod(AbstractResultSet::class, 'count')] @@ -66,6 +67,62 @@ public function buffer(): void $resultSet->buffer(); } + #[Test] + public function bufferAfterABufferedPassHasBegunKeepsBuffering(): void + { + $resultSet = $this->createResultSetMock(); + $resultSet->initialize(new ArrayIterator([['id' => 1], ['id' => 2]])); + $resultSet->buffer(); + $resultSet->current(); + $resultSet->next(); + + $resultSet->buffer(); + $resultSet->rewind(); + + static::assertSame(['id' => 1], $resultSet->current()); + } + + #[Test] + public function bufferAfterIteratingAnArrayDataSourceIsAllowed(): void + { + $resultSet = $this->createResultSetMock(); + $resultSet->initialize([['id' => 1], ['id' => 2]]); + $resultSet->current(); + $resultSet->next(); + + $resultSet->buffer(); + + static::assertTrue($resultSet->isBuffered()); + } + + /** + * @throws Exception + */ + #[Test] + public function bufferBeforeInitializeHoldsTheRowsOfTheDataSourceGivenLater(): void + { + $resultSet = $this->createResultSetMock(); + $resultSet->buffer(); + $resultSet->initialize(new NoRewindIterator(new ArrayIterator([['id' => 1], ['id' => 2]]))); + iterator_to_array($resultSet); + + static::assertSame([['id' => 1], ['id' => 2]], iterator_to_array($resultSet)); + } + + #[Test] + public function bufferCalledTwiceKeepsTheRowsAlreadyHeld(): void + { + $resultSet = $this->createResultSetMock(); + $resultSet->initialize(new ArrayIterator([['id' => 1]])); + $resultSet->buffer(); + $resultSet->current(); + + $resultSet->buffer(); + $resultSet->rewind(); + + static::assertSame(['id' => 1], $resultSet->current()); + } + /** * Test multiple iterations with buffer * @@ -212,6 +269,18 @@ public function currentReportsAnUninitialisedDataSourceRatherThanFailingOnNull() $resultSet->current(); } + #[Test] + public function currentReportsAnUninitialisedDataSourceWhileBuffering(): void + { + $resultSet = $this->createResultSetMock(); + $resultSet->buffer(); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage(RuntimeException::UNINITIALISED_DATA_SOURCE); + + $resultSet->current(); + } + #[Test] public function currentReturnsBufferedDataOnSecondPass(): void { @@ -246,6 +315,18 @@ public function currentReturnsNullForAFalseRowFromTheDataSource(): void static::assertNull($resultSet->current()); } + #[Test] + public function currentReturnsNullPastTheLastRowWhileBuffering(): void + { + $resultSet = $this->createResultSetMock(); + $resultSet->initialize(new ArrayIterator([['id' => 1]])); + $resultSet->buffer(); + $resultSet->current(); + $resultSet->next(); + + static::assertNull($resultSet->current()); + } + /** * @throws Exception */ @@ -533,6 +614,29 @@ public function next(): void static::assertSame(1, $resultSet->key()); } + #[Test] + public function nextReportsAnUninitialisedDataSourceWhileBuffering(): void + { + $resultSet = $this->createResultSetMock(); + $resultSet->buffer(); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage(RuntimeException::UNINITIALISED_DATA_SOURCE); + + $resultSet->next(); + } + + #[Test] + public function rewindReportsAnUninitialisedDataSourceRatherThanFailingOnNull(): void + { + $resultSet = $this->createResultSetMock(); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage(RuntimeException::UNINITIALISED_DATA_SOURCE); + + $resultSet->rewind(); + } + /** * @throws Exception */ diff --git a/test/unit/ResultSet/RowBufferTest.php b/test/unit/ResultSet/RowBufferTest.php deleted file mode 100644 index ee53d10c..00000000 --- a/test/unit/ResultSet/RowBufferTest.php +++ /dev/null @@ -1,172 +0,0 @@ -isPending()); - static::assertFalse($buffer->isStoring()); - static::assertFalse($buffer->isBuffered()); - } - - #[Test] - public function clearForgetsTheRowsItHeld(): void - { - $buffer = new RowBuffer(); - $buffer->store(); - $buffer->put(0, ['id' => 1]); - - $buffer->clear(); - - static::assertFalse($buffer->has(0)); - } - - #[Test] - public function clearLeavesTheStateAlone(): void - { - $buffer = new RowBuffer(); - $buffer->store(); - - $buffer->clear(); - - static::assertTrue($buffer->isStoring()); - } - - #[Test] - public function getReturnsNullForAPositionItHoldsNothingFor(): void - { - $buffer = new RowBuffer(); - $buffer->store(); - - static::assertNull($buffer->get(3)); - } - - #[Test] - public function hasIsFalseWhileNotStoringEvenAfterAPut(): void - { - $buffer = new RowBuffer(); - $buffer->useDataSource(); - $buffer->put(0, ['id' => 1]); - - static::assertFalse($buffer->has(0)); - } - - #[Test] - public function iterationCannotBeginAndThenBeBuffered(): void - { - $buffer = new RowBuffer(); - $buffer->startIteration(); - - self::expectException(RuntimeException::class); - self::expectExceptionMessage(RuntimeException::UNBUFFERED_ITERATION); - - $buffer->store(); - } - - #[Test] - public function iterationLeavesAStoringBufferStoring(): void - { - $buffer = new RowBuffer(); - $buffer->store(); - - $buffer->startIteration(); - - static::assertTrue($buffer->isStoring()); - } - - #[Test] - public function storeHoldsTheRowsPutIntoIt(): void - { - $buffer = new RowBuffer(); - $buffer->store(); - - $buffer->put(0, ['id' => 1]); - - static::assertTrue($buffer->has(0)); - static::assertSame(['id' => 1], $buffer->get(0)); - } - - #[Test] - public function storeLeavesAPassthroughBufferDeferringToItsDataSource(): void - { - $buffer = new RowBuffer(); - $buffer->useDataSource(); - - $buffer->store(); - - static::assertFalse($buffer->isStoring()); - static::assertTrue($buffer->isBuffered()); - } - - #[Test] - public function storeMakesTheBufferHoldRows(): void - { - $buffer = new RowBuffer(); - - $buffer->store(); - - static::assertTrue($buffer->isStoring()); - static::assertTrue($buffer->isBuffered()); - } - - #[Test] - public function storingTwiceKeepsTheRowsAlreadyHeld(): void - { - $buffer = new RowBuffer(); - $buffer->store(); - $buffer->put(0, ['id' => 1]); - - $buffer->store(); - - static::assertSame(['id' => 1], $buffer->get(0)); - } - - #[Test] - public function useDataSourceCountsAsBufferedWithoutStoring(): void - { - $buffer = new RowBuffer(); - - $buffer->useDataSource(); - - static::assertTrue($buffer->isBuffered()); - static::assertFalse($buffer->isStoring()); - static::assertFalse($buffer->isPending()); - } - - #[Test] - public function useDataSourceDropsTheRowsItWasHolding(): void - { - $buffer = new RowBuffer(); - $buffer->store(); - $buffer->put(0, ['id' => 1]); - - $buffer->useDataSource(); - - static::assertNull($buffer->get(0)); - } -} From 8a7521554f15390caccbbef70ca4d592a4727fe0 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 2 Oct 2026 16:13:15 +1000 Subject: [PATCH 11/17] Buffer raw rows and map them afresh on every pass A buffer held the rows mapRow() had shaped, so every pass handed out the same objects: a change made to a row in one pass showed up in the next, a row prototype set after a pass was ignored, and a buffer of ArrayObjects cost around 15 times the memory of the raw arrays plus cycle-collector scans. The buffer now holds the rows as the data source returned them and maps them on each read. ResultSet and RowPrototypeResultSet behave as they did in 0.6.x; HydratingResultSet now hydrates a fresh object on every pass instead of returning the same instance, which is documented. --- docs/book/result-set/advanced.md | 25 ++++++++++++++ src/ResultSet/AbstractResultSet.php | 31 +++++++---------- .../HydratingResultSetIntegrationTest.php | 9 +++-- .../unit/ResultSet/HydratingResultSetTest.php | 33 +++++++++---------- .../ResultSet/ResultSetIntegrationTest.php | 28 ++++++++++++++++ 5 files changed, 84 insertions(+), 42 deletions(-) diff --git a/docs/book/result-set/advanced.md b/docs/book/result-set/advanced.md index d0398275..00c914a0 100644 --- a/docs/book/result-set/advanced.md +++ b/docs/book/result-set/advanced.md @@ -200,6 +200,31 @@ RuntimeException: Buffering must be enabled before iteration is started ``` +### What a Buffer Holds + +A buffer holds the rows as the data source returned them, and every pass builds +its rows from those afresh. A row object from one pass is never handed out again, +so a change made to it does not carry into the next pass, and a new row prototype +or hydrator takes effect from the next pass: + +```php title="Each Pass Builds Its Own Rows" +$resultSet = new HydratingResultSet(new ReflectionHydrator(), new UserEntity()); +$resultSet->initialize($result); +$resultSet->buffer(); + +foreach ($resultSet as $user) { + $user->setName(strtoupper($user->getName())); +} + +foreach ($resultSet as $user) { + echo $user->getName(); // as read from the database, not upper-cased +} +``` + +**Note:** laminas-db and PhpDb 0.6 returned the same hydrated object on every pass +of a buffered `HydratingResultSet`. Code that relied on that identity, such as an +identity map keyed by row object, must keep its own reference to each object. + ### isBuffered() Method Checks if the result set is currently buffered: diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index 491ddeab..9216581f 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -56,12 +56,13 @@ abstract class AbstractResultSet implements ResultSetInterface private bool $storesRows = false; /** - * Rows held for later passes, keyed by position, already shaped by mapRow(). + * Raw rows held for later passes, keyed by position, and mapped afresh on each read + * so that no pass sees what a caller did to the rows of another. * * Only filled while Storing, and emptied by initialize() before the state can * leave Storing, so a row found here never needs the state checked as well. * - * @var array + * @var array */ private array $bufferedRows = []; @@ -355,20 +356,16 @@ protected function currentRow(): mixed /** @var Iterator $dataSource */ $dataSource = $this->dataSource; - $held = $this->bufferedRows[$this->position] ?? null; - - if (null !== $held) { - return $held; - } - /** @var mixed $row */ - $row = $dataSource->current(); + $row = $this->bufferedRows[$this->position] ?? $dataSource->current(); if (null === $row || false === $row) { return null; } - return $this->bufferedRows[$this->position] = $this->mapRow($row); + $this->bufferedRows[$this->position] = $row; + + return $this->mapRow($row); } $dataSource = $this->dataSource ?? throw RuntimeException::forUninitialisedDataSource(); @@ -377,24 +374,18 @@ protected function currentRow(): mixed $this->setBufferState(RowBufferState::Disabled); } - $held = $this->bufferedRows[$this->position] ?? null; - - if (null !== $held) { - return $held; - } - /** @var mixed $row */ - $row = $dataSource->current(); + $row = $this->bufferedRows[$this->position] ?? $dataSource->current(); if (null === $row || false === $row) { return null; } - if (RowBufferState::Storing !== $this->bufferState) { - return $this->mapRow($row); + if (RowBufferState::Storing === $this->bufferState) { + $this->bufferedRows[$this->position] = $row; } - return $this->bufferedRows[$this->position] = $this->mapRow($row); + return $this->mapRow($row); } /** diff --git a/test/unit/ResultSet/HydratingResultSetIntegrationTest.php b/test/unit/ResultSet/HydratingResultSetIntegrationTest.php index 04952f3d..ad9022f0 100644 --- a/test/unit/ResultSet/HydratingResultSetIntegrationTest.php +++ b/test/unit/ResultSet/HydratingResultSetIntegrationTest.php @@ -18,7 +18,7 @@ class HydratingResultSetIntegrationTest extends TestCase * @throws Exception */ #[Test] - public function currentWillReturnBufferedRow(): void + public function aSecondBufferedPassHydratesTheSameData(): void { $hydratingRs = new HydratingResultSet(); $hydratingRs->initialize(new ArrayIterator([ @@ -27,10 +27,9 @@ public function currentWillReturnBufferedRow(): void ])); $hydratingRs->buffer(); - // Get current object and rewind to verify same buffered object is returned - $obj1 = $hydratingRs->current(); + $first = $hydratingRs->current(); $hydratingRs->rewind(); - $obj2 = $hydratingRs->current(); - static::assertSame($obj1, $obj2); + + static::assertEquals($first, $hydratingRs->current()); } } diff --git a/test/unit/ResultSet/HydratingResultSetTest.php b/test/unit/ResultSet/HydratingResultSetTest.php index 3ac93c71..96a35026 100644 --- a/test/unit/ResultSet/HydratingResultSetTest.php +++ b/test/unit/ResultSet/HydratingResultSetTest.php @@ -39,6 +39,22 @@ final class HydratingResultSetTest extends TestCase private string $classMethodsHydratorClass; + #[Test] + public function aSecondBufferedPassHydratesAFreshObject(): void + { + $hydratingRs = new HydratingResultSet(); + $hydratingRs->initialize(new ArrayIterator([ + ['id' => 1, 'name' => 'one'], + ['id' => 2, 'name' => 'two'], + ])); + $hydratingRs->buffer(); + + $first = $hydratingRs->current(); + $hydratingRs->rewind(); + + static::assertNotSame($first, $hydratingRs->current()); + } + #[Test] public function constructorDefaultsToArraySerializableHydrator(): void { @@ -118,23 +134,6 @@ public function currentRejectsARowThatExposesNothing(): void $resultSet->current(); } - #[Test] - public function currentWithBufferReturnsBufferedObject(): void - { - $hydratingRs = new HydratingResultSet(); - $hydratingRs->initialize(new ArrayIterator([ - ['id' => 1, 'name' => 'one'], - ['id' => 2, 'name' => 'two'], - ])); - $hydratingRs->buffer(); - - $first = $hydratingRs->current(); - $hydratingRs->rewind(); - $buffered = $hydratingRs->current(); - - static::assertSame($first, $buffered); - } - #[Test] public function getHydrator(): void { diff --git a/test/unit/ResultSet/ResultSetIntegrationTest.php b/test/unit/ResultSet/ResultSetIntegrationTest.php index 0342a4e5..da73035b 100644 --- a/test/unit/ResultSet/ResultSetIntegrationTest.php +++ b/test/unit/ResultSet/ResultSetIntegrationTest.php @@ -84,6 +84,34 @@ public static function returnTypeProvider(): array return $cases; } + #[Test] + public function aChangeToARowInOneBufferedPassDoesNotReachTheNext(): void + { + $this->resultSet->initialize(new ArrayIterator([['id' => 1, 'name' => 'one']])); + $this->resultSet->buffer(); + + $row = $this->resultSet->current(); + static::assertInstanceOf(ArrayObject::class, $row); + $row['name'] = 'ONE'; + $this->resultSet->rewind(); + + static::assertSame(['id' => 1, 'name' => 'one'], $this->resultSet->current()?->getArrayCopy()); + } + + #[Test] + public function aRowPrototypeSetAfterABufferedPassShapesTheNext(): void + { + $this->resultSet->initialize(new ArrayIterator([['id' => 1]])); + $this->resultSet->buffer(); + $this->resultSet->current(); + + $prototype = new class extends ArrayObject {}; + $this->resultSet->setRowPrototype($prototype); + $this->resultSet->rewind(); + + static::assertInstanceOf($prototype::class, $this->resultSet->current()); + } + /** * @throws Exception * @throws \Exception From e5d0fede49bb382f1395bf097adf627e255b880f Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 2 Oct 2026 16:55:22 +1000 Subject: [PATCH 12/17] Honour getter overrides and keep hydrated rows across buffered passes mapRow() read the private row prototype and hydrator properties directly, so a subclass overriding getRowPrototype() or getHydrator() no longer decided how its rows were built. Each set now asks its getters once per data source and keeps the answer, which honours the overrides without a method call on every row. A buffered HydratingResultSet hydrated every row again on each pass, handing out new objects, so changes made to an entity in one pass were lost in the next. Result sets can now opt into holding the rows mapRow() built; HydratingResultSet does, and returns the same entities on every pass as laminas-db and earlier PhpDb releases did. ResultSet and RowPrototypeResultSet still build fresh rows from raw ones. --- docs/book/result-set/advanced.md | 23 +++--- src/ResultSet/AbstractResultSet.php | 70 +++++++++++++++++-- src/ResultSet/HydratingResultSet.php | 34 +++++++-- src/ResultSet/ResultSet.php | 17 ++++- src/ResultSet/RowPrototypeResultSet.php | 16 ++++- .../unit/ResultSet/HydratingResultSetTest.php | 64 ++++++++++++++++- .../ResultSet/ResultSetIntegrationTest.php | 32 +++++++++ .../ResultSet/RowPrototypeResultSetTest.php | 22 ++++++ 8 files changed, 250 insertions(+), 28 deletions(-) diff --git a/docs/book/result-set/advanced.md b/docs/book/result-set/advanced.md index 00c914a0..99255cf4 100644 --- a/docs/book/result-set/advanced.md +++ b/docs/book/result-set/advanced.md @@ -202,12 +202,17 @@ started ### What a Buffer Holds -A buffer holds the rows as the data source returned them, and every pass builds -its rows from those afresh. A row object from one pass is never handed out again, -so a change made to it does not carry into the next pass, and a new row prototype -or hydrator takes effect from the next pass: +What a buffered pass hands back depends on the kind of row. -```php title="Each Pass Builds Its Own Rows" +- `ResultSet` and `RowPrototypeResultSet` keep the rows as the data source returned + them and build each row afresh on every pass. A change made to an `ArrayObject` or + row prototype in one pass does not carry into the next. +- `HydratingResultSet` keeps the objects it hydrated on the first pass and hands the + same objects out on every later pass, as laminas-db and earlier PhpDb releases did. + A hydrated row is an entity with an identity of its own, so a change made to it in + one pass is still there in the next, and nothing is hydrated twice. + +```php title="Buffered Entities Keep Their Identity" $resultSet = new HydratingResultSet(new ReflectionHydrator(), new UserEntity()); $resultSet->initialize($result); $resultSet->buffer(); @@ -217,13 +222,13 @@ foreach ($resultSet as $user) { } foreach ($resultSet as $user) { - echo $user->getName(); // as read from the database, not upper-cased + echo $user->getName(); // upper-cased: the same objects as the first pass } ``` -**Note:** laminas-db and PhpDb 0.6 returned the same hydrated object on every pass -of a buffered `HydratingResultSet`. Code that relied on that identity, such as an -identity map keyed by row object, must keep its own reference to each object. +The row prototype and hydrator are read through `getRowPrototype()` and +`getHydrator()` once for each data source, so a subclass that overrides either getter +decides how its rows are built. ### isBuffered() Method diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index 9216581f..cf262d4f 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -55,6 +55,15 @@ abstract class AbstractResultSet implements ResultSetInterface */ private bool $storesRows = false; + /** + * Whether the buffer holds the rows mapRow() built rather than the raw rows. + * + * Derived from holdsMappedRows() by setBufferState(). A result set whose rows are + * objects with an identity of their own, such as hydrated entities, hands the same + * objects out on every pass; one whose rows are cheap copies builds them afresh. + */ + private bool $holdsMappedRows = false; + /** * Raw rows held for later passes, keyed by position, and mapped afresh on each read * so that no pass sees what a caller did to the rows of another. @@ -197,6 +206,7 @@ public function getFieldCount(): int public function initialize(iterable $dataSource): ResultSetInterface { $this->bufferedRows = []; + $this->resetResolvedConfiguration(); if ($dataSource instanceof ResultInterface) { $this->initializeFromResult($dataSource); @@ -353,19 +363,24 @@ protected function currentRow(): mixed } if ($this->storesRows) { + /** @var mixed $held */ + $held = $this->bufferedRows[$this->position] ?? null; + + if (null !== $held && $this->holdsMappedRows) { + return $held; + } + /** @var Iterator $dataSource */ $dataSource = $this->dataSource; /** @var mixed $row */ - $row = $this->bufferedRows[$this->position] ?? $dataSource->current(); + $row = $held ?? $dataSource->current(); if (null === $row || false === $row) { return null; } - $this->bufferedRows[$this->position] = $row; - - return $this->mapRow($row); + return $this->holdRow($row); } $dataSource = $this->dataSource ?? throw RuntimeException::forUninitialisedDataSource(); @@ -374,20 +389,46 @@ protected function currentRow(): mixed $this->setBufferState(RowBufferState::Disabled); } + /** @var mixed $held */ + $held = $this->bufferedRows[$this->position] ?? null; + + if (null !== $held && $this->holdsMappedRows) { + return $held; + } + /** @var mixed $row */ - $row = $this->bufferedRows[$this->position] ?? $dataSource->current(); + $row = $held ?? $dataSource->current(); if (null === $row || false === $row) { return null; } if (RowBufferState::Storing === $this->bufferState) { - $this->bufferedRows[$this->position] = $row; + return $this->holdRow($row); } return $this->mapRow($row); } + /** + * Whether buffered passes hand out the rows built on the first pass. + * + * False by default: the buffer keeps raw rows and every pass builds its own. A + * result set whose rows carry an identity of their own overrides this. + */ + protected function holdsMappedRows(): bool + { + return false; + } + + /** + * Forget anything a subclass resolved from its getters, so that the next row asks + * them again. Called by initialize(); a subclass's setters call it too. + */ + protected function resetResolvedConfiguration(): void + { + } + /** * The data source, once initialize() has supplied one. * @@ -458,5 +499,22 @@ private function setBufferState(RowBufferState $state): void $this->bufferState = $state; $this->readsDirectly = RowBufferState::Passthrough === $state || RowBufferState::Disabled === $state; $this->storesRows = RowBufferState::Storing === $state && null !== $this->dataSource; + $this->holdsMappedRows = RowBufferState::Storing === $state && $this->holdsMappedRows(); + } + + /** + * Keep a row for later passes and return it mapped. + * + * @return TRow + */ + private function holdRow(mixed $row): mixed + { + if ($this->holdsMappedRows) { + return $this->bufferedRows[$this->position] = $this->mapRow($row); + } + + $this->bufferedRows[$this->position] = $row; + + return $this->mapRow($row); } } diff --git a/src/ResultSet/HydratingResultSet.php b/src/ResultSet/HydratingResultSet.php index 14ad8619..e6c2bea0 100644 --- a/src/ResultSet/HydratingResultSet.php +++ b/src/ResultSet/HydratingResultSet.php @@ -19,6 +19,11 @@ */ class HydratingResultSet extends AbstractResultSet implements HydratingResultSetInterface { + /** getHydrator() and getRowPrototype(), asked once per data source rather than once per row. */ + private ?HydratorInterface $resolvedHydrator = null; + + private ?object $resolvedRowPrototype = null; + public function __construct( private ?HydratorInterface $hydrator = null, private ?object $rowPrototype = null, @@ -62,7 +67,8 @@ public function getRowPrototype(): object */ public function setHydrator(HydratorInterface $hydrator): ResultSetInterface { - $this->hydrator = $hydrator; + $this->hydrator = $hydrator; + $this->resolvedHydrator = null; return $this; } @@ -77,7 +83,8 @@ public function setObjectPrototype(object $objectPrototype): ResultSetInterface public function setRowPrototype( object $rowPrototype, ): ResultSetInterface&HydratingResultSetInterface { - $this->rowPrototype = $rowPrototype; + $this->rowPrototype = $rowPrototype; + $this->resolvedRowPrototype = null; return $this; } @@ -108,11 +115,26 @@ public function toArray(): array #[Override] protected function mapRow(mixed $row): object { - $this->rowPrototype ??= new ArrayObject(); - - return ($this->hydrator ??= new ArraySerializableHydrator())->hydrate( + return ($this->resolvedHydrator ??= $this->getHydrator())->hydrate( is_array($row) ? $row : $this->getArrayData($row), - clone $this->rowPrototype, + clone ($this->resolvedRowPrototype ??= $this->getRowPrototype()), ); } + + /** + * Hydrated rows are entities with an identity of their own, so a buffered set hands + * the same objects out on every pass, as laminas-db and earlier PhpDb releases did. + */ + #[Override] + protected function holdsMappedRows(): bool + { + return true; + } + + #[Override] + protected function resetResolvedConfiguration(): void + { + $this->resolvedHydrator = null; + $this->resolvedRowPrototype = null; + } } diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index caf1f475..b39ae099 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -31,6 +31,12 @@ class ResultSet extends AbstractResultSet implements ArrayObjectResultSetInterfa */ private readonly bool $fillsRowPrototype; + /** + * getRowPrototype(), asked once per data source rather than once per row, so that a + * subclass overriding the getter is honoured without a method call on every row. + */ + private ?ArrayObject $resolvedRowPrototype = null; + public function __construct( ResultSetReturnType|string $returnType = ResultSetReturnType::ArrayObject, private ArrayObject $rowPrototype = new ArrayObject( @@ -98,7 +104,8 @@ public function setArrayObjectPrototype( public function setRowPrototype( ArrayObject $rowPrototype, ): ResultSetInterface&ArrayObjectResultSetInterface { - $this->rowPrototype = $rowPrototype; + $this->rowPrototype = $rowPrototype; + $this->resolvedRowPrototype = null; return $this; } @@ -138,9 +145,15 @@ protected function mapRow(mixed $row): array|ArrayObject return $row; } - $ao = clone $this->rowPrototype; + $ao = clone ($this->resolvedRowPrototype ??= $this->getRowPrototype()); $ao->exchangeArray($row); return $ao; } + + #[Override] + protected function resetResolvedConfiguration(): void + { + $this->resolvedRowPrototype = null; + } } diff --git a/src/ResultSet/RowPrototypeResultSet.php b/src/ResultSet/RowPrototypeResultSet.php index d898fe26..5335cbdc 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -16,6 +16,9 @@ */ class RowPrototypeResultSet extends AbstractResultSet implements RowPrototypeResultSetInterface { + /** getRowPrototype(), asked once per data source rather than once per row. */ + private ?RowPrototypeInterface $resolvedRowPrototype = null; + public function __construct( private RowPrototypeInterface $rowPrototype, ) {} @@ -44,7 +47,8 @@ public function getRowPrototype(): RowPrototypeInterface public function setRowPrototype( RowPrototypeInterface $rowPrototype, ): ResultSetInterface&RowPrototypeResultSetInterface { - $this->rowPrototype = $rowPrototype; + $this->rowPrototype = $rowPrototype; + $this->resolvedRowPrototype = null; return $this; } @@ -71,14 +75,14 @@ public function toArray(): array protected function mapRow(mixed $row): RowPrototypeInterface { if (is_array($row)) { - return (clone $this->rowPrototype)->populate($row); + return (clone ($this->resolvedRowPrototype ??= $this->getRowPrototype()))->populate($row); } if ($row instanceof RowPrototypeInterface) { return $row; } - return (clone $this->rowPrototype)->populate($this->getArrayData($row)); + return (clone ($this->resolvedRowPrototype ??= $this->getRowPrototype()))->populate($this->getArrayData($row)); } /** {@inheritDoc} */ @@ -87,4 +91,10 @@ protected function unsupportedRowError(mixed $row): Exception\ValueError { return Exception\ValueError::forRowThatIsNotArrayDataOrPrototype(get_debug_type($row), static::class); } + + #[Override] + protected function resetResolvedConfiguration(): void + { + $this->resolvedRowPrototype = null; + } } diff --git a/test/unit/ResultSet/HydratingResultSetTest.php b/test/unit/ResultSet/HydratingResultSetTest.php index 96a35026..c5fb126d 100644 --- a/test/unit/ResultSet/HydratingResultSetTest.php +++ b/test/unit/ResultSet/HydratingResultSetTest.php @@ -40,7 +40,7 @@ final class HydratingResultSetTest extends TestCase private string $classMethodsHydratorClass; #[Test] - public function aSecondBufferedPassHydratesAFreshObject(): void + public function aSecondBufferedPassReturnsTheSameObject(): void { $hydratingRs = new HydratingResultSet(); $hydratingRs->initialize(new ArrayIterator([ @@ -52,7 +52,67 @@ public function aSecondBufferedPassHydratesAFreshObject(): void $first = $hydratingRs->current(); $hydratingRs->rewind(); - static::assertNotSame($first, $hydratingRs->current()); + static::assertSame($first, $hydratingRs->current()); + } + + #[Test] + public function aChangeToABufferedEntityCarriesIntoTheNextPass(): void + { + $hydratingRs = new HydratingResultSet(null, new ArrayObject([], ArrayObject::ARRAY_AS_PROPS)); + $hydratingRs->initialize(new ArrayIterator([ + ['id' => 1, 'name' => 'one'], + ['id' => 2, 'name' => 'two'], + ])); + $hydratingRs->buffer(); + + foreach ($hydratingRs as $row) { + $row['name'] = 'changed'; + } + + $names = []; + foreach ($hydratingRs as $row) { + $names[] = $row['name']; + } + + static::assertSame(['changed', 'changed'], $names); + } + + #[Test] + public function subclassGettersDecideHowRowsAreHydrated(): void + { + $hydratingRs = new class extends HydratingResultSet { + public int $hydratorCalls = 0; + + #[Override] + public function getHydrator(): ClassMethodsHydrator + { + $this->hydratorCalls++; + + return new ClassMethodsHydrator(); + } + + #[Override] + public function getRowPrototype(): object + { + return new class { + public ?int $id = null; + + public function setId(int $id): void + { + $this->id = $id * 10; + } + }; + } + }; + $hydratingRs->initialize(new ArrayIterator([['id' => 1], ['id' => 2]])); + + $ids = []; + foreach ($hydratingRs as $row) { + $ids[] = $row->id; + } + + static::assertSame([10, 20], $ids); + static::assertSame(1, $hydratingRs->hydratorCalls, 'getHydrator() is asked once per data source, not per row'); } #[Test] diff --git a/test/unit/ResultSet/ResultSetIntegrationTest.php b/test/unit/ResultSet/ResultSetIntegrationTest.php index da73035b..71fc68a9 100644 --- a/test/unit/ResultSet/ResultSetIntegrationTest.php +++ b/test/unit/ResultSet/ResultSetIntegrationTest.php @@ -475,4 +475,36 @@ protected function setUp(): void { $this->resultSet = new ResultSet(); } + + #[Test] + public function aSubclassGetRowPrototypeDecidesTheRowClass(): void + { + $resultSet = new class extends ResultSet { + #[Override] + public function getRowPrototype(): ArrayObject + { + return new class ([], ArrayObject::ARRAY_AS_PROPS) extends ArrayObject {}; + } + }; + $resultSet->initialize([['id' => 1]]); + + $row = $resultSet->current(); + + static::assertInstanceOf(ArrayObject::class, $row); + static::assertNotSame(ArrayObject::class, $row::class); + } + + #[Test] + public function aBufferedResultSetBuildsFreshRowsOnEveryPass(): void + { + $resultSet = new ResultSet(); + $resultSet->initialize(new ArrayIterator([['id' => 1], ['id' => 2]])); + $resultSet->buffer(); + + $first = $resultSet->current(); + $resultSet->rewind(); + + static::assertNotSame($first, $resultSet->current()); + static::assertEquals($first, $resultSet->current()); + } } diff --git a/test/unit/ResultSet/RowPrototypeResultSetTest.php b/test/unit/ResultSet/RowPrototypeResultSetTest.php index 5144e842..ed5ccef1 100644 --- a/test/unit/ResultSet/RowPrototypeResultSetTest.php +++ b/test/unit/ResultSet/RowPrototypeResultSetTest.php @@ -132,6 +132,28 @@ public function toArrayConvertsPrototypeRowsToArrays(): void ); } + #[Test] + public function aSubclassGetRowPrototypeDecidesTheRowClass(): void + { + $custom = $this->createRowPrototype(); + $resultSet = new class ($this->createRowPrototype(), $custom) extends RowPrototypeResultSet { + public function __construct(RowPrototypeInterface $prototype, private RowPrototypeInterface $custom) + { + parent::__construct($prototype); + } + + #[\Override] + public function getRowPrototype(): RowPrototypeInterface + { + return $this->custom; + } + }; + $resultSet->initialize([['id' => 1]]); + + static::assertSame($custom::class, $resultSet->current()::class); + static::assertNotSame($custom, $resultSet->current(), 'the prototype is cloned for each row'); + } + private function createRowPrototype(): RowPrototypeInterface { return new class implements RowPrototypeInterface { From 8fa7cdb6748a7db0db84225dd0129574a6cf77ab Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 2 Oct 2026 17:16:21 +1000 Subject: [PATCH 13/17] Drop the unreachable buffering branch from the first read and cover the new hooks currentRow() only reaches its last branch on the first read of an unbuffered set: Storing with a data source always takes the buffered branch, and every other settled state reads directly. The buffering code left in that branch could not run, which left a line uncovered. The tests now name the methods they exercise, so the new hooks count towards coverage: holdRow(), holdsMappedRows() and resetResolvedConfiguration(), plus RowPrototypeResultSet's constructor, getter and setter. --- src/ResultSet/AbstractResultSet.php | 25 ++++--------------- test/unit/ResultSet/AbstractResultSetTest.php | 3 +++ .../unit/ResultSet/HydratingResultSetTest.php | 20 +++++++++++++++ .../ResultSet/ResultSetIntegrationTest.php | 18 +++++++++++++ .../ResultSet/RowPrototypeResultSetTest.php | 20 +++++++++++++++ 5 files changed, 66 insertions(+), 20 deletions(-) diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index cf262d4f..082cd1b7 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -383,31 +383,16 @@ protected function currentRow(): mixed return $this->holdRow($row); } + // Only the first read of an unbuffered set lands here: Storing with a data source + // takes the branch above, and every other settled state reads directly. $dataSource = $this->dataSource ?? throw RuntimeException::forUninitialisedDataSource(); - if (RowBufferState::Pending === $this->bufferState) { - $this->setBufferState(RowBufferState::Disabled); - } - - /** @var mixed $held */ - $held = $this->bufferedRows[$this->position] ?? null; - - if (null !== $held && $this->holdsMappedRows) { - return $held; - } + $this->setBufferState(RowBufferState::Disabled); /** @var mixed $row */ - $row = $held ?? $dataSource->current(); - - if (null === $row || false === $row) { - return null; - } - - if (RowBufferState::Storing === $this->bufferState) { - return $this->holdRow($row); - } + $row = $dataSource->current(); - return $this->mapRow($row); + return null === $row || false === $row ? null : $this->mapRow($row); } /** diff --git a/test/unit/ResultSet/AbstractResultSetTest.php b/test/unit/ResultSet/AbstractResultSetTest.php index 045191bf..3f1b4912 100644 --- a/test/unit/ResultSet/AbstractResultSetTest.php +++ b/test/unit/ResultSet/AbstractResultSetTest.php @@ -40,6 +40,9 @@ #[CoversMethod(AbstractResultSet::class, 'valid')] #[CoversMethod(AbstractResultSet::class, 'rewind')] #[CoversMethod(AbstractResultSet::class, 'count')] +#[CoversMethod(AbstractResultSet::class, 'resetResolvedConfiguration')] +#[CoversMethod(AbstractResultSet::class, 'holdsMappedRows')] +#[CoversMethod(AbstractResultSet::class, 'holdRow')] final class AbstractResultSetTest extends TestCase { protected AbstractResultSet $resultSet; diff --git a/test/unit/ResultSet/HydratingResultSetTest.php b/test/unit/ResultSet/HydratingResultSetTest.php index c5fb126d..945bcf93 100644 --- a/test/unit/ResultSet/HydratingResultSetTest.php +++ b/test/unit/ResultSet/HydratingResultSetTest.php @@ -32,6 +32,10 @@ #[CoversMethod(HydratingResultSet::class, '__construct')] #[CoversMethod(HydratingResultSet::class, 'setRowPrototype')] #[CoversMethod(HydratingResultSet::class, 'getRowPrototype')] +#[CoversMethod(HydratingResultSet::class, 'holdsMappedRows')] +#[CoversMethod(HydratingResultSet::class, 'resetResolvedConfiguration')] +#[CoversMethod(AbstractResultSet::class, 'currentRow')] +#[CoversMethod(AbstractResultSet::class, 'holdRow')] #[Group('unit')] final class HydratingResultSetTest extends TestCase { @@ -332,4 +336,20 @@ protected function setUp(): void $this->arraySerializableHydratorClass = ArraySerializableHydrator::class; $this->classMethodsHydratorClass = ClassMethodsHydrator::class; } + + #[Test] + public function aNewHydratorOrPrototypeIsUsedForTheNextDataSource(): void + { + $hydratingRs = new HydratingResultSet(null, new ArrayObject()); + $hydratingRs->initialize(new ArrayIterator([['id' => 1]])); + static::assertInstanceOf(ArrayObject::class, $hydratingRs->current()); + + $hydratingRs->setRowPrototype(new stdClass()); + $hydratingRs->setHydrator(new \Laminas\Hydrator\ObjectPropertyHydrator()); + $hydratingRs->initialize(new ArrayIterator([['id' => 2]])); + + $row = $hydratingRs->current(); + static::assertInstanceOf(stdClass::class, $row); + static::assertSame(2, $row->id); + } } diff --git a/test/unit/ResultSet/ResultSetIntegrationTest.php b/test/unit/ResultSet/ResultSetIntegrationTest.php index 71fc68a9..7298673a 100644 --- a/test/unit/ResultSet/ResultSetIntegrationTest.php +++ b/test/unit/ResultSet/ResultSetIntegrationTest.php @@ -43,6 +43,10 @@ #[CoversMethod(ResultSet::class, 'setArrayObjectPrototype')] #[CoversMethod(ResultSet::class, 'setRowPrototype')] #[CoversMethod(ResultSet::class, 'toArray')] +#[CoversMethod(ResultSet::class, 'resetResolvedConfiguration')] +#[CoversMethod(AbstractResultSet::class, 'holdRow')] +#[CoversMethod(AbstractResultSet::class, 'holdsMappedRows')] +#[CoversMethod(AbstractResultSet::class, 'resetResolvedConfiguration')] #[Group('unit')] final class ResultSetIntegrationTest extends TestCase { @@ -507,4 +511,18 @@ public function aBufferedResultSetBuildsFreshRowsOnEveryPass(): void static::assertNotSame($first, $resultSet->current()); static::assertEquals($first, $resultSet->current()); } + + #[Test] + public function aBufferedPassAfterSetRowPrototypeUsesTheNewPrototype(): void + { + $resultSet = new ResultSet(); + $resultSet->initialize(new ArrayIterator([['id' => 1]])); + $resultSet->buffer(); + $resultSet->current(); + + $resultSet->setRowPrototype(new class ([], ArrayObject::ARRAY_AS_PROPS) extends ArrayObject {}); + $resultSet->rewind(); + + static::assertNotSame(ArrayObject::class, $resultSet->current()::class); + } } diff --git a/test/unit/ResultSet/RowPrototypeResultSetTest.php b/test/unit/ResultSet/RowPrototypeResultSetTest.php index ed5ccef1..0873a315 100644 --- a/test/unit/ResultSet/RowPrototypeResultSetTest.php +++ b/test/unit/ResultSet/RowPrototypeResultSetTest.php @@ -21,6 +21,10 @@ #[CoversMethod(RowPrototypeResultSet::class, 'current')] #[CoversMethod(RowPrototypeResultSet::class, 'mapRow')] #[CoversMethod(RowPrototypeResultSet::class, 'toArray')] +#[CoversMethod(RowPrototypeResultSet::class, '__construct')] +#[CoversMethod(RowPrototypeResultSet::class, 'getRowPrototype')] +#[CoversMethod(RowPrototypeResultSet::class, 'setRowPrototype')] +#[CoversMethod(RowPrototypeResultSet::class, 'resetResolvedConfiguration')] #[Group('unit')] final class RowPrototypeResultSetTest extends TestCase { @@ -172,4 +176,20 @@ public function toArray(): array } }; } + + #[Test] + public function setRowPrototypeReplacesThePrototypeForLaterRows(): void + { + $first = $this->createRowPrototype(); + $second = $this->createRowPrototype(); + $resultSet = new RowPrototypeResultSet($first); + $resultSet->initialize([['id' => 1], ['id' => 2]]); + $resultSet->current(); + + static::assertSame($resultSet, $resultSet->setRowPrototype($second)); + static::assertSame($second, $resultSet->getRowPrototype()); + + $resultSet->next(); + static::assertSame(['id' => 2], $resultSet->current()->toArray()); + } } From b96d571a1a8ab5e2b5760dbfee7be3f55df46dea Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 2 Oct 2026 18:57:32 +1000 Subject: [PATCH 14/17] Pass Mago on the getter and mapped-row changes - Resolve each row prototype in its own statement before cloning it. The formatter strips the parentheses from clone ($x ??= ...), and the analyzer then misreads the expression. - HOLDS_MAPPED_ROWS replaces the holdsMappedRows() hook as a protected bool constant read through static::. Its @var bool stops the analyzer taking the base class's false as the only possible value. - HydratingResultSet no longer overrides resetResolvedConfiguration(), which kept it within the method limit. It asks its getters on the first row and again after setHydrator() or setRowPrototype(). - Annotate the held row currentRow() returns as TRow. - The lint baseline allows a third Halstead finding in AbstractResultSet, for initialize(). --- lint-baseline.toml | 2 +- src/ResultSet/AbstractResultSet.php | 80 +++++------ src/ResultSet/HydratingResultSet.php | 36 +++-- src/ResultSet/ResultSet.php | 4 +- src/ResultSet/RowPrototypeResultSet.php | 18 ++- test/unit/ResultSet/AbstractResultSetTest.php | 1 - .../unit/ResultSet/HydratingResultSetTest.php | 125 +++++++++--------- .../ResultSet/ResultSetIntegrationTest.php | 93 +++++++------ .../ResultSet/RowPrototypeResultSetTest.php | 79 +++++------ 9 files changed, 211 insertions(+), 227 deletions(-) diff --git a/lint-baseline.toml b/lint-baseline.toml index de4db269..82306798 100644 --- a/lint-baseline.toml +++ b/lint-baseline.toml @@ -256,7 +256,7 @@ count = 1 file = "src/ResultSet/AbstractResultSet.php" code = "halstead" message = "Method has a high halstead difficulty" -count = 2 +count = 3 [[issues]] file = "src/ResultSet/AbstractResultSet.php" diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index 082cd1b7..62357202 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -36,6 +36,16 @@ abstract class AbstractResultSet implements ResultSetInterface /** How deeply initialize() will unwrap a chain of IteratorAggregate. */ private const int MAX_ITERATOR_DEPTH = 8; + /** + * Whether buffered passes hand out the rows built on the first pass. + * + * False by default: the buffer keeps raw rows and every pass builds its own. A + * result set whose rows carry an identity of their own redeclares this as true. + * + * @var bool + */ + protected const bool HOLDS_MAPPED_ROWS = false; + private RowBufferState $bufferState = RowBufferState::Pending; /** @@ -55,15 +65,6 @@ abstract class AbstractResultSet implements ResultSetInterface */ private bool $storesRows = false; - /** - * Whether the buffer holds the rows mapRow() built rather than the raw rows. - * - * Derived from holdsMappedRows() by setBufferState(). A result set whose rows are - * objects with an identity of their own, such as hydrated entities, hands the same - * objects out on every pass; one whose rows are cheap copies builds them afresh. - */ - private bool $holdsMappedRows = false; - /** * Raw rows held for later passes, keyed by position, and mapped afresh on each read * so that no pass sees what a caller did to the rows of another. @@ -366,7 +367,8 @@ protected function currentRow(): mixed /** @var mixed $held */ $held = $this->bufferedRows[$this->position] ?? null; - if (null !== $held && $this->holdsMappedRows) { + if (null !== $held && static::HOLDS_MAPPED_ROWS) { + /** @var TRow $held */ return $held; } @@ -395,25 +397,6 @@ protected function currentRow(): mixed return null === $row || false === $row ? null : $this->mapRow($row); } - /** - * Whether buffered passes hand out the rows built on the first pass. - * - * False by default: the buffer keeps raw rows and every pass builds its own. A - * result set whose rows carry an identity of their own overrides this. - */ - protected function holdsMappedRows(): bool - { - return false; - } - - /** - * Forget anything a subclass resolved from its getters, so that the next row asks - * them again. Called by initialize(); a subclass's setters call it too. - */ - protected function resetResolvedConfiguration(): void - { - } - /** * The data source, once initialize() has supplied one. * @@ -451,6 +434,12 @@ protected function getArrayData(mixed $row): array throw $this->unsupportedRowError($row); } + /** + * Forget anything a subclass resolved from its getters, so that the next row asks + * them again. Called by initialize(); a subclass's setters call it too. + */ + protected function resetResolvedConfiguration(): void {} + /** * The error describing a row this result set cannot read. * @@ -462,6 +451,22 @@ protected function unsupportedRowError(mixed $row): ValueError return ValueError::forRowThatIsNotArrayData(get_debug_type($row), static::class); } + /** + * Keep a row for later passes and return it mapped. + * + * @return TRow + */ + private function holdRow(mixed $row): mixed + { + if (static::HOLDS_MAPPED_ROWS) { + return $this->bufferedRows[$this->position] = $this->mapRow($row); + } + + $this->bufferedRows[$this->position] = $row; + + return $this->mapRow($row); + } + private function initializeFromResult(ResultInterface $result): void { $this->fieldCount = $result->getFieldCount(); @@ -484,22 +489,5 @@ private function setBufferState(RowBufferState $state): void $this->bufferState = $state; $this->readsDirectly = RowBufferState::Passthrough === $state || RowBufferState::Disabled === $state; $this->storesRows = RowBufferState::Storing === $state && null !== $this->dataSource; - $this->holdsMappedRows = RowBufferState::Storing === $state && $this->holdsMappedRows(); - } - - /** - * Keep a row for later passes and return it mapped. - * - * @return TRow - */ - private function holdRow(mixed $row): mixed - { - if ($this->holdsMappedRows) { - return $this->bufferedRows[$this->position] = $this->mapRow($row); - } - - $this->bufferedRows[$this->position] = $row; - - return $this->mapRow($row); } } diff --git a/src/ResultSet/HydratingResultSet.php b/src/ResultSet/HydratingResultSet.php index e6c2bea0..b5bddd6e 100644 --- a/src/ResultSet/HydratingResultSet.php +++ b/src/ResultSet/HydratingResultSet.php @@ -19,7 +19,16 @@ */ class HydratingResultSet extends AbstractResultSet implements HydratingResultSetInterface { - /** getHydrator() and getRowPrototype(), asked once per data source rather than once per row. */ + /** + * Hydrated rows are entities with an identity of their own, so a buffered set hands + * the same objects out on every pass, as laminas-db and earlier PhpDb releases did. + */ + protected const bool HOLDS_MAPPED_ROWS = true; + + /** + * getHydrator() and getRowPrototype(), asked on the first row rather than on every + * row, and asked again after setHydrator() or setRowPrototype(). + */ private ?HydratorInterface $resolvedHydrator = null; private ?object $resolvedRowPrototype = null; @@ -115,26 +124,13 @@ public function toArray(): array #[Override] protected function mapRow(mixed $row): object { - return ($this->resolvedHydrator ??= $this->getHydrator())->hydrate( - is_array($row) ? $row : $this->getArrayData($row), - clone ($this->resolvedRowPrototype ??= $this->getRowPrototype()), - ); - } + $data = is_array($row) ? $row : $this->getArrayData($row); - /** - * Hydrated rows are entities with an identity of their own, so a buffered set hands - * the same objects out on every pass, as laminas-db and earlier PhpDb releases did. - */ - #[Override] - protected function holdsMappedRows(): bool - { - return true; - } + $this->resolvedRowPrototype ??= $this->getRowPrototype(); + $prototype = clone $this->resolvedRowPrototype; - #[Override] - protected function resetResolvedConfiguration(): void - { - $this->resolvedHydrator = null; - $this->resolvedRowPrototype = null; + $this->resolvedHydrator ??= $this->getHydrator(); + + return $this->resolvedHydrator->hydrate($data, $prototype); } } diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index b39ae099..221bbd2d 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -145,7 +145,9 @@ protected function mapRow(mixed $row): array|ArrayObject return $row; } - $ao = clone ($this->resolvedRowPrototype ??= $this->getRowPrototype()); + $this->resolvedRowPrototype ??= $this->getRowPrototype(); + + $ao = clone $this->resolvedRowPrototype; $ao->exchangeArray($row); return $ao; diff --git a/src/ResultSet/RowPrototypeResultSet.php b/src/ResultSet/RowPrototypeResultSet.php index 5335cbdc..4f7042a3 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -74,27 +74,25 @@ public function toArray(): array #[Override] protected function mapRow(mixed $row): RowPrototypeInterface { - if (is_array($row)) { - return (clone ($this->resolvedRowPrototype ??= $this->getRowPrototype()))->populate($row); - } - if ($row instanceof RowPrototypeInterface) { return $row; } - return (clone ($this->resolvedRowPrototype ??= $this->getRowPrototype()))->populate($this->getArrayData($row)); + $this->resolvedRowPrototype ??= $this->getRowPrototype(); + + return (clone $this->resolvedRowPrototype)->populate(is_array($row) ? $row : $this->getArrayData($row)); } - /** {@inheritDoc} */ #[Override] - protected function unsupportedRowError(mixed $row): Exception\ValueError + protected function resetResolvedConfiguration(): void { - return Exception\ValueError::forRowThatIsNotArrayDataOrPrototype(get_debug_type($row), static::class); + $this->resolvedRowPrototype = null; } + /** {@inheritDoc} */ #[Override] - protected function resetResolvedConfiguration(): void + protected function unsupportedRowError(mixed $row): Exception\ValueError { - $this->resolvedRowPrototype = null; + return Exception\ValueError::forRowThatIsNotArrayDataOrPrototype(get_debug_type($row), static::class); } } diff --git a/test/unit/ResultSet/AbstractResultSetTest.php b/test/unit/ResultSet/AbstractResultSetTest.php index 3f1b4912..1aa4a628 100644 --- a/test/unit/ResultSet/AbstractResultSetTest.php +++ b/test/unit/ResultSet/AbstractResultSetTest.php @@ -41,7 +41,6 @@ #[CoversMethod(AbstractResultSet::class, 'rewind')] #[CoversMethod(AbstractResultSet::class, 'count')] #[CoversMethod(AbstractResultSet::class, 'resetResolvedConfiguration')] -#[CoversMethod(AbstractResultSet::class, 'holdsMappedRows')] #[CoversMethod(AbstractResultSet::class, 'holdRow')] final class AbstractResultSetTest extends TestCase { diff --git a/test/unit/ResultSet/HydratingResultSetTest.php b/test/unit/ResultSet/HydratingResultSetTest.php index 945bcf93..c358f441 100644 --- a/test/unit/ResultSet/HydratingResultSetTest.php +++ b/test/unit/ResultSet/HydratingResultSetTest.php @@ -9,6 +9,7 @@ use Exception; use Laminas\Hydrator\ArraySerializableHydrator; use Laminas\Hydrator\ClassMethodsHydrator; +use Laminas\Hydrator\ObjectPropertyHydrator; use Override; use PhpDb\ResultSet\AbstractResultSet; use PhpDb\ResultSet\Exception\RuntimeException; @@ -32,8 +33,6 @@ #[CoversMethod(HydratingResultSet::class, '__construct')] #[CoversMethod(HydratingResultSet::class, 'setRowPrototype')] #[CoversMethod(HydratingResultSet::class, 'getRowPrototype')] -#[CoversMethod(HydratingResultSet::class, 'holdsMappedRows')] -#[CoversMethod(HydratingResultSet::class, 'resetResolvedConfiguration')] #[CoversMethod(AbstractResultSet::class, 'currentRow')] #[CoversMethod(AbstractResultSet::class, 'holdRow')] #[Group('unit')] @@ -43,22 +42,6 @@ final class HydratingResultSetTest extends TestCase private string $classMethodsHydratorClass; - #[Test] - public function aSecondBufferedPassReturnsTheSameObject(): void - { - $hydratingRs = new HydratingResultSet(); - $hydratingRs->initialize(new ArrayIterator([ - ['id' => 1, 'name' => 'one'], - ['id' => 2, 'name' => 'two'], - ])); - $hydratingRs->buffer(); - - $first = $hydratingRs->current(); - $hydratingRs->rewind(); - - static::assertSame($first, $hydratingRs->current()); - } - #[Test] public function aChangeToABufferedEntityCarriesIntoTheNextPass(): void { @@ -82,41 +65,35 @@ public function aChangeToABufferedEntityCarriesIntoTheNextPass(): void } #[Test] - public function subclassGettersDecideHowRowsAreHydrated(): void + public function aNewHydratorOrPrototypeIsUsedForTheNextDataSource(): void { - $hydratingRs = new class extends HydratingResultSet { - public int $hydratorCalls = 0; - - #[Override] - public function getHydrator(): ClassMethodsHydrator - { - $this->hydratorCalls++; + $hydratingRs = new HydratingResultSet(null, new ArrayObject()); + $hydratingRs->initialize(new ArrayIterator([['id' => 1]])); + static::assertInstanceOf(ArrayObject::class, $hydratingRs->current()); - return new ClassMethodsHydrator(); - } + $hydratingRs->setRowPrototype(new stdClass()); + $hydratingRs->setHydrator(new ObjectPropertyHydrator()); + $hydratingRs->initialize(new ArrayIterator([['id' => 2]])); - #[Override] - public function getRowPrototype(): object - { - return new class { - public ?int $id = null; + $row = $hydratingRs->current(); + static::assertInstanceOf(stdClass::class, $row); + static::assertSame(2, $row->id); + } - public function setId(int $id): void - { - $this->id = $id * 10; - } - }; - } - }; - $hydratingRs->initialize(new ArrayIterator([['id' => 1], ['id' => 2]])); + #[Test] + public function aSecondBufferedPassReturnsTheSameObject(): void + { + $hydratingRs = new HydratingResultSet(); + $hydratingRs->initialize(new ArrayIterator([ + ['id' => 1, 'name' => 'one'], + ['id' => 2, 'name' => 'two'], + ])); + $hydratingRs->buffer(); - $ids = []; - foreach ($hydratingRs as $row) { - $ids[] = $row->id; - } + $first = $hydratingRs->current(); + $hydratingRs->rewind(); - static::assertSame([10, 20], $ids); - static::assertSame(1, $hydratingRs->hydratorCalls, 'getHydrator() is asked once per data source, not per row'); + static::assertSame($first, $hydratingRs->current()); } #[Test] @@ -287,6 +264,44 @@ public function setRowPrototypeStoresPrototype(): void static::assertSame($prototype, $hydratingRs->getRowPrototype()); } + #[Test] + public function subclassGettersDecideHowRowsAreHydrated(): void + { + $hydratingRs = new class extends HydratingResultSet { + public int $hydratorCalls = 0; + + #[Override] + public function getHydrator(): ClassMethodsHydrator + { + $this->hydratorCalls++; + + return new ClassMethodsHydrator(); + } + + #[Override] + public function getRowPrototype(): object + { + return new class { + public ?int $id = null; + + public function setId(int $id): void + { + $this->id = $id * 10; + } + }; + } + }; + $hydratingRs->initialize(new ArrayIterator([['id' => 1], ['id' => 2]])); + + $ids = []; + foreach ($hydratingRs as $row) { + $ids[] = $row->id; + } + + static::assertSame([10, 20], $ids); + static::assertSame(1, $hydratingRs->hydratorCalls, 'getHydrator() is asked once per data source, not per row'); + } + /** * @throws Exception * @todo Implement testToArray(). @@ -336,20 +351,4 @@ protected function setUp(): void $this->arraySerializableHydratorClass = ArraySerializableHydrator::class; $this->classMethodsHydratorClass = ClassMethodsHydrator::class; } - - #[Test] - public function aNewHydratorOrPrototypeIsUsedForTheNextDataSource(): void - { - $hydratingRs = new HydratingResultSet(null, new ArrayObject()); - $hydratingRs->initialize(new ArrayIterator([['id' => 1]])); - static::assertInstanceOf(ArrayObject::class, $hydratingRs->current()); - - $hydratingRs->setRowPrototype(new stdClass()); - $hydratingRs->setHydrator(new \Laminas\Hydrator\ObjectPropertyHydrator()); - $hydratingRs->initialize(new ArrayIterator([['id' => 2]])); - - $row = $hydratingRs->current(); - static::assertInstanceOf(stdClass::class, $row); - static::assertSame(2, $row->id); - } } diff --git a/test/unit/ResultSet/ResultSetIntegrationTest.php b/test/unit/ResultSet/ResultSetIntegrationTest.php index 7298673a..e338c056 100644 --- a/test/unit/ResultSet/ResultSetIntegrationTest.php +++ b/test/unit/ResultSet/ResultSetIntegrationTest.php @@ -45,7 +45,6 @@ #[CoversMethod(ResultSet::class, 'toArray')] #[CoversMethod(ResultSet::class, 'resetResolvedConfiguration')] #[CoversMethod(AbstractResultSet::class, 'holdRow')] -#[CoversMethod(AbstractResultSet::class, 'holdsMappedRows')] #[CoversMethod(AbstractResultSet::class, 'resetResolvedConfiguration')] #[Group('unit')] final class ResultSetIntegrationTest extends TestCase @@ -88,6 +87,34 @@ public static function returnTypeProvider(): array return $cases; } + #[Test] + public function aBufferedPassAfterSetRowPrototypeUsesTheNewPrototype(): void + { + $resultSet = new ResultSet(); + $resultSet->initialize(new ArrayIterator([['id' => 1]])); + $resultSet->buffer(); + $resultSet->current(); + + $resultSet->setRowPrototype(new class([], ArrayObject::ARRAY_AS_PROPS) extends ArrayObject {}); + $resultSet->rewind(); + + static::assertNotSame(ArrayObject::class, $resultSet->current()::class); + } + + #[Test] + public function aBufferedResultSetBuildsFreshRowsOnEveryPass(): void + { + $resultSet = new ResultSet(); + $resultSet->initialize(new ArrayIterator([['id' => 1], ['id' => 2]])); + $resultSet->buffer(); + + $first = $resultSet->current(); + $resultSet->rewind(); + + static::assertNotSame($first, $resultSet->current()); + static::assertEquals($first, $resultSet->current()); + } + #[Test] public function aChangeToARowInOneBufferedPassDoesNotReachTheNext(): void { @@ -116,6 +143,24 @@ public function aRowPrototypeSetAfterABufferedPassShapesTheNext(): void static::assertInstanceOf($prototype::class, $this->resultSet->current()); } + #[Test] + public function aSubclassGetRowPrototypeDecidesTheRowClass(): void + { + $resultSet = new class extends ResultSet { + #[Override] + public function getRowPrototype(): ArrayObject + { + return new class([], ArrayObject::ARRAY_AS_PROPS) extends ArrayObject {}; + } + }; + $resultSet->initialize([['id' => 1]]); + + $row = $resultSet->current(); + + static::assertInstanceOf(ArrayObject::class, $row); + static::assertNotSame(ArrayObject::class, $row::class); + } + /** * @throws Exception * @throws \Exception @@ -479,50 +524,4 @@ protected function setUp(): void { $this->resultSet = new ResultSet(); } - - #[Test] - public function aSubclassGetRowPrototypeDecidesTheRowClass(): void - { - $resultSet = new class extends ResultSet { - #[Override] - public function getRowPrototype(): ArrayObject - { - return new class ([], ArrayObject::ARRAY_AS_PROPS) extends ArrayObject {}; - } - }; - $resultSet->initialize([['id' => 1]]); - - $row = $resultSet->current(); - - static::assertInstanceOf(ArrayObject::class, $row); - static::assertNotSame(ArrayObject::class, $row::class); - } - - #[Test] - public function aBufferedResultSetBuildsFreshRowsOnEveryPass(): void - { - $resultSet = new ResultSet(); - $resultSet->initialize(new ArrayIterator([['id' => 1], ['id' => 2]])); - $resultSet->buffer(); - - $first = $resultSet->current(); - $resultSet->rewind(); - - static::assertNotSame($first, $resultSet->current()); - static::assertEquals($first, $resultSet->current()); - } - - #[Test] - public function aBufferedPassAfterSetRowPrototypeUsesTheNewPrototype(): void - { - $resultSet = new ResultSet(); - $resultSet->initialize(new ArrayIterator([['id' => 1]])); - $resultSet->buffer(); - $resultSet->current(); - - $resultSet->setRowPrototype(new class ([], ArrayObject::ARRAY_AS_PROPS) extends ArrayObject {}); - $resultSet->rewind(); - - static::assertNotSame(ArrayObject::class, $resultSet->current()::class); - } } diff --git a/test/unit/ResultSet/RowPrototypeResultSetTest.php b/test/unit/ResultSet/RowPrototypeResultSetTest.php index 0873a315..208bb82f 100644 --- a/test/unit/ResultSet/RowPrototypeResultSetTest.php +++ b/test/unit/ResultSet/RowPrototypeResultSetTest.php @@ -6,6 +6,7 @@ use ArrayIterator; use ArrayObject; +use Override; use PhpDb\ResultSet\AbstractResultSet; use PhpDb\ResultSet\Exception\ValueError; use PhpDb\ResultSet\RowPrototypeInterface; @@ -28,6 +29,30 @@ #[Group('unit')] final class RowPrototypeResultSetTest extends TestCase { + #[Test] + public function aSubclassGetRowPrototypeDecidesTheRowClass(): void + { + $custom = $this->createRowPrototype(); + $resultSet = new class($this->createRowPrototype(), $custom) extends RowPrototypeResultSet { + public function __construct( + RowPrototypeInterface $prototype, + private RowPrototypeInterface $custom, + ) { + parent::__construct($prototype); + } + + #[Override] + public function getRowPrototype(): RowPrototypeInterface + { + return $this->custom; + } + }; + $resultSet->initialize([['id' => 1]]); + + static::assertSame($custom::class, $resultSet->current()::class); + static::assertNotSame($custom, $resultSet->current(), 'the prototype is cloned for each row'); + } + #[Test] public function currentPopulatesThePrototypeFromARowCarryingItsValuesAsElements(): void { @@ -117,6 +142,22 @@ public function iterationYieldsNullForANullRowAndCarriesOn(): void static::assertSame([['id' => 1], null, ['id' => 3]], $rows); } + #[Test] + public function setRowPrototypeReplacesThePrototypeForLaterRows(): void + { + $first = $this->createRowPrototype(); + $second = $this->createRowPrototype(); + $resultSet = new RowPrototypeResultSet($first); + $resultSet->initialize([['id' => 1], ['id' => 2]]); + $resultSet->current(); + + static::assertSame($resultSet, $resultSet->setRowPrototype($second)); + static::assertSame($second, $resultSet->getRowPrototype()); + + $resultSet->next(); + static::assertSame(['id' => 2], $resultSet->current()->toArray()); + } + #[Test] public function toArrayConvertsPrototypeRowsToArrays(): void { @@ -136,28 +177,6 @@ public function toArrayConvertsPrototypeRowsToArrays(): void ); } - #[Test] - public function aSubclassGetRowPrototypeDecidesTheRowClass(): void - { - $custom = $this->createRowPrototype(); - $resultSet = new class ($this->createRowPrototype(), $custom) extends RowPrototypeResultSet { - public function __construct(RowPrototypeInterface $prototype, private RowPrototypeInterface $custom) - { - parent::__construct($prototype); - } - - #[\Override] - public function getRowPrototype(): RowPrototypeInterface - { - return $this->custom; - } - }; - $resultSet->initialize([['id' => 1]]); - - static::assertSame($custom::class, $resultSet->current()::class); - static::assertNotSame($custom, $resultSet->current(), 'the prototype is cloned for each row'); - } - private function createRowPrototype(): RowPrototypeInterface { return new class implements RowPrototypeInterface { @@ -176,20 +195,4 @@ public function toArray(): array } }; } - - #[Test] - public function setRowPrototypeReplacesThePrototypeForLaterRows(): void - { - $first = $this->createRowPrototype(); - $second = $this->createRowPrototype(); - $resultSet = new RowPrototypeResultSet($first); - $resultSet->initialize([['id' => 1], ['id' => 2]]); - $resultSet->current(); - - static::assertSame($resultSet, $resultSet->setRowPrototype($second)); - static::assertSame($second, $resultSet->getRowPrototype()); - - $resultSet->next(); - static::assertSame(['id' => 2], $resultSet->current()->toArray()); - } } From 3698cda245a09a113ca712231af9b9ac9eadcda6 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 2 Oct 2026 18:57:32 +1000 Subject: [PATCH 15/17] Bring the result set docs in line with the code - The fetch mode guidance says FETCH_NUM, FETCH_BOTH and FETCH_KEY_PAIR fail in HydratingResultSet, whose hydrators reject integer keys. - The AbstractResultSet API block shows the current signatures, mapRow() and the HOLDS_MAPPED_ROWS and resetResolvedConfiguration() extension points, in place of current() and toArray(). - The ResultSetReturnType listing includes Prototype, and the ResultSet constructor shows its real defaults. - The toArray() sample catches ValueError rather than RuntimeException, and notes that catch (\Exception) misses it. - What a Buffer Holds says when each set asks its getters again. --- docs/book/result-set/advanced.md | 17 +++++++++----- docs/book/result-set/examples.md | 18 +++++++++++---- docs/book/result-set/intro.md | 39 ++++++++++++++++++++------------ 3 files changed, 49 insertions(+), 25 deletions(-) diff --git a/docs/book/result-set/advanced.md b/docs/book/result-set/advanced.md index 99255cf4..a9af3d03 100644 --- a/docs/book/result-set/advanced.md +++ b/docs/book/result-set/advanced.md @@ -15,8 +15,8 @@ use ArrayObject; class ResultSet extends AbstractResultSet { public function __construct( - ResultSetReturnType $returnType = ResultSetReturnType::ArrayObject, - ?ArrayObject $rowPrototype = null + ResultSetReturnType|string $returnType = ResultSetReturnType::ArrayObject, + ArrayObject $rowPrototype = new ArrayObject([], ArrayObject::ARRAY_AS_PROPS) ); public function setRowPrototype( @@ -39,6 +39,7 @@ enum ResultSetReturnType: string { case ArrayObject = 'arrayobject'; case Array = 'array'; + case Prototype = 'prototype'; } ``` @@ -56,10 +57,12 @@ $resultSet = new ResultSet(ResultSetReturnType::Array); - `ResultSetReturnType::ArrayObject` (default) - Returns rows as ArrayObject instances +- `ResultSetReturnType::Prototype` - The same as `ArrayObject`: rows fill a clone + of the row prototype. It is the newer name, matching `setRowPrototype()` - `ResultSetReturnType::Array` - Returns rows as plain PHP arrays **`$rowPrototype`** - Custom ArrayObject prototype for row objects -(only used with ArrayObject mode) +(only used with ArrayObject and Prototype modes) #### Return Type Modes @@ -226,9 +229,11 @@ foreach ($resultSet as $user) { } ``` -The row prototype and hydrator are read through `getRowPrototype()` and -`getHydrator()` once for each data source, so a subclass that overrides either getter -decides how its rows are built. +Rows are built from what `getRowPrototype()` and `getHydrator()` return, so a subclass +that overrides either getter decides how its rows are built. `ResultSet` and +`RowPrototypeResultSet` ask once for each data source. `HydratingResultSet` asks on its +first row and again after `setRowPrototype()` or `setHydrator()`, so a getter whose +answer changes between data sources needs one of those setters called to take effect. ### isBuffered() Method diff --git a/docs/book/result-set/examples.md b/docs/book/result-set/examples.md index cf4f1484..3ecb0044 100644 --- a/docs/book/result-set/examples.md +++ b/docs/book/result-set/examples.md @@ -205,16 +205,26 @@ try { } ``` -**toArray() on non-castable rows:** +### ValueError + +**A row the result set cannot read**, such as a `PDO::FETCH_OBJ` row in a +`ResultSet`, is refused when it is read, whether by iterating, `current()` or +`toArray()`: ```php +use PhpDb\ResultSet\Exception\ValueError; + try { - $resultSet->toArray(); -} catch (RuntimeException $e) { - printf("Error: Could not convert row to array\n"); + $rows = $resultSet->toArray(); +} catch (ValueError $e) { + printf("Error: %s\n", $e->getMessage()); } ``` +`ValueError` extends PHP's `\ValueError`, which is an `Error` rather than an +`Exception`, so `catch (\Exception $e)` does not catch it. Catch `ValueError`, +`\Throwable`, or `PhpDb\Exception\ExceptionInterface`, which it implements. + ## Troubleshooting ### Property Access Not Working diff --git a/docs/book/result-set/intro.md b/docs/book/result-set/intro.md index a4e0b263..a8664b08 100644 --- a/docs/book/result-set/intro.md +++ b/docs/book/result-set/intro.md @@ -69,16 +69,13 @@ functionality: namespace PhpDb\ResultSet; use Iterator; -use IteratorAggregate; -use PhpDb\Adapter\Driver\ResultInterface; -abstract class AbstractResultSet implements Iterator, ResultSetInterface +abstract class AbstractResultSet implements ResultSetInterface { - public function initialize( - array|Iterator|IteratorAggregate|ResultInterface $dataSource - ): ResultSetInterface; - public function getDataSource(): - array|Iterator|IteratorAggregate|ResultInterface; + protected const bool HOLDS_MAPPED_ROWS = false; + + public function initialize(iterable $dataSource): ResultSetInterface; + public function getDataSource(): ?Iterator; public function getFieldCount(): int; public function buffer(): ResultSetInterface; @@ -86,16 +83,25 @@ abstract class AbstractResultSet implements Iterator, ResultSetInterface public function next(): void; public function key(): int; - public function current(): mixed; public function valid(): bool; public function rewind(): void; - public function count(): int; + public function count(): ?int; - public function toArray(): array; + abstract protected function mapRow(mixed $row): mixed; + protected function resetResolvedConfiguration(): void; } ``` +`ResultSetInterface` extends `Iterator` and `Countable` and adds `toArray()`. Each +concrete result set declares `current()` with its own row type and implements it by +returning `$this->currentRow()`, which serves the row from the buffer or the data source +and passes it through `mapRow()` to give it that shape. A custom result set implements +`current()`, `mapRow()` and `toArray()`. It can redeclare `HOLDS_MAPPED_ROWS` as `true` +to hand out the same row objects on every buffered pass, and override +`resetResolvedConfiguration()` to forget anything it cached when `initialize()` is given +a new data source. + ## HydratingResultSet `PhpDb\ResultSet\HydratingResultSet` is a more flexible `ResultSet` object @@ -186,10 +192,13 @@ and the result set have to agree; where they do not, the row is refused with a | `HydratingResultSet` | row data | the hydrated row prototype | | `RowPrototypeResultSet` | row data, or a `RowPrototypeInterface` | a populated `RowPrototypeInterface` | -With PDO that means `FETCH_ASSOC`, `FETCH_NUM`, `FETCH_BOTH`, `FETCH_NAMED` and -`FETCH_KEY_PAIR` suit the row-data sets, while `FETCH_OBJ` and `FETCH_LAZY` need -`ObjectResultSet`. `FETCH_BOUND` yields only a success flag and binds its columns by -reference, so no result set accepts its rows; read the bound variables instead. +With PDO that means `FETCH_ASSOC` and `FETCH_NAMED` suit every row-data set. +`FETCH_NUM`, `FETCH_BOTH` and `FETCH_KEY_PAIR` give rows with integer keys, which +`ResultSet`, `ArrayResultSet` and `RowPrototypeResultSet` accept but a hydrator cannot +map to properties, so `HydratingResultSet` fails on them with a `TypeError` from +laminas-hydrator. `FETCH_OBJ` and `FETCH_LAZY` need `ObjectResultSet`. `FETCH_BOUND` +yields only a success flag and binds its columns by reference, so no result set accepts +its rows; read the bound variables instead. ## Data Source Types From 228cccf4af76073a764a1f3f32e963fa3ae0cf09 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 2 Oct 2026 19:39:42 +1000 Subject: [PATCH 16/17] Cut per-row work from buffered reads - Storing gets separate branches for raw and mapped rows, so a buffered read no longer calls holdRow() or reads HOLDS_MAPPED_ROWS per row. - valid() has its own Storing branch, and its last path drops a held-row lookup that could never succeed. - next() counts how far it has advanced the data source rather than comparing its key() with the position, so a source keyed other than 0, 1, 2... is advanced instead of repeating its first row. - ResultSet and RowPrototypeResultSet resolve the row prototype when a data source or prototype is set, not on each row. --- src/ResultSet/AbstractResultSet.php | 85 ++++++++++++------- src/ResultSet/ResultSet.php | 9 +- src/ResultSet/RowPrototypeResultSet.php | 9 +- test/unit/ResultSet/AbstractResultSetTest.php | 15 +++- .../unit/ResultSet/HydratingResultSetTest.php | 13 ++- .../ResultSet/ResultSetIntegrationTest.php | 1 - 6 files changed, 89 insertions(+), 43 deletions(-) diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index 62357202..3f28c482 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -65,6 +65,12 @@ abstract class AbstractResultSet implements ResultSetInterface */ private bool $storesRows = false; + /** + * Whether rows are held as mapRow() built them, for a result set that redeclares + * HOLDS_MAPPED_ROWS. Derived alongside $storesRows by setBufferState(). + */ + private bool $storesMappedRows = false; + /** * Raw rows held for later passes, keyed by position, and mapped afresh on each read * so that no pass sees what a caller did to the rows of another. @@ -88,6 +94,14 @@ abstract class AbstractResultSet implements ResultSetInterface protected int $position = 0; + /** + * How many rows the data source has been advanced past while Storing. + * + * next() compares this, rather than the data source's key(), with the position, so + * a source keyed other than 0, 1, 2... is still advanced one row at a time. + */ + private int $sourcePosition = 0; + /** * Resolve an IteratorAggregate chain down to the Iterator it wraps. * @@ -129,6 +143,7 @@ public function buffer(): ResultSetInterface } $this->setBufferState(RowBufferState::Storing); + $this->sourcePosition = 0; if ($this->dataSource instanceof ResultInterface) { $this->dataSource->rewind(); @@ -206,7 +221,8 @@ public function getFieldCount(): int #[Override] public function initialize(iterable $dataSource): ResultSetInterface { - $this->bufferedRows = []; + $this->bufferedRows = []; + $this->sourcePosition = 0; $this->resetResolvedConfiguration(); if ($dataSource instanceof ResultInterface) { @@ -265,8 +281,9 @@ public function next(): void if ($this->storesRows) { /** @var Iterator $dataSource */ $dataSource = $this->dataSource; - if ($this->position === $dataSource->key()) { + if ($this->position === $this->sourcePosition) { $dataSource->next(); + $this->sourcePosition++; } $this->position++; @@ -316,11 +333,15 @@ public function valid(): bool return $dataSource->valid(); } - if (null !== ($this->bufferedRows[$this->position] ?? null)) { - return true; + if ($this->storesRows) { + /** @var Iterator $dataSource */ + $dataSource = $this->dataSource; + + return null !== ($this->bufferedRows[$this->position] ?? null) || $dataSource->valid(); } - return ($this->dataSource ?? throw RuntimeException::forUninitialisedDataSource())->valid(); + // Pending, or Storing before initialize() supplies a data source: nothing is held + return $this->dataSource()->valid(); } /** @@ -363,12 +384,11 @@ protected function currentRow(): mixed return null === $row || false === $row ? null : $this->mapRow($row); } - if ($this->storesRows) { - /** @var mixed $held */ + if ($this->storesMappedRows) { + /** @var TRow|null $held */ $held = $this->bufferedRows[$this->position] ?? null; - if (null !== $held && static::HOLDS_MAPPED_ROWS) { - /** @var TRow $held */ + if (null !== $held) { return $held; } @@ -376,13 +396,29 @@ protected function currentRow(): mixed $dataSource = $this->dataSource; /** @var mixed $row */ - $row = $held ?? $dataSource->current(); + $row = $dataSource->current(); + + if (null === $row || false === $row) { + return null; + } + + return $this->bufferedRows[$this->position] = $this->mapRow($row); + } + + if ($this->storesRows) { + /** @var Iterator $dataSource */ + $dataSource = $this->dataSource; + + /** @var mixed $row */ + $row = $this->bufferedRows[$this->position] ?? $dataSource->current(); if (null === $row || false === $row) { return null; } - return $this->holdRow($row); + $this->bufferedRows[$this->position] = $row; + + return $this->mapRow($row); } // Only the first read of an unbuffered set lands here: Storing with a data source @@ -435,8 +471,8 @@ protected function getArrayData(mixed $row): array } /** - * Forget anything a subclass resolved from its getters, so that the next row asks - * them again. Called by initialize(); a subclass's setters call it too. + * Refresh anything a subclass resolves from its getters, so that each data source + * reads them once. Called by initialize() before the data source is taken. */ protected function resetResolvedConfiguration(): void {} @@ -451,22 +487,6 @@ protected function unsupportedRowError(mixed $row): ValueError return ValueError::forRowThatIsNotArrayData(get_debug_type($row), static::class); } - /** - * Keep a row for later passes and return it mapped. - * - * @return TRow - */ - private function holdRow(mixed $row): mixed - { - if (static::HOLDS_MAPPED_ROWS) { - return $this->bufferedRows[$this->position] = $this->mapRow($row); - } - - $this->bufferedRows[$this->position] = $row; - - return $this->mapRow($row); - } - private function initializeFromResult(ResultInterface $result): void { $this->fieldCount = $result->getFieldCount(); @@ -486,8 +506,9 @@ private function initializeFromResult(ResultInterface $result): void private function setBufferState(RowBufferState $state): void { - $this->bufferState = $state; - $this->readsDirectly = RowBufferState::Passthrough === $state || RowBufferState::Disabled === $state; - $this->storesRows = RowBufferState::Storing === $state && null !== $this->dataSource; + $this->bufferState = $state; + $this->readsDirectly = RowBufferState::Passthrough === $state || RowBufferState::Disabled === $state; + $this->storesRows = RowBufferState::Storing === $state && null !== $this->dataSource; + $this->storesMappedRows = $this->storesRows && static::HOLDS_MAPPED_ROWS; } } diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index 221bbd2d..ace7e0f6 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -105,7 +105,7 @@ public function setRowPrototype( ArrayObject $rowPrototype, ): ResultSetInterface&ArrayObjectResultSetInterface { $this->rowPrototype = $rowPrototype; - $this->resolvedRowPrototype = null; + $this->resolvedRowPrototype = $this->getRowPrototype(); return $this; } @@ -145,9 +145,10 @@ protected function mapRow(mixed $row): array|ArrayObject return $row; } - $this->resolvedRowPrototype ??= $this->getRowPrototype(); + /** @var ArrayObject $prototype Resolved by initialize() before any row is read. */ + $prototype = $this->resolvedRowPrototype; - $ao = clone $this->resolvedRowPrototype; + $ao = clone $prototype; $ao->exchangeArray($row); return $ao; @@ -156,6 +157,6 @@ protected function mapRow(mixed $row): array|ArrayObject #[Override] protected function resetResolvedConfiguration(): void { - $this->resolvedRowPrototype = null; + $this->resolvedRowPrototype = $this->getRowPrototype(); } } diff --git a/src/ResultSet/RowPrototypeResultSet.php b/src/ResultSet/RowPrototypeResultSet.php index 4f7042a3..3f07494b 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -48,7 +48,7 @@ public function setRowPrototype( RowPrototypeInterface $rowPrototype, ): ResultSetInterface&RowPrototypeResultSetInterface { $this->rowPrototype = $rowPrototype; - $this->resolvedRowPrototype = null; + $this->resolvedRowPrototype = $this->getRowPrototype(); return $this; } @@ -78,15 +78,16 @@ protected function mapRow(mixed $row): RowPrototypeInterface return $row; } - $this->resolvedRowPrototype ??= $this->getRowPrototype(); + /** @var RowPrototypeInterface $prototype Resolved by initialize() before any row is read. */ + $prototype = $this->resolvedRowPrototype; - return (clone $this->resolvedRowPrototype)->populate(is_array($row) ? $row : $this->getArrayData($row)); + return (clone $prototype)->populate(is_array($row) ? $row : $this->getArrayData($row)); } #[Override] protected function resetResolvedConfiguration(): void { - $this->resolvedRowPrototype = null; + $this->resolvedRowPrototype = $this->getRowPrototype(); } /** {@inheritDoc} */ diff --git a/test/unit/ResultSet/AbstractResultSetTest.php b/test/unit/ResultSet/AbstractResultSetTest.php index 1aa4a628..cc5af842 100644 --- a/test/unit/ResultSet/AbstractResultSetTest.php +++ b/test/unit/ResultSet/AbstractResultSetTest.php @@ -8,6 +8,7 @@ use ArrayObject; use Exception; use IteratorAggregate; +use LimitIterator; use NoRewindIterator; use Override; use PDOStatement; @@ -41,7 +42,6 @@ #[CoversMethod(AbstractResultSet::class, 'rewind')] #[CoversMethod(AbstractResultSet::class, 'count')] #[CoversMethod(AbstractResultSet::class, 'resetResolvedConfiguration')] -#[CoversMethod(AbstractResultSet::class, 'holdRow')] final class AbstractResultSetTest extends TestCase { protected AbstractResultSet $resultSet; @@ -125,6 +125,19 @@ public function bufferCalledTwiceKeepsTheRowsAlreadyHeld(): void static::assertSame(['id' => 1], $resultSet->current()); } + #[Test] + public function bufferedPassAdvancesADataSourceKeyedOtherThanByPosition(): void + { + $resultSet = $this->createResultSetMock(); + $resultSet->buffer(); + $resultSet->initialize(new ArrayIterator(['a' => ['id' => 1], 'b' => ['id' => 2], 'c' => ['id' => 3]])); + + // A data source that is never advanced repeats its first row; a fourth read shows it + $rows = iterator_to_array(new LimitIterator($resultSet, 0, 4), preserve_keys: false); + + static::assertSame([['id' => 1], ['id' => 2], ['id' => 3]], $rows); + } + /** * Test multiple iterations with buffer * diff --git a/test/unit/ResultSet/HydratingResultSetTest.php b/test/unit/ResultSet/HydratingResultSetTest.php index c358f441..b2a9c92f 100644 --- a/test/unit/ResultSet/HydratingResultSetTest.php +++ b/test/unit/ResultSet/HydratingResultSetTest.php @@ -34,7 +34,6 @@ #[CoversMethod(HydratingResultSet::class, 'setRowPrototype')] #[CoversMethod(HydratingResultSet::class, 'getRowPrototype')] #[CoversMethod(AbstractResultSet::class, 'currentRow')] -#[CoversMethod(AbstractResultSet::class, 'holdRow')] #[Group('unit')] final class HydratingResultSetTest extends TestCase { @@ -42,6 +41,18 @@ final class HydratingResultSetTest extends TestCase private string $classMethodsHydratorClass; + #[Test] + public function aBufferedReadPastTheLastRowReturnsNull(): void + { + $hydratingRs = new HydratingResultSet(); + $hydratingRs->initialize(new ArrayIterator([['id' => 1]])); + $hydratingRs->buffer(); + $hydratingRs->current(); + $hydratingRs->next(); + + static::assertNull($hydratingRs->current()); + } + #[Test] public function aChangeToABufferedEntityCarriesIntoTheNextPass(): void { diff --git a/test/unit/ResultSet/ResultSetIntegrationTest.php b/test/unit/ResultSet/ResultSetIntegrationTest.php index e338c056..a7f8bff5 100644 --- a/test/unit/ResultSet/ResultSetIntegrationTest.php +++ b/test/unit/ResultSet/ResultSetIntegrationTest.php @@ -44,7 +44,6 @@ #[CoversMethod(ResultSet::class, 'setRowPrototype')] #[CoversMethod(ResultSet::class, 'toArray')] #[CoversMethod(ResultSet::class, 'resetResolvedConfiguration')] -#[CoversMethod(AbstractResultSet::class, 'holdRow')] #[CoversMethod(AbstractResultSet::class, 'resetResolvedConfiguration')] #[Group('unit')] final class ResultSetIntegrationTest extends TestCase From 7e78d27c1ab88911a631a947fc9dc8825aa7d7fe Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Fri, 2 Oct 2026 20:18:09 +1000 Subject: [PATCH 17/17] Throw UnexpectedValueException for rows a result set cannot read ValueError extended PHP's \ValueError, an Error, so catch (\Exception) missed a refused row. UnexpectedValueException extends the SPL class of the same name, which catch (\RuntimeException) and catch (\Exception) both reach, and joins ExceptionHierarchyTest. Each forRowThatIs...() constructor now has its own template constant, as the convention requires; the messages are unchanged. PhpDb\Exception\ValueError had no other user and is removed. --- docs/book/result-set/examples.md | 11 ++- docs/book/result-set/intro.md | 2 +- src/Exception/ValueError.php | 9 --- src/ResultSet/AbstractResultSet.php | 8 +-- src/ResultSet/ArrayResultSet.php | 4 +- .../Exception/UnexpectedValueException.php | 40 +++++++++++ src/ResultSet/Exception/ValueError.php | 45 ------------ src/ResultSet/HydratingResultSet.php | 6 +- src/ResultSet/ObjectResultSet.php | 12 ++-- src/ResultSet/ResultSet.php | 4 +- src/ResultSet/RowPrototypeResultSet.php | 11 +-- .../unit/Exception/ExceptionHierarchyTest.php | 1 + test/unit/ResultSet/ArrayResultSetTest.php | 6 +- ...t.php => UnexpectedValueExceptionTest.php} | 71 +++++++++---------- .../unit/ResultSet/HydratingResultSetTest.php | 8 +-- test/unit/ResultSet/ObjectResultSetTest.php | 8 +-- .../unit/ResultSet/ResultSetFetchModeTest.php | 6 +- .../ResultSet/ResultSetIntegrationTest.php | 4 +- .../ResultSet/RowPrototypeResultSetTest.php | 6 +- 19 files changed, 125 insertions(+), 137 deletions(-) delete mode 100644 src/Exception/ValueError.php create mode 100644 src/ResultSet/Exception/UnexpectedValueException.php delete mode 100644 src/ResultSet/Exception/ValueError.php rename test/unit/ResultSet/Exception/{ValueErrorTest.php => UnexpectedValueExceptionTest.php} (52%) diff --git a/docs/book/result-set/examples.md b/docs/book/result-set/examples.md index 3ecb0044..0b1c2158 100644 --- a/docs/book/result-set/examples.md +++ b/docs/book/result-set/examples.md @@ -205,25 +205,24 @@ try { } ``` -### ValueError +### UnexpectedValueException **A row the result set cannot read**, such as a `PDO::FETCH_OBJ` row in a `ResultSet`, is refused when it is read, whether by iterating, `current()` or `toArray()`: ```php -use PhpDb\ResultSet\Exception\ValueError; +use PhpDb\ResultSet\Exception\UnexpectedValueException; try { $rows = $resultSet->toArray(); -} catch (ValueError $e) { +} catch (UnexpectedValueException $e) { printf("Error: %s\n", $e->getMessage()); } ``` -`ValueError` extends PHP's `\ValueError`, which is an `Error` rather than an -`Exception`, so `catch (\Exception $e)` does not catch it. Catch `ValueError`, -`\Throwable`, or `PhpDb\Exception\ExceptionInterface`, which it implements. +It extends SPL's `\UnexpectedValueException`, so `catch (\RuntimeException $e)` and +`catch (\Exception $e)` catch it too, as does `PhpDb\Exception\ExceptionInterface`. ## Troubleshooting diff --git a/docs/book/result-set/intro.md b/docs/book/result-set/intro.md index a8664b08..39d014db 100644 --- a/docs/book/result-set/intro.md +++ b/docs/book/result-set/intro.md @@ -182,7 +182,7 @@ instead of calling `toArray()` on it. A result set fills its rows from row data, meaning an `array` or an `ArrayObject`. It will not transform a row the driver handed it into some other shape, so the fetch mode and the result set have to agree; where they do not, the row is refused with a -`PhpDb\ResultSet\Exception\ValueError` naming both types. +`PhpDb\ResultSet\Exception\UnexpectedValueException` naming both types. | Result set | Rows arrive as | Rows leave as | |---|---|---| diff --git a/src/Exception/ValueError.php b/src/Exception/ValueError.php deleted file mode 100644 index 699dbce5..00000000 --- a/src/Exception/ValueError.php +++ /dev/null @@ -1,9 +0,0 @@ - * - * @throws ValueError If the row is neither an array nor an ArrayObject. + * @throws UnexpectedValueException If the row is neither an array nor an ArrayObject. */ protected function getArrayData(mixed $row): array { @@ -482,9 +482,9 @@ protected function resetResolvedConfiguration(): void {} * Overridden by a result set that accepts more than row data, so that the message * names everything it would have taken. */ - protected function unsupportedRowError(mixed $row): ValueError + protected function unsupportedRowError(mixed $row): UnexpectedValueException { - return ValueError::forRowThatIsNotArrayData(get_debug_type($row), static::class); + return UnexpectedValueException::forRowThatIsNotArrayData(get_debug_type($row), static::class); } private function initializeFromResult(ResultInterface $result): void diff --git a/src/ResultSet/ArrayResultSet.php b/src/ResultSet/ArrayResultSet.php index d87af4d0..f9065dc1 100644 --- a/src/ResultSet/ArrayResultSet.php +++ b/src/ResultSet/ArrayResultSet.php @@ -19,7 +19,7 @@ class ArrayResultSet extends AbstractResultSet * @return array|null * * @throws Exception\RuntimeException - * @throws Exception\ValueError If a row is not row data. + * @throws Exception\UnexpectedValueException If a row is not row data. */ #[Override] public function current(): ?array @@ -42,7 +42,7 @@ public function toArray(): array /** * @return array * - * @throws Exception\ValueError If the row is not row data. + * @throws Exception\UnexpectedValueException If the row is not row data. */ #[Override] protected function mapRow(mixed $row): array diff --git a/src/ResultSet/Exception/UnexpectedValueException.php b/src/ResultSet/Exception/UnexpectedValueException.php new file mode 100644 index 00000000..9ad37542 --- /dev/null +++ b/src/ResultSet/Exception/UnexpectedValueException.php @@ -0,0 +1,40 @@ + * - * @throws Exception\ValueError If the row exposes nothing to read. + * @throws Exception\UnexpectedValueException If the row exposes nothing to read. */ private function rowToArray(object $row): array { @@ -84,7 +84,7 @@ private function rowToArray(object $row): array $data = get_object_vars($row); if ([] === $data) { - throw Exception\ValueError::forUnreadableRow(get_debug_type($row), static::class); + throw Exception\UnexpectedValueException::forUnreadableRow(get_debug_type($row), static::class); } return $data; diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index ace7e0f6..78ee864c 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -57,7 +57,7 @@ public function __construct( * @return array|ArrayObject|null * * @throws Exception\RuntimeException - * @throws Exception\ValueError If a row is not row data. + * @throws Exception\UnexpectedValueException If a row is not row data. */ #[Override] public function current(): array|ArrayObject|null @@ -128,7 +128,7 @@ public function toArray(): array * * @return array|ArrayObject * - * @throws Exception\ValueError If the row is neither an ArrayObject nor row data. + * @throws Exception\UnexpectedValueException If the row is neither an ArrayObject nor row data. */ #[Override] protected function mapRow(mixed $row): array|ArrayObject diff --git a/src/ResultSet/RowPrototypeResultSet.php b/src/ResultSet/RowPrototypeResultSet.php index 3f07494b..a08401bb 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -27,7 +27,7 @@ public function __construct( * Iterator: get current item * * @throws Exception\RuntimeException - * @throws Exception\ValueError If a row is not row data. + * @throws Exception\UnexpectedValueException If a row is not row data. */ #[Override] public function current(): ?RowPrototypeInterface @@ -69,7 +69,7 @@ public function toArray(): array * A row that already satisfies the prototype interface is the caller's own object * and is passed through untouched; row data populates a clone of the prototype. * - * @throws Exception\ValueError If the row is neither a RowPrototypeInterface nor row data. + * @throws Exception\UnexpectedValueException If the row is neither a RowPrototypeInterface nor row data. */ #[Override] protected function mapRow(mixed $row): RowPrototypeInterface @@ -92,8 +92,11 @@ protected function resetResolvedConfiguration(): void /** {@inheritDoc} */ #[Override] - protected function unsupportedRowError(mixed $row): Exception\ValueError + protected function unsupportedRowError(mixed $row): Exception\UnexpectedValueException { - return Exception\ValueError::forRowThatIsNotArrayDataOrPrototype(get_debug_type($row), static::class); + return Exception\UnexpectedValueException::forRowThatIsNotArrayDataOrPrototype( + get_debug_type($row), + static::class, + ); } } diff --git a/test/unit/Exception/ExceptionHierarchyTest.php b/test/unit/Exception/ExceptionHierarchyTest.php index 16c28db4..50f62b5c 100644 --- a/test/unit/Exception/ExceptionHierarchyTest.php +++ b/test/unit/Exception/ExceptionHierarchyTest.php @@ -73,6 +73,7 @@ final class ExceptionHierarchyTest extends TestCase MetadataException\RuntimeException::class => RuntimeException::class, ResultSetException\InvalidArgumentException::class => InvalidArgumentException::class, ResultSetException\RuntimeException::class => RuntimeException::class, + ResultSetException\UnexpectedValueException::class => UnexpectedValueException::class, RowGatewayException\InvalidArgumentException::class => InvalidArgumentException::class, RowGatewayException\RuntimeException::class => RuntimeException::class, SqlException\InvalidArgumentException::class => InvalidArgumentException::class, diff --git a/test/unit/ResultSet/ArrayResultSetTest.php b/test/unit/ResultSet/ArrayResultSetTest.php index 544c0ec4..1adc6901 100644 --- a/test/unit/ResultSet/ArrayResultSetTest.php +++ b/test/unit/ResultSet/ArrayResultSetTest.php @@ -8,7 +8,7 @@ use ArrayObject; use PhpDb\ResultSet\AbstractResultSet; use PhpDb\ResultSet\ArrayResultSet; -use PhpDb\ResultSet\Exception\ValueError; +use PhpDb\ResultSet\Exception\UnexpectedValueException; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; @@ -42,7 +42,7 @@ public function currentRejectsAnObjectRowRatherThanReducingIt(): void $resultSet = new ArrayResultSet(); $resultSet->initialize(new ArrayIterator([$row])); - self::expectException(ValueError::class); + self::expectException(UnexpectedValueException::class); self::expectExceptionMessage('A row of type "stdClass"'); $resultSet->current(); @@ -54,7 +54,7 @@ public function currentRejectsARowThatExposesNothing(): void $resultSet = new ArrayResultSet(); $resultSet->initialize(new ArrayIterator([new stdClass()])); - self::expectException(ValueError::class); + self::expectException(UnexpectedValueException::class); self::expectExceptionMessage('will not transform a row it did not create'); $resultSet->current(); diff --git a/test/unit/ResultSet/Exception/ValueErrorTest.php b/test/unit/ResultSet/Exception/UnexpectedValueExceptionTest.php similarity index 52% rename from test/unit/ResultSet/Exception/ValueErrorTest.php rename to test/unit/ResultSet/Exception/UnexpectedValueExceptionTest.php index b1280b11..59e040a0 100644 --- a/test/unit/ResultSet/Exception/ValueErrorTest.php +++ b/test/unit/ResultSet/Exception/UnexpectedValueExceptionTest.php @@ -4,8 +4,9 @@ namespace PhpDbTest\ResultSet\Exception; +use Exception; use PhpDb\Exception\ExceptionInterface; -use PhpDb\ResultSet\Exception\ValueError; +use PhpDb\ResultSet\Exception\UnexpectedValueException; use PhpDb\ResultSet\ObjectResultSet; use PhpDb\ResultSet\ResultSet; use PhpDb\ResultSet\RowPrototypeResultSet; @@ -13,95 +14,93 @@ use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -use ValueError as GlobalValueError; #[Group('unit')] -#[CoversMethod(ValueError::class, 'forUnreadableRow')] -#[CoversMethod(ValueError::class, 'forRowThatIsNotAnObject')] -#[CoversMethod(ValueError::class, 'forRowThatIsNotArrayData')] -#[CoversMethod(ValueError::class, 'forRowThatIsNotArrayDataOrPrototype')] -#[CoversMethod(ValueError::class, 'forUnsupportedRow')] -final class ValueErrorTest extends TestCase +#[CoversMethod(UnexpectedValueException::class, 'forUnreadableRow')] +#[CoversMethod(UnexpectedValueException::class, 'forRowThatIsNotAnObject')] +#[CoversMethod(UnexpectedValueException::class, 'forRowThatIsNotArrayData')] +#[CoversMethod(UnexpectedValueException::class, 'forRowThatIsNotArrayDataOrPrototype')] +final class UnexpectedValueExceptionTest extends TestCase { #[Test] public function forRowThatIsNotAnObjectNamesTheAcceptedTypes(): void { self::assertStringContainsString( 'which accepts an object', - ValueError::forRowThatIsNotAnObject('array', ObjectResultSet::class)->getMessage(), + UnexpectedValueException::forRowThatIsNotAnObject('array', ObjectResultSet::class)->getMessage(), ); } #[Test] - public function forRowThatIsNotArrayDataNamesTheAcceptedTypes(): void + public function forRowThatIsNotArrayDataIsCatchableAsAnException(): void { - self::assertStringContainsString( - 'which accepts an array or ArrayObject', - ValueError::forRowThatIsNotArrayData('PDORow', ResultSet::class)->getMessage(), + self::assertInstanceOf( + Exception::class, + UnexpectedValueException::forRowThatIsNotArrayData('bool', ResultSet::class), ); } #[Test] - public function forRowThatIsNotArrayDataOrPrototypeNamesTheAcceptedTypes(): void + public function forRowThatIsNotArrayDataNamesTheAcceptedTypes(): void { self::assertStringContainsString( - 'which accepts an array, ArrayObject or RowPrototypeInterface', - ValueError::forRowThatIsNotArrayDataOrPrototype('stdClass', RowPrototypeResultSet::class) - ->getMessage(), + 'which accepts an array or ArrayObject', + UnexpectedValueException::forRowThatIsNotArrayData('PDORow', ResultSet::class)->getMessage(), ); } #[Test] - public function forUnreadableRowNamesTheOffendingType(): void + public function forRowThatIsNotArrayDataNamesTheOffendingType(): void { self::assertStringContainsString( 'A row of type "PDORow"', - ValueError::forUnreadableRow('PDORow', ResultSet::class)->getMessage(), + UnexpectedValueException::forRowThatIsNotArrayData('PDORow', ResultSet::class)->getMessage(), ); } #[Test] - public function forUnreadableRowReturnsTheComponentExceptionType(): void + public function forRowThatIsNotArrayDataNamesTheResultSetThatRefusedIt(): void { - self::assertInstanceOf( - ExceptionInterface::class, - ValueError::forUnreadableRow('PDORow', ResultSet::class), + self::assertStringContainsString( + ResultSet::class, + UnexpectedValueException::forRowThatIsNotArrayData('PDORow', ResultSet::class)->getMessage(), ); } #[Test] - public function forUnsupportedRowNamesTheOffendingType(): void + public function forRowThatIsNotArrayDataOrPrototypeNamesTheAcceptedTypes(): void { self::assertStringContainsString( - 'A row of type "PDORow"', - ValueError::forRowThatIsNotArrayData('PDORow', ResultSet::class)->getMessage(), + 'which accepts an array, ArrayObject or RowPrototypeInterface', + UnexpectedValueException::forRowThatIsNotArrayDataOrPrototype('stdClass', RowPrototypeResultSet::class) + ->getMessage(), ); } #[Test] - public function forUnsupportedRowNamesTheResultSetThatRefusedIt(): void + public function forRowThatIsNotArrayDataReturnsTheComponentExceptionType(): void { - self::assertStringContainsString( - ResultSet::class, - ValueError::forRowThatIsNotArrayData('PDORow', ResultSet::class)->getMessage(), + self::assertInstanceOf( + ExceptionInterface::class, + UnexpectedValueException::forRowThatIsNotArrayData('bool', ResultSet::class), ); } #[Test] - public function forUnsupportedRowRemainsCatchableAsAValueError(): void + public function forUnreadableRowNamesTheOffendingType(): void { - self::assertInstanceOf( - GlobalValueError::class, - ValueError::forRowThatIsNotArrayData('bool', ResultSet::class), + self::assertStringContainsString( + 'A row of type "PDORow"', + UnexpectedValueException::forUnreadableRow('PDORow', ResultSet::class)->getMessage(), ); } #[Test] - public function forUnsupportedRowReturnsTheComponentExceptionType(): void + public function forUnreadableRowReturnsTheComponentExceptionType(): void { self::assertInstanceOf( ExceptionInterface::class, - ValueError::forRowThatIsNotArrayData('bool', ResultSet::class), + UnexpectedValueException::forUnreadableRow('PDORow', ResultSet::class), ); } } diff --git a/test/unit/ResultSet/HydratingResultSetTest.php b/test/unit/ResultSet/HydratingResultSetTest.php index b2a9c92f..22249ef1 100644 --- a/test/unit/ResultSet/HydratingResultSetTest.php +++ b/test/unit/ResultSet/HydratingResultSetTest.php @@ -13,7 +13,7 @@ use Override; use PhpDb\ResultSet\AbstractResultSet; use PhpDb\ResultSet\Exception\RuntimeException; -use PhpDb\ResultSet\Exception\ValueError; +use PhpDb\ResultSet\Exception\UnexpectedValueException; use PhpDb\ResultSet\HydratingResultSet; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; @@ -168,7 +168,7 @@ public function currentRejectsAnObjectRowRatherThanHydratingIt(): void $resultSet = new HydratingResultSet(new ArraySerializableHydrator(), new ArrayObject()); $resultSet->initialize(new ArrayIterator([$row])); - self::expectException(ValueError::class); + self::expectException(UnexpectedValueException::class); self::expectExceptionMessage('A row of type "stdClass"'); $resultSet->current(); @@ -180,7 +180,7 @@ public function currentRejectsARowThatExposesNothing(): void $resultSet = new HydratingResultSet(new ArraySerializableHydrator(), new ArrayObject()); $resultSet->initialize(new ArrayIterator([new stdClass()])); - self::expectException(ValueError::class); + self::expectException(UnexpectedValueException::class); self::expectExceptionMessage('will not transform a row it did not create'); $resultSet->current(); @@ -335,7 +335,7 @@ public function toArrayReportsARowTheHydratorCannotExtract(): void $hydratingRs = new HydratingResultSet(); $hydratingRs->initialize(new ArrayIterator([1, 2])); - self::expectException(ValueError::class); + self::expectException(UnexpectedValueException::class); self::expectExceptionMessage('A row of type "int"'); $hydratingRs->toArray(); diff --git a/test/unit/ResultSet/ObjectResultSetTest.php b/test/unit/ResultSet/ObjectResultSetTest.php index 87886761..d0f50071 100644 --- a/test/unit/ResultSet/ObjectResultSetTest.php +++ b/test/unit/ResultSet/ObjectResultSetTest.php @@ -6,7 +6,7 @@ use ArrayIterator; use ArrayObject; -use PhpDb\ResultSet\Exception\ValueError; +use PhpDb\ResultSet\Exception\UnexpectedValueException; use PhpDb\ResultSet\ObjectResultSet; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\DataProvider; @@ -44,7 +44,7 @@ public function currentRejectsARowThatIsNotAnObject(mixed $row, string $rowType) $resultSet = new ObjectResultSet(); $resultSet->initialize(new ArrayIterator([$row])); - self::expectException(ValueError::class); + self::expectException(UnexpectedValueException::class); self::expectExceptionMessage(sprintf('A row of type "%s"', $rowType)); $resultSet->current(); @@ -93,7 +93,7 @@ public function currentSaysWhichTypesItAccepts(): void $resultSet = new ObjectResultSet(); $resultSet->initialize(new ArrayIterator([['id' => 1]])); - self::expectException(ValueError::class); + self::expectException(UnexpectedValueException::class); self::expectExceptionMessage('which accepts an object'); $resultSet->current(); @@ -128,7 +128,7 @@ public function toArrayRejectsARowThatExposesNothing(): void $resultSet = new ObjectResultSet(); $resultSet->initialize(new ArrayIterator([new stdClass()])); - self::expectException(ValueError::class); + self::expectException(UnexpectedValueException::class); self::expectExceptionMessage('exposes no values'); $resultSet->toArray(); diff --git a/test/unit/ResultSet/ResultSetFetchModeTest.php b/test/unit/ResultSet/ResultSetFetchModeTest.php index 061a2936..f239aacc 100644 --- a/test/unit/ResultSet/ResultSetFetchModeTest.php +++ b/test/unit/ResultSet/ResultSetFetchModeTest.php @@ -9,7 +9,7 @@ use PDORow; use PhpDb\Adapter\Driver\Pdo\Result; use PhpDb\ResultSet\AbstractResultSet; -use PhpDb\ResultSet\Exception\ValueError; +use PhpDb\ResultSet\Exception\UnexpectedValueException; use PhpDb\ResultSet\ObjectResultSet; use PhpDb\ResultSet\ResultSet; use PhpDb\ResultSet\ResultSetInterface; @@ -121,7 +121,7 @@ public function boundFetchModeStillPopulatesTheBoundColumns(): void $resultSet = new ResultSet(); $resultSet->initialize($result); - self::expectException(ValueError::class); + self::expectException(UnexpectedValueException::class); try { iterator_to_array($resultSet, preserve_keys: false); @@ -159,7 +159,7 @@ public function fetchModesYieldingSomethingOtherThanRowDataAreRefused( int $fetchMode, string $rowType, ): void { - self::expectException(ValueError::class); + self::expectException(UnexpectedValueException::class); self::expectExceptionMessage(sprintf('A row of type "%s"', $rowType)); $this->rowsFor($fetchMode); diff --git a/test/unit/ResultSet/ResultSetIntegrationTest.php b/test/unit/ResultSet/ResultSetIntegrationTest.php index a7f8bff5..814a3a85 100644 --- a/test/unit/ResultSet/ResultSetIntegrationTest.php +++ b/test/unit/ResultSet/ResultSetIntegrationTest.php @@ -11,7 +11,7 @@ use PhpDb\ResultSet\AbstractResultSet; use PhpDb\ResultSet\Exception\InvalidArgumentException; use PhpDb\ResultSet\Exception\RuntimeException; -use PhpDb\ResultSet\Exception\ValueError; +use PhpDb\ResultSet\Exception\UnexpectedValueException; use PhpDb\ResultSet\ResultSet; use PhpDb\ResultSet\ResultSetReturnType; use PHPUnit\Framework\Attributes\CoversMethod; @@ -267,7 +267,7 @@ public function currentRejectsARowThatIsNotArrayData(): void $this->resultSet->initialize($mockResult); $this->resultSet->buffer(); - self::expectException(ValueError::class); + self::expectException(UnexpectedValueException::class); self::expectExceptionMessage('A row of type "string"'); $this->resultSet->current(); diff --git a/test/unit/ResultSet/RowPrototypeResultSetTest.php b/test/unit/ResultSet/RowPrototypeResultSetTest.php index 208bb82f..06b9cee1 100644 --- a/test/unit/ResultSet/RowPrototypeResultSetTest.php +++ b/test/unit/ResultSet/RowPrototypeResultSetTest.php @@ -8,7 +8,7 @@ use ArrayObject; use Override; use PhpDb\ResultSet\AbstractResultSet; -use PhpDb\ResultSet\Exception\ValueError; +use PhpDb\ResultSet\Exception\UnexpectedValueException; use PhpDb\ResultSet\RowPrototypeInterface; use PhpDb\ResultSet\RowPrototypeResultSet; use PHPUnit\Framework\Attributes\CoversMethod; @@ -74,7 +74,7 @@ public function currentRejectsAnObjectRowThatIsNotAPrototype(): void $row->name = 'one'; $resultSet->initialize(new ArrayIterator([$row])); - self::expectException(ValueError::class); + self::expectException(UnexpectedValueException::class); self::expectExceptionMessage('A row of type "stdClass"'); $resultSet->current(); @@ -86,7 +86,7 @@ public function currentRejectsARowThatExposesNothing(): void $resultSet = new RowPrototypeResultSet($this->createRowPrototype()); $resultSet->initialize(new ArrayIterator([new stdClass()])); - self::expectException(ValueError::class); + self::expectException(UnexpectedValueException::class); self::expectExceptionMessage('an array, ArrayObject or RowPrototypeInterface'); $resultSet->current();