Conversation
chore: AI Swagger 접근 보호 설정
refactor: 가정통신문 분석을 AI 서버 client로 전환
…lure-policy refactor: AI 서버 장애 시 분석 실패 정책 반영
…-save feat: AI 가정통신문 전체 분석 응답 저장 연동
…nto feat/#62-language-change
📝 WalkthroughWalkthrough이 PR은 뉴스레터 분석 파이프라인을 OpenAI 직접 호출에서 외부 AI 서버로 전환하고, 사용자 언어 코드 관리를 도입하며, 분석 실패 추적 및 재시도 기능을 구현합니다. ChangesAI 서버 통합 및 뉴스레터 실패 처리 재구성
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 `@deploy/docker-compose.yml`:
- Around line 109-110: 현재 docker-compose에 backend가 ai의 service_healthy에 하드 의존하고
있어 AI 헬스체크 실패 시 backend 재기동이 차단됩니다; docker-compose의 backend 서비스 설정에서 ai에 대한
"condition: service_healthy"를 제거하거나 대체하여 컨테이너 시작 의존성을 분리하세요 (예: 제거하거나
"condition: service_started"로 바꿔 AI 헬스 상태에 따른 재시작 제어를 애플리케이션 레이어로 위임). 변경 대상
식별자: 서비스 이름 "backend", "ai"와 조건 키 "condition: service_healthy".
In
`@src/main/java/com/gachi/be/domain/newsletter/pipeline/AiNewsletterClient.java`:
- Line 59: The current AiNewsletterClient logging writes full request/response
bodies (e.g., requestBody and the response variables logged around lines 75-77),
which may leak sensitive OCR text; change the logging in AiNewsletterClient so
log.info/log.error no longer prints full payloads but instead logs a masked or
truncated summary: include payload length, HTTP status code, and a trace
identifier (generate or pass a traceId), and if needed mask content (e.g., show
first N chars + "...[masked]") rather than the full requestBody/responseBody;
update all places where requestBody and responseBody are logged to use this
summarized format and ensure any error logs include the traceId and status but
not raw body content.
In
`@src/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterAiAnalyzer.java`:
- Around line 63-65: The summary is not being translated before storage: update
the pipeline so that when analysisResponse.summary() is present you first
normalize it (using normalizeSummary or similar) and then call translateIfNeeded
on that raw summary (pass language, "summary", and newsletterId) falling back to
translatedText when AI summary is absent; ensure the code paths around
normalizeSummary(analysisResponse.summary(), translatedText, originalText) and
translateIfNeeded(...) mirror what you did for title so
NewsletterServiceImpl.getSummary() returns the translated summary for non-KO
users.
- Around line 194-200: The preview title is using the original item.title()
instead of the stored, translated Checklist content; update the
CalendarPreviewEvent creation in saveExtractedItems() to use the saved
checklist's content (e.g., savedItem.checklist().content() or the
Checklist#getContent accessor) instead of item.title(), still applying
trimToMax(..., CHECKLIST_TEXT_MAX_LENGTH) and .trim(), and keep the rest of the
constructor (extractedDate, true, checklistIdList(savedItem.checklist()))
unchanged so CalendarRegisterServiceImpl.getPreview()/register() shows the
translated KO content consistently.
In
`@src/main/java/com/gachi/be/domain/newsletter/repository/NewsletterRepository.java`:
- Around line 104-114: The cancelInProgressByUserId update currently only sets
status and language, leaving failure metadata empty; modify the JPQL update in
NewsletterRepository (method cancelInProgressByUserId) to also set
n.failureStage = :failureStage and n.failureReason = :failureReason, add
corresponding parameters to the method signature (e.g., `@Param`("failureStage")
String failureStage, `@Param`("failureReason") String failureReason), and ensure
callers pass appropriate failureStage/failureReason values so failed documents
retain their failure metadata.
In
`@src/main/java/com/gachi/be/domain/newsletter/service/impl/NewsletterServiceImpl.java`:
- Around line 232-243: The transaction currently deletes DB-derived data
(checklistRepository.deleteByNewsletterId and
calendarEventRepository.deleteByNewsletterIdAndUserId) but leaves the Redis
preview key intact; inject a CalendarPreviewRedisService (add private final
CalendarPreviewRedisService calendarPreviewRedisService) into
NewsletterServiceImpl and, inside the same transactional flow before calling
TransactionSynchronizationManager.registerSynchronization(...), call the
calendarPreviewRedisService method that clears the preview for the
newsletter/user (e.g., deleteByNewsletterIdAndUserId(newsletterId, userId) or
deletePreview(newsletterId, userId)) so Redis preview is removed within the
retry transaction and cannot be consumed by
CalendarRegisterServiceImpl.getPreview()/register() from a stale state.
In
`@src/main/java/com/gachi/be/domain/user/dto/request/ChangeNotificationRequest.java`:
- Around line 6-7: ChangeNotificationRequest's notificationEnabled is declared
as primitive boolean with `@NotNull`, which doesn't enforce presence because
primitives can't be null; change the type to the boxed Boolean (or
Optional<Boolean>) so `@NotNull` can validate missing JSON values, update the
record signature in ChangeNotificationRequest accordingly, and adjust any
callers/consumers (constructors, mappers, tests) that instantiate or read
notificationEnabled to handle the boxed type (and its potential null) or unwrap
Optional as needed.
In `@src/main/java/com/gachi/be/domain/user/entity/User.java`:
- Around line 116-121: In User.updateLanguage, besides the existing null/blank
check, enforce the allowed language-code invariant by normalizing languageCode
(trim + toUpperCase) then validating it against the allowed set { "KO", "US",
"ZH", "VI" } (or an equivalent static Set/enum defined on the User class); if
the normalized code is not in that set, throw an IllegalArgumentException with a
clear message. Ensure the method stores only the validated normalized value in
this.languageCode and consider extracting the allowed codes to a private static
constant (e.g., ALLOWED_LANGUAGE_CODES) for reuse.
In `@src/main/java/com/gachi/be/global/config/external/AiServerProperties.java`:
- Around line 12-14: AiServerProperties currently allows connectTimeoutSeconds
and readTimeoutSeconds to be set to 0 or negative which will cause runtime
failures; add input validation in the AiServerProperties binding so these fields
are validated at startup (e.g., annotate the class with `@Validated` and add
`@Min`(1) to the connectTimeoutSeconds and readTimeoutSeconds fields, or perform
checks in their setters/constructor and throw IllegalArgumentException when
value <= 0) to fail fast and provide a clear error message referencing the
offending property names.
In `@src/main/resources/db/migration/V12__user_add_language_code.sql`:
- Around line 1-2: Add a DB-level CHECK constraint for the newly added
users.language_code column in V12__user_add_language_code.sql so only the
application-supported language codes are allowed; modify the ALTER TABLE users
... ADD COLUMN IF NOT EXISTS language_code VARCHAR(10) NOT NULL DEFAULT 'KO' to
include a CHECK (language_code IN (...)) (or add a separate ALTER TABLE ... ADD
CONSTRAINT ... CHECK) using the exact list of allowed codes your app expects so
invalid codes cannot be inserted even if app-side validation is bypassed.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: af95cd3d-b03f-4e9f-ba84-35346816bd62
📒 Files selected for processing (46)
build.gradledeploy/.env.exampledeploy/docker-compose.ymldeploy/nginx/nginx.confdeploy/nginx/nginx.https.template.confdocs/error-code.mddocs/newsletter-ai-analyze-integration.mddocs/newsletter-ai-failure-policy.mdsrc/main/java/com/gachi/be/domain/auth/dto/request/SignupRequest.javasrc/main/java/com/gachi/be/domain/auth/service/AuthenticatedUserResolver.javasrc/main/java/com/gachi/be/domain/auth/service/impl/AuthServiceImpl.javasrc/main/java/com/gachi/be/domain/calendar/service/CalendarPreviewRedisService.javasrc/main/java/com/gachi/be/domain/newsletter/api/controller/NewsletterController.javasrc/main/java/com/gachi/be/domain/newsletter/dto/response/NewsletterStatusResponse.javasrc/main/java/com/gachi/be/domain/newsletter/entity/Newsletter.javasrc/main/java/com/gachi/be/domain/newsletter/pipeline/AiNewsletterClient.javasrc/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterAiAnalyzer.javasrc/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterPipelineService.javasrc/main/java/com/gachi/be/domain/newsletter/repository/NewsletterRepository.javasrc/main/java/com/gachi/be/domain/newsletter/service/NewsletterService.javasrc/main/java/com/gachi/be/domain/newsletter/service/impl/NewsletterServiceImpl.javasrc/main/java/com/gachi/be/domain/user/api/controller/UserController.javasrc/main/java/com/gachi/be/domain/user/dto/request/ChangeLanguageRequest.javasrc/main/java/com/gachi/be/domain/user/dto/request/ChangeNotificationRequest.javasrc/main/java/com/gachi/be/domain/user/dto/response/UserMeResponse.javasrc/main/java/com/gachi/be/domain/user/entity/User.javasrc/main/java/com/gachi/be/domain/user/entity/enums/UserStatus.javasrc/main/java/com/gachi/be/domain/user/repository/UserRepository.javasrc/main/java/com/gachi/be/global/code/ErrorCode.javasrc/main/java/com/gachi/be/global/code/SuccessCode.javasrc/main/java/com/gachi/be/global/config/external/AiServerProperties.javasrc/main/java/com/gachi/be/global/config/external/ExternalApiConfig.javasrc/main/java/com/gachi/be/global/config/external/OpenAiProperties.javasrc/main/java/com/gachi/be/global/exception/ExternalApiException.javasrc/main/java/com/gachi/be/global/security/JwtAuthenticationFilter.javasrc/main/resources/application.ymlsrc/main/resources/db/migration/V11__newsletter_failure_reason.sqlsrc/main/resources/db/migration/V12__user_add_language_code.sqlsrc/main/resources/db/migration/V13__user_add_notification_enabled.sqlsrc/test/java/com/gachi/be/domain/auth/api/controller/AuthControllerIntegrationTest.javasrc/test/java/com/gachi/be/domain/auth/api/controller/AuthRateLimitIntegrationTest.javasrc/test/java/com/gachi/be/domain/child/api/controller/ChildControllerIntegrationTest.javasrc/test/java/com/gachi/be/domain/newsletter/dto/response/NewsletterStatusResponseTest.javasrc/test/java/com/gachi/be/domain/newsletter/entity/NewsletterTest.javasrc/test/java/com/gachi/be/domain/newsletter/pipeline/AiNewsletterClientTest.javasrc/test/java/com/gachi/be/domain/newsletter/pipeline/NewsletterAiAnalyzerTest.java
💤 Files with no reviewable changes (2)
- build.gradle
- src/main/java/com/gachi/be/global/config/external/OpenAiProperties.java
Comment on lines
+109
to
+110
| ai: | ||
| condition: service_healthy |
There was a problem hiding this comment.
백엔드 기동이 AI 헬스체크에 하드 의존합니다.
Line 109-110에서 backend가 ai의 service_healthy를 필수로 기다리면, AI 장애 시 백엔드 재기동이 막혀 비-AI API까지 함께 중단될 수 있습니다. AI 실패는 앱 레이어에서 처리하고 컨테이너 기동 의존성은 분리하는 게 안전합니다.
수정 제안
depends_on:
db:
condition: service_healthy
redis:
condition: service_healthy
- ai:
- condition: service_healthy📝 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
| ai: | |
| condition: service_healthy | |
| depends_on: | |
| db: | |
| condition: service_healthy | |
| redis: | |
| condition: service_healthy |
🤖 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 `@deploy/docker-compose.yml` around lines 109 - 110, 현재 docker-compose에
backend가 ai의 service_healthy에 하드 의존하고 있어 AI 헬스체크 실패 시 backend 재기동이 차단됩니다;
docker-compose의 backend 서비스 설정에서 ai에 대한 "condition: service_healthy"를 제거하거나 대체하여
컨테이너 시작 의존성을 분리하세요 (예: 제거하거나 "condition: service_started"로 바꿔 AI 헬스 상태에 따른 재시작
제어를 애플리케이션 레이어로 위임). 변경 대상 식별자: 서비스 이름 "backend", "ai"와 조건 키 "condition:
service_healthy".
| LocalDate.now(DEFAULT_ZONE), | ||
| DEFAULT_ZONE.getId(), | ||
| toDateCandidateRequests(dateCandidates))); | ||
| log.info("[AiNewsletterClient] 요청 body: {}", requestBody); |
There was a problem hiding this comment.
요청/응답 본문 원문 로그는 민감정보 유출 위험이 큽니다.
현재 로그에 OCR 원문/번역문 및 실패 응답 본문이 그대로 남습니다. 본문 전체 대신 길이·상태코드·추적 식별자만 남기도록 축약/마스킹해 주세요.
🔧 제안 수정안
- log.info("[AiNewsletterClient] 요청 body: {}", requestBody);
+ log.debug(
+ "[AiNewsletterClient] 분석 요청 전송. language={}, dateCandidatesCount={}, originalTextLength={}, translatedTextLength={}",
+ language != null ? language : "KO",
+ dateCandidates != null ? dateCandidates.size() : 0,
+ originalText != null ? originalText.length() : 0,
+ translatedText != null ? translatedText.length() : 0);
@@
- log.error(
- "[AiNewsletterClient] AI 서버 분석 실패. status={}, body={}",
- response.statusCode(),
- response.body());
+ log.error(
+ "[AiNewsletterClient] AI 서버 분석 실패. status={}, responseBodyLength={}",
+ response.statusCode(),
+ response.body() != null ? response.body().length() : 0);Also applies to: 75-77
🤖 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
`@src/main/java/com/gachi/be/domain/newsletter/pipeline/AiNewsletterClient.java`
at line 59, The current AiNewsletterClient logging writes full request/response
bodies (e.g., requestBody and the response variables logged around lines 75-77),
which may leak sensitive OCR text; change the logging in AiNewsletterClient so
log.info/log.error no longer prints full payloads but instead logs a masked or
truncated summary: include payload length, HTTP status code, and a trace
identifier (generate or pass a traceId), and if needed mask content (e.g., show
first N chars + "...[masked]") rather than the full requestBody/responseBody;
update all places where requestBody and responseBody are logged to use this
summarized format and ensure any error logs include the traceId and status but
not raw body content.
Comment on lines
+63
to
+65
| String rawTitle = normalizeTitle(analysisResponse.title(), originalText); | ||
| String title = translateIfNeeded(rawTitle, language, "title", newsletterId); | ||
| String summary = normalizeSummary(analysisResponse.summary(), translatedText, originalText); |
There was a problem hiding this comment.
요약문 번역 경로가 빠져 있습니다.
여기서는 title만 translateIfNeeded(...)를 타고, summary는 그대로 저장됩니다. NewsletterServiceImpl.getSummary()는 저장된 요약을 그대로 내려주므로 비KO 사용자는 제목/체크리스트는 번역되고 요약만 한국어로 남게 됩니다. AI 응답의 summary가 있을 때만 별도로 번역하고, 폴백 경로(translatedText)는 그대로 쓰는 쪽이 안전합니다.
가능한 수정 예시
String rawTitle = normalizeTitle(analysisResponse.title(), originalText);
String title = translateIfNeeded(rawTitle, language, "title", newsletterId);
- String summary = normalizeSummary(analysisResponse.summary(), translatedText, originalText);
+ String summary =
+ analysisResponse.summary() != null && !analysisResponse.summary().isBlank()
+ ? translateIfNeeded(
+ trimToMax(compact(analysisResponse.summary()), SUMMARY_MAX_LENGTH),
+ language,
+ "summary",
+ newsletterId)
+ : buildBaselineSummary(firstNonBlank(translatedText, originalText));🤖 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
`@src/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterAiAnalyzer.java`
around lines 63 - 65, The summary is not being translated before storage: update
the pipeline so that when analysisResponse.summary() is present you first
normalize it (using normalizeSummary or similar) and then call translateIfNeeded
on that raw summary (pass language, "summary", and newsletterId) falling back to
translatedText when AI summary is absent; ensure the code paths around
normalizeSummary(analysisResponse.summary(), translatedText, originalText) and
translateIfNeeded(...) mirror what you did for title so
NewsletterServiceImpl.getSummary() returns the translated summary for non-KO
users.
Comment on lines
+194
to
+200
| previewEvents.add( | ||
| new CalendarPreviewEvent( | ||
| "ai_evt_" + (previewEvents.size() + 1), | ||
| trimToMax(item.title().trim(), CHECKLIST_TEXT_MAX_LENGTH), | ||
| extractedDate, | ||
| true, | ||
| checklistIdList(savedItem.checklist()))); |
There was a problem hiding this comment.
캘린더 preview 제목이 번역 결과를 버리고 있습니다.
saveExtractedItems()에서 Checklist.content는 사용자 언어로 번역해 저장했는데, 여기서는 다시 item.title() 원문을 Redis preview에 넣고 있습니다. CalendarRegisterServiceImpl.getPreview()/register()는 이 preview를 그대로 UI와 일정 등록에 사용하므로 비KO 사용자는 preview만 한국어로 보게 됩니다. preview 제목은 이미 저장한 Checklist.content를 재사용하는 편이 일관됩니다.
가능한 수정 예시
previewEvents.add(
new CalendarPreviewEvent(
"ai_evt_" + (previewEvents.size() + 1),
- trimToMax(item.title().trim(), CHECKLIST_TEXT_MAX_LENGTH),
+ savedItem.checklist().getContent(),
extractedDate,
true,
checklistIdList(savedItem.checklist())));🤖 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
`@src/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterAiAnalyzer.java`
around lines 194 - 200, The preview title is using the original item.title()
instead of the stored, translated Checklist content; update the
CalendarPreviewEvent creation in saveExtractedItems() to use the saved
checklist's content (e.g., savedItem.checklist().content() or the
Checklist#getContent accessor) instead of item.title(), still applying
trimToMax(..., CHECKLIST_TEXT_MAX_LENGTH) and .trim(), and keep the rest of the
constructor (extractedDate, true, checklistIdList(savedItem.checklist()))
unchanged so CalendarRegisterServiceImpl.getPreview()/register() shows the
translated KO content consistently.
Comment on lines
+104
to
+114
| UPDATE Newsletter n | ||
| SET n.status = :failedStatus, | ||
| n.language = :newLanguage | ||
| WHERE n.userId = :userId | ||
| AND n.status IN :targetStatuses | ||
| """) | ||
| int cancelInProgressByUserId( | ||
| @Param("userId") Long userId, | ||
| @Param("targetStatuses") List<NewsletterStatus> targetStatuses, | ||
| @Param("failedStatus") NewsletterStatus failedStatus, | ||
| @Param("newLanguage") String newLanguage); |
There was a problem hiding this comment.
언어 변경 취소 경로에서도 실패 메타데이터를 채워야 합니다
cancelInProgressByUserId는 상태를 FAILED로 바꾸지만 failureStage/failureReason을 비워둡니다. 이 경로로 실패한 문서는 상태 조회 시 실패 원인 추적 정보가 누락됩니다.
수정 예시
`@Query`(
"""
UPDATE Newsletter n
SET n.status = :failedStatus,
- n.language = :newLanguage
+ n.language = :newLanguage,
+ n.failureStage = :failureStage,
+ n.failureReason = :failureReason
WHERE n.userId = :userId
AND n.status IN :targetStatuses
""")
int cancelInProgressByUserId(
`@Param`("userId") Long userId,
`@Param`("targetStatuses") List<NewsletterStatus> targetStatuses,
`@Param`("failedStatus") NewsletterStatus failedStatus,
- `@Param`("newLanguage") String newLanguage);
+ `@Param`("newLanguage") String newLanguage,
+ `@Param`("failureStage") String failureStage,
+ `@Param`("failureReason") String failureReason);🤖 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
`@src/main/java/com/gachi/be/domain/newsletter/repository/NewsletterRepository.java`
around lines 104 - 114, The cancelInProgressByUserId update currently only sets
status and language, leaving failure metadata empty; modify the JPQL update in
NewsletterRepository (method cancelInProgressByUserId) to also set
n.failureStage = :failureStage and n.failureReason = :failureReason, add
corresponding parameters to the method signature (e.g., `@Param`("failureStage")
String failureStage, `@Param`("failureReason") String failureReason), and ensure
callers pass appropriate failureStage/failureReason values so failed documents
retain their failure metadata.
Comment on lines
+232
to
+243
| checklistRepository.deleteByNewsletterId(newsletterId); | ||
| calendarEventRepository.deleteByNewsletterIdAndUserId(newsletterId, userId); | ||
| Newsletter saved = findNewsletterById(newsletterId); | ||
|
|
||
| TransactionSynchronizationManager.registerSynchronization( | ||
| new TransactionSynchronization() { | ||
| @Override | ||
| public void afterCommit() { | ||
| log.info("[Newsletter] 분석 재시도 파이프라인 트리거. newsletterId={}", newsletterId); | ||
| newsletterPipelineService.runPipeline(newsletterId); | ||
| } | ||
| }); |
There was a problem hiding this comment.
재시도 전에 Redis preview도 같이 비워야 합니다.
여기서는 DB 파생 데이터만 지우고 이전 preview 키는 그대로 남깁니다. 그런데 preview는 CalendarRegisterServiceImpl.getPreview()/register()에서 상태 검증 없이 그대로 소비되므로, 새 재시도가 saveCalendarPreview()까지 못 가고 다시 실패하면 사용자가 이전 분석의 일정을 그대로 열거나 등록할 수 있습니다. 재시도 트랜잭션 안에서 preview까지 같이 지워 두는 게 안전합니다.
가능한 수정 예시
checklistRepository.deleteByNewsletterId(newsletterId);
calendarEventRepository.deleteByNewsletterIdAndUserId(newsletterId, userId);
+ calendarPreviewRedisService.deletePreview(newsletterId);
Newsletter saved = findNewsletterById(newsletterId);추가로 이 필드 주입이 필요합니다.
private final CalendarPreviewRedisService calendarPreviewRedisService;🤖 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
`@src/main/java/com/gachi/be/domain/newsletter/service/impl/NewsletterServiceImpl.java`
around lines 232 - 243, The transaction currently deletes DB-derived data
(checklistRepository.deleteByNewsletterId and
calendarEventRepository.deleteByNewsletterIdAndUserId) but leaves the Redis
preview key intact; inject a CalendarPreviewRedisService (add private final
CalendarPreviewRedisService calendarPreviewRedisService) into
NewsletterServiceImpl and, inside the same transactional flow before calling
TransactionSynchronizationManager.registerSynchronization(...), call the
calendarPreviewRedisService method that clears the preview for the
newsletter/user (e.g., deleteByNewsletterIdAndUserId(newsletterId, userId) or
deletePreview(newsletterId, userId)) so Redis preview is removed within the
retry transaction and cannot be consumed by
CalendarRegisterServiceImpl.getPreview()/register() from a stale state.
Comment on lines
+6
to
+7
| public record ChangeNotificationRequest( | ||
| @NotNull(message = "notificationEnabled는 필수입니다.") boolean notificationEnabled) {} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Jakarta Bean Validation에서 @NotNull이 primitive boolean 필드에 실제로 검증되는지, 그리고 JSON 필드 누락 시 boolean vs Boolean 바인딩 차이가 무엇인지 공식 문서 기준으로 확인해줘.
💡 Result:
결론부터 말하면, Jakarta Bean Validation의 @NotNull이 primitive boolean(boolean 타입) 필드에 대해 “실제로 검증되는가?”에 대한 답은 “null 여부를 기준으로 한 @NotNull 검증은 의미가 없다(=검증 대상이 null이 될 수 없다)”입니다. 또한 JSON에서 필드가 누락되면, boolean(primitive)과 Boolean(래퍼) 바인딩 차이 때문에 “누락을 검증으로 잡을 수 있느냐”가 달라집니다. 1) @NotNull이 primitive boolean에 대해 검증되는가? - Jakarta Bean Validation의 @NotNull은 “annotated element must not be null(해당 엘리먼트는 null이 아니어야 함)”을 요구합니다.[1][2] - 그런데 Java의 primitive boolean 타입은 null 값을 가질 수 없습니다(primitive 타입은 null을 수용하지 않음). JSON-B 명시에서도 primitive 타입은 null을 받아들일 수 없어서(“Primitive types cannot accept null values…”) 필수/기본값 로직이 따로 존재함을 밝힙니다.[3][4] - 따라서 primitive boolean 필드에 @NotNull을 붙여도, 바인딩/프로퍼티 값이 애초에 null이 될 수 없으므로 @NotNull이 “누락/NULL” 케이스를 잡아주는 검증으로 동작하지 않습니다(실질적으로는 항상 null이 아닌 값이 들어오는 형태). 2) JSON 필드 누락 시 boolean vs Boolean 바인딩 차이 A. JSON-B 관점(공식 사양 기준) - JSON 문서에서 property가 “absent(누락)”인 경우: deserialization은 그 필드 값을 세팅하지 않으며(setter 호출도 하지 않음), thus original value of the field(필드의 기존 값)을 보존해야 한다고 명시돼 있습니다.[3][4] - 또한 primitive 타입은 null을 받아들일 수 없으므로, absent/nullable 처리와 별개로 primitive boolean에는 “필요한 값”이 들어가야 합니다. JSON-B에서는 primitive 타입 대신 래퍼/Optional 등이 null/부재를 표현할 수 있는 쪽으로 설계돼 있습니다.[3][4] - 요약하면: - boolean(primitive) 타입: JSON에서 필드가 누락되면 “null로 들어오는 게 아니라” 자바의 primitive 기본값(false) 또는 초기값 유지 형태로 귀결됩니다(결과적으로 @NotNull이 잡을 ‘null’ 상태가 생기지 않음).[3][4] - Boolean(래퍼) 타입: JSON에서 필드가 누락되면 필드에 null이 들어가거나(또는 기존 값이 유지) ‘null 상태’를 만들 수 있어, 그 다음 Bean Validation의 @NotNull이 누락을 잡는 형태가 됩니다(“must not be null”이 실제로 위반 가능).[1][2][3][4] B. @AssertTrue/@assertfalse와의 관련(공식 문맥: primitive/Boolean 모두 지원) - Hibernate Validator 문서에는 @AssertTrue/@assertfalse가 Boolean과 boolean 모두 지원된다고 나와 있으며, @NotNull은 “annotated value is not null” 체크로 정의됩니다.[5] - 다만 “누락”을 잡으려면 결국 null 여부를 판별할 수 있어야 하는데, primitive boolean은 null 표현이 불가능하므로 @NotNull로 ‘누락’을 검출하는 목적에는 맞지 않습니다. 3) 실무적으로 의미있는 방법(공식 문서가 직접 제시하는 방향에 근거한 결론) - “클라이언트가 반드시 JSON에서 필드를 제공해야 한다(누락이면 400)” 같은 요구사항을 Bean Validation @NotNull로 처리하려면, 해당 필드를 반드시 nullable을 표현할 수 있게 해야 합니다. - 즉 boolean primitive가 아니라 Boolean(또는 Optional 계열)로 모델링한 뒤 @NotNull을 거는 것이 @NotNull의 의미(‘null이 아니어야 함’)와 정확히 맞습니다.[1][2][3][4] - primitive boolean에 @NotNull을 붙이는 것만으로는 “누락”을 null 위반으로 잡지 못합니다. 원문 확인용 핵심 근거(요약) - @NotNull: “must not be null(해당 엘리먼트는 null이 아니어야)”[1][2] - JSON-B: JSON property absent 시 setter 미호출 + original value 보존[3][4] - JSON-B: primitive types는 null을 받아들일 수 없음 + primitive의 기본값 로직 표기[3][4]
Citations:
- 1: https://jakarta.ee/specifications/bean-validation/3.0/apidocs/jakarta/validation/constraints/notnull
- 2: https://jakarta.ee/specifications/bean-validation/3.1/apidocs/jakarta/validation/constraints/NotNull.html
- 3: https://jakarta.ee/specifications/jsonb/3.0/jakarta-jsonb-spec-3.0
- 4: https://github.com/jakartaee/jsonb-api/blob/master/spec/src/main/asciidoc/jsonb.adoc
- 5: https://docs.hibernate.org/validator/5.0/reference/en-US/html_single/
@NotNull을 boolean(primitive) 필드에 적용해 “필수”가 보장되지 않습니다.
@NotNull은 “null이 아니어야 함”을 검증하는데, boolean은 null을 가질 수 없습니다. 따라서 JSON에서 해당 프로퍼티가 누락되면(예: JSON-B 명세상 absent 시 setter 미호출/기본값 유지) @NotNull로 누락을 위반으로 잡기 어렵고 기본값(false)으로 처리될 수 있어 “필수 입력 검증” 의도와 어긋납니다.
- 해결:
boolean→Boolean(또는Optional<Boolean>)로 바꾼 뒤@NotNull을 적용
🤖 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
`@src/main/java/com/gachi/be/domain/user/dto/request/ChangeNotificationRequest.java`
around lines 6 - 7, ChangeNotificationRequest's notificationEnabled is declared
as primitive boolean with `@NotNull`, which doesn't enforce presence because
primitives can't be null; change the type to the boxed Boolean (or
Optional<Boolean>) so `@NotNull` can validate missing JSON values, update the
record signature in ChangeNotificationRequest accordingly, and adjust any
callers/consumers (constructors, mappers, tests) that instantiate or read
notificationEnabled to handle the boxed type (and its potential null) or unwrap
Optional as needed.
Comment on lines
+116
to
+121
| public void updateLanguage(String languageCode) { | ||
| if (languageCode == null || languageCode.isBlank()) { | ||
| throw new IllegalArgumentException("languageCode는 비어 있을 수 없습니다."); | ||
| } | ||
| this.languageCode = languageCode.trim().toUpperCase(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
updateLanguage에서 허용 언어 코드 불변식을 함께 강제해 주세요.
Line 120 기준으로 현재는 공백만 막고 임의 문자열이 저장될 수 있습니다. DTO 검증을 우회하는 경로를 대비해 엔티티에서도 KO|US|ZH|VI를 검증하는 게 안전합니다.
제안 수정안
+import java.util.Set;
+
public class User {
+ private static final Set<String> SUPPORTED_LANGUAGE_CODES = Set.of("KO", "US", "ZH", "VI");
+
public void updateLanguage(String languageCode) {
if (languageCode == null || languageCode.isBlank()) {
throw new IllegalArgumentException("languageCode는 비어 있을 수 없습니다.");
}
- this.languageCode = languageCode.trim().toUpperCase();
+ String normalized = languageCode.trim().toUpperCase();
+ if (!SUPPORTED_LANGUAGE_CODES.contains(normalized)) {
+ throw new IllegalArgumentException("지원하지 않는 languageCode입니다.");
+ }
+ this.languageCode = normalized;
}📝 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 void updateLanguage(String languageCode) { | |
| if (languageCode == null || languageCode.isBlank()) { | |
| throw new IllegalArgumentException("languageCode는 비어 있을 수 없습니다."); | |
| } | |
| this.languageCode = languageCode.trim().toUpperCase(); | |
| } | |
| public void updateLanguage(String languageCode) { | |
| if (languageCode == null || languageCode.isBlank()) { | |
| throw new IllegalArgumentException("languageCode는 비어 있을 수 없습니다."); | |
| } | |
| String normalized = languageCode.trim().toUpperCase(); | |
| if (!SUPPORTED_LANGUAGE_CODES.contains(normalized)) { | |
| throw new IllegalArgumentException("지원하지 않는 languageCode입니다."); | |
| } | |
| this.languageCode = normalized; | |
| } |
🤖 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 `@src/main/java/com/gachi/be/domain/user/entity/User.java` around lines 116 -
121, In User.updateLanguage, besides the existing null/blank check, enforce the
allowed language-code invariant by normalizing languageCode (trim + toUpperCase)
then validating it against the allowed set { "KO", "US", "ZH", "VI" } (or an
equivalent static Set/enum defined on the User class); if the normalized code is
not in that set, throw an IllegalArgumentException with a clear message. Ensure
the method stores only the validated normalized value in this.languageCode and
consider extracting the allowed codes to a private static constant (e.g.,
ALLOWED_LANGUAGE_CODES) for reuse.
Comment on lines
+12
to
+14
| private String baseUrl = "http://localhost:8000"; | ||
| private int connectTimeoutSeconds = 10; | ||
| private int readTimeoutSeconds = 120; |
There was a problem hiding this comment.
타임아웃 설정값 하한 검증이 없어 0/음수 설정 시 런타임 실패가 발생할 수 있습니다.
connectTimeoutSeconds/readTimeoutSeconds가 0 이하로 주입되면 HttpClient/HttpRequest 타임아웃 설정에서 예외가 발생합니다. 프로퍼티 바인딩 단계에서 검증해 조기 실패시키는 게 안전합니다.
🔧 제안 수정안
import lombok.Getter;
-import lombok.Setter;
import org.springframework.boot.context.properties.ConfigurationProperties;
`@Getter`
-@Setter
`@ConfigurationProperties`(prefix = "app.ai-server")
public class AiServerProperties {
private String baseUrl = "http://localhost:8000";
private int connectTimeoutSeconds = 10;
private int readTimeoutSeconds = 120;
+
+ public void setConnectTimeoutSeconds(int connectTimeoutSeconds) {
+ if (connectTimeoutSeconds < 1) {
+ throw new IllegalArgumentException(
+ "app.ai-server.connect-timeout-seconds must be >= 1");
+ }
+ this.connectTimeoutSeconds = connectTimeoutSeconds;
+ }
+
+ public void setReadTimeoutSeconds(int readTimeoutSeconds) {
+ if (readTimeoutSeconds < 1) {
+ throw new IllegalArgumentException(
+ "app.ai-server.read-timeout-seconds must be >= 1");
+ }
+ this.readTimeoutSeconds = readTimeoutSeconds;
+ }
}📝 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
| private String baseUrl = "http://localhost:8000"; | |
| private int connectTimeoutSeconds = 10; | |
| private int readTimeoutSeconds = 120; | |
| import lombok.Getter; | |
| import org.springframework.boot.context.properties.ConfigurationProperties; | |
| `@Getter` | |
| `@ConfigurationProperties`(prefix = "app.ai-server") | |
| public class AiServerProperties { | |
| private String baseUrl = "http://localhost:8000"; | |
| private int connectTimeoutSeconds = 10; | |
| private int readTimeoutSeconds = 120; | |
| public void setConnectTimeoutSeconds(int connectTimeoutSeconds) { | |
| if (connectTimeoutSeconds < 1) { | |
| throw new IllegalArgumentException( | |
| "app.ai-server.connect-timeout-seconds must be >= 1"); | |
| } | |
| this.connectTimeoutSeconds = connectTimeoutSeconds; | |
| } | |
| public void setReadTimeoutSeconds(int readTimeoutSeconds) { | |
| if (readTimeoutSeconds < 1) { | |
| throw new IllegalArgumentException( | |
| "app.ai-server.read-timeout-seconds must be >= 1"); | |
| } | |
| this.readTimeoutSeconds = readTimeoutSeconds; | |
| } | |
| } |
🤖 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 `@src/main/java/com/gachi/be/global/config/external/AiServerProperties.java`
around lines 12 - 14, AiServerProperties currently allows connectTimeoutSeconds
and readTimeoutSeconds to be set to 0 or negative which will cause runtime
failures; add input validation in the AiServerProperties binding so these fields
are validated at startup (e.g., annotate the class with `@Validated` and add
`@Min`(1) to the connectTimeoutSeconds and readTimeoutSeconds fields, or perform
checks in their setters/constructor and throw IllegalArgumentException when
value <= 0) to fail fast and provide a clear error message referencing the
offending property names.
Comment on lines
+1
to
+2
| ALTER TABLE users | ||
| ADD COLUMN IF NOT EXISTS language_code VARCHAR(10) NOT NULL DEFAULT 'KO'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
language_code 허용값을 DB 제약으로도 고정해 주세요.
현재는 컬럼만 추가되어 DB에 임의 코드가 들어갈 수 있습니다. 애플리케이션 검증 우회 상황을 대비해 CHECK 제약을 함께 두는 편이 안전합니다.
제안 수정안
ALTER TABLE users
ADD COLUMN IF NOT EXISTS language_code VARCHAR(10) NOT NULL DEFAULT 'KO';
+
+ALTER TABLE users
+ ADD CONSTRAINT chk_users_language_code
+ CHECK (language_code IN ('KO', 'US', 'ZH', 'VI'));📝 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
| ALTER TABLE users | |
| ADD COLUMN IF NOT EXISTS language_code VARCHAR(10) NOT NULL DEFAULT 'KO'; | |
| ALTER TABLE users | |
| ADD COLUMN IF NOT EXISTS language_code VARCHAR(10) NOT NULL DEFAULT 'KO'; | |
| ALTER TABLE users | |
| ADD CONSTRAINT chk_users_language_code | |
| CHECK (language_code IN ('KO', 'US', 'ZH', 'VI')); |
🤖 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 `@src/main/resources/db/migration/V12__user_add_language_code.sql` around lines
1 - 2, Add a DB-level CHECK constraint for the newly added users.language_code
column in V12__user_add_language_code.sql so only the application-supported
language codes are allowed; modify the ALTER TABLE users ... ADD COLUMN IF NOT
EXISTS language_code VARCHAR(10) NOT NULL DEFAULT 'KO' to include a CHECK
(language_code IN (...)) (or add a separate ALTER TABLE ... ADD CONSTRAINT ...
CHECK) using the exact list of allowed codes your app expects so invalid codes
cannot be inserted even if app-side validation is bypassed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 작업 요약
🌿 브랜치 정보
develop(기본)main(릴리즈)✅ 체크리스트
feat/refac/hotfix/chore/design/bugfix)feat/fix/refactor/docs/style/chore)🧪 테스트 결과
GitHub Actions
deploy-ec2실행 확인 (workflow_dispatch, ref:main)원격 배포 순서/재기동 확인
배포 후 컨테이너 상태 확인
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation