저작권 안내: 저작권자표시 Yes 상업적이용 No 컨텐츠변경 No

스프링5 입문

JSP 2.3

JPA 입문

DDD Start

인프런 객체 지향 입문 강의

기존 코드의 중복 코드: 흐름 제어 중복


최근에 정리한 코드는 다음과 같은 형태를 가졌다.


private ErrorLogger errorLogger;


public Response chooseMeeting(Request req) {

    String apiId = "E0004";

    String code = null;

    String jupno = null;

    String transt = null;

    try {

        jupno = request.getJupno();

        changeTranst(jupno, transt = "0", req.getId(), req.getIp()); // 전송전 상태

        Result result = tranService.chooseMeeting(req); // 전송 처리

        code = eocsResult.getCode();

        changeTranst(jupno, transt = "1", req.getId(), req.getIp()); // 전송완료 상태

    }catch (EocsException ex) {

        transt(jupno, transt = "2", req.getId(), request.getIp()); // 전송오류 상태

        code = ex.getEocsErrorCode();

        errorLogger.append(apiId, code, jupno, transt, req, req.getId(), req.getIp(),....);

    }catch (Exception e) {

        code = "C00003";

        errorLogger.append(apiId, code, jupno, transt, req, req.getId(), req.getIp()....);

    }

    return new Response(apiId , code);

}


private changeTranst(String jupno, String transt, String id, String ip) {

    ...

}


chooseMeeting() 메서드는 다음 흐름을 갖는다.

  1. 전송전 상태 변경: changeTranst()
  2. 실제 전송 업무 처리: tranService.chooseMeeting()
  3. 전송완료 상태 변경: changeTranst()
    1. 전송오류 발생시 전송오류 상태 변경 후 로그 기록
    2. 이 외 익셉션 로그 기록
  4. 결과 리턴

chooseMeeting() 메서드만 이런 코드를 가진 게 아니다. 이와 동일한 흐름을 갖는 메서드가 3개가 더 있었다. 차이점은 apiId와 전송 처리 코드뿐이었다. 예를 들어, 다른 코드는 아래와 같다.


public Response choosePip(PipRequest req) {

    String apiId = "E0003";

    String code = null;

    String jupno = null;

    String transt = null;

    try {

        jupno = request.getJupno();

        changeTranst(jupno, transt = "0", req.getId(), req.getIp()); // 전송전 상태

        Result result = tranService.choosePip(req); // 전송 처리

        code = eocsResult.getCode();

        changeTranst(jupno, transt = "1", req.getId(), req.getIp()); // 전송완료 상태

    }catch (EocsException ex) {

        transt(jupno, transt = "2", req.getId(), request.getIp()); // 전송오류 상태

        code = ex.getEocsErrorCode();

        errorLogger.append(apiId, code, jupno, transt, req, req.getId(), req.getIp(),....);

    }catch (Exception e) {

        code = "C00003";

        errorLogger.append(apiId, code, jupno, transt, req, req.getId(), req.getIp()....);

    }

    return new Response(apiId , code);

}



메서드 추출로 하려다가...


처음엔 대충 다음과 같은 모양을 상상하면서 메서도 추출로 가려했다.


public Response chooseMeeting(Request req) {

    String apiId = "E0004";

    return runTrans(apiId, req.getJupno(), req.getId(), req.getIp(), 

                         () -> tranService.chooseMeeting(req));

}


public Response choosePip(PipRequest req) {

    String apiId = "E0003";

    return runTrans(apiId, req.getJupno(), req.getId(), req.getIp(), 

                         () -> tranService.choosePip(req));

}


private Response runTrans(String apiId, 

                          String jupno, String id, String ip, // 상태처리나 로그 기록에 필요

                          Supplier<Result> transition) {

    String code = null;

    String transt = null;

    try {

        jupno = request.getJupno();

        changeTranst(jupno, transt = "0", id, ip); // 전송전 상태

        Result result = transition.get(); // 전송 처리

        code = eocsResult.getCode();

        changeTranst(jupno, transt = "1", id, ip); // 전송완료 상태

    }catch (EocsException ex) {

        transt(jupno, transt = "2", id, ip); // 전송오류 상태

        code = ex.getEocsErrorCode();

        errorLogger.append(apiId, code, jupno, transt, id, ip,....);

    }catch (Exception e) {

        code = "C00003";

        errorLogger.append(apiId, code, jupno, transt, id, ip,....);

    }

    return new Response(apiId , code);

}


private changeTranst(String jupno, String transt, String id, String ip) {

    ...

}


runTrans() 메서드는 로그를 남기거나 상태 변경을 처리하는 코드에서 필요한 값을 파라미터로 받는데, 파라미터가 다소 많다. 로그를 남길 때 사용할 jupno, id, ip를 구하는 코드가 달라서, 이 세 개 값을 파라미터로 전달해야 했다.


이렇게 되면 코드를 추가로 정리할 때 번잡함이 발생한다. 예를 들어, runTrans()에서 changeTranst()에 의미를 더 부여하기 위해 다음과 같이 변경한다고 하자.


private Response runTrans(String apiId, String jupno, 

                                   String id, String ip, Object req, Supplier<Result> transition) {

    String code = null;

    String transt = null;

    try {

        jupno = request.getJupno();

        transt = beforeTranst(jupno, id, ip); // 전송전 상태

        Result result = transition.get(); // 전송 처리

        code = eocsResult.getCode();

        transt = afterTranst(jupno, id, ip); // 전송완료 상태

    }catch (EocsException ex) {

        errorTranst(jupno, id, ip); // 전송오류 상태

        code = ex.getEocsErrorCode();

        errorLogger.append(apiId, code, jupno, transt, req, id, ip,....);

    }catch (Exception e) {

        code = "C00003";

        errorLogger.append(apiId, code, jupno, transt, req, id, ip,....);

    }

    return new Response(apiId , code);

}


private String beforeTranst(String jupno, String id, String ip) {

    changeTranst(jupno, "1", id, ip);

    return "1";

}

...afterTranst()와 errorTranst()도 비슷하게 구현


private changeTranst(String jupno, String transt, String id, String ip) {

    ...

}


runTrans() 메서드는 에러 로그를 기록하는데 transt 로컬 변수를 사용한다. 그래서 beforeTranst() 메서드는 상태 변경 실행후 transt 로컬 변수에 할당할 값을 리턴해야 한다. 게다가 beforeTranst() 메서드는 changeTranst() 메서드를 호출하기 위해 필요한 값을 파라미터 3개로 받고 있다.


흐름 처리를 수행하는 클래스 작성


메서드 추출로 중복을 없앨 수는 있지만, 그 결과로 만들어진 코드가 이쁘지 않다. 로컬 변수나 파라미터 개수가 많기에 이를 별도 객체로 뽑아서 중복을 없애는 것으로 방법을 바꿨다. 먼저 다음과 같이 프로세스를 처리하는 클래스를 만들었다.


public class TransitionProcess {

    private ErrorLogger errorLogger;


    private String apiId;

    private String jupno;

    private String id;

    private String ip;


    private String transt;

    private String code;


    @Builder

    public TransitionProcess(

            ErrorLogger errorLogger, String apiId,

            String jupno, String id, String ip) {

        this.errorLogger = errorLogger;

        this.apiId = apiId;

        this.jupno = jupno;

        this.id = id;

        this.ip = ip;

    }

    

    public Response runWith(Supplier<Result> transition) {

        try {

            beforeTranst();

            Result result = transition.get();

            code = result.getCode();

            afterTranst();

        } catch (EocsClientException ex) {

            transtError();

            code = ex.getEocsErrorCode();

            appendErrorLog(ex);

        } catch (Exception ex) {

            code = "C00003";

            appendErrorLog(ex);

        }

        return new Response(apiId , code);

    }


    private void beforeTranst() {

        transt = "0";

        changeTranst();

    }


    ...


    private void changeTranst() {

        ... // apiId, jupno, code, transt 등 필요한 값이 필드에 존재

    }


    private void appendError(Exception ex) {

        errorLogger.append(apiId, code, jupno, transt, id, ip, ex);

    }

}


TransitionProcess 클래스는 생성자를 통해서 로그 기록이나 상태 변경에 필요한 값을 받는다. runWith() 메서드는 전송 처리 기능을 함수형 인터페이스로 전달받는다.


상태 변경을 수행하는데 필요한 값(jupno, id, ip, transt)이 필드에 존재하므로 changeTranst() 메서드는 파라미터가 필요없다. 동일하게 beforeTranst() 메서드나 afterTranst() 메서드도 파라미터가 필요 없다.


에러 로그를 남기기 위한 appendError()도 동일하게 에러 로그를 기록하는데 필요한 값 중 Exception을 제외한 나머지는 필드에 존재한다. 그래서 파라미터로 Exception만 받으면 된다.


사실, TransitionProcess의 beforeTrans(), afterTrans(), errorTrans(), appendError() 메서드는 처음부터 존재한 것이 아니고 클래스로 추출한 뒤에 리팩토링하는 과정에서 생긴 것이다. 파라미터로 필요한 값을 전달할 필요가 없기 때문에 코드를 정리하기 더 쉽고, 메서드 호출에 파라미터가 없으므로 코드도 덜 복잡하다.


결과


다음은 흐름 처리 객체를 이용햇서 변경한 코드이다.


public Response chooseMeeting(Request req) {

    String apiId = "E0004";

    TransitionProcess process = TransitionProcess.builder()

        .errorLogger(errorLogger)

        .apiId(apiId)

        .jupno(req.getJupno())

        .id(req.getId())

        .ip(req.getIp())

        .build();

    return process.runWith(() -> tranService.chooseMeeting(req));

}


public Response choosePip(PipRequest req) {

    String apiId = "E0003";

    TransitionProcess process = TransitionProcess.builder()

        .errorLogger(errorLogger)

        .apiId(apiId)

        .jupno(req.getJupno())

        .id(req.getId())

        .ip(req.getIp())

        .build();

    return process.runWith(() -> tranService.choosePip(req));

}


// changeTranst 메서드 제거. TransitionProcess로 이동.


개인적으로는 처음 메서드 추출로 중복을 제거하려고 했던 것보다 더 깔끔하고 프로세스 처리가 별도 클래스로 빠져서 프로세스 분석이나 수정이 용이해진 것 같다.

Posted by 최범균 madvirus

댓글을 달아 주세요

김용훈님(http://bluepoet.me)과 진행했었던 리팩토링 사례를 공유합니다.


Posted by 최범균 madvirus

댓글을 달아 주세요

이 글은 http://www.slipp.net/questions/153 에 있는 내용을 보고 리팩토링을 과정을 한 번 정리해본 것이다.


1. 테스트 코드 일부 리팩토링 및 버그 수정


테스트 대상은 ApiUsersController 클래스이다. 이 클래스를 테스트하는 코드는 아래와 같다.


@RunWith(value = MockitoJUnitRunner.class)

public class ApiUsersControllerTest {

    @Mock

    private SocialUserService userService;


    @InjectMocks

    private ApiUsersController dut = new ApiUsersController();


    @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"));

    }

}


다음의 두 가지가 걸린다.

  • 상수처럼 쓰이는 것: "userId", "email@slipp.net", new SocialUser(1L), "false", "true"
  • loginUser 변수의 의미가 두 메서드에서 다름

이 두 가지를 정리하기 위해 먼저 상수처럼 쓰이는 것들을 상수로 정리한다.


@RunWith(value = MockitoJUnitRunner.class)

public class ApiUsersControllerTest {

    public static final String USER_ID = "userId";

    public static final String EMAIL = "email@slipp.net";

    public static final SocialUser USER1 = new SocialUser(1L);

    public static final String FALSE = "false";

    public static final String TRUE = "true";


    @Mock

    private SocialUserService userService;


    @InjectMocks

    private ApiUsersController dut = new ApiUsersController();


    @Test

    public void duplicateUserId_login_isSameUser() {

        when(userService.findByUserId(USER_ID)).thenReturn(USER1);

        String actual = dut.duplicateUserId(USER1, USER_ID);

        assertThat(actual, is(FALSE));

    }


    @Test

    public void duplicateUserId_login_isNotSameUser() {

        when(userService.findByUserId(USER_ID)).thenReturn(USER1);

        String actual = dut.duplicateUserId(new SocialUser(2L), USER_ID);

        assertThat(actual, is(TRUE));

    }


    @Test

    public void duplicateEmail_doesnot_existed() {

        String actual = dut.duplicateEmail(SocialUser.GUEST_USER, USER_ID, ProviderType.slipp);

        assertThat(actual, is(FALSE));

    }


    @Test

    public void duplicateEmail_login_isSameUser() {

        when(userService.findByEmailAndProviderId(EMAIL, ProviderType.slipp)).thenReturn(USER1);

        String actual = dut.duplicateEmail(USER1, EMAIL, ProviderType.slipp);

        assertThat(actual, is(FALSE));

    }


    @Test

    public void duplicateEmail_login_isNotSameUser() {

        when(userService.findByEmailAndProviderId(EMAIL, ProviderType.slipp)).thenReturn(USER1);

        String actual = dut.duplicateEmail(USER1, EMAIL, ProviderType.slipp);

        assertThat(actual, is(FALSE));

    }

}


리팩토링 후, 테스트를 돌려보니 정상 동작한다.


음,, 근데, duplicateEmail_login_isSameUser() 메서드와 duplicateEmail_login_isNotSameUser() 메서드가 구현이 동일하다. 이를 올바르게 변경하기 위해 duplicateEmail_login_isNotSameUser()를 다음과 같이 변경한다.


@RunWith(value = MockitoJUnitRunner.class)

public class ApiUsersControllerTest {

    ...

    public static final SocialUser USER2 = new SocialUser(2L);


    @Test

    public void duplicateUserId_login_isNotSameUser() {

        when(userService.findByUserId(USER_ID)).thenReturn(USER1);

        String actual = dut.duplicateUserId(USER2, USER_ID);

        assertThat(actual, is(TRUE));

    }

    ...

    @Test

    public void duplicateEmail_login_isNotSameUser() {

        when(userService.findByEmailAndProviderId(EMAIL, ProviderType.slipp)).thenReturn(USER1);

        String actual = dut.duplicateEmail(USER2, EMAIL, ProviderType.slipp);

        assertThat(actual, is(TRUE));

    }

}


2. 테스트 대상 코드


테스트 대상 클래스인 ApiUsersController 클래스는 아래와 같다.


@Controller

public class ApiUsersController {

    private SocialUserService userService;


    @RequestMapping("/duplicate_userid")

    @ResponseBody

