Skip to content

Commit

Permalink
Invalidate old query cache format (#6510)
Browse files Browse the repository at this point in the history
|      Q       |   A
|------------- | -----------
| Type         | improvement
| Fixed issues | Follows #6504

#### Summary

I'm changing the query cache format once again: Instead of storing a
tuple of arrays, I'm now storing the whole `ArrayResult` object. This
way, we can easily detect if we support the data returned from the cache
and we can handle future cache format changes in the `__unserialize()`
methods.

After #6504, we would run into errors if an app would attempt to us a
cache written with DBAL 4.1. With my change, the old cache is ignored
and the app would behave as if it had encountered a cache miss instead.

I've also added tests covering the serialization of `ArrayResult`
objects.
  • Loading branch information
derrabus authored Aug 30, 2024
1 parent d47b6b1 commit c56a608
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 11 deletions.
32 changes: 30 additions & 2 deletions src/Cache/ArrayResult.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
use Doctrine\DBAL\Exception\InvalidColumnIndex;

use function array_combine;
use function array_keys;
use function array_map;
use function array_values;
use function count;

/** @internal The class is internal to the caching layer implementation. */
Expand All @@ -21,8 +24,10 @@ final class ArrayResult implements Result
* @param list<list<mixed>> $rows The rows of the result. Each row must have the same number of columns
* as the number of column names.
*/
public function __construct(private readonly array $columnNames, private array $rows)
{
public function __construct(
private readonly array $columnNames,
private array $rows,
) {
}

public function fetchNumeric(): array|false
Expand Down Expand Up @@ -96,6 +101,29 @@ public function free(): void
$this->rows = [];
}

/** @return array{list<string>, list<list<mixed>>} */
public function __serialize(): array
{
return [$this->columnNames, $this->rows];
}

/** @param mixed[] $data */
public function __unserialize(array $data): void
{
// Handle objects serialized with DBAL 4.1 and earlier.
if (isset($data["\0" . self::class . "\0data"])) {
/** @var list<array<string, mixed>> $legacyData */
$legacyData = $data["\0" . self::class . "\0data"];

$this->columnNames = array_keys($legacyData[0] ?? []);
$this->rows = array_map(array_values(...), $legacyData);

return;
}

[$this->columnNames, $this->rows] = $data;
}

/** @return list<mixed>|false */
private function fetch(): array|false
{
Expand Down
10 changes: 4 additions & 6 deletions src/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -810,10 +810,8 @@ public function executeCacheQuery(string $sql, array $params, array $types, Quer
$value = [];
}

if (isset($value[$realKey])) {
[$columnNames, $rows] = $value[$realKey];

return new Result(new ArrayResult($columnNames, $rows), $this);
if (isset($value[$realKey]) && $value[$realKey] instanceof ArrayResult) {
return new Result($value[$realKey], $this);
}
} else {
$value = [];
Expand All @@ -828,7 +826,7 @@ public function executeCacheQuery(string $sql, array $params, array $types, Quer

$rows = $result->fetchAllNumeric();

$value[$realKey] = [$columnNames, $rows];
$value[$realKey] = new ArrayResult($columnNames, $rows);

$item->set($value);

Expand All @@ -839,7 +837,7 @@ public function executeCacheQuery(string $sql, array $params, array $types, Quer

$resultCache->save($item);

return new Result(new ArrayResult($columnNames, $rows), $this);
return new Result($value[$realKey], $this);
}

/**
Expand Down
66 changes: 66 additions & 0 deletions tests/Cache/ArrayResultTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,15 @@

use Doctrine\DBAL\Cache\ArrayResult;
use Doctrine\DBAL\Exception\InvalidColumnIndex;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\TestWith;
use PHPUnit\Framework\TestCase;

use function assert;
use function file_get_contents;
use function serialize;
use function unserialize;

class ArrayResultTest extends TestCase
{
private ArrayResult $result;
Expand Down Expand Up @@ -105,4 +111,64 @@ public function testSameColumnNames(): void

self::assertEquals([1, 2], $result->fetchNumeric());
}

public function testSerialize(): void
{
$result = unserialize(serialize($this->result));

self::assertSame([
[
'username' => 'jwage',
'active' => true,
],
[
'username' => 'romanb',
'active' => false,
],
], $result->fetchAllAssociative());

self::assertSame(2, $result->columnCount());
self::assertSame('username', $result->getColumnName(0));
}

public function testRowPointerIsNotSerialized(): void
{
$this->result->fetchAssociative();
$result = unserialize(serialize($this->result));

self::assertSame([
'username' => 'jwage',
'active' => true,
], $result->fetchAssociative());
}

#[DataProvider('provideSerializedResultFiles')]
public function testUnserialize(string $file): void
{
$serialized = file_get_contents($file);
assert($serialized !== false);
$result = unserialize($serialized);

self::assertInstanceOf(ArrayResult::class, $result);
self::assertSame([
[
'username' => 'jwage',
'active' => true,
],
[
'username' => 'romanb',
'active' => false,
],
], $result->fetchAllAssociative());

self::assertSame(2, $result->columnCount());
self::assertSame('username', $result->getColumnName(0));
}

/** @return iterable<string, array{string}> */
public static function provideSerializedResultFiles(): iterable
{
yield '4.1 format' => [__DIR__ . '/Fixtures/array-result-4.1.txt'];
yield '4.2 format' => [__DIR__ . '/Fixtures/array-result-4.2.txt'];
}
}
Binary file added tests/Cache/Fixtures/array-result-4.1.txt
Binary file not shown.
1 change: 1 addition & 0 deletions tests/Cache/Fixtures/array-result-4.2.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
O:31:"Doctrine\DBAL\Cache\ArrayResult":2:{i:0;a:2:{i:0;s:8:"username";i:1;s:6:"active";}i:1;a:2:{i:0;a:2:{i:0;s:5:"jwage";i:1;b:1;}i:1;a:2:{i:0;s:6:"romanb";i:1;b:0;}}}
23 changes: 20 additions & 3 deletions tests/Connection/CachedQueryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,25 @@ public function testCachedQueryWithChangedImplementationIsExecutedTwice(): void
)->fetchAllAssociative());
}

