Implemented Contact Fields API #34
Conversation
WalkthroughAdds Contact Fields support: new API interface and implementation, enum and DTOs for request/response, Contacts API and client factory wiring, unit tests with fixtures, and a Java example demonstrating CRUD operations for contact fields. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Client App
participant ContactsApi as MailtrapContactsApi
participant CF as ContactFieldsImpl
participant HTTP as HTTP Client
participant MT as Mailtrap API
App->>ContactsApi: contactsApi().contactFields()
App->>CF: getAllContactFields(accountId)
CF->>HTTP: GET /api/accounts/{accountId}/contacts/fields
HTTP->>MT: Request
MT-->>HTTP: 200 List<ContactFieldResponse>
HTTP-->>CF: Body
CF-->>App: List<ContactFieldResponse]
App->>CF: createContactField(accountId, request)
CF->>CF: validate(request)
CF->>HTTP: POST /api/accounts/{accountId}/contacts/fields
HTTP->>MT: Request
MT-->>HTTP: 201 ContactFieldResponse
HTTP-->>CF: Body
CF-->>App: ContactFieldResponse
App->>CF: getContactField(accountId, fieldId)
CF->>HTTP: GET /api/accounts/{accountId}/contacts/fields/{fieldId}
HTTP->>MT: Request
MT-->>HTTP: 200 ContactFieldResponse
HTTP-->>CF: Body
CF-->>App: ContactFieldResponse
App->>CF: updateContactField(accountId, fieldId, request)
CF->>CF: validate(request)
CF->>HTTP: PATCH /api/accounts/{accountId}/contacts/fields/{fieldId}
HTTP->>MT: Request
MT-->>HTTP: 200 ContactFieldResponse
HTTP-->>CF: Body
CF-->>App: ContactFieldResponse
App->>CF: deleteContactField(accountId, fieldId)
CF->>HTTP: DELETE /api/accounts/{accountId}/contacts/fields/{fieldId}
HTTP->>MT: Request
MT-->>HTTP: 204 No Content
HTTP-->>CF: —
CF-->>App: void
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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: 2
🧹 Nitpick comments (18)
src/test/resources/api/contactfields/createContactFieldRequest.json (1)
1-5: Optional: add a negative-case fixture.Consider adding a fixture with an invalid data_type (e.g., "bogus") to assert proper 4xx handling and error parsing.
src/test/resources/api/contactfields/createContactFieldResponse.json (1)
1-6: Optional: include timestamp/system fields if API returns them.If Mailtrap returns created_at/updated_at or system flags, consider extending the fixture to cover them.
src/test/resources/api/contactfields/updateContactFieldRequest.json (1)
1-4: Optional: add a minimal PATCH fixture.Add a fixture updating only one field (e.g., just "name") to verify partial update behavior and null/absent field handling.
src/test/resources/api/contactfields/getContactFieldResponse.json (1)
1-6: Optional: broaden enum coverage in fixtures.Add additional GET fixtures for other data types (boolean, number, date) to exercise enum parsing across all values.
src/test/resources/api/contactfields/getAllContactFieldsResponse.json (1)
1-20: Fixture OK; verify enum case-sensitivity.Values in "data_type" are lowercase ("text", "date", "integer"). Ensure ContactFieldDataType (or ObjectMapper) accepts these (case-insensitive or @JsonCreator/@jsonvalue). Otherwise deserialization will fail in tests using this fixture.
Optionally add fixtures for edge cases (empty list, unknown data_type) to harden parsing.
src/test/java/io/mailtrap/testutils/BaseTest.java (1)
21-24: Unify boxed vs primitive types for IDs.Most IDs here use Long; these new ones use primitive long. For consistency (and to avoid overload ambiguity in helpers), consider switching to Long.
- protected final long fieldId = 1L; - protected final long getFieldId = 777L; - protected final long updateFieldId = 999L; - protected final long deleteFieldId = 1111L; + protected final Long fieldId = 1L; + protected final Long getFieldId = 777L; + protected final Long updateFieldId = 999L; + protected final Long deleteFieldId = 1111L;src/main/java/io/mailtrap/client/api/MailtrapContactsApi.java (1)
14-22: Public constructor signature change — assess compatibility.Adding contactFields and relying on @requiredargsconstructor changes the required constructor arity (3 → 4). If any external code instantiates this class directly, this is a breaking change. If construction is only via MailtrapClientFactory, you’re fine.
Consider documenting this in the changelog/README and discouraging direct instantiation in favor of the factory.
src/main/java/io/mailtrap/model/response/contactfields/ContactFieldResponse.java (1)
7-20: Make DTO resilient to server-side additions.Add @JsonIgnoreProperties(ignoreUnknown = true) to avoid failures if the API adds fields.
@Data +@com.fasterxml.jackson.annotation.JsonIgnoreProperties(ignoreUnknown = true) public class ContactFieldResponse {examples/java/io/mailtrap/examples/contactfields/ContactFields.java (3)
9-12: Remove unused imports.These aren’t used. Trim to keep the example minimal.
-import java.util.Collections; -import java.util.List; -import java.util.Map;
13-13: Rename class to avoid shadowing the API interface name.This example’s class name matches io.mailtrap.api.contactfields.ContactFields, which can confuse readers and IDEs. Consider renaming.
-public class ContactFields { +public class ContactFieldsExample {
15-16: Make the example runnable via environment variables.Reading credentials from env (with sane fallbacks) improves DX and avoids checking secrets into code.
-private static final String TOKEN = "<YOUR MAILTRAP TOKEN>"; -private static final long ACCOUNT_ID = 1L; +private static final String TOKEN = System.getenv().getOrDefault("MAILTRAP_TOKEN", "<YOUR MAILTRAP TOKEN>"); +private static final long ACCOUNT_ID = Long.parseLong(System.getenv().getOrDefault("MAILTRAP_ACCOUNT_ID", "1"));src/main/java/io/mailtrap/model/request/contactfields/UpdateContactFieldRequest.java (1)
3-8: Avoid sending nulls in PATCH payloads.Updates often need partial payloads; include only provided fields to prevent accidental nulling on the server.
import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonInclude; import io.mailtrap.model.AbstractModel; import jakarta.validation.constraints.Size; import lombok.AllArgsConstructor; import lombok.Getter; +@JsonInclude(JsonInclude.Include.NON_NULL) public class UpdateContactFieldRequest extends AbstractModel {Also applies to: 9-12
src/main/java/io/mailtrap/model/ContactFieldDataType.java (1)
25-32: Null-safe enum deserialization.Guard against null to avoid NPE during JSON binding.
@JsonCreator public static ContactFieldDataType fromValue(String value) { + if (value == null) { + throw new IllegalArgumentException("Value cannot be null"); + } for (ContactFieldDataType type : ContactFieldDataType.values()) { if (type.value.equalsIgnoreCase(value)) { return type; } } throw new IllegalArgumentException("Unknown value: " + value); }src/main/java/io/mailtrap/model/request/contactfields/CreateContactFieldRequest.java (1)
6-8: Validate non-empty fields (if API requires).If the API disallows empty name/merge_tag, add @notblank for earlier feedback. Otherwise, ignore.
import io.mailtrap.model.ContactFieldDataType; import jakarta.validation.constraints.Size; +import jakarta.validation.constraints.NotBlank; import lombok.AllArgsConstructor; import lombok.Getter; @Size(max = 80) + @NotBlank private String name; @JsonProperty("data_type") private ContactFieldDataType dataType; @Size(max = 80) + @NotBlank @JsonProperty("merge_tag") private String mergeTag;Also applies to: 14-22
src/main/java/io/mailtrap/api/contactfields/ContactFieldsImpl.java (3)
34-36: Handle null request bodies explicitly to keep exceptions consistentIf
requestis null, Bean Validation will throwIllegalArgumentException, bypassing yourInvalidRequestBodyExceptioncontract. Add a null guard so callers consistently getInvalidRequestBodyException.Preferred central fix (adjust ApiResourceWithValidation once for all resources):
// in ApiResourceWithValidation protected <T> void validateRequestBodyAndThrowException(T object) { if (object == null) { throw new InvalidRequestBodyException("Invalid request body. Violations: request must not be null"); } String violations = customValidator.validateAndGetViolationsAsString(object); if (!violations.isEmpty()) { throw new InvalidRequestBodyException("Invalid request body. Violations: " + violations); } }Also applies to: 56-57
24-28: Use String.format placeholders instead of concatenating into the format stringSafer and more readable; avoids accidental
%interpretation if it ever appears inapiHost.- String.format(apiHost + "/api/accounts/%d/contacts/fields", accountId), + String.format("%s/api/accounts/%d/contacts/fields", apiHost, accountId), - String.format(apiHost + "/api/accounts/%d/contacts/fields", accountId), + String.format("%s/api/accounts/%d/contacts/fields", apiHost, accountId), - String.format(apiHost + "/api/accounts/%d/contacts/fields/%d", accountId, fieldId), + String.format("%s/api/accounts/%d/contacts/fields/%d", apiHost, accountId, fieldId), - String.format(apiHost + "/api/accounts/%d/contacts/fields/%d", accountId, fieldId), + String.format("%s/api/accounts/%d/contacts/fields/%d", apiHost, accountId, fieldId), - String.format(apiHost + "/api/accounts/%d/contacts/fields/%d", accountId, fieldId), + String.format("%s/api/accounts/%d/contacts/fields/%d", apiHost, accountId, fieldId),Also applies to: 36-41, 46-50, 58-63, 68-72
22-29: Validate identifier arguments are positiveGuard against
accountId <= 0andfieldId <= 0early to fail fast with a clear message instead of making a network call.Example approach:
private static void requirePositive(String name, long value) { if (value <= 0) throw new IllegalArgumentException(name + " must be positive"); } // then at method entry: requirePositive("accountId", accountId); // and where applicable: requirePositive("fieldId", fieldId);Also applies to: 31-43, 53-65, 66-74
src/main/java/io/mailtrap/api/contactfields/ContactFields.java (1)
20-21: Polish Javadoc phrasing and casing (“ID”)Minor clarity/consistency tweaks.
- * Create new Contact Fields (up to 40) + * Create a new Contact Field (up to 40 total per account) - * Get Contact Field by id + * Get a Contact Field by ID - * Delete existing Contact Field. - * You cannot delete a Contact Field which is used in Automations, Email Campaigns, and in conditions of Contact Segments + * Delete an existing Contact Field. + * You cannot delete a Contact Field that is used in Automations, Email Campaigns, or in conditions of Contact Segments.Also applies to: 29-31, 47-50
📜 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 (17)
examples/java/io/mailtrap/examples/contactfields/ContactFields.java(1 hunks)src/main/java/io/mailtrap/api/contactfields/ContactFields.java(1 hunks)src/main/java/io/mailtrap/api/contactfields/ContactFieldsImpl.java(1 hunks)src/main/java/io/mailtrap/client/api/MailtrapContactsApi.java(2 hunks)src/main/java/io/mailtrap/factory/MailtrapClientFactory.java(2 hunks)src/main/java/io/mailtrap/model/ContactFieldDataType.java(1 hunks)src/main/java/io/mailtrap/model/request/contactfields/CreateContactFieldRequest.java(1 hunks)src/main/java/io/mailtrap/model/request/contactfields/UpdateContactFieldRequest.java(1 hunks)src/main/java/io/mailtrap/model/response/contactfields/ContactFieldResponse.java(1 hunks)src/test/java/io/mailtrap/api/contactfields/ContactFieldsImplTest.java(1 hunks)src/test/java/io/mailtrap/testutils/BaseTest.java(1 hunks)src/test/resources/api/contactfields/createContactFieldRequest.json(1 hunks)src/test/resources/api/contactfields/createContactFieldResponse.json(1 hunks)src/test/resources/api/contactfields/getAllContactFieldsResponse.json(1 hunks)src/test/resources/api/contactfields/getContactFieldResponse.json(1 hunks)src/test/resources/api/contactfields/updateContactFieldRequest.json(1 hunks)src/test/resources/api/contactfields/updateContactFieldResponse.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
src/main/java/io/mailtrap/api/contactfields/ContactFields.java (1)
examples/java/io/mailtrap/examples/contactfields/ContactFields.java (1)
ContactFields(13-48)
src/main/java/io/mailtrap/api/contactfields/ContactFieldsImpl.java (3)
src/main/java/io/mailtrap/CustomValidator.java (1)
CustomValidator(12-56)src/main/java/io/mailtrap/api/apiresource/ApiResourceWithValidation.java (1)
ApiResourceWithValidation(7-26)src/main/java/io/mailtrap/Constants.java (1)
Constants(6-15)
src/main/java/io/mailtrap/model/request/contactfields/UpdateContactFieldRequest.java (2)
src/main/java/io/mailtrap/model/AbstractModel.java (1)
AbstractModel(10-29)src/main/java/io/mailtrap/model/request/contactfields/CreateContactFieldRequest.java (1)
Getter(10-24)
examples/java/io/mailtrap/examples/contactfields/ContactFields.java (1)
src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
MailtrapClientFactory(30-114)
src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
src/main/java/io/mailtrap/api/contactfields/ContactFieldsImpl.java (1)
ContactFieldsImpl(15-74)
src/test/java/io/mailtrap/api/contactfields/ContactFieldsImplTest.java (4)
src/main/java/io/mailtrap/Constants.java (1)
Constants(6-15)src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
MailtrapClientFactory(30-114)src/test/java/io/mailtrap/testutils/BaseTest.java (1)
BaseTest(6-25)src/test/java/io/mailtrap/testutils/TestHttpClient.java (1)
TestHttpClient(19-160)
src/main/java/io/mailtrap/model/request/contactfields/CreateContactFieldRequest.java (2)
src/main/java/io/mailtrap/model/AbstractModel.java (1)
AbstractModel(10-29)src/main/java/io/mailtrap/model/request/contactfields/UpdateContactFieldRequest.java (1)
Getter(9-20)
src/main/java/io/mailtrap/client/api/MailtrapContactsApi.java (1)
examples/java/io/mailtrap/examples/contactfields/ContactFields.java (1)
ContactFields(13-48)
🔇 Additional comments (10)
src/test/resources/api/contactfields/createContactFieldRequest.json (1)
1-5: Remove enum casing check ContactFieldDataType uses @jsonvalue to emit its lowercase string and @JsonCreator with equalsIgnoreCase, so"boolean"will deserialize correctly.src/test/resources/api/contactfields/createContactFieldResponse.json (1)
1-6: LGTM: response fixture looks consistent with request and model.id/name/data_type/merge_tag fields align with expected DTO shape.
src/test/resources/api/contactfields/updateContactFieldResponse.json (2)
1-6: LGTM: update response fixture is coherent.Values and id=999 match typical update flow used in tests.
1-6: Ensure data_type is omitted in request and preserved in response
- Confirmed existing tests load
updateContactFieldRequest.jsonwithout adata_typefield and assert onContactFieldDataType.TEXTin response.- Verify
UpdateContactFieldRequestclass uses@JsonInclude(Include.NON_NULL)(or equivalent) so nulldataTypeisn’t serialized.- No further code changes needed; tests already cover partial-update behavior.
src/test/resources/api/contactfields/updateContactFieldRequest.json (1)
1-4: merge_tag update supported; no change needed. TheUpdateContactFieldRequestmodel includesmerge_tagwithout immutability constraints, the service simply PATCHes the request (includingmerge_tag), and the Javadoc only forbids changingdata_type, notmerge_tag. Tests and fixtures remain correct.src/test/resources/api/contactfields/getContactFieldResponse.json (1)
1-6: LGTM: get-by-id fixture is clear and consistent.Fields map cleanly to ContactFieldResponse expectations.
src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (2)
9-9: Import addition is correct.
55-62: Wiring ContactFields into Contacts API looks correct; check for ripple effects.Instantiation and injection of ContactFieldsImpl align with other resources; constructor call updated to 4-arg form.
Please confirm there are no remaining call sites (tests/examples) using the old 3-arg MailtrapContactsApi constructor.
src/main/java/io/mailtrap/model/response/contactfields/ContactFieldResponse.java (1)
14-16: Confirm enum mapping matches payload values.Ensure ContactFieldDataType supports "text"/"date"/"integer" (lowercase) from fixtures (case-insensitive or custom @JsonCreator). Otherwise Jackson will throw an exception on getAll/getById responses.
src/test/java/io/mailtrap/api/contactfields/ContactFieldsImplTest.java (1)
51-58: Tests look solid.Good coverage of list/get happy path and type assertions.
| @Test | ||
| void test_deleteContactField() { | ||
| assertDoesNotThrow(() -> api.deleteContactField(accountId, deleteFieldId)); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm DELETE wiring uses Void.class in HTTP client.
Your TestHttpClient skips response body only when response type is Void.class. Ensure implementation passes Void.class to delete to match the mock.
🏁 Script executed:
#!/bin/bash
# Verify that ContactFieldsImpl.deleteContactField(...) passes Void.class to httpClient.delete(...)
set -euo pipefail
file_path="$(fd -a 'ContactFieldsImpl.java')"
echo "File: $file_path"
rg -n -C2 -P '\bdeleteContactField\s*\(' "$file_path" || true
rg -n -C2 -P 'httpClient\.delete\([^,]+,\s*[^,]+,\s*Void\.class\)' "$file_path" || { echo "Expected Void.class in delete call not found"; exit 1; }
echo "OK: Void.class used in delete call."Length of output: 942
The implementation of deleteContactField in ContactFieldsImpl does not pass Void.class into the httpClient.delete call. You need to update it to:
httpClient.delete(
String.format(apiHost + "/api/accounts/%d/contacts/fields/%d", accountId, fieldId),
Void.class
);So the mock in your tests will correctly recognize the response type.
🤖 Prompt for AI Agents
In src/test/java/io/mailtrap/api/contactfields/ContactFieldsImplTest.java around
lines 89 to 92, the test expects the ContactFieldsImpl.deleteContactField
implementation to call httpClient.delete with a Void response type; update
ContactFieldsImpl.deleteContactField to pass Void.class as the second argument
to httpClient.delete (using the same formatted URL for accountId and fieldId) so
the mocked call in tests matches the expected response type.
| import com.fasterxml.jackson.annotation.JsonCreator; | ||
| import com.fasterxml.jackson.annotation.JsonValue; | ||
|
|
||
| public enum ContactFieldDataType { |
There was a problem hiding this comment.
Minor: seems like a wrong indentation
| public enum ContactFieldDataType { | |
| public enum ContactFieldDataType { |
3 participants
Motivation
Next steps implementing Mailtrap API - Contact Fields
Changes
Summary by CodeRabbit
New Features
Documentation
Tests