Skip to content

refactor: Chat getMyChatRooms N+1 제거#55

Merged
seokjun01 merged 5 commits intodevelopfrom
fix/chat-getmychatrooms-nplus1
Feb 9, 2026
Merged

refactor: Chat getMyChatRooms N+1 제거#55
seokjun01 merged 5 commits intodevelopfrom
fix/chat-getmychatrooms-nplus1

Conversation

@seokjun01
Copy link
Copy Markdown
Collaborator

@seokjun01 seokjun01 commented Feb 8, 2026

📌 작업한 내용

  • getMyChatRooms에서 채팅방별 findLastMessageAt 반복 호출을 제거하고 배치 조회 방식으로 변경했습니다.
  • ChatMessageRepositoryPortfindLastMessageAtByChatRoomIds(List<Long>)를 추가했습니다.
  • ChatMessageJpaRepositoryIN + GROUP BY + MAX(createdAt) JPQL과 projection을 추가했습니다.
  • ChatMessageRepositoryImpl에서 배치 조회 결과를 Map<chatRoomId, lastMessageAt>으로 변환하도록 구현했습니다.
  • 변경 파일
    • src/main/java/tave/crezipsa/crezipsa/application/chat/usecase/ChatUseCaseImpl.java
    • src/main/java/tave/crezipsa/crezipsa/domain/chat/port/ChatMessageRepositoryPort.java
    • src/main/java/tave/crezipsa/crezipsa/infrastructure/chat/repository/ChatMessageJpaRepository.java
    • src/main/java/tave/crezipsa/crezipsa/infrastructure/chat/repository/ChatMessageRepositoryImpl.java

🔍 참고 사항

  • 쿼리 패턴: 1회 채팅방 목록 조회 + 1회 마지막 메시지 집계 조회로 N+1을 제거했습니다.
  • chatRoomIds가 비어있는 경우 emptyMap을 반환하도록 방어 로직을 넣었습니다.
  • ./gradlew test 로컬 실행 성공했습니다.

🖼️ 스크린샷

  • UI 변경 없음

🔗 관련 이슈

✅ 체크리스트

  • 로컬에서 빌드 및 테스트 완료
  • 코드 리뷰 반영 완료
  • 문서화 필요 여부 확인

Summary by CodeRabbit

  • Performance Improvements
    • Faster chat room list loading by fetching last-message timestamps in batch, reducing load time and database queries.
  • Bug Fixes
    • Fixed chat room title resolution and ensured an empty chat-room list returns cleanly when no rooms exist, preventing errors and improving stability.

- ChatMessageRepositoryPort에 findLastMessageAtByChatRoomIds(List<Long>) 추가\n- UseCase가 방별 단건 조회 대신 배치 조회 결과(Map)를 사용할 수 있도록 계약 확장
- chatRoomId IN + GROUP BY + MAX(createdAt) 형태의 배치 조회 쿼리 추가\n- projection 인터페이스(ChatRoomLastMessageAtProjection)로 roomId/lastMessageAt 매핑
- findLastMessageAtByChatRoomIds 구현 추가\n- 빈 chatRoomIds 입력 시 emptyMap 반환 가드 적용\n- projection 리스트를 Collectors.toMap으로 roomId -> lastMessageAt 구조로 변환
- 채팅방 목록 조회 후 roomIds를 추출해 마지막 메시지 시각을 1회 배치 조회\n- 기존 room별 findLastMessageAt 호출 제거\n- roomId 기준 Map 조회로 ChatRoomListResponse를 구성하도록 변경
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 8, 2026

📝 Walkthrough

Walkthrough

Implements batch retrieval of last-message timestamps for multiple chat rooms, refactors chat room listing to use the batch API, fixes a title-resolution method name, and returns an empty list early when a user has no rooms.

Changes

