-
Notifications
You must be signed in to change notification settings - Fork 0
feature/7929/commit-info-dev #110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,56 @@ | ||||||||||||||||||||
| package greencity.controller; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import greencity.constant.HttpStatuses; | ||||||||||||||||||||
| import greencity.dto.CommitInfoDto; | ||||||||||||||||||||
| import greencity.service.CommitInfoService; | ||||||||||||||||||||
| import io.swagger.v3.oas.annotations.Operation; | ||||||||||||||||||||
| import io.swagger.v3.oas.annotations.media.Content; | ||||||||||||||||||||
| import io.swagger.v3.oas.annotations.media.ExampleObject; | ||||||||||||||||||||
| import io.swagger.v3.oas.annotations.responses.ApiResponse; | ||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||
| import org.springframework.http.ResponseEntity; | ||||||||||||||||||||
| import org.springframework.web.bind.annotation.GetMapping; | ||||||||||||||||||||
| import org.springframework.web.bind.annotation.RequestMapping; | ||||||||||||||||||||
| import org.springframework.web.bind.annotation.RestController; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /** | ||||||||||||||||||||
| * Controller for fetching Git commit information. | ||||||||||||||||||||
| */ | ||||||||||||||||||||
| @RestController | ||||||||||||||||||||
| @RequestMapping("/commit-info") | ||||||||||||||||||||
| @RequiredArgsConstructor | ||||||||||||||||||||
| public class CommitInfoController { | ||||||||||||||||||||
|
Comment on lines
+19
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider adding security measures. The endpoint is publicly accessible. Consider:
@RestController
@RequestMapping("/commit-info")
@RequiredArgsConstructor
+@SecurityRequirement(name = "Bearer Authentication")
public class CommitInfoController {📝 Committable suggestion
Suggested change
|
||||||||||||||||||||
| private final CommitInfoService commitInfoService; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /** | ||||||||||||||||||||
| * Endpoint to fetch the latest Git commit hash and date. | ||||||||||||||||||||
| * | ||||||||||||||||||||
| * @return {@link CommitInfoDto} | ||||||||||||||||||||
| */ | ||||||||||||||||||||
| @Operation(summary = "Get the latest commit hash and date.") | ||||||||||||||||||||
| @ApiResponse( | ||||||||||||||||||||
| responseCode = "200", | ||||||||||||||||||||
| description = HttpStatuses.OK, | ||||||||||||||||||||
| content = @Content( | ||||||||||||||||||||
| mediaType = "application/json", | ||||||||||||||||||||
| examples = @ExampleObject( | ||||||||||||||||||||
| value = """ | ||||||||||||||||||||
| { | ||||||||||||||||||||
| "commitHash": "d6e70c46b39857846f3f13ca9756c39448ab3d6f", | ||||||||||||||||||||
| "commitDate": "16/12/2024 10:55:00" | ||||||||||||||||||||
| }"""))) | ||||||||||||||||||||
| @ApiResponse( | ||||||||||||||||||||
| responseCode = "404", | ||||||||||||||||||||
| description = HttpStatuses.NOT_FOUND, | ||||||||||||||||||||
| content = @Content( | ||||||||||||||||||||
| mediaType = "application/json", | ||||||||||||||||||||
| examples = @ExampleObject( | ||||||||||||||||||||
| value = """ | ||||||||||||||||||||
| { | ||||||||||||||||||||
| "message": "Git repository not initialized. Commit info is unavailable." | ||||||||||||||||||||
| }"""))) | ||||||||||||||||||||
| @GetMapping | ||||||||||||||||||||
| public ResponseEntity<CommitInfoDto> getCommitInfo() { | ||||||||||||||||||||
| return ResponseEntity.ok(commitInfoService.getLatestCommitInfo()); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+53
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add explicit error handling. The current implementation might throw uncaught exceptions. Consider:
@GetMapping
public ResponseEntity<CommitInfoDto> getCommitInfo() {
- return ResponseEntity.ok(commitInfoService.getLatestCommitInfo());
+ try {
+ return ResponseEntity.ok(commitInfoService.getLatestCommitInfo());
+ } catch (ResourceNotFoundException e) {
+ throw e; // Let global exception handler handle it
+ } catch (Exception e) {
+ log.error("Unexpected error while fetching commit info", e);
+ throw new ServiceException("Failed to fetch commit information");
+ }
}
|
||||||||||||||||||||
| } | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package greencity.dto; | ||
|
|
||
| import jakarta.validation.constraints.NotNull; | ||
| import lombok.AllArgsConstructor; | ||
| import lombok.Data; | ||
| import lombok.EqualsAndHashCode; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| /** | ||
| * DTO for commit information response. | ||
| */ | ||
| @Data | ||
| @AllArgsConstructor | ||
| @NoArgsConstructor | ||
| @EqualsAndHashCode | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can drop |
||
| public class CommitInfoDto { | ||
| @NotNull | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could annotate with |
||
| private String commitHash; | ||
| @NotNull | ||
| private String commitDate; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| package greencity.exception.exceptions; | ||
|
|
||
| import lombok.experimental.StandardException; | ||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.web.bind.annotation.ResponseStatus; | ||
|
|
||
| @StandardException | ||
| @ResponseStatus(HttpStatus.NOT_FOUND) | ||
| public class ResourceNotFoundException extends RuntimeException { | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| import greencity.exception.exceptions.VoiceMessageNotFoundException; | ||
| import greencity.exception.exceptions.UserIsNotAdminException; | ||
| import greencity.exception.exceptions.TariffNotFoundException; | ||
| import greencity.exception.exceptions.ResourceNotFoundException; | ||
| import lombok.AllArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.springframework.boot.web.error.ErrorAttributeOptions; | ||
|
|
@@ -129,4 +130,23 @@ public final ResponseEntity<Object> handleTariffNotFoundException(TariffNotFound | |
| log.trace(ex.getMessage(), ex); | ||
| return ResponseEntity.status(HttpStatus.NOT_FOUND).body(exceptionResponse); | ||
| } | ||
|
|
||
| /** | ||
| * Method intercepts exception {@link ResourceNotFoundException}. | ||
| * | ||
| * @param ex Exception that should be intercepted. | ||
| * @param request Contains details about the occurred exception. | ||
| * @return {@code ResponseEntity} which contains the HTTP status and body with | ||
| * the exception message. | ||
| */ | ||
| @ExceptionHandler(ResourceNotFoundException.class) | ||
| public final ResponseEntity<Object> handleResourceNotFoundException(ResourceNotFoundException ex, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. idk if ReponseEntity is the best return type. there are specific types for error in spring, you can look here: |
||
| WebRequest request) { | ||
| log.error(ex.getMessage(), ex); | ||
|
|
||
| ExceptionResponse exceptionResponse = new ExceptionResponse(getErrorAttributes(request)); | ||
| exceptionResponse.setMessage(ex.getMessage()); | ||
|
|
||
| return ResponseEntity.status(HttpStatus.NOT_FOUND).body(exceptionResponse); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| package greencity.service; | ||
|
|
||
| import greencity.dto.CommitInfoDto; | ||
|
|
||
| /** | ||
| * Interface for fetching Git commit information. | ||
| */ | ||
| public interface CommitInfoService { | ||
| /** | ||
| * Fetches the latest Git commit hash and date. | ||
| * | ||
| * @return {@link CommitInfoDto} | ||
| */ | ||
| CommitInfoDto getLatestCommitInfo(); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| package greencity.service.impl; | ||
|
|
||
| import greencity.constant.AppConstant; | ||
| import greencity.constant.ErrorMessage; | ||
| import greencity.dto.CommitInfoDto; | ||
| import greencity.exception.exceptions.ResourceNotFoundException; | ||
| import greencity.service.CommitInfoService; | ||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.time.ZoneId; | ||
| import java.time.format.DateTimeFormatter; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.eclipse.jgit.lib.Repository; | ||
| import org.eclipse.jgit.revwalk.RevCommit; | ||
| import org.eclipse.jgit.revwalk.RevWalk; | ||
| import org.eclipse.jgit.storage.file.FileRepositoryBuilder; | ||
| import org.springframework.stereotype.Service; | ||
|
|
||
| /** | ||
| * Service implementation for fetching Git commit information. | ||
| */ | ||
| @Service | ||
| @Slf4j | ||
| public class CommitInfoServiceImpl implements CommitInfoService { | ||
| private Repository repository; | ||
|
|
||
| private static final String COMMIT_REF = "HEAD"; | ||
|
|
||
| public CommitInfoServiceImpl() { | ||
| try { | ||
| repository = new FileRepositoryBuilder() | ||
| .setGitDir(new File(".git")) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could export ".git" to constant |
||
| .setMustExist(true) | ||
| .readEnvironment() | ||
| .findGitDir() | ||
| .build(); | ||
| } catch (IOException e) { | ||
| repository = null; | ||
| log.warn(ErrorMessage.WARNING_GIT_DIRECTORY_NOT_FOUND); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * {@inheritDoc} | ||
| * | ||
| * @throws ResourceNotFoundException if the Git repository cannot be found, is | ||
| * not initialized, or commit information | ||
| * cannot be fetched due to an I/O error. | ||
| */ | ||
| @Override | ||
| public CommitInfoDto getLatestCommitInfo() { | ||
| if (repository == null) { | ||
| throw new ResourceNotFoundException(ErrorMessage.GIT_REPOSITORY_NOT_INITIALIZED); | ||
| } | ||
|
|
||
| try (RevWalk revWalk = new RevWalk(repository)) { | ||
| RevCommit latestCommit = revWalk.parseCommit(repository.resolve(COMMIT_REF)); | ||
| String latestCommitHash = latestCommit.name(); | ||
| String latestCommitDate = DateTimeFormatter.ofPattern(AppConstant.DATE_FORMAT) | ||
| .withZone(ZoneId.of(AppConstant.UKRAINE_TIMEZONE)) | ||
| .format(latestCommit.getAuthorIdent().getWhenAsInstant()); | ||
|
|
||
| return new CommitInfoDto(latestCommitHash, latestCommitDate); | ||
| } catch (IOException e) { | ||
| throw new ResourceNotFoundException(ErrorMessage.FAILED_TO_FETCH_COMMIT_INFO + e.getMessage()); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| package greencity.controller; | ||
|
|
||
| import greencity.dto.CommitInfoDto; | ||
| import greencity.exception.exceptions.ResourceNotFoundException; | ||
| import greencity.service.CommitInfoService; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.extension.ExtendWith; | ||
| import org.mockito.InjectMocks; | ||
| import org.mockito.Mock; | ||
|
|
||
| import org.mockito.junit.jupiter.MockitoExtension; | ||
| import org.springframework.http.MediaType; | ||
| import org.springframework.test.web.servlet.MockMvc; | ||
| import org.springframework.test.web.servlet.setup.MockMvcBuilders; | ||
|
|
||
| import static org.mockito.Mockito.times; | ||
| import static org.mockito.Mockito.verify; | ||
| import static org.mockito.Mockito.when; | ||
| import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; | ||
| import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; | ||
| import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; | ||
|
|
||
| @ExtendWith(MockitoExtension.class) | ||
| class CommitInfoControllerTest { | ||
| @InjectMocks | ||
| private CommitInfoController commitInfoController; | ||
|
|
||
| @Mock | ||
| private CommitInfoService commitInfoService; | ||
|
|
||
| private MockMvc mockMvc; | ||
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| mockMvc = MockMvcBuilders.standaloneSetup(commitInfoController).build(); | ||
| } | ||
|
|
||
| private static final String COMMIT_INFO_URL = "/commit-info"; | ||
| private static final String COMMIT_HASH = "abc123"; | ||
| private static final String COMMIT_DATE = "16/12/2024 12:06:32"; | ||
|
Comment on lines
+39
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ideally, test class should not be concerned about test data, I suggest you to move it to ModelUtils, or something like that.. + less code in test class |
||
|
|
||
| @Test | ||
| void getCommitInfoReturnsSuccessTest() throws Exception { | ||
| CommitInfoDto commitInfoDto = new CommitInfoDto(COMMIT_HASH, COMMIT_DATE); | ||
| when(commitInfoService.getLatestCommitInfo()).thenReturn(commitInfoDto); | ||
|
|
||
| mockMvc.perform(get(COMMIT_INFO_URL).accept(MediaType.APPLICATION_JSON)) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.commitHash").value(COMMIT_HASH)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would be better to convert object through objectmapper and compare it by .equals(), but it is to your preference |
||
| .andExpect(jsonPath("$.commitDate").value(COMMIT_DATE)); | ||
|
|
||
| verify(commitInfoService, times(1)).getLatestCommitInfo(); | ||
| } | ||
|
|
||
| @Test | ||
| void getCommitInfoReturnsErrorTest() throws Exception { | ||
| when(commitInfoService.getLatestCommitInfo()).thenThrow(new ResourceNotFoundException()); | ||
|
|
||
| mockMvc.perform(get(COMMIT_INFO_URL)) | ||
| .andExpect(status().isNotFound()); | ||
|
|
||
| verify(commitInfoService, times(1)).getLatestCommitInfo(); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix code formatting to comply with project standards.
The pipeline failure indicates formatting issues. Please format the file before running validation.
📝 Committable suggestion
🧰 Tools
🪛 GitHub Actions: Build
[error] File has not been previously formatted. Please format file and commit before running validation!