fix(backend): add role guard to variety-preview and extract shared scoring method

- 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 <noreply@anthropic.com>
This commit is contained in:
2026-04-09 08:11:45 +02:00
parent 4333dc0d84
commit ea7113ec53
4 changed files with 61 additions and 81 deletions

View File

@@ -166,68 +166,12 @@ public class PlanningService {
private double simulateVarietyScore(WeekPlan plan, Recipe candidate, LocalDate slotDate, private double simulateVarietyScore(WeekPlan plan, Recipe candidate, LocalDate slotDate,
VarietyScoreConfig config, Set<UUID> recentlyCookedIds) { VarietyScoreConfig config, Set<UUID> recentlyCookedIds) {
// Build a simulated slot list: existing slots + candidate on slotDate
List<SimulatedSlot> simulatedSlots = new ArrayList<>(); List<SimulatedSlot> simulatedSlots = new ArrayList<>();
for (WeekPlanSlot slot : plan.getSlots()) { for (WeekPlanSlot slot : plan.getSlots()) {
simulatedSlots.add(new SimulatedSlot(slot.getRecipe(), slot.getSlotDate())); simulatedSlots.add(new SimulatedSlot(slot.getRecipe(), slot.getSlotDate()));
} }
simulatedSlots.add(new SimulatedSlot(candidate, slotDate)); simulatedSlots.add(new SimulatedSlot(candidate, slotDate));
return scoreFromSimulatedSlots(simulatedSlots, config, recentlyCookedIds);
List<String> 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<String, List<LocalDate>> 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<String, List<LocalDate>> 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<UUID, Long> 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));
} }
private record SimulatedSlot(Recipe recipe, LocalDate date) {} private record SimulatedSlot(Recipe recipe, LocalDate date) {}
@@ -247,54 +191,57 @@ public class PlanningService {
.map(cl -> cl.getRecipe().getId()) .map(cl -> cl.getRecipe().getId())
.collect(Collectors.toSet()); .collect(Collectors.toSet());
// Current score: simulate with only existing slots List<SimulatedSlot> currentSlots = plan.getSlots().stream()
double currentScore = computeCurrentScore(plan, config, recentlyCookedIds); .map(s -> new SimulatedSlot(s.getRecipe(), s.getSlotDate()))
.toList();
// Projected score: add candidate on the given date double currentScore = currentSlots.isEmpty() ? 10.0
: scoreFromSimulatedSlots(currentSlots, config, recentlyCookedIds);
double projectedScore = simulateVarietyScore(plan, candidate, date, config, recentlyCookedIds); double projectedScore = simulateVarietyScore(plan, candidate, date, config, recentlyCookedIds);
return new VarietyPreviewResponse(currentScore, projectedScore, projectedScore - currentScore); return new VarietyPreviewResponse(currentScore, projectedScore, projectedScore - currentScore);
} }
private double computeCurrentScore(WeekPlan plan, VarietyScoreConfig config, Set<UUID> recentlyCookedIds) { private double scoreFromSimulatedSlots(List<SimulatedSlot> slots, VarietyScoreConfig config,
List<WeekPlanSlot> slots = plan.getSlots(); Set<UUID> recentlyCookedIds) {
if (slots.isEmpty()) return 10.0;
List<String> checkedTagTypes = config.getRepeatTagTypes(); List<String> checkedTagTypes = config.getRepeatTagTypes();
double wTagRepeat = config.getWTagRepeat().doubleValue(); double wTagRepeat = config.getWTagRepeat().doubleValue();
double wIngredientOverlap = config.getWIngredientOverlap().doubleValue(); double wIngredientOverlap = config.getWIngredientOverlap().doubleValue();
double wRecentRepeat = config.getWRecentRepeat().doubleValue(); double wRecentRepeat = config.getWRecentRepeat().doubleValue();
double wPlanDuplicate = config.getWPlanDuplicate().doubleValue(); double wPlanDuplicate = config.getWPlanDuplicate().doubleValue();
// 1. Tag-type repeats on consecutive days
Map<String, List<LocalDate>> tagDays = new LinkedHashMap<>(); Map<String, List<LocalDate>> tagDays = new LinkedHashMap<>();
for (WeekPlanSlot slot : slots) { for (SimulatedSlot slot : slots) {
for (Tag tag : slot.getRecipe().getTags()) { for (Tag tag : slot.recipe.getTags()) {
if (checkedTagTypes.contains(tag.getTagType())) { 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(); long tagRepeatCount = tagDays.values().stream().filter(this::hasConsecutiveDays).count();
// 2. Non-staple ingredient overlaps on consecutive days
Map<String, List<LocalDate>> ingredientDays = new LinkedHashMap<>(); Map<String, List<LocalDate>> ingredientDays = new LinkedHashMap<>();
for (WeekPlanSlot slot : slots) { for (SimulatedSlot slot : slots) {
for (RecipeIngredient ri : slot.getRecipe().getIngredients()) { for (RecipeIngredient ri : slot.recipe.getIngredients()) {
if (!ri.getIngredient().isStaple()) { if (!ri.getIngredient().isStaple()) {
ingredientDays.computeIfAbsent(ri.getIngredient().getName(), k -> new ArrayList<>()) ingredientDays.computeIfAbsent(ri.getIngredient().getName(), k -> new ArrayList<>())
.add(slot.getSlotDate()); .add(slot.date);
} }
} }
} }
long ingredientOverlapCount = ingredientDays.values().stream().filter(this::hasConsecutiveDays).count(); long ingredientOverlapCount = ingredientDays.values().stream().filter(this::hasConsecutiveDays).count();
// 3. Recent repeats from cooking log
long recentRepeatCount = slots.stream() long recentRepeatCount = slots.stream()
.map(s -> s.getRecipe().getId()) .map(s -> s.recipe.getId())
.distinct() .distinct()
.filter(recentlyCookedIds::contains) .filter(recentlyCookedIds::contains)
.count(); .count();
// 4. Duplicate recipes within the plan
Map<UUID, Long> recipeCounts = slots.stream() Map<UUID, Long> 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() long duplicatePenaltyCount = recipeCounts.values().stream()
.filter(c -> c > 1) .filter(c -> c > 1)
.mapToLong(c -> c - 1) .mapToLong(c -> c - 1)

View File

@@ -98,6 +98,7 @@ public class WeekPlanController {
} }
@GetMapping("/{planId}/variety-preview") @GetMapping("/{planId}/variety-preview")
@RequiresHouseholdRole("member")
public VarietyPreviewResponse getVarietyPreview( public VarietyPreviewResponse getVarietyPreview(
Principal principal, Principal principal,
@PathVariable UUID planId, @PathVariable UUID planId,

View File

@@ -447,34 +447,65 @@ class PlanningServiceTest {
// ── Variety preview ── // ── Variety preview ──
@Test @Test
void getVarietyPreviewShouldReturnScoreDelta() { void getVarietyPreviewShouldReturnScoreDeltaForDifferentRecipe() {
var household = testHousehold(); var household = testHousehold();
var plan = testWeekPlan(household); var plan = testWeekPlan(household);
var planId = plan.getId(); var planId = plan.getId();
// Plan already has one slot (Mon) // Plan already has one slot (Mon) with Spaghetti
var existingRecipe = testRecipe(household, "Spaghetti"); var existingRecipe = testRecipe(household, "Spaghetti");
var slot = new WeekPlanSlot(plan, existingRecipe, WEEK_START); var slot = new WeekPlanSlot(plan, existingRecipe, WEEK_START);
setId(slot, WeekPlanSlot.class, UUID.randomUUID()); setId(slot, WeekPlanSlot.class, UUID.randomUUID());
plan.getSlots().add(slot); plan.getSlots().add(slot);
// Candidate is Lachsfilet (different recipe, no shared tags/ingredients)
var candidate = testRecipe(household, "Lachsfilet"); var candidate = testRecipe(household, "Lachsfilet");
var candidateId = candidate.getId(); var candidateId = candidate.getId();
when(weekPlanRepository.findById(planId)).thenReturn(Optional.of(plan)); when(weekPlanRepository.findById(planId)).thenReturn(Optional.of(plan));
when(recipeRepository.findByIdAndHouseholdIdAndDeletedAtIsNull(candidateId, HOUSEHOLD_ID)) when(recipeRepository.findByIdAndHouseholdIdAndDeletedAtIsNull(candidateId, HOUSEHOLD_ID))
.thenReturn(Optional.of(candidate)); .thenReturn(Optional.of(candidate));
when(varietyScoreConfigRepository.findByHouseholdId(HOUSEHOLD_ID)) when(varietyScoreConfigRepository.findByHouseholdId(HOUSEHOLD_ID)).thenReturn(Optional.empty());
.thenReturn(Optional.empty());
when(cookingLogRepository.findByHouseholdIdAndCookedOnAfter(eq(HOUSEHOLD_ID), any())) when(cookingLogRepository.findByHouseholdIdAndCookedOnAfter(eq(HOUSEHOLD_ID), any()))
.thenReturn(List.of()); .thenReturn(List.of());
var result = planningService.getVarietyPreview(HOUSEHOLD_ID, planId, candidateId, WEEK_START.plusDays(1)); 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 // 1 existing slot with no conflicts → currentScore = 10.0
assertThat(result.projectedScore()).isBetween(0.0, 10.0); // Adding a different recipe with no tags/ingredients → projectedScore = 10.0, delta = 0
assertThat(result.scoreDelta()).isEqualTo(result.projectedScore() - result.currentScore()); assertThat(result.currentScore()).isEqualTo(10.0);
assertThat(result.currentScore()).isBetween(0.0, 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 @Test

View File

@@ -265,4 +265,5 @@ class WeekPlanControllerTest {
.principal(() -> "member@example.com")) .principal(() -> "member@example.com"))
.andExpect(status().isForbidden()); .andExpect(status().isForbidden());
} }
} }