Implemented Suppressions API #35
Conversation
WalkthroughIntroduces a new Suppressions API: interface, implementation, models, client wiring, example usage, and tests. MailtrapEmailSendingApi now exposes suppressions(), wired via MailtrapClientFactory. Supports searching suppressions by email and deleting a suppression by ID. Adds DTOs/enums for JSON mapping and test fixtures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Application
participant Factory as MailtrapClientFactory
participant Client as MailtrapEmailSendingApi
participant Supp as SuppressionsImpl
participant HTTP as HTTP API
App->>Factory: createMailtrapClient(config)
Factory->>Client: new MailtrapEmailSendingApi(emails, domains, suppressions)
App->>Client: suppressions()
App->>Supp: search(accountId, email)
Supp->>HTTP: GET /api/accounts/{id}/suppressions?email={email}
HTTP-->>Supp: 200 [SuppressionsResponse...]
Supp-->>App: List<SuppressionsResponse>
App->>Supp: deleteSuppression(accountId, suppressionId)
Supp->>HTTP: DELETE /api/accounts/{id}/suppressions/{encodedId}
HTTP-->>Supp: 200 SuppressionsResponse
Supp-->>App: SuppressionsResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (15)
src/test/resources/api/suppressions/deleteSuppression.json (1)
1-16: Fixture looks good; tiny evergreen tweak optionalConsider using a past timestamp (e.g., 2024-01-01T00:00:00Z) to avoid “future” dates when reading logs; no impact on parsing.
src/test/java/io/mailtrap/testutils/BaseTest.java (1)
24-26: Use a deterministic charset and avoid form-encoding for path segments
- Prefer StandardCharsets.UTF_8 over Charset.defaultCharset() for consistent results.
- Since suppressionId is a UUID used in a path segment, encoding isn’t necessary; if you do encode, prefer a path-segment encoder over URLEncoder (which is for x-www-form-urlencoded). For this test it’s harmless, but worth aligning.
Apply within-range change:
- protected final String suppressionIdEncoded = URLEncoder.encode(suppressionId, Charset.defaultCharset()); + protected final String suppressionIdEncoded = URLEncoder.encode(suppressionId, java.nio.charset.StandardCharsets.UTF_8);Also consider updating similar usages for emailEncoded/contactUUIDEncoded above to StandardCharsets.UTF_8.
src/main/java/io/mailtrap/model/response/suppressions/SendingStream.java (1)
22-30: Harden fromValue against null/whitespace and be explicit about @JsonCreator modeAvoid NPE on null JSON and accept surrounding whitespace.
- @JsonCreator - public static SendingStream fromValue(String value) { - for (SendingStream type : SendingStream.values()) { - if (type.value.equalsIgnoreCase(value)) { - return type; - } - } - throw new IllegalArgumentException("Unknown value: " + value); - } + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + public static SendingStream fromValue(String value) { + if (value == null || value.isBlank()) { + throw new IllegalArgumentException("Unknown value: " + value); + } + final String v = value.trim(); + for (SendingStream type : SendingStream.values()) { + if (type.value.equalsIgnoreCase(v)) { + return type; + } + } + throw new IllegalArgumentException("Unknown value: " + value); + }src/main/java/io/mailtrap/model/response/suppressions/SuppressionType.java (1)
23-31: Same robustness improvements as SendingStream.fromValueMirror null/whitespace handling and explicit creator mode for consistency.
- @JsonCreator - public static SuppressionType fromValue(String value) { - for (SuppressionType type : SuppressionType.values()) { - if (type.value.equalsIgnoreCase(value)) { - return type; - } - } - throw new IllegalArgumentException("Unknown value: " + value); - } + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + public static SuppressionType fromValue(String value) { + if (value == null || value.isBlank()) { + throw new IllegalArgumentException("Unknown value: " + value); + } + final String v = value.trim(); + for (SuppressionType type : SuppressionType.values()) { + if (type.value.equalsIgnoreCase(v)) { + return type; + } + } + throw new IllegalArgumentException("Unknown value: " + value); + }src/test/resources/api/suppressions/searchSuppressions.json (1)
1-18: Fixture aligns with enums; optional timestamp nitValues map cleanly to SuppressionType.SPAM_COMPLAINT and SendingStream.BULK. Consider past timestamps for evergreen readability as noted on the delete fixture.
examples/java/io/mailtrap/examples/suppressions/Suppressions.java (3)
8-15: Avoid hardcoding secrets; read token from an env varKeeps examples safe while still runnable.
- private static final String TOKEN = "<YOUR MAILTRAP TOKEN>"; + private static final String TOKEN = + System.getenv().getOrDefault("MAILTRAP_TOKEN", "<YOUR MAILTRAP TOKEN>");
6-6: Rename example class to avoid name collision with API interfacePrevents import confusion when examples expand.
-public class Suppressions { +public class SuppressionsExample {
24-29: Guard destructive delete in exampleMake deletion explicit to avoid accidental data changes when copy-running.
- if (!searchResponse.isEmpty()) { + if (Boolean.parseBoolean(System.getenv().getOrDefault("DELETE_SUPPRESSION", "false")) + && !searchResponse.isEmpty()) {src/main/java/io/mailtrap/api/suppressions/Suppressions.java (2)
9-17: Clarify nullability/behavior ofImplementation accepts null (omits the param). Make the contract explicit.
- /** - * List and search suppressions by email. The endpoint returns up to 1000 suppressions per request. - * - * @param accountId - unique account ID - * @param email - search suppressions for this email - * @return a list of suppressions - */ + /** + * List and search suppressions. The endpoint returns up to 1000 suppressions per request. + * + * @param accountId - unique account ID + * @param email - optional; if null, returns all suppressions for the account; if provided, filters by email + * @return a list of suppressions + */
16-16: Optional overload for discoverabilityConsider an overload
List<SuppressionsResponse> search(long accountId)to make “list all” explicit, delegating to the existing method withnull.src/test/java/io/mailtrap/api/suppressions/SuppressionsImplTest.java (1)
21-45: Add a test for special-character encoding and for “list all”To future-proof encoding and optional email behavior, add:
- a test with an email containing “+” or spaces
- a test with
email = null(no query param expected)Example (non-diff, new tests):
@Test void test_search_withSpecialCharsEmail() { String trickyEmail = "first.last+tag@mailtrap.io"; TestHttpClient httpClient = new TestHttpClient(List.of( DataMock.build( Constants.GENERAL_HOST + "/api/accounts/" + accountId + "/suppressions", "GET", null, "api/suppressions/searchSuppressions.json", Map.of("email", trickyEmail) ) )); // build api with httpClient and assert like in test_search } @Test void test_search_listAll_whenEmailNull() { TestHttpClient httpClient = new TestHttpClient(List.of( DataMock.build( Constants.GENERAL_HOST + "/api/accounts/" + accountId + "/suppressions", "GET", null, "api/suppressions/searchSuppressions.json" ) )); // build api with httpClient and assert not empty, etc. }Also consider standardizing encoding on UTF‑8 in both BaseTest and SuppressionsImpl to avoid locale-specific defaults.
src/main/java/io/mailtrap/api/suppressions/SuppressionsImpl.java (3)
36-36: Validate suppressionId early.Fail fast on null/blank IDs to avoid confusing 4xx from the server.
@Override public SuppressionsResponse deleteSuppression(long accountId, String suppressionId) { + if (suppressionId == null || suppressionId.isBlank()) { + throw new IllegalArgumentException("suppressionId must not be null or blank"); + } return
25-25: Don’t send emptyTrim and drop when blank.
- var queryParams = RequestData.buildQueryParams(entry("email", Optional.ofNullable(email))); + var queryParams = RequestData.buildQueryParams( + entry("email", Optional.ofNullable(email).map(String::trim).filter(s -> !s.isEmpty())) + );
29-29: Style: avoid concatenation inside String.format.Clearer and avoids accidental double slashes.
- String.format(apiHost + "/api/accounts/%d/suppressions", accountId), + String.format("%s/api/accounts/%d/suppressions", apiHost, accountId),src/main/java/io/mailtrap/model/response/suppressions/SuppressionsResponse.java (1)
3-6: Harden JSON mapping: ignore unknowns and omit nulls.More resilient to backend additions and cleaner (re-)serialization.
import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; import lombok.Data; @@ -@Data +@JsonIgnoreProperties(ignoreUnknown = true) +@JsonInclude(JsonInclude.Include.NON_NULL) +@Data public class SuppressionsResponse {Also applies to: 8-9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
examples/java/io/mailtrap/examples/suppressions/Suppressions.java(1 hunks)src/main/java/io/mailtrap/api/suppressions/Suppressions.java(1 hunks)src/main/java/io/mailtrap/api/suppressions/SuppressionsImpl.java(1 hunks)src/main/java/io/mailtrap/client/api/MailtrapEmailSendingApi.java(2 hunks)src/main/java/io/mailtrap/factory/MailtrapClientFactory.java(2 hunks)src/main/java/io/mailtrap/model/response/suppressions/SendingStream.java(1 hunks)src/main/java/io/mailtrap/model/response/suppressions/SuppressionType.java(1 hunks)src/main/java/io/mailtrap/model/response/suppressions/SuppressionsResponse.java(1 hunks)src/test/java/io/mailtrap/api/suppressions/SuppressionsImplTest.java(1 hunks)src/test/java/io/mailtrap/testutils/BaseTest.java(1 hunks)src/test/resources/api/suppressions/deleteSuppression.json(1 hunks)src/test/resources/api/suppressions/searchSuppressions.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
examples/java/io/mailtrap/examples/suppressions/Suppressions.java (1)
src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
MailtrapClientFactory(31-116)
src/main/java/io/mailtrap/api/suppressions/Suppressions.java (1)
examples/java/io/mailtrap/examples/suppressions/Suppressions.java (1)
Suppressions(6-31)
src/main/java/io/mailtrap/api/suppressions/SuppressionsImpl.java (2)
src/main/java/io/mailtrap/Constants.java (1)
Constants(6-15)src/main/java/io/mailtrap/api/apiresource/ApiResource.java (1)
ApiResource(10-34)
src/test/java/io/mailtrap/api/suppressions/SuppressionsImplTest.java (4)
src/main/java/io/mailtrap/Constants.java (1)
Constants(6-15)src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
MailtrapClientFactory(31-116)src/test/java/io/mailtrap/testutils/BaseTest.java (1)
BaseTest(6-26)src/test/java/io/mailtrap/testutils/TestHttpClient.java (1)
TestHttpClient(19-160)
src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
src/main/java/io/mailtrap/api/suppressions/SuppressionsImpl.java (1)
SuppressionsImpl(16-44)
src/main/java/io/mailtrap/client/api/MailtrapEmailSendingApi.java (1)
examples/java/io/mailtrap/examples/suppressions/Suppressions.java (1)
Suppressions(6-31)
🔇 Additional comments (6)
src/main/java/io/mailtrap/client/api/MailtrapEmailSendingApi.java (2)
17-20: LGTM: clean wiring and fluent accessor for suppressions()Field addition aligns with existing Lombok patterns; accessor shape is consistent with emails() and domains().
15-20: No legacy usages found – constructor updated correctly. Only instantiation inMailtrapClientFactory(line 79) already uses all three parameters(emails, domains, suppressions).src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
74-80: LGTM: Suppressions wired into Sending APIInstantiation and injection look correct; host selection is handled in SuppressionsImpl.
src/test/java/io/mailtrap/api/suppressions/SuppressionsImplTest.java (2)
47-56: LGTM: search flow covered with field-level assertionsGood coverage of id/email/stream/type; mock query param matching protects against regressions.
58-67: LGTM: delete flow verifies ID round‑trip and enum mappingNicely validates path handling and response mapping.
src/main/java/io/mailtrap/api/suppressions/SuppressionsImpl.java (1)
18-21: No change required: SuppressionsImpl host is correct SuppressionsImpl and its tests consistently use Constants.GENERAL_HOST like other non-email APIs; centralizing host configuration remains an optional improvement.
| import java.net.URLEncoder; | ||
| import java.nio.charset.Charset; | ||
| import java.util.List; |
There was a problem hiding this comment.
Encode path segment correctly and use UTF-8 (URLEncoder here is incorrect).
URLEncoder is for form/query encoding and will turn spaces into '+', which is wrong for path segments. Also avoid platform-default charset.
Apply:
-import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;- String.format(apiHost + "/api/accounts/%d/suppressions/%s", accountId, URLEncoder.encode(suppressionId, Charset.defaultCharset())),
+ String.format(apiHost + "/api/accounts/%d/suppressions/%s", accountId, encodePathSegment(suppressionId)),Add this helper inside the class:
private static String encodePathSegment(String segment) {
return URLEncoder.encode(segment, StandardCharsets.UTF_8).replace("+", "%20");
}Also applies to: 39-39
🤖 Prompt for AI Agents
In src/main/java/io/mailtrap/api/suppressions/SuppressionsImpl.java around lines
9-11 (and also line 39), the code imports and uses URLEncoder with the
platform-default Charset which is incorrect for path segments and may produce
'+' for spaces; replace usages with a dedicated helper that URL-encodes using
StandardCharsets.UTF_8 and converts '+' to '%20', and add that helper as a
private static method inside the class (named e.g. encodePathSegment) so all
path segment encodings call it instead of raw URLEncoder calls.
There was a problem hiding this comment.
@vitalii-t Out of curiosity, how valid is this AI suggestion for modern Java?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@vittorius, It is a valid workaround, but since the suppressionId here is a UUID, encoding is not necessary and is done "just in case."
In general, this AI bot suggests valid things, especially in the "nitpicks" section. I collect some of them, and will edit for all APIs - like adding @JsonIgnoreProperties(ignoreUnknown = true) for the response classes, null checks for enums, and so on - some polishing stuff
| import java.net.URLEncoder; | ||
| import java.nio.charset.Charset; | ||
| import java.util.List; |
There was a problem hiding this comment.
@vitalii-t Out of curiosity, how valid is this AI suggestion for modern Java?
Motivation
Next steps implementing Mailtrap API - Suppressions
Changes
Summary by CodeRabbit
New Features
Documentation
Tests