mirror of
https://github.com/mealie-recipes/mealie.git
synced 2025-11-30 23:54:15 -05:00
fix: recipe recursion false positive (#6591)
Co-authored-by: Michael Genson <71845777+michael-genson@users.noreply.github.com> Co-authored-by: Michael Genson <genson.michael@gmail.com>
This commit is contained in:
committed by
GitHub
parent
3146e99b03
commit
8f1ce1a1c3
@@ -82,7 +82,6 @@ export function useParsedIngredientText(ingredient: RecipeIngredient, scale = 1,
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO: Add support for sub-recipes here?
|
|
||||||
const unitName = useUnitName(unit || undefined, usePluralUnit);
|
const unitName = useUnitName(unit || undefined, usePluralUnit);
|
||||||
const ingName = referencedRecipe ? referencedRecipe.name || "" : useFoodName(food || undefined, usePluralFood);
|
const ingName = referencedRecipe ? referencedRecipe.name || "" : useFoodName(food || undefined, usePluralFood);
|
||||||
|
|
||||||
|
|||||||
@@ -369,16 +369,21 @@ class RecipeService(RecipeServiceBase):
|
|||||||
|
|
||||||
return new_recipe
|
return new_recipe
|
||||||
|
|
||||||
def has_recursive_recipe_link(self, recipe: Recipe, visited: set[str] | None = None):
|
def has_recursive_recipe_link(self, recipe: Recipe, path: set[str] | None = None):
|
||||||
"""Recursively checks if a recipe links to itself through its ingredients."""
|
"""Recursively checks if a recipe links to itself through its ingredients."""
|
||||||
|
if path is None:
|
||||||
|
path = set()
|
||||||
|
|
||||||
if visited is None:
|
|
||||||
visited = set()
|
|
||||||
recipe_id = str(getattr(recipe, "id", None))
|
recipe_id = str(getattr(recipe, "id", None))
|
||||||
if recipe_id in visited:
|
|
||||||
|
# Check if this recipe is already in the current path (cycle detected)
|
||||||
|
if recipe_id in path:
|
||||||
return True
|
return True
|
||||||
|
|
||||||
visited.add(recipe_id)
|
# Add to the current path
|
||||||
|
path.add(recipe_id)
|
||||||
|
|
||||||
|
try:
|
||||||
ingredients = getattr(recipe, "recipe_ingredient", [])
|
ingredients = getattr(recipe, "recipe_ingredient", [])
|
||||||
for ing in ingredients:
|
for ing in ingredients:
|
||||||
try:
|
try:
|
||||||
@@ -386,8 +391,13 @@ class RecipeService(RecipeServiceBase):
|
|||||||
except (AttributeError, exceptions.NoEntryFound):
|
except (AttributeError, exceptions.NoEntryFound):
|
||||||
continue
|
continue
|
||||||
|
|
||||||
if self.has_recursive_recipe_link(sub_recipe, visited):
|
# Recursively check - path is modified in place and cleaned up via backtracking
|
||||||
|
if self.has_recursive_recipe_link(sub_recipe, path):
|
||||||
return True
|
return True
|
||||||
|
finally:
|
||||||
|
# Backtrack: remove this recipe from the path when done exploring this branch
|
||||||
|
path.discard(recipe_id)
|
||||||
|
|
||||||
return False
|
return False
|
||||||
|
|
||||||
def _pre_update_check(self, slug_or_id: str | UUID, new_data: Recipe) -> Recipe:
|
def _pre_update_check(self, slug_or_id: str | UUID, new_data: Recipe) -> Recipe:
|
||||||
|
|||||||
@@ -691,6 +691,130 @@ def test_recipe_recursion_cycle_three_level(api_client: TestClient, unique_user:
|
|||||||
assert "cannot reference itself" in response.text.lower()
|
assert "cannot reference itself" in response.text.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_recipe_recursion_valid_branched_chain(api_client: TestClient, unique_user: TestUser):
|
||||||
|
"""Test that valid branched nesting without cycles is allowed (d -> b -> a, d -> c -> a)."""
|
||||||
|
database = unique_user.repos
|
||||||
|
|
||||||
|
food = database.ingredient_foods.create(
|
||||||
|
SaveIngredientFood(
|
||||||
|
name=random_string(10),
|
||||||
|
group_id=unique_user.group_id,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
# Create recipe_a
|
||||||
|
recipe_a: Recipe = database.recipes.create(
|
||||||
|
Recipe(
|
||||||
|
name=random_string(10),
|
||||||
|
user_id=unique_user.user_id,
|
||||||
|
group_id=unique_user.group_id,
|
||||||
|
recipe_ingredient=[
|
||||||
|
RecipeIngredient(note="", food=food),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
# Create recipe_b
|
||||||
|
recipe_b: Recipe = database.recipes.create(
|
||||||
|
Recipe(
|
||||||
|
name=random_string(10),
|
||||||
|
user_id=unique_user.user_id,
|
||||||
|
group_id=unique_user.group_id,
|
||||||
|
recipe_ingredient=[
|
||||||
|
RecipeIngredient(note="", referenced_recipe=recipe_a),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
# Create recipe_c
|
||||||
|
recipe_c: Recipe = database.recipes.create(
|
||||||
|
Recipe(
|
||||||
|
name=random_string(10),
|
||||||
|
user_id=unique_user.user_id,
|
||||||
|
group_id=unique_user.group_id,
|
||||||
|
recipe_ingredient=[
|
||||||
|
RecipeIngredient(note="", referenced_recipe=recipe_a),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
# Create recipe_d to reference recipe_b and recipe_c
|
||||||
|
recipe_d: Recipe = database.recipes.create(
|
||||||
|
Recipe(
|
||||||
|
name=random_string(10),
|
||||||
|
user_id=unique_user.user_id,
|
||||||
|
group_id=unique_user.group_id,
|
||||||
|
recipe_ingredient=[
|
||||||
|
RecipeIngredient(note="", referenced_recipe=recipe_b),
|
||||||
|
RecipeIngredient(note="", referenced_recipe=recipe_c),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
)
|
||||||
|
recipe_url = api_routes.recipes_slug(recipe_d.slug)
|
||||||
|
response = api_client.get(recipe_url, headers=unique_user.token)
|
||||||
|
assert response.status_code == 200
|
||||||
|
recipe_data = json.loads(response.text)
|
||||||
|
|
||||||
|
response = api_client.put(recipe_url, json=recipe_data, headers=unique_user.token)
|
||||||
|
assert response.status_code == 200
|
||||||
|
|
||||||
|
|
||||||
|
def test_recipe_recursion_same_recipe_twice(api_client: TestClient, unique_user: TestUser):
|
||||||
|
"""Test that referencing the same recipe multiple times in one recipe is allowed.
|
||||||
|
|
||||||
|
This tests the bug where using the same sub-recipe twice (e.g., a spice mix used
|
||||||
|
in both marinade and vegetables) incorrectly triggered a cycle detection.
|
||||||
|
"""
|
||||||
|
database = unique_user.repos
|
||||||
|
|
||||||
|
food = database.ingredient_foods.create(
|
||||||
|
SaveIngredientFood(
|
||||||
|
name=random_string(10),
|
||||||
|
group_id=unique_user.group_id,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
# Create a spice mix recipe
|
||||||
|
sub_recipe = database.recipes.create(
|
||||||
|
Recipe(
|
||||||
|
name=random_string(10),
|
||||||
|
user_id=unique_user.user_id,
|
||||||
|
group_id=unique_user.group_id,
|
||||||
|
recipe_ingredient=[
|
||||||
|
RecipeIngredient(note="", food=food),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
# Create a main recipe that uses the spice mix twice
|
||||||
|
main_recipe = database.recipes.create(
|
||||||
|
Recipe(
|
||||||
|
name=random_string(10),
|
||||||
|
user_id=unique_user.user_id,
|
||||||
|
group_id=unique_user.group_id,
|
||||||
|
recipe_ingredient=[
|
||||||
|
RecipeIngredient(note="For marinade", referenced_recipe=sub_recipe),
|
||||||
|
RecipeIngredient(note="For vegetables", referenced_recipe=sub_recipe),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
# Verify we can fetch and update the recipe without errors
|
||||||
|
recipe_url = api_routes.recipes_slug(main_recipe.slug)
|
||||||
|
response = api_client.get(recipe_url, headers=unique_user.token)
|
||||||
|
assert response.status_code == 200
|
||||||
|
recipe_data = json.loads(response.text)
|
||||||
|
|
||||||
|
# Verify both ingredients are present
|
||||||
|
assert len(recipe_data["recipeIngredient"]) == 2
|
||||||
|
assert recipe_data["recipeIngredient"][0]["note"] == "For marinade"
|
||||||
|
assert recipe_data["recipeIngredient"][1]["note"] == "For vegetables"
|
||||||
|
|
||||||
|
# Try to update the recipe - this should not fail with a recursion error
|
||||||
|
response = api_client.put(recipe_url, json=recipe_data, headers=unique_user.token)
|
||||||
|
assert response.status_code == 200
|
||||||
|
|
||||||
|
|
||||||
def test_recipe_reference_deleted(api_client: TestClient, unique_user: TestUser):
|
def test_recipe_reference_deleted(api_client: TestClient, unique_user: TestUser):
|
||||||
"""Test that when a referenced recipe is deleted, the parent recipe remains intact."""
|
"""Test that when a referenced recipe is deleted, the parent recipe remains intact."""
|
||||||
database = unique_user.repos
|
database = unique_user.repos
|
||||||
|
|||||||
Reference in New Issue
Block a user