Skip to content

Commit

Permalink
Refactoring Database to fire events with the query attached instead o…
Browse files Browse the repository at this point in the history
…f saving all queries. Removed getQueries method from db. First step toward #105
  • Loading branch information
lonnieezell committed Dec 3, 2016
1 parent 6864e6f commit 30063c9
Show file tree
Hide file tree
Showing 11 changed files with 59 additions and 170 deletions.
90 changes: 14 additions & 76 deletions system/Database/BaseConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
*/

use CodeIgniter\DatabaseException;
use CodeIgniter\Hooks\Hooks;

/**
* Class BaseConnection
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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())))
Expand All @@ -583,19 +544,21 @@ 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);
}

$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
Expand Down Expand Up @@ -691,39 +654,14 @@ 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.
*
* @return mixed
*/
public function getLastQuery()
{
return end($this->queries);
return $this->lastQuery;
}

//--------------------------------------------------------------------
Expand All @@ -735,7 +673,7 @@ public function getLastQuery()
*/
public function showLastQuery()
{
return (string)end($this->queries);
return (string)$this->lastQuery;
}

//--------------------------------------------------------------------
Expand Down
6 changes: 4 additions & 2 deletions system/Database/BasePreparedQuery.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php namespace CodeIgniter\Database;

use CodeIgniter\Hooks\Hooks;

abstract class BasePreparedQuery implements PreparedQueryInterface
{
/**
Expand Down Expand Up @@ -115,8 +117,8 @@ public function execute(...$data)

$query->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));
Expand Down
28 changes: 0 additions & 28 deletions system/Database/ConnectionInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
*
Expand Down
12 changes: 11 additions & 1 deletion system/Database/Postgre/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
14 changes: 3 additions & 11 deletions tests/_support/Database/MockConnection.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class MockConnection extends BaseConnection

public $database;

public $saveQueries = true;
public $lastQuery;

//--------------------------------------------------------------------

Expand Down Expand Up @@ -34,28 +34,20 @@ public function query(string $sql, $binds = null)

$startTime = microtime(true);

$this->lastQuery = $query;

// Run the query
if (false === ($this->resultID = $this->simpleQuery($query->getQuery())))
{
$query->setDuration($startTime, $startTime);

// @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);
Expand Down
23 changes: 11 additions & 12 deletions tests/system/Database/BaseConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
22 changes: 7 additions & 15 deletions tests/system/Database/Builder/InsertTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public function testThrowsExceptionOnNoValuesSet()
}

//--------------------------------------------------------------------

public function testInsertBatch()
{
$builder = $this->db->table('jobs');
Expand All @@ -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() ));
}

//--------------------------------------------------------------------
Expand Down
Loading

0 comments on commit 30063c9

Please sign in to comment.