Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.2] fix: sessions were not cleared in some test cases #128

Merged
merged 4 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class Connection extends BaseConnection
* @param string $instanceId instance ID
* @param string $database
* @param string $tablePrefix
* @param array $config
* @param array<string, mixed> $config
* @param CacheItemPoolInterface|null $authCache
* @param SessionPoolInterface|null $sessionPool
*/
Expand Down
1 change: 1 addition & 0 deletions tests/Concerns/ManagesDataDefinitionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public function test_createDatabase_with_statements(): void
if (!empty(getenv('SPANNER_EMULATOR_HOST'))) {
$this->setUpEmulatorInstance($conn);
}
$this->beforeApplicationDestroyed(static fn () => $conn->clearSessionPool());

$conn->setEventDispatcher($events);
$conn->enableQueryLog();
Expand Down
11 changes: 6 additions & 5 deletions tests/ConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

use Colopl\Spanner\Connection;
use Colopl\Spanner\Events\MutatingData;
use Colopl\Spanner\Schema\Blueprint;
use Colopl\Spanner\Session\SessionInfo;
use Colopl\Spanner\TimestampBound\ExactStaleness;
use Colopl\Spanner\TimestampBound\MaxStaleness;
Expand All @@ -44,7 +43,6 @@
use Symfony\Component\Cache\Adapter\ArrayAdapter;
use function dirname;
use function fileperms;
use function mkdir;
use function sprintf;
use function substr;

Expand Down Expand Up @@ -290,7 +288,8 @@ public function test_AuthCache_works(): void
$config = $this->app['config']->get('database.connections.main');

$authCache = new ArrayAdapter();
$conn = new Connection($config['instance'], $config['database'], '', $config, $authCache);
$sessionPool = new CacheSessionPool(new ArrayAdapter());
$conn = new Connection($config['instance'], $config['database'], '', $config, $authCache, $sessionPool);
$this->setUpDatabaseOnce($conn);

$conn->selectOne('SELECT 1');
Expand All @@ -312,13 +311,14 @@ public function test_AuthCache_with_FileSystemAdapter(): void
self::assertSame('0644', substr(sprintf('%o', fileperms($outputPath)), -4));
}

public function testSessionPool(): void
public function test_session_pool(): void
{
$config = $this->app['config']->get('database.connections.main');

$cacheItemPool = new ArrayAdapter();
$cacheSessionPool = new CacheSessionPool($cacheItemPool);
$conn = new Connection($config['instance'], $config['database'], '', $config, null, $cacheSessionPool);
$this->setUpDatabaseOnce($conn);
$this->assertInstanceOf(Connection::class, $conn);

$conn->selectOne('SELECT 1');
Expand Down Expand Up @@ -359,7 +359,7 @@ public function test_listSessions(): void
$this->assertInstanceOf(SessionInfo::class, $sessions[0]);
}

public function testStaleReads(): void
public function test_stale_reads(): void
{
$conn = $this->getDefaultConnection();
$tableName = self::TABLE_NAME_USER;
Expand All @@ -373,6 +373,7 @@ public function testStaleReads(): void
$tx->executeUpdate("INSERT INTO ${tableName} (`userId`, `name`) VALUES ('${uuid}', '${name}')");
$timestamp = $tx->commit();
});
$db->close();
$this->assertNotEmpty($timestamp);

$timestampBound = new StrongRead();
Expand Down
38 changes: 27 additions & 11 deletions tests/SessionNotFoundTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ private function getSessionNotFoundConnection(string $sessionNotFoundErrorMode):
$cacheItemPool = new ArrayAdapter();
$cacheSessionPool = new CacheSessionPool($cacheItemPool);
$conn = new Connection($config['instance'], $config['database'], '', $config, null, $cacheSessionPool);

$this->setUpDatabaseOnce($conn);
return $conn;
}
Expand Down Expand Up @@ -170,7 +169,13 @@ public function testSessionNotFoundUnhandledError(): void
// if google changes it then string should be changed in Connection::SESSION_NOT_FOUND_CONDITION
$this->expectExceptionMessage($conn::SESSION_NOT_FOUND_CONDITION);

$conn->selectOne('SELECT 1');
try {
$conn->selectOne('SELECT 1');
} catch (QueryException $e) {
$conn->disconnect();
$conn->clearSessionPool();
throw $e;
}
}

public function testCursorSessionNotFoundUnhandledError(): void
Expand All @@ -188,7 +193,13 @@ public function testCursorSessionNotFoundUnhandledError(): void
// if google changes it then string should be changed in Connection::SESSION_NOT_FOUND_CONDITION
$this->expectExceptionMessage($conn::SESSION_NOT_FOUND_CONDITION);

iterator_to_array($cursor);
try {
iterator_to_array($cursor);
} catch (NotFoundException $e) {
$conn->disconnect();
$conn->clearSessionPool();
throw $e;
}
}

public function testInTransactionSessionNotFoundUnhandledError(): void
Expand All @@ -203,14 +214,19 @@ public function testInTransactionSessionNotFoundUnhandledError(): void

$passes = 0;

$conn->transaction(function () use ($conn, &$passes) {

if ($passes === 0) {
$this->deleteSession($conn);
$passes++;
}
try {
$conn->transaction(function () use ($conn, &$passes) {
if ($passes === 0) {
$this->deleteSession($conn);
$passes++;
}

$conn->selectOne('SELECT 12345');
});
$conn->selectOne('SELECT 12345');
});
} catch (NotFoundException $e) {
$conn->disconnect();
$conn->clearSessionPool();
throw $e;
}
}
}
24 changes: 7 additions & 17 deletions tests/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
namespace Colopl\Spanner\Tests;

use Colopl\Spanner\Connection;
use Colopl\Spanner\Session\SessionInfo;
use Colopl\Spanner\SpannerServiceProvider;
use Google\Cloud\Spanner\Bytes;
use Google\Cloud\Spanner\Date;
Expand All @@ -39,15 +40,6 @@ abstract class TestCase extends \Orchestra\Testbench\TestCase
protected const TABLE_NAME_ITEM_TAG = 'ItemTag';
protected const TABLE_NAME_ARRAY_TEST = 'ArrayTest';

/**
* @return void
*/
protected function tearDown(): void
{
$this->cleanupDatabase();
parent::tearDown();
}

/**
* @return string
*/
Expand Down Expand Up @@ -128,21 +120,19 @@ protected function setUpDatabaseOnce(Connection $conn): void
if (!$conn->databaseExists()) {
$conn->createDatabase($this->getTestDatabaseDDLs());
}
$this->beforeApplicationDestroyed(fn () => $this->cleanupDatabase($conn));
}

/**
* @param Connection $conn
* @return void
*/
protected function cleanupDatabase(): void
protected function cleanupDatabase(Connection $conn): void
{
foreach ($this->app['db']->getConnections() as $conn) {
if ($conn instanceof Connection) {
foreach ($conn->select("SELECT t.table_name FROM information_schema.tables as t WHERE t.table_schema = ''") as $row) {
$conn->table($row['table_name'])->truncate();
}
$conn->clearSessionPool();
}
foreach ($conn->select("SELECT t.table_name FROM information_schema.tables as t WHERE t.table_schema = ''") as $row) {
$conn->table($row['table_name'])->truncate();
}
$conn->clearSessionPool();
}

/**
Expand Down