Skip to content
This repository was archived by the owner on Feb 1, 2022. It is now read-only.

PHP-1510: Fix tests for legacy opcode rerouting #877

Merged
merged 4 commits into from
Apr 25, 2016
Merged

PHP-1510: Fix tests for legacy opcode rerouting #877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
419ccbe
PHP-1510: Extra is_gle_op() calls should be silent
jmikola Apr 21, 2016
ba9c5d7
PHP-1510: Fix assertions from write methods return values
jmikola Apr 21, 2016
f9d8e51
PHP-1510: Fix assertions for re-routed unacknowledged writes
jmikola Apr 21, 2016
16c9a1e
PHP-1511: append_gle should ignore unset fsync/journal from link
jmikola Apr 21, 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
38 changes: 24 additions & 14 deletions collection.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ ZEND_EXTERN_MODULE_GLOBALS(mongo)

zend_class_entry *mongo_ce_Collection = NULL;

static int is_gle_op(zval *coll, zval *options, mongo_server_options *server_options TSRMLS_DC);
static int is_gle_op(zval *coll, zval *options, mongo_server_options *server_options, int silent TSRMLS_DC);
Copy link
Member Author

Choose a reason for hiding this comment

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

From the commit message:

The extra is_gle_op() call was causing duplicate "safe" deprecation notices and MongoLog output in some tests. Existing code paths for write commands and legacy ops already include necessary output, so allow is_gle_op() to be silent when invoked for the purpose of routing unacknowledged writes to legacy opcodes.

static void do_gle_op(mongo_con_manager *manager, mongo_connection *connection, zval *cursor_z, mongo_buffer *buf, zval *return_value TSRMLS_DC);
static zval* append_getlasterror(zval *coll, mongo_buffer *buf, zval *options, mongo_connection *connection TSRMLS_DC);
static char *to_index_string(zval *zkeys, int *key_len TSRMLS_DC);
Expand Down Expand Up @@ -378,11 +378,13 @@ static zval* append_getlasterror(zval *coll, mongo_buffer *buf, zval *options, m
php_error_docref(NULL TSRMLS_CC, E_DEPRECATED, "The 'MongoCursor::$timeout' static property is deprecated, please call MongoCursor->timeout() instead");
}

/* Get the default value for journalling */
fsync = link->servers->options.default_fsync;
journal = link->servers->options.default_journal;

/* Read the default_* properties from the link */
if (link->servers->options.default_fsync != -1) {
fsync = link->servers->options.default_fsync;
}
if (link->servers->options.default_journal != -1) {
journal = link->servers->options.default_journal;
}
if (link->servers->options.default_w != -1) {
w = link->servers->options.default_w;
}
Expand Down Expand Up @@ -609,7 +611,7 @@ static int send_message(zval *this_ptr, mongo_connection *connection, mongo_buff
return 0;
}

if (is_gle_op(this_ptr, options, &link->servers->options TSRMLS_CC)) {
if (is_gle_op(this_ptr, options, &link->servers->options, NOISY TSRMLS_CC)) {
zval *cursor = append_getlasterror(this_ptr, buf, options, connection TSRMLS_CC);
if (cursor) {
do_gle_op(link->manager, connection, cursor, buf, return_value TSRMLS_CC);
Expand All @@ -632,7 +634,7 @@ static int send_message(zval *this_ptr, mongo_connection *connection, mongo_buff
* on whether a write concern ("w" or "safe") or fsync/journal options are
* specified.
*/
static int is_gle_op(zval *coll, zval *options, mongo_server_options *server_options TSRMLS_DC)
static int is_gle_op(zval *coll, zval *options, mongo_server_options *server_options, int silent TSRMLS_DC)
{
int gle_op = 0, default_fsync, default_journal, coll_w = 0;
zval *z_coll_w;
Expand Down Expand Up @@ -674,10 +676,14 @@ static int is_gle_op(zval *coll, zval *options, mongo_server_options *server_opt
}
break;
default:
php_error_docref(NULL TSRMLS_CC, E_WARNING, "The value of the 'w' option either needs to be a integer or string");
if (silent == NOISY) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "The value of the 'w' option either needs to be a integer or string");
}
}
} else if (zend_hash_find(HASH_P(options), "safe", strlen("safe") + 1, (void**) &gle_pp) == SUCCESS) {
php_error_docref(NULL TSRMLS_CC, E_DEPRECATED, "The 'safe' option is deprecated, please use 'w' instead");
if (silent == NOISY) {
php_error_docref(NULL TSRMLS_CC, E_DEPRECATED, "The 'safe' option is deprecated, please use 'w' instead");
}

switch (Z_TYPE_PP(gle_pp)) {
case IS_STRING:
Expand All @@ -691,7 +697,9 @@ static int is_gle_op(zval *coll, zval *options, mongo_server_options *server_opt
}
break;
default:
php_error_docref(NULL TSRMLS_CC, E_WARNING, "The value of the 'safe' option either needs to be a boolean or a string");
if (silent == NOISY) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "The value of the 'safe' option either needs to be a boolean or a string");
}
}
} else if (coll_w >= 1) {
gle_op = 1;
Expand Down Expand Up @@ -722,7 +730,9 @@ static int is_gle_op(zval *coll, zval *options, mongo_server_options *server_opt
gle_op = (coll_w >= 1 || default_fsync == 1 || default_journal == 1);
}

mongo_manager_log(MonGlo(manager), MLOG_IO, MLOG_FINE, "is_gle_op: %s", gle_op ? "yes" : "no");
if (silent == NOISY) {
mongo_manager_log(MonGlo(manager), MLOG_IO, MLOG_FINE, "is_gle_op: %s", gle_op ? "yes" : "no");
}
return gle_op;
}

Expand Down Expand Up @@ -1135,7 +1145,7 @@ static void php_mongo_collection_insert(zval *z_collection, zval *document, zval
RETURN_FALSE;
}

is_acknowledged = is_gle_op(z_collection, z_write_options, &link->servers->options TSRMLS_CC);
is_acknowledged = is_gle_op(z_collection, z_write_options, &link->servers->options, QUIET TSRMLS_CC);
supports_command = php_mongo_api_connection_supports_feature(connection, PHP_MONGO_API_WRITE_API);
supports_opcode = php_mongo_api_connection_supports_feature(connection, PHP_MONGO_API_RELEASE_2_4_AND_BEFORE);

Expand Down Expand Up @@ -1518,7 +1528,7 @@ static void php_mongocollection_update(zval *this_ptr, mongo_collection *c, zval
RETURN_FALSE;
}

is_acknowledged = is_gle_op(this_ptr, z_write_options, &link->servers->options TSRMLS_CC);
is_acknowledged = is_gle_op(this_ptr, z_write_options, &link->servers->options, QUIET TSRMLS_CC);
supports_command = php_mongo_api_connection_supports_feature(connection, PHP_MONGO_API_WRITE_API);
supports_opcode = php_mongo_api_connection_supports_feature(connection, PHP_MONGO_API_RELEASE_2_4_AND_BEFORE);

Expand Down Expand Up @@ -1637,7 +1647,7 @@ static void php_mongocollection_remove(zval *this_ptr, mongo_collection *c, zval
RETURN_FALSE;
}

is_acknowledged = is_gle_op(this_ptr, z_write_options, &link->servers->options TSRMLS_CC);
is_acknowledged = is_gle_op(this_ptr, z_write_options, &link->servers->options, QUIET TSRMLS_CC);
supports_command = php_mongo_api_connection_supports_feature(connection, PHP_MONGO_API_WRITE_API);
supports_opcode = php_mongo_api_connection_supports_feature(connection, PHP_MONGO_API_RELEASE_2_4_AND_BEFORE);

