-
Notifications
You must be signed in to change notification settings - Fork 0
fix: AI & Schedule #20
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
🤖 GPT PR Review결론: request-changes 요약: The PR lacks a detailed description and does not address potential security, contract, exception, performance, or testing issues. 주요 지적
제안 테스트
|
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.
Pull request overview
This PR refactors the schedule service to properly handle recurring schedules by expanding them in-memory, and adds formatting guidelines to AI prompt modules to prevent markdown output. The main improvements include:
- Comprehensive recurring schedule expansion logic supporting DAILY, WEEKLY, MONTHLY, and YEARLY patterns with various end conditions
- Refactored schedule service with better code organization and helper methods
- Added comprehensive test coverage for recurring schedule scenarios
- Updated AI prompt modules to explicitly prohibit markdown formatting in outputs
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
src/main/java/com/example/konnect_backend/domain/schedule/service/ScheduleService.java |
Major refactoring implementing in-memory repeat schedule expansion logic with helper methods for date calculation and validation |
src/test/java/com/example/konnect_backend/domain/schedule/service/ScheduleServiceTest.java |
New comprehensive test suite covering calendar queries, daily/monthly schedule retrieval, and various repeat patterns |
src/main/java/com/example/konnect_backend/domain/schedule/repository/ScheduleRepository.java |
Simplified repository methods to use fetch joins for repeat data, removing complex date-based queries |
src/main/java/com/example/konnect_backend/domain/schedule/repository/ScheduleRepeatRepository.java |
Removed unused findByUser method, keeping only essential operations |
src/main/java/com/example/konnect_backend/domain/schedule/entity/ScheduleRepeat.java |
Changed relationship from ManyToOne to OneToOne with Schedule, removed User reference |
src/main/java/com/example/konnect_backend/domain/schedule/entity/Schedule.java |
Added explicit LAZY fetch type to relationships for better performance control |
src/main/java/com/example/konnect_backend/domain/ai/service/prompt/UnifiedExtractorModule.java |
Added markdown prohibition rules to ensure clean text output |
src/main/java/com/example/konnect_backend/domain/ai/service/prompt/TranslatorModule.java |
Added markdown prohibition rules to ensure clean text output |
src/main/java/com/example/konnect_backend/domain/ai/service/prompt/SummarizerModule.java |
Added markdown prohibition rules to ensure clean text output |
src/main/java/com/example/konnect_backend/domain/ai/service/prompt/KoreanSimplifierModule.java |
Added markdown prohibition rules to ensure clean text output |
src/main/java/com/example/konnect_backend/domain/ai/service/prompt/DocumentClassifierModule.java |
Added markdown prohibition rules to ensure clean text output |
src/main/java/com/example/konnect_backend/domain/ai/service/prompt/DifficultExpressionExtractorModule.java |
Added markdown prohibition rules to ensure clean text output |
.claude/settings.local.json |
Added additional bash command permissions for local development |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Fetch Join으로 한 번에 조회 (N+1 방지) | ||
| List<Schedule> allSchedules = scheduleRepository.findAllByUserWithRepeat(user); |
Copilot
AI
Dec 3, 2025
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.
Performance concern: findAllByUserWithRepeat fetches ALL schedules for a user regardless of date range. For users with many historical schedules, this could load thousands of records into memory even when only querying a single month. Consider adding date range filtering to the query (e.g., schedules that could possibly intersect with the range based on start date and repeat settings) to reduce memory usage and improve performance.
| // Fetch Join으로 한 번에 조회 (N+1 방지) | |
| List<Schedule> allSchedules = scheduleRepository.findAllByUserWithRepeat(user); | |
| // 날짜 범위에 해당하는 일정만 조회 (성능 개선) | |
| List<Schedule> allSchedules = scheduleRepository.findAllByUserWithRepeatAndDateRange(user, rangeStart, rangeEnd); |
| LocalDate currentDate = findFirstDateInRange(scheduleStartDate, rangeStart, repeatType); | ||
|
|
||
| int count = 0; | ||
| int maxIterations = 1000; // 무한 루프 방지 |
Copilot
AI
Dec 3, 2025
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.
The magic number 1000 for maxIterations should be extracted as a named constant at the class level for better maintainability. This makes it easier to adjust the limit and clarifies its purpose as a safety mechanism against infinite loops in repeat schedule expansion.
| /** | ||
| * 반복 일정을 주어진 범위 내의 모든 날짜로 확장합니다. | ||
| */ | ||
| private List<LocalDate> expandRepeatSchedule(Schedule schedule, ScheduleRepeat repeat, | ||
| LocalDate rangeStart, LocalDate rangeEnd) { | ||
| List<LocalDate> expandedDates = new ArrayList<>(); | ||
| LocalDate scheduleStartDate = schedule.getStartDate().toLocalDate(); | ||
| RepeatType repeatType = repeat.getRepeatType(); | ||
|
|
||
| // 반복 종료일 계산 | ||
| LocalDate effectiveEndDate = calculateEffectiveEndDate(repeat, rangeEnd); | ||
|
|
||
| // rangeStart 이전의 날짜는 스킵하고 첫 번째 유효한 날짜부터 시작 | ||
| LocalDate currentDate = findFirstDateInRange(scheduleStartDate, rangeStart, repeatType); | ||
|
|
||
| int count = 0; | ||
| int maxIterations = 1000; // 무한 루프 방지 | ||
|
|
||
| while (currentDate != null && !currentDate.isAfter(effectiveEndDate) && count < maxIterations) { | ||
| // COUNT 제한 확인 | ||
| if (repeat.getRepeatEndType() == RepeatEndType.COUNT && repeat.getRepeatCount() != null) { | ||
| long currentCount = calculateRepeatCount(scheduleStartDate, currentDate, repeatType); | ||
| if (currentCount >= repeat.getRepeatCount()) { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // 범위 내 날짜만 추가 | ||
| if (isDateInRange(currentDate, rangeStart, rangeEnd)) { | ||
| expandedDates.add(currentDate); | ||
| } | ||
|
|
||
| currentDate = getNextRepeatDate(currentDate, repeatType, scheduleStartDate); | ||
| count++; | ||
| } | ||
|
|
||
| return expandedDates; | ||
| } |
Copilot
AI
Dec 3, 2025
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.
[nitpick] The complex repeat schedule expansion logic in this method lacks documentation on edge cases and assumptions. Consider adding documentation for:
- How it handles schedules that started before the query range
- Behavior when repeat end date extends beyond query range
- Monthly repeats on days that don't exist in all months (e.g., 31st)
- Performance characteristics and when the 1000 iteration limit might be hit
This would help future maintainers understand the algorithm and its limitations.
| /** | ||
| * 특정 기간 내 일정을 반복 설정과 함께 조회 | ||
| */ | ||
| @Query("SELECT DISTINCT s FROM Schedule s " + | ||
| "LEFT JOIN FETCH s.scheduleRepeat " + | ||
| "WHERE s.user = :user " + | ||
| "AND ((s.startDate >= :startDate AND s.startDate <= :endDate) " + | ||
| "OR (s.endDate >= :startDate AND s.endDate <= :endDate) " + | ||
| "OR (s.startDate <= :startDate AND s.endDate >= :endDate)) " + | ||
| "ORDER BY s.startDate ASC") | ||
| List<Schedule> findByUserAndDateRange(@Param("user") User user, | ||
| @Param("startDate") LocalDateTime startDate, | ||
| @Param("endDate") LocalDateTime endDate); | ||
|
|
||
| @Query("SELECT s FROM Schedule s WHERE s.user = :user " + | ||
| List<Schedule> findByUserAndDateRangeWithRepeat(@Param("user") User user, | ||
| @Param("startDate") LocalDateTime startDate, | ||
| @Param("endDate") LocalDateTime endDate); |
Copilot
AI
Dec 3, 2025
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.
The findByUserAndDateRangeWithRepeat method is defined but never used in the service layer. The service now uses findAllByUserWithRepeat and filters in memory instead. Consider either removing this unused method or documenting why it's kept for future use.
| // 12월 3, 10, 17일에만 일정이 있어야 함 (3회) | ||
| List<Integer> expectedDays = List.of(3, 10, 17); | ||
| long daysWithSchedule = result.stream() | ||
| .filter(CalendarDateResponse::getHasSchedule) | ||
| .count(); | ||
| assertThat(daysWithSchedule).isEqualTo(3); |
Copilot
AI
Dec 3, 2025
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.
Missing test coverage for the repeat count off-by-one bug. The test on line 310 asserts daysWithSchedule equals 3, but doesn't verify which specific dates have schedules. Due to the bug in calculateRepeatCount (which returns 0-based counts), this test would actually pass even though 4 occurrences might be generated. Add assertions to verify the exact dates to catch this bug.
| return ChronoUnit.DAYS.between(startDate, targetDate); | ||
| case WEEKLY: | ||
| return ChronoUnit.WEEKS.between(startDate, targetDate); | ||
| case MONTHLY: | ||
| return ChronoUnit.MONTHS.between(startDate, targetDate); | ||
| case YEARLY: | ||
| return ChronoUnit.YEARS.between(startDate, targetDate); | ||
| default: | ||
| return 0; |
Copilot
AI
Dec 3, 2025
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.
The calculateRepeatCount method is calculating the count incorrectly. It returns 0 for the first occurrence (when targetDate equals startDate), but the first occurrence should be count 1, not 0. This causes the COUNT-based repeat limit to be off by one.
For example, if repeatCount is 3:
- First occurrence:
calculateRepeatCountreturns 0, passes check (0 < 3) - Second occurrence: returns 1, passes check (1 < 3)
- Third occurrence: returns 2, passes check (2 < 3)
- Fourth occurrence: returns 3, fails check (3 >= 3) ❌ Should have stopped at 3rd
The fix should add 1 to the count or change the comparison to account for 0-based indexing.
| return ChronoUnit.DAYS.between(startDate, targetDate); | |
| case WEEKLY: | |
| return ChronoUnit.WEEKS.between(startDate, targetDate); | |
| case MONTHLY: | |
| return ChronoUnit.MONTHS.between(startDate, targetDate); | |
| case YEARLY: | |
| return ChronoUnit.YEARS.between(startDate, targetDate); | |
| default: | |
| return 0; | |
| return ChronoUnit.DAYS.between(startDate, targetDate) + 1; | |
| case WEEKLY: | |
| return ChronoUnit.WEEKS.between(startDate, targetDate) + 1; | |
| case MONTHLY: | |
| return ChronoUnit.MONTHS.between(startDate, targetDate) + 1; | |
| case YEARLY: | |
| return ChronoUnit.YEARS.between(startDate, targetDate) + 1; | |
| default: | |
| return 1; |
| LocalDate yearlyCandidate = LocalDate.of(rangeStart.getYear(), | ||
| scheduleStart.getMonth(), scheduleStart.getDayOfMonth()); | ||
| if (yearlyCandidate.isBefore(rangeStart)) { | ||
| yearlyCandidate = yearlyCandidate.plusYears(1); |
Copilot
AI
Dec 3, 2025
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.
Potential issue with leap year handling: When scheduling a yearly repeat on February 29th, the code at line 339 may throw a DateTimeException if the target year is not a leap year. The LocalDate.of() constructor will fail when trying to create February 29th in a non-leap year.
Consider using YearMonth.of(rangeStart.getYear(), scheduleStart.getMonth()).atEndOfMonth() for February dates, or add try-catch handling to fall back to the last valid day of the month.
| LocalDate yearlyCandidate = LocalDate.of(rangeStart.getYear(), | |
| scheduleStart.getMonth(), scheduleStart.getDayOfMonth()); | |
| if (yearlyCandidate.isBefore(rangeStart)) { | |
| yearlyCandidate = yearlyCandidate.plusYears(1); | |
| int targetYear = rangeStart.getYear(); | |
| int targetMonth = scheduleStart.getMonthValue(); | |
| int targetDay = scheduleStart.getDayOfMonth(); | |
| YearMonth yearMonth = YearMonth.of(targetYear, targetMonth); | |
| int lastDayOfMonth = yearMonth.lengthOfMonth(); | |
| int validDay = Math.min(targetDay, lastDayOfMonth); | |
| LocalDate yearlyCandidate = LocalDate.of(targetYear, targetMonth, validDay); | |
| if (yearlyCandidate.isBefore(rangeStart)) { | |
| yearMonth = YearMonth.of(targetYear + 1, targetMonth); | |
| lastDayOfMonth = yearMonth.lengthOfMonth(); | |
| validDay = Math.min(targetDay, lastDayOfMonth); | |
| yearlyCandidate = LocalDate.of(targetYear + 1, targetMonth, validDay); |
| @Query("SELECT DISTINCT s FROM Schedule s " + | ||
| "LEFT JOIN FETCH s.scheduleRepeat " + | ||
| "WHERE s.user = :user " + | ||
| "AND s.startDate >= :now " + | ||
| "ORDER BY s.startDate ASC") | ||
| List<Schedule> findUpcomingSchedules(@Param("user") User user, | ||
| @Param("now") LocalDateTime now); | ||
|
|
||
| List<Schedule> findByUserOrderByStartDateDesc(User user); | ||
|
|
||
| @Query("SELECT s FROM Schedule s WHERE s.user = :user " + | ||
| "AND DATE(s.startDate) = :date " + | ||
| "ORDER BY s.startDate ASC") | ||
| List<Schedule> findByUserAndDate(@Param("user") User user, | ||
| @Param("date") LocalDateTime date); | ||
|
|
||
| @Query("SELECT s FROM Schedule s WHERE s.user = :user " + | ||
| "AND s.startDate >= :startOfWeek AND s.startDate < :endOfWeek " + | ||
| "ORDER BY s.startDate ASC") | ||
| List<Schedule> findByUserAndWeek(@Param("user") User user, | ||
| @Param("startOfWeek") LocalDateTime startOfWeek, | ||
| @Param("endOfWeek") LocalDateTime endOfWeek); | ||
|
|
||
| @Query("SELECT DATE(s.startDate) as date, COUNT(s) as count " + | ||
| "FROM Schedule s WHERE s.user = :user " + | ||
| "AND s.startDate >= :startDate AND s.startDate < :endDate " + | ||
| "GROUP BY DATE(s.startDate)") | ||
| List<Object[]> findScheduleCountsByDate(@Param("user") User user, | ||
| @Param("startDate") LocalDateTime startDate, | ||
| @Param("endDate") LocalDateTime endDate); | ||
|
|
||
| @Query("SELECT s FROM Schedule s WHERE s.user = :user " + | ||
| "AND DATE(s.startDate) = CURRENT_DATE " + | ||
| "ORDER BY s.startDate ASC") | ||
| List<Schedule> findTodaySchedules(@Param("user") User user); | ||
| List<Schedule> findUpcomingSchedulesWithRepeat(@Param("user") User user, | ||
| @Param("now") LocalDateTime now); |
Copilot
AI
Dec 3, 2025
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.
The method findUpcomingSchedulesWithRepeat is defined but never called in the service layer. This appears to be an unused method that should either be utilized in the getRecentSchedules method (if it provides better performance) or removed to keep the codebase clean.
| private ScheduleRepeat weeklyRepeat; | ||
| private ScheduleRepeat monthlyRepeat; | ||
| private ScheduleRepeat dailyRepeat; |
Copilot
AI
Dec 3, 2025
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.
The test fields weeklyRepeat, monthlyRepeat, and dailyRepeat are declared but never used. Individual tests create their own repeat objects inline, making these fields unnecessary. Consider removing these unused fields to keep the test class clean.
| private ScheduleRepeat weeklyRepeat; | |
| private ScheduleRepeat monthlyRepeat; | |
| private ScheduleRepeat dailyRepeat; |
| @ExtendWith(MockitoExtension.class) | ||
| @DisplayName("ScheduleService 테스트") | ||
| class ScheduleServiceTest { | ||
|
|
||
| @Mock | ||
| private ScheduleRepository scheduleRepository; | ||
|
|
||
| @Mock | ||
| private ScheduleRepeatRepository scheduleRepeatRepository; | ||
|
|
||
| @Mock | ||
| private ScheduleAlarmRepository scheduleAlarmRepository; | ||
|
|
||
| @Mock | ||
| private UserRepository userRepository; | ||
|
|
||
| @Mock | ||
| private ChildRepository childRepository; | ||
|
|
||
| @InjectMocks | ||
| private ScheduleService scheduleService; | ||
|
|
||
| private User testUser; | ||
| private Schedule testSchedule; | ||
| private ScheduleRepeat weeklyRepeat; | ||
| private ScheduleRepeat monthlyRepeat; | ||
| private ScheduleRepeat dailyRepeat; | ||
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| testUser = User.builder() | ||
| .id(1L) | ||
| .name("테스트유저") | ||
| .build(); | ||
|
|
||
| // 반복 없는 일반 일정 (12월 5일) | ||
| testSchedule = Schedule.builder() | ||
| .scheduleId(1L) | ||
| .user(testUser) | ||
| .title("일반 일정") | ||
| .startDate(LocalDateTime.of(2025, 12, 5, 10, 0)) | ||
| .endDate(LocalDateTime.of(2025, 12, 5, 11, 0)) | ||
| .isAllDay(false) | ||
| .build(); | ||
| } | ||
|
|
||
| @Nested | ||
| @DisplayName("달력 조회 (getCalendarDates)") | ||
| class GetCalendarDatesTest { | ||
|
|
||
| @Test | ||
| @DisplayName("반복 없는 일정 - 해당 날짜에만 표시") | ||
| void noRepeatSchedule_showsOnlyOnStartDate() { | ||
| try (MockedStatic<SecurityUtil> securityUtil = mockStatic(SecurityUtil.class)) { | ||
| // given | ||
| securityUtil.when(SecurityUtil::getCurrentUserIdOrNull).thenReturn(1L); | ||
| when(userRepository.findById(1L)).thenReturn(Optional.of(testUser)); | ||
|
|
||
| Schedule schedule = Schedule.builder() | ||
| .scheduleId(1L) | ||
| .user(testUser) | ||
| .title("일반 일정") | ||
| .startDate(LocalDateTime.of(2025, 12, 15, 10, 0)) | ||
| .endDate(LocalDateTime.of(2025, 12, 15, 11, 0)) | ||
| .scheduleRepeat(null) | ||
| .build(); | ||
|
|
||
| when(scheduleRepository.findAllByUserWithRepeat(testUser)) | ||
| .thenReturn(List.of(schedule)); | ||
|
|
||
| // when | ||
| List<CalendarDateResponse> result = scheduleService.getCalendarDates(2025, 12); | ||
|
|
||
| // then | ||
| assertThat(result).hasSize(31); // 12월은 31일 | ||
|
|
||
| // 15일만 일정 있음 | ||
| CalendarDateResponse day15 = result.stream() | ||
| .filter(r -> r.getDate().getDayOfMonth() == 15) | ||
| .findFirst().orElseThrow(); | ||
| assertThat(day15.getHasSchedule()).isTrue(); | ||
| assertThat(day15.getScheduleCount()).isEqualTo(1); | ||
|
|
||
| // 다른 날짜는 일정 없음 | ||
| CalendarDateResponse day10 = result.stream() | ||
| .filter(r -> r.getDate().getDayOfMonth() == 10) | ||
| .findFirst().orElseThrow(); | ||
| assertThat(day10.getHasSchedule()).isFalse(); | ||
| assertThat(day10.getScheduleCount()).isEqualTo(0); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("매주 반복 일정 - 같은 요일에 모두 표시 (수요일)") | ||
| void weeklyRepeat_showsOnSameDayOfWeek() { | ||
| try (MockedStatic<SecurityUtil> securityUtil = mockStatic(SecurityUtil.class)) { | ||
| // given | ||
| securityUtil.when(SecurityUtil::getCurrentUserIdOrNull).thenReturn(1L); | ||
| when(userRepository.findById(1L)).thenReturn(Optional.of(testUser)); | ||
|
|
||
| // 12월 3일 수요일 시작, 매주 반복 | ||
| ScheduleRepeat repeat = ScheduleRepeat.builder() | ||
| .id(1L) | ||
| .repeatType(RepeatType.WEEKLY) | ||
| .repeatEndType(RepeatEndType.FOREVER) | ||
| .build(); | ||
|
|
||
| Schedule schedule = Schedule.builder() | ||
| .scheduleId(1L) | ||
| .user(testUser) | ||
| .title("매주 수요일 회의") | ||
| .startDate(LocalDateTime.of(2025, 12, 3, 10, 0)) // 수요일 | ||
| .endDate(LocalDateTime.of(2025, 12, 3, 11, 0)) | ||
| .scheduleRepeat(repeat) | ||
| .build(); | ||
|
|
||
| when(scheduleRepository.findAllByUserWithRepeat(testUser)) | ||
| .thenReturn(List.of(schedule)); | ||
|
|
||
| // when | ||
| List<CalendarDateResponse> result = scheduleService.getCalendarDates(2025, 12); | ||
|
|
||
| // then | ||
| // 12월의 수요일: 3, 10, 17, 24, 31일 | ||
| List<Integer> wednesdaysInDec = List.of(3, 10, 17, 24, 31); | ||
|
|
||
| for (CalendarDateResponse response : result) { | ||
| int day = response.getDate().getDayOfMonth(); | ||
| if (wednesdaysInDec.contains(day)) { | ||
| assertThat(response.getHasSchedule()) | ||
| .as("12월 %d일(수요일)에 일정이 있어야 함", day) | ||
| .isTrue(); | ||
| } else { | ||
| assertThat(response.getHasSchedule()) | ||
| .as("12월 %d일에는 일정이 없어야 함", day) | ||
| .isFalse(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("매달 반복 일정 - 같은 날짜에 표시 (매달 15일)") | ||
| void monthlyRepeat_showsOnSameDayOfMonth() { | ||
| try (MockedStatic<SecurityUtil> securityUtil = mockStatic(SecurityUtil.class)) { | ||
| // given | ||
| securityUtil.when(SecurityUtil::getCurrentUserIdOrNull).thenReturn(1L); | ||
| when(userRepository.findById(1L)).thenReturn(Optional.of(testUser)); | ||
|
|
||
| // 11월 15일 시작, 매달 반복 | ||
| ScheduleRepeat repeat = ScheduleRepeat.builder() | ||
| .id(1L) | ||
| .repeatType(RepeatType.MONTHLY) | ||
| .repeatEndType(RepeatEndType.FOREVER) | ||
| .build(); | ||
|
|
||
| Schedule schedule = Schedule.builder() | ||
| .scheduleId(1L) | ||
| .user(testUser) | ||
| .title("매달 15일 급여일") | ||
| .startDate(LocalDateTime.of(2025, 11, 15, 10, 0)) // 11월 15일 시작 | ||
| .endDate(LocalDateTime.of(2025, 11, 15, 11, 0)) | ||
| .scheduleRepeat(repeat) | ||
| .build(); | ||
|
|
||
| when(scheduleRepository.findAllByUserWithRepeat(testUser)) | ||
| .thenReturn(List.of(schedule)); | ||
|
|
||
| // when | ||
| List<CalendarDateResponse> result = scheduleService.getCalendarDates(2025, 12); | ||
|
|
||
| // then | ||
| // 12월 15일에만 일정이 있어야 함 | ||
| CalendarDateResponse day15 = result.stream() | ||
| .filter(r -> r.getDate().getDayOfMonth() == 15) | ||
| .findFirst().orElseThrow(); | ||
| assertThat(day15.getHasSchedule()).isTrue(); | ||
| assertThat(day15.getScheduleCount()).isEqualTo(1); | ||
|
|
||
| // 다른 날짜는 일정 없음 | ||
| long daysWithSchedule = result.stream() | ||
| .filter(CalendarDateResponse::getHasSchedule) | ||
| .count(); | ||
| assertThat(daysWithSchedule).isEqualTo(1); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("매일 반복 일정 - 기간 내 모든 날짜에 표시") | ||
| void dailyRepeat_showsOnEveryDay() { | ||
| try (MockedStatic<SecurityUtil> securityUtil = mockStatic(SecurityUtil.class)) { | ||
| // given | ||
| securityUtil.when(SecurityUtil::getCurrentUserIdOrNull).thenReturn(1L); | ||
| when(userRepository.findById(1L)).thenReturn(Optional.of(testUser)); | ||
|
|
||
| // 12월 1일 시작, 매일 반복, 12월 10일까지 | ||
| ScheduleRepeat repeat = ScheduleRepeat.builder() | ||
| .id(1L) | ||
| .repeatType(RepeatType.DAILY) | ||
| .repeatEndType(RepeatEndType.UNTIL_DATE) | ||
| .repeatEndDate(LocalDateTime.of(2025, 12, 10, 23, 59)) | ||
| .build(); | ||
|
|
||
| Schedule schedule = Schedule.builder() | ||
| .scheduleId(1L) | ||
| .user(testUser) | ||
| .title("매일 운동") | ||
| .startDate(LocalDateTime.of(2025, 12, 1, 7, 0)) | ||
| .endDate(LocalDateTime.of(2025, 12, 1, 8, 0)) | ||
| .scheduleRepeat(repeat) | ||
| .build(); | ||
|
|
||
| when(scheduleRepository.findAllByUserWithRepeat(testUser)) | ||
| .thenReturn(List.of(schedule)); | ||
|
|
||
| // when | ||
| List<CalendarDateResponse> result = scheduleService.getCalendarDates(2025, 12); | ||
|
|
||
| // then | ||
| // 12월 1일~10일에만 일정이 있어야 함 | ||
| for (CalendarDateResponse response : result) { | ||
| int day = response.getDate().getDayOfMonth(); | ||
| if (day >= 1 && day <= 10) { | ||
| assertThat(response.getHasSchedule()) | ||
| .as("12월 %d일에 일정이 있어야 함", day) | ||
| .isTrue(); | ||
| } else { | ||
| assertThat(response.getHasSchedule()) | ||
| .as("12월 %d일에는 일정이 없어야 함", day) | ||
| .isFalse(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("반복 횟수 제한 - COUNT 만큼만 표시") | ||
| void repeatWithCount_showsLimitedTimes() { | ||
| try (MockedStatic<SecurityUtil> securityUtil = mockStatic(SecurityUtil.class)) { | ||
| // given | ||
| securityUtil.when(SecurityUtil::getCurrentUserIdOrNull).thenReturn(1L); | ||
| when(userRepository.findById(1L)).thenReturn(Optional.of(testUser)); | ||
|
|
||
| // 12월 3일 수요일 시작, 매주 반복, 3회만 | ||
| ScheduleRepeat repeat = ScheduleRepeat.builder() | ||
| .id(1L) | ||
| .repeatType(RepeatType.WEEKLY) | ||
| .repeatEndType(RepeatEndType.COUNT) | ||
| .repeatCount(3L) | ||
| .build(); | ||
|
|
||
| Schedule schedule = Schedule.builder() | ||
| .scheduleId(1L) | ||
| .user(testUser) | ||
| .title("매주 수요일 회의 (3회)") | ||
| .startDate(LocalDateTime.of(2025, 12, 3, 10, 0)) | ||
| .endDate(LocalDateTime.of(2025, 12, 3, 11, 0)) | ||
| .scheduleRepeat(repeat) | ||
| .build(); | ||
|
|
||
| when(scheduleRepository.findAllByUserWithRepeat(testUser)) | ||
| .thenReturn(List.of(schedule)); | ||
|
|
||
| // when | ||
| List<CalendarDateResponse> result = scheduleService.getCalendarDates(2025, 12); | ||
|
|
||
| // then | ||
| // 12월 3, 10, 17일에만 일정이 있어야 함 (3회) | ||
| List<Integer> expectedDays = List.of(3, 10, 17); | ||
| long daysWithSchedule = result.stream() | ||
| .filter(CalendarDateResponse::getHasSchedule) | ||
| .count(); | ||
| assertThat(daysWithSchedule).isEqualTo(3); | ||
|
|
||
| for (CalendarDateResponse response : result) { | ||
| int day = response.getDate().getDayOfMonth(); | ||
| if (expectedDays.contains(day)) { | ||
| assertThat(response.getHasSchedule()).isTrue(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Nested | ||
| @DisplayName("일별 일정 조회 (getDailySchedules)") | ||
| class GetDailySchedulesTest { | ||
|
|
||
| @Test | ||
| @DisplayName("반복 일정이 해당 날짜에 확장되어 조회됨") | ||
| void repeatSchedule_expandedToTargetDate() { | ||
| try (MockedStatic<SecurityUtil> securityUtil = mockStatic(SecurityUtil.class)) { | ||
| // given | ||
| securityUtil.when(SecurityUtil::getCurrentUserIdOrNull).thenReturn(1L); | ||
| when(userRepository.findById(1L)).thenReturn(Optional.of(testUser)); | ||
|
|
||
| // 12월 3일 수요일 시작, 매주 반복 | ||
| ScheduleRepeat repeat = ScheduleRepeat.builder() | ||
| .id(1L) | ||
| .repeatType(RepeatType.WEEKLY) | ||
| .repeatEndType(RepeatEndType.FOREVER) | ||
| .build(); | ||
|
|
||
| Schedule schedule = Schedule.builder() | ||
| .scheduleId(1L) | ||
| .user(testUser) | ||
| .title("매주 수요일 회의") | ||
| .startDate(LocalDateTime.of(2025, 12, 3, 14, 0)) // 오후 2시 | ||
| .endDate(LocalDateTime.of(2025, 12, 3, 15, 0)) // 오후 3시 | ||
| .scheduleRepeat(repeat) | ||
| .build(); | ||
|
|
||
| when(scheduleRepository.findAllByUserWithRepeat(testUser)) | ||
| .thenReturn(List.of(schedule)); | ||
|
|
||
| // when - 12월 10일 (수요일) 조회 | ||
| List<ScheduleResponse> result = scheduleService.getDailySchedules(LocalDate.of(2025, 12, 10)); | ||
|
|
||
| // then | ||
| assertThat(result).hasSize(1); | ||
|
|
||
| ScheduleResponse response = result.get(0); | ||
| assertThat(response.getTitle()).isEqualTo("매주 수요일 회의"); | ||
| // 날짜는 조회한 날짜(12월 10일)로 변경되어야 함 | ||
| assertThat(response.getStartDate().toLocalDate()) | ||
| .isEqualTo(LocalDate.of(2025, 12, 10)); | ||
| // 시간은 원래 일정 시간 유지 | ||
| assertThat(response.getStartDate().getHour()).isEqualTo(14); | ||
| assertThat(response.getEndDate().getHour()).isEqualTo(15); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("해당 요일이 아닌 날짜 조회 시 빈 결과") | ||
| void wrongDayOfWeek_returnsEmpty() { | ||
| try (MockedStatic<SecurityUtil> securityUtil = mockStatic(SecurityUtil.class)) { | ||
| // given | ||
| securityUtil.when(SecurityUtil::getCurrentUserIdOrNull).thenReturn(1L); | ||
| when(userRepository.findById(1L)).thenReturn(Optional.of(testUser)); | ||
|
|
||
| // 12월 3일 수요일 시작, 매주 반복 | ||
| ScheduleRepeat repeat = ScheduleRepeat.builder() | ||
| .id(1L) | ||
| .repeatType(RepeatType.WEEKLY) | ||
| .repeatEndType(RepeatEndType.FOREVER) | ||
| .build(); | ||
|
|
||
| Schedule schedule = Schedule.builder() | ||
| .scheduleId(1L) | ||
| .user(testUser) | ||
| .title("매주 수요일 회의") | ||
| .startDate(LocalDateTime.of(2025, 12, 3, 14, 0)) | ||
| .endDate(LocalDateTime.of(2025, 12, 3, 15, 0)) | ||
| .scheduleRepeat(repeat) | ||
| .build(); | ||
|
|
||
| when(scheduleRepository.findAllByUserWithRepeat(testUser)) | ||
| .thenReturn(List.of(schedule)); | ||
|
|
||
| // when - 12월 11일 (목요일) 조회 | ||
| List<ScheduleResponse> result = scheduleService.getDailySchedules(LocalDate.of(2025, 12, 11)); | ||
|
|
||
| // then | ||
| assertThat(result).isEmpty(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Nested | ||
| @DisplayName("월별 일정 조회 (getMonthlySchedules)") | ||
| class GetMonthlySchedulesTest { | ||
|
|
||
| @Test | ||
| @DisplayName("매주 반복 일정이 해당 월의 모든 요일에 확장됨") | ||
| void weeklyRepeat_expandedToAllWeeksInMonth() { | ||
| try (MockedStatic<SecurityUtil> securityUtil = mockStatic(SecurityUtil.class)) { | ||
| // given | ||
| securityUtil.when(SecurityUtil::getCurrentUserIdOrNull).thenReturn(1L); | ||
| when(userRepository.findById(1L)).thenReturn(Optional.of(testUser)); | ||
|
|
||
| // 12월 3일 수요일 시작, 매주 반복 | ||
| ScheduleRepeat repeat = ScheduleRepeat.builder() | ||
| .id(1L) | ||
| .repeatType(RepeatType.WEEKLY) | ||
| .repeatEndType(RepeatEndType.FOREVER) | ||
| .build(); | ||
|
|
||
| Schedule schedule = Schedule.builder() | ||
| .scheduleId(1L) | ||
| .user(testUser) | ||
| .title("매주 수요일 회의") | ||
| .startDate(LocalDateTime.of(2025, 12, 3, 14, 0)) | ||
| .endDate(LocalDateTime.of(2025, 12, 3, 15, 0)) | ||
| .scheduleRepeat(repeat) | ||
| .build(); | ||
|
|
||
| when(scheduleRepository.findAllByUserWithRepeat(testUser)) | ||
| .thenReturn(List.of(schedule)); | ||
|
|
||
| // when | ||
| List<ScheduleResponse> result = scheduleService.getMonthlySchedules(2025, 12); | ||
|
|
||
| // then | ||
| // 12월의 수요일: 3, 10, 17, 24, 31일 = 5개 | ||
| assertThat(result).hasSize(5); | ||
|
|
||
| List<Integer> expectedDays = List.of(3, 10, 17, 24, 31); | ||
| List<Integer> actualDays = result.stream() | ||
| .map(r -> r.getStartDate().getDayOfMonth()) | ||
| .toList(); | ||
|
|
||
| assertThat(actualDays).containsExactlyElementsOf(expectedDays); | ||
| } | ||
| } | ||
| } | ||
| } |
Copilot
AI
Dec 3, 2025
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.
Missing test coverage for RepeatType.YEARLY schedules. The test suite covers DAILY, WEEKLY, and MONTHLY repeats, but doesn't test YEARLY repeats. Consider adding tests to verify yearly repeat behavior, especially edge cases like leap year dates (February 29th).
🤖 GPT PR Review결론: request-changes 요약: The PR lacks a detailed description and does not address potential security, contract, exception, performance, or testing issues. 주요 지적
제안 테스트
|
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.
Pull request overview
Copilot reviewed 29 out of 31 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .filter(alarm -> alarm.getAlarmTimeType() == AlarmTimeType.CUSTOM) | ||
| .filter(alarm -> alarm.getCustomMinutesBefore() != null) | ||
| .filter(alarm -> { | ||
| LocalDateTime alarmTime = alarm.getCustomMinutesBefore(); |
Copilot
AI
Dec 3, 2025
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.
The customMinutesBefore field is likely a Long representing minutes, not a LocalDateTime. This type mismatch will cause a compilation error. The field should be used to calculate the alarm time by subtracting minutes from the schedule's start date.
| LocalDateTime alarmTime = alarm.getCustomMinutesBefore(); | |
| LocalDateTime alarmTime = alarm.getSchedule().getStartDate().minusMinutes(alarm.getCustomMinutesBefore()); |
| private void saveRepeatSetting(Schedule schedule, ScheduleRepeatRequest repeatRequest) { | ||
| if (repeatRequest == null) { | ||
| return; | ||
| } | ||
|
|
||
| ScheduleRepeat repeat = ScheduleRepeat.builder() | ||
| .schedule(schedule) | ||
| .repeatType(repeatRequest.getRepeatType()) | ||
| .repeatEndType(repeatRequest.getRepeatEndType()) | ||
| .repeatEndDate(repeatRequest.getRepeatEndDate()) | ||
| .repeatCount(repeatRequest.getRepeatCount()) | ||
| .build(); | ||
|
|
||
| scheduleRepeatRepository.save(repeat); | ||
| } |
Copilot
AI
Dec 3, 2025
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.
The ScheduleRepeat entity was changed to have a OneToOne relationship with Schedule, but the user field was removed. However, the ScheduleAlarm entity in saveAlarmSettings still references user. Verify that removing the user field from ScheduleRepeat is intentional and doesn't break existing functionality or queries that depend on it.
| List<ScheduleAlarm> customAlarms = scheduleAlarmRepository.findAll().stream() | ||
| .filter(alarm -> alarm.getAlarmTimeType() == AlarmTimeType.CUSTOM) | ||
| .filter(alarm -> alarm.getCustomMinutesBefore() != null) | ||
| .filter(alarm -> { | ||
| LocalDateTime alarmTime = alarm.getCustomMinutesBefore(); | ||
| return !alarmTime.isBefore(checkStart) && !alarmTime.isAfter(checkEnd); | ||
| }) | ||
| .toList(); |
Copilot
AI
Dec 3, 2025
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.
Calling findAll() loads all schedule alarms into memory, which is inefficient. Create a custom query method in ScheduleAlarmRepository to filter custom alarms by time range at the database level to improve performance and reduce memory usage.
| List<ScheduleAlarm> customAlarms = scheduleAlarmRepository.findAll().stream() | |
| .filter(alarm -> alarm.getAlarmTimeType() == AlarmTimeType.CUSTOM) | |
| .filter(alarm -> alarm.getCustomMinutesBefore() != null) | |
| .filter(alarm -> { | |
| LocalDateTime alarmTime = alarm.getCustomMinutesBefore(); | |
| return !alarmTime.isBefore(checkStart) && !alarmTime.isAfter(checkEnd); | |
| }) | |
| .toList(); | |
| List<ScheduleAlarm> customAlarms = scheduleAlarmRepository | |
| .findByAlarmTimeTypeAndCustomMinutesBeforeBetween( | |
| AlarmTimeType.CUSTOM, | |
| checkStart, | |
| checkEnd | |
| ); |
| INDEX idx_fcm_token_user (user_id), | ||
| INDEX idx_fcm_token_token (token) |
Copilot
AI
Dec 3, 2025
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.
The unique constraint on token already creates an implicit index, making idx_fcm_token_token redundant. Remove the explicit index to avoid duplication.
| INDEX idx_fcm_token_user (user_id), | |
| INDEX idx_fcm_token_token (token) | |
| INDEX idx_fcm_token_user (user_id) |
🤖 GPT PR Review결론: request-changes 요약: The PR lacks a detailed description and does not address potential security, contract, exception, performance, or testing issues. 주요 지적
제안 테스트
|
No description provided.