[FIX] 푸시 알림 중복 발송 해결 (stage -> prod) #426
Conversation
* Merge branch 'prod' of https://github.com/Team-Clody/Clody_server int… (#351) * hotfix/#349: 한국 로디 지연 메세지 처리 * hotfix/#349: rollback * fix: COMPLETED 조건 확인 및 로깅강화 * fix: 에러 원인 분석 로그 추가 * fix: do-while에서 COMPLETE제거 * fix: 답장 처리 다시 gpt로 변경 * 일단 되던 상태로 revert * fix: 답장 재시도 로직 제거 * fix: 로깅 보강 * fix: 자동으로 미안하단 응답 넣기 * hotfix/#349: 로깅 강화 복구 * hotfix/#349: 로디 오류 발생 시 긴급 고정 문구로 답장주도록 수정 * hotfix/#349: 고정 답변 체크 join 수정 * hotfix/#349: 고정 답변 체크 로직 재수정 * hotfix/#349: isFixedAnswerRequired 로그 추가 * hotfix/#349: 대기 시간 늘리기 * hotfix/#349: thread run 실패 시 고정답변 강제 return * hotfix/#349: 재시도 및 쓰레드 실행 실패 이유 로깅 강화 * hotfix/#349: 로디 -> ko bedrock 테스트 배포 * hotfix/#349: infra yml 변경 * hotfix/#349: ko bedrock 호출 reformat * hotfix/#349: 시스템 상 고정 답변으로 rollback * fix/#285: 서버스캐닝 알림 off (#354) * [Fix] system instruction과 user input 분리 (#356) * chore/#: local 로그 설정 컬러로 변경 * fix/#355: system instruction과 user input 분리 * fix/#357: Alarm table Time column 저장 형태 반영 (#358) * Update README.md * hotFix/#359: SupportedLanguage 글로벌 대응 개선 (#360) * [Fix] REPLY_CONTENT_TIMEOUT 대응 (#361) * chore/#: local 로그 설정 컬러로 변경 * fix/#355: system instruction과 user input 분리 * fix: GPT 쓰레드 실패시 에러 던지도록 처리 * fix: waitForContent 레이스컨디션 대응하여 여유 확보(GPT Thread.Sleep 1000 대응) * chore: logging 수정 * refactor/#362 : Azure HttpClient timeout 조절에 따른 통신 및 광고 후 답장 로직 수정 (#363) * fix: 광고 대기 시간 float 살리기 * fix: 광고 대기 시간 float 살리기 * fix/#364 : 임시 저장 일기 조회 insert 순서대로 반환 (#365) * fix/#366 : 해외 유저 광고 에러 해결 (#367) * Fix/#368 : Spring-Azure 통신 시 ASSISTANT 응답 체크 고도화 (#369) * fix/#368 : spring-azure 통신 시 응답 규격 체크 고도화 * fix/#368 : messageContent null 체크 추가 * fix/#368 : normalize 추가 * Update README.md * chore: 광고 후 답장 로그 추가 * chore: 일기 작성 리마인드 알람 발송 로그 추가 * chore: 일기 답장 도착 알림 발송 로그 추가 * Feat/#370 : Spring-Azure 통신 횟수 제한 (#371) * feat/#370: azure openai 통신 부분 rate limit 추가 * chore: 일기 생성 로직 잘못된 주석 설명 올바르게 수정 * chore: 통신 횟수 초과 시 HttpStatus 변경 (#370) (#371) * Feat/#374: 일기 본문은 사용자만 확인할 수 있도록 프라이버시 개선 (#375) * feat/#374: 일기 본문은 사용자만 확인할 수 있도록 개선 * fix/#374: prod와 stage 브랜치 history 불일치 해결 * refactor/#374: jpa dirty checking 위험 제거 * Feat/#377: 알림 발송 실패 시 후처리 추가 (#378) * feat/#377: 알림 발송 실패 후처리 추가 * feat/#377: 트랜잭션 추가 * Feat/#379: 정규식 기반 욕설 필터링 개선 (#380) * feat/#379: 욕설 필터링 개선 * feat/#379: 이모지 정규식 개선 * refactor/#381: JwtUtil static -> di로 변경 (#382) * refactor/#383: 답장 조회 DDD 만족하도록 리펙토링 (#384) * refactor: 욕설 필터 상수화 개선 (#379) (#380) * feat/#376: 한국 유저 일기 답장 프롬프트 적용 Spring-Gemini LLM 통신 확인 완료 * feat/#376: Spring-Gemini LLM 통신 에러 핸들링 및 재시도 추가 * feat/#376: Spring-Gemini LLM 통신 RATE LIMIT 추가 --------- Co-authored-by: Sewon Min <[email protected]> Co-authored-by: givemethatsewon <[email protected]>
* refactor/#362 : azure httpClient timeout 조절에 따른 통신 및 광고 후 답장 로직 수정 * refactor : 발송 이전 검증 필터 추가 / 상세 로깅 추가 --------- Co-authored-by: rla124 <[email protected]>
WalkthroughThis PR introduces multiple new features across the application: a weekly diary calendar retrieval endpoint, user personal information update functionality (gender and birthDate), a journal prompt system for daily entries, reply content update capability with event handling, and extends user domain models to track gender and birthDate throughout the system. Changes span API controllers, application services, domain models, repositories, and infrastructure layers. Changes
Sequence DiagramssequenceDiagram
participant Client
participant PromptController as PromptController<br/>(Adapter)
participant PromptRetrieval as PromptRetrievalService<br/>(Application)
participant PromptQuery as PromptQueryService<br/>(Domain)
participant PromptRepo as PromptRepository<br/>(Domain)
participant JpaRepo as JpaJournalPrompt<br/>Repository
participant DB as Database
Client->>PromptController: GET /api/v1/journal/prompt<br/>(month, date, Time-Zone header)
PromptController->>PromptRetrieval: getJournalPrompt(month, date, ZoneId)
PromptRetrieval->>PromptRetrieval: Convert to PromptDateInfo
PromptRetrieval->>PromptQuery: getJournalPrompt(PromptDateInfo)
PromptQuery->>PromptRepo: findByMonthDay(monthDay)
PromptRepo->>JpaRepo: findByMonthDay(monthDay)
JpaRepo->>DB: Query JournalPrompt by MonthDay
DB-->>JpaRepo: JournalPrompt
JpaRepo-->>PromptRepo: Optional<JournalPrompt>
PromptRepo-->>PromptQuery: JournalPrompt
PromptQuery->>PromptQuery: Select content by timezone<br/>(KST→KO, else→EN)
PromptQuery-->>PromptRetrieval: JournalPromptInfo
PromptRetrieval->>PromptRetrieval: Map to PromptGetResponse
PromptRetrieval-->>PromptController: PromptGetResponse
PromptController-->>Client: 200 OK<br/>ApiResponse<PromptGetResponse>
sequenceDiagram
participant Client
participant UserController as UserController<br/>(Adapter)
participant UserUpdateService as UserUpdateService<br/>(Application)
participant UserCommandService as UserCommandService<br/>(Domain)
participant UserRepo as UserRepository
participant DB as Database
Client->>UserController: PATCH /user/personal/info<br/>UserPersonalInfoPatchRequest
UserController->>UserUpdateService: updateUserPersonalInfo(request)
UserUpdateService->>UserUpdateService: Convert gender via<br/>Gender.fromString()
UserUpdateService->>UserCommandService: patchUserPersonalInfo(gender, birthDate)
UserCommandService->>UserRepo: Update user entity
UserRepo->>DB: UPDATE user<br/>(gender, birthDate)
DB-->>UserRepo: ✓ Updated
UserRepo-->>UserCommandService: UserInfo
UserCommandService-->>UserUpdateService: UserInfo
UserUpdateService->>UserUpdateService: Map to<br/>UserPersonalInfoPatchResponse
UserUpdateService-->>UserController: UserPersonalInfoPatchResponse
UserController-->>Client: 200 OK<br/>ApiResponse<UserPersonalInfoPatchResponse>
sequenceDiagram
participant Domain as ReplyEventHandler<br/>(Domain Event)
participant ReplyUpdateService as ReplyUpdateService<br/>(Domain Service)
participant ReplyRepo as ReplyRepository
participant DB as Database
participant Reply as Reply Entity
Domain->>ReplyUpdateService: handleReplyMessage(message)<br/>with replyId, content
ReplyUpdateService->>ReplyRepo: Get Reply by ID
ReplyRepo->>DB: SELECT reply
DB-->>ReplyRepo: Reply entity
ReplyRepo-->>ReplyUpdateService: Reply
ReplyUpdateService->>Reply: updateContent(result)
Reply->>Reply: Set content
ReplyUpdateService->>Reply: setReplyProcessStatus(SUCCEED)
ReplyUpdateService->>Reply: disablePushNotification()
ReplyUpdateService->>ReplyRepo: Save updated Reply
ReplyRepo->>DB: UPDATE reply
DB-->>ReplyRepo: ✓ Persisted
Note over ReplyUpdateService: Transaction<br/>REQUIRES_NEW
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
clody-app/src/main/java/com/clody/clodyapi/user/application/service/UserApplicationService.java (2)
101-103: ReplaceSystem.out.println/System.err.printlnwith proper logging.The class has
@Slf4jannotation, so uselog.info()andlog.error()instead ofSystem.out/System.errfor consistent logging and proper log level control in production.Proposed fix
notificationService.sendRegistrationNotification(savedUser.getEmail(), userCount) - .subscribe(result -> System.out.println("알림 전송 완료: " + result), - error -> System.err.println("알림 전송 실패: " + error.getMessage())); + .subscribe(result -> log.info("알림 전송 완료: {}", result), + error -> log.error("알림 전송 실패: {}", error.getMessage()));
153-155: Same issue: ReplaceSystem.out/System.errwith proper logging.Consistent with the previous comment, use
log.info()andlog.error()here as well.Proposed fix
notificationService.sendRegistrationNotification(newUser.getEmail(), 1L) - .subscribe(result -> System.out.println("알림 전송 완료: " + result), - error -> System.err.println("알림 전송 실패: " + error.getMessage())); + .subscribe(result -> log.info("알림 전송 완료: {}", result), + error -> log.error("알림 전송 실패: {}", error.getMessage()));
🧹 Nitpick comments (23)
clody-app/src/main/java/com/clody/clodyapi/prompt/adapter/in/dto/PromptRequest.java (1)
10-17: Consider adding validation annotations.The
datefield is marked as required in the Swagger schema but lacks runtime validation annotations. Adding@NotNullfrom Jakarta Bean Validation would enforce the requirement at runtime and prevent null values.🔎 Proposed enhancement
package com.clody.clodyapi.prompt.adapter.in.dto; import io.swagger.v3.oas.annotations.media.Schema; +import jakarta.validation.constraints.NotNull; import java.time.MonthDay; @Schema( name = "PromptRequest", description = "일자 별 저널 프롬프트 요청 DTO") public record PromptRequest( @Schema( description = "유저의 현재 날짜(MM-DD)", example = "07-27", requiredMode = Schema.RequiredMode.REQUIRED ) + @NotNull MonthDay date ) { }clody-app/src/main/java/com/clody/clodyapi/diary/mapper/DiaryMapper.java (1)
14-14: Consider using explicit imports instead of wildcard.Wildcard imports can reduce code clarity and may cause issues with IDE auto-completion or when classes with similar names exist in different packages.
clody-app/src/main/java/com/clody/clodyapi/diary/adapter/in/web/DiaryController.java (1)
54-65: Typo in method name and potential parsing concern.
Method name
getWeeklyDiaryCalendercontains a typo - should begetWeeklyDiaryCalendar. Note: existing methodgetDiaryCalenderhas the same typo, so this may be intentional for consistency.Using
LocalDateTimedirectly as a@RequestParamrequires proper format handling. Ensure Spring is configured to parse the expected date-time format (e.g.,2025-12-20T10:30:00), or add@DateTimeFormatannotation to specify the pattern explicitly.clody-domain/src/main/java/com/clody/domain/reply/Reply.java (1)
192-194: Pre-existing bug: status parameter is ignored.The
statusparameter is unused—the method always setsSUCCEEDregardless of input. While this doesn't affect the current PR (which passesSUCCEED), it should be fixed to avoid confusion.🔎 Proposed fix
public void updateReplyProcessStatus(ReplyProcessStatus status) { - this.replyInfo = replyInfo.update(this.replyInfo.getVersion(), ReplyProcessStatus.SUCCEED); + this.replyInfo = replyInfo.update(this.replyInfo.getVersion(), status); }clody-domain/src/main/java/com/clody/domain/reply/event/ReplyEventHandler.java (1)
16-22: Consider adding logging for observability.The handler silently delegates to the service. Adding a log statement would help with debugging and monitoring, especially since this is part of a fix for duplicate notifications.
🔎 Proposed enhancement
@EventListener public void handleReplyMessage(Message message) { + log.info("Handling reply message: replyId={}", message.replyId()); replyUpdateService.updateReplyWithContent( message.replyId(), message.content() ); }clody-domain/src/main/java/com/clody/domain/reply/service/ReplyUpdateService.java (1)
12-15: Unused@Slf4jannotation.The logger is declared but not used. Either remove it or add logging for observability.
clody-infra/src/main/java/com/clody/infra/models/reply/repository/JpaReplyRepository.java (1)
31-31: Method signature follows Spring Data JPA conventions correctly.The
findAllByUserId(Long userId)method follows Spring Data JPA naming conventions and correctly generates a query to fetch all Reply entities where userId matches the parameter. However, verify the implementation ensures:
- Deduplication logic in callers — If used in notification flows, callers should deduplicate user IDs (via Set) before sending notifications to avoid sending duplicates to the same user
- Performance for large datasets — If users have many replies, pagination breaks down data into manageable chunks and improves performance and reduces memory usage; consider using a Pageable parameter variant if this method fetches all replies for a single user in high-volume scenarios
clody-infra/src/main/java/com/clody/infra/models/alarm/DiaryNotifyService.java (1)
45-51: Excellent deduplication strategy that solves the duplicate notification issue.The Set-based deduplication using
sentUserIdsensures each user receives at most one notification per execution, directly addressing issue #424.Optional: Remove redundant
.withSecond(0).Line 48's
.withSecond(0)is redundant sincetruncatedTo(ChronoUnit.MINUTES)on line 47 already sets both seconds and nanoseconds to 0.🔎 Proposed simplification
LocalTime currentTime = LocalTime.now(clock.withZone(ZoneDateTimeConverter.KST)) - .truncatedTo(ChronoUnit.MINUTES) - .withSecond(0); + .truncatedTo(ChronoUnit.MINUTES);clody-support/src/main/java/com/clody/support/dto/type/ErrorType.java (1)
95-98: Add trailing comma for consistency.The new enum constant
JOURNAL_PROMPT_NOT_FOUNDshould have a trailing comma for consistency with Java enum best practices, making future additions safer and reducing diff noise.🔎 Proposed fix
- JOURNAL_PROMPT_NOT_FOUND(HttpStatus.NOT_FOUND.value(), "저널 프롬프트가 존재하지 않는 날짜입니다.") + JOURNAL_PROMPT_NOT_FOUND(HttpStatus.NOT_FOUND.value(), "저널 프롬프트가 존재하지 않는 날짜입니다."), ;clody-domain/src/main/java/com/clody/domain/prompt/repository/PromptRepository.java (1)
7-9: Consider Optional return type or document exception behavior.The
findByMonthDaymethod returnsJournalPromptdirectly rather thanOptional<JournalPrompt>. This design forces the implementation to throw exceptions when no prompt exists for the given date, which is less conventional than returning Optional.If exception-throwing is intentional (as indicated by the infrastructure adapter throwing
JOURNAL_PROMPT_NOT_FOUND), consider adding JavaDoc to document this behavior clearly.🔎 Alternative: Use Optional return type
public interface PromptRepository { - JournalPrompt findByMonthDay(MonthDay monthDay); + /** + * Finds a journal prompt for the given month and day. + * @throws BusinessException with JOURNAL_PROMPT_NOT_FOUND if no prompt exists + */ + JournalPrompt findByMonthDay(MonthDay monthDay);Or refactor to return Optional:
public interface PromptRepository { - JournalPrompt findByMonthDay(MonthDay monthDay); + Optional<JournalPrompt> findByMonthDay(MonthDay monthDay); }clody-app/src/main/java/com/clody/clodyapi/prompt/application/port/in/PromptQueryUsecase.java (1)
7-9: Consider using MonthDay parameter or documenting valid ranges.The method accepts
int monthandint dateas separate parameters without documenting valid ranges or whether month is 0-indexed or 1-indexed. Since the domain already usesMonthDay(as seen in PromptRepository), consider accepting aMonthDayparameter directly, or add JavaDoc specifying valid ranges.🔎 Alternative approaches
Option 1: Use MonthDay parameter
public interface PromptQueryUsecase { - PromptGetResponse getJournalPrompt(int month, int date, ZoneId clientTimeZone); + PromptGetResponse getJournalPrompt(MonthDay monthDay, ZoneId clientTimeZone); }Option 2: Add JavaDoc for validation
public interface PromptQueryUsecase { + /** + * Retrieves journal prompt for the given date. + * @param month 1-based month (1-12) + * @param date day of month (1-31) + * @param clientTimeZone client's timezone + */ PromptGetResponse getJournalPrompt(int month, int date, ZoneId clientTimeZone); }clody-app/src/main/java/com/clody/clodyapi/user/adapter/in/dto/UserPersonalInfoPatchRequest.java (1)
5-8: Consider adding validation constraints.The record lacks validation annotations for the
genderandbirthDatefields. Consider:
- Gender validation: Add
@Patternto validate against expected values (e.g., "MALE", "FEMALE", etc.) matching the Gender enum, or document accepted values- BirthDate validation: Add constraints like
@Pastor@PastOrPresentto ensure valid birth dates- Nullability: Document whether these fields are optional for PATCH operations or add
@NotNullif required🔎 Proposed validation additions
package com.clody.clodyapi.user.adapter.in.dto; +import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.PastOrPresent; +import jakarta.validation.constraints.Pattern; import java.time.LocalDate; public record UserPersonalInfoPatchRequest( + @Pattern(regexp = "MALE|FEMALE|OTHER", message = "Invalid gender value") String gender, + @PastOrPresent(message = "Birth date must be in the past") LocalDate birthDate ) { }Note: Adjust the pattern regex to match your actual Gender enum values.
clody-app/src/main/java/com/clody/clodyapi/user/adapter/in/web/UserControllerImpl.java (1)
51-58: Consider adding request validation.The endpoint accepts
@RequestBodywithout@Validannotation. Ensure thatUserPersonalInfoPatchRequesthas appropriate validation constraints (e.g.,@NotNull,@Pastfor birthDate) and add@Validto enforce them at the controller level.🔎 Suggested validation enhancement
@Override @PatchMapping("/user/personal/info") public ResponseEntity<ApiResponse<UserPersonalInfoPatchResponse>> patchUserPersonalInfo( - @RequestBody final UserPersonalInfoPatchRequest userPersonalInfoPatchRequest) { + @Valid @RequestBody final UserPersonalInfoPatchRequest userPersonalInfoPatchRequest) { return ResponseEntity.status(HttpStatus.OK) .body(ApiResponse.success(SuccessType.OK_SUCCESS, userUpdateUsecase.updateUserPersonalInfo(userPersonalInfoPatchRequest))); }clody-domain/src/main/java/com/clody/domain/prompt/service/PromptQueryService.java (1)
21-24: Consider using locale-based content selection instead of timezone comparison.The current approach equates
ZoneId.equals(KST)with Korean language preference. This may not always be accurate—a user in Korea might prefer English content, or a Korean speaker abroad might want Korean content.If this is intentional business logic (timezone determines language), it's acceptable. Otherwise, consider using an explicit locale or language preference from user settings.
clody-app/src/main/java/com/clody/clodyapi/prompt/mapper/PromptMapper.java (1)
10-14: Consider making utility class non-instantiable.Static utility classes should typically have a private constructor to prevent instantiation and be marked
finalto prevent subclassing.🔎 Proposed fix
-public class PromptMapper { +public final class PromptMapper { + private PromptMapper() { + // Utility class + } + public static PromptDateInfo toPromptDateInfo(int month, int dayOfMonth, ZoneId clientTimeZone) {clody-app/src/main/java/com/clody/clodyapi/prompt/application/service/PromptRetrievalService.java (2)
16-16: Unused@Slf4jannotation.The
@Slf4jannotation is present butlogis never used in this class. Consider removing it to keep the code clean.Proposed fix
-@Slf4j @Service @RequiredArgsConstructor
22-28: Consider validating month and date parameters.Invalid values for
month(e.g., 0, 13) ordate(e.g., 0, 32) will causeDateTimeExceptionwhenPromptMapper.toPromptDateInfocreates aMonthDay. While the exception will propagate, explicit validation with a clear error message would improve the API experience.Verify how the downstream
PromptMapper.toPromptDateInfohandles invalid inputs and whether a global exception handler convertsDateTimeExceptionto a user-friendly response.clody-app/src/main/java/com/clody/clodyapi/user/adapter/in/dto/UserSignUpRequest.java (1)
7-16: Consider adding validation annotations for required fields.The record lacks validation annotations like
@NotBlankor@NotNullfor required fields. Additionally,birthDateshould validate that it's not in the future. This could allow invalid data to reach the service layer.import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.Past; public record UserSignUpRequest( @Schema(description = "로그인 플랫폼", example = "apple/kakao") @NotBlank String platform, @NotBlank String email, @NotBlank String name, String fcmToken, String gender, @Schema(description = "생년월일", example = "YYYY-MM-DD(1987-12-31)") @Past LocalDate birthDate ) {}clody-app/src/main/java/com/clody/clodyapi/prompt/adapter/in/web/PromptController.java (1)
19-19: Method namegetJournalPromptListsuggests a list but returns a single prompt.The method returns
PromptGetResponse(singular), but the name implies a list. Consider renaming togetJournalPromptfor clarity.clody-app/src/main/java/com/clody/clodyapi/prompt/adapter/in/web/PromptControllerImpl.java (1)
20-20: Unused@Slf4jannotation.The
logobject is not used in this class. Consider removing it for cleaner code.clody-app/src/main/java/com/clody/clodyapi/user/application/service/UserApplicationService.java (1)
84-86: Overly broad exception catch hides root causes.Catching
Exceptionis too broad and may mask specific issues. Consider catching more specific exceptions (e.g.,GeneralSecurityException,IOExceptionfrom Google OAuth) to allow unexpected errors to propagate properly.clody-domain/src/main/java/com/clody/domain/prompt/JournalPrompt.java (1)
1-30: Entity appears correctly structured for read-only prompt data.The entity uses proper JPA annotations, and the
MonthDayconverter with column length 5 correctly accommodates the "MM-DD" format. The absence of setters/builders suggests this is intentional read-only reference data populated externally.Minor style note: consider replacing the wildcard import
jakarta.persistence.*with explicit imports for better readability, though this is optional.clody-domain/src/main/java/com/clody/domain/user/User.java (1)
86-113: Consider reducing duplication in factory method.The Apple and Kakao branches differ only in the email source. This could be simplified:
🔎 Optional refactor to reduce duplication
- public static User createNewUser(String platformId, Platform platform, Gender gender, LocalDate birthDate, String appleEmail, String nickName, - String kakaoEmail) { - if (platform == Platform.APPLE) - { - return User.builder() - .platformID(platformId) - .platform(platform) - .gender(gender) - .birthDate(birthDate) - .email(appleEmail) - .nickName(nickName) - .is_deleted(false) - .build(); - } - else - { - return User.builder() - .platformID(platformId) - .platform(platform) - .gender(gender) - .birthDate(birthDate) - .email(kakaoEmail) - .nickName(nickName) - .is_deleted(false) - .build(); - } - - } + public static User createNewUser(String platformId, Platform platform, Gender gender, LocalDate birthDate, String appleEmail, String nickName, + String kakaoEmail) { + String email = (platform == Platform.APPLE) ? appleEmail : kakaoEmail; + return User.builder() + .platformID(platformId) + .platform(platform) + .gender(gender) + .birthDate(birthDate) + .email(email) + .nickName(nickName) + .is_deleted(false) + .build(); + }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (49)
clody-app/src/main/java/com/clody/clodyapi/diary/adapter/in/web/DiaryController.java(3 hunks)clody-app/src/main/java/com/clody/clodyapi/diary/adapter/in/web/DiaryControllerImpl.java(3 hunks)clody-app/src/main/java/com/clody/clodyapi/diary/application/port/in/DiaryRetrieverUsecase.java(1 hunks)clody-app/src/main/java/com/clody/clodyapi/diary/application/port/in/UserUpdateUsecase.java(1 hunks)clody-app/src/main/java/com/clody/clodyapi/diary/application/service/DiaryRetrieverService.java(2 hunks)clody-app/src/main/java/com/clody/clodyapi/diary/application/service/UserDeletionService.java(1 hunks)clody-app/src/main/java/com/clody/clodyapi/diary/application/service/UserUpdateService.java(2 hunks)clody-app/src/main/java/com/clody/clodyapi/diary/mapper/DiaryMapper.java(2 hunks)clody-app/src/main/java/com/clody/clodyapi/prompt/adapter/in/dto/PromptRequest.java(1 hunks)clody-app/src/main/java/com/clody/clodyapi/prompt/adapter/in/web/PromptController.java(1 hunks)clody-app/src/main/java/com/clody/clodyapi/prompt/adapter/in/web/PromptControllerImpl.java(1 hunks)clody-app/src/main/java/com/clody/clodyapi/prompt/adapter/out/dto/PromptGetResponse.java(1 hunks)clody-app/src/main/java/com/clody/clodyapi/prompt/application/port/in/PromptQueryUsecase.java(1 hunks)clody-app/src/main/java/com/clody/clodyapi/prompt/application/service/PromptRetrievalService.java(1 hunks)clody-app/src/main/java/com/clody/clodyapi/prompt/mapper/PromptMapper.java(1 hunks)clody-app/src/main/java/com/clody/clodyapi/reply/application/service/ReplyAdService.java(0 hunks)clody-app/src/main/java/com/clody/clodyapi/user/adapter/in/dto/UserPersonalInfoPatchRequest.java(1 hunks)clody-app/src/main/java/com/clody/clodyapi/user/adapter/in/dto/UserSignUpRequest.java(1 hunks)clody-app/src/main/java/com/clody/clodyapi/user/adapter/in/web/UserController.java(2 hunks)clody-app/src/main/java/com/clody/clodyapi/user/adapter/in/web/UserControllerImpl.java(2 hunks)clody-app/src/main/java/com/clody/clodyapi/user/adapter/out/dto/UserPersonalInfoPatchResponse.java(1 hunks)clody-app/src/main/java/com/clody/clodyapi/user/application/service/CustomOAuth2UserService.java(1 hunks)clody-app/src/main/java/com/clody/clodyapi/user/application/service/UserApplicationService.java(5 hunks)clody-app/src/main/java/com/clody/clodyapi/user/mapper/UserMapper.java(2 hunks)clody-domain/src/main/java/com/clody/domain/diary/dto/response/WeeklyDiaryGetResponse.java(1 hunks)clody-domain/src/main/java/com/clody/domain/diary/service/DiaryCommandService.java(1 hunks)clody-domain/src/main/java/com/clody/domain/diary/service/DiaryQueryService.java(4 hunks)clody-domain/src/main/java/com/clody/domain/diary/service/UserCommandService.java(3 hunks)clody-domain/src/main/java/com/clody/domain/diary/service/UserQueryService.java(1 hunks)clody-domain/src/main/java/com/clody/domain/prompt/JournalPrompt.java(1 hunks)clody-domain/src/main/java/com/clody/domain/prompt/dto/PromptDateInfo.java(1 hunks)clody-domain/src/main/java/com/clody/domain/prompt/dto/response/JournalPromptInfo.java(1 hunks)clody-domain/src/main/java/com/clody/domain/prompt/repository/PromptRepository.java(1 hunks)clody-domain/src/main/java/com/clody/domain/prompt/service/MonthDayConverter.java(1 hunks)clody-domain/src/main/java/com/clody/domain/prompt/service/PromptQueryService.java(1 hunks)clody-domain/src/main/java/com/clody/domain/reply/Reply.java(1 hunks)clody-domain/src/main/java/com/clody/domain/reply/event/ReplyEventHandler.java(1 hunks)clody-domain/src/main/java/com/clody/domain/reply/repository/ReplyRepository.java(2 hunks)clody-domain/src/main/java/com/clody/domain/reply/service/ReplyUpdateService.java(1 hunks)clody-domain/src/main/java/com/clody/domain/user/Gender.java(1 hunks)clody-domain/src/main/java/com/clody/domain/user/User.java(7 hunks)clody-domain/src/main/java/com/clody/domain/user/dto/UserInfo.java(1 hunks)clody-infra/src/main/java/com/clody/infra/models/alarm/DiaryNotifyService.java(3 hunks)clody-infra/src/main/java/com/clody/infra/models/prompt/JournalPromptRepositoryAdapter.java(1 hunks)clody-infra/src/main/java/com/clody/infra/models/prompt/JpaJournalPromptRepository.java(1 hunks)clody-infra/src/main/java/com/clody/infra/models/reply/RodyProcessorImpl.java(1 hunks)clody-infra/src/main/java/com/clody/infra/models/reply/repository/JpaReplyRepository.java(1 hunks)clody-infra/src/main/java/com/clody/infra/models/reply/repository/ReplyRepositoryAdapter.java(2 hunks)clody-support/src/main/java/com/clody/support/dto/type/ErrorType.java(1 hunks)
💤 Files with no reviewable changes (1)
- clody-app/src/main/java/com/clody/clodyapi/reply/application/service/ReplyAdService.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-30T12:25:16.877Z
Learnt from: givemethatsewon
Repo: Team-Clody/Clody_server PR: 249
File: clody-infra/src/main/java/com/clody/infra/models/alarm/ReplayNotificationJob.java:27-34
Timestamp: 2025-06-30T12:25:16.877Z
Learning: ReplyRepository.findByReplyId() method in the ReplyRepositoryAdapter implementation already includes proper exception handling using orElseThrow() with NotFoundException, so null checks are not needed when calling this method.
Applied to files:
clody-infra/src/main/java/com/clody/infra/models/reply/repository/ReplyRepositoryAdapter.javaclody-infra/src/main/java/com/clody/infra/models/reply/repository/JpaReplyRepository.javaclody-domain/src/main/java/com/clody/domain/reply/repository/ReplyRepository.java
🧬 Code graph analysis (8)
clody-app/src/main/java/com/clody/clodyapi/prompt/application/service/PromptRetrievalService.java (2)
clody-app/src/main/java/com/clody/clodyapi/prompt/mapper/PromptMapper.java (1)
PromptMapper(10-19)clody-domain/src/main/java/com/clody/domain/prompt/service/PromptQueryService.java (1)
Service(13-26)
clody-app/src/main/java/com/clody/clodyapi/diary/application/service/UserUpdateService.java (1)
clody-app/src/main/java/com/clody/clodyapi/user/mapper/UserMapper.java (1)
UserMapper(13-45)
clody-domain/src/main/java/com/clody/domain/user/Gender.java (1)
clody-support/src/main/java/com/clody/support/exception/auth/SignInException.java (1)
SignInException(7-12)
clody-app/src/main/java/com/clody/clodyapi/prompt/adapter/in/web/PromptControllerImpl.java (3)
clody-support/src/main/java/com/clody/support/util/TimeZoneUtils.java (1)
TimeZoneUtils(8-16)clody-app/src/main/java/com/clody/clodyapi/prompt/application/service/PromptRetrievalService.java (1)
Slf4j(16-29)clody-app/src/main/java/com/clody/clodyapi/reply/adapter/in/web/ReplyController.java (1)
RequestMapping(20-68)
clody-app/src/main/java/com/clody/clodyapi/diary/application/service/DiaryRetrieverService.java (1)
clody-app/src/main/java/com/clody/clodyapi/diary/mapper/DiaryMapper.java (1)
DiaryMapper(17-65)
clody-domain/src/main/java/com/clody/domain/prompt/service/PromptQueryService.java (1)
clody-domain/src/main/java/com/clody/domain/diary/service/ZoneDateTimeConverter.java (1)
ZoneDateTimeConverter(8-70)
clody-domain/src/main/java/com/clody/domain/diary/service/DiaryQueryService.java (1)
clody-domain/src/main/java/com/clody/domain/diary/service/ZoneDateTimeConverter.java (1)
ZoneDateTimeConverter(8-70)
clody-infra/src/main/java/com/clody/infra/models/alarm/DiaryNotifyService.java (1)
clody-domain/src/main/java/com/clody/domain/diary/service/ZoneDateTimeConverter.java (1)
ZoneDateTimeConverter(8-70)
⏰ 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-deploy
🔇 Additional comments (38)
clody-app/src/main/java/com/clody/clodyapi/prompt/adapter/in/dto/PromptRequest.java (1)
1-18: Critical: PR objectives and code changes are completely inconsistent.The PR title and objectives describe fixing duplicate push notification issues, but this file introduces a journal prompt request DTO with no relation to push notifications. Either:
- Wrong files were included in this PR, or
- The PR description is incorrect
Please verify this is the intended change for this PR.
clody-domain/src/main/java/com/clody/domain/diary/dto/response/WeeklyDiaryGetResponse.java (1)
3-9: LGTM!Simple and clean record definition with a factory method for creating weekly diary responses.
clody-domain/src/main/java/com/clody/domain/diary/service/DiaryCommandService.java (1)
96-98: Verify the behavioral change in date adjustment logic.The condition was simplified to remove the timezone check, meaning the time adjustment now applies to all client timezones, not just KST. This broadens the scope of when
kstDateis reassigned with the current KST time.Ensure this change is intentional and doesn't cause unintended side effects for users in non-KST timezones.
clody-app/src/main/java/com/clody/clodyapi/diary/application/port/in/DiaryRetrieverUsecase.java (1)
14-14: LGTM!Clean interface extension following the existing pattern.
clody-app/src/main/java/com/clody/clodyapi/diary/mapper/DiaryMapper.java (1)
61-63: LGTM!Consistent with the existing mapper pattern.
clody-app/src/main/java/com/clody/clodyapi/diary/application/service/DiaryRetrieverService.java (1)
35-39: LGTM!Clean implementation following the existing service pattern with proper delegation to the domain layer.
clody-app/src/main/java/com/clody/clodyapi/diary/adapter/in/web/DiaryControllerImpl.java (1)
66-76: LGTM!Implementation follows the established pattern in this controller. The same notes about the method name typo and
LocalDateTimeparsing from the interface review apply here.clody-domain/src/main/java/com/clody/domain/diary/service/DiaryQueryService.java (1)
451-465: LGTM - Clover count logic refactored to be year-agnostic.The refactored
getCloverCount()now correctly aggregates all valid clovers for a user without restricting to a specific year.clody-infra/src/main/java/com/clody/infra/models/reply/RodyProcessorImpl.java (1)
171-171: LGTM!Trivial formatting change adding a trailing newline at end of file.
clody-domain/src/main/java/com/clody/domain/reply/Reply.java (1)
207-210: LGTM!The new
updateContentmethod provides a direct content setter. Note that this differs frominsertContentFromRody(line 171-178) which includes version validation - this simpler path appears intentional for the event-driven update workflow.clody-domain/src/main/java/com/clody/domain/reply/service/ReplyUpdateService.java (1)
18-25: VerifyREQUIRES_NEWpropagation is intentional.Using
REQUIRES_NEWstarts a separate transaction that commits independently. This is appropriate if the caller is already in a transaction and you need this update to persist regardless of the outer transaction's outcome. However, if the event publisher and handler share the same thread/transaction context, this could lead to the update being committed even if the parent operation fails.clody-infra/src/main/java/com/clody/infra/models/reply/repository/ReplyRepositoryAdapter.java (1)
118-121: LGTM!Clean delegation implementation. Note that this method returns an empty list when no replies exist (unlike
findByIdwhich throwsNotFoundException), which is appropriate for bulk retrieval scenarios.clody-domain/src/main/java/com/clody/domain/reply/repository/ReplyRepository.java (2)
33-33: Verify the connection between this method and the push notification fix.The new
findAllByUserId(Long userId)method appears to be a generic repository method for fetching all replies for a given user. However, it's unclear how this relates to fixing duplicate push notifications, as mentioned in the PR objectives.Based on the PR description, the fix involves using a Set to ensure only users with unique
userIdreceive notifications. This repository method doesn't inherently provide deduplication—that logic would need to be in the caller.Verify how this method is used in the notification sending logic and confirm that proper deduplication (e.g., collecting distinct
userIdvalues into a Set) is applied before sending push notifications.
1-34: Rewrite comment based on verification:The Set-based deduplication logic you mentioned does exist in the codebase—specifically in DiaryNotifyService.sendDiaryAlarms() (line 51), which uses
Set<Long> sentUserIdswith filtering to prevent duplicate notifications. However, your observation that this logic is not visible in the ReplyRepository interface file is correct.The new
findAllByUserIdmethod added to ReplyRepository is not used by the current deduplication mechanism. The existing deduplication in DiaryNotifyService operates at the Alarm level (collecting user IDs from Alarm entities directly), whilefindAllByUserIdis currently used only in DiaryQueryService for clover count calculations. Verify that this new repository method aligns with the stated PR objectives, or clarify its intended use if it supports future notification changes.clody-infra/src/main/java/com/clody/infra/models/reply/repository/JpaReplyRepository.java (1)
1-32: AI summary is inconsistent with PR objectives.Similar to the domain repository, the AI-generated summary mentions removing and adding methods related to "weekly diary calendar retrieval" and other features. However, this PR is specifically about fixing duplicate push notifications, not adding new features.
The summary claims that
findFirstByUserIdAndDiaryCreatedDateBetweenAndReplyInfoReplyProcessStatusNotOrderByCreatedAtDescwas removed, but this method is not shown in the visible code (lines 29-30 show similar methods still present). Without seeing the removed code, it's unclear what actually changed.Most importantly, the code changes shown here (adding
findAllByUserId) don't directly implement the push notification deduplication fix described in the PR objectives.Please verify:
- Where the actual Set-based deduplication logic for push notifications is implemented
- Whether the files provided are the correct files for this PR
- How
findAllByUserIdis used in the notification sending flowclody-infra/src/main/java/com/clody/infra/models/alarm/DiaryNotifyService.java (3)
19-19: LGTM! Appropriate imports for the deduplication fix.The
InstantandHashSetimports support the timing measurements and user ID deduplication logic introduced in this fix.Also applies to: 23-23
1-121: AI summary does not match the actual code changes.The AI-generated summary describes features like "weekly diary calendar retrieval endpoint, user personal information update functionality, journal prompt system, and reply content update capability," but the actual changes in this file implement duplicate push notification prevention using Set-based deduplication, timing measurements, and improved logging.
54-58: Correct implementation of deduplication filter.The
.filter(alarm -> sentUserIds.add(alarm.getUser().getId()))pattern correctly ensures each userId is processed only once. TheSet.add()method returnstruewhen the element is successfully added (first occurrence) andfalsefor duplicates, which are then filtered out.clody-domain/src/main/java/com/clody/domain/prompt/service/MonthDayConverter.java (1)
9-28: LGTM! Converter is well-implemented.The
MonthDayConverteris null-safe, uses a shared formatter for performance, and follows JPA AttributeConverter patterns correctly. TheautoApply = truesetting means this converter will apply to allMonthDayfields globally, which appears intentional for consistent date formatting across the domain.clody-app/src/main/java/com/clody/clodyapi/diary/application/port/in/UserUpdateUsecase.java (1)
12-13: LGTM! Method signature is consistent.The new
updateUserPersonalInfomethod follows the same pattern as the existingupdateUserNamemethod, maintaining consistency in the use case interface design.clody-app/src/main/java/com/clody/clodyapi/user/mapper/UserMapper.java (1)
42-44: LGTM! Mapping method follows established patterns.The new
toUserPersonalInfoPatchResponsemethod is consistent with other mapper methods in this class (e.g.,toUserNamePatchResponse), maintaining a clean and uniform mapping layer.clody-domain/src/main/java/com/clody/domain/diary/service/UserCommandService.java (1)
26-27: LGTM: Consistent extension of UserInfo.The update correctly includes gender and birthDate in the UserInfo response, aligning with the extended user domain model.
clody-app/src/main/java/com/clody/clodyapi/diary/application/service/UserDeletionService.java (1)
21-21: LGTM: Consistent with extended UserInfo model.The update correctly includes gender and birthDate when mapping deleted user information.
clody-app/src/main/java/com/clody/clodyapi/user/application/service/CustomOAuth2UserService.java (1)
37-37: LGTM: Appropriate null handling for OAuth users.Passing
nullfor gender and birthDate is correct since Google OAuth doesn't provide these fields. Users can update this information later via the new/user/personal/infoendpoint.clody-domain/src/main/java/com/clody/domain/prompt/dto/response/JournalPromptInfo.java (1)
1-10: LGTM: Clean DTO pattern.The record and factory method follow clean Java patterns for immutable data transfer objects.
clody-infra/src/main/java/com/clody/infra/models/prompt/JournalPromptRepositoryAdapter.java (1)
11-23: LGTM: Standard repository adapter pattern.The implementation follows proper hexagonal architecture with appropriate error handling. The constructor injection and exception throwing for missing prompts are correctly implemented.
clody-app/src/main/java/com/clody/clodyapi/user/adapter/out/dto/UserPersonalInfoPatchResponse.java (1)
1-16: LGTM: Clean response DTO.The record follows proper DTO patterns with appropriate types and a factory method.
clody-app/src/main/java/com/clody/clodyapi/prompt/adapter/out/dto/PromptGetResponse.java (1)
1-10: LGTM!Clean and idiomatic record implementation for the API response DTO. The static factory method provides a clear construction pattern.
clody-domain/src/main/java/com/clody/domain/prompt/dto/PromptDateInfo.java (1)
1-13: LGTM!Well-designed value object using appropriate Java time types. The
offactory method follows standard naming conventions for value types.clody-app/src/main/java/com/clody/clodyapi/prompt/mapper/PromptMapper.java (1)
11-13:MonthDay.of()throwsDateTimeExceptionfor invalid input.If
monthordayOfMonthare invalid (e.g., month 13, or day 31 for February),MonthDay.of()will throw aDateTimeException. Ensure input validation occurs upstream (e.g., via@Validannotations on controller request parameters) so this exception is handled gracefully with an appropriate HTTP 400 response.clody-infra/src/main/java/com/clody/infra/models/prompt/JpaJournalPromptRepository.java (1)
10-13: No action needed. TheMonthDayJPA converter is properly configured.MonthDayConverteris annotated with@Converter(autoApply = true)and theJournalPromptentity applies it via@Convert(converter = MonthDayConverter.class)on themonthDayfield. The repository interface is correctly designed.clody-domain/src/main/java/com/clody/domain/prompt/service/PromptQueryService.java (1)
18-25: No changes needed. ThepromptRepository.findByMonthDay()method inJournalPromptRepositoryAdapteruses.orElseThrow()to throw aBusinessExceptionwhen the prompt is not found, ensuring the method never returns null. The code is safe fromNullPointerException.Likely an incorrect or invalid review comment.
clody-domain/src/main/java/com/clody/domain/user/dto/UserInfo.java (1)
8-18: LGTM!The record extension with
GenderandLocalDate birthDateis clean, and the factory method signature is appropriately updated to include the new fields.clody-app/src/main/java/com/clody/clodyapi/user/adapter/in/web/UserController.java (1)
39-46: LGTM!The new
patchUserPersonalInfoendpoint follows the established pattern frompatchUserName, with consistent Swagger annotations and proper request/response type definitions.clody-app/src/main/java/com/clody/clodyapi/prompt/adapter/in/web/PromptControllerImpl.java (1)
25-36: LGTM!The implementation follows the established pattern used in
ReplyController, properly parsing the timezone header and delegating to the use case layer.clody-domain/src/main/java/com/clody/domain/user/User.java (3)
39-44: Field additions look correct.Using
EnumType.STRINGfor the gender enum is the right choice for maintainability (avoids issues if enum order changes). Nullable columns are appropriate for optional profile data that may be collected after initial registration.
71-74: Consider adding basic validation for birthDate.The mutator works correctly, but there's no guard against invalid data (e.g., future birth dates). If validation is handled at the service/application layer, this is fine. Otherwise, consider adding a simple check.
116-136: Factory method updates look consistent.
createSimpleUsernot setting gender/birthDate (leaving them null) appears intentional for simplified user creation scenarios.ofGoogleUsercorrectly propagates the new fields. Both approaches align with the nullable field design.
| @Override | ||
| public UserPersonalInfoPatchResponse updateUserPersonalInfo(UserPersonalInfoPatchRequest userPersonalInfoPatchRequest) { | ||
| Gender gender = Gender.fromString(userPersonalInfoPatchRequest.gender()); | ||
| UserInfo userInfo = userCommandService.patchUserPersonalInfo(gender, userPersonalInfoPatchRequest.birthDate()); | ||
| return UserMapper.toUserPersonalInfoPatchResponse(userInfo); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find Gender enum and fromString method
find . -name "Gender.java" -o -name "*Gender*" -type f | head -20Repository: Team-Clody/Clody_server
Length of output: 129
🏁 Script executed:
cat -n ./clody-domain/src/main/java/com/clody/domain/user/Gender.javaRepository: Team-Clody/Clody_server
Length of output: 883
Improve Gender.fromString error handling and message clarity.
The method throws SignInException with error type USER_NOT_FOUND for invalid gender values, which is misleading—this exception suggests a user lookup failed, not that an invalid gender string was provided. Additionally, null inputs are implicitly handled by the filter and produce the same misleading error message. Consider:
- Use a more appropriate exception type (e.g.,
BusinessException) for invalid input validation - Use an error message that clearly states the problem (e.g., "Invalid gender value") and lists acceptable values ("male", "female", "none")
- Add explicit null input handling with a dedicated validation error
| @RequestMapping("/api/v1") | ||
| @RestController | ||
| public interface PromptController { | ||
| @Operation(summary = "일자 별 저널 프롬프트 조회 조회 ", description = "QueryString 을 이용해 저널 프롬프트 조회를 합니다.") |
There was a problem hiding this comment.
Typo in API description: duplicate "조회".
The description contains a duplicated word: "일자 별 저널 프롬프트 조회 조회".
Proposed fix
- @Operation(summary = "일자 별 저널 프롬프트 조회 조회 ", description = "QueryString 을 이용해 저널 프롬프트 조회를 합니다.")
+ @Operation(summary = "일자 별 저널 프롬프트 조회", description = "QueryString 을 이용해 저널 프롬프트를 조회합니다.")📝 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.
| @Operation(summary = "일자 별 저널 프롬프트 조회 조회 ", description = "QueryString 을 이용해 저널 프롬프트 조회를 합니다.") | |
| @Operation(summary = "일자 별 저널 프롬프트 조회", description = "QueryString 을 이용해 저널 프롬프트를 조회합니다.") |
🤖 Prompt for AI Agents
In
clody-app/src/main/java/com/clody/clodyapi/prompt/adapter/in/web/PromptController.java
around line 17, the @Operation summary contains a duplicated word "조회 조회";
update the summary to remove the duplicate so it reads e.g. "일자 별 저널 프롬프트 조회"
(and optionally adjust the description text for clarity if needed).
| @RequestParam @Parameter(name = "월", description = "조회할 월", required = true) final int month, | ||
| @RequestParam @Parameter(name = "일", description = "조회할 일", required = true) final int date |
There was a problem hiding this comment.
@parameter name attributes use Korean, which may cause Swagger issues.
The name attribute in @Parameter should match the actual parameter names (month, date) for proper Swagger documentation binding. Using Korean characters ("월", "일") here may cause display or functional issues in Swagger UI.
Proposed fix
- @RequestParam @Parameter(name = "월", description = "조회할 월", required = true) final int month,
- @RequestParam @Parameter(name = "일", description = "조회할 일", required = true) final int date
+ @RequestParam @Parameter(name = "month", description = "조회할 월", required = true) final int month,
+ @RequestParam @Parameter(name = "date", description = "조회할 일", required = true) final int date🤖 Prompt for AI Agents
In
clody-app/src/main/java/com/clody/clodyapi/prompt/adapter/in/web/PromptController.java
around lines 27-28, the @Parameter annotations use Korean strings ("월", "일") for
the name attribute which prevents proper Swagger binding; change the name
attribute to the actual Java parameter names ("month" and "date") (you can keep
the Korean text in the description if desired) so Swagger maps documentation to
the correct query parameters.
| Gender gender = Gender.fromString(request.gender()); | ||
| LocalDate birthDate = request.birthDate(); |
There was a problem hiding this comment.
Missing null-safety for gender and birthDate.
If request.gender() is null, Gender.fromString(null) will fail when comparing with enum values. Similarly, birthDate may be null. Consider handling null cases gracefully, especially if these fields are optional for existing users.
Proposed fix
- Gender gender = Gender.fromString(request.gender());
- LocalDate birthDate = request.birthDate();
+ Gender gender = request.gender() != null ? Gender.fromString(request.gender()) : Gender.NONE;
+ LocalDate birthDate = request.birthDate(); // null is acceptable if optional📝 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.
| Gender gender = Gender.fromString(request.gender()); | |
| LocalDate birthDate = request.birthDate(); | |
| Gender gender = request.gender() != null ? Gender.fromString(request.gender()) : Gender.NONE; | |
| LocalDate birthDate = request.birthDate(); // null is acceptable if optional |
| public int[] getWeeklyCloverCount(LocalDateTime clientNow, ZoneId clientTimeZone) { | ||
| Long userId = jwtUtil.getLoginMemberId(); | ||
| int[] weeklyCloverCountArr = new int[7]; | ||
|
|
||
| LocalDate startOfWeekInClientTime = clientNow | ||
| .atZone(clientTimeZone) | ||
| .toLocalDate() | ||
| .with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)); | ||
|
|
||
| for (int i = 0; i < 7; i++) { | ||
| LocalDate clientDate = startOfWeekInClientTime.plusDays(i); | ||
|
|
||
| Pair<LocalDateTime, LocalDateTime> kstRange = | ||
| ZoneDateTimeConverter.getKstRangeFromClientLocalYearMonthDay( | ||
| clientDate.getYear(), | ||
| clientDate.getMonthValue(), | ||
| clientDate.getDayOfMonth(), | ||
| clientTimeZone | ||
| ); | ||
|
|
||
| int cloverCount = fetchCloverCount(userId, kstRange.getFirst(), kstRange.getSecond()); | ||
| weeklyCloverCountArr[i] = cloverCount; | ||
| } | ||
|
|
||
| return weeklyCloverCountArr; | ||
| } |
There was a problem hiding this comment.
Potential logic inconsistency: fetchCloverCount returns diary count, not clover count.
The getWeeklyCloverCount method delegates to fetchCloverCount which counts diaries. However, the existing getCloverCount() method (lines 451-465) counts valid replies (filtered by is_read, non-deleted, etc.) to determine clover count.
This inconsistency means the weekly clover count represents diary entries per day, while the total clover count represents read replies. If "clover" should consistently mean "read replies," the weekly logic should also query replies rather than diaries.
🔎 Suggested fix if clovers should be based on replies
- private int fetchCloverCount(Long userId, LocalDateTime kstStart, LocalDateTime kstEnd) {
- return diaryRepository.findDiariesByUserIdAndCreatedAtBetween(userId, kstStart, kstEnd).size();
+ private int fetchCloverCount(Long userId, LocalDateTime kstStart, LocalDateTime kstEnd) {
+ List<Reply> replies = replyRepository.findRepliesByUserIdAndDiaryCreatedDateBetween(userId, kstStart, kstEnd);
+ return (int) replies.stream()
+ .filter(Reply::getIs_read)
+ .filter(reply -> reply.getReplyType() != ReplyType.DELETED)
+ .filter(reply -> !reply.getReplyInfo().isDeleted())
+ .count();
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
clody-domain/src/main/java/com/clody/domain/diary/service/DiaryQueryService.java
around lines 340-365, the weekly "clover" loop calls fetchCloverCount which
counts diaries; to be consistent with getCloverCount (lines ~451-465) clovers
must count valid replies (is_read = true, not deleted, reply.member.id = userId,
within KST range). Replace the current diary-count call by querying the Reply
repository using the same predicates used in getCloverCount (or add a new helper
like fetchReadReplyCount(LocalDateTime start, LocalDateTime end, Long userId)
that encapsulates that logic) and return that integer per day; ensure the new
helper matches the filters (isRead, not deleted, member id) and use it in the
for-loop so weeklyCloverCountArr reflects reply-based clover counts.
| public UserInfo getUser() { | ||
| User user = userRepository.findById(jwtUtil.getLoginMemberId()); | ||
| return UserInfo.toUserInfo(user.getEmail(), user.getNickName(), user.getPlatform()); | ||
| return UserInfo.toUserInfo(user.getEmail(), user.getNickName(), user.getPlatform(), user.getGender(), user.getBirthDate()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find User entity and UserInfo class files
find . -type f -name "User.java" | head -5
find . -type f -name "UserInfo.java" | head -5
find . -type f -name "*UserInfo*" | head -10Repository: Team-Clody/Clody_server
Length of output: 750
🏁 Script executed:
#!/bin/bash
# Read User entity file
echo "=== User.java ==="
cat ./clody-domain/src/main/java/com/clody/domain/user/User.java | head -100
echo -e "\n=== UserInfo.java ==="
cat ./clody-domain/src/main/java/com/clody/domain/user/dto/UserInfo.java | head -100Repository: Team-Clody/Clody_server
Length of output: 3140
🏁 Script executed:
#!/bin/bash
# Check the calling code in UserQueryService to see context
cat ./clody-domain/src/main/java/com/clody/domain/diary/service/UserQueryService.javaRepository: Team-Clody/Clody_server
Length of output: 757
Add null safety checks for gender and birthDate fields.
The User entity explicitly declares gender and birthDate with @Column(nullable = true), allowing these fields to be null. The getUser() method passes user.getGender() and user.getBirthDate() directly to UserInfo.toUserInfo() without null checks. While toUserInfo() accepts null values, this can lead to null fields in the returned UserInfo object. Ensure:
- Null values are handled in the method (either by checks before passing, defensive null handling in
toUserInfo(), or documentation that consumers must handle nulls) - Downstream code using these fields includes appropriate null checks
🤖 Prompt for AI Agents
In
clody-domain/src/main/java/com/clody/domain/diary/service/UserQueryService.java
around lines 16-19, the method passes user.getGender() and user.getBirthDate()
directly to UserInfo.toUserInfo() even though those fields are nullable; add
explicit null-safety before calling toUserInfo by replacing the direct calls
with safe values (e.g., map null gender to a default/UNKNOWN enum or an
Optional, and map null birthDate to an Optional or a default/presentational
value) or call a new overload of toUserInfo that accepts Optionals, then update
callers or document that UserInfo may contain nulls so downstream code performs
appropriate null checks; ensure the user object itself is validated (or throw a
controlled exception) before mapping.
| @Getter | ||
| public enum Gender { | ||
| MALE("male"), | ||
| FEMAIE("female"), |
There was a problem hiding this comment.
Typo: FEMAIE should be FEMALE.
The enum constant is misspelled as FEMAIE instead of FEMALE. This will cause fromString("female") to work, but the constant name itself is incorrect and confusing for developers.
Proposed fix
MALE("male"),
- FEMAIE("female"),
+ FEMALE("female"),
NONE("none");📝 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.
| FEMAIE("female"), | |
| MALE("male"), | |
| FEMALE("female"), | |
| NONE("none"); |
🤖 Prompt for AI Agents
In clody-domain/src/main/java/com/clody/domain/user/Gender.java around line 14,
the enum constant is misspelled as FEMAIE; rename it to FEMALE, update any code
references (imports/usages, switch/case, tests, serialization mappings) to use
the new constant name, run/adjust unit tests and recompile; if the enum name is
persisted (DB/JSON), migrate or map the old value as needed to preserve
compatibility.
| public static Gender fromString(String gender) { | ||
| return Arrays.stream(Gender.values()) | ||
| .filter(g -> g.gender.equals(gender)) | ||
| .findFirst() | ||
| .orElseThrow(() -> new SignInException(ErrorType.USER_NOT_FOUND)); | ||
| } |
There was a problem hiding this comment.
Semantically incorrect exception type for invalid gender input.
Throwing SignInException with ErrorType.USER_NOT_FOUND when gender parsing fails is misleading. This error type suggests a missing user, not an invalid input value. Consider using a more appropriate error type like INVALID_REQUEST or introducing INVALID_GENDER.
Proposed fix
public static Gender fromString(String gender) {
return Arrays.stream(Gender.values())
.filter(g -> g.gender.equals(gender))
.findFirst()
- .orElseThrow(() -> new SignInException(ErrorType.USER_NOT_FOUND));
+ .orElseThrow(() -> new BusinessException(ErrorType.INVALID_REQUEST));
}Committable suggestion skipped: line range outside the PR's diff.
| Instant endTime = Instant.now(clock); | ||
| log.info("[Alarm] Diary Alarm Sent. Target Count: {}, Time Taken: {} ms", | ||
| alarmTarget.size(), endTime.toEpochMilli() - startTime.toEpochMilli()); |
There was a problem hiding this comment.
Logging reports incorrect count.
Line 61 logs alarmTarget.size(), which is the total count of all alarms found at the current time (including both diary and draft alarms), not the count of diary alarms actually sent after filtering by isDiaryAlarm(), FCM token validity, and user deduplication.
🔎 Proposed fix to log accurate count
List<Alarm> alarmTarget = alarmRepository.findAllByTime(currentTime);
- alarmTarget.stream()
+ long sentCount = alarmTarget.stream()
.filter(Alarm::isDiaryAlarm)
.filter(alarm -> alarm.getFcmToken() != null && !alarm.getFcmToken().isEmpty())
.filter(alarm -> sentUserIds.add(alarm.getUser().getId())) // Set에 정상 추가 시 발송
- .forEach(this::sendLocalizedAlarm);
+ .peek(this::sendLocalizedAlarm)
+ .count();
Instant endTime = Instant.now(clock);
log.info("[Alarm] Diary Alarm Sent. Target Count: {}, Time Taken: {} ms",
- alarmTarget.size(), endTime.toEpochMilli() - startTime.toEpochMilli());
+ sentCount, endTime.toEpochMilli() - startTime.toEpochMilli());Committable suggestion skipped: line range outside the PR's diff.
| private void sendLocalizedAlarm(Alarm alarm) { | ||
| Long userId = alarm.getUser().getId(); | ||
| // 최신 reply 기준 언어 조회, 기본 값은 한국어 입니다. | ||
| SupportedLanguage supportedLanguage = replyRepository.findLatestLanguageTypeByUserId(userId); | ||
|
|
||
| if (alarm.getFcmToken() != null && !alarm.getFcmToken().isEmpty()) { | ||
| log.info("Sending Diary Alarm to User ID: {}, Language: {}, FCM Token: {}", | ||
| userId, supportedLanguage, alarm.getFcmToken()); | ||
| fcmService.sendDiaryAlarm(supportedLanguage, alarm.getFcmToken(), alarm.getUser().getId()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the DiaryNotifyService.java file and examine the context around lines 56 and 110-120
find . -name "DiaryNotifyService.java" -type fRepository: Team-Clody/Clody_server
Length of output: 147
🏁 Script executed:
#!/bin/bash
# Search for FcmService and its sendDiaryAlarm method
find . -name "FcmService.java" -type fRepository: Team-Clody/Clody_server
Length of output: 139
🏁 Script executed:
#!/bin/bash
# Search for ReplyRepository to understand findLatestLanguageTypeByUserId
find . -name "ReplyRepository.java" -type fRepository: Team-Clody/Clody_server
Length of output: 150
🏁 Script executed:
#!/bin/bash
# Read DiaryNotifyService.java to understand the context and filter at line 56
cat -n ./clody-infra/src/main/java/com/clody/infra/models/alarm/DiaryNotifyService.javaRepository: Team-Clody/Clody_server
Length of output: 6537
🏁 Script executed:
#!/bin/bash
# Read FcmService.java to check null handling in sendDiaryAlarm
cat -n ./clody-infra/src/main/java/com/clody/infra/external/fcm/FcmService.javaRepository: Team-Clody/Clody_server
Length of output: 6146
🏁 Script executed:
#!/bin/bash
# Read ReplyRepository.java to understand the return type
cat -n ./clody-domain/src/main/java/com/clody/domain/reply/repository/ReplyRepository.javaRepository: Team-Clody/Clody_server
Length of output: 1393
🏁 Script executed:
#!/bin/bash
# Find MessageContent class to check selectDiaryRemindAlarmLanguage implementation
find . -name "MessageContent.java" -type fRepository: Team-Clody/Clody_server
Length of output: 143
🏁 Script executed:
#!/bin/bash
# Also search for selectDiaryRemindAlarmLanguage in the codebase
rg "selectDiaryRemindAlarmLanguage" -A 10Repository: Team-Clody/Clody_server
Length of output: 3533
🏁 Script executed:
#!/bin/bash
# Read the full MessageContent.java to understand the switch statement and null handling
cat -n ./clody-infra/src/main/java/com/clody/infra/external/fcm/MessageContent.javaRepository: Team-Clody/Clody_server
Length of output: 1914
Fix null language handling and remove redundant FCM token check.
Line 113's replyRepository.findLatestLanguageTypeByUserId(userId) may return null if no reply exists for the user. This null value passed to selectDiaryRemindAlarmLanguage() will cause a NullPointerException since the switch statement has no null guard. Implement a null default matching the pattern used in sendDraftAlarms() (line 90-91): SupportedLanguage supportedLanguage = replyRepository.findLatestLanguageTypeByUserId(userId); if (supportedLanguage == null) supportedLanguage = SupportedLanguage.KO;
Additionally, the FCM token check at line 115 is redundant—the stream filter at line 56 already guarantees non-null and non-empty tokens before sendLocalizedAlarm is invoked. Remove the condition and execute the log and FCM call unconditionally.
🤖 Prompt for AI Agents
In
clody-infra/src/main/java/com/clody/infra/models/alarm/DiaryNotifyService.java
around lines 110-120, replyRepository.findLatestLanguageTypeByUserId(userId) can
return null and the current code may pass null into
selectDiaryRemindAlarmLanguage(), causing NPE; ensure you default null to
SupportedLanguage.KO (same pattern as sendDraftAlarms: if (supportedLanguage ==
null) supportedLanguage = SupportedLanguage.KO). Also remove the redundant FCM
token null/empty check here (the caller’s stream already guarantees
non-null/non-empty tokens) so always perform the log and call
fcmService.sendDiaryAlarm(...) unconditionally using the resolved
supportedLanguage and alarm.getFcmToken().
|
해당 PR은 현재 작업 중인 스프린트 기능이 모두 포함되어있어서 prod 브랜치로 머지하면 안 될 것 같습니다. 푸시 알림 관련 커밋만 따로 운영서버로 배포하면 좋을 것 같습니다. |
|
넵 ~ close 하겠습니다 @rla124 |
📣 Related Issue
📝 Summary
푸시 알림이 일부 유저에게 중복되어 발송되는 문제를 해결합니다.
🙏 Question & PR point
남은 작업
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.