Programming exercises: Exchange of Artemis programming exercises via CodeAbility Sharing Platform
#10989
Conversation
github-actions
bot
added
tests
server
End-to-End (E2E) Test Results Summary |
WalkthroughAdds a Sharing Platform integration behind the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant SharingPlatform as Sharing Platform
participant ArtemisSupport as SharingSupportResource
participant Connector as SharingConnectorService
Note over SharingPlatform,ArtemisSupport: Connector configuration handshake
SharingPlatform->>ArtemisSupport: GET /api/core/sharing/config?apiBaseUrl=...&installationName=... (Authorization: Bearer {key})
ArtemisSupport->>Connector: validateApiKey(key)
alt valid
ArtemisSupport->>Connector: getPluginConfig(apiBaseUrl, installationName)
Connector-->>ArtemisSupport: SharingPluginConfig
ArtemisSupport-->>SharingPlatform: 200 SharingPluginConfig
else invalid
ArtemisSupport-->>SharingPlatform: 401
end
sequenceDiagram
autonumber
participant Browser as Instructor Browser
participant ArtemisUI as Artemis Web UI
participant API as ExerciseSharingResource
participant Service as ExerciseSharingService
participant SharingPlatform as Sharing Platform
Note over Browser,SharingPlatform: Export flow
Browser->>ArtemisUI: Click "Export to Sharing"
ArtemisUI->>API: POST /api/programming/sharing/export/{exerciseId}?callBack=...
API->>Service: exportExerciseToSharing(exerciseId)
Service->>Service: create ZIP, token, HMAC, build exerciseUrl
Service-->>API: redirectUrl
API-->>ArtemisUI: 200 {redirectUrl}
ArtemisUI-->>Browser: open redirectUrl
Browser->>SharingPlatform: GET redirectUrl (on Sharing)
SharingPlatform->>API: GET /api/programming/sharing/export/{token}?sec=HMAC
alt valid token
API->>Service: getExportedExerciseByToken(token)
Service-->>API: ZIP stream
API-->>SharingPlatform: 200 application/zip
else invalid
API-->>SharingPlatform: 401
end
sequenceDiagram
autonumber
participant SharingPlatform as Sharing Platform
participant Browser as Instructor Browser
participant ArtemisUI as Artemis SharingComponent
participant API as ExerciseSharingResource
participant ImportSvc as ProgrammingExerciseImportFromFileService
Note over SharingPlatform,Browser: Import flow
SharingPlatform-->>Browser: Redirect to Artemis /sharing/import/{basketToken}?returnURL=...&apiBaseURL=...&checksum=...
Browser->>ArtemisUI: load /sharing/import/:basketToken
ArtemisUI->>API: GET /api/programming/sharing/import/basket?basketToken=...&returnURL=...&apiBaseURL=...&checksum=...
API->>API: validate checksum and API key presence
API-->>ArtemisUI: 200 ShoppingBasket
Browser->>ArtemisUI: select course & exercise, POST details
ArtemisUI->>API: POST /api/programming/sharing/import/basket/exercise-details (SharingInfoDTO)
API-->>ArtemisUI: 200 ProgrammingExercise (prefilled)
ArtemisUI->>API: POST /api/programming/sharing/setup-import (SharingSetupInfo)
API->>ImportSvc: importProgrammingExerciseFromFile(..., isImportFromSharing=true)
ImportSvc-->>API: ProgrammingExercise (created)
API-->>ArtemisUI: 200 ProgrammingExercise
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 44
🧹 Nitpick comments (63)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSetupInfo.java (3)
9-11: Improve JavaDoc for better clarity.The JavaDoc comment could be more descriptive and follow standard conventions.
Consider this improvement:
-/** - * the sharing info, wrapping the original sharing Info from the sharing platform and adding course and exercise info. - */ +/** + * Data wrapper that combines sharing information from the sharing platform with additional context. + * + * @param exercise the programming exercise to be shared or imported + * @param course the course context for the exercise + * @param sharingInfo the original sharing information from the sharing platform + */
9-11: Improve JavaDoc formatting and add parameter documentation.The JavaDoc comment should start with a capital letter and include parameter documentation for the record components.
/** - * the sharing info, wrapping the original sharing Info from the sharing platform and adding course and exercise info. + * The sharing info, wrapping the original sharing info from the sharing platform and adding course and exercise info. + * + * @param exercise the programming exercise to be shared + * @param course the course containing the exercise + * @param sharingInfo the original sharing information from the sharing platform */
13-13: Consider whether entities should be included in this data wrapper.The record contains
ProgrammingExerciseandCourseentities, which goes against the coding guideline "dtos:{java_records,no_entities,min_data,single_resp}". Since this appears to be a service-layer data holder rather than a REST DTO, consider whether this design is intentional or if separate DTOs should be used instead.src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingException.java (5)
16-31: Improve JavaDoc comments for accuracy.The JavaDoc comments are misleading - they all mention "connection failures" but the constructors are generic.
Consider this improvement:
- /** - * Creates a SharingException for connection failures. - */ + /** + * Creates a SharingException with the specified message. + * + * @param message the detail message + */ public SharingException(String message) { super(message); } - /** - * Creates a SharingException for connection failures. - * - * @param cause The underlying cause - */ + /** + * Creates a SharingException with the specified message and cause. + * + * @param message the detail message + * @param cause the underlying cause + */ public SharingException(String message, Throwable cause) { super(message, cause); }
16-31: Fix misleading and repetitive JavaDoc comments.The JavaDoc comments are repetitive and misleading. The second constructor is not specifically for "connection failures" as stated.
/** - * Creates a SharingException for connection failures. + * Creates a SharingException with a message. + * + * @param message the detail message */ public SharingException(String message) { super(message); } /** - * Creates a SharingException for connection failures. + * Creates a SharingException with a message and cause. * + * @param message the detail message * @param cause The underlying cause */
39-41: Consider a more generic factory method name.The
connectionErrormethod is specific to connection errors, but the class might be used for other sharing-related errors as well.Consider renaming for better reusability:
-public static SharingException connectionError(String endpoint, Throwable cause) { +public static SharingException withEndpoint(String endpoint, Throwable cause) { return new SharingException("Failed to connect to sharing platform at " + endpoint, cause); }This makes the method more generic while maintaining its current functionality.
16-21: Improve JavaDoc documentation for constructor.The JavaDoc comment says "Creates a SharingException for connection failures" but this constructor is generic and can be used for any sharing-related error, not just connection failures.
/** - * Creates a SharingException for connection failures. + * Creates a SharingException with the specified message. + * + * @param message the detail message */
23-30: Improve JavaDoc documentation for constructor with cause.Similar to the previous constructor, the JavaDoc is too specific about "connection failures" when this constructor can be used for any sharing exception.
/** - * Creates a SharingException for connection failures. + * Creates a SharingException with the specified message and cause. * + * @param message the detail message * @param cause The underlying cause */src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingHealthIndicator.java (5)
14-16: Improve JavaDoc documentation format.The JavaDoc comment should follow proper formatting conventions with a capital letter and proper punctuation.
-/** - * health indicator that shows the status of the sharing platform connector. - */ +/** + * Health indicator that shows the status of the sharing platform connector. + */
27-30: Remove unnecessary super() call.The explicit
super()call is redundant in Java constructors when calling the no-argument constructor of the parent class.public SharingHealthIndicator(SharingConnectorService sharingConnectorService) { - super(); this.sharingConnectorService = sharingConnectorService; }
32-35: Improve JavaDoc documentation format and grammar.The JavaDoc has formatting and grammatical issues that should be corrected.
-/** - * returns the main health status (up/down or unknown if config request from sharing platform is to long ago), together - * with a list of the 10 last log events for the sharing connector. - */ +/** + * Returns the main health status (up/down or unknown if config request from sharing platform is too long ago), together + * with a list of the 10 most recent log events for the sharing connector. + */
43-43: Consider making the timeout configurable.The 11-minute threshold is hardcoded, which makes it difficult to adjust for different environments or requirements without code changes.
Consider extracting this to a configurable property:
+@Value("${artemis.sharing.health-timeout-minutes:11}") +private int healthTimeoutMinutes; -else if (lastHealthStati.getLastConnect().isBefore(Instant.now().minus(11, ChronoUnit.MINUTES))) { +else if (lastHealthStati.getLastConnect().isBefore(Instant.now().minus(healthTimeoutMinutes, ChronoUnit.MINUTES))) {
50-55: Optimize iteration and formatting.The current approach with reverse iteration and string formatting could be simplified and made more efficient.
-for (int i = lastHealthStati.size() - 1; i >= 0; i--) { - SharingConnectorService.HealthStatus hs = lastHealthStati.get(i); +int counter = lastHealthStati.size(); +for (int i = lastHealthStati.size() - 1; i >= 0; i--) { + SharingConnectorService.HealthStatus hs = lastHealthStati.get(i); ZonedDateTime zonedTimestamp = hs.getTimeStamp().atZone(UTC); String timeStamp = TIME_STAMP_FORMATTER.format(zonedTimestamp); - health.withDetail(String.format("%3d: %s", i + 1, timeStamp), hs.getStatusMessage()); + health.withDetail(String.format("%3d: %s", counter--, timeStamp), hs.getStatusMessage()); }src/main/webapp/app/sharing/sharing.model.ts (3)
13-13: Fix typo in comment.There's a spelling error in the comment.
- /** checksum fo apiBaseURL and returnURL */ + /** checksum for apiBaseURL and returnURL */
26-31: Consider including checksum in clear method.The
clear()method resets most properties but doesn't reset thechecksumproperty, which might lead to inconsistent state.public clear(): void { this.basketToken = ''; this.selectedExercise = 0; this.returnURL = ''; this.apiBaseURL = ''; + this.checksum = ''; }
50-54: Consider using readonly for interface properties.The
ShoppingBasketinterface properties could be marked as readonly to prevent accidental mutations, especially for data coming from an external API.export interface ShoppingBasket { - exerciseInfo: Array<SearchResultDTO>; - userInfo: UserInfo; - tokenValidUntil: Date; + readonly exerciseInfo: readonly SearchResultDTO[]; + readonly userInfo: UserInfo; + readonly tokenValidUntil: Date; }src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorHealthCheckRegistryTest.java (3)
29-36: Improve test method name and add setup verification.The test method name could be more descriptive, and the test should verify the health indicator setup before asserting the result.
@Test -void healthUp() throws Exception { +void shouldReportHealthUpWhenSharingPlatformIsConnected() throws Exception { + // given sharingPlatformMockProvider.mockStatus(true); + // when final Health health = sharingHealthIndicator.health(); + + // then assertThat(health.getStatus()).isEqualTo(Status.UP); }
38-49: Improve test method name and add intermediate assertions.The test method name should be more descriptive, and the test could benefit from intermediate assertions to make the test more robust.
@Test -void testMaxStatusInfo() throws Exception { +void shouldLimitHealthHistorySizeToConfiguredMaximum() throws Exception { + // given sharingPlatformMockProvider.mockStatus(true); SharingConnectorService.HealthStatusWithHistory lastHealthStati = sharingConnectorService.getLastHealthStati(); + int initialSize = lastHealthStati.size(); + // when - adding more entries than the limit for (int i = 0; i < SharingConnectorService.HEALTH_HISTORY_LIMIT * 2; i++) { lastHealthStati.add(new SharingConnectorService.HealthStatus("Just Testing")); assertThat(lastHealthStati).size().isLessThanOrEqualTo(SharingConnectorService.HEALTH_HISTORY_LIMIT); } + + // then assertThat(lastHealthStati).size().isEqualTo(SharingConnectorService.HEALTH_HISTORY_LIMIT); }
51-57: Improve test method name and structure.The test method name should be more descriptive and follow the established testing pattern.
@Test -void healthDown() throws Exception { +void shouldReportHealthDownWhenSharingPlatformIsDisconnected() throws Exception { + // given sharingPlatformMockProvider.mockStatus(false); + // when final Health health = sharingHealthIndicator.health(); + + // then assertThat(health.getStatus()).isEqualTo(Status.DOWN); }src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts (1)
59-59: Remove commented code.The commented line should be removed to maintain code cleanliness.
- // const sharingWindow = window.open(redirectURL, 'sharing');src/main/webapp/app/sharing/sharing.component.ts (1)
154-155: Remove redundant return statement.The return statement in the navigation promise is unnecessary since the method doesn't return anything.
.then((nav) => { - return true; // true if navigation is successful + // Navigation completed });src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSupportResourceTest.java (2)
99-104: Improve test method naming to follow BDD conventions.Test method names should clearly describe the scenario being tested using a more descriptive pattern.
Consider renaming tests to be more descriptive:
- public void connectRequestFromSharingPlatform() throws Exception { + public void shouldReturnConfigurationWhenValidApiKeyProvided() throws Exception { - public void connectRequestFromSharingPlatformWithWrongApiKey() throws Exception { + public void shouldReturnUnauthorizedWhenInvalidApiKeyProvided() throws Exception { - public void connectRequestFromSharingPlatformWithMissingApiKey() throws Exception { + public void shouldReturnUnauthorizedWhenApiKeyMissing() throws Exception {Also applies to: 111-115, 122-127, 134-138, 145-149
103-103: Remove unused variable or add assertions.The
contentvariable is extracted but never used, suggesting missing assertions or dead code.Either add assertions on the content:
String content = result.getResponse().getContentAsString(); + assertThat(content).isNotEmpty(); + // Add specific content assertionsOr remove the unused variable:
- String content = result.getResponse().getContentAsString();src/main/webapp/app/sharing/sharing.component.html (2)
48-48: Move complex template expression to component method.The expression
getTokenExpiryDate().toLocaleString()should be moved to a component method for better testability and performance.Create a getter method in the component:
get formattedExpiryDate(): string { return this.getTokenExpiryDate().toLocaleString(); }Then update the template:
-<span jhiTranslate="artemisApp.sharing.expiresAt" [translateValues]="{ expirationDate: getTokenExpiryDate().toLocaleString() }">expires at: </span> +<span jhiTranslate="artemisApp.sharing.expiresAt" [translateValues]="{ expirationDate: formattedExpiryDate }">expires at: </span>
3-3: Remove inline styles in favor of CSS classes.Inline styles should be avoided in favor of CSS classes for better maintainability and consistency with theming guidelines.
Move inline styles to the component's CSS file:
.sharing-table { border-spacing: 50px 0px; border-collapse: separate; } .vertical-align-top { vertical-align: top; } .course-color-indicator { width: 15px; height: 20px; }Then update the template:
-<table style="border-spacing: 50px 0px; border-collapse: separate"> +<table class="sharing-table">Also applies to: 83-83
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java (3)
124-124: Remove unused variable declaration.The variable
oldShortNameis declared but never used, which violates the coding guidelines about avoiding unused variables.-var oldShortName = getProgrammingExerciseFromDetailsFile(importExerciseDir).getShortName();If this variable is needed for future functionality, add a TODO comment explaining its purpose.
97-98: Update method documentation to reflect the new parameter.The Javadoc should be updated to properly document the new
isImportFromSharingparameter./** * Imports a programming exercise from an uploaded zip file that has previously been downloaded from an Artemis instance. * It first extracts the contents of the zip file, then creates a programming exercise (same process as creating a new one), * then deletes the template content initially pushed to the repositories and copies over the extracted content * * @param originalProgrammingExercise the programming exercise that should be imported * @param zipFile the zip file that contains the exercise * @param course the course to which the exercise should be added * @param user the user initiating the import - * @param isImportFromSharing flag whether file import (false) of sharing import + * @param isImportFromSharing flag indicating whether this is a sharing platform import (true) or regular file import (false) * @return the imported programming exercise **/
175-181: Improve Javadoc for the overloaded method.The overloaded method should have more complete documentation explaining its purpose and relationship to the main method.
/** - * Overloaded method setting the isImportFromSharing flag to false as default + * Imports a programming exercise from an uploaded zip file with default settings. + * This is a convenience method that calls the main import method with isImportFromSharing set to false. + * + * @param programmingExerciseForImport the programming exercise that should be imported + * @param zipFile the zip file that contains the exercise + * @param course the course to which the exercise should be added + * @param user the user initiating the import + * @return the imported programming exercise + * @throws IOException if there is an error reading the files + * @throws GitAPIException if there is an error with Git operations + * @throws URISyntaxException if there is an error with URI parsing */src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingPlatformMockProvider.java (3)
20-22: Complete the class-level Javadoc documentation.The class documentation is incomplete and doesn't properly describe the class's purpose and functionality.
/** - * provides infrastructure to set up and shutdown + * Test utility class that provides infrastructure to mock the sharing platform + * for testing purposes. This mock provider simulates connections between Artemis + * and the sharing platform, allowing for isolated testing of sharing functionality. */
76-79: Complete the method documentation.The Javadoc comment for the
mockStatusmethod is incomplete and unclear./** - * registers or shuts down the required + * Mocks the sharing platform connection status. + * When success is true, establishes a connection to simulate a successful platform state. + * When success is false, resets the connection to simulate platform unavailability. * - * @param success Successful response or timeout. + * @param success true to simulate successful connection, false to simulate failure/timeout + * @throws Exception if the mock operation fails */
27-31: Consider making constants more configurable.The hardcoded URLs and test installation name could be made configurable through properties to improve test flexibility.
-protected static final String TEST_INSTALLATION_NAME = "ArtemisTestInstance"; +@Value("${artemis.sharing.test.installation-name:ArtemisTestInstance}") +private String testInstallationName; -public static final String SHARING_BASEURL = "http://localhost:9001/api"; +@Value("${artemis.sharing.test.base-url:http://localhost:9001/api}") +private String sharingBaseUrl; -public static final String SHARING_BASEURL_PLUGIN = SHARING_BASEURL + "/pluginIF/v0.1"; +private String getSharingPluginUrl() { + return sharingBaseUrl + "/pluginIF/v0.1"; +}src/main/webapp/app/sharing/sharing.scss (2)
16-17: Define magic numbers as CSS custom properties.The stylesheet contains many magic numbers that should be defined as CSS variables for better maintainability.
Add CSS custom properties at the top of the file:
:host { --curve-height: 100px; --shadow-offset: 1px; --shadow-blur: 40px; --font-size-large: 5em; --font-size-medium: 3em; --font-size-small: 2em; --spacing-small: 20px; --spacing-medium: 35px; --border-radius: 10px; --box-height: 250px; --icon-size-large: 80px; --icon-size-small: 40px; }Then replace magic numbers:
.curve { fill: #fff; - height: 100px; + height: var(--curve-height); width: 100%; position: absolute; bottom: 0; left: 0; }Also applies to: 28-28, 66-66
210-214: Consolidate repetitive hover animations.The hover animations for feature descriptions are duplicated and could be consolidated using a mixin or shared class.
Create a SCSS mixin for the slide-up animation:
@mixin slide-up-animation($top-position: 100px, $opacity: 1) { transition: 0.5s; &:hover { top: $top-position; opacity: $opacity; text-align: center; } } .container .box .feature-short-description { position: absolute; top: 100%; height: calc(100% - 120px); width: calc(100% - 40px); box-sizing: border-box; font-size: 18px; opacity: 0; @include slide-up-animation(100px, 1); }src/main/webapp/app/sharing/sharing.component.spec.ts (1)
24-203: Use single quotes consistently throughout the file.The TypeScript style guide requires single quotes for strings, but the file uses a mix of single and double quotes.
Examples of lines that need to be updated:
- Line 72:
{ id: 1, title: 'testCouse 1' },→{ id: 1, title: 'testCouse 1' },- Line 73:
{ id: 2, title: 'testCouse 2' },→{ id: 2, title: 'testCouse 2' },- Line 88:
courseReq.flush(courses);uses double quotes in the expectOne- Line 161:
{ message: 'Not Found' }→{ message: 'Not Found' }Apply this pattern throughout the file for consistency.
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java (3)
139-262: Use consistent and descriptive test method naming.Test methods have inconsistent naming - some start with "test" prefix while others don't. Follow a consistent naming pattern that describes what is being tested and the expected outcome.
Examples of improved naming:
- void isSharingPlatformUp() + void shouldReturnTrueWhenSharingPlatformIsEnabled() - void testImportBasket() + void shouldSuccessfullyImportBasketFromSharingPlatform() - void testImportBasketFail() + void shouldReturnNotFoundWhenBasketImportFails() - void importBasketNotFound() + void shouldReturnNotFoundWhenBasketDoesNotExist() - void importBasketWrongChecksum() + void shouldReturnBadRequestWhenChecksumIsInvalid()
141-178: Remove commented out debug statements.Remove the commented
.andDo(print())statements throughout the test file. These appear to be temporary debug statements that should not be committed.Lines to clean up:
- Line 141:
// .andDo(print())- Line 177:
/* .andDo(print()) */- Line 195:
/* .andDo(print()) */- Line 309:
/* .andDo(print()) */- Line 350:
.andDo(print())(this one is not commented and should either be removed or justified)Also applies to: 192-196, 309-310, 349-350, 402-402
359-390: Consider using DTOs instead of modifying entities for serialization.The
makeCourseJSONSerializablemethod is a workaround that modifies entity state to make it serializable. This violates the principle of using DTOs for data transfer and indicates potential architectural issues with directly serializing entities.Consider:
- Creating proper DTOs for data transfer instead of using entities directly
- Using
@JsonIgnoreor@JsonViewannotations to control serialization- Configuring Jackson to handle circular references properly
This would eliminate the need for this workaround method and improve the separation of concerns between persistence and presentation layers.
src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java (3)
50-50: Remove unnecessary @SuppressWarnings annotation.The
@SuppressWarnings("unused")annotation on the constructor is unnecessary since this is a standard dependency injection constructor that will be used by Spring.- @SuppressWarnings("unused") public SharingSupportResource(SharingConnectorService sharingConnectorService) {
73-73: Use a more descriptive variable name.The variable name
apiBaseUrl1is not descriptive and suggests this is a quick fix rather than intentional naming.- URL apiBaseUrl1; + URL parsedApiBaseUrl; try { - apiBaseUrl1 = URI.create(apiBaseUrl).toURL(); + parsedApiBaseUrl = URI.create(apiBaseUrl).toURL(); } catch (IllegalArgumentException | MalformedURLException e) { log.error("Bad URL", e); return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(null); } - return ResponseEntity.ok(sharingConnectorService.getPluginConfig(apiBaseUrl1, installationName)); + return ResponseEntity.ok(sharingConnectorService.getPluginConfig(parsedApiBaseUrl, installationName));
69-81: Consider extracting URL validation logic to improve readability.The URL parsing and validation logic could be extracted to a private method to improve the main method's readability and testability.
+ private Optional<URL> parseAndValidateUrl(String apiBaseUrl) { + try { + return Optional.of(URI.create(apiBaseUrl).toURL()); + } catch (IllegalArgumentException | MalformedURLException e) { + log.error("Bad URL: {}", apiBaseUrl, e); + return Optional.empty(); + } + } @GetMapping(SHARINGCONFIG_RESOURCE_PATH) public ResponseEntity<SharingPluginConfig> getConfig(@RequestHeader("Authorization") Optional<String> sharingApiKey, @RequestParam String apiBaseUrl, @RequestParam Optional<String> installationName) { if (sharingApiKey.isPresent() && sharingConnectorService.validate(sharingApiKey.get())) { log.info("Delivered Sharing Config "); - URL apiBaseUrl1; - try { - apiBaseUrl1 = URI.create(apiBaseUrl).toURL(); - } - catch (IllegalArgumentException | MalformedURLException e) { - log.error("Bad URL", e); - return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(null); - } - return ResponseEntity.ok(sharingConnectorService.getPluginConfig(apiBaseUrl1, installationName)); + Optional<URL> parsedUrl = parseAndValidateUrl(apiBaseUrl); + if (parsedUrl.isEmpty()) { + return ResponseEntity.badRequest().build(); + } + return ResponseEntity.ok(sharingConnectorService.getPluginConfig(parsedUrl.get(), installationName)); }src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingInfoDTOTest.java (4)
16-16: Use more descriptive test method name.The method name
someEqualsTestsis not descriptive enough. Test method names should clearly indicate what behavior is being tested.- void someEqualsTests() { + void testSharingInfoDTOEquals_shouldRespectAllFieldsExceptChecksum() {
20-22: Use more specific assertions for equals() contract.The equals() method test should be more comprehensive and use more specific assertions to clearly document the expected behavior.
- assertThat(si.equals(si2)).isTrue(); - assertThat(si.equals(si)).isTrue(); - assertThat(si.equals(null)).isFalse(); + assertThat(si).isEqualTo(si2); + assertThat(si).isEqualTo(si); + assertThat(si).isNotEqualTo(null); + assertThat(si).isNotEqualTo("different type");
70-72: Split long lines for better readability.The test lines are too long and should be split for better readability according to coding standards.
assertThatExceptionOfType(IllegalArgumentException.class) - .isThrownBy(() -> new SharingMultipartZipFile(null, this.getClass().getResource("./basket/sampleExercise.zip").openStream())); - assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> new SharingMultipartZipFile("no stream", null)); + .isThrownBy(() -> new SharingMultipartZipFile(null, + this.getClass().getResource("./basket/sampleExercise.zip").openStream())); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> new SharingMultipartZipFile("no stream", null));
15-50: Consider using parameterized tests for equals() method testing.The equals() method testing could be more maintainable and comprehensive using JUnit 5's parameterized tests.
This would improve test maintainability and make it easier to add new test cases:
@ParameterizedTest @MethodSource("provideEqualsTestCases") void testSharingInfoDTOEquals(SharingInfoDTO dto1, SharingInfoDTO dto2, boolean expected) { if (expected) { assertThat(dto1).isEqualTo(dto2); } else { assertThat(dto1).isNotEqualTo(dto2); } } private static Stream<Arguments> provideEqualsTestCases() { SharingInfoDTO base = new SharingInfoDTO(); SharingInfoDTO withPosition = new SharingInfoDTO(); withPosition.setExercisePosition(2); // ... more test cases return Stream.of( Arguments.of(base, new SharingInfoDTO(), true), Arguments.of(base, withPosition, false), // ... more test cases ); }src/main/webapp/app/programming/manage/grading/charts/programming-exercise-detail.component.with-sharing.spec.ts (1)
34-36: Consider consolidating test files if this becomes stable.The comment suggests this could be merged into the main test file once stabilized. This approach helps avoid test fragmentation.
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingMultipartZipFile.java (1)
49-67: Note limitation with available() method usage.The
isEmpty()andgetSize()methods rely onInputStream.available(), which only returns immediately available bytes and may not accurately reflect the total stream size or emptiness. However, based on previous feedback indicating this is a dummy implementation, this limitation may be acceptable for the current use case.src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java (1)
65-86: Consider more specific exception handling.The method has good input validation and delegation patterns, but the catch-all exception block (lines 82-85) that just logs and re-throws may not add significant value. Consider handling specific exceptions or removing the try-catch if it's only for logging.
- try { - return this.programmingExerciseImportFromFileService.importProgrammingExerciseFromFile(sharingSetupInfo.exercise(), zipFileO.get(), sharingSetupInfo.course(), user, - true); - } - catch (Exception e) { - log.error("Cannot create exercise", e); - throw e; - } + return this.programmingExerciseImportFromFileService.importProgrammingExerciseFromFile(sharingSetupInfo.exercise(), zipFileO.get(), sharingSetupInfo.course(), user, + true);src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceExportTest.java (1)
92-92: Consider moving ObjectMapper to setup method.The ObjectMapper field could be initialized in a @beforeeach method for better organization, or made static if it's immutable and reusable across test instances.
- private final com.fasterxml.jackson.databind.ObjectMapper objectMapper = new ObjectMapper().registerModule(new com.fasterxml.jackson.datatype.jsr310.JavaTimeModule()); + private ObjectMapper objectMapper; + + @BeforeEach + void setupObjectMapper() { + objectMapper = new ObjectMapper().registerModule(new JavaTimeModule()); + }src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java (4)
147-150: Improve error message for generic REST exceptions.The error message "Unrecognized property when importing exercise from Sharing" is misleading as it implies a deserialization issue, but this catch block handles all
RestClientExceptiontypes.- catch (RestClientException rpe) { - log.warn("Unrecognized property when importing exercise from Sharing", rpe); + catch (RestClientException rpe) { + log.warn("Failed to retrieve basket from sharing platform", rpe); return Optional.empty(); }
227-239: Consider a more robust solution for Docker URL handling.The current implementation is described as "just a weak implementation for local testing". For production use, consider using environment-specific configuration instead of string replacement.
Consider using Spring profiles or environment variables to configure the correct host URL for different environments (local, Docker, production) rather than runtime string replacement.
278-281: Remove redundant empty check.The empty check is immediately followed by throwing an exception, making the if statement unnecessary.
Optional<String> entryFromBasket = this.getEntryFromBasket(pattern, sharingInfo); - if (entryFromBasket.isEmpty()) - throw new NotFoundException("Could not retrieve exercise details from imported exercise"); - - String exerciseDetailString = entryFromBasket.get(); + String exerciseDetailString = entryFromBasket.orElseThrow( + () -> new NotFoundException("Could not retrieve exercise details from imported exercise") + );
322-322: Extract buffer size as a constant.The hardcoded buffer size should be defined as a class constant for better maintainability.
Add a constant at the class level:
private static final int BUFFER_SIZE = 102400;Then use it:
- byte[] buffer = new byte[102400]; + byte[] buffer = new byte[BUFFER_SIZE];src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java (4)
109-117: Fix duplicate field documentation.Both fields have the same comment "the shared secret api key", which is confusing.
/** - * the shared secret api key + * the action name for the sharing platform */ @Value("${artemis.sharing.actionname:Export to Artemis@somewhere}") private String actionName;
199-204: Improve installation name resolution for containerized environments.
InetAddress.getLocalHost()may return unhelpful values like "localhost" or container IDs in containerized environments.try { - this.installationName = installationName.orElse(InetAddress.getLocalHost().getCanonicalHostName()); + this.installationName = installationName + .or(() -> Optional.ofNullable(System.getenv("HOSTNAME"))) + .orElse(InetAddress.getLocalHost().getCanonicalHostName()); } catch (UnknownHostException e) { + log.warn("Failed to determine hostname", e); this.installationName = UNKNOWN_INSTALLATIONAME; }
219-224: Document or make configurable the API key length limit.The hardcoded length limit of 200 is arbitrary and undocumented.
+ private static final int MAX_API_KEY_LENGTH = 200; + public boolean validate(String apiKey) { - if (apiKey == null || apiKey.length() > 200) { + if (apiKey == null || apiKey.length() > MAX_API_KEY_LENGTH) { // this is just in case, somebody tries an attack lastHealthStati.add(new HealthStatus("Failed api Key validation"));
275-278: Log the full exception for better debugging.Only logging the exception message loses valuable stack trace information.
catch (Exception e) { - log.info("Failed to request reinitialization from Sharing Platform: {}", e.getMessage()); + log.warn("Failed to request reinitialization from Sharing Platform", e); }src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts (3)
82-96: Consider using a facade pattern to reduce component dependencies.The component has 14 service dependencies, which suggests it might be handling too many responsibilities. Consider creating a facade service to encapsulate related operations.
Group related services (e.g., all exercise-related services) behind a facade to simplify the component and improve testability.
132-135: Integrate sharing field visibility logic into the general approach.The special case for
SHORT_NAMEfield whensharingInfois present seems like a workaround that breaks the component's field visibility pattern.Consider adding sharing import as a proper mode in the
IS_DISPLAYED_IN_SIMPLE_MODEmapping or creating a more flexible field visibility system that can handle different import types consistently.
352-353: Consolidate import type checks to reduce duplication.The repeated checks for
isImportFromFile || isImportFromSharingviolate the DRY principle.Add a computed property or getter:
get isImportFromExternal(): boolean { return this.isImportFromFile || this.isImportFromSharing; }Then replace all occurrences of
this.isImportFromFile || this.isImportFromSharingwiththis.isImportFromExternal.Also applies to: 373-374, 520-520
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (1)
53-71: Remove redundant JavaDoc comments for fields.The JavaDoc comments for the fields simply restate the field names without adding value. Consider removing them to reduce noise in the code.
- /** - * a logger - */ private final Logger log = LoggerFactory.getLogger(ExerciseSharingResource.class); - /** - * the exercise sharing service - */ private final ExerciseSharingService exerciseSharingService; - /** - * the sharing connector service - */ private final SharingConnectorService sharingConnectorService; - /** - * the programming-exercise import from Sharing Service - */ private final ProgrammingExerciseImportFromSharingService programmingExerciseImportFromSharingService;src/main/webapp/app/sharing/sharing.route.ts (1)
16-23: Simplify the route structure.The current implementation creates unnecessary complexity with
SHARING_ROUTESconstant and nested structure for a single route. Consider simplifying the export.-const SHARING_ROUTES = [...sharingRoutes]; - export const featureOverviewState: Routes = [ { path: '', - children: SHARING_ROUTES, + children: sharingRoutes, }, ];This reduces the intermediate constant while maintaining the same functionality.
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorServiceTest.java (2)
18-22: Consider using static mocks instead of dependency injection.The test uses
@AutowiredforSharingPlatformMockProviderandSharingConnectorService, but the coding guidelines recommend "mock_strategy: static_mocks". Consider using@MockBeanor static mocking frameworks like Mockito's static mocking capabilities.
57-57: Optimize the huge key generation for better performance.The current approach creates a very large string using
repeat(50)which may be inefficient. Consider using a more concise approach for generating an oversized key.-String hugeKey = "huge" + "0123456789".repeat(50); +String hugeKey = "x".repeat(1000); // More concise way to create a large key
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (16)
.github/workflows/build.ymlis excluded by!**/*.ymldocs/admin/setup/sharing/sharingButtonArtemis.pngis excluded by!**/*.png,!**/*.pngdocs/admin/setup/sharing/sharingButtonSharing.pngis excluded by!**/*.png,!**/*.pngdocs/admin/setup/sharing/sharing_health1.pngis excluded by!**/*.png,!**/*.pngdocs/admin/setup/sharing/sharing_health2.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/artemis_import.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/artemis_import2.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/sharing_export.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/sharing_metadata.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/sharing_metadata2.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/sharing_namespace.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/sharing_search.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/sharing_success.pngis excluded by!**/*.png,!**/*.pngsrc/main/resources/config/application-dev.ymlis excluded by!**/*.ymlsrc/main/resources/config/application-sharing.ymlis excluded by!**/*.ymlsrc/test/resources/de/tum/cit/aet/artemis/programming/service/sharing/basket/sampleExercise.zipis excluded by!**/*.zip,!**/*.zip
📒 Files selected for processing (54)
build.gradle(2 hunks)docker/artemis/config/dev-local-vc-local-ci.env(1 hunks)docs/admin/extension-services.rst(1 hunks)docs/admin/setup/sharing.rst(1 hunks)docs/index.rst(1 hunks)docs/user/sharing.rst(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/RestTemplateConfiguration.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/dto/SharingInfoDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingException.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingHealthIndicator.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingMultipartZipFile.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSetupInfo.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java(1 hunks)src/main/webapp/app/app.routes.ts(1 hunks)src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.html(1 hunks)src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.ts(7 hunks)src/main/webapp/app/programming/manage/exercise/programming-exercise.component.ts(1 hunks)src/main/webapp/app/programming/manage/grading/charts/programming-exercise-detail.component.with-sharing.spec.ts(1 hunks)src/main/webapp/app/programming/manage/programming-exercise-management.route.ts(1 hunks)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts(1 hunks)src/main/webapp/app/programming/manage/update/programming-exercise-creation-config.ts(1 hunks)src/main/webapp/app/programming/manage/update/programming-exercise-update.component.html(3 hunks)src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts(17 hunks)src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts(1 hunks)src/main/webapp/app/sharing/search-result-dto.model.ts(1 hunks)src/main/webapp/app/sharing/sharing.component.html(1 hunks)src/main/webapp/app/sharing/sharing.component.spec.ts(1 hunks)src/main/webapp/app/sharing/sharing.component.ts(1 hunks)src/main/webapp/app/sharing/sharing.model.ts(1 hunks)src/main/webapp/app/sharing/sharing.route.ts(1 hunks)src/main/webapp/app/sharing/sharing.scss(1 hunks)src/main/webapp/i18n/de/programmingExercise.json(1 hunks)src/main/webapp/i18n/de/sharing.json(1 hunks)src/main/webapp/i18n/en/programmingExercise.json(1 hunks)src/main/webapp/i18n/en/sharing.json(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceExportTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingServiceTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorHealthCheckRegistryTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorServiceTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingInfoDTOTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingPlatformMockProvider.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSupportResourceTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationIndependentTest.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java(3 hunks)src/test/resources/de/tum/cit/aet/artemis/programming/service/sharing/basket/sampleBasket.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/programming/manage/exercise/programming-exercise.component.tssrc/main/webapp/app/app.routes.tssrc/main/webapp/app/programming/manage/programming-exercise-management.route.tssrc/main/webapp/app/sharing/sharing.route.tssrc/main/webapp/app/sharing/sharing.model.tssrc/main/webapp/app/sharing/search-result-dto.model.tssrc/main/webapp/app/programming/manage/update/programming-exercise-creation-config.tssrc/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.tssrc/main/webapp/app/sharing/sharing.component.tssrc/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.tssrc/main/webapp/app/sharing/sharing.component.spec.tssrc/main/webapp/app/programming/manage/grading/charts/programming-exercise-detail.component.with-sharing.spec.tssrc/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.tssrc/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts
`src/main/webapp/**/*.html`: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.htmlsrc/main/webapp/app/sharing/sharing.component.htmlsrc/main/webapp/app/programming/manage/update/programming-exercise-update.component.html
`src/main/webapp/i18n/de/**/*.json`: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/...
src/main/webapp/i18n/de/**/*.json: 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".
src/main/webapp/i18n/de/programmingExercise.jsonsrc/main/webapp/i18n/de/sharing.json
`src/main/java/**/*.java`: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,de...
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSetupInfo.javasrc/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.javasrc/main/java/de/tum/cit/aet/artemis/core/config/RestTemplateConfiguration.javasrc/main/java/de/tum/cit/aet/artemis/core/dto/SharingInfoDTO.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingHealthIndicator.javasrc/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingMultipartZipFile.javasrc/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingException.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.javasrc/main/java/de/tum/cit/aet/artemis/core/config/Constants.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
`src/test/java/**/*.java`: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_...
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/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationIndependentTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSupportResourceTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorHealthCheckRegistryTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingPlatformMockProvider.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingInfoDTOTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceExportTest.java
🧠 Learnings (2)
src/main/webapp/app/sharing/search-result-dto.model.ts (3)
Learnt from: Wallenstein61
PR: ls1intum/Artemis#9909
File: src/main/webapp/app/sharing/search-result-dto.model.ts:17-42
Timestamp: 2025-02-11T15:46:08.133Z
Learning: The types in `UserProvidedMetadataDTO` interface in `search-result-dto.model.ts` must match the sharing platform's contract exactly to maintain compatibility. Avoid modifying these types even if they don't follow internal TypeScript conventions.
Learnt from: Wallenstein61
PR: ls1intum/Artemis#9909
File: src/main/webapp/app/sharing/search-result-dto.model.ts:57-65
Timestamp: 2025-02-11T15:40:53.440Z
Learning: The ProjectDTO interface in src/main/webapp/app/sharing/search-result-dto.model.ts uses snake_case property names to maintain compatibility with the external sharing platform's API contract, which is an intentional deviation from Angular's camelCase convention.
Learnt from: Wallenstein61
PR: ls1intum/Artemis#9909
File: src/main/webapp/app/sharing/search-result-dto.model.ts:50-55
Timestamp: 2025-02-11T15:46:35.616Z
Learning: The `IExerciseType` enum in `src/main/webapp/app/sharing/search-result-dto.model.ts` must maintain its current naming (with 'I' prefix) and lowercase string values to ensure compatibility with the external sharing platform connector interface. This is an intentional exception to our TypeScript naming conventions.
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingMultipartZipFile.java (1)
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10593
File: src/main/java/de/tum/cit/aet/artemis/sharing/SharingMultipartZipFile.java:60-68
Timestamp: 2025-04-11T07:12:15.912Z
Learning: The SharingMultipartZipFile class in the sharing package is a dummy implementation that's not actually used in practice, so optimizations aren't necessary.
🧬 Code Graph Analysis (4)
src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java (2)
src/main/webapp/app/core/layouts/profiles/shared/profile.service.ts (1)
isProfileActive(30-32)src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (1)
Constants(8-523)
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.ts (1)
src/main/webapp/app/shared/detail-overview-list/detail-overview-list.component.ts (1)
DetailOverviewSection(26-29)
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (2)
src/main/webapp/app/sharing/sharing.model.ts (2)
Injectable(3-45)ShoppingBasket(50-54)src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-180)
src/main/webapp/app/sharing/sharing.component.spec.ts (4)
src/test/javascript/spec/helpers/mocks/service/mock-alert.service.ts (1)
MockAlertService(3-8)src/test/javascript/spec/helpers/mocks/mock-router.ts (1)
MockRouter(5-30)src/main/webapp/app/sharing/sharing.model.ts (1)
ShoppingBasket(50-54)src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-180)
🪛 GitHub Check: client-tests
src/main/webapp/app/programming/manage/grading/charts/programming-exercise-detail.component.with-sharing.spec.ts
[failure] 20-20:
Cannot find module '../../helpers/mocks/service/mock-sync-storage.service' or its corresponding type declarations.
[failure] 18-18:
Cannot find module '../../helpers/mocks/service/mock-ngb-modal.service' or its corresponding type declarations.
[failure] 14-14:
Cannot find module '../../helpers/mocks/service/mock-programming-exercise.service' or its corresponding type declarations.
[failure] 12-12:
Cannot find module '../../helpers/mocks/service/mock-profile.service' or its corresponding type declarations.
[failure] 11-11:
Cannot find module 'app/shared/layouts/profiles/profile.service' or its corresponding type declarations.
[failure] 9-9:
Cannot find module 'app/shared/statistics-graph/statistics.service' or its corresponding type declarations.
[failure] 7-7:
Cannot find module 'app/entities/course.model' or its corresponding type declarations.
[failure] 6-6:
Cannot find module '../../helpers/mocks/activated-route/mock-activated-route' or its corresponding type declarations.
[failure] 5-5:
Cannot find module 'app/entities/programming/programming-exercise.model' or its corresponding type declarations.
[failure] 4-4:
Cannot find module 'app/programming/manage/programming-exercise-detail.component' or its corresponding type declarations.
🪛 GitHub Check: client-tests-selected
src/main/webapp/app/programming/manage/grading/charts/programming-exercise-detail.component.with-sharing.spec.ts
[failure] 20-20:
Cannot find module '../../helpers/mocks/service/mock-sync-storage.service' or its corresponding type declarations.
[failure] 18-18:
Cannot find module '../../helpers/mocks/service/mock-ngb-modal.service' or its corresponding type declarations.
[failure] 14-14:
Cannot find module '../../helpers/mocks/service/mock-programming-exercise.service' or its corresponding type declarations.
[failure] 12-12:
Cannot find module '../../helpers/mocks/service/mock-profile.service' or its corresponding type declarations.
[failure] 11-11:
Cannot find module 'app/shared/layouts/profiles/profile.service' or its corresponding type declarations.
[failure] 9-9:
Cannot find module 'app/shared/statistics-graph/statistics.service' or its corresponding type declarations.
[failure] 7-7:
Cannot find module 'app/entities/course.model' or its corresponding type declarations.
[failure] 6-6:
Cannot find module '../../helpers/mocks/activated-route/mock-activated-route' or its corresponding type declarations.
[failure] 5-5:
Cannot find module 'app/entities/programming/programming-exercise.model' or its corresponding type declarations.
[failure] 4-4:
Cannot find module 'app/programming/manage/programming-exercise-detail.component' or its corresponding type declarations.
🪛 ast-grep (0.38.1)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
[warning] 404-404: Use of AES with ECB mode detected. ECB doesn't provide message confidentiality and is not semantically secure so should not be used. Instead, use a strong, secure cipher: Cipher.getInstance("AES/CBC/PKCS7PADDING"). See https://owasp.org/www-community/Using_the_Java_Cryptographic_Extensions for more information.
Context: Mac.getInstance(algorithm)
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
- https://googleprojectzero.blogspot.com/2022/10/rc4-is-still-considered-harmful.html
(use-of-aes-ecb-java)
[warning] 404-404: Triple DES (3DES or DESede) is considered deprecated. AES is the recommended cipher. Upgrade to use AES.
Context: Mac.getInstance(algorithm)
Note: [CWE-326]: Inadequate Encryption Strength [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://find-sec-bugs.github.io/bugs.htm#TDES_USAGE
- https://csrc.nist.gov/News/2017/Update-to-Current-Use-and-Deprecation-of-TDEA
(desede-is-deprecated-java)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build .war artifact
- 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: Analyse
- GitHub Check: client-style
- GitHub Check: server-style
- GitHub Check: server-tests
🔇 Additional comments (70)
src/main/webapp/app/programming/manage/exercise/programming-exercise.component.ts (2)
57-57: VerifySharingInfois injectable
Ensure the importedSharingInfoclass fromapp/sharing/sharing.modelis decorated with@Injectable(); otherwise providing it here will cause a runtime injection error.
61-61: Confirm provider scope
AddingSharingInfotoprovidersgives each component instance its own copy. If you need a singleton across the app, prefer usingprovidedIn: 'root'in the service definition instead of per-component providers.docs/index.rst (1)
42-42: Good integration in User Guide toctree
The newuser/sharingentry correctly surfaces the sharing platform guide alongside other user docs.docs/admin/extension-services.rst (1)
36-36: Extension Services toctree updated
Includingsetup/sharinghere properly advertises the sharing extension in the administration docs.docker/artemis/config/dev-local-vc-local-ci.env (1)
5-5: Activate sharing profile in Docker config
AppendingsharingtoSPRING_PROFILES_ACTIVEcorrectly enables the new Spring profile in the local CI/VC Docker environment.src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.html (1)
95-97: Verify conditional rendering
Confirm thatisExportToSharingEnabledis declared and updated in the component class (via theProgrammingExerciseSharingService) so the sharing button toggles as expected.src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationIndependentTest.java (2)
11-11: Add PROFILE_SHARING import for shared tests
The new sharing profile constant is correctly imported alongside existing profiles to enable sharing-related beans in independent tests.
47-48: Activate sharing profile in base test
IncludingPROFILE_SHARINGin@ActiveProfilesensures independent tests load sharing beans and contexts as intended.src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java (1)
242-243: Permit all for sharing endpoints
Explicitly allowing unauthenticated access to/api/sharing/**is acceptable here since controllers enforce security tokens.src/main/webapp/app/programming/manage/update/programming-exercise-creation-config.ts (1)
25-25: AddisImportFromSharingflag
IntroducingisImportFromSharingaligns the creation config with other import flags and supports the sharing workflow.build.gradle (2)
160-162: Add SharingPluginPlatformAPI dependency
The new dependency onorg.codeability:SharingPluginPlatformAPI:1.1.1is necessary for sharing platform integration.
455-457: Apply test.gradle after dependencies block
Moving theapply from: "gradle/test.gradle"ensures themockitoAgentconfiguration is applied without conflicts.src/main/webapp/i18n/de/sharing.json (1)
1-22: LGTM! German localization follows informal language guidelines.The German translations correctly use informal language (dutzen) as required by the coding guidelines:
- "Bitte melden dich als Instruktor an!" uses "dich" (informal)
- Other strings are appropriately informal and professional
The JSON structure is well-organized and follows established i18n patterns.
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java (3)
13-13: LGTM! Proper import of sharing profile constant.The import follows the established pattern for other profile constants and is necessary for the sharing feature integration.
78-78: LGTM! Sharing profile properly added to test configuration.The
PROFILE_SHARINGaddition to active profiles ensures that sharing-related functionality is available during integration tests, which aligns with the comprehensive testing approach mentioned in the PR objectives.
136-140: LGTM! Well-documented DockerClient field for test infrastructure.The protected
DockerClientfield with clear documentation allows subclasses to dynamically mock Docker operations, which supports the testing requirements for the local CI functionality mentioned in the PR.src/main/webapp/i18n/en/programmingExercise.json (1)
745-751: LGTM! Well-structured English localization for sharing functionality.The sharing localization strings are:
- Clearly written and professional
- Properly structured within the artemisIntelligence section
- Include appropriate error handling with dynamic message placeholder
- Consistent with existing i18n patterns in the file
src/main/webapp/app/programming/manage/update/programming-exercise-update.component.html (1)
3-3: LGTM! Consistent integration of sharing import support.The addition of
isImportFromSharingflag is implemented consistently across all relevant conditions in the template. The changes properly extend the existing import logic to support the new sharing platform import source while maintaining the correct conditional display behavior.Also applies to: 7-7, 19-19, 65-65
src/main/webapp/i18n/de/programmingExercise.json (1)
745-752: LGTM! Proper German localization for sharing feature.The new sharing-related translations are well-structured and use appropriate German terminology. The translations don't directly address the user, so the informal/formal language guideline doesn't apply here. The error message template correctly uses interpolation syntax.
src/main/java/de/tum/cit/aet/artemis/core/config/RestTemplateConfiguration.java (2)
8-8: LGTM! Proper import addition.The import for
PROFILE_SHARINGfollows the existing pattern and is correctly placed with other profile imports.
63-74: LGTM! Well-implemented RestTemplate bean.The
sharingRestTemplate()method follows the established pattern in this configuration class and adheres to Java coding guidelines:
- Proper JavaDoc documentation explaining the purpose
- Correct use of
@Beanand@Profileannotations- Code reuse by calling existing
createRestTemplate()method- Consistent with other RestTemplate bean definitions
src/main/webapp/app/programming/manage/programming-exercise-management.route.ts (1)
59-70: LGTM! Consistent route configuration for sharing import.The new route follows the established pattern in this file and adheres to Angular coding guidelines:
- Uses lazy loading with proper component import
- Correctly applies the existing resolver, authorities, and guards
- Consistent structure with other import routes
- Proper page title configuration
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSetupInfo.java (2)
12-14: LGTM! Well-structured record implementation.The record follows Java naming conventions and coding guidelines perfectly. The @Profile annotation ensures proper activation scope, and the record serves as an appropriate DTO combining related data.
12-13: LGTM! Well-designed record for DTO pattern.This record effectively follows the coding guidelines for DTOs using Java records with minimal data and single responsibility. The profile annotation correctly gates the sharing feature.
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorServiceTest.java (4)
13-32: LGTM! Excellent test class structure.The test class follows all coding guidelines perfectly:
- Extends appropriate base test class
- Uses constructor injection via @Autowired
- Implements proper setup/teardown with mock provider
- Uses descriptive test naming conventions
34-45: Good configuration properties testing.The test properly verifies configuration properties and API key behavior using the recommended assertThat assertions.
47-73: Comprehensive API key validation test coverage.Excellent test coverage for various API key scenarios:
- Null keys
- Invalid large keys
- Valid keys (with and without Bearer prefix)
- Fake/invalid keys
All tests use proper assertThat assertions as required by coding guidelines.
47-74: Excellent API key validation test coverage.The API key validation tests cover all edge cases appropriately with descriptive names and specific assertions. The test scenarios (null, oversized, valid, and fake keys) provide comprehensive coverage.
src/main/webapp/app/sharing/sharing.route.ts (5)
1-4: LGTM! Proper imports and conventions.The imports follow Angular conventions and use the required single quotes for strings. Good use of authority constants for role-based access control.
5-14: Well-structured route configuration.The route configuration follows Angular routing best practices:
- Proper authority restrictions for EDITOR, INSTRUCTOR, and ADMIN roles
- Correct page title configuration for localization
- Clean route structure
16-23: Good routing architecture pattern.The use of spread operator and nested route structure provides flexibility for future route additions while maintaining clean organization.
5-14: Well-structured route configuration.The route configuration follows Angular style guidelines correctly with proper authority restrictions, localization key usage, and single quotes for strings.
1-23: LGTM! Clean routing implementation.The routing configuration follows Angular best practices and coding guidelines effectively:
- Uses single quotes for strings ✓
- Proper authority restrictions for EDITOR, INSTRUCTOR, and ADMIN ✓
- Clean route structure with appropriate nesting ✓
- Follows Angular style guide conventions ✓
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingException.java (4)
10-15: LGTM! Proper exception class structure.Good use of @Profile annotation and proper serialVersionUID declaration following Java serialization best practices.
32-42: Good static factory method pattern.The
connectionErrorstatic factory method is well-designed and properly documented. It provides a convenient way to create connection-specific exceptions with a consistent message format.
10-14: Well-designed exception class structure.The exception class properly extends Exception, uses the sharing profile, and includes the required serialVersionUID for serialization compatibility.
39-41: LGTM! Well-designed static factory method.The
connectionErrorstatic factory method provides a clear and convenient way to create connection-specific exceptions with proper formatting.src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingServiceTest.java (1)
17-17: Approve the test class structure.The test class properly extends the appropriate base test class and follows the established testing patterns with proper setup and teardown methods.
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingPlatformMockProvider.java (1)
58-67: 🛠️ Refactor suggestionImprove error handling and method documentation.
The method should handle potential JSON parsing errors more gracefully and validate the response content.
public SharingPluginConfig connectRequestFromSharingPlatform() throws Exception { + if (sharingApiKey == null || sharingApiKey.isBlank()) { + throw new IllegalStateException("Sharing API key is not configured for testing"); + } + MvcResult result = restMockMvc - .perform(get("/api/sharing/config").queryParam("apiBaseUrl", SHARING_BASEURL_PLUGIN).queryParam("installationName", TEST_INSTALLATION_NAME) + .perform(get("/api/sharing/config").queryParam("apiBaseUrl", getSharingPluginUrl()).queryParam("installationName", testInstallationName) .header("Authorization", sharingApiKey).contentType(MediaType.APPLICATION_JSON)) .andExpect(content().contentType(MediaType.APPLICATION_JSON)).andExpected(status().isOk()).andReturn(); String content = result.getResponse().getContentAsString(); + + if (content == null || content.isBlank()) { + throw new IllegalStateException("Empty response received from sharing config endpoint"); + } + SharingPluginConfig sharingPluginConfig = objectMapper.readerFor(SharingPluginConfig.class).readValue(content); assertThat(sharingPluginConfig.pluginName).isEqualTo("Artemis Sharing Connector"); return sharingPluginConfig; }Likely an incorrect or invalid review comment.
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.ts (4)
55-55: LGTM! Clean integration of sharing functionality.The imports and component registration follow Angular best practices. The sharing component is properly added to the imports array for standalone component usage.
Also applies to: 69-70, 92-92
159-159: Good implementation of feature flag pattern.The
isExportToSharingEnabledflag and subscription management follow Angular patterns correctly. Constructor injection of the sharing service is appropriate.Also applies to: 173-173, 179-179
275-286: Well-implemented subscription with proper error handling.The subscription to
isSharingEnabled()includes appropriate error handling that defaults tofalsewhen the sharing service is unavailable. The mapping and error handling are robust.
294-294: Memory leak prevention properly implemented.The sharing subscription is correctly unsubscribed in
ngOnDestroy(), preventing memory leaks.src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (2)
369-372: Well-documented profile constant.The
PROFILE_SHARINGconstant follows Java naming conventions and includes clear documentation.
375-377: Consistent API path constants.The sharing-related path constants are well-organized and follow the existing pattern. The
SHARINGCONFIG_RESOURCE_IS_ENABLEDconstant correctly builds upon the base path.Also applies to: 484-497
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (5)
20-27: Service structure follows Angular best practices.The service uses proper dependency injection and follows the singleton pattern with
providedIn: 'root'. The API endpoint definitions are appropriately organized.
32-39: HTTP request properly structured.The
getSharedExercisesmethod correctly uses observables and HTTP client patterns. The parameter passing and response mapping are well-implemented.
72-93: Excellent circular reference handling.The
convertDataFromClientmethod properly handles circular dependencies by removing the exercise reference from participations. This prevents serialization issues when sending data to the backend.
101-110: Robust date conversion handling.The date conversion method properly handles potential undefined values and uses dayjs consistently with the rest of the application.
128-132: Simple and effective feature detection.The
isSharingEnabledmethod provides a clean way to check if sharing functionality is available. The HTTP response approach allows for proper error handling upstream.src/main/webapp/app/sharing/search-result-dto.model.ts (4)
4-12: Well-structured main interface.The
SearchResultDTOinterface provides a comprehensive structure for sharing platform search results, including all necessary metadata and statistics.
65-70: Enum naming follows external API requirements.The
IExerciseTypeenum maintains compatibility with the sharing platform connector interface. The naming convention (with 'I' prefix and lowercase values) is intentionally different from internal TypeScript conventions to ensure external API compatibility.
75-83: Snake_case naming required for API compatibility.The
ProjectDTOinterface correctly uses snake_case property names to maintain compatibility with the external sharing platform's API contract, which is an intentional and necessary deviation from Angular's camelCase convention.
26-51: Comprehensive metadata interface.The
UserProvidedMetadataDTOinterface correctly matches the sharing platform's contract. The types and structure are essential for maintaining API compatibility.docs/admin/setup/sharing.rst (1)
1-95: Excellent documentation structure and completeness.This setup guide provides comprehensive coverage of the sharing integration configuration, including:
- Clear prerequisites and access requirements
- Multiple configuration options (YAML and environment variables)
- Instructor access setup with EduID and GitLab options
- Troubleshooting guidance with health indicators
- Visual aids to guide administrators
The documentation follows a logical flow and provides practical examples for both Docker and non-Docker deployments.
docs/user/sharing.rst (1)
1-111: Well-structured user guide with clear workflows.This user documentation effectively guides users through the sharing functionality with:
- Clear background information and prerequisites
- Step-by-step export process with metadata guidance
- Detailed import workflow from the sharing platform
- Helpful screenshots and visual aids throughout
- Practical tips about login requirements and potential issues
The documentation balances technical accuracy with user-friendliness.
src/main/webapp/app/programming/manage/grading/charts/programming-exercise-detail.component.with-sharing.spec.ts (3)
110-153: Well-structured test setup with comprehensive mocking.The beforeEach setup properly configures all necessary mocks and dependencies for testing the sharing functionality. The use of MockProvider and specific mock services follows Angular testing best practices.
173-184: Good test coverage for successful sharing configuration.The test properly verifies that the component sets
isExportToSharingEnabledto true when the sharing API returns a successful response.
186-203: Effective error handling test for sharing configuration.The test correctly validates that the component handles 404 responses gracefully by setting
isExportToSharingEnabledto false.src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingMultipartZipFile.java (3)
15-19: Well-structured class with appropriate profile activation.The class properly implements required interfaces and uses the sharing profile for conditional activation.
25-31: Excellent defensive programming in constructor.Proper null validation and buffering of the input stream improve reliability and performance.
86-90: Proper resource cleanup implementation.The close method safely handles null checks and properly closes the underlying stream.
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java (2)
21-57: Excellent Spring service structure with proper dependency injection.The service correctly uses constructor injection, profile activation, and has well-defined dependencies with clear responsibilities.
75-77: Good defensive programming for course assignment.The code properly handles the case where the exercise doesn't have an associated course and assigns it from the sharing setup info.
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceExportTest.java (4)
39-63: Well-structured integration test setup.The test class properly extends the base integration test class and correctly manages the sharing platform mock with appropriate setup and teardown methods.
77-90: Effective error scenario testing.The test properly verifies error handling when repositories are missing, checking both status code and error message content.
94-125: Comprehensive successful export test.The test thoroughly validates the export workflow, including redirect URL parsing, parameter validation, and ZIP file download verification.
127-137: Well-implemented URL parsing helper method.The helper method properly handles URI parsing and parameter encoding for MockMvc request building. Good separation of concerns.
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (2)
105-117: Security properly implemented with PreAuthorize.Good use of PreAuthorize annotation to restrict access to instructors, editors, and admins. The method properly delegates the business logic to the service layer.
119-147:✅ Verification successful
Verify security configuration for sharing endpoints.
The AI summary mentions that security configuration allows unauthenticated access to
/api/sharing/**endpoints. Please verify if the missing PreAuthorize annotations ongetProblemStatementandgetExerciseDetailsare intentional, given that these endpoints handle checksum validation internally.
🏁 Script executed:
#!/bin/bash # Description: Check security configuration for sharing endpoints # Search for security configuration related to sharing endpoints rg -A 10 -B 5 "/api/sharing" --glob "**/*Security*.java" --glob "**/*Config*.java" # Check if there are any security filters or configurations specifically for sharing fd -e java -x grep -l "sharing.*permit" {} \; # Look for WebSecurityConfig or similar files that might configure these endpoints ast-grep --pattern 'class $_ { $$$ "/api/sharing" $$$ }'Length of output: 2716
Security configuration for
/api/sharing/**is intentional
TheSecurityConfigurationexplicitly calls.requestMatchers("/api/sharing/**").permitAll()with the note that sharing endpoints enforce their own checksum‐based token validation. The absence of@PreAuthorizeongetProblemStatementandgetExerciseDetailsis therefore expected—no changes needed.
src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java
Outdated
Show resolved
Hide resolved
...test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingServiceTest.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java
Outdated
Show resolved
Hide resolved
src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
Show resolved
Hide resolved
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
Outdated
Show resolved
Hide resolved
...est/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorServiceTest.java
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary |
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (5)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java (1)
242-251: Use Spring's task executor instead of creating a new executor.This has already been identified in past reviews. Creating a new single-thread executor for one task is inefficient. Spring provides task execution infrastructure that should be used instead.
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java (1)
264-264: Fix typo in comment.This typo has already been identified in past reviews and should be corrected.
src/main/webapp/app/sharing/sharing.component.ts (3)
80-83: Add null safety check for token expiry calculation.The method should handle the case where
tokenValidUntilmight be undefined or invalid.
88-95: Improve error handling to avoid exposing internal details.The error handling directly exposes server error messages to users, which can reveal sensitive information.
173-180: Improve error handling consistency.The error handling here is inconsistent with the pattern used elsewhere in the component.
🧹 Nitpick comments (7)
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts (2)
34-36: Consider consolidating test files for better maintainability.The comment indicates this is a temporary separation for sharing-specific tests. Consider either merging with the main test file or making this separation permanent with clear documentation.
If this separation is intended to be permanent, update the comment to reflect the rationale:
-/* - * just use a separate file for sharing aspects, could be merged into programming-exercise-detail.component.spec.ts if stabilized. - */ +/** + * Dedicated test suite for sharing functionality in ProgrammingExerciseDetailComponent. + * Separated to maintain focused testing of the sharing feature integration. + */
173-184: Enhance test assertions for better validation.The tests correctly verify the
isExportToSharingEnabledflag but could benefit from additional assertions to ensure complete behavior validation.Consider adding assertions to verify the HTTP request details and component state:
it('should be in sharing mode', async () => { // WHEN comp.ngOnInit(); comp.programmingExercise = mockProgrammingExercise; comp.programmingExerciseBuildConfig = mockProgrammingExercise.buildConfig; const req = httpMock.expectOne({ method: 'GET', url: 'api/sharing/config/isEnabled' }); + expect(req.request.method).toBe('GET'); + expect(req.request.url).toBe('api/sharing/config/isEnabled'); req.flush(true); // THEN expect(comp.isExportToSharingEnabled).toBeTruthy(); + expect(comp.programmingExercise).toEqual(mockProgrammingExercise); });Apply similar enhancements to the negative test case for consistency.
Also applies to: 186-203
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java (1)
37-37: Fix typo in constant name.The constant name has a typo that affects readability and professionalism.
- public static final String UNKNOWN_INSTALLATIONAME = "unknown installationame"; + public static final String UNKNOWN_INSTALLATION_NAME = "unknown installation name";src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java (2)
355-386: Extract utility method to test utility class.The
makeCourseJSONSerializablemethod is a utility that modifies course entities for JSON serialization. This violates the single responsibility principle for test classes.Consider extracting this method to a dedicated test utility class like
CourseTestUtilorJsonSerializationTestUtilto promote reusability across test classes and improve maintainability.public static void makeCourseJSONSerializable(Course course) { // existing implementation }
223-248: Consider extracting checksum utility methods.The
parseParamsToMap,calculateCorrectChecksum, andaddCorrectChecksummethods are utility functions that could be reused across other sharing-related tests.Consider extracting these to a shared test utility class like
SharingTestUtilto promote code reuse and reduce duplication across test classes in the sharing module.src/main/webapp/app/sharing/sharing.component.ts (2)
109-111: Consider more descriptive method name and validation.The method name
courseId()could be more descriptive, and there's no validation for course selection state.Consider renaming and adding validation:
- courseId(): number { - return this.selectedCourse?.id ?? 0; - } + getSelectedCourseId(): number { + if (!this.selectedCourse) { + throw new Error('No course selected'); + } + return this.selectedCourse.id; + }
126-128: Fix return type annotation for trackId method.The trackId method should explicitly return a number type to match Angular's TrackByFunction interface requirements.
- trackId(index: number, item: Course) { + trackId(index: number, item: Course): number { return item.id; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingException.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingHealthIndicator.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSetupInfo.java(1 hunks)src/main/webapp/app/app.routes.ts(1 hunks)src/main/webapp/app/environments/environment.override.ts(1 hunks)src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts(1 hunks)src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts(1 hunks)src/main/webapp/app/sharing/sharing.component.html(1 hunks)src/main/webapp/app/sharing/sharing.component.spec.ts(1 hunks)src/main/webapp/app/sharing/sharing.component.ts(1 hunks)src/main/webapp/app/sharing/sharing.model.ts(1 hunks)src/main/webapp/app/sharing/sharing.scss(1 hunks)src/main/webapp/i18n/en/sharing.json(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorHealthCheckRegistryTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingInfoDTOTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingPlatformMockProvider.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSupportResourceTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/main/webapp/app/environments/environment.override.ts
- src/main/webapp/app/sharing/sharing.component.html
- src/main/webapp/app/sharing/sharing.model.ts
🚧 Files skipped from review as they are similar to previous changes (15)
- src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java
- src/main/webapp/i18n/en/sharing.json
- src/main/webapp/app/app.routes.ts
- src/main/webapp/app/sharing/sharing.scss
- src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSetupInfo.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingHealthIndicator.java
- src/main/webapp/app/sharing/sharing.component.spec.ts
- src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingException.java
- src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSupportResourceTest.java
- src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingPlatformMockProvider.java
- src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorHealthCheckRegistryTest.java
- src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts
- src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingInfoDTOTest.java
🧰 Additional context used
📓 Path-based instructions (3)
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/sharing/sharing.component.tssrc/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts
`src/test/java/**/*.java`: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_...
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/programming/service/sharing/ExerciseSharingResourceImportTest.java
`src/main/java/**/*.java`: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,de...
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java
🪛 GitHub Check: client-tests-selected
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts
[failure] 49-49:
'submissionPolicyServiceStub' is declared but its value is never read.
[failure] 48-48:
'profileServiceStub' is declared but its value is never read.
[failure] 47-47:
'gitDiffReportStub' is declared but its value is never read.
[failure] 46-46:
'statisticsServiceStub' is declared but its value is never read.
[failure] 42-42:
'alertService' is declared but its value is never read.
[failure] 30-30:
Cannot find module 'app/entities/programming/build-log-statistics-dto' or its corresponding type declarations.
[failure] 27-27:
'"app/programming/shared/services/programming-language-feature/programming-language-feature.service"' has no exported member named 'ProgrammingLanguageFeature'. Did you mean 'ProgrammingLanguageFeatureService'?
[failure] 26-26:
Cannot find module 'app/shared/layouts/profiles/profile-info.model' or its corresponding type declarations.
[failure] 24-24:
Cannot find module 'app/entities/participation/solution-programming-exercise-participation.model' or its corresponding type declarations.
[failure] 23-23:
Cannot find module 'app/entities/participation/template-programming-exercise-participation.model' or its corresponding type declarations.
🪛 GitHub Check: client-tests
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts
[failure] 49-49:
'submissionPolicyServiceStub' is declared but its value is never read.
[failure] 48-48:
'profileServiceStub' is declared but its value is never read.
[failure] 47-47:
'gitDiffReportStub' is declared but its value is never read.
[failure] 46-46:
'statisticsServiceStub' is declared but its value is never read.
[failure] 42-42:
'alertService' is declared but its value is never read.
[failure] 30-30:
Cannot find module 'app/entities/programming/build-log-statistics-dto' or its corresponding type declarations.
[failure] 27-27:
'"app/programming/shared/services/programming-language-feature/programming-language-feature.service"' has no exported member named 'ProgrammingLanguageFeature'. Did you mean 'ProgrammingLanguageFeatureService'?
[failure] 26-26:
Cannot find module 'app/shared/layouts/profiles/profile-info.model' or its corresponding type declarations.
[failure] 24-24:
Cannot find module 'app/entities/participation/solution-programming-exercise-participation.model' or its corresponding type declarations.
[failure] 23-23:
Cannot find module 'app/entities/participation/template-programming-exercise-participation.model' or its corresponding type declarations.
⏰ Context from checks skipped due to timeout of 90000ms (6)
- 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: client-style
- GitHub Check: Analyse
- 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
🔇 Additional comments (3)
src/main/webapp/app/sharing/sharing.component.ts (3)
1-21: LGTM - Clean imports and proper dependency structure.The imports are well-organized and follow Angular conventions. Good use of standalone component imports and proper separation of concerns.
25-31: LGTM - Proper standalone component configuration.The component decorator correctly configures the standalone component with all necessary imports. The selector follows kebab-case naming convention as required.
33-62: LGTM - Well-structured component properties.Properties follow camelCase naming convention and avoid private prefixes as required by the coding guidelines. Good use of TypeScript typing and proper initialization.
...app/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts
Outdated
Show resolved
Hide resolved
...app/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts
Outdated
Show resolved
Hide resolved
...app/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java
Show resolved
Hide resolved
...va/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary |
2 similar comments
End-to-End (E2E) Test Results Summary |
End-to-End (E2E) Test Results Summary |
Michael-Breu-UIbk
force-pushed
the
feature/connecting-to-sharing-platform
branch
from
562e101 to
8f6ada8
Compare
End-to-End (E2E) Test Results Summary |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (11)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java (6)
259-259: Remove server-side HTML escaping for Markdown content.Server-side escaping of
problemStatementwill break Markdown formatting when rendered by the client-side Markdown renderer. The client should handle sanitization.- return Objects.requireNonNullElse(org.springframework.web.util.HtmlUtils.htmlEscape(problemStatement), "No Problem Statement found!"); + return Objects.requireNonNullElse(problemStatement, "No Problem Statement found!");
370-370: Use proper URL-safe Base64 encoding instead of manual padding removal.Manually removing Base64 padding can invalidate the encoding. Use URL-safe Base64 encoder without padding for clean, valid encoding.
- String tokenInB64 = Base64.getEncoder().encodeToString(token.getBytes()).replaceAll("=+$", ""); + String tokenInB64 = Base64.getUrlEncoder().withoutPadding().encodeToString(token.getBytes());
415-416: Don't return empty array on cryptographic failures.Returning an empty byte array on HMAC generation failure is a security vulnerability as it produces predictable output.
catch (NoSuchAlgorithmException | InvalidKeyException e) { - return Base64.getEncoder().encodeToString(new byte[] {}); + throw new SharingException("Failed to generate HMAC", e); }
404-406: Add null check for pre-shared key.The PSK retrieved from
getSharingApiKeyOrNull()could be null if not configured, causing a NullPointerException.String psk = sharingConnectorService.getSharingApiKeyOrNull(); + if (psk == null) { + throw new IllegalStateException("Sharing API key is not configured"); + } SecretKeySpec secretKeySpec = new SecretKeySpec(psk.getBytes(StandardCharsets.UTF_8), algorithm);
429-432: Use proper URL decoding instead of manual character replacement.Manual space-to-plus conversion indicates improper URL decoding. This should be handled at the REST endpoint level with proper
@RequestParamconfiguration.
443-449: Strengthen token validation against directory traversal.The decoded token is used directly in path construction, which could allow directory traversal attacks if the token contains path separators.
String decodedToken = new String(Base64.getDecoder().decode(b64Token)); + // Additional validation after decoding + if (decodedToken.contains("..") || decodedToken.contains("/") || decodedToken.contains("\\")) { + log.warn("Token contains invalid path characters: {}", b64Token); + return null; + } Path parent = Paths.get(repoDownloadClonePath, decodedToken + ".zip");src/main/webapp/app/sharing/sharing.component.spec.ts (2)
133-133: Fix typo in error message."Bakset" should be "Basket".
- { message: 'Bakset not found' }, // error body + { message: 'Basket not found' }, // error body
147-147: MissingtoBeBetween()custom matcher definition.The
.toBeBetween()matcher used here isn't defined anywhere in the codebase, causing test failures.Create a custom matcher in your test helpers:
beforeAll(() => { jasmine.addMatchers({ toBeBetween: () => ({ compare(actual: Date, floor: Date, ceiling: Date) { const pass = actual >= floor && actual <= ceiling; return { pass, message: pass ? `Expected ${actual} not to be between ${floor} and ${ceiling}` : `Expected ${actual} to be between ${floor} and ${ceiling}`, }; }, }), }); });src/main/webapp/app/sharing/sharing.component.ts (2)
80-84: Add null safety check for token expiry calculation.The method should handle the case where
tokenValidUntilmight be undefined or invalid.getTokenExpiryDate(): Date { - if (this.shoppingBasket) { + if (this.shoppingBasket?.tokenValidUntil) { return new Date(this.shoppingBasket.tokenValidUntil); } return new Date(); }
93-93: Avoid exposing internal error messages to users.Directly concatenating
res.messagein error alerts can expose sensitive internal information to users.- error: (res: HttpErrorResponse) => this.alertService.error('Cannot load courses: ' + res.message), + error: (res: HttpErrorResponse) => this.alertService.error('artemisApp.sharing.error.loadingCourses'),src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts (1)
23-30: Fix import path errors that prevent compilation.Multiple imports have incorrect paths causing compilation failures. Based on the relevant code snippets, here are the correct paths:
-import { TemplateProgrammingExerciseParticipation } from 'app/entities/participation/template-programming-exercise-participation.model'; -import { SolutionProgrammingExerciseParticipation } from 'app/entities/participation/solution-programming-exercise-participation.model'; +import { TemplateProgrammingExerciseParticipation } from 'app/exercise/shared/entities/participation/template-programming-exercise-participation.model'; +import { SolutionProgrammingExerciseParticipation } from 'app/exercise/shared/entities/participation/solution-programming-exercise-participation.model'; -import { ProfileInfo } from 'app/shared/layouts/profiles/profile-info.model'; -import { ProgrammingLanguageFeature, ProgrammingLanguageFeatureService } from 'app/programming/shared/services/programming-language-feature/programming-language-feature.service'; +import { ProfileInfo, ProgrammingLanguageFeature } from 'app/core/layouts/profiles/profile-info.model'; +import { ProgrammingLanguageFeatureService } from 'app/programming/shared/services/programming-language-feature/programming-language-feature.service'; -import { BuildLogStatisticsDTO } from 'app/entities/programming/build-log-statistics-dto'; +// Remove this unused import as BuildLogStatisticsDTO is not used in the test🧰 Tools
🪛 GitHub Check: client-tests
[failure] 30-30:
Cannot find module 'app/entities/programming/build-log-statistics-dto' or its corresponding type declarations.
[failure] 30-30:
'BuildLogStatisticsDTO' is declared but its value is never read.
[failure] 27-27:
'"app/programming/shared/services/programming-language-feature/programming-language-feature.service"' has no exported member named 'ProgrammingLanguageFeature'. Did you mean 'ProgrammingLanguageFeatureService'?
[failure] 26-26:
Cannot find module 'app/shared/layouts/profiles/profile-info.model' or its corresponding type declarations.
[failure] 24-24:
Cannot find module 'app/entities/participation/solution-programming-exercise-participation.model' or its corresponding type declarations.
[failure] 23-23:
Cannot find module 'app/entities/participation/template-programming-exercise-participation.model' or its corresponding type declarations.🪛 GitHub Check: client-tests-selected
[failure] 30-30:
Cannot find module 'app/entities/programming/build-log-statistics-dto' or its corresponding type declarations.
[failure] 30-30:
'BuildLogStatisticsDTO' is declared but its value is never read.
[failure] 27-27:
'"app/programming/shared/services/programming-language-feature/programming-language-feature.service"' has no exported member named 'ProgrammingLanguageFeature'. Did you mean 'ProgrammingLanguageFeatureService'?
[failure] 26-26:
Cannot find module 'app/shared/layouts/profiles/profile-info.model' or its corresponding type declarations.
[failure] 24-24:
Cannot find module 'app/entities/participation/solution-programming-exercise-participation.model' or its corresponding type declarations.
[failure] 23-23:
Cannot find module 'app/entities/participation/template-programming-exercise-participation.model' or its corresponding type declarations.
🧹 Nitpick comments (2)
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts (2)
126-129: Remove duplicate service mock assignment.The
profileService.getProfileInfomethod is mocked twice, creating redundant assignments.jest.spyOn(exerciseService, 'getDiffReport').mockReturnValue(of(gitDiffReport)); jest.spyOn(profileService, 'getProfileInfo').mockReturnValue(of(profileInfo)); jest.spyOn(submissionPolicyService, 'getSubmissionPolicyOfProgrammingExercise').mockReturnValue(of(undefined)); - - jest.spyOn(profileService, 'getProfileInfo').mockReturnValue(of(profileInfo));🧰 Tools
🪛 GitHub Check: client-tests
[failure] 129-129:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
[failure] 126-126:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.🪛 GitHub Check: client-tests-selected
[failure] 129-129:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
[failure] 126-126:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
46-58: Fix type assertion issue in mock object.The mock object doesn't fully match the
ProgrammingExercisetype structure, causing compilation warnings.- const mockProgrammingExercise = { + const mockProgrammingExercise: Partial<ProgrammingExercise> = { id: 1, categories: [{ category: 'Important' }], templateParticipation: { id: 1, } as TemplateProgrammingExerciseParticipation, solutionParticipation: { id: 2, } as SolutionProgrammingExerciseParticipation, buildConfig: { buildTool: 'GRADLE', }, - } as ProgrammingExercise; + };🧰 Tools
🪛 GitHub Check: client-tests
[failure] 46-46:
Conversion of type '{ id: number; categories: { category: string; }[]; templateParticipation: TemplateProgrammingExerciseParticipation; solutionParticipation: SolutionProgrammingExerciseParticipation; buildConfig: { ...; }; }' to type 'ProgrammingExercise' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.🪛 GitHub Check: client-tests-selected
[failure] 46-46:
Conversion of type '{ id: number; categories: { category: string; }[]; templateParticipation: TemplateProgrammingExerciseParticipation; solutionParticipation: SolutionProgrammingExerciseParticipation; buildConfig: { ...; }; }' to type 'ProgrammingExercise' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (16)
.github/workflows/build.ymlis excluded by!**/*.ymldocs/admin/setup/sharing/sharingButtonArtemis.pngis excluded by!**/*.png,!**/*.pngdocs/admin/setup/sharing/sharingButtonSharing.pngis excluded by!**/*.png,!**/*.pngdocs/admin/setup/sharing/sharing_health1.pngis excluded by!**/*.png,!**/*.pngdocs/admin/setup/sharing/sharing_health2.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/artemis_import.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/artemis_import2.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/sharing_export.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/sharing_metadata.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/sharing_metadata2.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/sharing_namespace.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/sharing_search.pngis excluded by!**/*.png,!**/*.pngdocs/user/sharing/sharing_success.pngis excluded by!**/*.png,!**/*.pngsrc/main/resources/config/application-dev.ymlis excluded by!**/*.ymlsrc/main/resources/config/application-sharing.ymlis excluded by!**/*.ymlsrc/test/resources/de/tum/cit/aet/artemis/programming/service/sharing/basket/sampleExercise.zipis excluded by!**/*.zip,!**/*.zip
📒 Files selected for processing (54)
build.gradle(2 hunks)docker/artemis/config/dev-local-vc-local-ci.env(1 hunks)docs/admin/extension-services.rst(1 hunks)docs/admin/setup/sharing.rst(1 hunks)docs/index.rst(1 hunks)docs/user/sharing.rst(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/RestTemplateConfiguration.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/dto/SharingInfoDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingException.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingHealthIndicator.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingMultipartZipFile.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSetupInfo.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java(1 hunks)src/main/webapp/app/app.routes.ts(1 hunks)src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.html(1 hunks)src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.ts(7 hunks)src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts(1 hunks)src/main/webapp/app/programming/manage/exercise/programming-exercise.component.ts(1 hunks)src/main/webapp/app/programming/manage/programming-exercise-management.route.ts(1 hunks)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts(1 hunks)src/main/webapp/app/programming/manage/update/programming-exercise-creation-config.ts(1 hunks)src/main/webapp/app/programming/manage/update/programming-exercise-update.component.html(3 hunks)src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts(17 hunks)src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts(1 hunks)src/main/webapp/app/sharing/search-result-dto.model.ts(1 hunks)src/main/webapp/app/sharing/sharing.component.html(1 hunks)src/main/webapp/app/sharing/sharing.component.spec.ts(1 hunks)src/main/webapp/app/sharing/sharing.component.ts(1 hunks)src/main/webapp/app/sharing/sharing.model.ts(1 hunks)src/main/webapp/app/sharing/sharing.route.ts(1 hunks)src/main/webapp/app/sharing/sharing.scss(1 hunks)src/main/webapp/i18n/de/programmingExercise.json(1 hunks)src/main/webapp/i18n/de/sharing.json(1 hunks)src/main/webapp/i18n/en/programmingExercise.json(1 hunks)src/main/webapp/i18n/en/sharing.json(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceExportTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingServiceTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorHealthCheckRegistryTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorServiceTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingInfoDTOTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingPlatformMockProvider.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSupportResourceTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationIndependentTest.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java(3 hunks)src/test/resources/de/tum/cit/aet/artemis/programming/service/sharing/basket/sampleBasket.json(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- docs/admin/extension-services.rst
- src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationIndependentTest.java
- build.gradle
- src/main/webapp/i18n/en/programmingExercise.json
- src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts
- src/main/webapp/app/sharing/sharing.component.html
- docs/admin/setup/sharing.rst
- src/main/webapp/app/sharing/search-result-dto.model.ts
🚧 Files skipped from review as they are similar to previous changes (39)
- docs/index.rst
- src/main/webapp/app/programming/manage/exercise/programming-exercise.component.ts
- src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.html
- src/main/webapp/app/programming/manage/update/programming-exercise-creation-config.ts
- src/main/webapp/app/programming/manage/update/programming-exercise-update.component.html
- src/main/webapp/i18n/en/sharing.json
- src/main/webapp/app/app.routes.ts
- src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java
- src/main/webapp/app/programming/manage/programming-exercise-management.route.ts
- src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java
- src/test/resources/de/tum/cit/aet/artemis/programming/service/sharing/basket/sampleBasket.json
- src/main/webapp/i18n/de/programmingExercise.json
- src/main/webapp/i18n/de/sharing.json
- src/main/java/de/tum/cit/aet/artemis/core/config/RestTemplateConfiguration.java
- src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSetupInfo.java
- src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingServiceTest.java
- src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingPlatformMockProvider.java
- src/main/webapp/app/sharing/sharing.model.ts
- src/main/webapp/app/sharing/sharing.scss
- src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorHealthCheckRegistryTest.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingException.java
- src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSupportResourceTest.java
- src/main/webapp/app/sharing/sharing.route.ts
- src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingHealthIndicator.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java
- docs/user/sharing.rst
- src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingMultipartZipFile.java
- src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts
- src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorServiceTest.java
- src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java
- src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.ts
- src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java
- src/main/java/de/tum/cit/aet/artemis/core/dto/SharingInfoDTO.java
- src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
- src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceExportTest.java
- src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java
🧰 Additional context used
📓 Path-based instructions (3)
`src/test/java/**/*.java`: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_...
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/programming/service/sharing/SharingInfoDTOTest.java
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/sharing/sharing.component.tssrc/main/webapp/app/sharing/sharing.component.spec.tssrc/main/webapp/app/programming/manage/update/programming-exercise-update.component.tssrc/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts
`src/main/java/**/*.java`: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,de...
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
🧠 Learnings (2)
📓 Common learnings
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.274Z
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.
src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts (1)
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts:1283-1303
Timestamp: 2025-06-06T14:12:04.548Z
Learning: Wallenstein61 prefers to minimize interference with original code when implementing new features, even if it means not following optimal patterns like avoiding nested subscriptions. This approach prioritizes compatibility and reduced risk over strict adherence to best practices in large established codebases.
🧬 Code Graph Analysis (3)
src/main/webapp/app/sharing/sharing.component.spec.ts (4)
src/test/javascript/spec/helpers/mocks/service/mock-alert.service.ts (1)
MockAlertService(3-8)src/test/javascript/spec/helpers/mocks/mock-router.ts (1)
MockRouter(5-30)src/main/webapp/app/sharing/sharing.model.ts (1)
ShoppingBasket(50-54)src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-180)
src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts (1)
src/main/webapp/app/programming/manage/update/programming-exercise-creation-config.ts (1)
ProgrammingExerciseCreationConfig(7-56)
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts (4)
src/main/webapp/app/core/layouts/profiles/profile-info.model.ts (1)
ProgrammingLanguageFeature(115-124)src/test/javascript/spec/helpers/mocks/service/mock-ngb-modal.service.ts (1)
MockNgbModalService(1-5)src/test/javascript/spec/helpers/mocks/mock-router.ts (1)
MockRouter(5-30)src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-180)
🪛 GitHub Check: client-tests
src/main/webapp/app/sharing/sharing.component.spec.ts
[failure] 2-2:
Cannot find module '../../helpers/mocks/service/mock-translate.service' or its corresponding type declarations.
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts
[failure] 129-129:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
[failure] 126-126:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
[failure] 46-46:
Conversion of type '{ id: number; categories: { category: string; }[]; templateParticipation: TemplateProgrammingExerciseParticipation; solutionParticipation: SolutionProgrammingExerciseParticipation; buildConfig: { ...; }; }' to type 'ProgrammingExercise' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
[failure] 30-30:
Cannot find module 'app/entities/programming/build-log-statistics-dto' or its corresponding type declarations.
[failure] 30-30:
'BuildLogStatisticsDTO' is declared but its value is never read.
[failure] 27-27:
'"app/programming/shared/services/programming-language-feature/programming-language-feature.service"' has no exported member named 'ProgrammingLanguageFeature'. Did you mean 'ProgrammingLanguageFeatureService'?
[failure] 26-26:
Cannot find module 'app/shared/layouts/profiles/profile-info.model' or its corresponding type declarations.
[failure] 24-24:
Cannot find module 'app/entities/participation/solution-programming-exercise-participation.model' or its corresponding type declarations.
[failure] 23-23:
Cannot find module 'app/entities/participation/template-programming-exercise-participation.model' or its corresponding type declarations.
🪛 GitHub Check: client-tests-selected
src/main/webapp/app/sharing/sharing.component.spec.ts
[failure] 2-2:
Cannot find module '../../helpers/mocks/service/mock-translate.service' or its corresponding type declarations.
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts
[failure] 129-129:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
[failure] 126-126:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
[failure] 46-46:
Conversion of type '{ id: number; categories: { category: string; }[]; templateParticipation: TemplateProgrammingExerciseParticipation; solutionParticipation: SolutionProgrammingExerciseParticipation; buildConfig: { ...; }; }' to type 'ProgrammingExercise' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
[failure] 30-30:
Cannot find module 'app/entities/programming/build-log-statistics-dto' or its corresponding type declarations.
[failure] 30-30:
'BuildLogStatisticsDTO' is declared but its value is never read.
[failure] 27-27:
'"app/programming/shared/services/programming-language-feature/programming-language-feature.service"' has no exported member named 'ProgrammingLanguageFeature'. Did you mean 'ProgrammingLanguageFeatureService'?
[failure] 26-26:
Cannot find module 'app/shared/layouts/profiles/profile-info.model' or its corresponding type declarations.
[failure] 24-24:
Cannot find module 'app/entities/participation/solution-programming-exercise-participation.model' or its corresponding type declarations.
[failure] 23-23:
Cannot find module 'app/entities/participation/template-programming-exercise-participation.model' or its corresponding type declarations.
🪛 ast-grep (0.38.1)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
[warning] 408-408: Use of AES with ECB mode detected. ECB doesn't provide message confidentiality and is not semantically secure so should not be used. Instead, use a strong, secure cipher: Cipher.getInstance("AES/CBC/PKCS7PADDING"). See https://owasp.org/www-community/Using_the_Java_Cryptographic_Extensions for more information.
Context: Mac.getInstance(algorithm)
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
- https://googleprojectzero.blogspot.com/2022/10/rc4-is-still-considered-harmful.html
(use-of-aes-ecb-java)
[warning] 408-408: Triple DES (3DES or DESede) is considered deprecated. AES is the recommended cipher. Upgrade to use AES.
Context: Mac.getInstance(algorithm)
Note: [CWE-326]: Inadequate Encryption Strength [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://find-sec-bugs.github.io/bugs.htm#TDES_USAGE
- https://csrc.nist.gov/News/2017/Update-to-Current-Use-and-Deprecation-of-TDEA
(desede-is-deprecated-java)
🪛 dotenv-linter (3.3.0)
docker/artemis/config/dev-local-vc-local-ci.env
[warning] 5-5: [IncorrectDelimiter] The SPRING_PROFILES_ACTIVE: artemis,scheduling,localci,localvc,buildagent,core,dev,docker,sharing key has incorrect delimiter
[warning] 5-5: [KeyWithoutValue] The SPRING_PROFILES_ACTIVE: artemis,scheduling,localci,localvc,buildagent,core,dev,docker,sharing key should be with a value or have an equal sign
[warning] 5-5: [LowercaseKey] The SPRING_PROFILES_ACTIVE: artemis,scheduling,localci,localvc,buildagent,core,dev,docker,sharing key should be in uppercase
⏰ Context from checks skipped due to timeout of 90000ms (10)
- 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: server-tests
- GitHub Check: server-style
- GitHub Check: Analyse
- 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
🔇 Additional comments (9)
docker/artemis/config/dev-local-vc-local-ci.env (1)
5-5: LGTM: Sharing profile correctly added.The addition of the "sharing" profile to
SPRING_PROFILES_ACTIVEproperly enables the sharing platform integration feature as documented.🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 5-5: [IncorrectDelimiter] The SPRING_PROFILES_ACTIVE: artemis,scheduling,localci,localvc,buildagent,core,dev,docker,sharing key has incorrect delimiter
[warning] 5-5: [KeyWithoutValue] The SPRING_PROFILES_ACTIVE: artemis,scheduling,localci,localvc,buildagent,core,dev,docker,sharing key should be with a value or have an equal sign
[warning] 5-5: [LowercaseKey] The SPRING_PROFILES_ACTIVE: artemis,scheduling,localci,localvc,buildagent,core,dev,docker,sharing key should be in uppercase
src/main/webapp/app/sharing/sharing.component.ts (1)
170-209: LGTM: Initialization and error handling improvements.The refactoring of
ngOnInit()with parameter validation, separateloadSharedExercises()method, and improved navigation error handling addresses the previous concerns well.src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingInfoDTOTest.java (1)
41-64: Well-structured test methods following best practices.The test methods properly validate the
SharingMultipartZipFilefunctionality with appropriate resource management using try-with-resources and comprehensive assertions for file properties and exception handling.src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts (6)
10-10: Well-integrated sharing service and properties.The integration of
ProgrammingExerciseSharingServiceandSharingInfofollows established patterns in the component with proper service injection and property initialization.Also applies to: 43-43, 82-96, 152-152, 207-207
471-471: Proper route segment detection for sharing import.The route parsing correctly identifies the
import-from-sharingsegment and triggers the appropriate setup method, following the same pattern used for other import types.Also applies to: 480-482
132-134: Appropriate UI field visibility control for sharing.The conditional display of the SHORT_NAME field when sharing info is present ensures proper user experience during sharing import workflows.
352-352: Consistent conditional checks for sharing import.The
isImportFromSharingflag is properly integrated into existing conditional logic throughout the component, maintaining consistency with other import types.Also applies to: 373-373, 520-520, 645-645, 722-722
771-780: Effective sharing import save implementation.The save logic for sharing import properly calls the sharing service, handles success/error scenarios, and integrates well with the existing save flow.
1308-1308: Complete configuration integration.The
isImportFromSharingflag is properly included in the programming exercise creation configuration, ensuring consistent state throughout the component tree.
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingInfoDTOTest.java
Outdated
Show resolved
Hide resolved
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingInfoDTOTest.java
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary |
1 similar comment
End-to-End (E2E) Test Results Summary |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/main/webapp/app/sharing/sharing.component.spec.ts (3)
2-2: Fix import path for mock translate service.The import path appears to be incorrect based on the project structure. The current relative path may not resolve properly.
Based on previous review feedback, this import path needs to be corrected to point to the actual location of the mock translate service file.
133-133: Fix typo in error message."Bakset" should be "Basket".
- { message: 'Bakset not found' }, // error body + { message: 'Basket not found' }, // error body
146-146: Missing custom matcher definition.The
.toBeBetween()matcher used here is not a standard Jasmine matcher and needs to be defined.This custom matcher must be implemented and registered before it can be used in tests. Consider using standard Jasmine matchers instead:
- expect(fixture.componentInstance.getTokenExpiryDate()).toBeBetween(new Date(Date.now() - 1000), new Date(Date.now() + 1000)); + const tokenExpiryDate = fixture.componentInstance.getTokenExpiryDate(); + expect(tokenExpiryDate.getTime()).toBeGreaterThanOrEqual(Date.now() - 1000); + expect(tokenExpiryDate.getTime()).toBeLessThanOrEqual(Date.now() + 1000);
🧹 Nitpick comments (4)
src/main/webapp/app/sharing/sharing.component.spec.ts (2)
71-72: Fix typo in course titles."testCouse" should be "testCourse" in both course objects.
- { id: 1, title: 'testCouse 1' }, - { id: 2, title: 'testCouse 2' }, + { id: 1, title: 'testCourse 1' }, + { id: 2, title: 'testCourse 2' },
186-192: Simplify redundant error testing.The try-catch block from lines 186-192 tests the same validation error as lines 180-185. This duplication is unnecessary.
- try { - sharingInfo.validate(); - throw new Error('Error expected, got none'); - } catch (err) { - expect(err).toBeInstanceOf(Error); - expect(err.message).toBe('Basket token is required'); - }src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts (2)
45-57: Fix type assertion for mock programming exercise.The static analysis correctly identifies a type mismatch. Use proper type assertion to resolve this.
- const mockProgrammingExercise = { + const mockProgrammingExercise = { id: 1, categories: [{ category: 'Important' }], templateParticipation: { id: 1, } as TemplateProgrammingExerciseParticipation, solutionParticipation: { id: 2, } as SolutionProgrammingExerciseParticipation, buildConfig: { buildTool: 'GRADLE', }, - } as ProgrammingExercise; + } as Partial<ProgrammingExercise>;🧰 Tools
🪛 GitHub Check: client-tests
[failure] 45-45:
Conversion of type '{ id: number; categories: { category: string; }[]; templateParticipation: TemplateProgrammingExerciseParticipation; solutionParticipation: SolutionProgrammingExerciseParticipation; buildConfig: { ...; }; }' to type 'ProgrammingExercise' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.🪛 GitHub Check: client-tests-selected
[failure] 45-45:
Conversion of type '{ id: number; categories: { category: string; }[]; templateParticipation: TemplateProgrammingExerciseParticipation; solutionParticipation: SolutionProgrammingExerciseParticipation; buildConfig: { ...; }; }' to type 'ProgrammingExercise' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
125-128: Remove duplicate mock setup.The
profileService.getProfileInfois mocked twice with identical configuration, creating unnecessary duplication.jest.spyOn(exerciseService, 'getDiffReport').mockReturnValue(of(gitDiffReport)); jest.spyOn(profileService, 'getProfileInfo').mockReturnValue(of(profileInfo)); jest.spyOn(submissionPolicyService, 'getSubmissionPolicyOfProgrammingExercise').mockReturnValue(of(undefined)); - jest.spyOn(profileService, 'getProfileInfo').mockReturnValue(of(profileInfo)); jest.spyOn(programmingLanguageFeatureService, 'getProgrammingLanguageFeature').mockReturnValue({ plagiarismCheckSupported: true, } as ProgrammingLanguageFeature);🧰 Tools
🪛 GitHub Check: client-tests
[failure] 128-128:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
[failure] 125-125:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.🪛 GitHub Check: client-tests-selected
[failure] 128-128:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
[failure] 125-125:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts(1 hunks)src/main/webapp/app/sharing/sharing.component.spec.ts(1 hunks)src/main/webapp/app/sharing/sharing.component.ts(1 hunks)src/test/javascript/spec/helpers/mocks/programming-exercise-creation-config-mock.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/test/javascript/spec/helpers/mocks/programming-exercise-creation-config-mock.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/webapp/app/sharing/sharing.component.ts
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/sharing/sharing.component.spec.tssrc/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts
🧠 Learnings (1)
📓 Common learnings
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.274Z
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.
🧬 Code Graph Analysis (2)
src/main/webapp/app/sharing/sharing.component.spec.ts (4)
src/test/javascript/spec/helpers/mocks/service/mock-alert.service.ts (1)
MockAlertService(3-8)src/test/javascript/spec/helpers/mocks/mock-router.ts (1)
MockRouter(5-30)src/main/webapp/app/sharing/sharing.model.ts (1)
ShoppingBasket(50-54)src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-180)
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts (4)
src/main/webapp/app/core/layouts/profiles/profile-info.model.ts (1)
ProgrammingLanguageFeature(115-124)src/test/javascript/spec/helpers/mocks/service/mock-ngb-modal.service.ts (1)
MockNgbModalService(1-5)src/test/javascript/spec/helpers/mocks/mock-router.ts (1)
MockRouter(5-30)src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-180)
🪛 GitHub Check: client-tests
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts
[failure] 128-128:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
[failure] 125-125:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
[failure] 45-45:
Conversion of type '{ id: number; categories: { category: string; }[]; templateParticipation: TemplateProgrammingExerciseParticipation; solutionParticipation: SolutionProgrammingExerciseParticipation; buildConfig: { ...; }; }' to type 'ProgrammingExercise' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
🪛 GitHub Check: client-tests-selected
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts
[failure] 128-128:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
[failure] 125-125:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
[failure] 45-45:
Conversion of type '{ id: number; categories: { category: string; }[]; templateParticipation: TemplateProgrammingExerciseParticipation; solutionParticipation: SolutionProgrammingExerciseParticipation; buildConfig: { ...; }; }' to type 'ProgrammingExercise' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: Codacy Static Code Analysis
- 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: client-style
- GitHub Check: server-tests
- GitHub Check: Build .war artifact
- GitHub Check: server-style
- GitHub Check: Analyse
- 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
🔇 Additional comments (8)
src/main/webapp/app/sharing/sharing.component.spec.ts (2)
75-110: LGTM! Comprehensive integration test.This test effectively covers the complete user flow from loading data to navigation, with proper mocking and assertions.
167-202: Excellent validation test coverage.The test thoroughly covers both positive and negative validation scenarios for the SharingInfo model, ensuring proper error handling and validation logic.
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts (6)
1-32: Import structure looks good.The imports are well-organized and include all necessary dependencies for testing the sharing functionality.
92-132: TestBed configuration follows best practices.The test module setup correctly provides all necessary services and mocks for testing the component.
🧰 Tools
🪛 GitHub Check: client-tests
[failure] 128-128:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
[failure] 125-125:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.🪛 GitHub Check: client-tests-selected
[failure] 128-128:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
[failure] 125-125:
Argument of type 'Observable' is not assignable to parameter of type 'ProfileInfo'.
138-151: Test structure follows Angular testing best practices.The test setup correctly uses HttpTestingController for HTTP mocking and includes proper teardown with
httpMock.verify().
152-163: Test case correctly verifies sharing enabled scenario.The test properly mocks the HTTP response and validates that the component sets
isExportToSharingEnabledto true when the API returns success.
165-182: Test case correctly verifies sharing disabled scenario.The test properly simulates a 404 error response and validates that the component sets
isExportToSharingEnabledto false when the sharing API is not available.
33-35: Good decision to separate sharing tests.Creating a dedicated test file for sharing functionality helps maintain clear test organization and can be easily integrated with the main test file when the feature stabilizes.
Michael-Breu-UIbk
force-pushed
the
feature/connecting-to-sharing-platform
branch
from
3bfaede to
3dd4d19
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java (1)
155-158: Potential NPE in finally when temp dir creation failsIf createTempDirectory throws, importExerciseDir stays null and scheduleDirectoryPathForRecursiveDeletion may NPE, masking the original error.
finally { // want to make sure the directories are deleted, even if an exception is thrown - fileService.scheduleDirectoryPathForRecursiveDeletion(importExerciseDir, 5); + if (importExerciseDir != null) { + fileService.scheduleDirectoryPathForRecursiveDeletion(importExerciseDir, 5); + } }
♻️ Duplicate comments (3)
build.gradle (1)
161-162: Use the short Gradle notation for this dependency.Maintainer feedback on an earlier iteration asked for the single-string form. Please switch to
implementation 'org.codeability:SharingPluginPlatformAPI:1.2.1'to stay consistent with the rest of the file and the prior review.src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java (1)
42-58: Add null check for sharingInfo.While the exercise and course are validated, there's no check for
sharingSetupInfo.sharingInfo()before callinggetCachedBasketItem(). This could cause aNullPointerException.Apply this diff:
public ProgrammingExercise importProgrammingExerciseFromSharing(SharingSetupInfo sharingSetupInfo) throws SharingException, IOException, GitAPIException, URISyntaxException { if (sharingSetupInfo.exercise() == null) { throw new SharingException("Exercise should not be null?"); } + if (sharingSetupInfo.sharingInfo() == null) { + throw new SharingException("Sharing info must not be null"); + } try (SharingMultipartZipFile zip = exerciseSharingService.getCachedBasketItem(sharingSetupInfo.sharingInfo())) {Based on coding guidelines
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingMultipartZipFile.java (1)
21-86: Buffer the ZIP content instead of reusing the raw stream
isEmpty()/getSize()callavailable()and every consumer reads the sameInputStream. OncegetBytes()ortransferTo()runs, the stream is exhausted and later callers observe zero bytes, whileavailable()was never a reliable size/emptiness check in the first place. Please read the stream once (e.g. into abyte[]or temp file), base size/empty checks on that buffer, and return a freshInputStreamper call.- private final InputStream inputStream; + private final byte[] data; @@ - this.inputStream = new BufferedInputStream(inputStream); + try { + this.data = inputStream.readAllBytes(); + } + catch (IOException e) { + throw new IllegalStateException("Failed to cache ZIP payload", e); + } @@ - public boolean isEmpty() { - try { - return this.inputStream.available() <= 0; - } - catch (IOException e) { - return true; // unreadable - } - } + public boolean isEmpty() { + return this.data.length == 0; + } @@ - public long getSize() { - try { - return this.inputStream.available(); - } - catch (IOException e) { - return 0; // unreadable - } - } + public long getSize() { + return this.data.length; + } @@ - public byte[] getBytes() throws IOException { - return this.inputStream.readAllBytes(); - } + public byte[] getBytes() { + return this.data.clone(); + } @@ - public InputStream getInputStream() throws IOException { - return this.inputStream; - } + public InputStream getInputStream() { + return new BufferedInputStream(new ByteArrayInputStream(this.data)); + } @@ - public void transferTo(File dest) throws IOException, IllegalStateException { - FileUtils.copyInputStreamToFile(this.inputStream, dest); - } + public void transferTo(File dest) throws IOException { + FileUtils.writeByteArrayToFile(dest, this.data); + } @@ - public void close() throws IOException { - if (this.inputStream != null) { - inputStream.close(); - } - } + public void close() { + // no-op; retained for MultipartFile contract symmetry + }
🧹 Nitpick comments (15)
src/main/webapp/app/programming/manage/exercise/programming-exercise.component.ts (1)
39-39: Remove unused SharingInfo import and provider.
SharingInfois never injected or referenced in this component or its template; delete the import (line 39) and the provider entry (line 43).src/main/webapp/app/sharing/sharing.component.html (1)
87-87: Consider moving static width/height to CSS.While the dynamic
backgroundColorbinding is necessary here, the staticwidth: '15px'andheight: '20px'values could be moved to the component's CSS file to follow the coding guidelines discouraging inline styles.Apply this change to move static dimensions to CSS:
In the component's CSS file, add:
.course-color-indicator { width: 15px; height: 20px; }Then update the template:
-<div [ngStyle]="{ backgroundColor: course.color || ARTEMIS_DEFAULT_COLOR, width: '15px', height: '20px' }"> </div> +<div class="course-color-indicator" [ngStyle]="{ backgroundColor: course.color || ARTEMIS_DEFAULT_COLOR }"> </div>Based on coding guidelines
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (1)
22-26: Consider using ApplicationConfigService for endpoint URLs.While the endpoints are defined as protected readonly properties, using
ApplicationConfigService.getEndpointFor()would make them more robust for deployments with non-root base hrefs or proxies.Based on past comments: This was marked as addressed, but the current code still shows hardcoded paths. If ApplicationConfigService is available in the final version, this comment can be ignored.
src/main/webapp/app/sharing/search-result-dto.model.ts (2)
50-55: Consider removing the "I" prefix from the enum name.The
IExerciseTypeenum uses an "I" prefix, which is typically associated with interfaces in some conventions but is discouraged in modern TypeScript/Angular style guides. If this naming is not mandated by the external API contract, consider renaming it toExerciseType.If the naming is not constrained by the external API:
-export enum IExerciseType { +export enum ExerciseType { COLLECTION = 'collection', PROGRAMMING_EXERCISE = 'programming exercise', EXERCISE = 'exercise', OTHER = 'other', }
67-72: LGTM! Consider adding JSDoc comments.The
MetadataFileDTOinterface correctly uses snake_case for external API compatibility. The same date handling verification suggested forProjectDTOapplies here for theindexing_dateproperty.Consider adding JSDoc comments to all interfaces in this file to improve code documentation and developer experience:
/** * Represents a search result from the sharing platform. */ export interface SearchResultDTO { // ... properties with individual comments } /** * Information about a plugin action that can be performed on a search result. */ export interface PluginActionInfo { // ... properties with individual comments } // ... etc for other interfacessrc/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java (4)
63-66: Normalize Authorization header more robustly (case-insensitive + trim)Handle “Bearer ” prefix ignoring case and trim whitespace before validation.
- final String BEARER_PREFIX = "Bearer "; - final Optional<String> token = sharingApiKey.map(v -> v.startsWith(BEARER_PREFIX) ? v.substring(BEARER_PREFIX.length()) : v); + final Optional<String> token = sharingApiKey + .map(String::trim) + .map(v -> v.regionMatches(true, 0, "Bearer ", 0, 7) ? v.substring(7).trim() : v);
70-78: Prefer StringUtils.isBlank for host checkCovers empty and whitespace-only hosts.
- if (StringUtils.isEmpty(parsedApiBaseUrl.getHost())) { + if (StringUtils.isBlank(parsedApiBaseUrl.getHost())) { log.warn("Rejected config request: apiBaseUrl missing host"); return ResponseEntity.badRequest().build(); }
54-55: Fix minor JavaDoc typo“optinonal” → “optional”
- * @param installationName a descriptive name of the sharing application (optinonal) + * @param installationName a descriptive name of the sharing application (optional)
90-104: Align JavaDoc with actual behavior (404 when profile disabled)Controller is gated by @ConditionalOnProperty; if disabled, the endpoint is absent (404), not 503. When enabled, it always returns 200 with true/false.
- * <li>Return a status 503, if the sharing profile is not enabled.</li> + * <li>Returns 404 if the sharing profile is not enabled (endpoint not exposed).</li> ... - * @return Status 200 and true if a Sharing ApiBaseUrl is present, false, if the Sharing Profile is enabled, however the connection not yet established. In case that sharing is - * not enabled Http-Status 503 is signalled, because - * this resource is not available! + * @return Status 200 with true if a Sharing ApiBaseUrl is present; false if sharing is enabled but not yet connected. If sharing is not enabled, the endpoint is not exposed + * (HTTP 404).src/main/webapp/app/sharing/sharing.component.ts (2)
75-83: Remove redundant loading flag assignmentfinalize already sets coursesLoading to false; the set in next is redundant.
next: (res: HttpResponse<Course[]>) => { this.courses = res.body!; - this.coursesLoading = false; }, error: (error: HttpErrorResponse) => { this.coursesLoading = false; this.alertService.error('artemisApp.sharing.error.loadingCourses'); },
155-157: Consider using DatePipe for localizationUsing Angular’s DatePipe in the template improves i18n consistency over toLocaleString.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java (4)
101-101: Clarify Javadoc for isImportFromSharingGrammar/intent is unclear. Suggest precise wording.
- * @param isImportFromSharing flag whether file import (false) of sharing import + * @param isImportFromSharing whether this is an import from the Sharing platform (true) or a local file upload (false)
104-106: Signature extension looks fine; complete Javadoc with thrown exceptionsAdd missing @throws in the Javadoc to match the method signature.
115-124: Remove redundant pre-extraction directory traversalWalking importExerciseDir before extraction only returns the temp root and changes nothing. It’s misleading; either move after extraction or remove. Recommend removal for simplicity. Based on learnings
- if (isImportFromSharing) { - // Exercises from Sharing are currently exported in a slightly different zip structure containing an additional root dir - try (Stream<Path> walk = Files.walk(importExerciseDir)) { - List<Path> directories = walk.filter(Files::isDirectory).toList(); - if (directories.isEmpty()) { - throw new BadRequestAlertException("No directories found for Sharing import", "programmingExercise", "noSharingDirFound"); - } - importExerciseDir = directories.getFirst(); - } - } + // no special handling needed before extraction; subsequent steps traverse recursively
181-195: Complete Javadoc of overload with all declared exceptionsDocument all thrown exceptions for consistency and IDE/help tooling.
/** * Overloaded method setting the isImportFromSharing flag to false as default * * @param programmingExerciseForImport the programming exercise that should be imported * @param zipFile the zip file that contains the exercise * @param course the course to which the exercise should be added * @param user the user initiating the import * @return the imported programming exercise - * @throws IOException if there is an error reading the file + * @throws IOException if there is an error reading the file + * @throws GitAPIException if Git operations fail + * @throws URISyntaxException if repository URIs are malformed */
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/webapp/app/sharing/sharing.component.ts (2)
71-86: Remove redundantcoursesLoadingassignments inside the subscription callbacks.The
finalizeoperator on line 75 already setscoursesLoading = falseregardless of success or error. The assignments on lines 79 and 82 are redundant.Apply this diff to simplify the code:
loadAll() { this.coursesLoading = true; this.courseService .getCourseOverview({}) .pipe(finalize(() => (this.coursesLoading = false))) .subscribe({ next: (res: HttpResponse<Course[]>) => { this.courses = res.body!; - this.coursesLoading = false; }, error: (error: HttpErrorResponse) => { - this.coursesLoading = false; this.alertService.error('artemisApp.sharing.error.loadingCourses'); }, }); }
49-49: Consider makingselectedCourseexplicitly optional in the type annotation.The property
selectedCourseis not initialized but accessed with optional chaining. While functionally correct, the type annotation doesn't reflect that it can be undefined before user selection.Apply this diff to make the type explicit:
- selectedCourse: Course; + selectedCourse?: Course;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/webapp/app/sharing/sharing.component.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/sharing/sharing.component.ts
🧠 Learnings (6)
📓 Common learnings
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.
📚 Learning: 2025-04-22T10:19:41.546Z
Learnt from: tobias-lippert
PR: ls1intum/Artemis#10714
File: src/main/webapp/app/quiz/manage/create-buttons/quiz-exercise-create-buttons.component.ts:11-15
Timestamp: 2025-04-22T10:19:41.546Z
Learning: In Angular 19, standalone components are the default, so components with an `imports` array don't need to explicitly declare `standalone: true` property in the component decorator.
Applied to files:
src/main/webapp/app/sharing/sharing.component.ts
📚 Learning: 2024-10-28T11:34:03.750Z
Learnt from: isabellagessl
PR: ls1intum/Artemis#9615
File: src/main/webapp/app/overview/exercise-details/course-exercise-details.module.ts:75-75
Timestamp: 2024-10-28T11:34:03.750Z
Learning: In Angular, when a non-standalone component declared in an NgModule needs to use a standalone component, the standalone component should be imported into the NgModule's `imports` array. This allows the non-standalone component to utilize the standalone component in its template.
Applied to files:
src/main/webapp/app/sharing/sharing.component.ts
📚 Learning: 2024-07-09T19:07:38.801Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8858
File: src/main/webapp/app/shared/exercise-filter/exercise-filter-modal.component.ts:185-188
Timestamp: 2024-07-09T19:07:38.801Z
Learning: For the PR ls1intum/Artemis#8858, avoid suggesting moving assignments out of expressions within the `resetFilter` method in `exercise-filter-modal.component.ts`.
Applied to files:
src/main/webapp/app/sharing/sharing.component.ts
📚 Learning: 2024-10-08T18:07:58.425Z
Learnt from: iyannsch
PR: ls1intum/Artemis#9379
File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:364-372
Timestamp: 2024-10-08T18:07:58.425Z
Learning: In the `initTheia` method of `CodeButtonComponent` (`code-button.component.ts`), removing the null check for `this.exercise` inside the `subscribe` callback causes a compile-time error, so it should be kept.
Applied to files:
src/main/webapp/app/sharing/sharing.component.ts
📚 Learning: 2024-09-30T08:34:49.363Z
Learnt from: iyannsch
PR: ls1intum/Artemis#9379
File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:143-170
Timestamp: 2024-09-30T08:34:49.363Z
Learning: In `code-button.component.ts`, `this.exercise.id` is checked before use, so it's acceptable to use the non-null assertion operator `!` on `this.exercise.id!`.
Applied to files:
src/main/webapp/app/sharing/sharing.component.ts
🧬 Code graph analysis (1)
src/main/webapp/app/sharing/sharing.component.ts (4)
src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts (1)
Component(9-71)src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-181)src/main/webapp/app/app.constants.ts (1)
ARTEMIS_DEFAULT_COLOR(19-19)src/main/webapp/app/sharing/sharing.model.ts (1)
ShoppingBasket(50-54)
⏰ 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: bean-instantiation-check
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: client-style
- GitHub Check: server-style
- GitHub Check: Analyse
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
…sharing.enabled (source code and documentation)
Michael-Breu-UIbk
force-pushed
the
feature/connecting-to-sharing-platform
branch
from
accd7b0 to
a5ea1f5
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingServiceTest.java (1)
221-228: Do not replace the mocked instructor authentication
@WithMockUseralready seeds an instructor context, but the manual call toAuthenticationFactory.createUsernamePasswordAuthentication(...)swaps it for an anonymous-only token. That masks authorization checks and makes the test run under the wrong privileges. Drop the manual override (or build an auth that retains the instructor role) so the test exercises the intended security scenario.- SecurityContextHolder.getContext().setAuthentication(AuthenticationFactory.createUsernamePasswordAuthentication(INSTRUCTOR1.toLowerCase()));
🧹 Nitpick comments (1)
build.gradle (1)
161-162: Revert to the short dependency notation.Please follow the earlier maintainer request and switch back to the single-string form:
implementation 'org.codeability:SharingPluginPlatformAPI:1.2.1'This keeps build.gradle consistent with the established style.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/main/resources/config/application-core.ymlis excluded by!**/*.ymlsrc/test/resources/config/application.ymlis excluded by!**/*.yml
📒 Files selected for processing (39)
build.gradle(2 hunks)docs/admin/setup/sharing.rst(1 hunks)docs/user/sharing.rst(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/RestTemplateConfiguration.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/core/dto/SharingInfoDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingException.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingHealthIndicator.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSetupInfo.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java(1 hunks)src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.ts(8 hunks)src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts(1 hunks)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.spec.ts(1 hunks)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts(1 hunks)src/main/webapp/app/programming/manage/update/programming-exercise-update.component.spec.ts(5 hunks)src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts(15 hunks)src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.spec.ts(1 hunks)src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts(1 hunks)src/main/webapp/app/sharing/sharing.component.html(1 hunks)src/main/webapp/app/sharing/sharing.component.spec.ts(1 hunks)src/main/webapp/app/sharing/sharing.component.ts(1 hunks)src/main/webapp/app/sharing/sharing.scss(1 hunks)src/main/webapp/i18n/de/health.json(1 hunks)src/main/webapp/i18n/de/sharing.json(1 hunks)src/main/webapp/i18n/en/health.json(1 hunks)src/main/webapp/i18n/en/sharing.json(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceExportTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingServiceTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorServiceTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingPlatformMockProvider.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSupportResourceTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationIndependentTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (15)
- src/main/java/de/tum/cit/aet/artemis/core/config/RestTemplateConfiguration.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingException.java
- src/main/webapp/app/sharing/sharing.scss
- src/main/webapp/app/sharing/sharing.component.spec.ts
- docs/user/sharing.rst
- docs/admin/setup/sharing.rst
- src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java
- src/main/java/de/tum/cit/aet/artemis/core/dto/SharingInfoDTO.java
- src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.spec.ts
- src/main/webapp/i18n/de/sharing.json
- src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts
- src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java
- src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingPlatformMockProvider.java
- src/main/webapp/i18n/de/health.json
- src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java
🧰 Additional context used
📓 Path-based instructions (4)
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/sharing/SharingSetupInfo.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingHealthIndicator.java
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.spec.tssrc/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.tssrc/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.tssrc/main/webapp/app/programming/manage/update/programming-exercise-update.component.spec.tssrc/main/webapp/app/programming/manage/update/programming-exercise-update.component.tssrc/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.tssrc/main/webapp/app/sharing/sharing.component.ts
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/programming/service/sharing/SharingConnectorServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSupportResourceTest.javasrc/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationIndependentTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceExportTest.java
🧠 Learnings (36)
📓 Common learnings
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.
📚 Learning: 2024-07-02T21:16:24.494Z
Learnt from: JohannesStoehr
PR: ls1intum/Artemis#8947
File: src/main/java/de/tum/in/www1/artemis/service/ExerciseService.java:760-762
Timestamp: 2024-07-02T21:16:24.494Z
Learning: Even when the `NotNull` annotation is used, adding a null check in methods is recommended to catch potential issues early and provide clear error messages.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSetupInfo.java
📚 Learning: 2025-01-28T17:32:42.886Z
Learnt from: magaupp
PR: ls1intum/Artemis#10204
File: src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/sarif/RubocopMessageProcessor.java:13-25
Timestamp: 2025-01-28T17:32:42.886Z
Learning: In Java, parameters without Nullable annotation are implicitly non-null by contract, so adding explicit null checks for such parameters is redundant.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSetupInfo.java
📚 Learning: 2025-09-01T10:20:40.706Z
Learnt from: Michael-Breu-UIbk
PR: ls1intum/Artemis#10989
File: src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts:132-149
Timestamp: 2025-09-01T10:20:40.706Z
Learning: In the Artemis codebase, Angular component test files for ProgrammingExerciseDetailComponent follow a pattern where the component is imported but not explicitly declared in TestBed.configureTestingModule(), yet TestBed.createComponent() still works successfully. This pattern is consistently used across test files like programming-exercise-detail.component.spec.ts and programming-exercise-detail.component.with-sharing.spec.ts.
Applied to files:
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.spec.tssrc/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.tssrc/main/webapp/app/programming/manage/update/programming-exercise-update.component.spec.ts
📚 Learning: 2025-08-18T08:23:16.976Z
Learnt from: Michael-Breu-UIbk
PR: ls1intum/Artemis#10989
File: src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java:287-292
Timestamp: 2025-08-18T08:23:16.976Z
Learning: In Artemis sharing functionality, the sharing-related API endpoints are organized under the `/api/programming/sharing/` path structure, not `/api/sharing/`. The ExerciseSharingResource endpoints like setup-import are correctly accessed via `/api/programming/sharing/setup-import`.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
📚 Learning: 2025-08-11T13:22:05.140Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java:128-131
Timestamp: 2025-08-11T13:22:05.140Z
Learning: In the ExerciseSharingResourceImportTest class in Artemis, mockServer.verify() is not needed in the tearDown method as the test design doesn't require strict verification of all mocked HTTP expectations. The mockServer field is managed differently in this test class.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingServiceTest.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/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingServiceTest.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSupportResourceTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceExportTest.java
📚 Learning: 2024-10-08T15:35:52.595Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8858
File: src/main/webapp/i18n/de/student-dashboard.json:0-0
Timestamp: 2024-10-08T15:35:52.595Z
Learning: Avoid suggesting the phrase "Keine Aufgaben passen zu den gewählten Filtern" for the translation key `noExercisesFoundWithAppliedFilter` in the PR ls1intum/Artemis#8858.
Applied to files:
src/main/webapp/app/sharing/sharing.component.html
📚 Learning: 2024-10-14T10:27:58.500Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8858
File: src/main/webapp/i18n/de/student-dashboard.json:36-37
Timestamp: 2024-10-14T10:27:58.500Z
Learning: Avoid suggesting the translation change for "noExercisesFoundWithAppliedFilter" in the PR ls1intum/Artemis#8858. The preferred translation is "Keine Aufgaben passen zu den gewählten Filtern."
Applied to files:
src/main/webapp/app/sharing/sharing.component.html
📚 Learning: 2024-07-09T10:08:14.161Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8858
File: src/main/webapp/app/shared/sidebar/sidebar.component.html:18-22
Timestamp: 2024-07-09T10:08:14.161Z
Learning: Translation keys `artemisApp.courseOverview.general.noExercisesFoundWithAppliedFilter` and `artemisApp.courseOverview.general.noDataFound` should be added to the relevant translation files to ensure consistent translations across the application.
Applied to files:
src/main/webapp/app/sharing/sharing.component.html
📚 Learning: 2024-07-09T19:04:37.833Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8858
File: src/main/webapp/app/shared/sidebar/sidebar.component.html:24-28
Timestamp: 2024-07-09T19:04:37.833Z
Learning: The translation keys `artemisApp.courseOverview.general.noExercisesFoundWithAppliedFilter` and `artemisApp.courseOverview.general.noDataFound` should be added to the relevant translation files, even if they are present in the HTML files.
Applied to files:
src/main/webapp/app/sharing/sharing.component.html
📚 Learning: 2025-06-10T12:26:42.449Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java:108-117
Timestamp: 2025-06-10T12:26:42.449Z
Learning: In ProgrammingExerciseImportFromFileService, the current directory traversal logic for sharing imports (walking importExerciseDir before extraction) is working correctly in practice and should not be changed to more complex solutions, even if there are theoretical issues with the approach.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java
📚 Learning: 2025-08-10T18:33:22.476Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/test/java/de/tum/cit/aet/artemis/programming/AbstractProgrammingIntegrationLocalCILocalVCTest.java:3-9
Timestamp: 2025-08-10T18:33:22.476Z
Learning: In the Artemis test framework, `MockitoBean` from `org.springframework.test.context.bean.override.mockito` is the standard annotation used for mocking beans in test classes, not `MockBean`. This is used consistently across test base classes like `AbstractProgrammingIntegrationLocalCILocalVCTest`, `AbstractSpringIntegrationIndependentTest`, and `AbstractSpringIntegrationLocalVCSamlTest`. The project also uses `MockitoSpyBean` from the same package.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSupportResourceTest.javasrc/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationIndependentTest.java
📚 Learning: 2025-04-11T07:12:15.912Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10593
File: src/main/java/de/tum/cit/aet/artemis/sharing/SharingMultipartZipFile.java:60-68
Timestamp: 2025-04-11T07:12:15.912Z
Learning: The SharingMultipartZipFile class in the sharing package is a dummy implementation that's not actually used in practice, so optimizations aren't necessary.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
📚 Learning: 2024-10-28T19:39:06.330Z
Learnt from: magaupp
PR: ls1intum/Artemis#9609
File: src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/sarif/SarifParser.java:25-26
Timestamp: 2024-10-28T19:39:06.330Z
Learning: In the `SarifParser` class, tests do not require injecting a different `ObjectMapper`, so it's acceptable to instantiate `ObjectMapper` within the class rather than injecting it via constructor.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
📚 Learning: 2025-08-10T18:34:23.185Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java:334-336
Timestamp: 2025-08-10T18:34:23.185Z
Learning: In the Artemis sharing platform integration, export tokens in ExerciseSharingService don't require OS-independent path normalization because the system doesn't switch between different operating systems. The tokens are generated and consumed on the same OS type.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
📚 Learning: 2024-10-08T15:35:42.972Z
Learnt from: julian-christl
PR: ls1intum/Artemis#7829
File: src/main/java/de/tum/in/www1/artemis/web/rest/ComplaintResponseResource.java:0-0
Timestamp: 2024-10-08T15:35:42.972Z
Learning: The user has fixed the issue regarding the redundant wildcard import in `ComplaintResponseResource.java` by removing it in commit 7e392e0.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
📚 Learning: 2025-08-10T18:26:40.529Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:186-192
Timestamp: 2025-08-10T18:26:40.529Z
Learning: In the Artemis sharing platform integration (ExerciseSharingResource.java), Wallenstein61 prefers to log invalid security tokens (HMAC tokens from the `sec` parameter) in warning messages for debugging and problem analysis purposes. They consider this acceptable since: 1) the tokens come from the user's browser and are already visible in URLs, 2) only invalid tokens are logged (already rejected by the system), and 3) it aids in troubleshooting authentication issues. The actual sensitive information is the shared secret key, not the HMAC tokens themselves.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
📚 Learning: 2025-08-10T18:27:53.012Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:199-202
Timestamp: 2025-08-10T18:27:53.012Z
Learning: The `/api/programming/sharing/export/{token}` endpoint in ExerciseSharingResource is designed for programmatic consumption by the sharing platform's REST client, not for browser downloads. It uses a custom "filename" header instead of Content-Disposition as part of its API contract with the external sharing platform service.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.javasrc/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceExportTest.java
📚 Learning: 2024-07-09T19:07:38.801Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8858
File: src/main/webapp/app/shared/exercise-filter/exercise-filter-modal.component.ts:185-188
Timestamp: 2024-07-09T19:07:38.801Z
Learning: For the PR ls1intum/Artemis#8858, avoid suggesting moving assignments out of expressions within the `resetFilter` method in `exercise-filter-modal.component.ts`.
Applied to files:
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.tssrc/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.tssrc/main/webapp/app/sharing/sharing.component.ts
📚 Learning: 2025-08-21T14:23:55.245Z
Learnt from: Michael-Breu-UIbk
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:109-116
Timestamp: 2025-08-21T14:23:55.245Z
Learning: In Artemis sharing platform integration, basketToken parameters are random UUIDs with sufficient entropy that they don't need to be included in checksum validation. The checksum validation for sharing endpoints focuses on returnURL and apiBaseURL parameters, and including basketToken is considered unnecessary due to the token's random nature making forgery very unlikely.
Applied to files:
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.tssrc/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
📚 Learning: 2025-06-09T10:07:29.838Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#10979
File: src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.ts:101-102
Timestamp: 2025-06-09T10:07:29.838Z
Learning: Angular templates support accessing protected class members since Angular 14. Properties used in template bindings do not need to be public - protected visibility is sufficient and often preferred for better encapsulation.
Applied to files:
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts
📚 Learning: 2025-04-01T17:19:55.677Z
Learnt from: tobias-lippert
PR: ls1intum/Artemis#10610
File: src/test/javascript/spec/service/guided-tour.service.spec.ts:193-193
Timestamp: 2025-04-01T17:19:55.677Z
Learning: In the guide tour service tests, the `router.navigateByUrl` mock should remain synchronous (returning boolean) rather than returning a Promise to maintain compatibility with existing test logic that depends on synchronous behavior.
Applied to files:
src/main/webapp/app/programming/manage/update/programming-exercise-update.component.spec.ts
📚 Learning: 2025-01-15T14:01:15.537Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#9909
File: src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.ts:757-766
Timestamp: 2025-01-15T14:01:15.537Z
Learning: When suggesting RxJS operators like switchMap, avoid claiming they improve readability as this is subjective and depends on the developer's familiarity with reactive programming patterns. Focus instead on technical benefits like better error handling and subscription management if recommending such patterns.
Applied to files:
src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts
📚 Learning: 2025-06-06T14:12:04.570Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts:1283-1303
Timestamp: 2025-06-06T14:12:04.570Z
Learning: Wallenstein61 prefers to minimize interference with original code when implementing new features, even if it means not following optimal patterns like avoiding nested subscriptions. This approach prioritizes compatibility and reduced risk over strict adherence to best practices in large established codebases.
Applied to files:
src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts
📚 Learning: 2025-06-06T14:45:29.888Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts:947-971
Timestamp: 2025-06-06T14:45:29.888Z
Learning: When introducing new features in large PRs, developers may intentionally choose not to refactor existing code to minimize impact on the source codebase, prioritize stability, and keep changes focused and reviewable. This is a valid engineering decision that should be respected.
Applied to files:
src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts
📚 Learning: 2025-06-10T12:30:58.714Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts:29-29
Timestamp: 2025-06-10T12:30:58.714Z
Learning: The sharingTab property and popup window logic in ProgrammingExerciseInstructorExerciseSharingComponent is an open task for implementing "return to artemis window" functionality, planned to be resolved in a future PR.
Applied to files:
src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts
📚 Learning: 2025-06-10T12:33:55.645Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts:57-61
Timestamp: 2025-06-10T12:33:55.645Z
Learning: In the Artemis sharing platform integration, redirect URLs returned from the sharing service are generated by Artemis's own backend and secured with checksums, making them safe from open redirect vulnerabilities. The sharing platform integration uses a secure communication model with API keys and checksum validation.
Applied to files:
src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts
📚 Learning: 2024-10-08T15:35:42.972Z
Learnt from: iyannsch
PR: ls1intum/Artemis#9379
File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:143-170
Timestamp: 2024-10-08T15:35:42.972Z
Learning: In `code-button.component.ts`, `this.exercise.id` is checked before use, so it's acceptable to use the non-null assertion operator `!` on `this.exercise.id!`.
Applied to files:
src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.tssrc/main/webapp/app/sharing/sharing.component.ts
📚 Learning: 2024-10-20T21:59:11.630Z
Learnt from: pzdr7
PR: ls1intum/Artemis#9505
File: src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.html:9-9
Timestamp: 2024-10-20T21:59:11.630Z
Learning: In Angular templates within the Artemis project (e.g., `src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.html`), properties like `selectedFile()`, `readOnlyManualFeedback()`, `highlightDifferences()`, and `course()` are signals. It is appropriate to call these signals directly in the template.
Applied to files:
src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts
📚 Learning: 2024-10-08T18:07:58.425Z
Learnt from: iyannsch
PR: ls1intum/Artemis#9379
File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:364-372
Timestamp: 2024-10-08T18:07:58.425Z
Learning: In the `initTheia` method of `CodeButtonComponent` (`code-button.component.ts`), removing the null check for `this.exercise` inside the `subscribe` callback causes a compile-time error, so it should be kept.
Applied to files:
src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.tssrc/main/webapp/app/sharing/sharing.component.ts
📚 Learning: 2025-04-22T10:19:41.546Z
Learnt from: tobias-lippert
PR: ls1intum/Artemis#10714
File: src/main/webapp/app/quiz/manage/create-buttons/quiz-exercise-create-buttons.component.ts:11-15
Timestamp: 2025-04-22T10:19:41.546Z
Learning: In Angular 19, standalone components are the default, so components with an `imports` array don't need to explicitly declare `standalone: true` property in the component decorator.
Applied to files:
src/main/webapp/app/sharing/sharing.component.ts
📚 Learning: 2024-10-28T11:34:03.750Z
Learnt from: isabellagessl
PR: ls1intum/Artemis#9615
File: src/main/webapp/app/overview/exercise-details/course-exercise-details.module.ts:75-75
Timestamp: 2024-10-28T11:34:03.750Z
Learning: In Angular, when a non-standalone component declared in an NgModule needs to use a standalone component, the standalone component should be imported into the NgModule's `imports` array. This allows the non-standalone component to utilize the standalone component in its template.
Applied to files:
src/main/webapp/app/sharing/sharing.component.ts
📚 Learning: 2025-06-06T15:11:12.114Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java:61-89
Timestamp: 2025-06-06T15:11:12.114Z
Learning: In health monitoring components like HealthStatusWithHistory in SharingConnectorService, it's acceptable to have unsynchronized reader methods (like getLastConnect()) when approximate/eventually consistent reads are sufficient for the use case. The performance benefit of avoiding synchronization overhead outweighs strict consistency requirements for monitoring data.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingHealthIndicator.java
📚 Learning: 2025-06-15T14:14:33.083Z
Learnt from: tobias-lippert
PR: ls1intum/Artemis#10951
File: src/main/java/de/tum/cit/aet/artemis/lti/config/DistributedStateAuthorizationRequestRepository.java:26-26
Timestamp: 2025-06-15T14:14:33.083Z
Learning: In the Artemis application, FullStartupEvent is published as the last event in the startup sequence, and lazy beans that need to respond to this event are guaranteed to be instantiated before or during its publication, so EventListener(FullStartupEvent.class) methods on Lazy beans will execute properly.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java
📚 Learning: 2025-08-04T11:27:40.470Z
Learnt from: tobias-lippert
PR: ls1intum/Artemis#11252
File: src/main/java/de/tum/cit/aet/artemis/core/service/telemetry/TelemetryService.java:45-45
Timestamp: 2025-08-04T11:27:40.470Z
Learning: In the Artemis application, there's a custom DeferredEagerBeanInitializer component that forces initialization of all lazy singleton beans after the FullStartupEvent is published. This allows beans to be marked Lazy for performance during startup while ensuring they're still initialized before serving requests. The PostConstruct methods on lazy beans will execute when the DeferredEagerBeanInitializer calls context.getBean(name) on them.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java
🧬 Code graph analysis (15)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingSetupInfo.java (2)
src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-181)src/main/webapp/app/programming/shared/entities/programming-exercise.model.ts (1)
ProgrammingExercise(47-96)
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.spec.ts (4)
src/main/webapp/app/sharing/sharing.model.ts (1)
ShoppingBasket(50-54)src/test/javascript/spec/helpers/mocks/service/mock-account.service.ts (1)
MockAccountService(7-45)src/main/webapp/app/programming/shared/entities/programming-exercise.model.ts (1)
ProgrammingExercise(47-96)src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-181)
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java (4)
src/test/java/de/tum/cit/aet/artemis/programming/AbstractProgrammingIntegrationLocalCILocalVCTest.java (1)
AbstractProgrammingIntegrationLocalCILocalVCTest(31-114)src/main/java/de/tum/cit/aet/artemis/programming/domain/Repository.java (1)
Repository(17-84)src/main/java/de/tum/cit/aet/artemis/programming/domain/VcsRepositoryUri.java (1)
VcsRepositoryUri(11-180)src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCRepositoryUri.java (1)
LocalVCRepositoryUri(17-318)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java (1)
Service(69-415)
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingServiceTest.java (2)
src/test/java/de/tum/cit/aet/artemis/core/authentication/AuthenticationFactory.java (1)
AuthenticationFactory(24-55)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingException.java (1)
SharingException(8-24)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java (3)
src/main/webapp/app/sharing/sharing.model.ts (2)
ShoppingBasket(50-54)validate(37-44)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java (1)
Service(16-59)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java (1)
Service(32-275)
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (3)
src/main/webapp/app/programming/shared/entities/programming-exercise.model.ts (1)
ProgrammingExercise(47-96)src/main/webapp/app/sharing/sharing.model.ts (2)
Injectable(3-45)ShoppingBasket(50-54)src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-181)
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (3)
src/main/webapp/app/sharing/sharing.model.ts (1)
ShoppingBasket(50-54)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingException.java (1)
SharingException(8-24)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (1)
setUpFromSharingImport(58-64)
src/main/webapp/app/programming/manage/update/programming-exercise-update.component.spec.ts (1)
src/main/webapp/app/programming/shared/entities/programming-exercise.model.ts (1)
ProgrammingExercise(47-96)
src/main/webapp/app/programming/manage/update/programming-exercise-update.component.ts (2)
src/main/webapp/app/programming/shared/entities/programming-exercise.model.ts (1)
ProgrammingExercise(47-96)src/main/webapp/app/programming/manage/update/programming-exercise-creation-config.ts (1)
ProgrammingExerciseCreationConfig(7-56)
src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts (1)
src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.ts (1)
Component(72-783)
src/main/webapp/app/sharing/sharing.component.ts (3)
src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-181)src/main/webapp/app/app.constants.ts (1)
ARTEMIS_DEFAULT_COLOR(19-19)src/main/webapp/app/sharing/sharing.model.ts (1)
ShoppingBasket(50-54)
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceExportTest.java (2)
src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-181)src/test/java/de/tum/cit/aet/artemis/programming/AbstractProgrammingIntegrationLocalCILocalVCTest.java (1)
AbstractProgrammingIntegrationLocalCILocalVCTest(31-114)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java (1)
Service(69-415)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingHealthIndicator.java (1)
src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingPlatformMockProvider.java (1)
Component(32-162)
🪛 ast-grep (0.39.5)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
[warning] 346-346: Triple DES (3DES or DESede) is considered deprecated. AES is the recommended cipher. Upgrade to use AES.
Context: Mac.getInstance(algorithm)
Note: [CWE-326]: Inadequate Encryption Strength [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://find-sec-bugs.github.io/bugs.htm#TDES_USAGE
- https://csrc.nist.gov/News/2017/Update-to-Current-Use-and-Deprecation-of-TDEA
(desede-is-deprecated-java)
[warning] 346-346: Use of AES with ECB mode detected. ECB doesn't provide message confidentiality and is not semantically secure so should not be used. Instead, use a strong, secure cipher: Cipher.getInstance("AES/CBC/PKCS7PADDING"). See https://owasp.org/www-community/Using_the_Java_Cryptographic_Extensions for more information.
Context: Mac.getInstance(algorithm)
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
- https://googleprojectzero.blogspot.com/2022/10/rc4-is-still-considered-harmful.html
(use-of-aes-ecb-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: 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-selected
- GitHub Check: server-tests
- GitHub Check: client-tests
- GitHub Check: server-style
- GitHub Check: bean-instantiation-check
- GitHub Check: Analyse
- GitHub Check: Mend Security Check
🔇 Additional comments (1)
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationIndependentTest.java (1)
46-46: Remove explicit Nebula disable is redundant. application.yml already setsnebula.enabled=false, so removingartemis.nebula.enabled=falsehas no effect on test behavior.
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
Outdated
Show resolved
Hide resolved
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.spec.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts
Show resolved
Hide resolved
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts
Show resolved
Hide resolved
...va/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
…sharing.enabled (source code and documentation)
Michael-Breu-UIbk
force-pushed
the
feature/connecting-to-sharing-platform
branch
from
a5ea1f5 to
05023da
Compare
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (1)
1-27: Re-establish ApplicationConfigService-backed endpoints
Hardcoding'api/…'bypasses the configuredSERVER_API_URL, so deployments served under a base href or behind a proxy will call the wrong backend routes. Reuse the project pattern by injectingApplicationConfigServiceand deriving each URL viagetEndpointFor(...)to keep the service environment-agnostic.-import { Injectable, inject } from '@angular/core'; +import { Injectable, inject } from '@angular/core'; +import { ApplicationConfigService } from 'app/core/config/application-config.service'; @@ export class ProgrammingExerciseSharingService { - protected readonly baseSharingConfigUrl = 'api/core/sharing/config'; - protected readonly resourceUrl = 'api/programming/sharing/import'; - protected readonly resourceUrlBasket = 'api/programming/sharing/import/basket/'; - protected readonly resourceUrlExport = 'api/programming/sharing/export'; - protected readonly resourceUrlSetupImport = 'api/programming/sharing/setup-import'; + private readonly applicationConfigService = inject(ApplicationConfigService); + + protected readonly baseSharingConfigUrl = this.applicationConfigService.getEndpointFor('api/core/sharing/config'); + protected readonly resourceUrl = this.applicationConfigService.getEndpointFor('api/programming/sharing/import'); + protected readonly resourceUrlBasket = this.applicationConfigService.getEndpointFor('api/programming/sharing/import/basket/'); + protected readonly resourceUrlExport = this.applicationConfigService.getEndpointFor('api/programming/sharing/export'); + protected readonly resourceUrlSetupImport = this.applicationConfigService.getEndpointFor('api/programming/sharing/setup-import');
🧹 Nitpick comments (3)
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (3)
57-80: Consider making the inner class static and using deleteIfExists.
- The inner class doesn't access outer class state, so it should be
staticto clarify independence and potentially save memory.Files.delete(this.path)throwsNoSuchFileExceptionif the file was already deleted. UsingFiles.deleteIfExists()is more robust and avoids the exception.Apply this diff:
- private static class AutoDeletingFileInputStream extends FileInputStream { + private static final class AutoDeletingFileInputStream extends FileInputStream { private final Path path; private AutoDeletingFileInputStream(@NotNull Path path) throws FileNotFoundException { super(path.toFile()); this.path = path; } @Override public void close() throws IOException { try { super.close(); } finally { try { - Files.delete(this.path); + Files.deleteIfExists(this.path); } catch (IOException e) { log.error("Could not delete temporary file {}", this.path, e); } } } }
122-136: Consider catching checked exceptions and returning structured error responses.The method declares four checked exceptions that will be handled by Spring's default exception handler, resulting in 500 responses with stack traces. This can expose internal implementation details.
Consider wrapping the call in a try-catch and returning appropriate error responses:
@PostMapping("setup-import") @EnforceAtLeastEditor -public ResponseEntity<ProgrammingExercise> setUpFromSharingImport(@RequestBody SharingSetupInfo sharingSetupInfo) - throws GitAPIException, SharingException, IOException, URISyntaxException { - ProgrammingExercise exercise = programmingExerciseImportFromSharingService.importProgrammingExerciseFromSharing(sharingSetupInfo); - return ResponseEntity.ok().body(exercise); +public ResponseEntity<ProgrammingExercise> setUpFromSharingImport(@RequestBody SharingSetupInfo sharingSetupInfo) { + try { + ProgrammingExercise exercise = programmingExerciseImportFromSharingService.importProgrammingExerciseFromSharing(sharingSetupInfo); + return ResponseEntity.ok().body(exercise); + } + catch (GitAPIException | SharingException | IOException | URISyntaxException e) { + log.error("Error importing exercise from sharing platform", e); + return ResponseEntity.internalServerError().build(); + } }
176-202: Consider catching FileNotFoundException for better error handling.The method declares
FileNotFoundException, which will result in a 500 response if the file cannot be opened. Since the file is generated by the application, a missing file likely indicates an internal issue rather than a client error.Consider catching the exception for clearer logging:
@GetMapping(SHARING_EXPORT_RESOURCE_PATH + "/{token}") -public ResponseEntity<Resource> exportExerciseToSharing(@PathVariable("token") String token, @RequestParam("sec") String sec) throws FileNotFoundException { +public ResponseEntity<Resource> exportExerciseToSharing(@PathVariable("token") String token, @RequestParam("sec") String sec) { if (!exerciseSharingService.validate(token, sec)) { log.warn("Security Token {} is not valid", sec); return ResponseEntity.status(401).build(); } Optional<Path> zipFilePath = exerciseSharingService.getExportedExerciseByToken(token); if (zipFilePath.isEmpty()) { return ResponseEntity.notFound().build(); } - InputStreamResource resource = new InputStreamResource(new AutoDeletingFileInputStream(zipFilePath.get())); - - return ResponseEntity.ok().contentLength(zipFilePath.get().toFile().length()).contentType(MediaType.APPLICATION_OCTET_STREAM) - .header("filename", zipFilePath.get().toString()).body(resource); + try { + InputStreamResource resource = new InputStreamResource(new AutoDeletingFileInputStream(zipFilePath.get())); + return ResponseEntity.ok().contentLength(zipFilePath.get().toFile().length()).contentType(MediaType.APPLICATION_OCTET_STREAM) + .header("filename", zipFilePath.get().toString()).body(resource); + } + catch (FileNotFoundException e) { + log.error("Exported file not found for token {}", token, e); + return ResponseEntity.internalServerError().build(); + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
build.gradle(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java(1 hunks)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.spec.ts(1 hunks)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.spec.ts
- build.gradle
🧰 Additional context used
📓 Path-based instructions (2)
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/web/ExerciseSharingResource.java
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts
🧠 Learnings (8)
📓 Common learnings
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.
📚 Learning: 2025-08-18T08:23:16.976Z
Learnt from: Michael-Breu-UIbk
PR: ls1intum/Artemis#10989
File: src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java:287-292
Timestamp: 2025-08-18T08:23:16.976Z
Learning: In Artemis sharing functionality, the sharing-related API endpoints are organized under the `/api/programming/sharing/` path structure, not `/api/sharing/`. The ExerciseSharingResource endpoints like setup-import are correctly accessed via `/api/programming/sharing/setup-import`.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.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/programming/web/ExerciseSharingResource.java
📚 Learning: 2025-08-10T18:27:53.012Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:199-202
Timestamp: 2025-08-10T18:27:53.012Z
Learning: The `/api/programming/sharing/export/{token}` endpoint in ExerciseSharingResource is designed for programmatic consumption by the sharing platform's REST client, not for browser downloads. It uses a custom "filename" header instead of Content-Disposition as part of its API contract with the external sharing platform service.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
📚 Learning: 2025-08-10T18:26:40.529Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:186-192
Timestamp: 2025-08-10T18:26:40.529Z
Learning: In the Artemis sharing platform integration (ExerciseSharingResource.java), Wallenstein61 prefers to log invalid security tokens (HMAC tokens from the `sec` parameter) in warning messages for debugging and problem analysis purposes. They consider this acceptable since: 1) the tokens come from the user's browser and are already visible in URLs, 2) only invalid tokens are logged (already rejected by the system), and 3) it aids in troubleshooting authentication issues. The actual sensitive information is the shared secret key, not the HMAC tokens themselves.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
📚 Learning: 2025-08-21T14:23:55.245Z
Learnt from: Michael-Breu-UIbk
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:109-116
Timestamp: 2025-08-21T14:23:55.245Z
Learning: In Artemis sharing platform integration, basketToken parameters are random UUIDs with sufficient entropy that they don't need to be included in checksum validation. The checksum validation for sharing endpoints focuses on returnURL and apiBaseURL parameters, and including basketToken is considered unnecessary due to the token's random nature making forgery very unlikely.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
📚 Learning: 2024-07-09T19:07:38.801Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8858
File: src/main/webapp/app/shared/exercise-filter/exercise-filter-modal.component.ts:185-188
Timestamp: 2024-07-09T19:07:38.801Z
Learning: For the PR ls1intum/Artemis#8858, avoid suggesting moving assignments out of expressions within the `resetFilter` method in `exercise-filter-modal.component.ts`.
Applied to files:
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts
📚 Learning: 2025-06-09T10:07:29.838Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#10979
File: src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.ts:101-102
Timestamp: 2025-06-09T10:07:29.838Z
Learning: Angular templates support accessing protected class members since Angular 14. Properties used in template bindings do not need to be public - protected visibility is sufficient and often preferred for better encapsulation.
Applied to files:
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts
🧬 Code graph analysis (2)
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (5)
src/main/webapp/app/sharing/sharing.model.ts (2)
ShoppingBasket(50-54)validate(37-44)src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java (1)
RestController(27-109)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingException.java (1)
SharingException(8-24)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (1)
setUpFromSharingImport(58-64)src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts (1)
exportExerciseToSharing(43-70)
src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (3)
src/main/webapp/app/programming/shared/entities/programming-exercise.model.ts (1)
ProgrammingExercise(47-96)src/main/webapp/app/sharing/sharing.model.ts (2)
Injectable(3-45)ShoppingBasket(50-54)src/main/webapp/app/core/course/shared/entities/course.model.ts (1)
Course(59-181)
⏰ 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/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-style
- GitHub Check: server-tests
- GitHub Check: bean-instantiation-check
- GitHub Check: Analyse
🔇 Additional comments (4)
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (4)
99-120: LGTM!The endpoint correctly validates the checksum before loading the basket and returns appropriate status codes (400 for validation failure, 404 if basket not found).
138-152: LGTM!The endpoint properly validates the checksum before retrieving exercise details and returns appropriate responses.
154-174: LGTM!The endpoint properly handles the export flow with appropriate error handling that doesn't expose internal details to the client.
82-97: LGTM!Constructor injection with final fields follows best practices. The constant
SHARING_EXPORT_RESOURCE_PATHprovides clear reusability.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java (1)
263-263: Compile error:ByteArrayOutputStreamhas notoString(Charset)overload.
ByteArrayOutputStreamdoes not have atoString(Charset)method. Convert to byte array first.Apply this diff:
- String entryContent = baos.toString(StandardCharsets.UTF_8); + String entryContent = new String(baos.toByteArray(), StandardCharsets.UTF_8);src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (1)
169-169: Manual JSON quoting causes double-encoding.Returning
"\"" + uriRedirect.toString() + "\""means Spring serializes it as a JSON string containing quote characters, producing double-encoded output like"\"http://...\"". The client then has to parse twice.Either return the plain string (Spring auto-serializes to JSON):
- return ResponseEntity.ok().body("\"" + uriRedirect.toString() + "\""); + return ResponseEntity.ok().body(uriRedirect.toString());Or return a structured DTO/Map for clarity:
- return ResponseEntity.ok().body("\"" + uriRedirect.toString() + "\""); + return ResponseEntity.ok(Map.of("redirectUrl", uriRedirect.toString()));
🧹 Nitpick comments (11)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java (4)
191-197: Improve log message wordingUse a concise message.
- lastHealthStati.add(new HealthStatus("Delivered Sharing Config Status to " + apiBaseUrl)); + lastHealthStati.add(new HealthStatus("Delivered sharing config to " + apiBaseUrl));
213-217: Precompile the Bearer token regexAvoid recompiling the pattern on every call.
- Pattern p = Pattern.compile("^Bearer\\s+(\\S+)$"); - Matcher m = p.matcher(apiKey); + Matcher m = BEARER_PATTERN.matcher(apiKey);Add once near other constants:
private static final Pattern BEARER_PATTERN = Pattern.compile("^Bearer\\s+(\\S+)$");
251-269: Skip reinit when API key is missing to avoid no-op external callsGuard against null/blank key before issuing the request.
void triggerReinit() { if (sharingUrl != null) { + if (sharingApiKey == null || sharingApiKey.isBlank()) { + log.warn("Skipping reinitialization request: missing Sharing API key"); + lastHealthStati.add(new HealthStatus("Skipped reinit: missing Sharing API key")); + return; + } log.info("Requesting reinitialization from Sharing Platform");
271-273: Avoid exposing mutable internal stateReturning the mutable history list allows external mutation. Consider returning an unmodifiable view or a copy, or exposing a read-only DTO.
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java (2)
43-45: Polish error message wording.The error message includes a question mark that makes it sound uncertain. Use a definitive statement instead.
Apply this diff:
- throw new SharingException("Exercise should not be null?"); + throw new SharingException("Exercise must not be null");
44-44: Improve error message clarity.The error message "Exercise should not be null?" contains a question mark, which makes it sound uncertain rather than assertive. This diminishes the clarity of the error.
Apply this diff:
- throw new SharingException("Exercise should not be null?"); + throw new SharingException("Exercise must not be null");src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (4)
58-81: LGTM! Minor suggestion: preferdeleteIfExists.The close() method correctly ensures the stream is closed even if deletion fails. Consider using
Files.deleteIfExists()instead ofFiles.delete()for slightly cleaner code (avoids throwing when file already gone).Optional diff:
try { - Files.delete(this.path); + Files.deleteIfExists(this.path); } catch (IOException e) {
201-202: Avoid exposing full server path in header.Line 202 uses
zipFilePath.get().toString()which exposes the complete server filesystem path. Even for internal/programmatic consumption, revealing internal paths is discouraged. Use only the filename.Apply this diff:
return ResponseEntity.ok().contentLength(zipFilePath.get().toFile().length()).contentType(MediaType.APPLICATION_OCTET_STREAM) - .header("filename", zipFilePath.get().toString()).body(resource); + .header("filename", zipFilePath.get().getFileName().toString()).body(resource);Based on learnings
169-169: Manual JSON string wrapping is fragile.The response body manually wraps the URI in quotes to create a JSON string. This is error-prone and can break if the URI contains special characters that need escaping.
Consider returning the URI directly and letting Spring's JSON serialization handle it, or use a proper DTO/Map:
- return ResponseEntity.ok().body("\"" + uriRedirect.toString() + "\""); + return ResponseEntity.ok().body(Map.of("sharingUrl", uriRedirect.toString()));Alternatively, if the client expects a plain string (not JSON), return without the quotes:
- return ResponseEntity.ok().body("\"" + uriRedirect.toString() + "\""); + return ResponseEntity.ok().body(uriRedirect.toString());
202-202: Filename header exposes full server path.While the "filename" header is intentional for the API contract with the sharing platform REST client, exposing the full path (
zipFilePath.get().toString()) reveals internal filesystem structure. Use only the filename portion.Based on learnings
Apply this diff:
- .header("filename", zipFilePath.get().toString()).body(resource); + .header("filename", zipFilePath.get().getFileName().toString()).body(resource);src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java (1)
79-79: Make constant private.The constant
MAX_EXPORT_TOKEN_LENGTHis only used within this class and tests. Consider using package-private visibility for better encapsulation if test access is needed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/de/tum/cit/aet/artemis/core/config/ArtemisConfigHelper.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/RestTemplateConfiguration.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingEnabled.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingHealthIndicator.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/de/tum/cit/aet/artemis/core/config/RestTemplateConfiguration.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingHealthIndicator.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/programming/service/sharing/SharingEnabled.javasrc/main/java/de/tum/cit/aet/artemis/core/config/ArtemisConfigHelper.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.javasrc/main/java/de/tum/cit/aet/artemis/core/config/Constants.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.javasrc/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java
🧠 Learnings (15)
📓 Common learnings
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.
📚 Learning: 2025-08-18T08:23:16.976Z
Learnt from: Michael-Breu-UIbk
PR: ls1intum/Artemis#10989
File: src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java:287-292
Timestamp: 2025-08-18T08:23:16.976Z
Learning: In Artemis sharing functionality, the sharing-related API endpoints are organized under the `/api/programming/sharing/` path structure, not `/api/sharing/`. The ExerciseSharingResource endpoints like setup-import are correctly accessed via `/api/programming/sharing/setup-import`.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.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/programming/web/ExerciseSharingResource.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
📚 Learning: 2025-08-10T18:27:53.012Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:199-202
Timestamp: 2025-08-10T18:27:53.012Z
Learning: The `/api/programming/sharing/export/{token}` endpoint in ExerciseSharingResource is designed for programmatic consumption by the sharing platform's REST client, not for browser downloads. It uses a custom "filename" header instead of Content-Disposition as part of its API contract with the external sharing platform service.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
📚 Learning: 2025-08-10T18:26:40.529Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:186-192
Timestamp: 2025-08-10T18:26:40.529Z
Learning: In the Artemis sharing platform integration (ExerciseSharingResource.java), Wallenstein61 prefers to log invalid security tokens (HMAC tokens from the `sec` parameter) in warning messages for debugging and problem analysis purposes. They consider this acceptable since: 1) the tokens come from the user's browser and are already visible in URLs, 2) only invalid tokens are logged (already rejected by the system), and 3) it aids in troubleshooting authentication issues. The actual sensitive information is the shared secret key, not the HMAC tokens themselves.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
📚 Learning: 2025-08-21T14:23:55.245Z
Learnt from: Michael-Breu-UIbk
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:109-116
Timestamp: 2025-08-21T14:23:55.245Z
Learning: In Artemis sharing platform integration, basketToken parameters are random UUIDs with sufficient entropy that they don't need to be included in checksum validation. The checksum validation for sharing endpoints focuses on returnURL and apiBaseURL parameters, and including basketToken is considered unnecessary due to the token's random nature making forgery very unlikely.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
📚 Learning: 2025-06-10T12:26:42.449Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java:108-117
Timestamp: 2025-06-10T12:26:42.449Z
Learning: In ProgrammingExerciseImportFromFileService, the current directory traversal logic for sharing imports (walking importExerciseDir before extraction) is working correctly in practice and should not be changed to more complex solutions, even if there are theoretical issues with the approach.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java
📚 Learning: 2025-06-06T15:11:12.114Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java:61-89
Timestamp: 2025-06-06T15:11:12.114Z
Learning: In health monitoring components like HealthStatusWithHistory in SharingConnectorService, it's acceptable to have unsynchronized reader methods (like getLastConnect()) when approximate/eventually consistent reads are sufficient for the use case. The performance benefit of avoiding synchronization overhead outweighs strict consistency requirements for monitoring data.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java
📚 Learning: 2025-06-15T14:14:33.083Z
Learnt from: tobias-lippert
PR: ls1intum/Artemis#10951
File: src/main/java/de/tum/cit/aet/artemis/lti/config/DistributedStateAuthorizationRequestRepository.java:26-26
Timestamp: 2025-06-15T14:14:33.083Z
Learning: In the Artemis application, FullStartupEvent is published as the last event in the startup sequence, and lazy beans that need to respond to this event are guaranteed to be instantiated before or during its publication, so EventListener(FullStartupEvent.class) methods on Lazy beans will execute properly.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java
📚 Learning: 2025-08-04T11:27:40.470Z
Learnt from: tobias-lippert
PR: ls1intum/Artemis#11252
File: src/main/java/de/tum/cit/aet/artemis/core/service/telemetry/TelemetryService.java:45-45
Timestamp: 2025-08-04T11:27:40.470Z
Learning: In the Artemis application, there's a custom DeferredEagerBeanInitializer component that forces initialization of all lazy singleton beans after the FullStartupEvent is published. This allows beans to be marked Lazy for performance during startup while ensuring they're still initialized before serving requests. The PostConstruct methods on lazy beans will execute when the DeferredEagerBeanInitializer calls context.getBean(name) on them.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java
📚 Learning: 2025-04-11T07:12:15.912Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10593
File: src/main/java/de/tum/cit/aet/artemis/sharing/SharingMultipartZipFile.java:60-68
Timestamp: 2025-04-11T07:12:15.912Z
Learning: The SharingMultipartZipFile class in the sharing package is a dummy implementation that's not actually used in practice, so optimizations aren't necessary.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
📚 Learning: 2024-10-28T19:39:06.330Z
Learnt from: magaupp
PR: ls1intum/Artemis#9609
File: src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/sarif/SarifParser.java:25-26
Timestamp: 2024-10-28T19:39:06.330Z
Learning: In the `SarifParser` class, tests do not require injecting a different `ObjectMapper`, so it's acceptable to instantiate `ObjectMapper` within the class rather than injecting it via constructor.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
📚 Learning: 2025-08-10T18:34:23.185Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java:334-336
Timestamp: 2025-08-10T18:34:23.185Z
Learning: In the Artemis sharing platform integration, export tokens in ExerciseSharingService don't require OS-independent path normalization because the system doesn't switch between different operating systems. The tokens are generated and consumed on the same OS type.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
📚 Learning: 2024-06-10T19:44:09.116Z
Learnt from: julian-christl
PR: ls1intum/Artemis#7829
File: src/main/java/de/tum/in/www1/artemis/web/rest/ComplaintResponseResource.java:0-0
Timestamp: 2024-06-10T19:44:09.116Z
Learning: The user has fixed the issue regarding the redundant wildcard import in `ComplaintResponseResource.java` by removing it in commit 7e392e0.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
📚 Learning: 2024-06-10T19:44:09.116Z
Learnt from: pzdr7
PR: ls1intum/Artemis#8703
File: src/main/java/de/tum/in/www1/artemis/web/rest/QuizParticipationResource.java:39-39
Timestamp: 2024-06-10T19:44:09.116Z
Learning: The `Profile(PROFILE_CORE)` annotation in `QuizParticipationResource.java` is used to ensure that all nodes, not just those with the scheduling profile, can handle quiz participations.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java
🧬 Code graph analysis (7)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingEnabled.java (2)
src/main/java/de/tum/cit/aet/artemis/core/config/ArtemisConfigHelper.java (1)
ArtemisConfigHelper(17-117)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (1)
isSharingEnabled(128-132)
src/main/java/de/tum/cit/aet/artemis/core/config/ArtemisConfigHelper.java (2)
src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (1)
Constants(8-531)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (1)
isSharingEnabled(128-132)
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (5)
src/main/webapp/app/sharing/sharing.model.ts (2)
ShoppingBasket(50-54)validate(37-44)src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java (1)
RestController(28-110)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingEnabled.java (1)
SharingEnabled(13-25)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (1)
setUpFromSharingImport(58-64)src/main/webapp/app/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts (1)
exportExerciseToSharing(43-70)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java (1)
Service(69-415)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java (1)
Service(69-415)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java (3)
src/main/webapp/app/sharing/sharing.model.ts (1)
ShoppingBasket(50-54)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ProgrammingExerciseImportFromSharingService.java (1)
Service(16-59)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingConnectorService.java (1)
Service(32-275)
src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java (3)
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (1)
RestController(49-205)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingEnabled.java (1)
SharingEnabled(13-25)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (1)
isSharingEnabled(128-132)
🪛 ast-grep (0.39.5)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
[warning] 346-346: Triple DES (3DES or DESede) is considered deprecated. AES is the recommended cipher. Upgrade to use AES.
Context: Mac.getInstance(algorithm)
Note: [CWE-326]: Inadequate Encryption Strength [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://find-sec-bugs.github.io/bugs.htm#TDES_USAGE
- https://csrc.nist.gov/News/2017/Update-to-Current-Use-and-Deprecation-of-TDEA
(desede-is-deprecated-java)
[warning] 346-346: Use of AES with ECB mode detected. ECB doesn't provide message confidentiality and is not semantically secure so should not be used. Instead, use a strong, secure cipher: Cipher.getInstance("AES/CBC/PKCS7PADDING"). See https://owasp.org/www-community/Using_the_Java_Cryptographic_Extensions for more information.
Context: Mac.getInstance(algorithm)
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
- https://googleprojectzero.blogspot.com/2022/10/rc4-is-still-considered-harmful.html
(use-of-aes-ecb-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: 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-tests
- GitHub Check: server-tests
- GitHub Check: server-style
- GitHub Check: client-style
- GitHub Check: Analyse
- GitHub Check: bean-instantiation-check
- GitHub Check: Mend Security Check
🔇 Additional comments (3)
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (1)
58-81: LGTM: AutoDeletingFileInputStream correctly implements resource cleanup.The inner class properly closes the stream in a try-finally block and handles deletion failures gracefully by logging errors. The implementation ensures that both the stream closure and file deletion are attempted even if one fails.
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java (2)
210-212: Package-private test accessor is acceptable.The
getRepositoryCache()method provides test access to the cache, which is a reasonable approach for testing cache behavior. Package-private visibility is appropriate here.
153-163: SharingMultipartZipFile correctly closes its InputStream
The class implements Closeable and its close() method invokes inputStream.close(), so no descriptor leak.
src/main/java/de/tum/cit/aet/artemis/core/config/ArtemisConfigHelper.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingEnabled.java
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (1)
170-174: Consider structured response over manual JSON quoting.Manually wrapping the URI in quotes can lead to double-encoding issues and is less maintainable than using Spring's automatic JSON serialization.
Apply this diff to use a structured response:
- return ResponseEntity.ok().body("\"" + uriRedirect.toString() + "\""); + return ResponseEntity.ok(Map.of("sharingUrl", uriRedirect.toString()));Alternatively, create a dedicated DTO if you need more structure in the future. This ensures proper JSON serialization without manual string manipulation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/de/tum/cit/aet/artemis/core/config/ArtemisConfigHelper.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingEnabled.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java
- src/main/java/de/tum/cit/aet/artemis/core/config/ArtemisConfigHelper.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/programming/service/sharing/SharingEnabled.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.javasrc/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java
🧠 Learnings (7)
📓 Common learnings
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.
📚 Learning: 2025-08-18T08:23:16.976Z
Learnt from: Michael-Breu-UIbk
PR: ls1intum/Artemis#10989
File: src/test/java/de/tum/cit/aet/artemis/programming/service/sharing/ExerciseSharingResourceImportTest.java:287-292
Timestamp: 2025-08-18T08:23:16.976Z
Learning: In Artemis sharing functionality, the sharing-related API endpoints are organized under the `/api/programming/sharing/` path structure, not `/api/sharing/`. The ExerciseSharingResource endpoints like setup-import are correctly accessed via `/api/programming/sharing/setup-import`.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.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/programming/web/ExerciseSharingResource.java
📚 Learning: 2025-08-10T18:27:53.012Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:199-202
Timestamp: 2025-08-10T18:27:53.012Z
Learning: The `/api/programming/sharing/export/{token}` endpoint in ExerciseSharingResource is designed for programmatic consumption by the sharing platform's REST client, not for browser downloads. It uses a custom "filename" header instead of Content-Disposition as part of its API contract with the external sharing platform service.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
📚 Learning: 2025-08-10T18:26:40.529Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:186-192
Timestamp: 2025-08-10T18:26:40.529Z
Learning: In the Artemis sharing platform integration (ExerciseSharingResource.java), Wallenstein61 prefers to log invalid security tokens (HMAC tokens from the `sec` parameter) in warning messages for debugging and problem analysis purposes. They consider this acceptable since: 1) the tokens come from the user's browser and are already visible in URLs, 2) only invalid tokens are logged (already rejected by the system), and 3) it aids in troubleshooting authentication issues. The actual sensitive information is the shared secret key, not the HMAC tokens themselves.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
📚 Learning: 2025-08-21T14:23:55.245Z
Learnt from: Michael-Breu-UIbk
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:109-116
Timestamp: 2025-08-21T14:23:55.245Z
Learning: In Artemis sharing platform integration, basketToken parameters are random UUIDs with sufficient entropy that they don't need to be included in checksum validation. The checksum validation for sharing endpoints focuses on returnURL and apiBaseURL parameters, and including basketToken is considered unnecessary due to the token's random nature making forgery very unlikely.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java
📚 Learning: 2024-06-10T19:44:09.116Z
Learnt from: pzdr7
PR: ls1intum/Artemis#8703
File: src/main/java/de/tum/in/www1/artemis/web/rest/QuizParticipationResource.java:39-39
Timestamp: 2024-06-10T19:44:09.116Z
Learning: The `Profile(PROFILE_CORE)` annotation in `QuizParticipationResource.java` is used to ensure that all nodes, not just those with the scheduling profile, can handle quiz participations.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java
🧬 Code graph analysis (3)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingEnabled.java (2)
src/main/java/de/tum/cit/aet/artemis/core/config/ArtemisConfigHelper.java (1)
ArtemisConfigHelper(17-117)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (1)
isSharingEnabled(128-132)
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (5)
src/main/webapp/app/sharing/sharing.model.ts (2)
ShoppingBasket(50-54)validate(37-44)src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java (1)
RestController(28-109)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingEnabled.java (1)
SharingEnabled(13-25)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingException.java (1)
SharingException(8-24)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (1)
setUpFromSharingImport(58-64)
src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java (3)
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (1)
RestController(49-214)src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingEnabled.java (1)
SharingEnabled(13-25)src/main/webapp/app/programming/manage/services/programming-exercise-sharing.service.ts (1)
isSharingEnabled(128-132)
⏰ 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: server-style
- GitHub Check: server-tests
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: client-tests-selected
- 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: bean-instantiation-check
- GitHub Check: Analyse
🔇 Additional comments (5)
src/main/java/de/tum/cit/aet/artemis/programming/service/sharing/SharingEnabled.java (1)
9-24: LGTM!The implementation correctly follows Spring's Condition pattern for feature gating. Creating
ArtemisConfigHelperin the constructor (rather than via dependency injection) is the appropriate approach, as Condition implementations are instantiated before the Spring context is fully initialized.src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java (2)
61-89: Solid implementation with security safeguards in place.The method correctly handles Bearer token extraction, validates the API key, and includes appropriate URL validation (host presence and http/https scheme checks) to mitigate SSRF risks. The error handling consistently uses
.build()to avoid null response bodies.
105-108: LGTM!The method correctly returns the availability status of the sharing platform connection. The endpoint behavior aligns with the updated Javadoc: it returns HTTP 200 with a boolean, and when the sharing profile is disabled, the controller is not registered (resulting in HTTP 404).
src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java (2)
58-81: Well-implemented cleanup mechanism.The
AutoDeletingFileInputStreamcorrectly handles file deletion with proper exception handling and OS-independent file operations. The try-finally structure ensures the file is deleted even if closing the stream fails, and deletion errors are appropriately logged without masking stream-close exceptions.
182-212: Security implementation correctly follows API contract.The endpoint properly uses custom token validation for authentication, which is appropriate for this public endpoint serving the external sharing platform. The security token validation at line 193, error responses, and file streaming with automatic cleanup are all correctly implemented.
Based on learnings.
| /** | ||
| * GET api/core/sharing/config | ||
| * Returns Sharing Plugin configuration to be used in context with Artemis. | ||
| * This configuration is requested by the sharing platform in general every 10 minutes. | ||
| * It is secured by the common secret api key token transferred by Authorization header. | ||
| * | ||
| * @param sharingApiKey the common secret api key token (transferred by Authorization header). | ||
| * @param apiBaseUrl the base url of the sharing application api (for callbacks) | ||
| * @param installationName a descriptive name of the sharing application (optinonal) | ||
| * | ||
| * @return Sharing Plugin configuration | ||
| * @see <a href="https://sharing-codeability.uibk.ac.at/development/sharing/codeability-sharing-platform/-/wikis/Setup/Connector-Interface-Setup">Connector Interface Setup</a> | ||
| * | ||
| */ |
There was a problem hiding this comment.
Fix typo in Javadoc.
Line 56 has a typo: "optinonal" should be "optional".
Apply this diff:
- * @param installationName a descriptive name of the sharing application (optinonal)
+ * @param installationName a descriptive name of the sharing application (optional)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * GET api/core/sharing/config | |
| * Returns Sharing Plugin configuration to be used in context with Artemis. | |
| * This configuration is requested by the sharing platform in general every 10 minutes. | |
| * It is secured by the common secret api key token transferred by Authorization header. | |
| * | |
| * @param sharingApiKey the common secret api key token (transferred by Authorization header). | |
| * @param apiBaseUrl the base url of the sharing application api (for callbacks) | |
| * @param installationName a descriptive name of the sharing application (optinonal) | |
| * | |
| * @return Sharing Plugin configuration | |
| * @see <a href="https://sharing-codeability.uibk.ac.at/development/sharing/codeability-sharing-platform/-/wikis/Setup/Connector-Interface-Setup">Connector Interface Setup</a> | |
| * | |
| */ | |
| /** | |
| * GET api/core/sharing/config | |
| * Returns Sharing Plugin configuration to be used in context with Artemis. | |
| * This configuration is requested by the sharing platform in general every 10 minutes. | |
| * It is secured by the common secret api key token transferred by Authorization header. | |
| * | |
| * @param sharingApiKey the common secret api key token (transferred by Authorization header). | |
| * @param apiBaseUrl the base url of the sharing application api (for callbacks) | |
| * @param installationName a descriptive name of the sharing application (optional) | |
| * | |
| * @return Sharing Plugin configuration | |
| * @see <a href="https://sharing-codeability.uibk.ac.at/development/sharing/codeability-sharing-platform/-/wikis/Setup/Connector-Interface-Setup">Connector Interface Setup</a> | |
| * | |
| */ |
🤖 Prompt for AI Agents
In src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java
around lines 47 to 60, the Javadoc for the parameter installationName contains a
typo "optinonal"; update that word to "optional" in the @param description so
the Javadoc reads "a descriptive name of the sharing application (optional)".
Ensure no other Javadoc text is changed.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
This pull resolves #9909
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Changes affecting Programming Exercises
Motivation and Context
The CodeAbility Project currently operates and maintains a platform for sharing (among other content) programming exercises between interested parties. It is open to the programming teaching community.
CodeAbility also operates an Artemis instance that serves its 6 Austrian partners as a platform for programming teaching.
Artemis is currently a major source of programming exercises, as well as a potential target for re-using programming exercises.
However the exchange between separated instances of Artemis is only supported by a manual exchange of zipped exercises files.
We have implemented a REST-based connector interface in the sharing platform to import and export exercises between interested applications.
This connector interface allows among other features the import and export of programming exercises in an appropriate format.
We have already implemented an prototype integration of Artemis with the sharing platform based on the connector interface in order to allow for the smooth sharing of exercises.
Description
An additional spring profile "sharing" is implemented, that enables the functionality to export/import programming exercises via the sharing plattform.
The sharing requires an registration of the artemis instance at [email protected]. During the registration a pre shared key is exchanged, that allows the communication between artemis and the sharing plattform.
When the sharing profile is enabled, (and configuration is initialized properly), the the sharing platform connects regularly with the artemis instance to query the connector configuration.
Additional the artemis instance can trigger an configuration request for the sharing plattform at application startup.
Details can be found at https://sharing-codeability.uibk.ac.at/development/sharing/codeability-sharing-platform/-/wikis/Artemis-Connection/Connector-Interface .
A preliminary setup guide can be found at https://development.pages.sharing-codeability.uibk.ac.at/artemis/artemis/admin/setup/sharing.html .
Steps for Testing
Prerequisites:
Follow the steps at https://development.pages.sharing-codeability.uibk.ac.at/artemis/artemis/user/sharing.html for export to Artemis and the import back to artemis.
Exam Mode Testing
This feature is not relevant for exam mode testing. (Just exercise imports)
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.
Review Progress
Performance Review
Preliminary Remark: Import and Export of programming exercises does not influence any performance with student features.
Code Review
Manual Tests
Exam Mode Test
Performance Tests
Test Coverage
Client
Server
See the manual at https://artemis-platform--10989.org.readthedocs.build/en/10989/user/sharing.html ,
Summary by CodeRabbit
New Features
Configuration
Documentation
Localization
Tests