Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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.
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
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 <em>per-thread</em> basis.
*
* <p>
* 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.
*
* <p>
* 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.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.
*
* <p>
* 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;

// 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<Integer> 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;
}
}
Original file line number Diff line number Diff line change
@@ -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 <em>after</em> thread A has counted but
* <em>before</em> 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<Throwable> 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);
}
}
6 changes: 6 additions & 0 deletions src/test/resources/application-test.properties
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ 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).
# 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
logging.level.com.digitalsanctuary.spring.user=INFO
Expand Down
Loading