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

fix: backport #150 and #166 to 5.x branch #242

Merged
merged 7 commits into from
Nov 20, 2024
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
9 changes: 7 additions & 2 deletions .github/workflows/run-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,10 @@ jobs:
uses: actions/checkout@v3
- run: echo "💡 The ${{ github.repository }} repository has been cloned to the runner."
- run: echo "🖥️ The workflow is now ready to test your code on the runner."
- name: Run test with docker compose
run: docker-compose run test
- name: Build docker image
run: make build
- name: Run test
run: make test
- name: Show failed container logs
if: failure()
run: make logs
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
# v5.4.0 (2024-11-20)

Fixed
- Match internals so that it lines up with laravel 10.34.0. (#242)

# v5.3.0 (2023-11-17)

Fixed
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"php": "^8.1",
"ext-grpc": "*",
"ext-json": "*",
"laravel/framework": "^10.32.0",
"laravel/framework": "^10.34.2",
"google/cloud-spanner": "^1.58.4",
"grpc/grpc": "^1.42",
"symfony/cache": "~6",
Expand Down
16 changes: 10 additions & 6 deletions src/Concerns/ManagesTransactions.php
Original file line number Diff line number Diff line change
Expand Up @@ -141,16 +141,20 @@ protected function performSpannerCommit(): void
$this->currentTransaction->commit();
}

$this->transactionsManager?->stageTransactions($this->getName());
[$levelBeingCommitted, $this->transactions] = [
$this->transactions,
max(0, $this->transactions - 1),
];

$this->transactions = max(0, $this->transactions - 1);
if ($this->isTransactionFinished()) {
$this->currentTransaction = null;
}

if ($this->afterCommitCallbacksShouldBeExecuted()) {
$this->transactionsManager?->commit($this->getName());
}
$this->transactionsManager?->commit(
$this->getName(),
$levelBeingCommitted,
$this->transactions
);
}

/**
Expand All @@ -164,7 +168,7 @@ protected function performRollBack($toLevel)

if ($this->currentTransaction !== null) {
try {
if ($this->currentTransaction->state() === Transaction::STATE_ACTIVE) {
if ($this->currentTransaction->state() === Transaction::STATE_ACTIVE && $this->currentTransaction->id() !== null) {
$this->currentTransaction->rollBack();
}
} finally {
Expand Down
2 changes: 1 addition & 1 deletion src/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ protected function escapeArray(array $value, bool $binary): string
{
if (array_is_list($value)) {
$escaped = array_map(function (mixed $v) use ($binary): string {
return !is_array($v)
return is_scalar($v)
? $this->escape($v, $binary)
: throw new LogicException('Nested arrays are not supported by Cloud Spanner');
}, $value);
Expand Down
16 changes: 12 additions & 4 deletions src/Query/Concerns/UsesMutations.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ trait UsesMutations
*/
public function insertUsingMutation(array $values)
{
$this->connection->insertUsingMutation($this->from, $values);
$this->connection->insertUsingMutation($this->getTableName(), $values);
}

/**
Expand All @@ -40,7 +40,7 @@ public function insertUsingMutation(array $values)
*/
public function updateUsingMutation(array $values)
{
$this->connection->updateUsingMutation($this->from, $values);
$this->connection->updateUsingMutation($this->getTableName(), $values);
}

/**
Expand All @@ -49,7 +49,7 @@ public function updateUsingMutation(array $values)
*/
public function insertOrUpdateUsingMutation(array $values)
{
$this->connection->insertOrUpdateUsingMutation($this->from, $values);
$this->connection->insertOrUpdateUsingMutation($this->getTableName(), $values);
}

/**
Expand All @@ -58,6 +58,14 @@ public function insertOrUpdateUsingMutation(array $values)
*/
public function deleteUsingMutation($keys)
{
$this->connection->deleteUsingMutation($this->from, $keys);
$this->connection->deleteUsingMutation($this->getTableName(), $keys);
}

/**
* @return string
*/
private function getTableName(): string
{
return (string)$this->getGrammar()->getValue($this->from);
}
}
21 changes: 21 additions & 0 deletions src/Query/Processor.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,27 @@ public function processColumnListing($results)
}, $results);
}

/**
* Process the results of a columns query.
*
* @inheritDoc
*/
public function processColumns($results)
{
return array_map(static function (array $result) {
return [
'name' => $result['COLUMN_NAME'],
'type_name' => preg_replace("/\([^)]+\)/", "", $result['SPANNER_TYPE']),
'type' => $result['SPANNER_TYPE'],
'collation' => null,
'nullable' => $result['IS_NULLABLE'] !== 'NO',
'default' => $result['COLUMN_DEFAULT'],
'auto_increment' => false,
'comment' => null,
];
}, $results);
}

/**
* @param array $results
* @return array
Expand Down
16 changes: 2 additions & 14 deletions src/Schema/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ class Builder extends BaseBuilder


/**
* @deprecated Will be removed in a future Laravel version.
*
* @return list<array{ name: string, type: string }>
*/
public function getAllTables()
Expand All @@ -44,20 +46,6 @@ public function getAllTables()
);
}

/**
* @inheritDoc
*/
public function getColumnListing($table)
{
$table = $this->connection->getTablePrefix().$table;

$results = $this->connection->select(
$this->grammar->compileColumnListing(), [$table]
);

return $this->connection->getPostProcessor()->processColumnListing($results);
}

/**
* @param string $table
* @return string[]
Expand Down
34 changes: 34 additions & 0 deletions src/Schema/Grammar.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ class Grammar extends BaseGrammar
protected $modifiers = ['Nullable', 'Default'];

/**
* Compile the query to determine if a table exists.
*
* @deprecated Will be removed in a future Laravel version.
*
* @return string
*/
public function compileTableExists()
Expand All @@ -48,6 +52,18 @@ public function compileTableExists()
}

