From 1d831e2ebc74e373fe5321a02ed0005c944a75c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 18 Sep 2024 13:45:11 +0200 Subject: [PATCH 1/7] Split GridFS bucket tests to improve performance of unit tests --- tests/GridFS/BucketFunctionalTest.php | 162 ----------------- tests/GridFS/BucketTest.php | 240 ++++++++++++++++++++++++++ 2 files changed, 240 insertions(+), 162 deletions(-) create mode 100644 tests/GridFS/BucketTest.php diff --git a/tests/GridFS/BucketFunctionalTest.php b/tests/GridFS/BucketFunctionalTest.php index 2f2d126d4..f9298c2ca 100644 --- a/tests/GridFS/BucketFunctionalTest.php +++ b/tests/GridFS/BucketFunctionalTest.php @@ -3,24 +3,16 @@ namespace MongoDB\Tests\GridFS; use MongoDB\BSON\Binary; -use MongoDB\BSON\ObjectId; -use MongoDB\Collection; -use MongoDB\Driver\ReadConcern; -use MongoDB\Driver\ReadPreference; -use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\GridFS\Bucket; -use MongoDB\GridFS\CollectionWrapper; use MongoDB\GridFS\Exception\CorruptFileException; use MongoDB\GridFS\Exception\FileNotFoundException; use MongoDB\GridFS\Exception\StreamException; use MongoDB\Model\BSONDocument; use MongoDB\Model\IndexInfo; use MongoDB\Operation\ListIndexes; -use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; use MongoDB\Tests\Fixtures\Codec\TestFileCodec; use MongoDB\Tests\Fixtures\Document\TestFile; -use ReflectionMethod; use stdClass; use function array_merge; @@ -51,58 +43,6 @@ */ class BucketFunctionalTest extends FunctionalTestCase { - /** @doesNotPerformAssertions */ - public function testValidConstructorOptions(): void - { - new Bucket($this->manager, $this->getDatabaseName(), [ - 'bucketName' => 'test', - 'chunkSizeBytes' => 8192, - 'readConcern' => new ReadConcern(ReadConcern::LOCAL), - 'readPreference' => new ReadPreference(ReadPreference::PRIMARY), - 'writeConcern' => new WriteConcern(WriteConcern::MAJORITY, 1000), - 'disableMD5' => true, - ]); - } - - /** @dataProvider provideInvalidConstructorOptions */ - public function testConstructorOptionTypeChecks(array $options): void - { - $this->expectException(InvalidArgumentException::class); - new Bucket($this->manager, $this->getDatabaseName(), $options); - } - - public static function provideInvalidConstructorOptions() - { - return self::createOptionDataProvider([ - 'bucketName' => self::getInvalidStringValues(true), - 'chunkSizeBytes' => self::getInvalidIntegerValues(true), - 'codec' => self::getInvalidDocumentCodecValues(), - 'disableMD5' => self::getInvalidBooleanValues(true), - 'readConcern' => self::getInvalidReadConcernValues(), - 'readPreference' => self::getInvalidReadPreferenceValues(), - 'typeMap' => self::getInvalidArrayValues(), - 'writeConcern' => self::getInvalidWriteConcernValues(), - ]); - } - - public function testConstructorShouldRequireChunkSizeBytesOptionToBePositive(): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Expected "chunkSizeBytes" option to be >= 1, 0 given'); - new Bucket($this->manager, $this->getDatabaseName(), ['chunkSizeBytes' => 0]); - } - - public function testConstructorWithCodecAndTypeMapOptions(): void - { - $options = [ - 'codec' => new TestDocumentCodec(), - 'typeMap' => ['root' => 'array', 'document' => 'array'], - ]; - - $this->expectExceptionObject(InvalidArgumentException::cannotCombineCodecAndTypeMap()); - new Bucket($this->manager, $this->getDatabaseName(), $options); - } - /** @dataProvider provideInputDataAndExpectedChunks */ public function testDelete($input, $expectedChunks): void { @@ -207,13 +147,6 @@ public function testDownloadToStream($input): void $this->assertStreamContents($input, $destination); } - /** @dataProvider provideInvalidStreamValues */ - public function testDownloadToStreamShouldRequireDestinationStream($destination): void - { - $this->expectException(InvalidArgumentException::class); - $this->bucket->downloadToStream('id', $destination); - } - public static function provideInvalidStreamValues(): array { return self::wrapValuesForDataProvider(self::getInvalidStreamValues()); @@ -260,13 +193,6 @@ public function testDownloadToStreamByName(): void $this->assertStreamContents('baz', $destination); } - /** @dataProvider provideInvalidStreamValues */ - public function testDownloadToStreamByNameShouldRequireDestinationStream($destination): void - { - $this->expectException(InvalidArgumentException::class); - $this->bucket->downloadToStreamByName('filename', $destination); - } - /** @dataProvider provideNonexistentFilenameAndRevision */ public function testDownloadToStreamByNameShouldRequireFilenameAndRevisionToExist($filename, $revision): void { @@ -452,43 +378,6 @@ public function testFindOneResetsInheritedBucketCodec(): void $this->assertSame(6, $fileDocument->length); } - public function testGetBucketNameWithCustomValue(): void - { - $bucket = new Bucket($this->manager, $this->getDatabaseName(), ['bucketName' => 'custom_fs']); - - $this->assertEquals('custom_fs', $bucket->getBucketName()); - } - - public function testGetBucketNameWithDefaultValue(): void - { - $this->assertEquals('fs', $this->bucket->getBucketName()); - } - - public function testGetChunksCollection(): void - { - $chunksCollection = $this->bucket->getChunksCollection(); - - $this->assertInstanceOf(Collection::class, $chunksCollection); - $this->assertEquals('fs.chunks', $chunksCollection->getCollectionName()); - } - - public function testGetChunkSizeBytesWithCustomValue(): void - { - $bucket = new Bucket($this->manager, $this->getDatabaseName(), ['chunkSizeBytes' => 8192]); - - $this->assertEquals(8192, $bucket->getChunkSizeBytes()); - } - - public function testGetChunkSizeBytesWithDefaultValue(): void - { - $this->assertEquals(261120, $this->bucket->getChunkSizeBytes()); - } - - public function testGetDatabaseName(): void - { - $this->assertEquals($this->getDatabaseName(), $this->bucket->getDatabaseName()); - } - public function testGetFileDocumentForStreamUsesTypeMap(): void { $metadata = ['foo' => 'bar']; @@ -580,21 +469,6 @@ public function testGetFileIdForStreamWithWritableStream(): void $this->assertEquals(1, $this->bucket->getFileIdForStream($stream)); } - /** @dataProvider provideInvalidGridFSStreamValues */ - public function testGetFileIdForStreamShouldRequireGridFSStreamResource($stream): void - { - $this->expectException(InvalidArgumentException::class); - $this->bucket->getFileIdForStream($stream); - } - - public function testGetFilesCollection(): void - { - $filesCollection = $this->bucket->getFilesCollection(); - - $this->assertInstanceOf(Collection::class, $filesCollection); - $this->assertEquals('fs.files', $filesCollection->getCollectionName()); - } - /** @dataProvider provideInputDataAndExpectedChunks */ public function testOpenDownloadStream($input): void { @@ -937,42 +811,6 @@ public function testDanglingOpenWritableStreamWithGlobalStreamWrapperAlias(): vo $this->assertSame(14, $fileDocument->length); } - public function testResolveStreamContextForRead(): void - { - $stream = $this->bucket->openUploadStream('filename'); - fwrite($stream, 'foobar'); - fclose($stream); - - $method = new ReflectionMethod($this->bucket, 'resolveStreamContext'); - $method->setAccessible(true); - - $context = $method->invokeArgs($this->bucket, ['gridfs://bucket/filename', 'rb', []]); - - $this->assertIsArray($context); - $this->assertArrayHasKey('collectionWrapper', $context); - $this->assertInstanceOf(CollectionWrapper::class, $context['collectionWrapper']); - $this->assertArrayHasKey('file', $context); - $this->assertIsObject($context['file']); - $this->assertInstanceOf(ObjectId::class, $context['file']->_id); - $this->assertSame('filename', $context['file']->filename); - } - - public function testResolveStreamContextForWrite(): void - { - $method = new ReflectionMethod($this->bucket, 'resolveStreamContext'); - $method->setAccessible(true); - - $context = $method->invokeArgs($this->bucket, ['gridfs://bucket/filename', 'wb', []]); - - $this->assertIsArray($context); - $this->assertArrayHasKey('collectionWrapper', $context); - $this->assertInstanceOf(CollectionWrapper::class, $context['collectionWrapper']); - $this->assertArrayHasKey('filename', $context); - $this->assertSame('filename', $context['filename']); - $this->assertArrayHasKey('options', $context); - $this->assertSame(['chunkSizeBytes' => 261120, 'disableMD5' => false], $context['options']); - } - /** * Asserts that an index with the given name exists for the collection. * diff --git a/tests/GridFS/BucketTest.php b/tests/GridFS/BucketTest.php new file mode 100644 index 000000000..29bc35a8c --- /dev/null +++ b/tests/GridFS/BucketTest.php @@ -0,0 +1,240 @@ +manager = new Manager('mongodb://server.test:27017'); + } + + /** @doesNotPerformAssertions */ + public function testValidConstructorOptions(): void + { + new Bucket($this->manager, $this->getDatabaseName(), [ + 'bucketName' => 'test', + 'chunkSizeBytes' => 8192, + 'readConcern' => new ReadConcern(ReadConcern::LOCAL), + 'readPreference' => new ReadPreference(ReadPreference::PRIMARY), + 'writeConcern' => new WriteConcern(WriteConcern::MAJORITY, 1000), + 'disableMD5' => true, + ]); + } + + /** @dataProvider provideInvalidConstructorOptions */ + public function testConstructorOptionTypeChecks(array $options): void + { + $this->expectException(InvalidArgumentException::class); + new Bucket($this->manager, $this->getDatabaseName(), $options); + } + + public static function provideInvalidConstructorOptions() + { + return self::createOptionDataProvider([ + 'bucketName' => self::getInvalidStringValues(true), + 'chunkSizeBytes' => self::getInvalidIntegerValues(true), + 'codec' => self::getInvalidDocumentCodecValues(), + 'disableMD5' => self::getInvalidBooleanValues(true), + 'readConcern' => self::getInvalidReadConcernValues(), + 'readPreference' => self::getInvalidReadPreferenceValues(), + 'typeMap' => self::getInvalidArrayValues(), + 'writeConcern' => self::getInvalidWriteConcernValues(), + ]); + } + + public function testConstructorShouldRequireChunkSizeBytesOptionToBePositive(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Expected "chunkSizeBytes" option to be >= 1, 0 given'); + new Bucket($this->manager, $this->getDatabaseName(), ['chunkSizeBytes' => 0]); + } + + public function testConstructorWithCodecAndTypeMapOptions(): void + { + $options = [ + 'codec' => new TestDocumentCodec(), + 'typeMap' => ['root' => 'array', 'document' => 'array'], + ]; + + $this->expectExceptionObject(InvalidArgumentException::cannotCombineCodecAndTypeMap()); + new Bucket($this->manager, $this->getDatabaseName(), $options); + } + + public static function provideInputDataAndExpectedChunks() + { + return [ + ['', 0], + ['foobar', 1], + [str_repeat('a', 261120), 1], + [str_repeat('a', 261121), 2], + [str_repeat('a', 522240), 2], + [str_repeat('a', 522241), 3], + [str_repeat('foobar', 43520), 1], + [str_repeat('foobar', 43521), 2], + [str_repeat('foobar', 87040), 2], + [str_repeat('foobar', 87041), 3], + ]; + } + + /** @dataProvider provideInvalidStreamValues */ + public function testDownloadToStreamShouldRequireDestinationStream($destination): void + { + $bucket = new Bucket($this->manager, $this->getDatabaseName()); + $this->expectException(InvalidArgumentException::class); + $bucket->downloadToStream('id', $destination); + } + + public static function provideInvalidStreamValues(): array + { + return self::wrapValuesForDataProvider(self::getInvalidStreamValues()); + } + + /** @dataProvider provideInvalidStreamValues */ + public function testDownloadToStreamByNameShouldRequireDestinationStream($destination): void + { + $bucket = new Bucket($this->manager, $this->getDatabaseName()); + $this->expectException(InvalidArgumentException::class); + $bucket->downloadToStreamByName('filename', $destination); + } + + public static function provideNonexistentFilenameAndRevision() + { + return [ + ['filename', 2], + ['filename', -3], + ['nonexistent-filename', 0], + ['nonexistent-filename', -1], + ]; + } + + public function testGetBucketNameWithCustomValue(): void + { + $bucket = new Bucket($this->manager, $this->getDatabaseName(), ['bucketName' => 'custom_fs']); + $this->assertEquals('custom_fs', $bucket->getBucketName()); + } + + public function testGetBucketNameWithDefaultValue(): void + { + $bucket = new Bucket($this->manager, $this->getDatabaseName()); + $this->assertEquals('fs', $bucket->getBucketName()); + } + + public function testGetChunksCollection(): void + { + $bucket = new Bucket($this->manager, $this->getDatabaseName()); + $chunksCollection = $bucket->getChunksCollection(); + + $this->assertInstanceOf(Collection::class, $chunksCollection); + $this->assertEquals('fs.chunks', $chunksCollection->getCollectionName()); + } + + public function testGetChunkSizeBytesWithCustomValue(): void + { + $bucket = new Bucket($this->manager, $this->getDatabaseName(), ['chunkSizeBytes' => 8192]); + + $this->assertEquals(8192, $bucket->getChunkSizeBytes()); + } + + public function testGetChunkSizeBytesWithDefaultValue(): void + { + $bucket = new Bucket($this->manager, $this->getDatabaseName()); + $this->assertEquals(261120, $bucket->getChunkSizeBytes()); + } + + public function testGetDatabaseName(): void + { + $bucket = new Bucket($this->manager, $this->getDatabaseName()); + $this->assertEquals($this->getDatabaseName(), $bucket->getDatabaseName()); + } + + public static function provideInvalidGridFSStreamValues(): array + { + return self::wrapValuesForDataProvider(array_merge(self::getInvalidStreamValues(), [fopen('php://temp', 'r')])); + } + + /** @dataProvider provideInvalidGridFSStreamValues */ + public function testGetFileIdForStreamShouldRequireGridFSStreamResource($stream): void + { + $bucket = new Bucket($this->manager, $this->getDatabaseName()); + $this->expectException(InvalidArgumentException::class); + $bucket->getFileIdForStream($stream); + } + + public function testGetFilesCollection(): void + { + $bucket = new Bucket($this->manager, $this->getDatabaseName()); + $filesCollection = $bucket->getFilesCollection(); + + $this->assertInstanceOf(Collection::class, $filesCollection); + $this->assertEquals('fs.files', $filesCollection->getCollectionName()); + } + + public function testResolveStreamContextForRead(): void + { + $bucket = new Bucket($this->manager, $this->getDatabaseName()); + $stream = $bucket->openUploadStream('filename'); + fwrite($stream, 'foobar'); + fclose($stream); + $method = new ReflectionMethod($bucket, 'resolveStreamContext'); + $context = $method->invokeArgs($bucket, ['gridfs://bucket/filename', 'rb', []]); + + $this->assertIsArray($context); + $this->assertArrayHasKey('collectionWrapper', $context); + $this->assertInstanceOf(CollectionWrapper::class, $context['collectionWrapper']); + $this->assertArrayHasKey('file', $context); + $this->assertIsObject($context['file']); + $this->assertInstanceOf(ObjectId::class, $context['file']->_id); + $this->assertSame('filename', $context['file']->filename); + } + + public function testResolveStreamContextForWrite(): void + { + $bucket = new Bucket($this->manager, $this->getDatabaseName()); + $method = new ReflectionMethod($bucket, 'resolveStreamContext'); + $context = $method->invokeArgs($bucket, ['gridfs://bucket/filename', 'wb', []]); + + $this->assertIsArray($context); + $this->assertArrayHasKey('collectionWrapper', $context); + $this->assertInstanceOf(CollectionWrapper::class, $context['collectionWrapper']); + $this->assertArrayHasKey('filename', $context); + $this->assertSame('filename', $context['filename']); + $this->assertArrayHasKey('options', $context); + $this->assertSame(['chunkSizeBytes' => 261120, 'disableMD5' => false], $context['options']); + } + + /** + * Return a list of invalid stream values. + */ + private static function getInvalidStreamValues(): array + { + return [null, 123, 'foo', [], hash_init('md5')]; + } +} From 71c286ebab5bca1610c7d0381737bfa30ef49fbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 18 Sep 2024 16:44:40 +0200 Subject: [PATCH 2/7] Don't recreate collections and indexes on each test for performance --- tests/GridFS/BucketFunctionalTest.php | 4 ++++ tests/GridFS/FunctionalTestCase.php | 25 ++++++++++++++++++++++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/tests/GridFS/BucketFunctionalTest.php b/tests/GridFS/BucketFunctionalTest.php index f9298c2ca..06e19174c 100644 --- a/tests/GridFS/BucketFunctionalTest.php +++ b/tests/GridFS/BucketFunctionalTest.php @@ -688,6 +688,10 @@ public function testUploadingFirstFileCreatesIndexes(): void public function testExistingIndexIsReused(): void { + // The collections may exist from other tests, ensure they are removed before and after the test + $this->dropCollection($this->getDatabaseName(), 'fs.chunks'); + $this->dropCollection($this->getDatabaseName(), 'fs.files'); + $this->filesCollection->createIndex(['filename' => 1.0, 'uploadDate' => 1], ['name' => 'test']); $this->chunksCollection->createIndex(['files_id' => 1.0, 'n' => 1], ['name' => 'test', 'unique' => true]); diff --git a/tests/GridFS/FunctionalTestCase.php b/tests/GridFS/FunctionalTestCase.php index d010ca929..a90da82f2 100644 --- a/tests/GridFS/FunctionalTestCase.php +++ b/tests/GridFS/FunctionalTestCase.php @@ -3,6 +3,7 @@ namespace MongoDB\Tests\GridFS; use MongoDB\Collection; +use MongoDB\Driver\Command; use MongoDB\GridFS\Bucket; use MongoDB\Tests\FunctionalTestCase as BaseFunctionalTestCase; @@ -28,10 +29,28 @@ public function setUp(): void parent::setUp(); $this->bucket = new Bucket($this->manager, $this->getDatabaseName()); - $this->bucket->drop(); - $this->chunksCollection = $this->createCollection($this->getDatabaseName(), 'fs.chunks'); - $this->filesCollection = $this->createCollection($this->getDatabaseName(), 'fs.files'); + $this->chunksCollection = new Collection($this->manager, $this->getDatabaseName(), 'fs.chunks'); + $this->filesCollection = new Collection($this->manager, $this->getDatabaseName(), 'fs.files'); + $this->chunksCollection->deleteMany([]); + $this->filesCollection->deleteMany([]); + } + + public function tearDown(): void + { + $this->chunksCollection->deleteMany([]); + $this->filesCollection->deleteMany([]); + + parent::tearDown(); + } + + public static function tearDownAfterClass(): void + { + $manager = static::createTestManager(); + $manager->executeCommand(self::getDatabaseName(), new Command(['drop' => 'fs.chunks'])); + $manager->executeCommand(self::getDatabaseName(), new Command(['drop' => 'fs.files'])); + + parent::tearDownAfterClass(); } /** From 3c5819a1ab5a513e5f767cbe179a679520f06049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 18 Sep 2024 16:53:17 +0200 Subject: [PATCH 3/7] Revert "Split GridFS bucket tests to improve performance of unit tests" This reverts commit 1d831e2ebc74e373fe5321a02ed0005c944a75c7. --- tests/GridFS/BucketFunctionalTest.php | 162 +++++++++++++++++ tests/GridFS/BucketTest.php | 240 -------------------------- 2 files changed, 162 insertions(+), 240 deletions(-) delete mode 100644 tests/GridFS/BucketTest.php diff --git a/tests/GridFS/BucketFunctionalTest.php b/tests/GridFS/BucketFunctionalTest.php index 06e19174c..67e482922 100644 --- a/tests/GridFS/BucketFunctionalTest.php +++ b/tests/GridFS/BucketFunctionalTest.php @@ -3,16 +3,24 @@ namespace MongoDB\Tests\GridFS; use MongoDB\BSON\Binary; +use MongoDB\BSON\ObjectId; +use MongoDB\Collection; +use MongoDB\Driver\ReadConcern; +use MongoDB\Driver\ReadPreference; +use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\GridFS\Bucket; +use MongoDB\GridFS\CollectionWrapper; use MongoDB\GridFS\Exception\CorruptFileException; use MongoDB\GridFS\Exception\FileNotFoundException; use MongoDB\GridFS\Exception\StreamException; use MongoDB\Model\BSONDocument; use MongoDB\Model\IndexInfo; use MongoDB\Operation\ListIndexes; +use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; use MongoDB\Tests\Fixtures\Codec\TestFileCodec; use MongoDB\Tests\Fixtures\Document\TestFile; +use ReflectionMethod; use stdClass; use function array_merge; @@ -43,6 +51,58 @@ */ class BucketFunctionalTest extends FunctionalTestCase { + /** @doesNotPerformAssertions */ + public function testValidConstructorOptions(): void + { + new Bucket($this->manager, $this->getDatabaseName(), [ + 'bucketName' => 'test', + 'chunkSizeBytes' => 8192, + 'readConcern' => new ReadConcern(ReadConcern::LOCAL), + 'readPreference' => new ReadPreference(ReadPreference::PRIMARY), + 'writeConcern' => new WriteConcern(WriteConcern::MAJORITY, 1000), + 'disableMD5' => true, + ]); + } + + /** @dataProvider provideInvalidConstructorOptions */ + public function testConstructorOptionTypeChecks(array $options): void + { + $this->expectException(InvalidArgumentException::class); + new Bucket($this->manager, $this->getDatabaseName(), $options); + } + + public static function provideInvalidConstructorOptions() + { + return self::createOptionDataProvider([ + 'bucketName' => self::getInvalidStringValues(true), + 'chunkSizeBytes' => self::getInvalidIntegerValues(true), + 'codec' => self::getInvalidDocumentCodecValues(), + 'disableMD5' => self::getInvalidBooleanValues(true), + 'readConcern' => self::getInvalidReadConcernValues(), + 'readPreference' => self::getInvalidReadPreferenceValues(), + 'typeMap' => self::getInvalidArrayValues(), + 'writeConcern' => self::getInvalidWriteConcernValues(), + ]); + } + + public function testConstructorShouldRequireChunkSizeBytesOptionToBePositive(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Expected "chunkSizeBytes" option to be >= 1, 0 given'); + new Bucket($this->manager, $this->getDatabaseName(), ['chunkSizeBytes' => 0]); + } + + public function testConstructorWithCodecAndTypeMapOptions(): void + { + $options = [ + 'codec' => new TestDocumentCodec(), + 'typeMap' => ['root' => 'array', 'document' => 'array'], + ]; + + $this->expectExceptionObject(InvalidArgumentException::cannotCombineCodecAndTypeMap()); + new Bucket($this->manager, $this->getDatabaseName(), $options); + } + /** @dataProvider provideInputDataAndExpectedChunks */ public function testDelete($input, $expectedChunks): void { @@ -147,6 +207,13 @@ public function testDownloadToStream($input): void $this->assertStreamContents($input, $destination); } + /** @dataProvider provideInvalidStreamValues */ + public function testDownloadToStreamShouldRequireDestinationStream($destination): void + { + $this->expectException(InvalidArgumentException::class); + $this->bucket->downloadToStream('id', $destination); + } + public static function provideInvalidStreamValues(): array { return self::wrapValuesForDataProvider(self::getInvalidStreamValues()); @@ -193,6 +260,13 @@ public function testDownloadToStreamByName(): void $this->assertStreamContents('baz', $destination); } + /** @dataProvider provideInvalidStreamValues */ + public function testDownloadToStreamByNameShouldRequireDestinationStream($destination): void + { + $this->expectException(InvalidArgumentException::class); + $this->bucket->downloadToStreamByName('filename', $destination); + } + /** @dataProvider provideNonexistentFilenameAndRevision */ public function testDownloadToStreamByNameShouldRequireFilenameAndRevisionToExist($filename, $revision): void { @@ -378,6 +452,43 @@ public function testFindOneResetsInheritedBucketCodec(): void $this->assertSame(6, $fileDocument->length); } + public function testGetBucketNameWithCustomValue(): void + { + $bucket = new Bucket($this->manager, $this->getDatabaseName(), ['bucketName' => 'custom_fs']); + + $this->assertEquals('custom_fs', $bucket->getBucketName()); + } + + public function testGetBucketNameWithDefaultValue(): void + { + $this->assertEquals('fs', $this->bucket->getBucketName()); + } + + public function testGetChunksCollection(): void + { + $chunksCollection = $this->bucket->getChunksCollection(); + + $this->assertInstanceOf(Collection::class, $chunksCollection); + $this->assertEquals('fs.chunks', $chunksCollection->getCollectionName()); + } + + public function testGetChunkSizeBytesWithCustomValue(): void + { + $bucket = new Bucket($this->manager, $this->getDatabaseName(), ['chunkSizeBytes' => 8192]); + + $this->assertEquals(8192, $bucket->getChunkSizeBytes()); + } + + public function testGetChunkSizeBytesWithDefaultValue(): void + { + $this->assertEquals(261120, $this->bucket->getChunkSizeBytes()); + } + + public function testGetDatabaseName(): void + { + $this->assertEquals($this->getDatabaseName(), $this->bucket->getDatabaseName()); + } + public function testGetFileDocumentForStreamUsesTypeMap(): void { $metadata = ['foo' => 'bar']; @@ -469,6 +580,21 @@ public function testGetFileIdForStreamWithWritableStream(): void $this->assertEquals(1, $this->bucket->getFileIdForStream($stream)); } + /** @dataProvider provideInvalidGridFSStreamValues */ + public function testGetFileIdForStreamShouldRequireGridFSStreamResource($stream): void + { + $this->expectException(InvalidArgumentException::class); + $this->bucket->getFileIdForStream($stream); + } + + public function testGetFilesCollection(): void + { + $filesCollection = $this->bucket->getFilesCollection(); + + $this->assertInstanceOf(Collection::class, $filesCollection); + $this->assertEquals('fs.files', $filesCollection->getCollectionName()); + } + /** @dataProvider provideInputDataAndExpectedChunks */ public function testOpenDownloadStream($input): void { @@ -815,6 +941,42 @@ public function testDanglingOpenWritableStreamWithGlobalStreamWrapperAlias(): vo $this->assertSame(14, $fileDocument->length); } + public function testResolveStreamContextForRead(): void + { + $stream = $this->bucket->openUploadStream('filename'); + fwrite($stream, 'foobar'); + fclose($stream); + + $method = new ReflectionMethod($this->bucket, 'resolveStreamContext'); + $method->setAccessible(true); + + $context = $method->invokeArgs($this->bucket, ['gridfs://bucket/filename', 'rb', []]); + + $this->assertIsArray($context); + $this->assertArrayHasKey('collectionWrapper', $context); + $this->assertInstanceOf(CollectionWrapper::class, $context['collectionWrapper']); + $this->assertArrayHasKey('file', $context); + $this->assertIsObject($context['file']); + $this->assertInstanceOf(ObjectId::class, $context['file']->_id); + $this->assertSame('filename', $context['file']->filename); + } + + public function testResolveStreamContextForWrite(): void + { + $method = new ReflectionMethod($this->bucket, 'resolveStreamContext'); + $method->setAccessible(true); + + $context = $method->invokeArgs($this->bucket, ['gridfs://bucket/filename', 'wb', []]); + + $this->assertIsArray($context); + $this->assertArrayHasKey('collectionWrapper', $context); + $this->assertInstanceOf(CollectionWrapper::class, $context['collectionWrapper']); + $this->assertArrayHasKey('filename', $context); + $this->assertSame('filename', $context['filename']); + $this->assertArrayHasKey('options', $context); + $this->assertSame(['chunkSizeBytes' => 261120, 'disableMD5' => false], $context['options']); + } + /** * Asserts that an index with the given name exists for the collection. * diff --git a/tests/GridFS/BucketTest.php b/tests/GridFS/BucketTest.php deleted file mode 100644 index 29bc35a8c..000000000 --- a/tests/GridFS/BucketTest.php +++ /dev/null @@ -1,240 +0,0 @@ -manager = new Manager('mongodb://server.test:27017'); - } - - /** @doesNotPerformAssertions */ - public function testValidConstructorOptions(): void - { - new Bucket($this->manager, $this->getDatabaseName(), [ - 'bucketName' => 'test', - 'chunkSizeBytes' => 8192, - 'readConcern' => new ReadConcern(ReadConcern::LOCAL), - 'readPreference' => new ReadPreference(ReadPreference::PRIMARY), - 'writeConcern' => new WriteConcern(WriteConcern::MAJORITY, 1000), - 'disableMD5' => true, - ]); - } - - /** @dataProvider provideInvalidConstructorOptions */ - public function testConstructorOptionTypeChecks(array $options): void - { - $this->expectException(InvalidArgumentException::class); - new Bucket($this->manager, $this->getDatabaseName(), $options); - } - - public static function provideInvalidConstructorOptions() - { - return self::createOptionDataProvider([ - 'bucketName' => self::getInvalidStringValues(true), - 'chunkSizeBytes' => self::getInvalidIntegerValues(true), - 'codec' => self::getInvalidDocumentCodecValues(), - 'disableMD5' => self::getInvalidBooleanValues(true), - 'readConcern' => self::getInvalidReadConcernValues(), - 'readPreference' => self::getInvalidReadPreferenceValues(), - 'typeMap' => self::getInvalidArrayValues(), - 'writeConcern' => self::getInvalidWriteConcernValues(), - ]); - } - - public function testConstructorShouldRequireChunkSizeBytesOptionToBePositive(): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Expected "chunkSizeBytes" option to be >= 1, 0 given'); - new Bucket($this->manager, $this->getDatabaseName(), ['chunkSizeBytes' => 0]); - } - - public function testConstructorWithCodecAndTypeMapOptions(): void - { - $options = [ - 'codec' => new TestDocumentCodec(), - 'typeMap' => ['root' => 'array', 'document' => 'array'], - ]; - - $this->expectExceptionObject(InvalidArgumentException::cannotCombineCodecAndTypeMap()); - new Bucket($this->manager, $this->getDatabaseName(), $options); - } - - public static function provideInputDataAndExpectedChunks() - { - return [ - ['', 0], - ['foobar', 1], - [str_repeat('a', 261120), 1], - [str_repeat('a', 261121), 2], - [str_repeat('a', 522240), 2], - [str_repeat('a', 522241), 3], - [str_repeat('foobar', 43520), 1], - [str_repeat('foobar', 43521), 2], - [str_repeat('foobar', 87040), 2], - [str_repeat('foobar', 87041), 3], - ]; - } - - /** @dataProvider provideInvalidStreamValues */ - public function testDownloadToStreamShouldRequireDestinationStream($destination): void - { - $bucket = new Bucket($this->manager, $this->getDatabaseName()); - $this->expectException(InvalidArgumentException::class); - $bucket->downloadToStream('id', $destination); - } - - public static function provideInvalidStreamValues(): array - { - return self::wrapValuesForDataProvider(self::getInvalidStreamValues()); - } - - /** @dataProvider provideInvalidStreamValues */ - public function testDownloadToStreamByNameShouldRequireDestinationStream($destination): void - { - $bucket = new Bucket($this->manager, $this->getDatabaseName()); - $this->expectException(InvalidArgumentException::class); - $bucket->downloadToStreamByName('filename', $destination); - } - - public static function provideNonexistentFilenameAndRevision() - { - return [ - ['filename', 2], - ['filename', -3], - ['nonexistent-filename', 0], - ['nonexistent-filename', -1], - ]; - } - - public function testGetBucketNameWithCustomValue(): void - { - $bucket = new Bucket($this->manager, $this->getDatabaseName(), ['bucketName' => 'custom_fs']); - $this->assertEquals('custom_fs', $bucket->getBucketName()); - } - - public function testGetBucketNameWithDefaultValue(): void - { - $bucket = new Bucket($this->manager, $this->getDatabaseName()); - $this->assertEquals('fs', $bucket->getBucketName()); - } - - public function testGetChunksCollection(): void - { - $bucket = new Bucket($this->manager, $this->getDatabaseName()); - $chunksCollection = $bucket->getChunksCollection(); - - $this->assertInstanceOf(Collection::class, $chunksCollection); - $this->assertEquals('fs.chunks', $chunksCollection->getCollectionName()); - } - - public function testGetChunkSizeBytesWithCustomValue(): void - { - $bucket = new Bucket($this->manager, $this->getDatabaseName(), ['chunkSizeBytes' => 8192]); - - $this->assertEquals(8192, $bucket->getChunkSizeBytes()); - } - - public function testGetChunkSizeBytesWithDefaultValue(): void - { - $bucket = new Bucket($this->manager, $this->getDatabaseName()); - $this->assertEquals(261120, $bucket->getChunkSizeBytes()); - } - - public function testGetDatabaseName(): void - { - $bucket = new Bucket($this->manager, $this->getDatabaseName()); - $this->assertEquals($this->getDatabaseName(), $bucket->getDatabaseName()); - } - - public static function provideInvalidGridFSStreamValues(): array - { - return self::wrapValuesForDataProvider(array_merge(self::getInvalidStreamValues(), [fopen('php://temp', 'r')])); - } - - /** @dataProvider provideInvalidGridFSStreamValues */ - public function testGetFileIdForStreamShouldRequireGridFSStreamResource($stream): void - { - $bucket = new Bucket($this->manager, $this->getDatabaseName()); - $this->expectException(InvalidArgumentException::class); - $bucket->getFileIdForStream($stream); - } - - public function testGetFilesCollection(): void - { - $bucket = new Bucket($this->manager, $this->getDatabaseName()); - $filesCollection = $bucket->getFilesCollection(); - - $this->assertInstanceOf(Collection::class, $filesCollection); - $this->assertEquals('fs.files', $filesCollection->getCollectionName()); - } - - public function testResolveStreamContextForRead(): void - { - $bucket = new Bucket($this->manager, $this->getDatabaseName()); - $stream = $bucket->openUploadStream('filename'); - fwrite($stream, 'foobar'); - fclose($stream); - $method = new ReflectionMethod($bucket, 'resolveStreamContext'); - $context = $method->invokeArgs($bucket, ['gridfs://bucket/filename', 'rb', []]); - - $this->assertIsArray($context); - $this->assertArrayHasKey('collectionWrapper', $context); - $this->assertInstanceOf(CollectionWrapper::class, $context['collectionWrapper']); - $this->assertArrayHasKey('file', $context); - $this->assertIsObject($context['file']); - $this->assertInstanceOf(ObjectId::class, $context['file']->_id); - $this->assertSame('filename', $context['file']->filename); - } - - public function testResolveStreamContextForWrite(): void - { - $bucket = new Bucket($this->manager, $this->getDatabaseName()); - $method = new ReflectionMethod($bucket, 'resolveStreamContext'); - $context = $method->invokeArgs($bucket, ['gridfs://bucket/filename', 'wb', []]); - - $this->assertIsArray($context); - $this->assertArrayHasKey('collectionWrapper', $context); - $this->assertInstanceOf(CollectionWrapper::class, $context['collectionWrapper']); - $this->assertArrayHasKey('filename', $context); - $this->assertSame('filename', $context['filename']); - $this->assertArrayHasKey('options', $context); - $this->assertSame(['chunkSizeBytes' => 261120, 'disableMD5' => false], $context['options']); - } - - /** - * Return a list of invalid stream values. - */ - private static function getInvalidStreamValues(): array - { - return [null, 123, 'foo', [], hash_init('md5')]; - } -} From e318798269315c41f31af9618304af9efe8c0854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 18 Sep 2024 17:03:55 +0200 Subject: [PATCH 4/7] Cleaning is done on tearDown only --- tests/GridFS/FunctionalTestCase.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/GridFS/FunctionalTestCase.php b/tests/GridFS/FunctionalTestCase.php index a90da82f2..290854d4c 100644 --- a/tests/GridFS/FunctionalTestCase.php +++ b/tests/GridFS/FunctionalTestCase.php @@ -32,8 +32,6 @@ public function setUp(): void $this->chunksCollection = new Collection($this->manager, $this->getDatabaseName(), 'fs.chunks'); $this->filesCollection = new Collection($this->manager, $this->getDatabaseName(), 'fs.files'); - $this->chunksCollection->deleteMany([]); - $this->filesCollection->deleteMany([]); } public function tearDown(): void From 03659847e8983548da6e9d836c6a55857aefff46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 18 Sep 2024 18:28:34 +0200 Subject: [PATCH 5/7] Remove collections before and after class, to ensure we always have the same clean setup --- tests/GridFS/FunctionalTestCase.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/GridFS/FunctionalTestCase.php b/tests/GridFS/FunctionalTestCase.php index 290854d4c..121d77148 100644 --- a/tests/GridFS/FunctionalTestCase.php +++ b/tests/GridFS/FunctionalTestCase.php @@ -42,13 +42,19 @@ public function tearDown(): void parent::tearDown(); } - public static function tearDownAfterClass(): void + /** + * The bucket's collections are created by the first test that runs and + * kept for all subsequent tests. This is done to avoid creating the + * collections and their indexes for each test, which would be slow. + * + * @beforeClass + * @afterClass + */ + public static function dropCollectionsBeforeAfterClass(): void { $manager = static::createTestManager(); $manager->executeCommand(self::getDatabaseName(), new Command(['drop' => 'fs.chunks'])); $manager->executeCommand(self::getDatabaseName(), new Command(['drop' => 'fs.files'])); - - parent::tearDownAfterClass(); } /** From 3f6586805ab5872d53818d4385a9f81f2c14d497 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 18 Sep 2024 18:43:39 +0200 Subject: [PATCH 6/7] Add comments and assertions to testExistingIndexIsReused --- tests/GridFS/BucketFunctionalTest.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/GridFS/BucketFunctionalTest.php b/tests/GridFS/BucketFunctionalTest.php index 67e482922..3456951a9 100644 --- a/tests/GridFS/BucketFunctionalTest.php +++ b/tests/GridFS/BucketFunctionalTest.php @@ -814,13 +814,22 @@ public function testUploadingFirstFileCreatesIndexes(): void public function testExistingIndexIsReused(): void { - // The collections may exist from other tests, ensure they are removed before and after the test + // The collections may exist from other tests, ensure they are removed + // before and after to avoid potential conflicts. $this->dropCollection($this->getDatabaseName(), 'fs.chunks'); $this->dropCollection($this->getDatabaseName(), 'fs.files'); + // Create indexes with different numeric types before interacting with + // GridFS to assert that the library respects the existing indexes and + // does not attempt to create its own. $this->filesCollection->createIndex(['filename' => 1.0, 'uploadDate' => 1], ['name' => 'test']); $this->chunksCollection->createIndex(['files_id' => 1.0, 'n' => 1], ['name' => 'test', 'unique' => true]); + $this->assertIndexExists('fs.files', 'test'); + $this->assertIndexExists('fs.chunks', 'test', function (IndexInfo $info): void { + $this->assertTrue($info->isUnique()); + }); + $this->bucket->uploadFromStream('filename', self::createStream('foo')); $this->assertIndexNotExists($this->filesCollection->getCollectionName(), 'filename_1_uploadDate_1'); From 71c313ecfc12d7af91b53d72c7fa4b34dee11b8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 18 Sep 2024 19:00:53 +0200 Subject: [PATCH 7/7] Fix drop collections when it doesn't exist --- tests/GridFS/FunctionalTestCase.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/GridFS/FunctionalTestCase.php b/tests/GridFS/FunctionalTestCase.php index 121d77148..785e1c909 100644 --- a/tests/GridFS/FunctionalTestCase.php +++ b/tests/GridFS/FunctionalTestCase.php @@ -3,8 +3,8 @@ namespace MongoDB\Tests\GridFS; use MongoDB\Collection; -use MongoDB\Driver\Command; use MongoDB\GridFS\Bucket; +use MongoDB\Operation\DropCollection; use MongoDB\Tests\FunctionalTestCase as BaseFunctionalTestCase; use function fopen; @@ -53,8 +53,9 @@ public function tearDown(): void public static function dropCollectionsBeforeAfterClass(): void { $manager = static::createTestManager(); - $manager->executeCommand(self::getDatabaseName(), new Command(['drop' => 'fs.chunks'])); - $manager->executeCommand(self::getDatabaseName(), new Command(['drop' => 'fs.files'])); + + (new DropCollection(self::getDatabaseName(), 'fs.chunks'))->execute($manager->selectServer()); + (new DropCollection(self::getDatabaseName(), 'fs.files'))->execute($manager->selectServer()); } /**