Expand Down
12 changes: 6 additions & 6 deletions tests/standalone/bug01085-001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ Test for PHP-1085: w=0 returns unexpected exception on failure (socketTimeoutMS

require_once "tests/utils/server.inc";

function assertFalse($value) {
function assertTrue($value) {
if ( ! is_bool($value)) {
printf("Expected boolean type but received %s\n", gettype($value));
return;
}

if ($value !== false) {
echo "Expected boolean false but received boolean true\n";
if ($value !== true) {
echo "Expected boolean true but received boolean false\n";
}
}

Expand All @@ -34,7 +34,7 @@ for ($i = 0; $i < 10; ++$i) {
array('x' => $i, 'y' => str_repeat('a', 4*1024*1024)),
array('w' => 0)
);
assertFalse($retval);
assertTrue($retval);
Copy link
Member Author

Choose a reason for hiding this comment

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

From the commit message:

Now that unacknowledged writes are routed through legacy opcodes, these write methods return true (as documented).

It appears that the false assertion was due to mongo_convert_write_api_return_to_legacy_retval() converting the response from an unacknowledged write command (likely an empty array) to a boolean.

}

echo "Testing update() with w=0\n";
Expand All @@ -44,15 +44,15 @@ $retval = $collection->update(
array('$set' => array('y' => 1)),
array('w' => 0)
);
assertFalse($retval);
assertTrue($retval);

echo "Testing remove() with w=0\n";

$retval = $collection->remove(
array('$where' => 'sleep(1) && false'),
array('w' => 0)
);
assertFalse($retval);
assertTrue($retval);

echo "Testing update() with w=1\n";

Expand Down
12 changes: 6 additions & 6 deletions tests/standalone/bug01085-002.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ Test for PHP-1085: w=0 returns unexpected exception on failure (socketTimeoutMS

require_once "tests/utils/server.inc";

function assertFalse($value) {
function assertTrue($value) {
if ( ! is_bool($value)) {
printf("Expected boolean type but received %s\n", gettype($value));
return;
}

if ($value !== false) {
echo "Expected boolean false but received boolean true\n";
if ($value !== true) {
echo "Expected boolean true but received boolean false\n";
}
}

Expand All @@ -32,7 +32,7 @@ for ($i = 0; $i < 10; ++$i) {
array('x' => $i, 'y' => str_repeat('a', 4*1024*1024)),
array('w' => 0, 'socketTimeoutMS' => 1)
);
assertFalse($retval);
assertTrue($retval);
}

echo "Testing update() with w=0\n";
Expand All @@ -42,15 +42,15 @@ $retval = $collection->update(
array('$set' => array('y' => 1)),
array('w' => 0, 'socketTimeoutMS' => 1)
);
assertFalse($retval);
assertTrue($retval);

echo "Testing remove() with w=0\n";

$retval = $collection->remove(
array('$where' => 'sleep(1) && false'),
array('w' => 0, 'socketTimeoutMS' => 1)
);
assertFalse($retval);
assertTrue($retval);

echo "Testing update() with w=1\n";

Expand Down
52 changes: 10 additions & 42 deletions tests/standalone/mongoclient-connection-options-fsync-2.6.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,11 @@ dump_writeConcern($mn);

echo "Setting it to false, per-query, and w=0 to force no-gle\n";
$doc = array("doc" => "ument");
printLogs(MongoLog::IO, MongoLog::FINE, '/is_gle_op/');
Copy link
Member Author

Choose a reason for hiding this comment

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

From the commit message:

Stream notifications cannot be used to assert the write concern of unacknowledged writes, since they are routed through legacy opcodes instead of write commands. Use MongoLog to assert that no GLE is issued for these writes.

$mc->test->bug572->insert($doc, array("fsync" => false, "w" => 0));
dump_writeConcern($mn);
$mc->test->bug572->update(array("_id" => $doc["_id"]), array("updated" => "doc"), array("fsync" => false, "w" => 0));
dump_writeConcern($mn);
$mc->test->bug572->remove(array("_id" => $doc["_id"]), array("fsync" => false, "w" => 0));
dump_writeConcern($mn);
MongoLog::setLevel(MongoLog::NONE);

$mc = new MongoClient($host, array("fsync" => false), array("context" => $ctx));

Expand All @@ -78,12 +77,11 @@ $mc = new MongoClient($host, array("fsync" => false, "w" => 0), array("context"

echo "Fsync disabled by default, and gle\n";
$doc = array("doc" => "ument");
printLogs(MongoLog::IO, MongoLog::FINE, '/is_gle_op/');
$mc->test->bug572->insert($doc);
dump_writeConcern($mn);
$mc->test->bug572->update(array("_id" => $doc["_id"]), array("updated" => "doc"));
dump_writeConcern($mn);
$mc->test->bug572->remove(array("_id" => $doc["_id"]));
dump_writeConcern($mn);
MongoLog::setLevel(MongoLog::NONE);

echo "Setting it to true, per-query, with gle=0\n";
$doc = array("doc" => "ument");
Expand Down Expand Up @@ -135,24 +133,9 @@ array(2) {
int(1)
}
Setting it to false, per-query, and w=0 to force no-gle
array(2) {
["fsync"]=>
bool(false)
["w"]=>
int(0)
}
array(2) {
["fsync"]=>
bool(false)
["w"]=>
int(0)
}
array(2) {
["fsync"]=>
bool(false)
["w"]=>
int(0)
}
is_gle_op: no
is_gle_op: no
is_gle_op: no
Fsync disabled by default
array(2) {
["fsync"]=>
Expand Down Expand Up @@ -192,24 +175,9 @@ array(2) {
int(1)
}
Fsync disabled by default, and gle
array(2) {
["fsync"]=>
bool(false)
["w"]=>
int(0)
}
array(2) {
["fsync"]=>
bool(false)
["w"]=>
int(0)
}
array(2) {
["fsync"]=>
bool(false)
["w"]=>
int(0)
}
is_gle_op: no
is_gle_op: no
is_gle_op: no
Setting it to true, per-query, with gle=0
array(2) {
["fsync"]=>
Expand Down
52 changes: 10 additions & 42 deletions tests/standalone/mongoclient-connection-options-journal-2.6.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,11 @@ dump_writeConcern($mn);

echo "Setting it to false, per-query, and w=0 to force no-gle\n";
$doc = array("doc" => "ument");
printLogs(MongoLog::IO, MongoLog::FINE, '/is_gle_op/');
$mc->test->bug572->insert($doc, array("j" => false, "w" => 0));
dump_writeConcern($mn);
$mc->test->bug572->update(array("_id" => $doc["_id"]), array("updated" => "doc"), array("j" => false, "w" => 0));
dump_writeConcern($mn);
$mc->test->bug572->remove(array("_id" => $doc["_id"]), array("j" => false, "w" => 0));
dump_writeConcern($mn);
MongoLog::setLevel(MongoLog::NONE);

$mc = new MongoClient($host, array("journal" => false), array("context" => $ctx));

Expand All @@ -79,12 +78,11 @@ $mc = new MongoClient($host, array("journal" => false, "w" => 0), array("context

echo "journal disabled by default, and gle\n";
$doc = array("doc" => "ument");
printLogs(MongoLog::IO, MongoLog::FINE, '/is_gle_op/');
$mc->test->bug572->insert($doc);
dump_writeConcern($mn);
$mc->test->bug572->update(array("_id" => $doc["_id"]), array("updated" => "doc"));
dump_writeConcern($mn);
$mc->test->bug572->remove(array("_id" => $doc["_id"]));
dump_writeConcern($mn);
MongoLog::setLevel(MongoLog::NONE);

echo "Setting it to true, per-query, with gle=0\n";
$doc = array("doc" => "ument");
Expand Down Expand Up @@ -136,24 +134,9 @@ array(2) {
int(1)
}
Setting it to false, per-query, and w=0 to force no-gle
array(2) {
["j"]=>
bool(false)
["w"]=>
int(0)
}
array(2) {
["j"]=>
bool(false)
["w"]=>
int(0)
}
array(2) {
["j"]=>
bool(false)
["w"]=>
int(0)
}
is_gle_op: no
is_gle_op: no
is_gle_op: no
journal disabled by default
array(2) {
["j"]=>
Expand Down Expand Up @@ -193,24 +176,9 @@ array(2) {
int(1)
}
journal disabled by default, and gle
array(2) {
["j"]=>
bool(false)
["w"]=>
int(0)
}
array(2) {
["j"]=>
bool(false)
["w"]=>
int(0)
}
array(2) {
["j"]=>
bool(false)
["w"]=>
int(0)
}
is_gle_op: no
is_gle_op: no
is_gle_op: no
Setting it to true, per-query, with gle=0
array(2) {
["j"]=>
Expand Down