[DDING-53] Feed 생성 API 구현 #197
Conversation
Walkthrough이 풀 리퀘스트는 클럽 피드 생성을 위한 새로운 API와 관련 서비스, 엔티티를 추가하는 변경사항을 포함하고 있습니다. 주요 변경 사항은 Changes
Sequence DiagramsequenceDiagram
participant Client
participant ClubFeedApi
participant ClubFeedController
participant FacadeClubFeedService
participant ClubService
participant FeedService
participant FileMetaDataService
Client->>ClubFeedApi: createFeed(request, principalDetails)
ClubFeedApi->>ClubFeedController: createFeed(request, principalDetails)
ClubFeedController->>FacadeClubFeedService: create(command)
FacadeClubFeedService->>ClubService: getClubByUserId()
FacadeClubFeedService->>FeedService: create(feed)
alt Feed is Image
FacadeClubFeedService->>FileMetaDataService: updateStatus(FEED_IMAGE)
else Feed is Video
FacadeClubFeedService->>FileMetaDataService: updateStatus(FEED_VIDEO)
end
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (11)
src/main/java/ddingdong/ddingdongBE/domain/feed/entity/Feed.java (1)
59-61: updateFeedType 메소드에 유효성 검사 추가를 고려해주세요feedType이 null이 되지 않도록 보호하는 것이 좋을 것 같습니다.
다음과 같이 수정하는 것을 제안드립니다:
public void updateFeedType(FeedType feedType) { + if (feedType == null) { + throw new IllegalArgumentException("FeedType은 null이 될 수 없습니다"); + } this.feedType = feedType; }또한, 메소드의 목적과 사용법을 설명하는 JavaDoc 주석을 추가하면 좋을 것 같습니다:
+/** + * Feed의 타입을 업데이트합니다. + * 이 메소드는 서버 측에서 파일 메타데이터를 기반으로 Feed 타입을 결정할 때 사용됩니다. + * + * @param feedType 설정할 새로운 Feed 타입 + * @throws IllegalArgumentException feedType이 null인 경우 + */ public void updateFeedType(FeedType feedType) {src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedServiceImpl.java (2)
28-30: 트랜잭션 범위 점검
@Transactional이 메서드 레벨에서 선언되어 있어, 로직 실행 중 예외 발생 시 트랜잭션이 롤백되는 점이 적절합니다.
40-47: FEED_IMAGE와 FEED_VIDEO 분기 로직 제안
현재 두 조건이 모두 단순 if문으로 분리되어 있는데, 향후 추가 미디어 타입이 늘어날 예정이라면 스위치 혹은 if-else 구문을 고려해볼 수도 있습니다.src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedService.java (1)
5-8: 인터페이스 설계
create메서드만을 노출하는 방식은 필요한 기능만 캡슐화하여 추상화에 충실한 설계로 보입니다. 확장 가능성을 위해 JavaDoc 주석 등을 추가해도 좋겠습니다.src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/request/CreateFeedRequest.java (1)
14-20: Command 객체 변환 시 향후 필드 확장 고려
현재toCommand에서activityContent,mediaId,user만 사용하고 있습니다. 추후 Feed 생성 로직이 복잡해질 가능성이 있다면, DTO와 Command 계층 간 확장성도 신중히 고려해보시길 권장합니다.src/main/java/ddingdong/ddingdongBE/domain/feed/controller/ClubFeedController.java (1)
15-20: 주 책임 분리에 대한 확인
현재createFeed가 단일 책임으로 보이지만, 인증/인가 이후의 비즈니스 로직이 점차 복잡해질 수 있습니다. 컨트롤러에서 수행해야 할 역할과 서비스 레이어에서 수행해야 할 역할을 명확하게 분리해두면 향후 유지보수에 용이합니다.src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedService.java (2)
23-23: 최신 피드 조회 구현 확인
findNewestAll()을 통해 최신 피드를 조회하는 메서드가 잘 구현되었습니다. 페이징, 정렬 기준 등에 대한 추가 확장이 필요한지 주기적으로 확인해보세요.
34-38: 생성 로직 분리에 대한 체크
create메서드는 단순히FeedRepository.save수행 후 ID를 반환합니다. 이후 트랜잭션 범위나 추가 라이프사이클 이벤트(알림이나 로그 등)가 필요하다면 메서드를 세분화하거나 구분하는 것을 검토해보세요.src/main/java/ddingdong/ddingdongBE/domain/feed/api/ClubFeedApi.java (1)
20-29: 성공적인 처리 후 반환값 검토 권장
createFeed메서드를void로 선언하셨는데, 생성된 리소스의 식별자(ID)나 관련 정보를 함께 반환하면, 클라이언트 측에서 추가 요청 없이 생성 결과를 활용할 수 있어 더 유용할 수 있습니다.src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeFeedService.java (1)
33-33: 최신 Feed 조회 로직
feedService.getNewestAll()호출 이후 정렬 기준이나 페이징 처리가 필요할 경우, 별도 파라미터를 활용해 성능 및 응답 크기를 제어할 수 있습니다.src/main/java/ddingdong/ddingdongBE/file/service/S3FileService.java (1)
110-118: Key 형식 검증 로직에서의 잠재적 안정성 이슈
parts.length != KEY_COMPONENT_COUNT조건을 통해 예외를 발생시키는 로직은 적절합니다. 다만 실제 운영 환경에서 Key 구조가 변경될 가능성이 있다면, 예외 메시지를 더 자세히 기록하거나, Key 포맷을 문서화해 유지보수를 쉽게 만들어 주시기 바랍니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/main/java/ddingdong/ddingdongBE/domain/feed/api/ClubFeedApi.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/controller/ClubFeedController.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/request/CreateFeedRequest.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/entity/Feed.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/entity/FeedType.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedService.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedServiceImpl.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeFeedService.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/service/FeedService.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedService.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/command/CreateFeedCommand.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/filemetadata/entity/DomainType.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/filemetadata/service/FileMetaDataService.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/filemetadata/service/FileMetaDataServiceImpl.java(1 hunks)src/main/java/ddingdong/ddingdongBE/file/service/S3FileService.java(2 hunks)src/main/resources/db/migration/V30__feed_url_modify_nullable.sql(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/resources/db/migration/V30__feed_url_modify_nullable.sql
🔇 Additional comments (19)
src/main/java/ddingdong/ddingdongBE/domain/feed/entity/Feed.java (2)
Line range hint
1-57: 엔티티 구조가 잘 설계되었습니다!
- JPA 어노테이션을 적절히 사용하였습니다
- Soft delete 구현이 잘 되어있습니다
- ManyToOne 관계에서 LAZY 로딩을 사용한 것이 좋습니다
Line range hint
33-34: nullable 필드 변경이 적절합니다thumbnailUrl과 fileUrl을 nullable로 변경한 것은 PR의 목적에 잘 부합합니다. 이는 파일 메타데이터를 통해 컨텐츠 타입을 서버에서 결정하는 접근 방식을 지원합니다.
Also applies to: 36-37
src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedServiceImpl.java (3)
22-26: 의존성 주입이 적절하게 구성되었습니다.
필요한 서비스들을 명확히 주입받고 있으므로 모듈 간 결합도가 높아지지 않도록 잘 설계하신 것 같습니다.
31-33: Club 객체 조회 로직 확인
clubService.getByUserId메서드를 통해Club엔티티를 획득하는 로직이 문제없이 수행되는지, 예외 처리(존재하지 않는 사용자 ID의 경우 등)가 필요한지 확인 부탁드립니다.
34-37: 콘텐츠 타입으로 FeedType을 설정하는 로직 검토
FeedType.findByContentType메서드에서 지원하지 않는 콘텐츠 타입인 경우IllegalArgumentException이 발생합니다. 해당 예외 처리가 상위 레벨(Controller 등)에서 적절히 처리되는지 확인해 주세요.src/main/java/ddingdong/ddingdongBE/domain/filemetadata/entity/DomainType.java (1)
13-14: FEED_IMAGE 및 FEED_VIDEO 상수 추가 확인
새로운 도메인 타입이 추가되어, 피드의 이미지/비디오를 명확히 구분할 수 있게 되었습니다. 관련 로직과의 호환성도 좋습니다.src/main/java/ddingdong/ddingdongBE/domain/feed/service/FeedService.java (1)
6-15: CRUD 메서드 구성이 깔끔합니다
getAllByClubId,getNewestAll,getById,create처럼 명확히 분리된 메서드로 관리 범위가 분명해졌습니다. 향후 확장할 때에도 일관성을 유지하기 좋겠습니다.src/main/java/ddingdong/ddingdongBE/domain/feed/entity/FeedType.java (1)
8-13: 콘텐츠 타입 식별 전략
findByContentType메서드가 대소문자를 구분하지 않고 타입을 찾는 로직은 유연성이 뛰어납니다. 단, 지원하지 않는 콘텐츠 타입의 에러 처리를 어떻게 할지(에러 메시지 표준화, 프런트에 대한 응답 등) 한 번 더 점검해 주세요.src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/command/CreateFeedCommand.java (2)
9-13: 레코드 정의 사용에 대한 칭찬
CreateFeedCommand를record로 정의하고,Builder패턴을 이용해 직관적으로 필드를 초기화할 수 있도록 한 점이 깔끔합니다.
15-20: 엔티티 변환 시 누락된 필드 확인 권장
mediaId,user필드가toEntity메서드에서 사용되지 않고 있습니다. 추후Feed엔티티가 확장될 가능성이 있다면, 해당 필드를 반영해줄 필요가 있습니다.src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/request/CreateFeedRequest.java (1)
7-12: API 요청 필드 설명에 대한 적절한 스키마 사용
@Schema애너테이션을 통해 API 문서화가 잘 되고 있습니다. 예시(example) 값도 실제 운영 시나리오를 잘 반영한 것으로 보입니다.src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedService.java (2)
14-16: 인터페이스 구현에 대한 적절한 @OverRide 사용
FeedService인터페이스를 구현하고,@Override를 통해 가독성과 유지보수를 향상했습니다. 명시적 오버라이드 사용을 잘 하셨습니다.
28-32: 리소스 예외 처리가 명확
getById에서ResourceNotFound예외를 던지고, 메시지로Feed(id: ...)를 찾을 수 없습니다.를 반환하는 것은 문제 상황을 명확히 알려줘서 좋습니다.src/main/java/ddingdong/ddingdongBE/domain/feed/api/ClubFeedApi.java (2)
1-9: 인터페이스 설계가 명확합니다.
새로운 인터페이스로 REST API를 정의함으로써 Controller 계층에 대한 의존성을 줄일 수 있어 유지보수와 확장성에 도움이 됩니다.
16-18: API 베이스 경로와 Tag 설정이 명확합니다.
@Tag와@RequestMapping("/server")의 선언으로 관련 API 그룹을 명확하게 구분하셨습니다.src/main/java/ddingdong/ddingdongBE/domain/filemetadata/service/FileMetaDataService.java (1)
13-14: UUID가 아닌String파라미터를 받는 이유 확인 요청
파일 메타데이터를 조회할 때String id를 입력받아 내부에서UUID변환을 수행하시는 것으로 보입니다. 만약 클라이언트 측에서 이미 UUID 문자열을 사용하고 있다면 일관성을 위해 처음부터UUID id형태로 받는 방안을 고려해보셔도 좋겠습니다.src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeFeedService.java (2)
22-22: FeedService로 의존성 교체
generalFeedService에서feedService로의 변경은 전체적인 서비스 구조를 단순화하고 명확하게 만듭니다. 의도가 분명해 보이며, 재사용성 측면에서도 좋아 보입니다.
26-26: 클럽 ID로 Feed 조회 시 예외 처리를 고려해 주세요
feedService.getAllByClubId(clubId)호출 시, 만약clubId가 존재하지 않거나 유효하지 않을 경우 예외가 발생할 가능성이 있습니다. 적절한 예외 처리가 필요할 수 있습니다.src/main/java/ddingdong/ddingdongBE/file/service/S3FileService.java (1)
36-37: 키 구분자 상수 도입은 가독성 향상에 유리합니다
KEY_SEPARATOR와KEY_COMPONENT_COUNT상수를 정의하여 Key 구조를 분명하게 제어할 수 있게 된 점이 좋습니다.
| return feeds.stream() | ||
| .map(FeedListQuery::from) | ||
| .toList(); | ||
| } | ||
|
|
||
| public FeedQuery getById(Long feedId) { | ||
| Feed feed = generalFeedService.getById(feedId); | ||
| Feed feed = feedService.getById(feedId); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
단일 Feed 조회 시 존재하지 않는 자원에 대한 예외 처리
feedService.getById(feedId)의 결과가 없을 때 예외 처리를 어떻게 할지 확인이 필요합니다. 응답 형태(404 Not Found 등)와 예외 메시지를 클라이언트에 일관성 있게 전달하도록 제안드립니다.
| @Override | ||
| public FileMetaData getById(String id) { | ||
| UUID uuid = UUID.fromString(id); | ||
| return fileMetaDataRepository.findById(uuid) | ||
| .orElseThrow(() -> new ResourceNotFound("해당 FileMetaData(id: " + uuid + ")를 찾을 수 없습니다.)")); | ||
| } |
There was a problem hiding this comment.
오탈자 수정이 필요합니다.
해당 예외 메시지에서 불필요한 ) 문자가 포함되어 있습니다. 아래와 같이 수정하여 문자열을 올바르게 닫아야 합니다.
- .orElseThrow(() -> new ResourceNotFound("해당 FileMetaData(id: " + uuid + ")를 찾을 수 없습니다.)"));
+ .orElseThrow(() -> new ResourceNotFound("해당 FileMetaData(id: " + uuid + ")를 찾을 수 없습니다."));📝 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.
| @Override | |
| public FileMetaData getById(String id) { | |
| UUID uuid = UUID.fromString(id); | |
| return fileMetaDataRepository.findById(uuid) | |
| .orElseThrow(() -> new ResourceNotFound("해당 FileMetaData(id: " + uuid + ")를 찾을 수 없습니다.)")); | |
| } | |
| @Override | |
| public FileMetaData getById(String id) { | |
| UUID uuid = UUID.fromString(id); | |
| return fileMetaDataRepository.findById(uuid) | |
| .orElseThrow(() -> new ResourceNotFound("해당 FileMetaData(id: " + uuid + ")를 찾을 수 없습니다.")); | |
| } |
src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/request/CreateFeedRequest.java
Outdated
Show resolved
Hide resolved
src/main/java/ddingdong/ddingdongBE/domain/feed/entity/Feed.java
Outdated
Show resolved
Hide resolved
src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedServiceImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedService.java
Show resolved
Hide resolved
src/main/resources/db/migration/V30__feed_url_modify_nullable.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/test/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedServiceTest.java (1)
29-44: 테스트 시나리오 확대 제안
피드 생성 시나리오가 정상적으로 동작함을 테스트하고 있지만, 예외 상황(예: 필수 필드 누락, Feed 객체에 잘못된 타입 주입 등)에 대한 단위 테스트도 추가 고려해보시면 좋겠습니다.src/test/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedServiceImplTest.java (1)
50-86: 생성된 Feed 및 FileMetaData 검증 범위 확장 제안
현재 테스트는 FeedType과 FileMetaData 상태만을 검증합니다. Feed에 설정된 activityContent나 club과의 연관관계 등, 다른 주요 필드에 대한 검증도 포함하면, 더 견고한 테스트가 될 것입니다.src/test/java/ddingdong/ddingdongBE/domain/feed/repository/FeedRepositoryTest.java (1)
47-50: 테스트 내activityContent중복 사용 가능성 점검
내용2등 비슷한 패턴의 문구가 반복될 경우, 향후 변경 및 유지 보수 시 혼동이 생길 가능성이 있습니다.
테스트의 가독성을 높이기 위해 상수로 관리하거나 의미 있는 문자열로 교체하는 방안을 고려해 보세요.src/main/java/ddingdong/ddingdongBE/domain/feed/entity/Feed.java (1)
53-55: FeedType 업데이트 메서드에 유효성 검증이 필요합니다.
updateFeedType메서드에서 다음 사항들을 고려해 주세요:
- null 체크
- 현재 타입과 동일한 타입으로의 불필요한 업데이트 방지
다음과 같이 수정하는 것을 제안드립니다:
public void updateFeedType(FeedType feedType) { + if (feedType == null) { + throw new IllegalArgumentException("FeedType은 null일 수 없습니다."); + } + if (this.feedType == feedType) { + return; + } this.feedType = feedType; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/request/CreateFeedRequest.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/entity/Feed.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedServiceImpl.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/command/CreateFeedCommand.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/FeedListQuery.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/FeedQuery.java(1 hunks)src/main/resources/db/migration/V31__feed_url_modify_nullable.sql(1 hunks)src/test/java/ddingdong/ddingdongBE/domain/feed/repository/FeedRepositoryTest.java(2 hunks)src/test/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedServiceImplTest.java(1 hunks)src/test/java/ddingdong/ddingdongBE/domain/feed/service/FacadeFeedServiceTest.java(0 hunks)src/test/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedServiceTest.java(1 hunks)src/test/java/ddingdong/ddingdongBE/domain/filemetadata/service/FileMetaDataServiceImplTest.java(1 hunks)src/test/java/ddingdong/ddingdongBE/file/service/S3FileServiceTest.java(4 hunks)
💤 Files with no reviewable changes (1)
- src/test/java/ddingdong/ddingdongBE/domain/feed/service/FacadeFeedServiceTest.java
✅ Files skipped from review due to trivial changes (2)
- src/main/resources/db/migration/V31__feed_url_modify_nullable.sql
- src/test/java/ddingdong/ddingdongBE/file/service/S3FileServiceTest.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (13)
src/test/java/ddingdong/ddingdongBE/domain/filemetadata/service/FileMetaDataServiceImplTest.java (3)
213-215: 테스트 메서드 명이 명확하여 가독성이 좋습니다.
테스트의 목적을 명확히 드러내는 메서드 이름으로, 유지보수와 협업 시 이해를 돕습니다.
219-227: FixtureMonkey 활용으로 테스트 데이터 생성이 간결합니다.
직접 빌드 로직을 구현하지 않고 FixtureMonkey를 사용함으로써, 테스트 데이터 준비 코드가 간결해지고 가독성이 좋아졌습니다.
229-234: ID가 존재하지 않는 경우의 예외 상황 테스트도 제안드립니다.
정상적인 ID 값을 사용하는 경우뿐만 아니라 존재하지 않는 ID에 대한 예외가 정상 동작하는지도 확인하면 더 견고한 테스트가 됩니다.src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/FeedListQuery.java (1)
16-16:thumbnailUrl을 항상null로 반환하는 결정에 대해 확인 필요
현재thumbnailUrl필드를 무조건null로 세팅하고 있습니다. 추후 프론트엔드에서 썸네일을 표시할 때 혼동이 있을 수 있으니, 추가적인 이미지 처리 로직이 필요할지 검토해보시기 바랍니다.src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/FeedQuery.java (1)
22-22:fileUrl을 항상null로 세팅하는 로직 점검
현재fileUrl을 날것으로null로만 처리하고 있어, 파일 경로가 필요한 기능에서 오작동할 여지가 있습니다. 실제 API 응답에서 파일 URL이 필요한 시점에는 어떻게 처리가 될지 검토가 필요해 보입니다.src/test/java/ddingdong/ddingdongBE/domain/feed/repository/FeedRepositoryTest.java (3)
42-45: 테스트 데이터 생성 로직이 적절합니다.
activityContent설정을 통해Feed를 생성하는 로직이 일관되며,FeedType설정도 적절해 보입니다.
Line range hint
52-56:Feed엔티티 ID 순서 가정에 대한 확인이 필요합니다.
테스트 결과 검증 시 ID가 1, 2, 3 순으로 할당된다고 가정하고 있습니다. 만약 DB 전략 혹은 다른 요인으로 인해 ID가 순차적으로 부여되지 않을 경우, 테스트가 깨질 위험이 있습니다.
[Testcontainers]나 [GeneratedValue(strategy = GenerationType.IDENTITY)] 사용 등 실제 환경의 시나리오와 일치하는지 확인해 주세요.
63-67: 정렬 순서 검증 로직이 명확합니다.
피드를 최신순(내림차순)으로 조회하여 인덱스별activityContent와 ID를 정확히 비교하는 로직이 가독성과 명확성을 모두 갖추고 있습니다.src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/command/CreateFeedCommand.java (1)
17-23:mediaId가toEntity에서 사용되지 않습니다.
CreateFeedCommand에 포함된mediaId필드가 실제Feed빌더에 반영되지 않고 있습니다.
해당 값이 추후에 또는 다른 로직에서 사용되는지 확인 바라며, 불필요하다면 제거를 고려해 보세요.src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/request/CreateFeedRequest.java (2)
8-9:activityContent에 대한 NotNull 검증 추가 권장
Feed엔티티에서activityContent가 null일 수 없다면, 여기에@NotBlank또는@NotNull검증 어노테이션을 부여하여 요청 유효성을 보장하는 것을 권장드립니다.
16-23:toCommand메서드의 역할이 명확합니다.
요청 DTO가CreateFeedCommand와 분리되어 있어, 계층 간 의존도를 줄이는 구조가 바람직합니다. 확장성 면에서도 좋은 설계입니다.src/main/java/ddingdong/ddingdongBE/domain/feed/entity/Feed.java (2)
47-51: 생성자가 적절하게 수정되었습니다.PR 설명에서 언급된 대로 파일 메타데이터를 통해 타입을 결정하는 방식으로 변경되어 생성자가 적절하게 수정되었습니다.
57-62: 타입 체크 메서드가 잘 구현되었습니다.
isImage()와isVideo()메서드를 통해 타입 안전성을 보장하고 가독성을 향상시켰습니다. 이는 PR 설명에서 언급된 "Video와 Image 타입에 따른 응답을 구분"하는데 도움이 될 것 같습니다.
| if (feed.isImage()) { | ||
| fileMetaDataService.updateStatusToCoupled(command.mediaId(), DomainType.FEED_IMAGE, createdId); | ||
| } | ||
|
|
||
| if (feed.isVideo()) { | ||
| fileMetaDataService.updateStatusToCoupled(command.mediaId(), DomainType.FEED_VIDEO, createdId); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
하나의 피드가 동시에 이미지와 비디오로 처리될 가능성에 대한 고려
feed.isImage()와 feed.isVideo()가 모두 true가 될 수 있는 상황이라면, fileMetaDataService.updateStatusToCoupled()가 두 번 호출될 소지가 있습니다.
실업무 로직상 ‘이미지 또는 비디오 중 하나’만 가능하다면 else if로 제어하여 중복 처리를 회피하는 방안을 고려해 주세요.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/ddingdong/ddingdongBE/domain/feed/entity/Feed.java (1)
53-58: 타입 체크 로직 개선을 고려해보세요.현재 구현은 간단하지만, 향후 새로운 타입이 추가될 경우 확장성 측면에서 제한이 있을 수 있습니다. 다형성을 활용한 설계를 고려해보시는 것은 어떨까요?
예시 리팩토링:
-public class Feed extends BaseEntity { +public abstract class Feed extends BaseEntity { // ... existing fields ... - private FeedType feedType; - public boolean isImage() { - return feedType == FeedType.IMAGE; - } - public boolean isVideo() { - return feedType == FeedType.VIDEO; - } + public abstract boolean isImage(); + public abstract boolean isVideo(); } +@Entity +public class ImageFeed extends Feed { + @Override + public boolean isImage() { + return true; + } + + @Override + public boolean isVideo() { + return false; + } } +@Entity +public class VideoFeed extends Feed { + @Override + public boolean isImage() { + return false; + } + + @Override + public boolean isVideo() { + return true; + } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/ddingdong/ddingdongBE/domain/feed/api/ClubFeedApi.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/request/CreateFeedRequest.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/entity/Feed.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedServiceImpl.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/ddingdong/ddingdongBE/domain/feed/api/ClubFeedApi.java
- src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedServiceImpl.java
- src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/request/CreateFeedRequest.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (2)
src/main/java/ddingdong/ddingdongBE/domain/feed/entity/Feed.java (2)
47-51: 생성자 변경이 적절해 보입니다.파일 관련 필드들을 제거하고 핵심 데이터만 남긴 것이 좋은 변경사항으로 보입니다.
Line range hint
31-35: FeedType 필드 유지에 대한 의견이전 리뷰에서 FeedType 제거가 제안되었으나, PR 설명에 따르면 응답 구분과 컨텐츠 타입 조회 복잡도 감소를 위해 의도적으로 유지한 것으로 보입니다. 이 결정은 타당해 보입니다.
|
🚀 작업 내용
🤔 고민했던 내용
Feed 테이블 내에 FeedType을 삭제할지 고민했습니다. 결론으로는 삭제하지 않기로 결정했고,
그 이유로 Feed 조회시 Video, Image에 대한 응답값이 다르고, FileMetaData의 key로 매번 contentType을 찾아야하는 번거로움을 줄일수 있기 때문입니다.
클라이언트가 요청 body에 type을 명시하는 방향도 고려하였지만, 이미 만들어진 fileMetaData key로 서버 내에서
컨텐츠 종류를 찾을 수 있어 이 방법으로 진행하였습니다.
다만, 서버 내에서 직접 확장자를 명시하여 어떤 콘텐츠 종류인지 구분하고 있는데, 모든 사용자의 콘텐츠 종류를 구분할 수 있을지 걱정되긴 합니다.
💬 리뷰 중점사항
Summary by CodeRabbit
릴리즈 노트
새로운 기능
개선 사항
기술적 변경
테스트