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

[6주차 미션] 구현 완료했습니다. 리뷰 부탁드립니다. #77

Open
wants to merge 21 commits into
base: hyeon
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,5 @@ dependencies {
runtime('net.rakugakibox.spring.boot:logback-access-spring-boot-starter:2.7.1')
testCompile('org.springframework.boot:spring-boot-starter-test')
testCompile('org.assertj:assertj-core:3.10.0')
implementation 'org.projectlombok:lombok'
}
51 changes: 18 additions & 33 deletions src/main/java/codesquad/answer/Answer.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@

import codesquad.qua.Question;
import codesquad.user.User;
import lombok.Getter;
import lombok.NoArgsConstructor;

import javax.persistence.*;

@Entity
@Getter
@NoArgsConstructor
public class Answer {

@Id
Expand All @@ -16,54 +20,35 @@ public class Answer {
@JoinColumn(foreignKey = @ForeignKey(name = "fk_answer_to_question"))
private Question question;

private String writer;
@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(foreignKey = @ForeignKey(name = "fk_answer_to_user"))
private User writer;

private String comment;

private boolean deletedFlag = false;
private boolean deletedFlag;

public boolean isDeletedFlag() {
return deletedFlag;
public Answer(Question question, User writer, String comment) {
this.question = question;
this.writer = writer;
this.comment = comment;
deletedFlag = false;
}

public void changeDeletedFlag() {
deletedFlag = true;
}

public String getWriter() {
return writer;
}

public void setWriter(String writer) {
this.writer = writer;
}

public Long getId() {
return id;
}

public Question getQuestion() {
return question;
}

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

public String getComment() {
return comment;
}

public void setComment(String comment) {
this.comment = comment;
}

public void addQuestion(Question question) {
question.getAnswers().add(this);
this.question = question;
}

public boolean equalsWriter(User user) {
return writer.equals(user.getName());
return writer.equals(user);
}

public ResponseAnswerDto toResponseAnswerDto() {
return new ResponseAnswerDto(id, comment, question.getId(), writer.getName());
}
}
72 changes: 0 additions & 72 deletions src/main/java/codesquad/answer/AnswerController.java

This file was deleted.

6 changes: 6 additions & 0 deletions src/main/java/codesquad/answer/AnswerRepository.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
package codesquad.answer;

import org.springframework.data.jpa.repository.EntityGraph;
import org.springframework.data.jpa.repository.JpaRepository;

import java.util.Optional;

public interface AnswerRepository extends JpaRepository<Answer, Long> {

@EntityGraph(attributePaths = {"question"})
Optional<Answer> findQuestionFetchJoinById(Long id);
}
62 changes: 62 additions & 0 deletions src/main/java/codesquad/answer/AnswerService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package codesquad.answer;

import codesquad.qua.Question;
import codesquad.qua.QuestionRepository;
import codesquad.response.Result;
import codesquad.user.User;
import codesquad.utils.SessionUtil;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import javax.servlet.http.HttpSession;
import java.util.NoSuchElementException;

@Service
@Slf4j
@RequiredArgsConstructor
public class AnswerService {

private final AnswerRepository answerRepository;
private final QuestionRepository questionRepository;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Answer의 비즈니스 로직을 담당하는 AnswerService에서 Question관련 데이터가 필요할 때 QuestionRepository를 직접 참조하는 방법도 있지만, QuestionService를 통해 참조하도록 할 수도 있을 거 같은데 어떤 차이가 있을까요?


public Result<ResponseAnswerDto> create(long questionId, RequestAnswerDto requestAnswerDto, HttpSession session) {
log.info("comment = {}", requestAnswerDto.getComment());

User user = SessionUtil.getUserBySession(session);

if (user == null) {
return Result.fail("로그인 하세요");
}

Question question = questionRepository.findById(questionId)
.orElseThrow(NoSuchElementException::new);

Answer answer = new Answer(question, user, requestAnswerDto.getComment());
answer.addQuestion(question);
answerRepository.save(answer);

return Result.ok(answer.toResponseAnswerDto());
}

@Transactional
public Result<ResponseAnswerDto> remove(long questionId, long answerId, HttpSession session) {
User user = SessionUtil.getUserBySession(session);

if (user == null) {
return Result.fail("로그인 하세요");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Answer를 db에서 조회하는 로직 다음 세션 유저 체크가 이뤄지고 있습니다. 개선할 점이 있어보이는데 어떻게 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

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

오,, 이 부분은 DB 접근은 무거운 작업이기 때문에 세션을 체크하고 나서 DB 접근하는 것이 좋아보입니다
감사합니다!

Copy link
Author

Choose a reason for hiding this comment

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

이 부분만 그런게 아니라 다른 곳도 그렇게 작성되어있네요!!


Answer answer = answerRepository.findQuestionFetchJoinById(answerId)
.orElseThrow(NoSuchElementException::new);

if (!answer.equalsWriter(user)) {
return Result.fail("다른 사람 답글은 삭제 못해요");
}

answer.changeDeletedFlag();

return Result.ok(answer.toResponseAnswerDto());
Copy link
Collaborator

Choose a reason for hiding this comment

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

ResponseAnswerDto를 Result로 한번 더 감싸는 방식으로 구현하셨는데 어떤 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

미션 명세에서 Result를 반환하라고 해서 이렇게 작성했습니다!

}
}
27 changes: 27 additions & 0 deletions src/main/java/codesquad/answer/ApiAnswerController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package codesquad.answer;

import codesquad.response.Result;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.web.bind.annotation.*;

import javax.servlet.http.HttpSession;

@RestController
@RequiredArgsConstructor
@Slf4j
public class ApiAnswerController {

private final AnswerService answerService;

@PostMapping("/questions/{question-id}/answers")
public Result<ResponseAnswerDto> create(@PathVariable("question-id") Long questionId, @RequestBody RequestAnswerDto requestAnswerDto, HttpSession session) {
return answerService.create(questionId, requestAnswerDto, session);
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 요청의 응답 상태코드는 어떻게 나갈까요?

Copy link
Author

Choose a reason for hiding this comment

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

create메서드는 성공, 실패 여부없이 Result 를 반환하기 때문에 200 OK가 나갈 것이라고 생각합니다!

}

@DeleteMapping("/questions/{question-id}/answers/{answer-id}")
public Result<ResponseAnswerDto> remove(@PathVariable("question-id") Long questionId,
@PathVariable("answer-id") Long answerId, HttpSession session) {
return answerService.remove(questionId, answerId, session);
}
}
10 changes: 10 additions & 0 deletions src/main/java/codesquad/answer/RequestAnswerDto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package codesquad.answer;

import lombok.Getter;
import lombok.Setter;

@Getter
@Setter
public class RequestAnswerDto {
private String comment;
}
16 changes: 16 additions & 0 deletions src/main/java/codesquad/answer/ResponseAnswerDto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package codesquad.answer;

import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.Setter;

@Getter
@Setter
@AllArgsConstructor
public class ResponseAnswerDto {

private long id;
private String comment;
private long questionId;
private String writer;
}
11 changes: 11 additions & 0 deletions src/main/java/codesquad/exception/QuestionDeleteException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package codesquad.exception;

public class QuestionDeleteException extends RuntimeException {

public QuestionDeleteException() {
}

public QuestionDeleteException(String message) {
super(message);
}
}
11 changes: 11 additions & 0 deletions src/main/java/codesquad/exception/QuestionEditException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package codesquad.exception;

public class QuestionEditException extends RuntimeException {

public QuestionEditException() {
}

public QuestionEditException(String message) {
super(message);
}
}
Loading