리팩토링? 어렵지 않아요 😎

🙂 심심함이 불러온 대형 작업..

지금 회사에 합류한지 약 2개월 반 정도의 시간이 흘렀을 때, 팀 리빌딩으로 인해 하반기 feature는 많지 않았고 무료하고 심심했던 저는 수습사원의 패기로 강력한 주장을 하게 되었는데..

팀에서 모두가 기피하던 저주받은(?) 프로젝트가 있었는데, 그 프로젝트를 리팩토링하고 조금 늦었지만 TDD(Test 뒷전 Development)를 해보겠다고 외친것이죠..

감사하게도 아무도 반대하는 분이 없었고, 속전 속결로 그 일을 맡아서 진행하게 되었습니다.
Jira Ticket 을 제게 할당하면서 파트장님은 정말 괜찮겠어요? 라는 걱정을 해주셨지만.. 신입의 패기는 무모하기에 "할 수 있다" 라는 말만 도돌이표처럼 반복해나갔습니다.

이 패기가 어떤 재앙을 부를지 모른채 말이죠..

😌 생각보다 핵심적인 서비스를 담당하게 되다

제가 다니는 회사는 협업툴을 B2B SaaS 형태로 제공하고 있습니다.
그 중에서도 가장 사용성 개선이 필요했고, 가장 많이 사용하는 기능인 Drive-Service의 전면 리팩토링을 진행하게 된 것입니다.

리팩토링을 하기로 마음을 먹었다면 분명 어떤 점에서 그렇게 느꼈는지 개선 방안이나 현재 문제점에 대해서 알고 있을 것입니다.

지금부터 어떤 문제점을 느꼈고, 어떤 방식으로 이 문제를 해결해 나가고 있는지 함께 나눠보고자 합니다.

😈 계층을 오염시키는 수많은 코드들

제가 가장 먼저 주목한 문제점으로는, Presentation-Layer에서 너무 많은 일을 하고 있다는 점이었습니다.
Presentation-Layer에서 Business 규칙을 검증하고 이벤트를 보내고 심지어 트랜잭션까지 관리하고 있는 코드도 존재했습니다.

@PatchMapping(path = ["/{id}"])
    @Transactional(readOnly = false)
    fun update(
        @PathVariable id: Long,
        @Valid @RequestBody body: Request,
    ): Any? {
        // Business Rule Validation
        checkMembers(....)

        eventPublisher.publish(event)
        return Any()
    }

Controller에서 회원과 드라이브가 유효한지 검사하는 private function을 관리하고 있었고, 이는 계층을 오염시키는 문제라고 보였습니다.
두 번째로는 드라이브의 변동으로 인해 이벤트를 발송할때 eventPublisher가 Kafka를 직접 이용하고 있었으므로 네트워크 문제 등으로 예외가 발생하면 Client의 응답 지연현상이 일어날 것으로 판단했습니다.

비즈니스 규칙에 대한 검증은 Business-Layer에서 책임지고, 이벤트 발송은 서비스 계층에서 해주되 직접적인 Kafka 통신을 하는 구현체를 쓰지 않고 ApplicationEventPublisher를 통해 내부 이벤트를 먼저 발송하기로 결정했습니다.

🙄 추상화의 부재

이 서비스는 생각보다 오랜 기간에 걸쳐서 개발되어 왔고, 이로 인해 꽤나 많은 기능을 제공하고 있습니다.
기능이 많아지고 하나의 서비스의 크기가 커져갈 수록 유지보수성이 떨어져간다고 느꼈습니다.

현재 Service 클래스들이 전부 구현체만 있고 Controller는 서비스의 세부 구현에 흔들릴 수 밖에 없는 구조로 개발되어 있습니다.

따라서 회원이 할 수 있는 행위별로 Atomic 하게 UseCase를 나눠서 최소한의 책임만 가진 Service 구현체를 만들고 Controller는 추상화 된 고수준 모듈인 UseCase를 의존하도록 변경하기로 결정했습니다.

@Service
@Transactional(rollbackFor = [Exception::class])
class DriveService(
    private val repository: Repository,
    private val thumb: Thumb,
) {
    private val log = KotlinLogging.logger {}

    @Autowired
    private lateinit var service1 : Service1

    @Autowired
    private lateinit var service2 : Service2

    @Autowired
    private lateinit var service3 : Service3

서비스가 커져가면서 한 서비스 클래스에 만든 책임을 부여하게 되고, 이로 인해 각 서비스 클래스들 간의 참조가 깊어지면서 순환 참조 문제도 발생하게 되었습니다.

Command와 Query를 분리하지 않음으로 인해서, 단순 조회만 해도 괜찮은 함수의 경우에는 매번 @Transactional(readOnly=true) 애너테이션을 붙여줘야 하는 불편함도 있었습니다.

이런 문제로 인해 서비스를 가장 작은 단위로 분리하고, 각 구현체간의 의존을 최대한 없게 만드는 것을 목표로 잡았습니다. 또한 너무 많은 의존 관계와 순환 참조는 테스트를 작성하는데 어려움을 줍니다.

테스트 코드를 작성하다 보면, 테스트 하기 어려운 구조일수록 전체적인 설계나 구조에 문제가 있을 수 있다는 것을 느끼면서 이와 같은 결정을 하게 된 것입니다.

😐 어떻게 분리해 나갈 수 있을까?

회사의 프로젝트는 Gradle 멀티 모듈로 구성되어 있기 때문에 각 프로젝트간 서로의 의존 문제가 있었고, 객체를 변환하는 Mapper 클래스들이 API 프로젝트에 포함되어 있어서, Domain-Service 프로젝트는 Mapper를 참조할 수 없는 문제도 발생했습니다.

따라서 Gradle 멀티 모듈들이 서로간의 의존성으로 인해 또 순환참조를 겪게 되어서 논리적인 개념에 비추어 볼때 하나로 보아도 되는 프로젝트들은 병합하고, 사용하지 않는 프로젝트는 삭제하여 이 문제부터 해결했습니다.

첫 번째 목표는 "작은 서비스 구현체부터 변경하되, 비즈니스 로직을 변경하지 않고 그대로 유지한다." 였습니다.
업무 파악을 하면서 진행해야 했던 리팩토링이었기 때문에 비즈니스 로직에 대한 이해를 다 하지 못하였기에 원본을 유지하고 인터페이스를 작게 유지하는 것에 초점을 두었습니다.

두 번째 목표는 "파일을 잘게 쪼개는 만큼, 쉽게 파일을 찾을 수 있게 하자." 입니다.
파일이 UseCase 단위가 작아짐에 따라서 기하급수적으로 늘어나게 되고 이로인해 너무 많은 파일이 생기게 됩니다.

그렇다면 유지보수성 측면에서 파일을 찾기 어려워 질 것이므로, 행위에 대한 prefix와 객체 이름에 대한 suffix 규칙을 만들고 폴더 구조 개편을 했습니다.

<행위><도메인><UseCase | Service> 와 같은 규칙으로 누구라도 알아볼 수 있게 명명 규칙을 만들었습니다.

또한 드라이브 안에 있는 폴더, 휴지통 등의 포함 관계는 최상위 폴더는 드라이브로 하고 폴더와 휴지통에 대한 폴더를 하위 폴더로 둠으로써 어떤 작업을 할때 작업 위치를 쉽게 찾을 수 있도록 개선했습니다.

😪 큰 위기를 맞이하다..

파트장님의 걱정을 뒤로하고 신규 멤버의 패기로 시작한 리팩토링은 큰 고난을 겪게 되었습니다.
떼어 내면 떼어 낼 수록 서로 간의 강한 참조로 인해 서버조차 실행되지 못할 정도의 문제를 가지고 있던 것입니다.

기존 필드 주입을 생성자 주입으로 변경하면서 순환 참조 문제가 발생하여 서버가 실행되지 않는 문제가 발생하게 되고, 서버를 켜려면 모든 코드를 전부 수정해야만 가능하게 되었습니다.

실행도 안해보고 어떻게 믿고 이 코드를 수정할 수 있을까라는 고민에 휩싸일 때쯤 좋은 아이디어가 생각이 나서 도전해보게 되었는데, 그건 바로 기존 파일을 직접 수정하지 않고 복사하여 ...Temp와 같이 만든 후에 전부 분리가 되면 원본을 삭제하고 수정된 파일을 API Controller 에서 의존하도록 하는 방법이였습니다.

이 방법을 사용하여 약 3000줄 이상의 코드를 에러 없이 안전하게 리팩토링 하였고, 이제는 검증의 시간이 다가오고 있었습니다.

비즈니스 로직을 고도화하기 전에 서비스 전체의 흐름을 이해하고 현재 동작하고자 하는 방식을 테스트로 작성하여 추후에 고도화 또는 로직 변경시 검증이 될거라고 생각했습니다.

🙄 Test Code를 작성하다!

테스트 코드를 작성해야 하는데 기존 테스트 코드 스펙은 Spock을 이용해 작성하고 있었습니다.
사내 구성원들에게 Spock이나 Kotest 모두 익숙한 방법은 아니였고, 모든 업무를 제가 정의하고 진행하라는 팀장님의 배려가 있었기 때문에 Test Code 작성 표준을 만들기에는 Kotest가 직관적이라고 생각했습니다.

따라서 Kotest + Mockk를 활용해서 테스트 코드를 작성하기 시작했습니다.
수 많은 클래스들에 대해서 테스트를 작성하려니 너무 막막했고 지루했습니다. 그래서 계층별로 가장 하위 계층부터 테스트를 하기로 결정했습니다.

첫 번째 대상은 Repository 입니다.
Repository Test는 현재 사용하는 DB인 MySQL 8.0 방언에 맞춰서 Inmemory-H2 Database를 활용해서 테스트를 진행했습니다.

	this.Given(".....") {
            val drive = fixture<Drive> {
                ...
            }

            driveRepository.save(drive)

            val authorityList = listOf(
                fixture<Authority> {
                   ...
                },
            )

            authorityRepository.saveAll(authorityList)

            When("삭제를 시도하면") {
                val deletedRows = authorityRepository.func(drive.id!!)
                val findAuthorityCount = authorityRepository.func()

                Then("정상적으로 삭제되어야 한다") {
                    deletedRows shouldbe 1
                }
            }
        }

Kotest의 BDD 스펙은 직관적으로 테스트의 목적을 이해할 수 있도록 되어있고, 가독성이 매우 좋습니다.
따라서 저는 BDD 스펙을 활용해서 테스트를 작성하고, Kotest의 infix function을 활용해 마치 글을 읽는 것처럼 편안한 테스트를 작성하도록 노력했습니다.

이렇게 100+ 개 이상의 Repository Test를 작성하고, 현재는 Business Layer Test를 작성하는 중입니다.
0개 -> 160개의 테스트를 작성하여 추후 Spring Rest Docs + Swagger를 이용해 문서 자동화를 이루려는 큰 목적을 가지고 있습니다.

테스트를 작성하면서 가장 들었던 고민은, 단순한 호출에 대한 것은 테스트 코드를 작성하지 않을 것인가?
어떤 예외 케이스까지 확인할 것인가? 그리고 도메인 객체 안에서 발생하는 예외가 서비스로 전파되는 것은 서비스 클래스 테스트에서 검증해야 하나?

이런 고민들이 시작되었습니다. 결론적으로 저는 아래와 같은 답을 내렸습니다.

현재는 단순 호출에 불과할 수 있지만 추후 로직의 변경 또는 추가가 있을 수 있으므로 테스트는 무조건 작성한다.
서비스 코드 안에서 직접 던져지는 예외라면 서비스 클래스 테스트시에 검증해야 한다.
도메인 객체 안에서 예외를 던져, 서비스 클래스에 전파되는 경우에는 도메인 객체 테스트시 검증해야 한다.

제 나름대로의 "단위 테스트" 라는 작은 개념의 테스트를 어떻게 할까라는 고민에서 나온 방법이였습니다.
Repository 계층에서 검증된 예외 발생을 Service 계층을 테스트하는데 굳이 Mocking 하지 않고 또 검증한다면 중복 검증이라는 생각이 들었습니다.

Mockist vs Classist는 항상 나오는 개념의 논쟁이지만 저는 하이브리드에 속합니다.
Repository와 같이 DB와 직접 통신하는 계층은 직접 Test 하되, 서비스 클래스와 같이 비즈니스 규칙을 적용하면 다른 컴포넌트는 Mocking 하여 사용합니다.

제 결론은 이렇게 검증의 목적과 그 대상을 명확하게 하여 세세하지만 컴팩트한 테스트 코드를 작성해서 프로덕트의 안정성을 확립하고 END-TO-END Test 를 통해서 최종 검증은 실제 상황처럼 진행하는 것입니다.

🫡 이벤트 발행을 내부 이벤트를 먼저 통하게 변경하다!

	eventPublisher.publishEvent(
            Event(
                obj = drive,
                type  = EventType.REGISTERED,
                id = id,
                response = response
            )
        )

이렇게 ApplicationEventPublisher를 통해서 이벤트를 발행하면, 내부 이벤트 Listener는 그 이벤트를 수신하게 됩니다.

	@Async
    @TransactionalEventListener(Event::class)
    fun handleEvent(event: Event) {
        try {
            logger.info { "[Consume Event] - ${event.type.value.uppercase()}_EVENT, Event Object = [$event], Time = [${generateLocalDateTimeForLogger()}]" }
            logger.info { "[Send Event] - Send ${event.type.value.uppercase()}_EVENT to Kafka, Time = [${generateLocalDateTimeForLogger()}]" }

            val eventPayload = Event(..., event.target)
                .eventType(event.type)
                .teamId(event.id)
                .build(event.response)
                
            kafkaTemplate.send(payload)

            logger.info { "[Send Finished] - Finished to Send ${event.type.value.uppercase()}_EVENT, Time = [${generateLocalDateTimeForLogger()}]" }
        } catch (e: Exception) {
            logger.error { "[Send Failed] - Fail to send ${event.type.value.uppercase()}_EVENT, Exception = [${e}], Time = [${generateLocalDateTimeForLogger()}]" }
        }
    }