spring-data-jpa アンチパターン

これまで使っていて体験したアンチパターン（だと勝手に思っている事例）集。
JPAと読み替えてよいものも含まれます。
spring-data-jpa / JPA の知見少ないので指摘歓迎。

join fetch地獄

例

@Repository
public interface ArticleRepository 
  extends JpaRepository<Article, Long> {

  // 詳細表示用
  // 記事配下のコメントまで取得する
  @Query("select a from Article a " + 
    "join fetch a.comments " + 
    "where a.id = ?1")
  Article findById(Long id);

  // 管理者向けの詳細表示用
  // 通常の詳細表示に加えて記事の作成者、コメントの作成者まで取得する
  @Query("select a from Article a " + 
    "join fetch a.comments c " + 
    "join fetch a.author " +
    "join fetch c.author " + 
    "where a.id = ?1")
  Article findByIdForAdmin(Long id);

  // 帳票出力用
  // 管理者向けの詳細表示に加えて作成者の所属部署まで取得する
  @Query("select a from Article a " + 
    "join fetch a.comments c " + 
    "join fetch a.author aa " +
    "join fetch c.author ca " + 
    "join fetch aa.belongedDepartment " + 
    "join fetch ca.belongedDepartment " + 
    "where a.id = ?1")
  Article findByIdForReport(Long id);

  // 以下、システムの成長・複雑度が増えるにしたがい増え続ける（実際増え続けた）
}

不吉な匂い

• 開発者「CSV出力機能を追加するにあたって取得する項目が増えたのでfindByIdForReport ()をコピペしてjoin fetchをいくつか追加したfindByIdForCsvReport() を作りますね^^」

問題点

• リポジトリ層なのにビューやドメインの表現が混入している
  • ForAdmin, ForReport
  • かといってfindByIdWithAuthorAndDepartmentみたいな命名にしても関連が深くなると絶対に諦めざるを得なくなるのでやめておいたほうがいい（経験談）
• フェッチパターンが増えるたびにメソッドが増えていくのが明白である
  • 画面や処理が増えるたびにリポジトリのメソッドも追加しないといけないとか嫌じゃない？
• 検索条件が同じ（idのみ）でフェッチ範囲が違うものだけでこの状態なので、もし検索条件がちょっと違うバージョンが増えたところを想像してみるとこの恐ろしさがわかるのではないかと思う

無差別left join fetch

例

@Repository
public interface ArticleRepository 
  extends JpaRepository<Article, Long> {

  @Query("select a from Article a " + 
    "left join fetch a.comments c " + 
    "left join fetch a.author aa " +
    "left join fetch c.author ca " + 
    "left join fetch aa.belongedDepartment " + 
    "left join fetch ca.belongedDepartment " + 
    // 以降、ユースケースが広がるたびにleft join fetchが増え続けてゆく...
    "where a.id = ?1")
  Article findById(Long id);
}

取得する可能性のある関連を全てleft join fetchする手法
前述の「join fetch地獄」を解消する手段として選択してしまうことがあった

不吉な匂い

• レビュア「あれ、この項目よく読むと必要ない気がするけどなんでフェッチしてんの？」
• 開発者A「あれ、この項目よく読むと必要なかった気がするけどなんでそう実装したんだっけ・・・」
• 開発者B「このJPQLクエリ最適化したいけど影響範囲が広くて辛い・・・」
• 開発者C「新たに必要な項目が増えたのでleft join fetch追加しますね^^」

問題点

• 全てのユースケースで必要になる項目をフェッチしている為、影響範囲が読みにくく、レビューコストも高い
  • view (html / json / 帳票 / etc)　のフォーマットから辿って当該fetchが本当に必要か判断する必要がある為
• シンプルな手法だが、安易な考えでやらかすとリファクタする時に死ぬ
  • なぜなら、フェッチ範囲が広がる場合は気軽に追加できるが、フェッチ範囲が狭くなる場合はよほど注意してレビューしない限り気づけないから（経験談）
• 単一取得ではそんなに気にしなくてもいいけど複数件取得だとパフォーマンスに問題が出る場合もある
  • でも上であげた問題に比べたら全然大したことないと思う

NamedEntityGraph地獄

例

