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:
@@ -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)
|
||||||
|
|||||||
@@ -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,
|
||||||
|
|||||||
@@ -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
|
||||||
|
|||||||
@@ -265,4 +265,5 @@ class WeekPlanControllerTest {
|
|||||||
.principal(() -> "member@example.com"))
|
.principal(() -> "member@example.com"))
|
||||||
.andExpect(status().isForbidden());
|
.andExpect(status().isForbidden());
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user