Skip to content

Commit 796fae3

Browse files
tanlisujmikola
tanlisu
and
jmikola
committed
Apply suggestions from code review
Co-authored-by: Jeremy Mikola <[email protected]> Co-authored-by: Jeremy Mikola <[email protected]>
1 parent 0db017c commit 796fae3

File tree

7 files changed

+21
-39
lines changed

7 files changed

+21
-39
lines changed

‎src/Collection.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,7 +1020,7 @@ public function mapReduce(JavascriptInterface $map, JavascriptInterface $reduce,
10201020
public function rename(string $toCollectionName, ?string $toDatabaseName = null, array $options = [])
10211021
{
10221022
if (! isset($toDatabaseName)) {
1023-
$toDatabaseName = $this->getDatabaseName();
1023+
$toDatabaseName = $this->databaseName;
10241024
}
10251025

10261026
if (! isset($options['typeMap'])) {
@@ -1033,7 +1033,7 @@ public function rename(string $toCollectionName, ?string $toDatabaseName = null,
10331033
$options['writeConcern'] = $this->writeConcern;
10341034
}
10351035

1036-
$operation = new RenameCollection($this->getDatabaseName(), $this->getCollectionName(), $toDatabaseName, $toCollectionName, $options);
1036+
$operation = new RenameCollection($this->databaseName, $this->collectionName, $toDatabaseName, $toCollectionName, $options);
10371037

10381038
return $operation->execute($server);
10391039
}

‎src/Database.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ public function modifyCollection($collectionName, array $collectionOptions, arra
487487
public function renameCollection(string $fromCollectionName, string $toCollectionName, ?string $toDatabaseName = null, array $options = [])
488488
{
489489
if (! isset($toDatabaseName)) {
490-
$toDatabaseName = $this->getDatabaseName();
490+
$toDatabaseName = $this->databaseName;
491491
}
492492

493493
if (! isset($options['typeMap'])) {
@@ -500,7 +500,7 @@ public function renameCollection(string $fromCollectionName, string $toCollectio
500500
$options['writeConcern'] = $this->writeConcern;
501501
}
502502

503-
$operation = new RenameCollection($this->getDatabaseName(), $fromCollectionName, $toDatabaseName, $toCollectionName, $options);
503+
$operation = new RenameCollection($this->databaseName, $fromCollectionName, $toDatabaseName, $toCollectionName, $options);
504504

505505
return $operation->execute($server);
506506
}

‎src/Operation/RenameCollection.php

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,18 +125,16 @@ public function execute(Server $server)
125125
throw UnsupportedException::writeConcernNotSupportedInTransaction();
126126
}
127127

128-
$commandOptions = [
128+
$cmd = [
129129
'renameCollection' => $this->fromNamespace,
130130
'to' => $this->toNamespace,
131131
];
132132

133133
if (isset($this->options['dropTarget'])) {
134-
$commandOptions['dropTarget'] = $this->options['dropTarget'];
134+
$cmd['dropTarget'] = $this->options['dropTarget'];
135135
}
136136

137-
$command = new Command($commandOptions);
138-
139-
$cursor = $server->executeWriteCommand('admin', $command, $this->createOptions());
137+
$cursor = $server->executeWriteCommand('admin', new Command($cmd), $this->createOptions());
140138

141139
if (isset($this->options['typeMap'])) {
142140
$cursor->setTypeMap($this->options['typeMap']);

‎tests/Collection/CollectionFunctionalTest.php

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -346,15 +346,14 @@ public function testRenameToSameDatabase(): void
346346
$this->assertCollectionDoesNotExist($this->getCollectionName());
347347
$this->assertCollectionExists($toCollectionName);
348348

349-
$document = $toCollection->findOne();
350-
$this->assertSameDocument(['_id' => 1], $document);
349+
$this->assertSameDocument(['_id' => 1], $toCollection->findOne());
351350
$toCollection->drop();
352351
}
353352

354353
public function testRenameToDifferentDatabase(): void
355354
{
356355
if ($this->isShardedCluster()) {
357-
$this->markTestSkipped('Test does not apply on sharded clusters: need source and target databases to be on the same primary shard.');
356+
$this->markTestSkipped('TODO: mongos requires the target database to exist');
358357
}
359358

360359
$toDatabaseName = $this->getDatabaseName() . '_renamed';
@@ -370,10 +369,8 @@ public function testRenameToDifferentDatabase(): void
370369
$this->assertCollectionDoesNotExist($this->getCollectionName());
371370
$this->assertCollectionExists($toCollectionName, $toDatabaseName);
372371

373-
$document = $toCollection->findOne();
374-
$this->assertSameDocument(['_id' => 1], $document);
372+
$this->assertSameDocument(['_id' => 1], $toCollection->findOne());
375373

376-
$toCollection->drop();
377374
$toDatabase->drop();
378375
}
379376

‎tests/Database/DatabaseFunctionalTest.php

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -233,15 +233,14 @@ public function testRenameCollectionToSameDatabase(): void
233233
$this->assertCollectionDoesNotExist($this->getCollectionName());
234234
$this->assertCollectionExists($toCollectionName);
235235

236-
$document = $toCollection->findOne();
237-
$this->assertSameDocument(['_id' => 1], $document);
236+
$this->assertSameDocument(['_id' => 1], $toCollection->findOne());
238237
$toCollection->drop();
239238
}
240239

241240
public function testRenameCollectionToDifferentDatabase(): void
242241
{
243242
if ($this->isShardedCluster()) {
244-
$this->markTestSkipped('Test does not apply on sharded clusters: need source and target databases to be on the same primary shard.');
243+
$this->markTestSkipped('TODO: mongos requires the target database to exist');
245244
}
246245

247246
$toDatabaseName = $this->getDatabaseName() . '_renamed';
@@ -264,10 +263,8 @@ public function testRenameCollectionToDifferentDatabase(): void
264263
$this->assertCollectionDoesNotExist($this->getCollectionName());
265264
$this->assertCollectionExists($toCollectionName, $toDatabaseName);
266265

267-
$document = $toCollection->findOne();
268-
$this->assertSameDocument(['_id' => 1], $document);
266+
$this->assertSameDocument(['_id' => 1], $toCollection->findOne());
269267

270-
$toCollection->drop();
271268
$toDatabase->drop();
272269
}
273270

‎tests/FunctionalTestCase.php

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,8 @@ protected function assertCollectionCount($namespace, $count): void
154154
* Asserts that a collection with the given name does not exist on the
155155
* server.
156156
*
157-
* If $databaseName is not specified, the current database will be used.
158-
*
159157
* @param string $collectionName
160-
* @param string $databaseName
158+
* @param string $databaseName Defaults to TestCase::getDatabaseName()
161159
*/
162160
protected function assertCollectionDoesNotExist(string $collectionName, ?string $databaseName = null): void
163161
{
@@ -183,14 +181,13 @@ protected function assertCollectionDoesNotExist(string $collectionName, ?string
183181
/**
184182
* Asserts that a collection with the given name exists on the server.
185183
*
186-
* If $databaseName is not specified, the current database will be used.
187184
* An optional $callback may be provided, which should take a CollectionInfo
188185
* argument as its first and only parameter. If a CollectionInfo matching
189186
* the given name is found, it will be passed to the callback, which may
190187
* perform additional assertions.
191188
*
192189
* @param string $collectionName
193-
* @param string $databaseName
190+
* @param string $databaseName Defaults to TestCase::getDatabaseName()
194191
* @param callable $callback
195192
*/
196193
protected function assertCollectionExists(string $collectionName, ?string $databaseName = null, ?callable $callback = null): void

‎tests/Operation/RenameCollectionFunctionalTest.php

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ function (array $event): void {
6969
);
7070
}
7171

72-
public function testRenameExistingCollection(): void
72+
public function testRenameCollectionToNonexistentTarget(): void
7373
{
7474
$server = $this->getPrimaryServer();
7575

@@ -90,11 +90,10 @@ public function testRenameExistingCollection(): void
9090
$this->assertCollectionExists($this->toCollectionName);
9191

9292
$operation = new FindOne($this->getDatabaseName(), $this->toCollectionName, []);
93-
$cursor = $operation->execute($server);
94-
$this->assertSameDocument(['_id' => 1], $cursor);
93+
$this->assertSameDocument(['_id' => 1], $operation->execute($server));
9594
}
9695

97-
public function testRenameExistingCollectionExistingTarget(): void
96+
public function testRenameCollectionExistingTarget(): void
9897
{
9998
$server = $this->getPrimaryServer();
10099

@@ -114,26 +113,20 @@ public function testRenameExistingCollectionExistingTarget(): void
114113
$this->getDatabaseName(),
115114
$this->toCollectionName
116115
);
117-
$commandResult = $operation->execute($server);
116+
$operation->execute($server);
118117
}
119118

120-
/**
121-
* @depends testRenameExistingCollection
122-
*/
123119
public function testRenameNonexistentCollection(): void
124120
{
125-
$this->assertCollectionDoesNotExist($this->getNamespace());
126-
127121
$this->expectException(CommandException::class);
128122
$this->expectExceptionCode(self::$errorCodeNamespaceNotFound);
129123
$operation = new RenameCollection(
130124
$this->getDatabaseName(),
131125
$this->getCollectionName(),
132126
$this->getDatabaseName(),
133-
$this->toCollectionName,
134-
['writeConcern' => $this->createDefaultWriteConcern()]
127+
$this->toCollectionName
135128
);
136-
$commandResult = $operation->execute($this->getPrimaryServer());
129+
$operation->execute($this->getPrimaryServer());
137130
}
138131

139132
public function testSessionOption(): void

0 commit comments

Comments
 (0)