@Entity
@Table(name = "articles")
@NamedEntityGraphs({
  @NamedEntityGraph(
    name = "Article.detail",
    attributeNodes = {
      @NamedAttributeNode("comments"),
    }),
  @NamedEntityGraph(
    name = "Article.detail.admin",
    attributeNodes = {
      @NamedAttributeNode(
        value = "comments", 
        subgraph = "Article.detail.admin.comments"),
      @NamedAttributeNode("author"),
    },
    subgraphs = {
      @NamedSubgraph(
        name = "Article.detail.admin.comments",
        attributeNodes = @NamedAttributeNode("author"))
    }),
  @NamedEntityGraph(
    name = "Article.report",
    attributeNodes = {
      @NamedAttributeNode(
        value = "comments", 
        subgraph = "Article.report.comments"),
      @NamedAttributeNode(
        value = "author", 
        subgraph = "Article.report.author"),
    },
    subgraphs = {
      @NamedSubgraph(
        name = "Article.report.comments",
        attributeNodes = @NamedAttributeNode(
          value = "author",
          sungraph = "Article.report.author")
      ),
      @NamedSubgraph(
        name = "Article.report.author",
        attributeNodes = @NamedAttributeNode("department")
      )
    })
  // 上記だけで済めばマシな方で場合によっては以下こういうのが無数に続く
})
public class Article {
  // ...
}

前述の2つの問題に対してフェッチ範囲の指定をJPQL文から分離したいという欲求から導入したが...

不吉な匂い

• 開発者A「EntityGraphの定義だけで500行超えたのでなんとかしなくては」
• 開発者B「この@NamedEntityGraph 長くて読みづらい・・・」
• 開発者C「全文検索かけたらこの@NamedEntityGraph使われてなかったんだけど・・・」
• 新規参入の開発者「？？？？」

問題点

• @NamedEntityGraphの性質上、フェッチパターンが増えた場合にアノテーション地獄に陥りやすい
• ドメイン層へのview知識の混入は依然として解決できない
• 慣れないと読みにくい（長いし）、慣れても割と読みにくい
• タイプセーフじゃないのでEntityのリファクタに弱い
  • JPAのメリットが活かせない
• 使用箇所を探すには全文検索するしかない
  • IDEの力に頼りにくい
  • 被りやすい名前をつけていると...
  • 定数を定義する方法も試したがなんかしっくりこない（個人の感想）
    • 定数値がユニークなことを保証できないからかも（かと言ってenumは使えないし）
• org.springframework.data.jpa.repository.EntityGraph の attributePathを使えば@NamedEntityGraphより簡潔に記述することはできる

責務混在

例

// 検索条件を定義するクラスとする
public class ArticleSpec {

  public static Specification<Article> findByName(@Nullable String name) {

    return (root, query, cb) -> {

      // 記事を書いた人もフェッチ
      root.fetch(Article_.author);

      // 記事名の昇順でソート
      // 注 標準で findAll(Specification, Sort) が用意されているのでソート条件だけなら分離は可能
      query.orderBy(cb.asc(root.get(Article_.name)));

      // 記事名が指定されていたら部分一致、そうでなければ検索条件なし
      if (StringUtils.isEmpty(name)) {
        return cb.conjunction();
      }
      return cb.like(root.get(Article_.name), "%" + name + "%"));
    }
  }
}

// 利用側
public class ArticleService {

  // ...

  public List<Article> search(String name) {
    return repository.findAll(ArticleSpec.findByName(name));
  }
}

不吉な匂い

• 開発者A「この検索条件流用したいんだけどソート条件が違うからコピペして似たようなの作っちゃいますね^^」
• 開発者B「この検索条件流用したいんだけどこの項目はフェッチする必要ないから（以下略）」

問題点

• 検索条件,フェッチ範囲,ソート条件が密結合している為、様々な要求に対応しづらい
• 仕様上、org.springframework.data.jpa.domain.Specificationを生成する過程でフェッチやソートが可能ではあるが可能であることとやるべきかは違う
  • 密結合している状態が正しいようなケースもあるので、そこはケースバイケース

改善案

上記アンチパターン（3つ）の共通な問題として

• 検索条件、フェッチ条件、ソート条件が混在している
• レイヤーを（綺麗に）分離できない
  • リポジトリ層にM/V相当の情報が存在
  • ドメイン層にV相当の情報が存在

ことが挙げられ、それらを分離するために必要なのは

Optional<T> repository.findById(ID, フェッチ範囲);
Optional<T> repository.findOne(検索条件, フェッチ範囲);
List<T> repository.findAll(検索条件, フェッチ範囲, Sort);
List<T> repository.findAll(フェッチ範囲, Sort);
Page<T> repository.findAll(検索条件, フェッチ範囲, Pageable);

のようなI/Fだと考えた結果、以下の方法を採用した。

2017/08時点でベストだと考えている解決方法

