MA_01 API 구현 및 테스트코드 작성 #15 #19
Conversation
Walkthrough이번 변경사항에서는 커플 정보를 조회하고 관리하는 기능이 대폭 확장되었습니다. 커플의 등급, D-day, 닉네임, 상태 메시지 등 다양한 정보를 제공하는 서비스( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller
participant Facade
participant CoupleInfoService
participant CoupleRepository
participant MemberRepository
User->>Controller: GET /v1/couple/profile (등급 조회)
Controller->>Facade: getGradeInfo(user)
Facade->>CoupleInfoService: getGrade(userId)
CoupleInfoService->>CoupleRepository: findByInvitorId / findByInviteeId
CoupleInfoService->>CoupleInfoService: computeGrade()
CoupleInfoService-->>Facade: grade(Int)
Facade-->>Controller: CoupleGradeResponse
Controller-->>User: ApiResponse<CoupleGradeResponse>
sequenceDiagram
participant User
participant Controller
participant Facade
participant CoupleInfoService
participant CoupleRepository
participant MemberRepository
User->>Controller: GET /v1/couple/nick-name
Controller->>Facade: nickName(user)
Facade->>CoupleInfoService: nickName(userId)
CoupleInfoService->>CoupleRepository: findByInvitorId / findByInviteeId
CoupleInfoService->>MemberRepository: findById(userId)
CoupleInfoService->>MemberRepository: findById(partnerId)
CoupleInfoService-->>Facade: NicknameResponse
Facade-->>Controller: NicknameResponse
Controller-->>User: ApiResponse<NicknameResponse>
Possibly related PRs
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
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:
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (18)
src/main/kotlin/gomushin/backend/couple/dto/response/StatusMessageResponse.kt (1)
1-13: 상태 메시지 응답 DTO가 적절히 구현되었습니다.
StatusMessageResponse클래스가 적절히 구현되었으며, companion object와 factory 메서드 패턴을 사용한 것이
좋은 설계입니다.다음과 같이 연산자 주변에 공백을 추가하면 가독성이 더 좋아질 것입니다:
- )=StatusMessageResponse( + ) = StatusMessageResponse(src/main/kotlin/gomushin/backend/couple/dto/response/CoupleGradeResponse.kt (1)
1-10: 커플 등급 응답 DTO가 적절히 구현되었습니다.
CoupleGradeResponse클래스의 구현은 적절하며, companion object를 사용한 factory 메서드 패턴이 일관성 있게 적용되었습니다.들여쓰기와 공백 사용이 프로젝트의 다른 파일들과 조금 일관성이 없어 보입니다. 다음과 같이 포맷팅을 조정하는 것이 좋겠습니다:
data class CoupleGradeResponse( - val grade : Int + val grade: Int ){ companion object{ - fun of(grade: Int) = CoupleGradeResponse( - grade + fun of(grade: Int) = CoupleGradeResponse( + grade ) } }src/main/kotlin/gomushin/backend/couple/domain/repository/CoupleRepository.kt (1)
6-9: Repository 메소드 추가가 적절히 구현되었습니다.
findByInvitorId와findByInviteeId메소드 추가는 PR의 커플 정보 조회 API 구현에 필요한 기능입니다. Spring Data JPA 네이밍 규칙을 올바르게 따르고 있습니다.Kotlin 코딩 스타일 가이드에 따라 콜론 앞뒤에 공백을 추가하는 것이 좋겠습니다:
- fun findByInvitorId(invitorId: Long) : Couple? - fun findByInviteeId(inviteeId: Long) : Couple? + fun findByInvitorId(invitorId: Long): Couple? + fun findByInviteeId(inviteeId: Long): Couple?src/main/kotlin/gomushin/backend/couple/dto/response/DdayResponse.kt (1)
3-17: 코드가 전반적으로 잘 구성되어 있으나 들여쓰기와 공백 일관성 개선이 필요합니다.데이터 클래스와 companion object 구조는 잘 되어 있으나, 아래 두 가지 점이 개선되면 좋겠습니다:
- 7번 줄 닫는 소괄호 뒤에 공백이 필요합니다:
){->) {- 9-11번 줄의 들여쓰기가 일관성이 없습니다. 10-11번 줄은 9번 줄과 동일한 들여쓰기 수준으로 맞춰주세요.
- ){ + ) { - fun of(sinceLove: Int?, - sinceMilitaryStart: Int?, - militaryEndLeft: Int?) = DdayResponse ( + fun of( + sinceLove: Int?, + sinceMilitaryStart: Int?, + militaryEndLeft: Int? + ) = DdayResponse(src/main/kotlin/gomushin/backend/couple/facade/CoupleFacade.kt (1)
36-56: 메서드 명명 규칙의 일관성이 필요합니다.새로 추가된 메서드들이 기능적으로는 잘 구현되었으나, 명명 규칙에 일관성이 없습니다:
getGradeInfo,checkConnect,getDday: 동사로 시작하는 메서드명 (권장됨)nickName,statusMessage: 명사로 시작하는 메서드명모든 메서드가 동사로 시작하도록 일관성 있게 수정하는 것이 좋습니다.
또한, 예외 처리 로직이 없습니다.
coupleInfoService의 메서드가 예외를 던질 가능성이 있다면 적절한 예외 처리를 추가하는 것이 좋습니다.- fun nickName(customUserDetails: CustomUserDetails) : NicknameResponse { + fun getNickName(customUserDetails: CustomUserDetails) : NicknameResponse { return coupleInfoService.nickName(customUserDetails.getId()) } - fun statusMessage(customUserDetails: CustomUserDetails): StatusMessageResponse { + fun getStatusMessage(customUserDetails: CustomUserDetails): StatusMessageResponse { val statusMessage = coupleInfoService.getStatusMessage(customUserDetails.getId()) return StatusMessageResponse.of(statusMessage) }src/test/kotlin/gomushin/backend/couple/domain/service/CoupleInfoServiceTest.kt (6)
32-33: 오타를 수정해주세요.DisplayName 주석에 "떄"라는 오타가 있습니다. "때"로 수정해주세요.
- @DisplayName("getGrade - 성공(invitorId가 주어졌을 떄)") + @DisplayName("getGrade - 성공(invitorId가 주어졌을 때)")
51-52: 오타를 수정해주세요.DisplayName 주석에 "떄"라는 오타가 있습니다. "때"로 수정해주세요.
- @DisplayName("getGrade - 성공(inviteeId가 주어졌을 떄)") + @DisplayName("getGrade - 성공(inviteeId가 주어졌을 때)")
72-86: 주석 처리된 테스트 코드에 대한 TODO 또는 설명을 추가해주세요.예외 처리 테스트가 주석 처리되어 있습니다. PR 설명에 언급된 대로 의존성 주입 문제로 인해 일시적으로 주석 처리된 것으로 보입니다. 코드에 주석이나 TODO를 추가하여 왜 주석 처리되었는지와 향후 계획을 명시하면 좋을 것 같습니다.
// @DisplayName("getGrade - 실패(couple이 아닐 때)") // @Test // fun getGrade_fail() { +// // TODO: 의존성 주입 이슈로 인해 테스트 실패. 향후 수정 예정 // // given // val invitorId = 1L //
106-109: 테스트 단언문에 실패 메시지를 추가하면 좋을 것 같습니다.테스트 실패 시 더 명확한 오류 메시지를 제공하기 위해 assert 문에 메시지를 추가하는 것이 좋습니다.
- assert(resultGradeOne == 1) - assert(resultGradeTwo == 2) - assert(resultGradeThree == 3) - assert(resultGradeFour == 4) + assert(resultGradeOne == 1) { "Grade should be 1 for the first month period" } + assert(resultGradeTwo == 2) { "Grade should be 2 for 2-7 months period" } + assert(resultGradeThree == 3) { "Grade should be 3 for 8-13 months period" } + assert(resultGradeFour == 4) { "Grade should be 4 for 14+ months period" }
190-219: 반복되는 테스트 데이터 설정을 리팩토링 하는 것이 좋을 것 같습니다.여러 테스트에서 반복적으로 사용되는 Member 객체 생성 코드를
@BeforeEach메서드로 추출하면 코드 중복을 줄이고 가독성을 높일 수 있습니다.
134-136: 테스트 검증 방식 통일 필요코드베이스 전체에서 일관된 테스트 검증 방식을 사용하는 것이 좋습니다. 이 테스트에서는
assertEquals를 사용하지만, 다른 테스트에서는assert를 사용합니다. 하나의 스타일로 통일하는 것이 좋습니다.- assertEquals(true, resultTrue1) - assertEquals(true, resultTrue2) - assertEquals(false, resultFalse) + assert(resultTrue1) { "User with ID 1 should be in a couple" } + assert(resultTrue2) { "User with ID 2 should be in a couple" } + assert(!resultFalse) { "User with ID 3 should not be in a couple" }또는 모든 테스트에서
assertEquals를 사용하도록 통일:- assert(resultGradeOne == 1) - assert(resultGradeTwo == 2) - assert(resultGradeThree == 3) - assert(resultGradeFour == 4) + assertEquals(1, resultGradeOne, "Grade should be 1 for the first month period") + assertEquals(2, resultGradeTwo, "Grade should be 2 for 2-7 months period") + assertEquals(3, resultGradeThree, "Grade should be 3 for 8-13 months period") + assertEquals(4, resultGradeFour, "Grade should be 4 for 14+ months period")src/test/kotlin/gomushin/backend/member/facade/CoupleFacadeTest.kt (2)
66-71: 설정된 couple 객체가 대부분의 테스트에서 사용되지 않습니다.
couple객체가setUp메서드에서 생성되었지만 대부분의 테스트 케이스에서 사용되지 않습니다. 주석을 추가하여 향후 사용 계획을 명시하거나, 미사용 시 제거를 고려해보세요.
83-84: 테스트 단언문에 실패 메시지를 추가하면 좋을 것 같습니다.실패 시 더 명확한 오류 메시지를 제공하기 위해 assert 문에 메시지를 추가하는 것이 좋습니다.
- assertEquals(1, result.grade) + assertEquals(1, result.grade, "등급이 예상과 다릅니다. 예상: 1, 실제: ${result.grade}")src/main/kotlin/gomushin/backend/couple/domain/service/CoupleInfoService.kt (4)
39-42: getCouple 메서드의 접근 제한자 고려
getCouple메서드는 내부 구현 세부 사항으로 보입니다. 이 메서드가 외부 클래스에서 호출될 필요가 없다면, 접근 제한자를internal또는private으로 변경하는 것이 적절할 수 있습니다.- fun getCouple(id : Long): Couple? { + private fun getCouple(id : Long): Couple? { return coupleRepository.findByInvitorId(id) ?: coupleRepository.findByInviteeId(id) }
61-63: computeDday 메서드에 주석 추가 필요
computeDday메서드의 계산 방향(양수는 day2가 day1보다 이후임을 의미, 음수는 반대)을 설명하는 주석을 추가하면 이해하기 쉬울 것 같습니다.+ /** + * 두 날짜 사이의 일수를 계산합니다. + * 반환 값이 양수이면 day2가 day1보다 이후 날짜임을, 음수이면 day2가 day1보다 이전 날짜임을 의미합니다. + */ fun computeDday(day1: LocalDate, day2: LocalDate) : Int { return ChronoUnit.DAYS.between(day1, day2).toInt() }
80-88: getStatusMessage 메서드가 null을 처리하는 방식 명확히 하기
getStatusMessage메서드는String?을 반환하지만, 반환되는coupleMember.statusMessage이 null일 때 어떻게 처리해야 하는지에 대한 설명이 없습니다. 메서드에 주석을 추가하여 null 반환 가능성과 해석 방법을 명확히 하는 것이 좋습니다.+ /** + * 커플 상대방의 상태 메시지를 반환합니다. + * 상태 메시지가 설정되지 않은 경우 null을 반환합니다. + */ fun getStatusMessage(id: Long): String? { val couple = getCouple(id) ?: throw BadRequestException("saranggun.couple.not-connected") val coupleMemberId = if (couple.invitorId == id) couple.inviteeId else couple.invitorId val coupleMember = memberRepository.findById(coupleMemberId).orElseThrow { - BadRequestException("sarangggun.couple.not-exist-couple") + BadRequestException("saranggun.couple.not-exist-couple") } return coupleMember.statusMessage }
15-25: 예외 메시지의 일관된 관리 방식 고려예외 메시지가 하드코딩된 문자열로 여러 곳에 분산되어 있습니다. 이는 유지보수와 다국어 지원에 어려움을 줄 수 있습니다. 오류 메시지를 중앙화된 상수 또는 메시지 프로퍼티 파일에서 관리하는 것을 고려해보세요.
src/main/kotlin/gomushin/backend/couple/presentation/CoupleInfoController.kt (1)
45-48: 메서드 이름 일관성 개선함수 이름이
dDay로 camelCase 형태가 아닌 것으로 보입니다. 다른 메서드들의 이름 패턴(profile, check, nickName, statusMessage)과 일관성을 유지하려면dday로 수정하는 것이 좋을 것 같습니다.- fun dDay( + fun dday( @AuthenticationPrincipal customUserDetails: CustomUserDetails ):ApiResponse<DdayResponse> { return ApiResponse.success(coupleFacade.getDday(customUserDetails))
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/kotlin/gomushin/backend/couple/domain/entity/Couple.kt(1 hunks)src/main/kotlin/gomushin/backend/couple/domain/repository/CoupleRepository.kt(1 hunks)src/main/kotlin/gomushin/backend/couple/domain/service/CoupleInfoService.kt(1 hunks)src/main/kotlin/gomushin/backend/couple/dto/response/CoupleGradeResponse.kt(1 hunks)src/main/kotlin/gomushin/backend/couple/dto/response/DdayResponse.kt(1 hunks)src/main/kotlin/gomushin/backend/couple/dto/response/NicknameResponse.kt(1 hunks)src/main/kotlin/gomushin/backend/couple/dto/response/StatusMessageResponse.kt(1 hunks)src/main/kotlin/gomushin/backend/couple/facade/CoupleFacade.kt(2 hunks)src/main/kotlin/gomushin/backend/couple/presentation/ApiPath.kt(1 hunks)src/main/kotlin/gomushin/backend/couple/presentation/CoupleInfoController.kt(1 hunks)src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt(1 hunks)src/test/kotlin/gomushin/backend/couple/domain/service/CoupleInfoServiceTest.kt(1 hunks)src/test/kotlin/gomushin/backend/member/facade/CoupleFacadeTest.kt(1 hunks)
🔇 Additional comments (6)
src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt (1)
28-29: 상태 메시지 필드 추가가 적절히 구현되었습니다.새로운
statusMessage필드를 Member 엔티티에 추가한 것은 PR에서 언급된 상태 메시지 API 구현에 필요한 변경사항입니다. 필드가 nullable로 설정되어 있고 기본값이 null로 지정된 것이 적절합니다.src/main/kotlin/gomushin/backend/couple/domain/entity/Couple.kt (1)
29-29: 코드 스타일 개선이 이루어졌습니다.괄호와 콜론 위치를 Kotlin 스타일 가이드에 맞게 변경한 것은 적절합니다.
src/main/kotlin/gomushin/backend/couple/dto/response/NicknameResponse.kt (1)
3-16: 코드가 깔끔하고 잘 구조화되어 있습니다!
NicknameResponse데이터 클래스가 사용자 닉네임과 커플 닉네임을 잘 캡슐화하고 있습니다. companion object의of팩토리 메서드를 통한 인스턴스 생성 방식도 Kotlin의 관용적인 패턴을 잘 따르고 있습니다.src/main/kotlin/gomushin/backend/couple/presentation/ApiPath.kt (1)
7-11: API 경로 상수 추가가 일관된 방식으로 잘 이루어졌습니다.새로 추가된 API 경로 상수들이 기존 명명 규칙과 패턴을 잘 따르고 있습니다. 각 엔드포인트의 목적이 명확하게 드러나는 경로로 잘 설계되었습니다.
src/main/kotlin/gomushin/backend/couple/facade/CoupleFacade.kt (2)
6-11: 필요한 의존성과 DTO 클래스들을 잘 가져왔습니다.
CoupleInfoService와 관련 응답 DTO 클래스들을 적절히 import 했습니다.
15-18: 새로운 서비스 의존성이 잘 추가되었습니다.기존
CoupleConnectService와 함께 새로운CoupleInfoService의존성이 적절히 추가되었습니다.
| `when`(coupleRepository.findByInvitorId(userId)).thenReturn(couple) | ||
| `when`(memberRepository.findById(userId)).thenReturn(Optional.of(user)) | ||
| `when`(memberRepository.findById(coupleUserId)).thenReturn(Optional.of(coupleUser)) | ||
|
|
There was a problem hiding this comment.
테스트 데이터 불일치 확인 필요
이 테스트에서 couple 객체는 invitorId = coupleUserId와 inviteeId = userId로 설정되어 있지만, 첫 번째 when 문에서는 coupleRepository.findByInvitorId(userId)를 모킹하고 있습니다. 이는 실제 사용 사례와 일치하지 않을 수 있습니다. 아래와 같이 수정이 필요합니다:
- `when`(coupleRepository.findByInvitorId(userId)).thenReturn(couple)
+ `when`(coupleRepository.findByInviteeId(userId)).thenReturn(couple)📝 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.
| `when`(coupleRepository.findByInvitorId(userId)).thenReturn(couple) | |
| `when`(memberRepository.findById(userId)).thenReturn(Optional.of(user)) | |
| `when`(memberRepository.findById(coupleUserId)).thenReturn(Optional.of(coupleUser)) | |
| `when`(coupleRepository.findByInviteeId(userId)).thenReturn(couple) | |
| `when`(memberRepository.findById(userId)).thenReturn(Optional.of(user)) | |
| `when`(memberRepository.findById(coupleUserId)).thenReturn(Optional.of(coupleUser)) |
| member2 = Member( | ||
| id = 2L, | ||
| name = "꽃신", | ||
| nickname = "꽃신닉네임", | ||
| email = "[email protected]", | ||
| birthDate = null, | ||
| profileImageUrl = null, | ||
| provider = Provider.KAKAO, | ||
| role = Role.MEMBER, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
테스트 데이터의 isCouple 필드 확인 필요
member2 객체에도 isCouple 필드 값이 설정되어 있지 않습니다. 커플 기능을 테스트하므로 isCouple = true로 설정하는 것이 더 적절합니다.
member2 = Member(
id = 2L,
name = "꽃신",
nickname = "꽃신닉네임",
email = "[email protected]",
birthDate = null,
profileImageUrl = null,
provider = Provider.KAKAO,
role = Role.MEMBER,
+ isCouple = true,
)📝 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.
| member2 = Member( | |
| id = 2L, | |
| name = "꽃신", | |
| nickname = "꽃신닉네임", | |
| email = "[email protected]", | |
| birthDate = null, | |
| profileImageUrl = null, | |
| provider = Provider.KAKAO, | |
| role = Role.MEMBER, | |
| ) | |
| member2 = Member( | |
| id = 2L, | |
| name = "꽃신", | |
| nickname = "꽃신닉네임", | |
| email = "[email protected]", | |
| birthDate = null, | |
| profileImageUrl = null, | |
| provider = Provider.KAKAO, | |
| role = Role.MEMBER, | |
| isCouple = true, | |
| ) |
| member1 = Member( | ||
| id = 1L, | ||
| name = "곰신", | ||
| nickname = "곰신닉네임", | ||
| email = "[email protected]", | ||
| birthDate = null, | ||
| profileImageUrl = null, | ||
| provider = Provider.KAKAO, | ||
| role = Role.MEMBER, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
테스트 데이터의 isCouple 필드 확인 필요
테스트에서 사용하는 member1 객체에 isCouple 필드 값이 설정되어 있지 않습니다(기본값은 false). 이는 커플 관련 작업을 테스트하는 데 잠재적인 문제가 될 수 있습니다. 실제 사용 사례를 더 정확하게 반영하기 위해 isCouple = true로 설정하는 것이 좋습니다.
member1 = Member(
id = 1L,
name = "곰신",
nickname = "곰신닉네임",
email = "[email protected]",
birthDate = null,
profileImageUrl = null,
provider = Provider.KAKAO,
role = Role.MEMBER,
+ isCouple = true,
)📝 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.
| member1 = Member( | |
| id = 1L, | |
| name = "곰신", | |
| nickname = "곰신닉네임", | |
| email = "[email protected]", | |
| birthDate = null, | |
| profileImageUrl = null, | |
| provider = Provider.KAKAO, | |
| role = Role.MEMBER, | |
| ) | |
| member1 = Member( | |
| id = 1L, | |
| name = "곰신", | |
| nickname = "곰신닉네임", | |
| email = "[email protected]", | |
| birthDate = null, | |
| profileImageUrl = null, | |
| provider = Provider.KAKAO, | |
| role = Role.MEMBER, | |
| isCouple = true, | |
| ) |
src/main/kotlin/gomushin/backend/couple/domain/service/CoupleInfoService.kt
Outdated
Show resolved
Hide resolved
| val coupleMember = memberRepository.findById(coupleMemberId).orElseThrow { | ||
| BadRequestException("sarangggun.couple.not-exist-couple") | ||
| } |
There was a problem hiding this comment.
오타를 수정해주세요.
예외 메시지에 "sarangggun"이라는 오타가 있습니다. "saranggun"으로 수정해주세요.
- BadRequestException("sarangggun.couple.not-exist-couple")
+ BadRequestException("saranggun.couple.not-exist-couple")📝 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.
| val coupleMember = memberRepository.findById(coupleMemberId).orElseThrow { | |
| BadRequestException("sarangggun.couple.not-exist-couple") | |
| } | |
| val coupleMember = memberRepository.findById(coupleMemberId).orElseThrow { | |
| BadRequestException("saranggun.couple.not-exist-couple") | |
| } |
| @ResponseStatus(HttpStatus.OK) | ||
| @GetMapping(ApiPath.COUPLE_DDAY_INFO) | ||
| @Operation(summary = "디데이 정보", description = "사귄지, 입대한지 얼마되었는지 그리고 전역까지 얼마나 남았는지") | ||
| fun dDay( | ||
| @AuthenticationPrincipal customUserDetails: CustomUserDetails | ||
| ):ApiResponse<DdayResponse> { | ||
| return ApiResponse.success(coupleFacade.getDday(customUserDetails)) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
API 오류 응답에 대한 문서화 필요
이 엔드포인트도 가능한 오류 응답에 대한 Swagger 문서화가 필요합니다.
| @ResponseStatus(HttpStatus.OK) | ||
| @GetMapping(ApiPath.COUPLE_NICKNAME) | ||
| @Operation(summary = "닉네임 조회", description = "userNickname = 내 닉네임, coupleNickName = 내 여(남)친 닉네임") | ||
| fun nickName( | ||
| @AuthenticationPrincipal customUserDetails: CustomUserDetails | ||
| ):ApiResponse<NicknameResponse>{ | ||
| return ApiResponse.success(coupleFacade.nickName(customUserDetails)) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
API 오류 응답에 대한 문서화 필요
이 엔드포인트도 가능한 오류 응답에 대한 Swagger 문서화가 필요합니다.
| @ResponseStatus(HttpStatus.OK) | ||
| @GetMapping(ApiPath.COUPLE_STATUS_MESSAGE) | ||
| @Operation(summary = "상태 메시지 조회", description = "상태 메시지 조회") | ||
| fun statusMessage( | ||
| @AuthenticationPrincipal customUserDetails: CustomUserDetails | ||
| ):ApiResponse<StatusMessageResponse>{ | ||
| return ApiResponse.success(coupleFacade.statusMessage(customUserDetails)) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
API 오류 응답에 대한 문서화 필요
이 엔드포인트도 가능한 오류 응답에 대한 Swagger 문서화가 필요합니다.
| @ResponseStatus(HttpStatus.OK) | ||
| @GetMapping(ApiPath.COUPLE_CHECK_CONNECT) | ||
| @Operation(summary = "커플 연동 여부", description = "커플 연동 여부 true, false로 불러오기") | ||
| fun check( | ||
| @AuthenticationPrincipal customUserDetails: CustomUserDetails | ||
| ): ApiResponse<Boolean> { | ||
| return ApiResponse.success(coupleFacade.checkConnect(customUserDetails)) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
API 오류 응답에 대한 문서화 필요
이 엔드포인트도 가능한 오류 응답에 대한 Swagger 문서화가 필요합니다.
| @GetMapping(ApiPath.COUPLE_PROFILE) | ||
| @Operation(summary = "프로필 조회", description = "입대일 날짜 기준으로 grade측정") | ||
| fun profile( | ||
| @AuthenticationPrincipal customUserDetails: CustomUserDetails | ||
| ): ApiResponse<CoupleGradeResponse> { | ||
| val grade = coupleFacade.getGradeInfo(customUserDetails) | ||
| return ApiResponse.success(grade) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
API 오류 응답에 대한 문서화 필요
Swagger 문서에 가능한 오류 응답 코드와 메시지에 대한 정보가 없습니다. @ApiResponse 어노테이션을 추가하여 가능한 오류 상황(예: 커플이 연동되지 않은 경우, 필수 데이터가 없는 경우 등)에 대한 응답을 문서화하면 API 사용자에게 도움이 될 것입니다.
@ResponseStatus(HttpStatus.OK)
@GetMapping(ApiPath.COUPLE_PROFILE)
@Operation(summary = "프로필 조회", description = "입대일 날짜 기준으로 grade측정")
+ @ApiResponses(value = [
+ ApiResponse(responseCode = "200", description = "성공"),
+ ApiResponse(responseCode = "400", description = "사용자가 커플이 아니거나 입대일이 등록되지 않은 경우",
+ content = [Content(schema = Schema(implementation = ErrorResponse::class))])
+ ])
fun profile(
@AuthenticationPrincipal customUserDetails: CustomUserDetails
): ApiResponse<CoupleGradeResponse> {
val grade = coupleFacade.getGradeInfo(customUserDetails)
return ApiResponse.success(grade)
}Committable suggestion skipped: line range outside the PR's diff.
| @ResponseStatus(HttpStatus.OK) | ||
| @GetMapping(ApiPath.COUPLE_PROFILE) | ||
| @Operation(summary = "프로필 조회", description = "입대일 날짜 기준으로 grade측정") | ||
| fun profile( |
There was a problem hiding this comment.
반환값이 grade만 있으면 이름이 profile은 안어울리는 것 같아.
getGrade 이런식으로 가는건 어떨까?
| @ResponseStatus(HttpStatus.OK) | ||
| @GetMapping(ApiPath.COUPLE_CHECK_CONNECT) | ||
| @Operation(summary = "커플 연동 여부", description = "커플 연동 여부 true, false로 불러오기") | ||
| fun check( |
There was a problem hiding this comment.
개인적인 의견인데, controller 계층의 역할은 클라이언트와의 연결고리라고 생각하거든.
만약 커플 연동 여부를 확인하는거라면 check 보단 좀 더 명시적인 이름을 사용하는 것이 좋을 것 같아.
| @ResponseStatus(HttpStatus.OK) | ||
| @GetMapping(ApiPath.COUPLE_DDAY_INFO) | ||
| @Operation(summary = "디데이 정보", description = "사귄지, 입대한지 얼마되었는지 그리고 전역까지 얼마나 남았는지") | ||
| fun dDay( |
There was a problem hiding this comment.
222
getDday 처럼 명시적으로 하는 게 좋을 것 같아
| @ResponseStatus(HttpStatus.OK) | ||
| @GetMapping(ApiPath.COUPLE_NICKNAME) | ||
| @Operation(summary = "닉네임 조회", description = "userNickname = 내 닉네임, coupleNickName = 내 여(남)친 닉네임") | ||
| fun nickName( |
| @ResponseStatus(HttpStatus.OK) | ||
| @GetMapping(ApiPath.COUPLE_STATUS_MESSAGE) | ||
| @Operation(summary = "상태 메시지 조회", description = "상태 메시지 조회") | ||
| fun statusMessage( |
| ?: coupleRepository.findByInviteeId(id) | ||
| } | ||
|
|
||
| fun checkCouple(id: Long): Boolean = getCouple(id) != null |
There was a problem hiding this comment.
엔터 쳐서 각 메서드 사이에 공백을 유지하는게 좋을 듯 !
There was a problem hiding this comment.
그리고 이거 member가 couple 연결이 되어있는지를 찾는 함수라면 member 엔티티에 isCouple 컬럼을 사용할 수 있을 것 같아
| return ChronoUnit.DAYS.between(day1, day2).toInt() | ||
| } | ||
|
|
||
| fun nickName(id: Long): NicknameResponse { |
There was a problem hiding this comment.
@transactional(readonly = true)
+
getNickname 처럼 메서드 명을 더 직관적으로 바꾸는 게 좋아보여.
| return NicknameResponse.of(userMember.nickname, coupleMember.nickname) | ||
| } | ||
|
|
||
| fun getStatusMessage(id: Long): String? { |
There was a problem hiding this comment.
@transactional(readonly = true)
There was a problem hiding this comment.
val couple = getCouple(id) ?: throw BadRequestException("saranggun.couple.not-connected")
val coupleMemberId = if (couple.invitorId == id) couple.inviteeId else couple.invitorId
val coupleMember = memberRepository.findById(coupleMemberId).orElseThrow {
BadRequestException("sarangggun.couple.not-exist-couple")
}이 코드가 반복되는 것 같은데, 따로 분리하는 것도 좋을 듯!
| return computeGrade(militaryStartDate, today) | ||
| } | ||
|
|
||
| fun computeGrade(militaryStartDate: LocalDate, today: LocalDate) : Int { |
There was a problem hiding this comment.
외부에서 호출 하지 않고, 서비스 내부에서만 사용되는 거라면 private 으로 캡슐화를 지켜줘도 좋을 것 같아.
| } | ||
|
|
||
| fun getCouple(id : Long): Couple? { | ||
| return coupleRepository.findByInvitorId(id) |
There was a problem hiding this comment.
여기 findByMemberId 로 JPQL 만들어서 쓸 순 없나?
쿼리 한 번으로 처리할 수 있게
아니면 findByInviteeIdOrInvitorId 이런 식으로
JPQL사용해서 커플객체 구할 때 findByInvitorId, findByInviteeId 두번 안쓰게 했음 일부 공통된 메서드 따로 분리시킴(유저의 커플 member객체 구하는 로직) 커플 여부 확인할 때 member의 isCouple컬럼 사용하게 수정 테스트코드도 수정한 내용 반영하여 고침
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/kotlin/gomushin/backend/couple/domain/service/CoupleInfoService.kt (1)
86-94:⚠️ Potential issuefindCoupleMember 메서드의 오타 수정 필요
sarangggun.couple.not-exist-couple오류 메시지에 오타가 있습니다. 'g'가 하나 더 들어가 있어 수정이 필요합니다.private fun findCoupleMember(id : Long): Member { val couple = coupleRepository.findByMemberId(id) ?: throw BadRequestException("saranggun.couple.not-connected") val coupleMemberId = if (couple.invitorId == id) couple.inviteeId else couple.invitorId val coupleMember = memberRepository.findById(coupleMemberId).orElseThrow { - BadRequestException("sarangggun.couple.not-exist-couple") + BadRequestException("saranggun.couple.not-exist-couple") } return coupleMember }
🧹 Nitpick comments (6)
src/main/kotlin/gomushin/backend/couple/domain/service/CoupleInfoService.kt (6)
21-27: getGrade 메서드에 대한 개선 제안이 메서드는 문제없이 잘 작성되었습니다. 다만 리팩토링 측면에서 다음과 같은 개선점을 고려해보세요:
- 반복되는
coupleRepository.findByMemberId(id)로직을 별도의 헬퍼 메서드로 추출하면 코드 중복을 줄일 수 있습니다.@Transactional(readOnly = true) fun getGrade(id: Long): Int { - val couple = coupleRepository.findByMemberId(id) - ?: throw BadRequestException("saranggun.couple.not-connected") + val couple = getCouple(id) val militaryStartDate = couple.militaryStartDate ?: throw BadRequestException("saranggun.couple.not-defined-militaryStartDate") val today = LocalDate.now() return computeGrade(militaryStartDate, today) }
29-39: computeGrade 메서드에 @transactional 애노테이션 추가 필요이 메서드는 데이터를 수정하지 않는 계산 메서드이므로, 다른 읽기 전용 메서드와 일관성을 위해
@Transactional(readOnly = true)애노테이션을 추가하는 것이 좋습니다.+@Transactional(readOnly = true) fun computeGrade(militaryStartDate: LocalDate, today: LocalDate) : Int { val period = Period.between(militaryStartDate, today) val totalMonths = period.years * 12 + period.months + if (period.days >= 1) 1 else 0
41-46: checkCouple 메서드명 개선 제안
checkCouple메서드명은 기능을 명확히 설명하지 못합니다. Boolean을 반환하는 메서드는 일반적으로 'is' 또는 'has'로 시작하는 것이 관례입니다.@Transactional(readOnly = true) -fun checkCouple(id: Long): Boolean { +fun isMemberInCouple(id: Long): Boolean { return memberRepository.findById(id) .orElseThrow{ BadRequestException("sarangggun.member.not-exist-member") } .isCouple }
49-63: getDday 메서드의 도메인 로직 이동 제안이 메서드는 Couple 엔티티의 값을 직접 가져와서 계산하고 있습니다. 이 로직을 Couple 엔티티 내부로 이동하면 도메인 응집도를 높일 수 있습니다(Tell-Don't-Ask 원칙).
@Transactional(readOnly = true) fun getDday(id: Long): DdayResponse { val couple = coupleRepository.findByMemberId(id) ?: throw BadRequestException("saranggun.couple.not-connected") - val today = LocalDate.now() - val sinceLove: Int? = couple.relationshipStartDate?.let { startLove -> - computeDday(startLove, today) - } - val sinceMilitaryStart : Int? = couple.militaryStartDate?.let { startMilitary -> - computeDday(startMilitary, today) - } - val militaryEndLeft : Int? = couple.militaryEndDate?.let { endMilitary -> - computeDday(endMilitary, today) - } - return DdayResponse.of(sinceLove, sinceMilitaryStart, militaryEndLeft) + return couple.calculateDdays() }그리고 Couple 엔티티에 다음과 같은 메서드를 추가:
fun calculateDdays(): DdayResponse { val today = LocalDate.now() val sinceLove = relationshipStartDate?.let { ChronoUnit.DAYS.between(it, today).toInt() } val sinceMilitaryStart = militaryStartDate?.let { ChronoUnit.DAYS.between(it, today).toInt() } val militaryEndLeft = militaryEndDate?.let { ChronoUnit.DAYS.between(it, today).toInt() } return DdayResponse.of(sinceLove, sinceMilitaryStart, militaryEndLeft) }
65-67: computeDday 메서드에 @transactional 애노테이션 추가 필요이 메서드는 데이터를 수정하지 않는 유틸리티 메서드이므로, 다른 읽기 전용 메서드와 일관되게
@Transactional(readOnly = true)애노테이션을 추가하는 것이 좋습니다.+@Transactional(readOnly = true) fun computeDday(day1: LocalDate, day2: LocalDate) : Int { return ChronoUnit.DAYS.between(day1, day2).toInt() }
70-78: getNickName 메서드명 일관성 개선메서드명에서 'N'이 대문자인
getNickName은 카멜 케이스 명명 규칙에 맞지 않습니다. 다른 메서드들과 일관성을 위해 'n'을 소문자로 바꿔주세요.@Transactional(readOnly = true) -fun getNickName(id: Long): NicknameResponse { +fun getNickname(id: Long): NicknameResponse { val userMember = memberRepository.findById(id).orElseThrow { BadRequestException("saranggun.member.not-found") } val coupleMember = findCoupleMember(id) return NicknameResponse.of(userMember.nickname, coupleMember.nickname) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/kotlin/gomushin/backend/couple/domain/repository/CoupleRepository.kt(1 hunks)src/main/kotlin/gomushin/backend/couple/domain/service/CoupleInfoService.kt(1 hunks)src/main/kotlin/gomushin/backend/couple/facade/CoupleFacade.kt(2 hunks)src/main/kotlin/gomushin/backend/couple/presentation/CoupleInfoController.kt(1 hunks)src/test/kotlin/gomushin/backend/couple/domain/service/CoupleInfoServiceTest.kt(1 hunks)src/test/kotlin/gomushin/backend/member/facade/CoupleFacadeTest.kt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/test/kotlin/gomushin/backend/member/facade/CoupleFacadeTest.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/kotlin/gomushin/backend/couple/facade/CoupleFacade.kt
- src/test/kotlin/gomushin/backend/couple/domain/service/CoupleInfoServiceTest.kt
- src/main/kotlin/gomushin/backend/couple/domain/repository/CoupleRepository.kt
- src/main/kotlin/gomushin/backend/couple/presentation/CoupleInfoController.kt
| package gomushin.backend.couple.domain.service | ||
|
|
||
| import gomushin.backend.core.infrastructure.exception.BadRequestException | ||
| import gomushin.backend.couple.domain.repository.CoupleRepository | ||
| import gomushin.backend.couple.dto.response.DdayResponse | ||
| import gomushin.backend.couple.dto.response.NicknameResponse | ||
| import gomushin.backend.member.domain.entity.Member | ||
| import gomushin.backend.member.domain.repository.MemberRepository | ||
| import org.springframework.stereotype.Service | ||
| import org.springframework.transaction.annotation.Transactional | ||
| import java.time.LocalDate | ||
| import java.time.Period | ||
| import java.time.temporal.ChronoUnit | ||
|
|
||
| @Service | ||
| class CoupleInfoService( | ||
| private val coupleRepository: CoupleRepository, | ||
| private val memberRepository: MemberRepository | ||
| ) { | ||
| @Transactional(readOnly = true) | ||
| fun getGrade(id: Long): Int { | ||
| val couple = coupleRepository.findByMemberId(id) | ||
| ?: throw BadRequestException("saranggun.couple.not-connected") | ||
| val militaryStartDate = couple.militaryStartDate ?: throw BadRequestException("saranggun.couple.not-defined-militaryStartDate") | ||
| val today = LocalDate.now() | ||
| return computeGrade(militaryStartDate, today) | ||
| } | ||
|
|
||
| fun computeGrade(militaryStartDate: LocalDate, today: LocalDate) : Int { | ||
| val period = Period.between(militaryStartDate, today) | ||
| val totalMonths = period.years * 12 + period.months + if (period.days >= 1) 1 else 0 | ||
|
|
||
| return when { | ||
| totalMonths < 2 -> 1 | ||
| totalMonths < 8 -> 2 | ||
| totalMonths < 14 -> 3 | ||
| else -> 4 | ||
| } | ||
| } | ||
|
|
||
| @Transactional(readOnly = true) | ||
| fun checkCouple(id: Long): Boolean { | ||
| return memberRepository.findById(id) | ||
| .orElseThrow{ BadRequestException("sarangggun.member.not-exist-member") } | ||
| .isCouple | ||
| } | ||
|
|
||
| @Transactional(readOnly = true) | ||
| fun getDday(id: Long): DdayResponse { | ||
| val couple = coupleRepository.findByMemberId(id) | ||
| ?: throw BadRequestException("saranggun.couple.not-connected") | ||
| val today = LocalDate.now() | ||
| val sinceLove: Int? = couple.relationshipStartDate?.let { startLove -> | ||
| computeDday(startLove, today) | ||
| } | ||
| val sinceMilitaryStart : Int? = couple.militaryStartDate?.let { startMilitary -> | ||
| computeDday(startMilitary, today) | ||
| } | ||
| val militaryEndLeft : Int? = couple.militaryEndDate?.let { endMilitary -> | ||
| computeDday(endMilitary, today) | ||
| } | ||
| return DdayResponse.of(sinceLove, sinceMilitaryStart, militaryEndLeft) | ||
| } | ||
|
|
||
| fun computeDday(day1: LocalDate, day2: LocalDate) : Int { | ||
| return ChronoUnit.DAYS.between(day1, day2).toInt() | ||
| } | ||
|
|
||
| @Transactional(readOnly = true) | ||
| fun getNickName(id: Long): NicknameResponse { | ||
| val userMember = memberRepository.findById(id).orElseThrow { | ||
| BadRequestException("saranggun.member.not-found") | ||
| } | ||
|
|
||
| val coupleMember = findCoupleMember(id) | ||
|
|
||
| return NicknameResponse.of(userMember.nickname, coupleMember.nickname) | ||
| } | ||
|
|
||
| @Transactional(readOnly = true) | ||
| fun getStatusMessage(id: Long): String? { | ||
| val coupleMember = findCoupleMember(id) | ||
| return coupleMember.statusMessage | ||
| } | ||
|
|
||
| private fun findCoupleMember(id : Long): Member { | ||
| val couple = coupleRepository.findByMemberId(id) ?: throw BadRequestException("saranggun.couple.not-connected") | ||
| val coupleMemberId = if (couple.invitorId == id) couple.inviteeId else couple.invitorId | ||
|
|
||
| val coupleMember = memberRepository.findById(coupleMemberId).orElseThrow { | ||
| BadRequestException("sarangggun.couple.not-exist-couple") | ||
| } | ||
| return coupleMember | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
코드 구조 및 중복 개선을 위한 리팩토링 제안
현재 서비스 코드에서 몇 가지 반복 패턴이 보입니다. 다음과 같은 리팩토링을 통해 코드 품질을 개선할 수 있습니다:
- 반복되는 Couple 조회 로직을 private 메서드로 추출
- 메서드 간 일관된 공백 유지
- 리포지토리 쿼리 최적화
+ // 반복되는 Couple 조회 로직을 추출
+ private fun getCouple(memberId: Long) =
+ coupleRepository.findByMemberId(memberId)
+ ?: throw BadRequestException("saranggun.couple.not-connected")또한 CoupleRepository에서 다음과 같은 최적화된 쿼리 메서드 추가를 고려해보세요:
// CoupleRepository 인터페이스에 추가
@Query("SELECT c FROM Couple c WHERE c.invitorId = :memberId OR c.inviteeId = :memberId")
fun findByMemberId(@Param("memberId") memberId: Long): Couple?There was a problem hiding this comment.
// CoupleRepository 인터페이스에 추가
@Query("SELECT c FROM Couple c WHERE c.invitorId = :memberId OR c.inviteeId = :memberId")
fun findByMemberId(@Param("memberId") memberId: Long): Couple?이 정도 추가하면 더 깔끔해질 수 있을 것 같은데
지금도 그냥 가도 될 듯
There was a problem hiding this comment.
@HoyeongJeon 안녕하세요. 말씀하신 대로 코드를 현재 상태로 유지해도 괜찮을 것 같습니다. 실제 코드를 보니 findByMemberId 메서드를 이미 사용하고 계신 것으로 보입니다. 필요에 따라 나중에 반복되는 Couple 조회 로직을 추출하는 부분도 고려해보실 수 있겠지만, 현재 구현도 충분히 명확합니다. 좋은 결정이십니다! 👍
✅ PR 유형
어떤 변경 사항이 있었나요?
📝 작업 내용
이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
#15
✏️ 관련 이슈
본인이 작업한 내용이 어떤 Issue Number와 관련이 있는지만 작성해주세요
-feats : #15
🎸 기타 사항 or 추가 코멘트
테스트 코드 중에 예외 처리 테스트 하고 싶었는데(주석처리 되어있음) 의존성 주입 문제 때문에 오류나서 일단은 보류해두었어
Summary by CodeRabbit
New Features
Bug Fixes
Tests