Skip to content

Commit e787475

Browse files
jmikolajmikola
authored
PHPLIB-712: Allow hint for unacknowledged writes using OP_MSG when supported 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.
1 parent 57cd7e0 commit e787475

33 files changed

+3713
-1682
lines changed

‎src/Operation/Delete.php

+17-5
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use function is_array;
3030
use function is_object;
3131
use function is_string;
32+
use function MongoDB\is_write_concern_acknowledged;
3233
use function MongoDB\server_supports_feature;
3334

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

49+
/** @var integer */
50+
private static $wireVersionForHint = 9;
51+
4852
/** @var int */
49-
private static $wireVersionForHintServerSideError = 5;
53+
private static $wireVersionForUnsupportedOptionServerSideError = 5;
5054

5155
/** @var string */
5256
private $databaseName;
@@ -146,10 +150,18 @@ public function execute(Server $server)
146150
throw UnsupportedException::collationNotSupported();
147151
}
148152

149-
/* Server versions >= 3.4.0 raise errors for unknown update
150-
* options. For previous versions, the CRUD spec requires a client-side
151-
* error. */
152-
if (isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForHintServerSideError)) {
153+
/* Server versions >= 3.4.0 raise errors for unsupported update options.
154+
* For previous versions, the CRUD spec requires a client-side error. */
155+
if (isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForUnsupportedOptionServerSideError)) {
156+
throw UnsupportedException::hintNotSupported();
157+
}
158+
159+
/* CRUD spec requires a client-side error when using "hint" with an
160+
* unacknowledged write concern on an unsupported server. */
161+
if (
162+
isset($this->options['writeConcern']) && ! is_write_concern_acknowledged($this->options['writeConcern']) &&
163+
isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForHint)
164+
) {
153165
throw UnsupportedException::hintNotSupported();
154166
}
155167

‎src/Operation/FindAndModify.php

+14-22
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
use function is_string;
3535
use function MongoDB\create_field_path_type_map;
3636
use function MongoDB\is_pipeline;
37+
use function MongoDB\is_write_concern_acknowledged;
3738
use function MongoDB\server_supports_feature;
3839

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

6263
/** @var integer */
63-
private static $wireVersionForHintServerSideError = 8;
64+
private static $wireVersionForUnsupportedOptionServerSideError = 8;
6465

6566
/** @var integer */
6667
private static $wireVersionForWriteConcern = 4;
@@ -245,11 +246,18 @@ public function execute(Server $server)
245246
throw UnsupportedException::collationNotSupported();
246247
}
247248

248-
/* Server versions >= 4.1.10 raise errors for unknown findAndModify
249-
* options (SERVER-40005), but the CRUD spec requires client-side errors
250-
* for server versions < 4.2. For later versions, we'll rely on the
251-
* server to either utilize the option or report its own error. */
252-
if (isset($this->options['hint']) && ! $this->isHintSupported($server)) {
249+
/* Server versions >= 4.2.0 raise errors for unsupported update options.
250+
* For previous versions, the CRUD spec requires a client-side error. */
251+
if (isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForUnsupportedOptionServerSideError)) {
252+
throw UnsupportedException::hintNotSupported();
253+
}
254+
255+
/* CRUD spec requires a client-side error when using "hint" with an
256+
* unacknowledged write concern on an unsupported server. */
257+
if (
258+
isset($this->options['writeConcern']) && ! is_write_concern_acknowledged($this->options['writeConcern']) &&
259+
isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForHint)
260+
) {
253261
throw UnsupportedException::hintNotSupported();
254262
}
255263

@@ -350,20 +358,4 @@ private function createOptions()
350358

351359
return $options;
352360
}
353-
354-
private function isAcknowledgedWriteConcern(): bool
355-
{
356-
if (! isset($this->options['writeConcern'])) {
357-
return true;
358-
}
359-
360-
return $this->options['writeConcern']->getW() > 1 || $this->options['writeConcern']->getJournal();
361-
}
362-
363-
private function isHintSupported(Server $server): bool
364-
{
365-
$requiredWireVersion = $this->isAcknowledgedWriteConcern() ? self::$wireVersionForHintServerSideError : self::$wireVersionForHint;
366-
367-
return server_supports_feature($server, $requiredWireVersion);
368-
}
369361
}

