Issue: (#12) 커플연결 #13
Conversation
|
Caution Review failedThe pull request is closed. """ Walkthrough이번 변경에서는 커플 연결 및 코드 생성 기능이 새롭게 도입되었습니다. 커플 연결을 위한 도메인, 서비스, 컨트롤러, DTO, 유틸리티, API 경로 상수 등이 추가되었고, 멤버 엔터티에 이름과 커플 상태 필드가 도입되었습니다. 멤버 정보 조회 및 온보딩 관련 코드와 테스트 코드의 패키지 구조 및 일부 반환 타입이 정비되었습니다. 보안 설정에서는 기본 인증 요구가 "MEMBER" 역할로 강화되었고, OAuth2 인증 성공 시 리다이렉트 URL이 로컬 개발 환경으로 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller as CoupleConnectController
participant Facade as CoupleFacade
participant Service as CoupleConnectService
participant Redis
participant DB as CoupleRepository/MemberRepository
User->>Controller: POST /v1/couple/code-generate
Controller->>Facade: requestCoupleCodeGeneration(userDetails)
Facade->>Service: generateCoupleCode(userId)
Service->>Redis: set(coupleCode, userId, 60min)
Service-->>Facade: coupleCode
Facade-->>Controller: coupleCode
Controller-->>User: coupleCode
User->>Controller: POST /v1/couple/connect {coupleCode}
Controller->>Facade: requestCoupleConnect(userDetails, request)
Facade->>Service: connectCouple(userId, coupleCode)
Service->>Redis: get(coupleCode)
Service->>DB: findById(invitorId), findById(inviteeId)
Service->>DB: CoupleRepository.save()
Service->>DB: Member.updateCoupleStatus()
Service-->>Facade: 성공/실패
Facade-->>Controller: 성공/실패
Controller-->>User: ApiResponse(Boolean)
Possibly related PRs
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🪧 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: 5
🧹 Nitpick comments (18)
src/main/kotlin/gomushin/backend/member/util/CoupleCodeGeneratorUtil.kt (1)
3-9: 코드 생성기 구현이 단순하고 효과적이지만 몇 가지 개선이 필요합니다.코드 생성 로직은 간결하고 이해하기 쉽습니다. 하지만 다음과 같은 몇 가지 고려사항이 있습니다:
- 코드 중복 가능성: 동시에 여러 코드 요청이 있을 경우 중복 코드가 생성될 수 있습니다.
- 패키지 위치: 이 유틸리티는
member.util패키지보다는couple관련 패키지에 위치하는 것이 더 적절할 수 있습니다.- 보안: 더 안전한 난수 생성 메커니즘을 고려할 수 있습니다.
다음과 같이 개선할 수 있습니다:
-object CoupleCodeGeneratorUtil { - fun generateCoupleCode(): String { - val characters = ('A'..'Z') + ('0'..'9') - return (1..6) - .map { characters.random() } - .joinToString("") - } -} +object CoupleCodeGeneratorUtil { + private val secureRandom = java.security.SecureRandom() + private val characters = ('A'..'Z') + ('0'..'9') + + fun generateCoupleCode(): String { + return (1..6) + .map { characters[secureRandom.nextInt(characters.size)] } + .joinToString("") + } +}src/main/kotlin/gomushin/backend/couple/dto/request/CoupleConnectRequest.kt (2)
3-5: 유효성 검사를 추가해 보세요.
coupleCode필드에 유효성 검사 어노테이션이 없습니다. 빈 문자열이나 null 값이 들어오지 않도록@NotBlank어노테이션을 추가하는 것이 좋습니다.+import javax.validation.constraints.NotBlank data class CoupleConnectRequest( + @NotBlank(message = "커플 코드는 필수입니다") val coupleCode: String )
1-5: KDoc 문서를 추가해 보세요.이 클래스의 목적을 명확히 하기 위해 KDoc 문서를 추가하는 것이 좋습니다.
package gomushin.backend.couple.dto.request +/** + * 커플 연결 요청에 사용되는 DTO + * @property coupleCode 커플 연결을 위한 고유 코드 + */ data class CoupleConnectRequest( val coupleCode: String )src/main/kotlin/gomushin/backend/core/oauth/service/CustomOAuth2UserService.kt (3)
36-36: 이름 설정 시 예외 처리를 추가해 보세요.OAuth 응답에서 받은
name값이 null이거나 빈 문자열일 경우를 처리하는 로직이 없습니다. 이런 경우 기본값을 제공하거나 오류를 발생시키는 처리가 필요합니다.- it.name = oAuth2Response.getName() + val name = oAuth2Response.getName() + it.name = if (name.isNullOrBlank()) "익명 사용자" else name
29-29: 오류 메시지를 프로젝트명에 맞게 수정해 보세요.오류 메시지에 "sarangggun.oauth.invalid-provider"라고 되어 있는데, 현재 프로젝트명은 "gomushin"입니다. 일관성을 위해 프로젝트명을 맞추는 것이 좋습니다.
- else -> throw BadRequestException("sarangggun.oauth.invalid-provider") + else -> throw BadRequestException("gomushin.oauth.invalid-provider")
54-54: 이름 설정 시 예외 처리를 추가해 보세요.새 회원 생성 시
name설정에 대한 예외 처리가 없습니다. OAuth 응답에서name값을 가져올 때 null이나 빈 문자열인 경우에 대한 처리가 필요합니다.- name = oAuth2Response.getName(), + name = oAuth2Response.getName().takeIf { !it.isNullOrBlank() } ?: "익명 사용자",src/main/kotlin/gomushin/backend/couple/domain/repository/CoupleRepository.kt (1)
1-6: KDoc 문서를 추가해 보세요.레포지토리 인터페이스에 대한 설명을 추가하면 이 인터페이스의 목적과 사용법을 더 명확하게 이해할 수 있습니다.
package gomushin.backend.couple.domain.repository import gomushin.backend.couple.domain.entity.Couple import org.springframework.data.jpa.repository.JpaRepository +/** + * 커플 관계 정보에 접근하기 위한 레포지토리 + * 커플 생성, 조회, 수정, 삭제 등의 기본 기능을 제공합니다. + */ interface CoupleRepository : JpaRepository<Couple, Long>src/main/kotlin/gomushin/backend/member/presentation/MemberInfoController.kt (2)
23-23: API 문서에 커플 정보가 포함됨을 명시해 보세요.반환 타입이
ApiResponse<MyInfoResponse>로 변경되었는데, 이제 API 응답에 커플 정보도 포함됨을 명시적으로 표시하는 것이 좋습니다.@GetMapping(ApiPath.MY_INFO) - @Operation(summary = "내 정보 조회", description = "getMyInfo") + @Operation( + summary = "내 정보 조회", + description = "회원의 기본 정보와 커플 연결 상태를 조회합니다", + responses = [ + io.swagger.v3.oas.annotations.responses.ApiResponse( + responseCode = "200", + description = "성공", + content = [Content(schema = Schema(implementation = MyInfoResponse::class))] + ) + ] + ) fun get( @AuthenticationPrincipal customUserDetails: CustomUserDetails ): ApiResponse<MyInfoResponse> {
22-26: 요청 처리 시 예외 처리를 추가해 보세요.사용자 정보 조회 시 발생할 수 있는 예외 상황(사용자를 찾을 수 없는 경우 등)에 대한 처리가 없습니다. 적절한 예외 처리를 추가하는 것이 좋습니다.
fun get( @AuthenticationPrincipal customUserDetails: CustomUserDetails ): ApiResponse<MyInfoResponse> { + if (customUserDetails.userId == null) { + throw BadRequestException("유효하지 않은 사용자입니다") + } val member = memberInfoFacade.getMemberInfo(customUserDetails) return ApiResponse.success(member) }src/main/kotlin/gomushin/backend/member/presentation/dto/response/MyInfoResponse.kt (1)
5-7: 응답 클래스 네이밍과 속성 타입에 대한 검토가 필요합니다.
GuestInfoResponse에서MyInfoResponse로의 이름 변경은 적절하지만,isCouple속성이Long타입인 것은 재검토가 필요합니다. 일반적으로 '~인지 아닌지'를 나타내는 속성은Boolean타입을 사용합니다. 만약 이 값이 커플 ID를 저장하는 용도라면,coupleId와 같은 더 명확한 이름을 사용하는 것이 좋을 것 같습니다.data class MyInfoResponse( val nickname: String, - val isCouple: Long, + val coupleId: Long?, // 커플 ID를 저장하는 경우 + val isCouple: Boolean, // 커플 상태만 표시하는 경우 ) {src/main/kotlin/gomushin/backend/couple/domain/entity/Couple.kt (1)
20-33: 날짜 필드에 대한 업데이트 메소드 추가를 고려해보세요.관계 시작/종료일, 군 복무 시작/종료일, 진급일 등 다양한 날짜 필드가 있지만, 이 필드들을 업데이트하는 메소드가 없습니다. 초기 구현에서 이 모든 필드가 필요한지 검토하고, 필요하다면 다음과 같은 업데이트 메소드 추가를 고려해보세요:
fun updateRelationshipDates(startDate: LocalDate?, endDate: LocalDate?) { this.relationshipStartDate = startDate this.relationshipEndDate = endDate } fun updateMilitaryDates(startDate: LocalDate?, endDate: LocalDate?) { this.militaryStartDate = startDate this.militaryEndDate = endDate } fun updateAdvancementDate(date: LocalDate?) { this.advancementDate = date }또한, 날짜 필드 간의 제약 조건(예: 시작일이 종료일보다 이전이어야 함)을 검증하는 로직 추가도 고려해보세요.
src/main/kotlin/gomushin/backend/couple/facade/CoupleFacade.kt (1)
16-16: 불필요한 빈 줄을 제거하세요.코드 가독성을 위해 불필요한 빈 줄은 제거하는 것이 좋습니다.
fun requestCoupleCodeGeneration(customUserDetails: CustomUserDetails) = coupleConnectService.generateCoupleCode(customUserDetails.getId()) - fun requestCoupleConnect(src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt (2)
39-40: Boolean 타입 사용 검토 필요커플 상태를 표현하기 위해 Long 타입(0L/1L)을 사용하고 있는데, 이는 일반적으로 Boolean 타입으로 표현하는 것이 더 명확합니다. 데이터베이스 스키마 변경이 필요하다면 마이그레이션 계획과 함께 검토해보세요.
-@Column(name = "is_couple", nullable = false) -var isCouple: Long = 0L, +@Column(name = "is_couple", nullable = false) +var isCouple: Boolean = false,
61-63: 커플 상태 업데이트 로직 검토 필요Boolean 타입을 사용하지 않는다면, 현재 구현된 토글 방식보다는 명시적인 상태 변경 메서드를 제공하는 것이 좋습니다. 또한 커플 연결/해제의 비즈니스 로직을 더 명확하게 표현할 수 있습니다.
-fun updateCoupleStatus() { - this.isCouple = if (this.isCouple == 0L) 1L else 0L -} +fun connectCouple() { + this.isCouple = 1L +} + +fun disconnectCouple() { + this.isCouple = 0L +}src/main/kotlin/gomushin/backend/couple/presentation/CoupleConnectController.kt (3)
14-16: Swagger 태그 이름과 설명이 일치하지 않습니다.Swagger 태그의 name은 "커플 코드 생성"이지만 컨트롤러는 생성뿐만 아니라 연결 기능도 포함하고 있습니다. 태그 이름을 "커플 연결 관리"와 같이 더 포괄적으로 변경하는 것이 좋습니다.
-@Tag(name = "커플 코드 생성", description = "CoupleConnectController") +@Tag(name = "커플 연결 관리", description = "CoupleConnectController")
31-34: API 설명에 포용적인 언어 사용 필요설명에 "남자친구(여자친구)"라는 표현은 특정 성별 관계만을 가정합니다. 더 포용적인 언어인 "파트너"나 "연인"으로 변경하는 것이 좋습니다.
- description = "커플 코드를 통해 남자친구(여자친구)와 연결합니다." + description = "커플 코드를 통해 파트너와 연결합니다."
38-41: 연결 결과 응답 개선 필요현재 연결 결과는 항상 true를 반환합니다. 연결 성공 여부를 실제로 반영하거나, 더 의미 있는 응답(예: 연결된 파트너 정보)을 제공하는 것이 API 사용자에게 더 유용할 수 있습니다.
-): ApiResponse<Boolean> { - coupleFacade.requestCoupleConnect(customUserDetails, request) - return ApiResponse.success(true) +): ApiResponse<CoupleConnectResponse> { + val result = coupleFacade.requestCoupleConnect(customUserDetails, request) + return ApiResponse.success(result)src/main/kotlin/gomushin/backend/couple/domain/service/CoupleConnectService.kt (1)
37-40: 예외 처리 메시지 개선 필요예외 메시지가 "sarangggun.couple.invalid-couple-code"와 같은 형태로 되어 있습니다. 이는 메시지 코드일 가능성이 높은데, 직접적인 메시지보다는 MessageSource를 통한 국제화 처리를 고려해보세요.
-val invitorId = getCoupleCodeOrNull(key) - ?: throw BadRequestException("sarangggun.couple.invalid-couple-code") -if (invitorId == userId) { - throw BadRequestException("sarangggun.couple.couple-code-same") +val invitorId = getCoupleCodeOrNull(key) + ?: throw BadRequestException(messageSource.getMessage("sarangggun.couple.invalid-couple-code", null, LocaleContextHolder.getLocale())) +if (invitorId == userId) { + throw BadRequestException(messageSource.getMessage("sarangggun.couple.couple-code-same", null, LocaleContextHolder.getLocale()))
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/main/kotlin/gomushin/backend/core/configuration/security/SecurityConfiguration.kt(1 hunks)src/main/kotlin/gomushin/backend/core/oauth/handler/CustomSuccessHandler.kt(1 hunks)src/main/kotlin/gomushin/backend/core/oauth/service/CustomOAuth2UserService.kt(2 hunks)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/CoupleConnectService.kt(1 hunks)src/main/kotlin/gomushin/backend/couple/dto/request/CoupleConnectRequest.kt(1 hunks)src/main/kotlin/gomushin/backend/couple/facade/CoupleFacade.kt(1 hunks)src/main/kotlin/gomushin/backend/couple/presentation/ApiPath.kt(1 hunks)src/main/kotlin/gomushin/backend/couple/presentation/CoupleConnectController.kt(1 hunks)src/main/kotlin/gomushin/backend/member/application/MemberInfoFacade.kt(1 hunks)src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt(2 hunks)src/main/kotlin/gomushin/backend/member/domain/service/MemberInfoService.kt(2 hunks)src/main/kotlin/gomushin/backend/member/presentation/MemberInfoController.kt(2 hunks)src/main/kotlin/gomushin/backend/member/presentation/dto/response/MyInfoResponse.kt(1 hunks)src/main/kotlin/gomushin/backend/member/util/CoupleCodeGeneratorUtil.kt(1 hunks)
🔇 Additional comments (18)
src/main/kotlin/gomushin/backend/couple/presentation/ApiPath.kt (1)
3-6: 상수 객체 적절히 구현되었습니다.API 경로 상수를 중앙화하여 관리하는 것은 좋은 접근 방식입니다. 이렇게 하면 경로 변경 시 한 곳에서만 수정하면 되므로 유지보수가 용이합니다.
src/main/kotlin/gomushin/backend/core/configuration/security/SecurityConfiguration.kt (1)
71-71:❓ Verification inconclusive
보안 설정이 더 엄격해졌습니다.
이전에는 인증된 사용자만 요구했지만, 이제는 "MEMBER" 역할이 있는 사용자만 접근 가능하도록 변경되었습니다. 이는 보안을 강화하는 좋은 변화이지만, 다음과 같은 점을 고려해야 합니다:
- 이 변경으로 "MEMBER" 역할이 없는 인증된 사용자들의 기존 접근이 차단될 수 있습니다.
- "MEMBER" 역할 할당 과정이 명확해야 합니다.
역할 변경의 영향도를 확인하기 위해 다음 스크립트를 실행해보세요:
🏁 Script executed:
#!/bin/bash # MEMBER 역할이 아닌 인증된 사용자가 있는지 확인 echo "Member 엔티티에서 ROLE_MEMBER가 아닌 사용자 검색..." find . -name "*.kt" | xargs grep -l "Member" | xargs grep -l "role" | xargs catLength of output: 17260
보안 설정 변경에 따른 역할 검증 요청
현재 보안 설정에서 모든 요청에 대해
"MEMBER"역할 보유 여부를 검사하도록 변경(it.anyRequest().hasRole("MEMBER"))되었습니다. 이는 보안 강화라는 측면에서는 긍정적이지만, 아래 사항들을 반드시 확인해 주시기 바랍니다:
인증된 사용자 역할 검증:
기존에 인증된 사용자라도 기본적으로Role.GUEST인 경우(예: 온보딩 과정을 거치지 않은 사용자)는 접근이 차단될 수 있습니다. 실제 비즈니스 플로우상 필요한 사용자가 모두Role.MEMBER로 전환되어야 하는지 확인해 주세요.역할 할당 프로세스 명확화:
OnboardingService와CustomOAuth2UserService에서 사용자 역할 할당이 일관되게 이루어지는지 점검해주시기 바랍니다. 특히, 기존 사용자와 신규 사용자에 대한 역할 업데이트 로직이 의도대로 동작하는지 검토해 주세요.이 변경 사항이 전체 시스템에 미치는 영향을 재검토 후, 필요시 추가 조치를 취해주시기 바랍니다.
src/main/kotlin/gomushin/backend/member/domain/service/MemberInfoService.kt (3)
6-6: Spring Data 확장 함수 사용 좋습니다.
findByIdOrNull확장 함수를 사용하면 null 처리가 더 간결해집니다.
16-18: 메소드 리팩토링이 잘 되었습니다.기존
getGuestInfo메소드를getById로 변경하고, 별도의findById메소드로 조회 로직을 분리한 것이 좋은 접근입니다. 조회 실패 시 예외 처리를 명확히 하고 있습니다.
20-23: Nullable 조회 메소드 추가가 유용합니다.
findById메소드를 별도로 추가하여 nullable한 Member를 반환하는 것은 다른 서비스에서 유연하게 활용할 수 있어 좋은 디자인입니다. 이러한 접근 방식은 커플 연결 기능 구현 시 더 많은 유연성을 제공할 것입니다.src/main/kotlin/gomushin/backend/member/application/MemberInfoFacade.kt (2)
5-5: 응답 타입 변경이 적절합니다.
GuestInfoResponse에서MyInfoResponse로 변경한 것은 데이터 모델 변경에 맞게 잘 대응했습니다.
12-15: 메소드 리팩토링이 코드를 간결하게 만들었습니다.기존의 조건부 로직이 제거되고 반환 타입이
Any에서 구체적인MyInfoResponse로 변경된 것은 타입 안정성 측면에서 좋은 개선입니다. 간결해진 로직으로 코드 가독성이 향상되었습니다.다만, 이전 코드에서 역할(role)에 따른 분기 처리가 있었던 것으로 보이는데, 해당 로직이 더 이상 필요하지 않은지 확인해 보세요.
src/main/kotlin/gomushin/backend/member/presentation/dto/response/MyInfoResponse.kt (1)
10-13: 팩토리 메소드가 잘 구현되었습니다.
of메소드가 Member 엔티티의 모든 필요 속성을 매핑하여 응답 객체를 생성하고 있습니다.src/main/kotlin/gomushin/backend/couple/domain/entity/Couple.kt (3)
7-9: 커플 엔티티 설계가 적절합니다.JPA 엔티티로 잘 설계되었으며, 테이블 명명도 적절합니다.
10-19: ID 필드와 관계 필드가 잘 정의되었습니다.ID 필드의 생성 전략과 invitorId, inviteeId 필드가 필수값으로 잘 설정되어 있습니다. 커플 관계의 양측을 명확히 구분한 점이 좋습니다.
35-45: 팩토리 메소드가 잘 구현되었습니다.최소한의 필수 필드만으로 객체를 생성하는 팩토리 메소드가 잘 구현되어 있습니다. 이는 코드의 가독성과 사용성을 높여줍니다.
src/main/kotlin/gomushin/backend/couple/facade/CoupleFacade.kt (2)
13-14: 메서드 구현이 깔끔합니다.코드가 간결하게 구현되어 있으며, CustomUserDetails에서 ID를 추출하여 서비스 레이어로 잘 위임하고 있습니다.
17-20: 메서드 구현이 깔끔합니다.requestCoupleConnect 메서드가 적절히 구현되어 있습니다. CustomUserDetails와 요청 객체를 받아 서비스 계층으로 잘 위임하고 있습니다.
src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt (3)
16-17: 필드 추가가 적절합니다.name 필드가 nullable=false로 적절하게 추가되었습니다.
45-47: 팩토리 메서드 파라미터 변경이 적절합니다.name을 필수 파라미터로 추가하고 nickname을 nullable로 변경한 것은 적절합니다.
52-53: nickname 기본값 설정이 적절합니다.nickname이 null일 경우 name을 기본값으로 사용하는 구현이 적절합니다.
src/main/kotlin/gomushin/backend/couple/domain/service/CoupleConnectService.kt (2)
20-23: 상수 정의가 적절합니다.커플 코드의 유효 시간과 Redis 키 접두사를 상수로 정의한 것은 좋은 방식입니다.
25-31: 커플 코드 생성 로직이 적절합니다.기존 코드를 삭제하고 새 코드를 생성하여 Redis에 저장하는 로직이 잘 구현되어 있습니다.
src/main/kotlin/gomushin/backend/core/oauth/handler/CustomSuccessHandler.kt
Show resolved
Hide resolved
src/main/kotlin/gomushin/backend/couple/domain/repository/CoupleRepository.kt
Show resolved
Hide resolved
src/main/kotlin/gomushin/backend/couple/domain/service/CoupleConnectService.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/gomushin/backend/couple/domain/service/CoupleConnectService.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/gomushin/backend/couple/domain/service/CoupleConnectService.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/kotlin/gomushin/backend/couple/domain/service/CoupleConnectService.kt (1)
33-53: 🛠️ Refactor suggestion트랜잭션 일관성 개선 필요
connectCouple메서드에서 Redis 키를 검증 후 마지막에 삭제하고 있습니다. 만약 Couple 저장 과정에서 예외가 발생하면 Redis 키는 이미 삭제되어 사용자가 다시 시도할 수 없게 됩니다.save(couple) delete(key)추가로, 트랜잭션의 일관성을 위해 Redis 키 삭제와 커플 생성이 하나의 논리적 트랜잭션으로 처리되어야 하지만, Redis는 Spring의 트랜잭션 관리 범위에 포함되지 않습니다. 이를 해결하기 위한 방법을 고려해보세요.
🧹 Nitpick comments (3)
src/main/kotlin/gomushin/backend/couple/domain/service/CoupleConnectService.kt (3)
47-49: 커플 상태 검증 로직 분리 필요커플 상태 검증 로직이
connectCouple메서드 내에서만 호출되고 있습니다.save메서드에서도 이 검증을 수행해야 다른 경로로save가 호출될 경우에도 검증이 이루어집니다.@Transactional fun save(couple: Couple): Couple { + if (isAlreadyConnected(couple.invitorId) || isAlreadyConnected(couple.inviteeId)) { + throw BadRequestException("sarangggun.couple.already-connected") + } updateCoupleStatus(couple.invitorId) updateCoupleStatus(couple.inviteeId) return coupleRepository.save(couple) }
82-88: 중복 코드 제거 필요
isAlreadyConnected메서드와updateCoupleStatus메서드에서 동일한 방식으로 Member를 조회하고 있습니다. 이 중복 코드를 제거하기 위해 회원 조회 로직을 별도의 메서드로 분리하는 것이 좋습니다.+private fun findMemberById(userId: Long) = + memberRepository.findById(userId).orElseThrow { + BadRequestException("sarangggun.member.not-found") + } @Transactional fun updateCoupleStatus(userId: Long) { - val member = memberRepository.findById(userId).orElseThrow { - BadRequestException("sarangggun.member.not-found") - } + val member = findMemberById(userId) member.updateCoupleStatus() } private fun isAlreadyConnected(userId: Long): Boolean { - val member = memberRepository.findById(userId).orElseThrow { - BadRequestException("sarangggun.member.not-found") - } + val member = findMemberById(userId) return member.isCouple > 0 }
1-89: 로깅 부재서비스 메서드 내에 로깅이 없습니다. 디버깅과 문제 추적을 위해 주요 작업 지점에 로깅을 추가하는 것이 좋습니다. 특히 커플 코드 생성, 커플 연결 성공, 실패와 같은 중요 이벤트에 대한 로깅이 필요합니다.
+import org.slf4j.LoggerFactory + @Service class CoupleConnectService( private val redisTemplate: StringRedisTemplate, private val coupleRepository: CoupleRepository, private val memberRepository: MemberRepository, ) { + + private val logger = LoggerFactory.getLogger(CoupleConnectService::class.java) // 로깅 추가 예시 fun generateCoupleCode(userId: Long): String { + logger.info("커플 코드 생성 시작 - userId: {}", userId) delete(userId.toString()) val coupleCode = CoupleCodeGeneratorUtil.generateCoupleCode() val key = getCoupleCodeKey(coupleCode) redisTemplate.opsForValue().set(key, userId.toString(), CODE_DURATION) + logger.info("커플 코드 생성 완료 - userId: {}", userId) return coupleCode }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/kotlin/gomushin/backend/couple/domain/service/CoupleConnectService.kt(1 hunks)
🔇 Additional comments (1)
src/main/kotlin/gomushin/backend/couple/domain/service/CoupleConnectService.kt (1)
62-68: 안전한 Optional 처리 구현 확인이전 리뷰에서 지적된 안전하지 않은 Optional 처리 문제를
orElseThrow를 사용하여 적절히 해결하였습니다. 이는 좋은 접근 방식입니다.
| fun generateCoupleCode(userId: Long): String { | ||
| delete(userId.toString()) | ||
| val coupleCode = CoupleCodeGeneratorUtil.generateCoupleCode() | ||
| val key = getCoupleCodeKey(coupleCode) | ||
| redisTemplate.opsForValue().set(key, userId.toString(), CODE_DURATION) | ||
| return coupleCode | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
코드 생성 전략 개선 필요
현재 코드는 새 커플 코드를 생성하기 전에 기존 코드를 삭제합니다. 이 방식은 만약 새 코드 생성 과정에서 오류가 발생할 경우 사용자의 기존 코드가 손실될 수 있습니다.
fun generateCoupleCode(userId: Long): String {
- delete(userId.toString())
val coupleCode = CoupleCodeGeneratorUtil.generateCoupleCode()
val key = getCoupleCodeKey(coupleCode)
redisTemplate.opsForValue().set(key, userId.toString(), CODE_DURATION)
+ delete(userId.toString())
return coupleCode
}📝 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.
| fun generateCoupleCode(userId: Long): String { | |
| delete(userId.toString()) | |
| val coupleCode = CoupleCodeGeneratorUtil.generateCoupleCode() | |
| val key = getCoupleCodeKey(coupleCode) | |
| redisTemplate.opsForValue().set(key, userId.toString(), CODE_DURATION) | |
| return coupleCode | |
| } | |
| fun generateCoupleCode(userId: Long): String { | |
| val coupleCode = CoupleCodeGeneratorUtil.generateCoupleCode() | |
| val key = getCoupleCodeKey(coupleCode) | |
| redisTemplate.opsForValue().set(key, userId.toString(), CODE_DURATION) | |
| delete(userId.toString()) | |
| return coupleCode | |
| } |
🛠️ Refactor suggestion
Redis 키 관리 전략 개선
현재 구현에서는 사용자 ID를 직접 키로 삭제하고, 다른 곳에서는 커플 코드로 키를 만들어 사용합니다. 이는 혼란을 줄 수 있으며, Redis에 사용자당 여러 키가 남을 수 있는 위험이 있습니다.
사용자 ID로 검색 가능한 보조 인덱스나 일관된 키 관리 전략을 구현하세요:
fun generateCoupleCode(userId: Long): String {
- delete(userId.toString())
val coupleCode = CoupleCodeGeneratorUtil.generateCoupleCode()
val key = getCoupleCodeKey(coupleCode)
+
+ // 사용자 ID로 이전 코드를 찾아 삭제
+ val userKey = getUserKey(userId)
+ val previousCode = redisTemplate.opsForValue().get(userKey)
+ if (previousCode != null) {
+ delete(getCoupleCodeKey(previousCode))
+ }
+
redisTemplate.opsForValue().set(key, userId.toString(), CODE_DURATION)
+ // 사용자 ID를 키로 현재 코드 저장 (인덱스 역할)
+ redisTemplate.opsForValue().set(userKey, coupleCode, CODE_DURATION)
return coupleCode
}
+private fun getUserKey(userId: Long): String {
+ return "USER_CODE:$userId"
+}📝 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.
| fun generateCoupleCode(userId: Long): String { | |
| delete(userId.toString()) | |
| val coupleCode = CoupleCodeGeneratorUtil.generateCoupleCode() | |
| val key = getCoupleCodeKey(coupleCode) | |
| redisTemplate.opsForValue().set(key, userId.toString(), CODE_DURATION) | |
| return coupleCode | |
| } | |
| fun generateCoupleCode(userId: Long): String { | |
| - delete(userId.toString()) | |
| + // 사용자 ID로 이전 코드를 찾아 삭제 | |
| + val userKey = getUserKey(userId) | |
| + val previousCode = redisTemplate.opsForValue().get(userKey) | |
| + if (previousCode != null) { | |
| + delete(getCoupleCodeKey(previousCode)) | |
| + } | |
| val coupleCode = CoupleCodeGeneratorUtil.generateCoupleCode() | |
| val key = getCoupleCodeKey(coupleCode) | |
| - redisTemplate.opsForValue().set(key, userId.toString(), CODE_DURATION) | |
| + redisTemplate.opsForValue().set(key, userId.toString(), CODE_DURATION) | |
| + // 사용자 ID를 키로 현재 코드 저장 (인덱스 역할) | |
| + redisTemplate.opsForValue().set(userKey, coupleCode, CODE_DURATION) | |
| return coupleCode | |
| } | |
| + private fun getUserKey(userId: Long): String { | |
| + return "USER_CODE:$userId" | |
| + } |
HoyeongJeon
force-pushed
the
issue/#12-couple-connect
branch
from
e3507ee to
57ad9a2
Compare
| var relationshipStartDate: LocalDate? = null, | ||
|
|
||
| @Column(name = "relationship_end_date") | ||
| var relationshipEndDate: LocalDate? = null, |
There was a problem hiding this comment.
요건 헤어진 날인가? 헤어지면 관련데이터 다 삭제되는데 컬럼이 필요한 이유가 있을지는 모르겠어
There was a problem hiding this comment.
이건 탈퇴 후 한 달 유예를 둘 수 있으니까 넣어놨는데.. 오늘 모각작 할 때 탈퇴 다시 물어볼게
| val id: Long = 0L, | ||
|
|
||
| @Column(name = "invitor_id", nullable = false) | ||
| val invitorId: Long = 0L, |
There was a problem hiding this comment.
유저 엔티티 값이라고 erd cloud에 나와있는데 그럼 user pk값이라고 봐도 무방해?
| ) : BaseEntity() { | ||
|
|
||
| @Column(name = "is_couple", nullable = false) | ||
| var isCouple: Long = 0L, |
There was a problem hiding this comment.
이거는 커플여부 판단하는건가? 만약 그렇다면 boolean으로 타입선언해도 되지 않아?
✅ PR 유형
어떤 변경 사항이 있었나요?
📝 작업 내용
이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
✏️ 관련 이슈
🎸 기타 사항 or 추가 코멘트
일단 현규랑 얘기해서 코드는 60분 기준으로 사라지도록 했어
연결 테스트는 했는데 잘 됐고, 테스트 코드는 다른 이슈 파서 적을 것 같아
Summary by CodeRabbit