Skip to content

Commit 567f750

Browse files
GromNaNGromNaN
committed
Review 2
1 parent 4edad3c commit 567f750

File tree

9 files changed

+50
-58
lines changed

9 files changed

+50
-58
lines changed

‎src/Collection.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -898,7 +898,7 @@ public function listIndexes(array $options = [])
898898
* Only available when used against a 7.0+ Atlas cluster.
899899
*
900900
* @param array{name?: string} $options Command options
901-
* @return Countable&Iterator<array{id: string, name: string, status: string, queryable: bool, latestDefinition: array}>
901+
* @return Countable&Iterator
902902
* @throws InvalidArgumentException for parameter/option parsing errors
903903
* @throws DriverRuntimeException for other driver errors (e.g. connection errors)
904904
* @see ListSearchIndexes::__construct() for supported options

‎src/Operation/DropSearchIndex.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public function execute(Server $server): void
8181
try {
8282
$server->executeCommand($this->databaseName, new Command($cmd));
8383
} catch (CommandException $e) {
84-
// Drop operations is idempotent. The server may return an error if the collection does not exist.
84+
// Drop operations are idempotent. The server may return an error if the collection does not exist.
8585
if ($e->getCode() !== self::ERROR_CODE_NAMESPACE_NOT_FOUND) {
8686
throw $e;
8787
}

‎tests/FunctionalTestCase.php

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use MongoDB\Operation\CreateCollection;
1717
use MongoDB\Operation\DatabaseCommand;
1818
use MongoDB\Operation\ListCollections;
19+
use MongoDB\Tests\SpecTests\DocumentsMatchConstraint;
1920
use stdClass;
2021
use UnexpectedValueException;
2122

@@ -59,7 +60,7 @@ abstract class FunctionalTestCase extends TestCase
5960
private array $configuredFailPoints = [];
6061

6162
/** @var array{int,{Collection,array}} */
62-
private $collectionsToCleanup = [];
63+
private array $collectionsToCleanup = [];
6364

6465
public function setUp(): void
6566
{
@@ -204,6 +205,21 @@ protected function assertSameObjectId($expectedObjectId, $actualObjectId): void
204205
$this->assertEquals((string) $expectedObjectId, (string) $actualObjectId);
205206
}
206207

208+
/**
209+
* Asserts that two given documents match.
210+
*
211+
* Extra keys in the actual value's document(s) will be ignored.
212+
*
213+
* @param array|object $expectedDocument
214+
* @param array|object $actualDocument
215+
*/
216+
public static function assertDocumentsMatch($expectedDocument, $actualDocument, string $message = ''): void
217+
{
218+
$constraint = new DocumentsMatchConstraint($expectedDocument, true, true);
219+
220+
static::assertThat($actualDocument, $constraint, $message);
221+
}
222+
207223
/**
208224
* Configure a fail point for the test.
209225
*
@@ -536,9 +552,9 @@ protected function isEnterprise(): bool
536552
throw new UnexpectedValueException('Could not determine server modules');
537553
}
538554

539-
public static function isAtlas(): bool
555+
public static function isAtlas(?string $uri = null): bool
540556
{
541-
return preg_match(self::ATLAS_TLD, getenv('MONGODB_URI') ?: '');
557+
return preg_match(self::ATLAS_TLD, $uri ?? static::getUri());
542558
}
543559

544560
/** @see https://www.mongodb.com/docs/manual/core/queryable-encryption/reference/shared-library/ */

‎tests/Model/IndexInputTest.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,12 @@ public function testConstructorShouldRequireKey(): void
1919
new IndexInput([]);
2020
}
2121

22-
public function testConstructorShouldRequireKeyToBeArrayOrObject(): void
22+
/** @dataProvider provideInvalidDocumentValues */
23+
public function testConstructorShouldRequireKeyToBeArrayOrObject($key): void
2324
{
2425
$this->expectException(InvalidArgumentException::class);
2526
$this->expectExceptionMessage('Expected "key" option to have type "document"');
26-
new IndexInput(['key' => 'foo']);
27+
new IndexInput(['key' => $key]);
2728
}
2829

