PHPC-1382: Allow applications to set maxTimeMS for commitTransaction

alcaeus · alcaeus · commit 804f383aea60 · 2019-08-21T19:30:29.000+02:00
diff --git a/src/MongoDB/Session.c b/src/MongoDB/Session.c
@@ -247,21 +247,50 @@ static PHP_METHOD(Session, getOperationTime)
 } /* }}} */
 
 /* Creates a opts structure from an array optionally containing an RP, RC,
- * and/or WC object. Returns NULL if no options were found, or there was an
- * invalid option. If there was an invalid option or structure, an exception
- * will be thrown too. */
+ * WC object, and/or maxCommitTimeMS int. Returns NULL if no options were found,
+ * or there was an invalid option. If there was an invalid option or structure,
+ * an exception will be thrown too. */
 mongoc_transaction_opt_t* php_mongodb_session_parse_transaction_options(zval* options TSRMLS_DC)
 {
 	mongoc_transaction_opt_t* opts = NULL;
 
+	if (php_array_existsc(options, "maxCommitTimeMS")) {
+		int64_t max_commit_time_ms = php_array_fetchc_long(options, "maxCommitTimeMS");
+
+		if (max_commit_time_ms < 0) {
+			phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Expected \"maxCommitTimeMS\" option to be >= 0, %" PRId64 " given", max_commit_time_ms);
+			/* Freeing opts is not needed here, as it can't be set yet. The
+			 * code is here to keep it consistent with the others in case more
+			 * options are added before this one. */
+			if (opts) {
+				mongoc_transaction_opts_destroy(opts);
+			}
+			return NULL;
+		}
+
+		if (max_commit_time_ms > UINT32_MAX) {
+			phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Expected \"maxCommitTimeMS\" option to be <= %" PRIu32 ", %" PRId64 " given", UINT32_MAX, max_commit_time_ms);
+			/* Freeing opts is not needed here, as it can't be set yet. The
+			 * code is here to keep it consistent with the others in case more
+			 * options are added before this one. */
+			if (opts) {
+				mongoc_transaction_opts_destroy(opts);
+			}
+			return NULL;
+		}
+
+		if (!opts) {
+			opts = mongoc_transaction_opts_new();
+		}
+
+		mongoc_transaction_opts_set_max_commit_time_ms(opts, max_commit_time_ms);
+	}
+
 	if (php_array_existsc(options, "readConcern")) {
 		zval* read_concern = php_array_fetchc(options, "readConcern");
 
 		if (Z_TYPE_P(read_concern) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(read_concern), php_phongo_readconcern_ce TSRMLS_CC)) {
 			phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Expected \"readConcern\" option to be %s, %s given", ZSTR_VAL(php_phongo_readconcern_ce->name), PHONGO_ZVAL_CLASS_OR_TYPE_NAME_P(read_concern));
-			/* Freeing opts is not needed here, as it can't be set yet. The
-			 * code is here to keep it consistent with the others in case more
-			 * options are added before this one. */
 			if (opts) {
 				mongoc_transaction_opts_destroy(opts);
 			}
diff --git a/tests/session/session-startTransaction_error-002.phpt b/tests/session/session-startTransaction_error-002.phpt
@@ -13,13 +13,14 @@ $manager = new MongoDB\Driver\Manager(URI);
 $session = $manager->startSession();
 
 $options = [
-    [ 'readConcern' => 42 ], 
+    [ 'maxCommitTimeMS' => -1 ],
+    [ 'readConcern' => 42 ],
     [ 'readConcern' => new stdClass ],
     [ 'readConcern' => new \MongoDB\Driver\WriteConcern( 2 ) ],
-    [ 'readPreference' => 42 ], 
+    [ 'readPreference' => 42 ],
     [ 'readPreference' => new stdClass ],
     [ 'readPreference' => new \MongoDB\Driver\ReadConcern( \MongoDB\Driver\ReadConcern::LOCAL ) ],
-    [ 'writeConcern' => 42 ], 
+    [ 'writeConcern' => 42 ],
     [ 'writeConcern' => new stdClass ],
     [ 'writeConcern' => new \MongoDB\Driver\ReadPreference( \MongoDB\Driver\ReadPreference::RP_SECONDARY ) ],
 
@@ -43,11 +44,17 @@ foreach ($options as $txnOptions) {
     }, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
 }
 
+echo raises(function() use ($session) {
+    $session->startTransaction([ 'maxCommitTimeMS' => new stdClass ]);
+}, E_NOTICE), "\n";
+
 ?>
 ===DONE===
 <?php exit(0); ?>
 --EXPECTF--
 OK: Got MongoDB\Driver\Exception\InvalidArgumentException
+Expected "maxCommitTimeMS" option to be >= 0, -1 given
+OK: Got MongoDB\Driver\Exception\InvalidArgumentException
 Expected "readConcern" option to be MongoDB\Driver\ReadConcern, int%S given
 OK: Got MongoDB\Driver\Exception\InvalidArgumentException
 Expected "readConcern" option to be MongoDB\Driver\ReadConcern, stdClass given
@@ -71,4 +78,6 @@ OK: Got MongoDB\Driver\Exception\InvalidArgumentException
 Expected "writeConcern" option to be MongoDB\Driver\WriteConcern, MongoDB\Driver\ReadPreference given
 OK: Got MongoDB\Driver\Exception\InvalidArgumentException
 Expected "writeConcern" option to be MongoDB\Driver\WriteConcern, MongoDB\Driver\ReadPreference given
+OK: Got E_NOTICE
+Object of class stdClass could not be converted to int
 ===DONE===
diff --git a/tests/session/session_error-001.phpt b/tests/session/session_error-001.phpt
@@ -7,13 +7,14 @@ require_once __DIR__ . "/../utils/basic.inc";
 $manager = new MongoDB\Driver\Manager();
 
 $options = [
-    [ 'readConcern' => 42 ], 
+    [ 'maxCommitTimeMS' => -1 ],
+    [ 'readConcern' => 42 ],
     [ 'readConcern' => new stdClass ],
     [ 'readConcern' => new \MongoDB\Driver\WriteConcern( 2 ) ],
-    [ 'readPreference' => 42 ], 
+    [ 'readPreference' => 42 ],
     [ 'readPreference' => new stdClass ],
     [ 'readPreference' => new \MongoDB\Driver\ReadConcern( \MongoDB\Driver\ReadConcern::LOCAL ) ],
-    [ 'writeConcern' => 42 ], 
+    [ 'writeConcern' => 42 ],
     [ 'writeConcern' => new stdClass ],
     [ 'writeConcern' => new \MongoDB\Driver\ReadPreference( \MongoDB\Driver\ReadPreference::RP_SECONDARY ) ],
 
@@ -36,17 +37,25 @@ $options = [
 
 foreach ($options as $txnOptions) {
     echo throws(function() use ($manager, $txnOptions) {
-        $session = $manager->startSession([
+        $manager->startSession([
             'defaultTransactionOptions' => $txnOptions
         ]);
     }, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
 }
 
+echo raises(function() use ($manager) {
+    $manager->startSession([
+        'defaultTransactionOptions' => [ 'maxCommitTimeMS' => new stdClass ]
+    ]);
+}, E_NOTICE), "\n";
+
 ?>
 ===DONE===
 <?php exit(0); ?>
 --EXPECTF--
 OK: Got MongoDB\Driver\Exception\InvalidArgumentException
+Expected "maxCommitTimeMS" option to be >= 0, -1 given
+OK: Got MongoDB\Driver\Exception\InvalidArgumentException
 Expected "readConcern" option to be MongoDB\Driver\ReadConcern, int%S given
 OK: Got MongoDB\Driver\Exception\InvalidArgumentException
 Expected "readConcern" option to be MongoDB\Driver\ReadConcern, stdClass given
@@ -74,4 +83,6 @@ OK: Got MongoDB\Driver\Exception\InvalidArgumentException
 Expected "defaultTransactionOptions" option to be an array, int%S given
 OK: Got MongoDB\Driver\Exception\InvalidArgumentException
 Expected "defaultTransactionOptions" option to be an array, stdClass given
+OK: Got E_NOTICE
+Object of class stdClass could not be converted to int
 ===DONE===
diff --git a/tests/utils/tools.php b/tests/utils/tools.php
@@ -518,6 +518,8 @@ function severityToString($type) {
     switch($type) {
     case E_WARNING:
         return "E_WARNING";
+    case E_NOTICE:
+        return "E_NOTICE";
     default:
         return "Some other #_$type";
     }
@@ -531,6 +533,8 @@ function raises($function, $type, $infunction = null) {
     try {
         $function();
     } catch(Exception $e) {
+        $exceptionname = get_class($e);
+
         if ($e instanceof ErrorException && $e->getSeverity() & $type) {
             if ($infunction) {
                 $trace = $e->getTrace();
@@ -551,7 +555,7 @@ function raises($function, $type, $infunction = null) {
         return $e->getMessage();
     }
 
-    echo "FAILED: Expected $exceptionname thrown!\n";
+    printf("FAILED: Expected %s thrown!\n", ErrorException::class);
     restore_error_handler();
 }
 function throws($function, $exceptionname, $infunction = null) {
