Skip to content

PHPLIB-1001, PHPLIB-1010, PHPLIB-1011: Improvements to $$type operator #991

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 11, 2022
Merged

PHPLIB-1001, PHPLIB-1010, PHPLIB-1011: Improvements to $$type operator #991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
691f74a
PHPLIB-1001: Support "number" alias in IsBsonType constraint
jmikola Oct 10, 2022
6cb886e
PHPLIB-1010: Relax acceptance of Int64 for $$type operators
jmikola Oct 11, 2022
61c17de
Remove code path for unused PHPUnit versions
jmikola Oct 11, 2022
7c4eac6
PHPLIB-1001: Support "number" alias for legacy $$type operator
jmikola Oct 11, 2022
77a5c0f
PHPLIB-1011: Support asserting multiple types in legacy $$type operator
jmikola Oct 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 16 additions & 153 deletions tests/SpecTests/DocumentsMatchConstraint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,11 @@

use ArrayObject;
use InvalidArgumentException;
use MongoDB\BSON\BinaryInterface;
use MongoDB\BSON\DBPointer;
use MongoDB\BSON\Decimal128;
use MongoDB\BSON\Int64;
use MongoDB\BSON\Javascript;
use MongoDB\BSON\MaxKey;
use MongoDB\BSON\MinKey;
use MongoDB\BSON\ObjectId;
use MongoDB\BSON\Regex;
use MongoDB\BSON\Symbol;
use MongoDB\BSON\Timestamp;
use MongoDB\BSON\Undefined;
use MongoDB\BSON\UTCDateTime;
use MongoDB\Model\BSONArray;
use MongoDB\Model\BSONDocument;
use MongoDB\Tests\UnifiedSpecTests\Constraint\IsBsonType;
use PHPUnit\Framework\Constraint\Constraint;
use PHPUnit\Framework\Constraint\IsInstanceOf;
use PHPUnit\Framework\Constraint\IsNull;
use PHPUnit\Framework\Constraint\IsType;
use PHPUnit\Framework\Constraint\LogicalAnd;
use PHPUnit\Framework\Constraint\LogicalNot;
use PHPUnit\Framework\Constraint\LogicalOr;
use RuntimeException;
use SebastianBergmann\Comparator\ComparisonFailure;
use SebastianBergmann\Comparator\Factory;
Expand All @@ -39,7 +22,12 @@
use function is_float;
use function is_int;
use function is_object;
use function method_exists;
use function PHPUnit\Framework\assertThat;
use function PHPUnit\Framework\containsOnly;
use function PHPUnit\Framework\isInstanceOf;
use function PHPUnit\Framework\isType;
use function PHPUnit\Framework\logicalAnd;
use function PHPUnit\Framework\logicalOr;
use function sprintf;

use const PHP_INT_SIZE;
Expand Down Expand Up @@ -132,143 +120,18 @@ private function doEvaluate($other, $description = '', $returnResult = false)
}

