[5기 남은찬, 박주한] Spring Boot JPA 게시판 구현 미션 제출합니다. #267

ParkJuhan94 · 2023-11-22T06:17:32Z

요구 사항

SpringDataJPA 를 설정한다.

datasource : h2 or mysql

엔티티를 구성한다

회원(User)
- id (PK) (auto increment)
- name
- age
- hobby
- created_at
- created_by
게시글(Post)
- id (PK) (auto increment)
- title
- content
- created_at
- created_by
회원과 게시글에 대한 연관관계를 설정한다.
- 회원과 게시글은 1:N 관계이다.
게시글 Repository를 구현한다. (PostRepository)

API를 구현한다.

게시글 조회
- 페이징 조회 (GET "/posts")
- 단건 조회 (GET "/posts/{id}")
게시글 작성 (POST "/posts")
게시글 수정 (POST "/posts/{id}")

REST-DOCS를 이용해서 문서화한다.

✅ PR 포인트 & 궁금한 점

Co-authored-by: name [email protected]

ordilov

과제 고생하셨습니다~!!

전반적으로 깔끔하게 작성해주신 점이 좋았습니다.
아쉬웠던 부분은 유효성 검사가 다 빠져있는데 이 부분은 조금 고민해보시면 좋을 것 같습니다!

ordilov · 2023-11-23T15:56:45Z

src/main/java/kdt/jpa/board/global/entity/BaseEntity.java

+
+    @LastModifiedDate
+    @Column(columnDefinition = "TIMESTAMP")
+    private LocalDateTime updatedDate;


질문) createdDate는 protected 로 updatedDate는 private으로 선언하신 이유가 있을까요?

왜 이렇게 선언돼있는지 모르겠네요 .. 실수한 것 같아요 수정하겠습니다!

ordilov · 2023-11-23T15:57:31Z

src/main/java/kdt/jpa/board/global/entity/BaseEntity.java

+
+    @LastModifiedDate
+    @Column(columnDefinition = "TIMESTAMP")
+    private LocalDateTime updatedDate;


질문) ZonedDateTime,Instant 대신 LocalDateTime을 사용하신 이유가 있을까요?

솔직히 특별한 이유 없고, 자바에선 항상 LocalDateTime 만 사용해와서 사용했습니다

지나가다가 발견해서 코멘트 남깁니다.
면접에서 자바8 기능에 대하여 종종 물어볼 수도 있어서 참고해보시면 좋을 것 같아요
https://min103ju.github.io/java/localdatetime/

ordilov · 2023-11-23T15:59:45Z

src/main/java/kdt/jpa/board/global/entity/BaseEntity.java

+
+    @LastModifiedDate
+    @Column(columnDefinition = "TIMESTAMP")
+    private LocalDateTime updatedDate;


LocalDateTime 필드의 네이밍을 ~Date로 선언하신 이유가 있을까요?
LocalDate 필드를 사용할 일이 있을 때 변수명을 보고 datetime 값을 가지고 있는지 date 값을 가지고 있는지 확인하기 어려울 것 같습니다.

넵 정확한 네이밍 으로 수정하겠습니다!

ordilov · 2023-11-25T02:34:17Z

src/main/java/kdt/jpa/board/global/entity/BaseEntity.java

+
+    @LastModifiedDate
+    @Column(columnDefinition = "TIMESTAMP")
+    private LocalDateTime updatedDate;


요구사항은 created_at 과 created_by를 만드는 걸로 기억하고 있는데 created_by는 빠진 이유가 있을까요?

넵 빠트린 created_by 필드도 추가해야겠습니다

ordilov · 2023-11-25T02:39:01Z

src/main/java/kdt/jpa/board/global/exception/BoardException.java

+@RequiredArgsConstructor
+public class BoardException extends RuntimeException {
+
+    private final String message;


질문) RuntimeException을 상속받았으면 message를 인자로 전달해줄 수 있는데 다시 정의하신 이유가 있을까요?

생각해보니 없어도 되는 필드같네요! 수정하겠습니다

ordilov · 2023-11-25T05:39:48Z

src/test/java/kdt/jpa/board/post/controller/PostControllerTest.java

테스트는 잘 작성해주셨는데 AsciiDoc 다른 파일들 (index.adoc 등?)은 구현 안하셨을까요?

ordilov · 2023-11-25T06:11:58Z

src/test/java/kdt/jpa/board/post/fixture/PostFixture.java

+
+    public static Post getPost(Long id) {
+        Post post = getPost();
+        ReflectionTestUtils.setField(post, "id", id);


get을 하는데 안에서 그 값을 설정하는게 어색한 거 같은데 어떻게 생각하시나요?
setField 를 사용할 땐 사용하는게 맞나 구조적으로 한 번 더 고민해보시면 좋습니다.

ordilov · 2023-11-25T06:19:21Z

src/test/java/kdt/jpa/board/post/service/PostServiceTest.java

+            Post post = PostFixture.getPost(1L);
+            User user = UserFixture.getUser();
+            CreatePostRequest request = new CreatePostRequest(post.getTitle(), post.getContent(), 1L);
+
+            given(postRepository.save(any(Post.class)))
+                .willReturn(post);
+            given(userRepository.findById(1L))
+                .willReturn(Optional.of(user));
+
+            //when
+            Long result = postService.createPost(request);
+
+            //then
+            assertThat(result).isEqualTo(1L);


테스트 픽스처 생성 부분에서 어색한 부분이 있습니다!
Post를 생성하는 테스트인데 픽스처에서 Post를 미리 만들어야할까요?

테스트하려는 부분도 어색한 부분이 있습니다.
ID는 유일성을 보장하는 값으로, 기존에 없는 값으로 어떤 값이 나오게 될지 모를텐데 ID가 일치하는지 확인해도 될까요?
실제로 유저가 게시글을 만들 때 잘 생성되었는지 확인하고 싶을 때, ID가 예상한대로 나왔구나 생각할지 고민해보시면 좋습니다.

ordilov · 2023-11-25T06:23:05Z

src/test/java/kdt/jpa/board/common/support/ApiTestSupport.java

+@Transactional
+@AutoConfigureMockMvc
+@ExtendWith(RestDocumentationExtension.class)
+public abstract class ApiTestSupport extends TestContainerSupport {


질문) ApiTest를 하는데 TestContainer 설정을 넣으신 이유가 궁금합니다.
ApiTest를 할때마다 데이터베이스가 꼭 필요할까요?

ordilov · 2023-11-25T07:40:35Z

src/test/resources/application-local.yml

local은 datasource를 mysql을 쓰고, 기본은 testcontainers를 쓰는게 살짝 profile 분리가 어색해 보입니다.

hoon-space

과제 고생하셨습니다.
리뷰 확인부탁드힙니다. ~~

hoon-space · 2023-11-26T08:16:06Z

src/main/java/kdt/jpa/board/global/entity/BaseEntity.java

+public class BaseEntity {
+
+    @CreatedDate
+    @Column(columnDefinition = "TIMESTAMP")
+    protected LocalDateTime createdDate;
+


인스턴스화하지 않을 객체이니 추상클래스로 변경해보는 것은 어때요 ??

앗 맞는말인거같아요! 수정하겠습니다

hoon-space · 2023-11-26T08:17:16Z

src/main/java/kdt/jpa/board/global/entity/BaseEntity.java

+    @Column(columnDefinition = "TIMESTAMP")
+    private LocalDateTime updatedDate;


개인적으로 모든 엔티티 필드에 컬럼 애너테이션을 달고 컬럼 이름을 명시하는 것을 선호해요

넵 한번 적용해보겠습니다

hoon-space · 2023-11-26T08:23:55Z

src/main/java/kdt/jpa/board/post/controller/PostController.java

+    public ResponseEntity<Long> savePost(@RequestBody CreatePostRequest request) {
+        Long postId = postService.createPost(request);
+
+        return ResponseEntity.ok(postId);


응답 값만 보아서는 어떤 값이 리턴이 되는 지 알기 어렵습니다.
객체화해서 그 객체 필드에 postId 를 전달하는 것이 명확할 것 같아여

이 부분에 대해서 솔직히 id 를 응답할 필요가 없을거같아서 리턴 타입 자체를 불린으로 바꾸려 하는데, 불린에 대한 응답도 한번 객체화하는게 좋을까요?

불린은 괜찬을듯

hoon-space · 2023-11-26T08:26:07Z

src/main/java/kdt/jpa/board/post/controller/PostController.java

+    }
+
+    @GetMapping
+    public ResponseEntity<PostListResponse> getPosts(@PageableDefault Pageable pageable) {


@PageableDefault 를 사용하면 어떤 효과가 있나여 ??

상세한 size 나 page 에 대해서 특별한 요구사항을 정하지 않아서 페이징 정보를 받지 않았을 때, 기본으로 @PageableDefault 에서 지정되는 디폴트 값으로 설정하려고 사용했습니다!

hoon-space · 2023-11-26T08:28:14Z

src/main/java/kdt/jpa/board/post/domain/Post.java

+    public Post(String title, String content, User user) {
+        this.title = title;
+        this.content = content;
+        this.user = user;


정적팩토리메서드 패턴을 바꿔도 좋을 것 같아요

hoon-space · 2023-11-26T08:33:10Z

src/main/java/kdt/jpa/board/post/service/dto/request/CreatePostRequest.java

+@JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY)
+public record CreatePostRequest(


fieldVisibility = JsonAutoDetect.Visibility.ANY 는 어떤 역할을 하나요 ????

정말 이유를 모르겠지만 @RequestBody 에서 레코드를 역직렬화를 못해서 지정했습니다
항상 이상없이 잘 돼왔었는데... 일단 급한대로 해당 어노테이션으로 해결했습니다

hoon-space · 2023-11-26T08:34:10Z

src/main/java/kdt/jpa/board/post/service/dto/response/PostListResponse.java

+public record PostListResponse(
+    Page<PostResponse> responses


reponses 보다는 postInfos 혹은 posts 정도가 나은 것 같아아요

hoon-space · 2023-11-26T08:34:47Z

src/main/java/kdt/jpa/board/user/domain/User.java

+@Table(name = "users")
+public class User extends BaseEntity {


테이블 이름을 복수형으로 선언하신 이유가 있을까요???

user 가 mysql 에서 이미 사용중인 네이밍이라 테이블 이름으로 사용이 안되는거로 알고있어서 복수형으로 선언했습니다!

아하 이미 사용중인 네임이였군여, 어쩐지 대부분 MEMBER, ACCOUNT 이런식으로 테이블 이름을 사용하더라고요 ㅎㅎㅎ

hoon-space · 2023-11-26T08:36:28Z

src/main/resources/logback.xml

+    </encoder>
+  </appender>
+
+  <logger name="io.undertow" level="OFF"/>


언더토우 사용하셨나요 ??? ㅎㅎㅎ

@ParkJuhan94

아 로거를 잘 보고싶어서 로그백을 가져오다가 필요 없는 부분이 포함되었습니다 !

아하 다른거 써보시길 기대했는데 아니였구만요 ㅋㅋㅋ

hoon-space · 2023-11-26T08:36:59Z

src/test/java/kdt/jpa/board/common/support/ApiTestSupport.java

+
+import java.nio.charset.StandardCharsets;
+
+@SpringBootTest


요거 없이 테스트할 수 없을까요 ???

controller 로 단위테스트로 할 수 있었는데, 컨트롤러 테스트 하는 겸 통합테스트를 한번 수행하려고 @SpringBootTest 를 사용했습니다
현재 성공 케이스에 대해서만 통합테스트로 진행하는데 이런식으로 간단한 통합테스트만 진행할바에 단위테스트로 진행하는게 좋을까요?

EunChanNam and others added 14 commits November 21, 2023 11:32

chore: 프로젝트 세팅

596aff2

Co-authored-by: name [email protected]

Merge branch 'prgrms-be-devcourse:main' into feat

6236623

feat: 도메인, 컨트롤러, 서비스, 레포, db셋팅 구현

e40522f

feat: post, user fixture 추가 & 테스트코드 작성

7be8ff8

feat: PostControllerTest 구현

b20a1bd

feat: 셋팅 추가

4a0b783

Changes by eunchan

e7fd8f7

fix: RequestBody 매핑 버그픽스

afaec39

feat: Api 테스트 셋팅

61229e9

feat: api 테스트 셋팅

f1d8044

fix: RestDocs pathVariable 버그 픽스

46d55c3

feat: PostControllerTest update 구현

d9ee9dc

feat: 게시물 페이징 조회 명세

0a53b2d

chore: gradle 주석 추가

f991580

ParkJuhan94 added Team-Hoon 🔥 매운맛 labels Nov 22, 2023

ParkJuhan94 requested review from ordilov and hoon-space November 22, 2023 06:17

ParkJuhan94 assigned EunChanNam and ParkJuhan94 Nov 22, 2023

ParkJuhan94 added 4 commits November 23, 2023 03:45

chore: logback.xml 추가

28239ca

chore: yaml db셋팅 추가

eac9231

chore: 들여쓰기 수정

7dc1afe

chore: 로컬 db 셋팅 추가

3e5e984

ordilov reviewed Nov 25, 2023

View reviewed changes

hoon-space reviewed Nov 26, 2023

View reviewed changes

hoon-space approved these changes Dec 3, 2023

View reviewed changes

		@Column(columnDefinition = "TIMESTAMP")
		private LocalDateTime updatedDate;

		@JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY)
		public record CreatePostRequest(

		@Table(name = "users")
		public class User extends BaseEntity {


		import java.nio.charset.StandardCharsets;

		@SpringBootTest

[5기 남은찬, 박주한] Spring Boot JPA 게시판 구현 미션 제출합니다. #267

Are you sure you want to change the base?

[5기 남은찬, 박주한] Spring Boot JPA 게시판 구현 미션 제출합니다. #267

Conversation

ParkJuhan94 commented Nov 22, 2023 • edited Loading

요구 사항

SpringDataJPA 를 설정한다.

엔티티를 구성한다

API를 구현한다.

REST-DOCS를 이용해서 문서화한다.

✅ PR 포인트 & 궁금한 점

ordilov 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

ParkJuhan94 Nov 26, 2023 • edited Loading

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

hoon-space 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ParkJuhan94 commented Nov 22, 2023 •

edited

Loading

ParkJuhan94 Nov 26, 2023 •

edited

Loading