‎src/Operation/Update.php

+17-5
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
use function is_string;
3333
use function MongoDB\is_first_key_operator;
3434
use function MongoDB\is_pipeline;
35+
use function MongoDB\is_write_concern_acknowledged;
3536
use function MongoDB\server_supports_feature;
3637

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

5758
/** @var integer */
58-
private static $wireVersionForHintServerSideError = 5;
59+
private static $wireVersionForHint = 8;
60+
61+
/** @var integer */
62+
private static $wireVersionForUnsupportedOptionServerSideError = 5;
5963

6064
/** @var string */
6165
private $databaseName;
@@ -203,10 +207,18 @@ public function execute(Server $server)
203207
throw UnsupportedException::collationNotSupported();
204208
}
205209

206-
/* Server versions >= 3.4.0 raise errors for unknown update
207-
* options. For previous versions, the CRUD spec requires a client-side
208-
* error. */
209-
if (isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForHintServerSideError)) {
210+
/* Server versions >= 3.4.0 raise errors for unsupported update options.
211+
* For previous versions, the CRUD spec requires a client-side error. */
212+
if (isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForUnsupportedOptionServerSideError)) {
213+
throw UnsupportedException::hintNotSupported();
214+
}
215+
216+
/* CRUD spec requires a client-side error when using "hint" with an
217+
* unacknowledged write concern on an unsupported server. */
218+
if (
219+
isset($this->options['writeConcern']) && ! is_write_concern_acknowledged($this->options['writeConcern']) &&
220+
isset($this->options['hint']) && ! server_supports_feature($server, self::$wireVersionForHint)
221+
) {
210222
throw UnsupportedException::hintNotSupported();
211223
}
212224

‎src/functions.php

+20
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use MongoDB\Driver\ReadPreference;
2424
use MongoDB\Driver\Server;
2525
use MongoDB\Driver\Session;
26+
use MongoDB\Driver\WriteConcern;
2627
use MongoDB\Exception\InvalidArgumentException;
2728
use MongoDB\Exception\RuntimeException;
2829
use MongoDB\Operation\WithTransaction;
@@ -239,6 +240,25 @@ function is_mapreduce_output_inline($out): bool
239240
return key($out) === 'inline';
240241
}
241242

