Extend create/update recipe endpoints to accept ingredients and steps - #1198
Conversation
5a5f688 to
62851d7
Compare
62851d7 to
e02194b
Compare
d99b3f4 to
bff6ac4
Compare
Adds two new IExceptionHandler implementations following the existing BusinessValidationExceptionHandler pattern, registered in Program.cs. RecipeRepository's title-uniqueness catch now throws ConflictException (409) instead of BusinessValidationException (422), per #1116. Also fixes a real stale-read bug surfaced while wiring this up: RecipeRepository.GetRecipeAsync/GetRecipesAsync called a private MapToDbModel helper *inside* the LINQ Select, which is not EF-translatable — EF had to materialize (and, with nothing suppressing tracking, track) a full RecipeEntity to run it client-side. Once UpdateRecipeAsync added a pre-transaction ownership-check read, that tracked entity went stale the moment ExecuteUpdateAsync wrote past the change tracker, and the post-update read-for-response reused the stale tracked instance via EF's identity resolution instead of the freshly committed row. Inlining the projection (and adding AsNoTracking) lets EF push it fully into SQL with no tracking at all. Part of #1116.
RecipeService.CreateRecipeAsync now upserts steps alongside ingredients in the same transaction. UpdateRecipeAsync fetches the existing recipe first (404 if missing), throws ForbiddenAccessException if the caller isn't the owner, then updates scalars, ingredients, and steps in one transaction. RecipeApi.UpdateRecipeAsync resolves the caller the same way the list/create endpoints already do and maps the service's not-found signal to a 404. UpsertRecipeValidator now accepts an empty (but non-null) ingredient list, matching the "zero ingredients/steps is a valid minimal recipe" requirement. Part of #1116.
Unit tests cover owner assignment on create, the 403/404 branches of update, and the relaxed empty-ingredients validation rule. Integration tests cover: create/update with both ingredients and steps in a single call, zero-ingredients/zero-steps acceptance, non-owner update returning 403 (via TestDatabaseSeeder), update of a nonexistent recipe returning 404, and the duplicate-title 422->409 status code change on both create and update. Part of #1116.
…recipe projection - Extract shared ProblemDetailsExceptionHandler<TException> base class; the three exception handlers (BusinessValidation/Conflict/ForbiddenAccess) were structurally identical aside from status/title/RFC type, which is what Sonar's duplication gate was flagging. - Reuse a single Expression<Func<RecipeEntity, DBModel.Recipe>> projection across GetRecipesAsync/GetRecipeAsync instead of two inlined copies. Kept as an Expression (not a method) so EF Core can still translate it inside .Select - a compiled method call there is what caused the original stale-tracked-entity bug this PR fixes.
bff6ac4 to
46283ba
Compare
|
There was a problem hiding this comment.
Pull request overview
Extends the MenuApi recipe authoring endpoints so create/update can accept and persist full recipe payloads (ingredients + steps) atomically, adds ownership enforcement for updates, and improves API error semantics (409 conflict, 403 forbidden). It also adjusts EF Core querying to avoid tracked-entity stale reads and updates validation + test coverage accordingly.
Changes:
- Wire step collection persistence into recipe create/update and add update-time ownership checks (403/404 behavior).
- Introduce shared ProblemDetails-based exception handling plus new Conflict/Forbidden exception types and switch duplicate-title to 409.
- Update validator + unit/integration tests to allow empty ingredient lists and cover the new behaviors.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/MenuApi/Validation/UpsertRecipeValidator.cs | Allows empty ingredient list while still rejecting null ingredients. |
| backend/MenuApi/Services/RecipeService.cs | Upserts steps during create/update; update now performs owner check and returns bool for not-found. |
| backend/MenuApi/Services/IRecipeService.cs | Updates service contract for update to include callerId and return not-found status. |
| backend/MenuApi/Repositories/RecipeRepository.cs | Uses an expression-based projection for EF friendliness; maps duplicate-title to ConflictException and adds no-tracking intent. |
| backend/MenuApi/Recipes/RecipeApi.cs | Updates endpoint metadata; PUT now returns 401/403/404/409 appropriately and returns IResult. |
| backend/MenuApi/Program.cs | Registers new exception handlers. |
| backend/MenuApi/Exceptions/ProblemDetailsExceptionHandler.cs | Adds shared base handler to emit consistent RFC-linked ProblemDetails payloads. |
| backend/MenuApi/Exceptions/ForbiddenAccessExceptionHandler.cs | Maps ForbiddenAccessException to 403 ProblemDetails. |
| backend/MenuApi/Exceptions/ForbiddenAccessException.cs | New exception type for forbidden updates. |
| backend/MenuApi/Exceptions/ConflictExceptionHandler.cs | Maps ConflictException to 409 ProblemDetails. |
| backend/MenuApi/Exceptions/ConflictException.cs | New exception type for conflict (duplicate title). |
| backend/MenuApi/Exceptions/BusinessValidationExceptionHandler.cs | Refactors 422 handler to reuse the new base ProblemDetails exception handler. |
| backend/MenuApi.Tests/Validation/UpsertRecipeValidatorTests.cs | Updates validator expectations (empty ingredients allowed; null ingredients rejected). |
| backend/MenuApi.Tests/Services/RecipeServiceTests.cs | Adds coverage for ownership/not-found paths and verifies steps upsert is invoked. |
| backend/MenuApi.Tests/Controllers/RecipeApiTests.cs | Updates PUT endpoint tests for IResult + 401/404 behavior. |
| backend/MenuApi.Integration.Tests/RecipeIntegrationTests.cs | Updates duplicate-title integration assertions from 422 to 409. |
| backend/MenuApi.Integration.Tests/RecipeCreateUpdateIntegrationTests.cs | Adds integration coverage for create/update with steps, minimal payload, 403 for non-owner, and 404 for missing recipe. |
Suppressed comments (1)
backend/MenuApi/Repositories/RecipeRepository.cs:58
- AsNoTracking() is applied after the projection. To ensure RecipeEntity instances are never tracked (and to make the stale-read fix robust), move AsNoTracking() onto the entity query before Select().
return await db.Recipes
.Where(r => r.Id == recipeId.Value)
.Select(ToDbModel)
.AsNoTracking()
.FirstOrDefaultAsync()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| using AwesomeAssertions; | ||
| using MenuApi.Integration.Tests.Factory; | ||
| using System.Net; | ||
| using System.Text; | ||
| using System.Text.Json; | ||
| using Xunit; |
| .OrderByDescending(r => r.UpdatedAtUtc) | ||
| .Take(take) | ||
| .Select(r => MapToDbModel(r)) | ||
| .Select(ToDbModel) | ||
| .AsNoTracking() | ||
| .ToListAsync() |



