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

[새리] step 5. XHR과 AJAX #142

Open
wants to merge 6 commits into
base: min27604
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ dependencies {
// h2 설정
implementation ('org.springframework.boot:spring-boot-starter-data-jpa')
implementation ('com.h2database:h2')

// swagger 2
compile group: 'io.springfox', name: 'springfox-swagger2', version: '2.9.2'
compile group: 'io.springfox', name: 'springfox-swagger-ui', version: '2.9.2'
Comment on lines +25 to +27
Copy link
Member

Choose a reason for hiding this comment

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

주석으로 어떤 의존성인지 파악할 수 있도록 해주신거 좋습니다.

이런 질문을 드릴 수 있을 것 같아요.

  • compileimplementation의 차이는 무엇인지 한 번 공부해보시겠어요?
  • Swagger는 REST API를 문서화하는 도구인데 이런 문서화도구는 왜 사용할까요?

}

test {
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/com/codessquad/qna/CommonExceptionHandler.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.codessquad.qna;

import com.codessquad.qna.domain.Result;
import com.codessquad.qna.exception.IllegalUserAccessException;
import com.codessquad.qna.exception.NotLoggedInException;
import com.codessquad.qna.exception.QuestionNotFoundException;
import com.codessquad.qna.exception.UserNotFoundException;
import org.springframework.web.bind.annotation.ControllerAdvice;
Expand All @@ -23,4 +25,9 @@ public String handleUserNotFoundException() {
public String handleQuestionNotFoundException() {
return "redirect:/";
}

@ExceptionHandler(NotLoggedInException.class)
public Result handleNotLoggedInException() {
Copy link
Member

Choose a reason for hiding this comment

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

저는 가급적이면 예외를 인자로 받아서 이를 통해서 예외 정보를 포함해 핸들링 해주는 것을 선호합니다.

return Result.fail("Login Needed");
}
}
32 changes: 29 additions & 3 deletions src/main/java/com/codessquad/qna/QnaApplication.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,38 @@

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.context.annotation.Bean;
import org.springframework.data.jpa.repository.config.EnableJpaAuditing;
import springfox.documentation.builders.ApiInfoBuilder;
import springfox.documentation.builders.PathSelectors;
import springfox.documentation.service.ApiInfo;
import springfox.documentation.spi.DocumentationType;
import springfox.documentation.spring.web.plugins.Docket;
import springfox.documentation.swagger2.annotations.EnableSwagger2;

@SpringBootApplication
@EnableJpaAuditing
@EnableSwagger2
public class QnaApplication {

public static void main(String[] args) {
SpringApplication.run(QnaApplication.class, args);
}
public static void main(String[] args) {
SpringApplication.run(QnaApplication.class, args);
}

@Bean
public Docket newsApi () {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public Docket newsApi () {
public Docket newsApi() {

newsApi라는 이름은 어떤 이유에서 지어주셨나요?

return new Docket(DocumentationType.SWAGGER_2)
.groupName("my-slipp")
.apiInfo(apiInfo())
.select()
.paths(PathSelectors.ant("/api/**"))
.build();
}

private ApiInfo apiInfo() {
return new ApiInfoBuilder()
.title("My Slipp API")
.description("My Slipp API")
.build();
}
}
29 changes: 13 additions & 16 deletions src/main/java/com/codessquad/qna/controller/AnswerController.java
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
package com.codessquad.qna.controller;

import com.codessquad.qna.domain.Answer;
import com.codessquad.qna.domain.Result;
import com.codessquad.qna.domain.User;
import com.codessquad.qna.exception.NotLoggedInException;
import com.codessquad.qna.service.AnswerService;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.*;

import javax.servlet.http.HttpSession;

import static com.codessquad.qna.HttpSessionUtils.getUserFromSession;
import static com.codessquad.qna.HttpSessionUtils.isLoginUser;

@Controller
@RequestMapping("/questions/{questionId}/answers")
@RestController
@RequestMapping("/api/questions/{questionId}/answers")
public class AnswerController {
private final AnswerService answerService;

Expand All @@ -23,25 +22,23 @@ public AnswerController(AnswerService answerService) {
}

@PostMapping
public String create(@PathVariable Long questionId, String contents, HttpSession session) {
public Answer create(@PathVariable Long questionId, String contents, HttpSession session) {
Copy link
Member

Choose a reason for hiding this comment

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

추가로 학습하실 거리를 제공해드릴게요.
REST API에서는 생성 성공시에 201 Created 응답을 반환해주곤 하는데요.
이를 어떻게 하면 Spring Applicaiton 상에서 제공해줄 수 있을까요?

Copy link
Member

Choose a reason for hiding this comment

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

자주 사용될 법한 응답코드에 대해서도 알아두시는 것을 추천드립니다!

if (!isLoginUser(session)) {
return "redirect:/users/login";
throw new NotLoggedInException();
}

User writer = getUserFromSession(session);
answerService.save(writer, contents, questionId);
return "redirect:/questions/{questionId}";
return answerService.save(writer, contents, questionId);
}

@DeleteMapping("/{id}")
public String delete(@PathVariable Long questionId, @PathVariable Long id, HttpSession session) {
public Result delete(@PathVariable Long questionId, @PathVariable Long id, HttpSession session) {
Copy link
Member

Choose a reason for hiding this comment

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

REST API에서는 삭제등을 성공했을 때, 별 다른 응답을 줄 필요가 없는경우
204 No Content 응답을 줍니다! 이는 성공했으나 Body에 아무런 내용도 포함하지 않는다는 의미에요!
그렇게 되면 Result는 굳이 필요없어지고, 예외가 발생했을 경우엔 예외에 해당하는 상태코드를 반환하므로써, 클라이언트는 상태코드를 보고 성공/실패 여부를 파악하고, 별도의 분기처리를 해주지 않아도 됩니다.

Copy link
Member

Choose a reason for hiding this comment

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

저는 id가 여러 개인 경우 answerId라고 특정지어주는것을 좋아합니다.

if (!isLoginUser(session)) {
return "redirect:/users/login";
throw new NotLoggedInException();
}

User loginUser = getUserFromSession(session);
answerService.deleteById(id, loginUser);

return "redirect:/questions/{questionId}";
// Returns either Result.ok() or Result.fail();
return answerService.deleteById(id, loginUser);
}
}
58 changes: 58 additions & 0 deletions src/main/java/com/codessquad/qna/domain/AbstractEntity.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package com.codessquad.qna.domain;

import com.fasterxml.jackson.annotation.JsonProperty;
import org.springframework.data.annotation.CreatedDate;
import org.springframework.data.annotation.LastModifiedDate;
import org.springframework.data.jpa.domain.support.AuditingEntityListener;

import javax.persistence.*;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.Objects;

@MappedSuperclass
@EntityListeners(AuditingEntityListener.class)
public class AbstractEntity {
Copy link
Member

Choose a reason for hiding this comment

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

Abstract 같은 키워드를 사용하는것은 안티패턴이고 최근에는 Base 같은 이름을 붙여주는 것이 더 일반적인 것 같아요~

private static final DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd kk:mm");
Copy link
Member

Choose a reason for hiding this comment

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

FORMATTER의 이름을 좀 더 우리 애플리케이션과 관련지어서 지어보는 것을 추천드리겠습니다!


@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
@JsonProperty
Copy link
Member

Choose a reason for hiding this comment

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

@JsonProperty를 사용해주신 이유는 무엇인가요?

private Long id;

@CreatedDate
private LocalDateTime createdDate;
Copy link
Member

Choose a reason for hiding this comment

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

이 데이터는 생성 일자 및 시간을 전부 저장할 필드기 때문에, 이름을 다르게 지어주는 것이 좋겠네요!


@LastModifiedDate
private LocalDateTime modifiedDate = LocalDateTime.now();
Copy link
Member

Choose a reason for hiding this comment

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

수정 일시를 현재로 해주는 이유는 무엇인가요?


public Long getId() {
return id;
}

public String getFormattedCreatedDate() { return createdDate.format(DATE_TIME_FORMATTER); }

public String getFormattedModifiedDate() { return modifiedDate.format(DATE_TIME_FORMATTER); }
Comment on lines +33 to +35
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public String getFormattedCreatedDate() { return createdDate.format(DATE_TIME_FORMATTER); }
public String getFormattedModifiedDate() { return modifiedDate.format(DATE_TIME_FORMATTER); }
public String getFormattedCreatedDate() {
return createdDate.format(DATE_TIME_FORMATTER);
}
public String getFormattedModifiedDate() {
return modifiedDate.format(DATE_TIME_FORMATTER);
}


@Override
public String toString() {
return "AbstractEntity{" +
"id=" + id +
", createdDate=" + getFormattedCreatedDate() +
", modifiedDated=" + getFormattedModifiedDate() +
'}';
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
AbstractEntity that = (AbstractEntity) o;
return Objects.equals(id, that.id);
}
Comment on lines +46 to +52
Copy link
Member

Choose a reason for hiding this comment

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

자 잘 생각해보면, id가 같다고 같은것이 맞을까요?
만약 Question과 Answer가 id가 같다면 equals는 어떤 응답을 반환해줄까요?


@Override
public int hashCode() {
return Objects.hash(id);
}
}
37 changes: 7 additions & 30 deletions src/main/java/com/codessquad/qna/domain/Answer.java
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
package com.codessquad.qna.domain;

import com.fasterxml.jackson.annotation.JsonManagedReference;

import javax.persistence.*;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.Objects;

@Entity
public class Answer {
private static final DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd kk:mm");

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

public class Answer extends AbstractEntity {
@ManyToOne
@JoinColumn(foreignKey = @ForeignKey(name = "fk_answer_writer"))
@JsonManagedReference
Copy link
Member

Choose a reason for hiding this comment

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

오 이 애노테이션을 찾아보고 사용하신건가요? 👍

private User writer;

@ManyToOne
Expand All @@ -24,20 +21,16 @@ public class Answer {
@Lob
Copy link
Member

Choose a reason for hiding this comment

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

아직은 DB영역까지 생각할 필요는 없지만, Lob이 어떤 형태로 저장되는지 한 번 살펴만 봅시다~

private String contents;

private final LocalDateTime timeCreated = LocalDateTime.now();

private boolean deleted = false;

protected Answer() {};
protected Answer() {}

public Answer(User writer, Question question, String contents) {
this.writer = writer;
this.question = question;
this.contents = contents;
}

public Long getId() { return id; }

public User getWriter() {
return writer;
}
Expand All @@ -50,8 +43,6 @@ public String getContents() {
return contents;
}

public String getFormattedTimeCreated() { return timeCreated.format(DATE_TIME_FORMATTER); }

public boolean isDeleted() {
return deleted;
}
Expand All @@ -60,28 +51,14 @@ public boolean isAnswerWriter(User writer) {
return this.writer.equals(writer);
}

public void delete() {
public void changeDeleteStatus() {
Copy link
Member

Choose a reason for hiding this comment

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

저는 이전 메서드 명이 더 나은것 같아요.
왜 변경되었나요?

this.deleted = true;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Answer answer = (Answer) o;
return Objects.equals(id, answer.id) &&
Objects.equals(question, answer.question);
}

@Override
public int hashCode() {
return Objects.hash(id, question);
}

@Override
public String toString() {
return "Answer{" +
"id=" + id +
super.toString() +
", writer=" + writer +
", questionId=" + question.getId() +
", contents='" + contents + '\'' +
Expand Down
35 changes: 10 additions & 25 deletions src/main/java/com/codessquad/qna/domain/Question.java
Original file line number Diff line number Diff line change
@@ -1,32 +1,30 @@
package com.codessquad.qna.domain;

import com.fasterxml.jackson.annotation.JsonBackReference;
import com.fasterxml.jackson.annotation.JsonManagedReference;

import javax.persistence.*;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.List;
import java.util.stream.Collectors;

@Entity
public class Question {
private static final DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd kk:mm");

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
public class Question extends AbstractEntity {

@ManyToOne
@JoinColumn(foreignKey = @ForeignKey(name = "fk_question_writer"))
@JsonManagedReference
private User writer;

private String title;

@Column(nullable = false, length = 2000)
private String contents;

private LocalDateTime timeCreated = LocalDateTime.now();

@OneToMany(mappedBy = "question")
@OrderBy("id ASC")
@OrderBy("id DESC")
@JsonBackReference
private List<Answer> answers;

private boolean deleted = false;
Expand All @@ -51,18 +49,6 @@ public String getContents() {
return contents;
}

public Long getId() {
return id;
}

public LocalDateTime getTimeCreated() {
return timeCreated;
}

public String getFormattedTimeCreated() {
return timeCreated.format(DATE_TIME_FORMATTER);
}

public List<Answer> getAnswers() {
return answers.stream().filter(answer -> !answer.isDeleted()).collect(Collectors.toList());
}
Comment on lines 52 to 54
Copy link
Member

Choose a reason for hiding this comment

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

이렇게 해주는 것도 좋지만, 쿼리에서 soft delete를 구현하는 방법도 있는데, 이걸 적용해보면 어떨까요?

Expand All @@ -71,7 +57,7 @@ public int getAnswerNumber() {
if (this.answers.isEmpty()) {
return 0;
}
return this.answers.size();
return getAnswers().size();
}
Comment on lines 57 to 61
Copy link
Member

Choose a reason for hiding this comment

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

이렇게 구현하게 되면, 단점이 하나 있습니다. Answer 전부를 불러오게 되는데요.
어떻게 해야할지 고민만! 해봐주세요!


public Question updateQuestion(Question modifiedQuestion) {
Expand All @@ -87,18 +73,17 @@ public boolean isSameUser(User user) {
public void delete() {
this.deleted = true;
for (Answer a : this.answers) {
Copy link
Member

Choose a reason for hiding this comment

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

변수명은 축약하기 보다는 answer라는 이름을 사용해주시는 것이 좋을 것 같고
불필요한 this는 읽는데 지장이 없다면 제거해주는 것이 좋다고 생각됩니다.

a.delete();
a.changeDeleteStatus();
}
}

@Override
public String toString() {
return "Question{" +
super.toString() +
"writer='" + writer + '\'' +
", title='" + title + '\'' +
", contents='" + contents + '\'' +
", id=" + id +
", timeCreated='" + timeCreated + '\'' +
'}';
}
}
25 changes: 25 additions & 0 deletions src/main/java/com/codessquad/qna/domain/Result.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package com.codessquad.qna.domain;

import com.fasterxml.jackson.annotation.JsonProperty;

public class Result {
Copy link
Member

Choose a reason for hiding this comment

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

굳이 없어도 되는 클래스라고 생각됩니다!
에러에 한해서는 고정된 형태를 응답하는게 좋겠지만, 성공응답은 굳이 형식을 강제할 이유는 없다고 생각이 돼요!


@JsonProperty
private boolean valid;

@JsonProperty
private String errorMessage;

private Result(boolean valid, String errorMessage) {
this.valid = valid;
this.errorMessage = errorMessage;
}

public static Result ok() {
return new Result(true, null);
}

public static Result fail(String errorMessage) {
return new Result(false, errorMessage);
}
}
Loading