/**
* @param mixed $actualValue
* @param string|string[] $expectedType
* @param mixed $actualValue
*/
private function assertBSONType(string $expectedType, $actualValue): void
private function assertBSONType($expectedType, $actualValue): void
{
switch ($expectedType) {
case 'double':
(new IsType('float'))->evaluate($actualValue);
assertThat(
$expectedType,
logicalOr(isType('string'), logicalAnd(isInstanceOf(BSONArray::class), containsOnly('string'))),
'$$type requires string or string[]'
);

return;

case 'string':
(new IsType('string'))->evaluate($actualValue);

return;

case 'object':
$constraints = [
new IsType('object'),
new LogicalNot(new IsInstanceOf(BSONArray::class)),
];

// LogicalAnd::fromConstraints was introduced in PHPUnit 6.5.0.
// This check can be removed when the PHPUnit dependency is bumped to that version
if (method_exists(LogicalAnd::class, 'fromConstraints')) {
$constraint = LogicalAnd::fromConstraints(...$constraints);
} else {
$constraint = new LogicalAnd();
$constraint->setConstraints($constraints);
}

$constraint->evaluate($actualValue);

return;

case 'array':
$constraints = [
new IsType('array'),
new IsInstanceOf(BSONArray::class),
];

// LogicalOr::fromConstraints was introduced in PHPUnit 6.5.0.
// This check can be removed when the PHPUnit dependency is bumped to that version
if (method_exists(LogicalOr::class, 'fromConstraints')) {
$constraint = LogicalOr::fromConstraints(...$constraints);
} else {
$constraint = new LogicalOr();
$constraint->setConstraints($constraints);
}

$constraint->evaluate($actualValue);

return;

case 'binData':
(new IsInstanceOf(BinaryInterface::class))->evaluate($actualValue);

return;

case 'undefined':
(new IsInstanceOf(Undefined::class))->evaluate($actualValue);

return;

case 'objectId':
(new IsInstanceOf(ObjectId::class))->evaluate($actualValue);

return;

case 'boolean':
(new IsType('bool'))->evaluate($actualValue);

return;

case 'date':
(new IsInstanceOf(UTCDateTime::class))->evaluate($actualValue);

return;

case 'null':
(new IsNull())->evaluate($actualValue);

return;

case 'regex':
(new IsInstanceOf(Regex::class))->evaluate($actualValue);

return;

case 'dbPointer':
(new IsInstanceOf(DBPointer::class))->evaluate($actualValue);

return;

case 'javascript':
(new IsInstanceOf(Javascript::class))->evaluate($actualValue);

return;

case 'symbol':
(new IsInstanceOf(Symbol::class))->evaluate($actualValue);

return;

case 'int':
(new IsType('int'))->evaluate($actualValue);

return;

case 'timestamp':
(new IsInstanceOf(Timestamp::class))->evaluate($actualValue);

return;

case 'long':
if (PHP_INT_SIZE == 4) {
(new IsInstanceOf(Int64::class))->evaluate($actualValue);
} else {
(new IsType('int'))->evaluate($actualValue);
}

return;

case 'decimal':
(new IsInstanceOf(Decimal128::class))->evaluate($actualValue);

return;

case 'minKey':
(new IsInstanceOf(MinKey::class))->evaluate($actualValue);

return;

case 'maxKey':
(new IsInstanceOf(MaxKey::class))->evaluate($actualValue);

return;
}
IsBsonType::anyOf(...(array) $expectedType)->evaluate($actualValue);
}

