Bump to PHP 8.1, improve type safety for Tasks

[norkunas]

Pull Request

Related issue

Fixes #703

What does this PR do?

• Bumps PHP requirement to 8.1;
• Adds missing types where there were none at all, or was impossible to use because of unions;
• Adds Task object for type safety to be returned instead of undocumented array shapes;
• Fully enables trailing_comma_in_multiline cs fixer rule;
• Disables php_unit_strict cs fixer rule, because it breaks tests;

This introduces a BC break for someone who is extending meilisearch classes, but not for those who consumes the data, as Task object was added implementing \ArrayAccess, so all raw data can still be accessed.

This introduces a BC break for task data access and for someone who is extending meilisearch classes.

Migration guide

Before

$promise = $this->client->createIndex('new-index', ['primaryKey' => 'objectID']);

$this->index->waitForTask($promise['taskUid']);

After

$task = $this->client->createIndex('new-index', ['primaryKey' => 'objectID']);

$this->index->waitForTask($task->getTaskUid());

Before

$task = $this->client->getTask($taskUid);

if ($task['status'] === 'succeeded') {
    // snip...
}

After

use MeiliSearch\Contracts\TaskStatus;

$task = $this->client->getTask($taskUid);

if ($task->getStatus() instanceof TaskStatus::Succeeded) {
    // snip...
}

Before

$task = $this->client->getTask($taskUid);

if ($task['type'] === 'indexCreation') {
    // snip...
}

After

use MeiliSearch\Contracts\TaskType;

$task = $this->client->getTask($taskUid);

if ($task->getType() instanceof TaskType::IndexCreation) {
    // snip...
}

PR checklist

Please check if your PR fulfills the following requirements:

• Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
• Have you read the contributing guidelines?
• Have you made sure that the title is accurate and descriptive of the changes?

Summary by CodeRabbit

• New Features

  • Introduced a Task object with detailed status and type enums, plus multiple task detail classes for enhanced task management and clarity.
  • Added comprehensive tests for the Task object and pagination settings.

• Breaking Changes

  • Raised minimum PHP version requirement to 8.1.
  • Updated many endpoints to return Task objects instead of arrays for asynchronous operations (documents, indexes, settings, dumps, snapshots, tasks).
  • Enforced stricter and more explicit type declarations across the codebase.

• Improvements

  • Enhanced type safety with explicit parameter and return types.
  • Streamlined exception classes with clearer type annotations.
  • Simplified and unified method signatures and return types for better consistency.
  • Updated test code to work with Task objects instead of raw arrays, improving clarity and type safety.

• Bug Fixes

  • Improved reliability and consistency of method signatures and expected outputs.

• Tests

  • Expanded and refined test coverage reflecting new Task object usage.
  • Removed redundant assertions and outdated test methods.
  • Updated tests to use method calls on Task objects rather than array access.

• Chores

  • Updated CI workflows to run tests only on PHP 8.1 and above.
  • Updated coding style configurations and dependency version constraints.

[codecov]

Codecov Report

Attention: Patch coverage is 97.92285% with 7 lines in your changes missing coverage. Please review.

Project coverage is 90.57%. Comparing base (368c73f) to head (80bbcb1).
Report is 70 commits behind head on main.

Files with missing lines	Patch %	Lines
src/Contracts/TaskDetails/DumpCreationDetails.php	0.00%	6 Missing ⚠️
src/Endpoints/Delegates/HandlesTasks.php	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #735      +/-   ##
==========================================
- Coverage   90.74%   90.57%   -0.18%     
==========================================
  Files          52       72      +20     
  Lines        1340     1602     +262     
==========================================
+ Hits         1216     1451     +235     
- Misses        124      151      +27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tacman]

YAY!

If these are to be deprecated, are they going to be replaced with direct access to the object?

    public function getData(): array
    {
        return $this->data;
    }

    // @todo: deprecate
    public function offsetExists(mixed $offset): bool
    {
        return \array_key_exists($offset, $this->data);
    }

    // @todo: deprecate
    public function offsetGet(mixed $offset): mixed
    {
        return $this->data[$offset] ?? null;
    }

    // @todo: deprecate
    public function offsetSet(mixed $offset, mixed $value): void
    {
        throw new \LogicException(\sprintf('Setting data on "%s::%s" is not supported.', get_debug_type($this), $offset));
    }

    // @todo: deprecate
    public function offsetUnset(mixed $offset): void
    {
        throw new \LogicException(\sprintf('Unsetting data on "%s::%s" is not supported.', get_debug_type($this), $offset));
    }

[norkunas]

@tacman if this PR will get merged, then you'll get access to both ways: $task->getError() or $task['error']
but imho previous way should be deprecated, because it does not provide types

[tacman]

