Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough일일 피드백 조회 기능을 추가하는 엔드투엔드 변경입니다. 저장소 계층에 Sequence DiagramsequenceDiagram
participant Client
participant Controller as FeedbackController
participant Service as FeedbackReadService
participant Repository as FeedbackRepository
participant Database
Client->>Controller: GET /daily/{date}
Controller->>Service: getDailyFeedbacks(userId, date)
Service->>Repository: findByUserIdAndFeedbackDateAndStatusAndDeletedAtIsNull
Repository->>Database: Query Feedback records
Database-->>Repository: List<Feedback>
Repository-->>Service: List<Feedback>
Note over Service: Map to GetDailyFeedbacksInfo
Service-->>Controller: GetDailyFeedbacksInfo
Note over Controller: Map to GetDailyFeedbacksResponse
Controller-->>Client: ResponseEntity<GetDailyFeedbacksResponse> (200 OK)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 근거:
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
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: 8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
back/src/docs/asciidoc/feedback.adoc(1 hunks)back/src/main/java/com/rouby/assistant/feedback/application/dto/GetDailyFeedbacksInfo.java(1 hunks)back/src/main/java/com/rouby/assistant/feedback/application/service/FeedbackReadService.java(2 hunks)back/src/main/java/com/rouby/assistant/feedback/application/usecase/CreateFeedbackUsecase.java(0 hunks)back/src/main/java/com/rouby/assistant/feedback/domain/repository/FeedbackRepository.java(2 hunks)back/src/main/java/com/rouby/assistant/feedback/infrastructure/persistence/jpa/FeedbackJpaRepository.java(2 hunks)back/src/main/java/com/rouby/assistant/feedback/presentation/FeedbackController.java(3 hunks)back/src/main/java/com/rouby/assistant/feedback/presentation/dto/GetDailyFeedbacksResponse.java(1 hunks)back/src/test/http/feedback.http(1 hunks)back/src/test/java/com/rouby/assistant/feedback/presentation/FeedbackControllerTest.java(2 hunks)back/src/test/java/com/rouby/common/support/ControllerTestSupport.java(2 hunks)
💤 Files with no reviewable changes (1)
- back/src/main/java/com/rouby/assistant/feedback/application/usecase/CreateFeedbackUsecase.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-13T12:03:54.680Z
Learnt from: HanaHww2
PR: HI-dle/rouby#249
File: back/src/main/java/com/rouby/assistant/feedback/application/service/FeedbackReadService.java:16-0
Timestamp: 2025-10-13T12:03:54.680Z
Learning: In the feedback feature, `FeedbackReadService.getRecentFeedbackInfoWithin1W` intentionally excludes today's feedback by using `LocalDate.now().atStartOfDay()` as the upper bound of the query range (7 days ago to today 00:00).
Applied to files:
back/src/main/java/com/rouby/assistant/feedback/application/service/FeedbackReadService.javaback/src/main/java/com/rouby/assistant/feedback/infrastructure/persistence/jpa/FeedbackJpaRepository.java
🧬 Code graph analysis (1)
back/src/main/java/com/rouby/assistant/feedback/presentation/FeedbackController.java (1)
back/src/main/java/com/rouby/assistant/feedback/application/service/FeedbackReadService.java (1)
RequiredArgsConstructor(12-34)
🔇 Additional comments (9)
back/src/test/java/com/rouby/common/support/ControllerTestSupport.java (1)
9-9: LGTM! 테스트 지원을 위한 Mock Bean 추가가 적절합니다.새로운 일간 피드백 조회 기능을 테스트하기 위해
FeedbackReadService의 Mock Bean을 추가한 것은 올바른 접근입니다. 기존 패턴과 일관성이 있습니다.Also applies to: 96-97
back/src/test/http/feedback.http (1)
10-11: LGTM! HTTP 테스트 요청 추가가 적절합니다.일간 피드백 조회를 위한 HTTP 테스트 요청이 올바르게 추가되었습니다.
back/src/docs/asciidoc/feedback.adoc (1)
26-39: LGTM! API 문서화가 잘 작성되었습니다.일간 피드백 조회 API에 대한 문서가 기존 패턴을 따라 명확하게 작성되었습니다.
back/src/main/java/com/rouby/assistant/feedback/domain/repository/FeedbackRepository.java (1)
23-23: LGTM! 리포지토리 인터페이스 메서드가 적절합니다.도메인 리포지토리 인터페이스에 일간 피드백 조회 메서드가 올바르게 추가되었습니다. JPA 구현체와 일치합니다.
back/src/main/java/com/rouby/assistant/feedback/presentation/FeedbackController.java (1)
39-48: 엔드포인트가 올바르게 구현되었습니다.검증 결과 모든 사항이 적절하게 처리됨이 확인되었습니다:
- 빈 피드백 리스트 처리:
GetDailyFeedbacksResponse.from()에서 빈 목록을Collections.emptyList()로 반환하며, 200 OK 응답과 함께 빈 배열을 전송하는 동작은 의도된 올바른 구현입니다.- 날짜 파싱 오류:
GlobalExceptionHandler에서MethodArgumentTypeMismatchException을 처리하며, 이는@PathVariable의 잘못된 날짜 형식을 적절히 400 응답으로 처리합니다.변경 사항을 승인합니다.
back/src/main/java/com/rouby/assistant/feedback/application/dto/GetDailyFeedbacksInfo.java (2)
14-23: null 안전 처리가 잘 되어있습니다.입력 리스트가 null이거나 비어있는 경우 빈 리스트를 반환하는 방어적 코딩이 적절합니다.
36-47: mood와 status는 DB 제약으로 보호되나 feedbackContent 필드의 null 안전성 확인 필요검증 결과, 원본 우려는 부분적으로 수정이 필요합니다:
- Line 41 (
mood.toString()): 안전 - Feedback 엔티티의 mood 필드는@Column(nullable = false)제약이 있습니다- Line 43 (
status.toString()): 안전 - Feedback 엔티티의 status 필드는@Column(nullable = false)제약이 있습니다- Line 42 (
feedbackContent.getContent()): 잠재적 위험 - feedbackContent는@Embedded필드이지만 명시적인nullable=false제약이 없습니다. 다만 현재 생성 로직에서 항상 FeedbackContent.of()로 초기화되므로 실제 NPE 위험은 낮습니다권장사항: feedbackContent 필드에
@Column(nullable = false)주석을 추가하거나, 방어적으로 null 체크를 추가하는 것을 검토하세요.back/src/main/java/com/rouby/assistant/feedback/presentation/dto/GetDailyFeedbacksResponse.java (2)
24-31: 중첩 record의 접근 제어자 일관성을 확인하세요.
GetDailyFeedbackResponse는 package-private인 반면,GetDailyFeedbacksInfo.GetFeedbackInfo는 public으로 선언되어 있습니다. 두 DTO가 유사한 역할을 하는데 접근 제어가 다릅니다.프레젠테이션 계층 DTO의 내부 타입이므로 package-private가 적절해 보이지만, 아키텍처 관점에서 의도적인 차이인지 확인이 필요합니다.
33-43: 입력 파라미터의 null 안전성을 확인하세요.
from(GetFeedbackInfo)메서드가info파라미터에 대한 null 체크 없이 필드들에 접근합니다.GetFeedbackInfo.from(Feedback)에서 이미 NPE 위험이 있으므로, 이 메서드에서도 null 입력이 전달될 가능성을 검증해주세요.최소한 다음 중 하나를 적용하는 것을 권장합니다:
- 메서드 파라미터에
@NonNull어노테이션 추가- 메서드 시작 부분에
Objects.requireNonNull(info)추가- 상위 레이어에서 null이 절대 전달되지 않음을 보장
back/src/main/java/com/rouby/assistant/feedback/application/dto/GetDailyFeedbacksInfo.java
Outdated
Show resolved
Hide resolved
back/src/main/java/com/rouby/assistant/feedback/application/service/FeedbackReadService.java
Show resolved
Hide resolved
| Optional<Feedback> findTop1ByUserIdAndCreatedAtBetweenOrderByCreatedAtDesc( | ||
| Long userId, LocalDateTime fromAt, LocalDateTime toAt); | ||
|
|
||
| List<Feedback> findByUserIdAndFeedbackDateAndStatusAndDeletedAtIsNull(Long userId, LocalDate date, Status status); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
쿼리 메서드가 올바르게 추가되었습니다. 성능 최적화를 고려하세요.
Spring Data JPA 메서드 네이밍 컨벤션을 따른 명확한 쿼리 메서드입니다. userId, feedbackDate, status, deletedAt 조건으로 필터링합니다.
성능 최적화를 위해 데이터베이스 인덱스 존재 여부를 확인하세요:
복합 인덱스 (user_id, feedback_date, status, deleted_at)가 없다면 추가를 고려하세요. 특히 사용자당 피드백이 많아질 경우 조회 성능에 영향을 줄 수 있습니다.
🏁 Script executed:
#!/bin/bash
# Description: Check for database index on feedback table for the new query
# Search for index definitions or migration files related to feedback table
fd -e sql -e xml | xargs rg -l "feedback" | xargs rg -i "index.*feedback.*(user_id|feedback_date|status|deleted_at)"
# Also check JPA entity for index annotations
rg -A5 "@Table|@Index" back/src/main/java/com/rouby/assistant/feedback/domain/entity/Feedback.javaLength of output: 0
더 정확한 검증을 위해 추가 스크립트를 실행하겠습니다.
더 정확한 검증을 위해 추가 스크립트를 실행하겠습니다.
🏁 Script executed:
#!/bin/bash
# Find and examine the Feedback entity file
echo "=== Searching for Feedback entity ==="
find . -name "Feedback.java" -type f 2>/dev/null | head -5
# Check the entity file for index annotations
echo -e "\n=== Checking Feedback entity for @Index annotations ==="
rg -A20 "@Entity|@Table" back/src/main/java/com/rouby/assistant/feedback/domain/entity/Feedback.java 2>/dev/null || echo "Entity file not found or no matches"
# Look for database migration files
echo -e "\n=== Searching for database migration files ==="
fd -e sql -e yml -e yaml | grep -iE "(migration|flyway|liquibase|db)" | head -10
# Search for any index definitions in the entire codebase
echo -e "\n=== Searching for @Index annotations anywhere ==="
rg -n "@Index" --type java 2>/dev/null | head -10 || echo "No @Index annotations found"Length of output: 1274
🏁 Script executed:
#!/bin/bash
# Find and examine the Feedback entity file
echo "=== Searching for Feedback entity ==="
find . -name "Feedback.java" -type f 2>/dev/null | grep -v target
# Check the entity file for index annotations
echo -e "\n=== Checking Feedback entity for @Index annotations ==="
rg -A20 "@Entity|@Table" --type java 2>/dev/null | grep -A20 "Feedback"
# Look for database migration files
echo -e "\n=== Searching for database migration files ==="
fd -e sql -e yml -e yaml 2>/dev/null | grep -iE "(migration|flyway|liquibase|db|ddl|schema)" | head -20
# Search for any index definitions in the entire codebase
echo -e "\n=== Searching for @Index annotations ==="
rg -B2 -A2 "@Index" --type java 2>/dev/nullLength of output: 5783
스키마 파일을 확인하여 데이터베이스 레벨에서 인덱스가 정의되어 있는지 검증하겠습니다.
🏁 Script executed:
#!/bin/bash
# Check schema.sql for feedback table and indexes
echo "=== Checking schema.sql for feedback table ==="
rg -A20 -i "create.*table.*feedback|feedback" back/src/main/resources/db/schema.sql
# Check if there are any index definitions for feedback table
echo -e "\n=== Searching for indexes on feedback table ==="
rg -i "create.*index.*feedback|feedback.*index" back/src/main/resources/db/schema.sql
# Display entire schema.sql if it's not too large
echo -e "\n=== Full schema.sql content ==="
cat back/src/main/resources/db/schema.sql | head -100Length of output: 1290
복합 인덱스 추가 권장 - 현재 인덱스가 새 쿼리를 커버하지 않습니다
Spring Data JPA 쿼리 메서드 이름 규칙은 올바르게 작성되었습니다. 그러나 검증 결과, 데이터베이스 스키마에는 (user_id, feedback_date, slot) 필터링만 지원하는 유니크 인덱스만 존재하며, 새로운 쿼리 메서드가 필요로 하는 (user_id, feedbackDate, status, deletedAt) 필터 조합을 커버하지 않습니다.
코드베이스의 다른 엔티티들(RoutineTask, Schedule, EmailLog)처럼 Feedback 엔티티에 @Table 어노테이션으로 복합 인덱스를 추가하세요:
@Entity
@Table(name = "feedback", indexes = {
@Index(name = "idx_feedback_user_date_status_deleted",
columnList = "user_id, feedback_date, status, deleted_at")
})🤖 Prompt for AI Agents
In
back/src/main/java/com/rouby/assistant/feedback/infrastructure/persistence/jpa/FeedbackJpaRepository.java
around line 22: the new query method
findByUserIdAndFeedbackDateAndStatusAndDeletedAtIsNull is not covered by the
existing DB index (which only covers user_id, feedback_date, slot); add a
composite index on (user_id, feedback_date, status, deleted_at) to the Feedback
entity so the query can use an index. Open the Feedback entity class and add a
@Table annotation (or update its existing @Table) to declare an index named like
idx_feedback_user_date_status_deleted with columnList "user_id, feedback_date,
status, deleted_at", then regenerate/migrate the schema (or add a migration) so
the index is created in the database.
back/src/main/java/com/rouby/assistant/feedback/presentation/dto/GetDailyFeedbacksResponse.java
Outdated
Show resolved
Hide resolved
back/src/main/java/com/rouby/assistant/feedback/presentation/dto/GetDailyFeedbacksResponse.java
Show resolved
Hide resolved
back/src/test/java/com/rouby/assistant/feedback/presentation/FeedbackControllerTest.java
Show resolved
Hide resolved
back/src/test/java/com/rouby/assistant/feedback/presentation/FeedbackControllerTest.java
Outdated
Show resolved
Hide resolved
| @GetMapping("/daily/{date}") | ||
| public ResponseEntity<GetDailyFeedbacksResponse> getDailyFeedback( | ||
| @AuthenticationPrincipal SecurityUser user, | ||
| @PathVariable @PastOrPresent LocalDate date) { |
There was a problem hiding this comment.
수고하셨습니다! @PastOrPresent는 잘 동작하는지 궁금합니다!
There was a problem hiding this comment.
잘 동작합니다. 관련 테스트 코드가 있습니다.
2 participants
#️⃣연관된 이슈
변경 타입
변경 내용
as-is
to-be(변경 후 설명을 여기에 작성)
체크리스트
코멘트
Summary by CodeRabbit
릴리스 노트
새로운 기능
문서
테스트