From aec79b2f12d9cf1394204a04725fb980743137a5 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Mon, 16 Apr 2018 15:28:21 -0400 Subject: [PATCH 1/4] Refactor command and query execute functions This corrects the function signatures to return a boolean indicating success instead of an integer (which was cast from a boolean). Additionally, it restructures phongo_execute_command() to use a cleanup label, in anticipation of implicit session changes. --- php_phongo.c | 49 ++++++++++++++++++++++++------------------------- php_phongo.h | 4 ++-- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/php_phongo.c b/php_phongo.c index 1fcf2db02..c09f73b4f 100644 --- a/php_phongo.c +++ b/php_phongo.c @@ -749,7 +749,7 @@ bool phongo_cursor_advance_and_check_for_error(mongoc_cursor_t *cursor TSRMLS_DC return true; } /* }}} */ -int phongo_execute_query(mongoc_client_t *client, const char *namespace, zval *zquery, zval *options, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC) /* {{{ */ +bool phongo_execute_query(mongoc_client_t *client, const char *namespace, zval *zquery, zval *options, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC) /* {{{ */ { const php_phongo_query_t *query; mongoc_cursor_t *cursor; @@ -827,7 +827,7 @@ static bson_t *create_wrapped_command_envelope(const char *db, bson_t *reply) return tmp; } -int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t type, const char *db, zval *zcommand, zval *options, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC) /* {{{ */ +bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t type, const char *db, zval *zcommand, zval *options, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC) /* {{{ */ { const php_phongo_command_t *command; bson_iter_t iter; @@ -837,38 +837,34 @@ int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t ty mongoc_cursor_t *cmd_cursor; zval *zreadPreference = NULL; zval *zsession = NULL; - int result; + bool result = false; + bool free_reply = false; command = Z_COMMAND_OBJ_P(zcommand); if ((type & PHONGO_OPTION_READ_CONCERN) && !phongo_parse_read_concern(options, &opts TSRMLS_CC)) { /* Exception should already have been thrown */ - bson_destroy(&opts); - return false; + goto cleanup; } if ((type & PHONGO_OPTION_READ_PREFERENCE) && !phongo_parse_read_preference(options, &zreadPreference TSRMLS_CC)) { /* Exception should already have been thrown */ - bson_destroy(&opts); - return false; + goto cleanup; } if (!phongo_parse_session(options, client, &opts, &zsession TSRMLS_CC)) { /* Exception should already have been thrown */ - bson_destroy(&opts); - return false; + goto cleanup; } if ((type & PHONGO_OPTION_WRITE_CONCERN) && !phongo_parse_write_concern(options, &opts, NULL TSRMLS_CC)) { /* Exception should already have been thrown */ - bson_destroy(&opts); - return false; + goto cleanup; } if (!BSON_APPEND_INT32(&opts, "serverId", server_id)) { phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Error appending \"serverId\" option"); - bson_destroy(&opts); - return false; + goto cleanup; } /* Although "opts" already always includes the serverId option, the read @@ -891,21 +887,18 @@ int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t ty default: /* Should never happen, but if it does: exception */ phongo_throw_exception(PHONGO_ERROR_LOGIC TSRMLS_CC, "Type '%d' should never have been passed to phongo_execute_command, please file a bug report", type); - bson_destroy(&opts); - return false; + goto cleanup; } + + free_reply = true; + if (!result) { phongo_throw_exception_from_bson_error_t(&error TSRMLS_CC); - bson_destroy(&reply); - bson_destroy(&opts); - return false; + goto cleanup; } - bson_destroy(&opts); - if (!return_value_used) { - bson_destroy(&reply); - return true; + goto cleanup; } /* According to mongoc_cursor_new_from_command_reply(), the reply bson_t @@ -926,16 +919,22 @@ int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t ty } cmd_cursor = mongoc_cursor_new_from_command_reply(client, &initial_reply, server_id); - bson_destroy(&reply); } else { bson_t *wrapped_reply = create_wrapped_command_envelope(db, &reply); cmd_cursor = mongoc_cursor_new_from_command_reply(client, wrapped_reply, server_id); - bson_destroy(&reply); } phongo_cursor_init_for_command(return_value, client, cmd_cursor, db, zcommand, zreadPreference, zsession TSRMLS_CC); - return true; + +cleanup: + bson_destroy(&opts); + + if (free_reply) { + bson_destroy(&reply); + } + + return result; } /* }}} */ /* }}} */ diff --git a/php_phongo.h b/php_phongo.h index f53ebe495..1356b206a 100644 --- a/php_phongo.h +++ b/php_phongo.h @@ -134,8 +134,8 @@ void phongo_readconcern_init (zval *return_value, const void phongo_readpreference_init (zval *return_value, const mongoc_read_prefs_t *read_prefs TSRMLS_DC); void phongo_writeconcern_init (zval *return_value, const mongoc_write_concern_t *write_concern TSRMLS_DC); bool phongo_execute_bulk_write (mongoc_client_t *client, const char *namespace, php_phongo_bulkwrite_t *bulk_write, zval *zwriteConcern, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC); -int phongo_execute_command (mongoc_client_t *client, php_phongo_command_type_t type, const char *db, zval *zcommand, zval *zreadPreference, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC); -int phongo_execute_query (mongoc_client_t *client, const char *namespace, zval *zquery, zval *zreadPreference, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC); +bool phongo_execute_command (mongoc_client_t *client, php_phongo_command_type_t type, const char *db, zval *zcommand, zval *zreadPreference, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC); +bool phongo_execute_query (mongoc_client_t *client, const char *namespace, zval *zquery, zval *zreadPreference, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC); bool phongo_cursor_advance_and_check_for_error(mongoc_cursor_t *cursor TSRMLS_DC); From 427854e092958b460cb16a5bde84ceb8965d94ae Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Thu, 12 Apr 2018 16:39:44 -0400 Subject: [PATCH 2/4] PHPC-1152: Create implicit session for commands This ensures that phongo_execute_command creates an implicit session (if supported and not explicit session was provided). In turn, this ensures that any command cursor shares the same session as its originating command. By creating a Session object, we can ensure that the implicit session is destroyed during garbage collection when the last reference is removed. --- php_phongo.c | 55 ++++++++ tests/cursor/bug1152-001.phpt | 133 ++++++++++++++++++ tests/cursor/bug1152-002.phpt | 131 +++++++++++++++++ tests/manager/manager-executeCommand-001.phpt | 2 +- tests/server/server-executeCommand-001.phpt | 2 +- 5 files changed, 321 insertions(+), 2 deletions(-) create mode 100644 tests/cursor/bug1152-001.phpt create mode 100644 tests/cursor/bug1152-002.phpt diff --git a/php_phongo.c b/php_phongo.c index c09f73b4f..b04a3c6fb 100644 --- a/php_phongo.c +++ b/php_phongo.c @@ -827,6 +827,28 @@ static bson_t *create_wrapped_command_envelope(const char *db, bson_t *reply) return tmp; } +static zval *phongo_create_implicit_session(mongoc_client_t *client TSRMLS_DC) /* {{{ */ +{ + mongoc_client_session_t *cs; + zval *zsession; + + cs = mongoc_client_start_session(client, NULL, NULL); + + if (!cs) { + return NULL; + } + +#if PHP_VERSION_ID >= 70000 + zsession = ecalloc(sizeof(zval), 1); +#else + ALLOC_INIT_ZVAL(zsession); +#endif + + phongo_session_init(zsession, cs TSRMLS_CC); + + return zsession; +} /* }}} */ + bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t type, const char *db, zval *zcommand, zval *options, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC) /* {{{ */ { const php_phongo_command_t *command; @@ -839,6 +861,7 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t zval *zsession = NULL; bool result = false; bool free_reply = false; + bool free_zsession = false; command = Z_COMMAND_OBJ_P(zcommand); @@ -857,6 +880,21 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t goto cleanup; } + /* If an explicit session was not provided, attempt to create an implicit + * client session (ignoring any errors). */ + if (!zsession) { + zsession = phongo_create_implicit_session(client TSRMLS_CC); + + if (zsession) { + free_zsession = true; + + if (!mongoc_client_session_append(Z_SESSION_OBJ_P(zsession)->client_session, &opts, NULL)) { + phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Error appending implicit \"sessionId\" option"); + goto cleanup; + } + } + } + if ((type & PHONGO_OPTION_WRITE_CONCERN) && !phongo_parse_write_concern(options, &opts, NULL TSRMLS_CC)) { /* Exception should already have been thrown */ goto cleanup; @@ -905,6 +943,7 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t * is ultimately destroyed on both success and failure. */ if (bson_iter_init_find(&iter, &reply, "cursor") && BSON_ITER_HOLDS_DOCUMENT(&iter)) { bson_t initial_reply = BSON_INITIALIZER; + bson_error_t error = {0}; bson_copy_to(&reply, &initial_reply); @@ -918,6 +957,13 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t bson_append_int64(&initial_reply, "batchSize", -1, command->batch_size); } + if (zsession && !mongoc_client_session_append(Z_SESSION_OBJ_P(zsession)->client_session, &initial_reply, &error)) { + phongo_throw_exception_from_bson_error_t(&error TSRMLS_CC); + bson_destroy(&initial_reply); + result = false; + goto cleanup; + } + cmd_cursor = mongoc_cursor_new_from_command_reply(client, &initial_reply, server_id); } else { bson_t *wrapped_reply = create_wrapped_command_envelope(db, &reply); @@ -934,6 +980,15 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t bson_destroy(&reply); } + if (free_zsession) { +#if PHP_VERSION_ID >= 70000 + zval_ptr_dtor(zsession); + efree(zsession); +#else + zval_ptr_dtor(&zsession); +#endif + } + return result; } /* }}} */ /* }}} */ diff --git a/tests/cursor/bug1152-001.phpt b/tests/cursor/bug1152-001.phpt new file mode 100644 index 000000000..545684846 --- /dev/null +++ b/tests/cursor/bug1152-001.phpt @@ -0,0 +1,133 @@ +--TEST-- +PHPC-1152: Command cursors should use the same session for getMore and killCursors (implicit) +--SKIPIF-- + + + + + + +--FILE-- +insert(['_id' => 1]); + $bulk->insert(['_id' => 2]); + $bulk->insert(['_id' => 3]); + $manager->executeBulkWrite(NS, $bulk); + + $command = new MongoDB\Driver\Command([ + 'aggregate' => COLLECTION_NAME, + 'pipeline' => [['$match' => new stdClass]], + 'cursor' => ['batchSize' => 2], + ]); + + MongoDB\Driver\Monitoring\addSubscriber($this); + + /* By creating two cursors with the same name, PHP's reference counting + * will destroy the first after the second is created. Note that + * mongoc_cursor_destroy also destroys implicit sessions and returns + * them to the LIFO pool. This sequencing allows us to test that getMore + * and killCursors use the session ID corresponding to the original + * aggregate command. */ + $cursor = $manager->executeCommand(DATABASE_NAME, $command); + $cursor->toArray(); + + $cursor = $manager->executeCommand(DATABASE_NAME, $command); + $cursor->toArray(); + + $cursor = $manager->executeCommand(DATABASE_NAME, $command); + $cursor = $manager->executeCommand(DATABASE_NAME, $command); + unset($cursor); + + MongoDB\Driver\Monitoring\removeSubscriber($this); + + /* We should expect two unique session IDs over the course of the test, + * since at most two implicit sessions would have been in use at any + * given time. */ + printf("Unique session IDs used: %d\n", count(array_unique($this->lsidByRequestId))); + } + + public function commandStarted(MongoDB\Driver\Monitoring\CommandStartedEvent $event) + { + $requestId = $event->getRequestId(); + $sessionId = bin2hex((string) $event->getCommand()->lsid->id); + + printf("%s session ID: %s\n", $event->getCommandName(), $sessionId); + + if ($event->getCommandName() === 'aggregate') { + if (isset($this->lsidByRequestId[$requestId])) { + throw new UnexpectedValueException('Previous command observed for request ID: ' . $requestId); + } + + $this->lsidByRequestId[$requestId] = $sessionId; + } + + if ($event->getCommandName() === 'getMore') { + $cursorId = $event->getCommand()->getMore; + + if ( ! isset($this->lsidByCursorId[$cursorId])) { + throw new UnexpectedValueException('No previous command observed for cursor ID: ' . $cursorId); + } + + printf("getMore used same session as aggregate: %s\n", $sessionId === $this->lsidByCursorId[$cursorId] ? 'yes' : 'no'); + } + + if ($event->getCommandName() === 'killCursors') { + $cursorId = $event->getCommand()->cursors[0]; + + if ( ! isset($this->lsidByCursorId[$cursorId])) { + throw new UnexpectedValueException('No previous command observed for cursor ID: ' . $cursorId); + } + + printf("killCursors used same session as aggregate: %s\n", $sessionId === $this->lsidByCursorId[$cursorId] ? 'yes' : 'no'); + } + } + + public function commandSucceeded(MongoDB\Driver\Monitoring\CommandSucceededEvent $event) + { + /* Associate the aggregate's session ID with its cursor ID so it can be + * looked up by the subsequent getMore or killCursors */ + if ($event->getCommandName() === 'aggregate') { + $cursorId = $event->getReply()->cursor->id; + $requestId = $event->getRequestId(); + + $this->lsidByCursorId[$cursorId] = $this->lsidByRequestId[$requestId]; + } + } + + public function commandFailed(MongoDB\Driver\Monitoring\CommandFailedEvent $event) + { + } +} + +(new Test)->executeCommand(); + +?> +===DONE=== + +--EXPECTF-- +aggregate session ID: %x +getMore session ID: %x +getMore used same session as aggregate: yes +aggregate session ID: %x +getMore session ID: %x +getMore used same session as aggregate: yes +aggregate session ID: %x +aggregate session ID: %x +killCursors session ID: %x +killCursors used same session as aggregate: yes +killCursors session ID: %x +killCursors used same session as aggregate: yes +Unique session IDs used: 2 +===DONE=== diff --git a/tests/cursor/bug1152-002.phpt b/tests/cursor/bug1152-002.phpt new file mode 100644 index 000000000..a70cbdafd --- /dev/null +++ b/tests/cursor/bug1152-002.phpt @@ -0,0 +1,131 @@ +--TEST-- +PHPC-1152: Command cursors should use the same session for getMore and killCursors (explicit) +--SKIPIF-- + + + + + + +--FILE-- +insert(['_id' => 1]); + $bulk->insert(['_id' => 2]); + $bulk->insert(['_id' => 3]); + $manager->executeBulkWrite(NS, $bulk); + + $command = new MongoDB\Driver\Command([ + 'aggregate' => COLLECTION_NAME, + 'pipeline' => [['$match' => new stdClass]], + 'cursor' => ['batchSize' => 2], + ]); + + $session = $manager->startSession(); + + MongoDB\Driver\Monitoring\addSubscriber($this); + + /* This uses the same sequencing as the implicit session test; however, + * we should expect all commands (aggregate, getMore, and killCursors) + * to use the same explicit session ID. */ + $cursor = $manager->executeCommand(DATABASE_NAME, $command, ['session' => $session]); + $cursor->toArray(); + + $cursor = $manager->executeCommand(DATABASE_NAME, $command, ['session' => $session]); + $cursor->toArray(); + + $cursor = $manager->executeCommand(DATABASE_NAME, $command, ['session' => $session]); + $cursor = $manager->executeCommand(DATABASE_NAME, $command, ['session' => $session]); + unset($cursor); + + MongoDB\Driver\Monitoring\removeSubscriber($this); + + /* We should expect one unique session ID over the course of the test, + * since all commands used the same explicit session. */ + printf("Unique session IDs used: %d\n", count(array_unique($this->lsidByRequestId))); + } + + public function commandStarted(MongoDB\Driver\Monitoring\CommandStartedEvent $event) + { + $requestId = $event->getRequestId(); + $sessionId = bin2hex((string) $event->getCommand()->lsid->id); + + printf("%s session ID: %s\n", $event->getCommandName(), $sessionId); + + if ($event->getCommandName() === 'aggregate') { + if (isset($this->lsidByRequestId[$requestId])) { + throw new UnexpectedValueException('Previous command observed for request ID: ' . $requestId); + } + + $this->lsidByRequestId[$requestId] = $sessionId; + } + + if ($event->getCommandName() === 'getMore') { + $cursorId = $event->getCommand()->getMore; + + if ( ! isset($this->lsidByCursorId[$cursorId])) { + throw new UnexpectedValueException('No previous command observed for cursor ID: ' . $cursorId); + } + + printf("getMore used same session as aggregate: %s\n", $sessionId === $this->lsidByCursorId[$cursorId] ? 'yes' : 'no'); + } + + if ($event->getCommandName() === 'killCursors') { + $cursorId = $event->getCommand()->cursors[0]; + + if ( ! isset($this->lsidByCursorId[$cursorId])) { + throw new UnexpectedValueException('No previous command observed for cursor ID: ' . $cursorId); + } + + printf("killCursors used same session as aggregate: %s\n", $sessionId === $this->lsidByCursorId[$cursorId] ? 'yes' : 'no'); + } + } + + public function commandSucceeded(MongoDB\Driver\Monitoring\CommandSucceededEvent $event) + { + /* Associate the aggregate's session ID with its cursor ID so it can be + * looked up by the subsequent getMore or killCursors */ + if ($event->getCommandName() === 'aggregate') { + $cursorId = $event->getReply()->cursor->id; + $requestId = $event->getRequestId(); + + $this->lsidByCursorId[$cursorId] = $this->lsidByRequestId[$requestId]; + } + } + + public function commandFailed(MongoDB\Driver\Monitoring\CommandFailedEvent $event) + { + } +} + +(new Test)->executeCommand(); + +?> +===DONE=== + +--EXPECTF-- +aggregate session ID: %x +getMore session ID: %x +getMore used same session as aggregate: yes +aggregate session ID: %x +getMore session ID: %x +getMore used same session as aggregate: yes +aggregate session ID: %x +aggregate session ID: %x +killCursors session ID: %x +killCursors used same session as aggregate: yes +killCursors session ID: %x +killCursors used same session as aggregate: yes +Unique session IDs used: 1 +===DONE=== diff --git a/tests/manager/manager-executeCommand-001.phpt b/tests/manager/manager-executeCommand-001.phpt index 5439ed573..4fa275f2d 100644 --- a/tests/manager/manager-executeCommand-001.phpt +++ b/tests/manager/manager-executeCommand-001.phpt @@ -55,7 +55,7 @@ object(MongoDB\Driver\Cursor)#%d (%d) { ["readPreference"]=> NULL ["session"]=> - NULL + %a ["isDead"]=> bool(false) ["currentIndex"]=> diff --git a/tests/server/server-executeCommand-001.phpt b/tests/server/server-executeCommand-001.phpt index d5ee14e8c..832757e7b 100644 --- a/tests/server/server-executeCommand-001.phpt +++ b/tests/server/server-executeCommand-001.phpt @@ -44,7 +44,7 @@ object(MongoDB\Driver\Cursor)#%d (%d) { ["readPreference"]=> NULL ["session"]=> - NULL + %a ["isDead"]=> bool(false) ["currentIndex"]=> From fdc75655078f898c36512873bf61f8d42d2eb7dc Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Mon, 16 Apr 2018 16:35:27 -0400 Subject: [PATCH 3/4] PHPC-1161: Free reference to Session once Cursor is exhausted --- src/MongoDB/Cursor.c | 20 +++++++++ tests/cursor/cursor-session-001.phpt | 63 ++++++++++++++++++++++++++ tests/cursor/cursor-session-002.phpt | 59 ++++++++++++++++++++++++ tests/cursor/cursor-session-003.phpt | 67 ++++++++++++++++++++++++++++ tests/cursor/cursor-session-004.phpt | 66 +++++++++++++++++++++++++++ 5 files changed, 275 insertions(+) create mode 100644 tests/cursor/cursor-session-001.phpt create mode 100644 tests/cursor/cursor-session-002.phpt create mode 100644 tests/cursor/cursor-session-003.phpt create mode 100644 tests/cursor/cursor-session-004.phpt diff --git a/src/MongoDB/Cursor.c b/src/MongoDB/Cursor.c index 55a2cfb5d..be2452067 100644 --- a/src/MongoDB/Cursor.c +++ b/src/MongoDB/Cursor.c @@ -28,6 +28,22 @@ zend_class_entry *php_phongo_cursor_ce; +/* Check if the cursor is exhausted (i.e. ID is zero) and free any reference to + * the session. Calling this function during iteration will allow an implicit + * session to return to the pool immediately after a getMore indicates that the + * server has no more results to return. */ +static void php_phongo_cursor_free_session_if_exhausted(php_phongo_cursor_t *cursor) /* {{{ */ +{ + if (mongoc_cursor_get_id(cursor->cursor)) { + return; + } + + if (!Z_ISUNDEF(cursor->session)) { + zval_ptr_dtor(&cursor->session); + ZVAL_UNDEF(&cursor->session); + } +} /* }}} */ + static void php_phongo_cursor_free_current(php_phongo_cursor_t *cursor) /* {{{ */ { if (!Z_ISUNDEF(cursor->visitor_data.zchild)) { @@ -117,6 +133,8 @@ static void php_phongo_cursor_iterator_move_forward(zend_object_iterator *iter T phongo_throw_exception_from_bson_error_t(&error TSRMLS_CC); } } + + php_phongo_cursor_free_session_if_exhausted(cursor); } /* }}} */ static void php_phongo_cursor_iterator_rewind(zend_object_iterator *iter TSRMLS_DC) /* {{{ */ @@ -147,6 +165,8 @@ static void php_phongo_cursor_iterator_rewind(zend_object_iterator *iter TSRMLS_ if (doc) { php_phongo_bson_to_zval_ex(bson_get_data(doc), doc->len, &cursor->visitor_data); } + + php_phongo_cursor_free_session_if_exhausted(cursor); } /* }}} */ static zend_object_iterator_funcs php_phongo_cursor_iterator_funcs = { diff --git a/tests/cursor/cursor-session-001.phpt b/tests/cursor/cursor-session-001.phpt new file mode 100644 index 000000000..4ee3cf1ae --- /dev/null +++ b/tests/cursor/cursor-session-001.phpt @@ -0,0 +1,63 @@ +--TEST-- +MongoDB\Driver\Cursor debug output for query cursor includes explicit session +--SKIPIF-- + + + + + +--FILE-- +insert(['_id' => 1]); +$bulk->insert(['_id' => 2]); +$bulk->insert(['_id' => 3]); +$manager->executeBulkWrite(NS, $bulk); + +$query = new MongoDB\Driver\Query([], ['batchSize' => 2]); +$session = $manager->startSession(); + +$cursor = $manager->executeQuery(NS, $query, ['session' => $session]); + +$iterator = new IteratorIterator($cursor); +$iterator->rewind(); +$iterator->next(); + +printf("Cursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no'); +var_dump($cursor); + +$iterator->next(); + +/* Per PHPC-1161, the Cursor will free a reference to the Session as soon as it + * is exhausted. While this is primarily done to ensure implicit sessions for + * command cursors are returned to the pool ASAP, it also applies to explicit + * sessions. */ +printf("\nCursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no'); +var_dump($cursor); + +?> +===DONE=== + +--EXPECTF-- +Cursor ID is zero: no +object(MongoDB\Driver\Cursor)#%d (%d) { + %a + ["session"]=> + object(MongoDB\Driver\Session)#%d (%d) { + %a + } + %a +} + +Cursor ID is zero: yes +object(MongoDB\Driver\Cursor)#%d (%d) { + %a + ["session"]=> + NULL + %a +} +===DONE=== diff --git a/tests/cursor/cursor-session-002.phpt b/tests/cursor/cursor-session-002.phpt new file mode 100644 index 000000000..8d7d20887 --- /dev/null +++ b/tests/cursor/cursor-session-002.phpt @@ -0,0 +1,59 @@ +--TEST-- +MongoDB\Driver\Cursor debug output for query cursor omits implicit session +--SKIPIF-- + + + + + +--FILE-- +insert(['_id' => 1]); +$bulk->insert(['_id' => 2]); +$bulk->insert(['_id' => 3]); +$manager->executeBulkWrite(NS, $bulk); + +$query = new MongoDB\Driver\Query([], ['batchSize' => 2]); + +$cursor = $manager->executeQuery(NS, $query); + +$iterator = new IteratorIterator($cursor); +$iterator->rewind(); +$iterator->next(); + +/* Implicit sessions for query cursors are never exposed to PHPC, as they are + * handled internally by libmongoc. Cursor debug ouput should never report such + * sessions. */ +printf("Cursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no'); +var_dump($cursor); + +$iterator->next(); + +printf("\nCursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no'); +var_dump($cursor); + +?> +===DONE=== + +--EXPECTF-- +Cursor ID is zero: no +object(MongoDB\Driver\Cursor)#%d (%d) { + %a + ["session"]=> + NULL + %a +} + +Cursor ID is zero: yes +object(MongoDB\Driver\Cursor)#%d (%d) { + %a + ["session"]=> + NULL + %a +} +===DONE=== diff --git a/tests/cursor/cursor-session-003.phpt b/tests/cursor/cursor-session-003.phpt new file mode 100644 index 000000000..e74dce1ef --- /dev/null +++ b/tests/cursor/cursor-session-003.phpt @@ -0,0 +1,67 @@ +--TEST-- +MongoDB\Driver\Cursor debug output for command cursor includes explicit session +--SKIPIF-- + + + + + +--FILE-- +insert(['_id' => 1]); +$bulk->insert(['_id' => 2]); +$bulk->insert(['_id' => 3]); +$manager->executeBulkWrite(NS, $bulk); + +$command = new MongoDB\Driver\Command([ + 'aggregate' => COLLECTION_NAME, + 'pipeline' => [['$match' => new stdClass]], + 'cursor' => ['batchSize' => 2], +]); +$session = $manager->startSession(); + +$cursor = $manager->executeCommand(DATABASE_NAME, $command, ['session' => $session]); + +$iterator = new IteratorIterator($cursor); +$iterator->rewind(); +$iterator->next(); + +printf("Cursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no'); +var_dump($cursor); + +$iterator->next(); + +/* Per PHPC-1161, the Cursor will free a reference to the Session as soon as it + * is exhausted. While this is primarily done to ensure implicit sessions for + * command cursors are returned to the pool ASAP, it also applies to explicit + * sessions. */ +printf("\nCursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no'); +var_dump($cursor); + +?> +===DONE=== + +--EXPECTF-- +Cursor ID is zero: no +object(MongoDB\Driver\Cursor)#%d (%d) { + %a + ["session"]=> + object(MongoDB\Driver\Session)#%d (%d) { + %a + } + %a +} + +Cursor ID is zero: yes +object(MongoDB\Driver\Cursor)#%d (%d) { + %a + ["session"]=> + NULL + %a +} +===DONE=== diff --git a/tests/cursor/cursor-session-004.phpt b/tests/cursor/cursor-session-004.phpt new file mode 100644 index 000000000..a782a3a5a --- /dev/null +++ b/tests/cursor/cursor-session-004.phpt @@ -0,0 +1,66 @@ +--TEST-- +MongoDB\Driver\Cursor debug output for command cursor includes implicit session +--SKIPIF-- + + + + + +--FILE-- +insert(['_id' => 1]); +$bulk->insert(['_id' => 2]); +$bulk->insert(['_id' => 3]); +$manager->executeBulkWrite(NS, $bulk); + +$command = new MongoDB\Driver\Command([ + 'aggregate' => COLLECTION_NAME, + 'pipeline' => [['$match' => new stdClass]], + 'cursor' => ['batchSize' => 2], +]); + +$cursor = $manager->executeCommand(DATABASE_NAME, $command); + +$iterator = new IteratorIterator($cursor); +$iterator->rewind(); +$iterator->next(); + +printf("Cursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no'); +var_dump($cursor); + +$iterator->next(); + +/* Unlike implicit sessions for query cursors, which are handled internally by + * libmongoc, PHPC-1152 emulates its own implicit sessions for command cursors + * in order to ensure that command cursors always share the same session as the + * originating command. */ +printf("\nCursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no'); +var_dump($cursor); + +?> +===DONE=== + +--EXPECTF-- +Cursor ID is zero: no +object(MongoDB\Driver\Cursor)#%d (%d) { + %a + ["session"]=> + object(MongoDB\Driver\Session)#%d (%d) { + %a + } + %a +} + +Cursor ID is zero: yes +object(MongoDB\Driver\Cursor)#%d (%d) { + %a + ["session"]=> + NULL + %a +} +===DONE=== From 1f222d0f78d78113ea53c5f8cc797f4f995bef54 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Mon, 16 Apr 2018 16:37:08 -0400 Subject: [PATCH 4/4] PHPC-1151: Bump Cursor var_dump array size for Session This should have done in cc9030465881533036cfb9b60fc077de116f3964. --- src/MongoDB/Cursor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MongoDB/Cursor.c b/src/MongoDB/Cursor.c index be2452067..8c63f66f7 100644 --- a/src/MongoDB/Cursor.c +++ b/src/MongoDB/Cursor.c @@ -460,7 +460,7 @@ static HashTable *php_phongo_cursor_get_debug_info(zval *object, int *is_temp TS *is_temp = 1; intern = Z_CURSOR_OBJ_P(object); - array_init_size(&retval, 9); + array_init_size(&retval, 10); if (intern->database) { ADD_ASSOC_STRING(&retval, "database", intern->database);