Makes is_pipeline accept empty list

GromNaN · GromNaN · commit 3da7d2e2af8e · 2023-05-24T12:19:43.000+02:00
diff --git a/src/Operation/Aggregate.php b/src/Operation/Aggregate.php
@@ -138,8 +138,7 @@ class Aggregate implements Executable, Explainable
      */
     public function __construct(string $databaseName, ?string $collectionName, array $pipeline, array $options = [])
     {
-        // Empty pipelines are allowed
-        if ($pipeline !== [] && ! is_pipeline($pipeline)) {
+        if (! is_pipeline($pipeline)) {
             throw new InvalidArgumentException('$pipeline is not a valid aggregation pipeline');
         }
 
diff --git a/src/Operation/BulkWrite.php b/src/Operation/BulkWrite.php
@@ -229,7 +229,7 @@ public function __construct(string $databaseName, string $collectionName, array
                         throw InvalidArgumentException::invalidType(sprintf('$operations[%d]["%s"][1]', $i, $type), $args[1], 'array or object');
                     }
 
-                    if (! is_first_key_operator($args[1]) && ! is_pipeline($args[1])) {
+                    if ($args[1] === [] || ! is_first_key_operator($args[1]) && ! is_pipeline($args[1])) {
                         throw new InvalidArgumentException(sprintf('First key in $operations[%d]["%s"][1] is neither an update operator nor a pipeline', $i, $type));
                     }
 
diff --git a/src/Operation/FindOneAndUpdate.php b/src/Operation/FindOneAndUpdate.php
@@ -114,7 +114,7 @@ public function __construct(string $databaseName, string $collectionName, $filte
             throw InvalidArgumentException::invalidType('$update', $update, 'array or object');
         }
 
-        if (! is_first_key_operator($update) && ! is_pipeline($update)) {
+        if ($update === [] || ! is_first_key_operator($update) && ! is_pipeline($update)) {
             throw new InvalidArgumentException('Expected an update document with operator as first key or a pipeline');
         }
 
diff --git a/src/Operation/ReplaceOne.php b/src/Operation/ReplaceOne.php
@@ -86,6 +86,10 @@ public function __construct(string $databaseName, string $collectionName, $filte
             throw InvalidArgumentException::invalidType('$replacement', $replacement, 'array or object');
         }
 
+        if ($replacement === []) {
+            throw new InvalidArgumentException('$replacement argument is empty');
+        }
+
         if (is_first_key_operator($replacement)) {
             throw new InvalidArgumentException('First key in $replacement argument is an update operator');
         }
diff --git a/src/Operation/Update.php b/src/Operation/Update.php
@@ -147,7 +147,7 @@ public function __construct(string $databaseName, string $collectionName, $filte
             throw InvalidArgumentException::invalidType('"multi" option', $options['multi'], 'boolean');
         }
 
-        if ($options['multi'] && ! is_first_key_operator($update) && ! is_pipeline($update)) {
+        if ($options['multi'] && ($update === [] || ! is_first_key_operator($update) && ! is_pipeline($update))) {
             throw new InvalidArgumentException('"multi" option cannot be true if $update is a replacement document');
         }
 
diff --git a/src/Operation/UpdateMany.php b/src/Operation/UpdateMany.php
@@ -89,7 +89,7 @@ public function __construct(string $databaseName, string $collectionName, $filte
             throw InvalidArgumentException::invalidType('$update', $update, 'array or object');
         }
 
-        if (! is_first_key_operator($update) && ! is_pipeline($update)) {
+        if ($update === [] || ! is_first_key_operator($update) && ! is_pipeline($update)) {
             throw new InvalidArgumentException('Expected an update document with operator as first key or a pipeline');
         }
 
diff --git a/src/Operation/UpdateOne.php b/src/Operation/UpdateOne.php
@@ -89,7 +89,7 @@ public function __construct(string $databaseName, string $collectionName, $filte
             throw InvalidArgumentException::invalidType('$update', $update, 'array or object');
         }
 
-        if (! is_first_key_operator($update) && ! is_pipeline($update)) {
+        if ($update === [] || ! is_first_key_operator($update) && ! is_pipeline($update)) {
             throw new InvalidArgumentException('Expected an update document with operator as first key or a pipeline');
         }
 
diff --git a/src/functions.php b/src/functions.php
@@ -213,7 +213,7 @@ function is_pipeline($pipeline): bool
     }
 
     if ($pipeline === []) {
-        return false;
+        return true;
     }
 
     $expectedKey = 0;
diff --git a/tests/FunctionsTest.php b/tests/FunctionsTest.php
@@ -225,7 +225,7 @@ public function providePipelines()
     {
         return [
             'Not an array' => [false, (object) []],
-            'Empty array' => [false, []],
+            'Empty array' => [true, []],
             'Non-sequential indexes in array' => [false, [1 => ['$group' => []]]],
             'Update document instead of pipeline' => [false, ['$set' => ['foo' => 'bar']]],
             'Invalid pipeline stage' => [false, [['group' => []]]],
diff --git a/tests/Operation/BulkWriteTest.php b/tests/Operation/BulkWriteTest.php
@@ -236,12 +236,13 @@ public function testUpdateManyUpdateArgumentTypeCheck($update): void
         ]);
     }
 
-    public function testUpdateManyUpdateArgumentRequiresOperatorsOrPipeline(): void
+    /** @dataProvider provideInvalidOperatorsOrPipeline */
+    public function testUpdateManyUpdateArgumentRequiresOperatorsOrPipeline($arg1): void
     {
         $this->expectException(InvalidArgumentException::class);
         $this->expectExceptionMessage('First key in $operations[0]["updateMany"][1] is neither an update operator nor a pipeline');
         new BulkWrite($this->getDatabaseName(), $this->getCollectionName(), [
-            [BulkWrite::UPDATE_MANY => [['_id' => ['$gt' => 1]], ['x' => 1]]],
+            [BulkWrite::UPDATE_MANY => [['_id' => ['$gt' => 1]], $arg1]],
         ]);
     }
 
@@ -313,12 +314,13 @@ public function testUpdateOneUpdateArgumentTypeCheck($update): void
         ]);
     }
 
-    public function testUpdateOneUpdateArgumentRequiresOperatorsOrPipeline(): void
+    /** @dataProvider provideInvalidOperatorsOrPipeline */
+    public function testUpdateOneUpdateArgumentRequiresOperatorsOrPipeline($arg1): void
     {
         $this->expectException(InvalidArgumentException::class);
         $this->expectExceptionMessage('First key in $operations[0]["updateOne"][1] is neither an update operator nor a pipeline');
         new BulkWrite($this->getDatabaseName(), $this->getCollectionName(), [
-            [BulkWrite::UPDATE_ONE => [['_id' => 1], ['x' => 1]]],
+            [BulkWrite::UPDATE_ONE => [['_id' => 1], $arg1]],
         ]);
     }
 
@@ -386,4 +388,12 @@ public function provideInvalidConstructorOptions()
 
         return $options;
     }
+
+    public function provideInvalidOperatorsOrPipeline()
+    {
+        return $this->wrapValuesForDataProvider([
+            [],
+            ['x' => 1],
+        ]);
+    }
 }
