Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
32 changes: 28 additions & 4 deletions src/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -707,10 +707,34 @@ public function delete(string $collection, array $filters = [], int $limit = 1,
*/
public function count(string $collection, array $filters, array $options): int
{
$result = $this->find($collection, $filters, $options);
$list = $result->cursor->firstBatch;

return \count($list);

// Use MongoDB's native count command with the working format instad of running find and count the results
$command = [
self::COMMAND_COUNT => $collection,
'query' => $this->toObject($filters),
];

Comment on lines 708 to +717
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Aug 31, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Align count() with find(): add defaults and cleanFilters()

Make filters/options optional and normalize filters like find() to correctly handle $and/$or/$nor. This prevents malformed queries and keeps API parity.

-    public function count(string $collection, array $filters, array $options): int
+    public function count(string $collection, array $filters = [], array $options = []): int
     {
-    
+        $filters = $this->cleanFilters($filters);
-        // Use MongoDB's native count command with the working format instad of running find and count the results
+        // Use MongoDB's native count command with the working format instead of running find and counting the results
         $command = [
             self::COMMAND_COUNT => $collection,
-            'query' => $this->toObject($filters),
+            'query' => $this->toObject($filters),
         ];
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function count(string $collection, array $filters, array $options): int
{
$result = $this->find($collection, $filters, $options);
$list = $result->cursor->firstBatch;
return \count($list);
// Use MongoDB's native count command with the working format instad of running find and count the results
$command = [
self::COMMAND_COUNT => $collection,
'query' => $this->toObject($filters),
];
public function count(string $collection, array $filters = [], array $options = []): int
{
$filters = $this->cleanFilters($filters);
// Use MongoDB's native count command with the working format instead of running find and counting the results
$command = [
self::COMMAND_COUNT => $collection,
'query' => $this->toObject($filters),
];
🤖 Prompt for AI Agents
In src/Client.php around lines 708 to 716, the count() method currently requires
filters and options and builds the command directly; update it to mirror find()
by making the $filters and $options parameters optional (default to empty
arrays), run the same filter normalization (call the existing
cleanFilters($filters) or equivalent to normalize $and/$or/$nor and other
structures) before converting to an object, and apply the same default options
used by find() (merge $options with defaults) so the MongoDB count command is
built from normalized filters and options to avoid malformed queries and
preserve API parity.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@shimonewman Do we need the cleanFilters since we don't use find anymore?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We still need to apply cleanFilters to the filters to be used by count, same as in find()
Added it back

// Add limit if specified
if (isset($options['limit'])) {
$command['limit'] = (int)$options['limit'];
}

// Add skip if specified
if (isset($options['skip'])) {
$command['skip'] = (int)$options['skip'];
}

// Add maxTimeMS if specified
if (isset($options['maxTimeMS'])) {
$command['maxTimeMS'] = (int)$options['maxTimeMS'];
}

try {
$result = $this->query($command);
return (int)$result;
} catch (Exception $e) {
return 0;
}
Comment on lines +732 to +738
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Don’t swallow exceptions in count(); let them surface or gate by an option

Returning 0 on any error hides real issues and is inconsistent with other methods that throw. Prefer bubbling the exception, or make suppression opt-in (e.g., options['suppressExceptions']).

-        try {
-        $result = $this->query($command);
-          return (int)$result;
-        } catch (Exception $e) {
-            return 0;
-        }
+        if (!empty($options['suppressExceptions'])) {
+            try {
+                return (int) $this->query($command);
+            } catch (Exception $e) {
+                return 0;
+            }
+        }
+        return (int) $this->query($command);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try {
$result = $this->query($command);
return (int)$result;
} catch (Exception $e) {
return 0;
}
if (!empty($options['suppressExceptions'])) {
try {
return (int) $this->query($command);
} catch (Exception $e) {
return 0;
}
}
return (int) $this->query($command);
🤖 Prompt for AI Agents
In src/Client.php around lines 732 to 737, the count() implementation currently
catches all Exceptions and returns 0 which swallows real errors; change it to
either let exceptions bubble (remove the try/catch) so callers receive the
thrown Exception, or implement an options flag (e.g.,
$this->options['suppressExceptions'] or a method parameter) that, when true,
returns 0 on error but otherwise rethrows the caught Exception; ensure any added
flag default preserves current behavior only if intentionally desired and update
the method signature/docblock accordingly.

}

/**
Expand Down
71 changes: 71 additions & 0 deletions tests/MongoTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -261,4 +261,75 @@ public function testUpsert()
self::assertEquals('USA', $documents[1]->country);
self::assertEquals('English', $documents[1]->language);
}

public function testCountMethod()
{

$collectionName = 'count_test';
$this->getDatabase()->createCollection($collectionName);

$documents = [];
for ($i = 1; $i <= 30; $i++) {
$documents[] = [
'name' => "Document {$i}",
'number' => $i,
'category' => 'test',
'created_at' => new \DateTime()
];
}

$this->getDatabase()->insertMany($collectionName, $documents);

$total = $this->getDatabase()->count($collectionName, [], []);
self::assertEquals(30, $total);

// Test count with filter (should be 1 for each specific number)
$total = $this->getDatabase()->count($collectionName, ['number' => 15], []);
self::assertEquals(1, $total);

// Test count with range filter (should be 10 for numbers 1-10)
$total = $this->getDatabase()->count($collectionName, ['number' => ['$lte' => 10]], []);
self::assertEquals(10, $total);

// Test count with limit (should be 5 for first 5 documents)
$total = $this->getDatabase()->count($collectionName, [], ['limit' => 5]);
self::assertEquals(5, $total);

// Test count with filter and limit (should be 3 for first 3 documents with number <= 10)
$total = $this->getDatabase()->count($collectionName, ['number' => ['$lte' => 10]], ['limit' => 3]);
self::assertEquals(3, $total);

// Test aggregation count - total documents
$aggregationResult = $this->getDatabase()->aggregate($collectionName, [
['$count' => 'total']
]);
self::assertEquals(30, $aggregationResult->cursor->firstBatch[0]->total);

// Test aggregation count with filter
$filteredAggregationResult = $this->getDatabase()->aggregate($collectionName, [
['$match' => ['number' => ['$lte' => 10]]],
['$count' => 'total']
]);
self::assertEquals(10, $filteredAggregationResult->cursor->firstBatch[0]->total);

// Test aggregation count with limit
$limitedAggregationResult = $this->getDatabase()->aggregate($collectionName, [
['$limit' => 7],
['$count' => 'total']
]);
self::assertEquals(7, $limitedAggregationResult->cursor->firstBatch[0]->total);

// Test aggregation count with group by
$groupedAggregationResult = $this->getDatabase()->aggregate($collectionName, [
['$group' => [
'_id' => '$category', // Group by category
'count' => ['$sum' => 1] // Count of documents in the group
]]
]);
self::assertEquals(30, $groupedAggregationResult->cursor->firstBatch[0]->count);
self::assertEquals('test', $groupedAggregationResult->cursor->firstBatch[0]->_id);


$this->getDatabase()->dropCollection($collectionName);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Loading