Feature - Contacts API (#126) #139
Conversation
- Added contacts management interfaces, models, requests, and validation - Added Unit Tests for Contact and Contacts Collection models - Added Integration tests for Contacts API - Added example project to deminstrate Contacts API usage - Implemented new Delete method which returns only Status code into Rest model
dr-3lo
requested review from
zhaparoff,
mklocek and
vittorius
as code owners
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ 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. 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: 11
🧹 Nitpick comments (40)
examples/Mailtrap.Example.Contact/appsettings.json (1)
14-16: Avoid committing real API keys; prefer env vars or user-secrets.
Keep the placeholder, but document using Mailtrap__ApiToken env var ordotnet user-secretsfor local runs to prevent accidental commits of secrets.I can add a short README snippet showing
dotnet user-secrets set "Mailtrap:ApiToken" "<API_KEY>".src/Mailtrap.Abstractions/Contacts/Converters/DateTimeToTicksJsonConverter.cs (1)
6-6: Optional: rename class to reflect units.
Consider renaming to DateTimeToUnixMillisecondsJsonConverter for clarity. Low priority.tests/Mailtrap.IntegrationTests/Contacts/GetAll_Success.json (1)
15-16: Timestamp units: ensure converter matches epoch-millisThese values look like Unix milliseconds. Verify DateTimeToTicksJsonConverter actually reads/writes Unix ms (not .NET ticks). If it’s Unix ms, consider renaming the converter for clarity.
Also applies to: 31-32, 46-47
src/Mailtrap/GlobalUsings.cs (1)
31-34: Consider adding Contacts.Responses for parityOther feature areas expose *.Responses globally; adding Contacts.Responses can reduce file-level usings and keep consistency.
Apply this diff:
global using Mailtrap.Contacts; global using Mailtrap.Contacts.Models; global using Mailtrap.Contacts.Requests; global using Mailtrap.Contacts.Validators; +global using Mailtrap.Contacts.Responses;src/Mailtrap.Abstractions/GlobalUsings.cs (1)
24-29: Scope and de-duplicate Contacts global usings
- Remove or consolidate duplicated global usings (Contacts, Contacts.Models, Contacts.Requests, Contacts.Validators) between src/Mailtrap.Abstractions/GlobalUsings.cs and src/Mailtrap/GlobalUsings.cs.
- Verify whether Contacts.Converters and Contacts.Responses are actually used across the Abstractions assembly; if not, move them to the individual files that need them.
src/Mailtrap/Core/Rest/Commands/DeleteWithStatusCodeResultRestResourceCommand.cs (1)
4-10: Remove unused generic type parameter
TResponseis never used in this command. Dropping it simplifies the type and prevents confusion.-internal sealed class DeleteWithStatusCodeResultRestResourceCommand<TResponse> : DeleteRestResourceCommand<HttpStatusCode> +internal sealed class DeleteWithStatusCodeResultRestResourceCommand : DeleteRestResourceCommand<HttpStatusCode>If the factory currently exposes
CreateDeleteWithStatusCodeResult<TResponse>(Uri), consider changing it to a non-generic:// In IRestResourceCommandFactory / RestResourceCommandFactory IResourceCommand<HttpStatusCode> CreateDeleteWithStatusCodeResult(Uri resourceUri);src/Mailtrap/Core/Rest/IRestResourceCommandFactory.cs (1)
10-10: Drop unused generic from CreateDeleteWithStatusCodeResult
- Remove the
<TResponse>parameter from the interface and implementation signatures in IRestResourceCommandFactory.cs and RestResourceCommandFactory.cs, and update the factory call tonew DeleteWithStatusCodeResultRestResourceCommand(_httpClientProvider, resourceUri)- Remove the
<HttpStatusCode>type argument from all call sites (e.g., in RestResource.cs) so consumers simply callCreateDeleteWithStatusCodeResult(resourceUri).src/Mailtrap/Core/Rest/RestResource.cs (1)
64-68: Align call site with non-generic factory (if you drop the generic)Once the factory is non-generic, update this invocation accordingly.
Apply this diff:
- => await RestResourceCommandFactory - .CreateDeleteWithStatusCodeResult<HttpStatusCode>(ResourceUri) + => await RestResourceCommandFactory + .CreateDeleteWithStatusCodeResult(ResourceUri) .Execute(cancellationToken) .ConfigureAwait(false);src/Mailtrap/Core/Rest/RestResourceCommandFactory.cs (1)
56-61: Remove superfluous generic on delete-with-status-code factoryMirror
CreatePostWithStatusCodeResult<TRequest>(generic on request only). The delete variant has no request payload, so the generic adds noise without value.Apply this diff:
- public IRestResourceCommand<HttpStatusCode> CreateDeleteWithStatusCodeResult<TResponse>(Uri resourceUri) - => new DeleteWithStatusCodeResultRestResourceCommand<TResponse>( + public IRestResourceCommand<HttpStatusCode> CreateDeleteWithStatusCodeResult(Uri resourceUri) + => new DeleteWithStatusCodeResultRestResourceCommand( _httpClientProvider, _httpRequestMessageFactory, _httpResponseHandlerFactory, resourceUri);Additionally (outside this hunk), make
DeleteWithStatusCodeResultRestResourceCommandnon-generic:- internal sealed class DeleteWithStatusCodeResultRestResourceCommand<TResponse> : DeleteRestResourceCommand<HttpStatusCode> + internal sealed class DeleteWithStatusCodeResultRestResourceCommand : DeleteRestResourceCommand<HttpStatusCode> { ... }tests/Mailtrap.UnitTests/Contacts/Requests/ContactRequestTests.cs (1)
39-51: Add positive boundary tests and make data deterministic
- Add success cases for lengths 2 and 100 to lock boundaries.
- Avoid randomness for invalid-length strings to keep tests deterministic.
Apply this minimal diff for determinism:
- var email = TestContext.CurrentContext.Random.GetString(length); + var email = new string('a', length);And add a new test (outside this hunk) to assert valid boundaries:
[Test] public void Validate_ShouldPass_OnBoundaryLengths([Values(2, 100)] int length) { var email = new string('a', length); var request = new ContactRequest(email); var result = request.Validate(); result.IsValid.Should().BeTrue(); }src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs (1)
15-16: Validate and clarify semantics for ListIds (positives, uniqueness, and empty behavior).
- Consider validating that list IDs are > 0 and unique to fail fast before hitting the API.
- Empty default
[]will serialize as an empty array; confirm the API expects an empty array vs omitting the property when no lists are provided.- If omission is preferred, make the property nullable and avoid default initialization.
Would you like me to add a dedicated
CreateContactRequestValidatorand wire it in the DTO validation?src/Mailtrap/Contacts/Requests/ContactRequestDto.cs (3)
31-36: Prefer delegating to the request’s own Validate() to avoid duplication.Leverage
Contact.Validate()to centralize rules and future-proof specialized validators.Apply:
- public ValidationResult Validate() - { - return ContactRequestValidator.Instance - .Validate(Contact) - .ToMailtrapValidationResult(); - } + public ValidationResult Validate() + { + return Contact.Validate(); + }
10-20: Docstring mismatch: property is get-only.Comment says “Gets or sets…”, but the property is read-only. Adjust for accuracy.
- /// Gets or sets contact request payload. + /// Gets contact request payload.
31-36: Optional: allow specialized validation (Create/Update) without changing base types.If you add request-specific validators (e.g.,
CreateContactRequestValidatorforListIds), pattern-match here and dispatch accordingly.I can draft the validator(s) and the small dispatch here if you’d like.
src/Mailtrap.Abstractions/Accounts/IAccountResource.cs (1)
157-160: Use proper exception types in XML docs for string parametersFor string inputs, prefer ArgumentNullException (null) and ArgumentException (empty/whitespace) over ArgumentOutOfRangeException.
- /// <exception cref="ArgumentOutOfRangeException"> - /// When <paramref name="idOrEmail"/> is null or empty. - /// </exception> + /// <exception cref="ArgumentNullException"> + /// When <paramref name="idOrEmail"/> is null. + /// </exception> + /// <exception cref="ArgumentException"> + /// When <paramref name="idOrEmail"/> is empty or consists only of whitespace. + /// </exception>src/Mailtrap.Abstractions/Contacts/IContactResource.cs (1)
4-6: Nit: singularize summaryIt represents a single contact resource.
-/// Represents Contacts resource. +/// Represents Contact resource.src/Mailtrap.Abstractions/Contacts/Validators/ContactRequestValidator.cs (1)
20-21: Validate email format in addition to lengthAdd EmailAddress() to catch obvious invalid emails; keeps length rule intact.
- RuleFor(r => r.Email).NotEmpty().Length(2, 100); + RuleFor(r => r.Email) + .NotEmpty() + .Length(2, 100) + .EmailAddress();src/Mailtrap.Abstractions/Contacts/Responses/ContactResponse.cs (2)
8-14: Fix misleading XML doc on Contact propertyIt's not always "created" data; it's used for get/update too.
- /// Gets created contact data. + /// Gets contact data.
15-17: Reconsider Populate + default instance; can mask missing "data"Pre-initializing and using JsonObjectCreationHandling.Populate may leave a default Contact when the API omits or nulls "data". If not needed, drop Populate. If you want to enforce presence, consider adding JsonRequired.
[JsonPropertyName("data")] - [JsonObjectCreationHandling(JsonObjectCreationHandling.Populate)] - public Contact Contact { get; set; } = new(); + public Contact Contact { get; set; } = new(); + // Alternatively: + // [JsonRequired] + // public Contact Contact { get; init; } = default!;examples/Mailtrap.Example.Contact/Program.cs (4)
19-23: Resolve services from a scope to respect scoped lifetimesSafer for clients registered as scoped (common with HttpClient-typed services).
-using IHost host = hostBuilder.Build(); - -ILogger<Program> logger = host.Services.GetRequiredService<ILogger<Program>>(); -IMailtrapClient mailtrapClient = host.Services.GetRequiredService<IMailtrapClient>(); +using IHost host = hostBuilder.Build(); +using IServiceScope scope = host.Services.CreateScope(); +ILogger<Program> logger = scope.ServiceProvider.GetRequiredService<ILogger<Program>>(); +IMailtrapClient mailtrapClient = scope.ServiceProvider.GetRequiredService<IMailtrapClient>();
26-26: Avoid hard-coded accountId; read from configKeeps the sample runnable without code edits.
-var accountId = 12345; +if (!int.TryParse(config["AccountId"], out var accountId)) + throw new InvalidOperationException("Missing or invalid Mailtrap:AccountId.");
63-69: Use structured logging of key fields instead of dumping the objectContact likely lacks a helpful ToString(). Log identifiers explicitly.
-logger.LogInformation("Contact: {Contact}", contactDetails); +logger.LogInformation("Contact: Id={Id}, Email={Email}, Status={Status}", contactDetails.Id, contactDetails.Email, contactDetails.Status); ... -logger.LogInformation("Updated Contact: {Contact}", updatedContact); +logger.LogInformation("Updated Contact: Id={Id}, Email={Email}, Status={Status}", updatedContact.Id, updatedContact.Email, updatedContact.Status);
79-81: Avoid FailFast in example codeFailFast terminates immediately and can skip flush/dispose. Prefer exit code.
- Environment.FailFast(ex.Message); - throw; + Environment.ExitCode = 1; + return;src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (2)
4-5: Fix summary punctuationDouble period.
-/// Represents Contacts collection resource.. +/// Represents Contacts collection resource.
19-20: Confirm naming vs. pagination semanticsIf the backend paginates /contacts, “GetAll” may be misleading. Align with existing collection APIs (e.g., List/Get) or document pagination.
tests/Mailtrap.UnitTests/Contacts/ContactCollectionResourceTests.cs (1)
41-46: Rename local from client → resourceImproves clarity and consistency with the type under test.
- var client = CreateResource(); + var resource = CreateResource(); ... - client.ResourceUri.Should().Be(_resourceUri); + resource.ResourceUri.Should().Be(_resourceUri);tests/Mailtrap.UnitTests/Contacts/ContactResourceTests.cs (2)
5-5: Fix class name typoContactct → ContactResourceTests.
-internal sealed class ContactctResourceTests +internal sealed class ContactResourceTests
42-46: Rename local from client → resourceMatches the SUT and other tests.
- var client = CreateResource(); + var resource = CreateResource(); ... - client.ResourceUri.Should().Be(_resourceUri); + resource.ResourceUri.Should().Be(_resourceUri);src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs (2)
35-36: Avoid emitting null unsubscribed flag in JSONAdd conditional ignore so null doesn’t serialize as "unsubscribed": null.
- [JsonPropertyName("unsubscribed")] - public bool? Unsubscribed { get; set; } + [JsonPropertyName("unsubscribed")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public bool? Unsubscribed { get; set; }
15-16: Validate and sanitize list IDs (positive, distinct, disjoint)To prevent server-side 4xxs, ensure IDs are > 0, deduplicated, and not present in both collections. Consider doing this during DTO mapping or via a dedicated validator for UpdateContactRequest.
I can sketch a small UpdateContactRequestValidator and wire it into the mapping if you’d like.
Also applies to: 25-26
src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs (2)
52-58: Stronger email validationAdd a format check to complement length validation.
Outside this file (ContactRequestValidator.cs):
- RuleFor(r => r.Email).NotEmpty().Length(2, 100); + RuleFor(r => r.Email) + .NotEmpty() + .Length(2, 100) + .EmailAddress();
17-17: Make Email init-only for immutability
Email is only assigned in the constructor (no externalEmail =usages outside constructors found). Update the property accordingly:- public string Email { get; set; } + public string Email { get; init; }src/Mailtrap/Contacts/ContactCollectionResource.cs (1)
17-18: Null-check request before mapping to DTOGuard early for clearer exceptions and consistency.
- public async Task<CreateContactResponse> Create(CreateContactRequest request, CancellationToken cancellationToken = default) - => await Create<CreateContactRequestDto, CreateContactResponse>(request.ToDto(), cancellationToken).ConfigureAwait(false); + public async Task<CreateContactResponse> Create(CreateContactRequest request, CancellationToken cancellationToken = default) + { + Ensure.NotNull(request, nameof(request)); + return await Create<CreateContactRequestDto, CreateContactResponse>(request.ToDto(), cancellationToken).ConfigureAwait(false); + }src/Mailtrap/Contacts/ContactResource.cs (1)
14-15: Null-check update request before mappingMirror the Create path guard to keep behavior consistent.
- public async Task<UpdateContactResponse> Update(UpdateContactRequest request, CancellationToken cancellationToken = default) - => await Update<UpdateContactRequestDto, UpdateContactResponse>(request.ToDto(), cancellationToken).ConfigureAwait(false); + public async Task<UpdateContactResponse> Update(UpdateContactRequest request, CancellationToken cancellationToken = default) + { + Ensure.NotNull(request, nameof(request)); + return await Update<UpdateContactRequestDto, UpdateContactResponse>(request.ToDto(), cancellationToken).ConfigureAwait(false); + }tests/Mailtrap.IntegrationTests/Contacts/ContactsIntegrationTests.cs (3)
115-152: Validation test can avoid DTO serialization during arrangeCalling request.ToDto() in expectations isn’t needed since the request should never be sent; removing it makes the test more robust.
- .WithJsonContent(request.ToDto(), _jsonSerializerOptions) - .Respond(HttpStatusCode.UnprocessableContent); + .Respond(HttpStatusCode.UnprocessableContent);
164-198: Strengthen assertions once custom comparer existsFollow the TODO to assert full equivalence to deserialized expected objects for GetDetails/Update to catch shape regressions.
I can wire a minimal IEqualityComparer/UpdateContactResponse and update the tests.
32-71: General test structure: factor common setup to reduce duplicationConsider extracting DI and MockHttp setup into helper methods or [SetUp] to keep tests focused on behavior.
Also applies to: 73-114, 154-198, 200-248, 250-288, 290-323
src/Mailtrap.Abstractions/Contacts/Models/Contact.cs (3)
35-38: Prefer JsonElement for flexible custom fields.Using object leads to boxed values and JsonElement surprises on round-trip. JsonElement keeps types precise and avoids boxing.
Apply this diff:
- public IDictionary<string, object> Fields { get; } = new Dictionary<string, object>(); + public IDictionary<string, JsonElement> Fields { get; } = new Dictionary<string, JsonElement>();Add (once, via global usings or this file):
using System.Text.Json;
16-16: Make Id init-only to express immutability.Id is server-assigned; preventing accidental mutation helps correctness.
- public string Id { get; set; } = string.Empty; + public string Id { get; init; } = string.Empty;
69-69: Consider init-only for server-managed timestamps.These are typically read-only from API responses.
- public DateTimeOffset? CreatedAt { get; set; } + public DateTimeOffset? CreatedAt { get; init; } - public DateTimeOffset? UpdatedAt { get; set; } + public DateTimeOffset? UpdatedAt { get; init; }Also applies to: 80-80
📜 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 (46)
Mailtrap.sln(3 hunks)examples/Mailtrap.Example.Contact/Mailtrap.Example.Contact.csproj(1 hunks)examples/Mailtrap.Example.Contact/Program.cs(1 hunks)examples/Mailtrap.Example.Contact/Properties/launchSettings.json(1 hunks)examples/Mailtrap.Example.Contact/appsettings.json(1 hunks)src/Mailtrap.Abstractions/Accounts/IAccountResource.cs(11 hunks)src/Mailtrap.Abstractions/Contacts/Converters/DateTimeToTicksJsonConverter.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/IContactResource.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Models/Contact.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Models/ContactAction.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Models/ContactStatus.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Responses/ContactResponse.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Responses/CreateContactResponse.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Responses/UpdateContactResponse.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Validators/ContactRequestValidator.cs(1 hunks)src/Mailtrap.Abstractions/GlobalUsings.cs(1 hunks)src/Mailtrap/Accounts/AccountResource.cs(1 hunks)src/Mailtrap/Contacts/ContactCollectionResource.cs(1 hunks)src/Mailtrap/Contacts/ContactResource.cs(1 hunks)src/Mailtrap/Contacts/Requests/ContactRequestDto.cs(1 hunks)src/Mailtrap/Contacts/Requests/ContactRequestExtensions.cs(1 hunks)src/Mailtrap/Contacts/Requests/CreateContactRequestDto.cs(1 hunks)src/Mailtrap/Contacts/Requests/UpdateContactRequestDto.cs(1 hunks)src/Mailtrap/Core/Constants/UrlSegments.cs(1 hunks)src/Mailtrap/Core/Rest/Commands/DeleteRestResourceCommand.cs(1 hunks)src/Mailtrap/Core/Rest/Commands/DeleteWithStatusCodeResultRestResourceCommand.cs(1 hunks)src/Mailtrap/Core/Rest/IRestResourceCommandFactory.cs(1 hunks)src/Mailtrap/Core/Rest/RestResource.cs(1 hunks)src/Mailtrap/Core/Rest/RestResourceCommandFactory.cs(1 hunks)src/Mailtrap/GlobalUsings.cs(1 hunks)tests/Mailtrap.IntegrationTests/Contacts/ContactsIntegrationTests.cs(1 hunks)tests/Mailtrap.IntegrationTests/Contacts/Create_Success.json(1 hunks)tests/Mailtrap.IntegrationTests/Contacts/GetAll_Success.json(1 hunks)tests/Mailtrap.IntegrationTests/Contacts/GetDetails_Success.json(1 hunks)tests/Mailtrap.IntegrationTests/Contacts/Update_Success.json(1 hunks)tests/Mailtrap.IntegrationTests/GlobalUsings.cs(1 hunks)tests/Mailtrap.IntegrationTests/TestConstants/UrlSegmentsTestConstants.cs(1 hunks)tests/Mailtrap.UnitTests/Contacts/ContactCollectionResourceTests.cs(1 hunks)tests/Mailtrap.UnitTests/Contacts/ContactResourceTests.cs(1 hunks)tests/Mailtrap.UnitTests/Contacts/Requests/ContactRequestTests.cs(1 hunks)tests/Mailtrap.UnitTests/GlobalUsings.cs(1 hunks)tests/Mailtrap.UnitTests/TestConstants/UrlSegmentsTestConstants.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (27)
src/Mailtrap.Abstractions/Contacts/Validators/ContactRequestValidator.cs (1)
src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs (1)
ContactRequest(44-49)
src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (6)
src/Mailtrap/Accounts/AccountResource.cs (1)
IContactCollectionResource(56-57)src/Mailtrap.Abstractions/Contacts/IContactResource.cs (3)
Task(19-19)Task(36-36)Task(56-56)src/Mailtrap/Contacts/ContactCollectionResource.cs (2)
Task(14-15)Task(17-18)src/Mailtrap/Contacts/ContactResource.cs (3)
Task(12-13)Task(14-15)Task(16-17)src/Mailtrap/Core/Rest/RestResource.cs (8)
Task(24-25)Task(27-28)Task(31-32)Task(34-35)Task(38-46)Task(48-56)Task(58-62)Task(64-68)src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs (1)
CreateContactRequest(19-19)
src/Mailtrap/Accounts/AccountResource.cs (3)
src/Mailtrap/Contacts/ContactCollectionResource.cs (2)
ContactCollectionResource(8-19)ContactCollectionResource(10-11)src/Mailtrap/Core/Constants/UrlSegments.cs (1)
UrlSegments(4-10)src/Mailtrap/Contacts/ContactResource.cs (2)
ContactResource(8-18)ContactResource(10-11)
tests/Mailtrap.UnitTests/Contacts/Requests/ContactRequestTests.cs (1)
src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs (1)
ContactRequest(44-49)
src/Mailtrap.Abstractions/Contacts/Models/ContactAction.cs (1)
src/Mailtrap.Abstractions/Core/Models/StringEnum.cs (1)
StringEnum(41-49)
src/Mailtrap/Core/Rest/Commands/DeleteRestResourceCommand.cs (1)
src/Mailtrap/Core/Rest/Commands/RestResourceCommand.cs (2)
RestResourceCommand(4-60)RestResourceCommand(15-34)
examples/Mailtrap.Example.Contact/Program.cs (3)
src/Mailtrap/Accounts/AccountResource.cs (2)
IContactCollectionResource(56-57)IContactResource(59-60)src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs (1)
CreateContactRequest(19-19)src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs (1)
UpdateContactRequest(39-39)
src/Mailtrap.Abstractions/Contacts/Models/ContactStatus.cs (1)
src/Mailtrap.Abstractions/Core/Models/StringEnum.cs (1)
StringEnum(41-49)
src/Mailtrap/Contacts/Requests/UpdateContactRequestDto.cs (3)
src/Mailtrap/Contacts/Requests/ContactRequestExtensions.cs (1)
UpdateContactRequestDto(8-8)src/Mailtrap/Contacts/Requests/ContactRequestDto.cs (1)
ContactRequestDto(22-27)src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs (1)
UpdateContactRequest(39-39)
tests/Mailtrap.UnitTests/Contacts/ContactCollectionResourceTests.cs (2)
tests/Mailtrap.UnitTests/TestConstants/UrlSegmentsTestConstants.cs (1)
UrlSegmentsTestConstants(4-18)src/Mailtrap/Contacts/ContactCollectionResource.cs (2)
ContactCollectionResource(8-19)ContactCollectionResource(10-11)
src/Mailtrap/Core/Rest/RestResourceCommandFactory.cs (2)
src/Mailtrap/Core/Rest/IRestResourceCommandFactory.cs (9)
IRestResourceCommand(6-6)IRestResourceCommand(7-7)IRestResourceCommand(8-8)IRestResourceCommand(9-9)IRestResourceCommand(10-10)IRestResourceCommand(12-12)IRestResourceCommand(14-14)IRestResourceCommand(15-15)IRestResourceCommand(16-16)src/Mailtrap/Core/Rest/Commands/DeleteWithStatusCodeResultRestResourceCommand.cs (2)
DeleteWithStatusCodeResultRestResourceCommand(4-20)DeleteWithStatusCodeResultRestResourceCommand(6-16)
tests/Mailtrap.UnitTests/Contacts/ContactResourceTests.cs (3)
tests/Mailtrap.UnitTests/Contacts/ContactCollectionResourceTests.cs (4)
TestFixture(4-51)Test(18-26)Test(28-36)Test(38-46)tests/Mailtrap.UnitTests/TestConstants/UrlSegmentsTestConstants.cs (1)
UrlSegmentsTestConstants(4-18)src/Mailtrap/Contacts/ContactResource.cs (2)
ContactResource(8-18)ContactResource(10-11)
src/Mailtrap.Abstractions/Contacts/IContactResource.cs (3)
src/Mailtrap/Accounts/AccountResource.cs (1)
IContactResource(59-60)src/Mailtrap/Contacts/ContactResource.cs (3)
Task(12-13)Task(14-15)Task(16-17)src/Mailtrap/Core/Rest/RestResource.cs (9)
Task(24-25)Task(27-28)Task(31-32)Task(34-35)Task(38-46)Task(48-56)Task(58-62)Task(64-68)Task(71-74)
src/Mailtrap/Core/Rest/IRestResourceCommandFactory.cs (1)
src/Mailtrap/Core/Rest/RestResourceCommandFactory.cs (9)
IRestResourceCommand(26-32)IRestResourceCommand(34-40)IRestResourceCommand(42-47)IRestResourceCommand(49-54)IRestResourceCommand(56-61)IRestResourceCommand(63-70)IRestResourceCommand(72-79)IRestResourceCommand(81-88)IRestResourceCommand(90-97)
src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs (2)
src/Mailtrap.Abstractions/Core/Extensions/Ensure.cs (2)
Ensure(9-82)NotNullOrEmpty(42-57)src/Mailtrap.Abstractions/Contacts/Validators/ContactRequestValidator.cs (2)
ContactRequestValidator(8-22)ContactRequestValidator(18-21)
src/Mailtrap/Contacts/Requests/ContactRequestExtensions.cs (4)
src/Mailtrap/Contacts/Requests/CreateContactRequestDto.cs (1)
CreateContactRequestDto(9-9)src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs (1)
CreateContactRequest(19-19)src/Mailtrap/Contacts/Requests/UpdateContactRequestDto.cs (1)
UpdateContactRequestDto(9-9)src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs (1)
UpdateContactRequest(39-39)
src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs (1)
src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs (1)
ContactRequest(44-49)
src/Mailtrap/Contacts/ContactResource.cs (6)
src/Mailtrap/Core/Rest/RestResource.cs (11)
RestResource(7-75)RestResource(14-21)Task(24-25)Task(27-28)Task(31-32)Task(34-35)Task(38-46)Task(48-56)Task(58-62)Task(64-68)Task(71-74)src/Mailtrap/Accounts/AccountResource.cs (1)
IContactResource(59-60)src/Mailtrap.Abstractions/Contacts/IContactResource.cs (3)
Task(19-19)Task(36-36)Task(56-56)src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs (1)
UpdateContactRequest(39-39)src/Mailtrap/Contacts/Requests/ContactRequestExtensions.cs (1)
UpdateContactRequestDto(8-8)src/Mailtrap/Contacts/Requests/UpdateContactRequestDto.cs (1)
UpdateContactRequestDto(9-9)
tests/Mailtrap.IntegrationTests/Contacts/ContactsIntegrationTests.cs (12)
src/Mailtrap/Configuration/MailtrapClientOptions.cs (1)
JsonSerializerOptions(148-154)tests/Mailtrap.IntegrationTests/TestConstants/UrlSegmentsTestConstants.cs (1)
UrlSegmentsTestConstants(4-21)src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (2)
Task(19-19)Task(36-36)src/Mailtrap.Abstractions/Contacts/IContactResource.cs (3)
Task(19-19)Task(36-36)Task(56-56)src/Mailtrap/Contacts/ContactCollectionResource.cs (2)
Task(14-15)Task(17-18)src/Mailtrap/Contacts/ContactResource.cs (3)
Task(12-13)Task(14-15)Task(16-17)src/Mailtrap/Core/Rest/RestResource.cs (6)
Task(24-25)Task(27-28)Task(31-32)Task(34-35)Task(38-46)Task(48-56)src/Mailtrap/Core/Constants/MimeTypes.cs (2)
MimeTypes(4-19)Application(6-11)src/Mailtrap/Core/Constants/HeaderValues.cs (1)
HeaderValues(7-12)src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs (1)
CreateContactRequest(19-19)src/Mailtrap/Core/Extensions/HttpMethodEx.cs (1)
HttpMethodEx(7-10)src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs (1)
UpdateContactRequest(39-39)
src/Mailtrap/Contacts/ContactCollectionResource.cs (7)
tests/Mailtrap.UnitTests/Contacts/ContactCollectionResourceTests.cs (1)
ContactCollectionResource(50-50)src/Mailtrap/Core/Rest/RestResource.cs (10)
RestResource(7-75)RestResource(14-21)Task(24-25)Task(27-28)Task(31-32)Task(34-35)Task(38-46)Task(48-56)Task(58-62)Task(64-68)src/Mailtrap/Accounts/AccountResource.cs (1)
IContactCollectionResource(56-57)src/Mailtrap.Abstractions/Contacts/IContactCollectionResource.cs (2)
Task(19-19)Task(36-36)src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs (1)
CreateContactRequest(19-19)src/Mailtrap/Contacts/Requests/ContactRequestExtensions.cs (1)
CreateContactRequestDto(6-6)src/Mailtrap/Contacts/Requests/CreateContactRequestDto.cs (1)
CreateContactRequestDto(9-9)
src/Mailtrap/Core/Rest/Commands/DeleteWithStatusCodeResultRestResourceCommand.cs (1)
src/Mailtrap/Core/Rest/Commands/DeleteRestResourceCommand.cs (2)
DeleteRestResourceCommand(4-18)DeleteRestResourceCommand(6-17)
src/Mailtrap.Abstractions/Contacts/Models/Contact.cs (1)
src/Mailtrap.Abstractions/Contacts/Converters/DateTimeToTicksJsonConverter.cs (2)
DateTimeToTicksJsonConverter(6-17)DateTimeOffset(8-11)
src/Mailtrap/Core/Rest/RestResource.cs (4)
src/Mailtrap.Abstractions/Contacts/IContactResource.cs (3)
Task(19-19)Task(36-36)Task(56-56)src/Mailtrap/Contacts/ContactResource.cs (3)
Task(12-13)Task(14-15)Task(16-17)src/Mailtrap/Core/Rest/Commands/RestResourceCommand.cs (1)
Task(37-52)src/Mailtrap/Core/Rest/RestResourceCommandFactory.cs (2)
RestResourceCommandFactory(4-98)RestResourceCommandFactory(11-23)
src/Mailtrap/Contacts/Requests/CreateContactRequestDto.cs (3)
src/Mailtrap/Contacts/Requests/ContactRequestExtensions.cs (1)
CreateContactRequestDto(6-6)src/Mailtrap/Contacts/Requests/ContactRequestDto.cs (1)
ContactRequestDto(22-27)src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs (1)
CreateContactRequest(19-19)
src/Mailtrap.Abstractions/Contacts/Converters/DateTimeToTicksJsonConverter.cs (1)
src/Mailtrap/Configuration/MailtrapClientOptions.cs (1)
JsonSerializerOptions(148-154)
src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs (1)
src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs (1)
ContactRequest(44-49)
src/Mailtrap/Contacts/Requests/ContactRequestDto.cs (3)
src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs (2)
ContactRequest(44-49)ValidationResult(53-58)src/Mailtrap.Abstractions/Core/Extensions/Ensure.cs (2)
Ensure(9-82)NotNull(18-33)src/Mailtrap.Abstractions/Contacts/Validators/ContactRequestValidator.cs (2)
ContactRequestValidator(8-22)ContactRequestValidator(18-21)
🔇 Additional comments (30)
examples/Mailtrap.Example.Contact/Properties/launchSettings.json (1)
1-10: LGTM for example launch profile.
Profile and DOTNET_ENVIRONMENT are appropriate for a console sample.src/Mailtrap.Abstractions/Contacts/Models/ContactStatus.cs (1)
6-24: Confirm status coverage vs Mailtrap APIOnly Subscribed/Unsubscribed are defined. If the API can return additional values (e.g., bounced/complained/etc.), add them now to avoid deserializing to Unknown at runtime.
tests/Mailtrap.IntegrationTests/Contacts/GetAll_Success.json (1)
34-41: Validate behavior when status is missingThis sample omits "status". Please confirm the model maps this to ContactStatus.Unknown (or intended default) and that tests assert this explicitly.
Mailtrap.sln (2)
166-169: Configs wired correctly for new projectDebug/Release mappings look good.
192-192: Nested under examples: LGTMProject is correctly nested under the examples solution folder.
tests/Mailtrap.UnitTests/TestConstants/UrlSegmentsTestConstants.cs (1)
17-17: LGTM: matches production segment"contacts" segment aligns with UrlSegments.ContactsSegment usage.
src/Mailtrap/Core/Constants/UrlSegments.cs (1)
9-9: LGTM: new Contacts segment is consistentMatches existing style for segments and keeps naming aligned. No issues.
tests/Mailtrap.IntegrationTests/Contacts/Create_Success.json (1)
1-18: Confirm timestamp units and model bindingcreated_at/updated_at look like Unix time in milliseconds. Ensure your JSON converters and DTOs for Contacts expect ms (not seconds or RFC3339), otherwise deserialization/assertions may misbehave.
tests/Mailtrap.IntegrationTests/GlobalUsings.cs (1)
16-16: LGTM: adds needed Contacts request types for ITsNo concerns.
examples/Mailtrap.Example.Contact/Mailtrap.Example.Contact.csproj (2)
7-9: LGTM: appsettings.json copy behaviorPreserveNewest is appropriate for a sample app; no change needed.
3-4: CPM enabled; VersionOverride is valid
Directory.Packages.props declares<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>, so using VersionOverride here is correct and no changes are needed.tests/Mailtrap.IntegrationTests/TestConstants/UrlSegmentsTestConstants.cs (1)
20-20: Contacts URL segment added correctlyValue matches expected "contacts" and is consistent with other segments.
src/Mailtrap/Contacts/Requests/UpdateContactRequestDto.cs (1)
4-10: DTO wiring looks correctMatches the established DTO pattern; constructor defers null-checks to base. No issues.
tests/Mailtrap.UnitTests/GlobalUsings.cs (1)
26-27: Appropriate global usings for Contacts testsKeeps test code concise; scope is limited to tests, so this is fine.
src/Mailtrap.Abstractions/Contacts/Responses/CreateContactResponse.cs (1)
6-8: Simple, correct response typeConsistent with the response hierarchy; no extra members needed.
src/Mailtrap/Core/Rest/Commands/DeleteRestResourceCommand.cs (1)
4-4: Unsealing to enable specialization is appropriateMaking this class inheritable is necessary for the status-code handler override; scope remains internal. LGTM.
tests/Mailtrap.UnitTests/Contacts/Requests/ContactRequestTests.cs (3)
7-13: Constructor null-guard test looks goodClear assertion on
ArgumentNullException. LGTM.
15-21: Empty-string guard test is correctMatches constructor contract. LGTM.
23-37: Field initialization test is fineStraightforward and readable. LGTM.
src/Mailtrap.Abstractions/Contacts/Requests/CreateContactRequest.cs (1)
15-16: No additional using required:JsonPropertyNameis available via the global using insrc/Mailtrap.Abstractions/GlobalUsings.cs:6.src/Mailtrap.Abstractions/Contacts/Models/ContactAction.cs (1)
15-24: LGTM: string-backed enum is consistent and minimal.Values align with expected payloads (“updated”, “created”) and with existing
StringEnum<T>usage. No changes needed.src/Mailtrap/Contacts/Requests/CreateContactRequestDto.cs (1)
7-10: LGTM: thin DTO wrapper keeps mapping explicit.Constructor passthrough is clear and consistent with other DTOs.
src/Mailtrap/Accounts/AccountResource.cs (1)
56-58: LGTM: Contacts() resource wiring mirrors existing pattern.Segment composition aligns with other collection resources.
src/Mailtrap.Abstractions/Accounts/IAccountResource.cs (1)
136-144: Contacts entrypoints look goodThe new Contacts() and Contact(...) members align with existing resource patterns.
src/Mailtrap.Abstractions/Contacts/Requests/UpdateContactRequest.cs (1)
16-16: Global LangVersion=12.0 enables C# 12 collection expressions
The root Directory.Build.props imports build/mailtrap-global.props, which defines<LangVersion>12.0</LangVersion>, so the[]initializer is fully supported—no change needed.src/Mailtrap.Abstractions/Contacts/Requests/ContactRequest.cs (1)
15-17: Confirm TFM supports JsonRequiredAttribute
No<TargetFramework>or<TargetFrameworks>entry was found—ensuresrc/Mailtrap.Abstractions/Mailtrap.Abstractions.csproj(or its imported props) targets .NET 7.0 or later soJsonRequiredAttributeis available.src/Mailtrap/Contacts/ContactCollectionResource.cs (1)
14-15: LGTM: list retrieval path is straightforwardUses base GetList with ConfigureAwait(false); matches rest of codebase.
src/Mailtrap/Contacts/ContactResource.cs (1)
12-13: LGTM: get and delete behaviors
- GetDetails delegates to base Get correctly.
- Delete uses the new status-code-only delete flow as intended.
Also applies to: 16-17
src/Mailtrap.Abstractions/Contacts/Models/Contact.cs (2)
3-7: Solid DTO shape and clear JSON mapping.Record + STJ attributes/read-only collections are a good fit here.
46-49: Confirm TargetFramework and LangVersion support C# 12 [] literals and JsonObjectCreationHandling.Populate.
The Abstractions csproj imports SDK settings frommailtrap-nuget.props(not in repo) and doesn’t declare<TargetFramework>or<LangVersion>directly. Ensure your build config targets net7.0+ (or net8.0) and sets LangVersion to 12 (or higher) so STJ 9.0.7 and C# 12 features compile.
src/Mailtrap.Abstractions/Contacts/Converters/DateTimeToTicksJsonConverter.cs
Outdated
Show resolved
Hide resolved
src/Mailtrap.Abstractions/Contacts/Responses/UpdateContactResponse.cs
Outdated
Show resolved
Hide resolved
Updates according to coderabbit - updated DateTimeOffset converter to support nullable type and switched format from ticks to milliseconds - update Contact model to use updated converter - Corrected description for UpdateContactResponse - corrected project guid for Contacts example project.
- removed file of remaned converter
Updates according to coderabbit comment: - Enhance Contact retrieval by adding null or empty check and URI encoding
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/Mailtrap.Abstractions/Contacts/Converters/DateTimeToUnixMsJsonConverter.cs (3)
3-6: Clarify XML doc to reflect nullable and bidirectional conversionAdjust the summary to explicitly mention nullable DateTimeOffset and both read/write behavior.
-/// Converts DateTimeOffset to Unix time milliseconds for JSON serialization. +/// Converts nullable DateTimeOffset values to/from Unix time milliseconds for JSON (de)serialization.
6-6: File/class name mismatchClass name includes "Nullable" but the file name does not. Align them for maintainability (prefer renaming the file to DateTimeToUnixMsNullableJsonConverter.cs).
8-22: Harden Read(): wrap range errors as JsonException (and optionally accept numeric strings)FromUnixTimeMilliseconds throws ArgumentOutOfRangeException for invalid values. Wrap it to surface a consistent JsonException. Optionally accept numbers encoded as strings to be lenient with upstream variations.
Minimal change (range wrapping):
- var ms = reader.GetInt64(); - return DateTimeOffset.FromUnixTimeMilliseconds(ms); + var ms = reader.GetInt64(); + try + { + return DateTimeOffset.FromUnixTimeMilliseconds(ms); + } + catch (ArgumentOutOfRangeException ex) + { + throw new JsonException("Invalid Unix time milliseconds value.", ex); + }Optional (also handle numeric strings):
- if (reader.TokenType != JsonTokenType.Number) - { - throw new JsonException($"Expected number for Unix time milliseconds but got {reader.TokenType}."); - } - - var ms = reader.GetInt64(); - try - { - return DateTimeOffset.FromUnixTimeMilliseconds(ms); - } - catch (ArgumentOutOfRangeException ex) - { - throw new JsonException("Invalid Unix time milliseconds value.", ex); - } + if (reader.TokenType == JsonTokenType.Number) + { + var ms = reader.GetInt64(); + try { return DateTimeOffset.FromUnixTimeMilliseconds(ms); } + catch (ArgumentOutOfRangeException ex) { throw new JsonException("Invalid Unix time milliseconds value.", ex); } + } + if (reader.TokenType == JsonTokenType.String) + { + var s = reader.GetString(); + if (long.TryParse(s, System.Globalization.NumberStyles.Integer, System.Globalization.CultureInfo.InvariantCulture, out var msFromString)) + { + try { return DateTimeOffset.FromUnixTimeMilliseconds(msFromString); } + catch (ArgumentOutOfRangeException ex) { throw new JsonException("Invalid Unix time milliseconds value.", ex); } + } + throw new JsonException("Invalid Unix time milliseconds string."); + } + throw new JsonException($"Expected number (or numeric string) for Unix time milliseconds but got {reader.TokenType}.");
📜 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 (4)
Mailtrap.sln(3 hunks)src/Mailtrap.Abstractions/Contacts/Converters/DateTimeToUnixMsJsonConverter.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Models/Contact.cs(1 hunks)src/Mailtrap.Abstractions/Contacts/Responses/UpdateContactResponse.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Mailtrap.Abstractions/Contacts/Responses/UpdateContactResponse.cs
- Mailtrap.sln
- src/Mailtrap.Abstractions/Contacts/Models/Contact.cs
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-04T08:53:30.310Z
Learnt from: vittorius
PR: railsware/mailtrap-dotnet#139
File: tests/Mailtrap.IntegrationTests/Contacts/GetDetails_Success.json:15-16
Timestamp: 2025-09-04T08:53:30.310Z
Learning: The DateTimeToTicksJsonConverter in the Mailtrap .NET client was redundant and problematic because System.Text.Json already handles DateTimeOffset serialization natively to ISO 8601 format, and other models in the codebase (TestingMessage, BillingUsagePeriod) successfully use DateTimeOffset properties without custom converters.
Applied to files:
src/Mailtrap.Abstractions/Contacts/Converters/DateTimeToUnixMsJsonConverter.cs
📚 Learning: 2025-09-04T08:53:06.267Z
Learnt from: vittorius
PR: railsware/mailtrap-dotnet#139
File: src/Mailtrap.Abstractions/Contacts/Models/Contact.cs:67-70
Timestamp: 2025-09-04T08:53:06.267Z
Learning: The Mailtrap .NET client uses default JSON serialization for DateTimeOffset properties without explicit JsonConverter attributes, as seen in models like TestingMessage. DateTime fields should not use the DateTimeToTicksJsonConverter since the API returns Unix epoch milliseconds, not .NET ticks.
Applied to files:
src/Mailtrap.Abstractions/Contacts/Converters/DateTimeToUnixMsJsonConverter.cs
📚 Learning: 2025-09-04T08:53:06.267Z
Learnt from: vittorius
PR: railsware/mailtrap-dotnet#139
File: src/Mailtrap.Abstractions/Contacts/Models/Contact.cs:67-70
Timestamp: 2025-09-04T08:53:06.267Z
Learning: In the Mailtrap .NET client, DateTimeOffset properties should not use explicit JsonConverter attributes. All models in the codebase (TestingMessage, Inbox, EmailAttachment, etc.) handle DateTimeOffset properties using default JSON serialization without custom converters, and the Contact model should follow this same pattern for consistency.
Applied to files:
src/Mailtrap.Abstractions/Contacts/Converters/DateTimeToUnixMsJsonConverter.cs
🧬 Code graph analysis (1)
src/Mailtrap.Abstractions/Contacts/Converters/DateTimeToUnixMsJsonConverter.cs (2)
src/Mailtrap/Core/Converters/StringEnumJsonConverterFactory.cs (1)
JsonConverter(25-31)src/Mailtrap/Configuration/MailtrapClientOptions.cs (1)
JsonSerializerOptions(148-154)
🔇 Additional comments (2)
src/Mailtrap.Abstractions/Contacts/Converters/DateTimeToUnixMsJsonConverter.cs (2)
24-33: LGTM: Write() correctly emits Unix ms or nullSerialization path is correct and matches the read semantics.
1-1: Namespace and usings are correct – the converter’s namespace (Mailtrap.Contacts.Converters) matches all other Abstractions converters, and the required JSON types are already covered by global usings.
|
|
||
| using var responseContent = await Feature.LoadFileToStringContent(); | ||
|
|
||
| //TODO: use expectedResponse to verify result when Contact's custom comparer is implemented |
There was a problem hiding this comment.
Did you mean to resolve this and other TODOs before opening PR, or that's for later? If for later, maybe we can create a GitHub issue for this so we don't forget.
There was a problem hiding this comment.
yeah, it is nice to have feature, however I not sure if it worth efforts to implement it, but definitely not as part of this PR. Will remove TODOs, and created #140 instead.
Co-authored-by: Marcin Klocek <[email protected]>
Removed TODOs, separete issue filed instead.
correction after merge #128
Implemented Contacts API as part of #126 task
Motivation
Contacts API support into .NET client
Changes
How to test
Summary by CodeRabbit
New Features
Documentation
Tests