-
Notifications
You must be signed in to change notification settings - Fork 0
Implement Contact Exports API #41
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
WalkthroughAdds a contact-exports feature: new API interface and implementation, request/response models and enums, client wiring updates, an example, unit tests, and JSON fixtures for create/get contact export endpoints. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Factory as MailtrapClientFactory
participant ContactsApi as MailtrapContactsApi
participant Exports as ContactExportsImpl
participant HTTP as Mailtrap HTTP API
Dev->>Factory: build(config)
Factory->>ContactsApi: construct(..., ContactExportsImpl)
ContactsApi-->>Dev: contactsApi
rect rgba(173,216,230,0.12)
Dev->>ContactsApi: contactExports().createContactExport(accountId, request)
ContactsApi->>Exports: createContactExport(accountId, request)
Exports->>HTTP: POST /api/accounts/{id}/contacts/exports
HTTP-->>Exports: 201 ContactExportResponse
Exports-->>ContactsApi: ContactExportResponse
ContactsApi-->>Dev: ContactExportResponse
end
rect rgba(220,248,198,0.12)
Dev->>ContactsApi: contactExports().getContactExport(accountId, exportId)
ContactsApi->>Exports: getContactExport(accountId, exportId)
Exports->>HTTP: GET /api/accounts/{id}/contacts/exports/{exportId}
HTTP-->>Exports: 200 ContactExportResponse
Exports-->>ContactsApi: ContactExportResponse
ContactsApi-->>Dev: ContactExportResponse
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
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.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src/test/resources/api/contactexports/getContactExportResponse.json (1)
1-7: Consider adding a “finished” response case.Add a second fixture with status "finished" and a non-null url to exercise final-state mapping.
src/test/resources/api/contactexports/createContactExportResponse.json (1)
1-7: Vary creation vs retrieval fixtures to broaden coverage.Consider using a different status (e.g., "queued" or "finished") here to validate enum and URL handling across states.
src/main/java/io/mailtrap/model/request/contactexports/CreateContactsExportRequest.java (1)
9-14: Optional: add a builder (with singular) for easier construction.Improves call-site ergonomics without breaking existing ctor usage.
package io.mailtrap.model.request.contactexports; import io.mailtrap.model.AbstractModel; +import lombok.Builder; import lombok.AllArgsConstructor; import lombok.Getter; import java.util.List; @Getter @AllArgsConstructor +@Builder public class CreateContactsExportRequest extends AbstractModel { - private List<ContactExportFilter> filters; + @lombok.Singular + private List<ContactExportFilter> filters; }src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilterName.java (1)
5-9: Remove unnecessary @requiredargsconstructor annotation.The
@RequiredArgsConstructorannotation is redundant here because the enum has no fields. Enums automatically provide constructors for their constants, and Lombok's@RequiredArgsConstructorgenerates a no‑arg constructor when there are no final fields, which adds no value.Apply this diff:
-@RequiredArgsConstructor public enum ContactExportFilterName { subscription_status, list_idsrc/main/java/io/mailtrap/model/request/contactexports/ContactExportFilterSubscriptionStatus.java (1)
6-6: Remove unused import.The
java.util.Arraysimport is not used in this file.Apply this diff:
import lombok.RequiredArgsConstructor; -import java.util.Arrays; - @Gettersrc/main/java/io/mailtrap/model/request/contactexports/ContactExportFilter.java (1)
5-6: Consider using getValue() pattern for filter names.Lines 22 and 26 use
enum.name()to obtain filter name strings ("list_id", "subscription_status"). While this works, it tightly couples the enum constant names to the API contract. If enum constants are renamed, the API contract breaks.The
ContactExportFilterOperatorandContactExportFilterSubscriptionStatusenums usegetValue()methods (see lines 22, 26-27 where they're called). Consider applying the same pattern toContactExportFilterNamefor consistency and encapsulation:// In ContactExportFilterName enum list_id("list_id"), subscription_status("subscription_status"); private final String value; // ... constructor and getValue()Then use:
list_id.getValue() // instead of list_id.name()This is optional if the enum names are carefully managed to match API requirements.
Also applies to: 21-27
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
examples/java/io/mailtrap/examples/contactexports/ContactExportsExample.java(1 hunks)src/main/java/io/mailtrap/api/contactexports/ContactExports.java(1 hunks)src/main/java/io/mailtrap/api/contactexports/ContactExportsImpl.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/request/contactexports/ContactExportFilter.java(1 hunks)src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilterName.java(1 hunks)src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilterOperator.java(1 hunks)src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilterSubscriptionStatus.java(1 hunks)src/main/java/io/mailtrap/model/request/contactexports/CreateContactsExportRequest.java(1 hunks)src/main/java/io/mailtrap/model/response/contactexports/ContactExportResponse.java(1 hunks)src/main/java/io/mailtrap/model/response/contactexports/ContactExportStatus.java(1 hunks)src/test/java/io/mailtrap/api/contactexports/ContactExportsImplTest.java(1 hunks)src/test/java/io/mailtrap/testutils/BaseTest.java(1 hunks)src/test/resources/api/contactexports/createContactExportRequest.json(1 hunks)src/test/resources/api/contactexports/createContactExportResponse.json(1 hunks)src/test/resources/api/contactexports/getContactExportResponse.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
src/main/java/io/mailtrap/model/request/contactexports/CreateContactsExportRequest.java (3)
src/main/java/io/mailtrap/model/AbstractModel.java (1)
AbstractModel(10-29)src/main/java/io/mailtrap/model/request/sendingdomains/SendingDomainsSetupInstructionsRequest.java (1)
AllArgsConstructor(7-13)src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilter.java (1)
Getter(8-28)
examples/java/io/mailtrap/examples/contactexports/ContactExportsExample.java (3)
src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
MailtrapClientFactory(33-130)examples/java/io/mailtrap/examples/contacts/ContactsExample.java (2)
ContactsExample(14-49)main(23-48)examples/java/io/mailtrap/examples/contactimports/ContactImportsExample.java (2)
ContactImportsExample(11-39)main(19-38)
src/main/java/io/mailtrap/api/contactexports/ContactExportsImpl.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-32)
src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilterOperator.java (2)
src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilter.java (1)
Getter(8-28)src/main/java/io/mailtrap/model/request/contactexports/CreateContactsExportRequest.java (1)
Getter(9-14)
src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilterSubscriptionStatus.java (2)
src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilter.java (1)
Getter(8-28)src/main/java/io/mailtrap/model/request/contactexports/CreateContactsExportRequest.java (1)
Getter(9-14)
src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilter.java (2)
src/main/java/io/mailtrap/client/api/MailtrapContactsApi.java (1)
Getter(15-24)src/main/java/io/mailtrap/model/request/contactexports/CreateContactsExportRequest.java (1)
Getter(9-14)
src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
src/main/java/io/mailtrap/api/contactexports/ContactExportsImpl.java (1)
ContactExportsImpl(10-35)
src/test/java/io/mailtrap/api/contactexports/ContactExportsImplTest.java (4)
src/main/java/io/mailtrap/Constants.java (1)
Constants(6-15)src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
MailtrapClientFactory(33-130)src/test/java/io/mailtrap/testutils/BaseTest.java (1)
BaseTest(6-29)src/test/java/io/mailtrap/testutils/TestHttpClient.java (1)
TestHttpClient(19-160)
🔇 Additional comments (16)
src/test/resources/api/contactexports/createContactExportRequest.json (1)
1-16: Fixture shape matches request model.Values and operators align with ContactExportFilter factories. No issues.
src/test/java/io/mailtrap/testutils/BaseTest.java (1)
21-22: LGTM.New IDs align with fixtures and improve test readability.
src/main/java/io/mailtrap/factory/MailtrapClientFactory.java (1)
73-81: Wiring looks correct.ContactExportsImpl is instantiated and passed into MailtrapContactsApi as expected. No issues spotted.
src/main/java/io/mailtrap/client/api/MailtrapContactsApi.java (1)
15-24: Constructor change safe — only the internal factory instantiates this class.
No externalnew MailtrapContactsApi(...)usages found; binary compatibility is preserved.src/main/java/io/mailtrap/api/contactexports/ContactExportsImpl.java (2)
18-25: LGTM!The method correctly constructs the URL and delegates to the HTTP client. The implementation follows the established pattern used in other API classes.
28-34: LGTM!The method correctly constructs the URL and delegates to the HTTP client. The implementation follows the established pattern used in other API classes.
src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilterOperator.java (1)
6-12: LGTM!The enum is well‑defined with a single
EQUALoperator and uses appropriate Lombok annotations for the value field.src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilterSubscriptionStatus.java (1)
8-15: LGTM!The enum is well‑defined with appropriate constants and uses Lombok annotations correctly.
src/main/java/io/mailtrap/api/contactexports/ContactExports.java (1)
6-25: LGTM!The interface is well‑defined with clear method signatures and comprehensive Javadoc. It follows the established pattern used in other API interfaces in the codebase.
src/main/java/io/mailtrap/model/response/contactexports/ContactExportResponse.java (1)
6-21: LGTM!The response DTO is well‑structured with appropriate Jackson annotations for JSON mapping and uses Lombok's
@Datafor standard accessors. The implementation follows the established pattern used in other response models.src/main/java/io/mailtrap/model/response/contactexports/ContactExportStatus.java (1)
7-30: LGTM!The enum is well‑implemented with proper Jackson annotations for JSON (de)serialization. The
fromValuemethod correctly handles case‑insensitive matching and throws an appropriate exception for unknown values. This follows the established pattern used in the codebase.src/test/java/io/mailtrap/api/contactexports/ContactExportsImplTest.java (3)
1-43: LGTM! Test structure follows established patterns.The test setup is well-structured and consistent with other API tests in the codebase. The mock configuration correctly maps POST and GET endpoints with appropriate request/response fixtures.
58-64: LGTM! Clean and focused test.The GET endpoint test is straightforward and correctly validates the response structure.
45-56: Resolved varargs serialization format: The mock fixture’svalueis[101], matching the varargs output. No changes needed.src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilter.java (2)
8-19: LGTM! Clean immutable value object design.The class follows best practices with:
- Final fields ensuring immutability
- Private constructor restricting instantiation to factory methods
- Lombok
@Getterfor concise accessor generation- Generic
Objecttype for value allowing different filter types
21-23: Varargs design allows flexible ID filtering.The
ids()method acceptsLong... ids(varargs), meaning:
- Single value:
ids(op, 101L)→ creates array[101L]- Multiple values:
ids(op, 101L, 102L)→ creates array[101L, 102L]- Array:
ids(op, new Long[]{101L, 102L})→ passes array throughThis design is appropriate for the method name (plural "ids"), enabling both single and multiple ID filters consistently.
examples/java/io/mailtrap/examples/contactexports/ContactExportsExample.java
Outdated
Show resolved
Hide resolved
examples/java/io/mailtrap/examples/contactexports/ContactExportsExample.java
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilterName.java (1)
3-6: Consider adding Javadoc to document the enum's purpose.The enum is functional and likely uses snake_case naming intentionally to match the API specification. However, adding Javadoc would improve maintainability by explaining the purpose of this enum and clarifying the unconventional naming convention.
Example:
+/** + * Predefined filter names for contact export requests. + * <p> + * Note: Enum constants use snake_case to match the Mailtrap API field names. + */ public enum ContactExportFilterName { + /** Filter by subscription status */ subscription_status, + /** Filter by list ID */ list_id }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/java/io/mailtrap/examples/contactexports/ContactExportsExample.java(1 hunks)src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilterName.java(1 hunks)src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilterSubscriptionStatus.java(1 hunks)src/test/java/io/mailtrap/api/contactexports/ContactExportsImplTest.java(1 hunks)src/test/resources/api/contactexports/getContactExportResponse.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/test/resources/api/contactexports/getContactExportResponse.json
- src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilterSubscriptionStatus.java
- src/test/java/io/mailtrap/api/contactexports/ContactExportsImplTest.java
- examples/java/io/mailtrap/examples/contactexports/ContactExportsExample.java
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 looks good, I'd just use more explicit naming for clarity
examples/java/io/mailtrap/examples/contactexports/ContactExportsExample.java
Outdated
Show resolved
Hide resolved
src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilter.java
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/test/java/io/mailtrap/api/contactexports/ContactExportsImplTest.java (2)
55-55: PreferassertEqualsfor enum comparison.While
assertSameworks for enums due to singleton instances,assertEqualsis more idiomatic and explicitly conveys value equality rather than identity checking.Apply this diff:
- assertSame(ContactExportStatus.STARTED, contactExportResponse.getStatus()); + assertEquals(ContactExportStatus.STARTED, contactExportResponse.getStatus());
63-63: PreferassertEqualsfor enum comparison.As with the previous test,
assertEqualsis more idiomatic thanassertSamefor comparing enum values.Apply this diff:
- assertSame(ContactExportStatus.FINISHED, contactExport.getStatus()); + assertEquals(ContactExportStatus.FINISHED, contactExport.getStatus());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/java/io/mailtrap/examples/contactexports/ContactExportsExample.java(1 hunks)src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilter.java(1 hunks)src/test/java/io/mailtrap/api/contactexports/ContactExportsImplTest.java(1 hunks)src/test/java/io/mailtrap/testutils/BaseTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/java/io/mailtrap/examples/contactexports/ContactExportsExample.java
- src/test/java/io/mailtrap/testutils/BaseTest.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/test/java/io/mailtrap/api/contactexports/ContactExportsImplTest.java (3)
src/main/java/io/mailtrap/Constants.java (1)
Constants(6-15)src/test/java/io/mailtrap/testutils/BaseTest.java (1)
BaseTest(6-29)src/test/java/io/mailtrap/testutils/TestHttpClient.java (1)
TestHttpClient(19-160)
src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilter.java (2)
src/main/java/io/mailtrap/client/api/MailtrapContactsApi.java (1)
Getter(15-24)src/main/java/io/mailtrap/model/request/contactexports/CreateContactsExportRequest.java (1)
Getter(9-14)
🔇 Additional comments (4)
src/test/java/io/mailtrap/api/contactexports/ContactExportsImplTest.java (1)
27-43: LGTM!The test setup correctly initializes the mock HTTP client with the expected endpoints and obtains the ContactExports API instance through the proper factory chain.
src/main/java/io/mailtrap/model/request/contactexports/ContactExportFilter.java (3)
21-23: LGTM!The parameter naming clearly indicates these are list IDs, addressing the previous review feedback. The factory method correctly constructs the filter using the enum name and operator value.
25-27: LGTM!The factory method correctly constructs the subscription status filter, following the same pattern as
listIDs()and properly extracting the enum value.
8-19: Ignore AbstractModel extension for nested models. ContactExportFilter is a plain POJO like other nested request types (e.g., Contact, Permission) and is correctly serialized by Jackson without extending AbstractModel.Likely an incorrect or invalid review comment.
Motivation
Next steps implementing Mailtrap API - Contact Exports
Changes
Summary by CodeRabbit
New Features
Documentation
Tests