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

[5기 최정은, 권효승] Spring Boot JPA 게시판 구현 미션 제출합니다. #275

Closed
wants to merge 51 commits into from

Conversation

JeongeunChoi
Copy link

@JeongeunChoi JeongeunChoi commented Nov 24, 2023

📌 과제 설명

Spring Boot JPA을 활용한 게시판 구현

👩‍💻 요구 사항과 구현 내용

SpringDataJPA 를 설정한다.

  • datasource : mysql

엔티티를 구성한다

  • 회원(User)
    • id (PK) (auto increment)
    • name
    • age
    • hobby
    • created_at(생성시간) DATETIME
    • updated_at(수정시간)
    • deleted_at(삭제시간)
  • 게시글(Post)
    • id (PK) (auto increment)
    • title
    • content
    • user_id
    • 게시글 종류
    • created_at(생성 시간)
    • updated_at(수정 시간)
    • deleted_at(삭제시간)
  • 회원과 게시글에 대한 연관관계 설정
  • 게시글 Repository를 구현

API를 구현한다.

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

REST-DOCS를 이용한 문서화

  • REST-DOCS를 이용한 문서화

✅ PR 포인트 & 궁금한 점

궁금한점

  • id 생성 전략 (table, sequence, identity) 중 어떤 상황에서 어떤 생성 전략을 쓰는지 궁금합니다!
  • id 값에는 보통 박싱된 타입인 Long을 사용하는데 원시타입이 아닌 Long을 사용하는 이유는 무엇인가요?
  • RestDocs Page 객체에 많은 값을 가지고 있는데, 클라이언트에게 모든 데이터를 반환하나요?(아니라면 어느 데이터를 반환하는지)
  • 컨트롤러 테스트는 보통 단위/통합 테스트 중 어떤 방식을 사용하시고 어떤 것을 검증하는 편인가요?

✅ 피드백 반영사항

  • updatedAt 변경 주체 애플리케이션 레벨로 변경
    Auditing 제거 및 setUpdatedAt 메서드 추가
  • GlobalExceptionHandler에서 구체적인 에러 반환하지 않도록 변경
  • 커서 기반 조회 hasNext 값을 통해 다음 데이터 유무에 대한 정보 추가
  • 리스트 조회(Pageable + Cursor) 시 필요한 정보만 반환하도록 변경
  • 리스트 조회(Pageable + Cursor) 시 필요한 데이터만 요청 받도록 변경
  • SuccessMessage 클래스 타입 안정성을 위해 Object -> Generic 클래스로 변경

궁금한 점

  1. PATCH가 아닌 PUT을 통해 전체 객체를 보내 수정을 하게 되면 nullable 체크를 하지 않아도 된다고 하셨는데, 결국 전체 객체를 받으면 더 많은 null 체크를 해야할 것 같은데 무슨 뜻인지 궁금해졌습니다!
  2. 커서 기반 조회 시 클라이언트에서 데이터의 마지막 id 값을 보내주어 조회할 수 있을 것 같은데, nextId를 서버에서 직접 생성하여 반환해주는 경우는 정확히 어떤 케이스인지 궁금합니다!
  3. 의도치 않은 예외 발생 시 Exception 메시지 처리는 실무에서 어떻게 처리하는지 궁금합니다!
  4. BaseEntity에 setUpdatedAt으로 시간을 수정하는 걸로 변경했는데, set 메서드가 외부에 노출되어도 괜찮을까요?(최선의 방법인지 궁금합니다.)

hyoguoo and others added 30 commits November 22, 2023 23:28
- 게시글 전체 조회
- 특정 게시글 조회
- 게시글 생성
- 게시글 수정

Co-Authored-By: hyoguoo <[email protected]>
Co-authored-by: JeongeunChoi <[email protected]>
hyoguoo and others added 16 commits November 26, 2023 15:00
Co-authored-by: JeongeunChoi <[email protected]>
Co-authored-by: JeongeunChoi <[email protected]>
Co-authored-by: JeongeunChoi <[email protected]>
Co-authored-by: JeongeunChoi <[email protected]>
Copy link

@SangHyunGil SangHyunGil left a comment

Choose a reason for hiding this comment

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

반영해주시느라 너무 고생많으셨어요 👍
관련해서 코멘트 몇 개 남겨두었고 확인 후 머지해주시면 될 것 같습니다 👍

PATCH가 아닌 PUT을 통해 전체 객체를 보내 수정을 하게 되면 nullable 체크를 하지 않아도 된다고 하셨는데, 결국 전체 객체를 받으면 더 많은 null 체크를 해야할 것 같은데 무슨 뜻인지 궁금해졌습니다!

PUT의 경우 클라이언트에서 수정할 객체의 모든 필드를 보내고 이를 전체 수정한다는 입장에서 별도 체크가 필요없다고 말씀드렸어요.
물론 클라이언트를 신뢰하지 못한다면 nullable한 필드가 있을지를 고려해야하지만 클라이언트를 신뢰한다는 입장에서 말씀드렸어요.

만약 PATCH API를 제공할 경우 여러가지 업데이트가 존재할 수 있을 것 같아요.
타이틀, 콘텐트 가 있다고 했을때 사용자는 타이틀만 업데이트하고싶을 수도 있고 콘텐트만 업데이트하고 싶을 수도 있을텐데 부분 업데이트다보니 수정이 되어야하는 필드만 골라 수정해야해서 nullable을 체크해야된다고 말씀드렸어요.

커서 기반 조회 시 클라이언트에서 데이터의 마지막 id 값을 보내주어 조회할 수 있을 것 같은데, nextId를 서버에서 직접 생성하여 반환해주는 경우는 정확히 어떤 케이스인지 궁금합니다!

예를 들어 아래와 같은 경우에요.
쇼핑 내역을 보여주는데 통합 내역에는 의류 정보 / 가전 정보 를 보여주고 있어요.
각각은 별도 서버에서 관리되고 있어서 이를 통합해서 내려주는 니즈가 있다고 했을때 커서값을 정해야하는데 특정 키 값을 기준으로 내려줄때 의류 정보의 id 값과 가전 정보의 id 값을 혼합해서 내려줘야하는 경우가 생길 수 있을 것 같아요.

의도치 않은 예외 발생 시 Exception 메시지 처리는 실무에서 어떻게 처리하는지 궁금합니다!

알 수 없는 에러라는 클 틀로 잡아서 내려주고 있어요.
Exception이라는 상위 클래스로 잡아 구체 커스텀 예외로 잡히지 않은 케이스에 대해 위처럼 표현해요.

BaseEntity에 setUpdatedAt으로 시간을 수정하는 걸로 변경했는데, set 메서드가 외부에 노출되어도 괜찮을까요?(최선의 방법인지 궁금합니다.)

저는 괜찮다고 생각해요.
set이 항상 문제가 된다고 생각하지는 않고 적절한 시점에 사용할 수 있다면 좋을 것 같아요.
다만, 접근제어자를 적절히 사용해보면 좋을 것 같아요.

Comment on lines +29 to +31
public boolean isSameName(String name) {
return this.getName().equals(name);
}

Choose a reason for hiding this comment

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

👍

@@ -5,12 +5,12 @@

@Getter
@JsonInclude(JsonInclude.Include.NON_NULL)
public class SuccessMessage {
public class SuccessMessage<T> {

Choose a reason for hiding this comment

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

👍

@@ -29,7 +29,7 @@ public ResponseEntity<ExceptionMessage> handleUserException(UserException e) {
@ExceptionHandler(Exception.class)
public ResponseEntity<ExceptionMessage> handleException(Exception e) {
log.error(e.getMessage());
ExceptionMessage exceptionMessage = new ExceptionMessage(e.getClass().getSimpleName(), e.getMessage());
ExceptionMessage exceptionMessage = new ExceptionMessage("Internal Server Error", "서버 내부 에러가 발생했습니다.");

Choose a reason for hiding this comment

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

나중에는 error 값을 별도로 [클라이언트 - 서버] 간 정의하여 사용해보는걸 추천해요 👍
그 이유는 클라이언트에서 별도 정의된 코드 기반으로 핸들링할 수 있는 장점이 있을 것 같아요 👍

}

@Transactional(readOnly = true)
public Slice<PostResponse> getAllPostsWithUserByCursor(Pageable pageable, Long cursorId) {
return postRepository.findAllWithUserByCursor(cursorId, pageable)
public Slice<PostResponse> getAllPostsWithUserByCursor(int size, Long cursorId) {

Choose a reason for hiding this comment

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

Slice를 사용하지 않고도 나중에 해보시면 좋을 것 같네요 👍

import java.util.List;

@Getter
public class PostsResponse {

Choose a reason for hiding this comment

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

페이징처리해서 내려주는건 어느정도 고정된 형식이 존재할 것 같아요.
Data만 변경되고 totalPages, totalElements는 동일하게 존재하구요.
그래서 나중에 공통화하는 방법을 고려해보면 좋을 것 같아요. (ex) 제네릭 활용)

Comment on lines +10 to +13
public class PostsCursorResponse {

private final List<PostResponse> content;
private final boolean hasNext;

Choose a reason for hiding this comment

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

커서도 공통화할 수 있을지 고민해보면 좋을 것 같네요 👍
그리고 슬라이스를 활용하는 방법이 괜찮을지 고민해봐주세요.

  • 슬라이스는 이전에 말씀드린대로 오프셋 기반으로 가져오지만 카운트 쿼리를 별도로 날리지 않은 형태에요.
  • 실제 구현은 jpql로 커서 방식으로 하지만 slice 활용으로 인해 혼란을 줄 수 있을 것 같아요.

Copy link
Author

Choose a reason for hiding this comment

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

"실제 구현은 jpql 커서 방식으로 진행하지만 반환값이 Slice라는 점이 혼란을 줄 수 있을 것 같다." 라고 이해했는데, 여기서는 Slice의 hasNext 값을 가져오기 위해 사용해보았는데, 보통 Slice 객체를 반환하지 않는 편인가요?

Choose a reason for hiding this comment

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

네 Slice 객체를 반환하지 않는 편이에요.
nextCursor와 데이터를 내려주는 편입니다.

build.gradle Outdated
Comment on lines 48 to 60
tasks.named('asciidoctor') {
inputs.dir snippetsDir
dependsOn test
}

asciidoctor {
dependsOn test
inputs.dir snippetsDir
}

asciidoctor.doFirst {
delete file('src/main/resources/static/docs')
}

Choose a reason for hiding this comment

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

rest docs 관련 설정을 설명해주실 수 있나요 😄

Copy link
Author

@JeongeunChoi JeongeunChoi Dec 12, 2023

Choose a reason for hiding this comment

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

asciidoctor 태스크를 설정하여
snippetsDir에 스니펫들을 저장하고,
test에 의존하도록하여 해당 태스크 실행 전 test가 먼저 실행되도록 하였는데

엇 처음 두개 설정이 동일한 작업이군요..! 수정해두겠습니다 ㅎㅎ

세번째 설정은 asciidoctor 태스크 실행 전에 해당 경로에 있는 파일 삭제하는 작업으로
이전에 생성된 문서를 제거하고 새로운 문서를 생성하기 위해 사용하였습니다.
해당 설정은 파일명 변경 시, 파일이 누적되는 것을 방지하기 위할 수 있으나 이번 프로젝트에서는 불필요한 것 같네요. 수정하도록 하겠습니다!

gradle 설정을 별로 신경쓰지 않았는데
언급해주셔서 다시 보니 불필요한 설정들도 보이네요.. 감사합니다~🙇‍♀️

spring:
datasource:
driver-class-name: com.mysql.cj.jdbc.Driver
url: jdbc:mysql://localhost:3306/default_db?createDatabaseIfNotExist=true&useUnicode=true&characterEncoding=utf8&useSSL=true

Choose a reason for hiding this comment

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

나중에는 테스트용 별도 embedded db를 활용하는 방법을 고민해봐주세요.
현재는 테스트에서도 디폴트로 프로덕션의 yml을 바라보기에 테스트 코드가 운영에 영향을 줄 수 있을 것 같아요.


public class PostException extends RuntimeException {

private final HttpStatus httpStatus;

Choose a reason for hiding this comment

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

PostException에서 PostErrorMessage 자체를 들고있는 방향으로 고민해봐도 좋을 것 같아요.
이미 PostErrorMessage에서 자세한 정보를 들고 있다보니 충분할 것 같다는 생각이 들어요.

Comment on lines 20 to 21
@Column(name = "age")
private Long age;

Choose a reason for hiding this comment

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

나중에 어플리케이션에서 나이에 대한 검증을 진행하고 다양한 값들에 대한 검증을 진행할때 유저에서 모든 책임을 가지게될 수 있을 것 같아요.
값 객체를 활용하는 방법에 대해서도 고민해보면 좋을 것 같아요.

@hyoguoo hyoguoo closed this Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants