fix session.getProperties()

hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java

@@ -86,7 +86,6 @@
 import static java.lang.Boolean.parseBoolean;
 import static java.lang.Integer.parseInt;
 import static java.lang.System.currentTimeMillis;
-import static java.util.Collections.unmodifiableMap;
 import static org.hibernate.CacheMode.fromJpaModes;
 import static org.hibernate.Timeouts.WAIT_FOREVER_MILLI;
 import static org.hibernate.cfg.AvailableSettings.CRITERIA_COPY_TREE;
@@ -2584,13 +2583,14 @@ public LockModeType getLockMode(Object entity) {
 	public void setProperty(String propertyName, Object value) {
 		checkOpen();
 		if ( value instanceof Serializable ) {
-			if ( propertyName != null ) { // store property for future reference:
+			if ( propertyName != null ) {
+				// store property for future reference
 				if ( properties == null ) {
-					properties = computeCurrentProperties();
+					properties = getInitialProperties();
 				}
 				properties.put( propertyName, value );
-				// now actually update the setting
-				// if it's one that affects this Session
+				// now actually update the setting if
+				// it's one that affects this Session
 				interpretProperty( propertyName, value );
 			}
 			else {
@@ -2662,7 +2662,7 @@ private void interpretProperty(String propertyName, Object value) {
 		}
 	}
 
-	private Map<String, Object> computeCurrentProperties() {
+	private Map<String, Object> getInitialProperties() {
 		final var map = new HashMap<>( getDefaultProperties() );
 		//The FLUSH_MODE is always set at Session creation time,
 		//so it needs special treatment to not eagerly initialize this Map:
@@ -2672,10 +2672,15 @@ private Map<String, Object> computeCurrentProperties() {
 
 	@Override
 	public Map<String, Object> getProperties() {
-		if ( properties == null ) {
-			properties = computeCurrentProperties();
-		}
-		return unmodifiableMap( properties );
+		// EntityManager Javadoc implies that the
+		// returned map should be a mutable copy,
+		// not an unmodifiable map. There's no
+		// good reason to cache the initial
+		// properties, since we have to copy them
+		// each time this method is called.
+		return properties == null
+				? getInitialProperties()
+				: new HashMap<>( properties );
 	}
 
 	@Override

hibernate-core/src/test/java/org/hibernate/orm/test/jpa/EntityManagerTest.java

@@ -307,13 +307,11 @@ public void testGet() throws Exception {
 	@Test
 	public void testGetProperties() {
 		inEntityManager( entityManager -> {
-			Map<String, Object> properties = entityManager.getProperties();
-			assertNotNull( properties );
-			assertThrows(
-					UnsupportedOperationException.class,
-					() -> properties.put( "foo", "bar" )
-			);
-			assertTrue( properties.containsKey( HibernateHints.HINT_FLUSH_MODE ) );
+			assertNotNull( entityManager.getProperties() );
+			assertTrue( entityManager.getProperties().containsKey( HibernateHints.HINT_FLUSH_MODE ) );
+			// according to Javadoc, getProperties() returns mutable copy
+			entityManager.getProperties().put( "foo", "bar" );
+			assertFalse( entityManager.getProperties().containsKey( "foo" ) );
 		} );
 	}
 

tooling/metamodel-generator/src/main/java/org/hibernate/processor/annotation/AnnotationMetaEntity.java

@@ -936,13 +936,12 @@ void addEventBus() {
 
 	/**
 	 * For usage with CDI, but outside Quarkus, Jakarta Data
-	 * repositories use {@code @PersistenceUnit} to obtain an
-	 * {@code EntityManagerFactory} via field injection. So in
-	 * that case we will need a {@link DefaultConstructor default
-	 * constructor}. We don't do this in Quarkus, because there
-	 * we can just inject the {@code StatelessSession} directly,
-	 * and so in Quarkus we don't need the default constructor
-	 * at all.
+	 * repositories use {@code @PersistenceUnit} to obtain
+	 * an {@code EntityManagerFactory} via field injection.
+	 * So here we need a {@linkplain DefaultConstructor
+	 * default constructor}. We don't need one in Quarkus,
+	 * because in Quarkus we can inject a container-managed
+	 * {@code StatelessSession} directly.
 	 */
 	boolean needsDefaultConstructor() {
 		return jakartaDataRepository

tooling/metamodel-generator/src/main/java/org/hibernate/processor/annotation/DefaultConstructor.java

@@ -21,6 +21,13 @@
 
 /**
  * Used by the container to instantiate a Jakarta Data repository.
+ * This is a constructor with no parameters, used to instantiate
+ * a repository which then uses field injection to obtain its
+ * dependencies. By contrast, a {@link RepositoryConstructor} has
+ * a parameter which accepts the session as an argument, allowing
+ * direct instantiation or constructor injection. This class is
+ * only needed because {@code @PersistenceUnit} is incompatible
+ * with CDI-style constructor injection.
  *
  * @author Gavin King
  */

tooling/metamodel-generator/src/main/java/org/hibernate/processor/annotation/RepositoryConstructor.java

@@ -14,7 +14,11 @@
 import static org.hibernate.processor.util.Constants.NONNULL;
 
 /**
- * A general purpose constructor which accepts the session.
+ * A general purpose constructor which accepts the session as an
+ * argument. This constructor is compatible with use via direct
+ * instantiation or CDI-style constructor injection. By contrast,
+ * {@link DefaultConstructor} is used to instantiate a repository
+ * which obtains its session using field injection.
  *
  * @author Gavin King
  */