    public String duplicateUserId(SocialUser loginUser, String userId) {

        SocialUser socialUser = userService.findByUserId(userId);

        if (socialUser == null) {

            return "false";

        }

        if (socialUser.isSameUser(loginUser)) {

            return "false";

        }

        return "true";

    }


    @RequestMapping("/duplicate_email")

    @ResponseBody

    public String duplicateEmail(SocialUser loginUser, String email, ProviderType providerType) {

        SocialUser socialUser = userService.findByEmailAndProviderId(email, providerType);

        if (socialUser == null) {

            return "false";

        }

        if (socialUser.isSameUser(loginUser)) {

            return "false";

        }

        return "true";

    }


}


http://www.slipp.net/questions/153 링크의 글을 보면 위의 중복된 코드를 제거하기 위한 작업에서 시작된 것을 알 수 있다. 음.... 중복 코드 제거 작업 이전에 한 가지 먼저 처리하고 싶은 것이 있다. 그것은 ApiUsersController 로부터 중복 검사 로직을 분리하는 것이다. ApiUsersController 클래스는 두 가지 책임을 가지고 있다.

  • 웹 요청 수신 및 처리 결과 응답
  • 중복 검사 구현

중복 검사 구현이 변경되어도 바뀌고, 응답 방식이 바뀌어도 바뀐다. 즉, 단일 책임 원칙 위반(SRP; Single Responsibility Principle)이다. 분리하자. 이거 분리하려면 리팩토링을 몇 단계를 거쳐야 한다.

  • 중복 검사 기능 제공하는 CheckDupService 클래스에 두 메서드를 복사하고,
  • CheckDupService 클래스의 객체를 생성하고
  • 그 객체에 위임하고
  • ApiUsersControllerTest 통과하나 확인하고,
  • CheckDupService 클래스를 CheckDupServiceImpl로 복사하고, CheckDupService를 인터페이스로 바꾸고,
  • ApiUsersControllerTest 클래스로부터 CheckDupServceImpl에 대한 단위 테스트를 작성해서 동작확인하고,
  • ApiUsersControllerTest 클래스는 CheckDupService 인터페이스를 Mock으로 하도록 바꾸고,
  • 등등등
음, 글로 일일이 정리하면 글 쓰다가 토가 나올지 모르니, 일단 중간 과정의 코드를 보자.

3. ApiUsersController에서 중복 검사 기능 별도 클래스로 분리

중복 검사 기능을 별도 클래스로 분리하고, 그것이 정상적으로 동작하는지 확인한 과정까지의 진행 결과이다. 먼저 CheckDupService 클래스의 코드는 아래와 같다. 두 메서드의 리턴 타입을 String에서 boolean으로 변경했다.

// ApiUsersController의 코드 상당 부분 이동
public class CheckDupService {
    private SocialUserService userService;

    public CheckDupService(SocialUserService userService) {
        this.userService = userService;
    }

    public boolean duplicateUserId(SocialUser loginUser, String userId) {
        SocialUser socialUser = userService.findByUserId(userId);
        if (socialUser == null)
            return false;
        return !socialUser.isSameUser(loginUser);
    }

    public boolean duplicateEmail(SocialUser loginUser, String email, ProviderType providerType) {
        SocialUser socialUser = userService.findByEmailAndProviderId(email, providerType);
        if (socialUser == null)
            return false;

        return !socialUser.isSameUser(loginUser);
    }
}

ApiUsersController 클래스는 이제 CheckDupService를 사용하도록 바뀐다.

@Controller
public class ApiUsersController {
    private CheckDupService checkDupService;

    public ApiUsersController(CheckDupService checkDupService) {
        this.checkDupService = checkDupService;
    }

    @RequestMapping("/duplicate_userid")
    @ResponseBody
    public String duplicateUserId(SocialUser loginUser, String userId) {
        return Boolean.toString(checkDupService.duplicateUserId(loginUser, userId));
    }

    @RequestMapping("/duplicate_email")
    @ResponseBody
    public String duplicateEmail(SocialUser loginUser, String email, ProviderType providerType) {
        return Boolean.toString(checkDupService.duplicateEmail(loginUser, email, providerType));
    }

}

ApiUsersController 클래스는 이제 아래와 같이 ApiUsersController에 CheckDupService 객체를 전달하도록 바뀐다.


@RunWith(value = MockitoJUnitRunner.class)

public class ApiUsersControllerTest {


    @Mock

    private SocialUserService userService;


    private ApiUsersController dut;


    @Before

    public void init() {

        CheckDupService checkDupService = new CheckDupService(userService);

        dut = new ApiUsersController(checkDupService);

    }


    // 테스트 메서드는 바뀌지 않음

    @Test

    public void duplicateUserId_login_isSameUser() {

        when(userService.findByUserId(USER_ID)).thenReturn(USER1);

        String actual = dut.duplicateUserId(USER1, USER_ID);

        assertThat(actual, is(FALSE));

    }


테스트를 돌려보면 통과다. 다음 단계로 넘어가자.


4. CheckDupService를 인터페이스로 추출하기


CheckDupService 클래스에서 인터페이스를 추출하자.


public interface CheckDupService {

    boolean duplicateUserId(SocialUser loginUser, String userId);


    boolean duplicateEmail(SocialUser loginUser, String email, ProviderType providerType);

}


// 기존 CheckDupService 클래스를 CheckDupServiceImpl 클래스로 변경

public class CheckDupServiceImpl implements CheckDupService {

    private SocialUserService userService;


    public CheckDupServiceImpl(SocialUserService userService) {

        this.userService = userService;

    }


테스트 클래스는 이제 CheckDupServiceImpl 클래스를 이용해서 객체를 생성하도록 바뀐다.


@RunWith(value = MockitoJUnitRunner.class)

public class ApiUsersControllerTest {

    ...

    @Before

    public void init() {

        CheckDupService checkDupService = new CheckDupServiceImpl(userService);

        dut = new ApiUsersController(checkDupService);

    }


5. CheckDupServiceImpl 클래스에 대한 단위 테스트 만들기


ApiUsersControllerTest 클래스는 사실상 CheckDupServiceImpl 클래스의 테스트에 가까워졌다. ApiUsersControllerTest 클래스를 복사해서 CheckDupServiceImpl 클래스에 대한 단위 테스트를 만들자.


@RunWith(value = MockitoJUnitRunner.class)

public class CheckDupServiceImplTest {

    public static final String USER_ID = "userId";

    public static final String EMAIL = "email@slipp.net";

    public static final SocialUser USER1 = new SocialUser(1L);

    public static final SocialUser USER2 = new SocialUser(2L);


    @Mock

    private SocialUserService userService;


    private CheckDupService checkDupService;


    @Before

    public void init() {

        checkDupService = new CheckDupServiceImpl(userService);

    }


    @Test

    public void duplicateUserId_login_isSameUser() {

        when(userService.findByUserId(USER_ID)).thenReturn(USER1);

        boolean actual = checkDupService.duplicateUserId(USER1, USER_ID);

        assertThat(actual, is(false));

    }


    @Test

    public void duplicateUserId_login_isNotSameUser() {

        when(userService.findByUserId(USER_ID)).thenReturn(USER1);

        boolean actual = checkDupService.duplicateUserId(new SocialUser(2L), USER_ID);

        assertThat(actual, is(true));

    }


    @Test

    public void duplicateEmail_doesnot_existed() {

        boolean actual = checkDupService.duplicateEmail(

               SocialUser.GUEST_USER, USER_ID, ProviderType.slipp);

        assertThat(actual, is(false));

    }


    @Test

    public void duplicateEmail_login_isSameUser() {

        when(userService.findByEmailAndProviderId(EMAIL, ProviderType.slipp)).thenReturn(USER1);

        boolean actual = checkDupService.duplicateEmail(USER1, EMAIL, ProviderType.slipp);

        assertThat(actual, is(false));

    }


    @Test

    public void duplicateEmail_login_isNotSameUser() {

        when(userService.findByEmailAndProviderId(EMAIL, ProviderType.slipp)).thenReturn(USER1);

        boolean actual = checkDupService.duplicateEmail(USER2, EMAIL, ProviderType.slipp);

        assertThat(actual, is(true));

    }

}


테스트 실행, 통과다.


6. ApiUsersControllerTest 클래스에서 UserService 관련 코드 제거


이제 ApiUsersController 클래스는 더 이상 UserService를 사용하지 않으므로, UserService Mock 객체를 만들 필요가 없다. ApiUsersController 협업 대상이 CheckDupService로 변경되었으므로, CheckDupService에 대한 Mock을 만들어서 테스트하도록 ApiUsersControllerTest를 바꾸자.


@RunWith(value = MockitoJUnitRunner.class)

public class ApiUsersControllerTest {

    public static final String USER_ID = "userId";

    public static final String EMAIL = "email@slipp.net";

    public static final SocialUser USER1 = new SocialUser(1L);

    public static final SocialUser USER2 = new SocialUser(2L);

    public static final String TRUE = "true";

    public static final String FALSE = "false";


    @Mock

    private CheckDupService checkDupService;


    private ApiUsersController dut;


    @Before

    public void init() {

        dut = new ApiUsersController(checkDupService);

    }


    @Test

    public void duplicateUserId_login_isSameUser() {

        when(

             checkDupService.duplicateUserId(any(SocialUser.class), anyString())

        ).thenReturn(Boolean.FALSE);

        String actual = dut.duplicateUserId(USER1, USER_ID);

        assertThat(actual, is(FALSE));

    }


    @Test

    public void duplicateUserId_login_isNotSameUser() {

        when(

            checkDupService.duplicateUserId(any(SocialUser.class), anyString())

        ).thenReturn(Boolean.TRUE);


        String actual = dut.duplicateUserId(USER2, USER_ID);

        assertThat(actual, is(TRUE));

    }


    /* 더 이상 필요 없으므로 삭제!

    @Test

    public void duplicateEmail_doesnot_existed() {

        String actual = dut.duplicateEmail(SocialUser.GUEST_USER, USER_ID, ProviderType.slipp);

        assertThat(actual, is(FALSE));

    }

    */


    @Test

    public void duplicateEmail_login_isSameUser() {

        when(

            checkDupService.duplicateEmail(

                any(SocialUser.class), anyString(), any(ProviderType.class))

        ).thenReturn(Boolean.FALSE);

        String actual = dut.duplicateEmail(USER1, EMAIL, ProviderType.slipp);

        assertThat(actual, is(FALSE));

    }


    @Test

    public void duplicateEmail_login_isNotSameUser() {

        when(

            checkDupService.duplicateEmail(

                any(SocialUser.class), anyString(), any(ProviderType.class))

        ).thenReturn(Boolean.TRUE);

        String actual = dut.duplicateEmail(USER2, EMAIL, ProviderType.slipp);

        assertThat(actual, is(TRUE));

    }

}


7. 다시 최초의 문제로 돌아가


최초의 문제는 ApiUsersController 클래스의 아이디/이메일이 이미 존재하는지 확인하는 코드에서 중복이 발생해서 그걸 빼고 싶은 것이었다. 이 부분이 지금은 CheckDupServiceImpl 클래스로 옮겨졌으니, 그 부분의 코드를 보자.


public class CheckDupServiceImpl implements CheckDupService {

    ...

    @Override

    public boolean duplicateUserId(SocialUser loginUser, String userId) {

        SocialUser socialUser = userService.findByUserId(userId);

        if (socialUser == null)

            return false;

        return !socialUser.isSameUser(loginUser);

    }


    @Override

    public boolean duplicateEmail(SocialUser loginUser, String email, ProviderType providerType) {

        SocialUser socialUser = userService.findByEmailAndProviderId(email, providerType);

        if (socialUser == null)

            return false;

        return !socialUser.isSameUser(loginUser);

    }

}


굵게 표시한 부분이 중복된 코드인데, 이를 어떻게 없애면 좋을까? 의미를 먼저 파악해보자.

  • userId와 일치하는 유저가 존재하는지 검사한다. 존재하지 않으면 false를 리턴한다.
  • 단, 현재 로그인한 유저의 userId를 입력한 경우에는 false를 리턴한다.
음,,, 두 번째의 의미를 잘 모르겠다. 이 의미를 알면 조금 더 정리해 볼 수 있을 것 같은데, 지금으로서는 단순히 메서드로 빼는 것 정도에서 끝내야 겠다.

public class CheckDupServiceImpl implements CheckDupService {
    private SocialUserService userService;

    public CheckDupServiceImpl(SocialUserService userService) {
        this.userService = userService;
    }

    @Override
    public boolean duplicateUserId(SocialUser loginUser, String userId) {
        SocialUser socialUser = userService.findByUserId(userId);
        return doSomeMeaningCheck(loginUser, socialUser);
    }

    @Override
    public boolean duplicateEmail(SocialUser loginUser, String email, ProviderType providerType) {
        SocialUser socialUser = userService.findByEmailAndProviderId(email, providerType);
        return doSomeMeaningCheck(loginUser, socialUser);
    }

    private boolean doSomeMeaningCheck(SocialUser loginUser, SocialUser socialUser) {
        if (socialUser == null) return false;
        return ! socialUser.isSameUser(loginUser);
    }
}

CheckDupServiceImplTest를 실행해보자. 녹색이다. 테스트 통과다. doSomeMeaningCheck() 메서드는 아직 의미를 정확하게 몰라서 메서드 이름을 이렇게 지었다.


8. 의미에 맞는 기능 분리


약간의 대화를 거쳐서 doSomeMeaningCheck() 메서드의 의미를 알았다. 음, 정확하게는 다음의 두 기능이 필요한 것이다.

  • 회원 가입시 사용할 사용자아이디/이메일 중복 검사 기능
  • 회원 정보 수정에서 사용자아이디/이메일 중복 검사 기능
    • 현재와 동일한 사용자아이디/이메일 입력시 중복으로 처리하지 않음

이 두 가지는 의미가 다소 다르기 때문에 분리된 메서드를 제공하는 것이 좋지 않을까 하는 생각이 든다. 예를 들면 아래와 같은 분리가 필요하지 않을까? 이건 나중에 다시 생각해보고, 쉬운 것 부터 해보자.


9. 코드 정리: 메서드 이름 변경


CheckDupService 인터페이스는 다음과 같이 정의되어 있다.


public interface CheckDupService {

    boolean duplicateUserId(SocialUser loginUser, String userId);

    boolean duplicateEmail(SocialUser loginUser, String email, ProviderType providerType);

}


duplicateUserId 란 이름은 "중복된 사용자 ID" 이므로, 검사한다는 의미를 부여할 수 있도록 아래와 같이 check라는 단어를 붙여보자.


public interface CheckDupService {

    boolean checkDuplicateUserId(SocialUser loginUser, String userId);


