-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BE] feat: 새로운 FCM 등록 기능 추가 (#998) #1002
base: dev
Are you sure you want to change the base?
Conversation
- it -> memberFCM
@Service | ||
@Transactional | ||
@RequiredArgsConstructor | ||
public class MemberFCMCommandService { | ||
|
||
private final MemberFCMRepository memberFCMRepository; | ||
private final MemberFCMExpiredAtPolicy memberFCMExpiredAtPolicy; | ||
private final MemberFCMRemoveOldTokensPolicy memberFCMDeleteOldTokensPolicy; | ||
|
||
// TODO 별도의 Service 클래스 생성해서 비즈니스 로직 이관 생각해 볼 것 | ||
public void registerFCM(Long memberId, String fcmToken) { | ||
List<MemberFCM> memberFCMs = memberFCMRepository.findAllByMemberId(memberId); | ||
LocalDateTime expiredAt = memberFCMExpiredAtPolicy.provide(); | ||
memberFCMs.stream() | ||
.filter(memberFCM -> memberFCM.isSameToken(fcmToken)) | ||
.findAny() | ||
.ifPresentOrElse(memberFCM -> { | ||
memberFCM.changeExpiredAt(expiredAt); | ||
}, () -> { | ||
memberFCMDeleteOldTokensPolicy.delete(memberFCMs); | ||
memberFCMRepository.save(new MemberFCM(memberId, fcmToken, expiredAt)); | ||
}); | ||
} | ||
|
||
public void deleteAllMemberFCM(Long memberId) { | ||
memberFCMRepository.deleteAllByMemberId(memberId); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FCM을 등록하는 메서드가 여러 정책에 의해 부피가 조금 큽니다. 😂
따라서 별도의 서비스 클래스로 나눠야 할 것 같기도 하네요.
Test Results246 files 246 suites 28s ⏱️ Results for commit 83d62b3. ♻️ This comment has been updated with latest results. |
@RequiredArgsConstructor | ||
public class MemberFCMExpiredAtPolicyImpl implements MemberFCMExpiredAtPolicy { | ||
|
||
private static final int EXPIRED_DAY_OFFSET = 180; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
자문자답인 것 같긴 한데..
보통 2학기에 축제가 9월에 열리니.. 내년 5월에 알림을 받으려면 1년 정도로 하는게 좋을 것 같네요. 😂
@Component | ||
@RequiredArgsConstructor | ||
public class MemberFCMRemoveOldTokensPolicy { | ||
|
||
private final MemberFCMRepository memberFCMRepository; | ||
|
||
public void delete(List<MemberFCM> memberFCMs) { | ||
memberFCMRepository.deleteByIn(memberFCMs); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
N개의 FCM 토큰을 허용한다면 아마 다음과 같이 로직을 변경할 수 있을 것 같네요.
var oldTokens = memberFCMs.stream()
.sorted(comparing(MemberFCM::getExpiredAt).reversed()) // 만료 시간 내림차순
.skip(n - 1) // 최대 소지 개수 - 1 개를 남기고 모두 삭제
.toList();
memberFCMRepository.deleteByIn(oldTokens);
@Test | ||
void 기존_토큰을_삭제하고_새로운_FCM_토큰을_등록한다() { | ||
// given | ||
given(memberFCMExpiredAtPolicy.provide()) | ||
.willReturn(LocalDateTime.parse("2077-06-30T18:00:00")); | ||
|
||
// when | ||
memberFCMCommandService.registerFCM(memberId, fcmToken); | ||
|
||
// then | ||
List<MemberFCM> fcmTokens = memberFCMRepository.findAllByMemberId(memberId); | ||
assertThat(fcmTokens) | ||
.map(MemberFCM::getFcmToken) | ||
.containsOnly(fcmToken); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
삭제 정책이 별도의 클래스로 분리가 되었기에.. 그저 새로운 토큰이 추가되었는지 검사만 하면 될 것 같기도 하네요.
assertThat(fcmTokens)
.map(MemberFCM::getFcmToken)
.contains(fcmToken);
📌 관련 이슈
✨ PR 세부 내용
이슈 내용 그대로 FCM을 등록하는 기능을 새롭게 추가했습니다.
FCM 토큰에
expiredAt
필드가 생겼기 때문에 DB 스키마 또한 변경되었습니다.이슈에 남겼던 FCM 등록 프로세스인데,
MemberFCMCommandServiceTest
는 해당 명세에 따라 작성되었습니다.만료 시간과 오래된 토큰의 삭제 정책 같이 변경이 될 수 있는 정책에 대해서는 별도의 클래스로 분리하여 사용하도록 했습니다.
(따라서 패키지 위치가 domain 입니다)
그 외 사항에 대해선 코드에 리뷰로 남기겠습니다!