Skip to content

PHPC-433: Persist topology state between requests #376

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.

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 9 commits into from
Sep 9, 2016
353 changes: 249 additions & 104 deletions php_phongo.c

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion php_phongo.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ ZEND_BEGIN_MODULE_GLOBALS(mongodb)
char *debug;
FILE *debug_fd;
bson_mem_vtable_t bsonMemVTable;
HashTable clients;
Copy link
Member Author

Choose a reason for hiding this comment

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

The Internals Book talks about using ALLOC_HASHTABLE(), but that is only useful for non-persistent hash tables. Allocating the value inside of our global struct seemed kosher.

ZEND_END_MODULE_GLOBALS(mongodb)

#if PHP_VERSION_ID >= 70000
Expand Down Expand Up @@ -134,7 +135,7 @@ void php_phongo_read_preference_to_zval(zval *retval, const mongoc_read_prefs_t
void php_phongo_write_concern_to_zval(zval *retval, const mongoc_write_concern_t *write_concern);
void php_phongo_cursor_to_zval(zval *retval, const mongoc_cursor_t *cursor);

bool phongo_manager_init(php_phongo_manager_t *manager, const char *uri_string, bson_t *bson_options, zval *driverOptions TSRMLS_DC);
void phongo_manager_init(php_phongo_manager_t *manager, const char *uri_string, zval *options, zval *driverOptions TSRMLS_DC);
void php_phongo_objectid_new_from_oid(zval *object, const bson_oid_t *oid TSRMLS_DC);
void php_phongo_cursor_id_new_from_id(zval *object, int64_t cursorid TSRMLS_DC);
void php_phongo_new_utcdatetime_from_epoch(zval *object, int64_t msec_since_epoch TSRMLS_DC);
Expand Down
1 change: 0 additions & 1 deletion php_phongo_structs.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ typedef struct {
typedef struct {
PHONGO_ZEND_OBJECT_PRE
mongoc_client_t *client;
char *pem_file;
PHONGO_ZEND_OBJECT_POST
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 forgot to remove this field in the PR for PHPC-605 when we dropped PHP streams.

} php_phongo_manager_t;

Expand Down
63 changes: 57 additions & 6 deletions src/MongoDB/Manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,55 @@ PHONGO_API zend_class_entry *php_phongo_manager_ce;

zend_object_handlers php_phongo_handler_manager;

/* Checks if driverOptions contains a stream context resource in the "context"
* key and incorporates any of its SSL options into the base array that did not
* already exist (i.e. array union). The "context" key is then unset from the
* base array.
*
* This handles the merging of any legacy SSL context options and also makes
* driverOptions suitable for serialization by removing the resource zval. */
static bool php_phongo_manager_merge_context_options(zval *zdriverOptions TSRMLS_DC)
{
php_stream_context *context;
zval *zcontext, *zcontextOptions;

if (!php_array_existsc(zdriverOptions, "context")) {
return true;
}

zcontext = php_array_fetchc(zdriverOptions, "context");
context = php_stream_context_from_zval(zcontext, 1);

if (!context) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "\"context\" driver option is not a valid Stream-Context resource");
return false;
}

#if PHP_VERSION_ID >= 70000
zcontextOptions = php_array_fetchc_array(&context->options, "ssl");
#else
zcontextOptions = php_array_fetchc_array(context->options, "ssl");
#endif

if (!zcontextOptions) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Stream-Context resource does not contain \"ssl\" options array");
return false;
}

/* Perform array union (see: add_function() in zend_operators.c) */
#if PHP_VERSION_ID >= 70000
zend_hash_merge(Z_ARRVAL_P(zdriverOptions), Z_ARRVAL_P(zcontextOptions), zval_add_ref, 0);
#else
{
zval *tmp;
zend_hash_merge(Z_ARRVAL_P(zdriverOptions), Z_ARRVAL_P(zcontextOptions), (void (*)(void *pData)) zval_add_ref, (void *) &tmp, sizeof(zval *), 0);
}
#endif

php_array_unsetc(zdriverOptions, "context");
return true;
}

/* {{{ proto void Manager::__construct([string $uri = "mongodb://127.0.0.1/"[, array $options = array()[, array $driverOptions = array()]]])
Constructs a new Manager */
PHP_METHOD(Manager, __construct)
Expand All @@ -60,26 +109,27 @@ PHP_METHOD(Manager, __construct)
char *uri_string = NULL;
phongo_zpp_char_len uri_string_len = 0;
zval *options = NULL;
bson_t bson_options = BSON_INITIALIZER;
zval *driverOptions = NULL;
SUPPRESS_UNUSED_WARNING(return_value) SUPPRESS_UNUSED_WARNING(return_value_ptr) SUPPRESS_UNUSED_WARNING(return_value_used)


zend_replace_error_handling(EH_THROW, phongo_exception_from_phongo_domain(PHONGO_ERROR_INVALID_ARGUMENT), &error_handling TSRMLS_CC);
intern = Z_MANAGER_OBJ_P(getThis());

/* Separate the driverOptions zval, since we may end up modifying it in
* php_phongo_manager_merge_context_options() below. */
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|s!a!a!", &uri_string, &uri_string_len, &options, &driverOptions) == FAILURE) {
zend_restore_error_handling(&error_handling TSRMLS_CC);
return;
}
zend_restore_error_handling(&error_handling TSRMLS_CC);