    boolean checkDuplicateEmail(SocialUser loginUser, String email, ProviderType providerType);

}


이름을 변경했으면 단위 테스트를 실행해서 통과되는지 확인한다.


ApiUsersController 클래스도 동일하게 check 단어를 붙여주자.


@Controller

public class ApiUsersController {

    ...

    @RequestMapping("/duplicate_userid")

    @ResponseBody

    public String checkDuplicateUserId(SocialUser loginUser, String userId) {

        return Boolean.toString(checkDupService.checkDuplicateUserId(loginUser, userId));

    }


    @RequestMapping("/duplicate_email")

    @ResponseBody

    public String checkDuplicateEmail(SocialUser loginUser, String email, ProviderType providerType) {

        return Boolean.toString(checkDupService.checkDuplicateEmail(loginUser, email, providerType));

    }


}


테스트 메서드의 이름도 변경해주자.


@RunWith(value = MockitoJUnitRunner.class)

public class CheckDupServiceImplTest {

    public static final String USER_ID = "userId";

    public static final String EMAIL = "email@slipp.net";

    public static final SocialUser USER1 = new SocialUser(1L);

    public static final SocialUser USER2 = new SocialUser(2L);


    @Mock

    private SocialUserService userService;


    private CheckDupService checkDupService;


    @Before

    public void init() {

        checkDupService = new CheckDupServiceImpl(userService);

    }


    @Test

    public void shouldReturnFalseWhenLoggedUserInputSelfCurrentUserId() {

        when(userService.findByUserId(USER_ID)).thenReturn(USER1);

        boolean actual = checkDupService.checkDuplicateUserId(USER1, USER_ID);

        assertThat(actual, is(false));

    }


    @Test

    public void shouldReturnTrueWhenLoggedUserInputUserIdOfAnotherUser() {

        when(userService.findByUserId(USER_ID)).thenReturn(USER1);

        boolean actual = checkDupService.checkDuplicateUserId(USER2, USER_ID);

        assertThat(actual, is(true));

    }


    @Test

    public void shouldReturnFalseWhenGuestInputEmailWhichDoesNotExist() {

        when(userService.findByEmailAndProviderId(USER_ID, ProviderType.slipp)).thenReturn(null);

        boolean actual = checkDupService.checkDuplicateEmail(SocialUser.GUEST_USER, USER_ID, ProviderType.slipp);

        assertThat(actual, is(false));

    }


    @Test

    public void shouldReturnFalseWhenLoggedUserInputSelfCurrentEmail() {

        when(userService.findByEmailAndProviderId(EMAIL, ProviderType.slipp)).thenReturn(USER1);

        boolean actual = checkDupService.checkDuplicateEmail(USER1, EMAIL, ProviderType.slipp);

        assertThat(actual, is(false));

    }


    @Test

    public void shouldReturnTrueWhenLoggedUserInputEmailOfOtherUser() {

        when(userService.findByEmailAndProviderId(EMAIL, ProviderType.slipp)).thenReturn(USER1);

        boolean actual = checkDupService.checkDuplicateEmail(USER2, EMAIL, ProviderType.slipp);

        assertThat(actual, is(true));

    }

}


@RunWith(value = MockitoJUnitRunner.class)

public class ApiUsersControllerTest {

    public static final String USER_ID = "userId";

    public static final String EMAIL = "email@slipp.net";

    public static final SocialUser USER1 = new SocialUser(1L);

    public static final SocialUser USER2 = new SocialUser(2L);

    public static final String TRUE = "true";

    public static final String FALSE = "false";


    @Mock

    private CheckDupService checkDupService;


    private ApiUsersController dut;


    @Before

    public void init() {

        dut = new ApiUsersController(checkDupService);

    }


    @Test

    public void shouldReturnFalseWhenCheckDuplicateUserIdReturnFalse() {

        when(checkDupService.checkDuplicateUserId(any(SocialUser.class), anyString())).thenReturn(Boolean.FALSE);

        String actual = dut.checkDuplicateUserId(USER1, USER_ID);

        assertThat(actual, is(FALSE));

    }


    @Test

    public void shouldReturnTrueWhenCheckDuplicateUserIdReturnTrue() {

        when(checkDupService.checkDuplicateUserId(any(SocialUser.class), anyString())).thenReturn(Boolean.TRUE);

        String actual = dut.checkDuplicateUserId(new SocialUser(2L), USER_ID);

        assertThat(actual, is(TRUE));

    }


    @Test

    public void shouldReturnFalseWhenCheckDuplicateEmailReturnFalse() {

        when(checkDupService.checkDuplicateEmail(any(SocialUser.class), anyString(), any(ProviderType.class))).thenReturn(Boolean.FALSE);

        String actual = dut.checkDuplicateEmail(USER1, EMAIL, ProviderType.slipp);

        assertThat(actual, is(FALSE));

    }


    @Test

    public void shouldReturnFalseWhenCheckDuplicateEmailReturnTrue() {

        when(checkDupService.checkDuplicateEmail(any(SocialUser.class), anyString(), any(ProviderType.class))).thenReturn(Boolean.TRUE);

        String actual = dut.checkDuplicateEmail(USER2, EMAIL, ProviderType.slipp);

        assertThat(actual, is(TRUE));

    }

}


의미에 맞는 기능분리까지 하고 싶으나, UI 코드의 상황을 잘 모르는 관계로 여기에서 마무리 지어본다.



Posted by 최범균 madvirus

댓글을 달아 주세요

  1. maruldy 2013.05.23 14:53 신고  댓글주소  수정/삭제  댓글쓰기

    안녕하세요. 종종 와서 도움받고 갑니다.. ^^;

    리팩토링 글을 읽다가.. 궁금한점이 있어서 댓글을 달게 됐습니다 ^^;
    java 에서 상호참조는 절대로 하면 안되는 건가요?

    어떤분은 상호참조는 안좋다.
    또 누구는 reference 처리만 잘해주면 좋다..
    패키지 분류만 잘 해놨다면 같은 패키지 내에서는 상호참조는 허용해도 좋다.

    등등 얘기가 있던데요.. 솔직히 어떤말이 정답인지를 잘 모르겠어서요..

    추상 클래스 , 인터페이스 기반이던 구현클래스 기반이던 일단 서로 참조를 하게 되면
    각각 클래스 내용을 수정할 때 심적 소모가 더 들것 같긴 하지만..
    그래도 쓰면 가끔 편할것 같아서 쓰고싶을때가 있다.. 정도로 생각하고 있습니다..

    최범균님은 어떻게 생각하시는지요.. 조언 부탁드립니다.

    • 최범균 madvirus 2013.05.23 16:48 신고  댓글주소  수정/삭제

      이런 류에 정답은 없지만,
      부득이한 경우가 아니라면 상호 참조는 가급적(!) 하지 않는 것이 좋을 것 같습니다. 특히, 서로 다른 패키지 간의 상호 참조/순환 참조는 하지 않는 것이 코드 변경에 유리하다고 생각합니다.
      물론, 같은 패키지 내에서는 필요에 따라 상호 참조가 발생할 수 있겠죠.

    • maruldy 2013.05.23 17:48 신고  댓글주소  수정/삭제

      부득이한 경우가 아니라면 상호 참조는 하지 않는것이 좋지만, 같은 패키지 내에서는 필요에 의해 상호참조를 허용할 수 있다는 말씀이시군요.

      시간 내서 답변 해주셔서 감사합니다. 참고 하겠습니다! ^^ㅎㅎ;

  2. 나그네 2014.03.11 22:37 신고  댓글주소  수정/삭제  댓글쓰기

    열심히 보구 배우는 개발자 입니다.
    위에 리펙토링 한 소스 보구 궁금한게 있어서 리플답니다.
    ApiUsersControllerTest에 각 테스트케이스에 when(checkDupService.checkDuplicateUserId(any(SocialUser.class), anyString())).thenReturn(Boolean.FALSE); 이런식으로 결과값을 미리 지정을 하시고 마지막에 assertThat(actual, is(FALSE)); 라고 작성하셨는데 when 문장이 필요한건가요?
    의미가 별루 없어 보이는데....
    이미 when문장에서 리턴값을 true 또는 false라고 명시하고 assert문장에서 그것이 맞는지 검증하는 부분은 불필요해보여서요..^^;
    어차피 String actual = dut.checkDuplicateUserId(USER1, USER_ID); 이문장은 내부적으로 checkDupService.checkDuplicateUserId(..., ..., ...)를 호출하기 때문에 무조건 thenReturn값을 반환할텐데...

    혹시나 다른 이유가 있어서 적으신건지 궁금해서요 ^^;;;

지금까지 한 번에 한 개의 변환 결과만 생성하는 기능을 구현했다. 이번엔 한 번에 여러 형식의 변환 결과를 생성해주는 기능을 구현해보도록 하자.


두 개 이상의 OutputFormat을 입력으로 받는 테스트 추가하기


TDD니까, 테스트를 먼저 작성해보도록 하자.


public class FfmpegTranscoderTest {

    ...

    private OutputFormat mp4Format;

    private OutputFormat mp4Format2;

    private OutputFormat aviFormat;


    @Before

    public void setup() {

        outputFormats = new ArrayList<OutputFormat>();

        mp4Format = new OutputFormat(160, 120, 150, Container.MP4,

                VideoCodec.H264, AudioCodec.AAC);

        mp4Format2 = new OutputFormat(80, 60, 80, Container.MP4,

                VideoCodec.H264, AudioCodec.AAC);

        aviFormat = new OutputFormat(160, 120, 150, Container.AVI,

                VideoCodec.MPEG4, AudioCodec.MP3);

        multimediaFile = new File("src/test/resources/sample.avi");


        transcoder = new FfmpegTranscoder();

    }

    ...

    private void executeTranscoderAndAssert() {

        List<File> transcodedFiles = transcoder.transcode(multimediaFile,

                outputFormats);

        assertEquals(outputFormats.size(), transcodedFiles.size());

        for (int i = 0; i < outputFormats.size(); i++) {

            assertTrue(transcodedFiles.get(i).exists());

            VideoFormatVerifier.verifyVideoFormat(outputFormats.get(i),

                    transcodedFiles.get(i));

        }

    }


    @Test

    public void transcodeWithTwoMp4OutputFormats() {

        outputFormats.add(mp4Format);

        outputFormats.add(mp4Format2);

        executeTranscoderAndAssert();

    }

}


새로운 크기를 갖는 OutputFormat 객체인 mp4Format2를 필드로 추가하였고, transcodeWithTwoMp4OutputFormats() 메서드를 이용해서 두 개의 변환 결과에 대한 테스트를 수행하도록 했다. executeTrancoderAndAssert() 메서드는 두 개 이상의 변환 요청을 검증할 수 있도록 수정하였다.


테스트를 실행해보자. 아래와 같은 메시지와 함께 테스트에 실패한다.


java.lang.AssertionError: expected:<160> but was:<80>

    at org.junit.Assert.fail(Assert.java:91)

    at org.junit.Assert.failNotEquals(Assert.java:645)

    at org.junit.Assert.assertEquals(Assert.java:126)

    at org.junit.Assert.assertEquals(Assert.java:470)

    at org.junit.Assert.assertEquals(Assert.java:454)

    at org.chimi....VideoFormatVerifier.assertVideoFile(VideoFormatVerifier.java:88)

    at org.chimi....VideoFormatVerifier.verify(VideoFormatVerifier.java:40)

    at org.chimi....VideoFormatVerifier.verifyVideoFormat(VideoFormatVerifier.java:18)

    at org.chimi.....executeTranscoderAndAssert(FfmpegTranscoderTest.java:64)

    at org.chimi....transcodeWithTwoMp4OutputFormats(FfmpegTranscoderTest.java:79)

    ... 


이유는 뭘까? FfmpegTranscoder 클래스는 다음과 같이 동일한 비디오 컨테이너에 대해 항상 동일한 파일을 생성한다. 따라서, 가장 마지막으로 요청한 OutputFormat의 결과만 최종적으로 남아있게 된다.


public class FfmpegTranscoder implements Transcoder {

    ...

    private String getFileName(OutputFormat format) {

        return "outputFile." + format.getFileExtension();

    }


}


예를 들어, 위 테스트의 경우 크기가 80*60인 비디오 파일이 최종적으로 남게 되고, 따라서 생성된 비디오를 검증하는 과정에서 첫 번째 변환 요청인 160*120 크기를 검사하는 과정에서 테스트를 통과하지 못하게 되었다.


이 테스트를 통과시키려면 FfmpegTranscoder의 getFileName() 메서드가 매번 서로 다른 파일 이름을 리턴해 주어야 한다. 변환된 결과 파일 이름의 경우 변환 시스템을 사용하는 곳에 따라 작명 규칙이 달라질 수 있을 것 같다. 그래서, 파일 이름을 생성하는 규칙을 NamingRule이라는 별도 인터페이스로 분리하고 FfmpegTranscoder는 NamingRule에 위임하도록 코드를 구성하는 게 좋을 것 같다. 아래는 그렇게 코딩한 결과이다.


public class FfmpegTranscoder implements Transcoder {


    private NamingRule namingRule;


    public FfmpegTranscoder(NamingRule namingRule) {

        this.namingRule = namingRule;

    }

    ...

    private String getFileName(OutputFormat format) {

        return namingRule.createName(format);

    }


}


NamingRule 인터페이스를 다음과 같이 만들었다.


public interface NamingRule {


    String createName(OutputFormat format);


}


FfmpegTranscoder의 생성자가 변경되었으므로, FfmpegTranscoderTest 클래스가 컴파일 에러가 난다. 컴파일 에러가 나지 않도록 NamingRule을 전달해주자.


public class FfmpegTranscoderTest {


    private Transcoder transcoder;

    ...

    private NamingRule namingRule;

    ...

    @Before

    public void setup() {

        outputFormats = new ArrayList<OutputFormat>();

        mp4Format = new OutputFormat(160, 120, 150, Container.MP4,

                VideoCodec.H264, AudioCodec.AAC);

        ...

        transcoder = new FfmpegTranscoder(namingRule); // 아직 namingRule은 null

    }


NamingRule의 Mock 객체를 만들어서 테스트를 통과시킬까 고민하다가, DefaultNamingRule이라는 걸 만드는 게 좋을 것 같다라는 결론에 도달했다. 다음과 같이 NamingRule 인터페이스에 이너 클래스로 DefaultNamingRule 클래스를 정의했고, DEFAULT 상수가 값으로 DefaultNameRule 객체를 갖도록 했다.


public interface NamingRule {


    String createName(OutputFormat format);


    public static final NamingRule DEFAULT = new DefaultNamingRule();


    public static class DefaultNamingRule implements NamingRule {


        private Random random = new Random();

        private String baseDir;


        public DefaultNamingRule() {

            baseDir = System.getProperty("java.io.tmpdir");

        }


        public void setBaseDir(String baseDir) {

            this.baseDir = baseDir;

        }


        @Override

        public String createName(OutputFormat format) {

            String fileName = getFileNameFromTime();

            File file = createFileFromFileNameAndFormat(format, baseDir,

                    fileName);

            return file.getPath();

        }


        private String getFileNameFromTime() {

            SimpleDateFormat dateFormat = new SimpleDateFormat("yyyyMMddHHmmss");

            String time = dateFormat.format(new Date());

            int num = random.nextInt(1000);

            String fileName = time + "_" + num;

            return fileName;

        }


        private File createFileFromFileNameAndFormat(OutputFormat format,

                String tempDir, String fileName) {

            File file = new File(tempDir, fileName + "."

                    + format.getFileExtension());

            return file;

        }

    }

}


이제 FfmpegTranscoderTest가 NamingRule로 NamingRule.DEFAULT를 사용하도록 수정할 차례이다.


public class FfmpegTranscoderTest {


    private Transcoder transcoder;

    private File multimediaFile;

    private List<OutputFormat> outputFormats;


    private NamingRule namingRule; // 필요 없으므로 삭제


    private OutputFormat mp4Format;

    private OutputFormat mp4Format2;

    private OutputFormat aviFormat;


    @Before

    public void setup() {

        outputFormats = new ArrayList<OutputFormat>();

        mp4Format = new OutputFormat(160, 120, 150, Container.MP4,

                VideoCodec.H264, AudioCodec.AAC);

        mp4Format2 = new OutputFormat(80, 60, 80, Container.MP4,

                VideoCodec.H264, AudioCodec.AAC);

        aviFormat = new OutputFormat(160, 120, 150, Container.AVI,

                VideoCodec.MPEG4, AudioCodec.MP3);

        multimediaFile = new File("src/test/resources/sample.avi");


        transcoder = new FfmpegTranscoder(NamingRule.DEFAULT);

    }


테스트를 실행한다. 녹색! 통과다. 서로 다른 포맷을 갖는 OutputFormat을 생성하는 기능 구현이 완료되었다.


OutputFormat: 컨테이너와 코덱 옵션


OutputFormat의 생성자는 컨테이너 정보와 코덱 정보를 함께 받고 있다. 하지만, 컨테이너의 기본 코덱을 사용할 수 있기 때문에 코덱 정보를 전달하지 않을 경우 컨테이너의 기본 코덱을 사용하도록 하는 것이 편리할 것 같다. 이를 위해 OutputFormat 생성자에 코덱 정보를 전달하지 않는 테스트 코드부터 작성해보자. OutputFormat를 생성할 때 컨테이너 정보만 전달해도 정상적으로 실행되는 지 확인하면 되므로  VideoConverterTest를 이용해보도록 하겠다.


public class VideoConverterTest {

    ...

    @Test

    public void transcodeWithOnlyContainer() {

        IMediaReader reader = ToolFactory.makeReader(SOURCE_FILE);

        

        OutputFormat outputFormat = new OutputFormat(WIDTH, HEIGHT, BITRATE,

                Container.AVI); // 일치하는 생성자 없으므로 컴파일 에러

        VideoConverter writer = new VideoConverter("target/sample.avi", reader,

                outputFormat);

        reader.addListener(writer);

        while (reader.readPacket() == null)

            do {

            } while (false);

        

        VideoFormatVerifier.verifyVideoFormat(outputFormat, new File(

                "target/sample.avi"));

    }

}


컴파일 에러를 없애야 하므로 OutputFormat에 생성자를 추가해주고, 다음과 같이 구현해 주었다.


public class OutputFormat {


    private int width;

    private int height;

    private int bitrate;

    private Container container;

    private VideoCodec videoCodec;

    private AudioCodec audioCodec;


    public OutputFormat(int width, int height, int bitrate,

            Container container, VideoCodec videoCodec, AudioCodec audioCodec) {

        this.width = width;

        this.height = height;

        this.bitrate = bitrate;

        this.container = container;

        this.videoCodec = videoCodec;

        this.audioCodec = audioCodec;

    }


    public OutputFormat(int width, int height, int bitrate, Container container) {

        this(width, height, bitrate, container, null, null);

    }


컴파일 에러를 없앴으므로, VideoConverterTest를 실행해보자. transcodeWithOnlyContainer() 테스트 메서드는 통과하지 못했다. 실패 메시지 및 관련 위치는 다음과 같다.


-- 실패 메시지

java.lang.AssertionError: expected:<null> but was:<MPEG4>

    at org.junit.Assert.fail(Assert.java:91)

    at org.junit.Assert.failNotEquals(Assert.java:645)

    at org.junit.Assert.assertEquals(Assert.java:126)

    at org.junit.Assert.assertEquals(Assert.java:145)

    at org....VideoFormatVerifier.assertVideoFile(VideoFormatVerifier.java:90)

    at org....VideoFormatVerifier.verify(VideoFormatVerifier.java:40)

    at org....VideoFormatVerifier.verifyVideoFormat(VideoFormatVerifier.java:18)

    at org....VideoConverterTest.transcodeWithOnlyContainer(VideoConverterTest.java:52)


-- 실패 위치

    private void assertVideoFile() {

        assertEquals(expectedFormat.getWidth(), width);

        assertEquals(expectedFormat.getHeight(), height);

        assertEquals(expectedFormat.getVideoCodec(),

                CodecValueConverter.toDomainVideoCodec(videoCodec)); // 테스트 실패

        assertEquals(expectedFormat.getAudioCodec(),

                CodecValueConverter.toDomainAudioCodec(audioCodec));

    }


OutputFormat을 생성하는 과정에서 videoCodec의 값으로 null을 전달했기 때문에, expectedFormat.getVideoCodec()가 null을 리턴했다. 동일하게 audioCodec도 null 값을 갖게 된다. 이 문제는 간단하게 처리할 수 있다. 다음과 같이 코덱을 전달받지 않는 OutputFormat 생성자에서 null 대신 Container의 기본 코덱을 전달하도록 코드를 수정해 주면 될 것 같다.


public class OutputFormat {

    ...

    public OutputFormat(int width, int height, int bitrate,

            Container container, VideoCodec videoCodec, AudioCodec audioCodec) {

        ...

        this.videoCodec = videoCodec;

        this.audioCodec = audioCodec;

    }


    public OutputFormat(int width, int height, int bitrate, Container container) {

        this(width, height, bitrate, container, 

                container.getDefaultVideoCodec(), container.getDefaultAudioCodec());

    }


다시 VideoConverterTest를 실행해 보자. 녹색! 통과다.


VideoConverterTest 작은 손질


VideoConverterTest를 정리할 타이밍이 왔다. 현재까지 만들어진 코드는 다음과 같다.


public class VideoConverterTest {


    private static final int WIDTH = 160;

    private static final int HEIGHT = 120;

    private static final int BITRATE = 150;

    private static final String SOURCE_FILE = "src/test/resources/sample.avi";

    private static final String TRANSCODED_FILE = "target/sample.mp4";


    @Test

    public void transcode() {

        IMediaReader reader = ToolFactory.makeReader(SOURCE_FILE);


        OutputFormat outputFormat = new OutputFormat(WIDTH, HEIGHT, BITRATE,

                Container.MP4, VideoCodec.H264, AudioCodec.AAC);

        VideoConverter writer = new VideoConverter(TRANSCODED_FILE, reader,

                outputFormat);

        reader.addListener(writer);

        while (reader.readPacket() == null)

            do {

            } while (false);


        VideoFormatVerifier.verifyVideoFormat(outputFormat, new File(

                TRANSCODED_FILE));

    }

    

    @Test

    public void transcodeWithOnlyContainer() {

        IMediaReader reader = ToolFactory.makeReader(SOURCE_FILE);

        

        OutputFormat outputFormat = new OutputFormat(WIDTH, HEIGHT, BITRATE,

                Container.AVI);

        VideoConverter writer = new VideoConverter("target/sample.avi", reader,

                outputFormat);

        reader.addListener(writer);

        while (reader.readPacket() == null)

            do {

            } while (false);

        

        VideoFormatVerifier.verifyVideoFormat(outputFormat, new File(

                "target/sample.avi"));

    }

}


두 테스트 메서드가 상당히 비슷하다. 우선, 두 메서드에서 사용되는 값들을 메서드의 앞 쪽에 변수로 선언해서 모으자.


public class VideoConverterTest {

    ...

    @Test

    public void transcode() {

        IMediaReader reader = ToolFactory.makeReader(SOURCE_FILE);

        OutputFormat outputFormat = new OutputFormat(WIDTH, HEIGHT, BITRATE,

                Container.MP4, VideoCodec.H264, AudioCodec.AAC);

        String outputFile = TRANSCODED_FILE;

        VideoConverter writer = new VideoConverter(outputFile, reader,

                outputFormat);

        reader.addListener(writer);

        while (reader.readPacket() == null)

            do {

            } while (false);


        VideoFormatVerifier.verifyVideoFormat(outputFormat,

                new File(outputFile));

    }


    @Test

    public void transcodeWithOnlyContainer() {

        IMediaReader reader = ToolFactory.makeReader(SOURCE_FILE);

        OutputFormat outputFormat = new OutputFormat(WIDTH, HEIGHT, BITRATE,

                Container.AVI);

        String outputFile = "target/sample.avi";

        VideoConverter writer = new VideoConverter(outputFile, reader,

                outputFormat);

        reader.addListener(writer);

        while (reader.readPacket() == null)

            do {

            } while (false);


        VideoFormatVerifier.verifyVideoFormat(outputFormat,

                new File(outputFile));

    }

}


두 메서드에서 reader, outputFormat, outputFile을 필드로 정의한다. 이클립스와 같은 IDE는 메서드 로컬 변수를 필드로 빼주는 기능을 제공하고 있으니 그 기능을 사용하면 편리하다. 아 그 전에 테스트 하는 것 잊지 말기 바란다.


public class VideoConverterTest {

    ...

    private IMediaReader reader;

    private OutputFormat outputFormat;

    private String outputFile;


    @Test

    public void transcode() {

        reader = ToolFactory.makeReader(SOURCE_FILE);

        outputFormat = new OutputFormat(WIDTH, HEIGHT, BITRATE,

                Container.MP4, VideoCodec.H264, AudioCodec.AAC);

        outputFile = TRANSCODED_FILE;

        VideoConverter writer = new VideoConverter(outputFile, reader,

                outputFormat);

        reader.addListener(writer);

        while (reader.readPacket() == null)

            do {

            } while (false);


        VideoFormatVerifier.verifyVideoFormat(outputFormat,

                new File(outputFile));

    }


    @Test

    public void transcodeWithOnlyContainer() {

        reader = ToolFactory.makeReader(SOURCE_FILE);

        outputFormat = new OutputFormat(WIDTH, HEIGHT, BITRATE,

                Container.AVI);

        outputFile = "target/sample.avi";

        VideoConverter writer = new VideoConverter(outputFile, reader,

                outputFormat);

        reader.addListener(writer);

        while (reader.readPacket() == null)

            do {

            } while (false);


        VideoFormatVerifier.verifyVideoFormat(outputFormat,

                new File(outputFile));

    }

}


이제 두 메서드에서 코드가 완전히 동일한 부분을 별도의 메서드로 분리해내자. 다음의 두 부분이 완전히 동일하다.

  • reader를 생성하는 부분 -> @Before를 붙인 setup 메서드로 이동
  • VideoConverter를 생성하고 변환하는 부분 -> testVideoConverter 메서드로 이동

public class VideoConverterTest {


    private static final int WIDTH = 160;

    private static final int HEIGHT = 120;

    private static final int BITRATE = 150;

    private static final String SOURCE_FILE = "src/test/resources/sample.avi";

    private static final String TRANSCODED_FILE = "target/sample.mp4";

    private IMediaReader reader;

    private OutputFormat outputFormat;

    private String outputFile;


    @Before

    public void setup() {

        reader = ToolFactory.makeReader(SOURCE_FILE);

    }


    @Test

    public void transcode() {

        outputFormat = new OutputFormat(WIDTH, HEIGHT, BITRATE, Container.MP4,

                VideoCodec.H264, AudioCodec.AAC);

        outputFile = TRANSCODED_FILE;

        testVideoConverter();

    }


    private void testVideoConverter() {

        VideoConverter writer = new VideoConverter(outputFile, reader,

                outputFormat);

        reader.addListener(writer);

        while (reader.readPacket() == null)

            do {

            } while (false);


        VideoFormatVerifier.verifyVideoFormat(outputFormat,

                new File(outputFile));

    }


    @Test

    public void transcodeWithOnlyContainer() {

        outputFormat = new OutputFormat(WIDTH, HEIGHT, BITRATE, Container.AVI);

        outputFile = "target/sample.avi";

        testVideoConverter();

    }

}


outputFormat과 outputFile을 초기화하는 부분도 비슷하다. 이 부분도 한 번 별도 메서드로 분리해보자.


public class VideoConverterTest {

    ...

    @Test

    public void transcode() {

        initOutput(Container.MP4, VideoCodec.H264, AudioCodec.AAC,

                TRANSCODED_FILE);

        testVideoConverter();

    }


    private void initOutput(Container outputContainer, VideoCodec videoCodec,

            AudioCodec audioCodec, String outputFileName) {

        if (videoCodec == null && audioCodec == null) {

            outputFormat = new OutputFormat(WIDTH, HEIGHT, BITRATE,

                    outputContainer);

        } else {

            outputFormat = new OutputFormat(WIDTH, HEIGHT, BITRATE,

                    outputContainer, videoCodec, audioCodec);

        }

        outputFile = outputFileName;

    }


    private void initOutput(Container outputContainer, String outputFileName) {

        initOutput(outputContainer, null, null, outputFileName);

        outputFile = outputFileName;

    }


    private void testVideoConverter() {

        ...

    }


    @Test

    public void transcodeWithOnlyContainer() {

        initOutput(Container.AVI, "target/sample.avi");

        testVideoConverter();

    }

}


이제 TRANSCODED_FILE 상수는 더 이상 의미가 없다. 이 상수를 사용하지 않도록 돌려 놓자.


지금까지의 작업으로 바뀐 VideoConverterTest는 다음과 같다.


public class VideoConverterTest {


    private static final int WIDTH = 160;

    private static final int HEIGHT = 120;

    private static final int BITRATE = 150;

    private static final String SOURCE_FILE = "src/test/resources/sample.avi";

    private IMediaReader reader;

    private OutputFormat outputFormat;

    private String outputFile;


    @Before

    public void setup() {

        reader = ToolFactory.makeReader(SOURCE_FILE);

    }


