Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Iris: Improve event handling for predictable scenarios #10025

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,10 @@ public void onBuildFailure(Result result) {
}
var exercise = validateExercise(participation.getExercise());

irisSettingsService.isActivatedForElseThrow(IrisEventType.BUILD_FAILED, exercise);
if (!irisSettingsService.isActivatedFor(IrisEventType.BUILD_FAILED, exercise)) {
log.info("Build failure event is not activated for exercise {}", exercise.getId());
return;
}

var participant = studentParticipation.getParticipant();
if (participant instanceof User user) {
Expand All @@ -215,7 +218,10 @@ public void onNewResult(Result result) {

var exercise = validateExercise(participation.getExercise());

irisSettingsService.isActivatedForElseThrow(IrisEventType.PROGRESS_STALLED, exercise);
if (!irisSettingsService.isActivatedFor(IrisEventType.PROGRESS_STALLED, exercise)) {
log.info("Progress stalled event is not activated for exercise {}", exercise.getId());
return;
}

var recentSubmissions = submissionRepository.findAllWithResultsByParticipationIdOrderBySubmissionDateAsc(studentParticipation.getId());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import java.util.TreeSet;
import java.util.function.Supplier;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.boot.context.event.ApplicationReadyEvent;
import org.springframework.context.annotation.Profile;
import org.springframework.context.event.EventListener;
Expand Down Expand Up @@ -60,6 +62,8 @@
@Profile(PROFILE_IRIS)
public class IrisSettingsService {

private static final Logger log = LoggerFactory.getLogger(IrisSettingsService.class);

private final IrisSettingsRepository irisSettingsRepository;

private final IrisSubSettingsService irisSubSettingsService;
Expand Down Expand Up @@ -476,38 +480,6 @@ public void isEnabledForElseThrow(IrisSubSettingsType type, Course course) {
}
}

/**
* Checks whether an Iris event is enabled for a course.
* Throws an exception if the chat feature is disabled.
* Throws an exception if the event is disabled.
*
* @param type The Iris event to check
* @param course The course to check
*/
public void isActivatedForElseThrow(IrisEventType type, Course course) {
isEnabledForElseThrow(IrisSubSettingsType.CHAT, course);

if (!isActivatedFor(type, course)) {
throw new AccessForbiddenAlertException("The Iris " + type.name() + " event is disabled for this course.", "Iris", "iris." + type.name().toLowerCase() + "Disabled");
}
}

/**
* Checks whether an Iris event is enabled for an exercise.
* Throws an exception if the chat feature is disabled.
* Throws an exception if the event is disabled.
*
* @param type The Iris event to check
* @param exercise The exercise to check
*/
public void isActivatedForElseThrow(IrisEventType type, Exercise exercise) {
isEnabledForElseThrow(IrisSubSettingsType.CHAT, exercise);

if (!isActivatedFor(type, exercise)) {
throw new AccessForbiddenAlertException("The Iris " + type.name() + " event is disabled for this exercise.", "Iris", "iris." + type.name().toLowerCase() + "Disabled");
}
}

/**
* Checks whether an Iris feature is enabled for a course.
*
Expand Down Expand Up @@ -540,6 +512,10 @@ public boolean isEnabledFor(IrisSubSettingsType type, Exercise exercise) {
* @return Whether the Iris event is active for the course
*/
public boolean isActivatedFor(IrisEventType type, Course course) {
if (!isEnabledFor(IrisSubSettingsType.CHAT, course)) {
log.debug("Chat is disabled for course {}", course.getId());
return false;
}
var settings = getCombinedIrisSettingsFor(course, false);
return isEventEnabledInSettings(settings, type);
}
Expand All @@ -552,6 +528,10 @@ public boolean isActivatedFor(IrisEventType type, Course course) {
* @return Whether the Iris event is active for the exercise
*/
public boolean isActivatedFor(IrisEventType type, Exercise exercise) {
if (!isEnabledFor(IrisSubSettingsType.CHAT, exercise)) {
log.debug("Chat is disabled for exercise {}", exercise.getId());
return false;
}
var settings = getCombinedIrisSettingsFor(exercise, false);
return isEventEnabledInSettings(settings, type);
}
Expand Down Expand Up @@ -760,15 +740,19 @@ private boolean isEventEnabledInSettings(IrisCombinedSettingsDTO settings, IrisE
return switch (type) {
case PROGRESS_STALLED -> {
if (settings.irisChatSettings().disabledProactiveEvents() != null) {
yield !settings.irisChatSettings().disabledProactiveEvents().contains(IrisEventType.PROGRESS_STALLED.name().toLowerCase());
var isEventEnabled = !settings.irisChatSettings().disabledProactiveEvents().contains(IrisEventType.PROGRESS_STALLED.name().toLowerCase());
log.debug("Event PROGRESS_STALLED enabled: {}", isEventEnabled);
yield isEventEnabled;
}
else {
yield true;
}
}
case BUILD_FAILED -> {
if (settings.irisChatSettings().disabledProactiveEvents() != null) {
yield !settings.irisChatSettings().disabledProactiveEvents().contains(IrisEventType.BUILD_FAILED.name().toLowerCase());
var isEventEnabled = !settings.irisChatSettings().disabledProactiveEvents().contains(IrisEventType.BUILD_FAILED.name().toLowerCase());
log.debug("Event BUILD_FAILED enabled: {}", isEventEnabled);
yield isEventEnabled;
}
else {
yield true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import de.tum.cit.aet.artemis.atlas.domain.competency.CompetencyJol;
import de.tum.cit.aet.artemis.core.domain.Course;
import de.tum.cit.aet.artemis.core.domain.User;
import de.tum.cit.aet.artemis.core.exception.AccessForbiddenAlertException;
import de.tum.cit.aet.artemis.core.user.util.UserUtilService;
import de.tum.cit.aet.artemis.exercise.domain.SubmissionType;
import de.tum.cit.aet.artemis.exercise.participation.util.ParticipationFactory;
Expand Down Expand Up @@ -264,7 +263,10 @@ void testShouldNotFireProgressStalledEventWithEventDisabled() {
createSubmissionWithScore(studentParticipation, 40);
createSubmissionWithScore(studentParticipation, 40);
var result = createSubmissionWithScore(studentParticipation, 40);
assertThatExceptionOfType(AccessForbiddenAlertException.class).isThrownBy(() -> pyrisEventService.trigger(new NewResultEvent(result)));
pyrisEventService.trigger(new NewResultEvent(result));

verify(irisExerciseChatSessionService, times(1)).onNewResult(any(Result.class));
verify(pyrisPipelineService, times(0)).executeExerciseChatPipeline(any(), any(), any(), any(), any());
}

@Test
Expand All @@ -278,7 +280,10 @@ void testShouldNotFireBuildFailedEventWhenEventSettingDisabled() {
irisExerciseChatSessionService.createChatSessionForProgrammingExercise(exercise, userUtilService.getUserByLogin(TEST_PREFIX + "student1"));
// Create a failing submission for the student.
var result = createFailingSubmission(studentParticipation);
assertThatExceptionOfType(AccessForbiddenAlertException.class).isThrownBy(() -> pyrisEventService.trigger(new NewResultEvent(result)));
pyrisEventService.trigger(new NewResultEvent(result));

verify(irisExerciseChatSessionService, times(1)).onBuildFailure(any(Result.class));
verify(pyrisPipelineService, times(0)).executeExerciseChatPipeline(any(), any(), any(), any(), any());
}

@Test
Expand Down
Loading