Add typed Stats object
#823
Conversation
norkunas
added
enhancement
WalkthroughAdds two final readonly value objects, Changes
Sequence Diagram(s)sequenceDiagram
participant App as Client/Application
participant Handler as HandlesSystem::stats()
participant API as Meilisearch API
participant Stats as Stats (fromArray)
participant Index as IndexStats (fromArray)
App->>Handler: stats()
activate Handler
Handler->>API: GET /stats
activate API
API-->>Handler: raw array
deactivate API
alt response is array
Handler->>Stats: Stats::fromArray(array)
activate Stats
Stats->>Index: IndexStats::fromArray(...) for each index
activate Index
Index-->>Stats: IndexStats instances
deactivate Index
Stats-->>Handler: Stats instance
deactivate Stats
Handler-->>App: Stats object
else response invalid
Handler-->>App: throws LogicException
end
deactivate Handler
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧬 Code graph analysis (2)src/Contracts/IndexStats.php (1)
src/Contracts/Stats.php (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #823 +/- ##
==========================================
- Coverage 89.78% 87.97% -1.82%
==========================================
Files 59 80 +21
Lines 1449 1721 +272
==========================================
+ Hits 1301 1514 +213
- Misses 148 207 +59 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/Contracts/IndexStats.php(1 hunks)src/Contracts/Stats.php(1 hunks)src/Endpoints/Delegates/HandlesSystem.php(2 hunks)tests/Contracts/StatsTest.php(1 hunks)tests/Endpoints/ClientTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-17T06:57:43.914Z
Learnt from: norkunas
Repo: meilisearch/meilisearch-php PR: 735
File: src/Contracts/SimilarDocumentsQuery.php:10-12
Timestamp: 2025-11-17T06:57:43.914Z
Learning: In the meilisearch-php project, docblock type annotations (such as `non-empty-string`, `positive-int`, etc.) are treated as contracts for static analysis tools rather than requiring runtime validation. If users violate these documented constraints, it's their responsibility.
Applied to files:
src/Contracts/IndexStats.php
🧬 Code graph analysis (5)
src/Endpoints/Delegates/HandlesSystem.php (3)
src/Contracts/Stats.php (2)
Stats(7-68)fromArray(59-67)src/Exceptions/LogicException.php (1)
LogicException(7-9)src/Contracts/Endpoint.php (1)
show(19-22)
tests/Endpoints/ClientTest.php (7)
tests/TestCase.php (2)
createEmptyIndex(131-136)safeIndexName(138-141)src/Endpoints/Delegates/HandlesDocuments.php (1)
addDocuments(46-49)src/Endpoints/Delegates/HandlesSystem.php (2)
stats(43-52)health(22-25)src/Contracts/Stats.php (3)
getDatabaseSize(25-28)getUsedDatabaseSize(33-36)getIndexes(46-49)src/Contracts/IndexStats.php (2)
getNumberOfDocuments(31-34)getFieldDistribution(76-79)src/Client.php (1)
Client(34-72)src/Exceptions/ApiException.php (1)
ApiException(9-95)
tests/Contracts/StatsTest.php (2)
src/Contracts/IndexStats.php (3)
IndexStats(7-104)isIndexing(52-55)fromArray(92-103)src/Contracts/Stats.php (6)
Stats(7-68)getDatabaseSize(25-28)getUsedDatabaseSize(33-36)getLastUpdate(38-41)getIndexes(46-49)fromArray(59-67)
src/Contracts/Stats.php (1)
src/Contracts/IndexStats.php (3)
__construct(17-26)fromArray(92-103)IndexStats(7-104)
src/Contracts/IndexStats.php (1)
src/Contracts/Stats.php (2)
__construct(14-20)fromArray(59-67)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Seer Code Review
🔇 Additional comments (6)
src/Contracts/IndexStats.php (1)
7-104: LGTM!The IndexStats value object is well-structured with proper readonly properties, getters, and a factory method. The implementation follows PHP best practices and provides type safety through constructor property promotion.
src/Endpoints/Delegates/HandlesSystem.php (1)
43-52: LGTM - Validation ensures type safety.The validation correctly guards against both null and non-array responses from the API. This is stricter than other methods in the trait (e.g.,
health()returns?arraydirectly), but aligns with the breaking-change nature of this PR to enforce typed returns.tests/Contracts/StatsTest.php (1)
13-71: LGTM!The tests provide good coverage for both direct construction and the
fromArrayfactory method, including validation of nestedIndexStatsobjects.tests/Endpoints/ClientTest.php (2)
211-235: LGTM!The test correctly validates the new typed Stats API by creating an index, adding documents, and asserting on the structured Stats object properties. The dynamic lookup for the index by prefix is appropriate given the randomized index names.
237-256: LGTM!The test reorganization improves clarity by establishing the exception expectation after client creation in
testBadClientUrl, and by adding a health check before the stats call intestHeaderWithoutApiKey.src/Contracts/Stats.php (1)
7-68: LGTM!The Stats value object is well-implemented with proper readonly properties, getters, and a factory method. The
fromArraymethod correctly handles nullablelastUpdateby converting it toDateTimeImmutablewhen present, and properly maps nested index data throughIndexStats::fromArray.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/Contracts/Stats.php (2)
7-20: LGTM!The class structure follows best practices for value objects with readonly properties and clear type constraints in PHPDoc for static analysis.
Consider adding runtime validation in the constructor to enforce non-negative constraints on
databaseSizeandusedDatabaseSize:public function __construct( private readonly int $databaseSize, private readonly int $usedDatabaseSize, private readonly ?\DateTimeImmutable $lastUpdate, private readonly array $indexes, ) { + if ($databaseSize < 0 || $usedDatabaseSize < 0) { + throw new \InvalidArgumentException('Database sizes must be non-negative'); + } }This would catch invalid data at construction time rather than relying solely on PHPDoc annotations. However, this is optional and may not be necessary if the API always returns valid data.
59-67: LGTM!The factory method correctly converts API response data into typed objects. The DateTimeImmutable conversion and IndexStats mapping logic is sound.
Consider adding defensive validation to handle malformed API responses more gracefully:
public static function fromArray(array $data): self { + if (!isset($data['databaseSize'], $data['usedDatabaseSize'], $data['indexes'])) { + throw new \InvalidArgumentException('Missing required keys in stats data'); + } + + if (!is_array($data['indexes'])) { + throw new \InvalidArgumentException('Indexes must be an array'); + } + return new self( $data['databaseSize'], $data['usedDatabaseSize'], null !== $data['lastUpdate'] ? new \DateTimeImmutable($data['lastUpdate']) : null, array_map(static fn (array $v) => IndexStats::fromArray($v), $data['indexes']), ); }This would provide clearer error messages if the API returns unexpected data. However, this is optional and depends on whether you want to fail fast with clear errors or trust the API contract.
src/Contracts/IndexStats.php (1)
92-103: LGTM!The factory method correctly maps array data to constructor parameters. The implementation is straightforward and consistent with
Stats::fromArray().Consider adding defensive validation to match the optional suggestion for
Stats::fromArray():public static function fromArray(array $data): self { + $required = ['numberOfDocuments', 'rawDocumentDbSize', 'avgDocumentSize', + 'isIndexing', 'numberOfEmbeddings', 'numberOfEmbeddedDocuments', + 'fieldDistribution']; + + foreach ($required as $key) { + if (!array_key_exists($key, $data)) { + throw new \InvalidArgumentException("Missing required key: {$key}"); + } + } + return new self( $data['numberOfDocuments'], $data['rawDocumentDbSize'], $data['avgDocumentSize'], $data['isIndexing'], $data['numberOfEmbeddings'], $data['numberOfEmbeddedDocuments'], $data['fieldDistribution'], ); }This would provide clearer error messages for malformed data. However, this is optional and depends on your error handling strategy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/Contracts/IndexStats.php(1 hunks)src/Contracts/Stats.php(1 hunks)src/Endpoints/Delegates/HandlesSystem.php(2 hunks)tests/Contracts/StatsTest.php(1 hunks)tests/Endpoints/ClientTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Contracts/StatsTest.php
🧰 Additional context used
🧬 Code graph analysis (4)
src/Endpoints/Delegates/HandlesSystem.php (2)
src/Contracts/Stats.php (2)
Stats(7-68)fromArray(59-67)src/Exceptions/LogicException.php (1)
LogicException(7-9)
tests/Endpoints/ClientTest.php (3)
src/Endpoints/Indexes.php (1)
stats(240-243)src/Contracts/Stats.php (3)
getDatabaseSize(25-28)getUsedDatabaseSize(33-36)getIndexes(46-49)src/Contracts/IndexStats.php (2)
getNumberOfDocuments(31-34)getFieldDistribution(76-79)
src/Contracts/IndexStats.php (1)
src/Contracts/Stats.php (2)
__construct(14-20)fromArray(59-67)
src/Contracts/Stats.php (1)
src/Contracts/IndexStats.php (3)
__construct(17-26)fromArray(92-103)IndexStats(7-104)
🔇 Additional comments (8)
src/Endpoints/Delegates/HandlesSystem.php (2)
7-7: LGTM!The new imports are necessary to support the typed Stats return value and validation error handling.
Also applies to: 13-13
43-52: LGTM!The implementation correctly converts the raw stats array into a strongly-typed
StatsContractobject. The defensive validation at line 47 is good practice for runtime safety.tests/Endpoints/ClientTest.php (3)
211-235: LGTM!The test properly validates the new typed
StatsAPI by:
- Creating test data with known field distribution
- Verifying database size getters work correctly
- Confirming index stats are accessible via the typed interface
- Asserting the correct document count and field distribution
237-244: LGTM!The reordering improves test clarity by setting up the client before declaring exception expectations.
246-256: LGTM!The reordering improves test flow by clearly separating the successful health check from the expected stats() failure.
src/Contracts/Stats.php (1)
22-49: LGTM!The getter methods correctly expose readonly properties with proper type annotations for static analysis.
src/Contracts/IndexStats.php (2)
7-26: LGTM!The class structure follows the same value object pattern as
Stats, with readonly properties and comprehensive type constraints for static analysis.
28-79: LGTM!The getter methods are correctly implemented with consistent naming conventions (including the conventional
isIndexing()for the boolean property) and proper type annotations.
2 participants
Pull Request
Related issue
Fixes #744
What does this PR do?
PR checklist
Please check if your PR fulfills the following requirements:
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.