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

[4기 정성현] Spring Boot JPA 게시판 구현 미션 제출합니다. #257

Open
wants to merge 10 commits into
base: kimihiqq
Choose a base branch
from

Conversation

kimihiqq
Copy link
Member

@kimihiqq kimihiqq commented Aug 6, 2023

📌 과제 설명

Spring Boot와 Spring Data JPA를 이용해 게시판 구현 미션을 진행하였습니다. 이후 REST-DOCS를 적용해 문서화 하였습니다!

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

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를 이용해서 문서화한다.

  • REST Docs 적용

✅ PR 포인트 & 궁금한 점

리뷰 감사합니다! 수정이 필요하거나 새롭게 추가해야할 부분이 있다면 코멘트 부탁드립니다!!


private void validateTitle(String title) {
if (title == null || title.isBlank()) {
throw new PostException(ErrorCode.INVALID_POST_INPUT, String.format("Invalid title: %s", title));
Copy link
Member

Choose a reason for hiding this comment

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

예외처리할 때 구체적으로 어떤 부분에서 예외가 발생했는지 데이터를 표시해주는 부분이 좋네요!

public PostResponse getPost(Long id) {
Post post = postRepository.findById(id)
.orElseThrow(() -> new IllegalArgumentException("해당 게시글이 없습니다. id=" + id));
return new PostResponse(post);
Copy link
Member

Choose a reason for hiding this comment

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

고수준인 도메인에 저수준인 dto가 의존하기 때문에 의존에도 문제 없고 코드도 깔끔해지네요! 저도 이 방법을 채택해야겠네요~

private Long userId;

public Post toEntity(User user) {
System.out.println("toentity" );
Copy link
Member

Choose a reason for hiding this comment

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

이건 삭제하셔도 될거 같습니다~

Copy link
Member Author

Choose a reason for hiding this comment

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

깜빡 잊고 있었습니다ㅠ 바로 수정하겠습니다!!

Copy link
Member

@Dev-Yesung Dev-Yesung left a comment

Choose a reason for hiding this comment

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

안녕하세요 성현님! JPA게시판 미션 고생하셨습니다!
제 수준에서 크게 바꿔야할 부분을 찾지 못했습니다...!
그만큼 깔끔하게 구현하신게 아닐까요 ㅎㅎ
할만한 부분이 없어서 좋았던 부분만 체크했습니다.
나머지 리뷰는 멘토님들께 맡기겠습니다!
데브코스 계속 화이팅입니다🙇‍♂️

@lleellee0
Copy link

@kimihiqq

심플하게 기본적인 기능들 잘 작성하셨는데요?
수고하셨습니다.

테스트 코드만 좀 추가해보시면 좋을 것 같아요!

  • 서비스를 통해 레포지토리까지 함께 테스트 할 수 있는 테스트 코드
  • 레포지토리는 모킹한 상태에서 서비스만 테스트 할 수 있는 테스트 코드

이 2가지 작성해보시기 바랍니다~

Comment on lines +23 to +26
@GetMapping
public ResponseEntity<Page<PostResponse>> getPosts(@PageableDefault(sort = "id", size = 10, direction = Sort.Direction.DESC) Pageable pageable) {
Page<PostResponse> responseDto = postService.getPosts(pageable);
return ResponseEntity.ok().body(responseDto);

Choose a reason for hiding this comment

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

아마 ok 안에 postResponseDto를 넣어주셔도 될겁니다

Comment on lines +35 to +36
@OneToMany(mappedBy = "user", cascade = CascadeType.ALL, orphanRemoval = true)
private List<Post> posts = new ArrayList<>();

Choose a reason for hiding this comment

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

포스트가 유저에 전적으로 의존적이라면 해당 코드가 유의미할 수 있는데 아니라면 조금 위험해보이네요
유저가 삭제되면 유저가 작성한 모든 포스트도 같이 삭제될것 같은데.. 상황에 따라 조금 고민이 필요해보이지만.. 지금은 괜찮을것 같네요

그리고 해당 양방향 맵핑을 활용해 로직에 녹여내는 코드가 없는것 같은데요 불필요한 맵핑은 지워주는게 좋습니다.

Comment on lines +32 to +36
@AutoConfigureMockMvc
@AutoConfigureRestDocs
@SpringBootTest
@Transactional
class PostApiControllerTest {

Choose a reason for hiding this comment

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

현재 레스트 도큐먼트 구성 목적으로만 테스트 코드가 존재해서
@AutoConfigureRestDocs
@WebMvcTest(PostController.class)
위 코드만으로 될것 같아요(아마도?)

추가로, 실제 응답을 검증하는 테스트는 없는것 같은데요 이번에 한번 작성해보시면 좋을것 같네요

Comment on lines +17 to +24
@ExceptionHandler(PostException.class)
@ResponseStatus(HttpStatus.BAD_REQUEST)
public ResponseEntity<ErrorResponse> handlePostException(PostException e) {
log.error("PostException Occurred: {}", e.getMessage(), e);

ErrorResponse errorResponse = new ErrorResponse(e.getErrorCode().getMessage(), e.getDebugMessage());
return new ResponseEntity<>(errorResponse, HttpStatus.BAD_REQUEST);
}

Choose a reason for hiding this comment

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

DebugMessage에 ErrorCode의 메시지를 기본 적으로 입력해주면 ErrorCode message는 제외시켜줘도 될것 같은데요
차라리 ErrorCode 자체를 넣거나(USER_ALREADY_EXISTS,,) 직접 정의한 코드(Http Status 코드 아님)를 넣어주면
클라이언트에서 예외 응답을 받았을때(400 에러 등) 분기처리를 할때 활용해 볼수 있게 될 것 같습니다

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.

4 participants