Skip to content

Commit

Permalink
NEW Add a trace comment for queries in dev mode
Browse files Browse the repository at this point in the history
  • Loading branch information
@GuySartorelli
GuySartorelli committed Feb 7, 2024
1 parent 5c753d5 commit f48e21c
Show file tree
Hide file tree
Showing 8 changed files with 338 additions and 67 deletions.
16 changes: 16 additions & 0 deletions src/Dev/SapphireTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Core\Injector\InjectorLoader;
use SilverStripe\Core\Kernel;
use SilverStripe\Core\Manifest\ClassLoader;
use SilverStripe\Core\Manifest\ModuleResourceLoader;
use SilverStripe\Dev\Constraint\SSListContains;
Expand Down Expand Up @@ -1231,4 +1232,19 @@ protected function mockSleep(int $seconds): DBDatetime

return $now;
}

/**
* Execute a callback in a given environment type and return the result.
*/
protected function executeInEnvironmentType(string $env, callable $callback): mixed
{
$kernel = Injector::inst()->get(Kernel::class);
$origMode = $kernel->getEnvironment();
$kernel->setEnvironment($env);
try {
return $callback();
} finally {
$kernel->setEnvironment($origMode);
}
}
}
126 changes: 126 additions & 0 deletions src/ORM/Connect/DBQueryBuilder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,15 @@
namespace SilverStripe\ORM\Connect;

use InvalidArgumentException;
use SilverStripe\Control\Director;
use SilverStripe\Core\Config\Configurable;
use SilverStripe\Core\Convert;
use SilverStripe\ORM\DataList;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\DataQuery;
use SilverStripe\ORM\DB;
use SilverStripe\ORM\ListDecorator;
use SilverStripe\ORM\Map;
use SilverStripe\ORM\Queries\SQLExpression;
use SilverStripe\ORM\Queries\SQLSelect;
use SilverStripe\ORM\Queries\SQLDelete;
Expand All @@ -16,6 +24,12 @@
*/
class DBQueryBuilder
{
use Configurable;

/**
* If true, a comment is added to each query indicating where that query's execution originated.
*/
private static bool $trace_query_origin = false;

/**
* Determines the line separator to use.
Expand Down Expand Up @@ -57,9 +71,121 @@ public function buildSQL(SQLExpression $query, &$parameters)
"Not implemented: query generation for type " . get_class($query)
);
}

if (static::config()->get('trace_query_origin')) {
$sql = $this->buildTraceComment() . $sql;
}

return $sql;
}

/**
* Builds an SQL comment indicating where the query was executed from.
*/
protected function buildTraceComment(): string
{
$comment = '/* ';

$ignoreBaseClasses = [
self::class,
DataQuery::class,
SQLExpression::class,
DB::class,
Database::class,
DBConnector::class,
DBSchemaManager::class,
TransactionManager::class,
ListDecorator::class,
Map::class,
];
$ignoreMethods = [
DataList::class => [
// these are used in almost all DataList query executions
'executeQuery',
'getFinalisedQuery',
'getIterator',
// these call a method on DataList (e.g. $this->toNestedArray())
'debug',
'setByIDList',
],
DataObject::class => [
'get_one',
'get_by_id',
]
];

$line = null;
$file = null;
$class = null;
$function = null;

// Don't include arguments in the trace (since we don't need them), and only go back 15 levels.
// Anything further than that and we've probably over-abstracted things.
$trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 15);
foreach ($trace as $i => $item) {
// We need to be able to look ahead one item in the trace, because the class/function values
// are talking about what is being *called* on this line, not the function this line lives in.
if (!isset($trace[$i+1])) {
return '/* Could not identify source of query */' . $this->getSeparator();
}
$caller = [
'file' => $item['file'] ?? null,
'line' => $item['line'] ?? null,
'class' => $trace[$i + 1]['class'] ?? null,
'function' => $trace[$i + 1]['function'] ?? null,
];

if ($caller['class'] !== null) {
// Don't report internal ORM operations for any of these classes
foreach ($ignoreBaseClasses as $ignore) {
if (is_a($caller['class'], $ignore, true)) {
// skip for both loops
continue 2;
}
}
if ($caller['function'] !== null) {
// Don't report internal ORM operations for any of these methods
foreach ($ignoreMethods as $class => $methodsToIgnore) {
if (is_a($caller['class'], $class, true) && in_array($caller['function'], $methodsToIgnore)) {
// skip for both loops
continue 2;
}
}
// Don't report internal ORM operations inside DataList which themselves directly call methods on DataQuery
if ($caller['class'] === DataList::class && is_a($item['class'] ?? '', DataQuery::class, true)) {
continue;
}
// Don't report internal ORM operations inside DataList for eagerloading or for any methods that iterate over the list itself
if ($caller['class'] === DataList::class &&
(str_starts_with($caller['function'], 'fetchEagerLoad')
|| is_a($item['class'] ?? '', DataList::class, true) && in_array($item['function'], $ignoreMethods[DataList::class]))
) {
continue;
}
}
}

// Get the relevant trace information if it's available
$file = $caller['file'];
$line = $caller['line'];
$class = $caller['class'];
$function = $caller['function'];
break;
}

// Indicate where the query was executed from, if we have that information.
if ($line && $file) {
$comment .= "Query executed from $file line $line";
} elseif ($class && $function) {
$comment .= "Query executed from {$class}::{$function}()";
} else {
$comment .= 'Could not identify source of query';
}

$comment .= ' */' . $this->getSeparator();
return $comment;
}

