PHPORM-50 PHPORM-65 Remove call to deprecated Collection::count for countDocuments

[GromNaN]

Fix PHPORM-50 and PHPORM-65

• MongoDB\Collection::count() is deprecated, replaced by MongoDB\Collection:: countDocuments() when an accurate result is expected.
• Fix sending session and other connection options in count call, so that it get the correct result in a transaction.

Suggested by @alcaeus #4 (comment)

[GromNaN]

I updated the PR to use estimatedDocumentCount because countDocuments doesn't work as expected within transactions. Jenssegers\Mongodb\Tests\TransactionTest::testQuery was failing.

[alcaeus]

I updated the PR to use estimatedDocumentCount because countDocuments doesn't work as expected within transactions. Jenssegers\Mongodb\Tests\TransactionTest::testQuery was failing.

For now this works, even though it uses the same logic as count itself. What was the issue with countDocuments in a transaction? It runs an aggregation pipeline to get results, which should be possible in a transaction.

[GromNaN]

In this test:

laravel-mongodb/tests/TransactionTest.php

Lines 297 to 303 in 9d9c7c8

	DB::beginTransaction();
	$count = DB::collection('users')->count();
	$this->assertEquals(0, $count);
	DB::collection('users')->insert(['name' => 'klinson', 'age' => 20, 'title' => 'admin']);
	$count = DB::collection('users')->count();
	$this->assertEquals(1, $count);
	DB::rollBack();

The 2nd call to count() line 301 return 0 with countDocuments, and 1 with count or estimatedDocumentCount.

I could have used ->get()->count() to count document returned, but the behavior of the Query\Builder::count() method would still have changed in transactions.

[GromNaN]

After some tests, I found that none of count and countDocuments has the correct result when used with a filter inside a transaction where document have been inserted.

I added a test that checks the behaviour of Query\Builder::count() with or without transaction.

[jmikola]

This should not be the case. Consider:

<?php

require __DIR__ . '/vendor/autoload.php';

$client = new MongoDB\Client('mongodb://localhost:27070/?replicaSet=rs0');
$collection = $client->test->foo;
$collection->drop();

$session = $client->startSession();

echo "Starting transaction\n";
$session->startTransaction();

printf("countDocuments: %d\n", $collection->countDocuments([], ['session' => $session]));

echo "Inserting two documents\n";
$collection->insertOne(['name' => 'klinson', 'age' => 20, 'title' => 'admin'], ['session' => $session]);
$collection->insertOne(['name' => 'bryan', 'age' => 18, 'title' => 'user'], ['session' => $session]);

printf("countDocuments: %d\n", $collection->countDocuments([], ['session' => $session]));

$filter = ['age' => 20];
printf("countDocuments(%s): %d\n", json_encode($filter), $collection->countDocuments($filter, ['session' => $session]));

echo "Committing transaction\n";
$session->commitTransaction();

printf("count: %d\n", $collection->count([], ['session' => $session]));
printf("countDocuments: %d\n", $collection->countDocuments([], ['session' => $session]));
printf("estimatedDocumentCount: %d\n", $collection->estimatedDocumentCount(['session' => $session]));

printf("count(%s): %d\n", json_encode($filter), $collection->count($filter, ['session' => $session]));
printf("countDocuments(%s): %d\n", json_encode($filter), $collection->countDocuments($filter, ['session' => $session]));

Output:

Starting transaction
countDocuments: 0
Inserting two documents
countDocuments: 2
countDocuments({"age":20}): 1
Committing transaction
count: 2
countDocuments: 2
estimatedDocumentCount: 2
count({"age":20}): 1
countDocuments({"age":20}): 1

Note that per Transactions: Count Operation, the count command cannot be run within a transaction, so your only option is countDocuments() there.

If countDocuments isn't getting an accurate view of the collection, that suggests the library may not be correctly associating it with the transaction (i.e. session may not be passed).

[jmikola]

estimatedDocumentCount is syntactic sugar for executing the count command without a filter option. That allows it to return an estimated result by consulting the collection metadata instead of executing a query.

I'd be hesitant to silently opt users into this behavior through the same query builder syntax. Another consideration is that you cannot run count within a transaction at all (see: Transactions: Count Operation).

IMO we should consistently use countDocuments here.

If we want to add support for estimatedDocumentCount we should invent some alternative syntax for that down the line.

This requires more overhead on the server side, but I think it'd ultimately serve users better since the counts will always be accurate and execute both within an without a transaction. I expect some users may also rely on the query builder's count() method for things like pagination (despite all the arguments against limit/skip-based pagination).

[GromNaN]

Your guess is correct, options where not passed to MongoDB\Collection::countDocuments().

[jmikola]

options where not passed to MongoDB\Collection::countDocuments().

I'm not sure if it's a bug dating back to the original transaction implementation, but that seems like something worth tracking in a separate JIRA ticket for visibility.

[GromNaN]

The options where already missing here: https://github.com/jenssegers/laravel-mongodb/blob/v3.9.5/src/Query/Builder.php#L287

Tracking in PHPORM-65