Skip to content

Commit 46711f5

Browse files
jmikolajmikola
committed
Merge pull request #814 
2 parents 2097bf3 + cceeda8 commit 46711f5

6 files changed

+297
-15
lines changed

‎php_phongo.c

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,17 @@ bool phongo_execute_bulk_write(mongoc_client_t *client, const char *namespace, p
673673
return false;
674674
}
675675

676+
/* If a write concern was not specified, libmongoc will use the client's
677+
* write concern; however, we should still fetch it for the write result.
678+
* Additionally, we need to check if an unacknowledged write concern would
679+
* conflict with an explicit session. */
680+
write_concern = zwriteConcern ? Z_WRITECONCERN_OBJ_P(zwriteConcern)->write_concern : mongoc_client_get_write_concern(client);
681+
682+
if (zsession && !mongoc_write_concern_is_acknowledged(write_concern)) {
683+
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Cannot combine \"session\" option with an unacknowledged write concern");
684+
return false;
685+
}
686+
676687
mongoc_bulk_operation_set_database(bulk, bulk_write->database);
677688
mongoc_bulk_operation_set_collection(bulk, bulk_write->collection);
678689
mongoc_bulk_operation_set_client(bulk, client);
@@ -682,13 +693,8 @@ bool phongo_execute_bulk_write(mongoc_client_t *client, const char *namespace, p
682693
mongoc_bulk_operation_set_client_session(bulk, Z_SESSION_OBJ_P(zsession)->client_session);
683694
}
684695

685-
/* If a write concern was not specified, libmongoc will use the client's
686-
* write concern; however, we should still fetch it for the write result. */
687-
write_concern = phongo_write_concern_from_zval(zwriteConcern TSRMLS_CC);
688-
if (write_concern) {
689-
mongoc_bulk_operation_set_write_concern(bulk, write_concern);
690-
} else {
691-
write_concern = mongoc_client_get_write_concern(client);
696+
if (zwriteConcern) {
697+
mongoc_bulk_operation_set_write_concern(bulk, Z_WRITECONCERN_OBJ_P(zwriteConcern)->write_concern);
692698
}
693699

694700
success = mongoc_bulk_operation_execute(bulk, &reply, &error);
@@ -862,6 +868,7 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t
862868
bool result = false;
863869
bool free_reply = false;
864870
bool free_zsession = false;
871+
bool is_unacknowledged_write_concern = false;
865872

866873
command = Z_COMMAND_OBJ_P(zcommand);
867874

@@ -880,9 +887,33 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t
880887
goto cleanup;
881888
}
882889

883-
/* If an explicit session was not provided, attempt to create an implicit
884-
* client session (ignoring any errors). */
885-
if (!zsession) {
890+
if (type & PHONGO_OPTION_WRITE_CONCERN) {
891+
zval *zwriteConcern = NULL;
892+
893+
if (!phongo_parse_write_concern(options, &opts, &zwriteConcern TSRMLS_CC)) {
894+
/* Exception should already have been thrown */
895+
goto cleanup;
896+
}
897+
898+
/* Determine if the explicit or inherited write concern is
899+
* unacknowledged so that we can ensure it does not conflict with an
900+
* explicit or implicit session. */
901+
if (zwriteConcern) {
902+
is_unacknowledged_write_concern = !mongoc_write_concern_is_acknowledged(Z_WRITECONCERN_OBJ_P(zwriteConcern)->write_concern);
903+
} else if (type != PHONGO_COMMAND_RAW) {
904+
is_unacknowledged_write_concern = !mongoc_write_concern_is_acknowledged(mongoc_client_get_write_concern(client));
905+
}
906+
}
907+
908+
if (zsession && is_unacknowledged_write_concern) {
909+
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Cannot combine \"session\" option with an unacknowledged write concern");
910+
goto cleanup;
911+
}
912+
913+
/* If an explicit session was not provided and the effective write concern
914+
* is not unacknowledged, attempt to create an implicit client session
915+
* (ignoring any errors). */
916+
if (!zsession && !is_unacknowledged_write_concern) {
886917
zsession = phongo_create_implicit_session(client TSRMLS_CC);
887918

888919
if (zsession) {
@@ -895,11 +926,6 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t
895926
}
896927
}
897928

898-
if ((type & PHONGO_OPTION_WRITE_CONCERN) && !phongo_parse_write_concern(options, &opts, NULL TSRMLS_CC)) {
899-
/* Exception should already have been thrown */
900-
goto cleanup;
901-
}
902-
903929
if (!BSON_APPEND_INT32(&opts, "serverId", server_id)) {
904930
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Error appending \"serverId\" option");
905931
goto cleanup;

‎tests/manager/bug1163-001.phpt

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
--TEST--
2+
PHPC-1163: Unacknowledged write concern should omit implicit session
3+
--XFAIL--
4+
Depends on CDRIVER-2615
5+
--SKIPIF--
6+
<?php if (PHP_INT_SIZE !== 8) { die("skip Can't represent 64-bit ints on a 32-bit platform"); } ?>
7+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
8+
<?php NEEDS('STANDALONE'); ?>
9+
<?php CLEANUP(STANDALONE); ?>
10+
--FILE--
11+
<?php
12+
require_once __DIR__ . "/../utils/basic.inc";
13+
14+
class Test implements MongoDB\Driver\Monitoring\CommandSubscriber
15+
{
16+
public function run()
17+
{
18+
$manager = new MongoDB\Driver\Manager(STANDALONE, ['w' => 0]);
19+
20+
MongoDB\Driver\Monitoring\addSubscriber($this);
21+
22+
$bulk = new MongoDB\Driver\BulkWrite;
23+
$bulk->insert(['x' => 1]);
24+
25+
echo "Testing executeBulkWrite\n";
26+
$manager->executeBulkWrite(NS, $bulk);
27+
28+
$command = new MongoDB\Driver\Command([
29+
'insert' => COLLECTION_NAME,
30+
'documents' => [['x' => 1]],
31+
]);
32+
33+
/* Note: executeCommand() and executeReadCommand() are not tested
34+
* because they do not inherit the client-level write concern. */
35+
36+
echo "\nTesting executeWriteCommand\n";
37+
$manager->executeWriteCommand(DATABASE_NAME, $command);
38+
39+
/* We can safely re-use the insert command with executeReadWriteCommand
40+
* because there is no readConcern to inherit. */
41+
echo "\nTesting executeReadWriteCommand\n";
42+
$manager->executeReadWriteCommand(DATABASE_NAME, $command);
43+
44+
MongoDB\Driver\Monitoring\removeSubscriber($this);
45+
}
46+
47+
public function commandStarted(MongoDB\Driver\Monitoring\CommandStartedEvent $event)
48+
{
49+
if ($event->getCommandName() === 'insert') {
50+
$command = $event->getCommand();
51+
$hasSession = isset($command->lsid);
52+
$writeConcern = isset($command->writeConcern) ? $command->writeConcern: null;
53+
54+
printf("insert command write concern: %s\n", json_encode($writeConcern));
55+
printf("insert command has session: %s\n", $hasSession ? 'yes' : 'no');
56+
}
57+
}
58+
59+
public function commandSucceeded(MongoDB\Driver\Monitoring\CommandSucceededEvent $event)
60+
{
61+
}
62+
63+
public function commandFailed(MongoDB\Driver\Monitoring\CommandFailedEvent $event)
64+
{
65+
}
66+
}
67+
68+
(new Test)->run();
69+
70+
?>
71+
===DONE===
72+
<?php exit(0); ?>
73+
--EXPECT--
74+
Testing executeBulkWrite
75+
insert command write concern: {"w":0}
76+
insert command has session: no
77+
78+
Testing executeWriteCommand
79+
insert command write concern: {"w":0}
80+
insert command has session: no
81+
82+
Testing executeReadWriteCommand
83+
insert command write concern: {"w":0}
84+
insert command has session: no
85+
===DONE===

