우리 가게 영업 안합니다

 

영업은 잠시 중단했다. 실행 화면 다시 볼 수 있겠지..?

 

백엔드 애플리케이션의 거의 전체를 리팩터링하고 있다.

처음에는 아샬님께서 리팩터링해주신 부분 위주로 수정하고 작업을 진행하면서 다른 부분들도 하나씩 수정해나가는 것이 목표였지만, 테스트를 작동시키기 위해서는 수정된 부분이 사용되고 있는 다른 Service 로직들도 수정해줘야 했다. 문제는 그 Service 로직들에는 여전히 모든 작업이 몰려 있었기 때문에 그 부분들의 인터페이스만 맞춰준다고 해결될 문제들이 아니었다. 결국 오늘 하루는 깔끔하게 리팩터링에만 투자하기로 했다. 

 

아직 모든 리팩터링 과정이 끝나지 않았다. 일부 Controller 테스트, Exception Hander 리팩터링, 예외 메세지를 프론트엔드에서 받아서 처리하는 방식을 마저 손봐야 한다. 일단 지금까지 리팩터링한 것들 중 인상 깊었던 내용을 정리했다.

 

 

진짜로 테스트하기 쉬워진다.

사용자가 운동 게시글에 참가를 신청했을 때 동작을 처리하는 로직의 소스코드가 아샬님의 리팩터링을 통해 다음과 같이 바뀌었다.

 

// services/JoinGameService.java

public RegisterGameResultDto joinGame(Long gameId,
                                      Long currentUserId) {
    User currentUser = userRepository.findById(currentUserId)
        .orElseThrow(() -> new UserNotFound(currentUserId));

    Game game = gameRepository.findById(gameId)
        .orElseThrow(() -> new GameNotFound(gameId));

    List<Register> registers = registerRepository.findByGameId(gameId);

    Register register = game.join(currentUser, registers);

    registerRepository.save(register);

    return new RegisterGameResultDto(gameId);
}

// models/Game.java

public Register join(User currentUser, List<Register> registers) {
    this.registers = registers;

    if (alreadyJoined(currentUser)) {
        throw new AlreadyJoinedGame(id);
    }

    if (isFull()) {
        throw new GameIsFull(id);
    }

    return new Register(currentUser, this);
}

private boolean alreadyJoined(User user) {
    return registers.stream()
        .filter(Register::active)
        .anyMatch(register -> register.match(user));
}

private boolean isFull() {
    long count = registers.stream()
        .filter(Register::accepted)
        .count();
    return targetMemberCount.reach(count);
}

// models/Register.java

public boolean active() {
    return status.equals(RegisterStatus.processing())
        || status.equals(RegisterStatus.accepted());
}

public boolean accepted() {
    return status.equals(RegisterStatus.accepted());
}

// models/GameTargetMemberCount.java

public boolean reach(long count) {
    return count >= value;
}

 

바뀐 지금의 로직과 이전 로직의 차이점은 Service에서 모두 처리하고 있었던 동작의 핵심 로직들을 책임이 있는 객체에게 분배하기 시작했다는 점이다. 이제 Service에서는 단지 동작을 수행하는 데 필요한 데이터들을 가져오고, 객체로부터 전달되는 결과를 저장하거나 반환하는 역할만을 맡는다. 동작의 책임을 맡은 객체들은 전달받은 데이터를 이용해 처리를 맡고, 처리를 위해 다른 객체의 협력이 필요할 경우 다른 객체에게 요청해 처리를 위임한다.

 

위의 소스코드를 예시로 들어보면, User를 Game에 참가시키기 위한 동작은 우선 Game 객체가 맡는다. Game 객체는 신청한 User 1명의 정보와, 자신에게 신청된 모든 신청(Register) 정보들을 받아온다.

 

Game 객체는 신청 User가 이미 신청했는지, 혹은 정원이 가득 찼는지를 Register를 하나씩 확인하면서 검사한다. 이때 Game 자신의 범위만으로 알아낼 수 없는 각 Register의 상태는, 확인하는 각각의 Register에게 신청 상태(RegisterStatus)를 알려줄 것을 요청하는 방식으로 동작을 위임한다.

 

Game의 정원이 가득 찼는지는 정원(TargetMemberCount) 객체에게 참가 상태인(accepted) Register의 개수를 전달해 정원을 비교한 결과값을 전달받는 방식으로 동작을 위임한다.

 

모든 위임한 동작들로부터 예외가 발생하지 않으면, 참가신청 상태의 Register를 하나 생성해 Service 레이어에 반환한다.

 

 

그리고 의외로 생각하지 못했던 좋은 점이 또 있었다. 객체의 동작을 잘 테스트하기 위해 fake 메서드의 인터페이스를 어떻게 설정하는 게 좋을지 고민하는 비용이 줄기 시작했다는 점이다.

 

Service에 모든 동작이 몰려 있던 이전에는 fake 메서드를 만들기 위해 너무 많은 내용들이 인터페이스로 전달되어야 했을 뿐더러, 그 형태는 사실상 생성자로 생성에 필요한 모든 값을 전달해 생성하는 것과 다를 바가 없었다. 그러나 로직의 핵심 동작을 테스트할 때는 객체의 모든 내용을 다 알아야 할 필요가 없이 동작에서 사용되는 데이터의 차이만을 명확하게 구분할 수 있으면 되었다.

 

