Open
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (42)
📝 WalkthroughWalkthroughIntroduces Email Logs API support to the Mailtrap Java client library, adding request/response models, comprehensive filtering capabilities, list and retrieval operations, factory integration, example code, and unit tests for email log management. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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 (7)
src/main/java/io/mailtrap/model/request/emaillogs/FilterSendingDomainId.java (1)
13-31: Consider extracting a common base class for duplicate filter logic.
FilterSendingDomainIdandFilterExact(and likely otherFilter*classes) share identicalOperatorenums and implementation logic. While the current approach provides clear type distinction for each filter field, you could reduce duplication by extracting a generic base class or using a shared enum.This is an optional improvement—the current design is valid and provides explicit typing for each filter category.
🤖 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/emaillogs/FilterSendingDomainId.java` around lines 13 - 31, FilterSendingDomainId duplicates the Operator enum and same operator/value logic found in FilterExact and other Filter* classes; extract a common abstract base (e.g., AbstractEmailLogFilter) that defines the Operator enum, private Operator operator and Object value fields, and implements getOperatorString() and getValue(), then have FilterSendingDomainId and FilterExact extend that base and remove their local Operator, operator, value, getOperatorString, and getValue declarations to eliminate duplication while preserving type-specific class names.src/main/java/io/mailtrap/model/response/emaillogs/EmailLogsListResponse.java (1)
11-12: Consider defensive initialization for the messages list.The
messagesfield could benullif the JSON response omits it, potentially causingNullPointerExceptionwhen callers iterate over it. Other models in this PR (e.g.,EmailLogMessageSummary.getCustomVariables()) provide null-safe accessors.♻️ Optional: Add null-safe getter or initialize the field
`@JsonProperty`("messages") -private List<EmailLogMessageSummary> messages; +private List<EmailLogMessageSummary> messages = new ArrayList<>();Or add a null-safe getter:
public List<EmailLogMessageSummary> getMessages() { return messages == null ? Collections.emptyList() : messages; }🤖 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/emaillogs/EmailLogsListResponse.java` around lines 11 - 12, The messages field in EmailLogsListResponse can be null if omitted in JSON, risking NPEs; make access null-safe by either initializing the field to an empty list (set messages = new ArrayList<>() by default) or add a null-safe accessor getMessages() that returns Collections.emptyList() when messages is null; update any callers to use EmailLogsListResponse.getMessages() if adding the getter.src/main/java/io/mailtrap/model/response/emaillogs/MessageStatus.java (1)
26-34: Consider adding anUNKNOWNfallback for forward compatibility.If the Mailtrap API introduces a new status value in the future,
fromValue()will throw anIllegalArgumentExceptioncausing deserialization to fail. Consider adding anUNKNOWNenum constant as a fallback.♻️ Optional: Add UNKNOWN fallback
public enum MessageStatus { DELIVERED("delivered"), NOT_DELIVERED("not_delivered"), ENQUEUED("enqueued"), - OPTED_OUT("opted_out"); + OPTED_OUT("opted_out"), + UNKNOWN("unknown"); // ... constructor and getValue() ... `@JsonCreator` public static MessageStatus fromValue(String value) { for (MessageStatus status : MessageStatus.values()) { if (status.value.equalsIgnoreCase(value)) { return status; } } - throw new IllegalArgumentException("Unknown MessageStatus value: " + value); + return UNKNOWN; } }🤖 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/emaillogs/MessageStatus.java` around lines 26 - 34, The fromValue(String) deserializer in MessageStatus currently throws IllegalArgumentException for unknown values and should be made forward-compatible: add an UNKNOWN enum constant to MessageStatus and update the MessageStatus.fromValue(String value) method to return MessageStatus.UNKNOWN when no match (preserving case-insensitive comparison) instead of throwing; ensure any existing usages that rely on exceptions are updated to handle the UNKNOWN fallback if needed.src/main/java/io/mailtrap/model/response/emaillogs/EmailLogMessageEvent.java (1)
23-24: Consider makingcreatedAtfield private for encapsulation.The field is declared
publicbut has explicit getter/setter methods defined. For consistency with standard JavaBean conventions and encapsulation, this should beprivate.♻️ Suggested fix
`@JsonProperty`("created_at") - public OffsetDateTime createdAt; + private OffsetDateTime createdAt;🤖 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/emaillogs/EmailLogMessageEvent.java` around lines 23 - 24, The createdAt field in class EmailLogMessageEvent is declared public but already has getter/setter methods; change the field visibility to private to follow JavaBean encapsulation conventions (update the declaration of createdAt to private OffsetDateTime createdAt) and keep the existing getCreatedAt() and setCreatedAt(...) methods so external access goes through those accessors.src/main/java/io/mailtrap/model/response/emaillogs/EmailLogEventType.java (1)
31-39: Add null check infromValueto prevent potential NPE.If
fromValue(null)is called,type.value.equalsIgnoreCase(value)will throw aNullPointerException. While Jackson typically won't pass null here, a defensive check improves robustness.🛡️ Suggested fix
`@JsonCreator` public static EmailLogEventType fromValue(String value) { + if (value == null) { + throw new IllegalArgumentException("EmailLogEventType value cannot be null"); + } for (EmailLogEventType type : EmailLogEventType.values()) { if (type.value.equalsIgnoreCase(value)) { return type; } } throw new IllegalArgumentException("Unknown EmailLogEventType value: " + value); }🤖 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/emaillogs/EmailLogEventType.java` around lines 31 - 39, The fromValue method in EmailLogEventType (annotated with `@JsonCreator`) can NPE when called with null because type.value.equalsIgnoreCase(value) is invoked; add a defensive null check at the start of fromValue (e.g. if value == null) and fail fast by throwing an IllegalArgumentException with a clear message (or handle null explicitly), then proceed with the existing loop that uses equalsIgnoreCase; this ensures no NPE and preserves current behavior when non-null values are provided.src/main/java/io/mailtrap/model/request/emaillogs/EmailLogsListFilters.java (1)
19-20: Consider usingOffsetDateTimefor date filters.Using
StringforsentAfterandsentBeforeputs the burden of ISO 8601 formatting on the caller. Consider usingOffsetDateTimefor compile-time type safety and letting the implementation format it, similar to howEmailLogMessage.sentAtusesOffsetDateTime.🤖 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/emaillogs/EmailLogsListFilters.java` around lines 19 - 20, Change the sentAfter and sentBefore fields in EmailLogsListFilters from String to java.time.OffsetDateTime (like EmailLogMessage.sentAt) and update their getters/setters and constructors accordingly; ensure any serialization or query-building code that consumes EmailLogsListFilters formats these OffsetDateTime values to the expected ISO 8601 string (or lets your JSON mapper handle it) so callers pass typed datetimes instead of raw strings.src/main/java/io/mailtrap/api/emaillogs/EmailLogsImpl.java (1)
97-105: Minor: redundantString.valueOfcall.On Line 92,
vis already aStringfromtoValueList(), so theString.valueOf(v)is redundant.♻️ Proposed simplification
if (value != null) { for (final String v : toValueList(value)) { - params.add(enc("filters[" + field + "][value]") + "=" + enc(String.valueOf(v))); + params.add(enc("filters[" + field + "][value]") + "=" + enc(v)); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/io/mailtrap/api/emaillogs/EmailLogsImpl.java` around lines 97 - 105, The map operation in toValueList unnecessarily calls String.valueOf on elements that are already Strings; update the stream mapping in toValueList (the lambda variable v) to cast to String instead of calling String.valueOf (e.g., replace map(String::valueOf) with map(v -> (String) v)) so you avoid the redundant conversion while keeping the singletonList branch unchanged.
🤖 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/api/emaillogs/EmailLogsImpl.java`:
- Around line 37-44: EmailLogsImpl.get currently interpolates sendingMessageId
directly into the path which can break URLs for IDs containing reserved
characters; update the method to URL-encode sendingMessageId before building the
url (e.g., use URLEncoder.encode(sendingMessageId, StandardCharsets.UTF_8) or an
equivalent encoder) and use the encoded value in the String.format call so the
final url variable is safe for httpClient.get.
In `@src/main/java/io/mailtrap/model/request/emaillogs/FilterSubject.java`:
- Around line 19-29: FilterSubject currently returns its raw value even for
valueless operators; update it so getValue() returns null when operator is
Operator.empty or Operator.not_empty to suppress the field during serialization,
and also add defensive validation in the FilterSubject constructor or
setValue(...) to throw an IllegalArgumentException if a non-null value is
provided while operator is Operator.empty or Operator.not_empty, so invalid
instances cannot be constructed; refer to the FilterSubject class, getValue(),
operator field, and the Operator.empty / Operator.not_empty enum members when
making these changes.
In `@src/main/java/io/mailtrap/model/request/emaillogs/FilterTo.java`:
- Around line 20-31: The FilterTo class currently allows any Object in the value
field which lets callers pass collections for unsupported operators; update the
validation so collections/arrays are only accepted when operator is ci_equal or
ci_not_equal. In FilterTo (check in the setter or in getValue()), detect if
value is a Collection or array and if so throw an IllegalArgumentException when
operator is null or operator.name() is not "ci_equal" or "ci_not_equal"; allow
null or single values for all operators still. Reference: FilterTo.value,
FilterTo.getValue(), and Operator (operator.name()) when implementing this
guard.
---
Nitpick comments:
In `@src/main/java/io/mailtrap/api/emaillogs/EmailLogsImpl.java`:
- Around line 97-105: The map operation in toValueList unnecessarily calls
String.valueOf on elements that are already Strings; update the stream mapping
in toValueList (the lambda variable v) to cast to String instead of calling
String.valueOf (e.g., replace map(String::valueOf) with map(v -> (String) v)) so
you avoid the redundant conversion while keeping the singletonList branch
unchanged.
In `@src/main/java/io/mailtrap/model/request/emaillogs/EmailLogsListFilters.java`:
- Around line 19-20: Change the sentAfter and sentBefore fields in
EmailLogsListFilters from String to java.time.OffsetDateTime (like
EmailLogMessage.sentAt) and update their getters/setters and constructors
accordingly; ensure any serialization or query-building code that consumes
EmailLogsListFilters formats these OffsetDateTime values to the expected ISO
8601 string (or lets your JSON mapper handle it) so callers pass typed datetimes
instead of raw strings.
In
`@src/main/java/io/mailtrap/model/request/emaillogs/FilterSendingDomainId.java`:
- Around line 13-31: FilterSendingDomainId duplicates the Operator enum and same
operator/value logic found in FilterExact and other Filter* classes; extract a
common abstract base (e.g., AbstractEmailLogFilter) that defines the Operator
enum, private Operator operator and Object value fields, and implements
getOperatorString() and getValue(), then have FilterSendingDomainId and
FilterExact extend that base and remove their local Operator, operator, value,
getOperatorString, and getValue declarations to eliminate duplication while
preserving type-specific class names.
In `@src/main/java/io/mailtrap/model/response/emaillogs/EmailLogEventType.java`:
- Around line 31-39: The fromValue method in EmailLogEventType (annotated with
`@JsonCreator`) can NPE when called with null because
type.value.equalsIgnoreCase(value) is invoked; add a defensive null check at the
start of fromValue (e.g. if value == null) and fail fast by throwing an
IllegalArgumentException with a clear message (or handle null explicitly), then
proceed with the existing loop that uses equalsIgnoreCase; this ensures no NPE
and preserves current behavior when non-null values are provided.
In
`@src/main/java/io/mailtrap/model/response/emaillogs/EmailLogMessageEvent.java`:
- Around line 23-24: The createdAt field in class EmailLogMessageEvent is
declared public but already has getter/setter methods; change the field
visibility to private to follow JavaBean encapsulation conventions (update the
declaration of createdAt to private OffsetDateTime createdAt) and keep the
existing getCreatedAt() and setCreatedAt(...) methods so external access goes
through those accessors.
In
`@src/main/java/io/mailtrap/model/response/emaillogs/EmailLogsListResponse.java`:
- Around line 11-12: The messages field in EmailLogsListResponse can be null if
omitted in JSON, risking NPEs; make access null-safe by either initializing the
field to an empty list (set messages = new ArrayList<>() by default) or add a
null-safe accessor getMessages() that returns Collections.emptyList() when
messages is null; update any callers to use EmailLogsListResponse.getMessages()
if adding the getter.
In `@src/main/java/io/mailtrap/model/response/emaillogs/MessageStatus.java`:
- Around line 26-34: The fromValue(String) deserializer in MessageStatus
currently throws IllegalArgumentException for unknown values and should be made
forward-compatible: add an UNKNOWN enum constant to MessageStatus and update the
MessageStatus.fromValue(String value) method to return MessageStatus.UNKNOWN
when no match (preserving case-insensitive comparison) instead of throwing;
ensure any existing usages that rely on exceptions are updated to handle the
UNKNOWN fallback if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4909d125-e413-4670-b812-fe21f5743e54
📒 Files selected for processing (45)
README.mdexamples/java/io/mailtrap/examples/emaillogs/EmailLogsExample.javasrc/main/java/io/mailtrap/api/emaillogs/EmailLogs.javasrc/main/java/io/mailtrap/api/emaillogs/EmailLogsImpl.javasrc/main/java/io/mailtrap/client/api/MailtrapEmailSendingApi.javasrc/main/java/io/mailtrap/factory/MailtrapClientFactory.javasrc/main/java/io/mailtrap/model/request/emaillogs/EmailLogFilter.javasrc/main/java/io/mailtrap/model/request/emaillogs/EmailLogsListFilters.javasrc/main/java/io/mailtrap/model/request/emaillogs/FilterClientIp.javasrc/main/java/io/mailtrap/model/request/emaillogs/FilterEvents.javasrc/main/java/io/mailtrap/model/request/emaillogs/FilterExact.javasrc/main/java/io/mailtrap/model/request/emaillogs/FilterFrom.javasrc/main/java/io/mailtrap/model/request/emaillogs/FilterMandatoryText.javasrc/main/java/io/mailtrap/model/request/emaillogs/FilterNumber.javasrc/main/java/io/mailtrap/model/request/emaillogs/FilterSendingDomainId.javasrc/main/java/io/mailtrap/model/request/emaillogs/FilterSendingIp.javasrc/main/java/io/mailtrap/model/request/emaillogs/FilterSendingStream.javasrc/main/java/io/mailtrap/model/request/emaillogs/FilterStatus.javasrc/main/java/io/mailtrap/model/request/emaillogs/FilterSubject.javasrc/main/java/io/mailtrap/model/request/emaillogs/FilterTo.javasrc/main/java/io/mailtrap/model/response/emaillogs/EmailLogEventType.javasrc/main/java/io/mailtrap/model/response/emaillogs/EmailLogMessage.javasrc/main/java/io/mailtrap/model/response/emaillogs/EmailLogMessageEvent.javasrc/main/java/io/mailtrap/model/response/emaillogs/EmailLogMessageEventBounce.javasrc/main/java/io/mailtrap/model/response/emaillogs/EmailLogMessageEventClick.javasrc/main/java/io/mailtrap/model/response/emaillogs/EmailLogMessageEventDelivery.javasrc/main/java/io/mailtrap/model/response/emaillogs/EmailLogMessageEventOpen.javasrc/main/java/io/mailtrap/model/response/emaillogs/EmailLogMessageEventReject.javasrc/main/java/io/mailtrap/model/response/emaillogs/EmailLogMessageEventSoftBounce.javasrc/main/java/io/mailtrap/model/response/emaillogs/EmailLogMessageEventSpam.javasrc/main/java/io/mailtrap/model/response/emaillogs/EmailLogMessageEventSuspension.javasrc/main/java/io/mailtrap/model/response/emaillogs/EmailLogMessageEventUnsubscribe.javasrc/main/java/io/mailtrap/model/response/emaillogs/EmailLogMessageSummary.javasrc/main/java/io/mailtrap/model/response/emaillogs/EmailLogsListResponse.javasrc/main/java/io/mailtrap/model/response/emaillogs/EventDetailsBounce.javasrc/main/java/io/mailtrap/model/response/emaillogs/EventDetailsClick.javasrc/main/java/io/mailtrap/model/response/emaillogs/EventDetailsDelivery.javasrc/main/java/io/mailtrap/model/response/emaillogs/EventDetailsOpen.javasrc/main/java/io/mailtrap/model/response/emaillogs/EventDetailsReject.javasrc/main/java/io/mailtrap/model/response/emaillogs/EventDetailsSpam.javasrc/main/java/io/mailtrap/model/response/emaillogs/EventDetailsUnsubscribe.javasrc/main/java/io/mailtrap/model/response/emaillogs/MessageStatus.javasrc/test/java/io/mailtrap/api/emaillogs/EmailLogsImplTest.javasrc/test/resources/api/emaillogs/getEmailLogMessageResponse.jsonsrc/test/resources/api/emaillogs/listEmailLogsResponse.json
src/main/java/io/mailtrap/model/request/emaillogs/FilterOptionalString.java
Show resolved
Hide resolved
mklocek
force-pushed
the
email-logs-api
branch
2 times, most recently
from
616ee9c to
428b090
Compare
piobeny
approved these changes
Mar 13, 2026
Contributor
Author
|
@coderabbitai review |
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.
Motivation
Add support for Email Logs API
Summary by CodeRabbit
New Features
Documentation