Lombokの@Dataが危険な場面｜ログ出力とHashSetの不具合を再現

@Dataを付けた利用者クラスをログ出力すると、表示したつもりのなかったpasswordHashまで文字列になります。さらに、そのクラスをHashSetへ入れてから表示名を変えると、同じオブジェクトなのにcontainsがfalseになることがあります。二つとも、ソースに書いていない生成メソッドがクラスの使い方に影響した例です。

この記事でわかること

@Dataが実際に生成したtoString・equals・hashCodeの読み方。
集合に残っているオブジェクトを、変更後に検索できなくなる再現手順。
表示してよい項目と、同一ユーザーを決める項目を分けて指定する方法。

まず@Dataが生成するメソッドを確認する

利用者一覧の一時データを扱う、架空のJavaアプリを考えます。利用者IDは生成時点で割り当て済みで、後から変わりません。表示名は更新できます。確認用の出力へ出してよいのは利用者IDだけ、同じ利用者かどうかも利用者IDで判定する、という条件にします。実際のプロジェクトでは、IDを出力してよいかと、そのIDがどの範囲で一意かを別途確認してください。

修正前のUnsafeUserは次のとおりです。ソースにはフィールド3個と@Dataしか書いていません。

package info.syachiku.samples.value;

import lombok.Data;

@Data
public final class UnsafeUser {
    private final String userId;
    private String displayName;
    private String passwordHash;
}

package info.syachiku.samples.value;

import lombok.Data;

@Data
public final class UnsafeUser {
    private final String userId;
    private String displayName;
    private String passwordHash;
}

@Dataは、getter、非finalフィールドのsetter、必須項目のコンストラクタ、toString、equalsとhashCodeをまとめて指定します。このクラスではfinalのuserIdを受け取るコンストラクタと、表示名・ハッシュのsetterが生成されます。明示したメソッドやコンストラクタがあるときの生成条件もあるため、アノテーション名だけで最終的なAPIを決めつけず、対象クラスを確認します。

生成されたUnsafeUserにはgetPasswordHash()のほか、次のtoStringがあります。Lombok 1.18.36のdelombok出力から抜粋しました。

    @Override
    public String toString() {
        return "UnsafeUser(userId=" + this.getUserId() + ", displayName=" + this.getDisplayName() + ", passwordHash=" + this.getPasswordHash() + ")";
    }

    @Override
    public String toString() {
        return "UnsafeUser(userId=" + this.getUserId() + ", displayName=" + this.getDisplayName() + ", passwordHash=" + this.getPasswordHash() + ")";
    }

System.out.println(user)でこのメソッドが使われ、表示名だけでなくハッシュも出ます。デモでは故障を確認するためにDUMMY-HASH-ONLYという固定文字列を使っています。ハッシュにした値ならログへ出してよい、という判断にはなりません。

toStringから外すだけでよいと合意済みなら、@ToString.Excludeが狭い変更です。サンプルのExcludedUserでは、ハッシュのダミー値は出力に現れなくなります。ただしE01のテストでは、ハッシュだけ違う二つのインスタンスは依然としてequalsで一致しません。表示から外す指定と、比較から外す指定は別々です。

package info.syachiku.samples.value;

import lombok.Data;

@Data
public final class ExcludedUser {
    private final String userId;
    private String displayName;
    @lombok.ToString.Exclude
    private String passwordHash;
}

package info.syachiku.samples.value;

import lombok.Data;

@Data
public final class ExcludedUser {
    private final String userId;
    private String displayName;
    @lombok.ToString.Exclude
    private String passwordHash;
}

今後もフィールドが増え、出してよい項目を先に絞れるなら、@ToString(onlyExplicitlyIncluded = true)で明示した項目だけを出す方が、追加したフィールドを毎回除外し忘れる問題を防げます。どちらの設定も、getterを使ったログ出力やJSONシリアライズまで防ぐ機能ではありません。toStringのテストが通ったことを、機密情報を一切出さない証拠にはしません。

表示名の変更でcontainsがfalseでも、要素は消えていない

