Skip to content

Commit 929a803

Browse files
tommyknockertommyknocker
committed
refactor: remove unnecessary reflection from ExternalReferenceProcessingTrait
- Replace reflection with direct property access using property_exists - Protected properties are accessible in trait scope when used in classes - Simplify tests by creating ExternalReferenceProcessingTraitTestHelper class - Add comprehensive SqlFormatter tests for MySQL, MariaDB, and MSSQL - Fix dialect-specific quoteIdentifier and quoteTable test assertions
1 parent c4230e7 commit 929a803

12 files changed

Lines changed: 2807 additions & 44 deletions

‎src/query/traits/ExternalReferenceProcessingTrait.php‎

Lines changed: 32 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -42,54 +42,42 @@ protected function isTableInCurrentQuery(string $tableName): bool
4242
}
4343

4444
// Check JOIN tables if joinBuilder is available
45-
// Use reflection to access protected property if needed
46-
try {
47-
$reflection = new \ReflectionClass($this);
48-
if ($reflection->hasProperty('joinBuilder')) {
49-
$property = $reflection->getProperty('joinBuilder');
50-
$property->setAccessible(true);
51-
$joinBuilder = $property->getValue($this);
52-
if ($joinBuilder !== null) {
53-
$joins = $joinBuilder->getJoins();
54-
foreach ($joins as $join) {
55-
// Extract table name/alias from JOIN clause (e.g., "INNER JOIN [tenants] AS t" or "LEFT JOIN users u")
56-
// Handle quoted identifiers: [tenants] AS t, "tenants" AS "t", tenants AS t, tenants t
57-
// Pattern: JOIN [table] AS [alias] or JOIN "table" AS "alias" or JOIN table AS alias or JOIN table alias
58-
// Match groups: [1]=[table], [2]="table", [3]=table, [4]=[alias], [5]="alias", [6]=alias
59-
if (preg_match('/JOIN\s+(?:\[([^\]]+)\]|"([^"]+)"|([a-zA-Z_][a-zA-Z0-9_]*))\s+(?:AS\s+)?(?:\[([^\]]+)\]|"([^"]+)"|([a-zA-Z_][a-zA-Z0-9_]*))/i', $join, $matches)) {
60-
// Check all possible alias positions (matches[4] for [alias], matches[5] for "alias", matches[6] for alias)
61-
// Filter out empty strings - check in order: [5] (double quotes), [4] (square brackets), [6] (unquoted)
62-
$alias = null;
63-
foreach ([5, 4, 6] as $i) {
64-
if (isset($matches[$i]) && $matches[$i] !== '') {
65-
$alias = $matches[$i];
66-
break;
67-
}
68-
}
69-
if ($alias && $alias === $tableName) {
70-
return true;
71-
}
45+
// Since we're in the same class scope (trait is used in class), protected properties are accessible directly
46+
if (property_exists($this, 'joinBuilder') && isset($this->joinBuilder)) {
47+
$joins = $this->joinBuilder->getJoins();
48+
foreach ($joins as $join) {
49+
// Extract table name/alias from JOIN clause (e.g., "INNER JOIN [tenants] AS t" or "LEFT JOIN users u")
50+
// Handle quoted identifiers: [tenants] AS t, "tenants" AS "t", tenants AS t, tenants t
51+
// Pattern: JOIN [table] AS [alias] or JOIN "table" AS "alias" or JOIN table AS alias or JOIN table alias
52+
// Match groups: [1]=[table], [2]="table", [3]=table, [4]=[alias], [5]="alias", [6]=alias
53+
// The pattern must stop before "ON" keyword to avoid capturing it as alias
54+
// Allow any characters before JOIN (e.g., "= JOIN" or "INNER JOIN")
55+
// Use non-greedy match to stop at first "ON"
56+
if (preg_match('/JOIN\s+(?:\[([^\]]+)\]|"([^"]+)"|([a-zA-Z_][a-zA-Z0-9_]*))(?:\s+AS\s+(?:\[([^\]]+)\]|"([^"]+)"|([a-zA-Z_][a-zA-Z0-9_]*)))?\s+ON/i', $join, $matches)) {
57+
// First check table name (matches[1] for [table], matches[2] for "table", matches[3] for table)
58+
$table = null;
59+
foreach ([2, 1, 3] as $i) {
60+
if (isset($matches[$i]) && $matches[$i] !== '') {
61+
$table = $matches[$i];
62+
break;
7263
}
7364
}
74-
}
75-
}
76-
} catch (\ReflectionException $e) {
77-
// If reflection fails, fall back to property_exists check
78-
if (property_exists($this, 'joinBuilder') && isset($this->joinBuilder)) {
79-
$joins = $this->joinBuilder->getJoins();
80-
foreach ($joins as $join) {
81-
if (preg_match('/JOIN\s+(?:\[([^\]]+)\]|"([^"]+)"|([a-zA-Z_][a-zA-Z0-9_]*))\s+(?:AS\s+)?(?:\[([^\]]+)\]|"([^"]+)"|([a-zA-Z_][a-zA-Z0-9_]*))/i', $join, $matches)) {
82-
$alias = null;
83-
foreach ([5, 4, 6] as $i) {
84-
if (isset($matches[$i]) && $matches[$i] !== '') {
85-
$alias = $matches[$i];
86-
break;
87-
}
88-
}
89-
if ($alias && $alias === $tableName) {
90-
return true;
65+
if ($table && $table === $tableName) {
66+
return true;
67+
}
68+
69+
// Then check all possible alias positions (matches[4] for [alias], matches[5] for "alias", matches[6] for alias)
70+
// Filter out empty strings - check in order: [5] (double quotes), [4] (square brackets), [6] (unquoted)
71+
$alias = null;
72+
foreach ([5, 4, 6] as $i) {
73+
if (isset($matches[$i]) && $matches[$i] !== '') {
74+
$alias = $matches[$i];
75+
break;
9176
}
9277
}
78+
if ($alias && $alias === $tableName) {
79+
return true;
80+
}
9381
}
9482
}
9583
}

‎tests/mariadb/DialectTests.php‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,4 +257,91 @@ public function testDistinctOnThrowsExceptionOnMySQL(): void
257257
->distinctOn('email')
258258
->get();
259259
}
260+
261+
public function testQuoteIdentifier(): void
262+
{
263+
$connection = self::$db->connection;
264+
assert($connection !== null);
265+
$dialect = $connection->getDialect();
266+
267+
// Test simple identifier
268+
$this->assertEquals('`column_name`', $dialect->quoteIdentifier('column_name'));
269+
270+
// Test identifier with backticks (note: quoteIdentifier doesn't escape, quoteTable does)
271+
$quoted = $dialect->quoteIdentifier('test`column');
272+
$this->assertEquals('`test`column`', $quoted);
273+
274+
// Test RawValue (should return as-is)
275+
$rawValue = new \tommyknocker\pdodb\helpers\values\RawValue('COUNT(*)');
276+
$this->assertEquals('COUNT(*)', $dialect->quoteIdentifier($rawValue));
277+
278+
// Test identifier with numbers
279+
$this->assertEquals('`column123`', $dialect->quoteIdentifier('column123'));
280+
}
281+
282+
public function testQuoteTable(): void
283+
{
284+
$connection = self::$db->connection;
285+
assert($connection !== null);
286+
$dialect = $connection->getDialect();
287+
288+
// Test simple table name
289+
$this->assertEquals('`users`', $dialect->quoteTable('users'));
290+
291+
// Test schema-qualified table
292+
$quoted = $dialect->quoteTable('schema.users');
293+
$this->assertStringContainsString('`schema`', $quoted);
294+
$this->assertStringContainsString('`users`', $quoted);
295+
296+
// Test table with alias (MySQL quoteTable adds alias as-is after first space)
297+
$quoted = $dialect->quoteTable('users AS u');
298+
$this->assertStringContainsString('`users`', $quoted);
299+
$this->assertStringContainsString('AS u', $quoted);
300+
}
301+
302+
public function testBuildDescribeSql(): void
303+
{
304+
$connection = self::$db->connection;
305+
assert($connection !== null);
306+
$dialect = $connection->getDialect();
307+
308+
$sql = $dialect->buildDescribeSql('users');
309+
$this->assertStringContainsString('DESCRIBE', $sql);
310+
$this->assertStringContainsString('users', $sql);
311+
}
312+
313+
public function testBuildShowIndexesSql(): void
314+
{
315+
$connection = self::$db->connection;
316+
assert($connection !== null);
317+
$dialect = $connection->getDialect();
318+
319+
$sql = $dialect->buildShowIndexesSql('users');
320+
$this->assertStringContainsString('SHOW INDEX', $sql);
321+
$this->assertStringContainsString('users', $sql);
322+
}
323+
324+
public function testBuildShowForeignKeysSql(): void
325+
{
326+
$connection = self::$db->connection;
327+
assert($connection !== null);
328+
$dialect = $connection->getDialect();
329+
330+
$sql = $dialect->buildShowForeignKeysSql('users');
331+
$this->assertStringContainsString('SELECT', $sql);
332+
$this->assertStringContainsString('KEY_COLUMN_USAGE', $sql);
333+
$this->assertStringContainsString('users', $sql);
334+
}
335+
336+
public function testBuildShowConstraintsSql(): void
337+
{
338+
$connection = self::$db->connection;
339+
assert($connection !== null);
340+
$dialect = $connection->getDialect();
341+
342+
$sql = $dialect->buildShowConstraintsSql('users');
343+
$this->assertStringContainsString('SELECT', $sql);
344+
$this->assertStringContainsString('TABLE_CONSTRAINTS', $sql);
345+
$this->assertStringContainsString('users', $sql);
346+
}
260347
}

0 commit comments

Comments
 (0)