PHPC-2493: Relocate writeConcern option to executeBulkWriteCommand()

jmikola · jmikola · commit 786f1ccbe382 · 2025-02-28T12:57:49.000-05:00
diff --git a/src/MongoDB/BulkWriteCommand.c b/src/MongoDB/BulkWriteCommand.c
@@ -55,10 +55,6 @@ mongoc_bulkwriteopts_t* phongo_bwc_assemble_opts(php_phongo_bulkwritecommand_t*
 	mongoc_bulkwriteopts_set_ordered(opts, intern->ordered);
 	mongoc_bulkwriteopts_set_verboseresults(opts, intern->verbose);
 
-	if (intern->write_concern) {
-		mongoc_bulkwriteopts_set_writeconcern(opts, intern->write_concern);
-	}
-
 	return opts;
 }
 
@@ -131,15 +127,13 @@ static PHP_METHOD(MongoDB_Driver_BulkWriteCommand, __construct)
 	Z_PARAM_ARRAY_OR_NULL(zoptions)
 	PHONGO_PARSE_PARAMETERS_END();
 
-	// TODO: Consider removing initialization for zero values
-	intern->bw            = mongoc_bulkwrite_new();
-	intern->bypass        = PHONGO_BULKWRITECOMMAND_BYPASS_UNSET;
-	intern->comment       = NULL;
-	intern->let           = NULL;
-	intern->num_ops       = 0;
-	intern->ordered       = true;
-	intern->verbose       = false;
-	intern->write_concern = NULL;
+	intern->bw      = mongoc_bulkwrite_new();
+	intern->bypass  = PHONGO_BULKWRITECOMMAND_BYPASS_UNSET;
+	intern->comment = NULL;
+	intern->let     = NULL;
+	intern->num_ops = 0;
+	intern->ordered = true;
+	intern->verbose = false;
 
 	if (!zoptions) {
 		return;
@@ -183,17 +177,6 @@ static PHP_METHOD(MongoDB_Driver_BulkWriteCommand, __construct)
 	if (php_array_existsc(zoptions, "verboseResults")) {
 		intern->verbose = php_array_fetchc_bool(zoptions, "verboseResults");
 	}
-
-	if (php_array_existsc(zoptions, "writeConcern")) {
-		zval* value = php_array_fetchc_deref(zoptions, "writeConcern");
-
-		if (Z_TYPE_P(value) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(value), php_phongo_writeconcern_ce)) {
-			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));
-			return;
-		}
-
-		intern->write_concern = mongoc_write_concern_copy(phongo_write_concern_from_zval(value));
-	}
 }
 
 static PHP_METHOD(MongoDB_Driver_BulkWriteCommand, count)
@@ -763,10 +746,6 @@ static void php_phongo_bulkwritecommand_free_object(zend_object* object)
 	if (!Z_ISUNDEF(intern->session)) {
 		zval_ptr_dtor(&intern->session);
 	}
-
-	if (intern->write_concern) {
-		mongoc_write_concern_destroy(intern->write_concern);
-	}
 }
 
 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
 		ADD_ASSOC_NULL_EX(&retval, "session");
 	}
 
-	if (intern->write_concern) {
-		zval write_concern;
-
-		php_phongo_write_concern_to_zval(&write_concern, intern->write_concern);
-		ADD_ASSOC_ZVAL_EX(&retval, "write_concern", &write_concern);
-	} else {
-		ADD_ASSOC_NULL_EX(&retval, "write_concern");
-	}
-
 done:
 	return Z_ARRVAL(retval);
 }
