From fca04ae1df6b2627e005fa99451b08367e9ad792 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 11 Nov 2019 15:41:23 +0100 Subject: [PATCH 1/2] PHPLIB-494: Test against null for options with default values --- tests/ClientTest.php | 2 +- tests/GridFS/BucketFunctionalTest.php | 6 +- tests/GridFS/WritableStreamFunctionalTest.php | 4 +- tests/Operation/AggregateTest.php | 4 +- tests/Operation/BulkWriteTest.php | 2 +- tests/Operation/FindAndModifyTest.php | 6 +- tests/Operation/FindOneAndReplaceTest.php | 2 +- tests/Operation/FindOneAndUpdateTest.php | 2 +- tests/Operation/InsertManyTest.php | 2 +- tests/Operation/UpdateTest.php | 4 +- tests/Operation/WatchTest.php | 4 +- tests/TestCase.php | 55 +++++++++++++------ 12 files changed, 56 insertions(+), 37 deletions(-) diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 372ad0b6f..d40e5497d 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -33,7 +33,7 @@ public function provideInvalidConstructorDriverOptions() { $options = []; - foreach ($this->getInvalidArrayValues() as $value) { + foreach ($this->getInvalidArrayValues(true) as $value) { $options[][] = ['typeMap' => $value]; } diff --git a/tests/GridFS/BucketFunctionalTest.php b/tests/GridFS/BucketFunctionalTest.php index c94be7245..c5cdfcd1a 100644 --- a/tests/GridFS/BucketFunctionalTest.php +++ b/tests/GridFS/BucketFunctionalTest.php @@ -62,15 +62,15 @@ public function provideInvalidConstructorOptions() { $options = []; - foreach ($this->getInvalidStringValues() as $value) { + foreach ($this->getInvalidStringValues(true) as $value) { $options[][] = ['bucketName' => $value]; } - foreach ($this->getInvalidIntegerValues() as $value) { + foreach ($this->getInvalidIntegerValues(true) as $value) { $options[][] = ['chunkSizeBytes' => $value]; } - foreach ($this->getInvalidBooleanValues() as $value) { + foreach ($this->getInvalidBooleanValues(true) as $value) { $options[][] = ['disableMD5' => $value]; } diff --git a/tests/GridFS/WritableStreamFunctionalTest.php b/tests/GridFS/WritableStreamFunctionalTest.php index e41141e1a..cadee3ad6 100644 --- a/tests/GridFS/WritableStreamFunctionalTest.php +++ b/tests/GridFS/WritableStreamFunctionalTest.php @@ -50,11 +50,11 @@ public function provideInvalidConstructorOptions() { $options = []; - foreach ($this->getInvalidIntegerValues() as $value) { + foreach ($this->getInvalidIntegerValues(true) as $value) { $options[][] = ['chunkSizeBytes' => $value]; } - foreach ($this->getInvalidBooleanValues() as $value) { + foreach ($this->getInvalidBooleanValues(true) as $value) { $options[][] = ['disableMD5' => $value]; } diff --git a/tests/Operation/AggregateTest.php b/tests/Operation/AggregateTest.php index 2838cb502..d8898b321 100644 --- a/tests/Operation/AggregateTest.php +++ b/tests/Operation/AggregateTest.php @@ -27,7 +27,7 @@ public function provideInvalidConstructorOptions() { $options = []; - foreach ($this->getInvalidBooleanValues() as $value) { + foreach ($this->getInvalidBooleanValues(true) as $value) { $options[][] = ['allowDiskUse' => $value]; } @@ -79,7 +79,7 @@ public function provideInvalidConstructorOptions() $options[][] = ['typeMap' => $value]; } - foreach ($this->getInvalidBooleanValues() as $value) { + foreach ($this->getInvalidBooleanValues(true) as $value) { $options[][] = ['useCursor' => $value]; } diff --git a/tests/Operation/BulkWriteTest.php b/tests/Operation/BulkWriteTest.php index 76b395c64..bd9069d8d 100644 --- a/tests/Operation/BulkWriteTest.php +++ b/tests/Operation/BulkWriteTest.php @@ -412,7 +412,7 @@ public function provideInvalidConstructorOptions() $options[][] = ['bypassDocumentValidation' => $value]; } - foreach ($this->getInvalidBooleanValues() as $value) { + foreach ($this->getInvalidBooleanValues(true) as $value) { $options[][] = ['ordered' => $value]; } diff --git a/tests/Operation/FindAndModifyTest.php b/tests/Operation/FindAndModifyTest.php index bb8a443b1..7f5cb99cc 100644 --- a/tests/Operation/FindAndModifyTest.php +++ b/tests/Operation/FindAndModifyTest.php @@ -40,7 +40,7 @@ public function provideInvalidConstructorOptions() $options[][] = ['maxTimeMS' => $value]; } - foreach ($this->getInvalidBooleanValues() as $value) { + foreach ($this->getInvalidBooleanValues(true) as $value) { $options[][] = ['new' => $value]; } @@ -48,7 +48,7 @@ public function provideInvalidConstructorOptions() $options[][] = ['query' => $value]; } - foreach ($this->getInvalidBooleanValues() as $value) { + foreach ($this->getInvalidBooleanValues(true) as $value) { $options[][] = ['remove' => $value]; } @@ -68,7 +68,7 @@ public function provideInvalidConstructorOptions() $options[][] = ['update' => $value]; } - foreach ($this->getInvalidBooleanValues() as $value) { + foreach ($this->getInvalidBooleanValues(true) as $value) { $options[][] = ['upsert' => $value]; } diff --git a/tests/Operation/FindOneAndReplaceTest.php b/tests/Operation/FindOneAndReplaceTest.php index 7d4d18ee9..974d413a3 100644 --- a/tests/Operation/FindOneAndReplaceTest.php +++ b/tests/Operation/FindOneAndReplaceTest.php @@ -49,7 +49,7 @@ public function provideInvalidConstructorOptions() $options[][] = ['projection' => $value]; } - foreach ($this->getInvalidIntegerValues() as $value) { + foreach ($this->getInvalidIntegerValues(true) as $value) { $options[][] = ['returnDocument' => $value]; } diff --git a/tests/Operation/FindOneAndUpdateTest.php b/tests/Operation/FindOneAndUpdateTest.php index 206284ef2..1391b7623 100644 --- a/tests/Operation/FindOneAndUpdateTest.php +++ b/tests/Operation/FindOneAndUpdateTest.php @@ -49,7 +49,7 @@ public function provideInvalidConstructorOptions() $options[][] = ['projection' => $value]; } - foreach ($this->getInvalidIntegerValues() as $value) { + foreach ($this->getInvalidIntegerValues(true) as $value) { $options[][] = ['returnDocument' => $value]; } diff --git a/tests/Operation/InsertManyTest.php b/tests/Operation/InsertManyTest.php index 6ee731126..0226c0014 100644 --- a/tests/Operation/InsertManyTest.php +++ b/tests/Operation/InsertManyTest.php @@ -48,7 +48,7 @@ public function provideInvalidConstructorOptions() $options[][] = ['bypassDocumentValidation' => $value]; } - foreach ($this->getInvalidBooleanValues() as $value) { + foreach ($this->getInvalidBooleanValues(true) as $value) { $options[][] = ['ordered' => $value]; } diff --git a/tests/Operation/UpdateTest.php b/tests/Operation/UpdateTest.php index eb3760ffc..76cd18b99 100644 --- a/tests/Operation/UpdateTest.php +++ b/tests/Operation/UpdateTest.php @@ -52,7 +52,7 @@ public function provideInvalidConstructorOptions() $options[][] = ['collation' => $value]; } - foreach ($this->getInvalidBooleanValues() as $value) { + foreach ($this->getInvalidBooleanValues(true) as $value) { $options[][] = ['multi' => $value]; } @@ -60,7 +60,7 @@ public function provideInvalidConstructorOptions() $options[][] = ['session' => $value]; } - foreach ($this->getInvalidBooleanValues() as $value) { + foreach ($this->getInvalidBooleanValues(true) as $value) { $options[][] = ['upsert' => $value]; } diff --git a/tests/Operation/WatchTest.php b/tests/Operation/WatchTest.php index 733284a88..8cf94254a 100644 --- a/tests/Operation/WatchTest.php +++ b/tests/Operation/WatchTest.php @@ -52,7 +52,7 @@ public function provideInvalidConstructorOptions() $options[][] = ['collation' => $value]; } - foreach ($this->getInvalidStringValues() as $value) { + foreach ($this->getInvalidStringValues(true) as $value) { $options[][] = ['fullDocument' => $value]; } @@ -64,7 +64,7 @@ public function provideInvalidConstructorOptions() $options[][] = ['readConcern' => $value]; } - foreach ($this->getInvalidReadPreferenceValues() as $value) { + foreach ($this->getInvalidReadPreferenceValues(true) as $value) { $options[][] = ['readPreference' => $value]; } diff --git a/tests/TestCase.php b/tests/TestCase.php index 10d71c55a..a5cbb6f83 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -14,6 +14,7 @@ use stdClass; use Traversable; use function array_map; +use function array_merge; use function array_values; use function call_user_func; use function getenv; @@ -165,91 +166,109 @@ protected function getDatabaseName() /** * Return a list of invalid array values. * + * @param boolean $includeNull + * * @return array */ - protected function getInvalidArrayValues() + protected function getInvalidArrayValues($includeNull = false) { - return [123, 3.14, 'foo', true, new stdClass()]; + return array_merge([123, 3.14, 'foo', true, new stdClass()], $includeNull ? [null] : []); } /** * Return a list of invalid boolean values. * + * @param boolean $includeNull + * * @return array */ - protected function getInvalidBooleanValues() + protected function getInvalidBooleanValues($includeNull = false) { - return [123, 3.14, 'foo', [], new stdClass()]; + return array_merge([123, 3.14, 'foo', [], new stdClass()], $includeNull ? [null] : []); } /** * Return a list of invalid document values. * + * @param boolean $includeNull + * * @return array */ - protected function getInvalidDocumentValues() + protected function getInvalidDocumentValues($includeNull = false) { - return [123, 3.14, 'foo', true]; + return array_merge([123, 3.14, 'foo', true], $includeNull ? [null] : []); } /** * Return a list of invalid integer values. * + * @param boolean $includeNull + * * @return array */ - protected function getInvalidIntegerValues() + protected function getInvalidIntegerValues($includeNull = false) { - return [3.14, 'foo', true, [], new stdClass()]; + return array_merge([3.14, 'foo', true, [], new stdClass()], $includeNull ? [null] : []); } /** * Return a list of invalid ReadPreference values. * + * @param boolean $includeNull + * * @return array */ - protected function getInvalidReadConcernValues() + protected function getInvalidReadConcernValues($includeNull = false) { - return [123, 3.14, 'foo', true, [], new stdClass(), new ReadPreference(ReadPreference::RP_PRIMARY), new WriteConcern(1)]; + return array_merge([123, 3.14, 'foo', true, [], new stdClass(), new ReadPreference(ReadPreference::RP_PRIMARY), new WriteConcern(1)], $includeNull ? [null] : []); } /** * Return a list of invalid ReadPreference values. * + * @param boolean $includeNull + * * @return array */ - protected function getInvalidReadPreferenceValues() + protected function getInvalidReadPreferenceValues($includeNull = false) { - return [123, 3.14, 'foo', true, [], new stdClass(), new ReadConcern(), new WriteConcern(1)]; + return array_merge([123, 3.14, 'foo', true, [], new stdClass(), new ReadConcern(), new WriteConcern(1)], $includeNull ? [null] : []); } /** * Return a list of invalid Session values. * + * @param boolean $includeNull + * * @return array */ - protected function getInvalidSessionValues() + protected function getInvalidSessionValues($includeNull = false) { - return [123, 3.14, 'foo', true, [], new stdClass(), new ReadConcern(), new ReadPreference(ReadPreference::RP_PRIMARY), new WriteConcern(1)]; + return array_merge([123, 3.14, 'foo', true, [], new stdClass(), new ReadConcern(), new ReadPreference(ReadPreference::RP_PRIMARY), new WriteConcern(1)], $includeNull ? [null] : []); } /** * Return a list of invalid string values. * + * @param boolean $includeNull + * * @return array */ - protected function getInvalidStringValues() + protected function getInvalidStringValues($includeNull = false) { - return [123, 3.14, true, [], new stdClass()]; + return array_merge([123, 3.14, true, [], new stdClass()], $includeNull ? [null] : []); } /** * Return a list of invalid WriteConcern values. * + * @param boolean $includeNull + * * @return array */ - protected function getInvalidWriteConcernValues() + protected function getInvalidWriteConcernValues($includeNull = false) { - return [123, 3.14, 'foo', true, [], new stdClass(), new ReadConcern(), new ReadPreference(ReadPreference::RP_PRIMARY)]; + return array_merge([123, 3.14, 'foo', true, [], new stdClass(), new ReadConcern(), new ReadPreference(ReadPreference::RP_PRIMARY)], $includeNull ? [null] : []); } /** From 86ff7317d091e1fa700d6414afcf4b0032ca2fa6 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 11 Nov 2019 15:41:40 +0100 Subject: [PATCH 2/2] PHPLIB-494: Fix validation of options with default values --- src/Client.php | 2 +- src/GridFS/Bucket.php | 8 ++++---- src/GridFS/WritableStream.php | 6 +++--- src/Operation/Watch.php | 6 +++++- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/Client.php b/src/Client.php index 20310802d..dc5d1fa79 100644 --- a/src/Client.php +++ b/src/Client.php @@ -95,7 +95,7 @@ public function __construct($uri = 'mongodb://127.0.0.1/', array $uriOptions = [ { $driverOptions += ['typeMap' => self::$defaultTypeMap]; - if (isset($driverOptions['typeMap']) && ! is_array($driverOptions['typeMap'])) { + if (! is_array($driverOptions['typeMap'])) { throw InvalidArgumentException::invalidType('"typeMap" driver option', $driverOptions['typeMap'], 'array'); } diff --git a/src/GridFS/Bucket.php b/src/GridFS/Bucket.php index dc1532ce8..97a6192d7 100644 --- a/src/GridFS/Bucket.php +++ b/src/GridFS/Bucket.php @@ -143,19 +143,19 @@ public function __construct(Manager $manager, $databaseName, array $options = [] 'disableMD5' => false, ]; - if (isset($options['bucketName']) && ! is_string($options['bucketName'])) { + if (! is_string($options['bucketName'])) { throw InvalidArgumentException::invalidType('"bucketName" option', $options['bucketName'], 'string'); } - if (isset($options['chunkSizeBytes']) && ! is_integer($options['chunkSizeBytes'])) { + if (! is_integer($options['chunkSizeBytes'])) { throw InvalidArgumentException::invalidType('"chunkSizeBytes" option', $options['chunkSizeBytes'], 'integer'); } - if (isset($options['chunkSizeBytes']) && $options['chunkSizeBytes'] < 1) { + if ($options['chunkSizeBytes'] < 1) { throw new InvalidArgumentException(sprintf('Expected "chunkSizeBytes" option to be >= 1, %d given', $options['chunkSizeBytes'])); } - if (isset($options['disableMD5']) && ! is_bool($options['disableMD5'])) { + if (! is_bool($options['disableMD5'])) { throw InvalidArgumentException::invalidType('"disableMD5" option', $options['disableMD5'], 'boolean'); } diff --git a/src/GridFS/WritableStream.php b/src/GridFS/WritableStream.php index d7f1e4c47..2e12285dd 100644 --- a/src/GridFS/WritableStream.php +++ b/src/GridFS/WritableStream.php @@ -114,15 +114,15 @@ public function __construct(CollectionWrapper $collectionWrapper, $filename, arr throw InvalidArgumentException::invalidType('"aliases" option', $options['aliases'], 'array of strings'); } - if (isset($options['chunkSizeBytes']) && ! is_integer($options['chunkSizeBytes'])) { + if (! is_integer($options['chunkSizeBytes'])) { throw InvalidArgumentException::invalidType('"chunkSizeBytes" option', $options['chunkSizeBytes'], 'integer'); } - if (isset($options['chunkSizeBytes']) && $options['chunkSizeBytes'] < 1) { + if ($options['chunkSizeBytes'] < 1) { throw new InvalidArgumentException(sprintf('Expected "chunkSizeBytes" option to be >= 1, %d given', $options['chunkSizeBytes'])); } - if (isset($options['disableMD5']) && ! is_bool($options['disableMD5'])) { + if (! is_bool($options['disableMD5'])) { throw InvalidArgumentException::invalidType('"disableMD5" option', $options['disableMD5'], 'boolean'); } diff --git a/src/Operation/Watch.php b/src/Operation/Watch.php index ef301603b..1c88b87be 100644 --- a/src/Operation/Watch.php +++ b/src/Operation/Watch.php @@ -178,10 +178,14 @@ public function __construct(Manager $manager, $databaseName, $collectionName, ar 'readPreference' => new ReadPreference(ReadPreference::RP_PRIMARY), ]; - if (isset($options['fullDocument']) && ! is_string($options['fullDocument'])) { + if (! is_string($options['fullDocument'])) { throw InvalidArgumentException::invalidType('"fullDocument" option', $options['fullDocument'], 'string'); } + if (! $options['readPreference'] instanceof ReadPreference) { + throw InvalidArgumentException::invalidType('"readPreference" option', $options['readPreference'], ReadPreference::class); + } + if (isset($options['resumeAfter']) && ! is_array($options['resumeAfter']) && ! is_object($options['resumeAfter'])) { throw InvalidArgumentException::invalidType('"resumeAfter" option', $options['resumeAfter'], 'array or object'); }