Skip to main content

Code Review — Task 01 (197183) — [Back] Substituir consultas concorrentes por consulta única na validação de marcas

Autor: IA (review automatizado)
Data: 2026-06-29
Escopo: [Back] Substituir consultas concorrentes por consulta única na validação de marcas
Modo: Branch QA/SP-2026-13 vs develop
Veredito: Aprovar com follow-ups menores (0 bugs, 4 suggestions, 4 nits)

Resumo​

A branch refatora ProductRepository.IsBrandsValid, substituindo o padrão N+1 com EF Core (N contextos paralelos com lock) por uma única consulta Dapper em lote via IDbConnectionFactory. Remove dependências não utilizadas (IConfiguration, IServiceProvider) do construtor e adiciona guards para schemaName nulo/vazio, products nulo e lista vazia, com deduplicação de IDs.

Os testes unitários foram ampliados de forma significativa: fakes Dapper compartilhados em DapperFakeDbInfrastructure.cs, suite dedicada ProductRepository_IsBrandsValidTests exercitando mapeamento real do Dapper (não mock de QueryAsync), e atualização dos construtores em ProductRepository_ResolveEcommercePriceTests. Spy classes duplicadas foram removidas de CartRepositoryQueryTests em favor do fake compartilhado.

Não foram encontrados bugs bloqueantes. O principal risco residual é a interpolação de schemaName no SQL (padrão pré-existente no repositório). A regra de negócio de multibrand e o binding Npgsql ANY(@ProductIds) parecem corretos e estão cobertos por testes.

