채팅방 API 서버 - ChatRoomService 개선(1)

https://github.com/bellringstar/keepham/tree/master/BACKEND/src/main/java/com/ssafy/keepham/domain/chatroom

keepham/BACKEND/src/main/java/com/ssafy/keepham/domain/chatroom at master · bellringstar/keepham

 

https://github.com/bellringstar/keepham-refact/tree/main/BACKEND/src/main/java/com/ssafy/keepham/domain/chatroom

keepham-refact/BACKEND/src/main/java/com/ssafy/keepham/domain/chatroom at main · bellringstar/keepham-refact

 

위에서 리팩토링 전의 전체 코드를 확인할 수 있습니다.

코드를 보면 아시겠지만 전체적으로 책임의 분리, 예외처리 일관성 부족, 동시성 이슈 등이 부족합니다.

---

엔티티가 정해졌으니 이제 서비스를 개선해보도록 하겠습니다.

서비스 개선에는 책임의 분리, 메서드 변경 등이 있지만 우선은 메서드들부터 하나씩 분석해 보고 개선하는 과정을 가져보겠습니다.

createRoom(전)

    @Transactional
    public ChatRoomResponse createRoom(ChatRoomRequest chatRoomRequest){
        var userInfo = userService.getLoginUserInfo();
        var userNickName = userInfo.getNickName();
        var entity = chatRoomConverter.toEntity(chatRoomRequest);
        var box = boxRepository.findFirstById(chatRoomRequest.getBoxId());
        if (box.isUsed()) {
            throw new ApiException(ErrorCode.BAD_REQUEST, "이미 사용중인 box입니다.");
        }
        box.setUsed(true);
        entity.setBox(box);
        return Optional.ofNullable(entity)
                .map(it -> {
                    it.setStatus(ChatRoomStatus.OPEN);
                    var newEntity = chatRoomRepository.save(it);
                    newEntity.setClosedAt(newEntity.getCreatedAt().plusHours(3));
                    newEntity.setSuperUserId(userNickName);
                    chatRoomManager.userJoin(newEntity.getId(), userService.getLoginUserInfo().getNickName());
                    return chatRoomConverter.toResponse(newEntity);
                })
                .orElseThrow(() -> new ApiException(ErrorCode.BAD_REQUEST));
    }

채팅방을 생성하는 메서드입니다. 쭉 살펴보죠.

우선 @Transactional 어노테이션이 있습니다. 해당 메소드에서 동작하는 작업들을 하나의 트랜잭션으로 묶고 있습니다. 전파 방식이나 격리 수준등의 별도 설정은 없는 디폴트 상태입니다.

@Transactional의 필요여부부터 따져보겠습니다. createRoom의 주요 작업은 다음과 같습니다.

1. 사용자 조회
2. Box 상태 조회 및 변경
3. ChatRoom 엔티티 생성 및 저장
4. chatRoomManager를 통해 사용자 참여

이 작업에 있어 Box의 상태 변경과 ChatRoom의 생성이 원자적으로 이루어져야 합니다.

또한 Box 업데이트와 ChatRoom 저장이라는 두 개의 별도 작업이 있습니다.

따라서 @Transactional 어노테이션이 필요한것으로 보입니다.

 

다음으로 입력값과 반환값입니다.

입력값으로 ChatRoom 생성을 위한 dto인 chatRoomRequest를 받고 있으며 반환값으로는 ChatRoom 생성의 결과인 ChatRoomResponse를 반환하고 있습니다.

여기서 고민점은 반환값입니다. ChatRoomResponse는 api response에 종속돼 있습니다. 하지만 저는 서비스계층은 프레젠테이션 계층과 독립적이어야 한다고 생각합니다.

따라서 ChatRoom 엔티티를 반환하는게 맞지 않을까 하는 고민이 있습니다. 그렇지만 ChatRoom 엔티티를 반환하게 되면 생기는 문제 또한 존재합니다.

프레젠테이션 계층에서 엔티티에 대한 의존이 생긴다는 것도 있지만 이 부분은 그렇게 큰 문제가 되지 않는다고 생각합니다.

더 큰 문제는 OSIV 문제입니다.

OSIV를 간단하게 설명하자면 영속성 컨텍스트의 생존 범위를 뷰 렌더링 시점까지 늘리는 기술입니다.

이를 통해 지연 로딩(Lazy Loading) 문제를 해결하고 컨트롤러와 뷰에서 영속성 객체를 편리하게 사용할 수 있도록 합니다.

그렇지만 영속성 컨텍스트가 오랜 시간 열려있기에 메모리 문제가 발생할 수 있습니다.

또한 데이터베이스 연결 시간이 늘어나기에 커넥션 문제 또한 발생할 수 있죠.

따라서 대부분의 경우 OSIV를 비활성화하게 됩니다.

이렇게 되면 프레젠테이션 계층에서 dto로 변환에 문제가 생길 수 있습니다.

종합적으로 판단한 결과 반환값을 dto인 ChatRoomResponse로 유지하는 게 맞다는 판단이 들었습니다.

 

이제 코드로 들어가 보겠습니다.

우선 setter를 많이 쓰고있는 부분은 모든 엔티티에서 setter를 없앨 예정이기에 수정을 할 예정입니다.

