Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ public class RedisConfig {

@Bean
public RedisConnectionFactory redisConnectionFactory() {
RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort);
// RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort);
RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration("localhost", 6379);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Redis 연결 정보가 하드코딩되어 있습니다. 이전처럼 @Value 어노테이션을 사용하여 application.yml 파일에서 설정을 읽어오도록 수정해야 합니다. 이렇게 하면 다른 환경(개발, 운영 등)에서 유연하게 설정을 변경할 수 있습니다.

Suggested change
RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration("localhost", 6379);
RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Redis 연결 정보가 "localhost:6379"로 하드코딩되어 있습니다. 기존에 @Value를 통해 설정 파일에서 값을 읽어오던 방식이 주석 처리되어 있는데, 이럴 경우 로컬 개발 환경을 제외한 다른 환경(테스트, 운영 등)에서 애플리케이션이 동작하지 않습니다. 테스트를 위해 임시로 변경한 것이라면, 커밋에는 포함되지 않도록 원복해야 합니다.

Suggested change
RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration("localhost", 6379);
RedisStandaloneConfiguration configuration = new RedisStandaloneConfiguration(redisHost, redisPort);

configuration.setPassword(redisPassword);
Comment on lines +26 to 28
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
// 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.


return new LettuceConnectionFactory(configuration);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package gdsc.binaryho.imhere.core.attendance.application;

import lombok.Getter;
import lombok.RequiredArgsConstructor;

@Getter
@RequiredArgsConstructor
public class AttendanceFailedEvent {

private final long lectureId;
private final long studentId;
private final Throwable exception;
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
package gdsc.binaryho.imhere.core.attendance.application;

import static gdsc.binaryho.imhere.core.attendance.domain.AttendanceHistory.createAcceptedAttendanceHistory;
import static gdsc.binaryho.imhere.core.attendance.domain.AttendanceHistory.createAwaitAttendanceHistory;
import static gdsc.binaryho.imhere.core.attendance.domain.AttendanceHistory.createFailedAttendanceHistory;

import gdsc.binaryho.imhere.core.attendance.application.port.AttendanceHistoryCacheRepository;
import gdsc.binaryho.imhere.core.attendance.domain.AttendanceHistory;
import lombok.RequiredArgsConstructor;
import lombok.extern.log4j.Log4j2;
import org.springframework.scheduling.annotation.Async;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.event.TransactionPhase;
import org.springframework.transaction.event.TransactionalEventListener;

@Log4j2
@Service
@RequiredArgsConstructor
public class AttendanceHistoryCacheService {
Expand All @@ -19,13 +25,41 @@ public class AttendanceHistoryCacheService {
@Async
@Transactional(propagation = Propagation.REQUIRES_NEW)
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
public void cache(StudentAttendedEvent event) {
public void cacheAttendanceHistory(AttendanceRequestedEvent event) {
long lectureId = event.getLectureId();
long studentId = event.getStudentId();
String timestamp = event.getTimestamp().toString();

AttendanceHistory attendanceHistory = AttendanceHistory.of(
lectureId, studentId, timestamp);
AttendanceHistory attendanceHistory = createAwaitAttendanceHistory(lectureId, studentId);
attendanceHistoryCacheRepository.cache(attendanceHistory);
}

@Async
@Transactional(propagation = Propagation.REQUIRES_NEW)
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
public void saveAttendance(AttendanceSaveSucceedEvent event) {
long lectureId = event.getLectureId();
long studentId = event.getStudentId();

AttendanceHistory attendanceHistory = createAcceptedAttendanceHistory(lectureId, studentId);
attendanceHistoryCacheRepository.cache(attendanceHistory);
}

@Async
@Transactional(propagation = Propagation.REQUIRES_NEW)
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
public void saveAttendance(AttendanceFailedEvent event) {
long lectureId = event.getLectureId();
long studentId = event.getStudentId();

AttendanceHistory attendanceHistory = createFailedAttendanceHistory(lectureId, studentId);
attendanceHistoryCacheRepository.cache(attendanceHistory);
logAttendanceFailed(studentId, lectureId, event.getException());
}

private void logAttendanceFailed(Long studentId, Long lectureId, RuntimeException exception) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

logAttendanceFailed 메서드의 exception 파라미터 타입이 RuntimeException으로 선언되어 있습니다. 하지만 AttendanceFailedEventThrowable 타입의 예외를 가지고 있어, Error와 같은 RuntimeException이 아닌 예외가 발생할 경우 ClassCastException이 발생할 수 있습니다. 파라미터 타입을 Throwable로 변경하여 안정성을 높이는 것이 좋습니다.

Suggested change
private void logAttendanceFailed(Long studentId, Long lectureId, RuntimeException exception) {
private void logAttendanceFailed(Long studentId, Long lectureId, Throwable exception) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

logAttendanceFailed 메서드를 호출할 때 event.getException()을 인자로 전달하고 있습니다. AttendanceFailedEventexception 필드는 Throwable 타입이지만, logAttendanceFailed 메서드는 RuntimeException 타입의 파라미터를 받도록 선언되어 있습니다. 만약 RuntimeException이 아닌 다른 Throwable(예: checked exception)이 이벤트에 담겨 전달될 경우, 이 지점에서 ClassCastException이 발생하여 비동기 이벤트 처리 로직이 중단될 수 있습니다. logAttendanceFailed 메서드의 파라미터 타입을 Throwable로 변경하여 안정성을 확보해야 합니다.

Suggested change
private void logAttendanceFailed(Long studentId, Long lectureId, RuntimeException exception) {
private void logAttendanceFailed(Long studentId, Long lectureId, Throwable exception) {

String exceptionName = exception.getClass().getName();
String exceptionMessage = exception.getMessage();
log.info("[출석 실패] 학생 id {}, 수업 id : {}, 예외 이름 : {}, 예외 메시지 : {}",
studentId, lectureId, exceptionName, exceptionMessage);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package gdsc.binaryho.imhere.core.attendance.application;

import lombok.Getter;
import lombok.RequiredArgsConstructor;

@Getter
@RequiredArgsConstructor
public class AttendanceRequestedEvent {

private final long lectureId;
private final long studentId;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package gdsc.binaryho.imhere.core.attendance.application;

import org.springframework.data.redis.core.RedisTemplate;

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);
}
},
Comment on lines +22 to +29
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.


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()
}
Comment on lines +50 to +52
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

컴파일 에러: 불완전한 메서드

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.

Suggested change
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.

}
Comment on lines +12 to +53

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

이 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 값을 반환해야 하지만 반환문이 없어 컴파일 오류가 발생합니다.

전체적인 로직을 재검토하고 수정해야 합니다.

Comment on lines +5 to +53

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

AttendanceSaveRequestStatus enum 구현에 몇 가지 심각한 문제가 있습니다.

  1. 잘못된 캐시 덮어쓰기 로직: PROCESSING.cache의 로직은 주석("출석 성공은 다른 상태를 덮어 쓰고, 다른 상태는 출석 성공을 덮어 쓸 수 없다")과 반대로 동작합니다. 현재 코드는 SUCCESS 상태일 때만 PROCESSING으로 덮어쓰도록 되어 있습니다. 또한, getAndDelete 후 조건이 맞지 않으면 값을 다시 설정하지 않아 데이터가 유실될 수 있습니다.
  2. 컴파일 오류: SUCCESS.cache에서 호출하는 redisTemplate.opsForValue().set(key, savedValue, value)는 존재하지 않는 메서드 시그니처이므로 컴파일 오류가 발생합니다.
  3. Redis 데이터 타입 불일치: PROCESSING, SUCCESS 상태는 Redis의 String 타입을 사용(opsForValue)하는 반면, FAILED, NO_REQUEST 상태는 Set 타입(opsForSet)을 사용하고 있습니다. 동일한 키에 대해 다른 데이터 타입을 사용하면 Redis에서 WRONGTYPE 오류가 발생하여 런타임에 장애를 일으킵니다.
  4. 미완성 코드: canCache 메서드가 구현되지 않은 채로 남아있습니다.

이러한 문제들은 출석 상태 관리 로직의 오작동을 유발하므로 반드시 수정되어야 합니다.

Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package gdsc.binaryho.imhere.core.attendance.application;

import gdsc.binaryho.imhere.core.attendance.domain.Attendance;
import gdsc.binaryho.imhere.core.attendance.infrastructure.AttendanceRepository;
import gdsc.binaryho.imhere.core.lecture.domain.Lecture;
import gdsc.binaryho.imhere.core.member.Member;
import lombok.RequiredArgsConstructor;
import lombok.extern.log4j.Log4j2;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Log4j2
@Service
@RequiredArgsConstructor
public class AttendanceSaveService {

private final AttendanceRepository attendanceRepository;

@Transactional
public void save(Attendance attendance) {
attendanceRepository.save(attendance);
logAttendanceHistory(attendance);
}

private void logAttendanceHistory(Attendance attendance) {
Member student = attendance.getStudent();
Lecture lecture = attendance.getLecture();
log.info("[출석 완료] {}({}) , 학생 : {} ({})",
lecture::getLectureName, lecture::getId,
student::getUnivId, student::getName);
Comment on lines +28 to +30

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

log.info 메서드에서 파라미터로 메서드 참조(lecture::getLectureName)를 사용하고 있습니다. SLF4J/Log4j2의 파라미터화된 로깅({})은 인자로 전달된 값의 toString()을 호출하여 로그 메시지를 만듭니다. 이 경우, 메서드 참조 객체의 toString() 결과가 출력되어 gdsc.binaryho.imhere.core.lecture.domain.Lecture$$...와 같이 의도하지 않은 로그가 남게 됩니다. 의도한 값을 로깅하려면 직접 메서드를 호출(lecture.getLectureName())해야 합니다.

Suggested change
log.info("[출석 완료] {}({}) , 학생 : {} ({})",
lecture::getLectureName, lecture::getId,
student::getUnivId, student::getName);
log.info("[출석 완료] {}({}) , 학생 : {} ({})",
lecture.getLectureName(), lecture.getId(),
student.getUnivId(), student.getName());

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package gdsc.binaryho.imhere.core.attendance.application;

import lombok.Getter;
import lombok.RequiredArgsConstructor;

@Getter
@RequiredArgsConstructor
public class AttendanceSaveSucceedEvent {

private final long lectureId;
private final long studentId;
}
Loading
Loading