/**
Expand Down
40 changes: 32 additions & 8 deletions tests/SpecTests/DocumentsMatchConstraintTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,34 +84,58 @@ public function testBSONTypeAssertions($type, $value): void

public function provideBSONTypes()
{
$undefined = toPHP(fromJSON('{ "undefined": {"$undefined": true} }'));
$symbol = toPHP(fromJSON('{ "symbol": {"$symbol": "test"} }'));
$dbPointer = toPHP(fromJSON('{ "dbPointer": {"$dbPointer": {"$ref": "phongo.test", "$id" : { "$oid" : "5a2e78accd485d55b405ac12" } }} }'));
$undefined = toPHP(fromJSON('{ "x": {"$undefined": true} }'))->x;
$symbol = toPHP(fromJSON('{ "x": {"$symbol": "test"} }'))->x;
$dbPointer = toPHP(fromJSON('{ "x": {"$dbPointer": {"$ref": "db.coll", "$id" : { "$oid" : "5a2e78accd485d55b405ac12" } }} }'))->x;
$int64 = unserialize('C:18:"MongoDB\BSON\Int64":28:{a:1:{s:7:"integer";s:1:"1";}}');
$long = PHP_INT_SIZE == 4 ? unserialize('C:18:"MongoDB\BSON\Int64":38:{a:1:{s:7:"integer";s:10:"4294967296";}}') : 4294967296;

return [
'double' => ['double', 1.4],
'string' => ['string', 'foo'],
'object' => ['object', new BSONDocument()],
'array' => ['array', ['foo']],
'binData' => ['binData', new Binary('', 0)],
'undefined' => ['undefined', $undefined->undefined],
'undefined' => ['undefined', $undefined],
'objectId' => ['objectId', new ObjectId()],
'boolean' => ['boolean', true],
'bool' => ['bool', true],
'date' => ['date', new UTCDateTime()],
'null' => ['null', null],
'regex' => ['regex', new Regex('.*')],
'dbPointer' => ['dbPointer', $dbPointer->dbPointer],
'dbPointer' => ['dbPointer', $dbPointer],
'javascript' => ['javascript', new Javascript('foo = 1;')],
'symbol' => ['symbol', $symbol->symbol],
'symbol' => ['symbol', $symbol],
'int' => ['int', 1],
'timestamp' => ['timestamp', new Timestamp(0, 0)],
'long' => ['long', PHP_INT_SIZE == 4 ? unserialize('C:18:"MongoDB\BSON\Int64":38:{a:1:{s:7:"integer";s:10:"4294967296";}}') : 4294967296],
'long(int64)' => ['long', $int64],
'long(long)' => ['long', $long],
'decimal' => ['decimal', new Decimal128('18446744073709551616')],
'minKey' => ['minKey', new MinKey()],
'maxKey' => ['maxKey', new MaxKey()],
'number(double)' => ['number', 1.4],
'number(decimal)' => ['number', new Decimal128('18446744073709551616')],
'number(int)' => ['number', 1],
'number(int64)' => ['number', $int64],
'number(long)' => ['number', $long],
];
}

public function testBSONTypeAssertionsWithMultipleTypes(): void
{
$c1 = new DocumentsMatchConstraint(['x' => ['$$type' => ['double', 'int']]]);

$this->assertResult(true, $c1, ['x' => 1], 'int is double or int');
$this->assertResult(true, $c1, ['x' => 1.4], 'double is double or int');
$this->assertResult(false, $c1, ['x' => 'foo'], 'string is not double or int');

$c2 = new DocumentsMatchConstraint(['x' => ['$$type' => ['number', 'string']]]);

$this->assertResult(true, $c2, ['x' => 1], 'int is number or string');
$this->assertResult(true, $c2, ['x' => 1.4], 'double is number or string');
$this->assertResult(true, $c2, ['x' => 'foo'], 'string is number or string');
$this->assertResult(false, $c2, ['x' => true], 'bool is not number or string');
}

/**
* @dataProvider errorMessageProvider
*/
Expand Down
12 changes: 5 additions & 7 deletions tests/UnifiedSpecTests/Constraint/IsBsonType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@
use function range;
use function sprintf;

use const PHP_INT_SIZE;

final class IsBsonType extends Constraint
{
use ConstraintTrait;
Expand Down Expand Up @@ -67,6 +65,7 @@ final class IsBsonType extends Constraint
'decimal',
'minKey',
'maxKey',
'number',
];

/** @var string */
Expand Down Expand Up @@ -152,11 +151,7 @@ private function doMatches($other): bool
return $other instanceof TimestampInterface;

case 'long':
if (PHP_INT_SIZE == 4) {
return $other instanceof Int64;
}

return is_int($other);
return is_int($other) || $other instanceof Int64;

case 'decimal':
return $other instanceof Decimal128Interface;
Expand All @@ -167,6 +162,9 @@ private function doMatches($other): bool
case 'maxKey':
return $other instanceof MaxKeyInterface;

case 'number':
return is_int($other) || $other instanceof Int64 || is_float($other) || $other instanceof Decimal128Interface;

default:
// This should already have been caught in the constructor
throw new LogicException('Unsupported type: ' . $this->type);
Expand Down
34 changes: 26 additions & 8 deletions tests/UnifiedSpecTests/Constraint/IsBsonTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ public function testConstraint($type, $value): void

