-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: 애플리케이션 재시작 시 PENDING 상태 알림 스케줄링 및 새벽 4시마다 지난 모임 삭제 기능 추가 #410
Changes from all commits
358528e
2ae437b
2b7a5a5
0ecba42
7329707
d1f6e0d
e2bae23
604ec35
bc45cff
3f4498b
c76171b
e779dc0
69d3195
185fae5
8ffcad9
70b61b0
51ddc35
1725cbf
15d7d0d
72f86c5
9ae47eb
eed3d75
69fb48a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
package com.ody.common.config; | ||
|
||
import java.time.Clock; | ||
import java.time.ZoneId; | ||
import lombok.extern.slf4j.Slf4j; | ||
import org.springframework.context.annotation.Bean; | ||
import org.springframework.context.annotation.Configuration; | ||
|
||
@Slf4j | ||
@Configuration | ||
public class ClockConfig { | ||
|
||
@Bean | ||
public Clock clock() { | ||
String zoneId = "Asia/Seoul"; | ||
log.info("시스템 타임 존 설정 : {}", zoneId); | ||
return Clock.system(ZoneId.of(zoneId)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,7 +2,9 @@ | |||||||||||||||||
|
||||||||||||||||||
import com.ody.meeting.domain.Meeting; | ||||||||||||||||||
import java.util.List; | ||||||||||||||||||
import java.util.Optional; | ||||||||||||||||||
import org.springframework.data.jpa.repository.JpaRepository; | ||||||||||||||||||
import org.springframework.data.jpa.repository.Modifying; | ||||||||||||||||||
import org.springframework.data.jpa.repository.Query; | ||||||||||||||||||
|
||||||||||||||||||
public interface MeetingRepository extends JpaRepository<Meeting, Long> { | ||||||||||||||||||
|
@@ -12,7 +14,17 @@ public interface MeetingRepository extends JpaRepository<Meeting, Long> { | |||||||||||||||||
from Meeting meeting | ||||||||||||||||||
right join Mate mate on meeting.id = mate.meeting.id | ||||||||||||||||||
where mate.member.id = :memberId | ||||||||||||||||||
and meeting.overdue = false | ||||||||||||||||||
""" | ||||||||||||||||||
) | ||||||||||||||||||
List<Meeting> findAllByMemberId(Long memberId); | ||||||||||||||||||
Comment on lines
+17
to
20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [제안] 헷갈릴 수 있을 것 같은데, 메서드명을 바꿔주면 어떤가요? NotOverdue 키워드가 들어가면 좋을 것 같아요
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 헷갈릴 수 있을 것 같다했는데 |
||||||||||||||||||
|
||||||||||||||||||
@Modifying(clearAutomatically = true) | ||||||||||||||||||
@Query("update Meeting m set m.overdue = true where m.overdue = false and m.date < CURRENT_DATE") | ||||||||||||||||||
void updateAllByNotOverdueMeetings(); | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [의견] 현재 새벽 4시마다 하고 있는 작업은
이렇게 2가지로 이해했어요! update 쿼리도 좋지만 더티 체킹을 최대한 활용해보는 것도 좋을 것 같아요. 서비스 로직이 레포지토리에 들어간 느낌을 받을 수 있다고도 생각해서요! 레포지토리 단에서는 삭제대상인 meeting들 만을 가져오고 삭제 + 구독 해지 작업을 애플리 케이션 단에서 해주는 건 어떨까요? 카키의 생각이 궁금합니다!! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오디톡 설명 완 ! |
||||||||||||||||||
|
||||||||||||||||||
@Query("select m from Meeting m where m.overdue = true and CAST(m.updatedAt AS LOCALDATE) = CURRENT_DATE") | ||||||||||||||||||
List<Meeting> findAllByUpdatedTodayAndOverdue(); | ||||||||||||||||||
|
||||||||||||||||||
Optional<Meeting> findByIdAndOverdueFalse(Long id); | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [제안] 네이밍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 쿼리메소드라서 안 될것같아요🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 쿼리 네이밍 메소드를 사용하기 위해서 위 처럼 작성했습니다 ! |
||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,16 +15,20 @@ | |
import com.ody.meeting.dto.response.MeetingWithMatesResponse; | ||
import com.ody.meeting.repository.MeetingRepository; | ||
import com.ody.member.domain.Member; | ||
import com.ody.notification.service.NotificationService; | ||
import com.ody.util.InviteCodeGenerator; | ||
import com.ody.util.TimeUtil; | ||
import java.time.LocalDateTime; | ||
import java.util.Comparator; | ||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
import lombok.RequiredArgsConstructor; | ||
import lombok.extern.slf4j.Slf4j; | ||
import org.springframework.scheduling.annotation.Scheduled; | ||
import org.springframework.stereotype.Service; | ||
import org.springframework.transaction.annotation.Transactional; | ||
|
||
@Slf4j | ||
@Service | ||
@RequiredArgsConstructor | ||
@Transactional(readOnly = true) | ||
|
@@ -35,6 +39,7 @@ public class MeetingService { | |
private final MateService mateService; | ||
private final MeetingRepository meetingRepository; | ||
private final MateRepository mateRepository; | ||
private final NotificationService notificationService; | ||
|
||
@Transactional | ||
public MeetingSaveResponseV1 saveV1(MeetingSaveRequestV1 meetingSaveRequestV1) { | ||
|
@@ -58,7 +63,7 @@ public Meeting findByInviteCode(String inviteCode) { | |
} | ||
|
||
public Meeting findById(Long meetingId) { | ||
return meetingRepository.findById(meetingId) | ||
return meetingRepository.findByIdAndOverdueFalse(meetingId) | ||
.orElseThrow(() -> new OdyNotFoundException("존재하지 않는 모임입니다.")); | ||
} | ||
|
||
|
@@ -75,7 +80,7 @@ public MeetingFindByMemberResponses findAllByMember(Member member) { | |
private MeetingFindByMemberResponse makeMeetingFindByMemberResponse(Member member, Meeting meeting) { | ||
int mateCount = mateRepository.countByMeetingId(meeting.getId()); | ||
Mate mate = mateRepository.findByMeetingIdAndMemberId(meeting.getId(), member.getId()) | ||
.orElseThrow(() -> new OdyNotFoundException("참여하고 있지 않는 약속방입니다")); | ||
.orElseThrow(() -> new OdyNotFoundException("참여하고 있지 않는 약속입니다")); | ||
return MeetingFindByMemberResponse.of(meeting, mateCount, mate); | ||
} | ||
|
||
|
@@ -93,4 +98,12 @@ public MateSaveResponseV2 saveMateAndSendNotifications(MateSaveRequestV2 mateSav | |
} | ||
return mateService.saveAndSendNotifications(mateSaveRequest, member, meeting); | ||
} | ||
|
||
@Scheduled(cron = "0 0 4 * * *", zone = "Asia/Seoul") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [질문] 스케쥴드 애너테이션은 처음 활용해보아서 그런데 cron 안에 들어있는 건 4시를 나타낸다고 보아도 좋을까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 넵 순서대로 다음과 같아요 *은 모든 값을 나타내므로, 해석하면 매 년, 매 요일, 매 월, 매 일 , 04:00:00에 스케줄링을 하는 것 입니다 ! |
||
public void scheduleOverdueMeetings() { | ||
meetingRepository.updateAllByNotOverdueMeetings(); | ||
List<Meeting> meetings = meetingRepository.findAllByUpdatedTodayAndOverdue(); | ||
log.info("약속 시간이 지난 약속들 overdue = true로 update 쿼리 실행"); | ||
notificationService.unSubscribeTopic(meetings); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,7 @@ | ||
package com.ody.notification.dto.request; | ||
|
||
import com.ody.notification.domain.FcmTopic; | ||
import com.ody.notification.domain.Notification; | ||
|
||
public record FcmSendRequest(FcmTopic fcmTopic, Notification notification) { | ||
public record FcmSendRequest(Notification notification) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. notification이 가져도 되는 정보라 생각해요! 좋은 리팩터링이네요 👍 |
||
|
||
} |
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.
[질문]
Notification에도 updatedAt 필드가 생겼네요.
의도한 동작이었을까요?
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.
BaseEntity를 상속받는 상황에서 어쩔수 없는 상황이라 생각했고
불필요한 정보도 아니라고 생각했습니다 😄
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.
[질문]
BaseEntity에서 updateAt이나 createAt은 Eta 필드로도 있는데요. 필드명이 같을 경우에 updateAt 반영이 하위 클래스의 같은 필드명인 updateAt에는 영향을 주지 않는지 궁금해요! 만약 영향을 주게 된다면 isMissing true 처리 같은 로직에 영향을 받을 수 있다고 생각이 되어서요!
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.
Eta
는BaseEntity
를 상속받고 있지 않아 영향이 없을 것 같습니다 !다만 필드명이 동일해서 헷갈리긴 할 것 같네요
혼동을 방지하기 위해서 데이터 생성, 변경 시간 측정을 위해 사용한
BaseEntity
의 필드명을 변경하는 것 보다는로직으로 활용하고 있는
Eta
의 필드 이름을 바꿔주는게 좋을 것 같은데 어떻게 생각하시나요 ?