[TEST] AI PR Review Test #135
Conversation
Walkthrough출석 저장 흐름을 이벤트 기반·비동기 방식으로 재구성하고, 출석 이력 표현을 “타임스탬프 목록”에서 “요청 상태(enum)”로 전환했다. 강의 오픈/참가자 캐시 관련 책임을 OpenLectureService로 이동했으며, 여러 포트/리포지토리/컨트롤러/테스트를 이에 맞게 수정했다. 일부 설정과 Redis 구성도 변경되었다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant StudentAttendanceService as StudentAttendanceSvc
participant OpenLectureService as OpenLectureSvc
participant AttendanceSaveService as AttendanceSaveSvc
participant Cache as AttendanceHistoryCacheService
participant Repo as AttendanceRepository
Client->>Controller: 출석 요청(lectureId)
Controller->>StudentAttendanceSvc: attend(lectureId)
StudentAttendanceSvc->>OpenLectureSvc: 오픈 강의/등록 검증
StudentAttendanceSvc-->>Cache: AttendanceRequestedEvent
StudentAttendanceSvc->>AttendanceSaveSvc: save(attendance) 비동기
AttendanceSaveSvc->>Repo: 저장
Repo-->>AttendanceSaveSvc: 저장 완료
AttendanceSaveSvc-->>StudentAttendanceSvc: 완료 콜백/퓨처
StudentAttendanceSvc-->>Cache: AttendanceSaveSucceedEvent
Note over Cache: 상태 SUCCESS 캐시
alt 저장 실패
AttendanceSaveSvc-->>StudentAttendanceSvc: 예외
StudentAttendanceSvc-->>Cache: AttendanceFailedEvent
Note over Cache: 상태 FAILED 캐시 및 로그
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @binary-ho, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a significant refactoring of the attendance management system, transitioning to an event-driven and asynchronous architecture. The primary goal is to enhance the robustness and responsiveness of attendance recording by decoupling the saving process from the initial request and providing more granular status tracking for attendance attempts.
Highlights
- Event-Driven Attendance Processing: Implemented new events (
AttendanceRequestedEvent,AttendanceSaveSucceedEvent,AttendanceFailedEvent) to manage the lifecycle of attendance requests, allowing for asynchronous processing and better error handling. - Asynchronous Attendance Saving: Introduced
AttendanceSaveServiceand integratedCompletableFutureto enable non-blocking persistence of attendance records, improving system responsiveness. - Enhanced Attendance Status Tracking: Replaced simple timestamp caching with a more comprehensive
AttendanceSaveRequestStatusenum (PROCESSING, SUCCESS, FAILED), providing real-time feedback on attendance attempts to students. - Refactored Caching Logic: Consolidated and streamlined caching mechanisms for open lectures and attendance history, moving responsibilities from deprecated services (
AttendeeCacheService,StudentAttendedEvent) into core services likeOpenLectureServiceandAttendanceHistoryCacheService. - Development Environment Updates: Adjusted Redis configuration to use
localhostfor development and updated test database configuration to PostgreSQL, reflecting changes in the development setup.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
안녕하세요. 코드 변경사항에 대한 리뷰를 공유드립니다. 이번 PR은 출석 처리 로직을 비동기 및 이벤트 기반 아키텍처로 리팩터링하는 중요한 변경을 포함하고 있습니다. 아키텍처 개선 방향은 좋지만, 현재 코드에는 컴파일을 막는 여러 심각한 버그와 보안 취약점이 발견되었습니다. 특히, AttendanceSaveRequestStatus enum의 로직 오류, StudentAttendanceService의 무한 재귀, 테스트 설정 파일에 하드코딩된 데이터베이스 인증 정보 등은 반드시 수정이 필요합니다. 아래에 각 파일별로 상세한 리뷰 내용을 정리했으니 확인 부탁드립니다.
| PROCESSING { | ||
| @Override | ||
| public void cache( | ||
| RedisTemplate<String, String> redisTemplate, String key, String value) { | ||
| String savedValue = redisTemplate.opsForValue().getAndDelete(key); | ||
| if (SUCCESS.name().equals(savedValue)) | ||
| redisTemplate.opsForValue().set(key, value); | ||
| } | ||
| }, | ||
|
|
||
| SUCCESS { | ||
| @Override | ||
| public void cache( | ||
| RedisTemplate<String, String> redisTemplate, String key, String value) { | ||
| String savedValue = redisTemplate.opsForValue().getAndDelete(key); | ||
| redisTemplate.opsForValue().set(key, savedValue, value); | ||
| } | ||
| }, | ||
|
|
||
| FAILED { | ||
| @Override | ||
| public void cache( | ||
| RedisTemplate<String, String> redisTemplate, String key, String value) { | ||
| redisTemplate.opsForSet().add(key, value); | ||
| } | ||
| }, | ||
|
|
||
| NO_REQUEST { | ||
| @Override | ||
| public void cache( | ||
| RedisTemplate<String, String> redisTemplate, String key, String value) { | ||
| redisTemplate.opsForSet().add(key, value); | ||
| } | ||
| }; | ||
|
|
||
| public abstract void cache( | ||
| RedisTemplate<String, String> redisTemplate, String key, String value); | ||
|
|
||
| public boolean canCache(String originalValue) { | ||
| this.name() | ||
| } | ||
| } |
There was a problem hiding this comment.
이 enum 클래스에 몇 가지 심각한 문제가 있어 컴파일 및 실행이 불가능합니다.
PROCESSING상태 (17-18행):if문에 중괄호{}가 없어redisTemplate.opsForValue().set(key, value);라인이 조건과 상관없이 항상 실행됩니다.SUCCESS상태 (27행):redisTemplate.opsForValue().set(key, savedValue, value)는 유효하지 않은 메서드 호출입니다.set메서드는 이런 형태의 파라미터를 받지 않습니다.- 데이터 타입 불일치:
PROCESSING,SUCCESS상태에서는opsForValue()(String)를 사용하고,FAILED,NO_REQUEST상태에서는opsForSet()(Set)을 사용하여 같은 키에 다른 데이터 타입을 사용하려고 합니다. 이는 Redis에서WRONGTYPE오류를 발생시킵니다. canCache메서드 (50-52행): 메서드가 구현되지 않았고boolean값을 반환해야 하지만 반환문이 없어 컴파일 오류가 발생합니다.
전체적인 로직을 재검토하고 수정해야 합니다.
| public boolean canCache(AttendanceSaveRequestStatus newValue) { | ||
|
|
||
| } |
There was a problem hiding this comment.
canCache 메서드는 boolean 값을 반환해야 하지만, 구현이 비어있고 반환문이 없어 컴파일 오류가 발생합니다. 메서드를 구현하거나, 아직 사용하지 않는다면 임시로 return false; 와 같이 기본 값을 반환하도록 수정해야 합니다.
public boolean canCache(AttendanceSaveRequestStatus newValue) {
// TODO: Implement caching strategy
return false;
}| public void cache(AttendanceHistory attendanceHistory) { | ||
| attendanceHistory.getAttendanceSaveRequestStatus(); | ||
| String key = attendanceHistory.getKey(); | ||
| String savedStatus = redisTemplate.opsForValue().get(key); | ||
| redisTemplate.opsForSet() | ||
| .add(key, attendanceHistory.getTimestamp()); | ||
| redisTemplate.expire(key, ATTENDANCE_HISTORY_EXPIRE_HOUR, TimeUnit.HOURS); | ||
| } |
There was a problem hiding this comment.
cache 메서드의 구현에 문제가 있습니다.
attendanceHistory.getTimestamp()(40행)를 호출하고 있는데,AttendanceHistory클래스에서timestamp필드와 관련 메서드가 제거되어 컴파일 오류가 발생합니다.attendanceSaveRequestStatus와savedStatus변수를 읽어오지만 사용하지 않고 있습니다.- 새로운 출석 상태(
PROCESSING,SUCCESS,FAILED)에 따른 캐싱 로직이 반영되지 않았습니다.AttendanceSaveRequestStatusenum에 정의된 로직과 일관되게 수정이 필요합니다.
| datasource: | ||
| url: jdbc:h2:~/mem-data; | ||
| driverClassName: org.h2.Driver | ||
| username: sa | ||
| password: | ||
| url: jdbc:postgresql://jinho-database.ckddmrlccdc1.ap-northeast-2.rds.amazonaws.com:5432/imhere-dev | ||
| username: postgres | ||
| password: d0YhFa6o3HJ8GqOzKhF5 | ||
| # driver-class-name: org.postgresql.Drive |
There was a problem hiding this comment.
데이터베이스 및 Redis 연결 정보(URL, 사용자 이름, 비밀번호)가 소스 코드에 하드코딩되어 있습니다. 이는 심각한 보안 취약점입니다. 비밀번호와 같은 민감한 정보는 환경 변수나 외부 설정 파일, 또는 Jasypt와 같은 암호화 라이브러리를 사용하여 안전하게 관리해야 합니다. 테스트 환경이라도 민감한 정보는 소스 코드에 포함하지 않는 것이 좋습니다.
| public RedisConnectionFactory redisConnectionFactory() { | ||
| RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort); | ||
| // RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort); | ||
| RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration("localhost", 6379); |
There was a problem hiding this comment.
Redis 연결 정보가 하드코딩되어 있습니다. 이전처럼 @Value 어노테이션을 사용하여 application.yml 파일에서 설정을 읽어오도록 수정해야 합니다. 이렇게 하면 다른 환경(개발, 운영 등)에서 유연하게 설정을 변경할 수 있습니다.
| RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration("localhost", 6379); | |
| RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort); |
| logAttendanceFailed(studentId, lectureId, event.getException()); | ||
| } | ||
|
|
||
| private void logAttendanceFailed(Long studentId, Long lectureId, RuntimeException exception) { |
There was a problem hiding this comment.
logAttendanceFailed 메서드의 exception 파라미터 타입이 RuntimeException으로 선언되어 있습니다. 하지만 AttendanceFailedEvent는 Throwable 타입의 예외를 가지고 있어, Error와 같은 RuntimeException이 아닌 예외가 발생할 경우 ClassCastException이 발생할 수 있습니다. 파라미터 타입을 Throwable로 변경하여 안정성을 높이는 것이 좋습니다.
| private void logAttendanceFailed(Long studentId, Long lectureId, RuntimeException exception) { | |
| private void logAttendanceFailed(Long studentId, Long lectureId, Throwable exception) { |
| // TODO : 테스트용 | ||
| @Transactional | ||
| public AttendanceNumberResponse openLectureAndGenerateAttendanceNumber(Long lectureId, int attendanceNumber) { | ||
| Lecture lecture = lectureRepository.findById(lectureId) | ||
| .orElseThrow(() -> LectureNotFoundException.EXCEPTION); | ||
| // authenticationHelper.verifyRequestMemberLogInMember(lecture.getMember().getId()); | ||
|
|
||
| lecture.setLectureState(LectureState.OPEN); | ||
| lecture.setLastOpeningTime(seoulDateTimeHolder.getSeoulDateTime()); | ||
|
|
||
| saveOpenLecture(lecture, attendanceNumber); | ||
| cacheAttendee(lecture); | ||
|
|
||
| log.info("[강의 OPEN] {} ({}), 출석 번호 : " + attendanceNumber | ||
| , lecture::getLectureName, lecture::getId); | ||
|
|
||
| return new AttendanceNumberResponse(attendanceNumber); | ||
| } |
There was a problem hiding this comment.
테스트 목적으로 보이는 openLectureAndGenerateAttendanceNumber 메서드가 프로덕션 코드에 포함되어 있습니다. (// TODO : 테스트용 주석). 특히 authenticationHelper.verifyRequestMemberLogInMember 인증 로직이 주석 처리되어 있어, 이 메서드가 노출될 경우 보안에 취약할 수 있습니다. 테스트용 코드는 테스트 코드셋(src/test/java)으로 옮기거나, 프로덕션 빌드에서는 제외되도록 설정하는 것이 안전합니다.
| public Boolean isStudentOpenLectureExist(Long studentId, Long lectureId) { | ||
| Set<Long> openLectureIds = findAllAttendLectureId(studentId); | ||
| return openLectureIds.contains(lectureId); | ||
| } |
There was a problem hiding this comment.
isStudentOpenLectureExist 메서드가 학생이 수강하는 모든 열린 강의 ID를 가져온 후, 애플리케이션 메모리에서 특정 강의 ID가 포함되어 있는지 확인하고 있습니다. 수강하는 강의가 많아질 경우 비효율적일 수 있습니다. Redis의 SISMEMBER 명령어를 사용하면 네트워크 왕복 및 메모리 사용을 줄여 더 효율적으로 확인할 수 있습니다.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🔭 Outside diff range comments (3)
src/main/java/gdsc/binaryho/imhere/core/auth/application/EmailVerificationService.java (2)
24-35: 트랜잭션 내부에서 이메일 발송 + 저장 순서 역전: 롤백 시 불일치/유실 위험현재 트랜잭션 안에서 메일을 먼저 보내고, 그 다음 코드를 저장합니다. 저장 실패나 롤백이 발생하면 사용자는 “검증 불가능한 코드”를 받게 됩니다. 최소한 저장 후 발송으로 순서를 바꾸고, 가능하면 커밋 이후(AfterCommit)에 발송되도록 이벤트 리스너로 분리하는 것이 안전합니다.
아래는 즉시 가능한 최소 수정입니다(발송을 저장 이후로 이동):
public void sendVerificationCodeByEmail(String recipient) { emailFormValidator.validateEmailForm(recipient); String verificationCode = UUID.randomUUID().toString(); - mailSender.sendEmailWithVerificationCode(recipient, verificationCode); - saveVerificationCodeWithRecipientAsKey(recipient, verificationCode); + saveVerificationCodeWithRecipientAsKey(recipient, verificationCode); + mailSender.sendEmailWithVerificationCode(recipient, verificationCode); log.info("[인증 이메일 발송] {}, 인증 번호 : {}", () -> recipient, () -> verificationCode); }권장(보다 안전): 저장만 트랜잭션으로 처리하고, ApplicationEventPublisher + @TransactionalEventListener(phase = AFTER_COMMIT)로 메일 발송을 분리해 커밋 후에만 발송되게 해주세요. 원하시면 이벤트/리스너 코드까지 생성해드리겠습니다.
33-35: 인증번호를 info 레벨 로그로 남기면 보안사고로 직결됩니다인증/검증에 사용되는 코드를 평문으로 info 로그에 남기지 마세요. 최소 마스킹 또는 제외가 필요합니다.
적용 예:
- log.info("[인증 이메일 발송] {}, 인증 번호 : {}", - () -> recipient, () -> verificationCode); + log.info("[인증 이메일 발송] {}", () -> recipient); + // 필요 시 디버그에서만 마스킹된 값 출력 + log.debug("[인증 이메일 발송 - masked] {}, 인증 번호 : ****", () -> recipient);src/main/java/gdsc/binaryho/imhere/core/lecture/application/LectureService.java (1)
127-143: 메서드 중복을 제거하세요
openLectureAndGenerateAttendanceNumber메서드의 두 버전이 거의 동일한 로직을 포함하고 있습니다. 코드 중복을 제거하기 위해 리팩토링이 필요합니다.@Transactional public AttendanceNumberResponse openLectureAndGenerateAttendanceNumber(Long lectureId) { + int attendanceNumber = generateRandomNumber(); + return openLectureAndGenerateAttendanceNumber(lectureId, attendanceNumber, true); +} + +private AttendanceNumberResponse openLectureAndGenerateAttendanceNumber( + Long lectureId, int attendanceNumber, boolean verifyAuth) { Lecture lecture = lectureRepository.findById(lectureId) .orElseThrow(() -> LectureNotFoundException.EXCEPTION); - authenticationHelper.verifyRequestMemberLogInMember(lecture.getMember().getId()); + + if (verifyAuth) { + authenticationHelper.verifyRequestMemberLogInMember(lecture.getMember().getId()); + } lecture.setLectureState(LectureState.OPEN); lecture.setLastOpeningTime(seoulDateTimeHolder.getSeoulDateTime()); - int attendanceNumber = generateRandomNumber(); saveOpenLecture(lecture, attendanceNumber); cacheAttendee(lecture); log.info("[강의 OPEN] {} ({}), 출석 번호 : " + attendanceNumber , lecture::getLectureName, lecture::getId); return new AttendanceNumberResponse(attendanceNumber); }
🧹 Nitpick comments (21)
src/main/resources/application-dev.yml (1)
26-28: hibernate.format_sql 주석 처리 의도 확인개발 환경에서 SQL 포매팅을 끈 것으로 보입니다. 현재 org.hibernate.SQL 로그 레벨이 debug이므로, 출력은 유지되나 가독성은 떨어질 수 있습니다. 의도된 변경인지 확인 부탁드립니다. 필요 시 properties 하위로 옮겨 정확히 활성화할 수 있습니다.
아래처럼 표준 키로 재활성화 가능합니다.
spring: jpa: hibernate: ddl-auto: none -# properties: -# hibernate: -# format_sql: true + properties: + hibernate: + format_sql: truesrc/test/resources/application.yml (1)
16-18: 테스트에서 ddl-auto: none 사용 시 스키마 준비 보장 필요마이그레이션 도구(Flyway/Liquibase)로 테스트 스키마를 항상 준비한다면 괜찮습니다. 그렇지 않다면 테스트가 스키마 미존재로 실패할 수 있습니다. 테스트 프로파일에서는 create-drop 혹은 마이그레이션 적용을 명시적으로 보장하는 구성이 권장됩니다.
예: 마이그레이션이 없다면 임시로 아래처럼 전환
-spring: - jpa: - hibernate: - ddl-auto: none +spring: + jpa: + hibernate: + ddl-auto: create-dropsrc/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceSaveSucceedEvent.java (1)
6-12: 자바 17+ 환경이라면 record로 간소화 가능선택 사항이지만, 보일러플레이트 제거와 의도 표현에 도움이 됩니다.
-@Getter -@RequiredArgsConstructor -public class AttendanceSaveSucceedEvent { - - private final long lectureId; - private final long studentId; -} +public record AttendanceSaveSucceedEvent(long lectureId, long studentId) {}src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceFailedEvent.java (1)
6-13: 이벤트 페이로드에 Throwable 직접 포함은 직렬화/로깅 리스크가 큼내부 이벤트라 하더라도 예외 객체를 그대로 들고 다니면(특히 캐시/메시징 경계 넘어갈 여지) 직렬화 실패, 과도한 페이로드, 그리고 로그/모니터링으로의 민감 정보(메시지/스택트레이스) 유출 리스크가 있습니다. 예외 클래스/메시지만 노출하거나, 예외 필드는 JSON 직렬화에서 제외하는 것을 권장합니다.
적용을 고려할 수 있는 최소 변경안 예시(직렬화 회피):
package gdsc.binaryho.imhere.core.attendance.application; +import com.fasterxml.jackson.annotation.JsonIgnore; import lombok.Getter; import lombok.RequiredArgsConstructor; @Getter @RequiredArgsConstructor public class AttendanceFailedEvent { private final long lectureId; private final long studentId; - private final Throwable exception; + @JsonIgnore + private final Throwable exception; }또는 이벤트에는
errorType/errorMessage만 전달하고, 실제 예외 객체는 핸들러 내부에서만 다루는 방식으로 단순화하는 것도 좋습니다. 현재 이 이벤트가 외부 메시지 브로커/Redis 등에 퍼블리시되거나 JSON 변환될 가능성이 있는지 확인 부탁드립니다. 해당 경계가 존재한다면 필수적으로 예외 필드의 직렬화 제외 또는 구조 변경이 필요합니다.src/main/java/gdsc/binaryho/imhere/core/lecture/application/port/AttendeeCacheRepository.java (1)
8-8: Boolean 대신 primitive boolean 사용 권장 및 시그니처 일관성 유지현재 인터페이스와 구현체들이 모두
openLectureIds.contains(...)로 항상 non-null 값을 반환하므로, 실제로NullPointerException리스크는 없지만, 반환값이 2치(존재/비존재)로 고정되어 있는 상황에서는 원시형boolean이 더 적합합니다. 인터페이스와 이를 구현·호출하는 클래스들의 메서드 시그니처를 통일하여 아래와 같이 변경을 권장드립니다.
- src/main/java/gdsc/binaryho/imhere/core/lecture/application/port/AttendeeCacheRepository.java
- 8줄:
Boolean isStudentOpenLectureExist(...)→boolean isStudentOpenLectureExist(...)- src/main/java/gdsc/binaryho/imhere/core/lecture/infrastructure/AttendeeCacheRedisRepository.java
- 25줄:
public Boolean isStudentOpenLectureExist(...)→public boolean isStudentOpenLectureExist(...)- src/test/java/gdsc/binaryho/imhere/mock/fakerepository/FakeAttendeeCacheRepository.java
- 16줄:
public Boolean isStudentOpenLectureExist(...)→public boolean isStudentOpenLectureExist(...)- src/main/java/gdsc/binaryho/imhere/core/lecture/application/OpenLectureService.java
- 40줄:
public Boolean isStudentOpenLectureExist(...)→public boolean isStudentOpenLectureExist(...)메서드명(
existsOpenLectureForStudent등) 변경은 팀 컨벤션에 따라 선택적으로 적용하시면 됩니다.src/test/java/gdsc/binaryho/imhere/mock/fakerepository/FakeAttendeeCacheRepository.java (1)
15-19: 인터페이스 변경 시 구현체도 boolean으로 정합성 유지상위 인터페이스를
boolean으로 변경하는 경우, 테스트용 페이크 구현체도 시그니처를 일치시켜 주세요.다음과 같이 수정하면 됩니다.
- public Boolean isStudentOpenLectureExist(Long studentId, Long lectureId) { + public boolean isStudentOpenLectureExist(Long studentId, Long lectureId) { Set<Long> openLectureIds = findAllAttendLectureId(studentId); return openLectureIds.contains(lectureId); }src/main/java/gdsc/binaryho/imhere/core/attendance/controller/AttendanceController.java (1)
56-62: API 요약 문구를 “기록” → “상태”로 업데이트 추천이제 응답이 타임스탬프 목록이 아니라 상태이므로, 스웨거 요약 문구도 “최근 출석 상태”로 맞추면 혼란을 줄일 수 있습니다.
- @Operation(summary = "학생이 듣는 강의의 최근 출석 기록을 가져오는 API") + @Operation(summary = "학생이 듣는 강의의 최근 출석 상태를 가져오는 API") @GetMapping(params = {STUDENT_ROLE_PARAM, RECENT_PARAM}) public ResponseEntity<StudentRecentAttendanceResponse> getStudentRecentAttendance( @PathVariable("lecture_id") Long lectureId) { return ResponseEntity.ok( studentAttendanceService.getStudentRecentAttendanceStatus(lectureId)); }src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceSaveService.java (1)
25-31: 로그 레벨을 재검토하세요출석 완료는 정상적인 비즈니스 흐름이므로
info레벨보다는debug레벨이 적절할 수 있습니다. 운영 환경에서 너무 많은 로그가 생성될 가능성이 있습니다.- log.info("[출석 완료] {}({}) , 학생 : {} ({})", + log.debug("[출석 완료] {}({}) , 학생 : {} ({})", lecture::getLectureName, lecture::getId, student::getUnivId, student::getName);src/main/java/gdsc/binaryho/imhere/core/lecture/application/OpenLectureService.java (2)
34-37: 네이밍 일관성 제안: 복수형과 도메인 용어 통일메서드명과 포트의 메서드명이 다소 어긋납니다.
- 현재: findAllOpenLectureIdByStudentId → 내부 위임: findAllAttendLectureId
- 제안: findAllOpenLectureIdsByStudentId 또는 findAllAttendLectureIdsByStudentId 로 복수형(Id → Ids) 및 도메인 용어 통일
큰 문제는 아니지만 가독성과 유지보수성 향상에 도움이 됩니다.
44-49: 비동기 캐시 이벤트 핸들러에 입력 검증 및 예외 로깅 추가 권장
@Async설정은src/main/java/gdsc/binaryho/imhere/ImhereApplication.java에@EnableAsync로 이미 활성화되어 있어 정상 작동합니다.AFTER_COMMIT+REQUIRES_NEW조합 자체는 적절하나, void 반환 비동기 메서드 특성상 예외가 사일런트하게 처리될 수 있으므로 아래와 같은 선택적 리팩토링을 권장드립니다.
- 입력값(null, 빈 리스트) 검증 추가
- 예외 발생 시 컨텍스트 정보와 함께 로깅 후 예외 재발생
@@ src/main/java/gdsc/binaryho/imhere/core/lecture/application/OpenLectureService.java:44 @Async @Transactional(propagation = Propagation.REQUIRES_NEW) @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) public void cacheStudent(AttendeeCacheEvent event) { - attendeeCacheRepository.cache(event.getLectureId(), event.getStudentIds()); + if (event == null || event.getLectureId() == null + || event.getStudentIds() == null || event.getStudentIds().isEmpty()) { + return; + } + try { + attendeeCacheRepository.cache(event.getLectureId(), event.getStudentIds()); + } catch (RuntimeException ex) { + // TODO: @Slf4j 등으로 로거 주입 후 아래 로깅 적용 권장 + log.warn("Attendee cache update failed. lectureId={}, count={}", + event.getLectureId(), event.getStudentIds().size(), ex); + throw ex; + } }src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceHistoryCacheService.java (3)
36-46: 오버로드된 이벤트 핸들러 메서드 네이밍 가독성 제안동일 메서드명(saveAttendance) 오버로드는 가능하지만 가독성/검색성 측면에서 구체적 네이밍 권장:
- saveAttendanceSucceeded(AttendanceSaveSucceedEvent event)
- saveAttendanceFailed(AttendanceFailedEvent event)
이름만으로 이벤트 흐름 추적이 쉬워집니다.
47-57: 실패 로깅 수준 및 스택트레이스 포함 권장현재 실패 로그는 info 레벨이며 스택트레이스를 포함하지 않습니다. 장애 원인 분석을 위해 WARN/ERROR와 Exception 전달을 권장합니다.
아래 보조 메서드 수정과 함께 적용하세요.
59-64: logAttendanceFailed 포맷 개선 및 예외 전달메시지에 스택트레이스가 포함되도록 exception 객체를 함께 로깅하세요.
적용 diff:
- private void logAttendanceFailed(Long studentId, Long lectureId, RuntimeException exception) { - String exceptionName = exception.getClass().getName(); - String exceptionMessage = exception.getMessage(); - log.info("[출석 실패] 학생 id {}, 수업 id : {}, 예외 이름 : {}, 예외 메시지 : {}", - studentId, lectureId, exceptionName, exceptionMessage); - } + private void logAttendanceFailed(Long studentId, Long lectureId, RuntimeException exception) { + log.warn("[출석 실패] 학생 id: {}, 수업 id: {}", studentId, lectureId, exception); + }src/main/java/gdsc/binaryho/imhere/core/attendance/application/StudentAttendanceService.java (8)
160-168: 기본 ForkJoinPool 사용 대신 전용 Executor 사용 권장 + thenRunAsync 적용
CompletableFuture.runAsync의 기본 풀(ForkJoinPool.commonPool)은 애플리케이션의 다른 비동기 작업과 간섭될 수 있습니다. 전용 Executor/TaskExecutor를 주입해 사용하면 격리와 모니터링이 수월합니다. 또한 이어지는 콜백도 동일 Executor에서 실행되도록thenRunAsync를 사용하세요.- CompletableFuture.runAsync( - () -> attendanceSaveService.save(attendance) - ).thenRun( - () -> publishAttendanceSaveSucceedEvent(attendance) - ).exceptionally( - exception -> publishAttendanceFailedEvent(attendance, exception) - ); + CompletableFuture.runAsync( + () -> attendanceSaveService.save(attendance), + attendanceSaveExecutor + ).thenRunAsync( + () -> publishAttendanceSaveSucceedEvent(attendance), + attendanceSaveExecutor + ).exceptionally( + exception -> publishAttendanceFailedEvent(attendance, exception) + );클래스 외부 변경(참고):
// 필드 추가 private final java.util.concurrent.Executor attendanceSaveExecutor; // 구성 예시 (Configuration 클래스) @Bean @Qualifier("attendanceSaveExecutor") public Executor attendanceSaveExecutor(TaskExecutorBuilder builder) { return builder .corePoolSize(4) .maxPoolSize(16) .queueCapacity(1000) .threadNamePrefix("attendance-save-") .build(); }
87-92: 오버로드된 메서드명으로 가독성 저하 — 의미가 드러나는 이름으로 변경 제안퍼블릭 메서드와 프라이빗 메서드가 동일한 이름을 오버로드하고 있어 읽기 흐름이 헷갈립니다. DB 조회 기반으로 상태를 계산한다는 의미가 드러나도록 이름을 바꾸면 가독성이 좋아집니다.
- private StudentRecentAttendanceResponse getStudentRecentAttendanceStatus(Long lectureId, Long studentId) { + private StudentRecentAttendanceResponse computeRecentAttendanceStatusFromDb(Long lectureId, Long studentId) {- return getStudentRecentAttendanceStatus(lectureId, studentId); + return computeRecentAttendanceStatusFromDb(lectureId, studentId);Also applies to: 80-81
109-117: 불필요한 래퍼 타입(Boolean) → 기본형(boolean)으로 단순화Null을 반환하지 않는 메서드라면 굳이
Boolean을 쓸 이유가 없습니다. 기본형으로 단순화하면 오토박싱 비용과 잠재적 null 문제를 제거할 수 있습니다.- private Boolean isRecentAttendancesExist(Long lectureId, Long studentId) { + private boolean isRecentAttendancesExist(Long lectureId, Long studentId) {
205-208: 자정 기준 시간 계산을 더 명확하게: truncatedTo(DAYS) 사용 제안연-월-일 기준의 시작 시각을 만들 때는
truncatedTo(ChronoUnit.DAYS)가 의도를 더 잘 드러내고 나노초까지 0으로 정규화합니다.- return seoulDateTimeHolder.from(milliseconds) - .withHour(0).withMinute(0).withSecond(0); + return seoulDateTimeHolder.from(milliseconds) + .truncatedTo(java.time.temporal.ChronoUnit.DAYS);
99-101: Between 경계 포함 여부 확인 요청Spring Data JPA의
Between는 보통 양 끝을 포함합니다. 그럴 경우timestamp.plusDays(1)의 정확히 00:00:00 데이터가 포함될 수 있습니다. 도메인 요구사항에 따라 다음을 검토해 주세요.
- 포함이 의도라면 그대로 유지
- 배타 상단 경계가 의도라면 리포지토리 메서드를
GreaterThanEqual+LessThan조합으로 분리하는 방안 고려필요하시면 리포지토리 시그니처 및 쿼리 메서드 리팩터링 패치도 만들어 드리겠습니다.
170-175: 이벤트 게시 코드 중복 — 간단한 헬퍼로 중복 제거 가능이벤트 생성/게시 로직이 3곳에서 반복됩니다. 작은 헬퍼를 두어 중복을 줄이면 유지보수가 쉬워집니다. 예:
publish(Object event)또는withIds(Attendance, BiFunction<Long,Long,Object>).예시:
private void publish(Object event) { eventPublisher.publishEvent(event); } private void publishRequested(Attendance a) { publish(new AttendanceRequestedEvent(a.getLecture().getId(), a.getStudent().getId())); }Also applies to: 177-183, 185-190
54-54: 상수 선언 컨벤션 정렬Java 컨벤션에 맞춰
static final순서를 권장합니다.- private final static Duration RECENT_TIME = Duration.ofHours(1L); + private static final Duration RECENT_TIME = Duration.ofHours(1L);
177-183: 실패 이벤트 발행 외에 로깅 추가 제안이벤트 리스너에서 로깅하겠지만, 실패 지점에서 바로 원인/컨텍스트를 남겨두면 트러블슈팅에 도움이 됩니다.
@Slf4j추가 후 에러 로그를 남기는 것을 권장합니다.예시:
// 클래스에 추가 // @Slf4j private Void publishAttendanceFailedEvent(Attendance attendance, Throwable throwable) { // log.error("Attendance save failed. lectureId={}, studentId={}", lectureId, studentId, throwable); ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (28)
src/main/java/gdsc/binaryho/imhere/config/redis/RedisConfig.java(1 hunks)src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceFailedEvent.java(1 hunks)src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceHistoryCacheService.java(2 hunks)src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceRequestedEvent.java(1 hunks)src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceSaveRequestStatus.java(1 hunks)src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceSaveService.java(1 hunks)src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceSaveSucceedEvent.java(1 hunks)src/main/java/gdsc/binaryho/imhere/core/attendance/application/StudentAttendanceService.java(4 hunks)src/main/java/gdsc/binaryho/imhere/core/attendance/application/StudentAttendedEvent.java(0 hunks)src/main/java/gdsc/binaryho/imhere/core/attendance/application/port/AttendanceHistoryCacheRepository.java(1 hunks)src/main/java/gdsc/binaryho/imhere/core/attendance/controller/AttendanceController.java(1 hunks)src/main/java/gdsc/binaryho/imhere/core/attendance/domain/AttendanceHistory.java(2 hunks)src/main/java/gdsc/binaryho/imhere/core/attendance/infrastructure/AttendanceHistoryCacheStrategy.java(1 hunks)src/main/java/gdsc/binaryho/imhere/core/attendance/infrastructure/AttendanceRedisCacheRepository.java(2 hunks)src/main/java/gdsc/binaryho/imhere/core/attendance/model/response/StudentRecentAttendanceResponse.java(1 hunks)src/main/java/gdsc/binaryho/imhere/core/auth/application/EmailVerificationService.java(1 hunks)src/main/java/gdsc/binaryho/imhere/core/lecture/application/AttendeeCacheService.java(0 hunks)src/main/java/gdsc/binaryho/imhere/core/lecture/application/LectureService.java(3 hunks)src/main/java/gdsc/binaryho/imhere/core/lecture/application/OpenLectureService.java(1 hunks)src/main/java/gdsc/binaryho/imhere/core/lecture/application/port/AttendeeCacheRepository.java(1 hunks)src/main/java/gdsc/binaryho/imhere/core/lecture/infrastructure/AttendeeCacheRedisRepository.java(1 hunks)src/main/resources/application-dev.yml(1 hunks)src/test/java/gdsc/binaryho/imhere/core/lecture/application/AttendeeCacheServiceTest.java(2 hunks)src/test/java/gdsc/binaryho/imhere/mock/TestContainer.java(2 hunks)src/test/java/gdsc/binaryho/imhere/mock/fakerepository/FakeAttendanceHistoryCacheRepository.java(2 hunks)src/test/java/gdsc/binaryho/imhere/mock/fakerepository/FakeAttendeeCacheRepository.java(1 hunks)src/test/java/gdsc/binaryho/imhere/presentation/AttendanceControllerTest.java(1 hunks)src/test/resources/application.yml(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/gdsc/binaryho/imhere/core/lecture/application/AttendeeCacheService.java
- src/main/java/gdsc/binaryho/imhere/core/attendance/application/StudentAttendedEvent.java
🧰 Additional context used
🧬 Code Graph Analysis (11)
src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceRequestedEvent.java (3)
src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceFailedEvent.java (1)
Getter(6-13)src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceSaveSucceedEvent.java (1)
Getter(6-12)src/main/java/gdsc/binaryho/imhere/core/attendance/domain/AttendanceHistory.java (1)
Getter(13-43)
src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceSaveSucceedEvent.java (2)
src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceFailedEvent.java (1)
Getter(6-13)src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceRequestedEvent.java (1)
Getter(6-12)
src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceFailedEvent.java (4)
src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceRequestedEvent.java (1)
Getter(6-12)src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceSaveSucceedEvent.java (1)
Getter(6-12)src/main/java/gdsc/binaryho/imhere/core/attendance/domain/AttendanceHistory.java (1)
Getter(13-43)src/main/java/gdsc/binaryho/imhere/core/attendance/model/response/StudentRecentAttendanceResponse.java (1)
Getter(7-16)
src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceHistoryCacheService.java (2)
src/main/java/gdsc/binaryho/imhere/core/attendance/infrastructure/AttendanceRedisCacheRepository.java (1)
Log4j2(13-49)src/main/java/gdsc/binaryho/imhere/core/attendance/application/StudentAttendanceService.java (1)
Service(37-209)
src/main/java/gdsc/binaryho/imhere/core/lecture/application/OpenLectureService.java (1)
src/main/java/gdsc/binaryho/imhere/core/attendance/application/StudentAttendanceService.java (1)
Service(37-209)
src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceSaveService.java (4)
src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceHistoryCacheService.java (1)
Log4j2(18-65)src/main/java/gdsc/binaryho/imhere/core/attendance/infrastructure/AttendanceRedisCacheRepository.java (1)
Log4j2(13-49)src/main/java/gdsc/binaryho/imhere/core/lecture/application/LectureService.java (1)
Log4j2(32-196)src/main/java/gdsc/binaryho/imhere/core/attendance/application/StudentAttendanceService.java (1)
Service(37-209)
src/test/java/gdsc/binaryho/imhere/core/lecture/application/AttendeeCacheServiceTest.java (1)
src/test/java/gdsc/binaryho/imhere/mock/TestContainer.java (1)
TestContainer(30-96)
src/main/java/gdsc/binaryho/imhere/core/attendance/model/response/StudentRecentAttendanceResponse.java (1)
src/main/java/gdsc/binaryho/imhere/core/attendance/domain/AttendanceHistory.java (1)
Getter(13-43)
src/main/java/gdsc/binaryho/imhere/core/attendance/domain/AttendanceHistory.java (1)
src/main/java/gdsc/binaryho/imhere/config/redis/RedisKeyConstants.java (1)
RedisKeyConstants(3-10)
src/main/java/gdsc/binaryho/imhere/core/attendance/application/StudentAttendanceService.java (4)
src/main/java/gdsc/binaryho/imhere/core/lecture/exception/LectureNotFoundException.java (1)
LectureNotFoundException(6-13)src/main/java/gdsc/binaryho/imhere/core/lecture/exception/LectureNotOpenException.java (1)
LectureNotOpenException(6-13)src/main/java/gdsc/binaryho/imhere/core/lecture/application/OpenLectureService.java (1)
Service(17-50)src/main/java/gdsc/binaryho/imhere/core/enrollment/exception/EnrollmentNotApprovedException.java (1)
EnrollmentNotApprovedException(6-13)
src/main/java/gdsc/binaryho/imhere/core/lecture/application/LectureService.java (1)
src/main/java/gdsc/binaryho/imhere/core/lecture/exception/LectureNotFoundException.java (1)
LectureNotFoundException(6-13)
🪛 Gitleaks (8.27.2)
src/test/resources/application.yml
34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (18)
src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceSaveSucceedEvent.java (1)
6-12: 이벤트 VO 구성 깔끔합니다필요 정보만 담은 불변 객체로 적절합니다. 현 상태로 충분히 명확합니다.
src/main/java/gdsc/binaryho/imhere/core/attendance/controller/AttendanceController.java (1)
61-61: LGTM — 서비스 메서드 위임 변경이 의도와 일치최근 출석 “상태”로 응답이 변경된 흐름에 맞춰
getStudentRecentAttendanceStatus로 위임한 점 적절합니다.src/test/java/gdsc/binaryho/imhere/presentation/AttendanceControllerTest.java (1)
82-82: 테스트 수정: 새로운 응답 스키마(AttendanceSaveRequestStatus) 반영
StudentRecentAttendanceResponse가 이제 타임스탬프 리스트가 아닌 enum 필드(recentAttendanceStatus)를 반환하므로, 테스트를 다음과 같이 갱신해 주세요.위치:
src/test/java/gdsc/binaryho/imhere/presentation/AttendanceControllerTest.java:82• 스텁 생성 및 반환
• GET 요청 URL 포맷 인자 제거
• JSONPath 단언을recentAttendanceStatus로 변경
• 새 enum import 추가@@ -82,12 +82,11 src/test/java/gdsc/binaryho/imhere/presentation/AttendanceControllerTest.java - long milliseconds = MOCK_ATTENDANCE.getTimestamp().toInstant(ZoneOffset.UTC).toEpochMilli(); - StudentRecentAttendanceResponse response = new StudentRecentAttendanceResponse( - List.of(MOCK_ATTENDANCE.getTimestamp().toString())); - given(studentAttendanceService.getStudentRecentAttendanceStatus(lectureId)) - .willReturn(response); + StudentRecentAttendanceResponse response = new StudentRecentAttendanceResponse( + AttendanceSaveRequestStatus.SUCCESS); + given(studentAttendanceService.getStudentRecentAttendanceStatus(lectureId)) + .willReturn(response); @@ - MockMvcRequestBuilders.get(String.format("/api/lecture/%d/attendance?" + STUDENT_ROLE_PARAM + "&" + RECENT_PARAM, lectureId, milliseconds)) + MockMvcRequestBuilders.get(String.format("/api/lecture/%d/attendance?" + STUDENT_ROLE_PARAM + "&" + RECENT_PARAM, lectureId)) @@ - .andExpect(jsonPath("$.timestamps").isNotEmpty()) - .andExpect(result -> { - String timestamp = JsonPath.read(result.getResponse().getContentAsString(), - "$.timestamps[0]"); - assertThat(MOCK_ATTENDANCE.getTimestamp().toString()).contains(timestamp); - }); + .andExpect(jsonPath("$.recentAttendanceStatus").value("SUCCESS"));추가 import:
import gdsc.binaryho.imhere.core.attendance.model.AttendanceSaveRequestStatus;src/main/java/gdsc/binaryho/imhere/core/lecture/infrastructure/AttendeeCacheRedisRepository.java (1)
24-28: 메서드 구현이 올바르게 작성되었습니다.새로운
isStudentOpenLectureExist메서드가 기존의findAllAttendLectureId를 재사용하여 효율적으로 구현되었으며, 인터페이스 요구사항을 올바르게 충족합니다.src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceRequestedEvent.java (1)
6-12: 이벤트 클래스 구조가 일관성 있게 설계되었습니다.다른 이벤트 클래스들(
AttendanceSaveSucceedEvent,AttendanceFailedEvent)과 동일한 구조와 패턴을 따르고 있어 일관성이 좋습니다. Lombok 애노테이션을 적절히 활용하여 boilerplate 코드를 줄였습니다.src/main/java/gdsc/binaryho/imhere/core/attendance/application/port/AttendanceHistoryCacheRepository.java (1)
3-3: 새로운 의존성이 적절하게 추가되었습니다.
AttendanceSaveRequestStatusimport가 새로운 API 메서드와 함께 추가되어 상태 기반 캐싱 전략으로의 전환을 지원합니다.src/test/java/gdsc/binaryho/imhere/core/lecture/application/AttendeeCacheServiceTest.java (2)
8-8: 서비스 리팩토링이 적절하게 반영되었습니다.
AttendeeCacheService에서OpenLectureService로의 전환이 테스트에 올바르게 적용되었습니다. 새로운 의존성(OpenLectureCacheRepository) 추가와 생성자 변경이 적절히 처리되었습니다.Also applies to: 19-19, 26-28
35-35: 메서드 호출 변경이 올바르게 적용되었습니다.
attendeeCacheService.cache()에서openLectureService.cacheStudent()로의 메서드 호출 변경이 새로운 서비스 구조에 맞게 올바르게 수정되었습니다.src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceSaveRequestStatus.java (1)
12-20: SUCCESS 상태를 덮어쓰지 못하는 로직에 문제가 있습니다Line 17에서 조건문이 있지만 Line 18이 항상 실행됩니다.
if블록에 중괄호가 없어서 의도와 다르게 동작합니다.@Override public void cache( RedisTemplate<String, String> redisTemplate, String key, String value) { String savedValue = redisTemplate.opsForValue().getAndDelete(key); - if (SUCCESS.name().equals(savedValue)) - redisTemplate.opsForValue().set(key, value); + if (!SUCCESS.name().equals(savedValue)) { + redisTemplate.opsForValue().set(key, value); + } }Likely an incorrect or invalid review comment.
src/main/java/gdsc/binaryho/imhere/core/lecture/application/OpenLectureService.java (3)
24-27: 조회 메서드에 readOnly 트랜잭션 적용 LGTM읽기 전용 트랜잭션으로 지정해 불필요한 flush 방지 및 성능/안정성 측면에서 적절합니다.
29-32: 출석번호 조회 readOnly 트랜잭션 적용 LGTMnull 허용을 통한 타임아웃 표현과 소비측 검증 흐름과도 잘 맞습니다.
39-42: Auto-unboxing NPE 위험 방지: Boolean → boolean 변경 및 안전 비교 적용OpenLectureService#isStudentOpenLectureExist에서 Boolean 반환 시 소비 측에서 오토언박싱(예:
!openLectureService.isStudentOpenLectureExist(...))을 하면 null일 때 NPE가 발생할 수 있습니다. 반환 타입을 primitive boolean으로 변경하고Boolean.TRUE.equals(...)를 사용해 안전 비교를 적용하세요. 호출부(StudentAttendanceService 등) 컴파일 오류는 없습니다.적용 diff:
- public Boolean isStudentOpenLectureExist(Long studentId, Long lectureId) { - return attendeeCacheRepository.isStudentOpenLectureExist(studentId, lectureId); - } + public boolean isStudentOpenLectureExist(Long studentId, Long lectureId) { + return Boolean.TRUE.equals( + attendeeCacheRepository.isStudentOpenLectureExist(studentId, lectureId) + ); + }src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceHistoryCacheService.java (1)
28-34: 요청 이벤트 기반 캐싱 전환 LGTMAFTER_COMMIT + REQUIRES_NEW로 본 트랜잭션과 분리하여 성능/일관성을 도모한 점 좋습니다. 대기 상태(Processing)로 초기 기록하는 도메인 모델링도 적절합니다.
src/test/java/gdsc/binaryho/imhere/mock/TestContainer.java (3)
71-73: OpenLectureService 의존성 주입 갱신 LGTMAttendeeCacheRepository 추가 주입이 테스트 컨테이너에 반영되었습니다.
84-87: StudentAttendanceService 생성자 변경 반영 LGTMLectureRepository 추가 인자와 히스토리 캐시 리포지토리 전달이 정합적으로 연결되었습니다.
91-94: LectureService 생성자 변경 반영 LGTMOpenLectureService를 주입하도록 업데이트된 부분이 전체 흐름과 일치합니다.
src/main/java/gdsc/binaryho/imhere/core/attendance/application/StudentAttendanceService.java (2)
60-67: 캐시 미스 분기 로직 검토 요청캐시 히트 시에는 별도
LectureState검증 없이 진행하고, 미스 시에는 수강 승인/오픈 검증을 거칩니다. 오픈 여부는findAttendanceNumber로도 간접 검증되지만, 예외 타입(예: NOT_OPEN vs TIME_EXCEEDED)이 분기별로 달라질 수 있습니다. 의도된 UX/오류 코드 설계인지 확인 부탁드립니다.
125-130: 출석번호 검증 흐름 적절함타임아웃(null)과 번호 불일치를 분리해 예외를 던지는 흐름이 명확합니다. 이 구간은 그대로 유지해도 좋겠습니다.
| // RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort); | ||
| RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration("localhost", 6379); | ||
| configuration.setPassword(redisPassword); |
There was a problem hiding this comment.
Redis 호스트/포트를 하드코딩하면 환경설정이 무력화됩니다
현재 테스트 yml은 6378 포트를 가리키는데, 코드가 6379를 강제하면서 프로파일별 설정을 모두 무시합니다. 구성 분리 원칙에 어긋나고 테스트/운영에서 장애로 이어질 수 있습니다. 프로퍼티 주입을 다시 사용해 주세요.
적용 diff:
-// RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort);
- RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration("localhost", 6379);
+ RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort);추가로 비밀번호가 비어있는 환경에서는 설정을 생략하는 것이 안전합니다.
- configuration.setPassword(redisPassword);
+ if (redisPassword != null && !redisPassword.isBlank()) {
+ configuration.setPassword(redisPassword);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort); | |
| RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration("localhost", 6379); | |
| configuration.setPassword(redisPassword); | |
| RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort); | |
| if (redisPassword != null && !redisPassword.isBlank()) { | |
| configuration.setPassword(redisPassword); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/gdsc/binaryho/imhere/config/redis/RedisConfig.java around lines
26-28, the Redis host/port are hardcoded to "localhost" and 6379 which overrides
injected properties and breaks profile-specific configuration; revert to use the
injected redisHost and redisPort fields when constructing
RedisStandaloneConfiguration and conditionally call
configuration.setPassword(redisPassword) only when redisPassword is non-empty
(null/blank check) so that environments without a password skip setting it.
| SUCCESS { | ||
| @Override | ||
| public void cache( | ||
| RedisTemplate<String, String> redisTemplate, String key, String value) { | ||
| String savedValue = redisTemplate.opsForValue().getAndDelete(key); | ||
| redisTemplate.opsForValue().set(key, savedValue, value); | ||
| } | ||
| }, |
There was a problem hiding this comment.
SUCCESS 캐싱 로직이 잘못되었습니다
Line 27에서 set 메서드에 세 개의 파라미터를 전달하고 있는데, 이는 TTL을 설정하는 메서드 시그니처입니다. 의도가 불명확합니다.
의도에 따라 다음 중 하나로 수정하세요:
@Override
public void cache(
RedisTemplate<String, String> redisTemplate, String key, String value) {
- String savedValue = redisTemplate.opsForValue().getAndDelete(key);
- redisTemplate.opsForValue().set(key, savedValue, value);
+ // 옵션 1: 기존 값과 관계없이 새 값으로 덮어쓰기
+ redisTemplate.opsForValue().set(key, value);
+
+ // 옵션 2: 기존 값이 SUCCESS가 아닐 때만 덮어쓰기
+ // String savedValue = redisTemplate.opsForValue().get(key);
+ // if (!SUCCESS.name().equals(savedValue)) {
+ // redisTemplate.opsForValue().set(key, value);
+ // }
}🤖 Prompt for AI Agents
In
src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceSaveRequestStatus.java
around lines 22–29, the SUCCESS.cache implementation calls
redisTemplate.opsForValue().set(key, savedValue, value) which uses the
TTL-overload incorrectly; change this to the intended overload: if you want to
store the new value, call redisTemplate.opsForValue().set(key, value); if you
intended to restore the previously read value, call
redisTemplate.opsForValue().set(key, savedValue); if you actually need a TTL,
call redisTemplate.opsForValue().set(key, value, duration, timeUnit) with an
explicit timeout and TimeUnit.
| public boolean canCache(String originalValue) { | ||
| this.name() | ||
| } |
There was a problem hiding this comment.
컴파일 에러: 불완전한 메서드
canCache 메서드가 불완전하여 컴파일 에러가 발생합니다.
public boolean canCache(String originalValue) {
- this.name()
+ // SUCCESS 상태는 다른 상태로 덮어쓸 수 없음
+ return !SUCCESS.name().equals(originalValue);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public boolean canCache(String originalValue) { | |
| this.name() | |
| } | |
| public boolean canCache(String originalValue) { | |
| // SUCCESS 상태는 다른 상태로 덮어쓸 수 없음 | |
| return !SUCCESS.name().equals(originalValue); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/gdsc/binaryho/imhere/core/attendance/application/AttendanceSaveRequestStatus.java
around lines 50 to 52, the canCache method body is incomplete and causes a
compile error; implement it to return whether the provided originalValue matches
this enum constant's name (use Objects.equals(originalValue, this.name()) to
avoid NPEs) and ensure java.util.Objects is imported if not already.
|
|
||
| void cache(AttendanceHistory attendanceHistory); | ||
|
|
||
| AttendanceSaveRequestStatus getRequestStatusByLectureIdAndStudentId(Long lectureId, Long studentId); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
API 시그니처 변경을 확인해주세요.
기존 List<AttendanceHistory> 반환 메서드가 제거되고 AttendanceSaveRequestStatus 반환 메서드로 대체되었습니다. 이 변경사항이 모든 호출 지점에서 올바르게 반영되었는지 확인이 필요합니다.
다음 스크립트로 기존 API 사용 여부를 확인해보겠습니다:
🏁 Script executed:
#!/bin/bash
# Description: 제거된 findAllByLectureIdAndStudentId 메서드의 사용처 확인
# 제거된 메서드 사용처 검색
rg -n "findAllByLectureIdAndStudentId" --type=java -C3
# 새로운 메서드 사용처 확인
rg -n "getRequestStatusByLectureIdAndStudentId" --type=java -C3Length of output: 5907
API 시그니처 변경 반영: 구현체에서 제거된 메서드 삭제 필요
기존 findAllByLectureIdAndStudentId 메서드는 인터페이스에서 완전히 제거되었으나, 구현체에 여전히 @Override와 함께 남아 있어 컴파일 에러가 발생합니다. 호출 지점은 모두 새 메서드(getRequestStatusByLectureIdAndStudentId)로 잘 옮겨진 것을 확인했습니다. 아래 파일에서 불필요한 구 메서드를 삭제하거나 @Override 애노테이션을 제거해주세요.
- src/main/java/gdsc/binaryho/imhere/core/attendance/infrastructure/AttendanceRedisCacheRepository.java
•findAllByLectureIdAndStudentId메서드 삭제 - src/test/java/gdsc/binaryho/imhere/mock/fakerepository/FakeAttendanceHistoryCacheRepository.java
•findAllByLectureIdAndStudentId메서드 삭제
🤖 Prompt for AI Agents
In
src/main/java/gdsc/binaryho/imhere/core/attendance/infrastructure/AttendanceRedisCacheRepository.java
(around where findAllByLectureIdAndStudentId is declared) and
src/test/java/gdsc/binaryho/imhere/mock/fakerepository/FakeAttendanceHistoryCacheRepository.java
(the mock implementation), remove the obsolete findAllByLectureIdAndStudentId
method declarations (including the method body and the @Override annotation) so
they no longer conflict with the updated interface; ensure only the new
getRequestStatusByLectureIdAndStudentId method remains and update imports if
needed.
| private boolean isRequestExist(AttendanceSaveRequestStatus attendanceSaveRequestStatus) { | ||
| return !attendanceSaveRequestStatus.equals(NO_REQUEST); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
null 안전성: equals 호출 순서 역전 필요
캐시가 null을 반환할 여지가 있다면 NPE가 납니다. 상수에서 비교하도록 순서를 바꿔 null-안전하게 해주세요.
- return !attendanceSaveRequestStatus.equals(NO_REQUEST);
+ return !NO_REQUEST.equals(attendanceSaveRequestStatus);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private boolean isRequestExist(AttendanceSaveRequestStatus attendanceSaveRequestStatus) { | |
| return !attendanceSaveRequestStatus.equals(NO_REQUEST); | |
| } | |
| private boolean isRequestExist(AttendanceSaveRequestStatus attendanceSaveRequestStatus) { | |
| return !NO_REQUEST.equals(attendanceSaveRequestStatus); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/gdsc/binaryho/imhere/core/attendance/application/StudentAttendanceService.java
around lines 83 to 86, the current equals call on attendanceSaveRequestStatus
can NPE if the cached value is null; change the comparison order to call equals
on the constant instead (i.e., return
!NO_REQUEST.equals(attendanceSaveRequestStatus)) so the check is null-safe.
| public StudentRecentAttendanceResponse(AttendanceSaveRequestStatus status) { | ||
| this.recentAttendanceStatus = status.name(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
null 체크가 필요합니다
AttendanceSaveRequestStatus가 null일 경우 NullPointerException이 발생할 수 있습니다.
public StudentRecentAttendanceResponse(AttendanceSaveRequestStatus status) {
+ if (status == null) {
+ this.recentAttendanceStatus = AttendanceSaveRequestStatus.NO_REQUEST.name();
+ return;
+ }
this.recentAttendanceStatus = status.name();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public StudentRecentAttendanceResponse(AttendanceSaveRequestStatus status) { | |
| this.recentAttendanceStatus = status.name(); | |
| } | |
| public StudentRecentAttendanceResponse(AttendanceSaveRequestStatus status) { | |
| if (status == null) { | |
| this.recentAttendanceStatus = AttendanceSaveRequestStatus.NO_REQUEST.name(); | |
| return; | |
| } | |
| this.recentAttendanceStatus = status.name(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/gdsc/binaryho/imhere/core/attendance/model/response/StudentRecentAttendanceResponse.java
around lines 13 to 15, the constructor calls status.name() without checking for
null which can throw NullPointerException; update the constructor to guard
against null (e.g., if status != null then recentAttendanceStatus =
status.name() else set recentAttendanceStatus to null or a safe default like
"UNKNOWN") so the field is always assigned safely.
| // TODO : 테스트용 | ||
| @Transactional | ||
| public AttendanceNumberResponse openLectureAndGenerateAttendanceNumber(Long lectureId, int attendanceNumber) { | ||
| Lecture lecture = lectureRepository.findById(lectureId) | ||
| .orElseThrow(() -> LectureNotFoundException.EXCEPTION); | ||
| // authenticationHelper.verifyRequestMemberLogInMember(lecture.getMember().getId()); | ||
|
|
||
| lecture.setLectureState(LectureState.OPEN); | ||
| lecture.setLastOpeningTime(seoulDateTimeHolder.getSeoulDateTime()); | ||
|
|
||
| saveOpenLecture(lecture, attendanceNumber); | ||
| cacheAttendee(lecture); | ||
|
|
||
| log.info("[강의 OPEN] {} ({}), 출석 번호 : " + attendanceNumber | ||
| , lecture::getLectureName, lecture::getId); | ||
|
|
||
| return new AttendanceNumberResponse(attendanceNumber); | ||
| } |
There was a problem hiding this comment.
테스트 전용 메서드를 프로덕션 코드에서 제거하세요
프로덕션 서비스 클래스에 테스트 전용 메서드가 포함되어 있습니다. 이는 보안 위험을 초래할 수 있으며, 인증 검증이 주석 처리되어 있어 더욱 위험합니다.
다음 중 하나의 방법을 고려하세요:
- 테스트 프로파일에서만 활성화되는 별도의 테스트 서비스 클래스로 이동
@Profile("test")애노테이션을 사용하여 테스트 환경에서만 활성화- 테스트에서 Mockito를 사용하여 원하는 동작을 모킹
- // TODO : 테스트용
- @Transactional
- public AttendanceNumberResponse openLectureAndGenerateAttendanceNumber(Long lectureId, int attendanceNumber) {
- Lecture lecture = lectureRepository.findById(lectureId)
- .orElseThrow(() -> LectureNotFoundException.EXCEPTION);
-// authenticationHelper.verifyRequestMemberLogInMember(lecture.getMember().getId());
-
- lecture.setLectureState(LectureState.OPEN);
- lecture.setLastOpeningTime(seoulDateTimeHolder.getSeoulDateTime());
-
- saveOpenLecture(lecture, attendanceNumber);
- cacheAttendee(lecture);
-
- log.info("[강의 OPEN] {} ({}), 출석 번호 : " + attendanceNumber
- , lecture::getLectureName, lecture::getId);
-
- return new AttendanceNumberResponse(attendanceNumber);
- }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // TODO : 테스트용 | |
| @Transactional | |
| public AttendanceNumberResponse openLectureAndGenerateAttendanceNumber(Long lectureId, int attendanceNumber) { | |
| Lecture lecture = lectureRepository.findById(lectureId) | |
| .orElseThrow(() -> LectureNotFoundException.EXCEPTION); | |
| // authenticationHelper.verifyRequestMemberLogInMember(lecture.getMember().getId()); | |
| lecture.setLectureState(LectureState.OPEN); | |
| lecture.setLastOpeningTime(seoulDateTimeHolder.getSeoulDateTime()); | |
| saveOpenLecture(lecture, attendanceNumber); | |
| cacheAttendee(lecture); | |
| log.info("[강의 OPEN] {} ({}), 출석 번호 : " + attendanceNumber | |
| , lecture::getLectureName, lecture::getId); | |
| return new AttendanceNumberResponse(attendanceNumber); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/gdsc/binaryho/imhere/core/lecture/application/LectureService.java
around lines 145 to 162 there is a test-only method left in production code
(authentication call commented out) which is a security risk; remove or restrict
it by either moving the method into a test-only service class used only by
tests, annotate the class or method with @Profile("test") so it is only active
in the test profile, or delete the method and update tests to use Mockito to
mock LectureService behavior; ensure any commented-out authentication is
restored or replaced by proper verification in production code and update tests
to use the new test-targeted service or mocks.
| public AttendanceHistories findAllByLectureIdAndStudentId(final long lectureId, final long studentId) { | ||
| List<AttendanceHistory> attendanceHistories = data.getOrDefault( | ||
| AttendanceHistory.convertToKey(lectureId, studentId), Collections.emptySet()) | ||
| .stream() | ||
| .map(timestamp -> new AttendanceHistory(lectureId, studentId, timestamp)) | ||
| .collect(Collectors.toList()); | ||
| return AttendanceHistories.of(attendanceHistories); | ||
| } |
There was a problem hiding this comment.
도메인 변경 반영 누락: 생성자 인자 타입 불일치 및 누락된 import
AttendanceHistory는 이제 timestamp 대신 AttendanceSaveRequestStatus를 보유합니다. 현재 new AttendanceHistory(lectureId, studentId, timestamp)는 컴파일 불가입니다. 또한 AttendanceHistories import가 없습니다.
- 매핑 시 문자열을 상태 enum으로 변환하세요.
- AttendanceHistories를 import 하세요.
적용 diff(매핑 수정):
- public AttendanceHistories findAllByLectureIdAndStudentId(final long lectureId, final long studentId) {
- List<AttendanceHistory> attendanceHistories = data.getOrDefault(
- AttendanceHistory.convertToKey(lectureId, studentId), Collections.emptySet())
- .stream()
- .map(timestamp -> new AttendanceHistory(lectureId, studentId, timestamp))
- .collect(Collectors.toList());
- return AttendanceHistories.of(attendanceHistories);
- }
+ public AttendanceHistories findAllByLectureIdAndStudentId(final long lectureId, final long studentId) {
+ List<AttendanceHistory> attendanceHistories = data.getOrDefault(
+ AttendanceHistory.convertToKey(lectureId, studentId), Collections.emptySet())
+ .stream()
+ .map(statusStr ->
+ new AttendanceHistory(
+ lectureId,
+ studentId,
+ AttendanceSaveRequestStatus.valueOf(statusStr)
+ )
+ )
+ .collect(Collectors.toList());
+ return AttendanceHistories.of(attendanceHistories);
+ }파일 상단에 import 추가(선택 범위 외 변경):
import gdsc.binaryho.imhere.core.attendance.application.AttendanceHistories;필요하시면 cache(...) 구현/내부 자료구조까지 맞춰 드리겠습니다.
🤖 Prompt for AI Agents
In
src/test/java/gdsc/binaryho/imhere/mock/fakerepository/FakeAttendanceHistoryCacheRepository.java
around lines 19 to 26, the code still constructs AttendanceHistory with a
timestamp string although AttendanceHistory now requires an
AttendanceSaveRequestStatus; update the mapping to convert the stored string to
the enum (e.g. AttendanceSaveRequestStatus.valueOf(someString)) and pass that
enum into the AttendanceHistory constructor, and add the missing import for
gdsc.binaryho.imhere.core.attendance.application.AttendanceHistories at the top
of the file; ensure the collection default/typing matches the stored string set
so the stream variable is a String before converting to the enum.
| @Override | ||
| public AttendanceSaveRequestStatus getRequestStatusByLectureIdAndStudentId( | ||
| Long lectureId, Long studentId) { | ||
|
|
||
| data.getOrDefault(AttendanceHistory.convertToKey(lectureId, studentId)) | ||
| return null; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
미구현 메서드와 문법 오류 수정 필요: 기본값(NO_REQUEST) 반환 및 상태 판독 로직 추가
현재 본문은 컴파일되지 않으며 null을 반환합니다. NO_REQUEST를 기본값으로 하고, 저장된 상태가 있으면 판독해 반환하세요.
적용 diff:
@Override
public AttendanceSaveRequestStatus getRequestStatusByLectureIdAndStudentId(
Long lectureId, Long studentId) {
-
- data.getOrDefault(AttendanceHistory.convertToKey(lectureId, studentId))
- return null;
+ String key = AttendanceHistory.convertToKey(lectureId, studentId);
+ Set<String> statuses = data.getOrDefault(key, Collections.emptySet());
+ if (statuses.isEmpty()) {
+ return AttendanceSaveRequestStatus.NO_REQUEST;
+ }
+ // 단순 판독 규칙: 존재하는 가장 "강한" 상태를 반환
+ if (statuses.contains(AttendanceSaveRequestStatus.PROCESSING.name())) {
+ return AttendanceSaveRequestStatus.PROCESSING;
+ }
+ if (statuses.contains(AttendanceSaveRequestStatus.FAILED.name())) {
+ return AttendanceSaveRequestStatus.FAILED;
+ }
+ if (statuses.contains(AttendanceSaveRequestStatus.SUCCESS.name())) {
+ return AttendanceSaveRequestStatus.SUCCESS;
+ }
+ return AttendanceSaveRequestStatus.NO_REQUEST;
}추가로, cache(...)가 현재 Set에 아무 값도 추가하지 않아 위 로직이 항상 NO_REQUEST를 반환하게 됩니다. 아래처럼 상태를 저장하도록 수정하세요(선택 범위 외 변경):
@Override
public void cache(AttendanceHistory attendanceHistory) {
data.computeIfAbsent(attendanceHistory.getKey(), k -> new HashSet<>())
.add(attendanceHistory.getAttendanceSaveRequestStatus().name());
}도메인 변경 전 잔존 사용처 정리 여부 확인:
🏁 Script executed:
#!/bin/bash
# 과거 API/필드 사용 흔적 탐지
rg -n -C2 --type=java '\bAttendanceHistory\.of\s*\('
rg -n -C2 --type=java '\bgetTimestamp\s*\('
rg -n -C2 --type=java 'new\s+AttendanceHistory\s*\('Length of output: 12259
미구현 메서드 구현·캐시 수정 및 도메인 API 사용 정리 필요
아래 작업을 반드시 수행하세요:
• getRequestStatusByLectureIdAndStudentId 구현
FakeAttendanceHistoryCacheRepository.java33–39행:
• key 생성 →data.getOrDefault(key, Collections.emptySet())
• 상태 집합이 비어있으면NO_REQUEST반환
• 프로세싱, 실패, 성공 순으로 “강한” 상태 반환
• cache() 메서드 수정
FakeAttendanceHistoryCacheRepository.java내cache(AttendanceHistory)구현부를와 같이 상태를 Set에 저장하도록 변경data.computeIfAbsent(attendanceHistory.getKey(), k -> new HashSet<>()) .add(attendanceHistory.getAttendanceSaveRequestStatus().name());
• 도메인 변경 전 잔존 API 사용처 제거/교체
AttendanceHistory.of(...)→ 도메인 팩토리 메서드(createAwait…,createAccepted…,createFailed…)로 전환getTimestamp()호출 재검토(필요 시 새 API로 대체)new AttendanceHistory(...)→ 팩토리 메서드 사용
잔존 사용처 위치(예시)
- src/main/java/.../AttendanceRedisCacheRepository.java:30–31행 (
AttendanceHistory.of,.add(...attendanceHistory.getTimestamp())) - src/main/java/.../StudentAttendanceResponse.java & LecturerAttendanceResponse.java:
attendance.getTimestamp() - src/test/java/.../FakeAttendanceHistoryCacheRepository.java:23–25행 (
new AttendanceHistory(...)) - 다수 테스트 클래스(LecturerAttendanceServiceTest, AttendanceControllerTest 등):
getTimestamp(),new AttendanceHistory
🤖 Prompt for AI Agents
In
src/test/java/gdsc/binaryho/imhere/mock/fakerepository/FakeAttendanceHistoryCacheRepository.java
around lines 33–39, implement getRequestStatusByLectureIdAndStudentId to read
the set via data.getOrDefault(AttendanceHistory.convertToKey(lectureId,
studentId), Collections.emptySet()), return
AttendanceSaveRequestStatus.NO_REQUEST if the set is empty, otherwise inspect
the stored status names and return the strongest status in the priority order
PROCESSING → FAILED → SUCCESS (i.e., if "PROCESSING" present return PROCESSING,
else if "FAILED" present return FAILED, else if "SUCCESS" present return
SUCCESS). Also modify the cache(AttendanceHistory) implementation in the same
class to store status names into a Set using
data.computeIfAbsent(attendanceHistory.getKey(), k -> new
HashSet<>()).add(attendanceHistory.getAttendanceSaveRequestStatus().name()).
Finally, replace legacy domain API usages noted in the comment: stop using
AttendanceHistory.of(...), new AttendanceHistory(...), and getTimestamp() —
switch calls to the new domain factory methods (createAwait...,
createAccepted..., createFailed...) and the new timestamp API where required
across the listed files.
| datasource: | ||
| url: jdbc:h2:~/mem-data; | ||
| driverClassName: org.h2.Driver | ||
| username: sa | ||
| password: | ||
| url: jdbc:postgresql://jinho-database.ckddmrlccdc1.ap-northeast-2.rds.amazonaws.com:5432/imhere-dev | ||
| username: postgres | ||
| password: d0YhFa6o3HJ8GqOzKhF5 | ||
| # driver-class-name: org.postgresql.Drive |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
테스트 설정에 실DB(RDS) URL/비밀번호 평문 노출 — 즉시 제거·로테이션 필요
테스트 리소스에 실제 RDS 엔드포인트와 비밀번호가 평문으로 커밋되었습니다. 이는 심각한 보안 이슈입니다. 즉시 해당 자격 증명을 폐기/로테이트하고, 커밋 히스토리에서 제거(필요 시 BFG/Git filter-repo)하세요. 테스트는 로컬/격리 환경(H2 또는 Testcontainers/Postgres)로 대체하는 것이 안전합니다.
[offer_assistance]
권장 수정(예: H2 인메모리로 복귀):
- datasource:
- url: jdbc:postgresql://jinho-database.ckddmrlccdc1.ap-northeast-2.rds.amazonaws.com:5432/imhere-dev
- username: postgres
- password: d0YhFa6o3HJ8GqOzKhF5
-# driver-class-name: org.postgresql.Drive
+ h2:
+ console:
+ enabled: true
+ path: /h2-console
+ datasource:
+ url: jdbc:h2:mem:testdb;MODE=PostgreSQL;DB_CLOSE_DELAY=-1;DATABASE_TO_UPPER=false
+ driverClassName: org.h2.Driver
+ username: sa
+ password:
+# driver-class-name: org.postgresql.Driver원하시면 Testcontainers(PostgreSQL) 기반 테스트 설정으로도 전환해 드릴 수 있습니다. 또한 저장소 전체에서 평문 비밀번호/키가 남아있는지 스캔해 보세요.
🏁 Script executed:
#!/bin/bash
# 저장소 전체에서 위험한 패턴 빠르게 점검
set -euo pipefail
echo "Plain password candidates (YAML):"
rg -nP -C2 --glob '!**/target/**' --glob '!**/build/**' '\bpassword:\s*(?!ENC\()(?!(\s*#)).+' || true
echo
echo "Hard-coded JDBC URLs:"
rg -nP -C2 --glob '!**/target/**' --glob '!**/build/**' 'jdbc:(postgresql|mysql|mariadb)://' || true
echo
echo "AWS RDS host patterns:"
rg -nP -C1 --glob '!**/target/**' --glob '!**/build/**' 'rds\.amazonaws\.com' || trueLength of output: 5506
실 DB(RDS) 테스트 설정 평문 자격증명 노출 — 즉시 제거 및 로테이션 필요
저장소 전체 스캔 결과, src/test/resources/application.yml 32~34행에서만 실제 RDS 엔드포인트와 비밀번호가 평문으로 노출되어 있습니다. 나머지 프로파일은 Jasypt 암호화(ENC/END) 방식이 적용되어 있습니다.
· 위치
- src/test/resources/application.yml (lines 32–34)
• url: jdbc:postgresql://jinho-database.ckddmrlccdc1.ap-northeast-2.rds.amazonaws.com:5432/imhere-dev
• username: postgres
• password: d0YhFa6o3HJ8GqOzKhF5
즉시 조치 항목
- 해당 자격증명(엔드포인트·비밀번호) 폐기 및 로테이션
- 커밋 히스토리에서 완전 제거 (BFG 또는 git filter-repo 활용)
- 테스트 리소스를 H2 인메모리 또는 Testcontainers(PostgreSQL) 등 격리 환경으로 전환
- 환경 변수(CI/CD 시크릿 매니저) 또는 Vault 등을 이용해 민감 정보 관리
예시: H2 인메모리로 대체
- datasource:
- url: jdbc:postgresql://jinho-database.ckddmrlccdc1.ap-northeast-2.rds.amazonaws.com:5432/imhere-dev
- username: postgres
- password: d0YhFa6o3HJ8GqOzKhF5
-# driver-class-name: org.postgresql.Driver
+ h2:
+ console:
+ enabled: true
+ path: /h2-console
+ datasource:
+ url: jdbc:h2:mem:testdb;MODE=PostgreSQL;DB_CLOSE_DELAY=-1;DATABASE_TO_UPPER=false
+ driverClassName: org.h2.Driver
+ username: sa
+ password:
+# driver-class-name: org.postgresql.Driver필요 시 Testcontainers 기반 설정 예시도 제공 가능합니다. 저장소 전역에서 평문 비밀번호/키 잔여 여부를 주기적으로 스캔할 것을 권장드립니다.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Gitleaks (8.27.2)
34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In src/test/resources/application.yml around lines 31 to 35, the file contains
plaintext production RDS credentials (URL, username, password) that must be
removed and remediated: immediately rotate/revoke those exposed DB credentials,
remove them from git history using BFG or git filter-repo (and force-push to
remote), replace the test profile with a non-production test DB (switch to H2
in-memory or Testcontainers PostgreSQL) and move any runtime credentials to
environment variables or a secrets manager (CI/CD secrets or Vault) so no
plaintext secrets remain in the repo.
|
@gemini-code-assist 이 Pull Request의 변경 사항을 설명하는 클래스 다이어그램을 Mermaid 문법으로 그려주세요. |
|
요청하신 Pull Request의 주요 변경 사항을 설명하는 클래스 다이어그램입니다. 비동기 출석 처리 및 이벤트 기반 캐싱 로직의 변화에 초점을 맞췄습니다. classDiagram
direction LR
class AttendanceController {
+takeAttendance()
+getStudentRecentAttendanceStatus()
}
class StudentAttendanceService {
-ApplicationEventPublisher eventPublisher
-AttendanceSaveService attendanceSaveService
-AttendanceHistoryCacheRepository attendanceHistoryCacheRepository
-OpenLectureService openLectureService
+takeAttendance()
+getStudentRecentAttendanceStatus()
}
class AttendanceSaveService {
+save(Attendance attendance)
}
class AttendanceHistoryCacheService {
+cacheAttendanceHistory(AttendanceRequestedEvent event)
+saveAttendance(AttendanceSaveSucceedEvent event)
+saveAttendance(AttendanceFailedEvent event)
}
class AttendanceHistory {
-AttendanceSaveRequestStatus status
}
class AttendanceSaveRequestStatus {
<<enum>>
PROCESSING
SUCCESS
FAILED
NO_REQUEST
}
class OpenLectureService {
+isStudentOpenLectureExist()
+cacheStudent(AttendeeCacheEvent event)
}
class LectureService {
+openLectureAndGenerateAttendanceNumber()
}
class AttendanceRequestedEvent {
<<event>>
}
class AttendanceSaveSucceedEvent {
<<event>>
}
class AttendanceFailedEvent {
<<event>>
}
class AttendeeCacheEvent {
<<event>>
}
class AttendanceRepository {
<<interface>>
}
class AttendanceHistoryCacheRepository {
<<interface>>
}
class AttendeeCacheRepository {
<<interface>>
}
class ApplicationEventPublisher {
<<interface>>
}
AttendanceController --> StudentAttendanceService : 출석 요청 및 조회
StudentAttendanceService --> ApplicationEventPublisher : 이벤트 발행
StudentAttendanceService --> AttendanceSaveService : 비동기 저장 위임
StudentAttendanceService --> OpenLectureService : 강의 상태 및 학생 존재 여부 확인
StudentAttendanceService --> AttendanceHistoryCacheRepository : 최신 출석 상태 조회
AttendanceSaveService --> AttendanceRepository : 출석 정보 저장
AttendanceHistoryCacheService ..> AttendanceRequestedEvent : 수신
AttendanceHistoryCacheService ..> AttendanceSaveSucceedEvent : 수신
AttendanceHistoryCacheService ..> AttendanceFailedEvent : 수신
AttendanceHistoryCacheService --> AttendanceHistoryCacheRepository : 출석 이력 캐시 업데이트
AttendanceHistory *-- AttendanceSaveRequestStatus : 상태 포함
OpenLectureService --> AttendeeCacheRepository : 학생 캐시 관리
OpenLectureService ..> AttendeeCacheEvent : 수신
LectureService --> OpenLectureService : 학생 캐시 책임 위임
LectureService --> ApplicationEventPublisher : 이벤트 발행
AttendanceHistoryCacheRepository <|.. AttendanceRedisCacheRepository : 구현
AttendeeCacheRepository <|.. AttendeeCacheRedisRepository : 구현
AttendanceRepository <|.. AttendanceJpaRepository : 구현 (예시)
|
|
@gemini-code-assist 이 Pull Request의 변경 사항을 거시적으로 이해할 수 있게, 로직 흐름을 순서도(flowchart)로 표현해 주세요. |
|
요청하신 Pull Request의 주요 변경 사항에 대한 로직 흐름 순서도입니다. 비동기 출석 처리 및 상태 기반 출석 이력 조회 흐름에 초점을 맞췄습니다. graph TD
subgraph 출석 요청 흐름
A["User clicks 'Take Attendance'"] --> B("controller.takeAttendance() 호출");
B --> C{Is user authenticated?};
B --> C["StudentAttendanceService.takeAttendance() 호출"];
C --> D{오픈 강의 캐시 존재 여부 확인};
D -- 캐시 없음 --> E[수강 신청 정보 검증];
D -- 캐시 있음 --> F[강의 정보 조회];
E --> G[출석 처리 로직 (attend() 호출)];
F --> G;
G --> H[AttendanceRequestedEvent 발행];
G --> I[AttendanceSaveService.save() 비동기 호출];
I --> J{비동기 저장 성공?};
J -- 성공 --> K[AttendanceSaveSucceedEvent 발행];
J -- 실패 --> L[AttendanceFailedEvent 발행];
K --> M[AttendanceHistoryCacheService: SUCCESS 상태 캐시];
L --> N[AttendanceHistoryCacheService: FAILED 상태 캐시 및 로그];
H --> O[AttendanceHistoryCacheService: PROCESSING 상태 캐시];
M --> P[응답 반환];
N --> P;
O --> P;
end
subgraph 최근 출석 상태 조회 흐름
Q[클라이언트: 최근 출석 상태 조회 요청] --> R{AttendanceController.getStudentRecentAttendanceStatus() 호출};
R --> S[StudentAttendanceService.getStudentRecentAttendanceStatus() 호출];
S --> T[AttendanceHistoryCacheRepository: 캐시된 출석 요청 상태 조회];
T --> U{조회된 상태가 NO_REQUEST가 아님?};
U -- 예 --> V[해당 상태 반환];
U -- 아니오 --> W[DB에서 최근 출석 기록 조회];
W --> X{최근 출석 기록 존재?};
X -- 예 --> Y[SUCCESS 상태 반환];
X -- 아니오 --> Z[NO_REQUEST 상태 반환];
V --> AA[응답 반환];
Y --> AA;
Z --> AA;
end
|
|
@gemini-code-assist 이 Pull Request의 변경 사항을 거시적으로 이해할 수 있게, 로직 흐름을 순서도(flowchart)로 표현해 주세요.
|
|
안녕하세요 @binary-ho님, 요청하신 Pull Request의 변경 사항에 대한 로직 흐름 순서도(flowchart)는 이전 댓글(Comment 5)에서 이미 제공해 드렸습니다. 해당 순서도는 비동기 출석 처리 및 상태 기반 출석 이력 조회 흐름을 중심으로 변경 사항을 거시적으로 표현하고 있습니다. Mermaid 문법으로 작성되었으며, 로직 흐름을 이해하는 데 도움이 될 것입니다. 혹시 이전 순서도에 특정 오류가 있거나, 추가적으로 설명이 필요한 부분이 있다면 구체적으로 알려주시면 자세히 답변해 드리겠습니다. |
|
@gemini-code-assist 해당 코드에서 에러가 있어 제대로 랜더링 되지 않아 재요청을 드린 것입니다. 다시 그려주세요
|
|
안녕하세요 @binary-ho님, 이전 순서도에서 렌더링 오류가 발생하여 불편을 드린 점 사과드립니다. 다시 요청하신 Pull Request의 변경 사항에 대한 로직 흐름 순서도(flowchart)를 Mermaid 문법으로 작성하여 제공해 드립니다. 이번에는 오류 없이 정상적으로 렌더링되기를 바랍니다. 이 순서도는 비동기 출석 처리 및 상태 기반 출석 이력 조회 흐름에 초점을 맞춰 주요 변경 사항을 거시적으로 표현하고 있습니다. graph TD
subgraph 출석 요청 흐름
A[클라이언트: 출석 요청] --> B(AttendanceController.takeAttendance() 호출);
B --> C(StudentAttendanceService.takeAttendance() 호출);
C --> D{오픈 강의 캐시 존재 여부 확인?};
D -- 캐시 없음 --> E[수강 신청 정보 검증 및 강의 정보 조회];
D -- 캐시 있음 --> F[강의 정보 조회];
E --> G[출석 처리 로직 (attend() 호출)];
F --> G;
G --> H[AttendanceRequestedEvent 발행 (PROCESSING 상태 캐시)];
G --> I[AttendanceSaveService.save() 비동기 호출 시작];
H --> J[클라이언트에 응답 반환 (비동기 처리 진행 중)];
subgraph 비동기 출석 저장 및 캐시 업데이트
I --> K{비동기 저장 성공?};
K -- 성공 --> L[AttendanceSaveSucceedEvent 발행];
K -- 실패 --> M[AttendanceFailedEvent 발행];
L --> N[AttendanceHistoryCacheService: SUCCESS 상태 캐시];
M --> O[AttendanceHistoryCacheService: FAILED 상태 캐시 및 로그];
end
end
subgraph 최근 출석 상태 조회 흐름
P[클라이언트: 최근 출석 상태 조회 요청] --> Q(AttendanceController.getStudentRecentAttendanceStatus() 호출);
Q --> R(StudentAttendanceService.getStudentRecentAttendanceStatus() 호출);
R --> S[AttendanceHistoryCacheRepository: 캐시된 출석 요청 상태 조회];
S --> T{조회된 상태가 NO_REQUEST가 아님?};
T -- 예 --> U[캐시된 상태 반환];
T -- 아니오 --> V{DB에서 최근 출석 기록 존재?};
V -- 예 --> W[SUCCESS 상태 반환];
V -- 아니오 --> X[NO_REQUEST 상태 반환];
U --> Y[응답 반환];
W --> Y;
X --> Y;
end
|
|
@gemini-code-assist 여전히 에러가 존재합니다. |
|
안녕하세요 @binary-ho님, 이전 순서도에서 렌더링 오류가 계속 발생하여 불편을 드린 점 다시 한번 사과드립니다. 제공해주신 오류 메시지를 확인한 결과, 노드 라벨 내의 괄호 이 문제를 해결하기 위해 노드 라벨 내의 이 순서도는 비동기 출석 처리 및 상태 기반 출석 이력 조회 흐름에 초점을 맞춰 주요 변경 사항을 거시적으로 표현하고 있습니다. graph TD
subgraph 출석 요청 흐름
A[클라이언트: 출석 요청] --> B(AttendanceController.takeAttendance 호출);
B --> C(StudentAttendanceService.takeAttendance 호출);
C --> D{오픈 강의 캐시 존재 여부 확인?};
D -- 캐시 없음 --> E[수강 신청 정보 검증 및 강의 정보 조회];
D -- 캐시 있음 --> F[강의 정보 조회];
E --> G[출석 처리 로직 (attend 호출)];
F --> G;
G --> H[AttendanceRequestedEvent 발행 (PROCESSING 상태 캐시)];
G --> I[AttendanceSaveService.save 비동기 호출 시작];
H --> J[클라이언트에 응답 반환 (비동기 처리 진행 중)];
subgraph 비동기 출석 저장 및 캐시 업데이트
I --> K{비동기 저장 성공?};
K -- 성공 --> L[AttendanceSaveSucceedEvent 발행];
K -- 실패 --> M[AttendanceFailedEvent 발행];
L --> N[AttendanceHistoryCacheService: SUCCESS 상태 캐시];
M --> O[AttendanceHistoryCacheService: FAILED 상태 캐시 및 로그];
end
end
subgraph 최근 출석 상태 조회 흐름
P[클라이언트: 최근 출석 상태 조회 요청] --> Q(AttendanceController.getStudentRecentAttendanceStatus 호출);
Q --> R(StudentAttendanceService.getStudentRecentAttendanceStatus 호출);
R --> S[AttendanceHistoryCacheRepository: 캐시된 출석 요청 상태 조회];
S --> T{조회된 상태가 NO_REQUEST가 아님?};
T -- 예 --> U[캐시된 상태 반환];
T -- 아니오 --> V{DB에서 최근 출석 기록 존재?};
V -- 예 --> W[SUCCESS 상태 반환];
V -- 아니오 --> X[NO_REQUEST 상태 반환];
U --> Y[응답 반환];
W --> Y;
X --> Y;
end
|
|
@gemini-code-assist 또 에러가 있습니다. 제 생각에는 전부 |
|
안녕하세요 @binary-ho님, 계속해서 렌더링 오류가 발생하여 다시 한번 불편을 드린 점 진심으로 사과드립니다. 제공해주신 오류 메시지를 바탕으로 분석한 결과, Mermaid 문법에서 노드 라벨 내에 괄호 이 문제를 해결하기 위해 모든 노드 라벨의 텍스트를 이중 따옴표 이 순서도는 비동기 출석 처리 및 상태 기반 출석 이력 조회 흐름에 초점을 맞춰 주요 변경 사항을 거시적으로 표현하고 있습니다. graph TD
subgraph 출석 요청 흐름
A["클라이언트: 출석 요청"] --> B("AttendanceController.takeAttendance() 호출");
B --> C("StudentAttendanceService.takeAttendance() 호출");
C --> D{"오픈 강의 캐시 존재 여부 확인?"};
D -- "캐시 없음" --> E["수강 신청 정보 검증 및 강의 정보 조회"];
D -- "캐시 있음" --> F["강의 정보 조회"];
E --> G["출석 처리 로직 (attend() 호출)"];
F --> G;
G --> H["AttendanceRequestedEvent 발행 (PROCESSING 상태 캐시)"];
G --> I["AttendanceSaveService.save() 비동기 호출 시작"];
H --> J["클라이언트에 응답 반환 (비동기 처리 진행 중)"];
subgraph 비동기 출석 저장 및 캐시 업데이트
I --> K{"비동기 저장 성공?"};
K -- "성공" --> L["AttendanceSaveSucceedEvent 발행"];
K -- "실패" --> M["AttendanceFailedEvent 발행"];
L --> N["AttendanceHistoryCacheService: SUCCESS 상태 캐시"];
M --> O["AttendanceHistoryCacheService: FAILED 상태 캐시 및 로그"];
end
end
subgraph 최근 출석 상태 조회 흐름
P["클라이언트: 최근 출석 상태 조회 요청"] --> Q("AttendanceController.getStudentRecentAttendanceStatus() 호출");
Q --> R("StudentAttendanceService.getStudentRecentAttendanceStatus() 호출");
R --> S["AttendanceHistoryCacheRepository: 캐시된 출석 요청 상태 조회"];
S --> T{"조회된 상태가 NO_REQUEST가 아님?"};
T -- "예" --> U["캐시된 상태 반환"];
T -- "아니오" --> V{"DB에서 최근 출석 기록 존재?"};
V -- "예" --> W["SUCCESS 상태 반환"];
V -- "아니오" --> X["NO_REQUEST 상태 반환"];
U --> Y["응답 반환"];
W --> Y;
X --> Y;
end
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
이번 PR은 출석 저장 로직을 비동기 처리로 변경하고 출석 상태 조회 기능을 개선하는 등 중요한 리팩토링을 포함하고 있습니다. 비동기 처리와 이벤트 기반 아키텍처를 도입하여 시스템의 응답성을 높이려는 시도는 매우 긍정적입니다. 하지만 리뷰 결과, 몇 가지 심각한 버그와 보안 취약점, 그리고 미완성된 코드가 발견되었습니다. 특히 Redis 데이터 타입 불일치, 컴파일 오류를 유발하는 코드, 테스트 환경에 하드코딩된 데이터베이스 인증 정보 등은 배포 시 심각한 문제를 일으킬 수 있으므로 반드시 수정이 필요합니다. 아래에 각 파일별로 상세한 리뷰 내용을 정리했으니 확인 후 반영 부탁드립니다.
| public RedisConnectionFactory redisConnectionFactory() { | ||
| RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort); | ||
| // RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort); | ||
| RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration("localhost", 6379); |
There was a problem hiding this comment.
Redis 연결 정보가 "localhost:6379"로 하드코딩되어 있습니다. 기존에 @Value를 통해 설정 파일에서 값을 읽어오던 방식이 주석 처리되어 있는데, 이럴 경우 로컬 개발 환경을 제외한 다른 환경(테스트, 운영 등)에서 애플리케이션이 동작하지 않습니다. 테스트를 위해 임시로 변경한 것이라면, 커밋에는 포함되지 않도록 원복해야 합니다.
| RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration("localhost", 6379); | |
| RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort); |
| logAttendanceFailed(studentId, lectureId, event.getException()); | ||
| } | ||
|
|
||
| private void logAttendanceFailed(Long studentId, Long lectureId, RuntimeException exception) { |
There was a problem hiding this comment.
logAttendanceFailed 메서드를 호출할 때 event.getException()을 인자로 전달하고 있습니다. AttendanceFailedEvent의 exception 필드는 Throwable 타입이지만, logAttendanceFailed 메서드는 RuntimeException 타입의 파라미터를 받도록 선언되어 있습니다. 만약 RuntimeException이 아닌 다른 Throwable(예: checked exception)이 이벤트에 담겨 전달될 경우, 이 지점에서 ClassCastException이 발생하여 비동기 이벤트 처리 로직이 중단될 수 있습니다. logAttendanceFailed 메서드의 파라미터 타입을 Throwable로 변경하여 안정성을 확보해야 합니다.
| private void logAttendanceFailed(Long studentId, Long lectureId, RuntimeException exception) { | |
| private void logAttendanceFailed(Long studentId, Long lectureId, Throwable exception) { |
| public enum AttendanceSaveRequestStatus { | ||
|
|
||
| /* | ||
| * 또한, 설문을 통해 유저들이 요청한 “출석 성공 조회”기능을 위해, 출석 요청, 성공, 예외 발생시 이벤트를 발행해 캐싱한다. | ||
| * 출석 성공은 다른 상태를 덮어 쓰고, 다른 상태는 출석 성공을 덮어 쓸 수 없다. | ||
| * */ | ||
|
|
||
| PROCESSING { | ||
| @Override | ||
| public void cache( | ||
| RedisTemplate<String, String> redisTemplate, String key, String value) { | ||
| String savedValue = redisTemplate.opsForValue().getAndDelete(key); | ||
| if (SUCCESS.name().equals(savedValue)) | ||
| redisTemplate.opsForValue().set(key, value); | ||
| } | ||
| }, | ||
|
|
||
| SUCCESS { | ||
| @Override | ||
| public void cache( | ||
| RedisTemplate<String, String> redisTemplate, String key, String value) { | ||
| String savedValue = redisTemplate.opsForValue().getAndDelete(key); | ||
| redisTemplate.opsForValue().set(key, savedValue, value); | ||
| } | ||
| }, | ||
|
|
||
| FAILED { | ||
| @Override | ||
| public void cache( | ||
| RedisTemplate<String, String> redisTemplate, String key, String value) { | ||
| redisTemplate.opsForSet().add(key, value); | ||
| } | ||
| }, | ||
|
|
||
| NO_REQUEST { | ||
| @Override | ||
| public void cache( | ||
| RedisTemplate<String, String> redisTemplate, String key, String value) { | ||
| redisTemplate.opsForSet().add(key, value); | ||
| } | ||
| }; | ||
|
|
||
| public abstract void cache( | ||
| RedisTemplate<String, String> redisTemplate, String key, String value); | ||
|
|
||
| public boolean canCache(String originalValue) { | ||
| this.name() | ||
| } | ||
| } |
There was a problem hiding this comment.
AttendanceSaveRequestStatus enum 구현에 몇 가지 심각한 문제가 있습니다.
- 잘못된 캐시 덮어쓰기 로직:
PROCESSING.cache의 로직은 주석("출석 성공은 다른 상태를 덮어 쓰고, 다른 상태는 출석 성공을 덮어 쓸 수 없다")과 반대로 동작합니다. 현재 코드는SUCCESS상태일 때만PROCESSING으로 덮어쓰도록 되어 있습니다. 또한,getAndDelete후 조건이 맞지 않으면 값을 다시 설정하지 않아 데이터가 유실될 수 있습니다. - 컴파일 오류:
SUCCESS.cache에서 호출하는redisTemplate.opsForValue().set(key, savedValue, value)는 존재하지 않는 메서드 시그니처이므로 컴파일 오류가 발생합니다. - Redis 데이터 타입 불일치:
PROCESSING,SUCCESS상태는 Redis의String타입을 사용(opsForValue)하는 반면,FAILED,NO_REQUEST상태는Set타입(opsForSet)을 사용하고 있습니다. 동일한 키에 대해 다른 데이터 타입을 사용하면 Redis에서WRONGTYPE오류가 발생하여 런타임에 장애를 일으킵니다. - 미완성 코드:
canCache메서드가 구현되지 않은 채로 남아있습니다.
이러한 문제들은 출석 상태 관리 로직의 오작동을 유발하므로 반드시 수정되어야 합니다.
| public void cache(AttendanceHistory attendanceHistory) { | ||
| attendanceHistory.getAttendanceSaveRequestStatus(); | ||
| String key = attendanceHistory.getKey(); | ||
| String savedStatus = redisTemplate.opsForValue().get(key); | ||
| redisTemplate.opsForSet() | ||
| .add(key, attendanceHistory.getTimestamp()); | ||
| redisTemplate.expire(key, ATTENDANCE_HISTORY_EXPIRE_HOUR, TimeUnit.HOURS); | ||
| } |
There was a problem hiding this comment.
cache 메서드에 여러 문제가 있습니다.
attendanceHistory.getTimestamp()를 호출하고 있는데,AttendanceHistory클래스에서getTimestamp()메서드가 제거되고getAttendanceSaveRequestStatus()로 변경되었기 때문에 컴파일 오류가 발생합니다.opsForValue().get(key)로String타입 값을 조회한 후,opsForSet().add(key, ...)로 동일한 키에Set타입 데이터를 추가하려고 시도하고 있습니다. 이는 Redis에서WRONGTYPE오류를 발생시키는 원인이 됩니다. 키 하나에는 하나의 데이터 타입만 저장할 수 있습니다.
AttendanceSaveRequestStatus enum의 로직과 연계하여 일관된 데이터 타입을 사용하도록 전체적인 재설계가 필요해 보입니다.
| datasource: | ||
| url: jdbc:h2:~/mem-data; | ||
| driverClassName: org.h2.Driver | ||
| username: sa | ||
| password: | ||
| url: jdbc:postgresql://jinho-database.ckddmrlccdc1.ap-northeast-2.rds.amazonaws.com:5432/imhere-dev | ||
| username: postgres | ||
| password: d0YhFa6o3HJ8GqOzKhF5 |
There was a problem hiding this comment.
테스트 설정 파일(application.yml)에 실제 데이터베이스(RDS)의 접속 정보와 인증 정보(username, password)가 하드코딩되어 있습니다. 인증 정보를 소스 코드에 포함하여 커밋하는 것은 매우 심각한 보안 취약점입니다. 이 정보가 유출될 경우 데이터베이스에 무단 접근이 가능해집니다. 환경 변수나 Jasypt와 같은 암호화 라이브러리를 사용하여 인증 정보를 안전하게 관리해야 합니다. 즉시 이력을 포함하여 저장소에서 해당 정보를 제거하는 조치가 필요합니다.
| log.info("[출석 완료] {}({}) , 학생 : {} ({})", | ||
| lecture::getLectureName, lecture::getId, | ||
| student::getUnivId, student::getName); |
There was a problem hiding this comment.
log.info 메서드에서 파라미터로 메서드 참조(lecture::getLectureName)를 사용하고 있습니다. SLF4J/Log4j2의 파라미터화된 로깅({})은 인자로 전달된 값의 toString()을 호출하여 로그 메시지를 만듭니다. 이 경우, 메서드 참조 객체의 toString() 결과가 출력되어 gdsc.binaryho.imhere.core.lecture.domain.Lecture$$...와 같이 의도하지 않은 로그가 남게 됩니다. 의도한 값을 로깅하려면 직접 메서드를 호출(lecture.getLectureName())해야 합니다.
| log.info("[출석 완료] {}({}) , 학생 : {} ({})", | |
| lecture::getLectureName, lecture::getId, | |
| student::getUnivId, student::getName); | |
| log.info("[출석 완료] {}({}) , 학생 : {} ({})", | |
| lecture.getLectureName(), lecture.getId(), | |
| student.getUnivId(), student.getName()); |
| @Transactional | ||
| public AttendanceNumberResponse openLectureAndGenerateAttendanceNumber(Long lectureId, int attendanceNumber) { | ||
| Lecture lecture = lectureRepository.findById(lectureId) | ||
| .orElseThrow(() -> LectureNotFoundException.EXCEPTION); | ||
| // authenticationHelper.verifyRequestMemberLogInMember(lecture.getMember().getId()); | ||
|
|
||
| lecture.setLectureState(LectureState.OPEN); | ||
| lecture.setLastOpeningTime(seoulDateTimeHolder.getSeoulDateTime()); | ||
|
|
||
| saveOpenLecture(lecture, attendanceNumber); | ||
| cacheAttendee(lecture); | ||
|
|
||
| log.info("[강의 OPEN] {} ({}), 출석 번호 : " + attendanceNumber | ||
| , lecture::getLectureName, lecture::getId); | ||
|
|
||
| return new AttendanceNumberResponse(attendanceNumber); | ||
| } |
There was a problem hiding this comment.
테스트용으로 추가된 openLectureAndGenerateAttendanceNumber 메서드에 몇 가지 문제가 있습니다.
// TODO : 테스트용주석과 함께 인증 로직(authenticationHelper.verifyRequestMemberLogInMember)이 주석 처리되어 있습니다. 이러한 코드가 프로덕션에 포함될 경우, 인증을 우회하여 누구나 강의를 열 수 있는 심각한 보안 허점이 될 수 있습니다. 테스트 전용 코드는src/test소스셋에 위치시키거나, 빌드 시 프로덕션 코드에 포함되지 않도록 프로필 분리 등의 조치가 필요합니다.log.info호출 시 메서드 참조를 사용하여 의도하지 않은 로그가 남게 됩니다. 실제 값을 로깅하도록 수정해야 합니다.
| StudentRecentAttendanceResponse response = new StudentRecentAttendanceResponse( | ||
| List.of(MOCK_ATTENDANCE.getTimestamp().toString())); | ||
| given(studentAttendanceService.getStudentRecentAttendance(lectureId)) | ||
| given(studentAttendanceService.getStudentRecentAttendanceStatus(lectureId)) | ||
| .willReturn(response); |
There was a problem hiding this comment.
학생은_최근_출석_정보를_조회할_수_있다 테스트가 변경된 API 명세와 맞지 않아 깨져 있습니다. StudentRecentAttendanceResponse의 생성자가 변경되었지만 테스트 코드에서는 이전 방식(new StudentRecentAttendanceResponse(List.of(...)))을 그대로 사용하고 있어 컴파일 오류가 발생합니다. 또한, 응답 필드가 timestamps에서 recentAttendanceStatus로 변경되었음에도 jsonPath("$.timestamps")를 검증하고 있습니다. 변경된 API에 맞게 테스트 코드를 수정해야 합니다.
| public boolean canCache(AttendanceSaveRequestStatus newValue) { | ||
|
|
||
| } |
There was a problem hiding this comment.
메서드 canCache가 비어있습니다. 미완성된 코드는 잠재적인 버그를 유발하거나 코드의 의도를 파악하기 어렵게 만들 수 있으므로, 구현을 완료하거나 필요 없는 코드라면 삭제하는 것이 좋습니다.
| public Boolean isStudentOpenLectureExist(Long studentId, Long lectureId) { | ||
| Set<Long> openLectureIds = findAllAttendLectureId(studentId); | ||
| return openLectureIds.contains(lectureId); | ||
| } |
There was a problem hiding this comment.
isStudentOpenLectureExist 메서드가 학생이 수강하는 모든 열린 강의 ID를 가져온 후(findAllAttendLectureId), Java 코드 내에서 특정 강의 ID가 포함되어 있는지 확인하고 있습니다. 학생이 수강하는 강의가 많아질수록 불필요한 데이터를 많이 전송받게 되어 성능 저하의 원인이 될 수 있습니다. Redis의 SISMEMBER 명령어를 사용하면 특정 Set에 원소가 존재하는지 여부를 O(1) 시간 복잡도로 확인할 수 있습니다. redisTemplate.opsForSet().isMember(key, member)를 사용하여 더 효율적으로 구현하는 것을 권장합니다.
| public Boolean isStudentOpenLectureExist(Long studentId, Long lectureId) { | |
| Set<Long> openLectureIds = findAllAttendLectureId(studentId); | |
| return openLectureIds.contains(lectureId); | |
| } | |
| public Boolean isStudentOpenLectureExist(Long studentId, Long lectureId) { | |
| String key = KEY_PREFIX + studentId; | |
| return redisTemplate.opsForSet().isMember(key, String.valueOf(lectureId)); | |
| } |
PR Review 테스트용으로 버린 변경사항으로 Open \n test
test
test
test
Summary by CodeRabbit