-
Notifications
You must be signed in to change notification settings - Fork 0
[Feat] 최신 상점 조회 API의 응원 내용 추가 #177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough최신 상점 조회 응답에 응원 문자열 목록( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as StoreController
participant Service as StoreService
participant Repo as StoreRepository
participant DB
Client->>Controller: GET /stores?...
Controller->>Service: getStores(page,size,category)
Service->>Repo: findAllByOrderByCreatedAtDesc(Pageable) [@EntityGraph(cheers)]
Repo->>DB: Query Stores + Cheers
DB-->>Repo: Stores with Cheers
Service->>Service: map Store -> StorePreviewResponse(..., cheerDescriptions)
Service-->>Controller: StoresResponse (includes cheerDescriptions)
Controller-->>Client: 200 OK
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
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.yaml 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (6)
✨ 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
Status, Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/main/java/eatda/domain/store/Store.java (2)
60-62: OneToMany 컬렉션 초기화 누락 — 새 엔티티/테스트 컨텍스트에서 NPE 가능성JPA가 영속 컨텍스트에서 컬렉션 프록시를 주입하더라도, 새로 빌더로 생성된 Store 등에서는
cheers가 null일 수 있어getCheerDescriptions()에서stream()호출 시 NPE 가능성이 있습니다. 기본 초기화 및 배치 페치 옵션을 함께 고려해 주세요.두 가지 대안 중 하나를 권장합니다.
- 대안 A: 컬렉션만 초기화
- @OneToMany(mappedBy = "store") - private List<Cheer> cheers; + @OneToMany(mappedBy = "store") + private List<Cheer> cheers = new ArrayList<>();추가 import:
import java.util.ArrayList;대안 B: N+1 완화까지 고려 (Hibernate 의존)
- @OneToMany(mappedBy = "store") - private List<Cheer> cheers; + @OneToMany(mappedBy = "store") + @BatchSize(size = 100) + private List<Cheer> cheers = new ArrayList<>();추가 import:
import org.hibernate.annotations.BatchSize;import java.util.ArrayList;orphanRemoval/cascade 설정은 도메인 요구사항(응원 삭제 정책)에 따라 별도 검토가 필요합니다.
97-101: Null-safe 처리 및 불필요한 null 설명 제거 제안
cheers가 null일 가능성에 대비하고, null 설명은 제외하면 소비자 입장에서 안정적입니다.다음과 같이 방어적으로 수정해 주세요.
- public List<String> getCheerDescriptions() { - return cheers.stream() - .map(Cheer::getDescription) - .toList(); - } + public List<String> getCheerDescriptions() { + List<Cheer> list = (cheers == null) ? List.of() : cheers; + return list.stream() + .map(Cheer::getDescription) + .filter(Objects::nonNull) + .toList(); + }추가 import:
import java.util.Objects;추가로, 빈 문자열 제거가 필요하다면
.filter(s -> !s.isBlank())를 덧붙일 수 있습니다.src/main/java/eatda/controller/store/StorePreviewResponse.java (1)
16-25: 미리보기 응답의 페이로드 제어(선택) — 상위 N개만 노출 고려스토어별 응원 수가 많을 경우 응답 페이로드가 커질 수 있습니다. 미리보기 성격이라면 상위 N개(예: 3개)만 노출을 고려해 보세요. 구현 위치는 도메인
getCheerDescriptions()또는 본 생성자 중 한 곳으로 일관되게 적용하면 됩니다.가능한 간단한 적용 예:
- store.getCheerDescriptions() + store.getCheerDescriptions().stream().limit(3).toList()정렬(최신순 등)이 필요하다면
Cheer의 생성/작성 시각을 기준으로 정렬 후 제한을 적용하세요.src/test/java/eatda/service/store/StoreServiceTest.java (1)
80-87: 검증 강도 보강 제안: 응원 내용 자체도 검증(선택)현재는 개수만 검증합니다. 매핑 정확도를 올리려면 응원 텍스트의 내용(예: not blank, 특정 프리픽스 등)까지 점검하면 회귀에 강합니다.
예:
assertThat(response.stores().get(0).cheerDescriptions()) .allMatch(s -> s != null && !s.isBlank());또는 테스트 픽스처에서 명시적 설명을 넣고
containsExactlyInAnyOrder(...)로 검증할 수 있습니다.src/main/java/eatda/repository/store/StoreRepository.java (1)
24-28: Pageable + @entitygraph(to-many) 사용 시 페이지 왜곡/중복 가능성 주의
@EntityGraph(attributePaths = {"cheers"})가 to-many 관계를 페치 조인 형태로 강제할 수 있어 페이징 시 중복/왜곡(또는 메모리 내 distinct) 이슈가 발생할 수 있습니다. 데이터가 늘어나면 성능/정합성 문제가 생길 수 있으니 확인 바랍니다.검토/대안:
- 단건/페이지 조회는 루트 엔티티만 페치하고, 두 번째 쿼리로
IN (:storeIds)형태로 cheers를 일괄 로딩해 맵핑(응원 텍스트만 필요 시 projection)합니다.- 또는
@BatchSize(size = N)(Hibernate) + LAZY 유지로 컬렉션 배치 로딩을 활용합니다.- 꼭 그래프가 필요하면 to-one만 그래프로 가져오고 to-many는 별도 경로로 로딩하세요.
증상 확인 방법:
- 다수 응원(cheers)이 달린 가게가 섞인 상태에서 페이지 경계(예: size=2) 테스트 시, 결과 수/정렬이 의도와 다른지 로그 SQL로 확인.
src/test/java/eatda/document/store/StoreDocumentTest.java (1)
119-121: cheerDescriptions 필드의 이미지 포함 여부 검증 및 문서화 개선
확인 결과StorePreviewResponse.cheerDescriptions는List<String>으로 문자열만 반환되며, 이미지 URL은 포함되어 있지 않습니다. API 의도에 따라 아래 중 하나를 선택해주세요:• 옵션 A: 이미지 URL도 함께 노출하도록 스키마를 객체 배열로 확장
- fieldWithPath("stores[].cheerDescriptions").type(ARRAY).description("음식점에 달린 응원 메시지") + fieldWithPath("stores[].cheers").type(ARRAY).description("음식점에 달린 응원 리스트"), + fieldWithPath("stores[].cheers[].description").type(STRING).description("응원 메시지"), + fieldWithPath("stores[].cheers[].imageUrl").type(STRING).description("응원 이미지 URL").optional()• 옵션 B: 텍스트만 제공하는 게 의도라면, 설명에 “이미지는 포함되지 않음”을 명시
- fieldWithPath("stores[].cheerDescriptions").type(ARRAY).description("음식점에 달린 응원 메시지") + fieldWithPath("stores[].cheerDescriptions").type(ARRAY).description("음식점에 달린 응원 메시지 텍스트 목록(이미지는 포함되지 않음)")대상 파일 및 위치
- src/test/java/eatda/document/store/StoreDocumentTest.java:120
📜 Review details
Configuration used: .coderabbit.yaml
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 (6)
src/main/java/eatda/controller/store/StorePreviewResponse.java(2 hunks)src/main/java/eatda/domain/store/Store.java(4 hunks)src/main/java/eatda/repository/store/StoreRepository.java(2 hunks)src/main/java/eatda/service/store/StoreService.java(2 hunks)src/test/java/eatda/document/store/StoreDocumentTest.java(2 hunks)src/test/java/eatda/service/store/StoreServiceTest.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/eatda/domain/store/Store.java (2)
src/main/java/eatda/domain/story/Story.java (1)
Table(27-171)src/main/java/eatda/domain/member/Member.java (1)
Table(18-119)
⏰ 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: test
🔇 Additional comments (5)
src/main/java/eatda/service/store/StoreService.java (2)
39-39: 읽기 전용 트랜잭션 지정 LGTM조회 전용 메서드에
@Transactional(readOnly = true)적용으로 성능 최적화 및 실수로 인한 쓰기 방지에 도움이 됩니다.
55-55: 읽기 전용 트랜잭션 지정 LGTM이미지 조회도 읽기 전용으로 적절합니다.
src/main/java/eatda/controller/store/StorePreviewResponse.java (1)
12-14: 응답 필드 확장 LGTM
cheerDescriptions추가가 PR 목적과 일치하며 역호환성(필드 추가) 측면에서도 안전합니다.src/test/java/eatda/service/store/StoreServiceTest.java (1)
62-74: 테스트 데이터 시나리오 개선 LGTM복수 회원/가게로 응원 관계를 구성하여 시나리오 현실성이 좋아졌습니다.
src/test/java/eatda/document/store/StoreDocumentTest.java (1)
129-133: StorePreviewResponse에 cheerDescriptions 샘플 데이터 추가 LGTM테스트 더미 데이터가 새로운 시그니처(List cheerDescriptions)를 잘 반영합니다. 문서화 필드와 일관성도 좋아 보입니다.
lvalentine6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이번 PR도 고생하셨습니다! 🎉
궁금증 몇개만 코멘트 남겼습니다!
|
|
||
| assertAll( | ||
| () -> assertThat(response.stores()).hasSize(size), | ||
| () -> assertThat(response.stores()).hasSize(2), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위에 size 선언은 안쓰게 되나요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위의 size 값을 쓰도록 변경했습니다.
| return new StoreResponse(store); | ||
| } | ||
|
|
||
| // TODO : N+1 문제 해결 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EntityGraph를 사용했어도 아직 N + 1 문제가 남아있나요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추후 지워질 예정이라 그대로 두겠습니다.
# Conflicts: # src/test/java/eatda/service/store/StoreServiceTest.java
|
|
🎉 This PR is included in version 1.4.0-develop.91 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.8.0-develop.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.8.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |



✨ 개요
🧾 관련 이슈
closed #173
🔍 참고 사항 (선택)
@OneToMany+@EntityGraph를 이용하여 N+1 문제를 일부 처리했습니다.Summary by CodeRabbit