Open
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 WebHooksHandler and five passthrough Client methods for webhook CRUD; introduces models Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WebHooksHandler
participant HttpClient
participant MeilisearchAPI as Meilisearch API
rect rgba(100,150,200,0.5)
Note over Client,MeilisearchAPI: List webhooks
Client->>WebHooksHandler: getWebhooks()
WebHooksHandler->>HttpClient: GET /webhooks
HttpClient->>MeilisearchAPI: GET /webhooks
MeilisearchAPI-->>HttpClient: Results<Webhook>
HttpClient-->>WebHooksHandler: Results<Webhook>
WebHooksHandler-->>Client: Results<Webhook>
end
rect rgba(150,200,100,0.5)
Note over Client,MeilisearchAPI: Create webhook
Client->>WebHooksHandler: createWebhook(request)
WebHooksHandler->>HttpClient: POST /webhooks (body)
HttpClient->>MeilisearchAPI: POST /webhooks
MeilisearchAPI-->>HttpClient: Webhook
HttpClient-->>WebHooksHandler: Webhook
WebHooksHandler-->>Client: Webhook
end
rect rgba(200,150,100,0.5)
Note over Client,MeilisearchAPI: Update webhook
Client->>WebHooksHandler: updateWebhook(uuid, request)
WebHooksHandler->>HttpClient: PATCH /webhooks/{uuid} (body)
HttpClient->>MeilisearchAPI: PATCH /webhooks/{uuid}
MeilisearchAPI-->>HttpClient: Webhook
HttpClient-->>WebHooksHandler: Webhook
WebHooksHandler-->>Client: Webhook
end
rect rgba(200,100,150,0.5)
Note over Client,MeilisearchAPI: Delete webhook
Client->>WebHooksHandler: deleteWebhook(uuid)
WebHooksHandler->>HttpClient: DELETE /webhooks/{uuid}
HttpClient->>MeilisearchAPI: DELETE /webhooks/{uuid}
MeilisearchAPI-->>HttpClient: (void)
HttpClient-->>WebHooksHandler: (void)
WebHooksHandler-->>Client: (void)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 5
🤖 Fix all issues with AI agents
In `@src/main/java/com/meilisearch/sdk/Client.java`:
- Around line 597-603: The Client.deleteWebhook method currently calls
WebHooksHandler.deleteWebhook(UUID) but omits declaring throws
MeilisearchException and lacks JavaDoc `@throws`; update the method signature of
Client.deleteWebhook(UUID webhook_uuid) to add "throws MeilisearchException",
and add a JavaDoc line documenting `@throws` MeilisearchException for consistency
with updateWebhook() and the underlying WebHooksHandler.deleteWebhook
declaration so the API surface matches the handler's exception contract.
In `@src/main/java/com/meilisearch/sdk/HttpClient.java`:
- Around line 156-168: The delete(String api, Class<T> targetClass) method
currently skips HTTP error handling when targetClass is null; always check the
response status from the raw HttpResponse returned by
this.client.delete(requestConfig) (e.g., httpRequest.getStatusCode()) and if
>=400 decode the error via jsonHandler.decode(httpRequest.getContent(),
APIError.class) and throw a MeilisearchApiException before returning null; keep
the existing behavior for the targetClass != null path (call
response.create(...) and return content) but perform the status check prior to
creating/returning anything so 4xx/5xx responses are never silently ignored.
In `@src/main/java/com/meilisearch/sdk/WebHooksHandler.java`:
- Around line 64-71: The Javadoc for the deleteWebhook method is incorrect (it
says "Update a webhook"); update the method comment in WebHooksHandler.java so
the description accurately reads that it deletes a webhook (e.g., "Delete a
webhook"), keep the `@param` webhookUuid and `@throws` MeilisearchException tags
intact, and ensure the summary and method description refer to deleteWebhook and
webhookUuid to avoid copy/paste mismatch.
In `@src/test/java/com/meilisearch/integration/WebhooksTest.java`:
- Around line 106-125: The testUpdateWebhook method currently ignores the return
value of updateWebhook() and mixes a Java assert with Hamcrest; change it to
capture the result of client.updateWebhook(webhook.getUuid(), webhookReq2) into
a Webhook updatedWebhook (or similarly named variable) and replace the plain
Java assert with Hamcrest assertions (e.g.,
assertThat(updatedWebhook.getHeaders().size(), is(1))) while asserting the
updatedWebhook's fields (uuid, headers, url, editable) to verify the update
succeeded; keep CreateUpdateWebhookRequest usage and validate the updated object
rather than the original webhook.
- Around line 128-141: In testDeleteWebhook in WebhooksTest, replace the Java
built-in assert (assert webhooks.getResults().length == 0) with a Hamcrest
assertion so the check always runs; change it to use assertThat from Hamcrest
(e.g., assertThat(webhooks.getResults().length, is(0)) or
assertThat(webhooks.getResults(), arrayWithSize(0))) after ensuring
assertThat/is or arrayWithSize are imported, referencing the testDeleteWebhook
method, the getWebhooks call and the Results<Webhook> returned by
webhooks.getResults(), and leave the webhook creation/deletion
(webhook.getUuid() and client.deleteWebhook) unchanged.
🧹 Nitpick comments (1)
src/test/java/com/meilisearch/integration/WebhooksTest.java (1)
7-38: Run webhook cleanup automatically after each test.Manual calls won’t execute if a test fails. Consider
@AfterEachto keep the environment clean.🧹 Proposed change
-import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; @@ - void cleanUp() { + `@AfterEach` + void cleanUp() { Results<Webhook> webhooks = client.getWebhooks(); for (Webhook wb : webhooks.getResults()) { client.deleteWebhook(wb.getUuid()); } }
valkrypton
force-pushed
the
add/webhooks-api
branch
2 times, most recently
from
b772314 to
e1864df
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/java/com/meilisearch/sdk/model/CreateUpdateWebhookRequest.java`:
- Around line 19-22: The CreateUpdateWebhookRequest constructor should fail fast
on invalid input: add explicit non-null checks for the url and headers
parameters in the CreateUpdateWebhookRequest(String url, HashMap<String, Object>
headers) constructor (e.g., using Objects.requireNonNull or manual checks) and
throw a clear exception such as IllegalArgumentException/NullPointerException
with a descriptive message if either is null so the object cannot be created
with malformed data.
- Around line 16-20: Change the CreateUpdateWebhookRequest class so the headers
field is typed as Map<String, String> (not HashMap<String, Object>) and both url
and headers are declared final; update the constructor signature
CreateUpdateWebhookRequest(String url, HashMap<String, Object> headers) to
accept Map<String, String> and assign to the new final fields, and update any
getter/setter or usages of CreateUpdateWebhookRequest,
CreateUpdateWebhookRequest.url and CreateUpdateWebhookRequest.headers throughout
the codebase to the new types/immutability (adjust callers to pass
Map<String,String> and to handle null header values where used).
src/main/java/com/meilisearch/sdk/model/CreateUpdateWebhookRequest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/meilisearch/sdk/model/CreateUpdateWebhookRequest.java
Outdated
Show resolved
Hide resolved
valkrypton
force-pushed
the
add/webhooks-api
branch
2 times, most recently
from
6c3f198 to
e6d039a
Compare
valkrypton
force-pushed
the
add/webhooks-api
branch
from
e6d039a to
9317713
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.code-samples.meilisearch.yaml:
- Around line 877-897: Rename the sample keys to the required names
(webhooks_get_1, webhooks_get_single_1, webhooks_post_1, webhooks_patch_1,
webhooks_delete_1) and fix each snippet to call the correct client methods with
proper arguments: use client.getWebhooks() for list, client.getWebhook(uuid) for
single, client.createWebhook(webhookReq1) with a defined
CreateUpdateWebhookRequest webhookReq1, client.updateWebhook(webhook.getUuid(),
webhookReq2) with webhookReq2 defined, and
client.deleteWebhook(webhook.getUuid()) after creating a webhook to obtain its
uuid; ensure variables referenced (webhookReq1, webhookReq2, webhook) are
declared in the same snippet and that headers/authorization are set on the
CreateUpdateWebhookRequest objects before passing them to
createWebhook/updateWebhook.
🧹 Nitpick comments (1)
src/test/java/com/meilisearch/integration/WebhooksTest.java (1)
35-40: Run webhook cleanup in@AfterEachto avoid leaked state on failures.Manual
cleanUp()calls won’t execute if a test aborts early, which can leave webhooks behind and make later tests flaky. Converting cleanup to@AfterEachalso removes duplication.♻️ Suggested refactor
import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ - void cleanUp() { + `@AfterEach` + public void cleanUp() { Results<Webhook> webhooks = client.getWebhooks(); for (Webhook wb : webhooks.getResults()) { client.deleteWebhook(wb.getUuid()); } }- cleanUp(); @@ - cleanUp(); @@ - cleanUp(); @@ - cleanUp(); @@ - cleanUp();
Author
Strift
previously approved these changes
Feb 17, 2026
Contributor
Strift
left a comment
Strift
left a comment
There was a problem hiding this comment.
Hello @valkrypton and thanks for this PR 🙌
Strift
requested changes
Feb 17, 2026
Contributor
Strift
left a comment
Strift
left a comment
There was a problem hiding this comment.
Can you fix the CI errors please?
You can run the tests locally to ensure they're passing :)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/com/meilisearch/sdk/model/CreateUpdateWebhookRequest.java (1)
13-14: Fields lack@Getter— the request object is opaque to SDK consumers.Both
urlandheadersare package-private with no getters. While Gson can serialize via reflection, users of this SDK cannot inspect aCreateUpdateWebhookRequestafter construction. Adding@Getterwould be consistent with howWebhookand other models expose their fields.Proposed fix
public class CreateUpdateWebhookRequest implements Serializable { - final String url; - final HashMap<String, Object> headers; + `@Getter` private final String url; + `@Getter` private final Map<String, String> headers;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/meilisearch/sdk/model/CreateUpdateWebhookRequest.java` around lines 13 - 14, Make the CreateUpdateWebhookRequest model expose its fields by adding Lombok getters: annotate the class or the two fields `url` and `headers` with `@Getter` and add the required import (`lombok.Getter`) so SDK consumers can read these values; ensure the annotation is applied to the `CreateUpdateWebhookRequest` class (or directly to `final String url` and `final HashMap<String, Object> headers`) consistent with other models like `Webhook`.src/main/java/com/meilisearch/sdk/model/Webhook.java (1)
13-24: Consider usingMapinterface instead ofHashMapfor theheadersfield type.Using the
Map<String, String>interface rather thanHashMap<String, String>is more flexible and idiomatic. This is a minor point and consistent with what was already flagged onCreateUpdateWebhookRequest.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/meilisearch/sdk/model/Webhook.java` around lines 13 - 24, The headers field in class Webhook is typed as HashMap which reduces flexibility; change the field declaration and the constructor parameter from HashMap<String, String> to the Map<String, String> interface (update the import to java.util.Map if needed) and keep the `@Getter` on headers and the rest of the constructor assignments the same (ensure the constructor signature Webhook(UUID uuid, String url, Map<String, String> headers, boolean isEditable) and the field declaration protected final Map<String, String> headers).
🤖 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/main/java/com/meilisearch/sdk/Client.java`:
- Around line 587-605: Rename the parameters webhook_uuid to camelCase
webhookUuid in the Client methods updateWebhook(UUID webhook_uuid,
CreateUpdateWebhookRequest ...) and deleteWebhook(UUID webhook_uuid) to follow
Java conventions and match getWebhook/getters; update the method signatures and
all internal references to use webhookUuid (including the calls to
this.webHooksHandler.updateWebhook(...) and
this.webHooksHandler.deleteWebhook(...)). Also add the missing Javadoc `@throws`
MeilisearchException tag to deleteWebhook to match the method signature and
other methods' docs. Ensure any callers/tests are updated to use the new
parameter name if they reference it by name (method call sites need no change),
and run a compile to catch any remaining references.
---
Duplicate comments:
In `@src/main/java/com/meilisearch/sdk/model/CreateUpdateWebhookRequest.java`:
- Around line 12-20: CreateUpdateWebhookRequest uses HashMap<String, Object> for
headers which allows non-string values and diverges from the API and Webhook
model; change the headers field and constructor parameter to Map<String, String>
(and update any related imports/annotations) so header values are typed as
strings (null still allowed to indicate removal) and avoid using concrete
HashMap in the API surface; update the class field "headers" and the constructor
signature in CreateUpdateWebhookRequest accordingly.
In `@src/main/java/com/meilisearch/sdk/WebHooksHandler.java`:
- Around line 68-77: The Javadoc for deleteWebhook still says the parameter is
"Unique identifier of a webhook to update"; update the Javadoc for the method
deleteWebhook(UUID webhookUuid) to correctly describe the param as "Unique
identifier of a webhook to delete" (and ensure the rest of the Javadoc remains
accurate for throws and method description).
---
Nitpick comments:
In `@src/main/java/com/meilisearch/sdk/model/CreateUpdateWebhookRequest.java`:
- Around line 13-14: Make the CreateUpdateWebhookRequest model expose its fields
by adding Lombok getters: annotate the class or the two fields `url` and
`headers` with `@Getter` and add the required import (`lombok.Getter`) so SDK
consumers can read these values; ensure the annotation is applied to the
`CreateUpdateWebhookRequest` class (or directly to `final String url` and `final
HashMap<String, Object> headers`) consistent with other models like `Webhook`.
In `@src/main/java/com/meilisearch/sdk/model/Webhook.java`:
- Around line 13-24: The headers field in class Webhook is typed as HashMap
which reduces flexibility; change the field declaration and the constructor
parameter from HashMap<String, String> to the Map<String, String> interface
(update the import to java.util.Map if needed) and keep the `@Getter` on headers
and the rest of the constructor assignments the same (ensure the constructor
signature Webhook(UUID uuid, String url, Map<String, String> headers, boolean
isEditable) and the field declaration protected final Map<String, String>
headers).
Comment on lines
+587
to
+605
| * @param webhook_uuid Uuid v4 identifier of a webhook. | ||
| * @param createUpdateWebhookRequest Request body containing new header or url. | ||
| * @return A single webhook instance. | ||
| * @throws MeilisearchException If an error occurs. | ||
| */ | ||
| public Webhook updateWebhook( | ||
| UUID webhook_uuid, CreateUpdateWebhookRequest createUpdateWebhookRequest) | ||
| throws MeilisearchException { | ||
| return this.webHooksHandler.updateWebhook(webhook_uuid, createUpdateWebhookRequest); | ||
| } | ||
|
|
||
| /** | ||
| * Delete a webhook and stop sending task completion data to the target URL. | ||
| * | ||
| * @param webhook_uuid Uuid v4 identifier of a webhook. | ||
| */ | ||
| public void deleteWebhook(UUID webhook_uuid) throws MeilisearchException { | ||
| this.webHooksHandler.deleteWebhook(webhook_uuid); | ||
| } |
Contributor
There was a problem hiding this comment.
Inconsistent parameter naming: webhook_uuid (snake_case) vs webhookUuid (camelCase).
updateWebhook and deleteWebhook use webhook_uuid while getWebhook (line 566) uses webhookUuid. Java convention is camelCase, and other Client methods follow this (e.g., apiKeyUid). Additionally, deleteWebhook Javadoc is missing the @throws MeilisearchException tag even though the signature declares it.
Proposed fix
/**
* Update the configuration for the specified webhook. To remove a field, set its value to null.
*
- * `@param` webhook_uuid Uuid v4 identifier of a webhook.
+ * `@param` webhookUuid Uuid v4 identifier of a webhook.
* `@param` createUpdateWebhookRequest Request body containing new header or url.
* `@return` A single webhook instance.
* `@throws` MeilisearchException If an error occurs.
*/
public Webhook updateWebhook(
- UUID webhook_uuid, CreateUpdateWebhookRequest createUpdateWebhookRequest)
+ UUID webhookUuid, CreateUpdateWebhookRequest createUpdateWebhookRequest)
throws MeilisearchException {
- return this.webHooksHandler.updateWebhook(webhook_uuid, createUpdateWebhookRequest);
+ return this.webHooksHandler.updateWebhook(webhookUuid, createUpdateWebhookRequest);
}
/**
* Delete a webhook and stop sending task completion data to the target URL.
*
- * `@param` webhook_uuid Uuid v4 identifier of a webhook.
+ * `@param` webhookUuid Uuid v4 identifier of a webhook.
+ * `@throws` MeilisearchException If an error occurs.
*/
- public void deleteWebhook(UUID webhook_uuid) throws MeilisearchException {
- this.webHooksHandler.deleteWebhook(webhook_uuid);
+ public void deleteWebhook(UUID webhookUuid) throws MeilisearchException {
+ this.webHooksHandler.deleteWebhook(webhookUuid);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/meilisearch/sdk/Client.java` around lines 587 - 605, Rename
the parameters webhook_uuid to camelCase webhookUuid in the Client methods
updateWebhook(UUID webhook_uuid, CreateUpdateWebhookRequest ...) and
deleteWebhook(UUID webhook_uuid) to follow Java conventions and match
getWebhook/getters; update the method signatures and all internal references to
use webhookUuid (including the calls to this.webHooksHandler.updateWebhook(...)
and this.webHooksHandler.deleteWebhook(...)). Also add the missing Javadoc
`@throws` MeilisearchException tag to deleteWebhook to match the method signature
and other methods' docs. Ensure any callers/tests are updated to use the new
parameter name if they reference it by name (method call sites need no change),
and run a compile to catch any remaining references.
Author
|
@Strift CI should pass now |
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 #879
What does this PR do?
/webhooksapiClientclass for webhook related operationsWebHookHandlerclass responsible for handling all api requestsPR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
New Features
Tests
Documentation
Chores