Development: Convert DTOs to records
#9385
Conversation
github-actions
bot
added
tests
server
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
|
Warning Rate limit exceeded@krusche has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 28 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe pull request introduces several changes across multiple Java classes, primarily focused on improving code quality and readability. Key modifications include refining JPQL queries in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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: 20
🧹 Outside diff range and nitpick comments (18)
src/main/java/de/tum/cit/aet/artemis/programming/dto/ProgrammingExerciseTestCaseDTO.java (1)
8-11: Consider enhancing JavaDoc to reflect record characteristicsWhile the documentation clearly explains the DTO's purpose, consider adding a note about the immutable nature of the record and its thread-safety benefits.
/** * This is a DTO for updating a programming exercise test case. * It is only allowed to alter the weight, bonus multiplier, bonus points and visibility flag of a test case from an * endpoint, the other attributes are generated automatically. + * This DTO is implemented as an immutable record, ensuring thread-safety and preventing unauthorized modifications after creation. */src/main/java/de/tum/cit/aet/artemis/core/service/DataExportScheduleService.java (1)
86-91: Consider extracting the timeout duration to a configuration property.The 60-minute timeout is hardcoded. Consider making it configurable through application properties for better flexibility.
+ @Value("${artemis.scheduling.data-export-timeout-minutes:60}") + private int dataExportTimeoutMinutes; // In the method: - if (!executor.awaitTermination(60, java.util.concurrent.TimeUnit.MINUTES)) { + if (!executor.awaitTermination(dataExportTimeoutMinutes, java.util.concurrent.TimeUnit.MINUTES)) {src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java (2)
92-93: Consider using a builder pattern for better readability.While the conversion to using the record's constructor is good, having 9 boolean parameters makes the code harder to maintain and understand. Consider introducing a builder pattern or a parameter object to make the intent of each parameter clearer.
Example approach:
var options = RepositoryExportOptions.builder() .withIndividualDueDates(false) .withRepositoryAccess(true) .withSubmissions(false) .withDueDate(programmingExercise.getDueDate()) .withBuildPlans(false) .withTests(false) .withStaticCodeAnalysis(false) .withUserAccess(true) .withParticipations(false) .build();
92-92: Enhance the comment about individual due dates.The current comment lacks context about why individual due dates are not supported and potential future implications.
Consider expanding the comment:
-// Athena currently does not support individual due dates +// TODO: Athena currently does not support individual due dates. This means all students share the same deadline. +// Future enhancement needed to support per-student deadlines (see JIRA-XXX).src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseTestCaseService.java (1)
Line range hint
1-150: Consider transaction boundaries for database operations.The service performs multiple database operations (saveAll, updateTasksFromProblemStatement) without explicit transaction boundaries. According to the coding guidelines (
avoid_transactions), consider:
- Delegating transaction management to the repository layer
- Making transaction boundaries explicit where needed
- Documenting transaction requirements
Consider wrapping the critical section in
updatemethod with appropriate transaction boundaries:@Transactional(readOnly = true) public Set<ProgrammingExerciseTestCase> update(...) { // Read operations... @Transactional protected Set<ProgrammingExerciseTestCase> saveChanges(Set<ProgrammingExerciseTestCase> updatedTests, ProgrammingExercise exercise) { testCaseRepository.saveAll(updatedTests); programmingExerciseTaskService.updateTasksFromProblemStatement(exercise); programmingTriggerService.setTestCasesChangedAndTriggerTestCaseUpdate(exercise.getId()); return updatedTests; } }src/main/java/de/tum/cit/aet/artemis/exam/service/ExamDateService.java (2)
Line range hint
180-187: Update JavaDoc to document nullable return value.While the
@Nullableannotation is correctly added, the JavaDoc comment should be updated to explicitly mention that the method can returnnullwhen the exam's start date is not set./** * Returns all individual exam end dates as determined by the working time of the student exams. * <p> * If no student exams are available, an empty set returned. * * @param exam the exam - * @return a set of all end dates. May return an empty set, if the exam has no start/end date or student exams cannot be found. + * @return a set of all end dates, or null if the exam has no start date. May return an empty set if no student exams are found. */
Line range hint
197-210: Consider design improvements for new methods.Several improvements could enhance the robustness of these methods:
- These static methods break the instance method pattern used throughout the service
- No validation for EXAM_START_WAIT_TIME_MINUTES being positive
- Potential NullPointerException in the @NotNull method if exam.getStartDate() is null
Consider this implementation:
- @Nullable - public static ZonedDateTime getExamProgrammingExerciseUnlockDate(ProgrammingExercise exercise) { - if (!exercise.isExamExercise()) { - return null; - } - return getExamProgrammingExerciseUnlockDate(exercise.getExerciseGroup().getExam()); - } - - @NotNull - public static ZonedDateTime getExamProgrammingExerciseUnlockDate(Exam exam) { - return exam.getStartDate().minusMinutes(EXAM_START_WAIT_TIME_MINUTES); - } + @Nullable + public ZonedDateTime getExamProgrammingExerciseUnlockDate(ProgrammingExercise exercise) { + if (!exercise.isExamExercise()) { + return null; + } + return getExamProgrammingExerciseUnlockDate(exercise.getExerciseGroup().getExam()); + } + + @NotNull + public ZonedDateTime getExamProgrammingExerciseUnlockDate(Exam exam) { + if (exam.getStartDate() == null) { + throw new IllegalArgumentException("Exam start date must be set"); + } + if (EXAM_START_WAIT_TIME_MINUTES <= 0) { + throw new IllegalStateException("EXAM_START_WAIT_TIME_MINUTES must be positive"); + } + return exam.getStartDate().minusMinutes(EXAM_START_WAIT_TIME_MINUTES); + }src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java (1)
119-125: Consider batching repository operations.The current implementation processes each participation independently. Consider batching repository operations to reduce the number of concurrent connections.
Example approach:
- Group participations by repository type
- Process each group in batches
- Use bulk operations where possible
src/main/java/de/tum/cit/aet/artemis/assessment/repository/ComplaintRepository.java (2)
269-270: Improved type safety in JPQL query.The changes enhance type safety by:
- Using explicit CASE expression for counting accepted complaints
- Using CAST for proper type conversion of SUM results to avoid potential precision loss
Consider extracting the CASE expressions into a Common Table Expression (CTE) to improve readability:
- SUM(CASE WHEN c.accepted = TRUE THEN 1L ELSE 0L END), - CAST(SUM(CASE WHEN c.accepted = TRUE THEN e.maxPoints ELSE 0.0 END) AS double) + SUM(accepted_count), + CAST(SUM(accepted_points) AS double) + FROM ( + SELECT c.*, + CASE WHEN c.accepted = TRUE THEN 1L ELSE 0L END as accepted_count, + CASE WHEN c.accepted = TRUE THEN e.maxPoints ELSE 0.0 END as accepted_points
422-423: Enhanced NULL handling in feedback request queries.The changes improve NULL handling in the CASE expressions for feedback requests, ensuring accurate counting and point calculations.
Consider adding an index on the
complaintTypeandacceptedcolumns to optimize these frequently filtered columns:CREATE INDEX idx_complaint_type_accepted ON complaint(complaint_type, accepted);Also applies to: 447-448
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (2)
437-440: Add input validation for participant identifiers.The current implementation directly splits the input string without proper validation. Consider adding validation to prevent potential injection attacks or malformed inputs.
if (!repositoryExportOptions.exportAllParticipants()) { + if (participantIdentifiers == null || participantIdentifiers.isBlank()) { + throw new BadRequestAlertException("Participant identifiers cannot be empty", ENTITY_NAME, "invalidParticipantIdentifiers"); + } participantIdentifiers = participantIdentifiers.replaceAll("\\s+", ""); + if (!participantIdentifiers.matches("^[\\w,.-]+$")) { + throw new BadRequestAlertException("Invalid characters in participant identifiers", ENTITY_NAME, "invalidParticipantIdentifiers"); + } participantIdentifierList.addAll(List.of(participantIdentifiers.split(","))); }🧰 Tools
🪛 ast-grep
[warning] 438-438: Detected a cookie where the
Secureflag is either missing or disabled. TheSecurecookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecureflag totrueso the cookie will only be sent over HTTPS.
Context: (participantIdentifiers.split(","))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 438-438: Detected a cookie where the
HttpOnlyflag is either missing or disabled. TheHttpOnlycookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnlyflag to true` in all other cases.
Context: (participantIdentifiers.split(","))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
447-457: Add JavaDoc to document the method's purpose and parameters.Consider adding documentation to improve code maintainability:
+ /** + * Filters and returns a list of programming exercise student participations based on export options. + * + * @param repositoryExportOptions Options controlling the export behavior + * @param programmingExercise The exercise containing the participations to filter + * @param participantIdentifierList List of specific participant identifiers to include + * @return List of filtered programming exercise student participations + */ private static List<ProgrammingExerciseStudentParticipation> getExportedStudentParticipations(RepositoryExportOptionsDTO repositoryExportOptions, ProgrammingExercise programmingExercise, Set<String> participantIdentifierList) {src/main/java/de/tum/cit/aet/artemis/exam/service/StudentExamService.java (1)
286-329: LGTM! Clean switch expression implementation.The switch expression effectively handles different exercise types with proper type safety and validation.
Consider making the default case more explicit about why no action is needed:
- default -> { - } + default -> { + // No action needed for other exercise types (e.g., ProgrammingExercise, FileUploadExercise) + // as they are handled separately + }src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java (1)
Line range hint
707-758: Enhance error handling for git operationsWhile error handling is good for code style normalization, consider implementing more detailed error handling for git operations to help with debugging issues.
try { log.debug("Normalizing code style for participation {}", participation); fileService.normalizeLineEndingsDirectory(repository.getLocalPath()); fileService.convertFilesInDirectoryToUtf8(repository.getLocalPath()); } catch (IOException ex) { - log.warn("Cannot normalize code style in the repository {} due to the following exception: {}", repository.getLocalPath(), ex.getMessage()); + log.warn("Cannot normalize code style in the repository {} due to the following exception: {}", repository.getLocalPath(), ex); + log.debug("Detailed stack trace for code style normalization failure:", ex); }src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseScheduleService.java (1)
994-1002: Consider enhancing completion logging.The completion logging could be more informative by including success statistics.
return CompletableFuture.allOf(futures.toArray(CompletableFuture[]::new)).thenApply(ignore -> { threadPool.shutdown(); - log.info("Finished executing (scheduled) task '{}' for programming exercise with id {}.", operationName, programmingExercise.getId()); + int totalParticipations = futures.size(); + int successCount = totalParticipations - failedOperations.size(); + log.info("Finished executing (scheduled) task '{}' for programming exercise '{}' (id: {}). " + + "Success: {}/{} participations", + operationName, programmingExercise.getTitle(), + programmingExercise.getId(), successCount, totalParticipations); if (!failedOperations.isEmpty()) { var failedIds = failedOperations.stream().map(participation -> participation.getId().toString()).collect(Collectors.joining(",")); log.warn("The (scheduled) task '{}' for programming exercise {} failed for these {} participations: {}", operation, programmingExercise.getId(), failedOperations.size(), failedIds); } return failedOperations; });🧰 Tools
🪛 ast-grep
[warning] 997-997: Detected a cookie where the
Secureflag is either missing or disabled. TheSecurecookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecureflag totrueso the cookie will only be sent over HTTPS.
Context: (Collectors.joining(","))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 997-997: Detected a cookie where the
HttpOnlyflag is either missing or disabled. TheHttpOnlycookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnlyflag to true` in all other cases.
Context: (Collectors.joining(","))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfigurationsrc/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java (2)
391-391: Consider using a builder pattern for better readability.The DTO instantiation with multiple boolean parameters makes it hard to understand what each parameter represents. Consider using a builder pattern or introducing meaningful variable names for better readability.
-var exportOptions = new RepositoryExportOptionsDTO(false, false, false, null, true, false, false, false, false); +var exportOptions = RepositoryExportOptionsDTO.builder() + .excludePracticeSubmissions(false) + .normalizeCodeStyle(false) + .anonymizeStudentId(false) + .filterForAssignment(null) + .exportTestCases(true) + .exportFiles(false) + .exportBuildLogs(false) + .exportFeedback(false) + .exportResults(false) + .build();
1573-1573: Consider using Optional for null values in test data.When creating test data with null values, consider using Optional to make the intent more explicit and improve readability.
-new ProgrammingExerciseTestCaseDTO(null, null, null, null, null) +ProgrammingExerciseTestCaseDTO.builder() + .id(Optional.empty()) + .weight(Optional.empty()) + .bonusMultiplier(Optional.empty()) + .bonusPoints(Optional.empty()) + .visibility(Optional.empty()) + .build()Also applies to: 1580-1580
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseGradingService.java (1)
Line range hint
1010-1036: Consider using interface types instead of implementation classes for variable declarations.In the
generateGradingStatisticsmethod, variables liketestCaseStatsMapandcategoryIssuesStudentsMapare declared using implementation classes (HashMap). It's recommended to use interface types (Map) for variable declarations to promote flexibility and adherence to programming best practices.Apply this diff to use interface types:
- final var testCaseStatsMap = new HashMap<String, ProgrammingExerciseGradingStatisticsDTO.TestCaseStats>(); + final Map<String, ProgrammingExerciseGradingStatisticsDTO.TestCaseStats> testCaseStatsMap = new HashMap<>(); ... - final var categoryIssuesStudentsMap = new HashMap<String, Map<Integer, Integer>>(); + final Map<String, Map<Integer, Integer>> categoryIssuesStudentsMap = new HashMap<>();🧰 Tools
🪛 ast-grep
[warning] 1094-1094: Detected a cookie where the
Secureflag is either missing or disabled. TheSecurecookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecureflag totrueso the cookie will only be sent over HTTPS.
Context: (testName, new ProgrammingExerciseGradingStatisticsDTO.TestCaseStats(0, 0))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 1119-1119: Detected a cookie where the
Secureflag is either missing or disabled. TheSecurecookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecureflag totrueso the cookie will only be sent over HTTPS.
Context: (category, new HashMap<>())
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 1094-1094: Detected a cookie where the
HttpOnlyflag is either missing or disabled. TheHttpOnlycookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnlyflag to true` in all other cases.
Context: (testName, new ProgrammingExerciseGradingStatisticsDTO.TestCaseStats(0, 0))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 1119-1119: Detected a cookie where the
HttpOnlyflag is either missing or disabled. TheHttpOnlycookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnlyflag to true` in all other cases.
Context: (category, new HashMap<>())
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (20)
- src/main/java/de/tum/cit/aet/artemis/assessment/repository/ComplaintRepository.java (6 hunks)
- src/main/java/de/tum/cit/aet/artemis/assessment/repository/ResultRepository.java (0 hunks)
- src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/core/dto/RepositoryExportOptionsDTO.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/core/service/DataExportScheduleService.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportExerciseCreationService.java (0 hunks)
- src/main/java/de/tum/cit/aet/artemis/exam/service/ExamDateService.java (3 hunks)
- src/main/java/de/tum/cit/aet/artemis/exam/service/StudentExamService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/dto/ProgrammingExerciseGradingStatisticsDTO.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/dto/ProgrammingExerciseTestCaseDTO.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/repository/BuildPlanRepository.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java (7 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseGradingService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseScheduleService.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseTestCaseService.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (2 hunks)
- src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseGradingServiceTest.java (2 hunks)
- src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java (11 hunks)
- src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseTestCaseServiceTest.java (2 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/de/tum/cit/aet/artemis/assessment/repository/ResultRepository.java
- src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportExerciseCreationService.java
✅ Files skipped from review due to trivial changes (2)
- src/main/java/de/tum/cit/aet/artemis/programming/repository/BuildPlanRepository.java
- src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseGradingServiceTest.java
🧰 Additional context used
📓 Path-based instructions (16)
src/main/java/de/tum/cit/aet/artemis/assessment/repository/ComplaintRepository.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_importssrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.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_importssrc/main/java/de/tum/cit/aet/artemis/core/dto/RepositoryExportOptionsDTO.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_importssrc/main/java/de/tum/cit/aet/artemis/core/service/DataExportScheduleService.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_importssrc/main/java/de/tum/cit/aet/artemis/exam/service/ExamDateService.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_importssrc/main/java/de/tum/cit/aet/artemis/exam/service/StudentExamService.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_importssrc/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_importssrc/main/java/de/tum/cit/aet/artemis/programming/dto/ProgrammingExerciseGradingStatisticsDTO.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_importssrc/main/java/de/tum/cit/aet/artemis/programming/dto/ProgrammingExerciseTestCaseDTO.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_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.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_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseGradingService.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_importssrc/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_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseTestCaseService.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_importssrc/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_importssrc/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.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: truesrc/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseTestCaseServiceTest.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
📓 Learnings (1)
src/main/java/de/tum/cit/aet/artemis/exam/service/StudentExamService.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-10-20T18:37:45.365Z 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.
🪛 ast-grep
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseGradingService.java
[warning] 1013-1013: Detected a cookie where the
Secureflag is either missing or disabled. TheSecurecookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecureflag totrueso the cookie will only be sent over HTTPS.
Context: (testCase.getTestName(), new ProgrammingExerciseGradingStatisticsDTO.TestCaseStats(0, 0))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 1013-1013: Detected a cookie where the
HttpOnlyflag is either missing or disabled. TheHttpOnlycookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnlyflag to true` in all other cases.
Context: (testCase.getTestName(), new ProgrammingExerciseGradingStatisticsDTO.TestCaseStats(0, 0))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 1094-1094: Detected a cookie where the
Secureflag is either missing or disabled. TheSecurecookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecureflag totrueso the cookie will only be sent over HTTPS.
Context: (testName, new ProgrammingExerciseGradingStatisticsDTO.TestCaseStats(0, 0))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 1119-1119: Detected a cookie where the
Secureflag is either missing or disabled. TheSecurecookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecureflag totrueso the cookie will only be sent over HTTPS.
Context: (category, new HashMap<>())
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 1094-1094: Detected a cookie where the
HttpOnlyflag is either missing or disabled. TheHttpOnlycookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnlyflag to true` in all other cases.
Context: (testName, new ProgrammingExerciseGradingStatisticsDTO.TestCaseStats(0, 0))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 1119-1119: Detected a cookie where the
HttpOnlyflag is either missing or disabled. TheHttpOnlycookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnlyflag to true` in all other cases.
Context: (category, new HashMap<>())
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfigurationsrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseScheduleService.java
[warning] 986-987: Detected a cookie where the
Secureflag is either missing or disabled. TheSecurecookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecureflag totrueso the cookie will only be sent over HTTPS.
Context: (String.format("'%s' failed for programming exercise with id %d for student repository with participation id %d", operationName,
programmingExercise.getId(), participation.getId()), exception)
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 997-997: Detected a cookie where the
Secureflag is either missing or disabled. TheSecurecookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecureflag totrueso the cookie will only be sent over HTTPS.
Context: (Collectors.joining(","))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 986-987: Detected a cookie where the
HttpOnlyflag is either missing or disabled. TheHttpOnlycookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnlyflag to true` in all other cases.
Context: (String.format("'%s' failed for programming exercise with id %d for student repository with participation id %d", operationName,
programmingExercise.getId(), participation.getId()), exception)
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 997-997: Detected a cookie where the
HttpOnlyflag is either missing or disabled. TheHttpOnlycookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnlyflag to true` in all other cases.
Context: (Collectors.joining(","))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfigurationsrc/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java
[warning] 438-438: Detected a cookie where the
Secureflag is either missing or disabled. TheSecurecookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecureflag totrueso the cookie will only be sent over HTTPS.
Context: (participantIdentifiers.split(","))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 438-438: Detected a cookie where the
HttpOnlyflag is either missing or disabled. TheHttpOnlycookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnlyflag to true` in all other cases.
Context: (participantIdentifiers.split(","))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfigurationsrc/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java
[warning] 1630-1631: Detected a cookie where the
HttpOnlyflag is either missing or disabled. TheHttpOnlycookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnlyflag to true` in all other cases.
Context: ("/api" + endpoint, updates, new TypeReference<List>() {
}, HttpStatus.OK)
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 1630-1631: Detected a cookie where the
Secureflag is either missing or disabled. TheSecurecookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecureflag totrueso the cookie will only be sent over HTTPS.
Context: ("/api" + endpoint, updates, new TypeReference<List>() {
}, HttpStatus.OK)
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
🔇 Additional comments (20)
src/main/java/de/tum/cit/aet/artemis/programming/dto/ProgrammingExerciseTestCaseDTO.java (1)
Line range hint
1-14: LGTM! Successfully converted to recordThe conversion to a Java record aligns perfectly with the PR objectives and coding guidelines. It provides:
- Immutability
- Reduced boilerplate
- Clear data structure
- Type safety
src/main/java/de/tum/cit/aet/artemis/programming/dto/ProgrammingExerciseGradingStatisticsDTO.java (3)
Line range hint
1-16: LGTM! Well-documented record declaration.The record declaration and documentation follow best practices:
- Proper JavaDoc with parameter descriptions
- Appropriate use of @JsonInclude annotation
- Clear and concise component naming
27-29: LGTM! Well-structured inner record.The TestCaseStats record is appropriately designed:
- Uses primitive types as recommended
- Follows single responsibility principle
- Properly annotated for JSON serialization
Line range hint
1-29: Verify UI compatibility in exam mode.Since the PR objectives mention exam mode UI verification, please ensure this DTO conversion doesn't affect the UI rendering.
Let's check for UI-related usages:
✅ Verification successful
No UI compatibility issues with DTO conversion
The conversion to record is safe because:
- The DTO is only used in the backend for REST API responses
- The JSON structure remains identical due to:
- Same field names and types
- Maintained
@JsonIncludeannotations- Identical serialization pattern verified in tests
- No direct UI component dependencies found
- The REST endpoint
/api/programming-exercises/{exerciseId}/grading-statisticsmaintains the same contract, ensuring UI consumers receive identical JSON structureThe exam mode UI will not be affected as this is a purely backend refactoring that preserves the JSON contract.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to this DTO in UI-related code echo "Searching for DTO usage in UI components..." rg -l "ProgrammingExerciseGradingStatisticsDTO" src/main/webapp/Length of output: 161
Script:
#!/bin/bash # Let's broaden our search to find all references to this DTO echo "Searching for all references to the DTO..." rg -l "ProgrammingExerciseGradingStatisticsDTO" # Let's also check for any REST endpoints that might use this DTO echo -e "\nSearching for REST endpoints using this DTO..." ast-grep --pattern 'class $_ { $$$ @$$$ public $_ $_(ProgrammingExerciseGradingStatisticsDTO $_) { $$$ } $$$ }' # Check for any test cases that might reveal usage patterns echo -e "\nSearching for test cases..." fd -e java -e ts --full-path ".*[Tt]est.*" --exec rg -l "ProgrammingExerciseGradingStatisticsDTO" {}Length of output: 1150
Script:
#!/bin/bash # Let's examine how this DTO is used in the service and resource classes echo "Checking ProgrammingExerciseGradingResource usage..." rg -A 10 "ProgrammingExerciseGradingStatisticsDTO" src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseGradingResource.java echo -e "\nChecking ProgrammingExerciseGradingService usage..." rg -A 10 "ProgrammingExerciseGradingStatisticsDTO" src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseGradingService.java echo -e "\nChecking test cases for serialization patterns..." rg -A 10 "ProgrammingExerciseGradingStatisticsDTO" src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseGradingServiceTest.javaLength of output: 9201
src/main/java/de/tum/cit/aet/artemis/core/dto/RepositoryExportOptionsDTO.java (1)
15-16: LGTM! Well-structured default constructor.The constructor properly initializes all fields with sensible defaults and correctly delegates to the canonical constructor.
src/main/java/de/tum/cit/aet/artemis/core/service/DataExportScheduleService.java (1)
75-94: Well-implemented concurrent processing with proper resource management.The implementation shows good practices:
- Thread-safe collection with
Collections.synchronizedSet- Appropriate timeout handling with
awaitTermination- Proper cleanup with
shutdownNow- Good error logging
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseTestCaseServiceTest.java (2)
98-98: LGTM: Clean conversion to record constructor.The change aligns well with the PR objective of converting DTOs to records, using the new record constructor pattern effectively.
129-131: LGTM: Clean stream-based conversion to record constructors.The change effectively uses Java streams with the new record constructor pattern, maintaining good functional style.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseTestCaseService.java (1)
72-73: LGTM! Clean stream operation with proper record access.The stream operation efficiently finds the matching test case using the correct record accessor syntax.
src/main/java/de/tum/cit/aet/artemis/exam/service/ExamDateService.java (2)
11-12: LGTM: Appropriate import additions for nullable annotations.The addition of Jakarta validation annotations follows best practices for null-safety.
199-203: Address TODO comment with proper design.The TODO comment suggests uncertainty about the exam exercise check. Consider:
- Moving this logic to the constructor/creation of ProgrammingExercise
- Using Optional to make the null case more explicit
- Adding a precondition check
Let's verify if this check is consistently implemented:
✅ Verification successful
Based on the search results, I can now generate the final response since I have enough context about how
isExamExercise()is used throughout the codebase.The null check for exam exercise is consistently implemented and serves a valid purpose.
The null check in
ExamDateServiceis appropriate because:
- The method is used by various components that handle both exam and non-exam exercises
- The codebase consistently checks
isExamExercise()before accessing exam-specific properties- The pattern of early return for non-exam exercises is used in multiple places:
ExamDateService.getLatestIndividualExamEndDate()ExamSubmissionService.isAllowedToSubmit()ExamAccessService.checkExamExerciseForExampleSolutionAccess()The TODO comment can be removed as the current implementation follows established patterns in the codebase for handling exam vs non-exam exercise scenarios.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar null checks for exam exercises rg "isExamExercise\(\)" -A 3Length of output: 72961
src/main/java/de/tum/cit/aet/artemis/assessment/repository/ComplaintRepository.java (2)
294-295: Consistent type handling across similar queries.The same type safety improvements are consistently applied across related queries for exercise and exam-specific complaints.
Also applies to: 319-320
345-345: Type safety improvement for maxPoints summation.The CAST operation ensures proper type handling for maxPoints summation.
Let's verify the column type of maxPoints to ensure CAST is necessary:
✅ Verification successful
Based on the search results, I can now provide a final response:
CAST operation is necessary for maxPoints summation in ComplaintRepository.java
The CAST operation is indeed necessary because:
- The
maxPointsfield is defined asDoubleinBaseExercise.java- In the SQL query context (ComplaintRepository.java), the CAST ensures proper type handling for aggregation functions (SUM)
- This prevents potential precision loss or type mismatch issues during database operations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the type of maxPoints column in the exercise table ast-grep --pattern 'class Exercise { $$$ $type maxPoints; $$$ }'Length of output: 196
Script:
#!/bin/bash # Let's try a different approach to find the Exercise entity and maxPoints field rg -A 5 -B 5 "class Exercise"Length of output: 136355
Script:
#!/bin/bash # Also search for JPA/Hibernate annotations related to maxPoints rg "@Column.*maxPoints|private.*maxPoints"Length of output: 878
Script:
#!/bin/bash # Search for maxPoints in repository queries rg "maxPoints" --type javaLength of output: 22693
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (2)
427-445: LGTM! Good refactoring to use immutable DTOs.The changes improve code maintainability by:
- Using Java record's immutable properties
- Extracting the participation filtering logic to a separate method
🧰 Tools
🪛 ast-grep
[warning] 438-438: Detected a cookie where the
Secureflag is either missing or disabled. TheSecurecookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecureflag totrueso the cookie will only be sent over HTTPS.
Context: (participantIdentifiers.split(","))
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 438-438: Detected a cookie where the
HttpOnlyflag is either missing or disabled. TheHttpOnlycookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnlyflag to true` in all other cases.
Context: (participantIdentifiers.split(","))
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
479-483: LGTM! Consistent use of immutable DTO methods.The changes maintain consistency with the immutable DTO pattern used throughout the class.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java (2)
264-264: LGTM: Constructor-based initialization of RepositoryExportOptionsDTOThe change aligns with the PR objective of converting DTOs to records by using constructor-based initialization instead of individual setters.
795-799: LGTM: Clear handling of late submission filteringThe implementation properly handles both individual due dates and specific filter dates for late submissions.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseScheduleService.java (1)
964-965: Good use of try-with-resources for ExecutorService management!The implementation ensures proper cleanup of thread pool resources.
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java (1)
1639-1640: LGTM! Well-structured helper method for DTO transformation.The
transformTestCasesToDtomethod is well-designed and follows good practices by:
- Using a descriptive method name
- Taking a Collection as input for flexibility
- Using stream operations efficiently
- Having a clear single responsibility
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseGradingService.java (1)
989-1007: Well-documented method with clear intent.The JavaDoc comments for
generateGradingStatisticsare comprehensive and provide a clear understanding of the method's purpose and implementation steps.
src/main/java/de/tum/cit/aet/artemis/programming/dto/ProgrammingExerciseTestCaseDTO.java
Show resolved
Hide resolved
...ain/java/de/tum/cit/aet/artemis/programming/dto/ProgrammingExerciseGradingStatisticsDTO.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/core/dto/RepositoryExportOptionsDTO.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/core/dto/RepositoryExportOptionsDTO.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/core/service/DataExportScheduleService.java
Show resolved
Hide resolved
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java
Show resolved
Hide resolved
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseGradingService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseGradingService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseGradingService.java
Show resolved
Hide resolved
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java
Outdated
Show resolved
Hide resolved
…erciseIntegrationTestService.java Co-authored-by: Johannes Stöhr <[email protected]>
krusche
dismissed stale reviews from julian-christl and JohannesStoehr
fixed
Checklist
General
Server
Motivation and Context
We want to use record instead of classes for DTOs due to their advantages, e.g. being immutable and having less cluttered code.
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
Only code review is necessary, when all checks (in particular server tests and e2e tests pass)
Code Review
Summary by CodeRabbit
New Features
Refactor
RepositoryExportOptionsDTOand improved its usage across various services.StudentExamServiceandProgrammingExerciseTestCaseService.Bug Fixes
Tests