MA_02 구현 #22 #25
MA_02 구현 #22 #25
Conversation
Walkthrough이번 변경 사항에서는 회원의 감정(emotion)과 상태 메시지(status message)를 조회 및 수정할 수 있는 기능이 새롭게 도입되었습니다. Member 엔티티의 emotion 속성이 String에서 Int?로 변경되었고, 관련 갱신 메서드가 추가되었습니다. 이를 위한 요청/응답 DTO와 서비스, 파사드, 컨트롤러 계층이 연동되었으며, API 경로도 새로 정의되었습니다. 인증 성공 후 리다이렉트 URL과 쿠키 secure 플래그도 수정되었습니다. 테스트 코드 역시 새로운 기능을 검증하는 테스트가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller
participant Facade
participant Service
participant MemberEntity
User->>Controller: POST /v1/member/my-emotion-and-status-message (emotion, statusMessage)
Controller->>Facade: updateMyEmotionAndStatusMessage(customUserDetails, request)
Facade->>Service: updateMyEmotionAndStatusMessage(memberId, request)
Service->>MemberEntity: updateEmotion(request.emotion)
Service->>MemberEntity: updateStatusMessage(request.statusMessage)
Service-->>Facade: 완료
Facade-->>Controller: 완료
Controller-->>User: ApiResponse<Boolean>(true)
sequenceDiagram
participant User
participant Controller
participant Facade
participant Service
participant MemberEntity
User->>Controller: GET /v1/member/my-status-message
Controller->>Facade: getMyStatusMessage(customUserDetails)
Facade->>Service: getById(memberId)
Service-->>Facade: MemberEntity
Facade->>Facade: MyStatusMessageResponse.of(MemberEntity)
Facade-->>Controller: MyStatusMessageResponse
Controller-->>User: ApiResponse<MyStatusMessageResponse>
Poem
✨ 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: 4
🧹 Nitpick comments (9)
src/main/kotlin/gomushin/backend/member/domain/service/MemberService.kt (1)
26-31: 반환 값 추가 고려업데이트 메서드의 구현이 적절하게 작성되었습니다. 그러나 업데이트된 Member 엔티티를 반환하는 것이 API 소비자에게 더 유용할 수 있습니다. 이렇게 하면 업데이트가 성공적으로 적용되었는지 확인할 수 있습니다.
-@Transactional -fun updateMyEmotionAndStatusMessage(id: Long, updateMyEmotionAndStatusMessageRequest: UpdateMyEmotionAndStatusMessageRequest) { +@Transactional +fun updateMyEmotionAndStatusMessage(id: Long, updateMyEmotionAndStatusMessageRequest: UpdateMyEmotionAndStatusMessageRequest): Member { val member = getById(id) member.updateEmotion(updateMyEmotionAndStatusMessageRequest.emotion) member.updateStatusMessage(updateMyEmotionAndStatusMessageRequest.statusMessage) + return member }src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt (1)
7-7: 사용되지 않는 import 제거 필요
@Transactional어노테이션이 import 되었지만 이 클래스 내에서 실제로 사용되지 않습니다. 사용하지 않는 import는 제거하는 것이 좋습니다.-import org.springframework.transaction.annotation.Transactionalsrc/main/kotlin/gomushin/backend/member/facade/MemberInfoFacade.kt (1)
24-25: 코드 스타일 일관성 개선 제안이 메소드는 표현식 본문(expression body) 구문을 사용하여 구현되었는데, 클래스 내 다른 메소드들이 블록 본문(block body)을 사용하고 있어 일관성이 떨어집니다. 코드 스타일의 일관성을 위해 블록 본문으로 변경하는 것을 고려해보세요.
-fun updateMyEmotionAndStatusMessage(customUserDetails: CustomUserDetails, updateMyEmotionAndStatusMessageRequest: UpdateMyEmotionAndStatusMessageRequest) - = memberService.updateMyEmotionAndStatusMessage(customUserDetails.getId(), updateMyEmotionAndStatusMessageRequest) +fun updateMyEmotionAndStatusMessage(customUserDetails: CustomUserDetails, updateMyEmotionAndStatusMessageRequest: UpdateMyEmotionAndStatusMessageRequest) { + return memberService.updateMyEmotionAndStatusMessage(customUserDetails.getId(), updateMyEmotionAndStatusMessageRequest) +}src/main/kotlin/gomushin/backend/member/dto/request/UpdateMyEmotionAndStatusMessageRequest.kt (1)
13-15: 에러 메시지 재사용성 검토세 개의 다른 validation 어노테이션에 동일한 에러 메시지가 사용되었습니다. 이는 의도적인 것으로 보이지만, 다양한 유효성 검증 실패 상황에 대해 좀 더 구체적인 에러 메시지를 제공하는 것이 사용자 경험 측면에서 더 좋을 수 있습니다.
-@field:NotNull(message = "sarangggun.member.invalid-emotion") -@field:Min(1, message = "sarangggun.member.invalid-emotion") -@field:Max(7, message = "sarangggun.member.invalid-emotion") +@field:NotNull(message = "sarangggun.member.missing-emotion") +@field:Min(1, message = "sarangggun.member.emotion-too-small") +@field:Max(7, message = "sarangggun.member.emotion-too-large")src/main/kotlin/gomushin/backend/core/infrastructure/exception/handler/GlobalExceptionHandler.kt (2)
19-40: 글로벌 예외 처리기 구현이 좋습니다만, 범위를 확장하면 좋을 것 같습니다.현재
MethodArgumentNotValidException만 처리하고 있는데, 임포트된ConstraintViolationException도 처리하는 메서드를 추가하는 것이 좋을 것 같습니다. 이는@RequestParam및@PathVariable에 대한 검증 실패를 처리할 수 있습니다.아래와 같이
ConstraintViolationException처리 메서드를 추가하는 것을 고려해보세요:@ExceptionHandler(ConstraintViolationException::class) @ResponseStatus(HttpStatus.BAD_REQUEST) fun handleConstraintViolationException(ex: ConstraintViolationException): ApiResponse<Nothing> { val code = ex.constraintViolations .firstOrNull() ?.message ?: "bad-request" return ApiResponse.error( ApiError.of( ApiErrorElement( SpringContextHolder.getBean(AppEnv::class.java).getId(), ExtendedHttpStatus.BAD_REQUEST, ApiErrorCode.of(code), ApiErrorMessage.of(code) ) ) ) }
21-40: 에러 메시지를 개별적으로 처리하는 것이 더 좋을 수 있습니다.현재 첫 번째 필드 에러의 메시지만 반환하고 있습니다. 클라이언트 입장에서는 모든 유효성 검증 오류를 한 번에 확인하는 것이 더 유용할 수 있습니다.
모든 검증 오류를 포함하는 방식으로 리팩토링하는 것을 고려해보세요. 예를 들어:
@ExceptionHandler(MethodArgumentNotValidException::class) @ResponseStatus(HttpStatus.BAD_REQUEST) fun handleMethodArgumentNotValidException(ex: MethodArgumentNotValidException): ApiResponse<Nothing> { val errors = ex.bindingResult.fieldErrors.map { mapOf( "field" to it.field, "message" to (it.defaultMessage ?: "invalid") ) } val code = "validation-failed" return ApiResponse.error( ApiError.of( ApiErrorElement( SpringContextHolder.getBean(AppEnv::class.java).getId(), ExtendedHttpStatus.BAD_REQUEST, ApiErrorCode.of(code), ApiErrorMessage.of(code), errors ) ) ) }이 접근 방식을 사용하기 위해서는
ApiErrorElement클래스에 추가 필드를 지원하는지 확인하세요.src/main/kotlin/gomushin/backend/member/presentation/MemberInfoController.kt (3)
5-6: 사용하지 않는 임포트가 있습니다.
BadRequestException을 임포트했지만 코드에서 사용하지 않고 있습니다.사용하지 않는 임포트를 제거하세요:
- import gomushin.backend.core.infrastructure.exception.BadRequestException
15-16: 사용하지 않는 임포트가 있습니다.
BindingResult를 임포트했지만 코드에서 사용하지 않고 있습니다.사용하지 않는 임포트를 제거하세요:
- import org.springframework.validation.BindingResult
50-59: 상태 이모지 및 메시지 업데이트 API 구현이 잘 되었습니다.유효성 검증을 위해
@Valid어노테이션을 사용한 것이 좋습니다. 이는 새로 구현된GlobalExceptionHandler와 함께 작동하여 입력값 검증에 실패할 경우 적절한 오류 응답을 반환할 것입니다.다만, 업데이트 성공 시 단순히
true를 반환하는 것보다 어떤 내용이 업데이트되었는지 더 상세한 응답을 제공하는 것이 API 소비자에게 더 유용할 수 있습니다.업데이트된 정보를 응답으로 반환하는 것을 고려해보세요:
- fun updateMyEmotionAndStatusMessage( - @AuthenticationPrincipal customUserDetails: CustomUserDetails, - @Valid @RequestBody updateMyEmotionAndStatusMessageRequest: UpdateMyEmotionAndStatusMessageRequest - ): ApiResponse<Boolean> { - memberInfoFacade.updateMyEmotionAndStatusMessage(customUserDetails, updateMyEmotionAndStatusMessageRequest) - return ApiResponse.success(true) + fun updateMyEmotionAndStatusMessage( + @AuthenticationPrincipal customUserDetails: CustomUserDetails, + @Valid @RequestBody updateMyEmotionAndStatusMessageRequest: UpdateMyEmotionAndStatusMessageRequest + ): ApiResponse<UpdateMyEmotionAndStatusMessageResponse> { + val result = memberInfoFacade.updateMyEmotionAndStatusMessage(customUserDetails, updateMyEmotionAndStatusMessageRequest) + return ApiResponse.success(result)이 변경을 위해서는 Facade 및 Service 계층에서도 업데이트된 정보를 반환하도록 수정이 필요합니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/kotlin/gomushin/backend/core/infrastructure/exception/handler/GlobalExceptionHandler.kt(1 hunks)src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt(3 hunks)src/main/kotlin/gomushin/backend/member/domain/service/MemberService.kt(2 hunks)src/main/kotlin/gomushin/backend/member/dto/request/UpdateMyEmotionAndStatusMessageRequest.kt(1 hunks)src/main/kotlin/gomushin/backend/member/dto/response/MyStatusMessageResponse.kt(1 hunks)src/main/kotlin/gomushin/backend/member/facade/MemberInfoFacade.kt(2 hunks)src/main/kotlin/gomushin/backend/member/presentation/ApiPath.kt(1 hunks)src/main/kotlin/gomushin/backend/member/presentation/MemberInfoController.kt(2 hunks)src/test/kotlin/gomushin/backend/member/domain/service/MemberServiceTest.kt(2 hunks)src/test/kotlin/gomushin/backend/member/facade/MemberInfoFacadeTest.kt(3 hunks)
🔇 Additional comments (9)
src/main/kotlin/gomushin/backend/member/presentation/ApiPath.kt (1)
6-7: API 경로 상수 추가 적절새로 추가된 API 경로 상수들이 기존 패턴을 잘 따르고 있으며, RESTful 원칙에 부합합니다. 상태 메시지 조회와 업데이트를 위한 엔드포인트 경로가 명확하게 정의되어 있습니다.
src/main/kotlin/gomushin/backend/member/dto/response/MyStatusMessageResponse.kt (1)
5-13: 응답 DTO 구현 적절응답 DTO가 간결하고 목적에 맞게 잘 구현되었습니다.
statusMessage를 nullable로 정의한 것은 모든 회원이 상태 메시지를 설정하지 않을 수 있음을 고려한 좋은 설계입니다. companion object의 factory 메서드를 통해 Member 엔티티로부터 응답 객체를 쉽게 생성할 수 있어 편리합니다.src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt (1)
46-47: 새로운 감정 상태 필드 추가 확인새로운
emotion필드가 nullable로 적절하게 추가되었습니다. 이는 회원의 감정 상태를 나타내기 위한 필드입니다.src/test/kotlin/gomushin/backend/member/facade/MemberInfoFacadeTest.kt (2)
41-41: 상태 메시지 초기화 확인테스트를 위한 Member 객체 생성 시 statusMessage 필드가 적절히 추가되었습니다.
62-72: 상태 메시지 조회 테스트 구현 확인상태 메시지 조회 기능에 대한 테스트가 적절하게 구현되었습니다. 테스트는 given-when-then 구조를 따르고 있으며, memberService와의 상호작용 및 결과값 검증이 포함되어 있습니다.
src/main/kotlin/gomushin/backend/member/facade/MemberInfoFacade.kt (1)
19-22: 상태 메시지 조회 메소드 구현 검토상태 메시지 조회 메소드가 적절하게 구현되었습니다. 사용자 식별을 위해 CustomUserDetails를 사용하고, Member 엔티티로부터 응답 DTO를 생성하는 패턴을 일관되게 따르고 있습니다.
src/main/kotlin/gomushin/backend/member/dto/request/UpdateMyEmotionAndStatusMessageRequest.kt (1)
10-22: 감정 및 상태 메시지 업데이트 요청 DTO 구현 검토감정(emotion)과 상태 메시지(statusMessage)를 업데이트하기 위한 DTO 클래스가 적절하게 설계되었습니다. 다음과 같은 장점이 있습니다:
- Swagger 스키마 어노테이션을 사용하여 API 문서화가 잘 되어 있습니다.
- 각 필드에 대한 유효성 검증이 적절히 구현되어 있습니다.
- 에러 메시지가 일관된 형식으로 정의되어 있어 국제화(i18n)에 적합합니다.
emotion 필드의 1~7까지의 값이 어떤 감정을 나타내는지 명확하게 설명되어 있어 개발자와 API 사용자 모두에게 도움이 됩니다.
src/main/kotlin/gomushin/backend/member/presentation/MemberInfoController.kt (2)
40-48: 상태 메시지 조회 API 구현이 잘 되었습니다.메서드가 명확하게 구현되어 있고, 인증된 사용자만 접근할 수 있는 보안 구조도 잘 갖추어져 있습니다. 응답 타입도 적절하게
ApiResponse<MyStatusMessageResponse>로 지정되어 있습니다.
23-25: 컨트롤러에 @validated 어노테이션 추가가 적절합니다.
@Validated어노테이션을 추가하여 요청 본문뿐만 아니라 경로 변수나 요청 파라미터에 대한 유효성 검증을 지원하도록 한 것이 좋습니다.
| @DisplayName("이모지 및 상태 메시지 업데이트 - 성공") | ||
| @Test | ||
| fun updateMyEmotionAndStatusMessage() { | ||
| // given | ||
| val memberId = 1L | ||
| val expectedMember = Member( | ||
| id = 1L, | ||
| name = "테스트", | ||
| nickname = "테스트 닉네임", | ||
| email = "[email protected]", | ||
| birthDate = null, | ||
| profileImageUrl = null, | ||
| provider = Provider.KAKAO, | ||
| role = Role.GUEST, | ||
| emotion = 1, | ||
| statusMessage = "상태 변경전" | ||
| ) | ||
| val updateMyEmotionAndStatusMessageRequest = UpdateMyEmotionAndStatusMessageRequest(2, "상태 변경후") | ||
| //when | ||
| `when`(memberRepository.findById(memberId)).thenReturn(Optional.of(expectedMember)) | ||
| val result = memberService.updateMyEmotionAndStatusMessage(memberId, updateMyEmotionAndStatusMessageRequest) | ||
| //then | ||
| assertEquals(expectedMember.emotion, updateMyEmotionAndStatusMessageRequest.emotion) | ||
| assertEquals(expectedMember.statusMessage, updateMyEmotionAndStatusMessageRequest.statusMessage) | ||
| } |
There was a problem hiding this comment.
테스트 메서드 개선 필요
테스트 구현에 몇 가지 개선이 필요합니다:
- 서비스 구현에서는
findByIdOrNull을 사용하지만 테스트에서는findById를 모킹하고 있어 불일치가 있습니다. - 서비스 메서드가 현재 아무것도 반환하지 않기 때문에 71번 줄의
result변수는 불필요합니다.
@DisplayName("이모지 및 상태 메시지 업데이트 - 성공")
@Test
fun updateMyEmotionAndStatusMessage() {
// given
val memberId = 1L
val expectedMember = Member(
id = 1L,
name = "테스트",
nickname = "테스트 닉네임",
email = "[email protected]",
birthDate = null,
profileImageUrl = null,
provider = Provider.KAKAO,
role = Role.GUEST,
emotion = 1,
statusMessage = "상태 변경전"
)
val updateMyEmotionAndStatusMessageRequest = UpdateMyEmotionAndStatusMessageRequest(2, "상태 변경후")
//when
- `when`(memberRepository.findById(memberId)).thenReturn(Optional.of(expectedMember))
- val result = memberService.updateMyEmotionAndStatusMessage(memberId, updateMyEmotionAndStatusMessageRequest)
+ `when`(memberRepository.findByIdOrNull(memberId)).thenReturn(expectedMember)
+ memberService.updateMyEmotionAndStatusMessage(memberId, updateMyEmotionAndStatusMessageRequest)
//then
assertEquals(expectedMember.emotion, updateMyEmotionAndStatusMessageRequest.emotion)
assertEquals(expectedMember.statusMessage, updateMyEmotionAndStatusMessageRequest.statusMessage)
}📝 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.
| @DisplayName("이모지 및 상태 메시지 업데이트 - 성공") | |
| @Test | |
| fun updateMyEmotionAndStatusMessage() { | |
| // given | |
| val memberId = 1L | |
| val expectedMember = Member( | |
| id = 1L, | |
| name = "테스트", | |
| nickname = "테스트 닉네임", | |
| email = "[email protected]", | |
| birthDate = null, | |
| profileImageUrl = null, | |
| provider = Provider.KAKAO, | |
| role = Role.GUEST, | |
| emotion = 1, | |
| statusMessage = "상태 변경전" | |
| ) | |
| val updateMyEmotionAndStatusMessageRequest = UpdateMyEmotionAndStatusMessageRequest(2, "상태 변경후") | |
| //when | |
| `when`(memberRepository.findById(memberId)).thenReturn(Optional.of(expectedMember)) | |
| val result = memberService.updateMyEmotionAndStatusMessage(memberId, updateMyEmotionAndStatusMessageRequest) | |
| //then | |
| assertEquals(expectedMember.emotion, updateMyEmotionAndStatusMessageRequest.emotion) | |
| assertEquals(expectedMember.statusMessage, updateMyEmotionAndStatusMessageRequest.statusMessage) | |
| } | |
| @DisplayName("이모지 및 상태 메시지 업데이트 - 성공") | |
| @Test | |
| fun updateMyEmotionAndStatusMessage() { | |
| // given | |
| val memberId = 1L | |
| val expectedMember = Member( | |
| id = 1L, | |
| name = "테스트", | |
| nickname = "테스트 닉네임", | |
| email = "[email protected]", | |
| birthDate = null, | |
| profileImageUrl = null, | |
| provider = Provider.KAKAO, | |
| role = Role.GUEST, | |
| emotion = 1, | |
| statusMessage = "상태 변경전" | |
| ) | |
| val updateMyEmotionAndStatusMessageRequest = UpdateMyEmotionAndStatusMessageRequest(2, "상태 변경후") | |
| //when | |
| `when`(memberRepository.findByIdOrNull(memberId)).thenReturn(expectedMember) | |
| memberService.updateMyEmotionAndStatusMessage(memberId, updateMyEmotionAndStatusMessageRequest) | |
| //then | |
| assertEquals(expectedMember.emotion, updateMyEmotionAndStatusMessageRequest.emotion) | |
| assertEquals(expectedMember.statusMessage, updateMyEmotionAndStatusMessageRequest.statusMessage) | |
| } |
| fun updateEmotion(emotion: Int) { | ||
| this.emotion = emotion | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
emotion 값 유효성 검증 추가 필요
updateEmotion 메소드에서 입력값의 유효성 검증이 누락되었습니다. DTO에서는 1에서 7 사이의 값으로 제한하고 있지만, 엔티티 수준에서도 이 검증이 필요합니다.
fun updateEmotion(emotion: Int) {
+ require(emotion in 1..7) { "감정 값은 1에서 7 사이여야 합니다." }
this.emotion = emotion
}| fun updateStatusMessage(statusMessage: String) { | ||
| this.statusMessage = statusMessage | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상태 메시지 길이 제한 검증 누락
updateStatusMessage 메소드에서 상태 메시지 길이에 대한 유효성 검증이 누락되었습니다. DTO에서는 최대 25자로 제한하고 있지만, 엔티티 수준에서도 이 검증이 필요합니다.
fun updateStatusMessage(statusMessage: String) {
+ require(statusMessage.length <= 25) { "상태 메시지는 25자 이하여야 합니다." }
this.statusMessage = statusMessage
}📝 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 updateStatusMessage(statusMessage: String) { | |
| this.statusMessage = statusMessage | |
| } | |
| fun updateStatusMessage(statusMessage: String) { | |
| require(statusMessage.length <= 25) { "상태 메시지는 25자 이하여야 합니다." } | |
| this.statusMessage = statusMessage | |
| } |
| @DisplayName("이모지 및 상태 메시지 업데이트") | ||
| @Test | ||
| fun updateMyEmotionAndStatusMessage() { | ||
| //given | ||
| val updateMyEmotionAndStatusMessageRequest = UpdateMyEmotionAndStatusMessageRequest(1, "좋은 날씨야") | ||
| //when | ||
| val result = memberInfoFacade.updateMyEmotionAndStatusMessage(customUserDetails, updateMyEmotionAndStatusMessageRequest) | ||
| //then | ||
| verify(memberService).updateMyEmotionAndStatusMessage(1L, updateMyEmotionAndStatusMessageRequest) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
이모지 및 상태 메시지 업데이트 테스트 개선 필요
이모지 및 상태 메시지 업데이트 기능에 대한 테스트가 구현되었지만, 결과값에 대한 구체적인 검증이 누락되었습니다. 서비스 메소드 호출 여부만 검증하고 있어, 실제 반환값이나 상태 변경에 대한 검증이 필요합니다.
@DisplayName("이모지 및 상태 메시지 업데이트")
@Test
fun updateMyEmotionAndStatusMessage() {
//given
val updateMyEmotionAndStatusMessageRequest = UpdateMyEmotionAndStatusMessageRequest(1, "좋은 날씨야")
+ val updatedMember = Member(
+ id = 1L,
+ name = "테스트",
+ nickname = "테스트 닉네임",
+ email = "[email protected]",
+ profileImageUrl = null,
+ provider = Provider.KAKAO,
+ role = Role.MEMBER,
+ emotion = 1,
+ statusMessage = "좋은 날씨야"
+ )
+ `when`(memberService.updateMyEmotionAndStatusMessage(1L, updateMyEmotionAndStatusMessageRequest)).thenReturn(updatedMember)
//when
val result = memberInfoFacade.updateMyEmotionAndStatusMessage(customUserDetails, updateMyEmotionAndStatusMessageRequest)
//then
verify(memberService).updateMyEmotionAndStatusMessage(1L, updateMyEmotionAndStatusMessageRequest)
+ assertEquals(updatedMember, result)
}
| data class UpdateMyEmotionAndStatusMessageRequest( | ||
| @Schema(description = "이모지(1 : 보고싶어요, 2: 기분 좋아요, 3 : 아무느낌 없어요, " + | ||
| "4 : 피곤해요, 5: 서운해요, 6 : 걱정돼요, 7 : 짜증나요)", example = "1") | ||
| @field:NotNull(message = "sarangggun.member.invalid-emotion") |
There was a problem hiding this comment.
dto 검증을 백엔드에서 해야할까?
검증도 톰캣 스레드를 사용하는건데, 요청이 많아지는 경우 스레드가 밀릴 수 있잖아
그래서 dto 검증은 클라이언트에서 하고 넘어오는게 더 낫지 않나라는 생각인데 어떻게 생각해?
| import org.springframework.web.bind.annotation.RestControllerAdvice | ||
|
|
||
| @RestControllerAdvice | ||
| class GlobalExceptionHandler { |
… 클라이언트에서 요청 데이터 검증하기로 결정) #22
kimyeoungrok
force-pushed
the
issue/#22-MA_02
branch
from
b41a168 to
94cbd0e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/kotlin/gomushin/backend/core/oauth/handler/CustomSuccessHandler.kt (1)
44-44:❓ Verification inconclusive
로컬 개발 환경용 리다이렉트 URL을 확인하세요.
개발 환경에서는 적절하지만, 프로덕션 환경에서는 하드코딩된 localhost URL을 사용하지 않도록 주의하세요. 환경별 설정을 통해 리다이렉트 URL을 관리하는 것이 더 좋은 방법입니다.
환경 변수나 프로퍼티 파일을 통해 리다이렉트 URL을 구성하는 것이 좋습니다:
- response.sendRedirect("http://localhost:5173") + val redirectUrl = environment.getProperty("oauth.redirect.url") ?: "http://localhost:5173" + response.sendRedirect(redirectUrl)
🏁 Script executed:
#!/bin/bash # 설정 파일에서 환경별 URL 설정이 있는지 확인 fd --type f "application.*properties|application.*yml" --exec grep -l "redirect|frontend|url" {} \;Length of output: 104
추가 검증을 위해 설정 파일과 코드에서 URL 키워드가 정의되어 있는지 확인해 주세요:
🏁 Script executed:
#!/bin/bash # 1) application*.properties, application*.yml 파일에서 URL/redirect 키워드 검색 fd -e properties -e yml | xargs grep -H "url\|redirect" # 2) Kotlin 코드에서 environment.getProperty 사용 검색 rg -n "environment\.getProperty" --glob "*.kt"Length of output: 106
환경별 리다이렉트 URL 구성 확인 필요
현재 애플리케이션 설정 파일(application.properties/yml)에서 리다이렉트 URL 프로퍼티를 찾을 수 없으며, 코드에서도
environment.getProperty사용이 확인되지 않습니다. 프로덕션 환경에서 하드코딩된 localhost URL이 사용되지 않도록 다음 내용을 검토해주세요:
- src/main/kotlin/gomushin/backend/core/oauth/handler/CustomSuccessHandler.kt에서
을 제거response.sendRedirect("http://localhost:5173")- application.properties 또는 application.yml에
형태로 프로퍼티 추가oauth.redirect.url=https://your-production-domain.com- 코드에서 환경변수를 통해 해당 URL을 읽어오도록 변경
- response.sendRedirect("http://localhost:5173") + val redirectUrl = environment.getProperty("oauth.redirect.url") + ?: throw IllegalStateException("oauth.redirect.url 설정이 필요합니다") + response.sendRedirect(redirectUrl)설정 파일 및 프로퍼티가 제대로 반영되는지 확인하고, 로컬/스테이징/프로덕션 환경에 맞게 각각 다른 URL을 지정해 주세요.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/kotlin/gomushin/backend/core/oauth/handler/CustomSuccessHandler.kt(1 hunks)src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt(2 hunks)src/main/kotlin/gomushin/backend/member/domain/service/MemberService.kt(2 hunks)src/main/kotlin/gomushin/backend/member/dto/request/UpdateMyEmotionAndStatusMessageRequest.kt(1 hunks)src/main/kotlin/gomushin/backend/member/dto/response/MyStatusMessageResponse.kt(1 hunks)src/main/kotlin/gomushin/backend/member/facade/MemberInfoFacade.kt(2 hunks)src/main/kotlin/gomushin/backend/member/presentation/ApiPath.kt(1 hunks)src/main/kotlin/gomushin/backend/member/presentation/MemberInfoController.kt(2 hunks)src/test/kotlin/gomushin/backend/member/domain/service/MemberServiceTest.kt(2 hunks)src/test/kotlin/gomushin/backend/member/facade/MemberInfoFacadeTest.kt(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt
🚧 Files skipped from review as they are similar to previous changes (8)
- src/main/kotlin/gomushin/backend/member/presentation/ApiPath.kt
- src/main/kotlin/gomushin/backend/member/domain/service/MemberService.kt
- src/main/kotlin/gomushin/backend/member/facade/MemberInfoFacade.kt
- src/main/kotlin/gomushin/backend/member/dto/response/MyStatusMessageResponse.kt
- src/test/kotlin/gomushin/backend/member/domain/service/MemberServiceTest.kt
- src/test/kotlin/gomushin/backend/member/facade/MemberInfoFacadeTest.kt
- src/main/kotlin/gomushin/backend/member/presentation/MemberInfoController.kt
- src/main/kotlin/gomushin/backend/member/dto/request/UpdateMyEmotionAndStatusMessageRequest.kt
| } | ||
|
|
||
| private fun createCookie(key: String, value: String): Cookie { | ||
| val cookie = Cookie(key, value) | ||
| cookie.path = "/" | ||
| cookie.isHttpOnly = true | ||
| cookie.secure = true | ||
| cookie.secure = false //Todo : 밋업 할 때는 true로 변경하기 |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
보안 취약점: 쿠키 secure 플래그가 비활성화되었습니다.
개발 환경에서는 적절할 수 있지만, 쿠키의 secure 플래그를 false로 설정하면 HTTP를 통해 민감한 쿠키가 전송될 수 있으므로 보안 위험이 있습니다. 주석에 작성된 대로 배포 시에는 반드시 true로 변경해야 합니다.
다음과 같이 환경에 따라 자동으로 설정되도록 변경할 것을 권장합니다:
- cookie.secure = false //Todo : 밋업 할 때는 true로 변경하기
+ cookie.secure = environment.activeProfiles.contains("prod") || environment.activeProfiles.contains("stage")🏁 Script executed:
#!/bin/bash
# 환경 설정 파일을 확인하여 개발/프로덕션 환경 분리가 되어있는지 확인
fd --type f "application-prod.*properties|application-prod.*yml" --exec grep -l "cookie|secure" {} \;Length of output: 106
쿠키 secure 플래그 설정 자동화 필요
현재 CustomSuccessHandler.kt의 cookie.secure = false가 하드코딩되어 있어 배포 시에도 HTTP로 민감한 쿠키가 전송될 수 있는 보안 취약점이 존재합니다. 운영 환경에서는 반드시 secure = true로 설정되어야 하므로, 환경에 따라 자동으로 설정하도록 아래와 같이 수정하세요.
- 환경 설정(application.yml 또는 application.properties)에 쿠키 보안 플래그용 프로퍼티 추가
예시 (application.yml):security: cookie: secure: ${COOKIE_SECURE:false}
- 코드에서 하드코딩된 값을 프로퍼티로 주입받도록 변경
// 핸들러 클래스 상단에 import 및 프로퍼티 주입 추가 @Value("\${security.cookie.secure}") private lateinit var cookieSecure: Boolean // 기존 코드 수정 // cookie.secure = false // Todo: 밋업 할 때는 true로 변경하기 cookie.secure = cookieSecure
- 운영(staging/production) 환경에서는 CI/CD 혹은 환경변수로
COOKIE_SECURE=true를 설정하여 자동 반영되도록 구성
위 작업을 통해 개발 환경에서는 기본값(false)을, 운영 환경에서는 true를 자동으로 적용하여 보안 취약점을 제거할 수 있습니다.
✅ PR 유형
어떤 변경 사항이 있었나요?
📝 작업 내용
#22
Summary by CodeRabbit
신규 기능
버그 수정
http://localhost:5173으로 변경되었습니다.테스트