Skip to content
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

Merged
merged 23 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
358528e
feat: 시스템 타임 존 설정 추가
hyeon0208 Aug 17, 2024
2ae437b
feat: 알림 테이블에 fcmTopic 컬럼 추가
hyeon0208 Aug 17, 2024
2b7a5a5
feat: 애플리케이션 시작 시 PENDING 상태 알림 스케줄링 적용
hyeon0208 Aug 17, 2024
0ecba42
feat: 지난 약속방 논리 삭제 스케줄링 추가
hyeon0208 Aug 17, 2024
7329707
test: 접근제어자 protected로 변경 및 getter 제거
hyeon0208 Aug 17, 2024
d1f6e0d
feat: 오늘 약속의 기한이 지난 약속 리스트 조회 기능 추가 및 이벤트 발행 기능 추가
hyeon0208 Aug 17, 2024
e2bae23
feat: fcm topic 구독 해제 기능 추가
hyeon0208 Aug 17, 2024
604ec35
test: 테스트 설명 추가
hyeon0208 Aug 18, 2024
bc45cff
refactor: Device Token getter 디미터 법칙 적용
hyeon0208 Aug 18, 2024
3f4498b
test: Base 추상 클래스 접근 제어자 수정
hyeon0208 Aug 18, 2024
c76171b
refactor: 개행 적용
hyeon0208 Aug 18, 2024
e779dc0
style: 벌크 쿼리 메서드명 수정
hyeon0208 Aug 18, 2024
69d3195
feat: 약속 참여 시간이 지난 약속방 참여 검증 추가
hyeon0208 Sep 7, 2024
185fae5
refactor: 새벽 4시 스케줄링 코드 이벤트 리스너 제거 및 트랜잭션 제거
hyeon0208 Sep 7, 2024
8ffcad9
Merge branch 'develop' into feature/395
hyeon0208 Sep 7, 2024
70b61b0
refactor: 머지 충돌 작업 해결
hyeon0208 Sep 7, 2024
51ddc35
refactor: 조회 메서드에 약속 기간이 지나지 않은 조건 추가
hyeon0208 Sep 7, 2024
1725cbf
feat: 기간이 지나지 않은 약속 단건 조회 메서드 추가
hyeon0208 Sep 7, 2024
15d7d0d
refactor: findFetchedMateById() 메서드 사용 시 약속 기한이 지난 약속 처리 로직을 service에…
hyeon0208 Sep 9, 2024
72f86c5
test: 기한이 지난 약속 조회 테스트 어떤 약속인지 명확하게 변수명 네이밍
hyeon0208 Sep 9, 2024
9ae47eb
style: 약속방 -> 약속으로 텍스트 변경
hyeon0208 Sep 9, 2024
eed3d75
style: 기간이 지나지 않은 약속방 전체 Mate 조회 메서드 네이밍 수정
hyeon0208 Sep 9, 2024
69fb48a
test: 2가지 검증 구문을 assertAll로 래핑
hyeon0208 Sep 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions backend/src/main/java/com/ody/common/config/ClockConfig.java
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));
}
}
7 changes: 6 additions & 1 deletion backend/src/main/java/com/ody/common/domain/BaseEntity.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,20 @@
import java.time.LocalDateTime;
import lombok.Getter;
import org.springframework.data.annotation.CreatedDate;
import org.springframework.data.annotation.LastModifiedDate;
import org.springframework.data.jpa.domain.support.AuditingEntityListener;