/**
* Builds a query from a SQLSelect expression
*
Expand Down
2 changes: 2 additions & 0 deletions tests/php/ORM/DataListEagerLoadingTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ private function stopCountingSelectQueries(): int
{
$s = ob_get_clean();
$s = preg_replace('/.*__START_ITERATE__/s', '', $s);
// Remove multi-line SQL comments and the whitespace after them
$s = preg_replace('#/\*[^\*]*\*/\s*#', '', $s);
$this->resetShowQueries();
return substr_count($s, ': SELECT');
}
Expand Down
24 changes: 21 additions & 3 deletions tests/php/ORM/DataListTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
use Exception;
use InvalidArgumentException;
use SilverStripe\Core\Convert;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Core\Injector\InjectorNotFoundException;
use SilverStripe\Core\Kernel;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\ORM\Connect\MySQLiConnector;
use SilverStripe\ORM\DataList;
Expand Down Expand Up @@ -301,7 +303,11 @@ public function testSql()
. $db->quoteString(DataObjectTest\TeamComment::class)
. ' END AS "RecordClassName" FROM "DataObjectTest_TeamComment"'
. ' ORDER BY "DataObjectTest_TeamComment"."Name" ASC';
$this->assertSQLEquals($expected, $list->sql($parameters));

// Must be tested in live mode, since dev mode adds SQL comments to the SQL output.
$sql = $this->executeInEnvironmentType('live', fn () => $list->sql());

$this->assertSQLEquals($expected, $sql);
}

public function provideJoin()
Expand Down Expand Up @@ -348,7 +354,13 @@ public function testJoin(string $joinMethod, string $joinType)
. '"DataObjectTest_TeamComment"."TeamID"'
. ' ORDER BY "DataObjectTest_TeamComment"."Name" ASC';

$this->assertSQLEquals($expected, $list->sql($parameters));
// Must be tested in live mode, since dev mode adds SQL comments to the SQL output.
$parameters = [];
$sql = $this->executeInEnvironmentType('live', function () use (&$parameters, $list) {
return $list->sql($parameters);
});

$this->assertSQLEquals($expected, $sql);
$this->assertEmpty($parameters);
}

Expand Down Expand Up @@ -382,7 +394,13 @@ public function testJoinParameterised(string $joinMethod, string $joinType)
. 'AND "DataObjectTest_Team"."Title" LIKE ?'
. ' ORDER BY "DataObjectTest_TeamComment"."Name" ASC';

$this->assertSQLEquals($expected, $list->sql($parameters));
// Must be tested in live mode, since dev mode adds SQL comments to the SQL output.
$parameters = [];
$sql = $this->executeInEnvironmentType('live', function () use (&$parameters, $list) {
return $list->sql($parameters);
});

$this->assertSQLEquals($expected, $sql);
$this->assertEquals(['Team%'], $parameters);
}

Expand Down
25 changes: 21 additions & 4 deletions tests/php/ORM/DataObjectLazyLoadingTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace SilverStripe\ORM\Tests;

use SilverStripe\Core\Injector\Injector;
use SilverStripe\ORM\DB;
use SilverStripe\ORM\DataList;
use SilverStripe\ORM\DataObject;
Expand Down Expand Up @@ -29,7 +30,11 @@ public function testQueriedColumnsID()
$playerList = new DataList(SubTeam::class);
$playerList = $playerList->setQueriedColumns(['ID']);
$expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."LastEdited", ' . '"DataObjectTest_Team"."Created", "DataObjectTest_Team"."ID", CASE WHEN ' . '"DataObjectTest_Team"."ClassName" IS NOT NULL THEN "DataObjectTest_Team"."ClassName" ELSE ' . $db->quoteString(Team::class) . ' END AS "RecordClassName", "DataObjectTest_Team"."Title" ' . 'FROM "DataObjectTest_Team" ' . 'LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = "DataObjectTest_Team"."ID" ' . 'WHERE ("DataObjectTest_Team"."ClassName" IN (?))' . ' ORDER BY "DataObjectTest_Team"."Title" ASC';
$this->assertSQLEquals($expected, $playerList->sql($parameters));

// Must be tested in live mode, since dev mode adds SQL comments to the SQL output.
$sql = $this->executeInEnvironmentType('live', fn () => $playerList->sql());

$this->assertSQLEquals($expected, $sql);
}

