Development: Convert athena endpoints to return file maps of repositories
#11357
Conversation
github-actions
bot
added
tests
server
ekayandan
changed the title
Development: Convert athena endpoints to return file maps of repositories
helios-aet
bot
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
End-to-End (E2E) Test Results Summary
|
|||||||||||||||||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||||||||||||||
…m:ls1intum/Artemis into chore/convert-athena-endpoints-to-filemap
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
WalkthroughReplaces ZIP-based repository export with Map<String,String> text-file content retrieval via RepositoryService; adds deadline-aware bare-repo APIs; consolidates instructor endpoints using RepositoryType.fromString; adds feedback-module null guards and module validation exception; updates localization keys and tests to use LocalVC and JSON map assertions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant REST as AthenaResource
participant Service as AthenaRepositoryExportService
participant RepoSvc as RepositoryService
participant VCS as Bare VCS Repo
Client->>REST: GET repository (exerciseId, submissionId / repositoryType)
REST->>Service: getStudentRepositoryFilesContent(...) / getInstructorRepositoryFilesContent(...)
Service->>Service: resolve exercise/participation -> repoUri (BadRequest if null)
alt deadline present
Service->>RepoSvc: getFilesContentFromBareRepositoryForLastCommitBeforeOrAt(repoUri, deadline)
else
Service->>RepoSvc: getFilesContentFromBareRepositoryForLastCommit(repoUri)
end
RepoSvc->>VCS: open bare repo, find commit (HEAD or last ≤ deadline)
VCS-->>RepoSvc: tree contents (text files only)
RepoSvc-->>Service: Map<String,String>
Service-->>REST: Map<String,String>
REST-->>Client: 200 OK (JSON map)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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 (13)
src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java (1)
54-56: Stabilize serverUrl injection to avoid nulls in CIProvide a default to prevent "null/api/…" expectations when server.url isn’t set.
Apply:
- @Value("${server.url}") + @Value("${server.url:http://localhost:8080}") private String serverUrl;src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java (1)
216-241: Commit selection by deadline: minor nits (epoch conversion + topo ordering)Current logic is correct. Two tiny hardening tweaks:
- Use ZonedDateTime.toEpochSecond().
- Add TOPO sort to keep traversal stable when commit times collide.
Apply:
- long epochSeconds = deadline.toInstant().getEpochSecond(); + long epochSeconds = deadline.toEpochSecond(); try (RevWalk walk = new RevWalk(repository)) { walk.markStart(walk.parseCommit(headCommitId)); walk.sort(RevSort.COMMIT_TIME_DESC, true); + walk.sort(RevSort.TOPO, true);src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java (3)
65-74: Fix Javadoc: thrown exception typeMethod can surface ServiceUnavailableException (via checkFeedbackSuggestions…), not AccessForbiddenException.
Apply:
- * @throws AccessForbiddenException if the feedback suggestions are not enabled for the given exercise + * @throws ServiceUnavailableException if the feedback suggestions are not enabled for the given exercise
103-109: Deadline handling LGTM; consider simplifying source of deadline (optional)Using RepositoryExportOptionsDTO just to read dueDate is a bit indirect; consider using programmingExercise.getDueDate() directly to reduce coupling.
95-104: Require submissionId for student repositories to fail fast with a clear message
Apply:- var submission = programmingSubmissionRepository.findById(submissionId).orElseThrow(); + java.util.Objects.requireNonNull(submissionId, "submissionId must be provided for student repositories"); + var submission = programmingSubmissionRepository.findById(submissionId).orElseThrow();src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaRepositoryExportServiceTest.java (2)
66-92: Rename test to reflect new contract (maps, not ZIP).Clarifies intent and avoids legacy terminology.
- void shouldExportRepository() throws Exception { + void shouldReturnFileMap() throws Exception {
85-92: Strengthen assertions and pass null submissionId for non-student repos.
- SOLUTION repo doesn’t need a submissionId; passing null is clearer.
- Assert non-empty maps to meet “assert_specificity” guideline.
- Map<String, String> resultSolutionRepo = athenaRepositoryExportService.getRepositoryFilesContent(programmingExerciseWithId.getId(), programmingSubmissionWithId.getId(), - RepositoryType.SOLUTION); + Map<String, String> resultSolutionRepo = athenaRepositoryExportService.getRepositoryFilesContent( + programmingExerciseWithId.getId(), null, RepositoryType.SOLUTION); - assertThat(resultStudentRepo).isNotNull(); // The student repository files are returned - assertThat(resultSolutionRepo).isNotNull(); // The solution repository files are returned + assertThat(resultStudentRepo).isNotNull().isNotEmpty(); // Student repository files are returned + assertThat(resultSolutionRepo).isNotNull().isNotEmpty(); // Solution repository files are returnedIf flakiness appears, ensure a committed file exists at HEAD in the relevant bare repos before invoking the service.
src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java (2)
390-414: LocalVC seeding looks good; add a positive test for student submission endpoint.Symmetry with template/solution/tests improves coverage of the new API surface.
Add this test (outside this hunk):
@Test void testStudentRepositoryExportEndpoint() throws Exception { // Enable Athena programmingExercise.setFeedbackSuggestionModule(ATHENA_MODULE_PROGRAMMING_TEST); programmingExerciseRepository.save(programmingExercise); // Seed repos (reuse the same setup as in testRepositoryExportEndpoint or extract to helper) var sourceRepo = new LocalRepository(defaultBranch); sourceRepo.configureRepos(localVCRepoPath, "athenaSrcLocalRepo2", "athenaSrcOriginRepo2"); var testsSlug = programmingExercise.getProjectKey().toLowerCase() + "-tests"; var testsUri = new LocalVCRepositoryUri(localVCBaseUri, programmingExercise.getProjectKey(), testsSlug); programmingExercise.setTestRepositoryUri(testsUri.toString()); programmingExerciseRepository.save(programmingExercise); var sourceUri = new LocalVCRepositoryUri(localVCBaseUri, sourceRepo.remoteBareGitRepoFile.toPath()); gitService.copyBareRepositoryWithoutHistory(sourceUri, new LocalVCRepositoryUri(programmingExercise.getTemplateRepositoryUri()), defaultBranch); gitService.copyBareRepositoryWithoutHistory(sourceUri, new LocalVCRepositoryUri(programmingExercise.getSolutionRepositoryUri()), defaultBranch); gitService.copyBareRepositoryWithoutHistory(sourceUri, new LocalVCRepositoryUri(programmingExercise.getTestRepositoryUri()), defaultBranch); var authHeaders = new HttpHeaders(); authHeaders.add(HttpHeaders.AUTHORIZATION, athenaSecret); String json = request.get("/api/athena/public/programming-exercises/" + programmingExercise.getId() + "/submissions/" + programmingSubmission.getId() + "/repository", HttpStatus.OK, String.class, authHeaders); Map<String, String> repoFiles = request.getObjectMapper().readValue(json, new com.fasterxml.jackson.core.type.TypeReference<Map<String, String>>() {}); assertThat(repoFiles).isNotNull().hasSize(1).containsOnlyKeys("README.md").containsEntry("README.md", "Initial commit"); }
419-423: JSON extraction — OK; consider using mapper helper for brevity.If the test infrastructure supports it, prefer a helper that deserializes directly to Map<String, String>.
- String json = request.get("/api/athena/public/programming-exercises/" + programmingExercise.getId() + "/" + urlSuffix, HttpStatus.OK, String.class, authHeaders); - Map<String, String> repoFiles = request.getObjectMapper().readValue(json, new TypeReference<Map<String, String>>() { - }); + Map<String, String> repoFiles = request.get( + "/api/athena/public/programming-exercises/" + programmingExercise.getId() + "/" + urlSuffix, + HttpStatus.OK, + new TypeReference<Map<String, String>>() {}, + authHeaders);src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java (4)
256-271: Make content negotiation explicit.Add produces = MediaType.APPLICATION_JSON_VALUE to the mapping.
- @GetMapping("public/programming-exercises/{exerciseId}/submissions/{submissionId}/repository") + @GetMapping(value = "public/programming-exercises/{exerciseId}/submissions/{submissionId}/repository", produces = org.springframework.http.MediaType.APPLICATION_JSON_VALUE)Optionally add:
import org.springframework.http.MediaType;and use MediaType.APPLICATION_JSON_VALUE.
274-287: Ditto: declare JSON explicitly for template repo.- @GetMapping("public/programming-exercises/{exerciseId}/repository/template") + @GetMapping(value = "public/programming-exercises/{exerciseId}/repository/template", produces = org.springframework.http.MediaType.APPLICATION_JSON_VALUE)
290-303: Ditto: declare JSON explicitly for solution repo.- @GetMapping("public/programming-exercises/{exerciseId}/repository/solution") + @GetMapping(value = "public/programming-exercises/{exerciseId}/repository/solution", produces = org.springframework.http.MediaType.APPLICATION_JSON_VALUE)
306-319: Ditto: declare JSON explicitly for tests repo.- @GetMapping("public/programming-exercises/{exerciseId}/repository/tests") + @GetMapping(value = "public/programming-exercises/{exerciseId}/repository/tests", produces = org.springframework.http.MediaType.APPLICATION_JSON_VALUE)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java(5 hunks)src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaRepositoryExportServiceTest.java(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/test/java/**/*.java
⚙️ CodeRabbit configuration file
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
Files:
src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaRepositoryExportServiceTest.java
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
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
Files:
src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.javasrc/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java
🧠 Learnings (5)
📚 Learning: 2025-02-11T12:05:49.151Z
Learnt from: janthoXO
PR: ls1intum/Artemis#9406
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java:209-209
Timestamp: 2025-02-11T12:05:49.151Z
Learning: In ProgrammingExerciseParticipationResource, exam-related authorization checks and sensitive information filtering for results and feedbacks are handled by resultService.filterSensitiveInformationIfNecessary().
Applied to files:
src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java
📚 Learning: 2025-06-06T14:47:54.300Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:122-135
Timestamp: 2025-06-06T14:47:54.300Z
Learning: In Artemis sharing platform integration (under "sharing" profile), endpoints in ExerciseSharingResource use checksum-based authentication with shared secrets instead of PreAuthorize annotations. These endpoints (like getProblemStatement, getExerciseDetails, loadShoppingBasket) validate security through sharingInfo.checkChecksum() calls rather than role-based access control, as they serve as API interfaces for external sharing platform communication.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java
📚 Learning: 2024-11-26T20:43:17.588Z
Learnt from: magaupp
PR: ls1intum/Artemis#9751
File: src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java:143-148
Timestamp: 2024-11-26T20:43:17.588Z
Learning: In `src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java`, the test package name assigned in `populateUnreleasedProgrammingExercise` does not need to adhere to naming conventions.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java
📚 Learning: 2025-08-08T08:50:28.791Z
Learnt from: tobias-lippert
PR: ls1intum/Artemis#11248
File: src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java:401-402
Timestamp: 2025-08-08T08:50:28.791Z
Learning: In src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java, method findStudentParticipationWithLatestSubmissionResultAndFeedbacksElseThrow(long), using List.of() for latestSubmission.setResults(...) is acceptable because the results list is not mutated afterward and is only returned to the client; no follow-up code appends to it.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java
📚 Learning: 2024-10-08T15:35:48.767Z
Learnt from: jakubriegel
PR: ls1intum/Artemis#8050
File: src/test/java/de/tum/in/www1/artemis/plagiarism/PlagiarismUtilService.java:125-136
Timestamp: 2024-10-08T15:35:48.767Z
Learning: The `createTeamTextExerciseAndSimilarSubmissions` method in `PlagiarismUtilService.java` uses fixed inputs as it is designed to be a test helper method for simplifying the setup of team exercises and submissions in tests.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Mend Security Check
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: server-style
- GitHub Check: server-tests
- GitHub Check: bean-instantiation-check
- GitHub Check: Analyse
🔇 Additional comments (10)
src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java (1)
100-103: Guard against trailing slash in serverUrl
Trim trailing slashes inline to avoid//in the constructed URI for the JSON assertion.- jsonPath("$.submission.repositoryUri").value(serverUrl + "/api/athena/public/programming-exercises/" + programmingExercise.getId() + "/submissions/3/repository")); + jsonPath("$.submission.repositoryUri").value(serverUrl.replaceAll("/+$", "") + "/api/athena/public/programming-exercises/" + programmingExercise.getId() + "/submissions/3/repository"));Verify that your
server.urlproperty (in test and application configs) doesn’t include a trailing slash or is consistently normalized to prevent double slashes.src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java (1)
247-251: Overload delegation LGTMClean delegation to the repository-based variant with proper resource handling.
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java (1)
78-93: Instructor repo branch: null-URI guard LGTMClear error when the template/solution/tests repository isn’t initialized yet; subsequent call uses bare repo API correctly.
src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaRepositoryExportServiceTest.java (2)
10-10: Import aligns with new API shape — LGTM.
100-102: Exception path update — LGTM.Correctly asserts ServiceUnavailable when feature is disabled.
src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java (4)
13-13: Import for Map — LGTM.
25-25: Import for TypeReference — LGTM.
70-72: Autowired participation util — LGTM.Used later to prepare template/solution participations.
425-447: Negative-path coverage — LGTM.Forbidden and service-unavailable cases are properly asserted.
src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java (1)
9-9: Map-based response import — LGTM.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
|
Caution An unexpected error occurred while opening a pull request: Reference update failed - https://docs.github.com/rest/git/refs#create-a-reference |
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||||||||
…m:ls1intum/Artemis into chore/convert-athena-endpoints-to-filemap
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java (1)
129-133: Consider simplifying the error message construction.The error message construction is verbose and repeats
repositoryType.name().toLowerCase()multiple times. Consider extracting this to a variable for clarity.Apply this diff:
var repoUri = programmingExercise.getRepositoryURI(repositoryType); if (repoUri == null) { - String errorKey = "error.invalid." + repositoryType.name().toLowerCase() + ".repository.url"; - throw new BadRequestAlertException("Repository URI is null for exercise " + exerciseId + " and repository type " + repositoryType + ". This may indicate that the " - + repositoryType.name().toLowerCase() + " repository has not been set up yet.", ENTITY_NAME, errorKey); + String repoTypeLower = repositoryType.name().toLowerCase(); + String errorKey = "error.invalid." + repoTypeLower + ".repository.url"; + throw new BadRequestAlertException("Repository URI is null for exercise " + exerciseId + " and repository type " + repositoryType + ". This may indicate that the " + + repoTypeLower + " repository has not been set up yet.", ENTITY_NAME, errorKey); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaRepositoryExportServiceTest.java(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java
- src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaRepositoryExportServiceTest.java
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
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
Files:
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java
🧠 Learnings (1)
📚 Learning: 2025-09-15T11:21:15.983Z
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11378
File: src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.java:72-79
Timestamp: 2025-09-15T11:21:15.983Z
Learning: SamuelRoettgermann prefers iterative development approach - initial implementations should be working starting points rather than full-fledged, finished implementations. Optimization and robustness improvements can be added later.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: server-style
- GitHub Check: bean-instantiation-check
- GitHub Check: Analyse
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java (1)
87-107: Critical: Missing feature gate check and exercise-submission validation.Based on past review feedback marked as addressed, this method should:
- Call
checkFeedbackSuggestionsOrAutomaticFeedbackEnabledElseThrow(programmingExercise)after loading the exercise to enforce the feature gate consistently with the instructor flow (line 130).- Validate that the submission's participation belongs to the specified
exerciseIdto prevent data leakage.Both validations are currently absent.
Apply this diff to add the missing validations:
public Map<String, String> getStudentRepositoryFilesContent(long exerciseId, Long submissionId) throws IOException { log.debug("Retrieving student repository file contents for exercise {}, submission {}", exerciseId, submissionId); ProgrammingExercise programmingExercise = programmingExerciseRepository.findByIdElseThrow(exerciseId); + checkFeedbackSuggestionsOrAutomaticFeedbackEnabledElseThrow(programmingExercise); ProgrammingSubmission submission = programmingSubmissionRepository.findByIdElseThrow(submissionId); ProgrammingExerciseStudentParticipation participation = programmingExerciseStudentParticipationRepository.findByIdElseThrow(submission.getParticipation().getId()); + + // Validate that the submission belongs to the specified exercise + if (participation.getProgrammingExercise() == null || !participation.getProgrammingExercise().getId().equals(exerciseId)) { + throw new BadRequestAlertException("Submission does not belong to the specified exercise", ENTITY_NAME, "error.submission.exercise.mismatch"); + } + LocalVCRepositoryUri repoUri = participation.getVcsRepositoryUri(); if (repoUri == null) { throw new BadRequestAlertException( "Repository URI is null for student participation " + participation.getId() + ". This may indicate that the student repository has not been set up yet.", ENTITY_NAME, "error.invalid.student.repository.url"); } ZonedDateTime deadline = programmingExercise.getDueDate(); if (deadline != null) { return repositoryService.getFilesContentFromBareRepositoryForLastCommitBeforeOrAt(repoUri, deadline); } else { return repositoryService.getFilesContentFromBareRepositoryForLastCommit(repoUri); } }
🧹 Nitpick comments (1)
src/main/webapp/i18n/de/error.json (1)
72-77: Use gender-inclusive wording for student message
Elsewhere in this file we rely on gender-inclusive phrasing (e.g., “Studierende:r”). Please update the new student-facing string accordingly—e.g. “Die Repository-URL der Studierenden …”—to stay consistent with existing locale style.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java(3 hunks)src/main/webapp/i18n/de/error.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/webapp/i18n/de/**/*.json
⚙️ CodeRabbit configuration file
German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
Files:
src/main/webapp/i18n/de/error.json
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
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
Files:
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java
🧠 Learnings (1)
📚 Learning: 2025-09-15T11:21:15.983Z
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11378
File: src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRoomDistributionService.java:72-79
Timestamp: 2025-09-15T11:21:15.983Z
Learning: SamuelRoettgermann prefers iterative development approach - initial implementations should be working starting points rather than full-fledged, finished implementations. Optimization and robustness improvements can be added later.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java
🧬 Code graph analysis (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCRepositoryUri.java (1)
LocalVCRepositoryUri(17-318)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: server-style
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: client-style
- GitHub Check: Analyse
- GitHub Check: bean-instantiation-check
🔇 Additional comments (5)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java (5)
1-28: LGTM! Clean imports following coding guidelines.All imports are specific (no star imports), properly organized, and relevant to the refactored functionality.
30-54: LGTM! Well-structured class with proper encapsulation.The class follows coding guidelines with proper annotations, immutable final fields, and a defensive constant for valid repository types.
55-61: LGTM! Proper constructor injection.The constructor follows dependency injection best practices as per coding guidelines.
63-74: LGTM! Helper method follows best practices.The method properly encapsulates the feature gate check with appropriate logging.
109-139: LGTM! Instructor repository method is well-implemented.The method properly validates repository types, enforces the feature gate, handles null URIs with appropriate exceptions, and follows all coding guidelines. The defensive validation against invalid repository types (including USER) is excellent.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||||||||
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
Co-authored-by: Tobias Lippert <[email protected]>
ekayandan
dismissed stale reviews from HawKhiem, SamuelRoettgermann, and coderabbitai[bot]
via
99efbf8
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
Checklist
General
Server
Changes affecting Programming Exercises
Motivation and Context
Athena and Artemis currently use zip files to transfer code repositories between them. On both sides, this introduces unnecessary overhead from repeatedly creating and extracting zip archives. The temporary files not only consume additional storage but also require periodic cleanup. Moreover, this process is more prone to I/O errors.
To address these issues, I am updating related programming exercise/submission functions to work with file maps of the repositories instead of zip files.
Description
Steps for Testing
Important! In order to test this PR, an Athena deployment to athena-test1 is required. Please check 'Testserver States' section below.
Prerequisites:
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
**!**We need the branch 'chore/programming-llm-improve-repository-imports' to be deployed in order to test this PR. Label below shows currently deployed branch on athena test server. If you see another branch name, please let me know so I can deploy it.
Review Progress
Performance Review
Code Review
Manual Tests
Performance Tests
Test Coverage
### Screenshots
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests
Documentation