From dbbe002254d5dc8a95ba2ccd2d1f30576583a080 Mon Sep 17 00:00:00 2001 From: Denny Lubitz Date: Fri, 1 Dec 2023 15:50:06 +0100 Subject: [PATCH 1/2] TASK Fix phpstan complains about QueryBuilder::execute return value --- .../src/Domain/Repository/ContentGraph.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php index 44d6d1b28dc..b6dd48ff7c6 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php @@ -154,6 +154,7 @@ public function findRootNodeAggregates( /** @var \Traversable $nodeAggregates The factory will return a NodeAggregate since the array is not empty */ $nodeAggregates = $this->nodeFactory->mapNodeRowsToNodeAggregates( + /** @phpstan-ignore-next-line */ $queryBuilder->execute()->fetchAllAssociative(), VisibilityConstraints::withoutRestrictions() ); @@ -177,6 +178,7 @@ public function findNodeAggregatesByType( ]); return $this->nodeFactory->mapNodeRowsToNodeAggregates( + /** @phpstan-ignore-next-line */ $queryBuilder->execute()->fetchAllAssociative(), VisibilityConstraints::withoutRestrictions() ); @@ -203,6 +205,7 @@ public function findNodeAggregateById( ]); return $this->nodeFactory->mapNodeRowsToNodeAggregate( + /** @phpstan-ignore-next-line */ $queryBuilder->execute()->fetchAllAssociative(), VisibilityConstraints::withoutRestrictions() ); @@ -233,6 +236,7 @@ public function findParentNodeAggregates( ]); return $this->nodeFactory->mapNodeRowsToNodeAggregates( + /** @phpstan-ignore-next-line */ $queryBuilder->execute()->fetchAllAssociative(), VisibilityConstraints::withoutRestrictions() ); @@ -271,6 +275,7 @@ public function findParentNodeAggregateByChildOriginDimensionSpacePoint( ]); return $this->nodeFactory->mapNodeRowsToNodeAggregate( + /** @phpstan-ignore-next-line */ $queryBuilder->execute()->fetchAllAssociative(), VisibilityConstraints::withoutRestrictions() ); @@ -286,6 +291,7 @@ public function findChildNodeAggregates( ): iterable { $queryBuilder = $this->buildChildNodeAggregateQuery($parentNodeAggregateId, $contentStreamId); return $this->nodeFactory->mapNodeRowsToNodeAggregates( + /** @phpstan-ignore-next-line */ $queryBuilder->execute()->fetchAllAssociative(), VisibilityConstraints::withoutRestrictions() ); @@ -305,6 +311,7 @@ public function findChildNodeAggregatesByName( ->setParameter('relationName', $name->value); return $this->nodeFactory->mapNodeRowsToNodeAggregates( + /** @phpstan-ignore-next-line */ $queryBuilder->execute()->fetchAllAssociative(), VisibilityConstraints::withoutRestrictions() ); @@ -323,6 +330,7 @@ public function findTetheredChildNodeAggregates( ->setParameter('tetheredClassification', NodeAggregateClassification::CLASSIFICATION_TETHERED->value); return $this->nodeFactory->mapNodeRowsToNodeAggregates( + /** @phpstan-ignore-next-line */ $queryBuilder->execute()->fetchAllAssociative(), VisibilityConstraints::withoutRestrictions() ); @@ -365,6 +373,7 @@ public function getDimensionSpacePointsOccupiedByChildNodeName( 'dimensionSpacePointHashes' => Connection::PARAM_STR_ARRAY, ]); $dimensionSpacePoints = []; + /** @phpstan-ignore-next-line */ foreach ($queryBuilder->execute()->fetchAllAssociative() as $hierarchyRelationData) { $dimensionSpacePoints[$hierarchyRelationData['dimensionspacepointhash']] = DimensionSpacePoint::fromJsonString($hierarchyRelationData['dimensionspacepoint']); } @@ -376,11 +385,13 @@ public function countNodes(): int $queryBuilder = $this->createQueryBuilder() ->select('COUNT(*)') ->from($this->tableNamePrefix . '_node'); + /** @phpstan-ignore-next-line */ return (int)$queryBuilder->execute()->fetchOne(); } public function findUsedNodeTypeNames(): iterable { + /** @phpstan-ignore-next-line */ $rows = $this->createQueryBuilder() ->select('DISTINCT nodetypename') ->from($this->tableNamePrefix . '_node') From b3645052052959da6e0243fdef0a0b4b24184cd2 Mon Sep 17 00:00:00 2001 From: bwaidelich Date: Fri, 1 Dec 2023 16:30:55 +0100 Subject: [PATCH 2/2] Extract DBAL interactions to functions with proper type hints and exception handling --- .../src/Domain/Repository/ContentGraph.php | 116 ++++++++---------- 1 file changed, 51 insertions(+), 65 deletions(-) diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php index b6dd48ff7c6..4c49a10ee24 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php @@ -15,8 +15,10 @@ namespace Neos\ContentGraph\DoctrineDbalAdapter\Domain\Repository; use Doctrine\DBAL\Connection; -use Doctrine\DBAL\DBALException; +use Doctrine\DBAL\Driver\Exception as DriverException; +use Doctrine\DBAL\Exception as DBALException; use Doctrine\DBAL\Query\QueryBuilder; +use Doctrine\DBAL\Result; use Neos\ContentGraph\DoctrineDbalAdapter\DoctrineDbalContentGraphProjection; use Neos\ContentGraph\DoctrineDbalAdapter\Domain\Projection\NodeRelationAnchorPoint; use Neos\ContentRepository\Core\DimensionSpace\DimensionSpacePoint; @@ -33,7 +35,6 @@ use Neos\ContentRepository\Core\Projection\ContentGraph\NodeAggregate; use Neos\ContentRepository\Core\Projection\ContentGraph\NodeAggregates; use Neos\ContentRepository\Core\Projection\ContentGraph\VisibilityConstraints; -use Neos\ContentRepository\Core\SharedModel\Exception\NodeTypeNotFoundException; use Neos\ContentRepository\Core\SharedModel\Exception\RootNodeAggregateDoesNotExist; use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateClassification; use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId; @@ -151,15 +152,7 @@ public function findRootNodeAggregates( ->andWhere('n.nodetypename = :nodeTypeName') ->setParameter('nodeTypeName', $filter->nodeTypeName->value); } - - /** @var \Traversable $nodeAggregates The factory will return a NodeAggregate since the array is not empty */ - $nodeAggregates = $this->nodeFactory->mapNodeRowsToNodeAggregates( - /** @phpstan-ignore-next-line */ - $queryBuilder->execute()->fetchAllAssociative(), - VisibilityConstraints::withoutRestrictions() - ); - - return NodeAggregates::fromArray(iterator_to_array($nodeAggregates)); + return NodeAggregates::fromArray(iterator_to_array($this->mapQueryBuilderToNodeAggregates($queryBuilder))); } public function findNodeAggregatesByType( @@ -176,18 +169,9 @@ public function findNodeAggregatesByType( 'contentStreamId' => $contentStreamId->value, 'nodeTypeName' => $nodeTypeName->value, ]); - - return $this->nodeFactory->mapNodeRowsToNodeAggregates( - /** @phpstan-ignore-next-line */ - $queryBuilder->execute()->fetchAllAssociative(), - VisibilityConstraints::withoutRestrictions() - ); + return $this->mapQueryBuilderToNodeAggregates($queryBuilder); } - /** - * @throws DBALException - * @throws \Exception - */ public function findNodeAggregateById( ContentStreamId $contentStreamId, NodeAggregateId $nodeAggregateId @@ -205,16 +189,13 @@ public function findNodeAggregateById( ]); return $this->nodeFactory->mapNodeRowsToNodeAggregate( - /** @phpstan-ignore-next-line */ - $queryBuilder->execute()->fetchAllAssociative(), + $this->fetchRows($queryBuilder), VisibilityConstraints::withoutRestrictions() ); } /** * @return iterable - * @throws DBALException - * @throws \Exception */ public function findParentNodeAggregates( ContentStreamId $contentStreamId, @@ -235,17 +216,9 @@ public function findParentNodeAggregates( 'contentStreamId' => $contentStreamId->value ]); - return $this->nodeFactory->mapNodeRowsToNodeAggregates( - /** @phpstan-ignore-next-line */ - $queryBuilder->execute()->fetchAllAssociative(), - VisibilityConstraints::withoutRestrictions() - ); + return $this->mapQueryBuilderToNodeAggregates($queryBuilder); } - /** - * @throws DBALException - * @throws \Exception - */ public function findParentNodeAggregateByChildOriginDimensionSpacePoint( ContentStreamId $contentStreamId, NodeAggregateId $childNodeAggregateId, @@ -275,31 +248,24 @@ public function findParentNodeAggregateByChildOriginDimensionSpacePoint( ]); return $this->nodeFactory->mapNodeRowsToNodeAggregate( - /** @phpstan-ignore-next-line */ - $queryBuilder->execute()->fetchAllAssociative(), + $this->fetchRows($queryBuilder), VisibilityConstraints::withoutRestrictions() ); } /** * @return iterable - * @throws DBALException|\Exception */ public function findChildNodeAggregates( ContentStreamId $contentStreamId, NodeAggregateId $parentNodeAggregateId ): iterable { $queryBuilder = $this->buildChildNodeAggregateQuery($parentNodeAggregateId, $contentStreamId); - return $this->nodeFactory->mapNodeRowsToNodeAggregates( - /** @phpstan-ignore-next-line */ - $queryBuilder->execute()->fetchAllAssociative(), - VisibilityConstraints::withoutRestrictions() - ); + return $this->mapQueryBuilderToNodeAggregates($queryBuilder); } /** * @return iterable - * @throws DBALException|NodeTypeNotFoundException */ public function findChildNodeAggregatesByName( ContentStreamId $contentStreamId, @@ -309,17 +275,11 @@ public function findChildNodeAggregatesByName( $queryBuilder = $this->buildChildNodeAggregateQuery($parentNodeAggregateId, $contentStreamId) ->andWhere('ch.name = :relationName') ->setParameter('relationName', $name->value); - - return $this->nodeFactory->mapNodeRowsToNodeAggregates( - /** @phpstan-ignore-next-line */ - $queryBuilder->execute()->fetchAllAssociative(), - VisibilityConstraints::withoutRestrictions() - ); + return $this->mapQueryBuilderToNodeAggregates($queryBuilder); } /** * @return iterable - * @throws DBALException|NodeTypeNotFoundException */ public function findTetheredChildNodeAggregates( ContentStreamId $contentStreamId, @@ -328,12 +288,7 @@ public function findTetheredChildNodeAggregates( $queryBuilder = $this->buildChildNodeAggregateQuery($parentNodeAggregateId, $contentStreamId) ->andWhere('cn.classification = :tetheredClassification') ->setParameter('tetheredClassification', NodeAggregateClassification::CLASSIFICATION_TETHERED->value); - - return $this->nodeFactory->mapNodeRowsToNodeAggregates( - /** @phpstan-ignore-next-line */ - $queryBuilder->execute()->fetchAllAssociative(), - VisibilityConstraints::withoutRestrictions() - ); + return $this->mapQueryBuilderToNodeAggregates($queryBuilder); } /** @@ -343,7 +298,6 @@ public function findTetheredChildNodeAggregates( * @param OriginDimensionSpacePoint $parentNodeOriginDimensionSpacePoint * @param DimensionSpacePointSet $dimensionSpacePointsToCheck * @return DimensionSpacePointSet - * @throws DBALException */ public function getDimensionSpacePointsOccupiedByChildNodeName( ContentStreamId $contentStreamId, @@ -373,8 +327,7 @@ public function getDimensionSpacePointsOccupiedByChildNodeName( 'dimensionSpacePointHashes' => Connection::PARAM_STR_ARRAY, ]); $dimensionSpacePoints = []; - /** @phpstan-ignore-next-line */ - foreach ($queryBuilder->execute()->fetchAllAssociative() as $hierarchyRelationData) { + foreach ($this->fetchRows($queryBuilder) as $hierarchyRelationData) { $dimensionSpacePoints[$hierarchyRelationData['dimensionspacepointhash']] = DimensionSpacePoint::fromJsonString($hierarchyRelationData['dimensionspacepoint']); } return new DimensionSpacePointSet($dimensionSpacePoints); @@ -385,17 +338,22 @@ public function countNodes(): int $queryBuilder = $this->createQueryBuilder() ->select('COUNT(*)') ->from($this->tableNamePrefix . '_node'); - /** @phpstan-ignore-next-line */ - return (int)$queryBuilder->execute()->fetchOne(); + $result = $queryBuilder->execute(); + if (!$result instanceof Result) { + throw new \RuntimeException(sprintf('Failed to count nodes. Expected result to be of type %s, got: %s', Result::class, get_debug_type($result)), 1701444550); + } + try { + return (int)$result->fetchOne(); + } catch (DriverException | DBALException $e) { + throw new \RuntimeException(sprintf('Failed to fetch rows from database: %s', $e->getMessage()), 1701444590, $e); + } } public function findUsedNodeTypeNames(): iterable { - /** @phpstan-ignore-next-line */ - $rows = $this->createQueryBuilder() + $rows = $this->fetchRows($this->createQueryBuilder() ->select('DISTINCT nodetypename') - ->from($this->tableNamePrefix . '_node') - ->execute()->fetchAllAssociative(); + ->from($this->tableNamePrefix . '_node')); return array_map(static fn (array $row) => NodeTypeName::fromString($row['nodetypename']), $rows); } @@ -431,4 +389,32 @@ private function createQueryBuilder(): QueryBuilder { return $this->client->getConnection()->createQueryBuilder(); } + + /** + * @param QueryBuilder $queryBuilder + * @return iterable + */ + private function mapQueryBuilderToNodeAggregates(QueryBuilder $queryBuilder): iterable + { + return $this->nodeFactory->mapNodeRowsToNodeAggregates( + $this->fetchRows($queryBuilder), + VisibilityConstraints::withoutRestrictions() + ); + } + + /** + * @return array> + */ + private function fetchRows(QueryBuilder $queryBuilder): array + { + $result = $queryBuilder->execute(); + if (!$result instanceof Result) { + throw new \RuntimeException(sprintf('Failed to execute query. Expected result to be of type %s, got: %s', Result::class, get_debug_type($result)), 1701443535); + } + try { + return $result->fetchAllAssociative(); + } catch (DriverException | DBALException $e) { + throw new \RuntimeException(sprintf('Failed to fetch rows from database: %s', $e->getMessage()), 1701444358, $e); + } + } }