Transactional의 Self Injection이 올바른가

TL; DR

올바르지 않고, 쓰지마. Self Injection은 기본적으로 Spring이랑 Spring Boot에서 기본 논점으로 막고 있음. 왜냐하면 무한 참조에 위험성을 가지고 있고 @Lazy를 위한 방식으로 처리할 수는 있는데.. 굳이 그렇게 쓰는거 자체가 전자에 발생할 위험성을 포함하고 있음. 스프링 부트의 경우는 아예 기본으로 순환 참조시 아예 Application 켜지지 않는 이슈도 있음. 아님 허용하고 Spring boot 전체가 전부 Self Injection이 가능해져야하기 때문에 비선호
기본적으로 Spring에서 권고하지 않는 사항이면, 안하는게 좋다고 생각함. 프레임워크기 때문에 굳이 벗어나는 행동을 하는건 좋지 않다고 생각함

왜 이 주제가 생각났는가?

팀내에서 소나큐브를 적용해보자는 이야기와 함께 우리 서비스에 대한 코드를 한번 소나큐브 스캐닝을 돌려서 수없이 많은 주의점을 뿜뿜하고 있는 소나큐브를 둘러보던 도중 한가지 특이한 걸 발견했다.
https://cloud-ci.sgs.com/sonar/coding_rules?languages=java&q=%40Transactional&open=java%3AS6809

Methods with Spring proxy should not be called via "this"

뭐 Spring Proxy 쓸때 당연한거긴하지만.. 클래스내 트랜잭션을 겹처서 쓰거나.. 아니면 트랜젝션이 없는 메소드 안에 트랜젝션이 있는 걸 불러와야하는 경우... this.000method() 해봐야 동작을 하지 않음.
위에서 말했던 것처럼 이런 건 내 기억에서는 트랜젝션이 중첩되는 경우가 아닌 이상 그냥 트랜젝션 안에서 최대한 온몸 비틀기를 했다.
문제는 트랜젝션의 중첩과 같은 경우인데.. 요약하면 부모 메소드는 @Transactional readonly 상태 → 자식 메소드는 @Transactional 한 상황. 그렇다면 Class 분리를 통해서 자식 메소드를 분리하고 부모메소드에 Class 주입을 해서 부모는 readonly 상태 → 클래스 주입된 자식 메소드는 트랜젝션이 정상 동작한다고 생각했고 실제로도 그렇게 개발을 종종 하기 때문에 난 항상 이렇게 쪼개왔었다.

근데 항상 어느정도의 의문이 있었는데.. 그러면 이런 중첩 트랜젝션의 경우 매번 클래스를 분리하라고? 예를들어서 단순하게 이 엔티티자체의 서비스와 크게 다르지도 않는데 중첩 트랜젝션할때마다 클래스를 쪼개서 하려면 트랜젝션을 위한 트랜잭션 전용 class가 하나 더생기는게 아닌가...? 뭐 이정도로 생각했다.

근데 위에서 봤던 링크는 이런 의견을 정면으로 반박하는거 같았다.

링크의 내용이 특별한게 뭐가 있었지?

링크의 핵심은 다음과 같다.

Compliant solution

@Service
public class AsyncNotificationProcessor implements NotificationProcessor {

  @Resource
  private AsyncNotificationProcessor

  @Override
  public void process(Notification notification) {

    asyncNotificationProcessor.processAsync(notification); // Compliant, call via injected proxy

  }

  @Async
  public processAsync(Notification notification) {
    // ...
  }
}

→ 자 그럼 다시 생각해보자 위에서 발생한 동일한 이슈를 다시 치환해서 보자면? Self Injection을 해주면 프록시 문제가 풀리고 자동으로 중첩 트랜젝션도 엄밀히 따지면 한 트랜잭션 내부에서 처리 가능하다는 거 아님? 사고가 흘러갔다.
결국 우리가 제일 문제가 된건...?

프록시 객체가 있음. → 근데 중첩 메소드를 사용하면 프록시 객체 내 프록시 한겹으로 더 감싸는 상태가 될수 없음 → injection 불가 AOP 형태에서는 중첩 트랜젝션이 불가능한데...
이게... self Injection을 통해서는 가능할 수도 있다는 것? 처럼 느껴졌다.
왜냐하면 어쨋든 자기 자신도 Bean 자체에 Class가 등록되있기 때문에 사실 그 자기 자신 클래스를 그대로 받아서 처리하면 되는거 아냐?? 라고 나름 근들갑 떨고 있는 셈이다.

