diff --git a/src/Phaseolies/Database/Entity/Builder.php b/src/Phaseolies/Database/Entity/Builder.php index 1b80e7a5..61438cfb 100644 --- a/src/Phaseolies/Database/Entity/Builder.php +++ b/src/Phaseolies/Database/Entity/Builder.php @@ -225,7 +225,7 @@ public function select(array|string ...$fields): self */ public function where($field, $operator = null, $value = null): self { - if (is_callable($field)) { + if ($this->isCallback($field)) { return $this->whereNested($field, 'AND'); } @@ -259,7 +259,7 @@ public function where($field, $operator = null, $value = null): self */ public function orWhere($field, $operator = null, $value = null): self { - if (is_callable($field)) { + if ($this->isCallback($field)) { return $this->whereNested($field, 'OR'); } @@ -744,7 +744,7 @@ private function escapeValue($value): string */ public function if($value, callable $callback, ?callable $default = null): self { - $payload = is_callable($value) ? $value() : $value; + $payload = $this->isCallback($value) ? $value() : $value; if ($payload === true || $this->hasValue($payload)) { $callback($this); @@ -1363,7 +1363,7 @@ public function load($relations, ?callable $callback = null): self foreach ($relations as $key => $value) { $relation = is_string($key) ? $key : $value; - $constraint = is_callable($value) ? $value : $callback; + $constraint = $this->isCallback($value) ? $value : $callback; if (str_contains($relation, '.')) { $this->loadNestedRelations($models, $relation, $constraint); @@ -1654,7 +1654,7 @@ public function embedCount($relations, ?callable $callback = null): self if (is_array($relations)) { foreach ($relations as $key => $value) { - if (is_callable($value)) { + if ($this->isCallback($value)) { $this->eagerLoad["count:{$key}"] = $value; } else { $this->eagerLoad["count:{$value}"] = null; @@ -1953,7 +1953,7 @@ public function embed($relations, ?callable $callback = null): self if (is_array($relations)) { foreach ($relations as $key => $value) { - if (is_callable($value)) { + if ($this->isCallback($value)) { // Format: ['comments' => function($q) { ... }] [$relationName, $columns] = $this->parseRelationWithColumns($key); $constraint = !empty($columns) diff --git a/src/Phaseolies/Database/Entity/Query/Builder.php b/src/Phaseolies/Database/Entity/Query/Builder.php index 0aa3f974..3dcd1449 100644 --- a/src/Phaseolies/Database/Entity/Query/Builder.php +++ b/src/Phaseolies/Database/Entity/Query/Builder.php @@ -123,7 +123,7 @@ public function select(array|string ...$fields): self */ public function where($field, $operator = null, $value = null): self { - if (is_callable($field)) { + if ($this->isCallback($field)) { return $this->whereNested($field, 'AND'); } @@ -157,7 +157,7 @@ public function where($field, $operator = null, $value = null): self */ public function orWhere($field, $operator = null, $value = null): self { - if (is_callable($field)) { + if ($this->isCallback($field)) { return $this->whereNested($field, 'OR'); } @@ -359,7 +359,7 @@ protected function getTableColumns(?string $table = null): array */ public function if($value, callable $callback, ?callable $default = null): self { - $payload = is_callable($value) ? $value() : $value; + $payload = $this->isCallback($value) ? $value() : $value; if ($payload === true || $this->hasValue($payload)) { $callback($this); diff --git a/src/Phaseolies/Database/Entity/Query/Grammar.php b/src/Phaseolies/Database/Entity/Query/Grammar.php index 457ad465..ff7464f5 100644 --- a/src/Phaseolies/Database/Entity/Query/Grammar.php +++ b/src/Phaseolies/Database/Entity/Query/Grammar.php @@ -28,6 +28,20 @@ protected function assertValidIdentifier(string $identifier): void } } + /** + * Determine if a value is a callback (closure, invokable or array callable). + * + * Strings are never treated as callbacks: names like "date" or "time" + * pass is_callable() but are column names or values here. + * + * @param mixed $value + * @return bool + */ + protected function isCallback(mixed $value): bool + { + return !is_string($value) && is_callable($value); + } + /** * Get the current driver * diff --git a/tests/Builder/StringArgumentNotCallableTest.php b/tests/Builder/StringArgumentNotCallableTest.php new file mode 100644 index 00000000..4846a736 --- /dev/null +++ b/tests/Builder/StringArgumentNotCallableTest.php @@ -0,0 +1,115 @@ +createMock(PDO::class); + $pdo->method('getAttribute')->willReturn('mysql'); + + return new Builder($pdo, 'users', StringArgumentModelStub::class, 15); + } + + private function queryBuilder(): QueryBuilder + { + $pdo = $this->createMock(PDO::class); + $pdo->method('getAttribute')->willReturn('mysql'); + + return new QueryBuilder($pdo, 'users'); + } + + public static function functionNamedColumns(): array + { + return [['date'], ['time'], ['key'], ['max'], ['min'], ['file'], ['end'], ['range']]; + } + + #[DataProvider('functionNamedColumns')] + public function testWhereOnColumnNamedLikePhpFunctionIsACondition(string $column): void + { + $sql = $this->entityBuilder()->where($column, 'x')->orWhere($column, '!=', 'y')->toSql(); + + $this->assertStringContainsString("{$column} = ?", $sql); + $this->assertStringContainsString("OR {$column} != ?", $sql); + } + + #[DataProvider('functionNamedColumns')] + public function testQueryBuilderWhereOnColumnNamedLikePhpFunctionIsACondition(string $column): void + { + $sql = $this->queryBuilder()->where($column, 'x')->orWhere($column, 'y')->toSql(); + + $this->assertStringContainsString("{$column} = ?", $sql); + $this->assertStringContainsString("OR {$column} = ?", $sql); + } + + public function testWhereStillAcceptsClosuresForNesting(): void + { + $sql = $this->entityBuilder() + ->where(fn($q) => $q->where('a', 1)->orWhere('b', 2)) + ->toSql(); + + $this->assertStringContainsString('(a = ? OR b = ?)', $sql); + } + + public function testWhereStillAcceptsInvokableObjectsForNesting(): void + { + $nested = new class { + public function __invoke($q): void + { + $q->where('a', 1); + } + }; + + $this->assertStringContainsString('(a = ?)', $this->queryBuilder()->where($nested)->toSql()); + } + + public function testIfDoesNotInvokeStringValuesAsFunctions(): void + { + $called = 0; + $callback = function () use (&$called) { + $called++; + }; + + // "phpinfo" would print the server configuration if invoked. + ob_start(); + $this->entityBuilder()->if('phpinfo', $callback); + $this->queryBuilder()->if('phpinfo', $callback); + $output = ob_get_clean(); + + $this->assertSame('', $output); + $this->assertSame(2, $called); + } + + public function testIfStillEvaluatesClosureConditions(): void + { + $called = false; + + $this->entityBuilder()->if(fn() => false, function () use (&$called) { + $called = true; + }); + + $this->assertFalse($called); + } +} + +class StringArgumentModelStub +{ + public function usesTimestamps(): bool + { + return false; + } +}