[육선우] sprint11 - #244
Hidden character warning
[육선우] sprint11#244ryuk6238-dot wants to merge 101 commits into
Conversation
…ser and Channel api
…dd UserAuthApi and ReadStatusApi
joonfluence
left a comment
joonfluence
left a comment
There was a problem hiding this comment.
sprint11 리뷰 요약
Spring Event 기반 파일 업로드 분리·알림, 비동기, Redis 캐시, Kafka 도입을 구현하셨습니다. 구조(TransactionalEventListener AFTER_COMMIT, MdcTaskDecorator, 전용 TaskExecutor)는 학습 의도를 잘 따르고 있습니다. 다만 아래 두 가지로 알림 생성/조회 플로우가 실제로는 동작하지 않아 수정이 필요합니다.
- 알림 생성: in-process 리스너(
NotificationRequiredEventListener)가//@Component로 비활성화되어 있고, 대체 경로인 Kafka 컨슈머(NotificationRequiredTopicListener)는 로그만 남기고 알림을 저장하지 않습니다. → 어느 경로로도 알림이 생성되지 않습니다. - 알림 조회:
findAllByReceiverId에@Cacheable이 아닌@CacheEvict가 붙어 있고 SpEL#request.receiverId()가 존재하지 않는 파라미터를 참조해 호출 시 예외가 발생합니다.
라인 코멘트로 상세 남겼습니다.
추가(라인 지정 불가)
- 저장소 위생(P3): 루트의
.DS_Store,.logs/*.log(예:application.2026-05-13.log6.8만 줄),data/storage/*,.discodeit/*.ser산출물이 커밋되어 추가분 18만 줄의 대부분을 차지합니다.discodeit/.gitignore는 하위 디렉터리에만 적용되니 **루트.gitignore**에.logs/,data/,.discodeit/,.DS_Store를 추가하고 이미 추적 중인 파일은git rm --cached로 제거해 주세요. - 캐시 이름 불일치(P3):
application.yaml의cache-names는channels/notifications/users인데 코드는channelsByUserId/notificationsByUserId/allUsers를 사용합니다. 선언된 이름이 실제로 쓰이지 않아 혼란을 줍니다. 일치시켜 주세요.
전반적으로 이벤트→리스너→비동기 흐름의 뼈대는 잘 잡으셨습니다. 위 동작 이슈만 정리되면 요구사항 충족에 가까워집니다. 셀프 리뷰 코멘트도 기대하겠습니다. 👍
|
|
||
| private final NotificationRepository notificationRepository; | ||
|
|
||
| @CacheEvict(cacheNames = "notificationsByUserId", key = "#request.receiverId()") |
There was a problem hiding this comment.
P1. 조회 메서드에 잘못된 @CacheEvict + 존재하지 않는 SpEL 변수
두 문제가 겹쳐 있습니다.
- 조회 메서드인데
@CacheEvict라 캐시가 채워지지 않고 매번 무효화만 됩니다. 캐싱 의도라면@Cacheable이어야 합니다. - SpEL
#request.receiverId()가 참조하는request파라미터가 메서드에 없습니다(파라미터는receiverId).@EnableCaching이 켜진 상태에서 캐시가 resolve되면 키 평가 시SpelEvaluationException(EL1008E)이 발생해 GET /api/notifications가 500으로 떨어집니다.
| @CacheEvict(cacheNames = "notificationsByUserId", key = "#request.receiverId()") | |
| @Cacheable(cacheNames = "notificationsByUserId", key = "#receiverId") |
더불어 41행 checkAndDelete의 allEntries = true도 사용자별 캐시이므로 key = "#requesterId" 단위 무효화를 고려해 주세요.
| try { | ||
| MessageCreatedEvent event = objectMapper.readValue(kafkaEvent, MessageCreatedEvent.class); | ||
|
|
||
| log.info("MessageCreatedEvent 기반 알림 생성 및 발송 성공 완료!"); |
There was a problem hiding this comment.
P2. Kafka 컨슈머가 알림을 생성하지 않습니다.
역직렬화 후 "알림 생성 및 발송 성공 완료!" 로그만 남기고 실제로 NotificationService/NotificationRepository를 호출해 알림을 저장하지 않습니다. 요구사항 "알림이 필요한 이벤트가 발행되었을 때 알림을 생성하세요"가 미충족 상태입니다(41~48행 RoleUpdated도 동일). 컨슈머에서 알림 생성 로직을 호출해 주세요.
| import org.springframework.transaction.event.TransactionalEventListener; | ||
|
|
||
| @Slf4j | ||
| //@Component |
There was a problem hiding this comment.
P2. 알림 리스너가 //@Component로 비활성화되어 있습니다.
실제 알림을 DB에 저장하는 이 리스너가 주석 처리되어 빈 등록이 안 됩니다. 위 Kafka 컨슈머도 알림을 저장하지 않으므로, 메시지/권한 알림이 어느 경로로도 생성되지 않습니다.
Kafka 경로로 일원화할 의도라면 컨슈머 쪽에 생성 로직을 구현하고 이 클래스는 정리하고, in-process 경로를 쓸 의도라면 활성화해 주세요. (활성화 시 73행 previousRoleName = "USER" 하드코딩은 실제 이전 권한이 아니며, 77행 com.sprint.mission.discodeit.entity.User 풀패키지 인라인 참조는 import 권장)
| private final KafkaTemplate<String, String> kafkaTemplate; | ||
| private final ObjectMapper objectMapper; | ||
|
|
||
| @Async("eventTaskExecutor") |
There was a problem hiding this comment.
P2. 존재하지 않는 Executor 빈을 참조합니다.
@Async("eventTaskExecutor")로 지정했지만 AsyncConfig에는 taskExecutor 빈만 정의돼 있고 eventTaskExecutor는 없습니다(38행도 동일). 지정 qualifier의 Executor를 찾지 못해 의도한 풀이 쓰이지 않거나 비동기 실행이 실패할 수 있습니다. "taskExecutor"로 맞추거나 전용 eventTaskExecutor 빈을 추가해 주세요.
| log.info("MessageCreatedEvent 기반 알림 생성 및 발송 성공 완료!"); | ||
| } catch (JsonProcessingException e) { | ||
| log.error("MessageCreatedEvent 카프카 메시지 역직렬화(Parsing) 실패", e); | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
P3. 역직렬화 실패 시 무한 재시도(poison pill) 위험
catch에서 throw new RuntimeException(e)만 하면 오프셋이 커밋되지 않아 동일 메시지를 무한 재소비합니다(51행도 동일). DefaultErrorHandler(백오프 + 재시도 한도) 또는 DLT 설정으로 독성 메시지를 격리하는 것을 권장합니다.
| private final BinaryContentStorage binaryContentStorage; | ||
| private final BinaryContentService binaryContentService; | ||
|
|
||
| @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) |
There was a problem hiding this comment.
P3. 스토리지 저장이 비동기가 아닙니다.
@TransactionalEventListener(AFTER_COMMIT)만 있고 @Async가 없어 바이너리 저장 I/O가 커밋 직후 요청 스레드에서 동기로 수행됩니다. "비동기 적용" 요구사항 취지에 맞게 @Async("taskExecutor") 추가를 고려해 주세요. (단, async로 전환하면 예외가 호출 스레드로 전파되지 않으므로 현재의 FAIL 상태 반영 로직이 더 중요해집니다.)
| import org.springframework.scheduling.annotation.EnableAsync; | ||
|
|
||
| @Configuration | ||
| @EnableAsync |
There was a problem hiding this comment.
P4. @EnableAsync 오기재(복붙 추정)
@EnableCaching은 이미 DiscodeitApplication에 있고, 캐시 설정 클래스에 @EnableAsync가 붙어 있어 의미상 맞지 않고 AsyncConfig와 중복됩니다. 제거를 권장합니다.
| @Bean | ||
| public RedisCacheConfiguration redisCacheConfiguration(ObjectMapper objectMapper) { | ||
| ObjectMapper redisObjectMapper = objectMapper.copy(); | ||
| redisObjectMapper.activateDefaultTyping( |
There was a problem hiding this comment.
P4. DefaultTyping.EVERYTHING + LaissezFaireSubTypeValidator
역직렬화 가젯 위험을 줄이기 위해 타입 정보 범위는 NON_FINAL 정도로 좁히는 것을 권장합니다. 학습용 캐시이지만 습관화 차원에서 남깁니다.
2 participants
요구사항
디스코드잇은 BinaryContent의 메타 데이터(DB)와 바이너리 데이터(FileSystem/S3)를 분리해 저장합니다.
이벤트를 받아 실제 바이너리 데이터를 저장하는 리스너를 구현하세요.
알림 API를 구현하세요
알림이 필요한 이벤트가 발행되었을 때 알림을 생성하세요.
비동기 실패 처리하기
캐시 적용하기
Kafka 환경을 구성하세요.
KafkaProduceRequiredEventListener를 구현하세요.
NotificationRequiredTopicListener를 구현하세요.
Docker Compose를 활용해 Redis를 구동하세요.
의존성을 추가하고, application.yml에 Redis 설정을 추가하세요.
Bean을 선언
기본
심화
주요 변경사항
스크린샷
멘토에게