Cohort / File(s) Summary
Use case / Chat listing
src/main/java/tave/crezipsa/crezipsa/application/chat/usecase/ChatUseCaseImpl.java
Refactors getMyChatRooms to early-return empty list when no rooms, compute room IDs, and use the new batch timestamp map instead of per-room lookups; fixes call to resolveChatRoomTitle (removed typo method).
Port / API
src/main/java/tave/crezipsa/crezipsa/domain/chat/port/ChatMessageRepositoryPort.java
Adds Map<Long, LocalDateTime> findLastMessageAtByChatRoomIds(List<Long> chatRoomIds) to the repository port interface.
JPA repository
src/main/java/tave/crezipsa/crezipsa/infrastructure/chat/repository/ChatMessageJpaRepository.java
Adds ChatRoomLastMessageAtProjection projection and @Query method findLastMessageAtByChatRoomIds returning projections of chatRoomId and lastMessageAt.
Repository implementation
src/main/java/tave/crezipsa/crezipsa/infrastructure/chat/repository/ChatMessageRepositoryImpl.java
Implements the new port method, handling null/empty inputs and converting projection list to Map<Long, LocalDateTime>.

Sequence Diagram(s)

sequenceDiagram
  participant UseCase as ChatUseCaseImpl
  participant Port as ChatMessageRepositoryPort
  participant Impl as ChatMessageRepositoryImpl
  participant JPA as ChatMessageJpaRepository
  participant DB as Database

  UseCase->>Port: findLastMessageAtByChatRoomIds(roomIds)
  Port->>Impl: findLastMessageAtByChatRoomIds(roomIds)
  Impl->>JPA: findLastMessageAtByChatRoomIds(roomIds)
  JPA->>DB: execute grouped MAX(created_at) query
  DB-->>JPA: projection list (chatRoomId, lastMessageAt)
  JPA-->>Impl: projection list
  Impl-->>Port: Map<roomId, lastMessageAt>
  Port-->>UseCase: Map<roomId, lastMessageAt>
  UseCase->>UseCase: build ChatRoomListResponse using map lookups
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through code with nimble paws,
Collected timestamps, fixed a typo's flaws.
Rooms now ask once, not one-by-one,
My queries prance beneath the sun.
🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: refactoring to remove N+1 query pattern in getMyChatRooms method by batch loading last message timestamps.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/chat-getmychatrooms-nplus1

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@seokjun01 seokjun01 requested a review from jinisim February 8, 2026 13:14
@seokjun01 seokjun01 self-assigned this Feb 8, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@src/main/java/tave/crezipsa/crezipsa/application/chat/usecase/ChatUseCaseImpl.java`:
- Around line 67-68: Replace the fully-qualified type usage in ChatUseCaseImpl
by adding an import for java.time.LocalDateTime at the top of the file and then
change the declaration "Map<Long, java.time.LocalDateTime> lastMessageAtByRoomId
= chatMessageRepository.findLastMessageAtByChatRoomIds(roomIds);" to use the
simple class name LocalDateTime; keep the variable name lastMessageAtByRoomId
and the call to chatMessageRepository.findLastMessageAtByChatRoomIds(roomIds)
unchanged.
🧹 Nitpick comments (1)
src/main/java/tave/crezipsa/crezipsa/application/chat/usecase/ChatUseCaseImpl.java (1)

57-77: Add @Transactional(readOnly = true) for this read-only method.

getMyChatRooms only performs reads. Marking it readOnly = true enables Hibernate dirty-checking optimizations and can route to a read replica if configured. The class-level @Transactional currently applies a read-write transaction.

 	`@Override`
+	`@Transactional`(readOnly = true)
 	public List<ChatRoomListResponse> getMyChatRooms(Long userId) {

As per coding guidelines: "Prefer @Transactional(readOnly = true) for read-only queries".

Comment on lines +67 to +68
Map<Long, java.time.LocalDateTime> lastMessageAtByRoomId =
chatMessageRepository.findLastMessageAtByChatRoomIds(roomIds);
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Use an import instead of the fully qualified java.time.LocalDateTime.

LocalDateTime is already imported in the other files in this PR. Add an import at the top and use the simple class name here.

Proposed fix

Add to imports:

import java.time.LocalDateTime;
-		Map<Long, java.time.LocalDateTime> lastMessageAtByRoomId =
+		Map<Long, LocalDateTime> lastMessageAtByRoomId =
🤖 Prompt for AI Agents
In
`@src/main/java/tave/crezipsa/crezipsa/application/chat/usecase/ChatUseCaseImpl.java`
around lines 67 - 68, Replace the fully-qualified type usage in ChatUseCaseImpl
by adding an import for java.time.LocalDateTime at the top of the file and then
change the declaration "Map<Long, java.time.LocalDateTime> lastMessageAtByRoomId
= chatMessageRepository.findLastMessageAtByChatRoomIds(roomIds);" to use the
simple class name LocalDateTime; keep the variable name lastMessageAtByRoomId
and the call to chatMessageRepository.findLastMessageAtByChatRoomIds(roomIds)
unchanged.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다른 점 지적해줄거없어?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

echo "=== Checking usage of resolveChatRooomTitle (with typo) ==="
rg -n 'resolveChatRooomTitle' --type java

echo -e "\n=== Checking usage of resolveChatRoomTitle (correct spelling) ==="
rg -n 'resolveChatRoomTitle' --type java

echo -e "\n=== Checking usage of validateChatRoomOwner ==="
rg -n 'validateChatRoomOwner' --type java

echo -e "\n=== Showing the full methods in ChatUseCaseImpl.java ==="
cat src/main/java/tave/crezipsa/crezipsa/application/chat/usecase/ChatUseCaseImpl.java | grep -A 5 "resolveChatR"

Length of output: 1234


@seokjun01 네, 추가로 발견한 문제들이 있습니다:

🐛 심각한 문제

1. 중복 메서드와 오타 (Line 97, 117)

resolveChatRooomTitle (o가 3개)와 resolveChatRoomTitle (정상 철자) 두 메서드가 중복으로 존재합니다:

// Line 97 - 오타가 있지만 실제로 사용 중 (line 35에서 호출)
private String resolveChatRooomTitle(String title) {
    return (title == null || title.isBlank())
        ? "새 채팅"
        : title;
}

// Line 117 - 정상 철자이지만 호출되지 않음 (dead code)
private String resolveChatRoomTitle(String title) {
    return (title == null || title.isBlank())
        ? "새 채팅"
        : title;
}

권장사항:

  • Line 117의 올바른 철자 메서드를 삭제
  • Line 97의 메서드 이름을 resolveChatRoomTitle로 수정

🧹 Dead Code

2. 사용되지 않는 메서드 (Line 110)

validateChatRoomOwner 메서드가 정의되어 있지만 코드베이스 어디에서도 호출되지 않습니다:

private void validateChatRoomOwner(Long chatRoomId, Long userId) {
    if (!chatRoomRepository
        .findByIdAndUserId(chatRoomId, userId)
        .isPresent()) {
        throw new CommonException(ErrorCode.CHAT_ROOM_NOT_FOUND);
    }
}

권장사항: 사용하지 않는다면 삭제하는 것이 좋습니다.

📝 스타일 개선

3. LocalDateTime import (이미 언급한 이슈)

  • Line 67: java.time.LocalDateTimeLocalDateTime + import 추가

🧬 Code Graph Analysis Results

[src/main/java/tave/crezipsa/crezipsa/global/exception/model/CommonException.java] (lines 5-17)

public class CommonException extends RuntimeException {

	private final BaseErrorCode errorCode;

	public CommonException(BaseErrorCode errorCode) {
		super(errorCode.getMessage());
		this.errorCode = errorCode;
	}

	public BaseErrorCode getErrorCode() {
		return errorCode;
	}
}

[src/main/java/tave/crezipsa/crezipsa/application/storyboard/usecase/StoryboardUsecaseImpl.java] (lines 24-203)

`@Service`
`@RequiredArgsConstructor`
`@Transactional`
public class StoryboardUsecaseImpl implements StoryboardUsecase {

	private final StoryboardRepositoryPort storyboardRepository;
	private final StoryboardCutRepositoryPort storyboardCutRepository;
	private final ChatMessageRepositoryPort chatMessageRepository;

	`@Override`
	public StoryboardEditorResponse create(Long userId, CreateStoryboardRequest request) {
		Long chatMessageId = request.chatMessageId();

		// 채팅에서 편집 진입한 경우만 검증
		if (chatMessageId != null) {
			ChatMessage message = chatMessageRepository.findById(chatMessageId);
			if (message == null) {
				throw new CommonException(ErrorCode.CHAT_NOT_FOUND);
			}
			if (message.getSenderType() != ChatMessage.SenderType.AI) {
				throw new CommonException(ErrorCode.INVALID_SENDER_TYPE);
			}
		}

		// 문서 생성 (스토리보드)
		Storyboard saved = storyboardRepository.save(
			Storyboard.create(userId, request.title(), chatMessageId)
		);

		// 기본 컷 1개 자동 생성
		int order = storyboardCutRepository.findNextOrder(saved.getId());
		StoryboardCut firstCut = storyboardCutRepository.save(
			StoryboardCut.create(saved.getId(), order)
		);

		return new StoryboardEditorResponse(
			saved.getId(),
			saved.getTitle(),
			saved.getSourceChatMessageId(),
			saved.getCreatedAt(),
			List.of(toCutResponse(firstCut))
		);
	}

	`@Override`
	`@Transactional`(readOnly = true)
	public StoryboardEditorResponse get(Long userId, Long storyboardId) {
		Storyboard sb = storyboardRepository.findById(storyboardId);
		if (sb == null || !sb.getUserId().equals(userId)) {
			throw new CommonException(ErrorCode.STORYBOARD_NOT_FOUND);
		}

		List<StoryboardCutResponse> cuts = storyboardCutRepository.findByStoryboardId(storyboardId)
			.stream()
			.map(this::toCutResponse)
			.toList();

		return new StoryboardEditorResponse(
			sb.getId(),
			sb.getTitle(),
			sb.getSourceChatMessageId(),
			sb.getCreatedAt(),
			cuts
		);
	}

	`@Override`
	`@Transactional`(readOnly = true)
	public List<StoryboardSummaryResponse> getMyList(Long userId) {
		return storyboardRepository.findByUserId(userId)
			.stream()
			.map(sb -> new StoryboardSummaryResponse(sb.getId(), sb.getTitle(), sb.getCreatedAt()))
			.toList();
	}

	`@Override`
	public void updateTitle(Long userId, Long storyboardId, UpdateStoryboardTitleRequest request) {
		Storyboard sb = getOwnedStoryboardOrThrow(userId, storyboardId);

		String title = request.title();
		if (title == null || title.isBlank()) {
			throw new CommonException(ErrorCode.INVALID_INPUT_VALUE);
		}

		storyboardRepository.save(sb.withTitle(title));
	}

	`@Override`
	public StoryboardCutResponse addCut(Long userId, Long storyboardId) {
		getOwnedStoryboardOrThrow(userId, storyboardId);

		int order = storyboardCutRepository.findNextOrder(storyboardId);
		StoryboardCut cut = storyboardCutRepository.save(
			StoryboardCut.create(storyboardId, order)
		);

		return toCutResponse(cut);
	}

	`@Override`
	public StoryboardCutResponse updateCut(Long userId, Long cutId, UpdateStoryboardCutRequest request) {
		StoryboardCut cut = getCutOrThrow(cutId);
		getOwnedStoryboardByCutOrThrow(userId, cut);

		StoryboardCut updated = StoryboardCut.builder()
			.id(cut.getId())
			.storyboardId(cut.getStoryboardId())
			.order(cut.getOrder())
			.cutComposition(request.cutComposition() != null ? request.cutComposition() : cut.getCutComposition())
			.script(request.script() != null ? request.script() : cut.getScript())
			.caption(request.caption() != null ? request.caption() : cut.getCaption())
			.etc(request.etc() != null ? request.etc() : cut.getEtc())
			.build();

		return toCutResponse(storyboardCutRepository.save(updated));

	}

	`@Override`
	public void deleteCut(Long userId, Long cutId) {

		StoryboardCut cut = storyboardCutRepository.findById(cutId);
		if (cut == null) {
			throw new CommonException(ErrorCode.STORYBOARD_CUT_NOT_FOUND);
		}

		Storyboard sb = storyboardRepository.findById(cut.getStoryboardId());
		if (sb == null || !sb.getUserId().equals(userId)) {
			throw new CommonException(ErrorCode.STORYBOARD_NOT_FOUND);
		}

		storyboardCutRepository.deleteById(cutId);
	}

	`@Override`
	public void deleteStoryboard(Long userId, Long storyboardId) {

		Storyboard sb = storyboardRepository.findById(storyboardId);
		if (sb == null || !sb.getUserId().equals(userId)) {
			throw new CommonException(ErrorCode.STORYBOARD_NOT_FOUND);
		}

		storyboardRepository.deleteById(storyboardId);
	}

	private StoryboardCutResponse toCutResponse(StoryboardCut c) {
		return new StoryboardCutResponse(
			c.getId(),
			c.getOrder(),
			c.getCutComposition(),
			c.getScript(),
			c.getCaption(),
			c.getEtc()
		);
	}
	private Storyboard getOwnedStoryboardOrThrow(Long userId, Long storyboardId) {
		Storyboard sb = storyboardRepository.findById(storyboardId);
		if (sb == null || !sb.getUserId().equals(userId)) {
			throw new CommonException(ErrorCode.STORYBOARD_NOT_FOUND);
		}
		return sb;
	}

	private StoryboardCut getCutOrThrow(Long cutId) {
		StoryboardCut cut = storyboardCutRepository.findById(cutId);
		if (cut == null) {
			throw new CommonException(ErrorCode.STORYBOARD_CUT_NOT_FOUND);
		}
		return cut;
	}

	private Storyboard getOwnedStoryboardByCutOrThrow(Long userId, StoryboardCut cut) {
		Storyboard sb = storyboardRepository.findById(cut.getStoryboardId());
		if (sb == null || !sb.getUserId().equals(userId)) {
			throw new CommonException(ErrorCode.STORYBOARD_NOT_FOUND);
		}
		return sb;
	}

}

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@src/main/java/tave/crezipsa/crezipsa/application/chat/usecase/ChatUseCaseImpl.java`:
- Around line 57-77: Add a method-level `@Transactional`(readOnly = true)
annotation to the getMyChatRooms method in ChatUseCaseImpl to override the
class-level transactional default; import
org.springframework.transaction.annotation.Transactional if missing so the
read-only optimization applies for this pure-read method (keep the existing
logic in getMyChatRooms, just add the annotation above the method signature).
🧹 Nitpick comments (1)
src/main/java/tave/crezipsa/crezipsa/application/chat/usecase/ChatUseCaseImpl.java (1)

79-95: Also add @Transactional(readOnly = true) to getChatDetail.

Same rationale — this is a pure read method.

Proposed fix
 	`@Override`
+	`@Transactional`(readOnly = true)
 	public ChatMessageResponse getChatDetail(Long userId, Long chatRoomId) {

As per coding guidelines, src/main/java/**/application/**/usecase/*.java: "Prefer @Transactional(readOnly = true) for read-only queries".

Comment on lines 57 to 77
@Override
public List<ChatRoomListResponse> getMyChatRooms(Long userId) {
return chatRoomRepository.findByUserId(userId).stream()
List<ChatRoom> rooms = chatRoomRepository.findByUserId(userId);
if (rooms.isEmpty()) {
return List.of();
}

List<Long> roomIds = rooms.stream()
.map(ChatRoom::getId)
.toList();
Map<Long, java.time.LocalDateTime> lastMessageAtByRoomId =
chatMessageRepository.findLastMessageAtByChatRoomIds(roomIds);

return rooms.stream()
.map(room -> new ChatRoomListResponse(
room.getId(),
room.getTitle(),
chatMessageRepository
.findLastMessageAt(room.getId())
.orElse(null)
lastMessageAtByRoomId.get(room.getId())
))
.toList();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

N+1 elimination looks correct — nice refactor.

The batch fetch with early return for empty rooms is clean and achieves the stated goal. One guideline concern:

Add @Transactional(readOnly = true) to this read-only method. The class-level @Transactional defaults to readOnly = false, which is suboptimal for pure reads — it prevents the persistence context and database driver from applying read-only optimizations.

Proposed fix
 	`@Override`
+	`@Transactional`(readOnly = true)
 	public List<ChatRoomListResponse> getMyChatRooms(Long userId) {

As per coding guidelines, src/main/java/**/application/**/usecase/*.java: "Prefer @Transactional(readOnly = true) for read-only queries".

🤖 Prompt for AI Agents
In
`@src/main/java/tave/crezipsa/crezipsa/application/chat/usecase/ChatUseCaseImpl.java`
around lines 57 - 77, Add a method-level `@Transactional`(readOnly = true)
annotation to the getMyChatRooms method in ChatUseCaseImpl to override the
class-level transactional default; import
org.springframework.transaction.annotation.Transactional if missing so the
read-only optimization applies for this pure-read method (keep the existing
logic in getMyChatRooms, just add the annotation above the method signature).

@seokjun01 seokjun01 merged commit 1236487 into develop Feb 9, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants