[한성재] sprint11 by seonghj · Pull Request #243 · codeit-bootcamp-spring/9-sprint-mission

seonghj · 2026-06-14T05:49:52Z

요구사항

기본

심화

Spring Kafka 도입하기
Redis Cache 도입하기

멘토에게

셀프 코드 리뷰를 통해 질문 이어가겠습니다.

기본 요구사함 프로파일 기반 설정 관리에 맞춘 프로파일 추가

Spring Validation 의존성을 추가

주요 Request에 제약 조건 관련 어노테이션 추가, GlobalExceptionHandler에 handleMethodArgumentNotValidException 추가

This reverts commit d399fa6.

joonfluence

전체 리뷰 요약

sprint11 요구사항(파일 업로드 분리·알림·비동기/실패처리·캐시·Kafka·Redis)을 폭넓게 구현했습니다. @TransactionalEventListener(AFTER_COMMIT) + @Async + REQUIRES_NEW 조합으로 커밋 후 비동기 처리하는 흐름, @Retryable/@Recover를 통한 S3 업로드 실패 복구 체계가 잘 잡혀 있습니다. 특히 이벤트 리스너·재시도·알림이 모두 주입된 빈(프록시)을 통한 외부 호출로 설계되어 self-invocation으로 인한 @Async/@Cacheable/@Transactional 프록시 우회 함정을 피했습니다 👍

다만 아래 P1·P2 항목은 반영이 필요해 REQUEST_CHANGES로 남깁니다.

P2 — `gradlew` 실행 권한 소실 (인라인 불가 항목)

gradlew의 파일 모드가 100755 → 100644로 변경되어 실행 권한이 사라졌습니다. deploy.yml/test.yml이 ./gradlew를 직접 호출하므로 CI에서 Permission denied로 빌드가 실패할 수 있습니다. 아래로 복구해 주세요.

git update-index --chmod=+x discodeit/gradlew

추가 참고 (낮은 우선순위)

(P4) BinaryContentCreatedEvent(byte[] bytes)가 AFTER_COMMIT 비동기 처리까지 파일 전체 바이트를 메모리에 보유합니다. 대용량/동시 업로드 시 메모리 압박이 우려되니 스트리밍 또는 임시 저장 후 키 전달을 고려해 보세요.
(P4) BinaryContentStorage 인터페이스에 추가된 recover(...)는 Spring Retry @Recover 구현 세부사항이라 인터페이스 계약에 두기엔 어색합니다. 구현체 내부로 한정하는 편이 자연스럽습니다.
(P3) BasicNotificationService.createMessageNotification의 수동 cache.evict(및 @CacheEvict)는 트랜잭션 커밋 전에 실행되어, 동시성 상황에서 다른 스레드가 커밋 전 데이터를 다시 캐싱할 여지가 있습니다.

전반적으로 sprint11 심화 요구사항까지 완성도 높게 구현하셨습니다. 위 보안/안정성 항목만 정리되면 좋겠습니다. 수고하셨습니다! 🙇

joonfluence · 2026-06-17T01:05:41Z

-  port: ${PORT:8080}
+jwt:
+  secret: bXktand0LXNlY3JldC1rZXktZm9yLXNwcmluZy1zZWN1cml0eQ==
+  access-token-validity-seconds: 30


[P1] 꼭 반영해주세요 — JWT 서명 비밀키가 레포지토리에 평문으로 커밋되어 있습니다. Base64는 인코딩일 뿐 암호화가 아니라 디코딩하면 my-jwt-secret-key-for-spring-security가 그대로 드러납니다. 저장소 접근 권한이 있는 누구나 임의 사용자의 토큰을 위조할 수 있는 심각한 취약점입니다. 다른 비밀값(DB/AWS)처럼 환경변수로 외부화해 주세요. 이미 노출된 키이므로 키 자체도 폐기 후 재발급이 필요합니다.

Suggested change

access-token-validity-seconds: 30

secret: ${JWT_SECRET}

joonfluence · 2026-06-17T01:05:41Z

-            userStatusRepository.deleteById(userStatus.getId());
-            binaryContentRepository.deleteById(removeUser.getProfile().getId());
-            userRepository.deleteById(removeUser.getId());
+          binaryContentRepository.deleteById(removeUser.getProfile().getId());


[P2] 적극적으로 고려해주세요 — 프로필은 nullable이고(create에서 orElse(null), AdminInitializer도 null로 생성) 실제로 프로필 없는 사용자가 존재할 수 있는데, getProfile()이 null이면 NPE가 발생해 RuntimeException으로 사용자 삭제가 항상 실패합니다. null 가드를 추가해 주세요.

if (removeUser.getProfile() != null) { binaryContentRepository.deleteById(removeUser.getProfile().getId()); }

joonfluence · 2026-06-17T01:05:41Z

    }

    @Override
+    @Cacheable(cacheNames = "users")


[P2] 적극적으로 고려해주세요 — findAll이 캐시하는 DTO에 getOnlineUserIds()로 **실시간 접속 상태(online)**가 함께 담깁니다. TTL 600초 + 사용자 CRUD 시에만 evict 되므로, 로그인/로그아웃으로 수시로 바뀌는 접속 상태가 최대 10분간 stale 상태로 노출됩니다. 캐시에는 변하지 않는 데이터만 담고 접속 상태는 조회 시점에 별도로 합성하는 구조를 권장합니다. (BasicChannelService.findAllByUserId도 동일 이슈)

joonfluence · 2026-06-17T01:05:41Z

    }

    @Override
+    @Cacheable(cacheNames = "userChannels", key = "#userId")


[P2] 적극적으로 고려해주세요 — findAllByUserId 캐시 DTO에도 jwtRegistry.getActiveUserIds()(실시간 접속 상태)가 포함됩니다. 채널 CRUD 시에만 evict 되어 접속 상태가 stale 해집니다. BasicUserService.findAll과 동일하게, 휘발성 접속 정보는 캐시에서 분리해 조회 시점에 합성하는 것을 권장합니다.

joonfluence · 2026-06-17T01:05:41Z

+jwt:
+  secret: bXktand0LXNlY3JldC1rZXktZm9yLXNwcmluZy1zZWN1cml0eQ==
+  access-token-validity-seconds: 30
+  refresh-token-validity-seconds: 604800


[P3] 웬만하면 반영해 주세요 — 액세스 토큰 유효시간이 30초입니다. 테스트 목적이 아니라면 의도와 달라 보입니다(예: 30분이면 1800). 의도된 값인지 확인 부탁드립니다.

joonfluence · 2026-06-17T01:05:41Z

+  }
+
+  @Retryable(
+      retryFor = {Exception.class},


[P3] 웬만하면 반영해 주세요 — retryFor = {Exception.class}로 모든 예외에 재시도하면 인증 오류·잘못된 요청 같은 영구적 실패에도 2s+4s 백오프를 낭비합니다. 일시적 오류(예: 5xx S3Exception/SdkClientException/IOException)로 좁히는 것을 권장합니다.

joonfluence · 2026-06-17T01:05:42Z

+
+  cache:
+    type: redis
+    cache-names:


[P3] 웬만하면 반영해 주세요 — 코드가 실제 사용하는 캐시명은 userChannels, userNotifications, users인데 여기 선언된 이름은 channels/notifications로 불일치합니다(이 선언이 무의미해짐). 또한 cache.type: redis인데 아래 caffeine.spec이 함께 있어 혼선이 있으니 한쪽으로 정리해 주세요.

joonfluence · 2026-06-17T01:05:42Z

  application:
    name: discodeit
+  main:
+    allow-bean-definition-overriding: true


[P3] 웬만하면 반영해 주세요 — 운영 프로파일에도 빈 오버라이딩이 켜져 있으면 중복 빈 정의 같은 설정 실수를 가려버립니다. 가능하면 테스트 프로파일(application-test.yaml)에만 한정해 주세요.

joonfluence · 2026-06-17T01:05:42Z

+import org.springframework.stereotype.Component;
+import org.springframework.transaction.event.TransactionalEventListener;
+
+//@Component


[P5] 사소한 의견 — Kafka 기반 리스너로 대체되어 //@Component로 비활성화된 클래스 전체가 주석으로 남아 있습니다. 데드코드는 삭제하는 편이 깔끔합니다.

joonfluence · 2026-06-17T01:05:42Z

    }
+
+//    try {
+//      Thread.sleep(3000);


