feature/#194 게시글 API 프론트 QA 반영 #195
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough이번 PR은 게시글 생성 관련 API와 내부 도메인 로직에 파일 연관 기능을 추가합니다. 구체적으로, Post 생성 메서드들의 시그니처에 Changes
Possibly related issues
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
CodeRabbit Configuration File (
|
Test Coverage Report
Files
|
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop #195 +/- ##
=============================================
- Coverage 93.18% 92.99% -0.20%
Complexity 129 129
=============================================
Files 48 48
Lines 367 371 +4
Branches 3 4 +1
=============================================
+ Hits 342 345 +3
Misses 21 21
- Partials 4 5 +1
Continue to review full report in Codecov by Sentry.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
aics-domain/src/testFixtures/java/comment/application/CommentQueryServiceTest.java (1)
57-59: 테스트 픽스처의 Post.create() 호출이 적절히 수정되었습니다.파일 첨부 기능이 테스트의 주요 관심사가 아니므로
null값을 전달하는 것이 적절합니다.코드의 의도를 명확히 하기 위해 다음과 같은 주석 추가를 고려해보세요:
private static Post saveTestPost() { FakePostRepository fakePostRepository = new FakePostRepository(); + // 댓글 테스트에서는 파일 첨부가 필요하지 않으므로 null 전달 Post commentedPost = Post.create("SW 부트캠프 4기 교육생 모집", "SW전문인재양성사업단에서는 SW부트캠프 4기 교육생을 모집합니다.", NEWS, User.builder().build(), null); return fakePostRepository.save(commentedPost); }aics-api/src/main/java/kgu/developers/api/post/presentation/response/PostSummaryResponse.java (1)
45-50: 문자열 처리 로직을 개선해 주세요.description 생성 로직에 다음과 같은 개선이 필요합니다:
- 매직 넘버 30을 상수로 분리
- 문자열 처리 로직을 별도의 유틸리티 메서드로 분리
- try-catch 대신 더 간단한 방식으로 구현
다음과 같이 개선해 보세요:
+private static final int DESCRIPTION_MAX_LENGTH = 30; + +private static String truncateContent(String content) { + return content.length() <= DESCRIPTION_MAX_LENGTH + ? content + : content.substring(0, DESCRIPTION_MAX_LENGTH); +} public static PostSummaryResponse from(Post post) { DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd"); - String description; - try { - description = post.getContent().substring(0, 30); - } catch (StringIndexOutOfBoundsException e) { - description = post.getContent(); - } + String description = truncateContent(post.getContent());aics-domain/src/main/java/kgu/developers/domain/post/application/command/PostCommandService.java (1)
21-31: 성능 최적화 제안컨트롤러에서 기본값으로 0을 전달할 때 불필요한 데이터베이스 조회가 발생할 수 있습니다.
다음과 같이 개선하는 것을 고려해보세요:
public Long createPost(String title, String content, Category category, Long fileId) { User author = userQueryService.me(); FileEntity file = null; - try { - file = fileQueryService.getFileById(fileId); - } catch (FileNotFoundException ignored) { - } + if (fileId != null && fileId > 0) { + try { + file = fileQueryService.getFileById(fileId); + } catch (FileNotFoundException e) { + log.debug("File not found for ID: {}. Creating post without file.", fileId); + } + } Post post = Post.create(title, content, category, author, file); return postRepository.save(post).getId(); }aics-admin/src/main/java/kgu/developers/admin/post/presentation/PostAdminControllerImpl.java (1)
32-33: API 설계 개선 제안파일 ID가 없는 경우를 나타내기 위해 0 대신 null을 사용하는 것이 더 명시적입니다.
다음과 같이 변경을 제안합니다:
- @RequestParam(required = false, defaultValue = "0") Long fileId, + @RequestParam(required = false) Long fileId,또한 API 문서화를 위해 파라미터에 대한 설명을 추가하는 것이 좋습니다:
@ApiParam(value = "첨부 파일 ID", example = "1") @RequestParam(required = false) Long fileIdaics-domain/src/testFixtures/java/post/domain/PostDomainTest.java (1)
29-29: 파일 관련 테스트 케이스 추가가 필요합니다.현재
null파일로만 테스트가 되어 있습니다. 다음과 같은 테스트 케이스 추가를 고려해주세요:
- 유효한 파일이 있는 경우의 Post 생성
- 파일 연관 관계 검증
+@Test +@DisplayName("파일이 있는 POST 객체를 생성할 수 있다") +public void createPostWithFile_Success() { + // given + FileEntity file = new FileEntity("test.txt", "text/plain", 1024L); + + // when + Post postWithFile = Post.create(TITLE, CONTENT, NEWS, user(), file); + + // then + assertNotNull(postWithFile); + assertEquals(file, postWithFile.getFile()); +}aics-admin/src/main/java/kgu/developers/admin/post/presentation/PostAdminController.java (1)
31-35: fileId 파라미터 처리 방식 개선이 필요합니다.현재 구현에서 몇 가지 개선 사항을 제안드립니다:
defaultValue = "0"은 유효하지 않은 ID를 의미할 수 있으므로,null을 사용하는 것이 더 명확할 수 있습니다.- 파라미터 설명에 기본값과 유효하지 않은 ID 처리 방식에 대한 설명을 추가하면 좋을 것 같습니다.
@Parameter( description = """ - 게시글에 저장할 파일의 ID 입니다. + 게시글에 저장할 파일의 ID 입니다. + - 파일이 없는 경우 생략 가능합니다. + - 유효하지 않은 ID가 전달된 경우 FileNotFoundException이 발생합니다. """, example = "1" -) @RequestParam(required = false, defaultValue = "0") Long fileId, +) @RequestParam(required = false) Long fileId,aics-domain/src/testFixtures/java/post/application/PostQueryServiceTest.java (1)
35-59: 파일이 있는 게시글에 대한 테스트 케이스 추가가 필요합니다.테스트 데이터 설정에서 모든 게시글이
file = null로 생성되어 있습니다. 다음과 같은 테스트 케이스 추가를 고려해주세요:
- 파일이 있는 게시글 조회
- 파일이 있는 게시글과 없는 게시글이 혼합된 상태에서의 페이징 조회
+// 파일이 있는 게시글 추가 +FileEntity testFile = new FileEntity("test.txt", "text/plain", 1024L); +fakePostRepository.save(Post.create( + "파일 첨부 게시글", "테스트용 내용", + NEWS, author, testFile +));aics-domain/src/testFixtures/java/post/application/PostCommandServiceTest.java (1)
65-68: 테스트 데이터 명확성 개선 필요fileId 값으로 1L을 하드코딩하고 있습니다. 테스트의 의도를 더 명확하게 전달하기 위해 상수로 분리하는 것이 좋겠습니다.
+ private static final Long TEST_FILE_ID = 1L; // ... - Long fileId = 1L; + Long fileId = TEST_FILE_ID;aics-api/src/testFixtures/java/post/application/PostFacadeTest.java (1)
70-70: 테스트 데이터 설정 개선 필요여러 Post 생성 시 모두 file을 null로 설정하고 있습니다. 다양한 파일 상태를 테스트하기 위해 일부 Post에는 파일을 연결하는 것이 좋겠습니다.
Also applies to: 74-74, 79-79
aics-admin/src/testFixtures/java/post/application/PostAdminFacadeTest.java (1)
67-67: Post.create 메서드에서 file 파라미터가 null로 전달되는 이유를 명시해주세요.테스트의 의도를 명확히 하기 위해 null 사용에 대한 주석을 추가하는 것이 좋습니다.
- "post title", "post content", NOTIFICATION, author, null + "post title", "post content", NOTIFICATION, author, null // 파일이 없는 게시글 생성 테스트
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
aics-admin/src/main/java/kgu/developers/admin/post/application/PostAdminFacade.java(1 hunks)aics-admin/src/main/java/kgu/developers/admin/post/presentation/PostAdminController.java(2 hunks)aics-admin/src/main/java/kgu/developers/admin/post/presentation/PostAdminControllerImpl.java(3 hunks)aics-admin/src/main/java/kgu/developers/admin/post/presentation/request/PostRequest.java(1 hunks)aics-admin/src/testFixtures/java/post/application/PostAdminFacadeTest.java(4 hunks)aics-api/src/main/java/kgu/developers/api/post/presentation/response/PostSummaryPageResponse.java(1 hunks)aics-api/src/main/java/kgu/developers/api/post/presentation/response/PostSummaryResponse.java(3 hunks)aics-api/src/testFixtures/java/comment/application/CommentFacadeTest.java(1 hunks)aics-api/src/testFixtures/java/post/application/PostFacadeTest.java(3 hunks)aics-domain/src/main/java/kgu/developers/domain/post/application/command/PostCommandService.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/post/domain/Post.java(1 hunks)aics-domain/src/testFixtures/java/comment/application/CommentCommandServiceTest.java(1 hunks)aics-domain/src/testFixtures/java/comment/application/CommentQueryServiceTest.java(1 hunks)aics-domain/src/testFixtures/java/comment/domain/CommentDomainTest.java(1 hunks)aics-domain/src/testFixtures/java/mock/FakeTestContainer.java(3 hunks)aics-domain/src/testFixtures/java/mock/TestContainer.java(1 hunks)aics-domain/src/testFixtures/java/post/application/PostCommandServiceTest.java(3 hunks)aics-domain/src/testFixtures/java/post/application/PostQueryServiceTest.java(1 hunks)aics-domain/src/testFixtures/java/post/domain/PostDomainTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- aics-admin/src/main/java/kgu/developers/admin/post/presentation/request/PostRequest.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (13)
aics-api/src/main/java/kgu/developers/api/post/presentation/response/PostSummaryPageResponse.java (1)
17-24: API 문서 예제가 적절하게 업데이트되었습니다!스키마 예제에 새로운 필드들이 잘 추가되었고, 예제 값들도 실제 사용 사례를 잘 반영하고 있습니다.
aics-api/src/main/java/kgu/developers/api/post/presentation/response/PostSummaryResponse.java (1)
59-59: 첨부 파일 여부 확인 로직이 필요합니다.TODO 주석이 있는 부분을 구현해야 합니다. PR 목적에 따르면
fileId가 추가되었으므로, 이를 활용하여 첨부 파일 여부를 확인하는 로직이 필요합니다.첨부 파일 여부를 확인하는 로직 구현이 필요하신가요? 도움이 필요하시다면 구체적인 구현 방안을 제안해 드릴 수 있습니다.
aics-domain/src/testFixtures/java/comment/domain/CommentDomainTest.java (1)
43-44: LGTM!
Post.create()메서드의 시그니처 변경에 맞춰 테스트 코드가 적절히 수정되었습니다.aics-admin/src/main/java/kgu/developers/admin/post/application/PostAdminFacade.java (1)
21-24: LGTM!서비스 계층의 변경사항이 Facade 계층에 올바르게 반영되었습니다.
aics-domain/src/main/java/kgu/developers/domain/post/domain/Post.java (2)
60-62: 파일 엔티티와의 관계 설정이 적절합니다.
@OneToOne관계와@JoinColumn을 사용한 매핑이 잘 구현되어 있습니다.
68-78: 정적 팩토리 메서드가 잘 구현되어 있습니다.파일 엔티티를 포함하도록 수정된
create메서드가 적절하게 구현되어 있습니다.aics-domain/src/testFixtures/java/comment/application/CommentCommandServiceTest.java (1)
53-53: 파일 엔티티 매개변수 처리 검토 필요테스트에서
Post.create에 null을 전달하고 있습니다. 파일이 없는 경우에 대한 명시적인 테스트 케이스를 추가하는 것이 좋겠습니다.aics-domain/src/testFixtures/java/post/application/PostCommandServiceTest.java (1)
33-34: 🛠️ Refactor suggestion파일 관련 테스트 케이스 보강 필요
파일 처리 기능이 추가되었지만, 다음과 같은 테스트 케이스가 누락되어 있습니다:
- 유효하지 않은 fileId를 전달하는 경우
- fileId가 null인 경우
- 파일이 존재하지 않는 경우
Also applies to: 37-40
aics-api/src/testFixtures/java/comment/application/CommentFacadeTest.java (1)
65-65: LGTM!댓글 테스트에서 Post 생성 시 파일 매개변수를 null로 전달하는 것이 적절합니다.
aics-api/src/testFixtures/java/post/application/PostFacadeTest.java (1)
44-47: 🛠️ Refactor suggestion파일 조회 서비스 통합 테스트 필요
FileQueryService가 추가되었지만, 파일 조회 관련 테스트 케이스가 누락되어 있습니다. 다음과 같은 테스트를 추가해주세요:
- 파일이 있는 게시글 조회
- 파일이 없는 게시글 조회
- 삭제된 파일이 연결된 게시글 조회
Also applies to: 50-50
aics-domain/src/testFixtures/java/mock/FakeTestContainer.java (1)
74-75: PostCommandService 생성자에 FileQueryService 의존성이 추가되었습니다.게시글 생성 시 파일 연관 기능이 추가됨에 따라 PostCommandService가 FileQueryService에 의존하게 되었습니다.
aics-admin/src/testFixtures/java/post/application/PostAdminFacadeTest.java (1)
81-85: fileId 유효성 검증이 필요합니다.현재 테스트에서는 fileId=1L을 하드코딩하여 사용하고 있습니다. 파일의 존재 여부를 검증하지 않으면 실제 환경에서 FileNotFoundException이 발생할 수 있습니다.
파일의 존재 여부를 검증하는 테스트 케이스를 추가하는 것을 권장합니다:
- 존재하지 않는 fileId로 게시글 생성 시 예외 발생 테스트
- null fileId로 게시글 생성 시 정상 동작 테스트
aics-domain/src/testFixtures/java/mock/TestContainer.java (1)
74-76: 파일 관련 서비스들이 적절한 순서로 초기화되었습니다.파일 서비스들이 PostCommandService에서 사용되기 전에 초기화되도록 순서가 잘 구성되어 있습니다.
Also applies to: 115-121
| FileEntity file = null; | ||
| try { | ||
| file = fileQueryService.getFileById(fileId); | ||
| } catch (FileNotFoundException ignored) { | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
예외 처리 로직 개선 필요
빈 catch 블록은 잠재적인 문제를 숨길 수 있습니다. 파일을 찾을 수 없는 경우에 대한 처리를 명확히 하는 것이 좋습니다.
다음과 같이 개선을 제안합니다:
FileEntity file = null;
try {
- file = fileQueryService.getFileById(fileId);
+ file = (fileId != null && fileId > 0) ? fileQueryService.getFileById(fileId) : null;
} catch (FileNotFoundException e) {
-
+ // 로그 추가
+ log.debug("File not found for ID: {}. Creating post without file.", fileId);
}📝 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.
| FileEntity file = null; | |
| try { | |
| file = fileQueryService.getFileById(fileId); | |
| } catch (FileNotFoundException ignored) { | |
| } | |
| FileEntity file = null; | |
| try { | |
| file = (fileId != null && fileId > 0) ? fileQueryService.getFileById(fileId) : null; | |
| } catch (FileNotFoundException e) { | |
| // 로그 추가 | |
| log.debug("File not found for ID: {}. Creating post without file.", fileId); | |
| } |
aics-admin/src/main/java/kgu/developers/admin/post/presentation/PostAdminController.java
Outdated
Show resolved
Hide resolved
aics-admin/src/main/java/kgu/developers/admin/post/presentation/PostAdminControllerImpl.java
Outdated
Show resolved
Hide resolved
aics-api/src/main/java/kgu/developers/api/post/presentation/response/PostSummaryResponse.java
Outdated
Show resolved
Hide resolved
...-domain/src/main/java/kgu/developers/domain/post/application/command/PostCommandService.java
Outdated
Show resolved
Hide resolved
Summary
FE에서 요청한 게시글 API QA를 반영합니다.
Tasks
To Reviewer
Post 생성 AP에 fileId를 추가하는 과정에서 null 값이 넘어오는 경우에 대해 예외처리를 할 때,
파급의 길이가 길고, 다소 투박하게 처리되는 것 같은 느낌을 받았습니다.
혹시 개선할만한 부분에 대해서 아이디어 주시면 감사하겠습니다!