次はログ出力とは別の故障です。UnsafeUserを表示名Aliceの状態でHashSetに入れ、同じインスタンスに対してsetDisplayName("Bob")を呼びます。前後で同じ引数contains(unsafe)を使います。

        Set<UnsafeUser> unsafeSet = new HashSet<>();
        unsafeSet.add(unsafe);
        System.out.println("unsafe.contains.before=" + unsafeSet.contains(unsafe));
        unsafe.setDisplayName("Bob");
        System.out.println("unsafe.contains.after=" + unsafeSet.contains(unsafe));
        System.out.println("unsafe.size=" + unsafeSet.size());
        System.out.println("unsafe.iterator.sameObject=" + (unsafeSet.iterator().next() == unsafe));

        Set<UnsafeUser> unsafeSet = new HashSet<>();
        unsafeSet.add(unsafe);
        System.out.println("unsafe.contains.before=" + unsafeSet.contains(unsafe));
        unsafe.setDisplayName("Bob");
        System.out.println("unsafe.contains.after=" + unsafeSet.contains(unsafe));
        System.out.println("unsafe.size=" + unsafeSet.size());
        System.out.println("unsafe.iterator.sameObject=" + (unsafeSet.iterator().next() == unsafe));

変更前のcontainsはtrue、変更後はfalseです。一方、size()は1のままで、反復して取得した参照はもとのunsafeと同じです。集合から削除されたのではなく、変更後のハッシュで検索すると、登録時に保存されたハッシュと一致しなくなります。この例のように、登録後に等価性へ関わる状態を変えたときの集合の振る舞いに依存してはいけません。

生成されたhashCodeでは、getUserId()・getDisplayName()・getPasswordHash()を読み、各値を計算に使います。equalsも同じ3項目を比べます。IDがfinalでも、残りのフィールドが比較対象なら、IDだけの等価性にはなりません。

    @Override
    public int hashCode() {
        final int PRIME = 59;
        int result = 1;
        final Object $userId = this.getUserId();
        result = result * PRIME + ($userId == null ? 43 : $userId.hashCode());
        final Object $displayName = this.getDisplayName();
        result = result * PRIME + ($displayName == null ? 43 : $displayName.hashCode());
        final Object $passwordHash = this.getPasswordHash();
        result = result * PRIME + ($passwordHash == null ? 43 : $passwordHash.hashCode());
        return result;
    }

    @Override
    public int hashCode() {
        final int PRIME = 59;
        int result = 1;
        final Object $userId = this.getUserId();
        result = result * PRIME + ($userId == null ? 43 : $userId.hashCode());
        final Object $displayName = this.getDisplayName();
        result = result * PRIME + ($displayName == null ? 43 : $displayName.hashCode());
        final Object $passwordHash = this.getPasswordHash();
        result = result * PRIME + ($passwordHash == null ? 43 : $passwordHash.hashCode());
        return result;
    }

この実行結果は、サンプルの値とJava 17のHashSetで確認したものです。フィールドを変更したら必ず別のバケットへ行く、すべての値で同じ失敗が再現する、とは限りません。ハッシュ衝突があっても、集合に入っている間に等価性を変えないという判断は同じです。

UnsafeUser.javaのdelombok出力全文（equalsも確認できます）

package info.syachiku.samples.value;

public final class UnsafeUser {
    private final String userId;
    private String displayName;
    private String passwordHash;

    public UnsafeUser(final String userId) {
        this.userId = userId;
    }

    public String getUserId() {
        return this.userId;
    }

    public String getDisplayName() {
        return this.displayName;
    }

    public String getPasswordHash() {
        return this.passwordHash;
    }

    public void setDisplayName(final String displayName) {
        this.displayName = displayName;
    }

    public void setPasswordHash(final String passwordHash) {
        this.passwordHash = passwordHash;
    }

    @Override
    public boolean equals(final Object o) {
        if (o == this) return true;
        if (!(o instanceof UnsafeUser)) return false;
        final UnsafeUser other = (UnsafeUser) o;
        final Object this$userId = this.getUserId();
        final Object other$userId = other.getUserId();
        if (this$userId == null ? other$userId != null : !this$userId.equals(other$userId)) return false;
        final Object this$displayName = this.getDisplayName();
        final Object other$displayName = other.getDisplayName();
        if (this$displayName == null ? other$displayName != null : !this$displayName.equals(other$displayName)) return false;
        final Object this$passwordHash = this.getPasswordHash();
        final Object other$passwordHash = other.getPasswordHash();
        if (this$passwordHash == null ? other$passwordHash != null : !this$passwordHash.equals(other$passwordHash)) return false;
        return true;
    }

    @Override
    public int hashCode() {
        final int PRIME = 59;
        int result = 1;
        final Object $userId = this.getUserId();
        result = result * PRIME + ($userId == null ? 43 : $userId.hashCode());
        final Object $displayName = this.getDisplayName();
        result = result * PRIME + ($displayName == null ? 43 : $displayName.hashCode());
        final Object $passwordHash = this.getPasswordHash();
        result = result * PRIME + ($passwordHash == null ? 43 : $passwordHash.hashCode());
        return result;
    }

    @Override
    public String toString() {
        return "UnsafeUser(userId=" + this.getUserId() + ", displayName=" + this.getDisplayName() + ", passwordHash=" + this.getPasswordHash() + ")";
    }
}

package info.syachiku.samples.value;

public final class UnsafeUser {
    private final String userId;
    private String displayName;
    private String passwordHash;

    public UnsafeUser(final String userId) {
        this.userId = userId;
    }

    public String getUserId() {
        return this.userId;
    }

    public String getDisplayName() {
        return this.displayName;
    }

    public String getPasswordHash() {
        return this.passwordHash;
    }

    public void setDisplayName(final String displayName) {
        this.displayName = displayName;
    }

    public void setPasswordHash(final String passwordHash) {
        this.passwordHash = passwordHash;
    }

    @Override
    public boolean equals(final Object o) {
        if (o == this) return true;
        if (!(o instanceof UnsafeUser)) return false;
        final UnsafeUser other = (UnsafeUser) o;
        final Object this$userId = this.getUserId();
        final Object other$userId = other.getUserId();
        if (this$userId == null ? other$userId != null : !this$userId.equals(other$userId)) return false;
        final Object this$displayName = this.getDisplayName();
        final Object other$displayName = other.getDisplayName();
        if (this$displayName == null ? other$displayName != null : !this$displayName.equals(other$displayName)) return false;
        final Object this$passwordHash = this.getPasswordHash();
        final Object other$passwordHash = other.getPasswordHash();
        if (this$passwordHash == null ? other$passwordHash != null : !this$passwordHash.equals(other$passwordHash)) return false;
        return true;
    }

    @Override
    public int hashCode() {
        final int PRIME = 59;
        int result = 1;
        final Object $userId = this.getUserId();
        result = result * PRIME + ($userId == null ? 43 : $userId.hashCode());
        final Object $displayName = this.getDisplayName();
        result = result * PRIME + ($displayName == null ? 43 : $displayName.hashCode());
        final Object $passwordHash = this.getPasswordHash();
        result = result * PRIME + ($passwordHash == null ? 43 : $passwordHash.hashCode());
        return result;
    }

    @Override
    public String toString() {
        return "UnsafeUser(userId=" + this.getUserId() + ", displayName=" + this.getDisplayName() + ", passwordHash=" + this.getPasswordHash() + ")";
    }
}

出力には許可項目、同一性には変わらない業務キーを使う

今回の仕様は「割り当て済みの利用者IDが同じなら同じ利用者」です。SafeUserでは、そのIDをfinalにしてコンストラクタで必須にします。equals/hashCodeに含める値はIDだけです。表示名とハッシュは変更できますが、比較には入りません。toStringも別の許可リストでIDだけに限定します。

package info.syachiku.samples.value;

import java.util.Objects;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.Setter;
import lombok.ToString;

@Getter
@EqualsAndHashCode(onlyExplicitlyIncluded = true)
@ToString(onlyExplicitlyIncluded = true)
public final class SafeUser {
    @EqualsAndHashCode.Include
    @ToString.Include
    private final String userId;

    @Setter
    private String displayName;
    @Setter
    private String passwordHash;

    public SafeUser(String userId) {
        this.userId = Objects.requireNonNull(userId, "assigned userId is required");
    }
}

package info.syachiku.samples.value;

import java.util.Objects;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.Setter;
import lombok.ToString;

@Getter
@EqualsAndHashCode(onlyExplicitlyIncluded = true)
@ToString(onlyExplicitlyIncluded = true)
public final class SafeUser {
    @EqualsAndHashCode.Include
    @ToString.Include
    private final String userId;

    @Setter
    private String displayName;
    @Setter
    private String passwordHash;

    public SafeUser(String userId) {
        this.userId = Objects.requireNonNull(userId, "assigned userId is required");
    }
}

二つのonlyExplicitlyIncludedは役割が違います。@EqualsAndHashCode.Includeは集合やマップの比較に関わり、@ToString.Includeは出力内容を決めます。今回は偶然同じuserIdに付きますが、ログ用の項目を増やすために比較対象まで増やす必要はありません。

そのクラスの要件	選ぶ方法	確認すること
入力を受け取るだけ。同じ値か比較しない	必要なgetter／setterだけ	値が同じ別インスタンスが等しいと期待していないか
割り当て済みIDで同じ利用者を識別	変更されないIDだけを等価性に含める	未採番やテナント違いでIDが重ならないか
値そのものがオブジェクトの意味	必要な値を固定し、業務の等価性を定義	金額のscale、単位、通貨などの区別が必要か
デバッグ出力の一部だけを除外	`@ToString.Exclude`を検討	出力以外のgetter／比較／JSONへ影響を広げていないか

一時的なフォームの入力を持つだけなら、ReferenceUserのようにgetterとsetterを明示し、equalsを生成しない選び方ができます。その場合、フィールドの値が一致する別インスタンスは等しくなりません。使う側で値比較が必要なら、その前提に合わせて比較の契約を設計します。

複数テナントでIDが同じになる場合は、テナントIDと利用者IDの両方が必要かもしれません。また、保存時までIDがないJPA Entityに、今回の「割り当て済みIDだけで同じ」という実装をそのまま当てはめると、保存前後で等価性が変わるなど別の問題を生みます。このサンプルは素のJavaクラスです。JPAの採番、継承、遅延ロード、プロキシでの同一性は検証していません。

11件を実行し、生成後のJavaでも同じ結果を再現する

ZIPを展開し、pom.xmlのあるディレクトリで次を実行します。MavenがLombokとJUnitを使ってビルドとテストを行います。IDEのプラグインを入れなくても、この手順は実行できます。

mvn -B test
java -cp target/classes info.syachiku.samples.value.LombokContractsDemo

mvn -B test
java -cp target/classes info.syachiku.samples.value.LombokContractsDemo

unsafe=UnsafeUser(userId=U-1, displayName=Alice, passwordHash=DUMMY-HASH-ONLY)
unsafe.contains.before=true
unsafe.contains.after=false
unsafe.size=1
unsafe.iterator.sameObject=true
safe=SafeUser(userId=U-1)
safe.contains.after=true
safe.sameIdDifferentState=true
excluded=ExcludedUser(userId=U-1, displayName=null)

unsafe=UnsafeUser(userId=U-1, displayName=Alice, passwordHash=DUMMY-HASH-ONLY)
unsafe.contains.before=true
unsafe.contains.after=false
unsafe.size=1
unsafe.iterator.sameObject=true
safe=SafeUser(userId=U-1)
safe.contains.after=true
safe.sameIdDifferentState=true
excluded=ExcludedUser(userId=U-1, displayName=null)

テストでは、故障版の文字列・等価性・集合検索を3件、修正版の出力・同一ID・別ID・変更後検索・未割当IDを5件、除外だけの修正、参照同一性、IDのsetterが存在しないことをそれぞれ1件確認します。故障版のテストも、観察した故障の振る舞いを期待値として固定しているため、正しい配布版ではすべて通ります。

修正版のコピーで表示名へ@EqualsAndHashCode.Includeを付けると、S02（同じIDで状態が違うときの等価性）とS04（状態変更後の集合検索）の2件が失敗します。配布版のSafeUserにはこの変更を含めていません。データを変えてから探すテストを入れておくと、IDだけの比較が後から壊れたときに検出できます。

配布したgenerated/は、説明用に書き直したJavaではなく、同梱のコマンドで得た出力です。以下で再生成できます。MAVEN_REPO_LOCALを設定していなければ、標準のMavenキャッシュからLombok 1.18.36を読みます。

sh generate-sources.sh
mkdir -p target/delombok-classes
javac --release 17 -d target/delombok-classes generated/*.java
java -cp target/delombok-classes info.syachiku.samples.value.LombokContractsDemo

sh generate-sources.sh
mkdir -p target/delombok-classes
javac --release 17 -d target/delombok-classes generated/*.java
java -cp target/delombok-classes info.syachiku.samples.value.LombokContractsDemo

ここでは生成後のJavaをLombokなしでコンパイルして動かし、同じ出力を得るところまで確認しました。環境はJava 17、Maven 3.9.5、依存キャッシュありです。別のLombokバージョン、初回依存取得、JPA環境では確認していません。

実行クラス LombokContractsDemo.javaの全文

package info.syachiku.samples.value;

import java.util.HashSet;
import java.util.Set;

public final class LombokContractsDemo {
    public static void main(String[] args) {
        UnsafeUser unsafe = new UnsafeUser("U-1");
        unsafe.setDisplayName("Alice");
        unsafe.setPasswordHash("DUMMY-HASH-ONLY");
        System.out.println("unsafe=" + unsafe);
        Set<UnsafeUser> unsafeSet = new HashSet<>();
        unsafeSet.add(unsafe);
        System.out.println("unsafe.contains.before=" + unsafeSet.contains(unsafe));
        unsafe.setDisplayName("Bob");
        System.out.println("unsafe.contains.after=" + unsafeSet.contains(unsafe));
        System.out.println("unsafe.size=" + unsafeSet.size());
        System.out.println("unsafe.iterator.sameObject=" + (unsafeSet.iterator().next() == unsafe));

        SafeUser safe = new SafeUser("U-1");
        safe.setDisplayName("Alice");
        safe.setPasswordHash("DUMMY-HASH-ONLY");
        Set<SafeUser> safeSet = new HashSet<>();
        safeSet.add(safe);
        safe.setDisplayName("Bob");
        System.out.println("safe=" + safe);
        System.out.println("safe.contains.after=" + safeSet.contains(safe));
        System.out.println("safe.sameIdDifferentState=" + safe.equals(new SafeUser("U-1")));
        ExcludedUser excluded = new ExcludedUser("U-1");
        excluded.setPasswordHash("DUMMY-HASH-ONLY");
        System.out.println("excluded=" + excluded);
    }
}

package info.syachiku.samples.value;

import java.util.HashSet;
import java.util.Set;

public final class LombokContractsDemo {
    public static void main(String[] args) {
        UnsafeUser unsafe = new UnsafeUser("U-1");
        unsafe.setDisplayName("Alice");
        unsafe.setPasswordHash("DUMMY-HASH-ONLY");
        System.out.println("unsafe=" + unsafe);
        Set<UnsafeUser> unsafeSet = new HashSet<>();
        unsafeSet.add(unsafe);
        System.out.println("unsafe.contains.before=" + unsafeSet.contains(unsafe));
        unsafe.setDisplayName("Bob");
        System.out.println("unsafe.contains.after=" + unsafeSet.contains(unsafe));
        System.out.println("unsafe.size=" + unsafeSet.size());
        System.out.println("unsafe.iterator.sameObject=" + (unsafeSet.iterator().next() == unsafe));

        SafeUser safe = new SafeUser("U-1");
        safe.setDisplayName("Alice");
        safe.setPasswordHash("DUMMY-HASH-ONLY");
        Set<SafeUser> safeSet = new HashSet<>();
        safeSet.add(safe);
        safe.setDisplayName("Bob");
        System.out.println("safe=" + safe);
        System.out.println("safe.contains.after=" + safeSet.contains(safe));
        System.out.println("safe.sameIdDifferentState=" + safe.equals(new SafeUser("U-1")));
        ExcludedUser excluded = new ExcludedUser("U-1");
        excluded.setPasswordHash("DUMMY-HASH-ONLY");
        System.out.println("excluded=" + excluded);
    }
}

11ケースのLombokContractsTest.javaの全文

package info.syachiku.samples.value;

import java.util.*;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.*;

class LombokContractsTest {
    @Test void U01_dataPrintsAllThreeFieldsIncludingDummyHash() {
        UnsafeUser user = unsafeUser();
        assertEquals("UnsafeUser(userId=U-1, displayName=Alice, passwordHash=DUMMY-HASH-ONLY)", user.toString());
    }
    @Test void U02_dataIncludesMutableFieldInEquality() {
        UnsafeUser left = unsafeUser();
        UnsafeUser right = unsafeUser();
        assertEquals(left, right);
        right.setDisplayName("Bob");
        assertNotEquals(left, right);
    }
    @Test void U03_hashSetLookupFailsAfterEqualityFieldChanges() {
        UnsafeUser user = unsafeUser();
        Set<UnsafeUser> users = new HashSet<>();
        users.add(user);
        assertTrue(users.contains(user));
        user.setDisplayName("Bob");
        assertFalse(users.contains(user));
        assertEquals(1, users.size());
        assertSame(user, users.iterator().next());
    }
    @Test void S01_safeToStringUsesOnlyApprovedUserId() {
        SafeUser user = safeUser();
        assertEquals("SafeUser(userId=U-1)", user.toString());
        assertFalse(user.toString().contains("DUMMY-HASH-ONLY"));
        assertFalse(user.toString().contains("Alice"));
    }
    @Test void S02_sameAssignedIdIsEqualDespiteDifferentState() {
        SafeUser left = safeUser();
        SafeUser right = new SafeUser("U-1");
        right.setDisplayName("Bob");
        right.setPasswordHash("OTHER-DUMMY-HASH");
        assertEquals(left, right);
        assertEquals(left.hashCode(), right.hashCode());
    }
    @Test void S03_differentAssignedIdIsDifferentUser() {
        assertNotEquals(safeUser(), new SafeUser("U-2"));
    }
    @Test void S04_hashSetLookupSurvivesStateChanges() {
        SafeUser user = safeUser();
        Set<SafeUser> users = new HashSet<>();
        users.add(user);
        int hashBefore = user.hashCode();
        user.setDisplayName("Bob");
        user.setPasswordHash("OTHER-DUMMY-HASH");
        assertEquals(hashBefore, user.hashCode());
        assertTrue(users.contains(user));
        assertTrue(users.contains(new SafeUser("U-1")));
        assertEquals(1, users.size());
    }
    @Test void S05_unassignedIdIsRejected() {
        assertThrows(NullPointerException.class, () -> new SafeUser(null));
    }
    @Test void E01_excludeChangesOnlyToString() {
        ExcludedUser left = new ExcludedUser("U-1");
        ExcludedUser right = new ExcludedUser("U-1");
        left.setPasswordHash("DUMMY-A");
        right.setPasswordHash("DUMMY-B");
        assertFalse(left.toString().contains("DUMMY-A"));
        assertNotEquals(left, right);
    }
    @Test void R01_getterSetterDoesNotCreateValueEquality() {
        ReferenceUser left = new ReferenceUser();
        ReferenceUser right = new ReferenceUser();
        left.setUserId("U-1");
        right.setUserId("U-1");
        assertNotEquals(left, right);
        assertEquals(left, left);
    }
    @Test void G01_safeUserDoesNotHaveIdSetter() {
        assertThrows(NoSuchMethodException.class, () -> SafeUser.class.getMethod("setUserId", String.class));
    }
    private UnsafeUser unsafeUser() {
        UnsafeUser user = new UnsafeUser("U-1");
        user.setDisplayName("Alice");
        user.setPasswordHash("DUMMY-HASH-ONLY");
        return user;
    }
    private SafeUser safeUser() {
        SafeUser user = new SafeUser("U-1");
        user.setDisplayName("Alice");
        user.setPasswordHash("DUMMY-HASH-ONLY");
        return user;
    }
}

package info.syachiku.samples.value;

import java.util.*;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.*;

class LombokContractsTest {
    @Test void U01_dataPrintsAllThreeFieldsIncludingDummyHash() {
        UnsafeUser user = unsafeUser();
        assertEquals("UnsafeUser(userId=U-1, displayName=Alice, passwordHash=DUMMY-HASH-ONLY)", user.toString());
    }
    @Test void U02_dataIncludesMutableFieldInEquality() {
        UnsafeUser left = unsafeUser();
        UnsafeUser right = unsafeUser();
        assertEquals(left, right);
        right.setDisplayName("Bob");
        assertNotEquals(left, right);
    }
    @Test void U03_hashSetLookupFailsAfterEqualityFieldChanges() {
        UnsafeUser user = unsafeUser();
        Set<UnsafeUser> users = new HashSet<>();
        users.add(user);
        assertTrue(users.contains(user));
        user.setDisplayName("Bob");
        assertFalse(users.contains(user));
        assertEquals(1, users.size());
        assertSame(user, users.iterator().next());
    }
    @Test void S01_safeToStringUsesOnlyApprovedUserId() {
        SafeUser user = safeUser();
        assertEquals("SafeUser(userId=U-1)", user.toString());
        assertFalse(user.toString().contains("DUMMY-HASH-ONLY"));
        assertFalse(user.toString().contains("Alice"));
    }
    @Test void S02_sameAssignedIdIsEqualDespiteDifferentState() {
        SafeUser left = safeUser();
        SafeUser right = new SafeUser("U-1");
        right.setDisplayName("Bob");
        right.setPasswordHash("OTHER-DUMMY-HASH");
        assertEquals(left, right);
        assertEquals(left.hashCode(), right.hashCode());
    }
    @Test void S03_differentAssignedIdIsDifferentUser() {
        assertNotEquals(safeUser(), new SafeUser("U-2"));
    }
    @Test void S04_hashSetLookupSurvivesStateChanges() {
        SafeUser user = safeUser();
        Set<SafeUser> users = new HashSet<>();
        users.add(user);
        int hashBefore = user.hashCode();
        user.setDisplayName("Bob");
        user.setPasswordHash("OTHER-DUMMY-HASH");
        assertEquals(hashBefore, user.hashCode());
        assertTrue(users.contains(user));
        assertTrue(users.contains(new SafeUser("U-1")));
        assertEquals(1, users.size());
    }
    @Test void S05_unassignedIdIsRejected() {
        assertThrows(NullPointerException.class, () -> new SafeUser(null));
    }
    @Test void E01_excludeChangesOnlyToString() {
        ExcludedUser left = new ExcludedUser("U-1");
        ExcludedUser right = new ExcludedUser("U-1");
        left.setPasswordHash("DUMMY-A");
        right.setPasswordHash("DUMMY-B");
        assertFalse(left.toString().contains("DUMMY-A"));
        assertNotEquals(left, right);
    }
    @Test void R01_getterSetterDoesNotCreateValueEquality() {
        ReferenceUser left = new ReferenceUser();
        ReferenceUser right = new ReferenceUser();
        left.setUserId("U-1");
        right.setUserId("U-1");
        assertNotEquals(left, right);
        assertEquals(left, left);
    }
    @Test void G01_safeUserDoesNotHaveIdSetter() {
        assertThrows(NoSuchMethodException.class, () -> SafeUser.class.getMethod("setUserId", String.class));
    }
    private UnsafeUser unsafeUser() {
        UnsafeUser user = new UnsafeUser("U-1");
        user.setDisplayName("Alice");
        user.setPasswordHash("DUMMY-HASH-ONLY");
        return user;
    }
    private SafeUser safeUser() {
        SafeUser user = new SafeUser("U-1");
        user.setDisplayName("Alice");
        user.setPasswordHash("DUMMY-HASH-ONLY");
        return user;
    }
}

実行設定 pom.xmlの全文

<?xml version="1.0" ?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <groupId>info.syachiku.samples</groupId>
  <artifactId>lombok-data-contracts</artifactId>
  <version>1.0.0</version>
  <properties>
    <maven.compiler.release>17</maven.compiler.release>
    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
  </properties>
  <dependencies>
    <dependency><groupId>org.projectlombok</groupId><artifactId>lombok</artifactId><version>1.18.36</version><scope>provided</scope></dependency>
    <dependency>
      <groupId>org.junit.jupiter</groupId>
      <artifactId>junit-jupiter</artifactId>
      <version>5.12.2</version>
      <scope>test</scope>
    </dependency>
  </dependencies>
  <build>
    <plugins>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-resources-plugin</artifactId>
        <version>3.2.0</version>
      </plugin>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-compiler-plugin</artifactId>
        <version>3.8.1</version>
      </plugin>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-surefire-plugin</artifactId>
        <version>3.5.6</version>
      </plugin>
    </plugins>
  </build>
</project>

<?xml version="1.0" ?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <groupId>info.syachiku.samples</groupId>
  <artifactId>lombok-data-contracts</artifactId>
  <version>1.0.0</version>
  <properties>
    <maven.compiler.release>17</maven.compiler.release>
    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
  </properties>
  <dependencies>
    <dependency><groupId>org.projectlombok</groupId><artifactId>lombok</artifactId><version>1.18.36</version><scope>provided</scope></dependency>
    <dependency>
      <groupId>org.junit.jupiter</groupId>
      <artifactId>junit-jupiter</artifactId>
      <version>5.12.2</version>
      <scope>test</scope>
    </dependency>
  </dependencies>
  <build>
    <plugins>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-resources-plugin</artifactId>
        <version>3.2.0</version>
      </plugin>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-compiler-plugin</artifactId>
        <version>3.8.1</version>
      </plugin>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-surefire-plugin</artifactId>
        <version>3.5.6</version>
      </plugin>
    </plugins>
  </build>
</project>

レビューでは「生成されるもの」と「業務で必要なもの」を照合する

「表示名を更新できるクラスですが、HashSetの同一判定は割り当て済みIDで維持します。生成されるequalsとhashCodeはIDだけに絞り、toStringも許可したIDだけを出すことを別のテストで確認しました。」

レビューで「@Dataをやめてください」とだけ書くと、何を直せば合格かが伝わりません。どの生成メソッドがどの入力で問題を起こしたか、選んだ等価性と出力条件がどのテストに対応するかを伝えます。HashSetに入れていないクラスでも、別のコードが将来キーとして使うなら、変更できる値を比較へ含めてよいかは設計上の問いとして残ります。

delombokで、自分のクラスに生成されたメソッドを確認したか。
toStringに出す値と、equals/hashCodeに含める値を別々に決めたか。
集合に入れた後に変わる値が、比較に入っていないか。
同じID・違うID・未採番の違いを、クラスの用途に合わせて扱ったか。
対象がDTOか、値オブジェクトか、永続化するEntityかを区別したか。

アノテーションを選ぶ前に、クラスの役割や同一性を決めたい人へ。変更に耐える設計をコード例から考える本です。

PR Java新人・若手向け

改訂新版良いコード／悪いコードで学ぶ設計入門

仙塲大也 (著)

レビューで直される理由を、Javaの具体例から理解する。

命名、責務、クラス設計、不変性などを良い例と悪い例で比較できます。現場のコードレビューと結びつけやすい一冊です。

設計の悪さを具体例から学びたい
レビュー指摘の判断軸を増やしたい

Amazonで詳細を見る

当サイトはAmazonアソシエイト・プログラムの参加者です。価格・在庫・配送条件はAmazonでご確認ください。

必要なメソッドを選び、生成結果とテストで説明できるようにする

今回の故障は、生成されたtoStringとequals/hashCodeが別々の契約を持つことを見落とした結果です。表示に使う値、同じと判断する値、後から変更できる値を決めてから、Lombokのアノテーションを選びます。読み手がその判断を追えるよう、元コード、生成後のコード、実行結果を同じ版に揃えておきます。

一次資料：Lombok @Data、Lombok @EqualsAndHashCode、Lombok @ToString、Lombok delombok、Java 17 Set。