slipp 소스 코드 구현하다가 다음과 같은 상황을 만났습니다. 프로젝트를 진행하면서 일반적으로 자주 발생하는 상황이라고 봅니다.

다음 코드의 목적은 다음과 같습니다. 이메일 주소와 사용자 아이디가 중복되었는지를 api로 제공해 주는 기능입니다. 단, 로그인 사용자가 개인 정보를 수정할 수 있기 때문에 로그인 사용자와 수정하는 정보가 같은 사용자인 경우에는 중복을 허용하고 있습니다.

@Controller
@RequestMapping("/api/users")
public class ApiUsersController {
    private static Logger log = LoggerFactory.getLogger(ApiUsersController.class);


    @Resource(name = "socialUserService")
    private SocialUserService userService;


    @RequestMapping("/duplicate_userid")
    public @ResponseBody String duplicateUserId(@LoginUser(required = false) SocialUser loginUser, String userId) {
        log.debug("userId : {}", userId);


        SocialUser socialUser = userService.findByUserId(userId);
        if (socialUser == null) {
            return "false";
        }
        
        if (socialUser.isSameUser(loginUser)) {
            return "false";
        }
        
        return "true";
    }


    @RequestMapping("/duplicate_email")
    public @ResponseBody String duplicateEmail(@LoginUser(required = false) SocialUser loginUser, String email, ProviderType providerType) {
        log.debug("email : {}", email);


        SocialUser socialUser = userService.findByEmailAndProviderId(email, providerType);
        if (socialUser == null) {
            return "false";
        }
        if (socialUser.isSameUser(loginUser)) {
            return "false";
        }
        return "true";
    }
}

return 값을 true/false String 문자열로 반환하는 부분은 무시해 주세요. boolean 값도 될 듯 한데 아직 테스트하지 못했습니다. 그 부분이 중요한 부분은 아니니까요? 테스트 코드는 다음과 같습니다.

@RunWith(MockitoJUnitRunner.class)
public class ApiUsersControllerTest {
    @Mock
    private SocialUserService userService;


    @InjectMocks
    private ApiUsersController dut = new ApiUsersController();
    
    @Test
    public void checkDuplicate_doesnot_existed() {
        String actual = dut.checkDuplicate(SocialUser.GUEST_USER, null);
        assertThat(actual, is("false"));
    }


    @Test
    public void duplicateUserId_login_isSameUser() {
        String userId = "userId";
        SocialUser loginUser = new SocialUser(1L);
        when(userService.findByUserId(userId)).thenReturn(loginUser);
        
        String actual = dut.duplicateUserId(loginUser, userId);
        assertThat(actual, is("false"));
    }
    
    @Test
    public void duplicateUserId_login_isNotSameUser() {
        String userId = "userId";
        SocialUser loginUser = new SocialUser(1L);
        when(userService.findByUserId(userId)).thenReturn(loginUser);
        
        String actual = dut.duplicateUserId(new SocialUser(2L), userId);
        assertThat(actual, is("true"));
    }
    
    @Test
    public void duplicateEmail_doesnot_existed() {
        String actual = dut.duplicateEmail(SocialUser.GUEST_USER, "userId", ProviderType.slipp);
        assertThat(actual, is("false"));
    }
    
    @Test
    public void duplicateEmail_login_isSameUser() {
        String email = "email@slipp.net";
        ProviderType providerType = ProviderType.slipp;
        SocialUser loginUser = new SocialUser(1L);
        when(userService.findByEmailAndProviderId(email, providerType)).thenReturn(loginUser);
        
        String actual = dut.duplicateEmail(loginUser, email, providerType);
        assertThat(actual, is("false"));
    }
    
    @Test
    public void duplicateEmail_login_isNotSameUser() {
        String email = "email@slipp.net";
        ProviderType providerType = ProviderType.slipp;
        SocialUser loginUser = new SocialUser(1L);
        when(userService.findByEmailAndProviderId(email, providerType)).thenReturn(loginUser);
        
        String actual = dut.duplicateEmail(loginUser, email, providerType);
        assertThat(actual, is("false"));
    }
}

