From 7f5cf2593a129dba4b900953fd0eb078b57c74db Mon Sep 17 00:00:00 2001 From: Devon Hillard Date: Sat, 11 Jul 2026 13:09:48 -0600 Subject: [PATCH 1/3] test: isolate entity-graph query-count assertion under parallel execution UserRepositoryEntityGraphTest.shouldLoadRolesAndPrivilegesInBoundedQueryCountViaEntityGraphFinder was intermittently failing (GH-337). It read the SessionFactory-global Hibernate Statistics prepared-statement counter, which is shared across @DataJpaTest classes in the cached Spring context and mutated by tests running concurrently on other threads under JUnit 5 parallel execution. The counter is not transactional, so @DataJpaTest rollback does not isolate it, producing counts higher than the asserted bound. Replace the global counter with a thread-local StatementInspector (StatementCountInspector) registered via spring.jpa.properties.hibernate.session_factory.statement_inspector. Each parallel test runs on its own thread and JDBC connection, so a thread-local statement count is naturally isolated from concurrent pollution. The assertion now uses isBetween(1, 3): the upper bound preserves the N+1 regression guard, and the lower bound fails loudly if the inspector is ever unregistered rather than silently passing as a false green. --- .../UserRepositoryEntityGraphTest.java | 26 ++++------ .../test/config/StatementCountInspector.java | 51 +++++++++++++++++++ .../resources/application-test.properties | 3 ++ 3 files changed, 64 insertions(+), 16 deletions(-) create mode 100644 src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspector.java diff --git a/src/test/java/com/digitalsanctuary/spring/user/persistence/repository/UserRepositoryEntityGraphTest.java b/src/test/java/com/digitalsanctuary/spring/user/persistence/repository/UserRepositoryEntityGraphTest.java index fc705c10..156a98b4 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/persistence/repository/UserRepositoryEntityGraphTest.java +++ b/src/test/java/com/digitalsanctuary/spring/user/persistence/repository/UserRepositoryEntityGraphTest.java @@ -5,8 +5,6 @@ import java.util.HashSet; import java.util.Set; import org.hibernate.Hibernate; -import org.hibernate.SessionFactory; -import org.hibernate.stat.Statistics; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.jpa.test.autoconfigure.TestEntityManager; @@ -15,7 +13,7 @@ import com.digitalsanctuary.spring.user.persistence.model.User; import com.digitalsanctuary.spring.user.test.annotations.DatabaseTest; import com.digitalsanctuary.spring.user.test.builders.UserTestDataBuilder; -import jakarta.persistence.EntityManager; +import com.digitalsanctuary.spring.user.test.config.StatementCountInspector; /** * Database-slice tests verifying that {@link UserRepository#findWithRolesByEmail(String)} eagerly loads the @@ -62,13 +60,6 @@ private void persistUserWithRolesAndPrivileges(String email) { entityManager.clear(); } - private Statistics statistics() { - EntityManager em = entityManager.getEntityManager(); - Statistics stats = em.getEntityManagerFactory().unwrap(SessionFactory.class).getStatistics(); - stats.setStatisticsEnabled(true); - return stats; - } - @Test void shouldInitializeRolesAndPrivilegesWhenLoadedViaEntityGraphFinder() { persistUserWithRolesAndPrivileges("graph@test.com"); @@ -121,8 +112,9 @@ void shouldNotInitializeRolesWhenLoadedViaPlainFindByEmail() { void shouldLoadRolesAndPrivilegesInBoundedQueryCountViaEntityGraphFinder() { persistUserWithRolesAndPrivileges("bounded@test.com"); - Statistics stats = statistics(); - stats.clear(); + // Count prepared statements per-thread rather than reading the SessionFactory-global Statistics counter, + // which is polluted by tests running concurrently on other threads under JUnit parallel execution (GH-337). + StatementCountInspector.reset(); User user = userRepository.findWithRolesByEmail("bounded@test.com"); // Force full traversal to prove no additional (N+1) queries are issued for privileges. @@ -132,10 +124,12 @@ void shouldLoadRolesAndPrivilegesInBoundedQueryCountViaEntityGraphFinder() { } assertThat(privilegeCount).isPositive(); - // A single entity-graph fetch should not degenerate into a per-role / per-privilege N+1 explosion. - // Allow a small bound to tolerate join-table fetch strategy differences across Hibernate versions. - assertThat(stats.getPrepareStatementCount()) + // Upper bound: a single entity-graph fetch should not degenerate into a per-role / per-privilege N+1 + // explosion. A small bound tolerates join-table fetch strategy differences across Hibernate versions. + // Lower bound: at least one statement must be counted, so a broken/unregistered inspector (which would + // read 0) fails loudly instead of passing as a false green. + assertThat(StatementCountInspector.getCount()) .as("EntityGraph finder should load roles + privileges in a bounded number of queries") - .isLessThanOrEqualTo(3); + .isBetween(1, 3); } } diff --git a/src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspector.java b/src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspector.java new file mode 100644 index 00000000..d3716076 --- /dev/null +++ b/src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspector.java @@ -0,0 +1,51 @@ +package com.digitalsanctuary.spring.user.test.config; + +import java.io.Serializable; +import org.hibernate.resource.jdbc.spi.StatementInspector; + +/** + * A Hibernate {@link StatementInspector} that counts prepared JDBC statements on a per-thread basis. + * + *

+ * The test suite runs with JUnit 5 parallel execution, and {@code @DataJpaTest} classes share a single cached Spring context (and therefore a + * single {@code SessionFactory} and its {@code SessionFactory}-global {@code Statistics} object). Reading + * {@code Statistics#getPrepareStatementCount()} to guard against N+1 query explosions is therefore polluted by statements issued from tests running + * concurrently on other threads: the counter is neither session-scoped nor transactional, so {@code @DataJpaTest} rollback does not isolate it. + * + *

+ * Because each parallel test executes on its own thread and JDBC connection, this inspector records its count in a {@link ThreadLocal}, yielding a + * stable, per-test measurement that is immune to concurrent pollution. A single instance is registered on the shared test {@code SessionFactory} via + * the {@code hibernate.session_factory.statement_inspector} property (see {@code application-test.yml}); the counter state is {@code static} so the + * inspecting instance created by Hibernate and the assertions in a test share the same per-thread counter. + * + *

+ * Usage: call {@link #reset()} immediately before the code under measurement, then assert on {@link #getCount()}. + */ +public final class StatementCountInspector implements StatementInspector, Serializable { + + private static final long serialVersionUID = 1L; + + private static final ThreadLocal COUNT = ThreadLocal.withInitial(() -> 0); + + /** + * Resets the calling thread's statement counter to zero. + */ + public static void reset() { + COUNT.set(0); + } + + /** + * Returns the number of prepared statements inspected on the calling thread since the last {@link #reset()}. + * + * @return the per-thread prepared-statement count + */ + public static int getCount() { + return COUNT.get(); + } + + @Override + public String inspect(String sql) { + COUNT.set(COUNT.get() + 1); + return sql; + } +} diff --git a/src/test/resources/application-test.properties b/src/test/resources/application-test.properties index b6a79f8e..431f502d 100644 --- a/src/test/resources/application-test.properties +++ b/src/test/resources/application-test.properties @@ -17,6 +17,9 @@ spring.jpa.properties.hibernate.dialect=org.hibernate.dialect.H2Dialect spring.jpa.hibernate.ddl-auto=create-drop spring.jpa.show-sql=false spring.jpa.properties.hibernate.format_sql=true +# Per-thread prepared-statement counter used by UserRepositoryEntityGraphTest to guard against N+1 +# regressions without reading the SessionFactory-global Statistics counter (see GH-337). +spring.jpa.properties.hibernate.session_factory.statement_inspector=com.digitalsanctuary.spring.user.test.config.StatementCountInspector # Logging logging.level.com.digitalsanctuary.spring.user=INFO From a4904f61accdeb32f05cdf36fcba82497d34f003 Mon Sep 17 00:00:00 2001 From: Devon Hillard Date: Sat, 11 Jul 2026 13:19:48 -0600 Subject: [PATCH 2/3] docs: correct StatementCountInspector wiring reference to application-test.properties --- .../spring/user/test/config/StatementCountInspector.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspector.java b/src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspector.java index d3716076..f9f0400b 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspector.java +++ b/src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspector.java @@ -15,8 +15,8 @@ *