필요한 인터페이스를 갖는 fake 메서드들을 적절히 생성해 보았다. 다음은 Game 객체의 join 동작을 검증하는 테스트 코드이다. 

 

// models/GameTest.java

@Test
void join() {
    Game game = Game.fake("운동 이름", "장소 이름");
    User user = User.fake(4L);
    List<Register> registers = List.of(
        Register.fake(1L, game.id(), RegisterStatus.accepted()),
        Register.fake(2L, game.id(), RegisterStatus.accepted()),
        Register.fake(3L, game.id(), RegisterStatus.accepted())
    );

    Register register = game.join(user, registers);
    assertThat(register.status()).isEqualTo(RegisterStatus.processing());
}

@Test
void alreadyJoined() {
    Game game = Game.fake("운동 이름", "장소 이름");
    User user = User.fake(1L);
    List<Register> registers = List.of(
        Register.fake(1L, game.id(), RegisterStatus.processing()),
        Register.fake(2L, game.id(), RegisterStatus.accepted()),
        Register.fake(3L, game.id(), RegisterStatus.accepted())
    );

    assertThrows(AlreadyJoinedGame.class, () -> game.join(user, registers));
}

@Test
void gameIsFull() {
    Game game = Game.fake(new GameTargetMemberCount(2));
    User user = User.fake(5L);
    List<Register> registers = List.of(
        Register.fake(1L, game.id(), RegisterStatus.accepted()),
        Register.fake(2L, game.id(), RegisterStatus.accepted()),
        Register.fake(3L, game.id(), RegisterStatus.rejected()),
        Register.fake(4L, game.id(), RegisterStatus.processing())
    );

    assertThrows(GameIsFull.class, () -> game.join(user, registers));
}

 

물론 여전히 상수값이 사용되어 인지에 어려운 부분들이 존재한다. (User와 Register에 전달되는 상수 숫자값은 User Id이다. 즉 이 테스트의 가독성을 높이기 위해서는 id 필드를 값 객체로 정의해야 한다. 이제는 id 값 객체를 정의해야 할 필요성이 생겼다.) 그러나 로직의 동작을 확인하기 위해 어떤 객체들과 값이 영향을 미치는지를 어렵지 않게 확인할 수 있다. 

 

fake 메서드의 다양한 인터페이스 정의는 객체의 동작 역할 분담과 맞물려 Service 테스트 코드의 크기를 줄이는 데 큰 영향을 끼쳤다. 약 260줄 가량 되던 JoinGameService의 테스트 코드의 크기를 적절한 fake 메서드의 사용과 로직의 분리를 통해 다음과 같이 줄일 수 있었다.

 

// services/JoinGameServiceTest.java

@Test
void joinGame() {
    Long userId = 4L;
    User user = User.fake(userId);
    given(userRepository.findById(userId)).willReturn(Optional.of(user));

    Long gameId = 1L;
    Game game = Game.fake(new GameTargetMemberCount(3));
    given(gameRepository.findById(gameId)).willReturn(Optional.of(game));

    List<Register> registers = List.of(
        Register.fake(1L, game.id(), RegisterStatus.accepted()),
        Register.fake(2L, game.id(), RegisterStatus.canceled()),
        Register.fake(3L, game.id(), RegisterStatus.rejected())
    );
    given(registerRepository.findByGameId(game.id()))
        .willReturn(registers);

    RegisterGameResultDto registerGameResultDto
        = joinGameService.joinGame(gameId, userId);

    assertThat(registerGameResultDto.getGameId()).isEqualTo(1L);
}

@Test
void joinGameWithInvalidUser() {
    Long invalidUserId = 5000L;
    given(userRepository.findById(invalidUserId))
        .willThrow(UserNotFound.class);

    Long gameId = 1L;

    assertThrows(UserNotFound.class, () -> {
        joinGameService.joinGame(gameId, invalidUserId);
    });

    verify(userRepository).findById(invalidUserId);
    verify(gameRepository, never()).findById(any(Long.class));
    verify(registerRepository, never()).findByGameId(any(Long.class));
}

@Test
void joinGameWithInvalidGame() {
    Long userId = 1L;
    User user = User.fake(userId);
    given(userRepository.findById(userId))
        .willReturn(Optional.of(user));

    Long invalidGameId = 9999L;
    given(gameRepository.findById(invalidGameId))
        .willThrow(GameNotFound.class);

    assertThrows(GameNotFound.class, () -> {
        joinGameService.joinGame(invalidGameId, userId);
    });

    verify(userRepository).findById(userId);
    verify(gameRepository).findById(invalidGameId);
    verify(registerRepository, never()).findByGameId(any(Long.class));
}

 

 

작업 내역을 올릴 수 있을지 브랜치를 확인하던 중 귀찮은 문제를 하나 발견했다. 이미 remote에서 merge를 마친 브랜치에 리팩터링 작업을 계속하고 있었던 것이다. 수정한 소스코드들을 잠시 어디다 모셔놓은 다음에 프로젝트는 전부 restore시키고, main으로 브랜치를 옮겨서 fetch/rebase해온 뒤 작업 브랜치를 다시 만들고 모셔놓은 소스코드들을 옮겨오면 될 것 같긴 하지만... 작업 시작 전 브랜치를 확인하는 습관... 잊지 말자.