diff --git a/docs/dev/guidelines/server.rst b/docs/dev/guidelines/server.rst index a4e510f362b0..7443a5f7a4f5 100644 --- a/docs/dev/guidelines/server.rst +++ b/docs/dev/guidelines/server.rst @@ -572,3 +572,61 @@ The course repository call takes care of throwing a ``404 Not Found`` exception ========================================== Always use ObjectMapper (Jackson) and do not use other libraries. If you find code that relies on gson, please consider to migrate it to use ObjectMapper! + +23. Use of Optionals in (non-)REST contexts +=========================================== + +We want to use Optional for method parameters in REST Controllers when it makes sense (in particular for non required request params). +We do not want to use it for method parameters anywhere else, in particular not for Services. + +``Optional`` is not meant to be used in these contexts, as it won't buy us anything: + + - in the domain model layer (not serializable) + - in DTOs (same reason) + - in input parameters of methods + - in constructor parameters + +There are two exceptions to this rule: + + - Optional services (e.g. due to specific profiles being or not being active, e.g. Optional versionControlService in ProgrammingSubmissionService) + - Input parameters of methods for RestControllers, e.g. @RequestParam(required = false) Optional lectureId in LectureResource + +Example: + +This is ok: + +.. code-block:: java + + class ProgrammingExerciseRepositoryService { + + // ... + + Optional versionControlService; + + // ... + } + +The use of Optional for the type of a field (and parameter in methods) is allowed here, since the ``VersionControlService`` is either fully enabled or disabled. + + +Do **not** write: + +.. code-block:: java + + @GetMapping(value = "complaints", params = { "courseId", "complaintType" }) + @EnforceAtLeastTutor + public ResponseEntity> getComplaintsByCourseId(@RequestParam Long courseId, @RequestParam ComplaintType complaintType, @RequestParam(required = "false") Long tutorId, @RequestParam(defaultValue = "false") boolean allComplaintsForTutor) { + // ... + } + +Do write: + +.. code-block:: java + + @GetMapping(value = "complaints", params = { "courseId", "complaintType" }) + @EnforceAtLeastTutor + public ResponseEntity> getComplaintsByCourseId(@RequestParam Long courseId, @RequestParam ComplaintType complaintType, @RequestParam Optional tutorId, @RequestParam(defaultValue = "false") boolean allComplaintsForTutor) { + // ... + } + +Note that not setting ``required`` will result in it being set to true automatically. Setting ``required`` to false is allowed only if you either make the type ``Optional`` or you provide a default value. diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/web/BonusResource.java b/src/main/java/de/tum/cit/aet/artemis/assessment/web/BonusResource.java index 8318cd961687..48680ed89c5d 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/web/BonusResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/web/BonusResource.java @@ -94,7 +94,7 @@ public BonusResource(BonusService bonusService, BonusRepository bonusRepository, */ @GetMapping("courses/{courseId}/exams/{examId}/bonus") @EnforceAtLeastStudent - public ResponseEntity getBonusForExam(@PathVariable Long courseId, @PathVariable Long examId, @RequestParam(required = false) boolean includeSourceGradeSteps) { + public ResponseEntity getBonusForExam(@PathVariable Long courseId, @PathVariable Long examId, @RequestParam(defaultValue = "false") boolean includeSourceGradeSteps) { log.debug("REST request to get bonus for exam: {}", examId); examAccessService.checkCourseAndExamAccessForStudentElseThrow(courseId, examId); diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/web/ComplaintResource.java b/src/main/java/de/tum/cit/aet/artemis/assessment/web/ComplaintResource.java index 5b7c0fd342cb..d884ef3d5ca3 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/web/ComplaintResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/web/ComplaintResource.java @@ -214,8 +214,8 @@ public ResponseEntity> getComplaintsForTestRunDashboard(Principa */ @GetMapping(value = "complaints", params = { "courseId", "complaintType" }) @EnforceAtLeastTutor - public ResponseEntity> getComplaintsByCourseId(@RequestParam Long courseId, @RequestParam ComplaintType complaintType, - @RequestParam(required = false) Long tutorId, @RequestParam(required = false) boolean allComplaintsForTutor) { + public ResponseEntity> getComplaintsByCourseId(@RequestParam Long courseId, @RequestParam ComplaintType complaintType, @RequestParam Optional tutorId, + @RequestParam(defaultValue = "false") boolean allComplaintsForTutor) { // Filtering by courseId Course course = courseRepository.findByIdElseThrow(courseId); @@ -224,11 +224,12 @@ public ResponseEntity> getComplaintsByCourseId(@RequestParam Lon boolean isAtLeastInstructor = authCheckService.isAtLeastInstructorInCourse(course, user); if (!isAtLeastInstructor) { - tutorId = user.getId(); + tutorId = Optional.of(user.getId()); } + List complaints; - if (tutorId == null) { + if (tutorId.isEmpty()) { complaints = complaintService.getAllComplaintsByCourseId(courseId); filterOutUselessDataFromComplaints(complaints, !isAtLeastInstructor); } @@ -239,7 +240,7 @@ else if (allComplaintsForTutor) { complaints.forEach(complaint -> complaint.filterForeignReviewer(user)); } else { - complaints = complaintService.getAllComplaintsByCourseIdAndTutorId(courseId, tutorId); + complaints = complaintService.getAllComplaintsByCourseIdAndTutorId(courseId, tutorId.get()); filterOutUselessDataFromComplaints(complaints, !isAtLeastInstructor); } @@ -257,7 +258,7 @@ else if (allComplaintsForTutor) { @GetMapping(value = "complaints", params = { "exerciseId", "complaintType" }) @EnforceAtLeastTutor public ResponseEntity> getComplaintsByExerciseId(@RequestParam Long exerciseId, @RequestParam ComplaintType complaintType, - @RequestParam(required = false) Long tutorId) { + @RequestParam Optional tutorId) { // Filtering by exerciseId Exercise exercise = exerciseRepository.findByIdElseThrow(exerciseId); User user = userRepository.getUserWithGroupsAndAuthorities(); @@ -267,19 +268,13 @@ public ResponseEntity> getComplaintsByExerciseId(@RequestParam L // Only instructors can access all complaints about an exercise without filtering by tutorId if (!isAtLeastInstructor) { - tutorId = userRepository.getUser().getId(); + tutorId = Optional.of(userRepository.getUser().getId()); } - List complaints; + List complaints = tutorId.map(id -> complaintService.getAllComplaintsByExerciseIdAndTutorId(exerciseId, id)) + .orElseGet(() -> complaintService.getAllComplaintsByExerciseId(exerciseId)); - if (tutorId == null) { - complaints = complaintService.getAllComplaintsByExerciseId(exerciseId); - filterOutUselessDataFromComplaints(complaints, !isAtLeastInstructor); - } - else { - complaints = complaintService.getAllComplaintsByExerciseIdAndTutorId(exerciseId, tutorId); - filterOutUselessDataFromComplaints(complaints, !isAtLeastInstructor); - } + filterOutUselessDataFromComplaints(complaints, !isAtLeastInstructor); return ResponseEntity.ok(getComplaintsByComplaintType(complaints, complaintType)); } diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/service/notifications/GroupNotificationScheduleService.java b/src/main/java/de/tum/cit/aet/artemis/communication/service/notifications/GroupNotificationScheduleService.java index 153baf29df8c..9f232fba90ae 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/service/notifications/GroupNotificationScheduleService.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/service/notifications/GroupNotificationScheduleService.java @@ -3,6 +3,7 @@ import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE; import java.time.ZonedDateTime; +import java.util.Optional; import org.springframework.context.annotation.Profile; import org.springframework.scheduling.annotation.Async; @@ -37,7 +38,7 @@ public GroupNotificationScheduleService(SingleUserNotificationService singleUser * @param exerciseAfterUpdate is the updated exercise (needed to check potential difference in release date) * @param notificationText holds the custom change message for the notification process */ - public void checkAndCreateAppropriateNotificationsWhenUpdatingExercise(Exercise exerciseBeforeUpdate, Exercise exerciseAfterUpdate, String notificationText) { + public void checkAndCreateAppropriateNotificationsWhenUpdatingExercise(Exercise exerciseBeforeUpdate, Exercise exerciseAfterUpdate, Optional notificationText) { // send exercise update notification groupNotificationService.notifyAboutExerciseUpdate(exerciseAfterUpdate, notificationText); diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/service/notifications/GroupNotificationService.java b/src/main/java/de/tum/cit/aet/artemis/communication/service/notifications/GroupNotificationService.java index 6e48ec307044..037ecda51b50 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/service/notifications/GroupNotificationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/service/notifications/GroupNotificationService.java @@ -25,6 +25,7 @@ import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -77,7 +78,7 @@ public GroupNotificationService(GroupNotificationRepository groupNotificationRep * @param exercise that is updated * @param notificationText that is used for the notification process */ - public void notifyAboutExerciseUpdate(Exercise exercise, String notificationText) { + public void notifyAboutExerciseUpdate(Exercise exercise, Optional notificationText) { if (exercise.isExamExercise()) { // Do not send an exercise-update notification if it's an exam exercise. @@ -90,10 +91,8 @@ public void notifyAboutExerciseUpdate(Exercise exercise, String notificationText return; } - if (notificationText != null) { - // sends an exercise-update notification - notifyStudentAndEditorAndInstructorGroupAboutExerciseUpdate(exercise, notificationText); - } + // sends an exercise-update notification + notificationText.ifPresent(text -> notifyStudentAndEditorAndInstructorGroupAboutExerciseUpdate(exercise, text)); } /** diff --git a/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ConversationResource.java b/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ConversationResource.java index 8ea63e4ce3f6..1fd0b9317612 100644 --- a/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ConversationResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/communication/web/conversation/ConversationResource.java @@ -249,7 +249,7 @@ public ResponseEntity> getResponsibleUsersForCodeOfCond @GetMapping("{courseId}/conversations/{conversationId}/members/search") @EnforceAtLeastStudent public ResponseEntity> searchMembersOfConversation(@PathVariable Long courseId, @PathVariable Long conversationId, - @RequestParam("loginOrName") String loginOrName, @RequestParam(value = "filter", required = false) ConversationMemberSearchFilters filter, Pageable pageable) { + @RequestParam("loginOrName") String loginOrName, @RequestParam(value = "filter") Optional filter, Pageable pageable) { log.debug("REST request to get members of conversation : {} with login or name : {} in course: {}", conversationId, loginOrName, courseId); if (pageable.getPageSize() > 20) { throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "The page size must not be greater than 20"); @@ -269,7 +269,7 @@ public ResponseEntity> searchMembersOfConversation(@Pa } } var searchTerm = loginOrName != null ? loginOrName.toLowerCase().trim() : ""; - var originalPage = conversationService.searchMembersOfConversation(course, conversationFromDatabase, pageable, searchTerm, Optional.ofNullable(filter)); + var originalPage = conversationService.searchMembersOfConversation(course, conversationFromDatabase, pageable, searchTerm, filter); var resultDTO = new ArrayList(); for (var user : originalPage) { diff --git a/src/main/java/de/tum/cit/aet/artemis/core/service/AuthorizationCheckService.java b/src/main/java/de/tum/cit/aet/artemis/core/service/AuthorizationCheckService.java index d75d3a5668bf..471c12dd2c3e 100644 --- a/src/main/java/de/tum/cit/aet/artemis/core/service/AuthorizationCheckService.java +++ b/src/main/java/de/tum/cit/aet/artemis/core/service/AuthorizationCheckService.java @@ -904,8 +904,8 @@ public boolean isAllowedToGetExamResult(Exercise exercise, StudentParticipation * @return true if caller is allowed to assess submissions */ @CheckReturnValue - public boolean isAllowedToAssessExercise(Exercise exercise, User user, Long resultId) { - return this.isAtLeastTeachingAssistantForExercise(exercise, user) && (resultId == null || isAtLeastInstructorForExercise(exercise, user)); + public boolean isAllowedToAssessExercise(Exercise exercise, User user, Optional resultId) { + return this.isAtLeastTeachingAssistantForExercise(exercise, user) && (resultId.isEmpty() || isAtLeastInstructorForExercise(exercise, user)); } public void checkGivenExerciseIdSameForExerciseInRequestBodyElseThrow(Long exerciseId, Exercise exerciseInRequestBody) { @@ -914,7 +914,7 @@ public void checkGivenExerciseIdSameForExerciseInRequestBodyElseThrow(Long exerc } } - public void checkIsAllowedToAssessExerciseElseThrow(Exercise exercise, User user, Long resultId) { + public void checkIsAllowedToAssessExerciseElseThrow(Exercise exercise, User user, Optional resultId) { if (!isAllowedToAssessExercise(exercise, user, resultId)) { throw new AccessForbiddenException("You are not allowed to assess this exercise!"); } diff --git a/src/main/java/de/tum/cit/aet/artemis/exam/service/ExamLiveEventsService.java b/src/main/java/de/tum/cit/aet/artemis/exam/service/ExamLiveEventsService.java index fa48e0c1a09c..09a3c8498f50 100644 --- a/src/main/java/de/tum/cit/aet/artemis/exam/service/ExamLiveEventsService.java +++ b/src/main/java/de/tum/cit/aet/artemis/exam/service/ExamLiveEventsService.java @@ -2,6 +2,8 @@ import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE; +import java.util.Optional; + import org.springframework.context.annotation.Profile; import org.springframework.scheduling.annotation.Async; import org.springframework.stereotype.Service; @@ -141,7 +143,7 @@ public void createAndSendWorkingTimeUpdateEvent(StudentExam studentExam, int new * @param instructor The user who performed the update */ @Async - public void createAndSendProblemStatementUpdateEvent(Exercise exercise, String message, User instructor) { + public void createAndSendProblemStatementUpdateEvent(Exercise exercise, Optional message, User instructor) { Exam exam = exercise.getExam(); studentExamRepository.findAllWithExercisesByExamId(exam.getId()).stream().filter(studentExam -> studentExam.getExercises().contains(exercise)) .forEach(studentExam -> this.createAndSendProblemStatementUpdateEvent(studentExam, exercise, message, instructor)); @@ -149,13 +151,14 @@ public void createAndSendProblemStatementUpdateEvent(Exercise exercise, String m /** * Send a problem statement update to the specified student. + * If the provided message is {@link Optional::empty}, the text content of the update will be set to {@link null} * * @param studentExam The student exam containing the exercise with updated problem statement * @param exercise The updated exercise * @param message The message to send * @param sentBy The user who performed the update */ - public void createAndSendProblemStatementUpdateEvent(StudentExam studentExam, Exercise exercise, String message, User sentBy) { + public void createAndSendProblemStatementUpdateEvent(StudentExam studentExam, Exercise exercise, Optional message, User sentBy) { var event = new ProblemStatementUpdateEvent(); // Common fields @@ -164,7 +167,7 @@ public void createAndSendProblemStatementUpdateEvent(StudentExam studentExam, Ex event.setCreatedBy(sentBy.getName()); // Specific fields - event.setTextContent(message); + event.setTextContent(message.orElse(null)); event.setProblemStatement(exercise.getProblemStatement()); event.setExerciseId(exercise.getId()); event.setExerciseName(exercise.getExerciseGroup().getTitle()); diff --git a/src/main/java/de/tum/cit/aet/artemis/exam/web/ExamResource.java b/src/main/java/de/tum/cit/aet/artemis/exam/web/ExamResource.java index 8223ba8e54a9..e882c20d801e 100644 --- a/src/main/java/de/tum/cit/aet/artemis/exam/web/ExamResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/exam/web/ExamResource.java @@ -1307,20 +1307,17 @@ public ResponseEntity> getAllSuspiciousExamSessio @RequestParam("differentStudentExamsSameBrowserFingerprint") boolean analyzeSessionsWithTheSameBrowserFingerprint, @RequestParam("sameStudentExamDifferentIPAddresses") boolean analyzeSessionsForTheSameStudentExamWithDifferentIpAddresses, @RequestParam("sameStudentExamDifferentBrowserFingerprints") boolean analyzeSessionsForTheSameStudentExamWithDifferentBrowserFingerprints, - @RequestParam("ipOutsideOfRange") boolean analyzeSessionsIpOutsideOfRange, @RequestParam(required = false) String ipSubnet) { + @RequestParam("ipOutsideOfRange") boolean analyzeSessionsIpOutsideOfRange, @RequestParam Optional ipSubnet) { log.debug("REST request to get all exam sessions that are suspicious for exam : {}", examId); Course course = courseRepository.findByIdElseThrow(courseId); authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, course, null); - if (analyzeSessionsIpOutsideOfRange) { - if (ipSubnet == null) { - throw new BadRequestAlertException("If you want to analyze sessions with IP outside of range, you need to provide a subnet", ENTITY_NAME, - "missingLowerOrUpperBoundIp"); - } + if (analyzeSessionsIpOutsideOfRange && ipSubnet.isEmpty()) { + throw new BadRequestAlertException("If you want to analyze sessions with IP outside of range, you need to provide a subnet", ENTITY_NAME, "missingLowerOrUpperBoundIp"); } SuspiciousSessionsAnalysisOptions options = new SuspiciousSessionsAnalysisOptions(analyzeSessionsWithTheSameIp, analyzeSessionsWithTheSameBrowserFingerprint, analyzeSessionsForTheSameStudentExamWithDifferentIpAddresses, analyzeSessionsForTheSameStudentExamWithDifferentBrowserFingerprints, analyzeSessionsIpOutsideOfRange); - return ResponseEntity.ok(examSessionService.retrieveAllSuspiciousExamSessionsByExamId(examId, options, Optional.ofNullable(ipSubnet))); + return ResponseEntity.ok(examSessionService.retrieveAllSuspiciousExamSessionsByExamId(examId, options, ipSubnet)); } /** diff --git a/src/main/java/de/tum/cit/aet/artemis/exam/web/StudentExamResource.java b/src/main/java/de/tum/cit/aet/artemis/exam/web/StudentExamResource.java index 181b6b8205b1..ba9909a446f5 100644 --- a/src/main/java/de/tum/cit/aet/artemis/exam/web/StudentExamResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/exam/web/StudentExamResource.java @@ -9,6 +9,7 @@ import java.time.temporal.ChronoUnit; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -528,11 +529,11 @@ public ResponseEntity getStudentExamForSummary(@PathVariable Long c @GetMapping("courses/{courseId}/exams/{examId}/student-exams/{studentExamId}/grade-summary") @EnforceAtLeastStudent public ResponseEntity getStudentExamGradesForSummary(@PathVariable long courseId, @PathVariable long examId, @PathVariable long studentExamId, - @RequestParam(required = false) Long userId) { + @RequestParam Optional userId) { long start = System.currentTimeMillis(); User currentUser = userRepository.getUserWithGroupsAndAuthorities(); log.debug("REST request to get the student exam grades of user with id {} for exam {} by user {}", userId, examId, currentUser.getLogin()); - User targetUser = userId == null ? currentUser : userRepository.findByIdWithGroupsAndAuthoritiesElseThrow(userId); + User targetUser = userId.map(userRepository::findByIdWithGroupsAndAuthoritiesElseThrow).orElse(currentUser); StudentExam studentExam = findStudentExamWithExercisesElseThrow(targetUser, examId, courseId, studentExamId); boolean isAtLeastInstructor = authorizationCheckService.isAtLeastInstructorInCourse(studentExam.getExam().getCourse(), currentUser); diff --git a/src/main/java/de/tum/cit/aet/artemis/exercise/domain/Submission.java b/src/main/java/de/tum/cit/aet/artemis/exercise/domain/Submission.java index 304c206938b8..ca8ec0984e80 100644 --- a/src/main/java/de/tum/cit/aet/artemis/exercise/domain/Submission.java +++ b/src/main/java/de/tum/cit/aet/artemis/exercise/domain/Submission.java @@ -6,6 +6,7 @@ import java.util.HashSet; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -359,8 +360,8 @@ public void setExampleSubmission(Boolean exampleSubmission) { * @param correctionRound for which not to remove results * @param resultId specific resultId */ - public void removeNotNeededResults(int correctionRound, Long resultId) { - if (correctionRound == 0 && resultId == null && getResults().size() >= 2) { + public void removeNotNeededResults(int correctionRound, Optional resultId) { + if (correctionRound == 0 && resultId.isEmpty() && getResults().size() >= 2) { var resultList = new ArrayList(); resultList.add(getFirstManualResult()); setResults(resultList); diff --git a/src/main/java/de/tum/cit/aet/artemis/exercise/repository/TeamRepository.java b/src/main/java/de/tum/cit/aet/artemis/exercise/repository/TeamRepository.java index b6a17585eab3..cabe7de55a85 100644 --- a/src/main/java/de/tum/cit/aet/artemis/exercise/repository/TeamRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/exercise/repository/TeamRepository.java @@ -117,13 +117,9 @@ SELECT COUNT(DISTINCT team) * @param teamOwnerId Optional user id by which to filter teams on their owner * @return List of teams */ - default List findAllByExerciseIdWithEagerStudents(Exercise exercise, Long teamOwnerId) { - if (teamOwnerId != null) { - return findAllByExerciseIdAndTeamOwnerIdWithEagerStudents(exercise.getId(), teamOwnerId); - } - else { - return findAllByExerciseIdWithEagerStudents(exercise.getId()); - } + default List findAllByExerciseIdWithEagerStudents(Exercise exercise, Optional teamOwnerId) { + return teamOwnerId.map(id -> findAllByExerciseIdAndTeamOwnerIdWithEagerStudents(exercise.getId(), id)) + .orElseGet(() -> findAllByExerciseIdWithEagerStudents(exercise.getId())); } /** diff --git a/src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java b/src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java index 816d2bfdb558..496b39474c8e 100644 --- a/src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java +++ b/src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java @@ -777,7 +777,7 @@ public void removeCompetency(@NotNull Set exercises, @NotNull CourseCo * @param updatedExercise the updated exercise * @param notificationText custom notification text */ - public void notifyAboutExerciseChanges(Exercise originalExercise, Exercise updatedExercise, String notificationText) { + public void notifyAboutExerciseChanges(Exercise originalExercise, Exercise updatedExercise, Optional notificationText) { if (originalExercise.isCourseExercise()) { groupNotificationScheduleService.checkAndCreateAppropriateNotificationsWhenUpdatingExercise(originalExercise, updatedExercise, notificationText); } diff --git a/src/main/java/de/tum/cit/aet/artemis/exercise/web/TeamResource.java b/src/main/java/de/tum/cit/aet/artemis/exercise/web/TeamResource.java index e33719fa821a..07e07fcabebd 100644 --- a/src/main/java/de/tum/cit/aet/artemis/exercise/web/TeamResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/exercise/web/TeamResource.java @@ -269,7 +269,7 @@ public ResponseEntity getTeam(@PathVariable long exerciseId, @PathVariable */ @GetMapping("exercises/{exerciseId}/teams") @EnforceAtLeastTutor - public ResponseEntity> getTeamsForExercise(@PathVariable long exerciseId, @RequestParam(value = "teamOwnerId", required = false) Long teamOwnerId) { + public ResponseEntity> getTeamsForExercise(@PathVariable long exerciseId, @RequestParam(value = "teamOwnerId") Optional teamOwnerId) { log.debug("REST request to get all Teams for the exercise with id : {}", exerciseId); Exercise exercise = exerciseRepository.findByIdElseThrow(exerciseId); authCheckService.checkHasAtLeastRoleForExerciseElseThrow(Role.TEACHING_ASSISTANT, exercise, null); diff --git a/src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadExerciseResource.java b/src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadExerciseResource.java index 84e361d2b4fe..4342543ba251 100644 --- a/src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadExerciseResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadExerciseResource.java @@ -259,7 +259,7 @@ public ResponseEntity> getAllExercisesOn @PutMapping("file-upload-exercises/{exerciseId}") @EnforceAtLeastEditor public ResponseEntity updateFileUploadExercise(@RequestBody FileUploadExercise fileUploadExercise, - @RequestParam(value = "notificationText", required = false) String notificationText, @PathVariable Long exerciseId) { + @RequestParam(value = "notificationText") Optional notificationText, @PathVariable Long exerciseId) { log.debug("REST request to update FileUploadExercise : {}", fileUploadExercise); // TODO: The route has an exerciseId but we don't do anything useful with it. Change route and client requests? @@ -403,7 +403,7 @@ public ResponseEntity exportSubmissions(@PathVariable long exerciseId, @PutMapping("file-upload-exercises/{exerciseId}/re-evaluate") @EnforceAtLeastEditor public ResponseEntity reEvaluateAndUpdateFileUploadExercise(@PathVariable long exerciseId, @RequestBody FileUploadExercise fileUploadExercise, - @RequestParam(value = "deleteFeedback", required = false) Boolean deleteFeedbackAfterGradingInstructionUpdate) { + @RequestParam(value = "deleteFeedback", defaultValue = "false") boolean deleteFeedbackAfterGradingInstructionUpdate) { log.debug("REST request to re-evaluate FileUploadExercise : {}", fileUploadExercise); // check that the exercise exists for given id @@ -415,6 +415,6 @@ public ResponseEntity reEvaluateAndUpdateFileUploadExercise( // Check that the user is authorized to update the exercise authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.EDITOR, course, null); exerciseService.reEvaluateExercise(fileUploadExercise, deleteFeedbackAfterGradingInstructionUpdate); - return updateFileUploadExercise(fileUploadExercise, null, fileUploadExercise.getId()); + return updateFileUploadExercise(fileUploadExercise, Optional.empty(), fileUploadExercise.getId()); } } diff --git a/src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.java b/src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.java index 0d7c4a23280f..f1839a5c92b3 100644 --- a/src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.java @@ -182,7 +182,7 @@ private static void checkFilePattern(MultipartFile file, FileUploadExercise exer @GetMapping("file-upload-submissions/{submissionId}") @EnforceAtLeastTutor public ResponseEntity getFileUploadSubmission(@PathVariable Long submissionId, - @RequestParam(value = "correction-round", defaultValue = "0") int correctionRound, @RequestParam(value = "resultId", required = false) Long resultId) { + @RequestParam(value = "correction-round", defaultValue = "0") int correctionRound, @RequestParam(value = "resultId") Optional resultId) { log.debug("REST request to get FileUploadSubmission with id: {}", submissionId); var fileUploadSubmission = fileUploadSubmissionRepository.findByIdElseThrow(submissionId); var studentParticipation = (StudentParticipation) fileUploadSubmission.getParticipation(); @@ -192,11 +192,12 @@ public ResponseEntity getFileUploadSubmission(@PathVariabl authCheckService.checkIsAllowedToAssessExerciseElseThrow(fileUploadExercise, user, resultId); // load submission with results either by resultId or by correctionRound - if (resultId != null) { + + if (resultId.isPresent()) { // load the submission with additional needed properties by resultId fileUploadSubmission = (FileUploadSubmission) submissionRepository.findOneWithEagerResultAndFeedbackAndAssessmentNote(submissionId); // check if result with the requested id exists - Result result = fileUploadSubmission.getManualResultsById(resultId); + Result result = fileUploadSubmission.getManualResultsById(resultId.get()); if (result == null) { return ResponseEntity.badRequest() .headers(HeaderUtil.createFailureAlert(applicationName, true, "FileUploadSubmission", "ResultNotFound", "No Result was found for the given ID.")) diff --git a/src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentResource.java b/src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentResource.java index 3b5e44b6ac2f..d5094e878cd1 100644 --- a/src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentResource.java @@ -113,7 +113,7 @@ public ResponseEntity createAttachment(@RequestPart Attachment attac @PutMapping(value = "attachments/{attachmentId}", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) @EnforceAtLeastEditor public ResponseEntity updateAttachment(@PathVariable Long attachmentId, @RequestPart Attachment attachment, @RequestPart(required = false) MultipartFile file, - @RequestParam(value = "notificationText", required = false) String notificationText) { + @RequestParam(value = "notificationText") Optional notificationText) { log.debug("REST request to update Attachment : {}", attachment); attachment.setId(attachmentId); @@ -132,9 +132,7 @@ public ResponseEntity updateAttachment(@PathVariable Long attachment } Attachment result = attachmentRepository.save(attachment); - if (notificationText != null) { - groupNotificationService.notifyStudentGroupAboutAttachmentChange(result, notificationText); - } + notificationText.ifPresent(text -> groupNotificationService.notifyStudentGroupAboutAttachmentChange(result, text)); return ResponseEntity.ok(result); } diff --git a/src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentUnitResource.java b/src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentUnitResource.java index 0c7c2996e162..031600bf683d 100644 --- a/src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentUnitResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentUnitResource.java @@ -9,6 +9,7 @@ import java.nio.file.Path; import java.util.List; import java.util.Objects; +import java.util.Optional; import org.apache.commons.io.FilenameUtils; import org.slf4j.Logger; @@ -123,7 +124,7 @@ public ResponseEntity getAttachmentUnit(@PathVariable Long attac @EnforceAtLeastEditor public ResponseEntity updateAttachmentUnit(@PathVariable Long lectureId, @PathVariable Long attachmentUnitId, @RequestPart AttachmentUnit attachmentUnit, @RequestPart Attachment attachment, @RequestPart(required = false) MultipartFile file, @RequestParam(defaultValue = "false") boolean keepFilename, - @RequestParam(value = "notificationText", required = false) String notificationText) { + @RequestParam(value = "notificationText") Optional notificationText) { log.debug("REST request to update an attachment unit : {}", attachmentUnit); AttachmentUnit existingAttachmentUnit = attachmentUnitRepository.findOneWithSlidesAndCompetencies(attachmentUnitId); checkAttachmentUnitCourseAndLecture(existingAttachmentUnit, lectureId); @@ -131,9 +132,7 @@ public ResponseEntity updateAttachmentUnit(@PathVariable Long le AttachmentUnit savedAttachmentUnit = attachmentUnitService.updateAttachmentUnit(existingAttachmentUnit, attachmentUnit, attachment, file, keepFilename); - if (notificationText != null) { - groupNotificationService.notifyStudentGroupAboutAttachmentChange(savedAttachmentUnit.getAttachment(), notificationText); - } + notificationText.ifPresent(text -> groupNotificationService.notifyStudentGroupAboutAttachmentChange(savedAttachmentUnit.getAttachment(), text)); return ResponseEntity.ok(savedAttachmentUnit); } diff --git a/src/main/java/de/tum/cit/aet/artemis/lti/service/LtiDynamicRegistrationService.java b/src/main/java/de/tum/cit/aet/artemis/lti/service/LtiDynamicRegistrationService.java index bb4ba0abf16e..e50a80c8ac85 100644 --- a/src/main/java/de/tum/cit/aet/artemis/lti/service/LtiDynamicRegistrationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/lti/service/LtiDynamicRegistrationService.java @@ -2,6 +2,7 @@ import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_LTI; +import java.util.Optional; import java.util.UUID; import org.slf4j.Logger; @@ -50,7 +51,7 @@ public LtiDynamicRegistrationService(LtiPlatformConfigurationRepository ltiPlatf * @param openIdConfigurationUrl the url to get the configuration from * @param registrationToken the token to be used to authenticate the POST request */ - public void performDynamicRegistration(String openIdConfigurationUrl, String registrationToken) { + public void performDynamicRegistration(String openIdConfigurationUrl, Optional registrationToken) { try { // Get platform's configuration Lti13PlatformConfiguration platformConfiguration = getLti13PlatformConfiguration(openIdConfigurationUrl); @@ -93,12 +94,10 @@ private Lti13PlatformConfiguration getLti13PlatformConfiguration(String openIdCo return platformConfiguration; } - private Lti13ClientRegistration postClientRegistrationToPlatform(String registrationEndpoint, String clientRegistrationId, String registrationToken) { + private Lti13ClientRegistration postClientRegistrationToPlatform(String registrationEndpoint, String clientRegistrationId, Optional registrationToken) { HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.APPLICATION_JSON); - if (registrationToken != null) { - headers.setBearerAuth(registrationToken); - } + registrationToken.ifPresent(headers::setBearerAuth); Lti13ClientRegistration lti13ClientRegistration = new Lti13ClientRegistration(artemisServerUrl, clientRegistrationId); Lti13ClientRegistration registrationResponse = null; diff --git a/src/main/java/de/tum/cit/aet/artemis/lti/web/admin/AdminLtiConfigurationResource.java b/src/main/java/de/tum/cit/aet/artemis/lti/web/admin/AdminLtiConfigurationResource.java index 0b5d3f71c2f3..4e9f3e292daa 100644 --- a/src/main/java/de/tum/cit/aet/artemis/lti/web/admin/AdminLtiConfigurationResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/lti/web/admin/AdminLtiConfigurationResource.java @@ -2,6 +2,7 @@ import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_LTI; +import java.util.Optional; import java.util.UUID; import org.slf4j.Logger; @@ -145,7 +146,7 @@ public ResponseEntity addLtiPlatformConfiguration(@RequestBody LtiPlatform */ @PostMapping("lti13/dynamic-registration") public ResponseEntity lti13DynamicRegistration(@RequestParam(name = "openid_configuration") String openIdConfiguration, - @RequestParam(name = "registration_token", required = false) String registrationToken) { + @RequestParam(name = "registration_token") Optional registrationToken) { authCheckService.checkIsAdminElseThrow(null); ltiDynamicRegistrationService.performDynamicRegistration(openIdConfiguration, registrationToken); diff --git a/src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingExerciseResource.java b/src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingExerciseResource.java index da9195b214d5..f54b182790c4 100644 --- a/src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingExerciseResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingExerciseResource.java @@ -216,7 +216,7 @@ public ResponseEntity> getAllExercisesOnPa @PutMapping("modeling-exercises") @EnforceAtLeastEditor public ResponseEntity updateModelingExercise(@RequestBody ModelingExercise modelingExercise, - @RequestParam(value = "notificationText", required = false) String notificationText) throws URISyntaxException { + @RequestParam(value = "notificationText") Optional notificationText) throws URISyntaxException { log.debug("REST request to update ModelingExercise : {}", modelingExercise); if (modelingExercise.getId() == null) { return createModelingExercise(modelingExercise); @@ -458,7 +458,7 @@ public ResponseEntity> checkPlagia @PutMapping("modeling-exercises/{exerciseId}/re-evaluate") @EnforceAtLeastEditor public ResponseEntity reEvaluateAndUpdateModelingExercise(@PathVariable long exerciseId, @RequestBody ModelingExercise modelingExercise, - @RequestParam(value = "deleteFeedback", required = false) Boolean deleteFeedbackAfterGradingInstructionUpdate) throws URISyntaxException { + @RequestParam(value = "deleteFeedback", defaultValue = "false") boolean deleteFeedbackAfterGradingInstructionUpdate) throws URISyntaxException { log.debug("REST request to re-evaluate ModelingExercise : {}", modelingExercise); modelingExerciseRepository.findByIdWithStudentParticipationsSubmissionsResultsElseThrow(exerciseId); @@ -472,6 +472,6 @@ public ResponseEntity reEvaluateAndUpdateModelingExercise(@Pat exerciseService.reEvaluateExercise(modelingExercise, deleteFeedbackAfterGradingInstructionUpdate); - return updateModelingExercise(modelingExercise, null); + return updateModelingExercise(modelingExercise, Optional.empty()); } } diff --git a/src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java b/src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java index 501309aea8e8..fd3fde8ce0e4 100644 --- a/src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java @@ -190,7 +190,7 @@ public ResponseEntity> getAllModelingSubmissions(@PathVariable @GetMapping("modeling-submissions/{submissionId}") @EnforceAtLeastStudent public ResponseEntity getModelingSubmission(@PathVariable Long submissionId, - @RequestParam(value = "correction-round", defaultValue = "0") int correctionRound, @RequestParam(value = "resultId", required = false) Long resultId, + @RequestParam(value = "correction-round", defaultValue = "0") int correctionRound, @RequestParam(value = "resultId") Optional resultId, @RequestParam(value = "withoutResults", defaultValue = "false") boolean withoutResults) { log.debug("REST request to get ModelingSubmission with id: {}", submissionId); // TODO CZ: include exerciseId in path to get exercise for auth check more easily? @@ -210,11 +210,11 @@ public ResponseEntity getModelingSubmission(@PathVariable Lo if (!withoutResults) { // load submission with results either by resultId or by correctionRound - if (resultId != null) { + if (resultId.isPresent()) { // load the submission with additional needed properties modelingSubmission = (ModelingSubmission) submissionRepository.findOneWithEagerResultAndFeedbackAndAssessmentNote(submissionId); // check if result exists - Result result = modelingSubmission.getManualResultsById(resultId); + Result result = modelingSubmission.getManualResultsById(resultId.get()); if (result == null) { return ResponseEntity.badRequest() .headers(HeaderUtil.createFailureAlert(applicationName, true, "ModelingSubmission", "ResultNotFound", "No Result was found for the given ID.")) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java index 0cbf73da21ff..0c5cb9c06e53 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java @@ -26,8 +26,6 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; -import jakarta.annotation.Nullable; - import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.StringUtils; @@ -593,7 +591,7 @@ public static Path getProgrammingLanguageTemplatePath(ProgrammingLanguage progra * @return the updates programming exercise from the database */ public ProgrammingExercise updateProgrammingExercise(ProgrammingExercise programmingExerciseBeforeUpdate, ProgrammingExercise updatedProgrammingExercise, - @Nullable String notificationText) throws JsonProcessingException { + Optional notificationText) throws JsonProcessingException { setURLsForAuxiliaryRepositoriesOfExercise(updatedProgrammingExercise); connectAuxiliaryRepositoriesToExercise(updatedProgrammingExercise); @@ -689,7 +687,7 @@ public void initParticipations(ProgrammingExercise programmingExercise) { * @param notificationText optional text for a notification to all students about the update * @return the updated ProgrammingExercise object. */ - public ProgrammingExercise updateTimeline(ProgrammingExercise updatedProgrammingExercise, @Nullable String notificationText) { + public ProgrammingExercise updateTimeline(ProgrammingExercise updatedProgrammingExercise, Optional notificationText) { ProgrammingExercise programmingExercise = programmingExerciseRepository.findByIdElseThrow(updatedProgrammingExercise.getId()); // create slim copy of programmingExercise before the update - needed for notifications (only release date needed) @@ -722,7 +720,7 @@ public ProgrammingExercise updateTimeline(ProgrammingExercise updatedProgramming * @return the updated ProgrammingExercise object. * @throws EntityNotFoundException if there is no ProgrammingExercise for the given id. */ - public ProgrammingExercise updateProblemStatement(ProgrammingExercise programmingExercise, String problemStatement, @Nullable String notificationText) + public ProgrammingExercise updateProblemStatement(ProgrammingExercise programmingExercise, String problemStatement, Optional notificationText) throws EntityNotFoundException { String oldProblemStatement = programmingExercise.getProblemStatement(); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java index 9833a9ae040b..e077265b8a03 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java @@ -119,26 +119,22 @@ public Map getFiles(Repository repository) { * @throws IOException If an I/O error occurs during the file content retrieval process. This could be due to issues with file access, network problems, etc. * @throws GitAPIException If an error occurs while interacting with the Git repository. This could be due to issues with repository access, invalid commit ids, etc. */ - public Map getFilesContentAtCommit(ProgrammingExercise programmingExercise, String commitId, RepositoryType repositoryType, + public Map getFilesContentAtCommit(ProgrammingExercise programmingExercise, String commitId, Optional repositoryType, ProgrammingExerciseParticipation participation) throws IOException, GitAPIException { + + boolean isTestRepository = repositoryType.isPresent() && repositoryType.get() == RepositoryType.TESTS; + var repoUri = isTestRepository ? programmingExercise.getVcsTestRepositoryUri() : participation.getVcsRepositoryUri(); + // Check if local VCS is active if (profileService.isLocalVcsActive()) { log.debug("Using local VCS for getting files at commit {} for participation {}", commitId, participation.getId()); // If local VCS is active, operate directly on the bare repository - var repoUri = repositoryType == RepositoryType.TESTS ? programmingExercise.getVcsTestRepositoryUri() : participation.getVcsRepositoryUri(); Repository repository = gitService.getBareRepository(repoUri); return getFilesContentFromBareRepository(repository, commitId); } else { log.debug("Checking out repo to get files at commit {} for participation {}", commitId, participation.getId()); - Repository repository; - if (repositoryType == RepositoryType.TESTS) { - repository = gitService.checkoutRepositoryAtCommit(programmingExercise.getVcsTestRepositoryUri(), commitId, true); - } - else { - // For other repository types, check out the repository at the commit - repository = gitService.checkoutRepositoryAtCommit(participation.getVcsRepositoryUri(), commitId, true); - } + Repository repository = gitService.checkoutRepositoryAtCommit(repoUri, commitId, true); // Get the files content from the working copy of the repository Map filesWithContent = getFilesContentFromWorkingCopy(repository); // Switch back to the default branch head diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java b/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java index d06fdf5fb975..d9d0a178468c 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java @@ -253,8 +253,7 @@ public ResponseEntity>> getLatestPendi */ @PutMapping("programming-exercise-participations/{participationId}/reset-repository") @EnforceAtLeastStudent - public ResponseEntity resetRepository(@PathVariable Long participationId, @RequestParam(required = false) Long gradedParticipationId) - throws GitAPIException, IOException { + public ResponseEntity resetRepository(@PathVariable Long participationId, @RequestParam Optional gradedParticipationId) throws GitAPIException, IOException { ProgrammingExerciseStudentParticipation participation = programmingExerciseStudentParticipationRepository.findByIdElseThrow(participationId); ProgrammingExercise exercise = programmingExerciseRepository.findByStudentParticipationIdWithTemplateParticipation(participationId) .orElseThrow(() -> new EntityNotFoundException("Programming Exercise for Participation", participationId)); @@ -268,16 +267,12 @@ public ResponseEntity resetRepository(@PathVariable Long participationId, throw new BadRequestAlertException("Cannot reset repository in an exam", ENTITY_NAME, "noRepoResetInExam"); } - VcsRepositoryUri sourceURL; - if (gradedParticipationId != null) { - ProgrammingExerciseStudentParticipation gradedParticipation = programmingExerciseStudentParticipationRepository.findByIdElseThrow(gradedParticipationId); + VcsRepositoryUri sourceURL = gradedParticipationId.map(id -> { + ProgrammingExerciseStudentParticipation gradedParticipation = programmingExerciseStudentParticipationRepository.findByIdElseThrow(id); participationAuthCheckService.checkCanAccessParticipationElseThrow(gradedParticipation); - sourceURL = gradedParticipation.getVcsRepositoryUri(); - } - else { - sourceURL = exercise.getVcsTemplateRepositoryUri(); - } + return gradedParticipation.getVcsRepositoryUri(); + }).orElseGet(exercise::getVcsTemplateRepositoryUri); programmingExerciseParticipationService.resetRepository(participation.getVcsRepositoryUri(), sourceURL); @@ -386,7 +381,7 @@ public ResponseEntity> getParticipationRepositoryFiles(@Path var participation = programmingExerciseStudentParticipationRepository.findByIdElseThrow(participationId); ProgrammingExercise exercise = programmingExerciseRepository.getProgrammingExerciseFromParticipationElseThrow(participation); authCheckService.checkHasAtLeastRoleForExerciseElseThrow(Role.INSTRUCTOR, exercise, null); - return ResponseEntity.ok(repositoryService.getFilesContentAtCommit(exercise, commitId, null, participation)); + return ResponseEntity.ok(repositoryService.getFilesContentAtCommit(exercise, commitId, Optional.empty(), participation)); } /** @@ -403,20 +398,20 @@ public ResponseEntity> getParticipationRepositoryFiles(@Path @GetMapping("programming-exercise/{exerciseId}/files-content-commit-details/{commitId}") @EnforceAtLeastStudent public ResponseEntity> getParticipationRepositoryFilesForCommitsDetailsView(@PathVariable long exerciseId, @PathVariable String commitId, - @RequestParam(required = false) Long participationId, @RequestParam(required = false) RepositoryType repositoryType) throws GitAPIException, IOException { - if (participationId != null) { - Participation participation = participationRepository.findByIdElseThrow(participationId); + @RequestParam Optional participationId, @RequestParam Optional repositoryType) throws GitAPIException, IOException { + if (participationId.isPresent()) { + Participation participation = participationRepository.findByIdElseThrow(participationId.get()); ProgrammingExerciseParticipation programmingExerciseParticipation = repositoryService.getAsProgrammingExerciseParticipationOfExerciseElseThrow(exerciseId, participation, ENTITY_NAME); ProgrammingExercise programmingExercise = programmingExerciseRepository.getProgrammingExerciseFromParticipation(programmingExerciseParticipation); participationAuthCheckService.checkCanAccessParticipationElseThrow(participation); // we only forward the repository type for the test repository, as the test repository is the only one that needs to be treated differently - return ResponseEntity.ok(repositoryService.getFilesContentAtCommit(programmingExercise, commitId, null, programmingExerciseParticipation)); + return ResponseEntity.ok(repositoryService.getFilesContentAtCommit(programmingExercise, commitId, Optional.empty(), programmingExerciseParticipation)); } - else if (repositoryType != null) { + else if (repositoryType.isPresent()) { ProgrammingExercise programmingExercise = programmingExerciseRepository.findByIdWithTemplateAndSolutionParticipationAndAuxiliaryRepositoriesElseThrow(exerciseId); authCheckService.checkHasAtLeastRoleForExerciseElseThrow(Role.EDITOR, programmingExercise, null); - var participation = repositoryType == RepositoryType.TEMPLATE ? programmingExercise.getTemplateParticipation() : programmingExercise.getSolutionParticipation(); + var participation = repositoryType.get() == RepositoryType.TEMPLATE ? programmingExercise.getTemplateParticipation() : programmingExercise.getSolutionParticipation(); return ResponseEntity.ok(repositoryService.getFilesContentAtCommit(programmingExercise, commitId, repositoryType, participation)); } else { diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java b/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java index 2225baa81759..4434bf7423db 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java @@ -279,7 +279,7 @@ public ResponseEntity createProgrammingExercise(@RequestBod @EnforceAtLeastEditor @FeatureToggle(Feature.ProgrammingExercises) public ResponseEntity updateProgrammingExercise(@RequestBody ProgrammingExercise updatedProgrammingExercise, - @RequestParam(value = "notificationText", required = false) String notificationText) throws JsonProcessingException { + @RequestParam(value = "notificationText") Optional notificationText) throws JsonProcessingException { log.debug("REST request to update ProgrammingExercise : {}", updatedProgrammingExercise); if (updatedProgrammingExercise.getId() == null) { throw new BadRequestAlertException("Programming exercise cannot have an empty id when updating", ENTITY_NAME, "noProgrammingExerciseId"); @@ -391,7 +391,7 @@ public ResponseEntity updateProgrammingExercise(@RequestBod @EnforceAtLeastEditor @FeatureToggle(Feature.ProgrammingExercises) public ResponseEntity updateProgrammingExerciseTimeline(@RequestBody ProgrammingExercise updatedProgrammingExercise, - @RequestParam(value = "notificationText", required = false) String notificationText) { + @RequestParam(value = "notificationText") Optional notificationText) { log.debug("REST request to update the timeline of ProgrammingExercise : {}", updatedProgrammingExercise); var existingProgrammingExercise = programmingExerciseRepository.findByIdElseThrow(updatedProgrammingExercise.getId()); var user = userRepository.getUserWithGroupsAndAuthorities(); @@ -414,7 +414,7 @@ public ResponseEntity updateProgrammingExerciseTimeline(@Re @PatchMapping("programming-exercises/{exerciseId}/problem-statement") @EnforceAtLeastEditor public ResponseEntity updateProblemStatement(@PathVariable long exerciseId, @RequestBody String updatedProblemStatement, - @RequestParam(value = "notificationText", required = false) String notificationText) { + @RequestParam(value = "notificationText") Optional notificationText) { log.debug("REST request to update ProgrammingExercise with new problem statement: {}", updatedProblemStatement); var programmingExercise = programmingExerciseRepository.findWithTemplateAndSolutionParticipationTeamAssignmentConfigCategoriesById(exerciseId) .orElseThrow(() -> new EntityNotFoundException("Programming Exercise", exerciseId)); @@ -758,7 +758,7 @@ public ResponseEntity reset(@PathVariable Long exerciseId, @RequestBody Pr @EnforceAtLeastEditor @FeatureToggle(Feature.ProgrammingExercises) public ResponseEntity reEvaluateAndUpdateProgrammingExercise(@PathVariable long exerciseId, @RequestBody ProgrammingExercise programmingExercise, - @RequestParam(value = "deleteFeedback", required = false) Boolean deleteFeedbackAfterGradingInstructionUpdate) throws JsonProcessingException { + @RequestParam(value = "deleteFeedback", required = false, defaultValue = "false") boolean deleteFeedbackAfterGradingInstructionUpdate) throws JsonProcessingException { log.debug("REST request to re-evaluate ProgrammingExercise : {}", programmingExercise); // check that the exercise exists for given id programmingExerciseRepository.findByIdElseThrow(exerciseId); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/web/hestia/ProgrammingExerciseGitDiffReportResource.java b/src/main/java/de/tum/cit/aet/artemis/programming/web/hestia/ProgrammingExerciseGitDiffReportResource.java index 5dad0330b670..d96d7da6bfc1 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/web/hestia/ProgrammingExerciseGitDiffReportResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/web/hestia/ProgrammingExerciseGitDiffReportResource.java @@ -3,6 +3,7 @@ import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE; import java.io.IOException; +import java.util.Optional; import org.eclipse.jgit.api.errors.GitAPIException; import org.slf4j.Logger; @@ -169,21 +170,21 @@ public ResponseEntity getGitDiffReportForSu @GetMapping("programming-exercises/{exerciseId}/commits/{commitHash1}/diff-report/{commitHash2}") @EnforceAtLeastStudent public ResponseEntity getGitDiffReportForCommits(@PathVariable long exerciseId, @PathVariable String commitHash1, - @PathVariable String commitHash2, @RequestParam(required = false) Long participationId, @RequestParam(required = false) RepositoryType repositoryType) + @PathVariable String commitHash2, @RequestParam Optional participationId, @RequestParam Optional repositoryType) throws GitAPIException, IOException { log.debug("REST request to get a diff report for two commits for commit {} and commit {} of participation {}", commitHash1, commitHash2, participationId); VcsRepositoryUri repositoryUri = null; - if (participationId != null) { - Participation participation = participationRepository.findByIdElseThrow(participationId); + if (participationId.isPresent()) { + Participation participation = participationRepository.findByIdElseThrow(participationId.get()); participationAuthCheckService.checkCanAccessParticipationElseThrow(participation); var programmingExerciseParticipation = repositoryService.getAsProgrammingExerciseParticipationOfExerciseElseThrow(exerciseId, participation, ENTITY_NAME); repositoryUri = programmingExerciseParticipation.getVcsRepositoryUri(); } - else if (repositoryType != null) { + else if (repositoryType.isPresent()) { ProgrammingExercise programmingExercise = programmingExerciseRepository.findByIdWithTemplateAndSolutionParticipationAndAuxiliaryRepositoriesElseThrow(exerciseId); authCheckService.checkHasAtLeastRoleForExerciseElseThrow(Role.TEACHING_ASSISTANT, programmingExercise, null); - repositoryUri = switch (repositoryType) { + repositoryUri = switch (repositoryType.get()) { case TEMPLATE -> programmingExercise.getTemplateParticipation().getVcsRepositoryUri(); case SOLUTION -> programmingExercise.getSolutionParticipation().getVcsRepositoryUri(); case TESTS -> programmingExercise.getVcsTestRepositoryUri(); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/web/repository/RepositoryProgrammingExerciseParticipationResource.java b/src/main/java/de/tum/cit/aet/artemis/programming/web/repository/RepositoryProgrammingExerciseParticipationResource.java index 6cb9df0b10ee..bdacf6933862 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/web/repository/RepositoryProgrammingExerciseParticipationResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/web/repository/RepositoryProgrammingExerciseParticipationResource.java @@ -212,15 +212,18 @@ public ResponseEntity> getFilesForPlagiarismView(@PathVari * GET /repository/{participationId}/files/{commitId} : Gets the files of the repository with the given participationId at the given commitId. * This enforces at least instructor access rights. * - * @param participationId the participationId of the repository we want to get the files from - * @param commitId the commitId of the repository we want to get the files from - * @param repositoryType the type of the repository (template, solution, tests) + * @param participationIdOpt the (optional) participationId of the repository we want to get the files from + * @param commitId the commitId of the repository we want to get the files from + * @param repositoryType the type of the repository (template, solution, tests) * @return a map with the file path as key and the file content as value */ @GetMapping(value = "repository-files-content/{commitId}", produces = MediaType.APPLICATION_JSON_VALUE) @EnforceAtLeastStudent - public ResponseEntity> getFilesAtCommit(@PathVariable String commitId, @RequestParam(required = false) Long participationId, - @RequestParam(required = false) RepositoryType repositoryType) { + public ResponseEntity> getFilesAtCommit(@PathVariable String commitId, @RequestParam(name = "participationId") Optional participationIdOpt, + @RequestParam Optional repositoryType) { + + var participationId = participationIdOpt.orElseThrow(NullPointerException::new); + log.debug("REST request to files for domainId {} at commitId {}", participationId, commitId); var participation = getProgrammingExerciseParticipation(participationId); var programmingExercise = programmingExerciseRepository.getProgrammingExerciseFromParticipationElseThrow(participation); diff --git a/src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java b/src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java index 118c5554a1f9..a687666ee426 100644 --- a/src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java @@ -260,7 +260,7 @@ public ResponseEntity createQuizExercise(@RequestPart("exercise") @PutMapping(value = "quiz-exercises/{exerciseId}", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) @EnforceAtLeastEditorInExercise public ResponseEntity updateQuizExercise(@PathVariable Long exerciseId, @RequestPart("exercise") QuizExercise quizExercise, - @RequestPart(value = "files", required = false) List files, @RequestParam(value = "notificationText", required = false) String notificationText) + @RequestPart(value = "files", required = false) List files, @RequestParam(value = "notificationText") Optional notificationText) throws IOException { log.info("REST request to update quiz exercise : {}", quizExercise); quizExercise.setId(exerciseId); diff --git a/src/main/java/de/tum/cit/aet/artemis/text/web/TextAssessmentResource.java b/src/main/java/de/tum/cit/aet/artemis/text/web/TextAssessmentResource.java index 900347ecc54e..17934b2726f3 100644 --- a/src/main/java/de/tum/cit/aet/artemis/text/web/TextAssessmentResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/text/web/TextAssessmentResource.java @@ -353,7 +353,7 @@ public ResponseEntity deleteAssessment(@PathVariable Long participationId, @GetMapping("text-submissions/{submissionId}/for-assessment") @EnforceAtLeastTutor public ResponseEntity retrieveParticipationForSubmission(@PathVariable Long submissionId, - @RequestParam(value = "correction-round", defaultValue = "0") int correctionRound, @RequestParam(value = "resultId", required = false) Long resultId) { + @RequestParam(value = "correction-round", defaultValue = "0") int correctionRound, @RequestParam(value = "resultId") Optional resultId) { log.debug("REST request to get data for tutors text assessment submission: {}", submissionId); final var textSubmission = textSubmissionRepository.findByIdWithParticipationExerciseResultAssessorAssessmentNoteElseThrow(submissionId); final Participation participation = textSubmission.getParticipation(); @@ -366,9 +366,9 @@ public ResponseEntity retrieveParticipationForSubmission(@PathVar authCheckService.checkIsAllowedToAssessExerciseElseThrow(exercise, user, resultId); Result result; - if (resultId != null) { + if (resultId.isPresent()) { // in case resultId is set we get result by id - result = textSubmission.getManualResultsById(resultId); + result = textSubmission.getManualResultsById(resultId.get()); if (result == null) { return ResponseEntity.badRequest() @@ -405,8 +405,8 @@ public ResponseEntity retrieveParticipationForSubmission(@PathVar textSubmission.getResults().forEach(res -> res.setSubmission(null)); // set result again as it was changed - if (resultId != null) { - result = textSubmission.getManualResultsById(resultId); + if (resultId.isPresent()) { + result = textSubmission.getManualResultsById(resultId.get()); textSubmission.setResults(Collections.singletonList(result)); } else { diff --git a/src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java b/src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java index 8f98cd911c51..cd873e16302c 100644 --- a/src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java @@ -245,8 +245,8 @@ public ResponseEntity createTextExercise(@RequestBody TextExercise */ @PutMapping("text-exercises") @EnforceAtLeastEditor - public ResponseEntity updateTextExercise(@RequestBody TextExercise textExercise, - @RequestParam(value = "notificationText", required = false) String notificationText) throws URISyntaxException { + public ResponseEntity updateTextExercise(@RequestBody TextExercise textExercise, @RequestParam(value = "notificationText") Optional notificationText) + throws URISyntaxException { log.debug("REST request to update TextExercise : {}", textExercise); if (textExercise.getId() == null) { return createTextExercise(textExercise); @@ -612,7 +612,7 @@ public ResponseEntity> checkPlagiarism @PutMapping("text-exercises/{exerciseId}/re-evaluate") @EnforceAtLeastEditor public ResponseEntity reEvaluateAndUpdateTextExercise(@PathVariable long exerciseId, @RequestBody TextExercise textExercise, - @RequestParam(value = "deleteFeedback", required = false) Boolean deleteFeedbackAfterGradingInstructionUpdate) throws URISyntaxException { + @RequestParam(value = "deleteFeedback", defaultValue = "false") boolean deleteFeedbackAfterGradingInstructionUpdate) throws URISyntaxException { log.debug("REST request to re-evaluate TextExercise : {}", textExercise); // check that the exercise exists for given id @@ -628,6 +628,6 @@ public ResponseEntity reEvaluateAndUpdateTextExercise(@PathVariabl exerciseService.reEvaluateExercise(textExercise, deleteFeedbackAfterGradingInstructionUpdate); - return updateTextExercise(textExercise, null); + return updateTextExercise(textExercise, Optional.empty()); } } diff --git a/src/main/java/de/tum/cit/aet/artemis/tutorialgroup/web/TutorialGroupSessionResource.java b/src/main/java/de/tum/cit/aet/artemis/tutorialgroup/web/TutorialGroupSessionResource.java index 4d9c9459f65c..7e2bff2af3e7 100644 --- a/src/main/java/de/tum/cit/aet/artemis/tutorialgroup/web/TutorialGroupSessionResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/tutorialgroup/web/TutorialGroupSessionResource.java @@ -13,12 +13,11 @@ import java.util.stream.Collectors; import jakarta.validation.Valid; -import jakarta.validation.constraints.Max; -import jakarta.validation.constraints.Min; import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.Size; import jakarta.ws.rs.BadRequestException; +import org.hibernate.validator.constraints.Range; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.context.annotation.Profile; @@ -170,12 +169,13 @@ public ResponseEntity update(@PathVariable Long courseId, @PatchMapping("courses/{courseId}/tutorial-groups/{tutorialGroupId}/sessions/{sessionId}/attendance-count") @EnforceAtLeastTutor public ResponseEntity updateAttendanceCount(@PathVariable Long courseId, @PathVariable Long tutorialGroupId, @PathVariable Long sessionId, - @RequestParam(required = false) @Min(0) @Max(3000) Integer attendanceCount) { + @RequestParam Optional<@Range(min = 0, max = 3000) Integer> attendanceCount) { // @Range is [from; to] + log.debug("REST request to update attendance count of session: {} of tutorial group: {} of course {} to {}", sessionId, tutorialGroupId, courseId, attendanceCount); var sessionToUpdate = this.tutorialGroupSessionRepository.findByIdElseThrow(sessionId); checkEntityIdMatchesPathIds(sessionToUpdate, Optional.of(courseId), Optional.of(tutorialGroupId), Optional.of(sessionId)); tutorialGroupService.isAllowedToModifySessionsOfTutorialGroup(sessionToUpdate.getTutorialGroup(), null); - sessionToUpdate.setAttendanceCount(attendanceCount); + sessionToUpdate.setAttendanceCount(attendanceCount.orElse(null)); var result = tutorialGroupSessionRepository.save(sessionToUpdate); return ResponseEntity.ok(TutorialGroupSession.preventCircularJsonConversion(result)); } diff --git a/src/test/java/de/tum/cit/aet/artemis/communication/notification/GroupNotificationServiceTest.java b/src/test/java/de/tum/cit/aet/artemis/communication/notification/GroupNotificationServiceTest.java index 4ccf0116d463..716e36a1e665 100644 --- a/src/test/java/de/tum/cit/aet/artemis/communication/notification/GroupNotificationServiceTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/communication/notification/GroupNotificationServiceTest.java @@ -36,6 +36,7 @@ import java.time.ZonedDateTime; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.Set; import org.junit.jupiter.api.AfterEach; @@ -281,7 +282,7 @@ private Notification verifyRepositoryCallWithCorrectNotificationAndReturnNotific */ @Test void testNotifyAboutExerciseUpdate_undefinedReleaseDate() { - groupNotificationService.notifyAboutExerciseUpdate(exercise, NOTIFICATION_TEXT); + groupNotificationService.notifyAboutExerciseUpdate(exercise, Optional.of(NOTIFICATION_TEXT)); verify(groupNotificationService).notifyStudentAndEditorAndInstructorGroupAboutExerciseUpdate(exercise, NOTIFICATION_TEXT); } @@ -291,7 +292,7 @@ void testNotifyAboutExerciseUpdate_undefinedReleaseDate() { @Test void testNotifyAboutExerciseUpdate_futureReleaseDate() { exercise.setReleaseDate(FUTURE_TIME); - groupNotificationService.notifyAboutExerciseUpdate(exercise, NOTIFICATION_TEXT); + groupNotificationService.notifyAboutExerciseUpdate(exercise, Optional.of(NOTIFICATION_TEXT)); verify(groupNotificationService, never()).notifyStudentAndEditorAndInstructorGroupAboutExerciseUpdate(exercise, NOTIFICATION_TEXT); } @@ -301,7 +302,7 @@ void testNotifyAboutExerciseUpdate_futureReleaseDate() { @Test void testNotifyAboutExerciseUpdate_correctReleaseDate_examExercise() { examExercise.setReleaseDate(CURRENT_TIME); - groupNotificationService.notifyAboutExerciseUpdate(examExercise, null); + groupNotificationService.notifyAboutExerciseUpdate(examExercise, Optional.empty()); verify(groupNotificationService, never()).notifyStudentAndEditorAndInstructorGroupAboutExerciseUpdate(any(), any()); } @@ -311,9 +312,9 @@ void testNotifyAboutExerciseUpdate_correctReleaseDate_examExercise() { @Test void testNotifyAboutExerciseUpdate_correctReleaseDate_courseExercise() { exercise.setReleaseDate(CURRENT_TIME); - groupNotificationService.notifyAboutExerciseUpdate(exercise, null); + groupNotificationService.notifyAboutExerciseUpdate(exercise, Optional.empty()); verify(groupNotificationService, never()).notifyStudentAndEditorAndInstructorGroupAboutExerciseUpdate(any(), any()); - groupNotificationService.notifyAboutExerciseUpdate(exercise, NOTIFICATION_TEXT); + groupNotificationService.notifyAboutExerciseUpdate(exercise, Optional.of(NOTIFICATION_TEXT)); verify(groupNotificationService).notifyStudentAndEditorAndInstructorGroupAboutExerciseUpdate(any(), any()); } @@ -361,7 +362,7 @@ private void testCheckNotificationForExerciseReleaseHelper(ZonedDateTime dateOfI updatedExercise.setReleaseDate(dateOfUpdatedExercise); updatedExercise.setAssessmentDueDate(dateOfUpdatedExercise); - groupNotificationScheduleService.checkAndCreateAppropriateNotificationsWhenUpdatingExercise(exercise, updatedExercise, NOTIFICATION_TEXT); + groupNotificationScheduleService.checkAndCreateAppropriateNotificationsWhenUpdatingExercise(exercise, updatedExercise, Optional.of(NOTIFICATION_TEXT)); verify(groupNotificationService).notifyAboutExerciseUpdate(any(), any()); diff --git a/src/test/java/de/tum/cit/aet/artemis/connectors/LtiDynamicRegistrationServiceTest.java b/src/test/java/de/tum/cit/aet/artemis/connectors/LtiDynamicRegistrationServiceTest.java index 0fc6914d30d5..2803a544a173 100644 --- a/src/test/java/de/tum/cit/aet/artemis/connectors/LtiDynamicRegistrationServiceTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/connectors/LtiDynamicRegistrationServiceTest.java @@ -8,6 +8,8 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.Optional; + import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -83,7 +85,7 @@ void badRequestWhenGetPlatformConfigurationFails() { doThrow(HttpClientErrorException.class).when(restTemplate).getForEntity(openIdConfigurationUrl, Lti13PlatformConfiguration.class); assertThatExceptionOfType(BadRequestAlertException.class) - .isThrownBy(() -> ltiDynamicRegistrationService.performDynamicRegistration(openIdConfigurationUrl, registrationToken)); + .isThrownBy(() -> ltiDynamicRegistrationService.performDynamicRegistration(openIdConfigurationUrl, Optional.of(registrationToken))); } @Test @@ -92,7 +94,7 @@ void badRequestWhenPlatformConfigurationEmpty() { when(restTemplate.getForEntity(openIdConfigurationUrl, Lti13PlatformConfiguration.class)).thenReturn(ResponseEntity.accepted().body(platformConfiguration)); assertThatExceptionOfType(BadRequestAlertException.class) - .isThrownBy(() -> ltiDynamicRegistrationService.performDynamicRegistration(openIdConfigurationUrl, registrationToken)); + .isThrownBy(() -> ltiDynamicRegistrationService.performDynamicRegistration(openIdConfigurationUrl, Optional.of(registrationToken))); } @Test @@ -102,7 +104,7 @@ void badRequestWhenRegistrationEndpointEmpty() { when(restTemplate.getForEntity(openIdConfigurationUrl, Lti13PlatformConfiguration.class)).thenReturn(ResponseEntity.accepted().body(platformConfiguration)); assertThatExceptionOfType(BadRequestAlertException.class) - .isThrownBy(() -> ltiDynamicRegistrationService.performDynamicRegistration(openIdConfigurationUrl, registrationToken)); + .isThrownBy(() -> ltiDynamicRegistrationService.performDynamicRegistration(openIdConfigurationUrl, Optional.of(registrationToken))); } @Test @@ -113,7 +115,7 @@ void badRequestWhenGetPostClientRegistrationFails() { doThrow(HttpClientErrorException.class).when(restTemplate).postForEntity(eq(platformConfiguration.registrationEndpoint()), any(), eq(Lti13ClientRegistration.class)); assertThatExceptionOfType(BadRequestAlertException.class) - .isThrownBy(() -> ltiDynamicRegistrationService.performDynamicRegistration(openIdConfigurationUrl, registrationToken)); + .isThrownBy(() -> ltiDynamicRegistrationService.performDynamicRegistration(openIdConfigurationUrl, Optional.of(registrationToken))); } @Test @@ -123,7 +125,7 @@ void performDynamicRegistrationSuccess() { when(restTemplate.postForEntity(eq(platformConfiguration.registrationEndpoint()), any(), eq(Lti13ClientRegistration.class))) .thenReturn(ResponseEntity.accepted().body(clientRegistrationResponse)); - ltiDynamicRegistrationService.performDynamicRegistration(openIdConfigurationUrl, registrationToken); + ltiDynamicRegistrationService.performDynamicRegistration(openIdConfigurationUrl, Optional.of(registrationToken)); verify(ltiPlatformConfigurationRepository).save(any()); verify(oAuth2JWKSService).updateKey(any()); @@ -136,7 +138,7 @@ void performDynamicRegistrationSuccessWithoutToken() { when(restTemplate.postForEntity(eq(platformConfiguration.registrationEndpoint()), any(), eq(Lti13ClientRegistration.class))) .thenReturn(ResponseEntity.accepted().body(clientRegistrationResponse)); - ltiDynamicRegistrationService.performDynamicRegistration(openIdConfigurationUrl, null); + ltiDynamicRegistrationService.performDynamicRegistration(openIdConfigurationUrl, Optional.empty()); verify(ltiPlatformConfigurationRepository).save(any()); verify(oAuth2JWKSService).updateKey(any()); diff --git a/src/test/java/de/tum/cit/aet/artemis/modeling/ModelingExerciseIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/modeling/ModelingExerciseIntegrationTest.java index ed0fd1106333..491650d43608 100644 --- a/src/test/java/de/tum/cit/aet/artemis/modeling/ModelingExerciseIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/modeling/ModelingExerciseIntegrationTest.java @@ -239,7 +239,7 @@ void testUpdateModelingExercise_asInstructor() throws Exception { params); assertThat(returnedModelingExercise.getGradingCriteria()).hasSameSizeAs(gradingCriteria); verify(groupNotificationService).notifyStudentAndEditorAndInstructorGroupAboutExerciseUpdate(returnedModelingExercise, notificationText); - verify(examLiveEventsService, never()).createAndSendProblemStatementUpdateEvent(eq(returnedModelingExercise), eq(notificationText), any()); + verify(examLiveEventsService, never()).createAndSendProblemStatementUpdateEvent(eq(returnedModelingExercise), eq(Optional.of(notificationText)), any()); verify(competencyProgressService, timeout(1000).times(1)).updateProgressForUpdatedLearningObjectAsync(eq(createdModelingExercise), eq(Optional.of(createdModelingExercise))); } @@ -267,7 +267,7 @@ void testUpdateModelingExerciseForExam_asInstructor() throws Exception { params); verify(groupNotificationService, never()).notifyStudentAndEditorAndInstructorGroupAboutExerciseUpdate(returnedModelingExercise, notificationText); - verify(examLiveEventsService, times(1)).createAndSendProblemStatementUpdateEvent(eq(returnedModelingExercise), eq(notificationText), any()); + verify(examLiveEventsService, times(1)).createAndSendProblemStatementUpdateEvent(eq(returnedModelingExercise), eq(Optional.of(notificationText)), any()); } @Test @@ -816,12 +816,12 @@ void testImport_team_modeChange() throws Exception { assertThat(exerciseToBeImported.getMode()).isEqualTo(ExerciseMode.TEAM); assertThat(exerciseToBeImported.getTeamAssignmentConfig().getMinTeamSize()).isEqualTo(teamAssignmentConfig.getMinTeamSize()); assertThat(exerciseToBeImported.getTeamAssignmentConfig().getMaxTeamSize()).isEqualTo(teamAssignmentConfig.getMaxTeamSize()); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(exerciseToBeImported, null)).isEmpty(); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(exerciseToBeImported, Optional.empty())).isEmpty(); sourceExercise = modelingExerciseRepository.findById(sourceExercise.getId()).orElseThrow(); assertThat(sourceExercise.getCourseViaExerciseGroupOrCourseMember().getId()).isEqualTo(course1.getId()); assertThat(sourceExercise.getMode()).isEqualTo(ExerciseMode.INDIVIDUAL); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(sourceExercise, null)).isEmpty(); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(sourceExercise, Optional.empty())).isEmpty(); } @Test @@ -855,12 +855,12 @@ void testImport_individual_modeChange() throws Exception { assertThat(exerciseToBeImported.getCourseViaExerciseGroupOrCourseMember().getId()).isEqualTo(course2.getId()); assertThat(exerciseToBeImported.getMode()).isEqualTo(ExerciseMode.INDIVIDUAL); assertThat(exerciseToBeImported.getTeamAssignmentConfig()).isNull(); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(exerciseToBeImported, null)).isEmpty(); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(exerciseToBeImported, Optional.empty())).isEmpty(); sourceExercise = modelingExerciseRepository.findById(sourceExercise.getId()).orElseThrow(); assertThat(sourceExercise.getCourseViaExerciseGroupOrCourseMember().getId()).isEqualTo(course1.getId()); assertThat(sourceExercise.getMode()).isEqualTo(ExerciseMode.TEAM); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(sourceExercise, null)).hasSize(1); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(sourceExercise, Optional.empty())).hasSize(1); } @Test diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java b/src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java index 95ba2804b3b6..312f461fa1e9 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java @@ -1004,12 +1004,12 @@ public void testImportProgrammingExercise_team_modeChange() throws Exception { assertThat(exerciseToBeImported.getMode()).isEqualTo(TEAM); assertThat(exerciseToBeImported.getTeamAssignmentConfig().getMinTeamSize()).isEqualTo(teamAssignmentConfig.getMinTeamSize()); assertThat(exerciseToBeImported.getTeamAssignmentConfig().getMaxTeamSize()).isEqualTo(teamAssignmentConfig.getMaxTeamSize()); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(exerciseToBeImported, null)).isEmpty(); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(exerciseToBeImported, Optional.empty())).isEmpty(); sourceExercise = programmingExerciseUtilService.loadProgrammingExerciseWithEagerReferences(sourceExercise); assertThat(sourceExercise.getMode()).isEqualTo(ExerciseMode.INDIVIDUAL); assertThat(sourceExercise.getTeamAssignmentConfig()).isNull(); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(sourceExercise, null)).isEmpty(); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(sourceExercise, Optional.empty())).isEmpty(); } // TEST @@ -1047,11 +1047,11 @@ public void testImportProgrammingExercise_individual_modeChange() throws Excepti assertThat(exerciseToBeImported.getMode()).isEqualTo(ExerciseMode.INDIVIDUAL); assertThat(exerciseToBeImported.getTeamAssignmentConfig()).isNull(); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(exerciseToBeImported, null)).isEmpty(); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(exerciseToBeImported, Optional.empty())).isEmpty(); sourceExercise = programmingExerciseUtilService.loadProgrammingExerciseWithEagerReferences(sourceExercise); assertThat(sourceExercise.getMode()).isEqualTo(TEAM); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(sourceExercise, null)).hasSize(1); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(sourceExercise, Optional.empty())).hasSize(1); } // TEST diff --git a/src/test/java/de/tum/cit/aet/artemis/quiz/QuizExerciseIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/quiz/QuizExerciseIntegrationTest.java index e787ee1533a0..47e7af87dfed 100644 --- a/src/test/java/de/tum/cit/aet/artemis/quiz/QuizExerciseIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/quiz/QuizExerciseIntegrationTest.java @@ -1406,13 +1406,13 @@ void testImportQuizExercise_team_modeChange() throws Exception { assertThat(changedQuiz.getMode()).isEqualTo(ExerciseMode.TEAM); assertThat(changedQuiz.getTeamAssignmentConfig().getMinTeamSize()).isEqualTo(1); assertThat(changedQuiz.getTeamAssignmentConfig().getMaxTeamSize()).isEqualTo(10); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(changedQuiz, null)).isEmpty(); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(changedQuiz, Optional.empty())).isEmpty(); quizExercise = quizExerciseTestRepository.findByIdElseThrow(quizExercise.getId()); assertThat(quizExercise.getMode()).isEqualTo(ExerciseMode.INDIVIDUAL); assertThat(quizExercise.getTeamAssignmentConfig()).isNull(); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(quizExercise, null)).isEmpty(); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(quizExercise, Optional.empty())).isEmpty(); } /** @@ -1450,11 +1450,11 @@ void testImportQuizExercise_individual_modeChange() throws Exception { assertThat(changedQuiz.getCourseViaExerciseGroupOrCourseMember().getId()).isEqualTo(course.getId()); assertThat(changedQuiz.getMode()).isEqualTo(ExerciseMode.INDIVIDUAL); assertThat(changedQuiz.getTeamAssignmentConfig()).isNull(); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(changedQuiz, null)).isEmpty(); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(changedQuiz, Optional.empty())).isEmpty(); quizExercise = quizExerciseTestRepository.findByIdElseThrow(quizExercise.getId()); assertThat(quizExercise.getMode()).isEqualTo(ExerciseMode.TEAM); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(quizExercise, null)).hasSize(1); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(quizExercise, Optional.empty())).hasSize(1); } /** diff --git a/src/test/java/de/tum/cit/aet/artemis/shared/architecture/ArchitectureTest.java b/src/test/java/de/tum/cit/aet/artemis/shared/architecture/ArchitectureTest.java index 2cbd4da31b86..9f5bfd243bfe 100644 --- a/src/test/java/de/tum/cit/aet/artemis/shared/architecture/ArchitectureTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/shared/architecture/ArchitectureTest.java @@ -31,6 +31,7 @@ import java.lang.reflect.Method; import java.nio.file.Files; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -53,7 +54,9 @@ import org.springframework.stereotype.Controller; import org.springframework.stereotype.Repository; import org.springframework.stereotype.Service; +import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.bind.annotation.ValueConstants; import com.fasterxml.jackson.annotation.JsonInclude; import com.hazelcast.core.HazelcastInstance; @@ -65,6 +68,7 @@ import com.tngtech.archunit.core.domain.JavaEnumConstant; import com.tngtech.archunit.core.domain.JavaMethod; import com.tngtech.archunit.core.domain.properties.HasAnnotations; +import com.tngtech.archunit.core.domain.properties.HasType; import com.tngtech.archunit.lang.ArchCondition; import com.tngtech.archunit.lang.ArchRule; import com.tngtech.archunit.lang.ConditionEvents; @@ -284,6 +288,35 @@ public void check(T item, ConditionEvents events) { }; } + @Test + void testNoRequiredFalseOnMethodParams() { + methods().should(notHaveRequestParamWithRequiredFalse()).check(allClasses); + } + + private ArchCondition notHaveRequestParamWithRequiredFalse() { + return new ArchCondition<>("Do not use 'required = false' with @RequestParam") { + + @Override + public void check(final JavaMethod method, final ConditionEvents events) { + + // We only care about the case where required is false and the Parameter is not an Optional, any other combination is fine. + final var violated = method.getParameters().stream().filter(not(HasType.Predicates.rawType(Optional.class))).flatMap(param -> param.getAnnotations().stream()) + .filter(HasType.Predicates.rawType(RequestParam.class)).filter(annotation -> { + final String value = (String) annotation.get("defaultValue").orElse(ValueConstants.DEFAULT_NONE); + return ValueConstants.DEFAULT_NONE.equals(value); + }).map(annotation -> annotation.get("required").orElse(true)) // if not set, the default is true + .map(Boolean.class::cast) // since we filter for the annotation and field we know this cast is safe! + .anyMatch(required -> !required); + + if (violated) { + final String message = String.format("Method %s uses 'required = false' without a default value set, use Optional instead or add a default value!", + method.getFullName()); + events.add(SimpleConditionEvent.violated(method, message)); + } + } + }; + } + private static ArchCondition beProfileAnnotated() { return new ArchCondition<>("be annotated with @Profile") { diff --git a/src/test/java/de/tum/cit/aet/artemis/text/TextExerciseIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/text/TextExerciseIntegrationTest.java index cbcd54bb78f3..279b54c806f7 100644 --- a/src/test/java/de/tum/cit/aet/artemis/text/TextExerciseIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/text/TextExerciseIntegrationTest.java @@ -985,13 +985,13 @@ void testImportTextExercise_team_modeChange() throws Exception { assertThat(exerciseToBeImported.getMode()).isEqualTo(ExerciseMode.TEAM); assertThat(exerciseToBeImported.getTeamAssignmentConfig().getMinTeamSize()).isEqualTo(teamAssignmentConfig.getMinTeamSize()); assertThat(exerciseToBeImported.getTeamAssignmentConfig().getMaxTeamSize()).isEqualTo(teamAssignmentConfig.getMaxTeamSize()); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(exerciseToBeImported, null)).isEmpty(); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(exerciseToBeImported, Optional.empty())).isEmpty(); sourceExercise = textExerciseRepository.findById(sourceExercise.getId()).orElseThrow(); assertThat(sourceExercise.getCourseViaExerciseGroupOrCourseMember().getId()).isEqualTo(course1.getId()); assertThat(sourceExercise.getMode()).isEqualTo(ExerciseMode.INDIVIDUAL); assertThat(sourceExercise.getTeamAssignmentConfig()).isNull(); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(sourceExercise, null)).isEmpty(); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(sourceExercise, Optional.empty())).isEmpty(); } @Test @@ -1025,12 +1025,12 @@ void testImportTextExercise_individual_modeChange() throws Exception { assertThat(exerciseToBeImported.getCourseViaExerciseGroupOrCourseMember().getId()).isEqualTo(course2.getId()); assertThat(exerciseToBeImported.getMode()).isEqualTo(ExerciseMode.INDIVIDUAL); assertThat(exerciseToBeImported.getTeamAssignmentConfig()).isNull(); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(exerciseToBeImported, null)).isEmpty(); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(exerciseToBeImported, Optional.empty())).isEmpty(); sourceExercise = textExerciseRepository.findById(sourceExercise.getId()).orElseThrow(); assertThat(sourceExercise.getCourseViaExerciseGroupOrCourseMember().getId()).isEqualTo(course1.getId()); assertThat(sourceExercise.getMode()).isEqualTo(ExerciseMode.TEAM); - assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(sourceExercise, null)).hasSize(1); + assertThat(teamRepository.findAllByExerciseIdWithEagerStudents(sourceExercise, Optional.empty())).hasSize(1); } @Test