단위 테스트 코드가 무지 복잡합니다. 데이터베이스 연동 부분도 있어서 Mockito까지 활용하고 있어요. 그런데 ApiUsersController를 보면 중복 부분이 있습니다. 이 중복을 제거하기 위해 다음과 같이 extract method 패턴을 활용해 리팩토링했습니다.

@Controller
@RequestMapping("/api/users")
public class ApiUsersController {
    private static Logger log = LoggerFactory.getLogger(ApiUsersController.class);


    @Resource(name = "socialUserService")
    private SocialUserService userService;


    @RequestMapping("/duplicateUserId")
    public @ResponseBody
    String duplicateUserId(@LoginUser(required = false) SocialUser loginUser, String userId) {
        log.debug("userId : {}", userId);


        SocialUser socialUser = userService.findByUserId(userId);
        return checkDuplicate(loginUser, socialUser);
    }


    @RequestMapping("/duplicateEmail")
    public @ResponseBody
    String duplicateEmail(@LoginUser(required = false) SocialUser loginUser, String email, ProviderType providerType) {
        log.debug("email : {}, providerType : {}", email, providerType);


        SocialUser socialUser = userService.findByEmailAndProviderId(email, providerType);
        return checkDuplicate(loginUser, socialUser);
    }


    private String checkDuplicate(SocialUser loginUser, SocialUser socialUser) {
        if (socialUser == null) {
            return "false";
        }
        if (socialUser.isSameUser(loginUser)) {
            return "false";
        }
        return "true";
    }
}

위와 같이 리팩토링을 한 후 단위 테스트 코드를 보니 단위 테스트 또한 중복된 코드라는 생각이 들더군요. 그래서 checkDuplicate() 메소드의 접근 제어를 private에서 default로 변경한 후 다음과 같이 단위 테스트 코드를 구현했습니다.

public class ApiUsersControllerTest {
    private ApiUsersController dut;
    
    @Before 
    public void setup() {
        dut = new ApiUsersController();
    }
    
    @Test
    public void checkDuplicate_doesnot_existed() {
        String actual = dut.checkDuplicate(SocialUser.GUEST_USER, null);
        assertThat(actual, is("false"));
    }


    @Test
    public void checkDuplicate_login_isSameUser() {
        SocialUser loginUser = new SocialUser(1L);
        SocialUser existedUser = loginUser;
        String actual = dut.checkDuplicate(loginUser, existedUser);
        assertThat(actual, is("false"));
    }
    
    @Test
    public void checkDuplicate_login_isNotSameUser() {
        SocialUser loginUser = new SocialUser(1L);
        SocialUser existedUser = new SocialUser(2L);
        String actual = dut.checkDuplicate(loginUser, existedUser);
        assertThat(actual, is("true"));
    }
}

이와 같이 checkDuplicate() 메소드에 대한 테스트를 진행했더니 굳이 Mockito를 쓸 필요도 없고 단위 테스트에 대한 중복도 제거 되었다는 생각이 듭니다.

그런데 위와 같이 단위 테스트를 진행하고 보니 ApiUsersController 코드의 일부가 단위 테스트가 되지 않은 상태로 남아 있습니다. 그렇다고 이 부분을 모두 테스트한다는 것은 중복과 낭비 요소가 발생할 것으로 판단되고요.

위와 같이 리팩토링을 하고 단위 테스트를 수정하면서 checkDuplicate() 메소드의 접근 제어자를 private에서 default로 변경해야 했습니다. 이 부분도 찜찜한 부분이 있죠. 그렇다면 checkDuplicate() 메소드는 ApiUsersController에 있는 것이 적합하지 않을 수도 있겠다는 느낌이 드네요. 이미 SocialUser는 충분히 커질만큼 커진 상태인데요. 어디로 들어가면 좋을까요? 별도의 Helper 클래스를 만드는 것을 고민해 볼 수 있을까요? 일반적으로 위와 같은 경우 어떤 방식으로 접근하는지요?