지속 성장 가능한 프로젝트를 위해 시도해볼 수 있는 것(1) - 서비스 계층을 서비스 계층 답게

해당 내용은
(현) 토스 개발자인 제미니 님의

지속 성장 가능한 소프트웨어를 만들어가는 방법 - 아티클
지속 성장 가능한 소프트웨어를 만들어가는 방법 영상 - 아티클

내용을 읽고 프로젝트에서 도입을 하며 느낀점 + 코드를 리팩토링 하며 느낀점들에 대해 다룹니다.
( 틀린 내용, 궁금한 점이 있으면 joyson5582@gmail.com 이나 댓글로 남겨주세요! )

왜 코드는 더러워지는가

사실, 코드가 더러워지는건 너무도 당연한 수순입니다.
우리의 프로젝트가 사용자의 요구 ( 또는 받아들일 설계 ) 를 반영하기 위해 기능을 좀 더 세밀하게 & 추가해야 하기 때문입니다.

저희 프로젝트에서 가장 중요한 ( 코드 리뷰를 매칭 시켜주는 ) 방 의 관련된 로직을 수행하는 RoomService 만 해도
13일 부터 19일 까지 8번의 수정 사항이 발생했습니다.

기능이 추가함에 따라 크기가 크든, 작든 꾸준히 코드를 쌓게 됩니다.

그리고, 이런 코드들은 단위로 얼마나 깔끔하게 짜든 서비스의 공간을 차지하게 됩니다.
( 위의 stream 구문은 문제를 느낄 요소가 없는 것 처럼요 )

기존 MVC 는 뭐가 문제일까?

흔히, 웹 개발자가 아는 가장 유명한 디자인 패턴은 MVC 패턴일 겁니다.

• Controller : 사용자 요청을 처리하고, Model - View 를 연결해준다.
• View : 사용자의 UI 를 담당한다.
• Model : 애플리케이션 핵심 데이터 & 비즈니스 로직을 담당한다.

자바 계열의 스프링, 자바스크립트 계열의 네스트(Nest.js), 파이선 계열의 장고(Django) 등 대부분의 웹 서버 프레임워크가 이를 도입하고 있습니다.

이런 MVC 패턴에서 서비스 레이어는 사실상 필연적입니다.
Controller Model 간 중간 계층을 하나 더 만들어서 비즈니스 로직을 수행함으로써 비즈니스 로직을 재사용하게 해줍니다.

추가적으로, TO(Transfer Object) 를 통해서

• 웹 요청을 일관성 & 용이하게
• 도메인 에서 사용자가 원하는 응답을 재사용 가능하게
  일반적으로 웹 프레임워크를 구축합니다.

그러면, 왜 디자인 패턴과 정형화된 레이어가 있는데 코드는 계속 더러워져만 갈까요?

서비스 계층은 해야하는게 너무나 많다.

이 서비스는 너무나도 많은 역할을 가지는 경우가 많습니다.
( 사실, 서비스 계층이 완벽하게 정립 및 재사용이 가능하다면, 최고일 겁니다. )

@Transactional
public RoomResponse update(long memberId, RoomUpdateRequest request) {
	Room room = getRoom(request.roomId());
	
	Member member = memberRepository.findById(memberId)
			.orElseThrow(() -> new CoreaException(ExceptionType.MEMBER_NOT_FOUND));
	
	if (room.isNotMatchingManager(memberId)) {
		throw new CoreaException(ExceptionType.MEMBER_IS_NOT_MANAGER);
	}

	if (room.isNotOpened()) {  
	    throw new CoreaException(ExceptionType.ROOM_STATUS_INVALID);  
	}

	Room updatedRoom = roomRepository.save(request.toEntity(room,member));
	Participation participation = participationRepository.findByRoomIdAndMemberId(updatedRoom.getId(), memberId)
			.orElseThrow(() -> new CoreaException(ExceptionType.NOT_ALREADY_APPLY));


	automaticMatchingRepository.save(new AutomaticMatching(room.getId(), request.recruitmentDeadline()));
	automaticMatchingService.matchOnRecruitmentDeadline(response);
	
	automaticUpdateRepository.save(new AutomaticUpdate(room.getId(), request.reviewDeadline()));
	automaticUpdateService.updateAtReviewDeadline(response);
	
	
	log.info("방을 수정했습니다. 방 id={}, 사용자 iD={}", room.getId(), member.getId());
	
	return RoomResponse.of(updatedRoom, participation.getMemberRole(), ParticipationStatus.MANAGER);
}

방을 수정하는 로직으로 예시를 살펴보겠습니다.

1. roomId 와 memberId 를 통해 조회를 합니다.
2. 방이 열려있는지 확인합니다.
3. 방의 매니저가 맞는지 확인합니다.
4. DTO 를 통해 방을 수정합니다.
5. Manager 가 참여한 역할을 찾습니다.
6. 스케줄러에 등록된 매칭 & 종료 시간을 수정합니다.
7. 방을 수정했다는 로그를 작성합니다.
8. 위의 값들을 통해 응답으로 변환합니다.

하나씩 나열해보면, 꽤나 타당하고 명확합니다.
이와 같이 서비스 계층은 계속 뚱뚱해져 가게 됩니다.

모호한 이름의 서비스

일반적으로 도메인과 서비스 이름을 1:1로 매칭을 시키는 경우가 있습니다.
( 또는, 컨트롤러와 1:1 매칭 RoomController - RoomService )

처음 도메인을 새로 만들 때는 나쁘지 않습니다.
방의 비즈니스 로직을 담당하는 서비스이자, 기능도 몇개가 되지 않아서 타당하게 보일 테니까요.

하지만, 기능이 늘어남에 따라 서비스 코드는 뚱뚱해지게 됩니다.

RoomService 니까..?

• 방을 생성 / 수정 / 삭제하는 기능이 있어야지 🙂
• 방의 상태 ( 오픈 / 진행 중 / 종료 ) 에 따른 조회하는 기능도 추가 해야지 🥲🥲
• 방에 참가한 참가자들을 보여주고 싶은데 RoomService 의 기능인가...?
• 사용자가 참가한 방을 보고 싶은데 또 RoomService 야? 😢😢

이렇게 하게 된 이유는
@RequestMapping("/rooms") 와 같이 경로 매핑을 rooms 로 했기에 , 동일한 경로는 다 같은 곳에 묶고 싶다는 욕심 때문도 있을 겁니다.

다양한 도메인에서 import, 긴 코드 라인(175 lines) 이 되게 됩니다.
이런 파일은 코드를 사용해야 하는 사람도 어려움을 겪을 겁니다. 😢

그러면, 이를 천천히 리팩토링 해나가보겠습니다.

Reader,Wrtier 를 통한 비즈니스 로직 명세화

방을 수정하는 로직을 다시 살펴보겠습니다.

1. roomId 와 memberId 를 통해 조회를 합니다.
2. 방이 열려있는지 확인합니다.
3. 방의 매니저가 맞는지 확인합니다.
4. DTO 를 통해 방을 수정합니다.
5. 방을 수정했다는 로그를 작성합니다.
6. Manager 가 참여한 역할을 찾습니다.
7. 스케줄러에 등록된 매칭 & 종료 시간을 수정합니다.
8. 위의 값들을 통해 응답으로 변환합니다.

하지만, 우리가 기대하는 방의 수정 로직이라면

• 방과 매니저가 있는걸 조회한다.
• 방과 매니저를 통해 요청대로 수정한다
• 수정한 방 정보에 맞게 스케줄러를 수정한다

정도 입니다.
그러면, 위의 기대를 충족하기 위해 Reader 와 Writer 를 만들어보겠습니다.

@Component  
@RequiredArgsConstructor  
@Transactional(readOnly = true)
public class RoomReader {  
  
    private final RoomRepository roomRepository;  
  
    public Room find(long roomId) {  
        return roomRepository.findById(roomId)  
                .orElseThrow(() ->  
                        new CoreaException(
	                        ExceptionType.ROOM_NOT_FOUND, String.format("해당 Id의 방이 없습니다. 입력된 Id=%d", roomId)));  
    }  
}

와 같이 Room 을 조회하는거 담당하는 Reader 입니다.

이 Reade Wrtier 를 좀 더 명확히 하고 싶다면?

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@Component  
@RequiredArgsConstructor  
@Transactional(readOnly = true)
public @interface Reader {
}

( 위와 같이 어노테이션도 만들어서 명확하게 명시 가능합니다. )

---

그냥 RoomRepository 를 사용해서 조회를 하는게 아닌, 방과 관련된 요소들을 조회 하는 책임을 가지는 클래스 입니다.
이를 통해서, 각 서비스 계층이 조회를 할 때마다 getXXX 을 가지는 것을 방지하게 해줍니다.
( 추가로, 예외도 일관성 있게 작성을 시켜줍니다. )

public Room update(Room room, Member member, RoomUpdateRequest request) {  
    validate(room, member);  
    return roomRepository.save(request.toEntity(room, member));  
}

private void validate(Room room, Member member) {  
    if (room.isNotMatchingManager(member.getId())) {  
        log.warn("인증되지 않은 방 변경 시도 방 생성자 id={}, 요청한 사용자 id={}", room.getId(), member.getId());  
        throw new CoreaException(ExceptionType.ROOM_MODIFY_AUTHORIZATION_ERROR);  
    }  
    if (room.isNotOpened()) {  
        throw new CoreaException(ExceptionType.ROOM_STATUS_INVALID);  
    }  
}

방을 수정하기 전
수정하려는 사람이 방을 만든 사람이 맞는지, 방이 열려있는지를 확인합니다.

public Room toEntity(Room room, Member member) {  
    return new Room(  
            roomId,  
            title, content,  
            matchingSize, repositoryLink,  
            thumbnailLink, keywords, currentParticipatsSize, limitedParticipants,  
            member, recruitmentDeadline,  
            reviewDeadline, classification,  
            room.getStatus()  
    );  
}

그 후, DTO 를 통해 엔티티를 만들어서 저장합니다.
( 저희 팀은 DTO -> 엔티티,도메인으로 변환하는걸 적극적으로 사용하기로 했습니다.
단순 웹 요청 / 응답에서 변환을 위해 RequestMapper 와 같은걸 만드는건 불필요하다고 판단했기 때문입니다. )

