Development: Migrate file URL paths for various entities

[ole-ve]

Do not deploy to test servers because of DB migrations

Checklist

General

• I tested all changes and their related features with all corresponding user types on a test server.
• Language: I followed the guidelines for inclusive, diversity-sensitive, and appreciative language.
• I chose a title conforming to the naming conventions for pull requests.

Server

• Important: I implemented the changes with a very good performance and prevented too many (unnecessary) and too complex database calls.
• I strictly followed the principle of data economy for all database calls.
• I strictly followed the server coding and design guidelines.
• I documented the Java code using JavaDoc style.

Motivation and Context

In #10416, we changed the URL paths to be prefixed by the respective module name.

Since we've adapted the asset paths (that are stored in the DB), existing assets can currently not be loaded after upgrading.

Also, we will remove the API base prefix for files (/api/core/files) from the paths stored literally in the database. Instead, the client has to append the prefix.

Description

Liquibase Migration that updates the paths for all entities that rely on this URL path. Additionally, includes all paths that need to be adapted.

Steps for Testing

Note: Needs to be tested locally.

1. Start Server with a commit before the above PR has been merged (example: 6fc669b)
2. Create the following entities:

• Course (Icon)
• Lecture Attachments - one type is enough
• Quiz Questions (Background Image and Drag-and-Drog pictures)
• User avatars
• Student exam signature & student exam image path
• file upload submissions
• create an exercise and place a static link to https://artemis-test.../api/core/files/testfile.png. the file doesn't have to exist

3. Check out develop, restart the server
4. See that some requests fail like on the course page (course icon). Only applies to previously existing assets, new ones will be saved correctly (even with develop).
5. Checkout this branch and start the server
6. For every created entity, check that opening this entity (includes loading the files in any way from the /api/core/file-endpoint works fine.

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

Code Review

• Code Review 2

Manual Tests

• Test 1

Summary by CodeRabbit

Summary by CodeRabbit

• New Features

  • Introduced an automated migration tool to update API paths in Jenkins job definitions for version upgrades, ensuring a smooth transition and providing a summary of the updates applied.
  • Added support for new file path handling, including public URL generation for various file types and improved attachment link management.
  • Enhanced user profile picture handling across various components by incorporating a public file prefix for image URLs.
  • Introduced new properties for managing file paths and links, enhancing the attachment and course icon functionalities.

• Bug Fixes

  • Enhanced error handling for file path resolution to ensure robustness in attachment and image URL processing.

• Documentation

  • Updated comments and documentation to reflect changes in file path handling and new utility functions.

• Style

  • Improved code readability and consistency in file path management across various components and services.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/test/java/de/tum/cit/aet/artemis/lecture/AttachmentUnitsIntegrationTest.java (1)

296-298: Consider using consistent variable naming pattern.

While the functionality is correct, there's an inconsistency in variable naming between the first attachment unit (using requestUrl) and the second attachment unit (using attachmentRequestUrl). Consider standardizing the variable naming for better readability and maintainability.

-String attachmentPathSecondUnit = attachmentUnitList.get(1).getAttachment().getLink();
-String attachmentRequestUrl = String.format("%s%s", ARTEMIS_FILE_PATH_PREFIX, attachmentPathSecondUnit);
-byte[] fileBytesSecond = request.get(attachmentRequestUrl, HttpStatus.OK, byte[].class);
+String requestUrl = String.format("%s%s", ARTEMIS_FILE_PATH_PREFIX, attachmentUnitList.get(1).getAttachment().getLink());
+byte[] fileBytesSecond = request.get(requestUrl, HttpStatus.OK, byte[].class);

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6052e30 and f2b2c44.

📒 Files selected for processing (5)

• src/test/java/de/tum/cit/aet/artemis/exam/ExamUserIntegrationTest.java (4 hunks)
• src/test/java/de/tum/cit/aet/artemis/fileupload/FileUploadSubmissionIntegrationTest.java (5 hunks)
• src/test/java/de/tum/cit/aet/artemis/lecture/AttachmentResourceIntegrationTest.java (3 hunks)
• src/test/java/de/tum/cit/aet/artemis/lecture/AttachmentUnitsIntegrationTest.java (3 hunks)
• src/test/java/de/tum/cit/aet/artemis/quiz/QuizExerciseIntegrationTest.java (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

• src/test/java/de/tum/cit/aet/artemis/exam/ExamUserIntegrationTest.java
• src/test/java/de/tum/cit/aet/artemis/lecture/AttachmentResourceIntegrationTest.java
• src/test/java/de/tum/cit/aet/artemis/quiz/QuizExerciseIntegrationTest.java

🧰 Additional context used

📓 Path-based instructions (1)

`src/test/java/**/*.java`: test_naming: descriptive; test_si...

src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

• src/test/java/de/tum/cit/aet/artemis/lecture/AttachmentUnitsIntegrationTest.java
• src/test/java/de/tum/cit/aet/artemis/fileupload/FileUploadSubmissionIntegrationTest.java

⏰ Context from checks skipped due to timeout of 90000ms (3)

• GitHub Check: server-style
• GitHub Check: server-tests
• GitHub Check: Analyse

🔇 Additional comments (9)

src/test/java/de/tum/cit/aet/artemis/fileupload/FileUploadSubmissionIntegrationTest.java (6)

3-3: Good addition of the constant import.

The addition of this import is aligned with the PR objective of migrating file URL paths. This constant will be used for constructing proper request URLs.

---

144-144: File path format correctly updated.

The file path has been correctly updated to no longer include the API endpoint prefix ("/api/core/files/"). This change is consistent with the migration described in the PR objective where file paths are now stored without the prefix.

---

172-173: URL construction pattern properly implemented.

The new code correctly uses the ARTEMIS_FILE_PATH_PREFIX constant to construct the full request URL at runtime, which is a more maintainable approach. This separates the storage format from the API endpoint format, making the system more flexible.

---

696-696: Consistent file path update in test case.

The file path format has been correctly updated in the test case, removing the API endpoint prefix ("/api/core/files/"). This ensures consistency with the new approach to file path handling.

---

705-705: Consistent file path update in test case.

The file path format has been correctly updated in the test case, removing the API endpoint prefix ("/api/core/files/"). This ensures consistency with the new approach to file path handling.

---

1-710:

✅ Verification successful

Verify all instances of file path updates.

The code changes look good and follow a consistent pattern of removing the API endpoint prefix from stored file paths and adding it at request time. However, it's important to verify that all instances where file paths are constructed follow this pattern across the entire PR.

---

🏁 Script executed:

#!/bin/bash
# Description: Search for any remaining instances of hardcoded "/api/core/files/" in file paths

# Search for hardcoded API path prefixes in Java files
echo "Searching for potential missed file path prefixes:"
rg -i "/api/core/files/" --type java

Length of output: 2281

---

Verified: File Path Updates Are Consistent

After running the search, the only occurrences of the hardcoded "/api/core/files/" string appear in the constant definition and in tests that explicitly reference the API endpoint. In the main code (especially in file upload-related logic), file paths are built without the prefix and are correctly prefixed at request time using the constant (ARTEMIS_FILE_PATH_PREFIX). This indicates that the changes follow the intended pattern across the PR.

• File paths in production (e.g., in the file upload submission integration tests) correctly remove the prefix when stored and add it upon constructing the request URL.
• Remaining hardcoded occurrences in tests and in Constants.java are expected and appropriate.

src/test/java/de/tum/cit/aet/artemis/lecture/AttachmentUnitsIntegrationTest.java (3)

3-3: Proper import of file path constant.

The import of the ARTEMIS_FILE_PATH_PREFIX constant from Constants.java is correctly added, ensuring consistency with the API path migration described in the PR objectives.

---

287-288: Correct URL path construction for first attachment unit.

The URL path is now properly constructed using the ARTEMIS_FILE_PATH_PREFIX constant, aligning with the migration of API paths mentioned in the PR objectives. This change ensures that previously existing assets can be loaded correctly after the upgrade.

---

285-303:

❓ Verification inconclusive

Verify attachment file loading works across all affected modules.

The test confirms that files can be loaded with the new URL path structure, but it might be worth ensuring that this works consistently across all modules that use file attachments. Consider adding verification for edge cases like file names with special characters.

---

🏁 Script executed:

#!/bin/bash
# Description: Check all file path references in the codebase to ensure consistent use of ARTEMIS_FILE_PATH_PREFIX

# Look for direct file path references that might need the prefix
echo "Finding direct file path patterns that might need updating:"
rg -l "getAttachment\(\).getLink\(\)" --type java | grep -v "AttachmentUnitsIntegrationTest.java"

# Check for uses of attachment links without the ARTEMIS_FILE_PATH_PREFIX
echo "Finding potential file path references without the prefix:"
rg -l "getAttachment\(\).getLink\(\)" --type java | xargs rg -l "request.get\(" | grep -v "AttachmentUnitsIntegrationTest.java" | xargs rg -A 3 "request.get\(.*getAttachment\(\).getLink\(\)"

# Check if prefix is correctly defined
echo "Verifying ARTEMIS_FILE_PATH_PREFIX definition:"
rg "ARTEMIS_FILE_PATH_PREFIX.*=" --type java

Length of output: 1364

---

Action Required: Ensure Consistent Attachment Loading Across Modules and Test Edge Cases

• The ARTEMIS_FILE_PATH_PREFIX is correctly defined in src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java and is used across multiple modules (e.g., FileResource, PyrisWebhookService, SlideSplitterService, LectureUnitService, AttachmentUnitService).
• The AttachmentUnitsIntegrationTest properly verifies file loading with the new URL structure by checking PDF page counts.
• Please review the attachment handling in the referenced modules to confirm that the new path construction works as expected in all cases.
• Consider adding tests—especially for file names with special characters—to ensure that edge cases are handled consistently.

[b-fein]

code reapprove

[ole-ve]

server test coverage thresholds are broken on develop, these changes have no influence on the (exam-)coverage