フェッチ範囲を規程するクラス

ExEntityGraph.java

// EntityGraphのラッパーのようなクラス
@RequiredArgsConstructor(access = AccessLevel.PRIVATE)
public class ExEntityGraph<T> {

  // fetch or load
  private final org.springframework.data.jpa.repository.EntityGraph.EntityGraphType type;

  // EntityManager -> EntityGraph<T> への変換関数
  private final Function<EntityManager, EntityGraph<T>> mapper;

  // 必要ならloadも定義
  public static <S> ExEntityGraph<S> fetch(Function<EntityManager, EntityGraph<S>> mapper) {
    return new ExEntityGraph<>(FETCH, mapper);
  }

  public String key() {
    return type.getKey();
  }

  public EntityGraph<T> value(EntityManager em) {
    return mapper.apply(em);
  }
}

ArticleViewSpec.java

// View仕様クラス
public class ArticleViewSpec {

  public static final ExEntityGraph<Article> 詳細 = ExEntityGraph.fetch(em -> {

    EntityGraph<Article> graph = em.createEntityGraph(Article.class);

    graph.addAttributeNodes(Article_.comments);

    return graph;
  });

  // エイリアスとしても使える（一覧の表示項目がたまたま詳細と同じになっているような場合）
  public static final ExEntityGraph<Article> 一覧 = 詳細;

  public static final ExEntityGraph<Article> 管理者用の詳細 = ...

  // ...
}

• @NamedEntityGraphとやっていることはほぼ同じ
• クラス名は自分がそうしているだけ
  • ArticleViewSpecification(s), ArticleGraph(s), ...

補足

View仕様と言っても複数の意味があり、

• 画面に「記事」と「そのコメント全て」を表示させたい
  • Viewの要求
• 「記事」中の「選択したコメント全て」を削除したい
  • Modelの要求

EntityGraphとしての表現は同じだが意味は異なる。
なので1クラスにView仕様を集めるのは得策ではないかも。

利点

• Entityからフェッチ範囲の指定を分離できる
• static MetaModelを使えるため割とタイプセーフ
  • 完全にタイプセーフではないけど説明すると長くなるので省略
• Javaオブジェクト化により参照されなくなったらすぐに分かる（IDEにそういう機能があれば）
• ドメインやユースケースに即した命名が可能
• @NamedEntityGraphよりは読みやすい（個人の感想）

リポジトリ、サービス

BaseRepository.java

// 実装（BaseRepositoryImpl.java）は省略
// やりたいことはExEntityGraph<T> を受け取るカスタムメソッドを定義してそれを実装するだけ
@NoRepositoryBean
public interface BaseRepository<T, ID>
  extends JpaRepository<T, ID>, JpaSpecificationExecutor<T> {

  // 実装では 
  // EntityManager#find(java.lang.Class<T>, java.lang.Object, java.util.Map<java.lang.String,java.lang.Object>)
  // の第3引数にヒントとして渡す
  // または
  // javax.persistence.TypedQuery#setHint(graph.key(), graph.value(em))
  // することでグラフを指定する
  Optional<T> findById(ID id, ExEntityGraph<T> graph);

  // findOne(Spec)
  // findAll()
  // findAll(Sort)
  // findAll(Spec)
  // findAll(Pageable)
  // etc...
  // も同様だけど長くなるので省略
}

ArticleRepository.java

@Repository
public interface ArticleRepository extends BaseRepository<Article, Long> {
  // ...
}

ArticleService.java

@RequiredArgsConstructor
public class ArticleService {

  private final ArticleRepository repository;

  public Article detail(Long id) {
    return repository.findById(id, ArticleViewSpec.詳細).orElseThrow(() -> NOT_FOUND);
  }
}

EntityGraphを利用する点は同じだが、こうすることで以下の利点がある

• View / ユースケースに引きずられる形でリポジトリのメソッドが増えていく状況を改善できる
• リポジトリがView / ユースケースを知っているという状態を無くすことができる
• 色々と使い回しできる
  • 検索条件は違うけどフェッチ範囲は同じ（実際によくある）
  • 検索条件は同じだけどフェッチ範囲は異なる（これも実際によくある）
  • etc...

適用できない場合

• コンストラクタ式（new式）を使いたい（※1）
• JPAプロバイダ依存の機能使ってる場合
• ネイティブクエリを使いたい
• 他にもあるかも

