diff --git a/README.md b/README.md index cfc82b8..2e86e84 100644 --- a/README.md +++ b/README.md @@ -306,6 +306,14 @@ $logs = $query->getResult(); The generated SQL still goes through `$wpdb->prepare()`. +Keep dynamic values out of query strings. The builder validates mapped field +paths and prepares parameter values, but it cannot make interpolated strings +safe. Put request data in `setParameter()` or `setParameters()` instead of +concatenating it into `where()`, `andWhere()`, `orWhere()`, `having()`, +`andHaving()`, `join()` conditions, or DQL strings. `orderBy()` only accepts +mapped field paths; choose from an allow-list before passing user-controlled +sort fields. + Supported query builder features: - `select()` diff --git a/composer.json b/composer.json index ea150ea..7764bbf 100644 --- a/composer.json +++ b/composer.json @@ -11,16 +11,14 @@ } ], "require": { - "php": "^8.4", + "php": "^8.5", "ext-json": "*", "symfony/console": "^8.0" }, "require-dev": { - "phpstan/phpstan": "^2.1", - "phpunit/phpunit": "^10.5", - "sympress/coding-standards": "dev-main", "sympress/kernel": "dev-main", - "sympress/migration": "dev-main" + "sympress/migration": "dev-main", + "sympress/qa": "dev-main" }, "autoload": { "psr-4": { @@ -41,19 +39,22 @@ "scripts": { "cs": [ "Composer\\Config::disableProcessTimeout", - "phpcs --standard=phpcs.xml.dist" + "qa cs" ], "cs:fix": [ "Composer\\Config::disableProcessTimeout", - "phpcbf --standard=phpcs.xml.dist" + "qa cs:fix" ], "static-analysis": [ "Composer\\Config::disableProcessTimeout", - "phpstan analyse --memory-limit=1G --no-progress -c phpstan.neon.dist" + "qa static-analysis" ], "tests": [ "Composer\\Config::disableProcessTimeout", - "phpunit --configuration phpunit.xml.dist --no-coverage" + "qa tests" + ], + "test": [ + "@tests" ], "qa": [ "@cs", @@ -66,7 +67,8 @@ "optimize-autoloader": true, "preferred-install": "dist", "allow-plugins": { - "dealerdirect/phpcodesniffer-composer-installer": true + "dealerdirect/phpcodesniffer-composer-installer": true, + "phpstan/extension-installer": true } }, "repositories": [ diff --git a/docs/api-and-architecture.md b/docs/api-and-architecture.md index 5e76323..44f3861 100644 --- a/docs/api-and-architecture.md +++ b/docs/api-and-architecture.md @@ -15,7 +15,7 @@ The SymPress ORM package provides Doctrine-inspired persistence primitives for W ### EntityManager -`SymPress\Orm\EntityManager` is the primary application API. It owns the unit of work, metadata access, repositories, query creation, events, optional second-level cache, and a normalized database connection. +`SymPress\Orm\EntityManager` is the primary application API. It owns the unit of work, metadata access, repositories, query creation, events, and optional second-level cache. Database connection normalization is delegated to `Dbal\ConnectionProvider`, while custom DQL functions and SQL output walkers are kept in `Query\DqlExtensionRegistry`. Typical usage: @@ -29,7 +29,7 @@ $entityManager->persist($log); $entityManager->flush(); ``` -When no `wpdb` instance is passed, the ORM resolves the global `$wpdb` lazily through `WpdbConnection`. +When no `wpdb` instance is passed, the ORM resolves the global `$wpdb` lazily through `ConnectionProvider` and `WpdbConnection`. ### UnitOfWork @@ -96,6 +96,8 @@ Supported DQL is a focused subset: `ORDER BY` accepts mapped field paths only, for example `l.createdAt`. Raw SQL fragments are rejected. +Do not interpolate request values into `where()`, `having()`, join conditions, or DQL strings. Those APIs accept expression snippets so mapped fields can be compiled, but dynamic values are only safe when bound through named or positional parameters. User-controlled sort choices should be mapped through an allow-list before calling `orderBy()` or `addOrderBy()`. + ### SchemaTool `SchemaTool` produces deterministic SQL for create, update, drop, validation, and schema hashes. It is used directly by console commands and by the migration bridge. diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 110adbf..6b65a4f 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -8,7 +8,7 @@ - + src diff --git a/src/Dbal/ConnectionProvider.php b/src/Dbal/ConnectionProvider.php new file mode 100644 index 0000000..1c445bd --- /dev/null +++ b/src/Dbal/ConnectionProvider.php @@ -0,0 +1,30 @@ +connection ??= new WpdbConnection(); + } +} diff --git a/src/Dbal/WpdbConnection.php b/src/Dbal/WpdbConnection.php index 31d1ad0..d765c3b 100644 --- a/src/Dbal/WpdbConnection.php +++ b/src/Dbal/WpdbConnection.php @@ -39,7 +39,15 @@ public function prepare(string $sql, mixed ...$parameters): string return $sql; } - return $this->database()->prepare($sql, ...$parameters); + // SQL is generated by the ORM/query builder layer before it reaches this WordPress adapter. + // @phpstan-ignore argument.type + $prepared = $this->database()->prepare($sql, ...$parameters); + + if (!is_string($prepared)) { + throw new \RuntimeException('Unable to prepare SQL statement.'); + } + + return $prepared; } public function fetchOne(string $sql, mixed ...$parameters): mixed @@ -50,7 +58,7 @@ public function fetchOne(string $sql, mixed ...$parameters): mixed public function fetchAllAssociative(string $sql, mixed ...$parameters): array { /** @var list> $rows */ - $rows = $this->database()->get_results($this->prepare($sql, ...$parameters), $this->arrayOutput()); + $rows = $this->database()->get_results($this->prepare($sql, ...$parameters), ARRAY_A); return $rows; } @@ -75,7 +83,7 @@ public function executeStatement(string $sql, mixed ...$parameters): bool|int return $this->database()->query($this->prepare($sql, ...$parameters)); } - public function lastInsertId(): int|string + public function lastInsertId(): int { return $this->database()->insert_id; } @@ -132,9 +140,4 @@ private function database(): \wpdb throw new \RuntimeException('Global $wpdb is not available.'); } - - private function arrayOutput(): string - { - return defined('ARRAY_A') ? constant('ARRAY_A') : 'ARRAY_A'; - } } diff --git a/src/EntityManager.php b/src/EntityManager.php index 70b5eef..7e73c1e 100644 --- a/src/EntityManager.php +++ b/src/EntityManager.php @@ -7,8 +7,8 @@ use SymPress\Orm\Collection\Collection; use SymPress\Orm\Collection\PersistentCollection; use SymPress\Orm\Cache\CacheInterface; +use SymPress\Orm\Dbal\ConnectionProvider; use SymPress\Orm\Dbal\ConnectionInterface; -use SymPress\Orm\Dbal\WpdbConnection; use SymPress\Orm\Exception\EntityManagerClosedException; use SymPress\Orm\Exception\OptimisticLockException; use SymPress\Orm\Exception\PessimisticLockException; @@ -27,6 +27,7 @@ use SymPress\Orm\Metadata\MetadataFactory; use SymPress\Orm\Query\CompiledQuery; use SymPress\Orm\Query\DqlCompiler; +use SymPress\Orm\Query\DqlExtensionRegistry; use SymPress\Orm\Query\Query; use SymPress\Orm\Query\QueryBuilder; @@ -37,7 +38,7 @@ final class EntityManager private UnitOfWork $unitOfWork; - private ?ConnectionInterface $connection; + private ConnectionProvider $connections; private EventManager $events; @@ -49,11 +50,7 @@ final class EntityManager /** @var array */ private array $entityListenerInstances = []; - /** @var array */ - private array $dqlFunctions = []; - - /** @var list */ - private array $outputWalkers = []; + private DqlExtensionRegistry $dqlExtensions; /** @var array */ private array $cacheRegionVersions = []; @@ -68,12 +65,15 @@ public function __construct( ?UnitOfWork $unitOfWork = null, ?EventManager $events = null, ?CacheInterface $secondLevelCache = null, + ?ConnectionProvider $connectionProvider = null, + ?DqlExtensionRegistry $dqlExtensions = null, ) { $this->unitOfWork = $unitOfWork ?? new UnitOfWork(); - $this->connection = $this->normalizeConnection($database); + $this->connections = $connectionProvider ?? ConnectionProvider::fromDatabase($database); $this->events = $events ?? new EventManager(); $this->secondLevelCache = $secondLevelCache; + $this->dqlExtensions = $dqlExtensions ?? new DqlExtensionRegistry(); } /** @param class-string $entityClass */ @@ -453,9 +453,9 @@ public function createQueryBuilder(): QueryBuilder public function createNativeQuery(CompiledQuery $query): Query { - if ($this->outputWalkers !== []) { + if ($this->dqlExtensions->hasOutputWalkers()) { $query = new CompiledQuery( - $this->applyOutputWalkers($query->sql), + $this->dqlExtensions->applyOutputWalkers($query->sql), $query->parameters, $query->resultMetadata, ); @@ -509,42 +509,22 @@ public function getConnection(): ConnectionInterface public function registerDqlFunction(string $name, callable $compiler): void { - $this->dqlFunctions[strtoupper($name)] = $compiler; + $this->dqlExtensions->registerFunction($name, $compiler); } public function addOutputWalker(callable $walker): void { - $this->outputWalkers[] = $walker; + $this->dqlExtensions->addOutputWalker($walker); } public function compileDqlFunctions(string $expression): string { - if ($this->dqlFunctions === []) { - return $expression; - } - - return preg_replace_callback( - '/\b([A-Z_][A-Z0-9_]*)\s*\(([^()]*)\)/i', - function (array $matches): string { - $compiler = $this->dqlFunctions[strtoupper($matches[1])] ?? null; - - if (!is_callable($compiler)) { - return $matches[0]; - } - - return $compiler(trim($matches[2])); - }, - $expression, - ) ?? $expression; + return $this->dqlExtensions->compileFunctions($expression); } public function applyOutputWalkers(string $sql): string { - foreach ($this->outputWalkers as $walker) { - $sql = $walker($sql); - } - - return $sql; + return $this->dqlExtensions->applyOutputWalkers($sql); } /** @param class-string|null $className */ @@ -683,7 +663,6 @@ private function updateEntity(object $entity, ClassMetadata $metadata, array $ch if ($version instanceof ColumnMetadata) { $where[$version->columnName] = $original[$version->columnName] ?? null; $nextVersion = (int) ($original[$version->columnName] ?? 0) + 1; - $this->hydrator->assign($entity, $version->propertyName, $nextVersion); $changes[$version->columnName] = $nextVersion; } @@ -704,6 +683,10 @@ private function updateEntity(object $entity, ClassMetadata $metadata, array $ch if ($version instanceof ColumnMetadata && $result === 0) { throw OptimisticLockException::lockFailed($entity); } + + if ($version instanceof ColumnMetadata) { + $this->hydrator->assign($entity, $version->propertyName, $nextVersion); + } } private function deleteEntity(object $entity, ClassMetadata $metadata): void @@ -1691,24 +1674,7 @@ private function hydrateRows(ClassMetadata $metadata, array $rows): array private function connection(): ConnectionInterface { - if ($this->connection instanceof ConnectionInterface) { - return $this->connection; - } - - return $this->connection = new WpdbConnection(); - } - - private function normalizeConnection(ConnectionInterface|\wpdb|null $database): ?ConnectionInterface - { - if ($database instanceof ConnectionInterface) { - return $database; - } - - if ($database instanceof \wpdb) { - return new WpdbConnection($database); - } - - return null; + return $this->connections->connection(); } /** @param array $parameters */ diff --git a/src/Query/DqlExtensionRegistry.php b/src/Query/DqlExtensionRegistry.php new file mode 100644 index 0000000..fe19be6 --- /dev/null +++ b/src/Query/DqlExtensionRegistry.php @@ -0,0 +1,59 @@ + */ + private array $functions = []; + + /** @var list */ + private array $outputWalkers = []; + + public function registerFunction(string $name, callable $compiler): void + { + $this->functions[strtoupper($name)] = $compiler; + } + + public function addOutputWalker(callable $walker): void + { + $this->outputWalkers[] = $walker; + } + + public function hasOutputWalkers(): bool + { + return $this->outputWalkers !== []; + } + + public function compileFunctions(string $expression): string + { + if ($this->functions === []) { + return $expression; + } + + return preg_replace_callback( + '/\b([A-Z_][A-Z0-9_]*)\s*\(([^()]*)\)/i', + function (array $matches): string { + $compiler = $this->functions[strtoupper($matches[1])] ?? null; + + if (!is_callable($compiler)) { + return $matches[0]; + } + + return $compiler(trim($matches[2])); + }, + $expression, + ) ?? $expression; + } + + public function applyOutputWalkers(string $sql): string + { + foreach ($this->outputWalkers as $walker) { + $sql = $walker($sql); + } + + return $sql; + } +} diff --git a/tests/Unit/DbalTest.php b/tests/Unit/DbalTest.php index 24fd1b6..faa6898 100644 --- a/tests/Unit/DbalTest.php +++ b/tests/Unit/DbalTest.php @@ -5,6 +5,7 @@ namespace SymPress\Orm\Tests\Unit; use PHPUnit\Framework\TestCase; +use SymPress\Orm\Dbal\ConnectionProvider; use SymPress\Orm\Dbal\WordPressSqlPlatform; use SymPress\Orm\Dbal\WpdbConnection; @@ -59,4 +60,27 @@ public function testWpdbConnectionFallsBackToGlobalWpdb(): void } } } + + public function testConnectionProviderNormalizesWpdbAndResolvesGlobalConnectionLazily(): void + { + $database = new \wpdb(); + $database->prefix = 'custom_'; + + self::assertSame('custom_', ConnectionProvider::fromDatabase($database)->connection()->tablePrefix()); + + $previousDatabase = $GLOBALS['wpdb'] ?? null; + $globalDatabase = new \wpdb(); + $globalDatabase->prefix = 'global_'; + $GLOBALS['wpdb'] = $globalDatabase; + + try { + self::assertSame('global_', ConnectionProvider::fromDatabase(null)->connection()->tablePrefix()); + } finally { + if ($previousDatabase instanceof \wpdb) { + $GLOBALS['wpdb'] = $previousDatabase; + } else { + unset($GLOBALS['wpdb']); + } + } + } } diff --git a/tests/Unit/EntityManagerTest.php b/tests/Unit/EntityManagerTest.php index 1bc523e..1502525 100644 --- a/tests/Unit/EntityManagerTest.php +++ b/tests/Unit/EntityManagerTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\TestCase; use SymPress\Orm\Cache\ArrayCache; +use SymPress\Orm\Collection\PersistentCollection; use SymPress\Orm\EntityHydrator; use SymPress\Orm\EntityManager; use SymPress\Orm\EntityState; @@ -154,9 +155,99 @@ public function testVersionedUpdateThrowsOptimisticLockExceptionWhenNoRowWasUpda $entityManager->persist($log); $log->status = 'sent'; - $this->expectException(OptimisticLockException::class); + try { + $entityManager->flush(); + self::fail('Expected an optimistic lock failure.'); + } catch (OptimisticLockException) { + self::assertFalse($entityManager->isOpen()); + self::assertSame(1, $log->version); + self::assertSame(['START TRANSACTION', 'ROLLBACK'], $database->queries); + } + } + + public function testVersionedUpdateIncrementsVersionAfterSuccessfulFlush(): void + { + $metadataFactory = new MetadataFactory(); + $registry = new EntityClassRegistry($metadataFactory, classes: [VersionedEmailLog::class]); + $database = new \wpdb(); + $database->countResult = 1; + $entityManager = new EntityManager($metadataFactory, $registry, new EntityHydrator(), $database); + $log = new VersionedEmailLog('log-1', 'queued', 1); + + $entityManager->persist($log); + $log->status = 'sent'; + $entityManager->flush(); + + self::assertSame(2, $log->version); + self::assertSame(['status' => 'sent', 'version' => 2], $database->updated[0]['data']); + self::assertSame(['id' => 'log-1', 'version' => 1], $database->updated[0]['where']); + self::assertSame(['START TRANSACTION', 'COMMIT'], $database->queries); + } + + public function testFlushRollsBackAndClosesEntityManagerWhenBatchInsertFails(): void + { + $metadataFactory = new MetadataFactory(); + $registry = new EntityClassRegistry($metadataFactory, classes: [MutableEmailLog::class]); + $database = new class extends \wpdb { + private int $insertCalls = 0; + + /** @param array $data */ + public function insert(string $table, array $data): bool|int + { + $this->insertCalls++; + + if ($this->insertCalls === 2) { + return false; + } + return parent::insert($table, $data); + } + }; + $entityManager = new EntityManager($metadataFactory, $registry, new EntityHydrator(), $database); + + $entityManager->persist(new MutableEmailLog('log-1', new \DateTimeImmutable('2026-06-13 10:00:00'), 'queued')); + $entityManager->persist(new MutableEmailLog('log-2', new \DateTimeImmutable('2026-06-13 10:01:00'), 'queued')); + + try { + $entityManager->flush(); + self::fail('Expected the second insert to fail.'); + } catch (\RuntimeException $exception) { + self::assertStringContainsString('Failed to insert', $exception->getMessage()); + self::assertFalse($entityManager->isOpen()); + self::assertCount(1, $database->inserted); + self::assertSame(['START TRANSACTION', 'ROLLBACK'], $database->queries); + } + } + + public function testFlushPersistsEntityChangesAfterLazyCollectionLoad(): void + { + $metadataFactory = new MetadataFactory(); + $registry = new EntityClassRegistry($metadataFactory, classes: [CachedAuthor::class, CachedArticle::class]); + $database = new \wpdb(); + $entityManager = new EntityManager($metadataFactory, $registry, new EntityHydrator(), $database); + $database->resultRows = [ + ['id' => 'author-1', 'name' => 'Ada'], + ]; + + $author = $entityManager->find(CachedAuthor::class, 'author-1'); + + self::assertInstanceOf(CachedAuthor::class, $author); + self::assertInstanceOf(PersistentCollection::class, $author->articles); + self::assertFalse($author->articles->isInitialized()); + + $author->name = 'Grace'; + $database->resultRows = [ + ['id' => 'article-1', 'title' => 'First', 'author_id' => 'author-1'], + ]; + + $articles = $author->articles->toArray(); $entityManager->flush(); + + self::assertCount(1, $articles); + self::assertSame('article-1', $articles[0]->id); + self::assertSame(['name' => 'Grace'], $database->updated[0]['data']); + self::assertSame(['id' => 'author-1'], $database->updated[0]['where']); + self::assertSame([], $database->deleted); } public function testPessimisticLockUsesBackedEnum(): void