Skip to content

PHPLIB-1147, PHPLIB-1148: Int64 improvements #1112

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

Merged
merged 3 commits into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion phpunit.evergreen.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
beStrictAboutOutputDuringTests="true"
beStrictAboutChangesToGlobalState="true"
colors="true"
bootstrap="vendor/autoload.php"
bootstrap="tests/bootstrap.php"
defaultTestSuite="Default Test Suite"
>

Expand Down
2 changes: 1 addition & 1 deletion phpunit.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
beStrictAboutOutputDuringTests="true"
beStrictAboutChangesToGlobalState="true"
colors="true"
bootstrap="vendor/autoload.php"
bootstrap="tests/bootstrap.php"
defaultTestSuite="Default Test Suite"
>

Expand Down
45 changes: 45 additions & 0 deletions tests/Comparator/Int64Comparator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

namespace MongoDB\Tests\Comparator;
Copy link
Member

Choose a reason for hiding this comment

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

This class could be in the src directory, with namespace MongoDB\Test\Comparator. The need to compare Int64 also exist for the developers using this lib.

Copy link
Member

Choose a reason for hiding this comment

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

Per our discussion during the team meeting (after this comment was made), we can look into this down the line. A separate ticket could be used to track that.

@alcaeus and I previously discussed publishing various utility functions from our test suite for the benefit of downstream libraries. Various functions for detecting server functionality or comparing BSON documents might prove useful for things like Doctrine and the Laravel integration. I don't believe we have a ticket for that, so I'd suggest grouping all of these suggestions under a single epic to create a collection of shared code downstream test suites. Then the ticket for the Int64 comparator can be grouped under there, along with anything else we consider.

Copy link
Member

@GromNaN GromNaN Jun 21, 2023

Choose a reason for hiding this comment

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

Understood, exposing such PHPUnit extension requires that we think about the supported PHPUnit versions compatibility promise. If a user feels the need for this comparator, please raise an issue or add a comment to the new task PHPLIB-1165.


use MongoDB\BSON\Int64;
use SebastianBergmann\Comparator\Comparator;
use SebastianBergmann\Comparator\ComparisonFailure;

use function is_numeric;
use function sprintf;

class Int64Comparator extends Comparator
{
public function accepts($expected, $actual)
{
// Only compare if either value is an Int64 and the other value is numeric
return ($expected instanceof Int64 && $this->isComparable($actual))
|| ($actual instanceof Int64 && $this->isComparable($expected));
}

public function assertEquals($expected, $actual, $delta = 0.0, $canonicalize = false, $ignoreCase = false): void
{
if ($expected == $actual) {
return;
}

throw new ComparisonFailure(
$expected,
$actual,
'',
'',
false,
sprintf(
'Failed asserting that %s matches expected %s.',
$this->exporter->export($actual),
$this->exporter->export($expected)
)
);
}

private function isComparable($value): bool
{
return $value instanceof Int64 || is_numeric($value);
}
}
211 changes: 211 additions & 0 deletions tests/Comparator/Int64ComparatorTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
<?php

namespace MongoDB\Tests\Comparator;

use Generator;
use MongoDB\BSON\Int64;
use PHPUnit\Framework\TestCase;
use SebastianBergmann\Comparator\ComparisonFailure;

class Int64ComparatorTest extends TestCase
{
/** @dataProvider provideAcceptsValues */
public function testAccepts(bool $expectedResult, $expectedValue, $actualValue): void
{
$this->assertSame($expectedResult, (new Int64Comparator())->accepts($expectedValue, $actualValue));
}

public static function provideAcceptsValues(): Generator
{
yield 'Expects Int64, Actual Int64' => [
'expectedResult' => true,
'expectedValue' => new Int64(123),
'actualValue' => new Int64(123),
];

yield 'Expects Int64, Actual int' => [
'expectedResult' => true,
'expectedValue' => new Int64(123),
'actualValue' => 123,
];

yield 'Expects Int64, Actual int string' => [
'expectedResult' => true,
'expectedValue' => new Int64(123),
'actualValue' => '123',
];

yield 'Expects Int64, Actual float' => [
'expectedResult' => true,
'expectedValue' => new Int64(123),
'actualValue' => 123.0,
];

yield 'Expects Int64, Actual float string' => [
'expectedResult' => true,
'expectedValue' => new Int64(123),
'actualValue' => '123.0',
];

yield 'Expects Int64, Actual non-numeric string' => [
'expectedResult' => false,
'expectedValue' => new Int64(123),
'actualValue' => 'foo',
];

yield 'Expects int, Actual Int64' => [
'expectedResult' => true,
'expectedValue' => 123,
'actualValue' => new Int64(123),
];

yield 'Expects int string, Actual Int64' => [
'expectedResult' => true,
'expectedValue' => '123',
'actualValue' => new Int64(123),
];

yield 'Expects float, Actual Int64' => [
'expectedResult' => true,
'expectedValue' => 123.0,
'actualValue' => new Int64(123),
];

yield 'Expects float string, Actual Int64' => [
'expectedResult' => true,
'expectedValue' => '123.0',
'actualValue' => new Int64(123),
];

yield 'Expects non-numeric string, Actual Int64' => [
'expectedResult' => false,
'expectedValue' => 'foo',
'actualValue' => new Int64(123),
];

yield 'Expects float, Actual float' => [
'expectedResult' => false,
'expectedValue' => 123.0,
'actualValue' => 123.0,
];

yield 'Expects numeric string, Actual numeric string' => [
'expectedResult' => false,
'expectedValue' => '123',
'actualValue' => '123',
];
}

/**
* @dataProvider provideMatchingAssertions
* @doesNotPerformAssertions
*/
public function testMatchingAssertions($expected, $actual): void
{
(new Int64Comparator())->assertEquals($expected, $actual);
}

public static function provideMatchingAssertions(): Generator
{
yield 'Expected Int64, Actual Int64' => [
'expected' => new Int64(8589934592),
'actual' => new Int64(8589934592),
];

yield 'Expected Int64, Actual int' => [
'expected' => new Int64(8589934592),
'actual' => 8589934592,
Copy link
Member

Choose a reason for hiding this comment

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

I realize we don't test PHPLIB on 32-bit platforms, but is this integer literal problematic or would PHP interpret it as a float?

];

yield 'Expected Int64, Actual int string' => [
'expected' => new Int64(8589934592),
'actual' => '8589934592',
];

yield 'Expected Int64, Actual float' => [
'expected' => new Int64(8589934592),
'actual' => 8589934592.0,
];

yield 'Expected Int64, Actual float string' => [
'expected' => new Int64(8589934592),
'actual' => '8589934592.0',
];

yield 'Expected int, Actual Int64' => [
'expected' => 8589934592,
'actual' => new Int64(8589934592),
];

yield 'Expected int string, Actual Int64' => [
'expected' => '8589934592',
'actual' => new Int64(8589934592),
];

yield 'Expected float, Actual Int64' => [
'expected' => 8589934592.0,
'actual' => new Int64(8589934592),
];

yield 'Expected float string, Actual Int64' => [
'expected' => '8589934592.0',
'actual' => new Int64(8589934592),
];
}

/** @dataProvider provideFailingValues */
public function testFailingAssertions($expected, $actual): void
{
$this->expectException(ComparisonFailure::class);

(new Int64Comparator())->assertEquals($expected, $actual);
}

public static function provideFailingValues(): Generator
{
yield 'Expected Int64, Actual Int64' => [
'expected' => new Int64(8589934592),
'actual' => new Int64(456),
];

yield 'Expected Int64, Actual int' => [
'expected' => new Int64(8589934592),
'actual' => 456,
];

yield 'Expected Int64, Actual int string' => [
'expected' => new Int64(8589934592),
'actual' => '456',
];

yield 'Expected Int64, Actual float' => [
'expected' => new Int64(8589934592),
'actual' => 8589934592.1,
];

yield 'Expected Int64, Actual float string' => [
'expected' => new Int64(8589934592),
'actual' => '8589934592.1',
];

yield 'Expected int, Actual Int64' => [
'expected' => 8589934592,
'actual' => new Int64(456),
];

yield 'Expected int string, Actual Int64' => [
'expected' => '8589934592',
'actual' => new Int64(456),
];

yield 'Expected float, Actual Int64' => [
'expected' => 8589934592.1,
'actual' => new Int64(456),
];

yield 'Expected float string, Actual Int64' => [
'expected' => '8589934592.1',
'actual' => new Int64(456),
];
}
}
11 changes: 0 additions & 11 deletions tests/SpecTests/ClientSideEncryption/FunctionalTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace MongoDB\Tests\SpecTests\ClientSideEncryption;