    @Test

    public void transcode() {

        initOutput(Container.MP4, VideoCodec.H264, AudioCodec.AAC,

                "target/sample.mp4");

        testVideoConverter();

    }


    @Test

    public void transcodeWithOnlyContainer() {

        initOutput(Container.AVI, "target/sample.avi");

        testVideoConverter();

    }


    private void initOutput(Container outputContainer, String outputFileName) {

        initOutput(outputContainer, null, null, outputFileName);

        outputFile = outputFileName;

    }


    private void initOutput(Container outputContainer, VideoCodec videoCodec,

            AudioCodec audioCodec, String outputFileName) {

        if (videoCodec == null && audioCodec == null) {

            outputFormat = new OutputFormat(WIDTH, HEIGHT, BITRATE,

                    outputContainer);

        } else {

            outputFormat = new OutputFormat(WIDTH, HEIGHT, BITRATE,

                    outputContainer, videoCodec, audioCodec);

        }

        outputFile = outputFileName;

    }


    private void testVideoConverter() {

        VideoConverter writer = new VideoConverter(outputFile, reader,

                outputFormat);

        reader.addListener(writer);

        while (reader.readPacket() == null)

            do {

            } while (false);


        VideoFormatVerifier.verifyVideoFormat(outputFormat,

                new File(outputFile));

    }

}


@Test 메서드가 단순해졌다. 새로운 컨테이너의 추가라든가, 코덱 변환 등을 테스트하고 싶을 때에 간단하게 테스트를 추가할 수 있게 되었다. testVideoConverter() 메서드와 initOutput() 메서드를 조금 더 정리하고 싶은 마음은 있지만, 일단 이 정도까지 코드를 정리하자.




Posted by 최범균 madvirus

댓글을 달아 주세요

  1. 전홍석 2015.12.04 11:08 신고  댓글주소  수정/삭제  댓글쓰기

    안녕하세요
    스프링 4 프로그래밍 입문을 구입하여 열공중인 중년 초보개발자 입니다.
    대용량의 동영상 converter작업할일이 있어 찾아보니 저자님 글이 있어
    학습하여 익히려 합니다.
    염치 불구하고 위 전체 소스를 받아서 도움받을수 있도록 부탁드립니다.


    • 최범균 madvirus 2015.12.04 14:31 신고  댓글주소  수정/삭제

      아,,, 이게 그냥 막 연습삼아서 하던거라서,,,, 코드가 있는지 모르겠네요. 업무용 장비에는 코드가 없네요. 아무래도 내용 보시면 관련 코드가 나오니까, 그 코드를 참조하셔야 할 것 같아요.

TDD 연습은 오늘도 계속된다. 지난 금요일 점심 때 살짝 작업해 본 코드를 GitHub에 커밋하는 걸 깜빡해서 주말에 진행하지 못했다. 음, 그덕에 오늘 밤에 진행한다.


Job에 새로운 모델 추가


오늘은 Job에 모델을 추가해볼 거다. 비디오 변환을 처리하면 변환된 결과 비디오 파일과 썸네일 파일이 생성되는데, 그 파일을 작업 요청자가 지정한 위치에 저장해야 한다. 이 위치는 FTP가 될 수도 있고, HTTP 파일 업로드가 될 수도 있고, 단순히 로컬 파일 시스템이 될 수도 있다. 자, 일단, 구현의 상세함은 뒤로 미루고 다음과 같이 결과 파일이 보관될 위치를 추상화해서 표현할 수 있을 것이다. 


public class Job {


    private Long id;

    private MediaSourceFile mediaSourceFile;

    private DestinationStorage destinationStorage;

    private State state;

    private Exception occurredException;


    public Job(Long id, MediaSourceFile mediaSourceFile,

            DestinationStorage destinationStorage) {

        this.id = id;

        this.mediaSourceFile = mediaSourceFile;

        this.destinationStorage = destinationStorage;

    }


뭔가 이름이 적당한 게 떠오르지 않는다. (이럴 땐 영어공부를 게을리한 필자가 안타깝다.) 하튼, 이름은 DestinationStorage로 가기로 했다. 나중에 더 좋은 이름이 떠오르면 이클립스의 힘을 빌려 쉽게 바꿀 수 있다.


새로운 타입이 출현했다. 아무것도 없으므로 컴파일 에러가 난다. DestinationStorage 타입을 생성해주자. 그리고, Job 클래스의 생성자가 변경되었으므로 테스트 코드에서 Mock을 이용해서 DestinationStorage를 전달해준다.


@RunWith(MockitoJUnitRunner.class)

public class TranscodingServiceImplTest {


    private Long jobId = new Long(1);

    @Mock

    private MediaSourceFile mediaSourceFile;

    @Mock

    private DestinationStorage destinationStorage;


    private Job mockJob;

    ...

    

    @Before