public function testQueriedColumnsFromBaseTableAndSubTable()
Expand All @@ -38,7 +43,11 @@ public function testQueriedColumnsFromBaseTableAndSubTable()
$playerList = new DataList(SubTeam::class);
$playerList = $playerList->setQueriedColumns(['Title', 'SubclassDatabaseField']);
$expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."LastEdited", ' . '"DataObjectTest_Team"."Created", "DataObjectTest_Team"."Title", ' . '"DataObjectTest_SubTeam"."SubclassDatabaseField", "DataObjectTest_Team"."ID", CASE WHEN ' . '"DataObjectTest_Team"."ClassName" IS NOT NULL THEN "DataObjectTest_Team"."ClassName" ELSE ' . $db->quoteString(Team::class) . ' END AS "RecordClassName" FROM "DataObjectTest_Team" ' . 'LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = "DataObjectTest_Team"."ID" WHERE ' . '("DataObjectTest_Team"."ClassName" IN (?)) ' . 'ORDER BY "DataObjectTest_Team"."Title" ASC';
$this->assertSQLEquals($expected, $playerList->sql($parameters));

// Must be tested in live mode, since dev mode adds SQL comments to the SQL output.
$sql = $this->executeInEnvironmentType('live', fn () => $playerList->sql());

$this->assertSQLEquals($expected, $sql);
}

public function testQueriedColumnsFromBaseTable()
Expand All @@ -47,7 +56,11 @@ public function testQueriedColumnsFromBaseTable()
$playerList = new DataList(SubTeam::class);
$playerList = $playerList->setQueriedColumns(['Title']);
$expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."LastEdited", ' . '"DataObjectTest_Team"."Created", "DataObjectTest_Team"."Title", "DataObjectTest_Team"."ID", ' . 'CASE WHEN "DataObjectTest_Team"."ClassName" IS NOT NULL THEN "DataObjectTest_Team"."ClassName" ELSE ' . $db->quoteString(Team::class) . ' END AS "RecordClassName" FROM "DataObjectTest_Team" ' . 'LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = "DataObjectTest_Team"."ID" WHERE ' . '("DataObjectTest_Team"."ClassName" IN (?)) ' . 'ORDER BY "DataObjectTest_Team"."Title" ASC';
$this->assertSQLEquals($expected, $playerList->sql($parameters));

// Must be tested in live mode, since dev mode adds SQL comments to the SQL output.
$sql = $this->executeInEnvironmentType('live', fn () => $playerList->sql());

$this->assertSQLEquals($expected, $sql);
}

public function testQueriedColumnsFromSubTable()
Expand All @@ -56,7 +69,11 @@ public function testQueriedColumnsFromSubTable()
$playerList = new DataList(SubTeam::class);
$playerList = $playerList->setQueriedColumns(['SubclassDatabaseField']);
$expected = 'SELECT DISTINCT "DataObjectTest_Team"."ClassName", "DataObjectTest_Team"."LastEdited", ' . '"DataObjectTest_Team"."Created", "DataObjectTest_SubTeam"."SubclassDatabaseField", ' . '"DataObjectTest_Team"."ID", CASE WHEN "DataObjectTest_Team"."ClassName" IS NOT NULL THEN ' . '"DataObjectTest_Team"."ClassName" ELSE ' . $db->quoteString(Team::class) . ' END ' . 'AS "RecordClassName", "DataObjectTest_Team"."Title" ' . 'FROM "DataObjectTest_Team" LEFT JOIN "DataObjectTest_SubTeam" ON "DataObjectTest_SubTeam"."ID" = ' . '"DataObjectTest_Team"."ID" WHERE ("DataObjectTest_Team"."ClassName" IN (?)) ' . 'ORDER BY "DataObjectTest_Team"."Title" ASC';
$this->assertSQLEquals($expected, $playerList->sql($parameters));

// Must be tested in live mode, since dev mode adds SQL comments to the SQL output.
$sql = $this->executeInEnvironmentType('live', fn () => $playerList->sql());

$this->assertSQLEquals($expected, $sql);
}

public function testNoSpecificColumnNamesBaseDataObjectQuery()
Expand Down
7 changes: 6 additions & 1 deletion tests/php/ORM/ManyManyListTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
use SilverStripe\ORM\FieldType\DBMoney;
use SilverStripe\ORM\ManyManyList;
use SilverStripe\Core\Convert;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Core\Kernel;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\Tests\DataObjectTest\Player;
Expand Down Expand Up @@ -437,7 +439,10 @@ public function testAppendExtraFieldsToQuery()
. ' "ManyManyListTest_ExtraFields_Clients"."ManyManyListTest_ExtraFieldsID" ='
. ' "ManyManyListTest_ExtraFields"."ID"';

$this->assertSQLEquals($expected, $list->sql($parameters));
// Must be tested in live mode, since dev mode adds SQL comments to the SQL output.
$sql = $this->executeInEnvironmentType('live', fn () => $list->sql());

$this->assertSQLEquals($expected, $sql);
}

/**
Expand Down
Loading

0 comments on commit f48e21c

Please sign in to comment.