feat: add getTaskDocuments method for Meilisearch v1.13#956
Closed
ashishvwl wants to merge 6 commits intomeilisearch:mainfrom
Closed
feat: add getTaskDocuments method for Meilisearch v1.13#956ashishvwl wants to merge 6 commits intomeilisearch:mainfrom
ashishvwl wants to merge 6 commits intomeilisearch:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new SDK method to fetch NDJSON documents for a specific async task: public Client.getTaskDocuments(int). Implementation delegates to TasksHandler which issues GET /tasks/{taskUid}/documents. Includes an integration test and a documentation code sample. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client\nrgba(52,152,219,0.5)
participant Handler as TasksHandler\nrgba(46,204,113,0.5)
participant Server as Meilisearch API\nrgba(231,76,60,0.5)
Client->>Handler: getTaskDocuments(taskUid)
Handler->>Server: GET /tasks/{taskUid}/documents
Server-->>Handler: 200 OK (NDJSON body)
Handler-->>Client: return NDJSON String
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/test/java/com/meilisearch/integration/TasksTest.java (2)
417-419: Several tests are assertion-light and can miss wrong payloads.The current checks (non-null/non-blank only) can still pass if the endpoint returns unexpected content. Please assert task-specific evidence (expected IDs/fields or expected empty result).
Example improvements for stronger assertions
@@ - // Create index task should not have documents - assertThat(taskDocuments, is(notNullValue())); + // Create-index task should not have task documents + List<String> lines = + Arrays.stream(taskDocuments.split("\\R")) + .filter(line -> !line.trim().isEmpty()) + .collect(Collectors.toList()); + assertThat(lines.size(), is(equalTo(0))); @@ - assertThat(taskDocuments, is(notNullValue())); - assertThat(taskDocuments, not(blankOrNullString())); + assertThat(taskDocuments, is(notNullValue())); + assertThat(taskDocuments, not(blankOrNullString())); + assertThat(taskDocuments, containsString("\"id\": 1")); + assertThat(taskDocuments, containsString("\"id\": 2")); + assertThat(taskDocuments, containsString("\"id\": 3"));Also applies to: 438-440, 455-457, 507-509, 537-539
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/meilisearch/integration/TasksTest.java` around lines 417 - 419, The tests in TasksTest use weak assertions like assertThat(taskDocuments, is(notNullValue())) which can miss incorrect payloads; update the checks (e.g., in the blocks handling taskDocuments and similar variables) to assert task-specific evidence such as expected task IDs, status fields, or that the documents list is empty when appropriate (for example assert expected id equals the created task id, assert status equals "succeeded"/"enqueued"/"failed" as applicable, or assert that documents.size() == 0) so each test validates concrete fields/values rather than just non-nullity; apply the same stronger assertions to the other mentioned locations (around the checks at 438-440, 455-457, 507-509, 537-539).
395-405: NDJSON contract check is too permissive.At Line 402 and Line 403, checking only for
{and}can pass non-NDJSON payloads. Please assert non-empty line count and per-line object shape more strictly.Proposed tightening for NDJSON assertions
- // NDJSON format has each object on a separate line - String[] lines = taskDocuments.split("\n"); - assertThat(lines.length, is(greaterThanOrEqualTo(1))); - - // Each line should be a valid JSON object - for (String line : lines) { - if (!line.trim().isEmpty()) { - assertThat(line, containsString("{")); - assertThat(line, containsString("}")); - } - } + // NDJSON format has one JSON object per non-empty line + List<String> lines = + Arrays.stream(taskDocuments.split("\\R")) + .filter(line -> !line.trim().isEmpty()) + .collect(Collectors.toList()); + assertThat(lines.size(), is(equalTo(2))); + + for (String line : lines) { + String trimmed = line.trim(); + assertTrue(trimmed.startsWith("{") && trimmed.endsWith("}")); + assertThat(trimmed, containsString("\"id\"")); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/meilisearch/integration/TasksTest.java` around lines 395 - 405, The current NDJSON check in TasksTest uses loose string checks on taskDocuments lines; instead parse and validate each non-blank line as JSON and assert it's an object. Update the loop that iterates over String[] lines (from taskDocuments) to trim and skip empty lines, then use your JSON parser (e.g., Jackson's ObjectMapper.readTree) to parse each line and assert the resulting JsonNode.isObject() (and optionally check for expected keys like "taskUid" or "indexUid"); also ensure the top-level lines.length assertion verifies at least one non-empty line. This replaces the containsString("{")/("}") checks with strict JSON-object validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/test/java/com/meilisearch/integration/TasksTest.java`:
- Line 515: Replace the hardcoded 999999 with a deterministic impossible UID
constant to avoid flakiness: add a private static final long INVALID_TASK_UID =
Long.MAX_VALUE (or similarly impossible value) in TasksTest and change the
assertion to assertThrows(MeilisearchException.class, () ->
client.getTaskDocuments(INVALID_TASK_UID)); this keeps the failing UID stable
across environments and clearly identifies the intent in the test.
---
Nitpick comments:
In `@src/test/java/com/meilisearch/integration/TasksTest.java`:
- Around line 417-419: The tests in TasksTest use weak assertions like
assertThat(taskDocuments, is(notNullValue())) which can miss incorrect payloads;
update the checks (e.g., in the blocks handling taskDocuments and similar
variables) to assert task-specific evidence such as expected task IDs, status
fields, or that the documents list is empty when appropriate (for example assert
expected id equals the created task id, assert status equals
"succeeded"/"enqueued"/"failed" as applicable, or assert that documents.size()
== 0) so each test validates concrete fields/values rather than just
non-nullity; apply the same stronger assertions to the other mentioned locations
(around the checks at 438-440, 455-457, 507-509, 537-539).
- Around line 395-405: The current NDJSON check in TasksTest uses loose string
checks on taskDocuments lines; instead parse and validate each non-blank line as
JSON and assert it's an object. Update the loop that iterates over String[]
lines (from taskDocuments) to trim and skip empty lines, then use your JSON
parser (e.g., Jackson's ObjectMapper.readTree) to parse each line and assert the
resulting JsonNode.isObject() (and optionally check for expected keys like
"taskUid" or "indexUid"); also ensure the top-level lines.length assertion
verifies at least one non-empty line. This replaces the
containsString("{")/("}") checks with strict JSON-object validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f88d4aa7-ec60-4ad3-9ce0-02ee531f5d6c
📒 Files selected for processing (1)
src/test/java/com/meilisearch/integration/TasksTest.java
Strift
requested changes
Mar 30, 2026
Contributor
Strift
left a comment
Strift
left a comment
There was a problem hiding this comment.
Hi @ashishvwl, and thanks for your contribution!
Can you fix the failing CI? I recommend running tests and linter locally before submitting for review.
Author
how to run this integration local ? can you help |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/com/meilisearch/integration/ClientTest.java (1)
396-397: Strengthen assertions to validate actual NDJSON payload content.On Line 396 and Line 397, checking only
notNullValueandcontainsString("\"id\"")is permissive and can hide regressions. Parse the first NDJSON line and assert expected fields (id,title) explicitly.Proposed test hardening
String taskDocuments = client.getTaskDocuments(addTask.getTaskUid()); assertThat(taskDocuments, is(notNullValue())); - assertThat(taskDocuments, containsString("\"id\"")); + String[] ndjsonLines = taskDocuments.strip().split("\\R"); + assertThat(ndjsonLines, is(arrayWithSize(1))); + + JsonObject firstDoc = JsonParser.parseString(ndjsonLines[0]).getAsJsonObject(); + assertThat(firstDoc.get("id").getAsInt(), is(equalTo(1))); + assertThat(firstDoc.get("title").getAsString(), is(equalTo("Document1")));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/meilisearch/integration/ClientTest.java` around lines 396 - 397, The assertions on taskDocuments in ClientTest are too weak; parse the first NDJSON line from the taskDocuments string, deserialize it (e.g., into a Map or a POJO) and assert the presence and expected values of the fields "id" and "title" instead of only checking containsString and notNullValue. Locate the test block around the taskDocuments variable in ClientTest, split the NDJSON by newline, parse the first line, and replace the containsString assertion with explicit assertions that the parsed object contains keys "id" and "title" and that their values match the expected types/values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/test/java/com/meilisearch/integration/ClientTest.java`:
- Around line 396-397: The assertions on taskDocuments in ClientTest are too
weak; parse the first NDJSON line from the taskDocuments string, deserialize it
(e.g., into a Map or a POJO) and assert the presence and expected values of the
fields "id" and "title" instead of only checking containsString and
notNullValue. Locate the test block around the taskDocuments variable in
ClientTest, split the NDJSON by newline, parse the first line, and replace the
containsString assertion with explicit assertions that the parsed object
contains keys "id" and "title" and that their values match the expected
types/values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 95ee1e8d-0a1c-41cc-b9da-2cc8b25f677e
📒 Files selected for processing (1)
src/test/java/com/meilisearch/integration/ClientTest.java
Contributor
|
Please refer to the contributing guide |
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
2 participants
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.
Pull Request
Related issue
Fixes #948
PR check List:
Update API method to allow retrieving a tasks documents
Add test cases for the new method
Add an example under the get_task_documents_1 YAML key in .code-samples.meilisearch.yaml. It should match the example in the documentation.
@Strift can you review this PR
Summary by CodeRabbit
New Features
Documentation
Tests