diff --git a/src/phongo_execute.c b/src/phongo_execute.c
@@ -342,8 +342,10 @@ bool phongo_execute_bulkwritecommand(zval* manager, php_phongo_bulkwritecommand_
 	mongoc_bulkwriteopts_t*              bw_opts = NULL;
 	mongoc_bulkwritereturn_t             bw_ret  = { 0 };
 	php_phongo_bulkwritecommandresult_t* bwcr;
-	zval*                                zsession = NULL;
-	bool                                 success  = true;
+	zval*                                zsession      = NULL;
+	zval*                                zwriteConcern = NULL;
+	const mongoc_write_concern_t*        write_concern = NULL;
+	bool                                 success       = true;
 
 	client = Z_MANAGER_OBJ_P(manager)->client;
 
@@ -352,16 +354,35 @@ bool phongo_execute_bulkwritecommand(zval* manager, php_phongo_bulkwritecommand_
 		return false;
 	}
 
+	if (!phongo_parse_write_concern(zoptions, NULL, &zwriteConcern)) {
+		/* Exception should already have been thrown */
+		return false;
+	}
+
+	/* If a write concern was not specified, libmongoc will use the client's
+	 * write concern. Check if an unacknowledged write concern would conflict
+	 * with an explicit session. */
+	write_concern = zwriteConcern ? Z_WRITECONCERN_OBJ_P(zwriteConcern)->write_concern : mongoc_client_get_write_concern(client);
+
+	if (zsession && !mongoc_write_concern_is_acknowledged(write_concern)) {
+		phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Cannot combine \"session\" option with an unacknowledged write concern");
+		return false;
+	}
+
 	mongoc_bulkwrite_set_client(bw, client);
 
 	bw_opts = phongo_bwc_assemble_opts(bwc);
 	mongoc_bulkwriteopts_set_serverid(bw_opts, server_id);
 
 	if (zsession) {
-		mongoc_bulkwrite_set_session(bw, Z_SESSION_OBJ_P(zsession)->client_session);
 		/* Save a reference to the session on the class struct to avoid leaving
 		 * a dangling pointer within mongoc_bulkwrite_t. */
 		ZVAL_ZVAL(&bwc->session, zsession, 1, 0);
+		mongoc_bulkwrite_set_session(bw, Z_SESSION_OBJ_P(zsession)->client_session);
+	}
+
+	if (zwriteConcern) {
+		mongoc_bulkwriteopts_set_writeconcern(bw_opts, Z_WRITECONCERN_OBJ_P(zwriteConcern)->write_concern);
 	}
 
 	bw_ret = mongoc_bulkwrite_execute(bw, bw_opts);
diff --git a/tests/bulkwritecommand/bulkwritecommand-debug-001.phpt b/tests/bulkwritecommand/bulkwritecommand-debug-001.phpt
@@ -0,0 +1,39 @@
+--TEST--
+MongoDB\Driver\BulkWriteCommand debug output
+--FILE--
+<?php
+
+require_once __DIR__ . "/../utils/basic.inc";
+
+$bulk = new MongoDB\Driver\BulkWriteCommand([
+    'bypassDocumentValidation' => true,
+    'comment' => 'foo',
+    'let' => ['foo' => 1],
+    'ordered' => true,
+    'verboseResults' => true,
+]);
+
+var_dump($bulk);
+
+?>
+===DONE===
+<?php exit(0); ?>
+--EXPECTF--
+object(MongoDB\Driver\BulkWriteCommand)#%d (%d) {
+  ["bypassDocumentValidation"]=>
+  bool(true)
+  ["comment"]=>
+  string(3) "foo"
+  ["let"]=>
+  object(stdClass)#%d (%d) {
+    ["foo"]=>
+    int(1)
+  }
+  ["ordered"]=>
+  bool(true)
+  ["verboseResults"]=>
+  bool(true)
+  ["session"]=>
+  NULL
+}
+===DONE===
diff --git a/tests/bulkwritecommand/bulkwritecommand-debug-002.phpt b/tests/bulkwritecommand/bulkwritecommand-debug-002.phpt
@@ -0,0 +1,53 @@
+--TEST--
+MongoDB\Driver\BulkWriteCommand debug output after execution
+--SKIPIF--
+<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
+<?php skip_if_not_live(); ?>
+<?php skip_if_server_version('<', '8.0'); ?>
+<?php skip_if_not_clean(); ?>
+--FILE--
+<?php
+
+require_once __DIR__ . "/../utils/basic.inc";
+
+$manager = create_test_manager();
+
+$tests = [
+    [],
+    ['session' => $manager->startSession()],
+];
+
+foreach ($tests as $options) {
+    $bulk = new MongoDB\Driver\BulkWriteCommand();
+    $bulk->insertOne(NS, ['x' => 1]);
+    $manager->executeBulkWriteCommand($bulk, $options);
+    var_dump($bulk);
+}
+
+?>
+===DONE===
+<?php exit(0); ?>
+--EXPECTF--
+object(MongoDB\Driver\BulkWriteCommand)#%d (%d) {
+  ["bypassDocumentValidation"]=>
+  NULL
+  ["ordered"]=>
+  bool(true)
+  ["verboseResults"]=>
+  bool(false)
+  ["session"]=>
+  NULL
+}
+object(MongoDB\Driver\BulkWriteCommand)#%d (%d) {
+  ["bypassDocumentValidation"]=>
+  NULL
+  ["ordered"]=>
+  bool(true)
+  ["verboseResults"]=>
+  bool(false)
+  ["session"]=>
+  object(MongoDB\Driver\Session)#%d (%d) {
+    %a
+  }
+}
+===DONE===
diff --git a/tests/bulkwritecommand/manager-executeBulkWriteCommand-001.phpt b/tests/bulkwritecommand/manager-executeBulkWriteCommand-001.phpt
@@ -1,5 +1,5 @@
 --TEST--
