Skip to content

PHPC-606: Use mongoc_server_description_t public API in Server methods #291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 26, 2016
Merged

PHPC-606: Use mongoc_server_description_t public API in Server methods #291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3f341ad
PHPC-606: Use mongoc_client_get_server_description()
jmikola Apr 19, 2016
e7e65cc
PHPC-606: Use mongoc_server_description_ismaster()
jmikola Apr 19, 2016
05b55cc
PHPC-606: Use mongoc_server_description_host()
jmikola Apr 19, 2016
c795d20
PHPC-606: Use mongoc_server_description_round_trip_time()
jmikola Apr 19, 2016
ac19194
PHPC-606: Translate strings from mongoc_server_description_type()
jmikola Apr 19, 2016
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
2 changes: 2 additions & 0 deletions phongo_compat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@
# define ZEND_HASH_APPLY_COUNT(ht) (ht)->u.v.nApplyCount
# define PHONGO_RETVAL_STRINGL(s, slen) RETVAL_STRINGL(s, slen)
# define PHONGO_RETURN_STRINGL(s, slen) RETURN_STRINGL(s, slen)
# define PHONGO_RETVAL_STRING(s) RETVAL_STRING(s)
# define PHONGO_RETURN_STRING(s) RETURN_STRING(s)
#else
# define phongo_char char
Expand Down Expand Up @@ -186,6 +187,7 @@
# define ZEND_HASH_APPLY_COUNT(ht) (ht)->nApplyCount
# define PHONGO_RETVAL_STRINGL(s, slen) RETVAL_STRINGL(s, slen, 1)
# define PHONGO_RETURN_STRINGL(s, slen) RETURN_STRINGL(s, slen, 1)
# define PHONGO_RETVAL_STRING(s) RETVAL_STRING(s, 1)
# define PHONGO_RETURN_STRING(s) RETURN_STRING(s, 1)
# define PHP_STREAM_CONTEXT(stream) ((php_stream_context*) (stream)->context)
#endif
Expand Down
73 changes: 51 additions & 22 deletions php_phongo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,19 @@ ZEND_DECLARE_MODULE_GLOBALS(mongodb)
#endif
#endif

php_phongo_server_description_type_map_t
php_phongo_server_description_type_map[PHONGO_SERVER_DESCRIPTION_TYPES] = {
{ PHONGO_SERVER_UNKNOWN, "Unknown" },
{ PHONGO_SERVER_STANDALONE, "Standalone" },
Copy link
Contributor

Choose a reason for hiding this comment

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

As the map is from string to int, shouldn't the order be the other way around here? I.e., first string, and then the int constant?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't think the field order was important. Each element is a two-field struct with type and name fields. The array is technically a map from the enum integer to a string. We can technically use the array offset in lieu of storing the type field and make this an array of strings, but I figured the two-field struct was more readable.

{ PHONGO_SERVER_MONGOS, "Mongos" },
{ PHONGO_SERVER_POSSIBLE_PRIMARY, "PossiblePrimary" },
{ PHONGO_SERVER_RS_PRIMARY, "RSPrimary" },
{ PHONGO_SERVER_RS_SECONDARY, "RSSecondary" },
{ PHONGO_SERVER_RS_ARBITER, "RSArbiter" },
{ PHONGO_SERVER_RS_OTHER, "RSOther" },
{ PHONGO_SERVER_RS_GHOST, "RSGhost" },
};

/* {{{ phongo_std_object_handlers */
zend_object_handlers phongo_std_object_handlers;

Expand Down Expand Up @@ -1275,55 +1288,71 @@ void php_phongo_objectid_new_from_oid(zval *object, const bson_oid_t *oid TSRMLS
bson_oid_to_string(oid, intern->oid);
} /* }}} */

