Skip to content

Commit c8054d2

Browse files
jmikolajmikola
committed
Use Matches for CollectionData and validate operator syntax
1 parent 8085eb4 commit c8054d2

File tree

3 files changed

+120
-45
lines changed

3 files changed

+120
-45
lines changed

‎tests/UnifiedSpecTests/CollectionData.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
use MongoDB\Driver\ReadConcern;
99
use MongoDB\Driver\ReadPreference;
1010
use MongoDB\Driver\WriteConcern;
11-
use MongoDB\Tests\UnifiedSpecTests\Constraint\DocumentsMatch;
11+
use MongoDB\Tests\UnifiedSpecTests\Constraint\Matches;
1212
use MultipleIterator;
1313
use stdClass;
1414
use function sprintf;
@@ -88,7 +88,9 @@ public function assertOutcome(Client $client)
8888
assertNotNull($expectedDocument);
8989
assertNotNull($actualDocument);
9090

91-
$constraint = new DocumentsMatch($expectedDocument, false, false);
91+
/* Disable extra root keys and operators when matching, which is
92+
* effectively an exact match that allows key order variation. */
93+
$constraint = new Matches($expectedDocument, null, false, false);
9294
assertThat($actualDocument, $constraint, sprintf('documents[%d] match', $i));
9395
}
9496
}

‎tests/UnifiedSpecTests/Constraint/Matches.php

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use MongoDB\Tests\UnifiedSpecTests\EntityMap;
1212
use PHPUnit\Framework\Constraint\Constraint;
1313
use PHPUnit\Framework\Constraint\LogicalOr;
14+
use PHPUnit\Framework\ExpectationFailedException;
1415
use RuntimeException;
1516
use SebastianBergmann\Comparator\ComparisonFailure;
1617
use SebastianBergmann\Comparator\Factory;
@@ -47,13 +48,21 @@ class Matches extends Constraint
4748
/** @var mixed */
4849
private $value;
4950

51+
/** @var bool */
52+
private $allowExtraRootKeys;
53+
54+
/** @var bool */
55+
private $allowOperators;
56+
5057
/** @var ComparisonFailure|null */
5158
private $lastFailure;
5259

53-
public function __construct($value, EntityMap $entityMap = null)
60+
public function __construct($value, EntityMap $entityMap = null, $allowExtraRootKeys = true, $allowOperators = true)
5461
{
5562
$this->value = self::prepare($value);
5663
$this->entityMap = $entityMap ?? new EntityMap();
64+
$this->allowExtraRootKeys = $allowExtraRootKeys;
65+
$this->allowOperators = $allowOperators;
5766
$this->comparatorFactory = Factory::getInstance();
5867
}
5968

