Add features of meilisearch 1.18 (queryVector and index renaming) #906
Conversation
WalkthroughAdds support for index renaming via a new IndexesHandler.updateIndexUid and Client.updateIndex overload, adds a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant IndexesHandler
participant API as Meilisearch API
Note over Client,API: Index rename via PATCH
Client->>Client: updateIndex("indexA", null, "indexB")
Client->>IndexesHandler: updateIndexUid("indexA","indexB")
IndexesHandler->>API: PATCH /indexes/indexA { "indexUid":"indexB" }
API-->>IndexesHandler: 202 TaskInfo
IndexesHandler-->>Client: TaskInfo
sequenceDiagram
participant SearchAPI
participant GsonHandler as GsonJsonHandler
participant SearchResult
Note over SearchAPI,SearchResult: queryVector mapping
SearchAPI->>GsonHandler: JSON with "queryVector":[0.1,0.2,0.3]
GsonHandler->>SearchResult: Populate fields
SearchResult->>SearchResult: queryVector = [0.1F,0.2F,0.3F]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/main/java/com/meilisearch/sdk/model/SearchResultPaginated.java (1)
26-26: Consider adding test coverage forqueryVectorin paginated resultsThe
queryVectorfield is consistent withSearchResultand will be exposed via Lombok getters, which is good. To avoid regressions, I’d add a small test that deserializes a paginated search response includingqueryVectorand asserts the values, similar toSearchResultQueryVectorTest.src/main/java/com/meilisearch/sdk/model/SwapIndexesParams.java (1)
13-13: Document the semantics of therenameflag on swap-indexes paramsThe
renameflag integrates nicely with the existing chained params and will serialize as"rename": truewhen set. To help users discover it, consider expanding the class-level or method-level docs (and any public docs) to briefly explain that whenrenameis true, the swap also renames the indexes according to Meilisearch 1.18 semantics.src/main/java/com/meilisearch/sdk/IndexesHandler.java (1)
124-136: Index rename handler is correct; consider a small guard for invalid input
updateIndexUidcorrectly constructs a{"indexUid": "<newUid>"}body and PATCHes/indexes/{uid}, matching the intended rename flow and the pattern used byupdatePrimaryKey.You might optionally add a simple precondition (e.g., reject null/blank
indexUid) to fail fast if this method is ever called directly with invalid input, instead of relying solely on theClientoverload to guard it.src/main/java/com/meilisearch/sdk/Client.java (1)
168-175: OverloadedupdateIndexworks; consider clarifying behavior and API shapeThe new overload cleanly routes:
- to
updateIndexUidwhenindexUid != null(rename), and- to
updatePrimaryKeyotherwise (primary key update),so behavior is correct and non-breaking for existing callers.
Two optional improvements to consider:
- Clarify the Javadoc to explicitly describe the precedence (“if
indexUidis non-null, the primary key argument is ignored”) so callers don’t assume both can be updated in one call.- Long term, a dedicated
renameIndex(String uid, String indexUid)entry point could make the API more explicit than a tri-parameter overload that multiplexes two concerns.src/test/java/com/meilisearch/sdk/SearchResultQueryVectorTest.java (1)
12-31: Consider adding edge case tests.The test validates the happy path for
queryVectordeserialization. Consider adding test cases for edge scenarios:
queryVectorfield is absent (should be null or empty)queryVectoris null in JSONqueryVectoris an empty arrayThis ensures robust handling of various API response formats.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.code-samples.meilisearch.yaml(1 hunks)data.ms/VERSION(1 hunks)data.ms/instance-uid(1 hunks)src/main/java/com/meilisearch/sdk/Client.java(2 hunks)src/main/java/com/meilisearch/sdk/IndexesHandler.java(1 hunks)src/main/java/com/meilisearch/sdk/model/SearchResult.java(1 hunks)src/main/java/com/meilisearch/sdk/model/SearchResultPaginated.java(1 hunks)src/main/java/com/meilisearch/sdk/model/SwapIndexesParams.java(1 hunks)src/test/java/com/meilisearch/sdk/IndexRenameTest.java(1 hunks)src/test/java/com/meilisearch/sdk/SearchResultQueryVectorTest.java(1 hunks)src/test/java/com/meilisearch/sdk/SwapIndexRenameTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/test/java/com/meilisearch/sdk/SearchResultQueryVectorTest.java (1)
src/main/java/com/meilisearch/sdk/json/GsonJsonHandler.java (1)
GsonJsonHandler(12-56)
src/test/java/com/meilisearch/sdk/SwapIndexRenameTest.java (1)
src/main/java/com/meilisearch/sdk/http/CustomOkHttpClient.java (1)
CustomOkHttpClient(18-126)
src/test/java/com/meilisearch/sdk/IndexRenameTest.java (1)
src/main/java/com/meilisearch/sdk/http/CustomOkHttpClient.java (1)
CustomOkHttpClient(18-126)
🔇 Additional comments (5)
data.ms/VERSION (1)
1-1: Ensure VERSION value stays in sync with published SDK version
1.26.0looks fine as a data marker, but please double-check it matches the actual SDK version used in packaging (e.g., Maven coordinates, release notes) so tests and metadata don’t drift.data.ms/instance-uid (1)
1-1: UUID fixture looks fineStatic instance UID value is acceptable as a deterministic fixture; no functional issues from this change.
.code-samples.meilisearch.yaml (1)
857-858: Rename sample correctly targets the newupdateIndexoverloadThe
rename_an_index_1snippet usesclient.updateIndex("indexA", null, "indexB");, which cleanly maps to “rename indexA to indexB” while leaving the primary key untouched. This aligns with the new overload semantics and fits well with the surrounding samples.src/main/java/com/meilisearch/sdk/model/SearchResult.java (1)
21-21:queryVectorfield addition is consistent and testableAdding
ArrayList<Float> queryVector;alongside existing response fields is consistent with the current model style and should deserialize correctly given the existing JSON mapping and the dedicated test.src/main/java/com/meilisearch/sdk/Client.java (1)
13-13: Wildcardjava.utilimport is acceptable given multiple usagesSwitching to
import java.util.*;is harmless here, since the class already depends on severaljava.utiltypes (Map, HashMap, Date, TimeZone, UUID). No action needed unless your style guide prefers explicit imports.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/test/java/com/meilisearch/sdk/IndexRenameTest.java (1)
62-80: Optional: align second test’s HTTP assertions with the first.The second test correctly verifies path and payload for different names. For symmetry and slightly stronger coverage, you could also assert the HTTP method and
Authorizationheader as in the first test.- RecordedRequest request = mockServer.takeRequest(); - assertThat(request.getPath(), equalTo("/indexes/oldIndex")); - - String requestBody = request.getBody().readUtf8(); + RecordedRequest request = mockServer.takeRequest(); + assertThat(request.getMethod(), equalTo("PATCH")); + assertThat(request.getPath(), equalTo("/indexes/oldIndex")); + assertThat(request.getHeader("Authorization"), equalTo("Bearer masterKey")); + + String requestBody = request.getBody().readUtf8(); assertThat(requestBody, containsString("\"indexUid\":\"newIndex\""));src/test/java/com/meilisearch/sdk/SwapIndexesRenameTest.java (2)
72-99: Non-rename swap test is good; header assertion is an optional enhancement.The test correctly covers
rename=false, index names, andtaskUid. For parity with the first test, you might also assert theAuthorizationheader, but the current coverage is acceptable.- RecordedRequest request = mockServer.takeRequest(); - assertThat(request.getMethod(), equalTo("POST")); - assertThat(request.getPath(), equalTo("/swap-indexes")); - - String requestBody = request.getBody().readUtf8(); + RecordedRequest request = mockServer.takeRequest(); + assertThat(request.getMethod(), equalTo("POST")); + assertThat(request.getPath(), equalTo("/swap-indexes")); + assertThat(request.getHeader("Authorization"), equalTo("Bearer masterKey")); + + String requestBody = request.getBody().readUtf8(); assertThat(requestBody, containsString("\"rename\":false"));
101-132: Consider slightly strengthening multi-pair swap assertions and reducing JSON-shape brittleness.The multi-pair test correctly verifies both index pairs and that the payload is a JSON array. To make it a bit more robust and explicit you could:
- Trim the body before
startsWith/endsWithso a trailing newline won’t break the test.- Optionally assert both
"rename":trueand"rename":falseare present to ensure per-pair flags are serialized.- String requestBody = request.getBody().readUtf8(); - - assertThat(requestBody, containsString("\"indexA\"")); - assertThat(requestBody, containsString("\"indexB\"")); - assertThat(requestBody, containsString("\"indexC\"")); - assertThat(requestBody, containsString("\"indexD\"")); - assertThat(requestBody, startsWith("[")); - assertThat(requestBody, endsWith("]")); + String requestBody = request.getBody().readUtf8(); + + assertThat(requestBody, containsString("\"indexA\"")); + assertThat(requestBody, containsString("\"indexB\"")); + assertThat(requestBody, containsString("\"indexC\"")); + assertThat(requestBody, containsString("\"indexD\"")); + assertThat(requestBody, containsString("\"rename\":true")); + assertThat(requestBody, containsString("\"rename\":false")); + + String trimmedBody = requestBody.trim(); + assertThat(trimmedBody, startsWith("[")); + assertThat(trimmedBody, endsWith("]"));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
build.gradle(1 hunks)src/test/java/com/meilisearch/sdk/IndexRenameTest.java(1 hunks)src/test/java/com/meilisearch/sdk/SwapIndexesRenameTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- build.gradle
🔇 Additional comments (4)
src/test/java/com/meilisearch/sdk/IndexRenameTest.java (2)
18-38: MockWebServer setup/teardown and Config wiring look correct.Creating a per-test MockWebServer, trimming the trailing slash from the base URL, and shutting the server down in
@AfterEachis clean and consistent; no issues here.
40-60: Rename test cleanly exercisesIndexesHandler.updateIndexUidover HTTP.The test validates:
- Response mapping via
TaskInfo.getTaskUid()- HTTP verb (
PATCH), path (/indexes/indexA), andAuthorizationheader- JSON payload including
"indexUid":"indexB"This gives good coverage of the new rename behavior in the handler.
src/test/java/com/meilisearch/sdk/SwapIndexesRenameTest.java (2)
19-39: Client + MockWebServer wiring is straightforward and consistent.Using
Configwith the trimmed mock base URL and constructing aClientper test, plus proper MockWebServer lifecycle, looks good.
41-70: Swap-with-rename test validates all critical request/response aspects.You’re asserting:
POST /swap-indexesAuthorizationheader- JSON body including
"rename":trueand both index namesTaskInfodeserialization viataskUidThis is solid coverage for the new
rename=truebehavior.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/test/java/com/meilisearch/sdk/IndexRenameTest.java (2)
40-82: Consider consolidating with @ParameterizedTest.Both test methods have identical structure and differ only in the index names and expected taskUid. Using
@ParameterizedTestwith@MethodSourceor@CsvSourcewould reduce duplication and make it easier to add more test cases.Example refactor:
@ParameterizedTest @CsvSource({ "indexA, indexB, 123", "oldIndex, newIndex, 456" }) void testRenameIndex(String oldUid, String newUid, int expectedTaskUid) throws Exception { String responseJson = String.format( "{\"taskUid\":%d,\"indexUid\":\"%s\",\"status\":\"enqueued\",\"type\":\"indexUpdate\",\"enqueuedAt\":\"2024-01-01T00:00:00Z\"}", expectedTaskUid, newUid); mockServer.enqueue(new MockResponse() .setResponseCode(202) .setBody(responseJson) .addHeader("Content-Type", "application/json")); TaskInfo result = handler.updateIndexUid(oldUid, newUid); assertThat(result, is(notNullValue())); assertThat(result.getTaskUid(), is(equalTo(expectedTaskUid))); RecordedRequest request = mockServer.takeRequest(); assertThat(request.getMethod(), equalTo("PATCH")); assertThat(request.getPath(), equalTo("/indexes/" + oldUid)); assertThat(request.getHeader("Authorization"), equalTo("Bearer masterKey")); String requestBody = request.getBody().readUtf8(); assertThat(requestBody, containsString("\"indexUid\":\"" + newUid + "\"")); }
40-82: Consider adding error case tests.The tests cover happy path scenarios well but don't verify error handling. Consider adding tests for:
- 404 response when the source index doesn't exist
- 400 response for invalid index names
- 409 response when the target index name already exists
- Null or empty string handling
Example error case test:
@Test void testRenameIndexNotFound() throws Exception { String errorJson = "{\"message\":\"Index `nonexistent` not found.\",\"code\":\"index_not_found\",\"type\":\"invalid_request\",\"link\":\"https://docs.meilisearch.com/errors#index_not_found\"}"; mockServer.enqueue(new MockResponse() .setResponseCode(404) .setBody(errorJson) .addHeader("Content-Type", "application/json")); // Assert that appropriate exception is thrown assertThrows(MeilisearchApiException.class, () -> { handler.updateIndexUid("nonexistent", "newName"); }); }src/test/java/com/meilisearch/sdk/SwapIndexesRenameTest.java (2)
41-100: Consider consolidating with @ParameterizedTest.The first two test methods have nearly identical structure and differ only in the
renameflag value and expected taskUid. Using@ParameterizedTestwould reduce duplication.Example refactor:
@ParameterizedTest @CsvSource({ "true, indexA, indexB, 789", "false, indexC, indexD, 790" }) void testSwapIndexesWithRenameFlag(boolean rename, String indexA, String indexB, int expectedTaskUid) throws Exception { String responseJson = String.format( "{\"taskUid\":%d,\"status\":\"enqueued\",\"type\":\"indexSwap\",\"enqueuedAt\":\"2024-01-01T00:00:00Z\"}", expectedTaskUid); mockServer.enqueue(new MockResponse() .setResponseCode(202) .setBody(responseJson) .addHeader("Content-Type", "application/json")); SwapIndexesParams[] params = { new SwapIndexesParams() .setIndexes(new String[]{indexA, indexB}) .setRename(rename) }; TaskInfo result = client.swapIndexes(params); assertThat(result, is(notNullValue())); assertThat(result.getTaskUid(), is(equalTo(expectedTaskUid))); RecordedRequest request = mockServer.takeRequest(); assertThat(request.getMethod(), equalTo("POST")); assertThat(request.getPath(), equalTo("/swap-indexes")); String requestBody = request.getBody().readUtf8(); assertThat(requestBody, containsString("\"rename\":" + rename)); assertThat(requestBody, containsString(indexA)); assertThat(requestBody, containsString(indexB)); }
41-137: Consider adding error and edge case tests.The tests cover happy path scenarios effectively but don't verify error handling or edge cases. Consider adding tests for:
- 404 response when one of the indexes doesn't exist
- 400 response for invalid payloads (e.g., empty
indexesarray, single index, mismatched array lengths)- Empty
SwapIndexesParams[]array- Null values in the params
Example edge case test:
@Test void testSwapIndexesWithEmptyArray() throws Exception { SwapIndexesParams[] params = {}; // Verify behavior - should it throw an exception or send empty array? // This depends on the API specification } @Test void testSwapIndexesNotFound() throws Exception { String errorJson = "{\"message\":\"Index `nonexistent` not found.\",\"code\":\"index_not_found\",\"type\":\"invalid_request\"}"; mockServer.enqueue(new MockResponse() .setResponseCode(404) .setBody(errorJson) .addHeader("Content-Type", "application/json")); SwapIndexesParams[] params = { new SwapIndexesParams() .setIndexes(new String[]{"nonexistent", "indexB"}) .setRename(true) }; assertThrows(MeilisearchApiException.class, () -> { client.swapIndexes(params); }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/test/java/com/meilisearch/sdk/IndexRenameTest.java(1 hunks)src/test/java/com/meilisearch/sdk/SwapIndexesRenameTest.java(1 hunks)
🔇 Additional comments (1)
src/test/java/com/meilisearch/sdk/SwapIndexesRenameTest.java (1)
102-137: Excellent test coverage for multiple index pairs.This test effectively validates that multiple swap operations are serialized correctly as a JSON array. The array structure validation (lines 134-136) is particularly valuable, ensuring the request body is properly formatted as an array rather than individual objects.
curquiza
changed the title
There was a problem hiding this comment.
Hey @yamiSukehiro2907 can you remove the data.ms?
Also, can you fix the tests in the CI?
Thanks for your pR
2 participants
Fixes #888
🚀 Meilisearch Java SDK – Support for Meilisearch 1.18
What does this PR do?
This PR adds full support for Meilisearch 1.18 features in the Java SDK.
It implements:
1.
queryVectorsupport in search responsesretrieveVectors=true, Meilisearch now returns a newqueryVectorfield.SearchResult.getQueryVector().SearchResultQueryVectorTest2. Index renaming using
PATCH /indexes/:uidIndexesHandler.updateIndex()to accept a newindexUidparameter for renaming.{ "indexUid": "indexB" }Added test:
IndexRenameTest3. Index renaming using swap-indexes
SwapIndexesParamsto includerename.IndexesHandler.swapIndexes()to send therenameflag.[ { "indexes": ["indexA", "indexB"], "rename": true } ]Added test:
SwapIndexRenameTest4. Code sample update
rename_an_index_1key in.code-samples.meilisearch.yamlSummary of changes
Source
SearchResult.javaIndexesHandler.javaSwapIndexesParams.javaIndex.java(if updated method exposed)Tests
SearchResultQueryVectorTest.javaIndexRenameTest.javaSwapIndexRenameTest.javaDocumentation
.code-samples.meilisearch.yamlPR Checklist
./gradlew clean build)spotlessApply)Summary by CodeRabbit
New Features
Tests
Chores