Updates project to .NET 10#64
Conversation
Migrates the project to .NET 10. This involves updating the target framework in project files and the Dockerfile. Adds a central package management file to manage package versions. Also adds a CI workflow to run unit tests on pull requests. Removes unused references and updates test projects to use MSTest runner. Updates logging implementation with logger enabled check.
jldsilva
added
.NET
WalkthroughMigração do repositório para .NET 10: centralização de props e pacotes, atualização de CI/Docker, inicialização de coleções em entidades/viewmodels, introdução de AddIfNotExists com testes, e aplicação de guards de logging IsEnabled em vários repositórios e serviços. Changes
Estimated code review effort🎯 3 (Moderada) | ⏱️ ~20 minutos
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
InvoiceReminder.JobScheduler.UnitTests/JobSettings/CronJobTests.cs (1)
52-69: Remove redundant guard from test assertions and verify IsEnabled call behavior.The test mocks
IsEnabledto always returntrue(line 52), then wraps the assertion in a guardif (_logger.IsEnabled(LogLevel.Information))(lines 62-69). Since the mock always returnstrue, this guard is redundant and obscures what the test is actually verifying.Issues:
- Guard in assertions is redundant: With
IsEnabledmocked to returntrue, the condition always passes, making the guard meaningless.- Missing verification: The test never verifies that
IsEnabled(LogLevel.Information)was actually called by the production code.- Incomplete coverage: No test scenario for when
IsEnabledreturnsfalse, which the production code explicitly handles (line 28-31 in CronJob.cs).Recommended fixes:
- Remove the guard from the assertion block—assertions should be direct and unconditional
- Add verification that
_logger.Received(1).IsEnabled(LogLevel.Information)was called- Add a second test scenario where
IsEnabledreturnsfalseand verify logging does not occurInvoiceReminder.JobScheduler/HostedService/QuartzHostedService.cs (1)
31-49: Guard de log está correto, mas o segundo construtor pode levar aNullReferenceException
- O novo guard com
_logger.IsEnabled(LogLevel.Error)em volta doLogErrorem caso de expressão cron inválida está consistente com o restante do código e não altera o fluxo de controle (a iteração continua comcontinue).- Porém, o segundo construtor (
QuartzHostedService(IJobFactory jobFactory, ISchedulerFactory schedulerFactory)) não inicializa_loggernem_schedules. Se esse construtor for usado e a expressão cron for inválida, a chamada a_logger.IsEnabled(...)resultará em NRE (e_schedulestambém poderá sernullem qualquer iteração).Sugestões:
- Ou remova o segundo construtor se não for mais necessário,
- ou faça com que ele delegue para o construtor principal, fornecendo um
ILoggere uma coleção vazia deJobSchedule,- ou inicialize explicitamente
_loggercom um logger “neutro” e_schedulescom[].Isso evita falhas em tempo de execução difíceis de diagnosticar.
♻️ Duplicate comments (2)
InvoiceReminder.JobScheduler.UnitTests/JobSettings/CronJobTests.cs (1)
102-119: Mesmo problema de padrão de teste com guard de logging.Este teste apresenta o mesmo problema de design identificado nas linhas 52-69: guard redundante no teste que não verifica adequadamente o comportamento do código de produção.
InvoiceReminder.API/InvoiceReminder.API.csproj (1)
21-21: Verifique se a remoção de contentfiles não afeta as ferramentas de design do EF Core.A remoção de
contentfilesdosIncludeAssetsparaMicrosoft.EntityFrameworkCore.Designpode afetar ferramentas de design-time. Esta mesma alteração foi aplicada emInvoiceReminder.Data.csproje requer verificação em ambos os projetos.
🧹 Nitpick comments (12)
InvoiceReminder.Domain/Extensions/EntityExtensions.cs (1)
3-22: Sugerir interface ou constraint para garantir presença da propriedade Id.O método usa reflexão porque não há garantia em tempo de compilação de que o tipo
Tpossui uma propriedadeId. Isso resulta em código frágil e lento.Considere criar uma interface para entidades com Id:
// Adicionar em um arquivo de infraestrutura public interface IEntity { Guid Id { get; } } // Atualizar suas entidades public class User : EntityDefaults, IEntity { // ... propriedades existentes }Em seguida, refatore a extensão:
public static class EntityExtensions { - public static void AddIfNotExists<T>(this T entity, ICollection<T> collection) where T : class + public static void AddIfNotExists<T>(this T entity, ICollection<T> collection) where T : class, IEntity { if (entity is null) { return; } - if (collection.FirstOrDefault(e => GetId(e).Equals(GetId(entity))) is null) + if (!collection.Any(e => e.Id == entity.Id)) { collection.Add(entity); } } - - private static Guid GetId<T>(T entity) where T : class - { - return (Guid)typeof(T).GetProperty("Id")!.GetValue(entity)!; - } }Benefícios:
- Segurança em tempo de compilação
- Melhor desempenho (sem reflexão)
- IntelliSense e refatoração funcionam corretamente
InvoiceReminder.ExternalServices/BarcodeReader/BarcodeReaderService.cs (1)
21-32: Guard de log está ok, mas há pequenos pontos de melhoria
- O guard com
_logger.IsEnabled(LogLevel.Error)em torno doLogErrorestá consistente com o padrão adotado no PR, sem alterar o fluxo de exceção.- O template
"{Messagem}"tem um pequeno typo; se for usado em consultas/alertas, considere padronizar para algo como"{Message}"ou um nome em pt-BR coerente (ex.:"{ErrorMessage}") para facilitar análise posterior.- Fora do trecho alterado, note que se
invoiceTypereceber um valor não tratado noswitch,_barcodeHandlerficanulleCreateInvoicegeraráNullReferenceException. Pode valer a pena lançar uma exceção mais clara ou ter um handler padrão para evitar um NRE difícil de rastrear.InvoiceReminder.ExternalServices/Telegram/TelegramMessageService.cs (1)
34-44: Uso deIsEnabledem erros do Telegram está consistenteOs guards com
_logger.IsEnabled(LogLevel.Error)nos doiscatchmantêm o comportamento atual (apenas logar e não propagar a exceção), reduzindo overhead quando o nível Error estiver desabilitado.Como melhoria opcional, você poderia incluir mais contexto nos logs (por exemplo,
chatIdou um identificador de correlação), para facilitar troubleshooting de falhas específicas de envio.InvoiceReminder.Data/Repository/UserRepository.cs (1)
18-19: Padrão de logging padronizado e bem aplicado
- A constante
LogExceptionMessagecentraliza o template e ajuda a manter consistência entre os logs do repositório.- Os guards com
_logger.IsEnabled(LogLevel.Warning/Error)antes de logar em cenários de cancelamento e exceção genérica evitam trabalho desnecessário quando o nível correspondente estiver desabilitado.- As mensagens continuam ricas em contexto (
contextualInfo) e você mantém o padrão de reempacotar emOperationCanceledException/DataLayerException, preservando a pilha original (ex).Alteração bem alinhada ao restante do PR, sem impactos negativos aparentes.
Also applies to: 68-71, 80-83, 122-125, 134-137
InvoiceReminder.API/InvoiceReminder.http (1)
198-260: Ajustes opcionais nos exemplos de cron e rotasAlguns ajustes que podem deixar os exemplos ainda mais fiéis ao comportamento real:
- Os exemplos de
cronExpression("0 0 * * *","0 0 * * 1") usam 5 campos; se o agendador estiver esperando 6/7 campos (incluindo segundos), talvez valha alinhar o exemplo ao formato realmente aceito para evitar erro de validação quando alguém copiar/colar.- Em vários endpoints de deleção (
DELETE /api/user,/api/invoice,/api/job_schedule,/api/scan_email) oidvem no corpo em vez de na rota. Se a API real aceitar apenas rota (ou ambos), pode ser útil acrescentar um comentário informando isso para evitar confusão.Nada bloqueante, apenas refinamentos para tornar o arquivo 100% consistente com o comportamento da API.
InvoiceReminder.ExternalServices/Gmail/GoogleOAuthService.cs (1)
176-183: Considere aplicar logging guardado de forma consistente.O método
RefreshAuthTokenAsyncrelança a exceção sem logging, o que é adequado neste contexto. Porém, se futuramente for adicionado logging aqui, mantenha o mesmo padrão de guardaIsEnabledutilizado nos outros métodos.InvoiceReminder.ExternalServices.UnitTests/SendMessage/SendMessageServiceTests.cs (2)
122-130: Verificação condicional desnecessária no teste.O bloco
if (_logger.IsEnabled(LogLevel.Error))é redundante aqui. O mock já foi configurado para retornartruena linha 115, então esteifsempre será executado. Isso adiciona complexidade sem valor ao teste.Remova o
ife mantenha apenas a verificação:- if (_logger.IsEnabled(LogLevel.Error)) - { - var eventId = Arg.Any<EventId>(); - var state = Arg.Is<object>(o => o.ToString().Contains("User not found")); - var loggedException = Arg.Any<Exception>(); - var formatter = Arg.Any<Func<object, Exception, string>>(); - - _logger.Received(1).Log(LogLevel.Error, eventId, state, loggedException, formatter); - } + var eventId = Arg.Any<EventId>(); + var state = Arg.Is<object>(o => o.ToString().Contains("User not found")); + var loggedException = Arg.Any<Exception>(); + var formatter = Arg.Any<Func<object, Exception, string>>(); + + _logger.Received(1).Log(LogLevel.Error, eventId, state, loggedException, formatter);
158-166: Verificação condicional desnecessária no teste.Mesmo problema do teste anterior. O bloco
ifé redundante pois o mock já foi configurado para retornartrue.Remova o
ife mantenha apenas a verificação:- if (_logger.IsEnabled(LogLevel.Warning)) - { - var eventId = Arg.Any<EventId>(); - var state = Arg.Is<object>(o => o.ToString().Contains("No Authentication Token found")); - var loggedException = Arg.Is<Exception>(e => e == null); - var formatter = Arg.Any<Func<object, Exception, string>>(); - - _logger.Received(1).Log(LogLevel.Warning, eventId, state, loggedException, formatter); - } + var eventId = Arg.Any<EventId>(); + var state = Arg.Is<object>(o => o.ToString().Contains("No Authentication Token found")); + var loggedException = Arg.Is<Exception>(e => e == null); + var formatter = Arg.Any<Func<object, Exception, string>>(); + + _logger.Received(1).Log(LogLevel.Warning, eventId, state, loggedException, formatter);InvoiceReminder.Data/Repository/EmailAuthTokenRepository.cs (1)
16-16: Padronização de logging de exceções ficou boaA constante
LogExceptionMessagee o uso de_logger.IsEnabledcomLogWarning/LogErrordeixam o logging mais consistente e previsível, sem alterar o fluxo das exceções (cancelamento continua virandoOperationCanceledExceptione demais erros viramDataLayerException). Se quiser evoluir depois, daria para extrair esse padrão (template +contextualInfo) para um helper compartilhado entre os repositórios a fim de reduzir repetição, mas não é obrigatório agora.Also applies to: 43-46, 55-58
InvoiceReminder.Data/Repository/ScanEmailDefinitionRepository.cs (1)
16-16: Logging de exceções está consistente em todos os métodosO uso de
LogExceptionMessageTemplatecombinado comIsEnabledeLogWarning/LogErrorpadroniza bem o tratamento de falhas nas três operações (GetBySenderBeneficiaryAsync,GetBySenderEmailAddressAsynceGetByUserIdAsync), mantendo o mesmo contexto nas mensagens e sem mudar o comportamento das exceções. Como isso se repete em vários repositórios, pode valer no futuro extrair um utilitário comum (por exemplo, um helper estático ou método protegido emBaseRepository) para reduzir duplicação, mas não é algo bloqueante.Also applies to: 44-47, 56-59, 87-90, 99-102, 129-132, 141-144
InvoiceReminder.API/Dockerfile (1)
4-4: Atualização para .NET 10 e dependência nativa parecem corretasA troca das imagens para
aspnet:10.0/sdk:10.0, a instalação dalibgssapi-krb5-2com elevação temporária pararoote o uso deDirectory.Build.props/Directory.Packages.propsantes dorestoreestão todos consistentes com o objetivo de migrar para .NET 10 e centralizar configuração. Como melhoria opcional, você poderia adicionar--no-install-recommendsaoapt-get installpara reduzir o tamanho da imagem.Also applies to: 11-16, 19-19, 23-23
InvoiceReminder.Infrastructure.UnitTests/Data/Repository/UnitOfWorkTests.cs (1)
70-88: Considere simplificar as asserções de logging no teste.O padrão atual configura o mock para retornar
trueemIsEnabled(linha 70) e depois verifica essa condição novamente antes de fazer as asserções (linhas 80-88). Como o teste controla o mock, a verificação condicional é redundante—o bloco sempre será executado.Se não houver um padrão arquitetural estabelecido para isso, considere simplificar:
_ = context.Users.Add(new User { Id = Guid.NewGuid() }); _ = _logger.IsEnabled(Arg.Any<LogLevel>()).Returns(true); // Act var dataLayerException = await Should.ThrowAsync<DataLayerException>( async () => await unitOfWork.SaveChangesAsync(TestContext.CancellationToken) ); // Assert context.Database.GetDbConnection().State.ShouldBe(ConnectionState.Closed); - if (_logger.IsEnabled(LogLevel.Error)) - { - var eventId = Arg.Any<EventId>(); - var state = Arg.Any<object>(); - var exception = Arg.Any<Exception>(); - var formatter = Arg.Any<Func<object, Exception, string>>(); - - _logger.Received(1).Log(LogLevel.Error, eventId, state, exception, formatter); - } + var eventId = Arg.Any<EventId>(); + var state = Arg.Any<object>(); + var exception = Arg.Any<Exception>(); + var formatter = Arg.Any<Func<object, Exception, string>>(); + + _logger.Received(1).Log(LogLevel.Error, eventId, state, exception, formatter); _ = dataLayerException.ShouldNotBeNull();Nota: Se este padrão de verificação condicional faz parte de uma convenção estabelecida no projeto para manter consistência com o código de produção, desconsidere esta sugestão.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (52)
.github/workflows/workflow-ci.yml(1 hunks)Directory.Build.props(1 hunks)Directory.Packages.props(1 hunks)InvoiceReminder.API.UnitTests/AuthenticationSetup/BearerSecuritySchemeTransformerTests.cs(1 hunks)InvoiceReminder.API.UnitTests/Endpoints/LoginEndpointTests.cs(1 hunks)InvoiceReminder.API.UnitTests/InvoiceReminder.API.UnitTests.csproj(1 hunks)InvoiceReminder.API/AuthenticationSetup/BearerSecuritySchemeTransformer.cs(2 hunks)InvoiceReminder.API/Dockerfile(1 hunks)InvoiceReminder.API/InvoiceReminder.API.csproj(1 hunks)InvoiceReminder.API/InvoiceReminder.http(1 hunks)InvoiceReminder.Application.UnitTests/AppServices/BaseAppServiceTests.cs(1 hunks)InvoiceReminder.Application.UnitTests/InvoiceReminder.Application.UnitTests.csproj(1 hunks)InvoiceReminder.Application/InvoiceReminder.Application.csproj(0 hunks)InvoiceReminder.Application/ViewModels/UserViewModel.cs(1 hunks)InvoiceReminder.ArchitectureTests/InvoiceReminder.ArchitectureTests.csproj(1 hunks)InvoiceReminder.Authentication/InvoiceReminder.Authentication.csproj(0 hunks)InvoiceReminder.CrossCutting.IoC/InvoiceReminder.CrossCutting.IoC.csproj(0 hunks)InvoiceReminder.Data/InvoiceReminder.Data.csproj(1 hunks)InvoiceReminder.Data/Repository/BaseRepository.cs(1 hunks)InvoiceReminder.Data/Repository/EmailAuthTokenRepository.cs(3 hunks)InvoiceReminder.Data/Repository/InvoiceRepository.cs(3 hunks)InvoiceReminder.Data/Repository/JobScheduleRepository.cs(3 hunks)InvoiceReminder.Data/Repository/ScanEmailDefinitionRepository.cs(7 hunks)InvoiceReminder.Data/Repository/UnitOfWork.cs(1 hunks)InvoiceReminder.Data/Repository/UserRepository.cs(5 hunks)InvoiceReminder.Domain/Entities/User.cs(1 hunks)InvoiceReminder.Domain/Extensions/EntityExtensions.cs(1 hunks)InvoiceReminder.Domain/Extensions/UserExtensions.cs(1 hunks)InvoiceReminder.Domain/InvoiceReminder.Domain.csproj(0 hunks)InvoiceReminder.DomainEntities.UnitTests/Extensions/EntityExtensionsTests.cs(1 hunks)InvoiceReminder.DomainEntities.UnitTests/Extensions/UserExtensionsTests.cs(2 hunks)InvoiceReminder.DomainEntities.UnitTests/InvoiceReminder.DomainEntities.UnitTests.csproj(1 hunks)InvoiceReminder.ExternalServices.UnitTests/BarcodeReader/BarcodeReaderServiceTests.cs(1 hunks)InvoiceReminder.ExternalServices.UnitTests/InvoiceReminder.ExternalServices.UnitTests.csproj(1 hunks)InvoiceReminder.ExternalServices.UnitTests/SendMessage/SendMessageServiceTests.cs(3 hunks)InvoiceReminder.ExternalServices/BackgroundServices/TelegramBotBackgroundService.cs(1 hunks)InvoiceReminder.ExternalServices/BarcodeReader/BarcodeReaderService.cs(1 hunks)InvoiceReminder.ExternalServices/Gmail/GoogleOAuthService.cs(4 hunks)InvoiceReminder.ExternalServices/InvoiceReminder.ExternalServices.csproj(0 hunks)InvoiceReminder.ExternalServices/SendMessage/SendMessageService.cs(4 hunks)InvoiceReminder.ExternalServices/Telegram/TelegramMessageService.cs(1 hunks)InvoiceReminder.Infrastructure.UnitTests/Data/Repository/BaseRepositoryTests.cs(1 hunks)InvoiceReminder.Infrastructure.UnitTests/Data/Repository/UnitOfWorkTests.cs(3 hunks)InvoiceReminder.Infrastructure.UnitTests/InvoiceReminder.Infrastructure.UnitTests.csproj(1 hunks)InvoiceReminder.JobScheduler.UnitTests/HostedService/QuartzHostedServiceTests.cs(2 hunks)InvoiceReminder.JobScheduler.UnitTests/InvoiceReminder.JobScheduler.UnitTests.csproj(1 hunks)InvoiceReminder.JobScheduler.UnitTests/JobSettings/CronJobTests.cs(4 hunks)InvoiceReminder.JobScheduler/HostedService/QuartzHostedService.cs(1 hunks)InvoiceReminder.JobScheduler/InvoiceReminder.JobScheduler.csproj(0 hunks)InvoiceReminder.JobScheduler/JobSettings/CronJob.cs(1 hunks)InvoiceReminder.UnitTests.Assets/InvoiceReminder.UnitTests.Assets.csproj(0 hunks)InvoiceReminder.sln(2 hunks)
💤 Files with no reviewable changes (7)
- InvoiceReminder.JobScheduler/InvoiceReminder.JobScheduler.csproj
- InvoiceReminder.Domain/InvoiceReminder.Domain.csproj
- InvoiceReminder.Application/InvoiceReminder.Application.csproj
- InvoiceReminder.UnitTests.Assets/InvoiceReminder.UnitTests.Assets.csproj
- InvoiceReminder.ExternalServices/InvoiceReminder.ExternalServices.csproj
- InvoiceReminder.CrossCutting.IoC/InvoiceReminder.CrossCutting.IoC.csproj
- InvoiceReminder.Authentication/InvoiceReminder.Authentication.csproj
🧰 Additional context used
🧬 Code graph analysis (5)
InvoiceReminder.JobScheduler.UnitTests/HostedService/QuartzHostedServiceTests.cs (1)
InvoiceReminder.JobScheduler/HostedService/QuartzHostedService.cs (2)
IJobDetail(157-170)ITrigger(146-155)
InvoiceReminder.DomainEntities.UnitTests/Extensions/UserExtensionsTests.cs (2)
InvoiceReminder.Domain/Extensions/EntityExtensions.cs (1)
Guid(18-21)InvoiceReminder.Domain/Entities/User.cs (2)
User(3-21)User(14-20)
InvoiceReminder.Domain/Extensions/UserExtensions.cs (4)
InvoiceReminder.Domain/Extensions/EntityExtensions.cs (1)
AddIfNotExists(5-16)InvoiceReminder.Domain/Entities/JobSchedule.cs (1)
JobSchedule(3-7)InvoiceReminder.Domain/Entities/EmailAuthToken.cs (1)
EmailAuthToken(3-12)InvoiceReminder.Domain/Entities/ScanEmailDefinition.cs (1)
ScanEmailDefinition(5-13)
InvoiceReminder.DomainEntities.UnitTests/Extensions/EntityExtensionsTests.cs (1)
InvoiceReminder.Domain/Extensions/EntityExtensions.cs (2)
Guid(18-21)AddIfNotExists(5-16)
InvoiceReminder.ExternalServices/Gmail/GoogleOAuthService.cs (1)
InvoiceReminder.Domain/Services/Configuration/ConfigurationService.cs (3)
GetSecret(40-43)GetSecret(45-48)GetSecret(50-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (43)
InvoiceReminder.ExternalServices.UnitTests/BarcodeReader/BarcodeReaderServiceTests.cs (1)
14-14: Classe de teste sealed está okMarcar a classe de teste como
sealedaqui é seguro e não afeta o comportamento dos testes, já que ela não é usada como base para outras classes. Ajudará a evitar herança acidental no futuro.InvoiceReminder.ExternalServices/BackgroundServices/TelegramBotBackgroundService.cs (1)
98-116: Guard de logging de erro está consistente e preserva o comportamentoA adição de
_logger.IsEnabled(LogLevel.Error)em ambos os caminhos de erro mantém o fluxo original e apenas evita o custo do log quando o nível Error estiver desabilitado, alinhando com o padrão do PR. As mensagens e placeholders ({ErrorCode},{Message}) estão corretos e usam oApiRequestExceptionapropriadamente.InvoiceReminder.Application/ViewModels/UserViewModel.cs (1)
29-31: LGTM! Inicialização defensiva das coleções.A inicialização das coleções no construtor previne
NullReferenceExceptione está alinhada com as mudanças na entidade de domínioUser.InvoiceReminder.Domain/Entities/User.cs (1)
14-20: LGTM! Inicialização adequada das coleções de navegação.O construtor inicializa todas as propriedades de coleção para arrays vazios, prevenindo
NullReferenceExceptione garantindo que as coleções sempre estejam prontas para uso. Essa é uma prática recomendada para entidades com relacionamentos de navegação.InvoiceReminder.Domain/Extensions/UserExtensions.cs (1)
14-17: Refatoração positiva, mas dependente das correções em EntityExtensions.A substituição da lógica manual por chamadas
AddIfNotExiststorna o código mais limpo e reduz duplicação. No entanto, este código herda os problemas de desempenho e segurança identificados emEntityExtensions.AddIfNotExists.Certifique-se de aplicar as correções sugeridas no arquivo
InvoiceReminder.Domain/Extensions/EntityExtensions.csantes de fazer merge desta PR.Por favor, confirme que as correções críticas de segurança e as otimizações de desempenho em
EntityExtensionsserão aplicadas.Also applies to: 23-26
InvoiceReminder.DomainEntities.UnitTests/Extensions/EntityExtensionsTests.cs (1)
1-236: Excelente cobertura de testes para a nova extensão AddIfNotExists!A suíte de testes é abrangente e cobre adequadamente:
- ✓ Cenários de borda (entidade nula, GUID vazio)
- ✓ Prevenção de duplicatas baseada em Id
- ✓ Compatibilidade com diferentes tipos de coleção (List, HashSet)
- ✓ Preservação de entidades existentes
- ✓ Múltiplas chamadas com a mesma entidade
Os testes estão bem estruturados com padrão AAA (Arrange-Act-Assert) e usam asserções claras do Shouldly.
InvoiceReminder.DomainEntities.UnitTests/Extensions/UserExtensionsTests.cs (3)
8-8: Bom uso do modificadorsealedna classe de teste.Marcar a classe de teste como
sealedé uma boa prática que pode proporcionar pequenas melhorias de desempenho e deixa claro que a classe não foi projetada para ser herdada.
438-438: Renomeação apropriada do método de teste.O nome
Handle_NewUserWithEmptyCollections_InitializesCollectionsreflete mais precisamente o comportamento atual, já que o construtor deUseragora inicializa as coleções como vazias (não nulas).
445-445: Remoção correta de inicializações redundantes.A remoção das inicializações explícitas está alinhada com o fato de que o construtor de
Useragora inicializa todas as coleções automaticamente. O teste continua validando o comportamento correto.InvoiceReminder.JobScheduler.UnitTests/HostedService/QuartzHostedServiceTests.cs (1)
12-12: LGTM! Classe de teste selada adequadamente.Selar classes de teste é uma boa prática quando não há intenção de herança, melhorando a clareza do design.
InvoiceReminder.JobScheduler.UnitTests/JobSettings/CronJobTests.cs (1)
12-12: LGTM! Classe de teste selada adequadamente.Selar a classe de teste segue as boas práticas quando não há necessidade de herança.
InvoiceReminder.API/AuthenticationSetup/BearerSecuritySchemeTransformer.cs (1)
38-41: Atribuição deSecuritySchemescompatível com a nova APIO uso de
ToDictionarycom cast explícito paraIOpenApiSecuritySchemedeixa a atribuição aderente ao novo tipo da propriedade e mantém o comportamento anterior (apenas esquema Bearer definido). Não vejo riscos aqui.InvoiceReminder.Application.UnitTests/AppServices/BaseAppServiceTests.cs (1)
9-10: Selar a classe de teste é adequadoDeixar
BaseAppServiceTestscomosealedexplicita que não deve ser herdada e está alinhado com o padrão adotado nas demais classes de teste do PR. Nenhum impacto funcional.InvoiceReminder.Infrastructure.UnitTests/Data/Repository/BaseRepositoryTests.cs (1)
10-12: Classe de teste selada de forma consistenteTornar
BaseRepositoryTestssealedé coerente com o restante da suíte de testes e deixa claro que ela não deve ser estendida. Nenhum problema identificado.InvoiceReminder.API/InvoiceReminder.http (1)
3-361: Arquivo.httpbem completo e organizadoGostei bastante da expansão do
InvoiceReminder.http: os blocos separados por contexto (LOGIN, USER, INVOICE, JOB SCHEDULE, SCAN EMAIL, GOOGLE OAUTH) com exemplos de payloads e cabeçalhos facilitam muito o uso da API durante o desenvolvimento. A padronização deAuthorization: Bearer your_jwt_token_heree o uso de GUIDs de exemplo tornam o arquivo seguro e autoexplicativo.InvoiceReminder.DomainEntities.UnitTests/InvoiceReminder.DomainEntities.UnitTests.csproj (1)
8-10: Configuração do MSTest Runner está correta.A configuração do projeto de testes está alinhada com as práticas recomendadas do MSTest para .NET 10, utilizando o runner nativo ao invés do VSTest.
InvoiceReminder.ExternalServices.UnitTests/InvoiceReminder.ExternalServices.UnitTests.csproj (1)
8-10: Configuração consistente com os demais projetos de teste.A configuração do runner MSTest está alinhada com os outros projetos de teste da solução.
InvoiceReminder.ExternalServices/Gmail/GoogleOAuthService.cs (2)
24-24: Boa refatoração com a constanteAppKeysSection.A introdução da constante
AppKeysSectionmelhora a manutenibilidade e segue o princípio DRY, evitando strings duplicadas.
116-119: Logging guardado aplicado corretamente.A verificação
IsEnabled(LogLevel.Error)antes do log evita alocações desnecessárias quando o nível de log não está habilitado.InvoiceReminder.ExternalServices/SendMessage/SendMessageService.cs (3)
18-18: Boa padronização do formato de mensagem de log.A constante
LogExceptionMessagepadroniza o formato das mensagens de exceção, facilitando a análise de logs.
83-86: Logging guardado aplicado corretamente paraOperationCanceledException.O padrão de verificação
IsEnabledantes do log está implementado corretamente.
113-116: Logging guardado aplicado corretamente emValidateUser.InvoiceReminder.ExternalServices.UnitTests/SendMessage/SendMessageServiceTests.cs (1)
15-15: Boa prática: classe de teste selada.Marcar classes de teste como
sealedé uma boa prática que melhora a performance e deixa claro que a classe não deve ser herdada.Directory.Packages.props (2)
46-46: Nota sobreZ.EntityFramework.Extensions.EFCore.Este é um pacote comercial que substitui o
EFCore.BulkExtensions. Certifique-se de que a licença está em conformidade com os requisitos do projeto.
15-46: Migração de pacotes para .NET 10 bem estruturada.A organização dos pacotes em grupos rotulados ("System Packages", "Unit Tests Packages") e a centralização das versões facilita a manutenção. A migração para versões 10.0.0 está consistente.
InvoiceReminder.API.UnitTests/AuthenticationSetup/BearerSecuritySchemeTransformerTests.cs (1)
4-4: Alinhamento de namespace do OpenAPIA troca para
using Microsoft.OpenApi;está consistente com as mudanças na classe de produção e não altera o comportamento dos testes. Nada a ajustar aqui.InvoiceReminder.Data/Repository/BaseRepository.cs (1)
28-39: Uso do CancellationToken emBulkInsertAsyncA atualização para passar o
cancellationTokende forma posicional emBulkInsertAsyncmantém a intenção original de propagar cancelamento para a operação em lote. A lógica de timestamp continua intacta. 👍InvoiceReminder.Data/Repository/InvoiceRepository.cs (1)
16-17: Padronização e guarda dos logs de exceçãoBoa melhoria ao introduzir
LogExceptionMessagee envolver osLogWarning/LogErrorem checagens deIsEnabled. Isso padroniza o formato de mensagem e evita trabalho de logging desnecessário quando o nível correspondente estiver desativado, mantendo o fluxo de exceção inalterado.Also applies to: 40-55
InvoiceReminder.sln (1)
3-4: Atualização de versão da Solution e inclusão doDirectory.Build.propsA atualização do cabeçalho para Visual Studio 18 e a adição de
Directory.Build.propsem Solution Items são mudanças administrativas que não impactam o build e facilitam a descoberta da configuração global. Tudo certo aqui.Also applies to: 65-65
InvoiceReminder.Data/Repository/UnitOfWork.cs (1)
39-42: Guarda de log de erro noSaveChangesAsyncO uso de
_logger.IsEnabled(LogLevel.Error)antes doLogErrorestá alinhado com o padrão aplicado nos demais repositórios e não altera o fluxo de transação/rollback. Implementação está ok.InvoiceReminder.JobScheduler/JobSettings/CronJob.cs (1)
28-31: Guard deLogInformationno job de cronA checagem de
_logger.IsEnabled(LogLevel.Information)antes de logar a mensagem deixa o job consistente com o restante da solução e evita logs quando o nível Info estiver desligado. Nada a ajustar aqui.InvoiceReminder.API.UnitTests/Endpoints/LoginEndpointTests.cs (1)
21-21: Classe de teste seladaMarcar
LoginEndpointTestscomosealedé compatível com MSTest e evita herança acidental em testes. Mudança simples e bem-vinda.Directory.Build.props (1)
1-10: Centralização de propriedades de build é corretaA centralização de
TargetFramework,LangVersion,ImplicitUsingseTieredPGOemDirectory.Build.propsé uma abordagem adequada e simplifica a manutenção.Quanto aos pontos levantados na revisão anterior:
<Nullable>disable</Nullable>: Nenhum.csprojindividual possui override para esta propriedade, indicando que a configuração global é intencional e consistente com o estado atual do código.
<UserSecretsId>na raiz: Não existem configurações deUserSecretsIdem projetos individuais. Em uma arquitetura típica, apenas projetos web (comoInvoiceReminder.API) utilizariam secrets, enquanto projetos de biblioteca (Domain, Data, Infrastructure, etc.) herdando esta propriedade não a utilizam. Logo, não há risco de mistura de segredos entre projetos.A configuração está funcional e sem conflitos.
Likely an incorrect or invalid review comment.
InvoiceReminder.JobScheduler.UnitTests/InvoiceReminder.JobScheduler.UnitTests.csproj (1)
8-10: Configuração do runner de testes está coerenteAs propriedades
EnableMSTestRunner,OutputTypeeUseVSTestestão alinhadas com a estratégia de executar os testes como executáveis e centralizar a configuração emDirectory.Build.props. Nada a ajustar aqui, só garantir que o pipeline/IDE está realmente usando esse modelo em todos os projetos de teste.InvoiceReminder.Application.UnitTests/InvoiceReminder.Application.UnitTests.csproj (1)
8-10: Consistência de configuração entre projetos de testeEste projeto segue o mesmo padrão de runner de teste (EnableMSTestRunner/OutputType/UseVSTest) dos demais, o que simplifica bastante manutenção e execução dos testes na solução. Nenhum ajuste necessário aqui.
InvoiceReminder.API.UnitTests/InvoiceReminder.API.UnitTests.csproj (1)
8-10: Projeto de testes de API alinhado com o novo modelo de execuçãoAs propriedades do runner (
EnableMSTestRunner,OutputType,UseVSTest) estão em linha com os demais projetos de teste, o que facilita a execução unificada no CI e localmente. Tudo ok aqui.InvoiceReminder.Data/Repository/JobScheduleRepository.cs (1)
16-16: Tratamento de erros de JobSchedule agora segue o padrão comumO repositório de
JobSchedulepassou a usar o mesmo template de log e os checks deIsEnabledpara warnings/erros que os demais repositórios, mantendo o contexto na mensagem e preservando o comportamento de exceção (OperationCanceledExceptionem cancelamento eDataLayerExceptionnos demais erros). A padronização ajuda bastante na observabilidade.Also applies to: 40-43, 52-55
InvoiceReminder.Infrastructure.UnitTests/InvoiceReminder.Infrastructure.UnitTests.csproj (1)
8-10: Projeto de testes de infraestrutura configurado de forma consistenteA adoção do mesmo conjunto de propriedades de execução de testes que o resto da solução evita surpresas (especialmente com EF Core e dependências de infraestrutura). Configuração parece correta e homogênea.
InvoiceReminder.ArchitectureTests/InvoiceReminder.ArchitectureTests.csproj (1)
8-10: LGTM! Configuração moderna do MSTest runner.A configuração do novo MSTest runner está alinhada com o MSTest.Sdk 4.0.2 e é consistente com a execução de testes baseada em executáveis implementada no workflow de CI.
InvoiceReminder.Data/InvoiceReminder.Data.csproj (2)
14-14: Verifique se a remoção de contentfiles não afeta as ferramentas de design do EF Core.A remoção de
contentfilesdosIncludeAssetsdo pacoteMicrosoft.EntityFrameworkCore.Designpode afetar templates de scaffolding e outras funcionalidades de design-time. Confirme que comandos comodotnet ef migrations addedotnet ef dbcontext scaffoldainda funcionam corretamente.
9-9: No critical issues found with the library migration to Z.EntityFramework.Extensions.EFCore.The replacement from EFCore.BulkExtensions to Z.EntityFramework.Extensions.EFCore is already in place and working correctly. The
BulkInsertAsyncextension method is being used properly throughout the codebase (BaseRepository, SendMessageService, and tests), and the lack of explicitusingstatements is expected since Z.EntityFramework.Extensions adds extension methods to DbContext that are automatically available when the NuGet package is referenced.Note: Z.EntityFramework.Extensions.EFCore is a commercial library. Ensure your organization has the appropriate licensing.
.github/workflows/workflow-ci.yml (1)
23-29: LGTM! Atualização para .NET 10 e configuração de build apropriada.A atualização para .NET 10.0.x e o uso explícito de
--configuration Debugestão corretos para o pipeline de CI.InvoiceReminder.Infrastructure.UnitTests/Data/Repository/UnitOfWorkTests.cs (1)
15-15: LGTM! Uso correto de sealed em classe de teste.Marcar a classe de teste como
sealedé uma boa prática, pois classes de teste geralmente não precisam de herança.
Simplifies unit tests by removing redundant checks for logger enablement. This streamlines the tests and makes them more readable without affecting their functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
InvoiceReminder.Domain/Extensions/EntityExtensions.cs (1)
19-22: Otimização de performance não aplicada.O comentário anterior sugeriu cachear
GetId(entity)fora do lambda para evitar recalcular para cada elemento. Essa otimização ainda não foi aplicada.+var entityId = EntityIdComparer<T>.GetId(entity); -if (collection.FirstOrDefault(e => EntityIdComparer<T>.GetId(e).Equals(EntityIdComparer<T>.GetId(entity))) is null) +if (!collection.Any(e => EntityIdComparer<T>.GetId(e).Equals(entityId))) { collection.Add(entity); }
🧹 Nitpick comments (4)
InvoiceReminder.ExternalServices.UnitTests/SendMessage/SendMessageServiceTests.cs (1)
115-127: Aprovo a refatoração da verificação de logs, mas considere verificar a exceção específica.A adição do mock
IsEnablede a refatoração da verificação de logs para usar argumentos explícitos torna o teste mais claro. No entanto, na linha 124,Arg.Any<Exception>()aceita qualquer exceção ao invés de verificar que a exceção logada é a mesma que foi lançada.Considere aplicar este diff para verificar a exceção específica:
- var loggedException = Arg.Any<Exception>(); + var loggedException = Arg.Is<Exception>(e => e == exception);InvoiceReminder.JobScheduler.UnitTests/JobSettings/CronJobTests.cs (1)
108-113: Considere simplificar a verificação de exceção nula.A verificação
Arg.Is<Exception>(e => e == null)na linha 110 funciona corretamente, mas pode ser simplificada passando diretamentenullou usandoArg.Is<Exception>(e => e is null)para maior clareza.Aplique esta alteração para simplificar:
- var loggedException = Arg.Is<Exception>(e => e == null); + var loggedException = null as Exception;Ou alternativamente:
- var loggedException = Arg.Is<Exception>(e => e == null); + var loggedException = Arg.Is<Exception>(e => e is null);InvoiceReminder.Domain/Extensions/EntityExtensions.cs (1)
26-36: Implementação deIEqualityComparer<T>não utilizada.A classe
EntityIdComparer<T>implementaIEqualityComparer<T>, mas apenas o método estáticoGetIdé utilizado. Os métodosEqualseGetHashCodeda interface nunca são chamados.Se a intenção era otimizar o HashSet com comparação por Id, o comparador deveria ser passado na construção do HashSet. Caso contrário, considere simplificar a classe.
Opção 1 - Simplificar removendo a interface não utilizada:
-internal sealed class EntityIdComparer<T> : IEqualityComparer<T> where T : class +internal static class EntityIdHelper<T> where T : class { - public bool Equals(T x, T y) - { - return !(x == null || y == null) && GetId(x).Equals(GetId(y)); - } - - public int GetHashCode(T obj) - { - return obj == null ? 0 : GetId(obj).GetHashCode(); - } - public static Guid GetId(T entity) { // ... implementation unchanged } }Opção 2 - Usar o comparador no HashSet (requer mudança na chamada):
var hashSetWithComparer = new HashSet<T>(new EntityIdComparer<T>());InvoiceReminder.Infrastructure.UnitTests/Data/Repository/UnitOfWorkTests.cs (1)
69-90: Use inlineArg.Any<T>()in log verification and consider isolating mock state between testsThe test correctly verifies that an error log occurs on exception, but the verification pattern can be improved:
Use inline
Arg.Any()matchers (best practice)
NSubstitute documentation recommends using argument matchers directly at the call site rather than in local variables. Replace lines 84–89 with:
var eventId = Arg.Any<EventId>();var state = Arg.Any<object>();var exception = Arg.Any<Exception>();var formatter = Arg.Any<Func<object, Exception, string>>();_logger.Received(1).Log(LogLevel.Error, eventId, state, exception, formatter);
_logger.Received(1).Log(LogLevel.Error,Arg.Any<EventId>(),Arg.Any<object>(),Arg.Any<Exception>(),Arg.Any<Func<object, Exception, string>>());
- Prevent mock state fragility (optional)
The_loggeris created in the constructor and shared across all tests. Currently only one test verifies logging, but future tests that log errors will causeReceived(1)to fail. Either recreate the mock in[TestInitialize]or call_logger.ClearReceivedCalls();at the start of each logging-verification test to ensure count accuracy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
InvoiceReminder.Domain/Extensions/EntityExtensions.cs(1 hunks)InvoiceReminder.ExternalServices.UnitTests/SendMessage/SendMessageServiceTests.cs(3 hunks)InvoiceReminder.Infrastructure.UnitTests/Data/Repository/UnitOfWorkTests.cs(3 hunks)InvoiceReminder.JobScheduler.UnitTests/HostedService/QuartzHostedServiceTests.cs(2 hunks)InvoiceReminder.JobScheduler.UnitTests/JobSettings/CronJobTests.cs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- InvoiceReminder.JobScheduler.UnitTests/HostedService/QuartzHostedServiceTests.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (7)
InvoiceReminder.ExternalServices.UnitTests/SendMessage/SendMessageServiceTests.cs (2)
15-15: LGTM! Boa prática tornar a classe de testes sealed.Tornar a classe de testes
sealedé uma boa prática que previne herança desnecessária e deixa clara a intenção de que esta é uma classe final.
147-160: LGTM! Refatoração melhora a clareza da verificação de logs.A adição do mock
IsEnablede a refatoração da verificação de logs usando argumentos explícitos torna o teste mais claro e legível. A verificação de que a exceção énull(linha 157) está correta para o cenário de warning.InvoiceReminder.JobScheduler.UnitTests/JobSettings/CronJobTests.cs (3)
12-12: Boa prática: classe de teste selada.Marcar a classe de teste como
sealedé uma boa prática que previne herança acidental e pode proporcionar pequenos ganhos de performance.
52-52: Configuração correta para logging protegido.A configuração do mock
_logger.IsEnabled(...)retornandotrueé necessária para permitir que o código de logging protegido execute durante os testes, possibilitando as verificações de log subsequentes.Also applies to: 99-99
62-66: Verifique a consistência das asserções de logging.Este teste usa
ReceivedWithAnyArgspara verificar o logging sem validar o conteúdo da mensagem, enquanto o testeExecute_SendMessageFails_ShouldNotThrowExceptionAndStillLog(linhas 108-113) usaReceived(1)com validação específica do conteúdo. Se a intenção é apenas verificar que o log foi chamado sem importar a mensagem, a abordagem está correta. Caso contrário, considere alinhar ambas as asserções para maior consistência.InvoiceReminder.Domain/Extensions/EntityExtensions.cs (1)
38-49: Implementação segura do GetId.A validação adequada foi adicionada para tratar casos onde a propriedade
Idnão existe, é nula, ou não é do tipoGuid. Isso resolve a preocupação de segurança levantada anteriormente.InvoiceReminder.Infrastructure.UnitTests/Data/Repository/UnitOfWorkTests.cs (1)
15-15: Classe de testesealedé uma boa escolha aquiMarcar a classe de teste como
sealedé coerente com o padrão do PR, evita herança acidental e não traz impacto negativo para os testes.
Ensures that duplicate entities are not added to collections by checking for existing entities with the same ID. This prevents potential data integrity issues and improves the reliability of the application when dealing with collections of entities.
1 participant
Migrates the project to .NET 10. This involves updating the target framework in project files and the Dockerfile.
Adds a central package management file to manage package versions.
Also adds a CI workflow to run unit tests on pull requests.
Removes unused references and updates test projects to use MSTest runner.
Updates logging implementation with logger enabled check.
Summary by CodeRabbit
Notas de Versão
New Features
Refactor
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.