That makes sense. I imagine there will be other things in 2.0 as well.

[Strift]

Hi, thanks for your PR.

Do we want to release this in a v2.x?

Can you add a "migration guide" to your first comment to highlight the required implementation changes?

[norkunas]

@Strift

Do we want to release this in a v2.x?

Based on semver, it should be in v2.x, but if you want to be in line with meilisearch/meilisearch repository, I think it's reasonable to release this in v1.

Can you add a "migration guide" to your first comment to highlight the required implementation changes?

Can you link to some previous comment where I could see how this looks like?

[Strift]

Hi @norkunas, no need to follow a specific format. A simple before/after of the relevant parts of the code is enough. Here's an example for meilisearch JS: https://github.com/meilisearch/meilisearch-js/releases/tag/v0.48.0

It's mainly so I have examples to refer to when writing the release changelog. Double-checking with the actual code changes is better, but the code samples are a good resource too for me, especially if I write the changelog a long time after reviewing the PR.

[norkunas]

@Strift I've updated description, is it ok for you?
Also it should be decided if we deprecate the array access or not.

[norkunas]

I also have a side idea for another improvement:
Add wait(int $timeoutInMs, int $intervalInMs) method to task.
So instead of:

$task = $this->client->createIndex('new-index', ['primaryKey' => 'objectID']);
$task = $this->index->waitForTask($task->getTaskUid());

we could write:

$task = $this->client->createIndex('new-index', ['primaryKey' => 'objectID'])->wait();

But not sure, as then we'd need for this method to work at least inject in in the Task object the Meilisearch\Endpoints\Tasks endpoint, or the http client, or the meilisearch client itself..

[curquiza]

Hello here!
Thank you for your PR @norkunas!
With @brunoocasali and the PHP community, we originally chose to make the PHP SDK follow the same version number as Meilisearch's one
Here, if we merge this branch, we will break this useful convention forever. Is it what we want?

[norkunas]

Hi, I'm for merging to v1 :) I think this is an acceptable BC break to provide type safety

[norkunas]

I also think that it would be good to have typed task details based on https://www.meilisearch.com/docs/reference/api/tasks#details, because now it's always array<mixed>, so static analyzers will always complain.

[coderabbitai]

Actionable comments posted: 5

♻️ Duplicate comments (1)

src/Contracts/Task.php (1)

168-168: Guard against missing indexUid key

indexUid can legitimately be absent in Meilisearch task payloads (e.g. system tasks).
Accessing an undefined key will emit a PHP warning in strict mode.

-            $data['indexUid'],
+            $data['indexUid'] ?? null,

Because the constructor already allows ?string, this change is harmless yet makes fromArray() resilient to such payloads.

🧹 Nitpick comments (4)

src/Contracts/TaskDetails/IndexUpdateDetails.php (1)

17-18: Comment refers to “index creation” in an update class

Minor wording inconsistency can confuse maintainers. Consider s/creation/update/ to reflect the class purpose.

src/Contracts/Task.php (3)

177-190: Consider using consistent null checking patterns

The code uses three different patterns to check for nulls or missing values:

1. null !== $data['details'] (for task details)
2. isset($data['startedAt']) (for dates)
3. $data['duration'] ?? null (for other properties)

While all approaches work, consistency would improve code readability and maintenance.

---

117-139: Consider adding deprecation notices for ArrayAccess methods

The ArrayAccess methods are marked with @todo: deprecate comments, but users might not notice these TODOs in documentation. Consider implementing actual deprecation notices using trigger_deprecation() to alert users when they use the array access syntax.

// @todo: deprecate
public function offsetGet(mixed $offset): mixed
{
+    trigger_deprecation('meilisearch/meilisearch-php', '1.x', 'Array access on Task objects is deprecated and will be removed in 2.0. Use the getter methods instead.');
    return $this->data[$offset] ?? null;
}

You could limit these notices to non-test environments or implement a flag to silence them if needed.

---

146-163: Consider improving PHPDoc for fromArray parameter

The PHPDoc for the fromArray method is comprehensive, but could be improved for better IDE support and readability:

1. Consider using @phpstan-param or @psalm-param for more precise type information
2. Document which task types expect which detail structures
3. Consider separating the complex array shape into multiple type definitions using PHPDoc templates

Example:

/**
 * @phpstan-type TaskData array{
 *     taskUid?: int,
 *     uid?: int,
 *     indexUid?: non-empty-string,
 *     ...
 * }
 * 
 * @param TaskData $data
 */

This would improve static analysis and code completion for developers extending this code.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite

📥 Commits

Reviewing files that changed from the base of the PR and between b8060d3 and 8d69c87.

📒 Files selected for processing (33)