[P5] 사소한 의견 — 지연 시뮬레이션용 Thread.sleep(3000) 주석 코드가 남아 있습니다. 제거 권장합니다.

seonghj and others added 30 commits March 25, 2026 13:38

dev + prod.yaml 생성

bb00da3

기본 요구사함 프로파일 기반 설정 관리에 맞춘 프로파일 추가

로그 적용 + 커스텀 예외 생성

99cf940

서비스에 커스텀 예외 적용

2e4dd0b

GlobalExceptionHandler 수정

a1e0ea9

build.gradle 수정

8f652c7

Spring Validation 의존성을 추가

유효성 검사 구현

0666e59

주요 Request에 제약 조건 관련 어노테이션 추가, GlobalExceptionHandler에 handleMethodArgumentNotValidException 추가

Spring Boot Actuator 추가

c461643

UserService 단위 테스트

dc3e32f

단위 테스트 BDDMockito 적용

60dfe8a

통합 테스트 추가

cf2ffc2

심화 구현 요소 추가

497032a

controller 로그 추가

5fdcb29

BinaryContentStorage 고도화

50e1c1d

s3BinaryContentStorageTest

ae45811

Update Dockerfile

c238858

Create test.yml

f95e257

Update build.gradle

42d7813

Update test.yml

3186279

test.yml working-directory 추가

ed3dff8

Update test.yml

b531bed

Update test.yml

bd60909

Update test.yml

7e8e537

build.gradle 수정

6ac825b

AwsProperties 수정

c2a59a4

..

3e566d3

Update test.yml

09633ca

Create README.md

2190a15

Create deploy.yml

dbecb6d

Update deploy.yml

cc38a36

Update deploy.yml

ecffc5d

seonghj added 29 commits April 10, 2026 18:47

Revert "Update deploy.yml"

854733c

This reverts commit d399fa6.

피드백 반영

5900baf

findAllByChannel_Id_Success 수정

01c113f

Update README.md

795490f

Update README.md

f58d53e

정적 리소스 변경

fcad759

회원가입/로그인 수정

6761350

로그아웃 추가

648228e

권한 정의

ff04a81

인가 - 권한 적용

2cf11e6

세션관리 고도화

d9e2e2e

로그인 고도화

0c31ce5

권한 적용 고도화

4f725f5

refactor: filterChain

51f5a86

fix: 회원가입 오류 수정

7c4b6b3

Apply .gitignore properly

6580aef

Remove tracked logs folder

edd59b4

JWT 컴포넌트 구현

256f3f6

기본 요구사항

a65071e

리팩토링 - 토큰 상태 관리

cd94ca4

fix: 피드백 반영

ae5185f

파일 업로드 로직 분리하기

d95d90c

알림 기능 추가하기

cebaf59

비동기 적용하기

40138e1

비동기 실패 처리하기

d851365

캐시 적용하기

ce32405

Spring Kafka 도입하기

c8db71f

Redis Cache 도입하기

55fe750

피드백 반영

63877be

joonfluence suggested changes Jun 17, 2026

View reviewed changes

Navigation Menu

Uh oh!

[한성재] sprint11 - #243

Hidden character warning

[한성재] sprint11#243
seonghj wants to merge 73 commits into
codeit-bootcamp-spring:한성재from
seonghj:한성재_sprint11

seonghj commented Jun 14, 2026

Uh oh!

joonfluence left a comment

Uh oh!

joonfluence Jun 17, 2026

Uh oh!

joonfluence Jun 17, 2026

Uh oh!

joonfluence Jun 17, 2026

Uh oh!

joonfluence Jun 17, 2026

Uh oh!

joonfluence Jun 17, 2026

Uh oh!

joonfluence Jun 17, 2026

Uh oh!

joonfluence Jun 17, 2026

Uh oh!

joonfluence Jun 17, 2026

Uh oh!

joonfluence Jun 17, 2026

Uh oh!

joonfluence Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Hidden character warning

Conversation

seonghj commented Jun 14, 2026

요구사항

기본

심화

멘토에게

Uh oh!

joonfluence left a comment

Choose a reason for hiding this comment

전체 리뷰 요약

P2 — gradlew 실행 권한 소실 (인라인 불가 항목)

추가 참고 (낮은 우선순위)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

P2 — `gradlew` 실행 권한 소실 (인라인 불가 항목)