※1 以下のようなCompoundSelectionへの変換関数を引数に取るI/Fを実装すればできなくもない

  // T : Entityの型
  // S : プロジェクションする型（非Entity）
  <S> List<S> findAll(
      Class<S> resultClazz,
      BiFunction<Root<T>, CriteriaBuilder, CompoundSelection<S>> selectionSupplier,
      Specification<T> spec);

やはり全てに適用するのは難しいのでそういうケースでは

• @Queryを使ったJPQL/SQLを書く
• ピンポイントでのリポジトリのカスタム

を選択する。

補足

似たような考え（外部からのEntityGraphの注入）を実現しているライブラリもある

spring-data-jpa-entity-graph

今後独自に拡張しなくてもこれが実現できるかも？

DATAJPA-1497で外部からEntityGraphを渡せるようにする提案が出ていた

とりあえずGetter/Setter

例

@Entity @EntityListener(...) @Getter @Setter
public class Item {

  @Id private Long id;

  // これは監査情報としてのみ利用しておりview等では使用していないものとする
  @CreatedDate  
  private LocalDateTime createdAt;

  private String name;

  private Boolean delete;
}

// 利用側
@RequiredArgsConstructor
public class ItemService {

  private final ItemRepository repository;

  public Item create(String name) {

    Item item = new Item();
    item.setName(name);
    return repository.save(item);
  }

  public Item update(String name) {

    Item item = repository.findOne(id)
    item.setName(name);
    return repository.save(item);
  }

  public Item delete(Long id) {

    Item item = repository.findOne(id)
    item.setDelete(true);
    return repository.save(item);
  }
}

不吉な匂い

レビュア「このEntity、Controllerとかいろんな場所で値がセットされてて読みにくいんだけど・・・」
開発者「どこからでも値がセットできるみたいだったのでそうしました（ドヤ）」

問題点

• どこからでも値をセットできる可能性がある（publicなsetterなので当然）
  • 別にJPAに限った話じゃない
  • 我々は賢いのでそんなことやらないは一部の恵まれた環境でしか通用しない
  • 嫌がらせでしかないがそれをできてしまうのが問題
• ドメインモデルが貧弱
  • Entityはデータの入れ物であるという考えなら別のORM使う方がいいと思う
• サンプルコードレベルではそれほど問題だと感じない**（と思うじゃん？）**
  • 実業務レベルになると辛くなるから大丈夫
• updateステートメントの最適化とはトレードオフになるかも
  • Eclipselink使ったことないからわかんない

改善案

@Entity @EntityListener(...) 
public class Item {

  @Getter @Id private Long id;

  // 利用しないならgetterは不要、そういうユースケースができてから考えればいい
  @CreatedDate
  private LocalDateTime createdAt;

  @Getter
  private String name;

  @Getter
  private Boolean delete;

  public static Item create(String name) {
    Item item = new Item();
    item.name = name;
    return item;
  }

  public Item update(String name) {
    this.name = name;
    return this;
  }

  public Item delete() {
    this.delete = true;
    return this;
  }
}

// 利用側
@RequiredArgsConstructor
public class ItemService {

  private final ItemRepository repository;

  public Item save(String name) {
    return repository.save(Item.create(name));
  }

  public Item update(String name) {

    Item item = repository.findOne(id);
    item.update(name);
    return repository.save(item);
  }

  public Item delete(Long id) {

    Item item = repository.findOne(id);
    item.delete();
    return repository.save(item);
  }
}

• 値を変更する操作が（強制的に）Entityに集約されることで影響範囲が読みやすくなる
  • リフレクションで値を変更するような暴挙には対応できないがそれは別の方法で弾く
• 値を変更する操作がひとまとまりになりユースケースを適切に表現できる
  • 商品名を変更しつつ論理削除する、論理削除状態で新規作成する等の業務としてありえない（ありえるかもしれないけど）操作を（定義しない限り）できなくする
• getterがない=上位のレイヤーでそれを読むケースが（ほぼ）ないことが自明になり自分にもレビュアにも優しい
• protected / パッケージスコープのアクセサを採用するのも場合によっては有り
  • 要は変更が可能なスコープを極力限定させたいだけ

おまけ：（Entityに操作があることで）Java8的な記述をやりやすい（mutableだけど）

public Item delete(Long id) {

  Item item = repository.findById(id)// Optionalを返すやつ
    .map(Item::delete)
    // 利用側（画面なのかAPIなのかはここでは考える必要ない）で適切に処理する
    .orElseThrow(() -> new 404に対応するような例外());

  return repository.save(item);
}

おまけ：対ぬるぽ力を高めることもできる

@Entity
public class Product {

  // ...