243+
/**
244+
* Return whether the write concern is acknowledged.
245+
*
246+
* This function is similar to mongoc_write_concern_is_acknowledged but does not
247+
* check the fsync option since that was never supported in the PHP driver.
248+
*
249+
* @internal
250+
* @see https://docs.mongodb.com/manual/reference/write-concern/
251+
* @param WriteConcern $writeConcern
252+
* @return boolean
253+
*/
254+
function is_write_concern_acknowledged(WriteConcern $writeConcern): bool
255+
{
256+
/* Note: -1 corresponds to MONGOC_WRITE_CONCERN_W_ERRORS_IGNORED, which is
257+
* deprecated synonym of MONGOC_WRITE_CONCERN_W_UNACKNOWLEDGED and slated
258+
* for removal in libmongoc 2.0. */
259+
return ($writeConcern->getW() !== 0 && $writeConcern->getW() !== -1) || $writeConcern->getJournal() === true;
260+
}
261+
242262
/**
243263
* Return whether the server supports a particular feature.
244264
*

‎tests/FunctionsTest.php

+29
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace MongoDB\Tests;
44

5+
use MongoDB\Driver\WriteConcern;
56
use MongoDB\Exception\InvalidArgumentException;
67
use MongoDB\Model\BSONArray;
78
use MongoDB\Model\BSONDocument;
@@ -12,6 +13,7 @@
1213
use function MongoDB\is_first_key_operator;
1314
use function MongoDB\is_mapreduce_output_inline;
1415
use function MongoDB\is_pipeline;
16+
use function MongoDB\is_write_concern_acknowledged;
1517

1618
/**
1719
* Unit tests for utility functions.
@@ -255,4 +257,31 @@ public function providePipelines()
255257
'DbRef in numeric field as object' => [false, (object) ['0' => ['$ref' => 'foo', '$id' => 'bar']]],
256258
];
257259
}
260+
261+
/**
262+
* @dataProvider provideWriteConcerns
263+
*/
264+
public function testIsWriteConcernAcknowledged($expected, WriteConcern $writeConcern): void
265+
{
266+
$this->assertSame($expected, is_write_concern_acknowledged($writeConcern));
267+
}
268+
269+
public function provideWriteConcerns(): array
270+
{
271+
// Note: WriteConcern constructor prohibits w=-1 or w=0 and journal=true
272+
return [
273+
'MONGOC_WRITE_CONCERN_W_MAJORITY' => [true, new WriteConcern(-3)],
274+
'MONGOC_WRITE_CONCERN_W_DEFAULT' => [true, new WriteConcern(-2)],
275+
'MONGOC_WRITE_CONCERN_W_DEFAULT and journal=true' => [true, new WriteConcern(-2, 0, true)],
276+
'MONGOC_WRITE_CONCERN_W_ERRORS_IGNORED' => [false, new WriteConcern(-1)],
277+
'MONGOC_WRITE_CONCERN_W_ERRORS_IGNORED and journal=false' => [false, new WriteConcern(-1, 0, false)],
278+
'MONGOC_WRITE_CONCERN_W_UNACKNOWLEDGED' => [false, new WriteConcern(0)],
279+
'MONGOC_WRITE_CONCERN_W_UNACKNOWLEDGED and journal=false' => [false, new WriteConcern(0, 0, false)],
280+
'w=1' => [true, new WriteConcern(1)],
281+
'w=1 and journal=false' => [true, new WriteConcern(1, 0, false)],
282+
'w=1 and journal=true' => [true, new WriteConcern(1, 0, true)],
283+
'majority' => [true, new WriteConcern(WriteConcern::MAJORITY)],
284+
'tag' => [true, new WriteConcern('tag')],
285+
];
286+
}
258287
}

‎tests/Operation/DeleteFunctionalTest.php

+41
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use MongoDB\Driver\BulkWrite;
88
use MongoDB\Driver\WriteConcern;
99
use MongoDB\Exception\BadMethodCallException;
10+
use MongoDB\Exception\UnsupportedException;
1011
use MongoDB\Operation\Delete;
1112
use MongoDB\Tests\CommandObserver;
1213

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

