diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java index e7e3f36f5..d91a82cf6 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java @@ -595,7 +595,7 @@ public abstract class QueryUtils { propertyPathModel = from.get(segment).getModel(); } - if (requiresJoin(propertyPathModel, model instanceof PluralAttribute, !property.hasNext(), isForSelection) + if (requiresOuterJoin(propertyPathModel, model instanceof PluralAttribute, !property.hasNext(), isForSelection) && !isAlreadyFetched(from, segment)) { Join join = getOrCreateJoin(from, segment); return (Expression) (property.hasNext() ? toExpressionRecursively(join, property.next(), isForSelection) @@ -616,7 +616,7 @@ public abstract class QueryUtils { * @param isForSelection is the property navigated for the selection part of the query? * @return wether an outer join is to be used for integrating this attribute in a query. */ - private static boolean requiresJoin(@Nullable Bindable propertyPathModel, boolean isPluralAttribute, + private static boolean requiresOuterJoin(@Nullable Bindable propertyPathModel, boolean isPluralAttribute, boolean isLeafProperty, boolean isForSelection) { if (propertyPathModel == null && isPluralAttribute) { @@ -633,27 +633,37 @@ public abstract class QueryUtils { return false; } + // if this path is an optional one to one attribute navigated from the not owning side we also need an explicit + // outer join to avoid https://hibernate.atlassian.net/browse/HHH-12712 and + // https://github.com/eclipse-ee4j/jpa-api/issues/170 + boolean isInverseOptionalOneToOne = PersistentAttributeType.ONE_TO_ONE == attribute.getPersistentAttributeType() + && !getAnnotationProperty(attribute, "mappedBy", "").isEmpty(); + // if this path is part of the select list we need to generate an explicit outer join in order to prevent Hibernate // to use an inner join instead. // see https://hibernate.atlassian.net/browse/HHH-12999. - if (isLeafProperty && !isForSelection && !attribute.isCollection()) { + if (isLeafProperty && !isForSelection && !attribute.isCollection() && !isInverseOptionalOneToOne) { return false; } + return getAnnotationProperty(attribute, "optional", true); + } + + private static T getAnnotationProperty(Attribute attribute, String propertyName, T defaultValue) { Class associationAnnotation = ASSOCIATION_TYPES.get(attribute.getPersistentAttributeType()); if (associationAnnotation == null) { - return true; + return defaultValue; } Member member = attribute.getJavaMember(); if (!(member instanceof AnnotatedElement)) { - return true; + return defaultValue; } Annotation annotation = AnnotationUtils.getAnnotation((AnnotatedElement) member, associationAnnotation); - return annotation == null ? true : (boolean) AnnotationUtils.getValue(annotation, "optional"); + return annotation == null ? defaultValue : (T) AnnotationUtils.getValue(annotation, propertyName); } static Expression toExpressionRecursively(Path path, PropertyPath property) { diff --git a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java index 503d85dc3..7858d6c98 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java @@ -22,6 +22,7 @@ import static org.mockito.Mockito.*; import java.util.Collections; import java.util.List; import java.util.Set; +import java.util.function.Consumer; import java.util.stream.Collectors; import javax.persistence.Entity; @@ -29,6 +30,7 @@ import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import javax.persistence.Id; import javax.persistence.OneToMany; +import javax.persistence.OneToOne; import javax.persistence.Persistence; import javax.persistence.PersistenceContext; import javax.persistence.criteria.CriteriaBuilder; @@ -93,6 +95,36 @@ public class QueryUtilsIntegrationTests { assertThat(getNonInnerJoins(root)).hasSize(1); } + @Test // DATAJPA-1238 + public void createsJoinForOptionalOneToOneInReverseDirection() { + + doInMerchantContext(emf -> { + + CriteriaBuilder builder = emf.getCriteriaBuilder(); + CriteriaQuery
query = builder.createQuery(Address.class); + Root
root = query.from(Address.class); + + QueryUtils.toExpressionRecursively(root, PropertyPath.from("merchant", Address.class)); + + assertThat(getNonInnerJoins(root)).hasSize(1); + }); + } + + @Test // DATAJPA-1238 + public void createsNoJoinForOptionalOneToOneInNormalDirection() { + + doInMerchantContext(emf -> { + + CriteriaBuilder builder = emf.getCriteriaBuilder(); + CriteriaQuery query = builder.createQuery(Merchant.class); + Root root = query.from(Merchant.class); + + QueryUtils.toExpressionRecursively(root, PropertyPath.from("address", Merchant.class)); + + assertThat(getNonInnerJoins(root)).isEmpty(); + }); + } + @Test // DATAJPA-401, DATAJPA-1238 public void doesNotCreateJoinForOptionalAssociationWithoutFurtherNavigation() { @@ -130,21 +162,31 @@ public class QueryUtilsIntegrationTests { @Test // DATAJPA-476 public void traversesPluralAttributeCorrectly() { - PersistenceProviderResolver originalPersistenceProviderResolver = PersistenceProviderResolverHolder - .getPersistenceProviderResolver(); + doInMerchantContext(((emf) -> { - try { - - PersistenceProviderResolverHolder.setPersistenceProviderResolver(new HibernateOnlyPersistenceProviderResolver()); - EntityManagerFactory entityManagerFactory = Persistence.createEntityManagerFactory("merchant"); - CriteriaBuilder builder = entityManagerFactory.createEntityManager().getCriteriaBuilder(); + CriteriaBuilder builder = emf.createEntityManager().getCriteriaBuilder(); CriteriaQuery query = builder.createQuery(Merchant.class); Root root = query.from(Merchant.class); QueryUtils.toExpressionRecursively(root, PropertyPath.from("employeesCredentialsUid", Merchant.class)); + })); + } + public void doInMerchantContext(Consumer emfConsumer) { + PersistenceProviderResolver originalPersistenceProviderResolver = PersistenceProviderResolverHolder + .getPersistenceProviderResolver(); + + EntityManagerFactory entityManagerFactory = null; + try { + + PersistenceProviderResolverHolder.setPersistenceProviderResolver(new HibernateOnlyPersistenceProviderResolver()); + entityManagerFactory = Persistence.createEntityManagerFactory("merchant"); + emfConsumer.accept(entityManagerFactory); } finally { PersistenceProviderResolverHolder.setPersistenceProviderResolver(originalPersistenceProviderResolver); + if (entityManagerFactory != null) { + entityManagerFactory.close(); + } } } @@ -203,20 +245,29 @@ public class QueryUtilsIntegrationTests { return 0; } + private Set> getNonInnerJoins(Root root) { + + return root.getJoins() // + .stream() // + .filter(j -> j.getJoinType() != JoinType.INNER) // + .collect(Collectors.toSet()); + } + @Entity @SuppressWarnings("unused") static class Merchant { @Id String id; @OneToMany Set employees; + + @OneToOne Address address; } - private Set> getNonInnerJoins(Root root) { - - return root.getJoins() // - .stream() // - .filter(j -> j.getJoinType() != JoinType.INNER) // - .collect(Collectors.toSet()); + @Entity + @SuppressWarnings("unused") + static class Address { + @Id String id; + @OneToOne(mappedBy = "address") Merchant merchant; } @Entity @@ -251,4 +302,5 @@ public class QueryUtilsIntegrationTests { @Override public void clearCachedProviders() {} } + } diff --git a/src/test/resources/META-INF/persistence.xml b/src/test/resources/META-INF/persistence.xml index ced3e4421..2360ed5d1 100644 --- a/src/test/resources/META-INF/persistence.xml +++ b/src/test/resources/META-INF/persistence.xml @@ -78,6 +78,7 @@ org.springframework.data.jpa.domain.sample.User org.springframework.data.jpa.repository.query.QueryUtilsIntegrationTests$Merchant + org.springframework.data.jpa.repository.query.QueryUtilsIntegrationTests$Address org.springframework.data.jpa.repository.query.QueryUtilsIntegrationTests$Employee org.springframework.data.jpa.repository.query.QueryUtilsIntegrationTests$Credential true