public function testOldCacheFormat(): void
{
$connection = $this->createConnection(1, ['foo'], [['bar']]);
$cache = new ArrayAdapter();
$qcp = new QueryCacheProfile(0, __FUNCTION__, $cache);

[$cacheKey, $realKey] = $qcp->generateCacheKeys('SELECT 1', [], [], []);
$cache->save(
$cache->getItem($cacheKey)->set([$realKey => [['foo' => 'bar']]]),
);

self::assertSame([['foo' => 'bar']], $connection->executeCacheQuery('SELECT 1', [], [], $qcp)
->fetchAllAssociative());
self::assertSame([['foo' => 'bar']], $connection->executeCacheQuery('SELECT 1', [], [], $qcp)
->fetchAllAssociative());

self::assertCount(1, $cache->getItem(__FUNCTION__)->get());
}

/**
* @param list<string> $columnNames
* @param list<list<mixed>> $rows
Expand All @@ -56,9 +75,7 @@ private function createConnection(int $expectedQueryCount, array $columnNames, a
$connection = $this->createMock(Driver\Connection::class);
$connection->expects(self::exactly($expectedQueryCount))
->method('query')
->willReturnCallback(static function () use ($columnNames, $rows): ArrayResult {
return new ArrayResult($columnNames, $rows);
});
->willReturnCallback(static fn (): ArrayResult => new ArrayResult($columnNames, $rows));

$driver = $this->createMock(Driver::class);
$driver->method('connect')
Expand Down

0 comments on commit c56a608

Please sign in to comment.