67+
public function testHintOptionUnsupportedClientSideError(): void
68+
{
69+
if (version_compare($this->getServerVersion(), '3.4.0', '>=')) {
70+
$this->markTestSkipped('server reports error for unsupported delete options');
71+
}
72+
73+
$operation = new Delete(
74+
$this->getDatabaseName(),
75+
$this->getCollectionName(),
76+
[],
77+
0,
78+
['hint' => '_id_']
79+
);
80+
81+
$this->expectException(UnsupportedException::class);
82+
$this->expectExceptionMessage('Hint is not supported by the server executing this operation');
83+
84+
$operation->execute($this->getPrimaryServer());
85+
}
86+
87+
public function testHintOptionAndUnacknowledgedWriteConcernUnsupportedClientSideError(): void
88+
{
89+
if (version_compare($this->getServerVersion(), '4.4.0', '>=')) {
90+
$this->markTestSkipped('hint is supported');
91+
}
92+
93+
$operation = new Delete(
94+
$this->getDatabaseName(),
95+
$this->getCollectionName(),
96+
[],
97+
0,
98+
['hint' => '_id_', 'writeConcern' => new WriteConcern(0)]
99+
);
100+
101+
$this->expectException(UnsupportedException::class);
102+
$this->expectExceptionMessage('Hint is not supported by the server executing this operation');
103+
104+
$operation->execute($this->getPrimaryServer());
105+
}
106+
66107
public function testSessionOption(): void
67108
{
68109
if (version_compare($this->getServerVersion(), '3.6.0', '<')) {

‎tests/Operation/FindAndModifyFunctionalTest.php

+38
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
use MongoDB\Driver\BulkWrite;
66
use MongoDB\Driver\ReadPreference;
7+
use MongoDB\Driver\WriteConcern;
8+
use MongoDB\Exception\UnsupportedException;
79
use MongoDB\Model\BSONDocument;
810
use MongoDB\Operation\FindAndModify;
911
use MongoDB\Tests\CommandObserver;
@@ -54,6 +56,42 @@ function (array $event): void {
5456
);
5557
}
5658

59+
public function testHintOptionUnsupportedClientSideError(): void
60+
{
61+
if (version_compare($this->getServerVersion(), '4.2.0', '>=')) {
62+
$this->markTestSkipped('server reports error for unsupported findAndModify options');
63+
}
64+
65+
$operation = new FindAndModify(
66+
$this->getDatabaseName(),
67+
$this->getCollectionName(),
68+
['remove' => true, 'hint' => '_id_']
69+
);
70+
71+
$this->expectException(UnsupportedException::class);
72+
$this->expectExceptionMessage('Hint is not supported by the server executing this operation');
73+
74+
$operation->execute($this->getPrimaryServer());
75+
}
76+
77+
public function testHintOptionAndUnacknowledgedWriteConcernUnsupportedClientSideError(): void
78+
{
79+
if (version_compare($this->getServerVersion(), '4.4.0', '>=')) {
80+
$this->markTestSkipped('hint is supported');
81+
}
82+
83+
$operation = new FindAndModify(
84+
$this->getDatabaseName(),
85+
$this->getCollectionName(),
86+
['remove' => true, 'hint' => '_id_', 'writeConcern' => new WriteConcern(0)]
87+
);
88+
89+
$this->expectException(UnsupportedException::class);
90+
$this->expectExceptionMessage('Hint is not supported by the server executing this operation');
91+
92+
$operation->execute($this->getPrimaryServer());
93+
}
94+
5795
public function testSessionOption(): void
5896
{
5997
if (version_compare($this->getServerVersion(), '3.6.0', '<')) {

‎tests/Operation/UpdateFunctionalTest.php

+41
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use MongoDB\Driver\BulkWrite;
88
use MongoDB\Driver\WriteConcern;
99
use MongoDB\Exception\BadMethodCallException;
10+
use MongoDB\Exception\UnsupportedException;
1011
use MongoDB\Operation\Update;
1112
use MongoDB\Tests\CommandObserver;
1213
use MongoDB\UpdateResult;
@@ -98,6 +99,46 @@ function (array $event): void {
9899
);
99100
}
100101

102+
public function testHintOptionUnsupportedClientSideError(): void
103+
{
104+
if (version_compare($this->getServerVersion(), '3.4.0', '>=')) {
105+
$this->markTestSkipped('server reports error for unsupported update options');
106+
}
107+
108+
$operation = new Update(
109+
$this->getDatabaseName(),
110+
$this->getCollectionName(),
111+
['_id' => 1],
112+
['$inc' => ['x' => 1]],
113+
['hint' => '_id_']
114+
);
115+
116+
$this->expectException(UnsupportedException::class);
117+
$this->expectExceptionMessage('Hint is not supported by the server executing this operation');
118+
119+
$operation->execute($this->getPrimaryServer());
120+
}
121+
122+
public function testHintOptionAndUnacknowledgedWriteConcernUnsupportedClientSideError(): void
123+
{
124+
if (version_compare($this->getServerVersion(), '4.2.0', '>=')) {
125+
$this->markTestSkipped('hint is supported');
126+
}
127+
128+
$operation = new Update(
129+
$this->getDatabaseName(),
130+
$this->getCollectionName(),
131+
['_id' => 1],
132+
['$inc' => ['x' => 1]],
133+
['hint' => '_id_', 'writeConcern' => new WriteConcern(0)]
134+
);
135+
136+
$this->expectException(UnsupportedException::class);
137+
$this->expectExceptionMessage('Hint is not supported by the server executing this operation');
138+
139+
$operation->execute($this->getPrimaryServer());
140+
}
141+
101142
public function testUpdateOne(): void
102143
{
103144
$this->createFixtures(3);

0 commit comments

Comments
 (0)