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

기본 댓글기능 추가 #14

Merged
merged 38 commits into from
Jan 11, 2025
Merged

기본 댓글기능 추가 #14

merged 38 commits into from
Jan 11, 2025

Conversation

gru3530
Copy link
Collaborator

@gru3530 gru3530 commented Jan 6, 2025

#7 댓글달기 기능 구현

댓글 정책

  1. 댓글의 내용은 비어있을 수 없다
  2. 댓글의 depth는 Level 2 까지만 허용한다.
    • 댓글 (Level 1) : 허용
    • 댓글의 댓글 (Level 2) : 허용
    • 댓글의 댓글의 댓글 (Level 3) : 비허용
  3. 댓글의 내용은 중복될 수 있다.

추가 변경 항목

  • CustomException 네이밍, 위치 변경

    • 위치 변경
      기존 : stargram.domain.user.exception
      변경 : stargram.domain.common.exception
    • 네이밍 변경
      • EmptyInputException -> InvalidInputException
  • Service 네이밍, 위치 변경

    • 위치 변경
      기존 : stargram.domain.user.service
      변경 : stargram.domain.common.service
    • 네이밍 변경
      • commonService -> userQueryService
  • 요청을 ResponseEntity를 활용하여 클라이언트에 데이터 전달 추가

테스트 JSON

{
    "userId": 1,
    "comment" : "Test Nest Comment",
    "parentCommentId": 1
}

요청 성공 시

{
    "code": "SUCCESS",
    "data": {
        "commentId": 4,
        "userId": 1,
        "postId": 6,
        "comment": "Test Nest Comment",
        "parentCommentId": 1,
        "createdAt": "2025-01-07T02:21:50.9621972"
    }
}

요청 실패 시 (댓글의 depth Level 3이상의 댓글인 경우)

{
    "code": "NESTED_COMMENT",
    "message": "대댓글을 작성할 수 없습니다"
}

gru added 30 commits December 27, 2024 16:56
/users 공통되어 @RequestMapping으로 통합
commonService에서 처리
명시적으로 표현하기위해 네이밍 변경
ApiRepsonse -> ApiResponseEnum
ApiResponseDto를 공통 결과 클래스 ApiResult로 변경
실패할 경우
{
    "code": "EMPTY_EMAIL",
    "message": "이메일이 비어있습니다",
    "data": null
}

성공할 경우

{
    "code": "SUCCESS",
    "message": "요청에 성공하였습니다",
}
JsonInclude 추가.
NON_NULL로 설정한 경우 빈 값도 들어갈 수 있기떄문에 NON_EMPTY로 설정
commonService -> CommonService
- UserGlobalException -> GlobalApiException 으로 변경
- 패키지 지정 구문 삭제
기존 : stargram.domain.user.exception
변경 : stargram.domain.common.exception
gru added 8 commits January 6, 2025 09:42
개체 역할에 맞도록 네이밍 변경
CommonService -> UserQueryService
- ApiResponseEnum
- ApiResult
- BaseDto

위 세개 클래스 공통으로 사용하여 common패키지로 위치 이동
다른 항목에서도 동일하게 사용할 수 있도록 포괄적인 네이밍으로 변경
- Comment Entity의 parentCommentId이 없으면 최상위 댓글
- 대대댓글 이하 Level의 댓글은 입력 불가
JPA사용하는 클래스를 CommentQueryService로 역할 분리
- parentCommentID가 없는 경우 최상위 댓글 (Level 1)
- parentCommentID가 있는 경우 대댓글 (Level 2)
- 상위 댓글의 parentCommentID가 있는경우 대대댓글(Level 3)

[정책]
댓글은 Level 2까지만 허용한다
@gru3530 gru3530 requested a review from if-else-f January 6, 2025 17:35

}


Copy link
Collaborator

Choose a reason for hiding this comment

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

빈칸은 제거해주세요
java는 마지막 1줄을 EOF로 남겨둡니다 컨벤션

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

컨벤션에 맞도록 빈칸 제거하였습니다


private final CommentService commentService;

@Transactional
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Transaction 을 왜 controller 까지 확장하셨나요?
서비스 로직이기 때문에 CommentService 로 가야하지 않을까요?

Copy link
Collaborator Author

@gru3530 gru3530 Jan 10, 2025

Choose a reason for hiding this comment

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

#19 를 기준으로
@Transational 을 서비스가 하나의 atomic이 되도록 queryService를 삭제하고 기존 CommentService로 통합하여 학습예정입니다


@Transactional
@PostMapping("/{postId}/comment")
public ResponseEntity<ApiResult> createComment(@RequestBody CommentRequestDto dto, @PathVariable Long postId) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

postId 에 String 이 들어오면 에러는 어떻게 반환되나요?

Copy link
Collaborator Author

@gru3530 gru3530 Jan 7, 2025

Choose a reason for hiding this comment

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

기존에 의도했던 ApiResponseEnum이 아닌 MethodArgumentTypeMismatchException이 발생하여 에러가 리턴됩니다.

Exception 패키지에만 추가하여 핸들링 할 수도 있으나
명확하게 에러 검출하기 위해 데이터 수신부에서 postId를 String으로 받아 Long으로 파싱하여
데이터 정합성 판단하도록 로직 구성하겠습니다


private final PostService postService;

@Transactional
Copy link
Collaborator

Choose a reason for hiding this comment

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

Controller 에선 여러 service가 호출될 수 있습니다.
Transactional 은 서비스로 넣어 하나의 서비스가 하나의 atomic이 되도록 스코프를 결정해주세요

Copy link
Collaborator Author

@gru3530 gru3530 Jan 10, 2025

Choose a reason for hiding this comment

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

#19 를 기준으로
Transational 을 서비스가 하나의 atomic이 되도록 queryService를 삭제하고 통합하여 학습예정입니다

import lombok.Getter;
import lombok.NoArgsConstructor;

@EntityListeners(AuditingEntityListener.class)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@EntityListeners 가 여러번 반복되는데
createdAt, updatedAt 을 하나로 묶어서 상속받는건 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

BaseEntity 클래스를 만들어 선언해두고 상속받아 재사용 및 중복코드를 방지하도록 구성하겠습니다

private long userId;

@Column(nullable = false)
private String content;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Content는 table에서 어떠한 column으로 선언하셨나요?

return userRepository.findById(userId).isPresent();
}

public boolean existByUserId(Long userId) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 쿼리와 public 함수의 이름이 같을 필요가 있을까요?
service 안에서 jpa repository를 호출하도록 만드는게 일을 줄일 수 있을것 같아보이는데요 어떻게 생각하시나요?

그리고 boolean이 반환타입이면, isXXX 혹은 hasXXX 와 같이 네이밍을 하면 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

service안에서 호출하도록 QueryService를 삭제하고 통합하였습니다.

boolean 반환타입을 isXXX or hasXXX로 변경하였습니다

#19

@gru3530 gru3530 merged commit a42986c into main Jan 11, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants