Conversation
Walkthrough이번 변경에서는 SMS 인증 코드 발송 기능이 승인(Approval) 프로세스에 통합되었습니다. 이를 위해 SMS 발송 서비스 및 DTO가 추가되고, 승인 컨트롤러에서 SMS 발송 성공 시에만 인증 정보가 저장되도록 로직이 수정되었습니다. 또한, Bean Validation이 도입되어 DTO 필드 유효성 검증이 강화되었고, 데이터베이스 매퍼와 VO에도 관련 필드가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ApprovalController
participant SendSmsService
participant DB
User->>ApprovalController: 인증 요청(전화번호 전달)
ApprovalController->>SendSmsService: sendSms(수신번호, 인증코드 포함 메시지)
SendSmsService-->>ApprovalController: 성공/실패 결과 반환
alt SMS 발송 성공
ApprovalController->>DB: 인증 정보(messageContent 포함) 저장
ApprovalController-->>User: 인증코드 발송 성공 응답
else SMS 발송 실패
ApprovalController-->>User: 인증코드 발송 실패 응답
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/main/java/com/barogagi/member/join/dto/UserIdCheckDTO.java (1)
13-16: 컨트롤러 중복 검증 로직 제거 및 Bean Validation 활용DTO에 이미 Bean Validation이 적용되어 있으므로,
JoinController에서 수동 검증(inputValidate.isEmpty)을 제거하고 메서드 파라미터에@Valid와BindingResult를 활용해주세요.• 수정 대상
src/main/java/com/barogagi/member/join/controller/JoinController.java
–if(inputValidate.isEmpty(userIdCheckDTO.getUserId())) { … }블록 제거
– 핸들러 메서드 시그니처에@Valid UserIdCheckDTO userIdCheckDTO및BindingResult bindingResult추가• 예시
@PostMapping("/join") public ResponseEntity<?> join( @Valid @RequestBody UserIdCheckDTO userIdCheckDTO, BindingResult bindingResult ) { if (bindingResult.hasErrors()) { // validation error 처리 String errorMsg = bindingResult.getFieldError().getDefaultMessage(); return ResponseEntity.badRequest().body(errorMsg); } // 이후 로직 }이렇게 변경하면 DTO에 선언된
@NotBlank,@Size,@Pattern검증을 자동으로 적용할 수 있습니다.
🧹 Nitpick comments (4)
pom.xml (1)
92-100: spring-boot-starter-validation 중복 여부와 SDK 버전 관리 확인 권장
spring-boot-starter-web의존성에 이미 Bean Validation이 전이(dependency transitively) 포함되어 있어 추가 선언이 불필요할 수 있습니다.
중복 선언을 제거하면 의존성 트리가 간결해지고 빌드 시간도 줄어듭니다.
net.nurigo:sdk버전 문자열을<properties>블록에 별도 변수로 분리하면 차후 업그레이드 시 한 곳에서 관리할 수 있어 유지보수성이 향상됩니다.필요성 검토 후 정리 부탁드립니다.
src/main/java/com/barogagi/sendSms/dto/SendSmsVO.java (1)
7-12: 필드에 Bean Validation 적용 및 기본값 초기화 재고
recipientTel,messageContent는 비어 있으면 안 되는 값으로 보입니다.
@NotBlank등의 검증 애너테이션을 붙이면 컨트롤러 단에서 빠르게 입력 오류를 감지할 수 있습니다.- 빈 문자열로 기본값을 두면 “값이 존재하지만 의미 없음” 상태가 생깁니다.
null로 두고 Validation으로 필수 여부를 보장하거나, 생성자(Builder 포함)에서 명시적으로 세팅하는 편이 의도 전달에 명확합니다.public class SendSmsVO extends DefaultVO { - private String recipientTel = ""; - private String messageContent = ""; + @NotBlank + private String recipientTel; + + @NotBlank + private String messageContent; }src/main/java/com/barogagi/sendSms/service/SendSmsService.java (1)
47-47: 중복된 할당을 제거하세요.
result변수가 이미 line 35에서true로 초기화되었으므로 중복 할당입니다.messageService.send(message); - result = true;src/main/java/com/barogagi/approval/controller/ApprovalController.java (1)
101-117: SMS 발송 성공 시에만 DB 저장하는 로직이 우수합니다.SMS 발송 실패 시 인증 정보를 DB에 저장하지 않는 로직은 데이터 일관성 측면에서 훌륭한 설계입니다. 각기 다른 결과 코드로 명확한 오류 구분도 잘 되어 있습니다.
더 나은 데이터 일관성을 위해
@Transactional어노테이션 추가를 고려해보세요:+ @Transactional public ApiResponse approvalTelSend(@RequestBody ApprovalSendVO approvalSendVO) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
pom.xml(1 hunks)src/main/java/com/Application.java(1 hunks)src/main/java/com/barogagi/approval/controller/ApprovalController.java(4 hunks)src/main/java/com/barogagi/approval/vo/ApprovalVO.java(1 hunks)src/main/java/com/barogagi/member/join/dto/NickNameDTO.java(1 hunks)src/main/java/com/barogagi/member/join/dto/UserIdCheckDTO.java(1 hunks)src/main/java/com/barogagi/member/login/dto/LoginDTO.java(2 hunks)src/main/java/com/barogagi/sendSms/dto/SendSmsVO.java(1 hunks)src/main/java/com/barogagi/sendSms/service/SendSmsService.java(1 hunks)src/main/resources/mapper/ApprovalMapper.xml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/main/java/com/barogagi/member/login/dto/LoginDTO.java (1)
Learnt from: RohDamin
PR: #4
File: src/main/java/com/barogagi/plan/command/ex_entity/PlanUserMemebershipInfo.java:7-11
Timestamp: 2025-07-08T13:26:56.806Z
Learning: RohDamin prefers to use @NoArgsConstructor(access = AccessLevel.PROTECTED) in JPA entities instead of the default public access level to prevent indiscriminate object creation while still allowing JPA to instantiate them.
🧬 Code Graph Analysis (2)
src/main/java/com/barogagi/sendSms/dto/SendSmsVO.java (1)
src/main/java/com/barogagi/config/vo/DefaultVO.java (1)
DefaultVO(6-10)
src/main/java/com/barogagi/member/join/dto/UserIdCheckDTO.java (2)
src/main/java/com/barogagi/member/join/vo/UserIdCheckVO.java (1)
UserIdCheckVO(7-11)src/main/java/com/barogagi/member/join/controller/JoinController.java (2)
JoinController(17-156)checkUserId(40-90)
🔇 Additional comments (9)
src/main/java/com/barogagi/approval/vo/ApprovalVO.java (1)
22-24: messageContent 컬럼 추가에 따른 길이·보안 검증 확인
messageContent가 자유 입력 문자열이라면
- DB 컬럼 타입/길이(예: VARCHAR vs TEXT)와 일치 여부,
- XSS · SQL Log 노출 등 보안 필터링 적용 여부를 함께 검토해 주세요.
특히 SMS 메시지는 사용자 데이터가 포함될 수 있으므로 저장 전 HTML/스크립트 제거 정책을 명확히 하는 것이 좋습니다.src/main/java/com/barogagi/member/login/dto/LoginDTO.java (1)
5-7: Jakarta Validation 전환 적용 확인 완료
javax.validation에서jakarta.validation으로의 전환이 올바르게 적용되었습니다.
정규식과 메시지도 변함없어 컴파일·런타임 호환성에 문제 없어 보입니다.Also applies to: 20-21
src/main/resources/mapper/ApprovalMapper.xml (1)
17-18: INSERT 구문의 MESSAGE_CONTENT 매핑 검증 필요
- 테이블
APPROVAL_NUM_INFO에MESSAGE_CONTENT컬럼이 실제 존재하고 길이가 충분한지 확인해 주세요.- 메시지 내용이 90바이트(기본 SMS) 초과 시 장문 SMS(LMS/MMS)로 전환되며 길이가 크게 늘 수 있어, 컬럼 타입은
TEXT계열이 안전합니다.배포 전 스키마와 매퍼의 동기화를 다시 한 번 점검하시기 바랍니다.
src/main/java/com/barogagi/member/join/dto/NickNameDTO.java (2)
4-6: Jakarta Validation으로의 마이그레이션이 올바르게 적용되었습니다.Spring Boot 3+ 환경에서
javax.validation에서jakarta.validation으로의 마이그레이션이 정확하게 수행되었습니다.
13-16: 닉네임 유효성 검증 규칙이 적절하게 설정되었습니다.한글, 영문, 숫자만 허용하는 패턴과 2-12자 길이 제한이 한국어 닉네임 정책에 적합합니다. 사용자 친화적인 오류 메시지도 잘 작성되었습니다.
src/main/java/com/barogagi/member/join/dto/UserIdCheckDTO.java (1)
4-6: Jakarta Validation 마이그레이션이 일관성 있게 적용되었습니다.NickNameDTO와 동일한 방식으로 jakarta validation으로 마이그레이션되어 일관성이 유지되었습니다.
src/main/java/com/barogagi/sendSms/service/SendSmsService.java (1)
49-58: 예외 처리 로직이 잘 구현되었습니다.Nurigo SDK의 특정 예외와 일반 예외를 구분하여 처리하고 적절한 로깅을 수행하고 있습니다.
src/main/java/com/barogagi/approval/controller/ApprovalController.java (2)
89-96: SMS 발송 로직이 적절하게 구현되었습니다.인증번호를 포함한 메시지 생성과 SMS 발송 로직이 명확하고 로깅도 충분히 되어 있습니다.
72-73: 전화번호 변수명이 명확하고 적절합니다.수신자 전화번호를 명확한 변수명으로 추출하여 가독성이 향상되었습니다.
| //@MapperScan("com.barogagi") | ||
| @MapperScan(basePackages = "com.barogagi.**.mapper") // MyBatis 매퍼만 있는 경로로 제한 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
매퍼 스캔 패턴 변경에 대한 검증 필요
@MapperScan 설정을 전체 패키지(com.barogagi)에서 특정 매퍼 패키지(com.barogagi.**.mapper)로 제한하는 것은 성능상 이점이 있지만, 기존 매퍼들이 새로운 패턴에 맞는 위치에 있는지 확인이 필요합니다.
또한 주석 처리된 코드는 제거하는 것이 좋습니다.
다음 스크립트를 실행하여 기존 매퍼 인터페이스들의 위치를 확인해 주세요:
-//@MapperScan("com.barogagi")
-@MapperScan(basePackages = "com.barogagi.**.mapper") // MyBatis 매퍼만 있는 경로로 제한
+@MapperScan(basePackages = "com.barogagi.**.mapper") // MyBatis 매퍼만 있는 경로로 제한🏁 Script executed:
#!/bin/bash
# 기존 매퍼 인터페이스들이 새로운 스캔 패턴에 포함되는지 확인
echo "=== 현재 프로젝트의 모든 매퍼 인터페이스 위치 확인 ==="
fd -e java -x grep -l "@Mapper\|extends.*Mapper" {} \; | head -20
echo -e "\n=== com.barogagi 패키지 하위의 매퍼 관련 파일들 ==="
fd -t f -e java . src/main/java/com/barogagi | grep -i mapper
echo -e "\n=== MyBatis XML 매퍼 파일들 ==="
fd -e xml . src/main/resources | head -10Length of output: 1852
매퍼 스캔 설정 검증 완료 및 주석 코드 제거 제안
새로 지정한 basePackages = "com.barogagi.**.mapper" 패턴이 프로젝트 내 모든 Mapper 인터페이스를 정확히 포함함을 확인했습니다. 스캔 범위 변경으로 인한 누락 이슈는 없습니다.
아래처럼 이전 주석 처리된 설정만 삭제해 주세요.
- //@MapperScan("com.barogagi")
@MapperScan(basePackages = "com.barogagi.**.mapper") // MyBatis 매퍼만 있는 경로로 제한📝 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.
| //@MapperScan("com.barogagi") | |
| @MapperScan(basePackages = "com.barogagi.**.mapper") // MyBatis 매퍼만 있는 경로로 제한 | |
| @MapperScan(basePackages = "com.barogagi.**.mapper") // MyBatis 매퍼만 있는 경로로 제한 |
🤖 Prompt for AI Agents
In src/main/java/com/Application.java around lines 10 to 11, remove the
commented-out old @MapperScan annotation to clean up the code since the new
basePackages setting "com.barogagi.**.mapper" correctly covers all Mapper
interfaces without omissions.
| @Autowired | ||
| private InputValidate inputValidate; | ||
|
|
||
| private final String API_SECRET_KEY; | ||
| @Autowired | ||
| private EncryptUtil encryptUtil; | ||
|
|
||
| @Autowired | ||
| public ApprovalController(Environment environment, | ||
| InputValidate inputValidate, | ||
| EncryptUtil encryptUtil, | ||
| AuthCodeService authCodeService, | ||
| ApprovalService approvalService){ | ||
| private AuthCodeService authCodeService; | ||
|
|
||
| this.API_SECRET_KEY = environment.getProperty("api.secret-key"); | ||
| @Autowired | ||
| private ApprovalService approvalService; | ||
|
|
||
| @Autowired | ||
| private SendSmsService sendSmsService; | ||
|
|
||
| private final String API_SECRET_KEY; | ||
|
|
||
| this.inputValidate = inputValidate; | ||
| this.encryptUtil = encryptUtil; | ||
| this.authCodeService = authCodeService; | ||
| this.approvalService = approvalService; | ||
| @Autowired | ||
| public ApprovalController(Environment environment){ | ||
| this.API_SECRET_KEY = environment.getProperty("api.secret-key"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
의존성 주입 패턴을 일관성 있게 통일하세요.
생성자 주입과 필드 주입이 혼재되어 있습니다. 모든 의존성을 생성자 주입으로 통일하는 것이 Spring의 권장사항입니다.
다음과 같이 생성자 주입으로 통일할 수 있습니다:
- @Autowired
- private InputValidate inputValidate;
-
- @Autowired
- private EncryptUtil encryptUtil;
-
- @Autowired
- private AuthCodeService authCodeService;
-
- @Autowired
- private ApprovalService approvalService;
-
- @Autowired
- private SendSmsService sendSmsService;
-
private final String API_SECRET_KEY;
+ private final InputValidate inputValidate;
+ private final EncryptUtil encryptUtil;
+ private final AuthCodeService authCodeService;
+ private final ApprovalService approvalService;
+ private final SendSmsService sendSmsService;
- @Autowired
- public ApprovalController(Environment environment){
+ public ApprovalController(Environment environment, InputValidate inputValidate,
+ EncryptUtil encryptUtil, AuthCodeService authCodeService,
+ ApprovalService approvalService, SendSmsService sendSmsService) {
this.API_SECRET_KEY = environment.getProperty("api.secret-key");
+ this.inputValidate = inputValidate;
+ this.encryptUtil = encryptUtil;
+ this.authCodeService = authCodeService;
+ this.approvalService = approvalService;
+ this.sendSmsService = sendSmsService;
}📝 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.
| @Autowired | |
| private InputValidate inputValidate; | |
| private final String API_SECRET_KEY; | |
| @Autowired | |
| private EncryptUtil encryptUtil; | |
| @Autowired | |
| public ApprovalController(Environment environment, | |
| InputValidate inputValidate, | |
| EncryptUtil encryptUtil, | |
| AuthCodeService authCodeService, | |
| ApprovalService approvalService){ | |
| private AuthCodeService authCodeService; | |
| this.API_SECRET_KEY = environment.getProperty("api.secret-key"); | |
| @Autowired | |
| private ApprovalService approvalService; | |
| @Autowired | |
| private SendSmsService sendSmsService; | |
| private final String API_SECRET_KEY; | |
| this.inputValidate = inputValidate; | |
| this.encryptUtil = encryptUtil; | |
| this.authCodeService = authCodeService; | |
| this.approvalService = approvalService; | |
| @Autowired | |
| public ApprovalController(Environment environment){ | |
| this.API_SECRET_KEY = environment.getProperty("api.secret-key"); | |
| } | |
| // Remove field injection; declare all dependencies as final | |
| private final String API_SECRET_KEY; | |
| private final InputValidate inputValidate; | |
| private final EncryptUtil encryptUtil; | |
| private final AuthCodeService authCodeService; | |
| private final ApprovalService approvalService; | |
| private final SendSmsService sendSmsService; | |
| public ApprovalController(Environment environment, | |
| InputValidate inputValidate, | |
| EncryptUtil encryptUtil, | |
| AuthCodeService authCodeService, | |
| ApprovalService approvalService, | |
| SendSmsService sendSmsService) { | |
| this.API_SECRET_KEY = environment.getProperty("api.secret-key"); | |
| this.inputValidate = inputValidate; | |
| this.encryptUtil = encryptUtil; | |
| this.authCodeService = authCodeService; | |
| this.approvalService = approvalService; | |
| this.sendSmsService = sendSmsService; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/barogagi/approval/controller/ApprovalController.java around
lines 27 to 47, the dependency injection uses a mix of field injection
(@Autowired on fields) and constructor injection. To follow Spring best
practices, refactor all dependencies to be injected via the constructor. Remove
@Autowired annotations from fields, add all dependencies as constructor
parameters, and assign them to final fields inside the constructor, including
the API_SECRET_KEY property from Environment.
| private String SEND_TEL = ""; | ||
| private String API_KEY = ""; | ||
| private String API_SECRET_KEY = ""; | ||
|
|
||
| public SendSmsService(Environment environment) { | ||
| this.SEND_TEL = environment.getProperty("send.sms.tel"); | ||
| this.API_KEY = environment.getProperty("send.sms.api-key"); | ||
| this.API_SECRET_KEY = environment.getProperty("send.sms.api-secret-key"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
설정 필드를 final로 선언하고 null 체크를 추가하세요.
설정 값들이 런타임에 변경되지 않으므로 final로 선언하고, Environment에서 로드된 값에 대한 null 체크가 필요합니다.
다음과 같이 개선할 수 있습니다:
- private String SEND_TEL = "";
- private String API_KEY = "";
- private String API_SECRET_KEY = "";
+ private final String SEND_TEL;
+ private final String API_KEY;
+ private final String API_SECRET_KEY;
public SendSmsService(Environment environment) {
- this.SEND_TEL = environment.getProperty("send.sms.tel");
- this.API_KEY = environment.getProperty("send.sms.api-key");
- this.API_SECRET_KEY = environment.getProperty("send.sms.api-secret-key");
+ this.SEND_TEL = environment.getRequiredProperty("send.sms.tel");
+ this.API_KEY = environment.getRequiredProperty("send.sms.api-key");
+ this.API_SECRET_KEY = environment.getRequiredProperty("send.sms.api-secret-key");
}📝 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.
| private String SEND_TEL = ""; | |
| private String API_KEY = ""; | |
| private String API_SECRET_KEY = ""; | |
| public SendSmsService(Environment environment) { | |
| this.SEND_TEL = environment.getProperty("send.sms.tel"); | |
| this.API_KEY = environment.getProperty("send.sms.api-key"); | |
| this.API_SECRET_KEY = environment.getProperty("send.sms.api-secret-key"); | |
| } | |
| private final String SEND_TEL; | |
| private final String API_KEY; | |
| private final String API_SECRET_KEY; | |
| public SendSmsService(Environment environment) { | |
| this.SEND_TEL = environment.getRequiredProperty("send.sms.tel"); | |
| this.API_KEY = environment.getRequiredProperty("send.sms.api-key"); | |
| this.API_SECRET_KEY = environment.getRequiredProperty("send.sms.api-secret-key"); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/barogagi/sendSms/service/SendSmsService.java around lines
18 to 26, the configuration fields SEND_TEL, API_KEY, and API_SECRET_KEY should
be declared as final since their values do not change at runtime. Additionally,
after loading these values from the Environment, add null checks to ensure they
are properly set and handle the case where any value is missing, such as
throwing an IllegalArgumentException with a clear message. This will improve
immutability and robustness of the service.
|
|
||
| boolean result = true; | ||
|
|
||
| DefaultMessageService messageService = NurigoApp.INSTANCE.initialize(API_KEY, API_SECRET_KEY, "https://api.solapi.com"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
API URL을 설정 가능하도록 외부화하세요.
하드코딩된 API URL을 application.properties로 이동시켜 환경별 설정이 가능하도록 해야 합니다.
- DefaultMessageService messageService = NurigoApp.INSTANCE.initialize(API_KEY, API_SECRET_KEY, "https://api.solapi.com");
+ String apiUrl = environment.getProperty("send.sms.api-url", "https://api.solapi.com");
+ DefaultMessageService messageService = NurigoApp.INSTANCE.initialize(API_KEY, API_SECRET_KEY, apiUrl);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/barogagi/sendSms/service/SendSmsService.java at line 37,
the API URL is hardcoded in the initialize method call. To fix this, move the
API URL string to application.properties as a configurable property, then inject
or read this property in the SendSmsService class and use it in the initialize
method instead of the hardcoded value. This allows environment-specific
configuration of the API URL.
4 participants
Changed
인증문자 발송 작업
📸 스크린샷 (선택)
📎 관련 이슈(선택)
Todo
Note
Summary by CodeRabbit
신규 기능
버그 수정
문서화
기타