Skip to content

Commit

Permalink
Extract Result from the Statement interface
Browse files Browse the repository at this point in the history
  • Loading branch information
morozov committed Jun 9, 2020
1 parent 3d8c575 commit 0e74935
Show file tree
Hide file tree
Showing 55 changed files with 1,617 additions and 1,875 deletions.
11 changes: 11 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
# Upgrade to 3.0

## BC BREAK changes in fetching statement results

1. The `Statement` interface no longer extends `ResultStatement`.
2. The `ResultStatement` interface has been renamed to `Result`.
3. Instead of returning `bool`, `Statement::execute()` now returns a `Result` that should be used for fetching the result data and metadata.
4. The functionality previously available via `Statement::closeCursor()` is now available via `Result::free()`. The behavior of fetching data from a freed result is no longer portable. In this case, some drivers will return `false` while others may throw an exception.

Additional related changes:

1. The `ArrayStatement` and `ResultCacheStatement` classes from the `Cache` package have been renamed to `ArrayResult` and `CachingResult` respectively and marked `@internal`.

## BC BREAK `Statement::rowCount()` is moved.

`Statement::rowCount()` has been moved to the `ResultStatement` interface where it belongs by definition.
Expand Down
2 changes: 1 addition & 1 deletion phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@

<!-- some statement classes close cursor using an empty while-loop -->
<rule ref="Generic.CodeAnalysis.EmptyStatement.DetectedWhile">
<exclude-pattern>src/Driver/SQLSrv/SQLSrvStatement.php</exclude-pattern>
<exclude-pattern>src/Driver/SQLSrv/Result.php</exclude-pattern>
</rule>

<!-- see https://github.com/doctrine/dbal/issues/3377 -->
Expand Down
4 changes: 3 additions & 1 deletion phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ parameters:
- '~unknown class OCI-(Lob|Collection)~'

# https://github.com/JetBrains/phpstorm-stubs/pull/766
- '~^Method Doctrine\\DBAL\\Driver\\Mysqli\\MysqliStatement::_fetch\(\) never returns null so it can be removed from the return typehint\.$~'
-
message: '~^Strict comparison using === between true and null will always evaluate to false\.$~'
path: %currentWorkingDirectory%/src/Driver/Mysqli/Result.php

# The ReflectionException in the case when the class does not exist is acceptable and does not need to be handled
- '~^Parameter #1 \$argument of class ReflectionClass constructor expects class-string<T of object>\|T of object, string given\.$~'
Expand Down
48 changes: 16 additions & 32 deletions src/Cache/ArrayStatement.php → src/Cache/ArrayResult.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,15 @@

use Doctrine\DBAL\Driver\FetchUtils;
use Doctrine\DBAL\Driver\Result;
use Doctrine\DBAL\Driver\ResultStatement;

use function array_values;
use function count;
use function reset;

/**
* @deprecated
* @internal The class is internal to the caching layer implementation.
*/
class ArrayStatement implements ResultStatement, Result
final class ArrayResult implements Result
{
/** @var mixed[] */
private $data;
Expand All @@ -37,35 +36,6 @@ public function __construct(array $data)
$this->columnCount = count($data[0]);
}

/**
* {@inheritdoc}
*
* @deprecated Use free() instead.
*/
public function closeCursor()
{
$this->free();

return true;
}

/**
* {@inheritdoc}
*/
public function columnCount()
{
return $this->columnCount;
}

public function rowCount(): int
{
if ($this->data === null) {
return 0;
}

return count($this->data);
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -126,6 +96,20 @@ public function fetchFirstColumn(): array
return FetchUtils::fetchFirstColumn($this);
}

public function rowCount(): int
{
if ($this->data === null) {
return 0;
}

return count($this->data);
}

public function columnCount(): int
{
return $this->columnCount;
}

public function free(): void
{
$this->data = [];
Expand Down
70 changes: 26 additions & 44 deletions src/Cache/ResultCacheStatement.php → src/Cache/CachingResult.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,11 @@
use Doctrine\DBAL\Driver\DriverException;
use Doctrine\DBAL\Driver\FetchUtils;
use Doctrine\DBAL\Driver\Result;
use Doctrine\DBAL\Driver\ResultStatement;

use function array_map;
use function array_values;

/**
* Cache statement for SQL results.
*
* A result is saved in multiple cache keys, there is the originally specified
* cache key which is just pointing to result rows by key. The following things
* have to be ensured:
Expand All @@ -24,12 +21,12 @@
* Also you have to realize that the cache will load the whole result into memory at once to ensure 2.
* This means that the memory usage for cached results might increase by using this feature.
*
* @deprecated
* @internal The class is internal to the caching layer implementation.
*/
class ResultCacheStatement implements ResultStatement, Result
class CachingResult implements Result
{
/** @var Cache */
private $resultCache;
private $cache;

/** @var string */
private $cacheKey;
Expand All @@ -40,8 +37,8 @@ class ResultCacheStatement implements ResultStatement, Result
/** @var int */
private $lifetime;

/** @var ResultStatement */
private $statement;
/** @var Result */
private $result;

/** @var array<int,array<string,mixed>>|null */
private $data;
Expand All @@ -51,38 +48,13 @@ class ResultCacheStatement implements ResultStatement, Result
* @param string $realKey
* @param int $lifetime
*/
public function __construct(ResultStatement $stmt, Cache $resultCache, $cacheKey, $realKey, $lifetime)
{
$this->statement = $stmt;
$this->resultCache = $resultCache;
$this->cacheKey = $cacheKey;
$this->realKey = $realKey;
$this->lifetime = $lifetime;
}

/**
* {@inheritdoc}
*
* @deprecated Use free() instead.
*/
public function closeCursor()
{
$this->free();

return true;
}

/**
* {@inheritdoc}
*/
public function columnCount()
public function __construct(Result $result, Cache $cache, $cacheKey, $realKey, $lifetime)
{
return $this->statement->columnCount();
}

public function rowCount(): int
{
return $this->statement->rowCount();
$this->result = $result;
$this->cache = $cache;
$this->cacheKey = $cacheKey;
$this->realKey = $realKey;
$this->lifetime = $lifetime;
}

/**
Expand Down Expand Up @@ -121,7 +93,7 @@ public function fetchOne()
public function fetchAllNumeric(): array
{
$this->store(
$this->statement->fetchAllAssociative()
$this->result->fetchAllAssociative()
);

return array_map('array_values', $this->data);
Expand All @@ -133,7 +105,7 @@ public function fetchAllNumeric(): array
public function fetchAllAssociative(): array
{
$this->store(
$this->statement->fetchAllAssociative()
$this->result->fetchAllAssociative()
);

return $this->data;
Expand All @@ -147,6 +119,16 @@ public function fetchFirstColumn(): array
return FetchUtils::fetchFirstColumn($this);
}

public function rowCount(): int
{
return $this->result->rowCount();
}

public function columnCount(): int
{
return $this->result->columnCount();
}

public function free(): void
{
$this->data = null;
Expand All @@ -163,7 +145,7 @@ private function fetch()
$this->data = [];
}

$row = $this->statement->fetchAssociative();
$row = $this->result->fetchAssociative();

if ($row !== false) {
$this->data[] = $row;
Expand Down Expand Up @@ -192,14 +174,14 @@ private function saveToCache(): void
return;
}

$data = $this->resultCache->fetch($this->cacheKey);
$data = $this->cache->fetch($this->cacheKey);

if ($data === false) {
$data = [];
}

$data[$this->realKey] = $this->data;

$this->resultCache->save($this->cacheKey, $data, $this->lifetime);
$this->cache->save($this->cacheKey, $data, $this->lifetime);
}
}
Loading

0 comments on commit 0e74935

Please sign in to comment.