-
Notifications
You must be signed in to change notification settings - Fork 208
PHPC-1761: Snapshot Reads #1243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ad20ac5
e1c4fe5
aceae37
6b4eb7c
962a9d2
226b3a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
1.18.0 | ||
1.11.1-20210804+git230369cd07 | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -805,6 +805,18 @@ static PHP_METHOD(Manager, startSession) | |
} | ||
} | ||
|
||
if (options && php_array_existsc(options, "snapshot")) { | ||
if (!cs_opts) { | ||
cs_opts = mongoc_session_opts_new(); | ||
} | ||
mongoc_session_opts_set_snapshot(cs_opts, php_array_fetchc_bool(options, "snapshot")); | ||
} | ||
|
||
if (cs_opts && mongoc_session_opts_get_causal_consistency(cs_opts) && mongoc_session_opts_get_snapshot(cs_opts)) { | ||
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Only one of \"causalConsistency\" and \"snapshot\" can be enabled"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. libmongoc also raises this error through mongoc_client_start_session; however, the code/domain used for that would lead to a PHPC throwing a RuntimeException. I handled this manually so we can ensure an InvalidArgumentException is thrown, which makes more sense since this is just option validation. |
||
goto cleanup; | ||
} | ||
|
||
/* If the Manager was created in a different process, reset the client so | ||
* that its session pool is cleared. This will ensure that we do not re-use | ||
* a server session (i.e. LSID) created by a parent process. */ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -740,8 +740,10 @@ static HashTable* php_phongo_session_get_debug_info(phongo_compat_object_handler | |
if (intern->client_session) { | ||
const mongoc_session_opt_t* cs_opts = mongoc_client_session_get_opts(intern->client_session); | ||
ADD_ASSOC_BOOL_EX(&retval, "causalConsistency", mongoc_session_opts_get_causal_consistency(cs_opts)); | ||
ADD_ASSOC_BOOL_EX(&retval, "snapshot", mongoc_session_opts_get_snapshot(cs_opts)); | ||
} else { | ||
ADD_ASSOC_NULL_EX(&retval, "causalConsistency"); | ||
ADD_ASSOC_NULL_EX(&retval, "snapshot"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: Session.c doesn't actually have getters for the options passed to I forget if we considered adding an API for the |
||
} | ||
|
||
if (intern->client_session) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,10 @@ | ||
--TEST-- | ||
MongoDB\Driver\Session with wrong defaultTransactionOptions | ||
MongoDB\Driver\Manager::startSession() with wrong defaultTransactionOptions | ||
--SKIPIF-- | ||
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?> | ||
<?php skip_if_not_libmongoc_crypto(); ?> | ||
<?php skip_if_not_live(); ?> | ||
<?php skip_if_server_version('<', '3.6'); ?> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous tests were missing a SKIPIF entirely. |
||
--FILE-- | ||
<?php | ||
require_once __DIR__ . "/../utils/basic.inc"; | ||
|
@@ -40,7 +45,7 @@ foreach ($options as $txnOptions) { | |
$manager->startSession([ | ||
'defaultTransactionOptions' => $txnOptions | ||
]); | ||
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n"; | ||
}, MongoDB\Driver\Exception\InvalidArgumentException::class), "\n"; | ||
} | ||
|
||
echo raises(function() use ($manager) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
--TEST-- | ||
MongoDB\Driver\Manager::startSession() snapshot and causalConsistency cannot both be true | ||
--DESCRIPTION-- | ||
Session spec prose test #1 | ||
https://github.com/mongodb/specifications/blob/master/source/sessions/tests/README.rst#prose-tests | ||
alcaeus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
--SKIPIF-- | ||
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?> | ||
<?php skip_if_not_libmongoc_crypto(); ?> | ||
<?php skip_if_not_live(); ?> | ||
<?php skip_if_server_version('<', '3.6'); ?> | ||
--FILE-- | ||
<?php | ||
require_once __DIR__ . "/../utils/basic.inc"; | ||
|
||
$manager = create_test_manager(); | ||
|
||
echo throws(function() use ($manager) { | ||
$manager->startSession([ | ||
'causalConsistency' => true, | ||
'snapshot' => true, | ||
]); | ||
}, MongoDB\Driver\Exception\InvalidArgumentException::class), "\n"; | ||
|
||
?> | ||
===DONE=== | ||
<?php exit(0); ?> | ||
--EXPECT-- | ||
OK: Got MongoDB\Driver\Exception\InvalidArgumentException | ||
Only one of "causalConsistency" and "snapshot" can be enabled | ||
===DONE=== |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
--TEST-- | ||
MongoDB\Driver\Session spec test: snapshot option is incompatible with writes | ||
--DESCRIPTION-- | ||
PHPC-1875: Disable writes on snapshot sessions | ||
--SKIPIF-- | ||
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?> | ||
<?php skip_if_not_libmongoc_crypto(); ?> | ||
<?php skip_if_not_live(); ?> | ||
<?php skip_if_server_version('<', '5.0'); ?> | ||
--FILE-- | ||
<?php | ||
require_once __DIR__ . "/../utils/basic.inc"; | ||
|
||
$manager = create_test_manager(); | ||
$session = $manager->startSession(['snapshot' => true]); | ||
|
||
$bulk = new MongoDB\Driver\BulkWrite(); | ||
$bulk->insert(['x' => 1]); | ||
|
||
try { | ||
$manager->executeBulkWrite(NS, $bulk, ['session' => $session]); | ||
} catch (MongoDB\Driver\Exception\BulkWriteException $e) { | ||
/* Note: we intentionally do not assert the server's error message for the | ||
* client specifying a read concern on a write command. It is sufficient to | ||
* assert that the error code is InvalidOptions(72). */ | ||
var_dump($e->getCode() === 72); | ||
} | ||
|
||
?> | ||
===DONE=== | ||
<?php exit(0); ?> | ||
--EXPECT-- | ||
bool(true) | ||
===DONE=== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disregard "1.11.1" here, as it's related to an outstanding issue with
calc_release_version.py
(CDRIVER-3315).