Develop #95
Merged
Develop #95
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…og_Server into feat/#60-editPost-ys
Contributor
Walkthrough게시글 수정 기능이 새롭게 추가되었습니다. 이를 위해 게시글 수정용 DTO와 컨트롤러 엔드포인트가 도입되었고, 도메인 엔티티와 서비스 계층에 필드별 수정 메서드 및 태그 비교, 유효성 검사, 수정 로직이 구현되었습니다. 예외 코드도 일부 보완되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PostController
participant PostService
participant PostRepository
participant TagRepository
participant Post
Client->>PostController: PATCH /posts/{postId}/edit (NewPostRequestDto)
PostController->>PostService: editPost(postId, newPostRequestDto)
PostService->>PostRepository: findById(postId)
PostService->>TagRepository: findAllById(newTagIds)
PostService->>Post: changeTitle(), changeCoverImage(), ... (필드별 변경)
PostService->>Post: clearTags(), addTags()
PostService->>Post: updateEditedAt()
PostService-->>PostController: void (수정 완료)
PostController-->>Client: 성공 응답 반환
Poem
✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/main/java/com/example/FixLog/service/PostService.java (2)
182-194: 변경사항 확인 로직 개선 필요현재 로직은 가독성이 떨어지고 유지보수가 어렵습니다.
별도의 메서드로 분리하여 개선할 수 있습니다.-// 아무것도 변경이 없으면 예외처리 -if (Objects.equals(post.getPostTitle(), newPostRequestDto.getPostTitle()) - & Objects.equals(post.getCoverImage(), newPostRequestDto.getCoverImageUrl()) - & Objects.equals(post.getProblem(), newPostRequestDto.getProblem()) - & Objects.equals(post.getErrorMessage(), newPostRequestDto.getErrorMessage()) - & Objects.equals(post.getEnvironment(), newPostRequestDto.getEnvironment()) - & Objects.equals(post.getReproduceCode(), newPostRequestDto.getReproduceCode()) - & Objects.equals(post.getSolutionCode(), newPostRequestDto.getSolutionCode()) - & Objects.equals(post.getCauseAnalysis(), newPostRequestDto.getCauseAnalysis()) - & Objects.equals(post.getReferenceLink(), newPostRequestDto.getReferenceLink()) - & Objects.equals(post.getExtraContent(), newPostRequestDto.getExtraContent()) - & compareTags(post.getPostTags(), tags)){ - throw new CustomException(ErrorCode.NO_CONTENT_CHANGED); -} +// 아무것도 변경이 없으면 예외처리 +if (hasNoChanges(post, newPostRequestDto, tags)) { + throw new CustomException(ErrorCode.NO_CONTENT_CHANGED); +}그리고 다음 메서드를 추가:
private boolean hasNoChanges(Post post, NewPostRequestDto dto, List<Tag> newTags) { return Objects.equals(post.getPostTitle(), dto.getPostTitle()) && Objects.equals(post.getCoverImage(), dto.getCoverImageUrl()) && Objects.equals(post.getProblem(), dto.getProblem()) && Objects.equals(post.getErrorMessage(), dto.getErrorMessage()) && Objects.equals(post.getEnvironment(), dto.getEnvironment()) && Objects.equals(post.getReproduceCode(), dto.getReproduceCode()) && Objects.equals(post.getSolutionCode(), dto.getSolutionCode()) && Objects.equals(post.getCauseAnalysis(), dto.getCauseAnalysis()) && Objects.equals(post.getReferenceLink(), dto.getReferenceLink()) && Objects.equals(post.getExtraContent(), dto.getExtraContent()) && compareTags(post.getPostTags(), newTags); }
152-157: 코드 중복 제거 권장
validatePost메서드가 중복되어 있습니다. 제네릭이나 상속을 활용하여 중복을 제거할 수 있습니다.두 DTO가 공통 인터페이스를 구현하도록 하거나, 하나의 DTO를 재사용하는 것을 고려해보세요.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/example/FixLog/controller/PostController.java(2 hunks)src/main/java/com/example/FixLog/domain/post/Post.java(1 hunks)src/main/java/com/example/FixLog/dto/post/NewPostRequestDto.java(1 hunks)src/main/java/com/example/FixLog/exception/ErrorCode.java(1 hunks)src/main/java/com/example/FixLog/service/PostService.java(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
src/main/java/com/example/FixLog/exception/ErrorCode.java (1)
33-34: 적절한 에러 코드 추가입니다!HTTP 상태 코드 변경과 새로운 에러 코드 추가가 모두 적절합니다:
IMAGE_UPLOAD_FAILED를BAD_REQUEST로 변경한 것은 클라이언트 측 문제임을 명확히 함NO_CONTENT_CHANGED는 게시글 수정 시 유용한 검증 기능을 제공src/main/java/com/example/FixLog/controller/PostController.java (1)
35-40: 서비스 계층editPost메서드 전체에서 권한 검증 여부 확인#!/bin/bash # editPost 메서드 전체 구현 출력 (권한 검증 로직 확인용) rg -n "public void editPost" -A100 src/main/java/com/example/FixLog/service/PostService.javasrc/main/java/com/example/FixLog/domain/post/Post.java (1)
82-118: 도메인 로직 구조 개선현재 구조는 적절하게 구현되어 있습니다. 각 필드별 수정 메서드가 명확하고 일관성 있게 작성되었습니다.
Comment on lines
+8
to
+21
| public class NewPostRequestDto { | ||
| private String postTitle; | ||
| private String coverImageUrl; | ||
| private String problem; | ||
| private String errorMessage; | ||
| private String environment; | ||
| private String reproduceCode; | ||
| private String solutionCode; | ||
| private String causeAnalysis; | ||
| private String referenceLink; | ||
| private String extraContent; | ||
|
|
||
| private List<Long> tags; | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
DTO 레벨 검증 추가 권장
현재 DTO에 검증 어노테이션이 없어 모든 검증을 서비스 계층에서 수행해야 합니다.
필수 필드에 대한 기본적인 검증을 DTO 레벨에서 수행하면 더 견고한 코드가 됩니다.
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.Size;
@Getter
public class NewPostRequestDto {
+ @NotBlank(message = "제목은 필수입니다")
+ @Size(max = 100, message = "제목은 100자 이내여야 합니다")
private String postTitle;
private String coverImageUrl;
+ @NotBlank(message = "문제 설명은 필수입니다")
private String problem;
+ @NotBlank(message = "에러 메시지는 필수입니다")
private String errorMessage;
+ @NotBlank(message = "환경 정보는 필수입니다")
private String environment;
+ @NotBlank(message = "재현 코드는 필수입니다")
private String reproduceCode;
+ @NotBlank(message = "해결 코드는 필수입니다")
private String solutionCode;
private String causeAnalysis;
private String referenceLink;
private String extraContent;
+ @NotNull(message = "태그는 필수입니다")
+ @Size(min = 1, message = "최소 하나의 태그를 선택해야 합니다")
private List<Long> tags;
}📝 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
| public class NewPostRequestDto { | |
| private String postTitle; | |
| private String coverImageUrl; | |
| private String problem; | |
| private String errorMessage; | |
| private String environment; | |
| private String reproduceCode; | |
| private String solutionCode; | |
| private String causeAnalysis; | |
| private String referenceLink; | |
| private String extraContent; | |
| private List<Long> tags; | |
| } | |
| import jakarta.validation.constraints.NotBlank; | |
| import jakarta.validation.constraints.NotNull; | |
| import jakarta.validation.constraints.Size; | |
| @Getter | |
| public class NewPostRequestDto { | |
| @NotBlank(message = "제목은 필수입니다") | |
| @Size(max = 100, message = "제목은 100자 이내여야 합니다") | |
| private String postTitle; | |
| private String coverImageUrl; | |
| @NotBlank(message = "문제 설명은 필수입니다") | |
| private String problem; | |
| @NotBlank(message = "에러 메시지는 필수입니다") | |
| private String errorMessage; | |
| @NotBlank(message = "환경 정보는 필수입니다") | |
| private String environment; | |
| @NotBlank(message = "재현 코드는 필수입니다") | |
| private String reproduceCode; | |
| @NotBlank(message = "해결 코드는 필수입니다") | |
| private String solutionCode; | |
| private String causeAnalysis; | |
| private String referenceLink; | |
| private String extraContent; | |
| @NotNull(message = "태그는 필수입니다") | |
| @Size(min = 1, message = "최소 하나의 태그를 선택해야 합니다") | |
| private List<Long> tags; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/FixLog/dto/post/NewPostRequestDto.java between
lines 8 and 21, the DTO lacks validation annotations, requiring all validation
to be done in the service layer. Add appropriate validation annotations such as
@NotNull or @NotBlank on mandatory fields to enforce basic validation at the DTO
level, improving code robustness and reducing service layer validation burden.
Comment on lines
+173
to
+177
| public void editPost(Long postId, NewPostRequestDto newPostRequestDto) { | ||
| Member member = memberService.getCurrentMemberInfo(); | ||
| Post post = postRepository.findById(postId) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.POST_NOT_FOUND)); | ||
|
|
Contributor
There was a problem hiding this comment.
중대한 보안 문제: 작성자 권한 검증 누락
현재 코드는 로그인한 사용자 정보만 가져올 뿐, 게시글 작성자와 비교하지 않습니다.
누구나 다른 사람의 게시글을 수정할 수 있는 심각한 보안 취약점이 있습니다.
@Transactional
public void editPost(Long postId, NewPostRequestDto newPostRequestDto) {
Member member = memberService.getCurrentMemberInfo();
Post post = postRepository.findById(postId)
.orElseThrow(() -> new CustomException(ErrorCode.POST_NOT_FOUND));
+
+ // 게시글 작성자 확인
+ if (!post.getUserId().equals(member)) {
+ throw new CustomException(ErrorCode.ACCESS_DENIED);
+ }📝 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
| public void editPost(Long postId, NewPostRequestDto newPostRequestDto) { | |
| Member member = memberService.getCurrentMemberInfo(); | |
| Post post = postRepository.findById(postId) | |
| .orElseThrow(() -> new CustomException(ErrorCode.POST_NOT_FOUND)); | |
| @Transactional | |
| public void editPost(Long postId, NewPostRequestDto newPostRequestDto) { | |
| Member member = memberService.getCurrentMemberInfo(); | |
| Post post = postRepository.findById(postId) | |
| .orElseThrow(() -> new CustomException(ErrorCode.POST_NOT_FOUND)); | |
| // 게시글 작성자 확인 | |
| if (!post.getUserId().equals(member)) { | |
| throw new CustomException(ErrorCode.ACCESS_DENIED); | |
| } | |
| // ... the rest of your update logic ... | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/FixLog/service/PostService.java around lines 173 to
177, the code fetches the current logged-in user but does not verify if this
user is the author of the post being edited. To fix this critical security
issue, add a check comparing the current member's ID with the post author's ID
after retrieving the post. If they do not match, throw an appropriate exception
to prevent unauthorized edits.
Comment on lines
+86
to
+88
| public void changeTitle(String newTitle){ | ||
| this.postTitle = newTitle; | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
필드 검증 로직 추가 권장
현재 setter 메서드들이 검증 없이 값을 직접 설정합니다.
최소한 필수 필드에 대한 null/blank 검증을 추가하는 것이 좋습니다.
public void changeTitle(String newTitle){
+ if (newTitle == null || newTitle.isBlank()) {
+ throw new IllegalArgumentException("제목은 비어있을 수 없습니다");
+ }
+ if (newTitle.length() > 100) {
+ throw new IllegalArgumentException("제목은 100자를 초과할 수 없습니다");
+ }
this.postTitle = newTitle;
}📝 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
| public void changeTitle(String newTitle){ | |
| this.postTitle = newTitle; | |
| } | |
| public void changeTitle(String newTitle){ | |
| if (newTitle == null || newTitle.isBlank()) { | |
| throw new IllegalArgumentException("제목은 비어있을 수 없습니다"); | |
| } | |
| if (newTitle.length() > 100) { | |
| throw new IllegalArgumentException("제목은 100자를 초과할 수 없습니다"); | |
| } | |
| this.postTitle = newTitle; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/FixLog/domain/post/Post.java around lines 86 to 88,
the changeTitle method sets the postTitle field without validation. Add
validation logic to check that newTitle is not null or blank before assigning it
to postTitle, and handle invalid input appropriately, such as throwing an
IllegalArgumentException.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
이슈 번호
작업 내용
기타
Summary by CodeRabbit
신규 기능
버그 수정
기타