public function provideTypes()
{
$undefined = toPHP(fromJSON('{ "undefined": {"$undefined": true} }'));
$symbol = toPHP(fromJSON('{ "symbol": {"$symbol": "test"} }'));
$dbPointer = toPHP(fromJSON('{ "dbPointer": {"$dbPointer": {"$ref": "phongo.test", "$id" : { "$oid" : "5a2e78accd485d55b405ac12" } }} }'));
$undefined = toPHP(fromJSON('{ "x": {"$undefined": true} }'))->x;
$symbol = toPHP(fromJSON('{ "x": {"$symbol": "test"} }'))->x;
$dbPointer = toPHP(fromJSON('{ "x": {"$dbPointer": {"$ref": "db.coll", "$id" : { "$oid" : "5a2e78accd485d55b405ac12" } }} }'))->x;
$int64 = unserialize('C:18:"MongoDB\BSON\Int64":28:{a:1:{s:7:"integer";s:1:"1";}}');
$long = PHP_INT_SIZE == 4 ? unserialize('C:18:"MongoDB\BSON\Int64":38:{a:1:{s:7:"integer";s:10:"4294967296";}}') : 4294967296;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our tests make the best case for providing a constructor for Int64. Can't wait to eventually get rid of it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember that a constructor would provide users a foot-gun allowing them to force 32-bit values to serialize as 64-bit, only to have BSON decoding convert them back to basic integers. Previous discussions on this:

Unless we implement some BSON codec API that allows users to coerce integer serialization (e.g. always use 64-bit BSON types), I think the current behavior is less likely to confuse folks while noting we can't please everyone.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't suggesting we add a constructor - apologies if this was misunderstood. I was alluding to getting rid of Int64 if and when we drop support for 32-bit platforms. It's wishful thinking at this point, but I hope that x86 goes out of style sometime this century ^^


return [
'double' => ['double', 1.4],
Expand All @@ -52,22 +54,28 @@ public function provideTypes()
'array(indexed array)' => ['array', ['foo']],
'array(BSONArray)' => ['array', new BSONArray()],
'binData' => ['binData', new Binary('', 0)],
'undefined' => ['undefined', $undefined->undefined],
'undefined' => ['undefined', $undefined],
'objectId' => ['objectId', new ObjectId()],
'bool' => ['bool', true],
'date' => ['date', new UTCDateTime()],
'null' => ['null', null],
'regex' => ['regex', new Regex('.*')],
'dbPointer' => ['dbPointer', $dbPointer->dbPointer],
'dbPointer' => ['dbPointer', $dbPointer],
'javascript' => ['javascript', new Javascript('foo = 1;')],
'symbol' => ['symbol', $symbol->symbol],
'symbol' => ['symbol', $symbol],
'javascriptWithScope' => ['javascriptWithScope', new Javascript('foo = 1;', ['x' => 1])],
'int' => ['int', 1],
'timestamp' => ['timestamp', new Timestamp(0, 0)],
'long' => ['long', PHP_INT_SIZE == 4 ? unserialize('C:18:"MongoDB\BSON\Int64":38:{a:1:{s:7:"integer";s:10:"4294967296";}}') : 4294967296],
'long(int64)' => ['long', $int64],
'long(long)' => ['long', $long],
'decimal' => ['decimal', new Decimal128('18446744073709551616')],
'minKey' => ['minKey', new MinKey()],
'maxKey' => ['maxKey', new MaxKey()],
'number(double)' => ['number', 1.4],
'number(decimal)' => ['number', new Decimal128('18446744073709551616')],
'number(int)' => ['number', 1],
'number(int64)' => ['number', $int64],
'number(long)' => ['number', $long],
];
}

Expand All @@ -89,10 +97,20 @@ public function testAnyOf(): void
$c = IsBsonType::anyOf('double', 'int');

$this->assertResult(true, $c, 1, 'int is double or int');
$this->assertResult(true, $c, 1.4, 'int is double or int');
$this->assertResult(true, $c, 1.4, 'double is double or int');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

$this->assertResult(false, $c, 'foo', 'string is not double or int');
}

public function testAnyOfWithNumberAlias(): void
{
$c = IsBsonType::anyOf('number', 'string');

$this->assertResult(true, $c, 1, 'int is number or string');
$this->assertResult(true, $c, 1.4, 'double is number or string');
$this->assertResult(true, $c, 'foo', 'string is number or string');
$this->assertResult(false, $c, true, 'bool is not number or string');
}

public function testErrorMessage(): void
{
$c = new IsBsonType('string');
Expand Down