리팩토링 회고 - 검증이라는 관심사의 분리와 복잡성 해결을 위한 노력

0. 개요

한 프로젝트의 취약점을 분석하고 Hotfix를 해야하는 상황이 있었습니다. 그 프로젝트의 코드의 복잡도가 너무 심하다고 느껴 잘못 건드렸다가 더 큰 오류를 낼 것 같을 정도로 고칠 엄두가 나지 않았는데요. 여러 부분에서 리팩토링이 필요했습니다. 본 글에서는 여러 리팩토링 방식 중 검증 방식을 분리하여 단순화하는 내용을 다룹니다. 개선점을 도출하고 이를 해결하는 방안으로서 시도한 과정 및 결과를 소개합니다.

 

• 예시 코드는 실제 코드가 아닌 컨셉 코드로 대체하였고 기타 네이밍도 추상화하여 작성하였습니다.

 

1. 목차

0. 개요
1. 목차
2. 배경
   1. 기존 코드
   2. 개선점 도출
3. 해결 아이디어
4. 1차 변경 → 애노테이션 방식
5. 2차 변경 → Optional을 이용한 방식
6. 결론

 

2. 배경

데이터 처리 서비스의 DataHandler 클래스에서 데이터 처리 로직이 구현되어 있습니다. 이 클래스는 다양한 검증 로직을 포함하고 있습니다. 검증 로직을 보다 효율적으로 수행하기 위해 고려해볼 점들이 있었습니다.

 

1. 기존 코드

GetPriceHandler 클래스는 충전 가격과 관련된 정보를 검증하고 처리하는 역할을 담당합니다. 클래스 내에서는 AssertUtil.isTrue()나 AssertUtil.isNull()과 같은 메서드를 통해 검증 로직을 수행하고 있습니다.

DataHandler 클래스는 데이터 처리와 관련된 검증 로직을 수행합니다. 클래스 내에서는 ValidationUtil.checkTrue()나 ValidationUtil.checkNull() 같은 메서드를 통해 검증을 수행합니다.

 

@Service
@Slf4j
public class DataHandler {
    private final DataServiceClient dataServiceClient;
    private final DataAnalysisClient dataAnalysisClient;
    // 클래스 구현 ...

    public String processData(RequestPayload requestPayload) {
        // 아주 길고 긴 데이터 처리 로직 ... 
        ValidationUtil.checkTrue(validCondition, "Invalid condition");
        // 로직 계속 ... 약 100 줄... 
    }
}

 

 

     /**
     * 조건이 참인지 검사하는 함수 조건이 거짓일 경우 예외 발생
     * <pre class="code">Assert.isTrue(i &gt; 0, "The value must be greater than zero");</pre>
     *
     * @param expression a boolean expression
     * @param message    the exception message to use if the assertion fails
     * @throws Exception - ILLEGAL_ARGUMENT
     */
     public class ValidationUtil {
       try {
        // 검증 메서드 구현 ...
    } 

    public static void isTrue(boolean expression, String message) {
        try {
            Assert.isTrue(expression, message);
        } catch (IllegalArgumentException e) {
            raiseException(ResponseCode.ILLEGAL_ARGUMENT, e.getMessage());
        }
    }

2. 개선점 도출

가독성

• 검증 알고리즘을 ValidationUtil 이라는 클래스에 템플릿으로 만들어 어플리케이션에서 전역적으로 사용하고 있습니다. 이 템플릿은 전체 어플리케이션에 걸쳐 검증 로직을 통합하므로 검증 로직을 쉽게 구현할 수 있다는 이점이 있습니다. 그러나 메인 코드의 흐름에 이러한 검증 로직이 자주 나타나기 때문에 메인 로직을 파악하기 어려워졌습니다.
  • 예를 들어, 아래와 같이 ... 
    이와 같은 코드는 중요한 비즈니스 로직 사이에 껴있어서 읽는 사람이 로직을 따라가기 힘들게 만듭니다.
  • final ResponseEntity<DTO> result = client.search(DTO.builder() .someField(DTOV1.getSomething()) .someField(something) .build()); ValidationUtil.isTrue(HttpStatus.OK.equals(result.getStatusCode()), ILLEGAL_EXCEPTION); final DTO dto = gson.fromJson(gson.toJson(result.getBody()), DTO.class); log.info("some logs..."); ValidationUtil.isTrue(HttpStatus.OK.equals(result.getStatusCode()), ILLEGAL_EXCEPTION);

테스트 코드

• 또 다른 문제는 테스트 코드 작성의 어려움이었습니다.
• 현재의 SomethingHandler 클래스에서는 다양한 검증 로직(ValidationUtil.isTrue, ValidationUtil.isNull 등)이 메인 로직에 직접 삽입되어 있습니다. 이로 인해 단위 테스트를 작성할 때에도 복잡한 Mocking과 Stubbing 작업이 필요하며, 여러 케이스에 대한 테스트를 효과적으로 수행하기 어렵습니다.
• 예를 들어, result.search() 메소드가 HttpStatus.OK를 반환하는지, 그리고 반환된 결과가 올바른지를 확인하려면 해당 메소드와 관련된 모든 부분을 Mocking 해야만 합니다. 이는 테스트 코드의 복잡성을 높이며, 실제 로직에 대한 견고한 테스트를 방해합니다.

 

진단

문제의 핵심은 메인 로직에 검증 로직이 깊게 결합되어 있다는 점이었습니다. 이런 강한 결합성 때문에 단위 테스트 작성이 어렵고, 코드를 처음 접하는 개발자가 로직의 플로우를 따라가는 데 많은 시간이 들게 만들었습니다.

3. 해결 아이디어

기능별 메서드로 분리하자

• 먼저 handle()메서드가 담당하는 역할의 책임을 분산하는 것입니다. 메서드가 하나의 기능만 담당하도록 기능 단위별로 분리합니다.
• 예를 들어, result 관련 로직을 처리하는 로직과 someModelType 검증을 처리하는 로직을 다음과 같이 메서드로 추출할 수 있습니다.
• public ResponseEntity<Dto> handleResult(Payload payload, Dto dto) { // result 관련 로직 } public ResponseEntity<SomeVo> handleModelType(Dto dto) { // modelType 관련 로직 }

• 이렇게 한 단위의 액션에 대해서 메서드로 나누어서 검증 책임을 메서드로 분산시킵니다. 메인 로직에서는 다음과 같이 간단히 메서드를 호출하여 결과만 확인할 수 있습니다.
• public String handle(OcppRequestPayload ocppRequestPayload, String data) { // ... ResponseEntity<DTO> result = handleResult(Payload, Dto); ResponseEntity<VO> otherResult = handleModelType(dto); // ... }

분리된 메서드에서 검증을 수행하자

• 위에서 작업한 메서드 분리를 통해 검증 로직을 각 메서드 안에 캡슐화할 수 있습니다.
• 각 메서드 내부에서 필요한 검증을 수행하되, 유효하지 않은 경우에는 예외를 발생시켜 메인 로직에서는 이를 캐치하여 적절히 처리할 수 있다. 예를 들어, handleResult 메서드 내에서 다음과 같이 검증을 수행할 수 있습니다.
• public ResponseEntity<DTO> handleResult(Payload payload, Dto dto) { // ... ValidationUtil.isTrue(HttpStatus.OK.equals(result.getStatusCode()), ILLEGAL_EXCEPTION); // ... }
• 이렇게 함으로써 메인 로직에서는 검증 로직을 신경 쓸 필요가 없어져 가독성과 유지보수성이 향상됩니다. 또한, 각 기능별 메서드는 자체적으로 검증 로직을 포함하므로 테스트 코드 작성도 용이해집니다.

다음 장에서는 이 아이디어를 확장시켜 검증 로직을 좀 더 개선해볼 수 있는 두 가지 안에 대해 제시합니다.

4. 1차 시도 → 애노테이션을 사용한 검증 로직 분리

AOP를 활용하여 검증 로직을 별도의 Aspect로 분리하고 이를 애노테이션을 통해 캐치하는 방식으로 검증 로직을 분리할 수 있을 것이라 생각한 것이 첫 번째 시도입니다.

개선한 부분과 기대 효과

애노테이션을 사용한 검증 로직의 분리는 주로 Aspect-Oriented Programming (AOP)를 활용합니다. 예를 들어, 다음과 같은 코드에서는 @ValidChargeFee 애노테이션을 사용하여 검증 로직을 단순화했습니다. 이 애노테이션은 ChargeTxCalculationRequestDTOV1 객체가 올바르게 구성되었는지를 검증하는 작업을 수행합니다.

 

@ValidateSomething
public String retrieveSomething(Dto response, String string) {
    return SomethingResolver.getSomething(something, response, something2);
}

가독성은 확실히 향상된다

애노테이션을 보면 어떤 검증이 필요한지 명확하게 알 수 있으며, 검증 로직 자체는 별도의 Aspect 클래스에서 관리됩니다. 이로 인해 메인 로직은 더 깔끔해지고 복잡성이 줄어들죠.

 

@Aspect
@Component
public class ValidateSomethingAspect {

@Around("@annotation(ValidateSomethingId)")
public Object validateChargeTransactionId(ProceedingJoinPoint joinPoint) throws Throwable {
    return validate(joinPoint);
}

private Object validate(ProceedingJoinPoint joinPoint) throws Throwable {

    if (joinPoint== null) {
        throw new UserHandlerException(ResponseCode.ILLEGAL_EXCEPTION);
    }
    Object[] args = joinPoint.getArgs();
    for (Object arg : args) {
        if (arg == null) {
            throw new UserHandlerException(ResponseCode.ILLEGAL_EXCEPTION);
        }
    }

    return joinPoint.proceed();
}
}

 

제약 사항

1. private 메서드에는 Spring Aop가 적용되지 않는다!

프록시 기반의 AOP는 객체에 대한 요청을 가로채는 프록시 객체를 생성하여 작동하는 방식입니다. 그러나 이 프록시 객체는 외부에서 접근 가능한 public 메서드에만 적용됩니다. 따라서 private 메서드에는 AOP가 적용되지 않아, 테스트를 위해서 객체의 내부를 노출시켜야 하는 주객전도 상황이 발생하였습니다.

 

AspectJ는 바이트 조작을 통해 더 깊은 레벨에서 AOP를 적용할 수 있으므로 private 메서드에도 Aspect를 적용할 수 있지만 불필요한 복잡성이 증대되는 또 다른 문제가 있었습니다.

 

2. 메서드 내부 호출에서도 마찬가지로 적용되지 않는다!

마찬가지로 프록시라는 특성에서 기인하는 문제입니다. 객체 내부에서 메서드를 호출할 때도 프록시를 거치지 않기 때문에 AOP 애노테이션이 적용되지 않습니다. 기능별 단위 메서드에서 각각을 검증하기 위해서는 잦은 내부 호출이 예상되는 상황이므로 이와 같은 문제에 대한 고려가 필요했습니다.

 

3. Springboot 테스트에서 일반적인 mock 테스트로는 aop 적용 여부가 테스트되지 않는다!

일반적인 mock 테스트에서는 mock 객체가 실제 객체가 아니므로 프록시를 거치지 않습니다. 마찬가지의 이유로 AOP 적용 여부를 테스트하기 어려워진다는 것입니다. 일례로 다음과 같은 코드는 다음과 같은 이유로 테스트에 통과하지 못합니다.

 

Expected java.lang.NullPointerException to be thrown, but nothing was thrown.
org.opentest4j.AssertionFailedError: Expected java.lang.NullPointerException to be thrown, but nothing was thrown.

 

 

@Test
@DisplayName("일반화된 메소드 이름이 null 값을 반환하는 경우, 특정 로직 검증")
void testGeneralizedMethodNameWithNullResponse() {
    // given
    GeneralRequestPayload requestPayload = GeneralFixtureCreator.getGeneralRequestPayload();
    GeneralServiceResponseDTO generalServiceResponse = GeneralFixtureCreator.getGeneralServiceResponse();
    GeneralTemplateResponseDTO generalTemplateResponse = GeneralFixtureCreator.getGeneralTemplateResponse();
    GeneralFeatureCodeVo generalFeatureCode = GeneralFixtureCreator.getGeneralFeatureCode();

    when(generalClient.createStart(anyString(), any())).thenReturn(null);
    when(generalResolver.getId(anyString().length(), any(), any())).thenReturn(null);

    // when & then
    assertThrows(NullPointerException.class, () -> {
        generalFeignSender.sendGeneralizedMethod(requestPayload, generalServiceResponse, generalTemplateResponse, generalFeatureCode);
    });
}

 

 

진단

결론적으로 AOP 기반의 애노테이션 검증 방식을 사용하면 가독성은 크게 향상되지만 부가적인 기타 이슈들이 제기되었습니다. 각 기능 별로 단위 테스트를 하기도 어렵고 테스트 코드 때문에 프로덕션 코드가 변경되어야 하는 문제도 생깁니다. 위에서 언급한 제약 사항 때문에 이 방식은 적용하기 힘들다고 판단 내렸습니다.

 

5. 2차 시도 → Optional을 이용한 검증 로직 분리

개선한 부분과 기대 효과

Optional을 사용하면 null 체크와 같은 검증 로직을 명시적이고 간결하게 작성할 수 있습니다. 또한, Optional 메서드의 체인을 통해 코드의 가독성을 유지하면서도 로직의 흐름을 더욱 명확하게 표현할 수 있습니다.

예시 코드 1

    private String retrieveTransactionId(ServiceResponseDTO serviceResponse, String identifier) {
        return transactionTypeResolver.getTransactionId(identifier.length(), serviceResponse, identifier)
            .orElseThrow(() -> new CustomException(ResponseCode.GENERAL_ERROR_CODE));
    }

 

 

여기서는 .orElseThrow()를 통해 getTransactionId 메서드가 null을 반환할 경우 즉시 예외를 발생시킵니다. 이러한 방식으로 검증 로직을 명확하게 할 수 있습니다.

 

예시 코드 2

    private <U extends ResponseDto> List<U> processResponseAsList(Class<U> type, ResponseEntity<?> authorizeResult) {
        return Optional.ofNullable(authorizeResult)
            .filter(response -> HttpStatus.OK.equals(response.getStatusCode()))
            .map(response -> (Page<?>) response.getBody())
            .map(Page::toList)
            .flatMap(list -> Optional.ofNullable(GsonGenericConverter.convert(gson, list, type)))
            .orElseThrow(() -> new UserHandlerException(ILLEGAL_EXCEPTION));
    }

 

이 코드에서는 다음과 같은 과정을 거치고 있습니다.

• Optional.ofNullable()을 통해 authorizeResult가 null이 아닌 경우에만 로직이 실행됩니다.
• filter()를 사용해 HTTP 상태 코드가 OK인 경우만 다음 단계로 넘어갑니다.
• map()을 통해 본문을 Page 객체로 변환합니다.
• 다시 map()을 통해 Page 객체를 List로 변환합니다.
• flatMap()과 Optional.ofNullable()을 이용해 Gson 변환을 시도하고, 실패하면 예외를 던집니다.

 

테스트 코드에서도 검증 로직을 작성하기가 수월해졌죠.

 

@Test
@DisplayName("특정 메서드가 null을 반환할 때 예외 처리가 정상적으로 작동하는지 검증한다.")
void testMethodWithNullResponse() {
    // given
    RequestPayloadType requestPayload = FixtureCreator.getRequestPayload();
    ServiceResponseType serviceResponse = FixtureCreator.getServiceResponse();
    TemplateResponseType templateResponse = FixtureCreator.getTemplateResponse();
    FeatureCodeType featureCode = FixtureCreator.getFeatureCode();

    when(client.createStart(anyString(), any())).thenReturn(null);
    when(resolver.getTransactionId(anyString().length(), any(), any())).thenReturn(Optional.empty());

    // when & then
    assertThrows(ExpectedExceptionType.class, () -> {
        feignSender.sendMethodWithTemplate(requestPayload, serviceResponse, templateResponse, featureCode);
    });
}

 

AOP를 적용할 때와는 다르게 @SpringBootTest 통합테스트를 할 필요가 없어졌으므로 단순 Mock 객체를 사용할 수 있습니다.

 

@ActiveProfiles("test")
@ExtendWith(MockitoExtension.class)
class FeignSenderTest {

@InjectMocks
private FeignSender feignSender;

@Mock
private CalculatorClientV1 calculatorClientV1;

@Mock
private AuthorizedUserTypeResolver authorizedUserTypeResolver;

// ... 

 

6. 결론

복잡한 코드를 대할 때 첫 번째로 리팩토링 할 수 있는 접근은 큰 그림에서 할 일들을 조금씩 분리해보는 것이 아닐까 싶습니다. 물론 클래스와 메서드를 잘 분리해서 작성하는 것은 처음 설계 때부터 잘 해야 하죠. 그러면 애초에 이 케이스에서 겪었던 것과 같은 복잡도의 문제가 발생하지 않습니다. 본 작업을 작성한지가 조금 오래되었는데 본 케이스에서 발생한 검증에 대한 문제는 Feign 요청을 별도의 패키지에서 validation과 함께 관리하도록 분리하자 자동적으로 해결된 경험을 했습니다.

 

그 다음 수순이 Optional과 같은 좀 더 함수형스러운 (?) 문법을 잘 활용하는 것입니다. if문 쓰지 않으려고 하기, depth가 늘어나면 위험 신호로 여기고 뭔가 개선 방법을 찾아보기 등 단순하고 좋은 코드를 쓰려면 실생활에서 "그래야 하겠다는 의지"부터 습관이 되어야 하는 것 같습니다.