void php_phongo_server_to_zval(zval *retval, const mongoc_server_description_t *sd) /* {{{ */
php_phongo_server_description_type_t php_phongo_server_description_type(mongoc_server_description_t *sd)
{
array_init(retval);

ADD_ASSOC_STRING(retval, "host", (char *)sd->host.host);
ADD_ASSOC_LONG_EX(retval, "port", sd->host.port);
ADD_ASSOC_LONG_EX(retval, "type", sd->type);
ADD_ASSOC_BOOL_EX(retval, "is_primary", sd->type == MONGOC_SERVER_RS_PRIMARY);
ADD_ASSOC_BOOL_EX(retval, "is_secondary", sd->type == MONGOC_SERVER_RS_SECONDARY);
ADD_ASSOC_BOOL_EX(retval, "is_arbiter", sd->type == MONGOC_SERVER_RS_ARBITER);
{
bson_iter_t iter;
zend_bool b = bson_iter_init_find_case(&iter, &sd->last_is_master, "hidden") && bson_iter_as_bool(&iter);
const char* name = mongoc_server_description_type(sd);
int i;

ADD_ASSOC_BOOL_EX(retval, "is_hidden", b);
for (i = 0; i < PHONGO_SERVER_DESCRIPTION_TYPES; i++) {
if (!strcmp(name, php_phongo_server_description_type_map[i].name)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How often, and when is this run? A whole loop is going to be slow. If this runs a lot, we should re-order the items in the map, so that the most likely show up first.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is only called in two places:

  • Server::getType()
  • Reporting "type" in the Server's debug info

Note that keeping the map order in sync with the enum allows us to do quick lookups on strings for the Server::isX() methods (and debug handler).

ADD_ASSOC_BOOL_EX(retval, "is_primary", !strcmp(mongoc_server_description_type(sd), php_phongo_server_description_type_map[PHONGO_SERVER_RS_PRIMARY].name));
ADD_ASSOC_BOOL_EX(retval, "is_secondary", !strcmp(mongoc_server_description_type(sd), php_phongo_server_description_type_map[PHONGO_SERVER_RS_SECONDARY].name));
ADD_ASSOC_BOOL_EX(retval, "is_arbiter", !strcmp(mongoc_server_description_type(sd), php_phongo_server_description_type_map[PHONGO_SERVER_RS_ARBITER].name));

I don't think either case is sensitive enough that we need to change this, though.

return php_phongo_server_description_type_map[i].type;
}
}
{
bson_iter_t iter;
zend_bool b = bson_iter_init_find_case(&iter, &sd->last_is_master, "passive") && bson_iter_as_bool(&iter);

ADD_ASSOC_BOOL_EX(retval, "is_passive", b);
}
if (sd->tags.len) {
return PHONGO_SERVER_UNKNOWN;
}

void php_phongo_server_to_zval(zval *retval, mongoc_server_description_t *sd) /* {{{ */
{
mongoc_host_list_t *host = mongoc_server_description_host(sd);
const bson_t *is_master = mongoc_server_description_ismaster(sd);
bson_iter_t iter;

array_init(retval);

ADD_ASSOC_STRING(retval, "host", host->host);
ADD_ASSOC_LONG_EX(retval, "port", host->port);
ADD_ASSOC_LONG_EX(retval, "type", php_phongo_server_description_type(sd));
ADD_ASSOC_BOOL_EX(retval, "is_primary", !strcmp(mongoc_server_description_type(sd), php_phongo_server_description_type_map[PHONGO_SERVER_RS_PRIMARY].name));
ADD_ASSOC_BOOL_EX(retval, "is_secondary", !strcmp(mongoc_server_description_type(sd), php_phongo_server_description_type_map[PHONGO_SERVER_RS_SECONDARY].name));
ADD_ASSOC_BOOL_EX(retval, "is_arbiter", !strcmp(mongoc_server_description_type(sd), php_phongo_server_description_type_map[PHONGO_SERVER_RS_ARBITER].name));
ADD_ASSOC_BOOL_EX(retval, "is_hidden", bson_iter_init_find_case(&iter, is_master, "hidden") && bson_iter_as_bool(&iter));
ADD_ASSOC_BOOL_EX(retval, "is_passive", bson_iter_init_find_case(&iter, is_master, "passive") && bson_iter_as_bool(&iter));

if (bson_iter_init_find(&iter, is_master, "tags") && BSON_ITER_HOLDS_DOCUMENT(&iter)) {
const uint8_t *bytes;
uint32_t len;
php_phongo_bson_state state = PHONGO_BSON_STATE_INITIALIZER;

/* Use native arrays for debugging output */
state.map.root_type = PHONGO_TYPEMAP_NATIVE_ARRAY;
state.map.document_type = PHONGO_TYPEMAP_NATIVE_ARRAY;

phongo_bson_to_zval_ex(bson_get_data(&sd->tags), sd->tags.len, &state);
bson_iter_document(&iter, &len, &bytes);
phongo_bson_to_zval_ex(bytes, len, &state);

#if PHP_VERSION_ID >= 70000
ADD_ASSOC_ZVAL_EX(retval, "tags", &state.zchild);
#else
ADD_ASSOC_ZVAL_EX(retval, "tags", state.zchild);
#endif
}

{
php_phongo_bson_state state = PHONGO_BSON_STATE_INITIALIZER;
/* Use native arrays for debugging output */
state.map.root_type = PHONGO_TYPEMAP_NATIVE_ARRAY;
state.map.document_type = PHONGO_TYPEMAP_NATIVE_ARRAY;

phongo_bson_to_zval_ex(bson_get_data(&sd->last_is_master), sd->last_is_master.len, &state);
phongo_bson_to_zval_ex(bson_get_data(is_master), is_master->len, &state);

#if PHP_VERSION_ID >= 70000
ADD_ASSOC_ZVAL_EX(retval, "last_is_master", &state.zchild);
#else
ADD_ASSOC_ZVAL_EX(retval, "last_is_master", state.zchild);
#endif
}
ADD_ASSOC_LONG_EX(retval, "round_trip_time", sd->round_trip_time);
ADD_ASSOC_LONG_EX(retval, "round_trip_time", (phongo_long) mongoc_server_description_round_trip_time(sd));

} /* }}} */

Expand Down
27 changes: 26 additions & 1 deletion php_phongo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,29 @@ ZEND_END_MODULE_GLOBALS(mongodb)

#include "php_phongo_classes.h"

/* This enum is necessary since mongoc_server_description_type_t is private and
* we need to translate strings returned by mongoc_server_description_type() to
* Server integer constants. */
typedef enum {
PHONGO_SERVER_UNKNOWN = 0,
PHONGO_SERVER_STANDALONE = 1,
PHONGO_SERVER_MONGOS = 2,
PHONGO_SERVER_POSSIBLE_PRIMARY = 3,
PHONGO_SERVER_RS_PRIMARY = 4,
PHONGO_SERVER_RS_SECONDARY = 5,
PHONGO_SERVER_RS_ARBITER = 6,
PHONGO_SERVER_RS_OTHER = 7,
PHONGO_SERVER_RS_GHOST = 8,
PHONGO_SERVER_DESCRIPTION_TYPES = 9,
} php_phongo_server_description_type_t;

typedef struct {
php_phongo_server_description_type_t type;
const char *name;
} php_phongo_server_description_type_map_t;

extern php_phongo_server_description_type_map_t php_phongo_server_description_type_map[];

typedef enum {
PHONGO_ERROR_INVALID_ARGUMENT = 1,
PHONGO_ERROR_RUNTIME = 2,
Expand Down Expand Up @@ -130,7 +153,9 @@ const mongoc_read_prefs_t* phongo_read_preference_from_zval(zval *zread_prefe
const mongoc_write_concern_t* phongo_write_concern_from_zval (zval *zwrite_concern TSRMLS_DC);
const php_phongo_query_t* phongo_query_from_zval (zval *zquery TSRMLS_DC);

void php_phongo_server_to_zval(zval *retval, const mongoc_server_description_t *sd);
php_phongo_server_description_type_t php_phongo_server_description_type(mongoc_server_description_t *sd);

void php_phongo_server_to_zval(zval *retval, mongoc_server_description_t *sd);
Copy link
Contributor

Choose a reason for hiding this comment

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

You ended up removing the "const" in front of the argument *sd. Was that on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

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

mongoc_client_get_server_description() doesn't return a const pointer, and the various mongoc_server_description_X() functions we now call in the debug handler also don't take a const pointer. I opted to just relax the argument rather than force a non-const cast within this function. That said, I don't believe they mutate the server description in any way.

void php_phongo_read_concern_to_zval(zval *retval, const mongoc_read_concern_t *read_concern);
void php_phongo_read_preference_to_zval(zval *retval, const mongoc_read_prefs_t *read_prefs);
void php_phongo_write_concern_to_zval(zval *retval, const mongoc_write_concern_t *write_concern);
Expand Down
Loading