Add missing endpoints#51
Open
IgorDobryn wants to merge 4 commits intomainfrom
Open
Conversation
📝 WalkthroughWalkthroughThis PR adds three new API modules—API Tokens, Sub-Accounts, and Webhooks—each with interface, implementation, and request/response models. It updates the client factory and composition to wire these APIs, adds environment configuration for Mailtrap credentials, updates the contacts API with a new retrieval method, and includes comprehensive test coverage with fixtures. ChangesAPI Tokens, Sub-Accounts, and Webhooks Features
Environment and Build Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.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: 2
♻️ Duplicate comments (1)
src/test/resources/api/apitokens/createApiTokenResponse.json (1)
11-11:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame
access_level: 100integer deserialization risk as inresetApiTokenResponse.json.The
access_levelfield here is also100(integer). The same@JsonCreatorverification applies — see the comment raised onresetApiTokenResponse.jsonline 11.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/resources/api/apitokens/createApiTokenResponse.json` at line 11, The test JSON uses "access_level": 100 (integer) which risks failing the model's `@JsonCreator` string-validated deserialization (same issue noted in resetApiTokenResponse.json); update src/test/resources/api/apitokens/createApiTokenResponse.json to make access_level a string value ("100") to match the `@JsonCreator` expectation, or alternatively update the model's `@JsonCreator` (the deserializer that validates the access_level field) to accept integers as well — reference the access_level field and the `@JsonCreator` in the token response model when making the change.
🧹 Nitpick comments (5)
src/main/java/io/mailtrap/model/request/subaccounts/CreateSubAccountRequest.java (1)
9-13: 💤 Low valueConsider adding
@ToStringfor logging/debugging support.Using
@Getter +@Setter`` instead of@Datais intentional and correct here — if a class has an explicit superclass, Lombok requires supplying a `callSuper` value for `@EqualsAndHashCode`; failure to do so results in a warning, which `@Data` would trigger. The only minor gap is that `@ToString` is not included, leaving the default `Object.toString()` (identity hash) for this request DTO, which can hinder logging.💡 Suggested addition
`@Getter` `@Setter` `@NoArgsConstructor` `@AllArgsConstructor` +@ToString public class CreateSubAccountRequest extends AbstractModel {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mailtrap/model/request/subaccounts/CreateSubAccountRequest.java` around lines 9 - 13, Add Lombok's `@ToString` to the CreateSubAccountRequest class so instances produce meaningful string output for logging; annotate the class with `@ToString` (and if AbstractModel contains relevant fields you want included in logs, use `@ToString`(callSuper = true)) alongside the existing `@Getter/`@Setter annotations on CreateSubAccountRequest.src/main/java/io/mailtrap/model/response/apitokens/ApiTokenWithToken.java (1)
6-8: ⚡ Quick win
toString()omits all parent fields.
@ToStringdoes not callsuperby default;callSuper = truemust be set explicitly, and it's only meaningful when extending a non-Objectclass. Here@DatageneratestoString()that only includestoken, silently dropping the six inherited fields (id,name,last4Digits, etc.). Since@EqualsAndHashCode(callSuper = true)is already present for consistency, add the matching@ToStringoverride.♻️ Proposed fix
import lombok.Data; import lombok.EqualsAndHashCode; +import lombok.ToString; `@Data` `@EqualsAndHashCode`(callSuper = true) +@ToString(callSuper = true) public class ApiTokenWithToken extends ApiToken {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mailtrap/model/response/apitokens/ApiTokenWithToken.java` around lines 6 - 8, The generated toString() on ApiTokenWithToken currently omits all fields inherited from ApiToken; add an explicit Lombok ToString with callSuper = true on the ApiTokenWithToken class (matching the existing EqualsAndHashCode(callSuper = true)) so the superclass fields (id, name, last4Digits, etc.) are included in the generated toString output.src/test/resources/api/webhooks/createWebhookResponse.json (1)
14-14: 💤 Low valueStatic analysis false positive — consider a scanner-friendly placeholder.
The flagged
signing_secretvalue is clearly synthetic test data and poses no actual security risk. That said, the hex-pattern string will continue to trip secret scanners in CI. Using a non-hex-pattern string avoids that noise:🔧 Suggested change to reduce scanner false positives
- "signing_secret": "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6" + "signing_secret": "test-signing-secret-placeholder"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/resources/api/webhooks/createWebhookResponse.json` at line 14, Replace the synthetic hex-looking value for the JSON key "signing_secret" with a scanner-friendly placeholder string (e.g., non-hex, clearly labeled test value like "REDACTED_TEST_SIGNING_SECRET") so automated secret scanners stop flagging it; update the value in the createWebhookResponse JSON where the "signing_secret" key is defined and ensure the placeholder still clearly indicates test-only usage.src/main/java/io/mailtrap/model/request/webhooks/UpdateWebhookRequest.java (1)
1-17: 💤 Low value
CreateWebhookRequestandUpdateWebhookRequestare structurally identical — consider a shared base.Both classes extend
AbstractModel, carry the same four Lombok annotations, and declare a singleWebhookInput webhookfield. A common abstract base would eliminate the duplication:♻️ Proposed refactor
+ // new file: AbstractWebhookRequest.java + `@Getter` + `@Setter` + `@NoArgsConstructor` + `@AllArgsConstructor` + public abstract class AbstractWebhookRequest extends AbstractModel { + private WebhookInput webhook; + }- `@Getter` - `@Setter` - `@NoArgsConstructor` - `@AllArgsConstructor` - public class CreateWebhookRequest extends AbstractModel { - private WebhookInput webhook; - } + public class CreateWebhookRequest extends AbstractWebhookRequest { }- `@Getter` - `@Setter` - `@NoArgsConstructor` - `@AllArgsConstructor` - public class UpdateWebhookRequest extends AbstractModel { - private WebhookInput webhook; - } + public class UpdateWebhookRequest extends AbstractWebhookRequest { }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mailtrap/model/request/webhooks/UpdateWebhookRequest.java` around lines 1 - 17, Create a shared abstract base (e.g., AbstractWebhookRequest) that extends AbstractModel and declares the WebhookInput webhook field plus the Lombok annotations (`@Getter`, `@Setter`, `@NoArgsConstructor`, `@AllArgsConstructor`); then update CreateWebhookRequest and UpdateWebhookRequest to simply extend that new base and remove the duplicated field and Lombok annotations from those subclasses so only the unique class names remain (retain reference to WebhookInput and AbstractModel).src/test/java/io/mailtrap/api/webhooks/WebhooksImplTest.java (1)
62-84: ⚡ Quick winConsider adding a
payloadFormatassertion to validate the new enum's round-trip deserialization.
WebhookPayloadFormatis introduced in this PR and its@JsonCreatorpath is exercised via fixtures (e.g.,getWebhookResponse.jsonincludespayload_format), but no test ever asserts the deserialized value. A single assertion in eithertest_createWebhookortest_getWebhookwould confirm the@JsonValue/@JsonCreatorwiring works end-to-end.✅ Suggested assertion (e.g., in
test_getWebhook)assertEquals(List.of(WebhookEventType.DELIVERY, WebhookEventType.BOUNCE), response.getData().getEventTypes()); +assertEquals(WebhookPayloadFormat.JSON, response.getData().getPayloadFormat());Also applies to: 96-104
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/io/mailtrap/api/webhooks/WebhooksImplTest.java` around lines 62 - 84, Add an assertion to verify that the new enum WebhookPayloadFormat round-trips correctly by asserting the deserialized payload format on the response object; for example, in test_createWebhook (where CreateWebhookResponse response is available) add an assertion that response.getData().getPayloadFormat() equals WebhookPayloadFormat.JSON to confirm the `@JsonValue/`@JsonCreator wiring works end-to-end.
🤖 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/io/mailtrap/model/WebhookEventType.java`:
- Around line 33-41: The fromValue method in WebhookEventType currently throws
IllegalArgumentException for unknown strings which will break deserialization of
List<WebhookEventType> eventTypes; change WebhookEventType.fromValue to handle
unknown values gracefully by returning either null or a dedicated UNKNOWN enum
constant instead of throwing—locate the static fromValue method in
WebhookEventType, mirror the fix used for WebhookType (check the loop over
WebhookEventType.values() and on no match return null or
WebhookEventType.UNKNOWN), and ensure callers that expect List<WebhookEventType>
can handle the null/UNKNOWN sentinel appropriately.
In `@src/main/java/io/mailtrap/model/WebhookType.java`:
- Around line 27-35: The current WebhookType.fromValue(String) method throws
IllegalArgumentException which breaks deserialization for unknown future enum
values; change the method to return a safe fallback instead of throwing: either
add an UNKNOWN sentinel constant to the WebhookType enum and return
WebhookType.UNKNOWN when no match is found, or return null from fromValue so
callers can handle absent values; update the `@JsonCreator` method
(WebhookType.fromValue) to implement the chosen fallback and apply the same
pattern to other enums using identical logic (e.g., WebhookPayloadFormat,
SendingStream, WebhookEventType) to ensure forward compatibility across the SDK.
---
Duplicate comments:
In `@src/test/resources/api/apitokens/createApiTokenResponse.json`:
- Line 11: The test JSON uses "access_level": 100 (integer) which risks failing
the model's `@JsonCreator` string-validated deserialization (same issue noted in
resetApiTokenResponse.json); update
src/test/resources/api/apitokens/createApiTokenResponse.json to make
access_level a string value ("100") to match the `@JsonCreator` expectation, or
alternatively update the model's `@JsonCreator` (the deserializer that validates
the access_level field) to accept integers as well — reference the access_level
field and the `@JsonCreator` in the token response model when making the change.
---
Nitpick comments:
In
`@src/main/java/io/mailtrap/model/request/subaccounts/CreateSubAccountRequest.java`:
- Around line 9-13: Add Lombok's `@ToString` to the CreateSubAccountRequest class
so instances produce meaningful string output for logging; annotate the class
with `@ToString` (and if AbstractModel contains relevant fields you want included
in logs, use `@ToString`(callSuper = true)) alongside the existing `@Getter/`@Setter
annotations on CreateSubAccountRequest.
In `@src/main/java/io/mailtrap/model/request/webhooks/UpdateWebhookRequest.java`:
- Around line 1-17: Create a shared abstract base (e.g., AbstractWebhookRequest)
that extends AbstractModel and declares the WebhookInput webhook field plus the
Lombok annotations (`@Getter`, `@Setter`, `@NoArgsConstructor`, `@AllArgsConstructor`);
then update CreateWebhookRequest and UpdateWebhookRequest to simply extend that
new base and remove the duplicated field and Lombok annotations from those
subclasses so only the unique class names remain (retain reference to
WebhookInput and AbstractModel).
In `@src/main/java/io/mailtrap/model/response/apitokens/ApiTokenWithToken.java`:
- Around line 6-8: The generated toString() on ApiTokenWithToken currently omits
all fields inherited from ApiToken; add an explicit Lombok ToString with
callSuper = true on the ApiTokenWithToken class (matching the existing
EqualsAndHashCode(callSuper = true)) so the superclass fields (id, name,
last4Digits, etc.) are included in the generated toString output.
In `@src/test/java/io/mailtrap/api/webhooks/WebhooksImplTest.java`:
- Around line 62-84: Add an assertion to verify that the new enum
WebhookPayloadFormat round-trips correctly by asserting the deserialized payload
format on the response object; for example, in test_createWebhook (where
CreateWebhookResponse response is available) add an assertion that
response.getData().getPayloadFormat() equals WebhookPayloadFormat.JSON to
confirm the `@JsonValue/`@JsonCreator wiring works end-to-end.
In `@src/test/resources/api/webhooks/createWebhookResponse.json`:
- Line 14: Replace the synthetic hex-looking value for the JSON key
"signing_secret" with a scanner-friendly placeholder string (e.g., non-hex,
clearly labeled test value like "REDACTED_TEST_SIGNING_SECRET") so automated
secret scanners stop flagging it; update the value in the createWebhookResponse
JSON where the "signing_secret" key is defined and ensure the placeholder still
clearly indicates test-only usage.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 382d1bdb-1ec4-4134-8033-69fdca8e920d
📒 Files selected for processing (57)
.envrc.gitignoresrc/main/java/io/mailtrap/api/apitokens/ApiTokens.javasrc/main/java/io/mailtrap/api/apitokens/ApiTokensImpl.javasrc/main/java/io/mailtrap/api/contacts/Contacts.javasrc/main/java/io/mailtrap/api/contacts/ContactsImpl.javasrc/main/java/io/mailtrap/api/subaccounts/SubAccounts.javasrc/main/java/io/mailtrap/api/subaccounts/SubAccountsImpl.javasrc/main/java/io/mailtrap/api/webhooks/Webhooks.javasrc/main/java/io/mailtrap/api/webhooks/WebhooksImpl.javasrc/main/java/io/mailtrap/client/MailtrapClient.javasrc/main/java/io/mailtrap/client/api/MailtrapGeneralApi.javasrc/main/java/io/mailtrap/client/api/MailtrapOrganizationsApi.javasrc/main/java/io/mailtrap/factory/MailtrapClientFactory.javasrc/main/java/io/mailtrap/http/impl/DefaultMailtrapHttpClient.javasrc/main/java/io/mailtrap/model/ResourceType.javasrc/main/java/io/mailtrap/model/WebhookEventType.javasrc/main/java/io/mailtrap/model/WebhookPayloadFormat.javasrc/main/java/io/mailtrap/model/WebhookType.javasrc/main/java/io/mailtrap/model/request/apitokens/ApiTokenResource.javasrc/main/java/io/mailtrap/model/request/apitokens/CreateApiTokenRequest.javasrc/main/java/io/mailtrap/model/request/subaccounts/CreateSubAccountRequest.javasrc/main/java/io/mailtrap/model/request/subaccounts/SubAccountInput.javasrc/main/java/io/mailtrap/model/request/webhooks/CreateWebhookRequest.javasrc/main/java/io/mailtrap/model/request/webhooks/UpdateWebhookRequest.javasrc/main/java/io/mailtrap/model/request/webhooks/WebhookInput.javasrc/main/java/io/mailtrap/model/response/apitokens/ApiToken.javasrc/main/java/io/mailtrap/model/response/apitokens/ApiTokenResource.javasrc/main/java/io/mailtrap/model/response/apitokens/ApiTokenWithToken.javasrc/main/java/io/mailtrap/model/response/contacts/GetContactData.javasrc/main/java/io/mailtrap/model/response/contacts/GetContactResponse.javasrc/main/java/io/mailtrap/model/response/subaccounts/SubAccount.javasrc/main/java/io/mailtrap/model/response/webhooks/CreateWebhookResponse.javasrc/main/java/io/mailtrap/model/response/webhooks/ListWebhooksResponse.javasrc/main/java/io/mailtrap/model/response/webhooks/Webhook.javasrc/main/java/io/mailtrap/model/response/webhooks/WebhookResponse.javasrc/main/java/io/mailtrap/model/response/webhooks/WebhookWithSecret.javasrc/test/java/io/mailtrap/api/apitokens/ApiTokensImplTest.javasrc/test/java/io/mailtrap/api/contacts/ContactsImplTest.javasrc/test/java/io/mailtrap/api/subaccounts/SubAccountsImplTest.javasrc/test/java/io/mailtrap/api/webhooks/WebhooksImplTest.javasrc/test/resources/api/apitokens/createApiTokenRequest.jsonsrc/test/resources/api/apitokens/createApiTokenResponse.jsonsrc/test/resources/api/apitokens/getApiTokenResponse.jsonsrc/test/resources/api/apitokens/listApiTokensResponse.jsonsrc/test/resources/api/apitokens/resetApiTokenResponse.jsonsrc/test/resources/api/contacts/getContactResponse.jsonsrc/test/resources/api/subaccounts/createSubAccountRequest.jsonsrc/test/resources/api/subaccounts/createSubAccountResponse.jsonsrc/test/resources/api/subaccounts/getSubAccountsResponse.jsonsrc/test/resources/api/webhooks/createWebhookRequest.jsonsrc/test/resources/api/webhooks/createWebhookResponse.jsonsrc/test/resources/api/webhooks/deleteWebhookResponse.jsonsrc/test/resources/api/webhooks/getWebhookResponse.jsonsrc/test/resources/api/webhooks/listWebhooksResponse.jsonsrc/test/resources/api/webhooks/updateWebhookRequest.jsonsrc/test/resources/api/webhooks/updateWebhookResponse.json
Comment on lines
+33
to
+41
| @JsonCreator | ||
| public static WebhookEventType fromValue(String value) { | ||
| for (WebhookEventType type : WebhookEventType.values()) { | ||
| if (type.value.equalsIgnoreCase(value)) { | ||
| return type; | ||
| } | ||
| } | ||
| throw new IllegalArgumentException("Unknown value: " + value); | ||
| } |
There was a problem hiding this comment.
Same forward-compatibility risk as WebhookType.fromValue.
The fromValue method throws IllegalArgumentException for any unknown event type. Because WebhookEventType is used in List<WebhookEventType> eventTypes, a single unrecognised value from the API will fail deserialization of the entire Webhook response. Apply the same null-return (or UNKNOWN sentinel) fix described for WebhookType.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/io/mailtrap/model/WebhookEventType.java` around lines 33 - 41,
The fromValue method in WebhookEventType currently throws
IllegalArgumentException for unknown strings which will break deserialization of
List<WebhookEventType> eventTypes; change WebhookEventType.fromValue to handle
unknown values gracefully by returning either null or a dedicated UNKNOWN enum
constant instead of throwing—locate the static fromValue method in
WebhookEventType, mirror the fix used for WebhookType (check the loop over
WebhookEventType.values() and on no match return null or
WebhookEventType.UNKNOWN), and ensure callers that expect List<WebhookEventType>
can handle the null/UNKNOWN sentinel appropriately.
Comment on lines
+27
to
+35
| @JsonCreator | ||
| public static WebhookType fromValue(String value) { | ||
| for (WebhookType type : WebhookType.values()) { | ||
| if (type.value.equalsIgnoreCase(value)) { | ||
| return type; | ||
| } | ||
| } | ||
| throw new IllegalArgumentException("Unknown value: " + value); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check existing enum fromValue implementations for exception vs null/unknown fallback
rg -n "fromValue\|IllegalArgumentException\|UNKNOWN" --type java \
-g "!WebhookType.java" -g "!WebhookEventType.java" \
-A 5 -B 2Repository: mailtrap/mailtrap-java
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Find all Java files that might be enums
fd -e java | xargs grep -l "enum\s\+\w\+\|@JsonCreator" | head -20Repository: mailtrap/mailtrap-java
Length of output: 1424
🏁 Script executed:
#!/bin/bash
# Check the actual WebhookType.java file location and content
find . -name "WebhookType.java" -o -name "WebhookEventType.java" | xargs cat -nRepository: mailtrap/mailtrap-java
Length of output: 2417
🏁 Script executed:
#!/bin/bash
# Look for all classes with `@JsonCreator` annotation
rg "@JsonCreator" --type java -B 2 -A 8Repository: mailtrap/mailtrap-java
Length of output: 18300
Forward-compatibility risk: @JsonCreator throwing IllegalArgumentException will break deserialization of any new enum value from the API.
When @JsonCreator throws IllegalArgumentException, Jackson wraps it in an InvalidFormatException and the entire response deserialization fails. For a public SDK, where the Mailtrap API may evolve by adding new webhook types (e.g., transactional), all existing clients will crash at runtime with no graceful recovery.
This pattern is systemic across the codebase—nearly all enum classes (WebhookPayloadFormat, SendingStream, WebhookEventType, and 12+ others) use the same strict approach. Consider adopting a lenient fallback strategy:
- Return
null(callers must guard on null), or - Add an
UNKNOWNsentinel constant and return it by default
Either approach maintains backward compatibility while allowing the SDK to gracefully handle future API evolution.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/io/mailtrap/model/WebhookType.java` around lines 27 - 35, The
current WebhookType.fromValue(String) method throws IllegalArgumentException
which breaks deserialization for unknown future enum values; change the method
to return a safe fallback instead of throwing: either add an UNKNOWN sentinel
constant to the WebhookType enum and return WebhookType.UNKNOWN when no match is
found, or return null from fromValue so callers can handle absent values; update
the `@JsonCreator` method (WebhookType.fromValue) to implement the chosen fallback
and apply the same pattern to other enums using identical logic (e.g.,
WebhookPayloadFormat, SendingStream, WebhookEventType) to ensure forward
compatibility across the SDK.
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
1 participant
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.
Motivation
Changes
Contacts
Webhooks
API Tokens
Sub-Accounts (Organizations)
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores