Skip to content

Commit 0e45605

Browse files
jmikolajmikola
authored
PHPC-1937: Manager::selectServer() defaults to primary read preference (#1267)
* Use .invalid TLD for invalid host in tests See: https://datatracker.ietf.org/doc/html/rfc2606 * PHPC-1937: Manager::selectServer() defaults to primary read preference
1 parent 93bd10b commit 0e45605

9 files changed

+66
-28
lines changed

‎phongo_compat.h

+7
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,13 @@ static inline zend_bool zend_ini_parse_bool(zend_string* str)
231231
} while (0)
232232
#endif
233233

234+
/* Z_PARAM_OBJECT_OF_CLASS_OR_NULL was introduced in PHP 8.0.
235+
* See: https://github.com/php/php-src/commit/e93d20ad7ebc1075ef1248a663935ee5ea69f1cd */
236+
#ifndef Z_PARAM_OBJECT_OF_CLASS_OR_NULL
237+
#define Z_PARAM_OBJECT_OF_CLASS_OR_NULL(dest, _ce) \
238+
Z_PARAM_OBJECT_OF_CLASS_EX(dest, _ce, 1, 0)
239+
#endif
240+
234241
/* Per https://wiki.php.net/rfc/internal_method_return_types, "Non-final
235242
* internal method return types - when possible - are declared tentatively in
236243
* PHP 8.1, and they will become enforced in PHP 9.0." This can be revisited

‎src/MongoDB/Manager.c

+10-13
Original file line numberDiff line numberDiff line change
@@ -721,25 +721,22 @@ static PHP_METHOD(Manager, removeSubscriber)
721721
phongo_apm_remove_subscriber(intern->subscribers, subscriber);
722722
} /* }}} */
723723

724-
/* {{{ proto MongoDB\Driver\Server MongoDB\Driver\Manager::selectServers(MongoDB\Driver\ReadPreference $readPreference)
725-
Returns a suitable Server for the given ReadPreference */
724+
/* {{{ proto MongoDB\Driver\Server MongoDB\Driver\Manager::selectServers([MongoDB\Driver\ReadPreference $readPreference = null])
725+
Selects a Server for the given ReadPreference (default: primary). */
726726
static PHP_METHOD(Manager, selectServer)
727727
{
728-
zend_error_handling error_handling;
729728
php_phongo_manager_t* intern;
730729
zval* zreadPreference = NULL;
731730
uint32_t server_id = 0;
732731

733-
intern = Z_MANAGER_OBJ_P(getThis());
732+
PHONGO_PARSE_PARAMETERS_START(0, 1)
733+
Z_PARAM_OPTIONAL
734+
Z_PARAM_OBJECT_OF_CLASS_OR_NULL(zreadPreference, php_phongo_readpreference_ce)
735+
PHONGO_PARSE_PARAMETERS_END();
734736

735-
zend_replace_error_handling(EH_THROW, phongo_exception_from_phongo_domain(PHONGO_ERROR_INVALID_ARGUMENT), &error_handling);
736-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "O", &zreadPreference, php_phongo_readpreference_ce) == FAILURE) {
737-
zend_restore_error_handling(&error_handling);
738-
return;
739-
}
740-
zend_restore_error_handling(&error_handling);
737+
intern = Z_MANAGER_OBJ_P(getThis());
741738

