[5기 송인재, 김현우] Spring Boot JPA 게시판 구현 미션 제출합니다. #278

ASak1104 · 2023-11-24T13:21:43Z

📌 과제 설명

👩‍💻 실행 방법

프로퍼티 설정

프로젝트 실행을 위해 main과 test 하위에 있는 resources/application.yml 파일을 수정해주세요.

spring:
  datasource:
    driver-class-name: com.mysql.cj.jdbc.Driver
    url: YOUR_DATABASE_URL
    username: YOUR_DB_USER_NAME
    password: YOUR_DB_PASSWORD

데이터베이스 설정

Dockerfile로 부터 이미지를 생성해주세요

$ docker build -t <IMAGE_NAME> .

아래 명령어를 통해 도커 컨테이너를 실행해주세요

$ docker run --name <CONTAINER_NAME> -e MYSQL_ROOT_PASSWORD=<PASSWORD> -e MYSQL_DATABASE=<DATABASE> -d -p <HOST_PORT>:<CONTAINER_PORT> <IMAGE_NAME>

✅ PR 포인트 & 궁금한 점

다룬 내용이 적지 않은 것 같아 노션에서 확인해주시면 감사하겠습니다!
가능하다면 코드 리뷰를 라이브로 진행하는 것도 좋을 것 같습니다!!
REST DOCS는 raw 파일로 보기 불편하기 때문에 해당 링크에서 보시는 걸 추천드립니다!!!

…ot-board-jpa-HI

use that of hibernate instead of jpa in order to avoid unexpected exception regarding of OS

…ot-board-jpa-HI

shoeone96 · 2023-11-25T13:03:01Z

src/main/java/com/devcourse/springbootboardjpahi/repository/UserRepository.java

PostRepository에서는 @repository annotation을 사용 안하셔서 둘 중 하나로 통일하는 게 좋을 것 같습니다.

앗 어노테이션 삭제하는 걸 깜빡했습니다. 감사합니다!

shoeone96 · 2023-11-25T13:06:03Z

src/main/java/com/devcourse/springbootboardjpahi/message/ControllerAdviceExceptionMessage.java

@NoArgsConstructor(access = AccessLevel.PRIVATE) 로 private 생성자를 만드는 방법도 있어서 소개해 드릴려고요

shoeone96 · 2023-11-25T13:12:33Z

src/main/java/com/devcourse/springbootboardjpahi/dto/PostResponse.java

Response로 나가는 데이터에 검증 절차를 넣는 건 어떻게 생각하시나요?
요청을 반활할 때도 저는 검증을 해주는 것도 나쁘지 않다고 생각해서요

알 수 없는 문제나 예기치 못한 DB 조작으로부터 방지하는 차원에서 검증을 하는 것으로 생각하면 좋을까요?

요구 사항을 만족하기 위한 처리 속도에 지장이 없다면 검증은 저도 다다익선이라고 생각합니다!

shoeone96 · 2023-11-25T13:14:55Z

src/main/java/com/devcourse/springbootboardjpahi/service/PostService.java

findAll 후 유저의 이름을 PostResponse에서 호출하게 되는데 이때 user의 정보를 가져오기 위해 쿼리가 한번 더 나가는 걸로 알고 있습니다. 이 요청 시에는 반드시 user의 이름을 가져와야 하니 한번에 join으로 호출하는 건 어떨가요?

확실히 두번 나가는 것보단 join이라도 한번 나가는 것이 좋을 것 같습니다!!

말씀해주신데로 Post 조회 후 각 Post에 대한 User를 찾기 위해 select 문이 추가로 호출되긴 합니다! 그런데 해당 쿼리는 JPA에서 자동으로 db에 날리는 쿼리라 join으로 호출하도록 커스텀 하는게 가능할 지 궁금하네요 🤔
단일 join 쿼리가 성능 상 더 빠를지도 궁금하고요!

shoeone96 · 2023-11-25T13:19:38Z

src/main/java/com/devcourse/springbootboardjpahi/service/PostService.java

혹시 builder 생성자를 숨기고 정적 스태택 메서드를 사용하는 방법은 어떻게 생각하시나요? 개인적으로는 생성자보다는 특정 상황에서 그에 맞는 생성자를 사용하자는 주의라 의견 남겨봤습니다.
(builder를 사용하는 순간 뭔가 서비스 메서드도 길어지는 느낌이 있어서요, 저는 생성할 때 필요한 최소한의 정보 dto 자체나 필요한 다른 정보만 넘기고 생성로직을 도메인에서만 사용하게 만드는 것을 선호합니다)

두가지 방식의 장단점이 명확하다고 생각합니다. 롬복에서 빌더 패턴을 쉽게 제공해주니 한 번 써보자는 취지에서 써 본 것이 컸습니다. 이번 과제에 사용해보면서 장점도 느꼈고, 말씀대로 단점도 느꼈습니다.

체감했던 장점으로는

도메인이 커지지 않으면서 서비스 메서드의 생성 로직이 생성자를 사용했을 때 보다 가독성이 괜찮다고 느껴졌고

파라미터 순서를 실수하는 경우가 적어지겠다 느꼈습니다.

롬복과 jpa를 사용하면서 게터와 세터 사용에 대해 무뎌지게 되고 기본적으로 멤버 변수에 final을 못 붙이니 잘못하면 나중에 애플리케이션이 커지면 문제가 될 수 있겠다 싶습니다. 그래서 저도 명확하게 정적 팩토리 메서드 사용하는 것이 더 나을 때가 많은 것 같습니다!

엔티티 자체가 리플렉션으로 만들어질 수 있어서 final 필드 적용이 불가하기 때문에 "롬복과 jpa" 그리고 빌더 패턴으로 인한 문제라기 보다 객체의 생성을 서비스에서 하는 것보단 엔티티에서 하는 것은 어떠한 가에 대해서 생각을 해볼 필요가 있는 것 같네요!

저는 개인적으로 엔티티가 DTO 객체의 존재 여부를 알 필요가 있을까? 라는 생각합니다! 엔티티는 각 필드에 대한 정보와 validation 정도를 포함하는 방식이 이상적인 것 같아서 서비스 로직의 코드가 길어지는 것이 엔티티 클래스의 코드가 길어지는 것 보다 훨씬 자연스러운 방향이 아닐까? 라고 생각합니다!

그리고 빌더 패턴을 사용하면 널 필드가 존재할 때 각 상황에 필요한 생성자를 모두 만들어야 되나? 아니면 전체 생성자에서 특정 필드에 null을 넣어야하나? 와 같은 고민을 해결할 수 있기 때문에 필드가 많고 상황에 따라 호출되는 생성자가 많다면 빌더 패턴을 적용하는 것이 좋은 선택지라고 생각합니다! 그리고 빌더 패턴을 사용하는 이유가 객체의 너무 많은 필드로 인해 코드 가독성을 해치는 것을 개행으로 해결하는 것이기 때문에 빌더 패턴으로 메서드가 길어지는 건 가독성과 큰 관련이 없다고 생각해요!

shoeone96 · 2023-11-25T13:23:21Z

src/main/java/com/devcourse/springbootboardjpahi/service/PostService.java

혹시 update 메서드에서 title과 content를 각각 수정하신 이유를 알 수 있을까요?
SRP로 해석하면 될까요?

저는 개인적으로 title과 content를 동시에 수정하는 메서드가 있는 것 보다 각 필드에 대한 setter를 호출하는게 더 좋은 것 같아요!
개인적으로 가독성이 좋은 것 같고 메서드 호출할 때 필드 순서로 인한 오류도 방지할 수 있고 추후에 각각 사용할 수 있으니 범용성도 넓은 것 같고요!

shoeone96 · 2023-11-25T13:28:37Z

src/main/java/com/devcourse/springbootboardjpahi/dto/UserResponse.java

생성하는 Response로직에서 생성한 데이터 정보 전체를 전달해주시는 이유를 알 수 있을까요?
프론트에서 점검하기 위한 용도인가요?

넵! 프론트에서 원하는 정보가 있다고 가정하고 했습니다!

yenzip

테스트 코드가 굉장히 꼼꼼하게 작성되었네요! 👍
코드를 살펴보면서 미처 생각하지 못했던 부분에 대해 많이 배웠습니다.

yenzip · 2023-11-25T15:35:38Z

src/main/java/com/devcourse/springbootboardjpahi/service/PostService.java

