環境

ArchUnit: 1.0.1

はじめに

サービスへのトラフィックが多くなってくるとリードレプリカを用意して DB の負荷を分散させたくなってきます。

SpringBoot の場合、reader / writer のどちらに振り分けるかの基準は @Transactional アノテーションの readOnly 属性を基準とすることができるので、これをもとに org.springframework.jdbc.datasource.lookup.AbstractRoutingDataSource を実装します。

ここで問題になるのが、「そもそも、各処理に正しく @Transactional(readOnly=true) が付いているか？」です。
やっていることは SELECT only なのに @Transactional (readOnly 未指定の場合は false、つまり writable) が付いてしまっているということは、よくあることだと思います。
人がやることなので間違いはありますし、サービスの規模が大きくなって後からリードレプリカを用意するようなケースもあるので、仕方が無いですね。

今回は、この readOnly=true の指定漏れを見つけるためのテストケースを ArchUnit で実装します。

間違っている指定箇所の特徴を抜き出す

アプリケーションは Controller, Service, Repository という古典的な三層レイヤで構成されているものとします。
@Transactional は Service に付いており、Service のメソッドがトランザクション境界となります。

ここで、readOnly=true と指定すべき Service メソッドの特徴を抜き出してみます。

public メソッドである。
インスタンス (non static) メソッドである。
@Transactional(readOnly=false) 相当、つまり writable な DataSource を使うような設定が間違って指定されている。
- この条件は素直に「 @Service が付いているクラス」としても良いかもしれません。
  私のプロジェクトでは、「すべてのトランザクション境界は Service」になっていますが、「すべての Service がトランザクション境界であるべきか？」は No なので、この条件に変えました。
Service メソッド内では読み取り系メソッドしか呼んでいない。
- 呼んでいる Repository のメソッドが読み取り系である。
- 呼んでいる別の Service が読み取り系である。

Repository の判定には、データアクセスレイヤとして MyBatis を使っていて、必ず自作の基底インタフェース MyBatisRepository を implements しているので、これを目印にします。
Repository のメソッドが読み取り系か否かは、私のプロジェクトの場合は Spring-Data JPA の命名規則をルールにメソッド名を付けていて、find、 count、 exists の接頭辞で見分けられそうでした。
入れ子で呼んでいる Service が読み取り系か否かは、その Service クラスあるいはメソッドに @Transactional(readOnly=true) が付いているか否かで判断できます。

今回は @Transactional による宣言的トランザクション管理のみを対象にします。PlatformTransactionManager や TransactionTemplate などを利用した命令的なトランザクション管理には対応していません。

実装

以上の条件をもとに実装した ArchTest が以下。

TransactionalArchTest.java

@AnalyzeClasses(
    packages = {"io.github.takeyoda"},
    importOptions = ImportOption.DoNotIncludeTests.class)
class TransactionalArchTest {

  @ArchTest
  public static final ArchRule
      READ_ONLY_METHOD_SHOULD_BE_ANNOTATED_WITH_TRANSACTIONAL_WITH_READ_ONLY =
          methods()
              .that()
              .arePublic()
              .and()
              .areNotStatic()
              .and(
                  DescribedPredicate.or(
                      HasOwner.Predicates.With.owner(
                          CanBeAnnotated.Predicates.annotatedWith(
                              transactionalWithReadOnly(false))),
                      CanBeAnnotated.Predicates.annotatedWith(transactionalWithReadOnly(false))))
              .and(callsOnlyReadOperation())
              .should()
              .beAnnotatedWith(transactionalWithReadOnly(true))
              .orShould()
              .beDeclaredInClassesThat()
              .areAnnotatedWith(transactionalWithReadOnly(true))
              .as(
                  "読み取り専用オペレーションしか呼んでいない Transactional なメソッドは @Transactional(readOnly=true) を指定しなければならない")
              .because("リードレプリカを有効に活用するため")
              .allowEmptyShould(true);

  private static DescribedPredicate<JavaAnnotation<?>> transactionalWithReadOnly(boolean readOnly) {
    return new DescribedPredicate<>("@Transactional(readOnly=%s)", readOnly) {
      @Override
      public boolean test(JavaAnnotation<?> javaAnnotation) {
        if (!javaAnnotation.getRawType().isEquivalentTo(Transactional.class)) {
          return false;
        }
        Transactional txn = javaAnnotation.as(Transactional.class);
        return txn.readOnly() == readOnly;
      }
    };
  }

  private static DescribedPredicate<JavaMethod> callsOnlyReadOperation() {
    final Predicate<JavaAccess<?>> isReadAccess =
        (call) -> {
          final JavaClass targetClass = call.getTargetOwner();
          final AccessTarget callMethod = call.getTarget();
          final String callMethodName = callMethod.getName();
          if (targetClass.isAssignableTo(MyBatisRepository.class)) {
            return callMethodName.startsWith("find")
                || callMethodName.startsWith("count")
                || callMethodName.startsWith("exists");
          } else if (targetClass.isAnnotatedWith(Service.class)) {
            if (callMethod.isAnnotatedWith(Transactional.class)) {
              return callMethod.isAnnotatedWith(transactionalWithReadOnly(true));
            }
            return targetClass
                .tryGetAnnotationOfType(Transactional.class)
                .map(Transactional::readOnly)
                .orElse(true);
          }
          // それ以外はデータアクセスと無関係なクラスとみなし不問とする
          return true;
        };

    return new DescribedPredicate<>("calls only read operation") {
      @Override
      public boolean test(JavaMethod javaMethod) {
        // write 系の処理は動詞が豊富なので判定が難しい。そのため
        // 「呼んでいる処理に write が含まれていないか？」ではなく、
        // 「呼んでいる処理が read だけか？」で判定する。
        return javaMethod.getMethodCallsFromSelf().stream().allMatch(isReadAccess)
            && javaMethod.getMethodReferencesFromSelf().stream().allMatch(isReadAccess);
      }
    };
  }
}

まとめ

この ArchTest を追加したことで、 @Transactional の指定が間違っている部分を機械的に洗い出すことができるようになりました。
何と言ってもレビュー前に開発者の手元 and/or CI の結果で気付けるのが大きいですね。