Feat: [FN-151] 카드셋 목록 조회시 즐겨찾기 수 정렬 기능#47
Conversation
|
Caution Review failedThe pull request is closed. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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: 1
🧹 Nitpick comments (19)
src/main/java/project/flipnote/cardset/entity/CardSetMetadata.java (1)
24-26: bookmarkCount 추가 자체는 좋습니다. 음수 방지·마이그레이션 고려 부탁드립니다.
- 운영 데이터에 기존 행이 있다면 DB 마이그레이션(기본값 0, NOT NULL)과 인덱스(bookmark_count, id)를 함께 적용해 주세요.
- 애플리케이션 단에서도 음수 방지를 명시하면 안전합니다(@min(0)). 아래처럼 보강을 제안합니다.
+import jakarta.validation.constraints.Min; @@ - @Column(nullable = false) - private int bookmarkCount; + @Column(nullable = false) + @Min(0) + private int bookmarkCount;src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustomImpl.java (3)
49-56: 정렬 속성 파싱을 대소문자 무시로 개선하세요.클라이언트가
bookmark,BOOKMARK등 혼용할 수 있어 실패 시 불필요한 폴백(ID 정렬)로 흐를 수 있습니다.+import java.util.Locale; @@ - sortField = CardSetSortField.valueOf(order.getProperty()); + sortField = CardSetSortField.valueOf(order.getProperty().toUpperCase(Locale.ROOT));
49-57: 알 수 없는 정렬 속성 처리 시 ID 정렬로 ‘같은 방향’을 유지하는 현재 동작은 혼란을 줄 수 있습니다.경고 로그 후 해당 정렬은 건너뛰고, 루프 이후의 기본 정렬(id.desc)만 적용하는 편이 예측 가능합니다.
- } catch (IllegalArgumentException iae) { - log.warn( - "Unknown sort property: {}. Valid values are {}", - order.getProperty(), Arrays.toString(CardSetSortField.values()), iae - ); - } + } catch (IllegalArgumentException iae) { + log.warn( + "Unknown sort property: {}. Valid values are {}", + order.getProperty(), Arrays.toString(CardSetSortField.values()) + ); + continue; + }
61-63: 정렬 성능 고려: 인덱스 권장BOOKMARK 정렬 사용 빈도가 높다면
(bookmark_count DESC, id DESC)인덱스를card_set_metadata에 추가해 주세요. LIKE 정렬도 동일 패턴을 권장합니다.src/main/java/project/flipnote/common/model/event/BulkBookmarkRemovedEvent.java (1)
5-10: 중복 ID 제거 목적이라면 Set 사용을 검토해 주세요.동일 카드셋 ID가 중복 전달될 수 있다면, 이벤트 레벨에서
Set<Long>으로 보장하는 편이 안전합니다(핸들러/서비스 영향 범위 확인 필요).-import java.util.List; +import java.util.List; +import java.util.Set; @@ - List<Long> targetIds, + Set<Long> targetIds,src/main/java/project/flipnote/bookmark/entity/BookmarkTargetType.java (1)
10-20: name() 기반 매핑은 암묵적 의존이라 취약합니다 — 명시적 switch 매핑으로 컴파일 타임 안전성 확보 권장새 상수가 추가/이름 변경될 때 런타임 실패 대신 컴파일 타임 미완료 경고를 얻을 수 있습니다.
- public BookmarkEventTargetType toEventType() { - try { - return BookmarkEventTargetType.valueOf(this.name()); - } catch (IllegalArgumentException e) { - log.error("Failed to map BookmarkTargetType '{}' to BookmarkEventTargetType", this.name(), e); - throw new IllegalStateException( - "Invalid mapping from BookmarkTargetType to BookmarkEventTargetType: " + this.name(), - e - ); - } - } + public BookmarkEventTargetType toEventType() { + return switch (this) { + case CARD_SET -> BookmarkEventTargetType.CARD_SET; + }; + }src/main/java/project/flipnote/cardset/listener/CardSetBookmarkAddedEventHandler.java (3)
27-30: 재시도 대상 예외 범위 축소 권장
DataAccessException전반은 영구 오류도 포함합니다. 교착·락획득 실패·타임아웃 등 일시 오류로 범위를 좁혀 불필요한 재시도를 줄이는 편이 안전합니다.적용 예시(diff):
- @Retryable( - maxAttempts = 3, - retryFor = DataAccessException.class, - backoff = @Backoff(delay = 2000, multiplier = 2) - ) + @Retryable( + maxAttempts = 3, + retryFor = { + org.springframework.dao.CannotAcquireLockException.class, + org.springframework.dao.DeadlockLoserDataAccessException.class, + org.springframework.dao.QueryTimeoutException.class + }, + backoff = @Backoff(delay = 2000, multiplier = 2) + )
33-38: 중복 이벤트 대비(idempotency) 고민이벤트가 드물게 중복 발행/처리될 경우 카운트가 과증가할 수 있습니다. Outbox+소비 idempotency 키(예: bookmark PK) 도입 또는 중복 방지 로직/쿼리(UPSERT 계열) 고려를 권장합니다.
40-46: 로그 필드 확장 제안현재 컨텍스트 로그는 충분하지만, 추적성 향상을 위해 traceId/MDC 또는 이벤트 식별자(있다면)를 함께 남기면 운영 분석이 쉬워집니다.
src/main/java/project/flipnote/cardset/service/CardSetService.java (4)
226-227: Javadoc 파라미터 정렬 니트
@param userId들여쓰기만 맞춰두면 문서 일관성이 좋아집니다.
240-241: Javadoc 파라미터 정렬 니트동일하게 들여쓰기 정리 권장.
281-284: 즐겨찾기 수 음수 방지 가드 필요이벤트 역전/중복 등으로 음수가 될 여지가 있습니다. DB 레벨에서
GREATEST(count-1,0)혹은 CHECK 제약(0 이상)으로 방지하거나, 저장소 쿼리를 안전 감소 쿼리로 교체하는 것을 권장합니다.
292-295: 벌크 감소 배치 처리 권장대량 ID 입력 시 IN 절 한도/바인딩 부담이 있을 수 있습니다. 500~1000 단위로 배치 분할 호출을 권장합니다.
적용 예시:
- public void decrementBookmarkCount(List<Long> cardSetIds) { - cardSetMetadataRepository.decrementBookmarkCount(cardSetIds); - } + public void decrementBookmarkCount(List<Long> cardSetIds) { + final int batchSize = 1000; + for (int i = 0; i < cardSetIds.size(); i += batchSize) { + List<Long> batch = cardSetIds.subList(i, Math.min(i + batchSize, cardSetIds.size())); + cardSetMetadataRepository.decrementBookmarkCount(batch); + } + }src/main/java/project/flipnote/cardset/listener/BulkCardSetBookmarkRemovedEventHandler.java (2)
37-38: 대량 ID 처리 시 배치 분할 권장
event.targetIds()가 큰 경우 단일 IN 업데이트는 비효율/한도 초과 위험이 있습니다. 서비스 메서드에서 배치 분할(예: 1000개 단위) 적용을 권장합니다.
40-46: 로그 폭주 방지
targetIds전체를 로그로 남기면 라인이 과도하게 길어질 수 있습니다.size만 남기거나 첫 N개만 샘플링 출력하는 방식을 권장합니다.src/main/java/project/flipnote/bookmark/service/BookmarkService.java (2)
68-69: 추가 이벤트 발행 위치 적절 + 신뢰성 강화 제안트랜잭션 내부 발행 + 리스너 AFTER_COMMIT 조합은 적절합니다. 다만 스프링 이벤트는 비내구적이므로 장애시 유실 위험이 있습니다. 트랜잭셔널 아웃박스/메시지 브로커(예: Kafka)로의 전환을 검토해 주세요.
89-90: 삭제 이벤트 발행 위치 적절동일 의견(아웃박스/브로커) 적용 권장.
src/main/java/project/flipnote/cardset/listener/CardSetBookmarkRemovedEventHandler.java (2)
37-38: 업데이트 결과 관측성 보강 제안메타데이터 행이 존재하지 않는 경우 0 row 업데이트가 될 수 있습니다. 서비스에서 영향 행 수를 확인해 0인 경우
WARN로그 남기면 운영 관측성에 도움이 됩니다. (서명 변경이 부담되면 메트릭 증가만이라도 고려)예시:
-@Transactional -public void decrementBookmarkCount(Long cardSetId) { - cardSetMetadataRepository.decrementBookmarkCount(cardSetId); -} +@Transactional +public void decrementBookmarkCount(Long cardSetId) { + int updated = cardSetMetadataRepository.decrementBookmarkCount(cardSetId); + if (updated == 0) { + log.warn("CardSetMetadata not found for decrement: cardSetId={}", cardSetId); + } +}
40-46: 에러 로그 메시지: 컨텍스트 확장 제안현재 로그는 충분하지만, 운영 추적성을 위해
traceId/eventId(있다면 MDC) 추가를 권장합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/main/java/project/flipnote/bookmark/entity/BookmarkTargetType.java(1 hunks)src/main/java/project/flipnote/bookmark/repository/BookmarkRepository.java(2 hunks)src/main/java/project/flipnote/bookmark/service/BookmarkService.java(6 hunks)src/main/java/project/flipnote/cardset/entity/CardSetMetadata.java(1 hunks)src/main/java/project/flipnote/cardset/listener/BulkCardSetBookmarkRemovedEventHandler.java(1 hunks)src/main/java/project/flipnote/cardset/listener/CardSetBookmarkAddedEventHandler.java(1 hunks)src/main/java/project/flipnote/cardset/listener/CardSetBookmarkRemovedEventHandler.java(1 hunks)src/main/java/project/flipnote/cardset/model/CardSetSortField.java(1 hunks)src/main/java/project/flipnote/cardset/repository/CardSetMetadataRepository.java(2 hunks)src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustomImpl.java(1 hunks)src/main/java/project/flipnote/cardset/service/CardSetService.java(4 hunks)src/main/java/project/flipnote/common/model/event/BookmarkAddedEvent.java(1 hunks)src/main/java/project/flipnote/common/model/event/BookmarkEventTargetType.java(1 hunks)src/main/java/project/flipnote/common/model/event/BookmarkRemovedEvent.java(1 hunks)src/main/java/project/flipnote/common/model/event/BulkBookmarkRemovedEvent.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/main/java/project/flipnote/cardset/listener/CardSetBookmarkAddedEventHandler.java (3)
src/main/java/project/flipnote/bookmark/service/BookmarkService.java (1)
RequiredArgsConstructor(31-155)src/main/java/project/flipnote/cardset/listener/CardSetBookmarkRemovedEventHandler.java (1)
Slf4j(18-47)src/main/java/project/flipnote/cardset/service/CardSetService.java (1)
Slf4j(39-296)
src/main/java/project/flipnote/cardset/listener/BulkCardSetBookmarkRemovedEventHandler.java (3)
src/main/java/project/flipnote/bookmark/service/BookmarkService.java (1)
RequiredArgsConstructor(31-155)src/main/java/project/flipnote/cardset/listener/CardSetBookmarkRemovedEventHandler.java (1)
Slf4j(18-47)src/main/java/project/flipnote/cardset/service/CardSetService.java (1)
Slf4j(39-296)
src/main/java/project/flipnote/bookmark/entity/BookmarkTargetType.java (3)
src/main/java/project/flipnote/cardset/listener/BulkCardSetBookmarkRemovedEventHandler.java (1)
Slf4j(18-47)src/main/java/project/flipnote/cardset/listener/CardSetBookmarkAddedEventHandler.java (1)
Slf4j(18-47)src/main/java/project/flipnote/cardset/listener/CardSetBookmarkRemovedEventHandler.java (1)
Slf4j(18-47)
src/main/java/project/flipnote/cardset/listener/CardSetBookmarkRemovedEventHandler.java (4)
src/main/java/project/flipnote/bookmark/service/BookmarkService.java (1)
RequiredArgsConstructor(31-155)src/main/java/project/flipnote/cardset/listener/BulkCardSetBookmarkRemovedEventHandler.java (1)
Slf4j(18-47)src/main/java/project/flipnote/cardset/listener/CardSetBookmarkAddedEventHandler.java (1)
Slf4j(18-47)src/main/java/project/flipnote/cardset/service/CardSetService.java (1)
Slf4j(39-296)
⏰ 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 (16)
src/main/java/project/flipnote/cardset/entity/CardSetMetadata.java (1)
30-31: 초기값 0 설정 OK.엔티티 생성 시 기본값을 명확히 해 가독성이 좋습니다.
src/main/java/project/flipnote/cardset/model/CardSetSortField.java (1)
8-8: BOOKMARK 정렬 필드 추가 LGTM.퍼블릭 API 확장에 문제 없습니다.
src/main/java/project/flipnote/common/model/event/BookmarkEventTargetType.java (1)
3-5: 이벤트 타깃 타입 enum 추가 LGTM.현 시점 요구사항에 적합합니다.
src/main/java/project/flipnote/common/model/event/BookmarkRemovedEvent.java (1)
3-8: BookmarkRemovedEvent 레코드 정의 LGTM.필수 필드 구성이 명확합니다.
src/main/java/project/flipnote/common/model/event/BookmarkAddedEvent.java (1)
3-8: BookmarkAddedEvent 레코드 정의 LGTM.핸들러에서 필요한 정보가 최소 단위로 잘 담겨 있습니다.
src/main/java/project/flipnote/cardset/listener/BulkCardSetBookmarkRemovedEventHandler.java (1)
25-31: 비동기 재시도 설정 점검(벌크 삭제 처리)단건 핸들러와 동일하게 설정 유효성(
@EnableAsync,@EnableRetry, 전용 Executor) 확인을 권장합니다.동일 스크립트로 점검 가능합니다(위 코멘트 참조).
src/main/java/project/flipnote/bookmark/service/BookmarkService.java (2)
40-41: 이벤트 퍼블리셔 주입 적절필드 추가로 이벤트 주도 흐름 연결이 명확해졌습니다.
141-154: 비공개 카드셋 벌크 제거 흐름 타당 — 매핑 검증·배치 권고
- 흐름(조회 → 삭제 → 이벤트 발행)은 적절합니다.
- 대용량 대상에 대해 삭제/이벤트를 배치(예: 500–1,000건 단위)로 분할해 트랜잭션·메모리·이벤트 처리 부담을 줄이세요.
- src/main/java/project/flipnote/bookmark/entity/BookmarkTargetType.java의 toEventType()는 BookmarkEventTargetType.valueOf(this.name())를 호출하고 매핑 실패 시 로그 후 IllegalStateException을 던집니다. 모든 도메인 타입이 BookmarkEventTargetType에 존재하는지 검증하고, 누락 시 안전한 처리(기본값 또는 명확한 에러) 및 단위 테스트 추가를 권장합니다.
- BulkBookmarkRemovedEvent 및 관련 단위 테스트가 저장소 검색에서 명확히 확인되지 않았으니 이벤트 클래스·테스트 존재 여부를 확인하세요.
src/main/java/project/flipnote/bookmark/repository/BookmarkRepository.java (1)
21-23: 삭제→조회 전환 확인 — deleteBy 호출 없음rg 결과 deleteByTargetTypeAndUserIdAndTargetIdIn 호출 없음. findAllByTargetTypeAndUserIdAndTargetIdIn은 정의(src/main/java/project/flipnote/bookmark/repository/BookmarkRepository.java:21) 및 호출(src/main/java/project/flipnote/bookmark/service/BookmarkService.java:143)에만 존재합니다.
src/main/java/project/flipnote/cardset/listener/CardSetBookmarkAddedEventHandler.java (1)
25-31: @async + @retryable + AFTER_COMMIT 동작 보증 확인 필요 — Enable 확인됨, Executor 분리 확인 요청
- 검증 결과: src/main/java/project/flipnote/common/config/AsyncConfig.java에 @EnableAsync, src/main/java/project/flipnote/common/config/RetryConfig.java에 @EnableRetry가 존재합니다.
- 미확인: 명시적 ThreadPoolTaskExecutor/TaskExecutor 빈이 검색되지 않았습니다. @async가 기본 executor로 실행되면 @retryable이 예상대로 동작하지 않을 수 있으니, 별도 Executor(bean) 사용 여부(또는 AsyncConfigurer 구현)와 AOP 프록시 순서(proxyTargetClass/@order 등)를 확인해 주세요.
src/main/java/project/flipnote/cardset/service/CardSetService.java (2)
264-273: 증가 메서드 구현 적절 — 승인; 마이그레이션·운영데이터 검증 필요
- 서비스 경계·트랜잭션은 적절합니다.
- 리포지토리/엔티티에 bookmarkCount 필드(@column(nullable = false) 및 생성자 기본값 0)와 증가/감소 쿼리가 존재함(파일: src/main/java/project/flipnote/cardset/entity/CardSetMetadata.java, src/main/java/project/flipnote/cardset/repository/CardSetMetadataRepository.java, src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustomImpl.java). 다만 저장소에서 Flyway/Liquibase 또는 src/main/resources/db/migration 같은 마이그레이션 스크립트는 발견되지 않았습니다. 운영 DB에 컬럼 추가(기본값 0, NOT NULL) 적용 여부와 기존 메타데이터 누락(메타 행 미생성) 여부를 확인하고, 미적용/누락 시 마이그레이션 및 백필을 적용하세요.
124-126: 정렬(북마크 수) 지원 확인 — 조치 불필요CardSetRepositoryCustomImpl에서 CardSetSortField.BOOKMARK가 cardSetMetadata.bookmarkCount로 매핑되어(useMetadata = true) 메타데이터 기반 정렬 로직이 구현되어 있으며, CardSetSearchRequest#getPageRequest가 Sort 정보를 전달하므로 북마크 수 기준 정렬이 지원됩니다. 확인 위치: src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustomImpl.java (orders.add(...bookmarkCount...)), src/main/java/project/flipnote/cardset/model/CardSetSearchRequest.java (getPageRequest).
src/main/java/project/flipnote/cardset/repository/CardSetMetadataRepository.java (3)
26-29: 북마크 수 증가 쿼리 추가: 동시성/엔티티 컨텍스트 처리 적절JPQL 업데이트 +
@Modifying(clearAutomatically = true, flushAutomatically = true)사용 적절합니다.
30-36: 감소 시 0 미만 방지 로직 적절
CASE WHEN m.bookmarkCount > 0 THEN ... ELSE 0 END로 언더플로 방지한 점 좋습니다.
38-44: 빈 리스트/대용량 IN 방어 필요
- 확인: 엔티티에 bookmarkCount가 @column(nullable = false)로 선언되어 있고 생성자에서 0으로 초기화되어 있음. (src/main/java/project/flipnote/cardset/entity/CardSetMetadata.java:24-31)
- 확인: 정렬 구현에서 bookmarkCount를 사용함 (src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustomImpl.java:62).
- 권고: 서비스 계층에서 null/빈 리스트 방어 및 DB IN 한도 대비 청크 처리 필요(현재 repository에 batch 메서드 존재: src/main/java/project/flipnote/cardset/repository/CardSetMetadataRepository.java:38-44). 아래 예시 적용 가능:
// src/main/java/project/flipnote/cardset/service/CardSetService.java @Transactional public void decrementBookmarkCount(List<Long> cardSetIds) { - cardSetMetadataRepository.decrementBookmarkCount(cardSetIds); + if (cardSetIds == null || cardSetIds.isEmpty()) { + return; + } + final int CHUNK = 900; // DB 한도 고려해 조정 + for (int i = 0; i < cardSetIds.size(); i += CHUNK) { + List<Long> slice = cardSetIds.subList(i, Math.min(i + CHUNK, cardSetIds.size())); + cardSetMetadataRepository.decrementBookmarkCount(slice); + } }
- 추가 확인 필요: 리포지토리에서 마이그레이션/DDL(인덱스 정의) 파일이 탐지되지 않음. bookmark_count에 대한 DB 인덱스(또는 필터와 함께 사용하는 경우 복합 인덱스) 존재 여부를 DB/마이그레이션에서 확인할 것.
src/main/java/project/flipnote/cardset/listener/CardSetBookmarkRemovedEventHandler.java (1)
25-31: AFTER_COMMIT + @async + @retryable 조합 적절 — @EnableAsync/@EnableRetry 활성화 확인됨
src/main/java/project/flipnote/common/config/AsyncConfig.java(@EnableAsync), src/main/java/project/flipnote/common/config/RetryConfig.java(@EnableRetry)에서 선언되어 있으므로 추가 조치 불필요합니다.
| } else if (sortField == CardSetSortField.BOOKMARK) { | ||
| orders.add(toOrderSpecifier(cardSetMetadata.bookmarkCount, order)); | ||
| useMetadata = true; |
There was a problem hiding this comment.
INNER JOIN으로 인한 결과 누락·total 불일치 가능성 — LEFT JOIN 권장
메타데이터가 없는 카드셋이 존재하면 BOOKMARK/LIKE 정렬 시 결과에서 누락되고(total는 전체 기준) 페이지네이션 불일치가 발생할 수 있습니다. LEFT JOIN으로 바꾸어 안전하게 정렬하세요.
- if (useMetadata) {
- selectQuery.join(cardSetMetadata).on(cardSet.id.eq(cardSetMetadata.id));
- }
+ if (useMetadata) {
+ selectQuery.leftJoin(cardSetMetadata).on(cardSet.id.eq(cardSetMetadata.id));
+ }보완 사항:
- LEFT JOIN 시 null 정렬이 앞서는 DB에서는 nulls last 처리(가능하면 Querydsl NullHandling)도 고려해 주세요.
- INNER JOIN을 고집해야 한다면 count 쿼리에도 동일 조인을 반영해 일관성을 맞춰야 합니다.
Also applies to: 79-81, 89-95
🤖 Prompt for AI Agents
In
src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustomImpl.java
around lines 61-63 (and similarly at 79-81 and 89-95), the current INNER JOIN on
cardSetMetadata causes card sets without metadata to be dropped and pagination
totals to mismatch; change those joins to LEFT JOIN so card sets with null
metadata are preserved, and ensure sorting handles nulls (e.g., apply Querydsl
nullsLast/nullsFirst handling so null metadata sorts as expected); if you must
keep INNER JOIN instead, apply the same INNER JOIN to the count query to keep
totals consistent.
2 participants
📝 변경 내용
✅ 체크리스트
💬 기타 참고 사항
Summary by CodeRabbit
신기능
개선