HHH-3930 Test and fix for unnecessary query that is issued when fetching inverse one-to-one

hibernate-core/src/main/java/org/hibernate/cfg/ToOneFkSecondPass.java

@@ -97,6 +97,7 @@ public class ToOneFkSecondPass extends FkSecondPass {
 								+ manyToOne.getReferencedEntityName()
 				);
 			}
+			manyToOne.setPropertyName( path );
 			BinderHelper.createSyntheticPropertyReference( columns, ref, null, manyToOne, false, buildingContext );
 			TableBinder.bindFk( ref, null, columns, manyToOne, unique, buildingContext );
 			/*

hibernate-core/src/main/java/org/hibernate/mapping/ManyToOne.java

@@ -32,6 +32,7 @@ public class ManyToOne extends ToOne {
 				getReferencedEntityName(),
 				referenceToPrimaryKey, 
 				getReferencedPropertyName(),
+				getPropertyName(),
 				isLazy(),
 				isUnwrapProxy(),
 				isIgnoreNotFound(),

hibernate-core/src/main/java/org/hibernate/mapping/ToOne.java

@@ -24,6 +24,7 @@ public abstract class ToOne extends SimpleValue implements Fetchable {
 	private FetchMode fetchMode;
 	protected String referencedPropertyName;
 	private String referencedEntityName;
+	private String propertyName;
 	private boolean embedded;
 	private boolean lazy = true;
 	protected boolean unwrapProxy;
@@ -61,6 +62,15 @@ public abstract class ToOne extends SimpleValue implements Fetchable {
 				null : referencedEntityName.intern();
 	}
 
+	public String getPropertyName() {
+		return propertyName;
+	}
+
+	public void setPropertyName(String propertyName) {
+		this.propertyName = propertyName==null ?
+				null : propertyName.intern();
+	}
+
 	@Override
 	public void setTypeUsingReflection(String className, String propertyName) throws MappingException {
 		if (referencedEntityName == null) {

hibernate-core/src/main/java/org/hibernate/type/ManyToOneType.java

@@ -16,10 +16,9 @@ import org.hibernate.HibernateException;
 import org.hibernate.MappingException;
 import org.hibernate.engine.internal.ForeignKeys;
 import org.hibernate.engine.jdbc.Size;
-import org.hibernate.engine.spi.EntityKey;
+import org.hibernate.engine.spi.*;
-import org.hibernate.engine.spi.Mapping;
-import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.persister.entity.EntityPersister;
+import org.hibernate.persister.entity.Loadable;
 
 /**
  * A many-to-one association to an entity.
@@ -27,6 +26,7 @@ import org.hibernate.persister.entity.EntityPersister;
  * @author Gavin King
  */
 public class ManyToOneType extends EntityType {
+	private final String propertyName;
 	private final boolean ignoreNotFound;
 	private boolean isLogicalOneToOne;
 
@@ -54,7 +54,7 @@ public class ManyToOneType extends EntityType {
 
 
 	/**
-	 * @deprecated Use {@link #ManyToOneType(TypeFactory.TypeScope, String, boolean, String, boolean, boolean, boolean, boolean ) } instead.
+	 * @deprecated Use {@link #ManyToOneType(TypeFactory.TypeScope, String, boolean, String, String, boolean, boolean, boolean, boolean ) } instead.
 	 */
 	@Deprecated
 	public ManyToOneType(
@@ -69,6 +69,10 @@ public class ManyToOneType extends EntityType {
 		this( scope, referencedEntityName, uniqueKeyPropertyName == null, uniqueKeyPropertyName, lazy, unwrapProxy, ignoreNotFound, isLogicalOneToOne );
 	}
 
+	/**
+	 * @deprecated Use {@link #ManyToOneType(TypeFactory.TypeScope, String, boolean, String, String, boolean, boolean, boolean, boolean ) } instead.
+	 */
+	@Deprecated
 	public ManyToOneType(
 			TypeFactory.TypeScope scope,
 			String referencedEntityName,
@@ -78,13 +82,28 @@ public class ManyToOneType extends EntityType {
 			boolean unwrapProxy,
 			boolean ignoreNotFound,
 			boolean isLogicalOneToOne) {
+		this( scope, referencedEntityName, referenceToPrimaryKey, uniqueKeyPropertyName, null, lazy, unwrapProxy, ignoreNotFound, isLogicalOneToOne );
+	}
+
+	public ManyToOneType(
+			TypeFactory.TypeScope scope,
+			String referencedEntityName,
+			boolean referenceToPrimaryKey,
+			String uniqueKeyPropertyName,
+			String propertyName,
+			boolean lazy,
+			boolean unwrapProxy,
+			boolean ignoreNotFound,
+			boolean isLogicalOneToOne) {
 		super( scope, referencedEntityName, referenceToPrimaryKey, uniqueKeyPropertyName, !lazy, unwrapProxy );
+		this.propertyName = propertyName;
 		this.ignoreNotFound = ignoreNotFound;
 		this.isLogicalOneToOne = isLogicalOneToOne;
 	}
 
 	public ManyToOneType(ManyToOneType original, String superTypeEntityName) {
 		super( original, superTypeEntityName );
+		this.propertyName = original.propertyName;
 		this.ignoreNotFound = original.ignoreNotFound;
 		this.isLogicalOneToOne = original.isLogicalOneToOne;
 	}
@@ -94,6 +113,11 @@ public class ManyToOneType extends EntityType {
 		return ignoreNotFound;
 	}
 
+	@Override
+	public String getPropertyName() {
+		return propertyName;
+	}
+
 	@Override
 	public boolean isAlwaysDirtyChecked() {
 		// always need to dirty-check, even when non-updateable;
@@ -210,6 +234,27 @@ public class ManyToOneType extends EntityType {
 				.isDirty( old, getIdentifier( current, session ), session );
 	}
 
+	@Override
+	public Object resolve(Object value, SharedSessionContractImplementor session, Object owner, Boolean overridingEager) throws HibernateException {
+		Object resolvedValue = super.resolve(value, session, owner, overridingEager);
+		if ( isLogicalOneToOne && value != null && getPropertyName() != null ) {
+			EntityEntry entry = session.getPersistenceContext().getEntry( owner );
+			if ( entry != null ) {
+				final Loadable ownerPersister = (Loadable) session.getFactory().getMetamodel().entityPersister( entry.getEntityName() );
+				EntityUniqueKey entityKey = new EntityUniqueKey(
+						ownerPersister.getEntityName(),
+						getPropertyName(),
+						value,
+						this,
+						ownerPersister.getEntityMode(),
+						session.getFactory()
+				);
+				session.getPersistenceContext().addEntity( entityKey, owner );
+			}
+		}
+		return resolvedValue;
+	}
+
 	@Override
 	public Serializable disassemble(
 			Object value,

hibernate-core/src/main/java/org/hibernate/type/TypeFactory.java

@@ -286,6 +286,10 @@ public final class TypeFactory implements Serializable {
 		);
 	}
 
+	/**
+	 * @deprecated Use {@link #manyToOne(String, boolean, String, String, boolean, boolean, boolean, boolean)} instead.
+	 */
+	@Deprecated
 	public EntityType manyToOne(
 			String persistentClass,
 			boolean referenceToPrimaryKey,
@@ -294,11 +298,33 @@ public final class TypeFactory implements Serializable {
 			boolean unwrapProxy,
 			boolean ignoreNotFound,
 			boolean isLogicalOneToOne) {
+		return manyToOne(
+				persistentClass,
+				referenceToPrimaryKey,
+				uniqueKeyPropertyName,
+				null,
+				lazy,
+				unwrapProxy,
+				ignoreNotFound,
+				isLogicalOneToOne
+		);
+	}
+
+	public EntityType manyToOne(
+			String persistentClass,
+			boolean referenceToPrimaryKey,
+			String uniqueKeyPropertyName,
+			String propertyName,
+			boolean lazy,
+			boolean unwrapProxy,
+			boolean ignoreNotFound,
+			boolean isLogicalOneToOne) {
 		return new ManyToOneType(
 				typeScope,
 				persistentClass,
 				referenceToPrimaryKey,
 				uniqueKeyPropertyName,
+				propertyName,
 				lazy,
 				unwrapProxy,
 				ignoreNotFound,

hibernate-core/src/test/java/org/hibernate/test/onetoone/bidirectional/BiDirectionalOneToOneFetchTest.java

@@ -0,0 +1,193 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2014, Red Hat Inc. or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Inc.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
+package org.hibernate.test.onetoone.bidirectional;
+
+import org.hibernate.engine.internal.StatisticalLoggingSessionEventListener;
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.junit.After;
+import org.junit.Test;
+
+import javax.persistence.*;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Test cases for fetch joining a bi-directional one-to-one mapping.
+ *
+ * @author Christian Beikov
+ */
+public class BiDirectionalOneToOneFetchTest extends BaseCoreFunctionalTestCase {
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class[] {
+				EntityA.class,
+				EntityB.class
+		};
+	}
+
+	@After
+	public void delete() {
+		inTransaction( s -> {
+			s.createQuery( "delete from EntityA" ).executeUpdate();
+			s.createQuery( "delete from EntityB" ).executeUpdate();
+		} );
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-3930" )
+	public void testEagerFetchBidirectionalOneToOneWithDirectFetching() {
+		inTransaction( session -> {
+			EntityA a = new EntityA( 1L, new EntityB( 2L ) );
+			
+			session.persist( a );
+			session.flush();
+			session.clear();
+
+			// Use atomic integer because we need something mutable
+			final AtomicInteger queryExecutionCount = new AtomicInteger();
+			
+			session.getEventListenerManager().addListener( new StatisticalLoggingSessionEventListener() {
+				@Override
+				public void jdbcExecuteStatementStart() {
+					super.jdbcExecuteStatementStart();
+					queryExecutionCount.getAndIncrement();
+				}
+			} );
+			
+			session.find( EntityA.class, 1L );
+			
+			assertEquals(
+					"Join fetching inverse one-to-one didn't use the object already present in the result set!",
+					1,
+					queryExecutionCount.get()
+			);
+		} );
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-3930" )
+	public void testFetchBidirectionalOneToOneWithOneJoinFetch() {
+		inTransaction( session -> {
+			EntityA a = new EntityA( 1L, new EntityB( 2L ) );
+
+			session.persist( a );
+			session.flush();
+			session.clear();
+
+			// Use atomic integer because we need something mutable
+			final AtomicInteger queryExecutionCount = new AtomicInteger();
+
+			session.getEventListenerManager().addListener( new StatisticalLoggingSessionEventListener() {
+				@Override
+				public void jdbcExecuteStatementStart() {
+					super.jdbcExecuteStatementStart();
+					queryExecutionCount.getAndIncrement();
+				}
+			} );
+
+			session.createQuery(
+					"from EntityA a join fetch a.b"
+			).list();
+
+			assertEquals(
+					"Join fetching inverse one-to-one didn't use the object already present in the result set!",
+					1,
+					queryExecutionCount.get()
+			);
+		} );
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-3930" )
+	public void testFetchBidirectionalOneToOneWithCircularJoinFetch() {
+		inTransaction( session -> {
+			EntityA a = new EntityA( 1L, new EntityB( 2L ) );
+
+			session.persist( a );
+			session.flush();
+			session.clear();
+
+			// Use atomic integer because we need something mutable
+			final AtomicInteger queryExecutionCount = new AtomicInteger();
+			session.getEventListenerManager().addListener( new StatisticalLoggingSessionEventListener() {
+				@Override
+				public void jdbcExecuteStatementStart() {
+					super.jdbcExecuteStatementStart();
+					queryExecutionCount.getAndIncrement();
+				}
+			} );
+
+			session.createQuery(
+					"from EntityA a join fetch a.b b join fetch b.a"
+			).list();
+
+			assertEquals(
+					"Join fetching inverse one-to-one didn't use the object already present in the result set!",
+					1,
+					queryExecutionCount.get()
+			);
+		} );
+	}
+
+	@Entity(name = "EntityA")
+	public static class EntityA {
+		
+		@Id
+		private Long id;
+		
+		@OneToOne(fetch = FetchType.EAGER, cascade = CascadeType.ALL)
+		@JoinColumn(name = "b_id")
+		private EntityB b;
+
+		public EntityA() {
+		}
+
+		public EntityA(Long id, EntityB b) {
+			this.id = id;
+			this.b = b;
+			this.b.a = this;
+		}
+	}
+
+	@Entity(name = "EntityB")
+	public static class EntityB {
+		
+		@Id
+		private Long id;
+		
+		@OneToOne(mappedBy = "b", fetch = FetchType.EAGER)
+		private EntityA a;
+
+		public EntityB() {
+		}
+
+		public EntityB(Long id) {
+			this.id = id;
+		}
+	}
+
+}