Fix/#171 카테고리 API 수정 #172
Fix/#171 카테고리 API 수정 #172
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough두 개의 엔드포인트(GET /api/v1/certification/job, GET /api/v1/certification/track)와 이를 지원하는 DTO·서비스 메서드·FavoriteRepository 쿼리가 추가되어 사용자 직무·트랙 기준 즐겨찾기 집계로 상위 3개 자격증을 순위화해 반환합니다. (50단어 이내) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Controller as CertificationController
participant Service as CertificationService
participant Repo as FavoriteRepository
participant DB as Database
Client->>Controller: GET /api/v1/certification/job (userId)
Controller->>Service: getCertificationJob(userId)
Service->>Repo: findTopByJobOrderByFavoriteCount(jobId, Pageable(0,3))
Repo->>DB: JPQL 쿼리 실행 (join Favorite/CertificationJob, group by, order by favoriteCount)
DB-->>Repo: List<Certification> (top 3)
Repo-->>Service: certifications
Service->>Service: AtomicInteger로 rank 부여 및 CertificationRankResponse 생성
Service-->>Controller: List<CertificationRankResponse>
Controller-->>Client: 200 SuccessResponse(SUCCESS_FETCH, data)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationScoreDto.java (1)
33-44: toString() 메서드에 새로운 필드 포함 권장
description필드가 record에 추가되었지만toString()메서드에는 포함되지 않았습니다. 디버깅이나 로깅 시 일관성을 위해 추가하는 것을 권장합니다.🔎 제안하는 수정
@Override public String toString() { return "CertificationScoreDto{" + "certificationId=" + certificationId + ", certificationName='" + certificationName + '\'' + ", certificationType='" + certificationType + '\'' + ", testType='" + testType + '\'' + ", tags=" + tags + ", recommendationScore=" + recommendationScore + ", isFavorite=" + isFavorite + + ", description='" + description + '\'' + '}'; }src/main/java/org/sopt/certi_server/domain/certification/controller/CertificationController.java (1)
85-92: 제네릭 타입을 구체적으로 명시하는 것을 권장합니다반환 타입에 와일드카드
<?>를 사용하는 대신<List<CertificationRankResponse>>로 명시하면 타입 안정성과 API 문서 가독성이 향상됩니다.🔎 제안하는 수정
@GetMapping("/job") @Operation(summary = "직무별 자격증 조회 API", description = "3순위 직무별 자격증을 조회합니다") -public ResponseEntity<SuccessResponse<?>> getTop3ByJob( +public ResponseEntity<SuccessResponse<List<CertificationRankResponse>>> getTop3ByJob( @AuthenticationPrincipal Long userId ){ List<CertificationRankResponse> certificationRankResponseList = certificationService.getCertificationJob(userId); return ResponseEntity.ok(SuccessResponse.of(SuccessCode.SUCCESS_FETCH, certificationRankResponseList)); } @GetMapping("/track") @Operation(summary = "계열별 자격증 조회 API", description = "3순위 계열별 자격증을 조회합니다") -public ResponseEntity<SuccessResponse<?>> getTop3ByTrack( +public ResponseEntity<SuccessResponse<List<CertificationRankResponse>>> getTop3ByTrack( @AuthenticationPrincipal Long userId ) { List<CertificationRankResponse> certificationRankResponseList = certificationService.getCertificationTrack(userId); return ResponseEntity.ok(SuccessResponse.of(SuccessCode.SUCCESS_FETCH, certificationRankResponseList)); }Also applies to: 94-102
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/org/sopt/certi_server/domain/certification/controller/CertificationController.javasrc/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationRankResponse.javasrc/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationScoreDto.javasrc/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationSimple.javasrc/main/java/org/sopt/certi_server/domain/certification/service/CertificationService.javasrc/main/java/org/sopt/certi_server/domain/favorite/repository/FavoriteRepository.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/org/sopt/certi_server/domain/favorite/repository/FavoriteRepository.java (2)
src/main/java/org/sopt/certi_server/domain/certification/repository/CertificationRepositoryCustomImpl.java (2)
CertificationRepositoryCustomImpl(19-56)findByJobAndFavorite(24-55)src/main/java/org/sopt/certi_server/domain/certification/repository/CertificationRepositoryCustom.java (2)
CertificationRepositoryCustom(8-10)findByJobAndFavorite(9-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-cd
🔇 Additional comments (4)
src/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationSimple.java (1)
19-19: LGTM!
description필드 추가가 적절하게 구현되었습니다. 기존 패턴을 잘 따르고 있습니다.Also applies to: 33-33
src/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationRankResponse.java (1)
1-19: LGTM!새로운 응답 DTO가 잘 설계되었습니다.
certificationType에 대한 null-safe 처리도 적절합니다.src/main/java/org/sopt/certi_server/domain/certification/service/CertificationService.java (1)
218-231: LGTM!
getCertificationTrack메서드는 사용자로부터 직접 track 정보를 가져오므로 안전합니다.AtomicInteger를 사용한 순위 할당 로직도 적절합니다.src/main/java/org/sopt/certi_server/domain/favorite/repository/FavoriteRepository.java (1)
50-62: LGTM!Track별 조회 쿼리는
c로만 그룹화하여 중복 문제가 없습니다. 올바르게 구현되었습니다.
src/main/java/org/sopt/certi_server/domain/certification/service/CertificationService.java
Show resolved
Hide resolved
src/main/java/org/sopt/certi_server/domain/favorite/repository/FavoriteRepository.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/org/sopt/certi_server/domain/certification/service/CertificationService.java (1)
207-207: 빈 리스트 처리 누락으로 인한 IndexOutOfBoundsException 위험 (중복)
userService.getUserJob(userId).jobList().get(0)호출 시 jobList가 비어있을 경우IndexOutOfBoundsException이 발생할 수 있습니다. 이 문제는 이전 리뷰에서 이미 지적되었으나 아직 수정되지 않았습니다.이전 리뷰 코멘트를 참고하여 jobList를 먼저 변수에 할당하고 isEmpty() 체크를 추가해주세요.
🧹 Nitpick comments (4)
.github/workflows/cd.yml (1)
45-45: Docker 이미지 태그에 버전 정보 추가를 권장합니다.현재 Docker 이미지 태그에 버전 정보(예:
:latest,:v1.0.0, 또는 커밋 SHA)가 없어 배포된 이미지의 버전 추적이 어려울 수 있습니다. 명시적인 태그를 추가하면 롤백 및 버전 관리가 용이합니다.🔎 버전 태그 추가 예시
- tags: seongmin0229/certi-server + tags: | + seongmin0229/certi-server:latest + seongmin0229/certi-server:${{ github.sha }}또한, Docker Hub 계정 변경이 의도된 것인지 확인해 주세요.
.github/workflows/ci.yml (1)
45-45: CI와 CD 워크플로우 간 일관성이 유지되었습니다.Docker 이미지 태그가
cd.yml과 동일하게 업데이트되어 일관성이 확보되었습니다. 다만,cd.yml의 리뷰 코멘트에서 언급한 것처럼 버전 태그 추가를 고려해 주세요.src/main/java/org/sopt/certi_server/domain/certification/service/CertificationService.java (2)
206-206: 불필요한 코드 및 단순화 가능한 부분두 가지 개선 사항을 제안합니다:
- Line 206:
user변수를 가져오지만 실제로 사용하지 않습니다.userService.getUser(userId)호출을 제거할 수 있습니다.- Line 215: 단순 순차 카운팅에
AtomicInteger는 과합니다. 동시성 제어가 필요 없으므로 일반 int 변수로 충분합니다.🔎 제안하는 개선안
public List<CertificationRankResponse> getCertificationJob(final Long userId){ - User user = userService.getUser(userId); String jobName = userService.getUserJob(userId).jobList().get(0); Job job = jobRepository.findByName(jobName) .orElseThrow(() -> new NotFoundException(ErrorCode.JOB_NOT_FOUND)); Pageable top3 = PageRequest.of(0, 3); List<Certification> certList = favoriteRepository.findTopByJobOrderByFavoriteCount(job.getId(), top3); - AtomicInteger rank = new AtomicInteger(1); + int rank = 1; - return certList.stream() - .map(cert -> new CertificationRankResponse(rank.getAndIncrement(), cert)) - .toList(); + List<CertificationRankResponse> result = new ArrayList<>(); + for (Certification cert : certList) { + result.add(new CertificationRankResponse(rank++, cert)); + } + return result;Also applies to: 215-215
230-230: AtomicInteger 단순화 가능
getCertificationJob메서드와 동일하게, 단순 순차 카운팅에는AtomicInteger대신 일반 int 변수를 사용하는 것이 더 간결합니다.🔎 제안하는 개선안
public List<CertificationRankResponse> getCertificationTrack(final Long userId){ User user = userService.getUser(userId); TrackType trackType = user.getTrack(); Pageable top3 = PageRequest.of(0, 3); List<Certification> certificationList = favoriteRepository.findTopCertificationsByTrack(trackType, top3); - AtomicInteger rank = new AtomicInteger(1); + int rank = 1; - return certificationList.stream() - .map(c -> new CertificationRankResponse(rank.getAndIncrement(), c)) - .toList(); + List<CertificationRankResponse> result = new ArrayList<>(); + for (Certification c : certificationList) { + result.add(new CertificationRankResponse(rank++, c)); + } + return result;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/cd.yml.github/workflows/ci.ymlsrc/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationScoreDto.javasrc/main/java/org/sopt/certi_server/domain/certification/service/CertificationService.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/sopt/certi_server/domain/certification/dto/response/CertificationScoreDto.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/org/sopt/certi_server/domain/certification/service/CertificationService.java (3)
src/main/java/org/sopt/certi_server/global/error/exception/NotFoundException.java (1)
NotFoundException(5-9)src/main/java/org/sopt/certi_server/domain/user/service/UserService.java (1)
Service(26-108)src/main/java/org/sopt/certi_server/domain/favorite/service/FavoriteService.java (1)
Service(19-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-cd
🔇 Additional comments (2)
src/main/java/org/sopt/certi_server/domain/certification/service/CertificationService.java (2)
20-20: 새로운 의존성 및 임포트 추가 확인새로운 기능을 지원하기 위한 임포트와
FavoriteRepository의존성이 적절히 추가되었습니다.Also applies to: 26-27, 32-32, 52-52
213-213: FavoriteRepository 메서드 확인 완료
findTopByJobOrderByFavoriteCount와findTopCertificationsByTrack메서드가 FavoriteRepository에 올바르게 구현되어 있습니다. 두 메서드 모두 @query 애노테이션으로 작성된 쿼리를 사용하며, Pageable 파라미터를 통해 결과를 제한하여 Top 3 자격증을 반환합니다.
|
|
||
| public List<CertificationRankResponse> getCertificationTrack(final Long userId){ | ||
| User user = userService.getUser(userId); | ||
| TrackType trackType = user.getTrack(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find src -name "User.java" -type f | head -20Repository: cerdeuk/CERTI-BE
Length of output: 124
🏁 Script executed:
cat -n src/main/java/org/sopt/certi_server/domain/certification/service/CertificationService.java | sed -n '220,235p'Repository: cerdeuk/CERTI-BE
Length of output: 725
🏁 Script executed:
rg -A 10 "TrackType.*track" src/main/java/org/sopt/certi_server/domain/user/entity/User.javaRepository: cerdeuk/CERTI-BE
Length of output: 737
🏁 Script executed:
rg "findTopCertificationsByTrack" src/main/java/org/sopt/certi_server/domain/certification/repository/Repository: cerdeuk/CERTI-BE
Length of output: 42
🏁 Script executed:
cat -n src/main/java/org/sopt/certi_server/domain/user/entity/User.java | sed -n '1,100p'Repository: cerdeuk/CERTI-BE
Length of output: 3137
🏁 Script executed:
rg "private TrackType track" -A 5 -B 2 src/main/java/org/sopt/certi_server/domain/user/entity/User.javaRepository: cerdeuk/CERTI-BE
Length of output: 253
🏁 Script executed:
rg "findTopCertifications" src/main/java/ --type javaRepository: cerdeuk/CERTI-BE
Length of output: 405
🏁 Script executed:
rg "getUser" src/main/java/org/sopt/certi_server/domain/user/service/UserService.java -A 10Repository: cerdeuk/CERTI-BE
Length of output: 1473
🏁 Script executed:
cat -n src/main/java/org/sopt/certi_server/domain/favorite/repository/FavoriteRepository.java | grep -A 15 "findTopCertificationsByTrack"Repository: cerdeuk/CERTI-BE
Length of output: 224
🏁 Script executed:
cat src/main/java/org/sopt/certi_server/domain/favorite/repository/FavoriteRepository.javaRepository: cerdeuk/CERTI-BE
Length of output: 2065
트랙 타입이 null인 경우 쿼리가 올바르게 작동하지 않습니다
User 엔티티의 track 필드는 nullable=false 제약이 없어 null이 될 수 있습니다. user.getTrack()이 null을 반환하면 이를 그대로 저장소 메서드에 전달하는데, 쿼리의 where u.track = :track 조건에서 null 값과의 비교로 인해 예상치 못한 결과가 반환됩니다. SQL에서 NULL = NULL은 false이므로 해당 사용자의 인증들이 조회되지 않습니다.
trackType이 null인 경우를 처리하거나, 사용자의 track이 반드시 존재해야 한다면 명시적으로 검증하세요.
🤖 Prompt for AI Agents
In
src/main/java/org/sopt/certi_server/domain/certification/service/CertificationService.java
around line 224, user.getTrack() can be null which breaks the repository query
using "where u.track = :track"; update the service to handle nulls explicitly:
check if trackType == null and either (a) call a repository method that queries
for u.track IS NULL (or add a new repository method for that case) or (b)
validate and throw a clear exception (e.g., IllegalStateException or a custom
validation exception) if a track is required; ensure the chosen branch uses a
repository query that matches null correctly or fails fast with a descriptive
error.
seongmin0229
left a comment
seongmin0229
left a comment
There was a problem hiding this comment.
수정사항 반영 후에 이 pr 먼저 머지하고 제 pr이랑 합쳐서 다시 머지하면 될 것 같습니다.
📣 Related Issue
📝 Summary
🙏 Question & PR point
Summary by CodeRabbit
새로운 기능
잡무/인프라
✏️ Tip: You can customize this high-level summary in your review settings.