Development: Introduce exam module API
#10262
Conversation
github-actions
bot
added
tests
server
WalkthroughThis pull request refactors numerous service, resource, and export modules across assessment, exam, exercise, fileupload, modeling, programming, quiz, and text packages. It replaces direct service or repository dependencies (e.g. ExamDateService, ExamRepository) with Optional‑wrapped API interfaces (e.g. ExamDateApi, ExamRepositoryApi) and introduces new API classes (such as ExamAccessApi, ExamApi, ExamMetricsApi, etc.). Methods are updated to use constructs like orElseThrow for error handling with ApiNotPresentException, and tests and architecture checks are revised accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Resource
participant OptionalApi
participant Exception
Client->>Resource: Request exam-related operation
Resource->>OptionalApi: Retrieve API instance (orElseThrow)
alt API present
OptionalApi-->>Resource: Return API instance
Resource->>OptionalApi: Execute requested method
OptionalApi-->>Resource: Return result
else API absent
OptionalApi-->>Resource: Throws ApiNotPresentException
Resource-->>Client: Returns error response
end
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (1)
src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseResource.java (1)
340-340: Submissions Sensitive Data Filtering Missing – Action RequiredOur investigation shows that none of the submission classes (or related helper methods) currently implement any logic to filter or mask sensitive information. The search for functions like
filterSensitiveInformationorgetSensitiveInformationreturned no results. This confirms that the TODO inExerciseResource.javais valid and must be addressed to mitigate potential security risks.
- No sensitive data filtering mechanism exists for submissions.
- Consider adding logic to sanitize or remove sensitive fields before sending responses.
🔗 Analysis chain
Address TODO comment regarding sensitive data in submissions.
The TODO comment indicates a potential security concern that should be addressed.
Let's verify if there are any sensitive fields in the submission classes:
Would you like me to help implement the sensitive data filtering for submissions? I can create a new issue to track this security enhancement.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for submission-related classes to identify potential sensitive fields ast-grep --pattern 'class $SUBMISSION extends $_ { $$$ }' # Search for any existing sensitive data filtering in submissions rg -A 5 'filterSensitiveInformation|getSensitiveInformation'Length of output: 70487
🧹 Nitpick comments (49)
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultWebsocketService.java (1)
79-80: Consider enhancing the error message for better debugging.While the API usage is correct, consider providing a more descriptive error message that includes context about the operation being performed.
- var api = examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); + var api = examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE, + "Required for checking individual exercise working period in exam"));src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java (1)
799-801: Consider enhancing error handling with more context.The API presence check and usage are correct, but consider adding more context to the error message to help with troubleshooting.
Apply this diff to enhance the error message:
- var api = examLiveEventsApi.orElseThrow(() -> new ApiNotPresentException(ExamLiveEventsApi.class, PROFILE_CORE)); + var api = examLiveEventsApi.orElseThrow(() -> new ApiNotPresentException(ExamLiveEventsApi.class, PROFILE_CORE, + "Required for sending problem statement updates in exam exercises"));src/test/java/de/tum/cit/aet/artemis/shared/architecture/module/AbstractModuleAccessArchitectureTest.java (2)
28-29: LGTM! Consider adding JavaDoc to document the ignored classes feature.The modifications consistently apply ignored classes filtering across all test methods, improving test flexibility. This aligns well with the modularization effort.
Consider adding JavaDoc to document the purpose and usage of the ignored classes feature, especially since this is an abstract class that other tests will extend. For example:
+ /** + * Executes architectural test to ensure classes outside the module only access API, domain, or DTO classes. + * Classes returned by {@link #getIgnoredClasses()} are excluded from this check. + */ @Test void shouldOnlyAccessApiDomainDto() {Also applies to: 36-37, 42-43
58-60: Add JavaDoc to document the purpose and contract of getIgnoredClasses.The method is well-designed but needs documentation to guide implementers.
Add JavaDoc to explain the method's purpose and contract:
+ /** + * Returns a set of classes that should be excluded from architectural checks. + * Subclasses can override this method to exclude specific classes from the architectural rules. + * + * @return a Set of Class<?> objects to be ignored in architectural checks, empty by default + */ protected Set<Class<?>> getIgnoredClasses() { return Set.of(); }src/main/java/de/tum/cit/aet/artemis/exam/api/ExerciseGroupApi.java (2)
11-13: Add JavaDoc documentation to describe the API's purpose.The class structure and annotations are well-designed for the exam module API. However, as per the PR objectives, Java code should be documented using JavaDoc style.
Add class-level documentation:
+/** + * API facade for exercise group operations in the exam module. + * This controller provides methods to interact with exercise groups while maintaining + * proper separation of concerns through the module API pattern. + */ @Profile(PROFILE_CORE) @Controller public class ExerciseGroupApi extends AbstractExamApi {
21-23: Add documentation and parameter validation.While the method implementation is correct, it should include JavaDoc documentation and parameter validation.
Apply these improvements:
+ /** + * Retrieves an exercise group by its ID. + * + * @param id the ID of the exercise group to find + * @return the exercise group + * @throws EntityNotFoundException if no exercise group exists with the given ID + * @throws IllegalArgumentException if the ID is negative + */ public ExerciseGroup findByIdElseThrow(long id) { + if (id < 0) { + throw new IllegalArgumentException("Exercise group ID cannot be negative"); + } return exerciseGroupRepository.findByIdElseThrow(id); }src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingSubmissionService.java (1)
122-124: Consider adding exam date validation.The comment indicates that due date checks are skipped for exam exercises. Given the new exam date API, consider adding explicit validation for exam dates to ensure consistency.
// Important: for exam exercises, we should NOT check the exercise due date, we only check if for course exercises if (dueDate.isPresent() && exerciseDateService.isAfterDueDate(participation) && participation.getInitializationDate().isBefore(dueDate.get())) { throw new AccessForbiddenException(); } +// For exam exercises, validate against exam dates +if (exercise.isExamExercise() && examDateApi.isPresent()) { + examDateApi.get().validateSubmissionDate(participation.getExercise().getExerciseGroup().getExam(), ZonedDateTime.now()) + .orElseThrow(() -> new AccessForbiddenException("Submission not allowed at this time")); +}src/main/java/de/tum/cit/aet/artemis/core/service/AuthorizationCheckService.java (1)
54-54: Consider using a null-object or fallback instead of injecting an Optional
Injecting anOptional<ExamDateApi>is valid, but a null-object (or no-op) implementation can sometimes simplify usage and reduce the need fororElseThrowchecks.src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingAssessmentResource.java (1)
132-190: Consider breaking down the large method into smaller, focused methods.The
saveProgrammingAssessmentmethod is quite long and handles multiple responsibilities. Consider extracting the validation logic into separate methods for better maintainability and reusability.Example refactor:
+ private void validateManualResult(Result newManualResult, String entityName) { + if (Boolean.FALSE.equals(newManualResult.isRated())) { + throw new BadRequestAlertException("Result is not rated", entityName, "resultNotRated"); + } + if (newManualResult.getScore() == null) { + throw new BadRequestAlertException("Score is required.", entityName, "scoreNull"); + } + if (newManualResult.getScore() < 100 && newManualResult.isSuccessful()) { + throw new BadRequestAlertException("Only result with score 100% can be successful.", entityName, "scoreAndSuccessfulNotMatching"); + } + } + + private void validateManualResultFeedback(Result newManualResult, String entityName) { + if (!newManualResult.getFeedbacks().isEmpty()) { + if (newManualResult.getFeedbacks().stream() + .anyMatch(feedback -> feedback.getType() == FeedbackType.MANUAL_UNREFERENCED + && feedback.getGradingInstruction() == null + && feedback.getDetailText() == null)) { + throw new BadRequestAlertException("In case tutor feedback is present, a feedback detail text is mandatory.", + entityName, "feedbackDetailTextNull"); + } + if (newManualResult.getFeedbacks().stream().anyMatch(feedback -> feedback.getCredits() == null)) { + throw new BadRequestAlertException("In case feedback is present, a feedback must contain points.", + entityName, "feedbackCreditsNull"); + } + } + } @ResponseStatus(HttpStatus.OK) @PutMapping("participations/{participationId}/manual-results") @EnforceAtLeastTutor public ResponseEntity<Result> saveProgrammingAssessment(@PathVariable Long participationId, @RequestParam(value = "submit", defaultValue = "false") boolean submit, @RequestBody Result newManualResult) { // ... existing code ... - if (Boolean.FALSE.equals(newManualResult.isRated())) { - throw new BadRequestAlertException("Result is not rated", ENTITY_NAME, "resultNotRated"); - } - // ... other validation code ... + validateManualResult(newManualResult, ENTITY_NAME); + validateManualResultFeedback(newManualResult, ENTITY_NAME); newManualResult = programmingAssessmentService.saveAndSubmitManualAssessment(participation, newManualResult, existingManualResult, user, submit); // ... rest of the code ... }src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java (2)
93-93: Consider documenting optional dependencies.
While this field is valid, adding a short Javadoc explaining theOptional<StudentExamApi>usage maintains clarity and consistency.
534-536: Clarify the negated logic inshouldHideExamExerciseResults.
Returning the inverse ofexamApi.shouldStudentSeeResult(studentExam, participation)can be slightly confusing. Consider adding a comment or renaming methods for clearer readability.src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java (1)
252-254: Consider extracting the exam date validation logic.While the implementation is correct, the exam date validation logic could be extracted into a separate private method to improve readability and reusability.
- var api = examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); - ZonedDateTime latestIndividualExamEndDate = api.getLatestIndividualExamEndDate(exam); + ZonedDateTime latestIndividualExamEndDate = getLatestIndividualExamEndDate(exam); + private ZonedDateTime getLatestIndividualExamEndDate(Exam exam) { + var api = examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); + return api.getLatestIndividualExamEndDate(exam); + }src/main/java/de/tum/cit/aet/artemis/exam/api/AbstractExamApi.java (1)
1-6: Consider adding module API documentation.Since this class is part of the server modularization effort and serves as a facade for exam-related operations, it would be beneficial to add module-level documentation explaining:
- The purpose and scope of the exam module API
- How to properly implement and use this API
- Integration guidelines for other modules
Example documentation:
/** * Abstract base class for the Exam Module API. * <p> * This API provides a facade for exam-related operations, allowing other modules * to interact with exam functionality without direct dependencies on concrete * implementations. Implementing classes should provide the actual exam management * logic while adhering to the defined contract. * <p> * Usage example: * <pre> * {@code * @Autowired * private Optional<ExamApi> examApi; * * public void someMethod() { * examApi.orElseThrow(() -> new ApiNotPresentException("Exam API")) * .getExam(examId); * } * } * </pre> * * @see AbstractApi */src/main/java/de/tum/cit/aet/artemis/text/web/TextSubmissionResource.java (1)
80-80: Consider avoiding Optional as a field
While storing anOptional<>in a field is not inherently wrong, it can be less idiomatic than storing the underlying instance and handling default behaviors in the constructor. Consider injecting a no-op object or a specialized fallback in place ofOptional<>.src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportExamCreationService.java (1)
67-72: Consider logging a warning whenStudentExamApiis unavailable.
Currently, the method silently returns if noStudentExamApiis present. A warning would help diagnose configuration issues.if (studentExamApi.isEmpty()) { + // Log a warning so that admins know why exam exports were skipped + log.warn("StudentExamApi is not present. Skipping exam data export for user {}", userId); return; }src/main/java/de/tum/cit/aet/artemis/core/service/export/CourseExamExportService.java (1)
248-248: Aggregating exercises for all exams in the course.
Streams the results and flattens them into a set. Be mindful of potential performance overhead with large-scale data sets.src/main/java/de/tum/cit/aet/artemis/exam/api/ExamLiveEventsApi.java (1)
22-24: Add JavaDoc and input validation.While the delegation pattern is correct, consider these improvements:
- Add JavaDoc documentation as per coding guidelines
- Add null checks for parameters before delegation
Here's the suggested implementation:
+ /** + * Creates and sends a problem statement update event for an exercise during an exam. + * + * @param exercise the exercise for which the update event is created + * @param message the update message to be sent + * @param instructor the instructor initiating the update + * @throws IllegalArgumentException if any parameter is null + */ public void createAndSendProblemStatementUpdateEvent(Exercise exercise, String message, User instructor) { + if (exercise == null || message == null || instructor == null) { + throw new IllegalArgumentException("Parameters must not be null"); + } examLiveEventsService.createAndSendProblemStatementUpdateEvent(exercise, message, instructor); }src/test/java/de/tum/cit/aet/artemis/exam/architecture/ExamApiArchitectureTest.java (1)
15-18: Enhance the inline documentation.While the comment explains the inheritance allowance, it would be better to format it as a proper JavaDoc comment above the method, explaining both the purpose of ignoring QuizPoolService and the inheritance allowance for ExamQuizQuestionsGenerator.
- @Override - protected Set<Class<?>> getIgnoredClasses() { - return Set.of(QuizPoolService.class); // Inheritance allowed from ExamQuizQuestionsGenerator as it literally defines an interface - } + /** + * Specifies classes to be ignored during architecture validation. + * QuizPoolService is excluded as inheritance is permitted from ExamQuizQuestionsGenerator, + * which defines an interface for quiz question generation. + * + * @return Set of classes to be ignored during architecture validation + */ + @Override + protected Set<Class<?>> getIgnoredClasses() { + return Set.of(QuizPoolService.class); + }src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseResource.java (2)
150-151: Enhance error handling in getExercise method.The API access pattern could be improved to handle the optional dependency more elegantly.
Consider extracting the API access pattern into a reusable helper method:
- var api = examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); - if (authCheckService.isAtLeastEditorForExercise(exercise, user)) { + var api = getExamDateApiOrThrow(); + if (authCheckService.isAtLeastEditorForExercise(exercise, user)) { // Add this helper method: + private ExamDateApi getExamDateApiOrThrow() { + return examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); + }Also applies to: 157-157
201-202: Enhance error handling in getExerciseForExampleSolution method.Similar to the previous comment, the API access pattern could be improved.
Consider extracting the API access pattern into a reusable helper method:
- var api = examAccessApi.orElseThrow(() -> new ApiNotPresentException(ExamAccessApi.class, PROFILE_CORE)); - api.checkExamExerciseForExampleSolutionAccessElseThrow(exercise); + var api = getExamAccessApiOrThrow(); + api.checkExamExerciseForExampleSolutionAccessElseThrow(exercise); // Add this helper method: + private ExamAccessApi getExamAccessApiOrThrow() { + return examAccessApi.orElseThrow(() -> new ApiNotPresentException(ExamAccessApi.class, PROFILE_CORE)); + }src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseScheduleService.java (3)
111-113: Constructor is growing large
While adding optional APIs is necessary, the constructor now has many parameters. To preserve readability and single responsibility, consider refactoring via a builder pattern or a dedicated configuration object.
871-872: Fetching exam via ExamApi
ThrowingApiNotPresentExceptioncovers the missing API scenario, but usingNoSuchElementExceptionfor a missing exam record exposes a less descriptive default message. Switching to a more domain-specific exception (e.g.,EntityNotFoundException) might improve clarity.
901-902: Handling missing student exam through StudentExamApi
Similar to theExamApiusage above, a more explicit exception thanNoSuchElementExceptionmay provide clearer context when a student exam cannot be found or retrieved.src/main/java/de/tum/cit/aet/artemis/exam/api/ExamApi.java (4)
25-27: Consider using a more descriptive controller annotation.Currently, the class is annotated with
@Profile(PROFILE_CORE)and@Controller. If these methods expose REST endpoints, consider using@RestControllerfor transparency and clarity in a Spring ecosystem.
29-36: Evaluate layering and clarity of responsibilities.This class holds both repository references and a service reference. While it may be intentional, mixing direct database access alongside service calls can blur responsibility lines. Consider delegating repository access to the service layer or to a dedicated data-access class and keeping
ExamApias a façade or orchestration layer.
70-72: Return the saved entity if needed.
public void save(Exam exam)does not return the persisted entity. If subsequent logic needs the updated state (e.g., generated IDs, new timestamps), consider returning the result of theexamRepository.save(...)call instead of void.
78-80: Consider more focused responsibility for exam filtering.
filterVisibleExams(Set<Exam> exams)is a domain-specific operation. Evaluate whether it belongs in a domain service or a dedicated repository method to keep APIs minimal and more cohesive.src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDateService.java (2)
13-15: Confirm necessity of Optional-based injection.Injecting optional dependencies (e.g.,
Optional<ExamDateApi>,Optional<StudentExamApi>) can introduce added complexity. Double-check that optional injection is the best design pattern or if a simpler feature toggle or conditional bean approach might suffice.
200-202: Avoid null returns from Optionals if possible.
var studentExam = api.findStudentExam(exercise, participation).orElse(null);leads to a manual null-check. Consider a more direct approach that handles the "empty" case—e.g., returning early or using a functional style to avoid potential confusion and reduce the chance of a NullPointerException.src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java (1)
198-205: Ensuring robust exam references during exercise deletion
The logic to retrieve and updateStudentExaminstances via the new API is coherent. Consider verifying thatexerciseGroupandexamare not null if data integrity might be uncertain, to avoid potential NullPointerExceptions. Otherwise, the approach looks good.src/main/java/de/tum/cit/aet/artemis/assessment/service/ComplaintService.java (1)
59-59: Injecting OptionalInjecting an
Optional<ExamApi>can be useful, but be mindful of the additional complexity if you expect this dependency to be present in most scenarios.src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.java (1)
87-87: Parameter ordering and readabilityConsider maintaining a consistent ordering for constructor parameters to keep them more readable.
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java (1)
83-83: Optional examSubmissionApi fieldWhile injecting an optional is helpful for modularization, double-check if a fallback approach might reduce exception handling overhead.
src/main/java/de/tum/cit/aet/artemis/core/exception/ApiNotPresentException.java (1)
12-18: Consider adding null validation for parameters.While the implementation is correct, consider adding null checks for both parameters to prevent potential NullPointerExceptions.
public ApiNotPresentException(Class<? extends AbstractApi> api, String profile) { + if (api == null || profile == null) { + throw new IllegalArgumentException("API class and profile name must not be null"); + } super(String.format("Api %s is not enabled, because Spring profile %s is not enabled. Did you enable it?", api.getName(), profile)); }src/main/java/de/tum/cit/aet/artemis/exam/api/ExamDeletionApi.java (1)
22-24: Consider adding transaction boundary and exception handling.While the implementation is correct, consider adding transaction boundary and handling potential exceptions from the service layer.
+ @Transactional public void delete(@NotNull long examId) { + try { examDeletionService.delete(examId); + } catch (Exception e) { + throw new RuntimeException("Failed to delete exam with ID: " + examId, e); + } }src/main/java/de/tum/cit/aet/artemis/exam/api/ExamUserApi.java (1)
23-25: Consider adding validation and caching.While the implementation is correct, consider adding parameter validation and caching for performance optimization.
+ @Cacheable(cacheNames = "examUsers", key = "#examUserId") public Optional<ExamUser> findWithExamById(long examUserId) { + if (examUserId <= 0) { + throw new IllegalArgumentException("Exam user ID must be positive"); + } return examUserRepository.findWithExamById(examUserId); }src/main/java/de/tum/cit/aet/artemis/exam/api/ExamAccessApi.java (1)
21-35: Consider adding parameter validation and logging.While the implementation is correct, consider adding parameter validation and logging for better debugging and error tracking.
+ private static final Logger log = LoggerFactory.getLogger(ExamAccessApi.class); public void checkCourseAndExamAccessForStudentElseThrow(Long courseId, Long examId) { + if (courseId == null || examId == null) { + throw new IllegalArgumentException("Course ID and exam ID must not be null"); + } + log.debug("Checking student access for course {} and exam {}", courseId, examId); examAccessService.checkCourseAndExamAccessForStudentElseThrow(courseId, examId); }src/main/java/de/tum/cit/aet/artemis/exam/api/ExamSubmissionApi.java (1)
23-33: Add JavaDoc documentation for public methods.Add documentation to describe the purpose, parameters, return values, and potential exceptions for each public method.
Example for the first method:
+/** + * Checks if a user is allowed to submit during an exam. + * + * @param exercise the exercise to check + * @param user the user attempting to submit + * @param withGracePeriod whether to include grace period in the check + * @return true if submission is allowed, false otherwise + * @throws IllegalArgumentException if exercise or user is null + */ public boolean isAllowedToSubmitDuringExam(Exercise exercise, User user, boolean withGracePeriod) { + if (exercise == null || user == null) { + throw new IllegalArgumentException("Exercise and user must not be null"); + } return examSubmissionService.isAllowedToSubmitDuringExam(exercise, user, withGracePeriod); }src/main/java/de/tum/cit/aet/artemis/exam/api/ExamDateApi.java (1)
38-40: Add parameter validation for examId.Add validation to ensure examId is positive.
public ZonedDateTime getLatestIndividualExamEndDate(Long examId) { + if (examId == null || examId <= 0) { + throw new IllegalArgumentException("examId must be positive"); + } return examDateService.getLatestIndividualExamEndDate(examId); }src/main/java/de/tum/cit/aet/artemis/exam/api/ExamMetricsApi.java (2)
20-24: Consider introducing a service layer.Direct repository access in the API layer bypasses potential business logic. Consider introducing an ExamMetricsService to maintain proper layering.
26-40: Add caching for date-based metrics.The date-based counting methods could benefit from caching to improve performance, especially for frequently accessed time ranges.
Example implementation:
+import org.springframework.cache.annotation.Cacheable; @Profile(PROFILE_CORE) @Controller public class ExamMetricsApi extends AbstractExamApi { + @Cacheable(value = "examMetrics", key = "'endDate_' + #minDate + '_' + #maxDate") public Integer countExamsWithEndDateBetween(ZonedDateTime minDate, ZonedDateTime maxDate) { + if (minDate == null || maxDate == null) { + throw new IllegalArgumentException("Date parameters must not be null"); + } + if (minDate.isAfter(maxDate)) { + throw new IllegalArgumentException("minDate must not be after maxDate"); + } return examRepository.countExamsWithEndDateBetween(minDate, maxDate); }src/main/java/de/tum/cit/aet/artemis/assessment/web/ParticipantScoreResource.java (1)
97-99: Consider extracting the API retrieval to a private method.The pattern
examApi.orElseThrow(() -> new ApiNotPresentException(ExamApi.class, PROFILE_CORE))is likely to be reused across methods. Consider extracting it to improve code reuse and maintainability.+ private ExamApi getExamApi() { + return examApi.orElseThrow(() -> new ApiNotPresentException(ExamApi.class, PROFILE_CORE)); + } public ResponseEntity<List<ScoreDTO>> getScoresOfExam(@PathVariable Long examId) { long start = System.currentTimeMillis(); log.debug("REST request to get exam scores for exam : {}", examId); - var api = examApi.orElseThrow(() -> new ApiNotPresentException(ExamApi.class, PROFILE_CORE)); - - Exam exam = api.findByIdWithExamUsersExerciseGroupsAndExercisesElseThrow(examId); + Exam exam = getExamApi().findByIdWithExamUsersExerciseGroupsAndExercisesElseThrow(examId);src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseScheduleService.java (1)
156-159: Consider extracting the API retrieval to a private method.The pattern
examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE))is likely to be reused. Consider extracting it to improve code reuse and maintainability.+ private ExamDateApi getExamDateApi() { + return examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); + } private void scheduleExamExercise(ModelingExercise exercise) { - var api = examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); + var api = getExamDateApi();src/main/java/de/tum/cit/aet/artemis/assessment/web/GradeStepResource.java (1)
110-111: Consider extracting the API retrieval to a private method.The pattern
examApi.orElseThrow(() -> new ApiNotPresentException(ExamApi.class, PROFILE_CORE))is duplicated across methods. Consider extracting it to improve code reuse and maintainability.+ private ExamApi getExamApi() { + return examApi.orElseThrow(() -> new ApiNotPresentException(ExamApi.class, PROFILE_CORE)); + } public ResponseEntity<GradeStepsDTO> getAllGradeStepsForExam(@PathVariable Long courseId, @PathVariable Long examId) { log.debug("REST request to get all grade steps for exam: {}", examId); - var api = examApi.orElseThrow(() -> new ApiNotPresentException(ExamApi.class, PROFILE_CORE)); + var api = getExamApi(); public ResponseEntity<GradeDTO> getGradeStepByPercentageForExam(@PathVariable Long courseId, @PathVariable Long examId, @RequestParam Double gradePercentage) { log.debug("REST request to get grade step for grade percentage {} for exam: {}", gradePercentage, examId); - var api = examApi.orElseThrow(() -> new ApiNotPresentException(ExamApi.class, PROFILE_CORE)); + var api = getExamApi();Also applies to: 219-220
src/main/java/de/tum/cit/aet/artemis/assessment/web/BonusResource.java (1)
101-102: Consider extracting the API retrieval to a private method.The pattern
examAccessApi.orElseThrow(() -> new ApiNotPresentException(ExamAccessApi.class, PROFILE_CORE))is duplicated across multiple methods. Consider extracting it to improve code reuse and maintainability.+ private ExamAccessApi getExamAccessApi() { + return examAccessApi.orElseThrow(() -> new ApiNotPresentException(ExamAccessApi.class, PROFILE_CORE)); + } public ResponseEntity<Bonus> getBonusForExam(@PathVariable Long courseId, @PathVariable Long examId, @RequestParam(required = false) boolean includeSourceGradeSteps) { log.debug("REST request to get bonus for exam: {}", examId); - var api = examAccessApi.orElseThrow(() -> new ApiNotPresentException(ExamAccessApi.class, PROFILE_CORE)); - api.checkCourseAndExamAccessForStudentElseThrow(courseId, examId); + getExamAccessApi().checkCourseAndExamAccessForStudentElseThrow(courseId, examId);Also applies to: 145-146, 175-176, 242-243, 292-293
src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java (1)
159-160: Consider extracting API retrieval to a helper method.The API retrieval and error handling pattern might be reused elsewhere in the class. Consider extracting it to a private helper method to improve reusability and maintainability.
+ private ExamDateApi getExamDateApi() { + return examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); + } if (isExamMode && !isAtLeastInstructor) { final Exam exam = exercise.getExerciseGroup().getExam(); - var api = examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); - ZonedDateTime latestExamDueDate = api.getLatestIndividualExamEndDate(exam.getId()); + ZonedDateTime latestExamDueDate = getExamDateApi().getLatestIndividualExamEndDate(exam.getId());src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (1)
537-538: Consider extracting API retrieval to a helper method.Similar to the suggestion in AssessmentService, the API retrieval and error handling pattern could be extracted to a helper method.
+ private ExamAccessApi getExamAccessApi() { + return examAccessApi.orElseThrow(() -> new ApiNotPresentException(ExamAccessApi.class, PROFILE_CORE)); + } if (programmingExercise.isExamExercise()) { - var api = examAccessApi.orElseThrow(() -> new ApiNotPresentException(ExamAccessApi.class, PROFILE_CORE)); - api.checkExamExerciseForExampleSolutionAccessElseThrow(programmingExercise); + getExamAccessApi().checkExamExerciseForExampleSolutionAccessElseThrow(programmingExercise); }src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java (1)
360-361: Consider extracting API retrieval to a helper method.Similar to the suggestions in other files, the API retrieval and error handling pattern could be extracted to a helper method.
+ private StudentExamApi getStudentExamApi() { + return studentExamApi.orElseThrow(() -> new ApiNotPresentException(StudentExamApi.class, PROFILE_CORE)); + } - var api = studentExamApi.orElseThrow(() -> new ApiNotPresentException(StudentExamApi.class, PROFILE_CORE)); + var api = getStudentExamApi();src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java (1)
717-719: Consider extracting API retrieval to a helper method.Similar to the suggestions in other files, the API retrieval and error handling pattern could be extracted to a helper method.
+ private ExamDateApi getExamDateApi() { + return examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); + } if (originalQuizExercise.isExamExercise()) { - var api = examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); - ZonedDateTime latestIndividualExamEndDate = api.getLatestIndividualExamEndDate(originalQuizExercise.getExerciseGroup().getExam()); + ZonedDateTime latestIndividualExamEndDate = getExamDateApi().getLatestIndividualExamEndDate(originalQuizExercise.getExerciseGroup().getExam());
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (54)
src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/service/ComplaintService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/web/BonusResource.java(9 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/web/GradeStepResource.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/web/GradingScaleResource.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/web/ParticipantScoreResource.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultWebsocketService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/MetricsBean.java(10 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/websocket/WebsocketConfiguration.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/core/exception/ApiNotPresentException.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/service/AuthorizationCheckService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java(16 hunks)src/main/java/de/tum/cit/aet/artemis/core/service/export/CourseExamExportService.java(8 hunks)src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportExamCreationService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/core/web/FileResource.java(7 hunks)src/main/java/de/tum/cit/aet/artemis/exam/api/AbstractExamApi.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/api/ExamAccessApi.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/api/ExamApi.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/api/ExamDateApi.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/api/ExamDeletionApi.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/api/ExamLiveEventsApi.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/api/ExamMetricsApi.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/api/ExamSubmissionApi.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/api/ExamUserApi.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/api/ExerciseGroupApi.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/api/StudentExamApi.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDateService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseResource.java(6 hunks)src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadSubmissionService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseScheduleService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingSubmissionService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseScheduleService.java(10 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingSubmissionService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingAssessmentResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java(6 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizPoolService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java(6 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizPoolResource.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizSubmissionResource.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/text/service/TextAssessmentService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/text/service/TextSubmissionService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/text/web/TextSubmissionResource.java(4 hunks)src/test/java/de/tum/cit/aet/artemis/exam/architecture/ExamApiArchitectureTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/shared/architecture/module/AbstractModuleAccessArchitectureTest.java(3 hunks)
🧰 Additional context used
📓 Path-based instructions (54)
src/main/java/de/tum/cit/aet/artemis/core/exception/ApiNotPresentException.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exam/api/AbstractExamApi.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamUserApi.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamLiveEventsApi.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/java/de/tum/cit/aet/artemis/exam/architecture/ExamApiArchitectureTest.java (1)
Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
src/main/java/de/tum/cit/aet/artemis/assessment/web/GradeStepResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exam/api/ExerciseGroupApi.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingAssessmentResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/assessment/service/ComplaintService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/assessment/web/GradingScaleResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultWebsocketService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDateService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/config/websocket/WebsocketConfiguration.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizPoolService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/java/de/tum/cit/aet/artemis/shared/architecture/module/AbstractModuleAccessArchitectureTest.java (1)
Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
src/main/java/de/tum/cit/aet/artemis/assessment/web/ParticipantScoreResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamDateApi.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamSubmissionApi.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/web/FileResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportExamCreationService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/text/service/TextSubmissionService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/text/service/TextAssessmentService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/text/web/TextSubmissionResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamAccessApi.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/assessment/web/BonusResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingSubmissionService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseScheduleService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizSubmissionResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/service/AuthorizationCheckService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamDeletionApi.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/service/export/CourseExamExportService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadSubmissionService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizPoolResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamApi.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exam/api/StudentExamApi.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/config/MetricsBean.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamMetricsApi.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingSubmissionService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseScheduleService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
📓 Learnings (10)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java (1)
Learnt from: Hialus
PR: ls1intum/Artemis#8607
File: src/main/java/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseResource.java:64-64
Timestamp: 2024-11-12T12:51:51.201Z
Learning: For the Artemis project, import statements are automatically managed by formatters and should not be commented on.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (1)
Learnt from: Hialus
PR: ls1intum/Artemis#8607
File: src/main/java/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseResource.java:64-64
Timestamp: 2024-11-12T12:51:51.201Z
Learning: For the Artemis project, import statements are automatically managed by formatters and should not be commented on.
src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java (1)
Learnt from: julian-christl
PR: ls1intum/Artemis#9322
File: src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java:0-0
Timestamp: 2024-11-12T12:51:51.201Z
Learning: In Artemis, an `ExerciseGroup` always has an associated `Exam`, so `exerciseGroup.exam` is never null.
src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportExamCreationService.java (1)
Learnt from: florian-glombik
PR: ls1intum/Artemis#7588
File: src/main/java/de/tum/in/www1/artemis/service/exam/ExamService.java:424-431
Timestamp: 2024-11-12T12:51:40.391Z
Learning: The grade is not relevant for unsubmitted exams, and no additional checks for submission status are needed when calculating exam scores as a bonus source.
src/main/java/de/tum/cit/aet/artemis/text/service/TextSubmissionService.java (1)
Learnt from: julian-christl
PR: ls1intum/Artemis#9322
File: src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java:0-0
Timestamp: 2024-11-12T12:51:51.201Z
Learning: In Artemis, an `ExerciseGroup` always has an associated `Exam`, so `exerciseGroup.exam` is never null.
src/main/java/de/tum/cit/aet/artemis/text/web/TextSubmissionResource.java (1)
Learnt from: florian-glombik
PR: ls1intum/Artemis#7588
File: src/main/java/de/tum/in/www1/artemis/service/exam/ExamService.java:424-431
Timestamp: 2024-11-12T12:51:40.391Z
Learning: The grade is not relevant for unsubmitted exams, and no additional checks for submission status are needed when calculating exam scores as a bonus source.
src/main/java/de/tum/cit/aet/artemis/assessment/web/BonusResource.java (2)
Learnt from: Strohgelaender
PR: ls1intum/Artemis#8030
File: src/main/java/de/tum/in/www1/artemis/service/CourseScoreCalculationService.java:21-49
Timestamp: 2024-11-12T12:52:03.805Z
Learning: The project consistently avoids using star-imports in Java files.
Learnt from: florian-glombik
PR: ls1intum/Artemis#7588
File: src/main/java/de/tum/in/www1/artemis/service/exam/ExamService.java:424-431
Timestamp: 2024-11-12T12:51:40.391Z
Learning: The grade is not relevant for unsubmitted exams, and no additional checks for submission status are needed when calculating exam scores as a bonus source.
src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingSubmissionService.java (1)
Learnt from: julian-christl
PR: ls1intum/Artemis#9322
File: src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java:0-0
Timestamp: 2024-11-12T12:51:51.201Z
Learning: In Artemis, an `ExerciseGroup` always has an associated `Exam`, so `exerciseGroup.exam` is never null.
src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadSubmissionService.java (1)
Learnt from: florian-glombik
PR: ls1intum/Artemis#7588
File: src/main/java/de/tum/in/www1/artemis/service/exam/ExamService.java:424-431
Timestamp: 2024-11-12T12:51:40.391Z
Learning: The grade is not relevant for unsubmitted exams, and no additional checks for submission status are needed when calculating exam scores as a bonus source.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingSubmissionService.java (1)
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#9303
File: src/main/java/de/tum/in/www1/artemis/service/exam/StudentExamService.java:296-300
Timestamp: 2024-11-12T12:51:40.391Z
Learning: When reviewing code changes in `StudentExamService.saveSubmission`, if the PR aims to improve readability without changing logic, avoid suggesting changes that alter logic, such as adding exceptions in the default case of switch statements.
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (158)
src/main/java/de/tum/cit/aet/artemis/text/service/TextAssessmentService.java (3)
26-26: LGTM!The import changes align with the module API introduction and follow Java naming conventions.
40-44: LGTM!The constructor changes properly implement:
- Constructor injection
- Optional API dependency
- Consistent naming conventions
45-46: LGTM!The super constructor call correctly propagates the optional API dependency.
src/main/java/de/tum/cit/aet/artemis/core/web/FileResource.java (4)
50-50: LGTM!The new imports are correctly added and necessary for the API-based implementation.
Also applies to: 65-65
120-120: LGTM!The change from repository to Optional API aligns with the modularization effort and follows best practices for optional dependencies.
136-136: LGTM!The constructor changes maintain proper dependency injection and correctly initialize the optional API.
Also applies to: 148-148
409-411: LGTM!The methods correctly handle the optional API with proper error handling. The implementation is consistent across both methods.
Also applies to: 427-429
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java (2)
29-29: LGTM! Import change aligns with modularization.The change from
ExamDateServicetoExamDateApialigns with the PR objective of server modularization.
50-50: LGTM! Constructor changes follow DI best practices.The changes demonstrate good practices:
- Using
Optionalfor optional dependencies- Following constructor injection pattern
- Properly propagating the API dependency to the superclass
Also applies to: 55-55
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultWebsocketService.java (3)
19-19: LGTM!The new imports align with the module API introduction and follow Java import guidelines.
Also applies to: 21-21
38-38: LGTM!The field declaration follows best practices:
- Uses
Optionalfor API dependency, enabling module isolation- Follows least access principle with
private finalmodifiers
46-50: LGTM!The constructor changes follow best practices:
- Uses constructor injection as required
- Properly handles the optional API dependency
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java (4)
60-65: LGTM!The imports are correctly organized and follow the coding guidelines.
133-133: LGTM!The field declaration correctly uses
Optional<ExamLiveEventsApi>for handling the optional API dependency, aligning with the server modularization effort.
145-145: LGTM!The constructor parameter is correctly updated to accept the optional API dependency.
167-167: LGTM!The field initialization correctly assigns the optional API parameter.
src/test/java/de/tum/cit/aet/artemis/shared/architecture/module/AbstractModuleAccessArchitectureTest.java (1)
3-4: LGTM!The added imports are necessary and properly organized.
Also applies to: 10-11
src/main/java/de/tum/cit/aet/artemis/exam/api/ExerciseGroupApi.java (2)
1-10: LGTM! Well-structured package and imports.The package structure aligns with the modularization effort, and imports follow best practices with no star imports.
15-19: LGTM! Proper dependency injection pattern.The code follows best practices with constructor injection and proper encapsulation of the repository dependency.
src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingSubmissionService.java (2)
32-32: LGTM!The import of
ExamDateApialigns with the server modularization effort.
69-73: Verify exam date API integration.The change from
ExamDateServicetoOptional<ExamDateApi>is correct and aligns with the modularization effort. However, we should verify the exam date functionality in exam-related methods.Let's verify the exam date API usage:
✅ Verification successful
Exam Date API integration verified.
The search confirms that the Optional is consistently used with proper error handling (e.g., using orElseThrow with an ApiNotPresentException) across exam-related methods—including in the ModelingSubmissionService and its related functionality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if exam date API is properly used in exam-related methods # Search for exam date API usage ast-grep --pattern 'examDateApi.orElseThrow($$$)' # Search for potential missing error handling rg -A 5 'examDateApi' | rg -v 'orElseThrow|isEmpty|isPresent'Length of output: 43794
src/main/java/de/tum/cit/aet/artemis/core/service/AuthorizationCheckService.java (5)
28-28: Import of ApiNotPresentException looks well-structured
No issues. This exception import clearly aligns with the usage in later lines.
33-33: New dependency import aligns with the exam API approach
No concerns with importingExamDateApi. The naming is consistent with existing conventions.
62-62: Constructor injection is in line with best practices
ProvidingexamDateApivia the constructor complies with recommended dependency injection principles.
65-65: Assigning the new optional dependency
Straightforward assignment; no issues spotted.
822-823: Optional API usage is consistent, but consider fallback
ThrowingApiNotPresentExceptionwhen the exam API is absent makes sense for mandatory exam checks. If partial functionality is needed in the future, consider a fallback or no-op.src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingAssessmentResource.java (3)
63-70: LGTM! Constructor changes align with modularization goals.The removal of
ExamServicedependency is consistent with the PR's objective of modularizing exam functionality. The constructor maintains proper dependency injection practices.
48-51: LGTM! REST endpoints follow best practices.The implementation demonstrates:
- Proper HTTP method usage and status codes
- Comprehensive security through @EnforceAtLeast* annotations
- Thorough input validation and error handling
- Clear separation of concerns
Also applies to: 79-106, 115-119, 129-190, 200-205
63-70: Changes successfully support exam module API introduction.The removal of direct
ExamServicedependency aligns perfectly with the PR's objective of introducing a module API for exam functionality as part of the server modularization effort.src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java (7)
30-30: Import addition looks good.
No issues found; this import is necessary for the new optional exam API approach.
39-40: New exam-related imports align with the introduced optional APIs.
They are consistent with the shift to API-based exam handling, providing clearer modular boundaries.
101-102: Introduction ofOptional<ExamApi>is consistent.
This aligns with the broader transition away from direct repository usage, ensuring the exam logic is accessed through an API.
107-108: Constructor signature changes are appropriately refactored.
This ensures dependencies forExamApiandStudentExamApiare handled in a single place, adhering to constructor injection guidelines.
119-119: Assignment ofstudentExamApiis straightforward.
No issues; the logic is consistent with the new fields.
123-123: Assignment ofexamApiis consistent.
No issues; follows the same pattern as other optional dependencies.
530-531: Ensure test coverage for absent APIs.
ThrowingApiNotPresentExceptionon absentexamApiorstudentExamApiis correct, but verify that a scenario where these APIs are missing is tested or handled gracefully in the calling code.src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java (4)
42-42: LGTM! Import changes align with the module API introduction.The imports are correctly updated to use the new API-based approach, removing the service dependency and adding the necessary API and exception imports.
Also applies to: 54-54
89-89: LGTM! Field declaration follows best practices.The field is correctly declared as final and uses Optional to handle the potential absence of the ExamDateApi, which aligns with the module API approach.
103-109: LGTM! Constructor injection is properly implemented.The constructor correctly injects the Optional and maintains immutability by assigning it to the final field.
1-389: Verify the impact of API changes on error handling.The transition from direct service usage to Optional introduces new error scenarios. Please ensure that:
- The UI properly handles ApiNotPresentException
- Error messages are clear when the exam module is not available
src/main/java/de/tum/cit/aet/artemis/exam/api/AbstractExamApi.java (1)
1-6: Verify architectural compliance.The class structure aligns well with the server modularization effort:
- Proper package structure under
exam.api- Extends
AbstractApifor consistent API hierarchy- Follows facade pattern for service/repository declarations
However, to ensure complete architectural compliance, verify:
- All exam-related operations are properly abstracted through this API
- No direct repository access bypasses this facade
✅ Verification successful
Architectural compliance verified.
- The abstract API in the correct package properly extends the global API interface.
- Production code consistently injects and uses the exam API via
Optional<ExamApi>, ensuring exam-related operations are abstracted.- Direct repository accesses outside the exam module appear confined to test cases, which is acceptable.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that exam-related operations are properly abstracted # and no direct repository access bypasses the facade # Check for direct repository usage outside the exam module rg -t java "ExamRepository|ExamDateRepository" --glob "!src/main/java/de/tum/cit/aet/artemis/exam/**" # Check for proper API usage pattern rg -t java "Optional<.*ExamApi>" -A 3Length of output: 55774
src/main/java/de/tum/cit/aet/artemis/core/config/websocket/WebsocketConfiguration.java (7)
61-61: No concerns about the new import.
ImportingApiNotPresentExceptionaligns with the introduction of the modular exam API approach.
67-67: ExamApi import looks good.
This import correctly references the newly introduced modular exam API.
95-95: Field injection with an Optional is consistent with the design.
UsingOptional<ExamApi>for a conditionally present exam module is appropriate for modularity.
109-109: Constructor signature change is properly handled.
Switching from a direct repository toOptional<ExamApi>embraces the new exam API abstraction.
116-116: Assignment of examApi is straightforward.
No issues with setting the field to the optional parameter.
328-328: Throwing ApiNotPresentException is appropriate.
Ensures that any exam-related functionality fails fast if the exam module is absent.
329-329: Validate DB call overhead in subscription check.
Although allowed for specific topics, verify that callingfindByIdElseThroweach time does not cause performance issues when many subscriptions occur.src/main/java/de/tum/cit/aet/artemis/text/web/TextSubmissionResource.java (5)
28-28: Import aligned with new exception handling approach
No issues here; importingApiNotPresentExceptionis consistent with the newly introduced optional API mechanism.
36-36: New import for ExamSubmissionApi looks good
This follows the refactor to use the exam module’s abstraction.
87-88: Constructor injection of the optional API is consistent
The constructor properly reflects the new dependency injection for the optional exam submission API.
98-98: Duplicate comment regarding Optional field assignment
The assignment is valid, but the same considerations about storing anOptional<>as a field (commented at line 80) apply here as well.
145-153: Verify concurrency handling and prevent possible double submission
The logic to throwApiNotPresentExceptionif missing and to check for multiple submissions is a good addition. However, ensure that concurrency is handled at the database level if multiple submissions come in simultaneously, as simple checks might still allow duplicates under race conditions. Also verify no redundant checks withtextSubmissionService.checkSubmissionAllowanceElseThrow().src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportExamCreationService.java (5)
25-27: No issues with new import statements.
Everything looks good here, and no star imports are used, which aligns with the guidelines.
43-43: Use of Optional forstudentExamApiis consistent.
Theprivate final Optional<StudentExamApi>field adheres to the least-access principle and constructor injection guidelines.
47-47: Use of Optional forexamApiis valid.
No immediate issues identified. This is in line with handling optional modules gracefully.
51-55: Constructor injection follows best practices.
Good job applying constructor-based dependency injection, matching the mandateddi:constructor_injectionguideline.
124-125: Use ofexamApi.orElseThrowis coherent with the new optional-API approach.
Gracefully handling the absence ofExamApiby throwing a specific exception clarifies error handling.src/main/java/de/tum/cit/aet/artemis/text/service/TextSubmissionService.java (4)
26-26: LGTM!The import change from
ExamDateServicetoExamDateApialigns with the server modularization effort.
58-59: LGTM!The super constructor call correctly propagates the API transition to the parent class.
56-57: LGTM! Verify error handling for optional API.The change to
Optional<ExamDateApi>aligns with the modularization effort. Since the API is now optional, ensure proper error handling when the API is not present.Run the following script to verify error handling:
✅ Verification successful
LGTM! Error handling for the optional ExamDateApi is implemented correctly.
Multiple services—including AssessmentService, ResultWebsocketService, ResultResource, SubmissionService, AuthorizationCheckService, ModelingExerciseScheduleService, ExerciseDateService, ExerciseResource, QuizExerciseResource, and ProgrammingExerciseScheduleService—consistently use theorElseThrowpattern to throw anApiNotPresentExceptionwhen the API is missing, and there are no unsafe calls toexamDateApi.get(). This confirms that the dependency is safely handled wherever it’s used.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for proper error handling of optional ExamDateApi # Search for ApiNotPresentException handling rg -A 5 "examDateApi.orElseThrow\(\s*\(\s*\)\s*->\s*new\s*ApiNotPresentException" . # Search for direct usage without proper checks rg "examDateApi.get\(\)" .Length of output: 9714
81-89: Verify exam-related functionality with the new API.Since this code handles exam exercises differently (e.g., due date checks and submission handling), ensure that the transition to
ExamDateApimaintains the existing exam-specific behavior.Run the following script to verify exam-related functionality:
✅ Verification successful
Exam functionality is preserved with the new ExamDateApi integration.
After verifying the codebase, we found that exam-specific conditions still rely on checking
exercise.isExamExercise()and related API calls. In particular, the text submission logic sets submissions as submitted for exam exercises (regardless of due dates), and the new ExamDateApi is actively used in services such as ExerciseDateService, SubmissionService, and several exam-specific utility classes. No evidence was found that the exam-related behavior has been compromised by the API transition.
- In
TextSubmissionService.java, exam exercises bypass the usual due-date restrictions, correctly marking submissions as submitted.- Multiple components (e.g., in the assessment, exercise, and exam modules) confirm that checks and operations for exam exercises are routed through the new ExamDateApi.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for exam-specific functionality that might be affected by the API change # Search for exam-specific conditions and API usage rg -A 5 "exercise.isExamExercise\(\)" . # Search for exam date checks rg -A 5 "getDueDate.*exam" .Length of output: 48682
src/main/java/de/tum/cit/aet/artemis/core/service/export/CourseExamExportService.java (11)
36-36: Import usage forApiNotPresentExceptionis valid.
No issues with adding a specific import for this custom exception.
40-40: New import forExamApiis consistent.
This aligns with the shift from using direct repository access to API-based access.
81-81: Optional-based dependency injection.
Storing theExamApiin anOptionalis a suitable design choice for handling the possible absence of the module at runtime.
89-89: Constructor injection ofOptional<ExamApi>.
Replaces the repository dependency with the new API injection. This maintains the constructor injection principle.
98-98: Assignment ofthis.examApi.
Straightforward assignment of the optional API field.
182-183: Retrieving the API or throwingApiNotPresentException.
This approach ensures the workflow halts gracefully if the exam module is not available.
208-208: Invokingapi.findAllExercisesWithDetailsByExamId.
Fetching exam exercises via the new API call is consistent with the modularization effort.
237-238: Consistent pattern for retrieving the exam API.
Checks for presence or throws to avoid null references.
245-245:api.findByCourseIdusage.
Replaces direct repository calls, aligning with the decoupled design.
312-313: Ensuring API availability inexportExams.
Same pattern of safely retrieving theExamApias in earlier methods.
333-334: Fetching and exporting exam exercises.
Logical flow: retrieve exercises from the new API, then pass them along toexportExam. No immediate issues noted.src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizPoolResource.java (2)
5-5: LGTM!The import statements are well-organized and follow Java conventions.
Also applies to: 25-25
50-57: LGTM!The changes effectively implement constructor injection and modular API design:
- Uses constructor injection as per guidelines
- Properly handles optional dependency
- Maintains single responsibility
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamLiveEventsApi.java (3)
1-11: LGTM! Clean imports and package structure.The imports are specific and well-organized, following the coding guidelines.
16-20: LGTM! Clean dependency injection pattern.The implementation follows best practices:
- Uses constructor injection as per guidelines
- Follows least access principle with private final field
12-14: LGTM! Class structure aligns with API design pattern.The class is properly configured as a Spring controller and follows the inheritance hierarchy.
Let's verify the base class implementation:
✅ Verification successful
LGTM! The base class implementation in
src/main/java/de/tum/cit/aet/artemis/exam/api/AbstractExamApi.javaconfirms thatExamLiveEventsApicorrectly extends a validAbstractExamApithat implements theAbstractApiinterface without additional complexity.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the base class implementation ast-grep --pattern 'class AbstractExamApi { $$$ }'Length of output: 178
Script:
#!/bin/bash # Search for AbstractExamApi.java in the main source directory to avoid UTF-8 issues in test files echo "Searching for AbstractExamApi.java in src/main/java..." fd "AbstractExamApi.java" src/main/java # For each file found, display the first 30 lines to inspect the base class content for file in $(fd "AbstractExamApi.java" src/main/java); do echo "----------------------------------------" echo "Inspecting file: $file" head -n 30 "$file" echo "----------------------------------------" doneLength of output: 918
src/test/java/de/tum/cit/aet/artemis/exam/architecture/ExamApiArchitectureTest.java (2)
1-7: LGTM! Clean and well-organized imports.The package structure and imports are minimal, specific, and well-organized.
10-13: Verify the ARTEMIS_PACKAGE constant.The implementation looks correct, but let's verify the ARTEMIS_PACKAGE constant definition.
✅ Verification successful
ARTEMIS_PACKAGE constant correctly defined in shared abstract test class
- The constant is defined in
src/test/java/de/tum/cit/aet/artemis/shared/architecture/AbstractArchitectureTest.javaas"de.tum.cit.aet.artemis".ExamApiArchitectureTest(which extendsAbstractModuleAccessArchitectureTest) uses it as part of its return value, yielding"de.tum.cit.aet.artemis.exam".🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for ARTEMIS_PACKAGE constant definition ast-grep --pattern 'class AbstractModuleAccessArchitectureTest { $$$ $_ ARTEMIS_PACKAGE = $_; $$$ }'Length of output: 232
Script:
#!/bin/bash # Search for "ARTEMIS_PACKAGE" definition in all Java files, excluding problematic test data files. rg -n "ARTEMIS_PACKAGE" --type java --glob "!.*/test-data/*"Length of output: 12846
Script:
#!/bin/bash # Check the inheritance of ExamApiArchitectureTest to see which base class provides ARTEMIS_PACKAGE. rg -n "class ExamApiArchitectureTest" src/test/java/de/tum/cit/aet/artemis/exam/architecture/ExamApiArchitectureTest.javaLength of output: 202
src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadSubmissionService.java (3)
36-36: LGTM!The import change aligns with the modularization effort and follows Java import guidelines.
63-67: LGTM! Constructor changes follow best practices.The changes demonstrate good practices by:
- Using constructor injection
- Making the API dependency optional and explicit
- Properly propagating the optional dependency to the superclass
94-94: Verify exam-related functionality with the new API.The comment indicates special handling for exam exercises. Let's verify that the exam date checks still work correctly with the new Optional.
✅ Verification successful
Exam-related functionality is verified and remains correct.
The investigation confirmed that:
- The FileUploadSubmissionService correctly extends SubmissionService, which applies separate handling for exam exercises.
- Exam exercises are properly identified using the
isExamExercise()check (e.g., in the conditionif (exercise.isExamExercise() || exerciseDateService.isBeforeDueDate(participation))), ensuring that due date checks do not interfere with exam submissions.- The new usage of
Optional<ExamDateApi>is consistent across related services and does not adversely affect exam date checks.No modifications are needed to maintain exam-related functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for exam-related date checks in the superclass to verify correct API usage ast-grep --pattern 'examDateApi.orElseThrow($$$).get$$$' # Search for potential issues with exam date handling rg -A 5 'isExamExercise.*getDueDate|getDueDate.*isExamExercise'Length of output: 248
Script:
#!/bin/bash # Search for exam API usage across the repository to ensure proper exam-related handling echo "Searching for examDateApi usage:" rg -i "examDateApi" || true echo -e "\nSearching for Optional<ExamDateApi> usage:" rg -i "Optional<ExamDateApi>" || true echo -e "\nSearching for exam exercise checks (isExamExercise):" rg -i "isExamExercise" || true echo -e "\nListing the declaration of FileUploadSubmissionService to check its parent class:" grep "class FileUploadSubmissionService" -R src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadSubmissionService.java || true echo -e "\nListing Java files with 'Exam' in their file name for further inspection:" fd -HI "Exam" -e java || trueLength of output: 56221
src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseResource.java (4)
37-37: LGTM! Import changes align with modularization goals.The new imports support the transition from concrete service implementations to API interfaces, which aligns with the PR's server modularization objectives.
Also applies to: 45-46
90-90: LGTM! Field declarations follow best practices.The use of
Optional<>for API dependencies:
- Follows constructor injection principle
- Makes optional dependencies explicit
- Enables graceful handling of missing API implementations
Also applies to: 102-102
109-109: LGTM! Constructor changes maintain clean dependency injection.The constructor properly initializes the optional API dependencies while maintaining the existing initialization order.
Also applies to: 112-112, 122-122, 127-127
351-356: Remove temporary ICER paper feature flag.The code contains a temporary feature flag for a paper, which should be reviewed for removal.
Let's check if this feature flag is still needed:
Consider removing this temporary code or adding a TODO with a timeline for removal if still needed.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseScheduleService.java (6)
3-3: All new imports for optional exam APIs
These imports cleanly introduce the optional exam APIs and the newApiNotPresentException. They adhere to the no-star-import rule and are consistent with the evolving modular design.Also applies to: 15-15, 45-45, 50-52
98-102: Optional fields are consistent with modular design
DefiningexamApi,studentExamApi, andexamDateApiasOptionalfields is a suitable approach to handle absent APIs gracefully. The final fields coupled with constructor injection satisfy dependency-injection best practices.
123-125: Fields assignment is straightforward
Assigning the optional fields here is aligned with the constructor injection pattern, ensuring clarity in how these dependencies are injected and used.
421-422: Conditional throw for missing ExamDateApi
UsingorElseThrowwithPROFILE_COREindicates that the exam date API is mandatory under the core profile. Confirm that tying this specifically toPROFILE_COREmeets future requirements, e.g., if an exam-specific profile is introduced.
431-432: Scheduling backup logic during an ongoing exam
This branch handles the case where we're inside the exam conduction window. Consider extending test coverage for edge cases (e.g., sudden time extensions) to ensure this logic always triggers as intended.
441-441: Rescheduling exercises for deployment during exams
The call torescheduleProgrammingExerciseDuringExamConductionis made if the exam is still in progress, which is crucial for extended working times. Verify there are no concurrency issues if multiple scheduling updates happen simultaneously.src/main/java/de/tum/cit/aet/artemis/exam/api/ExamApi.java (1)
46-48: Check custom exception handling behavior.Calling
examRepository.findByIdElseThrow(id)implies throwing a custom exception when the exam is not found. Ensure that Spring maps this exception correctly (e.g., an appropriate HTTP status for REST clients) and that downstream classes handle it properly.src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDateService.java (2)
27-34: Handle missing exam-related APIs consistently.Storing
Optional<ExamDateApi>andOptional<StudentExamApi>in fields is workable, but ensure that every consumer method gracefully handles absent APIs or throws well-defined exceptions to signal incomplete functionality.
81-86: Validate edge cases for exam time checks.When
examDateApiis present, the logic delegates toisIndividualExerciseWorkingPeriodOverorisExamWithGracePeriodOver. Verify that edge cases like time zone differences, minimal exam durations, or boundary conditions (e.g., exam start equals current time) are tested and handled.src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizPoolService.java (2)
50-57: Confirm that using Optional aligns with module design.The
Optional<ExamApi>pattern is consistent with making the exam module optional. Still, verify that all quiz-related features (including updates and retrievals) handle the absent case gracefully or a dedicated fallback is provided.
68-69: Ensure a suitable error response on missing or invalid exam.By calling
examApi.orElseThrow(…)and thenfindByIdElseThrow(examId), an exception is thrown if the exam is not found or the API is absent. Confirm that your error-handling logic properly conveys the root cause (missing exam vs. missing API) to the user or client.src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java (2)
24-26: Imports for new API-based exception and interface look good
No issues found with the addition ofApiNotPresentExceptionandStudentExamApiimports.
64-64: Optional-based constructor injection is correctly applied
UsingOptional<StudentExamApi>with constructor injection aligns with DI best practices. No concerns noted regarding usage.Also applies to: 88-88, 98-98
src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizSubmissionResource.java (3)
6-6: New imports for optional exam submission API
No issues with addingOptionalandApiNotPresentExceptionimports. They facilitate the new approach well.Also applies to: 28-28, 36-36
77-77: Optional examSubmissionApi injection
Constructor injection ofOptional<ExamSubmissionApi>is consistent with the new architecture. The assignment appears correct.Also applies to: 80-80, 88-88
242-250: Conditional exam submission handling
The usage ofexamSubmissionApi.orElseThrow(...)for exam exercises and subsequent API calls is appropriate. Make sure the cast inpreventMultipleSubmissionsis guaranteed safe (i.e., it truly returns aQuizSubmission). Otherwise, the approach looks solid.src/main/java/de/tum/cit/aet/artemis/assessment/web/GradingScaleResource.java (4)
32-32: Added imports for API-based exam handling
No concerns. These imports introduceApiNotPresentExceptionandExamApiproperly.Also applies to: 40-40
64-64: Optional examApi constructor injection
Injecting the exam API as anOptionalaligns with the new modular approach. Implementation looks consistent.Also applies to: 70-70, 75-75
179-179: API usage in createGradingScaleForExam
ThrowingApiNotPresentExceptionwhen the API is absent is a clear error-handling strategy. The calls to find and save the exam viaexamApilook correct. Consider verifying that the exam truly belongs to the retrieved course if relevant.Also applies to: 185-185, 188-188
233-233: API usage in updateGradingScaleForExam
The same pattern of retrieving the API, loading the exam, and saving changes holds well. Ensure the exam-course consistency is verified if necessary.Also applies to: 236-236, 243-243
src/main/java/de/tum/cit/aet/artemis/assessment/service/ComplaintService.java (5)
30-30: Introduce ApiNotPresentExceptionImporting
ApiNotPresentExceptionhere aligns with the new modular error handling strategy.
33-33: ExamApi importBringing in
ExamApiis a clear step toward modularizing exam-related logic.
64-64: Constructor signature changeAdopting constructor injection for the optional API dependency preserves compatibility with the new architecture.
68-68: Explicit assignment of examApiStraightforward assignment. No issues found.
103-104: Graceful handling of missing APIUsing
orElseThrowto ensure a consistent exception path if the API isn’t present is a clean way to fail fast.src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.java (6)
34-34: ApiNotPresentException importThis import provides a unified way to handle missing dependencies.
43-43: ExamSubmissionApi importPulling in the new
ExamSubmissionApialigns with the ongoing modularization effort.
81-81: Optional examSubmissionApi fieldStoring the API as an optional is consistent with how the code now conditionally depends on exam logic.
85-85: Expanded constructor for exam logicThe updated constructor properly accommodates the new optional exam submission API.
94-94: Setting the optional fieldStraightforward assignment to store the optional dependency.
132-138: Exam submission checksGood approach for verifying submission allowances and preventing multiple submissions using the retrieved
ExamSubmissionApi.src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java (5)
37-37: ApiNotPresentException importImporting this exception ensures a uniform way to detect missing APIs.
45-45: ExamSubmissionApi importBringing in the core exam submission interface supports the new modular exam concept.
89-90: Revised constructor for examSubmissionApiWiring
Optional<ExamSubmissionApi>follows the broader architectural pattern—clean transition from a service-based to an API-based design.
95-95: Field assignmentAssigning
examSubmissionApiin the constructor is straightforward and maintains consistency throughout the class.
143-150: Exam logic integrationEnclosing exam-specific checks in an
if (exercise.isExamExercise())block and callingpreventMultipleSubmissionsfosters clarity and modular behavior.src/main/java/de/tum/cit/aet/artemis/core/config/MetricsBean.java (6)
39-47: Imports look consistent and necessary.
No issues found with these newly introduced imports for the API classes.
94-96: Introduction of optional fields for Exam & StudentExam APIs.
These finalOptional<ExamMetricsApi>andOptional<StudentExamApi>fields align with the new architecture.
181-182: Constructor signature and field assignments are correct.
The constructor properly injects and assigns the new optional API fields.Also applies to: 191-192
464-466: API usage for counting exams and exam users looks good.
The calls toapi.countExamsWithEndDateBetween(...),api.countExamUsersInExamsWithEndDateBetween(...), etc., appear correctly integrated.Also applies to: 468-469
618-618: Active exam retrieval and gauge metric settings.
Retrieving the active course exams and setting theactiveExamsGaugeandexamsGaugelogically follows the new API usage.Also applies to: 631-632
661-662: StudentExam API usage is correct.
These lines correctly retrieve and use theStudentExamApifor counting exam-specific data.Also applies to: 666-666
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingSubmissionService.java (4)
34-43: API imports introduced for exam submission checks.
The new imports cleanly separate exam submission logic from direct repository usage.
94-94: Optional field addition.
This field nicely reflects the new modular approach.
115-115: Constructor & superclass call updates.
These changes properly propagate the new exam-related APIs through the constructor and super calls.Also applies to: 120-120, 128-128
305-307: ExamSubmissionApi usage is valid.
The call toexamSubmissionApi.orElseThrow(...)references the matchingExamSubmissionApi.class.src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (9)
86-100: Newly introduced exceptions and exam-related API imports.
These imports appear consistent with the overall modularization vision.
152-160: Optional fields for exam-related services and APIs.
Good step towards decoupling the service logic from direct repository calls.
229-232: Extended constructor signature with new optional API dependencies.
Accurately handles the injection of newly introduced interfaces (ExamApi, ExamDeletionApi, etc.).
242-242: Setting the newly introduced fields in the constructor.
These assignments correctly wire up the optional exam APIs.Also applies to: 251-251, 254-254, 257-257
363-364: ExamApi usage to retrieve exams for a course.
The code logically fetches exam data viaexamApi.orElseThrow.
425-426: ExamMetricsApi / ExamApi / ExamDeletionApi usage.
The references toapi.countVisibleExams(...),api.countByCourseId(...), anddeletionApi.delete(...)appear consistent.Also applies to: 504-504, 575-581
637-638: ExerciseGroupApi usage is correct.
The call toexerciseGroupApi.orElseThrow(...)appropriately referencesExerciseGroupApi.class.
493-494:⚠️ Potential issueFix the class reference in the exception.
Again, forexamMetricsApi.orElseThrow(...), useExamMetricsApi.classin yourApiNotPresentException.-var api = examMetricsApi.orElseThrow(() -> new ApiNotPresentException(ExamApi.class, PROFILE_CORE)); +var api = examMetricsApi.orElseThrow(() -> new ApiNotPresentException(ExamMetricsApi.class, PROFILE_CORE));Likely invalid or redundant comment.
405-406:⚠️ Potential issueFix the class reference in the exception.
You are invokingexamMetricsApi.orElseThrowbut passingExamApi.class. Instead, passExamMetricsApi.class.-var api = examMetricsApi.orElseThrow(() -> new ApiNotPresentException(ExamApi.class, PROFILE_CORE)); +var api = examMetricsApi.orElseThrow(() -> new ApiNotPresentException(ExamMetricsApi.class, PROFILE_CORE));Likely invalid or redundant comment.
src/main/java/de/tum/cit/aet/artemis/core/exception/ApiNotPresentException.java (1)
6-10: LGTM! Well-documented exception class.The class is well-documented with clear JavaDoc explaining its purpose and follows the single responsibility principle.
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamDeletionApi.java (2)
12-14: LGTM! Well-structured API class.The class follows proper Spring conventions with appropriate profile and controller annotations.
16-20: LGTM! Proper constructor injection.Constructor injection is correctly implemented following the coding guidelines.
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamUserApi.java (2)
13-15: LGTM! Well-structured API class.The class follows proper Spring conventions and extends the appropriate base class.
17-21: LGTM! Proper constructor injection.Constructor injection is correctly implemented following the coding guidelines.
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamAccessApi.java (2)
11-13: LGTM! Well-structured API class.The class follows proper Spring conventions and extends the appropriate base class.
15-19: LGTM! Proper constructor injection.Constructor injection is correctly implemented following the coding guidelines.
src/main/java/de/tum/cit/aet/artemis/assessment/web/ParticipantScoreResource.java (1)
6-6: LGTM! Clean dependency injection of the exam API.The changes correctly implement the modularization by:
- Using Optional for API dependency injection
- Following constructor-based dependency injection
- Maintaining immutability with final fields
Also applies to: 38-38, 44-49
src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseScheduleService.java (1)
57-57: LGTM! Clean dependency injection of the exam date API.The changes correctly implement the modularization by:
- Using Optional for API dependency injection
- Following constructor-based dependency injection
- Maintaining immutability with final fields
Also applies to: 59-67
src/main/java/de/tum/cit/aet/artemis/assessment/web/GradeStepResource.java (1)
58-58: LGTM! Clean dependency injection of the exam API.The changes correctly implement the modularization by:
- Using Optional for API dependency injection
- Following constructor-based dependency injection
- Maintaining immutability with final fields
Also applies to: 66-77
src/main/java/de/tum/cit/aet/artemis/assessment/web/BonusResource.java (1)
71-71: LGTM! Clean dependency injection of the exam access API.The changes correctly implement the modularization by:
- Using Optional for API dependency injection
- Following constructor-based dependency injection
- Maintaining immutability with final fields
Also applies to: 77-86
src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java (1)
57-57: LGTM! Field declaration follows the new API-based approach.The change from
ExamDateServicetoOptional<ExamDateApi>aligns with the server modularization effort.src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java (2)
67-67: LGTM! The dependency injection changes look good.The changes correctly replace the direct
ExamDateServicedependency with an optionalExamDateApi, aligning with the server modularization effort.Also applies to: 97-97, 107-107
693-694: LGTM! Proper null handling for the optional API.The code correctly handles the case when the API is not present by throwing
ApiNotPresentExceptionwith appropriate profile information.src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (2)
188-188: LGTM! The dependency injection changes look good.The changes correctly replace the direct
ExamRepositorydependency with an optionalExamApi, aligning with the server modularization effort.Also applies to: 200-200, 221-221
664-671: LGTM! Proper null handling for the optional API.The code correctly handles the case when the API is not present by returning an empty set, maintaining graceful degradation of functionality.
src/test/java/de/tum/cit/aet/artemis/exam/architecture/ExamApiArchitectureTest.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/core/config/MetricsBean.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/core/config/MetricsBean.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseScheduleService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamApi.java (4)
26-37: LGTM! Consider adding class-level JavaDoc.The class structure follows best practices with proper dependency injection and access modifiers. Consider adding class-level JavaDoc to document the API's purpose and responsibilities.
Add class-level documentation:
+/** + * API facade for exam-related operations, providing a unified interface for exam management + * as part of the server modularization effort. + */ @Profile(PROFILE_CORE) @Controller public class ExamApi extends AbstractExamApi {
43-45: Consider moving static business logic to API layer.The static method delegation suggests this business logic could be owned by the API layer instead of the service layer.
Consider moving the logic to this class:
- public boolean shouldStudentSeeResult(StudentExam studentExam, StudentParticipation participation) { - return ExamService.shouldStudentSeeResult(studentExam, participation); - } + public boolean shouldStudentSeeResult(StudentExam studentExam, StudentParticipation participation) { + // Move the business logic here since it's a pure function + if (studentExam == null || participation == null) { + return false; + } + // Add the rest of the logic from ExamService + return true; // Simplified for example + }
83-86: Simplify complex stream operations for better readability and performance.The nested stream operations could be simplified for better readability and potentially better performance.
Consider this refactoring:
- public Set<Exercise> getExercisesByCourseId(long courseId) { - return examRepository.findByCourseIdWithExerciseGroupsAndExercises(courseId).stream().flatMap(e -> e.getExerciseGroups().stream()).filter(Objects::nonNull) - .map(ExerciseGroup::getExercises).flatMap(Collection::stream).collect(Collectors.toSet()); - } + public Set<Exercise> getExercisesByCourseId(long courseId) { + Set<Exercise> exercises = new HashSet<>(); + List<Exam> exams = examRepository.findByCourseIdWithExerciseGroupsAndExercises(courseId); + for (Exam exam : exams) { + for (ExerciseGroup group : exam.getExerciseGroups()) { + if (group != null) { + exercises.addAll(group.getExercises()); + } + } + } + return exercises; + }This approach:
- Is more readable and easier to debug
- May perform better by avoiding multiple stream operations
- Makes the logic flow clearer
1-88: Well-structured API implementation aligning with modularization goals.The implementation successfully creates a facade for exam operations, following coding guidelines and best practices. The class effectively encapsulates exam-related functionality while maintaining proper separation of concerns.
Consider documenting the API contract and usage patterns in the README or developer documentation to help other teams integrate with this new module API.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamApi.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamApi.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: server-style
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Analyse
…e exam module is not present this is an explicit design decision to rather falsely reject access than falsely allow access. this could occur if a future PROFILE_EXAM is not present in a multi-node setup
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (1)
421-424: Enhance the error message for better debugging.The exam access check logic is well-modularized, but the error message could be more descriptive.
Consider this improvement:
- var api = examAccessApi.orElseThrow(() -> new ApiNotPresentException(ExamAccessApi.class, PROFILE_CORE)); + var api = examAccessApi.orElseThrow(() -> new ApiNotPresentException(ExamAccessApi.class, PROFILE_CORE, "Required for checking exam access in text editor"));src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java (2)
329-332: Consider extracting API retrieval pattern.The pattern
orElseThrow(() -> new ApiNotPresentException(...))is repeated across multiple methods. Consider extracting this into a utility method to reduce duplication.+private <T> T getRequiredApi(Optional<T> api, Class<T> apiClass) { + return api.orElseThrow(() -> new ApiNotPresentException(apiClass, PROFILE_CORE)); +}Usage:
-var api = examAccessApi.orElseThrow(() -> new ApiNotPresentException(ExamAccessApi.class, PROFILE_CORE)); +var api = getRequiredApi(examAccessApi, ExamAccessApi.class);
393-458: Consider decomposing the method for better maintainability.The method is quite long and handles multiple responsibilities including:
- Participation validation
- Access control
- Exam result access checks
- Submission filtering
- Result filtering
Consider extracting these into separate methods for better maintainability and testability.
Example refactoring:
+private void validateParticipation(StudentParticipation participation) { + if (participation.getExercise() == null) { + throw new BadRequestAlertException("The exercise belonging to the participation is null.", "modelingExercise", "exerciseEmpty"); + } + if (!(participation.getExercise() instanceof ModelingExercise)) { + throw new BadRequestAlertException("The exercise of the participation is not a modeling exercise.", "modelingExercise", "wrongExerciseType"); + } +} +private List<Submission> filterSubmissionsWithResults(Set<Submission> submissions, + StudentParticipation participation, ModelingExercise exercise, boolean isStudent) { + return submissions.stream() + .filter(submission -> { + submission.setParticipation(participation); + List<Result> filteredResults = filterResults(submission.getResults(), exercise, isStudent); + if (!filteredResults.isEmpty()) { + submission.setResults(filteredResults); + return true; + } + return false; + }) + .toList(); +} +private List<Result> filterResults(Set<Result> results, ModelingExercise exercise, boolean isStudent) { + return results.stream() + .filter(result -> shouldIncludeResult(result, exercise, isStudent)) + .peek(Result::filterSensitiveInformation) + .sorted(Comparator.comparing(Result::getCompletionDate).reversed()) + .toList(); +}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/de/tum/cit/aet/artemis/exam/api/ExamAccessApi.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/service/ExamAccessService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/de/tum/cit/aet/artemis/exam/service/ExamAccessService.java
- src/main/java/de/tum/cit/aet/artemis/exam/api/ExamAccessApi.java
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/java/**/*.java`: naming:CamelCase; principles:{sin...
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.javasrc/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: server-style
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (5)
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (3)
54-54: LGTM!The new imports are necessary for the exam module API integration.
Also applies to: 73-73
159-159: LGTM!The field declaration follows best practices:
- Properly encapsulated with
private- Immutable with
final- Uses
Optionalfor the API dependency, aligning with the modularization effort
172-173: LGTM!The constructor changes follow dependency injection best practices:
- Maintains constructor injection pattern
- Properly initializes the new optional API field
Also applies to: 198-198
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java (2)
84-86: LGTM! Well-structured dependency injection.The use of Optional for API dependencies aligns well with the modularization objectives and follows good dependency injection practices.
Also applies to: 92-93
147-154: LGTM! Clean implementation of exam submission checks.The implementation properly handles API availability and maintains clear separation of concerns.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/test/java/de/tum/cit/aet/artemis/exam/service/ExamAccessServiceTest.java (3)
444-456: Add JavaDoc to explain the test case.The test case should include a JavaDoc comment explaining why non-exam exercises are not allowed to get exam results.
+ /** + * Tests that attempting to get exam results for a non-exam exercise throws a ConflictException. + * This is because exam results are only applicable to exercises that are part of an exam. + */ @Test @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") void testAllowedToGetExamResult_nonExamExercise() {
444-515: Extract common test setup to reduce duplication.The test methods share common setup code for creating student participation. Consider extracting this to a helper method.
+ private StudentParticipation createStudentParticipation(de.tum.cit.aet.artemis.exercise.domain.Exercise exercise, User student) { + StudentParticipation participation = new StudentParticipation(); + participation.setExercise(exercise); + participation.setParticipant(student); + return studentParticipationRepository.save(participation); + }Then use it in the test methods:
- StudentParticipation participation = new StudentParticipation(); - participation.setExercise(exercise); - participation.setParticipant(student1); - studentParticipationRepository.save(participation); + StudentParticipation participation = createStudentParticipation(exercise, student1);
458-472: Add JavaDoc to remaining test methods.The remaining test methods should also include JavaDoc comments explaining the test scenarios.
+ /** + * Tests that exam results can be accessed when the exam period is still ongoing. + */ @Test void testAllowedToGetExamResult_periodNotOver() { + /** + * Tests that exam results can be accessed when the exam period is over and results have been published. + */ @Test void testAllowedToGetExamResult_periodOver_resultsPublished() { + /** + * Tests that exam results cannot be accessed when the exam period is over but results have not been published. + */ @Test void testAllowedToGetExamResult_periodOver_forbidden() {Also applies to: 474-493, 495-515
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java (2)
147-154: Improve error handling for exam submission checks.While the API checks are correct, the error handling could be enhanced:
- The error message when API is not present could be more user-friendly
- Consider adding specific error codes for each type of check failure
- var api = examSubmissionApi.orElseThrow(() -> new ApiNotPresentException(ExamSubmissionApi.class, PROFILE_CORE)); + var api = examSubmissionApi.orElseThrow(() -> { + var message = "Exam submission functionality is not available. Please contact support."; + return new ApiNotPresentException(ExamSubmissionApi.class, PROFILE_CORE, message); + });
419-422: Consider consolidating duplicate exam access checks.This exam access check is duplicated from the previous occurrence. Consider extracting it to a private method to avoid code duplication.
+ private void checkExamAccess(ModelingExercise exercise, StudentParticipation participation, User user) { + if (exercise.isExamExercise()) { + var api = examAccessApi.orElseThrow(() -> new ApiNotPresentException(ExamAccessApi.class, PROFILE_CORE)); + api.checkIfAllowedToGetExamResult(exercise, participation, user); + } + } + @GetMapping("participations/{participationId}/submissions-with-results") @EnforceAtLeastStudent public ResponseEntity<List<Submission>> getSubmissionsWithResultsForParticipation(@PathVariable long participationId) { // ... existing code ... - if (modelingExercise.isExamExercise()) { - var api = examAccessApi.orElseThrow(() -> new ApiNotPresentException(ExamAccessApi.class, PROFILE_CORE)); - api.checkIfAllowedToGetExamResult(modelingExercise, participation, user); - } + checkExamAccess(modelingExercise, participation, user);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java(6 hunks)src/test/java/de/tum/cit/aet/artemis/exam/service/ExamAccessServiceTest.java(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`src/main/java/**/*.java`: naming:CamelCase; principles:{sin...
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.javasrc/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java
`src/test/java/**/*.java`: test_naming: descriptive; test_si...
src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
src/test/java/de/tum/cit/aet/artemis/exam/service/ExamAccessServiceTest.java
📓 Learnings (1)
src/test/java/de/tum/cit/aet/artemis/exam/service/ExamAccessServiceTest.java (3)
Learnt from: valentin-boehm
PR: ls1intum/Artemis#7384
File: src/test/java/de/tum/in/www1/artemis/exam/StudentExamIntegrationTest.java:988-993
Timestamp: 2024-11-12T12:51:46.554Z
Learning: The `testSubmitStudentExam_differentUser` method does not require additional checks to verify the state of `studentExam1` after receiving a `HttpStatus.FORBIDDEN` because the control flow in the `StudentExamResource` is straightforward and ensures no state change occurs.
Learnt from: valentin-boehm
PR: ls1intum/Artemis#7384
File: src/test/java/de/tum/in/www1/artemis/exam/StudentExamIntegrationTest.java:2836-2846
Timestamp: 2024-11-12T12:51:51.201Z
Learning: The `testAbandonStudentExamNotInTime` method does not require additional checks to verify the state of `studentExam1` after receiving a `HttpStatus.FORBIDDEN` because the control flow in the `StudentExamResource` is straightforward and ensures no state change occurs.
Learnt from: julian-christl
PR: ls1intum/Artemis#9322
File: src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java:0-0
Timestamp: 2024-11-12T12:51:51.201Z
Learning: In Artemis, an `ExerciseGroup` always has an associated `Exam`, so `exerciseGroup.exam` is never null.
🔇 Additional comments (7)
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (4)
54-54: LGTM!The new imports are correctly added and follow Java conventions.
Also applies to: 73-73
159-159: LGTM!The field declaration follows best practices:
- Uses
Optionalto make the exam module API optional- Correctly declared as private and final
- Follows Java naming conventions
172-173: LGTM!The constructor changes follow best practices:
- Uses constructor injection as per coding guidelines
- Correctly initializes the new field
- Maintains consistent parameter ordering
Also applies to: 198-198
421-424: LGTM!The exam result access check changes improve modularity:
- Correctly delegates access check to the new API
- Properly handles the case when the API is not present
- Maintains the security check functionality while making it more modular
src/test/java/de/tum/cit/aet/artemis/exam/service/ExamAccessServiceTest.java (1)
37-38: LGTM!The new imports and fields are properly organized and follow the project's conventions.
Also applies to: 39-40, 73-74, 92-92
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java (2)
84-86: LGTM! Clean dependency injection of optional APIs.The change from direct service dependencies to optional APIs follows good practices:
- Uses constructor injection
- Follows interface segregation principle
- Maintains backward compatibility through Optional wrapper
Also applies to: 98-100
329-332: Verify exam access check implementation.The exam access check is correctly placed but we should verify if the API implementation properly handles all edge cases.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for all implementations of ExamAccessApi to verify edge case handling ast-grep --pattern 'class $_ implements ExamAccessApi { $$$ checkIfAllowedToGetExamResult($_, $_, $_) { $$$ } }'Length of output: 248
We'll need to re-run our search while excluding test resource files that might be causing encoding issues. Let's run a script to search for all occurrences of ExamAccessApi implementations and, specifically, its method checkIfAllowedToGetExamResult in our main source directory.
Please run this script so we can inspect the output and confirm whether the implementations handle all the edge cases as expected.
🏁 Script executed:
#!/bin/bash # Search for classes implementing ExamAccessApi in main source files (ignoring test resources) echo "Searching for ExamAccessApi implementations:" rg --type java "class .* implements ExamAccessApi" --glob '!src/test/resources/**' -A 5 echo "" echo "Searching for implementations of checkIfAllowedToGetExamResult:" rg --type java "checkIfAllowedToGetExamResult" --glob '!src/test/resources/**' -A 5Length of output: 6752
Exam access check implementation verified
- The implementation in ExamAccessService.java correctly checks if the exercise is an exam and manages authorization via the authorizationCheckService.
- The ExamAccessServiceTest.java file covers multiple edge cases (e.g., non-exam exercise handling, period not over, and forbidden access scenarios), confirming proper exception handling.
- The ModelingSubmissionResource correctly delegates to the ExamAccessApi, which in turn calls ExamAccessService for validations.
helios-aet
bot
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
ole-ve
dismissed stale reviews from simonbohnen and coderabbitai[bot]
via
85aa983
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java (1)
154-160: 🛠️ Refactor suggestionConsider splitting the class to reduce constructor parameters.
The constructor has 20+ dependencies, which suggests the class has too many responsibilities. This violates the Single Responsibility Principle and makes the class harder to maintain and test.
Consider:
- Splitting the class into smaller, focused controllers (e.g.,
QuizExerciseManagementResource,QuizExerciseEvaluationResource).- Using the Builder pattern or a configuration class to manage dependencies.
src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (1)
152-289: 🛠️ Refactor suggestionConsider breaking down the service into smaller components.
The constructor has 42 parameters, which is a strong indicator that this service class has too many responsibilities. This violates the Single Responsibility Principle and makes the code harder to maintain.
Consider:
- Breaking down the service into smaller, focused services (e.g., CourseEnrollmentService, CourseArchiveService, etc.)
- Using a builder pattern or configuration object to manage dependencies
- Creating facade services to group related dependencies
Example approach using a configuration object:
public class CourseServiceConfig { private final Optional<ExamMetricsApi> examMetricsApi; private final CourseRepository courseRepository; // ... other dependencies // Constructor with all dependencies // Getters for dependencies } public class CourseService { private final CourseServiceConfig config; public CourseService(CourseServiceConfig config) { this.config = config; // Initialize essential fields from config } }
♻️ Duplicate comments (1)
src/main/java/de/tum/cit/aet/artemis/core/config/MetricsBean.java (1)
440-441:⚠️ Potential issueFix the class reference in the exceptions.
The exception's class reference doesn't match the API being used.
Apply this diff to fix both instances:
- ExamMetricsApi api = examMetricsApi.orElseThrow(() -> new ApiNotPresentException(ExamApi.class, PROFILE_CORE)); + ExamMetricsApi api = examMetricsApi.orElseThrow(() -> new ApiNotPresentException(ExamMetricsApi.class, PROFILE_CORE));Also applies to: 604-604
🧹 Nitpick comments (29)
src/main/java/de/tum/cit/aet/artemis/core/config/websocket/WebsocketConfiguration.java (2)
329-330: Enhance error handling with more specific exception message.While the API usage is correct, the error message could be more informative for debugging purposes.
- ExamRepositoryApi api = examRepositoryApi.orElseThrow(() -> new ApiNotPresentException(ExamRepositoryApi.class, PROFILE_CORE)); + ExamRepositoryApi api = examRepositoryApi.orElseThrow(() -> + new ApiNotPresentException(ExamRepositoryApi.class, PROFILE_CORE, + "Required for websocket subscription to exam topics"));
318-325: Address the TODO comment regarding TA permissions.There's an unresolved TODO comment about whether TAs should be allowed to subscribe to exam exercises.
Would you like me to help create an issue to track and resolve this TODO? I can help draft a detailed proposal for the permission model.
src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (1)
664-671: LGTM! Robust handling of optional exam API.The implementation properly handles the optional exam API:
- Safely checks for API presence using
isPresent()- Provides a sensible default (empty set) when API is not available
- Uses appropriate date range for active exam search
This change aligns well with the modular architecture by:
- Allowing the exam module to be optional
- Maintaining loose coupling between modules
- Providing clear fallback behavior
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (1)
421-424: Consider combining the exam check condition with API retrieval.The current implementation can be made more concise by combining the exam check condition with the API retrieval.
Consider this refactoring:
- if (textExercise.isExamExercise()) { - ExamAccessApi api = examAccessApi.orElseThrow(() -> new ApiNotPresentException(ExamAccessApi.class, PROFILE_CORE)); - api.checkIfAllowedToGetExamResult(textExercise, participation, user); - } + if (textExercise.isExamExercise()) { + examAccessApi.orElseThrow(() -> new ApiNotPresentException(ExamAccessApi.class, PROFILE_CORE)) + .checkIfAllowedToGetExamResult(textExercise, participation, user); + }src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java (1)
799-801: Consider extracting the API retrieval for better readability.The API usage is correct, but readability could be improved by extracting the API retrieval into a separate line.
Consider this refactoring:
- ExamLiveEventsApi api = examLiveEventsApi.orElseThrow(() -> new ApiNotPresentException(ExamLiveEventsApi.class, PROFILE_CORE)); - User instructor = userRepository.getUser(); - api.createAndSendProblemStatementUpdateEvent(updatedExercise, notificationText, instructor); + ExamLiveEventsApi api = examLiveEventsApi.orElseThrow( + () -> new ApiNotPresentException(ExamLiveEventsApi.class, PROFILE_CORE) + ); + User instructor = userRepository.getUser(); + api.createAndSendProblemStatementUpdateEvent(updatedExercise, notificationText, instructor);src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseScheduleService.java (3)
421-422: Ensure consistent error handling pattern.The API access pattern:
- ExamDateApi api = examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); + final ExamDateApi api = examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE));Consider:
- Adding
finalmodifier for immutability- Extracting the error message to a constant if reused elsewhere
871-873: Consider extracting API access pattern to a utility method.The pattern of accessing APIs with orElseThrow is repeated multiple times:
+ private <T> T getApi(Optional<T> api, Class<T> apiClass) { + return api.orElseThrow(() -> new ApiNotPresentException(apiClass, PROFILE_CORE)); + }Then use it like:
- ExamRepositoryApi api = examRepositoryApi.orElseThrow(() -> new ApiNotPresentException(ExamRepositoryApi.class, PROFILE_CORE)); + ExamRepositoryApi api = getApi(examRepositoryApi, ExamRepositoryApi.class);Benefits:
- Reduces code duplication
- Centralizes error handling
- Makes the code more maintainable
Also applies to: 901-903
967-967: Consider implementing the suggested TODO.The TODO comment suggests implementing batch operations for VCS server operations:
// TODO: we should think about executing those operations again in batches to avoid issues on the vcs serverThis could help prevent potential performance issues or rate limiting on the VCS server.
Would you like me to help design a batching mechanism for these operations?
src/main/java/de/tum/cit/aet/artemis/core/service/export/CourseExamExportService.java (2)
183-184: Consider extracting the repeated API retrieval pattern.The
orElseThrowpattern is duplicated across multiple methods. Consider extracting it to a private helper method to improve maintainability and reduce duplication.+ private ExamRepositoryApi getExamApi() { + return examRepositoryApi.orElseThrow(() -> + new ApiNotPresentException(ExamRepositoryApi.class, PROFILE_CORE)); + } public Optional<Path> exportExam(Exam exam, Path outputDir, List<String> exportErrors) { - ExamRepositoryApi api = examRepositoryApi.orElseThrow(() -> - new ApiNotPresentException(ExamRepositoryApi.class, PROFILE_CORE)); + ExamRepositoryApi api = getExamApi();Also applies to: 238-239, 313-314
527-534: Consider enhancing error handling for file operations.The
writeFilemethod could benefit from additional error handling:
- Check if the parent directory exists and is writable
- Handle potential disk space issues
- Consider adding a retry mechanism for transient I/O errors
private Path writeFile(List<String> data, Path outputDir, String fileName) throws IOException { Path outputFile = outputDir.resolve(fileName); + if (!Files.isWritable(outputDir)) { + throw new IOException("Output directory is not writable: " + outputDir); + } try (BufferedWriter writer = Files.newBufferedWriter(outputFile, StandardCharsets.UTF_8)) { for (String line : data) { writer.write(line); writer.write(System.lineSeparator()); } + } catch (IOException e) { + log.error("Failed to write file {}: {}", fileName, e.getMessage()); + throw e; } return outputFile; }src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultWebsocketService.java (1)
79-80: Enhance error handling readability and maintainability.While the implementation is functionally correct, the error handling could be improved for better maintainability and readability.
Consider extracting the API retrieval into a private method:
- ExamDateApi api = examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); - isWorkingPeriodOver = api.isIndividualExerciseWorkingPeriodOver(exercise.getExam(), studentParticipation); + isWorkingPeriodOver = getExamDateApi().isIndividualExerciseWorkingPeriodOver(exercise.getExam(), studentParticipation); // Add this private method: + private ExamDateApi getExamDateApi() { + return examDateApi.orElseThrow(() -> + new ApiNotPresentException( + ExamDateApi.class, + "Exam date API is required for exam exercise operations in " + PROFILE_CORE + ) + ); + }Benefits:
- More descriptive error message
- Reusable error handling
- Improved readability of the main method
src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportExamCreationService.java (3)
43-47: Document Optional API dependencies.Consider adding JavaDoc to explain the behavior when APIs are not present, as these are now optional dependencies.
+ /** + * Optional API for student exam operations. If not present, exam export functionality will be skipped. + */ private final Optional<StudentExamApi> studentExamApi; + /** + * Optional API for exam operations. Required for retrieving exam grades. + * @throws ApiNotPresentException if the API is not present when accessing exam grades + */ private final Optional<ExamApi> examApi;
67-72: Add logging for skipped export.Consider adding a log message when skipping the export due to missing API to improve observability.
if (studentExamApi.isEmpty()) { + log.debug("Skipping exam export: StudentExamApi not present"); return; }
124-125: Enhance error message for missing API.The error message could be more specific about the operation being attempted.
- ExamApi api = examApi.orElseThrow(() -> new ApiNotPresentException(ExamApi.class, PROFILE_CORE)); + ExamApi api = examApi.orElseThrow(() -> + new ApiNotPresentException(ExamApi.class, PROFILE_CORE, "Required for retrieving student exam grades"));src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java (2)
199-205: Enhance error message for API not present scenario.While the error handling is correct, consider adding more context to help diagnose API availability issues.
- StudentExamApi api = studentExamApi.orElseThrow(() -> new ApiNotPresentException(StudentExamApi.class, PROFILE_CORE)); + StudentExamApi api = studentExamApi.orElseThrow(() -> + new ApiNotPresentException(StudentExamApi.class, PROFILE_CORE, + "Required for deleting exam exercise with ID: " + exercise.getId()));
152-224: Consider breaking down the delete method for better maintainability.The delete method handles multiple concerns (channel deletion, exercise unit deletion, plagiarism results, participations, etc.). Consider extracting these into separate private methods to improve readability and maintainability.
Example refactoring:
private void deleteExerciseChannel(Exercise exercise, long start) { Channel exerciseChannel = channelRepository.findChannelByExerciseId(exercise.getId()); channelService.deleteChannel(exerciseChannel); log.info("Deleting the channel took {}", TimeLogUtil.formatDurationFrom(start)); } private void deleteExerciseUnits(long exerciseId) { List<ExerciseUnit> exerciseUnits = this.exerciseUnitRepository.findByIdWithCompetenciesBidirectional(exerciseId); for (ExerciseUnit exerciseUnit : exerciseUnits) { lectureUnitService.removeLectureUnit(exerciseUnit); } } // ... similar methods for other deletion taskssrc/main/java/de/tum/cit/aet/artemis/core/web/FileResource.java (1)
409-411: Consider extracting common API access pattern.The API access pattern is repeated in both
getUserSignatureandgetExamUserImage. Consider extracting this into a private helper method to reduce duplication:+private ExamUser getExamUserOrThrow(Long examUserId) { + ExamUserApi api = examUserApi.orElseThrow(() -> new ApiNotPresentException(ExamUserApi.class, PROFILE_CORE)); + return api.findWithExamById(examUserId).orElseThrow(); +} public ResponseEntity<byte[]> getUserSignature(@PathVariable Long examUserId) { log.debug("REST request to get signature for exam user : {}", examUserId); - ExamUserApi api = examUserApi.orElseThrow(() -> new ApiNotPresentException(ExamUserApi.class, PROFILE_CORE)); - ExamUser examUser = api.findWithExamById(examUserId).orElseThrow(); + ExamUser examUser = getExamUserOrThrow(examUserId); // ... } public ResponseEntity<byte[]> getExamUserImage(@PathVariable Long examUserId) { log.debug("REST request to get image for exam user : {}", examUserId); - ExamUserApi api = examUserApi.orElseThrow(() -> new ApiNotPresentException(ExamUserApi.class, PROFILE_CORE)); - ExamUser examUser = api.findWithExamById(examUserId).orElseThrow(); + ExamUser examUser = getExamUserOrThrow(examUserId); // ... }Also applies to: 427-429
src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizSubmissionResource.java (1)
242-250: Consider improving error handling and documentation.While the logic is correct, consider these improvements:
- Add more specific error messages in the ApiNotPresentException
- Extract exam-specific logic into a private method for better readability
- Enhance the comment to detail what "further checks" are being applied
Consider this refactor:
- if (quizExercise.isExamExercise()) { - ExamSubmissionApi api = examSubmissionApi.orElseThrow(() -> new ApiNotPresentException(ExamSubmissionApi.class, PROFILE_CORE)); - - // Apply further checks if it is an exam submission - api.checkSubmissionAllowanceElseThrow(quizExercise, user); - - // Prevent multiple submissions (currently only for exam submissions) - quizSubmission = (QuizSubmission) api.preventMultipleSubmissions(quizExercise, quizSubmission, user); - } + if (quizExercise.isExamExercise()) { + quizSubmission = handleExamSubmission(quizExercise, quizSubmission, user); + } + /** + * Handles exam-specific submission logic including: + * - Verifying exam submission API availability + * - Checking if the user is allowed to submit + * - Preventing multiple submissions for the same exercise + * + * @throws ApiNotPresentException if exam submission API is not available + */ + private QuizSubmission handleExamSubmission(QuizExercise quizExercise, QuizSubmission quizSubmission, User user) { + ExamSubmissionApi api = examSubmissionApi.orElseThrow( + () -> new ApiNotPresentException(ExamSubmissionApi.class, PROFILE_CORE, "Exam submission API is required for exam exercises")); + + api.checkSubmissionAllowanceElseThrow(quizExercise, user); + return (QuizSubmission) api.preventMultipleSubmissions(quizExercise, quizSubmission, user); + }src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizPoolService.java (1)
110-120: Consider simplifying the optional handling.The nested if statement could be refactored using Optional's map() for cleaner code.
- public Optional<QuizPool> findWithQuizGroupsAndQuestionsByExamId(Long examId) { - Optional<QuizPool> quizPoolOptional = quizPoolRepository.findWithEagerQuizQuestionsByExamId(examId); - if (quizPoolOptional.isPresent()) { - QuizPool quizPool = quizPoolOptional.get(); - List<Long> quizGroupIds = quizPool.getQuizQuestions().stream().map(QuizQuestion::getQuizGroupId).filter(Objects::nonNull).toList(); - List<QuizGroup> quizGroups = quizGroupRepository.findAllById(quizGroupIds); - quizPool.setQuizGroups(quizGroups); - reassignQuizQuestion(quizPool, quizGroups); - } - return quizPoolOptional; + public Optional<QuizPool> findWithQuizGroupsAndQuestionsByExamId(Long examId) { + return quizPoolRepository.findWithEagerQuizQuestionsByExamId(examId) + .map(quizPool -> { + List<Long> quizGroupIds = quizPool.getQuizQuestions().stream() + .map(QuizQuestion::getQuizGroupId) + .filter(Objects::nonNull) + .toList(); + List<QuizGroup> quizGroups = quizGroupRepository.findAllById(quizGroupIds); + quizPool.setQuizGroups(quizGroups); + reassignQuizQuestion(quizPool, quizGroups); + return quizPool; + }); + }src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java (1)
718-720: Improve the error message for API not present exception.While the API usage is correct, consider providing a more user-friendly error message that explains what went wrong and how to fix it.
- ExamDateApi api = examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); + ExamDateApi api = examDateApi.orElseThrow(() -> + new ApiNotPresentException("Cannot re-evaluate exam quiz: Exam date service is not available. " + + "Please ensure the exam module is properly configured.", ExamDateApi.class, PROFILE_CORE));src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingSubmissionService.java (1)
73-73: Consider further dependency reduction opportunities.While this PR helps reduce dependencies through modularization, consider further improvements:
- Extract submission processing logic to a separate service
- Split exam-related and course-related submission handling
- Consider using the Command pattern for different submission types
src/main/java/de/tum/cit/aet/artemis/text/web/TextSubmissionResource.java (1)
145-153: Consider extracting exam-specific checks to improve readability.While the implementation is functionally correct, the method's readability could be improved by extracting the exam-specific checks into a separate private method.
Consider this refactoring:
private ResponseEntity<TextSubmission> handleTextSubmission(long exerciseId, TextSubmission textSubmission) { long start = System.currentTimeMillis(); final var user = userRepository.getUserWithGroupsAndAuthorities(); final var exercise = textExerciseRepository.findByIdElseThrow(exerciseId); - if (exercise.isExamExercise()) { - ExamSubmissionApi api = examSubmissionApi.orElseThrow(() -> new ApiNotPresentException(ExamSubmissionApi.class, PROFILE_CORE)); - - // Apply further checks if it is an exam submission - api.checkSubmissionAllowanceElseThrow(exercise, user); - - // Prevent multiple submissions (currently only for exam submissions) - textSubmission = (TextSubmission) api.preventMultipleSubmissions(exercise, textSubmission, user); - } + textSubmission = handleExamSpecificChecks(exercise, textSubmission, user); // Check if the user is allowed to submit textSubmissionService.checkSubmissionAllowanceElseThrow(exercise, textSubmission, user); // ... rest of the method } + + private TextSubmission handleExamSpecificChecks(Exercise exercise, TextSubmission textSubmission, User user) { + if (!exercise.isExamExercise()) { + return textSubmission; + } + ExamSubmissionApi api = examSubmissionApi.orElseThrow(() -> + new ApiNotPresentException(ExamSubmissionApi.class, PROFILE_CORE)); + + api.checkSubmissionAllowanceElseThrow(exercise, user); + return (TextSubmission) api.preventMultipleSubmissions(exercise, textSubmission, user); + }src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java (1)
249-251: Consider extracting the exam end date check into a separate method.The exam end date validation logic could be moved to a private method to improve readability and reusability.
- ExamDateApi api = examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); - ZonedDateTime latestIndividualExamEndDate = api.getLatestIndividualExamEndDate(exam); + ZonedDateTime latestIndividualExamEndDate = getLatestIndividualExamEndDate(exam); + private ZonedDateTime getLatestIndividualExamEndDate(Exam exam) { + ExamDateApi api = examDateApi.orElseThrow(() -> new ApiNotPresentException(ExamDateApi.class, PROFILE_CORE)); + return api.getLatestIndividualExamEndDate(exam); + }src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (4)
362-386: Consider extracting exam-related logic to a separate method.The method
findOneWithExercisesAndLecturesAndExamsAndCompetenciesAndTutorialGroupsAndFaqForUserhas too many responsibilities. It handles exercises, lectures, exams, competencies, tutorial groups, and FAQs, making it hard to maintain and test.Consider extracting exam-related logic to a separate method:
private void loadExamsForCourse(Course course, User user) { ExamRepositoryApi examRepoApi = examRepositoryApi.orElseThrow( () -> new ApiNotPresentException(ExamRepositoryApi.class, PROFILE_CORE)); course.setExams(examRepoApi.findByCourseIdForUser( course.getId(), user.getId(), user.getGroups(), ZonedDateTime.now())); if (authCheckService.isOnlyStudentInCourse(course, user)) { course.setExams(examRepoApi.filterVisibleExams(course.getExams())); } }
404-425: Consider caching exam counts for better performance.The
findAllActiveWithExercisesForUsermethod makes multiple database calls through the API to count exams. For courses with many exams, this could impact performance.Consider:
- Caching exam counts with appropriate cache invalidation
- Batch processing exam counts for multiple courses
Example caching approach:
@Cacheable(value = "examCounts", key = "#courseIds") public Map<Long, Long> getExamCountsForCourses(Set<Long> courseIds, ZonedDateTime now) { ExamMetricsApi api = examMetricsApi.orElseThrow( () -> new ApiNotPresentException(ExamMetricsApi.class, PROFILE_CORE)); return api.countVisibleExams(courseIds, now).stream() .collect(Collectors.toMap( CourseContentCount::courseId, CourseContentCount::count)); }
574-582: Enhance error handling and logging for exam deletion.The
deleteExamsOfCoursemethod could benefit from better error handling and logging to track deletion progress and failures.Consider enhancing the implementation:
private void deleteExamsOfCourse(Course course) { var deletionApi = examDeletionApi.orElseThrow( () -> new ApiNotPresentException(ExamDeletionApi.class, PROFILE_CORE)); ExamRepositoryApi api = examRepositoryApi.orElseThrow( () -> new ApiNotPresentException(ExamRepositoryApi.class, PROFILE_CORE)); List<Exam> exams = api.findByCourseId(course.getId()); log.info("Starting deletion of {} exams for course {}", exams.size(), course.getId()); for (Exam exam : exams) { try { log.debug("Deleting exam {} for course {}", exam.getId(), course.getId()); deletionApi.delete(exam.getId()); } catch (Exception e) { log.error("Failed to delete exam {} for course {}: {}", exam.getId(), course.getId(), e.getMessage()); // Consider whether to throw or continue with other exams } } }
1020-1023: Maintain consistent error handling across API calls.The error handling approach in
cleanupCoursediffers from other methods. While other methods useorElseThrow, this one usesmap().orElse().Consider maintaining consistency with other API calls:
Set<Exercise> examExercises = examRepositoryApi .orElseThrow(() -> new ApiNotPresentException(ExamRepositoryApi.class, PROFILE_CORE)) .getExercisesByCourseId(courseId);src/main/java/de/tum/cit/aet/artemis/assessment/web/GradeStepResource.java (2)
110-111: Enhance error message for API not present exception.The error handling for missing API is correct, but the error message could be more descriptive for better debugging.
Consider enhancing the error message:
- ExamRepositoryApi api = examRepositoryApi.orElseThrow(() -> new ApiNotPresentException(ExamRepositoryApi.class, PROFILE_CORE)); + ExamRepositoryApi api = examRepositoryApi.orElseThrow(() -> + new ApiNotPresentException(ExamRepositoryApi.class, PROFILE_CORE, "Exam module is required for grade step operations"));Also applies to: 114-114, 219-220, 224-224
215-236: Consider caching grade step calculations.The
getGradeStepByPercentageForExammethod performs grade step calculations that could benefit from caching, especially for frequently accessed exam results.Consider implementing a caching strategy for grade step calculations to improve performance, particularly during peak periods when many students check their exam results simultaneously.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/service/ComplaintService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/web/BonusResource.java(9 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/web/GradeStepResource.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/web/GradingScaleResource.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/web/ParticipantScoreResource.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultWebsocketService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/MetricsBean.java(10 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/websocket/WebsocketConfiguration.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java(14 hunks)src/main/java/de/tum/cit/aet/artemis/core/service/export/CourseExamExportService.java(8 hunks)src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportExamCreationService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/core/web/FileResource.java(7 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDateService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseResource.java(7 hunks)src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseScheduleService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseScheduleService.java(10 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingSubmissionService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java(6 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizPoolService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java(6 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizSubmissionResource.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java(6 hunks)src/main/java/de/tum/cit/aet/artemis/text/web/TextSubmissionResource.java(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- src/main/java/de/tum/cit/aet/artemis/assessment/service/ResultService.java
- src/main/java/de/tum/cit/aet/artemis/assessment/service/AssessmentService.java
- src/main/java/de/tum/cit/aet/artemis/assessment/web/GradingScaleResource.java
- src/main/java/de/tum/cit/aet/artemis/assessment/web/BonusResource.java
- src/main/java/de/tum/cit/aet/artemis/assessment/web/ParticipantScoreResource.java
- src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java
- src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java
- src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseResource.java
- src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseScheduleService.java
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/java/**/*.java`: naming:CamelCase; principles:{sin...
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/web/FileResource.javasrc/main/java/de/tum/cit/aet/artemis/text/web/TextSubmissionResource.javasrc/main/java/de/tum/cit/aet/artemis/assessment/web/GradeStepResource.javasrc/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.javasrc/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.javasrc/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDateService.javasrc/main/java/de/tum/cit/aet/artemis/assessment/web/ResultWebsocketService.javasrc/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.javasrc/main/java/de/tum/cit/aet/artemis/core/config/websocket/WebsocketConfiguration.javasrc/main/java/de/tum/cit/aet/artemis/quiz/service/QuizPoolService.javasrc/main/java/de/tum/cit/aet/artemis/assessment/service/ComplaintService.javasrc/main/java/de/tum/cit/aet/artemis/quiz/web/QuizSubmissionResource.javasrc/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.javasrc/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportExamCreationService.javasrc/main/java/de/tum/cit/aet/artemis/core/config/MetricsBean.javasrc/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseScheduleService.javasrc/main/java/de/tum/cit/aet/artemis/core/service/export/CourseExamExportService.javasrc/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.javasrc/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.javasrc/main/java/de/tum/cit/aet/artemis/core/service/CourseService.javasrc/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingSubmissionService.java
🧠 Learnings (3)
src/main/java/de/tum/cit/aet/artemis/text/web/TextSubmissionResource.java (1)
Learnt from: florian-glombik
PR: ls1intum/Artemis#7588
File: src/main/java/de/tum/in/www1/artemis/service/exam/ExamService.java:424-431
Timestamp: 2024-11-12T12:51:40.391Z
Learning: The grade is not relevant for unsubmitted exams, and no additional checks for submission status are needed when calculating exam scores as a bonus source.
src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportExamCreationService.java (1)
Learnt from: florian-glombik
PR: ls1intum/Artemis#7588
File: src/main/java/de/tum/in/www1/artemis/service/exam/ExamService.java:424-431
Timestamp: 2024-11-12T12:51:40.391Z
Learning: The grade is not relevant for unsubmitted exams, and no additional checks for submission status are needed when calculating exam scores as a bonus source.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingSubmissionService.java (1)
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#9303
File: src/main/java/de/tum/in/www1/artemis/service/exam/StudentExamService.java:296-300
Timestamp: 2024-11-12T12:51:40.391Z
Learning: When reviewing code changes in `StudentExamService.saveSubmission`, if the PR aims to improve readability without changing logic, avoid suggesting changes that alter logic, such as adding exceptions in the default case of switch statements.
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: client-style
- GitHub Check: server-tests
- GitHub Check: client-tests
- GitHub Check: server-style
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: Analyse
🔇 Additional comments (69)
src/main/java/de/tum/cit/aet/artemis/core/config/websocket/WebsocketConfiguration.java (3)
61-61: LGTM! Clean import additions.The new imports for
ApiNotPresentExceptionandExamRepositoryApiare correctly added and align with the PR's objective of introducing the exam module API.Also applies to: 67-67
95-95: LGTM! Field declaration follows best practices.The field declaration correctly uses
Optional<ExamRepositoryApi>and follows the principle of least access with theprivate finalmodifiers.
107-117: LGTM! Constructor injection properly implemented.The constructor correctly:
- Uses constructor injection for dependency management
- Maintains immutability by assigning to final fields
- Handles the Optional wrapper appropriately
src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (2)
111-112: LGTM! Clean import declarations.The new imports for the exam module API are well-organized and follow Java import guidelines.
188-188: LGTM! Clean dependency injection implementation.The transition from direct repository dependency to optional API is well-implemented:
- Uses constructor injection
- Follows Spring's dependency injection best practices
- Properly handles optional dependencies
Also applies to: 200-201, 221-221
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDateService.java (5)
13-15: LGTM! New imports align with modularization effort.The added imports for API interfaces and exceptions support the transition to a modular exam API structure.
27-35: LGTM! Clean dependency injection with Optional APIs.The changes follow best practices:
- Constructor injection adheres to coding guidelines
- Optional wrapping provides proper null safety
- Changes align with the modularization effort
81-87: LGTM! Proper error handling with Optional APIs.The changes maintain the existing logic while properly handling API absence using orElseThrow pattern.
201-202: LGTM! Consistent API usage pattern.The changes follow the same pattern as other methods:
- Proper error handling with orElseThrow
- Maintains null safety with Optional
23-212: Consider adding unit tests for API integration.While the implementation looks good, it would be beneficial to add unit tests to verify:
- Behavior when APIs are not present
- Error handling with ApiNotPresentException
- Integration with exam module APIs
Would you like me to help generate unit test cases for these scenarios?
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (3)
54-54: LGTM!The new imports are correctly added and necessary for the exam access API functionality.
Also applies to: 73-73
159-159: LGTM!The field declaration follows the established pattern for API dependencies and properly uses Optional for modularity.
173-173: LGTM!The constructor changes properly integrate the new exam access API dependency.
Also applies to: 198-198
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java (3)
133-133: LGTM! Field declaration follows best practices.The change from
ExamLiveEventsServicetoOptional<ExamLiveEventsApi>aligns with the modularization effort and follows Java best practices for dependency injection.
145-145: LGTM! Constructor injection is properly implemented.The constructor parameter update correctly implements constructor-based dependency injection with the new API interface.
167-167: LGTM! Field initialization is correct.The field is properly initialized with the injected Optional API.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseScheduleService.java (3)
3-3: LGTM! Clean import organization.The new imports are well-organized and follow a logical grouping pattern:
- Core imports for API exceptions
- Exam API interfaces
Also applies to: 45-45, 50-52
98-102: LGTM! Well-structured field declarations.The transition from concrete implementations to Optional-wrapped APIs:
- Follows dependency inversion principle
- Enables better modularity and testing
- Maintains final modifier for immutability
108-131: LGTM! Constructor properly handles dependency injection.The constructor:
- Accepts Optional API interfaces
- Maintains proper initialization order
- Follows constructor injection pattern
src/main/java/de/tum/cit/aet/artemis/core/config/MetricsBean.java (3)
39-39: LGTM! Good modularization approach.The replacement of direct repository dependencies with Optional-wrapped API interfaces aligns well with the modularization objectives. This approach provides better flexibility and loose coupling.
Also applies to: 45-47, 94-96
181-182: LGTM! Constructor injection maintained.The constructor changes correctly implement dependency injection for the new API interfaces while maintaining the Spring best practice of constructor injection.
Also applies to: 191-192
465-469: LGTM! Clean API integration.The integration of exam-related APIs is well implemented:
- Proper error handling with orElseThrow
- Consistent usage pattern across different metrics calculations
- Maintains existing functionality while using the new modular approach
Also applies to: 618-618, 631-632, 662-666
src/main/java/de/tum/cit/aet/artemis/core/service/export/CourseExamExportService.java (1)
36-41: LGTM! Clean transition to API-based dependency injection.The changes effectively implement the modular API approach by:
- Replacing direct repository dependency with Optional
- Using constructor injection for the API dependency
- Adding necessary imports
Also applies to: 81-82, 89-90, 98-99
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultWebsocketService.java (3)
19-19: LGTM! Clean import additions.The new imports are necessary and follow Java import guidelines.
Also applies to: 21-21
38-38: LGTM! Well-structured field declaration.The field declaration follows best practices:
- Uses constructor injection
- Maintains immutability with
final- Follows least access principle with
private- Properly wraps the API in
Optionalfor modularity
46-53: LGTM! Clean constructor implementation.The constructor follows dependency injection best practices and maintains immutability.
src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportExamCreationService.java (2)
25-27: LGTM! Well-organized imports.The new imports for API interfaces and exceptions are properly organized and follow Java import conventions.
51-57: LGTM! Well-structured constructor injection.The constructor properly injects optional dependencies, aligning with dependency injection principles and the PR's modularization goals.
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java (2)
24-26: LGTM! Clean transition to module API.The changes align well with the PR's modularization objectives, using Optional for flexible dependency handling.
Also applies to: 64-64
88-88: LGTM! Constructor injection properly implemented.The transition to Optional maintains clean dependency injection patterns.
Also applies to: 99-99
src/main/java/de/tum/cit/aet/artemis/core/web/FileResource.java (3)
50-50: LGTM! Import changes align with modularization.The addition of
ApiNotPresentExceptionandExamUserApiimports supports the transition to module-based APIs.Also applies to: 65-65
120-120: LGTM! Field and constructor changes support API modularization.The replacement of
ExamUserRepositorywithOptional<ExamUserApi>aligns with the server modularization effort, providing a clean facade for repository access.Also applies to: 136-136, 148-148
409-411: LGTM! Method changes properly handle optional API access.The changes in
getUserSignatureandgetExamUserImagemethods:
- Correctly handle the optional API using
orElseThrow- Maintain proper error handling with
ApiNotPresentException- Preserve authorization checks
- Follow a consistent pattern across the codebase
Also applies to: 427-429
src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizSubmissionResource.java (2)
6-6: LGTM! Clean import additions.The new imports are well-organized and follow Java conventions.
Also applies to: 28-28, 36-36
77-90: LGTM! Well-structured dependency injection.The changes follow best practices:
- Constructor injection is used appropriately
- Optional wrapper handles the API dependency elegantly
- Field is correctly marked as final
src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizPoolService.java (3)
21-21: LGTM! Import changes align with API-based architecture.The new imports support the transition to a modular API-based architecture while maintaining clean code organization.
Also applies to: 24-24
50-58: LGTM! Field and constructor changes follow best practices.The changes demonstrate:
- Proper use of constructor injection
- Appropriate use of Optional for API dependency
- Clean transition to modular architecture
67-70: LGTM! Method changes handle API dependency appropriately.The update method properly:
- Unwraps the Optional using orElseThrow
- Provides clear error messaging with ApiNotPresentException
- Maintains clean error handling patterns
src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java (3)
53-53: LGTM!The new imports support the transition from service to API as part of the server modularization effort.
Also applies to: 72-72
126-126: LGTM!The field declaration correctly uses
Optional<ExamDateApi>and follows Java best practices with thefinalmodifier.
168-168: LGTM!The field initialization is correct and follows Java best practices.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingSubmissionService.java (4)
34-34: LGTM! Import changes align with modularization goals.The new imports for
ApiNotPresentExceptionand exam APIs (ExamDateApi,ExamSubmissionApi) are correctly organized and support the server modularization effort.Also applies to: 42-43
94-94: LGTM! Field declaration follows dependency injection best practices.The change from direct service dependency to
Optional<ExamSubmissionApi>follows constructor injection best practices and supports the modularization effort.
114-115: LGTM! Constructor changes maintain dependency injection pattern.The constructor changes correctly:
- Replace direct service dependencies with Optional-wrapped APIs
- Update the super() call with the new exam date API
- Initialize the exam submission API field
Also applies to: 120-120, 128-128
305-306: LGTM! Method changes properly handle API dependencies.The changes to
isAllowedToSubmit:
- Correctly handle the optional API using orElseThrow
- Provide clear error messaging with ApiNotPresentException
- Maintain the original method contract while adapting to the new API structure
src/main/java/de/tum/cit/aet/artemis/text/web/TextSubmissionResource.java (2)
28-37: LGTM! Clean modularization of exam submission handling.The changes effectively modularize the exam submission functionality by introducing the ExamSubmissionApi and using Optional for better null handling.
Also applies to: 80-80
84-100: LGTM! Constructor follows dependency injection best practices.The constructor properly initializes the new Optional field while maintaining clean dependency injection patterns.
src/main/java/de/tum/cit/aet/artemis/assessment/service/ComplaintService.java (4)
30-30: LGTM!The new imports are correctly added and follow the project's package structure.
Also applies to: 33-33
59-59: LGTM!The field declaration correctly uses
Optional<ExamRepositoryApi>and follows Java naming conventions.
64-64: LGTM!The constructor changes correctly implement constructor injection with the new
Optional<ExamRepositoryApi>parameter.Also applies to: 68-68
103-104: LGTM!The changes correctly handle API availability with proper error handling while maintaining the original functionality.
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java (7)
30-30: LGTM!The new imports are correctly added and follow the project's package structure.
Also applies to: 45-46
84-88: LGTM!The field declarations correctly use Optional for API interfaces and follow Java naming conventions.
92-93: LGTM!The constructor changes correctly implement constructor injection with the new API interfaces.
Also applies to: 98-101
147-154: LGTM!The changes correctly handle API availability with proper error handling while maintaining the original functionality for exam submissions.
329-332: LGTM!The changes correctly handle API availability with proper error handling while maintaining the original functionality for exam access checks.
419-422: LGTM!The changes correctly handle API availability with proper error handling while maintaining the original functionality for exam access checks.
64-67: LGTM!The class maintains its responsibility as a REST controller while correctly integrating the new module API changes.
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java (3)
42-53: LGTM! Clean import changes.The imports are correctly updated to reflect the new API-based approach, removing the direct service dependency and adding necessary imports for the API interface and exception handling.
87-87: LGTM! Clean dependency injection changes.The changes follow best practices:
- Uses constructor injection
- Properly wraps the API in Optional
- Maintains immutability with final field
Also applies to: 101-114
230-288:❓ Verification inconclusive
Verify error handling in the createResultForExternalSubmission method.
The method handles various error cases but should be tested thoroughly with the new API integration:
- When ExamDateApi is not present (Optional is empty)
- When getLatestIndividualExamEndDate returns null
- When the exam end date validation fails
🏁 Script executed:
#!/bin/bash # Search for test cases covering these scenarios ast-grep --pattern 'class $_ { $$$ @Test $$$ void $_($$$) { $$$ createResultForExternalSubmission($$$) $$$ } $$$ }'Length of output: 141
Attention: Verify Error Handling in External Submission API
The overall error handling logic in
createResultForExternalSubmissionappears sound, but please confirm that the following cases are thoroughly covered by tests:
- Scenario when
ExamDateApiis not present (i.e. theOptionalis empty).- Case where
getLatestIndividualExamEndDatereturnsnull.- Proper handling when the exam end date validation fails.
There was no clear automated test coverage found using our
ast-grepsearch. Manual verification of the test suite is recommended to ensure these conditions are effectively tested with the new API integration.src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (1)
86-100: LGTM! Clean import organization.The new exam-related API imports are well-organized and follow Java conventions.
src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.java (3)
34-34: LGTM! Import changes align with the new API-based approach.The addition of
ApiNotPresentExceptionandExamSubmissionApiimports supports the transition from service-based to API-based exam submission handling.Also applies to: 43-43
81-81: LGTM! Constructor injection follows best practices.The changes properly implement constructor injection for the new
ExamSubmissionApidependency, maintaining immutability withfinalfields and usingOptionalfor the API dependency.Also applies to: 85-97
132-138:❓ Verification inconclusive
Verify error handling for exam submissions.
The new exam submission handling looks good, but we should verify the error handling in two scenarios:
- When the exam API is not present
- When multiple submissions are prevented
Let's check the error handling implementation:
🏁 Script executed:
#!/bin/bash # Search for ApiNotPresentException handling in tests rg -A 5 "ApiNotPresentException.*ExamSubmissionApi" # Search for multiple submission prevention tests ast-grep --pattern 'void test$_multiple_submissions() { $$$ api.preventMultipleSubmissions($$$) $$$ }'Length of output: 313
Action Required: Verify Error Handling for Exam Submissions
The updated exam submission handling appears complete. However, please ensure that:
- Missing Exam API: When the exam API is not available, the call to
orElseThrowshould appropriately throw anApiNotPresentExceptionas expected.- Multiple Submissions Prevention: The behavior of
api.preventMultipleSubmissionscorrectly handles scenarios where multiple submissions are not allowed.Current automated searches did not yield clear evidence (or relevant tests) for these error paths. I recommend manually verifying that these cases are covered in the tests or adding explicit tests for them if they are missing.
src/main/java/de/tum/cit/aet/artemis/assessment/web/GradeStepResource.java (4)
27-27: LGTM! Import changes align with modularization.The addition of
ApiNotPresentExceptionandExamRepositoryApiimports supports the transition to module-based APIs.Also applies to: 34-34
58-58: LGTM! Field declaration follows best practices.The change from
ExamRepositorytoOptional<ExamRepositoryApi>aligns with:
- The modularization effort mentioned in PR objectives
- Constructor-based dependency injection
- Proper encapsulation with
private finalmodifier
67-73: LGTM! Constructor injection properly handles optional dependency.The constructor changes maintain clean dependency injection while supporting the new optional API.
106-123:❓ Verification inconclusive
Verify exam results publication check.
The method checks
exam.resultsPublished()for non-instructors, but we should verify this check is consistent across all exam-related endpoints.
🏁 Script executed:
#!/bin/bash # Search for resultsPublished() checks across the codebase rg -A 3 "resultsPublished\(\)" --type javaLength of output: 193
Consistency of exam results publication check
The current endpoint in
GradeStepResource.javaproperly enforces that non-instructors cannot access grade steps unless exam results are published. However, a repository-wide search forresultsPublished()usingrg --type javadid not yield any output due to file filtering issues. Please manually verify (or use an alternative search command, e.g., withfdandrgwithout type filters) that all exam-related endpoints consistently perform this check.
- Action Required: Confirm that endpoints exposing exam data (especially those that should restrict non-instructor access based on publication status) include a similar conditional check using
exam.resultsPublished().- Reference Location:
src/main/java/de/tum/cit/aet/artemis/assessment/web/GradeStepResource.java, Lines 106–123
There was a problem hiding this comment.
code overall lgtm. A small nitpick about the variable names, would be great if you could smth other than api in the cases where the variable is used in multiple places, like: GradingScaleResource, MetricsBean, CourseService, CourseExamExportService, ExerciseDeletionService, ModelingExerciseScheduleService.
Checklist
General
Server
Motivation and Context
As part of the server modularization, we "facade" the service/repository declaration and method calls to module-external classes via a module API provided by the respective module.
Description
These changes add the module API for exam functionalty.
Out of scope: Adding a
PROFILE_EXAM(or similar) to disable the module on server start. This requires a lot of frontend changes and testing.Steps for Testing
Exam Mode Testing
Note: Testing is not limited to these steps as the changes touch all inter-module communication to the exam module.
Basically try to test all the functionality of exams regarding submissions, courses, and import/export.
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Summary by CodeRabbit
New Features
Refactor
Tests