Skip to content

Commit

Permalink
PHPLIB-712: Allow hint for unacknowledged writes using OP_MSG when su…
Browse files Browse the repository at this point in the history
…pported by the server (#859)

* Sync unified CRUD spec tests

Synced with mongodb/specifications@59a5dad

Note: aggregate tests are intentionally excluded since those will be synced by #854

* PHPLIB-711: Unwrap BulkWriteException when evaluating isClientError

* Client-side errors for hint with update, delete, and findAndModify

The CRUD spec requires a client-side error if hint is used and the server would not otherwise raise an error for an unsupported option. This was already being done, but this commit renames an internal constant and adds functional tests.

A client-side error is also required if hint is used with an unacknowledged write concern and the server does not support hint for the operation (regardless of error reporting for unsupported options). This was previously done only for FindAndModify; however, it failed to detect some acknowledged write concerns (PHPLIB-712). That issue has been fixed and the commit additionally adds this behavior for update and delete operations.
  • Loading branch information
jmikola committed Aug 31, 2021
1 parent 57cd7e0 commit e787475
Show file tree
Hide file tree
Showing 33 changed files with 3,713 additions and 1,682 deletions.
22 changes: 17 additions & 5 deletions src/Operation/Delete.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use function is_array;
use function is_object;
use function is_string;
use function MongoDB\is_write_concern_acknowledged;
use function MongoDB\server_supports_feature;

/**
Expand All @@ -45,8 +46,11 @@ class Delete implements Executable, Explainable
/** @var integer */
private static $wireVersionForCollation = 5;

/** @var integer */
private static $wireVersionForHint = 9;

/** @var int */
private static $wireVersionForHintServerSideError = 5;
private static $wireVersionForUnsupportedOptionServerSideError = 5;

/** @var string */
private $databaseName;
Expand Down Expand Up @@ -146,10 +150,18 @@ public function execute(Server $server)
throw UnsupportedException::collationNotSupported();
}

/* Server versions >= 3.4.0 raise errors for unknown update
* options. For previous versions, the CRUD spec requires a client-side
* error. */
if (isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForHintServerSideError)) {
/* Server versions >= 3.4.0 raise errors for unsupported update options.
* For previous versions, the CRUD spec requires a client-side error. */
if (isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForUnsupportedOptionServerSideError)) {
throw UnsupportedException::hintNotSupported();
}

/* CRUD spec requires a client-side error when using "hint" with an
* unacknowledged write concern on an unsupported server. */
if (
isset($this->options['writeConcern']) && ! is_write_concern_acknowledged($this->options['writeConcern']) &&
isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForHint)
) {
throw UnsupportedException::hintNotSupported();
}

Expand Down
36 changes: 14 additions & 22 deletions src/Operation/FindAndModify.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
use function is_string;
use function MongoDB\create_field_path_type_map;
use function MongoDB\is_pipeline;
use function MongoDB\is_write_concern_acknowledged;
use function MongoDB\server_supports_feature;

/**
Expand All @@ -60,7 +61,7 @@ class FindAndModify implements Executable, Explainable
private static $wireVersionForHint = 9;

/** @var integer */
private static $wireVersionForHintServerSideError = 8;
private static $wireVersionForUnsupportedOptionServerSideError = 8;

/** @var integer */
private static $wireVersionForWriteConcern = 4;
Expand Down Expand Up @@ -245,11 +246,18 @@ public function execute(Server $server)
throw UnsupportedException::collationNotSupported();
}

/* Server versions >= 4.1.10 raise errors for unknown findAndModify
* options (SERVER-40005), but the CRUD spec requires client-side errors
* for server versions < 4.2. For later versions, we'll rely on the
* server to either utilize the option or report its own error. */
if (isset($this->options['hint']) && ! $this->isHintSupported($server)) {
/* Server versions >= 4.2.0 raise errors for unsupported update options.
* For previous versions, the CRUD spec requires a client-side error. */
if (isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForUnsupportedOptionServerSideError)) {
throw UnsupportedException::hintNotSupported();
}

/* CRUD spec requires a client-side error when using "hint" with an
* unacknowledged write concern on an unsupported server. */
if (
isset($this->options['writeConcern']) && ! is_write_concern_acknowledged($this->options['writeConcern']) &&
isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForHint)
) {
throw UnsupportedException::hintNotSupported();
}

Expand Down Expand Up @@ -350,20 +358,4 @@ private function createOptions()

return $options;
}

private function isAcknowledgedWriteConcern(): bool
{
if (! isset($this->options['writeConcern'])) {
return true;
}

return $this->options['writeConcern']->getW() > 1 || $this->options['writeConcern']->getJournal();
}

private function isHintSupported(Server $server): bool
{
$requiredWireVersion = $this->isAcknowledgedWriteConcern() ? self::$wireVersionForHintServerSideError : self::$wireVersionForHint;

return server_supports_feature($server, $requiredWireVersion);
}
}
22 changes: 17 additions & 5 deletions src/Operation/Update.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
use function is_string;
use function MongoDB\is_first_key_operator;
use function MongoDB\is_pipeline;
use function MongoDB\is_write_concern_acknowledged;
use function MongoDB\server_supports_feature;

/**
Expand All @@ -55,7 +56,10 @@ class Update implements Executable, Explainable
private static $wireVersionForDocumentLevelValidation = 4;

/** @var integer */
private static $wireVersionForHintServerSideError = 5;
private static $wireVersionForHint = 8;

/** @var integer */
private static $wireVersionForUnsupportedOptionServerSideError = 5;

/** @var string */
private $databaseName;
Expand Down Expand Up @@ -203,10 +207,18 @@ public function execute(Server $server)
throw UnsupportedException::collationNotSupported();
}

/* Server versions >= 3.4.0 raise errors for unknown update
* options. For previous versions, the CRUD spec requires a client-side
* error. */
if (isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForHintServerSideError)) {
/* Server versions >= 3.4.0 raise errors for unsupported update options.
* For previous versions, the CRUD spec requires a client-side error. */
if (isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForUnsupportedOptionServerSideError)) {
throw UnsupportedException::hintNotSupported();
}

/* CRUD spec requires a client-side error when using "hint" with an
* unacknowledged write concern on an unsupported server. */
if (
isset($this->options['writeConcern']) && ! is_write_concern_acknowledged($this->options['writeConcern']) &&
isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForHint)
) {
throw UnsupportedException::hintNotSupported();
}

Expand Down
20 changes: 20 additions & 0 deletions src/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use MongoDB\Driver\ReadPreference;
use MongoDB\Driver\Server;
use MongoDB\Driver\Session;
use MongoDB\Driver\WriteConcern;
use MongoDB\Exception\InvalidArgumentException;
use MongoDB\Exception\RuntimeException;
use MongoDB\Operation\WithTransaction;
Expand Down Expand Up @@ -239,6 +240,25 @@ function is_mapreduce_output_inline($out): bool
return key($out) === 'inline';
}

/**
* Return whether the write concern is acknowledged.
*
* This function is similar to mongoc_write_concern_is_acknowledged but does not
* check the fsync option since that was never supported in the PHP driver.
*
* @internal
* @see https://docs.mongodb.com/manual/reference/write-concern/
* @param WriteConcern $writeConcern
* @return boolean
*/
function is_write_concern_acknowledged(WriteConcern $writeConcern): bool
{
/* Note: -1 corresponds to MONGOC_WRITE_CONCERN_W_ERRORS_IGNORED, which is
* deprecated synonym of MONGOC_WRITE_CONCERN_W_UNACKNOWLEDGED and slated
* for removal in libmongoc 2.0. */
return ($writeConcern->getW() !== 0 && $writeConcern->getW() !== -1) || $writeConcern->getJournal() === true;
}

/**
* Return whether the server supports a particular feature.
*
Expand Down
29 changes: 29 additions & 0 deletions tests/FunctionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace MongoDB\Tests;

use MongoDB\Driver\WriteConcern;
use MongoDB\Exception\InvalidArgumentException;
use MongoDB\Model\BSONArray;
use MongoDB\Model\BSONDocument;
Expand All @@ -12,6 +13,7 @@
use function MongoDB\is_first_key_operator;
use function MongoDB\is_mapreduce_output_inline;
use function MongoDB\is_pipeline;
use function MongoDB\is_write_concern_acknowledged;

/**
* Unit tests for utility functions.
Expand Down Expand Up @@ -255,4 +257,31 @@ public function providePipelines()
'DbRef in numeric field as object' => [false, (object) ['0' => ['$ref' => 'foo', '$id' => 'bar']]],
];
}

/**
* @dataProvider provideWriteConcerns
*/
public function testIsWriteConcernAcknowledged($expected, WriteConcern $writeConcern): void
{
$this->assertSame($expected, is_write_concern_acknowledged($writeConcern));
}

public function provideWriteConcerns(): array
{
// Note: WriteConcern constructor prohibits w=-1 or w=0 and journal=true
return [
'MONGOC_WRITE_CONCERN_W_MAJORITY' => [true, new WriteConcern(-3)],
'MONGOC_WRITE_CONCERN_W_DEFAULT' => [true, new WriteConcern(-2)],
'MONGOC_WRITE_CONCERN_W_DEFAULT and journal=true' => [true, new WriteConcern(-2, 0, true)],
'MONGOC_WRITE_CONCERN_W_ERRORS_IGNORED' => [false, new WriteConcern(-1)],
'MONGOC_WRITE_CONCERN_W_ERRORS_IGNORED and journal=false' => [false, new WriteConcern(-1, 0, false)],
'MONGOC_WRITE_CONCERN_W_UNACKNOWLEDGED' => [false, new WriteConcern(0)],
'MONGOC_WRITE_CONCERN_W_UNACKNOWLEDGED and journal=false' => [false, new WriteConcern(0, 0, false)],
'w=1' => [true, new WriteConcern(1)],
'w=1 and journal=false' => [true, new WriteConcern(1, 0, false)],
'w=1 and journal=true' => [true, new WriteConcern(1, 0, true)],
'majority' => [true, new WriteConcern(WriteConcern::MAJORITY)],
'tag' => [true, new WriteConcern('tag')],
];
}
}
41 changes: 41 additions & 0 deletions tests/Operation/DeleteFunctionalTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use MongoDB\Driver\BulkWrite;
use MongoDB\Driver\WriteConcern;
use MongoDB\Exception\BadMethodCallException;
use MongoDB\Exception\UnsupportedException;
use MongoDB\Operation\Delete;
use MongoDB\Tests\CommandObserver;

Expand Down Expand Up @@ -63,6 +64,46 @@ public function testDeleteMany(): void
$this->assertSameDocuments($expected, $this->collection->find());
}

public function testHintOptionUnsupportedClientSideError(): void
{
if (version_compare($this->getServerVersion(), '3.4.0', '>=')) {
$this->markTestSkipped('server reports error for unsupported delete options');
}

$operation = new Delete(
$this->getDatabaseName(),
$this->getCollectionName(),
[],
0,
['hint' => '_id_']
);

$this->expectException(UnsupportedException::class);
$this->expectExceptionMessage('Hint is not supported by the server executing this operation');

$operation->execute($this->getPrimaryServer());
}

public function testHintOptionAndUnacknowledgedWriteConcernUnsupportedClientSideError(): void
{
if (version_compare($this->getServerVersion(), '4.4.0', '>=')) {
$this->markTestSkipped('hint is supported');
}

$operation = new Delete(
$this->getDatabaseName(),
$this->getCollectionName(),
[],
0,
['hint' => '_id_', 'writeConcern' => new WriteConcern(0)]
);

$this->expectException(UnsupportedException::class);
$this->expectExceptionMessage('Hint is not supported by the server executing this operation');

$operation->execute($this->getPrimaryServer());
}

public function testSessionOption(): void
{
if (version_compare($this->getServerVersion(), '3.6.0', '<')) {
Expand Down
38 changes: 38 additions & 0 deletions tests/Operation/FindAndModifyFunctionalTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

use MongoDB\Driver\BulkWrite;
use MongoDB\Driver\ReadPreference;
use MongoDB\Driver\WriteConcern;
use MongoDB\Exception\UnsupportedException;
use MongoDB\Model\BSONDocument;
use MongoDB\Operation\FindAndModify;
use MongoDB\Tests\CommandObserver;
Expand Down Expand Up @@ -54,6 +56,42 @@ function (array $event): void {
);
}

public function testHintOptionUnsupportedClientSideError(): void
{
if (version_compare($this->getServerVersion(), '4.2.0', '>=')) {
$this->markTestSkipped('server reports error for unsupported findAndModify options');
}

$operation = new FindAndModify(
$this->getDatabaseName(),
$this->getCollectionName(),
['remove' => true, 'hint' => '_id_']
);

$this->expectException(UnsupportedException::class);
$this->expectExceptionMessage('Hint is not supported by the server executing this operation');

$operation->execute($this->getPrimaryServer());
}

public function testHintOptionAndUnacknowledgedWriteConcernUnsupportedClientSideError(): void
{
if (version_compare($this->getServerVersion(), '4.4.0', '>=')) {
$this->markTestSkipped('hint is supported');
}

$operation = new FindAndModify(
$this->getDatabaseName(),
$this->getCollectionName(),
['remove' => true, 'hint' => '_id_', 'writeConcern' => new WriteConcern(0)]
);

$this->expectException(UnsupportedException::class);
$this->expectExceptionMessage('Hint is not supported by the server executing this operation');

$operation->execute($this->getPrimaryServer());
}

public function testSessionOption(): void
{
if (version_compare($this->getServerVersion(), '3.6.0', '<')) {
Expand Down
41 changes: 41 additions & 0 deletions tests/Operation/UpdateFunctionalTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use MongoDB\Driver\BulkWrite;
use MongoDB\Driver\WriteConcern;
use MongoDB\Exception\BadMethodCallException;
use MongoDB\Exception\UnsupportedException;
use MongoDB\Operation\Update;
use MongoDB\Tests\CommandObserver;
use MongoDB\UpdateResult;
Expand Down Expand Up @@ -98,6 +99,46 @@ function (array $event): void {
);
}

public function testHintOptionUnsupportedClientSideError(): void
{
if (version_compare($this->getServerVersion(), '3.4.0', '>=')) {
$this->markTestSkipped('server reports error for unsupported update options');
}

$operation = new Update(
$this->getDatabaseName(),
$this->getCollectionName(),
['_id' => 1],
['$inc' => ['x' => 1]],
['hint' => '_id_']
);

$this->expectException(UnsupportedException::class);
$this->expectExceptionMessage('Hint is not supported by the server executing this operation');

$operation->execute($this->getPrimaryServer());
}

public function testHintOptionAndUnacknowledgedWriteConcernUnsupportedClientSideError(): void
{
if (version_compare($this->getServerVersion(), '4.2.0', '>=')) {
$this->markTestSkipped('hint is supported');
}

$operation = new Update(
$this->getDatabaseName(),
$this->getCollectionName(),
['_id' => 1],
['$inc' => ['x' => 1]],
['hint' => '_id_', 'writeConcern' => new WriteConcern(0)]
);

$this->expectException(UnsupportedException::class);
$this->expectExceptionMessage('Hint is not supported by the server executing this operation');

$operation->execute($this->getPrimaryServer());
}

public function testUpdateOne(): void
{
$this->createFixtures(3);
Expand Down

0 comments on commit e787475

Please sign in to comment.