Conversation
- 유효성 검증 - CursorBasedList -> Response DTO 파싱 - Response DTO에 필요한 필드 추가 쿼리 요청
Walkthrough이 변경사항은 기록장(Record) 조회 API의 전체 구조를 오프셋 기반 페이지네이션에서 커서 기반 페이지네이션으로 전환하고, DTO 및 응답 구조를 대폭 리팩토링합니다. 이에 따라 관련 서비스, 포트, 어댑터, 레포지토리, 도메인, 테스트 코드가 모두 커서 방식에 맞게 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Service
participant Repository
participant VoteRepo
participant LikeRepo
Client->>Controller: GET /records?cursor=...&type=...
Controller->>Service: search(RecordSearchQuery)
Service->>Repository: searchGroupRecordsByLatest(..., Cursor, ...)
Repository->>Repository: QueryDSL로 PostQueryDto 리스트 조회
Service->>VoteRepo: findVoteItemsByVoteIds(Set<voteId>, userId)
Service->>LikeRepo: findPostIdsLikedByUser(Set<postId>, userId)
Service->>Service: PostDto/VoteItemDto 매핑 및 블러처리
Service-->>Controller: RecordSearchResponse(postList, nextCursor, isLast)
Controller-->>Client: 응답 반환
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🧰 Additional context used🧠 Learnings (3)📓 Common learningssrc/main/java/konkuk/thip/record/adapter/out/persistence/repository/RecordQueryRepositoryImpl.java (2)Learnt from: seongjunnoh Learnt from: buzz0331 src/main/java/konkuk/thip/record/application/service/RecordSearchService.java (3)Learnt from: seongjunnoh Learnt from: buzz0331 Learnt from: seongjunnoh ⏰ 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)
🔇 Additional comments (5)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (11)
src/main/java/konkuk/thip/record/adapter/out/persistence/constants/SortType.java (1)
3-5: MINE 상수의 의미적 일관성을 검토해주세요.
CREATED_AT,LIKE_COUNT,COMMENT_COUNT는 정렬 기준이지만,MINE은 필터링 조건으로 보입니다. 의미적으로 다른 용도의 상수들이 하나의 enum에 있는 것이 적절한지 검토가 필요합니다.다음과 같은 대안을 고려해보세요:
- 정렬 전용 enum과 필터 전용 enum으로 분리
- 또는 enum에 각 상수의 역할을 명시하는 메서드 추가
public enum SortType { CREATED_AT, LIKE_COUNT, COMMENT_COUNT, MINE; public boolean isFilterType() { return this == MINE; } }src/main/java/konkuk/thip/common/util/Cursor.java (1)
18-18: DEFAULT_PAGE_SIZE를 설정 가능하게 만드는 것을 고려해주세요.하드코딩된 기본 페이지 크기는 유연성을 제한할 수 있습니다. 설정 파일이나 상수 클래스로 관리하는 것을 고려해보세요.
// application.yml 또는 별도 상수 클래스에서 관리 private static final int DEFAULT_PAGE_SIZE = Integer.parseInt(System.getProperty("cursor.default.page.size", "10"));src/main/java/konkuk/thip/room/adapter/out/jpa/RoomParticipantJpaEntity.java (1)
44-52: 테스트를 위한 유틸리티 메소드에 기본 검증 로직 추가를 고려해보세요.
@VisibleForTesting어노테이션을 통해 테스트 목적임을 명확히 표시한 것은 좋습니다. 다만 기본적인 검증 로직(예:currentPage >= 0,userPercentage범위 검증)을 추가하면 테스트 데이터의 무결성을 보장할 수 있을 것 같습니다.@VisibleForTesting public void updateCurrentPage(int currentPage) { + if (currentPage < 0) { + throw new IllegalArgumentException("currentPage must be non-negative"); + } this.currentPage = currentPage; } @VisibleForTesting public void updateUserPercentage(double userPercentage) { + if (userPercentage < 0.0 || userPercentage > 100.0) { + throw new IllegalArgumentException("userPercentage must be between 0.0 and 100.0"); + } this.userPercentage = userPercentage; }src/main/java/konkuk/thip/record/application/port/in/dto/RecordSearchQuery.java (1)
5-17: 새로운 검색 쿼리 DTO 구조가 잘 설계되었습니다.파라미터들을 하나의 record로 통합한 것은 코드 유지보수성 측면에서 좋은 접근입니다. 다만 몇 가지 개선 사항을 고려해보세요:
- 유효성 검증: 필수 필드들에 대한 validation 어노테이션 추가를 고려해보세요
- 문서화:
type,sort등 제네릭한 파라미터명에 대한 JavaDoc 추가를 권장합니다- nullable 필드 처리: 모든 필드가 nullable하므로 사용하는 곳에서 null 체크가 필요합니다
다음과 같은 개선을 고려해보세요:
+import jakarta.validation.constraints.NotNull; + +/** + * 기록장 검색을 위한 쿼리 파라미터 + * @param type 검색 타입 (mine, all 등) + * @param sort 정렬 기준 (latest, likes, comments 등) + */ @Builder public record RecordSearchQuery( + @NotNull Long roomId, String type, String sort,src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java (1)
91-114: 배치 투표 아이템 조회 구현이 효율적으로 작성되었습니다.QueryDSL 프로젝션과 서브쿼리를 적절히 활용하여 한 번의 쿼리로 필요한 모든 데이터를 가져오는 구조가 좋습니다. N+1 문제 해결에 핵심적인 역할을 하는 메서드입니다.
성능 최적화를 위해 다음 사항들을 고려해보세요:
- 인덱스 확인:
vote_participant테이블의(vote_item_id, user_id)복합 인덱스 존재 여부- 빈 Set 처리:
voteIds가 비어있을 때의 처리 로직 추가@Override public List<VoteItemQueryDto> mapVoteItemsByVoteIds(Set<Long> voteIds, Long userId) { + if (voteIds.isEmpty()) { + return Collections.emptyList(); + } + QVoteItemJpaEntity voteItem = QVoteItemJpaEntity.voteItemJpaEntity;src/main/java/konkuk/thip/record/application/port/out/RecordQueryPort.java (1)
9-15: 메서드 통합을 고려해보세요현재 정렬 타입별로 별도의 메서드가 정의되어 있는데,
RecordQueryRepository처럼SortType파라미터를 받는 단일 메서드로 통합하면 인터페이스가 더 간결해질 수 있습니다.- CursorBasedList<PostQueryDto> searchGroupRecordsByLatest(Long roomId, Long userId, Cursor cursor, Integer pageStart, Integer pageEnd, Boolean isOverview); - - CursorBasedList<PostQueryDto> searchGroupRecordsByLike(Long roomId, Long userId, Cursor cursor, Integer pageStart, Integer pageEnd, Boolean isOverview); - - CursorBasedList<PostQueryDto> searchGroupRecordsByComment(Long roomId, Long userId, Cursor cursor, Integer pageStart, Integer pageEnd, Boolean isOverview); + CursorBasedList<PostQueryDto> searchGroupRecords(Long roomId, Long userId, Cursor cursor, Integer pageStart, Integer pageEnd, Boolean isOverview, SortType sortType);src/test/java/konkuk/thip/record/adapter/in/web/RecordSearchApiTest.java (2)
89-94: 조건문 로직을 더 명확하게 개선하세요
isLast가false일 때nextCursor가 비어있지 않은지도 확인하는 것이 좋습니다.- if(isLast) { - assertThat(nextCursor.isNull()).isTrue(); - } else { - assertThat(nextCursor.isTextual()).isTrue(); - } + if(isLast) { + assertThat(nextCursor.isNull()).isTrue(); + } else { + assertThat(nextCursor.isTextual()).isTrue(); + assertThat(nextCursor.asText()).isNotEmpty(); + }
405-464: 테스트 데이터 생성 메서드의 중복을 줄이세요
createTestData()와createTestDataWithLockedContent()메서드에서 사용자, 책, 방 생성 로직이 중복됩니다. 공통 로직을 추출하면 유지보수성이 향상됩니다.예를 들어, 기본 엔티티 생성을 위한 헬퍼 메서드를 추가할 수 있습니다:
private UserJpaEntity createTestUser(AliasJpaEntity alias) { return userJpaRepository.save(UserJpaEntity.builder() .oauth2Id("kakao_123") .nickname("테스트사용자") .imageUrl("http://user.img") .role(UserRole.USER) .aliasForUserJpaEntity(alias) .build()); } private BookJpaEntity createTestBook() { return bookJpaRepository.save(BookJpaEntity.builder() .isbn("1234567890") .title("테스트책") .authorName("작가") .publisher("출판사") .pageCount(200) .description("책 설명") .imageUrl("http://book.img") .bestSeller(false) .build()); }src/main/java/konkuk/thip/record/adapter/in/web/response/RecordSearchResponse.java (1)
36-38: 불필요한 정적 팩토리 메서드
VoteItemDto는 이미 record이고@Builder가 있으므로 정적 팩토리 메서드of가 중복됩니다. record의 기본 생성자나 빌더를 사용하는 것이 더 일관성 있습니다.- public static VoteItemDto of(Long voteItemId, String itemName, int percentage, boolean isVoted) { - return new VoteItemDto(voteItemId, itemName, percentage, isVoted); - }src/main/java/konkuk/thip/record/adapter/out/persistence/repository/RecordQueryRepositoryImpl.java (2)
148-183: 커서 조건 생성 로직에 주석을 추가하세요
buildCursorPredicateForSortType메서드의 로직이 복잡합니다. 각 정렬 타입별 커서 처리 방식을 설명하는 주석을 추가하면 유지보수성이 향상됩니다.예시:
case MINE -> { // 내 기록 정렬: 총평(내림차순) > 페이지(내림차순) > ID(내림차순) Integer isOverview = cursor.getInteger(0); Integer page = cursor.getInteger(1); Long postId = cursor.getLong(2); // ... }
125-146: 매직 넘버 대신 상수를 사용하세요
CaseBuilder의otherwise절에서 사용되는 0과 같은 매직 넘버를 상수로 정의하면 코드의 의도가 더 명확해집니다.+ private static final int DEFAULT_PAGE = 0; + private static final int DEFAULT_OVERVIEW_FLAG = 0; + private NumberExpression<Integer> pageExpr() { return new CaseBuilder() .when(post.instanceOf(RecordJpaEntity.class)).then(record.page) .when(post.instanceOf(VoteJpaEntity.class)).then(vote.page) - .otherwise(0); + .otherwise(DEFAULT_PAGE); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
build.gradle(1 hunks)src/main/java/konkuk/thip/common/util/Cursor.java(1 hunks)src/main/java/konkuk/thip/post/adapter/out/persistence/PostLikeJpaRepository.java(1 hunks)src/main/java/konkuk/thip/post/adapter/out/persistence/PostLikeQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/post/application/port/out/PostLikeQueryPort.java(1 hunks)src/main/java/konkuk/thip/record/adapter/in/web/RecordQueryController.java(2 hunks)src/main/java/konkuk/thip/record/adapter/in/web/response/RecordDto.java(0 hunks)src/main/java/konkuk/thip/record/adapter/in/web/response/RecordSearchResponse.java(1 hunks)src/main/java/konkuk/thip/record/adapter/in/web/response/VoteDto.java(0 hunks)src/main/java/konkuk/thip/record/adapter/out/persistence/RecordQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/record/adapter/out/persistence/RecordSearchTypeParams.java(1 hunks)src/main/java/konkuk/thip/record/adapter/out/persistence/constants/PostType.java(1 hunks)src/main/java/konkuk/thip/record/adapter/out/persistence/constants/SortType.java(1 hunks)src/main/java/konkuk/thip/record/adapter/out/persistence/repository/RecordQueryRepository.java(1 hunks)src/main/java/konkuk/thip/record/adapter/out/persistence/repository/RecordQueryRepositoryImpl.java(2 hunks)src/main/java/konkuk/thip/record/application/port/in/dto/RecordSearchQuery.java(1 hunks)src/main/java/konkuk/thip/record/application/port/in/dto/RecordSearchResult.java(0 hunks)src/main/java/konkuk/thip/record/application/port/in/dto/RecordSearchUseCase.java(1 hunks)src/main/java/konkuk/thip/record/application/port/out/RecordQueryPort.java(1 hunks)src/main/java/konkuk/thip/record/application/port/out/dto/PostQueryDto.java(1 hunks)src/main/java/konkuk/thip/record/application/service/RecordSearchService.java(2 hunks)src/main/java/konkuk/thip/room/adapter/out/jpa/RoomParticipantJpaEntity.java(2 hunks)src/main/java/konkuk/thip/vote/adapter/out/persistence/VoteCommandPersistenceAdapter.java(0 hunks)src/main/java/konkuk/thip/vote/adapter/out/persistence/VoteQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepository.java(1 hunks)src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java(3 hunks)src/main/java/konkuk/thip/vote/application/port/out/VoteCommandPort.java(0 hunks)src/main/java/konkuk/thip/vote/application/port/out/VoteQueryPort.java(1 hunks)src/main/java/konkuk/thip/vote/application/port/out/dto/VoteItemQueryDto.java(1 hunks)src/main/java/konkuk/thip/vote/domain/VoteItem.java(2 hunks)src/test/java/konkuk/thip/record/adapter/in/web/RecordQueryControllerTest.java(0 hunks)src/test/java/konkuk/thip/record/adapter/in/web/RecordSearchApiTest.java(1 hunks)src/test/java/konkuk/thip/record/adapter/out/persistence/RecordQueryRepositoryImplTest.java(0 hunks)src/test/java/konkuk/thip/record/application/service/RecordSearchServiceTest.java(0 hunks)src/test/java/konkuk/thip/vote/domain/VoteItemTest.java(1 hunks)
💤 Files with no reviewable changes (8)
- src/main/java/konkuk/thip/vote/application/port/out/VoteCommandPort.java
- src/main/java/konkuk/thip/vote/adapter/out/persistence/VoteCommandPersistenceAdapter.java
- src/main/java/konkuk/thip/record/application/port/in/dto/RecordSearchResult.java
- src/test/java/konkuk/thip/record/application/service/RecordSearchServiceTest.java
- src/test/java/konkuk/thip/record/adapter/out/persistence/RecordQueryRepositoryImplTest.java
- src/test/java/konkuk/thip/record/adapter/in/web/RecordQueryControllerTest.java
- src/main/java/konkuk/thip/record/adapter/in/web/response/RecordDto.java
- src/main/java/konkuk/thip/record/adapter/in/web/response/VoteDto.java
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/main/java/konkuk/thip/post/adapter/out/persistence/PostLikeJpaRepository.java (1)
src/main/java/konkuk/thip/post/adapter/out/persistence/PostLikeQueryPersistenceAdapter.java (1)
Repository(9-19)
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java (1)
src/main/java/konkuk/thip/vote/adapter/out/persistence/VoteQueryPersistenceAdapter.java (1)
Repository(19-40)
src/test/java/konkuk/thip/record/adapter/in/web/RecordSearchApiTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(26-225)
src/main/java/konkuk/thip/record/adapter/out/persistence/RecordQueryPersistenceAdapter.java (1)
src/main/java/konkuk/thip/record/adapter/out/persistence/repository/RecordQueryRepositoryImpl.java (1)
Repository(27-209)
⏰ 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: build
🔇 Additional comments (31)
build.gradle (1)
71-73: Guava 의존성 스코프 재검토 요청
- RoomParticipantJpaEntity.java에서
import com.google.common.annotations.VisibleForTesting;
및@VisibleForTesting어노테이션 사용이 확인되었습니다.- 현재 build.gradle(71–73줄)에
compileOnly 'com.google.guava:guava:33.0.0-jre'로만 선언되어 있어, 런타임 classpath에는 Guava가 포함되지 않습니다.- @VisibleForTesting 어노테이션의 리텐션 정책(SOURCE/CLASS/RUNTIME)을 확인 후:
- 런타임에도 어노테이션 클래스가 필요하다면
implementation또는provided로 변경- 테스트 코드 전용이라면
testImplementation으로 변경- 컴파일 시점에만 필요하다면
compileOnly유지위 사항을 검토하여 적절한 스코프로 조정해주세요.
src/main/java/konkuk/thip/common/util/Cursor.java (1)
73-81: 타입 변환 예외 처리가 잘 구현되었습니다.제네릭을 활용한 타입 변환 로직과 상세한 예외 메시지가 훌륭합니다. 디버깅에 도움이 될 것입니다.
src/main/java/konkuk/thip/record/adapter/out/persistence/RecordSearchTypeParams.java (1)
25-25: 예외 메시지가 명확하게 개선되었습니다.사용자가 어떤 값이 허용되는지 명확히 알 수 있고, 현재 입력된 잘못된 값도 함께 표시하여 디버깅에 도움이 됩니다.
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepository.java (1)
16-16: N+1 쿼리 문제 해결을 위한 훌륭한 접근 방식입니다.여러 voteId를 배치로 처리하는 이 메서드는 PR 목표에서 언급된 N+1 쿼리 문제를 효과적으로 해결할 것입니다. 메서드명도 의도를 명확히 표현하고 있습니다.
이전의 개별 조회 방식에서 배치 처리로 변경하여 2N번의 쿼리를 3번의 쿼리로 줄이는 최적화가 잘 반영되었습니다.
src/main/java/konkuk/thip/record/adapter/out/persistence/constants/PostType.java (1)
5-17: 깔끔한 enum 구현으로 포스트 타입 통합을 지원합니다.기존의 다형성 처리에서 통합된 포스트 추상화로의 리팩토링을 위한 기본적인 상수 정의가 잘 구현되었습니다. 간단하고 명확한 구조로 커서 기반 페이지네이션과 쿼리 레이어 전반에서 일관된 포스트 타입 구분을 제공할 것으로 보입니다.
src/main/java/konkuk/thip/vote/application/port/out/dto/VoteItemQueryDto.java (1)
6-21: N+1 쿼리 문제 해결을 위한 효과적인 DTO 설계입니다.@QueryProjection 어노테이션과 함께 배치 쿼리를 지원하는 구조로 잘 설계되었습니다. 모든 필드에 대한 null 검증도 적절하게 구현되어 있어 안전한 데이터 전송을 보장합니다.
src/main/java/konkuk/thip/record/application/port/in/dto/RecordSearchUseCase.java (1)
7-7: 매개변수 통합으로 인터페이스가 더욱 깔끔해졌습니다.여러 개의 개별 매개변수를
RecordSearchQuery객체 하나로 통합한 것은 좋은 리팩토링입니다. 커서 기반 페이지네이션 도입과 함께 인터페이스의 유지보수성과 가독성이 향상되었습니다.src/main/java/konkuk/thip/post/adapter/out/persistence/PostLikeJpaRepository.java (1)
17-19: N+1 쿼리 문제 해결을 위한 효과적인 배치 쿼리 구현입니다.네이티브 SQL 쿼리를 사용하여 여러 포스트의 좋아요 상태를 한 번에 조회할 수 있도록 구현된 것이 좋습니다.
@Param어노테이션을 통한 매개변수 바인딩과 IN 절을 활용한 배치 처리가 적절하게 구현되었습니다.src/main/java/konkuk/thip/vote/application/port/out/VoteQueryPort.java (1)
15-15: N+1 쿼리 문제 해결을 위한 배치 조회 메서드 추가가 훌륭합니다.개별 투표 확인에서 배치 조회로 변경한 것은 성능 향상에 큰 도움이 될 것입니다. PR 목표에서 언급한 "2N개의 쿼리에서 API 호출당 3개의 쿼리로 감소"를 달성하는 핵심 변경사항입니다.
src/main/java/konkuk/thip/record/application/port/out/dto/PostQueryDto.java (2)
8-20: QueryDSL 통합을 위한 @QueryProjection 사용이 적절합니다.커서 기반 페이지네이션과 함께 사용하기에 적합한 구조입니다. 통합된 포스트 DTO로 레코드와 투표 포스트를 모두 처리할 수 있게 설계된 것이 좋습니다.
21-34: 통합 DTO와 null 검증: 설계는 적절하나 DB 매핑 확인 필요모든 필드에 대한
Assert.notNull()검증은 데이터 무결성을 높여주지만, 실제 저장소 측에서도 동일하게 null을 허용하지 않는지 확인해야 합니다.확인이 필요한 사항:
- JPA 엔티티 클래스에서
@Column(nullable = false)설정 여부 (profileImageUrl,nickName등)- 실제 데이터베이스 스키마의 NOT NULL 제약 조건
추가로, 생성자 검증 로직이 자주 호출될 경우 성능에 미칠 영향도 검토해 보세요.
src/main/java/konkuk/thip/post/application/port/out/PostLikeQueryPort.java (1)
5-8: 배치 조회로의 인터페이스 단순화가 훌륭합니다.개별 포스트별 좋아요 확인에서 배치 조회로 변경한 것은 N+1 쿼리 문제 해결에 핵심적인 역할을 합니다. 메서드 시그니처도 명확하고 효율적입니다.
src/main/java/konkuk/thip/vote/adapter/out/persistence/repository/VoteQueryRepositoryImpl.java (1)
102-109: 서브쿼리를 통한 투표 여부 확인이 적절합니다.
JPAExpressions.exists()를 사용한 서브쿼리 구조가 정확하고, 사용자별 투표 여부를 효율적으로 확인할 수 있습니다. 관련된 persistence adapter의findVoteItemsByVoteIds메서드와도 잘 연동됩니다.src/test/java/konkuk/thip/vote/domain/VoteItemTest.java (1)
1-74: 포괄적인 테스트 커버리지로 잘 작성되었습니다.새로운 정적 메서드
calculatePercentages에 대한 테스트가 다양한 시나리오를 포함하여 잘 작성되었습니다. 특히 소수점 반올림 처리와 합계가 정확히 100이 되는지 확인하는 부분이 좋습니다.src/main/java/konkuk/thip/post/adapter/out/persistence/PostLikeQueryPersistenceAdapter.java (1)
16-18: N+1 쿼리 문제 해결을 위한 배치 쿼리로 개선되었습니다.기존의 개별 포스트별 좋아요 확인 방식에서 배치 쿼리 방식으로 변경하여 성능이 크게 개선되었습니다. PR 목표에 명시된 N+1 쿼리 문제 해결에 부합하는 변경사항입니다.
src/main/java/konkuk/thip/record/adapter/in/web/RecordQueryController.java (2)
20-31: 한글 주석으로 API 파라미터 설명이 명확해졌습니다.새로운 파라미터들(
isPageFilter,cursor)에 대한 설명이 잘 작성되어 있어 API 사용자가 이해하기 쉽습니다.
44-56: 빌더 패턴을 통한 파라미터 처리가 개선되었습니다.
RecordSearchQuery빌더 패턴을 사용하여 파라미터 전달이 명확해지고 유지보수성이 향상되었습니다. 기본값 설정도 적절합니다.src/main/java/konkuk/thip/vote/adapter/out/persistence/VoteQueryPersistenceAdapter.java (1)
31-39: 배치 쿼리를 통한 투표 아이템 조회로 성능이 개선되었습니다.기존의 개별 투표별 조회 방식에서 배치 쿼리 방식으로 변경하여 N+1 쿼리 문제를 해결했습니다. 스트림 API를 사용한 그룹핑 로직도 적절합니다.
src/main/java/konkuk/thip/vote/domain/VoteItem.java (1)
33-76: 백분율 계산 알고리즘이 크게 개선되었습니다.새로운 정적 메서드는 다음과 같은 장점을 제공합니다:
- 배치 처리로 성능 향상
- 합계가 정확히 100이 되도록 보장
- 소수점 반올림을 공정하게 분배하는 정교한 알고리즘
- 엣지 케이스(총합 0, 빈 리스트) 적절히 처리
특히 소수점 부분을 기준으로 정렬하여 나머지 퍼센트를 분배하는 로직이 매우 잘 구현되었습니다.
src/main/java/konkuk/thip/record/adapter/out/persistence/repository/RecordQueryRepository.java (1)
11-13: 인터페이스 변경이 적절합니다커서 기반 페이지네이션으로의 전환과 통합된
PostQueryDto사용이 PR 목표와 잘 부합합니다. 메서드 시그니처가 명확하고 파라미터 구성도 적절합니다.src/test/java/konkuk/thip/record/adapter/in/web/RecordSearchApiTest.java (1)
63-604: 통합 테스트가 포괄적으로 잘 작성되었습니다커서 기반 페이지네이션, 정렬, 필터링, 접근 제어 등 주요 기능들을 잘 검증하고 있습니다. 오류 케이스 처리도 적절합니다.
src/main/java/konkuk/thip/record/adapter/in/web/response/RecordSearchResponse.java (1)
7-41: 응답 구조가 PR 목표에 잘 부합합니다커서 기반 페이지네이션, 통합된 Post 구조, 그리고 블러 처리를 위한
isLocked플래그 등이 적절히 구현되었습니다.src/main/java/konkuk/thip/record/adapter/out/persistence/repository/RecordQueryRepositoryImpl.java (2)
47-57: N+1 쿼리 문제가 효과적으로 해결되었습니다QueryDSL projection과 join을 활용하여 단일 쿼리로 필요한 데이터를 조회하는 방식이 적절합니다. PR 목표에서 언급한 2N 쿼리를 3개 쿼리로 줄이는 최적화가 잘 구현되었습니다.
Also applies to: 83-93
194-208: 프로젝션 메서드가 잘 구현되었습니다
postTypeExpr()주석에서 언급한 것처럼 향후 상속 구조 변경 시 type 필드로 구분하는 방향이 좋아 보입니다.src/main/java/konkuk/thip/record/application/service/RecordSearchService.java (4)
49-109: N+1 쿼리 문제 해결이 효과적으로 구현되었습니다!투표 항목과 좋아요 정보를 일괄 조회하여 성능을 최적화한 접근이 좋습니다. PR 목표에 명시된 대로 API 호출당 3개의 쿼리로 줄이는 방식이 잘 구현되었습니다.
119-140: 블러 처리 로직이 잘 구현되었습니다!읽지 않은 페이지에 대한 블러 처리가 PR 목표에 맞게 올바르게 구현되었습니다.
isLocked상태에 따라 content와 voteItems를 적절히 처리하는 방식이 좋습니다.
142-169: 투표 항목 처리 로직이 깔끔하게 구현되었습니다!도메인 모델에 계산을 위임하고, IntStream을 활용한 인덱스 기반 매핑이 효율적입니다.
210-216: 내 기록 조회 검증 로직이 적절합니다.불필요한 파라미터 전달을 방지하는 검증이 잘 구현되었습니다.
src/main/java/konkuk/thip/record/adapter/out/persistence/RecordQueryPersistenceAdapter.java (3)
20-30: 커서 기반 페이지네이션이 잘 구현되었습니다!PR 목표에 명시된 복합 커서 구조가 올바르게 구현되었으며, UTF-8 인코딩을 통해 '|' 구분자 문제를 해결한 접근이 좋습니다.
44-66: 좋아요 및 댓글 정렬 커서 구현이 적절합니다.각 정렬 타입에 맞는 필드를 커서에 포함시켜 일관된 페이지네이션을 보장합니다.
37-40: 필드 매핑 일관성 확인됨
PostQueryDto의postDate필드는RecordQueryRepositoryImpl의selectPostQueryDto()에서post.createdAt으로 매핑되어 있어,CREATED_AT정렬 기준과 완전히 일치합니다.
추가 수정이 필요 없습니다.
| public static Cursor from(String encoded, int pageSize) { | ||
| if (encoded == null || !encoded.contains("|")) { | ||
| return new Cursor(List.of(), pageSize); // 빈 커서 생성 | ||
| } | ||
| String decoded = URLDecoder.decode(encoded, StandardCharsets.UTF_8); | ||
| List<String> parts = Arrays.asList(decoded.split(SPLIT_DELIMITER)); | ||
| return new Cursor(parts, pageSize); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
커서 디코딩 로직의 견고성을 개선해주세요.
현재 encoded.contains("|") 체크만으로는 불완전한 커서 문자열을 정확히 검증하기 어렵습니다.
다음 케이스들을 고려해보세요:
- 빈 문자열
"" - 파이프만 있는 경우
"|"또는"||" - URL 디코딩 실패 케이스
public static Cursor from(String encoded, int pageSize) {
if (encoded == null || encoded.trim().isEmpty()) {
return new Cursor(List.of(), pageSize);
}
try {
String decoded = URLDecoder.decode(encoded, StandardCharsets.UTF_8);
if (decoded.trim().isEmpty()) {
return new Cursor(List.of(), pageSize);
}
List<String> parts = Arrays.asList(decoded.split(SPLIT_DELIMITER));
return new Cursor(parts, pageSize);
} catch (Exception e) {
throw new IllegalArgumentException("잘못된 커서 형식입니다: " + encoded, e);
}
}🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/util/Cursor.java around lines 36 to 43,
improve the robustness of the cursor decoding logic by replacing the simple
encoded.contains("|") check with a more comprehensive validation. First, check
if the encoded string is null or empty (after trimming) and return an empty
cursor if so. Then, wrap the URL decoding in a try-catch block to handle
decoding failures gracefully by throwing an IllegalArgumentException with a
clear message. After decoding, verify the decoded string is not empty before
splitting and creating the Cursor. This ensures all edge cases like empty
strings, strings with only pipes, and decoding errors are properly handled.
| private CursorBasedList<PostQueryDto> getGroupRecordBySortParams(String sort, Long roomId, Long userId, Cursor cursor, Integer pageStart, Integer pageEnd, Boolean isPageFilter, Boolean isOverview) { | ||
| return switch(RecordSearchSortParams.from(sort)) { | ||
| case LATEST -> recordQueryPort.searchGroupRecordsByLatest(roomId, userId, cursor, pageStart, pageEnd, isOverview); | ||
| case LIKE -> recordQueryPort.searchGroupRecordsByLike(roomId, userId, cursor, pageStart, pageEnd, isOverview); | ||
| case COMMENT -> recordQueryPort.searchGroupRecordsByComment(roomId, userId, cursor, pageStart, pageEnd, isOverview); | ||
| }; | ||
| } |
There was a problem hiding this comment.
isPageFilter 파라미터가 사용되지 않습니다.
getGroupRecordBySortParams 메서드에서 isPageFilter 파라미터를 받고 있지만 실제로 사용되지 않습니다. PR 목표에서 페이지 필터 칩의 토글 상태를 나타내는 중요한 파라미터로 언급되었으므로, 구현이 누락된 것으로 보입니다.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/record/application/service/RecordSearchService.java
around lines 111 to 117, the isPageFilter parameter is declared but not used in
the getGroupRecordBySortParams method. To fix this, incorporate the isPageFilter
parameter into the calls to recordQueryPort methods or the method logic as
intended, ensuring it affects the query or filtering behavior according to the
page filter toggle state described in the PR goals.
| //todo : 블러 처리 contents 길이에 맞게 수정 | ||
| private String createBlurredString(String contents) { | ||
| if (contents == null || contents.isEmpty()) { | ||
| return contents; // 빈 문자열이나 null은 그대로 반환 | ||
| } | ||
| // 블러 처리 로직 (예: 내용의 일부를 '띱'으로 대체) | ||
| return contents.replaceAll(".", BLURRED_STRING); // 모든 문자를 '띱'으로 대체 | ||
| } |
There was a problem hiding this comment.
블러 처리 로직에 심각한 오류가 있습니다.
현재 구현은 각 문자를 전체 BLURRED_STRING으로 대체하여 매우 긴 문자열을 생성합니다. 예를 들어 "안녕"은 "여긴 못 지나가지롱여긴 못 지나가지롱"가 됩니다.
다음과 같이 수정하는 것을 제안합니다:
- private String createBlurredString(String contents) {
- if (contents == null || contents.isEmpty()) {
- return contents; // 빈 문자열이나 null은 그대로 반환
- }
- // 블러 처리 로직 (예: 내용의 일부를 '띱'으로 대체)
- return contents.replaceAll(".", BLURRED_STRING); // 모든 문자를 '띱'으로 대체
- }
+ private String createBlurredString(String contents) {
+ if (contents == null || contents.isEmpty()) {
+ return contents;
+ }
+ // 컨텐츠 길이에 따라 적절한 블러 메시지 반환
+ if (contents.length() <= 10) {
+ return BLURRED_STRING;
+ }
+ // 긴 컨텐츠의 경우 일부만 보여주고 블러 처리
+ return contents.substring(0, 5) + "... " + BLURRED_STRING;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //todo : 블러 처리 contents 길이에 맞게 수정 | |
| private String createBlurredString(String contents) { | |
| if (contents == null || contents.isEmpty()) { | |
| return contents; // 빈 문자열이나 null은 그대로 반환 | |
| } | |
| // 블러 처리 로직 (예: 내용의 일부를 '띱'으로 대체) | |
| return contents.replaceAll(".", BLURRED_STRING); // 모든 문자를 '띱'으로 대체 | |
| } | |
| private String createBlurredString(String contents) { | |
| if (contents == null || contents.isEmpty()) { | |
| return contents; | |
| } | |
| // 컨텐츠 길이에 따라 적절한 블러 메시지 반환 | |
| if (contents.length() <= 10) { | |
| return BLURRED_STRING; | |
| } | |
| // 긴 컨텐츠의 경우 일부만 보여주고 블러 처리 | |
| return contents.substring(0, 5) + "... " + BLURRED_STRING; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/record/application/service/RecordSearchService.java
around lines 171 to 178, the createBlurredString method replaces each character
with the entire BLURRED_STRING, causing the output to be excessively long. To
fix this, modify the method so that it returns a string consisting of the
BLURRED_STRING repeated exactly as many times as the length of the input
contents, rather than replacing each character individually. This will ensure
the blurred string length matches the original content length without
unnecessary repetition.
src/main/java/konkuk/thip/record/application/service/RecordSearchService.java
Outdated
Show resolved
Hide resolved
src/main/java/konkuk/thip/vote/application/port/out/dto/VoteItemQueryDto.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/konkuk/thip/record/application/service/RecordSearchService.java (1)
111-117: 이전 리뷰 코멘트가 해결되지 않았습니다.
isPageFilter매개변수가 여전히 사용되지 않고 있습니다. PR 목표에서 페이지 필터 칩의 토글 상태를 나타내는 중요한 매개변수로 언급했으므로, 이를 실제 쿼리 로직에 적용해야 합니다.다음과 같이
isPageFilter를 활용하도록 수정하는 것을 제안합니다:- private CursorBasedList<PostQueryDto> getGroupRecordBySortParams(String sort, Long roomId, Long userId, Cursor cursor, Integer pageStart, Integer pageEnd, Boolean isPageFilter, Boolean isOverview) { - return switch(RecordSearchSortParams.from(sort)) { - case LATEST -> recordQueryPort.searchGroupRecordsByLatest(roomId, userId, cursor, pageStart, pageEnd, isOverview); - case LIKE -> recordQueryPort.searchGroupRecordsByLike(roomId, userId, cursor, pageStart, pageEnd, isOverview); - case COMMENT -> recordQueryPort.searchGroupRecordsByComment(roomId, userId, cursor, pageStart, pageEnd, isOverview); - }; + private CursorBasedList<PostQueryDto> getGroupRecordBySortParams(String sort, Long roomId, Long userId, Cursor cursor, Integer pageStart, Integer pageEnd, Boolean isPageFilter, Boolean isOverview) { + return switch(RecordSearchSortParams.from(sort)) { + case LATEST -> recordQueryPort.searchGroupRecordsByLatest(roomId, userId, cursor, pageStart, pageEnd, isPageFilter, isOverview); + case LIKE -> recordQueryPort.searchGroupRecordsByLike(roomId, userId, cursor, pageStart, pageEnd, isPageFilter, isOverview); + case COMMENT -> recordQueryPort.searchGroupRecordsByComment(roomId, userId, cursor, pageStart, pageEnd, isPageFilter, isOverview); + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/konkuk/thip/record/application/service/RecordSearchService.java(2 hunks)
🔇 Additional comments (8)
src/main/java/konkuk/thip/record/application/service/RecordSearchService.java (8)
5-9: 새로운 의존성들이 적절히 추가되었습니다.커서 기반 페이지네이션 구현을 위한
Cursor,CursorBasedList및 예외 처리를 위한BusinessException등 필요한 유틸리티 클래스들이 추가되었습니다.
50-61: 통합된 쿼리 객체와 초기 검증 로직이 잘 구현되었습니다.
RecordSearchQuery객체를 사용하여 매개변수를 통합하고, 책과 참가자 정보를 미리 조회하여 유효성을 검증하는 것이 좋은 접근 방식입니다.
67-85: 타입별 분기 처리가 명확하게 구현되었습니다.Switch expression을 사용하여 GROUP과 MINE 타입에 따른 처리를 명확히 분리했습니다. 각각에 대한 검증 로직도 적절히 분리되어 있습니다.
87-96: N+1 쿼리 문제가 효과적으로 해결되었습니다.투표 항목과 좋아요 정보를 배치로 조회하여 N+1 쿼리 문제를 해결한 것은 훌륭한 성능 개선입니다. Stream API를 활용한 데이터 변환 로직도 깔끔합니다.
119-140: DTO 변환 로직이 잘 구조화되었습니다.잠금 상태 확인, 작성자 여부 판단, 블러 처리 등의 로직이 명확하게 구현되어 있습니다. 배치로 조회한 데이터를 효율적으로 활용하고 있습니다.
151-169: 투표 항목 비율 계산 로직이 개선되었습니다.도메인 로직에 비율 계산을 위임하고, IntStream을 활용한 DTO 매핑이 효율적입니다. 잠금 상태에 따른 블러 처리도 적절히 적용되었습니다.
192-220: 검증 로직의 논리적 오류가 수정되었습니다.이전 리뷰에서 지적된 AND 연산자 문제가 OR 연산자로 수정되어 올바른 검증 로직이 구현되었습니다. 다양한 필터 조합에 대한 검증 규칙도 명확하게 정의되어 있습니다.
222-228: 내 기록 조회에 대한 엄격한 검증이 구현되었습니다.MINE 타입에서는 필요한 매개변수를 제외하고는 모두 null이어야 한다는 제약이 명확하게 구현되어 있습니다.
| private String createBlurredString(String contents) { | ||
| if (contents == null || contents.isEmpty()) { | ||
| return contents; | ||
| } | ||
| if (pageNum < 1) { | ||
| throw new InvalidStateException(ErrorCode.API_INVALID_PARAM, new IllegalArgumentException("pageNum은 1 이상의 값이어야 합니다.")); | ||
|
|
||
| int originalLength = contents.length(); | ||
| int blurLen = BLURRED_STRING.length(); | ||
|
|
||
| // 필요한 전체 반복 횟수 계산 | ||
| int repeat = originalLength / blurLen; | ||
|
|
||
| StringBuilder sb = new StringBuilder(originalLength); | ||
|
|
||
| // 몫 만큼 반복 | ||
| for (int i = 0; i < repeat + 1; i++) { | ||
| sb.append(BLURRED_STRING); | ||
| } | ||
| return pageNum; | ||
| } | ||
|
|
||
| private Sort buildSort(RecordSearchSortParams sort) { | ||
| return switch (sort) { | ||
| case LIKE -> Sort.by(Sort.Direction.DESC, "likeCount"); | ||
| case COMMENT -> Sort.by(Sort.Direction.DESC, "commentCount"); | ||
| default -> Sort.by(Sort.Direction.DESC, "createdAt"); | ||
| }; | ||
| return sb.toString(); | ||
| } |
There was a problem hiding this comment.
블러 처리 로직이 부분적으로 개선되었지만 여전히 문제가 있습니다.
이전 리뷰에서 지적된 각 문자를 전체 BLURRED_STRING으로 대체하는 문제는 해결되었지만, 새로운 구현에서도 문제가 있습니다.
현재 로직은 repeat + 1번 반복하여 원본보다 항상 긴 문자열을 생성합니다. 또한 원본 길이가 블러 문자열보다 짧을 때 불필요하게 긴 문자열을 생성할 수 있습니다.
다음과 같이 수정하는 것을 제안합니다:
private String createBlurredString(String contents) {
if (contents == null || contents.isEmpty()) {
return contents;
}
-
- int originalLength = contents.length();
- int blurLen = BLURRED_STRING.length();
-
- // 필요한 전체 반복 횟수 계산
- int repeat = originalLength / blurLen;
-
- StringBuilder sb = new StringBuilder(originalLength);
-
- // 몫 만큼 반복
- for (int i = 0; i < repeat + 1; i++) {
- sb.append(BLURRED_STRING);
- }
-
- return sb.toString();
+ // 원본 길이에 관계없이 적절한 블러 메시지 반환
+ return BLURRED_STRING;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private String createBlurredString(String contents) { | |
| if (contents == null || contents.isEmpty()) { | |
| return contents; | |
| } | |
| if (pageNum < 1) { | |
| throw new InvalidStateException(ErrorCode.API_INVALID_PARAM, new IllegalArgumentException("pageNum은 1 이상의 값이어야 합니다.")); | |
| int originalLength = contents.length(); | |
| int blurLen = BLURRED_STRING.length(); | |
| // 필요한 전체 반복 횟수 계산 | |
| int repeat = originalLength / blurLen; | |
| StringBuilder sb = new StringBuilder(originalLength); | |
| // 몫 만큼 반복 | |
| for (int i = 0; i < repeat + 1; i++) { | |
| sb.append(BLURRED_STRING); | |
| } | |
| return pageNum; | |
| } | |
| private Sort buildSort(RecordSearchSortParams sort) { | |
| return switch (sort) { | |
| case LIKE -> Sort.by(Sort.Direction.DESC, "likeCount"); | |
| case COMMENT -> Sort.by(Sort.Direction.DESC, "commentCount"); | |
| default -> Sort.by(Sort.Direction.DESC, "createdAt"); | |
| }; | |
| return sb.toString(); | |
| } | |
| private String createBlurredString(String contents) { | |
| if (contents == null || contents.isEmpty()) { | |
| return contents; | |
| } | |
| // 원본 길이에 관계없이 적절한 블러 메시지 반환 | |
| return BLURRED_STRING; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/record/application/service/RecordSearchService.java
between lines 171 and 190, the createBlurredString method currently generates a
blurred string longer than the original by repeating BLURRED_STRING repeat + 1
times without trimming. To fix this, calculate the exact number of repeats
needed and then truncate the final string to match the original content length
exactly, ensuring the blurred string is never longer than the original.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/konkuk/thip/record/adapter/out/persistence/repository/RecordQueryRepositoryImpl.java (1)
200-200: 주석의 TODO 항목을 추적하는 것이 좋겠습니다.상속 구조 해지에 대한 주석이 있는데, 이는 향후 리팩토링 계획을 나타내는 것 같습니다.
향후 상속 구조 해지 작업을 위한 이슈를 생성하여 추적하는 것이 좋겠습니다. 필요하시면 관련 이슈를 생성해드릴 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/konkuk/thip/record/adapter/out/persistence/repository/RecordQueryRepositoryImpl.java(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: buzz0331
PR: THIP-TextHip/THIP-Server#75
File: src/main/java/konkuk/thip/vote/adapter/out/persistence/VoteQueryRepositoryImpl.java:50-83
Timestamp: 2025-07-14T14:19:38.796Z
Learning: Vote와 VoteItem 엔티티는 자주 함께 사용되므로, N+1 문제를 방지하기 위해 양방향 매핑과 fetch join을 고려하는 것이 좋습니다. 특히 기록장 조회 API 등에서도 함께 사용될 가능성이 높습니다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.820Z
Learning: THIP 프로젝트에서 Record와 Vote는 Room에 속하지만 Feed는 Room에 속하지 않는 구조이며, 댓글 작성 시 Record/Vote에 대해서만 사용자가 해당 Room의 참가자인지 검증이 필요하다.
Learnt from: buzz0331
PR: THIP-TextHip/THIP-Server#78
File: src/main/java/konkuk/thip/user/application/port/out/FollowingQueryPort.java:3-3
Timestamp: 2025-07-14T18:22:56.538Z
Learning: THIP 프로젝트에서는 Query API(조회 API)에 한해서는 application 계층에서 adapter.in.web.response 패키지의 response DTO를 직접 참조하는 것을 허용함. 이는 CQRS 아키텍처에서 읽기 전용 작업의 효율성을 위한 팀 컨벤션임.
src/main/java/konkuk/thip/record/adapter/out/persistence/repository/RecordQueryRepositoryImpl.java (1)
Learnt from: seongjunnoh
PR: #101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.820Z
Learning: THIP 프로젝트에서 Record와 Vote는 Room에 속하지만 Feed는 Room에 속하지 않는 구조이며, 댓글 작성 시 Record/Vote에 대해서만 사용자가 해당 Room의 참가자인지 검증이 필요하다.
⏰ 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: build
🔇 Additional comments (11)
src/main/java/konkuk/thip/record/adapter/out/persistence/repository/RecordQueryRepositoryImpl.java (11)
5-7: 새로운 QueryDSL 표현식 import가 적절하게 추가되었습니다.커서 기반 페이지네이션과 복합 정렬을 위한
CaseBuilder,NumberExpression,StringExpressionimport가 올바르게 추가되었습니다.
9-16: 새로운 의존성들이 적절하게 import되었습니다.
StatusType,Cursor,SortType,PostQueryDto,QPostQueryDto등 커서 기반 페이지네이션과 통합 DTO를 위한 필수 클래스들이 올바르게 import되었습니다.
34-37: QueryDSL Q클래스 필드 선언이 적절합니다.모든 필요한 엔티티의 Q클래스가 선언되어 있고, join 연산과 조건절 구성에 필요한 구조가 잘 갖춰져 있습니다.
39-58: findMyRecords 메서드 구현이 올바릅니다.커서 기반 페이지네이션을 구현하고 있으며, 조건절 구성과 정렬이 적절히 처리되고 있습니다. QueryDSL 프로젝션을 사용한 DTO 변환도 효율적입니다.
60-75: N+1 문제 해결을 위한 조건절 구성이 효율적입니다.
BooleanBuilder를 사용하여 Record와 Vote 엔티티를 다형성으로 처리하는 방식이 적절하며, 단일 쿼리로 두 타입의 포스트를 모두 조회할 수 있도록 구현되었습니다.
77-95: findGroupRecordsOrderBySortType 메서드가 유연한 정렬을 지원합니다.페이지 범위, 총평 여부, 정렬 타입을 모두 고려한 복합 조건 조회가 가능하며, 커서 기반 페이지네이션이 올바르게 구현되었습니다.
97-125: buildRecordVoteCondition 메서드의 필터링 로직이 정확합니다.총평(
isOverview) 여부에 따른 분기 처리와 페이지 범위 필터링이 Record와 Vote 엔티티 모두에 일관성 있게 적용되었습니다. 활성 상태 필터링도 적절히 포함되어 있습니다.
127-149: 다형성 엔티티를 위한 Case 표현식들이 잘 구현되었습니다.
pageExpr(),isOverviewExpr(),postTypeExpr()메서드들이 Record와 Vote 엔티티의 차이를 적절히 처리하고 있으며, 정렬과 필터링에 필요한 통합된 표현식을 제공합니다.
151-186: 커서 조건절 구성이 각 정렬 타입별로 정확히 구현되었습니다.각
SortType에 대해 적절한 커서 비교 로직이 구현되어 있으며, 동일 값에 대한 보조 정렬 기준(postId)도 올바르게 처리되었습니다. 특히MINE타입의 복합 정렬 조건이 정확합니다.
188-195: 정렬 조건이 각 SortType에 맞게 올바르게 정의되었습니다.모든 정렬 타입에 대해 일관된 내림차순 정렬과 보조 정렬 기준이 적용되어 있으며,
MINE타입의 복합 정렬(isOverview→page→postId)이 요구사항에 맞게 구현되었습니다.
197-211: QueryDSL 프로젝션을 통한 DTO 변환이 효율적입니다.
QPostQueryDto를 사용한 프로젝션으로 엔티티에서 DTO로의 변환이 쿼리 레벨에서 처리되어 성능상 이점이 있습니다. 모든 필요한 필드가 적절히 매핑되어 있습니다.
There was a problem hiding this comment.
고생많으셨습니다!!
반환할 데이터의 타입도 다르고, 정렬 조건도 많았는데, Cursor 클래스 도입으로 인해 QueryDSL 의 코드가 훨씬 보기 편한거 같네요!
service가 너무 많은 책임을 가지고 있다는 생각이 들지만, 저희가 매핑, 유효성 검증 등을 위한 객체는 따로 생각하지않고 그냥 서비스의 private 메서드로 분리하는 방식으로 개발하고 있으니 이 부분도 괜찮은 것 같습니다!!
리뷰 요구사항에 언급하신 Cursor 객체도 현재 방식이라면 추후에 cursor 로 사용할 데이터 타입이 추가된다하더라도, 그냥 파싱용 메서드하나 추가해두면 되니 좋은 것 같습니다!!
| .toList(); | ||
| private void validateGroupRecordFilters(Integer pageStart, Integer pageEnd, Boolean isPageFilter, Boolean isOverview, int bookPageSize, double currentPercentage) { |
| private void validateMyRecordFilters(Integer pageStart, Integer pageEnd, Boolean isPageFilter, Boolean isOverview, String sort) { | ||
| // 모든 파라미터중 하나라도 null이 아닌 경우 예외 발생 |
| boolean isLocked = participant.getCurrentPage() < dto.page(); | ||
| boolean isWriter = dto.userId().equals(userId); | ||
| String content = isLocked ? createBlurredString(dto.content()) : dto.content(); |
| private String createBlurredString(String contents) { | ||
| if (contents == null || contents.isEmpty()) { | ||
| return contents; | ||
| } | ||
| if (pageNum < 1) { | ||
| throw new InvalidStateException(ErrorCode.API_INVALID_PARAM, new IllegalArgumentException("pageNum은 1 이상의 값이어야 합니다.")); | ||
|
|
||
| int originalLength = contents.length(); | ||
| int blurLen = BLURRED_STRING.length(); | ||
|
|
||
| // 필요한 전체 반복 횟수 계산 | ||
| int repeat = originalLength / blurLen; | ||
|
|
||
| StringBuilder sb = new StringBuilder(originalLength); | ||
|
|
||
| // 몫 만큼 반복 | ||
| for (int i = 0; i < repeat + 1; i++) { | ||
| sb.append(BLURRED_STRING); | ||
| } |
| // VoteItem 한번에 조회 (투표 게시물에 대한 투표 항목 조회) | ||
| Map<Long, List<VoteItemQueryDto>> voteItemQueryMap = voteQueryPort.findVoteItemsByVoteIds(cursorBasedList.contents().stream() | ||
| .filter(postQueryDto -> postQueryDto.postType().equals("VOTE")) | ||
| .map(PostQueryDto::postId) | ||
| .collect(Collectors.toSet()), userId); | ||
|
|
||
| // 사용자가 좋아요를 누른 게시물 ID 목록 조회 | ||
| Set<Long> likedPostIds = postLikeQueryPort.findPostIdsLikedByUser(cursorBasedList.contents().stream() | ||
| .map(PostQueryDto::postId) | ||
| .collect(Collectors.toSet()), userId); |
There was a problem hiding this comment.
LGTM
[일단 보여줘야하는 기록 + 투표 조회한 후에, 투표일 경우 투표 아이템 조회 & 사용자가 좋아한 기록 조회] 방식 좋은 것 같네요!!
| private BooleanBuilder buildMyRecordCondition(Long roomId, Long userId) { | ||
| BooleanBuilder where = new BooleanBuilder(); | ||
|
|
||
| return new PageImpl<>(resultList, pageable, total); | ||
| } | ||
| BooleanBuilder voteCondition = new BooleanBuilder(); | ||
| voteCondition.and(post.instanceOf(VoteJpaEntity.class)) | ||
| .and(vote.roomJpaEntity.roomId.eq(roomId)); | ||
|
|
||
| BooleanBuilder recordCondition = new BooleanBuilder(); | ||
| recordCondition.and(post.instanceOf(RecordJpaEntity.class)) | ||
| .and(record.roomJpaEntity.roomId.eq(roomId)); |
There was a problem hiding this comment.
LGTM 내 기록 조회 중, record 조건과 vote 조건식을 분리한 후 or 로 묶으니 가독성이 좋네요!
| private BooleanBuilder buildCursorPredicateForSortType(SortType sortType, Cursor cursor) { | ||
| BooleanBuilder builder = new BooleanBuilder(); | ||
|
|
||
| switch (sortType) { | ||
| case CREATED_AT -> { | ||
| LocalDateTime createdAt = cursor.getLocalDateTime(0); | ||
| Long postId = cursor.getLong(1); | ||
| builder.and(post.createdAt.lt(createdAt) | ||
| .or(post.createdAt.eq(createdAt).and(post.postId.lt(postId)))); | ||
| } | ||
| case LIKE_COUNT -> { | ||
| Integer likeCount = cursor.getInteger(0); | ||
| Long postId = cursor.getLong(1); | ||
| builder.and(post.likeCount.lt(likeCount) | ||
| .or(post.likeCount.eq(likeCount).and(post.postId.lt(postId)))); | ||
| } | ||
| case COMMENT_COUNT -> { | ||
| Integer commentCount = cursor.getInteger(0); | ||
| Long postId = cursor.getLong(1); | ||
| builder.and(post.commentCount.lt(commentCount) | ||
| .or(post.commentCount.eq(commentCount).and(post.postId.lt(postId)))); | ||
| } | ||
| case MINE -> { | ||
| Integer isOverview = cursor.getInteger(0); | ||
| Integer page = cursor.getInteger(1); | ||
| Long postId = cursor.getLong(2); | ||
| builder.and( | ||
| isOverviewExpr().lt(isOverview) | ||
| .or(isOverviewExpr().eq(isOverview).and(pageExpr().lt(page))) | ||
| .or(isOverviewExpr().eq(isOverview).and(pageExpr().eq(page)).and(post.postId.lt(postId))) | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| return builder; | ||
| } |
| private QPostQueryDto selectPostQueryDto() { | ||
| return new QPostQueryDto( | ||
| post.postId, | ||
| postTypeExpr(), //추후에 상속 구조 해지시 type 필드로 구분 |
| @DisplayName("[단위] VoteItem 단위 테스트") | ||
| class VoteItemTest { |
| @DisplayName("[통합] RecordSearchApiTest 테스트") | ||
| @AutoConfigureMockMvc(addFilters = false) | ||
| @Transactional | ||
| class RecordSearchApiTest { |
There was a problem hiding this comment.
조회 api 통합 테스트는 given 으로 준비한 테스트용 데이터가 내부적으로 필터링 + 정렬 조건 을 잘 거친 후에, 제대로 된 데이터가 응답으로 나오는지(= response 의 내용 검증을 통해) 나름 정밀하게 테스트하는게 좋지 않나 라는 생각이 있었는데,
지금 통합 테스트처럼 데이터 존재 여부, 특정 조건의 데이터 값 검증 정도만 해도 괜찮은 것 같네요!
There was a problem hiding this comment.
음 사실 저도 그 부분에서 고민을 조금 했는데 내부 반환 값 자체는 서비스 요구사항이 바뀔 때마다 얼마든지 변동될 가능성이 있기 때문에 큰 틀로 정렬, 페이징, 필터링 처리가 잘 이루어지는지에 초점을 맞춰서 테스트하려고 했습니다!
hd0rable
left a comment
hd0rable
left a comment
There was a problem hiding this comment.
너무너무 수고하셨습니다.. 🥹🥹 너무너무 든든하네여 현준띠니 ㅎ
|
|
||
| private final RecordSearchUseCase recordSearchUseCase; | ||
|
|
||
| /** |
| import lombok.Getter; | ||
|
|
||
| @Getter | ||
| public enum PostType { |
There was a problem hiding this comment.
엇 이거 게시물 타입을 나타내는 PostType과 이름이 같아서 헷갈릴 것같은데 RoomPostType 이라던가 방 기록시에 게시물 타입을 나타내는 이름으로 명시하면 어떨까요??
There was a problem hiding this comment.
엇 저는 추후에 상속구조를 해지할 때 싱글 테이블에서 각 Post의 종류를 구분하기 위한 enum으로 정의해두긴 했습니다! RoomPostType으로 우선 변경해둘까요? 아니면, 네이밍을 유지하고 FEED를 추가할까요?
There was a problem hiding this comment.
아아 그럼 제가 사용하고있는 PostType그대로 써도될것같습니다!! 어차피 같은역할아닌가욧?
There was a problem hiding this comment.
넵 pull 받아서 사용하겠습니다!!
| private QPostQueryDto selectPostQueryDto() { | ||
| return new QPostQueryDto( | ||
| post.postId, | ||
| postTypeExpr(), //추후에 상속 구조 해지시 type 필드로 구분 |
| @JoinColumn(name = "room_id") | ||
| private RoomJpaEntity roomJpaEntity; | ||
|
|
||
| @VisibleForTesting |
| /** | ||
| * 투표 항목의 비율을 계산 | ||
| * @param counts 각 투표 항목의 카운트 리스트 | ||
| * @return 각 항목의 백분율을 나타내는 리스트 | ||
| */ | ||
| public static List<Integer> calculatePercentages(List<Integer> counts) { | ||
| int total = counts.stream().mapToInt(Integer::intValue).sum(); | ||
| int n = counts.size(); | ||
| List<Integer> result = new ArrayList<>(Collections.nCopies(n, 0)); | ||
|
|
||
| if (total == 0 || n == 0) { | ||
| return result; | ||
| } | ||
|
|
||
| double[] fractional = new double[n]; | ||
| int sum = 0; | ||
|
|
||
| for (int i = 0; i < n; i++) { | ||
| double exact = counts.get(i) * 100.0 / total; | ||
| int base = (int) exact; // 정수 부분 | ||
| result.set(i, base); | ||
| fractional[i] = exact - base; // 소수 부분 | ||
| sum += base; | ||
| } | ||
|
|
||
| int remaining = 100 - sum; | ||
|
|
||
| // fractional과 index를 묶어서 정렬 | ||
| List<int[]> order = new ArrayList<>(); | ||
| for (int i = 0; i < n; i++) { | ||
| order.add(new int[]{i, (int) (fractional[i] * 1_000_000)}); // 정밀도 위해 정수화 | ||
| } | ||
|
|
||
| // fractional 값이 큰 순서대로 내림차순 정렬 | ||
| order.sort((a, b) -> Integer.compare(b[1], a[1])); | ||
|
|
||
| // 남은 %를 fractional이 큰 순서대로 분배 | ||
| for (int i = 0; i < remaining; i++) { | ||
| int index = order.get(i % n)[0]; | ||
| result.set(index, result.get(index) + 1); | ||
| } | ||
|
|
||
| return result; |
There was a problem hiding this comment.
역시 문귀, 문악, 문종, 문신 장다운 코드같습니다 🤴🏻
| .findFirst() | ||
| .orElseThrow( | ||
| () -> new InvalidStateException(ErrorCode.API_INVALID_PARAM, new IllegalArgumentException("현재 타입 조건 param : " + value)) | ||
| () -> new InvalidStateException(ErrorCode.API_INVALID_PARAM, new IllegalArgumentException("type은 group 또는 mine이어야 합니다. 현재 타입 조건 param : " + value)) |
| private final RoomParticipantCommandPort roomParticipantCommandPort; | ||
|
|
||
| private static final int DEFAULT_PAGE_SIZE = 10; | ||
| private static final String BLURRED_STRING = "여긴 못 지나가지롱~~"; |
3 participants
#️⃣ 연관된 이슈
📝 작업 내용
기록장 조회 api를 모두 수정했습니다.
반영사항은 다음과 같습니다.
📸 스크린샷
💬 리뷰 요구사항
Cursor 객체는 CursorBasedList와 함께 커서 페이징 처리시에 자주 사용 될 것 같으니 개선 사항이 필요해보이면 언제든지 리뷰 부탁드릴게요~
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
신규 기능
개선 및 변경
버그 수정
테스트
기타