  // スキーマもEntityの定義と同じものとする

  @Getter// not nullなので普通にgetter定義してもいい
  @Column(nullable = false)
  private String name;

  @ManyToOne
  @JoinColumn(nullable = true)// nullableなのであえてgetterは定義せずに
  private Category category;

  public Optional<Category> category() {// Optionalなアクセサを定義
    return Optional.ofNullable(category);
  }
}

システム間でのEntity設計の使い回し

例

• 同じ会社が開発している商品情報管理システム, 商品情報APIという別のシステム
• どっちも同じDBを参照しているものとする
• 商品情報APIでは基本的にデータの読み込みしかしないものとする
  • 特にマスタ系テーブルへの書き込みは絶対にやってはならない
• （なぜか）どっちもJPAが採用されているものとする

商品情報管理システムのCategory.java

@Entity
@EntityListeners(AuditingEntityListener.class)
public class Category {

  @Id
  private Long id;

  @CreatedDate
  private LocalDateTime createdAt;

  @CreatedBy
  @ManyToOne(fetch = FetchType.LAZY)
  private User createdBy;

  @LastModifiedDate
  private LocalDateTime updatedAt;

  @LastModifiedBy
  @ManyToOne(fetch = FetchType.LAZY)
  private User updatedBy;

  private String name;
}

商品情報APIシステムのCategory.java

// 商品情報管理システムのCategory.javaと全く同じ
// Categoryに対する操作はリードオンリーとする
@Entity
@EntityListeners(AuditingEntityListener.class)// 不要
public class Category {

  @Id
  private Long id;

  @CreatedDate// 不要
  private LocalDateTime createdAt;// 要件次第で不要

  @CreatedBy// 不要
  @ManyToOne(fetch = FetchType.LAZY)
  private User createdBy;// 要件次第で不要

  @LastModifiedDate// 不要
  private LocalDateTime updatedAt;// 要件次第で不要

  @LastModifiedBy// 不要
  @ManyToOne(fetch = FetchType.LAZY)
  private User updatedBy;// 要件次第で不要

  private String name;
}

不吉な匂い

商品情報管理システムの開発者「このレコードの更新日時が勝手に書き換わってるけど何度確認してもそんなバグは見つからない・・・」

問題点

• システム間でEntityの定義を共有している
  • 共有方法がコピペ、モジュール化しているかは関係ない
  • JPAのEntity≠DBのレコード
  • テーブルのスキーマは同じでもEntityの設計はシステムや業務のコンテキストによって異なる

改善案

• 商品情報APIシステムではJPAを採用しない
  • そもそも論になるけどシステムの性格に応じたものを選択すればいい
• それでもJPAを採用する場合
  • システムのコンテキストに合ったEntityを設計する

商品情報APIシステムのCategory.java（新）

@Entity
public class Category {

  @Id
  @Column(insertable=false)
  private Long id;

  // createdAtとかは必要になったら定義すればいい

  // もしnameに特殊な操作が必要ならEmbeddableにする設計にしてもいい
  @Column(insertable=false, updatable=false)// 変更しちゃダメなのが自明になる
  private String name;
}

こうすることで、商品情報APIシステムから可能な操作が（完全にではないが）限定され、開発者が考えなければいけないこと、やらなければいけないこと（テストとのパターンとか）が減って幸せになれる可能性が上がる。
（もちろんJPAを採用しないことで幸せになれる可能性もある）

素直すぎるEntity

例

@Entity
public class User {

  @Id
  Long id;

  String firstName;

  String lastName;

  String email;

  public boolean isNameValid() {
    return // firstName, lastNameがシステム的に正しいかをチェックする何か
  }

  public boolean isEmailValid() {
    return // メールアドレスがシステム的に正しいかをチェックする何か
  }

  public String emailDomain() {
    return // メールアドレスからドメイン名を抜き出す処理
  }
}

@Entity
public class Customer {

  @Id
  Long id;

  String firstName;

  String lastName;

  String email;

  public boolean isNameValid() {
    return // firstName, lastNameがシステム的に正しいかをチェックする何か
  }

  // 以下、名前に関する操作が増えるたびに追加

  public boolean isEmailValid() {
    return // メールアドレスがシステム的に正しいかをチェックする何か
  }

  public String emailDomain() {
    return // メールアドレスからドメイン名を抜き出す処理
  }

  // 以下、メールアドレスに関する操作が増えるたびに追加

  // 他のフィールドについても同様
}

不吉な匂い

