Commit info #436
Commit info #436
Conversation
Removed the requirement along with adding more tests to cover more edge cases
WalkthroughThis pull request introduces a new feature for retrieving Git commit information in the application. A new Changes
Sequence DiagramsequenceDiagram
Client->>CommitInfoController: GET /commit-info
CommitInfoController->>CommitInfoService: getLatestCommitInfo()
CommitInfoService->>GitRepository: Retrieve latest commit
GitRepository-->>CommitInfoService: Return commit details
CommitInfoService-->>CommitInfoController: Return CommitInfoDto
CommitInfoController-->>Client: Respond with commit information
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
service/pom.xml (1)
Update dependencies to their latest stable versions
Based on the verification results, here are the recommended version updates:
spring-boot-starter-mail: Update from 2.2.2.RELEASE to 3.2.2 (matching your Spring Boot version)commons-fileupload: Update from 1.3 to 1.5google-http-client-jackson2: Update from 1.38.1 to 1.45.3awaitility: Update from 3.1.6 to 4.2.2While no active security vulnerabilities were found through GitHub's advisory database, updating these dependencies is still important for:
- Maintaining compatibility with Spring Boot 3.x
- Getting bug fixes and performance improvements
- Following best practices for dependency management
Note: For spring-boot-starter-mail, use version 3.2.2 to match your Spring Boot version instead of the latest 3.4.1, as mixing Spring Boot versions can lead to compatibility issues.
🔗 Analysis chain
Line range hint
1-165: Update outdated dependencies for security and compatibilitySeveral dependencies are significantly outdated and should be updated:
spring-boot-starter-mail:2.2.2.RELEASE- This version is from Spring Boot 2.x while the project uses Spring Boot 3.2.2commons-fileupload:1.3- Known vulnerabilities exist in this versiongoogle-http-client-jackson2:1.38.1- Outdated versionawaitility:3.1.6- Much newer versions availablePlease update these dependencies to their latest compatible versions to ensure security and proper integration with Spring Boot 3.2.2.
Let's verify the latest versions and security advisories for these dependencies:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check latest versions and security advisories for outdated dependencies # Check Maven Central for latest versions echo "Latest versions from Maven Central:" for dep in "org.springframework.boot:spring-boot-starter-mail" "commons-fileupload:commons-fileupload" "com.google.http-client:google-http-client-jackson2" "org.awaitility:awaitility"; do echo "$dep:" curl -s "https://search.maven.org/solrsearch/select?q=g:${dep%:*}+AND+a:${dep#*:}&rows=1&wt=json" | jq -r '.response.docs[].latestVersion' done # Check for security advisories for pkg in "spring-boot-starter-mail" "commons-fileupload" "google-http-client-jackson2" "awaitility"; do echo "Security advisories for $pkg:" gh api graphql -f query=" { securityVulnerabilities(first: 5, ecosystem: MAVEN, package: \"$pkg\") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }" doneLength of output: 4220
🧹 Nitpick comments (13)
service-api/src/main/java/greencity/constant/ErrorMessage.java (2)
61-62: Consider consistent naming and messaging conventions.
Most constants simply describe the scenario without the "WARNING:" prefix. You could align this message to the style of the other error messages by removing the "WARNING:" annotation in the string.- public static final String WARNING_GIT_DIRECTORY_NOT_FOUND = - "WARNING: .git directory not found. Git commit info will be unavailable."; + public static final String GIT_DIRECTORY_NOT_FOUND = + ".git directory not found. Git commit info will be unavailable.";
65-65: Remove trailing colon or provide placeholder for exception details.
The message currently ends with a colon, implying further details would be appended. Consider removing it or appending a placeholder (e.g.,%s) if more context is provided later.- public static final String FAILED_TO_FETCH_COMMIT_INFO = "Failed to fetch commit info due to I/O error: "; + public static final String FAILED_TO_FETCH_COMMIT_INFO = "Failed to fetch commit info due to I/O error";service-api/src/main/java/greencity/exception/exceptions/ResourceNotFoundException.java (1)
7-9: Consider adding aserialVersionUID.
Since this class extendsRuntimeException(which implementsSerializable), including aserialVersionUIDis often recommended to avoid potentialInvalidClassExceptionissues when deserializing in certain environments. It’s optional but considered best practice for custom exceptions.@StandardException @ResponseStatus(HttpStatus.NOT_FOUND) -public class ResourceNotFoundException extends RuntimeException { +public class ResourceNotFoundException extends RuntimeException { + private static final long serialVersionUID = 1L; }service-api/src/main/java/greencity/service/CommitInfoService.java (1)
8-15: Document potential exceptions thrown.
Although this interface is straightforward, consider indicating that an implementation may throwResourceNotFoundExceptionif the repository cannot be accessed or found. This improves clarity for API consumers and maintainers.service-api/src/main/java/greencity/dto/commitinfo/CommitInfoDto.java (1)
12-21: Use explicit validation messages for@NotNull.
Providing custom messages for the@NotNullannotations can help clients and maintainers quickly identify which field is invalid. For example:@Data @AllArgsConstructor @NoArgsConstructor @EqualsAndHashCode public class CommitInfoDto { - @NotNull + @NotNull(message = "Commit hash must not be null") private String commitHash; - @NotNull + @NotNull(message = "Commit date must not be null") private String commitDate; }service-api/src/main/java/greencity/constant/AppConstant.java (1)
8-8: Consider using ISO 8601 format for better interoperability.
While"dd/MM/yyyy HH:mm:ss"may be satisfactory for some use cases, the ISO 8601 date format ("yyyy-MM-dd'T'HH:mm:ss'Z'") is more conventional for services that may interact globally.service/src/main/java/greencity/service/CommitInfoServiceImpl.java (3)
28-39: Handle repository initialization failure more gracefully.
When theFileRepositoryBuilderfails to locate.git, therepositoryis set to null and a log warning is recorded. This choice is acceptable, but consider providing a more descriptive log or fallback strategy, such as prompting for configuration or referencing environment variables.
45-48: Validate the repository state earlier.
You are throwingResourceNotFoundExceptiononly ingetLatestCommitInfo(). Consider an additional check after the repository is constructed to fail fast if no valid.gitdirectory is found, avoiding postponed errors at runtime.
50-60: Employ more specific exception handling.
Both repository resolution and commit parsing trigger aResourceNotFoundException. If it’s important to differentiate between initialization failures vs. commit resolution errors, consider leveraging different exception messages or dedicated exception types to help pinpoint root causes.core/src/main/java/greencity/controller/CommitInfoController.java (2)
19-20: Controller endpoint organization best practice.
It is common to prefix all endpoints with a version (e.g./api/v1/commit-info) to help manage future backward incompatibilities.
59-60: Return a descriptive response entity.
Optionally, you can include additional information or standard success messages in your response body or headers, though returning just the DTO is also valid.core/src/test/java/greencity/controller/CommitInfoControllerTest.java (1)
56-64: Verify 404 error response structure.
While you confirm the status code, verifying the response content could provide a more robust test, ensuring the error message remains consistent with expected behavior.service/src/test/java/greencity/service/CommitInfoServiceImplTest.java (1)
77-92: Test the logged warning.
Consider verifying whether a warning log is emitted (using a mock logging framework) when.gitis absent and anIOExceptionis thrown. This ensures consistent feedback to developers/operators.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
core/src/main/java/greencity/config/SecurityConfig.java(2 hunks)core/src/main/java/greencity/controller/CommitInfoController.java(1 hunks)core/src/main/resources/application.properties(1 hunks)core/src/test/java/greencity/controller/CommitInfoControllerTest.java(1 hunks)pom.xml(1 hunks)service-api/src/main/java/greencity/constant/AppConstant.java(1 hunks)service-api/src/main/java/greencity/constant/ErrorMessage.java(1 hunks)service-api/src/main/java/greencity/dto/commitinfo/CommitInfoDto.java(1 hunks)service-api/src/main/java/greencity/exception/exceptions/ResourceNotFoundException.java(1 hunks)service-api/src/main/java/greencity/service/CommitInfoService.java(1 hunks)service/pom.xml(1 hunks)service/src/main/java/greencity/service/CommitInfoServiceImpl.java(1 hunks)service/src/test/java/greencity/service/CommitInfoServiceImplTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- core/src/main/resources/application.properties
🔇 Additional comments (11)
service-api/src/main/java/greencity/constant/ErrorMessage.java (1)
63-64: No issues found.
The naming convention for this constant is clear and follows the existing patterns.core/src/main/java/greencity/controller/CommitInfoController.java (1)
30-56: OpenAPI/Swagger annotations are well-applied.
The use of@Operationand@ApiResponseannotations provides a clear and detailed API specification. Keep it up!core/src/test/java/greencity/controller/CommitInfoControllerTest.java (1)
44-55: Good test coverage for the success scenario.
It thoroughly verifies the JSON structure. Checks for each field are accurate and well-aligned with the service’s contract.service/src/test/java/greencity/service/CommitInfoServiceImplTest.java (4)
60-75: Commendable approach to mocking theFileRepositoryBuilder.
UsingmockConstructionis a good way to verify the builder logic without polluting other tests. The test clearly distinguishes success vs. failure.
95-107: Accurate error message validation.
Verifying that the thrownResourceNotFoundExceptionincludes the correct message improves maintainability and clarity, ensuring that changing error messages intentionally breaks the test.
109-130: Great coverage of date/format consistency.
The test ensures that the commit date is formatted properly. Maintaining these tests will help detect regressions if the format changes inadvertently.
132-158: Robust negative path checks.
TestingIOExceptionscenarios (fromrepository.resolveandrevWalk.parseCommit) ensures reliable error handling. This improves resiliency of the commit info fetching mechanism.core/src/main/java/greencity/config/SecurityConfig.java (2)
55-55: Constant name is descriptive and consistent.
No issues found with naming convention or usage so far.
123-124: Ensure no sensitive data is exposed by/commit-info.
Allowing unrestricted public access to commit metadata could reveal sensitive information (e.g., commit hashes or branch references). Verify that this endpoint doesn’t leak details unintended for general users.Below is a script to locate all references to
COMMIT_INFOin the codebase. Ensure that usage is limited to safe exposures:✅ Verification successful
Let me gather more information about the implementation of the commit info service and controller to understand what information is being exposed.
Let me try a different approach using ripgrep to find the implementations.
Let me check one more thing to see the complete endpoint implementation and the DTO structure.
The
/commit-infoendpoint exposes limited, non-sensitive metadataBased on the implementation review:
- The endpoint only returns basic commit metadata (hash and date)
- The service uses the local
.gitdirectory to fetch only the latest commit information- The exposed data is already publicly available to anyone with repository access
- No sensitive information like branch names, commit messages, or file changes is exposed
The current implementation appears to be safe as it:
- Only exposes the commit hash and date of the HEAD commit
- Uses proper error handling for failed Git operations
- Has a well-defined DTO that limits the exposed fields
- Is properly documented with OpenAPI annotations
The original security concern about sensitive data exposure is not warranted in this case, as the implementation follows secure practices and only exposes non-sensitive metadata that is typically public in nature.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Searching references to COMMIT_INFO rg -A 3 'COMMIT_INFO'Length of output: 3795
Script:
#!/bin/bash # Find CommitInfoController implementation ast-grep --pattern 'class CommitInfoController { $$$ }' # Find CommitInfoService interface and implementation ast-grep --pattern 'interface CommitInfoService { $$$ }' ast-grep --pattern 'class CommitInfoServiceImpl $_ { $$$ }'Length of output: 184
Script:
#!/bin/bash # Search for CommitInfo related implementations rg -A 10 "class CommitInfoController" rg -A 10 "class CommitInfoServiceImpl" rg -A 10 "interface CommitInfoService" # Also search for the actual endpoint implementation rg -A 10 "@GetMapping.*commit-info"Length of output: 5040
Script:
#!/bin/bash # Find the complete CommitInfoController endpoint implementation rg -A 15 '@Operation\(summary = "Get the latest commit hash and date."\)' # Find CommitInfoDto structure rg -A 10 'class CommitInfoDto'Length of output: 2308
pom.xml (1)
40-40: Consider simplifying the JGit version numberThe current timestamp-based version
7.1.0.202411261347-rcould be simplified to7.1.0for better maintainability while still ensuring compatibility with Java 21 and Spring Boot 3.2.2.Let's verify the latest stable version and any security advisories:
service/pom.xml (1)
161-165: LGTM! JGit dependency is correctly configured.The dependency is properly added with the version reference from the parent POM.
| @AllArgsConstructor | ||
| @NoArgsConstructor | ||
| @EqualsAndHashCode | ||
| public class CommitInfoDto { |
There was a problem hiding this comment.
maybe use 'record' in this case?
|
|
NOT THE ACTUAL PR: CHECK #441 |
#7929
Summary by CodeRabbit
Release Notes
New Features
/commit-infoendpoint to retrieve the latest Git commit informationDocumentation
Chores
The release introduces a new feature to expose Git commit details through a RESTful API, with comprehensive error handling and testing.