[Feat] 저장한 책,피드 조회 api 개발#255
Conversation
오타 수정 (#254)
…d-feed-book # Conflicts: # src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.java
Walkthrough저장한 책(GET /books/saved) 및 커서 기반 저장한 피드(GET /feeds/saved) 조회 기능이 추가되고, 관련 유스케이스·서비스·매퍼·DTO·퍼시스턴스·레포지토리·통합 테스트가 추가/수정되었으며 SavedFeeds 도메인과 일부 에러코드 및 검증 로직이 제거/대체되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant C as BookQueryController
participant UC as BookShowSavedListUseCase\n(BookSavedListService)
participant Q as BookQueryPort
participant M as BookQueryMapper
Client->>C: GET /books/saved (userId)
C->>UC: getSavedBookList(userId)
UC->>Q: findSavedBooksByUserId(userId)
Q-->>UC: List<Book>
UC->>M: toBookShowSavedInfoResultList(books)
M-->>UC: List<BookShowSavedInfoResult>
UC-->>C: BookShowSavedListResponse
C-->>Client: BaseResponse(data)
sequenceDiagram
actor Client
participant C as FeedQueryController
participant UC as FeedSavedListUseCase\n(FeedShowSavedListService)
participant Q as FeedQueryPort
participant L as PostLikeQueryPort
participant M as FeedQueryMapper
Client->>C: GET /feeds/saved?cursor=...
C->>UC: getSavedFeedList(userId, cursor)
UC->>Q: findSavedFeedsByCreatedAt(userId, Cursor)
Q-->>UC: CursorBasedList<FeedQueryDto>
UC->>L: findPostIdsLikedByUser(userId, feedIds)
L-->>UC: Set<Long> likedIds
UC->>M: map dto -> FeedShowSavedInfoDto (likedIds, userId)
M-->>UC: List<FeedShowSavedInfoDto>
UC-->>C: FeedShowSavedListResponse (list, nextCursor, isLast)
C-->>Client: BaseResponse(data)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ 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)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Test Results411 tests 411 ✅ 30s ⏱️ Results for commit 487abba. ♻️ This comment has been updated with latest results. |
![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 (25)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepository.java (2)
29-29: 파라미터 명을lastSavedAt으로 변경해 의도를 명확히 해주세요.저장한 피드의 커서가 savedCreatedAt 기준임을 반영하면 가독성과 일관성이 좋아집니다. 구현(Impl)에서
lastSavedAt를 쓰는 것으로 보이며, 시그니처 충돌은 없습니다(파라미터 이름은 시그니처에 영향 없음).적용 제안:
- List<FeedQueryDto> findSavedFeedsByCreatedAt(Long userId, LocalDateTime lastCreatedAt, int size); + List<FeedQueryDto> findSavedFeedsByCreatedAt(Long userId, LocalDateTime lastSavedAt, int size);
27-30: 정렬/커서 기준을 Javadoc으로 명확히 문서화해 주세요.첫 페이지에서 null 허용, 정렬 키(savedCreatedAt), 중복 방지용 타이브레이커 사용 여부 등을 명시하면 사용 측 혼선을 줄일 수 있습니다.
예시(주석 추가만, 코드 변경 없음):
List<Long> findLatestPublicFeedCreatorsIn(Set<Long> userIds, int size); + /** + * 사용자가 저장한 피드를 savedCreatedAt 내림차순으로 조회합니다. + * 첫 페이지 조회 시 lastSavedAt은 null을 전달합니다. + * 구현에서는 안정 정렬을 위해 savedCreatedAt 동률 시 보조키(예: feedId desc)를 사용하기를 권장합니다. + */ - List<FeedQueryDto> findSavedFeedsByCreatedAt(Long userId, LocalDateTime lastCreatedAt, int size); + List<FeedQueryDto> findSavedFeedsByCreatedAt(Long userId, LocalDateTime lastSavedAt, int size);src/main/java/konkuk/thip/book/application/port/in/BookShowSavedListUseCase.java (2)
7-9: 메서드 계약(정렬/무페이징)을 Javadoc으로 명시해 주세요.요구사항상 무페이징·최신 저장순임을 인터페이스에 명확히 남기면 호출자 혼동을 줄입니다.
public interface BookShowSavedListUseCase { - List<BookShowSavedInfoResult> getSavedBookList(Long userId); + /** + * 사용자가 저장한 책을 '저장 시각' 최신순으로 모두 반환합니다(무페이징). + * 대용량 사용자를 고려해 추후 페이징 전환 가능성을 열어두세요. + */ + List<BookShowSavedInfoResult> getSavedBookList(Long userId); }
7-9: 무페이징 반환의 상한/리스크 검토 권장저장 수가 많은 사용자의 경우 메모리·응답시간 이슈가 생길 수 있습니다. 최소한 상한(limit) 또는 구성 옵션을 두거나, 추후 페이징 전환 계획을 남겨두면 운영 리스크를 줄일 수 있습니다.
src/main/java/konkuk/thip/book/application/port/in/dto/BookShowSavedInfoResult.java (1)
3-12: saved 전용 응답에서isSaved는 항상 true일 가능성이 큽니다.해당 DTO가 "저장 목록 전용"이라면
isSaved는 중복 정보일 수 있습니다. 공용 DTO 재사용 목적이라면 유지, 그렇지 않다면 제거를 고려해도 됩니다.src/main/java/konkuk/thip/book/adapter/in/web/response/BookShowSavedListResponse.java (1)
10-12: 리스트를 불변 복사로 감싸 응답 객체의 방어적 복사 적용 제안외부에서 전달된 리스트 변경이 응답 오브젝트에 전파되지 않도록 List.copyOf로 감싸는 것을 권장합니다(JDK 10+).
- public static BookShowSavedListResponse of(List<BookShowSavedInfoResult> bookSavedInfoResultList) { - return new BookShowSavedListResponse(bookSavedInfoResultList); - } + public static BookShowSavedListResponse of(List<BookShowSavedInfoResult> bookSavedInfoResultList) { + return new BookShowSavedListResponse(List.copyOf(bookSavedInfoResultList)); + }src/main/java/konkuk/thip/feed/application/port/in/FeedSavedListUseCase.java (2)
5-7: 메서드 계약(Javadoc)으로 커서 의미와 정렬 키를 명시하세요.호출자에게 기대 포맷과 정렬 기준(savedCreatedAt desc, tie-breaker 포함)을 명확히 전달하는 것이 안전합니다.
public interface FeedSavedListUseCase { - FeedShowSavedListResponse getSavedFeedList(Long userId, String cursor); + /** + * 사용자가 저장한 피드를 커서 기반으로 조회합니다. + * - 정렬: savedCreatedAt 내림차순(동률 시 보조키로 안정 정렬) + * - cursor: 첫 페이지는 null, 이후에는 이전 응답의 nextCursor 사용 + */ + FeedShowSavedListResponse getSavedFeedList(Long userId, String cursor); }
5-7: Cursor VO 도입 검토String 대신 이미 존재하는
CursorVO를 파라미터로 승격하면 경계가 명확해지고 테스트/검증이 쉬워집니다. 또한, 커서 포맷(예: base64(feedId, savedCreatedAt) 및 안정 정렬을 위한 보조키 포함)도 인터페이스 주석으로 명시해주세요.• 파일:
src/main/java/konkuk/thip/feed/application/port/in/FeedSavedListUseCase.java
- 메서드 시그니처:
- FeedShowSavedListResponse getSavedFeedList(Long userId, String cursor);
- /**
- 저장된 피드 리스트 조회
- @param userId 조회 대상 사용자 ID
- @param cursor 페이징 커서 (format: base64(feedId, savedCreatedAt), 정렬 보조키 포함)
- */
- FeedShowSavedListResponse getSavedFeedList(Long userId, Cursor cursor);
• 위와 같이 변경 시 테스트/검증 로직을
CursorVO 내부에 캡슐화할 수 있어 재사용성과 일관성이 높아집니다.
• 기존src/main/java/konkuk/thip/common/util/Cursor.java를 활용해 주세요.src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java (1)
177-181: alias 매핑 일관성 및 boolean 상수 표기 개선
- aliasName: 다른 매핑(예: Line 147)에서는
Alias.from(...).getValue()를 사용하고, 여기서는source = "dto.alias"를 사용합니다. 표현이 달라 응답 스키마가 엔드포인트별로 불일치할 수 있습니다. 한쪽으로 통일하는 것이 좋습니다.- isSaved: MapStruct에서 boolean 상수는
constant = "true"도 가능하지만, 표현 일관성을 위해expression = "java(true)"를 권장합니다.- @Mapping(target = "aliasName", source = "dto.alias") + @Mapping(target = "aliasName", expression = "java(Alias.from(dto.alias()).getValue())") @Mapping(target = "aliasColor", expression = "java(Alias.from(dto.alias()).getColor())") - @Mapping(target = "isSaved", constant = "true") + @Mapping(target = "isSaved", expression = "java(true)") @Mapping(target = "isLiked", expression = "java(likedFeedIds.contains(dto.feedId()))")src/test/java/konkuk/thip/book/adapter/in/web/BookShowSavedListApiTest.java (2)
42-48: 테스트 데이터 시딩 흐름 적절별칭(Alias) → 유저 → 도서 → 저장관계 순으로 안정적으로 시딩하고 있어, API 동작 검증에 충분합니다.
추가로, “최신 저장 순 정렬” 요구를 검증하려면 2권 이상 저장 후 정렬을 검증하는 케이스를 하나 더 추가할 수 있습니다. 원하시면 보일러플레이트 최소화한 케이스를 제안드리겠습니다.
53-59: 응답 필드 보강 검증 제안: isSaved 확인현재 ISBN만 검증하고 있습니다. 저장 리스트 응답의 핵심 속성인
isSaved=true도 함께 검증하면 회귀에 강해집니다.mockMvc.perform(get("/books/saved") .requestAttr("userId", user.getUserId())) .andExpect(status().isOk()) .andExpect(jsonPath("$.data.bookList").isArray()) .andExpect(jsonPath("$.data.bookList.length()").value(1)) - .andExpect(jsonPath("$.data.bookList[0].isbn").value(savedBook.getIsbn())); + .andExpect(jsonPath("$.data.bookList[0].isbn").value(savedBook.getIsbn())) + .andExpect(jsonPath("$.data.bookList[0].isSaved").value(true));src/main/java/konkuk/thip/book/application/mapper/BookQueryMapper.java (2)
41-49: 저장 도서 매핑 LGTM + boolean 상수 표기 미세 개선 제안도메인 Book → BookShowSavedInfoResult 매핑은 적절합니다.
isSaved는 상수 매핑으로 충분하나, 표현 통일을 위해expression = "java(true)"도 고려해 볼 수 있습니다.- @Mapping(target = "isSaved", constant = "true") + @Mapping(target = "isSaved", expression = "java(true)")
50-50: List 매핑 메서드 중복 여부 확인MapStruct는 단건 매핑이 정의되어 있으면 동일 시그니처의 리스트 매핑을 자동 생성합니다. 명시적 리스트 매핑이 가독성 측면에서 도움이 되면 유지해도 무방하나, 불필요하다면 제거해 단순화할 수 있습니다.
이 메서드가 외부에서 직접 참조되어야 하는 특별한 이유가 없다면 자동 생성에 맡기는 방안을 검토해 주세요.
src/main/java/konkuk/thip/feed/application/port/out/FeedQueryPort.java (1)
35-41: 용어 모호성 제거: CreatedAt 기준 명시(저장 시각 기준인지 피드 시각 기준인지)현재 메서드명/주석만으로는 createdAt이 피드의 생성시각인지(savedCreatedAt) 저장 시각인지 혼동될 수 있습니다. 주석에 기준 시각을 명확히 남겨 주세요. 향후 유지보수 리스크를 줄입니다.
아래처럼 주석 보강을 추천합니다:
- /** - * 저장된 피드 조회 - */ + /** + * 저장된 피드 조회 + * 기준: 사용자가 피드를 '저장한 시각(savedCreatedAt)' 내림차순 + 커서 기반 페이지네이션 + * 반환: FeedQueryDto(savedCreatedAt 포함) + */ Set<Long> findSavedFeedIdsByUserIdAndFeedIds(Set<Long> feedIds, Long userId); boolean existsSavedFeedByUserIdAndFeedId(Long userId, Long feedId); CursorBasedList<FeedQueryDto> findSavedFeedsByCreatedAt(Long userId, Cursor cursor);src/main/java/konkuk/thip/feed/application/service/FeedShowSavedListService.java (4)
23-27: 고정 PAGE_SIZE(10) 상수의 구성화 검토하드코딩 상수는 초기 구현엔 충분하나, 트래픽/UX 조정 시 변경 비용이 큽니다. 구성 프로퍼티로 외부화하거나 상수 모듈로 공통화하는 방안을 권장합니다.
예: 추후
@ConfigurationProperties로feed.saved.page-size를 주입받아 사용
31-33: 변수명 오해 소지: nextCursor → requestCursor입력 커서를 파싱해 생성하는데 변수명이 nextCursor라 혼동될 수 있습니다. requestCursor 등으로 변경하면 가독성이 좋아집니다.
- Cursor nextCursor = Cursor.from(cursor, PAGE_SIZE); + Cursor requestCursor = Cursor.from(cursor, PAGE_SIZE);이후 참조도 함께 변경 필요.
34-39: 주석 오타: ‘책’ → ‘피드’설명과 실제 기능 불일치(타이포)입니다.
- // 2. 유저가 저장한 책 최신순으로 (페이징 처리 포함) - CursorBasedList<FeedQueryDto> result = feedQueryPort.findSavedFeedsByCreatedAt(userId, nextCursor); + // 2. 유저가 저장한 피드 최신순으로 (페이징 처리 포함) + CursorBasedList<FeedQueryDto> result = feedQueryPort.findSavedFeedsByCreatedAt(userId, requestCursor);
40-42: 빈 입력 최적화: 좋아요 조회 불필요 호출 방지조회 결과가 비어있으면 likes 쿼리를 생략해 왕복 비용을 줄일 수 있습니다.
- // 3. 유저가 좋아한 피드들 조회 - Set<Long> likedFeedIdsByUser = postLikeQueryPort.findPostIdsLikedByUser(feedIds, userId); + // 3. 유저가 좋아한 피드들 조회 (빈 입력 최적화) + Set<Long> likedFeedIdsByUser = feedIds.isEmpty() + ? Set.of() + : postLikeQueryPort.findPostIdsLikedByUser(feedIds, userId);src/main/java/konkuk/thip/feed/adapter/in/web/response/FeedShowSavedListResponse.java (1)
22-22: contentUrls 타입을 List으로 전환 고려컬렉션 응답은 배열보다 List가 직렬화/문서화/호환성 측면에서 일관적입니다(프로젝트 내 다른 응답들과의 일관성도 고려).
- String[] contentUrls, + java.util.List<String> contentUrls,참고: 매퍼/테스트에서도 동일 타입으로 맞춰야 합니다.
src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java (1)
104-111: 엔드포인트 추가 👍 — 스웨거 설명 보강 제안(정렬 기준/페이징 없음)요구사항상 “저장한 책을 최신 저장순, 페이징 없이” 반환합니다. 스웨거 설명에 이 조건을 명시하면 프론트/QA 커뮤니케이션이 명확해집니다. 예외 스펙이 있다면 ExceptionDescription도 추가를 검토해 주세요.
- @Operation( - summary = "저장한 책 조회", - description = "사용자가 저장한 책을 조회 합니다." - ) + @Operation( + summary = "저장한 책 조회", + description = "사용자가 저장한 책을 최신 저장순으로 조회합니다. (페이징 없음)" + )src/main/java/konkuk/thip/feed/application/port/out/dto/FeedQueryDto.java (1)
25-27: savedCreatedAt 추가는 타당하지만, 하위 사용처에서 null-safe 보장이 필요합니다
- DTO에
@Nullable LocalDateTime savedCreatedAt가 추가된 점은 저장 정렬/커서 용도로 적절합니다.- 단, 어댑터에서 커서를 생성할 때
savedCreatedAt().toString()직접 호출하는 코드가 있습니다. 저장하지 않은 피드 경로(기존 조회들)에서는savedCreatedAt가 null로 들어오므로 NPE 위험이 있습니다. 동 PR 내FeedQueryPersistenceAdapter.findSavedFeedsByCreatedAt에서만 사용되도록 되어 있지만, 미래 사용처 확장에 대비해 null-safe 사용을 권장합니다. 해당 파일에 구체적인 방어 코드 제안을 남겼습니다.Also applies to: 44-46, 62-63
src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryPersistenceAdapter.java (1)
102-105: 메서드 파라미터 순서 일관성 제안
- 포트 시그니처:
findSavedFeedIdsByUserIdAndFeedIds(Set<Long> feedIds, Long userId)- JPA 리포지토리 호출:
findSavedFeedIdsByUserIdAndFeedIds(userId, feedIds)레이어마다 파라미터 순서가 달라 가독성에 혼선을 줄 수 있습니다. 팀 컨벤션에 맞춰 (userId, feedIds) 또는 (feedIds, userId)로 일관화하는 것을 권장합니다.
src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSavedListApiTest.java (1)
177-227: 커서 인코딩 규약 검증 필요
- 테스트에서 커서를 DB의
LocalDateTime.toString()으로 직접 구성해 param에 전달합니다. 실제 컨트롤러/유틸의 커서 인코딩 규약(Cursor#toEncodedString)과 동일한지 확인 필요합니다. 규약이 다르면 운영에서 커서 파싱 실패 가능성이 있습니다.필요 시, 첫 페이지 응답의 nextCursor 값을 그대로 사용해 두 번째 요청을 만드는 식으로 테스트를 수정할 수 있습니다. 원하시면 해당 방향으로 테스트 리팩토링 패치를 제안드리겠습니다.
또한, “저장했지만 이후 비공개/비활성화된 피드”가 목록에서 적절히 제외되는지(혹은 의도에 맞게 표시되는지) 커버리지가 비어 있습니다. 추가 케이스를 작성해 드릴까요?
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (2)
369-371: QueryDSL null 표현식 타입 명시로 제네릭 경고/호환성 개선
QFeedQueryDto의 파라미터 타입에 맞춰 null 표현식 타입을 명시하면 안전합니다.다음과 같이 타입을 명시해 주세요.
- Expressions.nullExpression(), - Expressions.nullExpression() + Expressions.nullExpression(Boolean.class), + Expressions.nullExpression(java.time.LocalDateTime.class)
431-459: 저장 목록 조회 성능/인덱스 조언
- 쿼리 패턴이
savedFeed.user_id = ? AND savedFeed.created_at < ? ORDER BY savedFeed.created_at DESC LIMIT size+1입니다. DB에(user_id, created_at DESC)복합 인덱스가 있으면 커서 페이징 효율이 크게 향상됩니다.DB 마이그레이션/인덱스 상태 확인 후 없다면 추가를 권장드립니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (22)
src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java(3 hunks)src/main/java/konkuk/thip/book/adapter/in/web/response/BookShowSavedListResponse.java(1 hunks)src/main/java/konkuk/thip/book/application/mapper/BookQueryMapper.java(2 hunks)src/main/java/konkuk/thip/book/application/port/in/BookShowSavedListUseCase.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/dto/BookShowSavedInfoResult.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookSavedListService.java(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(0 hunks)src/main/java/konkuk/thip/feed/adapter/in/web/FeedQueryController.java(2 hunks)src/main/java/konkuk/thip/feed/adapter/in/web/response/FeedShowSavedListResponse.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepository.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java(9 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/SavedFeedJpaRepository.java(1 hunks)src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java(1 hunks)src/main/java/konkuk/thip/feed/application/port/in/FeedSavedListUseCase.java(1 hunks)src/main/java/konkuk/thip/feed/application/port/out/FeedQueryPort.java(2 hunks)src/main/java/konkuk/thip/feed/application/port/out/dto/FeedQueryDto.java(3 hunks)src/main/java/konkuk/thip/feed/application/service/FeedSavedService.java(2 hunks)src/main/java/konkuk/thip/feed/application/service/FeedShowSavedListService.java(1 hunks)src/main/java/konkuk/thip/feed/domain/SavedFeeds.java(0 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookShowSavedListApiTest.java(1 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSavedListApiTest.java(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
- src/main/java/konkuk/thip/feed/domain/SavedFeeds.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/test/java/konkuk/thip/book/adapter/in/web/BookShowSavedListApiTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(29-413)
⏰ 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 (13)
src/main/java/konkuk/thip/book/adapter/in/web/response/BookShowSavedListResponse.java (1)
7-13: 간단하고 명확한 래퍼 구조, 좋습니다.정적 팩토리 메서드로 생성 경로도 일관적입니다.
src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java (1)
182-186: postDate 기준 확인 필요: 저장시각 vs. 게시시각저장한 피드 목록은 정렬/페이징을 savedCreatedAt으로 처리했다는 PR 설명과 달리, 화면 표시용
postDate는dto.createdAt()(게시 시각)을 사용합니다. UX 요건에 따라 저장 시각을 보여줘야 한다면 아래처럼savedCreatedAt을 우선 적용하는 식이 더 자연스러울 수 있습니다. 의도하신 바를 확인 부탁드립니다.- @Mapping( - target = "postDate", - expression = "java(DateUtil.formatBeforeTime(dto.createdAt()))" - ) + @Mapping( + target = "postDate", + expression = "java(DateUtil.formatBeforeTime(dto.savedCreatedAt() != null ? dto.savedCreatedAt() : dto.createdAt()))" + )설계 결정이 “게시 시각 노출, 저장 시각은 정렬에만 사용”이라면, 현재 구현으로도 무방합니다.
src/test/java/konkuk/thip/book/adapter/in/web/BookShowSavedListApiTest.java (1)
25-31: 테스트 환경 구성 적절SpringBootTest + MockMvc(without filters) + test profile + @transactional로 롤백 보장, 통합 테스트 기본 세팅이 깔끔합니다.
src/main/java/konkuk/thip/feed/adapter/in/web/FeedQueryController.java (1)
34-34: 의존성 주입 추가 LGTMSaved 피드 조회 유스케이스 주입이 자연스럽고 기존 흐름과 일관적입니다.
src/main/java/konkuk/thip/feed/application/port/out/FeedQueryPort.java (1)
52-56: 태그 조회 메서드 재배치 OK포트 내 위치 변경은 영향이 없으며, 시그니처가 유지되고 있어 문제 없습니다.
src/main/java/konkuk/thip/book/application/service/BookSavedListService.java (2)
14-20: 서비스 구성 및 의존성 주입 적절읽기 전용 트랜잭션, 포트/매퍼 의존성 구성 모두 적절합니다.
21-26: 정렬 보장 확인(최신 저장순), 대용량 반환 주의
- PR 설명상 “저장한 최신순, 페이징 없이 반환”이 요구사항입니다. 현재 서비스 계층에서는 정렬을 강제하지 않으니, bookQueryPort.findSavedBooksByUserId(userId)가 저장시각 기준 내림차순을 보장하는지 확인이 필요합니다.
- 페이징 없이 전량 반환이므로 사용자의 저장 수가 많을 경우 메모리/응답시간 영향이 있을 수 있습니다. 현 요구사항 유지하되, 추후 요청 규모 증가 시 서버/클라이언트 최적화 옵션을 검토해 주세요.
정렬 보장을 확인하는 간단한 점검:
- Repository/Adapter 쿼리에
ORDER BY saved.created_at DESC가 명시되어 있는지 확인- 통합 테스트에 최신 저장순 검증 케이스 존재 여부 확인
src/main/java/konkuk/thip/feed/application/service/FeedShowSavedListService.java (1)
48-52: 커서 계약 확인: 마지막 페이지 시 nextCursor 처리현재 isLast는
!result.hasNext()로 정확합니다. 한편 nextCursor가 마지막 페이지일 때 null(또는 빈 문자열)로 일관되게 반환되는지 계약을 확인해 주세요. 클라이언트 페이징 루프 종료 조건과 직결됩니다.점검 포인트:
- CursorBasedList.nextCursor()의 반환 규칙 문서/테스트 확인
- 마지막 페이지 응답 예시(JSON)에서 nextCursor 값 확인
src/main/java/konkuk/thip/feed/adapter/in/web/response/FeedShowSavedListResponse.java (1)
5-9: 응답 스키마 구성 적절페이징 커서와 isLast를 함께 노출하는 구조 합리적입니다.
src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java (1)
32-32: 신규 UseCase 주입 OK컨트롤러 의존성 주입과 역할 분리 적절합니다.
src/main/java/konkuk/thip/feed/application/service/FeedSavedService.java (1)
34-41: 행위 검증 로직(중복 저장/비저장 삭제) 분기 명확 — 가독성과 유지보수성 향상
validateSaveFeedAction으로 예외 분리를 명확히 하여 의도가 잘 드러납니다.- 저장/삭제의 실제 수행도
command.isSaved()한 곳에서 결정되어 읽기 좋습니다.src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSavedListApiTest.java (1)
54-118: 시나리오 구성 및 단언 충실 — 핵심 필드 검증이 잘 되어 있습니다
- 저장 순서 제어를 위해 created_at을 덮어써 최신순을 검증하는 방식 좋습니다.
- isSaved/isLiked, contentUrls/likeCount/commentCount 등 응답 필드도 잘 커버합니다.
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (1)
262-285: savedCreatedAt 전달 경로 일관화 좋습니다
toDto시그니처 확장 및 기존 호출부에서 null로 명시 전달해 주셔서, saved 경로와 일반 경로의 책임이 명확합니다.Also applies to: 83-85, 105-106, 196-197, 215-216
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (1)
262-285: toDto 확장( savedCreatedAt ) 반영 OK — 경계값 처리 한 가지 제안구현 자체는 문제 없습니다. 다만 다음 상황에 대한 방어를 가볍게 고려해볼 수 있습니다.
- e.getContentList()가 빈 컬렉션이 아닌 null로 들어오는 경우 NullPointerException 가능성
선호하시면 아래처럼 방어적으로 처리해둘 수 있습니다.
- String[] urls = e.getContentList().stream() + String[] urls = java.util.Optional.ofNullable(e.getContentList()) + .orElseGet(java.util.List::of) + .stream() .map(ContentJpaEntity::getContentUrl) .toArray(String[]::new);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/main/java/konkuk/thip/feed/adapter/in/web/FeedQueryController.java(2 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/konkuk/thip/feed/adapter/in/web/FeedQueryController.java
⏰ 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 (3)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (3)
41-41: SavedFeed QPath 추가 적절SavedFeed 경로 객체 추가로 이후 저장 피드 전용 쿼리 작성이 수월해졌습니다. 별다른 이슈 없습니다.
83-84: toDto 시그니처 변경 반영 OK기존 호출부에서 savedCreatedAt을 null로 일관되게 전달하고 있어 의도한 backwards compatibility가 유지됩니다.
Also applies to: 105-106, 196-197, 215-216
369-371: QFeedQueryDto 생성자 시그니처 수동 확인 및 null 타입 명시 권고스크립트 실행 결과 QFeedQueryDto 정의 파일을 찾지 못해 인자 개수·순서·타입을 자동 검증할 수 없습니다.
toQueryDto() 호출부와 생성자 시그니처가 정확히 일치하는지 수동으로 확인해 주세요. 또한 Querydsl Projection에 null을 넘길 때는 타입을 명시하는 것이 안전합니다.
- 확인 사항
- QFeedQueryDto 생성자 파라미터 수·순서·타입 vs. toQueryDto() 호출부 인자 일치 여부
- Boolean, LocalDateTime 등 null 인자를 전달할 때는 Expressions.constant((Type) null) 사용 권장
- Expressions.nullExpression(), - Expressions.nullExpression() + Expressions.constant((java.lang.Boolean) null), // isPriorityFeed 등 Boolean 타입 + Expressions.constant((java.time.LocalDateTime) null) // savedCreatedAt 등 DateTime 타입
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (1)
421-439: 컬렉션(fetch join) + 페이징 동시 사용 → 중복/페이지 왜곡 위험 (최소 수정안 제안)
feed.contentList에 대한 컬렉션 fetch join과limit(size+1)를 함께 사용하면 SQL 레벨에서 row 폭증으로 limit가 “루트 기준”이 아닌 “조인된 row 기준”으로 적용되어 페이지 왜곡과 중복이 발생할 수 있습니다.select(savedFeed).distinct()만으로는 limit 적용 시점을 보정하지 못합니다.최소 수정안: 컬렉션 fetch join을 제거하세요. 이 경우 N+1 가능성은 있으나 적어도 페이징 왜곡은 방지됩니다.
private List<SavedFeedJpaEntity> getSavedFeedJpaEntities(Long userId, LocalDateTime lastSavedAt, int size) { List<SavedFeedJpaEntity> savedFeeds = jpaQueryFactory .select(savedFeed).distinct() .from(savedFeed) .leftJoin(savedFeed.feedJpaEntity, feed).fetchJoin() - .leftJoin(feed.contentList, content).fetchJoin() .leftJoin(feed.userJpaEntity, user).fetchJoin() .leftJoin(user.aliasForUserJpaEntity, alias).fetchJoin() .leftJoin(feed.bookJpaEntity, book).fetchJoin() .where( savedFeed.userJpaEntity.userId.eq(userId), savedFeed.feedJpaEntity.status.eq(StatusType.ACTIVE), savedFeed.feedJpaEntity.userJpaEntity.userId.eq(userId) .or(savedFeed.feedJpaEntity.isPublic.eq(true)), lastSavedAt != null ? savedFeed.createdAt.lt(lastSavedAt) : Expressions.TRUE ) .orderBy(savedFeed.createdAt.desc()) .limit(size + 1) .fetch(); return savedFeeds; }권장(좀 더 견고) 2단계 패턴은 다음 코멘트에 별도로 제안합니다.
🧹 Nitpick comments (5)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (5)
262-286: toDto에 savedCreatedAt 매핑 추가 OK — alias null-safety 점검 권장DTO에
savedCreatedAt추가/매핑은 적절합니다. 다만e.getUserJpaEntity().getAliasForUserJpaEntity()가 null일 가능성이 있다면 현재 체이닝 접근부(Line 272-273)는 NPE 위험이 있습니다. 기존 TODO도 있어 후속 반영 시 아래와 같이 방어 코드를 권장합니다.다음과 같이 로컬 변수로 alias 엔티티를 받아 null-safe 접근을 권장합니다:
private FeedQueryDto toDto(FeedJpaEntity e, Integer priority, LocalDateTime savedCreatedAt) { - String[] urls = e.getContentList().stream() + String[] urls = e.getContentList().stream() .map(ContentJpaEntity::getContentUrl) .toArray(String[]::new); - boolean isPriorityFeed = (priority != null && priority == 1); + boolean isPriorityFeed = (priority != null && priority == 1); + AliasJpaEntity aliasEntity = e.getUserJpaEntity().getAliasForUserJpaEntity(); return FeedQueryDto.builder() .feedId(e.getPostId()) .creatorId(e.getUserJpaEntity().getUserId()) .creatorNickname(e.getUserJpaEntity().getNickname()) - .creatorProfileImageUrl(e.getUserJpaEntity().getAliasForUserJpaEntity().getImageUrl()) - .alias(e.getUserJpaEntity().getAliasForUserJpaEntity().getValue()) + .creatorProfileImageUrl(aliasEntity != null ? aliasEntity.getImageUrl() : null) + .alias(aliasEntity != null ? aliasEntity.getValue() : null) .createdAt(e.getCreatedAt()) ... .isPriorityFeed(isPriorityFeed) .savedCreatedAt(savedCreatedAt) .build(); }
349-372: QFeedQueryDto 투영 시 불리언은 null 대신 FALSE 전달 권장마지막 두 파라미터가
isPriorityFeed,savedCreatedAt순서라면, 불리언에Expressions.nullExpression()을 주는 것보다Expressions.FALSE가 명시적이고 타입 안정적입니다.savedCreatedAt은 null 허용이면 그대로 유지해도 됩니다.적용 예시:
feed.commentCount, feed.isPublic, - Expressions.nullExpression(), + Expressions.FALSE, Expressions.nullExpression()
406-419: SavedFeed 조회는 “ID만 페이징 → 상세 fetch join”의 2단계 패턴으로 전환 권장기존 코드의 컬렉션 fetch join 제거만으로는 N+1 가능성이 남습니다. 다른 경로들과 일관되게 2단계 패턴(IDs 페이지 → 상세 fetch join)으로 바꾸면, 페이징 왜곡과 N+1을 모두 방지할 수 있습니다.
아래는 방향성 예시입니다(실제 적용 시 적절히 메서드 분리/네이밍 정리 필요):
- public List<FeedQueryDto> findSavedFeedsByCreatedAt(Long userId, LocalDateTime lastSavedAt, int size) { - - // 1. SavedFeed를 한 번에 페이징 조회하며 feed와 연관 엔티티 fetch join - List<SavedFeedJpaEntity> savedFeeds = getSavedFeedJpaEntities(userId, lastSavedAt, size); - if (savedFeeds.isEmpty()) { - return List.of(); - } - - // 2. 저장순대로 FeedQueryDto 변환 (Feed 및 savedCreatedAt 정보 포함) - return savedFeeds.stream() - .map(saved -> toDto(saved.getFeedJpaEntity(), null, saved.getCreatedAt())) - .collect(Collectors.toList()); - } - - private List<SavedFeedJpaEntity> getSavedFeedJpaEntities(Long userId, LocalDateTime lastSavedAt, int size) { - List<SavedFeedJpaEntity> savedFeeds = jpaQueryFactory - .select(savedFeed).distinct() - .from(savedFeed) - .leftJoin(savedFeed.feedJpaEntity, feed).fetchJoin() - .leftJoin(feed.contentList, content).fetchJoin() - .leftJoin(feed.userJpaEntity, user).fetchJoin() - .leftJoin(user.aliasForUserJpaEntity, alias).fetchJoin() - .leftJoin(feed.bookJpaEntity, book).fetchJoin() - .where( - savedFeed.userJpaEntity.userId.eq(userId), - savedFeed.feedJpaEntity.status.eq(StatusType.ACTIVE), - savedFeed.feedJpaEntity.userJpaEntity.userId.eq(userId) - .or(savedFeed.feedJpaEntity.isPublic.eq(true)), - lastSavedAt != null ? savedFeed.createdAt.lt(lastSavedAt) : Expressions.TRUE - ) - .orderBy(savedFeed.createdAt.desc()) - .limit(size + 1) - .fetch(); - return savedFeeds; - } + public List<FeedQueryDto> findSavedFeedsByCreatedAt(Long userId, LocalDateTime lastSavedAt, int size) { + // 1) savedFeed에서 (feedId, savedAt)만 size+1로 조회 + List<Tuple> tuples = jpaQueryFactory + .select(savedFeed.feedJpaEntity.postId, savedFeed.createdAt) + .from(savedFeed) + .where( + savedFeed.userJpaEntity.userId.eq(userId), + savedFeed.feedJpaEntity.status.eq(StatusType.ACTIVE), + savedFeed.feedJpaEntity.userJpaEntity.userId.eq(userId) + .or(savedFeed.feedJpaEntity.isPublic.eq(true)), + lastSavedAt != null ? savedFeed.createdAt.lt(lastSavedAt) : Expressions.TRUE + ) + .orderBy(savedFeed.createdAt.desc()) + .limit(size + 1) + .fetch(); + if (tuples.isEmpty()) return List.of(); + + // 2) 상세 엔티티 fetch join 조회 + List<Long> feedIds = tuples.stream().map(t -> t.get(0, Long.class)).toList(); + Map<Long, LocalDateTime> savedAtMap = tuples.stream() + .collect(Collectors.toMap(t -> t.get(0, Long.class), t -> t.get(1, LocalDateTime.class))); + List<FeedJpaEntity> entities = fetchFeedEntitiesByIds(feedIds); // 여기서 contentList 등 fetch join + Map<Long, FeedJpaEntity> entityMap = entities.stream() + .collect(Collectors.toMap(FeedJpaEntity::getPostId, e -> e)); + + // 3) 원본 저장순서 유지 + 누락 방지 + return feedIds.stream() + .map(entityMap::get) + .filter(java.util.Objects::nonNull) + .map(e -> toDto(e, null, savedAtMap.get(e.getPostId()))) + .collect(Collectors.toList()); + }장점:
- 페이징 왜곡 없음
- 컬렉션 안전한 fetch join 유지
- 저장순서 보존 및 누락 ID 방어 용이
Also applies to: 421-441
407-407: @nullable 일관 적용 권장
lastSavedAt를 null 허용으로 사용하고 있으므로 시그니처에도@Nullable을 붙여 다른 메서드들과 일관성을 맞추는 것을 권장합니다.아래처럼 간단히 반영 가능합니다.
- public List<FeedQueryDto> findSavedFeedsByCreatedAt(Long userId, LocalDateTime lastSavedAt, int size) { + public List<FeedQueryDto> findSavedFeedsByCreatedAt(Long userId, @Nullable LocalDateTime lastSavedAt, int size) {- private List<SavedFeedJpaEntity> getSavedFeedJpaEntities(Long userId, LocalDateTime lastSavedAt, int size) { + private List<SavedFeedJpaEntity> getSavedFeedJpaEntities(Long userId, @Nullable LocalDateTime lastSavedAt, int size) {Also applies to: 421-421
431-436: 저장 피드 조회 성능 최적화를 위한 인덱스 제안where/orderBy가
(user_id, created_at DESC)에 강하게 의존합니다. SavedFeed 테이블에 복합 인덱스(user_id, created_at DESC)를 추가하면 정렬 비용과 범위 스캔 효율이 크게 좋아집니다. Feed 테이블도(status, is_public, user_id)조합을 자주 사용한다면 보조 인덱스 고려가 좋습니다.가능한 DDL 예시(MySQL):
CREATE INDEX ix_saved_feed_user_created_at ON saved_feed (user_id, created_at DESC); -- (선택) feed 조회 패턴에 따라 CREATE INDEX ix_feed_status_public_user ON feed (status, is_public, user_id);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java(9 hunks)
⏰ 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 (6)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (6)
41-41: QSavedFeed 엔티티 필드 추가 적절saved feed 조회 로직을 구현하기 위한
QS 해당 엔티티필드 추가는 방향성 문제 없습니다.
82-84: toDto 시그니처 변경 반영 OK (팔로잉 우선순위 경로)세 번째 인자(
savedCreatedAt=null) 전달로 시그니처 변경이 잘 반영되었습니다.
104-106: toDto 시그니처 변경 반영 OK (최신순 경로)문제 없습니다.
195-197: toDto 시그니처 변경 반영 OK (내 피드 최신순 경로)문제 없습니다.
214-216: toDto 시그니처 변경 반영 OK (특정 유저 피드 최신순 경로)문제 없습니다.
405-419: Cursor 기반 페이징: size+1 → CursorBasedList에서 trimming 및 hasNext 처리됨
FeedQueryRepositoryImpl에서limit(size+1)로 더 조회한 뒤,FeedQueryPersistenceAdapter.findSavedFeedsByCreatedAt에서 반환된List<FeedQueryDto>를
CursorBasedList.of(queryList, size, …)로 감싸며
queryList.size() > size로hasNext계산subList(0, size)로 불필요한 한 건을 잘라냄extractor로 올바른nextCursor생성서비스 계층(
FeedShowSavedListService)은 이CursorBasedList를 그대로 사용해
feedList,nextCursor(),!hasNext()값을 전달하므로 별도 수정 없이 페이징 계약이 준수되고 있습니다.
seongjunnoh
left a comment
seongjunnoh
left a comment
There was a problem hiding this comment.
고생하셨습니다! 저장한 피드 조회 부분의 QueryDSL 코드 쪽에 리뷰 남겼는데, 확인 부탁드립니다!
| @Override | ||
| @Transactional(readOnly = true) | ||
| public FeedShowSavedListResponse getSavedFeedList(Long userId, String cursor) { | ||
| // 1. 커서 생성 | ||
| Cursor nextCursor = Cursor.from(cursor, PAGE_SIZE); | ||
|
|
||
| // 2. 유저가 저장한 책 최신순으로 (페이징 처리 포함) | ||
| CursorBasedList<FeedQueryDto> result = feedQueryPort.findSavedFeedsByCreatedAt(userId, nextCursor); | ||
| Set<Long> feedIds = result.contents().stream() | ||
| .map(FeedQueryDto::feedId) | ||
| .collect(Collectors.toUnmodifiableSet()); | ||
|
|
||
| // 3. 유저가 좋아한 피드들 조회 | ||
| Set<Long> likedFeedIdsByUser = postLikeQueryPort.findPostIdsLikedByUser(feedIds, userId); | ||
|
|
||
| // 4. response 로의 매핑 | ||
| List<FeedShowSavedListResponse.FeedShowSavedInfoDto> feedList = result.contents().stream() | ||
| .map(dto -> feedQueryMapper.toFeedShowSavedListResponse(dto, likedFeedIdsByUser, userId)) | ||
| .toList(); | ||
|
|
||
| return new FeedShowSavedListResponse( | ||
| feedList, | ||
| result.nextCursor(), | ||
| !result.hasNext() | ||
| ); | ||
| } |
| boolean isPublic, | ||
| @Nullable Boolean isPriorityFeed | ||
| @Nullable Boolean isPriorityFeed, | ||
| @Nullable LocalDateTime savedCreatedAt |
There was a problem hiding this comment.
넵 저장한 피드 조회가 피드를 저장한 시간떄문에 불가피하게 추가했습니닷
| private List<SavedFeedJpaEntity> getSavedFeedJpaEntities(Long userId, LocalDateTime lastSavedAt, int size) { | ||
| List<SavedFeedJpaEntity> savedFeeds = jpaQueryFactory | ||
| .select(savedFeed).distinct() | ||
| .from(savedFeed) | ||
| .leftJoin(savedFeed.feedJpaEntity, feed).fetchJoin() | ||
| .leftJoin(feed.contentList, content).fetchJoin() | ||
| .leftJoin(feed.userJpaEntity, user).fetchJoin() | ||
| .leftJoin(user.aliasForUserJpaEntity, alias).fetchJoin() | ||
| .leftJoin(feed.bookJpaEntity, book).fetchJoin() | ||
| .where( | ||
| savedFeed.userJpaEntity.userId.eq(userId), | ||
| savedFeed.feedJpaEntity.status.eq(StatusType.ACTIVE), | ||
| savedFeed.feedJpaEntity.userJpaEntity.userId.eq(userId) | ||
| .or(savedFeed.feedJpaEntity.isPublic.eq(true)), | ||
| lastSavedAt != null ? savedFeed.createdAt.lt(lastSavedAt) : Expressions.TRUE | ||
| ) | ||
| .orderBy(savedFeed.createdAt.desc()) | ||
| .limit(size + 1) | ||
| .fetch(); | ||
| return savedFeeds; | ||
| } |
There was a problem hiding this comment.
contents, alias 의 DB 구조 변경하면서 jpa 엔티티의 연관관계에 nullable 속성을 추가한 후, 기존 QueryDSL 코드에서 left join -> join 으로 개선할 수 있는 코드들을 수정한다면 성능을 한층 더 개선할 수 있지 않나 싶습니다! @hd0rable @buzz0331
p1 : 추가로 현재 코드에서는 savedFeed와 여러 jpa entity들을 fetch join 한 후 ordering을 하고 있는데,
이때 join 대상들 중 contents 는 feed 와 1:n 관계이므로 한 피드가 여러 contents row를 가지고 있는 경우 join 결과 서로 다른 row로 인식되어 페이징 처리가 원하는 대로 이루어지지 않는 이슈가 있습니다!
이 때문에 저도 바로 여러 엔티티들을 fetch join하여 조회한 후 dto로 변환하는게 아니라,
- 조회할 루트 엔티티의 id 값만을 페이징 처리하여 조회
- 반환을 위해 필요한 다른 엔티티의 정보를 fetch join
- dto로 변환
하는 3단계를 거쳤습니다! (findLatestFeedsByCreatedAt 메서드를 참고해주시면 됩니다!)
그런데 지금 현준님이 contents 테이블을 수정하시는 중으로 알고있는데
- 지금 일단 api의 배포가 먼저이니 현재 코드를 수정하여 배포
- 현준님 contents 테이블 수정이 끝나면, 이를 반영하여 코드 수정 & api 배포
방향이 있을 것 같은데, 저는 api 배포가 우선이니 1번이 어떤가 싶습니다! @buzz0331 @hd0rable
There was a problem hiding this comment.
엇 제가 이렇게하려다가 페이징 이슈는 생각못하고 저장한 책을 기준으로 조회하려다보니 Id를 가져올때 savedFeed의 feed를가져오면서 join이 추가적으로 2번생겨서 이렇게 구현한거긴핮니다 추가적인 join2번으로 id조회하는 기존방식으로 일단 수정해놓고 comtet수정되면 그때맞춰서 수정하겠슴니당
There was a problem hiding this comment.
contents, alias 의 DB 구조 변경하면서 jpa 엔티티의 연관관계에 nullable 속성을 추가한 후, 기존 QueryDSL 코드에서 left join -> join 으로 개선할 수 있는 코드들을 수정한다면 성능을 한층 더 개선할 수 있지 않나 싶습니다! @hd0rable @buzz0331
p1 : 추가로 현재 코드에서는 savedFeed와 여러 jpa entity들을 fetch join 한 후 ordering을 하고 있는데, 이때 join 대상들 중 contents 는 feed 와 1:n 관계이므로 한 피드가 여러 contents row를 가지고 있는 경우 join 결과 서로 다른 row로 인식되어 페이징 처리가 원하는 대로 이루어지지 않는 이슈가 있습니다!
이 때문에 저도 바로 여러 엔티티들을 fetch join하여 조회한 후 dto로 변환하는게 아니라,
- 조회할 루트 엔티티의 id 값만을 페이징 처리하여 조회
- 반환을 위해 필요한 다른 엔티티의 정보를 fetch join
- dto로 변환
하는 3단계를 거쳤습니다! (findLatestFeedsByCreatedAt 메서드를 참고해주시면 됩니다!)
그런데 지금 현준님이 contents 테이블을 수정하시는 중으로 알고있는데
- 지금 일단 api의 배포가 먼저이니 현재 코드를 수정하여 배포
- 현준님 contents 테이블 수정이 끝나면, 이를 반영하여 코드 수정 & api 배포
방향이 있을 것 같은데, 저는 api 배포가 우선이니 1번이 어떤가 싶습니다! @buzz0331 @hd0rable
위에서 기술한 이슈를 눈으로 직접 확인한게 아니라, 이론적으로 이러지 않을까? 라는 생각에 1->2->3 번의 방법으로 코드를 구현하였고, 희진님께도 이 방식을 제안드린거였는데
직접 확인해보고 싶어서 현재 브랜치에 추가로 테스트 코드를 통해 테스트 하였습니다
테스트 시나리오 : contents를 여러개 가지는 feed들을 여러개 생성한 후, 저장한 피드 조회시 동일한 피드가 여러개 조회되는지 아닌지
-> 제가 생각한 이슈가 발생한다면 api 응답에 동일한 피드가 여러개 조회되어야 함
그런데 놀랍게도 hibernate 가 알아서 oneToMany 와의 fetch join 시에 distinct를 적용해줘서 제가 우려했던 이슈가 발생하지 않고, 페이징 처리가 잘 되는 것을 확인했습니다!!
하지만 좀 찾아보니 이는 hibernate가 메모리 상에서 fetch join된 여러 중복된 row들의 중복을 제거해주는 것이므로, 기능적으로는 잘 동작할 지 몰라도 성능상의 이슈가 존재할 것 같습니다
https://delvering.tistory.com/52
https://github.com/hibernate/hibernate-orm/blob/6.0/migration-guide.adoc#distinct
There was a problem hiding this comment.
넵넵 말씀하신대로 jpa에서 fetch join + 페이징 처리 + distinct를 함께 사용하면 limit 조건을 무시하는 이슈가 있어서 현재 구현된 코드는 사실상 limit를 빼고 조건에 맞는 모든 피드를 메모리에 올려서 subList를 통해 페이징 처리를 하는 거라고 보면 될 것 같아요! 따라서 제가 특정 책으로 작성된 피드 조회 api에서 사용했던 쿼리처럼 Content를 서브 쿼리를 통해 조회하는 것은 어떻게 생각하시나요??
다음과 같이 한번의 쿼리로 QueryProjection을 사용해서 조회하는 것이기 때문에 굳이 fetch join을 사용할 필요도 없고 메모리에 부담도 생기지 않을 것 같습니다!
/**
* 사용자가 저장한 피드를 저장순으로 조회
*/
@Override
public List<FeedQueryDto> findSavedFeedsByCreatedAt(Long userId, LocalDateTime lastSavedAt, int size) {
BooleanExpression where = savedFeed.userJpaEntity.userId.eq(userId)
.and(savedFeed.feedJpaEntity.status.eq(StatusType.ACTIVE))
.and(
savedFeed.feedJpaEntity.userJpaEntity.userId.eq(userId)
.or(savedFeed.feedJpaEntity.isPublic.eq(true))
);
if (lastSavedAt != null) {
where = where.and(savedFeed.createdAt.lt(lastSavedAt));
}
return jpaQueryFactory
.select(toSavedFeedQueryDto())
.from(savedFeed)
.join(savedFeed.feedJpaEntity, feed)
.join(feed.userJpaEntity, user)
.join(feed.bookJpaEntity, book)
.where(where)
.orderBy(savedFeed.createdAt.desc())
.limit(size + 1)
.fetch();
}
/**
* SavedFeed 전용 DTO 매핑
*/
private QFeedQueryDto toSavedFeedQueryDto() {
return new QFeedQueryDto(
feed.postId,
feed.userJpaEntity.userId,
user.nickname,
user.aliasForUserJpaEntity.imageUrl,
user.aliasForUserJpaEntity.value,
feed.createdAt,
book.isbn,
book.title,
book.authorName,
feed.content,
// Content는 N:1 방지 위해 서브쿼리 사용
JPAExpressions
.select(contentUrlAggExpr())
.from(content)
.where(content.postJpaEntity.postId.eq(feed.postId)),
feed.likeCount,
feed.commentCount,
feed.isPublic,
Expressions.nullExpression(),
savedFeed.createdAt
);
}There was a problem hiding this comment.
이렇게 수정하면 toDto 메서드에 추가된 파라미터도 제거해도 될 것 같네요!
| .where( | ||
| savedFeed.userJpaEntity.userId.eq(userId), | ||
| savedFeed.feedJpaEntity.status.eq(StatusType.ACTIVE), | ||
| savedFeed.feedJpaEntity.userJpaEntity.userId.eq(userId) | ||
| .or(savedFeed.feedJpaEntity.isPublic.eq(true)), | ||
| lastSavedAt != null ? savedFeed.createdAt.lt(lastSavedAt) : Expressions.TRUE |
There was a problem hiding this comment.
굳굳
저장한 피드들 중, 내가 작성한 공개/비공개 피드 or 남이 작성한 공개 피드를 조회해야한다
라는 요구사항이 있었나 보군요! 좋습니다
There was a problem hiding this comment.
엇 이거왜 이렇게 되어있지 ㅎ.. 누락된거같아요 남이 작성한 공개한 피드/ 비공개피드인데 내가 작성한 피드가 조회되어야합니다.. 수정하겠습니닷
| public record BookShowSavedInfoResult( | ||
| Long bookId, | ||
| String bookTitle, | ||
| String authorName, | ||
| String publisher, | ||
| String bookImageUrl, | ||
| String isbn, | ||
| boolean isSaved | ||
| ) { | ||
| } |
There was a problem hiding this comment.
p3 : 엇 저희 저번에 공통 dto의 활용에 대해서 컨벤션 같은걸 정한게 있을까요??
BookShowSavedInfoResult를 책 관련 정보를 조회하는 여러 api 들이 공유하는 result dto 로 정의하신 것 같은데,
response 의 inner class vs 여러 response 들이 공유하는 result dto 의 활용에 대해서 이전에 얘기가 마무리된게 있는지 헷갈려서 여쭤봅니다!
There was a problem hiding this comment.
저는 inner class를 사용하는 것보다는, 지금 희진님이 해주신 것처럼 최종 adapter 계층의 response DTO가 Result를 리스트로 갖는 일급 컬렉션 형태가 헥사고날 아키텍처에 더 가깝다고 생각합니다.
저희가 지금까지 이 패턴을 활용하기 어려웠던 이유는, 페이징 처리된 조회 API에서는 페이징 변수와 함께 데이터를 반환해야 했기 때문이라고 봅니다. 다만, 페이징이 필요 없는 조회 API의 경우에는 지금처럼 Result 패턴을 적용하는 게 더 깔끔하다고 생각하는데, 이에 대해서는 내일 한 번 같이 논의해보면 좋겠습니다.
There was a problem hiding this comment.
저희 딱히 컨벤션같은게 없던걸로 저도 알고있어서.. book쪽은 result dto활용 feed는 inner class를 사용하는 걸로 도메인별로 그렇게 구현되어있길래.. 도메인쪽 구현방법 따라가도록 구현했습니다
…254) - 테스트 코드가 성공하면 중복되는 데이터가 반환되는 것임 - 테스트 코드가 실패하면 현재 QueryDSL 코드로도 페이징 처리가 제대로 이루어지는 것임 - 결과는 테스트 코드 실패
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSavedListApiTest.java (4)
57-121: 정렬·메타 필드 검증 충실. 첫 페이지인 경우 nextCursor/isLast도 명시적으로 검증 권장핵심 시나리오(저장 최신순, creator/contents/like/comment, isSaved/isLiked)가 잘 커버됩니다. 응답 계약을 더 견고히 하려면, 총 개수가 페이지 크기(예: 10) 미만인 경우 nextCursor=null, isLast=true임을 함께 검증하는 것을 추천합니다.
아래와 같이 두 줄을 추가하면, 페이징 계약이 명확해집니다.
mockMvc.perform(get("/feeds/saved") .requestAttr("userId", me.getUserId())) .andExpect(status().isOk()) .andExpect(jsonPath("$.data.feedList", hasSize(2))) + .andExpect(jsonPath("$.data.nextCursor", nullValue())) + .andExpect(jsonPath("$.data.isLast", is(true))) // 저장한 최신순 -> f1 먼저 .andExpect(jsonPath("$.data.feedList[0].feedId", is(f1.getPostId().intValue())))
151-156: 주석 표현 및 들여쓰기 미세 수정 제안
- 주석의 주어가 실제 의도(i 인덱스 기준)와 혼동될 수 있습니다. “feedId” 대신 “i(인덱스)”로 명시하면 읽기 쉬워집니다.
- for 앞의 불필요한 선행 공백 1칸 제거 권장.
- // created_at 덮어쓰기 feedId가 작을수록 최신 저장순 - for (int i = 0; i < 12; i++) { + // created_at 덮어쓰기: 인덱스 i가 작을수록 최신(= 더 최근에 저장) + for (int i = 0; i < 12; i++) {
232-312: 버그 재현 테스트는 향후 ‘고정 실패’ 위험 — @disabled로 격리 + DisplayName 정리 제안이 테스트는 “컬렉션 fetch join + limit로 인한 1:N 행 폭발로 루트 엔티티가 중복되어 unique 개수가 10 미만”인 현상을 ‘재현’하는 용도입니다.
- 현재 DisplayName은 “첫 페이지 결과 개수가 10개보다 적게 반환된다”로 되어 있으나, 실제 검증은 “응답 크기는 10이지만 feedId 중복으로 distinct 개수가 10 미만”임을 확인합니다. 명칭을 일치시키는 편이 혼동이 줄어듭니다.
- 또한 이 테스트는 근본 원인이 해결되면(올바른 페이징으로 수정) 지속적으로 실패하게 됩니다. CI 안정성을 위해 @disabled로 격리하거나, 향후 버그가 해결되면 기대값을 ‘중복 없음(=distinct.size()==ids.size())’으로 바꾸는 전환 계획을 권합니다.
아래와 같이 변경을 제안합니다(일단 비활성화하고, 이슈 번호를 함께 남겨두면 좋습니다).
- @Test - @DisplayName("[깨짐 재현] 최신 저장 피드에 contents가 많으면 첫 페이지 결과 개수가 10개보다 적게 반환된다") + @Disabled("컬렉션 fetch join + limit로 인한 1:N 중복 재현 테스트 — 최적화 전까지 비활성화 (ex: THIP2025-205)") + @Test + @DisplayName("[버그 재현] 컬렉션 fetch join + limit로 1:N 중복 발생(응답 10건이나 unique는 10 미만)") void saved_feed_paging_breaks_with_many_contents() throws Exception {그리고 Disabled 사용을 위해 import 추가가 필요합니다(아래 별도 코멘트 참고).
근본 해결 방향(참고):
- 키셋 페이징: saved_feeds(created_at, saved_id) 기준으로 루트(Post/Feed) id만 먼저 페이징(서브쿼리/윈도우) → 이후 콘텐츠 컬렉션은 별도 select(in절 + batch fetch)로 조회.
- JPQL/QueryDSL에서 컬렉션 fetch join + limit 지양, 필요 시 distinct 루트 + 변환 단계에서 컬렉션 결합.
원하시면, 현재 레포지토리 구현(RepositoryImpl) 기준으로 keyset 페이징 쿼리/매퍼 스케치까지 드리겠습니다.
16-24: @disabled 사용을 위한 import 추가위 ‘버그 재현’ 테스트를 @disabled로 마킹하기 위해 아래 import를 추가하세요.
import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Disabled; import org.springframework.beans.factory.annotation.Autowired;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSavedListApiTest.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSavedListApiTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(29-413)
🔇 Additional comments (3)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSavedListApiTest.java (3)
38-43: 통합 테스트 구성(L2C) 적절@SpringBootTest + @AutoConfigureMockMvc(addFilters = false) + @transactional 조합으로 API 흐름을 안정적으로 검증하고, 보안 필터 영향도 제거해 기능 검증에 집중할 수 있게 되어 있습니다. 클래스 레벨 설정은 그대로 유지해도 좋겠습니다.
157-178: 첫 페이지 페이징 동작 검증 적절첫 페이지 응답의 크기(10), nextCursor 존재, isLast=false, 그리고 정렬 일관성(0~9 인덱스의 feedId)이 잘 검증되어 있습니다.
213-229: 커서 포맷 및 비교 연산 검증 완료
- Cursor.from(...).getLocalDateTime()가
LocalDateTime.parse(ISO_LOCAL_DATE_TIME)로 구현되어 테스트의toString()형식과 완전히 호환됩니다.FeedQueryRepositoryImpl#getSavedFeedJpaEntities에서savedFeed.createdAt.lt(lastSavedAt)(엄격한 <) 조건을 사용하고 있어, 동일 타임스탬프의 중복 조회 없이 테스트 기대대로 동작합니다.해당 부분에 추가 수정 없이 머지 가능합니다.
buzz0331
left a comment
buzz0331
left a comment
There was a problem hiding this comment.
수고하셨습니다~ 저도 성준님과 마찬가지로 쿼리 쪽에 리뷰를 좀 남겨보았는데 확인부탁드려요~
| public record BookShowSavedInfoResult( | ||
| Long bookId, | ||
| String bookTitle, | ||
| String authorName, | ||
| String publisher, | ||
| String bookImageUrl, | ||
| String isbn, | ||
| boolean isSaved | ||
| ) { | ||
| } |
There was a problem hiding this comment.
저는 inner class를 사용하는 것보다는, 지금 희진님이 해주신 것처럼 최종 adapter 계층의 response DTO가 Result를 리스트로 갖는 일급 컬렉션 형태가 헥사고날 아키텍처에 더 가깝다고 생각합니다.
저희가 지금까지 이 패턴을 활용하기 어려웠던 이유는, 페이징 처리된 조회 API에서는 페이징 변수와 함께 데이터를 반환해야 했기 때문이라고 봅니다. 다만, 페이징이 필요 없는 조회 API의 경우에는 지금처럼 Result 패턴을 적용하는 게 더 깔끔하다고 생각하는데, 이에 대해서는 내일 한 번 같이 논의해보면 좋겠습니다.
| @Operation( | ||
| summary = "저장한 책 조회", | ||
| description = "사용자가 저장한 책을 조회 합니다." | ||
| ) | ||
| @GetMapping("/books/saved") | ||
| public BaseResponse<BookShowSavedListResponse> showSavedBookList(@Parameter(hidden = true) @UserId final Long userId) { | ||
| return BaseResponse.ok(BookShowSavedListResponse.of(bookShowSavedListUseCase.getSavedBookList(userId))); | ||
| } |
There was a problem hiding this comment.
저장한 책 조회에서는 일부러 커서를 넣지 않으신건가요?
There was a problem hiding this comment.
책 조회는 무한스크롤이 요구사항이 아닌것으로 알고있습니당
| private List<SavedFeedJpaEntity> getSavedFeedJpaEntities(Long userId, LocalDateTime lastSavedAt, int size) { | ||
| List<SavedFeedJpaEntity> savedFeeds = jpaQueryFactory | ||
| .select(savedFeed).distinct() | ||
| .from(savedFeed) | ||
| .leftJoin(savedFeed.feedJpaEntity, feed).fetchJoin() | ||
| .leftJoin(feed.contentList, content).fetchJoin() | ||
| .leftJoin(feed.userJpaEntity, user).fetchJoin() | ||
| .leftJoin(user.aliasForUserJpaEntity, alias).fetchJoin() | ||
| .leftJoin(feed.bookJpaEntity, book).fetchJoin() | ||
| .where( | ||
| savedFeed.userJpaEntity.userId.eq(userId), | ||
| savedFeed.feedJpaEntity.status.eq(StatusType.ACTIVE), | ||
| savedFeed.feedJpaEntity.userJpaEntity.userId.eq(userId) | ||
| .or(savedFeed.feedJpaEntity.isPublic.eq(true)), | ||
| lastSavedAt != null ? savedFeed.createdAt.lt(lastSavedAt) : Expressions.TRUE | ||
| ) | ||
| .orderBy(savedFeed.createdAt.desc()) | ||
| .limit(size + 1) | ||
| .fetch(); | ||
| return savedFeeds; | ||
| } |
There was a problem hiding this comment.
넵넵 말씀하신대로 jpa에서 fetch join + 페이징 처리 + distinct를 함께 사용하면 limit 조건을 무시하는 이슈가 있어서 현재 구현된 코드는 사실상 limit를 빼고 조건에 맞는 모든 피드를 메모리에 올려서 subList를 통해 페이징 처리를 하는 거라고 보면 될 것 같아요! 따라서 제가 특정 책으로 작성된 피드 조회 api에서 사용했던 쿼리처럼 Content를 서브 쿼리를 통해 조회하는 것은 어떻게 생각하시나요??
다음과 같이 한번의 쿼리로 QueryProjection을 사용해서 조회하는 것이기 때문에 굳이 fetch join을 사용할 필요도 없고 메모리에 부담도 생기지 않을 것 같습니다!
/**
* 사용자가 저장한 피드를 저장순으로 조회
*/
@Override
public List<FeedQueryDto> findSavedFeedsByCreatedAt(Long userId, LocalDateTime lastSavedAt, int size) {
BooleanExpression where = savedFeed.userJpaEntity.userId.eq(userId)
.and(savedFeed.feedJpaEntity.status.eq(StatusType.ACTIVE))
.and(
savedFeed.feedJpaEntity.userJpaEntity.userId.eq(userId)
.or(savedFeed.feedJpaEntity.isPublic.eq(true))
);
if (lastSavedAt != null) {
where = where.and(savedFeed.createdAt.lt(lastSavedAt));
}
return jpaQueryFactory
.select(toSavedFeedQueryDto())
.from(savedFeed)
.join(savedFeed.feedJpaEntity, feed)
.join(feed.userJpaEntity, user)
.join(feed.bookJpaEntity, book)
.where(where)
.orderBy(savedFeed.createdAt.desc())
.limit(size + 1)
.fetch();
}
/**
* SavedFeed 전용 DTO 매핑
*/
private QFeedQueryDto toSavedFeedQueryDto() {
return new QFeedQueryDto(
feed.postId,
feed.userJpaEntity.userId,
user.nickname,
user.aliasForUserJpaEntity.imageUrl,
user.aliasForUserJpaEntity.value,
feed.createdAt,
book.isbn,
book.title,
book.authorName,
feed.content,
// Content는 N:1 방지 위해 서브쿼리 사용
JPAExpressions
.select(contentUrlAggExpr())
.from(content)
.where(content.postJpaEntity.postId.eq(feed.postId)),
feed.likeCount,
feed.commentCount,
feed.isPublic,
Expressions.nullExpression(),
savedFeed.createdAt
);
}| private List<SavedFeedJpaEntity> getSavedFeedJpaEntities(Long userId, LocalDateTime lastSavedAt, int size) { | ||
| List<SavedFeedJpaEntity> savedFeeds = jpaQueryFactory | ||
| .select(savedFeed).distinct() | ||
| .from(savedFeed) | ||
| .leftJoin(savedFeed.feedJpaEntity, feed).fetchJoin() | ||
| .leftJoin(feed.contentList, content).fetchJoin() | ||
| .leftJoin(feed.userJpaEntity, user).fetchJoin() | ||
| .leftJoin(user.aliasForUserJpaEntity, alias).fetchJoin() | ||
| .leftJoin(feed.bookJpaEntity, book).fetchJoin() | ||
| .where( | ||
| savedFeed.userJpaEntity.userId.eq(userId), | ||
| savedFeed.feedJpaEntity.status.eq(StatusType.ACTIVE), | ||
| savedFeed.feedJpaEntity.userJpaEntity.userId.eq(userId) | ||
| .or(savedFeed.feedJpaEntity.isPublic.eq(true)), | ||
| lastSavedAt != null ? savedFeed.createdAt.lt(lastSavedAt) : Expressions.TRUE | ||
| ) | ||
| .orderBy(savedFeed.createdAt.desc()) | ||
| .limit(size + 1) | ||
| .fetch(); | ||
| return savedFeeds; | ||
| } |
There was a problem hiding this comment.
이렇게 수정하면 toDto 메서드에 추가된 파라미터도 제거해도 될 것 같네요!
| @Override | ||
| @Transactional(readOnly = true) | ||
| public FeedShowSavedListResponse getSavedFeedList(Long userId, String cursor) { | ||
| // 1. 커서 생성 | ||
| Cursor nextCursor = Cursor.from(cursor, PAGE_SIZE); | ||
|
|
||
| // 2. 유저가 저장한 책 최신순으로 (페이징 처리 포함) | ||
| CursorBasedList<FeedQueryDto> result = feedQueryPort.findSavedFeedsByCreatedAt(userId, nextCursor); | ||
| Set<Long> feedIds = result.contents().stream() | ||
| .map(FeedQueryDto::feedId) | ||
| .collect(Collectors.toUnmodifiableSet()); | ||
|
|
||
| // 3. 유저가 좋아한 피드들 조회 | ||
| Set<Long> likedFeedIdsByUser = postLikeQueryPort.findPostIdsLikedByUser(feedIds, userId); | ||
|
|
||
| // 4. response 로의 매핑 | ||
| List<FeedShowSavedListResponse.FeedShowSavedInfoDto> feedList = result.contents().stream() | ||
| .map(dto -> feedQueryMapper.toFeedShowSavedListResponse(dto, likedFeedIdsByUser, userId)) | ||
| .toList(); | ||
|
|
||
| return new FeedShowSavedListResponse( | ||
| feedList, | ||
| result.nextCursor(), | ||
| !result.hasNext() | ||
| ); | ||
| } |
| List<FeedShowSavedListResponse.FeedShowSavedInfoDto> feedList = result.contents().stream() | ||
| .map(dto -> feedQueryMapper.toFeedShowSavedListResponse(dto, likedFeedIdsByUser, userId)) | ||
| .toList(); |
There was a problem hiding this comment.
이거 List까지 MapStruct를 사용해서 매핑이 가능합니다!! 딱히 가독성을 해치진 않아서 수정하진 않으셔도 될 것 같습니다! 반환 변수만 var로 수정 가능할까요??
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (1)
348-371: QFeedQueryDto 생성자에 전달하는 null은 타입 명시가 안전합니다
Expressions.nullExpression()는 비(무)타입으로 생성되어,QFeedQueryDto생성자 파라미터 타입(Boolean, LocalDateTime 등)과 불일치 시 컴파일 오류/런타임 캐스팅 문제가 생길 수 있습니다. 타입이 명확한 상수 null을 넘기는 것이 안전합니다.아래처럼 수정 제안드립니다.
- Expressions.nullExpression(), - Expressions.nullExpression() + Expressions.constant((Boolean) null), + Expressions.constant((java.time.LocalDateTime) null)
♻️ Duplicate comments (1)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (1)
405-429: 커서 안정성: savedFeed.createdAt 단일 정렬/커서로는 페이지 누락 발생 가능현재 커서가
createdAt단일 컬럼이며 정렬도 동일 기준입니다. 동일 타임스탬프가 다수 존재할 때 첫 페이지에서limit size+1로 잘려 나간 동일createdAt레코드가 다음 페이지에서createdAt < lastSavedAt조건에 의해 영원히 제외되어 “미조회 누락”이 발생합니다. 복합 커서(예:(createdAt, savedFeedId)내림차순)로 보조 정렬/커서를 도입해야 합니다.아래는 최소 변경 예시입니다(실제 PK 속성명은 엔티티에 맞게 교체 필요:
savedFeed.savedFeedId자리).- public List<FeedQueryDto> findSavedFeedsByCreatedAt(Long userId, LocalDateTime lastSavedAt, int size) { + public List<FeedQueryDto> findSavedFeedsByCreatedAt(Long userId, @jakarta.annotation.Nullable LocalDateTime lastSavedAt, @jakarta.annotation.Nullable Long lastSavedId, int size) { @@ - if (lastSavedAt != null) { - where = where.and(savedFeed.createdAt.lt(lastSavedAt)); - } + if (lastSavedAt != null) { + com.querydsl.core.types.dsl.BooleanExpression cursor = + savedFeed.createdAt.lt(lastSavedAt); + if (lastSavedId != null) { + cursor = cursor.or( + savedFeed.createdAt.eq(lastSavedAt) + .and(savedFeed.savedFeedId.lt(lastSavedId)) // FIXME: 실제 PK 속성명으로 교체 + ); + } + where = where.and(cursor); + } @@ - .orderBy(savedFeed.createdAt.desc()) + .orderBy(savedFeed.createdAt.desc(), savedFeed.savedFeedId.desc()) // FIXME: 실제 PK 속성명으로 교체추가로, 인덱스
(user_id, created_at desc, <pk> desc)를 권장합니다.
🧹 Nitpick comments (2)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (2)
422-425: alias 경로에 대한 암시적 조인 대신 명시적 조인으로 의도와 조인 타입을 고정하세요
toSavedFeedQueryDto()에서user.aliasForUserJpaEntity.*를 선택하므로 암시적 조인이 생성됩니다. 조인 타입을 명시해 N+1/의도치 않은 inner join을 예방하고 가독성을 높이는 편이 좋습니다. alias가 옵셔널이면 left join, 필수면 inner join을 사용하세요..join(savedFeed.feedJpaEntity, feed) - .join(feed.userJpaEntity, user) + .join(feed.userJpaEntity, user) + .leftJoin(user.aliasForUserJpaEntity, alias) // alias가 필수면 .join(...) 사용 .join(feed.bookJpaEntity, book)
446-451: contentUrl 집계 시 정렬 미지정 — 결정적 순서 보장을 위해 ORDER BY 고려
group_concat의 집계 결과 순서는 보장되지 않습니다. 클라이언트가 일관된 순서를 기대한다면 콘텐츠의 정의된 순서 컬럼(예: 순번, PK 등)으로 ORDER BY를 추가하는 것을 권장합니다. 예:group_concat(url ORDER BY seq)형태. 실제 컬럼명에 맞춰 적용해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java(3 hunks)src/main/java/konkuk/thip/feed/application/service/FeedShowSavedListService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/konkuk/thip/feed/application/service/FeedShowSavedListService.java
🔇 Additional comments (2)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (2)
41-41: SavedFeed Q타입 필드 추가 적절저장한 피드 조회 경로에 필요한 QSavedFeed 필드 주입 잘 추가되었습니다.
419-429: 프로젝션 기반(one-shot) 조회로 1:N 페치조인/페이징 왜곡 회피 — 굿SavedFeed 경로에서 DTO 프로젝션 + 서브쿼리로 콘텐츠를 집계하는 접근으로, 1:N fetch join + limit 조합에서 발생하는 중복/왜곡 리스크를 잘 피했습니다.
size+1전략도 적절합니다.
| Expressions.nullExpression(), | ||
| savedFeed.createdAt | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
toSavedFeedQueryDto의 isPriorityFeed 자리 null 전달도 타입을 명시하세요
여기도 Boolean 자리에 무타입 null이 전달되고 있습니다. 아래처럼 수정하면 안전합니다.
- Expressions.nullExpression(),
+ Expressions.constant((Boolean) null),📝 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.
| Expressions.nullExpression(), | |
| savedFeed.createdAt | |
| ); | |
| Expressions.constant((Boolean) null), | |
| savedFeed.createdAt |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java
around lines 454 to 456, the call currently uses Expressions.nullExpression()
for the isPriorityFeed Boolean field without a type; change it to a typed null
expression such as Expressions.nullExpression(Boolean.class) (or
Expressions.constant(Boolean.class, null)) so the null is explicitly typed as
Boolean to avoid raw-type ambiguity and ensure type safety.
hd0rable
dismissed
buzz0331’s stale review
The merge-base changed after approval.
| push: | ||
| branches: | ||
| - 'main' | ||
| - 'develop' |
3 participants
#️⃣ 연관된 이슈
📝 작업 내용
📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit