Feat: [FN-144][FN-160][FN-161] 즐겨찾기 기능#43
Merged
Merged
Conversation
Walkthrough그룹 탈퇴 이벤트(GroupLeftEvent) 도입과 함께, 트랜잭션 커밋 이후 비동기/재시도 리스너가 사용자별 비공개 카드세트 북마크를 일괄 제거하도록 추가되었고, 북마크·카드세트 관련 API들이 사용자 가시성(viewable) 기반으로 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as Application
participant TX as Transaction Manager
participant Pub as Event Publisher
participant L as GroupLeftCleanupBookmarkListener
participant BS as BookmarkService
participant CS as CardSetService
participant BR as BookmarkRepository
participant CR as CardSetRepository
User->>App: 그룹 탈퇴 요청 (groupId, userId)
App->>TX: 트랜잭션 커밋
TX-->>Pub: AFTER_COMMIT 이벤트 발행
Pub-->>L: GroupLeftEvent(groupId, userId)
note over L: @Async + @Retryable(maxAttempts=3, backoff=2s, multiplier=2)
L->>BS: removePrivateCardSetBookmarks(groupId, userId)
BS->>CS: findPrivateCardSetIds(groupId)
CS->>CR: findPrivateIdsByGroupId(groupId)
CR-->>CS: Set<Long> privateIds
alt privateIds 존재
BS->>BR: deleteByTargetTypeAndUserIdAndTargetIdIn(CARD_SET, userId, ids)
BR-->>BS: deletedCount
else 없음
note over BS: 삭제할 항목 없음(무시)
end
note over L,BS: 모든 재시도 실패 시 recover()에서 오류 로그
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (1 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
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: 7
🧹 Nitpick comments (8)
src/main/java/project/flipnote/cardset/repository/CardSetRepository.java (1)
4-4: Set 사용 OK, 다만 호출부에서 순서 의존 금지반환 컬렉션이 Set이면 순서가 보장되지 않습니다. 정렬이 필요하면 List 반환과 정렬 기준을 명시하세요.
src/main/java/project/flipnote/bookmark/repository/BookmarkRepository.java (1)
4-4: Collection으로 변경 시 import 업데이트 필요아래처럼 교체하세요.
-import java.util.Set; +import java.util.Collection;src/main/java/project/flipnote/bookmark/service/fetcher/BookmarkCardSetFetcher.java (1)
32-36: 빈 ID 집합 단축 반환으로 불필요한 DB 호출 제거
targetIds가 비어있을 때 서비스 호출을 생략하면 낭비를 줄일 수 있습니다.- public Map<Long, CardSetBookmarkResponse> fetchByIds(Set<Long> targetIds, Long userId) { - return cardSetService.findViewableCardSetsByIds(targetIds, userId).stream() + public Map<Long, CardSetBookmarkResponse> fetchByIds(Set<Long> targetIds, Long userId) { + if (targetIds == null || targetIds.isEmpty()) { + return java.util.Collections.emptyMap(); + } + return cardSetService.findViewableCardSetsByIds(targetIds, userId).stream() .map(CardSetBookmarkResponse::from) .collect(Collectors.toMap(CardSetBookmarkResponse::getId, Function.identity())); }src/main/java/project/flipnote/bookmark/service/BookmarkTargetFetchService.java (1)
41-47: 빈 집합 단축 반환으로 미미하지만 확실한 최적화호출 빈도가 높은 경로라면 의미 있는 미세 최적화입니다.
- public Map<Long, T> fetchByTypeAndIds( + public Map<Long, T> fetchByTypeAndIds( BookmarkTargetType targetType, Set<Long> targetIds, Long userId ) { BookmarkTargetFetcher<T> targetFetcher = getFetcher(targetType); - - return targetFetcher.fetchByIds(targetIds, userId); + if (targetIds == null || targetIds.isEmpty()) { + return java.util.Collections.emptyMap(); + } + return targetFetcher.fetchByIds(targetIds, userId); }src/main/java/project/flipnote/bookmark/listener/GroupLeftCleanupBookmarkListener.java (2)
23-27: 재시도 예외 범위를 한정하세요 (불필요한 영구 실패 재시도 방지).현재
@Retryable은 모든Exception을 재시도 대상으로 간주합니다. 네트워크/DB의 일시적 오류만 재시도하도록 예외를 한정하면 안정성과 관찰성이 좋아집니다.아래처럼 transient 계열만 포함하도록 제안합니다.
@@ +import org.springframework.dao.CannotAcquireLockException; +import org.springframework.dao.CannotSerializeTransactionException; +import org.springframework.dao.ConcurrencyFailureException; +import org.springframework.dao.QueryTimeoutException; +import org.springframework.transaction.CannotCreateTransactionException; @@ - @Retryable( - maxAttempts = 3, - backoff = @Backoff(delay = 2000, multiplier = 2) - ) + @Retryable( + include = { + ConcurrencyFailureException.class, + CannotAcquireLockException.class, + CannotSerializeTransactionException.class, + CannotCreateTransactionException.class, + QueryTimeoutException.class + }, + maxAttempts = 3, + backoff = @Backoff(delay = 2000, multiplier = 2) + )
34-37: Recover에서 후속 조치(경보/재처리 경로)를 추가하면 좋습니다.로그 외에 알림(예: PagerDuty/Slack) 또는 DLQ/아웃박스 적재로 사후 처리 가능성을 확보해 주세요. 실패 누적 카운터(metric)도 함께 기록하면 운영에 유용합니다.
src/main/java/project/flipnote/cardset/service/CardSetService.java (2)
231-235: 없는 카드셋인 경우 false 반환 의미를 명확히 해주세요.존재하지 않는 카드셋에도
false를 돌려 404와 동일시하지 않습니다. 호출부가 존재 유무와 접근 권한을 구분해야 한다면 Javadoc에 명시하거나 메서드명을existsAndViewable등으로 더 분명히 하는 것을 고려해 주세요.
246-252: N+1 가능성: 정책 검사 반복 호출을 저장소 계층으로 내려 최적화 제안.
findAllById(...).stream().filter(cardSetPolicyService.isCardSetViewable(...))는 카드셋마다 추가 쿼리가 발생할 수 있습니다. 규칙이 허용한다면 “뷰어블” 조건을 리포지토리 쿼리로 푸시다운하여 한 번에 필터링하거나, 호출 전 필요한 멤버십/권한 정보를 일괄 프리패치하는 방식으로 N+1을 제거해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/project/flipnote/bookmark/listener/GroupLeftCleanupBookmarkListener.java(1 hunks)src/main/java/project/flipnote/bookmark/repository/BookmarkRepository.java(2 hunks)src/main/java/project/flipnote/bookmark/service/BookmarkPolicyService.java(1 hunks)src/main/java/project/flipnote/bookmark/service/BookmarkService.java(5 hunks)src/main/java/project/flipnote/bookmark/service/BookmarkTargetFetchService.java(1 hunks)src/main/java/project/flipnote/bookmark/service/fetcher/BookmarkCardSetFetcher.java(1 hunks)src/main/java/project/flipnote/bookmark/service/fetcher/BookmarkTargetFetcher.java(1 hunks)src/main/java/project/flipnote/cardset/repository/CardSetRepository.java(2 hunks)src/main/java/project/flipnote/cardset/service/CardSetPolicyService.java(1 hunks)src/main/java/project/flipnote/cardset/service/CardSetService.java(2 hunks)src/main/java/project/flipnote/common/model/event/GroupLeftEvent.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/main/java/project/flipnote/common/model/event/GroupLeftEvent.java (4)
src/main/java/project/flipnote/common/model/event/GroupInvitationCreatedEvent.java (1)
GroupInvitationCreatedEvent(3-7)src/main/java/project/flipnote/common/model/event/GroupJoinRequestedEvent.java (1)
GroupJoinRequestedEvent(5-10)src/main/java/project/flipnote/common/model/event/UserRegisteredEvent.java (1)
UserRegisteredEvent(3-7)src/main/java/project/flipnote/common/model/event/UserWithdrawnEvent.java (1)
UserWithdrawnEvent(3-6)
src/main/java/project/flipnote/cardset/service/CardSetPolicyService.java (1)
src/main/java/project/flipnote/cardset/entity/CardSet.java (1)
Getter(22-69)
src/main/java/project/flipnote/cardset/repository/CardSetRepository.java (1)
src/main/java/project/flipnote/cardset/entity/CardSet.java (1)
Getter(22-69)
src/main/java/project/flipnote/bookmark/repository/BookmarkRepository.java (2)
src/main/java/project/flipnote/bookmark/entity/Bookmark.java (1)
Getter(19-59)src/main/java/project/flipnote/like/repository/LikeRepository.java (1)
LikeRepository(12-18)
src/main/java/project/flipnote/cardset/service/CardSetService.java (1)
src/main/java/project/flipnote/cardset/entity/CardSet.java (1)
Getter(22-69)
src/main/java/project/flipnote/bookmark/listener/GroupLeftCleanupBookmarkListener.java (2)
src/main/java/project/flipnote/bookmark/service/BookmarkService.java (1)
RequiredArgsConstructor(26-130)src/main/java/project/flipnote/notification/listener/GroupJoinRequestedEventListener.java (2)
Slf4j(16-40)Async(23-31)
src/main/java/project/flipnote/bookmark/service/fetcher/BookmarkCardSetFetcher.java (1)
src/main/java/project/flipnote/like/service/fetcher/LikeCardSetFetcher.java (1)
RequiredArgsConstructor(16-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (10)
src/main/java/project/flipnote/common/model/event/GroupLeftEvent.java (1)
3-7: LGTM — 이벤트 페이로드 설계 적절groupId, userId만 담은 불변 record로 충분합니다. 이후 리스너에서의 처리와도 잘 맞습니다.
src/main/java/project/flipnote/bookmark/service/BookmarkPolicyService.java (1)
19-21: 검증 완료 — BookmarkPolicyService의 모든 호출부가validateTargetViewable(.., userId)로 업데이트되었습니다.src/main/java/project/flipnote/cardset/service/CardSetPolicyService.java (1)
54-58: 조회 권한 검증 위임 깔끔합니다예외 코드 유지하면서
isCardSetViewable로 위임한 방향성 좋습니다.src/main/java/project/flipnote/bookmark/service/fetcher/BookmarkCardSetFetcher.java (1)
27-29: 뷰어빌리티 기반 체크로 전환 적절합니다즐겨찾기 대상의 단순 존재 여부 대신 사용자별 가시성으로 검증한 점 좋습니다.
src/main/java/project/flipnote/bookmark/service/BookmarkTargetFetchService.java (1)
33-37: API를 사용자 컨텍스트 인지형으로 확장한 점 👍
isTargetViewable에userId를 도입해 정책 일관성이 좋아졌습니다.src/main/java/project/flipnote/bookmark/service/BookmarkService.java (4)
21-21: CardSetService 주입으로 도메인 간 결합 명확화그룹 탈퇴 후 비공개 카드셋 정리를 위해 필요한 의존성 추가가 타당합니다.
Also applies to: 34-34
48-48: 대상 검증을 '존재' → '가시성'으로 변경한 점 동의사용자별 접근 정책과 즐겨찾기 UX가 일치합니다.
105-105: 목록 조회 시 사용자 컨텍스트 반영 OK
fetchByTypeAndIds(..., userId)로 비가시 대상이 자연스럽게 걸러집니다.
104-106: Null 대상 응답 허용 여부 확인 요청가시성 필터링으로
targetMap에 없는 항목은BookmarkResponse의target이 null이 됩니다. API 계약이 이를 허용하는지, 아니면 해당 북마크를 응답에서 제외해야 하는지 확인이 필요합니다.src/main/java/project/flipnote/cardset/service/CardSetService.java (1)
146-146: 시그니처 변경 반영 OK.
validateCardSetViewable(cardSet, userId)로의 전환이 일관됩니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/project/flipnote/common/config/RetryConfig.java (1)
6-8: 환경별 토글 가능하도록 속성 기반 활성화 제안테스트나 로컬에서 필요 시 Retry를 끌 수 있도록 속성 조건을 두는 방안을 제안합니다. 운영 기본 ON, 필요 시
flipnote.retry.enabled=false로 비활성화.package project.flipnote.common.config; import org.springframework.context.annotation.Configuration; import org.springframework.retry.annotation.EnableRetry; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @EnableRetry @Configuration +@ConditionalOnProperty(name = "flipnote.retry.enabled", havingValue = "true", matchIfMissing = true) public class RetryConfig { }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/project/flipnote/bookmark/service/BookmarkService.java(5 hunks)src/main/java/project/flipnote/cardset/repository/CardSetRepository.java(2 hunks)src/main/java/project/flipnote/cardset/service/CardSetPolicyService.java(1 hunks)src/main/java/project/flipnote/cardset/service/CardSetService.java(2 hunks)src/main/java/project/flipnote/common/config/RetryConfig.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/project/flipnote/cardset/repository/CardSetRepository.java
- src/main/java/project/flipnote/bookmark/service/BookmarkService.java
- src/main/java/project/flipnote/cardset/service/CardSetPolicyService.java
- src/main/java/project/flipnote/cardset/service/CardSetService.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/project/flipnote/common/config/RetryConfig.java (4)
src/main/java/project/flipnote/common/config/SchedulerConfig.java (1)
EnableScheduling(6-9)src/main/java/project/flipnote/common/config/AuditingConfig.java (1)
AuditingConfig(6-10)src/main/java/project/flipnote/common/config/AsyncConfig.java (1)
AsyncConfig(12-29)src/main/java/project/flipnote/common/config/ShedLockConfig.java (1)
EnableSchedulerLock(11-19)
🔇 Additional comments (3)
src/main/java/project/flipnote/common/config/RetryConfig.java (3)
6-9: 전역 Retry 활성화 구성 적절@Retryable/@recover 기반 리스너가 동작하도록 하는 최소 설정으로 충분합니다. 별도 이슈 없어 보입니다.
3-9: AOP/Retry 의존성 확인 완료
build.gradle에spring-boot-starter-aop및spring-retry의존성이 모두 포함되어 있습니다.
6-8: Async와 Retryable 조합 동작 검증
GroupLeftCleanupBookmarkListener에서 @async와 @retryable(maxAttempts=3, backoff=@backoff(delay=2000, multiplier=2))가 결합될 때 실제 비즈니스 로직 실행에 재시도가 적용되는지, 재시도 횟수 및 백오프가 의도대로 로그에 기록되는지 확인해 주세요.
stoneTiger0912
approved these changes
Sep 15, 2025
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.
📝 변경 내용
✅ 체크리스트
💬 기타 참고 사항
Summary by CodeRabbit