Add method to get task documents#516
Open
claudeaceae wants to merge 2 commits intomeilisearch:mainfrom
Open
Conversation
Implements the endpoint introduced in Meilisearch v1.13 for retrieving documents processed by a given task. Handles NDJSON response format by parsing each line as a separate JSON document. Closes meilisearch#513 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdded support for retrieving documents for a task via Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Tasks as Tasks
participant HTTP as HTTP Request
participant Decoder as NDJSON Decoder
participant Continuation as Async Continuation
Client->>Tasks: getTaskDocuments(taskUid, completion)
Tasks->>HTTP: GET /tasks/{taskUid}/documents
HTTP-->>Tasks: NDJSON payload (raw Data)
Tasks->>Decoder: decodeNDJSON(data)
Decoder->>Decoder: split lines & decode each JSON line
Decoder-->>Tasks: [T] (decoded documents)
Tasks->>Client: completion(.success([T]))
Client->>Client: async overload (withCheckedThrowingContinuation)
Client->>Tasks: getTaskDocuments(taskUid, completion) via continuation bridge
Tasks-->>Continuation: resume(return or throw)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Sources/MeiliSearch/Tasks.swift (1)
165-177: Consider a lower-allocation NDJSON decode path.Current decoding is correct, but converting
Data -> String -> Data per lineadds avoidable copies for large task outputs. Splitting raw bytes by newline and decoding each chunk would be leaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/MeiliSearch/Tasks.swift` around lines 165 - 177, The current decodeNDJSON converts Data -> String -> Data per line causing extra allocations; change it to split the raw Data by newline bytes and decode each Data slice directly. In decodeNDJSON<T: Decodable>, remove the String conversion and use Data's byte-splitting (e.g., data.split(separator: 0x0A)) to get per-line Data chunks, skip empty chunks, optionally trim a trailing CR (0x0D) from each chunk, and pass each chunk directly to Constants.customJSONDecoder.decode(T.self, from: chunk); keep throwing MeiliSearch.Error.invalidJSON on any non-UTF8/invalid chunk paths as before.Tests/MeiliSearchUnitTests/TasksTests.swift (1)
41-51: Please add one malformed-NDJSON test case.You now cover happy and empty paths; adding a malformed line case would lock down error behavior for the new parser.
Example test shape
+ func testGetTaskDocumentsMalformedResponse() async { + session.pushData(""" + {"id":1} + not-json + """) + + struct SimpleDoc: Decodable { let id: Int } + + do { + _ = try await self.client.getTaskDocuments(taskUid: 1) as [SimpleDoc] + XCTFail("Expected decoding to fail") + } catch { + XCTAssertNotNil(error) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/MeiliSearchUnitTests/TasksTests.swift` around lines 41 - 51, Add a new async test (e.g., testGetTaskDocumentsMalformedNDJSON) mirroring testGetTaskDocumentsEmptyResponse but push a malformed NDJSON payload into the fake session using session.pushData (for example one valid JSON line then a broken line like "{\"id\":1}\n{\"id\":}" or a non-JSON line), declare the same SimpleDoc struct used by testGetTaskDocumentsEmptyResponse, then call try await self.client.getTaskDocuments(taskUid: 1) and assert that it throws (use XCTAssertThrowsError or await XCTAssertThrowsError) and optionally verify the thrown error is a DecodingError; reference client.getTaskDocuments and session.pushData when locating where to add the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/MeiliSearch/Tasks.swift`:
- Around line 165-177: The current decodeNDJSON converts Data -> String -> Data
per line causing extra allocations; change it to split the raw Data by newline
bytes and decode each Data slice directly. In decodeNDJSON<T: Decodable>, remove
the String conversion and use Data's byte-splitting (e.g., data.split(separator:
0x0A)) to get per-line Data chunks, skip empty chunks, optionally trim a
trailing CR (0x0D) from each chunk, and pass each chunk directly to
Constants.customJSONDecoder.decode(T.self, from: chunk); keep throwing
MeiliSearch.Error.invalidJSON on any non-UTF8/invalid chunk paths as before.
In `@Tests/MeiliSearchUnitTests/TasksTests.swift`:
- Around line 41-51: Add a new async test (e.g.,
testGetTaskDocumentsMalformedNDJSON) mirroring testGetTaskDocumentsEmptyResponse
but push a malformed NDJSON payload into the fake session using session.pushData
(for example one valid JSON line then a broken line like "{\"id\":1}\n{\"id\":}"
or a non-JSON line), declare the same SimpleDoc struct used by
testGetTaskDocumentsEmptyResponse, then call try await
self.client.getTaskDocuments(taskUid: 1) and assert that it throws (use
XCTAssertThrowsError or await XCTAssertThrowsError) and optionally verify the
thrown error is a DecodingError; reference client.getTaskDocuments and
session.pushData when locating where to add the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c51b0333-da7c-40a8-8c9b-d82cdeecca46
📒 Files selected for processing (5)
.code-samples.meilisearch.yamlSources/MeiliSearch/Async/Client+async.swiftSources/MeiliSearch/Client.swiftSources/MeiliSearch/Tasks.swiftTests/MeiliSearchUnitTests/TasksTests.swift
Split raw Data bytes by newline instead of converting Data -> String -> Data per line, reducing allocations for large task outputs. Also handles \r\n line endings. Adds test case for malformed NDJSON input. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Author
|
Thanks for the review! Both nitpicks addressed in eaf2a60:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Tests/MeiliSearchUnitTests/TasksTests.swift (1)
53-67: Minor: Redundant assertion in catch block.The
XCTAssertNotNil(error)assertion on line 65 is unnecessary—errorwithin acatchblock is guaranteed to be non-nil. The test already proves an error occurred by reaching the catch block without hittingXCTFail.Consider removing the redundant assertion or, if you want stronger validation, assert on a specific error type (e.g.,
DecodingError).♻️ Optional: Strengthen error validation or remove redundant assertion
do { let _: [SimpleDoc] = try await self.client.getTaskDocuments(taskUid: 1) XCTFail("Expected decoding to fail") } catch { - XCTAssertNotNil(error) + XCTAssertTrue(error is DecodingError, "Expected DecodingError but got \(type(of: error))") }Or simply remove the assertion if specific error type validation isn't needed:
do { let _: [SimpleDoc] = try await self.client.getTaskDocuments(taskUid: 1) XCTFail("Expected decoding to fail") - } catch { - XCTAssertNotNil(error) - } + } catch { }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/MeiliSearchUnitTests/TasksTests.swift` around lines 53 - 67, The catch block in testGetTaskDocumentsMalformedResponse contains a redundant XCTAssertNotNil(error); remove that assertion or replace it with a stronger check such as asserting the error type (for example XCTAssertTrue(error is DecodingError)) so the test either simply verifies an error was thrown by reaching the catch or validates the error from getTaskDocuments(taskUid:) is a DecodingError; update the testGetTaskDocumentsMalformedResponse function accordingly (referencing the SimpleDoc struct and the call to self.client.getTaskDocuments).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Tests/MeiliSearchUnitTests/TasksTests.swift`:
- Around line 53-67: The catch block in testGetTaskDocumentsMalformedResponse
contains a redundant XCTAssertNotNil(error); remove that assertion or replace it
with a stronger check such as asserting the error type (for example
XCTAssertTrue(error is DecodingError)) so the test either simply verifies an
error was thrown by reaching the catch or validates the error from
getTaskDocuments(taskUid:) is a DecodingError; update the
testGetTaskDocumentsMalformedResponse function accordingly (referencing the
SimpleDoc struct and the call to self.client.getTaskDocuments).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ce28f82b-1038-4511-86de-4289a8db6cc3
📒 Files selected for processing (2)
Sources/MeiliSearch/Tasks.swiftTests/MeiliSearchUnitTests/TasksTests.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/MeiliSearch/Tasks.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getTaskDocumentsmethod implementingGET /tasks/{taskUid}/documents(Meilisearch v1.13)get_task_documents_1Closes #513
Summary by CodeRabbit
New Features
Documentation
Tests