From 83446936209e719eef63827ad77da7e331cb326d Mon Sep 17 00:00:00 2001 From: Andreas Heigl Date: Wed, 29 Apr 2020 11:20:25 +0200 Subject: [PATCH 01/10] Fix issue with empty array for IN-clause When an IN-clause is created with an empty array PostgreSQL will blow up with an SQL-Error as the resulting SQL would be ` x IN()` which is not allowed. This commit works around that by returning a query-part that always returns false as querying for something that is not part of something else will always return false. --- src/PostgresDocumentStore.php | 11 +++ tests/PostgresDocumentStoreTest.php | 117 ++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+) diff --git a/src/PostgresDocumentStore.php b/src/PostgresDocumentStore.php index 4e7ef62..8bf7265 100644 --- a/src/PostgresDocumentStore.php +++ b/src/PostgresDocumentStore.php @@ -750,6 +750,14 @@ private function filterToWhereClause(Filter $filter, $argsCount = 0): array [$innerFilterStr, $args, $argsCount] = $this->filterToWhereClause($innerFilter, $argsCount); if($innerFilter instanceof DocumentStore\Filter\AnyOfFilter || $innerFilter instanceof DocumentStore\Filter\AnyOfDocIdFilter) { + if ($argsCount === 0) { + return [ + substr_replace(' 1 != 1 ', ' 1 = 1 ', $innerFilterStr), + $args, + $argsCount + ]; + } + $inPos = strpos($innerFilterStr, ' IN('); $filterStr = substr_replace($innerFilterStr, ' NOT IN(', $inPos, 4 /* " IN(" */); return [$filterStr, $args, $argsCount]; @@ -795,6 +803,9 @@ private function isPropFilter(Filter $filter): bool private function makeInClause(string $prop, array $valList, int $argsCount, bool $jsonEncode = false): array { + if ($valList === []) { + return [' 1 != 1 ', [], 0]; + } $argList = []; $params = \implode(",", \array_map(function ($val) use (&$argsCount, &$argList, $jsonEncode) { $param = ":a$argsCount"; diff --git a/tests/PostgresDocumentStoreTest.php b/tests/PostgresDocumentStoreTest.php index 6753023..b8dc9b0 100644 --- a/tests/PostgresDocumentStoreTest.php +++ b/tests/PostgresDocumentStoreTest.php @@ -203,6 +203,32 @@ public function it_handles_any_of_filter() $this->assertCount(2, $filteredDocs); } + /** + * @test + */ + public function it_handles_any_of_filter_with_empty_args() + { + $collectionName = 'test_any_of_filter_with_empty_args'; + $this->documentStore->addCollection($collectionName); + + $doc1 = ["foo" => "bar"]; + $doc2 = ["foo" => "baz"]; + $doc3 = ["foo" => "bat"]; + + $docs = [$doc1, $doc2, $doc3]; + + array_walk($docs, function (array $doc) use ($collectionName) { + $this->documentStore->addDoc($collectionName, Uuid::uuid4()->toString(), $doc); + }); + + $filteredDocs = $this->documentStore->filterDocs( + $collectionName, + new AnyOfFilter("foo", []) + ); + + $this->assertCount(0, $filteredDocs); + } + /** * @test */ @@ -262,6 +288,37 @@ public function it_handles_not_any_of_filter() $this->assertSame('baz', $filteredDocs[0]['foo']); } + /** + * @test + */ + public function it_handles_not_any_of_filter_with_empty_args() + { + $collectionName = 'test_not_any_of_filter_with_empty_args'; + $this->documentStore->addCollection($collectionName); + + $doc1 = ["foo" => "bar"]; + $doc2 = ["foo" => "baz"]; + $doc3 = ["foo" => "bat"]; + + $docs = [$doc1, $doc2, $doc3]; + + array_walk($docs, function (array $doc) use ($collectionName) { + $this->documentStore->addDoc($collectionName, Uuid::uuid4()->toString(), $doc); + }); + + $filteredDocs = $this->documentStore->filterDocs( + $collectionName, + new NotFilter(new AnyOfFilter("foo", [])) + ); + + $filteredDocs = iterator_to_array($filteredDocs); + + $this->assertCount(3, $filteredDocs); + + $this->assertSame('baz', $filteredDocs[1]['foo']); + $this->assertSame('bat', $filteredDocs[2]['foo']); + } + /** * @test */ @@ -316,6 +373,36 @@ public function it_handles_any_of_doc_id_filter() $this->assertEquals(['bar', 'baz'], $vals); } + /** + * @test + */ + public function it_handles_any_of_doc_id_filter_with_empty_args() + { + $collectionName = 'test_any_of_doc_id_filter_with_empty_args'; + $this->documentStore->addCollection($collectionName); + + $firstDocId = Uuid::uuid4()->toString(); + $secondDocId = Uuid::uuid4()->toString(); + $thirdDocId = Uuid::uuid4()->toString(); + + $this->documentStore->addDoc($collectionName, $firstDocId, ['foo' => 'bar']); + $this->documentStore->addDoc($collectionName, $secondDocId, ['foo' => 'bat']); + $this->documentStore->addDoc($collectionName, $thirdDocId, ['foo' => 'baz']); + + $filteredDocs = \iterator_to_array($this->documentStore->filterDocs( + $collectionName, + new AnyOfDocIdFilter([]) + )); + + $this->assertCount(0, $filteredDocs); + + $vals = array_map(function (array $doc) { + return $doc['foo']; + }, $filteredDocs); + + $this->assertEquals([], $vals); + } + /** * @test */ @@ -346,6 +433,36 @@ public function it_handles_not_any_of_id_filter() $this->assertEquals(['bat'], $vals); } + /** + * @test + */ + public function it_handles_not_any_of_id_filter_with_empty_args() + { + $collectionName = 'test_any_of_doc_id_filter_with_empty_args'; + $this->documentStore->addCollection($collectionName); + + $firstDocId = Uuid::uuid4()->toString(); + $secondDocId = Uuid::uuid4()->toString(); + $thirdDocId = Uuid::uuid4()->toString(); + + $this->documentStore->addDoc($collectionName, $firstDocId, ['foo' => 'bar']); + $this->documentStore->addDoc($collectionName, $secondDocId, ['foo' => 'bat']); + $this->documentStore->addDoc($collectionName, $thirdDocId, ['foo' => 'baz']); + + $filteredDocs = \iterator_to_array($this->documentStore->filterDocs( + $collectionName, + new NotFilter(new AnyOfDocIdFilter([])) + )); + + $this->assertCount(3, $filteredDocs); + + $vals = array_map(function (array $doc) { + return $doc['foo']; + }, $filteredDocs); + + $this->assertEquals(['bar', 'bat', 'baz'], $vals); + } + /** * @test */ From 076ee4b76e9657bf147f2fb69961eb0ac8ca1d9f Mon Sep 17 00:00:00 2001 From: Gary Lockett Date: Sun, 27 Sep 2020 17:47:28 +0100 Subject: [PATCH 02/10] tests: confirm that (and how) associative array values are handled in updates --- docker-compose.yml | 3 +- phpunit.xml.dist | 2 +- tests/PostgresDocumentStoreTest.php | 65 +++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index b7c1b34..834e7fd 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -9,7 +9,7 @@ services: - PROOPH_ENV=dev - PDO_DSN=pgsql:host=postgres port=5432 dbname=event_engine - PDO_USER=postgres - - PDO_PWD= + - PDO_PWD=test postgres: image: postgres:alpine @@ -17,3 +17,4 @@ services: - 5432:5432 environment: - POSTGRES_DB=event_engine + - POSTGRES_PASSWORD=test diff --git a/phpunit.xml.dist b/phpunit.xml.dist index f0ed2fe..d1121ea 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -22,7 +22,7 @@ - + diff --git a/tests/PostgresDocumentStoreTest.php b/tests/PostgresDocumentStoreTest.php index b8dc9b0..bd23fbc 100644 --- a/tests/PostgresDocumentStoreTest.php +++ b/tests/PostgresDocumentStoreTest.php @@ -151,6 +151,71 @@ public function it_adds_collection_with_multi_field_index_unique(): void $this->assertStringStartsWith('CREATE UNIQUE INDEX', $indexes[1]['indexdef']); } + /** + * @test + */ + public function it_adds_and_updates_a_doc() + { + $collectionName = 'test_adds_and_updates_a_doc'; + $this->documentStore->addCollection($collectionName); + + $doc = [ + 'some' => [ + 'prop' => 'foo', + 'other' => [ + 'nested' => 42 + ] + ], + 'baz' => 'bat', + ]; + + $docId = Uuid::uuid4()->toString(); + $this->documentStore->addDoc($collectionName, $docId, $doc); + + $persistedDoc = $this->documentStore->getDoc($collectionName, $docId); + + $this->assertEquals($doc, $persistedDoc); + + $doc['baz'] = 'changed val'; + + $this->documentStore->updateDoc($collectionName, $docId, $doc); + + $filter = new EqFilter('baz', 'changed val'); + + $filteredDocs = $this->documentStore->findDocs($collectionName, $filter); + + $this->assertCount(1, $filteredDocs); + } + + /** + * @test + */ + public function it_updates_a_subset_of_a_doc() + { + $collectionName = 'test_updates_a_subset_of_a_doc'; + $this->documentStore->addCollection($collectionName); + + $doc = [ + 'some' => [ + 'prop' => 'foo', + 'other' => 'bar' + ], + 'baz' => 'bat', + ]; + + $docId = Uuid::uuid4()->toString(); + $this->documentStore->addDoc($collectionName, $docId, $doc); + + $this->documentStore->updateDoc($collectionName, $docId, [ + 'some' => [ + 'prop' => 'fuzz' + ] + ]); + + $filteredDocs = array_values(iterator_to_array($this->documentStore->findDocs($collectionName, new EqFilter('some.prop', 'fuzz')))); + $this->assertArrayNotHasKey('other', $filteredDocs[0]['some']); + } + /** * @test */ From 5a179ea2c7855f76c56ba2c9664d39953e6a7cc8 Mon Sep 17 00:00:00 2001 From: Gary Lockett Date: Sun, 27 Sep 2020 22:05:03 +0100 Subject: [PATCH 03/10] feature: add replaceDoc and replaceMany methods --- src/PostgresDocumentStore.php | 75 +++++++++++++++++++++++++++++ tests/PostgresDocumentStoreTest.php | 58 ++++++++++++++++++++++ 2 files changed, 133 insertions(+) diff --git a/src/PostgresDocumentStore.php b/src/PostgresDocumentStore.php index 8bf7265..b18817b 100644 --- a/src/PostgresDocumentStore.php +++ b/src/PostgresDocumentStore.php @@ -19,6 +19,7 @@ use EventEngine\DocumentStore\Postgres\Exception\InvalidArgumentException; use EventEngine\DocumentStore\Postgres\Exception\RuntimeException; use EventEngine\Util\VariableType; + use function implode; use function is_string; use function json_decode; @@ -433,6 +434,80 @@ public function upsertDoc(string $collectionName, string $docId, array $docOrSub } } + /** + * @param string $collectionName + * @param string $docId + * @param array $doc + * @throws \Throwable if updating did not succeed + */ + public function replaceDoc(string $collectionName, string $docId, array $doc): void + { + $metadataStr = ''; + $metadata = []; + + if($this->useMetadataColumns && array_key_exists('metadata', $doc)) { + $metadata = $doc['metadata']; + unset($doc['metadata']); + + + foreach ($metadata as $k => $v) { + $metadataStr .= ', '.$k.' = :'.$k; + } + } + + $cmd = <<schemaName($collectionName)}.{$this->tableName($collectionName)} +SET doc = :doc{$metadataStr} +WHERE id = :id +; +EOT; + $this->transactional(function () use ($cmd, $docId, $doc, $metadata) { + $this->connection->prepare($cmd)->execute(array_merge([ + 'id' => $docId, + 'doc' => json_encode($doc) + ], $metadata)); + }); + } + + /** + * @param string $collectionName + * @param Filter $filter + * @param array $set + * @throws \Throwable in case of connection error or other issues + */ + public function replaceMany(string $collectionName, Filter $filter, array $set): void + { + [$filterStr, $args] = $this->filterToWhereClause($filter); + + $where = $filterStr? "WHERE $filterStr" : ''; + + $metadataStr = ''; + $metadata = []; + + if($this->useMetadataColumns && array_key_exists('metadata', $set)) { + $metadata = $set['metadata']; + unset($set['metadata']); + + + foreach ($metadata as $k => $v) { + $metadataStr .= ', '.$k.' = :'.$k; + } + } + + $cmd = <<schemaName($collectionName)}.{$this->tableName($collectionName)} +SET doc = :doc{$metadataStr} +$where; +EOT; + + $args['doc'] = json_encode($set); + $args = array_merge($args, $metadata); + + $this->transactional(function () use ($cmd, $args) { + $this->connection->prepare($cmd)->execute($args); + }); + } + /** * @param string $collectionName * @param string $docId diff --git a/tests/PostgresDocumentStoreTest.php b/tests/PostgresDocumentStoreTest.php index b8dc9b0..10671d5 100644 --- a/tests/PostgresDocumentStoreTest.php +++ b/tests/PostgresDocumentStoreTest.php @@ -151,6 +151,64 @@ public function it_adds_collection_with_multi_field_index_unique(): void $this->assertStringStartsWith('CREATE UNIQUE INDEX', $indexes[1]['indexdef']); } + /** + * @test + */ + public function it_replaces_a_doc() + { + $collectionName = 'test_replaces_a_doc'; + $this->documentStore->addCollection($collectionName); + + $doc = [ + 'some' => [ + 'prop' => 'foo', + 'other' => [ + 'nested' => 42 + ] + ], + 'baz' => 'bat', + ]; + + $docId = Uuid::uuid4()->toString(); + $this->documentStore->addDoc($collectionName, $docId, $doc); + + $doc = ['baz' => 'changed val']; + + $this->documentStore->replaceDoc($collectionName, $docId, $doc); + + $filter = new EqFilter('baz', 'changed val'); + + $filteredDocs = $this->documentStore->findDocs($collectionName, $filter); + + $this->assertCount(1, $filteredDocs); + } + + /** + * @test + */ + public function it_replaces_many() + { + $collectionName = 'test_replaces_many'; + $this->documentStore->addCollection($collectionName); + + $this->documentStore->addDoc($collectionName, Uuid::uuid4()->toString(), ['some' => ['prop' => 'foo', 'other' => ['prop' => 'bat']]]); + $this->documentStore->addDoc($collectionName, Uuid::uuid4()->toString(), ['some' => ['prop' => 'bar', 'other' => ['prop' => 'bat']]]); + $this->documentStore->addDoc($collectionName, Uuid::uuid4()->toString(), ['some' => ['prop' => 'bar']]); + + $doc = ['some' => ['prop' => 'fuzz']]; + $this->documentStore->replaceMany( + $collectionName, + new EqFilter('some.other.prop', 'bat'), + $doc + ); + + $filteredDocs = array_values(iterator_to_array($this->documentStore->findDocs($collectionName, new EqFilter('some.prop', 'fuzz')))); + + $this->assertCount(2, $filteredDocs); + $this->assertEquals($doc, $filteredDocs[0]); + $this->assertEquals($doc, $filteredDocs[1]); + } + /** * @test */ From 8171a2db7bf7bab6c73051affef2a1a8101d82a0 Mon Sep 17 00:00:00 2001 From: Gary Lockett Date: Tue, 6 Oct 2020 22:01:22 +0100 Subject: [PATCH 04/10] fix(deps): update event-engine/php-persistence to ^0.8 chore(deps): update dependencies to latest PHP 7.2 version --- composer.json | 22 +++++++++++----------- phpunit.xml.dist | 1 - 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/composer.json b/composer.json index 4c18e4b..860458e 100644 --- a/composer.json +++ b/composer.json @@ -16,21 +16,21 @@ } ], "require": { - "php": "^7.1", + "php": "^7.2", "ext-pdo": "*", - "event-engine/php-persistence": "^0.7" + "event-engine/php-persistence": "^0.8" }, "require-dev": { + "infection/infection": "^0.15.3", + "malukenho/docheader": "^0.1.8", + "phpspec/prophecy": "^1.12.1", + "phpstan/phpstan": "^0.12.48", + "phpstan/phpstan-strict-rules": "^0.12.5", + "phpunit/phpunit": "^8.5.8", + "prooph/php-cs-fixer-config": "^0.3.1", + "ramsey/uuid" : "^4.1.1", "roave/security-advisories": "dev-master", - "ramsey/uuid" : "^3.6", - "infection/infection": "^0.11.0", - "malukenho/docheader": "^0.1.4", - "phpspec/prophecy": "^1.7", - "phpstan/phpstan": "^0.10.5", - "phpstan/phpstan-strict-rules": "^0.10.1", - "phpunit/phpunit": "^8.0", - "prooph/php-cs-fixer-config": "^0.3", - "satooshi/php-coveralls": "^1.0" + "php-coveralls/php-coveralls": "^2.2.0" }, "autoload": { "psr-4": { diff --git a/phpunit.xml.dist b/phpunit.xml.dist index d1121ea..529e2db 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -7,7 +7,6 @@ convertWarningsToExceptions="true" processIsolation="false" stopOnFailure="false" - syntaxCheck="false" bootstrap="vendor/autoload.php" > From 7ad5feb168bc1bc269d8a00a7623ba2fe50b57ff Mon Sep 17 00:00:00 2001 From: Zacharias Luiten Date: Sat, 30 Jan 2021 16:44:58 +0100 Subject: [PATCH 05/10] - Allow PHP 8 - Updated (dev) dependencies - Bugfix: replaced substr_replace with str_repalce - Updated doc header with copyright year --- composer.json | 4 ++-- docker-compose.yml | 2 +- src/Exception/InvalidArgumentException.php | 2 +- src/Exception/PostgresDocumentStoreException.php | 2 +- src/Exception/RuntimeException.php | 2 +- src/Index/RawSqlIndexCmd.php | 8 ++++++++ src/Metadata/Column.php | 8 ++++++++ src/Metadata/MetadataColumnIndex.php | 8 ++++++++ src/PostgresDocumentStore.php | 4 ++-- tests/MetadataPostgresDocumentStoreTest.php | 8 ++++++++ tests/PostgresDocumentStoreTest.php | 2 +- tests/SchemedPostgresDocumentStoreTest.php | 2 +- tests/TestUtil.php | 2 +- 13 files changed, 43 insertions(+), 11 deletions(-) diff --git a/composer.json b/composer.json index 860458e..d83e631 100644 --- a/composer.json +++ b/composer.json @@ -16,7 +16,7 @@ } ], "require": { - "php": "^7.2", + "php": "^7.2 || ^8.0", "ext-pdo": "*", "event-engine/php-persistence": "^0.8" }, @@ -27,7 +27,7 @@ "phpstan/phpstan": "^0.12.48", "phpstan/phpstan-strict-rules": "^0.12.5", "phpunit/phpunit": "^8.5.8", - "prooph/php-cs-fixer-config": "^0.3.1", + "prooph/php-cs-fixer-config": "^0.4.0", "ramsey/uuid" : "^4.1.1", "roave/security-advisories": "dev-master", "php-coveralls/php-coveralls": "^2.2.0" diff --git a/docker-compose.yml b/docker-compose.yml index 834e7fd..18581e1 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -2,7 +2,7 @@ version: '2' services: php: - image: prooph/php:7.2-cli + image: prooph/php:8.0-cli volumes: - .:/app environment: diff --git a/src/Exception/InvalidArgumentException.php b/src/Exception/InvalidArgumentException.php index 6cb0c61..4d271f4 100644 --- a/src/Exception/InvalidArgumentException.php +++ b/src/Exception/InvalidArgumentException.php @@ -1,7 +1,7 @@ + * (c) 2019-2021 prooph software GmbH * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. diff --git a/src/Exception/PostgresDocumentStoreException.php b/src/Exception/PostgresDocumentStoreException.php index b58a56f..a4de8ee 100644 --- a/src/Exception/PostgresDocumentStoreException.php +++ b/src/Exception/PostgresDocumentStoreException.php @@ -1,7 +1,7 @@ + * (c) 2019-2021 prooph software GmbH * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. diff --git a/src/Exception/RuntimeException.php b/src/Exception/RuntimeException.php index d0c6274..efef111 100644 --- a/src/Exception/RuntimeException.php +++ b/src/Exception/RuntimeException.php @@ -1,7 +1,7 @@ + * (c) 2019-2021 prooph software GmbH * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. diff --git a/src/Index/RawSqlIndexCmd.php b/src/Index/RawSqlIndexCmd.php index 87c6c8c..b776179 100644 --- a/src/Index/RawSqlIndexCmd.php +++ b/src/Index/RawSqlIndexCmd.php @@ -1,4 +1,12 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + declare(strict_types=1); namespace EventEngine\DocumentStore\Postgres\Index; diff --git a/src/Metadata/Column.php b/src/Metadata/Column.php index 276a893..2101565 100644 --- a/src/Metadata/Column.php +++ b/src/Metadata/Column.php @@ -1,4 +1,12 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + declare(strict_types=1); namespace EventEngine\DocumentStore\Postgres\Metadata; diff --git a/src/Metadata/MetadataColumnIndex.php b/src/Metadata/MetadataColumnIndex.php index 02e98a6..4948466 100644 --- a/src/Metadata/MetadataColumnIndex.php +++ b/src/Metadata/MetadataColumnIndex.php @@ -1,4 +1,12 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + declare(strict_types=1); namespace EventEngine\DocumentStore\Postgres\Metadata; diff --git a/src/PostgresDocumentStore.php b/src/PostgresDocumentStore.php index b18817b..ddbbfc2 100644 --- a/src/PostgresDocumentStore.php +++ b/src/PostgresDocumentStore.php @@ -1,7 +1,7 @@ + * (c) 2019-2021 prooph software GmbH * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. @@ -827,7 +827,7 @@ private function filterToWhereClause(Filter $filter, $argsCount = 0): array if($innerFilter instanceof DocumentStore\Filter\AnyOfFilter || $innerFilter instanceof DocumentStore\Filter\AnyOfDocIdFilter) { if ($argsCount === 0) { return [ - substr_replace(' 1 != 1 ', ' 1 = 1 ', $innerFilterStr), + str_replace(' 1 != 1 ', ' 1 = 1 ', $innerFilterStr), $args, $argsCount ]; diff --git a/tests/MetadataPostgresDocumentStoreTest.php b/tests/MetadataPostgresDocumentStoreTest.php index 402d3fc..1b5b7bd 100644 --- a/tests/MetadataPostgresDocumentStoreTest.php +++ b/tests/MetadataPostgresDocumentStoreTest.php @@ -1,4 +1,12 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + declare(strict_types=1); namespace EventEngine\DocumentStoreTest\Postgres; diff --git a/tests/PostgresDocumentStoreTest.php b/tests/PostgresDocumentStoreTest.php index 93d5a50..1b43640 100644 --- a/tests/PostgresDocumentStoreTest.php +++ b/tests/PostgresDocumentStoreTest.php @@ -1,7 +1,7 @@ + * (c) 2019-2021 prooph software GmbH * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. diff --git a/tests/SchemedPostgresDocumentStoreTest.php b/tests/SchemedPostgresDocumentStoreTest.php index 4c513ad..180377d 100644 --- a/tests/SchemedPostgresDocumentStoreTest.php +++ b/tests/SchemedPostgresDocumentStoreTest.php @@ -1,7 +1,7 @@ + * (c) 2019-2021 prooph software GmbH * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. diff --git a/tests/TestUtil.php b/tests/TestUtil.php index 8ce61bf..9efe706 100644 --- a/tests/TestUtil.php +++ b/tests/TestUtil.php @@ -1,7 +1,7 @@ + * (c) 2019-2021 prooph software GmbH * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. From 92b60a8cdaeb7c985545764f25a7a6678b54da07 Mon Sep 17 00:00:00 2001 From: Alexander Miertsch Date: Sat, 6 Feb 2021 23:36:29 +0100 Subject: [PATCH 06/10] Update composer.json --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index d83e631..ecfca2e 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,7 @@ "require": { "php": "^7.2 || ^8.0", "ext-pdo": "*", - "event-engine/php-persistence": "^0.8" + "event-engine/php-persistence": "^0.9" }, "require-dev": { "infection/infection": "^0.15.3", From 6162636b2843f920a916691e3db829dd4baee108 Mon Sep 17 00:00:00 2001 From: Sandro Keil Date: Tue, 25 May 2021 13:36:16 +0200 Subject: [PATCH 07/10] Check if doc is null - Close #22 --- src/PostgresDocumentStore.php | 2 +- tests/PostgresDocumentStoreTest.php | 26 +++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/PostgresDocumentStore.php b/src/PostgresDocumentStore.php index ddbbfc2..2d9362c 100644 --- a/src/PostgresDocumentStore.php +++ b/src/PostgresDocumentStore.php @@ -427,7 +427,7 @@ public function upsertDoc(string $collectionName, string $docId, array $docOrSub { $doc = $this->getDoc($collectionName, $docId); - if($doc) { + if ($doc !== null) { $this->updateDoc($collectionName, $docId, $docOrSubset); } else { $this->addDoc($collectionName, $docId, $docOrSubset); diff --git a/tests/PostgresDocumentStoreTest.php b/tests/PostgresDocumentStoreTest.php index 1b43640..55922eb 100644 --- a/tests/PostgresDocumentStoreTest.php +++ b/tests/PostgresDocumentStoreTest.php @@ -150,7 +150,7 @@ public function it_adds_collection_with_multi_field_index_unique(): void ); $this->assertStringStartsWith('CREATE UNIQUE INDEX', $indexes[1]['indexdef']); } - + /** * @test */ @@ -216,6 +216,30 @@ public function it_updates_a_subset_of_a_doc() $this->assertArrayNotHasKey('other', $filteredDocs[0]['some']); } + /** + * @test + */ + public function it_upserts_empty_array_doc(): void + { + $collectionName = 'test_upserts_empty_doc'; + $this->documentStore->addCollection($collectionName); + + $doc = []; + + $docId = Uuid::uuid4()->toString(); + $this->documentStore->addDoc($collectionName, $docId, $doc); + + // be aware that this will add the data as an entry to the array which is wrong, because it should be transformed to an object + $this->documentStore->upsertDoc($collectionName, $docId, [ + 'some' => [ + 'prop' => 'fuzz', + ], + ]); + + $doc = $this->documentStore->getDoc($collectionName, $docId); + $this->assertArrayHasKey('some', $doc[0], \var_export($doc, true)); + } + /** * @test */ From 1937cc12e48880984b7c164f530a1072377b10a9 Mon Sep 17 00:00:00 2001 From: Zacharias Luiten Date: Tue, 23 Feb 2021 12:16:11 +0100 Subject: [PATCH 08/10] Extract filter processing logic to a seperate class --- composer.json | 7 +- src/Filter/FilterClause.php | 34 ++++ src/Filter/FilterProcessor.php | 19 +++ src/Filter/PostgresFilterProcessor.php | 199 +++++++++++++++++++++++ src/PostgresDocumentStore.php | 213 ++++++------------------- 5 files changed, 306 insertions(+), 166 deletions(-) create mode 100644 src/Filter/FilterClause.php create mode 100644 src/Filter/FilterProcessor.php create mode 100644 src/Filter/PostgresFilterProcessor.php diff --git a/composer.json b/composer.json index ecfca2e..8470564 100644 --- a/composer.json +++ b/composer.json @@ -17,11 +17,12 @@ ], "require": { "php": "^7.2 || ^8.0", + "ext-json": "*", "ext-pdo": "*", "event-engine/php-persistence": "^0.9" }, "require-dev": { - "infection/infection": "^0.15.3", + "infection/infection": "^0.26.6", "malukenho/docheader": "^0.1.8", "phpspec/prophecy": "^1.12.1", "phpstan/phpstan": "^0.12.48", @@ -45,6 +46,10 @@ "config": { "sort-packages": true, "platform": { + }, + "allow-plugins": { + "ocramius/package-versions": true, + "infection/extension-installer": true } }, "prefer-stable": true, diff --git a/src/Filter/FilterClause.php b/src/Filter/FilterClause.php new file mode 100644 index 0000000..8147c88 --- /dev/null +++ b/src/Filter/FilterClause.php @@ -0,0 +1,34 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace EventEngine\DocumentStore\Postgres\Filter; + +final class FilterClause +{ + private $clause; + private $args; + + public function __construct(?string $clause, array $args = []) + { + $this->clause = $clause; + $this->args = $args; + } + + public function clause(): ?string + { + return $this->clause; + } + + public function args(): array + { + return $this->args; + } +} diff --git a/src/Filter/FilterProcessor.php b/src/Filter/FilterProcessor.php new file mode 100644 index 0000000..3a62c53 --- /dev/null +++ b/src/Filter/FilterProcessor.php @@ -0,0 +1,19 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace EventEngine\DocumentStore\Postgres\Filter; + +use EventEngine\DocumentStore\Filter\Filter; + +interface FilterProcessor +{ + public function process(Filter $filter): FilterClause; +} diff --git a/src/Filter/PostgresFilterProcessor.php b/src/Filter/PostgresFilterProcessor.php new file mode 100644 index 0000000..c8091c3 --- /dev/null +++ b/src/Filter/PostgresFilterProcessor.php @@ -0,0 +1,199 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace EventEngine\DocumentStore\Postgres\Filter; + +use EventEngine\DocumentStore; +use EventEngine\DocumentStore\Filter\Filter; +use EventEngine\DocumentStore\Postgres\Exception\InvalidArgumentException; +use EventEngine\DocumentStore\Postgres\Exception\RuntimeException; + +/** + * Default filter processor class for converting a filter to a where clause. + */ +final class PostgresFilterProcessor implements FilterProcessor +{ + /** + * @var bool + */ + private $useMetadataColumns; + + public function __construct(bool $useMetadataColumns = false) + { + $this->useMetadataColumns = $useMetadataColumns; + } + + public function process(Filter $filter): FilterClause + { + [$filterClause, $args] = $this->processFilter($filter); + + return new FilterClause($filterClause, $args); + } + + /** + * @param Filter $filter + * @param int $argsCount + * @return array + */ + private function processFilter(Filter $filter, int $argsCount = 0): array + { + if($filter instanceof DocumentStore\Filter\AnyFilter) { + if($argsCount > 0) { + throw new InvalidArgumentException('AnyFilter cannot be used together with other filters.'); + } + return [null, [], $argsCount]; + } + + if($filter instanceof DocumentStore\Filter\AndFilter) { + [$filterA, $argsA, $argsCount] = $this->processFilter($filter->aFilter(), $argsCount); + [$filterB, $argsB, $argsCount] = $this->processFilter($filter->bFilter(), $argsCount); + return ["($filterA AND $filterB)", array_merge($argsA, $argsB), $argsCount]; + } + + if($filter instanceof DocumentStore\Filter\OrFilter) { + [$filterA, $argsA, $argsCount] = $this->processFilter($filter->aFilter(), $argsCount); + [$filterB, $argsB, $argsCount] = $this->processFilter($filter->bFilter(), $argsCount); + return ["($filterA OR $filterB)", array_merge($argsA, $argsB), $argsCount]; + } + + switch (get_class($filter)) { + case DocumentStore\Filter\DocIdFilter::class: + /** @var DocumentStore\Filter\DocIdFilter $filter */ + return ["id = :a$argsCount", ["a$argsCount" => $filter->val()], ++$argsCount]; + case DocumentStore\Filter\AnyOfDocIdFilter::class: + /** @var DocumentStore\Filter\AnyOfDocIdFilter $filter */ + return $this->makeInClause('id', $filter->valList(), $argsCount); + case DocumentStore\Filter\AnyOfFilter::class: + /** @var DocumentStore\Filter\AnyOfFilter $filter */ + return $this->makeInClause($this->propToJsonPath($filter->prop()), $filter->valList(), $argsCount, $this->shouldJsonEncodeVal($filter->prop())); + case DocumentStore\Filter\EqFilter::class: + /** @var DocumentStore\Filter\EqFilter $filter */ + $prop = $this->propToJsonPath($filter->prop()); + return ["$prop = :a$argsCount", ["a$argsCount" => $this->prepareVal($filter->val(), $filter->prop())], ++$argsCount]; + case DocumentStore\Filter\GtFilter::class: + /** @var DocumentStore\Filter\GtFilter $filter */ + $prop = $this->propToJsonPath($filter->prop()); + return ["$prop > :a$argsCount", ["a$argsCount" => $this->prepareVal($filter->val(), $filter->prop())], ++$argsCount]; + case DocumentStore\Filter\GteFilter::class: + /** @var DocumentStore\Filter\GteFilter $filter */ + $prop = $this->propToJsonPath($filter->prop()); + return ["$prop >= :a$argsCount", ["a$argsCount" => $this->prepareVal($filter->val(), $filter->prop())], ++$argsCount]; + case DocumentStore\Filter\LtFilter::class: + /** @var DocumentStore\Filter\LtFilter $filter */ + $prop = $this->propToJsonPath($filter->prop()); + return ["$prop < :a$argsCount", ["a$argsCount" => $this->prepareVal($filter->val(), $filter->prop())], ++$argsCount]; + case DocumentStore\Filter\LteFilter::class: + /** @var DocumentStore\Filter\LteFilter $filter */ + $prop = $this->propToJsonPath($filter->prop()); + return ["$prop <= :a$argsCount", ["a$argsCount" => $this->prepareVal($filter->val(), $filter->prop())], ++$argsCount]; + case DocumentStore\Filter\LikeFilter::class: + /** @var DocumentStore\Filter\LikeFilter $filter */ + $prop = $this->propToJsonPath($filter->prop()); + $propParts = explode('->', $prop); + $lastProp = array_pop($propParts); + $prop = implode('->', $propParts) . '->>'.$lastProp; + return ["$prop iLIKE :a$argsCount", ["a$argsCount" => $filter->val()], ++$argsCount]; + case DocumentStore\Filter\NotFilter::class: + /** @var DocumentStore\Filter\NotFilter $filter */ + $innerFilter = $filter->innerFilter(); + + if (!$this->isPropFilter($innerFilter)) { + throw new RuntimeException('Not filter cannot be combined with a non prop filter!'); + } + + [$innerFilterStr, $args, $argsCount] = $this->processFilter($innerFilter, $argsCount); + + if($innerFilter instanceof DocumentStore\Filter\AnyOfFilter || $innerFilter instanceof DocumentStore\Filter\AnyOfDocIdFilter) { + if ($argsCount === 0) { + return [ + str_replace(' 1 != 1 ', ' 1 = 1 ', $innerFilterStr), + $args, + $argsCount + ]; + } + + $inPos = strpos($innerFilterStr, ' IN('); + $filterStr = substr_replace($innerFilterStr, ' NOT IN(', $inPos, 4 /* " IN(" */); + return [$filterStr, $args, $argsCount]; + } + + return ["NOT $innerFilterStr", $args, $argsCount]; + case DocumentStore\Filter\InArrayFilter::class: + /** @var DocumentStore\Filter\InArrayFilter $filter */ + $prop = $this->propToJsonPath($filter->prop()); + return ["$prop @> :a$argsCount", ["a$argsCount" => '[' . $this->prepareVal($filter->val(), $filter->prop()) . ']'], ++$argsCount]; + case DocumentStore\Filter\ExistsFilter::class: + /** @var DocumentStore\Filter\ExistsFilter $filter */ + $prop = $this->propToJsonPath($filter->prop()); + $propParts = explode('->', $prop); + $lastProp = trim(array_pop($propParts), "'"); + $parentProps = implode('->', $propParts); + return ["JSONB_EXISTS($parentProps, '$lastProp')", [], $argsCount]; + default: + throw new RuntimeException('Unsupported filter type. Got ' . get_class($filter)); + } + } + + private function makeInClause(string $prop, array $valList, int $argsCount, bool $jsonEncode = false): array + { + if ($valList === []) { + return [' 1 != 1 ', [], 0]; + } + $argList = []; + $params = \implode(",", \array_map(function ($val) use (&$argsCount, &$argList, $jsonEncode) { + $param = ":a$argsCount"; + $argList["a$argsCount"] = $jsonEncode? \json_encode($val) : $val; + $argsCount++; + return $param; + }, $valList)); + + return ["$prop IN($params)", $argList, $argsCount]; + } + + private function shouldJsonEncodeVal(string $prop): bool + { + if($this->useMetadataColumns && strpos($prop, 'metadata.') === 0) { + return false; + } + + return true; + } + + private function propToJsonPath(string $field): string + { + if($this->useMetadataColumns && strpos($field, 'metadata.') === 0) { + return str_replace('metadata.', '', $field); + } + + return "doc->'" . str_replace('.', "'->'", $field) . "'"; + } + + private function isPropFilter(Filter $filter): bool + { + switch (get_class($filter)) { + case DocumentStore\Filter\AndFilter::class: + case DocumentStore\Filter\OrFilter::class: + case DocumentStore\Filter\NotFilter::class: + return false; + default: + return true; + } + } + + private function prepareVal($value, string $prop) + { + if(!$this->shouldJsonEncodeVal($prop)) { + return $value; + } + + return \json_encode($value); + } +} diff --git a/src/PostgresDocumentStore.php b/src/PostgresDocumentStore.php index ddbbfc2..39e0925 100644 --- a/src/PostgresDocumentStore.php +++ b/src/PostgresDocumentStore.php @@ -16,8 +16,9 @@ use EventEngine\DocumentStore\Index; use EventEngine\DocumentStore\OrderBy\OrderBy; use EventEngine\DocumentStore\PartialSelect; -use EventEngine\DocumentStore\Postgres\Exception\InvalidArgumentException; use EventEngine\DocumentStore\Postgres\Exception\RuntimeException; +use EventEngine\DocumentStore\Postgres\Filter\PostgresFilterProcessor; +use EventEngine\DocumentStore\Postgres\Filter\FilterProcessor; use EventEngine\Util\VariableType; use function implode; @@ -37,24 +38,35 @@ final class PostgresDocumentStore implements DocumentStore\DocumentStore */ private $connection; + /** + * @var FilterProcessor + */ + private $filterProcessor; + private $tablePrefix = 'em_ds_'; private $docIdSchema = 'UUID NOT NULL'; private $manageTransactions; - private $useMetadataColumns = false; + private $useMetadataColumns; public function __construct( \PDO $connection, string $tablePrefix = null, string $docIdSchema = null, bool $transactional = true, - bool $useMetadataColumns = false + bool $useMetadataColumns = false, + FilterProcessor $filterProcessor = null ) { $this->connection = $connection; $this->connection->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION); + if (null === $filterProcessor) { + $filterProcessor = new PostgresFilterProcessor($useMetadataColumns); + } + $this->filterProcessor = $filterProcessor; + if(null !== $tablePrefix) { $this->tablePrefix = $tablePrefix; } @@ -384,9 +396,11 @@ public function updateDoc(string $collectionName, string $docId, array $docOrSub */ public function updateMany(string $collectionName, Filter $filter, array $set): void { - [$filterStr, $args] = $this->filterToWhereClause($filter); + $filterClause = $this->filterProcessor->process($filter); + $filterStr = $filterClause->clause(); + $args = $filterClause->args(); - $where = $filterStr? "WHERE $filterStr" : ''; + $where = $filterStr ? "WHERE $filterStr" : ''; $metadataStr = ''; $metadata = []; @@ -477,7 +491,9 @@ public function replaceDoc(string $collectionName, string $docId, array $doc): v */ public function replaceMany(string $collectionName, Filter $filter, array $set): void { - [$filterStr, $args] = $this->filterToWhereClause($filter); + $filterClause = $this->filterProcessor->process($filter); + $filterStr = $filterClause->clause(); + $args = $filterClause->args(); $where = $filterStr? "WHERE $filterStr" : ''; @@ -534,7 +550,9 @@ public function deleteDoc(string $collectionName, string $docId): void */ public function deleteMany(string $collectionName, Filter $filter): void { - [$filterStr, $args] = $this->filterToWhereClause($filter); + $filterClause = $this->filterProcessor->process($filter); + $filterStr = $filterClause->clause(); + $args = $filterClause->args(); $where = $filterStr? "WHERE $filterStr" : ''; @@ -603,7 +621,9 @@ public function getPartialDoc(string $collectionName, PartialSelect $partialSele */ public function filterDocs(string $collectionName, Filter $filter, int $skip = null, int $limit = null, OrderBy $orderBy = null): \Traversable { - [$filterStr, $args] = $this->filterToWhereClause($filter); + $filterClause = $this->filterProcessor->process($filter); + $filterStr = $filterClause->clause(); + $args = $filterClause->args(); $where = $filterStr ? "WHERE $filterStr" : ''; @@ -634,7 +654,9 @@ public function filterDocs(string $collectionName, Filter $filter, int $skip = n */ public function findDocs(string $collectionName, Filter $filter, int $skip = null, int $limit = null, OrderBy $orderBy = null): \Traversable { - [$filterStr, $args] = $this->filterToWhereClause($filter); + $filterClause = $this->filterProcessor->process($filter); + $filterStr = $filterClause->clause(); + $args = $filterClause->args(); $where = $filterStr ? "WHERE $filterStr" : ''; @@ -662,7 +684,9 @@ public function findDocs(string $collectionName, Filter $filter, int $skip = nul public function findPartialDocs(string $collectionName, PartialSelect $partialSelect, Filter $filter, int $skip = null, int $limit = null, OrderBy $orderBy = null): \Traversable { - [$filterStr, $args] = $this->filterToWhereClause($filter); + $filterClause = $this->filterProcessor->process($filter); + $filterStr = $filterClause->clause(); + $args = $filterClause->args(); $select = $this->makeSelect($partialSelect); @@ -698,7 +722,9 @@ public function findPartialDocs(string $collectionName, PartialSelect $partialSe */ public function filterDocIds(string $collectionName, Filter $filter): array { - [$filterStr, $args] = $this->filterToWhereClause($filter); + $filterClause = $this->filterProcessor->process($filter); + $filterStr = $filterClause->clause(); + $args = $filterClause->args(); $where = $filterStr ? "WHERE {$filterStr}" : ''; $query = "SELECT id FROM {$this->schemaName($collectionName)}.{$this->tableName($collectionName)} {$where}"; @@ -721,7 +747,9 @@ public function filterDocIds(string $collectionName, Filter $filter): array */ public function countDocs(string $collectionName, Filter $filter): int { - [$filterStr, $args] = $this->filterToWhereClause($filter); + $filterClause = $this->filterProcessor->process($filter); + $filterStr = $filterClause->clause(); + $args = $filterClause->args(); $where = $filterStr? "WHERE $filterStr" : ''; @@ -756,105 +784,6 @@ private function transactional(callable $callback) } } - private function filterToWhereClause(Filter $filter, $argsCount = 0): array - { - if($filter instanceof DocumentStore\Filter\AnyFilter) { - if($argsCount > 0) { - throw new InvalidArgumentException('AnyFilter cannot be used together with other filters.'); - } - return [null, [], $argsCount]; - } - - if($filter instanceof DocumentStore\Filter\AndFilter) { - [$filterA, $argsA, $argsCount] = $this->filterToWhereClause($filter->aFilter(), $argsCount); - [$filterB, $argsB, $argsCount] = $this->filterToWhereClause($filter->bFilter(), $argsCount); - return ["($filterA AND $filterB)", array_merge($argsA, $argsB), $argsCount]; - } - - if($filter instanceof DocumentStore\Filter\OrFilter) { - [$filterA, $argsA, $argsCount] = $this->filterToWhereClause($filter->aFilter(), $argsCount); - [$filterB, $argsB, $argsCount] = $this->filterToWhereClause($filter->bFilter(), $argsCount); - return ["($filterA OR $filterB)", array_merge($argsA, $argsB), $argsCount]; - } - - switch (get_class($filter)) { - case DocumentStore\Filter\DocIdFilter::class: - /** @var DocumentStore\Filter\DocIdFilter $filter */ - return ["id = :a$argsCount", ["a$argsCount" => $filter->val()], ++$argsCount]; - case DocumentStore\Filter\AnyOfDocIdFilter::class: - /** @var DocumentStore\Filter\AnyOfDocIdFilter $filter */ - return $this->makeInClause('id', $filter->valList(), $argsCount); - case DocumentStore\Filter\AnyOfFilter::class: - /** @var DocumentStore\Filter\AnyOfFilter $filter */ - return $this->makeInClause($this->propToJsonPath($filter->prop()), $filter->valList(), $argsCount, $this->shouldJsonEncodeVal($filter->prop())); - case DocumentStore\Filter\EqFilter::class: - /** @var DocumentStore\Filter\EqFilter $filter */ - $prop = $this->propToJsonPath($filter->prop()); - return ["$prop = :a$argsCount", ["a$argsCount" => $this->prepareVal($filter->val(), $filter->prop())], ++$argsCount]; - case DocumentStore\Filter\GtFilter::class: - /** @var DocumentStore\Filter\GtFilter $filter */ - $prop = $this->propToJsonPath($filter->prop()); - return ["$prop > :a$argsCount", ["a$argsCount" => $this->prepareVal($filter->val(), $filter->prop())], ++$argsCount]; - case DocumentStore\Filter\GteFilter::class: - /** @var DocumentStore\Filter\GteFilter $filter */ - $prop = $this->propToJsonPath($filter->prop()); - return ["$prop >= :a$argsCount", ["a$argsCount" => $this->prepareVal($filter->val(), $filter->prop())], ++$argsCount]; - case DocumentStore\Filter\LtFilter::class: - /** @var DocumentStore\Filter\LtFilter $filter */ - $prop = $this->propToJsonPath($filter->prop()); - return ["$prop < :a$argsCount", ["a$argsCount" => $this->prepareVal($filter->val(), $filter->prop())], ++$argsCount]; - case DocumentStore\Filter\LteFilter::class: - /** @var DocumentStore\Filter\LteFilter $filter */ - $prop = $this->propToJsonPath($filter->prop()); - return ["$prop <= :a$argsCount", ["a$argsCount" => $this->prepareVal($filter->val(), $filter->prop())], ++$argsCount]; - case DocumentStore\Filter\LikeFilter::class: - /** @var DocumentStore\Filter\LikeFilter $filter */ - $prop = $this->propToJsonPath($filter->prop()); - $propParts = explode('->', $prop); - $lastProp = array_pop($propParts); - $prop = implode('->', $propParts) . '->>'.$lastProp; - return ["$prop iLIKE :a$argsCount", ["a$argsCount" => $filter->val()], ++$argsCount]; - case DocumentStore\Filter\NotFilter::class: - /** @var DocumentStore\Filter\NotFilter $filter */ - $innerFilter = $filter->innerFilter(); - - if (!$this->isPropFilter($innerFilter)) { - throw new RuntimeException('Not filter cannot be combined with a non prop filter!'); - } - - [$innerFilterStr, $args, $argsCount] = $this->filterToWhereClause($innerFilter, $argsCount); - - if($innerFilter instanceof DocumentStore\Filter\AnyOfFilter || $innerFilter instanceof DocumentStore\Filter\AnyOfDocIdFilter) { - if ($argsCount === 0) { - return [ - str_replace(' 1 != 1 ', ' 1 = 1 ', $innerFilterStr), - $args, - $argsCount - ]; - } - - $inPos = strpos($innerFilterStr, ' IN('); - $filterStr = substr_replace($innerFilterStr, ' NOT IN(', $inPos, 4 /* " IN(" */); - return [$filterStr, $args, $argsCount]; - } - - return ["NOT $innerFilterStr", $args, $argsCount]; - case DocumentStore\Filter\InArrayFilter::class: - /** @var DocumentStore\Filter\InArrayFilter $filter */ - $prop = $this->propToJsonPath($filter->prop()); - return ["$prop @> :a$argsCount", ["a$argsCount" => '[' . $this->prepareVal($filter->val(), $filter->prop()) . ']'], ++$argsCount]; - case DocumentStore\Filter\ExistsFilter::class: - /** @var DocumentStore\Filter\ExistsFilter $filter */ - $prop = $this->propToJsonPath($filter->prop()); - $propParts = explode('->', $prop); - $lastProp = trim(array_pop($propParts), "'"); - $parentProps = implode('->', $propParts); - return ["JSONB_EXISTS($parentProps, '$lastProp')", [], $argsCount]; - default: - throw new RuntimeException('Unsupported filter type. Got ' . get_class($filter)); - } - } - private function propToJsonPath(string $field): string { if($this->useMetadataColumns && strpos($field, 'metadata.') === 0) { @@ -864,34 +793,6 @@ private function propToJsonPath(string $field): string return "doc->'" . str_replace('.', "'->'", $field) . "'"; } - private function isPropFilter(Filter $filter): bool - { - switch (get_class($filter)) { - case DocumentStore\Filter\AndFilter::class: - case DocumentStore\Filter\OrFilter::class: - case DocumentStore\Filter\NotFilter::class: - return false; - default: - return true; - } - } - - private function makeInClause(string $prop, array $valList, int $argsCount, bool $jsonEncode = false): array - { - if ($valList === []) { - return [' 1 != 1 ', [], 0]; - } - $argList = []; - $params = \implode(",", \array_map(function ($val) use (&$argsCount, &$argList, $jsonEncode) { - $param = ":a$argsCount"; - $argList["a$argsCount"] = $jsonEncode? \json_encode($val) : $val; - $argsCount++; - return $param; - }, $valList)); - - return ["$prop IN($params)", $argList, $argsCount]; - } - private function makeSelect(PartialSelect $partialSelect): string { $select = 'id as "'.self::PARTIAL_SELECT_DOC_ID.'", '; @@ -1016,24 +917,6 @@ private function indexToSqlCmd(Index $index, string $collectionName): string return $cmd; } - private function prepareVal($value, string $prop) - { - if(!$this->shouldJsonEncodeVal($prop)) { - return $value; - } - - return \json_encode($value); - } - - private function shouldJsonEncodeVal(string $prop): bool - { - if($this->useMetadataColumns && strpos($prop, 'metadata.') === 0) { - return false; - } - - return true; - } - private function getIndexName(Index $index): ?string { if(method_exists($index, 'name')) { @@ -1066,12 +949,12 @@ private function tableName(string $collectionName): string return mb_strtolower($this->tablePrefix . $collectionName); } - private function schemaName(string $collectionName): string - { - $schemaName = 'public'; - if (false !== $dotPosition = strpos($collectionName, '.')) { - $schemaName = substr($collectionName, 0, $dotPosition); - } - return mb_strtolower($schemaName); - } + private function schemaName(string $collectionName): string + { + $schemaName = 'public'; + if (false !== $dotPosition = strpos($collectionName, '.')) { + $schemaName = substr($collectionName, 0, $dotPosition); + } + return mb_strtolower($schemaName); + } } From 909c5d46bbb8fa99e41f5606e787fcc95719c54e Mon Sep 17 00:00:00 2001 From: Zacharias Luiten Date: Mon, 1 Mar 2021 12:18:38 +0100 Subject: [PATCH 09/10] Extract order by processing logic to a separate class --- src/OrderBy/OrderByClause.php | 34 +++++++++ src/OrderBy/OrderByProcessor.php | 19 +++++ src/OrderBy/PostgresOrderByProcessor.php | 60 ++++++++++++++++ src/PostgresDocumentStore.php | 66 +++++++++--------- tests/PostgresDocumentStoreTest.php | 89 ++++++++++++++++++++++++ 5 files changed, 237 insertions(+), 31 deletions(-) create mode 100644 src/OrderBy/OrderByClause.php create mode 100644 src/OrderBy/OrderByProcessor.php create mode 100644 src/OrderBy/PostgresOrderByProcessor.php diff --git a/src/OrderBy/OrderByClause.php b/src/OrderBy/OrderByClause.php new file mode 100644 index 0000000..3567467 --- /dev/null +++ b/src/OrderBy/OrderByClause.php @@ -0,0 +1,34 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace EventEngine\DocumentStore\Postgres\OrderBy; + +final class OrderByClause +{ + private $clause; + private $args; + + public function __construct(?string $clause, array $args = []) + { + $this->clause = $clause; + $this->args = $args; + } + + public function clause(): ?string + { + return $this->clause; + } + + public function args(): array + { + return $this->args; + } +} diff --git a/src/OrderBy/OrderByProcessor.php b/src/OrderBy/OrderByProcessor.php new file mode 100644 index 0000000..3c04b47 --- /dev/null +++ b/src/OrderBy/OrderByProcessor.php @@ -0,0 +1,19 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace EventEngine\DocumentStore\Postgres\OrderBy; + +use EventEngine\DocumentStore\OrderBy\OrderBy; + +interface OrderByProcessor +{ + public function process(OrderBy $orderBy): OrderByClause; +} diff --git a/src/OrderBy/PostgresOrderByProcessor.php b/src/OrderBy/PostgresOrderByProcessor.php new file mode 100644 index 0000000..4d6ef4e --- /dev/null +++ b/src/OrderBy/PostgresOrderByProcessor.php @@ -0,0 +1,60 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace EventEngine\DocumentStore\Postgres\OrderBy; + +use EventEngine\DocumentStore; +use EventEngine\DocumentStore\OrderBy\OrderBy; + +final class PostgresOrderByProcessor implements OrderByProcessor +{ + /** + * @var bool + */ + private $useMetadataColumns; + + public function __construct(bool $useMetadataColumns = false) + { + $this->useMetadataColumns = $useMetadataColumns; + } + + public function process(OrderBy $orderBy): OrderByClause + { + [$orderByClause, $args] = $this->processOrderBy($orderBy); + + return new OrderByClause($orderByClause, $args); + } + + private function processOrderBy(OrderBy $orderBy): array + { + if($orderBy instanceof DocumentStore\OrderBy\AndOrder) { + [$sortA, $sortAArgs] = $this->processOrderBy($orderBy->a()); + [$sortB, $sortBArgs] = $this->processOrderBy($orderBy->b()); + + return ["$sortA, $sortB", array_merge($sortAArgs, $sortBArgs)]; + } + + /** @var DocumentStore\OrderBy\Asc|DocumentStore\OrderBy\Desc $orderBy */ + $direction = $orderBy instanceof DocumentStore\OrderBy\Asc ? 'ASC' : 'DESC'; + $prop = $this->propToJsonPath($orderBy->prop()); + + return ["{$prop} $direction", []]; + } + + private function propToJsonPath(string $field): string + { + if($this->useMetadataColumns && strpos($field, 'metadata.') === 0) { + return str_replace('metadata.', '', $field); + } + + return "doc->'" . str_replace('.', "'->'", $field) . "'"; + } +} diff --git a/src/PostgresDocumentStore.php b/src/PostgresDocumentStore.php index 0dc0072..3d3d936 100644 --- a/src/PostgresDocumentStore.php +++ b/src/PostgresDocumentStore.php @@ -17,8 +17,11 @@ use EventEngine\DocumentStore\OrderBy\OrderBy; use EventEngine\DocumentStore\PartialSelect; use EventEngine\DocumentStore\Postgres\Exception\RuntimeException; -use EventEngine\DocumentStore\Postgres\Filter\PostgresFilterProcessor; use EventEngine\DocumentStore\Postgres\Filter\FilterProcessor; +use EventEngine\DocumentStore\Postgres\Filter\PostgresFilterProcessor; +use EventEngine\DocumentStore\Postgres\OrderBy\OrderByClause; +use EventEngine\DocumentStore\Postgres\OrderBy\OrderByProcessor; +use EventEngine\DocumentStore\Postgres\OrderBy\PostgresOrderByProcessor; use EventEngine\Util\VariableType; use function implode; @@ -43,6 +46,11 @@ final class PostgresDocumentStore implements DocumentStore\DocumentStore */ private $filterProcessor; + /** + * @var OrderByProcessor + */ + private $orderByProcessor; + private $tablePrefix = 'em_ds_'; private $docIdSchema = 'UUID NOT NULL'; @@ -57,7 +65,8 @@ public function __construct( string $docIdSchema = null, bool $transactional = true, bool $useMetadataColumns = false, - FilterProcessor $filterProcessor = null + FilterProcessor $filterProcessor = null, + OrderByProcessor $orderByProcessor = null ) { $this->connection = $connection; $this->connection->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION); @@ -67,6 +76,11 @@ public function __construct( } $this->filterProcessor = $filterProcessor; + if (null === $orderByProcessor) { + $orderByProcessor = new PostgresOrderByProcessor($useMetadataColumns); + } + $this->orderByProcessor = $orderByProcessor; + if(null !== $tablePrefix) { $this->tablePrefix = $tablePrefix; } @@ -441,7 +455,7 @@ public function upsertDoc(string $collectionName, string $docId, array $docOrSub { $doc = $this->getDoc($collectionName, $docId); - if ($doc !== null) { + if($doc !== null) { $this->updateDoc($collectionName, $docId, $docOrSubset); } else { $this->addDoc($collectionName, $docId, $docOrSubset); @@ -625,12 +639,16 @@ public function filterDocs(string $collectionName, Filter $filter, int $skip = n $filterStr = $filterClause->clause(); $args = $filterClause->args(); + $orderByClause = $orderBy ? $this->orderByProcessor->process($orderBy) : new OrderByClause(null, []); + $orderByStr = $orderByClause->clause(); + $orderByArgs = $orderByClause->args(); + $where = $filterStr ? "WHERE $filterStr" : ''; $offset = $skip !== null ? "OFFSET $skip" : ''; $limit = $limit !== null ? "LIMIT $limit" : ''; - $orderBy = $orderBy ? "ORDER BY " . implode(', ', $this->orderByToSort($orderBy)) : ''; + $orderBy = $orderByStr ? "ORDER BY $orderByStr" : ''; $query = <<connection->prepare($query); - $stmt->execute($args); + $stmt->execute(array_merge($args, $orderByArgs)); while($row = $stmt->fetch(\PDO::FETCH_ASSOC)) { yield json_decode($row['doc'], true); @@ -658,12 +676,16 @@ public function findDocs(string $collectionName, Filter $filter, int $skip = nul $filterStr = $filterClause->clause(); $args = $filterClause->args(); + $orderByClause = $orderBy ? $this->orderByProcessor->process($orderBy) : new OrderByClause(null, []); + $orderByStr = $orderByClause->clause(); + $orderByArgs = $orderByClause->args(); + $where = $filterStr ? "WHERE $filterStr" : ''; $offset = $skip !== null ? "OFFSET $skip" : ''; $limit = $limit !== null ? "LIMIT $limit" : ''; - $orderBy = $orderBy ? "ORDER BY " . implode(', ', $this->orderByToSort($orderBy)) : ''; + $orderBy = $orderByStr ? "ORDER BY $orderByStr" : ''; $query = <<connection->prepare($query); - $stmt->execute($args); + $stmt->execute(array_merge($args, $orderByArgs)); while($row = $stmt->fetch(\PDO::FETCH_ASSOC)) { yield $row['id'] => json_decode($row['doc'], true); @@ -688,6 +710,10 @@ public function findPartialDocs(string $collectionName, PartialSelect $partialSe $filterStr = $filterClause->clause(); $args = $filterClause->args(); + $orderByClause = $orderBy ? $this->orderByProcessor->process($orderBy) : new OrderByClause(null, []); + $orderByStr = $orderByClause->clause(); + $orderByArgs = $orderByClause->args(); + $select = $this->makeSelect($partialSelect); $where = $filterStr ? "WHERE $filterStr" : ''; @@ -695,7 +721,7 @@ public function findPartialDocs(string $collectionName, PartialSelect $partialSe $offset = $skip !== null ? "OFFSET $skip" : ''; $limit = $limit !== null ? "LIMIT $limit" : ''; - $orderBy = $orderBy ? "ORDER BY " . implode(', ', $this->orderByToSort($orderBy)) : ''; + $orderBy = $orderByStr ? "ORDER BY $orderByStr" : ''; $query = <<connection->prepare($query); - $stmt->execute($args); + $stmt->execute(array_merge($args, $orderByArgs)); while($row = $stmt->fetch(\PDO::FETCH_ASSOC)) { yield $row[self::PARTIAL_SELECT_DOC_ID] => $this->transformPartialDoc($partialSelect, $row); @@ -870,28 +896,6 @@ private function transformPartialDoc(PartialSelect $partialSelect, array $select return $partialDoc; } - private function orderByToSort(DocumentStore\OrderBy\OrderBy $orderBy): array - { - $sort = []; - - if($orderBy instanceof DocumentStore\OrderBy\AndOrder) { - /** @var DocumentStore\OrderBy\Asc|DocumentStore\OrderBy\Desc $orderByA */ - $orderByA = $orderBy->a(); - $direction = $orderByA instanceof DocumentStore\OrderBy\Asc ? 'ASC' : 'DESC'; - $prop = $this->propToJsonPath($orderByA->prop()); - $sort[] = "{$prop} $direction"; - - $sortB = $this->orderByToSort($orderBy->b()); - - return array_merge($sort, $sortB); - } - - /** @var DocumentStore\OrderBy\Asc|DocumentStore\OrderBy\Desc $orderBy */ - $direction = $orderBy instanceof DocumentStore\OrderBy\Asc ? 'ASC' : 'DESC'; - $prop = $this->propToJsonPath($orderBy->prop()); - return ["{$prop} $direction"]; - } - private function indexToSqlCmd(Index $index, string $collectionName): string { if($index instanceof DocumentStore\FieldIndex) { diff --git a/tests/PostgresDocumentStoreTest.php b/tests/PostgresDocumentStoreTest.php index 55922eb..c46f417 100644 --- a/tests/PostgresDocumentStoreTest.php +++ b/tests/PostgresDocumentStoreTest.php @@ -23,6 +23,9 @@ use EventEngine\DocumentStore\Filter\LtFilter; use EventEngine\DocumentStore\Filter\NotFilter; use EventEngine\DocumentStore\Filter\OrFilter; +use EventEngine\DocumentStore\OrderBy\AndOrder; +use EventEngine\DocumentStore\OrderBy\Asc; +use EventEngine\DocumentStore\OrderBy\Desc; use EventEngine\DocumentStore\PartialSelect; use PHPUnit\Framework\TestCase; use EventEngine\DocumentStore\FieldIndex; @@ -813,6 +816,92 @@ public function it_counts_any_of_filter() $this->assertSame(2, $count); } + /** + * @test + */ + public function it_handles_order_by() + { + $collectionName = 'test_it_handles_order_by'; + $this->documentStore->addCollection($collectionName); + + $this->documentStore->addDoc($collectionName, Uuid::uuid4()->toString(), ['some' => ['prop' => 'foo']]); + $this->documentStore->addDoc($collectionName, Uuid::uuid4()->toString(), ['some' => ['prop' => 'bar']]); + $this->documentStore->addDoc($collectionName, Uuid::uuid4()->toString(), ['some' => ['prop' => 'bas']]); + + $filteredDocs = \array_values(\iterator_to_array($this->documentStore->findDocs( + $collectionName, + new AnyFilter(), + null, + null, + Asc::fromString('some.prop') + ))); + + $this->assertCount(3, $filteredDocs); + + $this->assertEquals( + [ + ['some' => ['prop' => 'bar']], + ['some' => ['prop' => 'bas']], + ['some' => ['prop' => 'foo']], + ], + $filteredDocs + ); + + $filteredDocs = \array_values(\iterator_to_array($this->documentStore->findDocs( + $collectionName, + new AnyFilter(), + null, + null, + Desc::fromString('some.prop') + ))); + + $this->assertCount(3, $filteredDocs); + + $this->assertEquals( + [ + ['some' => ['prop' => 'foo']], + ['some' => ['prop' => 'bas']], + ['some' => ['prop' => 'bar']], + ], + $filteredDocs + ); + } + + /** + * @test + */ + public function it_handles_and_order_by() + { + $collectionName = 'test_it_handles_order_by'; + $this->documentStore->addCollection($collectionName); + + $this->documentStore->addDoc($collectionName, Uuid::uuid4()->toString(), ['some' => ['prop' => 'foo', 'other' => ['prop' => 'bas']]]); + $this->documentStore->addDoc($collectionName, Uuid::uuid4()->toString(), ['some' => ['prop' => 'bar', 'other' => ['prop' => 'bat']]]); + $this->documentStore->addDoc($collectionName, Uuid::uuid4()->toString(), ['some' => ['prop' => 'bar']]); + + $filteredDocs = \array_values(\iterator_to_array($this->documentStore->findDocs( + $collectionName, + new AnyFilter(), + null, + null, + AndOrder::by( + Asc::fromString('some.prop'), + Desc::fromString('some.other') + ) + ))); + + $this->assertCount(3, $filteredDocs); + + $this->assertEquals( + [ + ['some' => ['prop' => 'bar']], + ['some' => ['prop' => 'bar', 'other' => ['prop' => 'bat']]], + ['some' => ['prop' => 'foo', 'other' => ['prop' => 'bas']]], + ], + $filteredDocs + ); + } + /** * @test */ From 10ab64d7a62881d256dd439627831d2fe820914a Mon Sep 17 00:00:00 2001 From: Arne De Smedt Date: Fri, 17 Jan 2025 18:52:05 +0100 Subject: [PATCH 10/10] Remove php8.4 deprecations --- src/Index/RawSqlIndexCmd.php | 2 +- src/PostgresDocumentStore.php | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Index/RawSqlIndexCmd.php b/src/Index/RawSqlIndexCmd.php index b776179..01d6a07 100644 --- a/src/Index/RawSqlIndexCmd.php +++ b/src/Index/RawSqlIndexCmd.php @@ -35,7 +35,7 @@ public static function fromArray(array $data): Index return new self($data['sql'], $data['name'] ?? null); } - public function __construct(string $sql, string $name = null) + public function __construct(string $sql, ?string $name = null) { $this->sql = $sql; $this->name = $name; diff --git a/src/PostgresDocumentStore.php b/src/PostgresDocumentStore.php index 3d3d936..23cf01b 100644 --- a/src/PostgresDocumentStore.php +++ b/src/PostgresDocumentStore.php @@ -61,12 +61,12 @@ final class PostgresDocumentStore implements DocumentStore\DocumentStore public function __construct( \PDO $connection, - string $tablePrefix = null, - string $docIdSchema = null, + ?string $tablePrefix = null, + ?string $docIdSchema = null, bool $transactional = true, bool $useMetadataColumns = false, - FilterProcessor $filterProcessor = null, - OrderByProcessor $orderByProcessor = null + ?FilterProcessor $filterProcessor = null, + ?OrderByProcessor $orderByProcessor = null ) { $this->connection = $connection; $this->connection->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION); @@ -633,7 +633,7 @@ public function getPartialDoc(string $collectionName, PartialSelect $partialSele /** * @inheritDoc */ - public function filterDocs(string $collectionName, Filter $filter, int $skip = null, int $limit = null, OrderBy $orderBy = null): \Traversable + public function filterDocs(string $collectionName, Filter $filter, ?int $skip = null, ?int $limit = null, ?OrderBy $orderBy = null): \Traversable { $filterClause = $this->filterProcessor->process($filter); $filterStr = $filterClause->clause(); @@ -670,7 +670,7 @@ public function filterDocs(string $collectionName, Filter $filter, int $skip = n /** * @inheritDoc */ - public function findDocs(string $collectionName, Filter $filter, int $skip = null, int $limit = null, OrderBy $orderBy = null): \Traversable + public function findDocs(string $collectionName, Filter $filter, ?int $skip = null, ?int $limit = null, ?OrderBy $orderBy = null): \Traversable { $filterClause = $this->filterProcessor->process($filter); $filterStr = $filterClause->clause(); @@ -704,7 +704,7 @@ public function findDocs(string $collectionName, Filter $filter, int $skip = nul } } - public function findPartialDocs(string $collectionName, PartialSelect $partialSelect, Filter $filter, int $skip = null, int $limit = null, OrderBy $orderBy = null): \Traversable + public function findPartialDocs(string $collectionName, PartialSelect $partialSelect, Filter $filter, ?int $skip = null, ?int $limit = null, ?OrderBy $orderBy = null): \Traversable { $filterClause = $this->filterProcessor->process($filter); $filterStr = $filterClause->clause();