Feat: Migrate AI grading pipeline to Kafka async messaging & optimize Redis config - #57
Conversation
… Redis config - Replace local Spring event with AiGradingProducer message publishing in RoomServiceImpl - Convert AiGradingEventListener to @KafkaListener async consumer to eliminate CPU bottleneck and blocking - Add KafkaTopics.AI_GRADING_EVENTS constant and AiGradingEventDto - Explicitly declare LettuceConnectionFactory bean in RedisConfig to resolve Redisson StackOverflowError - Add default environment placeholders in application.yaml and S3StorageService for local setup
|
Warning Review limit reached
Next review available in: 49 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAI 채점 시작 경로를 Spring 이벤트에서 Kafka 발행·구독 방식으로 변경했습니다. 이벤트 DTO에 ChangesAI 채점 Kafka 이벤트 흐름
런타임 저장소 처리
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant RoomServiceImpl
participant AiGradingProducer
participant Kafka
participant AiGradingEventListener
participant Gemini
RoomServiceImpl->>AiGradingProducer: sendAiGradingEvent 호출
AiGradingProducer->>Kafka: AiGradingEventDto 발행
Kafka->>AiGradingEventListener: AiGradingEventDto 전달
AiGradingEventListener->>Gemini: 채점 요청
Gemini-->>AiGradingEventListener: 채점 응답 반환
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@momogo-ai/src/main/java/com/momogo/ai/grading/listener/AiGradingEventListener.java`:
- Around line 45-47: Update handleStartAiGradingEvent so exceptions from Gemini
processing or saveGradingResults are not swallowed; allow them to propagate to
the Kafka listener infrastructure. Preserve successful handling, while ensuring
failed messages remain uncommitted and follow the configured
RetryTopicConfiguration retry/DLT flow.
In `@momogo-core/src/main/java/com/momogo/core/common/config/RedisConfig.java`:
- Around line 29-37: Remove the manual redisConnectionFactory() bean, or
otherwise leave the Redis configuration class empty, so Spring Boot’s
auto-configured RedisConnectionFactory remains active. If manual creation is
required for multiple connection modes, replace the host/port/password-only
mapping with RedisProperties-based handling that preserves URL, database,
username, sentinel, cluster, SSL, and timeout settings.
In
`@momogo-core/src/main/java/com/momogo/core/domain/room/dto/event/AiGradingEventDto.java`:
- Around line 6-18: Add an immutable eventId to AiGradingEventDto and update its
factory methods to require or generate it appropriately; in
momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.java:18-22,
generate a new eventId for every published event. In
momogo-ai/src/main/java/com/momogo/ai/grading/listener/AiGradingEventListener.java:41-45,
atomically claim the eventId through a DB inbox or processing record before
invoking Gemini, skip already-claimed events, and provide lease/retry recovery
for claims whose processing fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c633d7a-ea83-42f3-a745-93eb6b8a24f6
📒 Files selected for processing (8)
momogo-ai/src/main/java/com/momogo/ai/grading/listener/AiGradingEventListener.javamomogo-api/src/main/resources/application.yamlmomogo-core/src/main/java/com/momogo/core/common/config/KafkaTopics.javamomogo-core/src/main/java/com/momogo/core/common/config/RedisConfig.javamomogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.javamomogo-core/src/main/java/com/momogo/core/domain/room/dto/event/AiGradingEventDto.javamomogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.javamomogo-core/src/main/java/com/momogo/core/domain/room/service/RoomServiceImpl.java
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@momogo-ai/src/main/java/com/momogo/ai/grading/listener/AiGradingEventListener.java`:
- Around line 45-47: Update handleStartAiGradingEvent so exceptions from Gemini
processing or saveGradingResults are not swallowed; allow them to propagate to
the Kafka listener infrastructure. Preserve successful handling, while ensuring
failed messages remain uncommitted and follow the configured
RetryTopicConfiguration retry/DLT flow.
In `@momogo-core/src/main/java/com/momogo/core/common/config/RedisConfig.java`:
- Around line 29-37: Remove the manual redisConnectionFactory() bean, or
otherwise leave the Redis configuration class empty, so Spring Boot’s
auto-configured RedisConnectionFactory remains active. If manual creation is
required for multiple connection modes, replace the host/port/password-only
mapping with RedisProperties-based handling that preserves URL, database,
username, sentinel, cluster, SSL, and timeout settings.
In
`@momogo-core/src/main/java/com/momogo/core/domain/room/dto/event/AiGradingEventDto.java`:
- Around line 6-18: Add an immutable eventId to AiGradingEventDto and update its
factory methods to require or generate it appropriately; in
momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.java:18-22,
generate a new eventId for every published event. In
momogo-ai/src/main/java/com/momogo/ai/grading/listener/AiGradingEventListener.java:41-45,
atomically claim the eventId through a DB inbox or processing record before
invoking Gemini, skip already-claimed events, and provide lease/retry recovery
for claims whose processing fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c633d7a-ea83-42f3-a745-93eb6b8a24f6
📒 Files selected for processing (8)
momogo-ai/src/main/java/com/momogo/ai/grading/listener/AiGradingEventListener.javamomogo-api/src/main/resources/application.yamlmomogo-core/src/main/java/com/momogo/core/common/config/KafkaTopics.javamomogo-core/src/main/java/com/momogo/core/common/config/RedisConfig.javamomogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.javamomogo-core/src/main/java/com/momogo/core/domain/room/dto/event/AiGradingEventDto.javamomogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.javamomogo-core/src/main/java/com/momogo/core/domain/room/service/RoomServiceImpl.java
🛑 Comments failed to post (1)
momogo-core/src/main/java/com/momogo/core/common/config/RedisConfig.java (1)
29-37: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail rg -n -C 4 \ 'spring\.data\.redis|RedisProperties|RedisConnectionDetails|RedisStandaloneConfiguration|RedisSentinelConfiguration|RedisClusterConfiguration|LettuceClientConfiguration|ssl\.enabled|ssl\.bundle|database|username|timeout|url' \ --glob '*.yml' \ --glob '*.yaml' \ --glob '*.properties' \ --glob '*.java' .Repository: MoMoGo-QuizPlatform/MoMoGo
Length of output: 20409
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail printf '--- RedisConfig.java ---\n' cat -n momogo-core/src/main/java/com/momogo/core/common/config/RedisConfig.java printf '\n--- All application config snippets around redis/settings ---\n' for f in $(git ls-files '*application*.yml' '*application*.yaml' '*application*.properties'); do echo "### $f" nl -ba "$f" | grep -i -E 'redis|spring\.data|timeout|ssl|cluster|sentinel|database|username|password|host:|port:' || true done printf '\n--- Build files relevant to Spring Boot/Redis ---\n' for f in $(git ls-files 'pom.xml' 'build.gradle' 'settings.gradle' 'gradle.properties'); do echo "### $f" nl -ba "$f" | grep -i -E 'spring boot|spring-boot|redis|data redis|java' || true done printf '\n--- Static check: RedisConfig fields and factory mapping ---\n' python3 - <<'PY' from pathlib import Path import re p=Path('momogo-core/src/main/java/com/momogo/core/common/config/RedisConfig.java') s=p.read_text() print('has_host_field=', bool(re.search(r'private\s+String\s+host;', s))) print('has_port_field=', bool(re.search(r'private\s+int\s+port;', s))) print('has_password_field=', bool(re.search(r'private\s+String\s+password;', s))) print('uses_default_database=', bool(re.search(r'new\s+RedisStandaloneConfiguration\s*\(\s*host\s*,\s*port\s*\)\s*;', s))) print('sets_pass=', bool(re.search(r'configuration\.setPassword\s*\(', s))) print('uses_sentinel=', bool(re.search(r'RedisSentinelConfiguration', s))) print('uses_cluster=', bool(re.search(r'RedisClusterConfiguration', s))) print('uses_connection_url=', bool(re.search(r'new\s+LettuceConnectionFactory\s*\(\s*[\'"`](redis|rediss):\s*', s))) print('uses_config_properties=', bool(re.search(r'`@Autowired`\s+private\s+.*RedisProperties|RedisProperties\s+\w+', s))) print('uses_client_configuration=', bool(re.search(r'LettuceClientConfiguration', s))) PYRepository: MoMoGo-QuizPlatform/MoMoGo
Length of output: 255
🏁 Script executed:
#!/usr/bin/env bash set -euo pipefail echo "--- RedisConfig.java ---" cat -n momogo-core/src/main/java/com/momogo/core/common/config/RedisConfig.java echo echo "--- Redis/config settings snippets in application config files ---" for f in $(git ls-files '*application*.yml' '*application*.yaml' '*application*.properties'); do echo "### $f" nl -ba "$f" | grep -i -E 'redis|spring\.data|timeout|ssl|cluster|sentinel|database|username|password|host:|port:' || true done echo echo "--- Build files relevant to Spring Boot/Redis ---" for f in $(git ls-files 'pom.xml' 'build.gradle' 'settings.gradle' 'gradle.properties'); do echo "### $f" nl -ba "$f" | grep -i -E 'spring boot|spring-boot|redis|data redis|java' || true done echo echo "--- Static check: RedisConfig fields and factory mapping ---" python3 - <<'PY' from pathlib import Path import re p=Path('momogo-core/src/main/java/com/momogo/core/common/config/RedisConfig.java') s=p.read_text() print('has_host_field=', bool(re.search(r'private\s+String\s+host;', s))) print('has_port_field=', bool(re.search(r'private\s+int\s+port;', s))) print('has_password_field=', bool(re.search(r'private\s+String\s+password;', s))) print('uses_default_database=', bool(re.search(r'new\s+RedisStandaloneConfiguration\s*\(\s*host\s*,\s*port\s*\)\s*;', s))) print('sets_pass=', bool(re.search(r'configuration\.setPassword\s*\(', s))) print('uses_sentinel=', bool(re.search(r'RedisSentinelConfiguration', s))) print('uses_cluster=', bool(re.search(r'RedisClusterConfiguration', s))) print('uses_connection_url=', bool(re.search(r'new\s+LettuceConnectionFactory\s*\(\s*[\'"`](redis|rediss):\s*', s))) print('uses_config_properties=', bool(re.search(r'`@Autowired`\s+private\s+.*RedisProperties|RedisProperties\s+\w+', s))) print('uses_client_configuration=', bool(re.search(r'LettuceClientConfiguration', s))) PYRepository: MoMoGo-QuizPlatform/MoMoGo
Length of output: 3758
🌐 Web query:
Spring Boot redis connection factory bean replaces auto configuration RedisConnectionFactory auto Configuration💡 Result:
When you define your own RedisConnectionFactory bean in a Spring Boot application, it automatically replaces the default auto-configured RedisConnectionFactory [1][2]. This behavior occurs because the Spring Boot auto-configuration for Redis uses the
@ConditionalOnMissingBeanannotation, which ensures that the auto-configured bean is only created if no other bean of that type exists in the application context [3][4][5]. Key points regarding this replacement: 1. Automatic Replacement: By declaring a method annotated with@Beanthat returns a RedisConnectionFactory, you signal to Spring Boot that you want to take control of the Redis connection configuration [1][2]. Spring Boot respects this and backs off from providing its own implementation [5]. 2. Impact on Dependent Beans: Several other auto-configured beans, such as RedisTemplate and StringRedisTemplate, depend on the presence of a single RedisConnectionFactory bean [3][4]. If you provide a custom factory, these templates will automatically use your custom bean instead of the one provided by auto-configuration [1][2]. 3. Managing Multiple Factories: If your application requires multiple Redis connections (e.g., primary and secondary caches), defining multiple RedisConnectionFactory beans will cause a NoUniqueBeanDefinitionException because Spring Boot’s auto-configured templates (like redisTemplate) expect a single, uniquely identifiable bean [6][7]. In such cases, you must mark one as@Primary, or create custom RedisTemplate instances that explicitly reference the specific connection factory beans you have defined [6][7]. 4. Advanced Customization: Instead of completely replacing the factory, Spring Boot offers alternative ways to customize the connection, such as registering beans of type LettuceClientConfigurationBuilderCustomizer or JedisClientConfigurationBuilderCustomizer, or by defining beans of type RedisStandaloneConfiguration, RedisSentinelConfiguration, or RedisClusterConfiguration [2]. These approaches allow you to keep the benefits of auto-configuration while modifying specific parameters [2]. If you find that you need to replicate the complex internal logic of Spring Boot's connection configuration (e.g., handling pool configuration, SSL, or sentinel setups) within your custom bean, note that the internal configuration classes like LettuceConnectionConfiguration are not part of the public API and cannot be easily reused [8][9]. In such scenarios, you may need to manually instantiate and configure the connection factory as demonstrated in your own code [8].Citations:
- 1: https://docs.spring.io/spring-boot/reference/data/nosql.html
- 2: https://docs.spring.io/spring-boot/3.3/reference/data/nosql.html
- 3: https://javadoc.io/static/org.springframework.boot/spring-boot-autoconfigure/2.4.7/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfiguration.html
- 4: https://github.com/spring-projects/spring-boot/blob/3.4.x/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfiguration.java
- 5: https://github.com/spring-projects/spring-boot/blob/8f4bf233b4895a4fade5aff41e0a309f90ba3193/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/JedisConnectionConfiguration.java
- 6: spring-projects/spring-boot#9363
- 7: https://stackoverflow.com/questions/53140017/confusion-about-no-qualifying-bean-of-type-redisconnectionfactory
- 8: spring-projects/spring-data-redis#2502
- 9: spring-projects/spring-boot#39160
RedisConnectionFactory 수동 생성을 자제하세요.
redisConnectionFactory()가 있으면 Spring Boot Redis 자동 설정의 기본RedisConnectionFactory가 대체됩니다. 이 빈은host,port,password와RedisStandaloneConfiguration의 기본 DB(0)만 사용하므로, 향후spring.data.redis.url,database,username,sentinel,cluster, SSL, timeout 설정이 적용되지 않을 수 있습니다.
- 현재 Redis 설정이 standalone/password 전용이라면 이 빈을 제거하거나,
LetusConfig를 비우도록 조정해 자동 설정을 유지하세요.- 여러 연결 모드를 사용해야 한다면
RedisProperties를 통해 명시적으로 모든 운영 설정을 매핑하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@momogo-core/src/main/java/com/momogo/core/common/config/RedisConfig.java` around lines 29 - 37, Remove the manual redisConnectionFactory() bean, or otherwise leave the Redis configuration class empty, so Spring Boot’s auto-configured RedisConnectionFactory remains active. If manual creation is required for multiple connection modes, replace the host/port/password-only mapping with RedisProperties-based handling that preserves URL, database, username, sentinel, cluster, SSL, and timeout settings.Source: Path instructions
…is config - Re-throw exceptions in AiGradingEventListener to activate global Kafka retry & DLT policy - Add immutable UUID eventId to AiGradingEventDto for message idempotency - Add whenComplete callback in AiGradingProducer for async delivery tracking - Bind Spring Boot RedisProperties in RedisConfig to support database/timeout properties
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java (4)
37-38: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftSecurity Misconfiguration (CWE-16)
Reachability: Internal
운영 환경에서 S3 기본값 폴백을 분리하세요.
@Value("${app.aws.s3-bucket:momogo-s3}")와@Value("${app.aws.region:ap-northeast-2}")는 모든 Spring profile에서 적용됩니다.app.storage.type=s3인데AWS_S3_BUCKET또는AWS_REGION이 누락되면momogo-s3/ap-northeast-2를 사용해 S3Client가 생성되고 파일 업로드/삭제가 예상 bucket이 아닌 버킷으로 진행될 수 있습니다. API, batch, realtime 설정에도 같은 기본값이 있어 기동 단계에서 드러나지 않습니다. 운영/배포 profile에서는 필수 속성으로 설정해 fail-fast를 보장하고, 기본값은 로컬 설정만 사용하도록 분리하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java` around lines 37 - 38, Update the S3StorageService constructor’s bucket and region configuration so production and deployment profiles require explicitly configured AWS_S3_BUCKET and AWS_REGION values, causing startup to fail when either is missing. Move the momogo-s3 and ap-northeast-2 fallback values into local-only configuration, and align the API, batch, and realtime profile settings so operational profiles no longer provide these defaults.
125-130: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win[경미]
maxRetries와 실제 재시도 횟수를 일치시키세요.
maxRetries=3이고attempt <= maxRetries이면 총 3회 시도하며 초기 요청 이후 재시도는 2회입니다. 현재 주석은 최대 3회 재시도로 설명하고 백오프도100 -> 200 -> 400으로 표시합니다.3회 재시도가 요구사항이면 총 시도를 4회로 변경하세요. 총 3회 시도가 의도라면 변수를
maxAttempts로 변경하고 주석과 테스트를 수정하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java` around lines 125 - 130, executeDeleteWithRetry의 maxRetries 의미와 반복 조건을 일치시키세요. 요구사항이 초기 요청 이후 3회 재시도라면 총 4회 시도되도록 반복 상한과 백오프 흐름을 수정하고, 그렇지 않다면 변수를 maxAttempts로 변경해 총 3회 시도임을 명확히 하며 관련 주석과 테스트도 갱신하세요.
87-92: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win[주요] S3 삭제 실패를 정상 완료로 숨기지 마세요.
잘못된 URL이면
key == null에서 반환합니다. 4xx 오류도 반환합니다. 마지막 transient 오류는 로그만 남기고 메서드가 정상 종료합니다. 따라서storageService.delete가 정상 반환하고,momogo-core/src/main/java/com/momogo/core/common/storage/listener/FileDeleteEventListener.java의 Line 47-57이 삭제 완료를 기록합니다.AccessDenied또는 네트워크 오류가 발생해도 객체가 남고 성공으로 기록될 수 있습니다.명시적으로 허용한 not-found만 idempotent 성공으로 처리하세요. 그 외에는 기존 공통
BusinessException을 발생시키거나 명시적인 실패 결과를 반환하세요.InterruptedException을 처리한 뒤에는 즉시 루프를 종료해야 합니다. 재시도가 필요하면 상위 listener의 예외 전파 정책도 함께 확인하세요.Also applies to: 140-165
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java` around lines 87 - 92, Update S3StorageService.delete and executeDeleteWithRetry so only an explicitly recognized not-found response is treated as idempotent success; invalid URLs, other 4xx responses, exhausted transient retries, and non-interruption failures must propagate the existing BusinessException or another explicit failure result. After handling InterruptedException, restore the interrupt status and terminate the retry loop immediately. Verify FileDeleteEventListener’s deletion-success path receives and propagates these failures instead of recording completion.
22-22: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win[경미]
parseS3Key()에서URI.getPath()를 두 번 디코드하지 마세요.
URI.getPath()는 Java 21에서 URI path percent-encoded octets를 이미 디코드합니다. 이후URLDecoder.decode()를 호출하면application/x-www-form-urlencoded규칙의+가 공백으로 변환됩니다.https://bucket.../dir/a+b.txt처럼 저장된 S3 key가 URL path%2B로 인코딩되면, 현재 로직은a+b.txt가 아니라a b.txt를 삭제 대상으로 바꿉니다.最简单的 해결책은
URI.create(fileUrl).getPath()만 사용하고, S3 object key를 직접 percent-encode하지 않고 저장한 상태로 유지하는 것입니다. raw path가 필요하면getRawPath()와 URI path 규격에 맞는 percent-decoding을 별도로 사용하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java` at line 22, Update parseS3Key() to use URI.create(fileUrl).getPath() directly and remove the URLDecoder import/call, avoiding double decoding and preserving literal '+' characters in S3 keys; retain the resulting key unchanged rather than applying additional percent-encoding.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.java`:
- Around line 22-29: Update startAiGrading and the AiGradingProducer send flow
so Kafka publication failures are propagated to the caller instead of only being
logged, allowing the transaction that marks grading in progress to roll back;
use synchronous send completion/error handling such as awaiting the send result,
while preserving the existing success and failure logging.
---
Outside diff comments:
In
`@momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.java`:
- Around line 37-38: Update the S3StorageService constructor’s bucket and region
configuration so production and deployment profiles require explicitly
configured AWS_S3_BUCKET and AWS_REGION values, causing startup to fail when
either is missing. Move the momogo-s3 and ap-northeast-2 fallback values into
local-only configuration, and align the API, batch, and realtime profile
settings so operational profiles no longer provide these defaults.
- Around line 125-130: executeDeleteWithRetry의 maxRetries 의미와 반복 조건을 일치시키세요.
요구사항이 초기 요청 이후 3회 재시도라면 총 4회 시도되도록 반복 상한과 백오프 흐름을 수정하고, 그렇지 않다면 변수를 maxAttempts로
변경해 총 3회 시도임을 명확히 하며 관련 주석과 테스트도 갱신하세요.
- Around line 87-92: Update S3StorageService.delete and executeDeleteWithRetry
so only an explicitly recognized not-found response is treated as idempotent
success; invalid URLs, other 4xx responses, exhausted transient retries, and
non-interruption failures must propagate the existing BusinessException or
another explicit failure result. After handling InterruptedException, restore
the interrupt status and terminate the retry loop immediately. Verify
FileDeleteEventListener’s deletion-success path receives and propagates these
failures instead of recording completion.
- Line 22: Update parseS3Key() to use URI.create(fileUrl).getPath() directly and
remove the URLDecoder import/call, avoiding double decoding and preserving
literal '+' characters in S3 keys; retain the resulting key unchanged rather
than applying additional percent-encoding.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9fb0bda2-0734-416c-91d0-8d0e6f09815d
📒 Files selected for processing (5)
momogo-ai/src/main/java/com/momogo/ai/grading/listener/AiGradingEventListener.javamomogo-core/src/main/java/com/momogo/core/common/config/RedisConfig.javamomogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.javamomogo-core/src/main/java/com/momogo/core/domain/room/dto/event/AiGradingEventDto.javamomogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.java
🚧 Files skipped from review as they are similar to previous changes (2)
- momogo-core/src/main/java/com/momogo/core/common/config/RedisConfig.java
- momogo-ai/src/main/java/com/momogo/ai/grading/listener/AiGradingEventListener.java
…is/S3 configs - Kafka 리스너 실패 시 예외 재전파를 통해 전역 지수 백오프 재시도 및 DLT(Dead Letter Topic) 격리 정책 활성화 - AiGradingEventDto에 불변 UUID eventId 필드를 추가하여 메시지 중복 재전달 시 멱등성(Idempotency) 보장 - AiGradingProducer의 메시지 전송 실패 시 비동기 콜백에서 DB 방 상태(isAiGradingInProgress=false)를 즉시 원복하여 상태 정체 방지 - RedisConfig에 Spring Boot RedisProperties를 주입받아 Redisson StackOverflowError 차단 및 database/timeout 운영 설정 호환 - S3StorageService의 자바 코드 레벨 하드코딩 기본값을 제거하여 운영 환경 Fail-Fast를 보장하고, 404 제외 삭제 실패 예외를 전파 - parseS3Key의 중복 URLDecoder를 제거하여 파일명 내 '+' 기호가 공백으로 오변환되는 문제 차단
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.java (1)
31-44: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMajor: ack 손실 시 중복 채점 트리거를 막으세요.
kafkaTemplate.send()실패 콜백에서saveGradingResults()가 완료됐는데도clearAiGradingStatus()를 호출할 수 있습니다. 이 경로는 같은 방 채점이 정상 실행됐을 때isAiGradingInProgress상태를 이미 해제하므로, 관리자가 같은roomId로 다시 채점을 시작할 수 있습니다. 성공 처리까지 이어지는 경로로는saveGradingResults()가 호출한 후 상태를 해제하고, 실패 콜백에서는saveGradingResults()를 호출하지 않도록 분리하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.java` around lines 31 - 44, Separate the Kafka completion flow around kafkaTemplate.send and saveGradingResults so the successful grading path saves results before clearing isAiGradingInProgress. Ensure the failure callback does not invoke saveGradingResults or clear a status that may already have been released by successful grading, preventing duplicate triggers for the same roomId.
♻️ Duplicate comments (1)
momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.java (1)
35-40: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMinor: 복구(clearAiGradingStatus) 자체가 실패하면 room 상태가 다시 정체될 수 있습니다.
catch블록은 복구 실패를 로그로만 남깁니다. Kafka 발행 실패와 복구 호출 실패가 함께 일어나면 room은isAiGradingInProgress = true로 영구히 남습니다. 이 경우는 원래 방지하려던 문제(영구 정체)가 다른 실패 경로로 재발하는 것입니다.간단한 개선으로, 복구 실패 시 메트릭이나 알람을 남기면 운영자가 수동으로 개입할 수 있습니다.
} catch (Exception clearEx) { log.error("[AiGradingProducer] AI 채점 상태 복구 실패 - roomId: {}", roomId, clearEx); + // 예: 알람/메트릭 전송으로 운영자가 수동 개입할 수 있게 함 + // alertService.notify("AI 채점 상태 복구 실패", roomId, clearEx); }발생 빈도는 낮지만(두 실패가 겹쳐야 함), room이 영구히 채점 불가 상태로 남는 사용자 영향은 작지 않습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.java` around lines 35 - 40, Update the clearAiGradingStatus failure handling in AiGradingProducer so it emits an operational metric or alert in addition to the existing error log, including the roomId and failure context, enabling manual intervention when recovery fails.
🧹 Nitpick comments (1)
momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.java (1)
16-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff설계 개선 제안:
@Lazy RoomService순환 의존성을 없애는 것을 검토하세요.
AiGradingProducer가RoomService를 주입받고,RoomServiceImpl.startAiGrading은 다시AiGradingProducer를 호출합니다. 두 컴포넌트가 서로를 참조하는 순환 의존성이 생겼습니다.@Lazy는 순환 의존성으로 인한 빈 생성 오류를 피하는 임시 조치이며, 근본 원인인 설계 문제를 해결하지 않습니다.순환 의존성이 있으면 다음 문제가 생깁니다.
- 두 클래스의 책임 경계가 불명확해집니다.
AiGradingProducer가 발행 책임과 room 상태 복구 책임을 함께 가지게 됩니다.- 향후
RoomService의 인터페이스가 커지면AiGradingProducer가 불필요하게 넓은 의존성을 갖게 됩니다.대안으로, room의 AI 채점 상태만 다루는 좁은 인터페이스(예:
AiGradingStatusUpdater)를 분리해 두 클래스가 이 인터페이스에만 의존하도록 하면 순환 의존성 없이 동일한 기능을 구현할 수 있습니다.public interface AiGradingStatusUpdater { void clearAiGradingStatus(UUID roomId); }
RoomServiceImpl이 이 인터페이스를 구현하고,AiGradingProducer는@Lazy없이 이 인터페이스만 주입받으면 순환 의존성이 사라집니다. 다만 이 리팩토링은 인터페이스 분리와 스프링 빈 재구성이 필요해 즉시 급한 작업은 아닙니다.As per path instructions,
**/main/**/*.javarequires reviewing 서비스/도메인 설계 and 클린코드/리팩토링 관점, 순환 의존성은 서비스 설계 관점에서 개선 대상입니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.java` around lines 16 - 25, AiGradingProducer의 `@Lazy` RoomService 의존성을 제거해 순환 의존성을 해소하세요. RoomServiceImpl이 구현하는 좁은 AiGradingStatusUpdater 인터페이스를 분리하고, 필요한 clearAiGradingStatus(UUID roomId) 동작만 노출하도록 변경한 뒤 AiGradingProducer가 이 인터페이스를 주입받게 하세요. 관련 Spring 빈 구성을 갱신해 기존 AI 채점 상태 복구 동작은 유지하세요.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.java`:
- Around line 31-44: Separate the Kafka completion flow around
kafkaTemplate.send and saveGradingResults so the successful grading path saves
results before clearing isAiGradingInProgress. Ensure the failure callback does
not invoke saveGradingResults or clear a status that may already have been
released by successful grading, preventing duplicate triggers for the same
roomId.
---
Duplicate comments:
In
`@momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.java`:
- Around line 35-40: Update the clearAiGradingStatus failure handling in
AiGradingProducer so it emits an operational metric or alert in addition to the
existing error log, including the roomId and failure context, enabling manual
intervention when recovery fails.
---
Nitpick comments:
In
`@momogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.java`:
- Around line 16-25: AiGradingProducer의 `@Lazy` RoomService 의존성을 제거해 순환 의존성을
해소하세요. RoomServiceImpl이 구현하는 좁은 AiGradingStatusUpdater 인터페이스를 분리하고, 필요한
clearAiGradingStatus(UUID roomId) 동작만 노출하도록 변경한 뒤 AiGradingProducer가 이 인터페이스를
주입받게 하세요. 관련 Spring 빈 구성을 갱신해 기존 AI 채점 상태 복구 동작은 유지하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 98629453-407f-43e3-9d2b-0369fb79241a
📒 Files selected for processing (2)
momogo-core/src/main/java/com/momogo/core/common/storage/S3StorageService.javamomogo-core/src/main/java/com/momogo/core/domain/room/kafka/producer/AiGradingProducer.java
idktomorrow
left a comment
There was a problem hiding this comment.
제가 꼼꼼히 체크해봤는데 크게 문제되는 부분은 없어 보입니다
작성하시느라 고생 많으셨습니다!! ❤️
| */ | ||
| @KafkaListener( | ||
| topics = KafkaTopics.AI_GRADING_EVENTS, | ||
| groupId = "momogo-ai-grading-group" |
There was a problem hiding this comment.
이 부분 하드코딩도 괜찮지만 application.yml 프로퍼티로 빼는 것이 추후 수정이나 확인하기 용이 할 것 같아서
고려해 보셔도 좋을 듯 합니다.
| // Kafka Producer 메시지 발행 | ||
| aiGradingProducer.sendAiGradingEvent(roomId, null); | ||
| } | ||
|
|
There was a problem hiding this comment.
이번 PR 범위는 아니지만 카프카 메시지 중복 관련 문제가 있을 까 해서 보다가
saveGradingResults가 UserRoomAnswer.grade()로 답안을 무조건 덮어쓰고 있는데
관리자가 manualGradeAnswer로 특정 답안을 이미 수동 채점한 뒤에 AI 채점 결과가 뒤늦게 딸려 들어오면 수동 채점 결과가 다시 AI 판정으로 덮어써질 수 있어 보입니다. UserRoomAnswer에 isManuallyGraded 같은 플래그를 두고, saveGradingResults에서 이미 수동 채점된 답안은 스킵하도록 하면 간단히 막을 수 있을 것 같은데, 이번 PR이 아니어도 후속 이슈로 고려하셔도 좋을 것 같습니다!
…mer group ID - UserRoomAnswer 엔티티에 isManuallyGraded 필드를 추가하고, saveGradingResults에서 수동 채점 완료된 답안은 AI 채점 결과 덮어쓰기를 스킵하도록 보완 - TBL_USER_ROOM_ANSWER 테이블 DDL(schema.sql)에 is_manually_graded 컬럼 추가 - AiGradingEventListener의 @KafkaListener groupId를 application.yaml의 app.kafka.ai-grading-group-id 프로퍼티로 분리하여 유연성 향상
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@db/schema.sql`:
- Around line 103-110: Update the database migration flow for
TBL_USER_ROOM_ANSWER, not just db/schema.sql, by adding a versioned migration
that adds is_manually_graded with the required default and NOT NULL constraint.
Match the repository’s existing deployment/migration convention, or add the
corresponding migration mechanism if none exists, so existing installations
receive the column safely.
In
`@momogo-core/src/main/java/com/momogo/core/domain/room/entity/UserRoomAnswer.java`:
- Around line 69-72: Update the UserRoomAnswer manual grading and AI result
persistence flow so manual grading is decided atomically rather than relying on
a stale isManuallyGraded read. Prefer an update guarded by isManuallyGraded =
false before saving AI isCorrect, and skip the AI save when no row is affected;
alternatively, apply the same pessimistic or optimistic lock to both paths.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d007bbd1-a9fd-4113-9047-61c929dc2ffb
📒 Files selected for processing (5)
db/schema.sqlmomogo-ai/src/main/java/com/momogo/ai/grading/listener/AiGradingEventListener.javamomogo-api/src/main/resources/application.yamlmomogo-core/src/main/java/com/momogo/core/domain/room/entity/UserRoomAnswer.javamomogo-core/src/main/java/com/momogo/core/domain/room/service/RoomServiceImpl.java
🚧 Files skipped from review as they are similar to previous changes (2)
- momogo-core/src/main/java/com/momogo/core/domain/room/service/RoomServiceImpl.java
- momogo-ai/src/main/java/com/momogo/ai/grading/listener/AiGradingEventListener.java
… DB update query - UserRoomAnswerRepository에 DB 레벨의 조건부 원자적 UPDATE 쿼리(updateIsCorrectIfNotManuallyGraded) 추가 - saveGradingResults에서 엔티티 메모리 덮어쓰기 대신 원자적 DB UPDATE를 실행하여 관리자 수동 채점과의 동시성 덮어쓰기 레이스 조건(Race Condition) 100% 차단
작업 내용
StackOverflowError및IllegalStateException을 근본적으로 해결했습니다.변경 사항
1. ⚡️ Kafka 기반 AI 주관식 자동 채점 파이프라인 전환
KafkaTopics.AI_GRADING_EVENTS("ai-grading-events") 상수 및AiGradingEventDto생성AiGradingProducer신설 및RoomServiceImpl에서 기존 로컬 스프링 이벤트 대신 Kafka 메시지를 비동기 발행하도록 수정AiGradingEventListener에@KafkaListener(topics = KafkaTopics.AI_GRADING_EVENTS, groupId = "momogo-ai-grading-group")를 적용하여 수험생 답안 채점을 백그라운드에서 분산 처리하도록 전환2. 🔌 Redis 커넥션 팩토리 최적화 (
RedisConfig)RedisConfig에LettuceConnectionFactory빈(redisConnectionFactory())을 명시적으로 직접 등록StackOverflowError) 및 버전에 따른IllegalStateException현상을 완전하게 차단3. 🛡️ 로컬 구동 프로퍼티 안전성 확보
application.yaml및S3StorageService에 기본 플레이스홀더(${AWS_S3_BUCKET:momogo-s3}) 설정 추가체크리스트
참고 사항
MomogoApiApplication구동 ➔ 채점 요청 발행 ➔@KafkaListener수신 ➔ Gemini 채점(3건) ➔ DB 반영 완료 로그Empirical 검증 완료관련 이슈
Summary by CodeRabbit