diff --git a/backend/src/main/java/de/effigenix/application/production/AddRecipeIngredient.java b/backend/src/main/java/de/effigenix/application/production/AddRecipeIngredient.java index 0725fff..b2f2b06 100644 --- a/backend/src/main/java/de/effigenix/application/production/AddRecipeIngredient.java +++ b/backend/src/main/java/de/effigenix/application/production/AddRecipeIngredient.java @@ -12,10 +12,13 @@ public class AddRecipeIngredient { private final RecipeRepository recipeRepository; private final AuthorizationPort authorizationPort; + private final RecipeCycleChecker cycleChecker; - public AddRecipeIngredient(RecipeRepository recipeRepository, AuthorizationPort authorizationPort) { + public AddRecipeIngredient(RecipeRepository recipeRepository, AuthorizationPort authorizationPort, + RecipeCycleChecker cycleChecker) { this.recipeRepository = recipeRepository; this.authorizationPort = authorizationPort; + this.cycleChecker = cycleChecker; } public Result execute(AddRecipeIngredientCommand cmd, ActorId performedBy) { @@ -37,6 +40,13 @@ public class AddRecipeIngredient { } } + if (cmd.subRecipeId() != null) { + switch (cycleChecker.check(cmd.recipeId(), cmd.subRecipeId())) { + case Result.Failure(var err) -> { return Result.failure(err); } + case Result.Success(var ignored) -> { } + } + } + var draft = new IngredientDraft( cmd.position(), cmd.articleId(), cmd.quantity(), cmd.uom(), cmd.subRecipeId(), cmd.substitutable() diff --git a/backend/src/main/java/de/effigenix/application/production/RecipeCycleChecker.java b/backend/src/main/java/de/effigenix/application/production/RecipeCycleChecker.java new file mode 100644 index 0000000..3c8ba49 --- /dev/null +++ b/backend/src/main/java/de/effigenix/application/production/RecipeCycleChecker.java @@ -0,0 +1,68 @@ +package de.effigenix.application.production; + +import de.effigenix.domain.production.*; +import de.effigenix.shared.common.Result; + +import java.util.*; + +public class RecipeCycleChecker { + + private final RecipeRepository recipeRepository; + + public RecipeCycleChecker(RecipeRepository recipeRepository) { + this.recipeRepository = recipeRepository; + } + + public Result check(String parentRecipeId, String subRecipeId) { + if (parentRecipeId.equals(subRecipeId)) { + return Result.failure(new RecipeError.CyclicDependencyDetected( + List.of(parentRecipeId, subRecipeId))); + } + + var visited = new HashSet(); + var stack = new ArrayDeque>(); + stack.push(List.of(parentRecipeId, subRecipeId)); + + while (!stack.isEmpty()) { + var path = stack.pop(); + var currentId = path.get(path.size() - 1); + + if (!visited.add(currentId)) { + continue; + } + + Recipe recipe; + switch (recipeRepository.findById(RecipeId.of(currentId))) { + case Result.Failure(var err) -> + { return Result.failure(new RecipeError.RepositoryFailure(err.message())); } + case Result.Success(var opt) -> { + if (opt.isEmpty()) { + continue; + } + recipe = opt.get(); + } + } + + for (var ingredient : recipe.ingredients()) { + var childSubRecipeId = ingredient.subRecipeId(); + if (childSubRecipeId == null) { + continue; + } + + if (childSubRecipeId.equals(parentRecipeId)) { + var cyclePath = new ArrayList<>(path); + cyclePath.add(childSubRecipeId); + return Result.failure(new RecipeError.CyclicDependencyDetected(cyclePath)); + } + + if (!visited.contains(childSubRecipeId)) { + var newPath = new ArrayList<>(path); + newPath.add(childSubRecipeId); + stack.push(newPath); + } + } + } + + return Result.success(null); + } +} diff --git a/backend/src/main/java/de/effigenix/domain/production/RecipeError.java b/backend/src/main/java/de/effigenix/domain/production/RecipeError.java index bd1db52..7bba67c 100644 --- a/backend/src/main/java/de/effigenix/domain/production/RecipeError.java +++ b/backend/src/main/java/de/effigenix/domain/production/RecipeError.java @@ -1,5 +1,7 @@ package de.effigenix.domain.production; +import java.util.List; + public sealed interface RecipeError { String code(); @@ -63,6 +65,11 @@ public sealed interface RecipeError { @Override public String code() { return "UNAUTHORIZED"; } } + record CyclicDependencyDetected(List cyclePath) implements RecipeError { + @Override public String code() { return "RECIPE_CYCLIC_DEPENDENCY"; } + @Override public String message() { return "Cyclic dependency detected: " + String.join(" → ", cyclePath); } + } + record RepositoryFailure(String message) implements RecipeError { @Override public String code() { return "REPOSITORY_ERROR"; } } diff --git a/backend/src/main/java/de/effigenix/infrastructure/config/ProductionUseCaseConfiguration.java b/backend/src/main/java/de/effigenix/infrastructure/config/ProductionUseCaseConfiguration.java index b167e79..bcd0bb7 100644 --- a/backend/src/main/java/de/effigenix/infrastructure/config/ProductionUseCaseConfiguration.java +++ b/backend/src/main/java/de/effigenix/infrastructure/config/ProductionUseCaseConfiguration.java @@ -5,6 +5,7 @@ import de.effigenix.application.production.ArchiveRecipe; import de.effigenix.application.production.AddProductionStep; import de.effigenix.application.production.AddRecipeIngredient; import de.effigenix.application.production.CreateRecipe; +import de.effigenix.application.production.RecipeCycleChecker; import de.effigenix.application.production.GetRecipe; import de.effigenix.application.production.ListRecipes; import de.effigenix.application.production.RemoveProductionStep; @@ -23,8 +24,14 @@ public class ProductionUseCaseConfiguration { } @Bean - public AddRecipeIngredient addRecipeIngredient(RecipeRepository recipeRepository, AuthorizationPort authorizationPort) { - return new AddRecipeIngredient(recipeRepository, authorizationPort); + public RecipeCycleChecker recipeCycleChecker(RecipeRepository recipeRepository) { + return new RecipeCycleChecker(recipeRepository); + } + + @Bean + public AddRecipeIngredient addRecipeIngredient(RecipeRepository recipeRepository, AuthorizationPort authorizationPort, + RecipeCycleChecker recipeCycleChecker) { + return new AddRecipeIngredient(recipeRepository, authorizationPort, recipeCycleChecker); } @Bean diff --git a/backend/src/main/java/de/effigenix/infrastructure/production/web/exception/ProductionErrorHttpStatusMapper.java b/backend/src/main/java/de/effigenix/infrastructure/production/web/exception/ProductionErrorHttpStatusMapper.java index 618e79b..5ae5937 100644 --- a/backend/src/main/java/de/effigenix/infrastructure/production/web/exception/ProductionErrorHttpStatusMapper.java +++ b/backend/src/main/java/de/effigenix/infrastructure/production/web/exception/ProductionErrorHttpStatusMapper.java @@ -19,6 +19,7 @@ public final class ProductionErrorHttpStatusMapper { case RecipeError.NotInDraftStatus e -> 409; case RecipeError.InvalidStatusTransition e -> 409; case RecipeError.NoIngredients e -> 400; + case RecipeError.CyclicDependencyDetected e -> 400; case RecipeError.Unauthorized e -> 403; case RecipeError.RepositoryFailure e -> 500; }; diff --git a/backend/src/test/java/de/effigenix/application/production/AddRecipeIngredientTest.java b/backend/src/test/java/de/effigenix/application/production/AddRecipeIngredientTest.java new file mode 100644 index 0000000..dafa653 --- /dev/null +++ b/backend/src/test/java/de/effigenix/application/production/AddRecipeIngredientTest.java @@ -0,0 +1,134 @@ +package de.effigenix.application.production; + +import de.effigenix.application.production.command.AddRecipeIngredientCommand; +import de.effigenix.domain.production.*; +import de.effigenix.shared.common.Result; +import de.effigenix.shared.security.ActorId; +import de.effigenix.shared.security.AuthorizationPort; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.List; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +@DisplayName("AddRecipeIngredient Use Case") +class AddRecipeIngredientTest { + + @Mock private RecipeRepository recipeRepository; + @Mock private AuthorizationPort authPort; + @Mock private RecipeCycleChecker cycleChecker; + + private AddRecipeIngredient addRecipeIngredient; + private ActorId performedBy; + + @BeforeEach + void setUp() { + addRecipeIngredient = new AddRecipeIngredient(recipeRepository, authPort, cycleChecker); + performedBy = ActorId.of("admin-user"); + } + + private Recipe draftRecipe() { + return Recipe.create(new RecipeDraft( + "Bratwurst", 1, RecipeType.FINISHED_PRODUCT, + null, 85, 14, "100", "KILOGRAM" + )).unsafeGetValue(); + } + + @Test + @DisplayName("should_AddIngredient_When_NoSubRecipeId") + void should_AddIngredient_When_NoSubRecipeId() { + var recipe = draftRecipe(); + when(authPort.can(performedBy, ProductionAction.RECIPE_WRITE)).thenReturn(true); + when(recipeRepository.findById(recipe.id())).thenReturn(Result.success(Optional.of(recipe))); + when(recipeRepository.save(any())).thenReturn(Result.success(null)); + + var cmd = new AddRecipeIngredientCommand( + recipe.id().value(), 1, "article-123", "5.5", "KILOGRAM", null, false); + + var result = addRecipeIngredient.execute(cmd, performedBy); + + assertThat(result.isSuccess()).isTrue(); + assertThat(result.unsafeGetValue().ingredients()).hasSize(1); + verifyNoInteractions(cycleChecker); + } + + @Test + @DisplayName("should_AddIngredient_When_SubRecipeIdWithoutCycle") + void should_AddIngredient_When_SubRecipeIdWithoutCycle() { + var recipe = draftRecipe(); + when(authPort.can(performedBy, ProductionAction.RECIPE_WRITE)).thenReturn(true); + when(recipeRepository.findById(recipe.id())).thenReturn(Result.success(Optional.of(recipe))); + when(recipeRepository.save(any())).thenReturn(Result.success(null)); + when(cycleChecker.check(recipe.id().value(), "sub-recipe-1")).thenReturn(Result.success(null)); + + var cmd = new AddRecipeIngredientCommand( + recipe.id().value(), 1, "article-123", "5.5", "KILOGRAM", "sub-recipe-1", false); + + var result = addRecipeIngredient.execute(cmd, performedBy); + + assertThat(result.isSuccess()).isTrue(); + verify(cycleChecker).check(recipe.id().value(), "sub-recipe-1"); + } + + @Test + @DisplayName("should_FailWithUnauthorized_When_ActorLacksPermission") + void should_FailWithUnauthorized_When_ActorLacksPermission() { + when(authPort.can(performedBy, ProductionAction.RECIPE_WRITE)).thenReturn(false); + + var cmd = new AddRecipeIngredientCommand( + "recipe-1", 1, "article-123", "5.5", "KILOGRAM", "sub-recipe-1", false); + + var result = addRecipeIngredient.execute(cmd, performedBy); + + assertThat(result.isFailure()).isTrue(); + assertThat(result.unsafeGetError()).isInstanceOf(RecipeError.Unauthorized.class); + verifyNoInteractions(cycleChecker); + verify(recipeRepository, never()).save(any()); + } + + @Test + @DisplayName("should_FailWithRecipeNotFound_When_RecipeDoesNotExist") + void should_FailWithRecipeNotFound_When_RecipeDoesNotExist() { + when(authPort.can(performedBy, ProductionAction.RECIPE_WRITE)).thenReturn(true); + when(recipeRepository.findById(RecipeId.of("nonexistent"))).thenReturn(Result.success(Optional.empty())); + + var cmd = new AddRecipeIngredientCommand( + "nonexistent", 1, "article-123", "5.5", "KILOGRAM", "sub-recipe-1", false); + + var result = addRecipeIngredient.execute(cmd, performedBy); + + assertThat(result.isFailure()).isTrue(); + assertThat(result.unsafeGetError()).isInstanceOf(RecipeError.RecipeNotFound.class); + verifyNoInteractions(cycleChecker); + verify(recipeRepository, never()).save(any()); + } + + @Test + @DisplayName("should_FailWithCyclicDependency_When_CycleDetected") + void should_FailWithCyclicDependency_When_CycleDetected() { + var recipe = draftRecipe(); + when(authPort.can(performedBy, ProductionAction.RECIPE_WRITE)).thenReturn(true); + when(recipeRepository.findById(recipe.id())).thenReturn(Result.success(Optional.of(recipe))); + when(cycleChecker.check(recipe.id().value(), "sub-recipe-1")) + .thenReturn(Result.failure(new RecipeError.CyclicDependencyDetected( + List.of(recipe.id().value(), "sub-recipe-1", recipe.id().value())))); + + var cmd = new AddRecipeIngredientCommand( + recipe.id().value(), 1, "article-123", "5.5", "KILOGRAM", "sub-recipe-1", false); + + var result = addRecipeIngredient.execute(cmd, performedBy); + + assertThat(result.isFailure()).isTrue(); + assertThat(result.unsafeGetError()).isInstanceOf(RecipeError.CyclicDependencyDetected.class); + verify(recipeRepository, never()).save(any()); + } +} diff --git a/backend/src/test/java/de/effigenix/application/production/RecipeCycleCheckerTest.java b/backend/src/test/java/de/effigenix/application/production/RecipeCycleCheckerTest.java new file mode 100644 index 0000000..7bee407 --- /dev/null +++ b/backend/src/test/java/de/effigenix/application/production/RecipeCycleCheckerTest.java @@ -0,0 +1,185 @@ +package de.effigenix.application.production; + +import de.effigenix.domain.production.*; +import de.effigenix.shared.common.Quantity; +import de.effigenix.shared.common.RepositoryError; +import de.effigenix.shared.common.Result; +import de.effigenix.shared.common.UnitOfMeasure; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.math.BigDecimal; +import java.time.LocalDateTime; +import java.util.List; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +@DisplayName("RecipeCycleChecker") +class RecipeCycleCheckerTest { + + @Mock private RecipeRepository recipeRepository; + + private RecipeCycleChecker cycleChecker; + + @BeforeEach + void setUp() { + cycleChecker = new RecipeCycleChecker(recipeRepository); + } + + private Recipe recipeWithSubRecipes(String id, String... subRecipeIds) { + var ingredients = new java.util.ArrayList(); + int pos = 1; + for (var subId : subRecipeIds) { + ingredients.add(Ingredient.reconstitute( + IngredientId.generate(), pos++, "article-1", + Quantity.of(new BigDecimal("1"), UnitOfMeasure.KILOGRAM).unsafeGetValue(), + subId, false)); + } + return Recipe.reconstitute( + RecipeId.of(id), new RecipeName("Recipe-" + id), 1, RecipeType.FINISHED_PRODUCT, + null, new YieldPercentage(100), 14, + Quantity.of(new BigDecimal("100"), UnitOfMeasure.KILOGRAM).unsafeGetValue(), + RecipeStatus.DRAFT, ingredients, List.of(), + LocalDateTime.now(), LocalDateTime.now()); + } + + private Recipe recipeWithoutSubRecipes(String id) { + return recipeWithSubRecipes(id); + } + + @Test + @DisplayName("should_Allow_When_LinearChainWithoutCycle") + void should_Allow_When_LinearChainWithoutCycle() { + // A -> B -> C (no cycle) + when(recipeRepository.findById(RecipeId.of("B"))).thenReturn(Result.success(Optional.of(recipeWithSubRecipes("B", "C")))); + when(recipeRepository.findById(RecipeId.of("C"))).thenReturn(Result.success(Optional.of(recipeWithoutSubRecipes("C")))); + + var result = cycleChecker.check("A", "B"); + + assertThat(result.isSuccess()).isTrue(); + } + + @Test + @DisplayName("should_DetectCycle_When_SelfReference") + void should_DetectCycle_When_SelfReference() { + var result = cycleChecker.check("A", "A"); + + assertThat(result.isFailure()).isTrue(); + var error = (RecipeError.CyclicDependencyDetected) result.unsafeGetError(); + assertThat(error.cyclePath()).containsExactly("A", "A"); + } + + @Test + @DisplayName("should_DetectCycle_When_DirectCycle") + void should_DetectCycle_When_DirectCycle() { + // A -> B -> A + when(recipeRepository.findById(RecipeId.of("B"))).thenReturn(Result.success(Optional.of(recipeWithSubRecipes("B", "A")))); + + var result = cycleChecker.check("A", "B"); + + assertThat(result.isFailure()).isTrue(); + var error = (RecipeError.CyclicDependencyDetected) result.unsafeGetError(); + assertThat(error.cyclePath()).containsExactly("A", "B", "A"); + } + + @Test + @DisplayName("should_DetectCycle_When_IndirectCycle") + void should_DetectCycle_When_IndirectCycle() { + // A -> B -> C -> A + when(recipeRepository.findById(RecipeId.of("B"))).thenReturn(Result.success(Optional.of(recipeWithSubRecipes("B", "C")))); + when(recipeRepository.findById(RecipeId.of("C"))).thenReturn(Result.success(Optional.of(recipeWithSubRecipes("C", "A")))); + + var result = cycleChecker.check("A", "B"); + + assertThat(result.isFailure()).isTrue(); + var error = (RecipeError.CyclicDependencyDetected) result.unsafeGetError(); + assertThat(error.cyclePath()).containsExactly("A", "B", "C", "A"); + } + + @Test + @DisplayName("should_Allow_When_SubRecipeNotFound") + void should_Allow_When_SubRecipeNotFound() { + when(recipeRepository.findById(RecipeId.of("B"))).thenReturn(Result.success(Optional.empty())); + + var result = cycleChecker.check("A", "B"); + + assertThat(result.isSuccess()).isTrue(); + } + + @Test + @DisplayName("should_Allow_When_DiamondDependency") + void should_Allow_When_DiamondDependency() { + // A -> B -> D, A -> C -> D (diamond, no cycle) + when(recipeRepository.findById(RecipeId.of("B"))).thenReturn(Result.success(Optional.of(recipeWithSubRecipes("B", "D")))); + when(recipeRepository.findById(RecipeId.of("D"))).thenReturn(Result.success(Optional.of(recipeWithoutSubRecipes("D")))); + + var result = cycleChecker.check("A", "B"); + + assertThat(result.isSuccess()).isTrue(); + } + + @Test + @DisplayName("should_Allow_When_MixedIngredientsWithAndWithoutSubRecipeId") + void should_Allow_When_MixedIngredientsWithAndWithoutSubRecipeId() { + // B has ingredients: one with subRecipeId=null, one with subRecipeId=C + var ingredients = List.of( + Ingredient.reconstitute( + IngredientId.generate(), 1, "article-1", + Quantity.of(new BigDecimal("1"), UnitOfMeasure.KILOGRAM).unsafeGetValue(), + null, false), + Ingredient.reconstitute( + IngredientId.generate(), 2, "article-2", + Quantity.of(new BigDecimal("2"), UnitOfMeasure.KILOGRAM).unsafeGetValue(), + "C", false) + ); + var recipeB = Recipe.reconstitute( + RecipeId.of("B"), new RecipeName("Recipe-B"), 1, RecipeType.FINISHED_PRODUCT, + null, new YieldPercentage(100), 14, + Quantity.of(new BigDecimal("100"), UnitOfMeasure.KILOGRAM).unsafeGetValue(), + RecipeStatus.DRAFT, ingredients, List.of(), + LocalDateTime.now(), LocalDateTime.now()); + + when(recipeRepository.findById(RecipeId.of("B"))).thenReturn(Result.success(Optional.of(recipeB))); + when(recipeRepository.findById(RecipeId.of("C"))).thenReturn(Result.success(Optional.of(recipeWithoutSubRecipes("C")))); + + var result = cycleChecker.check("A", "B"); + + assertThat(result.isSuccess()).isTrue(); + } + + @Test + @DisplayName("should_DetectCycle_When_OnlyOneBranchIsCyclic") + void should_DetectCycle_When_OnlyOneBranchIsCyclic() { + // A -> B, B has sub-recipes C and D, C is a leaf, D -> A (cycle) + when(recipeRepository.findById(RecipeId.of("B"))).thenReturn(Result.success(Optional.of(recipeWithSubRecipes("B", "C", "D")))); + lenient().when(recipeRepository.findById(RecipeId.of("C"))).thenReturn(Result.success(Optional.of(recipeWithoutSubRecipes("C")))); + when(recipeRepository.findById(RecipeId.of("D"))).thenReturn(Result.success(Optional.of(recipeWithSubRecipes("D", "A")))); + + var result = cycleChecker.check("A", "B"); + + assertThat(result.isFailure()).isTrue(); + var error = (RecipeError.CyclicDependencyDetected) result.unsafeGetError(); + assertThat(error.cyclePath()).endsWith("A"); + assertThat(error.cyclePath().getFirst()).isEqualTo("A"); + } + + @Test + @DisplayName("should_FailWithRepositoryFailure_When_RepositoryError") + void should_FailWithRepositoryFailure_When_RepositoryError() { + when(recipeRepository.findById(RecipeId.of("B"))) + .thenReturn(Result.failure(new RepositoryError.DatabaseError("connection lost"))); + + var result = cycleChecker.check("A", "B"); + + assertThat(result.isFailure()).isTrue(); + assertThat(result.unsafeGetError()).isInstanceOf(RecipeError.RepositoryFailure.class); + } +}