Development: Improve git URI handling for LocalVC

[Hialus]

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) database calls.
• I strictly followed the server coding and design guidelines.
• I added multiple integration tests (Spring) related to the features (with a high test coverage).
• I added pre-authorization annotations according to the guidelines and checked the course groups for all new REST Calls (security).
• I documented the Java code using JavaDoc style.

Changes affecting Programming Exercises

• High priority: I tested all changes and their related features with all corresponding user types on a test server configured with the integrated lifecycle setup (LocalVC and LocalCI).

Motivation and Context

Currently the LocalVCRepositoryUri to create it from a URL is not very well done:

• It limits the accepted URL to a single host -> This causes issues in local testing and is also not sensible in production, where we have multiple hostnames
• There were certain other improvements that could be made to improve the code quality

Description

I removed the check and improved the code.
Added JavaDoc and tests to make sure the URI handling is correct

Steps for Testing

Prerequisites:

• 1 Instructor
• ICL test server

1. Open the code editor somewhere and submit
2. Clone the repo locally and commit & push

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked

Review Progress

Performance Review

Code Review

• Code Review 1
• Code Review 2

Manual Tests

• Test 1
• Test 2

Summary by CodeRabbit

• Refactor
  • Enhanced the VcsRepositoryUri class with new methods for manipulating and extracting information from VCS repository URIs.
  • Modified the LocalVCRepositoryUri constructor for improved URL parsing, validation, and error handling.
• Tests
  • Introduced test cases in RepositoryUriTests for parsing and validating various types of repository URIs.

[coderabbitai]

Walkthrough

The recent changes in the Artemis project focus on enhancing URI management by simplifying the constructor of LocalVCRepositoryUri. The update involves removing the localVCBaseUrl parameter, which improves URI handling, error management, and validation processes.

Changes

Files	Change Summary
.../domain/VcsRepositoryUri.java	Enhanced VcsRepositoryUri with new methods for manipulating and extracting information from VCS repository URIs. Added methods for setting a username, extracting repository names, project keys, and repository slugs.
.../service/connectors/localvc/LocalVCRepositoryUri.java	Modified LocalVCRepositoryUri constructor to handle URL parsing and validation differently. Removed localVCBaseUrl parameter, updated URI creation, path parsing logic, and improved error handling.
.../uri/RepositoryUriTests.java	Introduced test cases for parsing and validating various types of repository URIs, covering scenarios like handling valid and invalid URIs, extracting project keys, repository slugs, and checking URI segments.
.../architecture/ArchitectureTest.java	Reordered and modified test methods in ArchitectureTest.java. Split testNoJUnit4 method into testNoJUnit4 and testClassNameAndVisibility. Moved logic related to public tests to testClassNameAndVisibility.
.../uri/RepositoryUriTest.java	Added functionality for testing parsing and validation of repository URIs, including local and remote paths, URLs, 'git' suffix, and malformed URIs. Tests cover constructing valid URIs, handling practice repositories, extracting repository details, file URIs, equality checks, and repository name extraction.

---

Recent Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 536c7f8 and 8732003.

Files selected for processing (2)

• src/test/java/de/tum/in/www1/artemis/architecture/ArchitectureTest.java (1 hunks)
• src/test/java/de/tum/in/www1/artemis/uri/RepositoryUriTest.java (1 hunks)

Additional Context Used

Path-based Instructions (2)

src/test/java/de/tum/in/www1/artemis/uri/RepositoryUriTest.java (1)

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

src/test/java/de/tum/in/www1/artemis/architecture/ArchitectureTest.java (1)

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

Additional comments not posted (27)

src/test/java/de/tum/in/www1/artemis/uri/RepositoryUriTest.java (8)

24-38: Ensure comprehensive testing of the new LocalVCRepositoryUri constructor.

The test testValidUrlWithGit checks various aspects of the URL parsing, but consider adding more edge cases or malformed URLs to ensure robustness.

---

41-46: Good use of exception testing for URLs without 'git'.

This test correctly identifies URLs that are missing the 'git' directory, which is crucial for the expected structure.

---

48-53: Effective testing for URL segment sufficiency.

The test testUrlWithInsufficientSegments effectively checks for URLs that do not have enough segments following 'git', which is a necessary validation.

---

55-60: Proper validation for the '.git' suffix in repository slugs.

This test ensures that the repository slug ends with '.git', which is a required format for the repository URIs.

---

62-73: Consider revising the test for local repository paths.

This test checks the transformation of a local path into a repository URI. Ensure that the LocalVCRepositoryUri constructor correctly handles different types of paths and edge cases.

---

