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

Programming exercises: Fix an issue with missing schedules to combine template commits #10215

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ dependencies {
implementation "io.sentry:sentry-logback:${sentry_version}"
implementation "io.sentry:sentry-spring-boot-starter-jakarta:${sentry_version}"

// NOTE: the following six dependencies use the newer versions explicitly to avoid other dependencies to use older versions
// NOTE: the following dependencies use the newer versions explicitly to avoid other dependencies to use older versions
implementation "ch.qos.logback:logback-classic:${logback_version}"
implementation "ch.qos.logback:logback-core:${logback_version}"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,13 @@ public void updateSchedulingForReleasedExercises(Exercise exercise) {
private void scheduleNotificationForReleasedExercise(Exercise exercise) {
try {
checkSecurityUtils();
scheduleService.scheduleTask(exercise, ExerciseLifecycle.RELEASE, () -> {
scheduleService.scheduleExerciseTask(exercise, ExerciseLifecycle.RELEASE, () -> {
checkSecurityUtils();
Exercise foundCurrentVersionOfScheduledExercise = exerciseRepository.findByIdElseThrow(exercise.getId());
if (checkIfTimeIsCorrectForScheduledTask(foundCurrentVersionOfScheduledExercise.getReleaseDate())) {
groupNotificationService.notifyAllGroupsAboutReleasedExercise(foundCurrentVersionOfScheduledExercise);
}
});
}, "notify about release exercise");
log.debug("Scheduled notify about started exercise after due date for exercise '{}' (#{}) for {}.", exercise.getTitle(), exercise.getId(), exercise.getReleaseDate());
}
catch (Exception exception) {
Expand Down Expand Up @@ -155,13 +155,13 @@ public void updateSchedulingForAssessedExercisesSubmissions(Exercise exercise) {
private void scheduleNotificationForAssessedExercisesSubmissions(Exercise exercise) {
try {
checkSecurityUtils();
scheduleService.scheduleTask(exercise, ExerciseLifecycle.ASSESSMENT_DUE, () -> {
scheduleService.scheduleExerciseTask(exercise, ExerciseLifecycle.ASSESSMENT_DUE, () -> {
checkSecurityUtils();
Exercise foundCurrentVersionOfScheduledExercise = exerciseRepository.findByIdElseThrow(exercise.getId());
if (checkIfTimeIsCorrectForScheduledTask(foundCurrentVersionOfScheduledExercise.getAssessmentDueDate())) {
singleUserNotificationService.notifyUsersAboutAssessedExerciseSubmission(foundCurrentVersionOfScheduledExercise);
}
});
}, "notify about assessed exercise submission");
log.debug("Scheduled notify about assessed exercise submission after assessment due date for exercise '{}' (#{}) at {}.", exercise.getTitle(), exercise.getId(),
exercise.getAssessmentDueDate());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,18 @@ public class TaskSchedulingConfiguration {

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

/**
* Create a Task Scheduler with virtual threads and a pool size of 4.
*
* @return the Task Scheduler bean that can be injected into any service to schedule tasks
*/
@Bean(name = "taskScheduler")
public TaskScheduler taskScheduler() {
log.debug("Creating Task Scheduler ");
var scheduler = new ThreadPoolTaskScheduler();
scheduler.setVirtualThreads(true);
krusche marked this conversation as resolved.
Show resolved Hide resolved
scheduler.setPoolSize(4);
scheduler.initialize();
return scheduler;
}
}

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions src/main/java/de/tum/cit/aet/artemis/core/util/Tuple.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package de.tum.cit.aet.artemis.core.util;

import java.io.Serializable;

/**
* Immutable tuple object.
*
* @param <X> first param.
* @param <Y> second param.
* @param <F> first param.
* @param <S> second param.
*/
public record Tuple<X, Y>(X x, Y y) {
public record Tuple<F, S>(F first, S second) implements Serializable {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're already changing this i'd suggest renaming it to Pair to make it consistent with conventions of other programming languages. A pair has inherently a first and second member. Tuples should support arbitrarily many members.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package de.tum.cit.aet.artemis.core.web.admin;

import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE;
import static tech.jhipster.web.util.PaginationUtil.generatePaginationHttpHeaders;

import java.util.List;

import org.springframework.context.annotation.Profile;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.servlet.support.ServletUriComponentsBuilder;

import de.tum.cit.aet.artemis.core.security.annotations.EnforceAdmin;
import de.tum.cit.aet.artemis.core.service.ScheduleService;

/**
* REST controller for getting the audit events.
*/
@Profile(PROFILE_CORE)
@EnforceAdmin
@RestController
@RequestMapping("api/admin/")
public class AdminScheduleResource {

private final ScheduleService scheduleService;

public AdminScheduleResource(ScheduleService scheduleService) {
this.scheduleService = scheduleService;
}

/**
* GET /exercise-schedules : get a page of scheduled events.
*
* @param pageable the pagination information
* @return the ResponseEntity with status 200 (OK) and the list of ScheduledExerciseEvents in body
*/
@GetMapping("exercise-schedules")
public ResponseEntity<List<ScheduleService.ScheduledExerciseEvent>> getAll(Pageable pageable) {
Page<ScheduleService.ScheduledExerciseEvent> page = scheduleService.findAllExerciseEvents(pageable);
HttpHeaders headers = generatePaginationHttpHeaders(ServletUriComponentsBuilder.fromCurrentRequest(), page);
return new ResponseEntity<>(page.getContent(), headers, HttpStatus.OK);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@
import de.tum.cit.aet.artemis.quiz.domain.QuizExercise;

public enum ExerciseLifecycle implements IExerciseLifecycle {
SHORTLY_BEFORE_RELEASE {

@Override
public ZonedDateTime getDateFromExercise(Exercise exercise) {
return exercise.getReleaseDate() != null ? exercise.getReleaseDate().minusSeconds(15) : null;
}

@Override
public ZonedDateTime getDateFromQuizBatch(QuizBatch quizBatch, QuizExercise quizExercise) {
return getDateFromExercise(quizExercise);
}
},

RELEASE {

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public ExerciseLifecycleService(@Qualifier("taskScheduler") TaskScheduler schedu
*/
public ScheduledFuture<?> scheduleTask(Exercise exercise, ZonedDateTime lifecycleDate, ExerciseLifecycle lifecycle, Runnable task) {
final ScheduledFuture<?> future = scheduler.schedule(task, lifecycleDate.toInstant());
log.debug("Scheduled Task for Exercise \"{}\" (#{}) to trigger on {}.", exercise.getTitle(), exercise.getId(), lifecycle);
log.debug("Scheduled Task for Exercise \"{}\" (#{}) to trigger on {} - {}", exercise.getTitle(), exercise.getId(), lifecycle, lifecycleDate);
return future;
}

Expand Down Expand Up @@ -101,7 +101,7 @@ public ScheduledFuture<?> scheduleTask(QuizExercise exercise, QuizBatch batch, E
public Set<ScheduledFuture<?>> scheduleMultipleTasks(Exercise exercise, ExerciseLifecycle lifecycle, Set<Tuple<ZonedDateTime, Runnable>> tasks) {
final Set<ScheduledFuture<?>> futures = new HashSet<>();
for (var task : tasks) {
var future = scheduler.schedule(task.y(), task.x().toInstant());
var future = scheduler.schedule(task.second(), task.first().toInstant());
futures.add(future);
}
log.debug("Scheduled {} Tasks for Exercise \"{}\" (#{}) to trigger on {}.", tasks.size(), exercise.getTitle(), exercise.getId(), lifecycle.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ public ResponseEntity<Participation> startParticipation(@PathVariable Long exerc
// 2) create a scheduled lock operation (see ProgrammingExerciseScheduleService)
// var task = programmingExerciseScheduleService.lockStudentRepository(participation);
// 3) add the task to the schedule service
// scheduleService.scheduleTask(exercise, ExerciseLifecycle.DUE, task);
// scheduleService.scheduleExerciseTask(exercise, ExerciseLifecycle.DUE, task);
}

// remove sensitive information before sending participation to the client
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ private void scheduleCourseExercise(ModelingExercise exercise) {

// For any course exercise that needsToBeScheduled (buildAndTestAfterDueDate and/or manual assessment)
if (exercise.getDueDate() != null && ZonedDateTime.now().isBefore(exercise.getDueDate())) {
scheduleService.scheduleTask(exercise, ExerciseLifecycle.DUE, () -> buildModelingClusters(exercise).run());
scheduleService.scheduleExerciseTask(exercise, ExerciseLifecycle.DUE, () -> buildModelingClusters(exercise).run(), "build modeling clusters after due date");
log.debug("Scheduled build modeling clusters after due date for Modeling Exercise '{}' (#{}) for {}.", exercise.getTitle(), exercise.getId(), exercise.getDueDate());
}
else {
Expand All @@ -160,7 +160,8 @@ private void scheduleExamExercise(ModelingExercise exercise) {
if (ZonedDateTime.now().isBefore(examDateService.getLatestIndividualExamEndDateWithGracePeriod(exam))) {
var buildDate = endDate.plusMinutes(EXAM_END_WAIT_TIME_FOR_COMPASS_MINUTES);
exercise.setClusterBuildDate(buildDate);
scheduleService.scheduleTask(exercise, ExerciseLifecycle.BUILD_COMPASS_CLUSTERS_AFTER_EXAM, () -> buildModelingClusters(exercise).run());
scheduleService.scheduleExerciseTask(exercise, ExerciseLifecycle.BUILD_COMPASS_CLUSTERS_AFTER_EXAM, () -> buildModelingClusters(exercise).run(),
"build modeling clusters after exam");
}
log.debug("Scheduled Exam Modeling Exercise '{}' (#{}).", exercise.getTitle(), exercise.getId());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,10 +320,10 @@ public Repository getOrCheckoutRepository(VcsRepositoryUri repoUri, boolean pull
return getOrCheckoutRepository(repoUri, repoUri, localPath, pullOnGet, defaultBranch);
}

public Repository getOrCheckoutRepositoryIntoTargetDirectory(VcsRepositoryUri repoUri, VcsRepositoryUri targetUrl, boolean pullOnGet)
public Repository getOrCheckoutRepositoryIntoTargetDirectory(VcsRepositoryUri repoUri, VcsRepositoryUri targetUri, boolean pullOnGet)
throws GitAPIException, GitException, InvalidPathException {
Path localPath = getDefaultLocalPathOfRepo(targetUrl);
return getOrCheckoutRepository(repoUri, targetUrl, localPath, pullOnGet);
Path localPath = getDefaultLocalPathOfRepo(targetUri);
return getOrCheckoutRepository(repoUri, targetUri, localPath, pullOnGet);
}

public Repository getOrCheckoutRepository(VcsRepositoryUri repoUri, Path localPath, boolean pullOnGet) throws GitAPIException, GitException, InvalidPathException {
Expand Down Expand Up @@ -1015,7 +1015,7 @@ public boolean accept(java.io.File directory, String fileName) {
}

/**
* Lists all files and directories within the given repository, excluding symbolic links.
* Returns all files and directories within the given repository in a map, excluding symbolic links.
* This method performs a file scan and filters out symbolic links.
* It supports bare and checked-out repositories.
* <p>
Expand All @@ -1026,7 +1026,7 @@ public boolean accept(java.io.File directory, String fileName) {
* @return A {@link Map} where each key is a {@link File} object representing a file or directory, and each value is
* the corresponding {@link FileType} (FILE or FOLDER). The map excludes symbolic links.
*/
public Map<File, FileType> listFilesAndFolders(Repository repo) {
public Map<File, FileType> getFilesAndFolders(Repository repo) {
FileAndDirectoryFilter filter = new FileAndDirectoryFilter();

Iterator<java.io.File> itr = FileUtils.iterateFilesAndDirs(repo.getLocalPath().toFile(), filter, filter);
Expand All @@ -1048,13 +1048,13 @@ public Map<File, FileType> listFilesAndFolders(Repository repo) {
}

/**
* List all files in the repository. In an empty git repo, this method returns 0.
* List all files in the repository. In an empty git repo, this method returns en empty list.
*
* @param repo Local Repository Object.
* @return Collection of File objects
*/
@NotNull
public Collection<File> listFiles(Repository repo) {
public Collection<File> getFiles(Repository repo) {
// Check if list of files is already cached
if (repo.getFiles() == null) {
FileAndDirectoryFilter filter = new FileAndDirectoryFilter();
Expand Down Expand Up @@ -1083,7 +1083,7 @@ public Optional<File> getFileByName(Repository repo, String filename) {
// Makes sure the requested file is part of the scanned list of files.
// Ensures that it is not possible to do bad things like filename="../../passwd"

for (File file : listFilesAndFolders(repo).keySet()) {
for (File file : getFilesAndFolders(repo).keySet()) {
if (file.toString().equals(filename)) {
return Optional.of(file);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ private void upgradeTemplateFiles(ProgrammingExercise exercise, RepositoryType r
String templatePomDir = repositoryType == RepositoryType.TESTS ? "test/maven/projectTemplate" : repositoryType.getName();
Resource[] templatePoms = getTemplateResources(exercise, templatePomDir + "/**/" + POM_FILE);
Repository repository = gitService.getOrCheckoutRepository(exercise.getRepositoryURL(repositoryType), true);
List<File> repositoryPoms = gitService.listFiles(repository).stream().filter(file -> Objects.equals(file.getName(), POM_FILE)).toList();
List<File> repositoryPoms = gitService.getFiles(repository).stream().filter(file -> Objects.equals(file.getName(), POM_FILE)).toList();

// Validate that template and repository have the same number of pom.xml files, otherwise no upgrade will take place
if (templatePoms.length == 1 && repositoryPoms.size() == 1) {
Expand Down Expand Up @@ -272,7 +272,7 @@ private Predicate<Plugin> isPlugin(String groupId, String artifactId) {
}

private Optional<File> getFileByName(Repository repository, String filename) {
return gitService.listFilesAndFolders(repository).keySet().stream().filter(file -> Objects.equals(filename, file.getName())).findFirst();
return gitService.getFilesAndFolders(repository).keySet().stream().filter(file -> Objects.equals(filename, file.getName())).findFirst();
}

private Optional<Resource> getFileByName(Resource[] resources, String filename) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ private void removeAuxiliaryRepository(AuxiliaryRepository auxiliaryRepository)
private void setupTemplateAndPush(final RepositoryResources repositoryResources, final String templateName, final ProgrammingExercise programmingExercise, final User user)
throws IOException, GitAPIException {
// Only copy template if repo is empty
if (!gitService.listFiles(repositoryResources.repository).isEmpty()) {
if (!gitService.getFiles(repositoryResources.repository).isEmpty()) {
return;
}

Expand Down Expand Up @@ -358,7 +358,7 @@ private static Path getRepoAbsoluteLocalPath(final Repository repository) {
*/
private void setupTestTemplateAndPush(final RepositoryResources resources, final ProgrammingExercise programmingExercise, final User user) throws IOException, GitAPIException {
// Only copy template if repo is empty
if (gitService.listFiles(resources.repository).isEmpty()
if (gitService.getFiles(resources.repository).isEmpty()
&& (programmingExercise.getProgrammingLanguage() == ProgrammingLanguage.JAVA || programmingExercise.getProgrammingLanguage() == ProgrammingLanguage.KOTLIN)) {
setupJVMTestTemplateAndPush(resources, programmingExercise, user);
}
Expand Down
Loading
Loading