Skip to content

Commit 79e33fc

Browse files
jmikolajmikola
committed
PHPC-2493: Relocate writeConcern option to executeBulkWriteCommand()
1 parent cf10b08 commit 79e33fc

10 files changed

+436
-54
lines changed

‎src/MongoDB/BulkWriteCommand.c

Lines changed: 7 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,6 @@ mongoc_bulkwriteopts_t* phongo_bwc_assemble_opts(php_phongo_bulkwritecommand_t*
5555
mongoc_bulkwriteopts_set_ordered(opts, intern->ordered);
5656
mongoc_bulkwriteopts_set_verboseresults(opts, intern->verbose);
5757

58-
if (intern->write_concern) {
59-
mongoc_bulkwriteopts_set_writeconcern(opts, intern->write_concern);
60-
}
61-
6258
return opts;
6359
}
6460

@@ -131,15 +127,13 @@ static PHP_METHOD(MongoDB_Driver_BulkWriteCommand, __construct)
131127
Z_PARAM_ARRAY_OR_NULL(zoptions)
132128
PHONGO_PARSE_PARAMETERS_END();
133129

134-
// TODO: Consider removing initialization for zero values
135-
intern->bw = mongoc_bulkwrite_new();
136-
intern->bypass = PHONGO_BULKWRITECOMMAND_BYPASS_UNSET;
137-
intern->comment = NULL;
138-
intern->let = NULL;
139-
intern->num_ops = 0;
140-
intern->ordered = true;
141-
intern->verbose = false;
142-
intern->write_concern = NULL;
130+
intern->bw = mongoc_bulkwrite_new();
131+
intern->bypass = PHONGO_BULKWRITECOMMAND_BYPASS_UNSET;
132+
intern->comment = NULL;
133+
intern->let = NULL;
134+
intern->num_ops = 0;
135+
intern->ordered = true;
136+
intern->verbose = false;
143137

144138
if (!zoptions) {
145139
return;
@@ -183,17 +177,6 @@ static PHP_METHOD(MongoDB_Driver_BulkWriteCommand, __construct)
183177
if (php_array_existsc(zoptions, "verboseResults")) {
184178
intern->verbose = php_array_fetchc_bool(zoptions, "verboseResults");
185179
}
186-
187-
if (php_array_existsc(zoptions, "writeConcern")) {
188-
zval* value = php_array_fetchc_deref(zoptions, "writeConcern");
189-
190-
if (Z_TYPE_P(value) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(value), php_phongo_writeconcern_ce)) {
191-
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Expected \"writeConcern\" option to be %s, %s given", ZSTR_VAL(php_phongo_writeconcern_ce->name), zend_zval_type_name(value));
192-
return;
193-
}
194-
195-
intern->write_concern = mongoc_write_concern_copy(phongo_write_concern_from_zval(value));
196-
}
197180
}
198181

199182
static PHP_METHOD(MongoDB_Driver_BulkWriteCommand, count)
@@ -763,10 +746,6 @@ static void php_phongo_bulkwritecommand_free_object(zend_object* object)
763746
if (!Z_ISUNDEF(intern->session)) {
764747
zval_ptr_dtor(&intern->session);
765748
}
766-
767-
if (intern->write_concern) {
768-
mongoc_write_concern_destroy(intern->write_concern);
769-
}
770749
}
771750

772751
static zend_object* php_phongo_bulkwritecommand_create_object(zend_class_entry* class_type)
@@ -828,15 +807,6 @@ static HashTable* php_phongo_bulkwritecommand_get_debug_info(zend_object* object
828807
ADD_ASSOC_NULL_EX(&retval, "session");
829808
}
830809

831-
if (intern->write_concern) {
832-
zval write_concern;
833-
834-
php_phongo_write_concern_to_zval(&write_concern, intern->write_concern);
835-
ADD_ASSOC_ZVAL_EX(&retval, "write_concern", &write_concern);
836-
} else {
837-
ADD_ASSOC_NULL_EX(&retval, "write_concern");
838-
}
839-
840810
done:
841811
return Z_ARRVAL(retval);
842812
}