742-
if (!php_phongo_manager_select_server(false, true, zreadPreference, NULL, intern->client, &server_id)) {
739+
if (!php_phongo_manager_select_server(false, false, zreadPreference, NULL, intern->client, &server_id)) {
743740
/* Exception should already have been thrown */
744741
return;
745742
}
@@ -879,8 +876,8 @@ ZEND_BEGIN_ARG_INFO_EX(ai_Manager_removeSubscriber, 0, 0, 1)
879876
ZEND_ARG_OBJ_INFO(0, subscriber, MongoDB\\Driver\\Monitoring\\Subscriber, 0)
880877
ZEND_END_ARG_INFO()
881878

882-
ZEND_BEGIN_ARG_INFO_EX(ai_Manager_selectServer, 0, 0, 1)
883-
ZEND_ARG_OBJ_INFO(0, readPreference, MongoDB\\Driver\\ReadPreference, 0)
879+
ZEND_BEGIN_ARG_INFO_EX(ai_Manager_selectServer, 0, 0, 0)
880+
ZEND_ARG_OBJ_INFO(0, readPreference, MongoDB\\Driver\\ReadPreference, 1)
884881
ZEND_END_ARG_INFO()
885882

886883
ZEND_BEGIN_ARG_INFO_EX(ai_Manager_startSession, 0, 0, 0)

‎tests/manager/manager-executeBulkWrite_error-007.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ MongoDB\Driver\Manager::executeBulkWrite() should not issue warning before excep
55
require_once __DIR__ . "/../utils/basic.inc";
66

77
// Invalid host cannot be resolved
8-
$manager = create_test_manager('mongodb://invalid.host:27017', ['serverSelectionTimeoutMS' => 1]);
8+
$manager = create_test_manager('mongodb://example.invalid:27017', ['serverSelectionTimeoutMS' => 1]);
99

1010
echo throws(function() use ($manager) {
1111
$bulk = new MongoDB\Driver\BulkWrite;

‎tests/manager/manager-executeCommand_error-001.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ require_once __DIR__ . "/../utils/basic.inc";
77
$command = new MongoDB\Driver\Command(['ping' => 1]);
88

99
// Invalid host cannot be resolved
10-
$manager = create_test_manager('mongodb://invalid.host:27017', ['serverSelectionTimeoutMS' => 1]);
10+
$manager = create_test_manager('mongodb://example.invalid:27017', ['serverSelectionTimeoutMS' => 1]);
1111

1212
echo throws(function() use ($manager, $command) {
1313
$manager->executeCommand(DATABASE_NAME, $command);

‎tests/manager/manager-executeQuery_error-001.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ require_once __DIR__ . "/../utils/basic.inc";
77
$query = new MongoDB\Driver\Query([]);
88

99
// Invalid host cannot be resolved
10-
$manager = create_test_manager('mongodb://invalid.host:27017', ['serverSelectionTimeoutMS' => 1]);
10+
$manager = create_test_manager('mongodb://example.invalid:27017', ['serverSelectionTimeoutMS' => 1]);
1111

1212
echo throws(function() use ($manager, $query) {
1313
$manager->executeQuery(NS, $query);

‎tests/manager/manager-selectserver-001.phpt renamed to ‎tests/manager/manager-selectServer-001.phpt

+14-10
Original file line numberDiff line numberDiff line change
@@ -8,40 +8,44 @@ MongoDB\Driver\Manager::selectServer() select a server from SDAM based on ReadPr
88
<?php
99
require_once __DIR__ . "/../utils/basic.inc";
1010

11-
$rp = new MongoDB\Driver\ReadPreference(MongoDB\Driver\ReadPreference::RP_PRIMARY);
1211
$manager = create_test_manager();
12+
13+
$rp = new MongoDB\Driver\ReadPreference('primary');
1314
$server = $manager->selectServer($rp);
14-
$rp2 = new MongoDB\Driver\ReadPreference(MongoDB\Driver\ReadPreference::RP_PRIMARY);
15+
16+
$rp2 = new MongoDB\Driver\ReadPreference('primary');
1517
$server2 = $manager->selectServer($rp2);
1618

1719
// load fixtures for test
1820
$bulk = new \MongoDB\Driver\BulkWrite();
19-
$bulk->insert(array('_id' => 1, 'x' => 2, 'y' => 3));
20-
$bulk->insert(array('_id' => 2, 'x' => 3, 'y' => 4));
21-
$bulk->insert(array('_id' => 3, 'x' => 4, 'y' => 5));
21+
$bulk->insert(['_id' => 1, 'x' => 2, 'y' => 3]);
22+
$bulk->insert(['_id' => 2, 'x' => 3, 'y' => 4]);
23+
$bulk->insert(['_id' => 3, 'x' => 4, 'y' => 5]);
2224
$server->executeBulkWrite(NS, $bulk);
2325

24-
$query = new MongoDB\Driver\Query(array('x' => 3), array('projection' => array('y' => 1)));
26+
$query = new MongoDB\Driver\Query(['x' => 3], ['projection' => ['y' => 1]]);
2527
$cursor = $server->executeQuery(NS, $query);
2628

2729
var_dump($cursor instanceof MongoDB\Driver\Cursor);
2830
var_dump($server == $cursor->getServer());
2931
var_dump(iterator_to_array($cursor));
3032

31-
$query = new MongoDB\Driver\Query(array('x' => 3), array('projection' => array('y' => 1)));
33+
$query = new MongoDB\Driver\Query(['x' => 3], ['projection' => ['y' => 1]]);
3234
$cursor = $server2->executeQuery(NS, $query);
3335

3436
var_dump($cursor instanceof MongoDB\Driver\Cursor);
3537
var_dump($server2 == $cursor->getServer());
3638
var_dump(iterator_to_array($cursor));
3739

3840
$bulk = new \MongoDB\Driver\BulkWrite();
39-
$bulk->insert(array('_id' => 1, 'x' => 2, 'y' => 3));
40-
$bulk->insert(array('_id' => 2, 'x' => 3, 'y' => 4));
41-
$bulk->insert(array('_id' => 3, 'x' => 4, 'y' => 5));
41+
$bulk->insert(['_id' => 1, 'x' => 2, 'y' => 3]);
42+
$bulk->insert(['_id' => 2, 'x' => 3, 'y' => 4]);
43+
$bulk->insert(['_id' => 3, 'x' => 4, 'y' => 5]);
44+
4245
throws(function() use($server2, $bulk) {
4346
$server2->executeBulkWrite(NS, $bulk);
4447
}, "MongoDB\Driver\Exception\BulkWriteException");
48+
4549
?>
4650
===DONE===
4751
<?php exit(0); ?>

‎tests/manager/manager-selectServer-002.phpt

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
--TEST--
2+
MongoDB\Driver\Manager::selectServer() defaults to primary read preference
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php skip_if_not_live(); ?>
6+
--FILE--
7+
<?php
8+
9+
use MongoDB\Driver\Server;
10+
11+
require_once __DIR__ . "/../utils/basic.inc";
12+
13+
/* Specifying a client-level read preference will also ensure that it is not
14+
* inherited by php_phongo_manager_select_server. */
15+
$manager = create_test_manager(null, ['readPreference' => 'secondary']);
16+
17+
function isPrimary(Server $server): bool {
18+
return in_array($server->getType(), [Server::TYPE_STANDALONE, Server::TYPE_MONGOS, Server::TYPE_RS_PRIMARY, Server::TYPE_LOAD_BALANCER]);
19+
}
20+
21+
var_dump(isPrimary($manager->selectServer()));
22+
var_dump(isPrimary($manager->selectServer(null)));
23+
24+
?>
25+
===DONE===
26+
<?php exit(0); ?>
27+
--EXPECT--
28+
bool(true)
29+
bool(true)
30+
===DONE===

‎tests/manager/manager-selectserver_error-001.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ require_once __DIR__ . "/../utils/basic.inc";
77
$rp = new MongoDB\Driver\ReadPreference(MongoDB\Driver\ReadPreference::RP_PRIMARY);
88

99
// Invalid host cannot be resolved
10-
$manager = create_test_manager('mongodb://invalid.host:27017', ['serverSelectionTimeoutMS' => 1]);
10+
$manager = create_test_manager('mongodb://example.invalid:27017', ['serverSelectionTimeoutMS' => 1]);
1111

1212
echo throws(function() use ($manager, $rp) {
1313
$manager->selectServer($rp);

‎tests/standalone/bug0655.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ require_once __DIR__ . "/../utils/basic.inc";
77
$command = new MongoDB\Driver\Command(['ping' => 1]);
88

99
// Invalid host cannot be resolved
10-
$manager = create_test_manager('mongodb://invalid.host:27017', ['connectTimeoutMS' => 1]);
10+
$manager = create_test_manager('mongodb://example.invalid:27017', ['connectTimeoutMS' => 1]);
1111

1212
echo throws(function() use ($manager, $command) {
1313
$manager->executeCommand(DATABASE_NAME, $command);

0 commit comments

Comments
 (0)