[Fix] Bizgo 설정 변경 및 ContactType Validation 변경. - #400
Conversation
- 인터뷰 질문, 미션, 데이팅 시험 관련 기본 데이터 마이그레이션 추가 - Flyway 설정 로직에 비활성화 처리 조건 추가 - Hibernate DDL 설정 방식 변경 및 스키마 생성 스크립트 설정 추가 #377
# Conflicts: # src/main/java/deepple/deepple/common/config/FlywayConfig.java
- k6 테스트 설정 파일 및 API 테스트 스크립트 추가 - 테스트 환경 변수 및 부하 프로파일 설정 - README에 k6 관련 문서 및 실행 방법 추가 #394
- 성능 테스트용 SQL 데이터 생성 스크립트 추가 - 회원, 소개, 좋아요, 매칭, 알림 데이터 포함 - 데이터 검증 및 README 파일 추가 #394
|
Warning Rate limit exceeded@Kong-TaeHyeon has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 14 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Walkthrough이 PR은 Bizgo SMS 전송을 token 기반에서 API key 기반으로 전환하고 관련 토큰 관리 코드를 삭제합니다. Match 도메인의 이벤트(MatchRequestedEvent, MatchRespondedEvent)에 contactType 필드를 추가하여 이벤트 페이로드가 확장됩니다. MemberProfileService의 검증 메서드가 contactType 매개변수를 받도록 바뀌고, 관련 예외 클래스명이 ContactTypeSettingNeededException로 변경되었습니다. BizgoMessageRequest DTO 구조가 리스트 기반 복합 구조로 변경되었고, k6 부하 스크립트와 Gradle의 Jakarta dependency 버전 고정이 포함됩니다. Sequence Diagram(s)sequenceDiagram
participant App as Application (BizgoMessanger)
participant Token as BizgoTokenHandler
participant Redis as RedisCache
participant Lock as RedissonLock
participant Bizgo as Bizgo API
App->>Token: request auth token
Token->>Redis: get token
alt token present
Redis-->>Token: token
else token absent
Token->>Lock: acquire lock
Lock-->>Token: lock acquired
Token->>Redis: re-check token
alt still absent
Token->>Bizgo: POST /auth/token (client creds)
Bizgo-->>Token: auth response (token)
Token->>Redis: set token w/ expiry
end
Token->>Lock: release lock
end
Token-->>App: return token
App->>Bizgo: POST /send/sms (Authorization: Bearer token)
Bizgo-->>App: 200/4xx
sequenceDiagram
participant App as Application (BizgoMessanger)
participant Bizgo as Bizgo API
participant Config as Config (bizgo.api-key)
App->>Config: read bizgo.api-key
App->>Bizgo: POST /api/comm/v1/send/omni (Authorization: apiKey)
Bizgo-->>App: 200/4xx
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Test results756 tests 756 ✅ 15s ⏱️ Results for commit 5e44961. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/resources/application-prod.yml (1)
94-100: Production 환경에서 민감한 credential의 default 값 제거 필요.Production 환경에서
BIZGO_API_KEY에 default 값api-key를 설정하는 것은 보안 위험이 있습니다. Environment variable이 설정되지 않았을 때 placeholder 값으로 동작하게 되면 의도하지 않은 API 호출이나 보안 취약점이 발생할 수 있습니다.Production 환경에서는 필수 credential이 누락되었을 때 애플리케이션이 시작되지 않도록 default 값을 제거하는 것을 권장합니다.
🔒 Production 환경 보안 강화 제안
bizgo: api-url: ${BIZGO_API_URL:api-url} - api-key: ${BIZGO_API_KEY:api-key} + api-key: ${BIZGO_API_KEY} client-id: ${BIZGO_CLIENT_ID:client-id} client-password: ${BIZGO_CLIENT_PASSWORD:client_password} from-phone-number: ${BIZGO_FROM_PHONE_NUMBER:from-phone-number}다른 민감한 credential들(
client-password등)도 동일하게 default 값 제거를 고려해보세요.src/main/java/deepple/deepple/member/command/infra/member/sms/BizgoMessanger.java (1)
33-47: Authorization header에 "ApiKey" prefix가 필요합니다.Line 40의 Authorization header가 raw apiKey를 직접 전송하고 있습니다. Bizgo OMNI API 문서에 따르면, Authorization header는 다음 형식 중 하나를 요구합니다:
Authorization: Bearer <JWT_access_token>, 또는Authorization: ApiKey <api_key>현재 코드는 prefix 없이 raw apiKey를 전송하므로, 모든 요청이 401/403 error로 실패할 것입니다. 다음과 같이 수정해야 합니다:
.header("Authorization", "ApiKey " + apiKey)또한 coding guidelines에 따르면, 외부 API 호출을 domain interface를 통해 abstract해야 합니다. BizgoMessanger를 직접 @service로 구현하는 대신, domain layer에서 정의한 interface를 infra layer에서 구현하도록 리팩토링이 필요합니다.
🤖 Fix all issues with AI agents
In
@src/main/java/deepple/deepple/member/command/application/member/MemberProfileService.java:
- Around line 22-23: Replace the string constants PHONE_NUMBER and KAKAO in
MemberProfileService with the domain enum ContactType: remove the private static
String constants and update all usages inside MemberProfileService to use
ContactType.PHONE_NUMBER and ContactType.KAKAO (or the appropriate enum values)
instead of raw strings; update method parameters, local variables, and any
conversions so signatures and logic accept/use ContactType (or convert to/from
ContactType at the boundary, e.g., persistence/DTO layer) and add the necessary
import for the ContactType enum, ensuring type-safe handling throughout the
service.
- Around line 50-57: validateContactTypeSetting currently accepts a nullable
String and skips validation for null or unexpected values; change its signature
to accept a ContactType enum (or validate/convert the String to ContactType
up-front), explicitly handle null by either throwing
ContactTypeSettingNeededException or returning early with a clear comment, and
add a default branch that throws ContactTypeSettingNeededException for any
unexpected ContactType; ensure you still fetch the Member via
getMemberById(memberId) and check Member.getPhoneNumber() for
ContactType.PHONE_NUMBER and Member.getKakaoId() for ContactType.KAKAO, throwing
ContactTypeSettingNeededException when the required field is missing.
🧹 Nitpick comments (10)
k6/scripts/member-profile.js (1)
6-7: Environment variable 기반 설정 고려.기존에는
TARGET_RPS등 environment variable로 설정값을 주입받았으나, 현재 hardcoded 상수로 변경되었습니다.lib/config.js에서__ENV.BASE_URL을 사용하는 패턴과 일관성을 위해, 테스트 환경별 유연성이 필요하다면 environment variable fallback 패턴을 고려해 보세요.const MAX_VUS = parseInt(__ENV.MAX_VUS) || 1000; const MEMBER_COUNT = parseInt(__ENV.MEMBER_COUNT) || 1000000;build.gradle (1)
49-50: Explicit annotation processor dependencies는 Spring Boot BOM과 중복됩니다.Spring Boot 3.3.5의 dependency management는 jakarta.annotation-api:2.1.1과 jakarta.persistence-api:3.1.0을 이미 관리하고 있습니다. 빌드 파일에서 명시적으로 선언한 버전이 BOM의 관리되는 버전과 정확히 일치하므로 버전 충돌 문제는 없습니다.
그러나 이 explicit annotation processor dependencies는 redundant합니다. 다음 두 가지 방식 중 하나를 선택하시기 바랍니다:
- 권장: 명시적 버전 선언을 제거하고 Spring Boot BOM의 dependency management에 의존합니다. QueryDSL annotation processing은 BOM의 관리되는 Jakarta API 버전으로 정상 작동합니다.
- 대안: 명시적 버전을 유지하되, 주석을 추가하여 의도적 명시(explicit clarity)임을 기록합니다.
명시적 선언이 필요한 특별한 이유가 없다면 제거를 권장합니다.
src/main/java/deepple/deepple/match/command/domain/match/event/MatchRespondedEvent.java (1)
15-20: ContactType을 String 대신 enum으로 타입 정의를 고려하세요.현재
contactType필드가String타입으로 정의되어 있습니다. Domain-driven design 원칙에 따르면 value object를 사용하는 것이 권장되며, 특히 제한된 값 집합을 가진 경우 enum 타입을 사용하면 type safety와 컴파일 타임 검증이 가능합니다.PR 설명과 코딩 가이드라인에서 언급된 바와 같이 value object를 domain modeling에 적극 활용해야 합니다.
♻️ Enum 타입 사용 제안
ContactTypeenum이 이미 존재한다면, factory method 시그니처를 다음과 같이 변경하는 것을 고려하세요:- private final String contactType; + private final ContactType contactType; - public static MatchRespondedEvent of(Long requesterId, Long responderId, MatchStatus matchStatus, - String contactType) { - return new MatchRespondedEvent(requesterId, responderId, matchStatus.toString(), contactType); + public static MatchRespondedEvent of(Long requesterId, Long responderId, MatchStatus matchStatus, + ContactType contactType) { + return new MatchRespondedEvent(requesterId, responderId, matchStatus.toString(), contactType.name());만약 event가 serialization을 위해 String이 필요하다면, 현재 구조를 유지하되 factory method parameter만 enum으로 받고 내부에서
.name()으로 변환하는 것이 type safety 측면에서 유리합니다.src/main/java/deepple/deepple/match/command/domain/match/event/MatchRequestedEvent.java (1)
16-20: ContactType을 String 대신 enum으로 타입 정의를 고려하세요.
MatchRespondedEvent와 동일하게,contactType필드가String타입으로 정의되어 있습니다. Type safety와 domain modeling 관점에서ContactTypeenum을 사용하는 것이 권장됩니다.Domain event는 도메인 개념을 명확하게 표현해야 하며, 제한된 값 집합을 가진 속성은 enum으로 표현하는 것이 SOLID 원칙과 DDD 원칙에 부합합니다.
♻️ Enum 타입 사용 제안
ContactTypeenum이 이미 존재한다면, factory method 시그니처를 다음과 같이 변경하는 것을 고려하세요:- private final String contactType; + private final ContactType contactType; public static MatchRequestedEvent of(long requesterId, @NonNull String requesterName, long responderId, - String matchType, String contactType) { - return new MatchRequestedEvent(requesterId, requesterName, responderId, matchType, contactType); + String matchType, ContactType contactType) { + return new MatchRequestedEvent(requesterId, requesterName, responderId, matchType, contactType.name());만약 event serialization을 위해 String이 필요하다면, factory method parameter만 enum으로 받고 내부에서
.name()으로 변환하세요.src/test/java/deepple/deepple/member/command/application/member/MemberProfileServiceTest.java (2)
222-235:assertThatNoException()사용 개선 필요Line 234의
assertThatNoException()는 단독으로 사용될 때 실제로 아무것도 검증하지 않습니다. 메서드 호출이 예외를 던지지 않는지 확인하려면assertThatCode()를 사용하세요.♻️ 권장 수정안
// When - memberProfileService.validateContactTypeSetting(memberId, contactType); - - // Then - assertThatNoException(); + // When & Then + assertThatCode(() -> memberProfileService.validateContactTypeSetting(memberId, contactType)) + .doesNotThrowAnyException();
238-254: String literal 대신 상수 사용 권장Test에서
"PHONE_NUMBER"와"KAKAO"string literal을 직접 사용하고 있습니다. 이는 오타 위험이 있으며, production code에 정의된 상수(예:MatchContactTypeenum)와 일치하는지 검증하기 어렵습니다.♻️ 권장 수정안
MatchContactTypeenum이 있다면 해당 enum 값을 사용하세요:@Test @DisplayName("ContactType 이 PHONE_NUMBER or KAKAO 일 경우, 유효성을 검증한다.") void shouldValidateWhenContactTypeIsInKakaoOrPhonNumber() { // Given final Long memberId = 1L; - final List<String> contactTypes = List.of("PHONE_NUMBER","KAKAO"); + final List<String> contactTypes = List.of( + MatchContactType.PHONE_NUMBER.name(), + MatchContactType.KAKAO.name() + ); Member member = Mockito.mock(Member.class); when(memberCommandRepository.findById(memberId)).thenReturn(Optional.of(member)); when(member.getPhoneNumber()).thenReturn(null); when(member.getKakaoId()).thenReturn(null); // When & Then for (String contactType : contactTypes) { assertThatThrownBy(() -> memberProfileService.validateContactTypeSetting(memberId, contactType)) .isInstanceOf(ContactTypeSettingNeededException.class); } }또는
@ParameterizedTest를 사용하여 더 명확하게 작성할 수 있습니다.src/main/java/deepple/deepple/member/presentation/member/MemberExceptionHandler.java (1)
80-87: Method 이름과 exception type 불일치Method 이름이
handlePrimaryContactTypeSettingNeededException이지만, 실제로 처리하는 exception type은ContactTypeSettingNeededException입니다. 일관성을 위해 method 이름을 변경하는 것이 좋습니다.♻️ 권장 수정안
@ExceptionHandler(ContactTypeSettingNeededException.class) - public ResponseEntity<BaseResponse<Void>> handlePrimaryContactTypeSettingNeededException( + public ResponseEntity<BaseResponse<Void>> handleContactTypeSettingNeededException( ContactTypeSettingNeededException e) { log.warn("매치 요청/응답에 실패하였습니다. {}", e.getMessage()); return ResponseEntity.badRequest() .body(BaseResponse.of(StatusType.BAD_REQUEST, e.getMessage())); }src/main/java/deepple/deepple/match/command/domain/match/Match.java (1)
82-93: Null contactType 처리 확인
reject()와expire()method에서contactType에null을 전달하는 것은 의미상 적절합니다. 이 경우 연락처 교환이 발생하지 않으므로 contactType이 필요하지 않습니다.다만,
MatchRespondedEvent에 overloaded factory method를 제공하거나, Optional을 사용하면 의도를 더 명확하게 표현할 수 있습니다.💡 Optional 개선안 (Event class 수정)
MatchRespondedEventclass에 contactType이 필요 없는 경우를 위한 overloaded method를 추가할 수 있습니다:// MatchRespondedEvent.java public static MatchRespondedEvent of(Long requesterId, Long responderId, MatchStatus matchStatus) { return of(requesterId, responderId, matchStatus, null); }이렇게 하면 호출 측에서 null을 명시적으로 전달하지 않아도 됩니다:
public void reject(String responderName) { validateChangeStatus(); status = MatchStatus.REJECTED; - Events.raise(MatchRespondedEvent.of(requesterId, responderId, status, null)); + Events.raise(MatchRespondedEvent.of(requesterId, responderId, status)); Events.raise(MatchRejectedEvent.of(requesterId, responderId, responderName)); } public void expire() { validateChangeStatus(); status = MatchStatus.EXPIRED; - Events.raise(MatchRespondedEvent.of(requesterId, responderId, status, null)); + Events.raise(MatchRespondedEvent.of(requesterId, responderId, status)); }src/main/java/deepple/deepple/member/command/infra/member/sms/BizgoMessanger.java (2)
23-24: API key 설정 필드 추가 확인됨.Configuration에서
bizgo.api-key를 주입받는 필드가 추가되었습니다.startup 시점에 API key 유효성 검증을 추가하면 runtime error를 조기에 발견할 수 있습니다 (예:
@PostConstruct로 null/empty 체크).
26-31: Timeout/Fallback retry 로직 구현 필요.Token 기반 인증을 제거하면서 retry 로직도 함께 제거되었습니다. TODO comment에서 언급한 대로 timeout error에 대한 fallback 및 재시도 로직이 필요합니다.
현재는 일시적인 네트워크 오류나 timeout 발생 시 즉시 실패하게 되어 사용자 경험에 영향을 줄 수 있습니다.
Resilience4j 또는 Spring Retry를 사용한 재시도 로직 구현이 필요하신가요? 구현 예시를 생성해드릴 수 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
build.gradlek6/scripts/member-profile.jssrc/main/java/deepple/deepple/match/command/domain/match/Match.javasrc/main/java/deepple/deepple/match/command/domain/match/event/MatchRequestedEvent.javasrc/main/java/deepple/deepple/match/command/domain/match/event/MatchRespondedEvent.javasrc/main/java/deepple/deepple/member/command/application/member/MemberProfileService.javasrc/main/java/deepple/deepple/member/command/application/member/exception/ContactTypeSettingNeededException.javasrc/main/java/deepple/deepple/member/command/infra/member/MemberProfileEventHandler.javasrc/main/java/deepple/deepple/member/command/infra/member/sms/BizgoMessanger.javasrc/main/java/deepple/deepple/member/command/infra/member/sms/BizgoTokenHandler.javasrc/main/java/deepple/deepple/member/presentation/member/MemberExceptionHandler.javasrc/main/resources/application-dev.ymlsrc/main/resources/application-local.ymlsrc/main/resources/application-prod.ymlsrc/test/java/deepple/deepple/member/command/application/member/MemberProfileServiceTest.javasrc/test/resources/application.yml
💤 Files with no reviewable changes (1)
- src/main/java/deepple/deepple/member/command/infra/member/sms/BizgoTokenHandler.java
🧰 Additional context used
📓 Path-based instructions (3)
**/{application,application-*.{properties,yml,yaml}}
📄 CodeRabbit inference engine (CLAUDE.md)
Configure Spring Boot application properties including port (default 8080), Swagger UI at
/swagger-ui.htmlwhen enabled, and MySQL, Redis, AWS S3, Firebase, and App Store Server API integrations
Files:
src/main/resources/application-prod.ymlsrc/main/resources/application-dev.ymlsrc/main/resources/application-local.yml
**/*.java
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.java: Use JPA with QueryDSL for persistence, MySQL with Flyway for migrations, JWT tokens and Spring Security for authentication, and Redis with Redisson for caching in Java Spring Boot files
Implement domain events using the local event system (Events.java) for domain event handling, with events published and handled within the same process by event handlers in the infrastructure layer
Use Lombok to reduce boilerplate in Java domain entities
Use value objects heavily for domain modeling in Java domain layer
Abstract all external API calls through domain interfaces in Java code
Project uses Java 21 language features in all Java code
Files:
src/main/java/deepple/deepple/member/command/application/member/exception/ContactTypeSettingNeededException.javasrc/main/java/deepple/deepple/member/command/infra/member/MemberProfileEventHandler.javasrc/main/java/deepple/deepple/member/presentation/member/MemberExceptionHandler.javasrc/main/java/deepple/deepple/member/command/infra/member/sms/BizgoMessanger.javasrc/main/java/deepple/deepple/member/command/application/member/MemberProfileService.javasrc/main/java/deepple/deepple/match/command/domain/match/Match.javasrc/test/java/deepple/deepple/member/command/application/member/MemberProfileServiceTest.javasrc/main/java/deepple/deepple/match/command/domain/match/event/MatchRespondedEvent.javasrc/main/java/deepple/deepple/match/command/domain/match/event/MatchRequestedEvent.java
⚙️ CodeRabbit configuration file
**/*.java: In addition to existing rules, review code based on: - Domain-driven design: enforce aggregate boundaries and immutable value objects - Clean code: single responsibility methods, meaningful names, no duplication - Object-oriented programming: SOLID(SRP, OCP, LSP, ISP, DIP) principles - Performance and scalability: consider high traffic scenarios, database query optimization, proper indexing, caching strategies, connection pool management, and async processing where appropriate
Files:
src/main/java/deepple/deepple/member/command/application/member/exception/ContactTypeSettingNeededException.javasrc/main/java/deepple/deepple/member/command/infra/member/MemberProfileEventHandler.javasrc/main/java/deepple/deepple/member/presentation/member/MemberExceptionHandler.javasrc/main/java/deepple/deepple/member/command/infra/member/sms/BizgoMessanger.javasrc/main/java/deepple/deepple/member/command/application/member/MemberProfileService.javasrc/main/java/deepple/deepple/match/command/domain/match/Match.javasrc/test/java/deepple/deepple/member/command/application/member/MemberProfileServiceTest.javasrc/main/java/deepple/deepple/match/command/domain/match/event/MatchRespondedEvent.javasrc/main/java/deepple/deepple/match/command/domain/match/event/MatchRequestedEvent.java
**/*Test.java
📄 CodeRabbit inference engine (CLAUDE.md)
Write unit tests for domain objects and value objects, integration tests for repositories using
@DataJpaTest, and service tests using mocks and test slices, following the same package structure as main code
Files:
src/test/java/deepple/deepple/member/command/application/member/MemberProfileServiceTest.java
🧠 Learnings (2)
📚 Learning: 2025-12-11T08:55:55.277Z
Learnt from: CR
Repo: deepple-dev/deepple-api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-11T08:55:55.277Z
Learning: Applies to **/*Test.java : Write unit tests for domain objects and value objects, integration tests for repositories using `DataJpaTest`, and service tests using mocks and test slices, following the same package structure as main code
Applied to files:
src/test/java/deepple/deepple/member/command/application/member/MemberProfileServiceTest.java
📚 Learning: 2025-12-11T08:55:55.277Z
Learnt from: CR
Repo: deepple-dev/deepple-api PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-11T08:55:55.277Z
Learning: Applies to **/*.java : Use JPA with QueryDSL for persistence, MySQL with Flyway for migrations, JWT tokens and Spring Security for authentication, and Redis with Redisson for caching in Java Spring Boot files
Applied to files:
build.gradle
🧬 Code graph analysis (3)
k6/scripts/member-profile.js (2)
k6/scripts/lib/auth.js (1)
authHeaders(25-30)k6/scripts/lib/config.js (2)
config(1-3)config(1-3)
src/main/java/deepple/deepple/member/command/application/member/MemberProfileService.java (1)
src/main/java/deepple/deepple/member/command/application/member/exception/ContactTypeSettingNeededException.java (1)
ContactTypeSettingNeededException(3-7)
src/test/java/deepple/deepple/member/command/application/member/MemberProfileServiceTest.java (1)
src/main/java/deepple/deepple/member/command/application/member/exception/ContactTypeSettingNeededException.java (1)
ContactTypeSettingNeededException(3-7)
⏰ 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: test
🔇 Additional comments (14)
k6/scripts/member-profile.js (5)
18-24: LGTM!단계적 부하 증가 패턴이 적절합니다.
ramping-arrival-rateexecutor에서 target은 RPS를 의미하며, MAX_VUS는 충분히 확보되어 있습니다.
33-39: Cross-gender lookup 로직 정상 동작 확인.Member ID parity 기반 성별 구분 및 modular arithmetic을 활용한 target 계산 로직이 올바릅니다. 주석도 명확합니다.
주의사항: 이 로직은 test data가 정확히 1~1,000,000 범위의 member ID를 가지며, 홀수=남성/짝수=여성 규칙을 따른다고 가정합니다. Test data 생성 스크립트와 일치하는지 확인이 필요합니다.
45-47: LGTM!Sequential member ID 할당 로직이 명확합니다. 1~1000 범위에서 500명의 남성(홀수)과 500명의 여성(짝수)이 테스트에 참여합니다.
62-64: LGTM!새로운
getIntroducedMemberId함수 호출이 올바르게 적용되었습니다.
71-73: LGTM!Check label 변경은 minor cosmetic change입니다.
src/main/java/deepple/deepple/member/command/infra/member/MemberProfileEventHandler.java (1)
18-18: 변경사항이 올바르게 적용되었습니다.Event에서
contactType을 추출하여 validation service로 전달하는 로직이 정확합니다. Event-driven architecture 패턴을 올바르게 따르고 있습니다.Also applies to: 23-23
src/main/java/deepple/deepple/member/command/application/member/exception/ContactTypeSettingNeededException.java (1)
3-6: Exception 클래스 이름 변경이 적절합니다.기존
PrimaryContactTypeSettingNeededException에서ContactTypeSettingNeededException으로의 변경은 새로운 validation 로직의 의미를 더 잘 반영합니다.src/test/java/deepple/deepple/member/command/application/member/MemberProfileServiceTest.java (2)
4-4: Import 변경 사항 확인새로운 test case를 위한 import 변경이 적절합니다.
Also applies to: 15-15, 19-19, 24-24
209-219: MemberNotFoundException test 확인Member가 존재하지 않을 때 예외가 발생하는 것을 올바르게 검증합니다.
src/main/java/deepple/deepple/match/command/domain/match/Match.java (2)
57-71: MatchRequestedEvent에 contactType 추가 확인매칭 요청 시
contactType.name()을 event에 포함하는 변경이 적절합니다. 이는 event handler에서 해당 contact type을 검증할 수 있도록 합니다.
73-80: MatchRespondedEvent에 contactType 추가 확인매칭 수락 시
contactType.name()을 event에 포함하는 변경이 적절합니다.src/test/resources/application.yml (1)
70-76: 테스트 설정 변경 확인됨.test resource에 hardcoded placeholder 값으로
api-key를 추가한 것은 적절합니다.src/main/resources/application-local.yml (1)
86-92: Local 환경 설정 변경 확인됨.environment variable
BIZGO_API_KEY를 사용하며 default 값을 제공하는 것은 local 개발 환경에 적절합니다.src/main/resources/application-dev.yml (1)
90-96: Dev 환경 설정 변경 확인됨.Development 환경에
api-key설정이 추가되었으며, default 값을 제공하는 것은 개발 편의성을 위해 적절합니다.
* [Refac] : When Match Request or Response, Don't Check Primary Contatct Type * [fix] : Change BaseURL * [Fix] : Using Enum
* [Refac] : When Match Request or Response, Don't Check Primary Contatct Type * [fix] : Change BaseURL * [Fix] : Using Enum * [Refac] Change BizgoRequestBody
44f64b3 to
7ea2a8d
Compare
* [Refac] : When Match Request or Response, Don't Check Primary Contatct Type * [fix] : Change BaseURL * [Fix] : Using Enum * [Refac] Change BizgoRequestBody * [Fix] : Use Record for serialization
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/deepple/deepple/member/command/infra/member/sms/BizgoMessanger.java (1)
36-46: RestClient 응답이 consume되지 않아 요청이 완료되지 않을 수 있습니다.
.retrieve()및.onStatus()이후 terminal operation (.toBodilessEntity()또는.body())이 누락되었습니다. 현재 상태에서는 HTTP 요청이 실제로 완료되지 않거나 resource leak이 발생할 수 있습니다.🐛 수정 제안
restClient.post() .uri(requestURL) .header("Content-Type", "application/json") .header("Accept", "application/json") .header("Authorization", apiKey) .body(new BizgoMessageRequest(message, fromPhoneNumber, phoneNumber)) .retrieve() .onStatus(HttpStatusCode::isError, (request, httpResponse) -> { throw new BizgoMessageSendException(httpResponse.getStatusCode().value()); } - ); + ) + .toBodilessEntity(); }
🧹 Nitpick comments (2)
src/main/java/deepple/deepple/member/command/application/member/MemberProfileService.java (1)
48-54: 타입 안정성 개선을 위해 enum 파라미터 사용을 고려하세요.현재 구현은
String타입의contactType파라미터를 받아PrimaryContactTypeenum의 이름과 비교합니다. 이 방식은 컴파일 타임에 잘못된 값이 전달되는 것을 방지할 수 없으며, 유효하지 않은contactType값이 전달될 경우 validation을 조용히 통과시킵니다.Learning에 따르면 null
contactType은 의도적으로 허용되지만, 현재 구현에서는 "INVALID_VALUE"와 같은 잘못된 문자열도 예외 없이 통과됩니다.파라미터 타입을
PrimaryContactType으로 변경하면:
- Compile-time type safety 확보
- Invalid value 전달 방지
- 코드 의도가 명확해짐
♻️ 제안하는 리팩토링
-public void validateContactTypeSetting(Long memberId, String contactType) { +public void validateContactTypeSetting(Long memberId, PrimaryContactType contactType) { Member member = getMemberById(memberId); - if (PrimaryContactType.PHONE_NUMBER.name().equals(contactType) && member.getPhoneNumber() == null) { + if (contactType == PrimaryContactType.PHONE_NUMBER && member.getPhoneNumber() == null) { throw new ContactTypeSettingNeededException(); - } else if (PrimaryContactType.KAKAO.name().equals(contactType) && member.getKakaoId() == null) { + } else if (contactType == PrimaryContactType.KAKAO && member.getKakaoId() == null) { throw new ContactTypeSettingNeededException(); } }Note: Caller 쪽에서 String을 enum으로 변환하는 로직 추가 필요 (예:
PrimaryContactType.valueOf(contactTypeString))src/main/java/deepple/deepple/member/command/infra/member/sms/BizgoMessanger.java (1)
26-31: TODO: Timeout 재시도 로직 구현 필요.Timeout 에러에 대한 fallback 재시도 로직이 TODO로 남아있습니다. Resilience4j 또는 Spring Retry를 사용하여 exponential backoff와 함께 retry 로직을 구현하는 것을 권장합니다.
재시도 로직 구현 코드를 생성하거나 이 작업을 추적할 issue를 생성해 드릴까요?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/deepple/deepple/member/command/application/member/MemberProfileService.javasrc/main/java/deepple/deepple/member/command/infra/member/sms/BizgoMessanger.javasrc/main/java/deepple/deepple/member/command/infra/member/sms/dto/BizgoMessageRequest.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.java: Use JPA with QueryDSL for persistence, MySQL with Flyway for migrations, JWT tokens and Spring Security for authentication, and Redis with Redisson for caching in Java Spring Boot files
Implement domain events using the local event system (Events.java) for domain event handling, with events published and handled within the same process by event handlers in the infrastructure layer
Use Lombok to reduce boilerplate in Java domain entities
Use value objects heavily for domain modeling in Java domain layer
Abstract all external API calls through domain interfaces in Java code
Project uses Java 21 language features in all Java code
Files:
src/main/java/deepple/deepple/member/command/application/member/MemberProfileService.javasrc/main/java/deepple/deepple/member/command/infra/member/sms/BizgoMessanger.javasrc/main/java/deepple/deepple/member/command/infra/member/sms/dto/BizgoMessageRequest.java
⚙️ CodeRabbit configuration file
**/*.java: In addition to existing rules, review code based on: - Domain-driven design: enforce aggregate boundaries and immutable value objects - Clean code: single responsibility methods, meaningful names, no duplication - Object-oriented programming: SOLID(SRP, OCP, LSP, ISP, DIP) principles - Performance and scalability: consider high traffic scenarios, database query optimization, proper indexing, caching strategies, connection pool management, and async processing where appropriate
Files:
src/main/java/deepple/deepple/member/command/application/member/MemberProfileService.javasrc/main/java/deepple/deepple/member/command/infra/member/sms/BizgoMessanger.javasrc/main/java/deepple/deepple/member/command/infra/member/sms/dto/BizgoMessageRequest.java
🧠 Learnings (1)
📚 Learning: 2026-01-11T07:56:33.892Z
Learnt from: Kong-TaeHyeon
Repo: deepple-dev/deepple-api PR: 400
File: src/main/java/deepple/deepple/member/command/application/member/MemberProfileService.java:50-57
Timestamp: 2026-01-11T07:56:33.892Z
Learning: In MemberProfileService.validateContactTypeSetting, passing null for contactType is intentional and used to skip validation in non-match scenarios (e.g., reject, expire). This allows the same method to be reused for both match request/response validation and other flows where contact type validation is not needed.
Applied to files:
src/main/java/deepple/deepple/member/command/application/member/MemberProfileService.java
🧬 Code graph analysis (1)
src/main/java/deepple/deepple/member/command/application/member/MemberProfileService.java (1)
src/main/java/deepple/deepple/member/command/application/member/exception/ContactTypeSettingNeededException.java (1)
ContactTypeSettingNeededException(3-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (3)
src/main/java/deepple/deepple/member/command/infra/member/sms/BizgoMessanger.java (1)
40-40: Authorization header 형식을 API 명세와 대조하여 확인하세요.일반적으로 API key 인증은
Bearer {key},ApiKey {key}또는 별도의 custom header (예:X-API-Key)를 사용합니다. 현재 raw key 값을 직접 사용하고 있는데, Bizgo API 명세에서 요구하는 형식과 일치하는지 확인이 필요합니다.src/main/java/deepple/deepple/member/command/infra/member/sms/dto/BizgoMessageRequest.java (2)
15-21:ref필드가 null로 설정됩니다. API에서 nullable 허용 여부를 확인하세요.Convenience constructor에서
ref를null로 설정하고 있습니다. Bizgo API가 이 필드에 대해 null 또는 누락을 허용하는지 확인이 필요합니다. Jackson 직렬화 시null값이 JSON에 포함되므로, 필요하다면@JsonInclude(JsonInclude.Include.NON_NULL)어노테이션 적용을 고려하세요.
5-13: Record와 nested records를 활용한 구조가 적절합니다.Bizgo API의 복합 요청 구조를 immutable value objects로 잘 모델링했습니다. Nested records 사용으로 도메인 모델이 명확하고 타입 안전성이 확보됩니다.
9ef7594 to
81627ad
Compare
3 participants
Summary by CodeRabbit
신규 기능
개선
테스트
기타
✏️ Tip: You can customize this high-level summary in your review settings.
참고 자료
노트