Develop - #216
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a study attendance history tracking feature, including a new JPA entity, repository, public controller endpoint, and updates to the check/uncheck attendance services to persist history. It also migrates the timetable client from the NEIS API to the DataGSM API, removing obsolete properties and controller parameters. The review feedback highlights critical improvement opportunities: optimizing the JPA delete method in StudyAttendanceHistoryRepository to use a bulk delete query for better performance, and reordering the database and Redis operations in both CheckStudyAttendanceServiceImpl and UncheckStudyAttendanceServiceImpl to prevent data inconsistency if a database transaction rolls back after Redis state changes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| fun deleteByUserIdAndAttendedDate( | ||
| userId: Long, | ||
| attendedDate: LocalDate, | ||
| ) |
There was a problem hiding this comment.
Spring Data JPA의 기본 deleteBy... 메서드는 대상을 먼저 조회(SELECT)한 후 개별적으로 삭제(DELETE)를 수행하므로 성능상 비효율적입니다. 단일 쿼리로 즉시 삭제할 수 있도록 @Modifying과 @Query를 사용한 벌크 삭제 쿼리로 변경하는 것을 권장합니다.
@Modifying
@Query("DELETE FROM StudyAttendanceHistoryJpaEntity h WHERE h.user.id = :userId AND h.attendedDate = :attendedDate")
fun deleteByUserIdAndAttendedDate(
@Param("userId") userId: Long,
@Param("attendedDate") attendedDate: LocalDate,
)References
- Prefer bulk delete operations over individual entity deletions when cleaning up related data to avoid performance overhead.
| studyRedisAdapter.checkAttendance(userId) | ||
| val today = LocalDate.now(clock) | ||
| if (!studyAttendanceHistoryRepository.existsByUserIdAndAttendedDate(userId, today)) { | ||
| studyAttendanceHistoryRepository.save( | ||
| StudyAttendanceHistoryJpaEntity(user = user, attendedDate = today), | ||
| ) | ||
| } |
There was a problem hiding this comment.
데이터베이스 저장(studyAttendanceHistoryRepository.save) 중 예외(예: 동시 요청으로 인한 Unique Constraint 위반 등)가 발생하면 트랜잭션이 롤백되지만, 이미 실행된 Redis 작업(studyRedisAdapter.checkAttendance)은 롤백되지 않아 데이터 일관성이 깨질 수 있습니다.
안전한 트랜잭션 처리를 위해 데이터베이스 쓰기 작업을 Redis 상태 변경 및 SSE 브로드캐스트보다 먼저 수행하도록 순서를 변경하는 것을 권장합니다.
| studyRedisAdapter.checkAttendance(userId) | |
| val today = LocalDate.now(clock) | |
| if (!studyAttendanceHistoryRepository.existsByUserIdAndAttendedDate(userId, today)) { | |
| studyAttendanceHistoryRepository.save( | |
| StudyAttendanceHistoryJpaEntity(user = user, attendedDate = today), | |
| ) | |
| } | |
| val today = LocalDate.now(clock) | |
| if (!studyAttendanceHistoryRepository.existsByUserIdAndAttendedDate(userId, today)) { | |
| studyAttendanceHistoryRepository.save( | |
| StudyAttendanceHistoryJpaEntity(user = user, attendedDate = today), | |
| ) | |
| } | |
| studyRedisAdapter.checkAttendance(userId) |
| studyRedisAdapter.cancelAttendance(userId) | ||
| studyAttendanceHistoryRepository.deleteByUserIdAndAttendedDate(userId, LocalDate.now(clock)) |
There was a problem hiding this comment.
데이터베이스 삭제 작업 중 예외가 발생할 경우 트랜잭션은 롤백되지만, 이미 실행된 Redis 작업(studyRedisAdapter.cancelAttendance)은 롤백되지 않아 상태 불일치가 발생할 수 있습니다.
데이터베이스 작업을 먼저 수행한 후 Redis 상태를 변경하도록 순서를 변경하는 것을 권장합니다.
| studyRedisAdapter.cancelAttendance(userId) | |
| studyAttendanceHistoryRepository.deleteByUserIdAndAttendedDate(userId, LocalDate.now(clock)) | |
| studyAttendanceHistoryRepository.deleteByUserIdAndAttendedDate(userId, LocalDate.now(clock)) | |
| studyRedisAdapter.cancelAttendance(userId) |
#️⃣연관된 이슈
📝작업 내용
스크린샷 (선택)
💬리뷰 요구사항(선택)