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.

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

[step1] 질문 삭제하기 기능 리팩토링, 제출합니다. #446

Merged
merged 22 commits into from
Apr 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/qna/CannotDeleteException.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package qna;

public class CannotDeleteException extends Exception {
public class CannotDeleteException extends RuntimeException {
private static final long serialVersionUID = 1L;

public CannotDeleteException(String message) {
Expand Down
72 changes: 72 additions & 0 deletions src/main/java/qna/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# 1단계 - 질문 삭제하기
## 질문 삭제하기 요구사항
- 질문 데이터를 완전히 삭제하는 것이 아니라 데이터의 상태를 삭제 상태(deleted - boolean type)로 변경한다.
- 로그인 사용자와 질문한 사람이 같은 경우 삭제 가능하다.
- 답변이 없는 경우 삭제가 가능하다.
- 질문자와 답변 글의 모든 답변자 같은 경우삭제가 가능하다.
- 질문을 삭제할 때 답변 또한 삭제해야 하며, 답변의 삭제 또한 삭제 상태(deleted)를 변경
- 한다.
- 질문자와 답변자가 다른경우 답변을 삭제할 수 없다.
- 질문과 답변 삭제 이력에 대한 정보를 DeleteHistory를 활용해 남긴다.


## 프로그래밍 요구사항
- qna.service.QnaService의 deleteQuestion()는 앞의 질문 삭제 기능을 구현한 코드이다. 이 메소드는 단위 테스트하기 어려운 코드와 단위 테스트 가능한 코드가 섞여 있다.
- 단위 테스트하기 어려운 코드와 단위 테스트 가능한 코드를 분리해 단위 테스트 가능한 코드 에 대해 단위 테스트를 구현한다.


<prd><code>

public class QnAService {

public void deleteQuestion(User loginUser, long questionId) throws CannotDeleteException {
Question question = findQuestionById(questionId);
if (!question.isOwner(loginUser)) {
throw new CannotDeleteException("질문을 삭제할 권한이 없습니다.");
}

List<Answer> answers = question.getAnswers();
for (Answer answer : answers) {
if (!answer.isOwner(loginUser)) {
throw new CannotDeleteException("다른 사람이 쓴 답변이 있어 삭제할 수 없습니다.");
}
}

List<DeleteHistory> deleteHistories = new ArrayList<>();
question.setDeleted(true);
deleteHistories.add(new DeleteHistory(ContentType.QUESTION, questionId, question.getWriter(), LocalDateTime.now()));
for (Answer answer : answers) {
answer.setDeleted(true);
deleteHistories.add(new DeleteHistory(ContentType.ANSWER, answer.getId(), answer.getWriter(), LocalDateTime.now()));
}
deleteHistoryService.saveAll(deleteHistories);
}
}
</code></pre>


##힌트1
- 객체의 상태 데이터를 꺼내지(get)말고 메시지를 보낸다.
- 규칙 8: 일급 콜렉션을 쓴다.
- Question의 List를 일급 콜렉션으로 구현해 본다.
- 규칙 7: 3개 이상의 인스턴스 변수를 가진 클래스를 쓰지 않는다.
- 인스턴스 변수의 수를 줄이기 위해 도전한다.


##힌트2
- 테스트하기 쉬운 부분과 테스트하기 어려운 부분을 분리해 테스트 가능한 부분만 단위테스트한다.


## 구현 사항
### 테스트
- Answer 테스트코드 추가
- Question 테스트코드 추가
- User 테스트코드 추가

### 리팩토링
- List<T> 일급 객체로 분리
- Question.List<Answer>
- QnAService.List<DeleteHistory>

- QnAService.deleteQuestion 수정

17 changes: 17 additions & 0 deletions src/main/java/qna/domain/Answer.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
package qna.domain;

import qna.CannotDeleteException;
import qna.NotFoundException;
import qna.UnAuthorizedException;

import javax.persistence.*;
import java.time.LocalDateTime;

@Entity
public class Answer extends AbstractEntity {
private static final String ANSWER_NOT_OWNER = "다른 사람이 쓴 답변이 있어 삭제할 수 없습니다.";
private static final boolean DELETE_TRUE = true;

@ManyToOne(optional = false)
@JoinColumn(foreignKey = @ForeignKey(name = "fk_answer_writer"))
private User writer;
Expand Down Expand Up @@ -72,4 +77,16 @@ public void toQuestion(Question question) {
public String toString() {
return "Answer [id=" + getId() + ", writer=" + writer + ", contents=" + contents + "]";
}

public DeleteHistory toDeleteHistory(User loginUser) {
validDelete(loginUser);
this.deleted = DELETE_TRUE;
return new DeleteHistory(ContentType.ANSWER, getId(), getWriter(), LocalDateTime.now());
}

private void validDelete(User loginUser) {
if (!isOwner(loginUser)) {
throw new CannotDeleteException(ANSWER_NOT_OWNER);
}
}
}
30 changes: 30 additions & 0 deletions src/main/java/qna/domain/Answers.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package qna.domain;

import org.hibernate.annotations.Where;

import javax.persistence.CascadeType;
import javax.persistence.Embeddable;
import javax.persistence.OneToMany;
import javax.persistence.OrderBy;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

@Embeddable
public class Answers {

@OneToMany(mappedBy = "question", cascade = CascadeType.ALL)
@Where(clause = "deleted = false")
@OrderBy("id ASC")
private List<Answer> answers = new ArrayList<>();

public void add(Answer answer) {
answers.add(answer);
}

public List<DeleteHistory> answersToDeleteHistories(User loginUser) {
return answers.stream()
.map(answer -> answer.toDeleteHistory(loginUser))
.collect(Collectors.toList());
}
}
25 changes: 25 additions & 0 deletions src/main/java/qna/domain/DeleteHistories.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package qna.domain;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

public class DeleteHistories {
private final List<DeleteHistory> deleteHistories;

private DeleteHistories(List<DeleteHistory> deleteHistories) {
this.deleteHistories = deleteHistories;
}

public static DeleteHistories of(User loginUser, Question question) {
List<DeleteHistory> result = new ArrayList<>();
result.add(question.questionToDeleteHistory(loginUser));
result.addAll(question.answersToDeleteHistories(loginUser));

return new DeleteHistories(result);
}

public List<DeleteHistory> getDeleteHistories() {
return Collections.unmodifiableList(deleteHistories);
}
}
31 changes: 24 additions & 7 deletions src/main/java/qna/domain/Question.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package qna.domain;

import org.hibernate.annotations.Where;
import qna.CannotDeleteException;

import javax.persistence.*;
import java.util.ArrayList;
import java.time.LocalDateTime;
import java.util.List;

@Entity
public class Question extends AbstractEntity {
private static final String QUESTION_NOT_OWNER = "질문을 삭제할 권한이 없습니다.";
private static final boolean DELETE_TRUE = true;

@Column(length = 100, nullable = false)
private String title;

Expand All @@ -18,10 +21,8 @@ public class Question extends AbstractEntity {
@JoinColumn(foreignKey = @ForeignKey(name = "fk_question_writer"))
private User writer;

@OneToMany(mappedBy = "question", cascade = CascadeType.ALL)
@Where(clause = "deleted = false")
@OrderBy("id ASC")
private List<Answer> answers = new ArrayList<>();
@Embedded
private Answers answers = new Answers();

private boolean deleted = false;

Expand Down Expand Up @@ -84,12 +85,28 @@ public boolean isDeleted() {
return deleted;
}

public List<Answer> getAnswers() {
public Answers getAnswers() {
return answers;
}

@Override
public String toString() {
return "Question [id=" + getId() + ", title=" + title + ", contents=" + contents + ", writer=" + writer + "]";
}

public DeleteHistory questionToDeleteHistory(User loginUser) {
validDelete(loginUser);
this.deleted = DELETE_TRUE;
return new DeleteHistory(ContentType.QUESTION, getId(), getWriter(), LocalDateTime.now());
}

private void validDelete(User loginUser) {
if (!isOwner(loginUser)) {
throw new CannotDeleteException(QUESTION_NOT_OWNER);
}
}

public List<DeleteHistory> answersToDeleteHistories(User loginUser) {
return answers.answersToDeleteHistories(loginUser);
}
}
2 changes: 1 addition & 1 deletion src/main/java/qna/domain/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public void update(User loginUser, User target) {
this.email = target.email;
}

private boolean matchUserId(String userId) {
public boolean matchUserId(String userId) {
return this.userId.equals(userId);
}

Expand Down
26 changes: 4 additions & 22 deletions src/main/java/qna/service/QnAService.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@
import qna.domain.*;

import javax.annotation.Resource;
import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.List;

@Service("qnaService")
public class QnAService {
Expand All @@ -35,24 +32,9 @@ public Question findQuestionById(Long id) {
@Transactional
public void deleteQuestion(User loginUser, long questionId) throws CannotDeleteException {
Question question = findQuestionById(questionId);
if (!question.isOwner(loginUser)) {
throw new CannotDeleteException("질문을 삭제할 권한이 없습니다.");
}

List<Answer> answers = question.getAnswers();
for (Answer answer : answers) {
if (!answer.isOwner(loginUser)) {
throw new CannotDeleteException("다른 사람이 쓴 답변이 있어 삭제할 수 없습니다.");
}
}

List<DeleteHistory> deleteHistories = new ArrayList<>();
question.setDeleted(true);
deleteHistories.add(new DeleteHistory(ContentType.QUESTION, questionId, question.getWriter(), LocalDateTime.now()));
for (Answer answer : answers) {
answer.setDeleted(true);
deleteHistories.add(new DeleteHistory(ContentType.ANSWER, answer.getId(), answer.getWriter(), LocalDateTime.now()));
}
deleteHistoryService.saveAll(deleteHistories);

DeleteHistories deleteHistories = DeleteHistories.of(loginUser, question);

deleteHistoryService.saveAll(deleteHistories.getDeleteHistories());
}
}
76 changes: 76 additions & 0 deletions src/test/java/qna/domain/AnswerTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,82 @@
package qna.domain;

import org.junit.Test;
import org.junit.jupiter.api.DisplayName;
import qna.CannotDeleteException;
import qna.NotFoundException;
import qna.UnAuthorizedException;

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

@DisplayName("답변")
public class AnswerTest {
public static final Answer A1 = new Answer(UserTest.JAVAJIGI, QuestionTest.Q1, "Answers Contents1");
public static final Answer A2 = new Answer(UserTest.SANJIGI, QuestionTest.Q1, "Answers Contents2");

@Test
@DisplayName("작성자 확인")
public void isAnswer() throws Exception {
//given
User javajigi = UserTest.JAVAJIGI;
User sanjigi = UserTest.SANJIGI;

//when

//then
assertThat(A1.isOwner(javajigi)).isTrue();
assertThat(A2.isOwner(sanjigi)).isTrue();
}

@Test
@DisplayName("작성자 null 에러 확인")
public void writerIsNullException() throws Exception {
//given
//when
assertThatThrownBy(() -> {
new Answer(null, QuestionTest.Q1, "Answers Contents1");
}).isInstanceOf(UnAuthorizedException.class);
//then

}

@Test
@DisplayName("게시글 null 에러 확인")
public void questionIsNullException() throws Exception {
//given

//when
assertThatThrownBy(() -> {
new Answer(UserTest.JAVAJIGI, null, "Answers Contents1");
}).isInstanceOf(NotFoundException.class);
//then
}

@Test
@DisplayName("삭제여부 변경 테스트")
public void setDelete() throws Exception {
//given
boolean deleteTrue = true;
boolean deleteFalse = false;

//when
A1.setDeleted(deleteTrue);
A2.setDeleted(deleteFalse);

//then
assertThat(A1.isDeleted()).isTrue();
assertThat(A2.isDeleted()).isFalse();
}

@Test
@DisplayName("삭제 불가 예외 테스트")
public void deleteHistoryException() throws Exception {
//given

//when
assertThatThrownBy(() -> {
A1.toDeleteHistory(UserTest.SANJIGI);
}).isInstanceOf(CannotDeleteException.class);

//then
}
}