開発者A「追加カラムが増えるたびに書く処理も増えるからクラスが長くなってきた・・・」
開発者B「メールアドレスの入力チェックロジックが変わったけど他にも同じ箇所あったっけ・・・」

問題点

• テーブルの構造をそのままEntityに落としてしまっている
  • 結果としてメールアドレスをチェックするという責務がメールアドレス自身ではなく、ユーザ・顧客Entityに漏れる（firstName, lastNameも同様）
  • Entity=DBのレコードという考えはJPA的によくない
• firstName, lastNameはまとめて1つのクラスとして扱いたいし、メールアドレスもメールアドレスクラスがいい
  • こういう状況を難しい言葉で言うとインピーダンスミスマッチというらしい（リレーショナルデータベースの世界とオブジェクト指向プログラミングの世界とのミスマッチ的な）
• ロジックの共通化ならValidationUtil.isEmailValid(String)とかでできるけど筋は悪い
  • User, CustomerにisEmailValid()が定義されている状況そのものが問題なので

改善案

@Entity
public class User {

  @Id
  Long id;

  @Embedded// 定義の詳細は省略
  PersonName name;

  @Embedded// 定義の詳細は省略
  EmailAddress email;
}

@Entity
public class Customer {

  @Id
  Long id;

  @Embedded// 定義の詳細は省略
  PersonName name;

  @Embedded// 定義の詳細は省略
  EmailAddress email;
}

@Embeddable
@NoArgsConstructor(access = PROTECTED)
@AllArgsConstructor
public class PersonName {

  private String first;

  private String last;

  public boolean isValid() {
    return // firstName, lastNameがシステム的に正しいか返す何か
  }

  // ...
}

@Embeddable
@NoArgsConstructor(access = PROTECTED)
@AllArgsConstructor
public class EmailAddress {

  private String value;

  public boolean isValid() {
    return // メールアドレスがシステム的に正しいか返す何か
  }

  public String domain() {
    return // メールアドレスからドメイン名を抜き出した結果
  }

  // ...
}

• 責務が整理されてスッキリ、保守性も上がった気がする
• 小さいクラスになったことでテストもしやすいし変なイージーミスする確率も減る

ポエム

JPAの一番のいいところはこのインピーダンスミスマッチの解消手段を提供している点にあると思っています。
もちろん完全に解消することはできないのでそういう場合は自分で工夫する必要があります。

よく言われているような

• DB製品が変わった場合でも...
• 実装プロバイダが変わった場合でも...

はそういう場面に直面したことないので正直どうでもいいと思っています。

じゃあユーザ、顧客の一方のメールアドレスチェック仕様が変わったらどうすんの問題

• 継承使えばいいじゃん？
  • 使えません（Hibernateは）、他のプロバイダは知らん
  • JPAは多分そこまで面倒見てくれないので自分でなんとかしないとだめ

1. 全く別のクラスにする

• 仕様が違う（または最初は同じだったけど後で変わった）ならもはや別のクラスだよ派
• 共通の振る舞いが必要ならinterface使う
• 自分的にはコレが好み

// PersonName -> UserName, CustomerName にリファクタ
public class UserName implements Name {

  private String first;

  private String last;

  public boolean isValid() {
    return // ユーザのfirstName, lastNameがシステム的に正しいかを返す何か
  }

  @Override
  public String displayName() {
    return first + " " + last;
  }
  // ...
}

public class CustomerName implements Name {

  private String first;

  private String last;

  public boolean isValid() {
    return // 顧客のfirstName, lastNameがシステム的に正しいかを返す何か
  }

  @Override
  public String displayName() {
    return first + " " + last + " " + "様";
  }
  // ...
}

2. 別のメソッドを生やす

public class PersonName {

  private String first;

  private String last;

  public boolean isUserValid() {
    return // ユーザ用
  }

  public boolean isCustomerValid() {
    return // 顧客用
  }

  // または

  public boolean isValid(Type type) {// typeは適当なenum

    switch(type) {
      case USER :
        return // ユーザ用
      case CUSTOMER :
        return // 顧客用 
    }
    
    throw new IllegalArgumentException("引数がおかしい");
  }

  // ...
}

• 無駄に複雑になるし呼び間違いする可能性あるし好みじゃない

Null Embeddable問題

nullableなカラムだとEmbeddableもnullになっちゃうよ問題

@Entity
public class Product {

