HHH-16515 - Add o.h.stat to nullness checking

Signed-off-by: Jan Schatteman <jschatte@redhat.com>

checkerstubs/jboss.logging.astub

@@ -0,0 +1,9 @@
+// Checkerframework stubs for the jboss.logging module
+
+package org.jboss.logging;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+public interface BasicLogger {
+    void tracef(String format, @Nullable Object param1, @Nullable Object param2);
+}

gradle/java-module.gradle

@@ -525,7 +525,7 @@ checkerFramework {
 	extraJavacArgs = [
 	        '-AsuppressWarnings=initialization',
 			"-Astubs=${project.rootDir}/checkerstubs",
-			'-AonlyDefs=^org\\.hibernate\\.(jpamodelgen|spi|pretty|(action|context|bytecode)\\.spi)\\.'
+			'-AonlyDefs=^org\\.hibernate\\.(jpamodelgen|spi|pretty|stat|(action|context|bytecode)\\.spi)\\.'
 	]
 }
 

hibernate-core/src/main/java/org/hibernate/stat/CacheableDataStatistics.java

@@ -8,6 +8,8 @@ package org.hibernate.stat;
 
 import java.io.Serializable;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * @author Steve Ebersole
  */
@@ -17,7 +19,7 @@ public interface CacheableDataStatistics extends Serializable {
 	/**
 	 * The name of the region where this data is cached.
 	 */
-	String getCacheRegionName();
+	@Nullable String getCacheRegionName();
 
 	/**
 	 * The number of times this data has been into its configured cache region

hibernate-core/src/main/java/org/hibernate/stat/Statistics.java

@@ -8,6 +8,8 @@ package org.hibernate.stat;
 
 import java.time.Instant;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * Exposes statistics collected from all sessions belonging to a
  * particular {@link org.hibernate.SessionFactory}.
@@ -101,7 +103,7 @@ public interface Statistics {
 	 *         if either query result caching is not enabled, or no
 	 *         query cache region exists with the given name
 	 */
-	CacheRegionStatistics getQueryRegionStatistics(String regionName);
+	@Nullable CacheRegionStatistics getQueryRegionStatistics(String regionName);
 
 	/**
 	 * Obtain the statistics for either a domain data or query result
@@ -120,7 +122,7 @@ public interface Statistics {
 	 * @return the statistics for the named region, or {@code null} if
 	 *         there is no region with the given name
 	 */
-	CacheRegionStatistics getCacheRegionStatistics(String regionName);
+	@Nullable CacheRegionStatistics getCacheRegionStatistics(String regionName);
 
     /**
      * The global number of entity deletes.
@@ -160,7 +162,7 @@ public interface Statistics {
 	/**
 	 * The query string for the slowest query.
 	 */
-	String getQueryExecutionMaxTimeQueryString();
+	@Nullable String getQueryExecutionMaxTimeQueryString();
 
     /**
      * The global number of cached queries successfully retrieved from
@@ -194,12 +196,12 @@ public interface Statistics {
 	/**
 	 * The region for the maximum natural id query time.
 	 */
-	String getNaturalIdQueryExecutionMaxTimeRegion();
+	@Nullable String getNaturalIdQueryExecutionMaxTimeRegion();
 
 	/**
 	 * The entity name for the maximum natural id query time.
 	 */
-	String getNaturalIdQueryExecutionMaxTimeEntity();
+	@Nullable String getNaturalIdQueryExecutionMaxTimeEntity();
 
     /**
      * The global number of cached natural id lookups successfully
@@ -309,7 +311,7 @@ public interface Statistics {
 	Instant getStart();
 
 	/**
-	 * The {@linkplain Instant#toEpochMilli()}  milliseconds}) since the
+	 * The {@linkplain Instant#toEpochMilli()}  milliseconds since the
 	 * initial creation of this instance, or since the last time
 	 * {@link #clear()} was called.
 	 *

hibernate-core/src/main/java/org/hibernate/stat/internal/AbstractCacheableDataStatistics.java

@@ -10,18 +10,21 @@ import java.util.concurrent.atomic.LongAdder;
 import java.util.function.Supplier;
 
 import org.hibernate.cache.spi.Region;
+import org.hibernate.internal.util.NullnessUtil;
 import org.hibernate.stat.CacheableDataStatistics;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * @author Steve Ebersole
  */
 public abstract class AbstractCacheableDataStatistics implements CacheableDataStatistics {
-	private final String cacheRegionName;
+	private final @Nullable String cacheRegionName;
-	private final LongAdder cacheHitCount;
+	private final @Nullable LongAdder cacheHitCount;
-	private final LongAdder cacheMissCount;
+	private final @Nullable LongAdder cacheMissCount;
-	private final LongAdder cachePutCount;
+	private final @Nullable LongAdder cachePutCount;
 
-	public AbstractCacheableDataStatistics(Supplier<Region> regionSupplier) {
+	public AbstractCacheableDataStatistics(Supplier<@Nullable Region> regionSupplier) {
 		final Region region = regionSupplier.get();
 		if ( region == null ) {
 			this.cacheRegionName = null;
@@ -38,7 +41,7 @@ public abstract class AbstractCacheableDataStatistics implements CacheableDataSt
 	}
 
 	@Override
-	public String getCacheRegionName() {
+	public @Nullable String getCacheRegionName() {
 		return cacheRegionName;
 	}
 
@@ -47,7 +50,7 @@ public abstract class AbstractCacheableDataStatistics implements CacheableDataSt
 			return NOT_CACHED_COUNT;
 		}
 
-		return cacheHitCount.sum();
+		return NullnessUtil.castNonNull( cacheHitCount ).sum();
 	}
 
 	public long getCachePutCount() {
@@ -55,7 +58,7 @@ public abstract class AbstractCacheableDataStatistics implements CacheableDataSt
 			return NOT_CACHED_COUNT;
 		}
 
-		return cachePutCount.sum();
+		return NullnessUtil.castNonNull( cachePutCount ).sum();
 	}
 
 	public long getCacheMissCount() {
@@ -63,7 +66,7 @@ public abstract class AbstractCacheableDataStatistics implements CacheableDataSt
 			return NOT_CACHED_COUNT;
 		}
 
-		return cacheMissCount.sum();
+		return NullnessUtil.castNonNull( cacheMissCount ).sum();
 	}
 
 	public void incrementCacheHitCount() {
@@ -71,7 +74,7 @@ public abstract class AbstractCacheableDataStatistics implements CacheableDataSt
 			throw new IllegalStateException( "Illegal attempt to increment cache hit count for non-cached data" );
 		}
 
-		cacheHitCount.increment();
+		NullnessUtil.castNonNull( cacheHitCount ).increment();
 	}
 
 	public void incrementCacheMissCount() {
@@ -79,7 +82,7 @@ public abstract class AbstractCacheableDataStatistics implements CacheableDataSt
 			throw new IllegalStateException( "Illegal attempt to increment cache miss count for non-cached data" );
 		}
 
