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

* docs : 1단계 정리 2021. 03. 31. 요구사항 및 구현사항 내역 정리 Resolves : loop-study See also : wheejuni * test : Answer 테스트 추가 2021. 03. 31. - 작성자 확인 테스트 코드 추가 - 작성자 null 에러 테스트 코드 추가 - 게시글 null 에러 테스트 코드 추가 Resolves : loop-study See also : wheejuni * test : Answer 테스트 추가 2021. 03. 31. - 삭제여부 변경 테스트 추가 Resolves : loop-study See also : wheejuni * test : Answer @DisplayName 변경 2021. 03. 31. - 댓글 -> 답변으로 변경 Resolves : loop-study See also : wheejuni * test : User 테스트 추가 2021. 03. 31. - 아이디 변경 테스트 추가 - 비밀번호 변경 테스트 추가 - 이름 변경 테스트 추가 - 이메일 변경 테스트 추가 - 유저 업데이트 테스트 추가 Resolves : loop-study See also : wheejuni * test : User 테스트 추가 2021. 03. 31. - 아이디 테스트 추가 - 패스워드 테스트 추가 Resolves : loop-study See also : wheejuni * test : User 테스트 추가 2021. 03. 31. - 아이디 테스트 추가 - 패스워드 테스트 추가 Resolves : loop-study See also : wheejuni * test : Question 테스트 추가 2021. 03. 31. - 제목 변경 테스트 추가 - 삭제 여부 변경 테스트 추가 Resolves : loop-study See also : wheejuni * test : Answers 테스트 추가 2021. 03. 31. AnswresTest 생성 Resolves : loop-study See also : wheejuni * test : Answers 테스트 추가 2021. 03. 31. AnswresTest 생성 Resolves : loop-study See also : wheejuni * test : Answers 테스트 추가 2021. 03. 31. - 답변 작성자 존재 테스트 - 답변 작성자 미존재 테스트 Resolves : loop-study See also : wheejuni * feat : Answers 기능 2021. 03. 31. - isOwner 추가 Resolves : loop-study See also : wheejuni * refector : Answers 기능 변경 2021. 03. 31. - anyMatch -> allMatch 변경 Resolves : loop-study See also : wheejuni * test : 오타 수정 2021. 03. 31. - true -> false 로 변경 Resolves : loop-study See also : wheejuni * feat : QnA 리팩토링 2021. 03. 31. - DeleteHistories 추가 - Question 의 List<Answer> -> Answers 로 변경 - Question delete 메소드 추가 Resolves : loop-study See also : wheejuni * refector : 삭제 가능 책임 이동 2021. 03. 31.동 삭제 가능 여부 로직 QnAService -> Question 으로 이동 Resolves : loop-study See also : wheejuni * refector : QnAService 개선 2021. 03. 31. 코드 개선 Resolves : loop-study See also : wheejuni * refector : 삭제이력 추가 개선 2021. 03. 31. 각 객체에게 삭제이력 반환하는 방식으로 변경 Resolves : loop-study See also : wheejuni * refector : 미사용 메소드 삭제 2021. 03. 31. 미사용 getAnswers() 삭제 Resolves : loop-study See also : wheejuni * refector : 피드백 반영 2021. 04. 01. - Answers isAllDelete 삭제, answersToDeleteHistories 에서 delete 상태 변경. Resolves : loop-study See also : wheejuni * refector : 피드백 반영 및 수정 2021. 04. 01. - DeleteHistories -> 정적팩토리메소드로 변경 - Question -> canDelete 삭제, 각 객체의 toDeleteHistory 에서 삭제가능 판단 - this.delete = true -> 상수로 변경 Resolves : loop-study See also : wheejuni * refector : 코드 삭제 2021. 04. 01. 불필요한 throw CannotDeleteException 제거 Resolves : loop-study See also : wheejuni
next-step · Apr 1, 2021 · 542b9c3 · 542b9c3
1 parent be01497
commit 542b9c3
Show file tree

Hide file tree

Showing 11 changed files with 454 additions and 31 deletions.
diff --git a/src/main/java/qna/CannotDeleteException.java b/src/main/java/qna/CannotDeleteException.java
@@ -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) {

diff --git a/src/main/java/qna/README.md b/src/main/java/qna/README.md
@@ -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 수정
+
diff --git a/src/main/java/qna/domain/Answer.java b/src/main/java/qna/domain/Answer.java
@@ -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;
@@ -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);
+        }
+    }
 }
diff --git a/src/main/java/qna/domain/Answers.java b/src/main/java/qna/domain/Answers.java
@@ -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());
+    }
+}
diff --git a/src/main/java/qna/domain/DeleteHistories.java b/src/main/java/qna/domain/DeleteHistories.java
@@ -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);
+    }
+}
diff --git a/src/main/java/qna/domain/Question.java b/src/main/java/qna/domain/Question.java
@@ -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;
 
@@ -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;
 
@@ -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);
+    }
 }
diff --git a/src/main/java/qna/domain/User.java b/src/main/java/qna/domain/User.java
@@ -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);
     }
 

diff --git a/src/main/java/qna/service/QnAService.java b/src/main/java/qna/service/QnAService.java
@@ -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 {
@@ -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());
     }
 }
diff --git a/src/test/java/qna/domain/AnswerTest.java b/src/test/java/qna/domain/AnswerTest.java
@@ -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
+    }
 }