[Feat/#13] SnsHandler 구현 #22
Conversation
- findByDeviceToken() 메서드 추가하여 디바이스 토큰 문자열을 받아 해당하는 DeviceTokenentity 조회
- SNS 핸들러에서 실패한 푸시 알림의 디바이스 토큰을 처리 - findUserByTokenIds()에서 디바이스 토큰 리스트를 받아 해당하는 DeviceTokenEntity 리스트를 조회 - mapDeviceTokenEntityToInfoDto()에서 DeviceTokenEntity를 UserTokenInfoDto로 변환 - findDeviceTokenEntityByPk()에서 PK 문자열로부터 DeviceTokenEntity를 조회
- SNS 이벤트에서 디바이스 토큰 추출 - 실패한 푸시 알림 처리 - Map 기반 토큰 매칭: deviceTokenEntities를 Map으로 전처리하여 매 record 마다 전체 리스트 순회 로직 제거 - 개별 record 처리 실패를 격리하여 배치 전체 실패 방지
|
Warning Rate limit exceeded@sung-silver has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 44 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 (2)
WalkthroughSnsHandler Lambda 구현, 관련 서비스·레포지토리 확장, SAM 로컬 테스트 문서·샘플 이벤트 및 배포 템플릿을 추가하여 SNS 실패 피드백을 수신해 토큰 조회·정리·실패 이력 저장 흐름을 도입했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant SNS as AWS SNS
participant Handler as SnsHandler
participant DevTS as DeviceTokenService
participant Repo as DeviceTokenRepository
participant DDB as DynamoDB
participant Cleaner as InvalidEndpointCleaner
participant History as HistoryService
SNS->>Handler: SNS Event (Records)
Handler->>Handler: validate & extract tokens (ObjectMapper)
Handler->>DevTS: findUserByTokenIds(tokens)
DevTS->>Repo: findByDeviceToken(token) (loop)
Repo->>DDB: Query PARTITION = TOKEN_PREFIX + token
DDB-->>Repo: DeviceTokenEntity
Repo-->>DevTS: Optional<DeviceTokenEntity>
DevTS-->>Handler: List<UserTokenInfoDto>
loop per token with invalid endpoint
Handler->>Cleaner: deleteToken(userId, deviceToken) / unsubscribeEndpoint(...)
Cleaner->>History: createFailureHistoryDto(...) -> persist
History->>DDB: persist failure records
end
Handler-->>SNS: return status string
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![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
🧹 Nitpick comments (3)
test-sns-handler.sh (1)
42-46: AWS 프로필을 설정 가능하도록 개선을 고려하세요.AWS 프로필이
sopt-platform으로 하드코딩되어 있어 다른 프로필을 사용하는 개발자에게 불편할 수 있습니다.🔎 개선 제안
+# AWS 프로필 설정 (기본값: sopt-platform) +AWS_PROFILE=${AWS_PROFILE:-"sopt-platform"} + # 5. 테스트 실행 echo "🧪 SnsHandler 테스트 실행 중..." sam local invoke SnsHandlerFunction \ --event "$EVENT_FILE" \ --env-vars params-dev.json \ - --profile sopt-platform \ + --profile "$AWS_PROFILE" \ --debug사용 예시:
# 기본 프로필 사용 ./test-sns-handler.sh # 다른 프로필 사용 AWS_PROFILE=my-profile ./test-sns-handler.shsrc/main/java/com/sopt/push/lambda/SnsHandler.java (1)
180-194: null 반환 대신 빈 Set 사용을 고려해보세요.
buildUserIds와buildMessageIds메서드가 비어있을 때null을 반환하는 패턴은 NPE 위험을 증가시킬 수 있습니다. 일반적으로 빈 컬렉션을 반환하는 것이 더 안전한 패턴입니다.🔎 제안하는 개선 방법
private Set<String> buildUserIds(String userId) { Set<String> userIds = new HashSet<>(); if (userId != null && !userId.isBlank()) { userIds.add(userId); } - return userIds.isEmpty() ? null : userIds; + return userIds; } private Set<String> buildMessageIds(String messageId) { Set<String> messageIds = new HashSet<>(); if (messageId != null && !messageId.isBlank()) { messageIds.add(messageId); } - return messageIds.isEmpty() ? null : messageIds; + return messageIds; }단,
CreateHistoryDto나HistoryService가 빈 Set과 null을 다르게 처리해야 하는 비즈니스 요구사항이 있다면 현재 구현을 유지하세요.src/main/java/com/sopt/push/service/UserService.java (1)
71-77: 메서드 이름이 실제 동작과 일치하지 않습니다.메서드 이름이
findDeviceTokenEntityByPk이지만, 실제로는 pk에서 deviceToken을 추출한 후deviceTokenRepository.findByDeviceToken()을 호출합니다. 메서드 이름이 구현 세부사항을 정확하게 반영하지 못하고 있습니다.🔎 제안하는 개선 방법
메서드 이름을 실제 동작에 맞게 변경하는 것을 고려해보세요:
-public DeviceTokenEntity findDeviceTokenEntityByPk(String pk) { +public DeviceTokenEntity extractAndFindDeviceToken(String tokenPk) { - if (pk == null || !pk.startsWith(TOKEN_PREFIX)) { + if (tokenPk == null || !tokenPk.startsWith(TOKEN_PREFIX)) { return null; } - String deviceToken = pk.substring(TOKEN_PREFIX.length()); + String deviceToken = tokenPk.substring(TOKEN_PREFIX.length()); return deviceTokenRepository.findByDeviceToken(deviceToken).orElse(null); }또는 더 간단하게:
-public DeviceTokenEntity findDeviceTokenEntityByPk(String pk) { - if (pk == null || !pk.startsWith(TOKEN_PREFIX)) { - return null; - } - String deviceToken = pk.substring(TOKEN_PREFIX.length()); - return deviceTokenRepository.findByDeviceToken(deviceToken).orElse(null); +public DeviceTokenEntity findByTokenPk(String tokenPk) { + if (tokenPk == null || !tokenPk.startsWith(TOKEN_PREFIX)) { + return null; + } + String deviceToken = tokenPk.substring(TOKEN_PREFIX.length()); + return deviceTokenRepository.findByDeviceToken(deviceToken).orElse(null); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
SNSHANDLER_LOCAL_TESTING_GUIDE.md(1 hunks)SNSHANDLER_QUICK_START.md(1 hunks)events/README.md(1 hunks)events/sns-event-sample.json(1 hunks)events/sns-event-single.json(1 hunks)src/main/java/com/sopt/push/common/Constants.java(1 hunks)src/main/java/com/sopt/push/config/AppFactory.java(3 hunks)src/main/java/com/sopt/push/lambda/SnsHandler.java(1 hunks)src/main/java/com/sopt/push/repository/DeviceTokenRepository.java(2 hunks)src/main/java/com/sopt/push/service/InvalidEndpointCleaner.java(1 hunks)src/main/java/com/sopt/push/service/UserService.java(2 hunks)template.yaml(1 hunks)test-sns-handler.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/sopt/push/service/UserService.java (2)
src/main/java/com/sopt/push/repository/DeviceTokenRepository.java (1)
DeviceTokenRepository(13-41)src/main/java/com/sopt/push/repository/UserRepository.java (1)
UserRepository(11-32)
🪛 Gitleaks (8.30.0)
events/README.md
[high] 15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (15)
src/main/java/com/sopt/push/common/Constants.java (1)
39-39: LGTM!SNS 메시지에서 토큰을 추출하기 위한 상수가 적절하게 추가되었습니다.
events/sns-event-single.json (1)
1-23: LGTM!SNS 이벤트 샘플 구조가 올바르게 작성되었습니다. 테스트용 플레이스홀더 값들이 적절히 포함되어 있습니다.
template.yaml (1)
157-163: SNS 이벤트 소스 설정이 누락되었습니다.
SnsHandlerFunction이 정의되었지만 SNS 토픽 구독 설정이 없습니다. Lambda가 SNS 알림을 받으려면 이벤트 소스 설정이 필요합니다.다음 중 하나의 방법으로 설정이 필요합니다:
- template.yaml에 Events 섹션 추가 (권장):
SnsHandlerFunction: Type: AWS::Serverless::Function Properties: FunctionName: !Sub "sopt-push-notification-lambda-sns-${Stage}" Handler: com.sopt.push.lambda.SnsHandler::handleRequest CodeUri: build/libs/app.jar Role: !GetAtt PushLambdaRole.Arn Events: SnsEvent: Type: SNS Properties: Topic: !Ref PushFailureTopicArn # Parameters에 추가 필요
- 별도로 SNS 구독을 수동 설정하는 경우, 배포 가이드에 이를 명시해야 합니다.
현재 어떤 방식으로 SNS 구독을 설정할 계획인지 확인해 주세요.
events/sns-event-sample.json (1)
1-41: LGTM!복수 레코드를 포함한 SNS 이벤트 샘플이 올바르게 구성되었습니다. 배치 처리 테스트에 유용합니다.
src/main/java/com/sopt/push/service/InvalidEndpointCleaner.java (1)
25-25: 메서드 파라미터 순서가 일관되게 적용되었습니다.
DeviceTokenService.deleteToken메서드 시그니처는public void deleteToken(String userId, String deviceToken)이고, 호출 지점tokenService.deleteToken(token.userId(), token.deviceToken())은 이 시그니처와 일치합니다. 코드베이스에서 발견된 유일한 호출 지점이 올바른 파라미터 순서를 사용하고 있으므로 추가 조치는 불필요합니다.src/main/java/com/sopt/push/repository/DeviceTokenRepository.java (1)
35-40: LGTM!디바이스 토큰으로 엔티티를 조회하는 새로운 메서드가 올바르게 구현되었습니다. TOKEN_PREFIX를 사용하여 파티션 키를 구성하고, QueryConditional로 쿼리를 실행하는 방식이 적절합니다.
src/main/java/com/sopt/push/config/AppFactory.java (1)
22-26: LGTM!서비스를 인스턴스 필드로 리팩토링하고 getter 메서드를 추가하여 SnsHandler가 AppFactory를 통해 서비스에 접근할 수 있도록 한 변경사항이 적절합니다. 기존 코드에 영향을 주지 않으면서 깔끔하게 구조가 개선되었습니다.
Also applies to: 40-46, 71-89
SNSHANDLER_LOCAL_TESTING_GUIDE.md (1)
1-205: 로컬 테스트 가이드가 매우 포괄적입니다.SAM CLI를 사용한 로컬 테스트 방법이 잘 문서화되어 있으며, 사전 요구사항부터 문제 해결까지 모든 단계가 명확하게 설명되어 있습니다. 다양한 테스트 시나리오와 디버깅 팁도 유용합니다.
src/main/java/com/sopt/push/lambda/SnsHandler.java (4)
38-55: LGTM!두 개의 생성자를 제공하여 프로덕션 환경(AppFactory 사용)과 테스트 환경(명시적 DI) 모두를 지원하는 구조가 적절합니다.
57-101: 배치 처리 로직이 잘 구현되었습니다.SNS 이벤트를 검증하고, 토큰을 추출한 후, 각 레코드를 개별적으로 처리하는 흐름이 명확합니다. 특히 Line 84-90에서 각 레코드 처리 중 발생한 예외를 개별적으로 처리하여 한 레코드의 실패가 전체 배치를 중단시키지 않도록 한 점이 좋습니다.
196-216: 많은 필드가 null인 CreateHistoryDto 생성을 확인해주세요.실패 로그 생성 시 대부분의 필드가
null로 설정되어 있습니다. SNS 실패 처리의 특성상 제한된 정보만 있는 것이 정상이라면 문제없지만, 추후 로그 분석이나 디버깅 시 정보 부족으로 인한 어려움이 있을 수 있습니다.다음 사항을 확인해주세요:
title,content등의 필드에 기본값(예: "SNS Delivery Failure")을 설정할 필요가 있는지errorCode,errorMessage필드에 SNS 메시지에서 추출 가능한 에러 정보를 포함할 수 있는지
144-170: LGTM!JSON 파싱 시 다양한 예외 상황(null SNS, blank message, missing token)을 체계적으로 처리하고 있으며, 각 단계에서 적절한 검증을 수행합니다.
src/main/java/com/sopt/push/service/UserService.java (3)
20-24: LGTM!DeviceTokenRepository 의존성이 적절하게 추가되었습니다.
63-69: LGTM!디바이스 토큰 목록으로 엔티티를 조회하는 로직이 올바르게 구현되었습니다. Optional 처리로 존재하지 않는 토큰을 안전하게 스킵합니다.
79-95: LGTM!DeviceTokenEntity를 UserTokenInfoDto로 매핑하는 로직이 올바르게 구현되었습니다. pk/sk의 역할이 UserEntity와 반대인 점을 정확하게 처리하고 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
SNSHANDLER_LOCAL_TESTING_GUIDE.md (3)
99-108: 환경 변수 섹션에 필수 변수와 선택 변수 구분을 추가하세요.
params-dev.json예제가 제공되어 있지만, 모든 환경 변수가 필수인지 선택 사항인지 명시되지 않았습니다. 다음을 추가하면 가이드의 실용성이 높아질 것입니다:
- 각 환경 변수의 필수 여부 표시
- 기본값이 있는 경우 그 값 명시
- 각 변수의 용도 간단히 설명
예를 들어:
{ "SnsHandlerFunction": { "DYNAMODB_TABLE": "your-dynamodb-table-name", // 필수 "PLATFORM_APPLICATION_iOS": "arn:aws:sns:...", // 필수 // ... 등등 } }이렇게 구조화하면 사용자가 누락된 변수를 놓치기 쉬워집니다.
129-135: 테스트 이벤트 파일 커스터마이징 섹션에 더 실용적인 가이드를 추가하세요.현재 섹션은 개념적 설명만 제공하고 있습니다. 다음 항목들을 추가하면 개발자들이 더 빠르게 테스트를 설정할 수 있습니다:
- SNS 이벤트 JSON의 정확한 구조 (완전한 예제)
Token값이 실제 DynamoDB 테이블에서 어떻게 조회되는지- 배치 처리 테스트용 여러 레코드 추가 방법의 구체적 예제
- 각 필드가 무엇을 나타내는지 주석 추가
187-203: 승인 - 문제 해결 섹션이 잘 구성되어 있습니다.문제 해결 섹션은 일반적인 에러와 해결 방법을 명확하게 제시하고 있어 좋습니다. 다만 다음이 추가되면 더욱 유용할 것입니다:
- SAM CLI 버전 호환성 확인 방법
- Java 21 설치 후 PATH 확인 방법
- 빌드 캐시 문제로 인한 "stale JAR" 문제 해결 방법 (
gradle clean shadowJar)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
SNSHANDLER_LOCAL_TESTING_GUIDE.md(1 hunks)
🔇 Additional comments (1)
SNSHANDLER_LOCAL_TESTING_GUIDE.md (1)
76-86: 테스트 이벤트 파일과 환경 변수 파일의 존재 확인이 필요합니다.가이드에서
events/sns-event-single.json,events/sns-event-sample.json, 그리고params-dev.json파일을 참조하고 있으나, 이 파일들이 이 PR에 포함되어 있지 않습니다. 사용자가 이 파일들을 직접 생성해야 하는지, 아니면 저장소에 이미 존재해야 하는지 명확히 하기 위해 다음을 확인해주세요:
- 샘플 이벤트 파일이 저장소에 포함되어 있는가?
params-dev.json이.gitignore에 의해 제외되었는가? (환경 변수 파일의 경우 흔함)- 파일이 존재하지 않을 경우, 가이드에 파일 생성 방법을 추가해야 합니다.
seoyeonjin
left a comment
seoyeonjin
left a comment
There was a problem hiding this comment.
수고하셨습니다 !!
테스트파일까지 작성해주시고 넘넘 좋습니당~~
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/sopt/push/service/DeviceTokenService.java (1)
51-57: 토큰 조회 전에 유효성 검증을 추가하면 좋겠습니다.입력 리스트에 null 또는 빈 문자열이 포함된 경우 불필요한 조회가 발생할 수 있습니다. 조회 전에 필터링을 추가하는 것을 권장합니다.
🔎 개선 제안
public List<DeviceTokenEntity> findUserByTokenIds(List<String> deviceTokens) { List<DeviceTokenEntity> result = new ArrayList<>(); - for (String deviceToken : deviceTokens) { + for (String deviceToken : deviceTokens) { + if (deviceToken == null || deviceToken.isBlank()) { + continue; + } deviceTokenRepository.findByDeviceToken(deviceToken).ifPresent(result::add); } return result; }src/main/java/com/sopt/push/lambda/SnsHandler.java (1)
68-73: 중복 토큰 처리 시 로깅을 추가하는 것을 권장합니다.Line 73의 merge 함수
(a, b) -> a는 중복된 토큰이 발견되었을 때 첫 번째 값을 유지하면서 조용히 넘어갑니다. 중복이 발생하는 것은 예상치 못한 상황일 수 있으므로, 로깅을 추가하여 추적할 수 있도록 하는 것이 좋습니다.🔎 개선 제안
Map<String, UserTokenInfoDto> tokenMap = deviceTokenEntities.stream() .map(deviceTokenService::mapDeviceTokenEntityToInfoDto) .collect( Collectors.toMap( - UserTokenInfoDto::deviceToken, Function.identity(), (a, b) -> a)); + UserTokenInfoDto::deviceToken, + Function.identity(), + (a, b) -> { + log.warn("Duplicate device token found: {}", a.deviceToken()); + return a; + }));
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
SNSHANDLER_LOCAL_TESTING_GUIDE.mdevents/README.mdsrc/main/java/com/sopt/push/lambda/SnsHandler.javasrc/main/java/com/sopt/push/service/DeviceTokenService.javasrc/main/java/com/sopt/push/service/UserService.javatemplate.yaml
✅ Files skipped from review due to trivial changes (1)
- SNSHANDLER_LOCAL_TESTING_GUIDE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/sopt/push/service/UserService.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/com/sopt/push/service/DeviceTokenService.java (1)
src/main/java/com/sopt/push/repository/DeviceTokenRepository.java (1)
DeviceTokenRepository(13-41)
src/main/java/com/sopt/push/lambda/SnsHandler.java (3)
src/main/java/com/sopt/push/config/AppFactory.java (1)
AppFactory(16-90)src/main/java/com/sopt/push/service/DeviceTokenService.java (1)
DeviceTokenService(15-76)src/main/java/com/sopt/push/service/HistoryService.java (1)
HistoryService(14-56)
🔇 Additional comments (6)
events/README.md (1)
1-146: 문서가 명확하고 포괄적입니다.SNS 이벤트 파일 수정 가이드가 잘 작성되었습니다. 필수/선택/불필요 항목을 명확히 구분하고, 실제 예시와 테스트 시나리오를 포함하여 개발자가 쉽게 따라할 수 있습니다.
template.yaml (1)
162-173: SnsHandler 구성이 올바르게 설정되었습니다.Lambda 함수 정의와 SNS 이벤트 구독 설정이 적절합니다. 이전 리뷰 코멘트에서 언급된 SNS 구독 설정이 정상적으로 추가되었습니다.
src/main/java/com/sopt/push/lambda/SnsHandler.java (3)
103-123: 레코드 처리 로직이 깔끔합니다.조기 반환(early return)을 활용하여 유효하지 않은 케이스를 효율적으로 처리하고 있으며, 적절한 로깅도 포함되어 있습니다.
135-161: 토큰 추출 로직이 방어적으로 잘 작성되었습니다.null 체크, 빈 문자열 검증, JSON 파싱 예외 처리 등이 포괄적으로 구현되어 있어 안전합니다. 예외 발생 시 null을 반환하여 전체 처리를 중단하지 않는 것도 적절합니다.
125-133: InvalidEndpointCleaner.clean 메서드 시그니처는 올바릅니다.
InvalidEndpointCleaner클래스의clean메서드 시그니처는public void clean(UserTokenInfoDto token)이므로, 라인 129에서invalidEndpointCleaner.clean(userTokenInfoDto)를 호출하는 것이 올바릅니다. 파라미터 타입이 정확히 일치합니다.src/main/java/com/sopt/push/service/DeviceTokenService.java (1)
59-75: Platform.fromValue는 적절히 예외 처리되고 있습니다.Platform.fromValue() 메서드는 유효하지 않은 플랫폼 값(null, 공백, 또는 미지의 값)에 대해 BusinessException을 발생시킵니다. 이 메서드는 SnsHandler에서 스트림 작업 내에서 호출되며, 해당 호출은 Exception을 처리하는 try-catch 블록으로 감싸져 있으므로 예외가 적절히 포착되고 로깅됩니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/com/sopt/push/service/UserService.java (1)
49-54: 메서드 구현이 올바르게 되어 있습니다.DynamoDB 삭제 로직이 명확하고 PREFIX 상수를 활용한 키 구성이 적절합니다.
다만, 호출하는 쪽에서 검증이 이루어지지 않는다면 파라미터에 대한 null 체크나 빈 문자열 검증을 추가하는 것을 고려해보세요.
🔎 선택적 개선안: 파라미터 유효성 검증 추가
public void deleteUser(String userId, String deviceToken) { + if (userId == null || userId.isBlank() || deviceToken == null || deviceToken.isBlank()) { + throw new IllegalArgumentException("userId and deviceToken must not be null or blank"); + } String userPk = USER_PREFIX + userId; String tokenSk = TOKEN_PREFIX + deviceToken; userRepository.delete(userPk, tokenSk); }src/main/java/com/sopt/push/lambda/SnsHandler.java (2)
95-102: Stream API 사용을 고려해보세요.현재 구현은 정상적으로 동작하지만, Java Stream API를 사용하면 더 간결하고 함수형 스타일로 작성할 수 있습니다.
🔎 Stream API를 활용한 리팩터링 제안
- private Map<SNSEvent.SNSRecord, String> extractRecordTokens(List<SNSEvent.SNSRecord> records) { - Map<SNSEvent.SNSRecord, String> recordTokenMap = new HashMap<>(); - for (SNSEvent.SNSRecord record : records) { - String token = extractTokenFromRecord(record); - recordTokenMap.put(record, token); - } - return recordTokenMap; - } + private Map<SNSEvent.SNSRecord, String> extractRecordTokens(List<SNSEvent.SNSRecord> records) { + return records.stream() + .collect(Collectors.toMap( + Function.identity(), + this::extractTokenFromRecord)); + }
175-195: CreateHistoryDto 생성 시 가독성 개선을 고려하세요.많은 null 파라미터로 인해 어떤 필드가 어떤 값인지 파악하기 어렵습니다. 빌더 패턴이나 네임드 파라미터를 사용하면 가독성과 유지보수성이 향상됩니다.
#!/bin/bash # CreateHistoryDto가 빌더 패턴을 지원하는지 확인 ast-grep --pattern $'class CreateHistoryDto { $$$ public static class Builder { $$$ } $$$ }'빌더 패턴이 아직 지원되지 않는다면, CreateHistoryDto에 빌더를 추가하는 것을 고려해보세요.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/sopt/push/config/AppFactory.javasrc/main/java/com/sopt/push/lambda/SnsHandler.javasrc/main/java/com/sopt/push/service/DeviceTokenService.javasrc/main/java/com/sopt/push/service/UserService.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/sopt/push/config/AppFactory.java
- src/main/java/com/sopt/push/service/DeviceTokenService.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/sopt/push/lambda/SnsHandler.java (3)
src/main/java/com/sopt/push/config/AppFactory.java (1)
AppFactory(16-90)src/main/java/com/sopt/push/service/DeviceTokenService.java (1)
DeviceTokenService(15-76)src/main/java/com/sopt/push/service/HistoryService.java (1)
HistoryService(14-56)
🔇 Additional comments (9)
src/main/java/com/sopt/push/service/UserService.java (1)
13-15: LGTM! Lombok 어노테이션 적용이 적절합니다.과거 리뷰에서 제안된 대로
@RequiredArgsConstructor를 올바르게 적용하여 생성자 보일러플레이트를 제거했습니다.src/main/java/com/sopt/push/lambda/SnsHandler.java (8)
39-46: 생성자 구현이 적절합니다.이전 리뷰 논의에 따라 테스트용 생성자를 제거하고 Lambda용 기본 생성자만 유지한 것이 적절합니다. AppFactory를 통한 의존성 주입도 올바르게 구현되었습니다.
50-55: 이벤트 검증 로직이 명확합니다.null 및 빈 레코드 검증이 적절하게 구현되었습니다. early return 패턴이 잘 적용되어 있습니다.
76-82: 개별 레코드 오류 격리가 잘 구현되었습니다.각 레코드를 독립적으로 처리하고 개별 실패가 전체 배치를 중단하지 않도록 한 것은 Lambda의 복원력을 높이는 좋은 설계입니다.
104-124: 레코드 처리 로직이 적절합니다.토큰 검증, 사용자 정보 조회, 무효 엔드포인트 처리로 이어지는 흐름이 명확합니다. Line 112에서 토큰이 존재하지 않는 경우에만 로깅하는 것은 null 토큰(예상되는 케이스)과 미등록 토큰(추적이 필요한 케이스)을 구분하는 좋은 접근입니다.
126-134: 엔드포인트 정리 실패 시 예외가 전파되지 않는 것을 확인하세요.Line 129-133에서
invalidEndpointCleaner.clean()실패 시 예외를 로깅만 하고 전파하지 않습니다. 이는 정리 실패가 이력 로그 생성(line 127)을 방해하지 않도록 하는 복원력 있는 설계로 보이지만, 정리 실패가 반복될 경우 무효 토큰이 계속 남아있을 수 있습니다.다음을 확인해주세요:
- 이러한 실패가 모니터링되고 있는지
- 정리 실패 시 재시도 메커니즘이 있는지
136-162: 토큰 추출 로직이 방어적으로 잘 구현되었습니다.SNS 메시지 파싱 과정에서 발생할 수 있는 모든 엣지 케이스(null sns, 빈 메시지, 누락된 토큰 노드 등)를 적절하게 처리하고 있습니다. 예외 발생 시 null을 반환하여 전체 배치 처리를 중단하지 않는 것도 올바른 접근입니다.
164-173: null 처리 방식이 이전 리뷰 논의를 반영했습니다.이전 리뷰 코멘트에서 언급된
Collections.emptySet()대신null을 사용하는 방식으로 변경되었습니다. Line 166과 169에서 유효하지 않은 경우null을 반환하는 것은 "데이터 없음"을 의미론적으로 명확하게 표현하며, 이는 적절한 선택입니다.Based on learnings: 이전 리뷰에서 논의된 내용이 반영되었습니다.
86-92: 예외 타입은 Lambda의 재시도 정책에 영향을 주지 않습니다.Lambda가 함수로부터 오류를 받으면 기본적으로 2회 추가 실행을 시도하며, 함수 오류는 함수의 코드 또는 Lambda 런타임에서 반환된 오류를 포함합니다. 함수의 코드나 Lambda 런타임이 오류를 반환할 때 Lambda 응답의 상태 코드는 200 OK이며, X-Amz-Function-Error 헤더로 오류 발생 여부가 표시됩니다. 예외를 RuntimeException으로 래핑하든 checked exception으로 두든 재시도 동작은 동일합니다. 현재 코드의 예외 처리는 적절합니다.
Likely an incorrect or invalid review comment.
Related Issue 🚀
Work Description ✏️
PR Point 📸