진짜 고민은 서비스에서 다른 서비스를 의존하고 있다는 것입니다.

예를 들어 현재 ChatRoomService는 UserService를 의존하고 있습니다.

이는 높은 결합도를 갖게 되므로 UserService가 변경되면 ChatRoomService도 영향을 받게 됩니다.

어떤 방식으로 개선하는게 좋을까요?

1. repository 의존예를 들어 단순한 조회가 아니라 일종의 비즈니스 로직이 들어간 경우라면? 코드가 중복이 발생하게 되고 유지보수성이 떨어지게 됩니다.
2. 그렇다면 UserRepository를 의존하는 게 맞을까? 이 역시도 아니라고 생각합니다.
3. controller에서 다양한 서비스를 의존 이 방법은 하나의 트랜잭션으로 묶을 수 없기에 최악이라고 생각한다.
4. 여러 Service를 묶는 하나의 Service만들기 일종의 Facade Pattern이다. 하지만 이는 복잡성을 불러일으킨다.

기존의 방식대로 Service에서 Service를 의존해도 문제는 없습니다.

대신 다른 도메인도 관리하게 돼 책임이 불분명해지는 문제와 순환 참조가 걱정일 뿐이죠.

그래도 순환참조는 서비스를 쪼개서 순환 참조가 되지 않도록 개선할 수도 있습니다.

Facade 방식은 별도의 관리할 객체가 늘어난다는 복잡성이 문제입니다.

결론적으로는 의존성을 줄이고 결합도를 낮추는 설계를 선호하기 때문에 Facade 패턴을 도입해 개선을 하도록 하겠습니다.

이렇게 되면 위에서 서비스에서 dto를 반환하기로 한 결정이 좀 수정이 돼야 합니다.

하위 서비스에서는 entity를 반환하고 Facade에서 dto로 변환해 반환하는 방식으로 처리하도록 하겠습니다.

이렇게 하니 대수술이 필요해졌습니다.

특히 UserService 등 제가 구현하지 않은 부분들의 수정도 필요해졌습니다. 우선은 임시로 구현해 놓도록 하겠습니다.

createRoom(후)

    @Transactional
    public ChatRoom createRoom(ChatRoomRequest chatRoomRequest, Box box, User user, Store store) {
        ChatRoom chatRoom = ChatRoom.builder()
                .title(chatRoomRequest.getTitle())
                .store(store)
                .box(box)
                .maxPeopleNumber(chatRoomRequest.getMaxPeopleNumber())
                .superUser(user)
                .locked(chatRoomRequest.isLocked())
                .password(chatRoomRequest.getPassword())
                .build();

        return chatRoomRepository.save(chatRoom);
    }

createRoom 메서드는 단순하게 chatRoom을 만들어 저장하는 역할만 하게 변경됐습니다.

@Service
@RequiredArgsConstructor
@Slf4j
public class ChatRoomFacade {

    private final ChatRoomConverter chatRoomConverter;
    private final ChatRoomManager chatRoomManager;
    private final ChatRoomService chatRoomService;
    private final BoxService boxService;
    private final StoreService storeService;

    @Transactional
    public ChatRoomResponse createRoom(ChatRoomRequest chatRoomRequest, User user) {
        Box box = boxService.getValidBox(chatRoomRequest.getBoxId());
        Store store = storeService.getAvailableStore(chatRoomRequest.getStoreId());

        ChatRoom chatRoom = chatRoomService.createRoom(chatRoomRequest, box, user, store);
        box.markAsUsed();
        return chatRoomConverter.toResponse(chatRoom);
    }
}

그리고 다른 서비스를 의존하는 Facade 서비스가 생성됐습니다.

이를 통해 서비스 간의 의존성을 줄일 수 있었습니다.

 

이제 남은 건 동시성 문제입니다.

현재 Box의 사용 여부를 판단 한 다음 채팅방을 생성하고 Box의 사용여부를 변경하고 있습니다.

이 부분이 동시성 이슈가 있을 수 있는 부분입니다.

예를 들어 여러 사용자가 동시에 동일한 boxId로 채팅방 생성 요청을 보냈을 때, 두 개 이상의 스레드가 동시에 boxService.getValidBox(boxId)를 호출하게 됩니다.

이때 만약 동시에 호출된 경우 둘 다 Box가 사용 중이 아니라고 판단할 수 있고 이로 인해 동일한 Box가 여러 ChatRoom에 할당되는 문제가 발생하게 됩니다.

 

이를 해결하기 위해 락을 사용할 것입니다.

이제 비관적락을 쓸 건지 낙관적락을 쓸건지 선택해야 합니다.

테스트를 해보는 게 제일 좋지만 현재 환경상 그러기 힘드므로 이론적으로 선택을 하겠습니다.

채팅방을 생성하면서 같은 박스를 선택하는 경우는 충돌이 빈번하게 발생할 것으로 예상됩니다.

@Lock(LockModeType.PESSIMISTIC_WRITE)
@Query("SELECT b FROM Box b WHERE b.id = :id")
Optional<Box> findByIdForUpdate(@Param("id") Long id);

따라서 비관적 락을 적용해 Box를 조회하고 사용 가능 여부를 확인하는 과정을 원자적으로 처리하도록 하였습니다.