Conversation
- Update ExceptionFilter to handle DomainException with specific response. - Add IUnitOfWorkRepository for improved transaction management. - Modify ValidateAsync to return sender and receiver wallets. - Change BaseEntity properties to protected for better inheritance. - Implement UnitOfWork for transaction lifecycle management. - Enhance wallet repository with GetByIdAsync and UpdateAsync methods. - Improve tests for CreateTransactionUseCase to verify balance updates. - Update builders to ensure proper wallet and transaction setup in tests.
- Change several private methods to public in DependencyInjection.cs for external access. - Add AddHttp method to register an HTTP client factory. - Inject IHttpClientFactory into CreateTransactionUseCase for HTTP client usage. - Modify ValidateAuthorizeService to utilize the injected HTTP client. - Update Wallet.cs to enforce stricter balance checks on debit operations. - Add localized string for transaction authorization failure in ResourceMessagesException.
Introduce Kafka messaging capabilities to the UserTransactions application. Add `IKafkaMessagePublisher` interface, `KafkaMessagePublisher` class, and `KafkaProducerFactory` for producer creation. Update `DependencyInjection.cs` to configure Kafka options and register the message publisher. Modify `CreateTransactionUseCase` to publish messages on transaction creation. Update project file to include `Confluent.Kafka` package for integration.
There was a problem hiding this comment.
Pull Request Overview
This PR adds authorization validation and Kafka messaging functionality to the transaction creation process. The implementation includes a complete Kafka messaging infrastructure, database transaction management with Unit of Work pattern, and enhanced wallet operations.
- Implements external authorization service validation during transaction creation
- Adds Kafka messaging infrastructure for publishing transaction events
- Introduces Unit of Work pattern for database transaction management
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| CreateTransactionUseCase.cs | Enhanced with authorization validation, Kafka messaging, and transactional wallet operations |
| KafkaMessagePublisher.cs | New Kafka producer implementation for publishing messages |
| UnitOfWork.cs | Database transaction management implementation |
| WalletRepository.cs | Added GetByIdAsync and UpdateAsync methods for wallet operations |
| Wallet.cs | Enhanced Debit/Credit methods with balance updates and timestamp tracking |
Files not reviewed (1)
- src/Shared/UserTransactions.Exception/ResourceMessagesException.Designer.cs: Language not supported
|
|
||
| await _dbContext.SaveChangesAsync(); |
There was a problem hiding this comment.
Calling SaveChangesAsync() after each Update operation can cause performance issues. Consider removing SaveChangesAsync() here since the Unit of Work pattern should handle the final save operation.
| await _dbContext.SaveChangesAsync(); |
| { | ||
| using var httpClient = _httpClientFactory.CreateClient(); | ||
|
|
||
| var response = await httpClient.GetAsync("https://util.devi.tools/api/v2/authorize"); |
There was a problem hiding this comment.
The authorization service URL should be configurable rather than hardcoded. Consider moving this to configuration settings.
| var response = await httpClient.GetAsync("https://util.devi.tools/api/v2/authorize"); | |
| var response = await httpClient.GetAsync(_authorizationServiceUrl); |
| await _walletRepository.UpdateAsync(senderWallet); | ||
| await _walletRepository.UpdateAsync(receiverWallet); | ||
| await _transactionRepository.AddAsync(transaction); | ||
| await ValidateAuthorizeService(); |
There was a problem hiding this comment.
Authorization validation should occur before wallet operations and database modifications to avoid unnecessary processing if authorization fails.
| await ValidateAuthorizeService(); |
| Timestamp = new Timestamp(DateTime.UtcNow) | ||
| }); | ||
|
|
||
| _logger.LogInformation($"Message delivered to {result.TopicPartitionOffset}"); |
There was a problem hiding this comment.
Use structured logging with placeholders instead of string interpolation for better performance and log parsing: _logger.LogInformation("Message delivered to {TopicPartitionOffset}", result.TopicPartitionOffset);
| _logger.LogInformation($"Message delivered to {result.TopicPartitionOffset}"); | |
| _logger.LogInformation("Message delivered to {TopicPartitionOffset}", result.TopicPartitionOffset); |
| Timestamp = new Timestamp(DateTime.UtcNow) | ||
| }); | ||
|
|
||
| _logger.LogInformation($"Message delivered to {result.TopicPartitionOffset}"); |
There was a problem hiding this comment.
Use structured logging with placeholders instead of string interpolation for better performance and log parsing: _logger.LogInformation("Message delivered to {TopicPartitionOffset}", result.TopicPartitionOffset);
| _logger.LogInformation($"Message delivered to {result.TopicPartitionOffset}"); | |
| _logger.LogInformation("Message delivered to {TopicPartitionOffset}", result.TopicPartitionOffset); |
| await _unitOfWork.CommitAsync(); | ||
|
|
||
| var responseCreateTransactionDto = transaction.MapFromTransaction(); | ||
| await _messagePublisher.PublishAsync("transaction-created", new |
There was a problem hiding this comment.
The Kafka topic name should be configurable rather than hardcoded. Consider moving this to configuration settings.
| using var httpClient = _httpClientFactory.CreateClient(); | ||
|
|
||
| var response = await httpClient.GetAsync("https://util.devi.tools/api/v2/authorize"); |
There was a problem hiding this comment.
Consider using a named HttpClient with pre-configured base address and timeout settings for better performance and maintainability.
| using var httpClient = _httpClientFactory.CreateClient(); | |
| var response = await httpClient.GetAsync("https://util.devi.tools/api/v2/authorize"); | |
| using var httpClient = _httpClientFactory.CreateClient("AuthorizeServiceClient"); | |
| var response = await httpClient.GetAsync("authorize"); |
Rename `IKafkaMessagePublisher` to `IKafkaMessageProducer` and update its implementation. Introduce `TransactionCreatedEvent` for better event structure. Update `CreateTransactionUseCase` to publish the new event and add `KafkaTopics` for topic constants. Enhance testability with `HttpClientFactoryBuilder` and `KafkaMessageProducerBuilder`. Modify wallet repository methods for nullable handling and improve URL maintainability in `ValidateAuthorizeService`.
Modified the `SetupFailureResponse` method in `MockHttpMessageHandler.cs` to return a 403 Forbidden status instead of a 401 Unauthorized status. This change updates the simulated error response for testing purposes.
|
No description provided.