uniform reporting of null argument to EM, Session, and StatelessSession methods

gavinking · gavinking · commit 7d9c170d1726 · 2025-06-01T13:44:12.000+02:00
diff --git a/hibernate-core/src/main/java/org/hibernate/Session.java b/hibernate-core/src/main/java/org/hibernate/Session.java
@@ -488,8 +488,7 @@ public interface Session extends SharedSessionContract, EntityManager {
 	 *
 	 * @param object the managed entity to evict
 	 *
-	 * @throws NullPointerException if the passed object is {@code null}
-	 * @throws IllegalArgumentException if the passed object is not mapped as an entity
+	 * @throws IllegalArgumentException if the given object is not an entity
 	 */
 	void evict(Object object);
 
@@ -1026,7 +1025,10 @@ public interface Session extends SharedSessionContract, EntityManager {
 	void refresh(Object object, LockOptions lockOptions);
 
 	/**
-	 * Return the entity name for a persistent entity.
+	 * Return the entity name for the given persistent entity.
+	 * <p>
+	 * If the given entity is an uninitialized proxy, the proxy is initialized by
+	 * side effect.
 	 *
 	 * @param object a persistent entity associated with this session
 	 *
diff --git a/hibernate-core/src/main/java/org/hibernate/event/spi/DeleteEvent.java b/hibernate-core/src/main/java/org/hibernate/event/spi/DeleteEvent.java
@@ -25,7 +25,7 @@ public class DeleteEvent extends AbstractEvent {
 	public DeleteEvent(Object object, EventSource source) {
 		super(source);
 		if (object == null) {
-			throw new IllegalArgumentException( "attempt to create delete event with null entity" );
+			throw new IllegalArgumentException( "Entity may not be null" );
 		}
 		this.object = object;
 	}
diff --git a/hibernate-core/src/main/java/org/hibernate/event/spi/EvictEvent.java b/hibernate-core/src/main/java/org/hibernate/event/spi/EvictEvent.java
@@ -19,6 +19,9 @@ public class EvictEvent extends AbstractEvent {
 
 	public EvictEvent(Object object, EventSource source) {
 		super(source);
+		if (object == null) {
+			throw new IllegalArgumentException( "Entity may not be null" );
+		}
 		this.object = object;
 	}
 
diff --git a/hibernate-core/src/main/java/org/hibernate/event/spi/LockEvent.java b/hibernate-core/src/main/java/org/hibernate/event/spi/LockEvent.java
@@ -33,12 +33,24 @@ public LockEvent(String entityName, Object original, LockOptions lockOptions, Ev
 
 	public LockEvent(Object object, LockMode lockMode, EventSource source) {
 		super(source);
+		if (object == null) {
+			throw new IllegalArgumentException( "Entity may not be null" );
+		}
+		if (lockMode == null) {
+			throw new IllegalArgumentException( "LockMode may not be null" );
+		}
 		this.object = object;
 		this.lockOptions = lockMode.toLockOptions();
 	}
 
 	public LockEvent(Object object, LockOptions lockOptions, EventSource source) {
 		super(source);
+		if (object == null) {
+			throw new IllegalArgumentException( "Entity may not be null" );
+		}
+		if (lockOptions == null) {
+			throw new IllegalArgumentException( "LockOptions may not be null" );
+		}
 		this.object = object;
 		this.lockOptions = lockOptions;
 	}
diff --git a/hibernate-core/src/main/java/org/hibernate/event/spi/MergeEvent.java b/hibernate-core/src/main/java/org/hibernate/event/spi/MergeEvent.java
@@ -28,18 +28,14 @@ public MergeEvent(String entityName, Object original, Object id, EventSource sou
 		this(entityName, original, source);
 		this.requestedId = id;
 		if ( requestedId == null ) {
-			throw new IllegalArgumentException(
-					"attempt to create merge event with null identifier"
-				);
+			throw new IllegalArgumentException( "Identifier may not be null" );
 		}
 	}
 
 	public MergeEvent(Object object, EventSource source) {
 		super(source);
 		if ( object == null ) {
-			throw new IllegalArgumentException(
-					"attempt to create merge event with null entity"
-				);
+			throw new IllegalArgumentException( "Entity may not be null" );
 		}
 		this.original = object;
 	}
diff --git a/hibernate-core/src/main/java/org/hibernate/event/spi/PersistEvent.java b/hibernate-core/src/main/java/org/hibernate/event/spi/PersistEvent.java
@@ -24,9 +24,7 @@ public PersistEvent(String entityName, Object original, EventSource source) {
 	public PersistEvent(Object object, EventSource source) {
 		super(source);
 		if ( object == null ) {
-			throw new IllegalArgumentException(
-					"attempt to create event with null entity"
-			);
+			throw new IllegalArgumentException( "Entity may not be null" );
 		}
 		this.object = object;
 	}
diff --git a/hibernate-core/src/main/java/org/hibernate/event/spi/RefreshEvent.java b/hibernate-core/src/main/java/org/hibernate/event/spi/RefreshEvent.java
@@ -25,7 +25,7 @@ public class RefreshEvent extends AbstractEvent {
 	public RefreshEvent(Object object, EventSource source) {
 		super(source);
 		if (object == null) {
-			throw new IllegalArgumentException("Attempt to generate refresh event with null object");
+			throw new IllegalArgumentException("Entity may not be null");
 		}
 		this.object = object;
 	}
@@ -38,15 +38,15 @@ public RefreshEvent(String entityName, Object object, EventSource source){
 	public RefreshEvent(Object object, LockMode lockMode, EventSource source) {
 		this(object, source);
 		if (lockMode == null) {
-			throw new IllegalArgumentException("Attempt to generate refresh event with null lock mode");
+			throw new IllegalArgumentException("LockMode may not be null");
 		}
 		this.lockOptions.setLockMode(lockMode);
 	}
 
 	public RefreshEvent(Object object, LockOptions lockOptions, EventSource source) {
 		this(object, source);
 		if (lockOptions == null) {
-			throw new IllegalArgumentException("Attempt to generate refresh event with null lock request");
+			throw new IllegalArgumentException("LockMode may not be null");
 		}
 		this.lockOptions = lockOptions;
 	}
diff --git a/hibernate-core/src/main/java/org/hibernate/event/spi/ReplicateEvent.java b/hibernate-core/src/main/java/org/hibernate/event/spi/ReplicateEvent.java
@@ -27,14 +27,10 @@ public ReplicateEvent(String entityName, Object object, ReplicationMode replicat
 		this.entityName = entityName;
 
 		if ( object == null ) {
-			throw new IllegalArgumentException(
-					"attempt to create replication strategy with null entity"
-			);
+			throw new IllegalArgumentException( "Entity may not be null" );
 		}
 		if ( replicationMode == null ) {
-			throw new IllegalArgumentException(
-					"attempt to create replication strategy with null replication mode"
-			);
+			throw new IllegalArgumentException( "ReplicationMode may not be null" );
 		}
 
 		this.object = object;
diff --git a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java
@@ -1550,6 +1550,9 @@ public EntityPersister getEntityPersister(final String entityName, final Object
 	public Object getIdentifier(Object object) {
 		checkOpen();
 		checkTransactionSynchStatus();
+		if ( object == null ) {
+			throw new IllegalArgumentException( "Entity may not be null" );
+		}
 		final LazyInitializer lazyInitializer = extractLazyInitializer( object );
 		if ( lazyInitializer != null ) {
 			if ( lazyInitializer.getSession() != this ) {
@@ -1809,6 +1812,11 @@ public String bestGuessEntityName(Object object, EntityEntry entry) {
 	public String getEntityName(Object object) {
 		checkOpen();
 //		checkTransactionSynchStatus();
+
+		if ( object == null ) {
+			throw new IllegalArgumentException( "Entity may not be null" );
+		}
+
 		final LazyInitializer lazyInitializer = extractLazyInitializer( object );
 		if ( lazyInitializer != null ) {
 			if ( !persistenceContext.containsProxy( object ) ) {
@@ -2774,8 +2782,13 @@ public LockModeType getLockMode(Object entity) {
 			throw new TransactionRequiredException( "No active transaction" );
 		}
 
+		if ( entity == null ) {
+			throw new IllegalArgumentException( "Entity may not be null" );
+		}
 		if ( !contains( entity ) ) {
-			throw getExceptionConverter().convert( new IllegalArgumentException( "Entity not associated with the persistence context" ) );
+			// convert() calls markForRollbackOnly()
+			throw getExceptionConverter()
+					.convert( new IllegalArgumentException( "Entity not associated with the persistence context" ) );
 		}
 
 		return LockModeTypeHelper.getLockModeType( getCurrentLockMode( entity ) );
diff --git a/hibernate-core/src/main/java/org/hibernate/internal/StatelessSessionImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/StatelessSessionImpl.java
@@ -1207,15 +1207,21 @@ public Object getContextEntityIdentifier(Object object) {
 	@Override
 	public String guessEntityName(Object entity) {
 		checkOpen();
+		if ( entity == null ) {
+			throw new IllegalArgumentException( "Entity may not be null" );
+		}
 		return entity.getClass().getName();
 	}
 
 	@Override
-	public EntityPersister getEntityPersister(String entityName, Object object) {
+	public EntityPersister getEntityPersister(String entityName, Object entity) {
 		checkOpen();
+		if ( entity == null ) {
+			throw new IllegalArgumentException( "Entity may not be null" );
+		}
 		return entityName == null
-				? requireEntityPersister( guessEntityName( object ) )
-				: requireEntityPersister( entityName ).getSubclassEntityPersister( object, getFactory() );
+				? requireEntityPersister( guessEntityName( entity ) )
+				: requireEntityPersister( entityName ).getSubclassEntityPersister( entity, getFactory() );
 	}
 
 	@Override
diff --git a/hibernate-core/src/main/java/org/hibernate/jpa/internal/PersistenceUnitUtilImpl.java b/hibernate-core/src/main/java/org/hibernate/jpa/internal/PersistenceUnitUtilImpl.java
@@ -83,7 +83,7 @@ public <T> Class<? extends T> getClass(T entity) {
 	@Override
 	public Object getIdentifier(Object entity) {
 		if ( entity == null ) {
-			throw new IllegalArgumentException( "Passed entity cannot be null" );
+			throw new IllegalArgumentException( "Entity may not be null" );
 		}
 
 		final LazyInitializer lazyInitializer = extractLazyInitializer( entity );
@@ -107,7 +107,7 @@ else if ( isManagedEntity( entity ) ) {
 	@Override
 	public Object getVersion(Object entity) {
 		if ( entity == null ) {
-			throw new IllegalArgumentException( "Passed entity cannot be null" );
+			throw new IllegalArgumentException( "Entity may not be null" );
 		}
 
 		final LazyInitializer lazyInitializer = extractLazyInitializer( entity );
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/eviction/EvictionTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/eviction/EvictionTest.java
@@ -55,9 +55,9 @@ public void testEvictingNull() {
 		session.beginTransaction();
 		try {
 			session.evict( null );
-			fail( "Expecting evict(null) to throw NPE" );
+			fail( "Expecting evict(null) to throw IAE" );
 		}
-		catch (NullPointerException expected) {
+		catch (IllegalArgumentException expected) {
 		}
 		session.getTransaction().commit();
 		session.close();
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/NullEntityManagerArgumentsTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/NullEntityManagerArgumentsTest.java
@@ -0,0 +1,41 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
+package org.hibernate.orm.test.nullargs;
+
+import jakarta.persistence.LockModeType;
+import org.hibernate.testing.orm.junit.EntityManagerFactoryScope;
+import org.hibernate.testing.orm.junit.Jpa;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+import java.util.function.Consumer;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@Jpa
+class NullEntityManagerArgumentsTest {
+	@Test void test(EntityManagerFactoryScope scope) {
+		scope.inTransaction( em -> {
+			var operations = List.<Consumer<?>>of(
+					em::persist,
+					em::refresh,
+					em::merge,
+					em::detach,
+					em::remove,
+					e -> em.lock( e, LockModeType.NONE ),
+					em::getLockMode,
+					em::contains
+			);
+			operations.forEach( c -> {
+						try {
+							c.accept( null );
+						}
+						catch ( IllegalArgumentException e ) {
+							assertTrue( e.getMessage().startsWith( "Entity may not be null" ) );
+						}
+					} );
+		} );
+	}
+}
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/NullSessionArgumentsTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/NullSessionArgumentsTest.java
@@ -0,0 +1,67 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
+package org.hibernate.orm.test.nullargs;
+
+import jakarta.persistence.LockModeType;
+import org.hibernate.ReplicationMode;
+import org.hibernate.testing.orm.junit.DomainModel;
+import org.hibernate.testing.orm.junit.SessionFactory;
+import org.hibernate.testing.orm.junit.SessionFactoryScope;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+import java.util.function.BiConsumer;
+import java.util.function.Consumer;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@SessionFactory @DomainModel
+class NullSessionArgumentsTest {
+	@Test void test(SessionFactoryScope scope) {
+		scope.getSessionFactory().inTransaction( session -> {
+			var operations = List.<Consumer<?>>of(
+					session::persist,
+					session::refresh,
+					session::merge,
+					session::detach,
+					session::evict,
+					session::remove,
+					e -> session.lock( e, LockModeType.NONE ),
+					e -> session.replicate( e, ReplicationMode.EXCEPTION ),
+					session::getLockMode,
+					session::getEntityName,
+					session::getIdentifier,
+					session::contains
+			);
+			operations.forEach( c -> {
+						try {
+							c.accept( null );
+						}
+						catch ( IllegalArgumentException e ) {
+							assertTrue( e.getMessage().startsWith( "Entity may not be null" ) );
+						}
+					} );
+		} );
+	}
+	@Test void testWithEntityName(SessionFactoryScope scope) {
+		scope.inStatelessTransaction( ss -> {
+			var operations = List.<BiConsumer<String,Object>>of(
+					ss::insert,
+					ss::upsert,
+					ss::update,
+					ss::delete,
+					ss::refresh
+			);
+			operations.forEach( c -> {
+				try {
+					c.accept( "Entity", null );
+				}
+				catch ( IllegalArgumentException e ) {
+					assertTrue( e.getMessage().startsWith( "Entity may not be null" ) );
+				}
+			} );
+		} );
+	}
+}
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/NullStatelessSessionArgumentsTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/nullargs/NullStatelessSessionArgumentsTest.java

Original file line number	Diff line number	Diff line change
`@@ -488,8 +488,7 @@ public interface Session extends SharedSessionContract, EntityManager {`
`488`	`488`	`*`
`489`	`489`	`* @param object the managed entity to evict`
`490`	`490`	`*`
`491`		`- * @throws NullPointerException if the passed object is {@code null}`
`492`		`- * @throws IllegalArgumentException if the passed object is not mapped as an entity`
	`491`	`+ * @throws IllegalArgumentException if the given object is not an entity`
`493`	`492`	`*/`
`494`	`493`	`void evict(Object object);`
`495`	`494`
`@@ -1026,7 +1025,10 @@ public interface Session extends SharedSessionContract, EntityManager {`
`1026`	`1025`	`void refresh(Object object, LockOptions lockOptions);`
`1027`	`1026`
`1028`	`1027`	`/**`
`1029`		`- * Return the entity name for a persistent entity.`
	`1028`	`+ * Return the entity name for the given persistent entity.`
	`1029`	`+ * <p>`
	`1030`	`+ * If the given entity is an uninitialized proxy, the proxy is initialized by`
	`1031`	`+ * side effect.`
`1030`	`1032`	`*`
`1031`	`1033`	`* @param object a persistent entity associated with this session`
`1032`	`1034`	`*`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ public class DeleteEvent extends AbstractEvent {`
`25`	`25`	`public DeleteEvent(Object object, EventSource source) {`
`26`	`26`	`super(source);`
`27`	`27`	`if (object == null) {`
`28`		`- throw new IllegalArgumentException( "attempt to create delete event with null entity" );`
	`28`	`+ throw new IllegalArgumentException( "Entity may not be null" );`
`29`	`29`	`}`
`30`	`30`	`this.object = object;`
`31`	`31`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,9 @@ public class EvictEvent extends AbstractEvent {`
`19`	`19`
`20`	`20`	`public EvictEvent(Object object, EventSource source) {`
`21`	`21`	`super(source);`
	`22`	`+ if (object == null) {`
	`23`	`+ throw new IllegalArgumentException( "Entity may not be null" );`
	`24`	`+ }`
`22`	`25`	`this.object = object;`
`23`	`26`	`}`
`24`	`27`
Original file line number	Diff line number	Diff line change
`@@ -28,18 +28,14 @@ public MergeEvent(String entityName, Object original, Object id, EventSource sou`
`28`	`28`	`this(entityName, original, source);`
`29`	`29`	`this.requestedId = id;`
`30`	`30`	`if ( requestedId == null ) {`
`31`		`- throw new IllegalArgumentException(`
`32`		`- "attempt to create merge event with null identifier"`
`33`		`- );`
	`31`	`+ throw new IllegalArgumentException( "Identifier may not be null" );`
`34`	`32`	`}`
`35`	`33`	`}`
`36`	`34`
`37`	`35`	`public MergeEvent(Object object, EventSource source) {`
`38`	`36`	`super(source);`
`39`	`37`	`if ( object == null ) {`
`40`		`- throw new IllegalArgumentException(`
`41`		`- "attempt to create merge event with null entity"`
`42`		`- );`
	`38`	`+ throw new IllegalArgumentException( "Entity may not be null" );`
`43`	`39`	`}`
`44`	`40`	`this.original = object;`
`45`	`41`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,9 +24,7 @@ public PersistEvent(String entityName, Object original, EventSource source) {`
`24`	`24`	`public PersistEvent(Object object, EventSource source) {`
`25`	`25`	`super(source);`
`26`	`26`	`if ( object == null ) {`
`27`		`- throw new IllegalArgumentException(`
`28`		`- "attempt to create event with null entity"`
`29`		`- );`
	`27`	`+ throw new IllegalArgumentException( "Entity may not be null" );`
`30`	`28`	`}`
`31`	`29`	`this.object = object;`
`32`	`30`	`}`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ public class RefreshEvent extends AbstractEvent {`
`25`	`25`	`public RefreshEvent(Object object, EventSource source) {`
`26`	`26`	`super(source);`
`27`	`27`	`if (object == null) {`
`28`		`- throw new IllegalArgumentException("Attempt to generate refresh event with null object");`
	`28`	`+ throw new IllegalArgumentException("Entity may not be null");`
`29`	`29`	`}`
`30`	`30`	`this.object = object;`
`31`	`31`	`}`
`@@ -38,15 +38,15 @@ public RefreshEvent(String entityName, Object object, EventSource source){`
`38`	`38`	`public RefreshEvent(Object object, LockMode lockMode, EventSource source) {`
`39`	`39`	`this(object, source);`
`40`	`40`	`if (lockMode == null) {`
`41`		`- throw new IllegalArgumentException("Attempt to generate refresh event with null lock mode");`
	`41`	`+ throw new IllegalArgumentException("LockMode may not be null");`
`42`	`42`	`}`
`43`	`43`	`this.lockOptions.setLockMode(lockMode);`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`public RefreshEvent(Object object, LockOptions lockOptions, EventSource source) {`
`47`	`47`	`this(object, source);`
`48`	`48`	`if (lockOptions == null) {`
`49`		`- throw new IllegalArgumentException("Attempt to generate refresh event with null lock request");`
	`49`	`+ throw new IllegalArgumentException("LockMode may not be null");`
`50`	`50`	`}`
`51`	`51`	`this.lockOptions = lockOptions;`
`52`	`52`	`}`