[refactor] Content 테이블 삭제 후 Feed에 종속#260
Conversation
WalkthroughContent 테이블/엔티티/레포지토리를 제거하고 Feed에 문자열 콘텐츠 리스트를 TEXT(JSON) 칼럼으로 내재화했습니다. ContentList 값객체와 JPA AttributeConverter를 도입하고, 매퍼·쿼리·서비스·도메인·테스트 전반을 ContentList 기반으로 마이그레이션했으며 이미지 개수 초과 에러 코드를 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Controller
participant Service
participant Repo as FeedCommandPersistenceAdapter
participant DB as Database
Client->>Controller: Create/Update Feed (image URLs)
Controller->>Service: DTO -> Domain(Feed with ContentList)
Service->>Service: ContentList.validateImageCount()
Service->>Repo: save/update(Feed)
Repo->>DB: persist Feed (content_list TEXT via Converter)
DB-->>Repo: persisted
Repo-->>Service: saved entity
Service-->>Controller: Response
Controller-->>Client: Result
sequenceDiagram
autonumber
participant Client
participant Controller
participant QueryRepo as FeedQueryRepositoryImpl
participant DB as Database
Client->>Controller: Fetch Feeds
Controller->>QueryRepo: query feeds
QueryRepo->>DB: select feeds (content_list)
DB-->>QueryRepo: rows with content_list
QueryRepo->>QueryRepo: ContentList JSON -> ContentList 객체
QueryRepo-->>Controller: DTO(contentUrls[])
Controller-->>Client: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
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
🪧 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
CodeRabbit Configuration File (
|
Test Results415 tests 415 ✅ 31s ⏱️ Results for commit 45f78a9. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (18)
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1)
163-167: 메시지 용어 통일 제안: '컨텐츠' → '콘텐츠'프로젝트 전반의 메시지에서 '콘텐츠' 표기가 다수이며(예: 테스트 메시지), 본 항목만 '컨텐츠'로 표기되어 있습니다. 사용자 메시지 일관성을 위해 용어 통일을 권장합니다.
적용 diff:
- CONTENT_LIST_SIZE_OVERFLOW(HttpStatus.BAD_REQUEST, 165000, "컨텐츠 리스트의 크기가 초과되었습니다. 최대 3개까지 가능합니다."), + CONTENT_LIST_SIZE_OVERFLOW(HttpStatus.BAD_REQUEST, 165000, "콘텐츠 리스트의 크기가 초과되었습니다. 최대 3개까지 가능합니다."),src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java (1)
59-64: S3 업로드 중 예외 시 부분 업로드 누수 가능성 점검 권장현재 imageUrls 변수는 uploadImages가 성공적으로 반환된 후에만 채워지므로, 업로드 과정에서 예외가 발생하면 이미 업로드된 일부 객체를 추적/정리할 수 없습니다. S3 포트에 원자적 업로드 또는 부분 성공 시 반환/롤백 기능을 부여해 누수 가능성을 줄이는 것을 권장합니다.
가능한 대안:
- S3 포트에 트랜잭션성 제공(업로드 실패 시 내부에서 이미 업로드된 객체를 롤백).
- 업로드를 개별 단위로 수행하며 성공한 키를 누적, 예외 시 누적 키를 기반으로 삭제 시도.
예시(개념 스케치):
List<String> imageUrls = List.of(); try { imageUrls = s3CommandPort.uploadImages(images); // 실패 시 내부적으로 롤백 또는 부분 성공 리스트 제공 // ... } catch (Exception e) { if (!imageUrls.isEmpty()) { s3CommandPort.deleteImages(imageUrls); } throw e; }src/main/java/konkuk/thip/feed/adapter/out/mapper/FeedMapper.java (1)
28-29: null-safe 매핑은 좋습니다. 단, 도메인/엔티티 간 ContentList 인스턴스 공유를 피하기 위한 방어적 복사 고려현재 도메인 Feed의 ContentList 인스턴스를 JPA 엔티티에 그대로 주입하면, 동일 객체 참조 공유로 인한 후속 변경 사이드이펙트 가능성이 있습니다. 매핑 시 불변 스냅샷을 기반으로 새 ContentList를 생성해 주입하는 방식이 안전합니다.
아래처럼 방어적 복사를 적용해 주세요.
- .contentList(feed.getContentList() != null ? feed.getContentList() : ContentList.empty()) + .contentList(feed.getContentList() != null + ? ContentList.of(feed.getContentList().toUnmodifiableList()) + : ContentList.empty())- .contentList(feedJpaEntity.getContentList() != null ? feedJpaEntity.getContentList() : ContentList.empty()) + .contentList(feedJpaEntity.getContentList() != null + ? ContentList.of(feedJpaEntity.getContentList().toUnmodifiableList()) + : ContentList.empty())Also applies to: 46-47
src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateApiTest.java (2)
149-150: 인덱스 기반 단일 요소 비교 대신 전체 일치 검증으로 의도를 명확히현재 검증은 첫 요소만 확인합니다. 리스트 전체가 기대와 정확히 일치함을 보장하려면 containsExactly 사용이 더 명확합니다.
- assertThat(updatedFeed.getContentList().get(0)).isEqualTo("https://s3-mock/image-2.jpg"); + assertThat(updatedFeed.getContentList()).containsExactly("https://s3-mock/image-2.jpg");
196-197: 이미지 검증도 전체 일치 검증으로 통일위 테스트와 동일하게, 전체 일치 검증이 의도를 더 잘 드러냅니다.
- assertThat(updated.getContentList().get(0)).isEqualTo("https://s3-mock/image-2.jpg"); + assertThat(updated.getContentList()).containsExactly("https://s3-mock/image-2.jpg");src/main/java/konkuk/thip/feed/application/service/FeedUpdateService.java (2)
26-29: remainImageUrls 길이 사전 검증은 유용하지만, 중복 검증 최소화 고려ContentList가 이미 크기 제약을 강제하므로, 서비스 레벨의 동일 검증은 중복일 수 있습니다. 빠른 실패가 목적이면 유지, 단일 책임 원칙을 선호하면 Feed.updateImages 내부(또는 ContentList 생성 시)에만 위임하는 것도 고려해볼 만합니다.
58-64: 이미지 삭제 대상 계산은 업데이트 전 스냅샷 기준으로 수행 필요 (현재 구현은 오동작 소지 + 미사용 메서드)
- 현재 로직은 feed.getContentList() 기준으로 필터링하는데, 업데이트 이후 호출되면 항상 빈 목록이 되어 S3 삭제가 누락될 수 있습니다.
- 또한 이 메서드는 현재 호출되지 않습니다. 혼란 방지를 위해 호출부를 추가하거나 메서드를 제거하는 것이 좋습니다.
아래처럼 업데이트 전 스냅샷을 취해 삭제 대상을 계산하도록 변경을 권고합니다.
@@ private void applyPartialFeedUpdate(Feed feed, FeedUpdateCommand command) { if (command.remainImageUrls() != null) { - feed.updateImages(command.userId(), command.remainImageUrls()); + // 업데이트 이전 이미지 스냅샷 + List<String> before = feed.getContentList().toUnmodifiableList(); + // 도메인 상태 변경 + feed.updateImages(command.userId(), command.remainImageUrls()); + // S3 정리(임시). TODO 주석 참고: 이벤트 기반/배치로 이관 시 제거 + handleFeedImageDelete(before, command.remainImageUrls()); } @@ - private void handleFeedImageDelete(Feed feed, List<String> remainImageUrls) { - List<String> toDelete = feed.getContentList().stream() - .filter(url -> !remainImageUrls.contains(url)) - .toList(); + private void handleFeedImageDelete(List<String> beforeImageUrls, List<String> remainImageUrls) { + if (remainImageUrls == null) { + return; + } + List<String> toDelete = beforeImageUrls.stream() + .filter(url -> !remainImageUrls.contains(url)) + .toList(); if (!toDelete.isEmpty()) { s3CommandPort.deleteImages(toDelete); } }만약 현재 PR 단계에서 S3 삭제를 의도적으로 보류하는 것이라면, 메서드 자체를 제거하거나
@SuppressWarnings("unused")및 Javadoc으로 의도를 명시해 혼란을 줄일 것을 제안드립니다.src/main/java/konkuk/thip/feed/adapter/out/jpa/converter/ContentListJsonConverter.java (2)
19-25: null을 DB에 '[]'로 저장하는 정책, 의도 확인 필요현재 null ContentList는 DB에 빈 배열("[]")로 직렬화됩니다. 도메인에서 "없음(null)"과 "빈 목록([])"을 동일하게 취급한다면 괜찮지만, 구분이 필요하다면 null을 그대로 저장하는 편이 낫습니다.
@Override public String convertToDatabaseColumn(ContentList attribute) { try { - List<String> list = attribute == null ? List.of() : attribute.toUnmodifiableList(); - return objectMapper.writeValueAsString(list); + if (attribute == null) { + return null; + } + return objectMapper.writeValueAsString(attribute.toUnmodifiableList()); } catch (JsonProcessingException e) { throw new IllegalStateException("ContentList 직렬화 실패", e); } }
33-37: 역직렬화 타입 지정 간결화 및 가독성 향상 제안TypeFactory 대신 TypeReference를 사용하면 가독성이 좋아집니다.
- List<String> list = objectMapper.readValue( - dbData, - objectMapper.getTypeFactory().constructCollectionType(List.class, String.class) - ); + List<String> list = objectMapper.readValue( + dbData, + new com.fasterxml.jackson.core.type.TypeReference<List<String>>() {} + );또는 import를 추가해 더 간결하게 사용할 수 있습니다:
// 파일 상단 import 추가 import com.fasterxml.jackson.core.type.TypeReference;src/test/java/konkuk/thip/feed/domain/FeedTest.java (1)
39-41: ContentList 적용 LGTM + 이미지 개수 초과 케이스 테스트 보강 제안ContentList.of(...) 사용은 적절합니다. 다만, ContentList가 최대 3개 제한을 강제하므로(값 객체 내부 검증), 4개 이상 입력 시 CONTENT_LIST_SIZE_OVERFLOW가 발생하는 도메인 규칙 테스트를 추가해 주세요.
예시 테스트 코드(참고):
@Test @DisplayName("ContentList: 이미지가 4개 이상이면 InvalidStateException(CONTENT_LIST_SIZE_OVERFLOW)이 발생한다.") void contentList_size_overflow_throws() { assertThrows(InvalidStateException.class, () -> Feed.builder() .id(1L) .creatorId(1L) .content("내용") .isPublic(true) .tagList(List.of()) .contentList(ContentList.of(List.of("u1","u2","u3","u4"))) .build() ); }src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedCommandPersistenceAdapter.java (1)
66-68: 코드 정리 및 null-안전성 강화 제안
FeedCommandPersistenceAdapter의 레거시 Content 처리용 주석(66–67, 80–81줄)을 삭제해 가독성을 높여주세요.FeedJpaEntity.updateFrom(Feed)에서contentList를 올바르게 반영하고 있으므로(컨버터 적용 포함), 별도 조치 불필요합니다.applyFeedTags(feed, …)호출 전에feed.getTagList()가 null일 수 있으니, 아래 중 하나를 선택해 NPE를 방지해주세요.
- 도메인
Feed생성 시tagList를 항상 빈 리스트로 초기화(@Builder.Default 등)applyFeedTags진입부에 null 검사 추가:List<Tag> tags = feed.getTagList(); if (tags == null || tags.isEmpty()) return; …- 변경 위치
- src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedCommandPersistenceAdapter.java: 66–67, 80–81줄 주석 삭제
- src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedCommandPersistenceAdapter.java: applyFeedTags 호출 직전에 null/empty 검증 추가
src/main/java/konkuk/thip/feed/application/port/out/dto/FeedQueryDto.java (2)
41-41: 주석 갱신 필요: GROUP_CONCAT 언급은 현재 구조와 불일치이제 ContentList(JSON 컬럼 → 컨버터)를 그대로 투영하므로, “GROUP_CONCAT 결과” 주석은 오해의 소지가 있습니다. JSON/컨버터 기반 매핑으로 주석을 업데이트해 주세요.
적용 예:
- ContentList contentList, // GROUP_CONCAT 결과 + ContentList contentList, // JSON 컬럼(ContentList) 컨버터 결과
59-66: isPublic null-세이프 처리 권장레코드 필드는 primitive boolean이므로, 생성자 인자의 Boolean isPublic가 null이면 NPE가 발생합니다. 기본값 false로 안전하게 변환하세요.
적용 diff:
- isPublic, + Boolean.TRUE.equals(isPublic),src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSingleApiTest.java (1)
91-97: isSaved 양의 케이스 커버 OK + 음의 케이스 보강 제안me가 f1을 저장했음을 명시적으로 구성해 isSaved=true를 검증한 점 좋습니다. 보완으로 isSaved=false 케이스(저장하지 않은 사용자 또는 저장하지 않은 피드)에 대한 단일 테스트를 추가하면 회귀 방지에 도움이 됩니다.
원하시면 음의 케이스 테스트 메서드 뼈대를 제공하겠습니다.
src/main/java/konkuk/thip/feed/domain/Feed.java (2)
153-159: updateImages가 ‘추가’를 허용하지 않는 도메인 규칙 확인 필요현재 구현은 새 이미지 목록을 설정하기 전에 validateOwnsImages(newImageUrls)를 호출하여 “새 목록이 기존 목록의 부분집합”임을 강제합니다. 이는 이미지 ‘추가’를 불허하고 ‘제거/재정렬’만 허용합니다. 요구사항과 일치하는지 확인 부탁드립니다.
- 만약 ‘추가’도 허용해야 한다면:
- 추가는 Content 업로드/검증 후에 feed.contentList에 병합하는 별도 유스케이스로 분리하거나,
- updateImages에서는 max size 검증만 하고 소유 검증은 “제거/교체” 시나리오에 한정하는 방식을 고려해 볼 수 있습니다.
원하시면 두 가지 유스케이스(추가/정렬·제거)로 메서드를 분리한 예시를 제안드리겠습니다.
161-170: 소유 검증 메시지/필터링 합리적이나 빈 문자열 허용 여부 점검 권장
- null/blank는 myImageUrls에서 제외되어 후보에 포함되면 예외가 발생합니다. 의도된 정책인지 확인 바랍니다.
- 에러 메시지에 문제 URL을 포함시키는 것은 디버깅에 유용합니다.
필요 시 빈 문자열/공백 URL은 사전에 필터링하거나, 메시지에 따옴표로 감싸 가독성을 높일 수 있습니다.
src/main/java/konkuk/thip/feed/domain/value/ContentList.java (2)
11-21: 값객체 설계 자체는 적절, 다만 maxSize 동등성 포함 여부는 의도 확인현재
equals/hashCode는contents만 비교하고 있어,maxSize가 달라도 동등하다고 판단됩니다. 의도된 설계라면 OK이며, 아니라면maxSize도 비교에 포함해야 합니다.원하시면 동등성에
maxSize를 포함하는 패치도 제공하겠습니다.
40-44: 공용 정적 검증기의 정책 고정은 OK, 단 JPA 더티체킹 주의정적 검증기에서 기본 정책(3개)을 노출하는 것은 유틸리티 용도로 합리적입니다. 다만 이 클래스는 가변 컬렉션을 감싸며 JPA AttributeConverter로 저장됩니다. JPA 더티체킹이 인스턴스 내부 변경을 감지하는지(스냅샷 비교 시
equals가 호출되는지) 반드시 확인해 주세요. 필요 시, 엔티티에서는 교체 할당(새 인스턴스 생성) 패턴을 권장합니다.간단한 통합 테스트 권장 시나리오:
- 엔티티 로드 →
getContentList().add("x")→ flush- SQL UPDATE가 발생하는지 확인
테스트 템플릿 제공 가능합니다. 원하시면 추가해 드릴게요.
📜 Review details
Configuration used: CodeRabbit UI
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 (27)
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/jpa/ContentJpaEntity.java(0 hunks)src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java(4 hunks)src/main/java/konkuk/thip/feed/adapter/out/jpa/converter/ContentListJsonConverter.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/mapper/ContentMapper.java(0 hunks)src/main/java/konkuk/thip/feed/adapter/out/mapper/FeedMapper.java(3 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedCommandPersistenceAdapter.java(3 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/Content/ContentJpaRepository.java(0 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java(4 hunks)src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java(2 hunks)src/main/java/konkuk/thip/feed/application/port/out/dto/FeedQueryDto.java(4 hunks)src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java(2 hunks)src/main/java/konkuk/thip/feed/application/service/FeedUpdateService.java(3 hunks)src/main/java/konkuk/thip/feed/domain/Feed.java(6 hunks)src/main/java/konkuk/thip/feed/domain/value/ContentList.java(1 hunks)src/test/java/konkuk/thip/common/util/TestEntityFactory.java(4 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/BasicFeedShowAllApiTest.java(0 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateApiTest.java(0 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateControllerTest.java(2 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedDeleteApiTest.java(0 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowMineApiTest.java(0 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSingleApiTest.java(2 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSpecificUserApiTest.java(0 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateApiTest.java(3 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateControllerTest.java(1 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FollowingPriorityFeedShowAllApiTest.java(0 hunks)src/test/java/konkuk/thip/feed/domain/FeedTest.java(3 hunks)
💤 Files with no reviewable changes (9)
- src/test/java/konkuk/thip/feed/adapter/in/web/FeedDeleteApiTest.java
- src/main/java/konkuk/thip/feed/adapter/out/mapper/ContentMapper.java
- src/test/java/konkuk/thip/feed/adapter/in/web/BasicFeedShowAllApiTest.java
- src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateApiTest.java
- src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSpecificUserApiTest.java
- src/test/java/konkuk/thip/feed/adapter/in/web/FollowingPriorityFeedShowAllApiTest.java
- src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowMineApiTest.java
- src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/Content/ContentJpaRepository.java
- src/main/java/konkuk/thip/feed/adapter/out/jpa/ContentJpaEntity.java
🧰 Additional context used
🧬 Code Graph Analysis (9)
src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java (1)
src/main/java/konkuk/thip/feed/domain/value/ContentList.java (1)
ContentList(11-133)
src/main/java/konkuk/thip/feed/adapter/out/jpa/converter/ContentListJsonConverter.java (1)
src/main/java/konkuk/thip/feed/domain/value/ContentList.java (1)
ContentList(11-133)
src/main/java/konkuk/thip/feed/adapter/out/mapper/FeedMapper.java (1)
src/main/java/konkuk/thip/feed/domain/value/ContentList.java (1)
ContentList(11-133)
src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java (1)
src/main/java/konkuk/thip/feed/domain/value/ContentList.java (1)
ContentList(11-133)
src/main/java/konkuk/thip/feed/application/port/out/dto/FeedQueryDto.java (1)
src/main/java/konkuk/thip/feed/domain/value/ContentList.java (1)
ContentList(11-133)
src/main/java/konkuk/thip/feed/domain/Feed.java (1)
src/main/java/konkuk/thip/feed/domain/value/ContentList.java (1)
ContentList(11-133)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
src/main/java/konkuk/thip/feed/domain/value/ContentList.java (1)
ContentList(11-133)
src/main/java/konkuk/thip/feed/application/service/FeedUpdateService.java (1)
src/main/java/konkuk/thip/feed/domain/value/ContentList.java (1)
ContentList(11-133)
src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java (1)
src/main/java/konkuk/thip/feed/domain/value/ContentList.java (1)
ContentList(11-133)
⏰ 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 (27)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateControllerTest.java (2)
20-20: 와일드카드 static import 사용: OK테스트 내 ErrorCode 참조 가독성이 개선되고, 충돌 여지가 낮아 보입니다. 현재 컨텍스트에서는 적절합니다.
156-159: 새 에러코드(CONTENT_LIST_SIZE_OVERFLOW) 기반 검증으로 테스트 의도 명확해짐하드코딩된 메시지 대신 enum의 코드/메시지를 사용하여 테스트 안정성이 좋아졌습니다.
src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java (1)
36-37: 업로드 전 이미지 개수 도메인 검증으로 fail-fast 달성: 굿ContentList.validateImageCount 호출로 도메인 규칙과 에러코드가 일관되게 적용됩니다.
src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateControllerTest.java (1)
183-184: 이미지 개수 초과 검증의 에러코드/메시지 정합성 확보: OK도메인 규칙과 에러코드를 반영한 단언으로 변경되어 일관성이 좋아졌습니다.
src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java (1)
108-111: ContentList → String[] 매핑 단순화 및 null-safe 처리: 좋습니다toArray(String[]::new) 사용으로 불필요한 스트림/복사를 피하면서 깔끔하게 변환되었습니다. null 시 빈 배열 반환도 적절합니다.
src/main/java/konkuk/thip/feed/adapter/out/mapper/FeedMapper.java (1)
8-8: ContentList 도입에 따른 매핑 의존성 정리, 적절합니다도메인 값 객체(ContentList)를 직접 매핑하도록 정리된 점 좋습니다. null 회피 전략과 일관성 있는 타입 사용에 도움이 됩니다.
src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateApiTest.java (1)
40-40: MockMvc patch 정적 임포트 사용 적절합니다가독성 측면에서 요청 빌더가 간결해집니다. 유지해도 무방합니다.
src/main/java/konkuk/thip/feed/application/service/FeedUpdateService.java (1)
8-8: ContentList로 검증 책임을 값 객체로 모은 점 좋습니다도메인 불변 조건을 값 객체로 축약해 응집도를 높였습니다.
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
311-312: 테스트 엔티티에서 ContentList.empty()/of(...) 사용으로 일관성 및 null-안전성 확보, 좋습니다
- 빈 콘텐츠는 empty(), URL 리스트는 of(...)로 감싸 도메인 제약을 공유합니다.
- ContentList.of는 내부적으로 null을 빈 리스트로 처리하므로, 테스트 데이터 생성 시 NPE 가능성도 낮아집니다.
Also applies to: 333-334, 357-358
src/test/java/konkuk/thip/feed/domain/FeedTest.java (2)
5-5: 도메인 값 객체(ContentList)로의 테스트 마이그레이션 적절Feed 도메인 공개 API 변경을 테스트에 잘 반영했습니다. ContentList 사용으로 도메인 규칙을 테스트 레벨에서 명확히 표현할 수 있어 좋습니다.
259-260: 빈 첨부 리스트 기본값 명시 적절makeFeedWithPublicStatus에서 ContentList.empty()를 명시해 초기 상태를 분명히 한 점 좋습니다. 테스트 가독성과 의도 표현에 도움이 됩니다.
src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedCommandPersistenceAdapter.java (1)
6-9: 와일드카드 제거로 의존성 명시성 향상구체 import로 변경하여 의존성 경계를 명확히 한 점 좋습니다.
src/main/java/konkuk/thip/feed/application/port/out/dto/FeedQueryDto.java (1)
68-73: ContentList → String[] 변환 구현 깔끔함null/빈 컬렉션 처리 후 toArray(String[]::new) 사용은 명확하고 효율적입니다.
src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSingleApiTest.java (3)
7-8: SavedFeed 의존 추가 및 import 정리 적절
- SavedFeedJpaEntity/Repository 도입으로 isSaved 검증이 자연스럽습니다.
- Hamcrest 와일드카드 제거도 가독성에 도움 됩니다.
Also applies to: 11-11
56-57: Repository 주입 추가 적절isSaved 시나리오 구성을 위해 SavedFeedJpaRepository 주입이 필요하며 적절히 반영되어 있습니다.
62-62: 정리 순서에 SavedFeed 포함 OK테스트 간 간섭 방지를 위해 저장 피드 매핑을 teardown에 포함한 점 좋습니다.
src/main/java/konkuk/thip/feed/domain/Feed.java (3)
46-46: contentList 기본값을 값 객체로 초기화한 점 적절ContentList.empty()로 초기화하여 불변 규칙을 한 곳에서 보장하도록 한 선택이 바람직합니다.
61-79: 이미지 개수 검증 이관 자연스러움Feed 레벨의 이미지 개수 검증 주석 처리 후 ContentList 생성으로 위임하는 구조가 일관되고 중복 검증을 제거합니다.
81-84: null → empty 매핑 명확convertToContentList에서 null 입력을 empty로 처리해 NPE를 방지하는 방식이 안정적입니다.
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (6)
4-7: 명시적 QueryDSL 타입 임포트로 가독성 향상와일드카드 대신 구체 타입을 임포트하여 의존 타입이 명확해졌습니다. 유지보수에 유리합니다.
12-15: Q타입/엔티티 임포트 정리 적절컨텍스트에 필요한 Q타입만 명시적으로 임포트했습니다. 쿼리 가독성에 도움 됩니다.
269-279: ContentList → String[] 변환 방식 적절
f.getContentList().toArray(String[]::new)사용은 타입 안정적이고 성능/가독성 면에서 적절합니다.
435-435: SavedFeed용 프로젝션도 동일한 타입 확인 필요위와 동일하게
toSavedFeedQueryDto()에서도feed.contentList를 넘깁니다. 두 경로 간 타입 불일치가 없도록 일관성을 확인해 주세요.동일 스크립트로 함께 검증 가능합니다.
265-285: Alias 매핑은 항상 존재하므로 null 체크 불필요합니다
JPA 매핑(@JoinColumn(nullable = false))과 DB 제약조건, 그리고 THIP 프로젝트의 1:1 User–Alias 관계(모든 사용자는 반드시 alias 보유)로 인해f.getUserJpaEntity().getAliasForUserJpaEntity()는 null이 될 수 없습니다. 따라서 제안하신 null 세이프티 코드는 실제로 불필요합니다.Likely an incorrect or invalid review comment.
361-361: 확인: QFeedQueryDto 생성자 ContentList 파라미터 정합성 확인됨FeedQueryDto.java의 @QueryProjection 생성자 시그니처를 검토한 결과,
String[] contentUrls대신ContentList contentList를 파라미터로 받고 있으며 내부에서convertToArray(contentList)로 변환하고 있어,feed.contentList를 그대로 전달해도 컴파일 및 런타임 매핑 문제가 없습니다. 추가 조치는 불필요합니다.src/main/java/konkuk/thip/feed/domain/value/ContentList.java (2)
80-99: remove/clear에 대한 검증 생략은 합리적삭제/비우기에서는 상한 검증이 불필요하므로 현재 구현(검증 미적용)은 타당합니다.
129-133: 불변 스냅샷 제공 API 유용
toUnmodifiableList()로 외부 노출 시 불변 스냅샷을 제공하는 접근이 좋습니다. 위에서 제안한 방어적 복사에도 활용 가능합니다.
| // JSON 문자열로 저장되는 단일 컬럼 | ||
| @Convert(converter = ContentListJsonConverter.class) | ||
| @Column(name = "content_list", columnDefinition = "TEXT", nullable = false) | ||
| private ContentList contentList = ContentList.empty(); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
ContentList JSON 컬럼: null 처리와 마이그레이션 확인 필요
- 컬럼을
nullable = false로 선언하셨는데, 기존 데이터에NULL이 남아 있다면 로딩 시contentList가null로 들어와 NPE가 발생할 수 있습니다. AttributeConverter에서 DBNULL/빈 문자열('')에 대해ContentList.empty()를 반환하도록 보장되는지 확인 부탁드립니다. 또한 마이그레이션 시NULL을 전부'[]'로 업데이트하는 데이터 마이그레이션이 필요합니다. columnDefinition = "TEXT"는 MySQL/H2/PostgreSQL에서 일반적으로 호환되지만, Dialect에 따라 길이/인덱싱 전략이 달라질 수 있으므로 대용량(이미지 URL 다수) 환경에서의 쿼리 비용도 고려 바랍니다.
다음 스크립트로 컨버터의 null 처리와 마이그레이션 스크립트 유무를 확인할 수 있습니다.
🏁 Script executed:
#!/bin/bash
# ContentListJsonConverter의 null 처리 확인
fd -t f 'ContentListJsonConverter.java' | xargs -I{} sh -c 'echo "=== {} ==="; sed -n "1,200p" "{}"'
# Flyway/Liquibase 마이그레이션 내 content_list 초기화 여부 확인
rg -n -C2 'content_list|feeds' --type sqlLength of output: 1875
🏁 Script executed:
#!/bin/bash
echo "마이그레이션 스크립트(.sql, .xml, .yml, .yaml)에서 content_list 및 feeds 참조 검색"
rg -nC2 --regexp='content_list' --regexp='feeds' --glob '*.sql' --glob '*.xml' --glob '*.yml' --glob '*.yaml'Length of output: 397
ContentList JSON 컬럼: Converter 안전 처리 확인 완료 및 기존 데이터 마이그레이션 추가 필요
-
ContentListJsonConverter.convertToEntityAttribute에서
dbData == null || dbData.isBlank()인 경우ContentList.empty()를 반환하므로 NPE 위험이 없습니다.
(확인 위치:
src/main/java/konkuk/thip/feed/adapter/out/jpa/converter/ContentListJsonConverter.java) -
하지만 기존 테이블에 NULL 또는 빈 문자열(
'')로 저장된 레코드가 있다면, 애플리케이션 업그레이드 후 최초 조회 시 빈 리스트가 아닌 의도치 않은 값이 들어올 수 있으므로 반드시 데이터 마이그레이션을 적용해야 합니다.
예시 SQL:UPDATE feed SET content_list = '[]' WHERE content_list IS NULL OR TRIM(content_list) = '';
– migration 스크립트( Flyway·Liquibase·직접 SQL 등 )에 위 쿼리를 추가해 주세요.
-
@Column(columnDefinition = "TEXT")는 대부분 DB에서 호환되나, Dialect별로 대용량 TEXT 타입 인덱싱·조회 비용이 달라질 수 있습니다.
이미지 URL 다수 저장 시 성능 영향이 예상된다면, 별도 프로파일링 또는 별도 저장소 사용을 검토해 보시기 바랍니다.
| public FeedJpaEntity(String content, Integer likeCount, Integer commentCount, UserJpaEntity userJpaEntity, Boolean isPublic, int reportCount, BookJpaEntity bookJpaEntity, ContentList contentList) { | ||
| super(content, likeCount, commentCount, userJpaEntity); | ||
| this.isPublic = isPublic; | ||
| this.reportCount = reportCount; | ||
| this.bookJpaEntity = bookJpaEntity; | ||
| this.contentList = contentList; | ||
| if(contentList != null) this.contentList = contentList; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ContentList 참조의 방어적 복사 필요
현재 빌더에서 외부로부터 전달된 ContentList 인스턴스를 그대로 필드에 대입하고 있습니다. 이 인스턴스가 다른 곳에서도 공유/변경될 경우, 영속성 컨텍스트 내부 엔티티의 상태가 의도치 않게 변할 수 있습니다. 값객체는 가급적 방어적 복사를 적용하세요.
적용 제안:
- if(contentList != null) this.contentList = contentList;
+ if (contentList != null) {
+ // 외부 참조 차단을 위한 방어적 복사
+ this.contentList = ContentList.of(contentList.toUnmodifiableList());
+ }📝 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.
| public FeedJpaEntity(String content, Integer likeCount, Integer commentCount, UserJpaEntity userJpaEntity, Boolean isPublic, int reportCount, BookJpaEntity bookJpaEntity, ContentList contentList) { | |
| super(content, likeCount, commentCount, userJpaEntity); | |
| this.isPublic = isPublic; | |
| this.reportCount = reportCount; | |
| this.bookJpaEntity = bookJpaEntity; | |
| this.contentList = contentList; | |
| if(contentList != null) this.contentList = contentList; | |
| } | |
| public FeedJpaEntity(String content, | |
| Integer likeCount, | |
| Integer commentCount, | |
| UserJpaEntity userJpaEntity, | |
| Boolean isPublic, | |
| int reportCount, | |
| BookJpaEntity bookJpaEntity, | |
| ContentList contentList) { | |
| super(content, likeCount, commentCount, userJpaEntity); | |
| this.isPublic = isPublic; | |
| this.reportCount = reportCount; | |
| this.bookJpaEntity = bookJpaEntity; | |
| if (contentList != null) { | |
| // 외부 참조 차단을 위한 방어적 복사 | |
| this.contentList = ContentList.of(contentList.toUnmodifiableList()); | |
| } | |
| } |
| this.contentList = feed.getContentList(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
updateFrom에서도 ContentList 방어적 복사 권장
도메인 Feed로부터 받은 ContentList를 그대로 참조하면 동일한 공유 문제(엔티티 외부에서의 변경 전파)가 발생할 수 있습니다.
적용 제안:
- this.contentList = feed.getContentList();
+ this.contentList = ContentList.of(feed.getContentList().toUnmodifiableList());📝 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.
| this.contentList = feed.getContentList(); | |
| } | |
| this.contentList = ContentList.of(feed.getContentList().toUnmodifiableList()); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java around
lines 65-66, updateFrom currently assigns this.contentList =
feed.getContentList(); which exposes the internal collection to external
mutation; change this to perform a defensive copy (e.g., assign a new list
populated from feed.getContentList() or use Collections.emptyList() when null)
so the JPA entity holds its own list instance and external changes to the domain
Feed won't mutate the entity's state.
| private ContentList(List<String> values, int maxSize) { | ||
| this.maxSize = maxSize; | ||
| this.contents = new ArrayList<>(values == null ? List.of() : values); | ||
| validate(); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
maxSize 유효성 검증 추가 필요
maxSize가 음수인 경우를 방어하지 않습니다. 도메인 상 한도는 0 이상이어야 하므로 즉시 예외를 던지도록 해주세요.
적용 제안:
- private ContentList(List<String> values, int maxSize) {
- this.maxSize = maxSize;
- this.contents = new ArrayList<>(values == null ? List.of() : values);
- validate();
- }
+ private ContentList(List<String> values, int maxSize) {
+ if (maxSize < 0) {
+ throw new IllegalArgumentException("maxSize must be >= 0");
+ }
+ this.maxSize = maxSize;
+ this.contents = new ArrayList<>(values == null ? List.of() : values);
+ validate();
+ }📝 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 ContentList(List<String> values, int maxSize) { | |
| this.maxSize = maxSize; | |
| this.contents = new ArrayList<>(values == null ? List.of() : values); | |
| validate(); | |
| } | |
| private ContentList(List<String> values, int maxSize) { | |
| if (maxSize < 0) { | |
| throw new IllegalArgumentException("maxSize must be >= 0"); | |
| } | |
| this.maxSize = maxSize; | |
| this.contents = new ArrayList<>(values == null ? List.of() : values); | |
| validate(); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/feed/domain/value/ContentList.java around lines 22
to 27, add validation to reject negative maxSize: check if maxSize < 0 at the
start of the constructor and immediately throw an IllegalArgumentException (with
a clear message like "maxSize must be non-negative") before assigning fields or
calling validate(), so the domain invariant (maxSize >= 0) is enforced early.
| public static ContentList of(List<String> values, int maxSize) { | ||
| return new ContentList(new ArrayList<>(values), maxSize); | ||
| } |
There was a problem hiding this comment.
of(values, maxSize)에서 null 입력 NPE 가능성
현재 new ArrayList<>(values)로 즉시 복사하여 values == null일 때 NPE가 발생합니다. 다른 팩토리(of(List))와 동일하게 null-세이프하게 처리하세요.
패치:
- public static ContentList of(List<String> values, int maxSize) {
- return new ContentList(new ArrayList<>(values), maxSize);
- }
+ public static ContentList of(List<String> values, int maxSize) {
+ return new ContentList(values, maxSize);
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/feed/domain/value/ContentList.java around lines 32
to 34, the factory method of(List<String> values, int maxSize) currently calls
new ArrayList<>(values) which throws NPE when values is null; make it null-safe
like the other of(List) by treating a null values as an empty list (e.g., use
values == null ? new ArrayList<>() : new ArrayList<>(values) or
Objects.requireNonNullElseGet(values, ArrayList::new) to create the copy) and
then construct the ContentList with that copy and the provided maxSize.
| private void validate() { | ||
| validateImageCount(contents.size()); | ||
| //todo 필요 시 URL 형식 검증 추가 가능 | ||
| } | ||
|
|
There was a problem hiding this comment.
인스턴스별 maxSize가 실제로 적용되지 않는 버그
validate()에서 DEFAULT_MAX_SIZE를 사용하는 validateImageCount()를 호출하고 있어, 생성자에서 전달한 maxSize가 전혀 반영되지 않습니다. 즉, of(values, maxSize)가 의미가 없어집니다. 인스턴스 필드 maxSize를 사용하도록 수정해야 합니다.
수정 제안:
- private void validate() {
- validateImageCount(contents.size());
- //todo 필요 시 URL 형식 검증 추가 가능
- }
+ private void validate() {
+ if (contents.size() > maxSize) {
+ throw new InvalidStateException(CONTENT_LIST_SIZE_OVERFLOW);
+ }
+ // todo 필요 시 URL 형식 검증 추가 가능
+ }참고: 외부에서 기본 정책 검증용으로 validateImageCount(int size)를 계속 사용할 수 있도록 유지하되, 내부 인스턴스 검증은 maxSize를 준수해야 합니다.
📝 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 void validate() { | |
| validateImageCount(contents.size()); | |
| //todo 필요 시 URL 형식 검증 추가 가능 | |
| } | |
| private void validate() { | |
| if (contents.size() > maxSize) { | |
| throw new InvalidStateException(CONTENT_LIST_SIZE_OVERFLOW); | |
| } | |
| // todo 필요 시 URL 형식 검증 추가 가능 | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/feed/domain/value/ContentList.java around lines 46
to 50, the validate() method incorrectly calls
validateImageCount(contents.size()) which uses DEFAULT_MAX_SIZE, so the
instance-level maxSize passed to the constructor is ignored; change validate()
to call the instance field (validateImageCount(contents.size(), this.maxSize) or
call an instance overload) so the object's maxSize is enforced, while keeping
the existing validateImageCount(int size) overload for external default-policy
checks.
There was a problem hiding this comment.
뒤 성준님 pr에서 의견 남겨두었습니다! 참고 부탁드릴게여
There was a problem hiding this comment.
@buzz0331 네, 알겠습니다! 성준님께서 해당 PR의 피드백을 확인하실 것 같습니다. 팀 간 의견 공유 감사드립니다.
🧠 Learnings used
Learnt from: buzz0331
PR: THIP-TextHip/THIP-Server#266
File: src/main/java/konkuk/thip/feed/domain/value/ContentList.java:16-21
Timestamp: 2025-08-20T08:03:04.788Z
Learning: ContentList 클래스의 maxSize 필드는 현재 사용되지 않지만 의도적인 설계입니다. 현재는 Feed 생성 시 이미지 업로드 전 유효성 검증을 위해 static validateImageCount 메서드를 사용하며, maxSize는 향후 이미지 업로드 방식(presigned URL 등) 변경 시 사용할 예정입니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (6)
367-373: 주석과 구현 불일치: ISBN 검색에서 ‘내 피드 제외’ 조건이 누락되어 있습니다메서드 주석은 “다른 사용자 공개 피드”를 의도하지만, 현재 구현은 내 공개 피드도 포함됩니다. 요구사항이 “내 피드 제외”라면 아래와 같이 조건을 복구해 주세요. 아니라면 주석을 수정해 의도를 명확히 해주세요.
private BooleanExpression feedByBooksFilter(String isbn, Long userId) { return feed.status.eq(StatusType.ACTIVE) .and(feed.bookJpaEntity.isbn.eq(isbn)) -// .and(feed.userJpaEntity.userId.ne(userId)) - .and(feed.isPublic.eq(true)); + .and(feed.userJpaEntity.userId.ne(userId)) + .and(feed.isPublic.eq(true)); }
263-283: Alias NPE 가능성과 contentUrls 널 처리 보강alias 관계가 optional이거나 데이터 정합성 이슈가 있을 경우 NPE가 날 수 있습니다. 또한 contentList가 null일 가능성에 대비해 방어 코드를 추가하는 편이 안전합니다.
private FeedQueryDto toDto(FeedJpaEntity f, Integer priority) { boolean isPriorityFeed = (priority != null && priority == 1); - return FeedQueryDto.builder() + var aliasEntity = f.getUserJpaEntity().getAliasForUserJpaEntity(); + return FeedQueryDto.builder() .feedId(f.getPostId()) .creatorId(f.getUserJpaEntity().getUserId()) .creatorNickname(f.getUserJpaEntity().getNickname()) - .creatorProfileImageUrl(f.getUserJpaEntity().getAliasForUserJpaEntity().getImageUrl()) // TODO : DB에 String alias 만 저장하면 수정해야함 - .alias(f.getUserJpaEntity().getAliasForUserJpaEntity().getValue()) + .creatorProfileImageUrl(aliasEntity != null ? aliasEntity.getImageUrl() : null) // TODO : DB에 String alias 만 저장하면 수정해야함 + .alias(aliasEntity != null ? aliasEntity.getValue() : null) .createdAt(f.getCreatedAt()) .isbn(f.getBookJpaEntity().getIsbn()) .bookTitle(f.getBookJpaEntity().getTitle()) .bookAuthor(f.getBookJpaEntity().getAuthorName()) .contentBody(f.getContent()) - .contentUrls(f.getContentList().toArray(String[]::new)) + .contentUrls(f.getContentList() != null ? f.getContentList().toArray(String[]::new) : new String[0]) .likeCount(f.getLikeCount()) .commentCount(f.getCommentCount()) .isPublic(f.getIsPublic()) .isPriorityFeed(isPriorityFeed) .build(); }
346-365: QueryProjection(contentList)과 수동 매핑(contentUrls)의 타입 일관성 확인 필요toQueryDto/toSavedFeedQueryDto에서는
feed.contentList(List/값객체)를 그대로 투영하고, toDto에서는String[]로 변환합니다. FeedQueryDto의 생성자(@QueryProjection) 파라미터 타입과 빌더 세터 타입이 일치하는지 확인해 주세요. 현재처럼 이질적인 타입이 공존하면 직렬화/문서화와 프론트 소비 측면에서 혼란이 생길 수 있습니다.권장:
- 도메인/DTO에서 content를 일관되게 List(또는 ContentList 값객체)로 유지하고, 컨트롤러 계층에서만 필요 시 배열로 변환.
- 또는 배열을 표준으로 삼을 경우, QueryProjection에서도
feed.contentList→ 배열 변환을 적용하여 타입을 맞추기.Also applies to: 420-439
114-148: Keyset 페이징 안정성: 동률(createdAt/priority)에서 중복/누락 가능성. 3차 정렬 키(postId) 추가 권장현재 ORDER BY는
priority DESC, createdAt DESC만 사용합니다. 동일 priority/createdAt 레코드가 많은 경우 페이지 경계에서 중복/누락이 발생할 수 있습니다. 안정적인 정렬을 위해postId DESC를 3차 키로 추가하는 것을 권장합니다. 커서에도 3차 키를 반영하면 가장 견고합니다.최소 변경(정렬 안정성 향상):
- .orderBy(priority.desc(), feed.createdAt.desc()) + .orderBy(priority.desc(), feed.createdAt.desc(), feed.postId.desc())권장(완전한 keyset): lastPostId 커서를 추가하고 비교식을 3키로 확장
- private List<Tuple> fetchFeedIdsAndPriorityByFollowingPriority(Long userId, Integer lastPriority, LocalDateTime lastCreatedAt, int size) { + private List<Tuple> fetchFeedIdsAndPriorityByFollowingPriority(Long userId, Integer lastPriority, LocalDateTime lastCreatedAt, Long lastPostId, int size) { NumberExpression<Integer> priority = new CaseBuilder() ... - BooleanExpression cursorCondition = (lastPriority != null && lastCreatedAt != null) - ? priority.lt(lastPriority) - .or(priority.eq(lastPriority) - .and(feed.createdAt.lt(lastCreatedAt))) - : Expressions.TRUE; + BooleanExpression cursorCondition = + (lastPriority != null && lastCreatedAt != null && lastPostId != null) + ? priority.lt(lastPriority) + .or(priority.eq(lastPriority) + .and( + feed.createdAt.lt(lastCreatedAt) + .or(feed.createdAt.eq(lastCreatedAt).and(feed.postId.lt(lastPostId))) + )) + : Expressions.TRUE; - return jpaQueryFactory + return jpaQueryFactory .select(feed.postId, priority) ... - .orderBy(priority.desc(), feed.createdAt.desc()) + .orderBy(priority.desc(), feed.createdAt.desc(), feed.postId.desc()) .limit(size + 1) .fetch(); }추가로,
following을 LEFT JOIN하는 현재 방식은 중복 가능성이 있습니다(UNIQUE 제약이 없을 경우). EXISTS 서브쿼리로 바꾸면 더 단정해집니다(선택)..when( JPAExpressions.selectOne().from(following) .where(following.userJpaEntity.userId.eq(userId) .and(following.followingUserJpaEntity.userId.eq(feed.userJpaEntity.userId))) .exists() .and(feed.isPublic.isTrue()) ).then(1)
79-86: 순서 보존 시 null 엔티티 안전성 보강 및 priorityMap 접근 NPE 방지
entityMap.get(id)가 null을 반환할 가능성(동시성으로 삭제되었거나 상태가 바뀐 경우)에 대비해 필터링이 필요합니다. 또한priorityMap.get(e.getPostId())는 null일 수 있으므로getOrDefault가 더 안전합니다.- List<FeedJpaEntity> ordered = feedIds.stream() - .map(entityMap::get) - .toList(); + List<FeedJpaEntity> ordered = feedIds.stream() + .map(entityMap::get) + .filter(java.util.Objects::nonNull) + .toList(); - return ordered.stream() - .map(e -> toDto(e, priorityMap.get(e.getPostId()))) - .toList(); + return ordered.stream() + .map(e -> toDto(e, priorityMap.getOrDefault(e.getPostId(), null))) + .toList();같은 패턴을 사용하는 곳에도 동일 적용 권장:
- Line 101-109 (
findLatestFeedsByCreatedAt)- Line 191-199 (
findMyFeedsByCreatedAt)- Line 211-218 (
findSpecificUserFeedsByCreatedAt)
247-261: 파라미터 명 혼동: userId → feedOwnerId로 일관성 향상메서드 이름은
SpecificUser지만 파라미터 명은userId라 혼동 여지가 있습니다. 가독성을 위해feedOwnerId로 변경을 제안합니다(호출부도 같이 정리).-private List<Long> fetchSpecificUserFeedIdsByCreatedAt(Long userId, LocalDateTime lastCreatedAt, int size) { +private List<Long> fetchSpecificUserFeedIdsByCreatedAt(Long feedOwnerId, LocalDateTime lastCreatedAt, int size) { return jpaQueryFactory .select(feed.postId) .from(feed) .where( feed.status.eq(StatusType.ACTIVE), - feed.userJpaEntity.userId.eq(userId), + feed.userJpaEntity.userId.eq(feedOwnerId), feed.isPublic.eq(Boolean.TRUE), lastCreatedAt != null ? feed.createdAt.lt(lastCreatedAt) : Expressions.TRUE ) .orderBy(feed.createdAt.desc()) .limit(size + 1) .fetch(); }
📜 Review details
Configuration used: CodeRabbit UI
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 (2)
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/konkuk/thip/common/exception/code/ErrorCode.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: build
🔇 Additional comments (1)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (1)
59-87: size + 1 반환 정책 확인여러 메서드에서
limit(size + 1)로 조회한 결과를 그대로 반환합니다. 상위 계층이 “hasNext” 판단을 위해 size+1 목록을 기대한다면 OK입니다. 만약 API 응답 규격상 결과 리스트는 정확히size개여야 한다면, 리포지토리 계층에서 슬라이싱하거나 서비스 계층에서 슬라이싱이 보장되어야 합니다. 현재 컨트랙트를 사용하는 서비스/컨트롤러의 기대치를 한번 확인해 주세요.Also applies to: 96-109, 183-199, 202-218, 306-315, 335-344, 405-415
2 participants
#️⃣ 연관된 이슈
📝 작업 내용
이미지 url을 담아두던 Content 테이블을 제거하고 Feed 도메인에 완전 종속시켰습니다.
Feed와 Content는 1:N 관계였기 때문에 이를 풀어내기 위해 AttributeConverter를 적용하여 ContentList라는 일급 컬렉션을 JSON 형태로 파싱하여 DB에 저장되도록 하였습니다.
ContentList라는 일급 컬렉션을 도입한 효과는 다음과 같습니다.a. 페이징 처리시 구현 난이도 down
b. 성능 개선 (N+1 문제 자동 해결)
📸 스크린샷
💬 리뷰 요구사항
현재 FeedQueryRepositoryImpl에 있는 쿼리문의 쿼리 호출 자체는 변경하지 않고 내부적으로 조건만 바꿔둔 상태입니다. 초반에 저희는 Content 테이블이 있다는 전제하에 fetch join + 페이징 처리시 발생하는 문제를 우회하기 위해 다음과 같은 2가지 방법을 사용했었습니다.
2번에 해당하는 방법은 어느정도 해결되었지만, 1번은 생각보다 쿼리가 복잡해서 바로 해결하지는 못하였습니다. 추후에, 차차 리팩토링을 진행해봐야될 것 같습니다!
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
신기능
버그 수정
리팩터링
테스트