Skip to content

PHPLIB-485: Inherit collection's typeMap in distinct #688

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 1 commit into from
Oct 9, 2019
Merged

PHPLIB-485: Inherit collection's typeMap in distinct #688

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions src/Collection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,10 @@ public function distinct($fieldName, $filter = [], array $options = [])
$options['readPreference'] = $this->readPreference;
}

if (! isset($options['typeMap'])) {
$options['typeMap'] = $this->typeMap;
}

$server = select_server($this->manager, $options);

if (! isset($options['readConcern']) && server_supports_feature($server, self::$wireVersionForReadConcern) && ! is_in_transaction($options)) {
Expand Down
85 changes: 85 additions & 0 deletions tests/Collection/CollectionFunctionalTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
use MongoDB\Tests\CommandObserver;
use function array_filter;
use function call_user_func;
use function is_scalar;
use function json_encode;
use function strchr;
use function usort;
use function version_compare;

/**
Expand Down Expand Up @@ -170,6 +173,88 @@ function (array $event) {
);
}

/**
* @dataProvider provideTypeMapOptionsAndExpectedDocuments
*/
public function testDistinctWithTypeMap(array $typeMap, array $expectedDocuments)
{
$bulkWrite = new BulkWrite(['ordered' => true]);
$bulkWrite->insert([
'x' => (object) ['foo' => 'bar'],
]);
$bulkWrite->insert(['x' => 4]);
$bulkWrite->insert([
'x' => (object) ['foo' => ['foo' => 'bar']],
]);
$this->manager->executeBulkWrite($this->getNamespace(), $bulkWrite);

$values = $this->collection->withOptions(['typeMap' => $typeMap])->distinct('x');

/* This sort callable sorts all scalars to the front of the list. All
* non-scalar values are sorted by running json_encode on them and
* comparing their string representations.
*/
$sort = function ($a, $b) {
Copy link
Member

@jmikola jmikola Oct 9, 2019

Choose a reason for hiding this comment

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

Would it be easier to just assert that the count of $expectedDocuments and $values is equal and then loop over $expectedDocuments and assert that each of its elements exists in $values?

I realize the only scalar is 4 at the moment, but this seems like it could be fragile if additional scalars were added to the test data.

Copy link
Member Author

@alcaeus alcaeus Oct 9, 2019

Choose a reason for hiding this comment

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

I shamelessly copied this from DistinctFunctionalTest, so I had to double-check what it does. This method generally sorts scalars to the top and non-scalars to the bottom, but each group is sorted within itself, so adding multiple scalars shouldn't pose an issue. Non-scalars are encoded to JSON before sorting, so they are sorted by their JSON representation.

if (is_scalar($a) && ! is_scalar($b)) {
return -1;
}

if (! is_scalar($a)) {
if (is_scalar($b)) {
return 1;
}

$a = json_encode($a);
$b = json_encode($b);
}

return $a < $b ? -1 : 1;
};

usort($expectedDocuments, $sort);
usort($values, $sort);

$this->assertEquals($expectedDocuments, $values);
}

public function provideTypeMapOptionsAndExpectedDocuments()
{
return [
'No type map' => [
['root' => 'array', 'document' => 'array'],
[
['foo' => 'bar'],
4,
['foo' => ['foo' => 'bar']],
],
],
'array/array' => [
['root' => 'array', 'document' => 'array'],
[
['foo' => 'bar'],
4,
['foo' => ['foo' => 'bar']],
],
],
'object/array' => [
['root' => 'object', 'document' => 'array'],
[
(object) ['foo' => 'bar'],
4,
(object) ['foo' => ['foo' => 'bar']],
],
],
'array/stdClass' => [
['root' => 'array', 'document' => 'stdClass'],
[
['foo' => 'bar'],
4,
['foo' => (object) ['foo' => 'bar']],
],
],
];
}

public function testDrop()
{
$writeResult = $this->collection->insertOne(['x' => 1]);
Expand Down