if (options) {
phongo_zval_to_bson(options, PHONGO_BSON_NONE, &bson_options, NULL TSRMLS_CC);
if (driverOptions && !php_phongo_manager_merge_context_options(driverOptions TSRMLS_CC)) {
/* Exception should already have been thrown */
return;
}

phongo_manager_init(intern, uri_string ? uri_string : PHONGO_MANAGER_URI_DEFAULT, &bson_options, driverOptions TSRMLS_CC);
bson_destroy(&bson_options);
phongo_manager_init(intern, uri_string ? uri_string : PHONGO_MANAGER_URI_DEFAULT, options, driverOptions TSRMLS_CC);
}
/* }}} */

Expand Down Expand Up @@ -359,7 +409,8 @@ static void php_phongo_manager_free_object(phongo_free_object_arg *object TSRMLS
zend_object_std_dtor(&intern->std TSRMLS_CC);

if (intern->client) {
mongoc_client_destroy(intern->client);
MONGOC_DEBUG("Not destroying persistent client for Manager");
intern->client = NULL;
}

#if PHP_VERSION_ID < 70000
Expand Down
2 changes: 0 additions & 2 deletions tests/connect/standalone-x509-auth-001.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ $driverOptions = [
'weak_cert_validation' => false,
'ca_file' => $SSL_DIR . '/ca.pem',
'pem_file' => $SSL_DIR . '/client.pem',
// TODO: this doesn't appear to have any effect. Does the PEM file not have a password?
'pem_pwd' => 'qwerty',
];

$manager = new MongoDB\Driver\Manager(STANDALONE_X509, ['ssl' => true], $driverOptions);
Expand Down
2 changes: 0 additions & 2 deletions tests/connect/standalone-x509-auth-002.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ $driverOptions = [
'allow_self_signed' => false, // "weak_cert_validation" alias
'cafile' => $SSL_DIR . '/ca.pem', // "ca_file" alias
'local_cert' => $SSL_DIR . '/client.pem', // "pem_file" alias
// TODO: this doesn't appear to have any effect. Does the PEM file not have a password?
'passphrase' => 'qwerty', // "pem_pwd" alias
],
]),
];
Expand Down
7 changes: 5 additions & 2 deletions tests/manager/manager-debug-001.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,18 @@ require_once __DIR__ . "/../utils/basic.inc";
$name = tempnam(sys_get_temp_dir(), "PHONGO");
unlink($name);
mkdir($name);
$manager = new MongoDB\Driver\Manager(STANDALONE, array(), array("debug" => $name));
ini_set('mongodb.debug', $name);
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 was changed for PHPC-704, since we no longer want to support INI modifications via a constructor argument. As a result, I need to manually enable/disable the INI setting below.

$manager = new MongoDB\Driver\Manager(STANDALONE);
$bulk = new MongoDB\Driver\BulkWrite();
$bulk->insert(array('_id' => 1, 'x' => 1));
$result = $manager->executeBulkWrite(NS, $bulk);
ini_set('mongodb.debug', 'off');
foreach(glob($name."/*") as $file);
$content = file($file);
unlink($file);
rmdir($name);

echo $content[0];
echo $content[0], $content[1];
Copy link
Member Author

Choose a reason for hiding this comment

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

Previously, the debug option was applied after URI parsing, which mean that the first debugging output would have been the "Creating Manager" line. Since we need to alter the INI setting before invoking the constructor, the first line is now "Connection string". I've elected to output both for the test.

foreach($content as $line) {
if (strpos($line, "mongoc_bulk_operation_execute")) {
echo $line;
Expand All @@ -30,6 +32,7 @@ foreach($content as $line) {
===DONE===
<?php exit(0); ?>
--EXPECTF--
[%s] PHONGO: DEBUG > Connection string: '%s'
[%s] PHONGO: DEBUG > Creating Manager, phongo-1.%d.%d%S[%s] - mongoc-1.%s(%s), libbson-1.%s(%s), php-%s
[%s] mongoc: TRACE > ENTRY: mongoc_bulk_operation_execute():%d
[%s] mongoc: TRACE > EXIT: mongoc_bulk_operation_execute():%d
Expand Down
2 changes: 1 addition & 1 deletion tests/manager/manager-debug-002.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ $bulk = new MongoDB\Driver\BulkWrite();
$bulk->insert(array('_id' => 1, 'x' => 1));
$result = $manager->executeBulkWrite(NS, $bulk);

ini_set("mongodb.debug", "off");
?>
Copy link
Member Author

Choose a reason for hiding this comment

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

Best I can tell, the request reverts to the original INI setting (mongodb.debug=stderr from the --INI-- block for this test). Disabling debugging inside the request here does not prevent further output from cleaning up the persistent clients during MSHUTDOWN.

===DONE===
<?php exit(0); ?>
Expand All @@ -23,3 +22,4 @@ ini_set("mongodb.debug", "off");
[%s] PHONGO: DEBUG > Creating Manager, phongo-1.%d.%d%S[%s] - mongoc-1.%s(%s), libbson-1.%s(%s), php-%s
%a
===DONE===
%a