‎src/phongo_execute.c

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -342,8 +342,10 @@ bool phongo_execute_bulkwritecommand(zval* manager, php_phongo_bulkwritecommand_
342342
mongoc_bulkwriteopts_t* bw_opts = NULL;
343343
mongoc_bulkwritereturn_t bw_ret = { 0 };
344344
php_phongo_bulkwritecommandresult_t* bwcr;
345-
zval* zsession = NULL;
346-
bool success = true;
345+
zval* zsession = NULL;
346+
zval* zwriteConcern = NULL;
347+
const mongoc_write_concern_t* write_concern = NULL;
348+
bool success = true;
347349

348350
client = Z_MANAGER_OBJ_P(manager)->client;
349351

@@ -352,16 +354,35 @@ bool phongo_execute_bulkwritecommand(zval* manager, php_phongo_bulkwritecommand_
352354
return false;
353355
}
354356

357+
if (!phongo_parse_write_concern(zoptions, NULL, &zwriteConcern)) {
358+
/* Exception should already have been thrown */
359+
return false;
360+
}
361+
362+
/* If a write concern was not specified, libmongoc will use the client's
363+
* write concern. Check if an unacknowledged write concern would conflict
364+
* with an explicit session. */
365+
write_concern = zwriteConcern ? Z_WRITECONCERN_OBJ_P(zwriteConcern)->write_concern : mongoc_client_get_write_concern(client);
366+
367+
if (zsession && !mongoc_write_concern_is_acknowledged(write_concern)) {
368+
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Cannot combine \"session\" option with an unacknowledged write concern");
369+
return false;
370+
}
371+
355372
mongoc_bulkwrite_set_client(bw, client);
356373

357374
bw_opts = phongo_bwc_assemble_opts(bwc);
358375
mongoc_bulkwriteopts_set_serverid(bw_opts, server_id);
359376

360377
if (zsession) {
361-
mongoc_bulkwrite_set_session(bw, Z_SESSION_OBJ_P(zsession)->client_session);
362378
/* Save a reference to the session on the class struct to avoid leaving
363379
* a dangling pointer within mongoc_bulkwrite_t. */
364380
ZVAL_ZVAL(&bwc->session, zsession, 1, 0);
381+
mongoc_bulkwrite_set_session(bw, Z_SESSION_OBJ_P(zsession)->client_session);
382+
}
383+
384+
if (zwriteConcern) {
385+
mongoc_bulkwriteopts_set_writeconcern(bw_opts, Z_WRITECONCERN_OBJ_P(zwriteConcern)->write_concern);
365386
}
366387

367388
bw_ret = mongoc_bulkwrite_execute(bw, bw_opts);

‎tests/bulkwritecommand/bulkwritecommand-debug-001.phpt

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
MongoDB\Driver\BulkWriteCommand debug output
3+
--FILE--
4+
<?php
5+
6+
require_once __DIR__ . "/../utils/basic.inc";
7+
8+
$bulk = new MongoDB\Driver\BulkWriteCommand([
9+
'bypassDocumentValidation' => true,
10+
'comment' => 'foo',
11+
'let' => ['foo' => 1],
12+
'ordered' => true,
13+
'verboseResults' => true,
14+
]);
15+
16+
var_dump($bulk);
17+
18+
?>
19+
===DONE===
20+
<?php exit(0); ?>
21+
--EXPECTF--
22+
object(MongoDB\Driver\BulkWriteCommand)#%d (%d) {
23+
["bypassDocumentValidation"]=>
24+
bool(true)
25+
["comment"]=>
26+
string(3) "foo"
27+
["let"]=>
28+
object(stdClass)#%d (%d) {
29+
["foo"]=>
30+
int(1)
31+
}
32+
["ordered"]=>
33+
bool(true)
34+
["verboseResults"]=>
35+
bool(true)
36+
["session"]=>
37+
NULL
38+
}
39+
===DONE===

‎tests/bulkwritecommand/bulkwritecommand-debug-002.phpt

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
--TEST--
2+
MongoDB\Driver\BulkWriteCommand debug output after execution
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php skip_if_not_live(); ?>
6+
<?php skip_if_server_version('<', '8.0'); ?>
7+
<?php skip_if_not_clean(); ?>
8+
--FILE--
9+
<?php
10+
11+
require_once __DIR__ . "/../utils/basic.inc";
12+
13+
$manager = create_test_manager();
14+
15+
$tests = [
16+
[],
17+
['session' => $manager->startSession()],
18+
];
19+
20+
foreach ($tests as $options) {
21+
$bulk = new MongoDB\Driver\BulkWriteCommand();
22+
$bulk->insertOne(NS, ['x' => 1]);
23+
$manager->executeBulkWriteCommand($bulk, $options);
24+
var_dump($bulk);
25+
}
26+
27+
?>
28+
===DONE===
29+
<?php exit(0); ?>
30+
--EXPECTF--
31+
object(MongoDB\Driver\BulkWriteCommand)#%d (%d) {
32+
["bypassDocumentValidation"]=>
33+
NULL
34+
["ordered"]=>
35+
bool(true)
36+
["verboseResults"]=>
37+
bool(false)
38+
["session"]=>
39+
NULL
40+
}
41+
object(MongoDB\Driver\BulkWriteCommand)#%d (%d) {
42+
["bypassDocumentValidation"]=>
43+
NULL
44+
["ordered"]=>
45+
bool(true)
46+
["verboseResults"]=>
47+
bool(false)
48+
["session"]=>
49+
object(MongoDB\Driver\Session)#%d (%d) {
50+
%a
51+
}
52+
}
53+
===DONE===

