[Feat] 가게의 담긴 이야기 기능 구현 #93
Conversation
|
""" Walkthrough이번 변경사항에서는 "Article" 엔티티 관련 API, 서비스, 레포지토리, DTO, 테스트 및 문서화가 새롭게 도입되었습니다. 컨트롤러, 서비스, 레포지토리 계층이 추가되었으며, 이에 대한 단위 테스트, 통합 테스트, 문서화 테스트, 그리고 테스트용 fixture 클래스가 함께 구현되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ArticleController
participant ArticleService
participant ArticleRepository
participant ImageService
Client->>ArticleController: GET /api/articles?size={size}
ArticleController->>ArticleService: getAllArticles(size)
ArticleService->>ArticleRepository: findAllByOrderByCreatedAtDesc(PageRequest)
ArticleRepository-->>ArticleService: List<Article>
loop for each Article
ArticleService->>ImageService: getPresignedUrl(imageKey)
ImageService-->>ArticleService: imageUrl
end
ArticleService-->>ArticleController: ArticlesResponse
ArticleController-->>Client: ResponseEntity<ArticlesResponse> (HTTP 200)
Estimated code review effort3 (~40 minutes) Suggested labels
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
src/main/java/eatda/controller/article/ArticlesResponse.java (1)
5-8: 리스트 불변성 확보를 고려해 보세요
record자체는 불변이지만, 필드로 받은List객체는 외부에서 수정될 수 있습니다. 방어적 복사를 통해 불변성을 완전히 보장하는 것을 권장합니다.public record ArticlesResponse( - List<ArticleResponse> articles +) { + public ArticlesResponse(List<ArticleResponse> articles) { + this.articles = List.copyOf(articles); + } }src/main/java/eatda/controller/article/ArticleResponse.java (1)
5-12: URL 필드 타입 검토 권장
articleUrl,imageUrl은 단순String대신java.net.URI또는URL타입을 고려하면 타입 안전성과 파싱 오류를 컴파일 타임에 방지할 수 있습니다. API 스펙 상 문자열로 충분하다면 유지해도 무방하지만, 내부 로직에서 URL 조작이 필요하다면 변경을 권장드립니다.src/main/java/eatda/controller/article/ArticleController.java (1)
13-24: @RestController 사용을 고려해보세요.현재 @controller와 ResponseEntity를 함께 사용하고 있는데, REST API의 경우 @RestController를 사용하면 더 간결하게 구현할 수 있습니다.
-@Controller +@RestController @RequiredArgsConstructor public class ArticleController { private final ArticleService articleService; @GetMapping("/api/articles") - public ResponseEntity<ArticlesResponse> getArticles(@RequestParam(defaultValue = "3") @Min(1) @Max(50) int size) { - return ResponseEntity.status(HttpStatus.OK) - .body(articleService.getAllArticles(size)); - } + public ArticlesResponse getArticles(@RequestParam(defaultValue = "3") @Min(1) @Max(50) int size) { + return articleService.getAllArticles(size); + }src/main/java/eatda/service/article/ArticleService.java (1)
19-34: 페이지네이션 로직 개선이 필요합니다.현재 구현에서 페이지 번호가 0으로 하드코딩되어 있어 첫 번째 페이지만 조회할 수 있습니다. 향후 확장성을 고려하여 페이지 번호도 파라미터로 받을 수 있도록 개선을 제안합니다.
-public ArticlesResponse getAllArticles(int size) { - PageRequest pageRequest = PageRequest.of(0, size); +public ArticlesResponse getAllArticles(int page, int size) { + PageRequest pageRequest = PageRequest.of(page, size);또한 size 파라미터에 대한 유효성 검증 추가를 고려해보세요:
public ArticlesResponse getAllArticles(int size) { + if (size <= 0) { + throw new IllegalArgumentException("Size must be greater than 0"); + } PageRequest pageRequest = PageRequest.of(0, size);src/test/java/eatda/document/article/ArticleDocumentTest.java (1)
44-61: 테스트 데이터의 시간 값을 고정하는 것을 고려해보세요.Mock 응답에서
LocalDateTime.now()를 사용하고 있어 테스트 실행 시점에 따라 다른 값이 생성됩니다. 테스트의 일관성을 위해 고정된 시간 값 사용을 제안합니다.- LocalDateTime.now() + LocalDateTime.of(2024, 1, 1, 12, 0, 0)- LocalDateTime.now() + LocalDateTime.of(2024, 1, 2, 12, 0, 0)src/test/java/eatda/fixture/ArticleGenerator.java (1)
18-20: 생성자 주입 방식을 Lombok으로 통일하는 것을 고려해보세요.서비스 레이어에서는
@RequiredArgsConstructor를 사용하고 있는데, 일관성을 위해 이 클래스에서도 동일한 방식을 사용하는 것을 제안합니다.+import lombok.RequiredArgsConstructor; @Component +@RequiredArgsConstructor public class ArticleGenerator { - private final ArticleRepository articleRepository; - - public ArticleGenerator(ArticleRepository articleRepository) { - this.articleRepository = articleRepository; - } + private final ArticleRepository articleRepository;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/eatda/controller/article/ArticleController.java(1 hunks)src/main/java/eatda/controller/article/ArticleResponse.java(1 hunks)src/main/java/eatda/controller/article/ArticlesResponse.java(1 hunks)src/main/java/eatda/repository/article/ArticleRepository.java(1 hunks)src/main/java/eatda/service/article/ArticleService.java(1 hunks)src/test/java/eatda/controller/BaseControllerTest.java(2 hunks)src/test/java/eatda/controller/article/ArticleControllerTest.java(1 hunks)src/test/java/eatda/document/BaseDocumentTest.java(2 hunks)src/test/java/eatda/document/Tag.java(1 hunks)src/test/java/eatda/document/article/ArticleDocumentTest.java(1 hunks)src/test/java/eatda/fixture/ArticleGenerator.java(1 hunks)src/test/java/eatda/service/BaseServiceTest.java(3 hunks)src/test/java/eatda/service/article/ArticleServiceTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/test/java/eatda/controller/article/ArticleControllerTest.java (1)
Learnt from: leegwichan
PR: #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/controller/article/ArticleControllerTest.java (2)
src/test/java/eatda/document/article/ArticleDocumentTest.java (1)
Nested(22-80)src/test/java/eatda/service/article/ArticleServiceTest.java (1)
Nested(18-38)
🔇 Additional comments (12)
src/test/java/eatda/document/Tag.java (1)
10-10: 새 태그 추가 확인 완료
ARTICLE_API상수 추가로 문서 태그 체계가 잘 확장되었습니다. 다른 테스트/문서 클래스에서 해당 태그를 사용하도록만 점검해 주시면 될 것 같습니다.src/test/java/eatda/document/BaseDocumentTest.java (1)
60-64: ArticleService 목 객체 스텁 확인 요청
ArticleService를 목(mock)으로 주입하셨지만, 기본 스텁이 없는 경우 해당 서비스 호출부에서null반환으로 NPE가 발생할 수 있습니다. 실제 테스트들(ArticleDocumentTest등)에서 필요한 동작을when(...).thenReturn(...)으로 명시적으로 정의했는지 다시 한 번 확인해 주세요.src/test/java/eatda/controller/BaseControllerTest.java (1)
62-64: ArticleGenerator 주입 확인 완료새로운
ArticleGenerator픽스처가 BaseControllerTest에 잘 통합되었습니다. 기존 생성자 패턴과 일관성을 유지하고 있어 문제 없습니다.src/test/java/eatda/service/BaseServiceTest.java (4)
10-10: LGTM! Article 관련 import 추가가 적절합니다.ArticleGenerator import가 다른 Generator들과 일관성 있게 추가되었습니다.
14-14: LGTM! ArticleRepository import 추가가 적절합니다.ArticleRepository import가 다른 Repository들과 일관성 있게 추가되었습니다.
50-51: LGTM! ArticleGenerator 의존성 주입이 적절합니다.다른 Generator들과 동일한 패턴으로 @Autowired를 통해 ArticleGenerator를 주입하고 있습니다.
62-63: LGTM! ArticleRepository 의존성 주입이 적절합니다.다른 Repository들과 동일한 패턴으로 @Autowired를 통해 ArticleRepository를 주입하고 있습니다.
src/main/java/eatda/repository/article/ArticleRepository.java (1)
1-12: LGTM! ArticleRepository 구현이 적절합니다.Spring Data JPA의 표준 패턴을 잘 따르고 있으며, 메서드 네이밍 규칙과 페이징 지원이 올바르게 구현되었습니다.
src/main/java/eatda/service/article/ArticleService.java (1)
21-31: 스트림 처리 로직이 올바르게 구현되었습니다.Repository에서 조회한 엔티티를 DTO로 변환하는 과정이 깔끔하게 구현되었으며, ImageService를 통한 presigned URL 생성도 적절히 처리되었습니다.
src/test/java/eatda/document/article/ArticleDocumentTest.java (2)
25-40: REST Docs 문서화 설정이 잘 구성되었습니다.요청 파라미터와 응답 필드에 대한 문서화가 상세하게 정의되어 있으며, 한국어 설명이 명확합니다. API 문서 생성에 필요한 모든 요소가 포함되었습니다.
67-79: API 테스트 실행 로직이 적절합니다.REST Docs 필터 설정과 API 호출, 응답 검증이 올바르게 구현되었습니다. 문서 생성과 테스트 검증이 함께 이루어지는 구조가 좋습니다.
src/test/java/eatda/fixture/ArticleGenerator.java (1)
22-41: 메서드 오버로딩을 통한 유연한 Article 생성 로직이 잘 구현되었습니다.기본값부터 모든 필드를 지정하는 방식까지 단계적으로 오버로딩된 메서드들이 테스트에서 다양한 시나리오를 지원할 수 있도록 잘 설계되었습니다.
leegwichan
left a comment
leegwichan
left a comment
There was a problem hiding this comment.
/noti 부탁드렸던 '가게에 담긴 이야기' 기능 구현해주셔서 감사합니다!
간단한 코멘트 몇 개 남겼으니 확인 부탁드릴께요!
| public record ArticleResponse( | ||
| Long id, | ||
| String title, | ||
| String subtitle, | ||
| String articleUrl, | ||
| String imageUrl, | ||
| LocalDateTime createdAt | ||
| ) { | ||
| } |
There was a problem hiding this comment.
[진짜 궁금한 질문] id, createdAt 은 FE에서 따로 사용하지 않는 것 같은데, 혹시 추가하신 이유가 있을까요?
There was a problem hiding this comment.
일단... 필요할지도 모르니 다 주려고 하긴 했는데 지금은 필요없는것 같아서 삭제할께요 😅
| assertThat(response.articles()).hasSize(3); | ||
| List<String> titles = response.articles().stream() | ||
| .map(ArticleResponse::title) | ||
| .toList(); | ||
| assertThat(titles).containsExactly( | ||
| "아티클 제목 5", | ||
| "아티클 제목 4", | ||
| "아티클 제목 3" | ||
| ); |
There was a problem hiding this comment.
[제안] 이런 Assertion 구문도 있는데, 어떻게 생각하시나요?
| assertThat(response.articles()).hasSize(3); | |
| List<String> titles = response.articles().stream() | |
| .map(ArticleResponse::title) | |
| .toList(); | |
| assertThat(titles).containsExactly( | |
| "아티클 제목 5", | |
| "아티클 제목 4", | |
| "아티클 제목 3" | |
| ); | |
| assertThat(response.articles()).hasSize(3) | |
| .extracting(ArticleResponse::title) | |
| .containsExactly("아티클 제목 5", "아티클 제목 4", "아티클 제목 3"); |
There was a problem hiding this comment.
좋습니다! 이전에도 저 메서드가 머리속에 있었는데 다시 기억해둬야겠군요 😅
|
/noti 리뷰 반영했습니다! |
|
|
🎉 This PR is included in version 1.4.0-develop.26 🎉 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 📦🚀 |
✨ 개요
🧾 관련 이슈
#88
🔍 참고 사항 (선택)
Summary by CodeRabbit
신규 기능
테스트