    public void setup() {

        mockJob = new Job(jobId, mediaSourceFile, destinationStorage);

        when(mediaSourceFile.getSourceFile()).thenReturn(mockMultimediaFile);


        transcodingService = new TranscodingServiceImpl(transcoder,

                thumbnailExtractor, createdFileSender, jobResultNotifier,

                jobRepository);


바꿨으니 당연히 뭘 해야 하는지 알 거다. 테스트를 실행해보자. 기존 코드에 문제가 없음을 확인한다.


앞서 Job에 MediaSourceFile을 추가했던 과정을 생각해보자. (기억이 안 나면 http://javacan.tistory.com/221 글을 다시 보자.) 그 과정과 마찬가지로 다음과 같은 작업을 진행할 것이다.

  1. 목적지를 알고 있는 것은 DestinationStorage이므로, Job이 DestinationStorage에 파일을 저장해 달라고 요청하도록 코드를 수정한다.
  2. 이렇게 되면 CreatedFileSaver가 필요없지므로 삭제한다.
자 일단 Job이 DestinationStorage에게 파일을 저장해달라고 요청하도록 코드를 수정한다.

public class Job {

    private Long id;
    private MediaSourceFile mediaSourceFile;
    private DestinationStorage destinationStorage;
    ...
    public void transcode(Transcoder transcoder,
            ThumbnailExtractor thumbnailExtractor,
            CreatedFileSaver createdFileSaver,
            JobResultNotifier jobResultNotifier) {
        try {
            ...
            changeState(Job.State.STORING);
            storeCreatedFilesToStorage(multimediaFiles, thumbnails,
                    createdFileSaver);
            changeState(Job.State.NOTIFYING);
            notifyJobResultToRequester(jobResultNotifier);
            changeState(Job.State.COMPLETED);
        } catch (RuntimeException ex) {
            exceptionOccurred(ex);
            throw ex;
        }
    }

    private void storeCreatedFilesToStorage(List<File> multimediaFiles,
            List<File> thumbnails, CreatedFileSaver createdFileSaver) {
        destinationStorage.save(multimediaFiles, thumbnails);
        createdFileSaver.store(multimediaFiles, thumbnails, id);
    }

createdFileSaver를 호출하는 코드는 필요없으므로 삭제한다. destinationStorage는 save 메서드가 없어서 컴파일 에러가 난다. 컴파일 에러가 나지 않도록 save() 메서드를 추가해 준다. 이클립스의 만능키 Ctrl+1을 누루면 바로 추가되므로 쉽게 컴파일 에러를 제거할 수 있다.


public interface DestinationStorage {


    void save(List<File> multimediaFiles, List<File> thumbnails);


}


오케이 일단 뭔가 바꿨다. 테스트 코드를 돌려보자. .... 빨간불. 통과에 실패했다. 원인은 아래와 같다.


Wanted but not invoked:

createdFileSender.store([], [], 1);

-> at org...TranscodingServiceImplTest.verifyCollaboration(TranscodingServiceImplTest.java:117)

at org...TranscodingServiceImplTest.verifyCollaboration(TranscodingServiceImplTest.java:117)

...


실패 원인은 createdFileSender.store()가 호출되지 않은 것 때문이다. 앞서 Job 클래스의 storeCreatedFilesToStorage() 메서드에서 createFileSaver 객체의 store() 메서드를 호출하는 부분을 삭제했기 때문에 발생한 것이다. 이제 이 부분은 createdFileSender가 아니라 destinationStorage.save() 메서드가 호출되었는지 검증하는 코드로 바꿔야 한다. 그래서, 아래와 같이 테스트 코드에서 협업 객체들의 메서드 호출 여부를 검증하는 부분을 변경하였다.


    private void verifyCollaboration(VerifyOption verifyOption) {

        if (verifyOption.transcoderNever)

            verify(transcoder, never()).transcode(any(File.class), anyLong());

        else

            verify(transcoder, only()).transcode(mockMultimediaFile, jobId);


        if (verifyOption.thumbnailExtractorNever)

            verify(thumbnailExtractor, never()).extract(any(File.class),

                    anyLong());

        else

            verify(thumbnailExtractor, only()).extract(mockMultimediaFile,

                    jobId);


        if (verifyOption.destinationStorageNever) // createdFileSaverNever를 이름 변경

            verify(destinationStorage, never()).save(anyListOf(File.class),

                    anyListOf(File.class));

        else

            verify(destinationStorage, only()).save(mockMultimediaFiles,

                    mockThumbnails);


        if (verifyOption.createdFileSenderNever)

            verify(createdFileSender, never()).store(anyListOf(File.class),

                    anyListOf(File.class), anyLong());

        else

            verify(createdFileSender, only()).store(mockMultimediaFiles,

                    mockThumbnails, jobId);


        if (verifyOption.jobResultNotifierNever)

            verify(jobResultNotifier, never()).notifyToRequester(jobId);

        else

            verify(jobResultNotifier, only()).notifyToRequester(jobId);

    }


destinationStorage Mock 객체의 save() 메서드가 호출되는 지의 여부로 변경했으므로,  다시 테스트를 실행해 보자. 6개의 테스트 메서드 중에서 1개가 실패가 난다. 실패한 테스트는 아래 메서드이다.


    @Test

    public void transcodeFailBecauseExceptionOccuredAtCreatedFileSender() {

        doThrow(mockException).when(createdFileSender).store(

                mockMultimediaFiles, mockThumbnails, jobId);


        executeFailingTranscodeAndAssertFail(Job.State.STORING);


        VerifyOption verifyOption = new VerifyOption();

        verifyOption.jobResultNotifierNever = true;


        verifyCollaboration(verifyOption);

    }


    private void executeFailingTranscodeAndAssertFail(State expectedLastState) {

        try {

            transcodingService.transcode(jobId);

            fail("발생해야 함"); // 익셉션이 발생하지 않아서 실패

        } catch (Exception ex) {

            assertSame(mockException, ex);

        }

        ...

    }



검증하는 코드에서는 익셉션이 발생해야 통과되는데, Job 객체를 수정하는 바람에 더 이상 createdFileSaver 객체가 호출되지 않게 되었다. 위 코드는 createdFileSaver 대신 새롭게 추가된 모델인 destinationStorage의 save() 메서드에서 익셉션을 발생하도록 수정해주면 통과 시킬 수 있을 것 같다. 메서드 이름도 함께 변경해 주자.


    @Test

    public void transcodeFailBecauseExceptionOccuredAtDestinationStorage() {

        doThrow(mockException).when(destinationStorage).save(

                mockMultimediaFiles, mockThumbnails);


        executeFailingTranscodeAndAssertFail(Job.State.STORING);


        VerifyOption verifyOption = new VerifyOption();

        verifyOption.jobResultNotifierNever = true;


        verifyCollaboration(verifyOption);

    }


다시 테스트 코드 실행. 모두 녹색. 통과다.


이제 필요 없어진 코드가 있다. 바로 CreatedFileSaver 이다. 이제 이 타입을 사용하는 코드를 모두 과감하게 삭제한다. (주석으로 막아서 테스트를 실행하고 통과되면 지우는 방법을 점진적으로 삭제한다.)


Job 클래스의 작은 리팩토링


필자와 개인적으로 친분이 있는 분께서 http://javacan.tistory.com/221#comment9371876와 같은 의견을 주셨다. 그 중 첫 번째 Job 클래스 부분에 대한 피드백은 아래 코드에서 구조적인 반복이 나타난다는 점이었다. 상태를 변경하고 작업 실행하고, 또 상태를 변경하고 작업을 실행하는 구조를 갖고 있다.


    public void transcode(Transcoder transcoder,

            ThumbnailExtractor thumbnailExtractor,

            JobResultNotifier jobResultNotifier) {

        try {

            changeState(Job.State.MEDIASOURCECOPYING);

            File multimediaFile = copyMultimediaSourceToLocal();

            changeState(Job.State.TRANSCODING);

            List<File> multimediaFiles = transcode(multimediaFile, transcoder);

            changeState(Job.State.EXTRACTINGTHUMBNAIL);

            List<File> thumbnails = extractThumbnail(multimediaFile,

                    thumbnailExtractor);

            changeState(Job.State.STORING);

            storeCreatedFilesToStorage(multimediaFiles, thumbnails);

            changeState(Job.State.NOTIFYING);

            notifyJobResultToRequester(jobResultNotifier);

            changeState(Job.State.COMPLETED);

        } catch (RuntimeException ex) {

            exceptionOccurred(ex);

            throw ex;

        }

    }


우선, 상태 변경과 작업 실행 순서 관리를 transcode()가 처리하고 있는데, 이는 transcode()의 역할을 잘못 잡은 것 같다. 실제 상태의 변화는 각각의 메서드에서 처리하고 transcode() 메서드는 순서대로 작업이 실행되도록 하는 것이 적당한 역할을 수행하는 것으로 생각된다. 그래서, 상태 변경은 각 메서드에서 처리하도록 수정했다. 그리고 완료 상태로 변경하는 부분은 completed()라는 메서드를 만들어서 그 메서드에서 처리하도록 했다.


public class Job {

    ...

    public void transcode(Transcoder transcoder,

            ThumbnailExtractor thumbnailExtractor,

            JobResultNotifier jobResultNotifier) {

        try {

            File multimediaFile = copyMultimediaSourceToLocal();

            List<File> multimediaFiles = transcode(multimediaFile, transcoder);

            List<File> thumbnails = extractThumbnail(multimediaFile,

                    thumbnailExtractor);

            storeCreatedFilesToStorage(multimediaFiles, thumbnails);

            notifyJobResultToRequester(jobResultNotifier);

            completed();

        } catch (RuntimeException ex) {

            exceptionOccurred(ex);

            throw ex;

        }

    }


    private File copyMultimediaSourceToLocal() {

        changeState(Job.State.MEDIASOURCECOPYING);

        return mediaSourceFile.getSourceFile();

    }


    private List<File> transcode(File multimediaFile, Transcoder transcoder) {

        changeState(Job.State.TRANSCODING);

        return transcoder.transcode(multimediaFile, id);

    }


    private List<File> extractThumbnail(File multimediaFile,

            ThumbnailExtractor thumbnailExtractor) {

        changeState(Job.State.EXTRACTINGTHUMBNAIL);

        return thumbnailExtractor.extract(multimediaFile, id);

    }


    private void storeCreatedFilesToStorage(List<File> multimediaFiles,

            List<File> thumbnails) {

        changeState(Job.State.STORING);

        destinationStorage.save(multimediaFiles, thumbnails);

    }


    private void notifyJobResultToRequester(JobResultNotifier jobResultNotifier) {

        changeState(Job.State.NOTIFYING);

        jobResultNotifier.notifyToRequester(id);

    }


    private void completed() {

        changeState(Job.State.COMPLETED);

    }


}


이제 Job은 작업을 순서대로 실행하는 한 가지 일만을 책임지게 되었으므로 정리가 된 느낌이다. 하지만, 여전히 각각의 메서드들은 상태변경-작업실행 요청의 반복적 구조를 갖고 있다. 이런 구조를 제거할 수 있는 다른 방법이 있을 것도 같다. 예를 들어, 각 단계를 State 같은 걸로 추상화하고 State가 지정된 순서대로 전이되도록 StateChain 같은 걸 만들어서 지정한 순서대로 각 단계의 작업이 실행되도록 할 수 있을 것 같다. 하지만, 이런 구조를 갖는 것은 현재로서는 유연함에서 오는 이점보다 복잡함에서 오는 단점이 더 많은 것 같다.


그래서, 위의 구조적 반복이 여전히 남아 있긴 하지만, 일단 이쯤에서 정리한다. 혹시 Job의 하위 타입이 생긴다거나 새로운 상태가 생긴다거나 실행 순서의 변경이 발생하게 된다면, 그 때 가서 다시 리팩토링을 해 보도록 하자. (이 정도로 지인분의 요구가 충족될지는........ 이 정도로 만족하시지요!)


테스트의 VerifyOption에 기능 주기


지인분께서 http://javacan.tistory.com/221#comment9371876에 남기신 두 번째 피드백은 verifyOption이 검증과 관련된 정보를 갖고 있으니, VerifyOption을 단순 데이터 구조가 아닌 기능을 제공하는 객체로 만들어보자는 내용이다. 음, 이 부분은 고민이 좀 된다. 테스트 코드에서 사용되는 데이터인데 그것까지 객체로 만들 필요가 있을까 하는 고민이었다. 그래도 변경하는게 어렵지 않으니 바꿔보자.


뭔가 협업 객체가 올바르게 호출되는지 여부를 확인하는 코드는 아래와 같은 방식으로 사용된다.


    @Test

    public void transcodeFailBecauseExceptionOccuredAtTranscoder() {

        when(transcoder.transcode(mockMultimediaFile, jobId)).thenThrow(

                mockException);


        executeFailingTranscodeAndAssertFail(Job.State.TRANSCODING);


        VerifyOption verifyOption = new VerifyOption();

        verifyOption.thumbnailExtractorNever = true;

        verifyOption.destinationStorageNever = true;

        verifyOption.jobResultNotifierNever = true;


        verifyCollaboration(verifyOption);

    }


    private void verifyCollaboration(VerifyOption verifyOption) {

        if (verifyOption.transcoderNever)

            verify(transcoder, never()).transcode(any(File.class), anyLong());

        else

            verify(transcoder, only()).transcode(mockMultimediaFile, jobId);


        if (verifyOption.thumbnailExtractorNever)

            verify(thumbnailExtractor, never()).extract(any(File.class),

                    anyLong());

        else

            verify(thumbnailExtractor, only()).extract(mockMultimediaFile,

                    jobId);


        if (verifyOption.destinationStorageNever)

            verify(destinationStorage, never()).save(anyListOf(File.class),

                    anyListOf(File.class));

        else

            verify(destinationStorage, only()).save(mockMultimediaFiles,

                    mockThumbnails);


        if (verifyOption.jobResultNotifierNever)

            verify(jobResultNotifier, never()).notifyToRequester(jobId);

        else

            verify(jobResultNotifier, only()).notifyToRequester(jobId);

    }


    public class VerifyOption {

        public boolean transcoderNever;

        public boolean thumbnailExtractorNever;

        public boolean destinationStorageNever;

        public boolean jobResultNotifierNever;

    }

}


우선, verifyCollaboration 메서드를 VerifyOption의 public 메서드로 이동시킨다. 물론, 옮기면서 this를 사용하도록 변경한다. 그리고, 각 테스트 메서드에서는 VerifyOption의 verifyCollaboration() 메서드를 호출하도록 변경한다. 그럼 코드는 아래와 같은 바뀐다.


    @Test

    public void transcodeFailBecauseExceptionOccuredAtDestinationStorage() {

        doThrow(mockException).when(destinationStorage).save(

                mockMultimediaFiles, mockThumbnails);


        executeFailingTranscodeAndAssertFail(Job.State.STORING);


        VerifyOption verifyOption = new VerifyOption();

        verifyOption.jobResultNotifierNever = true;


        verifyOption.verifyCollaboration();

    }


    public class VerifyOption {

        public boolean transcoderNever;

        public boolean thumbnailExtractorNever;

        public boolean destinationStorageNever;

        public boolean jobResultNotifierNever;


        public void verifyCollaboration() {

            if (this.transcoderNever)

                verify(transcoder, never()).transcode(any(File.class),

                        anyLong());

            else

                verify(transcoder, only()).transcode(mockMultimediaFile, jobId);


            if (this.thumbnailExtractorNever)

                verify(thumbnailExtractor, never()).extract(any(File.class),

                        anyLong());

            else

                verify(thumbnailExtractor, only()).extract(mockMultimediaFile,

                        jobId);


            if (this.destinationStorageNever)

                verify(destinationStorage, never()).save(anyListOf(File.class),

                        anyListOf(File.class));

            else

                verify(destinationStorage, only()).save(mockMultimediaFiles,

                        mockThumbnails);


            if (this.jobResultNotifierNever)

                verify(jobResultNotifier, never()).notifyToRequester(jobId);

            else

                verify(jobResultNotifier, only()).notifyToRequester(jobId);

        }

    }


바꿨으니 테스트 실행이다. 녹색! 통과되었다.


이름이 마음에 안 든다. 이름을 바꾸자. VerifyOption은 더 이상 단순 옵션이 아니다. 실제 검증을 수행하는 역할을 갖게 되었으므로, CollaborationVerifier와 같이 역할에 맞는 이름을 부여한다. verifyCollaboration() 메서드는 순히 verify()로 바꿔도 될 것 같다. 웃, verify()로 이름을 변경하려고 했더니 메서드 내부에서 사용한 Mockito의 verify()가 컴파일 에러가 난다. 음, 다른 이름을 찾아보자. 음.. 딱히 이름이 안 떠오른다. 일단 verifyCollaboration()으로 놔두자.


    @Test

    public void transcodeFailBecauseExceptionOccuredAtDestinationStorage() {

        doThrow(mockException).when(destinationStorage).save(

                mockMultimediaFiles, mockThumbnails);


        executeFailingTranscodeAndAssertFail(Job.State.STORING);


        CollaborationVerifier colVerifier = new CollaborationVerifier();

        colVerifier.jobResultNotifierNever = true;


        colVerifier.verifyCollaboration();

     }


    public class CollaborationVerifier {

        public boolean transcoderNever;

        public boolean thumbnailExtractorNever;

        public boolean destinationStorageNever;

        public boolean jobResultNotifierNever;


        public void verifyCollaboration() {

            if (this.transcoderNever)

                verify(transcoder, never()).transcode(any(File.class),

                        anyLong());

            else

                verify(transcoder, only()).transcode(mockMultimediaFile, jobId);


            ...

        }


    }

}


이름을 바꿨으니 다시 테스트! 녹색! 통과되었다.


CollaborationVerifier와 관련된 다음 고민은 아래와 같다.

  • transcoderNever, thumbnailExtractorNever 등의 필드를 private으로 바꿔야 하나?
  • verifyCollaboration() 메서드에서 접근하는 transcoder 등의 객체를 필드로 정의해야 하나?
  • CollaborationVerifier 클래스를 private으로 바꿔야 하나?
음, 아주 짧게만 고민해 봤는데 (슬슬 졸리기 시작한다), 현재로서 CollaborationVerifier 클래스는 TranscodingServiceImplTest 내부에서만 사용되므로, 필드가 public이어도 상관없을 것 같고 TranscodingServiceImplTest 클래스의 필드에 직접 접근해도 문제가 없다. 일단 지금은 CollaborationVerifier를 private으로 하는 것만으로 충분할 것 같다.


Posted by 최범균 madvirus

댓글을 달아 주세요

연습 4-1(http://javacan.tistory.com/220)에 이어 계속해서 Job 부분을 만들어 나가자. 


새로운 모델의 출현: MediaSourceFile


Job은 원본 미디어 파일 정보를 갖고 있어야 한다. 미디어 원본 파일을 MediaSourceFile로 추상화하고, 아래와 같이 이 타입을 필드로 갖도록 코드를 만들었다.


public class Job {

    ...

   private MediaSourceFile mediaSourceFile;

   ...

   public Job(Long id, MediaSourceFile mediaSourceFile) {

       this.id = id;

       this.mediaSourceFile = mediaSourceFile;

   }

   ...

}


빨간 줄이 나오니까 컴파일 에러가 나지 않도록 변경해 주자. 먼저 MediaSourceFile을 생성할 것이다. 아직 MediaSourceFile의 정확한 구현을 알 수 없으므로 일단 인터페이스로 만든다. 인터페이스만 있고 메서드는 없다.


// 아직 기능 없음

public interface MediaSourceFile {


}


Job 클래스의 컴파일 에러는 사라졌는데, 대신 TranscodingServiceImplTest 코드에서 컴파일 에러가 발생한다.


@RunWith(MockitoJUnitRunner.class)

public class TranscodingServiceImplTest {


    private Long jobId = new Long(1);

    private Job mockJob = new Job(jobId);


Job의 생성자가 변경되어서 컴파일 에러가 발생하고 있다. Job 객체를 생성하려면 MediaSourceFile 구현체가 필요하다. 일단, MediaSourceFile에 대한 Mock 객체를 만들고, 그 Mock 객체를 받도록 변경한다. Job 객체를 생성하는 위치는 setup() 메서드로 이동시킨다.


@RunWith(MockitoJUnitRunner.class)

public class TranscodingServiceImplTest {


    private Long jobId = new Long(1);

    @Mock

    private MediaSourceFile mediaSourceFile;

    

    private Job mockJob;

    ...


    @Before

    public void setup() {

        mockJob = new Job(jobId, mediaSourceFile);

        ...

뭔가 바꾸면 해야 할 게 있다. 그렇다, 바로 테스트 실행이다. 테스트 실행 ... 녹색바! 통과다.


MediaSourceFile은 미디어 파일을 복사하는 것과 관련되었으므로, Job 클래스의 copyMultimediaSourceToLocal() 메서드를 보자.


public class Job {


    private Long id;

    private MediaSourceFile mediaSourceFile;

    ...


    public Job(Long id, MediaSourceFile mediaSourceFile) {

        this.id = id;

        this.mediaSourceFile = mediaSourceFile;

    }


    public void transcode(MediaSourceCopier mediaSourceCopier,

            Transcoder transcoder, ThumbnailExtractor thumbnailExtractor,

            CreatedFileSaver createdFileSaver,

            JobResultNotifier jobResultNotifier) {

        try {

            changeState(Job.State.MEDIASOURCECOPYING);

            File multimediaFile = copyMultimediaSourceToLocal(mediaSourceCopier);

            ...

        } catch (RuntimeException ex) {

            exceptionOccurred(ex);

            throw ex;

        }

    }


    private File copyMultimediaSourceToLocal(MediaSourceCopier mediaSourceCopier) {

        return mediaSourceCopier.copy(id); // mediaSourceFile을 줘야 하나?

    }



copyMultimediaSourceToLocal() 메서드는 mediaSourceCopier 객체의 copy() 메서드를 호출할 때 id 필드를 값으로 주고 있다. 읽어올 미디어 원본 파일 정보가 mediaSourceFile 필드에 있으므로, id 대신 mediaSourceFile을 건내주면 될 것 같다.


음... 잠깐! MediaSourceFile에 다음과 같은 메서드를 정의하면 어떨까?


public interface MediaSourceFile {

    public File getSourceFile();

}


또는


public interface MediaSourceFile {

    public void writeTo(File file);

}


두 가지 중 어떤 것을 선택해도 mediaSourceCopier의 필요성이 사라진다. 오호~ 그렇군. 조금 고민하다가 파일이 어디에 보관될지 여부는 Job이 스스로 결정하는 게 좋을 듯 싶어, getSourceFile() 메서드를 사용하기로  결정했다. 결정했으니 바꿔보자.

  • MediaSourceFile 인터페이스에 getSourceFile() 메서드를 추가한다.
  • copyMultimediaSourceToLocal() 메서드가 mediaSourceFile.getSourceFile()를 사용하도록 변경한다.
  • 이렇게 바꾸면, copyMultimediaSourceToLocal() 메서드가 MediaSourceCopier를 필요로 하지 않는다. 그러니 파라미터에서 mediaSourceCopier를 제거한다.
Job에 적용한 결과는 아래와 같다.

public class Job {

    public void transcode(MediaSourceCopier mediaSourceCopier,
            Transcoder transcoder, ThumbnailExtractor thumbnailExtractor,
            CreatedFileSaver createdFileSaver,
            JobResultNotifier jobResultNotifier) {
        try {
            changeState(Job.State.MEDIASOURCECOPYING);
            File multimediaFile = copyMultimediaSourceToLocal(); // mediaSourceCopier 빠짐
            ...
        } catch (RuntimeException ex) {
            exceptionOccurred(ex);
            throw ex;
        }
    }

    private File copyMultimediaSourceToLocal() { // mediaSourceCopier 파라미터 제거
        return mediaSourceFile.getSourceFile();
    }

테스트를 실행해 보자. 앗! 모두 실패다. 실패나는 이유를 봤더니 mediaSourceCopier Mock 객체가 더 이상 호출되지 않아서 발생하는 문제였다. 단위 테스트 코드에서 mediaSourceCopier를 verify 하는 부분을 삭제한다.

    private void verifyCollaboration(VerifyOption verifyOption) {
        verify(mediaSourceCopier, only()).copy(jobId);

        if (verifyOption.transcoderNever)
            verify(transcoder, never()).transcode(any(File.class), anyLong());
        else
            verify(transcoder, only()).transcode(mockMultimediaFile, jobId);

        ...
    }

다시 실행해 보자. 앗! 그래도 모두 실패다. 위 코드에서 trancoder.trancode() 메서드가 호출되었는지 여부를 확인하는 부분에서 실패가 난다. trancoder.trancode()의 호출 여부를 검증할 때 사용한 파라미터는 mockMultimediaFile인데 실제로 Job 객체에서 mediaSourceFile.getSourceFile()은 null을 리턴한다. 그래서 검증에 실패하였다.

통과하게 하는 방법은 간단하다. 테스트 코드의 setup() 메서드에서 mediaSourceFile을 위해 생성한 Mock 객체의 getSourceFile() 메서드가 mockMultimediaFile을 리턴하도록 설정해주면 된다.

@RunWith(MockitoJUnitRunner.class)
public class TranscodingServiceImplTest {
    ...
@Before
public void setup() {
mockJob = new Job(jobId, mediaSourceFile);
when(mediaSourceFile.getSourceFile()).thenReturn(mockMultimediaFile);
transcodingService = new TranscodingServiceImpl(mediaSourceCopier,
transcoder, thumbnailExtractor, createdFileSender,
jobResultNotifier, jobRepository);

...

다시 테스트. 6개의 테스트 메서드 중에 한 개가 실패했다. 그 메서드는 다음과 같다.

    @Test
    public void transcodeFailBecauseExceptionOccuredAtMediaSourceCopier() {
        when(mediaSourceCopier.copy(jobId)).thenThrow(mockException);

        executeFailingTranscodeAndAssertFail(Job.State.MEDIASOURCECOPYING);

        VerifyOption verifyOption = new VerifyOption();
        verifyOption.transcoderNever = true;
        verifyOption.thumbnailExtractorNever = true;
        verifyOption.createdFileSenderNever = true;
        verifyOption.jobResultNotifierNever = true;

        verifyCollaboration(verifyOption);
    }

MediaSourceCopier는 더 이상 호출되지 않기 때문에 mediaSourceCopier가 익셉션을 발생시키도록 만들고, 실제로 그렇게 되는지 확인하는 건 의미가 없다. mediaSourceCopier 대신 mediaSourceFile가 익셉션을 발생시키는 경우를 테스트 하도록 수정하는 것이 맞다. 그래서 아래와 같이 테스트를 바꾼다.

    @Test
    public void transcodeFailBecauseExceptionOccuredAtMediaSourceFile() {
        when(mediaSourceFile.getSourceFile()).thenThrow(mockException);

        executeFailingTranscodeAndAssertFail(Job.State.MEDIASOURCECOPYING);

        VerifyOption verifyOption = new VerifyOption();
        verifyOption.transcoderNever = true;
        verifyOption.thumbnailExtractorNever = true;
        verifyOption.createdFileSenderNever = true;
        verifyOption.jobResultNotifierNever = true;

        verifyCollaboration(verifyOption);
    }

테스트 실행,,, 녹색! 모두 통과다. 이로서 Job과 관련된 새로운 모델인 MediaSourceFile을 추가하고 MediaSourceFile이 원본으로부터 파일을 가져오도록 설계를 변경하는데 성공했다.


MediaSourceCopier는 이제 필요 없어요!


MediaSourceFile의 등장으로 MediaSourceCopier는 필요 없어졌다. 이제 과감하게 MediaSourceCopier를 소스 코드에서 제거할 차례이다. MediaSourceCopier 타입을 없애고, MediaSourceCopier 타입의 필드, 초기화 코드, 파라미터 등을 모두 제거한다.


// Job 클래스에서 제거

public class Job {

    ....

    public void transcode(MediaSourceCopier mediaSourceCopier,

            Transcoder transcoder, ThumbnailExtractor thumbnailExtractor,

            CreatedFileSaver createdFileSaver,

            JobResultNotifier jobResultNotifier) {

        try {



// TranscodingServceImpl 클래스에서 제거

public class TranscodingServiceImpl implements TranscodingService {

    private MediaSourceCopier mediaSourceCopier;

    private Transcoder transcoder;

    ...


    public TranscodingServiceImpl(MediaSourceCopier mediaSourceCopier,

            Transcoder transcoder, ThumbnailExtractor thumbnailExtractor,

            CreatedFileSaver createdFileSaver,

            JobResultNotifier jobResultNotifier,

            JobRepository jobRepository) {

        this.mediaSourceCopier = mediaSourceCopier;

        this.transcoder = transcoder;

        ...

    }


    @Override

    public void transcode(Long jobId) {

        Job job = jobRepository.findById(jobId);

        job.transcode(mediaSourceCopier, transcoder, thumbnailExtractor,

                createdFileSaver, jobResultNotifier);

    }



// 테스트 코드에서 제거

@RunWith(MockitoJUnitRunner.class)

public class TranscodingServiceImplTest {

    ...

    @Mock

    private MediaSourceCopier mediaSourceCopier;

    @Mock

    private Transcoder transcoder;

    ...


    @Before

    public void setup() {

        mockJob = new Job(jobId, mediaSourceFile);

        when(mediaSourceFile.getSourceFile()).thenReturn(mockMultimediaFile);

        

        transcodingService = new TranscodingServiceImpl(mediaSourceCopier,

                transcoder, thumbnailExtractor, createdFileSender,

                jobResultNotifier, jobRepository);


        when(jobRepository.findById(jobId)).thenReturn(mockJob);

        when(mediaSourceCopier.copy(jobId)).thenReturn(mockMultimediaFile);

        ...

    }


코드를 수정했으니까, 다음으로 할 작업은 테스트 실행이다. 테스트 실행~.. 녹색! 모두 통과다.


테스트 코드를 이용한 안정적인 리팩토링


TDD를 이용해서 TranscodingServce의 구현부터 Job의 일부 구현까지 진행되었다. 한 번 정리해 보자.


최초의 TDD 결과물은 아래와 같았다.



하지만, 결과물이 마음에 안 들었고 그래서 Job으로 기능을 옮겼다. 물론, 테스트는 그대로 유지하면서. 그래서 다음과 같이 Job이 출현하고 불필요해진 두 개의 타입이 사라졌다.



그리고, 새로운 도메인 모델인 MediaSourceFile을 추가하는 과정에서 일부 로직이 MediaSourceFile로 이동했고, 이 과정에서 MediaSourceCopier 타입이 또 사라졌다.



그리고, 무엇보다도 중요한 건, 테스트 코드를 통해서 이러한 변화 과정을 안정적으로 진행했다는 점이다.



Posted by 최범균 madvirus

댓글을 달아 주세요

  1. 백명석 2012.10.26 20:11 신고  댓글주소  수정/삭제  댓글쓰기

    verifyCollaboration(verifyOption); -> verifyOption.verify();가 어떨까 ?
    글고
    dosomething(); changeState(); 의 구조적 반복 제거...
    ㅎㅎ
    재밌는데. 근데 이런걸 github에서 했으면 정말 재밌었을 듯.
    블로그에 댓글 놀이로는...

http://javacan.tistory.com/218#comment9368196 댓글을 보자. 사실 필자가 하고 만들고 싶었던 코드는 지금까지 작성했던 그런 코드가 아니었다. 그래 마음에 안 든다. 마음에 안 드는 것들은 다음과 같다.

  • TranscodingServiceImp이 Job의 상태 변경을 관리한다.
    • Job이 자기의 상태 변화를 직접 관리하는 것이 아니라 남이 변경을 한다.
    • Job이 작업 순서를 관리하는 게 아니라 남이 관리해 준다.
      • 향후 작업 순서가 바뀌거나 Job의 종류에 따라 작업 순서가 달려져야 할 경우, TranscodingServiceImpl에 변화가 생긴다. 뭔가 Job의 다형성이 안 될 것 같은 느낌이 든다.
이런 불만때문에, 리팩토링을 하기로 했다. 오, 리팩토링! 한 번 바꿔보자. 두려워할 필요는 없다. 우리에겐 리팩토링을 진행할 수 있도록 안전판 역할을 하는 테스트 코드가 있지 않은가!

TranscodingServiceImpl의 기능을 Job으로 이동시키기

우선 TranscodingServiceImpl의 transcode() 메서드를 Job에 복사한다. 물론, 관련 private 메서드도 복사한다.

public class Job {
    ...
    // TranscodingServiceImpl 클래스의 transcode 메서드를 복사
    public void transcode(Long jobId) {
        changeJobState(jobId, Job.State.MEDIASOURCECOPYING);
        File multimediaFile = copyMultimediaSourceToLocal(jobId);
        changeJobState(jobId, Job.State.TRANSCODING);
        List<File> multimediaFiles = transcode(multimediaFile, jobId);
        changeJobState(jobId, Job.State.EXTRACTINGTHUMBNAIL);
        List<File> thumbnails = extractThumbnail(multimediaFile, jobId);
        changeJobState(jobId, Job.State.STORING);
        storeCreatedFilesToStorage(multimediaFiles, thumbnails, jobId);
        changeJobState(jobId, Job.State.NOTIFYING);
        notifyJobResultToRequester(jobId);
        changeJobState(jobId, Job.State.COMPLETED);
    }

    private void changeJobState(Long jobId, State newJobState) {
        jobStateChanger.chageJobState(jobId, newJobState);
    }

    private File copyMultimediaSourceToLocal(Long jobId) {
        try {
            return mediaSourceCopier.copy(jobId);
        } catch (RuntimeException ex) {
            transcodingExceptionHandler.notifyToJob(jobId, ex);
            throw ex;
        }
    }

    private List<File> transcode(File multimediaFile, Long jobId) {
        try {
            return transcoder.transcode(multimediaFile, jobId);
        } catch (RuntimeException ex) {
            transcodingExceptionHandler.notifyToJob(jobId, ex);
            throw ex;
        }
    }

    private List<File> extractThumbnail(File multimediaFile, Long jobId) {
        try {
            return thumbnailExtractor.extract(multimediaFile, jobId);
        } catch (RuntimeException ex) {
            transcodingExceptionHandler.notifyToJob(jobId, ex);
            throw ex;
        }
    }

    private void storeCreatedFilesToStorage(List<File> multimediaFiles,
            List<File> thumbnails, Long jobId) {
        try {
            createdFileSaver.store(multimediaFiles, thumbnails, jobId);
        } catch (RuntimeException ex) {
            transcodingExceptionHandler.notifyToJob(jobId, ex);
            throw ex;
        }
    }

    private void notifyJobResultToRequester(Long jobId) {
        try {
            jobResultNotifier.notifyToRequester(jobId);
        } catch (RuntimeException ex) {
            transcodingExceptionHandler.notifyToJob(jobId, ex);
            throw ex;
        }
    }

}

컴파일 에러가 난다. 일단 컴파일 에러부터 없애자. Job의 transcode() 메서드가 필요한 협업 객체를 전달받도록 하고, 전달받은 협업 객체를 다시 각각의 private 메서드 호출시 전달하는 방법으로 컴파일 에러를 없앨 것이다. 음.... 아니다. 필자가 생각한 결과가 있는데 컴파일 에러부터 먼저 없애고 단계적으로 수행하면 뭔가 많이 돌아간다. 그래서 몇 가지를 한 번에 처리할 것이다. 그 내용은 아래와 같다.
  • transcode() 메서드가 jobId를 전달받는데, 그럴 필요가 없다. 왜냐면, jobId를 알고 있는 건 Job이기 때문이다. Job에 이를 위해 id 필드를 추가한다. 그리고, 각 메서드는 jobId 파라미터 대신 id 필드를 사용하도록 바꾼다.
  • transcodingExceptionHandler가 필요했던 이유는 TranscodingServiceImpl이 에러 사실을 Job에 알리기 위함이었다. 위 코드가 Job 내부에서 실행되므로 이제 에러가 발생하면 Job이 알게 된다. 따라서, try-catch 블록은 transcode() 메서드에만 위치하면 되며, 나머지 다른 메서드의 try-catch는 필요 없다. 또한, transcodingExceptionHandler도 필요없다.
  • jobStateChanger가 필요했던 이유는 TranscodingServceImpl에서 job의 상태를 변경하기 위함이었다. 이제 Job 객체가 실행 흐름을 제어하므로 스스로 상태를 변경할 수 있게 되었다. 따라서, jobStateChanger가 필요 없다.
  • 또한, changeState() 메서드가 있으므로 changeJobState() 메서드는 더 이상 필요 없다. trancode() 메서드 내부에서 changeJobState() 메서드 대신에 changeState() 메서드를 호출하도록 변경하자.
  • mediaSourceCopier, jobResultNotifier 등의 협업 객체는 transcode() 메서드를 통해서 전달받도록 하고, transcode() 메서드가 다시 알맞은 협업 객체를 전달해주는 방식으로 바꾼다.
다음은 위 내용을 적용한 Job 클래스 코드이다.

public class Job {

    public static enum State {
        MEDIASOURCECOPYING, TRANSCODING, EXTRACTINGTHUMBNAIL, STORING, NOTIFYING, COMPLETED
    }

    private Long id;
    private State state;
    ...
    public void transcode(MediaSourceCopier mediaSourceCopier,
            Transcoder transcoder, ThumbnailExtractor thumbnailExtractor,
            CreatedFileSaver createdFileSaver,
            JobResultNotifier jobResultNotifier) {
        try {
            changeState(Job.State.MEDIASOURCECOPYING);
            File multimediaFile = copyMultimediaSourceToLocal(mediaSourceCopier);
            changeState(Job.State.TRANSCODING);
            List<File> multimediaFiles = transcode(multimediaFile, transcoder);
            changeState(Job.State.EXTRACTINGTHUMBNAIL);
            List<File> thumbnails = extractThumbnail(multimediaFile,
                    thumbnailExtractor);
            changeState(Job.State.STORING);
            storeCreatedFilesToStorage(multimediaFiles, thumbnails,
                    createdFileSaver);
            changeState(Job.State.NOTIFYING);
            notifyJobResultToRequester(jobResultNotifier);
            changeState(Job.State.COMPLETED);
        } catch (RuntimeException ex) { // 익셉션 처리를 각 메서드가 아닌 한 곳에서 수행
            exceptionOccurred(ex);
        }
    }

    private File copyMultimediaSourceToLocal(MediaSourceCopier mediaSourceCopier) {
        return mediaSourceCopier.copy(id);
    }

    private List<File> transcode(File multimediaFile, Transcoder transcoder) {
        return transcoder.transcode(multimediaFile, id);
    }

    private List<File> extractThumbnail(File multimediaFile,
            ThumbnailExtractor thumbnailExtractor) {
        return thumbnailExtractor.extract(multimediaFile, id);
    }

    private void storeCreatedFilesToStorage(List<File> multimediaFiles,
            List<File> thumbnails, CreatedFileSaver createdFileSaver) {
        createdFileSaver.store(multimediaFiles, thumbnails, id);
    }

    private void notifyJobResultToRequester(JobResultNotifier jobResultNotifier) {
        jobResultNotifier.notifyToRequester(id);
    }

}

Job 클래스에 처리 기능이 만들어졌으므로, 다음으로 할 작업은 TranscodingServceImpl 클래스의 transcode() 메서드가 Job 객체의 transcode() 메서드를 호출하도록 변경하는 것이다. 자 과감함이 필요할 때다. TranscodingServiceImpl 클래스에 있던 copyMultimediaSourceToLocal() 메서드 등은 필요 없다. 다 지워버리자. 바뀐 코드는 다음과 같다.

public class TranscodingServiceImpl implements TranscodingService {
    private MediaSourceCopier mediaSourceCopier;
    private Transcoder transcoder;
    private ThumbnailExtractor thumbnailExtractor;
    private CreatedFileSaver createdFileSaver;
    private JobResultNotifier jobResultNotifier;
    private JobStateChanger jobStateChanger;
    private TranscodingExceptionHandler transcodingExceptionHandler;
    private JobRepository jobRepository;

    public TranscodingServiceImpl(MediaSourceCopier mediaSourceCopier,
            Transcoder transcoder, ThumbnailExtractor thumbnailExtractor,
            CreatedFileSaver createdFileSaver,
            JobResultNotifier jobResultNotifier,
            JobStateChanger jobStateChanger,
            TranscodingExceptionHandler transcodingExceptionHandler,
            JobRepository jobRepository) {
        this.mediaSourceCopier = mediaSourceCopier;
        this.transcoder = transcoder;
        this.thumbnailExtractor = thumbnailExtractor;
        this.createdFileSaver = createdFileSaver;
        this.jobResultNotifier = jobResultNotifier;
        this.jobStateChanger = jobStateChanger;
        this.transcodingExceptionHandler = transcodingExceptionHandler;
        this.jobRepository = jobRepository;
    }

    @Override
    public void transcode(Long jobId) {
        Job job = jobRepository.findById(jobId);
        job.transcode(mediaSourceCopier, transcoder, thumbnailExtractor, 
                createdFileSaver, jobResultNotifier);
    }

//    private void changeJobState(Long jobId, State newJobState) {
//        jobStateChanger.chageJobState(jobId, newJobState);
//    }
//
//    private File copyMultimediaSourceToLocal(Long jobId) {
//        try {
//            return mediaSourceCopier.copy(jobId);
//        } catch (RuntimeException ex) {
//            transcodingExceptionHandler.notifyToJob(jobId, ex);
//            throw ex;
//        }
//    }
... // 다른 메서드도 주석으로 처리해버림 (이따 지울 것임)

transcode() 메서드는 JobRepository에서 jobId에 해당하는 Job을 구한 뒤에 해당 Job 객체의 transcode() 메서드를 호출하는 것으로 바뀌었다. jobRepository 필드가 없어서 컴파일 에러가 나므로, jobRepository 필드를 추가해 주고, 생성자를 통해서 전달받도록 추가 구현했다.

TranscodingServiceImpl 클래스의 생성자에 변화가 생겼으므로, 테스트 코드에서 컴파일 에러가 난다. 에러가 나지 않도록 아래와 같이 setup() 메서드를 수정하자.

public class TranscodingServiceImplTest {
    .,.
    @Before
    public void setup() {
        transcodingService = new TranscodingServiceImpl(mediaSourceCopier,
                transcoder, thumbnailExtractor, createdFileSender,
                jobResultNotifier, jobStateChanger,
                transcodingExceptionHandler, jobRepository);

        when(jobRepository.findById(jobId)).thenReturn(mockJob);
        when(mediaSourceCopier.copy(jobId)).thenReturn(mockMultimediaFile);
        when(transcoder.transcode(mockMultimediaFile, jobId)).thenReturn(
                mockMultimediaFiles);
        when(thumbnailExtractor.extract(mockMultimediaFile, jobId)).thenReturn(
                mockThumbnails);
        ...

TranscodingServceImpl의 주요 기능을 Job으로 옮기는 작업이 끝났다. 이제 테스트 코드를 실행할 차례이다. 테스트 코드 실행 .... 두근 두근 두근,,, 켁,, 빨간색이다. 전체 테스트 메서드에서 다 에러가 났다. transcodeFailBecauseExceptionOccuredAtTranscoder() 메서드에의 실패 이유를 살펴보니 아래와 같다.

java.lang.AssertionError: 발생해야 함
    at org.junit.Assert.fail(Assert.java:91)
    at org.....executeFailingTranscodeAndAssertFail(TranscodingServiceImplTest.java:168)
    at org.....transcodeFailBecauseExceptionOccuredAtTranscoder(TranscodingServiceImplTest.java:186)
    ...

관련 테스트 코드 부분은 아래와 같다.

    private void executeFailingTranscodeAndAssertFail(State expectedLastState) {
        try {
            transcodingService.transcode(jobId);
            fail("발생해야 함"); // 테스트 실패 위치
        } catch (Exception ex) {
            assertSame(mockException, ex);
        }

아! 조금 코드를 파 보니, 처리 과정에서 익셉션이 발생하면 Job의 transcode() 메서드가 내부적으로 익셉션을 catch 한 뒤에 재전파하지 않는다. 일단, 통과되도록 아래와 같이 수정하자.

public class Job {
    ...
    public void transcode(MediaSourceCopier mediaSourceCopier,
            Transcoder transcoder, ThumbnailExtractor thumbnailExtractor,
            CreatedFileSaver createdFileSaver,
            JobResultNotifier jobResultNotifier) {
        try {
            changeState(Job.State.MEDIASOURCECOPYING);
            File multimediaFile = copyMultimediaSourceToLocal(mediaSourceCopier);
            ...
            changeState(Job.State.COMPLETED);
        } catch (RuntimeException ex) {
            exceptionOccurred(ex);
            throw ex;
        }
    }

다시 테스트를 실행하자. 아 그래도 빨간색이다. 좀 더 파보자. 오케이,, 원인을 찾았다. Job 객체의 id 필드는 항상 null 이다. 그래서, 테스트 코드의 Mock 객체들이 동작을 안 한 것이다. 이는 Job 객체의 id 필드를 1로 할당해주는 것으로 해결할 수 있다. 일단 테스트 통과를 위해 Job 클래스의 id 필드를 초기화할 수 있도록 생성자를 만들고, 테스트 클래스에서 1을 전달해주자.

// Job 클래스에 생성자 추가
public class Job {

    private Long id;
    ...    
    public Job(Long id) {
        this.id = id;
    }



// 테스트 코드의 Job 객체 생성 시 ID 값 전달
@RunWith(MockitoJUnitRunner.class)
public class TranscodingServiceImplTest {

    private Long jobId = new Long(1);
    private Job mockJob = new Job(jobId);


음, 다시 테스트를 실행해 보자. 두그 두그 두그 두그.... 녹색! 야호!

테스트를 통과했으니, 앞에 TranscodingServiceImpl에서 주석으로 처리했던 부분을 시원하게 지워버리자.

필요 없는 인터페이스 삭제하기

이제 다음으로 할 작업은 필요없어진 타입을 삭제할 차례이다. 그 두 타입은 다음의 두 개이다.
  • JobStateChanger
  • TranscodingExceptionHandler
이 두 타입은 TranscodingServceImpl이 진행 상태 및 익셉션 상황을 Job에게 알리기 위해 출현했었다. 이 두 타입이 더 이상 필요 없어 졌으니 시원하게 지워버리자. 지우면 TranscodingServiceImpl 클래스가 이 두 타입이 없다고 컴파일 에러가 날 것이다. 컴파일 에러를 처리하자. 물론, 테스트 코드에도 컴파일 에러가 생길테니 그것도 처리하자.

@RunWith(MockitoJUnitRunner.class)
public class TranscodingServiceImplTest {

    @Mock
    private MediaSourceCopier mediaSourceCopier;
    @Mock
    private Transcoder transcoder;
    @Mock
    private ThumbnailExtractor thumbnailExtractor;
    @Mock
    private CreatedFileSaver createdFileSender;
    @Mock
    private JobResultNotifier jobResultNotifier;
    @Mock
    private JobRepository jobRepository;
    @Mock
    private JobStateChanger jobStateChanger;
    @Mock
    private TranscodingExceptionHandler transcodingExceptionHandler;

    private TranscodingService transcodingService;

    private File mockMultimediaFile = mock(File.class);
    private List<File> mockMultimediaFiles = new ArrayList<File>();
    private List<File> mockThumbnails = new ArrayList<File>();
    private RuntimeException mockException = new RuntimeException();

    @Before
    public void setup() {
        transcodingService = new TranscodingServiceImpl(mediaSourceCopier,
                transcoder, thumbnailExtractor, createdFileSender,
                jobResultNotifier, /* jobStateChanger,
                transcodingExceptionHandler, */ jobRepository);

        when(jobRepository.findById(jobId)).thenReturn(mockJob);
        when(mediaSourceCopier.copy(jobId)).thenReturn(mockMultimediaFile);
        when(transcoder.transcode(mockMultimediaFile, jobId)).thenReturn(
                mockMultimediaFiles);
        when(thumbnailExtractor.extract(mockMultimediaFile, jobId)).thenReturn(
                mockThumbnails);

        /* 코드 삭제
        doAnswer(new Answer<Object>() {
            @Override
            public Object answer(InvocationOnMock invocation) throws Throwable {
                Job.State newState = (State) invocation.getArguments()[1];
                mockJob.changeState(newState);
                return null;
            }
        }).when(jobStateChanger).chageJobState(anyLong(), any(Job.State.class));

        doAnswer(new Answer<Object>() {
            @Override
            public Object answer(InvocationOnMock invocation) throws Throwable {
                RuntimeException ex = (RuntimeException) invocation
                        .getArguments()[1];
                mockJob.exceptionOccurred(ex);
                return null;
            }
        }).when(transcodingExceptionHandler).notifyToJob(anyLong(),
                any(RuntimeException.class));
        */
    }

이번엔 파일도 삭제했다. 하지만 불안할 필요는 없다. 테스트가 있잖은가! 테스트를 돌려보자. 녹색! 통과다.

Job의 public 메서드 중 private인 것 처리

Job 클래스에서 changeState() 메서드와 exceptionOccurred() 메서드는 더 이상 외부에서 호출되지 않는다. 이 메서드를 호출하기 위해서 사용됐던 JobStateChanger와 TranscodingExceptionHandler가 저 멀리 사라지지 않았는가! 이 두 메서드를 private으로 바꾸자.

    private void changeState(State newState) {
        this.state = newState;
    }

    private void exceptionOccurred(RuntimeException ex) {
        occurredException = ex;
    }

바꿨다. 테스트를 돌리자. 녹색! 통과다.

한 발 나아갔으나, 아직도 부족한 것...

TranscodingServceImpl에서 Job으로 작업 처리를 이동시킴으로써 협업해야 했던 두 개의 타입이 사라졌다. Job의 일부 메서드도 private으로 바꿨다. 그래도 뭔가 아직 부족하다. 그 부분은 아래와 같다.
  • Job이 기능을 수행하기 위해 필요한 협업 객체를 Job에게 전달하기 위해 TranscodingServceImpl 객체가 mediaSourceCopier, transcoder 등의 필드를 갖고 있다.
TranscodingServceImpl 객체는 내부적으로 mediaSourceCopier 객체의 메서드를 호출하지 않는다. 그럼에도 불구하고 mediaSourceCopier를 필드로 갖고 있다. jobRepository를 제외한 나머지 필드는 사실 TranscodingServceImpl은 필요로 하지 않는 것들이다. 그럼에도 불구하고 필드로 갖고 있는 것이다.

public class TranscodingServiceImpl implements TranscodingService {
    private MediaSourceCopier mediaSourceCopier;
    private Transcoder transcoder;
    private ThumbnailExtractor thumbnailExtractor;
    private CreatedFileSaver createdFileSaver;
    private JobResultNotifier jobResultNotifier;
    private JobRepository jobRepository;

    public TranscodingServiceImpl(MediaSourceCopier mediaSourceCopier,
            Transcoder transcoder, ThumbnailExtractor thumbnailExtractor,
            CreatedFileSaver createdFileSaver,
            JobResultNotifier jobResultNotifier,
            JobRepository jobRepository) {
        this.mediaSourceCopier = mediaSourceCopier;
        this.transcoder = transcoder;
        this.thumbnailExtractor = thumbnailExtractor;
        this.createdFileSaver = createdFileSaver;
        this.jobResultNotifier = jobResultNotifier;
        this.jobRepository = jobRepository;
    }

    @Override
    public void transcode(Long jobId) {
        Job job = jobRepository.findById(jobId);
        job.transcode(mediaSourceCopier, transcoder, thumbnailExtractor,
                createdFileSaver, jobResultNotifier);
    }

}

Job 객체가 필요로 하는 협업 객체가 바뀌면 TranscodingServceImpl도 바꿔야 한다. 이런 문제를 없애려면 빨간색으로 표시한 부분을 모두 삭제하거나 또는 최소화해야 할 것 같다.

어떻게 하면 이를 줄일 수 있을까? 조금 더 고민을 해 보고 싶지만,, 지금 시간이 23시 25분이다. 졸리다. 자야겠다. 나머지 리팩토링은 내일 저녁에 마저 진행해 보자.


Posted by 최범균 madvirus

댓글을 달아 주세요

  1. 백명석 2012.10.26 13:01 신고  댓글주소  수정/삭제  댓글쓰기

    알고 있겠지만. 보는 이들을 위해... job#transcode가 너무 크다.
    "function should do onething !!"
    "Extract till you drop !!"
    그리고 의존성 문제는 spring의 경우 @Configurable이나 aspectJ로 해소하는 방법밖에 없지 않을까나 ?

    • 최범균 madvirus 2012.10.26 15:39 신고  댓글주소  수정/삭제

      AspectJ를 사용하지 않게 되면,,,,, 아무래도 Locator 같은 것이 출현하겠죠? 푸,, AspectJ 사용하면 단위 테스트 목적으로 별도 메서드나 기능을 만들어야 해서,, 그게 좀 짱나요. ^^;

  2. 백명석 2012.10.26 18:52 신고  댓글주소  수정/삭제  댓글쓰기

    File multimediaFile = copyMultimediaSourceToLocal(mediaSourceCopier);
    changeState(Job.State.TRANSCODING);
    처럼. doSomething then changeState의 구조적 반복도 제거해야...