Skip to content
This repository was archived by the owner on Dec 23, 2021. It is now read-only.

HHVM-196: Manager::getServers() should use mongoc_client_get_server_descriptions() #78

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bson.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "hphp/runtime/base/array-iterator.h"
#include "hphp/runtime/base/execution-context.h"
#include "hphp/runtime/base/type-string.h"
#include "hphp/util/logger.h"

#include "bson.h"
#include "utils.h"
Expand Down
31 changes: 20 additions & 11 deletions src/MongoDB/Driver/Manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -465,13 +465,23 @@ const StaticString

Array HHVM_METHOD(MongoDBDriverManager, __debugInfo)
{
MongoDBDriverManagerData* data = Native::data<MongoDBDriverManagerData>(this_);
MongoDBDriverManagerData *data = Native::data<MongoDBDriverManagerData>(this_);
size_t i;
mongoc_server_description_t **sds;
size_t n;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as https://github.com/mongodb/mongo-hhvm-driver/pull/78/files#r57771157. Should everything be aligned here, or just forgo it?

Array retval = Array::Create();
Array servers = Array::Create();

retval.add(s_MongoDBDriverManager_uri, (char*) mongoc_uri_get_string(data->m_client->uri));

mongoc_set_for_each(data->m_client->topology->description.servers, mongodb_driver_add_server_debug_wrapper, &servers);
sds = mongoc_client_get_server_descriptions(data->m_client, &n);
for (i = 0; i < n; i++) {
if (sds[i]->type == MONGOC_SERVER_UNKNOWN) {
continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that mongodb_driver_add_server_debug_wrapper() never excluded servers in the unknown state, so this is a behavioral change. That said, it's probably quite insignificant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, but by not skipping it, a test failed.

}

mongodb_driver_add_server_debug_wrapper(sds[i], &servers);
}

retval.add(s_MongoDBDriverManager_cluster, servers);

Expand Down Expand Up @@ -566,23 +576,22 @@ Object HHVM_METHOD(MongoDBDriverManager, getReadPreference)

Array HHVM_METHOD(MongoDBDriverManager, getServers)
{
MongoDBDriverManagerData *data = Native::data<MongoDBDriverManagerData>(this_);
size_t i;
mongoc_set_t *set;
MongoDBDriverManagerData *data = Native::data<MongoDBDriverManagerData>(this_);
size_t i;
mongoc_server_description_t **sds;
size_t n;

Array retval = Array::Create();

set = data->m_client->topology->description.servers;
for (i = 0; i < set->items_len; i++) {
mongoc_server_description_t *sd = (mongoc_server_description_t*) set->items[i].item;

if (sd->type == MONGOC_SERVER_UNKNOWN) {
sds = mongoc_client_get_server_descriptions(data->m_client, &n);
for (i = 0; i < n; i++) {
if (sds[i]->type == MONGOC_SERVER_UNKNOWN) {
continue;
}

retval.add(
(int64_t) i,
hippo_mongo_driver_server_create_from_id(data->m_client, sd->id)
hippo_mongo_driver_server_create_from_id(data->m_client, sds[i]->id)
);

}
Expand Down
62 changes: 62 additions & 0 deletions tests/MongoDBDriverManager_debugInfo.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
--TEST--
MongoDB\Driver\Manager::__debugInfo()
--FILE--
<?php
$manager = new MongoDB\Driver\Manager();

$command = new MongoDB\Driver\Command(array('ping' => 1));
$manager->executeCommand("test", $command);

var_dump( $manager->__debugInfo() );
?>
--EXPECTF--
array(2) {
["uri"]=>
string(%d) "%s"
["cluster"]=>
array(1) {
[0]=>
array(10) {
["host"]=>
string(%d) "%s"
["port"]=>
int(%d)
["type"]=>
int(1)
["is_primary"]=>
bool(false)
["is_secondary"]=>
bool(false)
["is_arbiter"]=>
bool(false)
["is_hidden"]=>
bool(false)
["is_passive"]=>
bool(false)
["last_is_master"]=>
array(8) {
["ismaster"]=>
bool(true)
["maxBsonObjectSize"]=>
int(%d)
["maxMessageSizeBytes"]=>
int(%d)
["maxWriteBatchSize"]=>
int(%d)
["localTime"]=>
object(MongoDB\BSON\UTCDateTime)#4 (1) {
["milliseconds"]=>
int(%d)
}
["maxWireVersion"]=>
int(%d)
["minWireVersion"]=>
int(%d)
["ok"]=>
float(1)
}
["round_trip_time"]=>
int(%d)
}
}
}
36 changes: 36 additions & 0 deletions tests/MongoDBDriverManager_getServers.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
--TEST--
MongoDB\Driver\Manager::getServers()
--FILE--
<?php
function assertServerType($type) {
if ($type === MongoDB\Driver\Server::TYPE_STANDALONE) {
printf("Found standalone server type: %d\n", $type);
} else {
printf("Unexpected server type: %d\n", $type);
}
}

$manager = new MongoDB\Driver\Manager();

$servers = $manager->getServers();
printf("Known servers: %d\n", count($servers));

echo "Pinging\n";
$command = new MongoDB\Driver\Command(array('ping' => 1));
$manager->executeCommand("test", $command);

$servers = $manager->getServers();
printf("Known servers: %d\n", count($servers));

foreach ($servers as $server) {
printf("Found server: %s:%d\n", $server->getHost(), $server->getPort());
assertServerType($server->getType());
}

?>
--EXPECTF--
Known servers: 0
Pinging
Known servers: 1
Found server: %s:%d
Found standalone server type: 1