[fix] #198 - isBooking update 안되는 이슈 해결 #199
Conversation
There was a problem hiding this comment.
updateBookingStatus 메서드는 Booking 엔티티의 bookingStatus 필드가 있어서 혼란을 줄 것 같습니다!
해당 메서드는 schedule 엔티티의 isBooking 필드를 업데이트 하는 것이기 때문에, updateBookingAvailability이 적절해 보입니다!
There was a problem hiding this comment.
저도 updateBookingAvailability가 더 적절해보이네요! rename하겠습니다 :)
| public void updateBookingStatus(Schedule schedule) { | ||
| boolean isBookingAvailable = isBookingAvailable(schedule); | ||
| if (schedule.isBooking() != isBookingAvailable) { | ||
| schedule.setBooking(isBookingAvailable); | ||
| schedule.updateIsBooking(isBookingAvailable); | ||
| scheduleRepository.save(schedule); | ||
| } | ||
| } |
There was a problem hiding this comment.
@Transactional
public void updateBookingStatus(Schedule schedule) {
boolean isBookingAvailable = isBookingAvailable(schedule);
if (schedule.isBooking() == isBookingAvailable) {
return;
}
schedule.updateIsBooking(isBookingAvailable);
scheduleRepository.save(schedule);
}얼리리턴으로 바꿔주세요!
|
|
||
| public void updateIsBooking(boolean isBooking) { | ||
| this.isBooking = isBooking; | ||
| } |
| @@ -64,7 +64,7 @@ public class PerformanceService { | |||
| private final BookingRepository bookingRepository; | |||
| private final PerformanceImageRepository performanceImageRepository; | |||
|
|
|||
| @Transactional(readOnly = true) | |||
| @Transactional | |||
There was a problem hiding this comment.
해당 오류를 발견하신 거에 대해서 고생하셨다고 말씀드리고 싶습니다!
그런데 다만 우려되는 점은 과연 단순 조회 메서드에 update 책임까지 부여하는게 맞는가? 에 대한 의문이 듭니다.
그 이유는 다음과 같습니다.
기존에 readOnly=true로 설정해둔 이유는 단순 조회 메서드 이기에 성능을 개선하기 위해 true 옵션을 붙인 걸로 기억합니다. 하지만 readOnly를 없애고 update를 로직을 포함하는 순간 get + update가 되어버려 두 개의 책임을 갖는 메서드가 되어버린 것 같다고 생각이 듭니다.. (동시성 이슈도 있을 수 있고요)
특정 시간에만 작업을 수행하여 시스템 부하를 최소화하고 정확하게 공연 종료 시점에 예매를 마감하도록 할 수 있어야 하는데, 이 부분에 대해서 조사를 해보았더니 TaskScheduler를 사용한 동적 스케줄링을 사용하면, 공연 종료 시점에 예매를 정확하게 마감하는 방법을 사용할 수 있을 것 같아요!
그렇게 된다면 기존 get 메서드는 readOnly=true 옵션으로 가는 방식으로도 갈 수 있을 것 같습니다
https://xxeol.tistory.com/53
참고 하면 좋을 블로그를 첨부하겠습니다!
There was a problem hiding this comment.
예매를 막는 경우가 공연 종료 시점뿐만 아니라 매진일 때도 있어서 공연 종료 시점에 예매를 막도록 taskScheduler를 이용해 동적 스케쥴링을 하는 것으로는 매진인 경우의 수에 대처하지 못합니다ㅠㅠ
사용자의 플로우가 공연 상세페이지와 공연 예매페이지에서 노출되는 공연 예매 가능 여부를 보고 바로 예매로 이어지기 때문에 조회 시 isBooking에 대한 확인 및 update는 필수여서 readOnly=true 옵션을 붙이기 힘들 것 같습니다.
다만 매진 여부와 공연 종료 여부를 모두 확인 후 isBooking을 업데이트하던 기존 방식에서 공연 종료 시점에 따른 isBooking 업데이트는 스케쥴링으로 분리하고, 매진 여부만 조회 시점에 확인하는 방식으로 리팩토링하면 연산량이 줄어 성능상 이점은 있을 것 같습니다.
이러한 리팩토링에 대해서는 어떻게 생각하시는지 궁금합니다!
There was a problem hiding this comment.
예매를 막는 경우가 공연 종료 시점뿐만 아니라 매진일 때도 있어서 공연 종료 시점에 예매를 막도록 taskScheduler를 이용해 동적 스케쥴링을 하는 것으로는 매진인 경우의 수에 대처하지 못합니다ㅠㅠ 사용자의 플로우가 공연 상세페이지와 공연 예매페이지에서 노출되는 공연 예매 가능 여부를 보고 바로 예매로 이어지기 때문에 조회 시 isBooking에 대한 확인 및 update는 필수여서 readOnly=true 옵션을 붙이기 힘들 것 같습니다. 다만 매진 여부와 공연 종료 여부를 모두 확인 후 isBooking을 업데이트하던 기존 방식에서 공연 종료 시점에 따른 isBooking 업데이트는 스케쥴링으로 분리하고, 매진 여부만 조회 시점에 확인하는 방식으로 리팩토링하면 연산량이 줄어 성능상 이점은 있을 것 같습니다. 이러한 리팩토링에 대해서는 어떻게 생각하시는지 궁금합니다!
공연 종료 시점에 따른 isBooking 업데이트는 task 스케쥴링으로 분리하는 방식은 좋으나, 매진 여부를 조회 시점에 확인하는 방식보다는 다음과 같은 방법이 더 좋을 것 같다고 생각됩니다.
isBooking이 변할 수 있는 케이스는 티켓 예매와 예매 취소라고 생각됩니다. 따라서 다음의 방식을 제안드립니다!
- 티켓 예매 시, 예매가 완료된 직후 해당 공연의 모든 스케줄의 매진 여부를 체크하여 모두 매진 시 예매를 막는 방법
- 예매 취소 시, 취소가 완료된 직후 예매를 다시 활성화 하는 방법(모든 스케줄을 체크할 필요가 없는 이유는 취소표가 나왔기에 최소 하나의 회차는 예매가 가능한 상태이기 때문)
이유는 사용자가 페이지를 열 때마다 서버가 모든 스케줄을 조회하여 매진 상태를 체크해야 합니다. 사용자가 많아질수록 서버 부하가 증가하고, 성능 저하로 이어질 수 있기 때문이고, 불필요한 연산을 줄였으면 하기 때문입니다!
There was a problem hiding this comment.
추가 코멘트 남깁니다!
지금 회차 테이블에 isBooking을 관리하고 계신데, 공연 예매 버튼을 막기 위해서는 존재하는 모든 회차의 isBooking이 false여야 합니다!
따라서 공연 예매를 막는건 매진 시점이 아니라 정확히 말하면 모든 회차가 매진 시점이 였을때여야 합니다!!
There was a problem hiding this comment.
티켓 예매 직후와 예매 취소 직후 isBooking을 update하는 방법 좋네요~
추가 코멘트 관련해서는 일부 회차만 매진되거나 공연이 지난 경우가 있기 때문에 회차별로 isBooking이 다를 수 있습니다!
프론트 측에서도 각 회차의 isBooking 값에 따라 일부만 false인 경우 예매 페이지에서 그 회차 예매만 선택 불가능하도록 구현하신 걸로 알고 있습니다. 그리고 모든 회차의 isBooking이 false인 경우는 아예 예매 페이지로 넘어갈 수 없도록 구현하신 걸로 알고 있습니다. 그래서 공연 예매를 막기 위해 모든 회차의 isBooking이 false일 필요는 없습니다!
There was a problem hiding this comment.
티켓 예매 직후와 예매 취소 직후 isBooking을 update하는 방법 좋네요~ 추가 코멘트 관련해서는 일부 회차만 매진되거나 공연이 지난 경우가 있기 때문에 회차별로 isBooking이 다를 수 있습니다! 프론트 측에서도 각 회차의 isBooking 값에 따라 일부만 false인 경우 예매 페이지에서 그 회차 예매만 선택 불가능하도록 구현하신 걸로 알고 있습니다. 그리고 모든 회차의 isBooking이 false인 경우는 아예 예매 페이지로 넘어갈 수 없도록 구현하신 걸로 알고 있습니다. 그래서 공연 예매를 막기 위해 모든 회차의 isBooking이 false일 필요는 없습니다!
제가 이해한 게 맞는지 봐주세요 ㅎㅎ
저희는 회차별 isBooking을 true/false로 주되, 모든 회차의 isBooking이 false인 경우는 아예 예매 페이지로 넘어갈 수 없도록, 그리고 취소표가 나왔을 경우 예매 페이지로 넘어갈 수 있도록 예매 버튼을 활성화 시키는 것에 대한 부분은 프론트측에 맡기자는 말씀이실까요?
이 방법이 맞다면 저는 좋은 것 같다고 생각이 듭니다!
There was a problem hiding this comment.
네 맞습니다!! 프론트 측과 한번 더 체크하겠습니다~
| if (schedule.isBooking() != isBookingAvailable) { | ||
| schedule.setBooking(isBookingAvailable); | ||
| schedule.updateIsBooking(isBookingAvailable); | ||
| scheduleRepository.save(schedule); |
There was a problem hiding this comment.
isBooking과 isBookingAvailable의 상태가 다르면 isBooking의 상태로 업데이트를 수행하시는 것 같습니다!
isBookingAvailable이 true로 되기 위해서는 현재 예매 상태가 true여야하고, 남은 티켓 수가 1개 이상이여야 하고, 현재 시간이 공연 종료 시간 이전이어야 예매가 가능한 것 같은데, 해당 로직에서는 예매 불가능 -> 예매 가능으로 바꿀수 없습니다!
예매 불가능(isBooking()이 false인 상태)에서 예매 가능 상태(isBooking()이 true인 상태)로 바꾸려면 schedule.isBooking() && availableTicketCount > 0 && LocalDateTime.now().isBefore(performanceEndTime이 true가 되어야 합니다. 그런데 이미 schedule.isBooking()이 false이기 때문에 false가 되게 됩니다!
그래서 return 조건문을 다음과 같이 변경하셔야 할 것 같습니다!
return availableTicketCount > 0 && LocalDateTime.now().isBefore(performanceEndTime);
There was a problem hiding this comment.
제가 놓친 부분이었네요 꼼꼼한 확인 감사합니다!
회차 별로 다르게 관리됩니다! |
Related issue 🛠
Work Description ✏️
공연상세페이지나 공연예매페이지에서 공연관련 정보를 GET할 때 원래 duedate와 잔여티켓수를 바탕으로 schedule의 isBooking 필드가 update돼야하는데 안되는 것을 확인했습니다.
공연상세페이지 조회에서는 상황에 맞는 값으로 isBooking이 조회는 됐으나 db에 해당 값이 update가 안됐고, 공연예매페이지 조회에서는 조회와 update가 모두 잘됐습니다.
확인해본 결과 PerformanceService의 공연상세페이지 조회 메소드인 getPerformanceDetail의 @transactional 설정이 readOnly=true로 돼있어서 공연상세페이지 조회 시 isBooking 필드가 db에서 업데이트되지 못했음을 확인했고 readOnly 설정을 없애며 해결했습니다.
또한 기존에는 Schedule 엔티티 클래스의 @Setter를 사용해 isBooking 필드를 수동으로 설정했지만, 이 경우 필드 변경이 적절히 감지되지 않을 가능성도 있어 엔티티의 isBooking 필드를 수정하는 메서드인 updateIsBooking 명시적으로 추가해 사용하는 것으로 refactoring 했습니다.
Trouble Shooting ⚽️
Related ScreenShot 📷
공연 상세페이지 조회 테스트
공연상세페이지 조회에서 공연날짜가 지난 공연의 isBooking false로 update 확인
performance_id 7번은 공연날짜가 지났으며 기존 is_booking이 true로 저장돼있었습니다.
is_booking이 false로 바뀌어 조회되고 db에도 반영된것을 확인했습니다.
공연상세페이지 조회에서 잔여티켓수가 0인(매진된) 공연의 isBooking false로 update 확인
performance_id 8번은 잔여티켓수가 0이며 기존 is_booking이 true로 저장돼있었습니다.
is_booking이 false로 바뀌어 조회되고 db에도 반영된것을 확인했습니다.
공연상세페이지 조회에서 공연날짜가 지났고 잔여티켓수가 0인(매진된) 공연의 isBooking false로 update 확인
performance_id 9번은 공연날짜가 지났고 잔여티켓수가 0이며 기존 is_booking이 true로 저장돼있었습니다.
is_booking이 false로 바뀌어 조회되고 db에도 반영된것을 확인했습니다.
공연상세페이지 조회에서 공연날짜 전이고 잔여티켓이 존재하는 공연의 isBooking true로 유지되는 것 확인
performance_id 12번은 공연날짜 전이고 잔여티켓이 존재하며 기존 is_booking이 true로 저장돼있었습니다.
is_booking이 true로 유지되어 조회되고 db에서도 유지된것을 확인했습니다.
공연 예매페이지 조회 테스트
공연예매페이지 조회에서 공연날짜가 지난 공연의 isBooking false로 update 확인
performance_id 13번은 공연날짜가 지났으며 기존 is_booking이 true로 저장돼있었습니다.
is_booking이 false로 바뀌어 조회되고 db에도 반영된것을 확인했습니다.
공연예매페이지 조회에서 잔여티켓수가 0인(매진된) 공연의 isBooking false로 update 확인
performance_id 14번은 잔여티켓수가 0이며 기존 is_booking이 true로 저장돼있었습니다.
is_booking이 false로 바뀌어 조회되고 db에도 반영된것을 확인했습니다.
공연예매페이지 조회에서 공연날짜가 지났고 잔여티켓수가 0인(매진된) 공연의 isBooking false로 update 확인
performance_id 15번은 공연날짜가 지났고 잔여티켓수가 0이며 기존 is_booking이 true로 저장돼있었습니다.
is_booking이 false로 바뀌어 조회되고 db에도 반영된것을 확인했습니다.
공연예매페이지 조회에서 공연날짜 전이고 잔여티켓이 존재하는 공연의 isBooking true로 유지되는 것 확인
performance_id 12번은 공연날짜 전이고 잔여티켓이 존재하며 기존 is_booking이 true로 저장돼있었습니다.
is_booking이 true로 유지되어 조회되고 db에서도 유지된것을 확인했습니다.
Uncompleted Tasks 😅
To Reviewers 📢