  // コードネーム（最初に設定されるためnot null制約がついているものとする）
  @Embedded
  @AttributeOverride(name = "value", 
    column = @Column(name = "code_name", nullable = false)
  private Name codeName;

  // 正式な商品名（決まっていない場合もあるのでnullableと設計されたものとする）
  @Embedded
  @AttributeOverride(name = "value", 
    column = @Column(name = "name", nullable = true)
  private Name name;
}

public void test() {

  // name = NULLのレコードを取得した場合
  Product product = repository.findOne(id);

  product.getCodeName().getValue();// ok
  product.getName().getValue();// NullPointerExceptionが発生する悲しい結果になる
}

解決方法

• nullableなカラムのないDB設計に変える
• Hibernate 5.1以上 : hibernate.create_empty_composites.enabled=true
• Hibernate 5.1未満 : workaroundだけどstackoverflowにたくさんあるので調べればすぐに出てくる

参考
HHH-7610

補足: create_empty_composites は（2019/11現在）stableなオプションではない

HHH000483: An experimental feature has been enabled (hibernate.create_empty_composites.enabled=true) that instantiates empty composite/embedded objects when all of its attribute values are null. This feature has known issues and should not be used in production until it is stabilized. See Hibernate Jira issue HHH-11936 for details.

補足： Embeddableと値オブジェクトは違う

@Embeddable
class CategoryName {

  @ManyToOne(fetch = FetchType.LAZY)
  private Category category;

  // カテゴリが「その他」の場合に入力される名前
  private String name;

  public Name getName() {

    if (category.isOther()) {
      return new Name(name);
    }
    return category.getName();
  }
}

上記みたいにEntityをEmbeddableに含むこともできる。
が、EntityGraphベースのフェッチ戦略を採用している場合は無理にこのようなEmbeddableを作らない方がいいかもしれない
というのも(2018/09現在の)Hibernateだと以下はいずれも通らず、JPQLでjoin fetchするしか選択肢がなくなる為。
（あまり詳しく調べてないので自信なし）

graph
  .addSubGraph(SomeEntity_.someEmbedded)
  .addAttributes(SomeEmbedded_.someEntity);

graph
  .addAttributes("someEmbedded.someEntity");

Entityにspring-securityのUserDetailsを実装する

@Entity
public class User implements UserDetails {

  @Id
  private final Integer id;

  @ElementCollection(targetClass = Role.class)
  @CollectionTable
  @Enumerated(EnumType.STRING)
  private final Set<Role> authorities;

  @Column
  private final String email;

  @Column
  private final String password;

  @Column
  private final boolean enabled;

  @Override
  public Collection<? extends GrantedAuthority> getAuthorities() {
    return authorities;
  }

  @Override
  public String getPassword() {
    return password;
  }

  @Override
  public String getUsername() {
    return email;
  }

  @Override
  public boolean isAccountNonExpired() {
    return true;
  }

  @Override
  public boolean isAccountNonLocked() {
    return true;
  }

  @Override
  public boolean isCredentialsNonExpired() {
    return true;
  }

  @Override
  public boolean isEnabled() {
    return enabled;
  }
}

認証ありのアプリケーションをspring-securityで実装しているとこういう（Entityにo.s.s.c.userdetails.UserDetailsを実装した）コードを書いてしまうかもしれない（つーか書いてしまった）

何故ダメか

UserDetailsはSerializableである

今後ユーザにカラムが100個増えたり、所属部署とか友達ユーザ一覧みたいな関連が増えたらセッションデータが膨らむ。
また、関連EntityにもSerializableが要求されるようになって辛くなる。
ユーザ画像みたいなのをBlobのようなSerializableじゃないクラスで受けることになったら困る

どうすれば良いか

public class LoginUser implements UserDetails {

  private final Collection<? extends GrantedAuthority> authorities;

  private final String username;

  private final String password;

  private final boolean enabled;

  // その他必要な情報を追加

  LoginUser(User user) {
    this.authorities = user.getRoles();
    this.username = user.getEmail();
    this.password = user.getPassword();
    this.enabled = user.getActive();
  }

  @Override
  public Collection<? extends GrantedAuthority> getAuthorities() {
    return authorities;
  }

  @Override
  public String getPassword() {
    return password;
  }

  @Override
  public String getUsername() {
    return username;
  }

  @Override
  public boolean isAccountNonExpired() {
    return true;
  }

  @Override
  public boolean isAccountNonLocked() {
    return true;
  }

  @Override
  public boolean isCredentialsNonExpired() {
    return true;
  }

  @Override
  public boolean isEnabled() {
    return enabled;
  }
}

public class MyUserDetailsService implements UserDetailsService {

  private final Users users;

  @Override
  public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
    return users.get(byEmail(username))
        .map(LoginUser::new)
        .orElseThrow(() -> new UsernameNotFoundException(username));
  }
}

@Controller
@RequestMapping("/")
public class ProductDetailController {

