diff --git a/openaev-api/src/main/java/io/openaev/migration/V5_21__Add_delete_cascade_exercise_email_reply_to.java b/openaev-api/src/main/java/io/openaev/migration/V5_21__Add_delete_cascade_exercise_email_reply_to.java new file mode 100644 index 00000000000..0972e4c53de --- /dev/null +++ b/openaev-api/src/main/java/io/openaev/migration/V5_21__Add_delete_cascade_exercise_email_reply_to.java @@ -0,0 +1,41 @@ +package io.openaev.migration; + +import java.sql.Statement; +import org.flywaydb.core.api.migration.BaseJavaMigration; +import org.flywaydb.core.api.migration.Context; +import org.springframework.stereotype.Component; + +@Component +public class V5_21__Add_delete_cascade_exercise_email_reply_to extends BaseJavaMigration { + + @Override + public void migrate(Context context) throws Exception { + try (Statement statement = context.getConnection().createStatement()) { + statement.execute( + """ + DO $$ + BEGIN + -- Drop existing constraint if present + IF EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'fk_exercise_id' + AND conrelid = 'exercise_mails_reply_to'::regclass + ) THEN + ALTER TABLE exercise_mails_reply_to DROP CONSTRAINT fk_exercise_id; + END IF; + + -- Re-create with ON DELETE CASCADE if not already present + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'fk_exercise_id' + AND conrelid = 'exercise_mails_reply_to'::regclass + ) THEN + ALTER TABLE exercise_mails_reply_to + ADD CONSTRAINT fk_exercise_id + FOREIGN KEY (exercise_id) REFERENCES exercises(exercise_id) ON DELETE CASCADE; + END IF; + END $$; + """); + } + } +} diff --git a/openaev-api/src/main/java/io/openaev/rest/exercise/ExerciseApi.java b/openaev-api/src/main/java/io/openaev/rest/exercise/ExerciseApi.java index ea09831001c..bf75ba879e0 100644 --- a/openaev-api/src/main/java/io/openaev/rest/exercise/ExerciseApi.java +++ b/openaev-api/src/main/java/io/openaev/rest/exercise/ExerciseApi.java @@ -564,10 +564,8 @@ public Exercise updateExerciseLessons( resourceId = "#exerciseId", actionPerformed = Action.DELETE, resourceType = ResourceType.SIMULATION) - @Transactional(rollbackFor = Exception.class) public void deleteExercise(@PathVariable String exerciseId) { - Exercise exercise = exerciseService.exercise(exerciseId); - exerciseRepository.delete(exercise); + exerciseService.deleteById(exerciseId); } @GetMapping({EXERCISE_URI + "/{exerciseId}", TENANT_EXERCISE_URI + "/{exerciseId}"}) diff --git a/openaev-api/src/main/java/io/openaev/rest/exercise/service/ExerciseService.java b/openaev-api/src/main/java/io/openaev/rest/exercise/service/ExerciseService.java index 322ab56eb75..bb855aea70f 100644 --- a/openaev-api/src/main/java/io/openaev/rest/exercise/service/ExerciseService.java +++ b/openaev-api/src/main/java/io/openaev/rest/exercise/service/ExerciseService.java @@ -553,7 +553,9 @@ public Exercise saveSimulation(Exercise simulation) { * @param simulationId ID of the simulation to delete */ public void deleteById(String simulationId) { + existsByIdAndTenantId(simulationId); exerciseRepository.deleteById(simulationId); + log.info("Simulation {} deleted by user {}", simulationId, currentUser().getId()); } @Transactional(rollbackFor = Exception.class) diff --git a/openaev-api/src/test/java/io/openaev/rest/exercise/ExerciseApiTest.java b/openaev-api/src/test/java/io/openaev/rest/exercise/ExerciseApiTest.java index 1c586edd282..651f7975b56 100644 --- a/openaev-api/src/test/java/io/openaev/rest/exercise/ExerciseApiTest.java +++ b/openaev-api/src/test/java/io/openaev/rest/exercise/ExerciseApiTest.java @@ -596,6 +596,64 @@ void replacingTeamsShouldPersistNewTeamListInDatabase() throws Exception { } } + @Nested + @DisplayName("Delete simulation") + @WithMockUser(withCapabilities = {Capability.DELETE_ASSESSMENT, Capability.MANAGE_ASSESSMENT}) + class DeleteSimulation { + + @Test + @DisplayName("Should delete exercise and cascade related data") + void given_existingExercise_should_deleteExerciseAndCascadeRelatedData() throws Exception { + // Arrange + User user = userRepository.save(UserFixture.getUser("Del", "USER", "del-user@fake.email")); + USER_IDS.add(user.getId()); + + Team team = teamRepository.save(TeamFixture.getTeam(user, "DelTeam", false)); + TEAM_IDS.add(team.getId()); + + Exercise exercise = ExerciseFixture.createDefaultCrisisExercise(); + exercise.setTeams(List.of(team)); + exercise.setReplyTos(List.of("reply@test.com")); + Exercise exerciseSaved = exerciseRepository.save(exercise); + EXERCISE_IDS.add(exerciseSaved.getId()); + + ExerciseTeamUser exerciseTeamUser = new ExerciseTeamUser(); + exerciseTeamUser.setExercise(exerciseSaved); + exerciseTeamUser.setTeam(team); + exerciseTeamUser.setUser(user); + exerciseTeamUserRepository.save(exerciseTeamUser); + + entityManager.flush(); + entityManager.clear(); + + // Act + mvc.perform(delete(EXERCISE_URI + "/" + exerciseSaved.getId()).with(csrf())) + .andExpect(status().is2xxSuccessful()); + + entityManager.flush(); + entityManager.clear(); + + // Assert + assertFalse( + exerciseRepository.findById(exerciseSaved.getId()).isPresent(), + "Exercise should be deleted"); + assertTrue( + exerciseTeamUserRepository.rawByExerciseIds(List.of(exerciseSaved.getId())).isEmpty(), + "Exercise team users should be cascade-deleted"); + } + + @Test + @DisplayName("Should return 404 when exercise does not exist") + void given_nonExistentExercise_should_returnNotFound() throws Exception { + // Arrange + String nonExistentId = UUID.randomUUID().toString(); + + // Act & Assert + mvc.perform(delete(EXERCISE_URI + "/" + nonExistentId).with(csrf())) + .andExpect(status().isNotFound()); + } + } + @Nested @DisplayName("Inject check") @WithMockUser(isAdmin = true) diff --git a/openaev-api/src/test/java/io/openaev/rest/exercise/service/ExerciseServiceUnitTest.java b/openaev-api/src/test/java/io/openaev/rest/exercise/service/ExerciseServiceUnitTest.java index b2ba7207535..31124f6c3d8 100644 --- a/openaev-api/src/test/java/io/openaev/rest/exercise/service/ExerciseServiceUnitTest.java +++ b/openaev-api/src/test/java/io/openaev/rest/exercise/service/ExerciseServiceUnitTest.java @@ -5,6 +5,8 @@ import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; +import io.openaev.config.OpenAEVPrincipal; +import io.openaev.config.SessionHelper; import io.openaev.config.cache.LicenseCacheManager; import io.openaev.context.TenantContext; import io.openaev.database.model.*; @@ -422,9 +424,36 @@ void saveSimulation_shouldDelegate() { } @Test - void deleteById_shouldDelegate() { - mockedExerciseService.deleteById("id"); - verify(exerciseRepository).deleteById("id"); + void deleteById_shouldCheckTenantAndDelegate() { + try (MockedStatic tc = mockStatic(TenantContext.class); + MockedStatic sh = mockStatic(SessionHelper.class)) { + // Arrange + tc.when(TenantContext::getCurrentTenant).thenReturn("tenant-1"); + when(exerciseRepository.existsByIdAndTenantId("id", "tenant-1")).thenReturn(true); + OpenAEVPrincipal principal = mock(OpenAEVPrincipal.class); + when(principal.getId()).thenReturn("user-1"); + sh.when(SessionHelper::currentUser).thenReturn(principal); + + // Act + mockedExerciseService.deleteById("id"); + + // Assert + verify(exerciseRepository).existsByIdAndTenantId("id", "tenant-1"); + verify(exerciseRepository).deleteById("id"); + } + } + + @Test + void deleteById_shouldThrowWhenExerciseNotInTenant() { + try (MockedStatic tc = mockStatic(TenantContext.class)) { + // Arrange + tc.when(TenantContext::getCurrentTenant).thenReturn("tenant-1"); + when(exerciseRepository.existsByIdAndTenantId("id", "tenant-1")).thenReturn(false); + + // Act & Assert + assertThrows(ElementNotFoundException.class, () -> mockedExerciseService.deleteById("id")); + verify(exerciseRepository, never()).deleteById(anyString()); + } } } diff --git a/openaev-model/src/main/java/io/openaev/database/repository/ExerciseRepository.java b/openaev-model/src/main/java/io/openaev/database/repository/ExerciseRepository.java index bdf13328ed9..d30e9a00454 100644 --- a/openaev-model/src/main/java/io/openaev/database/repository/ExerciseRepository.java +++ b/openaev-model/src/main/java/io/openaev/database/repository/ExerciseRepository.java @@ -29,6 +29,10 @@ public interface ExerciseRepository boolean existsByIdAndTenantId(@NotNull String id, @NotNull String tenantId); + @Modifying + @Query(value = "DELETE FROM exercises WHERE exercise_id = :simulationId", nativeQuery = true) + void deleteById(@Param("simulationId") String simulationId); + /** Called by background job (scheduled task) — cross-tenant scoped by design. */ @Query(value = "select e from Exercise e where e.status = 'SCHEDULED' and e.start <= :start") List findAllShouldBeInRunningState(@Param("start") Instant start);