From 30063c9f792739e91acb369f76d801f10f223f97 Mon Sep 17 00:00:00 2001 From: Lonnie Ezell Date: Fri, 2 Dec 2016 21:13:43 -0600 Subject: [PATCH] Refactoring Database to fire events with the query attached instead of saving all queries. Removed getQueries method from db. First step toward #105 --- system/Database/BaseConnection.php | 90 +++---------------- system/Database/BasePreparedQuery.php | 6 +- system/Database/ConnectionInterface.php | 28 ------ system/Database/Postgre/Connection.php | 12 ++- tests/_support/Database/MockConnection.php | 14 +-- tests/system/Database/BaseConnectionTest.php | 23 +++-- tests/system/Database/Builder/InsertTest.php | 22 ++--- tests/system/Database/Builder/UpdateTest.php | 14 ++- tests/system/Database/Live/SelectTest.php | 2 +- .../source/database/configuration.rst | 3 +- user_guide_src/source/database/helpers.rst | 15 +--- 11 files changed, 59 insertions(+), 170 deletions(-) diff --git a/system/Database/BaseConnection.php b/system/Database/BaseConnection.php index 7c46ba997dd9..e7a765c58127 100644 --- a/system/Database/BaseConnection.php +++ b/system/Database/BaseConnection.php @@ -37,6 +37,7 @@ */ use CodeIgniter\DatabaseException; +use CodeIgniter\Hooks\Hooks; /** * Class BaseConnection @@ -188,23 +189,15 @@ abstract class BaseConnection implements ConnectionInterface */ public $failover = []; - /** - * Whether to keep an in-memory history of queries - * for debugging and timeline purposes. - * - * @var bool - */ - public $saveQueries = false; - //-------------------------------------------------------------------- /** - * Array of query objects that have executed + * The last query object that was executed * on this connection. * * @var array */ - protected $queries = []; + protected $lastQuery; /** * Connection ID @@ -495,40 +488,6 @@ abstract public function getVersion(); //-------------------------------------------------------------------- - /** - * Specifies whether this connection should keep queries objects or not. - * - * @param bool $save - */ - public function saveQueries($save = false) - { - $this->saveQueries = $save; - - return $this; - } - - //-------------------------------------------------------------------- - - /** - * Stores a new query with the object. This is primarily used by - * the prepared queries. - * - * @param \CodeIgniter\Database\Query $query - * - * @return $this - */ - public function addQuery(Query $query) - { - if ($this->saveQueries) - { - $this->queries[] = $query; - } - - return $this; - } - - //-------------------------------------------------------------------- - /** * Executes the query against the database. * @@ -574,7 +533,9 @@ public function query(string $sql, $binds = null, $queryClass = 'CodeIgniter\\Da $startTime = microtime(true); - + // Always save the last query so we can use + // the getLastQuery() method. + $this->lastQuery = $query; // Run the query for real if (! $this->pretend && false === ($this->resultID = $this->simpleQuery($query->getQuery()))) @@ -583,9 +544,10 @@ public function query(string $sql, $binds = null, $queryClass = 'CodeIgniter\\Da // @todo deal with errors - if ($this->saveQueries && ! $this->pretend) + if (! $this->pretend) { - $this->queries[] = $query; + // Let others do something with this query. + Hooks::trigger('DBQuery', $query); } return new $resultClass($this->connID, $this->resultID); @@ -593,9 +555,10 @@ public function query(string $sql, $binds = null, $queryClass = 'CodeIgniter\\Da $query->setDuration($startTime); - if ($this->saveQueries && ! $this->pretend) + if (! $this->pretend) { - $this->queries[] = $query; + // Let others do somethign with this query + Hooks::trigger('DBQuery', $query); } // If $pretend is true, then we just want to return @@ -691,31 +654,6 @@ public function prepare(\Closure $func, array $options = []) //-------------------------------------------------------------------- - /** - * Returns an array containing all of the - * - * @return array - */ - public function getQueries(): array - { - return $this->queries; - } - - //-------------------------------------------------------------------- - - /** - * Returns the total number of queries that have been performed - * on this connection. - * - * @return mixed - */ - public function getQueryCount() - { - return count($this->queries); - } - - //-------------------------------------------------------------------- - /** * Returns the last query's statement object. * @@ -723,7 +661,7 @@ public function getQueryCount() */ public function getLastQuery() { - return end($this->queries); + return $this->lastQuery; } //-------------------------------------------------------------------- @@ -735,7 +673,7 @@ public function getLastQuery() */ public function showLastQuery() { - return (string)end($this->queries); + return (string)$this->lastQuery; } //-------------------------------------------------------------------- diff --git a/system/Database/BasePreparedQuery.php b/system/Database/BasePreparedQuery.php index 04427c72cd2d..b96e63772527 100644 --- a/system/Database/BasePreparedQuery.php +++ b/system/Database/BasePreparedQuery.php @@ -1,5 +1,7 @@ setDuration($startTime); - // Save it to the connection - $this->db->addQuery($query); + // Let others do something with this query + Hooks::trigger('DBQuery', $query); // Return a result object $resultClass = str_replace('PreparedQuery', 'Result', get_class($this)); diff --git a/system/Database/ConnectionInterface.php b/system/Database/ConnectionInterface.php index 06a125553ee5..238216eadba9 100644 --- a/system/Database/ConnectionInterface.php +++ b/system/Database/ConnectionInterface.php @@ -142,15 +142,6 @@ public function getVersion(); //-------------------------------------------------------------------- - /** - * Specifies whether this connection should keep queries objects or not. - * - * @param bool $doLog - */ - public function saveQueries($doLog = false); - - //-------------------------------------------------------------------- - /** * Orchestrates a query against the database. Queries must use * Database\Statement objects to store the query and build it. @@ -192,25 +183,6 @@ public function table($tableName); //-------------------------------------------------------------------- - /** - * Returns an array containing all of the - * - * @return array - */ - public function getQueries(): array; - - //-------------------------------------------------------------------- - - /** - * Returns the total number of queries that have been performed - * on this connection. - * - * @return mixed - */ - public function getQueryCount(); - - //-------------------------------------------------------------------- - /** * Returns the last query's statement object. * diff --git a/system/Database/Postgre/Connection.php b/system/Database/Postgre/Connection.php index 592c085ff2c6..b564f17fcb58 100644 --- a/system/Database/Postgre/Connection.php +++ b/system/Database/Postgre/Connection.php @@ -83,8 +83,18 @@ public function connect($persistent = false) $this->buildDSN(); } + // Strip pgsql if exists + if (mb_strpos($this->DSN, 'pgsql:') === 0) + { + $this->DSN = mb_substr($this->DSN, 6); + } + + // Convert semicolons to spaces. + $this->DSN = str_replace(';', ' ', $this->DSN); + $this->connID = $persistent === true - ? pg_pconnect($this->DSN) : pg_connect($this->DSN); + ? pg_pconnect($this->DSN) + : pg_connect($this->DSN); if ($this->connID !== false) { diff --git a/tests/_support/Database/MockConnection.php b/tests/_support/Database/MockConnection.php index 56ed9f590bc7..687e3f71e29a 100644 --- a/tests/_support/Database/MockConnection.php +++ b/tests/_support/Database/MockConnection.php @@ -6,7 +6,7 @@ class MockConnection extends BaseConnection public $database; - public $saveQueries = true; + public $lastQuery; //-------------------------------------------------------------------- @@ -34,6 +34,8 @@ public function query(string $sql, $binds = null) $startTime = microtime(true); + $this->lastQuery = $query; + // Run the query if (false === ($this->resultID = $this->simpleQuery($query->getQuery()))) { @@ -41,21 +43,11 @@ public function query(string $sql, $binds = null) // @todo deal with errors - if ($this->saveQueries) - { - $this->queries[] = $query; - } - return false; } $query->setDuration($startTime); - if ($this->saveQueries) - { - $this->queries[] = $query; - } - $resultClass = str_replace('Connection', 'Result', get_class($this)); return new $resultClass($this->connID, $this->resultID); diff --git a/tests/system/Database/BaseConnectionTest.php b/tests/system/Database/BaseConnectionTest.php index aa581de77680..cd8c11d5b6bb 100644 --- a/tests/system/Database/BaseConnectionTest.php +++ b/tests/system/Database/BaseConnectionTest.php @@ -47,13 +47,13 @@ class BaseConnectionTest extends \CIUnitTestCase 'failover' => [], 'saveQueries' => true, ]; - + //-------------------------------------------------------------------- - - public function testSavesConfigOptions() + + public function testSavesConfigOptions() { $db = new MockConnection($this->options); - + $this->assertSame('localhost', $db->hostname); $this->assertSame('first', $db->username); $this->assertSame('last', $db->password); @@ -70,23 +70,22 @@ public function testSavesConfigOptions() $this->assertSame(false, $db->compress); $this->assertSame(true, $db->strictOn); $this->assertSame([], $db->failover); - $this->assertSame(true, $db->saveQueries); } - + //-------------------------------------------------------------------- - - public function testConnectionThrowExceptionWhenCannotConnect() + + public function testConnectionThrowExceptionWhenCannotConnect() { $db = new MockConnection($this->options); - + $this->setExpectedException('CodeIgniter\DatabaseException', 'Unable to connect to the database.'); - + $db->shouldReturn('connect', false) ->initialize(); } - + //-------------------------------------------------------------------- - + public function testCanConnectAndStoreConnection() { $db = new MockConnection($this->options); diff --git a/tests/system/Database/Builder/InsertTest.php b/tests/system/Database/Builder/InsertTest.php index 0d7c42f6c598..ca24690e4b85 100644 --- a/tests/system/Database/Builder/InsertTest.php +++ b/tests/system/Database/Builder/InsertTest.php @@ -46,7 +46,7 @@ public function testThrowsExceptionOnNoValuesSet() } //-------------------------------------------------------------------- - + public function testInsertBatch() { $builder = $this->db->table('jobs'); @@ -61,25 +61,17 @@ public function testInsertBatch() $builder->insertBatch($insertData, true, true); - $queries = $this->db->getQueries(); - - $q1 = $queries[0]; - $q2 = $queries[1]; + $query = $this->db->getLastQuery(); - $this->assertTrue($q1 instanceof Query); - $this->assertTrue($q2 instanceof Query); + $this->assertTrue($query instanceof Query); - $raw1 = "INSERT INTO \"jobs\" (\"description\", \"id\", \"name\") VALUES (:description,:id,:name)"; - $raw2 = "INSERT INTO \"jobs\" (\"description\", \"id\", \"name\") VALUES (:description0,:id0,:name0)"; + $raw = "INSERT INTO \"jobs\" (\"description\", \"id\", \"name\") VALUES (:description0,:id0,:name0)"; - $this->assertEquals($raw1, str_replace("\n", ' ', $q1->getOriginalQuery() )); - $this->assertEquals($raw2, str_replace("\n", ' ', $q2->getOriginalQuery() )); + $this->assertEquals($raw, str_replace("\n", ' ', $query->getOriginalQuery() )); - $expected1 = "INSERT INTO \"jobs\" (\"description\", \"id\", \"name\") VALUES ('Theres something in your teeth',2,'Commedian')"; - $expected2 = "INSERT INTO \"jobs\" (\"description\", \"id\", \"name\") VALUES ('Iam yellow',3,'Cab Driver')"; + $expected = "INSERT INTO \"jobs\" (\"description\", \"id\", \"name\") VALUES ('Iam yellow',3,'Cab Driver')"; - $this->assertEquals($expected1, str_replace("\n", ' ', $q1->getQuery() )); - $this->assertEquals($expected2, str_replace("\n", ' ', $q2->getQuery() )); + $this->assertEquals($expected, str_replace("\n", ' ', $query->getQuery() )); } //-------------------------------------------------------------------- diff --git a/tests/system/Database/Builder/UpdateTest.php b/tests/system/Database/Builder/UpdateTest.php index 47945f31e24b..a712799f3c83 100644 --- a/tests/system/Database/Builder/UpdateTest.php +++ b/tests/system/Database/Builder/UpdateTest.php @@ -16,11 +16,11 @@ public function setUp() } //-------------------------------------------------------------------- - - public function testUpdate() + + public function testUpdate() { $builder = new BaseBuilder('jobs', $this->db); - + $builder->where('id', 1)->update(['name' => 'Programmer'], null, null, true); $expectedSQL = "UPDATE \"jobs\" SET \"name\" = :name WHERE \"id\" = :id"; @@ -29,7 +29,7 @@ public function testUpdate() $this->assertEquals($expectedSQL, str_replace("\n", ' ', $builder->getCompiledUpdate())); $this->assertEquals($expectedBinds, $builder->getBinds()); } - + //-------------------------------------------------------------------- public function testUpdateInternalWhereAndLimit() @@ -87,11 +87,7 @@ public function testUpdateBatch() $builder->updateBatch($updateData, 'id'); - $query = $this->db->getQueries(); - - $this->assertTrue(is_array($query)); - - $query = $query[0]; + $query = $this->db->getLastQuery(); $this->assertTrue($query instanceof MockQuery); diff --git a/tests/system/Database/Live/SelectTest.php b/tests/system/Database/Live/SelectTest.php index 16a8a3dfb5e0..d5a03d52ec4d 100644 --- a/tests/system/Database/Live/SelectTest.php +++ b/tests/system/Database/Live/SelectTest.php @@ -144,4 +144,4 @@ public function testSelectDistinctCanBeTurnedOff() } //-------------------------------------------------------------------- -} \ No newline at end of file +} diff --git a/user_guide_src/source/database/configuration.rst b/user_guide_src/source/database/configuration.rst index 1a361bfdce47..a91c5ff95ded 100644 --- a/user_guide_src/source/database/configuration.rst +++ b/user_guide_src/source/database/configuration.rst @@ -29,7 +29,6 @@ prototype:: 'compress' => FALSE, 'strictOn' => FALSE, 'failover' => array(), - 'saveQueries' => true ]; The name of the class property is the connection name, and can be used @@ -195,7 +194,7 @@ Explanation of Values: - 'sqlsrv' and 'pdo/sqlsrv' drivers accept TRUE/FALSE - 'MySQLi' and 'pdo/mysql' drivers accept an array with the following options: - + - 'ssl_key' - Path to the private key file - 'ssl_cert' - Path to the public key certificate file - 'ssl_ca' - Path to the certificate authority file diff --git a/user_guide_src/source/database/helpers.rst b/user_guide_src/source/database/helpers.rst index 1f510eef9f9f..297c75151fcd 100644 --- a/user_guide_src/source/database/helpers.rst +++ b/user_guide_src/source/database/helpers.rst @@ -10,7 +10,7 @@ Information From Executing a Query The insert ID number when performing database inserts. .. note:: If using the PDO driver with PostgreSQL, or using the Interbase - driver, this function requires a $name parameter, which specifies the + driver, this function requires a $name parameter, which specifies the appropriate sequence to check for the insert id. **$db->affectedRows()** @@ -27,17 +27,6 @@ Displays the number of affected rows, when doing "write" type queries Returns a Query object that represents the last query that was run (the query string, not the result). - -.. note:: Disabling the **saveQueries** setting in your database - configuration will render this function useless. - -**$db->getQueries()** - -Returns an array of Query objects that represent all of the queries ran on this connection. - -.. note:: Disabling the **saveQueries** setting in your database - configuration will render this function useless. - Information About Your Database =============================== @@ -48,7 +37,7 @@ Submit the table name in the first parameter. This is part of Query Builder. Example:: echo $db->table('my_table')->countAll(); - + // Produces an integer, like 25 **$db->getPlatform()**