‎tests/manager/manager-executeBulkWrite_error-010.phpt

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
MongoDB\Driver\Manager::executeBulkWrite() cannot combine session with unacknowledged write concern
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php NEEDS_CRYPTO(); ?>
6+
<?php NEEDS('STANDALONE'); ?>
7+
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
8+
--FILE--
9+
<?php
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
echo throws(function() {
13+
$manager = new MongoDB\Driver\Manager(STANDALONE);
14+
15+
$bulk = new MongoDB\Driver\BulkWrite;
16+
$bulk->insert(['x' => 1]);
17+
18+
$manager->executeBulkWrite(NS, $bulk, [
19+
'session' => $manager->startSession(),
20+
'writeConcern' => new MongoDB\Driver\WriteConcern(0),
21+
]);
22+
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
23+
24+
echo throws(function() {
25+
$manager = new MongoDB\Driver\Manager(STANDALONE, ['w' => 0]);
26+
27+
$bulk = new MongoDB\Driver\BulkWrite;
28+
$bulk->insert(['x' => 1]);
29+
30+
$manager->executeBulkWrite(NS, $bulk, [
31+
'session' => $manager->startSession(),
32+
]);
33+
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
34+
35+
?>
36+
===DONE===
37+
<?php exit(0); ?>
38+
--EXPECT--
39+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
40+
Cannot combine "session" option with an unacknowledged write concern
41+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
42+
Cannot combine "session" option with an unacknowledged write concern
43+
===DONE===

‎tests/manager/manager-executeCommand_error-003.phpt

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
MongoDB\Driver\Manager::executeCommand() cannot combine session with unacknowledged write concern
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php NEEDS_CRYPTO(); ?>
6+
<?php NEEDS('STANDALONE'); ?>
7+
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
8+
--FILE--
9+
<?php
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
echo throws(function() {
13+
$manager = new MongoDB\Driver\Manager(STANDALONE);
14+
15+
$command = new MongoDB\Driver\Command([
16+
'insert' => COLLECTION_NAME,
17+
'documents' => [['x' => 1]],
18+
]);
19+
20+
$manager->executeCommand(DATABASE_NAME, $command, [
21+
'session' => $manager->startSession(),
22+
'writeConcern' => new MongoDB\Driver\WriteConcern(0),
23+
]);
24+
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
25+
26+
?>
27+
===DONE===
28+
<?php exit(0); ?>
29+
--EXPECT--
30+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
31+
Cannot combine "session" option with an unacknowledged write concern
32+
===DONE===

‎tests/manager/manager-executeReadWriteCommand_error-002.phpt

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
MongoDB\Driver\Manager::executeReadWriteCommand() cannot combine session with unacknowledged write concern
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php NEEDS_CRYPTO(); ?>
6+
<?php NEEDS('STANDALONE'); ?>
7+
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
8+
--FILE--
9+
<?php
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
/* insert should not normally be used with executeReadWriteCommand(), but we are
13+
* only testing executeReadWriteCommand()'s option validation. */
14+
echo throws(function() {
15+
$manager = new MongoDB\Driver\Manager(STANDALONE);
16+
17+
$command = new MongoDB\Driver\Command([
18+
'insert' => COLLECTION_NAME,
19+
'documents' => [['x' => 1]],
20+
]);
21+
22+
$manager->executeReadWriteCommand(DATABASE_NAME, $command, [
23+
'session' => $manager->startSession(),
24+
'writeConcern' => new MongoDB\Driver\WriteConcern(0),
25+
]);
26+
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
27+
28+
echo throws(function() {
29+
$manager = new MongoDB\Driver\Manager(STANDALONE, ['w' => 0]);
30+
31+
$command = new MongoDB\Driver\Command([
32+
'insert' => COLLECTION_NAME,
33+
'documents' => [['x' => 1]],
34+
]);
35+
36+
$manager->executeReadWriteCommand(DATABASE_NAME, $command, [
37+
'session' => $manager->startSession(),
38+
]);
39+
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
40+
41+
?>
42+
===DONE===
43+
<?php exit(0); ?>
44+
--EXPECT--
45+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
46+
Cannot combine "session" option with an unacknowledged write concern
47+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
48+
Cannot combine "session" option with an unacknowledged write concern
49+
===DONE===

‎tests/manager/manager-executeWriteCommand_error-002.phpt

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
--TEST--
2+
MongoDB\Driver\Manager::executeWriteCommand() cannot combine session with unacknowledged write concern
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php NEEDS_CRYPTO(); ?>
6+
<?php NEEDS('STANDALONE'); ?>
7+
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
8+
--FILE--
9+
<?php
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
echo throws(function() {
13+
$manager = new MongoDB\Driver\Manager(STANDALONE);
14+
15+
$command = new MongoDB\Driver\Command([
16+
'insert' => COLLECTION_NAME,
17+
'documents' => [['x' => 1]],
18+
]);
19+
20+
$manager->executeWriteCommand(DATABASE_NAME, $command, [
21+
'session' => $manager->startSession(),
22+
'writeConcern' => new MongoDB\Driver\WriteConcern(0),
23+
]);
24+
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
25+
26+
echo throws(function() {
27+
$manager = new MongoDB\Driver\Manager(STANDALONE, ['w' => 0]);
28+
29+
$command = new MongoDB\Driver\Command([
30+
'insert' => COLLECTION_NAME,
31+
'documents' => [['x' => 1]],
32+
]);
33+
34+
$manager->executeWriteCommand(DATABASE_NAME, $command, [
35+
'session' => $manager->startSession(),
36+
]);
37+
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
38+
39+
?>
40+
===DONE===
41+
<?php exit(0); ?>
42+
--EXPECT--
43+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
44+
Cannot combine "session" option with an unacknowledged write concern
45+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
46+
Cannot combine "session" option with an unacknowledged write concern
47+
===DONE===

0 commit comments

Comments
 (0)