From 2feaecadc9edd66bf4cbea3dff4f5101f67ba6bf Mon Sep 17 00:00:00 2001 From: Daniel Gee Date: Fri, 22 May 2026 21:11:32 +0100 Subject: [PATCH 1/4] feat: add lookup-before-insert for Ingredient canonical rows (#978) Before inserting a new Ingredient, query for an existing row with the same name. If found, return the existing row instead of inserting a duplicate (client-transparent get-or-create). The lookup uses a simple EF Where clause so SQL Server's default CI collation handles case-insensitive matching transparently. OrderBy(Id) ensures the oldest canonical row is returned if legacy duplicates exist. Unit tests cover new-row creation and same-name reuse. Integration tests cover same-name reuse and case-insensitive reuse (the latter exercising SQL Server CI collation end-to-end). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../IngredientIntegrationTests.cs | 32 +++++++ .../Repositories/IngredientRepositoryTests.cs | 88 +++++++++++++++++++ .../Repositories/IngredientRepository.cs | 30 +++++++ 3 files changed, 150 insertions(+) create mode 100644 backend/MenuApi.Tests/Repositories/IngredientRepositoryTests.cs diff --git a/backend/MenuApi.Integration.Tests/IngredientIntegrationTests.cs b/backend/MenuApi.Integration.Tests/IngredientIntegrationTests.cs index 529b274a..b1bd7239 100644 --- a/backend/MenuApi.Integration.Tests/IngredientIntegrationTests.cs +++ b/backend/MenuApi.Integration.Tests/IngredientIntegrationTests.cs @@ -100,6 +100,38 @@ public async Task Create_Ingredient_With_Duplicate_Units_Returns_Unique_Units([S units.Should().ContainSingle(u => u.Name == "Grams"); } + [Theory, AutoData] + public async Task Create_Ingredient_With_Same_Name_Twice_Returns_Same_Id([StringLength(50, MinimumLength = 1)] string ingredientName) + { + using var client = await fixture.GetHttpClient(); + + var (firstId, _, _) = await PostIngredientAsync(client, ingredientName, [4]); + var (secondId, _, _) = await PostIngredientAsync(client, ingredientName, [1]); + + secondId.Should().Be(firstId); + + using var listResponse = await client.GetAsync("/api/ingredient"); + await listResponse.ShouldHaveStatusCode(HttpStatusCode.OK); + + var data = await listResponse.Content.ReadAsStringAsync(); + var ingredients = JsonSerializer.Deserialize>(data, jsonOptions); + ingredients.Should().NotBeNull(); + ingredients!.Count(i => i.Name == ingredientName).Should().Be(1); + } + + [Theory, AutoData] + public async Task Create_Ingredient_With_Different_Case_Returns_Same_Id([StringLength(50, MinimumLength = 1)] string ingredientName) + { + using var client = await fixture.GetHttpClient(); + var upperName = ingredientName.ToUpperInvariant(); + var lowerName = ingredientName.ToLowerInvariant(); + + var (firstId, _, _) = await PostIngredientAsync(client, upperName, [4]); + var (secondId, _, _) = await PostIngredientAsync(client, lowerName, [1]); + + secondId.Should().Be(firstId); + } + internal async Task<(int Id, string Name, List Units)> PostIngredientAsync( HttpClient client, string name, List unitIds) { diff --git a/backend/MenuApi.Tests/Repositories/IngredientRepositoryTests.cs b/backend/MenuApi.Tests/Repositories/IngredientRepositoryTests.cs new file mode 100644 index 00000000..7a530e26 --- /dev/null +++ b/backend/MenuApi.Tests/Repositories/IngredientRepositoryTests.cs @@ -0,0 +1,88 @@ +using AwesomeAssertions; +using MenuDB; +using MenuDB.Data; +using MenuApi.Repositories; +using MenuApi.ValueObjects; +using MenuApi.ViewModel; +using Microsoft.EntityFrameworkCore; +using Xunit; + +namespace MenuApi.Tests.Repositories; + +public class IngredientRepositoryTests +{ + [Fact] + public async Task CreateIngredientAsync_Creates_New_Ingredient_When_Name_Does_Not_Exist() + { + var cancellationToken = TestContext.Current.CancellationToken; + await using var db = CreateDbContext(); + await SeedUnitsAsync(db, cancellationToken); + var sut = new IngredientRepository(db); + + var result = await sut.CreateIngredientAsync(new NewIngredient + { + Name = IngredientName.From("Sugar"), + UnitIds = [4], + }); + + result.Id.Value.Should().BeGreaterThan(0); + result.Name.Should().Be(IngredientName.From("Sugar")); + result.Units.Should().ContainSingle(u => u.Name == IngredientUnitName.From("Grams")); + + var count = await db.Ingredients.CountAsync(cancellationToken); + count.Should().Be(1); + } + + [Fact] + public async Task CreateIngredientAsync_Returns_Existing_Ingredient_When_Name_Already_Exists() + { + var cancellationToken = TestContext.Current.CancellationToken; + await using var db = CreateDbContext(); + await SeedUnitsAsync(db, cancellationToken); + + var unitType = new UnitTypeEntity { Id = 99, Name = "Other" }; + var otherUnit = new UnitEntity { Id = 99, Name = "Cup", UnitTypeId = 99, UnitType = unitType }; + db.UnitTypes.Add(unitType); + db.Units.Add(otherUnit); + await db.SaveChangesAsync(cancellationToken); + + var sut = new IngredientRepository(db); + var first = await sut.CreateIngredientAsync(new NewIngredient + { + Name = IngredientName.From("Sugar"), + UnitIds = [4], + }); + + var second = await sut.CreateIngredientAsync(new NewIngredient + { + Name = IngredientName.From("Sugar"), + UnitIds = [99], + }); + + second.Id.Should().Be(first.Id); + second.Units.Should().ContainSingle(u => u.Name == IngredientUnitName.From("Grams")); + + var count = await db.Ingredients.CountAsync(i => i.Name == "Sugar", cancellationToken); + count.Should().Be(1); + } + + private static MenuDbContext CreateDbContext() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + + return new MenuDbContext(options); + } + + private static async Task SeedUnitsAsync(MenuDbContext db, CancellationToken cancellationToken) + { + var unitType = new UnitTypeEntity { Id = 3, Name = "Weight" }; + var unit = new UnitEntity { Id = 4, Name = "Grams", UnitTypeId = 3, UnitType = unitType }; + + db.UnitTypes.Add(unitType); + db.Units.Add(unit); + + await db.SaveChangesAsync(cancellationToken); + } +} diff --git a/backend/MenuApi/Repositories/IngredientRepository.cs b/backend/MenuApi/Repositories/IngredientRepository.cs index 977d0ec7..759b8c80 100644 --- a/backend/MenuApi/Repositories/IngredientRepository.cs +++ b/backend/MenuApi/Repositories/IngredientRepository.cs @@ -39,6 +39,36 @@ public class IngredientRepository(MenuDbContext db) : IIngredientRepository { ArgumentNullException.ThrowIfNull(newIngredient); + var existing = await db.Ingredients + .Where(i => i.Name == newIngredient.Name.Value) + .OrderBy(i => i.Id) + .Select(i => new + { + i.Id, + i.Name, + Units = i.IngredientUnits.Select(iu => new + { + iu.Unit.Name, + iu.Unit.Abbreviation, + UnitType = iu.Unit.UnitType.Name, + }) + }) + .FirstOrDefaultAsync() + .ConfigureAwait(false); + + if (existing is not null) + { + return new ViewModel.Ingredient + { + Id = IngredientId.From(existing.Id), + Name = IngredientName.From(existing.Name), + Units = existing.Units.Select(u => new ViewModel.IngredientUnit( + IngredientUnitName.From(u.Name), + u.Abbreviation is not null ? IngredientUnitAbbreviation.From(u.Abbreviation) : null, + IngredientUnitType.From(u.UnitType))), + }; + } + var unitIds = newIngredient.UnitIds.Distinct().ToList(); var entity = new IngredientEntity From c4b7169a53f4c23b659b2a0bf49fe64d2e8208aa Mon Sep 17 00:00:00 2001 From: Daniel Gee Date: Fri, 22 May 2026 21:18:32 +0100 Subject: [PATCH 2/4] fix: extract /api/ingredient URL into constant (S1192) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../IngredientIntegrationTests.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/backend/MenuApi.Integration.Tests/IngredientIntegrationTests.cs b/backend/MenuApi.Integration.Tests/IngredientIntegrationTests.cs index b1bd7239..67261249 100644 --- a/backend/MenuApi.Integration.Tests/IngredientIntegrationTests.cs +++ b/backend/MenuApi.Integration.Tests/IngredientIntegrationTests.cs @@ -12,6 +12,8 @@ namespace MenuApi.Integration.Tests; [Collection("API Host Collection")] public class IngredientIntegrationTests { + private const string IngredientEndpoint = "/api/ingredient"; + private readonly JsonSerializerOptions jsonOptions; private readonly ApiTestFixture fixture; @@ -28,7 +30,7 @@ public IngredientIntegrationTests(ApiTestFixture fixture) public async Task Get_Ingredients_Returns_Ok() { using var client = await fixture.GetHttpClient(); - using var response = await client.GetAsync("/api/ingredient"); + using var response = await client.GetAsync(IngredientEndpoint); await response.ShouldHaveStatusCode(HttpStatusCode.OK); @@ -78,7 +80,7 @@ public async Task Create_Ingredient_Then_Get_Ingredients_Contains_Created([Strin var (createdId, _, _) = await PostIngredientAsync(client, ingredientName, [3]); - using var response = await client.GetAsync("/api/ingredient"); + using var response = await client.GetAsync(IngredientEndpoint); await response.ShouldHaveStatusCode(HttpStatusCode.OK); var data = await response.Content.ReadAsStringAsync(); @@ -110,7 +112,7 @@ public async Task Create_Ingredient_With_Same_Name_Twice_Returns_Same_Id([String secondId.Should().Be(firstId); - using var listResponse = await client.GetAsync("/api/ingredient"); + using var listResponse = await client.GetAsync(IngredientEndpoint); await listResponse.ShouldHaveStatusCode(HttpStatusCode.OK); var data = await listResponse.Content.ReadAsStringAsync(); @@ -137,7 +139,7 @@ public async Task Create_Ingredient_With_Different_Case_Returns_Same_Id([StringL { var body = new NewIngredient { Name = name, UnitIds = unitIds }; var requestContent = new StringContent(JsonSerializer.Serialize(body, jsonOptions), Encoding.UTF8, "application/json"); - using var response = await client.PostAsync("/api/ingredient", requestContent); + using var response = await client.PostAsync(IngredientEndpoint, requestContent); await response.ShouldHaveStatusCode(HttpStatusCode.OK); From 0121b6619568f4cd81584fc5d190aec388871616 Mon Sep 17 00:00:00 2001 From: Daniel Gee Date: Fri, 22 May 2026 21:21:15 +0100 Subject: [PATCH 3/4] refactor: address Copilot PR review comments - Extract shared IngredientProjection/UnitProjection records and a MapToViewModel helper so the EF projection and ViewModel mapping are defined once across GetIngredientsAsync and both paths of CreateIngredientAsync (addresses S1192-style duplication comment) - Fix Create_Ingredient_With_Different_Case_Returns_Same_Id: prepend 'X' to the AutoFixture name so ToUpperInvariant/ToLowerInvariant are always distinct regardless of input character set - Fix same-name list-count assertion to use OrdinalIgnoreCase so it is robust against the shared-DB CI-collation edge case Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../IngredientIntegrationTests.cs | 11 +-- .../Repositories/IngredientRepository.cs | 70 ++++++------------- 2 files changed, 27 insertions(+), 54 deletions(-) diff --git a/backend/MenuApi.Integration.Tests/IngredientIntegrationTests.cs b/backend/MenuApi.Integration.Tests/IngredientIntegrationTests.cs index 67261249..5e9761fa 100644 --- a/backend/MenuApi.Integration.Tests/IngredientIntegrationTests.cs +++ b/backend/MenuApi.Integration.Tests/IngredientIntegrationTests.cs @@ -118,15 +118,18 @@ public async Task Create_Ingredient_With_Same_Name_Twice_Returns_Same_Id([String var data = await listResponse.Content.ReadAsStringAsync(); var ingredients = JsonSerializer.Deserialize>(data, jsonOptions); ingredients.Should().NotBeNull(); - ingredients!.Count(i => i.Name == ingredientName).Should().Be(1); + ingredients!.Count(i => string.Equals(i.Name, ingredientName, StringComparison.OrdinalIgnoreCase)).Should().Be(1); } [Theory, AutoData] - public async Task Create_Ingredient_With_Different_Case_Returns_Same_Id([StringLength(50, MinimumLength = 1)] string ingredientName) + public async Task Create_Ingredient_With_Different_Case_Returns_Same_Id([StringLength(49, MinimumLength = 1)] string ingredientName) { using var client = await fixture.GetHttpClient(); - var upperName = ingredientName.ToUpperInvariant(); - var lowerName = ingredientName.ToLowerInvariant(); + + // Prepend "X" to guarantee at least one cased letter so upper/lower are always distinct + var baseName = "X" + ingredientName; + var upperName = baseName.ToUpperInvariant(); + var lowerName = baseName.ToLowerInvariant(); var (firstId, _, _) = await PostIngredientAsync(client, upperName, [4]); var (secondId, _, _) = await PostIngredientAsync(client, lowerName, [1]); diff --git a/backend/MenuApi/Repositories/IngredientRepository.cs b/backend/MenuApi/Repositories/IngredientRepository.cs index 759b8c80..83cef2be 100644 --- a/backend/MenuApi/Repositories/IngredientRepository.cs +++ b/backend/MenuApi/Repositories/IngredientRepository.cs @@ -10,29 +10,14 @@ public class IngredientRepository(MenuDbContext db) : IIngredientRepository public async Task> GetIngredientsAsync() { var rows = await db.Ingredients - .Select(i => new - { + .Select(i => new IngredientProjection( i.Id, i.Name, - Units = i.IngredientUnits.Select(iu => new - { - iu.Unit.Name, - iu.Unit.Abbreviation, - UnitType = iu.Unit.UnitType.Name, - }) - }) + i.IngredientUnits.Select(iu => new UnitProjection(iu.Unit.Name, iu.Unit.Abbreviation, iu.Unit.UnitType.Name)))) .ToListAsync() .ConfigureAwait(false); - return rows.Select(i => new ViewModel.Ingredient - { - Id = IngredientId.From(i.Id), - Name = IngredientName.From(i.Name), - Units = i.Units.Select(u => new ViewModel.IngredientUnit( - IngredientUnitName.From(u.Name), - u.Abbreviation is not null ? IngredientUnitAbbreviation.From(u.Abbreviation) : null, - IngredientUnitType.From(u.UnitType))), - }); + return rows.Select(MapToViewModel); } public async Task CreateIngredientAsync(ViewModel.NewIngredient newIngredient) @@ -42,31 +27,16 @@ public class IngredientRepository(MenuDbContext db) : IIngredientRepository var existing = await db.Ingredients .Where(i => i.Name == newIngredient.Name.Value) .OrderBy(i => i.Id) - .Select(i => new - { + .Select(i => new IngredientProjection( i.Id, i.Name, - Units = i.IngredientUnits.Select(iu => new - { - iu.Unit.Name, - iu.Unit.Abbreviation, - UnitType = iu.Unit.UnitType.Name, - }) - }) + i.IngredientUnits.Select(iu => new UnitProjection(iu.Unit.Name, iu.Unit.Abbreviation, iu.Unit.UnitType.Name)))) .FirstOrDefaultAsync() .ConfigureAwait(false); if (existing is not null) { - return new ViewModel.Ingredient - { - Id = IngredientId.From(existing.Id), - Name = IngredientName.From(existing.Name), - Units = existing.Units.Select(u => new ViewModel.IngredientUnit( - IngredientUnitName.From(u.Name), - u.Abbreviation is not null ? IngredientUnitAbbreviation.From(u.Abbreviation) : null, - IngredientUnitType.From(u.UnitType))), - }; + return MapToViewModel(existing); } var unitIds = newIngredient.UnitIds.Distinct().ToList(); @@ -83,28 +53,28 @@ public class IngredientRepository(MenuDbContext db) : IIngredientRepository var created = await db.Ingredients .Where(i => i.Id == entity.Id) - .Select(i => new - { + .Select(i => new IngredientProjection( i.Id, i.Name, - Units = i.IngredientUnits.Select(iu => new - { - iu.Unit.Name, - iu.Unit.Abbreviation, - UnitType = iu.Unit.UnitType.Name, - }) - }) + i.IngredientUnits.Select(iu => new UnitProjection(iu.Unit.Name, iu.Unit.Abbreviation, iu.Unit.UnitType.Name)))) .FirstAsync() .ConfigureAwait(false); - return new ViewModel.Ingredient + return MapToViewModel(created); + } + + private static ViewModel.Ingredient MapToViewModel(IngredientProjection p) => + new() { - Id = IngredientId.From(created.Id), - Name = IngredientName.From(created.Name), - Units = created.Units.Select(u => new ViewModel.IngredientUnit( + Id = IngredientId.From(p.Id), + Name = IngredientName.From(p.Name), + Units = p.Units.Select(u => new ViewModel.IngredientUnit( IngredientUnitName.From(u.Name), u.Abbreviation is not null ? IngredientUnitAbbreviation.From(u.Abbreviation) : null, IngredientUnitType.From(u.UnitType))), }; - } + + private sealed record IngredientProjection(int Id, string Name, IEnumerable Units); + + private sealed record UnitProjection(string Name, string? Abbreviation, string UnitType); } From b8d881a47f566a15e81cccfd4e10cc1cb6e7379d Mon Sep 17 00:00:00 2001 From: Daniel Gee Date: Fri, 22 May 2026 21:34:18 +0100 Subject: [PATCH 4/4] test: lock in get-or-create contract for unknown UnitIds on existing ingredient When the canonical ingredient row already exists, UnitIds from the request are intentionally ignored. This test makes that contract explicit and prevents accidental regression toward validating unit IDs on the lookup path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Repositories/IngredientRepositoryTests.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/backend/MenuApi.Tests/Repositories/IngredientRepositoryTests.cs b/backend/MenuApi.Tests/Repositories/IngredientRepositoryTests.cs index 7a530e26..5950599c 100644 --- a/backend/MenuApi.Tests/Repositories/IngredientRepositoryTests.cs +++ b/backend/MenuApi.Tests/Repositories/IngredientRepositoryTests.cs @@ -66,6 +66,32 @@ public async Task CreateIngredientAsync_Returns_Existing_Ingredient_When_Name_Al count.Should().Be(1); } + [Fact] + public async Task CreateIngredientAsync_Returns_Existing_Ingredient_When_Name_Already_Exists_Even_With_Unknown_UnitId() + { + // UnitIds are only used when inserting a new row. When the canonical row already exists, + // the provided UnitIds are intentionally ignored — the existing ingredient is returned as-is. + var cancellationToken = TestContext.Current.CancellationToken; + await using var db = CreateDbContext(); + await SeedUnitsAsync(db, cancellationToken); + + var sut = new IngredientRepository(db); + var first = await sut.CreateIngredientAsync(new NewIngredient + { + Name = IngredientName.From("Sugar"), + UnitIds = [4], + }); + + var second = await sut.CreateIngredientAsync(new NewIngredient + { + Name = IngredientName.From("Sugar"), + UnitIds = [9999], // non-existent unit ID — ignored because ingredient already exists + }); + + second.Id.Should().Be(first.Id); + second.Units.Should().ContainSingle(u => u.Name == IngredientUnitName.From("Grams")); + } + private static MenuDbContext CreateDbContext() { var options = new DbContextOptionsBuilder()