@@ -66,7 +75,13 @@ public function evaluate($other, $description = '', $returnResult = false)
6675
try {
6776
$this->assertMatches($this->value, $other);
6877
$success = true;
78+
} catch (ExpectationFailedException $e) {
79+
/* Rethrow internal assertion failures (e.g. operator type checks,
80+
* EntityMap errors), which are logical errors in the code/test. */
81+
throw $e;
6982
} catch (RuntimeException $e) {
83+
/* This will generally catch internal errors from failAt(), which
84+
* include a key path to pinpoint the failure. */
7085
$this->lastFailure = new ComparisonFailure(
7186
$this->value,
7287
$other,
@@ -98,16 +113,10 @@ private function assertEquals($expected, $actual, string $keyPath)
98113

99114
try {
100115
$this->comparatorFactory->getComparatorFor($expected, $actual)->assertEquals($expected, $actual);
101-
} catch (ComparisonFailure $failure) {
102-
throw new ComparisonFailure(
103-
$expected,
104-
$actual,
105-
// No diff is required
106-
'',
107-
'',
108-
false,
109-
(empty($keyPath) ? '' : sprintf('Field path "%s": ', $keyPath)) . $failure->getMessage()
110-
);
116+
} catch (ComparisonFailure $e) {
117+
/* Disregard other ComparisonFailure fields, as evaluate() only uses
118+
* the message when creating its own ComparisonFailure. */
119+
self::failAt($e->getMessage(), $keyPath);
111120
}
112121
}
113122

@@ -150,7 +159,7 @@ private function assertMatchesArray(BSONArray $expected, $actual, string $keyPat
150159

151160
private function assertMatchesDocument(BSONDocument $expected, $actual, string $keyPath)
152161
{
153-
if (self::isOperator($expected)) {
162+
if ($this->allowOperators && self::isOperator($expected)) {
154163
$this->assertMatchesOperator($expected, $actual, $keyPath);
155164

156165
return;
@@ -164,11 +173,12 @@ private function assertMatchesDocument(BSONDocument $expected, $actual, string $
164173
foreach ($expected as $key => $expectedValue) {
165174
$actualKeyExists = $actual->offsetExists($key);
166175

167-
if ($expectedValue instanceof BSONDocument && self::isOperator($expectedValue)) {
176+
if ($this->allowOperators && $expectedValue instanceof BSONDocument && self::isOperator($expectedValue)) {
168177
$operatorName = self::getOperatorName($expectedValue);
169178

170-
// TODO: Validate structure of operators
171179
if ($operatorName === '$$exists') {
180+
assertInternalType('bool', $expectedValue['$$exists'], '$$exists requires bool');
181+
172182
if ($expectedValue['$$exists'] && ! $actualKeyExists) {
173183
self::failAt(sprintf('$actual does not have expected key "%s"', $key), $keyPath);
174184
}
@@ -200,8 +210,8 @@ private function assertMatchesDocument(BSONDocument $expected, $actual, string $
200210
);
201211
}
202212

203-
// Ignore extra fields in root documents
204-
if (empty($keyPath)) {
213+
// Ignore extra keys in root documents
214+
if ($this->allowExtraRootKeys && empty($keyPath)) {
205215
return;
206216
}
207217

@@ -216,8 +226,13 @@ private function assertMatchesOperator(BSONDocument $operator, $actual, string $
216226
{
217227
$name = self::getOperatorName($operator);
218228

219-
// TODO: Validate structure of operators
220229
if ($name === '$$type') {
230+
assertThat(
231+
$operator['$$type'],
232+
logicalOr(isType('string'), logicalAnd(isInstanceOf(BSONArray::class), containsOnly('string'))),
233+
'$$type requires string or string[]'
234+
);
235+
221236
$types = is_string($operator['$$type']) ? [$operator['$$type']] : $operator['$$type'];
222237
$constraints = [];
223238

@@ -235,6 +250,8 @@ private function assertMatchesOperator(BSONDocument $operator, $actual, string $
235250
}
236251

237252
if ($name === '$$matchesEntity') {
253+
assertInternalType('string', $operator['$$matchesEntity'], '$$matchesEntity requires string');
254+
238255
$this->assertMatches(
239256
$this->prepare($this->entityMap[$operator['$$matchesEntity']]),
240257
$actual,
@@ -245,6 +262,9 @@ private function assertMatchesOperator(BSONDocument $operator, $actual, string $
245262
}
246263

247264
if ($name === '$$matchesHexBytes') {
265+
assertInternalType('string', $operator['$$matchesHexBytes'], '$$matchesHexBytes requires string');
266+
assertRegExp('/^([0-9a-fA-F]{2})+$/', $operator['$$matchesHexBytes'], '$$matchesHexBytes requires pairs of hex chars');
267+
248268
if (! is_resource($actual) || get_resource_type($actual) != "stream") {
249269
self::failAt(sprintf('%s is not a stream', $this->exporter()->shortenedExport($actual)), $keyPath);
250270
}
@@ -274,11 +294,11 @@ private function assertMatchesOperator(BSONDocument $operator, $actual, string $
274294
}
275295

276296
if ($name === '$$sessionLsid') {
297+
assertInternalType('string', $operator['$$sessionLsid'], '$$sessionLsid requires string');
298+
277299
$session = $this->entityMap[$operator['$$sessionLsid']];
278300

279-
if (! $session instanceof Session) {
280-
self::failAt(sprintf('entity "%s" is not a session', $operator['$$sessionLsid']), $keyPath);
281-
}
301+
assertInstanceOf(Session::class, $session, '$$sessionLsid requires session entity');
282302

283303
$this->assertEquals(
284304
self::prepare($session->getLogicalSessionId()),

‎tests/UnifiedSpecTests/Constraint/MatchesTest.php

Lines changed: 76 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,26 @@ class MatchesTest extends FunctionalTestCase
1818
public function testMatchesDocument()
1919
{
2020
$c = new Matches(['x' => 1, 'y' => ['a' => 1, 'b' => 2]]);
21-
2221
$this->assertResult(false, $c, ['x' => 1, 'y' => 2], 'Incorrect value');
2322
$this->assertResult(true, $c, ['x' => 1, 'y' => ['a' => 1, 'b' => 2]], 'Exact match');
2423
$this->assertResult(true, $c, ['x' => 1, 'y' => ['a' => 1, 'b' => 2], 'z' => 3], 'Extra keys in root are permitted');
2524
$this->assertResult(false, $c, ['x' => 1, 'y' => ['a' => 1, 'b' => 2, 'c' => 3]], 'Extra keys in embedded are not permitted');
2625
$this->assertResult(true, $c, ['y' => ['b' => 2, 'a' => 1], 'x' => 1], 'Root and embedded key order is not significant');
2726
}
2827

28+
public function testDoNotAllowExtraRootKeys()
29+
{
30+
$c = new Matches(['x' => 1], null, false);
31+
$this->assertResult(false, $c, ['x' => 1, 'y' => 1], 'Extra keys in root are prohibited');
32+
}
33+
34+
public function testDoNotAllowOperators()
35+
{
36+
$c = new Matches(['x' => ['$$exists' => true]], null, true, false);
37+
$this->assertResult(false, $c, ['x' => 1], 'Operators are not processed');
38+
$this->assertResult(true, $c, ['x' => ['$$exists' => true]], 'Operators are not processed but compared as-is');
39+
}
40+
2941
public function testOperatorExists()
3042
{
3143
$c = new Matches(['x' => ['$$exists' => true]]);
@@ -52,12 +64,10 @@ public function testOperatorExists()
5264
public function testOperatorType()
5365
{
5466
$c = new Matches(['x' => ['$$type' => 'string']]);
55-
5667
$this->assertResult(true, $c, ['x' => 'foo'], 'string matches string type');
5768
$this->assertResult(false, $c, ['x' => 1], 'integer does not match string type');
5869

5970
$c = new Matches(['x' => ['$$type' => ['string', 'bool']]]);
60-
6171
$this->assertResult(true, $c, ['x' => 'foo'], 'string matches [string,bool] type');
6272
$this->assertResult(true, $c, ['x' => true], 'bool matches [string,bool] type');
6373
$this->assertResult(false, $c, ['x' => 1], 'integer does not match [string,bool] type');
@@ -70,30 +80,19 @@ public function testOperatorMatchesEntity()
7080
$entityMap['object'] = ['y' => 1];
7181

7282
$c = new Matches(['x' => ['$$matchesEntity' => 'integer']], $entityMap);
73-
7483
$this->assertResult(true, $c, ['x' => 1], 'value matches integer entity (embedded)');
7584
$this->assertResult(false, $c, ['x' => 2], 'value does not match integer entity (embedded)');
7685
$this->assertResult(false, $c, ['x' => ['y' => 1]], 'value does not match integer entity (embedded)');
7786

7887
$c = new Matches(['x' => ['$$matchesEntity' => 'object']], $entityMap);
79-
8088
$this->assertResult(true, $c, ['x' => ['y' => 1]], 'value matches object entity (embedded)');
8189
$this->assertResult(false, $c, ['x' => 1], 'value does not match object entity (embedded)');
8290
$this->assertResult(false, $c, ['x' => ['y' => 1, 'z' => 2]], 'value does not match object entity (embedded)');
8391

8492
$c = new Matches(['$$matchesEntity' => 'object'], $entityMap);
85-
8693
$this->assertResult(true, $c, ['y' => 1], 'value matches object entity (root-level)');
8794
$this->assertResult(true, $c, ['x' => 2, 'y' => 1], 'value matches object entity (root-level)');
8895
$this->assertResult(false, $c, ['x' => ['y' => 1, 'z' => 2]], 'value does not match object entity (root-level)');
89-
90-
$c = new Matches(['$$matchesEntity' => 'undefined'], $entityMap);
91-
92-
$this->assertResult(false, $c, 'undefined', 'value does not match undefined entity (root-level)');
93-
94-
$c = new Matches(['x' => ['$$matchesEntity' => 'undefined']], $entityMap);
95-
96-
$this->assertResult(false, $c, ['x' => 'undefined'], 'value does not match undefined entity (embedded)');
9796
}
9897

9998
public function testOperatorMatchesHexBytes()
@@ -107,13 +106,11 @@ public function testOperatorMatchesHexBytes()
107106
rewind($stream2);
108107

109108
$c = new Matches(['$$matchesHexBytes' => 'DEADBEEF']);
110-
111109
$this->assertResult(true, $c, $stream1, 'value matches hex bytes (root-level)');
112110
$this->assertResult(false, $c, $stream2, 'value does not match hex bytes (root-level)');
113111
$this->assertResult(false, $c, 1, 'value is not a stream');
114112

115113
$c = new Matches(['x' => ['$$matchesHexBytes' => '90ABCDEF']]);
116-
117114
$this->assertResult(true, $c, ['x' => $stream2], 'value matches hex bytes (embedded)');
118115
$this->assertResult(false, $c, ['x' => $stream1], 'value does not match hex bytes (embedded)');
119116
$this->assertResult(false, $c, ['x' => 1], 'value is not a stream');
@@ -122,14 +119,12 @@ public function testOperatorMatchesHexBytes()
122119
public function testOperatorUnsetOrMatches()
123120
{
124121
$c = new Matches(['$$unsetOrMatches' => ['x' => 1]]);
125-
126122
$this->assertResult(true, $c, null, 'null value is considered unset (root-level)');
127123
$this->assertResult(true, $c, ['x' => 1], 'value matches (root-level)');
128124
$this->assertResult(true, $c, ['x' => 1, 'y' => 1], 'value matches (root-level)');
129125
$this->assertResult(false, $c, ['x' => 2], 'value does not match (root-level)');
130126

131127
$c = new Matches(['x' => ['$$unsetOrMatches' => ['y' => 1]]]);
132-
133128
$this->assertResult(true, $c, new stdClass(), 'missing value is considered unset (embedded)');
134129
$this->assertResult(false, $c, ['x' => null], 'null value is not considered unset (embedded)');
135130
$this->assertResult(true, $c, ['x' => ['y' => 1]], 'value matches (embedded)');
@@ -151,14 +146,12 @@ public function testOperatorSessionLsid()
151146
$lsidWithExtraField = (array) $session->getLogicalSessionId() + ['y' => 1];
152147

153148
$c = new Matches(['$$sessionLsid' => 'session'], $entityMap);
154-
155149
$this->assertResult(true, $c, $session->getLogicalSessionId(), 'session LSID matches (root-level)');
156150
$this->assertResult(false, $c, $lsidWithWrongId, 'session LSID does not match (root-level)');
157151
$this->assertResult(false, $c, $lsidWithExtraField, 'session LSID does not match (root-level)');
158152
$this->assertResult(false, $c, 1, 'session LSID does not match (root-level)');
159153

160154
$c = new Matches(['x' => ['$$sessionLsid' => 'session']], $entityMap);
161-
162155
$this->assertResult(true, $c, ['x' => $session->getLogicalSessionId()], 'session LSID matches (embedded)');
163156
$this->assertResult(false, $c, ['x' => $lsidWithWrongId], 'session LSID does not match (embedded)');
164157
$this->assertResult(false, $c, ['x' => $lsidWithExtraField], 'session LSID does not match (embedded)');
@@ -173,9 +166,9 @@ public function testErrorMessages($expectedMessagePart, Matches $constraint, $ac
173166
try {
174167
$constraint->evaluate($actualValue);
175168
$this->fail('Expected a comparison failure');
176-
} catch (ExpectationFailedException $failure) {
177-
$this->assertStringContainsString('Failed asserting that expected value matches actual value.', $failure->getMessage());
178-
$this->assertStringContainsString($expectedMessagePart, $failure->getMessage());
169+
} catch (ExpectationFailedException $e) {
170+
$this->assertStringContainsString('Failed asserting that expected value matches actual value.', $e->getMessage());
171+
$this->assertStringContainsString($expectedMessagePart, $e->getMessage());
179172
}
180173
}
181174

@@ -250,6 +243,66 @@ public function errorMessageProvider()
250243
];
251244
}
252245

246+
/**
247+
* @dataProvider operatorErrorMessageProvider
248+
*/
249+
public function testOperatorSyntaxValidation($expectedMessage, Matches $constraint)
250+
{
251+
$this->expectException(ExpectationFailedException::class);
252+
$this->expectExceptionMessage($expectedMessage);
253+
254+
$constraint->evaluate(['x' => 1], '', true);
255+
}
256+
257+
public function operatorErrorMessageProvider()
258+
{
259+
$entityMap = new EntityMap();
260+
$entityMap['notSession'] = 1;
261+
262+
return [
263+
'$$exists type' => [
264+
'$$exists requires bool',
265+
new Matches(['x' => ['$$exists' => 1]]),
266+
],
267+
'$$type type (string)' => [
268+
'$$type requires string or string[]',
269+
new Matches(['x' => ['$$type' => 1]]),
270+
],
271+
'$$type type (string[])' => [
272+
'$$type requires string or string[]',
273+
new Matches(['x' => ['$$type' => [1]]]),
274+
],
275+
'$$matchesEntity type' => [
276+
'$$matchesEntity requires string',
277+
new Matches(['x' => ['$$matchesEntity' => 1]]),
278+
],
279+
'$$matchesEntity undefined entity' => [
280+
'No entity is defined for "undefined"',
281+
new Matches(['$$matchesEntity' => 'undefined']),
282+
],
283+
'$$matchesHexBytes type' => [
284+
'$$matchesHexBytes requires string',
285+
new Matches(['$$matchesHexBytes' => 1]),
286+
],
287+
'$$matchesHexBytes string format' => [
288+
'$$matchesHexBytes requires pairs of hex chars',
289+
new Matches(['$$matchesHexBytes' => 'f00']),
290+
],
291+
'$$sessionLsid type' => [
292+
'$$sessionLsid requires string',
293+
new Matches(['x' => ['$$sessionLsid' => 1]]),
294+
],
295+
'$$sessionLsid undefined entity' => [
296+
'No entity is defined for "undefined"',
297+
new Matches(['$$sessionLsid' => 'undefined']),
298+
],
299+
'$$sessionLsid invalid entity' => [
300+
'$$sessionLsid requires session entity',
301+
new Matches(['x' => ['$$sessionLsid' => 'notSession']], $entityMap),
302+
],
303+
];
304+
}
305+
253306
private function assertResult($expected, Matches $constraint, $value, $message)
254307
{
255308
$this->assertSame($expected, $constraint->evaluate($value, '', true), $message);

0 commit comments

Comments
 (0)