[FEAT] 알림 토큰 생성, 삭제 API 구현 #296
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fcm에 종속적인 파일을 삭제했습니다.
WalkthroughFCM(Firebase Admin) 의존성 추가 및 회원 푸시 토큰 관리 기능(엔티티, 리포지토리, 서비스, 컨트롤러, DTO, 예외, 변환기, API 문서)이 멀티레이어로 추가되었고 보안 설정에 Changes
Sequence Diagram(s)%%{init: {"themeVariables":{"actorBorder":"#2b6cb0","actorBackground":"#ebf8ff","sequenceNumber":"#666"}}}%%
sequenceDiagram
participant Client as Client
participant Controller as MemberPushTokenController
participant Service as NotificationTokenService
participant Provider as NotificationProvider
participant Repo as MemberPushTokenRepository
participant DB as Database
rect rgb(220,235,255)
Note over Client,Controller: 푸시 토큰 생성 흐름
Client->>Controller: POST /api/pushToken (memberId, pushToken, provider)
Controller->>Service: create(memberId, request)
Service->>Provider: NotificationProvider.find(provider)
alt provider 존재
Provider-->>Service: provider enum
Service->>Repo: findByPushToken(pushToken)
alt 토큰 없음
Repo-->>Service: empty
Service->>Repo: save(model)
Repo->>DB: INSERT
DB-->>Repo: OK
Repo-->>Service: saved model
else 토큰 존재
Repo-->>Service: existing model -> renew/skip save
end
else provider 없음
Provider-->>Service: throw NotFoundNotificationProviderException
end
Service-->>Controller: void
Controller-->>Client: 201 CREATED
end
rect rgb(255,240,230)
Note over Client,Controller: 푸시 토큰 삭제 흐름
Client->>Controller: DELETE /api/pushToken (memberId, pushToken)
Controller->>Service: delete(memberId, request)
Service->>Repo: findByPushToken(pushToken)
alt 토큰 없음
Repo-->>Service: empty -> throw NotFoundPushTokenException
else 토큰 존재
Repo-->>Service: model
alt memberId 일치
Service->>Repo: deleteByPushToken(pushToken)
Repo->>DB: DELETE
DB-->>Repo: OK
else 불일치
Service-->>Service: throw DeniedDeletePushTokenException
end
end
Service-->>Controller: void
Controller-->>Client: 200 OK
end
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)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-03-01T17:43:11.312ZApplied to files:
🧬 Code graph analysis (1)eeos/src/main/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.java (2)
⏰ 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)
🔇 Additional comments (4)
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: 10
🧹 Nitpick comments (6)
eeos/src/main/java/com/blackcompany/eeos/notification/application/usecase/CreateMemberPushTokenUsecase.java (1)
6-7: 인터페이스의 @component 어노테이션 제거 권장인터페이스에
@Component어노테이션을 사용하는 것은 Spring의 일반적인 관례가 아닙니다. 스테레오타입 어노테이션(@Component,@Service등)은 일반적으로 구현 클래스에 적용됩니다.현재 구조에서는 구현체인
NotificationTokenService가 적절한 어노테이션(@Service등)을 가지고 있으므로, 인터페이스의@Component는 제거하는 것이 명확합니다.🔎 제안하는 수정
-@Component public interface CreateMemberPushTokenUsecase { void create(Long memberId, CreateMemberPushTokenRequest request); }eeos/gradle/libs.versions.toml (1)
13-13: Firebase Admin SDK 버전 업데이트 권장Firebase Admin SDK 9.2.0 버전은 현재 최신 버전인 9.7.0(2025년 9월 24일 릴리스)보다 뒤처져 있습니다. 9.2.0에는 알려진 보안 취약점(CVE)이 없지만, 최신 버전으로 업데이트하여 버그 수정과 개선사항을 받을 것을 권장합니다.
eeos/build.gradle.kts (1)
67-68: Firebase Admin SDK 버전 업그레이드 권고현재 사용 중인 Firebase Admin SDK 9.2.0은 알려진 보안 취약점이 없으나, 최신 버전 9.7.0 (2025년 9월 24일 출시)이 있습니다. 최신 버전으로 업그레이드하면 버그 수정 및 의존성 업데이트의 이점을 얻을 수 있습니다.
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/JpaMemberPushTokenRepository.java (1)
27-27: java.time.LocalDateTime 사용을 고려해보세요.
java.sql.Timestamp대신java.time.LocalDateTime을 파라미터로 직접 사용하는 것이 최신 Java 날짜/시간 API를 활용하는 더 나은 방법입니다. Spring Data JPA는LocalDateTime을 자동으로 변환해줍니다.🔎 제안하는 개선안
-int deleteByUpdatedDateBefore(Timestamp updatedDate); +int deleteByUpdatedDateBefore(LocalDateTime updatedDate);단, 이 경우 MemberPushTokenRepositoryImpl의 변환 로직도 제거할 수 있습니다.
eeos/src/main/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.java (2)
46-50: 삭제된 토큰 수 반환 고려현재 메서드는 void를 반환하므로 실제로 토큰이 삭제되었는지 알 수 없습니다. 디버깅, 로깅, 또는 API 응답을 위해 삭제된 토큰의 수를 반환하는 것을 고려해보세요.
🔎 제안하는 수정안
@Override @Transactional -public void deleteAllMemberPushTokens(Long memberId) { - memberPushTokenRepository.deleteByMemberId(memberId); +public int deleteAllMemberPushTokens(Long memberId) { + int deletedCount = memberPushTokenRepository.deleteByMemberId(memberId); + log.debug("Deleted {} push tokens for member {}", deletedCount, memberId); + return deletedCount; }참고: 이를 위해서는
DeleteAllMemberPushTokensUsecase인터페이스와MemberPushTokenRepository.deleteByMemberId()메서드의 반환 타입도 함께 수정해야 합니다.
52-63: 삭제 로직 최적화 고려현재 구현은 두 단계로 동작합니다:
- Line 55-58: 토큰을 조회하여 소유권 확인
- Line 62: pushToken 문자열로 토큰 삭제
잠재적 개선 사항:
- TOCTOU 이슈: 소유권 확인(Line 59)과 삭제(Line 62) 사이에 다른 트랜잭션이 토큰을 삭제할 수 있습니다. 트랜잭션 격리 수준에 따라 조용히 실패할 수 있습니다.
- 성능: Line 62에서 pushToken 문자열로 다시 검색하는 대신, 이미 조회한 모델의 ID를 사용하여 삭제할 수 있습니다.
🔎 제안하는 수정안
Repository에 ID 기반 삭제 메서드가 있다면:
@Override @Transactional public void delete(Long memberId, DeleteMemberPushTokenRequest request) { MemberPushTokenModel model = memberPushTokenRepository .findByPushToken(request.getPushToken()) .orElseThrow(() -> new NotFoundPushTokenException(request.getPushToken())); if (!model.getMemberId().equals(memberId)) { throw new DeniedDeletePushTokenException(); } - memberPushTokenRepository.deleteByPushToken(request.getPushToken()); + memberPushTokenRepository.deleteById(model.getId()); }이렇게 하면:
- 두 번째 조회를 피할 수 있습니다
- TOCTOU 위험이 약간 감소합니다 (ID가 더 구체적이므로)
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
eeos/build.gradle.ktseeos/gradle/libs.versions.tomleeos/src/main/java/com/blackcompany/eeos/config/security/SecurityFilterChainConfig.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/dto/CreateMemberPushTokenRequest.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/dto/DeleteMemberPushTokenRequest.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/exception/DeniedDeletePushTokenException.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/exception/NotFoundNotificationProviderException.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/exception/NotFoundPushTokenException.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/model/MemberPushTokenModel.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/model/NotificationProvider.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/model/converter/MemberPushTokenEntityConverter.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/respository/MemberPushTokenRepository.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/usecase/CreateMemberPushTokenUsecase.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/usecase/DeleteAllMemberPushTokensUsecase.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/usecase/DeleteMemberPushTokenUsecase.javaeeos/src/main/java/com/blackcompany/eeos/notification/persistence/JpaMemberPushTokenRepository.javaeeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.javaeeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenRepositoryImpl.javaeeos/src/main/java/com/blackcompany/eeos/notification/presentation/controller/MemberPushTokenController.javaeeos/src/main/java/com/blackcompany/eeos/notification/presentation/docs/MemberPushTokenApi.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-16T14:22:46.594Z
Learnt from: rlajm1203
Repo: JNU-econovation/EEOS-BE PR: 256
File: eeos/src/main/java/com/blackcompany/eeos/config/security/UnknownEndpointFilter.java:29-41
Timestamp: 2025-07-16T14:22:46.594Z
Learning: EEOS-BE 프로젝트에서는 여러 개의 SecurityFilterChain을 등록하고, 모든 API 엔드포인트들을 시큐리티 필터에 매칭시켜 놓았습니다. UnknownEndpointFilter는 SecurityMatcher에 걸리지 않는 요청들만 처리하는 catch-all 역할을 하므로, 정상적인 API 요청들이 이 필터에 도달하지 않습니다.
Applied to files:
eeos/src/main/java/com/blackcompany/eeos/config/security/SecurityFilterChainConfig.java
🧬 Code graph analysis (7)
eeos/src/main/java/com/blackcompany/eeos/notification/application/dto/DeleteMemberPushTokenRequest.java (1)
eeos/src/main/java/com/blackcompany/eeos/notification/application/dto/CreateMemberPushTokenRequest.java (1)
Getter(9-16)
eeos/src/main/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.java (2)
eeos/src/main/java/com/blackcompany/eeos/notification/application/exception/DeniedDeletePushTokenException.java (1)
DeniedDeletePushTokenException(6-17)eeos/src/main/java/com/blackcompany/eeos/notification/application/exception/NotFoundPushTokenException.java (1)
NotFoundPushTokenException(6-21)
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java (1)
eeos/src/main/java/com/blackcompany/eeos/notification/application/model/MemberPushTokenModel.java (1)
Getter(9-18)
eeos/src/main/java/com/blackcompany/eeos/notification/application/usecase/CreateMemberPushTokenUsecase.java (1)
eeos/src/main/java/com/blackcompany/eeos/notification/application/model/converter/MemberPushTokenEntityConverter.java (1)
Component(8-31)
eeos/src/main/java/com/blackcompany/eeos/notification/application/dto/CreateMemberPushTokenRequest.java (1)
eeos/src/main/java/com/blackcompany/eeos/notification/application/dto/DeleteMemberPushTokenRequest.java (1)
Getter(9-15)
eeos/src/main/java/com/blackcompany/eeos/notification/application/model/MemberPushTokenModel.java (1)
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java (1)
Getter(20-45)
eeos/src/main/java/com/blackcompany/eeos/notification/presentation/docs/MemberPushTokenApi.java (1)
eeos/src/main/java/com/blackcompany/eeos/common/presentation/response/ApiResponseBody.java (1)
ApiResponseBody(7-24)
⏰ 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)
eeos/src/main/java/com/blackcompany/eeos/config/security/SecurityFilterChainConfig.java (1)
90-91: LGTM!인증된 사용자의 푸시 토큰 관리 엔드포인트가 올바르게 등록되었습니다. 기존
/api/calendars/**패턴과 일관성 있게 구현되어 있습니다.eeos/src/main/java/com/blackcompany/eeos/notification/application/respository/MemberPushTokenRepository.java (1)
9-24: 메서드 시그니처 잘 설계됨리포지토리 메서드들이 적절하게 설계되었습니다:
Optional사용으로 null-safety 보장- 삭제 메서드의
int반환으로 영향받은 행 수 추적 가능- 다양한 조회 조건 지원 (토큰, 멤버 ID, 프로바이더, 날짜 기반)
eeos/src/main/java/com/blackcompany/eeos/notification/application/exception/NotFoundNotificationProviderException.java (1)
6-20: 구현이 올바릅니다.예외 클래스 구조가 적절하고, HTTP 404 상태 코드가 존재하지 않는 provider 조회에 맞습니다. 에러 메시지 포맷팅도 명확합니다.
eeos/src/main/java/com/blackcompany/eeos/notification/application/model/NotificationProvider.java (1)
7-24: LGTM!열거형 구조와
find()메서드 구현이 적절합니다. 대소문자 무관 검색으로 클라이언트 요청을 안전하게 처리합니다.eeos/src/main/java/com/blackcompany/eeos/notification/presentation/docs/MemberPushTokenApi.java (1)
12-25: LGTM!API 인터페이스 정의가 명확하고 Swagger 문서화가 잘 되어 있습니다. 일관된 응답 타입과 적절한 유효성 검증 어노테이션이 적용되어 있습니다.
eeos/src/main/java/com/blackcompany/eeos/notification/application/model/converter/MemberPushTokenEntityConverter.java (1)
8-31: LGTM!엔티티와 모델 간의 양방향 변환이 올바르게 구현되었습니다. 빌더 패턴을 일관되게 사용하고 있으며, 필드명 차이(provider/notificationProvider)도 적절히 매핑되었습니다.
eeos/src/main/java/com/blackcompany/eeos/notification/application/model/MemberPushTokenModel.java (1)
9-18: 잘 구현되었습니다!모델 클래스가 프로젝트 컨벤션을 잘 따르고 있으며, Lombok 어노테이션이 적절하게 사용되었습니다.
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/JpaMemberPushTokenRepository.java (2)
14-19: 표준 쿼리 메서드가 올바르게 정의되었습니다.Spring Data JPA의 메서드 네이밍 규칙을 잘 따르고 있습니다.
21-23: 트랜잭션 처리는 이미 올바르게 구현되어 있습니다.
NotificationTokenService의deleteAllMemberPushTokens메서드(line 47)는 이미@Transactional어노테이션을 적용하고 있으며, 이 메서드 내에서deleteByMemberId를 호출합니다. 클래스 레벨의@Transactional(readOnly = true)설정을 메서드 레벨의@Transactional로 적절히 오버라이드하고 있으므로 추가 작업은 필요하지 않습니다.Likely an incorrect or invalid review comment.
eeos/src/main/java/com/blackcompany/eeos/notification/presentation/controller/MemberPushTokenController.java (2)
32-38: 토큰 생성 엔드포인트가 올바르게 구현되었습니다.REST 규칙을 잘 따르고 있으며, 적절한 상태 코드(201 Created)와 요청 검증이 적용되어 있습니다.
40-53: 삭제 엔드포인트들이 잘 설계되었습니다.단일 삭제와 전체 삭제 엔드포인트가 명확하게 분리되어 있으며, 멤버 인증이 적절하게 적용되어 있습니다.
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java (1)
20-27: 엔티티 구조가 프로젝트 컨벤션을 잘 따르고 있습니다.Lombok 어노테이션과 상속 구조가 적절하게 사용되었습니다.
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenRepositoryImpl.java (2)
22-38: 조회 메서드들이 올바르게 구현되었습니다.JPA 레포지토리로의 위임과 엔티티-모델 변환이 적절하게 처리되고 있습니다.
40-61: 변경 메서드들이 잘 구현되었습니다.레포지토리 위임과 엔티티-모델 변환 로직이 적절하며, 저장 메서드가 양방향 변환을 올바르게 처리하고 있습니다.
...in/java/com/blackcompany/eeos/notification/application/dto/CreateMemberPushTokenRequest.java
Outdated
Show resolved
Hide resolved
...in/java/com/blackcompany/eeos/notification/application/dto/DeleteMemberPushTokenRequest.java
Show resolved
Hide resolved
...com/blackcompany/eeos/notification/application/exception/DeniedDeletePushTokenException.java
Show resolved
Hide resolved
...ava/com/blackcompany/eeos/notification/application/exception/NotFoundPushTokenException.java
Show resolved
Hide resolved
...va/com/blackcompany/eeos/notification/application/respository/MemberPushTokenRepository.java
Outdated
Show resolved
Hide resolved
...com/blackcompany/eeos/notification/application/usecase/DeleteAllMemberPushTokensUsecase.java
Outdated
Show resolved
Hide resolved
...ava/com/blackcompany/eeos/notification/application/usecase/DeleteMemberPushTokenUsecase.java
Outdated
Show resolved
Hide resolved
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java
Show resolved
Hide resolved
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
eeos/src/main/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.java (1)
31-44: 중복 토큰 생성 가능성 (기존 리뷰 내용)이전 리뷰에서 이미 지적된 동시성 문제가 여전히 존재합니다:
- Line 34의
findByPushToken체크와 Line 42의save사이에 경쟁 조건이 발생할 수 있습니다- 데이터베이스 레벨의 unique constraint가 없어 중복 토큰이 생성될 수 있습니다
이전 리뷰의 제안사항을 참고하여 수정해주세요.
🧹 Nitpick comments (2)
eeos/src/main/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.java (2)
21-21: 미사용 @slf4j 어노테이션 제거 고려코드 내에서 log를 사용하지 않고 있습니다. 불필요한 의존성을 제거하는 것을 권장합니다.
🔎 제안하는 수정
-@Slf4j public class NotificationTokenService
46-50: 삭제된 토큰 개수 반환 고려현재 리포지토리의
deleteByMemberId는 삭제된 레코드 개수를 반환하지만, 서비스 메서드에서는 이를 활용하지 않습니다. 디버깅과 모니터링을 위해 삭제 개수를 반환하거나 로깅하는 것을 고려해보세요.🔎 제안하는 수정
@Override @Transactional -public void deleteAllMemberPushTokens(Long memberId) { - memberPushTokenRepository.deleteByMemberId(memberId); +public int deleteAllMemberPushTokens(Long memberId) { + return memberPushTokenRepository.deleteByMemberId(memberId); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
eeos/src/main/java/com/blackcompany/eeos/notification/application/dto/CreateMemberPushTokenRequest.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/exception/DeniedDeletePushTokenException.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/exception/NotFoundPushTokenException.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/repository/MemberPushTokenRepository.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/usecase/CreateMemberPushTokenUsecase.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/usecase/DeleteAllMemberPushTokensUsecase.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/usecase/DeleteMemberPushTokenUsecase.javaeeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.javaeeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenRepositoryImpl.java
🚧 Files skipped from review as they are similar to previous changes (3)
- eeos/src/main/java/com/blackcompany/eeos/notification/application/usecase/DeleteMemberPushTokenUsecase.java
- eeos/src/main/java/com/blackcompany/eeos/notification/application/exception/DeniedDeletePushTokenException.java
- eeos/src/main/java/com/blackcompany/eeos/notification/application/usecase/CreateMemberPushTokenUsecase.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-02-22T16:03:14.347Z
Learnt from: rlajm1203
Repo: JNU-econovation/EEOS-BE PR: 213
File: eeos/src/main/java/com/blackcompany/eeos/auth/presentation/interceptor/AuthInterceptor.java:33-36
Timestamp: 2025-02-22T16:03:14.347Z
Learning: When suggesting additional validation checks, first verify if the underlying components (like TokenExtractor and TokenResolver in auth interceptors) already implement those checks to avoid redundancy.
Applied to files:
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java
🧬 Code graph analysis (3)
eeos/src/main/java/com/blackcompany/eeos/notification/application/dto/CreateMemberPushTokenRequest.java (1)
eeos/src/main/java/com/blackcompany/eeos/notification/application/dto/DeleteMemberPushTokenRequest.java (1)
Getter(9-15)
eeos/src/main/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.java (2)
eeos/src/main/java/com/blackcompany/eeos/notification/application/exception/DeniedDeletePushTokenException.java (1)
DeniedDeletePushTokenException(6-17)eeos/src/main/java/com/blackcompany/eeos/notification/application/exception/NotFoundPushTokenException.java (1)
NotFoundPushTokenException(6-19)
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java (1)
eeos/src/main/java/com/blackcompany/eeos/notification/application/model/MemberPushTokenModel.java (1)
Getter(9-18)
⏰ 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 (8)
eeos/src/main/java/com/blackcompany/eeos/notification/application/dto/CreateMemberPushTokenRequest.java (1)
14-15: 이전 리뷰 피드백이 반영되어 있습니다!
pushToken과provider필드 모두@NotBlank를 사용하여 일관성 있는 검증을 적용하고 있습니다.DeleteMemberPushTokenRequest와 동일한 검증 전략을 사용하여 빈 문자열과 공백만 있는 문자열을 적절히 방어하고 있습니다.eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java (1)
43-44: 이전 리뷰 피드백들이 모두 반영되었습니다!
pushToken필드에 대해 제기된 두 가지 이슈가 모두 해결되었습니다:
- 유니크 제약 조건 추가:
unique = true속성이 추가되어 동시성 환경에서도 중복 토큰 저장을 데이터베이스 레벨에서 방지합니다.- 컬럼명 네이밍 규칙 통일:
"_push_token"으로 변경되어 다른 컬럼들과 일관되게 snake_case를 사용합니다.eeos/src/main/java/com/blackcompany/eeos/notification/application/usecase/DeleteAllMemberPushTokensUsecase.java (1)
3-5: 이전 리뷰 피드백이 반영되었습니다!인터페이스에서
@Component애노테이션이 제거되었습니다. 이제 Spring의 권장 사항에 따라 인터페이스는 순수한 계약 정의만 담고 있으며, 구현체에서@Service또는@Component로 빈 등록이 이루어지는 올바른 구조입니다.eeos/src/main/java/com/blackcompany/eeos/notification/application/exception/NotFoundPushTokenException.java (1)
10-18: 이전 보안 이슈가 해결되었습니다!민감한 푸시 토큰 값이 예외 메시지에서 제거되었습니다:
pushToken필드가 완전히 제거되어 민감 정보 저장을 방지합니다.getMessage()가 일반적인 메시지만 반환하여 로그나 모니터링 시스템에 토큰이 노출되지 않습니다.이는 중요한 보안 개선입니다.
eeos/src/main/java/com/blackcompany/eeos/notification/application/repository/MemberPushTokenRepository.java (1)
9-24: 잘 설계된 리포지토리 인터페이스입니다!메서드 시그니처가 적절하게 설계되어 있습니다:
- 조회 메서드들은
Optional과List를 적절히 사용하여 결과의 존재 여부를 명확히 표현합니다.- 삭제 메서드들은 영향받은 행 수(
int)를 반환하여 호출자가 작업 결과를 확인할 수 있습니다.deleteByUpdatedDateBefore메서드는 오래된 토큰 정리에 유용한 설계입니다.모든 메서드 명명이 Spring Data 규칙을 따르고 있으며, 도메인 로직을 명확히 표현하고 있습니다.
eeos/src/main/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.java (1)
52-63: 소유권 검증 로직이 올바르게 구현되었습니다토큰 삭제 전 소유권을 검증하는 보안 로직이 적절하게 구현되어 있습니다. Long 타입 비교에
.equals()를 사용한 것도 정확합니다.eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenRepositoryImpl.java (2)
14-38: 리포지토리 어댑터 패턴이 깔끔하게 구현되었습니다JPA 리포지토리와 애플리케이션 레이어 사이의 어댑터 구현이 일관성 있고 명확합니다. 엔티티/모델 변환 로직도 적절하게 분리되어 있습니다.
50-54: JPA 리포지토리 인터페이스가 명시적으로Timestamp타입을 요구합니다.JpaMemberPushTokenRepository.deleteByUpdatedDateBefore(Timestamp)의 메서드 시그니처를 보면 현재의LocalDateTime에서Timestamp로의 변환이 필요합니다. 공개 인터페이스에서 Java 8 시간 API를 사용하고 JPA 계층에서 JDBC Timestamp를 사용하는 것은 적절한 추상화 계층 패턴이므로 변환을 제거할 수 없습니다.
토큰 활동상태를 의미하는 lastActiveAt 필드를 추가했습니다.
토큰 중복의 경우 토큰 활동 시간을 의미하는 lastActiveAt 필드를 변경하는 로직으로 수정하였습니다.
There was a problem hiding this comment.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In
@eeos/src/main/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.java:
- Around line 57-66: The delete method has a TOCTOU race between validating
ownership via memberPushTokenRepository.findByPushToken(...) and calling
memberPushTokenRepository.deleteByPushToken(...); replace this two-step check
with a single conditional delete that uses both memberId and pushToken to
atomically remove the row: call a repository method like
deleteByMemberIdAndPushToken(memberId, request.getPushToken()) and check its
returned deleted count, throwing NotFoundPushTokenException when 0; remove the
separate ownership check and DeniedDeletePushTokenException branch so deletion
is safe against concurrent reassignments.
- Around line 30-47: The create method in NotificationTokenService currently
calls existingModel.renew(memberId) which silently transfers token ownership
when memberPushTokenRepository.findByPushToken(...) finds an existing token;
confirm intended behavior and either document/log it or prevent reassignment:
update NotificationTokenService.create to check the existing model's memberId
from memberPushTokenRepository.findByPushToken(request.getPushToken()) and if it
differs from the incoming memberId, either (A) log an audit message and proceed
with existingModel.renew(memberId) (and persist) if reassignment is intended, or
(B) throw a specific exception (e.g., IllegalStateException or a custom
TokenConflictException) to reject re-registration and return/do not call renew;
ensure you reference existingModel.renew,
CreateMemberPushTokenRequest.getPushToken/getProvider, and
memberPushTokenRepository.save when implementing the chosen path and add a brief
comment explaining the business decision.
In
@eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java:
- Around line 21-49: MemberPushTokenEntity defines a new table but no DB
migration exists, so add a Flyway/V1-style migration (e.g.
V1.00.0.2__add_member_push_token_table.sql) that creates table member_push_token
with columns matching the entity: member_push_token_id PK,
member_push_token_member_id (FK to members table), member_push_token_provider,
member_push_token_push_token (with UNIQUE constraint),
member_push_token_last_activate_at, plus the BaseEntity default columns
(created_date, updated_date, is_deleted); ensure column names use the
ENTITY_PREFIX convention from MemberPushTokenEntity and add the FK constraint
and indexes as appropriate so Hibernate validate mode succeeds on startup.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
eeos/src/main/java/com/blackcompany/eeos/notification/application/model/MemberPushTokenModel.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/model/converter/MemberPushTokenEntityConverter.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.javaeeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: kssumin
Repo: JNU-econovation/EEOS-BE PR: 218
File: eeos/src/main/java/com/blackcompany/eeos/target/presentation/controller/AttendWeightPolicyController.java:19-20
Timestamp: 2025-03-01T17:43:11.312Z
Learning: 사용자 kssumin은 토큰 기반의 ROLE 검증 기능을 별도 이슈로 해결할 예정이며, 관리자 권한 검증 관련 TODO 항목은 현재 PR에서 다루지 않기로 했습니다.
📚 Learning: 2025-02-22T16:03:14.347Z
Learnt from: rlajm1203
Repo: JNU-econovation/EEOS-BE PR: 213
File: eeos/src/main/java/com/blackcompany/eeos/auth/presentation/interceptor/AuthInterceptor.java:33-36
Timestamp: 2025-02-22T16:03:14.347Z
Learning: When suggesting additional validation checks, first verify if the underlying components (like TokenExtractor and TokenResolver in auth interceptors) already implement those checks to avoid redundancy.
Applied to files:
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java
📚 Learning: 2025-02-22T16:03:14.347Z
Learnt from: rlajm1203
Repo: JNU-econovation/EEOS-BE PR: 213
File: eeos/src/main/java/com/blackcompany/eeos/auth/presentation/interceptor/AuthInterceptor.java:33-36
Timestamp: 2025-02-22T16:03:14.347Z
Learning: When reviewing auth-related code in EEOS, check both TokenExtractor and TokenResolver implementations as they might already include necessary validation and error handling logic.
Applied to files:
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java
🧬 Code graph analysis (2)
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java (1)
eeos/src/main/java/com/blackcompany/eeos/notification/application/model/MemberPushTokenModel.java (1)
Getter(10-24)
eeos/src/main/java/com/blackcompany/eeos/notification/application/model/MemberPushTokenModel.java (1)
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java (1)
Getter(21-49)
⏰ 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)
eeos/src/main/java/com/blackcompany/eeos/notification/application/model/converter/MemberPushTokenEntityConverter.java (1)
8-33: LGTM!양방향 변환 로직이 정확하며, 필드 매핑이 일관성 있게 구현되어 있습니다. 프로젝트의 AbstractEntityConverter 패턴을 올바르게 따르고 있습니다.
eeos/src/main/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.java (1)
49-53: LGTM!사용자의 모든 토큰을 삭제하는 로직이 명확하게 구현되어 있습니다.
eeos/src/main/java/com/blackcompany/eeos/notification/application/model/MemberPushTokenModel.java (1)
10-24: LGTM!도메인 모델이 깔끔하게 구현되어 있습니다.
renew메서드는memberId를 업데이트하는데, 이는 NotificationTokenService에서 토큰 재할당에 사용되는 핵심 로직입니다.해당 재할당 로직의 의도에 대한 검토는 NotificationTokenService 리뷰 코멘트를 참고해주세요.
...in/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.java
Show resolved
Hide resolved
...in/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.java
Show resolved
Hide resolved
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java
Show resolved
Hide resolved
rlajm1203
approved these changes
Jan 5, 2026
Comment on lines
+14
to
+24
| public class MemberPushTokenModel implements AbstractModel { | ||
| private Long id; | ||
| private Long memberId; | ||
| private NotificationProvider notificationProvider; | ||
| private String pushToken; | ||
| private LocalDateTime lastActiveAt; | ||
|
|
||
| public MemberPushTokenModel renew(Long memberId) { | ||
| return this.toBuilder().memberId(memberId).lastActiveAt(LocalDateTime.now()).build(); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
private LocalDateTime lastActiveAt 은 왜 필요한가요?
Member
Author
There was a problem hiding this comment.
말이 길어졌습니다..ㅎ
요약 : 토큰 신선도 관리를 위해서 도입했고, 토큰 사용시마다 lastActiveAt 값을 현재시간으로 변경, 저장하기위해 도입했습니다!
알림 토큰의 경우 생성시 타임스탬프를 기록해서 신선도 관리가 필요합니다.
기존에 base entity의 update_date를 사용하려고 했었는데 동일한 토큰에 대해서 post 요청이 오는 상황이 있어요 (로그인 -> fcm에서 이전 발급한 토큰을 그대로 반환 -> db 에 저장된것과 동일한 토큰 post 요청 및 사용 날짜 기록)
이 경우에 마지막으로 사용한 날짜를 update_date 로 하면 jpa dirty checking 에 걸리지 않을 수도 있겠다 생각했습니다 (기존 저장된 토큰과 비교해서 변경사항이 없기 때문)
이때문에 lastActiveAt 이라는 필드를 만들게 되었고, 사용자가 로그인 후 기존 알림 토큰을 사용할때 lastActiveAt 값이 현재시간으로 업데이트되어 마지막으로 토큰을 사용한 시간을 기록하도록 구현했습니다!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📌 관련 이슈
closes #294
✒️ 작업 내용
스크린샷 🏞️ (선택)
💬 REVIEWER에게 요구사항 💬
https://www.notion.so/EEOS-2c2348dfe77f8076b16ae07418d1499b?source=copy_link#2d7348dfe77f80af86e3fe7b95c67a16
Summary by CodeRabbit
새로운 기능
보안
잡무(Chore)
✏️ Tip: You can customize this high-level summary in your review settings.