그럼 테스트 코드를 통해 가능한지부터 확인해보자

일단 아무런 말이 필요 없다 되는지 확인하기 위해서는 그냥 중첩 트랜젝션을 사용해보면 뭐 결론 날것이다.
일단 일종의 upsert 기능을 구현해보면 중첩 트랜젝션처럼 구현해볼 수 있는데, 일종의 readonly transactional를 작성해보려했는데...
굵직한 예시만 굳이 표현하자면 대략 아래와 같은 코드로 중첩 트랜젝션을 구성하려 했다.

package com.example.self_injection;  


import jakarta.annotation.Resource;  
import lombok.RequiredArgsConstructor;  
import lombok.extern.slf4j.Slf4j;  
import org.springframework.beans.factory.annotation.Autowired;  
import org.springframework.stereotype.Service;  
import org.springframework.transaction.annotation.Propagation;  
import org.springframework.transaction.annotation.Transactional;  

import java.util.Map;  

@Service  
@RequiredArgsConstructor  
@Transactional(readOnly = true)  
@Slf4j  
public class TestService {  

    private final TestEntityRepository testEntityRepository;  
    private final TestLoggingEntityRepository testLoggingEntityRepository;  
    private final TransactionalService transactionalService;  
    private TestService testService;  

    @Autowired  
    public void setTestService(TestService testService) {  
        this.testService = testService;  
    }  

    @Transactional  
    public void save(Map<String, String> map) {  
        testEntityRepository.save(TestEntity.builder().name(map.get("name")).build());  
    }  


    public void getReadOnlyMethod(Long id){  
        log.info("Transactional ReadOnly Start");  
        TestEntity te = testEntityRepository.findById(id).orElse(null);  
        if(te == null) {  
            log.info("해당 번호는 없다. 해당 메소드 종료");  
            return;  
        }  
        TestLoggingEntity testLoggingEntity = TestLoggingEntity.builder().id(te.getId())  
                .name("123").actionName("action").build();  
        testService.upsertMethod(testLoggingEntity);  
        log.info("Transactional ReadOnly End");  
    }  


    @Transactional(propagation = Propagation.REQUIRES_NEW)  
    public void upsertMethod(TestLoggingEntity testLoggingEntity) {  
        log.info("Transactional Start");  
        testLoggingEntityRepository.findById(testLoggingEntity.getId()).ifPresentOrElse(  
                entity -> {  
                    entity.updateAction("action1");  
                    testLoggingEntityRepository.save(entity);  
                },  
                () -> testLoggingEntityRepository.save(testLoggingEntity)  
        );  
        log.info("Transactional end");  
    }  

}

하지만 해당 코드 실행은 실패한다. 에러로그는 다음과 같다.

┌─────┐
|  testService
└─────┘


Action:

Relying upon circular references is discouraged and they are prohibited by default. Update your application to remove the dependency cycle between beans. As a last resort, it may be possible to break the cycle automatically by setting spring.main.allow-circular-references to true.

Spring을 좀 해봤다 싶으면 당연하게도 이 문제가 있다는걸 인지할 수 밖에 없다. 순환 참조 오류는 당연하니 뭐 그렇다 쳐도... 왜 이걸 Spring Boot나 Spring에서 막았는지 궁금했다. 그리고 명확한 의도가 뭔지를 찾아보고 싶어졌다.

그럼 왜 순환 참조를 막았을까?

확실하게 이러이러해서 막았다. 확언까진 못찾았어도 그래도 흐름 자체는 찾았다.
나처럼 생각한 개발자들이 꽤 많았는데, 해당하는 걸 막는 거 까지는 아니여도 해당하는 이슈들을 찾았다. 아래 링크는 해당하는 circular reference를 금지하는 옵션에 대해서 만든 이슈이다.
https://github.com/spring-projects/spring-boot/issues/27652

Prohibit circular references by default · Issue #27652 · spring-projects/spring-boot