@MappedSuperclass
@EntityListeners(AuditingEntityListener.class)
@Getter
public class BaseEntity {

@CreatedDate
@Column(updatable = false)
@NotNull
@CreatedDate
private LocalDateTime createdAt;

@NotNull
@LastModifiedDate
private LocalDateTime updatedAt;
Comment on lines +23 to +25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[질문]

Notification에도 updatedAt 필드가 생겼네요.
의도한 동작이었을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BaseEntity를 상속받는 상황에서 어쩔수 없는 상황이라 생각했고
불필요한 정보도 아니라고 생각했습니다 😄

Copy link
Contributor

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 처리 같은 로직에 영향을 받을 수 있다고 생각이 되어서요!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EtaBaseEntity를 상속받고 있지 않아 영향이 없을 것 같습니다 !
다만 필드명이 동일해서 헷갈리긴 할 것 같네요
혼동을 방지하기 위해서 데이터 생성, 변경 시간 측정을 위해 사용한 BaseEntity의 필드명을 변경하는 것 보다는
로직으로 활용하고 있는 Eta의 필드 이름을 바꿔주는게 좋을 것 같은데 어떻게 생각하시나요 ?

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ public interface MateRepository extends JpaRepository<Mate, Long> {
select mate
from Mate mate
join fetch mate.member
where mate.meeting.id = :meetingId
where mate.meeting.id = :meetingId
and mate.meeting.overdue = false
""")
List<Mate> findAllByMeetingId(Long meetingId);
List<Mate> findAllByOverdueFalseMeetingId(Long meetingId);

@Query("""
select mate
Expand Down
22 changes: 12 additions & 10 deletions backend/src/main/java/com/ody/mate/service/MateService.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,10 @@
import com.ody.eta.dto.request.MateEtaRequest;
import com.ody.eta.service.EtaService;
import com.ody.mate.domain.Mate;
import com.ody.mate.dto.request.MateSaveRequest;
import com.ody.mate.dto.request.MateSaveRequestV2;
import com.ody.mate.dto.request.NudgeRequest;
import com.ody.mate.dto.response.MateSaveResponse;
import com.ody.mate.dto.response.MateSaveResponseV2;
import com.ody.mate.repository.MateRepository;
import com.ody.meeting.domain.Coordinates;
import com.ody.meeting.domain.Meeting;
import com.ody.meeting.dto.response.MateEtaResponsesV2;
import com.ody.member.domain.Member;
Expand All @@ -34,15 +31,16 @@ public class MateService {
private final NotificationService notificationService;
private final RouteService routeService;


@Transactional
public MateSaveResponseV2 saveAndSendNotifications(
MateSaveRequestV2 mateSaveRequest,
Member member,
Meeting meeting
) {
public MateSaveResponseV2 saveAndSendNotifications(MateSaveRequestV2 mateSaveRequest, Member member, Meeting meeting) {
if (mateRepository.existsByMeetingIdAndMemberId(meeting.getId(), member.getId())) {
throw new OdyBadRequestException("약속에 이미 참여한 회원입니다.");
}
if (meeting.isOverdue()) {
throw new OdyBadRequestException("참여 가능한 시간이 지난 약속에 참여할 수 없습니다.");
}

Mate mate = saveMateAndEta(mateSaveRequest, member, meeting);
notificationService.saveAndSendNotifications(meeting, mate, member.getDeviceToken());
return MateSaveResponseV2.from(meeting);
Expand All @@ -60,7 +58,7 @@ private Mate saveMateAndEta(MateSaveRequestV2 mateSaveRequest, Member member, Me

public List<Mate> findAllByMeetingIdIfMate(Member member, long meetingId) {
findByMeetingIdAndMemberId(meetingId, member.getId());
return mateRepository.findAllByMeetingId(meetingId);
return mateRepository.findAllByOverdueFalseMeetingId(meetingId);
}

@Transactional
Expand All @@ -76,8 +74,12 @@ public void nudge(NudgeRequest nudgeRequest) {
}

private Mate findFetchedMate(Long mateId) {
return mateRepository.findFetchedMateById(mateId)
Mate mate = mateRepository.findFetchedMateById(mateId)
.orElseThrow(() -> new OdyBadRequestException("존재하지 않는 약속 참여자입니다."));
if (mate.getMeeting().isOverdue()) {
throw new OdyBadRequestException("기한이 지난 약속입니다.");
}
return mate;
}

private boolean canNudge(Mate mate) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public interface MeetingControllerSwagger {
),
@ApiResponse(
responseCode = "404",
description = "존재하지 않은 약속방이거나 약속방 일원이 아닌 경우",
description = "존재하지 않은 약속이거나 약속 일원이 아닌 경우",
content = @Content(schema = @Schema(implementation = ProblemDetail.class))
)
}
Expand Down
7 changes: 5 additions & 2 deletions backend/src/main/java/com/ody/meeting/domain/Meeting.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@

@Entity
@Getter
@AllArgsConstructor
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@AllArgsConstructor
public class Meeting extends BaseEntity {

@Id
Expand All @@ -42,8 +42,11 @@ public class Meeting extends BaseEntity {
@NotNull
private String inviteCode;

@NotNull
private boolean overdue;

public Meeting(String name, LocalDate date, LocalTime time, Location target, String inviteCode) {
this(null, name, date, TimeUtil.trimSecondsAndNanos(time), target, inviteCode);
this(null, name, date, TimeUtil.trimSecondsAndNanos(time), target, inviteCode, false);
}

public void updateInviteCode(String inviteCode) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[제안]

헷갈릴 수 있을 것 같은데, 메서드명을 바꿔주면 어떤가요? NotOverdue 키워드가 들어가면 좋을 것 같아요

Suggested change
and meeting.overdue = false
"""
)
List<Meeting> findAllByMemberId(Long memberId);
and meeting.overdue = false
"""
)
List<Meeting> findAllNotOverdueByMemberId(Long memberId);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

헷갈릴 수 있을 것 같다했는데
메서드를 만든 저부터 헷갈려서 Overdue가 아님에도 부정어를 붙이지 않았었네요 😅 반영완료 !


@Modifying(clearAutomatically = true)
@Query("update Meeting m set m.overdue = true where m.overdue = false and m.date < CURRENT_DATE")
void updateAllByNotOverdueMeetings();
Copy link
Contributor

@coli-geonwoo coli-geonwoo Aug 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[의견]

현재 새벽 4시마다 하고 있는 작업은

  1. 논리 삭제
  2. 논리 삭제 된 meeting의 구독 삭제

이렇게 2가지로 이해했어요!

update 쿼리도 좋지만 더티 체킹을 최대한 활용해보는 것도 좋을 것 같아요. 서비스 로직이 레포지토리에 들어간 느낌을 받을 수 있다고도 생각해서요!

레포지토리 단에서는 삭제대상인 meeting들 만을 가져오고 삭제 + 구독 해지 작업을 애플리 케이션 단에서 해주는 건 어떨까요?

카키의 생각이 궁금합니다!!

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[제안] 네이밍 findNotOverdueById로 하는 거 어떤가요?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

쿼리메소드라서 안 될것같아요🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

쿼리 네이밍 메소드를 사용하기 위해서 위 처럼 작성했습니다 !
근데 확실히 네이밍 쿼리가 가독성이 그리 좋지 않은 것 같긴해요 😂

}
17 changes: 15 additions & 2 deletions backend/src/main/java/com/ody/meeting/service/MeetingService.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand All @@ -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("존재하지 않는 모임입니다."));
}

Expand All @@ -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);
}

Expand All @@ -93,4 +98,12 @@ public MateSaveResponseV2 saveMateAndSendNotifications(MateSaveRequestV2 mateSav
}
return mateService.saveAndSendNotifications(mateSaveRequest, member, meeting);
}