@Transactional  
public RoomResponse update(long memberId, RoomUpdateRequest request) {  
    Room room = roomReader.find(request.roomId());  
    Member member = memberReader.findOne(memberId);  
  
    Room updatedRoom = roomWriter.update(room, member, request);
    ...

이와 같은 식으로 변환이 가능해집니다.
그러면, 왜 이렇게 해야 하는가? 에 대해서 의견을 남기겠습니다.
( 프로젝트 내 Reader/Writer 도입에 대한 토론 )

더 다듬어서 여기에 정리를 해보면

정책 및 코드 & 엔티티 변경이 서비스에 영향을 주지 않는다

정책부터 설명을 해보겠습니다.

현재, 방을 만들 떄 방장이 방에 참여하는 로직을 포함합니다.
( 방을 만들고, 방장이 리뷰어로 참여해야 한다는 정책 때문에 )

나중에, 방장이 굳이 리뷰어로 참여할 필요가 없을거 같은데?와 같이 변경이 된다면?

// RoomWriter
public Room create(Member member, RoomCreateRequest request) {  
	Room room = roomRepository.save(request.toEntity(member));  
	participationWriter.create(room, member, MemberRole.REVIEWER, ParticipationStatus.MANAGER);  
	log.info("방을 생성했습니다. 방 생성자 id={}, 요청한 사용자 id={}", room.getId(), room.getManagerId());  
	return room;  
}

해당 로직에서, participationWriter 부분만 제거를 하면 됩니다.
즉, 서비스는 코드를 수정하지 않아도 됩니다.

두 번째로, 코드 & 엔티티 변경 부분 입니다.
방이 현재 참여하는 인원을 가지고 있는 있습니다.

public class Room extends BaseTimeEntity {

	...
	private int currentParticipantsSize;
	
	...
	
	public void participate() {  
	    validateOpened();  
	    if (currentParticipantsSize >= limitedParticipantsSize) {  
	        throw new CoreaException(ExceptionType.ROOM_PARTICIPANT_EXCEED);  
	    }  
	    currentParticipantsSize += 1;  
	}  
	  
	private void validateOpened() {  
	    if (status.isNotOpened()) {  
	        throw new CoreaException(ExceptionType.ROOM_STATUS_INVALID);  
	    }  
	}
}

나중에, 이를 제거하고 count(*) 와 같이 알아오게 된다면?

public RoomInfo readRoomInfo(long roomId) {  
    Room room = find(roomId);  
    long participationCount = participationReader.countParticipantsByRoomId(roomId);  
    return new RoomInfo(...);  
}

와 같이 reader 를 통해 참가자의 수를 가져오게 됩니다.
서비스는 여전히 변경을 알지 못합니다.

이에 대해서는 실용주의 프로그래머 에서 아래 내용에도 같이 나와 있습니다.

( 좋은 인사이트를 준 아루에게 감사를 🙏🙏 )

재 사용 될 수 있게

가장 쉬우면서 어려운 설명입니다.
예시와 함께 좀 더 설명해보겠습니다.

방을 참여할 때

• 자신이 어떤 역할로 참여하길 원하는지 ( 리뷰어/리뷰이 둘다, 리뷰어, 리뷰이 )
• 자신이 몇명을 해주길 원하는지
• 방장인지 / 그냥 참여하는지
  를 결정해서 참여합니다.

//RoomService
public RoomResponse create(long memberId, RoomCreateRequest request) {
	...
	Participation participation = new Participation(room, manager);
	participationRepository.save(participation);
}

// Participation
public Participation(Room room, Member member) {  
    this(null, room, member, MemberRole.REVIEWER, ParticipationStatus.MANAGER, room.getMatchingSize());  
}

방장이 방을 생성하면서 참가하는 부분입니다.

// ParticipationService
private Participation saveParticipation(ParticipationRequest request) {
	    ...

        Participation participation = new Participation(getRoom(request.roomId()), member, memberRole, request.matchingSize());
        participation.participate();
        return participationRepository.save(participation);
    }
}

// Participation
public void participate() {  
    room.participate();  
}

public Participation(Room room, Member member, MemberRole role, int matchingSize) {
        this(null, room, member, role, ParticipationStatus.PARTICIPATED, matchingSize);
        debug(room.getId(), member.getId());
}

참가자가 참여하는 부분입니다.
( 의도적으로, Reader/Writer 를 쓰지 않은 예전 코드입니다. )

추가로, 생성자 오버로딩은 정말 정말 추천하지 않습니다..
테스트 편의성을 위해서나, 코드 간결성을 위해서 할 수 있으나
이는 결국 기능이나 엔티티 변경이 일어날 때 다시 돌아오게 되어 있습니다.
( 처음에는 편해지나, 분기 처리 및 모든 걸 다룰 수 없다는 의미

• 현재 저희는 픽스처에서만 사용하는 생성자가 있는데 30 정도 부분에서 사용되서 제거도 못합니다..😢 )

당장에는 코드적으로 문제도 되지 않고, 길이도 길지 않아서 인지를 못할 수 있습니다.

하지만, 참가를 하는 로직이 두 곳에서 따로 관리가 된다는 문제가 발생합니다.