  @GetMapping
  public String index(@AuthenticationPrincipal LoginUser user) {
    return "...";
  }
}

最低でもこんな感じにspring-securityがJPAの事を（あまり）知らないでいいようにしておく。
コンストラクタにUserが残るけど、気になるならinterface使って完全に分離する（class JpaLoginUser implements LoginUserみたいに）

Open-In-View

application.properties

# デフォルト値はtrue
spring.jpa.open-in-view=true

何故ダメか

@Entity
class Article {

  @Getter
  private Set<Comment> comments;
}

こういうEntityを

<ul>
  <!--/*@thymesVar id="a" type="Article"*/-->
  <li th:each="c : ${a.comments}" th:text="${c.content}"></li>
</li>

こういうhtml (thymeleafのテンプレート、jsonとかでも同じ)でレンダリングした場合

commentsがフェッチ済み？	s.j.open-in-view
Y	true	問題なし
Y	false	問題なし
N	true	問題なし（だが追加クエリが発生）
N	false	例外（何の例外かは忘れた）が発生

となり、open-in-view=falseにしているとview層での遅延フェッチができなくなる為、意図しない追加クエリが発生しなくなる。

OSIV (open-session-in-view) とも呼ばれているようにviewの処理中はsessionをopenしておくという機能。
実際はサーブレットフィルタ（OpenSessionInViewFilter）によってリクエスト完了までセッションを開いているみたい。

しかし

これをfalseにした場合、fetchしたはずの関連が実はfetchされないというバグがJPA実装側にあった場合に詰むという問題がある（経験済）ため、盲目的にopen-in-view=falseにするのは危険だと考えています。

自分の場合はローカルや検証で使用するprofileのみopen-in-view=falseにする事でエラーチェッカー的に使用することにしています。（最悪のケースでも動く為）

view層も含めた結合テストを網羅（追加クエリが発生しない事を保証できている）しているような環境なら大丈夫です。（見た事ないけど）

参考
https://vladmihalcea.com/the-open-session-in-view-anti-pattern/
https://github.com/spring-projects/spring-boot/issues/7107

監視しない

特にアンチパターンという訳ではないけど、spring-boot 2.1.0(多分)からHibernateの各種メトリクスがactuatorから（簡単に）取れるようになったので、既に使っている場合は有効にした方が良い、くらいの感じ

spring.jpa.properties.hibernate.generate_statistics=true

としておけば自動的に有効になる

参考
公式ドキュメント

Hibernate 5.4.26.Final以降はorg.hibernate:hibernate-micrometerを使う

Static Metamodelを使わない

JPAとしてはオプショナルな機能（なくても別にいい）だけど、もはや使わないのはアンチパターンな気がしてきたので追記

Static Metamodelって？

大雑把に言うと、

@Entity
public class User {

  private String name;
}

のようなEntityから

@Generated(value = "org.hibernate.jpamodelgen.JPAMetaModelEntityProcessor")
@StaticMetamodel(User.class)
public abstract class User_ {

  public static volatile SingularAttribute<User, String> name;

  public static final String NAME = "name";
}

生成された、↑のようなメタ情報を持つクラス（を自動生成してくれる機能）

何故あると嬉しいか

// spring-mvc のコード

// もしname -> userName とかに変わったら実行時エラー
@GetMapping
public String userList_bad(@PageableDefault(sort = "name") Pageable pageable) {
  // ...
}

// もしname -> userName とかに変わってもコンパイルエラーになる！！！
@GetMapping
public String userList_good(@PageableDefault(sort = User_.NAME) Pageable pageable) {
  // ...
}

こんな感じでフィールド名（やSingularAttribute、PluralAttribute）のメタ情報を参照できる為、フィールド名の変更に割と強くなる

導入方法

• spring-bootベース
• 5.3.9.Final
• build/generated/* にメタモデルを生成

build.gradle

dependencies {
  annotationProcessor("org.hibernate:hibernate-jpamodelgen")
  // https://hibernate.atlassian.net/browse/HHH-13127
  // 5.4.0からは以下は不要になるはず
  annotationProcessor("javax.xml.bind:jaxb-api")
}

sourceSets.main.java.srcDirs += "${buildDir}/generated"

compileJava {
  options.annotationProcessorGeneratedSourcesDirectory = file('build/generated')
}

コンテキスト混在

todo 後で書く