[Bug/#483] 저장 api 수정 - #485
Conversation
개요
변경사항
예상 코드 리뷰 난이도🎯 2 (단순) | ⏱️ ~10분 추천 레이블
시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/main/java/stackpot/stackpot/feed/service/FeedQueryServiceImpl.java (3)
205-207: 주석 처리된 코드 제거 권장아래 코드 블록이 주석으로 남아 있습니다. 대체 로직이 이미 적용되어 있으므로 제거하는 것이 좋습니다.
♻️ 제안
-// List<Feed> feeds = (nextCursor == null) -// ? feedRepository.findByUser_Id(userId, pageable) -// : feedRepository.findByUserIdAndFeedIdBefore(userId, nextCursor, pageable);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/stackpot/stackpot/feed/service/FeedQueryServiceImpl.java` around lines 205 - 207, The commented-out conditional assignment for feeds in FeedQueryServiceImpl (the block using feedRepository.findByUser_Id and feedRepository.findByUserIdAndFeedIdBefore with nextCursor) is obsolete and should be removed; open the method in FeedQueryServiceImpl where the variable nextCursor and List<Feed> feeds are handled and delete that commented code block so only the active logic remains (remove the lines containing feedRepository.findByUser_Id(...) and feedRepository.findByUserIdAndFeedIdBefore(...)).
129-138: [추천 개선] 스트림 내countByFeedN+1 쿼리 → 배치 조회로 전환 권장
getPreViewFeeds(Line 134),getFeedsByUserId(Line 224),getFeeds(Line 269),searchByUserIdByKeyword(Line 501) 등에서feedSaveRepository.countByFeed(feed)또는countByFeedId를 스트림 내에서 per-feed 호출하고 있어 N+1 쿼리가 발생합니다.getLikedFeedsWithPaging에서 이미countSavesByFeedIds(feedIds)로 배치 조회하는 패턴을 사용 중이므로, 다른 메서드에도 동일한 방식을 적용하는 것을 권장합니다.♻️ 개선 예시 (getPreViewFeeds 기준)
+ List<Long> feedPageIds = feedResults.stream() + .map(Feed::getFeedId) + .collect(Collectors.toList()); + Map<Long, Integer> saveCountMap = feedSaveRepository.countSavesByFeedIds(feedPageIds).stream() + .collect(Collectors.toMap( + row -> (Long) row[0], + row -> ((Long) row[1]).intValue() + )); List<FeedResponseDto.FeedDto> feedDtoList = feedResults.stream() .map(feed -> { boolean isOwner = (user != null) && Objects.equals(user.getId(), feed.getUser().getUserId()); Boolean isLiked = (isAuthenticated && userId != null) ? likedFeedIds.contains(feed.getFeedId()) : null; Boolean isSaved = (isAuthenticated && userId != null) ? savedFeedIds.contains(feed.getFeedId()) : null; - int saveCount = feedSaveRepository.countByFeed(feed); + int saveCount = saveCountMap.getOrDefault(feed.getFeedId(), 0); return feedConverter.feedDto(feed, isOwner, isLiked, isSaved, saveCount); })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/stackpot/stackpot/feed/service/FeedQueryServiceImpl.java` around lines 129 - 138, The stream mapping in feedDtoList calls feedSaveRepository.countByFeed(feed) per item causing N+1 queries; change to batch-fetch save counts before mapping: collect feedIds from feedResults, call the existing batch method (e.g., countSavesByFeedIds(feedIds) or add a repository method that returns Map<FeedId,Integer>), build a Map<feedId, saveCount>, then in the feedResults.stream().map(...) use map.get(feed.getFeedId()) (fallback 0) instead of countByFeed; apply the same pattern used in getLikedFeedsWithPaging to getPreViewFeeds, getFeedsByUserId, getFeeds, searchByUserIdByKeyword and keep calling feedConverter.feedDto(feed, isOwner, isLiked, isSaved, saveCount).
342-345: [선택적 개선] 현재 페이지 feedIds 범위로 쿼리 스코프 축소 고려
findFeedIdsByUserId는 유저의 전체 좋아요/저장 ID를 반환하지만, 실제로 필요한 것은 현재 페이지의feedIds에 해당하는 항목만입니다. 특히countSavesByFeedIds(feedIds)가 이미 스코프 쿼리 패턴을 사용하고 있으므로, 같은 방식으로 좋아요/저장 ID 조회도 최적화할 수 있습니다. 인터랙션이 많은 사용자일수록 불필요한 데이터 로드를 줄일 수 있습니다.♻️ 개선 예시
- List<Long> likedFeedIds = feedLikeRepository.findFeedIdsByUserId(user.getId()); - // 미리 저장한 피드 ID 조회 - List<Long> savedFeedIds = feedSaveRepository.findFeedIdsByUserId(user.getId()); + // 현재 페이지 feedIds 범위 내에서만 조회 + List<Long> likedFeedIds = feedLikeRepository.findFeedIdsByUserIdAndFeedIds(user.getId(), feedIds); + List<Long> savedFeedIds = feedSaveRepository.findFeedIdsByUserIdAndFeedIds(user.getId(), feedIds);(리포지토리에
findFeedIdsByUserIdAndFeedIds메서드 추가 필요)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/stackpot/stackpot/feed/service/FeedQueryServiceImpl.java` around lines 342 - 345, The current calls to feedLikeRepository.findFeedIdsByUserId and feedSaveRepository.findFeedIdsByUserId load all interactions for the user; restrict the query to only the current page's feedIds by adding and using repository methods like findFeedIdsByUserIdAndFeedIds(userId, feedIds) and findSavedFeedIdsByUserIdAndFeedIds(userId, feedIds) (or a single method name consistent with your repo), then replace the two calls in FeedQueryServiceImpl (the variables likedFeedIds and savedFeedIds) to pass the existing feedIds collection so only relevant IDs are fetched.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/java/stackpot/stackpot/feed/service/FeedQueryServiceImpl.java`:
- Around line 205-207: The commented-out conditional assignment for feeds in
FeedQueryServiceImpl (the block using feedRepository.findByUser_Id and
feedRepository.findByUserIdAndFeedIdBefore with nextCursor) is obsolete and
should be removed; open the method in FeedQueryServiceImpl where the variable
nextCursor and List<Feed> feeds are handled and delete that commented code block
so only the active logic remains (remove the lines containing
feedRepository.findByUser_Id(...) and
feedRepository.findByUserIdAndFeedIdBefore(...)).
- Around line 129-138: The stream mapping in feedDtoList calls
feedSaveRepository.countByFeed(feed) per item causing N+1 queries; change to
batch-fetch save counts before mapping: collect feedIds from feedResults, call
the existing batch method (e.g., countSavesByFeedIds(feedIds) or add a
repository method that returns Map<FeedId,Integer>), build a Map<feedId,
saveCount>, then in the feedResults.stream().map(...) use
map.get(feed.getFeedId()) (fallback 0) instead of countByFeed; apply the same
pattern used in getLikedFeedsWithPaging to getPreViewFeeds, getFeedsByUserId,
getFeeds, searchByUserIdByKeyword and keep calling feedConverter.feedDto(feed,
isOwner, isLiked, isSaved, saveCount).
- Around line 342-345: The current calls to
feedLikeRepository.findFeedIdsByUserId and
feedSaveRepository.findFeedIdsByUserId load all interactions for the user;
restrict the query to only the current page's feedIds by adding and using
repository methods like findFeedIdsByUserIdAndFeedIds(userId, feedIds) and
findSavedFeedIdsByUserIdAndFeedIds(userId, feedIds) (or a single method name
consistent with your repo), then replace the two calls in FeedQueryServiceImpl
(the variables likedFeedIds and savedFeedIds) to pass the existing feedIds
collection so only relevant IDs are fetched.
PR 타입(하나 이상의 PR 타입을 선택해주세요)
반영 브랜치
dev -> main
작업 내용
테스트 결과
ex) 베이스 브랜치에 포함되기 위한 코드는 모두 정상적으로 동작해야 합니다. 결과물에 대한 스크린샷, GIF, 혹은 라이브
Summary by CodeRabbit
버그 수정