Skip to content

Commit

Permalink
PHPLIB-476: Consider transaction readPreference in select_server
Browse files Browse the repository at this point in the history
This also refactors the conditionals in extract_session_from_options and extract_read_preference_from_options to improve readability.

select_server() previously did not consider the read preference of an active transaction. This isn't very significant, as transactions require a primary read preference, but it is correct to do so.
  • Loading branch information
jmikola committed Sep 28, 2023
1 parent 74d19f8 commit 7262c64
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 11 deletions.
32 changes: 21 additions & 11 deletions src/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -544,11 +544,11 @@ function with_transaction(Session $session, callable $callback, array $transacti
*/
function extract_session_from_options(array $options): ?Session
{
if (! isset($options['session']) || ! $options['session'] instanceof Session) {
return null;
if (isset($options['session']) && $options['session'] instanceof Session) {
$options['session'];
}

return $options['session'];
return null;
}

/**
Expand All @@ -558,33 +558,43 @@ function extract_session_from_options(array $options): ?Session
*/
function extract_read_preference_from_options(array $options): ?ReadPreference
{
if (! isset($options['readPreference']) || ! $options['readPreference'] instanceof ReadPreference) {
return null;
if (isset($options['readPreference']) && $options['readPreference'] instanceof ReadPreference) {
return $options['readPreference'];
}

return $options['readPreference'];
return null;
}

/**
* Performs server selection, respecting the readPreference and session options
* (if given)
* Performs server selection, respecting the readPreference and session options.
*
* The pinned server for an active transaction takes priority, followed by an
* operation-level read preference, followed by an active transaction's read
* preference, followed by a primary read preference.
*
* @internal
*/
function select_server(Manager $manager, array $options): Server
{
$session = extract_session_from_options($options);
$server = $session instanceof Session ? $session->getServer() : null;

// Pinned server for an active transaction takes priority
if ($server !== null) {
return $server;
}

// Operation read preference takes priority
$readPreference = extract_read_preference_from_options($options);
if (! $readPreference instanceof ReadPreference) {
// TODO: PHPLIB-476: Read transaction read preference once PHPC-1439 is implemented
$readPreference = new ReadPreference(ReadPreference::PRIMARY);

// Read preference for an active transaction takes priority
if ($readPreference === null && $session instanceof Session && $session->isInTransaction()) {
/* Session::getTransactionOptions() should always return an array if the
* session is in a transaction, but we can be defensive. */
$readPreference = extract_read_preference_from_options($session->getTransactionOptions() ?? []);
}

// Manager::selectServer() defaults to a primary read preference
return $manager->selectServer($readPreference);
}

Expand Down
51 changes: 51 additions & 0 deletions tests/Functions/SelectServerFunctionalTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

namespace MongoDB\Tests\Functions;

use MongoDB\Driver\ReadPreference;
use MongoDB\Driver\Server;
use MongoDB\Tests\FunctionalTestCase;

use function MongoDB\select_server;

class SelectServerFunctionalTest extends FunctionalTestCase
{
/** @dataProvider providePinnedOptions */
public function testSelectServerPrefersPinnedServer(array $options): void
{
$this->skipIfTransactionsAreNotSupported();

if (! $this->isShardedCluster()) {
$this->markTestSkipped('Pinning requires a sharded cluster');
}

if ($this->isLoadBalanced()) {
$this->markTestSkipped('libmongoc does not pin for load-balanced topology');
}

/* By default, the Manager under test is created with a single-mongos
* URI. Explicitly create a Client with multiple mongoses. */
$client = static::createTestClient(static::getUri(true));

// Collection must be created before the transaction starts
$this->createCollection($this->getDatabaseName(), $this->getCollectionName());

$session = $client->startSession();
$session->startTransaction();

$collection = $client->selectCollection($this->getDatabaseName(), $this->getCollectionName());
$collection->find([], ['session' => $session]);

$this->assertTrue($session->isInTransaction());
$this->assertInstanceOf(Server::class, $session->getServer(), 'Session is pinned');
$this->assertEquals($session->getServer(), select_server($client->getManager(), ['session' => $session]));
}

public static function providePinnedOptions(): array
{
return [
[['readPreference' => new ReadPreference(ReadPreference::PRIMARY_PREFERRED)]],
[[]],
];
}
}

0 comments on commit 7262c64

Please sign in to comment.