-
Notifications
You must be signed in to change notification settings - Fork 0
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
이력서 조회순 정렬 기능 추가 #281
이력서 조회순 정렬 기능 추가 #281
Conversation
이력서 조회순으로 조회 가능하도록 view count 추가, 이력서 개별 조회 로직에서 view count 증가
이력서가 없을 때 404 에러를 발생시키던 것에서 빈 값을 주는 방법으로 변경
findAll 제거 및 page -> slice 로 변경
view count 추가로 개별 조회 시 view가 증가, 조회 로직에서 output값으로 view_count 추가
WalkthroughThe changes introduce view count tracking and management into the resume domain. The resume retrieval endpoints have been updated to use a Slice-based pagination mechanism instead of Page, and a new endpoint has been added to return resumes sorted by view count. Additionally, response objects and conversion logic are updated to include the view count data. The repository now supports retrieving non-deleted resumes with pagination, and transactional support with view count incrementation is added to resume retrieval. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant RC as ResumeController
participant RS as ResumeService
participant R as ResumeRepository
participant D as Resume (Domain)
participant C as ResumeConverter
U->>RC: Request resume details (GET resumeId)
RC->>RS: getResume(resumeId)
RS->>R: findById(resumeId)
R-->>RS: Return resume
RS->>D: increaseViewCount()
RS->>C: Convert resume (include viewCount)
C-->>RS: Return ResumeResponse
RS-->>RC: Return response
RC-->>U: Send updated resume details
sequenceDiagram
participant U as User
participant RC as ResumeController
participant RS as ResumeService
participant R as ResumeRepository
participant C as ResumeConverter
U->>RC: Request resumes by view count (GET with page & size)
RC->>RS: searchResumesByView(page, size)
RS->>R: findResumesByDeletedAtIsNull(Pageable)
R-->>RS: Return Slice of resumes
RS->>C: Convert resumes (include viewCount)
C-->>RS: Return list of PageableResumeResponse
RS-->>RC: Return CommonResponse with resume list
RC-->>U: Send sorted resume list
Poem
✨ Finishing Touches
🪧 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 (
|
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.
Code review by ChatGPT
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.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
backend/src/main/java/com/techeer/backend/api/resume/converter/ResumeConverter.java (1)
58-62
:⚠️ Potential issueRemove duplicate resumeName field.
The
resumeName
field is set twice in the builder. Remove the duplicate line to avoid confusion and potential issues.Apply this diff to fix the duplication:
.viewCount(resume.getViewCount()) .resumeName(resume.getName()) .userName(resume.getUser().getUsername()) - .resumeName(resume.getName()) .position(resume.getPosition().getValue())
🧹 Nitpick comments (3)
backend/src/main/java/com/techeer/backend/api/resume/service/ResumeService.java (1)
52-56
: Consider adding error handling for empty results.While switching from
Page
toSlice
is a good optimization for large datasets, consider handling the case when no resumes are found.public Slice<Resume> getResumePage(Pageable pageable) { // 페이지네이션을 적용하여 레포지토리에서 데이터를 가져옴 Slice<Resume> resumes = resumeRepository.findResumesByDeletedAtIsNull(pageable); + if (!resumes.hasContent()) { + log.info("No resumes found for page {}", pageable.getPageNumber()); + } return resumes; }backend/src/main/java/com/techeer/backend/api/resume/domain/Resume.java (1)
111-113
: Consider thread safety for view count increments.The
increaseViewCount
method might face race conditions under high concurrency. Consider using atomic operations.+import jakarta.persistence.Version; @Entity public class Resume extends BaseEntity { + @Version + private Long version; public void increaseViewCount() { - this.viewCount++; + this.viewCount = this.viewCount + 1; } }This change adds optimistic locking to handle concurrent modifications safely.
backend/src/main/java/com/techeer/backend/api/resume/controller/ResumeController.java (1)
155-167
: Consider refactoring to reduce code duplication and improve flexibility.The implementation works but could be improved in two ways:
- There's significant code duplication with the
searchResumes
method.- The sort direction is hardcoded to
desc
.Consider these improvements:
- Extract common pagination logic:
private List<PageableResumeResponse> getPageableResponses(Pageable pageable) { Slice<Resume> resumeList = resumeService.getResumePage(pageable); return resumeList.stream() .map(ResumeConverter::toPageableResumeResponse) .collect(Collectors.toList()); }
- Make sort direction configurable:
- @GetMapping("/resumes/view") + @GetMapping("/resumes/view") public CommonResponse<List<PageableResumeResponse>> searchResumesByView( @RequestParam(name = "page") int page, - @RequestParam(name = "size") int size) { + @RequestParam(name = "size") int size, + @RequestParam(name = "direction", defaultValue = "DESC") Sort.Direction direction) { - final Pageable pageable = PageRequest.of(page, size, Sort.by(Sort.Order.desc("viewCount"))); + final Pageable pageable = PageRequest.of(page, size, Sort.by(direction, "viewCount")); - Slice<Resume> resumeList = resumeService.getResumePage(pageable); - - List<PageableResumeResponse> pageableResumeResponse = resumeList.stream() - .map(ResumeConverter::toPageableResumeResponse) - .collect(Collectors.toList()); + List<PageableResumeResponse> pageableResumeResponse = getPageableResponses(pageable); return CommonResponse.of(SuccessCode.OK, pageableResumeResponse); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/src/main/java/com/techeer/backend/api/resume/controller/ResumeController.java
(2 hunks)backend/src/main/java/com/techeer/backend/api/resume/converter/ResumeConverter.java
(2 hunks)backend/src/main/java/com/techeer/backend/api/resume/domain/Resume.java
(4 hunks)backend/src/main/java/com/techeer/backend/api/resume/dto/response/PageableResumeResponse.java
(1 hunks)backend/src/main/java/com/techeer/backend/api/resume/dto/response/ResumeResponse.java
(1 hunks)backend/src/main/java/com/techeer/backend/api/resume/repository/ResumeRepository.java
(2 hunks)backend/src/main/java/com/techeer/backend/api/resume/service/ResumeService.java
(3 hunks)
🔇 Additional comments (7)
backend/src/main/java/com/techeer/backend/api/resume/repository/ResumeRepository.java (1)
7-7
: LGTM!The Pageable import is correctly added to support pagination functionality.
backend/src/main/java/com/techeer/backend/api/resume/dto/response/ResumeResponse.java (1)
17-17
: LGTM! The view count field is properly integrated.The
viewCount
field is appropriately added with the correct type and access level, maintaining consistency with other fields in the response DTO.backend/src/main/java/com/techeer/backend/api/resume/dto/response/PageableResumeResponse.java (1)
19-19
: LGTM! The view count field is properly integrated.The
viewCount
field is appropriately added with consistent type and access level, maintaining parity withResumeResponse
.backend/src/main/java/com/techeer/backend/api/resume/service/ResumeService.java (1)
31-39
: LGTM! Transaction and view count increment are properly implemented.The
@Transactional
annotation ensures atomic operations for view count updates, and the increment is correctly placed after fetching the resume.backend/src/main/java/com/techeer/backend/api/resume/domain/Resume.java (2)
63-64
: LGTM! View count field is properly defined.The
viewCount
field is correctly annotated with@Column
and uses an appropriate type for counting views.
88-93
: LGTM! Null safety is properly handled.The
@PrePersist
method ensures thatviewCount
is never null when persisting a new resume.backend/src/main/java/com/techeer/backend/api/resume/controller/ResumeController.java (1)
121-121
: LGTM! Good choice using Slice instead of Page.Using
Slice
instead ofPage
is more efficient for pagination as it only needs to check for the existence of the next page, avoiding the unnecessary count query thatPage
performs.
@@ -24,4 +25,6 @@ public interface ResumeRepository extends JpaRepository<Resume, Long>, ResumeRep | |||
Resume findFirstByUserOrderByCreatedAtDesc(User user); | |||
|
|||
Slice<Resume> findResumeByUser(User user); | |||
|
|||
Slice<Resume> findResumesByDeletedAtIsNull(Pageable pageable); |
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.
🛠️ Refactor suggestion
Consider adding view count sorting capability.
While the method correctly implements pagination for non-deleted resumes, it doesn't support the PR's objective of sorting resumes by view count. Consider adding a method that includes ordering by view count.
Here's a suggested implementation:
- Slice<Resume> findResumesByDeletedAtIsNull(Pageable pageable);
+ Slice<Resume> findResumesByDeletedAtIsNullOrderByViewCountDesc(Pageable pageable);
Alternatively, you could use a custom query to have more control over the sorting:
+ @Query("SELECT r FROM Resume r WHERE r.deletedAt IS NULL ORDER BY r.viewCount DESC")
Slice<Resume> findResumesByDeletedAtIsNull(Pageable pageable);
📝 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.
Slice<Resume> findResumesByDeletedAtIsNull(Pageable pageable); | |
Slice<Resume> findResumesByDeletedAtIsNullOrderByViewCountDesc(Pageable pageable); |
.viewCount(resume.getViewCount()) | ||
.resumeName(resume.getName()) |
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.
Remove duplicate resumeName field.
The resumeName
field is set twice in the builder. Remove the duplicate line to avoid confusion and potential issues.
Apply this diff to fix the duplication:
.viewCount(resume.getViewCount())
- .resumeName(resume.getName())
.position(resume.getPosition().getValue())
📝 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.
.viewCount(resume.getViewCount()) | |
.resumeName(resume.getName()) | |
.viewCount(resume.getViewCount()) | |
.position(resume.getPosition().getValue()) |
} | ||
|
||
@PrePersist | ||
public void prePersist() { |
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.
Entity를 build할때 항상 viewCount는 0인데
왜 저장하기 전에 viewCount가 null인지 확인하는거야??
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.
빌드할 때 0으로 설정하긴 했는데, 이후에 조회했을 때 DB 계속 0이 아니라 null로 저장이 되더라고 그래서 null값 허용 안하게 하려고 이렇게 했어
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.
뷰 올라가는거 확인
변경 사항
테스트 결과 (테스트를 했으면)
Summary by CodeRabbit
New Features
Refactor