[SRLT-111] 전문가 상세정보를 조회한다 #67
Conversation
|
Note
|
| Cohort / File(s) | 변경 요약 |
|---|---|
퍼시스턴스 어댑터 src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java, src/main/java/starlight/adapter/expert/persistence/ExpertRepository.java |
Repository에서 EntityGraph 제거, ID 기반 다단계 fetch 메서드(findAllIds, fetchExpertsWithCareersByIds, fetchExpertsWithTagsByIds, fetchExpertsWithCategoriesByIds) 추가. ExpertJpa는 QueryPort·LookupPort 구현으로 확장하고 fetchWithCollections 헬퍼 추가 및 메서드명/시그니처 변경. |
도메인 모델 src/main/java/starlight/domain/expert/entity/Expert.java, src/main/java/starlight/domain/expert/entity/ExpertCareer.java |
Expert에 oneLineIntroduction, detailedIntroduction 추가. careers 매핑을 String 컬렉션에서 OneToMany ExpertCareer 엔티티로 변경 및 신규 ExpertCareer 엔티티 추가(생성/업데이트 API 포함). |
애플리케이션 포트/서비스·UseCase 재배치 src/main/java/starlight/application/expert/required/ExpertQueryPort.java, .../provided/ExpertDetailQueryUseCase.java, src/main/java/starlight/application/expert/ExpertDetailQueryService.java, (삭제) src/main/java/starlight/application/expert/ExpertQueryService.java |
기존 ExpertQuery/ExpertFinder 기반 구조를 ExpertQueryPort/ExpertLookupPort/ExpertApplicationLookupPort 등으로 분리·리네임. ExpertDetailQueryUseCase 추가 및 구현(ExpertDetailQueryService)으로 검색·상세 흐름 재구성. 단순 위임 서비스(ExpertQueryService) 제거. |
애플리케이션 DTO 추가/변경 src/main/java/starlight/application/expert/provided/dto/ExpertDetailResult.java, .../ExpertCareerResult.java |
ExpertDetailResult/ExpertCareerResult 레코드 추가 및 domain→DTO 변환 로직(from) 추가. |
웹 어댑터·컨트롤러·응답 DTO src/main/java/starlight/adapter/expert/webapi/ExpertController.java, .../dto/ExpertDetailResponse.java, .../dto/ExpertCareerResponse.java, .../dto/ExpertListResponse.java, src/main/java/starlight/adapter/expert/webapi/swagger/ExpertQueryApiDoc.java |
컨트롤러가 ExpertDetailQueryUseCase 주입으로 전환. 검색 응답 타입을 ExpertListResponse로 변경, 상세 엔드포인트(/v1/experts/{id}) 추가. ExpertDetailResponse가 ExpertDetailResult 기반으로 재구성되고 careers가 ExpertCareerResponse로 변경. OpenAPI 예시 및 시그니처 갱신. |
ExpertApplication 관련 변경 src/main/java/starlight/adapter/expertApplication/persistence/ExpertApplicationJpa.java, .../persistence/ExpertApplicationRepository.java, .../webapi/ExpertApplicationController.java, .../provided/ExpertApplicationQueryUseCase.java, .../provided/ExpertApplicationServiceUseCase.java, .../required/ExpertLookupPort.java, .../required/ExpertApplicationLookupPort.java |
신청 건수 집계용 프로젝션·쿼리(countByExpertIds) 추가. ExpertApplicationJpa에 countByExpertIds 구현 추가. 컨트롤러/서비스 타입이 UseCase/LookupPort로 교체. |
ExpertReport 관련 리팩토링 src/main/java/starlight/application/expertReport/ExpertReportService.java, .../provided/ExpertReportServiceUseCase.java, .../required/ExpertLookupPort.java, src/main/java/starlight/adapter/expertReport/webapi/ExpertReportController.java, .../dto/ExpertReportResponse.java |
서비스와 제공 인터페이스를 UseCase로 리네임하고 ExpertFinder 의존을 ExpertLookupPort로 교체. ExpertReportResponse에서 ExpertDetailResult 기반 매핑 사용으로 변경. |
테스트 변경 src/test/java/.../ExpertRepositoryTest.java, src/test/java/.../ExpertControllerTest.java, (삭제) src/test/java/.../ExpertQueryServiceTest.java |
테스트 데이터가 ExpertCareer/DTO 기반으로 전환. 일부 위임형 서비스 테스트 파일 삭제. |
기타 설정·문서·정리 .gitignore, .github/PULL_REQUEST_TEMPLATE/why-what-how.md, 개발가이드.md, src/main/java/starlight/bootstrap/SecurityConfig.java, src/main/java/starlight/StarlightApplication.java, src/main/java/starlight/bootstrap/*.java, src/main/java/starlight/adapter/order/* |
.gitignore에 AGENTS.md 추가. 보안 설정에 "/v1/experts/*" permitAll 추가. PR 템플릿·개발가이드 보강, 불필요한 import/필드 정리 등 소소한 정리. |
Sequence Diagram(s)
sequenceDiagram
autonumber
participant Client
participant Controller as ExpertController
participant UseCase as ExpertDetailQueryService
participant QueryPort as ExpertQueryPort / ExpertJpa
participant AppLookup as ExpertApplicationLookupPort / ExpertApplicationJpa
participant RepoE as ExpertRepository
participant RepoA as ExpertApplicationRepository
Client->>Controller: GET /v1/experts?categories=...
Controller->>UseCase: search(categories)
alt categories empty
UseCase->>QueryPort: findAllWithCareersTagsCategories()
else
UseCase->>QueryPort: findByAllCategories(categories)
end
QueryPort->>RepoE: findAllIds() / findAllByIds(...)
RepoE-->>QueryPort: ids / experts (base)
QueryPort->>RepoE: fetchExpertsWithCareersByIds(ids)
RepoE-->>QueryPort: career rows
QueryPort->>RepoE: fetchExpertsWithTagsByIds(ids)
RepoE-->>QueryPort: tag rows
QueryPort->>RepoE: fetchExpertsWithCategoriesByIds(ids)
RepoE-->>QueryPort: category rows
QueryPort-->>UseCase: assembled Expert list
UseCase->>AppLookup: countByExpertIds(expertIds)
AppLookup->>RepoA: countByExpertIds(ids)
RepoA-->>AppLookup: projections
AppLookup-->>UseCase: Map<expertId,count>
UseCase-->>Controller: List<ExpertDetailResult>
Controller-->>Client: ApiResponse<List<ExpertListResponse>>
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
- [SRLT-97] Chore: 전문가를 로딩할때, 한번에 로딩한다 #57 — ExpertRepository/ExpertJpa의 페치 전략 변경 및 ID 기반 다단계 조회 구현과 직접적으로 연관됩니다.
- [SRLT-14] 전문가를 필터링하고 조회한다 #25 — Expert 쿼리·퍼시스턴스와 포트/서비스 재구성 영역에서 중복 변경이 있어 코드 수준 연관성이 높습니다.
- [SRLT-25] 토큰 기반 링크 유효성 설계 #36 — ExpertReport 관련 서비스·컨트롤러·DTO 변경과 연계되어 리포트 쪽 리팩토링과 관련됩니다.
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 2.99% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목이 주요 변경사항을 정확하게 반영하고 있습니다. 전문가 상세정보 조회 기능 구현이라는 핵심 의도가 명확하게 드러나 있습니다. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
SRLT-111-전문가-상세페이지-api
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 help to get the list of available commands and usage tips.
Test Results251 tests 251 ✅ 10s ⏱️ Results for commit 6f5335a. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java (1)
43-56: 배치 페칭 패턴에 심각한 결함이 있습니다.현재 구현은 N+1 문제를 방지하지 못합니다:
fetchCareers(ids)의 반환값만experts변수에 저장되고,fetchTags(ids)와fetchCategories(ids)의 반환값이 버려집니다.- 각 fetch 메서드는 별도의 Expert 객체 인스턴스를 반환하므로, 반환된
experts리스트에는 careers만 로드되고 tags와 categories는 초기화되지 않습니다.- 이후 코드에서
expert.getTags()또는expert.getCategories()에 접근하면 지연 로딩(lazy loading)으로 인해 N+1 문제가 발생합니다.개선 방안:
Repository에 이미 존재하는findAllWithDetails()메서드(29-31줄)를 사용하세요.@EntityGraph를 통해 한 번의 쿼리로 모든 연관 엔티티를 효율적으로 로드합니다. ExpertJpa의 메서드도 동일한 이름으로 Repository 메서드를 단순히 위임하는 것으로 충분합니다.
🧹 Nitpick comments (11)
.gitignore (1)
39-49: 중복된 .gitignore 항목을 정리해주세요.현재 파일에 다음 항목들이 중복되어 있습니다:
.env(라인 39, 42).vscode/(라인 37, 43).idea/(라인 21, 44)AGENTS.md 추가는 좋지만, 이 기회에 중복된 항목을 하나로 통합하여 가독성을 개선하는 것을 권장합니다.
🔎 정리 제안
### VS Code ### .vscode/ .env .DS_Store *.p8 -.env -.vscode/ .idea/ -node_modules/ -dist/ +node_modules/ +dist/ *.log /docs/ /AGENTS.mdsrc/main/java/starlight/domain/expert/entity/ExpertCareer.java (1)
33-34: 진행 중인 경력에 대한careerEndedAt처리 검토
careerEndedAt이nullable=false로 설정되어 있습니다. 현재 진행 중인 경력(종료일 미정)의 경우 어떻게 처리할지 고려가 필요합니다. 의도된 설계라면 무시하셔도 됩니다.src/test/java/starlight/adapter/expert/webapi/ExpertControllerTest.java (1)
96-116: 헬퍼 메서드 시그니처에서 불필요한throws Exception제거 권장
expertResult()메서드가 실제로 예외를 던지지 않으므로throws Exception선언을 제거할 수 있습니다.🔎 수정 제안
- private ExpertDetailResult expertResult(Long id, String name, Set<TagCategory> cats) throws Exception { + private ExpertDetailResult expertResult(Long id, String name, Set<TagCategory> cats) {src/main/java/starlight/adapter/expertApplication/persistence/ExpertApplicationJpa.java (1)
56-60: 불필요한 타입 캐스팅 제거 가능
ExpertIdCountProjection::getCount()가 이미long을 반환하므로(long)캐스팅이 불필요합니다.🔎 수정 제안
return repository.countByExpertIds(expertIds).stream() .collect(Collectors.toMap( ExpertApplicationRepository.ExpertIdCountProjection::getExpertId, - p -> (long) p.getCount() + ExpertApplicationRepository.ExpertIdCountProjection::getCount ));src/test/java/starlight/adapter/expert/persistence/ExpertRepositoryTest.java (1)
66-76: 테스트 헬퍼 메서드가 적절히 구현되었습니다.
ExpertCareer도메인 모델 변경에 맞춰 테스트 설정이 잘 업데이트되었습니다.테스트 데이터의 일관성을 위해
LocalDateTime.now()대신 고정된 시간 값을 사용하면 테스트 재현성이 향상될 수 있습니다:🔎 선택적 개선안
private ExpertCareer career(Expert expert, int orderIndex, String title) { + LocalDateTime fixedTime = LocalDateTime.of(2024, 1, 1, 0, 0); ExpertCareer career = ExpertCareer.of( title, "desc", - LocalDateTime.now().minusMonths(1), - LocalDateTime.now() + fixedTime.minusMonths(1), + fixedTime ); ReflectionTestUtils.setField(career, "expert", expert); ReflectionTestUtils.setField(career, "orderIndex", orderIndex); return career; }src/main/java/starlight/adapter/expertReport/webapi/dto/ExpertReportResponse.java (1)
22-31: 하드코딩된0L값에 대한 의도를 명확히 해주세요.
ExpertDetailResult.from(expert, 0L)에서applicationCount로0L을 하드코딩하고 있습니다. 리포트 컨텍스트에서 실제 지원 횟수가 필요하지 않다면, 이 의도를 주석으로 명시하거나ExpertDetailResult에applicationCount없이 생성하는 별도의 팩토리 메서드를 고려해보세요.🔎 선택적 리팩토링 예시
// ExpertDetailResult에 추가 public static ExpertDetailResult fromWithoutApplicationCount(Expert expert) { return from(expert, 0L); // 또는 null }또는 현재 코드에 주석 추가:
return new ExpertReportResponse( + // 리포트 컨텍스트에서는 applicationCount가 필요하지 않으므로 0으로 설정 ExpertDetailResponse.from(ExpertDetailResult.from(expert, 0L)),src/main/java/starlight/application/expert/provided/dto/ExpertDetailResult.java (1)
27-34:distinct()호출이 중복될 수 있습니다.
Expert엔티티에서tags와categories는Set으로 정의되어 있어 이미 중복이 제거된 상태입니다.stream().distinct()는Set에 대해 불필요한 연산이 될 수 있습니다.🔎 간소화된 코드
- List<String> categories = expert.getCategories().stream() - .map(TagCategory::name) - .distinct() - .toList(); + List<String> categories = expert.getCategories().stream() + .map(TagCategory::name) + .toList(); - List<String> tags = expert.getTags().stream() - .distinct() - .toList(); + List<String> tags = new ArrayList<>(expert.getTags());src/main/java/starlight/adapter/expert/persistence/ExpertRepository.java (1)
20-27: 빈 리스트 전달 시 동작 확인 필요
ids가 빈 리스트일 경우IN :ids절의 동작이 JPA 구현체에 따라 다를 수 있습니다. 호출부에서 빈 리스트 체크를 하거나, 메서드 내에서 early return을 고려해 주세요.src/main/java/starlight/application/expertApplication/ExpertApplicationService.java (2)
46-62: 예외 처리 범위 검토
catch (Exception e)로 모든 예외를 잡아EXPERT_FEEDBACK_REQUEST_FAILED로 래핑하고 있습니다. 로그에 원본 예외가 기록되어 디버깅은 가능하지만, 클라이언트에게 더 구체적인 에러 유형을 전달하려면 예외 타입별 분기를 고려해볼 수 있습니다.
79-86:registerApplicationRecord접근 제어자 검토현재
public으로 선언되어 있지만, 내부에서만 사용되는 것으로 보입니다. 외부 노출이 불필요하다면private으로 변경하거나, 테스트 목적이라면protected또는 패키지 프라이빗으로 조정해 주세요.🔎 접근 제어자 변경 제안
- public void registerApplicationRecord(Long expertId, Long planId) { + private void registerApplicationRecord(Long expertId, Long planId) {src/main/java/starlight/adapter/expertApplication/webapi/ExpertApplicationController.java (1)
33-42: 불필요한throws Exception제거 권장
ExpertApplicationService.requestFeedback()이 내부에서 모든 예외를ExpertApplicationException으로 래핑하고 있으므로, 컨트롤러 메서드의throws Exception선언은 불필요합니다. 실제로 던져지는 예외 타입을 명시하거나 제거하는 것이 좋습니다.🔎 throws 절 제거 제안
@PostMapping(value = "/{expertId}/request", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) public ApiResponse<String> requestFeedback( @PathVariable Long expertId, @RequestParam Long businessPlanId, @RequestParam("file") MultipartFile file, @AuthenticationPrincipal AuthDetails auth - ) throws Exception { + ) { applicationServiceUseCase.requestFeedback(expertId, businessPlanId, file, auth.getUser().getName()); return ApiResponse.success("피드백 요청이 전달되었습니다."); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
.gitignoresrc/main/java/starlight/adapter/expert/persistence/ExpertJpa.javasrc/main/java/starlight/adapter/expert/persistence/ExpertRepository.javasrc/main/java/starlight/adapter/expert/webapi/ExpertController.javasrc/main/java/starlight/adapter/expert/webapi/dto/ExpertCareerResponse.javasrc/main/java/starlight/adapter/expert/webapi/dto/ExpertDetailResponse.javasrc/main/java/starlight/adapter/expertApplication/persistence/ExpertApplicationJpa.javasrc/main/java/starlight/adapter/expertApplication/persistence/ExpertApplicationRepository.javasrc/main/java/starlight/adapter/expertApplication/webapi/ExpertApplicationController.javasrc/main/java/starlight/adapter/expertReport/webapi/ExpertReportController.javasrc/main/java/starlight/adapter/expertReport/webapi/dto/ExpertReportResponse.javasrc/main/java/starlight/application/expert/ExpertDetailQueryService.javasrc/main/java/starlight/application/expert/ExpertQueryService.javasrc/main/java/starlight/application/expert/provided/ExpertDetailQueryUseCase.javasrc/main/java/starlight/application/expert/provided/ExpertFinder.javasrc/main/java/starlight/application/expert/provided/dto/ExpertCareerResult.javasrc/main/java/starlight/application/expert/provided/dto/ExpertDetailResult.javasrc/main/java/starlight/application/expert/required/ExpertApplicationLookupPort.javasrc/main/java/starlight/application/expert/required/ExpertQueryPort.javasrc/main/java/starlight/application/expertApplication/ExpertApplicationQueryService.javasrc/main/java/starlight/application/expertApplication/ExpertApplicationService.javasrc/main/java/starlight/application/expertApplication/provided/ExpertApplicationQueryUseCase.javasrc/main/java/starlight/application/expertApplication/provided/ExpertApplicationServiceUseCase.javasrc/main/java/starlight/application/expertApplication/required/ExpertLookupPort.javasrc/main/java/starlight/application/expertReport/ExpertReportService.javasrc/main/java/starlight/application/expertReport/provided/ExpertReportServiceUseCase.javasrc/main/java/starlight/application/expertReport/required/ExpertLookupPort.javasrc/main/java/starlight/domain/expert/entity/Expert.javasrc/main/java/starlight/domain/expert/entity/ExpertCareer.javasrc/test/java/starlight/adapter/expert/persistence/ExpertRepositoryTest.javasrc/test/java/starlight/adapter/expert/webapi/ExpertControllerTest.javasrc/test/java/starlight/application/expert/ExpertQueryServiceTest.java
💤 Files with no reviewable changes (3)
- src/main/java/starlight/application/expert/ExpertQueryService.java
- src/main/java/starlight/application/expert/provided/ExpertFinder.java
- src/test/java/starlight/application/expert/ExpertQueryServiceTest.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/starlight/domain/expert/entity/ExpertCareer.java (1)
src/main/java/starlight/domain/expert/entity/Expert.java (1)
Getter(19-60)
src/main/java/starlight/adapter/expertApplication/persistence/ExpertApplicationJpa.java (1)
src/main/java/starlight/domain/expertApplication/exception/ExpertApplicationException.java (1)
ExpertApplicationException(6-11)
⏰ 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). (2)
- GitHub Check: build-and-push
- GitHub Check: test
🔇 Additional comments (26)
src/main/java/starlight/application/expertApplication/provided/ExpertApplicationServiceUseCase.java (1)
7-9: LGTM! UseCase 명명 규칙 적용 확인헥사고날 아키텍처의 provided 포트로서
UseCase접미사를 사용하여 인터페이스를 명명한 점이 PR 목표와 일치합니다.src/test/java/starlight/adapter/expert/webapi/ExpertControllerTest.java (1)
47-58: LGTM! 테스트 케이스 구조 적절UseCase 기반 아키텍처로 전환하면서 테스트도 적절히 업데이트되었습니다. Mock 설정과 검증 로직이 명확합니다.
src/main/java/starlight/application/expertApplication/provided/ExpertApplicationQueryUseCase.java (1)
5-7: LGTM! 명확한 쿼리 UseCase 인터페이스단일 책임 원칙을 따르는 간결한 인터페이스입니다. 헥사고날 아키텍처의 provided 포트로서 적절합니다.
src/main/java/starlight/application/expert/required/ExpertApplicationLookupPort.java (1)
6-8: LGTM! 크로스 도메인 LookupPort 패턴 적용Expert 도메인에서 ExpertApplication 데이터를 조회하기 위한 LookupPort를 consumer 도메인에 배치한 것은 PR 목표에 부합합니다.
{OtherDomain}LookupPort명명 규칙이 일관되게 적용되었습니다.src/main/java/starlight/adapter/expert/webapi/ExpertController.java (1)
22-29: LGTM! UseCase 기반 컨트롤러 리팩토링 완료컨트롤러가
ExpertDetailQueryUseCase에 위임하고, 응답 DTO 매핑은ExpertDetailResponse.fromAllResults()에서 처리하는 구조가 깔끔합니다. 어댑터 계층에서 엔티티 의존성을 제거한 점이 PR 목표와 일치합니다.src/main/java/starlight/adapter/expertApplication/persistence/ExpertApplicationJpa.java (1)
49-65: LGTM! LookupPort 구현 적절null/empty 입력 처리, 에러 핸들링, 로깅이 기존 메서드들과 일관된 패턴으로 구현되었습니다.
ExpertApplicationLookupPort를 어댑터에서 구현하여 크로스 도메인 조회를 지원하는 구조가 적절합니다.src/main/java/starlight/application/expertReport/provided/ExpertReportServiceUseCase.java (1)
10-19: LGTM! UseCase 명명 규칙 적용인터페이스 이름이
ExpertReportServiceUseCase로 변경되어 PR의 명명 규칙과 일치합니다.참고:
saveReport메서드가 도메인 엔티티ExpertReport와ExpertReportDetail을 직접 노출하고 있습니다. 다른 UseCase들처럼 DTO 기반으로 전환할지 향후 검토해 보시는 것도 좋겠습니다.src/main/java/starlight/application/expertApplication/required/ExpertLookupPort.java (1)
1-8: 헥사고날 아키텍처 패턴을 잘 따르고 있습니다.
ExpertLookupPort가 consumer 도메인(expertApplication)에 위치하여 cross-domain lookup 책임이 명확히 분리되었습니다.다만,
starlight.application.expertReport.required.ExpertLookupPort와 동일한 인터페이스명을 사용하고 있어 혼동 가능성이 있습니다. PR 리뷰 질문에서 언급하신 대로ExpertLookupPort대신 도메인별로 구분된 네이밍(예:ExpertForApplicationLookupPort또는 패키지 수준에서 구분)을 고려해 보시는 것도 좋겠습니다.src/main/java/starlight/application/expertApplication/ExpertApplicationQueryService.java (1)
11-21: LGTM! Query 서비스가 적절히 구현되었습니다.
@Transactional(readOnly = true)적용으로 읽기 전용 트랜잭션 최적화가 적절합니다.- UseCase 인터페이스 구현과 Port 위임 패턴이 헥사고날 아키텍처 원칙에 부합합니다.
- 생성자 주입을 통한 의존성 관리가 잘 되어 있습니다.
src/main/java/starlight/application/expertReport/required/ExpertLookupPort.java (1)
8-13: Port 설계가 적절합니다.
findByIdWithDetails와findByIds메서드 분리를 통해 단건/배치 조회를 효율적으로 지원합니다.한 가지 확인 사항:
Expert도메인 엔티티를 직접 반환하는데, 이 방식이 cross-domain 의존성 측면에서 의도된 설계인지 확인 부탁드립니다. 헥사고날 아키텍처에서는 도메인 간 결합을 줄이기 위해 별도 DTO를 사용하는 경우도 있습니다. 다만, 현재 구조에서 lookup 용도로만 사용된다면 실용적인 선택일 수 있습니다.src/main/java/starlight/application/expert/provided/ExpertDetailQueryUseCase.java (1)
9-11: UseCase 인터페이스가 잘 정의되었습니다.
ExpertDetailResultDTO를 반환하여 도메인 엔티티 노출을 방지하고 있습니다.Set<TagCategory>파라미터를 통한 유연한 필터링 지원이 좋습니다.provided패키지에 위치하여 어댑터 계층에서 사용되는 use case임이 명확합니다.src/main/java/starlight/adapter/expertReport/webapi/ExpertReportController.java (1)
11-26: UseCase 인터페이스 의존으로 전환이 적절합니다.
ExpertReportService구현체 대신ExpertReportServiceUseCase인터페이스에 의존하도록 변경하여 헥사고날 아키텍처의 의존성 역전 원칙을 잘 따르고 있습니다.src/main/java/starlight/application/expert/provided/dto/ExpertCareerResult.java (1)
7-25: Record를 활용한 DTO 설계가 깔끔합니다.불변성을 보장하는 Java record와 정적 팩토리 메서드(
from) 패턴이 잘 적용되었습니다. 도메인 엔티티에서 DTO로의 변환 책임이 명확하게 분리되어 있습니다.한 가지 고려사항:
ExpertCareer의 필드 중 nullable한 값이 있다면(예:careerEndedAt이 진행 중인 경력일 경우 null),from메서드에서 null 체크 없이 바로 매핑해도 record가 null을 허용하므로 문제는 없지만, 명시적인 문서화나@Nullable어노테이션 추가를 고려해 볼 수 있습니다.src/main/java/starlight/adapter/expertApplication/persistence/ExpertApplicationRepository.java (1)
21-32: Projection 인터페이스와 집계 쿼리가 적절히 구현되었으며, 빈 리스트 처리도 잘 갖춰져 있습니다.Spring Data JPA projection 패턴을 효과적으로 활용하고 있습니다.
ExpertApplicationJpa어댑터에서countByExpertIds호출 시 빈 리스트 또는 null 값에 대한 방어 로직(lines 52-53)이 이미 구현되어 있어,IN ()절 에러 발생 가능성이 제거되었습니다. 리포지토리 메서드는 실제로 유효한 데이터가 전달될 때만 실행되므로 안전합니다.src/main/java/starlight/application/expert/required/ExpertQueryPort.java (1)
9-14: 인터페이스 설계가 적절합니다.
ExpertQueryPort로의 네이밍 변경과 조회 전용 메서드만 유지한 설계가 헥사고날 아키텍처의 포트 패턴에 부합합니다. 상세 조회(findById,findByIdWithDetails)가ExpertLookupPort로 분리된 것은 책임 분리 관점에서 좋은 접근입니다.src/main/java/starlight/adapter/expert/webapi/dto/ExpertCareerResponse.java (1)
7-29: DTO 레코드 구현이 깔끔합니다.
ExpertCareerResult로부터의 변환 로직을 DTO 내부에from()정적 팩토리 메서드로 캡슐화한 것은 좋은 패턴입니다. 이렇게 하면 매핑 책임이 명확해지고, 어댑터 레이어에서 도메인 의존성을 제거할 수 있습니다.src/main/java/starlight/application/expert/provided/dto/ExpertDetailResult.java (1)
22-50: 전체적인 DTO 매핑 구조가 잘 설계되었습니다.
Expert도메인 엔티티에서ExpertDetailResult애플리케이션 DTO로의 변환을 정적 팩토리 메서드로 캡슐화한 것은 헥사고날 아키텍처에서 바람직한 패턴입니다. 도메인 모델과 외부 인터페이스 간의 결합도를 낮추는 데 기여합니다.src/main/java/starlight/application/expert/ExpertDetailQueryService.java (1)
17-40: 서비스 구현이 깔끔하고 아키텍처에 부합합니다.
ExpertDetailQueryService가UseCase인터페이스를 구현하고, 필요한 포트들을 조합하여 비즈니스 로직을 수행하는 구조가 헥사고날 아키텍처의 애플리케이션 레이어 패턴을 잘 따르고 있습니다.@Transactional(readOnly = true)적용과getOrDefault를 통한 안전한 기본값 처리도 적절합니다.src/main/java/starlight/adapter/expert/webapi/dto/ExpertDetailResponse.java (1)
32-57: DTO 매핑 구조 개선이 잘 되었습니다.
ExpertDetailResponse가 이제 도메인 엔티티(Expert) 대신 애플리케이션 레이어 DTO(ExpertDetailResult)를 입력으로 받도록 변경되었습니다. 이는 어댑터 레이어에서 도메인 의존성을 제거하고, 계층 간 결합도를 낮추는 헥사고날 아키텍처의 원칙에 부합합니다.
fromAllResults메서드명도 입력 타입 변경을 명확히 반영하고 있어 좋습니다.src/main/java/starlight/domain/expert/entity/Expert.java (1)
46-48:@ElementCollection에서@OneToMany로의 전환이 적절합니다.
careers를 단순 문자열 컬렉션에서ExpertCareer엔티티와의 양방향 관계로 변경한 것은 확장성과 데이터 무결성 측면에서 좋은 개선입니다.cascade = ALL과orphanRemoval = true설정으로 부모-자식 라이프사이클 관리가 적절히 구성되었습니다.다만, 기존 데이터 마이그레이션이 필요할 수 있으니 확인해주세요.
src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java (1)
22-24: 두ExpertLookupPort인터페이스의 역할과 구현 방식은 적절합니다검토 결과,
expertApplication의ExpertLookupPort와expertReport의ExpertLookupPort는 서로 다른 메서드 계약을 가지고 있습니다:
- expertApplication:
findById(Long id)- 기본 전문가 조회- expertReport:
findByIdWithDetails(Long id),findByIds(Set<Long> expertIds)- 상세 정보 및 배치 조회각 도메인 모듈이 필요로 하는 기능이 다르므로, 두 인터페이스를 하나의 어댑터로 구현하되 FQN으로 구분하는 현재 방식은 적절합니다. 추후 각 모듈의 요구사항이 크게 달라지면 분리를 검토하되, 지금 단계에서 통합할 필요는 없습니다.
src/main/java/starlight/application/expertReport/ExpertReportService.java (3)
29-41: 헥사고날 아키텍처 패턴 적용 LGTM
ExpertReportServiceUseCase인터페이스 구현과ExpertLookupPort를 통한 크로스 도메인 조회 분리가 잘 적용되었습니다. 소비 도메인에서 필요한 포트를 직접 정의하는 방식이 의존성 역전 원칙에 부합합니다.
85-93: 조회수 증가 및 Expert 조회 로직 확인
report.incrementViewCount()호출 후expertLookupPort를 통해 Expert를 조회하는 흐름이 적절합니다. 트랜잭션 내에서 처리되어 조회수 변경이 정상적으로 반영됩니다.
95-112: 배치 조회 패턴 적용 Good
findByIds를 사용한 배치 조회로 N+1 문제를 방지하고 있습니다.@Transactional(readOnly = true)적용도 적절합니다. 다만expertIds가 빈 Set일 경우findByIds호출이 불필요하므로, early return 추가를 고려해볼 수 있습니다.src/main/java/starlight/application/expertApplication/ExpertApplicationService.java (1)
30-36: UseCase/Port 패턴 일관성 Good
ExpertApplicationServiceUseCase구현과ExpertLookupPort,ExpertReportServiceUseCase의존성 주입이 PR 목표에 맞게 잘 적용되었습니다. 크로스 도메인 조회를 소비자 도메인의 포트로 분리한 점이 좋습니다.src/main/java/starlight/adapter/expertApplication/webapi/ExpertApplicationController.java (1)
23-24: Adapter 계층에서 UseCase 인터페이스 의존 Good컨트롤러가
ExpertApplicationQueryUseCase와ExpertApplicationServiceUseCase인터페이스에만 의존하도록 변경되어, 헥사고날 아키텍처의 포트-어댑터 분리가 잘 적용되었습니다.
src/main/java/starlight/adapter/expert/persistence/ExpertRepository.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/java/starlight/adapter/expert/persistence/ExpertRepository.java (1)
15-25: ID 기반 페치 전략 도입 확인
MultipleBagFetchException을 피하기 위한 ID 기반 페치 전략은 적절합니다. 각 컬렉션(careers, tags, categories)을 별도 쿼리로 페치하면 Hibernate가 영속성 컨텍스트에서 동일 엔티티를 병합합니다.다만,
findAllIds()의 확장성 문제는 이전 리뷰에서 지적된 바 있습니다. 전문가 수가 수천 건 이상으로 증가할 경우 페이지네이션 또는 스트리밍 방식을 고려해 주세요.
🧹 Nitpick comments (1)
src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java (1)
57-63: ID 기반 페치 패턴의 영속성 컨텍스트 의존성
fetchCareers,fetchTags,fetchCategories호출 후 첫 번째 쿼리 결과(experts)만 반환합니다. 이는 Hibernate 영속성 컨텍스트가 동일 ID의 엔티티를 자동 병합한다는 가정에 의존합니다.
@Transactional경계 내에서 동작하므로 정상 작동하지만, 이 패턴에 대한 주석을 추가하면 유지보수에 도움이 됩니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/starlight/adapter/expert/persistence/ExpertJpa.javasrc/main/java/starlight/adapter/expert/persistence/ExpertRepository.javasrc/main/java/starlight/adapter/expert/webapi/ExpertController.javasrc/main/java/starlight/adapter/expert/webapi/dto/ExpertDetailResponse.javasrc/main/java/starlight/adapter/expert/webapi/dto/ExpertListResponse.javasrc/main/java/starlight/adapter/expert/webapi/swagger/ExpertQueryApiDoc.javasrc/main/java/starlight/application/expert/ExpertDetailQueryService.javasrc/main/java/starlight/application/expert/provided/ExpertDetailQueryUseCase.javasrc/main/java/starlight/application/expert/required/ExpertQueryPort.javasrc/main/java/starlight/bootstrap/SecurityConfig.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/starlight/application/expert/ExpertDetailQueryService.java (1)
src/main/java/starlight/application/expertApplication/ExpertApplicationQueryService.java (1)
Service(11-22)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: build-and-push
🔇 Additional comments (13)
src/main/java/starlight/bootstrap/SecurityConfig.java (1)
74-74: 보안 설정은 현재 구조에 맞게 올바르게 구성되어 있습니다.ExpertController의 실제 엔드포인트는
/v1/experts(목록)과/v1/experts/{expertId}(상세) 두 가지뿐입니다. 현재 설정된"/v1/experts"와"/v1/experts/*"패턴은 이 두 엔드포인트를 완벽하게 커버하므로 추가 변경이 필요하지 않습니다.Likely an incorrect or invalid review comment.
src/main/java/starlight/application/expert/provided/ExpertDetailQueryUseCase.java (1)
9-14: 유스케이스 인터페이스 설계 적절헥사고날 아키텍처의 "provided" 포트로서
ExpertDetailQueryUseCase가 잘 정의되었습니다. 도메인 엔티티 대신ExpertDetailResultDTO를 반환하여 어댑터 계층과의 의존성을 깔끔하게 분리했습니다.src/main/java/starlight/application/expert/required/ExpertQueryPort.java (1)
9-16: 포트 네이밍 및 책임 분리 개선
ExpertQuery→ExpertQueryPort네이밍 변경으로 헥사고날 아키텍처 컨벤션을 준수합니다.findById와findExpertMapByIds가 제거되고 다른 도메인의LookupPort로 분리된 점도 적절합니다.src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java (1)
22-24: 다중 인터페이스 구현 시 import 고려FQN(Fully Qualified Name)으로 인터페이스를 선언하는 것은 동작하지만, 가독성을 위해 별칭(alias) import를 고려해 볼 수 있습니다. 현재 두
ExpertLookupPort가 동일한 이름을 가지므로 FQN 사용은 불가피합니다.src/main/java/starlight/adapter/expert/webapi/swagger/ExpertQueryApiDoc.java (1)
83-103: API 문서 개선 확인검색 API(
ExpertListResponse)와 상세 조회 API(ExpertDetailResponse)의 분리가 명확합니다. Swagger 예시에oneLineIntroduction,workedPeriod, 구조화된careers객체가 반영되어 프론트엔드 개발자에게 도움이 됩니다.src/main/java/starlight/application/expert/ExpertDetailQueryService.java (1)
42-48: 단일 조회 로직 적절
findById메서드가expertQueryPort.findByIdWithDetails를 호출하고 application count를 조회하여ExpertDetailResult로 변환하는 흐름이 명확합니다.src/main/java/starlight/adapter/expert/webapi/ExpertController.java (1)
22-38: 컨트롤러 유스케이스 패턴 적용 확인
ExpertFinder대신ExpertDetailQueryUseCase를 주입받아 사용하는 구조가 헥사고날 아키텍처의 "provided port" 패턴을 잘 따르고 있습니다. 컨트롤러가 얇게 유지되며 비즈니스 로직은 서비스 계층에 위임됩니다.
ExpertListResponse.fromAll()과ExpertDetailResponse.from()정적 팩토리 메서드를 통한 DTO 변환도 깔끔합니다.src/main/java/starlight/adapter/expert/webapi/dto/ExpertDetailResponse.java (2)
6-48: DTO 설계 및 엔티티 의존성 제거 확인
ExpertDetailResponse가 도메인 엔티티(Expert) 대신ExpertDetailResult를 입력으로 받도록 변경되어 어댑터 계층의 도메인 의존성이 제거되었습니다. 이는 PR 목표에 부합합니다.
careers타입이List<String>에서List<ExpertCareerResponse>로 변경되어 더 풍부한 정보(orderIndex, careerTitle)를 제공합니다.
50-54:fromAllResults메서드의 실제 사용 여부 확인 필요현재
ExpertController.search()에서는ExpertListResponse.fromAll()을 사용하고 있습니다. IDE의 검색 기능을 통해fromAllResults()메서드가 실제로 호출되는 곳이 있는지 확인해 주세요. 사용처가 없다면 불필요한 메서드이므로 제거를 권장합니다.src/main/java/starlight/adapter/expert/webapi/dto/ExpertListResponse.java (4)
8-18: 레코드 구조가 깔끔합니다.어댑터 레이어 DTO로서 적절한 구조를 가지고 있으며, 필드 명명도 명확합니다. 애플리케이션 레이어의 ExpertDetailResult를 웹 API 응답으로 변환하는 역할이 잘 분리되어 있습니다.
40-44: 리스트 매핑 로직이 간결합니다.
fromAll()메서드가 깔끔하게 구현되어 있습니다. 컨트롤러에서 이 메서드를 호출할 때 null 입력이 전달될 가능성이 있는지 확인하시면 좋습니다만, 일반적으로 컨트롤러 레이어에서 검증이 이루어지므로 현재 구현도 적절합니다.
46-56: 중첩 레코드 구조가 적절합니다.
ExpertCareerSummaryResponse를 중첩 레코드로 정의한 것은 좋은 설계입니다. 경력 요약 정보의 스코프가 명확하고, MAX_CAREERS 제한과 함께 리스트 응답의 의도를 잘 표현하고 있습니다.
21-38: null 안전성 검증 완료 - 우려사항 없음
ExpertDetailResult.careers()호출 시 NPE 발생 위험은 실제로 존재하지 않습니다.Expert엔티티에서careers필드가new ArrayList<>()로 초기화되므로 항상 null이 아닌 리스트를 반환합니다. 현재 코드는 안전합니다.Likely an incorrect or invalid review comment.
src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java
Outdated
Show resolved
Hide resolved
src/main/java/starlight/application/expert/ExpertDetailQueryService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
개발가이드.md (2)
35-41: 네이밍 규칙이 명확하나, LookupPort의 역할 구분이 더 명시되면 좋겠습니다.PR의 핵심인 Cross-domain LookupPort 패턴이 문서화되었습니다. 다만
*Port와OtherDomainLookupPort의 관계를 더 명확히 하면 개발자들이 어떤 상황에 LookupPort를 쓸지 판단하기 쉬워질 것 같습니다.예를 들어, 이런 식으로 보충하면 어떨까요:
*Port: 도메인 내부 또는 외부 시스템과의 계약OtherDomainLookupPort: 타 도메인의 엔티티/데이터 조회만 담당 (소비자 도메인에서 정의)
43-44: 개발자 온보딩에 실질적인 도움이 될 섹션입니다.로컬 개발 환경 구성을 위한 명확한 명령어가 제공되어 좋습니다. 추가로, dev 프로필에서 로드되는 데이터(테스트 데이터, 스키마 초기화 등)에 대해 한 줄 설명이 있으면 새 개발자들이 더 쉽게 이해할 것 같습니다.
src/main/java/starlight/domain/expert/entity/ExpertCareer.java (2)
36-45: 양방향 관계의 일관성을 유지하는 헬퍼 메서드 추가를 권장합니다.
of()팩토리 메서드가 모든 필드를 올바르게 초기화하고 있지만, 생성된ExpertCareer를Expert의careers컬렉션에 추가하지 않습니다. 이는 양방향 관계에서 흔히 발생하는 불일치 문제로, 플러시 전 메모리 상태가 일관되지 않을 수 있습니다.
Expert엔티티에 다음과 같은 헬퍼 메서드를 추가하는 것을 고려해보세요:// Expert.java에 추가 public void addCareer(ExpertCareer career) { this.careers.add(career); // career.expert는 이미 of()에서 설정됨 }또는 ExpertCareer 생성 시 양방향 관계를 모두 설정하도록 팩토리 메서드를 수정:
public static ExpertCareer of(Expert expert, int orderIndex, String title, String explanation, LocalDateTime startedAt, LocalDateTime endedAt) { ExpertCareer expertCareer = new ExpertCareer(); expertCareer.expert = expert; expertCareer.orderIndex = orderIndex; expertCareer.careerTitle = title; expertCareer.careerExplanation = explanation; expertCareer.careerStartedAt = startedAt; expertCareer.careerEndedAt = endedAt; expert.getCareers().add(expertCareer); // 양방향 관계 유지 return expertCareer; }
42-43: 날짜 유효성 검증 추가를 고려하세요.
careerStartedAt과careerEndedAt사이의 논리적 관계(시작일 ≤ 종료일)를 검증하지 않습니다. 잘못된 날짜 범위가 저장될 수 있습니다.🔎 제안하는 검증 로직
public static ExpertCareer of(Expert expert, int orderIndex, String title, String explanation, LocalDateTime startedAt, LocalDateTime endedAt) { + if (startedAt.isAfter(endedAt)) { + throw new IllegalArgumentException("Career start date must not be after end date"); + } ExpertCareer expertCareer = new ExpertCareer(); // ... 나머지 코드 } public void update(String title, String explanation, LocalDateTime startedAt, LocalDateTime endedAt) { + if (startedAt.isAfter(endedAt)) { + throw new IllegalArgumentException("Career start date must not be after end date"); + } this.careerTitle = title; // ... 나머지 코드 }Also applies to: 50-51
src/main/java/starlight/domain/expert/entity/Expert.java (2)
33-37:oneLineIntroduction필드에 길이 제약을 추가하는 것을 고려하세요."한 줄 소개"의 의도를 고려하면 적절한 최대 길이 제약을 추가하는 것이 좋습니다. 예를 들어:
@Column(length = 200) private String oneLineIntroduction;
detailedIntroduction의 경우, 긴 텍스트를 저장해야 한다면@Lob또는 적절한length설정을 고려하세요.
43-45: OneToMany 관계 설정이 적절합니다.
cascade = CascadeType.ALL과orphanRemoval = true조합으로 Expert와 ExpertCareer 간 생명주기가 올바르게 관리되며,@OrderBy를 통해 정렬 순서도 보장됩니다.다만, 양방향 관계를 안전하게 관리하기 위한 헬퍼 메서드(예:
addCareer())가 없습니다. ExpertCareer.java 리뷰에서 제안한 대로, Expert에 헬퍼 메서드를 추가하면 도메인 레이어에서 관계 일관성을 더 쉽게 유지할 수 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/starlight/adapter/expert/persistence/ExpertJpa.javasrc/main/java/starlight/domain/expert/entity/Expert.javasrc/main/java/starlight/domain/expert/entity/ExpertCareer.javasrc/test/java/starlight/adapter/expert/persistence/ExpertRepositoryTest.java개발가이드.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/starlight/adapter/expert/persistence/ExpertRepositoryTest.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-26T15:20:04.699Z
Learnt from: SeongHo5356
Repo: StartUpLight/STARLIGHT_BE PR: 67
File: src/main/java/starlight/adapter/expert/persistence/ExpertRepository.java:15-16
Timestamp: 2025-12-26T15:20:04.699Z
Learning: StarLight 플랫폼에서 전문가(Expert) 데이터는 최대 200명 수준으로 유지되므로, `findAllIds()`로 전체 ID를 메모리에 로드하는 방식이 적절하며 페이지네이션이나 스트리밍 처리가 필요하지 않습니다.
Applied to files:
src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java
🧬 Code graph analysis (1)
src/main/java/starlight/domain/expert/entity/ExpertCareer.java (1)
src/main/java/starlight/domain/expert/entity/Expert.java (1)
Getter(16-57)
⏰ 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). (2)
- GitHub Check: build-and-push
- GitHub Check: test
🔇 Additional comments (6)
개발가이드.md (1)
28-33: 아키텍처 지침이 PR 목표와 잘 정렬되었습니다.헥사고날 아키텍처의 핵심 원칙들(포트 의존성, 도메인 단위 유스케이스 노출, Cross-domain LookupPort 규칙)이 명확하게 정의되어 있습니다. 특히 31번 라인의 "Outbound 포트는 소비자 도메인에서 정의한다"는 원칙은 PR의 ExpertApplicationLookupPort 패턴을 강화합니다.
src/main/java/starlight/domain/expert/entity/ExpertCareer.java (1)
11-34: 엔티티 구조가 적절하게 설계되었습니다.JPA 매핑과 필드 제약조건이 올바르게 설정되어 있습니다. 과거 리뷰에서 지적된
expert와orderIndex필드 초기화 문제는 현재 코드의of()메서드(38-39번 라인)에서 해결되었습니다.src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java (4)
22-24: LookupPort 패턴을 통한 도메인 간 의존성 분리가 적절합니다.ExpertJpa가 여러 도메인의 LookupPort를 구현하여 각 도메인(ExpertReport, ExpertApplication)이 독립적인 포트 인터페이스를 통해 Expert 데이터를 조회할 수 있도록 했습니다. 이는 도메인 간 직접적인 유스케이스 의존성을 피하고 헥사고날 아키텍처 원칙을 잘 따르는 설계입니다.
55-62: 전체 조회 로직이 명확하고 적절합니다.
findAllIds()→fetchWithCollections()로 이어지는 2단계 조회 방식이 명확하며, 메서드명이 페치하는 컬렉션(Careers, Tags, Categories)을 정확히 반영하고 있습니다. 학습 내용에 따르면 Expert 데이터가 최대 200명 수준이므로 전체 ID를 메모리에 로드하는 방식이 적절합니다.
76-81: 메서드 리네이밍과 구현이 깔끔합니다.
findExpertMapByIds에서findByIds로 이름이 간결해졌고,repository.findAllByIds()를 활용한 구현이 명확합니다. Map 변환 로직도 표준적인 Stream API 패턴을 잘 따르고 있습니다.
83-88: 헬퍼 메서드 추출이 적절하며 다단계 페치 패턴이 명확합니다.반복되는 컬렉션 페치 로직을
fetchWithCollections로 잘 추출했습니다. careers 조회 후 tags와 categories를 순차적으로 페치하는 패턴이 명확하게 드러나며, 각 단계가 동일한 Expert 엔티티 인스턴스를 기준으로 연관 컬렉션을 채우는 방식으로 동작합니다.참고로, 이 패턴은 JPA 영속성 컨텍스트에서 엔티티 동일성이 유지되는 것을 전제로 합니다. Repository의
fetchExpertsWithTagsByIds와fetchExpertsWithCategoriesByIds가 line 84에서 로드된 Expert 엔티티들을 올바르게 변경하는지 확인이 필요하지만, 전체 아키텍처 컨텍스트상 적절한 설계로 보입니다.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/starlight/adapter/expert/webapi/ExpertControllerTest.java (1)
44-96: 테스트 데이터와 기대값 간 경력 개수 불일치를 수정하세요.Lines 55, 77, 95에서
careers.length()값으로 3을 기대하고 있으나, 헬퍼 메서드expertResult()는 4개의 경력을 생성합니다 (lines 116-121). 이로 인해 테스트가 실패합니다.PR 설명에 따르면 목록 응답에서는 최대 3개의 경력만 노출한다고 명시되어 있으므로, 실제 응답 DTO에서 경력을 3개로 제한하는 로직이 구현되어 있을 것으로 추정됩니다. 하지만 테스트에서는 이 변환 로직을 거치지 않고
ExpertDetailResult를 직접 반환하므로 4개 모두 포함됩니다.해결 방안:
- 헬퍼 메서드를 3개 경력만 생성하도록 수정하거나
- 테스트 기대값을 4로 수정하되, 실제 컨트롤러/서비스 레이어에서 3개로 제한하는 로직이 올바르게 동작하는지 별도로 검증해야 합니다.
🔎 제안하는 수정 (방안 1: 헬퍼를 3개로 제한)
private ExpertDetailResult expertResult(Long id, String name, Set<TagCategory> cats) throws Exception { List<ExpertCareerResult> careers = List.of( new ExpertCareerResult(1L, 0, "A사 PO", "설명", null, null), new ExpertCareerResult(2L, 1, "B사 PM", "설명", null, null), - new ExpertCareerResult(3L, 2, "C사 리드", "설명", null, null), - new ExpertCareerResult(4L, 3, "D사 CTO", "설명", null, null) + new ExpertCareerResult(3L, 2, "C사 리드", "설명", null, null) );
🧹 Nitpick comments (3)
src/test/java/starlight/adapter/expert/webapi/ExpertControllerTest.java (3)
40-42: [선택적] MockitoBean 선언 간 공백 제거를 고려하세요.Line 42의
JpaMetamodelMappingContext선언 전 불필요한 빈 줄이 있습니다. 일관성을 위해 제거를 고려해보세요.🔎 제안하는 수정
@MockitoBean ExpertDetailQueryUseCase expertDetailQuery; - @MockitoBean JpaMetamodelMappingContext jpaMetamodelMappingContext;
98-112: 상세 조회 테스트의 검증 범위를 확대하는 것을 고려하세요.신규 추가된
detail()테스트는 기본적인 필드 검증을 수행하고 있으나, 다음 필드들에 대한 추가 검증을 고려해보세요:
name,introduction,description등 주요 전문가 정보careers배열의 존재 여부 및 구조imageUrl,hourlyRate등 상세 필드Line 110의
categories.doesNotExist()검증은 학습된 내역에 따라 의도적으로 제외된 것이 맞습니다.🔎 제안하는 추가 검증
mockMvc.perform(get("/v1/experts/10")) .andExpect(status().isOk()) .andExpect(jsonPath("$.result").value("SUCCESS")) .andExpect(jsonPath("$.data.id").value(10L)) + .andExpect(jsonPath("$.data.name").value("김철수")) + .andExpect(jsonPath("$.data.introduction").value("한줄소개")) + .andExpect(jsonPath("$.data.careers").isArray()) + .andExpect(jsonPath("$.data.careers.length()").value(4)) .andExpect(jsonPath("$.data.applicationCount").value(0)) .andExpect(jsonPath("$.data.categories").doesNotExist()) .andExpect(jsonPath("$.data.tags").isArray());참고:
careers.length()값은 위의 경력 개수 불일치 이슈가 해결된 후 적절히 조정하세요.
115-137: [선택적] 불필요한 throws 선언을 제거하세요.헬퍼 메서드
expertResult의 시그니처에throws Exception이 선언되어 있으나, 메서드 내부에서 체크 예외를 발생시키는 코드가 없습니다.🔎 제안하는 수정
-private ExpertDetailResult expertResult(Long id, String name, Set<TagCategory> cats) throws Exception { +private ExpertDetailResult expertResult(Long id, String name, Set<TagCategory> cats) { List<ExpertCareerResult> careers = List.of(
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/starlight/StarlightApplication.javasrc/main/java/starlight/adapter/order/persistence/OrderRepositoryJpa.javasrc/main/java/starlight/adapter/order/webapi/OrderController.javasrc/main/java/starlight/bootstrap/ObjectStorageConfig.javasrc/main/java/starlight/bootstrap/SwaggerConfig.javasrc/test/java/starlight/adapter/expert/webapi/ExpertControllerTest.java
💤 Files with no reviewable changes (5)
- src/main/java/starlight/bootstrap/ObjectStorageConfig.java
- src/main/java/starlight/adapter/order/webapi/OrderController.java
- src/main/java/starlight/StarlightApplication.java
- src/main/java/starlight/bootstrap/SwaggerConfig.java
- src/main/java/starlight/adapter/order/persistence/OrderRepositoryJpa.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-26T15:38:51.383Z
Learnt from: SeongHo5356
Repo: StartUpLight/STARLIGHT_BE PR: 67
File: src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java:36-51
Timestamp: 2025-12-26T15:38:51.383Z
Learning: StarLight의 Expert 조회에서 `findByIdWithCareersAndTags()` 메서드는 단건 상세 조회용으로, ExpertDetailResponse가 categories 필드를 포함하지 않기 때문에 의도적으로 categories 페치를 제외합니다. ExpertListResponse만 categories를 포함합니다.
Applied to files:
src/test/java/starlight/adapter/expert/webapi/ExpertControllerTest.java
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: build-and-push
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java (2)
66-84: Line 77의 반환값 무시가 혼란을 줄 수 있습니다.
fetchWithCollections(ids)를 호출하지만 반환값을 사용하지 않고 있습니다. JPA의 managed entity 특성상 메서드가 side-effect로 컬렉션을 채우기 때문에 기능적으로는 정상이지만, 코드 리더에게 혼란을 줄 수 있습니다.다음 중 하나를 고려해보세요:
Option 1: 명시적으로 변수에 할당하여 의도 표현
List<Expert> fetchedExperts = fetchWithCollections(ids); // fetchedExperts는 experts와 동일한 managed 인스턴스 return experts;Option 2: void 반환 메서드 추가로 의도 명확화
// ExpertJpa에 추가 private void enrichWithCollections(List<Long> ids) { repository.fetchExpertsWithCareersByIds(ids); repository.fetchExpertsWithTagsByIds(ids); repository.fetchExpertsWithCategoriesByIds(ids); }Option 3: 간단한 주석 추가
fetchWithCollections(ids); // Side-effect: managed entities에 컬렉션 로드
22-24: 다중 도메인 포트 구현은 프로젝트의 의도된 패턴으로 보입니다.분석 결과,
ExpertJpa가 다중 도메인의 포트를 구현하는 것은 이 프로젝트의 표준 아키텍처 패턴입니다:
ExpertApplicationJpa도 동일하게 2개의 포트를 구현하고 있습니다 (ExpertApplicationQuery+ExpertApplicationLookupPort)expertReport도메인의ExpertReportService는expertReport.ExpertLookupPort만 의존합니다expertApplication도메인의ExpertApplicationService는expertApplication.ExpertLookupPort만 의존합니다- 각 도메인은 자신의 포트 인터페이스를 통해서만 어댑터와 상호작용합니다
따라서 현재 구조는 헥사고날 아키텍처의 원칙을 잘 따르고 있습니다 (도메인은 포트를 정의하고, 포트의 실제 구현은 어댑터에 은닉됨).
다만, 제안하신 사항이 여전히 타당할 수 있습니다:
- 두
ExpertLookupPort인터페이스가 동일한 이름을 가지므로, 유지보수 시 혼란을 줄 수 있습니다- 패키지 경로로만 구분되는 현재 상태가 명확한지 검토할 가치가 있습니다
선택사항:
- 현재 패턴 유지: 팀의 표준이 이것이라면, 코드는 정상입니다
- 도메인별 포트명 차별화:
ExpertReportLookupPort,ExpertApplicationLookupPort같이 도메인 프리픽스를 추가하면 더 명확할 것 같습니다
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-26T15:38:51.383Z
Learnt from: SeongHo5356
Repo: StartUpLight/STARLIGHT_BE PR: 67
File: src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java:36-51
Timestamp: 2025-12-26T15:38:51.383Z
Learning: StarLight의 Expert 조회에서 `findByIdWithCareersAndTags()` 메서드는 단건 상세 조회용으로, ExpertDetailResponse가 categories 필드를 포함하지 않기 때문에 의도적으로 categories 페치를 제외합니다. ExpertListResponse만 categories를 포함합니다.
Applied to files:
src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java
📚 Learning: 2025-12-26T15:20:04.699Z
Learnt from: SeongHo5356
Repo: StartUpLight/STARLIGHT_BE PR: 67
File: src/main/java/starlight/adapter/expert/persistence/ExpertRepository.java:15-16
Timestamp: 2025-12-26T15:20:04.699Z
Learning: StarLight 플랫폼에서 전문가(Expert) 데이터는 최대 200명 수준으로 유지되므로, `findAllIds()`로 전체 ID를 메모리에 로드하는 방식이 적절하며 페이지네이션이나 스트리밍 처리가 필요하지 않습니다.
Applied to files:
src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: build-and-push
🔇 Additional comments (4)
src/main/java/starlight/adapter/expert/persistence/ExpertJpa.java (4)
36-51: 구현이 올바릅니다.단건 상세 조회 시 categories를 의도적으로 제외한 것이
ExpertDetailResponse의 DTO 구조와 일치합니다. 에러 핸들링과 로깅도 적절하게 구현되어 있습니다.
- ✅ 3-query 전략으로 N+1 문제 방지
- ✅ ExpertException 재throw 및 일반 예외 래핑 적절
- ✅ 빈 결과에 대한 명확한 예외 처리
Based on learnings, ExpertDetailResponse는 categories 필드를 포함하지 않으므로 현재 구현이 정확합니다.
94-99: 3-query 전략 구현이 깔끔합니다.카테시안 곱 문제를 해결하기 위한 split collection loading 전략이 명확하게 구현되었습니다:
- 1차: careers 페치
- 2차: tags 페치
- 3차: categories 페치
JPA의 persistence context 덕분에 3번의 쿼리가 동일한 Expert 인스턴스에 컬렉션을 채우게 되므로, 성능과 데이터 일관성을 모두 확보한 좋은 접근입니다.
PR에서 언급하신 Notion 문서의 fetch 전략 가이드라인과도 일치하는 구현이네요!
55-63: 목록 조회 로직이 명확하고 효율적입니다.전체 ID를 먼저 조회한 후
fetchWithCollections로 필요한 컬렉션을 로드하는 2단계 접근이 깔끔합니다.Based on learnings, StarLight 플랫폼의 Expert 데이터는 최대 200명 수준이므로
findAllIds()로 전체 ID를 메모리에 로드하는 현재 전략이 적절합니다.
87-92: 메서드 리네이밍과 구현이 개선되었습니다.
findExpertMapByIds→findByIds로 메서드명이 더 간결해졌고, Stream API를 활용한 Map 변환도 깔끔합니다.
| public class ExpertJpa implements ExpertQuery { | ||
| public class ExpertJpa implements ExpertQueryPort, | ||
| starlight.application.expertReport.required.ExpertLookupPort, | ||
| starlight.application.expertApplication.required.ExpertLookupPort { |
There was a problem hiding this comment.
패키지 경로 떼고 import 하면 좋을 것 같은데 혹시 의도된걸까용
There was a problem hiding this comment.
동일한 이름의 LookupPort가 두 패키지에 있어서 충돌 때문에 FQN을 사용했습니다. 현재 저희팀 컨벤션 상, FQN을 유지하는 방법이 최선일 것 같은데, 네이밍 자체를 변경하는 게 좋을까요 ?
@2ghrms
#️⃣연관된 이슈
📝작업 내용
🔎코드 설명
💬고민사항 및 리뷰 요구사항
비고 (Optional)
Summary by CodeRabbit
새로운 기능
개선 사항
문서
✏️ Tip: You can customize this high-level summary in your review settings.