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

테스트 코드 추가 및 리팩토링 #28

Open
wants to merge 28 commits into
base: main
Choose a base branch
from

Conversation

gru3530
Copy link
Collaborator

@gru3530 gru3530 commented Feb 11, 2025

적용 항목

#24 코멘트 받은 항목을 보완 적용한다.
#25 팔로워/팔로잉 가져오기를 구현한다.
#27 단위 테스트를 위한 테스트 코드를 작성한다.

팔로워/팔로잉 가져오기 정책

API 문서
1. url 문서의 9, 10 번 api를 통해 팔로워/팔로잉을 가져온다.
2. url의 userID에 해당하는 유저를 찾을 수 없는 경우 404 에러를 반환한다.
3. 팔로워/팔로잉한 유저가 없을 경우 빈 리스트(List<Long>)를 반환한다.

추가 변경 항목

  • ResponseEntity 전달 삭제
    • Controller 내에 위치한 ResponseEntity<T> 삭제.
    • 비즈니스 로직에서 에러 발생시 Exception을 통하여 ResponseEntity 리턴
  • Repository SQL 주석 보완
  • 각 Service의 테스트 코드 작성
  • Controller내 @Valid어노테이션 적용
    • dto에서 데이터 검증 책임 삭제
    • GolbalExceptionHandler에서 @Valid관련 에러 핸들링하여 클라이언트에 전달

@gru3530 gru3530 requested a review from if-else-f February 11, 2025 12:40
@gru3530 gru3530 changed the title 테스트 코드 추가 테스트 코드 추가 및 리팩토링 Feb 11, 2025
when(followRepository.existsByFollowerIdAndFollowingId(followDto.getFollowerId(),
followDto.getFollowingId())).thenReturn(true);
//then
DuplicateException exception = assertThrows(DuplicateException.class, () -> followService.followUser(followDto));
Copy link
Collaborator

Choose a reason for hiding this comment

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

assertThatThrownBy(() -> followService.followUser(followDto))
     .using()
     .isEqualTo(new DuplicateException(ApiResponseEnum.ALREADY_FOLLOWING));

assertThat(exception.getResponseEnum()).isEqualTo(ApiResponseEnum.ALREADY_FOLLOWING);
}

@DisplayName("follow API followingID와 followID 동일한 경우 에러 반환 테스트")
Copy link
Collaborator

Choose a reason for hiding this comment

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

ollow API followingID와 followID 동일한 경우 DuplicateException 을 반환한다.


@DisplayName("특정 유저의 팔로잉들을 불러오는 API 통합테스트")
@Test
void getFollowings() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

void 팔로잉_리스트를_반환한다() {

}

@if-else-f
Copy link
Collaborator

github action 으로 테스트 코드 실행하기

assertThrows(InvalidInputException.class, () -> {
ParseUtil.parseToLong(input);
});
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Screenshot 2025-02-12 at 11 03 26 PM

@@ -20,6 +21,7 @@ public class User extends BaseEntity {
@Column(unique = true, nullable = false)
private String userName;

@Email
Copy link
Collaborator

Choose a reason for hiding this comment

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

오 이런게 있었군요 email validation이 진짜 어려운데

Copy link
Collaborator Author

@gru3530 gru3530 Feb 17, 2025

Choose a reason for hiding this comment

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

Local Part git
Domain git

레퍼런스 코드에 의하면 Hibernate-validator에서
@을 기준으로 Split을 통해 LOCAL(ID부분)과 Domain(Email)을 나누어 각각 정규식으로 valid체크하는것 같습니다.
정규식이 많이 어렵게 생겼네요...

private void checkDuplicateFollow(FollowPair followPair) {
if (hasFollow(followPair)) {
private void checkDuplicateFollow(FollowDto followDto) {
if (hasFollow(followDto)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

언제 private method로 hasFollow 를 만들고, 언제 followDto.hasFollow() 를 만들게 될까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

코멘트의 의도를 제대로 파악하지 못했습니다.

hasFollow() 메서드 네이밍이 명확하지 않다는 말씀이실까요?

followDto.hasFollow()를 만들지 않은 이유는 db를 조회하여 follow 정보가 있는지를 확인하는 메서드인데, dto 역할에 맞지 않는다 생각했습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

아 서비스에서 private method를 만드는것과, followDto 에 public method를 만드는것
어떨때 도메인에 응집력을 둘 것인지, service에 응집력을 둘 것인지 판단하는 기준이요 ㅎㅎ

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FollowDto에서 hasFollow()와 같은 public 메서드를 만들면 DB조회 작업이 추가되어 응집도🔽 결합도⬆️ 되어버려
DTO에서는 이를 고려하여 validate 관련은 도메인에 응집력을 두고, 각 역할에 해당하는 service에 응집력을 둘것 같습니다

Copy link
Collaborator

@if-else-f if-else-f left a comment

Choose a reason for hiding this comment

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

테스트 코드 많이 추가한건 너무 좋아요~
그런데 몇몇 기능에 대한 테스트코드를 더 추가하면 좋지 싶습니다

@if-else-f
Copy link
Collaborator

any updates?

@gru3530 gru3530 force-pushed the feature/enterTestCode branch from 4e4baf7 to 1eeb1c4 Compare February 25, 2025 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants