diff --git a/README.md b/README.md index 4985061663..531dd84bf9 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,8 @@ # 학습 관리 시스템(Learning Management System) + +## 단계별 문서 +- [🚀 1단계 - 레거시 코드 리팩터링](./docs/01-refactoring.md) + ## 진행 방법 * 학습 관리 시스템의 수강신청 요구사항을 파악한다. * 요구사항에 대한 구현을 완료한 후 자신의 github 아이디에 해당하는 브랜치에 Pull Request(이하 PR)를 통해 코드 리뷰 요청을 한다. diff --git a/docs/01-refactoring.md b/docs/01-refactoring.md new file mode 100644 index 0000000000..9bf8f783bd --- /dev/null +++ b/docs/01-refactoring.md @@ -0,0 +1,37 @@ +# 🚀 1단계 - 레거시 코드 리팩터링 +*** + +## 코드 리뷰 +> PR 링크 : [#790](https://github.com/next-step/java-lms/pull/790) + +## 나의 학습 목표 + +### 1. TDD 사이클을 의식하며 기능을 구현한다. +- `실패 → 성공 → 리팩터링` 과정으로 작업한다. +- 습관처럼 이 과정을 지나치면, 다시 돌아와 사이클을 반복한다. +- TDD 사이클 습관을 만들자. + +### 2. 도메인에 특화된 이름을 짓는다. +- AI에게 추천 받는 습관을 만들자. + +### 3. 반복된 피드백은 피하자. +- 같은 피드백을 반복하지 않도록, 체크리스트를 만들어 점검하자. + +## 질문 삭제하기 요구사항 + +- [x] 질문 데이터를 완전히 삭제하는 것이 아니라 데이터의 상태를 삭제 상태(deleted - boolean type)로 변경한다. +- [x] 로그인 사용자와 질문한 사람이 같은 경우 삭제 가능하다. +- [x] 답변이 없는 경우 삭제가 가능하다. +- [x] 질문자와 답변 글의 모든 답변자가 같은 경우 삭제가 가능하다. +- [x] 질문을 삭제할 때 답변 또한 삭제해야 하며, 답변의 삭제 또한 삭제 상태(deleted)를 변경한다. +- [x] 질문자와 답변자가 다른 경우 답변을 삭제할 수 없다. +- [x] 질문과 답변 삭제 이력에 대한 정보를 DeleteHistory를 활용해 남긴다. + +## 리팩터링 요구사항 + +- [x] nextstep.qna.service.QnaService의 deleteQuestion()는 앞의 질문 삭제 기능을 구현한 코드이다. + 이 메소드는 단위 테스트하기 어려운 코드와 단위 테스트 가능한 코드가 섞여 있다. +- [x] QnaService의 deleteQuestion() 메서드에 단위 테스트 가능한 코드(핵심 비지니스 로직)를 도메인 모델 객체에 구현한다. +- [x] QnaService의 비지니스 로직을 도메인 모델로 이동하는 리팩터링을 진행할 때 TDD로 구현한다. +- [x] QnaService의 deleteQuestion() 메서드에 대한 단위 테스트는 src/test/java 폴더 nextstep.qna.service.QnaServiceTest이다. + 도메인 모델로 로직을 이동한 후에도 QnaServiceTest의 모든 테스트는 통과해야 한다. \ No newline at end of file diff --git a/docs/check-list.md b/docs/check-list.md new file mode 100644 index 0000000000..c54ca0d887 --- /dev/null +++ b/docs/check-list.md @@ -0,0 +1,12 @@ +# ✔️ PR 전 확인해야될 목록 +*** + +- 객체지향 생활체조 원칙을 준수하였는가 ? +- TDD 사이클로 구현하였는가 ? +- TDD 사이클 단위로 커밋하였는가 ? +- 객체에게 상태를 노출시키지 않고 책임을 맡겼는가 ? +- 테슽으는 객체의 책임과 결과만 검증했는가 ? +- 불변성을 최대한 유지했는가 ? +- 도메인 용어로 네이밍 했는가 ? +- 협력 구조가 자연스럽게 읽히는가 ? +- 같은 피드백을 반복하지는 않았는가 ? \ No newline at end of file diff --git a/src/main/java/nextstep/qna/domain/Answer.java b/src/main/java/nextstep/qna/domain/Answer.java index cf681811e7..e6d1da5bc1 100644 --- a/src/main/java/nextstep/qna/domain/Answer.java +++ b/src/main/java/nextstep/qna/domain/Answer.java @@ -1,40 +1,37 @@ package nextstep.qna.domain; +import nextstep.qna.CannotDeleteException; import nextstep.qna.NotFoundException; import nextstep.qna.UnAuthorizedException; import nextstep.users.domain.NsUser; -import java.time.LocalDateTime; - -public class Answer { +public class Answer extends SoftDeletableModel { private Long id; private NsUser writer; private Question question; - private String contents; - - private boolean deleted = false; - - private LocalDateTime createdDate = LocalDateTime.now(); - - private LocalDateTime updatedDate; + private QuestionBody contents; public Answer() { } public Answer(NsUser writer, Question question, String contents) { - this(null, writer, question, contents); + this(null, writer, question, new QuestionBody(contents)); } public Answer(Long id, NsUser writer, Question question, String contents) { + this(id, writer, question, new QuestionBody(contents)); + } + + public Answer(Long id, NsUser writer, Question question, QuestionBody contents) { this.id = id; - if(writer == null) { + if (writer == null) { throw new UnAuthorizedException(); } - if(question == null) { + if (question == null) { throw new NotFoundException(); } @@ -47,13 +44,12 @@ public Long getId() { return id; } - public Answer setDeleted(boolean deleted) { - this.deleted = deleted; - return this; + private void updateDeleted() { + deleted(); } public boolean isDeleted() { - return deleted; + return getDeleted(); } public boolean isOwner(NsUser writer) { @@ -64,14 +60,18 @@ public NsUser getWriter() { return writer; } - public String getContents() { - return contents; - } public void toQuestion(Question question) { this.question = question; } + public void validateAnswerOwner(NsUser loginUser) throws CannotDeleteException { + if (!this.isOwner(loginUser)) { + throw new CannotDeleteException("다른 사람이 쓴 답변이 있어 삭제할 수 없습니다."); + } + updateDeleted(); + } + @Override public String toString() { return "Answer [id=" + getId() + ", writer=" + writer + ", contents=" + contents + "]"; diff --git a/src/main/java/nextstep/qna/domain/Answers.java b/src/main/java/nextstep/qna/domain/Answers.java new file mode 100644 index 0000000000..38132c0ea5 --- /dev/null +++ b/src/main/java/nextstep/qna/domain/Answers.java @@ -0,0 +1,36 @@ +package nextstep.qna.domain; + +import nextstep.qna.CannotDeleteException; +import nextstep.users.domain.NsUser; + +import java.time.LocalDateTime; +import java.util.ArrayList; +import java.util.List; + +public class Answers { + + private List answers = new ArrayList<>(); + + public void add(Answer answer) { + this.answers.add(answer); + } + + public List getAnswers() { + return answers; + } + + public void validateAnswerOwner(NsUser loginUser) throws CannotDeleteException { + for (Answer answer : this.answers) { + answer.validateAnswerOwner(loginUser); + + } + } + + public List addDeleteAnswerHistory() { + List deleteHistories = new ArrayList<>(); + for (Answer answer : this.answers) { + deleteHistories.add(new DeleteHistory(ContentType.ANSWER, answer.getId(), answer.getWriter(), LocalDateTime.now())); + } + return deleteHistories; + } +} diff --git a/src/main/java/nextstep/qna/domain/Question.java b/src/main/java/nextstep/qna/domain/Question.java index b623c52c76..cc397a565b 100644 --- a/src/main/java/nextstep/qna/domain/Question.java +++ b/src/main/java/nextstep/qna/domain/Question.java @@ -1,27 +1,22 @@ package nextstep.qna.domain; +import nextstep.qna.CannotDeleteException; import nextstep.users.domain.NsUser; import java.time.LocalDateTime; import java.util.ArrayList; import java.util.List; -public class Question { +public class Question extends SoftDeletableModel { private Long id; - private String title; + private QuestionBody title; - private String contents; + private QuestionBody contents; private NsUser writer; - private List answers = new ArrayList<>(); - - private boolean deleted = false; - - private LocalDateTime createdDate = LocalDateTime.now(); - - private LocalDateTime updatedDate; + private Answers answers = new Answers(); public Question() { } @@ -31,6 +26,10 @@ public Question(NsUser writer, String title, String contents) { } public Question(Long id, NsUser writer, String title, String contents) { + this(id, writer, new QuestionBody(title), new QuestionBody(contents)); + } + + public Question(Long id, NsUser writer, QuestionBody title, QuestionBody contents) { this.id = id; this.writer = writer; this.title = title; @@ -41,52 +40,56 @@ public Long getId() { return id; } - public String getTitle() { - return title; + public NsUser getWriter() { + return writer; } - public Question setTitle(String title) { - this.title = title; - return this; + public void addAnswer(Answer answer) { + answer.toQuestion(this); + this.answers.add(answer); } - public String getContents() { - return contents; + public boolean isOwner(NsUser loginUser) { + return writer.equals(loginUser); } - public Question setContents(String contents) { - this.contents = contents; - return this; + public boolean isDeleted() { + return getDeleted(); } - public NsUser getWriter() { - return writer; + private void validateOwner(NsUser loginUser) throws CannotDeleteException { + if (!this.isOwner(loginUser)) { + throw new CannotDeleteException("질문을 삭제할 권한이 없습니다."); + } + updateDeleted(); } - public void addAnswer(Answer answer) { - answer.toQuestion(this); - answers.add(answer); + public void delete(NsUser loginUser) throws CannotDeleteException { + validateOwner(loginUser); + answers.validateAnswerOwner(loginUser); } - public boolean isOwner(NsUser loginUser) { - return writer.equals(loginUser); - } + public List toDeleteHistories(){ + List deleteHistories = new ArrayList<>(); + deleteHistories.add(new DeleteHistory(ContentType.QUESTION, this.id, this.writer, LocalDateTime.now())); - public Question setDeleted(boolean deleted) { - this.deleted = deleted; - return this; + List answerDeleteHistories = addDeleteAnswerHistory(); + deleteHistories.addAll(answerDeleteHistories); + + return deleteHistories; } - public boolean isDeleted() { - return deleted; + private List addDeleteAnswerHistory() { + return this.answers.addDeleteAnswerHistory(); } - public List getAnswers() { - return answers; + private void updateDeleted() { + deleted(); } @Override public String toString() { return "Question [id=" + getId() + ", title=" + title + ", contents=" + contents + ", writer=" + writer + "]"; } + } diff --git a/src/main/java/nextstep/qna/domain/QuestionBody.java b/src/main/java/nextstep/qna/domain/QuestionBody.java new file mode 100644 index 0000000000..b6dbe7946b --- /dev/null +++ b/src/main/java/nextstep/qna/domain/QuestionBody.java @@ -0,0 +1,10 @@ +package nextstep.qna.domain; + +public class QuestionBody { + + private String value; + + public QuestionBody(String value) { + this.value = value; + } +} diff --git a/src/main/java/nextstep/qna/domain/SoftDeletableModel.java b/src/main/java/nextstep/qna/domain/SoftDeletableModel.java new file mode 100644 index 0000000000..59d81c02ba --- /dev/null +++ b/src/main/java/nextstep/qna/domain/SoftDeletableModel.java @@ -0,0 +1,20 @@ +package nextstep.qna.domain; + +import java.time.LocalDateTime; + +public abstract class SoftDeletableModel { + + private LocalDateTime createdDate = LocalDateTime.now(); + + private LocalDateTime updatedDate; + + private boolean deleted = false; + + protected void deleted(){ + this.deleted = true; + } + + protected boolean getDeleted() { + return deleted; + } +} diff --git a/src/main/java/nextstep/qna/service/QnAService.java b/src/main/java/nextstep/qna/service/QnAService.java index 5741c84d65..fe5ae4effb 100644 --- a/src/main/java/nextstep/qna/service/QnAService.java +++ b/src/main/java/nextstep/qna/service/QnAService.java @@ -8,8 +8,6 @@ import org.springframework.transaction.annotation.Transactional; import javax.annotation.Resource; -import java.time.LocalDateTime; -import java.util.ArrayList; import java.util.List; @Service("qnaService") @@ -26,24 +24,9 @@ public class QnAService { @Transactional public void deleteQuestion(NsUser loginUser, long questionId) throws CannotDeleteException { Question question = questionRepository.findById(questionId).orElseThrow(NotFoundException::new); - if (!question.isOwner(loginUser)) { - throw new CannotDeleteException("질문을 삭제할 권한이 없습니다."); - } + question.delete(loginUser); - List answers = question.getAnswers(); - for (Answer answer : answers) { - if (!answer.isOwner(loginUser)) { - throw new CannotDeleteException("다른 사람이 쓴 답변이 있어 삭제할 수 없습니다."); - } - } - - List 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())); - } + List deleteHistories = question.toDeleteHistories(); deleteHistoryService.saveAll(deleteHistories); } } diff --git a/src/test/java/nextstep/qna/domain/AnswerTest.java b/src/test/java/nextstep/qna/domain/AnswerTest.java index 8e80ffb429..9a674727a5 100644 --- a/src/test/java/nextstep/qna/domain/AnswerTest.java +++ b/src/test/java/nextstep/qna/domain/AnswerTest.java @@ -1,8 +1,43 @@ package nextstep.qna.domain; +import nextstep.qna.CannotDeleteException; +import nextstep.qna.NotFoundException; +import nextstep.qna.UnAuthorizedException; import nextstep.users.domain.NsUserTest; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.*; public class AnswerTest { public static final Answer A1 = new Answer(NsUserTest.JAVAJIGI, QuestionTest.Q1, "Answers Contents1"); public static final Answer A2 = new Answer(NsUserTest.SANJIGI, QuestionTest.Q1, "Answers Contents2"); + + @Test + void 다른_사용자가_작성한_답변이_존재하면_에러발생(){ + assertThrows(CannotDeleteException.class, () -> { + A1.validateAnswerOwner(NsUserTest.SANJIGI); + }); + } + + @Test + void 답변삭제() throws CannotDeleteException { + A1.validateAnswerOwner(A1.getWriter()); + assertThat(A1.isDeleted()).isTrue(); + } + + @Test + void 작성자가_null이면_에러발생(){ + Assertions.assertThrows(UnAuthorizedException.class, () -> { + new Answer(null, QuestionTest.Q1, "Contents1"); + }); + } + + @Test + void 질문이_null이면_에러발생(){ + Assertions.assertThrows(NotFoundException.class, () -> { + new Answer(NsUserTest.JAVAJIGI, null, "Contents1"); + }); + } } diff --git a/src/test/java/nextstep/qna/domain/AnswersTest.java b/src/test/java/nextstep/qna/domain/AnswersTest.java new file mode 100644 index 0000000000..fd432885d4 --- /dev/null +++ b/src/test/java/nextstep/qna/domain/AnswersTest.java @@ -0,0 +1,18 @@ +package nextstep.qna.domain; + +import nextstep.users.domain.NsUserTest; +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.Test; + + +class AnswersTest { + public static final Question Q1 = new Question(NsUserTest.JAVAJIGI, "title1", "contents1"); + + @Test + void 답변_생성(){ + Answers answers = new Answers(); + answers.add(new Answer(NsUserTest.JAVAJIGI, Q1, "content1")); + Assertions.assertThat(answers.getAnswers()).hasSize(1); + } + +} \ No newline at end of file diff --git a/src/test/java/nextstep/qna/domain/QuestionTest.java b/src/test/java/nextstep/qna/domain/QuestionTest.java index 3b87823963..894b837c39 100644 --- a/src/test/java/nextstep/qna/domain/QuestionTest.java +++ b/src/test/java/nextstep/qna/domain/QuestionTest.java @@ -1,8 +1,27 @@ package nextstep.qna.domain; +import nextstep.qna.CannotDeleteException; import nextstep.users.domain.NsUserTest; +import org.junit.jupiter.api.Test; + + +import static org.assertj.core.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.*; public class QuestionTest { public static final Question Q1 = new Question(NsUserTest.JAVAJIGI, "title1", "contents1"); public static final Question Q2 = new Question(NsUserTest.SANJIGI, "title2", "contents2"); + + @Test + void 글쓴이가_로그인한_유저와_같지않으면_에러발생(){ + assertThrows(CannotDeleteException.class, () -> { + Q1.delete(Q2.getWriter()); + }); + } + + @Test + void 질문_삭제() throws CannotDeleteException { + Q1.delete(Q1.getWriter()); + assertThat(Q1.isDeleted()).isTrue(); + } }