• src/Contracts/Task.php (1 hunks)
• src/Contracts/TaskDetails.php (1 hunks)
• src/Contracts/TaskDetails/DocumentAdditionOrUpdateDetails.php (1 hunks)
• src/Contracts/TaskDetails/DocumentDeletionDetails.php (1 hunks)
• src/Contracts/TaskDetails/DocumentEditionDetails.php (1 hunks)
• src/Contracts/TaskDetails/DumpCreationDetails.php (1 hunks)
• src/Contracts/TaskDetails/IndexCreationDetails.php (1 hunks)
• src/Contracts/TaskDetails/IndexDeletionDetails.php (1 hunks)
• src/Contracts/TaskDetails/IndexSwapDetails.php (1 hunks)
• src/Contracts/TaskDetails/IndexUpdateDetails.php (1 hunks)
• src/Contracts/TaskDetails/SettingsUpdateDetails.php (1 hunks)
• src/Contracts/TaskDetails/TaskCancelationDetails.php (1 hunks)
• src/Contracts/TaskDetails/TaskDeletionDetails.php (1 hunks)
• src/Contracts/TaskError.php (1 hunks)
• src/Contracts/TaskStatus.php (1 hunks)
• src/Contracts/TaskType.php (1 hunks)
• src/Endpoints/Delegates/HandlesDocuments.php (8 hunks)
• src/Endpoints/Delegates/HandlesDumps.php (1 hunks)
• src/Endpoints/Delegates/HandlesIndex.php (2 hunks)
• src/Endpoints/Delegates/HandlesSettings.php (20 hunks)
• src/Endpoints/Delegates/HandlesSnapshots.php (1 hunks)
• src/Endpoints/Delegates/HandlesSystem.php (2 hunks)
• src/Endpoints/Delegates/HandlesTasks.php (3 hunks)
• src/Endpoints/Dumps.php (1 hunks)
• src/Endpoints/Indexes.php (5 hunks)
• src/Endpoints/Snapshots.php (1 hunks)
• src/Endpoints/Tasks.php (2 hunks)
• tests/Contracts/TaskTest.php (1 hunks)
• tests/Endpoints/DocumentsTest.php (3 hunks)
• tests/Endpoints/DumpTest.php (1 hunks)
• tests/Endpoints/SnapshotsTest.php (1 hunks)
• tests/MockTask.php (1 hunks)
• tests/Settings/EmbeddersTest.php (3 hunks)

✅ Files skipped from review due to trivial changes (6)

• src/Contracts/TaskDetails/DumpCreationDetails.php
• src/Contracts/TaskDetails/IndexSwapDetails.php
• src/Contracts/TaskDetails/DocumentDeletionDetails.php
• src/Contracts/TaskError.php
• src/Contracts/TaskDetails/IndexDeletionDetails.php
• src/Contracts/TaskDetails/DocumentAdditionOrUpdateDetails.php

🚧 Files skipped from review as they are similar to previous changes (18)

• src/Endpoints/Delegates/HandlesSystem.php
• src/Contracts/TaskStatus.php
• tests/Endpoints/DumpTest.php
• src/Contracts/TaskType.php
• tests/Contracts/TaskTest.php
• src/Endpoints/Dumps.php
• src/Endpoints/Snapshots.php
• tests/Endpoints/SnapshotsTest.php
• tests/MockTask.php
• src/Endpoints/Delegates/HandlesIndex.php
• src/Endpoints/Delegates/HandlesDumps.php
• tests/Settings/EmbeddersTest.php
• src/Endpoints/Tasks.php
• src/Endpoints/Delegates/HandlesSnapshots.php
• src/Endpoints/Delegates/HandlesTasks.php
• src/Endpoints/Delegates/HandlesDocuments.php
• src/Endpoints/Indexes.php
• src/Endpoints/Delegates/HandlesSettings.php

🔇 Additional comments (14)

src/Contracts/TaskDetails.php (1)

10-16: Interface is concise and well-scoped

The generic TaskDetails contract is minimal yet sufficient, and the fromArray(): self factory keeps implementers consistent.
No issues spotted here.

src/Contracts/TaskDetails/IndexCreationDetails.php (1)

17-18: Doc-block wording copy-paste

The description still says “supplied during index creation”, but this is the creation details class itself, so the wording is correct here but later classes reuse it verbatim. Nothing actionable for this file.

src/Contracts/TaskDetails/TaskDeletionDetails.php (1)

1-38: Well-structured immutable data class.

This is a well-implemented immutable class for task deletion details that follows good PHP 8.1 patterns with readonly properties and proper type declarations. The PHPDoc annotations are comprehensive and provide clear context about each property's purpose.

tests/Endpoints/DocumentsTest.php (5)

9-11: Appropriate import updates for the new Task model.

The imports have been correctly updated to support the new Task-based approach, replacing the previous array-based implementation.

Also applies to: 17-17

---

370-377: Correctly updated mock to return Task object.

The mock implementation now returns a Task object instead of void, aligning with the new typed Task implementation. This is a good change to ensure the tests properly validate the expected return types.

---

379-381: Consistent Task object return in mock.

The second case in the mock callback correctly returns a Task object, maintaining consistency with the updated return type expectations.

---

853-860: Updated mock return type for updateDocumentsCsv.

Similar to the previous mocks, this one has been correctly updated to return a Task object instead of the previous array or void return type.

---

862-864: Consistent Task object return in mock.

The second case in the updateDocumentsCsv mock callback correctly returns a Task object, maintaining consistency across the test suite.

src/Contracts/TaskDetails/TaskCancelationDetails.php (1)

1-38: Well-structured immutable data class for task cancellation details.

This class follows the same good implementation patterns as other task details classes, with readonly properties, comprehensive PHPDoc annotations, and a clean factory method. The documentation comments provide helpful context about each property's meaning.

src/Contracts/TaskDetails/SettingsUpdateDetails.php (4)

9-53: Comprehensive type definitions for settings properties.

The PHPDoc annotations provide excellent type safety with detailed nested type definitions for all possible settings properties. This will be very helpful for static analysis tools and IDE auto-completion.

---

56-95: Well-documented constructor parameters.

The parameter annotations maintain the same level of detail as the class PHPDoc, ensuring type consistency throughout the implementation.

---

97-119: Clean constructor implementation with readonly properties.

Excellent use of PHP 8.1 readonly properties to create an immutable value object. All 20 properties are properly typed and declared as nullable where appropriate.

---

121-145: Effective factory method for object creation.

The fromArray method properly handles optional fields with the null coalescing operator, making instantiation from API responses straightforward and reliable.

src/Contracts/Task.php (1)

1-195: Overall excellent implementation of the Task class with strong typing

This is a well-designed immutable Task object with comprehensive type safety. I like the implementation of both getter methods and ArrayAccess compatibility, which will help with migration from the previous array-based approach.

[coderabbitai]

Actionable comments posted: 1

🔭 Outside diff range comments (2)

tests/Endpoints/DocumentsTest.php (2)

835-836: ⚠️ Potential issue

Missed update to Task object usage pattern.

There's an inconsistency here compared to the rest of the changes. This still uses the old array access pattern.

-        $addPromise = $index->addDocumentsCsv($documentCsv);
-        $index->waitForTask($addPromise['taskUid']);
+        $task = $index->addDocumentsCsv($documentCsv);
+        $index->waitForTask($task->getTaskUid());

---

899-900: ⚠️ Potential issue

Another instance of missed Task object update.

This is another spot where the old array access pattern is still being used instead of the new method call pattern.

-        $addPromise = $index->addDocumentsNdjson($documentNdJson);
-        $index->waitForTask($addPromise['taskUid']);
+        $task = $index->addDocumentsNdjson($documentNdJson);
+        $index->waitForTask($task->getTaskUid());

♻️ Duplicate comments (1)

tests/Contracts/TaskTest.php (1)

63-69: Array access test looks good but could be in a dedicated test method.

This test for array access retrocompatibility is important for backward compatibility. While it could be moved to a dedicated test method for better organization, keeping it here is also reasonable as discussed in previous comments.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite

📥 Commits

Reviewing files that changed from the base of the PR and between c1bafab and d6acbbb.

📒 Files selected for processing (35)

• src/Contracts/TasksResults.php (2 hunks)
• tests/Contracts/TaskTest.php (1 hunks)
• tests/Endpoints/BatchesTest.php (1 hunks)
• tests/Endpoints/ClientTest.php (3 hunks)
• tests/Endpoints/DocumentsTest.php (38 hunks)
• tests/Endpoints/FacetSearchTest.php (1 hunks)
• tests/Endpoints/IndexTest.php (6 hunks)
• tests/Endpoints/MultiSearchTest.php (1 hunks)
• tests/Endpoints/SearchNestedFieldsTest.php (4 hunks)
• tests/Endpoints/SearchTest.php (23 hunks)
• tests/Endpoints/SimilarDocumentsTest.php (1 hunks)
• tests/Endpoints/TasksTest.php (4 hunks)
• tests/Endpoints/TenantTokenTest.php (3 hunks)
• tests/Settings/DisplayedAttributesTest.php (2 hunks)
• tests/Settings/DistinctAttributeTest.php (1 hunks)
• tests/Settings/EmbeddersTest.php (4 hunks)
• tests/Settings/FacetSearchTest.php (1 hunks)
• tests/Settings/FacetingAttributesTest.php (1 hunks)
• tests/Settings/FilterableAttributesTest.php (3 hunks)
• tests/Settings/LocalizedAttributesTest.php (2 hunks)
• tests/Settings/NonSeparatorTokensTest.php (1 hunks)
• tests/Settings/PaginationTest.php (1 hunks)
• tests/Settings/PrefixSearchTest.php (1 hunks)
• tests/Settings/ProximityPrecisionTest.php (1 hunks)
• tests/Settings/RankingRulesTest.php (2 hunks)
• tests/Settings/SearchCutoffMsTest.php (1 hunks)
• tests/Settings/SearchableAttributesTest.php (1 hunks)
• tests/Settings/SeparatorTokensTest.php (1 hunks)
• tests/Settings/SettingsTest.php (3 hunks)
• tests/Settings/SortableAttributesTest.php (1 hunks)
• tests/Settings/StopWordsTest.php (1 hunks)
• tests/Settings/SynonymsTest.php (1 hunks)
• tests/Settings/TypoToleranceTest.php (1 hunks)
• tests/Settings/WordDictionaryTest.php (2 hunks)
• tests/TestCase.php (1 hunks)