Summary
POST/PUTnow accept ingredients and steps together, persisted in a single transaction (steps were plumbed by Epic [Epic] Core recipe model changes (DB schema) #1098 but never wired into create/update).PUTnow checks ownership: 403 for non-owners, 404 for a missing recipe.(OwnerUserId, Title)now returns 409 (was 422) via a newConflictException/handler; a newForbiddenAccessException/handler backs the 403.UpsertRecipeValidatornow accepts an empty ingredient list (zero ingredients/steps is a valid minimal recipe).Also fixes a real stale-read bug found while wiring the ownership check:
RecipeRepository.GetRecipeAsync/GetRecipesAsynccalled a private mapping method inside a LINQSelect, which isn't EF-translatable — EF had to materialize (and, with nothing suppressing tracking, track) a fullRecipeEntityto run it. Once the new pre-update ownership check added a read before the write, that tracked entity went stale the momentExecuteUpdateAsyncwrote past the change tracker, and the post-update read-for-response reused the stale tracked instance via EF's identity resolution instead of the freshly committed row — title/scalar updates were silently reverting in the response (ingredients/steps updated fine). Fixed by inlining the projection with.AsNoTracking().Closes #1116
Part of #1099
Test plan
dotnet test MenuApi.Tests(unit)dotnet test MenuApi.Integration.Tests— create/update with ingredients+steps, non-owner 403, duplicate-title 409 (both create and update paths)