Diff: 5 arquivos, +1085 / -531 linhas
Commit: 8d715d0 — fix(#197129): refatora validação de marcas e ajusta testes unitários

Comparação de cobertura de testes (develop vs QA/SP-2026-13)​

Comando executado em ambas as branches (sem fetch):

dotnet test CoezzionServiceCart.sln \
  --collect:"XPlat Code Coverage" \
  --settings coverlet.runsettings.xml

Filtro do coverlet.runsettings.xml: [Cart.API]*, [Cart.Domain]*, [Cart.Infrastructure]*.

Cobertura geral (solução)​

MétricadevelopQA/SP-2026-13Delta
Line (total)39,20%39,69%+0,49 pp
Branch (total)41,98%43,05%+1,07 pp
Method (total)32,35%32,62%+0,27 pp

Cobertura por módulo​

Módulodevelop (Line / Branch)QA/SP-2026-13 (Line / Branch)Delta LineDelta Branch
Cart.Infrastructure15,75% / 2,70%17,51% / 10,27%+1,76 pp+7,57 pp
Cart.Domain31,96% / 31,11%31,96% / 31,11%0,00 pp0,00 pp
Cart.API51,56% / 49,77%51,56% / 49,77%0,00 pp0,00 pp

Cobertura da classe alterada (ProductRepository)​

BranchLineBranch
develop76,47%0,00%
QA/SP-2026-13100,00%75,00%

Conclusão de cobertura: A branch melhora a cobertura onde importa para esta task. O ganho concentrado em Cart.Infrastructure (+1,76 pp line, +7,57 pp branch) reflete os novos testes de IsBrandsValid. ProductRepository passa de 76,47% para 100% de cobertura de linha e de 0% para 75% de branch — indicando que o refactor e a suite de testes cobrem o método refatorado e seus ramos de decisão. Cart.Domain e Cart.API permanecem inalterados, como esperado.

Achados​

Issue 1 — Severity: suggestion​

  • File: src/Cart.Infrastructure/Data/Repositories/ProductRepository.cs:97
  • Description: schemaName é interpolado diretamente no SQL (FROM {schemaName}."Products") com apenas guards de null/whitespace. Diferente de @ProductIds, o schema não é parametrizado. Valor malformado ou hostil pode alterar a estrutura da query (SQL injection). Callers passam schemas confiáveis e outros repositórios usam o mesmo padrão, mas IsBrandsValid recebe schemaName como parâmetro explícito.
  • Suggestion: Validar schemaName contra allowlist (ex.: ^[a-z][a-z0-9_]*$) antes de montar o SQL, ou usar helper compartilhado de quoting/escape. Adicionar teste que asserta rejeição de fragmentos maliciosos sem abrir conexão.
  • Status: open

Issue 2 — Severity: suggestion​

  • File: src/Cart.UnitTests/Infrastructure/Data/Repositories/ProductRepository_IsBrandsValidTests.cs:323
  • Description: ProductRepository_ResolveEcommercePriceTests inclui testes "Property 2" que assertam valores sensíveis como parâmetros e ausência de literais no CommandText. ProductRepository_IsBrandsValidTests verifica @ProductIds e valores bound, mas não asserta que IDs (ex.: 10, 20, 30) estão ausentes da string SQL.
  • Suggestion: Adicionar [Theory] espelhando ResolveEcommercePrice_QuandoExecutaQuery_SqlContemPlaceholdersENaoValoresLiterais, usando CapturingMultiRowFakeDbConnection e Assert.DoesNotContain(id.ToString(), sql) para cada ID.
  • Status: open

Issue 3 — Severity: suggestion​

  • File: src/Cart.UnitTests/Infrastructure/Data/Repositories/ProductRepository_IsBrandsValidTests.cs:29
  • Description: Cobertura de regra de negócio cobre AREZZO, BRIZZA, AREZZO+BRIZZA e mixes inválidos com SCHUTZ, mas omite edge cases: marca única não-AREZZO/BRIZZA (ex.: só SCHUTZ deve retornar true por brandsSet.Count == 1) e pares inválidos como BRIZZA+SCHUTZ sem AREZZO.
  • Suggestion: Adicionar [Fact] para SingleBrandSchutz_Executed_ReturnsTrue e BrizzaAndSchutz_Executed_ReturnsFalse.
  • Status: open

Issue 4 — Severity: suggestion​

  • File: src/Cart.UnitTests/Infrastructure/Data/Repositories/Fakes/DapperFakeDbInfrastructure.cs:520
  • Description: MultiRowFakeDbDataReader.GetValue indexa _rows[_currentRowIndex] quando _currentRowIndex ainda é -1 (antes de Read()), o que lançaria ArgumentOutOfRangeException. GetFieldType já guarda com _currentRowIndex >= 0 ? _currentRowIndex : 0.
  • Suggestion: Aplicar o mesmo guard em GetValue, IsDBNull e GetFieldValue<T>.
  • Status: open

Issue 5 — Severity: nit​

  • File: src/Cart.Infrastructure/Data/Repositories/ProductRepository.cs:141
  • Description: Expressão SetEquals quebrada em duas linhas dentro do return, prejudicando legibilidade e possivelmente falhando em format checks.
  • Suggestion: Manter em uma linha: SetEquals([(int)ActionBrands.AREZZO, (int)ActionBrands.BRIZZA]).
  • Status: open

Issue 6 — Severity: nit​

  • File: src/Cart.UnitTests/Infrastructure/Data/Repositories/Fakes/DapperFakeDbInfrastructure.cs:647
  • Description: Arquivo termina sem newline final.
  • Suggestion: Adicionar newline final.
  • Status: open

Issue 7 — Severity: nit​

  • File: src/Cart.UnitTests/Infrastructure/Data/Repositories/Fakes/DapperFakeDbInfrastructure.cs:293
  • Description: FakeDbParameterCollection e CapturingFakeDbParameterCollection são quase idênticas (~45 linhas duplicadas).
  • Suggestion: Extrair implementação base compartilhada ou delegação para reduzir manutenção.
  • Status: open

Issue 8 — Severity: nit​

  • File: src/Cart.UnitTests/Infrastructure/Data/Repositories/ProductRepository_IsBrandsValidTests.cs:12
  • Description: ProductRepository_ResolveEcommercePriceTests documenta escopo com XML summary e [Trait("Layer", "Unit")]. ProductRepository_IsBrandsValidTests não tem summary de classe e usa traits apenas por método.
  • Suggestion: Adicionar <summary> de classe e alinhar traits com classes irmãs.
  • Status: open

Contagem de issues​

SeveridadeQuantidade
bug0
suggestion4
nit4
Total8