-MongoDB\Driver\BulkWriteCommand::__construct()
+MongoDB\Driver\Manager::executeBulkWriteCommand()
 --SKIPIF--
 <?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
 <?php skip_if_not_live(); ?>
@@ -26,25 +26,12 @@ $bulk->deleteMany(NS, ['_id' => ['$gt' => 2]]);
 
 $result = $manager->executeBulkWriteCommand($bulk);
 
-var_dump($bulk);
 var_dump($result);
 
 ?>
 ===DONE===
 <?php exit(0); ?>
 --EXPECTF--
-object(MongoDB\Driver\BulkWriteCommand)#%d (%d) {
-  ["bypassDocumentValidation"]=>
-  NULL
-  ["ordered"]=>
-  bool(true)
-  ["verboseResults"]=>
-  bool(true)
-  ["session"]=>
-  NULL
-  ["write_concern"]=>
-  NULL
-}
 object(MongoDB\Driver\BulkWriteCommandResult)#%d (%d) {
   ["isAcknowledged"]=>
   bool(true)
diff --git a/tests/bulkwritecommand/manager-executeBulkWriteCommand_error-001.phpt b/tests/bulkwritecommand/manager-executeBulkWriteCommand_error-001.phpt
@@ -0,0 +1,31 @@
+--TEST--
+MongoDB\Driver\Manager::executeBulkWriteCommand() prohibits session and unacknowledged write concern
+--SKIPIF--
+<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
+<?php skip_if_not_live(); ?>
+<?php skip_if_server_version('<', '8.0'); ?>
+<?php skip_if_not_clean(); ?>
+--FILE--
+<?php
+
+require_once __DIR__ . "/../utils/basic.inc";
+
+$manager = create_test_manager();
+
+$bulk = new MongoDB\Driver\BulkWriteCommand();
+$bulk->insertOne(NS, ['_id' => 1]);
+
+echo throws(function() use ($manager, $bulk) {
+    $manager->executeBulkWriteCommand($bulk, [
+        'session' => $manager->startSession(),
+        'writeConcern' => new MongoDB\Driver\WriteConcern(0),
+    ]);
+}, MongoDB\Driver\Exception\InvalidArgumentException::class), "\n";
+
+?>
+===DONE===
+<?php exit(0); ?>
+--EXPECT--
+OK: Got MongoDB\Driver\Exception\InvalidArgumentException
+Cannot combine "session" option with an unacknowledged write concern
+===DONE===
diff --git a/tests/bulkwritecommand/manager-executeBulkWriteCommand_error-002.phpt b/tests/bulkwritecommand/manager-executeBulkWriteCommand_error-002.phpt
@@ -0,0 +1,30 @@
+--TEST--
+MongoDB\Driver\Manager::executeBulkWriteCommand() prohibits session and unacknowledged write concern (inherited)
+--SKIPIF--
+<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
+<?php skip_if_not_live(); ?>
+<?php skip_if_server_version('<', '8.0'); ?>
+<?php skip_if_not_clean(); ?>
+--FILE--
+<?php
+
+require_once __DIR__ . "/../utils/basic.inc";
+
+$manager = create_test_manager(URI, ['w' => 0]);
+
+$bulk = new MongoDB\Driver\BulkWriteCommand();
+$bulk->insertOne(NS, ['_id' => 1]);
+
+echo throws(function() use ($manager, $bulk) {
+    $manager->executeBulkWriteCommand($bulk, [
+        'session' => $manager->startSession(),
+    ]);
+}, MongoDB\Driver\Exception\InvalidArgumentException::class), "\n";
+
+?>
+===DONE===
+<?php exit(0); ?>
+--EXPECT--
+OK: Got MongoDB\Driver\Exception\InvalidArgumentException
+Cannot combine "session" option with an unacknowledged write concern
+===DONE===
diff --git a/tests/bulkwritecommand/server-executeBulkWriteCommand-001.phpt b/tests/bulkwritecommand/server-executeBulkWriteCommand-001.phpt
diff --git a/tests/bulkwritecommand/server-executeBulkWriteCommand_error-001.phpt b/tests/bulkwritecommand/server-executeBulkWriteCommand_error-001.phpt
diff --git a/tests/bulkwritecommand/server-executeBulkWriteCommand_error-002.phpt b/tests/bulkwritecommand/server-executeBulkWriteCommand_error-002.phpt
