Skip to content

Commit 482397c

Browse files
amotlamotl
committed
Doctrine3: Apply suggestions by CodeRabbit, and more copy editing
1 parent b0f7700 commit 482397c

File tree

14 files changed

+90
-84
lines changed

14 files changed

+90
-84
lines changed

‎docs/connect.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ If you plan to query CrateDB via DBAL, you can get a connection from the
3636
'port' => 4200
3737
);
3838
$connection = \Doctrine\DBAL\DriverManager::getConnection($params);
39-
$schemaManager = $connection->getSchemaManager();
39+
$schemaManager = $connection->createSchemaManager();
4040
4141
With these connection parameters, the ``DriverManager`` will attempt to
4242
authenticate as ``crate`` with a CrateDB node listening on ``localhost:4200``.

‎src/Crate/DBAL/Driver/PDOCrate/CrateStatement.php‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,18 +37,17 @@
3737
*/
3838
final class CrateStatement implements StatementInterface
3939
{
40+
private PDOInterface $pdo;
41+
private PDOStatement $stmt;
4042

4143
/**
42-
* @var PDOInterface
44+
* @param string $sql
45+
* @param array<string,mixed> $options
4346
*/
44-
private $pdo;
45-
46-
private PDOStatement $stmt;
47-
48-
public function __construct(PDOInterface $pdo, $sql)
47+
public function __construct(PDOInterface $pdo, $sql, $options = [])
4948
{
50-
$this->pdo = $pdo;
51-
$this->stmt = $pdo->prepare($sql);
49+
$this->pdo = $pdo;
50+
$this->stmt = $pdo->prepare($sql, $options);
5251
}
5352

5453
/**
@@ -90,7 +89,7 @@ public function rowCount(): int
9089
*/
9190
public function bindValue($param, $value, $type = ParameterType::STRING)
9291
{
93-
$this->stmt->bindValue($param, $value, $type);
92+
return $this->stmt->bindValue($param, $value, $type);
9493
}
9594

9695
/**
@@ -132,17 +131,17 @@ public function fetchColumn($column_number = 0)
132131
/**
133132
* {@inheritDoc}
134133
*/
135-
public function closeCursor(): int
134+
public function closeCursor(): bool
136135
{
137136
return $this->stmt->closeCursor();
138137
}
139138

140139
/**
141-
* Gets the wrapped driver statement.
140+
* Gets the wrapped CrateDB PDOStatement.
142141
*
143-
* @return Driver\Statement
142+
* @return PDOStatement
144143
*/
145-
public function getWrappedStatement()
144+
public function getWrappedStatement(): PDOStatement
146145
{
147146
return $this->stmt;
148147
}

‎src/Crate/DBAL/Driver/PDOCrate/Driver.php‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@
2727
use Crate\DBAL\Schema\CrateSchemaManager;
2828
use Doctrine\DBAL\Connection;
2929
use Doctrine\DBAL\Platforms\AbstractPlatform;
30-
use Doctrine\DBAL\VersionAwarePlatformDriver;
30+
use Doctrine\DBAL\Schema\AbstractSchemaManager;
3131

32-
class Driver implements \Doctrine\DBAL\Driver, VersionAwarePlatformDriver
32+
class Driver implements \Doctrine\DBAL\Driver
3333
{
3434
const VERSION = '4.0.3';
3535
const NAME = 'crate';
@@ -41,8 +41,12 @@ class Driver implements \Doctrine\DBAL\Driver, VersionAwarePlatformDriver
4141
* {@inheritDoc}
4242
* @return PDOConnection The database connection.
4343
*/
44-
public function connect(array $params, $username = null, $password = null, array $driverOptions = array())
45-
{
44+
public function connect(
45+
array $params,
46+
$username = null,
47+
$password = null,
48+
array $driverOptions = array(),
49+
): PDOConnection {
4650
return new PDOConnection($this->constructPdoDsn($params), $username, $password, $driverOptions);
4751
}
4852

@@ -51,7 +55,7 @@ public function connect(array $params, $username = null, $password = null, array
5155
*
5256
* @return string The DSN.
5357
*/
54-
private function constructPdoDsn(array $params)
58+
private function constructPdoDsn(array $params): string
5559
{
5660
$dsn = self::NAME . ':';
5761
if (isset($params['host']) && $params['host'] != '') {
@@ -67,40 +71,40 @@ private function constructPdoDsn(array $params)
6771
/**
6872
* {@inheritDoc}
6973
*/
70-
public function getDatabasePlatform()
74+
public function getDatabasePlatform(): AbstractPlatform
7175
{
7276
return new CratePlatform4();
7377
}
7478

7579
/**
7680
* {@inheritDoc}
7781
*/
78-
public function getSchemaManager(Connection $conn, AbstractPlatform $platform)
82+
public function getSchemaManager(Connection $conn, AbstractPlatform $platform): AbstractSchemaManager
7983
{
8084
// Added by Doctrine 3.
81-
return new CrateSchemaManager($conn, $conn->getDatabasePlatform());
85+
return new CrateSchemaManager($conn, $platform);
8286
}
8387

8488
/**
8589
* {@inheritDoc}
8690
*/
87-
public function getName()
91+
public function getName(): string
8892
{
8993
return self::NAME;
9094
}
9195

9296
/**
9397
* {@inheritDoc}
9498
*/
95-
public function getDatabase(Connection $conn)
99+
public function getDatabase(Connection $conn): string|null
96100
{
97101
return null;
98102
}
99103

100104
/**
101105
* {@inheritDoc}
102106
*/
103-
public function createDatabasePlatformForVersion($version)
107+
public function createDatabasePlatformForVersion($version): AbstractPlatform
104108
{
105109
if (version_compare($version, self::VERSION_057, "<")) {
106110
return new CratePlatform();

‎src/Crate/DBAL/Driver/PDOCrate/PDOConnection.php‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -91,20 +91,6 @@ public function exec($sql): int
9191
}
9292
}
9393

94-
/**
95-
* {@inheritDoc}
96-
*/
97-
public function quer2y(string $sql): Result
98-
{
99-
try {
100-
$result = $this->connection->query($sql);
101-
assert($result !== false);
102-
return new Result($result);
103-
} catch (PDOException $exception) {
104-
throw Exception::new($exception);
105-
}
106-
}
107-
10894
public function query(string $sql): ResultInterface
10995
{
11096
try {
@@ -121,7 +107,7 @@ public function quote($value, $type = ParameterType::STRING): string
121107
return $this->connection->quote($value, $type);
122108
}
123109

124-
public function lastInsertId($name = null): string
110+
public function lastInsertId($name = null): string|false
125111
{
126112
return $this->connection->lastInsertId($name);
127113
}

‎src/Crate/DBAL/Schema/CrateSchemaManager.php‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
*/
2222
namespace Crate\DBAL\Schema;
2323

24-
use Crate\DBAL\Platforms\CratePlatform4;
24+
use Crate\DBAL\Platforms\CratePlatform;
2525
use Doctrine\DBAL\Schema\AbstractSchemaManager;
2626
use Doctrine\DBAL\Schema\Column;
2727
use Doctrine\DBAL\Types\Type;
@@ -30,7 +30,7 @@
3030
/**
3131
* Schema manager for the CrateDB RDBMS.
3232
*
33-
* @extends AbstractSchemaManager<CratePlatform4>
33+
* @extends AbstractSchemaManager<CratePlatform>
3434
*/
3535
class CrateSchemaManager extends AbstractSchemaManager
3636
{
@@ -130,20 +130,20 @@ private function flatten(array $array, string $prefix = '') : array
130130
/**
131131
* {@inheritDoc}
132132
*/
133-
public function listTableDetails($tableName) : Table
133+
public function listTableDetails($name) : Table
134134
{
135-
$columns = $this->listTableColumns($tableName);
136-
$indexes = $this->listTableIndexes($tableName);
135+
$columns = $this->listTableColumns($name);
136+
$indexes = $this->listTableIndexes($name);
137137
$options = [];
138138

139-
$s = $this->_conn->fetchAssociative($this->_platform->getTableOptionsSQL($tableName));
139+
$s = $this->_conn->fetchAssociative($this->_platform->getTableOptionsSQL($name));
140140

141141
$options['sharding_routing_column'] = $s['clustered_by'];
142142
$options['sharding_num_shards'] = $s['number_of_shards'];
143143
$options['partition_columns'] = $s['partitioned_by'];
144144
$options['table_options'] = self::flatten($s['settings']);
145145
$options['table_options']['number_of_replicas'] = $s['number_of_replicas'];
146146
$options['table_options']['column_policy'] = $s['column_policy'];
147-
return new Table($tableName, $columns, $indexes, [], [], $options);
147+
return new Table($name, $columns, $indexes, [], [], $options);
148148
}
149149
}

‎test/Crate/Test/DBAL/Functional/BindingTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public function testBindNamedValue()
8888

8989
public function testBindTimestamp()
9090
{
91-
if ($this->_conn->getSchemaManager()->tablesExist("foo")) {
91+
if ($this->_conn->createSchemaManager()->tablesExist("foo")) {
9292
$this->execute("DROP TABLE foo");
9393
}
9494

‎test/Crate/Test/DBAL/Functional/ConnectionTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public function testBasicAuthConnection()
5555
$credentials = $conn->getNativeConnection()->getAttribute(PDOCrateDB::CRATE_ATTR_HTTP_BASIC_AUTH);
5656

5757
// No leaks any longer: Any empty array is all we got.
58-
$this->assertEquals($credentials, array());
58+
$this->assertEquals(array(), $credentials);
5959
}
6060

6161
public function testGetConnection()

‎test/Crate/Test/DBAL/Functional/DataAccessTest.php‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@
2727
use Doctrine\DBAL\Exception as DBALException;
2828
use Doctrine\DBAL\ParameterType;
2929
use Doctrine\DBAL\Schema\Column;
30-
use Doctrine\DBAL\Types\Type;
3130
use Doctrine\DBAL\Connection;
31+
use Doctrine\DBAL\Schema\Table;
32+
use Doctrine\DBAL\Types\Type;
3233
use Doctrine\DBAL\Types\Types;
3334
use PDO;
3435

@@ -44,15 +45,16 @@ public function setUp() : void
4445
if (self::$generated === false) {
4546
self::$generated = true;
4647
/* @var $sm \Doctrine\DBAL\Schema\AbstractSchemaManager */
47-
$sm = $this->_conn->getSchemaManager();
48-
$table = new \Doctrine\DBAL\Schema\Table("fetch_table");
48+
$sm = $this->_conn->createSchemaManager();
49+
$table = new Table("fetch_table");
4950
$table->addColumn('test_int', 'integer');
5051
$table->addColumn('test_string', 'string');
5152
$table->addColumn('test_datetime', 'timestamp', array('notnull' => false));
5253
$table->addColumn('test_array', 'array', array('columnDefinition'=>'ARRAY(STRING)'));
5354
$platformOptions = array(
5455
'type' => MapType::STRICT,
5556
'fields' => array(
57+
// Those intentionally use DBAL types.
5658
new Column('id', Type::getType('integer'), array()),
5759
new Column('name', Type::getType('string'), array()),
5860
new Column('value', Type::getType('float'), array()),
@@ -78,7 +80,7 @@ public function setUp() : void
7880
public function tearDown() : void
7981
{
8082
if (self::$generated === true) {
81-
$this->_conn->getSchemaManager()->dropTable('fetch_table');
83+
$this->_conn->createSchemaManager()->dropTable('fetch_table');
8284
self::$generated = false;
8385
}
8486
}
@@ -93,7 +95,7 @@ public function testPrepareWithBindValue()
9395
$stmt->bindValue(2, 'foo', PDO::PARAM_STR);
9496
$result = $stmt->executeQuery();
9597

96-
$row = $result->fetch(PDO::FETCH_ASSOC);
98+
$row = $result->fetchAssociative();
9799
$row = array_change_key_case($row, \CASE_LOWER);
98100
$this->assertEquals(array('test_int' => 1, 'test_string' => 'foo'), $row);
99101
}
@@ -158,6 +160,7 @@ public function testPrepareWithFetchColumn()
158160

159161
$stmt->bindParam(1, $paramInt, PDO::PARAM_INT);
160162
$stmt->bindParam(2, $paramStr, PDO::PARAM_STR);
163+
$stmt->execute();
161164
$column = $stmt->getWrappedStatement()->fetchColumn();
162165

163166
$this->assertEquals(1, $column);
@@ -533,8 +536,9 @@ public function testEmptyFetchColumnReturnsFalse()
533536
{
534537
$this->_conn->executeStatement('DELETE FROM fetch_table');
535538
$this->refresh("fetch_table");
536-
$this->assertFalse($this->_conn->prepare('SELECT test_int FROM fetch_table')->getWrappedStatement()->fetchColumn());
537-
$this->assertFalse($this->_conn->prepare('SELECT test_int FROM fetch_table')->getWrappedStatement()->fetchColumn());
539+
$stmt = $this->_conn->prepare('SELECT test_int FROM fetch_table');
540+
$stmt->execute();
541+
$this->assertFalse($stmt->getWrappedStatement()->fetchColumn());
538542
}
539543

540544
/**

‎test/Crate/Test/DBAL/Functional/ModifyLimitQueryTest.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
use Crate\Test\DBAL\DBALFunctionalTestCase;
2626
use Doctrine\DBAL\Exception as DBALException;
27+
use Doctrine\DBAL\Schema\Table;
2728

2829
class ModifyLimitQueryTest extends DBALFunctionalTestCase
2930
{
@@ -35,16 +36,16 @@ public function setUp() : void
3536

3637
if (!self::$tableCreated) {
3738
/* @var $sm \Doctrine\DBAL\Schema\AbstractSchemaManager */
38-
$table = new \Doctrine\DBAL\Schema\Table("modify_limit_table");
39+
$table = new Table("modify_limit_table");
3940
$table->addColumn('test_int', 'integer');
4041
$table->setPrimaryKey(array('test_int'));
4142

42-
$table2 = new \Doctrine\DBAL\Schema\Table("modify_limit_table2");
43+
$table2 = new Table("modify_limit_table2");
4344
$table2->addColumn('id', 'integer', array('autoincrement' => true));
4445
$table2->addColumn('test_int', 'integer');
4546
$table2->setPrimaryKey(array('id'));
4647

47-
$sm = $this->_conn->getSchemaManager();
48+
$sm = $this->_conn->createSchemaManager();
4849
$sm->createTable($table);
4950
$sm->createTable($table2);
5051
self::$tableCreated = true;
@@ -55,7 +56,7 @@ public function tearDown() : void
5556
{
5657
parent::tearDown();
5758
if (self::$tableCreated) {
58-
$sm = $this->_conn->getSchemaManager();
59+
$sm = $this->_conn->createSchemaManager();
5960
try {
6061
$sm->dropTable('modify_limit_table');
6162
$sm->dropTable('modify_limit_table2');

‎test/Crate/Test/DBAL/Functional/NamedParametersTest.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Crate\Test\DBAL\DBALFunctionalTestCase;
66
use Doctrine\DBAL\Connection;
7+
use Doctrine\DBAL\Schema\Table;
78
use PDO;
89

910
/**
@@ -106,16 +107,16 @@ public function setUp() : void
106107
{
107108
parent::setUp();
108109

109-
if (!$this->_conn->getSchemaManager()->tablesExist("ddc1372_foobar")) {
110+
if (!$this->_conn->createSchemaManager()->tablesExist("ddc1372_foobar")) {
110111
try {
111-
$table = new \Doctrine\DBAL\Schema\Table("ddc1372_foobar");
112+
$table = new Table("ddc1372_foobar");
112113
$table->addColumn('id', 'integer');
113114
$table->addColumn('foo','string');
114115
$table->addColumn('bar','string');
115116
$table->setPrimaryKey(array('id'));
116117

117118

118-
$sm = $this->_conn->getSchemaManager();
119+
$sm = $this->_conn->createSchemaManager();
119120
$sm->createTable($table);
120121

121122
$this->_conn->insert('ddc1372_foobar', array(
@@ -147,9 +148,9 @@ public function setUp() : void
147148
public function tearDown() : void
148149
{
149150
parent::tearDown();
150-
if ($this->_conn->getSchemaManager()->tablesExist("ddc1372_foobar")) {
151+
if ($this->_conn->createSchemaManager()->tablesExist("ddc1372_foobar")) {
151152
try {
152-
$sm = $this->_conn->getSchemaManager();
153+
$sm = $this->_conn->createSchemaManager();
153154
$sm->dropTable('ddc1372_foobar');
154155
} catch(\Exception $e) {
155156
$this->fail($e->getMessage());

0 commit comments

Comments
 (0)