-		cacheMissCount.increment();
+		NullnessUtil.castNonNull( cacheMissCount ).increment();
 	}
 
 	public void incrementCachePutCount() {
@@ -87,7 +90,7 @@ public abstract class AbstractCacheableDataStatistics implements CacheableDataSt
 			throw new IllegalStateException( "Illegal attempt to increment cache put count for non-cached data" );
 		}
 
-		cachePutCount.increment();
+		NullnessUtil.castNonNull( cachePutCount ).increment();
 	}
 
 	protected void appendCacheStats(StringBuilder buf) {

hibernate-core/src/main/java/org/hibernate/stat/internal/StatisticsImpl.java

@@ -12,6 +12,7 @@ import java.util.List;
 import java.util.Objects;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.LongAdder;
+import java.util.function.Function;
 
 import org.hibernate.boot.spi.SessionFactoryOptions;
 import org.hibernate.cache.spi.CacheImplementor;
@@ -20,6 +21,7 @@ import org.hibernate.cache.spi.QueryResultsRegion;
 import org.hibernate.cache.spi.Region;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.internal.CoreMessageLogger;
+import org.hibernate.internal.util.NullnessUtil;
 import org.hibernate.metamodel.model.domain.NavigableRole;
 import org.hibernate.metamodel.spi.MappingMetamodelImplementor;
 import org.hibernate.persister.entity.EntityPersister;
@@ -27,6 +29,9 @@ import org.hibernate.service.Service;
 import org.hibernate.stat.Statistics;
 import org.hibernate.stat.spi.StatisticsImplementor;
 
+import org.checkerframework.checker.initialization.qual.UnknownInitialization;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 import static org.hibernate.internal.CoreLogging.messageLogger;
 
 /**
@@ -79,12 +84,12 @@ public class StatisticsImpl implements StatisticsImplementor, Service {
 	private final LongAdder naturalIdCachePutCount = new LongAdder();
 	private final LongAdder naturalIdQueryExecutionCount = new LongAdder();
 	private final AtomicLong naturalIdQueryExecutionMaxTime = new AtomicLong();
-	private volatile String naturalIdQueryExecutionMaxTimeRegion;
+	private volatile @Nullable String naturalIdQueryExecutionMaxTimeRegion;
-	private volatile String naturalIdQueryExecutionMaxTimeEntity;
+	private volatile @Nullable String naturalIdQueryExecutionMaxTimeEntity;
 
 	private final LongAdder queryExecutionCount = new LongAdder();
 	private final AtomicLong queryExecutionMaxTime = new AtomicLong();
-	private volatile String queryExecutionMaxTimeQueryString;
+	private volatile @Nullable String queryExecutionMaxTimeQueryString;
 	private final LongAdder queryCacheHitCount = new LongAdder();
 	private final LongAdder queryCacheMissCount = new LongAdder();
 	private final LongAdder queryCachePutCount = new LongAdder();
@@ -201,7 +206,7 @@ public class StatisticsImpl implements StatisticsImplementor, Service {
 		resetStart();
 	}
 
-	private void resetStart() {
+	private void resetStart(@UnknownInitialization StatisticsImpl this) {
 		startTime = Instant.now();
 	}
 
@@ -237,9 +242,11 @@ public class StatisticsImpl implements StatisticsImplementor, Service {
 
 	@Override
 	public EntityStatisticsImpl getEntityStatistics(String entityName) {
-		return entityStatsMap.getOrCompute(
+		return NullnessUtil.castNonNull(
-				entityName,
+					entityStatsMap.getOrCompute(
-				this::instantiateEntityStatistics
+							entityName,
+							this::instantiateEntityStatistics
+					)
 		);
 	}
 
@@ -341,10 +348,12 @@ public class StatisticsImpl implements StatisticsImplementor, Service {
 
 	@Override
 	public CollectionStatisticsImpl getCollectionStatistics(String role) {
-		return collectionStatsMap.getOrCompute(
+		return NullnessUtil.castNonNull(
-				role,
+					collectionStatsMap.getOrCompute(
-				this::instantiateCollectionStatistics
+						role,
-		);
+						this::instantiateCollectionStatistics
+					)
+				);
 	}
 
 	@Override
@@ -429,9 +438,11 @@ public class StatisticsImpl implements StatisticsImplementor, Service {
 
 	@Override
 	public NaturalIdStatisticsImpl getNaturalIdStatistics(String rootEntityName) {
-		return naturalIdQueryStatsMap.getOrCompute(
+		return NullnessUtil.castNonNull(
-				rootEntityName,
+					naturalIdQueryStatsMap.getOrCompute(
-				this::instantiateNaturalStatistics
+						rootEntityName,
+						this::instantiateNaturalStatistics
+					)
 		);
 	}
 
@@ -446,12 +457,12 @@ public class StatisticsImpl implements StatisticsImplementor, Service {
 	}
 
 	@Override
-	public String getNaturalIdQueryExecutionMaxTimeRegion() {
+	public @Nullable String getNaturalIdQueryExecutionMaxTimeRegion() {
 		return naturalIdQueryExecutionMaxTimeRegion;
 	}
 
 	@Override
-	public String getNaturalIdQueryExecutionMaxTimeEntity() {
+	public @Nullable String getNaturalIdQueryExecutionMaxTimeEntity() {
 		return naturalIdQueryExecutionMaxTimeEntity;
 	}
 
@@ -540,18 +551,29 @@ public class StatisticsImpl implements StatisticsImplementor, Service {
 
 	@Override
 	public CacheRegionStatisticsImpl getDomainDataRegionStatistics(String regionName) {
-		return l2CacheStatsMap.getOrCompute(
+		return NullnessUtil.castNonNull(
-				regionName,
+					l2CacheStatsMap.getOrCompute(
-				this::instantiateCacheRegionStatistics
+						regionName,
+						this::instantiateCacheRegionStatistics
+					)
 		);
 	}
 
 	@Override
-	public CacheRegionStatisticsImpl getQueryRegionStatistics(final String regionName) {
+	public @Nullable CacheRegionStatisticsImpl getQueryRegionStatistics(final String regionName) {
-		return l2CacheStatsMap.getOrCompute( regionName, this::computeQueryRegionStatistics );
+		return l2CacheStatsMap.getOrCompute(
+				regionName,
+
+				new Function<>() {
+					@Override
+					public @Nullable CacheRegionStatisticsImpl apply(String regionName1) {
+						return StatisticsImpl.this.computeQueryRegionStatistics( regionName1 );
+					}
+				}
+		);
 	}
 
-	private CacheRegionStatisticsImpl computeQueryRegionStatistics(final String regionName) {
+	private @Nullable CacheRegionStatisticsImpl computeQueryRegionStatistics(final String regionName) {
 		final QueryResultsCache regionAccess = cache.getQueryResultsCacheStrictly( regionName );
 		if ( regionAccess == null ) {
 			return null; //this null value will be cached
@@ -563,14 +585,19 @@ public class StatisticsImpl implements StatisticsImplementor, Service {
 
 
 	@Override
-	public CacheRegionStatisticsImpl getCacheRegionStatistics(String regionName) {
+	public @Nullable CacheRegionStatisticsImpl getCacheRegionStatistics(String regionName) {
 		if ( ! secondLevelCacheEnabled ) {
 			return null;
 		}
 
 		return l2CacheStatsMap.getOrCompute(
 				regionName,
-				this::createCacheRegionStatistics
+				new Function<>() {
+					@Override
+					public @Nullable CacheRegionStatisticsImpl apply(String regionName1) {
+						return StatisticsImpl.this.createCacheRegionStatistics( regionName1 );
+					}
+				}
 		);
 	}
 
@@ -630,9 +657,11 @@ public class StatisticsImpl implements StatisticsImplementor, Service {
 
 	@Override
 	public QueryStatisticsImpl getQueryStatistics(String queryString) {
-		return queryStatsMap.getOrCompute(
+		return NullnessUtil.castNonNull(
-				queryString,
+					queryStatsMap.getOrCompute(
-				QueryStatisticsImpl::new
+						queryString,
+						QueryStatisticsImpl::new
+					)
 		);
 	}
 
@@ -657,7 +686,7 @@ public class StatisticsImpl implements StatisticsImplementor, Service {
 	}
 
 	@Override
-	public String getQueryExecutionMaxTimeQueryString() {
+	public @Nullable String getQueryExecutionMaxTimeQueryString() {
 		return queryExecutionMaxTimeQueryString;
 	}
 
@@ -765,9 +794,11 @@ public class StatisticsImpl implements StatisticsImplementor, Service {
 	}
 
 	private CacheRegionStatisticsImpl getQueryRegionStats(String regionName) {
-		return l2CacheStatsMap.getOrCompute(
+		return NullnessUtil.castNonNull(
-				regionName,
+					l2CacheStatsMap.getOrCompute(
-				this::instantiateCacheRegionStatsForQueryResults
+						regionName,
+						this::instantiateCacheRegionStatsForQueryResults
+					)
 		);
 	}
 
@@ -975,7 +1006,7 @@ public class StatisticsImpl implements StatisticsImplementor, Service {
 		return new CacheRegionStatisticsImpl( cache.getQueryResultsCache( regionName ).getRegion() );
 	}
 
-	private CacheRegionStatisticsImpl createCacheRegionStatistics(final String regionName) {
+	private @Nullable CacheRegionStatisticsImpl createCacheRegionStatistics(final String regionName) {
 		Region region = cache.getRegion( regionName );
 
 		if ( region == null ) {

hibernate-core/src/main/java/org/hibernate/stat/internal/StatisticsInitiator.java

@@ -19,6 +19,8 @@ import org.hibernate.stat.spi.StatisticsImplementor;
 
 import org.jboss.logging.Logger;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * @author Steve Ebersole
  */
@@ -49,7 +51,7 @@ public class StatisticsInitiator implements SessionFactoryServiceInitiator<Stati
 
 	private StatisticsImplementor initiateServiceInternal(
 			SessionFactoryImplementor sessionFactory,
-			Object configValue,
+			@Nullable Object configValue,
 			ServiceRegistryImplementor registry) {
 
 		final StatisticsFactory statisticsFactory;

hibernate-core/src/main/java/org/hibernate/stat/internal/StatsNamedContainer.java

@@ -10,8 +10,11 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.function.Function;
 
+import org.hibernate.internal.util.NullnessUtil;
 import org.hibernate.internal.util.collections.BoundedConcurrentHashMap;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * Decorates a ConcurrentHashMap implementation to make sure the methods are being
  * used correctly for the purpose of Hibernate's statistics.
@@ -64,7 +67,7 @@ public final class StatsNamedContainer<V> {
 	 * sure the function is invoked at most once: we don't need this guarantee, and prefer to reduce risk of blocking.
 	 */
 	@SuppressWarnings("unchecked")
-	public V getOrCompute(final String key, final Function<String, V> function) {
+	public @Nullable V getOrCompute(final String key, final Function<String, V> function) {
 		final Object v1 = map.get( key );
 		if ( v1 != null ) {
 			if ( v1 == NULL_TOKEN ) {
@@ -90,7 +93,7 @@ public final class StatsNamedContainer<V> {
 		}
 	}
 
-	public V get(final String key) {
+	public @Nullable V get(final String key) {
 		final Object o = map.get( key );
 		if ( o == NULL_TOKEN) {
 			return null;

hibernate-micrometer/src/main/java/org/hibernate/stat/HibernateMetrics.java

@@ -6,21 +6,19 @@
  */
 package org.hibernate.stat;
 
+import io.micrometer.common.lang.NonNullApi;
+import io.micrometer.common.lang.NonNullFields;
 import io.micrometer.core.instrument.FunctionCounter;
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.Tags;
 import io.micrometer.core.instrument.TimeGauge;
 import io.micrometer.core.instrument.binder.MeterBinder;
-import io.micrometer.core.lang.NonNullApi;
-import io.micrometer.core.lang.NonNullFields;
-import io.micrometer.core.lang.Nullable;
 
 import org.hibernate.SessionFactory;
-import org.hibernate.stat.Statistics;
 
-import jakarta.persistence.EntityManagerFactory;
+import org.checkerframework.checker.nullness.qual.Nullable;
-import jakarta.persistence.PersistenceException;
+
 import java.util.Arrays;
 import java.util.concurrent.TimeUnit;
 import java.util.function.ToDoubleFunction;
@@ -38,8 +36,7 @@ public class HibernateMetrics implements MeterBinder {
 	private final String cacheFactoryPrefix;
 	private final Iterable<Tag> tags;
 
-	@Nullable
+	private final @Nullable Statistics statistics;
-	private final Statistics statistics;
 
 	/**
 	 * Create {@code HibernateMetrics} and bind to the specified meter registry.
@@ -342,28 +339,11 @@ public class HibernateMetrics implements MeterBinder {
 		// In 5.3, getDomainDataRegionStatistics (a new method) will throw an IllegalArgumentException
 		// if the region can't be resolved.
 		try {
-			return statistics.getDomainDataRegionStatistics( regionName ) != null;
+			return statistics != null && statistics.getDomainDataRegionStatistics( regionName ) != null;
 		}
 		catch (IllegalArgumentException e) {
 			return false;
 		}
 	}
 
-	/**
-	 * Unwrap the {@link SessionFactory} from {@link EntityManagerFactory}.
-	 *
-	 * @param entityManagerFactory {@link EntityManagerFactory} to unwrap
-	 *
-	 * @return unwrapped {@link SessionFactory}
-	 */
-	@Nullable
-	private SessionFactory unwrap(EntityManagerFactory entityManagerFactory) {
-		try {
-			return entityManagerFactory.unwrap( SessionFactory.class );
-		}
-		catch (PersistenceException ex) {
-			return null;
-		}
-	}
-
 }