* Because each parallel test executes on its own thread and JDBC connection, this inspector records its count in a {@link ThreadLocal}, yielding a * stable, per-test measurement that is immune to concurrent pollution. A single instance is registered on the shared test {@code SessionFactory} via - * the {@code hibernate.session_factory.statement_inspector} property (see {@code application-test.yml}); the counter state is {@code static} so the - * inspecting instance created by Hibernate and the assertions in a test share the same per-thread counter. + * the {@code hibernate.session_factory.statement_inspector} property (see {@code application-test.properties}); the counter state is {@code static} so + * the inspecting instance created by Hibernate and the assertions in a test share the same per-thread counter. * *

* Usage: call {@link #reset()} immediately before the code under measurement, then assert on {@link #getCount()}. From 80e0cdde19e4d9f93b7eeaf46b54ba1bbb10eff5 Mon Sep 17 00:00:00 2001 From: Devon Hillard Date: Sat, 11 Jul 2026 13:29:51 -0600 Subject: [PATCH 3/3] test: add StatementCountInspector unit test and document registration tradeoffs Address non-blocking PR review feedback on #341: - Add StatementCountInspectorTest covering reset/count/inspect semantics and cross-thread isolation (an interleaved two-thread scenario proving one thread's reset/count cannot perturb another's), locking in the mechanism independently of its one consumer test. - Document that the inspector is registered profile-wide by design and that the per-statement ThreadLocal increment is an inert, negligible overhead for tests that never read the counter. - Note that the ThreadLocal value is intentionally left set (not removed) for the pooled worker thread's lifetime, since every measurement calls reset() first. --- .../test/config/StatementCountInspector.java | 3 + .../config/StatementCountInspectorTest.java | 99 +++++++++++++++++++ .../resources/application-test.properties | 3 + 3 files changed, 105 insertions(+) create mode 100644 src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspectorTest.java diff --git a/src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspector.java b/src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspector.java index f9f0400b..78599e5a 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspector.java +++ b/src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspector.java @@ -25,6 +25,9 @@ public final class StatementCountInspector implements StatementInspector, Serial private static final long serialVersionUID = 1L; + // The counter is intentionally left set (rather than removed) for the calling thread's lifetime: JUnit's parallel + // executor reuses a bounded worker-thread pool, so at most one boxed Integer lingers per pool thread, and every + // measurement calls reset() first, so a stale value from a prior test can never leak into a new one. private static final ThreadLocal COUNT = ThreadLocal.withInitial(() -> 0); /** diff --git a/src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspectorTest.java b/src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspectorTest.java new file mode 100644 index 00000000..2c55f003 --- /dev/null +++ b/src/test/java/com/digitalsanctuary/spring/user/test/config/StatementCountInspectorTest.java @@ -0,0 +1,99 @@ +package com.digitalsanctuary.spring.user.test.config; + +import static org.assertj.core.api.Assertions.assertThat; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link StatementCountInspector}. These exercise the counter mechanism directly (no Spring context, no Hibernate) so the thread-local + * reset/count semantics that isolate {@code UserRepositoryEntityGraphTest} from parallel-execution pollution are locked in independently of that one + * consumer test (GH-337). + */ +class StatementCountInspectorTest { + + @Test + void shouldReportZeroCountAfterReset() { + StatementCountInspector inspector = new StatementCountInspector(); + inspector.inspect("select 1"); + + StatementCountInspector.reset(); + + assertThat(StatementCountInspector.getCount()).isZero(); + } + + @Test + void shouldIncrementCountForEachInspectCall() { + StatementCountInspector inspector = new StatementCountInspector(); + StatementCountInspector.reset(); + + inspector.inspect("select 1"); + inspector.inspect("select 2"); + inspector.inspect("select 3"); + + assertThat(StatementCountInspector.getCount()).isEqualTo(3); + } + + @Test + void shouldReturnSqlUnchangedWhenInspecting() { + StatementCountInspector inspector = new StatementCountInspector(); + String sql = "select * from users where email = ?"; + + assertThat(inspector.inspect(sql)).isEqualTo(sql); + } + + /** + * The core property behind the GH-337 fix: one thread's counting — including its {@link StatementCountInspector#reset()} — must not perturb + * another thread's count. Two threads are interleaved so that thread B fully resets and counts after thread A has counted but + * before thread A reads its final value; thread A must still observe only its own statements. + */ + @Test + void shouldIsolateCountsBetweenThreads() throws InterruptedException { + StatementCountInspector inspector = new StatementCountInspector(); + CountDownLatch threadAHasCounted = new CountDownLatch(1); + CountDownLatch threadBIsDone = new CountDownLatch(1); + AtomicInteger threadAFinalCount = new AtomicInteger(-1); + AtomicInteger threadBFinalCount = new AtomicInteger(-1); + AtomicReference failure = new AtomicReference<>(); + + Thread threadA = new Thread(() -> { + try { + StatementCountInspector.reset(); + inspector.inspect("a1"); + inspector.inspect("a2"); + inspector.inspect("a3"); + threadAHasCounted.countDown(); + assertThat(threadBIsDone.await(5, TimeUnit.SECONDS)).isTrue(); + // Thread B has since reset and counted on its own thread; A's count must be untouched. + threadAFinalCount.set(StatementCountInspector.getCount()); + } catch (Throwable t) { + failure.compareAndSet(null, t); + } + }); + + Thread threadB = new Thread(() -> { + try { + assertThat(threadAHasCounted.await(5, TimeUnit.SECONDS)).isTrue(); + StatementCountInspector.reset(); + inspector.inspect("b1"); + inspector.inspect("b2"); + threadBFinalCount.set(StatementCountInspector.getCount()); + threadBIsDone.countDown(); + } catch (Throwable t) { + failure.compareAndSet(null, t); + threadBIsDone.countDown(); + } + }); + + threadA.start(); + threadB.start(); + threadA.join(TimeUnit.SECONDS.toMillis(10)); + threadB.join(TimeUnit.SECONDS.toMillis(10)); + + assertThat(failure.get()).isNull(); + assertThat(threadAFinalCount.get()).as("thread A must see only its own 3 statements").isEqualTo(3); + assertThat(threadBFinalCount.get()).as("thread B must see only its own 2 statements").isEqualTo(2); + } +} diff --git a/src/test/resources/application-test.properties b/src/test/resources/application-test.properties index 431f502d..e2f8ef01 100644 --- a/src/test/resources/application-test.properties +++ b/src/test/resources/application-test.properties @@ -19,6 +19,9 @@ spring.jpa.show-sql=false spring.jpa.properties.hibernate.format_sql=true # Per-thread prepared-statement counter used by UserRepositoryEntityGraphTest to guard against N+1 # regressions without reading the SessionFactory-global Statistics counter (see GH-337). +# Registered profile-wide by design: it is the simplest place to wire a StatementInspector onto the +# shared test SessionFactory. Every JPA test therefore runs a single ThreadLocal increment per prepared +# statement, which is an inert, negligible overhead for tests that never read the counter. spring.jpa.properties.hibernate.session_factory.statement_inspector=com.digitalsanctuary.spring.user.test.config.StatementCountInspector # Logging