feat: separate payment domain #42
Conversation
WalkthroughThe changes in this pull request primarily involve a significant restructuring of the payment-related namespaces and project references within the solution. The project previously named "Bmb.Payment.Core" has been renamed to "Bmb.Payment.Domain," and various files across the solution have been updated to reflect this change. Additionally, new interfaces and classes have been introduced in the domain layer, while several core contracts have been removed. The modifications also include updates to method signatures and import statements across multiple files to ensure alignment with the new domain structure. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant NotificationsController
participant PaymentGateway
participant PaymentRepository
Client->>NotificationsController: Post payment event
NotificationsController->>PaymentGateway: CreatePaymentAsync(OrderDto)
PaymentGateway->>PaymentRepository: SaveAsync(Payment)
PaymentRepository-->>PaymentGateway: Payment saved
PaymentGateway-->>NotificationsController: Payment created
NotificationsController-->>Client: 200 OK
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 13
🧹 Outside diff range and nitpick comments (59)
src/Bmb.Payment.Application/UseCases/ICreatePaymentUseCase.cs (2)
Line range hint
1-7: LGTM! Clean Architecture principles well applied.The interface is correctly placed in the Application layer and properly depends on the Domain layer. The namespace restructuring aligns well with DDD principles by establishing clear boundaries for the payment domain.
5-5: Consider strengthening the contract design.A few suggestions to enhance the interface contract:
- Consider renaming
Executeto a more domain-specific name likeCreatePaymentto better reflect the ubiquitous language- Consider making the return type non-nullable (
Paymentinstead ofPayment?) and use a Result/Either pattern to handle failure cases explicitlyExample refactor:
- Task<Domain.Entities.Payment?> Execute(Guid orderId, Domain.ValueObjects.PaymentType paymentType); + Task<Result<Domain.Entities.Payment>> CreatePayment(Guid orderId, Domain.ValueObjects.PaymentType paymentType);src/Bmb.Payment.Application/UseCases/IUpdatePaymentStatusUseCase.cs (1)
1-1: LGTM: Clean separation of domain boundariesThe namespace change from Core to Domain properly aligns with DDD principles by establishing clear bounded contexts for the payment domain.
src/Bmb.Payment.Domain/Contracts/IPaymentGatewayFactoryMethod.cs (1)
7-10: Consider renaming interface and improving factory method pattern implementation
- The interface name could be more concise (e.g.,
IPaymentGatewayFactory).- Consider using a more specific return type that encapsulates the payment gateway creation logic.
Here's a suggested improvement:
-public interface IPaymentGatewayFactoryMethod +public interface IPaymentGatewayFactory { - IPaymentGateway Create(ValueObjects_PaymentType paymentType); + IPaymentGateway Create(PaymentType type); }Consider these architectural improvements:
- Follow the Factory Method pattern more closely by defining specific factory methods for each payment type
- Consider using a registration mechanism for payment gateways to make the system more extensible
- Think about adding validation or error handling for unsupported payment types
src/Bmb.Payment.Domain/Contracts/IPaymentGateway.cs (1)
7-12: Interface design looks good, consider adding documentationThe interface follows the Single Responsibility Principle and has clear, focused methods. However, adding XML documentation would improve clarity, especially for the nullable return type in
GetPaymentStatusAsync.Add documentation:
+/// <summary> +/// Defines operations for interacting with a payment gateway +/// </summary> public interface IPaymentGateway { + /// <summary> + /// Creates a new payment for the specified order + /// </summary> + /// <param name="order">The order to create payment for</param> + /// <returns>The created payment entity</returns> Task<Entities.Payment> CreatePaymentAsync(Order order); + /// <summary> + /// Retrieves the current status of a payment + /// </summary> + /// <param name="paymentId">The unique identifier of the payment</param> + /// <returns>The payment status if found, null otherwise</returns> Task<ValueObjects_PaymentStatus?> GetPaymentStatusAsync(string paymentId); }src/Bmb.Payment.FakePayment.Gateway/ServiceCollectionsExtensions.cs (1)
Line range hint
11-14: Consider making the payment type configuration more flexibleWhile the current implementation is correct, consider extracting the payment type string into a configuration to make it more maintainable and reusable across the application.
public static void AddFakePaymentGateway(this IServiceCollection services) { - services.AddKeyedScoped<IPaymentGateway, FakePaymentGatewayService>($"Payment-{nameof(PaymentType.Test)}"); + const string PAYMENT_TYPE_KEY = nameof(PaymentType.Test); + services.AddKeyedScoped<IPaymentGateway, FakePaymentGatewayService>($"Payment-{PAYMENT_TYPE_KEY}"); }src/Bmb.Payment.DynamoDb/ServiceExtensions.cs (1)
Line range hint
11-15: Consider enhancing the DynamoDB configuration.The
configurationparameter is currently unused. Consider implementing the options pattern for DynamoDB settings and AWS client configuration.Here's a suggested implementation:
public static void ConfigurePersistenceApp(this IServiceCollection services, IConfiguration configuration) { + services.Configure<DynamoDbOptions>(configuration.GetSection("DynamoDb")); + services.AddAWSService<IAmazonDynamoDB>(); services.AddScoped<IPaymentRepository, PaymentRepository>(); } +public class DynamoDbOptions +{ + public string TableName { get; set; } + public string Region { get; set; } +}src/Bmb.Payment.Api/Mappers.cs (1)
Line range hint
15-15: Consider using a factory method for PaymentType creationDirect casting to domain value objects can bypass domain invariants and validation rules. Consider using a factory method or constructor that ensures the value object's invariants.
- return new CreateOrderPaymentRequestDto(request.OrderId, (PaymentType)request.PaymentType); + return new CreateOrderPaymentRequestDto(request.OrderId, PaymentType.From(request.PaymentType));This approach allows the domain to enforce its invariants and validation rules when creating value objects.
src/Bmb.Payment.Domain/Contracts/IPaymentRepository.cs (2)
7-16: Add XML documentation and consider return type refinementThe interface follows DDD principles by being focused on a single aggregate root (Payment). However, there are some improvements to consider:
- Missing XML documentation for the interface and its methods
- The
SaveAsyncmethod returns the entire entity, which might expose more data than necessary- The method names could be more explicit about their business operations
Consider these improvements:
+/// <summary> +/// Defines the contract for persisting and retrieving Payment aggregates +/// </summary> public interface IPaymentRepository { + /// <summary> + /// Persists a new payment or updates an existing one + /// </summary> + /// <param name="payment">The payment aggregate to save</param> + /// <returns>The ID of the saved payment</returns> - Task<Entities.Payment> SaveAsync(Entities.Payment payment); + Task<PaymentId> SaveAsync(Entities.Payment payment); + /// <summary> + /// Retrieves a payment by its unique identifier + /// </summary> Task<Entities.Payment?> GetPaymentAsync(PaymentId paymentId); + /// <summary> + /// Retrieves a payment by its external reference and type + /// </summary> - Task<Entities.Payment?> GetPaymentAsync(string externalReference, ValueObjects_PaymentType paymentType); + Task<Entities.Payment?> GetPaymentByReferenceAsync(string externalReference, DomainPaymentType paymentType); + /// <summary> + /// Updates only the status of an existing payment + /// </summary> Task<bool> UpdatePaymentStatusAsync(Entities.Payment payment); }
7-16: Consider adding domain events for payment status changesFollowing DDD principles, payment status changes should potentially raise domain events for other bounded contexts to react to.
Consider:
- Adding a domain event when payment status changes
- Using a richer domain model that includes event raising capabilities
- Ensuring the repository implementation handles these events appropriately
Would you like me to provide an example implementation of this pattern?
src/Bmb.Payment.Masstransit/OrderCreatedPaymentConsumer.cs (3)
2-2: Consider introducing an anti-corruption layer for external eventsWhile the namespace change correctly reflects the domain separation, the consumer directly converts external integration events to domain DTOs. Consider introducing an anti-corruption layer to protect the domain boundary and reduce coupling.
Consider:
- Creating a separate application service to handle the translation
- Moving the
ToDto()extension method to an anti-corruption layer- Using domain events instead of integration events within the domain boundary
Line range hint
13-19: Add error handling for gateway operationsThe
SaveCopyAsyncoperation lacks error handling. Failed gateway operations could lead to lost messages or unclear error states.Consider adding proper error handling:
public async Task Consume(ConsumeContext<OrderCreated> context) { using var scope = logger.BeginScope("Processing {Message} for {OrderCode} {OrderId}", context.Message, context.Message.OrderTrackingCode, context.Message.Id); { + try + { await ordersGateway.SaveCopyAsync(context.Message.ToDto()); logger.LogInformation("Message processed: {Message}", context.Message); + } + catch (Exception ex) + { + logger.LogError(ex, "Failed to save order copy for {OrderId}", context.Message.Id); + throw; + } } }
Line range hint
15-18: Remove redundant scope blockThe extra curly braces create an unnecessary scope block within the
usingstatement.using var scope = logger.BeginScope("Processing {Message} for {OrderCode} {OrderId}", context.Message, context.Message.OrderTrackingCode, context.Message.Id); -{ await ordersGateway.SaveCopyAsync(context.Message.ToDto()); logger.LogInformation("Message processed: {Message}", context.Message); -}src/Bmb.Payment.MercadoPago.Gateway/Bmb.Payment.MercadoPago.Gateway.csproj (1)
Update mercadopago-sdk package to version 2.4.1
The mercadopago-sdk package (currently at 2.3.8) has a newer version 2.4.1 available. For the Microsoft packages, since we couldn't verify the latest versions through the API, and considering they are recent 8.0.x versions from February 2024, they appear to be up-to-date.
- Update
mercadopago-sdkfrom version 2.3.8 to 2.4.1🔗 Analysis chain
Line range hint
10-15: Verify package versions for updates and security patchesSome packages have specific version constraints. Let's verify if these are the latest stable versions and check for any security advisories.
Let me try another approach to verify the package versions using a different NuGet API endpoint.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check NuGet packages for latest versions and known vulnerabilities # Check latest versions for each package for package in "mercadopago-sdk" "Microsoft.AspNetCore.Mvc.Core" "Microsoft.Extensions.Configuration.Binder" "Microsoft.Extensions.Logging.Abstractions" "Microsoft.Extensions.Options"; do echo "Checking $package..." curl -s "https://api.nuget.org/v3/registration5-semver1/$package/index.json" | jq -r '.items[0].upper' done # Check for known vulnerabilities using GitHub Advisory Database gh api graphql -f query=' { securityVulnerabilities(first: 10, ecosystem: NUGET, package: "mercadopago-sdk") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Length of output: 2627
Script:
#!/bin/bash # Description: Check NuGet packages using catalog API # Function to check package version check_package() { local package=$1 echo "Checking $package..." curl -s "https://api.nuget.org/v3-flatcontainer/$package/index.json" | jq -r '.versions[-1]' } # Check each package check_package "mercadopago-sdk" check_package "Microsoft.AspNetCore.Mvc.Core" check_package "Microsoft.Extensions.Configuration.Binder" check_package "Microsoft.Extensions.Logging.Abstractions" check_package "Microsoft.Extensions.Options" # Also check if there's a newer SDK version from MercadoPago curl -s "https://api.nuget.org/v3-flatcontainer/mercadopago.net/index.json" | jq -r '.versions[-1]'Length of output: 1930
src/Bmb.Payment.Controllers/PaymentGatewayFactory.cs (4)
1-5: Consider relocating PaymentGatewayFactory to the infrastructure layerThe factory is currently in the Controllers namespace but handles infrastructure concerns (service resolution). According to Clean Architecture principles, this should be moved to the infrastructure layer since it deals with concrete implementations and framework dependencies.
Consider moving this class to a more appropriate namespace like
Bmb.Payment.Infrastructure.Factories.
Line range hint
8-8: Consider abstracting service resolution from the factoryDirect dependency on IServiceProvider couples the factory to the DI container. This violates the Dependency Inversion Principle.
Consider introducing an abstraction:
- public sealed class PaymentGatewayFactory(IServiceProvider provider) : IPaymentGatewayFactoryMethod + public interface IPaymentGatewayResolver + { + IPaymentGateway ResolveGateway(string gatewayKey); + } + + public sealed class PaymentGatewayFactory(IPaymentGatewayResolver resolver) : IPaymentGatewayFactoryMethod
Line range hint
11-17: Replace string-based service resolution with strongly-typed constantsUsing string interpolation for service keys is fragile and error-prone. It makes the codebase harder to maintain and refactor.
Consider introducing constants:
+ private static class PaymentGatewayKeys + { + public const string MercadoPago = "Payment-MercadoPago"; + public const string Test = "Payment-Test"; + } var gateway = paymentType switch { - PaymentType.MercadoPago => provider.GetKeyedService<IPaymentGateway>($"Payment-{nameof(PaymentType.MercadoPago)}"), - PaymentType.Test => provider.GetKeyedService<IPaymentGateway>($"Payment-{nameof(PaymentType.Test)}"), + PaymentType.MercadoPago => resolver.ResolveGateway(PaymentGatewayKeys.MercadoPago), + PaymentType.Test => resolver.ResolveGateway(PaymentGatewayKeys.Test), _ => null };
Error handling is inconsistent across payment gateway implementations
- MercadoPagoService catches exceptions and returns null, which differs from the PaymentGatewayFactory throwing a DomainException
- FakePaymentGatewayService has no error handling at all
- This inconsistency could lead to unpredictable behavior and makes error handling harder at the consumer level
Recommendations:
- Standardize error handling across all payment gateway implementations
- Consider throwing domain-specific exceptions instead of returning null
- Document the expected error handling behavior in the IPaymentGateway interface
🔗 Analysis chain
Line range hint
19-19: Verify error handling across payment gateway implementationsThe domain exception is appropriate here, but we should ensure consistent error handling across all payment gateway implementations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all payment gateway implementations to verify error handling ast-grep --pattern 'class $_ : IPaymentGateway { $$$ }'Length of output: 59
Script:
#!/bin/bash # Search for files containing IPaymentGateway interface or implementation rg -l "IPaymentGateway" # Then search for error handling patterns in those files rg "throw new" -A 2 -B 2Length of output: 2866
Script:
#!/bin/bash # Let's examine the payment gateway implementations more closely rg -A 10 "class.*PaymentGateway.*implements.*IPaymentGateway" || rg -A 10 "class.*PaymentGateway.*:.*IPaymentGateway" # Also check the interface definition cat src/Bmb.Payment.Domain/Contracts/IPaymentGateway.csLength of output: 2785
Script:
#!/bin/bash # Let's check the MercadoPago implementation as well cat src/Bmb.Payment.MercadoPago.Gateway/MercadoPagoService.cs # And look for any try-catch blocks in payment-related files rg -A 5 "try\s*{" src/Bmb.Payment.*Length of output: 6426
src/Bmb.Payment.FakePayment.Gateway/FakePaymentGatewayService.cs (1)
12-12: Consider implementing IPaymentFactoryWhile the implementation is correct, creating domain entities directly in the gateway layer might violate DDD principles. Consider introducing a factory interface in the domain layer for creating Payment entities.
Example refactor:
+ private readonly IPaymentFactory _paymentFactory; + + public FakePaymentGatewayService(IPaymentFactory paymentFactory) + { + _paymentFactory = paymentFactory; + } public Task<Domain.Entities.Payment> CreatePaymentAsync(OrderDto order) { var id = Guid.NewGuid(); - return Task.FromResult(new Domain.Entities.Payment - { - Id = new PaymentId(id), - OrderId = order.Id, - PaymentType = PaymentType.Test, - Amount = order.Total, - Created = DateTime.Now, - Status = PaymentStatus.Pending, - QrCode = "https://fake.qrcode.com", - ExternalReference = id.ToString() - }); + return Task.FromResult(_paymentFactory.Create( + id, + order.Id, + PaymentType.Test, + order.Total, + "https://fake.qrcode.com" + ));Also applies to: 15-15
src/Bmb.Payment.MercadoPago.Gateway/ServiceCollectionsExtensions.cs (1)
Line range hint
1-33: LGTM! Clean architecture and DDD principles are well appliedThe service registration follows clean architecture principles by:
- Keeping infrastructure concerns (MercadoPago gateway) separate from domain logic
- Using dependency injection for loose coupling
- Properly using keyed services for payment gateway registration
src/Bmb.Orders.Gateway/Repository/OrderReplicaRepository.cs (5)
5-5: Consider restructuring to better reflect bounded context boundariesWhile the namespace change from
CoretoDomainaligns with DDD principles, the current structure raises some architectural concerns:
- The class name
OrderReplicaRepositorydoesn't clearly indicate its relationship with the Payment domain- The location in
Orders.Gatewaymight blur the boundaries between the Orders and Payment bounded contextsConsider renaming to
PaymentOrderReplicaRepositoryto better express its role in the Payment context.
Line range hint
13-13: Move table name to configurationThe hardcoded table name should be moved to configuration for better maintainability and environment-specific settings.
- private const string OrdersReplicaTable = "Payment-OrdersReplica"; + private readonly string _ordersReplicaTable; + + public OrderReplicaRepository(IAmazonDynamoDB database, IConfiguration configuration) + { + _ordersReplicaTable = configuration.GetValue<string>("DynamoDB:OrdersReplicaTable") + ?? throw new ArgumentNullException("DynamoDB:OrdersReplicaTable"); + }
Line range hint
29-33: Enhance error handling in GetCopyAsyncThe current error handling loses valuable context. Consider:
- Including the orderId in the error message
- Preserving the original exception details
- throw new DomainException("Error getting order from replica", e); + throw new DomainException( + $"Failed to retrieve order replica for orderId: {orderId}", + new Dictionary<string, object> { { "OrderId", orderId } }, + e);
Line range hint
35-38: Add error handling to SaveCopyAsyncThe method lacks error handling, which could lead to unhandled exceptions in production.
public Task SaveCopyAsync(OrderDto order, CancellationToken cancellationToken = default) { - return database.PutItemAsync(OrdersReplicaTable, order.ToDynamoDb(), cancellationToken); + try + { + return database.PutItemAsync(_ordersReplicaTable, order.ToDynamoDb(), cancellationToken); + } + catch (AmazonDynamoDBException e) + { + throw new DomainException( + $"Failed to save order replica for orderId: {order.Id}", + new Dictionary<string, object> { { "OrderId", order.Id } }, + e); + } }
Line range hint
9-11: Consider abstracting infrastructure dependenciesTo better align with Clean Architecture principles, consider introducing an abstraction layer for DynamoDB operations. This would:
- Make the code more testable
- Reduce coupling to AWS infrastructure
- Make it easier to change the storage implementation
-internal class OrderReplicaRepository(IAmazonDynamoDB database) : IOrdersGateway +internal class OrderReplicaRepository(IDocumentStore documentStore) : IOrdersGatewaysrc/Bmb.Payment.Application/UseCases/UpdatePaymentStatusUseCase.cs (1)
Line range hint
19-38: Consider enhancing domain model integrity and error handlingThe current implementation has several areas for improvement:
- Accepting nullable Payment might indicate a missing domain invariant
- Boolean return type doesn't effectively communicate domain outcomes
- Method mixes status update and event publishing responsibilities
Consider these improvements:
- public async Task<bool> Execute(Domain.Entities.Payment? payment, PaymentStatus status) + public async Task<PaymentUpdateResult> Execute(Domain.Entities.Payment payment, PaymentStatus status) { - if (payment is not null && payment.Status + if (payment.Status is not PaymentStatus.Approved and not PaymentStatus.Cancelled and not PaymentStatus.Rejected) { - payment.Status = status; - payment.Updated = DateTime.UtcNow; + try + { + payment.UpdateStatus(status); // Move status validation to domain entity + var updated = await _paymentRepository.UpdatePaymentStatusAsync(payment); + if (updated && payment.IsApproved()) + { + await _dispatcher.PublishAsync(new OrderPaymentConfirmed(payment.Id, payment.OrderId)); + } + return PaymentUpdateResult.Success(); + } + catch (InvalidPaymentStateException ex) + { + return PaymentUpdateResult.Failed(ex.Message); + } } - return false; + return PaymentUpdateResult.Failed("Payment status cannot be updated"); }This refactoring:
- Enforces domain invariants by not accepting null
- Uses a result object to better express domain outcomes
- Moves status update logic to the domain entity
Consider extracting event publishing to a separate domain service to better separate concerns.
src/Bmb.Payment.DynamoDb/Repository/InMemoryPaymentRepository.cs (1)
Line range hint
31-40: Improve update operation with proper validationThe update method needs similar improvements:
- Missing parameter validation
- Missing cancellation token support
- Consider using optimistic concurrency
Apply this diff:
- public Task<bool> UpdatePaymentStatusAsync(Domain.Entities.Payment payment) + public Task<bool> UpdatePaymentStatusAsync(Domain.Entities.Payment payment, CancellationToken cancellationToken = default) { + ArgumentNullException.ThrowIfNull(payment); var index = _payments.FindIndex(p => p.Id == payment.Id); if (index < 0) { return Task.FromResult(false); } + // Consider adding version check here for optimistic concurrency _payments[index] = payment; return Task.FromResult(true); }src/Bmb.Payment.Domain/Entities/Payment.cs (2)
41-41: Improve method naming and consistencyThe method could be more expressive about the business rule it represents.
Consider this improvement:
- public bool IsApproved() => Status == PaymentStatus.Approved; + public bool HasBeenApproved() => Status == PaymentStatus.Approved;
11-42: Consider enhancing domain model with events and invariantsFrom a Clean Architecture and DDD perspective, the Payment entity could benefit from additional patterns.
Consider these architectural improvements:
- Add domain events for significant state changes (e.g., PaymentApproved, PaymentRejected)
- Implement invariant checks using a Guard class
- Consider using Value Objects for Amount and ExternalReference
- Add a factory method for common creation scenarios
Example domain event:
public class PaymentApprovedEvent : DomainEvent { public PaymentId PaymentId { get; } public decimal Amount { get; } public PaymentApprovedEvent(PaymentId paymentId, decimal amount) { PaymentId = paymentId; Amount = amount; } }src/Bmb.Payment.Application/UseCases/CreatePaymentUseCase.cs (2)
26-26: Consider enhancing error handling with domain-specific result typeWhile the current implementation is solid, consider returning a domain-specific result type instead of nullable Payment to better encapsulate business outcomes and error states.
Example implementation:
- public async Task<Domain.Entities.Payment?> Execute(Guid orderId, PaymentType paymentType) + public async Task<PaymentResult> Execute(Guid orderId, PaymentType paymentType)Where
PaymentResultcould be:public class PaymentResult { public bool Success { get; } public Domain.Entities.Payment? Payment { get; } public string? ErrorMessage { get; } private PaymentResult(bool success, Domain.Entities.Payment? payment = null, string? errorMessage = null) { Success = success; Payment = payment; ErrorMessage = errorMessage; } public static PaymentResult Successful(Domain.Entities.Payment payment) => new(true, payment); public static PaymentResult Failed(string error) => new(false, errorMessage: error); }
Line range hint
13-24: Add XML documentation for public membersThe class follows clean architecture and SOLID principles well, but would benefit from XML documentation for better maintainability.
Add documentation like this:
+ /// <summary> + /// Use case for creating a new payment for an order + /// </summary> public class CreatePaymentUseCase : ICreatePaymentUseCase { private readonly IPaymentGatewayFactoryMethod _paymentGatewayFactory; private readonly IOrdersGateway _ordersGateway; private readonly IDispatcher _dispatcher; + /// <summary> + /// Initializes a new instance of the CreatePaymentUseCase + /// </summary> public CreatePaymentUseCase(...)tests/Bmb.Payment.Application.Test/UseCases/UpdatePaymentStatusUseCaseTest.cs (1)
28-28: Consider using domain-specific test buildersWhile AutoFixture is convenient, consider creating a dedicated test builder for Payment entities to ensure domain invariants and business rules are respected in tests.
Example approach:
-var payment = new Fixture().Create<Domain.Entities.Payment>(); +var payment = PaymentBuilder + .Create() + .WithDefaultValues() + .Build();src/Bmb.Payment.DynamoDb/Presenter.cs (2)
Line range hint
9-23: Add validation for required fields before mappingWhile the mapping logic is clean, consider adding validation to ensure required fields are present before creating the DynamoDB item. This would prevent potential runtime errors if the domain entity is in an invalid state.
internal static Dictionary<string, AttributeValue> ToDynamoDbItem(this Domain.Entities.Payment payment) { + if (payment == null) throw new ArgumentNullException(nameof(payment)); + if (payment.Id == null) throw new ArgumentException("Payment ID is required", nameof(payment)); + if (string.IsNullOrEmpty(payment.ExternalReference)) throw new ArgumentException("External reference is required", nameof(payment)); + return new Dictionary<string, AttributeValue>
Line range hint
25-46: Consider factory pattern for domain entity creationThe direct instantiation of the Payment entity might violate DDD principles. Consider introducing a factory method or builder pattern to ensure domain invariants and business rules are properly enforced during entity creation.
- internal static Domain.Entities.Payment ToDomain(this Dictionary<string, AttributeValue> item) + internal static Domain.Entities.Payment ToDomain(this Dictionary<string, AttributeValue> item) { + ValidateRequiredAttributes(item); + var paymentData = ExtractPaymentData(item); + return Domain.Entities.Payment.Create(paymentData); + } + + private static void ValidateRequiredAttributes(Dictionary<string, AttributeValue> item) + { + var requiredFields = new[] { "Id", "OrderId", "Status", "PaymentType", "Amount" }; + foreach (var field in requiredFields) + { + if (!item.ContainsKey(field)) + throw new KeyNotFoundException($"Required field '{field}' is missing"); + } }Additionally, consider extracting the
ConvertUpdatedDateTimehelper method to a shared utility class for reuse across the codebase.src/Bmb.Payment.DI/ServiceCollectionsExtensions.cs (3)
Line range hint
20-31: Consider separating infrastructure configurationsThe
IoCSetupmethod currently configures multiple infrastructure concerns (persistence, caching, DynamoDB, etc.) in a single place. Consider splitting these into separate extension methods by infrastructure type to improve maintainability and follow Interface Segregation Principle more strictly.Example refactoring:
- public static void IoCSetup(this IServiceCollection serviceCollection, IConfiguration configuration) + public static IServiceCollection AddPaymentInfrastructure(this IServiceCollection services, IConfiguration configuration) + { + return services + .AddPersistence(configuration) + .AddCaching(configuration) + .AddDataStorage(configuration); + } + + public static IServiceCollection AddPaymentApplicationServices(this IServiceCollection services) + { + return services + .AddUseCases() + .AddControllers() + .AddPaymentGateways(); + }
Line range hint
57-67: Consider abstracting database-specific health checksThe health check configuration is tightly coupled to DynamoDB. Consider introducing an abstraction layer to make it database-agnostic and align better with Clean Architecture principles.
Example approach:
+ public interface IStorageHealthCheck + { + string Name { get; } + Task<bool> IsHealthy(); + } - .AddDynamoDbHealthCheck("Payments") + .AddCheck<PaymentStorageHealthCheck>("Payments")
Line range hint
69-72: Avoid direct infrastructure dependencies in configurationThe direct instantiation of
AmazonDynamoDBClientcouples the configuration to a specific implementation. Consider using options pattern and factory pattern to maintain better separation of concerns.Example improvement:
+ public class DynamoDbOptions + { + public string Region { get; set; } + public string ServiceUrl { get; set; } + } - services.AddScoped<IAmazonDynamoDB>(_ => new AmazonDynamoDBClient()); + services.Configure<DynamoDbOptions>(configuration.GetSection("DynamoDb")); + services.AddScoped<IAmazonDynamoDB>(sp => { + var options = sp.GetRequiredService<IOptions<DynamoDbOptions>>().Value; + return new AmazonDynamoDBClient(new AmazonDynamoDBConfig + { + ServiceURL = options.ServiceUrl, + RegionEndpoint = RegionEndpoint.GetBySystemName(options.Region) + }); + });src/Bmb.Payment.Api/Controllers/NotificationsController.cs (4)
Line range hint
16-24: Consider encapsulating PaymentType enum in domain modelWhile the controller follows clean architecture principles with proper dependency injection, the direct usage of
PaymentTypeenum in the controller layer might violate DDD principles. Consider encapsulating this domain concept within a proper domain service or factory.Example approach:
public interface IPaymentTypeResolver { PaymentProvider ResolveFromWebhook(string provider); } // Usage in controller await _paymentService.SyncPaymentStatusWithGatewayAsync( @event.Data.Id, _paymentTypeResolver.ResolveFromWebhook("mercadopago") );
Line range hint
39-41: Validate xRequestId headerThe newly added xRequestId parameter lacks validation. Consider adding validation to ensure it's not null or empty for proper request tracing.
public async Task<IActionResult> Post([FromBody] MercadoPagoWebhookEvent @event, [FromHeader(Name = "x-signature")] string xSignature, - [FromHeader(Name = "x-request-id")] string xRequestId) + [FromHeader(Name = "x-request-id")] string xRequestId) +{ + if (string.IsNullOrEmpty(xRequestId)) + { + return BadRequest("x-request-id header is required"); + }
Line range hint
46-52: Add error handling for payment synchronizationThe background operation lacks error handling. Consider adding try-catch blocks and error logging to handle potential failures during payment synchronization.
Response.OnCompleted(async () => { using (_logger.BeginScope("Processing payment {PaymentId}", @event.Data.Id)) { + try { await _paymentService.SyncPaymentStatusWithGatewayAsync(@event.Data.Id, PaymentType.MercadoPago); + } + catch (Exception ex) + { + _logger.LogError(ex, "Failed to sync payment {PaymentId}", @event.Data.Id); + } } });
Consider using MassTransit for reliable payment status synchronization
Based on the codebase analysis, I noticed that MassTransit is already configured in the project (
Bmb.Payment.Masstransit.csprojand related dependencies). Instead of usingResponse.OnCompleted, you should leverage the existing message queue infrastructure for reliable payment status synchronization:
- Move the payment synchronization logic to a MassTransit consumer
- Publish a payment status update event from the webhook handler
- This ensures reliable processing even if the application restarts
🔗 Analysis chain
Line range hint
44-54: Consider using a background service instead of Response.OnCompletedUsing
Response.OnCompletedfor payment status synchronization might lead to lost operations if the application shuts down before the task completes. Consider using a proper background service or message queue for reliability.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there's already a background service implementation ast-grep --pattern 'class $_ : BackgroundService'Length of output: 51
Script:
#!/bin/bash # Let's check for any background processing or queue-related implementations rg -i "background|queue|worker|processor" --type cs -A 5 # Also check for any hosted services ast-grep --pattern 'class $_ : IHostedService' # Look for any message broker or queue configurations rg -i "rabbitmq|servicebus|kafka|message.*queue" --type csLength of output: 600
Script:
#!/bin/bash # Let's check the project files for any message processing infrastructure fd -e csproj -x cat {} # Check for any dependency injection configurations related to background processing rg -i "addhosted|addbackground|addscoped.*service|addsingleton.*service" --type cs -A 2 # Look for any existing payment processing patterns rg -i "payment.*process|process.*payment" --type cs -A 5Length of output: 21196
src/Bmb.Payment.DynamoDb/Repository/PaymentRepository.cs (4)
11-13: Consider externalizing DynamoDB configurationThe table name is hardcoded as a constant. This could make it difficult to change the configuration for different environments.
Consider moving the table name to configuration:
- private const string PaymentTable = "Payments"; + private readonly string _paymentTable; + public PaymentRepository(IAmazonDynamoDB database, IConfiguration configuration) + { + _paymentTable = configuration.GetValue<string>("DynamoDB:PaymentTable") ?? "Payments"; + }
Line range hint
14-39: Enhance error handling for database operationsThe current implementation doesn't handle potential DynamoDB exceptions, which could lead to unhandled errors in the application layer.
Consider adding proper exception handling:
public async Task<Domain.Entities.Payment> SaveAsync(Domain.Entities.Payment payment) { + ArgumentNullException.ThrowIfNull(payment); + try + { await database.PutItemAsync(PaymentTable, payment.ToDynamoDbItem()); return payment; + } + catch (AmazonDynamoDBException ex) + { + throw new RepositoryException("Failed to save payment", ex); + } }
Line range hint
64-81: Improve update operation robustnessThe current implementation has several potential improvements:
- Boolean return type provides limited error information
- No optimistic concurrency control
- Direct dependency on HTTP status codes leaks infrastructure details
Consider implementing these improvements:
- public async Task<bool> UpdatePaymentStatusAsync(Domain.Entities.Payment payment) + public async Task UpdatePaymentStatusAsync(Domain.Entities.Payment payment, string? expectedVersion = null) { var attributes = payment.ToDynamoDbItem(); attributes.Remove("Id"); + var conditionExpression = expectedVersion != null ? "Version = :expectedVersion" : null; var request = new UpdateItemRequest { TableName = PaymentTable, Key = new Dictionary<string, AttributeValue> { { "Id", new AttributeValue { S = payment.Id.Value.ToString() } } }, - AttributeUpdates = attributes.ToDictionary(x => x.Key, x => new AttributeValueUpdate { Action = AttributeAction.PUT, Value = x.Value }) + AttributeUpdates = attributes.ToDictionary(x => x.Key, x => new AttributeValueUpdate { Action = AttributeAction.PUT, Value = x.Value }), + ConditionExpression = conditionExpression }; - var response = await database.UpdateItemAsync(request); - return response.HttpStatusCode == HttpStatusCode.OK; + try + { + await database.UpdateItemAsync(request); + } + catch (ConditionalCheckFailedException) + { + throw new ConcurrencyException($"Payment {payment.Id} was modified by another process"); + } }
Line range hint
1-81: Consider enhancing the repository pattern implementationWhile the current implementation follows basic DDD principles, consider these architectural improvements:
- Add a Unit of Work pattern for transaction management
- Implement repository-specific interfaces for better domain alignment
- Add domain event publishing capability
This would better support domain-driven design and maintain a cleaner separation of concerns. Would you like me to provide example implementations for any of these patterns?
tests/Bmb.Payment.Api.Test/PaymentGatewayFactoryTest.cs (3)
3-4: Namespace changes align with DDD principlesThe migration from
CoretoDomainnamespace improves domain isolation and better reflects the bounded context of payments. This change supports the separation of concerns and reduces coupling between domains.
Line range hint
17-91: Consider enhancing test coverage and maintainabilityWhile the test structure is solid, consider these improvements:
- Add test for null serviceProvider scenario
- Extract the "Payment-" prefix to a constant
- Consider using test data constants for better maintainability
Example improvement:
public class PaymentGatewayFactoryTest { + private const string PAYMENT_PREFIX = "Payment-"; private readonly IServiceCollection _serviceCollection; // ... existing code ... [Fact] public void Create_MercadoPago_ReturnsCorrectService() { // Arrange _serviceCollection.AddKeyedScoped<IPaymentGateway, MercadoPagoService>( - $"Payment-{nameof(PaymentType.MercadoPago)}"); + $"{PAYMENT_PREFIX}{nameof(PaymentType.MercadoPago)}"); // ... rest of the test ... } + [Fact] + public void Create_NullServiceProvider_ThrowsException() + { + // Arrange + IServiceProvider serviceProvider = null; + + // Act + var action = () => new PaymentGatewayFactory(serviceProvider); + + // Assert + action.Should().ThrowExactly<ArgumentNullException>(); + }
Line range hint
23-33: Consider decoupling gateway configurationsThe test setup reveals tight coupling between payment gateways and their specific configurations. Consider introducing a configuration abstraction to make the gateways more modular and easier to test.
Example approach:
public interface IPaymentGatewayConfiguration { string GetConfigurationValue(string key); } public class MercadoPagoConfiguration : IPaymentGatewayConfiguration { private readonly MercadoPagoOptions _options; public MercadoPagoConfiguration(MercadoPagoOptions options) => _options = options; public string GetConfigurationValue(string key) => key switch { "AccessToken" => _options.AccessToken, "NotificationUrl" => _options.NotificationUrl, "WebhookSecret" => _options.WebhookSecret, _ => throw new ArgumentException($"Unknown configuration key: {key}") }; }tests/Bmb.Payment.DynamoDb.Test/Repository/PaymentRepositoryTest.cs (4)
29-29: LGTM! The entity namespace change aligns with DDD principles.The movement of the Payment entity from Core to Domain namespace better reflects the domain-centric architecture, improving the separation of concerns.
Consider adding more specific assertions to verify the persistence of domain invariants and business rules, strengthening the domain model validation.
49-49: Enhance test coverage with comprehensive assertions.While the entity namespace change is correct, the test assertions could be more thorough.
Consider adding assertions for all relevant payment properties:
using (new AssertionScope()) { result.Should().NotBeNull(); + result.Id.Should().Be(expectedPayment.Id); + result.ExternalReference.Should().Be(expectedPayment.ExternalReference); + result.PaymentType.Should().Be(expectedPayment.PaymentType); result.Status.Should().Be(expectedPayment.Status); }
72-72: Consider adding edge case tests for the external reference lookup.While the namespace change is correct, the test coverage could be improved.
Consider adding the following test cases:
- When multiple payments exist with the same external reference but different payment types
- When no payment exists for the given external reference
- When the external reference is null or empty
98-98: Consider enhancing the status update operation design.While the namespace change aligns with DDD, the current implementation could better reflect domain-driven design principles.
Suggestions for improvement:
- Instead of returning a boolean, consider returning the updated Payment entity or using a Result/Either type to better handle domain outcomes
- Add assertions to verify that the status was actually updated in DynamoDB
- Consider implementing status transition rules as part of the domain model to ensure valid state changes
tests/Bmb.Payment.Application.Test/UseCases/CreatePaymentUseCaseTest.cs (2)
6-8: Good domain separation!The namespace restructuring from
CoretoDomainbetter reflects DDD principles by explicitly separating the payment domain. The addition ofValueObjectsnamespace also shows proper domain modeling.Consider maintaining this clear boundary separation in future changes to ensure the payment domain remains cohesive and independent.
42-45: Consider enhancing test data generationThe Payment entity construction is correctly updated to use the new domain namespace. However, we could improve the test maintainability.
Consider creating a custom AutoFixture customization for Payment entities:
public class PaymentCustomization : ICustomization { public void Customize(IFixture fixture) { fixture.Customize<Domain.Entities.Payment>(composer => composer .With(p => p.Id, () => new PaymentId(Guid.NewGuid())) .With(p => p.Status, PaymentStatus.Pending)); } }Then use it in tests:
var fixture = new Fixture().Customize(new PaymentCustomization()); var expectedPayment = fixture.Create<Domain.Entities.Payment>();This would:
- Centralize domain entity creation logic
- Make tests more maintainable
- Ensure consistent test data across all tests
tests/Bmb.Payment.Controllers.Test/PaymentServiceTests.cs (1)
44-44: Maintain consistency in entity referencesWhile both lines correctly reference the domain entity, they use different styles:
- Line 44:
Domain.Entities.Payment- Line 54:
Bmb.Payment.Domain.Entities.PaymentConsider using the same reference style throughout for better maintainability.
- .ReturnsAsync(new Fixture().Create<Domain.Entities.Payment>()); + .ReturnsAsync(new Fixture().Create<Bmb.Payment.Domain.Entities.Payment>());Also applies to: 54-54
src/Bmb.Payment.MercadoPago.Gateway/MercadoPagoService.cs (4)
Line range hint
16-31: Consider extracting mapping logic to improve SRP adherenceWhile the service correctly acts as an anti-corruption layer between MercadoPago and the domain, it currently handles both integration logic and object mapping. Consider extracting the mapping methods into a dedicated
MercadoPagoMapperclass to better adhere to the Single Responsibility Principle.Example structure:
public class MercadoPagoMapper { private const decimal IntegrationPrice = 0.01M; public PaymentCreateRequest MapToPaymentRequest(OrderDto order) { // Move mapping logic here } public DomainPaymentStatus MapToPaymentStatus(string status) { // Move status mapping here } }
Line range hint
33-65: Improve domain invariant handling and error managementSeveral improvements could strengthen the domain integrity:
- Instead of returning null on errors, consider using a Result pattern or throwing domain-specific exceptions
- PaymentId generation should be handled by the domain layer
- Consider introducing domain-specific error types
Example implementation:
public async Task<DomainPayment> CreatePaymentAsync(OrderDto order) { try { var mercadoPagoPayment = await CreateMercadoPagoPaymentAsync(order); return _paymentFactory.Create( // Factory from domain mercadoPagoPayment.Status, mercadoPagoPayment.PointOfInteraction.TransactionData.QrCode, mercadoPagoPayment.Id.ToString(), order.Id, paymentCreateRequest.TransactionAmount.Value ); } catch (Exception e) { _logger.LogError(e, "Error creating payment for Order {OrderId}", order.Id); throw new PaymentCreationFailedException(order.Id, e); // Domain exception } }
Line range hint
98-113: Strengthen domain boundaries in status handlingThe current implementation exposes infrastructure concerns to the domain layer:
- Consider using a non-nullable domain status with an "Unknown" state instead of returning null
- Move status mapping to a dedicated value object in the domain layer
Example approach:
public async Task<DomainPaymentStatus> GetPaymentStatusAsync(string paymentId) { var requestOptions = new RequestOptions() { AccessToken = _mercadoPagoOptions.AccessToken, }; var client = new PaymentClient(); var payment = await client.GetAsync(long.Parse(paymentId), requestOptions); return DomainPaymentStatus.FromExternalStatus(payment?.Status ?? "unknown"); }
Line range hint
115-167: Move hard-coded values to configurationSeveral values are currently hard-coded that should be configurable:
- Email "guest@mercadofiado.com"
- Identification type "CPF"
- Category "food"
- Payment method "pix"
- Statement descriptor
- Expiration time (15 minutes)
Consider moving these to the
MercadoPagoOptionsconfiguration class:public class MercadoPagoOptions { public string AccessToken { get; set; } public string WebhookSecret { get; set; } public string GuestEmail { get; set; } public string DefaultCategory { get; set; } public string PaymentMethod { get; set; } public string StatementDescriptor { get; set; } public TimeSpan PaymentExpiration { get; set; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (55)
Bmb Payment.sln(1 hunks)src/Bmb.Orders.Gateway/Bmb.Orders.Gateway.csproj(1 hunks)src/Bmb.Orders.Gateway/Presenter.cs(1 hunks)src/Bmb.Orders.Gateway/Repository/InMemoryOrdersGateway.cs(1 hunks)src/Bmb.Orders.Gateway/Repository/OrderReplicaRepository.cs(1 hunks)src/Bmb.Orders.Gateway/ServiceCollectionsExtensions.cs(1 hunks)src/Bmb.Payment.Api/Controllers/NotificationsController.cs(1 hunks)src/Bmb.Payment.Api/Mappers.cs(1 hunks)src/Bmb.Payment.Application/Bmb.Payment.Application.csproj(1 hunks)src/Bmb.Payment.Application/UseCases/CreatePaymentUseCase.cs(2 hunks)src/Bmb.Payment.Application/UseCases/ICreatePaymentUseCase.cs(1 hunks)src/Bmb.Payment.Application/UseCases/IUpdatePaymentStatusUseCase.cs(1 hunks)src/Bmb.Payment.Application/UseCases/UpdatePaymentStatusUseCase.cs(2 hunks)src/Bmb.Payment.Controllers/Contracts/IPaymentService.cs(1 hunks)src/Bmb.Payment.Controllers/Dto/CreateOrderPaymentRequestDto.cs(1 hunks)src/Bmb.Payment.Controllers/PaymentGatewayFactory.cs(1 hunks)src/Bmb.Payment.Controllers/PaymentService.cs(1 hunks)src/Bmb.Payment.Controllers/Presenter.cs(1 hunks)src/Bmb.Payment.Core/Contracts/IPaymentGateway.cs(0 hunks)src/Bmb.Payment.Core/Contracts/IPaymentGatewayFactoryMethod.cs(0 hunks)src/Bmb.Payment.Core/Contracts/IPaymentRepository.cs(0 hunks)src/Bmb.Payment.DI/ServiceCollectionsExtensions.cs(1 hunks)src/Bmb.Payment.Domain/Bmb.Payment.Domain.csproj(1 hunks)src/Bmb.Payment.Domain/Contracts/IPaymentGateway.cs(1 hunks)src/Bmb.Payment.Domain/Contracts/IPaymentGatewayFactoryMethod.cs(1 hunks)src/Bmb.Payment.Domain/Contracts/IPaymentRepository.cs(1 hunks)src/Bmb.Payment.Domain/Entities/Payment.cs(1 hunks)src/Bmb.Payment.Domain/IOrdersGateway.cs(1 hunks)src/Bmb.Payment.Domain/OrderReplicaDto.cs(1 hunks)src/Bmb.Payment.Domain/ValueObjects/PaymentStatus.cs(1 hunks)src/Bmb.Payment.Domain/ValueObjects/PaymentType.cs(1 hunks)src/Bmb.Payment.DynamoDb/Bmb.Payment.DynamoDb.csproj(1 hunks)src/Bmb.Payment.DynamoDb/Presenter.cs(2 hunks)src/Bmb.Payment.DynamoDb/Repository/InMemoryPaymentRepository.cs(1 hunks)src/Bmb.Payment.DynamoDb/Repository/PaymentRepository.cs(3 hunks)src/Bmb.Payment.DynamoDb/ServiceExtensions.cs(1 hunks)src/Bmb.Payment.FakePayment.Gateway/Bmb.Payment.FakePayment.Gateway.csproj(1 hunks)src/Bmb.Payment.FakePayment.Gateway/FakePaymentGatewayService.cs(1 hunks)src/Bmb.Payment.FakePayment.Gateway/ServiceCollectionsExtensions.cs(1 hunks)src/Bmb.Payment.Masstransit/Bmb.Payment.Masstransit.csproj(1 hunks)src/Bmb.Payment.Masstransit/Dispatcher.cs(2 hunks)src/Bmb.Payment.Masstransit/OrderCreatedPaymentConsumer.cs(1 hunks)src/Bmb.Payment.Masstransit/Presenter.cs(1 hunks)src/Bmb.Payment.Masstransit/ServiceCollectionsExtensions.cs(1 hunks)src/Bmb.Payment.MercadoPago.Gateway/Bmb.Payment.MercadoPago.Gateway.csproj(1 hunks)src/Bmb.Payment.MercadoPago.Gateway/MercadoPagoService.cs(1 hunks)src/Bmb.Payment.MercadoPago.Gateway/ServiceCollectionsExtensions.cs(1 hunks)tests/Bmb.Orders.Gateway.Test/Repository/OrderReplicaRepositoryTest.cs(1 hunks)tests/Bmb.Payment.Api.Test/Controllers/NotificationsControllerTest.cs(3 hunks)tests/Bmb.Payment.Api.Test/PaymentGatewayFactoryTest.cs(1 hunks)tests/Bmb.Payment.Application.Test/UseCases/CreatePaymentUseCaseTest.cs(2 hunks)tests/Bmb.Payment.Application.Test/UseCases/UpdatePaymentStatusUseCaseTest.cs(2 hunks)tests/Bmb.Payment.Controllers.Test/PaymentServiceTests.cs(4 hunks)tests/Bmb.Payment.DynamoDb.Test/Repository/PaymentRepositoryTest.cs(4 hunks)tests/Bmb.Payment.Masstransit.Test/OrderCreatedPaymentConsumerTest.cs(1 hunks)
💤 Files with no reviewable changes (3)
- src/Bmb.Payment.Core/Contracts/IPaymentGateway.cs
- src/Bmb.Payment.Core/Contracts/IPaymentGatewayFactoryMethod.cs
- src/Bmb.Payment.Core/Contracts/IPaymentRepository.cs
✅ Files skipped from review due to trivial changes (15)
- Bmb Payment.sln
- src/Bmb.Orders.Gateway/Presenter.cs
- src/Bmb.Orders.Gateway/Repository/InMemoryOrdersGateway.cs
- src/Bmb.Payment.Controllers/Contracts/IPaymentService.cs
- src/Bmb.Payment.Controllers/Dto/CreateOrderPaymentRequestDto.cs
- src/Bmb.Payment.Controllers/PaymentService.cs
- src/Bmb.Payment.Controllers/Presenter.cs
- src/Bmb.Payment.Domain/Bmb.Payment.Domain.csproj
- src/Bmb.Payment.Domain/IOrdersGateway.cs
- src/Bmb.Payment.Domain/OrderReplicaDto.cs
- src/Bmb.Payment.Domain/ValueObjects/PaymentStatus.cs
- src/Bmb.Payment.Domain/ValueObjects/PaymentType.cs
- src/Bmb.Payment.Masstransit/Presenter.cs
- tests/Bmb.Orders.Gateway.Test/Repository/OrderReplicaRepositoryTest.cs
- tests/Bmb.Payment.Masstransit.Test/OrderCreatedPaymentConsumerTest.cs
🧰 Additional context used
📓 Path-based instructions (31)
src/Bmb.Orders.Gateway/Repository/OrderReplicaRepository.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Orders.Gateway/ServiceCollectionsExtensions.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.Api/Controllers/NotificationsController.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.Api/Mappers.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.Application/UseCases/CreatePaymentUseCase.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.Application/UseCases/ICreatePaymentUseCase.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.Application/UseCases/IUpdatePaymentStatusUseCase.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.Application/UseCases/UpdatePaymentStatusUseCase.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.Controllers/PaymentGatewayFactory.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.DI/ServiceCollectionsExtensions.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.Domain/Contracts/IPaymentGateway.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.Domain/Contracts/IPaymentGatewayFactoryMethod.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.Domain/Contracts/IPaymentRepository.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.Domain/Entities/Payment.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.DynamoDb/Presenter.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.DynamoDb/Repository/InMemoryPaymentRepository.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.DynamoDb/Repository/PaymentRepository.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.DynamoDb/ServiceExtensions.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.FakePayment.Gateway/FakePaymentGatewayService.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.FakePayment.Gateway/ServiceCollectionsExtensions.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.Masstransit/Dispatcher.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.Masstransit/OrderCreatedPaymentConsumer.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.Masstransit/ServiceCollectionsExtensions.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.MercadoPago.Gateway/MercadoPagoService.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
src/Bmb.Payment.MercadoPago.Gateway/ServiceCollectionsExtensions.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
tests/Bmb.Payment.Api.Test/Controllers/NotificationsControllerTest.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
tests/Bmb.Payment.Api.Test/PaymentGatewayFactoryTest.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
tests/Bmb.Payment.Application.Test/UseCases/CreatePaymentUseCaseTest.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
tests/Bmb.Payment.Application.Test/UseCases/UpdatePaymentStatusUseCaseTest.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
tests/Bmb.Payment.Controllers.Test/PaymentServiceTests.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
tests/Bmb.Payment.DynamoDb.Test/Repository/PaymentRepositoryTest.cs (1)
Pattern **/*.cs: Validate all files and structures against Domain Driven Design, SOLID and Clean Architecture principles.
🔇 Additional comments (39)
src/Bmb.Payment.Application/UseCases/IUpdatePaymentStatusUseCase.cs (1)
7-7: Consider adding parameter validation
The nullable payment parameter might lead to runtime issues if not properly validated. Consider either:
- Making the parameter non-nullable if a payment is always required
- Adding validation logic in the implementing class
src/Bmb.Payment.Domain/Contracts/IPaymentGatewayFactoryMethod.cs (1)
5-10: Verify the integration with existing payment gateways
Let's ensure this new factory interface aligns with existing payment gateway implementations.
✅ Verification successful
Let me gather more information about the implementation details of the factory and gateways to ensure proper integration.
The factory interface is properly integrated with existing payment gateways
The verification shows that:
- The
PaymentGatewayFactorycorrectly implementsIPaymentGatewayFactoryMethod - Two payment gateways (
MercadoPagoServiceandFakePaymentGatewayService) implementIPaymentGateway - The factory's
Createmethod handles all enum values fromPaymentType(Test and MercadoPago) - The implementation uses keyed services for dependency injection, ensuring proper gateway resolution
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing payment gateway implementations
echo "Searching for payment gateway implementations..."
rg -t cs "class.*:.*IPaymentGateway" -g "!obj/*"
# Search for existing factory implementations
echo "Searching for existing factory implementations..."
rg -t cs "class.*:.*IPaymentGatewayFactory" -g "!obj/*"
# Search for PaymentType enum usage
echo "Searching for PaymentType enum usage..."
ast-grep --pattern 'enum PaymentType'
Length of output: 1309
Script:
#!/bin/bash
# Check PaymentGatewayFactory implementation
echo "Checking PaymentGatewayFactory implementation..."
rg -t cs -A 10 "class PaymentGatewayFactory"
# Check MercadoPago and FakePayment gateway implementations
echo "Checking gateway service implementations..."
rg -t cs -A 5 "class MercadoPagoService"
rg -t cs -A 5 "class FakePaymentGatewayService"
# Check PaymentType enum definition
echo "Checking PaymentType enum definition..."
rg -t cs -A 10 "enum PaymentType"
Length of output: 4409
src/Bmb.Orders.Gateway/ServiceCollectionsExtensions.cs (1)
Line range hint 1-15: LGTM! Clean DI registration implementation
The service collection extension follows good practices:
- Single responsibility for DI configuration
- Interface-based registration
- Proper scoping of dependencies
src/Bmb.Payment.FakePayment.Gateway/ServiceCollectionsExtensions.cs (1)
2-3: LGTM! Namespace changes align with DDD principles
The transition from .Core to .Domain better reflects the bounded context of the payment domain and improves the architectural clarity.
src/Bmb.Payment.DynamoDb/ServiceExtensions.cs (1)
2-2: LGTM! Clean Architecture compliance verified.
The namespace change from Core to Domain better reflects the DDD boundaries and maintains proper Clean Architecture layering, with infrastructure (DynamoDb) depending on domain contracts.
src/Bmb.Payment.Api/Mappers.cs (1)
3-3: LGTM: Clean Architecture alignment
The namespace change from Core to Domain.ValueObjects properly reflects the domain-centric organization of the codebase and maintains clean architectural boundaries.
src/Bmb.Payment.Application/Bmb.Payment.Application.csproj (2)
16-16: LGTM! Project reference update aligns with domain-driven design
The change from Core to Domain project reference properly reflects the architectural shift towards domain-driven design, where the domain layer encapsulates the business logic and rules.
16-16: Verify all project references are updated consistently
Let's ensure all projects depending on the former Core project have been updated to reference the new Domain project.
✅ Verification successful
Project references have been updated correctly
The verification shows that there are no remaining references to the old Core project, and all relevant projects are properly referencing the new Domain project. The following projects have the correct Domain project reference:
- Bmb.Orders.Gateway
- Bmb.Payment.Application
- Bmb.Payment.DynamoDb
- Bmb.Payment.FakePayment.Gateway
- Bmb.Payment.Masstransit
- Bmb.Payment.MercadoPago.Gateway
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to the old Core project
# and verify all projects are referencing the new Domain project
# Search for any remaining references to Core project
echo "Checking for remaining references to Core project..."
rg -l "Bmb\.Payment\.Core" --type csproj
# Search for projects referencing the new Domain project
echo "Checking projects referencing the new Domain project..."
rg -l "Bmb\.Payment\.Domain" --type csproj
Length of output: 385
Script:
#!/bin/bash
# Let's try a different approach using find and grep to locate .csproj files
# and check their contents
# Find all .csproj files
find . -name "*.csproj" -type f -exec sh -c '
echo "=== Checking file: $1 ==="
grep -H "Bmb\.Payment\.\(Core\|Domain\)" "$1"
' sh {} \;
Length of output: 2508
src/Bmb.Payment.Domain/Contracts/IPaymentRepository.cs (1)
7-16: Verify repository implementation patterns across the domain
The interface follows repository pattern but let's verify consistency across the codebase.
✅ Verification successful
Let me try a different approach to verify the repository patterns.
Repository pattern implementation is consistent across the codebase
The verification shows consistent repository pattern implementation:
- Interface
IPaymentRepositoryfollows standard async repository pattern with CRUD operations - Two implementations found:
PaymentRepositoryandInMemoryPaymentRepository, both correctly implementing the interface - All repository methods consistently use async/await pattern with Task return types
- Method naming conventions are consistent with *Async suffix
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent repository patterns across the domain
# Check for other repository interfaces
echo "Checking repository interface patterns..."
ast-grep --pattern 'interface I$_Repository
{
$$$
}'
# Check for repository implementations
echo "Checking repository implementations..."
ast-grep --pattern 'class $_Repository : I$_Repository
{
$$$
}'
Length of output: 304
Script:
#!/bin/bash
# Search for all repository interfaces and implementations with ripgrep for better coverage
echo "Searching for repository interfaces..."
rg "interface I\w+Repository" -g "*.cs" -A 5
echo -e "\nSearching for repository implementations..."
rg "class \w+Repository\s*:\s*I\w+Repository" -g "*.cs" -A 5
echo -e "\nSearching for repository method patterns..."
rg "Task<.*?>\s+\w+Async" -g "*.cs" --type cs
Length of output: 6095
src/Bmb.Payment.Masstransit/Bmb.Payment.Masstransit.csproj (1)
16-16: LGTM! Project reference update aligns with domain separation.
The change from Bmb.Payment.Core to Bmb.Payment.Domain correctly reflects the architectural shift towards better domain separation.
Let's verify the consistency of this architectural change across the solution:
✅ Verification successful
Project reference update is consistent across the solution
The verification shows that:
- No references to the old
Bmb.Payment.Coreproject remain in any .csproj files - The new
Bmb.Payment.Domainproject is properly referenced by all dependent projects - The solution file has been updated to include the new project structure
This confirms that the architectural change has been consistently implemented across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all references to the old Core project have been updated to Domain
# and that there are no lingering references that might cause build issues.
# Check for any remaining references to the old Core project
echo "Checking for any remaining references to Bmb.Payment.Core:"
rg -l "Bmb\.Payment\.Core" --type csproj
# Check for references to the new Domain project to ensure consistency
echo "Checking references to Bmb.Payment.Domain:"
rg -l "Bmb\.Payment\.Domain" --type csproj
# Look for potential broken references in solution files
echo "Checking solution files for project references:"
rg -l "Bmb\.Payment\.(Core|Domain)" --type sln
Length of output: 553
Script:
#!/bin/bash
# Search for project files and solution files without relying on file type
echo "Checking for any remaining references to Bmb.Payment.Core:"
fd -e csproj -x rg -l "Bmb\.Payment\.Core" {}
echo -e "\nChecking references to Bmb.Payment.Domain:"
fd -e csproj -x rg -l "Bmb\.Payment\.Domain" {}
echo -e "\nChecking solution files for project references:"
fd -e sln -x rg -l "Bmb\.Payment\.(Core|Domain)" {}
Length of output: 896
src/Bmb.Orders.Gateway/Bmb.Orders.Gateway.csproj (1)
16-16: LGTM! Project reference update aligns with domain separation.
The change from Bmb.Payment.Core to Bmb.Payment.Domain correctly reflects the architectural decision to separate the payment domain.
Let's verify the project reference:
✅ Verification successful
Project reference update verified and no legacy references found
The verification confirms:
- The referenced project
Bmb.Payment.Domain.csprojexists at the correct location - No remaining references to the old
Bmb.Payment.Corenamespace were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the new project reference exists and the solution builds correctly
# Check if the referenced project exists
fd -t f "Bmb.Payment.Domain.csproj$"
# Check for any broken references in the solution
rg -l "Bmb\.Payment\.Core"
Length of output: 116
src/Bmb.Payment.Masstransit/OrderCreatedPaymentConsumer.cs (1)
2-2: Verify complete namespace migration
Let's ensure all references to the old Core namespace have been updated consistently.
✅ Verification successful
No remaining references to old Core namespace found
Based on the comprehensive search results, there are no remaining references to the old Bmb.Payment.Core namespace in the codebase. All relevant code is properly using either Bmb.Payment.Domain or other current namespaces. The namespace migration appears to be complete and consistent.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to the old namespace
rg "Bmb\.Payment\.Core" --type cs
# Search for potential missed interface implementations
ast-grep --pattern 'using Bmb.Payment.Core;'
Length of output: 82
Script:
#!/bin/bash
# Let's first check all C# files in the repository to understand the namespace structure
fd -e cs -x head -n 5 {}
# Then search for any references to Payment.Domain namespace to verify the migration
rg "Bmb\.Payment\.Domain" --type cs -C 2
# Also check for any potential interface implementations or type references
ast-grep --pattern 'namespace Bmb.Payment.$_'
Length of output: 33835
src/Bmb.Payment.MercadoPago.Gateway/Bmb.Payment.MercadoPago.Gateway.csproj (1)
18-18: LGTM: Project reference updated correctly
The change from Core to Domain project reference aligns well with the PR objective of separating the payment domain.
src/Bmb.Payment.FakePayment.Gateway/Bmb.Payment.FakePayment.Gateway.csproj (1)
25-25: LGTM! Project reference updated correctly.
The change from Core to Domain aligns with the PR objective of separating the payment domain.
Let's verify that all test cases have been updated to use the new domain namespace:
✅ Verification successful
Project reference and namespace updates verified successfully
The verification shows that:
- No references to the old
Bmb.Payment.Corenamespace were found in the codebase - Both files (
FakePaymentGatewayService.csandServiceCollectionsExtensions.cs) are properly using the newBmb.Payment.Domainnamespace
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to the old Core namespace in test files
# Expected: No occurrences of the old namespace should be found
# Search for any remaining references to Bmb.Payment.Core in test files
rg -l "Bmb\.Payment\.Core" --type cs $(fd -e cs . "src/Bmb.Payment.FakePayment.Gateway/")
# Search for test files using the new domain namespace to confirm proper updates
rg -l "Bmb\.Payment\.Domain" --type cs $(fd -e cs . "src/Bmb.Payment.FakePayment.Gateway/")
Length of output: 580
Script:
#!/bin/bash
# Let's check the actual content of these files to see if there are any remaining Core references
rg "Bmb\.Payment\.Core" -A 2 -B 2 src/Bmb.Payment.FakePayment.Gateway/FakePaymentGatewayService.cs src/Bmb.Payment.FakePayment.Gateway/ServiceCollectionsExtensions.cs
Length of output: 168
src/Bmb.Payment.DynamoDb/Bmb.Payment.DynamoDb.csproj (2)
24-24: LGTM! Project reference updated correctly
The project reference has been properly updated to reflect the new domain-driven structure, changing from Core to Domain.
24-24: Verify solution-wide impact of the domain separation
Let's verify that all necessary project references have been updated consistently across the solution.
✅ Verification successful
Let me verify the content of the solution file to understand the project structure better.
Domain separation has been properly implemented across the solution
The verification shows that:
- No references to the old
Bmb.Payment.Coreproject remain in the solution - The new
Bmb.Payment.Domainproject is correctly referenced by all dependent projects:- Bmb.Payment.Application
- Bmb.Payment.DynamoDb
- Bmb.Payment.MercadoPago.Gateway
- Bmb.Payment.FakePayment.Gateway
- Bmb.Payment.Masstransit
- Bmb.Orders.Gateway
The domain separation has been consistently implemented across the entire solution.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to Bmb.Payment.Core across the solution
# Expected: No references to the old Core project should remain
# Search for any remaining references to the old Core project
rg -l "Bmb\.Payment\.Core" --type-add 'proj:*.{csproj,sln}' --type proj
# Search for references to the new Domain project to ensure consistency
rg -l "Bmb\.Payment\.Domain" --type-add 'proj:*.{csproj,sln}' --type proj
Length of output: 535
Script:
#!/bin/bash
# Check the content of the solution file to see all project references
cat "Bmb Payment.sln"
# Also check the content of project files that reference Domain
for proj in $(rg -l "Bmb\.Payment\.Domain" --type-add 'proj:*.{csproj,sln}' --type proj); do
if [[ "$proj" == *.csproj ]]; then
echo "=== $proj ==="
cat "$proj"
fi
done
Length of output: 18993
src/Bmb.Payment.FakePayment.Gateway/FakePaymentGatewayService.cs (2)
3-5: LGTM: Clean Architecture compliance
The updated imports correctly reference the domain layer, adhering to Clean Architecture principles where dependencies point inward toward the domain.
Line range hint 1-31: Verify test coverage exclusion
The class is marked with [ExcludeFromCodeCoverage]. Since this is a fake implementation for testing, verify if other test classes depend on this implementation.
✅ Verification successful
Let me check the actual usage of this class in the test file to understand if excluding it from code coverage is appropriate.
Code coverage exclusion is appropriate
The [ExcludeFromCodeCoverage] attribute is correctly applied to FakePaymentGatewayService as it's a test double used only in PaymentGatewayFactoryTest.cs for verifying the payment gateway factory's behavior. The class serves as a fake implementation for testing purposes and doesn't need code coverage itself.
- The class is registered with dependency injection for test payment type
- It's used to verify that the factory correctly instantiates the fake gateway for test scenarios
- No other test dependencies were found that would require coverage of this implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for references to FakePaymentGatewayService in test files
rg -l "FakePaymentGatewayService" | rg -i "test"
Length of output: 102
Script:
#!/bin/bash
# Check how FakePaymentGatewayService is used in the test file
rg -A 5 "FakePaymentGatewayService" "tests/Bmb.Payment.Api.Test/PaymentGatewayFactoryTest.cs"
Length of output: 543
src/Bmb.Payment.MercadoPago.Gateway/ServiceCollectionsExtensions.cs (1)
3-4: Verify namespace changes across the codebase
The namespace changes from Core to Domain align well with DDD principles by properly separating domain concerns. Let's verify the consistency of these changes across the codebase.
✅ Verification successful
The results show no remaining references to Core.Contracts and consistent usage of Domain.Contracts across the codebase. Let me verify the ValueObjects namespace usage to ensure complete consistency.
Namespace changes are consistent across the codebase
The verification confirms that:
- All references to
Core.Contractshave been completely removed Domain.Contractsis consistently used across the codebaseDomain.ValueObjectsis properly referenced across multiple components- No legacy references to
Core.ValueObjectsremain
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all Core.Contracts references have been updated to Domain.Contracts
# Check for any remaining references to Core.Contracts
echo "Checking for any remaining references to Core.Contracts..."
rg -l "Bmb\.Payment\.Core\.Contracts"
# Check for consistent usage of Domain.Contracts
echo "Verifying consistent usage of Domain.Contracts..."
rg -l "Bmb\.Payment\.Domain\.Contracts"
Length of output: 1518
Script:
#!/bin/bash
# Check for consistent usage of Domain.ValueObjects
echo "Verifying consistent usage of Domain.ValueObjects..."
rg -l "Bmb\.Payment\.Domain\.ValueObjects"
# Check for any remaining references to Core.ValueObjects
echo "Checking for any remaining references to Core.ValueObjects..."
rg -l "Bmb\.Payment\.Core\.ValueObjects"
Length of output: 1913
src/Bmb.Payment.Masstransit/ServiceCollectionsExtensions.cs (2)
3-3: LGTM! Namespace change aligns with DDD principles
The change from Core to Domain namespace better reflects the bounded context of the payment domain and follows DDD principles.
Line range hint 1-39: Consider further domain separation improvements
While the namespace change is a good start, consider these additional improvements to strengthen the domain separation:
- The
OrderCreatedPaymentConsumersuggests a potential domain boundary crossing. Consider implementing an anti-corruption layer (ACL) to translate between the Orders and Payment domains. - The
IDispatcherinterface might be better placed in an application layer if it's orchestrating domain operations.
Let's verify the consumer's dependencies:
src/Bmb.Payment.Masstransit/Dispatcher.cs (2)
1-2: LGTM: Clean architecture principles are well applied
The import changes and class structure align well with Domain-Driven Design principles by properly separating concerns and using domain core events.
Line range hint 1-39: Verify event handling patterns across bounded contexts
As part of the payment domain separation, ensure that:
- Integration events are used only for cross-bounded context communication
- Notification events remain within the payment bounded context
- Event schemas are properly versioned for backward compatibility
Let's verify the event usage patterns:
✅ Verification successful
Let me gather more information about the event implementations and their usage patterns.
Let me try one final verification with modified commands to check cross-boundary communication patterns.
Event handling patterns are correctly implemented across bounded contexts
Based on the verification results:
- Integration events are properly used for cross-boundary communication (OrderCreated event from Orders to Payment context)
- Notifications are correctly scoped within the Payment bounded context
- Event schemas follow a consistent pattern with proper interfaces (IBmbIntegrationEvent, IBmbNotification)
Key observations:
- The
OrderCreatedPaymentConsumercorrectly handles the integration event from the Orders context - The
Dispatcherimplementation properly separates notification and integration event publishing - The event handling infrastructure is well-structured with clear boundaries between domains
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for proper event type usage across bounded contexts
echo "Checking integration event usage..."
rg -t cs "IBmbIntegrationEvent" -C 3
echo "Checking notification event usage..."
rg -t cs "IBmbNotification" -C 3
# Verify event handlers in other bounded contexts
echo "Checking event handlers..."
ast-grep --pattern 'class $_ : IConsumer<$_>'
Length of output: 3980
Script:
#!/bin/bash
# Check for concrete implementations of events
echo "Checking concrete integration events..."
rg -t cs "class.*:.*IBmbIntegrationEvent" -C 2
echo "Checking concrete notification events..."
rg -t cs "class.*:.*IBmbNotification" -C 2
# Check for event consumers/handlers
echo "Checking event consumers..."
rg -t cs "IConsumer<.*>" -C 2
# Check for cross-boundary event references
echo "Checking cross-boundary event usage..."
fd -e cs -E "test" -E "Tests" | xargs rg "using.*Bmb\.((?!Payment).)*\.Events"
Length of output: 1298
Script:
#!/bin/bash
# Check for the OrderCreated event definition and usage
echo "Checking OrderCreated event..."
rg -t cs "OrderCreated" -C 3
# Check for any other event definitions in the codebase
echo "Checking event definitions..."
ast-grep --pattern 'class $_ : $event where $event'
# Check for cross-boundary imports
echo "Checking imports from other domains..."
rg -t cs "^using Bmb\." -C 0
Length of output: 20737
src/Bmb.Payment.Application/UseCases/UpdatePaymentStatusUseCase.cs (2)
1-4: LGTM: Clean separation of concerns in namespace organization
The namespace organization properly separates cross-cutting concerns (events) from domain-specific components, adhering to Clean Architecture principles and DDD bounded contexts.
Line range hint 8-18: LGTM: Well-structured class following SOLID principles
The class demonstrates:
- Interface segregation (ISP) through focused interface implementation
- Dependency inversion (DIP) via constructor injection
- Single responsibility (SRP) handling payment status updates
src/Bmb.Payment.DynamoDb/Repository/InMemoryPaymentRepository.cs (1)
1-4: Consider moving ValueObjects to the payment domain
The import of Domain.Core.ValueObjects might violate domain boundaries. Since we're separating the payment domain, consider moving relevant value objects to Bmb.Payment.Domain.ValueObjects to maintain domain autonomy.
Let's check what value objects are being used from the core domain:
src/Bmb.Payment.Application/UseCases/CreatePaymentUseCase.cs (1)
5-7: LGTM: Clean namespace organization
The namespace changes properly reflect the separation of payment domain from core, following clean architecture principles.
tests/Bmb.Payment.Application.Test/UseCases/UpdatePaymentStatusUseCaseTest.cs (2)
5-6: LGTM: Proper domain separation
The namespace changes correctly reflect the domain-driven design approach by moving from generic Core to a more focused Domain structure.
32-34: LGTM: Well-structured test with proper verifications
The test maintains good practices by:
- Verifying the repository call with correct domain entity type
- Using explicit verification of mock calls
- Properly scoping multiple assertions
src/Bmb.Payment.DynamoDb/Presenter.cs (1)
Line range hint 1-46: Verify Clean Architecture boundaries
The presenter correctly handles the infrastructure concerns of mapping between domain and persistence models. However, to strengthen the architecture:
- Consider moving this class to a dedicated "Infrastructure.Persistence" namespace
- Implement an interface in the domain layer that this presenter satisfies
- Use dependency injection to maintain proper dependency direction
#!/bin/bash
# Check current architecture boundaries
echo "Checking domain dependencies..."
rg --type csproj -l "Bmb.Payment.Domain" | while read -r file; do
echo "Project referencing domain: $file"
cat "$file" | grep -A 3 "<ProjectReference.*Bmb.Payment.Domain"
donesrc/Bmb.Payment.DI/ServiceCollectionsExtensions.cs (1)
6-6: LGTM! Namespace change aligns with DDD principles
The change from Core to Domain better reflects the bounded context and domain-centric approach of DDD.
src/Bmb.Payment.Api/Controllers/NotificationsController.cs (1)
3-3: LGTM: Proper domain separation
The namespace change correctly reflects the separation of the payment domain, following DDD principles by importing value objects from the domain layer.
src/Bmb.Payment.DynamoDb/Repository/PaymentRepository.cs (1)
4-6: Consider completing the domain separation
There's still a reference to Bmb.Domain.Core.ValueObjects while other namespaces have been migrated to the new domain structure. This might indicate an incomplete separation of concerns.
Consider migrating the remaining Core dependencies to maintain consistency with the domain-centric architecture.
tests/Bmb.Payment.Controllers.Test/PaymentServiceTests.cs (2)
5-5: LGTM: Clean namespace organization aligned with DDD principles
The updated imports properly reflect the domain separation, maintaining a clear boundary between the domain layer and other architectural components.
Also applies to: 10-11
62-62: Verify domain boundary isolation
The entity references have been properly updated to use the domain namespace, but let's verify that no core dependencies leak into the domain layer.
Also applies to: 83-83
✅ Verification successful
Let me gather more information about the domain boundary isolation by checking for any potential leaks in the opposite direction.
Domain boundary isolation is properly maintained
The verification confirms that:
- Domain project has no external project references
- Only domain-specific imports are used within domain entities
- Other layers correctly reference the domain layer without circular dependencies
- Entity references in tests properly use the domain namespace
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the domain layer remains pure with no dependencies on other layers
# Check for any potential layer violations in project references
find . -name "*.csproj" -exec grep -l "Bmb.Payment.Domain" {} \; | while read proj; do
echo "Analyzing dependencies in $proj"
grep -A 10 "<ProjectReference" "$proj" | grep "Bmb.Payment"
done
# Verify that Domain entities are not exposed through non-domain interfaces
ast-grep --pattern 'interface I$_Repository {
$$$
Task<Bmb.Payment.Domain.Entities.$_> $_($$$);
$$$
}'
Length of output: 2658
Script:
#!/bin/bash
# Check if domain project has any dependencies on other layers
grep -A 10 "<ProjectReference" ./src/Bmb.Payment.Domain/Bmb.Payment.Domain.csproj
# Check for any infrastructure or application types being used in domain entities
ast-grep --pattern 'namespace Bmb.Payment.Domain.Entities {
$$$
class $_ {
$$$
}
}'
# Look for any non-domain namespaces used within domain entities
rg "using Bmb\.Payment\." ./src/Bmb.Payment.Domain/ -A 1
Length of output: 436
tests/Bmb.Payment.Api.Test/Controllers/NotificationsControllerTest.cs (3)
6-6: LGTM: Clean domain separation
The updated import aligns with DDD principles by properly isolating domain concepts into their dedicated namespace.
Line range hint 18-135: LGTM: Well-structured test implementation
The test class demonstrates good testing practices:
- Clear Arrange-Act-Assert structure
- Comprehensive scenario coverage
- Strong assertions and verifications
- Clean dependency management
45-45: Verify consistent domain entity usage
The Payment entity references have been correctly updated to use the new domain namespace. Let's verify this change is consistent across the codebase.
Also applies to: 120-120
✅ Verification successful
The results show that there are no occurrences of the old namespace Domain.Core.Entities.Payment in the codebase, while the new namespace Domain.Entities.Payment is consistently used across all relevant files. Let me generate the final response:
Payment entity namespace change is consistently applied
The verification confirms that the namespace change for the Payment entity has been properly implemented across the entire codebase. All references now use Domain.Entities.Payment and no legacy references to Domain.Core.Entities.Payment remain.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all Payment entity references use the new domain namespace
# and no old references remain
# Check for any remaining references to the old namespace
rg -l "Domain\.Core\.Entities\.Payment"
# Check for consistent usage of the new namespace
rg -l "Domain\.Entities\.Payment"
Length of output: 1005
src/Bmb.Payment.MercadoPago.Gateway/MercadoPagoService.cs (1)
3-5: LGTM: Namespace changes align with DDD principles
The transition from Core to Domain namespace better reflects the DDD architectural style and provides clearer separation of concerns.
Also applies to: 11-12
| using Bmb.Domain.Core.ValueObjects; | ||
| using PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType; | ||
| using ValueObjects_PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify PaymentType imports to avoid confusion
The current import structure with multiple PaymentType references and aliases creates unnecessary complexity. Consider consolidating the PaymentType references to avoid confusion.
-using Bmb.Domain.Core.ValueObjects;
-using PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType;
-using ValueObjects_PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType;
+using Bmb.Payment.Domain.ValueObjects;Committable suggestion skipped: line range outside the PR's diff.
|
|
||
| public interface IPaymentGateway | ||
| { | ||
| Task<Entities.Payment> CreatePaymentAsync(OrderDto order); |
There was a problem hiding this comment.
Replace OrderDto with domain entity in gateway interface
Using OrderDto in the domain layer violates clean architecture principles. The domain layer should be independent of application concerns and should use domain entities or value objects instead of DTOs.
Consider:
- Creating a domain entity/aggregate for Order
- Moving DTO transformation to the application layer
-Task<Entities.Payment> CreatePaymentAsync(OrderDto order);
+Task<Entities.Payment> CreatePaymentAsync(Order order);Committable suggestion skipped: line range outside the PR's diff.
| using Bmb.Domain.Core.ValueObjects; | ||
| using PaymentStatus = Bmb.Payment.Domain.ValueObjects.PaymentStatus; | ||
| using ValueObjects_PaymentStatus = Bmb.Payment.Domain.ValueObjects.PaymentStatus; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify PaymentStatus references to improve clarity
The current setup has multiple PaymentStatus references which could lead to confusion:
- One from Domain.Core
- Two aliases from Payment.Domain
This might indicate a violation of DDD bounded contexts or unnecessary complexity.
Consider:
- Consolidating PaymentStatus to a single location
- If different PaymentStatus types are needed, use more meaningful aliases
-using Bmb.Domain.Core.ValueObjects;
-using PaymentStatus = Bmb.Payment.Domain.ValueObjects.PaymentStatus;
-using ValueObjects_PaymentStatus = Bmb.Payment.Domain.ValueObjects.PaymentStatus;
+using Bmb.Payment.Domain.ValueObjects;Committable suggestion skipped: line range outside the PR's diff.
| @@ -1,6 +1,6 @@ | |||
| using System.Diagnostics.CodeAnalysis; | |||
| using Bmb.Orders.Gateway.Repository; | |||
| using Bmb.Payment.Core; | |||
| using Bmb.Payment.Domain; | |||
There was a problem hiding this comment.
💡 Codebase verification
Refactor cross-domain dependency to respect bounded contexts
The verification reveals a significant coupling between Orders and Payment domains:
OrderDtois defined in Payment.Domain but represents an Order entityIOrdersGatewayinterface is also defined in Payment.Domain but implemented in Orders.Gateway- The DTO contains payment-specific fields (
PaymentId) mixed with order data
Recommendations:
- Move
OrderDtoandIOrdersGatewayto a shared kernel or contracts project - Split the DTO into
OrderDto(in Orders domain) andPaymentOrderDto(in Payment domain) - Consider using domain events for cross-boundary communication instead of direct references
🔗 Analysis chain
Review the domain dependency between Orders and Payment
While the namespace change from Core to Domain better reflects DDD principles, the direct reference from Orders.Gateway to Payment.Domain might indicate a coupling between bounded contexts. Consider if this dependency could be inverted or isolated through an anti-corruption layer or domain events.
Let's analyze the usage of Payment.Domain types in the Orders context:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check how Payment.Domain types are used in Orders.Gateway
# to determine if the dependency is necessary or could be decoupled
# Search for any usage of types from Payment.Domain
rg -l "Bmb\.Payment\.Domain\." src/Bmb.Orders.Gateway/
# Check for specific Payment.Domain type usages in the codebase
ast-grep --pattern 'using Bmb.Payment.Domain;'
Length of output: 1197
Script:
#!/bin/bash
# Let's check the actual usage of Payment.Domain types in Orders.Gateway files
# Check ServiceCollectionsExtensions.cs
echo "=== ServiceCollectionsExtensions.cs ==="
rg -A 5 "Bmb\.Payment\.Domain" src/Bmb.Orders.Gateway/ServiceCollectionsExtensions.cs
# Check Presenter.cs
echo "=== Presenter.cs ==="
rg -A 5 "Bmb\.Payment\.Domain" src/Bmb.Orders.Gateway/Presenter.cs
# Check OrderReplicaRepository.cs
echo "=== OrderReplicaRepository.cs ==="
rg -A 5 "Bmb\.Payment\.Domain" src/Bmb.Orders.Gateway/Repository/OrderReplicaRepository.cs
# Check InMemoryOrdersGateway.cs
echo "=== InMemoryOrdersGateway.cs ==="
rg -A 5 "Bmb\.Payment\.Domain" src/Bmb.Orders.Gateway/Repository/InMemoryOrdersGateway.cs
# Check project references
echo "=== Project References ==="
cat src/Bmb.Orders.Gateway/Bmb.Orders.Gateway.csproj
Length of output: 2022
Script:
#!/bin/bash
# Let's check what types from Payment.Domain are actually used in the Orders.Gateway
# Check for specific Payment.Domain type usages in OrderReplicaRepository
echo "=== OrderReplicaRepository Implementation ==="
rg -C 10 "class OrderReplicaRepository" src/Bmb.Orders.Gateway/Repository/OrderReplicaRepository.cs
# Check for specific Payment.Domain type usages in InMemoryOrdersGateway
echo "=== InMemoryOrdersGateway Implementation ==="
rg -C 10 "class InMemoryOrdersGateway" src/Bmb.Orders.Gateway/Repository/InMemoryOrdersGateway.cs
# Check for interface definition
echo "=== IOrdersGateway Interface ==="
ast-grep --pattern 'interface IOrdersGateway'
Length of output: 2621
Script:
#!/bin/bash
# Let's check the IOrdersGateway interface definition and OrderDto to understand the coupling
# Check IOrdersGateway interface
echo "=== IOrdersGateway Interface Definition ==="
rg -C 10 "interface IOrdersGateway" src/Bmb.Payment.Domain/IOrdersGateway.cs
# Check OrderDto definition
echo "=== OrderDto Definition ==="
rg -C 10 "class OrderDto" src/Bmb.Payment.Domain/
# Check if OrderDto is used elsewhere in the Payment domain
echo "=== OrderDto Usage in Payment Domain ==="
rg -l "OrderDto" src/Bmb.Payment.Domain/
Length of output: 2094
| using Bmb.Domain.Core.ValueObjects; | ||
| using PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType; | ||
| using ValueObjects_PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify import statements to avoid confusion
The current import statements create unnecessary complexity with aliases:
- Two imports for
PaymentTypewith different aliases - The alias
ValueObjects_PaymentTypedoesn't follow C# naming conventions
Consider this simplified approach:
using Bmb.Domain.Core.ValueObjects;
-using PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType;
-using ValueObjects_PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType;
+using DomainPaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| using Bmb.Domain.Core.ValueObjects; | |
| using PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType; | |
| using ValueObjects_PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType; | |
| using Bmb.Domain.Core.ValueObjects; | |
| using DomainPaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType; |
| [ExcludeFromCodeCoverage] | ||
| public class InMemoryPaymentRepository : IPaymentRepository | ||
| { | ||
| private readonly List<Domain.Core.Entities.Payment> _payments = new(); | ||
| private readonly List<Domain.Entities.Payment> _payments = new(); |
There was a problem hiding this comment.
💡 Codebase verification
Move InMemoryPaymentRepository to test project
The InMemoryPaymentRepository is misplaced in the main DynamoDb project. The codebase shows:
- A proper PaymentRepository implementation exists in the same directory
- The DynamoDb project is registered with PaymentRepository as the implementation for IPaymentRepository
- Test projects already use Moq for mocking IPaymentRepository
Action items:
- Move InMemoryPaymentRepository to Bmb.Payment.DynamoDb.Test project
- Remove [ExcludeFromCodeCoverage] attribute as it will be in test project
- Consider using Moq instead for consistency with other tests
🔗 Analysis chain
Clarify the purpose of excluding code coverage
The [ExcludeFromCodeCoverage] attribute on a repository implementation is concerning. If this is a test double, consider:
- Moving it to a test project
- Marking it as internal
- Adding XML documentation to clarify its testing purpose
Let's check if this is used in tests:
Let me gather more information to understand the repository structure and its usage better.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find references to InMemoryPaymentRepository
rg -l "InMemoryPaymentRepository"
# Check if there's a proper repository implementation
ast-grep --pattern 'class $_Repository : IPaymentRepository'
Length of output: 161
Script:
#!/bin/bash
# Check for test projects and test files
fd -e csproj
rg -l "Test" -g "*.csproj"
# Look for DynamoDb repository implementation
rg -l "DynamoDb.*Repository"
# Check service registration/DI setup
rg -A 5 "IPaymentRepository" -g "*.cs"
Length of output: 8802
| using Bmb.Domain.Core.Base; | ||
| using Bmb.Domain.Core.ValueObjects; | ||
| using Bmb.Payment.Domain.ValueObjects; | ||
| using PaymentStatus = Bmb.Payment.Domain.ValueObjects.PaymentStatus; | ||
| using PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType; | ||
| using ValueObjects_PaymentStatus = Bmb.Payment.Domain.ValueObjects.PaymentStatus; | ||
| using ValueObjects_PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify using directives by removing redundant aliases
The code currently uses both direct type references and aliases for the same types, which reduces readability. Consider using consistent type references throughout the code.
Apply this diff to clean up the using directives:
using Bmb.Domain.Core.Base;
using Bmb.Domain.Core.ValueObjects;
using Bmb.Payment.Domain.ValueObjects;
-using PaymentStatus = Bmb.Payment.Domain.ValueObjects.PaymentStatus;
-using PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType;
-using ValueObjects_PaymentStatus = Bmb.Payment.Domain.ValueObjects.PaymentStatus;
-using ValueObjects_PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType;Then update all references in the code to use the direct type names.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| using Bmb.Domain.Core.Base; | |
| using Bmb.Domain.Core.ValueObjects; | |
| using Bmb.Payment.Domain.ValueObjects; | |
| using PaymentStatus = Bmb.Payment.Domain.ValueObjects.PaymentStatus; | |
| using PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType; | |
| using ValueObjects_PaymentStatus = Bmb.Payment.Domain.ValueObjects.PaymentStatus; | |
| using ValueObjects_PaymentType = Bmb.Payment.Domain.ValueObjects.PaymentType; | |
| using Bmb.Domain.Core.Base; | |
| using Bmb.Domain.Core.ValueObjects; | |
| using Bmb.Payment.Domain.ValueObjects; | |
| public Payment() | ||
| { | ||
| Created = DateTime.UtcNow; | ||
| } | ||
|
|
||
| public Payment(PaymentId id, string qrCode, decimal amount, ValueObjects_PaymentType paymentType = ValueObjects_PaymentType.Test) | ||
| : base(id) | ||
| { | ||
| Id = id; | ||
| Status = ValueObjects_PaymentStatus.Pending; | ||
| QrCode = qrCode; | ||
| Amount = amount; | ||
| Created = DateTime.UtcNow; | ||
| PaymentType = paymentType; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve constructor implementation and validation
The constructors need improvement in terms of validation and avoiding redundant assignments.
Apply these improvements:
- public Payment()
+ protected Payment()
{
Created = DateTime.UtcNow;
}
public Payment(PaymentId id, string qrCode, decimal amount, PaymentType paymentType = PaymentType.Test)
: base(id)
{
- Id = id; // Redundant, already set by base(id)
+ if (string.IsNullOrWhiteSpace(qrCode))
+ throw new DomainException("QrCode is required");
+ if (amount <= 0)
+ throw new DomainException("Amount must be greater than zero");
+
Status = PaymentStatus.Pending;
QrCode = qrCode;
Amount = amount;
Created = DateTime.UtcNow;
PaymentType = paymentType;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public Payment() | |
| { | |
| Created = DateTime.UtcNow; | |
| } | |
| public Payment(PaymentId id, string qrCode, decimal amount, ValueObjects_PaymentType paymentType = ValueObjects_PaymentType.Test) | |
| : base(id) | |
| { | |
| Id = id; | |
| Status = ValueObjects_PaymentStatus.Pending; | |
| QrCode = qrCode; | |
| Amount = amount; | |
| Created = DateTime.UtcNow; | |
| PaymentType = paymentType; | |
| } | |
| protected Payment() | |
| { | |
| Created = DateTime.UtcNow; | |
| } | |
| public Payment(PaymentId id, string qrCode, decimal amount, ValueObjects_PaymentType paymentType = ValueObjects_PaymentType.Test) | |
| : base(id) | |
| { | |
| if (string.IsNullOrWhiteSpace(qrCode)) | |
| throw new DomainException("QrCode is required"); | |
| if (amount <= 0) | |
| throw new DomainException("Amount must be greater than zero"); | |
| Status = ValueObjects_PaymentStatus.Pending; | |
| QrCode = qrCode; | |
| Amount = amount; | |
| Created = DateTime.UtcNow; | |
| PaymentType = paymentType; | |
| } |
| public class Payment : Entity<PaymentId>, IAggregateRoot | ||
| { | ||
| public ValueObjects_PaymentType PaymentType { get; set; } = ValueObjects_PaymentType.Test; | ||
|
|
||
| public Guid OrderId { get; set; } | ||
|
|
||
| public string ExternalReference { get; set; } | ||
|
|
||
| public string QrCode { get; set; } | ||
|
|
||
| public decimal Amount { get; set; } | ||
|
|
||
| public ValueObjects_PaymentStatus Status { get; set; } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strengthen domain model encapsulation and validation
The current implementation with public setters violates DDD principles by allowing direct state mutation without business rules enforcement.
Consider these improvements:
- Make properties private set and provide methods for state changes
- Add validation for Amount (should be positive)
- Make OrderId non-nullable and validate
- Add validation for ExternalReference and QrCode
- Implement proper state transitions for Status
Example implementation:
public class Payment : Entity<PaymentId>, IAggregateRoot
{
- public ValueObjects_PaymentType PaymentType { get; set; }
+ public PaymentType PaymentType { get; private set; }
- public Guid OrderId { get; set; }
+ public Guid OrderId { get; private set; }
- public string ExternalReference { get; set; }
+ public string ExternalReference { get; private set; }
- public string QrCode { get; set; }
+ public string QrCode { get; private set; }
- public decimal Amount { get; set; }
+ public decimal Amount { get; private set; }
- public ValueObjects_PaymentStatus Status { get; set; }
+ public PaymentStatus Status { get; private set; }
+ public void SetOrderId(Guid orderId)
+ {
+ if (orderId == Guid.Empty)
+ throw new DomainException("OrderId cannot be empty");
+ OrderId = orderId;
+ }
+
+ public void SetAmount(decimal amount)
+ {
+ if (amount <= 0)
+ throw new DomainException("Amount must be greater than zero");
+ Amount = amount;
+ }Committable suggestion skipped: line range outside the PR's diff.
| using Amazon.DynamoDBv2.Model; | ||
| using Bmb.Domain.Core.ValueObjects; | ||
| using Bmb.Payment.Domain.ValueObjects; | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
The Core namespace import can be safely removed
Looking at the full content of Presenter.cs, I can see that no types from Bmb.Domain.Core.ValueObjects are actually being used in the code. The only value objects used are PaymentId, PaymentStatus, and PaymentType, which are all from the Payment domain (Bmb.Payment.Domain.ValueObjects). The Core namespace import is redundant and should be removed to maintain clean domain boundaries.
🔗 Analysis chain
Complete domain separation by moving all value objects to the Domain namespace
The presence of both Bmb.Domain.Core.ValueObjects and Bmb.Payment.Domain.ValueObjects suggests incomplete domain separation. Consider moving all payment-related value objects to the Domain namespace to maintain clean boundaries.
Let me try another verification approach to check the usage of Core namespace and understand the value objects better.
Let me check what specific value objects are being used in the Presenter.cs file to better understand if the Core namespace is actually needed.
Let me check what types are actually being used from the Core namespace.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for remaining Core namespace usage in the codebase
rg -l "Bmb.Domain.Core.ValueObjects" | while read -r file; do
echo "File still using Core namespace: $file"
rg -A 2 "Bmb.Domain.Core.ValueObjects" "$file"
done
Length of output: 159
Script:
#!/bin/bash
# Search for Core and Domain value objects usage
echo "Files using Core ValueObjects:"
rg -l "using.*Bmb\.Domain\.Core\.ValueObjects"
echo -e "\nFiles using Payment Domain ValueObjects:"
rg -l "using.*Bmb\.Payment\.Domain\.ValueObjects"
echo -e "\nValue object definitions and usages:"
ast-grep --pattern 'class $_ : ValueObject'
Length of output: 2872
Script:
#!/bin/bash
# Check specific value object usage in Presenter.cs
echo "Value object usage in Presenter.cs:"
rg -A 2 "Core\.ValueObjects|Domain\.ValueObjects" "src/Bmb.Payment.DynamoDb/Presenter.cs"
echo -e "\nFull content of Presenter.cs to understand context:"
cat "src/Bmb.Payment.DynamoDb/Presenter.cs"
Length of output: 2414
Script:
#!/bin/bash
# Search for actual usage of types from Core namespace
echo "Searching for Core value object usage in the code:"
rg -A 5 "new.*Core\.ValueObjects\.|: Core\.ValueObjects\." "src/Bmb.Payment.DynamoDb/Presenter.cs"
echo -e "\nChecking PaymentId definition:"
rg -l "class PaymentId"
Length of output: 312
|
|
🎉 This PR is included in version 1.4.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.