Skip to content

[refactor] alias, category, tag jpa 구조 수정#266

Merged
seongjunnoh merged 51 commits into
developfrom
refactor/#257-alias-category-tag-jpa-structure
Aug 20, 2025
Merged

[refactor] alias, category, tag jpa 구조 수정#266
seongjunnoh merged 51 commits into
developfrom
refactor/#257-alias-category-tag-jpa-structure

Conversation

@seongjunnoh
Copy link
Copy Markdown
Collaborator

@seongjunnoh seongjunnoh commented Aug 19, 2025

#️⃣ 연관된 이슈

closes #257

📝 작업 내용

기존에는 alias, category, tag DB table 을 정의 후, jpa entity로 취급 & 도메인 레이어 상에서는 enum 으로 관리하여 api들을 구현하였는데,

  1. 이 값들을 DB table로 정의하니 join이 추가적으로 발생
  2. 같은 데이터를 DB 테이블, enum 이렇게 중복해서 관리하니 유지보수 포인트가 2배가 되버림

크게 위 2개의 이슈가 있다고 판단하여,

  • alias, category, tag 을 저장하는 DB table 삭제 및 jpa entity 삭제
  • 대신 UserJpaEntity 컬럼으로 alias enum 추가, RoomJpaEntity 테이블의 컬럼으로 category enum 추가, FeedJpaEntity 테이블의 컬럼으로 List 을 추가

하였습니다.

위와 같이 수정하면서

  • 기존 QueryDto 의 Query Projection 생성자 필드로 정의된 alias, category, tag 의 정보들을 enum 그 자체로 수정 & 생성자 내부에서 enum의 정보를 파싱
  • Feed는 여러 Tag 들을 가질 수 있어야 하므로, FeedJpaEnttiy 에 List 를 AttributeConverter 를 활용하여 정의 (List 를 TagLIst 라는 일급 컬렉션으로 다시 감싸긴 했습니다 by @buzz0331)
  • alias, category, tag 와 관련된 모든 영속성 코드 삭제 및 수정
  • 기존 프로덕션 코드 및 테스트 코드 전부 수정

하는 과정을 거쳤습니다.

📸 스크린샷

💬 리뷰 요구사항

워낙 자주 사용되었던 jpa 구조를 건드렸고, @buzz0331 님과 같이 작업하는 중간에 refactor/#251-delete-content-table 브랜치를 머지하여서 변경된 파일의 개수가 상당히 많습니다.

일단 모든 테스트 코드가 통과되도록 수정해놨는데, 번거로우시겠지만 각자 담당한 api 들의 수정된 프로덕션 코드, 테스트 코드를 한번씩 확인해주시면 감사하겠습니다!!

📌 PR 진행 시 이러한 점들을 참고해 주세요

* P1 : 꼭 반영해 주세요 (Request Changes) - 이슈가 발생하거나 취약점이 발견되는 케이스 등
* P2 : 반영을 적극적으로 고려해 주시면 좋을 것 같아요 (Comment)
* P3 : 이런 방법도 있을 것 같아요~ 등의 사소한 의견입니다 (Chore)

Summary by CodeRabbit

  • 신기능
    • 피드 작성 정보가 enum 기반(Category/Tag) 매핑으로 제공됩니다.
    • 피드 콘텐츠/태그가 JSON 기반 리스트로 저장·조회되며 최대 개수 등 제약이 적용됩니다.
    • 팔로잉 우선 피드에 다중 기준(우선순위+생성일) 정렬이 추가되어 노출 품질이 향상되었습니다.
  • 버그 수정
    • 이미지/태그 유효성 검증 강화로 잘못된 입력에 대한 처리 안정성이 개선되었습니다.
  • 변경/개선
    • 별칭(Alias)·카테고리(Category) 도메인 값 사용으로 전반적인 조회/응답 구조가 단순화되었습니다.
    • 에러 코드 추가: 태그 중복, 태그 개수 초과, 콘텐츠 개수 초과, 존재하지 않는 콘텐츠.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Aug 19, 2025

Walkthrough

Alias/Category/Tag를 JPA 엔티티에서 도메인 값(열거/래퍼)으로 전환. TagList/ContentList 도입 및 JSON 컨버터 추가. 관련 JPA 엔티티/레포 제거와 매퍼/어댑터/쿼리 투영 전면 수정. EnumMappings로 카테고리-태그/별칭 매핑 제공. 에러코드 추가. 테스트 전반 도메인 값 기반으로 마이그레이션.

Changes

Cohort / File(s) Summary
Domain value 전환
src/main/java/konkuk/thip/user/domain/value/Alias.java, .../room/domain/value/Category.java, .../feed/domain/value/Tag.java, .../feed/domain/value/TagList.java, .../feed/domain/value/ContentList.java, .../feed/domain/Content.java
Alias/Category/Tag 패키지 이동·도입, TagList/ContentList 추가(검증 포함), 기존 Content 삭제.
Enum 매핑/유틸
src/main/java/konkuk/thip/common/util/EnumMappings.java
Alias↔Category, Category↔Tags, Tag→Category 매핑 제공 및 충돌 검증. 공개 조회 메서드 추가.
Feed JPA/매퍼/어댑터
.../feed/adapter/out/jpa/FeedJpaEntity.java, .../converter/TagListJsonConverter.java, .../mapper/FeedMapper.java, .../persistence/FeedCommandPersistenceAdapter.java, .../persistence/FeedQueryPersistenceAdapter.java, .../persistence/repository/FeedQueryRepository*.java, .../persistence/repository/FeedTag/FeedTagJpaRepository.java, .../persistence/repository/Tag/TagJpaRepository.java, .../jpa/TagJpaEntity.java, .../jpa/FeedTagJpaEntity.java
Feed에 ContentList/TagList JSON 저장으로 변경. Tag/FeedTag 엔티티·레포 제거. 매퍼/어댑터에서 태그·콘텐츠 연관 처리 제거 및 메서드 시그니처 단순화. findAllTags 제거.
Feed DTO/매퍼 변경
.../feed/application/port/out/dto/FeedQueryDto.java, .../feed/application/mapper/FeedQueryMapper.java
DTO 투영이 Alias 객체/ContentList 사용으로 변경. 태그 카테고리 결과 매핑 입력을 Map<Category,List>로 변경.
Feed 서비스 검증 변경
.../feed/application/service/FeedCreateService.java, .../FeedUpdateService.java, .../common/swagger/SwaggerResponseDescription.java, .../common/exception/code/ErrorCode.java
태그/이미지 검증을 TagList/ContentList 정적 검증으로 교체. 에러코드(TAG_SHOULD_BE_UNIQUE/TAG_LIST_SIZE_OVERFLOW/CONTENT_LIST_SIZE_OVERFLOW/CONTENT_NOT_FOUND) 추가 및 스웨거 반영.
User JPA/매퍼/어댑터
.../user/adapter/out/jpa/UserJpaEntity.java, .../mapper/UserMapper.java, .../persistence/UserCommandPersistenceAdapter.java, .../persistence/Following*Adapter.java, .../persistence/repository/following/FollowingQueryRepository*.java, .../persistence/repository/alias/*
User.alias를 Enum으로 저장. AliasJpaEntity/레포/쿼리 제거. 최신 팔로워 조회가 UserJpaEntity 리스트 반환 후 Alias에서 이미지 추출.
User 쿼리 DTO/투영
.../user/adapter/out/persistence/repository/UserQueryRepositoryImpl.java, .../user/application/port/out/dto/UserQueryDto.java, .../FollowingQueryDto.java
투영이 별칭 필드 묶음을 Alias 객체 단일 인자로 대체. 생성자/검증 로직 조정.
Room JPA/리포지토리/매퍼/어댑터
.../room/adapter/out/jpa/RoomJpaEntity.java, .../mapper/RoomMapper.java, .../persistence/RoomCommandPersistenceAdapter.java, .../persistence/RoomQueryPersistenceAdapter.java, .../persistence/repository/RoomQueryRepository*.java, .../jpa/CategoryJpaEntity.java, .../persistence/repository/category/CategoryJpaRepository.java
Room.category를 Enum으로 저장. Category 엔티티/레포 제거. 카테고리 파라미터/필터를 Category 값 타입으로 변경. 별칭 색상 조회는 EnumMappings 사용.
Comment/RoomPost/Following 쿼리 투영 변경
.../comment/adapter/out/persistence/repository/CommentQueryRepositoryImpl.java, .../comment/application/port/out/dto/CommentQueryDto.java, .../roompost/.../AttendanceCheckQueryRepositoryImpl.java, .../record/RecordQueryRepositoryImpl.java, .../roompost/application/port/out/dto/*
별칭 조인 제거, user.alias 직접 사용. 관련 DTO 생성자에 Alias 인자 추가.
Room/Feed 애플리케이션 서비스
.../room/application/service/*.java, .../room/application/port/out/RoomQueryPort.java, .../user/application/service/*.java, .../config/AwsS3ImageUrlInitializer.java
Category/Alias 임포트 경로 변경 및 EnumMappings 사용. 기존 흐름 유지.
테스트 마이그레이션/팩토리
src/test/java/**/*, .../common/util/TestEntityFactory.java
테스트 전반에서 Alias/Category/Tag JPA 의존 제거. 도메인 값 사용으로 변경. TestEntityFactory가 도메인 값/TagList/ContentList 기반으로 재작성.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant API as FeedWriteInfoController
  participant S as FeedShowWriteInfoService
  participant M as EnumMappings
  participant MAP as FeedQueryMapper

  C->>API: GET /feeds/write-info
  API->>S: showFeedWriteInfo()
  S->>M: getCategoryToTags()
  M-->>S: Map<Category, List<Tag>>
  S->>MAP: toTagsWithCategoryResult(map)
  MAP-->>S: List<TagsWithCategoryResult>
  S-->>API: Response DTO
  API-->>C: 200 OK (categoryList + tagList)
Loading
sequenceDiagram
  autonumber
  participant S as FollowingQueryPersistenceAdapter
  participant R as FollowingQueryRepository
  participant U as UserJpaEntity
  participant A as Alias(enum)

  S->>R: findLatestFollowers(userId, size)
  R-->>S: List<UserJpaEntity>
  S->>U: stream().map(getAlias)
  U-->>S: A
  S-->>S: map(Alias::getImageUrl)
  S-->>S: collect to List<String>
  S-->>S: return URLs
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Assessment against linked issues

Objective Addressed Explanation
Alias/Category/Tag JPA 제거 및 도메인 값 전환 (#257)
관련 레포지토리/조인/투영 제거·치환 (#257)
매퍼/어댑터/서비스 시그니처 및 로직 정리 (#257)
테스트 전반 도메인 값 기반으로 마이그레이션 (#257)

Assessment against linked issues: Out-of-scope changes

Code Change Explanation
콘텐츠 개수/존재 검증 에러코드 추가 및 사용 변경 (ErrorCode.java, SwaggerResponseDescription.java, ContentList.java) 이슈는 alias/category/tag 구조 변경에 한정되어 있으며, 콘텐츠 리스트 검증/에러코드 도입은 별도 범주일 가능성이 높음.
FeedQueryRepositoryImpl에 팔로잉 우선순위 계산 로직(CaseBuilder) 추가 별칭/카테고리/태그 구조 변경과 직접적인 관련이 불명확한 쿼리 우선순위 확장임.

Possibly related PRs

Suggested reviewers

  • hd0rable

Poem

토끼는 귀를 쫑긋, 태그를 정돈해
별칭은 색과 빛, 값으로 빚어내
카테고리 길잡이는 지도처럼 정해져
DB의 숲을 떠나, 열거의 들로 가네
딸깍, 테스트 뛰놀고
톡, 에러코드 노래해
깡총! 새 길 위로 코드가 춤추네 🐇✨

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 Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/#257-alias-category-tag-jpa-structure

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Aug 19, 2025

Test Results

412 tests   412 ✅  32s ⏱️
124 suites    0 💤
124 files      0 ❌

Results for commit e89d772.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (8)
src/main/java/konkuk/thip/roompost/adapter/out/persistence/repository/attendancecheck/AttendanceCheckQueryRepositoryImpl.java (1)

26-41: 소프트 딜리트 상태 필터 누락 가능성 — status = ACTIVE 조건 추가 권고

AttendanceCheck는 status 필드로 소프트 딜리트를 구현합니다(과거 회고 학습). 리스트 조회에서도 INACTIVE/EXPIRED를 제외해야 무결성이 보장됩니다.

아래와 같이 상태 필터를 추가해 주세요.

@@
-import com.querydsl.core.types.dsl.BooleanExpression;
+import com.querydsl.core.types.dsl.BooleanExpression;
+import konkuk.thip.common.jpa.StatusType;
@@
-        BooleanExpression roomPredicate = attendanceCheck.roomJpaEntity.roomId.eq(roomId);
-        BooleanExpression cursorPredicate = (lastCreatedAt == null) ? null : attendanceCheck.createdAt.lt(lastCreatedAt);
+        BooleanExpression roomPredicate = attendanceCheck.roomJpaEntity.roomId.eq(roomId);
+        BooleanExpression cursorPredicate = (lastCreatedAt == null) ? null : attendanceCheck.createdAt.lt(lastCreatedAt);
+        BooleanExpression statusPredicate = attendanceCheck.status.eq(StatusType.ACTIVE);
@@
-                .where(roomPredicate, cursorPredicate)
+                .where(roomPredicate, cursorPredicate, statusPredicate)

추가로 createdAt DESC 커서 전략은 현재 프로젝트의 선호 방향(LocalDateTime 단일 커서)과 일치합니다.

src/main/java/konkuk/thip/feed/application/service/FeedUpdateService.java (1)

58-66: FeedUpdateService.handleFeedImageDelete: null 안전성·성능 개선 및 미사용 메서드 정리 필요

실제 S3 이미지 삭제는 FeedCreateService.java:64 (catch 블록)에서만 수행되고 있으며, handleFeedImageDelete는 어디에서도 호출되지 않습니다.
따라서 아래 작업을 필수 리팩토링으로 진행해 주세요.

• 미사용 메서드 정리

  • handleFeedImageDelete가 참조되지 않으므로 제거하거나 Javadoc/TODO 주석으로 의도를 명확히 표시
    • null 안전성 및 성능 개선
  • remainImageUrls null 체크 후 Set으로 변환
  • contains 반복 비용을 O(n)으로 개선
    • 향후 이벤트 기반 삭제로 전환 권장
  • 업데이트 후 @TransactionalEventListener(phase = AFTER_COMMIT)로 삭제 이벤트 발행

예시 diff:

-    private void handleFeedImageDelete(Feed feed, List<String> remainImageUrls) {
-        List<String> toDelete = feed.getContentList().stream()
-                .filter(url -> !remainImageUrls.contains(url))
-                .toList();
-        if (!toDelete.isEmpty()) {
-            s3CommandPort.deleteImages(toDelete);
-        }
-    }
+    /**
+     * TODO: 현재 미사용 상태. 업데이트 후 @TransactionalEventListener(AFTER_COMMIT) 이벤트 기반 삭제로 전환 예정
+     */
+    private void handleFeedImageDelete(Feed feed, List<String> remainImageUrls) {
+        var remain = (remainImageUrls == null)
+                ? java.util.Collections.emptySet()
+                : new java.util.HashSet<>(remainImageUrls);
+        var toDelete = feed.getContentList().stream()
+                .filter(url -> !remain.contains(url))
+                .toList();
+        if (!toDelete.isEmpty()) {
+            s3CommandPort.deleteImages(toDelete);
+        }
+    }
src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java (1)

30-32: FeedQueryMapper의 aliasName 매핑을 표시용 값(value)으로 통일해야 합니다

현재 toFeedShowAllResponse(30번 라인)과 마지막 메서드(178번 라인)는 dto.alias 코드를 그대로 주입하고 있어, toFeedRelatedWithBookDto(148번 라인)에서 사용하는 Alias.from(...).getValue() 표현과 불일치합니다. UI에 표시할 별칭은 코드가 아닌 사용자가 볼 수 있는 값(value)이므로, 아래와 같이 변경해주세요:

  • 변경 대상
    • src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:30
    • src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:178
--- a/src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java
@@ -30,7 +30,7 @@
-    @Mapping(target = "aliasName", source = "dto.alias")
+    @Mapping(target = "aliasName", expression = "java(Alias.from(dto.alias()).getValue())")
@@ -178,7 +178,7 @@
-    @Mapping(target = "aliasName", source = "dto.alias")
+    @Mapping(target = "aliasName", expression = "java(Alias.from(dto.alias()).getValue())")

이렇게 하면 30번·178번 라인 모두 getValue()로 통일되어, 148번 라인에서 이미 사용 중인 방식과 일관성을 유지할 수 있습니다.

src/main/java/konkuk/thip/feed/application/port/out/dto/FeedQueryDto.java (1)

26-26: Null 가능성 있는 isPublic 언박싱 NPE 위험

레코드 필드는 boolean isPublic인데, @QueryProjection 생성자 파라미터는 Boolean isPublic입니다. 현재 this(...)에 그대로 전달되어 null이면 NPE가 납니다.

다음과 같이 null-safe 하게 처리해 주세요:

-                isPublic,
+                (isPublic != null && isPublic),

Also applies to: 44-47, 62-62

src/main/java/konkuk/thip/feed/domain/Feed.java (1)

30-31: Boolean isPublic로 인한 NPE 가능성 (불리언 언박싱)

여러 곳에서 !this.isPublic 또는 !isPublic이 사용되는데, 필드/인자가 Boolean이므로 null이면 NPE가 발생합니다. 접근제어/뷰권한 검증 경로에서 치명적입니다.

아래와 같이 primitive로 변경하고, 입력은 null-safe로 변환하세요.

-    private Boolean isPublic;
+    private boolean isPublic;
-    public static Feed withoutId(String content, Long creatorId, Boolean isPublic, Long targetBookId,
+    public static Feed withoutId(String content, Long creatorId, Boolean isPublic, Long targetBookId,
                                  List<String> tagValues, List<String> imageUrls) {
   ...
-                .isPublic(isPublic)
+                .isPublic(isPublic != null && isPublic)
-    public void updateVisibility(Long userId, Boolean isPublic) {
+    public void updateVisibility(Long userId, boolean isPublic) {
         validateCreator(userId);
         this.isPublic = isPublic;
     }

Also applies to: 120-121, 207-209

src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateControllerTest.java (1)

74-83: 검증 헬퍼가 잘못된 feedId/userId를 사용함 (검증 실패 원인 왜곡 위험)

현재 assertBadRequest는 하드코딩된 feedId(1)와 userId(100L)를 사용합니다. 이로 인해

  • 존재하지 않는 feed → 404/403 등으로 먼저 떨어져 태그/이미지 검증을 통과하지 못함
  • 권한 없는 user → 403이 먼저 발생

등의 오탐 가능성이 큽니다. 검증 대상 feed와 작성자 id를 파라미터로 받도록 수정하세요.

-    private void assertBadRequest(int expectedCode, Map<String, Object> request, String message) throws Exception {
-        mockMvc.perform(patch("/feeds/1")
-                                .requestAttr("userId", 100L)
+    private void assertBadRequest(long feedId, long userId, int expectedCode, Map<String, Object> request, String message) throws Exception {
+        mockMvc.perform(patch("/feeds/" + feedId)
+                                .requestAttr("userId", userId)
                                 .contentType(MediaType.APPLICATION_JSON)
                                 .content(objectMapper.writeValueAsBytes(request))
                 )
                 .andExpect(status().isBadRequest())
                 .andExpect(jsonPath("$.code").value(expectedCode))
                 .andExpect(jsonPath("$.message", containsString(message)));
     }
src/main/java/konkuk/thip/room/adapter/out/persistence/repository/RoomQueryRepositoryImpl.java (1)

175-186: 추천 방 조회에 ACTIVE 상태 필터 누락

공개/모집중만 조건으로는 비활성(삭제/중지) 방이 포함될 수 있습니다. 다른 메서드들과 동일하게 ACTIVE 조건을 추가하세요.

                 .where(
                         room.category.eq(category)
                                 .and(room.startDate.after(LocalDate.now()))     // 모집 마감 시각 > 현재 시각
-                                .and(room.roomId.ne(roomId))// 현재 방 제외
-                                .and(room.isPublic.isTrue()) // 공개방 만
+                                .and(room.roomId.ne(roomId))                     // 현재 방 제외
+                                .and(room.isPublic.isTrue())                      // 공개방 만
+                                .and(room.status.eq(StatusType.ACTIVE))          // 비활성 방 제외
                 )
src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateApiTest.java (1)

158-170: 전체 수정 시나리오도 검증 전 clear 필요

일관성 있게 캐시를 비워 실제 DB 커밋 상태를 검증하세요.

적용 예시:

         // then
         result.andExpect(status().isOk());
 
-        FeedJpaEntity updated = feedJpaRepository.findById(feedId).orElseThrow();
+        em.clear();
+        FeedJpaEntity updated = feedJpaRepository.findById(feedId).orElseThrow();
         // 1. 본문
         assertThat(updated.getContent()).isEqualTo("전부 수정되는 피드 테스트");
         // 2. 공개 여부
         assertThat(updated.getIsPublic()).isFalse();
         // 3. 이미지
         assertThat(updated.getContentList()).hasSize(1);
         assertThat(updated.getContentList().get(0)).isEqualTo("https://s3-mock/image-2.jpg");
         // 4. 태그 갯수
-        long tagCount = feedJpaRepository.findById(feedId).orElseThrow().getTagList().size();
+        long tagCount = feedJpaRepository.findById(feedId).orElseThrow().getTagList().size();
         assertThat(tagCount).isEqualTo(2);

추가로, 태그 “갯수” 뿐 아니라 실제 값 동일성(집합 비교)도 검증하면 회귀 방지에 유리합니다. 필요 시 요청에 보낸 값과 updated.getTagList()containsExactlyInAnyOrder로 비교하는 어서션 추가 가능.

Comment thread src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java
String creatorNickname,
String creatorProfileImageUrl,
String alias,
Alias alias,
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

alias 파라미터 null 처리 필요

DB/조인 상황에 따라 Alias가 없을 수 있습니다. 현재는 alias.getImageUrl()/getValue()를 바로 호출하여 NPE 위험이 있습니다.

다음과 같이 @Nullable 표시 및 null-safe 접근을 권장합니다.

-            Alias alias,
+            @jakarta.annotation.Nullable Alias alias,
-                alias.getImageUrl(),
-                alias.getValue(),
+                alias != null ? alias.getImageUrl() : null,
+                alias != null ? alias.getValue() : null,

Also applies to: 52-54

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음 QueryDto 에서 null 체크를 매번 하는게 좋은걸까??

QueryDto 는 QueryDSL 코드에서의 반환값을 우리가 정의한 도메인의 읽기모델로 변환해서 사용하기 위한 dto인데, 이때 QueryDSL 에서 조회한 값이 jpa nullable = false 속성일 경우는 굳이 dto 내부에서 null 체크를 할 필요가 없지 않나?

방어적인 코드를 위해서 dto 생성자에서 null 체크를 하는게 좋다는 의미인거야? @coderabbitai

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seongjunnoh 정확한 지적이십니다!

당신 말씀이 맞습니다. UserJpaEntityalias 필드가 @Column(nullable = false)로 정의되어 있고, @Enumerated(EnumType.STRING)으로 enum 값을 직접 저장하는 구조에서는 QueryDSL이 user.alias를 조회할 때 항상 non-null Alias 값을 반환할 것입니다.

이 경우 DTO 생성자에서 null 체크는 실제로 일어나지 않을 상황에 대한 불필요한 방어코드가 됩니다. DB 제약조건이 이미 null 방지를 보장하고 있으니까요.

제가 제안한 null 체크는 일반적인 방어적 프로그래밍 관점에서 나온 것이었는데, 귀하의 도메인 모델 설계와 DB 제약조건을 고려할 때는 오히려 불필요한 복잡성을 추가하는 것 같습니다.

현재 코드 그대로 두시는 것이 더 깔끔하고 실용적인 접근이라고 생각합니다.

  ∩   ∩
 (  ._. )
o_(")(")

thoughtful rabbit


✏️ Learnings added
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#266
File: src/main/java/konkuk/thip/feed/application/port/out/dto/FeedQueryDto.java:35-35
Timestamp: 2025-08-20T08:01:11.226Z
Learning: seongjunnoh는 DB 제약조건(nullable = false)이 이미 보장하는 상황에서 DTO 생성자의 추가적인 null 체크를 불필요한 방어코드로 보며, 실용적인 구현을 선호한다. 기술적 완벽성보다 실제 도메인 모델의 제약조건을 신뢰하는 접근을 취한다.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.

Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.

Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.

Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.371Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.

Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.

Comment on lines +29 to 31
TagList.validateTags(Tag.fromList(command.tagList()));
ContentList.validateImageCount(command.remainImageUrls() != null ? command.remainImageUrls().size() : 0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Tag 리스트 null 처리 누락 가능성 (NPE 위험)

TagList.validateTags(Tag.fromList(command.tagList()))command.tagList()가 null인 경우 NPE가 발생할 수 있습니다. 현재 applyPartialFeedUpdate에서는 command.tagList() != null일 때만 업데이트를 수행하므로, 검증도 동일한 조건으로 제한하는 게 안전합니다.

아래처럼 null 가드를 추가해 주세요.

-        TagList.validateTags(Tag.fromList(command.tagList()));
+        if (command.tagList() != null) {
+            TagList.validateTags(Tag.fromList(command.tagList()));
+        }

추가로, 태그 검증이 애플리케이션 서비스와 도메인(예: feed.updateTags) 양쪽에 중복된다면 한쪽으로 일원화하는 것을 권장합니다. 유지보수성과 불변 조건 일관성이 좋아집니다.


🏁 Script executed:

#!/bin/bash
set -eu
# Display the first 200 lines of the service to inspect method context
sed -n '1,200p' src/main/java/konkuk/thip/feed/application/service/FeedUpdateService.java
# Locate the applyPartialFeedUpdate method signature and its usage of command.tagList()
rg -n "applyPartialFeedUpdate" src/main/java/konkuk/thip/feed/application/service/FeedUpdateService.java
rg -n "command\.tagList" src/main/java/konkuk/thip/feed/application/service/FeedUpdateService.java

Length of output: 2988


TagList 검증 시 null 체크 필요 (NPE 위험)

updateFeed 메서드 초반부에서 command.tagList()null인 경우
TagList.validateTags(Tag.fromList(command.tagList())) 호출 시 NPE가 발생할 수 있습니다.
이후 applyPartialFeedUpdate에서는 이미 null 체크 후 업데이트하므로, 검증 단계에도 동일하게 null 가드를 추가해야 합니다.

  • 위치: src/main/java/konkuk/thip/feed/application/service/FeedUpdateService.java (29행)
  • 수정 제안:
-        TagList.validateTags(Tag.fromList(command.tagList()));
+        if (command.tagList() != null) {
+            TagList.validateTags(Tag.fromList(command.tagList()));
+        }

또한, 애플리케이션 서비스(FeedUpdateService)와 도메인(feed.updateTags) 양쪽에서 태그 검증이 중복된다면
한쪽으로 일원화하여 유지보수성과 불변 조건 일관성을 높일 것을 권장합니다.

📝 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.

Suggested change
TagList.validateTags(Tag.fromList(command.tagList()));
ContentList.validateImageCount(command.remainImageUrls() != null ? command.remainImageUrls().size() : 0);
if (command.tagList() != null) {
TagList.validateTags(Tag.fromList(command.tagList()));
}
ContentList.validateImageCount(command.remainImageUrls() != null ? command.remainImageUrls().size() : 0);
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/feed/application/service/FeedUpdateService.java
around lines 29-31, the call
TagList.validateTags(Tag.fromList(command.tagList())) can throw NPE when
command.tagList() is null; guard the call by checking command.tagList() != null
(or normalize to an empty list) before invoking Tag.fromList and validateTags
(e.g., only call validateTags when tagList is non-null), and consider removing
or consolidating duplicate tag validation between this service and the
feed.updateTags domain method so validation lives in a single place.

Comment on lines 162 to 168
public void updateImages(Long userId, List<String> newImageUrls) {
validateCreator(userId);
validateImageCount(newImageUrls.size());
// validateImageCount(newImageUrls.size());
validateOwnsImages(newImageUrls);

this.contentList = convertToContentList(newImageUrls);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

이미지 업데이트 시 “신규 이미지 추가 금지”로 잠재적 기능 장애

updateImages에서 validateOwnsImages(newImageUrls)는 “새로 전달된 URL들이 현재 피드에 이미 존재해야 한다”는 제약을 강제합니다. 이로 인해 피드 이미지 수정 시 신규 이미지 추가가 불가능해집니다(현실 시나리오와 상충).

다음 중 하나로 수정 제안:

  • 단순화: 업데이트 시 소유권 검증을 제거하고, 삭제 플로우에서만 “해당 피드에 존재하는지”를 검증.
  • 또는 “신규 추가는 허용, 제거 대상만 현재 피드에 존재하는지 검증”으로 로직 재설계.

예시(간단 제거안):

     public void updateImages(Long userId, List<String> newImageUrls) {
         validateCreator(userId);
-//        validateImageCount(newImageUrls.size());
-        validateOwnsImages(newImageUrls);
-
         this.contentList = convertToContentList(newImageUrls);
     }

또는 검증 메서드명을 validateAllBelongToThisFeed 등으로 명확히 하여, 실제 용도(삭제 확인)에 맞게 호출 지점을 재배치하는 것을 권장합니다.

Also applies to: 170-179

🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/feed/domain/Feed.java around lines 162-168 (also
apply same change to lines 170-179), validateOwnsImages(newImageUrls) is
incorrectly enforcing that incoming image URLs must already belong to the feed,
preventing adding new images; remove this ownership check from updateImages and
instead only validate ownership in deletion flows (or rename the method to
validateAllBelongToThisFeed and call it from the removeImages/delete path).
Update updateImages to validate creator and image count only, then convert and
set the contentList; ensure ownership validation is preserved for deletion
endpoints where we verify removed URLs exist on the feed.

Comment on lines +16 to +21
private static final int DEFAULT_MAX_SIZE = 3;

private final List<String> contents;

private final int maxSize;

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

maxSize가 검증에 반영되지 않음 — API 의미 불일치

of(List, int maxSize)로 커스텀 최대값을 지정할 수 있는데, 실제 검증은 DEFAULT_MAX_SIZE만 사용합니다. 이는 숨은 버그로 이어질 수 있습니다.

두 가지 중 하나를 선택하세요.

옵션 A) 인스턴스 수준으로 검증 변경(권장):

-    public static void validateImageCount(int size) {
-        if (size > DEFAULT_MAX_SIZE) {
-            throw new InvalidStateException(CONTENT_LIST_SIZE_OVERFLOW);
-        }
-    }
+    private void validateImageCount(int size) {
+        if (size > maxSize) {
+            throw new InvalidStateException(CONTENT_LIST_SIZE_OVERFLOW);
+        }
+    }

옵션 B) 커스텀 maxSize 지원을 당장 쓰지 않는다면, maxSize 필드와 of(List, int maxSize) 팩토리를 제거하여 API를 단순화하세요. 필요 시 확장 시점에 다시 도입하는 것이 안전합니다.

Also applies to: 40-49

🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/feed/domain/value/ContentList.java around lines
16-21 (and similarly lines 40-49), the class stores a maxSize field but
validation still uses DEFAULT_MAX_SIZE causing API/behavior mismatch; either (A
- recommended) update validation and factory to validate against the instance's
maxSize (use the provided maxSize in of(List,int) and any constructors so
contents.size() <= this.maxSize) and adjust error messages accordingly, or (B)
remove the maxSize field and the of(List,int) factory so the class always uses
DEFAULT_MAX_SIZE; implement one consistent approach across both creation paths
and update unit tests and callers to match the chosen behavior.

Copy link
Copy Markdown
Contributor

@buzz0331 buzz0331 Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코래 말처럼 maxSize를 사용하고 있지 않은데 이 부분은 현재 Feed 생성시에 이미지 업로드 전에 유효성 검증을 하기 위해서 static 메서드로 추가해둔건데 마찬가지로 이미지 업로드 방식을 바꾸면 maxSize가 사용 가능해질 것 같습니다!

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@buzz0331 이해했습니다! 현재는 피드 생성 시 이미지 업로드 전 유효성 검증을 위한 static 메서드가 필요하고, maxSize 필드는 향후 이미지 업로드 방식 변경에 대비한 준비 단계인 것이군요.

단계적 접근 방식으로 API를 미리 준비해두신 것 같습니다. 향후 presigned URL 방식으로 전환되면 인스턴스별 maxSize 검증이 의미를 가지게 되겠네요.

설명해주셔서 감사합니다! 🚀


✏️ Learnings added
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 등) 변경 시 사용할 예정입니다.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment on lines 91 to 93
result.andExpect(status().isOk());
long tagCount = feedTagJpaRepository.findAll().stream()
.filter(ft -> ft.getFeedJpaEntity().getPostId().equals(feedId))
.count();
long tagCount = feedJpaRepository.findById(feedId).orElseThrow().getTagList().size();
assertThat(tagCount).isEqualTo(2);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

MockMvc 요청 후 1차 캐시(영속성 컨텍스트)로 인해 갱신 내용 미반영 가능성 — clear 필요

테스트 클래스가 @transactional이어서 기존 영속성 컨텍스트에 캐싱된 엔티티가 그대로 반환될 수 있습니다. 요청 처리 트랜잭션과 테스트 트랜잭션이 분리되므로, 검증 전에 EntityManager.clear()로 캐시를 비워야 합니다.

아래처럼 한 줄 추가를 권장합니다:

         // then
         result.andExpect(status().isOk());
+        em.clear(); // 테스트 영속성 컨텍스트 초기화(갱신 내용 재조회)
         long tagCount = feedJpaRepository.findById(feedId).orElseThrow().getTagList().size();
         assertThat(tagCount).isEqualTo(2);

추가 필드 주입(파일 상단 필드 선언부)에 필요합니다:

import jakarta.persistence.EntityManager;

@Autowired private EntityManager em;
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateApiTest.java around
lines 91 to 93, the test reads the entity after performing a MockMvc request but
because the test class is @Transactional the persistence context may still hold
a cached entity and not reflect changes from the request's transaction; to fix,
inject an EntityManager (add import jakarta.persistence.EntityManager and
@Autowired private EntityManager em; at the test class fields) and call
em.clear() immediately after result.andExpect(status().isOk()); before fetching
the entity so the subsequent repository findById reads fresh state from the
database.

Comment on lines 123 to 127
result.andExpect(status().isOk());
FeedJpaEntity updatedFeed = feedJpaRepository.findById(feedId).orElseThrow();
assertThat(updatedFeed.getContentList()).hasSize(1);
assertThat(updatedFeed.getContentList().get(0).getContentUrl()).isEqualTo("https://s3-mock/image-2.jpg");
List<ContentJpaEntity> contentRows = contentJpaRepository.findAll().stream()
.filter(c -> c.getPostJpaEntity().getPostId().equals(feedId))
.toList();
assertThat(contentRows).hasSize(1);
assertThat(contentRows.get(0).getContentUrl()).isEqualTo("https://s3-mock/image-2.jpg");
assertThat(updatedFeed.getContentList().get(0)).isEqualTo("https://s3-mock/image-2.jpg");
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

이미지 유지 테스트도 동일 이슈 — 검증 전 clear 필요

동일한 1차 캐시 문제를 방지하려면 검증 전에 em.clear() 호출이 필요합니다.

적용 예시:

         // then
         result.andExpect(status().isOk());
+        em.clear();
         FeedJpaEntity updatedFeed = feedJpaRepository.findById(feedId).orElseThrow();
         assertThat(updatedFeed.getContentList()).hasSize(1);
         assertThat(updatedFeed.getContentList().get(0)).isEqualTo("https://s3-mock/image-2.jpg");
📝 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.

Suggested change
result.andExpect(status().isOk());
FeedJpaEntity updatedFeed = feedJpaRepository.findById(feedId).orElseThrow();
assertThat(updatedFeed.getContentList()).hasSize(1);
assertThat(updatedFeed.getContentList().get(0).getContentUrl()).isEqualTo("https://s3-mock/image-2.jpg");
List<ContentJpaEntity> contentRows = contentJpaRepository.findAll().stream()
.filter(c -> c.getPostJpaEntity().getPostId().equals(feedId))
.toList();
assertThat(contentRows).hasSize(1);
assertThat(contentRows.get(0).getContentUrl()).isEqualTo("https://s3-mock/image-2.jpg");
assertThat(updatedFeed.getContentList().get(0)).isEqualTo("https://s3-mock/image-2.jpg");
}
// then
result.andExpect(status().isOk());
em.clear();
FeedJpaEntity updatedFeed = feedJpaRepository.findById(feedId).orElseThrow();
assertThat(updatedFeed.getContentList()).hasSize(1);
assertThat(updatedFeed.getContentList().get(0)).isEqualTo("https://s3-mock/image-2.jpg");
}
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateApiTest.java around
lines 123 to 127, the test reads the updated entity from the repository but
doesn't clear the persistence context first, so the first-level (EntityManager)
cache may return stale state; before fetching the entity for assertions, call
entityManager.clear() (or em.clear()) to evict cached entities so the repository
will load the current DB state, then perform the assertions.

Comment on lines +143 to 146
req.put("tagList", List.of(PHYSICS.getValue(), CHEMISTRY.getValue(), KOREAN_NOVEL.getValue(),
FOREIGN_NOVEL.getValue(), CLASSIC_LITERATURE.getValue(), HISTORY.getValue()));
assertBadRequest(INVALID_FEED_COMMAND.getCode(), req, "태그는 최대 5개까지 입력할 수 있습니다.");
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

검증 헬퍼 시그니처 변경에 맞춘 호출부 수정

검증이 권한/존재성 체크에 막히지 않도록, 실제 생성된 feedId/작성자 id를 사용하세요.

-            req.put("tagList", List.of(PHYSICS.getValue(), CHEMISTRY.getValue(), KOREAN_NOVEL.getValue(),
-                    FOREIGN_NOVEL.getValue(), CLASSIC_LITERATURE.getValue(), HISTORY.getValue()));
-            assertBadRequest(INVALID_FEED_COMMAND.getCode(), req, "태그는 최대 5개까지 입력할 수 있습니다.");
+            req.put("tagList", List.of(
+                    PHYSICS.getValue(), CHEMISTRY.getValue(), KOREAN_NOVEL.getValue(),
+                    FOREIGN_NOVEL.getValue(), CLASSIC_LITERATURE.getValue(), HISTORY.getValue()
+            ));
+            assertBadRequest(savedFeedId, creatorUserId, INVALID_FEED_COMMAND.getCode(), req, "태그는 최대 5개까지 입력할 수 있습니다.");
📝 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.

Suggested change
req.put("tagList", List.of(PHYSICS.getValue(), CHEMISTRY.getValue(), KOREAN_NOVEL.getValue(),
FOREIGN_NOVEL.getValue(), CLASSIC_LITERATURE.getValue(), HISTORY.getValue()));
assertBadRequest(INVALID_FEED_COMMAND.getCode(), req, "태그는 최대 5개까지 입력할 수 있습니다.");
}
req.put("tagList", List.of(
PHYSICS.getValue(), CHEMISTRY.getValue(), KOREAN_NOVEL.getValue(),
FOREIGN_NOVEL.getValue(), CLASSIC_LITERATURE.getValue(), HISTORY.getValue()
));
assertBadRequest(savedFeedId, creatorUserId, INVALID_FEED_COMMAND.getCode(), req, "태그는 최대 5개까지 입력할 수 있습니다.");
}
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateControllerTest.java
around lines 143 to 146, the test still calls the validation helper with the old
signature causing authorization/existence checks to block validation; update the
test to create (or retrieve) a real feed and its author id before invoking the
helper and pass those ids into the new assertBadRequest signature so the request
validation runs without failing on auth/existence — i.e., create the feed (or
use test fixture), obtain feedId and authorId, put your tagList into req as
before, then call assertBadRequest with the new parameters including the actual
feedId and authorId and the expected error code/message.

Comment on lines +152 to 154
req.put("tagList", List.of(KOREAN_NOVEL.getValue(), KOREAN_NOVEL.getValue()));
assertBadRequest(INVALID_FEED_COMMAND.getCode(), req, TAG_SHOULD_BE_UNIQUE.getMessage());
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

검증 헬퍼 시그니처 변경에 맞춘 호출부 수정 (중복 태그 케이스)

동일하게 feedId/작성자 id를 사용하도록 변경하세요.

-            req.put("tagList", List.of(KOREAN_NOVEL.getValue(), KOREAN_NOVEL.getValue()));
-            assertBadRequest(INVALID_FEED_COMMAND.getCode(), req, TAG_SHOULD_BE_UNIQUE.getMessage());
+            req.put("tagList", List.of(KOREAN_NOVEL.getValue(), KOREAN_NOVEL.getValue()));
+            assertBadRequest(savedFeedId, creatorUserId, INVALID_FEED_COMMAND.getCode(), req, TAG_SHOULD_BE_UNIQUE.getMessage());
📝 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.

Suggested change
req.put("tagList", List.of(KOREAN_NOVEL.getValue(), KOREAN_NOVEL.getValue()));
assertBadRequest(INVALID_FEED_COMMAND.getCode(), req, TAG_SHOULD_BE_UNIQUE.getMessage());
}
req.put("tagList", List.of(KOREAN_NOVEL.getValue(), KOREAN_NOVEL.getValue()));
assertBadRequest(savedFeedId, creatorUserId, INVALID_FEED_COMMAND.getCode(), req, TAG_SHOULD_BE_UNIQUE.getMessage());
}
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateControllerTest.java
around lines 152-154, the test call for the duplicate-tag case needs to match
the updated validation helper signature: change the assertBadRequest invocation
to pass the feedId and the authorId (writer id) as the identifiers used by the
helper, along with the existing expected error code, request body and message;
ensure the order matches the new signature so the helper receives feedId,
authorId, INVALID_FEED_COMMAND.getCode(), req,
TAG_SHOULD_BE_UNIQUE.getMessage().

Comment thread src/test/java/konkuk/thip/feed/domain/FeedTest.java
Comment on lines +42 to +50
// 삭제용 피드 태그 양방향 매핑 관계
@OneToMany(mappedBy = "feedJpaEntity", cascade = CascadeType.REMOVE, orphanRemoval = true)
private List<FeedTagJpaEntity> feedTags = new ArrayList<>();
@Column(name = "tag_list", columnDefinition = "TEXT")
@Convert(converter = TagListJsonConverter.class)
private TagList tagList = TagList.of(List.of());
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p3 : @buzz0331 님 ContentLIst는 nullable = false, TagList 는 nullable = true 로 설정하신 이유가 있을까요??
기획의도상으로는 피드는 content, tag 둘다 nullable 로 알고 있는데, jpa 제약 조건을 다르게 명시하신 이유가 궁금합니다!

Copy link
Copy Markdown
Contributor

@buzz0331 buzz0331 Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어 이거 저도 지금확인하고 리뷰 남기려고 왔는데 ㅋㅋㅋ 이미 남겨두셨네요! 현재 저희 상속전략인 single table 전략과 기획의도를 모두 충족하기 위해서 nullable = true로 통일 해야 할 것 같습니다.

Comment on lines 58 to 59
if(contentList != null) this.contentList = contentList;
this.contentList = contentList;
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@buzz0331 이 부분 중복된 코드같은데 if 절 아래의 this.contentList = contentList 는 삭제해도 될까요?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵넵 제가 잘못 작성해둔 것 같네요 ㅎ 제거해주시면 감사하겠습니다

Copy link
Copy Markdown
Contributor

@buzz0331 buzz0331 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

와우 정말 귀찮은 작업이셨을텐데 고생많으셨습니다!! 꼼꼼히 잘해주셨네요!! 최고십니다~~

Comment on lines +23 to +59
static {
// Alias -> Category
EnumMap<Alias, Category> a2c = new EnumMap<>(Alias.class);
a2c.put(Alias.WRITER, Category.LITERATURE); // 문학가
a2c.put(Alias.SCIENTIST, Category.SCIENCE_IT); // 과학자
a2c.put(Alias.SOCIOLOGIST, Category.SOCIAL_SCIENCE); // 사회학
a2c.put(Alias.ARTIST, Category.ART); // 예술가
a2c.put(Alias.PHILOSOPHER, Category.HUMANITY); // 철학자
aliasToCategory = Collections.unmodifiableMap(a2c);

// Category -> Tags
EnumMap<Category, List<Tag>> c2t = new EnumMap<>(Category.class);
c2t.put(Category.LITERATURE, List.of( // 문학
Tag.KOREAN_NOVEL, Tag.FOREIGN_NOVEL, Tag.CLASSIC_LITERATURE, Tag.ESSAY, Tag.POETRY,
Tag.PLAY, Tag.DETECTIVE_NOVEL, Tag.FANTASY_NOVEL, Tag.ROMANCE_NOVEL, Tag.LITERARY_THEORY
));
c2t.put(Category.SCIENCE_IT, List.of( // 과학·IT
Tag.GENERAL_SCIENCE, Tag.PHYSICS, Tag.CHEMISTRY, Tag.BIOLOGY, Tag.ASTRONOMY,
Tag.EARTH_SCIENCE, Tag.MATHEMATICS, Tag.GENERAL_ENGINEERING,
Tag.COMPUTER_ENGINEERING, Tag.PROGRAMMING, Tag.IT_GENERAL
));
c2t.put(Category.SOCIAL_SCIENCE, List.of( // 사회과학
Tag.SOCIOLOGY, Tag.LAW, Tag.GENERAL_POLITICS, Tag.POLITICAL_SCIENCE,
Tag.ECONOMICS, Tag.BUSINESS_ADMIN, Tag.JURISPRUDENCE, Tag.EDUCATION,
Tag.PSYCHOLOGY, Tag.MEDIA, Tag.INTERNATIONAL_RELATIONS, Tag.SOCIAL_ISSUES,
Tag.MARKETING, Tag.INVESTMENT, Tag.STARTUP, Tag.GENERAL_ECONOMY
));
c2t.put(Category.HUMANITY, List.of( // 인문학
Tag.PHILOSOPHY, Tag.HISTORY, Tag.RELIGION, Tag.CLASSICS, Tag.LINGUISTICS,
Tag.CULTURAL_ANTHROPOLOGY, Tag.HUMANISTIC_ESSAY, Tag.EASTERN_PHILOSOPHY,
Tag.WESTERN_PHILOSOPHY, Tag.WORLD_HISTORY, Tag.KOREAN_HISTORY,
Tag.HISTORICAL_ESSAY, Tag.ANCIENT_HISTORY, Tag.MODERN_HISTORY
));
c2t.put(Category.ART, List.of(
Tag.ART, Tag.MUSIC, Tag.MOVIE, Tag.PHOTO, Tag.DANCE,
Tag.THEATER, Tag.DESIGN, Tag.ARCHITECTURE, Tag.GENERAL_ART
));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

굿굿 확실히 이렇게 해두면 나중에 매핑 바꿔야 되는 요구사항이 들어올때 유연하게 대응 가능할 것 같네여

Comment thread src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java
Comment on lines +37 to +39
TagList.validateTags(Tag.fromList(command.tagList()));
// todo 나중에 presignedURL로 바꾸면 ContentList.of로 변경
ContentList.validateImageCount(images != null ? images.size() : 0);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어제 말했다시피 나중에 이미지 업로드를 presignedURL 기반으로 바꾸면 여기서 유효성 검증을 하지 않고 모두 Feed 내부에서 TagList와 ContentList를 생성하면서 유효성 검증 하는게 좋을 것 같습니다! Tag와 Content는 결국 Feed의 값 객체이니 생명 주기를 Feed 내부로 갖는게 더 도메인 주도적일 것 같습니다~ (그래서 todo로 남겨두었습니다 ㅎ)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 확인했습니다! 좋습니다!
이와 관련해서 기존 코드들의 create 방식을 현준님이 작성한 리뷰처럼 통일하는 것도 좋은 것 같습니다!

Comment on lines +16 to +21
private static final int DEFAULT_MAX_SIZE = 3;

private final List<String> contents;

private final int maxSize;

Copy link
Copy Markdown
Contributor

@buzz0331 buzz0331 Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코래 말처럼 maxSize를 사용하고 있지 않은데 이 부분은 현재 Feed 생성시에 이미지 업로드 전에 유효성 검증을 하기 위해서 static 메서드로 추가해둔건데 마찬가지로 이미지 업로드 방식을 바꾸면 maxSize가 사용 가능해질 것 같습니다!

Comment on lines +35 to +45
public static void validateTags(List<Tag> tags) {
if (tags.size() > MAX_SIZE) {
throw new InvalidStateException(INVALID_FEED_COMMAND,
new IllegalArgumentException("태그는 최대 " + MAX_SIZE + "개까지 입력할 수 있습니다."));
}
long distinctCount = tags.stream().distinct().count();
if (distinctCount != tags.size()) {
throw new InvalidStateException(INVALID_FEED_COMMAND,
new IllegalArgumentException(TAG_SHOULD_BE_UNIQUE.getMessage()));
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 구현하면서 조금 고민했던 부분인데요, 현재 내부에서 기존에 사용하던 INVALID_FEED_COMMAND 예외를 그대로 터뜨리는 게 맞을지에 대해 의문이 있습니다.
TagList는 피드 생성/수정 시에만 사용되는 게 아니라 다른 맥락에서도 활용될 수 있기 때문에, 도메인 관점에서 보면 INVALID_FEED_COMMAND보다는 TAG_COUNT_OVERFLOW 같은 전용 에러 코드를 정의하는 것이 더 적절하지 않을까 생각했습니다.

참고로, ContentList 내부에서도 피드와 직접적인 연관보다는 자체적인 제약 조건 위반 시 별도의 에러를 발생하도록 구현해 두었습니다.
이와 같은 맥락에서 TagList 또한 독립적인 하위 도메인 단위의 제약 조건 위반을 명확히 표현하는 쪽이 일관성 있고 의미가 명확할 것 같습니다.

두 분은 어떻게 생각하시나요? @seongjunnoh @hd0rable

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 TagList에서는 TAG_COUNT_OVERFLOW 처럼 도메인의 현재 동작에 맞는 에러를 던제는게 디버깅에도 용이하고, 유지보수성이 더 높은 코드라고 생각합니다!

수정하는게 좋을 것 같습니다!!

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a 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 (1)
src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java (1)

53-60: 생성자 null-coalescing 처리로 이전 NPE/nullable 이슈 해결됨

이전 커밋에서 지적된 contentList의 이중 대입 및 null 가드 무력화 문제가 해소되었습니다. TagList도 empty로 통일된 점 일관성 좋아요.

🧹 Nitpick comments (2)
src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java (2)

62-70: updateFrom에서도 null 대입 방지 일관성 유지 권장

생성자에서는 empty() 기본값으로 null을 방지하지만, updateFrom은 전달값을 그대로 대입합니다. feed.getContentList()/getTagList()가 null일 경우 TEXT 컬럼에 null이 저장되어 의미가 달라질 수 있습니다. 생성자와 동일하게 empty로 coalesce 해 일관성을 유지하는 것을 권장합니다.

적용 diff:

-        this.contentList = feed.getContentList();
-        this.tagList = feed.getTagList();
+        this.contentList = (feed.getContentList() != null) ? feed.getContentList() : ContentList.empty();
+        this.tagList = (feed.getTagList() != null) ? feed.getTagList() : TagList.empty();

48-50: nullable 명시로 스키마 의도 명확화

  • @columnnullable = true를 추가해 컬럼이 null 허용임을 명시하세요.

[선택 사항] JSONB+GIN 인덱스 전략 검토

  • 현재 레포 내 tag_list 기준 필터링/정렬 조회 로직이 없어 즉각적인 성능 문제는 없습니다.
  • 향후 태그 기반 검색/정렬 기능이 필요해질 경우 PostgreSQL jsonb 타입으로 변경 후 GIN 인덱스(jsonb_path_ops)를 도입하는 마이그레이션을 고려하세요.

적용 예시 diff:

-    @Column(name = "tag_list", columnDefinition = "TEXT")
+    @Column(name = "tag_list", nullable = true, columnDefinition = "TEXT")
     @Convert(converter = TagListJsonConverter.class)
     private TagList tagList = TagList.empty();
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between e71d741 and a79cf0d.

📒 Files selected for processing (1)
  • src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.371Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
🧬 Code Graph Analysis (1)
src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java (2)
src/main/java/konkuk/thip/feed/domain/TagList.java (1)
  • TagList (12-107)
src/main/java/konkuk/thip/feed/domain/value/ContentList.java (1)
  • ContentList (11-133)
🔇 Additional comments (2)
src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java (2)

7-11: JSON 컨버터/값 객체 import 추가 적절

도메인 값 객체(ContentList/TagList) + AttributeConverter 조합으로 의존성 경계가 명확해졌고, join 비용 제거 방향과 일치합니다. 이 변경은 유지보수성과 성능 관점에서 합리적입니다.


39-43: DB 벤더 확인 필요: PostgreSQL 여부 검증 요청

현재 코드베이스에서 spring.datasource.url, spring.jpa.database-platform, hibernate.dialect 설정을 찾지 못해 DB 벤더를 확정할 수 없습니다.

  • 실제 사용 중인 DB가 PostgreSQL인지 확인해주세요.
    • PostgreSQL 사용 시 적용 권장 사항:
      @Column(name = "content_list", nullable = true, columnDefinition = "jsonb") 로 변경
      • 태그/콘텐츠 검색 성능을 위해 GIN 인덱스 적용 검토
  • @Column(nullable = true) 명시로 스키마와 의도를 명확히 하는 것도 권장드립니다.

Comment on lines -58 to +59
if(contentList != null) this.contentList = contentList;
this.contentList = contentList;
this.tagList = tagList != null ? tagList : TagList.of(List.of());
this.contentList = contentList != null ? contentList : ContentList.empty();
this.tagList = tagList != null ? tagList : TagList.empty();
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

굳굳 확인했습니다!

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (7)
src/test/java/konkuk/thip/room/domain/RoomParticipantTest.java (1)

63-63: 부동소수점 비교는 허용 오차 기반으로 검증하는 편이 안전합니다

현재 케이스(예: 5/20, 1/20)는 이진 표현이 정확해 깨지지 않겠지만, 추후 값이 달라지면 정밀도 이슈로 테스트가 깨질 수 있습니다. AssertJ의 isCloseTo + within 사용을 권장합니다.

적용 예시:

-        assertThat(roomParticipant.getUserPercentage()).isEqualTo(ratio * 100);
+        assertThat(roomParticipant.getUserPercentage()).isCloseTo(ratio * 100, within(1e-9));

두 곳(Line 63, Line 85)에 동일하게 적용해 주세요.

추가 import:

import static org.assertj.core.api.Assertions.within;

Also applies to: 85-85

src/test/java/konkuk/thip/room/adapter/in/web/RoomParticipantDeleteApiTest.java (4)

66-74: 중복되는 셋업 로직(DRY) 제안

setUpWithOnlyHost/setUpWithParticipant가 alias/category/book 생성 로직을 반복합니다. 테스트 유지보수를 위해 공통 헬퍼로 합치는 것을 권장합니다.

예시(아이디어):

  • 공통 메서드: setUpRoom(boolean withParticipant, double hostPct, double memberPct)
  • 내부에서 alias/category/book 생성 후 createRoom(..., withParticipant ? 2 : 1) 호출
  • withParticipant=true인 경우에만 RoomParticipant 저장 및 참여도 세팅

76-88: memberCount를 직접 세팅하는 방식은 테스트 오차 가능성이 있습니다.

현재는 참가자 수(1/2)와 memberCount가 일치하지만, 셋업 변경 시 어긋날 수 있습니다. 참가자 저장 이후 실제 카운트를 기반으로 room의 memberCount를 갱신하는 패턴을 고려해 주세요(예: 저장 후 countByRoomId로 재계산).


90-106: TestEntityFactory를 활용해 사용자 생성 중복을 제거하세요.

현재 빌더 블록이 중복되고 기본속성(nicknameUpdatedAt 등)이 분산됩니다. Factory 메서드를 사용하면 테스트가 간결해지고 일관성이 높아집니다.

다음 변경을 권장합니다:

-        host = userJpaRepository.save(UserJpaEntity.builder()
-                .oauth2Id("kakao_432708231")
-                .nickname("user")
-                .nicknameUpdatedAt(LocalDate.now().minusMonths(7))
-                .role(UserRole.USER)
-                .alias(alias)
-                .build());
+        host = userJpaRepository.save(TestEntityFactory.createUser(alias, "user"));

-        participant = userJpaRepository.save(UserJpaEntity.builder()
-                .oauth2Id("kakao_12345678")
-                .nickname("user123")
-                .nicknameUpdatedAt(LocalDate.now().minusMonths(7))
-                .role(UserRole.USER)
-                .alias(alias)
-                .build());
+        participant = userJpaRepository.save(TestEntityFactory.createUser(alias, "user123"));

127-141: 검증 전 em.flush()/em.clear()로 1차 캐시 영향 제거 권장

테스트 메서드가 @transactional이므로 동일 트랜잭션의 1차 캐시에 의해 findById가 캐시된 엔티티를 재사용할 수 있습니다. 요청 수행 직후 flush/clear로 DB 상태를 확실히 반영한 뒤 검증하면 플래키니스가 줄어듭니다.

다음 코드를 요청 직후에 추가하는 것을 제안합니다:

                 .andExpect(status().isOk());
 
+        // 1차 캐시를 비워 컨트롤러 수행 결과를 확실히 반영
+        em.flush();
+        em.clear();
 
         // 방에서 참여자 정보가 사라졌는지 확인
         boolean exists = roomParticipantJpaRepository.existsByUserIdAndRoomId(participant.getUserId(), room.getRoomId());
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (2)

164-167: ContentList 에러코드 추가 OK — 주석 컨벤션/메시지 상수화 관련 소소한 정리 제안

  • 주석 컨벤션: 기존 섹션들과 동일하게 콜론 앞뒤 공백을 두면 일관성이 좋아집니다.
  • 메시지의 “최대 3개”는 비즈니스 상수로 보입니다. 향후 변경 가능성을 고려하면, ErrorCode 메시지에는 상수값을 박아두지 않고(또는 문구를 일반화) 응답 detail 필드 등에 동적으로 limit 값을 내려주는 방식을 검토해볼 만합니다. 현재 변경은 실용적으로 충분하며 필수는 아닙니다.

주석 공백 컨벤션 정렬(선택):

-    /**
-     * 165000: ContentList error
-     */
+    /**
+     * 165000 : ContentList error
+     */

162-163: 에러 코드 값(160009) 중복 없음 확인됨

  • 160009은 ErrorCode.java 내에서 유일하게 존재합니다.
  • TAG_SHOULD_BE_UNIQUE
    ErrorCode.java
    TagList.java (도메인 레벨 검증)
    FeedCreateControllerTest.java, FeedUpdateControllerTest.java (테스트)
    에서 참조되고 있어 사용처가 적절히 연결되어 있습니다.

선택적 네이밍 니트픽:
기존 패턴(TAG_NAME_NOT_MATCH, TAG_NOT_FOUND 등)과 톤을 맞추려면 상수명을 재검토해 보세요.
예시:

- TAG_SHOULD_BE_UNIQUE(HttpStatus.BAD_REQUEST, 160009, "피드에 등록된 태그는 중복될 수 없습니다."),
+ TAG_DUPLICATED_NOT_ALLOWED(HttpStatus.BAD_REQUEST, 160009, "피드에 등록된 태그는 중복될 수 없습니다."),

아키텍처/도메인 정책 제안:
중복 태그 처리 방식을 ‘에러 반환’(현재 구현)에서 ‘자동 제거’(예: LinkedHashSet)로 변경할지 제품 의사결정을 고려하시기 바랍니다.
자동 제거를 선택하면 순서 보장과 중복 제거를 동시에 달성할 수 있습니다.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between a79cf0d and da67129.

📒 Files selected for processing (5)
  • src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1 hunks)
  • src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java (2 hunks)
  • src/main/java/konkuk/thip/room/adapter/out/persistence/repository/RoomQueryRepositoryImpl.java (10 hunks)
  • src/test/java/konkuk/thip/room/adapter/in/web/RoomParticipantDeleteApiTest.java (4 hunks)
  • src/test/java/konkuk/thip/room/domain/RoomParticipantTest.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java
  • src/main/java/konkuk/thip/room/adapter/out/persistence/repository/RoomQueryRepositoryImpl.java
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.371Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
🧬 Code Graph Analysis (1)
src/test/java/konkuk/thip/room/adapter/in/web/RoomParticipantDeleteApiTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
  • TestEntityFactory (30-394)
🔇 Additional comments (3)
src/test/java/konkuk/thip/room/domain/RoomParticipantTest.java (1)

110-110: EOL 개행 추가로 포맷 정합성 개선 — OK

파일 끝 개행 추가로 POSIX 관례와 여러 도구(린터/포매터, Git diff)와의 호환성이 좋아집니다. 기능 변화는 없습니다.

src/test/java/konkuk/thip/room/adapter/in/web/RoomParticipantDeleteApiTest.java (2)

13-17: LGTM: Alias/Category를 도메인 Enum으로 전환한 import가 적절합니다.

JPA 엔티티 의존 제거 목적과 일치하며, 테스트 전반에서 enum 사용이 일관적입니다.


56-63: LGTM: 테스트 픽스처를 도메인 Enum 팩토리로 전환했습니다.

TestEntityFactory의 createLiteratureAlias/createLiteratureCategory 사용이 PR 방향성과 부합합니다.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (3)

402-420: SavedFeed 투영도 isPriorityFeed를 false로 명시해 null 삼중상태 제거

SavedFeed 목록에서는 우선순위 개념이 없으므로 false 고정이 더 자연스럽습니다. 또한 앞선 투영과 일관성 유지에 도움이 됩니다.

     private QFeedQueryDto toSavedFeedQueryDto() {
         return new QFeedQueryDto(
                 feed.postId,
                 feed.userJpaEntity.userId,
                 user.nickname,
                 user.alias,
                 feed.createdAt,
                 book.isbn,
                 book.title,
                 book.authorName,
                 feed.content,
                 feed.contentList,
                 feed.likeCount,
                 feed.commentCount,
                 feed.isPublic,
-                Expressions.nullExpression(),
+                Expressions.constant(Boolean.FALSE),
                 savedFeed.createdAt
         );
     }

62-71: 팔로잉 우선순위 조회에서 중복 방지 및 순서 보존 보강 제안

following 조인 특성상(데이터 정합성 보장 실패 시) 동일 feed.postId가 중복으로 나올 가능성을 배제할 수 없습니다. 이후 ID→엔티티 재조회 흐름은 entityMap으로 마지막 항목만 남기며, 결과적으로 ordered 목록에 null이 들어가거나 의도치 않은 누락/중복이 발생할 수 있습니다. ID 수집 시 LinkedHashSet으로 중복 제거 및 순서 보존을 권장합니다. (이전에 선호하신 방식)

-        List<Long> feedIds = tuples.stream()
-                .map(tuple -> tuple.get(0, Long.class))
-                .toList();
+        List<Long> feedIds = tuples.stream()
+                .map(tuple -> tuple.get(0, Long.class))
+                .collect(Collectors.toCollection(java.util.LinkedHashSet::new))
+                .stream()
+                .toList();

추가로, ID/우선순위를 한 번에 distinct 처리하려면 ID 서브쿼리 단계에서 .distinct()를 고려해도 됩니다. 다만 DB 카디널리티와 실행 계획에 따라 성능 차이가 있으므로 선택적으로 검토 바랍니다.


110-144: 우선순위 커서 페이지네이션 조건은 타당 — 단, 인덱스 최적화로 실성능 확보 필요

쿼리 형태상 다음 인덱스를 권장합니다.

  • feed: (status, is_public, created_at), (user_id, created_at), (book_id) 각 커버링 인덱스
  • following: unique(user_id, following_user_id) + (user_id) 단일 인덱스
  • saved_feed: (user_id, created_at), (feed_id)

실운영에서 size+1 패턴의 커서 페이징은 인덱스 적합성이 체감 성능을 좌우합니다. 현재 조건식과 orderBy(priority desc, created_at desc)를 고려해 우선 created_at 기반 정렬 인덱스를 점검하시길 권장합니다.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between da67129 and a78bed9.

📒 Files selected for processing (1)
  • src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.371Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
⏰ 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 (4)
src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java (4)

4-7: QueryDSL DSL 임포트 전환 적절 — 우선순위 계산 및 커서 조건 구성에 필요한 최소 임포트만 유지됨

JPAExpressions 제거하고 BooleanExpression/CaseBuilder/Expressions/NumberExpression로 치환한 방향성 좋습니다. 불필요한 와일드카드 임포트도 없어 가독성과 유지보수성이 좋아졌습니다.


12-15: 리팩터링 방향과 일치하는 Q 엔티티 사용으로 정합성 확보

Feed/Alias/Content 테이블 제거 후, FeedJpaEntity 중심으로의 질의에 맞춰 QFeedJpaEntity/QSavedFeedJpaEntity 임포트 구성이 정합적입니다.


329-347: QFeedQueryDto toQueryDto에서 isPriorityFeed 기본값 false 지정 및 일관성 유지

toQueryDto() 메서드에서 현재 isPriorityFeed와 savedCreatedAt을 Expressions.nullExpression()으로 채우고 있는데, 다른 DTO 변환 경로(toDto)에서는 isPriorityFeed를 명시적으로 false로 설정합니다. API 일관성을 위해 기본값을 false로 명시적으로 주입하는 것을 권장합니다.

수정 제안:

         return new QFeedQueryDto(
             feed.postId,
             feed.userJpaEntity.userId,
             user.nickname,
             user.alias,
             feed.createdAt,
             book.isbn,
             book.title,
             book.authorName,
             feed.content,
             feed.contentList,
             feed.likeCount,
             feed.commentCount,
             feed.isPublic,
-            Expressions.nullExpression(),      // isPriorityFeed
-            Expressions.nullExpression()       // savedCreatedAt
+            Expressions.constant(false),      // isPriorityFeed: 기본 false
+            Expressions.nullExpression()      // savedCreatedAt: 해당 없음
         );

검증 포인트:

  • FeedQueryDto(@QueryProjection) 생성자 마지막 두 파라미터가 Boolean(isPriorityFeed), LocalDateTime(savedCreatedAt)인지 확인
  • QFeedQueryDto 생성자도 해당 순서로 Expression, Expression을 받는지 확인
  • feed.contentList 필드 타입(List<…>)이 QFeedQueryDto 생성자 파라미터와 호환되는지 확인

246-266: Alias·ContentList null 안전성 보강 불필요; TODO 주석만 정리 필요

  • UserJpaEntity.alias는 1:1 매핑(nullable=false)로 NPE 발생 불가능

  • FeedJpaEntity.contentList는 빌더/필드 초기화 시 항상 ContentList.empty()로 설정되어 null 아님

  • 따라서 제안된 null 검사 코드는 제거해도 무방

  • // TODO : DB에 String alias 만 저장하면 수정해야함 주석만 최신 모델(enum 기반)과 맞게 제거 또는 수정

  • alias 파생값 계산 책임을 DTO 내부로 일원화 검토

Likely an incorrect or invalid review comment.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedRelatedWithBookApiTest.java (4)

13-13: 미사용 Category import/지역변수 정리 권장

도메인 Enum으로의 전환 자체는 적절합니다. 다만 본 테스트 클래스에서는 Category를 실제로 사용하지 않으므로 import와 지역변수를 제거해 경고를 없애는 편이 좋습니다.

적용 예시:

- import konkuk.thip.room.domain.Category;
-        Category category = TestEntityFactory.createLiteratureCategory();
-        Category category = TestEntityFactory.createLiteratureCategory();

Also applies to: 272-272, 314-314


299-309: 자기 글 포함 검증을 isWriter 플래그로 보강 제안

현재는 feedId 집합만 검증하고 있습니다. 동일 조건의 다른 이벤트가 개입되더라도, isWriter가 올바르게 매핑되는지만 추가로 확인하면 회귀 방지에 도움이 됩니다.

아래와 같이 보강 검증을 추가해 주세요:

         assertThat(feedIds.size()).isEqualTo(2);
         assertThat(feedIds).containsExactlyInAnyOrder(
                 myPublic.getPostId(),
                 othersPublic.getPostId()
         );
+
+        // isWriter 교차 검증: 내 공개 피드만 isWriter=true, 타인 피드는 isWriter=false
+        boolean foundMyWriterAssert = false;
+        boolean foundOthersWriterAssert = false;
+        for (JsonNode f : feeds) {
+            long id = f.path("feedId").asLong();
+            if (id == myPublic.getPostId()) {
+                assertThat(f.path("isWriter").asBoolean()).isTrue();
+                foundMyWriterAssert = true;
+            } else if (id == othersPublic.getPostId()) {
+                assertThat(f.path("isWriter").asBoolean()).isFalse();
+                foundOthersWriterAssert = true;
+            }
+        }
+        assertThat(foundMyWriterAssert).isTrue();
+        assertThat(foundOthersWriterAssert).isTrue();

329-346: 팩토리 오버로드로 likeCount 초기화해 엔티티 변이 최소화

updateLikeCount(...)로 사후 수정하기보다, 팩토리 오버로드에서 초기값을 지정하면 의도가 더 명확하고(불변 초기화), 루틴 호출도 줄어듭니다.

-        FeedJpaEntity publicFeed1 = TestEntityFactory.createFeed(
-                author,
-                book,
-                List.of("http://img/1.jpg", "http://img/2.jpg"),
-                true
-        );
-        publicFeed1.updateLikeCount(7); // 좋아요 정렬을 위해 수치 조정
+        FeedJpaEntity publicFeed1 = TestEntityFactory.createFeed(
+                author, book, true, 7, 0, List.of("http://img/1.jpg", "http://img/2.jpg")
+        );

-        FeedJpaEntity publicFeed2 = TestEntityFactory.createFeed(
-                author,
-                book,
-                List.of("http://img/3.jpg"),
-                true
-        );
-        publicFeed2.updateLikeCount(3);
+        FeedJpaEntity publicFeed2 = TestEntityFactory.createFeed(
+                author, book, true, 3, 0, List.of("http://img/3.jpg")
+        );

참고: TestEntityFactory에 (user, book, isPublic, likeCount, commentCount, imageUrls) 오버로드가 존재합니다.


368-375: 반복문 내 엔티티 변이 제거로 테스트 단순화

루프마다 updateLikeCount(...) 호출 대신 팩토리에서 likeCount를 초기화하면 테스트가 더 읽기 쉽고, 변이를 줄여 예측 가능성이 높아집니다.

-            FeedJpaEntity f = TestEntityFactory.createFeed(
-                    author,
-                    book,
-                    List.of("http://img/" + i + ".jpg"),
-                    true
-            );
-            f.updateLikeCount(count - i); // 다양한 likeCount 부여
+            FeedJpaEntity f = TestEntityFactory.createFeed(
+                    author, book, true, count - i, 0, List.of("http://img/" + i + ".jpg")
+            );
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between a78bed9 and 0ac1bd7.

📒 Files selected for processing (1)
  • src/test/java/konkuk/thip/feed/adapter/in/web/FeedRelatedWithBookApiTest.java (6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.371Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
🧬 Code Graph Analysis (1)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedRelatedWithBookApiTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
  • TestEntityFactory (30-394)
⏰ 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 (3)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedRelatedWithBookApiTest.java (3)

259-259: 테스트 의도 명시적인 DisplayName 변경 좋습니다

비공개 제외 + 자기 자신 공개 포함이라는 요구사항을 제목에서 분명히 드러내고 있어 가독성이 좋아졌습니다.


262-271: 도메인 타입 기반 셋업 전환 LGTM

  • Alias를 도메인 Enum으로 사용하고, UserJpaEntity 빌더에 .alias(...)를 직접 설정하는 방향 전환이 일관되고 단순합니다.
  • Feed 생성도 TestEntityFactory의 도메인 지향 팩토리를 사용해 의도가 분명합니다.

Also applies to: 277-279


313-315: 도메인 Alias 사용으로 테스트 셋업 간결화 LGTM

  • 문학/과학 별 Alias를 테스트 팩토리로 생성해 일관성이 좋아졌습니다.
  • UserJpaEntity 빌더에 .alias(alias) 적용도 변경된 도메인 모델과 일치합니다.

Also applies to: 321-321, 353-353, 360-360

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedRelatedWithBookApiTest.java (1)

248-251: 마지막 페이지 nextCursor를 문자열 "null"로 단언하는 것은 취약합니다

현재 마지막 페이지에서

  • JsonNode.asText()로 추출한 값을 "null" 문자열과 비교합니다.
  • 반면, 다른 테스트(책 미존재)에서는 nextCursor의 미존재를 검증하고 있습니다.

API가 nextCursor를 아예 누락하거나 JSON null로 반환하는 합리적인 구현과 호환되지 않습니다. 미존재 또는 null 모두 허용하도록 단언을 바꾸는 것이 안전합니다.

다음과 같이 수정해 주세요:

-            String thirdCursor = thirdRoot.path("data").path("nextCursor").asText();
-            assertThat(thirdCursor).isEqualTo("null"); // 다음 커서가 없어야 함
+            // 다음 커서는 미존재 또는 null이어야 함
+            assertThat(thirdRoot.path("data").hasNonNull("nextCursor")).isFalse();
🧹 Nitpick comments (3)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedRelatedWithBookApiTest.java (3)

272-272: 미사용 변수 제거 제안: Category

해당 메서드에서는 Category를 사용하지 않습니다. 불필요한 준비 코드는 제거하는 편이 좋습니다.

다음 변경을 권장합니다:

-        Category category = TestEntityFactory.createLiteratureCategory();

299-309: 검증 보강 제안: isWriter 매핑도 명시적으로 확인하세요

현재 공개/비공개 및 자기/타인 필터는 feedId로만 검증됩니다. isWriter의 true/false도 함께 단언하면 회귀를 더 잘 잡을 수 있습니다.

아래와 같이 보강을 제안합니다(기존 검증 바로 아래 추가):

         List<Long> feedIds = new ArrayList<>();
         for (JsonNode f : feeds) {
             feedIds.add(f.path("feedId").asLong());
         }
 
+        // isWriter 매핑 검증: 내 공개 피드는 true, 타인의 공개 피드는 false
+        boolean writerTrueFound = false;
+        for (JsonNode f : feeds) {
+            long id = f.path("feedId").asLong();
+            if (id == myPublic.getPostId()) {
+                assertThat(f.path("isWriter").asBoolean()).isTrue();
+                writerTrueFound = true;
+            }
+            if (id == othersPublic.getPostId()) {
+                assertThat(f.path("isWriter").asBoolean()).isFalse();
+            }
+        }
+        assertThat(writerTrueFound).isTrue();

313-314: 미사용 변수 제거 제안: Category

prepareDataForFeeds에서도 Category 변수가 사용되지 않습니다. 정리해 주세요.

-        Category category = TestEntityFactory.createLiteratureCategory();
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 0ac1bd7 and 6ec8adc.

📒 Files selected for processing (1)
  • src/test/java/konkuk/thip/feed/adapter/in/web/FeedRelatedWithBookApiTest.java (6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.371Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
🧬 Code Graph Analysis (1)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedRelatedWithBookApiTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
  • TestEntityFactory (30-394)
⏰ 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 (8)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedRelatedWithBookApiTest.java (8)

13-13: 도메인 Enum 의존으로의 전환 LGTM

JPA 엔티티 기반에서 도메인 enum(Alias/Category)으로의 import 전환이 테스트 의도와 일치합니다.

Also applies to: 17-17


30-30: ArrayList 도입 LGTM

아래 검증 코드(피드 ID 수집)에 맞춰 합리적인 선택입니다.


262-262: 요청자 생성 시 도메인 Alias 사용 전환 LGTM

UserJpaEntity.alias에 도메인 Alias를 직접 주입하도록 변경된 부분이 PR 방향과 일치합니다.

Also applies to: 269-269


277-279: 도메인 팩토리 기반 피드 생성 LGTM

공개/비공개 피드 생성이 간결해졌고 PR 취지(도메인 전환)에 부합합니다.


321-321: 요청자 생성 시 도메인 Alias 사용 전환 LGTM

반영 적절합니다.


329-329: 이미지 포함 피드 생성 호출 LGTM

ContentList/DTO 매핑을 검증하는 기반 데이터로 적절합니다.

Also applies to: 337-337


353-353: 과학 도메인 Alias 사용 LGTM

다양한 시나리오 데이터 구성에 적절합니다.

Also applies to: 360-360


368-368: 루프 내 피드 생성 LGTM

likeCount 역순 부여로 정렬/페이징 검증에 유용합니다.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)
src/main/java/konkuk/thip/user/domain/User.java (1)

53-60: 닉네임 null 입력 시 NPE 가능성 — 방어 로직 추가 권장

updateUserInfo에서 isNicknameUpdateRequest가 true인데 nickname이 null인 경우, validateCanUpdateNickname 내부 nickname.isBlank()에서 NPE가 발생합니다. 유효성 예외(ErrorCode.USER_NICKNAME_CANNOT_BE_BLANK)로 정규화해 주는 편이 안전합니다. 또한 LocalDate.now()는 여러 번 호출하지 말고 지역 변수로 한 번만 평가하는 것이 좋습니다.

아래와 같이 보완을 제안합니다.

-    private void validateCanUpdateNickname(String nickname) {
-        if(nickname.isBlank()) { // 빈칸 불가
+    private void validateCanUpdateNickname(String nickname) {
+        if (nickname == null || nickname.isBlank()) { // null/빈칸 불가
             throw new InvalidStateException(ErrorCode.USER_NICKNAME_CANNOT_BE_BLANK);
         }
         if(nickname.length() > 10) { // 10자 이상 불가
             throw new InvalidStateException(ErrorCode.USER_NICKNAME_TOO_LONG);
         }
         // 닉네임을 변경한지 6개월이 지나지 않았으면 닉네임 업데이트 불가
-        if(nicknameUpdatedAt != null && nicknameUpdatedAt.isAfter(LocalDate.now().minusMonths(6))) {
+        LocalDate today = LocalDate.now();
+        if (nicknameUpdatedAt != null && nicknameUpdatedAt.isAfter(today.minusMonths(6))) {
             throw new InvalidStateException(ErrorCode.USER_NICKNAME_UPDATE_TOO_FREQUENT);
         }
         if(nickname.equals(this.nickname)) { // 현재 닉네임과 같으면 업데이트 불가
             throw new InvalidStateException(ErrorCode.USER_NICKNAME_CANNOT_BE_SAME);
         }
     }

추가로, followerCountInteger이므로 매핑/복원 시 null일 수 있습니다. 도메인 불변식을 강화하려면:

  • 필드 타입을 int로 변경하거나,
  • 생성 시 0 보장(@builder 기본값 or 생성자 검증),
  • 증가/감소 메서드에 null 방어 로직

중 하나를 적용하는 것을 권장합니다.

Also applies to: 62-76

src/main/java/konkuk/thip/room/application/service/RoomSearchService.java (1)

53-71: 정렬 분기에서 미처리(default) 케이스 시 null 반환 위험 — 명시적 예외로 변경 권장

현재 switch에서 열거값이 추가되면 result가 null로 반환될 수 있습니다. 명시적으로 예외를 던져 문제를 조기에 드러내는 편이 안전합니다. 동시에 불필요한 result 변수를 제거할 수 있습니다.

-    private CursorBasedList<RoomQueryDto> executeRecruitingRoomSearch(RoomSearchQuery query, Category category, RoomSearchSortParam sortParam, Cursor cursor) {
-        CursorBasedList<RoomQueryDto> result = null;
+    private CursorBasedList<RoomQueryDto> executeRecruitingRoomSearch(
+            RoomSearchQuery query, Category category, RoomSearchSortParam sortParam, Cursor cursor) {
         if (category == null) {
             switch (sortParam) {
                 case DEADLINE:
                     return roomQueryPort.searchRecruitingRoomsByDeadline(query.keyword(), cursor);
                 case MEMBER_COUNT:
                     return roomQueryPort.searchRecruitingRoomsByMemberCount(query.keyword(), cursor);
+                default:
+                    throw new IllegalArgumentException("지원하지 않는 정렬 조건입니다: " + sortParam);
             }
         } else {
             switch (sortParam) {
                 case DEADLINE:
                     return roomQueryPort.searchRecruitingRoomsWithCategoryByDeadline(query.keyword(), category, cursor);
                 case MEMBER_COUNT:
                     return roomQueryPort.searchRecruitingRoomsWithCategoryByMemberCount(query.keyword(), category, cursor);
+                default:
+                    throw new IllegalArgumentException("지원하지 않는 정렬 조건입니다: " + sortParam);
             }
         }
-        return result;
+        // 도달 불가
     }
src/main/java/konkuk/thip/feed/domain/value/Tag.java (1)

87-95: null/공백 입력 처리 부재로 인한 NPE 가능성 — 방어 로직 필수

from(String value)에서 value가 null이면 equalsIgnoreCase 호출 시 NPE가 발생합니다. 클라이언트 파라미터 특성상 공백/누락이 들어올 수 있으므로 방어 로직이 필요합니다. 또한 불필요한 오동작을 막기 위해 trim 처리도 권장합니다.

-    public static Tag from(String value) {
-        for (Tag tagName : Tag.values()) {
-            if (tagName.value.equalsIgnoreCase(value)) {
-                return tagName;
-            }
-        }
-        throw new InvalidStateException(TAG_NAME_NOT_MATCH);
-    }
+    public static Tag from(String value) {
+        if (value == null || value.isBlank()) {
+            throw new InvalidStateException(TAG_NAME_NOT_MATCH);
+        }
+        String normalized = value.trim();
+        for (Tag tagName : Tag.values()) {
+            if (tagName.value.equalsIgnoreCase(normalized)) {
+                return tagName;
+            }
+        }
+        throw new InvalidStateException(TAG_NAME_NOT_MATCH);
+    }
src/main/java/konkuk/thip/feed/domain/Feed.java (1)

118-124: Boolean 필드 NPE 위험 — null-safe 비교로 접근 제어 강화 필요

isPublic가 박싱 타입(Boolean)이라 null일 경우 !isPublic에서 NPE가 발생, 500으로 튈 수 있습니다. 접근 제어 로직이므로 방어 코드를 권장합니다.

적용 제안(diff):

-    private void validatePrivateAccessPermission(Long userId, String action) {
-        if (!this.isPublic && !this.creatorId.equals(userId)) {
+    private void validatePrivateAccessPermission(Long userId, String action) {
+        if (Boolean.FALSE.equals(this.isPublic) && !Objects.equals(this.creatorId, userId)) {
             throw new InvalidStateException(FEED_ACCESS_FORBIDDEN,
                     new IllegalArgumentException(String.format("비공개 글은 작성자만 %s 있습니다.", action)));
         }
     }
-    public void validateViewPermission(Long userId) {
-        if (!isPublic && !creatorId.equals(userId)) {
+    public void validateViewPermission(Long userId) {
+        if (Boolean.FALSE.equals(isPublic) && !Objects.equals(creatorId, userId)) {
             throw new BusinessException(FEED_CAN_NOT_SHOW_PRIVATE_ONE);
         }
     }

Also applies to: 196-200

src/main/java/konkuk/thip/room/adapter/out/persistence/repository/RoomQueryRepositoryImpl.java (1)

175-189: 추천 방 조회에서 ACTIVE 상태 필터가 누락되었습니다

다른 공개 목록성 조회들과 달리 Line 181~185에는 room.status.eq(StatusType.ACTIVE) 조건이 없습니다. 비활성 방이 섞일 수 있습니다. 아래처럼 보강 제안합니다.

                 .where(
                         room.category.eq(category)
                                 .and(room.startDate.after(LocalDate.now()))     // 모집 마감 시각 > 현재 시각
                                 .and(room.roomId.ne(roomId))// 현재 방 제외
-                                .and(room.isPublic.isTrue()) // 공개방 만
+                                .and(room.isPublic.isTrue()) // 공개방 만
+                                .and(room.status.eq(StatusType.ACTIVE))
                 )

추가로 LocalDate.now()를 여러 곳에서 직접 호출하기보다, 메서드 시작에서 today를 한 번 계산해 재사용하면 테스트 안정성과 일관성이 높아집니다.

Also applies to: 195-195

♻️ Duplicate comments (9)
src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java (1)

37-40: 태그/이미지 유효성 검증 위치와 중복 계산 최소화 제안

  • 현재 TagList.validateTags(Tag.fromList(...))는 변환과 검증을 한 줄에서 수행합니다. 변환 결과를 지역 변수로 보관하면 이후 재계산을 줄일 수 있습니다.
  • 더 나아가 피드의 불변식(태그/콘텐츠 개수·중복)은 애그리거트 루트(Feed) 내부에서 보장하는 편이 DDD에 부합합니다. 과거 코멘트(TODO)와도 일치합니다.
-        TagList.validateTags(Tag.fromList(command.tagList()));
+        List<Tag> tags = Tag.fromList(command.tagList());
+        TagList.validateTags(tags);
         // todo 나중에 presignedURL로 바꾸면 ContentList.of로 변경
         ContentList.validateImageCount(images != null ? images.size() : 0);

가능하다면 Feed.withoutId(...) 시그니처를 TagList/ContentList를 직접 받도록 확장하고, 생성자 내부에서 유효성 검증을 일원화하는 방향을 추천합니다. 원하시면 단계적 마이그레이션 패치를 제안드릴 수 있습니다.

src/main/java/konkuk/thip/feed/application/service/FeedUpdateService.java (1)

29-30: TagList 검증 null 가드 누락으로 NPE 가능 (이전 코멘트와 동일 이슈)

command.tagList()가 null이면 TagList.validateTags(Tag.fromList(...)) 호출 시 NPE가 발생합니다. 아래와 같이 null 가드를 추가해 주세요. 검증 중복(서비스 vs 도메인)도 함께 정리 권장합니다.

-        TagList.validateTags(Tag.fromList(command.tagList()));
+        if (command.tagList() != null) {
+            TagList.validateTags(Tag.fromList(command.tagList()));
+        }
src/test/java/konkuk/thip/feed/domain/FeedTest.java (1)

66-88: TagList/ContentList 제약 테스트 복구 권장 (주석 제거 후 값 객체 기준으로 재작성)

핵심 도메인 제약(태그 최대/중복, 이미지 최대)에 대한 회귀 방지가 필요합니다. 새 값 객체 API(TagList/ContentList)에 맞춰 테스트를 복구해 주세요.

아래 예시를 파일 하단에 추가하는 것을 제안합니다(주석 블록 삭제):

@Test
@DisplayName("TagList: 태그가 5개 초과 시 InvalidStateException이 발생한다.")
void tagList_exceedsMax_throws() {
    assertThrows(InvalidStateException.class, () ->
            TagList.of(List.of(
                    KOREAN_NOVEL, KOREAN_NOVEL, KOREAN_NOVEL,
                    KOREAN_NOVEL, KOREAN_NOVEL, KOREAN_NOVEL // 6개
            ))
    );
}

@Test
@DisplayName("TagList: 중복 태그 있을 경우 InvalidStateException이 발생한다.")
void tagList_withDuplicates_throws() {
    assertThrows(InvalidStateException.class,
            () -> TagList.of(List.of(KOREAN_NOVEL, KOREAN_NOVEL)));
}

@Test
@DisplayName("ContentList: 이미지가 3개 초과 시 InvalidStateException(CONTENT_LIST_SIZE_OVERFLOW)이 발생한다.")
void contentList_exceedsMax_throws() {
    assertThrows(InvalidStateException.class,
            () -> ContentList.of(List.of("u1", "u2", "u3", "u4")));
}

원하시면 제가 PR에 테스트 추가 커밋을 준비하겠습니다.

src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateControllerTest.java (2)

72-81: 검증 헬퍼가 고정 feedId/userId를 사용 — 유효성 검증 전에 404/403으로 단락될 수 있음

태그·이미지 유효성 검증을 의도대로 타려면 실제 저장된 feedId/작성자 userId를 사용해야 합니다. 헬퍼 시그니처를 변경해 주세요.

적용 제안(diff):

-    private void assertBadRequest(int expectedCode, Map<String, Object> request, String message) throws Exception {
-        mockMvc.perform(patch("/feeds/1")
-                                .requestAttr("userId", 100L)
+    private void assertBadRequest(Long feedId, Long userId, int expectedCode, Map<String, Object> request, String message) throws Exception {
+        mockMvc.perform(patch("/feeds/" + feedId)
+                                .requestAttr("userId", userId)
                                 .contentType(MediaType.APPLICATION_JSON)
                                 .content(objectMapper.writeValueAsBytes(request))
                 )
                 .andExpect(status().isBadRequest())
                 .andExpect(jsonPath("$.code").value(expectedCode))
                 .andExpect(jsonPath("$.message", containsString(message)));
     }

141-144: 헬퍼 시그니처 변경에 맞춰 호출부 업데이트 필요

태그 개수 초과/중복 케이스 모두 실제 savedFeedId/creatorUserId를 전달하세요. 그래야 권한/존재성 체크에 막히지 않습니다.

적용 제안(diff):

-            req.put("tagList", List.of(
-                    PHYSICS.getValue(), CHEMISTRY.getValue(), KOREAN_NOVEL.getValue(),
-                    FOREIGN_NOVEL.getValue(), CLASSIC_LITERATURE.getValue(), HISTORY.getValue()
-            ));
-            assertBadRequest(TAG_LIST_SIZE_OVERFLOW.getCode(), req, TAG_LIST_SIZE_OVERFLOW.getMessage());
+            req.put("tagList", List.of(
+                    PHYSICS.getValue(), CHEMISTRY.getValue(), KOREAN_NOVEL.getValue(),
+                    FOREIGN_NOVEL.getValue(), CLASSIC_LITERATURE.getValue(), HISTORY.getValue()
+            ));
+            assertBadRequest(savedFeedId, creatorUserId, TAG_LIST_SIZE_OVERFLOW.getCode(), req, TAG_LIST_SIZE_OVERFLOW.getMessage());
-            req.put("tagList", List.of(KOREAN_NOVEL.getValue(), KOREAN_NOVEL.getValue()));
-            assertBadRequest(TAG_SHOULD_BE_UNIQUE.getCode(), req, TAG_SHOULD_BE_UNIQUE.getMessage());
+            req.put("tagList", List.of(KOREAN_NOVEL.getValue(), KOREAN_NOVEL.getValue()));
+            assertBadRequest(savedFeedId, creatorUserId, TAG_SHOULD_BE_UNIQUE.getCode(), req, TAG_SHOULD_BE_UNIQUE.getMessage());

Also applies to: 150-152

src/main/java/konkuk/thip/feed/domain/Feed.java (1)

162-169: 이미지 업데이트가 ‘신규 추가’를 금지 — 의도 확인 및 네이밍/로직 재정렬 제안

현재 contentList.validateOwnImages(newImageUrls)로 “신규 URL 추가”가 불가합니다. 업데이트에서 신규 추가를 허용해야 한다면 로직 조정이 필요합니다. 반대로 “보유한 것만 유지(삭제 전용)”이 의도라면 메서드명을 명확히 하는 편이 좋습니다.

옵션 A(신규 추가 허용):

     public void updateImages(Long userId, List<String> newImageUrls) {
         validateCreator(userId);
-//        validateImageCount(newImageUrls.size());
-//        validateOwnsImages(newImageUrls);
-        contentList.validateOwnImages(newImageUrls);
-
         this.contentList = convertToContentList(newImageUrls);
     }

옵션 B(삭제 전용 의미를 명확히):

  • 메서드명 retainImages(or updateImagesByRetention)로 변경
  • 삭제 플로우에서만 validateOwnImages 호출, 일반 업데이트는 A안처럼 처리
src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateApiTest.java (2)

1-27: EntityManager 주입 누락 — import/필드 추가 필요

em.clear()를 호출하려면 EntityManager를 주입해야 합니다.

적용 제안(diff):

 import org.springframework.transaction.annotation.Transactional;
+import jakarta.persistence.EntityManager;
     @Autowired private FeedJpaRepository feedJpaRepository;
+    @Autowired private EntityManager em;

Also applies to: 45-57


89-93: MockMvc 요청 후 1차 캐시로 인한 미반영 문제 가능 — em.clear()로 정합성 보장

테스트가 @transactional이므로 기존 영속성 컨텍스트가 캐시한 엔티티를 그대로 반환할 수 있습니다. 검증 전에 EntityManager를 clear하세요.

적용 제안(diff):

         // then
-        result.andExpect(status().isOk());
+        result.andExpect(status().isOk());
+        em.clear(); // 테스트 영속성 컨텍스트 초기화
         long tagCount = feedJpaRepository.findById(feedId).orElseThrow().getTagList().size();
         // then
-        result.andExpect(status().isOk());
+        result.andExpect(status().isOk());
+        em.clear();
         FeedJpaEntity updatedFeed = feedJpaRepository.findById(feedId).orElseThrow();
         // then
-        result.andExpect(status().isOk());
+        result.andExpect(status().isOk());
+        em.clear();
 
         FeedJpaEntity updated = feedJpaRepository.findById(feedId).orElseThrow();

추가로, 클래스 상단에 EntityManager 주입이 필요합니다(아래 별도 코드 참조).

Also applies to: 121-126, 156-170

src/main/java/konkuk/thip/room/adapter/out/persistence/repository/RoomQueryRepositoryImpl.java (1)

421-427: userId null-safe 보강 제안(중복 코멘트 참고)

home용 추천에서 비로그인 케이스가 있다면 null-safe 처리가 필수입니다. 위 동일 제안 diff 참고 바랍니다.

Comment on lines +40 to 42
.tagList(feedJpaEntity.getTagList())
.contentList(feedJpaEntity.getContentList() != null ? feedJpaEntity.getContentList() : ContentList.empty())
.createdAt(feedJpaEntity.getCreatedAt())
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

toDomainEntity에서 tagList null 처리 누락 — NPE 가능성 있음

  • contentList는 null-safe 처리했지만 tagList는 그대로 주입합니다. 마이그레이션/이관 데이터에 따라 null이 들어올 수 있어 NPE 위험이 있습니다. contentList와 동일하게 기본값을 할당하세요.
-                .tagList(feedJpaEntity.getTagList())
+                .tagList(feedJpaEntity.getTagList() != null ? feedJpaEntity.getTagList() : TagList.empty())
                 .contentList(feedJpaEntity.getContentList() != null ? feedJpaEntity.getContentList() : ContentList.empty())
📝 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.

Suggested change
.tagList(feedJpaEntity.getTagList())
.contentList(feedJpaEntity.getContentList() != null ? feedJpaEntity.getContentList() : ContentList.empty())
.createdAt(feedJpaEntity.getCreatedAt())
.tagList(feedJpaEntity.getTagList() != null
? feedJpaEntity.getTagList()
: TagList.empty())
.contentList(feedJpaEntity.getContentList() != null
? feedJpaEntity.getContentList()
: ContentList.empty())
.createdAt(feedJpaEntity.getCreatedAt())
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/feed/adapter/out/mapper/FeedMapper.java around
lines 40 to 42, the mapper sets tagList directly from feedJpaEntity which can be
null and cause an NPE; change the mapping to perform a null-safe assignment
(same approach as contentList) by substituting a default empty tag list when
feedJpaEntity.getTagList() is null so the domain entity never receives null.

Comment on lines +52 to +55
public static List<Tag> createTagsFromCategory(Category category) {
return EnumMappings.tagsFrom(category);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

카테고리 → 태그 생성 시 TagList 최대 5개 제약과 충돌 위험

현재 구현은 해당 카테고리의 모든 태그를 반환합니다. 이후 TagList.of(tags)로 넘기면 MAX_SIZE(5) 초과로 InvalidStateException이 발생할 수 있습니다. 기본적으로 5개 이하로 잘라 반환하도록 수정하거나, limit 파라미터가 있는 오버로드를 추가하세요.

최소 수정안:

-    public static List<Tag> createTagsFromCategory(Category category) {
-        return EnumMappings.tagsFrom(category);
-    }
+    public static List<Tag> createTagsFromCategory(Category category) {
+        List<Tag> all = EnumMappings.tagsFrom(category);
+        return all.subList(0, Math.min(5, all.size())); // TagList 최대 크기 고려
+    }

선택사항(참고용, 범위 외 코드 스니펫): limit 오버로드 추가

public static List<Tag> createTagsFromCategory(Category category, int limit) {
    List<Tag> all = EnumMappings.tagsFrom(category);
    return all.subList(0, Math.min(limit, all.size()));
}
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/common/util/TestEntityFactory.java around lines 52
to 55, createTagsFromCategory currently returns all tags for a category which
can exceed TagList.MAX_SIZE(5) and trigger InvalidStateException; modify the
method to return at most five tags (trim the list to Math.min(5, size)) or add
an overloaded method with a limit parameter that returns a subList(0,
Math.min(limit, size)) so callers never receive more than the allowed number of
tags.

import konkuk.thip.room.adapter.out.persistence.repository.category.CategoryJpaRepository;
import konkuk.thip.room.adapter.out.persistence.repository.roomparticipant.RoomParticipantJpaRepository;
import konkuk.thip.user.adapter.out.jpa.AliasJpaEntity;
import konkuk.thip.room.domain.value.Category;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

레거시 JPA 엔티티/빌더 흔적 점검 자동화 제안

리포지토리 전반에서 Alias/Category/Tag의 JPA 의존을 제거하는 큰 리팩터였으므로, 잔존 레퍼런스가 없는지 한 번에 확인해보면 안전합니다.

다음 스크립트로 잔존 참조를 탐지할 수 있습니다:

스크립트 결과가 비어야 정상입니다. 필요 시 제가 후속 커밋 정리를 도와드릴게요.

Also applies to: 16-16


🏁 Script executed:

#!/bin/bash
# 리팩터 후 남아있으면 안 되는 식별자/메서드 검색
set -e

echo "== Searching legacy JPA entities =="
rg -nP -C2 '\b(AliasJpaEntity|CategoryJpaEntity|TagJpaEntity)\b'

echo "== Searching legacy builder methods =="
rg -nP -C2 '(aliasForUserJpaEntity\s*\(|categoryJpaEntity\s*\()'

echo "== Searching for removed repos/usages (if existed) =="
rg -nP -C2 '(AliasJpaRepository|CategoryJpaRepository|TagJpaRepository)'

Length of output: 1086


레거시 JPA 참조(주석) 제거 요청

리포지토리 전반 JPA 의존 제거 후 제공해주신 스크립트 점검 결과, 다음 테스트 파일에서 주석 처리된 레거시 JPA 엔티티 참조가 발견되었습니다. 코드 일관성을 위해 해당 주석도 삭제 부탁드립니다.

  • src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSavedListApiTest.java:232–234 (// AliasJpaEntity alias = …, // UserJpaEntity me = …, // BookJpaEntity book = …)
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowSavedListApiTest.java
around lines 232 to 234, remove the commented legacy JPA entity references (the
lines starting with "//        AliasJpaEntity alias = …", "//       
UserJpaEntity me = …", "//        BookJpaEntity book = …"); simply delete those
commented lines to keep the codebase JPA-free and consistent, then run the test
suite to ensure no remaining references or compilation issues.

.andExpect(jsonPath("$.code").value(ErrorCode.ROOM_HOST_CANNOT_LEAVE.getCode()));
}
} No newline at end of file
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

MockMvc + @transactional 조합으로 인한 1차 캐시(영속성 컨텍스트) 정합성 문제 가능성

본 테스트 클래스는 @transactional이 적용되어 있습니다(라인 37). MockMvc는 별도 트랜잭션에서 동작하므로, 이후 동일 트랜잭션/영속성 컨텍스트에서 조회(findById 등) 시 DB의 최신 상태가 아닌 1차 캐시의 값을 반환할 수 있습니다. 평균 진행도/인원수 검증 시 간헐적 실패나 비결정적 동작의 원인이 됩니다.

해결 방안(택1 또는 병행):

  • 테스트 클래스에서 @transactional 제거(권장), 혹은 Propagation.NOT_SUPPORTED로 전환.
  • 각 MockMvc 호출 직후 조회하기 전에 em.clear()(또는 em.flush(); em.clear();) 호출로 1차 캐시를 비웁니다.

예시(해당 테스트 메서드들에 적용):

// leaveRoom_success
mockMvc.perform(delete("/rooms/" + room.getRoomId() + "/leave")
        .requestAttr("userId", participant.getUserId())
        .contentType(MediaType.APPLICATION_JSON))
    .andExpect(status().isOk());

// 최신 상태 보장
em.clear();

// 이후 조회/검증
boolean exists = roomParticipantJpaRepository.existsByUserIdAndRoomId(...);
RoomJpaEntity updateRoom = roomJpaRepository.findById(...).orElseThrow();

필요하시면 제가 @transactional 제거 vs clear() 적용 두 방식으로 수정 PR 패치도 바로 드리겠습니다.

🤖 Prompt for AI Agents
In
src/test/java/konkuk/thip/room/adapter/in/web/RoomParticipantDeleteApiTest.java
around line 168 (class annotated with @Transactional at line 37), the MockMvc
calls run in a separate transaction causing stale 1st-level cache reads in
subsequent repository lookups; either remove the class-level @Transactional (or
set Propagation.NOT_SUPPORTED) to let each request commit normally, or after
each mockMvc.perform(...) call invoke the EntityManager to clear the persistence
context (em.clear() or em.flush(); em.clear()) before any
repository.findById()/existsBy... assertions so you read the DB’s latest state.
Ensure an EntityManager is available in the test (inject/autowire) if you choose
the clear() approach.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (8)
src/main/java/konkuk/thip/room/domain/value/Category.java (1)

35-45: Category.from(String) null/blank 입력 시 NPE → 도메인 예외로 일관되게 변환 권장

현재 value.trim()에서 NPE가 발생할 수 있습니다. 아래처럼 null/blank를 사전에 검증하면 API 일관성이 높아지고 500 계열을 방지할 수 있습니다.

-    public static Category from(String value) {
-        return Arrays.stream(Category.values())
-                .filter(categoryName -> categoryName.getValue().equals(value.trim()))
+    public static Category from(String value) {
+        if (value == null || value.isBlank()) {
+            throw new InvalidStateException(CATEGORY_NOT_MATCH,
+                    new IllegalArgumentException("존재하지 않는 카테고리입니다. value: " + value));
+        }
+        final String normalized = value.trim();
+        return Arrays.stream(Category.values())
+                .filter(categoryName -> categoryName.getValue().equals(normalized))
                 .findFirst()
                 .orElseThrow(
                         () -> new InvalidStateException(CATEGORY_NOT_MATCH,
                                 new IllegalArgumentException(
                                         String.format("존재하지 않는 카테고리입니다. value: %s", value)
                                 ))
                 );
     }
src/main/java/konkuk/thip/feed/domain/value/Tag.java (1)

87-94: null/blank 입력 시 NPE 가능 + 선형 탐색 비용: from()/fromList() 방어 및 성능 보강 필요

  • 현재 from(null) 호출 시 equalsIgnoreCase에서 NPE 발생 여지가 있습니다. fromList도 내부에서 from을 호출하므로 리스트에 null이 포함되면 NPE가 납니다.
  • from이 매번 Tag.values()를 순회하므로, 다수 호출 시 비용이 누적됩니다. 정규화된 문자열 -> Tag 매핑을 캐시하면 O(1)로 줄일 수 있습니다.

아래 패치를 적용하면

  • null/blank 입력 방어
  • 공백(trailing/leading) 허용
  • 영문 부분(예: "IT") 대소문자 무시
  • 해시맵 기반 O(1) 조회
    를 달성할 수 있습니다.
@@
-import java.util.ArrayList;
-import java.util.List;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
@@
     private final String value;
 
+    // value(표시 문자열) -> enum 상수 매핑 캐시 (소문자/ROOT 로케일 기준)
+    private static final Map<String, Tag> LOOKUP_BY_VALUE;
+    static {
+        Map<String, Tag> map = new HashMap<>();
+        for (Tag tag : Tag.values()) {
+            map.put(tag.value.toLowerCase(Locale.ROOT), tag);
+        }
+        LOOKUP_BY_VALUE = Collections.unmodifiableMap(map);
+    }
+
     public static Tag from(String value) {
-        for (Tag tagName : Tag.values()) {
-            if (tagName.value.equalsIgnoreCase(value)) {
-                return tagName;
-            }
-        }
-        throw new InvalidStateException(TAG_NAME_NOT_MATCH);
+        if (value == null || value.isBlank()) {
+            throw new InvalidStateException(TAG_NAME_NOT_MATCH);
+        }
+        Tag matched = LOOKUP_BY_VALUE.get(value.trim().toLowerCase(Locale.ROOT));
+        if (matched != null) {
+            return matched;
+        }
+        throw new InvalidStateException(TAG_NAME_NOT_MATCH);
     }
@@
     public static List<Tag> fromList(List<String> values) {
         if (values == null || values.isEmpty()) return List.of();
 
         List<Tag> tags = new ArrayList<>();
         List<String> invalidValues = new ArrayList<>();
 
         for (String value : values) {
             try {
                 tags.add(Tag.from(value));
             } catch (InvalidStateException e) {
                 invalidValues.add(value);
             }
         }
 
         if (!invalidValues.isEmpty()) {
             String message = "다음 태그 이름이 유효하지 않습니다: " + String.join(", ", invalidValues);
             throw new InvalidStateException(TAG_NAME_NOT_MATCH, new IllegalArgumentException(message));
         }
 
         return tags;
     }

Also applies to: 96-116, 7-11, 85-88

src/main/java/konkuk/thip/user/domain/value/Alias.java (2)

25-27: baseUrlSupplier 등록 시 null 방어 필요

null Supplier 등록이 가능해 NPE를 유발할 수 있습니다. 등록 시점에 방어 로직을 추가해 주세요.

-    public static void registerBaseUrlSupplier(Supplier<String> supplier) {
-        baseUrlSupplier = supplier;
-    }
+    public static void registerBaseUrlSupplier(Supplier<String> supplier) {
+        if (supplier == null) {
+            throw new InvalidStateException(ErrorCode.AWS_BUCKET_BASE_URL_NOT_CONFIGURED);
+        }
+        baseUrlSupplier = supplier;
+    }

39-41: getImageUrl에서 NPE 및 이중 슬래시 발생 가능성

  • baseUrlSupplier 미등록 시 NPE 발생 가능
  • baseUrl 후행 슬래시 + imageUrl 선행 슬래시로 “//” 발생 가능

안전하게 조합/검증하도록 수정이 필요합니다.

-    public String getImageUrl() {
-       return baseUrlSupplier.get() + imageUrl;
-    }
+    public String getImageUrl() {
+        if (baseUrlSupplier == null) {
+            throw new InvalidStateException(ErrorCode.AWS_BUCKET_BASE_URL_NOT_CONFIGURED);
+        }
+        String base = baseUrlSupplier.get();
+        if (base == null || base.isBlank()) {
+            throw new InvalidStateException(ErrorCode.AWS_BUCKET_BASE_URL_NOT_CONFIGURED);
+        }
+        String normalizedBase = base.endsWith("/") ? base.substring(0, base.length() - 1) : base;
+        String normalizedPath = imageUrl.startsWith("/") ? imageUrl : "/" + imageUrl;
+        return normalizedBase + normalizedPath;
+    }
src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedCommandPersistenceAdapter.java (1)

64-67: updateFrom 메서드에 ContentList/TagList 갱신 로직 추가 필요

현재 updateFrom(Feed feed)가 스칼라 필드(내용, 공개 여부, 카운트 등)만 복사하고 있어, contentListtagList 변경 내용이 영속화되지 않습니다. 아래 위치를 확인하고 수정해주세요.

  • 파일: src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java
  • 메서드: updateFrom(Feed feed) (라인 62~67)

예시 수정:

 public void updateFrom(Feed feed) {
     this.content      = feed.getContent();
     this.isPublic     = feed.getIsPublic();
     this.reportCount  = feed.getReportCount();
     this.likeCount    = feed.getLikeCount();
     this.commentCount = feed.getCommentCount();
+    // ContentList/TagList 반영 추가
+    this.contentList = feed.getContentList();
+    this.tagList     = feed.getTagList();
 }

필요에 따라 기존 ContentList/TagList 인스턴스에 병합(update) 로직으로 구현해도 좋습니다.

src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java (3)

30-34: aliasName 매핑 불일치: 일관되게 Alias.getValue()로 변환 필요

현재 aliasNamesource = "dto.alias"로 들어가 반면, aliasColorAlias.from(dto.alias()).getColor()로 변환되고 있습니다. 다른 메서드(toFeedRelatedWithBookDto)에서는 aliasNameAlias.from(...).getValue()를 사용합니다. 응답의 일관성과 클라이언트 표시 값 보장을 위해 본 메서드도 동일하게 값 변환을 적용해 주세요.

-    @Mapping(target = "aliasName", source = "dto.alias")
+    @Mapping(target = "aliasName", expression = "java(Alias.from(dto.alias()).getValue())")
     @Mapping(target = "aliasColor", expression = "java(Alias.from(dto.alias()).getColor())")

178-181: Saved 리스트 매핑도 동일 이슈: aliasName 값 변환 누락

FeedShowSavedListResponse 매핑에서도 aliasName이 원문 그대로 전달됩니다. 위와 동일하게 변환을 적용해 주세요.

-    @Mapping(target = "aliasName", source = "dto.alias")
+    @Mapping(target = "aliasName", expression = "java(Alias.from(dto.alias()).getValue())")
     @Mapping(target = "aliasColor", expression = "java(Alias.from(dto.alias()).getColor())")

120-133: toTagsWithCategoryResult: 불필요한 grouping/flatMapping으로 순서 손실 및 오버헤드 발생

입력은 이미 Map<Category, List<Tag>>이며, EnumMappingsEnumMap을 반환하므로 현재 구현은

  • 불필요한 groupingBy + flatMapping(O(n) 이상 오버헤드)
  • HashSet 사용으로 태그/카테고리 응답 순서 비결정성 문제 유발(클라이언트 입장에서 불안정)

카테고리/태그의 선언 순서를 유지하면서 중복만 제거하도록 단순화해 주세요.

-    default List<TagsWithCategoryResult> toTagsWithCategoryResult(Map<Category, List<Tag>> categoryToTags) {
-        Map<String, Set<String>> grouped = categoryToTags.entrySet().stream()
-                .collect(Collectors.groupingBy(
-                        e -> e.getKey().getValue(),
-                        Collectors.flatMapping(
-                                e -> e.getValue().stream().map(Tag::getValue),
-                                Collectors.toSet()
-                        )
-                ));
-
-        return grouped.entrySet().stream()
-                .map(e -> new TagsWithCategoryResult(e.getKey(), new ArrayList<>(e.getValue())))
-                .toList();
-    }
+    default List<TagsWithCategoryResult> toTagsWithCategoryResult(Map<Category, List<Tag>> categoryToTags) {
+        if (categoryToTags == null || categoryToTags.isEmpty()) {
+            return List.of();
+        }
+        // EnumMap 입력 순서를 보존하고, 태그는 encounter order를 유지하며 distinct 처리
+        List<TagsWithCategoryResult> results = new ArrayList<>(categoryToTags.size());
+        for (Map.Entry<Category, List<Tag>> e : categoryToTags.entrySet()) {
+            String categoryName = e.getKey().getValue();
+            List<String> tagValues = e.getValue().stream()
+                    .map(Tag::getValue)
+                    .distinct() // 순서 보존하며 중복 제거
+                    .toList();
+            results.add(new TagsWithCategoryResult(categoryName, tagValues));
+        }
+        return results;
+    }

부가 이점

  • 입력 순서(카테고리)와 태그 순서를 안정적으로 유지
  • 스트림 중첩/그룹핑 제거로 CPU/GC 부담 감소
♻️ Duplicate comments (9)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateControllerTest.java (3)

72-81: 검증 헬퍼가 구 시그니처/하드코딩된 ID를 사용 — 태그/이미지 검증이 권한/존재성 체크에 가로막힙니다

현재 assertBadRequest/feeds/1, userId=100L로 고정되어 있어, 유효성 검증(태그 개수 초과/중복 등) 전에 404/403이 먼저 발생할 수 있습니다. 상단 셋업에서 확보한 savedFeedIdcreatorUserId를 사용하도록 헬퍼 시그니처/호출부를 업데이트해야 합니다.

수정 제안:

-    private void assertBadRequest(int expectedCode, Map<String, Object> request, String message) throws Exception {
-        mockMvc.perform(patch("/feeds/1")
-                                .requestAttr("userId", 100L)
+    private void assertBadRequest(Long feedId, Long userId, int expectedCode, Map<String, Object> request, String message) throws Exception {
+        mockMvc.perform(patch("/feeds/" + feedId)
+                                .requestAttr("userId", userId)
                                 .contentType(MediaType.APPLICATION_JSON)
                                 .content(objectMapper.writeValueAsBytes(request))
                 )
                 .andExpect(status().isBadRequest())
                 .andExpect(jsonPath("$.code").value(expectedCode))
                 .andExpect(jsonPath("$.message", containsString(message)));
     }

141-144: 헬퍼 시그니처 변경에 따른 호출부 수정 필요 (태그 개수 초과 케이스)

유효한 feedId/작성자 id를 넘겨 실제 유효성 검증이 실행되도록 변경하세요.

-            req.put("tagList", List.of(PHYSICS.getValue(), CHEMISTRY.getValue(), KOREAN_NOVEL.getValue(),
-                    FOREIGN_NOVEL.getValue(), CLASSIC_LITERATURE.getValue(), HISTORY.getValue()));
-            assertBadRequest(TAG_LIST_SIZE_OVERFLOW.getCode(), req, TAG_LIST_SIZE_OVERFLOW.getMessage());
+            req.put("tagList", List.of(
+                    PHYSICS.getValue(), CHEMISTRY.getValue(), KOREAN_NOVEL.getValue(),
+                    FOREIGN_NOVEL.getValue(), CLASSIC_LITERATURE.getValue(), HISTORY.getValue()
+            ));
+            assertBadRequest(savedFeedId, creatorUserId, TAG_LIST_SIZE_OVERFLOW.getCode(), req, TAG_LIST_SIZE_OVERFLOW.getMessage());

150-152: 헬퍼 시그니처 변경에 따른 호출부 수정 필요 (중복 태그 케이스)

동일하게 feedId/작성자 id를 전달하도록 수정하세요.

-            req.put("tagList", List.of(KOREAN_NOVEL.getValue(), KOREAN_NOVEL.getValue()));
-            assertBadRequest(TAG_SHOULD_BE_UNIQUE.getCode(), req, TAG_SHOULD_BE_UNIQUE.getMessage());
+            req.put("tagList", List.of(KOREAN_NOVEL.getValue(), KOREAN_NOVEL.getValue()));
+            assertBadRequest(savedFeedId, creatorUserId, TAG_SHOULD_BE_UNIQUE.getCode(), req, TAG_SHOULD_BE_UNIQUE.getMessage());
src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java (1)

37-39: 이전 리뷰 맥락과 정합

“검증을 Feed 내부로 이관” TODO는 기존 합의 내용과 부합합니다. presignedURL 전환 타이밍에 이관하면 좋겠습니다.

src/test/java/konkuk/thip/feed/domain/FeedTest.java (1)

66-88: 주석 처리된 태그/이미지 제약 테스트 복구 필요: TagList/ContentList 기준으로 재작성하세요

리팩터링 핵심 제약(태그 최대 5개, 중복 금지, 이미지 최대 3개)에 대한 회귀가 막히지 않도록 테스트를 활성화해 주세요. 아래와 같이 대체 가능합니다.

-//    @Test
-//    @DisplayName("validateTags: 태그가 5개 초과 시 InvalidStateException이 발생한다.")
-//    void validateTags_exceedsMax_throws() {
-//        List<String> tags = List.of("a", "b", "c", "d", "e", "f");
-//
-//        InvalidStateException ex = assertThrows(InvalidStateException.class,
-//                () -> Feed.validateTags(tags));
-//
-//        assertEquals(INVALID_FEED_COMMAND,  ex.getErrorCode());
-//        assertTrue(ex.getCause().getMessage().contains("최대 5개"));
-//    }
-//
-//    @Test
-//    @DisplayName("validateTags: 중복 태그 있을 경우 InvalidStateException이 발생한다.")
-//    void validateTags_withDuplicates_throws() {
-//        List<String> tags = List.of("a", "b", "a");
-//
-//        InvalidStateException ex = assertThrows(InvalidStateException.class,
-//                () -> Feed.validateTags(tags));
-//
-//        assertEquals(INVALID_FEED_COMMAND, ex.getErrorCode());
-//        assertTrue(ex.getCause().getMessage().contains("중복"));
-//    }
+    @Test
+    @DisplayName("TagList: 태그가 5개 초과 시 InvalidStateException이 발생한다.")
+    void tagList_exceedsMax_throws() {
+        assertThrows(InvalidStateException.class, () ->
+                TagList.of(List.of(
+                        KOREAN_NOVEL, KOREAN_NOVEL, KOREAN_NOVEL,
+                        KOREAN_NOVEL, KOREAN_NOVEL, KOREAN_NOVEL // 총 6개 → 사이즈 초과
+                ))
+        );
+    }
+
+    @Test
+    @DisplayName("TagList: 중복 태그가 있을 경우 InvalidStateException이 발생한다.")
+    void tagList_withDuplicates_throws() {
+        assertThrows(InvalidStateException.class,
+                () -> TagList.of(List.of(KOREAN_NOVEL, KOREAN_NOVEL)));
+    }
+
+    @Test
+    @DisplayName("ContentList: 이미지가 3개 초과 시 InvalidStateException이 발생한다.")
+    void contentList_exceedsMax_throws() {
+        assertThrows(InvalidStateException.class,
+                () -> ContentList.of(List.of("u1", "u2", "u3", "u4")));
+    }
src/main/java/konkuk/thip/feed/domain/Feed.java (1)

162-169: 이미지 업데이트 시 신규 추가가 불가능해지는 로직 문제(중복 제기)

contentList.validateOwnImages(newImageUrls)는 “새 URL이 기존 피드에 모두 존재해야 함”을 강제합니다. 결과적으로 이미지 추가가 불가능합니다. 실사용 시나리오와 충돌하는 제약이므로 제거 또는 역할 재정의가 필요합니다.

  • 권장: 업데이트에서는 신규 추가 허용(크기 제한만 검증). 삭제 플로우에서만 “삭제 대상이 현재 피드에 존재하는지” 확인.

수정안:

     public void updateImages(Long userId, List<String> newImageUrls) {
         validateCreator(userId);
-//        validateImageCount(newImageUrls.size());
-//        validateOwnsImages(newImageUrls);
-        contentList.validateOwnImages(newImageUrls);
-
         this.contentList = convertToContentList(newImageUrls);
     }
src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateApiTest.java (3)

121-126: 이미지 유지 시나리오도 동일 이슈 — 검증 전 em.clear() 필요

업데이트 직후 조회가 캐시된 엔티티를 반환할 수 있습니다. 아래처럼 clear 추가 바랍니다.

 // then
 result.andExpect(status().isOk());
+em.clear();
 FeedJpaEntity updatedFeed = feedJpaRepository.findById(feedId).orElseThrow();
 assertThat(updatedFeed.getContentList()).hasSize(1);
 assertThat(updatedFeed.getContentList().get(0)).isEqualTo("https://s3-mock/image-2.jpg");

156-170: 전체 수정 시나리오도 동일 — em.clear() 후 재조회 권장

동일한 1차 캐시 문제를 방지하기 위해 검증 이전 clear가 필요합니다.

 // then
 result.andExpect(status().isOk());
 
-FeedJpaEntity updated = feedJpaRepository.findById(feedId).orElseThrow();
+em.clear();
+FeedJpaEntity updated = feedJpaRepository.findById(feedId).orElseThrow();
 // 1. 본문
 assertThat(updated.getContent()).isEqualTo("전부 수정되는 피드 테스트");

89-93: MockMvc 요청 후 1차 캐시로 인한 갱신 미반영 가능 — em.clear() 호출 추가 필요

@test 메서드가 @transactional이므로, 요청 트랜잭션과 테스트 트랜잭션이 분리되어 테스트의 영속성 컨텍스트가 이전 스냅샷을 들고 있을 수 있습니다. 검증 전 EntityManager.clear()로 1차 캐시를 비우세요.

아래처럼 한 줄 추가:

 // then
 result.andExpect(status().isOk());
+em.clear(); // 테스트 영속성 컨텍스트 초기화(갱신 내용 재조회)
 long tagCount = feedJpaRepository.findById(feedId).orElseThrow().getTagList().size();
 assertThat(tagCount).isEqualTo(2);

필요한 필드/임포트 추가:

+import jakarta.persistence.EntityManager;
 @Autowired private FeedJpaRepository feedJpaRepository;
+@Autowired private EntityManager em;

Comment on lines +64 to +69
@Override
public boolean add(Tag tag) {
boolean res = tags.add(tag);
validate();
return res;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

불변식 위반 시 내부 상태가 변한 채로 예외가 던져집니다: 예외 안전한 변경 적용 필요

현재 add/addAll/set/add(index)는 “변경 후 검증”을 수행합니다. 검증 실패 시 예외가 발생하더라도 내부 리스트는 이미 변경되어 불변식이 깨진 상태로 남습니다. 도메인 값 객체의 신뢰성 측면에서 치명적입니다.

아래처럼 “선검증 후 변경” 또는 “사본 검증 후 커밋” 패턴으로 바꿔주세요.

@@
     @Override
     public boolean add(Tag tag) {
-        boolean res = tags.add(tag);
-        validate();
-        return res;
+        // pre-validate
+        if (tags.size() >= MAX_SIZE) {
+            throw new InvalidStateException(TAG_LIST_SIZE_OVERFLOW,
+                    new IllegalArgumentException("태그는 최대 " + MAX_SIZE + "개까지 입력할 수 있습니다."));
+        }
+        if (tags.contains(tag)) {
+            throw new InvalidStateException(TAG_SHOULD_BE_UNIQUE);
+        }
+        return tags.add(tag);
     }
@@
     @Override
     public boolean addAll(Collection<? extends Tag> c) {
-        boolean res = tags.addAll(c);
-        validate();
-        return res;
+        if (c == null || c.isEmpty()) return false;
+        List<Tag> copy = new ArrayList<>(tags);
+        copy.addAll(c);
+        validateTags(copy);
+        return tags.addAll(c);
     }
@@
     @Override
     public Tag set(int index, Tag element) {
-        Tag prev = tags.set(index, element);
-        validate();
-        return prev;
+        List<Tag> copy = new ArrayList<>(tags);
+        Tag prev = copy.set(index, element);
+        validateTags(copy);
+        tags.set(index, element);
+        return prev;
     }
@@
     @Override
     public void add(int index, Tag element) {
-        tags.add(index, element);
-        validate();
+        List<Tag> copy = new ArrayList<>(tags);
+        copy.add(index, element);
+        validateTags(copy);
+        tags.add(index, element);
     }

참고: 동일한 문제 패턴이 ContentList에도 존재합니다. 일관되게 수정하는 것을 권장합니다.

Also applies to: 71-76, 78-84, 85-89

🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/feed/domain/value/TagList.java around lines 64-89,
methods like add/addAll/set/add(index) currently perform the modification then
call validate(), which leaves the internal list mutated if validation throws;
change these to be exception-safe by either performing validation on the new
value before mutating or by operating on a defensive copy and only replacing the
internal list after validate() succeeds (i.e., create a copy, apply the change
to the copy, call validate() on the prospective state, and if valid assign the
copy back to the internal field and return the result); apply the same pattern
consistently to the other affected ranges (71-76, 78-84, 85-89) and to
ContentList as noted.

//todo 필요 시 URL 형식 검증 추가 가능
}

public void validateOwnImages(List<String> newImageUrls) {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p3 : 이거 파라미터 이름이 newImageUrls 인데, 새로운 이미지를 검증하는게 아니라, 기존에 존재하는 이미지(= 즉 지금 삭제하길 원하는 이미지의 url) 를 검증하는거 아닌가요??

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아니군요 제가 피드 수정 api 로직을 착각했습니다~~

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java (1)

53-60: 생성자에서 null->empty 코얼레싱으로 NPE/제약 이슈 해소 — 잘 처리되었습니다

contentList/tagList에 대해 empty()로 일관되게 초기화하여 이전의 null 저장 문제를 해소했습니다. 생성 시점 불변성 보장 관점에서 바람직합니다.

🧹 Nitpick comments (2)
src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java (1)

41-42: columnDefinition("TEXT") 대신 @lob(또는 DB-native JSON) 고려하면 이식성/성능에 유리

현재 TEXT로 고정하면 DB 벤더 종속성이 커집니다. 아래 둘 중 하나를 권장합니다.

  • 이식성 중시: @lob + @column(name="...")로 선언해 벤더 의존을 낮춤
  • DB 기능 활용: 운영 DB가 JSON/JSONB를 지원한다면 네이티브 타입(예: Postgres JSONB, MySQL JSON)을 사용해 인덱싱/함수 활용 이점 확보

또한(선택) Hibernate bytecode enhancement를 사용 중이라면 @basic(fetch = LAZY)로 대용량 JSON 컬럼 지연 로딩을 명시할 수 있습니다. 미사용 환경에선 무시될 수 있습니다.

적용 예시:

-    @Column(name = "content_list", columnDefinition = "TEXT")
+    @Lob
+    @Column(name = "content_list")
     private ContentList contentList = ContentList.empty();

-    @Column(name = "tag_list", columnDefinition = "TEXT")
+    @Lob
+    @Column(name = "tag_list")
     @Convert(converter = TagListJsonConverter.class)
     private TagList tagList = TagList.empty();

(선택) 지연 로딩을 명시하려면:

-    @Lob
+    @Lob
+    @Basic(fetch = FetchType.LAZY)
     @Column(name = "content_list")

필요한 import (파일 상단에 추가):

import jakarta.persistence.Lob;
import jakarta.persistence.Basic;
import jakarta.persistence.FetchType;

Also applies to: 48-50

src/main/java/konkuk/thip/feed/application/port/out/dto/FeedQueryDto.java (1)

68-73: 빈 배열 생성 비용 미세 최적화 제안

고빈도 호출일 경우 new String[0] 반복 생성 대신 상수 재사용을 권장합니다. 기능 동일, 미세한 GC/할당 비용 절감 효과가 있습니다.

-        if (contentList == null || contentList.isEmpty()) {
-            return new String[0];
-        }
+        if (contentList == null || contentList.isEmpty()) {
+            return EMPTY_STRINGS;
+        }

다음 상수 선언을 본 레코드 바디 상단에 추가해 주세요:

private static final String[] EMPTY_STRINGS = new String[0];
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between d91c598 and e89d772.

📒 Files selected for processing (3)
  • src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java (3 hunks)
  • src/main/java/konkuk/thip/feed/application/port/out/dto/FeedQueryDto.java (3 hunks)
  • src/main/java/konkuk/thip/roompost/adapter/out/persistence/repository/record/RecordQueryRepositoryImpl.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/konkuk/thip/roompost/adapter/out/persistence/repository/record/RecordQueryRepositoryImpl.java
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.371Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
📚 Learning: 2025-08-20T08:01:11.243Z
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#266
File: src/main/java/konkuk/thip/feed/application/port/out/dto/FeedQueryDto.java:35-35
Timestamp: 2025-08-20T08:01:11.243Z
Learning: seongjunnoh는 DB 제약조건(nullable = false)이 이미 보장하는 상황에서 DTO 생성자의 추가적인 null 체크를 불필요한 방어코드로 보며, 실용적인 구현을 선호한다. 기술적 완벽성보다 실제 도메인 모델의 제약조건을 신뢰하는 접근을 취한다.

Applied to files:

  • src/main/java/konkuk/thip/feed/application/port/out/dto/FeedQueryDto.java
🧬 Code Graph Analysis (1)
src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java (2)
src/main/java/konkuk/thip/feed/domain/value/TagList.java (1)
  • TagList (11-105)
src/main/java/konkuk/thip/feed/domain/value/ContentList.java (1)
  • ContentList (13-146)
⏰ 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 (5)
src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java (2)

8-11: TagList/컨버터 도입으로 조인 제거 — 방향성 좋습니다

TagListJsonConverter + TagList 값객체 채택으로 불필요한 조인을 제거하고 엔티티 단순화를 달성했습니다. 전체 조회 경로가 짧아져 읽기 성능과 유지보수성이 좋아질 변경입니다.


41-42: 컨버터 null 처리 일관성 확인 완료

TagListJsonConverter와 ContentListJsonConverter 모두

  • convertToDatabaseColumn(null) 시 빈 리스트(JSON "[]")로 직렬화
  • convertToEntityAttribute(null 또는 blank) 시 빈 객체(TagList.of/ContentList.empty)로 역직렬화

양방향 정책이 일관되게 적용되어 nullable 컬럼 운영 시에도 NPE 없이 안전합니다.

src/main/java/konkuk/thip/feed/application/port/out/dto/FeedQueryDto.java (3)

6-6: Alias enum 도입으로 타입 안정성/조인 제거 가능 — 방향성 좋습니다

User의 별칭을 도메인 enum으로 직접 받도록 바꾼 점, 타입 안정성과 유지보수성 측면에서 적절합니다. 불필요한 조인/매핑 로직을 줄일 수 있어 쿼리 비용도 감소할 가능성이 큽니다.


52-53: alias 파생값 직접 매핑 OK — 불필요한 null 체크 없이 간결

alias.getImageUrl(), alias.getValue()로 직결 매핑한 선택은 도메인 제약(DB not null)에 의존하는 실용적 접근입니다. 이전 스레드 합의대로 별도 null 방어 로직 없이 유지하는 것이 깔끔합니다.


31-47: 필수: QFeedQueryDto 호출부 전면 점검 및 isPublic null 안전성 보장

  • Q타입 재생성 후 new QFeedQueryDto(...) 호출부 인자의 순서·타입이 변경된 시그니처와 완전히 일치하는지 확인
  • JPA 엔티티(Feed)에서 매핑된 isPublic 컬럼이 nullable=false로 설정되어 있거나, 쿼리 결과가 절대 null이 될 수 없도록 검증
  • NPE 위험 제거를 위해 아래 중 하나를 선택 적용
    • 생성자 파라미터 타입을 원시 boolean으로 변경
      - Boolean isPublic,
      + boolean isPublic,
    • null-safe 디폴트 적용
      -                isPublic,
      +                isPublic != null && isPublic,

위 검토가 완료된 후 머지하세요.

Comment on lines 68 to 70
this.contentList = feed.getContentList();
this.tagList = feed.getTagList();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

updateFrom에서도 null-coalescing 적용 필요 (생성자와의 일관성 유지)

지금 상태에선 업데이트 경로로 null이 들어오면 엔티티 필드가 null로 저장될 수 있어, 생성자 경로(항상 empty 보장)와 의미가 엇갈립니다. 일관된 도메인 불변성을 위해 updateFrom에서도 empty로 코얼레싱하세요.

-        this.contentList = feed.getContentList();
-        this.tagList = feed.getTagList();
+        this.contentList = (feed.getContentList() != null) ? feed.getContentList() : ContentList.empty();
+        this.tagList = (feed.getTagList() != null) ? feed.getTagList() : TagList.empty();
📝 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.

Suggested change
this.contentList = feed.getContentList();
this.tagList = feed.getTagList();
}
this.contentList = (feed.getContentList() != null) ? feed.getContentList() : ContentList.empty();
this.tagList = (feed.getTagList() != null) ? feed.getTagList() : TagList.empty();
}
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntity.java around
lines 68 to 70, updateFrom assigns feed.getContentList() and feed.getTagList()
directly which allows nulls; change updateFrom to coalesce nulls to empty lists
(matching the constructor behavior) by replacing direct assignments with
null-safe assignments that set an empty list when feed.getContentList() or
feed.getTagList() is null.

@seongjunnoh seongjunnoh merged commit 4219deb into develop Aug 20, 2025
4 checks passed
@seongjunnoh seongjunnoh deleted the refactor/#257-alias-category-tag-jpa-structure branch August 20, 2025 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[THIP2025-305] [refactor] alias, category, tag jpa 구조 변경

3 participants