-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor] Cheer - CheerTag @OneToMany 로 개선
#166
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
|
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. ✨ 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: 3
🔭 Outside diff range comments (1)
src/main/java/eatda/service/cheer/CheerService.java (1)
70-75: 리스트 조회 시 N+1 위험 — Cheer의 store/태그 페치 전략 필요toCheersResponse/서비스에서 cheer.getStore()와 cheer.getCheerTagNames()를 호출하고 있는데, 현재 CheerRepository의 findAll... 메서드들은 fetch join이나 EntityGraph를 사용하지 않아 N+1이 발생합니다. (검증한 파일: CheerRepository, CheerService, Cheer.java, CheerTags.java, CheerResponse/CheerPreviewResponse)
수정이 필요한 위치(핵심):
- src/main/java/eatda/service/cheer/CheerService.java
- getCheers(...) → cheerRepository.findAllByOrderByCreatedAtDesc(...) → toCheersResponse(...) (cheer.getStore() 사용)
- getCheersByStoreId(...) → findAllByStoreOrderByCreatedAtDesc(...) (태그 접근 가능)
- src/main/java/eatda/repository/cheer/CheerRepository.java
- findAllByOrderByCreatedAtDesc(Pageable pageable)
- findAllByStoreOrderByCreatedAtDesc(Store store, Pageable pageable)
(현재 @EntityGraph/@query(fetch join) 없음)- src/main/java/eatda/domain/cheer/Cheer.java
- @manytoone(fetch = FetchType.LAZY) private Store store;
- src/main/java/eatda/domain/cheer/CheerTags.java
- @onetomany(mappedBy = "cheer") private List values; → getNames()가 컬렉션 접근
권장 대응(간단/안전한 순서):
Store는 ManyToOne이므로 우선 EntityGraph로 한번에 페치:
- CheerRepository에 추가
import org.springframework.data.jpa.repository.EntityGraph;
@entitygraph(attributePaths = {"store"})
List findAllByOrderByCreatedAtDesc(Pageable pageable);태그(컬렉션)는 페이지네이션과 함께 컬렉션 fetch-join하면 잘못된 페이징/중복 문제가 발생하므로 직접 컬렉션을 fetch-join하지 마세요. 대신 하나를 선택:
패턴 A (권장): IDs 먼저 페이지네이션으로 조회 → 해당 IDs로 연관관계(fetch join) 조회
@query("select c.id from Cheer c order by c.createdAt desc")
List findIdsOrderByCreatedAtDesc(Pageable pageable);@query("select distinct c from Cheer c left join fetch c.store left join fetch c.cheerTags.values where c.id in :ids")
List findAllWithStoreAndTagsByIdIn(List ids);서비스: ids → findAllWithStoreAndTagsByIdIn(ids) → DTO로 매핑
패턴 B: 기본 조회(엔티티그래프로 store만) 후 CheerTagRepository에 findAllByCheerIdIn(cheerIds)로 태그를 한 번에 로드해 매핑
또는 Hibernate 배치(fetch) 설정(@batchsize) 사용
요약(요청): CheerRepository의 페치 전략(최소 store) 적용 또는 서비스에서 두-단계 조회(IDs→fetch join) / 태그 일괄 조회 방식 중 하나를 도입해 주세요. 현재 상태에서는 N+1 위험이 확인되어 수정이 필요합니다.
🧹 Nitpick comments (9)
src/test/java/eatda/service/cheer/CheerServiceTest.java (1)
149-168: 빈 태그 케이스 테스트 추가, 시나리오 적합합니다
- 요청 태그가 빈 리스트일 때 저장/응답 흐름을 검증하는 테스트가 실제 사용 시나리오와 맞고, 응답의 tags가 비어있음을 명확히 보장합니다. LGTM.
- 추가 제안(선택): 응답 검증 외에도, 실제로 저장된 Cheer 엔티티의 getCheerTagNames()가 빈 리스트인지까지 확인하면(예: 최신 저장된 Cheer 로드 후) 영속성 레벨 검증까지 커버되어 회귀에 더 강해집니다.
src/test/java/eatda/domain/cheer/CheerTagsTest.java (1)
21-36: 테스트 픽스처 직결 생성은 간결하지만, 재사용 가능한 빌더/픽스처로 추출 고려DEFAULT_MEMBER/DEFAULT_STORE/DEFAULT_CHEER를 이 파일 내에서 직접 구성하는 방식은 간단하나, 다른 테스트에서도 유사 픽스처가 반복될 가능성이 큽니다. 공용 테스트 픽스처/빌더(예: TestFixtures, TestDataFactory)로 추출하면 중복 제거 및 유지보수성이 개선됩니다.
src/main/java/eatda/domain/cheer/Cheer.java (2)
75-77: null 입력 방어 로직 추가 제안request.tags()가 null인 경우 NPE가 발생할 수 있습니다. 빈 리스트로 정규화하면 안전합니다.
다음과 같이 보완을 제안합니다:
- public void setCheerTags(List<CheerTagName> cheerTagNames) { - this.cheerTags = new CheerTags(this, cheerTagNames); - } + public void setCheerTags(List<CheerTagName> cheerTagNames) { + List<CheerTagName> safe = (cheerTagNames == null) ? Collections.emptyList() : cheerTagNames; + this.cheerTags = new CheerTags(this, safe); + }
79-84: 외부 불변성 보장(선택): 방어적 복사로 리스트 반환현재 구현도 null-안전하지만, 반환 리스트가 외부에서 변경 불가능함을 보장하려면 방어적 복사를 고려할 수 있습니다.
- return cheerTags.getNames(); + return List.copyOf(cheerTags.getNames());src/main/java/eatda/controller/cheer/CheerResponse.java (1)
16-23: Response가 Cheer의 태그를 직접 참조하도록 단순화한 점 좋습니다
- 기존 CheerTag 리스트 의존을 제거하고, 도메인에서 직접 태그명을 가져오는 방식으로 응답 생성이 단순/견고해졌습니다. LGTM.
- 선택 제안: API 소비자 입장에서 태그 순서가 중요하다면(정렬 기대), 반환 전에 정렬을 보장하는 것이 안전합니다. 현재 테스트는 anyOrder 기반이라 계약이 모호할 수 있습니다.
정렬 보장을 고려한다면 예를 들어 다음과 같이 정렬된 리스트를 전달할 수 있습니다(카테고리/이름 기준은 정책에 맞게 조정):
- cheer.getCheerTagNames() + cheer.getCheerTagNames().stream() + .sorted() // Enum 자연 순서 기준. 필요시 Comparator 구현 + .toList()src/main/java/eatda/service/cheer/CheerService.java (3)
17-17: 미사용 의존성 제거: CheerTagRepository현재 CheerTagRepository를 주입하지만 사용하지 않습니다. 제거해 의존성/빌드 타임을 줄이세요.
다음 변경을 권장합니다:
- import eatda.repository.cheer.CheerTagRepository;- private final CheerTagRepository cheerTagRepository;Also applies to: 36-36
47-49: Store 조회/저장 동시성 TODO: 낙관적 해결 패턴 제안현재 TODO로 표기된 중복 저장 경쟁 상태는 다음 패턴으로 완화 가능합니다.
- kakaoId에 유니크 인덱스 부여
- orElseGet(save) 시도 → DataIntegrityViolationException 캐치 → findByKakaoId 재조회
예시 패턴:
Store store = storeRepository.findByKakaoId(result.kakaoId()) .orElseGet(() -> { try { return storeRepository.save(result.toStore()); } catch (DataIntegrityViolationException e) { return storeRepository.findByKakaoId(result.kakaoId()).orElseThrow(); } });
82-85: N+1 TODO 처리: 페치 전략/프로젝션 제안CheerInStoreResponse 생성 시 연관 엔티티 접근(작성자, 태그, 스토어 등)이 필요하다면 fetch join 또는 DTO 프로젝션으로 한번에 가져오도록 전환하세요. 대량 페이지 처리 시 성능 차이가 큽니다.
가능한 접근:
- Repository 레벨에서 join fetch 쿼리 메서드 제공
- Spring Data Projections/DTO 바인딩 사용으로 select 필드를 한정
src/main/java/eatda/domain/cheer/CheerTags.java (1)
23-27: 파라미터 네이밍 오타 및 널 체크 추가 제안
- 네이밍:
cheerTagsNames→cheerTagNames(일관성)- 안정성:
cheer와 리스트에 대한 null 방어 추가 권장- 스타일: 동일 파일 내 stream 사용을
.toList()로 통일해 가독성 향상적용 제안(diff):
- public CheerTags(Cheer cheer, List<CheerTagName> cheerTagsNames) { - validate(cheerTagsNames); - this.values = cheerTagsNames.stream() - .map(name -> new CheerTag(cheer, name)) - .collect(Collectors.toList()); + public CheerTags(Cheer cheer, List<CheerTagName> cheerTagNames) { + Objects.requireNonNull(cheer, "cheer must not be null"); + Objects.requireNonNull(cheerTagNames, "cheerTagNames must not be null"); + validate(cheerTagNames); + this.values = cheerTagNames.stream() + .map(name -> new CheerTag(cheer, name)) + .toList(); }추가 import(선택):
import java.util.Objects;
📜 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/cheer/CheerResponse.java(1 hunks)src/main/java/eatda/domain/cheer/Cheer.java(3 hunks)src/main/java/eatda/domain/cheer/CheerTags.java(2 hunks)src/main/java/eatda/service/cheer/CheerService.java(1 hunks)src/test/java/eatda/domain/cheer/CheerTagsTest.java(3 hunks)src/test/java/eatda/service/cheer/CheerServiceTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: leegwichan
PR: YAPP-Github/26th-Web-Team-1-BE#159
File: src/main/java/eatda/domain/cheer/CheerTag.java:28-30
Timestamp: 2025-08-12T03:22:34.003Z
Learning: CheerTag와 Cheer 엔티티 간의 연관관계는 의도적으로 단방향 매핑(ManyToOne)으로 설계되었습니다. Cheer 엔티티에서 CheerTag 컬렉션으로의 양방향 매핑(OneToMany)은 불필요합니다.
Learnt from: leegwichan
PR: YAPP-Github/26th-Web-Team-1-BE#157
File: src/main/java/eatda/domain/cheer/CheerTagNames.java:19-26
Timestamp: 2025-08-12T03:10:30.178Z
Learning: CheerTagNames 클래스에서 현재 비즈니스 정책은 "각 타입별 2개"만 존재하며, 전체 태그 개수 제한은 현재 구현 범위에 포함되지 않음. 카테고리 추가 시 추후 고려 예정.
📚 Learning: 2025-08-12T03:22:34.003Z
Learnt from: leegwichan
PR: YAPP-Github/26th-Web-Team-1-BE#159
File: src/main/java/eatda/domain/cheer/CheerTag.java:28-30
Timestamp: 2025-08-12T03:22:34.003Z
Learning: CheerTag와 Cheer 엔티티 간의 연관관계는 의도적으로 단방향 매핑(ManyToOne)으로 설계되었습니다. Cheer 엔티티에서 CheerTag 컬렉션으로의 양방향 매핑(OneToMany)은 불필요합니다.
Applied to files:
src/main/java/eatda/controller/cheer/CheerResponse.javasrc/main/java/eatda/domain/cheer/Cheer.javasrc/main/java/eatda/domain/cheer/CheerTags.java
📚 Learning: 2025-08-12T03:10:30.178Z
Learnt from: leegwichan
PR: YAPP-Github/26th-Web-Team-1-BE#157
File: src/main/java/eatda/domain/cheer/CheerTagNames.java:19-26
Timestamp: 2025-08-12T03:10:30.178Z
Learning: CheerTagNames 클래스에서 현재 비즈니스 정책은 "각 타입별 2개"만 존재하며, 전체 태그 개수 제한은 현재 구현 범위에 포함되지 않음. 카테고리 추가 시 추후 고려 예정.
Applied to files:
src/main/java/eatda/domain/cheer/Cheer.javasrc/test/java/eatda/domain/cheer/CheerTagsTest.java
⏰ 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 (7)
src/test/java/eatda/domain/cheer/CheerTagsTest.java (3)
46-47: 정상 케이스(카테고리별 최대 개수 내) 검증 적절CheerTags(DEFAULT_CHEER, tagNames) 생성에서 예외가 발생하지 않음을 확인하는 포지티브 테스트가 명확합니다. 태그 정책 변경 시 깨지기 쉬운 부분을 잘 커버합니다.
61-65: 정책 위반(카테고리별 최대 초과) 시 예외 코드 검증 적절정확한 BusinessErrorCode(EXCEED_CHEER_TAGS_PER_TYPE)까지 검증하는 점 좋습니다. 정책 변경 시 회귀를 빠르게 탐지할 수 있습니다.
71-75: 중복 태그 방지 검증 적절중복 입력 시 CHEER_TAGS_DUPLICATED을 기대하는 테스트가 명확합니다. 도메인 제약을 잘 보호합니다.
src/main/java/eatda/service/cheer/CheerService.java (1)
49-53: 확인 완료 — ImageStorage.getPreSignedUrl의 null/빈 키 처리와 테스트 존재
- 태그 저장 흐름의 단일 엔티티 퍼시스트로 정리되어 명확합니다.
- ImageStorage.getPreSignedUrl은 imageKey == null || imageKey.isEmpty()인 경우 null을 반환하도록 구현되어 있으며, 해당 동작을 검증하는 단위 테스트가 존재합니다. ExternalImageStorage는 유효한 ImageKey에 대해 FileClient로 위임합니다. 따라서 CheerService에서의 호출은 현재 안전합니다.
주의가 필요한 위치:
- src/main/java/eatda/storage/image/ImageStorage.java — getPreSignedUrl(@nullable ImageKey): null/빈 체크 후 null 반환
- src/test/java/eatda/storage/image/ImageStorageTest.java — 이미지_키가_null이면__null을_반환한다(), 이미지_키가_비어있으면_null을_반환한다(), 캐시/조회 관련 테스트들
- src/main/java/eatda/storage/image/ExternalImageStorage.java — getPreSignedUrl(ImageKey) → fileClient.getPreSignedUrl(...) (위임)
- src/test/java/eatda/storage/image/ExternalImageStorageTest.java — 위임/반환 동작 검증
src/main/java/eatda/domain/cheer/CheerTags.java (3)
5-8: LGTM — Embeddable/연관관계 매핑을 위한 JPA import 추가가 적절합니다.Embeddable + OneToMany 구성에 필요한 JPA import 구성이 올바릅니다.
11-12: LGTM — @NoArgsConstructor(PROTECTED)로 JPA 프록시/리플렉션 호환 보장Embeddable에 대한 기본 생성자 가시성 설정이 적절합니다.
14-16: 설계 전환 확인 필요: Embeddable 내부에 OneToMany를 두어 사실상 양방향으로 전환장점(도메인 계층에서 태그 일관성 검증·삭제 전파 용이)은 분명하지만, 과거 러닝 메모에 따르면 Cheer → CheerTag 컬렉션 매핑은 의도적으로 회피했던 것으로 기록되어 있습니다. 이번 PR 목적에 따라 방향을 바꾼 것이 맞는지 최종 확인 부탁드립니다.
(참고한 러닝: 이전에는 단방향(ManyToOne)만 유지하기로 했음 — 본 코멘트는 해당 러닝을 근거로 합니다.)
|
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
♻️ Duplicate comments (2)
src/main/java/eatda/domain/cheer/CheerTags.java (2)
20-28: orphanRemoval 안전 패턴(clear → addAll) 채택 — 이전 리스크 해소됨Embeddable 인스턴스를 교체하지 않고 영속 컬렉션을 mutate(clear/addAll)하는 패턴으로 전환되어 orphanRemoval 미동작/중복 누적 리스크가 해소되었습니다. 아주 좋습니다.
55-59: 조치 필요 — CheerTags.getNames()의 LAZY 컬렉션 접근으로 인한 N+1 / LazyInitializationException 위험CheerTags.getNames()는 values를 스트리밍해 이름을 꺼내는데, values가 LAZY라면 다수 엔티티 처리 시 N+1이 발생하거나 영속성 컨텍스트 밖에서 LazyInitializationException이 발생할 수 있습니다.
문제되는 코드 (참고):
public List<CheerTagName> getNames() { return values.stream() .map(CheerTag::getName) .toList(); }검증 결과 (레포지토리 검색):
- cheerTags/values를 JOIN FETCH로 미리 로드하는 쿼리 없음(레포 전체 검색 결과 없음).
- @batchsize 또는 hibernate.default_batch_fetch_size 설정 없음.
- getNames() 호출 위치: src/main/java/eatda/domain/cheer/Cheer.java:85 (한 곳만 발견).
권장 대응(하나 이상 적용):
- 조회 레이어에서 컬렉션을 선로딩(JOIN FETCH)하거나, 태그 이름만 별도 projection/DTO로 한 번에 조회하도록 쿼리 변경.
- 대량 접근이 예상되면 @batchsize 또는 hibernate.default_batch_fetch_size 설정 검토.
- 서비스/DTO 변환 단계에서 엔티티의 컬렉션에 직접 접근하지 않도록 리팩터링.
수정이 필요한 위치:
- src/main/java/eatda/domain/cheer/CheerTags.java — getNames() (약 55–59줄)
- src/main/java/eatda/domain/cheer/Cheer.java — getNames() 호출 위치 (줄 85)
🧹 Nitpick comments (6)
src/test/java/eatda/domain/cheer/CheerTagsTest.java (2)
21-35: 테스트 픽스처 하드코딩 → 공용 Fixture/Builder로 추출 제안여러 테스트에서 재사용될 가능성이 높은 Member/Store/Cheer 픽스처는 TestFixture(혹은 Mother) 유틸로 추출하면 중복을 줄이고 유지보수가 쉬워집니다.
37-56: 추가 제안: setTags 재호출/조회 동작까지 검증하면 회 regressions 방지에 유용합니다아래 2가지를 추가로 테스트해 두면 변경에 강한 테스트가 됩니다.
- setTags를 두 번 호출했을 때 기존 값이 적절히 제거되는지(고아 삭제 시나리오의 단위 수준 검증)
- setTags 이후 getNames가 기대하는 태그 이름 목록을 반환하는지
예시 테스트 메서드:
@Test void setTags_두_번_호출시_이전_값이_정상_초기화된다() { CheerTags cheerTags = new CheerTags(); cheerTags.setTags(DEFAULT_CHEER, List.of(CheerTagName.OLD_STORE_MOOD, CheerTagName.ENERGETIC)); // 두 번째 호출: 더 적은/다른 태그로 교체 cheerTags.setTags(DEFAULT_CHEER, List.of(CheerTagName.OLD_STORE_MOOD)); assertThat(cheerTags.getNames()).containsExactly(CheerTagName.OLD_STORE_MOOD); }원하시면 위 테스트를 포함해 추가 케이스까지 반영한 커밋 패치를 만들어 드릴게요.
src/main/java/eatda/domain/cheer/CheerTags.java (4)
5-8: 컬렉션 순서 비결정성 → @orderby로 API 안정성 확보 제안응답/표시에 순서가 의미가 있거나(예: 일관된 정렬 필요), 테스트의 플래키니스를 줄이려면 정렬을 명시하세요.
아래와 같이 @orderby를 추가하는 것을 권장합니다:
import eatda.exception.BusinessException; import jakarta.persistence.CascadeType; import jakarta.persistence.Embeddable; import jakarta.persistence.OneToMany; +import jakarta.persistence.OrderBy; import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; @OneToMany(mappedBy = "cheer", cascade = CascadeType.ALL, orphanRemoval = true) + @OrderBy("id ASC") private List<CheerTag> values = new ArrayList<>();Also applies to: 17-18
23-24: 혼동 유발 주석 제거 권장생성자에서 이미 cheer를 전달하고 있어 “cheer is set later” 주석은 현재 구현과 상충합니다. 제거를 권장합니다.
- .map(name -> new CheerTag(cheer, name)) // cheer is set later + .map(name -> new CheerTag(cheer, name))
8-10: 널 입력 방어 + API 오용 방지(가시성 축소) 제안
- 방어적 프로그래밍: cheer / cheerTagNames가 null일 경우 명시적으로 실패하게 해두면 문제를 빠르게 탐지할 수 있습니다.
- 접근 제한자: setTags는 도메인 내부 조립 용도에 가깝습니다. public → package-private로 축소를 고려해 주세요(외부 오용 방지).
import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; +import java.util.Objects; - public void setTags(Cheer cheer, List<CheerTagName> cheerTagNames) { + void setTags(Cheer cheer, List<CheerTagName> cheerTagNames) { + Objects.requireNonNull(cheer, "cheer must not be null"); + Objects.requireNonNull(cheerTagNames, "cheerTagNames must not be null"); validate(cheerTagNames); List<CheerTag> cheerTags = cheerTagNames.stream() .map(name -> new CheerTag(cheer, name)) .toList(); this.values.clear(); this.values.addAll(cheerTags); }Also applies to: 20-28
17-18: 네이밍 가독성 제안: values → tags도메인 의미가 드러나도록 필드명을 tags로 바꾸면 가독성이 올라갑니다(연관된 접근자/메서드에서도 의미가 더 분명해집니다).
📜 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 (3)
src/main/java/eatda/domain/cheer/Cheer.java(4 hunks)src/main/java/eatda/domain/cheer/CheerTags.java(2 hunks)src/test/java/eatda/domain/cheer/CheerTagsTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/eatda/domain/cheer/Cheer.java
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: leegwichan
PR: YAPP-Github/26th-Web-Team-1-BE#159
File: src/main/java/eatda/domain/cheer/CheerTag.java:28-30
Timestamp: 2025-08-12T03:22:34.003Z
Learning: CheerTag와 Cheer 엔티티 간의 연관관계는 의도적으로 단방향 매핑(ManyToOne)으로 설계되었습니다. Cheer 엔티티에서 CheerTag 컬렉션으로의 양방향 매핑(OneToMany)은 불필요합니다.
📚 Learning: 2025-08-12T03:22:34.003Z
Learnt from: leegwichan
PR: YAPP-Github/26th-Web-Team-1-BE#159
File: src/main/java/eatda/domain/cheer/CheerTag.java:28-30
Timestamp: 2025-08-12T03:22:34.003Z
Learning: CheerTag와 Cheer 엔티티 간의 연관관계는 의도적으로 단방향 매핑(ManyToOne)으로 설계되었습니다. Cheer 엔티티에서 CheerTag 컬렉션으로의 양방향 매핑(OneToMany)은 불필요합니다.
Applied to files:
src/main/java/eatda/domain/cheer/CheerTags.java
⏰ 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/test/java/eatda/domain/cheer/CheerTagsTest.java (2)
45-48: 정상/빈 입력 케이스 검증이 명확합니다새로운 API(CheerTags.setTags)에 맞춰 성공 시나리오를 잘 커버하고 있습니다. 👍
Also applies to: 53-56
60-68: 에러 시나리오(타입별 최대 개수 초과, 중복) 테스트가 적절합니다예외 코드 매칭까지 포함해 검증 포인트가 명확합니다. 👍
Also applies to: 71-79
src/main/java/eatda/domain/cheer/CheerTags.java (3)
12-18: 설계 재검토 요청: 과거 의도(단방향 유지)와의 정합성 확인장기 학습 메모에 따르면 과거에는 CheerTag↔Cheer를 단방향(ManyToOne)로 의도적으로 설계했습니다. 이번 PR은 Cheer 쪽 @onetomany(양방향/역방향 컬렉션)를 도입합니다. 도메인 요구사항/조회 패턴 변화에 따른 전략 전환이라면 OK입니다. 다만 팀 합의/가이드라인과의 정합성만 한 번 더 확인 부탁드립니다.
30-37: 검증 로직 간결하고 명확합니다중복/타입별 최대 개수 초과를 분리해 명확히 예외를 던지는 구조가 좋습니다.
46-53: 집계 방식 적절합니다groupingBy(type)+counting로 최대값을 산출하는 방식이 가장 단순하고 명확합니다. 성능/가독성 균형 좋습니다.
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도 고생하셨습니다! 🦖
응원 단독 조회에서는 N + 1은 없을것 같은데
저희 홈 화면에서 여러개의 응원과 함께 연관된 태그를 보여줄때는
발생할 수 있을것 같아요!
일단 지금은 N + 1 문제는 넘어가고 나중에 다시 고민해보시죠 😄
| validate(cheerTagNames); | ||
| this.cheerTagNames = List.copyOf(cheerTagNames); | ||
| List<CheerTag> cheerTags = cheerTagNames.stream() | ||
| .map(name -> new CheerTag(cheer, name)) // cheer is set later |
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.
어엄? 이 주석은 무엇일까요??
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에서 지우도록 하겠습니다.
|
🎉 This PR is included in version 1.4.0-develop.85 🎉 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 📦🚀 |



✨ 개요
@OneToMany로 개선@OneToMany조회에서는 N+1이 발생하지 않음을 확인🧾 관련 이슈
closed #163
🔍 참고 사항 (선택)
Summary by CodeRabbit