Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REFACTOR] 투표 도메인 리팩터링 #727

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

[REFACTOR] 투표 도메인 리팩터링 #727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3c78b11
refactor: 사용하지 않는 메서드 삭제
aiaiaiai1 Oct 9, 2023
91bb5aa
refactor: (#514) Post 객체에서 vote 의존성 제거
aiaiaiai1 Oct 9, 2023
def5a4f
test: (#514) VoteService, Vote 테스트 코드 추가
aiaiaiai1 Oct 9, 2023
1f91424
refactor: (#514) docs 개행 수정
aiaiaiai1 Oct 9, 2023
7735524
Merge branch 'dev' into refactor/#514
aiaiaiai1 Oct 9, 2023
0f56c56
test: (#514) 테스트 실패 코드 수정
aiaiaiai1 Oct 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 1 addition & 37 deletions backend/src/main/java/com/votogether/domain/post/entity/Post.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@
import com.votogether.domain.common.BaseEntity;
import com.votogether.domain.member.entity.Member;
import com.votogether.domain.post.entity.comment.Comment;
import com.votogether.domain.post.exception.PostExceptionType;
import com.votogether.domain.post.exception.PostOptionExceptionType;
import com.votogether.domain.vote.entity.Vote;
import com.votogether.domain.vote.exception.VoteExceptionType;
import com.votogether.global.exception.BadRequestException;
import jakarta.persistence.CascadeType;
import jakarta.persistence.Column;
import jakarta.persistence.Embedded;
Expand Down Expand Up @@ -128,38 +123,7 @@ public void closeEarly() {
this.postDeadline.close();
}

public Vote makeVote(final Member voter, final PostOption postOption) {
validateDeadLine();
validateVoter(voter);
validatePostOption(postOption);

final Vote vote = Vote.builder()
.member(voter)
.build();

postOption.addVote(vote);
return vote;
}

public void validateDeadLine() {
if (isClosed()) {
throw new BadRequestException(PostExceptionType.CLOSED);
}
}

private void validateVoter(final Member voter) {
if (Objects.equals(this.writer.getId(), voter.getId())) {
throw new BadRequestException(VoteExceptionType.CANNOT_VOTE_MY_POST);
}
}

private void validatePostOption(final PostOption postOption) {
if (!hasPostOption(postOption)) {
throw new BadRequestException(PostOptionExceptionType.NOT_FOUND);
}
}

private boolean hasPostOption(final PostOption postOption) {
public boolean hasPostOption(final PostOption postOption) {
return this.postOptions.contains(postOption);
}

Expand Down
12 changes: 10 additions & 2 deletions backend/src/main/java/com/votogether/domain/vote/controller/VoteControllerDocs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ public interface VoteControllerDocs {
content = @Content(schema = @Schema(implementation = ExceptionResponse.class))),
@ApiResponse(
responseCode = "404",
description = "1.존재하지 않는 게시글\t\n2.존재하지 않는 선택지",
description = """
1.존재하지 않는 게시글

2.존재하지 않는 선택지
""",
content = @Content(schema = @Schema(implementation = ExceptionResponse.class)))
})
ResponseEntity<Void> vote(
Expand All @@ -37,7 +41,11 @@ ResponseEntity<Void> vote(
@ApiResponse(responseCode = "200", description = "투표 수정 성공"),
@ApiResponse(
responseCode = "404",
description = "1.존재하지 않는 게시글\t\n2.존재하지 않는 선택지",
description = """
1.존재하지 않는 게시글

2.존재하지 않는 선택지
""",
content = @Content(schema = @Schema(implementation = ExceptionResponse.class)))
})
ResponseEntity<Void> changeVote(
Expand Down
10 changes: 8 additions & 2 deletions backend/src/main/java/com/votogether/domain/vote/entity/Vote.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import com.votogether.domain.common.BaseEntity;
import com.votogether.domain.member.entity.Member;
import com.votogether.domain.post.entity.PostOption;
import com.votogether.domain.vote.exception.VoteExceptionType;
import com.votogether.global.exception.BadRequestException;
import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.GeneratedValue;
Expand Down Expand Up @@ -37,15 +39,19 @@ public class Vote extends BaseEntity {

@Builder
private Vote(final Member member, final PostOption postOption) {
validateVotingRights(member, postOption);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

생성자에게 꼼꼼한 검증은 좋은 방법이라고 생각합니다 👍

한 가지 우려되는 점은 Vote에서 Post 의존성을 가지게 되면 Vote -> Post -> PostOption -> Vote의 순환 참조가 발생할 수 있는 위험이 증가한다고 생각이 들어요.

따라서 해당 검증은 Service 계층의 비즈니스 로직에서 처리하는게 어떤지 물어보고 싶어요 :)

this.member = member;
this.postOption = postOption;
}

public boolean isVoteByMember(final Member member) {
return this.member.equals(member);
private void validateVotingRights(final Member member, final PostOption postOption) {
if (postOption.getPost().isWriter(member)) {
throw new BadRequestException(VoteExceptionType.CANNOT_VOTE_MY_POST);
}
}

public void setPostOption(final PostOption postOption) {
this.postOption = postOption;
}

}
3 changes: 1 addition & 2 deletions backend/src/main/java/com/votogether/domain/vote/exception/VoteExceptionType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@
@Getter
public enum VoteExceptionType implements ExceptionType {

CANNOT_VOTE_MY_POST(700, "해당 게시글 작성자는 투표할 수 없습니다."),
CANNOT_VOTE_MY_POST(700, "게시글 작성자는 투표할 수 없습니다."),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍👍

NOT_FOUND(701, "참여한 투표가 존재하지 않습니다."),
ALREADY_VOTED(702, "이미 참여한 투표입니다."),
;


private final int code;
private final String message;

Expand Down
32 changes: 27 additions & 5 deletions backend/src/main/java/com/votogether/domain/vote/service/VoteService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,37 @@ public void vote(
final Post post = postRepository.findById(postId)
.orElseThrow(() -> new NotFoundException(PostExceptionType.NOT_FOUND));

validateAlreadyVoted(member, post);

final PostOption postOption = postOptionRepository.findById(postOptionId)
.orElseThrow(() -> new NotFoundException(PostOptionExceptionType.NOT_FOUND));

final Vote vote = post.makeVote(member, postOption);
validateAlreadyVoted(member, post);
final Vote vote = createVote(member, post, postOption);
voteRepository.save(vote);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 생략해도 변경 감지에 의해 vote가 자동으로 insert 될 것 같아요

}

private Vote createVote(final Member member, final Post post, final PostOption postOption) {
validateDeadline(post);
validatePostOption(post, postOption);
Comment on lines +46 to +47
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

검증은 생성 메서드와 별도로 한 곳에 묶어두는 건 어떨까요?!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

숨겨진 게시글에 대한 검증도 있으면 좋을 것 같아요 ㅎㅎ


final Vote vote = Vote.builder()
.member(member)
.postOption(postOption)
.build();
postOption.addVote(vote);
return vote;
Comment on lines +49 to +54
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createVote메서드에선 postOption 셋팅을 제외한 Vote를 만드는 로직만 남겨두고, 양방향 관계를 맺어주는 건 이미 postOption.addVote(vote) 여기서 해주고 있으니 차라리 밑의 예시처럼 구현하는 것은 어떨까요?

private Vote createVote(final Member member) {
    return Vote.builder()
                .member(member)
                .build();
}

그리고 vote 메서드에서

postOption.addVote(vote);

이 코드를 진행하는 형식입니다.

createVote 메서드의 파라미터 수도 줄일 수 있고, 코드의 흐름을 파악하기 더 수월할 것 같아요 :)

}

private void validatePostOption(final Post post, final PostOption postOption) {
if (!post.hasPostOption(postOption)) {
throw new BadRequestException(PostOptionExceptionType.NOT_FOUND);
}
}

private void validateDeadline(final Post post) {
if (post.isClosed()) {
throw new BadRequestException(PostExceptionType.CLOSED);
}
}

private void validateAlreadyVoted(final Member member, final Post post) {
final List<PostOption> postOptions = post.getPostOptions();
Expand Down Expand Up @@ -72,8 +94,8 @@ public void changeVote(
.orElseThrow(() -> new NotFoundException(PostOptionExceptionType.NOT_FOUND));

voteRepository.delete(originVote);
final Vote vote = post.makeVote(member, newPostOption);
voteRepository.save(vote);
final Vote newVote = createVote(member, post, newPostOption);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분도 위의 리뷰 내용과 동일하게 적용 될 것 같습니다.

voteRepository.save(newVote);
}

}
24 changes: 13 additions & 11 deletions ...end/src/test/java/com/votogether/domain/post/repository/PostCustomRepositoryImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,18 +462,19 @@ void getPostsClosed() {
@DisplayName("게시글을 최신순으로 조회한다.")
void getPostsByLatest() {
// given
Member member = memberTestPersister.builder().save();
Post postA = postTestPersister.postBuilder().writer(member).save();
Post postB = postTestPersister.postBuilder().writer(member).save();
Member voter = memberTestPersister.builder().save();
Member writer = memberTestPersister.builder().save();
Post postA = postTestPersister.postBuilder().writer(writer).save();
Post postB = postTestPersister.postBuilder().writer(writer).save();
PostOption postOptionA = postTestPersister.postOptionBuilder().post(postA).sequence(1).save();
PostOption postOptionB = postTestPersister.postOptionBuilder().post(postB).sequence(1).save();
voteTestPersister.builder().postOption(postOptionA).member(member).save();
voteTestPersister.builder().postOption(postOptionB).member(member).save();
voteTestPersister.builder().postOption(postOptionA).member(voter).save();
voteTestPersister.builder().postOption(postOptionB).member(voter).save();

// when
Pageable pageable = PageRequest.of(0, 10);
List<Post> result = postCustomRepository.findPostsByVotedWithFilteringAndPaging(
member,
voter,
PostClosingType.ALL,
PostSortType.LATEST,
pageable
Expand All @@ -490,19 +491,20 @@ void getPostsByLatest() {
@DisplayName("게시글을 인기순으로 조회한다.")
void getPostsByHot() {
// post
Member member = memberTestPersister.builder().save();
Member voter = memberTestPersister.builder().save();
Member writer = memberTestPersister.builder().save();
Post postA = postTestPersister.postBuilder().save();
PostOption postOptionA = postTestPersister.postOptionBuilder().post(postA).sequence(1).save();
voteTestPersister.builder().postOption(postOptionA).member(member).save();
voteTestPersister.builder().postOption(postOptionA).member(voter).save();
voteTestPersister.builder().postOption(postOptionA).save();
Post postB = postTestPersister.postBuilder().writer(member).save();
Post postB = postTestPersister.postBuilder().writer(writer).save();
PostOption postOptionB = postTestPersister.postOptionBuilder().post(postB).sequence(1).save();
voteTestPersister.builder().postOption(postOptionB).member(member).save();
voteTestPersister.builder().postOption(postOptionB).member(voter).save();

// when
Pageable pageable = PageRequest.of(0, 10);
List<Post> result = postCustomRepository.findPostsByVotedWithFilteringAndPaging(
member,
voter,
PostClosingType.ALL,
PostSortType.HOT,
pageable
Expand Down
31 changes: 31 additions & 0 deletions backend/src/test/java/com/votogether/domain/vote/entity/VoteTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.votogether.domain.vote.entity;

import static org.assertj.core.api.Assertions.assertThatThrownBy;

import com.votogether.domain.member.entity.Member;
import com.votogether.domain.post.entity.Post;
import com.votogether.domain.post.entity.PostOption;
import com.votogether.global.exception.BadRequestException;
import java.time.LocalDateTime;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.test.util.ReflectionTestUtils;

class VoteTest {

@Test
@DisplayName("게시글 작성자가 투표를 하면 예외가 발생한다.")
void voteException() {
// given
Member writer = Member.builder().nickname("글쓴이").build();
ReflectionTestUtils.setField(writer, "id", 1L);
Post post = Post.builder().title("제목").content("내용").deadline(LocalDateTime.now()).writer(writer).build();
PostOption postOption = PostOption.builder().post(post).content("내용").build();

// when, then
assertThatThrownBy(() -> Vote.builder().member(writer).postOption(postOption).build())
.isInstanceOf(BadRequestException.class)
.hasMessage("게시글 작성자는 투표할 수 없습니다.");
}

}
Loading