@Scheduled(cron = "0 0 4 * * *", zone = "Asia/Seoul")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[질문]

스케쥴드 애너테이션은 처음 활용해보아서 그런데 cron 안에 들어있는 건 4시를 나타낸다고 보아도 좋을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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,11 +1,18 @@
package com.ody.notification.domain;

import com.ody.meeting.domain.Meeting;
import jakarta.persistence.Column;
import jakarta.persistence.Embeddable;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@Embeddable
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class FcmTopic {

@Column(name = "fcm_topic")
private String value;

public FcmTopic(Meeting meeting) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import com.ody.common.domain.BaseEntity;
import com.ody.mate.domain.Mate;
import com.ody.member.domain.DeviceToken;
import jakarta.persistence.Embedded;
import jakarta.persistence.Entity;
import jakarta.persistence.EnumType;
import jakarta.persistence.Enumerated;
Expand Down Expand Up @@ -44,23 +46,46 @@ public class Notification extends BaseEntity {
@NotNull
private NotificationStatus status;

public Notification(Mate mate, NotificationType type, LocalDateTime sendAt, NotificationStatus status) {
this(null, mate, type, sendAt, status);
@Embedded
private FcmTopic fcmTopic;

public Notification(
Mate mate,
NotificationType type,
LocalDateTime sendAt,
NotificationStatus status,
FcmTopic fcmTopic
) {
this(null, mate, type, sendAt, status, fcmTopic);
}

public static Notification createEntry(Mate mate) {
return new Notification(mate, NotificationType.ENTRY, LocalDateTime.now(), NotificationStatus.PENDING);
return new Notification(mate, NotificationType.ENTRY, LocalDateTime.now(), NotificationStatus.PENDING, null);
}

public static Notification createDepartureReminder(Mate mate, LocalDateTime sendAt) {
return new Notification(mate, NotificationType.DEPARTURE_REMINDER, sendAt, NotificationStatus.PENDING);
public static Notification createDepartureReminder(Mate mate, LocalDateTime sendAt, FcmTopic fcmTopic) {
return new Notification(
mate,
NotificationType.DEPARTURE_REMINDER,
sendAt,
NotificationStatus.PENDING,
fcmTopic
);
}

public static Notification createNudge(Mate mate) {
return new Notification(mate, NotificationType.NUDGE, LocalDateTime.now(), NotificationStatus.PENDING);
return new Notification(mate, NotificationType.NUDGE, LocalDateTime.now(), NotificationStatus.PENDING, null);
}

public void updateStatusToDone() {
this.status = NotificationStatus.DONE;
}

public String getFcmTopicValue() {
return fcmTopic.getValue();
}

public DeviceToken getMateDeviceToken() {
return mate.getMemberDeviceToken();
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.ody.notification.domain.message;

import com.google.firebase.messaging.Message;
import com.ody.notification.domain.FcmTopic;
import com.ody.notification.domain.Notification;
import lombok.Getter;

Expand All @@ -10,11 +9,11 @@ public class PushMessage {

private final Message message;

public PushMessage(FcmTopic topic, Notification notification) {
public PushMessage(Notification notification) {
this.message = Message.builder()
.putData("type", notification.getType().toString())
.putData("nickname", notification.getMate().getNicknameValue())
.setTopic(topic.getValue())
.setTopic(notification.getFcmTopicValue())
.build();
}
}
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notification이 가져도 되는 정보라 생각해요! 좋은 리팩터링이네요 👍


}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.ody.notification.repository;

import com.ody.notification.domain.Notification;
import com.ody.notification.domain.NotificationStatus;
import com.ody.notification.domain.NotificationType;
import java.util.List;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
Expand All @@ -16,4 +18,15 @@ public interface NotificationRepository extends JpaRepository<Notification, Long
order by noti.sendAt asc
""")
List<Notification> findAllMeetingLogs(Long meetingId);

List<Notification> findAllByTypeAndStatus(NotificationType type, NotificationStatus status);

@Query("""
select noti
from Notification noti
join fetch Mate mate on noti.mate.id = mate.id and mate.meeting.id = :meetingId
join fetch Member member on mate.member.id = member.id
where noti.type = :type
""")
List<Notification> findAllMeetingIdAndType(Long meetingId, NotificationType type);
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public class FcmPushSender {
@Transactional
public void sendPushNotification(FcmSendRequest fcmSendRequest) {
Notification notification = fcmSendRequest.notification();
PushMessage pushMessage = new PushMessage(fcmSendRequest.fcmTopic(), notification);
PushMessage pushMessage = new PushMessage(notification);
sendMessage(pushMessage.getMessage(), notification);
}

Expand Down
Loading
Loading