✅ Files skipped from review due to trivial changes (1)

• tests/Endpoints/SearchTest.php

🚧 Files skipped from review as they are similar to previous changes (24)

• tests/Settings/SearchCutoffMsTest.php
• tests/Settings/DisplayedAttributesTest.php
• tests/Settings/NonSeparatorTokensTest.php
• tests/Settings/SeparatorTokensTest.php
• tests/Settings/SynonymsTest.php
• tests/Settings/ProximityPrecisionTest.php
• tests/Settings/TypoToleranceTest.php
• tests/Settings/SettingsTest.php
• tests/Settings/FacetSearchTest.php
• tests/Settings/FilterableAttributesTest.php
• tests/Settings/WordDictionaryTest.php
• tests/Settings/EmbeddersTest.php
• tests/Settings/PaginationTest.php
• tests/Settings/StopWordsTest.php
• tests/Settings/PrefixSearchTest.php
• tests/Settings/LocalizedAttributesTest.php
• tests/TestCase.php
• tests/Endpoints/TasksTest.php
• tests/Settings/DistinctAttributeTest.php
• tests/Settings/FacetingAttributesTest.php
• tests/Settings/RankingRulesTest.php
• tests/Settings/SearchableAttributesTest.php
• tests/Endpoints/IndexTest.php
• tests/Settings/SortableAttributesTest.php

🧰 Additional context used

🧬 Code Graph Analysis (6)

tests/Endpoints/SimilarDocumentsTest.php (2)

src/Endpoints/Delegates/HandlesTasks.php (1)

• waitForTask (46-49)

src/Contracts/Task.php (1)

• getTaskUid (46-49)

tests/Endpoints/BatchesTest.php (3)

src/Contracts/BatchesQuery.php (1)

• BatchesQuery (9-68)

src/Endpoints/Delegates/TasksQueryTrait.php (1)

• setUids (58-63)

src/Contracts/Task.php (1)

• getTaskUid (46-49)

tests/Endpoints/FacetSearchTest.php (3)

src/Endpoints/Delegates/HandlesSettings.php (1)

• updateFilterableAttributes (247-250)

src/Endpoints/Delegates/HandlesTasks.php (1)

• waitForTask (46-49)

src/Contracts/Task.php (1)

• getTaskUid (46-49)

tests/Endpoints/ClientTest.php (3)

src/Endpoints/Delegates/HandlesIndex.php (5)

• createIndex (56-59)
• updateIndex (64-67)
• index (32-35)
• getIndex (40-43)
• deleteIndex (48-51)

src/Endpoints/Delegates/HandlesTasks.php (1)

• waitForTask (46-49)

src/Contracts/Task.php (2)

• getTaskUid (46-49)
• getIndexUid (54-57)

tests/Endpoints/MultiSearchTest.php (4)

src/Endpoints/Delegates/HandlesDocuments.php (2)

• updateDocuments (105-108)
• addDocumentsCsv (53-56)

src/Endpoints/Delegates/HandlesTasks.php (1)

• waitForTask (46-49)

src/Contracts/Task.php (1)

• getTaskUid (46-49)

tests/TestCase.php (2)

• createEmptyIndex (134-140)
• safeIndexName (142-145)

tests/Endpoints/DocumentsTest.php (4)

src/Contracts/Task.php (2)

• Task (19-195)
• getTaskUid (46-49)

tests/MockTask.php (2)

• MockTask (11-22)
• create (13-21)

tests/TestCase.php (2)

• TestCase (16-175)
• createEmptyIndex (134-140)

