From ea7113ec5366176c27775f5b6d4b1df20be730d3 Mon Sep 17 00:00:00 2001 From: Marcel Raddatz Date: Thu, 9 Apr 2026 08:11:45 +0200 Subject: [PATCH] fix(backend): add role guard to variety-preview and extract shared scoring method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add @RequiresHouseholdRole("member") to GET /{planId}/variety-preview endpoint to require household membership (was accessible to any authenticated user) - Extract scoreFromSimulatedSlots() private method eliminating duplicate logic between simulateVarietyScore() and the old computeCurrentScore() - Fix loose variety preview test assertions (isBetween → exact assertEquals) - Add test verifying negative scoreDelta when candidate is a duplicate recipe Co-Authored-By: Claude Sonnet 4.6 --- .../recipeapp/planning/PlanningService.java | 93 ++++--------------- .../planning/WeekPlanController.java | 1 + .../planning/PlanningServiceTest.java | 47 ++++++++-- .../planning/WeekPlanControllerTest.java | 1 + 4 files changed, 61 insertions(+), 81 deletions(-) diff --git a/backend/src/main/java/com/recipeapp/planning/PlanningService.java b/backend/src/main/java/com/recipeapp/planning/PlanningService.java index 6d78414..0e39c54 100644 --- a/backend/src/main/java/com/recipeapp/planning/PlanningService.java +++ b/backend/src/main/java/com/recipeapp/planning/PlanningService.java @@ -166,68 +166,12 @@ public class PlanningService { private double simulateVarietyScore(WeekPlan plan, Recipe candidate, LocalDate slotDate, VarietyScoreConfig config, Set recentlyCookedIds) { - // Build a simulated slot list: existing slots + candidate on slotDate List simulatedSlots = new ArrayList<>(); for (WeekPlanSlot slot : plan.getSlots()) { simulatedSlots.add(new SimulatedSlot(slot.getRecipe(), slot.getSlotDate())); } simulatedSlots.add(new SimulatedSlot(candidate, slotDate)); - - List checkedTagTypes = config.getRepeatTagTypes(); - double wTagRepeat = config.getWTagRepeat().doubleValue(); - double wIngredientOverlap = config.getWIngredientOverlap().doubleValue(); - double wRecentRepeat = config.getWRecentRepeat().doubleValue(); - double wPlanDuplicate = config.getWPlanDuplicate().doubleValue(); - - // 1. Tag-type repeats on consecutive days - Map> tagDays = new LinkedHashMap<>(); - for (SimulatedSlot slot : simulatedSlots) { - for (Tag tag : slot.recipe.getTags()) { - if (checkedTagTypes.contains(tag.getTagType())) { - tagDays.computeIfAbsent(tag.getName(), k -> new ArrayList<>()) - .add(slot.date); - } - } - } - long tagRepeatCount = tagDays.values().stream() - .filter(this::hasConsecutiveDays) - .count(); - - // 2. Non-staple ingredient overlaps on consecutive days - Map> ingredientDays = new LinkedHashMap<>(); - for (SimulatedSlot slot : simulatedSlots) { - for (RecipeIngredient ri : slot.recipe.getIngredients()) { - if (!ri.getIngredient().isStaple()) { - ingredientDays.computeIfAbsent(ri.getIngredient().getName(), k -> new ArrayList<>()) - .add(slot.date); - } - } - } - long ingredientOverlapCount = ingredientDays.values().stream() - .filter(this::hasConsecutiveDays) - .count(); - - // 3. Recent repeats from cooking log - long recentRepeatCount = simulatedSlots.stream() - .map(s -> s.recipe.getId()) - .distinct() - .filter(recentlyCookedIds::contains) - .count(); - - // 4. Duplicate recipes within the simulated plan - Map recipeCounts = simulatedSlots.stream() - .collect(Collectors.groupingBy(s -> s.recipe.getId(), Collectors.counting())); - long duplicatePenaltyCount = recipeCounts.values().stream() - .filter(c -> c > 1) - .mapToLong(c -> c - 1) - .sum(); - - double score = 10.0; - score -= tagRepeatCount * wTagRepeat; - score -= ingredientOverlapCount * wIngredientOverlap; - score -= recentRepeatCount * wRecentRepeat; - score -= duplicatePenaltyCount * wPlanDuplicate; - return Math.max(0, Math.min(10, score)); + return scoreFromSimulatedSlots(simulatedSlots, config, recentlyCookedIds); } private record SimulatedSlot(Recipe recipe, LocalDate date) {} @@ -247,54 +191,57 @@ public class PlanningService { .map(cl -> cl.getRecipe().getId()) .collect(Collectors.toSet()); - // Current score: simulate with only existing slots - double currentScore = computeCurrentScore(plan, config, recentlyCookedIds); - - // Projected score: add candidate on the given date + List currentSlots = plan.getSlots().stream() + .map(s -> new SimulatedSlot(s.getRecipe(), s.getSlotDate())) + .toList(); + double currentScore = currentSlots.isEmpty() ? 10.0 + : scoreFromSimulatedSlots(currentSlots, config, recentlyCookedIds); double projectedScore = simulateVarietyScore(plan, candidate, date, config, recentlyCookedIds); return new VarietyPreviewResponse(currentScore, projectedScore, projectedScore - currentScore); } - private double computeCurrentScore(WeekPlan plan, VarietyScoreConfig config, Set recentlyCookedIds) { - List slots = plan.getSlots(); - if (slots.isEmpty()) return 10.0; - + private double scoreFromSimulatedSlots(List slots, VarietyScoreConfig config, + Set recentlyCookedIds) { List checkedTagTypes = config.getRepeatTagTypes(); double wTagRepeat = config.getWTagRepeat().doubleValue(); double wIngredientOverlap = config.getWIngredientOverlap().doubleValue(); double wRecentRepeat = config.getWRecentRepeat().doubleValue(); double wPlanDuplicate = config.getWPlanDuplicate().doubleValue(); + // 1. Tag-type repeats on consecutive days Map> tagDays = new LinkedHashMap<>(); - for (WeekPlanSlot slot : slots) { - for (Tag tag : slot.getRecipe().getTags()) { + for (SimulatedSlot slot : slots) { + for (Tag tag : slot.recipe.getTags()) { if (checkedTagTypes.contains(tag.getTagType())) { - tagDays.computeIfAbsent(tag.getName(), k -> new ArrayList<>()).add(slot.getSlotDate()); + tagDays.computeIfAbsent(tag.getName(), k -> new ArrayList<>()).add(slot.date); } } } long tagRepeatCount = tagDays.values().stream().filter(this::hasConsecutiveDays).count(); + // 2. Non-staple ingredient overlaps on consecutive days Map> ingredientDays = new LinkedHashMap<>(); - for (WeekPlanSlot slot : slots) { - for (RecipeIngredient ri : slot.getRecipe().getIngredients()) { + for (SimulatedSlot slot : slots) { + for (RecipeIngredient ri : slot.recipe.getIngredients()) { if (!ri.getIngredient().isStaple()) { ingredientDays.computeIfAbsent(ri.getIngredient().getName(), k -> new ArrayList<>()) - .add(slot.getSlotDate()); + .add(slot.date); } } } long ingredientOverlapCount = ingredientDays.values().stream().filter(this::hasConsecutiveDays).count(); + // 3. Recent repeats from cooking log long recentRepeatCount = slots.stream() - .map(s -> s.getRecipe().getId()) + .map(s -> s.recipe.getId()) .distinct() .filter(recentlyCookedIds::contains) .count(); + // 4. Duplicate recipes within the plan Map recipeCounts = slots.stream() - .collect(Collectors.groupingBy(s -> s.getRecipe().getId(), Collectors.counting())); + .collect(Collectors.groupingBy(s -> s.recipe.getId(), Collectors.counting())); long duplicatePenaltyCount = recipeCounts.values().stream() .filter(c -> c > 1) .mapToLong(c -> c - 1) diff --git a/backend/src/main/java/com/recipeapp/planning/WeekPlanController.java b/backend/src/main/java/com/recipeapp/planning/WeekPlanController.java index c86183f..a8dea57 100644 --- a/backend/src/main/java/com/recipeapp/planning/WeekPlanController.java +++ b/backend/src/main/java/com/recipeapp/planning/WeekPlanController.java @@ -98,6 +98,7 @@ public class WeekPlanController { } @GetMapping("/{planId}/variety-preview") + @RequiresHouseholdRole("member") public VarietyPreviewResponse getVarietyPreview( Principal principal, @PathVariable UUID planId, diff --git a/backend/src/test/java/com/recipeapp/planning/PlanningServiceTest.java b/backend/src/test/java/com/recipeapp/planning/PlanningServiceTest.java index b5b4aa2..6ab3f52 100644 --- a/backend/src/test/java/com/recipeapp/planning/PlanningServiceTest.java +++ b/backend/src/test/java/com/recipeapp/planning/PlanningServiceTest.java @@ -447,34 +447,65 @@ class PlanningServiceTest { // ── Variety preview ── @Test - void getVarietyPreviewShouldReturnScoreDelta() { + void getVarietyPreviewShouldReturnScoreDeltaForDifferentRecipe() { var household = testHousehold(); var plan = testWeekPlan(household); var planId = plan.getId(); - // Plan already has one slot (Mon) + // Plan already has one slot (Mon) with Spaghetti var existingRecipe = testRecipe(household, "Spaghetti"); var slot = new WeekPlanSlot(plan, existingRecipe, WEEK_START); setId(slot, WeekPlanSlot.class, UUID.randomUUID()); plan.getSlots().add(slot); + // Candidate is Lachsfilet (different recipe, no shared tags/ingredients) var candidate = testRecipe(household, "Lachsfilet"); var candidateId = candidate.getId(); when(weekPlanRepository.findById(planId)).thenReturn(Optional.of(plan)); when(recipeRepository.findByIdAndHouseholdIdAndDeletedAtIsNull(candidateId, HOUSEHOLD_ID)) .thenReturn(Optional.of(candidate)); - when(varietyScoreConfigRepository.findByHouseholdId(HOUSEHOLD_ID)) - .thenReturn(Optional.empty()); + when(varietyScoreConfigRepository.findByHouseholdId(HOUSEHOLD_ID)).thenReturn(Optional.empty()); when(cookingLogRepository.findByHouseholdIdAndCookedOnAfter(eq(HOUSEHOLD_ID), any())) .thenReturn(List.of()); var result = planningService.getVarietyPreview(HOUSEHOLD_ID, planId, candidateId, WEEK_START.plusDays(1)); - // With no penalties, projected score should be 10.0; current (1 slot, no conflicts) is also 10.0 - assertThat(result.projectedScore()).isBetween(0.0, 10.0); - assertThat(result.scoreDelta()).isEqualTo(result.projectedScore() - result.currentScore()); - assertThat(result.currentScore()).isBetween(0.0, 10.0); + // 1 existing slot with no conflicts → currentScore = 10.0 + // Adding a different recipe with no tags/ingredients → projectedScore = 10.0, delta = 0 + assertThat(result.currentScore()).isEqualTo(10.0); + assertThat(result.projectedScore()).isEqualTo(10.0); + assertThat(result.scoreDelta()).isEqualTo(0.0); + } + + @Test + void getVarietyPreviewShouldReturnNegativeDeltaForDuplicateRecipe() { + var household = testHousehold(); + var plan = testWeekPlan(household); + var planId = plan.getId(); + + // Plan already has Spaghetti on Mon + var existingRecipe = testRecipe(household, "Spaghetti"); + var slot = new WeekPlanSlot(plan, existingRecipe, WEEK_START); + setId(slot, WeekPlanSlot.class, UUID.randomUUID()); + plan.getSlots().add(slot); + + // Candidate is the same Spaghetti recipe → triggers duplicate penalty (wPlanDuplicate = 2.0) + when(weekPlanRepository.findById(planId)).thenReturn(Optional.of(plan)); + when(recipeRepository.findByIdAndHouseholdIdAndDeletedAtIsNull(existingRecipe.getId(), HOUSEHOLD_ID)) + .thenReturn(Optional.of(existingRecipe)); + when(varietyScoreConfigRepository.findByHouseholdId(HOUSEHOLD_ID)).thenReturn(Optional.empty()); + when(cookingLogRepository.findByHouseholdIdAndCookedOnAfter(eq(HOUSEHOLD_ID), any())) + .thenReturn(List.of()); + + var result = planningService.getVarietyPreview( + HOUSEHOLD_ID, planId, existingRecipe.getId(), WEEK_START.plusDays(1)); + + // currentScore = 10.0 (1 slot, no conflicts) + // projectedScore = 10.0 - 1 * 2.0 (duplicate penalty) = 8.0 + assertThat(result.currentScore()).isEqualTo(10.0); + assertThat(result.projectedScore()).isEqualTo(8.0); + assertThat(result.scoreDelta()).isEqualTo(-2.0); } @Test diff --git a/backend/src/test/java/com/recipeapp/planning/WeekPlanControllerTest.java b/backend/src/test/java/com/recipeapp/planning/WeekPlanControllerTest.java index 4169294..e827493 100644 --- a/backend/src/test/java/com/recipeapp/planning/WeekPlanControllerTest.java +++ b/backend/src/test/java/com/recipeapp/planning/WeekPlanControllerTest.java @@ -265,4 +265,5 @@ class WeekPlanControllerTest { .principal(() -> "member@example.com")) .andExpect(status().isForbidden()); } + }