From 3e1251f1188e007f38060e8919277370232ddf90 Mon Sep 17 00:00:00 2001 From: Anton Fedurtsya Date: Wed, 9 Oct 2024 09:49:21 +0300 Subject: [PATCH 1/3] OXDEV-8771 Allow selectors to be optional Signed-off-by: Anton Fedurtsya --- .../GeneralTableDataSelector.php | 24 +++++++++----- .../RelatedTableDataSelector.php | 26 +++++++++++----- .../GeneralTableDataSelectorTest.php | 26 ++++++++++++++++ .../RelatedTableDataSelectorTest.php | 31 +++++++++++++++++++ 4 files changed, 92 insertions(+), 15 deletions(-) diff --git a/src/UserData/Infrastructure/GeneralTableDataSelector.php b/src/UserData/Infrastructure/GeneralTableDataSelector.php index 371934f..6ee2a00 100644 --- a/src/UserData/Infrastructure/GeneralTableDataSelector.php +++ b/src/UserData/Infrastructure/GeneralTableDataSelector.php @@ -19,6 +19,7 @@ public function __construct( private string $selectionTable, private string $filterColumn, private QueryBuilderFactoryInterface $queryBuilderFactory, + private bool $optional = false, ) { } @@ -35,14 +36,23 @@ public function getSelectionTable(): string public function getDataForColumnValue(string $columnValue): array { $queryBuilder = $this->queryBuilderFactory->create(); - $queryBuilder->select('*') - ->from($this->selectionTable) - ->where($this->filterColumn . ' = :filterValue') - ->setParameter('filterValue', $columnValue); - /** @var Result $result */ - $result = $queryBuilder->execute(); /** @phpstan-ignore missingType.iterableValue */ + try { + $queryBuilder->select('*') + ->from($this->selectionTable) + ->where($this->filterColumn . ' = :filterValue') + ->setParameter('filterValue', $columnValue); - return $result->fetchAllAssociative(); + /** @var Result $result */ + $result = $queryBuilder->execute(); /** @phpstan-ignore missingType.iterableValue */ + + return $result->fetchAllAssociative(); + } catch (\Exception $e) { + if (!$this->optional) { + throw $e; + } + } + + return []; } } diff --git a/src/UserData/Infrastructure/RelatedTableDataSelector.php b/src/UserData/Infrastructure/RelatedTableDataSelector.php index 06ea121..345d51f 100644 --- a/src/UserData/Infrastructure/RelatedTableDataSelector.php +++ b/src/UserData/Infrastructure/RelatedTableDataSelector.php @@ -21,6 +21,7 @@ public function __construct( private string $relationCondition, private string $filterColumn, private QueryBuilderFactoryInterface $queryBuilderFactory, + private bool $optional = false, ) { } @@ -37,15 +38,24 @@ public function getSelectionTable(): string public function getDataForColumnValue(string $columnValue): array { $queryBuilder = $this->queryBuilderFactory->create(); - $queryBuilder->select($this->selectionTable . '.*') - ->from($this->primaryTable) - ->innerJoin($this->primaryTable, $this->selectionTable, $this->selectionTable, $this->relationCondition) - ->where($this->filterColumn . ' = :filterValue') - ->setParameter('filterValue', $columnValue); - /** @var Result $result */ - $result = $queryBuilder->execute(); /** @phpstan-ignore missingType.iterableValue */ + try { + $queryBuilder->select($this->selectionTable . '.*') + ->from($this->primaryTable) + ->innerJoin($this->primaryTable, $this->selectionTable, $this->selectionTable, $this->relationCondition) + ->where($this->filterColumn . ' = :filterValue') + ->setParameter('filterValue', $columnValue); - return $result->fetchAllAssociative(); + /** @var Result $result */ + $result = $queryBuilder->execute(); /** @phpstan-ignore missingType.iterableValue */ + + return $result->fetchAllAssociative(); + } catch (\Exception $e) { + if (!$this->optional) { + throw $e; + } + } + + return []; } } diff --git a/tests/Integration/UserData/Infrastructure/GeneralTableDataSelectorTest.php b/tests/Integration/UserData/Infrastructure/GeneralTableDataSelectorTest.php index ebdee9d..0850130 100644 --- a/tests/Integration/UserData/Infrastructure/GeneralTableDataSelectorTest.php +++ b/tests/Integration/UserData/Infrastructure/GeneralTableDataSelectorTest.php @@ -48,4 +48,30 @@ public function testSelectorSelectsDataFromTableFilteredByColumnValue(): void $this->assertSame($collectionName, $sut->getCollection()); $this->assertSame('oxuser', $sut->getSelectionTable()); } + + public function testSelectorExplodesOnWrongQuery(): void + { + $sut = new GeneralTableDataSelector( + collection: uniqid(), + selectionTable: uniqid(), + filterColumn: uniqid(), + queryBuilderFactory: $this->get(QueryBuilderFactoryInterface::class), + ); + + $this->expectException(\Exception::class); + $sut->getDataForColumnValue(self::USER_ID); + } + + public function testOptionalFlagDoesNotExplodeOnQueryError(): void + { + $sut = new GeneralTableDataSelector( + collection: uniqid(), + selectionTable: uniqid(), + filterColumn: uniqid(), + queryBuilderFactory: $this->get(QueryBuilderFactoryInterface::class), + optional: true, + ); + + $this->assertSame([], $sut->getDataForColumnValue(self::USER_ID)); + } } diff --git a/tests/Integration/UserData/Infrastructure/RelatedTableDataSelectorTest.php b/tests/Integration/UserData/Infrastructure/RelatedTableDataSelectorTest.php index e2d9131..d409a33 100644 --- a/tests/Integration/UserData/Infrastructure/RelatedTableDataSelectorTest.php +++ b/tests/Integration/UserData/Infrastructure/RelatedTableDataSelectorTest.php @@ -61,4 +61,35 @@ public function testSelectorSelectsDataFromTableFilteredByColumnValue(): void $this->assertSame($collectionName, $sut->getCollection()); $this->assertSame('oxorderfiles', $sut->getSelectionTable()); } + + public function testSelectorExplodesOnWrongQuery(): void + { + $sut = new RelatedTableDataSelector( + collection: $collectionName = uniqid(), + primaryTable: uniqid(), + selectionTable: uniqid(), + relationCondition: 'oxorderfiles.OXORDERID = oxorder.OXID', + filterColumn: 'oxorder.OXUSERID', + queryBuilderFactory: $this->get(QueryBuilderFactoryInterface::class) + ); + + $this->expectException(\Exception::class); + $sut->getDataForColumnValue(self::USER_ID); + } + + public function testOptionalFlagDoesNotExplodeOnQueryError(): void + { + $sut = new RelatedTableDataSelector( + collection: uniqid(), + primaryTable: uniqid(), + selectionTable: uniqid(), + relationCondition: 'oxorderfiles.OXORDERID = oxorder.OXID', + filterColumn: 'oxorder.OXUSERID', + queryBuilderFactory: $this->get(QueryBuilderFactoryInterface::class), + optional: true + + ); + + $this->assertSame([], $sut->getDataForColumnValue(self::USER_ID)); + } } From e1d73330ddf3212c903266f7dd35ee5ad6d95b76 Mon Sep 17 00:00:00 2001 From: Anton Fedurtsya Date: Wed, 9 Oct 2024 09:49:48 +0300 Subject: [PATCH 2/3] OXDEV-8771 Test currently configured selectors Signed-off-by: Anton Fedurtsya --- src/UserData/services.yaml | 1 + .../UserData/Infrastructure/SelectorTest.php | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 tests/Integration/UserData/Infrastructure/SelectorTest.php diff --git a/src/UserData/services.yaml b/src/UserData/services.yaml index 1c91870..17da597 100644 --- a/src/UserData/services.yaml +++ b/src/UserData/services.yaml @@ -178,6 +178,7 @@ services: $selectionTable: 'oxobject2role' $relationCondition: 'oxuser.OXID = oxobject2role.OXOBJECTID' $filterColumn: 'oxuser.OXID' + $optional: true tags: - { name: 'oe.gdpr.user_data' } diff --git a/tests/Integration/UserData/Infrastructure/SelectorTest.php b/tests/Integration/UserData/Infrastructure/SelectorTest.php new file mode 100644 index 0000000..6d3f59f --- /dev/null +++ b/tests/Integration/UserData/Infrastructure/SelectorTest.php @@ -0,0 +1,24 @@ +get(CollectionAggregationServiceInterface::class); + $aggregate->collectUserData(uniqid()); + + $this->addToAssertionCount(1); + } +} \ No newline at end of file From c1f9b215d2de6a69366aae73be04c35188dd9bd0 Mon Sep 17 00:00:00 2001 From: Anton Fedurtsya Date: Wed, 9 Oct 2024 09:57:07 +0300 Subject: [PATCH 3/3] OXDEV-8771 Fix coding style issues Signed-off-by: Anton Fedurtsya --- src/UserData/Infrastructure/GeneralTableDataSelector.php | 3 +++ src/UserData/Infrastructure/RelatedTableDataSelector.php | 3 +++ .../UserData/Infrastructure/RelatedTableDataSelectorTest.php | 1 - tests/Integration/UserData/Infrastructure/SelectorTest.php | 2 +- 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/UserData/Infrastructure/GeneralTableDataSelector.php b/src/UserData/Infrastructure/GeneralTableDataSelector.php index 6ee2a00..528c22d 100644 --- a/src/UserData/Infrastructure/GeneralTableDataSelector.php +++ b/src/UserData/Infrastructure/GeneralTableDataSelector.php @@ -14,6 +14,9 @@ class GeneralTableDataSelector implements DataSelectorInterface { + /** + * @SuppressWarnings(PHPMD.BooleanArgumentFlag) Not different behaviour, just protection from explosion + */ public function __construct( private string $collection, private string $selectionTable, diff --git a/src/UserData/Infrastructure/RelatedTableDataSelector.php b/src/UserData/Infrastructure/RelatedTableDataSelector.php index 345d51f..44391b4 100644 --- a/src/UserData/Infrastructure/RelatedTableDataSelector.php +++ b/src/UserData/Infrastructure/RelatedTableDataSelector.php @@ -14,6 +14,9 @@ class RelatedTableDataSelector implements DataSelectorInterface { + /** + * @SuppressWarnings(PHPMD.BooleanArgumentFlag) Not different behaviour, just protection from explosion + */ public function __construct( private string $collection, private string $primaryTable, diff --git a/tests/Integration/UserData/Infrastructure/RelatedTableDataSelectorTest.php b/tests/Integration/UserData/Infrastructure/RelatedTableDataSelectorTest.php index d409a33..a2bda02 100644 --- a/tests/Integration/UserData/Infrastructure/RelatedTableDataSelectorTest.php +++ b/tests/Integration/UserData/Infrastructure/RelatedTableDataSelectorTest.php @@ -87,7 +87,6 @@ public function testOptionalFlagDoesNotExplodeOnQueryError(): void filterColumn: 'oxorder.OXUSERID', queryBuilderFactory: $this->get(QueryBuilderFactoryInterface::class), optional: true - ); $this->assertSame([], $sut->getDataForColumnValue(self::USER_ID)); diff --git a/tests/Integration/UserData/Infrastructure/SelectorTest.php b/tests/Integration/UserData/Infrastructure/SelectorTest.php index 6d3f59f..879d356 100644 --- a/tests/Integration/UserData/Infrastructure/SelectorTest.php +++ b/tests/Integration/UserData/Infrastructure/SelectorTest.php @@ -21,4 +21,4 @@ public function testConfiguredSelectors(): void $this->addToAssertionCount(1); } -} \ No newline at end of file +}