/**
* Compile the query to determine the tables.
*
* @return string
*/
public function compileTables()
{
return 'select `table_name` as name from information_schema.tables where table_schema = \'\' and table_type = \'BASE TABLE\'';
}

/**
* @deprecated Will be removed in a future Laravel version.
*
* @return string
*/
public function compileGetAllTables()
Expand All @@ -56,6 +72,10 @@ public function compileGetAllTables()
}

/**
* Compile the query to determine the list of columns.
*
* @deprecated Will be removed in a future Laravel version.
*
* @return string
*/
public function compileColumnListing()
Expand All @@ -73,6 +93,20 @@ public function compileIndexListing()
return 'select index_name as `index_name` from information_schema.indexes where table_schema = \'\' and table_name = ?';
}

/**
* Compile the query to determine the columns.
*
* @param string $table
* @return string
*/
public function compileColumns($table)
{
return sprintf(
'select * from information_schema.columns where table_schema = \'\' and table_name = %s',
$this->quoteString($table),
);
}

/**
* Compile a create table command.
*
Expand Down
2 changes: 1 addition & 1 deletion tests/Query/BuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,7 @@ public function test_whereLike(): void
$caughtException = $ex;
}
if (getenv('SPANNER_EMULATOR_HOST')) {
$this->assertStringContainsString('INTERNAL', $caughtException?->getMessage());
$this->assertStringContainsString('Invalid UTF-8', $caughtException?->getMessage());
} else {
$this->assertStringContainsString('Invalid request proto: an error was encountered during deserialization of the request proto.', $caughtException?->getMessage());
}
Expand Down
50 changes: 47 additions & 3 deletions tests/Schema/BuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,23 +187,67 @@ public function testCreateInterleavedTable(): void
$this->assertTrue($sb->hasTable(self::TABLE_NAME_RELATION_CHILD_INTERLEAVED));
}

public function test_getTables(): void
{
$conn = $this->getDefaultConnection();
$sb = $conn->getSchemaBuilder();
$table = $this->generateTableName(class_basename(__CLASS__));

$sb->create($table, function (Blueprint $table) {
$table->uuid('id');
$table->primary('id');
});

/** @var array{ name: string, type: string } $row */
$row = Arr::first(
$sb->getTables(),
static fn (array $row): bool => $row['name'] === $table,
);

$this->assertSame($table, $row['name']);
}

public function test_getColumns(): void
{
$conn = $this->getDefaultConnection();
$sb = $conn->getSchemaBuilder();
$table = $this->generateTableName(class_basename(__CLASS__));

$sb->create($table, function (Blueprint $table) {
$table->uuid('id');
$table->primary('id');
});

$this->assertSame([
'name' => 'id',
'type_name' => 'STRING',
'type' => 'STRING(36)',
'collation' => null,
'nullable' => false,
'default' => null,
'auto_increment' => false,
'comment' => null,
], Arr::first($sb->getColumns($table)));
}

public function test_getAllTables(): void
{
$conn = $this->getDefaultConnection();
$sb = $conn->getSchemaBuilder();
$table = $this->generateTableName(class_basename(__CLASS__));

$sb->create(self::TABLE_NAME_CREATED, function (Blueprint $table) {
$sb->create($table, function (Blueprint $table) {
$table->uuid('id');
$table->primary('id');
});

/** @var array{ name: string, type: string } $row */
$row = Arr::first(
$sb->getAllTables(),
static fn (array $row): bool => $row['name'] === self::TABLE_NAME_CREATED,
static fn (array $row): bool => $row['name'] === $table,
);

$this->assertSame(self::TABLE_NAME_CREATED, $row['name']);
$this->assertSame($table, $row['name']);
$this->assertSame('BASE TABLE', $row['type']);
}
}
56 changes: 2 additions & 54 deletions tests/SessionNotFoundTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,13 @@ public function test_session_not_found_on_cursor(): void
$passes = 0;

$conn->transaction(function () use ($conn, &$passes) {
$cursor = $conn->cursor('SELECT 12345');

if ($passes === 0) {
$this->deleteSession($conn);
$passes++;
}

$cursor = $conn->cursor('SELECT 12345');

$this->assertEquals([12345], $cursor->current());

$passes++;
Expand Down Expand Up @@ -201,56 +201,4 @@ public function test_session_not_found_throw_exception_on_query(): void
throw $e;
}
}

public function test_session_not_found_throw_exception_on_cursor(): void
{
$conn = $this->getSessionNotFoundConnection(Connection::THROW_EXCEPTION);

$cursor = $conn->cursor('SELECT 1');

// deliberately delete session on spanner side
$this->deleteSession($conn);

$this->expectException(NotFoundException::class);

// the string is used in sessionNotFoundWrapper() to catch 'session not found' error,
// if google changes it then string should be changed in Connection::SESSION_NOT_FOUND_CONDITION
$this->expectExceptionMessage($conn::SESSION_NOT_FOUND_CONDITION);

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

public function test_session_not_found_throw_exception_in_transaction(): void
{
$conn = $this->getSessionNotFoundConnection(Connection::THROW_EXCEPTION);

$this->expectException(NotFoundException::class);

// the string is used in sessionNotFoundWrapper() to catch 'session not found' error,
// if google changes it then string should be changed in Connection::SESSION_NOT_FOUND_CONDITION
$this->expectExceptionMessage($conn::SESSION_NOT_FOUND_CONDITION);

$passes = 0;

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

$conn->selectOne('SELECT 12345');
});
} catch (NotFoundException $e) {
$conn->disconnect();
$conn->clearSessionPool();
throw $e;
}
}
}
Loading
Loading