Development: Introduce athena module API
#10294
Conversation
github-actions
bot
added
tests
server
WalkthroughThis pull request consolidates several Athena-related service dependencies by introducing new API classes. A base abstract class (AbstractAthenaApi) and concrete implementations (AthenaApi and AthenaFeedbackApi) now encapsulate functionalities that were previously split across multiple services. Many instances of AthenaScheduleService, AthenaModuleService, AthenaSubmissionSelectionService, AthenaFeedbackSendingService, and AthenaFeedbackSuggestionsService have been replaced by AthenaApi or AthenaFeedbackApi. Additionally, a new ApiNotPresentException is introduced and architectural tests for the Athena module have been added or updated. Changes
Sequence Diagram(s)sequenceDiagram
participant IMS as InstanceMessageReceiveService
participant AA as AthenaApi
IMS->>AA: scheduleExerciseForAthenaIfRequired(exercise)
AA-->>IMS: Confirmation/Result
sequenceDiagram
participant Client as API Client
participant AFA as AthenaFeedbackApi
Client->>AFA: getTextFeedbackSuggestions(exercise, submission, isGraded)
AFA-->>Client: List<TextFeedbackDTO>
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 PMD (7.8.0)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseCodeReviewFeedbackService.java[ERROR] Error at ruleset.xml:58:5 59| 67| 72| 76| 79| 80| 82| 83| 84| 85| 86| 87| 88| 91| 92| 107| 120| 125| 135| 138| 185| src/main/java/de/tum/cit/aet/artemis/athena/api/AthenaFeedbackApi.java[ERROR] Error at ruleset.xml:58:5 59| 67| 72| 76| 79| 80| 82| 83| 84| 85| 86| 87| 88| 91| 92| 107| 120| 125| 135| 138| 185| src/main/java/de/tum/cit/aet/artemis/athena/api/AbstractAthenaApi.java[ERROR] Error at ruleset.xml:58:5 59| 67| 72| 76| 79| 80| 82| 83| 84| 85| 86| 87| 88| 91| 92| 107| 120| 125| 135| 138| 185|
✨ 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: 0
🧹 Nitpick comments (3)
src/main/java/de/tum/cit/aet/artemis/athena/api/AthenaFeedbackApi.java (1)
1-3: Consider declaring this as a REST endpoint if consumed as an external API.
The current class is annotated with @controller, which typically expects view-based rendering. If this API is meant for REST-based usage, switching to @RestController (or adding @responsebody to the relevant methods) might simplify usage and avoid confusion.src/main/java/de/tum/cit/aet/artemis/athena/api/AthenaApi.java (1)
34-56: Consider adding null checks and validation.While the service delegation is clean, consider adding parameter validation and null checks before delegating to services.
Example for
scheduleExerciseForAthenaIfRequired:public void scheduleExerciseForAthenaIfRequired(Exercise exercise) { + if (exercise == null) { + throw new IllegalArgumentException("Exercise cannot be null"); + } athenaScheduleService.scheduleExerciseForAthenaIfRequired(exercise); }src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (1)
1-1468: Great architectural improvement!The changes successfully implement the module API pattern by:
- Encapsulating athena-specific functionality behind a well-defined API
- Reducing direct dependencies on internal services
- Making the interaction between components more structured
This aligns well with the PR objectives of improving server modularization.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/main/java/de/tum/cit/aet/artemis/athena/api/AbstractAthenaApi.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/api/AthenaApi.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/api/AthenaFeedbackApi.java(1 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/messaging/InstanceMessageReceiveService.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/exercise/service/SubmissionService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadSubmissionService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseFeedbackService.java(7 hunks)src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingSubmissionService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseCodeReviewFeedbackService.java(6 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingSubmissionService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java(6 hunks)src/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseFeedbackService.java(6 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/TextAssessmentResource.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java(7 hunks)src/test/java/de/tum/cit/aet/artemis/athena/architecture/AthenaApiArchitectureTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/shared/architecture/module/AbstractModuleAccessArchitectureTest.java(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/de/tum/cit/aet/artemis/athena/api/AbstractAthenaApi.java
🧰 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/core/exception/ApiNotPresentException.javasrc/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadSubmissionService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingSubmissionService.javasrc/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.javasrc/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseFeedbackService.javasrc/main/java/de/tum/cit/aet/artemis/text/web/TextAssessmentResource.javasrc/main/java/de/tum/cit/aet/artemis/core/service/messaging/InstanceMessageReceiveService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.javasrc/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingSubmissionService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseCodeReviewFeedbackService.javasrc/main/java/de/tum/cit/aet/artemis/text/service/TextSubmissionService.javasrc/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.javasrc/main/java/de/tum/cit/aet/artemis/athena/api/AthenaApi.javasrc/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseFeedbackService.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.javasrc/main/java/de/tum/cit/aet/artemis/athena/api/AthenaFeedbackApi.javasrc/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.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/athena/architecture/AthenaApiArchitectureTest.javasrc/test/java/de/tum/cit/aet/artemis/shared/architecture/module/AbstractModuleAccessArchitectureTest.java
📓 Learnings (3)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.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/service/ProgrammingExerciseCodeReviewFeedbackService.java (1)
Learnt from: undernagruzez
PR: ls1intum/Artemis#8498
File: src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseCodeReviewFeedbackService.java:236-236
Timestamp: 2024-11-12T12:51:40.391Z
Learning: Rate limit exceptions for AI feedback requests are handled in the client app.
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.
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: client-tests
- GitHub Check: client-style
- GitHub Check: server-tests
- GitHub Check: server-style
- GitHub Check: Analyse
🔇 Additional comments (54)
src/main/java/de/tum/cit/aet/artemis/athena/api/AthenaFeedbackApi.java (2)
29-39: Constructor injection is correctly used.
This approach is consistent with the coding guidelines and promotes cleaner object instantiation. No changes required here.
40-63: Validate exception handling strategy.
All these public methods can throw a NetworkingException (or call downstream methods that can). Ensure that any upstream callers or controllers properly handle these exceptions and provide a meaningful response to the user.src/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseFeedbackService.java (4)
26-27: Imports and optional injection transition to AthenaFeedbackApi look good.
Replacing direct usage of AthenaFeedbackSuggestionsService with AthenaFeedbackApi aligns with the new architecture. No issues identified.Also applies to: 42-42
56-58: Constructor injection of AthenaFeedbackApi is consistent with best practices.
This change keeps the code modular and testable.
76-78: Asynchronous invocation might mask exceptions.
Because you're using CompletableFuture.runAsync, exceptions thrown within the lambda won't be rethrown on the calling thread. Ensure your logs or error handlers capture problems effectively, as the main thread won't see them.Also applies to: 86-86
122-123: ApiNotPresentException usage ensures robust fallback logic.
Throwing this exception clarifies when the Athena profile is unavailable. Good practice for optional dependencies. No changes needed.src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseFeedbackService.java (4)
23-23: Refactoring to AthenaFeedbackApi is structurally consistent.
Replacing AthenaFeedbackSuggestionsService with AthenaFeedbackApi (in alignment with the new architecture) follows the single-responsibility principle.Also applies to: 25-25, 42-42
54-56: Constructor injection approach is on point.
Ensures dependencies are explicit and promotes testability.
73-75: CompletableFuture consideration & exception handling.
Similar to the text feedback service, the asynchronous feedback generation process should be carefully monitored and logged in case of exceptions, ensuring they don’t silently fail.Also applies to: 86-86
165-167: Lambda stream usage is clear and concise.
The filtering step using .filter(...) before mapping is straightforward and easy to read. No changes needed.src/test/java/de/tum/cit/aet/artemis/athena/architecture/AthenaApiArchitectureTest.java (1)
8-19: LGTM! Well-structured architecture test implementation.The test class follows best practices:
- Descriptive naming convention
- Proper extension of base test class
- Clear method overrides
src/main/java/de/tum/cit/aet/artemis/core/exception/ApiNotPresentException.java (1)
6-19: LGTM! Well-designed exception class with clear documentation.The implementation follows best practices:
- Clear JavaDoc documentation
- Informative error message
- Proper parameter usage
src/main/java/de/tum/cit/aet/artemis/athena/api/AthenaApi.java (2)
18-20: LGTM! Proper Spring configuration.The class is correctly configured with:
- Profile-specific activation
- Controller annotation
- Proper inheritance
22-32: LGTM! Clean dependency injection pattern.The implementation follows best practices:
- Constructor injection
- Final fields for immutability
- Clear service dependencies
src/test/java/de/tum/cit/aet/artemis/shared/architecture/module/AbstractModuleAccessArchitectureTest.java (4)
27-32: LGTM! Improved architecture rule with ignored classes support.The modification correctly excludes ignored classes from the architecture rule check.
35-38: LGTM! Enhanced inheritance check with exclusions.The rule properly validates API class inheritance while respecting ignored classes.
41-44: LGTM! Updated controller annotation check.The rule correctly validates API class structure with proper exclusions.
58-60: LGTM! Clean default implementation.The method provides a good default empty set, allowing subclasses to override when needed.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java (4)
23-23: LGTM!The import statement correctly references the new API class.
44-44: LGTM!The field declaration follows Java best practices with proper access modifiers and type declaration.
48-54: LGTM!The constructor properly injects the new API dependency and maintains the same functionality.
143-145: LGTM!The method correctly checks for API presence and delegates the feedback sending to the API.
src/main/java/de/tum/cit/aet/artemis/text/service/TextSubmissionService.java (2)
20-20: LGTM!The import statement correctly references the new API class.
57-59: LGTM!The constructor properly injects the new API dependency and maintains the same functionality.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseCodeReviewFeedbackService.java (5)
23-23: LGTM!The import statements correctly reference the new API class and the exception class.
Also applies to: 26-26
50-50: LGTM!The field declaration follows Java best practices with proper access modifiers and type declaration.
64-70: LGTM!The constructor properly injects the new API dependency and maintains the same functionality.
90-92: LGTM!The method correctly checks for API presence and delegates the rate limit check to the API.
137-138: LGTM!The code properly handles the case when the API is not present by throwing a specific exception.
src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingSubmissionService.java (2)
26-26: LGTM!The import statement correctly references the new API class.
71-73: LGTM!The constructor properly injects the new API dependency and maintains the same functionality.
src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadSubmissionService.java (1)
26-26: LGTM! Clean refactoring to use the new AthenaApi.The changes correctly update the import statement and constructor to use the new AthenaApi while maintaining the Optional pattern for dependency injection.
Also applies to: 65-67
src/main/java/de/tum/cit/aet/artemis/core/service/messaging/InstanceMessageReceiveService.java (1)
18-18: LGTM! Clean refactoring to use the new AthenaApi.The changes correctly:
- Update the import statement and field type to use AthenaApi
- Maintain the Optional pattern for dependency injection
- Update the method calls to use the new API methods
Also applies to: 52-52, 70-71, 75-75, 234-234, 239-239
src/main/java/de/tum/cit/aet/artemis/text/web/TextAssessmentResource.java (1)
43-43: LGTM! Clean refactoring to use the new AthenaFeedbackApi.The changes correctly:
- Update the import statement and field type to use AthenaFeedbackApi
- Maintain the Optional pattern for dependency injection
- Update the method calls to use the new API methods
Also applies to: 102-102, 108-108, 120-120, 523-525
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (1)
44-44: LGTM! Clean refactoring to use the new AthenaApi.The changes correctly:
- Update the import statement and field type to use AthenaApi
- Maintain the Optional pattern for dependency injection
- Update the method call to use the new API method
Also applies to: 132-132, 141-142, 157-157, 248-248
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (4)
43-43: Import changes managed by formatter.
155-155: LGTM! Field declaration correctly uses Optional for dependency injection.The change from
AthenaModuleServicetoAthenaApiis consistent with the module API refactoring.
228-228: LGTM! Access control check correctly handles both cases.The change preserves the existing behavior while using the new API:
- If athenaApi is present: checks access using the new API
- If athenaApi is not present: disables feedback suggestions
281-284: LGTM! Access control and validation checks are correctly implemented.The changes maintain the required checks while using the new API:
- Access control check with proper fallback
- Additional validation to prevent module changes after due date
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingSubmissionService.java (2)
32-32: Import changes managed by formatter.
118-120: LGTM! Constructor correctly updated to use the new API.The changes maintain consistency:
- Parameter type updated to
Optional<AthenaApi>- Super call correctly passes the new parameter
src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java (4)
34-34: Import changes managed by formatter.
92-92: LGTM! Field declaration correctly uses Optional for dependency injection.The change from
AthenaSubmissionSelectionServicetoAthenaApiis consistent with the module API refactoring.
97-98: LGTM! Constructor correctly updated to use the new API.The parameter type is correctly updated to
Optional<AthenaApi>while maintaining the parameter order.
273-276: LGTM! Method correctly uses the new API while maintaining behavior.The changes preserve the existing functionality:
- Checks if feedback suggestions are enabled and API is present
- Correctly retrieves the proposed submission ID using the new API
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (5)
44-44: Import changes managed by formatter.
155-155: LGTM! Field declaration correctly uses Optional for dependency injection.The change from
AthenaModuleServicetoAthenaApiis consistent with the module API refactoring.
168-191: LGTM! Constructor correctly updated to use the new API.The changes maintain consistency:
- Parameter type updated to
Optional<AthenaApi>- Field assignment correctly uses the new parameter
248-248: LGTM! Access control check correctly handles both cases.The change preserves the existing behavior while using the new API:
- If athenaApi is present: checks access using the new API
- If athenaApi is not present: disables feedback suggestions
342-345: LGTM! Access control and validation checks are correctly implemented.The changes maintain the required checks while using the new API:
- Access control check with proper fallback
- Additional validation to prevent module changes after due date
src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (4)
67-67: LGTM!The import statement for
AthenaApiis correctly placed and follows Java import conventions.
178-178: LGTM!The field declaration follows best practices:
- Uses
finalfor immutability- Uses
Optionalto handle potential null values- Follows Java naming conventions
198-199: LGTM!The constructor changes are consistent with the field changes and follow dependency injection best practices.
Also applies to: 218-218
333-336: LGTM!The method call is correctly updated to use the new API and maintains proper null safety using
Optional#ifPresent.
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 athena.
Steps for Testing
Can only be test on TS1, TS2, or TS3 (athena needs to be enabled)
Basically try to test all the functionality of athena regarding submissions, courses, and import/export.
Exam Mode Testing
not relevant - athena is not used for exam-related functionality
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Summary by CodeRabbit
New Features
• Introduced comprehensive endpoints that enhance exercise management, including scheduling, cancellation, submission proposals, and tailored feedback suggestions for text, programming, and modeling exercises.
Refactor
• Streamlined Athena-related functionality across courses, submissions, and assessments to ensure a more consistent and reliable user experience.