use MongoDB\BSON\Int64;
use MongoDB\Client;
use MongoDB\Driver\WriteConcern;
use MongoDB\Tests\SpecTests\FunctionalTestCase as BaseFunctionalTestCase;
Expand All @@ -14,8 +13,6 @@
use function is_executable;
use function is_readable;
use function sprintf;
use function strlen;
use function unserialize;

use const DIRECTORY_SEPARATOR;
use const PATH_SEPARATOR;
Expand Down Expand Up @@ -69,14 +66,6 @@ protected static function insertKeyVaultData(Client $client, ?array $keyVaultDat
$collection->insertMany($keyVaultData);
}

protected static function createInt64(string $value): Int64
{
$array = sprintf('a:1:{s:7:"integer";s:%d:"%s";}', strlen($value), $value);
$int64 = sprintf('C:%d:"%s":%d:{%s}', strlen(Int64::class), Int64::class, strlen($array), $array);

return unserialize($int64);
}

private function createTestCollection(?stdClass $encryptedFields = null, ?stdClass $jsonSchema = null): void
{
$context = $this->getContext();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use MongoDB\BSON\Binary;
use MongoDB\BSON\Decimal128;
use MongoDB\BSON\Document;
use MongoDB\BSON\Int64;
use MongoDB\BSON\UTCDateTime;
use MongoDB\Driver\ClientEncryption;
use MongoDB\Driver\Exception\EncryptionException;
Expand Down Expand Up @@ -168,8 +169,8 @@ public static function provideTypeAndRangeOpts(): Generator
yield 'Long' => [
'Long',
[
'min' => self::createInt64('0'),
'max' => self::createInt64('200'),
'min' => new Int64(0),
'max' => new Int64(200),
'sparsity' => 1,
],
];
Expand Down Expand Up @@ -467,7 +468,7 @@ private static function getCastCallableForType(string $type): callable

case 'Long':
return function (int $value) {
return self::createInt64((string) $value);
return new Int64($value);
};

default:
Expand Down
14 changes: 2 additions & 12 deletions tests/SpecTests/ClientSideEncryptionSpecTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@
use function json_decode;
use function sprintf;
use function str_repeat;
use function strlen;
use function substr;
use function unserialize;

use const DIRECTORY_SEPARATOR;
use const PATH_SEPARATOR;
Expand Down Expand Up @@ -1862,14 +1860,6 @@ public static function provideRewrapManyDataKeySrcAndDstProviders()
}
}

private function createInt64(string $value): Int64
{
$array = sprintf('a:1:{s:7:"integer";s:%d:"%s";}', strlen($value), $value);
$int64 = sprintf('C:%d:"%s":%d:{%s}', strlen(Int64::class), Int64::class, strlen($array), $array);

return unserialize($int64);
}

private function createTestCollection(?stdClass $encryptedFields = null, ?stdClass $jsonSchema = null): void
{
$context = $this->getContext();
Expand Down Expand Up @@ -1936,7 +1926,7 @@ private function encryptCorpusValue(string $fieldName, stdClass $data, ClientEnc
/* Note: workaround issue where mongocryptd refuses to encrypt
* 32-bit integers if schemaMap defines a "long" BSON type. */
$value = $data->type === 'long' && ! $data->value instanceof Int64
? $this->createInt64($data->value)
? new Int64($data->value)
: $data->value;

$encrypted = $clientEncryption->encrypt($value, $encryptionOptions);
Expand Down Expand Up @@ -1987,7 +1977,7 @@ private function prepareCorpusData(string $fieldName, stdClass $data, ClientEncr
/* Note: workaround issue where mongocryptd refuses to encrypt
* 32-bit integers if schemaMap defines a "long" BSON type. */
if ($data->type === 'long' && ! $data->value instanceof Int64) {
$data->value = $this->createInt64($data->value);
$data->value = new Int64($data->value);
}

return $data;
Expand Down
Loading