Skip to content

Commit

Permalink
[FU-357] 날짜별 스케줄 검증로직 개선 및 리팩터링 (#91)
Browse files Browse the repository at this point in the history
* FU-357 feat: 날짜별 스케줄 상태 별 스케줄 충돌 검증

- 예약오픈 <-> 예약중지 간 충돌 불가
- 예약확정 <-> 예약오픈,예약중지 간 충돌 가능

* FU-357 feat: 날짜별 스케줄과 기본스케줄 단위 일치 보장

* FU-357 refactor: 날짜별 스케줄 검증 전담 클래스 분리

서비스 클래스를 간결하게 관리하고, 단일 책임 원칙(SRP)을 따르기 위해 검증 전담 클래스를 분리하였음

* FU-357 refactor: 스케줄 충돌 검증 코드 리팩터링 및 가독성 개선
  • Loading branch information
rheeri authored Jan 23, 2025
1 parent c8bd77a commit 6318e87
Show file tree
Hide file tree
Showing 9 changed files with 274 additions and 125 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ public enum ScheduleErrorCode implements ErrorCode {
DAILY_SCHEDULE_NOT_FOUND(500, "해당하는 날짜별 스케줄을 찾을 수 없습니다."),
DAILY_SCHEDULE_OVERLAP(400, "해당 일정에 이미 등록된 스케줄이 있습니다."),
DAILY_SCHEDULE_IN_PAST(400, "현재 시점 이전의 스케줄은 등록할 수 없습니다."),
START_TIME_AFTER_END_TIME(400, "시작시간과 종료시간이 올바르지 않습니다.");
START_TIME_AFTER_END_TIME(400, "시작시간과 종료시간이 올바르지 않습니다."),
INVALID_SCHEDULE_UNIT(400, "기본스케줄 단위와 일치하지 않습니다.");

private final int httpStatus;
private final String message;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.fasterxml.jackson.annotation.JsonProperty;

public enum ScheduleUnit {

@JsonProperty("THIRTY_MINUTES")
THIRTY_MINUTES,
@JsonProperty("SIXTY_MINUTES")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import com.foru.freebe.member.entity.Member;
import com.foru.freebe.schedule.entity.DailySchedule;
import com.foru.freebe.schedule.entity.ScheduleStatus;

public interface DailyScheduleRepository extends JpaRepository<DailySchedule, Long> {
Optional<DailySchedule> findByMemberAndId(Member member, Long scheduleId);
Expand All @@ -18,7 +19,8 @@ public interface DailyScheduleRepository extends JpaRepository<DailySchedule, Lo

@Query("SELECT ds FROM DailySchedule ds WHERE ds.member = :photographer "
+ "AND ds.date = :date "
+ "AND ((ds.startTime < :endTime AND ds.endTime > :startTime))")
List<DailySchedule> findOverlappingSchedules(Member photographer, LocalDate date, LocalTime startTime,
LocalTime endTime);
+ "AND ((ds.startTime < :endTime AND ds.endTime > :startTime))"
+ "AND ds.scheduleStatus IN (:scheduleStatuses)")
List<DailySchedule> findConflictingSchedulesByStatuses(Member photographer, LocalDate date, LocalTime startTime,
LocalTime endTime, List<ScheduleStatus> scheduleStatuses);
}
Original file line number Diff line number Diff line change
@@ -1,24 +1,23 @@
package com.foru.freebe.schedule.service;

import java.time.LocalTime;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import com.foru.freebe.errors.errorcode.MemberErrorCode;
import com.foru.freebe.errors.errorcode.ScheduleErrorCode;
import com.foru.freebe.errors.exception.RestApiException;
import com.foru.freebe.member.entity.Member;
import com.foru.freebe.member.repository.MemberRepository;
import com.foru.freebe.schedule.dto.BaseScheduleDto;
import com.foru.freebe.schedule.dto.ScheduleUnitDto;
import com.foru.freebe.schedule.entity.BaseSchedule;
import com.foru.freebe.schedule.entity.DayOfWeek;
import com.foru.freebe.schedule.entity.OperationStatus;
import com.foru.freebe.schedule.repository.BaseScheduleRepository;
import com.foru.freebe.errors.errorcode.MemberErrorCode;
import com.foru.freebe.errors.errorcode.ScheduleErrorCode;
import com.foru.freebe.errors.exception.RestApiException;
import com.foru.freebe.member.entity.Member;
import com.foru.freebe.member.repository.MemberRepository;

import lombok.RequiredArgsConstructor;

Expand Down Expand Up @@ -62,7 +61,7 @@ public void updateSchedule(BaseScheduleDto baseScheduleDto, Member photographer)
}

public void createDefaultSchedule(Member photographer) {
for(DayOfWeek dayOfWeek : DayOfWeek.values()) {
for (DayOfWeek dayOfWeek : DayOfWeek.values()) {
BaseSchedule baseSchedule = BaseSchedule.builder()
.photographer(photographer)
.dayOfWeek(dayOfWeek)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
package com.foru.freebe.schedule.service;

import java.time.Clock;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.util.List;
import java.util.stream.Collectors;

Expand All @@ -26,7 +23,7 @@
@Transactional
public class DailyScheduleService {
private final DailyScheduleRepository dailyScheduleRepository;
private final Clock clock;
private final DailyScheduleValidator validator;

public List<DailyScheduleResponse> getDailySchedules(Member photographer, DailyScheduleMonthlyRequest request) {
return dailyScheduleRepository.findByMember(photographer)
Expand All @@ -38,9 +35,10 @@ public List<DailyScheduleResponse> getDailySchedules(Member photographer, DailyS
}

public DailyScheduleAddResponse addDailySchedule(Member photographer, DailyScheduleRequest request) {
validateTimeRange(request.getStartTime(), request.getEndTime());
validateScheduleInFuture(request);
validateScheduleOverlap(photographer, request);
validator.validateTimeRange(request.getStartTime(), request.getEndTime());
validator.validateScheduleUnit(photographer.getScheduleUnit(), request.getStartTime(), request.getEndTime());
validator.validateScheduleInFuture(request);
validator.validateConflictingSchedules(photographer, request);

DailySchedule dailySchedule = DailySchedule.builder()
.member(photographer)
Expand All @@ -55,13 +53,14 @@ public DailyScheduleAddResponse addDailySchedule(Member photographer, DailySched
}

public void updateDailySchedule(Member photographer, Long scheduleId, DailyScheduleRequest request) {
validateTimeRange(request.getStartTime(), request.getEndTime());
validator.validateTimeRange(request.getStartTime(), request.getEndTime());
validator.validateScheduleUnit(photographer.getScheduleUnit(), request.getStartTime(), request.getEndTime());

DailySchedule dailySchedule = dailyScheduleRepository.findByMemberAndId(photographer, scheduleId)
.orElseThrow(() -> new RestApiException(ScheduleErrorCode.DAILY_SCHEDULE_NOT_FOUND));

validateScheduleInFuture(request);
validateScheduleOverlap(photographer, request, scheduleId);
validator.validateScheduleInFuture(request);
validator.validateConflictingSchedules(photographer, request, scheduleId);

dailySchedule.updateScheduleStatus(request.getScheduleStatus());
dailySchedule.updateDate(request.getDate());
Expand All @@ -74,41 +73,6 @@ public void deleteDailySchedule(Member photographer, Long scheduleId) {
.orElseThrow(() -> new RestApiException(ScheduleErrorCode.DAILY_SCHEDULE_NOT_FOUND)));
}

private void validateScheduleOverlap(Member member, DailyScheduleRequest request) {
List<DailySchedule> overlappingSchedules = dailyScheduleRepository.findOverlappingSchedules(member,
request.getDate(), request.getStartTime(), request.getEndTime());

if (!overlappingSchedules.isEmpty()) {
throw new RestApiException(ScheduleErrorCode.DAILY_SCHEDULE_OVERLAP);
}
}

private void validateTimeRange(LocalTime startTime, LocalTime endTime) {
if (startTime.isAfter(endTime) || startTime.equals(endTime)) {
throw new RestApiException(ScheduleErrorCode.START_TIME_AFTER_END_TIME);
}
}

private void validateScheduleInFuture(DailyScheduleRequest request) {
LocalDateTime requestDateTime = request.getDate().atTime(request.getStartTime());

if (requestDateTime.isBefore(LocalDateTime.now(clock))) {
throw new RestApiException(ScheduleErrorCode.DAILY_SCHEDULE_IN_PAST);
}
}

private void validateScheduleOverlap(Member member, DailyScheduleRequest request, Long scheduleId) {
List<DailySchedule> overlappingSchedules = dailyScheduleRepository.findOverlappingSchedules(member,
request.getDate(), request.getStartTime(), request.getEndTime());

if (overlappingSchedules.size() == 1 && overlappingSchedules.get(0).getId().equals(scheduleId)) {
return;
}
if (!overlappingSchedules.isEmpty()) {
throw new RestApiException(ScheduleErrorCode.DAILY_SCHEDULE_OVERLAP);
}
}

private DailyScheduleResponse toDailyScheduleResponse(DailySchedule dailySchedule) {
return DailyScheduleResponse.builder()
.scheduleId(dailySchedule.getId())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package com.foru.freebe.schedule.service;

import java.time.Clock;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.util.List;

import org.springframework.stereotype.Service;

import com.foru.freebe.errors.errorcode.ScheduleErrorCode;
import com.foru.freebe.errors.exception.RestApiException;
import com.foru.freebe.member.entity.Member;
import com.foru.freebe.member.entity.ScheduleUnit;
import com.foru.freebe.schedule.dto.DailyScheduleRequest;
import com.foru.freebe.schedule.entity.DailySchedule;
import com.foru.freebe.schedule.entity.ScheduleStatus;
import com.foru.freebe.schedule.repository.DailyScheduleRepository;

import jakarta.transaction.Transactional;
import lombok.RequiredArgsConstructor;

@Service
@RequiredArgsConstructor
@Transactional
public class DailyScheduleValidator {
private final Clock clock;
private final DailyScheduleRepository dailyScheduleRepository;

public void validateTimeRange(LocalTime startTime, LocalTime endTime) {
if (startTime.isAfter(endTime) || startTime.equals(endTime)) {
throw new RestApiException(ScheduleErrorCode.START_TIME_AFTER_END_TIME);
}
}

public void validateScheduleUnit(ScheduleUnit scheduleUnit, LocalTime startTime, LocalTime endTime) {
switch (scheduleUnit) {
case SIXTY_MINUTES -> {
if (startTime.getMinute() != 0 || endTime.getMinute() != 0) {
throw new RestApiException(ScheduleErrorCode.INVALID_SCHEDULE_UNIT);
}
}
case THIRTY_MINUTES -> {
if (startTime.getMinute() != 0 && startTime.getMinute() != 30) {
throw new RestApiException(ScheduleErrorCode.INVALID_SCHEDULE_UNIT);
} else if (endTime.getMinute() != 0 && endTime.getMinute() != 30) {
throw new RestApiException(ScheduleErrorCode.INVALID_SCHEDULE_UNIT);
}
}
}
}

public void validateScheduleInFuture(DailyScheduleRequest request) {
LocalDateTime requestDateTime = request.getDate().atTime(request.getStartTime());

if (requestDateTime.isBefore(LocalDateTime.now(clock))) {
throw new RestApiException(ScheduleErrorCode.DAILY_SCHEDULE_IN_PAST);
}
}

public void validateConflictingSchedules(Member member, DailyScheduleRequest request) {
List<ScheduleStatus> scheduleStatuses = determineConflictingStatuses(request.getScheduleStatus());

List<DailySchedule> overlappingSchedules = dailyScheduleRepository.findConflictingSchedulesByStatuses(member,
request.getDate(), request.getStartTime(), request.getEndTime(), scheduleStatuses);

if (!overlappingSchedules.isEmpty()) {
throw new RestApiException(ScheduleErrorCode.DAILY_SCHEDULE_OVERLAP);
}
}

public void validateConflictingSchedules(Member member, DailyScheduleRequest request, Long scheduleId) {
List<ScheduleStatus> scheduleStatuses = determineConflictingStatuses(request.getScheduleStatus());

List<DailySchedule> conflictingSchedules = dailyScheduleRepository.findConflictingSchedulesByStatuses(member,
request.getDate(), request.getStartTime(), request.getEndTime(), scheduleStatuses);

if (conflictingSchedules.isEmpty() || isSelfConflictOnly(scheduleId, conflictingSchedules)) {
return;
}

throw new RestApiException(ScheduleErrorCode.DAILY_SCHEDULE_OVERLAP);
}

private List<ScheduleStatus> determineConflictingStatuses(ScheduleStatus scheduleStatus) {
if (scheduleStatus == ScheduleStatus.CONFIRMED) {
return List.of(ScheduleStatus.CONFIRMED);
} else {
return List.of(ScheduleStatus.OPEN, ScheduleStatus.CLOSED);
}
}

private boolean isSelfConflictOnly(Long scheduleId, List<DailySchedule> conflictingSchedules) {
return conflictingSchedules.size() == 1 && conflictingSchedules.get(0).getId().equals(scheduleId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.mockito.Mockito.*;

import java.time.LocalTime;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
Expand All @@ -11,6 +12,7 @@
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

import com.foru.freebe.member.entity.Member;
import com.foru.freebe.member.entity.Role;
import com.foru.freebe.schedule.dto.BaseScheduleDto;
Expand All @@ -21,7 +23,6 @@

@ExtendWith(MockitoExtension.class)
class BaseScheduleServiceTest {

@Mock
private BaseScheduleRepository baseScheduleRepository;

Expand Down Expand Up @@ -92,10 +93,12 @@ void updateScheduleForActivation() {
Assertions.assertEquals(LocalTime.of(9, 0), existingBaseSchedule.getStartTime());
Assertions.assertEquals(LocalTime.of(18, 0), existingBaseSchedule.getEndTime());

verify(baseScheduleRepository, times(1)).findByDayOfWeekAndPhotographerId(baseScheduleDto.getDayOfWeek(), photographer.getId());
verify(baseScheduleRepository, times(1)).findByDayOfWeekAndPhotographerId(baseScheduleDto.getDayOfWeek(),
photographer.getId());
}

private BaseScheduleDto createBaseScheduleDto(DayOfWeek dayOfWeek, LocalTime startTime, LocalTime endTime, OperationStatus operationStatus) {
private BaseScheduleDto createBaseScheduleDto(DayOfWeek dayOfWeek, LocalTime startTime, LocalTime endTime,
OperationStatus operationStatus) {
return BaseScheduleDto.builder()
.dayOfWeek(dayOfWeek)
.startTime(startTime)
Expand Down
Loading

0 comments on commit 6318e87

Please sign in to comment.