Skip to content

[4기 박현지, 최지연] 컨트롤러 구현 및 테스트(RestDocs 적용), 엔티티 VO 분리 #263

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

Open
wants to merge 19 commits into
base: hyeonjiyeon
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: 3 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ java {
}

configurations {
asciidoctorExt
compileOnly {
extendsFrom annotationProcessor
}
Expand All @@ -33,9 +34,9 @@ dependencies {
runtimeOnly 'com.h2database:h2'
annotationProcessor 'org.projectlombok:lombok'
testImplementation 'org.springframework.boot:spring-boot-starter-test'
asciidoctorExt 'org.springframework.restdocs:spring-restdocs-asciidoctor'
testImplementation 'org.springframework.restdocs:spring-restdocs-mockmvc'
implementation 'org.springframework.boot:spring-boot-starter-validation:3.0.4'
implementation 'org.springframework.boot:spring-boot-starter-security'
}

tasks.named('test') {
Expand All @@ -45,5 +46,6 @@ tasks.named('test') {

tasks.named('asciidoctor') {
inputs.dir snippetsDir
configurations 'asciidoctorExt'
dependsOn test
}
76 changes: 76 additions & 0 deletions src/docs/asciidoc/index.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
:hardbreaks:
ifndef::snippets[]
:snippets: ../../build/generated-snippets
endif::[]

:icons: font
:source-highlighter: highlightjs
:toc: left
:toclevels: 2
:sectlinks:
:sectnums:
:docinfo: shared-head

== 게시글

=== 게시글 생성

==== [POST] /api/v1/posts

.Request
include::{snippets}/post-save/http-request.adoc[]

.Request Fields
include::{snippets}/post-save/request-fields.adoc[]

.Response
include::{snippets}/post-save/http-response.adoc[]

=== 게시글 조회

==== [GET] /api/v1/posts/{id}

.Request
include::{snippets}/post-get-one/http-request.adoc[]

.Response
include::{snippets}/post-get-one/http-response.adoc[]

.Response Fields
include::{snippets}/post-get-one/response-fields.adoc[]

=== 게시글 페이지 조회

==== [GET] /api/v1/posts?page={page}&size={size}&sort={sort}

.Request
include::{snippets}/post-get-by-page/http-request.adoc[]

.Response
include::{snippets}/post-get-by-page/http-response.adoc[]

.Response Fields
include::{snippets}/post-get-by-page/response-fields.adoc[]

=== 게시글 수정

==== [PATCH] /api/v1/posts/{id}

.Request
include::{snippets}/post-update/http-request.adoc[]

.Request Fields
include::{snippets}/post-update/request-fields.adoc[]

.Response
include::{snippets}/post-update/http-response.adoc[]

=== 게시글 삭제

==== [DELETE] /api/v1/posts/{id}

.Request
include::{snippets}/post-delete/http-request.adoc[]

.Response
include::{snippets}/post-delete/http-response.adoc[]
764 changes: 764 additions & 0 deletions src/docs/asciidoc/index.html

Large diffs are not rendered by default.

34 changes: 34 additions & 0 deletions src/main/java/org/prgms/boardservice/domain/post/Content.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package org.prgms.boardservice.domain.post;

import jakarta.persistence.Column;
import jakarta.persistence.Embeddable;
import jakarta.persistence.Lob;
import lombok.AccessLevel;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;

import static org.prgms.boardservice.util.ErrorMessage.INVALID_POST_CONTENT;
import static org.springframework.util.StringUtils.hasText;

@Embeddable
@EqualsAndHashCode
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Getter
public class Content {

@Lob
@Column(nullable = false)
private String content;

public Content(String content) {
validateContentLength(content);
this.content = content;
}

private void validateContentLength(String value) {
if (!hasText(value) || value.length() > 500) {

Choose a reason for hiding this comment

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

if 문의 조건들을 boolean type의 method으로 extract 해서 리펙토링해도 좋을것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

이미 해당 조건들은 validateContentLength라는 메서드로 분리했기 때문에 조건을 또 한 번의 메서드로 분리하게 되면 오히려 한눈에 파악하기 어렵다고 생각했습니다..!

throw new IllegalArgumentException(INVALID_POST_CONTENT.getMessage());
}
}
}
48 changes: 15 additions & 33 deletions src/main/java/org/prgms/boardservice/domain/post/Post.java
Original file line number Diff line number Diff line change
@@ -1,59 +1,41 @@
package org.prgms.boardservice.domain.post;

import jakarta.persistence.*;
import jakarta.validation.constraints.NotBlank;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;
import jakarta.validation.constraints.NotNull;
import lombok.*;
import org.hibernate.annotations.DynamicUpdate;
import org.prgms.boardservice.domain.BaseTime;

import static org.prgms.boardservice.util.ErrorMessage.INVALID_POST_CONTENT;
import static org.prgms.boardservice.util.ErrorMessage.INVALID_POST_TITLE;

@Builder

Choose a reason for hiding this comment

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

public 생성자도 사용하고 builder도 사용하고있나오?

@Getter
@Entity
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@AllArgsConstructor
@DynamicUpdate
Copy link

Choose a reason for hiding this comment

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

@DynamicUpdate를 사용한 이유가 무엇인가요?

Choose a reason for hiding this comment

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

jpa 공부하면서 컬럼 수가 굉장히 많은 경우가 아니라면 @DynamicUpdate 를 사용하는 것이 오히려 좋지 않을 수 있다는 것을 알게되었습니다. 해당 어노테이션 삭제하겠습니다!

@EqualsAndHashCode(of = "id", callSuper = false)
public class Post extends BaseTime {

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

@Column(length = 20)
@NotBlank
private String title;
@Embedded
private Title title;

@Lob
@NotBlank
private String content;
@Embedded
private Content content;
Comment on lines +25 to +26
Copy link

Choose a reason for hiding this comment

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

@Embedded를 사용할 때 주의사항이 뭐가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

같은 타입을 사용하게 되면 @AttributeOverride로 속성 명을 재정의 해줘야 하고 동등성 비교를 위해 equalshashCode를 재정의 해야 합니다!


@NotNull
private Long userId;

public Post(String title, String content) {
validateTitleLength(title);
validateContentLength(content);

public Post(Title title, Content content, Long userId) {
this.title = title;
this.content = content;
this.userId = userId;
}

public void update(String title, String content) {
validateTitleLength(title);
validateContentLength(content);

public void update(Title title, Content content) {
this.title = title;
this.content = content;
}

private void validateTitleLength(String title) {
if (!hasText(title) || title.length() > 20) {
throw new IllegalArgumentException(INVALID_POST_TITLE.getMessage());
}
}

private void validateContentLength(String content) {
if (!hasText(content) || content.length() > 500) {
throw new IllegalArgumentException(INVALID_POST_CONTENT.getMessage());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package org.prgms.boardservice.domain.post;

import org.prgms.boardservice.domain.post.dto.PageResponse;
import org.prgms.boardservice.domain.post.dto.PostCreateRequest;
import org.prgms.boardservice.domain.post.dto.PostResponse;
import org.prgms.boardservice.domain.post.dto.PostUpdateRequest;
import org.prgms.boardservice.domain.post.vo.PostUpdate;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.*;

import java.net.URI;
import java.util.NoSuchElementException;

@RestController
@RequestMapping("/api/v1/posts")
public class PostController {

private final PostService postService;

public PostController(PostService postService) {
this.postService = postService;
}

@ExceptionHandler(NoSuchElementException.class)

Choose a reason for hiding this comment

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

👍 예외 처리까지 하셨네요

Choose a reason for hiding this comment

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

Exception을 그런데 받는 이유가 있나요? NoSuchElementException를 받아도 되지 않나요?

Copy link
Author

Choose a reason for hiding this comment

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

private ResponseEntity<String> noSuchElementExceptionHandle(NoSuchElementException exception) {
return ResponseEntity.badRequest()
.body(exception.getMessage());
}

@PostMapping
public ResponseEntity<Void> create(@RequestBody PostCreateRequest postCreateRequest) {
Long postId = postService.create(postCreateRequest.toEntity());

return ResponseEntity.created(URI.create("/api/v1/posts/" + postId))
.build();
}

@GetMapping("/{id}")
public ResponseEntity<PostResponse> getOne(@PathVariable Long id) throws NoSuchElementException {
PostResponse postResponse = new PostResponse(postService.getById(id));

return ResponseEntity.ok(postResponse);
Comment on lines +41 to +44
Copy link
Member

Choose a reason for hiding this comment

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

NoSuchElementException을 던지는 것을 명시해준 이유가 있을까요?

Copy link

@ICCHOI ICCHOI Aug 16, 2023

Choose a reason for hiding this comment

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

저도 Exception을 달아둔 이유가 궁금하네요

}

@GetMapping
public ResponseEntity<PageResponse<PostResponse>> getPage(Pageable pageable) throws NoSuchElementException {
Page<Post> page = postService.getByPage(pageable);
Page<PostResponse> postResponseDtoPage = page.map(PostResponse::new);

return ResponseEntity.ok(new PageResponse<>(postResponseDtoPage));
}

@PatchMapping("/{id}")
Copy link

Choose a reason for hiding this comment

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

Put이 아니라 Patch인 이유가 있을까요?

Choose a reason for hiding this comment

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

이미 존재하는 자원에 대해서 부분적인 수정만 이루어지는 것을 의도해서 Patch 가 더 적합하다고 생각했습니다.

public ResponseEntity<Void> update(@RequestBody PostUpdateRequest postUpdateRequest, @PathVariable Long id) {
Long postId = postService.update(new PostUpdate(id, postUpdateRequest.title(), postUpdateRequest.content()));

return ResponseEntity.noContent()
.location(URI.create("/api/v1/posts/" + postId))
.build();
}

@DeleteMapping("/{id}")
public ResponseEntity<Void> delete(@PathVariable Long id) {
postService.deleteById(id);

return ResponseEntity.noContent()
.build();
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package org.prgms.boardservice.domain.post;

import org.prgms.boardservice.domain.post.vo.PostUpdateVo;
import org.prgms.boardservice.domain.post.vo.PostUpdate;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Service;
Expand All @@ -24,11 +24,11 @@ public Long create(Post post) {
return postRepository.save(post).getId();
}

public Long update(PostUpdateVo postUpdateVo) {
Post findPost = postRepository.findById(postUpdateVo.id())
public Long update(PostUpdate postUpdate) {
Post findPost = postRepository.findById(postUpdate.id())
.orElseThrow(() -> new NoSuchElementException(NOT_FOUND_POST.getMessage()));

findPost.update(postUpdateVo.title(), postUpdateVo.content());
findPost.update(new Title(postUpdate.title()), new Content(postUpdate.content()));

return postRepository.save(findPost).getId();
}
Expand All @@ -45,6 +45,7 @@ public Page<Post> getByPage(Pageable pageable) {
}

public void deleteById(Long id) {
postRepository.findById(id).orElseThrow(() -> new NoSuchElementException(NOT_FOUND_POST.getMessage()));
postRepository.deleteById(id);
}

Expand Down
32 changes: 32 additions & 0 deletions src/main/java/org/prgms/boardservice/domain/post/Title.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package org.prgms.boardservice.domain.post;

import jakarta.persistence.Column;
import jakarta.persistence.Embeddable;
import lombok.AccessLevel;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;

import static org.prgms.boardservice.util.ErrorMessage.INVALID_POST_TITLE;
import static org.springframework.util.StringUtils.hasText;

@Embeddable
@EqualsAndHashCode
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Getter
public class Title {

@Column(length = 20, nullable = false)
private String title;

public Title(String title) {
validateTitleLength(title);
this.title = title;
}

private void validateTitleLength(String value) {
if (!hasText(value) || value.length() > 20) {

Choose a reason for hiding this comment

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

Assert.hasText(value, message) 사용 고려해봐도 괜찮을것같아요

Copy link

Choose a reason for hiding this comment

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

여기의 20과 title의 lenght, db의 제약조건 등, 타이틀 길이를 변경하려면 해야하는 일이 너무 많아지는 것 같다는 생각이 들어요.

throw new IllegalArgumentException(INVALID_POST_TITLE.getMessage());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package org.prgms.boardservice.domain.post.dto;

import org.springframework.data.domain.Page;

import java.util.List;

public record PageResponse<E>(
List<E> data,
PageableResponse<E> pageable
) {
public PageResponse(Page<E> page) {
this(page.getContent(), new PageableResponse<>(page));
}

private record PageableResponse<E>(
boolean first,
boolean last,
int number,
int size,
String sort,
int totalPages,
long totalElements
) {
private PageableResponse(Page<E> page) {
this(
page.isFirst(),
page.isLast(),
page.getNumber(),
page.getSize(),
page.getSort().toString(),
page.getTotalPages(),
page.getTotalElements()
);
}
}
}
Loading