Skip to content
25 changes: 25 additions & 0 deletions docs/step1요구사항.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# 문자열 사칙 연산 계산기 구현
## 기능 요구사항

## 삭제 방식 변경
- [x] 질문 데이터를 완전히 삭제하지 않는다(soft delete)
- [x] deleted 상태(boolean type)를 true로 변경한다

## 삭제 권한 검증
- [x] 로그인 사용자와 질문 작성자가 같은 경우에만 삭제 가능하다
- [x] 답변이 없는 경우 삭제가 가능하다
- [x] 질문자와 모든 답변 작성자가 같은 경우 삭제가 가능하다
- [x] 질문자와 답변자가 다른 경우 질문을 삭제할 수 없다

## 연관 데이터 처리
- [x] 질문을 삭제할 때 관련된 모든 답변도 함께 삭제한다
- [x] 답변의 삭제도 soft deled로 deleted 상태를 true로 변경한다

## 삭제 이력 관리
- [x] 질문 삭제 시 DeleteHistory를 생성하여 이력을 남긴다
- [x] 답변 삭제 시에도 DeleteHistory를 생성하여 이력을 남긴다


## 리팩터링 요구사항
- [ ] deleteQuestion() 메서드의 비즈니스 로직을 도메인으로 이동한다.

14 changes: 5 additions & 9 deletions src/main/java/nextstep/qna/domain/Answer.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,6 @@ public Long getId() {
return id;
}

public Answer setDeleted(boolean deleted) {
this.deleted = deleted;
return this;
}

public boolean isDeleted() {
return deleted;
}
Expand All @@ -64,14 +59,15 @@ public NsUser getWriter() {
return writer;
}

public String getContents() {
return contents;
}

public void toQuestion(Question question) {
this.question = question;
}

public DeleteHistory delete(){
Copy link
Contributor

Choose a reason for hiding this comment

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

현재 구현한 방식과 아래와 같이 글쓴이 인지 여부를 이 메서드가 판단하도록 구현하는 것 중 어느 방식으로 구현하는 것이 좋을지 고민해 본다.
위 두 가지 방식 중 선택한 방법으로 구현하는 것이 좋은 이유에 대해 피드백으로 남겨본다.

    public void delete(NsUser loginUser){
        // 글쓴이 인지를 판단하는 로직 검증 후 삭제 상태로 변경
    }

this.deleted = true;
return new DeleteHistory(ContentType.ANSWER, id, writer, LocalDateTime.now());
}
Comment on lines +66 to +69
Copy link
Contributor

Choose a reason for hiding this comment

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

객체의 삭제 상태를 변경하는 책임과 DeleteHistory 생성하고 반환하는 책임
위와 같이 두 가지 책임을 가지고 있는 느낌이 드는데 어떻게 생각하나?
https://vimeo.com/1137582691 영상에서 추천하는 메서드 이름 짓기 원칙을 따라 이름을 짓고 메서드 분리가 필요하다면 메서드 분리해 보면 어떨까?


@Override
public String toString() {
return "Answer [id=" + getId() + ", writer=" + writer + ", contents=" + contents + "]";
Expand Down
38 changes: 38 additions & 0 deletions src/main/java/nextstep/qna/domain/Answers.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package nextstep.qna.domain;

import nextstep.qna.CannotDeleteException;
import nextstep.users.domain.NsUser;

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

public class Answers {
Copy link
Contributor

Choose a reason for hiding this comment

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

일급 콜렉션 추가 👍

private final List<Answer> answers;

public Answers() {
this(new ArrayList<>());
}

public Answers(List<Answer> answers) {
this.answers = new ArrayList<>(answers);
}

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

public List<DeleteHistory> delete(NsUser loginUser) throws CannotDeleteException {
checkDeletable(loginUser);
List<DeleteHistory> histories = new ArrayList<>();
for (Answer answer : answers) {
histories.add(answer.delete());
}
return histories;
}

private void checkDeletable(NsUser loginUser) throws CannotDeleteException {
if (answers.stream().anyMatch(answer -> !answer.isOwner(loginUser))) {
throw new CannotDeleteException("다른 사람의 답변이 존재하여 삭제할 수 없습니다.");
}
}
}
19 changes: 19 additions & 0 deletions src/main/java/nextstep/qna/domain/BaseEntity.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package nextstep.qna.domain;

import java.time.LocalDateTime;

public abstract class BaseEntity {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

private Long id;
private LocalDateTime createdDate = LocalDateTime.now();
private LocalDateTime updatedDate;

public BaseEntity() {}

public BaseEntity(Long id) {
this.id = id;
}

public Long getId() {
return id;
}
}
12 changes: 12 additions & 0 deletions src/main/java/nextstep/qna/domain/DeleteHistory.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,18 @@ public DeleteHistory(ContentType contentType, Long contentId, NsUser deletedBy,
this.createdDate = createdDate;
}

public ContentType getContentType() {
return contentType;
}

public Long getContentId() {
return contentId;
}

public NsUser getDeletedBy() {
return deletedBy;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand Down
69 changes: 26 additions & 43 deletions src/main/java/nextstep/qna/domain/Question.java
Original file line number Diff line number Diff line change
@@ -1,62 +1,37 @@
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 {
private Long id;

private String title;

private String contents;
public class Question extends BaseEntity {
private QuestionContent content;

private NsUser writer;

private List<Answer> answers = new ArrayList<>();
private Answers answers = new Answers();

private boolean deleted = false;

private LocalDateTime createdDate = LocalDateTime.now();

private LocalDateTime updatedDate;

public Question() {
super();
}

public Question(NsUser writer, String title, String contents) {
this(0L, writer, title, contents);
}

public Question(Long id, NsUser writer, String title, String contents) {
this.id = id;
this.writer = writer;
this.title = title;
this.contents = contents;
}

public Long getId() {
return id;
}

public String getTitle() {
return title;
}

public Question setTitle(String title) {
this.title = title;
return this;
}

public String getContents() {
return contents;
this(id, writer, new QuestionContent(title, contents));
}

public Question setContents(String contents) {
this.contents = contents;
return this;
public Question(Long id, NsUser writer, QuestionContent content) {
super(id);
this.writer = writer;
this.content = content;
}

public NsUser getWriter() {
Expand All @@ -72,21 +47,29 @@ public boolean isOwner(NsUser loginUser) {
return writer.equals(loginUser);
}

public Question setDeleted(boolean deleted) {
this.deleted = deleted;
return this;
}

public boolean isDeleted() {
return deleted;
}

public List<Answer> getAnswers() {
return answers;
public List<DeleteHistory> delete(NsUser loginUser) throws CannotDeleteException {
if (!isOwner(loginUser)) {
throw new CannotDeleteException("질문을 삭제할 권한이 없습니다.");
}

List<DeleteHistory> deleteHistories = new ArrayList<>();
deleteHistories.add(deleteQuestion());
deleteHistories.addAll(answers.delete(loginUser));

return deleteHistories;
}

private DeleteHistory deleteQuestion() {
this.deleted = true;
return new DeleteHistory(ContentType.QUESTION, getId(), writer, LocalDateTime.now());
}

@Override
public String toString() {
return "Question [id=" + getId() + ", title=" + title + ", contents=" + contents + ", writer=" + writer + "]";
return "Question [id=" + getId() + ", title=" + content.getTitle() + ", contents=" + content.getContents() + ", writer=" + writer + "]";
}
}
18 changes: 18 additions & 0 deletions src/main/java/nextstep/qna/domain/QuestionContent.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package nextstep.qna.domain;

public class QuestionContent {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

private String title;
private String contents;

public QuestionContent(String title, String contents) {
this.title = title;
this.contents = contents;
}
public String getTitle() {
return title;
}

public String getContents() {
return contents;
}
}
26 changes: 5 additions & 21 deletions src/main/java/nextstep/qna/service/QnAService.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@

import nextstep.qna.CannotDeleteException;
import nextstep.qna.NotFoundException;
import nextstep.qna.domain.*;
import nextstep.qna.domain.AnswerRepository;
import nextstep.qna.domain.DeleteHistory;
import nextstep.qna.domain.Question;
import nextstep.qna.domain.QuestionRepository;
import nextstep.users.domain.NsUser;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

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

@Service("qnaService")
Expand All @@ -26,24 +27,7 @@ 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("질문을 삭제할 권한이 없습니다.");
}

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()));
}
List<DeleteHistory> deleteHistories = question.delete(loginUser);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
List<DeleteHistory> deleteHistories = question.delete(loginUser);
question.delete(loginUser);
List<DeleteHistory> deleteHistories = question.deleteHistories();

이와 같이 삭제 메시지와 deleteHistories 생성을 분리하는 것과 현재와 같이 구현하는 것 중 어느 접근 방식이 좋을까?
각각의 장단점은?

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

import nextstep.qna.CannotDeleteException;
import nextstep.users.domain.NsUserTest;
import org.junit.jupiter.api.Test;

import java.util.Arrays;
import java.util.List;

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

public class AnswersTest {

@Test
public void 모든_답변이_같은작성자면_삭제_가능() throws CannotDeleteException {
Question question = new Question(1L, NsUserTest.JAVAJIGI, "title", "contents");
Answer answer1 = new Answer(11L, NsUserTest.JAVAJIGI, question, "answer1");
Answer answer2 = new Answer(12L, NsUserTest.JAVAJIGI, question, "answer2");
Answers answers = new Answers(Arrays.asList(answer1, answer2));

List<DeleteHistory> delete = answers.delete(NsUserTest.JAVAJIGI);
}

@Test
public void 다른_작성자의_답변이_있으면_예외() {
Question question = new Question(1L, NsUserTest.JAVAJIGI, "title", "contents");
Answer answer1 = new Answer(11L, NsUserTest.JAVAJIGI, question, "answer1");
Answer answer2 = new Answer(12L, NsUserTest.SANJIGI, question, "answer2");
Answers answers = new Answers(Arrays.asList(answer1, answer2));

assertThatThrownBy(() -> {
answers.delete(NsUserTest.JAVAJIGI);
}).isInstanceOf(CannotDeleteException.class)
.hasMessageContaining("다른 사람의 답변이 존재하여 삭제할 수 없습니다.");
}

@Test
public void 모든_답변_삭제_및_DeleteHistory_반환() throws CannotDeleteException {
Question question = new Question(1L, NsUserTest.JAVAJIGI, "title", "contents");
Answer answer1 = new Answer(11L, NsUserTest.JAVAJIGI, question, "answer1");
Answer answer2 = new Answer(12L, NsUserTest.JAVAJIGI, question, "answer2");
Answers answers = new Answers(Arrays.asList(answer1, answer2));

List<DeleteHistory> deleteHistories = answers.delete(NsUserTest.JAVAJIGI);

assertThat(answer1.isDeleted()).isTrue();
assertThat(answer2.isDeleted()).isTrue();
assertThat(deleteHistories).hasSize(2);
assertThat(deleteHistories.get(0).getContentType()).isEqualTo(ContentType.ANSWER);
assertThat(deleteHistories.get(1).getContentType()).isEqualTo(ContentType.ANSWER);
}
}
Loading