-
Notifications
You must be signed in to change notification settings - Fork 2
[Fix] 하루 요약 알림 전송되지 않는 문제 발생 해결 #242
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
Conversation
- 예약 알림이 TaskScheduler에 의해 전송될때 요약 일기가 생성되는데 이때 Security Context가 없어 문제 발생, TaskScheduler를 등록할때 userId를 같이 넘겨줘서 해결 - FCM에 알림 전달 후 누락되지 않도록 우선순위 부여
riadan710
left a comment
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.
빨리 적용해서 테스트 해봐야겠네요! 수고하셨습니다 👍
|
/gemini review |
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.
Code Review
하루 요약 알림이 전송되지 않는 문제를 해결하기 위한 변경 사항을 검토했습니다. 예약된 작업(Task)에서 SecurityContext가 없어 발생하는 문제를 해결하기 위해 userId를 직접 전달하는 접근 방식은 올바릅니다. 또한 FCM 메시지에 우선순위를 부여하여 알림 전달 신뢰도를 높인 점도 좋은 개선입니다. 하지만 리뷰 과정에서 몇 가지 중요한 수정이 필요한 부분을 발견했습니다. 알림 대상의 일기를 조회할 때 모든 사용자의 일기를 조회하는 심각한 버그가 있었고, 사용자를 찾지 못했을 때 NullPointerException이 발생할 수 있는 문제도 확인되었습니다. 또한, 코드 중복과 디버그용 출력문이 남아있는 부분도 개선이 필요합니다. 자세한 내용은 각 파일에 남긴 코멘트를 참고해주세요.
| List<Diaries> diaries = diaryRepository.findAllByCreatedAtBetween( | ||
| today.atStartOfDay(), // 오늘 00:00:00 | ||
| today.plusDays(1).atStartOfDay() // 내일 00:00:00 | ||
| ); |
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.
현재 코드는 특정 사용자가 아닌 모든 사용자의 하루치 일기를 가져오고 있습니다. diaryRepository.findAllByCreatedAtBetween 메소드는 사용자 ID로 필터링하지 않기 때문에, 다른 사용자가 작성한 일기 수에 따라 알림 조건이 잘못 판단될 수 있습니다.
예를 들어, A 사용자가 일기를 2개 이상 작성하면 B 사용자가 일기를 하나도 작성하지 않았더라도 B 사용자에게 요약 알림 생성 로직이 트리거될 수 있습니다.
user 객체를 사용하여 특정 사용자의 일기만 가져오도록 수정해야 합니다. SummaryCommandServiceImpl에서 사용된 diaryRepository.findAllByUserIdAndDate 메소드를 사용하는 것을 고려해볼 수 있습니다. 이렇게 하면 다른 서비스 로직과 일관성도 유지할 수 있습니다.
List<Diaries> diaries = diaryRepository.findAllByUserIdAndDate(
user,
today.atStartOfDay(),
today.atTime(LocalTime.MAX)
);|
|
||
| @Override | ||
| public SummaryDiary summarizeByDailySummaryAlarm(Long userId, DiarySummaryRequestDTO.SummarizeDTO request) { | ||
| Users user = userRepository.findById(userId).orElse(null); |
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.
userRepository.findById(userId).orElse(null)을 사용하여 사용자를 조회하고 있습니다. 만약 userId에 해당하는 사용자가 존재하지 않을 경우 user 변수는 null이 됩니다. 이 null 값은 이후 diaryRepository.findAllByUserIdAndDate(user, ...) 호출 시 NullPointerException을 발생시킬 수 있습니다.
다른 서비스에서처럼 orElseThrow를 사용하여 사용자를 찾지 못한 경우 예외를 발생시키는 것이 더 안전합니다.
| Users user = userRepository.findById(userId).orElse(null); | |
| Users user = userRepository.findById(userId).orElseThrow(() -> new ExceptionHandler(ErrorStatus.USER_NOT_FOUND)); |
| } | ||
| if (getDailySummaryAlarm.getAlarmTime().isAfter(LocalTime.now())) { // 현재 시간보다 이전 알림 시간은 예약하지 않음 | ||
| LocalDateTime ALARM_TIME = getAlarmLocalDateTime(getDailySummaryAlarm.getAlarmTime()); | ||
| System.out.println("하루 요약 알림 예약!!!"); |
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.
| @Override | ||
| public SummaryDiary summarizeByDailySummaryAlarm(Long userId, DiarySummaryRequestDTO.SummarizeDTO request) { | ||
| Users user = userRepository.findById(userId).orElse(null); | ||
| LocalDate day = request.getDate(); | ||
|
|
||
| LocalDateTime todayStart = day.atStartOfDay(); | ||
| LocalDateTime todayEnd = day.atTime(LocalTime.MAX); | ||
|
|
||
| List<Diaries> diaries = diaryRepository.findAllByUserIdAndDate(user, todayStart, todayEnd); | ||
| String prompt = PromptBuilder.buildPrompt(diaries); | ||
|
|
||
| ChatGPTRequestDTO gptRequest = new ChatGPTRequestDTO(model, prompt); | ||
| ChatGPTResponseDTO responseDTO = restTemplate.postForObject(apiUrl, gptRequest, ChatGPTResponseDTO.class); | ||
|
|
||
| String content = responseDTO.getChoices().get(0).getMessage().getContent(); | ||
|
|
||
| String rawKey = diaries.stream() | ||
| .map(d -> d.getId() + ":" + d.getUpdatedAt().toString()) | ||
| .sorted() | ||
| .collect(Collectors.joining(",")); | ||
|
|
||
| String diaryKeyHash = generateSHA256(rawKey); | ||
|
|
||
| SummaryDiary summaryDiary = SummaryDiary.builder() | ||
| .date(day) | ||
| .content(content) | ||
| .users(user) | ||
| .diaryKeyHash(diaryKeyHash) | ||
| .build(); | ||
|
|
||
| summaryDiaryRepository.save(summaryDiary); | ||
| return summaryDiary; | ||
| } |
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.
새로 추가된 summarizeByDailySummaryAlarm 메소드는 기존의 summarize 메소드와 거의 동일한 로직을 가지고 있습니다. 코드 중복은 유지보수성을 저하시킬 수 있습니다.
공통 로직을 별도의 private 메소드로 추출하여 중복을 제거하는 리팩토링을 제안합니다. 예를 들어, Users 객체와 LocalDate를 인자로 받아 SummaryDiary를 생성하고 반환하는 private 메소드를 만들 수 있습니다.
@Override
public SummaryDiary summarize(DiarySummaryRequestDTO.SummarizeDTO request) {
Long userId = SecurityUtil.getCurrentUserId();
Users user = userRepository.findById(userId)
.orElseThrow(() -> new ExceptionHandler(ErrorStatus.USER_NOT_FOUND));
return createSummaryForUser(user, request.getDate());
}
@Override
public SummaryDiary summarizeByDailySummaryAlarm(Long userId, DiarySummaryRequestDTO.SummarizeDTO request) {
Users user = userRepository.findById(userId)
.orElseThrow(() -> new ExceptionHandler(ErrorStatus.USER_NOT_FOUND));
return createSummaryForUser(user, request.getDate());
}
private SummaryDiary createSummaryForUser(Users user, LocalDate day) {
// ... 공통 요약 생성 로직 ...
}이렇게 하면 코드의 재사용성이 높아지고 유지보수가 용이해집니다.
…dAtBetween)에서 사용자도 조건에 추가되도록 변경 / 1분마다 실행되는 processDailySummaryDiary메소드(하루 일기 요약) 주석처리 - diaryRepository.findAllByCreatedAtBetween 메소드를 사용자도 조건에 추가되도록 diaryRepository.findAllByUsersAndCreatedAtBetween로 변경
#️⃣연관된 이슈
📝작업 내용
🔎코드 설명
💬고민사항 및 리뷰 요구사항 (Optional)
비고 (Optional)