-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Implement Webhooks module (beta) #62
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
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.
3 issues found across 14 files
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="src/main/java/com/resend/services/webhooks/dto/WebhookDTO.java">
<violation number="1" location="src/main/java/com/resend/services/webhooks/dto/WebhookDTO.java:9">
Rule violated: **Initialisms and Acronyms Naming Conventions**
Rename the class to use camel-case for the acronym (e.g., WebhookDto) to comply with the Initialisms and Acronyms Naming Conventions rule.</violation>
</file>
<file name="src/main/java/com/resend/services/webhooks/Webhooks.java">
<violation number="1" location="src/main/java/com/resend/services/webhooks/Webhooks.java:114">
Rule violated: **Initialisms and Acronyms Naming Conventions**
`URLHelper` uses the all-caps acronym `URL`; please rename it to follow the Initialisms and Acronyms Naming Conventions (e.g., `UrlHelper`).</violation>
</file>
<file name="src/test/java/com/resend/services/webhooks/WebhooksTest.java">
<violation number="1" location="src/test/java/com/resend/services/webhooks/WebhooksTest.java:25">
These tests replace the Webhooks service under test with a Mockito mock, so the real implementation never runs; the assertions merely confirm stubbed return values and will miss regressions. Please instantiate or integrate the actual service (mocking only its dependencies) so the tests exercise real behavior.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
lucasfcosta
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.
The existing code LGTM, but it seems like we're missing the verify method that was also on the Node SDK though.
It's not an API route so it does not appear on the API reference.
Here's more info:
https://github.com/resend/resend-node/blob/88e26d4c384bb62c91146307c23f43bd4fab84b9/src/webhooks/webhooks.ts#L90-L98
https://docs.svix.com/receiving/verifying-payloads/how
https://docs.svix.com/receiving/verifying-payloads/how-manual
lucasfcosta
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.
It feels weird to me that users have to follow a pattern like this to be able to pass the data into the function. I wonder if we should pass them separately. I don't work with Java day-to-day though, so I don't know what's more idiomatic. I'll leave this up to you.
WebhookHeaders headers = WebhookHeaders.builder()
.add("svix-id", msgId)
.add("svix-timestamp", timestamp)
.add("svix-signature", signature)
.build();
|
@lucasfcosta changed the impl by removing the headers class and adding it directly to a map in the class options. I think this way it feels more idiomatic: |
lucasfcosta
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.
Cool! That looks okay to me. I don't write Java frequently enough to judge with detail but I trust you if you think that's more idiomatic.
Summary by cubic
Adds a beta Webhooks module to the Java SDK so you can create, update, get, list (with pagination), and delete webhooks via Resend.webhooks(). This lets apps manage webhook endpoints and events directly in code.