해당 링크는 Spring Boot에서 기본적으로 Circular Reference를 막는 정책을 쓰고 있는데... 문제는.. 해당하는 이슈 댓글이 엄청 달렸다. 댓글의 요지도 위에처럼 굳이.. 한 Layer를 더 만들어내는 방식이 아닌 자기 참조를 통한 proxy 방식이 "왜 틀린지?" 에 대해서 항변하고 있어 굉장히 옳은 말이라고 생각했는데 이 이슈의 결론은 결국

흠.. 전 Spring Boot 대용? Spring 가서 알아보고 오쇼처럼 들렸기 때문에 뭐 다시 Spring 가서 확인하는 수 밖에 없다고 생각했다.
직접적으로 Spring에서 뭐 Circular Reference가 뭐가 문제고.. 지양하는 바가 뭔가 뚜렸해보이진 않았는데... 이슈 몇개들로 대략적인 추론은 가능해보였다.
https://github.com/spring-projects/spring-framework/issues/28299

Clarify recommendation on self-injection for legitimate use cases (e.g. @Transactional re-entrant calls) · Issue #28299 · spri

ㄴ 이 이슈같은 경우는 결국 이 Self Injection의 합법적인 Use Case를 알려달라는 이슈였고 해당하는 이슈는 .... 황당하게도 주석하나정도로 처리되긴했었다.
https://github.com/spring-projects/spring-framework/commit/22b6d66a286256556a21d83a16e82b756b732da8

Document @Order behavior on @Configuration classes versus @Bean methods · spring-projects/spring-framework@22b6d66

주석에 이런식으로 작성이 되어있었는데

자체 주입(self-injection)은 대체 수단이라는 점에 유의하세요. 실제로는 최후의 수단으로만 자체 참조를 사용해야 합니다(예: 동일한 인스턴스에서 빈의 트랜잭션 프록시를 통해 동일한 인스턴스에서 다른 메서드를 호출하는 경우).
이러한 시나리오에서는 영향을 받는 메서드를 별도의 델리게이트 빈으로 분리하는 것을 고려하세요.

결국 이러한 문제를 자체 참조자체가 최대한 미루고 미루는 존자라는 것이고... 권고안 자체도 별로 Class 분리 방안을 찾는게 좋다는 뜻으로 보였다.
Self Injection 이러한 이슈가 생길때마다 위의 이슈로 계속 이슈로 연결해주셨던 @snicoll이 지속적으로 Self Injection에 대한 몇개 없지만, 그 적은 근거로.. "무한루프가 돌기 쉽다." 정도를 들긴했다. 그리고 해당하는 답변에서 계속 말하는건 쓰지마라! 이긴 했기 때문에 쓰진 않겠다만...
결국 스프링 자체적으로 금지 시켜놓은 가이드라고 보면 되지만... 호용성은... ? 긴 하다. 굳이.. infinite loop를 돈다고 해서 순환참조 방식을 막을 필요가 있으려나...?

애매모호한 결론

말 그대로 결론을 내리기 참으로 어려운 주제이다. 그러니까 개인 혹은 팀 자체에서 결정할 문제기 때문이라 생각한다. 이 논의 자체가 개발자들 사이에서도 논란(?)이지도 않겠지만... 해당하는 이슈를 어찌 바라봐야할지는 결정하는 사람의 몫인거 같긴했다.
내 입장은 글 적기 전까지만 해도 그냥 써도 괜찮지 않겠냐쪽이긴 했지만 + 소나큐브의 코드 수정에 대한 권위적 위치를 보고 그렇게 수정해야겠다싶어 수정하려 했지만, 글을 다 작성한 지금의 입장은 많이 달라졌다. 스프링쪽에서 굳이 권고하지않는 행동을 할 필요 없다는 글들을 좀 읽고 나니까 안하는게 낫겠거니하면서 방향을 틀었다.
글을 쓴 이유도 어차피 전자에 가까운 마인드로 적었는데 오히려 찾고나니 후자로 좀 더 이야기가 변경되었다. 그리고 또한, 이렇게 되면 SonarQube의 룰이 이상한 것도 찾았기에 SonarQube쪽에도 해당하는 룰 변경 요청을 해볼까 생각중이다.
그리고 SonarQube의 룰 변경에 대해서 해당 이슈의 논의가 진행된다면, 블로그에서 조금 더 이야기를 풀어 보겠다.