diff --git a/tests/Operation/ReplaceOneTest.php b/tests/Operation/ReplaceOneTest.php
@@ -31,11 +31,11 @@ public function testConstructorReplacementArgument($replacement): void
         new ReplaceOne($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], $replacement);
     }
 
-    /** @dataProvider provideUpdateDocuments */
-    public function testConstructorReplacementArgumentRequiresNoOperators($replacement): void
+    /** @dataProvider provideInvalidReplacementDocuments */
+    public function testConstructorReplacementArgumentRequiresDocument($replacement, string $exceptionMessage): void
     {
         $this->expectException(InvalidArgumentException::class);
-        $this->expectExceptionMessage('First key in $replacement argument is an update operator');
+        $this->expectExceptionMessage($exceptionMessage);
         new ReplaceOne($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], $replacement);
     }
 
@@ -48,12 +48,14 @@ public function provideReplacementDocuments()
         ]);
     }
 
-    public function provideUpdateDocuments()
+    public function provideInvalidReplacementDocuments()
     {
-        return $this->wrapValuesForDataProvider([
-            ['$set' => ['y' => 1]],
-            (object) ['$set' => ['y' => 1]],
-            new BSONDocument(['$set' => ['y' => 1]]),
-        ]);
+        return [
+            [[], '$replacement argument is empty'],
+            [['$set' => ['y' => 1]], 'First key in $replacement argument is an update operator'],
+            [(object) ['$set' => ['y' => 1]], 'First key in $replacement argument is an update operator'],
+            [new BSONDocument(['$set' => ['y' => 1]]), 'First key in $replacement argument is an update operator'],
+            [[['$group' => ['_id' => 1]]], '$replacement argument is a pipeline'],
+        ];
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -138,8 +138,7 @@ class Aggregate implements Executable, Explainable`
`138`	`138`	`*/`
`139`	`139`	`public function __construct(string $databaseName, ?string $collectionName, array $pipeline, array $options = [])`
`140`	`140`	`{`
`141`		`- // Empty pipelines are allowed`
`142`		`- if ($pipeline !== [] && ! is_pipeline($pipeline)) {`
	`141`	`+ if (! is_pipeline($pipeline)) {`
`143`	`142`	`throw new InvalidArgumentException('$pipeline is not a valid aggregation pipeline');`
`144`	`143`	`}`
`145`	`144`
Original file line number	Diff line number	Diff line change
`@@ -229,7 +229,7 @@ public function __construct(string $databaseName, string $collectionName, array`
`229`	`229`	`throw InvalidArgumentException::invalidType(sprintf('$operations[%d]["%s"][1]', $i, $type), $args[1], 'array or object');`
`230`	`230`	`}`
`231`	`231`
`232`		`- if (! is_first_key_operator($args[1]) && ! is_pipeline($args[1])) {`
	`232`	`+ if ($args[1] === [] \|\| ! is_first_key_operator($args[1]) && ! is_pipeline($args[1])) {`
`233`	`233`	`throw new InvalidArgumentException(sprintf('First key in $operations[%d]["%s"][1] is neither an update operator nor a pipeline', $i, $type));`
`234`	`234`	`}`
`235`	`235`
Original file line number	Diff line number	Diff line change
`@@ -114,7 +114,7 @@ public function __construct(string $databaseName, string $collectionName, $filte`
`114`	`114`	`throw InvalidArgumentException::invalidType('$update', $update, 'array or object');`
`115`	`115`	`}`
`116`	`116`
`117`		`- if (! is_first_key_operator($update) && ! is_pipeline($update)) {`
	`117`	`+ if ($update === [] \|\| ! is_first_key_operator($update) && ! is_pipeline($update)) {`
`118`	`118`	`throw new InvalidArgumentException('Expected an update document with operator as first key or a pipeline');`
`119`	`119`	`}`
`120`	`120`
Original file line number	Diff line number	Diff line change
`@@ -86,6 +86,10 @@ public function __construct(string $databaseName, string $collectionName, $filte`
`86`	`86`	`throw InvalidArgumentException::invalidType('$replacement', $replacement, 'array or object');`
`87`	`87`	`}`
`88`	`88`
	`89`	`+ if ($replacement === []) {`
	`90`	`+ throw new InvalidArgumentException('$replacement argument is empty');`
	`91`	`+ }`
	`92`	`+`
`89`	`93`	`if (is_first_key_operator($replacement)) {`
`90`	`94`	`throw new InvalidArgumentException('First key in $replacement argument is an update operator');`
`91`	`95`	`}`
Original file line number	Diff line number	Diff line change
`@@ -147,7 +147,7 @@ public function __construct(string $databaseName, string $collectionName, $filte`
`147`	`147`	`throw InvalidArgumentException::invalidType('"multi" option', $options['multi'], 'boolean');`
`148`	`148`	`}`
`149`	`149`
`150`		`- if ($options['multi'] && ! is_first_key_operator($update) && ! is_pipeline($update)) {`
	`150`	`+ if ($options['multi'] && ($update === [] \|\| ! is_first_key_operator($update) && ! is_pipeline($update))) {`
`151`	`151`	`throw new InvalidArgumentException('"multi" option cannot be true if $update is a replacement document');`
`152`	`152`	`}`
`153`	`153`
Original file line number	Diff line number	Diff line change
`@@ -89,7 +89,7 @@ public function __construct(string $databaseName, string $collectionName, $filte`
`89`	`89`	`throw InvalidArgumentException::invalidType('$update', $update, 'array or object');`
`90`	`90`	`}`
`91`	`91`
`92`		`- if (! is_first_key_operator($update) && ! is_pipeline($update)) {`
	`92`	`+ if ($update === [] \|\| ! is_first_key_operator($update) && ! is_pipeline($update)) {`
`93`	`93`	`throw new InvalidArgumentException('Expected an update document with operator as first key or a pipeline');`
`94`	`94`	`}`
`95`	`95`
Original file line number	Diff line number	Diff line change
`@@ -213,7 +213,7 @@ function is_pipeline($pipeline): bool`
`213`	`213`	`}`
`214`	`214`
`215`	`215`	`if ($pipeline === []) {`
`216`		`- return false;`
	`216`	`+ return true;`
`217`	`217`	`}`
`218`	`218`
`219`	`219`	`$expectedKey = 0;`
Original file line number	Diff line number	Diff line change
`@@ -225,7 +225,7 @@ public function providePipelines()`
`225`	`225`	`{`
`226`	`226`	`return [`
`227`	`227`	`'Not an array' => [false, (object) []],`
`228`		`- 'Empty array' => [false, []],`
	`228`	`+ 'Empty array' => [true, []],`
`229`	`229`	`'Non-sequential indexes in array' => [false, [1 => ['$group' => []]]],`
`230`	`230`	`'Update document instead of pipeline' => [false, ['$set' => ['foo' => 'bar']]],`
`231`	`231`	`'Invalid pipeline stage' => [false, [['group' => []]]],`
Original file line number	Diff line number	Diff line change
`@@ -236,12 +236,13 @@ public function testUpdateManyUpdateArgumentTypeCheck($update): void`
`236`	`236`	`]);`
`237`	`237`	`}`
`238`	`238`
`239`		`- public function testUpdateManyUpdateArgumentRequiresOperatorsOrPipeline(): void`
	`239`	`+ /** @dataProvider provideInvalidOperatorsOrPipeline */`
	`240`	`+ public function testUpdateManyUpdateArgumentRequiresOperatorsOrPipeline($arg1): void`
`240`	`241`	`{`
`241`	`242`	`$this->expectException(InvalidArgumentException::class);`
`242`	`243`	`$this->expectExceptionMessage('First key in $operations[0]["updateMany"][1] is neither an update operator nor a pipeline');`
`243`	`244`	`new BulkWrite($this->getDatabaseName(), $this->getCollectionName(), [`
`244`		`- [BulkWrite::UPDATE_MANY => [['_id' => ['$gt' => 1]], ['x' => 1]]],`
	`245`	`+ [BulkWrite::UPDATE_MANY => [['_id' => ['$gt' => 1]], $arg1]],`
`245`	`246`	`]);`
`246`	`247`	`}`
`247`	`248`
`@@ -313,12 +314,13 @@ public function testUpdateOneUpdateArgumentTypeCheck($update): void`
`313`	`314`	`]);`
`314`	`315`	`}`
`315`	`316`
`316`		`- public function testUpdateOneUpdateArgumentRequiresOperatorsOrPipeline(): void`
	`317`	`+ /** @dataProvider provideInvalidOperatorsOrPipeline */`
	`318`	`+ public function testUpdateOneUpdateArgumentRequiresOperatorsOrPipeline($arg1): void`
`317`	`319`	`{`
`318`	`320`	`$this->expectException(InvalidArgumentException::class);`
`319`	`321`	`$this->expectExceptionMessage('First key in $operations[0]["updateOne"][1] is neither an update operator nor a pipeline');`
`320`	`322`	`new BulkWrite($this->getDatabaseName(), $this->getCollectionName(), [`
`321`		`- [BulkWrite::UPDATE_ONE => [['_id' => 1], ['x' => 1]]],`
	`323`	`+ [BulkWrite::UPDATE_ONE => [['_id' => 1], $arg1]],`
`322`	`324`	`]);`
`323`	`325`	`}`
`324`	`326`
`@@ -386,4 +388,12 @@ public function provideInvalidConstructorOptions()`
`386`	`388`
`387`	`389`	`return $options;`
`388`	`390`	`}`
	`391`	`+`
	`392`	`+ public function provideInvalidOperatorsOrPipeline()`
	`393`	`+ {`
	`394`	`+ return $this->wrapValuesForDataProvider([`
	`395`	`+ [],`
	`396`	`+ ['x' => 1],`
	`397`	`+ ]);`
	`398`	`+ }`
`389`	`399`	`}`
Original file line number	Diff line number	Diff line change
`@@ -31,11 +31,11 @@ public function testConstructorReplacementArgument($replacement): void`
`31`	`31`	`new ReplaceOne($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], $replacement);`
`32`	`32`	`}`
`33`	`33`
`34`		`- /** @dataProvider provideUpdateDocuments */`
`35`		`- public function testConstructorReplacementArgumentRequiresNoOperators($replacement): void`
	`34`	`+ /** @dataProvider provideInvalidReplacementDocuments */`
	`35`	`+ public function testConstructorReplacementArgumentRequiresDocument($replacement, string $exceptionMessage): void`
`36`	`36`	`{`
`37`	`37`	`$this->expectException(InvalidArgumentException::class);`
`38`		`- $this->expectExceptionMessage('First key in $replacement argument is an update operator');`
	`38`	`+ $this->expectExceptionMessage($exceptionMessage);`
`39`	`39`	`new ReplaceOne($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], $replacement);`
`40`	`40`	`}`
`41`	`41`
`@@ -48,12 +48,14 @@ public function provideReplacementDocuments()`
`48`	`48`	`]);`
`49`	`49`	`}`
`50`	`50`
`51`		`- public function provideUpdateDocuments()`
	`51`	`+ public function provideInvalidReplacementDocuments()`
`52`	`52`	`{`
`53`		`- return $this->wrapValuesForDataProvider([`
`54`		`- ['$set' => ['y' => 1]],`
`55`		`- (object) ['$set' => ['y' => 1]],`
`56`		`- new BSONDocument(['$set' => ['y' => 1]]),`
`57`		`- ]);`
	`53`	`+ return [`
	`54`	`+ [[], '$replacement argument is empty'],`
	`55`	`+ [['$set' => ['y' => 1]], 'First key in $replacement argument is an update operator'],`
	`56`	`+ [(object) ['$set' => ['y' => 1]], 'First key in $replacement argument is an update operator'],`
	`57`	`+ [new BSONDocument(['$set' => ['y' => 1]]), 'First key in $replacement argument is an update operator'],`
	`58`	`+ [[['$group' => ['_id' => 1]]], '$replacement argument is a pipeline'],`
	`59`	`+ ];`
`58`	`60`	`}`
`59`	`61`	`}`