From 89ca8f8ce4fe0ba3ecf29f4bfa09f73bece33cf0 Mon Sep 17 00:00:00 2001 From: Daniel Gee Date: Sat, 23 May 2026 21:57:27 +0100 Subject: [PATCH] Expose conflict when same ingredient+unit has different amounts in recipe When a recipe request contains the same ingredient+unit combination more than once with different amounts, the behavior was previously undefined: exact duplicates were silently absorbed by Distinct(), but conflicting-amount pairs both survived into UpsertRecipeIngredientsAsync where the composite PK (RecipeId, IngredientId, UnitId) would cause an unhandled DbUpdateException and an HTTP 500. This is a business-significant duplicate: the same ingredient+unit appearing with two different amounts in one request is contradictory and the user must resolve it. The fix adds explicit conflict detection in RecipeService.NormalizeRecipeIngredients: after Distinct() removes exact duplicates, any remaining (IngredientName, UnitName) group with more than one entry throws a BusinessValidationException with all conflicting pairs named. BusinessValidationExceptionHandler returns HTTP 422. Both recipe endpoints already declare ProducesProblem(422) in the OpenAPI contract, so no contract or frontend changes are required. Closes #980 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../RecipeWithIngredientsIntegrationTests.cs | 65 +++++++++++++++ .../Services/RecipeServiceTests.cs | 79 +++++++++++++++++++ backend/MenuApi/Services/RecipeService.cs | 20 ++++- 3 files changed, 163 insertions(+), 1 deletion(-) diff --git a/backend/MenuApi.Integration.Tests/RecipeWithIngredientsIntegrationTests.cs b/backend/MenuApi.Integration.Tests/RecipeWithIngredientsIntegrationTests.cs index 492ec8ae..1656e9c5 100644 --- a/backend/MenuApi.Integration.Tests/RecipeWithIngredientsIntegrationTests.cs +++ b/backend/MenuApi.Integration.Tests/RecipeWithIngredientsIntegrationTests.cs @@ -235,6 +235,71 @@ public async Task Update_Recipe_With_Duplicate_Equivalent_Existing_Ingredient_Re returnedIngredients[0].Amount.Should().Be(125m); } + [Theory, AutoData] + public async Task Create_Recipe_With_Conflicting_Ingredient_Amounts_Returns_UnprocessableEntity( + [StringLength(50, MinimumLength = 1)] string ingredientName, + [StringLength(500, MinimumLength = 1)] string recipeName) + { + using var client = await fixture.GetHttpClient(); + + await PostIngredientAsync(client, ingredientName, [4]); + + var newRecipe = new NewRecipe + { + Name = recipeName, + Ingredients = + [ + new RecipeIngredient { Name = ingredientName, Unit = "Grams", Amount = 100m }, + new RecipeIngredient { Name = ingredientName, Unit = "Grams", Amount = 200m }, + ] + }; + + using var content = new StringContent(JsonSerializer.Serialize(newRecipe, jsonOptions), Encoding.UTF8, "application/json"); + using var response = await client.PostAsync("/api/recipe", content); + + await response.ShouldHaveStatusCode(HttpStatusCode.UnprocessableEntity); + + var responseBody = await response.Content.ReadAsStringAsync(); + using var doc = JsonDocument.Parse(responseBody); + doc.RootElement.GetProperty("status").GetInt32().Should().Be(422); + } + + [Theory, AutoData] + public async Task Update_Recipe_With_Conflicting_Ingredient_Amounts_Returns_UnprocessableEntity( + [StringLength(50, MinimumLength = 1)] string ingredientName, + [StringLength(500, MinimumLength = 1)] string recipeName, + [StringLength(500, MinimumLength = 1)] string updatedRecipeName) + { + using var client = await fixture.GetHttpClient(); + + await PostIngredientAsync(client, ingredientName, [4]); + + var (recipeId, _, _) = await PostRecipeAsync(client, new NewRecipe + { + Name = recipeName, + Ingredients = [new RecipeIngredient { Name = ingredientName, Unit = "Grams", Amount = 100m }] + }); + + var updateBody = new NewRecipe + { + Name = updatedRecipeName, + Ingredients = + [ + new RecipeIngredient { Name = ingredientName, Unit = "Grams", Amount = 100m }, + new RecipeIngredient { Name = ingredientName, Unit = "Grams", Amount = 200m }, + ] + }; + + using var content = new StringContent(JsonSerializer.Serialize(updateBody, jsonOptions), Encoding.UTF8, "application/json"); + using var response = await client.PutAsync($"/api/recipe/{recipeId}", content); + + await response.ShouldHaveStatusCode(HttpStatusCode.UnprocessableEntity); + + var responseBody = await response.Content.ReadAsStringAsync(); + using var doc = JsonDocument.Parse(responseBody); + doc.RootElement.GetProperty("status").GetInt32().Should().Be(422); + } + private async Task PostIngredientAsync(HttpClient client, string name, List unitIds) { var body = new NewIngredient { Name = name, UnitIds = unitIds }; diff --git a/backend/MenuApi.Tests/Services/RecipeServiceTests.cs b/backend/MenuApi.Tests/Services/RecipeServiceTests.cs index 0e56ebfc..e5b43021 100644 --- a/backend/MenuApi.Tests/Services/RecipeServiceTests.cs +++ b/backend/MenuApi.Tests/Services/RecipeServiceTests.cs @@ -1,6 +1,7 @@ using AwesomeAssertions; using FakeItEasy; using MenuDB; +using MenuApi.Exceptions; using MenuApi.Repositories; using MenuApi.Services; using MenuApi.ValueObjects; @@ -189,4 +190,82 @@ public async Task UpdateRecipe_Should_Throw_Exception_For_null_newRecipeAsync(Re var result = await fun.Should().ThrowAsync(); result.And.ParamName.Should().Be("newRecipe"); } + + [Fact] + public async Task CreateRecipeAsync_Throws_BusinessValidationException_When_Same_IngredientUnit_Has_Conflicting_Amounts() + { + var recipeName = RecipeName.From("Cake"); + + await sut.Invoking(s => s.CreateRecipeAsync(new NewRecipe + { + Name = recipeName, + Ingredients = + [ + new RecipeIngredient { Name = IngredientName.From("Sugar"), Unit = IngredientUnitName.From("Grams"), Amount = IngredientAmount.From(100m) }, + new RecipeIngredient { Name = IngredientName.From("Sugar"), Unit = IngredientUnitName.From("Grams"), Amount = IngredientAmount.From(200m) }, + ], + })).Should().ThrowAsync(); + + A.CallTo(() => recipeRepository.CreateRecipeAsync(A._)).MustNotHaveHappened(); + A.CallTo(() => recipeRepository.UpsertRecipeIngredientsAsync(A._, A>._)).MustNotHaveHappened(); + } + + [Fact] + public async Task UpdateRecipeAsync_Throws_BusinessValidationException_When_Same_IngredientUnit_Has_Conflicting_Amounts() + { + var recipeId = RecipeId.From(1); + var recipeName = RecipeName.From("Cake"); + + await sut.Invoking(s => s.UpdateRecipeAsync(recipeId, new NewRecipe + { + Name = recipeName, + Ingredients = + [ + new RecipeIngredient { Name = IngredientName.From("Sugar"), Unit = IngredientUnitName.From("Grams"), Amount = IngredientAmount.From(100m) }, + new RecipeIngredient { Name = IngredientName.From("Sugar"), Unit = IngredientUnitName.From("Grams"), Amount = IngredientAmount.From(200m) }, + ], + })).Should().ThrowAsync(); + + A.CallTo(() => recipeRepository.UpdateRecipeAsync(A._, A._)).MustNotHaveHappened(); + A.CallTo(() => recipeRepository.UpsertRecipeIngredientsAsync(A._, A>._)).MustNotHaveHappened(); + } + + [Fact] + public async Task CreateRecipeAsync_Reports_All_Conflicting_IngredientUnit_Pairs() + { + var recipeName = RecipeName.From("Cake"); + + var ex = await sut.Invoking(s => s.CreateRecipeAsync(new NewRecipe + { + Name = recipeName, + Ingredients = + [ + new RecipeIngredient { Name = IngredientName.From("Sugar"), Unit = IngredientUnitName.From("Grams"), Amount = IngredientAmount.From(100m) }, + new RecipeIngredient { Name = IngredientName.From("Sugar"), Unit = IngredientUnitName.From("Grams"), Amount = IngredientAmount.From(200m) }, + new RecipeIngredient { Name = IngredientName.From("Flour"), Unit = IngredientUnitName.From("Grams"), Amount = IngredientAmount.From(300m) }, + new RecipeIngredient { Name = IngredientName.From("Flour"), Unit = IngredientUnitName.From("Grams"), Amount = IngredientAmount.From(400m) }, + ], + })).Should().ThrowAsync(); + + ex.And.Message.Should().Contain("Sugar"); + ex.And.Message.Should().Contain("Flour"); + } + + [Fact] + public async Task CreateRecipeAsync_Exact_Duplicate_Then_Conflicting_Detects_Conflict() + { + // Three entries: two exact duplicates (silently absorbed) + one with different amount = conflict + var recipeName = RecipeName.From("Cake"); + + await sut.Invoking(s => s.CreateRecipeAsync(new NewRecipe + { + Name = recipeName, + Ingredients = + [ + new RecipeIngredient { Name = IngredientName.From("Sugar"), Unit = IngredientUnitName.From("Grams"), Amount = IngredientAmount.From(100m) }, + new RecipeIngredient { Name = IngredientName.From("Sugar"), Unit = IngredientUnitName.From("Grams"), Amount = IngredientAmount.From(100m) }, + new RecipeIngredient { Name = IngredientName.From("Sugar"), Unit = IngredientUnitName.From("Grams"), Amount = IngredientAmount.From(200m) }, + ], + })).Should().ThrowAsync(); + } } diff --git a/backend/MenuApi/Services/RecipeService.cs b/backend/MenuApi/Services/RecipeService.cs index 4491f5ba..748b6d3c 100644 --- a/backend/MenuApi/Services/RecipeService.cs +++ b/backend/MenuApi/Services/RecipeService.cs @@ -1,4 +1,5 @@ using MenuDB; +using MenuApi.Exceptions; using MenuApi.MappingProfiles; using MenuApi.Repositories; using MenuApi.ValueObjects; @@ -71,6 +72,23 @@ await strategy.ExecuteAsync(async () => private static IReadOnlyList NormalizeRecipeIngredients(IEnumerable recipeIngredients) { ArgumentNullException.ThrowIfNull(recipeIngredients); - return [.. ViewModelMapper.Map(recipeIngredients).Distinct()]; + + var deduped = ViewModelMapper.Map(recipeIngredients).Distinct().ToList(); + + // After exact-duplicate removal, any (IngredientName, UnitName) group with more than one + // entry represents the same ingredient+unit with conflicting amounts — a business conflict. + var conflicts = deduped + .GroupBy(i => (i.IngredientName, i.UnitName)) + .Where(g => g.Count() > 1) + .Select(g => $"'{g.Key.IngredientName.Value}' with unit '{g.Key.UnitName.Value}'") + .ToList(); + + if (conflicts.Count > 0) + { + throw new BusinessValidationException( + $"The following ingredients appear more than once with conflicting amounts: {string.Join(", ", conflicts)}."); + } + + return deduped; } }