• 로직이 더 추가될 때마다 두 곳에서 모두 반영이 되야 한다.
• 테스트 코드에서 일관성이 깨질 수 있다. ( 어떤 곳은 이를 사용해 처리, 다른 곳은 다르게 사용해 처리 )

이 역시도 Writer 를 사용해서 처리해보겠습니다.

public Participation create(Room room, Member member, MemberRole memberRole, ParticipationStatus participationStatus) {  
    return create(room, member, memberRole, participationStatus, room.getMatchingSize());  
}  
  
public Participation create(Room room, Member member, MemberRole memberRole, int matchingSize) {  
    return create(room, member, memberRole, ParticipationStatus.PARTICIPATED, matchingSize);  
}  
  
private Participation create(Room room, Member member, MemberRole memberRole, ParticipationStatus participationStatus, int matchingSize) {  
    Participation participation = participationRepository.save(new Participation(room, member, memberRole, participationStatus, matchingSize));  
    room.participate();  
    logCreateParticipation(participation);  
    return participation;  
}

이와 같이 로직을 묶어서 사용을 가능할 거 같습니다.
그냥 안으로 넣은게 다 아니야? 라고 생각할 수 있어서 좀 더 저의 생각을 쓰겠습니다.

방에 참가하는 로직에 들어가는 값은

- 자신이 어떤 역할로 참여하길 원하는지 ( 리뷰어/리뷰이 둘다, 리뷰어, 리뷰이 )
- 자신이 몇명을 해주길 원하는지
- 방장인지 / 그냥 참여하는지

라고 했습니다.
Room,Member 는 필연적입니다. ( 멤버가 방을 참여하는데 두개가 없으면...? )

어떤 역할과 방장 / 그냥 참여하는지는 미묘하게 묶여 있습니다.
이때, 방의 기본으로 참여할 때는 역할과 권한을 둘 다 받기로 했습니다.

당장은 방장이 리뷰어 이지만, 어떻게 될지 모를 뿐더러
매칭 크기 역시, 그냥 설정을 하지 않고 방에 참여할 수도 있는 확장성도 존재합니다.

두 번째는 포괄적인 경우의 기능입니다.

// RoomService
participationWriter.create(room, manager, MemberRole.REVIEWER, ParticipationStatus.MANAGER);

// ParticipationService
participationWriter.create(room, member, memberRole, request.matchingSize());

이와 같이 서비스에서 사용이 가능해집니다.

왜 DTO 로 안 묶었어?

현재 다른 곳에서는 Service 만을 위한 DTO 를 부분 부분 도입하고 있습니다.
( Input, Output 네이밍과 함께 )

Service 단에서 살아있는 DTO 이므로
도메인도 가져도 되므로, 더 효율적이라 판단은 하고 있습니다.

하지만, 당장 매개변수가 늘어날 경우는 없다고 판단 + 명확하게 여러 곳에서 사용될 기능이 아니라고 판단해 만들지 않았습니다.

서비스 계층이 서비스를 호출하는게 괜찮은가

( 이 내용은 ### 모호한 이름의 서비스 의 내용과도 유사한 의미를 내포합니다. )

7. 스케줄러에 등록된 매칭 & 종료 시간을 수정합니다.

이제, 이 7번 을 리팩토링 해보겠습니다.

방을 수정/삭제 할 때 이 스케줄러의 변경 역시도 필연적입니다.
DB 에 저장이 되었는데 스케줄러에서는 저장이 제대로 안되면 문제가 발생합니다.

그렇기에, 트랜잭션도 같은 단위로 묶여야 합니다.

...
automaticMatchingRepository.save(new AutomaticMatching(room.getId(), request.recruitmentDeadline()));
automaticMatchingService.matchOnRecruitmentDeadline(response);

automaticUpdateRepository.save(new AutomaticUpdate(room.getId(), request.reviewDeadline()));
automaticUpdateService.updateAtReviewDeadline(response);
...

이와 같이 코드를 포함 시킬 수 밖에 없을까요...?
이를 해결 하기 위해

roomAutomaticService.updateMatchingTime(updatedRoom.getId(),updatedRoom.getRecruitmentDeadline());
roomAutomaticService.updateReviewDeadlineTime(updatedRoom.getId(),updatedRoom.getReviewDeadline());

와 같이 다른 서비스로 분리해서 호출을 할 수 있습니다.

//RoomAutomaticService
@Transactional  
public void updateMatchingTime(long roomId, LocalDateTime recruitmentDeadline) {  
    Room room = roomReader.find(roomId);  
    AutomaticMatching automaticMatching = automaticMatchingReader.findWithRoom(room);  
  
    automaticMatchingWriter.updateTime(automaticMatching, recruitmentDeadline);  
    automaticMatchingScheduler.modifyTask(room);  
}

기존 Room 을 넘기지 않고, id 를 넘기고 재 조회를 하는 이유는
다른 컨트롤러 또는 로직에서도 재사용이 가능하게 하기 위함입니다.

이제 해결이 된 걸까요? 제 관점은 조금 다릅니다. 🙂

서비스가 다른 서비스 계층을 사용하는건 뜨거운 논쟁이 될 수 있습니다.
( 이에 대해서는 # BE | Service에서 Service를 의존할까 Repository(Dao)를 의존할까 예전 선배 기수도 다룬 적이 있는 내용입니다. )

제 의견으로는 정말 온전한 서비스 계층이 된다면 다른 서비스 계층을 호출할 일이 없다고 생각이 듭니다. ( 물론, 파사드 패턴이라면 모르겠지만 )

위의 코드도

// AutomaticMatchingWriter
public AutomaticMatching create(Room room) {  
    AutomaticMatching entity = new AutomaticMatching(room.getId(), room.getRecruitmentDeadline());  
    automaticMatchingScheduler.matchOnRecruitmentDeadline(entity);  
    return automaticMatchingRepository.save(entity);  
}

위와 같이 Writer 가 Scheduler 까지 같이 가지면 더 의도적이게 됩니다.
( 똑같은 생명주기라고 생각하기에 )

단순히, 도메인을 CRUD 하는게 아니라 비즈니스 로직 수행을 위해 구현 계층을 쌓아간다고 생각하면 훨씬 쉽게 만들어 갈 수 있을겁니다.

결론

리팩토링은 당장 효과를 보기 어려울 수 있습니다.
( 오히려, 성능을 떨어뜨리거나 에러를 유발할 가능성이 더 높구요 🥲 )

리팩토링을 해도, 코드가 더러워지는게 두려워 Reader 와 Writer 의 API 를 하나 뚫을 때는 계속 생각을 할 수 있습니다.
( 이게 해당 Reader 에 있어야 하는 코드인가? 재 사용 하려면 어떻게 해야 할까? )

이런 요소들이 하나 하나가 쌓여서 코드를 더욱 쉽고 수정하기 용이하게 해줍니다.

175 줄 이였던 코드는 79줄 까지 줄어들었습니다!
( 라인이 줄어드는게 꼭 좋은 점은 아닐 수 있습니다. 물론 )

기능이 늘어남에 따라 코드가 더욱 비대해지고, 더러워 질 수 있습니다.
그때마다 코드에 관심을 가지고, 의식적으로 변경을 해야 할 겁니다.
( 다음에 기능 확장하거나, 에러를 마주 쳤을 때 한숨을 쉬지 않으려면 )

위 내용들은 https://github.com/woowacourse-teams/2024-corea 에서 적용하는 내용들입니다.

다음은 한참 나중에 다루겠지만 검증을 외부로 분리하기 , 외부 API & 테스트 관리하기 그리고 비동기 역할에 맞게 테스트하기 정도에 대해 다뤄볼거 같네요 🙂

감사합니다!