Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public String index() {
@GetMapping("/home")
public String home(Model model, @SessionAttribute("userId") Long userId) {

User user = userService.getUserByUserId(userId);
User user = userService.getUser(userId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
User user = userService.getUser(userId);
User user = userService.findById(userId);

service의 메서드명을 repository에 맞추자는 예시는 위같은 형식을 의미했습니다. getUser는 어떤 파라미터를 바탕으로 조회하는 지 의미를 표현함에 있어 메서드명이 모호하다고 생각합니다.

GithubRepoPageResponse response = githubRepoService.getGithubRepoInfos(userId, 0);

model.addAttribute("repositories", response); // 레포지토리 리스트
Expand All @@ -57,7 +57,7 @@ public String loginFailure() {
@GetMapping("/mypage")
public String myPage(Model model, @SessionAttribute("userId") Long userId) {

User user = userService.getUserByUserId(userId);
User user = userService.getUser(userId);
model.addAttribute("avatarUrl", user.getAvatarUrl());
model.addAttribute("username", user.getUsername());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class UserService {
private final RedisTemplate<String, String> redisTemplate;
private final GithubOAuth2WebClient githubOAuth2WebClient;

public User getUserByUserId(Long userId) {
public User getUser(Long userId) {

return userRepository.findById(userId)
.orElseThrow(() -> new GeneralException(UserException.USER_NOT_FOUND));
Expand Down
131 changes: 131 additions & 0 deletions src/test/java/com/specialwarriors/conal/UserServiceTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package com.specialwarriors.conal;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;

import com.specialwarriors.conal.common.auth.oauth.GithubOAuth2WebClient;
import com.specialwarriors.conal.common.exception.GeneralException;
import com.specialwarriors.conal.user.domain.User;
import com.specialwarriors.conal.user.exception.UserException;
import com.specialwarriors.conal.user.repository.UserRepository;
import com.specialwarriors.conal.user.service.UserService;
import java.util.Optional;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.data.redis.core.RedisTemplate;
import org.springframework.data.redis.core.ValueOperations;

@ExtendWith(MockitoExtension.class)
public class UserServiceTest {

@Mock
private UserRepository userRepository;

@Mock
private RedisTemplate<String, String> redisTemplate;

@Mock
private GithubOAuth2WebClient githubOAuth2WebClient;

@InjectMocks
private UserService userService;

private User mockUser;

@BeforeEach
void setUp() {
mockUser = new User(1, "홍길동", "fdsf");
}

@Nested
@DisplayName("유저 아이디로 사용자를 조회할 때")
class FindUserById {

@Test
@DisplayName("성공한다")
void success() {

// given
long userId = 1L;
given(userRepository.findById(userId)).willReturn(Optional.of(mockUser));

// when
User result = userService.getUser(userId);

// then
assertThat(result).isEqualTo(mockUser);
verify(userRepository).findById(userId);
}

@Test
@DisplayName("시용자가 존재하지 않으면 예외를 던진다")
void throwsExceptionWhenUserNotFoundById() {

// given
long userId = 1L;
given(userRepository.findById(userId)).willReturn(Optional.empty());

// when, then
assertThatThrownBy(() -> userService.getUser(userId))
.isInstanceOf(GeneralException.class)
.extracting("exception")
.isEqualTo(UserException.USER_NOT_FOUND);
}
}

@Nested
@DisplayName("사용자를 삭제할 때")
class DeleteUser {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이너 테스트 클래스의 경우 DeleteUserTest와 같이 Test postfix를 붙여 테스트라는 것을 명확히 나타내는 것은 어떨까요?


@Test
@DisplayName("깃허브 토큰과 세션을 삭제한다")
void deletesGithubTokenAndSessionWhenUserIsDeleted() {

// given
long userId = 1L;
String githubTokenKey = "github:token:1";
String githubTokenValue = "fake-access-token";

given(userRepository.findById(userId)).willReturn(Optional.of(mockUser));

ValueOperations<String, String> ops = mock(ValueOperations.class);
given(redisTemplate.opsForValue()).willReturn(ops);
given(ops.get(githubTokenKey)).willReturn(githubTokenValue);

// when
userService.deleteUser(userId);

// then
verify(githubOAuth2WebClient).unlink(githubTokenValue);
verify(redisTemplate).unlink(githubTokenKey);
verify(userRepository).deleteById(userId);
}

@Test
@DisplayName("사용자가 존재하지 않으면 예외를 던진다")
void throwsExceptionWhenDeletingNonexistentUser() {

// given
long userId = 1L;
given(userRepository.findById(userId)).willReturn(Optional.empty());

// when, then
assertThatThrownBy(() -> userService.deleteUser(userId))
.isInstanceOf(GeneralException.class)
.extracting("exception")
.isEqualTo(UserException.USER_NOT_FOUND);

verify(userRepository).findById(userId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

해피 케이스에서 이미 확인한 로직의 동작을 예외 케이스에서 부가적으로 확인할 필요가 있을 까요? 검토 부탁드립니다.

}
}

}