+    private final PostRepository postRepository;
+    private final UserRepository userRepository;
+
+    public PostResponse create(CreatePostRequest request) {


update에는 @transactional을 사용하셨는데, create에는 사용하지 않은 이유가 있을까요?

update는 더티 체킹을 위해 트랜잭션이 필요하지만 create는 필요하지 않다고 판단해서 사용하지 않았습니다

게시글을 저장할 때 조회 문을 제외하면 단일 inseret 쿼리가 호출되기 때문에 따로 롤백이 필요없다고 생각했어요!

yenzip · 2023-11-25T15:39:50Z

src/main/java/com/devcourse/springbootboardjpahi/service/UserService.java

+import org.springframework.stereotype.Service;
+
+@Service
+@RequiredArgsConstructor


UserService에서는 @transactional을 사용하지 않은 이유가 있을까요?

필요하지 않다고 생각해서 사용하지 않았습니다!

yenzip · 2023-11-25T15:46:11Z

src/main/resources/application.yaml

+  data:
+    web:
+      pageable:
+        default-page-size: 10


application.yml에서는 기본 페이지 사이즈를 설정해주셨군요!
@PageableDefault(size = 10)을 사용하여 기본 페이지 크기를 설정해보는 것은 어떨까요?

그것도 좋은 방법이라고 생각합니다!!

맞습니다! 코드로서 관리했을 때의 이점이 있어서 해당 어노테이션을 사용하는 것도 좋을 것 같아요!
저는 기본으로 페이징되는 사이즈를 변경하기 위해 코드를 다시 빌드할 필요는 없지 않을까 라고 생각해서 프로퍼티로 정의했습니다!

yenzip · 2023-11-25T16:18:16Z

src/main/java/com/devcourse/springbootboardjpahi/domain/Post.java

+    @GeneratedValue(strategy = GenerationType.IDENTITY)
+    private long id;
+
+    @Column(name = "title", nullable = false)


title에서 글자수를 제한할 수 있는 설정을 추가해보는 것은 어떨까요?

글자 수 제한을 이번 과제에선 고려하지 않는 방향으로 진행했는데 말씀대로 필요한 경우가 있을 수도 있다는 생각이 듭니다!

twotwobread · 2023-12-04T15:39:15Z

src/main/resources/application.yaml

+  datasource:
+    driver-class-name: com.mysql.cj.jdbc.Driver
+    url: jdbc:mysql://localhost:33366/prod
+    username:


env로 관리해보면 좋을 것 같습니다.
datasource:
driver-class-name: com.mysql.cj.jdbc.Driver
url: jdbc:mysql://localhost:3306/${MYSQL_DATABASE}
username: ${MYSQL_USER}
password: ${MYSQL_PASSWORD}

이런 느낌으로 쓰는데 MYSQL_USER 환경변수들을 가진 env 파일을 두고 거기서 읽어오는 방식으로 바꾸고 env 파일은 git에 올리지 않으면 보안적으로 더 좋을 것 같습니다.

cmd 명령을 통해 환경변수를 받는 경우에도 위처럼 하면 yml 세팅도 따로 할 필요가 없을 것 같습니다.

twotwobread · 2023-12-04T15:42:39Z

src/main/java/com/devcourse/springbootboardjpahi/controller/PostController.java

+    private final PostService postService;
+
+    @GetMapping
+    public ResponseEntity<PageResponse<PostResponse>> find(Pageable pageable) {


offset, pageSize 값에 대한 validation은 필요없나요?? 현재는 컨텐츠의 총 개수를 넘는 요청이 오거나 음수의 페이지를 요청하는 데이터가 왔을 때 어떻게 동작하나요??

twotwobread · 2023-12-04T15:49:17Z

src/main/java/com/devcourse/springbootboardjpahi/dto/PageResponse.java

+        List<T> content
+) {
+
+    public static <E> PageResponse<E> from(Page<E> page) {


Page도 추상체라서 의존성이 낮을 것 같고 반환되는 json 형태도 거의 유사할 것 같은데 PageResponse로의 변환을 해준 이유가 있을까요??

twotwobread · 2023-12-04T15:53:28Z

src/main/java/com/devcourse/springbootboardjpahi/dto/CreatePostRequest.java

+        @NotNull(message = PostExceptionMessage.NULL_CONTENT)
+        String content,
+        @Positive(message = PostExceptionMessage.INVALID_USER_ID)
+        Long userId


Null인 경우도 validation 해주면 좋을 것 같습니다.

primitive로 id 값을 줘서 그런 거라면 dto에서도 primitive를 써도 될 것 같습니다.

twotwobread · 2023-12-04T16:13:11Z

src/test/java/com/devcourse/springbootboardjpahi/controller/PostControllerTest.java

+import org.springframework.test.web.servlet.ResultActions;
+
+@WebMvcTest(PostController.class)
+class PostControllerTest {


개인적으론 해당 mockMvc를 이용한 테스트가 docs 테스트에서도 충분히 experct될 수 있다는 생각이 들어서 TestRestTemplate와 같은 방법들을 찾아보고 이를 이용하여 e2e 테스트를 짜보면 어떨까는 생각이 듭니다!

twotwobread · 2023-12-04T16:15:07Z

src/test/java/com/devcourse/springbootboardjpahi/domain/PostTest.java

+        String content = faker.howIMetYourMother().catchPhrase();
+
+        // when
+        Post expected = Post.builder()


객체를 생성하는 부분들이 중복되니까 test용 util 함수로 빼서 테스트에서 사용해보면 좋을 것 같습니다.

twotwobread · 2023-12-04T16:22:42Z

src/test/java/com/devcourse/springbootboardjpahi/service/PostServiceTest.java

+
+        // then
+        assertThat(postDetailResponse)
+                .hasFieldOrPropertyWithValue("id", post.id())


usingRecursiveComparision 이거 이용해보면 어떨까 생각합니다. property를 문자열로 직접 입력해야 한다는 부분이 좋게 느껴지진 않습니다.
https://ksh-coding.tistory.com/100#%F0%9F%8E%AF%203.%C2%A0%20AssertJ%EC%9D%98%20usingRecursiveComparison%20%EC%82%B4%ED%8E%B4%EB%B3%B4%EA%B8%B0-1

IjjS and others added 30 commits November 21, 2023 14:09

init: initialize the project

b7821b8

feat: defint entities

55974ec

chore: configure the datasource

9bc80e2

chore: configure the datasource

8391918

fix: remove last comma

1976759

fix: change generate strategy

91b19d6

Merge branch 'main' of https://github.com/spring-comes-to-us/springbo…

1c4b600

…ot-board-jpa-HI

fix: allow null value for author and modifier

6794825

fix: change primitive type to wrapper

fe828a6

chore: add faker dependency

a450a44

test: user instance test

8adcfe9

Merge branch 'main' of https://github.com/spring-comes-to-us/springbo…

670ffc4

…ot-board-jpa-HI

chore: fix typo

bfa82a4

Merge branch 'main' of https://github.com/spring-comes-to-us/springbo…

9ea8acd

…ot-board-jpa-HI

chore: add foreign key to users

6a7a416

enhance: add mappings to post and user

b5098b8

test: test simple mapping test of user and posts

fa95ab1

fix: fix auditing strategy

00c2636

use that of hibernate instead of jpa in order to avoid unexpected exception regarding of OS

refactor: enhance readability

0a122a1

test: fix incorrect exception thrown

1a7dadd

feat: implement jpa repository

c2b1477

feat: add create and find all services of user

086071c

chore: specify properties

0b9c79b

refactor: change the purpose of dto

257d93b

feat: add user controller

1012a54

feat: add getter

c154bf7

refactor: change return type to dto

5cdba05

test: add post test

d4245bd

test: rename to reasonable test method names

9a45326

Merge branch 'main' of https://github.com/spring-comes-to-us/springbo…

f97f9cb

…ot-board-jpa-HI

IjjS and others added 13 commits November 24, 2023 15:00

chore: add asciidoc dependencies and task definitions

6e5cdeb

feat: add post controller documentation

bd71f4c

feat: add post controller documentation

f606f15

test: separate docs tests

95cfd91

test: add thread sleep before updating post

405736f

style: reformat blank line convention

6fa6390

test: remove unnecessary print

3dcfe85

test: set post content to nullable field

7f6ce0d

test: add user controller rest docs test

885ff42

test: add thread sleep before updating post content

6864f67

docs: add user api rest docs

5213ca5

refactor: change error message to constant

d552593

feat: add no such element exception handler

06b670c

ASak1104 added 🔥 매운맛 Team-Marco labels Nov 24, 2023

ASak1104 requested review from ksy90101, twotwobread, shoeone96 and yenzip November 24, 2023 13:21

ASak1104 assigned ASak1104 and IjjS Nov 24, 2023

ASak1104 added 6 commits November 24, 2023 22:40

test: add message validation

32b85e9

test: add tests no such user and post

2547eeb

test: add rest docs about not exist post detail

ea18cd5

test: rollback post content field type

2611601

add rest docs about updating not exist post

ec83d42

add rest docs about not exist user post

4ed70ec

shoeone96 reviewed Nov 25, 2023

View reviewed changes

yenzip reviewed Nov 25, 2023

View reviewed changes

twotwobread reviewed Dec 4, 2023

View reviewed changes

[5기 송인재, 김현우] Spring Boot JPA 게시판 구현 미션 제출합니다. #278

Are you sure you want to change the base?

[5기 송인재, 김현우] Spring Boot JPA 게시판 구현 미션 제출합니다. #278

Conversation

ASak1104 commented Nov 24, 2023 • edited

📌 과제 설명

👩‍💻 실행 방법

프로퍼티 설정

데이터베이스 설정

✅ PR 포인트 & 궁금한 점

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ASak1104 Nov 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ASak1104 Nov 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ASak1104 Nov 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yenzip left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ASak1104 commented Nov 24, 2023 •

edited

ASak1104 Nov 30, 2023 •

edited

ASak1104 Nov 30, 2023 •

edited

ASak1104 Nov 30, 2023 •

edited