diff --git a/.github/workflows/phpstan.yml b/.github/workflows/phpstan.yml index a3c140f..1728c4e 100644 --- a/.github/workflows/phpstan.yml +++ b/.github/workflows/phpstan.yml @@ -7,7 +7,18 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - - uses: php-actions/composer@v6 + + - name: Check out Datastore prerequisite + uses: actions/checkout@v4 + with: + repository: phpnomad/datastore + ref: codex/ci-prerequisites-clean + path: .ci/datastore + + - name: Install dependencies with Datastore prerequisite + run: | + composer config repositories.datastore '{"type":"path","url":".ci/datastore","options":{"symlink":false,"versions":{"phpnomad/datastore":"2.0.99"}}}' + composer update phpnomad/datastore --with-dependencies --no-interaction --prefer-dist - name: PHPStan Static Analysis uses: php-actions/phpstan@v3 diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index 93c7b1a..7bd4fbb 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -9,9 +9,24 @@ jobs: steps: - uses: actions/checkout@v3 - - uses: php-actions/composer@v6 + - name: Check out Datastore prerequisite + uses: actions/checkout@v4 + with: + repository: phpnomad/datastore + ref: codex/ci-prerequisites-clean + path: .ci/datastore - - name: PHPUnit Tests - uses: php-actions/phpunit@v3 + - name: Set up PHP + uses: shivammathur/setup-php@v2 with: - configuration: phpunit.xml \ No newline at end of file + php-version: '8.2' + tools: composer + coverage: none + + - name: Install dependencies with Datastore prerequisite + run: | + composer config repositories.datastore '{"type":"path","url":".ci/datastore","options":{"symlink":false,"versions":{"phpnomad/datastore":"2.0.99"}}}' + composer update phpnomad/datastore --with-dependencies --no-interaction --prefer-dist + + - name: PHPUnit Tests + run: vendor/bin/phpunit --configuration phpunit.xml diff --git a/composer.json b/composer.json index dd909d2..519ca1d 100644 --- a/composer.json +++ b/composer.json @@ -29,6 +29,7 @@ "phpnomad/cache": "^1.0", "phpnomad/chrono": "^1.0", "phpnomad/datastore": "^2.0", + "phpnomad/event": "^1.0", "phpnomad/singleton": "^1.0", "phpnomad/logger": "^1.0" }, diff --git a/composer.lock b/composer.lock index 7e0ca1c..d798178 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "bd34040462bfd6ac5f170bd83fa1d6b5", + "content-hash": "638b813cd6561a46cabaad819171e33a", "packages": [ { "name": "phpnomad/cache", @@ -187,6 +187,46 @@ }, "time": "2025-01-13T11:51:33+00:00" }, + { + "name": "phpnomad/event", + "version": "1.0.1", + "source": { + "type": "git", + "url": "https://github.com/phpnomad/event.git", + "reference": "b418d8fc90af18b7cb37e118795f9dd92ffeb538" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/phpnomad/event/zipball/b418d8fc90af18b7cb37e118795f9dd92ffeb538", + "reference": "b418d8fc90af18b7cb37e118795f9dd92ffeb538", + "shasum": "" + }, + "require-dev": { + "phpnomad/tests": "^0.1.0 || ^0.3.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "PHPNomad\\Events\\": "lib/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Alex Standiford", + "email": "alex@standiford.us" + } + ], + "homepage": "https://github.com/phpnomad/core", + "support": { + "issues": "https://github.com/phpnomad/event/issues", + "source": "https://github.com/phpnomad/event/tree/1.0.1" + }, + "time": "2026-06-12T10:56:48+00:00" + }, { "name": "phpnomad/logger", "version": "1.0.0", diff --git a/lib/Abstracts/IdentifiableDatabaseDatastoreHandler.php b/lib/Abstracts/IdentifiableDatabaseDatastoreHandler.php index ccbf7ed..1788570 100644 --- a/lib/Abstracts/IdentifiableDatabaseDatastoreHandler.php +++ b/lib/Abstracts/IdentifiableDatabaseDatastoreHandler.php @@ -8,10 +8,11 @@ use PHPNomad\Datastore\Interfaces\DataModel; use PHPNomad\Datastore\Interfaces\Datastore; use PHPNomad\Datastore\Interfaces\DatastoreHasCounts; +use PHPNomad\Datastore\Interfaces\DatastoreHasIdentityQuery; use PHPNomad\Datastore\Interfaces\DatastoreHasPrimaryKey; use PHPNomad\Datastore\Interfaces\DatastoreHasWhere; -abstract class IdentifiableDatabaseDatastoreHandler implements Datastore, DatastoreHasPrimaryKey, DatastoreHasWhere, DatastoreHasCounts +abstract class IdentifiableDatabaseDatastoreHandler implements Datastore, DatastoreHasPrimaryKey, DatastoreHasWhere, DatastoreHasCounts, DatastoreHasIdentityQuery { use WithDatastoreHandlerMethods; @@ -45,4 +46,4 @@ public function update($id, array $attributes): void { $this->updateCompound(['id' => $id], $attributes); } -} \ No newline at end of file +} diff --git a/lib/Traits/WithDatastoreHandlerMethods.php b/lib/Traits/WithDatastoreHandlerMethods.php index dbd3ac6..75b862a 100644 --- a/lib/Traits/WithDatastoreHandlerMethods.php +++ b/lib/Traits/WithDatastoreHandlerMethods.php @@ -2,6 +2,7 @@ namespace PHPNomad\Database\Traits; +use InvalidArgumentException; use PHPNomad\Cache\Enums\Operation; use PHPNomad\Datastore\Events\RecordCreated; use PHPNomad\Datastore\Events\RecordDeleted; @@ -352,14 +353,20 @@ protected function hydrateItems(array $data): array * @param int|null $offset * @return array * @throws DatastoreErrorException + * @throws InvalidArgumentException */ public function findIds(array $conditions, ?int $limit = null, ?int $offset = null): array { + $this->validateIdentityQuery($conditions, $limit, $offset); + + $this->serviceProvider->clauseBuilder + ->reset() + ->useTable($this->table); $this->serviceProvider->queryBuilder + ->reset() ->from($this->table) ->select(...$this->table->getFieldsForIdentity()); - if ($limit) { $this->serviceProvider->queryBuilder->limit($limit); } @@ -373,6 +380,85 @@ public function findIds(array $conditions, ?int $limit = null, ?int $offset = nu return $this->serviceProvider->queryStrategy->query($this->serviceProvider->queryBuilder); } + /** + * Validate the optional identity-query contract without changing the behavior of other query methods. + * + * @param array $conditions + */ + protected function validateIdentityQuery(array $conditions, ?int $limit, ?int $offset): void + { + if ($limit !== null && $limit <= 0) { + throw new InvalidArgumentException('The identity-query limit must be greater than zero.'); + } + + if ($offset !== null && $offset < 0) { + throw new InvalidArgumentException('The identity-query offset must not be negative.'); + } + + if ($conditions === []) { + return; + } + + if (!$this->isIdentityQueryList($conditions)) { + throw new InvalidArgumentException('Identity-query conditions must be a list of groups.'); + } + + foreach ($conditions as $group) { + if (!is_array($group) + || !array_key_exists('clauses', $group) + || !is_array($group['clauses']) + || !$this->isIdentityQueryList($group['clauses']) + || $group['clauses'] === [] + ) { + throw new InvalidArgumentException('Each identity-query group must contain a non-empty list of clauses.'); + } + + foreach (['type', 'groupType'] as $key) { + if (array_key_exists($key, $group) && !is_string($group[$key])) { + throw new InvalidArgumentException(sprintf('Identity-query group "%s" must be a string.', $key)); + } + } + + foreach ($group['clauses'] as $clause) { + if (!is_array($clause) + || !array_key_exists('column', $clause) + || !array_key_exists('operator', $clause) + || !is_string($clause['operator']) + || $clause['operator'] === '' + ) { + throw new InvalidArgumentException('Each identity-query clause must contain a column and string operator.'); + } + + $column = $clause['column']; + if (is_string($column)) { + if ($column === '') { + throw new InvalidArgumentException('Each identity-query clause column must be a non-empty string or non-empty list of strings.'); + } + + continue; + } + + if (!is_array($column) || !$this->isIdentityQueryList($column) || $column === []) { + throw new InvalidArgumentException('Each identity-query clause column must be a non-empty string or non-empty list of strings.'); + } + + foreach ($column as $field) { + if (!is_string($field) || $field === '') { + throw new InvalidArgumentException('Each identity-query clause column must be a non-empty string or non-empty list of strings.'); + } + } + } + } + } + + /** + * @param array $values + */ + protected function isIdentityQueryList(array $values): bool + { + return array_values($values) === $values; + } + /** * Gets the models from the specified list of IDs. * diff --git a/tests/Unit/Abstracts/IdentifiableDatabaseDatastoreHandlerTest.php b/tests/Unit/Abstracts/IdentifiableDatabaseDatastoreHandlerTest.php new file mode 100644 index 0000000..c2ad661 --- /dev/null +++ b/tests/Unit/Abstracts/IdentifiableDatabaseDatastoreHandlerTest.php @@ -0,0 +1,361 @@ +makeHandler( + new RecordingIdentityQueryBuilder(), + new RecordingIdentityClauseBuilder(), + $this->createMock(QueryStrategy::class) + ); + + self::assertInstanceOf(DatastoreHasIdentityQuery::class, $handler); + } + + public function testFindIdsResetsStateAndBuildsAnIdentityOnlyQuery(): void + { + $queryBuilder = new RecordingIdentityQueryBuilder(); + $clauseBuilder = new RecordingIdentityClauseBuilder(); + $queryStrategy = $this->createMock(QueryStrategy::class); + $expected = [['tenantId' => 10, 'recordId' => 42]]; + $queryStrategy->expects(self::once()) + ->method('query') + ->with(self::identicalTo($queryBuilder)) + ->willReturn($expected); + [$handler, $table] = $this->makeHandler($queryBuilder, $clauseBuilder, $queryStrategy); + + $result = $handler->findIds([[ + 'type' => 'unexpected', + 'groupType' => 'unexpected', + 'clauses' => [[ + 'column' => 'deletedAt', + 'operator' => 'IS NULL', + ]], + ]], 2, 3); + + self::assertSame($expected, $result); + self::assertSame(1, $queryBuilder->resetCalls); + self::assertSame($table, $queryBuilder->table); + self::assertSame(['tenantId', 'recordId'], $queryBuilder->selectedFields); + self::assertSame(2, $queryBuilder->limitValue); + self::assertSame(3, $queryBuilder->offsetValue); + self::assertSame(1, $clauseBuilder->resetCalls); + self::assertCount(1, $clauseBuilder->groups); + self::assertSame('AND', $clauseBuilder->groups[0]['logic']); + self::assertInstanceOf(RecordingIdentityClauseBuilder::class, $clauseBuilder->groups[0]['builder']); + self::assertSame([ + ['method' => 'where', 'field' => 'deletedAt', 'operator' => 'IS NULL', 'values' => []], + ], $clauseBuilder->groups[0]['builder']->predicates); + self::assertSame($clauseBuilder, $queryBuilder->whereClause); + } + + public function testFindIdsAllowsAnUnfilteredQueryWithNullLimitAndZeroOffset(): void + { + $queryBuilder = new RecordingIdentityQueryBuilder(); + $clauseBuilder = new RecordingIdentityClauseBuilder(); + $queryStrategy = $this->createMock(QueryStrategy::class); + $queryStrategy->expects(self::once()) + ->method('query') + ->with(self::identicalTo($queryBuilder)) + ->willReturn([]); + [$handler] = $this->makeHandler($queryBuilder, $clauseBuilder, $queryStrategy); + + self::assertSame([], $handler->findIds([], null, 0)); + self::assertSame(['tenantId', 'recordId'], $queryBuilder->selectedFields); + self::assertNull($queryBuilder->limitValue); + self::assertNull($queryBuilder->offsetValue); + self::assertNull($queryBuilder->whereClause); + } + + /** + * @param array $conditions + * @dataProvider invalidIdentityQueries + */ + public function testFindIdsRejectsInvalidInputBeforeQueryExecution(array $conditions, ?int $limit, ?int $offset): void + { + $queryBuilder = new RecordingIdentityQueryBuilder(); + $clauseBuilder = new RecordingIdentityClauseBuilder(); + $queryStrategy = $this->createMock(QueryStrategy::class); + $queryStrategy->expects(self::never())->method('query'); + [$handler] = $this->makeHandler($queryBuilder, $clauseBuilder, $queryStrategy); + + try { + $handler->findIds($conditions, $limit, $offset); + self::fail('Expected invalid identity-query input to be rejected.'); + } catch (InvalidArgumentException $exception) { + self::assertNotSame('', $exception->getMessage()); + } + + self::assertSame(0, $queryBuilder->resetCalls); + self::assertSame(0, $clauseBuilder->resetCalls); + } + + /** @return array, int|null, int|null}> */ + public static function invalidIdentityQueries(): array + { + $validClause = ['column' => 'id', 'operator' => '=']; + + return [ + 'zero limit' => [[], 0, null], + 'negative limit' => [[], -1, null], + 'negative offset' => [[], null, -1], + 'conditions are not a list' => [['group' => ['clauses' => [$validClause]]], null, null], + 'group is not an array' => [['group'], null, null], + 'group has no clauses' => [[[]], null, null], + 'group has empty clauses' => [[['clauses' => []]], null, null], + 'clauses are not a list' => [[['clauses' => ['first' => $validClause]]], null, null], + 'clause is not an array' => [[['clauses' => ['clause']]], null, null], + 'clause has no column' => [[['clauses' => [['operator' => '=']]]], null, null], + 'clause has no operator' => [[['clauses' => [['column' => 'id']]]], null, null], + 'operator is not a string' => [[['clauses' => [['column' => 'id', 'operator' => 3]]]], null, null], + 'operator is empty' => [[['clauses' => [['column' => 'id', 'operator' => '']]]], null, null], + 'column is not a string or list' => [[['clauses' => [['column' => 3, 'operator' => '=']]]], null, null], + 'column string is empty' => [[['clauses' => [['column' => '', 'operator' => '=']]]], null, null], + 'column list is empty' => [[['clauses' => [['column' => [], 'operator' => '=']]]], null, null], + 'columns are not a list' => [[['clauses' => [['column' => ['id' => 'id'], 'operator' => '=']]]], null, null], + 'column list contains non-string' => [[['clauses' => [['column' => ['id', 3], 'operator' => '=']]]], null, null], + 'column list contains empty string' => [[['clauses' => [['column' => ['id', ''], 'operator' => '=']]]], null, null], + 'type is not a string' => [[['type' => null, 'clauses' => [$validClause]]], null, null], + 'group type is not a string' => [[['groupType' => [], 'clauses' => [$validClause]]], null, null], + ]; + } + + /** + * @return array{IdentityQueryHandlerFixture, Table} + */ + private function makeHandler( + QueryBuilder $queryBuilder, + ClauseBuilder $clauseBuilder, + QueryStrategy $queryStrategy + ): array { + $table = $this->createMock(Table::class); + $table->method('getFieldsForIdentity')->willReturn(['tenantId', 'recordId']); + $serviceProvider = new DatabaseServiceProvider( + $this->createMock(LoggerStrategy::class), + $queryStrategy, + $queryBuilder, + $clauseBuilder, + $this->createMock(CacheableService::class), + $this->createMock(EventStrategy::class) + ); + + return [new IdentityQueryHandlerFixture($serviceProvider, $table), $table]; + } +} + +final class IdentityQueryHandlerFixture extends IdentifiableDatabaseDatastoreHandler +{ + public function __construct(DatabaseServiceProvider $serviceProvider, Table $table) + { + $this->serviceProvider = $serviceProvider; + $this->table = $table; + } +} + +final class RecordingIdentityQueryBuilder implements QueryBuilder +{ + public int $resetCalls = 0; + + /** @var list */ + public array $selectedFields = ['staleField']; + + public ?Table $table = null; + + public ?ClauseBuilder $whereClause = null; + + public ?int $limitValue = 99; + + public ?int $offsetValue = 99; + + public function useTable(Table $table) + { + $this->table = $table; + return $this; + } + + public function select(string $field, string ...$fields) + { + $this->selectedFields = array_values(array_merge($this->selectedFields, [$field], $fields)); + return $this; + } + + public function from(Table $table) + { + $this->table = $table; + return $this; + } + + public function where(?ClauseBuilder $clauseBuilder) + { + $this->whereClause = $clauseBuilder; + return $this; + } + + public function leftJoin(Table $table, string $column, string $onColumn) + { + return $this; + } + + public function rightJoin(Table $table, string $column, string $onColumn) + { + return $this; + } + + public function groupBy(string $column, string ...$columns) + { + return $this; + } + + public function sum(string $fieldToSum, ?string $alias = null) + { + return $this; + } + + public function count(string $fieldToCount, ?string $alias = null) + { + return $this; + } + + public function limit(int $limit) + { + $this->limitValue = $limit; + return $this; + } + + public function offset(int $offset) + { + $this->offsetValue = $offset; + return $this; + } + + public function orderBy(string $field, string $order) + { + return $this; + } + + public function build(): string + { + return 'identity query'; + } + + public function reset() + { + ++$this->resetCalls; + $this->selectedFields = []; + $this->table = null; + $this->whereClause = null; + $this->limitValue = null; + $this->offsetValue = null; + return $this; + } + + public function resetClauses(string $clause, string ...$clauses) + { + return $this; + } +} + +final class RecordingIdentityClauseBuilder implements ClauseBuilder +{ + public int $resetCalls = 0; + + public ?Table $table = null; + + /** @var list}> */ + public array $predicates = []; + + /** @var list */ + public array $groups = []; + + public function useTable(Table $table) + { + $this->table = $table; + return $this; + } + + public function where($field, string $operator, ...$values) + { + $this->predicates[] = [ + 'method' => 'where', + 'field' => $field, + 'operator' => $operator, + 'values' => $values, + ]; + return $this; + } + + public function andWhere($field, string $operator, ...$values) + { + $this->predicates[] = [ + 'method' => 'andWhere', + 'field' => $field, + 'operator' => $operator, + 'values' => $values, + ]; + return $this; + } + + public function orWhere($field, string $operator, ...$values) + { + $this->predicates[] = [ + 'method' => 'orWhere', + 'field' => $field, + 'operator' => $operator, + 'values' => $values, + ]; + return $this; + } + + public function group(string $logic, ClauseBuilder ...$clauses) + { + foreach ($clauses as $builder) { + $this->groups[] = ['method' => 'group', 'logic' => $logic, 'builder' => $builder]; + } + return $this; + } + + public function andGroup(string $logic, ClauseBuilder ...$clauses) + { + foreach ($clauses as $builder) { + $this->groups[] = ['method' => 'andGroup', 'logic' => $logic, 'builder' => $builder]; + } + return $this; + } + + public function orGroup(string $logic, ClauseBuilder ...$clauses) + { + foreach ($clauses as $builder) { + $this->groups[] = ['method' => 'orGroup', 'logic' => $logic, 'builder' => $builder]; + } + return $this; + } + + public function build(): string + { + return 'identity conditions'; + } + + public function reset() + { + ++$this->resetCalls; + $this->predicates = []; + $this->groups = []; + return $this; + } +}