‎tests/bulkwritecommand/manager-executeBulkWriteCommand-001.phpt

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
--TEST--
2-
MongoDB\Driver\BulkWriteCommand::__construct()
2+
MongoDB\Driver\Manager::executeBulkWriteCommand()
33
--SKIPIF--
44
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
55
<?php skip_if_not_live(); ?>
@@ -26,25 +26,12 @@ $bulk->deleteMany(NS, ['_id' => ['$gt' => 2]]);
2626

2727
$result = $manager->executeBulkWriteCommand($bulk);
2828

29-
var_dump($bulk);
3029
var_dump($result);
3130

3231
?>
3332
===DONE===
3433
<?php exit(0); ?>
3534
--EXPECTF--
36-
object(MongoDB\Driver\BulkWriteCommand)#%d (%d) {
37-
["bypassDocumentValidation"]=>
38-
NULL
39-
["ordered"]=>
40-
bool(true)
41-
["verboseResults"]=>
42-
bool(true)
43-
["session"]=>
44-
NULL
45-
["write_concern"]=>
46-
NULL
47-
}
4835
object(MongoDB\Driver\BulkWriteCommandResult)#%d (%d) {
4936
["isAcknowledged"]=>
5037
bool(true)

‎tests/bulkwritecommand/manager-executeBulkWriteCommand_error-001.phpt

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
--TEST--
2+
MongoDB\Driver\Manager::executeBulkWriteCommand() prohibits session and unacknowledged write concern
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php skip_if_not_live(); ?>
6+
<?php skip_if_server_version('<', '8.0'); ?>
7+
<?php skip_if_not_clean(); ?>
8+
--FILE--
9+
<?php
10+
11+
require_once __DIR__ . "/../utils/basic.inc";
12+
13+
$manager = create_test_manager();
14+
15+
$bulk = new MongoDB\Driver\BulkWriteCommand();
16+
$bulk->insertOne(NS, ['_id' => 1]);
17+
18+
echo throws(function() use ($manager, $bulk) {
19+
$manager->executeBulkWriteCommand($bulk, [
20+
'session' => $manager->startSession(),
21+
'writeConcern' => new MongoDB\Driver\WriteConcern(0),
22+
]);
23+
}, MongoDB\Driver\Exception\InvalidArgumentException::class), "\n";
24+
25+
?>
26+
===DONE===
27+
<?php exit(0); ?>
28+
--EXPECT--
29+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
30+
Cannot combine "session" option with an unacknowledged write concern
31+
===DONE===

‎tests/bulkwritecommand/manager-executeBulkWriteCommand_error-002.phpt

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
--TEST--
2+
MongoDB\Driver\Manager::executeBulkWriteCommand() prohibits session and unacknowledged write concern (inherited)
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php skip_if_not_live(); ?>
6+
<?php skip_if_server_version('<', '8.0'); ?>
7+
<?php skip_if_not_clean(); ?>
8+
--FILE--
9+
<?php
10+
11+
require_once __DIR__ . "/../utils/basic.inc";
12+
13+
$manager = create_test_manager(URI, ['w' => 0]);
14+
15+
$bulk = new MongoDB\Driver\BulkWriteCommand();
16+
$bulk->insertOne(NS, ['_id' => 1]);
17+
18+
echo throws(function() use ($manager, $bulk) {
19+
$manager->executeBulkWriteCommand($bulk, [
20+
'session' => $manager->startSession(),
21+
]);
22+
}, MongoDB\Driver\Exception\InvalidArgumentException::class), "\n";
23+
24+
?>
25+
===DONE===
26+
<?php exit(0); ?>
27+
--EXPECT--
28+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
29+
Cannot combine "session" option with an unacknowledged write concern
30+
===DONE===

0 commit comments

Comments
 (0)