src/Endpoints/Delegates/HandlesDocuments.php (18)

• addDocuments (43-46)
• addDocumentsInBatches (66-75)
• addDocumentsCsv (53-56)
• addDocumentsJson (48-51)
• addDocumentsNdjson (58-61)
• updateDocuments (105-108)
• updateDocumentsInBatches (128-137)
• updateDocumentsByFunction (177-180)
• addDocumentsCsvInBatches (80-89)
• addDocumentsNdjsonInBatches (94-103)
• deleteDocument (187-190)
• deleteDocuments (192-205)
• deleteAllDocuments (182-185)
• updateDocumentsJson (110-113)
• updateDocumentsCsv (115-118)
• updateDocumentsNdjson (120-123)
• updateDocumentsCsvInBatches (142-151)
• updateDocumentsNdjsonInBatches (156-165)

🔇 Additional comments (23)

tests/Endpoints/SimilarDocumentsTest.php (1)

26-26: Good update to use the typed Task object methods

This change properly updates the code to use the new getTaskUid() method instead of array access, improving type safety as intended in this PR.

tests/Endpoints/BatchesTest.php (1)

39-39: Type-safe access with Task object methods

The code correctly uses the getTaskUid() method from the new Task object instead of array access, aligning with the goal of improved type safety in this PR.

tests/Endpoints/FacetSearchTest.php (1)

21-22: Well implemented Task object usage

The change correctly switches from array access to using the strongly typed Task object with its getTaskUid() method, improving type safety and code clarity. The variable naming ($task vs likely previous $promise) is also more semantic.

tests/Endpoints/SearchNestedFieldsTest.php (4)

18-19: Good implementation of Task object methods

Well executed conversion from array access to type-safe method access using the new Task object.

---

70-71: Proper Task object method usage

Correctly implements the new Task object pattern with getTaskUid() method.

---

89-90: Consistent Task object pattern implementation

Good consistency in using the new Task object and its method for accessing the task UID.

---

110-114: Well implemented Task object usage for updateSettings

The code correctly uses the Task object's getTaskUid() method when waiting for the settings update task to complete.

tests/Endpoints/MultiSearchTest.php (2)

24-25: Migration to typed Task object

The code has been updated to use the new Task object instead of an array, showing the correct migration pattern where the variable was renamed to $task and its UID is accessed via getTaskUid() method instead of array access. This improves type safety while maintaining the same functionality.

---

33-34: Consistent Task object usage

The addDocumentsCsv() method now returns a Task object, and the code properly uses the getTaskUid() method to access the task identifier, maintaining a consistent pattern across all task-related operations. The change improves type safety and readability.

tests/Endpoints/TenantTokenTest.php (2)

45-46: Proper migration to typed Task object

The code has been updated appropriately to use the new Task object, with the variable renamed from $promise to $task and array access replaced with the type-safe getTaskUid() method.

---

58-59: Consistent Task object usage

The task response handling has been properly updated to use method access instead of array access, maintaining consistency with the new type-safe approach.

tests/Endpoints/ClientTest.php (3)

96-100: Proper migration to Task object for createIndex

The code has been correctly updated to use the new Task object return type for the createIndex method, accessing task properties through methods instead of array keys for improved type safety.

---

121-124: Good use of Task methods

The code correctly uses both getTaskUid() for waiting on the task and getIndexUid() to obtain the index identifier from the task object. This demonstrates proper use of the new typed Task object's API.

---

137-138: Consistent Task object usage

The deleteIndex method now returns a Task object, and the code properly uses the getTaskUid() method to access the task identifier, maintaining a consistent pattern across all index-related operations.

src/Contracts/TasksResults.php (2)

31-31: Improved type safety in results mapping

The constructor now maps each result array to a Task object using Task::fromArray(), ensuring type safety for task results. This is a good implementation that transforms raw array data into strongly-typed objects.

---

40-40: Updated return type annotation

The return type annotation for getResults() has been properly updated to reflect that it now returns an array of Task objects rather than an array of arrays. This change ensures type consistency and improves static analysis capabilities.

tests/Contracts/TaskTest.php (2)

1-116: Test coverage looks comprehensive and well-structured.

The test class thoroughly validates the behavior of the new Task class, including:

• Full and partial initialization
• Getter methods for all properties
• Array access retrocompatibility
• Immutability enforcement

The test cases ensure both the object interface ($task->getTaskUid()) and array access ($task['indexUid']) work correctly, which supports the backward compatibility described in the PR objectives.

---

96-104: Great immutability checks.

Testing the immutability constraints of the Task class is important for ensuring type safety. These checks verify that users can't mistakenly modify task objects through array access.

tests/Endpoints/DocumentsTest.php (5)

7-17: Updated imports reflect the new type-safe architecture.

