Skip to content

Commit 6264022

Browse files
authored
1단계 레거시 코드 리펙터링 (#791)
* docs: 1단계 요구사항 정리 * feat: 질문 삭제를 soft delete로 변경한다 * docs: 리팩터링 요구사항 추가 * refactor: 삭제 로직을 도메인 로직으로 이동 * refactor: stream을 활용해서 ident줄이기 * refactor: answers 일급컬렉션 적용 * refactor: answers의 delete가 검증과 삭제의 책임을 모두 갖도록 수정 * refactor: 미사용 getter와 모든 setter를 제거한다 * refactor: baseEntity 분리 * refactor: QuestionContent 분리
1 parent de587b9 commit 6264022

File tree

10 files changed

+252
-73
lines changed

10 files changed

+252
-73
lines changed

docs/step1요구사항.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# 문자열 사칙 연산 계산기 구현
2+
## 기능 요구사항
3+
4+
## 삭제 방식 변경
5+
- [x] 질문 데이터를 완전히 삭제하지 않는다(soft delete)
6+
- [x] deleted 상태(boolean type)를 true로 변경한다
7+
8+
## 삭제 권한 검증
9+
- [x] 로그인 사용자와 질문 작성자가 같은 경우에만 삭제 가능하다
10+
- [x] 답변이 없는 경우 삭제가 가능하다
11+
- [x] 질문자와 모든 답변 작성자가 같은 경우 삭제가 가능하다
12+
- [x] 질문자와 답변자가 다른 경우 질문을 삭제할 수 없다
13+
14+
## 연관 데이터 처리
15+
- [x] 질문을 삭제할 때 관련된 모든 답변도 함께 삭제한다
16+
- [x] 답변의 삭제도 soft deled로 deleted 상태를 true로 변경한다
17+
18+
## 삭제 이력 관리
19+
- [x] 질문 삭제 시 DeleteHistory를 생성하여 이력을 남긴다
20+
- [x] 답변 삭제 시에도 DeleteHistory를 생성하여 이력을 남긴다
21+
22+
23+
## 리팩터링 요구사항
24+
- [ ] deleteQuestion() 메서드의 비즈니스 로직을 도메인으로 이동한다.
25+

src/main/java/nextstep/qna/domain/Answer.java

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,6 @@ public Long getId() {
4747
return id;
4848
}
4949

50-
public Answer setDeleted(boolean deleted) {
51-
this.deleted = deleted;
52-
return this;
53-
}
54-
5550
public boolean isDeleted() {
5651
return deleted;
5752
}
@@ -64,14 +59,15 @@ public NsUser getWriter() {
6459
return writer;
6560
}
6661

67-
public String getContents() {
68-
return contents;
69-
}
70-
7162
public void toQuestion(Question question) {
7263
this.question = question;
7364
}
7465

66+
public DeleteHistory delete(){
67+
this.deleted = true;
68+
return new DeleteHistory(ContentType.ANSWER, id, writer, LocalDateTime.now());
69+
}
70+
7571
@Override
7672
public String toString() {
7773
return "Answer [id=" + getId() + ", writer=" + writer + ", contents=" + contents + "]";
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package nextstep.qna.domain;
2+
3+
import nextstep.qna.CannotDeleteException;
4+
import nextstep.users.domain.NsUser;
5+
6+
import java.util.ArrayList;
7+
import java.util.List;
8+
9+
public class Answers {
10+
private final List<Answer> answers;
11+
12+
public Answers() {
13+
this(new ArrayList<>());
14+
}
15+
16+
public Answers(List<Answer> answers) {
17+
this.answers = new ArrayList<>(answers);
18+
}
19+
20+
public void add(Answer answer) {
21+
answers.add(answer);
22+
}
23+
24+
public List<DeleteHistory> delete(NsUser loginUser) throws CannotDeleteException {
25+
checkDeletable(loginUser);
26+
List<DeleteHistory> histories = new ArrayList<>();
27+
for (Answer answer : answers) {
28+
histories.add(answer.delete());
29+
}
30+
return histories;
31+
}
32+
33+
private void checkDeletable(NsUser loginUser) throws CannotDeleteException {
34+
if (answers.stream().anyMatch(answer -> !answer.isOwner(loginUser))) {
35+
throw new CannotDeleteException("다른 사람의 답변이 존재하여 삭제할 수 없습니다.");
36+
}
37+
}
38+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package nextstep.qna.domain;
2+
3+
import java.time.LocalDateTime;
4+
5+
public abstract class BaseEntity {
6+
private Long id;
7+
private LocalDateTime createdDate = LocalDateTime.now();
8+
private LocalDateTime updatedDate;
9+
10+
public BaseEntity() {}
11+
12+
public BaseEntity(Long id) {
13+
this.id = id;
14+
}
15+
16+
public Long getId() {
17+
return id;
18+
}
19+
}

src/main/java/nextstep/qna/domain/DeleteHistory.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,18 @@ public DeleteHistory(ContentType contentType, Long contentId, NsUser deletedBy,
2626
this.createdDate = createdDate;
2727
}
2828

29+
public ContentType getContentType() {
30+
return contentType;
31+
}
32+
33+
public Long getContentId() {
34+
return contentId;
35+
}
36+
37+
public NsUser getDeletedBy() {
38+
return deletedBy;
39+
}
40+
2941
@Override
3042
public boolean equals(Object o) {
3143
if (this == o) return true;
Lines changed: 26 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,62 +1,37 @@
11
package nextstep.qna.domain;
22

3+
import nextstep.qna.CannotDeleteException;
34
import nextstep.users.domain.NsUser;
45

56
import java.time.LocalDateTime;
67
import java.util.ArrayList;
78
import java.util.List;
89

9-
public class Question {
10-
private Long id;
11-
12-
private String title;
13-
14-
private String contents;
10+
public class Question extends BaseEntity {
11+
private QuestionContent content;
1512

1613
private NsUser writer;
1714

18-
private List<Answer> answers = new ArrayList<>();
15+
private Answers answers = new Answers();
1916

2017
private boolean deleted = false;
2118

22-
private LocalDateTime createdDate = LocalDateTime.now();
23-
24-
private LocalDateTime updatedDate;
25-
2619
public Question() {
20+
super();
2721
}
2822

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

3327
public Question(Long id, NsUser writer, String title, String contents) {
34-
this.id = id;
35-
this.writer = writer;
36-
this.title = title;
37-
this.contents = contents;
38-
}
39-
40-
public Long getId() {
41-
return id;
42-
}
43-
44-
public String getTitle() {
45-
return title;
46-
}
47-
48-
public Question setTitle(String title) {
49-
this.title = title;
50-
return this;
51-
}
52-
53-
public String getContents() {
54-
return contents;
28+
this(id, writer, new QuestionContent(title, contents));
5529
}
5630

57-
public Question setContents(String contents) {
58-
this.contents = contents;
59-
return this;
31+
public Question(Long id, NsUser writer, QuestionContent content) {
32+
super(id);
33+
this.writer = writer;
34+
this.content = content;
6035
}
6136

6237
public NsUser getWriter() {
@@ -72,21 +47,29 @@ public boolean isOwner(NsUser loginUser) {
7247
return writer.equals(loginUser);
7348
}
7449

75-
public Question setDeleted(boolean deleted) {
76-
this.deleted = deleted;
77-
return this;
78-
}
79-
8050
public boolean isDeleted() {
8151
return deleted;
8252
}
8353

84-
public List<Answer> getAnswers() {
85-
return answers;
54+
public List<DeleteHistory> delete(NsUser loginUser) throws CannotDeleteException {
55+
if (!isOwner(loginUser)) {
56+
throw new CannotDeleteException("질문을 삭제할 권한이 없습니다.");
57+
}
58+
59+
List<DeleteHistory> deleteHistories = new ArrayList<>();
60+
deleteHistories.add(deleteQuestion());
61+
deleteHistories.addAll(answers.delete(loginUser));
62+
63+
return deleteHistories;
64+
}
65+
66+
private DeleteHistory deleteQuestion() {
67+
this.deleted = true;
68+
return new DeleteHistory(ContentType.QUESTION, getId(), writer, LocalDateTime.now());
8669
}
8770

8871
@Override
8972
public String toString() {
90-
return "Question [id=" + getId() + ", title=" + title + ", contents=" + contents + ", writer=" + writer + "]";
73+
return "Question [id=" + getId() + ", title=" + content.getTitle() + ", contents=" + content.getContents() + ", writer=" + writer + "]";
9174
}
9275
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package nextstep.qna.domain;
2+
3+
public class QuestionContent {
4+
private String title;
5+
private String contents;
6+
7+
public QuestionContent(String title, String contents) {
8+
this.title = title;
9+
this.contents = contents;
10+
}
11+
public String getTitle() {
12+
return title;
13+
}
14+
15+
public String getContents() {
16+
return contents;
17+
}
18+
}

src/main/java/nextstep/qna/service/QnAService.java

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@
22

33
import nextstep.qna.CannotDeleteException;
44
import nextstep.qna.NotFoundException;
5-
import nextstep.qna.domain.*;
5+
import nextstep.qna.domain.AnswerRepository;
6+
import nextstep.qna.domain.DeleteHistory;
7+
import nextstep.qna.domain.Question;
8+
import nextstep.qna.domain.QuestionRepository;
69
import nextstep.users.domain.NsUser;
710
import org.springframework.stereotype.Service;
811
import org.springframework.transaction.annotation.Transactional;
912

1013
import javax.annotation.Resource;
11-
import java.time.LocalDateTime;
12-
import java.util.ArrayList;
1314
import java.util.List;
1415

1516
@Service("qnaService")
@@ -26,24 +27,7 @@ public class QnAService {
2627
@Transactional
2728
public void deleteQuestion(NsUser loginUser, long questionId) throws CannotDeleteException {
2829
Question question = questionRepository.findById(questionId).orElseThrow(NotFoundException::new);
29-
if (!question.isOwner(loginUser)) {
30-
throw new CannotDeleteException("질문을 삭제할 권한이 없습니다.");
31-
}
32-
33-
List<Answer> answers = question.getAnswers();
34-
for (Answer answer : answers) {
35-
if (!answer.isOwner(loginUser)) {
36-
throw new CannotDeleteException("다른 사람이 쓴 답변이 있어 삭제할 수 없습니다.");
37-
}
38-
}
39-
40-
List<DeleteHistory> deleteHistories = new ArrayList<>();
41-
question.setDeleted(true);
42-
deleteHistories.add(new DeleteHistory(ContentType.QUESTION, questionId, question.getWriter(), LocalDateTime.now()));
43-
for (Answer answer : answers) {
44-
answer.setDeleted(true);
45-
deleteHistories.add(new DeleteHistory(ContentType.ANSWER, answer.getId(), answer.getWriter(), LocalDateTime.now()));
46-
}
30+
List<DeleteHistory> deleteHistories = question.delete(loginUser);
4731
deleteHistoryService.saveAll(deleteHistories);
4832
}
4933
}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package nextstep.qna.domain;
2+
3+
import nextstep.qna.CannotDeleteException;
4+
import nextstep.users.domain.NsUserTest;
5+
import org.junit.jupiter.api.Test;
6+
7+
import java.util.Arrays;
8+
import java.util.List;
9+
10+
import static org.assertj.core.api.Assertions.assertThat;
11+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
12+
13+
public class AnswersTest {
14+
15+
@Test
16+
public void 모든_답변이_같은작성자면_삭제_가능() throws CannotDeleteException {
17+
Question question = new Question(1L, NsUserTest.JAVAJIGI, "title", "contents");
18+
Answer answer1 = new Answer(11L, NsUserTest.JAVAJIGI, question, "answer1");
19+
Answer answer2 = new Answer(12L, NsUserTest.JAVAJIGI, question, "answer2");
20+
Answers answers = new Answers(Arrays.asList(answer1, answer2));
21+
22+
List<DeleteHistory> delete = answers.delete(NsUserTest.JAVAJIGI);
23+
}
24+
25+
@Test
26+
public void 다른_작성자의_답변이_있으면_예외() {
27+
Question question = new Question(1L, NsUserTest.JAVAJIGI, "title", "contents");
28+
Answer answer1 = new Answer(11L, NsUserTest.JAVAJIGI, question, "answer1");
29+
Answer answer2 = new Answer(12L, NsUserTest.SANJIGI, question, "answer2");
30+
Answers answers = new Answers(Arrays.asList(answer1, answer2));
31+
32+
assertThatThrownBy(() -> {
33+
answers.delete(NsUserTest.JAVAJIGI);
34+
}).isInstanceOf(CannotDeleteException.class)
35+
.hasMessageContaining("다른 사람의 답변이 존재하여 삭제할 수 없습니다.");
36+
}
37+
38+
@Test
39+
public void 모든_답변_삭제_및_DeleteHistory_반환() throws CannotDeleteException {
40+
Question question = new Question(1L, NsUserTest.JAVAJIGI, "title", "contents");
41+
Answer answer1 = new Answer(11L, NsUserTest.JAVAJIGI, question, "answer1");
42+
Answer answer2 = new Answer(12L, NsUserTest.JAVAJIGI, question, "answer2");
43+
Answers answers = new Answers(Arrays.asList(answer1, answer2));
44+
45+
List<DeleteHistory> deleteHistories = answers.delete(NsUserTest.JAVAJIGI);
46+
47+
assertThat(answer1.isDeleted()).isTrue();
48+
assertThat(answer2.isDeleted()).isTrue();
49+
assertThat(deleteHistories).hasSize(2);
50+
assertThat(deleteHistories.get(0).getContentType()).isEqualTo(ContentType.ANSWER);
51+
assertThat(deleteHistories.get(1).getContentType()).isEqualTo(ContentType.ANSWER);
52+
}
53+
}

0 commit comments

Comments
 (0)