[Feat] 스토리 목록 조회 구현 #84
Conversation
Walkthrough새로운 GET API 엔드포인트( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant StoryController
participant StoryService
participant StoryRepository
participant ImageService
Client->>StoryController: GET /api/stories
StoryController->>StoryService: getPagedStoryPreviews()
StoryService->>StoryRepository: findAllByOrderByCreatedAtDesc(PageRequest)
StoryRepository-->>StoryService: Page<Story>
loop 각 Story에 대해
StoryService->>ImageService: getPresignedUrl(imageKey)
ImageService-->>StoryService: imageUrl
end
StoryService-->>StoryController: StoriesResponse
StoryController-->>Client: HTTP 200 + StoriesResponse
Suggested labels
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (9)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (8)
⏰ 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)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
|
📌 최신 ERD가 자동 생성되었습니다. 👉 ERD 보러가기 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/eatda/controller/story/StoryController.java (1)
30-30: API 경로에 일관성을 위해 슬래시를 추가하는 것을 고려해보세요.다른 엔드포인트와의 일관성을 위해
/api/stories로 수정하는 것이 좋겠습니다.- @GetMapping("api/stories") + @GetMapping("/api/stories")src/test/java/eatda/document/story/StoryDocumentTest.java (1)
155-177: 테스트 메서드 구현이 적절함테스트가 올바르게 구현되어 있습니다:
- 목 응답 데이터 생성
- 서비스 메서드 목킹
- GET 요청 수행 및 상태 코드 검증
하지만 응답 본문 검증이 누락되어 있습니다.
응답 본문 검증을 추가하는 것을 고려해보세요:
- response.then().statusCode(200); + response.then() + .statusCode(200) + .body("stories.size()", equalTo(2)) + .body("stories[0].storyId", equalTo(1)) + .body("stories[1].storyId", equalTo(2));또한
hamcrest.Matchers.equalToimport도 필요합니다.src/main/java/eatda/service/story/StoryService.java (1)
72-72: 변수명 개선 제안변수명
orderByPage가 의미를 명확히 전달하지 못합니다.더 명확한 변수명으로 변경을 고려해보세요:
- Page<Story> orderByPage = storyRepository.findAllByOrderByCreatedAtDesc(pageable); + Page<Story> storyPage = storyRepository.findAllByOrderByCreatedAtDesc(pageable);그리고 아래 코드도 함께 수정:
- return new StoriesResponse( - orderByPage.getContent().stream() + return new StoriesResponse( + storyPage.getContent().stream()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/eatda/controller/story/StoryController.java(2 hunks)src/main/java/eatda/repository/story/StoryRepository.java(1 hunks)src/main/java/eatda/service/story/StoryService.java(3 hunks)src/main/resources/db/seed/dev/V4__dev_add_story_data.sql(1 hunks)src/main/resources/db/seed/local/V4__local_add_story_data.sql(1 hunks)src/test/java/eatda/controller/story/StoryControllerTest.java(3 hunks)src/test/java/eatda/document/story/StoryDocumentTest.java(3 hunks)src/test/java/eatda/service/story/StoryServiceTest.java(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/test/java/eatda/controller/story/StoryControllerTest.java (1)
Learnt from: leegwichan
PR: YAPP-Github/26th-Web-Team-1-BE#60
File: src/test/java/eatda/controller/store/StoreControllerTest.java:10-32
Timestamp: 2025-07-09T07:56:50.612Z
Learning: 컨트롤러 테스트에서 MockitoBean으로 의존성을 모킹한 경우, 상세한 비즈니스 로직 검증보다는 컨트롤러 계층의 동작(라우팅, 파라미터 처리, 응답 구조 등)을 검증하는 것이 더 적절합니다. 모킹된 데이터에 대한 상세 검증은 의미가 없기 때문입니다.
🧬 Code Graph Analysis (1)
src/test/java/eatda/document/story/StoryDocumentTest.java (2)
src/test/java/eatda/controller/story/StoryControllerTest.java (1)
Nested(31-56)src/test/java/eatda/service/story/StoryServiceTest.java (2)
Nested(25-69)Nested(71-120)
🔇 Additional comments (17)
src/main/resources/db/seed/dev/V4__dev_add_story_data.sql (1)
1-5: 개발 환경용 시드 데이터가 잘 구성되어 있습니다.다양한 음식점 카테고리(한식, 양식, 중식)와 현실적인 한국어 설명이 포함되어 있고, 일관된 주소 형식과 이미지 키 패턴을 사용하고 있습니다. 더미 카카오 ID도 실제 데이터와 충돌하지 않도록 적절히 설정되었습니다.
src/main/java/eatda/controller/story/StoryController.java (1)
31-34: 스토리 목록 조회(/api/stories)는 공개 API로 설계된 것으로 보입니다
- 다른 GET 엔드포인트들은 인증이 필요한 경우 메서드 시그니처에
LoginMember member를 포함하고 있고,getStories()는 의도적으로LoginMember없이 공개 접근을 허용하고 있습니다.인증이 필요하다면 해당 메서드에
LoginMember member파라미터를 추가해 주세요.src/main/resources/db/seed/local/V4__local_add_story_data.sql (1)
1-9: 로컬 환경용 시드 데이터가 잘 구성되어 있습니다.개발 환경보다 더 다양한 데이터(7개 항목)를 제공하고 있으며, ETC 카테고리에 카페와 패스트푸드 등이 포함되어 포괄적인 테스트가 가능합니다. 개발 환경 시드 데이터와 일관된 구조를 유지하고 있습니다.
src/test/java/eatda/controller/story/StoryControllerTest.java (1)
58-79: 컨트롤러 테스트가 적절하게 구현되었습니다.서비스 계층을 모킹하여 컨트롤러의 라우팅, 응답 구조, 상태 코드를 검증하는 적절한 컨트롤러 테스트입니다. 인증 헤더와 응답 구조 검증이 잘 구현되어 있습니다.
src/test/java/eatda/service/story/StoryServiceTest.java (1)
71-120: 서비스 계층 테스트가 잘 구현되었습니다.페이지네이션 로직과 이미지 URL 생성을 포함하여 서비스의 핵심 기능을 적절히 테스트하고 있습니다. 스토리 순서 검증을 통해 최신순 정렬이 올바르게 동작하는지 확인하고 있습니다.
src/test/java/eatda/document/story/StoryDocumentTest.java (4)
8-8: 새로운 import 추가가 적절함
fieldWithPathimport가 응답 필드 문서화를 위해 올바르게 추가되었습니다.
10-10: StoriesResponse import 추가 확인새로운 응답 DTO 클래스가 올바르게 import되었습니다.
21-21: List import 추가 확인목록 데이터 처리를 위한 List import가 적절히 추가되었습니다.
137-153: REST Docs 문서화 설정이 완전함스토리 목록 조회 API에 대한 문서화 설정이 잘 구성되어 있습니다:
- 요청 헤더(Authorization) 문서화
- 응답 필드 구조 문서화 (stories, storyId, imageUrl)
- 한국어 설명이 명확함
src/main/java/eatda/repository/story/StoryRepository.java (4)
4-6: 페이지네이션 지원을 위한 import 추가 확인JpaRepository 사용과 페이지네이션을 위한 필요한 import들이 올바르게 추가되었습니다.
8-8: Repository에서 JpaRepository로 변경 확인표준 CRUD 작업과 페이지네이션 지원을 위해 JpaRepository로 변경한 것이 적절합니다.
10-10: 페이지네이션 메서드 구현이 올바름
findAllByOrderByCreatedAtDesc메서드가 Spring Data JPA 명명 규칙을 따르며 올바르게 구현되었습니다:
- 메서드명이 의도를 명확히 표현
- 생성일 기준 내림차순 정렬
- 페이지네이션 지원
10-10: StoryRepository의 커스텀 getById 사용처 없음 확인
- 코드베이스 전체에서
storyRepository.getById(...)호출이 발견되지 않음- StoryRepository는
findAllByOrderByCreatedAtDesc만 제공되며, 제거된getById는 실제로 사용되지 않음- 따라서 커스텀
getById제거로 인한 영향은 없으며, 추가 조치가 필요하지 않습니다src/main/java/eatda/service/story/StoryService.java (4)
5-5: 새로운 응답 DTO import 추가 확인StoriesResponse import가 올바르게 추가되었습니다.
18-20: 페이지네이션 관련 import 추가 확인Spring Data의 페이지네이션 기능을 위한 import들이 적절히 추가되었습니다.
28-29: 페이지네이션 상수 정의가 적절함페이지네이션 파라미터가 상수로 잘 정의되어 있습니다:
PAGE_START_NUMBER = 0: 첫 번째 페이지부터 시작PAGE_SIZE = 5: 적절한 페이지 크기PR 목적에 명시된 "최신 5개 스토리만 반환"과 일치합니다.
69-82: 스토리 목록 조회 메서드 구현이 우수함메서드가 잘 구현되어 있습니다:
- 읽기 전용 트랜잭션 어노테이션 적절
- 페이지네이션 설정 올바름
- Stream API를 사용한 깔끔한 매핑
- presigned URL 생성으로 보안 고려
구현이 PR 목적과 정확히 일치하며 모범 사례를 따르고 있습니다.
📄 Terraform Plan Summary🛡️ Common InfrastructureStatus: ✅ No Changes 🛠️ Development EnvironmentStatus: ✅ No Changes 📋 Full Results: View in Actions |
leegwichan
left a comment
leegwichan
left a comment
There was a problem hiding this comment.
/noti 승로님 고생하셨습니다! 등록 로직보다는 확실히 작업량이 적네요.
관련해서 코멘트 달았으니 선택적으로 반영해주세요! 다음 작업도 화이팅!
|
|
||
| public interface StoryRepository extends Repository<Story, Long> { | ||
| public interface StoryRepository extends JpaRepository<Story, Long> { | ||
|
|
||
| Story save(Story story); | ||
|
|
||
| Optional<Story> findById(Long id); | ||
|
|
||
| default Story getById(Long id) { | ||
| return findById(id) | ||
| .orElseThrow(() -> new BusinessException(BusinessErrorCode.STORY_NOT_FOUND)); | ||
| } | ||
| Page<Story> findAllByOrderByCreatedAtDesc(Pageable pageable); | ||
| } |
There was a problem hiding this comment.
[제안] 이건 제가 이전 PR에서 JpaRepository로 잘못 작성했었네요;; 그래서 다시 Repository를 상속하게 하면 좋을 것 같아요!
There was a problem hiding this comment.
으악 이 부분은 제가 사용하면서도 의문이 들었던 지점인데요.
리포지토리 계층에서 도메인 예외를 던지는 게 맞나...? 라는 고민이 들었습니다.
서비스에서 orElseThrow로 처리하는 게 귀찮은 건 맞지만,
구조적으로 이게 올바른 방향인가? 라는 생각이 들어요.
특히 기본 메서드인 save, findById 등을 다시 선언하면서
Repository만 상속하는 방식이 실용적인가에 대한 의문도 들고요.
그래서 지금은 기능도 많지 않고 복잡하지 않으니
그냥 JpaRepository 상속 방식으로 가는 게 어떨까… 조심스럽게 의견 드려봅니다 🙏
There was a problem hiding this comment.
서비스에서 orElseThrow로 처리하는 게 귀찮은 건 맞지만, 구조적으로 이게 올바른 방향인가? 라는 생각이 들어요.
저도 내용에 동의하는 바에요! Repository에서 서비스 관련 예외를 던진다는게 조금 애매하다고 생각합니다.
그냥 JpaRepository 상속 방식으로 가는 게 어떨까… 조심스럽게 의견 드려봅니다 🙏
저도 좋다고 생각합니다. 대신 제 몇가지 의견이 있어요
- 대신
findById().orElseThrow()로직을 줄이기 위해서 서비스가 서비스를 참조하는 일은 없었으면 좋겠습니다. (생각보다 순환 참조가 종종 일어나더라구요...) - 그렇게 되면,
findById().orElseThrow()가 여러 서비스 로직에서 중복적으로 일어날 거에요. 매번 "어 얘 없으면 던지는 에러 코드가 뭐였지?", "이거 에러 코드 바꿔야하는데 전부 어디에 쓰여 있지?" 등의 골치아픈 일이 일어날 수 있어요.
이번 PR에서는 넘어가고, 한 번 이런 주제들을 모아서 시간 잡아서 의견 나눠보죠!
There was a problem hiding this comment.
험... 그러게요
대신 findById().orElseThrow() 로직을 줄이기 위해서 서비스가 서비스를 참조하는 일은 없었으면 좋겠습니다. (생각보다 순환 참조가 종종 일어나더라구요...)
순환 참조 문제는 저도 공감합니다.
외부 API를 사용하지 않는 서비스라면, 다른 서비스를 직접 참조하는 건 최대한 피하는 방향으로 생각하고 있었어요.
그렇게 되면, findById().orElseThrow() 가 여러 서비스 로직에서 중복적으로 일어날 거에요. 매번 "어 얘 없으면 던지는 에러 코드가 뭐였지?", "이거 에러 코드 바꿔야하는데 전부 어디에 쓰여 있지?" 등의 골치아픈 일이 일어날 수 있어요.
흠 이거는 충분히 발생할수 있는일이긴 하네요...
뭔가 좀 절충점은 리포지토리마다 사이드카 느낌으로 헬퍼 클래스를 두는 방법도 있을것 같은데..
이건 또 너무 오버가 아닌가 싶기도 하고 어렵네요...
public class MemberQueryHelper {
public Member getById(Long id) {
return memberRepository.findById(id)
.orElseThrow(...);
}
}
There was a problem hiding this comment.
[제안] 해당 버전 prod 환경에서도 추가가 필요하지 않을까요? (단순한 빈 파일로요)
(작동에는 문제는 없지만, 버저닝에 혼란을 줄 수 있다는 ChatGPT의 의견이 있었습니다.)
There was a problem hiding this comment.
오… 그럴 수도 있겠네요
확실히 Flyway 버저닝 관점에선 prod에도 버전 맞춰두는 게 깔끔하긴 하죠
반영하겠습니다!
| RestDocsRequest requestDocument = request() | ||
| .tag(Tag.STORY_API) | ||
| .summary("스토리 목록 조회") | ||
| .description("스토리 목록을 페이지네이션하여 조회합니다.") | ||
| .requestHeader( | ||
| headerWithName(HttpHeaders.AUTHORIZATION).description("액세스 토큰") | ||
| ); |
There was a problem hiding this comment.
액세스 토큰은 해당 API 에 필요 없으니까 제거해주시면 좋을 것 같아요!
There was a problem hiding this comment.
아! 저희 이거 로그인 안한 사용자도 볼수있는거였죠!?
수정했습니다!
| Response response = given() | ||
| .header("Authorization", accessToken()) | ||
| .when() | ||
| .get("/api/stories"); |
| @Nested | ||
| class GetPagedStoryPreviews extends BaseServiceTest { | ||
|
|
||
| private StoryService storyService; | ||
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| storyService = new StoryService(storeService, imageService, storyRepository, memberRepository); | ||
| } |
There was a problem hiding this comment.
[제안] 제가 머지 작업 진행하면서 파일 위쪽을 바꿔 놓았는데, 머지하시기 전에 확인해주시면 좋을 것 같아요!
| private static final int PAGE_START_NUMBER = 0; | ||
| private static final int PAGE_SIZE = 5; |
There was a problem hiding this comment.
[선택] 현재는 FE 분들과 size는 5개 고정하시기로 이야기 된거죠?
조금 유연하게 만드려면 쿼리 파라미터로 size를 받는 것도 좋을 것 같아요!
There was a problem hiding this comment.
맞습니다.. 5개로 고정해버리기로 이야기 했는데...
사실 이 부분이 기획에 따라 하도 변동이 많을것 같아서 일단 고정해뒀습니다..
1차 이후에 요구사항에 맞춰서 수정해보도록 하겠습니다!
|
/noti 리뷰에 답변 남겼습니다! |
lvalentine6
force-pushed
the
feat/PRODUCT-149
branch
from
2f50a98 to
6978e06
Compare
📄 Terraform Plan Summary🛡️ Common InfrastructureStatus: ✅ No Changes 🛠️ Development EnvironmentStatus: ✅ No Changes 📋 Full Results: View in Actions |
|
📌 최신 ERD가 자동 생성되었습니다. 👉 ERD 보러가기 |
|
|
🎉 This PR is included in version 1.4.0-develop.22 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.5.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
✨ 개요
🧾 관련 이슈
#66
🔍 참고 사항 (선택)
Summary by CodeRabbit
Summary by CodeRabbit
신규 기능
데이터베이스
테스트