2930
/** @dataProvider provideInvalidFieldOrderValues */
@@ -39,11 +40,12 @@ public function provideInvalidFieldOrderValues()
3940
return $this->wrapValuesForDataProvider([true, [], new stdClass()]);
4041
}
4142

42-
public function testConstructorShouldRequireNameToBeString(): void
43+
/** @dataProvider provideInvalidStringValues */
44+
public function testConstructorShouldRequireNameToBeString($name): void
4345
{
4446
$this->expectException(InvalidArgumentException::class);
4547
$this->expectExceptionMessage('Expected "name" option to have type "string"');
46-
new IndexInput(['key' => ['x' => 1], 'name' => 1]);
48+
new IndexInput(['key' => ['x' => 1], 'name' => $name]);
4749
}
4850

4951
/**

‎tests/Model/SearchIndexInputTest.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,20 @@ public function testConstructorIndexDefinitionMustBeDefined(): void
1616
new SearchIndexInput([]);
1717
}
1818

19-
public function testConstructorIndexDefinitionMustBeADocument(): void
19+
/** @dataProvider provideInvalidDocumentValues */
20+
public function testConstructorIndexDefinitionMustBeADocument($definition): void
2021
{
2122
$this->expectException(InvalidArgumentException::class);
2223
$this->expectExceptionMessage('Expected "definition" option to have type "document"');
23-
new SearchIndexInput(['definition' => 'foo']);
24+
new SearchIndexInput(['definition' => $definition]);
2425
}
2526

26-
public function testConstructorShouldRequireNameToBeString(): void
27+
/** @dataProvider provideInvalidStringValues */
28+
public function testConstructorShouldRequireNameToBeString($name): void
2729
{
2830
$this->expectException(InvalidArgumentException::class);
2931
$this->expectExceptionMessage('Expected "name" option to have type "string"');
30-
new SearchIndexInput(['definition' => ['mapping' => ['dynamid' => true]], 'name' => 1]);
32+
new SearchIndexInput(['definition' => ['mapping' => ['dynamic' => true]], 'name' => $name]);
3133
}
3234

3335
public function testBsonSerialization(): void

‎tests/SpecTests/FunctionalTestCase.php

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -81,21 +81,6 @@ public static function assertCommandReplyMatches(stdClass $expected, stdClass $a
8181
throw new LogicException(sprintf('%s does not assert CommandSucceededEvents', static::class));
8282
}
8383

84-
/**
85-
* Asserts that two given documents match.
86-
*
87-
* Extra keys in the actual value's document(s) will be ignored.
88-
*
89-
* @param array|object $expectedDocument
90-
* @param array|object $actualDocument
91-
*/
92-
public static function assertDocumentsMatch($expectedDocument, $actualDocument, string $message = ''): void
93-
{
94-
$constraint = new DocumentsMatchConstraint($expectedDocument, true, true);
95-
96-
static::assertThat($actualDocument, $constraint, $message);
97-
}
98-
9984
/**
10085
* Assert omitted top-level fields in command documents.
10186
*

‎tests/SpecTests/SearchIndexSpecTest.php

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,10 @@
1111
use function count;
1212
use function hrtime;
1313
use function iterator_to_array;
14-
use function json_encode;
1514
use function random_bytes;
1615
use function sleep;
1716
use function sprintf;
1817

19-
use const JSON_THROW_ON_ERROR;
20-
2118
/**
2219
* Functional tests for the Atlas Search index management.
2320
*
@@ -27,6 +24,7 @@
2724
class SearchIndexSpecTest extends FunctionalTestCase
2825
{
2926
private const WAIT_TIMEOUT_SEC = 300;
27+
private const STATUS_READY = 'READY';
3028

3129
public function setUp(): void
3230
{
@@ -56,15 +54,11 @@ public function testCreateAndListSearchIndexes(): void
5654
);
5755
$this->assertSame($name, $createdName);
5856

59-
[$index] = $this->waitForIndexes($collection, fn ($indexes) => $this->allIndexesAreQueryable($indexes));
60-
61-
$this->assertSame($name, $index->name);
57+
$indexes = $this->waitForIndexes($collection, fn ($indexes) => $this->allIndexesAreQueryable($indexes));
6258

63-
// Convert to JSON to compare nested associative arrays and nested objects
64-
$this->assertJsonStringEqualsJsonString(
65-
json_encode($mapping, JSON_THROW_ON_ERROR),
66-
json_encode($index->latestDefinition, JSON_THROW_ON_ERROR),
67-
);
59+
$this->assertCount(1, $indexes);
60+
$this->assertSame($name, $indexes[0]->name);
61+
$this->assertDocumentsMatch($mapping, $indexes[0]->latestDefinition);
6862
}
6963

7064
/**
@@ -86,15 +80,11 @@ public function testCreateMultipleIndexesInBatch(): void
8680

8781
$indexes = $this->waitForIndexes($collection, fn ($indexes) => $this->allIndexesAreQueryable($indexes));
8882

83+
$this->assertCount(2, $indexes);
8984
foreach ($names as $key => $name) {
9085
$index = $indexes[$key];
9186
$this->assertSame($name, $index->name);
92-
93-
// Convert to JSON to compare nested associative arrays and nested objects
94-
$this->assertJsonStringEqualsJsonString(
95-
json_encode($mapping, JSON_THROW_ON_ERROR),
96-
json_encode($index->latestDefinition, JSON_THROW_ON_ERROR),
97-
);
87+
$this->assertDocumentsMatch($mapping, $index->latestDefinition);
9888
}
9989
}
10090

@@ -147,15 +137,11 @@ public function testUpdateSearchIndex(): void
147137
$mapping = ['mappings' => ['dynamic' => true]];
148138
$collection->updateSearchIndex($name, $mapping);
149139

150-
[$index] = $this->waitForIndexes($collection, fn ($indexes) => $this->allIndexesAreQueryable($indexes));
151-
152-
$this->assertSame($name, $index->name);
140+
$indexes = $this->waitForIndexes($collection, fn ($indexes) => $this->allIndexesAreQueryable($indexes));
153141

154-
// Convert to JSON to compare nested associative arrays and nested objects
155-
$this->assertJsonStringEqualsJsonString(
156-
json_encode($mapping, JSON_THROW_ON_ERROR),
157-
json_encode($index->latestDefinition, JSON_THROW_ON_ERROR),
158-
);
142+
$this->assertCount(1, $indexes);
143+
$this->assertSame($name, $indexes[0]->name);
144+
$this->assertDocumentsMatch($mapping, $indexes[0]->latestDefinition);
159145
}
160146

161147
/**
@@ -208,7 +194,7 @@ private function allIndexesAreQueryable(array $indexes): bool
208194
return false;
209195
}
210196

211-
if (! $index->status === 'READY') {
197+
if (! $index->status === self::STATUS_READY) {
212198
return false;
213199
}
214200
}

‎tests/UnifiedSpecTests/Operation.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,10 @@ private function executeForCollection(Collection $collection)
577577
return $collection->updateSearchIndex($args['name'], $args['definition']);
578578

579579
case 'listSearchIndexes':
580-
return $collection->listSearchIndexes($args + (array) ($args['aggregationOptions'] ?? []));
580+
$args += (array) ($args['aggregationOptions'] ?? []);
581+
unset($args['aggregationOptions']);
582+
583+
return $collection->listSearchIndexes($args);
581584

582585
default:
583586
Assert::fail('Unsupported collection operation: ' . $this->name);

‎tests/UnifiedSpecTests/UnifiedTestRunner.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,6 @@
4949
*/
5050
final class UnifiedTestRunner
5151
{
52-
public const ATLAS_TLD = 'mongodb.net';
53-
5452
public const SERVER_ERROR_INTERRUPTED = 11601;
5553
public const SERVER_ERROR_UNAUTHORIZED = 13;
5654

@@ -83,7 +81,7 @@ public function __construct(string $internalClientUri)
8381
/* Atlas prohibits killAllSessions. Inspect the connection string to
8482
* determine if we should avoid calling killAllSessions(). This does
8583
* mean that lingering transactions could block test execution. */
86-
if ($this->isServerless() || strpos($internalClientUri, self::ATLAS_TLD) !== false) {
84+
if ($this->isServerless() || FunctionalTestCase::isAtlas($internalClientUri)) {
8785
$this->allowKillAllSessions = false;
8886
}
8987

0 commit comments

Comments
 (0)