75-86: Review the handling of remote repository paths.

Similar to the local path test, this test should verify the correct handling of remote paths. Ensure comprehensive testing to cover various path formats and potential errors.

---

88-99: Good coverage for invalid repository paths.

This test checks for invalid paths and ensures that the appropriate exception is thrown, which is crucial for robust error handling in the URI parsing logic.

---

130-139: Ensure correct handling of malformed URIs.

This test effectively checks for malformed URIs and ensures that the appropriate exceptions are thrown, which is important for robust error handling.

src/test/java/de/tum/in/www1/artemis/architecture/ArchitectureTest.java (19)

95-96: Ensure no JUnit 4 imports are used.

This test correctly checks for the absence of JUnit 4 imports across the test classes, aligning with the migration to JUnit 5.

---

98-108: Validate test class names and visibility.

This test ensures that test classes and methods have appropriate names and visibility, which is important for maintaining a clean and understandable test suite.

---

Line range hint 110-115: Check for correct usage of StringUtils and RandomStringUtils.

These tests ensure that StringUtils and RandomStringUtils are used correctly and not from unintended packages, which helps maintain dependency hygiene.

---

Line range hint 117-118: Ensure no JUnit Jupiter assertions are used.

This test checks for the absence of JUnit Jupiter assertions, aligning with the project's testing framework choices.

---

Line range hint 120-121: Check for correct usage of Collectors.toList().

This test ensures that the deprecated Collectors.toList() method is not used, promoting more modern alternatives.

---

Line range hint 123-131: Validate nullness annotations usage.

This test checks for the correct use of nullness annotations, ensuring that they are used consistently and correctly across the codebase.

---

Line range hint 133-134: Ensure correct usage of SimpMessageSendingOperations.

This test ensures that SimpMessageSendingOperations is used correctly and only within the appropriate context, which helps maintain proper encapsulation and modularity.

---

Line range hint 136-137: Check for correct file operation usage.

This test ensures that file operations are performed using the correct methods, which is important for robust file handling and error prevention.

---

Line range hint 139-140: Validate logging practices.

This test checks for the correct use of logging practices, ensuring that logs are handled appropriately and consistently across the project.

---

Line range hint 142-143: Check for correct logger field practices.

This test ensures that logger fields are named and modified correctly, which helps maintain consistency and readability in the code.

---

Line range hint 145-146: Ensure correct JSON implementations are used.

This test checks for the correct use of JSON-related classes, ensuring that only the intended libraries are used for JSON processing.

---

Line range hint 148-149: Check for exclusion of Gson usage.

This test checks for the absence of Gson usage, aligning with the project's decision to use alternative JSON parsing libraries.

---

Line range hint 151-152: Ensure no direct calls to Git.commit().

This test ensures that direct calls to Git.commit() are avoided, promoting the use of the GitService.commit() method for consistency.

---

Line range hint 154-155: Check for correct Hazelcast usage in constructors.

This test ensures that Hazelcast is not used directly in constructors, which helps avoid potential performance issues during application startup.

---

Line range hint 157-158: Ensure Spring components are correctly annotated with @Profile.

This test checks for the correct use of the @Profile annotation, ensuring that Spring components can be appropriately managed during application startup.

---

Line range hint 160-161: Validate the use of @JsonInclude with NON_EMPTY.

This test ensures that the @JsonInclude annotation is used correctly with the NON_EMPTY option, which helps reduce JSON verbosity.

---

Line range hint 163-164: Check for correct import practices for RestControllers.

This test ensures that RestControllers are not improperly imported, which helps maintain clean and modular architecture.

---

Line range hint 166-167: Ensure @Autowired is not used for field injection.

This test checks for the absence of field injection via @Autowired, promoting better dependency injection practices.

---

Line range hint 169-170: Check for correct implementation of authorization test classes.

This test ensures that authorization test classes are implemented correctly and cover all necessary endpoints, which is crucial for security testing.

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 1

[coderabbitai]

Actionable comments posted: 0

Out of diff range and nitpick comments (1)

src/main/java/de/tum/in/www1/artemis/domain/VcsRepositoryUri.java (1)

Line range hint 103-113: Optimize folder name extraction from URI.

Consider simplifying the logic for extracting folder names from URIs to improve maintainability.

- path = path.replaceAll("\\.git$", "");
- path = path.replaceAll("/$", "");
- path = path.replaceAll("^/.*scm", "");
- path = path.replaceAll("^.*?/git/", "/");
+ path = path.replaceFirst("^.*?/git/", "").replaceAll("\\.git$", "").replaceAll("/$", "");

[coderabbitai]

Actionable comments posted: 2