-
Notifications
You must be signed in to change notification settings - Fork 1
[KUIT-58] AWS SQS 기반 디스코드 역할 동기화 #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- SqsConfig 추가 및 SQS 리스너 설정 적용 - RoleChangeListener로 SQS 이벤트 수신 및 재시도 정책 적용 - RoleChangeEventService 처리 파이프라인 구현 - ProcessedEventLog 엔티티/리포지토리로 eventId 기반 중복 처리 방지
Walkthrough디스코드 봇이 Guild 멤버 역할 변경을 감지해 SQS에 이벤트를 발행하고, Spring 워커가 SQS 메시지를 롱폴링으로 수신해 멱등 처리 후 DB와 멤버 역할을 동기화하도록 AWS SDK v1 → v2 마이그레이션 및 배포/CI 수정이 포함된 기능 추가 및 관련 리팩터링이 이루어졌습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor DiscordUser as Discord 사용자
participant DiscordBot as Discord Bot
participant SQS as AWS SQS
participant SpringWorker as Spring 워커
participant DB as Database
rect rgb(220,235,255)
Note over DiscordUser,DiscordBot: 역할 변경 감지
DiscordUser->>DiscordBot: GUILD_MEMBER_UPDATE 이벤트
DiscordBot->>DiscordBot: 부분 멤버 fetch → 역할 diff 계산
end
rect rgb(255,245,220)
Note over DiscordBot,SQS: 이벤트 발행
DiscordBot->>DiscordBot: eventId(UUID), occurredAt 생성
DiscordBot->>SQS: SendMessage(DiscordRoleChangeEvent JSON)
SQS-->>DiscordBot: SendMessageResponse
end
rect rgb(225,255,220)
Note over SQS,SpringWorker: 메시지 수신(롱폴링)
SpringWorker->>SQS: ReceiveMessage (long poll)
SQS-->>SpringWorker: Message(body)
end
rect rgb(250,240,240)
Note over SpringWorker,DB: 멱등 처리 및 적용
SpringWorker->>SpringWorker: parse → validate eventId
SpringWorker->>DB: insert ProcessedEventLog (멱등 시 예외 발생)
alt already processed
DB-->>SpringWorker: constraint violation
SpringWorker->>SQS: DeleteMessage (skip reprocess)
else new event
DB-->>SpringWorker: inserted
SpringWorker->>SpringWorker: RoleService.applyDiscordRoleChangeEvent
SpringWorker->>DB: MemberRole insert/delete
SpringWorker->>SQS: DeleteMessage
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/deploy.sh (1)
74-78: 네트워크 연결 순서 오류: 컨테이너 시작 전에 네트워크 연결을 시도합니다.
blue컨테이너를 시작하기 전에 네트워크 연결을 시도하고 있어, 컨테이너가 존재하지 않으면 실패합니다. Green 배포 흐름(라인 46, 49)과 동일하게 순서를 맞춰야 합니다.🔎 제안하는 수정
echo "1. get blue image" docker compose -f /home/ubuntu/docker-compose.yml pull blue - # connect blue container to monitoring network (ignore error if already connected) - docker network connect monitoring blue 2>/dev/null || true - echo "2. blue container up" docker compose -f /home/ubuntu/docker-compose.yml up -d blue + + # connect blue container to monitoring network (ignore error if already connected) + docker network connect monitoring blue 2>/dev/null || true
🧹 Nitpick comments (11)
discord-bot/Dockerfile (2)
1-1: 베이스 이미지 버전을 명시적으로 고정하는 것을 권장합니다.
node:20-alpine대신node:20.18.1-alpine같이 구체적인 버전을 명시하면 빌드 재현성이 향상됩니다.🔎 권장 수정사항
-FROM node:20-alpine +FROM node:20.18.1-alpine
1-13: 보안 및 운영 개선을 위한 추가 설정 고려.다음 개선사항을 고려해보세요:
- 비root 사용자로 컨테이너 실행 (보안 강화)
- HEALTHCHECK 지시자 추가 (오케스트레이션 환경에서 컨테이너 상태 모니터링 개선)
🔎 권장 개선안
FROM node:20-alpine WORKDIR /app # 의존성 설치 COPY package*.json ./ RUN npm ci --omit=dev # 소스 복사 COPY . . # 비root 사용자로 전환 USER node # 헬스체크 추가 HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ CMD node -e "console.log('healthy')" || exit 1 # 실행 CMD ["node", "index.js"]docker-compose.yml (1)
22-32: Discord Bot 서비스 구성이 적절합니다.
restart: unless-stopped설정으로 안정성이 확보되었습니다.선택적 개선 사항으로 health check 추가를 고려해보세요:
🔎 Health check 추가 제안
discord-bot: container_name: discord-bot image: konkukkuit/kuitee-bot env_file: - .env environment: AWS_REGION: ap-northeast-2 LOG_LEVEL: info restart: unless-stopped + healthcheck: + test: ["CMD", "node", "-e", "process.exit(0)"] + interval: 30s + timeout: 10s + retries: 3 networks: - monitoringsrc/main/java/com/kuit/kupage/infra/config/SqsConfig.java (1)
13-14: SQS 리전 설정 일관성 개선
discord-bot/sqsProducer.js에서는 리전이"ap-northeast-2"로 하드코딩되어 있지만, Java의SqsConfig에서는 환경 변수(spring.cloud.aws.region.static)로부터 주입받고 있습니다. 두 컴포넌트의 리전을 일관되게 관리하기 위해 Discord bot도 환경 변수 기반으로 변경하는 것을 권장합니다. 기존의SQS_QUEUE_URL,LOG_LEVEL같은 환경 변수와 마찬가지로AWS_REGION환경 변수를 추가하여 런타임에 설정할 수 있도록 하면, 여러 환경(개발, 스테이징, 프로덕션 등)에서 더 유연하게 대응할 수 있습니다.src/main/java/com/kuit/kupage/domain/role/service/RoleService.java (1)
199-200: 로그 메시지 개선 권장최종 로그에서
addedRoleIds.size()와removedRoleIds.size()를 출력하지만, 실제로 추가/삭제된 수(toInsert.size(),toDelete.size())와 다를 수 있습니다. 디버깅 시 혼란을 줄 수 있으니 실제 처리된 수를 로깅하는 것이 좋습니다.🔎 수정 제안
- log.info("[역할변경반영] 처리 완료. eventId={}, discordUserId={}, 추가={}, 제거={}", - event.eventId(), memberDiscordId, addedRoleIds.size(), removedRoleIds.size()); + log.info("[역할변경반영] 처리 완료. eventId={}, discordUserId={}, 요청추가={}, 실제추가={}, 요청제거={}, 실제제거={}", + event.eventId(), memberDiscordId, addedRoleIds.size(), toInsert.size(), removedRoleIds.size(), toDelete.size());discord-bot/sqsProducer.js (1)
20-20: 리전 설정 환경변수화 고려리전이
"ap-northeast-2"로 하드코딩되어 있습니다. 현재 설정과 일치하지만, 향후 유연성을 위해 환경변수로 분리하는 것도 고려해볼 수 있습니다.🔎 수정 제안 (선택사항)
-const sqs = new SQSClient({ region: "ap-northeast-2" }); +const sqs = new SQSClient({ region: process.env.AWS_REGION || "ap-northeast-2" });scripts/deploy.sh (1)
51-60: Health check 무한 루프에 타임아웃/재시도 제한이 없습니다.컨테이너가 정상적으로 시작되지 않을 경우 배포 스크립트가 무한히 대기하게 됩니다. 최대 재시도 횟수와
curl타임아웃을 추가하는 것을 권장합니다.🔎 제안하는 수정 (green health check)
+ MAX_RETRIES=30 + RETRY_COUNT=0 - while [ 1 = 1 ]; do + while [ $RETRY_COUNT -lt $MAX_RETRIES ]; do echo "3. green health check..." sleep 3 + RETRY_COUNT=$((RETRY_COUNT + 1)) - REQUEST=$(curl http://localhost:8081) # green으로 request + REQUEST=$(curl -s -m 5 http://localhost:8081) # green으로 request if [ -n "$REQUEST" ]; then # 서비스 가능하면 health check 중지 echo "health check success" break ; fi done; + + if [ $RETRY_COUNT -ge $MAX_RETRIES ]; then + echo "health check failed after $MAX_RETRIES attempts" + exit 1 + fisrc/main/java/com/kuit/kupage/infra/handler/RoleChangeListener.java (1)
55-56: 로그에 메시지 본문 전체를 기록하면 민감 정보가 노출될 수 있습니다.
body에discordUserId,discordLoginId등이 포함되어 있어 로그에 PII가 남을 수 있습니다. 디버깅 목적이라면eventId만 기록하거나 민감 필드를 마스킹하는 것을 고려해 주세요.src/main/java/com/kuit/kupage/infra/dto/DiscordRoleChangeEvent.java (1)
28-31:throws Exception은 너무 광범위합니다.보다 구체적인 예외 타입(
JsonProcessingException)을 선언하면 호출부에서 예외 처리가 명확해집니다.🔎 제안하는 수정
- public static DiscordRoleChangeEvent fromJson(String body, ObjectMapper objectMapper) throws Exception { + public static DiscordRoleChangeEvent fromJson(String body, ObjectMapper objectMapper) throws JsonProcessingException { JsonNode root = objectMapper.readTree(body); return fromJson(root); }
JsonProcessingExceptionimport가 필요합니다:import com.fasterxml.jackson.core.JsonProcessingException;src/main/java/com/kuit/kupage/common/config/S3Config.java (2)
26-32:S3Client도 마찬가지로destroyMethod를 명시하는 것이 좋습니다.
S3Client는SdkAutoCloseable을 구현하므로, 종료 시 리소스를 정리하려면destroyMethod = "close"를 추가해야 합니다.🔎 제안하는 수정
- @Bean + @Bean(destroyMethod = "close") @ConditionalOnMissingBean public S3Client s3Client() {
38-44:S3Presigner는AutoCloseable이므로 애플리케이션 종료 시 리소스 해제가 필요합니다.
S3Presigner는 내부적으로 HTTP 클라이언트를 관리하므로, 종료 시close()를 호출해야 합니다.@Bean(destroyMethod = "close")를 추가하여 Spring이 자동으로 빈 소멸 시 리소스를 정리하도록 구성하세요.🔎 제안하는 수정
@Bean(destroyMethod = "close") @ConditionalOnMissingBean public S3Presigner s3Presigner() { return S3Presigner.builder() .region(Region.of(region)) .build(); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
discord-bot/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (28)
.github/workflows/Github-Action-CD.ymlbuild.gradlediscord-bot/.gitignorediscord-bot/Dockerfilediscord-bot/bot.jsdiscord-bot/index.jsdiscord-bot/package.jsondiscord-bot/sqsProducer.jsdocker-compose.ymlscripts/deploy.shsrc/main/java/com/kuit/kupage/common/config/S3Config.javasrc/main/java/com/kuit/kupage/common/config/SecurityConfig.javasrc/main/java/com/kuit/kupage/common/file/PresignedUrlController.javasrc/main/java/com/kuit/kupage/common/file/PresignedUrlService.javasrc/main/java/com/kuit/kupage/common/file/S3Service.javasrc/main/java/com/kuit/kupage/common/response/ResponseCode.javasrc/main/java/com/kuit/kupage/domain/role/service/RoleService.javasrc/main/java/com/kuit/kupage/infra/config/SqsConfig.javasrc/main/java/com/kuit/kupage/infra/domain/ProcessedEventLog.javasrc/main/java/com/kuit/kupage/infra/dto/DiscordRoleChangeEvent.javasrc/main/java/com/kuit/kupage/infra/handler/RoleChangeEventService.javasrc/main/java/com/kuit/kupage/infra/handler/RoleChangeListener.javasrc/main/java/com/kuit/kupage/infra/repository/ProcessedEventRepository.javasrc/main/resources/application.ymlsrc/main/resources/db/migration/V2__create_processed_events.sqlsrc/test/java/com/kuit/kupage/global/db/FlywayMigrationTest.javasrc/test/java/com/kuit/kupage/unit/common/S3ServiceTest.javasrc/test/resources/application-test.yml
💤 Files with no reviewable changes (1)
- src/test/java/com/kuit/kupage/global/db/FlywayMigrationTest.java
🧰 Additional context used
🧬 Code graph analysis (4)
discord-bot/bot.js (1)
discord-bot/sqsProducer.js (2)
log(15-15)publishRoleChangeEvent(41-74)
src/main/java/com/kuit/kupage/infra/handler/RoleChangeListener.java (1)
src/main/java/com/kuit/kupage/infra/handler/RoleChangeEventService.java (1)
Slf4j(19-57)
discord-bot/sqsProducer.js (1)
discord-bot/bot.js (2)
log(19-19)payload(81-92)
src/main/java/com/kuit/kupage/infra/config/SqsConfig.java (1)
discord-bot/sqsProducer.js (1)
sqs(20-20)
🪛 Biome (2.1.2)
discord-bot/bot.js
[error] 17-19: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 19-19: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 19-21: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 21-22: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 106-106: await is only allowed within async functions and at the top levels of modules.
(parse)
discord-bot/sqsProducer.js
[error] 18-19: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 19-20: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 53-74: Illegal use of an export declaration outside of a module
not allowed inside scripts
(parse)
discord-bot/index.js
[error] 1-1: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
🪛 Shellcheck (0.11.0)
scripts/deploy.sh
[warning] 3-3: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (27)
src/main/java/com/kuit/kupage/common/config/SecurityConfig.java (1)
87-87: PR 목적과의 연관성을 확인해주세요.이 PR의 주요 목적은 AWS SQS 기반 Discord 역할 동기화 구현인데,
localhost:9000을 CORS 허용 origin에 추가한 이유가 명확하지 않습니다.
- Discord Bot 로컬 테스트용인가요?
- 다른 로컬 서비스(예: 모니터링, 어드민 페이지)용인가요?
- 이 변경사항이 이번 PR의 범위에 포함되어야 하는지 확인 부탁드립니다.
만약 개발 환경 전용이라면, 프로파일 기반으로 분리하는 것도 고려해볼 수 있습니다.
discord-bot/.gitignore (1)
1-1: LGTM!Node.js 프로젝트의 표준 gitignore 패턴이 올바르게 적용되었습니다.
src/main/java/com/kuit/kupage/common/file/PresignedUrlController.java (1)
27-27: LGTM!코드 가독성을 위한 포맷팅 변경이 적절하게 적용되었습니다.
discord-bot/package.json (2)
1-19: 패키지 구성이 적절합니다.Discord bot 프로젝트의 기본 구성이 올바르게 설정되었습니다. 다만, 향후 테스트 스크립트 구현과 의존성 버전 관리에 유의해주세요.
13-18: 현재 사용 중인 NPM 패키지 버전들은 모두 최신 버전입니다.검증 결과:
@aws-sdk/client-sqs@^3.960.0,discord.js@^14.25.1,pino@^10.1.0,uuid@^13.0.0모두 현재 최신 버전이며, 각 패키지에 대해 보고된 직접적인 보안 취약점은 없습니다.다만, NPM 생태계에서 공급망 공격(타이포스쿼팅, 악의적 패키지)이 보고되고 있으므로,
npm audit또는 Snyk/GitHub Dependabot 같은 의존성 스캐너를 정기적으로 실행하여 패키지 진위를 확인하고 새로운 취약점을 모니터링하시기 바랍니다.build.gradle (1)
32-32: Spring Cloud 2024.0.3 버전은 최신 버전입니다.Spring Cloud 2024.0.3은 2024.0.x 라인의 최신 릴리스(2025년 12월 12일)이며, 더 이상의 버전 업데이트는 필요하지 않습니다. Spring Cloud 생태계에 공개된 CVE(CVE-2024-37084, CVE-2024-22263, CVE-2024-22271, CVE-2024-22236)가 있으나, 이들은 모두 Data Flow, Function Web, Contract 등 이 프로젝트에서 사용하지 않는 서브프로젝트에만 영향을 미칩니다. 현재 사용 중인 spring-cloud-starter-openfeign는 식별된 보안 이슈의 영향을 받지 않습니다.
src/main/resources/db/migration/V2__create_processed_events.sql (1)
1-12: LGTM! 멱등 처리를 위한 테이블 구조가 적절합니다.
event_id에 unique 제약조건이 설정되어 있어 중복 이벤트 처리 방지에 효과적입니다. 파일 끝에 개행 문자를 추가하는 것을 권장합니다.src/main/java/com/kuit/kupage/common/response/ResponseCode.java (1)
102-103: LGTM! AWS 인프라 오류 코드 추가가 적절합니다.7000번대로 AWS(S3, SQS) 관련 오류를 분리하여 관리하는 것이 좋습니다. 기존 패턴과 일관성 있게 추가되었습니다.
src/test/resources/application-test.yml (1)
45-56: LGTM! SQS 리스너 설정이 적절합니다.Long Polling을 위한
poll-timeout: 20s설정과message-visibility: 60s가 SQS 모범 사례에 부합합니다.src/main/java/com/kuit/kupage/infra/config/SqsConfig.java (1)
16-22: LGTM! SqsClient 빈 구성이 적절합니다.
@ConditionalOnMissingBean사용으로 테스트 시 목(mock) 주입이 용이합니다. AWS SDK v2의 기본 자격 증명 체인(DefaultCredentialsProvider)을 사용하여 EC2 IAM 역할, 환경 변수 등에서 자동으로 자격 증명을 가져옵니다.src/main/java/com/kuit/kupage/infra/repository/ProcessedEventRepository.java (1)
6-7: 현재 구현이 적절하며 추가 쿼리 메서드는 불필요함멱등 처리는 이미 최적화되어 있습니다.
ProcessedEventLog엔티티의eventId칼럼에 유니크 제약조건이 있고,tryMarkProcessed()메서드에서DataIntegrityViolationException을 캐치하여 중복 이벤트를 감지합니다. 이 방식은 데이터베이스 수준의 원자성을 보장하며,existsByEventId()추가 조회보다 효율적입니다(TOCTOU 문제 회피, 불필요한 쿼리 제거).src/main/java/com/kuit/kupage/infra/handler/RoleChangeEventService.java (1)
27-56: LGTM! 멱등 처리 패턴이 올바르게 구현되었습니다.
@Transactional을 사용하여ProcessedEventLog저장과 역할 변경 반영을 하나의 트랜잭션으로 묶었습니다.roleService.applyDiscordRoleChangeEvent()에서 예외가 발생하면 트랜잭션이 롤백되어 이벤트가 재시도될 수 있으며, 이는 EDA 파이프라인에서 올바른 동작입니다.src/main/resources/application.yml (1)
42-57: LGTM! SQS Long Polling 설정이 적절합니다.
poll-timeout: 20s: SQS Long Polling 최대값으로 적절합니다.max-messages-per-poll: 10: SQS 최대 수신 개수입니다.message-visibility: 60s: 처리 시간을 고려한 적절한 설정입니다.참고로
spring.cloud.aws(Lines 43-53)와 최상위cloud.aws(Lines 64-69) 두 개의 블록이 있는데, Spring Cloud AWS와 커스텀 설정을 분리한 것으로 보입니다.discord-bot/bot.js (1)
28-43: LGTM! 역할 diff 로직이 깔끔합니다.Set을 활용한 O(1) 조회와
@everyone역할 제외 처리가 올바르게 구현되었습니다.discord-bot/sqsProducer.js (1)
41-73: LGTM! 재시도 로직이 잘 구현되었습니다.지수 백오프와 jitter를 적용한 재시도 정책이 올바르게 구현되었습니다.
maxAttempts = 5와 최대 3초 대기 시간은 일시적인 네트워크 오류를 처리하기에 적절합니다..github/workflows/Github-Action-CD.yml (2)
14-20: LGTM! 경로 기반 조건부 빌드 설정이 좋습니다.
dorny/paths-filter를 사용하여discord-bot/**경로 변경 시에만 Discord Bot 이미지를 빌드/푸시하도록 설정했습니다. 불필요한 빌드를 방지하는 효율적인 접근입니다.
124-126: SQS 환경변수 추가 확인
SQS_QUEUE_URL과SQS_QUEUE_NAME시크릿이 GitHub Repository Settings에 설정되어 있는지 확인해주세요.src/main/java/com/kuit/kupage/common/file/S3Service.java (1)
43-62: LGTM! AWS SDK v2로의 마이그레이션이 깔끔합니다.
try-with-resources를 사용하여 InputStream 리소스 관리가 개선되었습니다.- SDK v2의 빌더 패턴을 올바르게 활용했습니다.
- 기존
PUBLIC_READACL과 에러 핸들링이 유지되었습니다.scripts/deploy.sh (1)
10-27: Discord Bot 이미지 변경 감지 로직이 잘 구현되었습니다.이미지가 실제로 변경된 경우에만 컨테이너를 재생성하여 Gateway 연결의 안정성을 유지하는 전략이 적절합니다.
src/test/java/com/kuit/kupage/unit/common/S3ServiceTest.java (2)
31-40: AWS SDK v2로의 테스트 마이그레이션이 적절합니다.
S3Client모킹과ReflectionTestUtils를 활용한 필드 주입으로 테스트 격리가 잘 구현되었습니다.
59-76: 실패 케이스 테스트가 적절히 구현되었습니다.
RuntimeException을 던지도록 모킹하여KupageException으로 적절히 변환되는지 검증하고 있습니다.src/main/java/com/kuit/kupage/infra/handler/RoleChangeListener.java (1)
30-62: 에러 처리 전략이 적절합니다.파싱/검증 실패는 재시도 없이 폐기하고, 일시적 장애는 재시도하도록 분기 처리가 잘 되어있습니다. 멱등 처리를 위한
eventId검증도 적절합니다.src/main/java/com/kuit/kupage/infra/dto/DiscordRoleChangeEvent.java (2)
62-76: 다양한 날짜 형식 지원이 잘 구현되었습니다.
LocalDateTime과OffsetDateTime형식을 모두 처리하여 Discord Bot에서 보내는 다양한 ISO-8601 형식에 유연하게 대응할 수 있습니다.
96-107: 리스트 추출 헬퍼가 null-safe하게 잘 구현되었습니다.빈 배열, null, 공백 문자열을 모두 적절히 처리하고 있습니다.
src/main/java/com/kuit/kupage/infra/domain/ProcessedEventLog.java (2)
12-19: 멱등 처리를 위한 엔티티 설계가 적절합니다.
event_id에 unique constraint를 적용하여DataIntegrityViolationException을 통한 중복 감지 방식이 잘 구현되었습니다.
36-44: 팩토리 메서드가 깔끔하게 구현되었습니다.
DiscordRoleChangeEvent에서ProcessedEventLog로의 변환이 명확합니다.eventId는 DB 레벨에서nullable=false로 보호되고,RoleChangeListener에서 이미 blank 검증을 수행하므로 추가 검증은 불필요합니다.src/main/java/com/kuit/kupage/common/file/PresignedUrlService.java (1)
36-45: 헬퍼 메서드가 깔끔하게 구현되었습니다.빌더 패턴을 활용하여 선택적
contentType처리가 명확합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/kuit/kupage/domain/role/service/RoleService.java (1)
186-197: 역할 제거 시 예외 처리 추가를 고려하세요.역할 추가 로직(lines 175-183)에는
DataIntegrityViolationException처리가 있지만, 제거 로직에는 예외 처리가 없습니다. 일관성과 안정성을 위해 제거 시에도 예외 처리를 추가하는 것을 권장합니다.🔎 제안하는 수정안
if (!toDelete.isEmpty()) { - memberRoleRepository.deleteAll(toDelete); - log.info("[역할변경반영] 역할 {}개 제거 완료. eventId={}, discordUserId={}", - toDelete.size(), event.eventId(), memberDiscordId); + try { + memberRoleRepository.deleteAll(toDelete); + log.info("[역할변경반영] 역할 {}개 제거 완료. eventId={}, discordUserId={}", + toDelete.size(), event.eventId(), memberDiscordId); + } catch (Exception e) { + log.warn("[역할변경반영] 역할 제거 중 예외가 발생했습니다. eventId={}, discordUserId={}, message={}", + event.eventId(), memberDiscordId, e.getMessage()); + } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
discord-bot/bot.jsdiscord-bot/index.jsdiscord-bot/package.jsonsrc/main/java/com/kuit/kupage/domain/role/service/RoleService.java
🚧 Files skipped from review as they are similar to previous changes (3)
- discord-bot/index.js
- discord-bot/package.json
- discord-bot/bot.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
src/main/java/com/kuit/kupage/domain/role/service/RoleService.java (5)
124-129: 입력 검증이 적절합니다.discordUserId에 대한 null/blank 체크와 early return 패턴이 올바르게 구현되어 있습니다.
131-137: 빈 리스트 처리가 적절합니다.null 체크 및 빈 리스트에 대한 early return이 올바르게 구현되어 있습니다.
139-146: 역할 조회 로직이 효율적입니다.이벤트에 포함된 역할만 조회하여 Map으로 구성하는 접근이 적절합니다.
158-184: 역할 추가 로직이 올바르게 구현되었습니다.중복 체크, 예외 처리, 로깅이 적절하게 구현되어 있으며,
DataIntegrityViolationException처리로 동시성 상황도 고려되었습니다.
199-201: 처리 완료 로깅이 적절합니다.이벤트 처리 결과에 대한 상세한 로그가 잘 작성되어 있어 디버깅과 모니터링에 유용합니다.
| List<MemberRole> existingForMember = memberRoleRepository.findByMemberDiscordId(memberDiscordId).stream() | ||
| .filter(mr -> memberDiscordId.equals(mr.getMemberDiscordId())) | ||
| .toList(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
불필요한 중복 필터링을 제거하세요.
findByMemberDiscordId(memberDiscordId)가 이미 해당 멤버의 역할만 반환하므로, Line 150의 추가 필터링은 불필요합니다.
🔎 수정 제안
- List<MemberRole> existingForMember = memberRoleRepository.findByMemberDiscordId(memberDiscordId).stream()
- .filter(mr -> memberDiscordId.equals(mr.getMemberDiscordId()))
- .toList();
+ List<MemberRole> existingForMember = memberRoleRepository.findByMemberDiscordId(memberDiscordId);📝 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.
| List<MemberRole> existingForMember = memberRoleRepository.findByMemberDiscordId(memberDiscordId).stream() | |
| .filter(mr -> memberDiscordId.equals(mr.getMemberDiscordId())) | |
| .toList(); | |
| List<MemberRole> existingForMember = memberRoleRepository.findByMemberDiscordId(memberDiscordId); |
🤖 Prompt for AI Agents
In src/main/java/com/kuit/kupage/domain/role/service/RoleService.java around
lines 149 to 151, remove the redundant stream filter because
findByMemberDiscordId(memberDiscordId) already returns only that member's roles;
replace the current chained .stream().filter(...).toList() usage with the direct
repository result (e.g., assign the List returned by findByMemberDiscordId) so
no extra filtering is performed and types remain consistent.
이슈 번호
Close #58
개요
작업사항
쿼리 발생 내역
총합
Summary by CodeRabbit
릴리스 노트
새 기능
배포 개선
업그레이드
✏️ Tip: You can customize this high-level summary in your review settings.