-
Notifications
You must be signed in to change notification settings - Fork 133
Add support for export API #882
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds support for Meilisearch /export API: new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant SDK as Java SDK
participant Server as Meilisearch Server
User->>SDK: Build ExportRequest (url, apiKey?, payloadSize?, indexes?)
User->>SDK: call export(request)
rect rgb(220,240,255)
SDK->>Server: POST /export (body: ExportRequest JSON)
Server-->>SDK: TaskInfo (enqueued)
end
SDK-->>User: return TaskInfo
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.code-samples.meilisearch.yaml (1)
846-849: Fix builder usage: Lombok builders don't usesetXxx(...).
ExportRequest.builder().setUrl(...)won’t compile. Lombok’s builder uses the field name as the setter, i.e.url(...).Apply this diff:
-export_post_1: |- - ExportRequest request = ExportRequest.builder().setUrl("http://anothermeiliinstance:7070").build(); - client.export(request); +export_post_1: |- + ExportRequest request = ExportRequest.builder().url("http://anothermeiliinstance:7070").build(); + client.export(request);
🧹 Nitpick comments (11)
src/main/java/com/meilisearch/sdk/Client.java (1)
221-231: Add input validation and fix minor Javadoc casing.
- Validate
requestandrequest.urlearly to fail fast on misuse.- Javadoc: “Instances” -> “instances”.
Apply this diff:
- /** - * Triggers the export of documents between Meilisearch Instances. + /** + * Triggers the export of documents between Meilisearch instances. * * @param request Export request parameters * @return Meilisearch API response as TaskInfo * @throws MeilisearchException if an error occurs * @see <a href="https://www.meilisearch.com/docs/reference/api/export">API specification</a> */ public TaskInfo export(ExportRequest request) throws MeilisearchException { - return config.httpClient.post("/export", request, TaskInfo.class); + if (request == null) { + throw new IllegalArgumentException("ExportRequest must not be null"); + } + if (request.getUrl() == null || request.getUrl().isEmpty()) { + throw new IllegalArgumentException("ExportRequest.url must not be null or empty"); + } + return config.httpClient.post("/export", request, TaskInfo.class); }src/test/java/com/meilisearch/integration/ClientTest.java (1)
325-329: Rename local variable for clarity.Rename
snapshottoexportTaskto avoid confusion with snapshot tests.- Task snapshot = client.getTask(task.getTaskUid()); + Task exportTask = client.getTask(task.getTaskUid()); - assertThat(task.getStatus(), is(equalTo(TaskStatus.ENQUEUED))); - assertThat(snapshot.getType(), is(equalTo("export"))); + assertThat(task.getStatus(), is(equalTo(TaskStatus.ENQUEUED))); + assertThat(exportTask.getType(), is(equalTo("export")));src/main/java/com/meilisearch/sdk/ExportIndexFilter.java (2)
15-19: Fix Javadoc class name.This Javadoc refers to ExportRequest; it should reference ExportIndexFilter.
- /** - * Method that returns the JSON String of the ExportRequest + /** + * Method that returns the JSON String of the ExportIndexFilter * - * @return JSON String of the ExportRequest query + * @return JSON String of the ExportIndexFilter */
22-27: Only includeoverrideSettingswhen true (avoid noisy JSON).The current code always emits
"overrideSettings": false. Emit it only when true for cleaner payloads.- JSONObject jsonObject = - new JSONObject() - .putOpt("filter", this.filter) - .putOpt("overrideSettings", this.overrideSettings); + JSONObject jsonObject = new JSONObject(); + if (this.filter != null) { + jsonObject.put("filter", this.filter); + } + if (this.overrideSettings) { + jsonObject.put("overrideSettings", true); + } return jsonObject.toString();src/test/java/com/meilisearch/sdk/ExportRequestTest.java (7)
18-22: Avoid brittle JSON string equality; assert structure via JSONObjectComparing the raw JSON string ties the test to key ordering. Parse and assert fields instead.
- String expected = "{\"overrideSettings\":false}"; - assertThat(filter.toString(), is(equalTo(expected))); + JSONObject json = new JSONObject(filter.toString()); + assertThat(json.getBoolean("overrideSettings"), is(false)); + assertThat(json.has("filter"), is(false)); assertThat(filter.getFilter(), is(nullValue())); assertThat(filter.isOverrideSettings(), is(false));
27-30: Make filter serialization check order-agnosticSame concern: compare JSON structurally to avoid depending on key order.
- String expected = "{\"overrideSettings\":true}"; - assertThat(filter.toString(), is(equalTo(expected))); + JSONObject json = new JSONObject(filter.toString()); + assertThat(json.getBoolean("overrideSettings"), is(true)); + assertThat(json.has("filter"), is(false)); assertThat(filter.isOverrideSettings(), is(true));
35-38: Assert JSON fields instead of exact stringOrder of "filter" and "overrideSettings" should not matter; assert keys/values.
- String expected = "{\"filter\":\"status = 'active'\",\"overrideSettings\":false}"; - assertThat(filter.toString(), is(equalTo(expected))); + JSONObject json = new JSONObject(filter.toString()); + assertThat(json.getString("filter"), is(equalTo("status = 'active'"))); + assertThat(json.getBoolean("overrideSettings"), is(false)); assertThat(filter.getFilter(), is(equalTo("status = 'active'")));
63-69: Replace JSON string equality with structural comparisonJSONObject.toString() can reorder keys. Use JSONObject#similar for semantic equality.
- assertThat(expectedJson.toString(), is(json.toString())); + assertThat(json.similar(expectedJson), is(true));
70-77: Optional: assert absence rather than null for nested 'filter'If the intended wire format omits nulls, assert absence to catch accidental inclusion of "filter": null.
- assertThat(starIndex.isNull("filter"), is(true)); + assertThat(starIndex.has("filter"), is(false));If nulls are acceptable/desirable in payloads, keep the current check. Please confirm the expected API contract.
13-16: Clarify parity between toString() and HTTP serializationMost tests validate toString()-produced JSON. Ensure the HTTP client uses the same serialization (key naming, null handling) to avoid divergences between unit tests and actual requests. If a different serializer (e.g., Gson/Jackson) is used, consider adding a serializer-focused test or aligning toString with it.
I can add a test that serializes via the actual HTTP layer to validate the final payload if desired.
45-47: Minor Hamcrest style nit (optional)You can drop either is() or equalTo() for brevity without losing clarity: assertThat(x, is(y)) or assertThat(x, equalTo(y)).
Also applies to: 70-73, 95-97
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.code-samples.meilisearch.yaml(1 hunks)src/main/java/com/meilisearch/sdk/Client.java(1 hunks)src/main/java/com/meilisearch/sdk/ExportIndexFilter.java(1 hunks)src/main/java/com/meilisearch/sdk/ExportRequest.java(1 hunks)src/test/java/com/meilisearch/integration/ClientTest.java(2 hunks)src/test/java/com/meilisearch/sdk/ExportRequestTest.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/com/meilisearch/sdk/ExportIndexFilter.java (1)
src/main/java/com/meilisearch/sdk/ExportRequest.java (1)
Builder(7-33)
src/main/java/com/meilisearch/sdk/ExportRequest.java (1)
src/main/java/com/meilisearch/sdk/ExportIndexFilter.java (1)
Builder(6-28)
🔇 Additional comments (3)
src/main/java/com/meilisearch/sdk/Client.java (1)
229-231: API wiring looks correct.The endpoint, payload type, and return type are consistent with other Client methods.
src/main/java/com/meilisearch/sdk/ExportRequest.java (1)
13-16: Fields conform to Meilisearch v1.16 Export API specAll request‐body fields match the official spec for Meilisearch v1.16:
• src/main/java/com/meilisearch/sdk/ExportRequest.java
– url: String (required)
– apiKey: String (optional, accepts null)
– payloadSize: String (optional, human-readable with unit)
– indexes: Map<String,ExportIndexFilter> (optional)• src/main/java/com/meilisearch/sdk/ExportIndexFilter.java
– filter: String (optional, null allowed)
– overrideSettings: boolean (optional, defaults to false)No changes are needed.
src/test/java/com/meilisearch/sdk/ExportRequestTest.java (1)
44-49: Good: robust null checks for optional fieldsUsing JSONObject.isNull handles both absent and explicit null cases for apiKey/indexes. This keeps tests resilient across serializer strategies.
| JSONObject jsonObject = | ||
| new JSONObject() | ||
| .put("url", this.url) | ||
| .putOpt("apiKey", this.apiKey) | ||
| .putOpt("payloadSize", this.payloadSize) | ||
| .putOpt("indexes", this.indexes); | ||
| return jsonObject.toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not expose secrets in toString(); redact apiKey.
toString() is frequently logged. Emitting a raw API key is a security risk.
Apply this diff to redact the key:
- JSONObject jsonObject =
- new JSONObject()
- .put("url", this.url)
- .putOpt("apiKey", this.apiKey)
- .putOpt("payloadSize", this.payloadSize)
- .putOpt("indexes", this.indexes);
+ String redactedApiKey = (this.apiKey == null) ? null : "REDACTED";
+ JSONObject jsonObject =
+ new JSONObject()
+ .put("url", this.url)
+ .putOpt("apiKey", redactedApiKey)
+ .putOpt("payloadSize", this.payloadSize)
+ .putOpt("indexes", this.indexes);
return jsonObject.toString();If you need an unredacted JSON string for tests, consider adding a dedicated method like toJson(boolean redactSecrets) and update tests accordingly. I can draft that change and the test updates if you want.
🤖 Prompt for AI Agents
In src/main/java/com/meilisearch/sdk/ExportRequest.java around lines 25 to 31,
the toString() method currently includes the raw apiKey, which is a security
risk. Modify the method to redact the apiKey value by replacing it with a
placeholder like "REDACTED" before returning the JSON string. Alternatively,
create a new method such as toJson(boolean redactSecrets) that conditionally
redacts the apiKey, and update tests to use this method accordingly.
curquiza
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @ernestorbemx
Sorry for the delay
Can you fix the tests?
ExportRequestTest > toStringSimpleExportIndexFilter() FAILED
java.lang.AssertionError:
Expected: is "{\"overrideSettings\":false}"
but: was "{}"
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
at com.meilisearch.sdk.ExportRequestTest.toStringSimpleExportIndexFilter(ExportRequestTest.java:19)
ExportRequestTest > toStringExportIndexFilterWithFilter() FAILED
java.lang.AssertionError:
Expected: is "{\"filter\":\"status = 'active'\",\"overrideSettings\":false}"
but: was "{\"filter\":\"status = 'active'\"}"
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
at com.meilisearch.sdk.ExportRequestTest.toStringExportIndexFilterWithFilter(ExportRequestTest.java:36)
Gradle Test Executor 1 finished executing tests.
|
@curquiza Oops, my bad, forgot to include tests too. It should be passing now. |
curquiza
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank youuu
bors merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.code-samples.meilisearch.yaml (1)
850-854: Verify alignment with the documentation example.Based on the past review comment, this code sample should reflect the example from the Meilisearch documentation. Please confirm that the current implementation matches the reference example linked in the previous review.
What is the structure of the export_post_1 example in the Meilisearch documentation at https://github.com/meilisearch/documentation/blob/f1562b2b98919c440f0fb124a5cfc8d66456e77b/.code-samples.meilisearch.yaml#L1470-L1481?
🧹 Nitpick comments (1)
src/test/java/com/meilisearch/integration/ClientTest.java (1)
316-330: Verify the export target URL in the integration test.The test uses
getMeilisearchHost()as the export target URL, which points to the same instance being tested. In a typical export scenario, you would export to a different Meilisearch instance. Please confirm whether:
- This is intentional for testing purposes (e.g., the export endpoint accepts the same instance as target in test scenarios)
- The export functionality is designed to handle self-exports
- A different URL should be used for a more realistic integration test
Additionally, consider verifying that the export task eventually reaches a terminal state (succeeded or failed) rather than just checking it was enqueued, to ensure the export actually processes the request correctly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.code-samples.meilisearch.yaml(1 hunks)src/test/java/com/meilisearch/integration/ClientTest.java(2 hunks)
🔇 Additional comments (1)
src/test/java/com/meilisearch/integration/ClientTest.java (1)
10-11: LGTM!The new imports for
ExportIndexFilter,ExportRequest,HashMap, andMapare appropriate for the export test functionality being added.Also applies to: 19-20
Pull Request
Related issue
Fixes #877
What does this PR do?
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit