feat: 마이페이지 응답 요소 추가 #437
feat: 마이페이지 응답 요소 추가 #437
Conversation
- 멘티의 경우는 관심 국가 어디인지 - 멘토의 경우는 수학 대학교 어디인지
Gyuhyeok99
requested review from
lsy1307,
nayonsoso,
whqtker and
wibaek
as code owners
Walkthrough1. CountryRepository에 Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 1
🧹 Nitpick comments (7)
src/main/java/com/example/solidconnection/location/country/repository/CountryRepository.java (1)
14-23: 응답 순서를 보장하기 위한 ORDER BY 추가 제안테스트/클라이언트 일관성을 위해 정렬을 명시해두면 좋습니다. 현재 DISTINCT만 있으므로 다건일 때 순서가 비결정적일 수 있습니다.
- 선택지: 한글명 오름차순 정렬을 추가합니다.
- 효과: 테스트 안정성과 API 일관성 향상.
다음 변경을 제안합니다:
@Query(""" SELECT DISTINCT c.koreanName FROM Country c WHERE c.code IN ( SELECT ic.countryCode FROM InterestedCountry ic WHERE ic.siteUserId = :siteUserId ) - """) + ORDER BY c.koreanName ASC + """) List<String> findKoreanNamesBySiteUserId(@Param("siteUserId") long siteUserId);src/main/java/com/example/solidconnection/siteuser/dto/MyPageResponse.java (2)
22-26: NULL만 제외할지, 빈 리스트도 제외할지 정책 결정 필요멘티의 관심국가가 빈 리스트일 때도 필드를 보내지 않으려면 NON_EMPTY가 더 적합합니다. 현재는 NON_NULL이라 빈 리스트는 응답에 포함됩니다.
- 현행 유지: 빈 리스트도 응답에 포함.
- 변경 제안: 빈 리스트 비노출(NON_EMPTY)로 페이로드 최소화.
다음과 같이 변경을 고려해 주세요:
- @JsonInclude(JsonInclude.Include.NON_NULL) + @JsonInclude(JsonInclude.Include.NON_EMPTY) List<String> interestedCountries, @JsonInclude(JsonInclude.Include.NON_NULL) String attendedUniversity) {
28-40: 불변성 강화: 리스트 방어적 복사 권장record라도 List는 가변이므로, 생성 시 방어적 복사로 내부 불변성을 보장하는 것이 안전합니다.
- 장점: 외부에서 전달된 리스트 변경이 DTO 내부로 전파되는 것을 차단.
- 영향: 직렬화/역직렬화에는 변화 없음.
레코드의 compact constructor를 추가해 주세요(선택):
// 변경 제안: 레코드 본문에 compact constructor 추가 public record MyPageResponse( /* 기존 필드들 */ ) { public MyPageResponse { if (interestedCountries != null) { interestedCountries = List.copyOf(interestedCountries); } } public static MyPageResponse of(SiteUser siteUser, int likedUnivApplyInfoCount, List<String> interestedCountries, String attendedUniversity) { // 기존 구현 유지 return new MyPageResponse( siteUser.getNickname(), siteUser.getProfileImageUrl(), siteUser.getRole(), siteUser.getAuthType(), siteUser.getEmail(), 0, 0, likedUnivApplyInfoCount, interestedCountries, attendedUniversity ); } }src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (1)
55-65: switch 표현식으로 Role 분기 리팩토링 제안
- switch 표현식 도입
MENTEE와 MENTOR 분기를 깔끔하게 구분해 가독성을 높입니다.- 기본(default) 처리
ADMIN 및 기타 Role에 대한 기본 MyPageResponse를 반환해 누락 케이스를 방지합니다.- 확장성 강화
새로운 Role이 추가될 때 컴파일러 경고를 통해 누락된 케이스를 알려줍니다.예시:
var baseResponse = MyPageResponse.of(siteUser, likedUnivApplyInfoCount, null, null); return switch (siteUser.getRole()) { case MENTEE -> { var countries = countryRepository.findKoreanNamesBySiteUserId(siteUser.getId()); yield MyPageResponse.of(siteUser, likedUnivApplyInfoCount, countries, null); } case MENTOR -> { Mentor mentor = mentorRepository.findBySiteUserId(siteUser.getId()) .orElseThrow(() -> new CustomException(MENTOR_NOT_FOUND)); String univName = universityRepository.findById(mentor.getUniversityId()) .orElseThrow(() -> new CustomException(UNIVERSITY_NOT_FOUND)) .getKoreanName(); yield MyPageResponse.of(siteUser, likedUnivApplyInfoCount, null, univName); } default -> baseResponse; };위 리팩토링은 선택 사항이지만, 역할이 늘어날 때 안정성과 가독성을 크게 높여 줄 것입니다.
src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (3)
86-109: 멘티 케이스: 다건/정렬 케이스와 빈 리스트 케이스까지 보강 제안현재 단일 관심국가만 검증합니다. 다건/정렬, 빈 리스트 시나리오까지 추가하면 회귀에 강해집니다.
- 다건/정렬: 관심국가가 여러 개인 경우 정렬 정책(예: 한글명 오름차순)을 검증합니다.
- 빈 리스트: 관심국가가 없을 때 응답 필드가 빈 리스트인지(null 제외) 혹은 비노출(정책에 따라)을 검증합니다.
예시 검증 아이디어:
- 다건일 때: assertThat(response.interestedCountries()).isSortedAccordingTo(Comparator.naturalOrder());
- 또는: containsExactly("국가A", "국가B") 형태로 결정적 순서 검증.
111-135: 멘토 케이스: 에러 경로 테스트(멘토/대학 누락) 추가 권장정상 플로우는 좋습니다. 예외 플로우(멘토 엔티티 부재, 대학 부재)를 함께 검증하면 커버리지가 더 탄탄해집니다.
- 멘토 없음: mentorRepository.findBySiteUserId가 empty인 경우 MENTOR_NOT_FOUND 확인.
- 대학 없음: mentor.universityId가 잘못된 경우 UNIVERSITY_NOT_FOUND 확인.
원하시면 위 두 케이스를 포함한 테스트 메서드 골격을 생성해 드리겠습니다.
136-146: 테스트 유틸화: 좋아요 지원정보 생성 헬퍼의 재사용성 극대화createLikedUnivApplyInfos는 유용합니다. 공용 테스트 유틸 클래스로 승격하면 다른 테스트도 중복을 줄일 수 있습니다.
- 장점: 중복 제거, 유지보수성 향상.
- 영향: 다른 테스트 클래스에서도 손쉽게 재사용.
필요 시 support 모듈/패키지의 TestUtils로 이동을 고려해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/example/solidconnection/location/country/repository/CountryRepository.java(1 hunks)src/main/java/com/example/solidconnection/siteuser/dto/MyPageResponse.java(3 hunks)src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java(3 hunks)src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java(5 hunks)
⏰ 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: build
🔇 Additional comments (1)
src/main/java/com/example/solidconnection/location/country/repository/CountryRepository.java (1)
14-23: 필드 검증 완료
필드 존재 여부 확인
-InterestedCountry엔티티에countryCode및siteUserId필드가 실제로 선언되어 있습니다.
-Country엔티티에code필드가 선언되어 있습니다.결론
- JPQL 서브쿼리에 사용된 엔티티 및 필드명이 모두 매핑과 완벽히 일치합니다.
- 별도의 수정이나 추가 검증 없이 안전하게 배포 가능합니다.
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java
Outdated
Show resolved
Hide resolved
nayonsoso
left a comment
nayonsoso
left a comment
There was a problem hiding this comment.
@JsonInclude(JsonInclude.Include.NON_NULL)
정말 유용한 어노테이션을 배워갑니다, 감사드려요 😁
저는 지금까지 dto interface를 사용해서 하나의 api 에서 다른 응답이 나올 수 있게 했는데
이 어노테이션을 사용하는게 더 좋았겠네요..^^ㅠ
| int likedUnivApplyInfoCount, | ||
|
|
||
| public static MyPageResponse of(SiteUser siteUser, int likedUnivApplyInfoCount) { | ||
| @JsonInclude(JsonInclude.Include.NON_NULL) |
There was a problem hiding this comment.
import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL; 처럼
static import 를 추가하여 @JsonInclude(JsonInclude.Include.NON_NULL)를 @JsonInclude(NON_NULL)처럼 간단히 만드는건 어떨까요?
이렇게 하면 한눈에 더 잘 들어올 것 같습니다😊
|
|
||
| public static MyPageResponse of(SiteUser siteUser, int likedUnivApplyInfoCount) { | ||
| @JsonInclude(JsonInclude.Include.NON_NULL) | ||
| List<String> interestedCountries, |
There was a problem hiding this comment.
List<String> 를 응답하는 부분에서 고민되긴 하네요🤔
ui 상으로는 그냥 국가 이름 하나만 있어서요.
게시물에서 첫번째 이미지의 썸네일만 내려주는 것처럼 하나만 내려줘야할지? 아니면 목록으로 다 내려줘야할지...
개인적인 판단으로는 지금처럼 List<String> 으로 전달하여 전체를 다 보여주는게 좋을 것 같긴한데, 기획팀과 이야기가 되어야 할 것 같습니다.
혹시 이와 관련된 내용이 이야기된적이 있었던가요..?
There was a problem hiding this comment.
저도 처음엔 String으로만 하려고 했으나... 디비엔 List로 들어가있어서 영서님 생각처럼 다 보여주는 게 나을 거 같다고 생각했습니다..
기획팀에 따로 이야기는 안했어서 여쭤보겠습니다!
| MyPageResponse myPageResponse = null; | ||
| if (siteUser.getRole() == Role.MENTEE) { | ||
| List<String> interestedCountries = countryRepository.findKoreanNamesBySiteUserId(siteUser.getId()); | ||
| myPageResponse = MyPageResponse.of(siteUser, likedUnivApplyInfoCount, interestedCountries, null); | ||
| } | ||
| else if (siteUser.getRole() == Role.MENTOR) { | ||
| Mentor mentor = mentorRepository.findBySiteUserId(siteUser.getId()) | ||
| .orElseThrow(() -> new CustomException(MENTOR_NOT_FOUND)); | ||
| University university = universityRepository.findById(mentor.getUniversityId()) | ||
| .orElseThrow(() -> new CustomException(UNIVERSITY_NOT_FOUND)); | ||
| myPageResponse = MyPageResponse.of(siteUser, likedUnivApplyInfoCount, null, university.getKoreanName()); | ||
| } | ||
| return myPageResponse; |
There was a problem hiding this comment.
| MyPageResponse myPageResponse = null; | |
| if (siteUser.getRole() == Role.MENTEE) { | |
| List<String> interestedCountries = countryRepository.findKoreanNamesBySiteUserId(siteUser.getId()); | |
| myPageResponse = MyPageResponse.of(siteUser, likedUnivApplyInfoCount, interestedCountries, null); | |
| } | |
| else if (siteUser.getRole() == Role.MENTOR) { | |
| Mentor mentor = mentorRepository.findBySiteUserId(siteUser.getId()) | |
| .orElseThrow(() -> new CustomException(MENTOR_NOT_FOUND)); | |
| University university = universityRepository.findById(mentor.getUniversityId()) | |
| .orElseThrow(() -> new CustomException(UNIVERSITY_NOT_FOUND)); | |
| myPageResponse = MyPageResponse.of(siteUser, likedUnivApplyInfoCount, null, university.getKoreanName()); | |
| } | |
| return myPageResponse; | |
| List<String> interestedCountries = null; | |
| String universityKoreanName = null; | |
| if (siteUser.getRole() == Role.MENTEE) { | |
| interestedCountries = countryRepository.findKoreanNamesBySiteUserId(siteUser.getId()); | |
| } else if (siteUser.getRole() == Role.MENTOR) { | |
| Mentor mentor = mentorRepository.findBySiteUserId(siteUser.getId()) | |
| .orElseThrow(() -> new CustomException(MENTOR_NOT_FOUND)); | |
| University university = universityRepository.findById(mentor.getUniversityId()) | |
| .orElseThrow(() -> new CustomException(UNIVERSITY_NOT_FOUND)); | |
| universityKoreanName = university.getKoreanName(); | |
| } | |
| return MyPageResponse.of(siteUser, likedUnivApplyInfoCount, interestedCountries, universityKoreanName); |
이렇게 하면 MyPageResponse 인스턴스에 대한 재할당이 없어질 것 같습니다~
|
@JsonInclude(JsonInclude.Include.NON_NULL)는 저도 이번에 새롭게 알게된 어노테이션인데 알게된김에 사용해봤습니다 ㅎㅎ |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (1)
76-81: 동일 닉네임 재요청 시 불필요한 예외 가능성 제거.
- 현재 로직은 닉네임이 동일해도 유니크 검증과 쿨다운 검증을 수행하여, 본인 닉네임으로도 NICKNAME_ALREADY_EXISTED가 날 수 있습니다. 2) 또한 변경 의사가 없는 요청에도 쿨다운 제한이 적용됩니다. 3) 동일 값이면 스킵하도록 가드 조건을 추가해 주세요.
다음과 같이 수정 제안드립니다.
- if (nickname != null) { + if (nickname != null && !nickname.equals(user.getNickname())) { validateNicknameNotChangedRecently(user.getNicknameModifiedAt()); validateNicknameUnique(nickname); user.setNickname(nickname); user.setNicknameModifiedAt(LocalDateTime.now()); }추가로 Repository에 existsByNicknameAndIdNot 같은 메서드가 있으면 사용하는 것도 안전합니다.
🧹 Nitpick comments (5)
src/main/java/com/example/solidconnection/siteuser/dto/MyPageResponse.java (1)
12-28: 역할별 DTO 분리 고려(선택).
- 도메인 확장으로 역할별 필드가 더 늘어날 경우, 단일 DTO의 조건부 필드가 복잡도를 올립니다. 2) MyPageResponseMentee / MyPageResponseMentor 같은 뷰 모델로 분리하면 타입 안정성과 스키마 명확성이 좋아집니다.
원하시면 두 DTO와 어댑터 메서드 ofMentee/ofMentor 스케치 코드를 드리겠습니다.
src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (2)
97-107: 검증 표현 개선(가독성): 리스트 첫 원소 비교 대신 전체 일치 검증.
- 현재는 index 기반 비교로 의도가 드러나지 않습니다. 2) containsExactly를 사용하면 기대 리스트를 명확히 표현할 수 있습니다.
다음으로 교체를 제안합니다.
- () -> assertThat(response.interestedCountries().get(0)).isEqualTo(country.getKoreanName()), + () -> assertThat(response.interestedCountries()).containsExactly(country.getKoreanName()),
110-133: 역할 외 시나리오 테스트 추가 제안(선택).
- ADMIN 등 MENTEE/MENTOR 이외 역할에서 두 필드가 null로 반환되는지(또는 예외 처리되는지) 행태를 고정하면 회귀를 막을 수 있습니다. 2) 간단한 픽스처로 사용자 역할만 바꾼 테스트를 하나 더 추가하는 것을 권장합니다.
원하시면 테스트 템플릿 코드를 제공하겠습니다.
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (2)
54-65: 역할 분기 가독성 개선(선택).
- if/else-if 대신 switch(Role) 사용 시 분기 확장이 쉬워집니다. 2) default에서 명시적으로 아무 것도 세팅하지 않음을 드러내면 의도가 더 명확해집니다.
예시:
- switch (siteUser.getRole()) { case MENTEE: …; break; case MENTOR: …; break; default: /* no-op */ }
110-113: 메서드 의도 명확화(선택): 이중 부정 제거.
- isDefaultProfileImage + !isDefaultProfileImage(...)는 읽기 비용이 큽니다. 2) hasCustomProfileImage로 네이밍을 바꾸고 조건을 정극화하면 가독성이 좋아집니다.
다음과 같이 변경을 제안합니다.
- private boolean isDefaultProfileImage(String profileImageUrl) { - String prefix = "profile/"; - return profileImageUrl == null || !profileImageUrl.startsWith(prefix); - } + private boolean hasCustomProfileImage(String profileImageUrl) { + String prefix = "profile/"; + return profileImageUrl != null && profileImageUrl.startsWith(prefix); + }- if (!isDefaultProfileImage(user.getProfileImageUrl())) { + if (hasCustomProfileImage(user.getProfileImageUrl())) { s3Service.deleteExProfile(user.getId()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/main/java/com/example/solidconnection/location/country/repository/CountryRepository.java(1 hunks)src/main/java/com/example/solidconnection/siteuser/dto/MyPageResponse.java(3 hunks)src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java(3 hunks)src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/example/solidconnection/location/country/repository/CountryRepository.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T21:43:45.760Z
Learnt from: nayonsoso
PR: solid-connection/solid-connect-server#448
File: src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java:118-121
Timestamp: 2025-08-12T21:43:45.760Z
Learning: SiteUser 도메인의 updatePassword 메서드에서는 파라미터 이름을 newEncodedPassword로 하거나 Javadoc을 추가해서 인코딩된 비밀번호가 들어와야 한다는 것을 명시해야 합니다.
Applied to files:
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java
🔇 Additional comments (3)
src/main/java/com/example/solidconnection/siteuser/dto/MyPageResponse.java (3)
24-28: 역할별 필드 분리에 @JsonInclude(NON_NULL) 적용 좋습니다.
- 멘티/멘토 구분이 직관적입니다. 2) 불필요한 필드가 응답에서 빠져 페이로드가 줄어듭니다. 3) FE 단에서 조건 분기하기에도 편리합니다.
21-23: JSON 프로퍼티 이름 확인: likedUniversityCount 사용이 호환 요구사항과 일치하는지 점검해 주세요.
- 기존 공개 API가 likedUnivApplyInfoCount였는지, likedUniversityCount였는지에 따라 브레이킹 변경이 될 수 있습니다. 2) 문서/FE 의존성도 함께 확인해 주세요.
필요 시 API 문서/프론트 코드 점검을 도와드릴게요.
30-43: 정적 팩토리 시그니처 확장 — 호출부 점검 완료 (조치 불필요)레포지토리 전체 검색 결과 2-파라미터 호출(MyPageResponse.of(siteUser, ...))은 없습니다.
대신 4-파라미터 호출 1건이 확인되었습니다.
따라서 컴파일 실패 우려는 해소되었습니다.
호출 위치 확인
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java:65: return MyPageResponse.of(siteUser, likedUnivApplyInfoCount, interestedCountries, universityKoreanName).권장 조치
추가 수정 불필요합니다.
src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (1)
60-68: 조건 분기를 switch로 단순화 (선택)
- if/else 체인을 switch로 변환하면 역할 추가 시 확장성이 좋아집니다.
- default 블록을 활용해 명시적으로 "추가 필드 없음"을 드러낼 수 있습니다.
참고 스니펫:
switch (siteUser.getRole()) { case MENTEE -> interestedCountries = countryRepository.findKoreanNamesBySiteUserId(siteUser.getId()); case MENTOR -> { Mentor mentor = mentorRepository.findBySiteUserId(siteUser.getId()) .orElseThrow(() -> new CustomException(MENTOR_NOT_FOUND)); Long universityId = mentor.getUniversityId(); if (universityId == null) throw new CustomException(UNIVERSITY_NOT_FOUND); University university = universityRepository.findById(universityId) .orElseThrow(() -> new CustomException(UNIVERSITY_NOT_FOUND)); universityKoreanName = university.getKoreanName(); } default -> { /* 추가 필드 없음 */ } }src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (2)
91-112: 멘티 경로 테스트의 단언을 더 견고하게 개선 제안현재 첫 요소만 검사(get(0))하고 있어 다건/정렬 이슈를 잡기 어렵습니다. containsExactly로 단언을 강화하는 것을 권장합니다.
- () -> assertThat(response.likedUnivApplyInfoCount()).isEqualTo(likedUnivApplyInfoCount), - () -> assertThat(response.interestedCountries().get(0)).isEqualTo(country.getKoreanName()), + () -> assertThat(response.likedUnivApplyInfoCount()).isEqualTo(likedUnivApplyInfoCount), + () -> assertThat(response.interestedCountries()) + .containsExactly(country.getKoreanName()), () -> assertThat(response.attendedUniversity()).isNull()
91-112: JSON 직렬화 수준에서 NON_NULL 동작 검증 추가 (선택)현재는 DTO 접근자 수준 검증입니다. @JsonInclude(NON_NULL) 동작을 보장하려면 컨트롤러/직렬화 통합 테스트를 1건 추가하는 걸 권장합니다.
참고 스니펫(WebMvcTest 예시):
@AutoConfigureMockMvc @WebMvcTest(MyPageController.class) class MyPageControllerJsonTest { @Autowired MockMvc mvc; @Test void 멘티_JSON_응답에는_attendedUniversity가_포함되지_않는다() throws Exception { // when & then mvc.perform(get("/api/v1/mypage").with(user("mentee").roles("MENTEE"))) .andExpect(status().isOk()) .andExpect(jsonPath("$.interestedCountries").isArray()) .andExpect(jsonPath("$.attendedUniversity").doesNotExist()); } @Test void 멘토_JSON_응답에는_interestedCountries가_포함되지_않는다() throws Exception { mvc.perform(get("/api/v1/mypage").with(user("mentor").roles("MENTOR"))) .andExpect(status().isOk()) .andExpect(jsonPath("$.attendedUniversity").isString()) .andExpect(jsonPath("$.interestedCountries").doesNotExist()); } }Also applies to: 115-138
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/java/com/example/solidconnection/location/country/repository/CountryRepository.java(1 hunks)src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java(3 hunks)src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/example/solidconnection/location/country/repository/CountryRepository.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). (1)
- GitHub Check: build
🔇 Additional comments (4)
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (2)
44-46: 의존성 주입 구성 깔끔: 역할별 데이터 소스 분리 적절
- CountryRepository, MentorRepository, UniversityRepository를 서비스에 명확히 주입해 역할별 조회 경로가 분리되었습니다.
- 가독성과 변경 용이성이 좋아졌고, 테스트 대역 구성에도 유리합니다.
58-69: 역할별 분기 + NON_NULL 응답 전략이 요구사항에 정확히 부합
- MENTEE일 때 interestedCountries만, MENTOR일 때 attendedUniversity만 채워서 반환합니다.
- 나머지 역할(예: ADMIN 등)은 두 필드가 null로 유지되어 @JsonInclude(NON_NULL) 정책에 따라 응답에서 자연스럽게 제외됩니다.
- 단일 반환 구문으로 정리되어 NPE 리스크가 제거되었습니다.
src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (2)
62-73: 테스트 픽스처/리포지토리 주입이 정확하고 목적에 부합
- InterestedCountryRepository, MentorFixture, CountryFixture까지 주입해 역할별 경로를 현실적으로 시뮬레이션합니다.
- 통합 성격의 테스트로 회귀 방지력이 높습니다.
85-92: 확인: siteUserFixture.사용자()는 Role.MENTEE를 반환하므로 별도 수정 불필요합니다.SiteUserFixture의 기본 사용자 생성 메서드들이 .role(Role.MENTEE)로 SiteUser를 생성함을 확인했습니다.
검증 결과
- src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixture.java의 사용자() 계열 메서드들이 .role(Role.MENTEE)로 SiteUser를 생성합니다.
- src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixtureBuilder.java에는 role 필드와 role(Role) 빌더가 있어 역할 변경은 가능하지만, 기본 픽스처는 MENTEE로 정의되어 있습니다.
결론
- MyPageServiceTest#setUp()의 user = siteUserFixture.사용자();는 MENTEE 가정을 충족하므로 현재 테스트에서는 추가 조치가 필요하지 않습니다.
| if (siteUser.getRole() == Role.MENTEE) { | ||
| interestedCountries = countryRepository.findKoreanNamesBySiteUserId(siteUser.getId()); | ||
| } else if (siteUser.getRole() == Role.MENTOR) { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
관심 국가 목록의 중복 제거와 정렬 보장 확인 필요
- 응답의 일관성을 위해 DISTINCT 적용 및 정렬 기준(예: 가나다순)을 Repository 쿼리에서 보장하는지 확인이 필요합니다.
- 사용자 경험을 위해 정렬 기준이 명확하면 좋습니다(테스트에서도 검증 가능).
검증 스크립트(Repository 쿼리 확인):
🏁 Script executed:
#!/bin/bash
# CountryRepository의 findKoreanNamesBySiteUserId 쿼리에 DISTINCT/ORDER BY가 있는지 확인
rg -n "findKoreanNamesBySiteUserId" -A 6 -B 3Length of output: 2196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/main/java/com/example/solidconnection/location/country/repository/CountryRepository.java"
echo "== FILE: $file =="
sed -n '1,240p' "$file" || trueLength of output: 1272
확인: DISTINCT는 적용되어 있으나 정렬(ORDER BY)이 없어 추가가 필요합니다
CountryRepository.findKoreanNamesBySiteUserId 쿼리에 SELECT DISTINCT는 이미 적용되어 있습니다.
하지만 ORDER BY가 없어 반환되는 국가 목록의 정렬(예: 가나다순)이 보장되지 않습니다.
권장 변경은 쿼리 끝에 ORDER BY c.koreanName ASC를 명시해 응답 일관성을 확보하는 것입니다.
-
정렬 쿼리 추가.
- 파일: src/main/java/com/example/solidconnection/location/country/repository/CountryRepository.java.
- 제안: 쿼리 끝에 ORDER BY c.koreanName ASC 추가 (예: ORDER BY c.koreanName ASC).
-
테스트 추가.
- 제안: Repository 또는 MyPageService 단위/통합 테스트에서 반환되는 국가 목록이 중복 없이 정렬되어 있는지 검증하세요.
- 참고: MyPageService의 호출부(src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java, 라인 60-62)는 변경 불필요합니다.
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/location/country/repository/CountryRepository.java
(update the query used by findKoreanNamesBySiteUserId) add an ORDER BY clause so
the query ends with "ORDER BY c.koreanName ASC" to guarantee consistent
alphabetical ordering of distinct Korean names; then add a repository or service
unit/integration test that calls the repository (or MyPageService) to assert the
returned list contains no duplicates and is sorted ascending by koreanName; note
MyPageService
(src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java
lines 60-62) does not require changes.
| Mentor mentor = mentorRepository.findBySiteUserId(siteUser.getId()) | ||
| .orElseThrow(() -> new CustomException(MENTOR_NOT_FOUND)); | ||
| University university = universityRepository.findById(mentor.getUniversityId()) | ||
| .orElseThrow(() -> new CustomException(UNIVERSITY_NOT_FOUND)); | ||
| universityKoreanName = university.getKoreanName(); |
There was a problem hiding this comment.
멘토의 대학 ID null 처리 누락: IllegalArgumentException 가능성
멘토는 존재하지만 mentor.getUniversityId()가 null이면 repository.findById(null) 호출로 IllegalArgumentException이 발생할 수 있습니다. 도메인 제약이 있더라도 방어 코드를 둬야 운영 안전성이 올라갑니다.
- 제안: universityId null 가드 후, 명시적으로 UNIVERSITY_NOT_FOUND 예외를 던지세요.
적용 diff:
Mentor mentor = mentorRepository.findBySiteUserId(siteUser.getId())
.orElseThrow(() -> new CustomException(MENTOR_NOT_FOUND));
- University university = universityRepository.findById(mentor.getUniversityId())
+ Long universityId = mentor.getUniversityId();
+ if (universityId == null) {
+ throw new CustomException(UNIVERSITY_NOT_FOUND);
+ }
+ University university = universityRepository.findById(universityId)
.orElseThrow(() -> new CustomException(UNIVERSITY_NOT_FOUND));
universityKoreanName = university.getKoreanName();📝 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.
| Mentor mentor = mentorRepository.findBySiteUserId(siteUser.getId()) | |
| .orElseThrow(() -> new CustomException(MENTOR_NOT_FOUND)); | |
| University university = universityRepository.findById(mentor.getUniversityId()) | |
| .orElseThrow(() -> new CustomException(UNIVERSITY_NOT_FOUND)); | |
| universityKoreanName = university.getKoreanName(); | |
| Mentor mentor = mentorRepository.findBySiteUserId(siteUser.getId()) | |
| .orElseThrow(() -> new CustomException(MENTOR_NOT_FOUND)); | |
| Long universityId = mentor.getUniversityId(); | |
| if (universityId == null) { | |
| throw new CustomException(UNIVERSITY_NOT_FOUND); | |
| } | |
| University university = universityRepository.findById(universityId) | |
| .orElseThrow(() -> new CustomException(UNIVERSITY_NOT_FOUND)); | |
| universityKoreanName = university.getKoreanName(); |
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java
around lines 63 to 67, the code calls
universityRepository.findById(mentor.getUniversityId()) without guarding against
mentor.getUniversityId() being null, which can throw IllegalArgumentException;
add a null check for mentor.getUniversityId() and if null explicitly throw new
CustomException(UNIVERSITY_NOT_FOUND) before calling findById, otherwise proceed
to call findById and retrieve university.getKoreanName().
| @Test | ||
| void 멘토의_마이페이지_정보를_조회한다() { | ||
| // given | ||
| SiteUser mentorUser = siteUserFixture.멘토(1, "mentor"); | ||
| University university = univApplyInfoFixture.괌대학_A_지원_정보().getUniversity(); | ||
| mentorFixture.멘토(mentorUser.getId(), university.getId()); | ||
| int likedUnivApplyInfoCount = createLikedUnivApplyInfos(mentorUser); | ||
|
|
||
| // when | ||
| MyPageResponse response = myPageService.getMyPageInfo(mentorUser.getId()); | ||
|
|
||
| // then | ||
| Assertions.assertAll( | ||
| () -> assertThat(response.nickname()).isEqualTo(mentorUser.getNickname()), | ||
| () -> assertThat(response.profileImageUrl()).isEqualTo(mentorUser.getProfileImageUrl()), | ||
| () -> assertThat(response.role()).isEqualTo(mentorUser.getRole()), | ||
| () -> assertThat(response.email()).isEqualTo(mentorUser.getEmail()), | ||
| // () -> assertThat(response.likedPostCount()).isEqualTo(user.getLikedPostList().size()), | ||
| // todo : 좋아요한 게시물 수 반환 기능 추가와 함께 수정요망 | ||
| () -> assertThat(response.likedUnivApplyInfoCount()).isEqualTo(likedUnivApplyInfoCount), | ||
| () -> assertThat(response.attendedUniversity()).isEqualTo(university.getKoreanName()), | ||
| () -> assertThat(response.interestedCountries()).isNull() | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
멘토 경로의 예외 시나리오 테스트 추가 제안 (커버리지 보강)
- 멘토-사용자 존재하지만 Mentor 레코드 없음 → MENTOR_NOT_FOUND.
- Mentor 존재하지만 universityId null 또는 존재하지 않음 → UNIVERSITY_NOT_FOUND.
- 위 두 케이스는 서비스가 명시적으로 예외를 던지므로 반드시 회귀 테스트로 커버하는 게 안전합니다.
참고 테스트 스니펫(새 메서드로 추가 권장):
@Test
void 멘토정보가_없으면_예외를_던진다() {
// given
SiteUser mentorUser = siteUserFixture.멘토(1, "mentorWithoutRecord");
// mentorFixture.멘토(...) 호출 생략으로 Mentor 미생성
// when & then
Assertions.assertAll(
() -> org.assertj.core.api.Assertions.assertThatThrownBy(
() -> myPageService.getMyPageInfo(mentorUser.getId()))
.isInstanceOf(CustomException.class)
.hasMessageContaining("MENTOR_NOT_FOUND")
);
}
@Test
void 멘토의_대학정보가_없으면_예외를_던진다() {
// given
SiteUser mentorUser = siteUserFixture.멘토(1, "mentorNoUniv");
// mentorFixture를 사용해 universityId를 null로 생성 가능한지 확인 필요
// (불가하면 잘못된 universityId를 주입하여 UNIVERSITY_NOT_FOUND를 유도)
// when & then
org.assertj.core.api.Assertions.assertThatThrownBy(
() -> myPageService.getMyPageInfo(mentorUser.getId()))
.isInstanceOf(CustomException.class)
.hasMessageContaining("UNIVERSITY_NOT_FOUND");
}원하시면 픽스처에 universityId null 케이스 지원을 위한 헬퍼도 추가해 드리겠습니다.
🤖 Prompt for AI Agents
In
src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java
around lines 115 to 138, add two new test methods to cover mentor-path exception
scenarios: (1) create a SiteUser via siteUserFixture as a mentor user but do NOT
create a Mentor record, then call myPageService.getMyPageInfo(userId) and assert
that it throws the expected CustomException with a message containing
"MENTOR_NOT_FOUND"; (2) create a SiteUser and a Mentor record whose universityId
is null or an invalid/non-existent id (use fixture helper or inject a bad id),
then call myPageService.getMyPageInfo(userId) and assert that it throws
CustomException with a message containing "UNIVERSITY_NOT_FOUND"; use
assertThatThrownBy for assertions and follow existing fixture patterns for
setup.
3 participants
관련 이슈
작업 내용
에 대한 정보를 추가하였습니다.
@JsonInclude(JsonInclude.Include.NON_NULL)를 활용해서 멘토의 경우는 attendedUniversity만 멘티의 경우는 interestedCountries만 응답하도록 하였습니다
(@JsonInclude(JsonInclude.Include.NON_NULL)가 붙어있으면 null값인 경우 응답에서 제외합니다.)
특이 사항
리뷰 요구사항 (선택)