Merged
Conversation
This reverts commit 41e413fca9f00da7b226218afd8fc2267065cad5.
…to feat/29-note-post
# Conflicts: # src/main/java/com/proovy/domain/note/controller/NoteController.java
📝 WalkthroughWalkthrough대화(Conversation) 도메인 엔티티·리포지토리와 메시지 관련 엔티티들이 추가됐고, 노트 생성/목록 조회 컨트롤러·서비스·DTO가 도입되었습니다. AssetRepository에 사용자·노트 기반 집계/조회 메서드가 추가되고, 중복 상수 제거 및 ErrorCode 확장이 이루어졌습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Controller as NoteController
participant Service as NoteServiceImpl
participant AssetRepo as AssetRepository
participant NoteRepo as NoteRepository
participant ConvRepo as ConversationRepository
participant MessageRepo as MessageRepository
participant DB as Database
User->>Controller: POST /api/notes (CreateNoteRequest)
Controller->>Service: createNote(userId, request)
Service->>AssetRepo: findAllByIdInAndUserId(assetIds, userId)
AssetRepo->>DB: SELECT assets WHERE id IN (...) AND user_id=...
DB-->>AssetRepo: assets
Service->>NoteRepo: save(note)
NoteRepo->>DB: INSERT note
DB-->>NoteRepo: note(id)
Service->>ConvRepo: save(conversation)
ConvRepo->>DB: INSERT conversation
DB-->>ConvRepo: conversation(id)
Service->>MessageRepo: save(userMessage)
MessageRepo->>DB: INSERT message (USER)
DB-->>MessageRepo: userMessage(id)
Service->>MessageRepo: save(assistantMessage)
MessageRepo->>DB: INSERT message (ASSISTANT)
DB-->>MessageRepo: assistantMessage(id)
Service-->>Controller: CreateNoteResponse
Controller-->>User: ApiResponse<CreateNoteResponse>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
🤖 Fix all issues with AI agents
In `@src/main/java/com/proovy/domain/note/service/NoteServiceImpl.java`:
- Around line 48-50: In NoteServiceImpl.createNote the call
request.firstMessage().length() can throw NPE if firstMessage() is null; fix by
making the log null-safe—either check request.firstMessage() for null before
calling length or replace the expression with a null-safe alternative (e.g., use
a conditional to compute length as 0 when null or log the message with a %s
placeholder without invoking length). Update the log.info call in createNote to
use that null-safe expression so no .length() is invoked on a null value.
- Around line 251-259: The code is causing N+1 queries because buildNoteDto
calls conversationRepository.countByNoteId(...) and
assetRepository.countByNoteId(...) per note and also ignores the fetched
planType by hardcoding conversationLimit; fix by adding batch repository methods
(e.g., countConversationsByNoteIds and countAssetsByNoteIds) that return counts
for a List<Long> noteIds, call those once using
notePage.getContent().stream().map(Note::getId).toList() to build Map<Long,Long>
conversationCounts and assetCounts, update buildNoteDto (or overload it) to
accept counts (e.g., buildNoteDto(note, conversationCounts.get(note.getId()),
assetCounts.get(note.getId()), conversationLimit)) and use the retrieved
planType to compute conversationLimit dynamically before mapping the page
content.
🧹 Nitpick comments (6)
src/main/java/com/proovy/domain/note/dto/response/NoteListResponse.java (1)
14-24: 정수 필드 nullable 여부 확인
conversationCount,assetCount등의 카운트 필드가Integer로 선언되어 있어 null이 허용됩니다. 이 값들이 항상 존재해야 한다면int프리미티브 타입 사용을 고려해보세요. null이 의미 있는 상태(예: 아직 계산되지 않음)를 나타낸다면 현재Integer사용이 적절합니다.src/main/java/com/proovy/domain/note/dto/response/CreateNoteResponse.java (1)
33-33: 불일치하는 import 사용
java.util.List가 이미 line 4에서 import되어 있으므로, line 24의List<MentionedAssetDto>와 일관성을 위해 단순히List<String>을 사용하세요.♻️ 수정 제안
public record AssistantMessageDto( Long messageId, String content, - java.util.List<String> usedTools, + List<String> usedTools, String status, LocalDateTime createdAt ) {src/main/java/com/proovy/domain/note/dto/request/CreateNoteRequest.java (1)
13-15: 리스트 크기 제한 고려
mentionedAssetIds와mentionedToolCodes리스트에 크기 제한이 없습니다. 악의적인 요청으로 매우 큰 리스트가 전송될 경우 성능 문제가 발생할 수 있습니다.♻️ 리스트 크기 제한 추가 예시
+import jakarta.validation.constraints.Size; + public record CreateNoteRequest( `@NotBlank`(message = "메시지 내용을 입력해주세요.") `@Size`(max = 5000, message = "메시지는 5000자 이내로 입력해주세요.") String firstMessage, + `@Size`(max = 20, message = "첨부 자산은 최대 20개까지 가능합니다.") List<Long> mentionedAssetIds, + `@Size`(max = 10, message = "도구는 최대 10개까지 선택 가능합니다.") List<String> mentionedToolCodes )src/main/java/com/proovy/domain/note/controller/NoteController.java (1)
100-112: 입력값 검증 및 응답 일관성 검토 필요
페이지네이션 파라미터 검증:
page와size에 음수 값이 전달될 경우 예기치 않은 동작이 발생할 수 있습니다.@Min(0)등의 검증 어노테이션 추가를 권장합니다.응답 메서드 불일치:
createNote는ApiResponse.of("COMMON201", ...)를 사용하고,getNoteList는ApiResponse.success(...)를 사용합니다. 일관된 패턴 사용을 권장합니다.♻️ 입력값 검증 추가 제안
+import jakarta.validation.constraints.Min; +import jakarta.validation.constraints.Max; public ApiResponse<NoteListResponse> getNoteList( `@AuthenticationPrincipal` UserPrincipal userPrincipal, `@Parameter`(description = "페이지 번호 (0부터 시작)", example = "0") - `@RequestParam`(defaultValue = "0") int page, + `@RequestParam`(defaultValue = "0") `@Min`(0) int page, `@Parameter`(description = "페이지당 노트 수 (최대 50)", example = "20") - `@RequestParam`(defaultValue = "20") int size, + `@RequestParam`(defaultValue = "20") `@Min`(1) `@Max`(50) int size, `@Parameter`(description = "정렬 기준", example = "lastUsedAt,desc") `@RequestParam`(defaultValue = "lastUsedAt,desc") String sort )src/main/java/com/proovy/domain/note/service/NoteServiceImpl.java (2)
123-152: MessageTool 생성 로직 중복User Message와 Assistant Message에 대한
MessageTool생성 로직이 거의 동일합니다. 헬퍼 메서드로 추출하면 중복을 줄이고 유지보수성을 높일 수 있습니다.♻️ 헬퍼 메서드 추출 제안
private void saveMessageTools(Message message, List<String> toolCodes) { if (toolCodes != null && !toolCodes.isEmpty()) { List<MessageTool> messageTools = toolCodes.stream() .map(toolCode -> MessageTool.builder() .message(message) .toolCode(toolCode) .build()) .toList(); messageToolRepository.saveAll(messageTools); } }사용:
-// 9. User Message의 Tool 연결 -if (request.mentionedToolCodes() != null && !request.mentionedToolCodes().isEmpty()) { - List<MessageTool> messageTools = request.mentionedToolCodes().stream() - .map(toolCode -> MessageTool.builder() - .message(userMessage) - .toolCode(toolCode) - .build()) - .toList(); - messageToolRepository.saveAll(messageTools); -} +// 9. User Message의 Tool 연결 +saveMessageTools(userMessage, request.mentionedToolCodes());
224-231: 하드코딩된 대화 제한 값Line 228에서
conversationLimit을 50으로 하드코딩하고 있습니다. Line 62에서 이미planType을 조회하고 있으므로, 플랜 기반으로 동적인 값을 사용하는 것이 일관성 있습니다.♻️ 수정 제안
- return new CreateNoteResponse( - note.getId(), - note.getTitle(), - "USER", // 현재는 AI 사용하지 않으므로 USER로 표시 - 50, // 기본 대화 제한 - firstConversationDto, - note.getCreatedAt() - ); + return new CreateNoteResponse( + note.getId(), + note.getTitle(), + "USER", // 현재는 AI 사용하지 않으므로 USER로 표시 + planType.getConversationLimit(), + firstConversationDto, + note.getCreatedAt() + );이를 위해
buildCreateNoteResponse메서드에planType파라미터를 추가하거나,createNote메서드 내에서 직접 값을 참조해야 합니다.
src/main/java/com/proovy/domain/note/service/NoteServiceImpl.java
Outdated
Show resolved
Hide resolved
Comment on lines
+251
to
+259
| // 5. 사용자의 활성 플랜 조회 (대화 제한 수 계산용) | ||
| com.proovy.domain.user.entity.PlanType planType = userPlanRepository.findActivePlanTypeByUserId(userId) | ||
| .orElse(com.proovy.domain.user.entity.PlanType.FREE); | ||
| int conversationLimit = 50; // 기본값 | ||
|
|
||
| // 6. DTO 변환 | ||
| List<NoteListResponse.NoteDto> noteDtos = notePage.getContent().stream() | ||
| .map(note -> buildNoteDto(note, conversationLimit)) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
N+1 쿼리 성능 문제 및 미사용 변수
두 가지 이슈가 있습니다:
-
N+1 쿼리 문제:
buildNoteDto내에서 각 노트마다conversationRepository.countByNoteId()와assetRepository.countByNoteId()를 호출하여, 페이지 크기가 50일 경우 최대 100개의 추가 쿼리가 발생합니다. -
미사용 변수:
planType을 조회하지만 실제로 사용하지 않고conversationLimit을 50으로 하드코딩하고 있습니다.
♻️ N+1 쿼리 해결을 위한 개선 방안
배치 쿼리로 한 번에 카운트를 조회하는 방식을 권장합니다:
// Repository에 배치 조회 메서드 추가
`@Query`("SELECT n.id, COUNT(c) FROM Note n LEFT JOIN Conversation c ON c.note = n WHERE n.id IN :noteIds GROUP BY n.id")
List<Object[]> countConversationsByNoteIds(`@Param`("noteIds") List<Long> noteIds);
// Service에서 사용
List<Long> noteIds = notePage.getContent().stream().map(Note::getId).toList();
Map<Long, Long> conversationCounts = conversationRepository.countConversationsByNoteIds(noteIds)
.stream().collect(Collectors.toMap(arr -> (Long) arr[0], arr -> (Long) arr[1]));또한 planType을 활용하여 conversationLimit을 동적으로 설정하세요:
-int conversationLimit = 50; // 기본값
+int conversationLimit = planType.getConversationLimit();🤖 Prompt for AI Agents
In `@src/main/java/com/proovy/domain/note/service/NoteServiceImpl.java` around
lines 251 - 259, The code is causing N+1 queries because buildNoteDto calls
conversationRepository.countByNoteId(...) and assetRepository.countByNoteId(...)
per note and also ignores the fetched planType by hardcoding conversationLimit;
fix by adding batch repository methods (e.g., countConversationsByNoteIds and
countAssetsByNoteIds) that return counts for a List<Long> noteIds, call those
once using notePage.getContent().stream().map(Note::getId).toList() to build
Map<Long,Long> conversationCounts and assetCounts, update buildNoteDto (or
overload it) to accept counts (e.g., buildNoteDto(note,
conversationCounts.get(note.getId()), assetCounts.get(note.getId()),
conversationLimit)) and use the retrieved planType to compute conversationLimit
dynamically before mapping the page content.
Collaborator
Author
N+1 문제 수정 이후에도 동일하게 동작하는 것 확인했습니다.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/com/proovy/domain/note/service/NoteServiceImpl.java`:
- Around line 171-178: generateSimpleTitle currently calls
firstMessage.replaceAll(...) without a null check, which can throw a
NullPointerException when firstMessage is null (createNote has already guarded
some paths). Fix generateSimpleTitle by adding a null check at the start of the
method (in method generateSimpleTitle) and handle null safely (e.g., return an
empty string or a default title) before calling replaceAll/trim/substring so no
NPE occurs; update callers if they expect a non-null return.
🧹 Nitpick comments (5)
src/main/java/com/proovy/domain/conversation/repository/ConversationRepository.java (1)
23-27: 타입 안전하지 않은 반환 타입 및 Javadoc 불일치
List<Map<String, Object>>반환 타입은 서비스 레이어에서 수동 캐스팅이 필요하고 컴파일 타임 타입 검사가 불가능합니다. 또한 Javadoc에는Map<노트ID, 대화개수>를 반환한다고 명시되어 있지만, 실제로는List<Map<String, Object>>를 반환합니다.Spring Data JPA projection interface 사용을 권장합니다.
♻️ Projection 인터페이스를 사용한 타입 안전 개선
// Projection 인터페이스 정의 (별도 파일 또는 Repository 내부) public interface NoteCountProjection { Long getNoteId(); Long getCount(); }- /** - * 여러 노트의 대화 개수를 한 번에 조회 (배치 쿼리) - * `@param` noteIds 노트 ID 목록 - * `@return` Map<노트ID, 대화개수> - */ - `@Query`("SELECT c.note.id AS noteId, COUNT(c) AS count " + - "FROM Conversation c " + - "WHERE c.note.id IN :noteIds " + - "GROUP BY c.note.id") - List<Map<String, Object>> countByNoteIdIn(`@Param`("noteIds") List<Long> noteIds); + /** + * 여러 노트의 대화 개수를 한 번에 조회 (배치 쿼리) + * `@param` noteIds 노트 ID 목록 + * `@return` List<NoteCountProjection> 노트별 대화 개수 목록 + */ + `@Query`("SELECT c.note.id AS noteId, COUNT(c) AS count " + + "FROM Conversation c " + + "WHERE c.note.id IN :noteIds " + + "GROUP BY c.note.id") + List<NoteCountProjection> countByNoteIdIn(`@Param`("noteIds") List<Long> noteIds);src/main/java/com/proovy/domain/asset/repository/AssetRepository.java (1)
46-55: ConversationRepository와 동일한 타입 안전성 문제
ConversationRepository.countByNoteIdIn과 동일하게List<Map<String, Object>>반환 타입을 사용하고 있습니다. 두 Repository에서 공통 projection 인터페이스를 사용하면 일관성과 타입 안전성을 모두 확보할 수 있습니다.♻️ 공통 Projection 인터페이스 적용
- /** - * 여러 노트의 자산 개수를 한 번에 조회 (배치 쿼리) - * `@param` noteIds 노트 ID 목록 - * `@return` Map<노트ID, 자산개수> - */ - `@Query`("SELECT a.noteId AS noteId, COUNT(a) AS count " + - "FROM Asset a " + - "WHERE a.noteId IN :noteIds " + - "GROUP BY a.noteId") - List<Map<String, Object>> countByNoteIdIn(`@Param`("noteIds") List<Long> noteIds); + /** + * 여러 노트의 자산 개수를 한 번에 조회 (배치 쿼리) + * `@param` noteIds 노트 ID 목록 + * `@return` List<NoteCountProjection> 노트별 자산 개수 목록 + */ + `@Query`("SELECT a.noteId AS noteId, COUNT(a) AS count " + + "FROM Asset a " + + "WHERE a.noteId IN :noteIds " + + "GROUP BY a.noteId") + List<NoteCountProjection> countByNoteIdIn(`@Param`("noteIds") List<Long> noteIds);src/main/java/com/proovy/domain/note/service/NoteServiceImpl.java (3)
224-232: 하드코딩된 값들 개선 필요
"USER"와 대화 제한50이 하드코딩되어 있습니다. 추후 AI 제목 생성 기능 추가 시titleSource값이 변경될 수 있고, 대화 제한도 요금제에 따라 달라져야 합니다.♻️ 상수 또는 파라미터로 분리
+ private static final String TITLE_SOURCE_USER = "USER"; + private static final int DEFAULT_CONVERSATION_LIMIT = 50; + // CreateNoteResponse return new CreateNoteResponse( note.getId(), note.getTitle(), - "USER", // 현재는 AI 사용하지 않으므로 USER로 표시 - 50, // 기본 대화 제한 + TITLE_SOURCE_USER, + planType.getConversationLimit(), firstConversationDto, note.getCreatedAt() );
317-345: 정렬 파라미터 파싱 로직 검증파싱 로직이 잘 구현되어 있습니다.
lastUsedAt→updatedAt매핑, 허용된 필드 검증, 기본값 처리가 적절합니다. 다만, 허용되지 않은 정렬 필드 요청 시 경고 로그를 추가하면 디버깅에 도움이 됩니다.♻️ 잘못된 정렬 필드에 대한 경고 로그 추가
// 허용된 정렬 필드만 사용 if (!Set.of("updatedAt", "createdAt", "title").contains(property)) { + log.warn("허용되지 않은 정렬 필드 요청: {}, 기본값 updatedAt 사용", property); property = "updatedAt"; }
361-364: 대화 사용률 계산 시 정수 오버플로우 가능성
conversationCount가 매우 큰 값일 경우(float) conversationCount / conversationLimit * 100계산에서 예상치 못한 결과가 발생할 수 있습니다. 현재 비즈니스 로직상 큰 문제는 아니지만, 100%를 초과하지 않도록 상한선을 두는 것이 안전합니다.♻️ 사용률 상한선 적용
// 대화 사용률 계산 int conversationUsagePercent = conversationLimit > 0 - ? Math.round((float) conversationCount / conversationLimit * 100) + ? Math.min(100, Math.round((float) conversationCount / conversationLimit * 100)) : 0;
Comment on lines
+171
to
+178
| private String generateSimpleTitle(String firstMessage) { | ||
| // 첫 메시지에서 최대 50자까지 제목으로 사용 | ||
| String title = firstMessage.replaceAll("\\s+", " ").trim(); | ||
| if (title.length() > 50) { | ||
| title = title.substring(0, 50); | ||
| } | ||
| return title; | ||
| } |
There was a problem hiding this comment.
firstMessage가 null일 경우 NullPointerException 발생 가능
createNote 메서드의 로깅에서는 null 체크를 하지만(Line 50-51), generateSimpleTitle에서는 null 체크 없이 replaceAll()을 호출합니다. firstMessage가 null인 경우 NPE가 발생합니다.
🐛 null 체크 추가
private String generateSimpleTitle(String firstMessage) {
+ if (firstMessage == null || firstMessage.isBlank()) {
+ return "새 노트";
+ }
// 첫 메시지에서 최대 50자까지 제목으로 사용
String title = firstMessage.replaceAll("\\s+", " ").trim();
if (title.length() > 50) {
title = title.substring(0, 50);
}
return title;
}🤖 Prompt for AI Agents
In `@src/main/java/com/proovy/domain/note/service/NoteServiceImpl.java` around
lines 171 - 178, generateSimpleTitle currently calls
firstMessage.replaceAll(...) without a null check, which can throw a
NullPointerException when firstMessage is null (createNote has already guarded
some paths). Fix generateSimpleTitle by adding a null check at the start of the
method (in method generateSimpleTitle) and handle null safely (e.g., return an
empty string or a default title) before calling replaceAll/trim/substring so no
NPE occurs; update callers if they expect a non-null return.
gaeunee2
approved these changes
Jan 27, 2026
Collaborator
gaeunee2
left a comment
gaeunee2
left a comment
There was a problem hiding this comment.
테스트 확인했습니다. 머지 해주셔도 될 것 같아요!
This was referenced Feb 7, 2026
Merged
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 관련 이슈
🏷️ PR 타입
📝 작업 내용
구현된 사항
1. Note 엔티티 수정
Note.javaupdatedAt필드 (기존 컬럼 활용)@LastModifiedDate어노테이션으로 자동 업데이트lastUsedAt초기화2. Repository 계층 수정
NoteRepository
Page<Note> findByUserId(Long userId, Pageable pageable)추가ConversationRepository
long countByNoteId(Long noteId)추가AssetRepository
long countByNoteId(Long noteId)추가3. Response DTO 생성
NoteListResponse.javaNoteDto: 개별 노트 정보PageInfo: 페이지네이션 정보4. Service 계층 구현
NoteServiceImpl.javagetNoteList()메서드 구현5. Controller 엔드포인트 추가
NoteController.javaGET /api/notes엔드포인트 구현page(기본값: 0)size(기본값: 20, 최대: 50)sort(기본값: "lastUsedAt,desc")📸 스크린샷
COMMON200 노트목록 조회 성공
✅ 체크리스트
📎 기타 참고사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
✏️ Tip: You can customize this high-level summary in your review settings.