The imports have been properly updated to include the necessary classes for the new type-safe approach: Task, TaskType, and MockTask.

---

24-27: API usage updated to use the new Task object methods.

The changes from array access ($promise['taskUid']) to method calls ($task->getTaskUid()) improve type safety as intended by the PR.

---

35-40: Variable naming consistently updated for batch operations.

Good job updating both the variable name ($tasks instead of $promises) and the iteration approach to use the Task object methods.

---

376-387: Mock callback updated to return Task objects.

The callback methods have been correctly updated to return Task objects created via MockTask::create() instead of void, maintaining consistency with the production code.

---

871-882: Mock callbacks correctly return Task objects.

The mock callback method has been correctly updated to return Task objects via MockTask::create() with the appropriate task type.

[brunoocasali]

When we introduce a new task type in the engine does will this code break?

[norkunas]

Yes, this lib must be prepared to accept new task types/statuses/task details on new meilisearch releases, same as with other features, that @Strift or others adds when meilisearch gets them :)

[brunoocasali]

But when I mean "break," will the user really see a parsing error or something like this if the type is not supported?

[norkunas]

It will break with a php error.
But if you want we can throw a custom error saying that this lib version is not supported with your meilisearch instance or something..

[brunoocasali]

I'm unsure if this is enough when we decided internally to move Meilisearch (the engine) to v1. One of the requirements for the integrations would be to be able to keep updating the engine no matter which version you have in the SDK.

So far, we've been successful with that promise if I'm not mistaken 😅. Would it be possible to handle it in a way we can redirect the code to a non-typed task type? So if a user updates Meilisearch, it won't break for them, and to get the full capabilities from the SDK, it will just require them to later upgrade the SDK, WDYT?

[norkunas]

Enum is a closed list, it's impossible. Unless making it nullable and then allow to be null if value is unknown

[norkunas]

But any change in api could break this sdk, so..

[norkunas]

One of the requirements for the integrations would be to be able to keep updating the engine no matter which version you have in the SDK.

if that's a requirement, then we can resort back to using strings, which is sad, but that's your decision

[brunoocasali]

@norkunas After further discussing the subject with @curquiza, we decided it would be more prudent to release it as v2 since it introduces a few breaking changes, especially the deprecation of PHP 7.

If that becomes a burden for our users later, we can introduce a table to showcase which SDK version supports which Meilisearch version. In any case, I think that's better, the engine and their SDKs are independent pieces of software that should be able to evolve independently and should remain adherent to semver rules :)

So, since we are doing a v2, feel free to suggest other potential breaking changes so we can release it in the same package!

Thanks a lot for the PR by the way, loved it!

CC: @Strift

[norkunas]

@norkunas After further discussing the subject with @curquiza, we decided it would be more prudent to release it as v2 since it introduces a few breaking changes, especially the deprecation of PHP 7.

If that becomes a burden for our users later, we can introduce a table to showcase which SDK version supports which Meilisearch version. In any case, I think that's better, the engine and their SDKs are independent pieces of software that should be able to evolve independently and should remain adherent to semver rules :)

So, since we are doing a v2, feel free to suggest other potential breaking changes so we can release it in the same package!

Thanks a lot for the PR by the way, loved it!

CC: @Strift

Sounds good 😉

[norkunas]

Then I think we can remove array access of task data here?

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 6e40c21 and e27e04e.

📒 Files selected for processing (8)

• phpunit.xml.dist (2 hunks)
• src/Contracts/Task.php (1 hunks)
• src/Contracts/TaskDetails/DocumentAdditionOrUpdateDetails.php (1 hunks)
• src/Contracts/TaskDetails/DocumentDeletionDetails.php (1 hunks)
• src/Contracts/TaskDetails/TaskDeletionDetails.php (1 hunks)
• tests/Contracts/TaskTest.php (1 hunks)
• tests/Endpoints/DocumentsTest.php (38 hunks)
• tests/Endpoints/IndexTest.php (5 hunks)

✅ Files skipped from review due to trivial changes (1)

• phpunit.xml.dist

🚧 Files skipped from review as they are similar to previous changes (5)

• tests/Contracts/TaskTest.php
• src/Contracts/TaskDetails/DocumentDeletionDetails.php
• tests/Endpoints/IndexTest.php
• src/Contracts/TaskDetails/DocumentAdditionOrUpdateDetails.php
• tests/Endpoints/DocumentsTest.php

🧰 Additional context used

🧬 Code Graph Analysis (1)

src/Contracts/Task.php (10)

src/Contracts/TaskDetails/DocumentAdditionOrUpdateDetails.php (3)

• DocumentAdditionOrUpdateDetails (15-34)
• __construct (21-25)
• fromArray (27-33)

src/Contracts/TaskDetails/DocumentDeletionDetails.php (3)

• DocumentDeletionDetails (16-38)
• __construct (23-28)
• fromArray (30-37)

src/Contracts/TaskDetails/DocumentEditionDetails.php (3)

• DocumentEditionDetails (18-42)
• __construct (23-30)
• fromArray (32-41)

src/Contracts/TaskDetails/IndexCreationDetails.php (3)

• IndexCreationDetails (14-30)
• __construct (19-22)
• fromArray (24-29)

src/Contracts/TaskDetails/IndexDeletionDetails.php (3)

• IndexDeletionDetails (14-30)
• __construct (19-22)
• fromArray (24-29)

src/Contracts/TaskDetails/IndexSwapDetails.php (3)

• IndexSwapDetails (14-30)
• __construct (19-22)
• fromArray (24-29)

src/Contracts/TaskDetails/IndexUpdateDetails.php (3)

• IndexUpdateDetails (14-30)
• __construct (19-22)
• fromArray (24-29)

src/Contracts/TaskDetails/SettingsUpdateDetails.php (3)

• SettingsUpdateDetails (54-146)
• __construct (97-119)
• fromArray (121-145)

src/Contracts/TaskError.php (3)

• __construct (15-21)
• TaskError (7-40)
• fromArray (31-39)

src/Contracts/TaskDetails.php (1)

• fromArray (15-15)

🔇 Additional comments (11)

src/Contracts/TaskDetails/TaskDeletionDetails.php (3)

9-16: Well-structured type annotation for TaskDetails implementation

The type annotation clearly documents the expected array structure for task deletion details, which enhances code clarity and helps with static analysis.

---

18-22: Thorough documentation of property parameters

The PHPDoc annotations provide excellent context about each parameter, including their meaning and potential null states, which helps developers understand the data model.

---

23-28: Constructor with readonly properties ensures immutability

The use of readonly properties (PHP 8.1 feature) ensures that task details remain immutable after creation, which is appropriate for this data transfer object pattern.

src/Contracts/Task.php (8)

21-38: Well-designed immutable task model with comprehensive type annotations

The Task class is designed as an immutable data container with clear type definitions for each property. The use of readonly properties from PHP 8.1 ensures that task objects cannot be modified after creation, which helps prevent bugs related to state mutation.

---

41-105: Comprehensive getter methods provide type-safe access to task properties

The class provides getter methods for all properties with appropriate return type declarations, which enhances type safety and IDE autocomplete support.

---

107-110: Utility method for checking task completion state

The isFinished() method provides a convenient way to check if a task is no longer in progress, which simplifies client code that needs to wait for task completion.

---

112-128: Thorough documentation of the expected array structure

The PHPDoc for fromArray provides detailed type information about the expected input array, which helps with static analysis and serves as documentation for developers.

---

129-143: Safe handling of optional fields with null coalescing operator

The method properly handles optional fields with the null coalescing operator, including the fix for indexUid that was identified in a previous review.

---

156-158: Good documentation for special case handling

The comment explaining why SnapshotCreation tasks don't have details is helpful for future maintainers, as requested in a previous review.

---

160-161: Safe error handling with existence check

Using array_key_exists before accessing the error data ensures the code won't generate warnings when the error key is missing from the response.

---

144-159:

❌ Incorrect review comment

Consider adding a default case to handle unknown task types

While the current implementation covers all known task types, it doesn't have a fallback for handling new task types that might be introduced in future Meilisearch versions.

---

🏁 Script executed:

#!/bin/bash
# Look for relevant code that might handle unknown task types elsewhere

echo "Checking TaskType enum for extensibility:"
cat src/Contracts/TaskType.php || echo "File not found"

echo "\nChecking for error handling when encountering unknown task types:"
rg -i "unknown task type" --type php

Length of output: 934

---

We need to inspect the surrounding context to confirm if an explicit default branch or a graceful failure is already implemented elsewhere in Task::fromArray(). Let’s pull a few more lines around the match to see how unhandled cases are treated:

---

🏁 Script executed:

#!/bin/bash
# Show lines 130–180 to get surrounding context of the match statement
sed -n '130,180p' src/Contracts/Task.php

Length of output: 2430

---

No default needed—match is already exhaustive and safer without a fallback

PHP 8.1 enums combined with a match ensure that all TaskType cases are handled, and if a new case is ever added (or if an invalid value is passed), you’ll get a clear exception (ValueError from TaskType::from() or UnhandledMatchError), rather than silently falling back. Introducing a default arm would hide unhandled cases.

Likely an incorrect or invalid review comment.

[brunoocasali]

Then I think we can remove array access of task data here?

If you think we have more benefits by removing it than by keeping, I agree in removing it.