From 8b03499b5344a9a076b5ced6d497b542e1b5b2d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 29 Sep 2023 10:59:46 +0200 Subject: [PATCH] Static analysis --- psalm-baseline.xml | 6 +++++- src/Builder/BuilderEncoder.php | 2 +- src/Builder/Pipeline.php | 30 ++++++++++++++++++++++++------ tests/Builder/BuilderCodecTest.php | 23 ++++++++++++++--------- 4 files changed, 44 insertions(+), 17 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 2657e5c58..a06fddc08 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -65,7 +65,6 @@ $query $result $result[] - $stage $val $val @@ -83,6 +82,11 @@ + + + ($value is NativeType ? BSONType : $value) + + $options diff --git a/src/Builder/BuilderEncoder.php b/src/Builder/BuilderEncoder.php index 2f157d793..dc57f7b61 100644 --- a/src/Builder/BuilderEncoder.php +++ b/src/Builder/BuilderEncoder.php @@ -55,7 +55,7 @@ public function encode($value): stdClass|array|string // A pipeline is encoded as a list of stages if ($value instanceof Pipeline) { $encoded = []; - foreach ($value->stages as $stage) { + foreach ($value->getIterator() as $stage) { $encoded[] = $this->encodeIfSupported($stage); } diff --git a/src/Builder/Pipeline.php b/src/Builder/Pipeline.php index 039ff5f8b..19f400eb0 100644 --- a/src/Builder/Pipeline.php +++ b/src/Builder/Pipeline.php @@ -2,22 +2,40 @@ namespace MongoDB\Builder; +use ArrayIterator; +use IteratorAggregate; use MongoDB\Builder\Stage\StageInterface; use MongoDB\Exception\InvalidArgumentException; +use Traversable; use function array_is_list; +use function array_merge; -class Pipeline +/** @template-implements IteratorAggregate */ +class Pipeline implements IteratorAggregate { - public array $stages; + /** @var StageInterface[] */ + private array $stages = []; - public function __construct( - StageInterface ...$stages - ) { + public function __construct(StageInterface ...$stages) + { + $this->add(...$stages); + } + + /** @return $this */ + public function add(StageInterface ...$stages): static + { if (! array_is_list($stages)) { throw new InvalidArgumentException('Expected $stages argument to be a list, got an associative array.'); } - $this->stages = $stages; + $this->stages = array_merge($this->stages, $stages); + + return $this; + } + + public function getIterator(): Traversable + { + return new ArrayIterator($this->stages); } } diff --git a/tests/Builder/BuilderCodecTest.php b/tests/Builder/BuilderCodecTest.php index d129d5495..dbf6b48e6 100644 --- a/tests/Builder/BuilderCodecTest.php +++ b/tests/Builder/BuilderCodecTest.php @@ -16,19 +16,23 @@ use function array_walk; use function is_array; +/** + * @todo This annotation is not enough as this PHP file needs to use named arguments, that can't compile on PHP 7.4 + * @requires PHP 8.0 + */ class BuilderCodecTest extends TestCase { + /** @see https://www.mongodb.com/docs/manual/reference/operator/aggregation/match/#equality-match */ public function testPipeline(): void { $pipeline = new Pipeline( - Stage::match(Aggregation::eq('$foo', 1)), - Stage::match(Aggregation::ne('$foo', 2)), + // @todo array is accepted by the stage class, but we expect an object. The driver accepts both. + Stage::match((object) ['author' => 'dave']), Stage::limit(1), ); $expected = [ - ['$match' => ['$eq' => ['$foo', 1]]], - ['$match' => ['$ne' => ['$foo', 2]]], + ['$match' => ['author' => 'dave']], ['$limit' => 1], ]; @@ -70,15 +74,16 @@ public function testPerformCount(): void * @see https://www.mongodb.com/docs/manual/reference/operator/aggregation/filter/#examples * @dataProvider provideAggregationFilterLimit */ - public function testAggregationFilter($limit, $expectedLimit): void + public function testAggregationFilter(?int $limit, array $expectedLimit): void { $pipeline = new Pipeline( Stage::project([ 'items' => Aggregation::filter( - input: Expression::arrayFieldPath('items'), - cond: Aggregation::gte(Expression::variable('item.price'), 100), - as: 'item', - limit: $limit, + // @todo use named argument once we can require PHP 8 + Expression::arrayFieldPath('items'), + Aggregation::gte(Expression::variable('item.price'), 100), + 'item', + $limit, ), ]), );