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 a6fbf2e7e..761164b3f 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 @@ -18,17 +18,24 @@ package org.springframework.data.jpa.repository.query; import static java.util.regex.Pattern.*; import static javax.persistence.metamodel.Attribute.PersistentAttributeType.*; +import java.lang.annotation.Annotation; +import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Member; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.persistence.EntityManager; +import javax.persistence.ManyToOne; +import javax.persistence.OneToOne; import javax.persistence.Parameter; import javax.persistence.Query; import javax.persistence.TypedQuery; @@ -42,9 +49,9 @@ import javax.persistence.criteria.Root; import javax.persistence.metamodel.Attribute; import javax.persistence.metamodel.Attribute.PersistentAttributeType; import javax.persistence.metamodel.Bindable; -import javax.persistence.metamodel.Bindable.BindableType; import javax.persistence.metamodel.ManagedType; +import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Order; import org.springframework.data.mapping.PropertyPath; @@ -81,7 +88,7 @@ public abstract class QueryUtils { private static final String EQUALS_CONDITION_STRING = "%s.%s = :%s"; private static final Pattern ORDER_BY = Pattern.compile(".*order\\s+by\\s+.*", CASE_INSENSITIVE); - private static final Set ASSOCIATION_TYPES; + private static final Map> ASSOCIATION_TYPES; static { @@ -104,13 +111,13 @@ public abstract class QueryUtils { COUNT_MATCH = compile(builder.toString(), CASE_INSENSITIVE); - Set persistentAttributeTypes = new HashSet(); - persistentAttributeTypes.add(ONE_TO_ONE); - persistentAttributeTypes.add(ONE_TO_MANY); - persistentAttributeTypes.add(MANY_TO_ONE); - persistentAttributeTypes.add(MANY_TO_MANY); + Map> persistentAttributeTypes = new HashMap>(); + persistentAttributeTypes.put(ONE_TO_ONE, OneToOne.class); + persistentAttributeTypes.put(ONE_TO_MANY, null); + persistentAttributeTypes.put(MANY_TO_ONE, ManyToOne.class); + persistentAttributeTypes.put(MANY_TO_MANY, null); - ASSOCIATION_TYPES = Collections.unmodifiableSet(persistentAttributeTypes); + ASSOCIATION_TYPES = Collections.unmodifiableMap(persistentAttributeTypes); } /** @@ -442,7 +449,7 @@ public abstract class QueryUtils { propertyPathModel = from.get(property.getSegment()).getModel(); } - if (property.isCollection() || isEntityPath(propertyPathModel)) { + if (property.isCollection() || requiresJoin(propertyPathModel)) { Join join = getOrCreateJoin(from, property.getSegment()); return (Expression) (property.hasNext() ? toExpressionRecursively(join, property.next()) : join); } else { @@ -452,29 +459,38 @@ public abstract class QueryUtils { } /** - * Returns whether the given {@code propertyPathModel} can be considered referring an entity. + * Returns whether the given {@code propertyPathModel} requires the creation of a join. This is the case if we find a + * non-optional association. * * @param propertyPathModel must not be {@literal null}. * @return */ - private static boolean isEntityPath(Bindable propertyPathModel) { + private static boolean requiresJoin(Bindable propertyPathModel) { - if (BindableType.ENTITY_TYPE.equals(propertyPathModel.getBindableType())) { + if (!(propertyPathModel instanceof Attribute)) { + return false; + } + + Attribute attribute = (Attribute) propertyPathModel; + + if (!ASSOCIATION_TYPES.containsKey(attribute.getPersistentAttributeType())) { + return false; + } + + Class associationAnnotation = ASSOCIATION_TYPES.get(attribute.getPersistentAttributeType()); + + if (associationAnnotation == null) { + return false; + } + + Member member = attribute.getJavaMember(); + + if (!(member instanceof AnnotatedElement)) { return true; } - if (propertyPathModel instanceof Attribute) { - - Attribute attribute = (Attribute) propertyPathModel; - - if (attribute.isAssociation()) { - return true; - } - - return ASSOCIATION_TYPES.contains(attribute.getPersistentAttributeType()); - } - - return false; + Annotation annotation = AnnotationUtils.getAnnotation((AnnotatedElement) member, associationAnnotation); + return annotation == null ? true : (Boolean) AnnotationUtils.getValue(annotation, "optional"); } static Expression toExpressionRecursively(Path path, PropertyPath property) { diff --git a/src/test/java/org/springframework/data/jpa/domain/sample/Customer.java b/src/test/java/org/springframework/data/jpa/domain/sample/Customer.java new file mode 100644 index 000000000..a6819e3af --- /dev/null +++ b/src/test/java/org/springframework/data/jpa/domain/sample/Customer.java @@ -0,0 +1,28 @@ +/* + * Copyright 2013 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.jpa.domain.sample; + +import javax.persistence.Entity; +import javax.persistence.Id; + +/** + * @author Oliver Gierke + */ +@Entity +public class Customer { + + @Id Long id; +} diff --git a/src/test/java/org/springframework/data/jpa/domain/sample/Order.java b/src/test/java/org/springframework/data/jpa/domain/sample/Order.java new file mode 100644 index 000000000..3016651f7 --- /dev/null +++ b/src/test/java/org/springframework/data/jpa/domain/sample/Order.java @@ -0,0 +1,32 @@ +/* + * Copyright 2013 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.jpa.domain.sample; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.ManyToOne; +import javax.persistence.Table; + +/** + * @author Oliver Gierke + */ +@Entity +@Table(name = "ORDERS") +public class Order { + + @Id Long id; + @ManyToOne(optional = false) Customer customer; +} diff --git a/src/test/java/org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java new file mode 100644 index 000000000..5c3127805 --- /dev/null +++ b/src/test/java/org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java @@ -0,0 +1,49 @@ +/* + * Copyright 2013 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.jpa.repository.query; + +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +import java.util.Set; + +import javax.persistence.criteria.Join; +import javax.persistence.criteria.JoinType; +import javax.persistence.criteria.Root; + +import org.springframework.data.jpa.domain.sample.Order; +import org.springframework.test.context.ContextConfiguration; + +/** + * @author Oliver Gierke + */ +@ContextConfiguration("classpath:eclipselink.xml") +public class EclipseLinkQueryUtilsIntegrationTests extends QueryUtilsIntegrationTests { + + /** + * Required as EclipseLink generates an inner join for plain association traversal. + */ + @Override + protected void assertNoJoinRequestedForOptionalAssociation(Root root) { + + Set> joins = root.getJoins(); + assertThat(joins, hasSize(1)); + + Join join = joins.iterator().next(); + assertThat(join.getAttribute().getName(), is("manager")); + assertThat(join.getJoinType(), is(JoinType.INNER)); + } +} diff --git a/src/test/java/org/springframework/data/jpa/repository/query/OpenJpaQueryUtilsIntegrationTests.java b/src/test/java/org/springframework/data/jpa/repository/query/OpenJpaQueryUtilsIntegrationTests.java new file mode 100644 index 000000000..aa89c66b1 --- /dev/null +++ b/src/test/java/org/springframework/data/jpa/repository/query/OpenJpaQueryUtilsIntegrationTests.java @@ -0,0 +1,26 @@ +/* + * Copyright 2013 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.jpa.repository.query; + +import org.springframework.test.context.ContextConfiguration; + +/** + * @author Oliver Gierke + */ +@ContextConfiguration("classpath:openjpa.xml") +public class OpenJpaQueryUtilsIntegrationTests extends QueryUtilsIntegrationTests { + +} 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 070efe7ed..682b4c166 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 @@ -26,6 +26,7 @@ import javax.persistence.criteria.Root; import org.junit.Test; import org.junit.runner.RunWith; +import org.springframework.data.jpa.domain.sample.Order; import org.springframework.data.jpa.domain.sample.User; import org.springframework.data.mapping.PropertyPath; import org.springframework.test.context.ContextConfiguration; @@ -60,4 +61,36 @@ public class QueryUtilsIntegrationTests { assertThat(from.getJoins(), hasSize(1)); } + + /** + * @see DATAJPA-401 + */ + @Test + public void createsJoinForOptionalAssociation() { + + CriteriaBuilder builder = em.getCriteriaBuilder(); + CriteriaQuery query = builder.createQuery(User.class); + Root root = query.from(User.class); + + QueryUtils.toExpressionRecursively(root, PropertyPath.from("manager", User.class)); + + assertThat(root.getJoins(), hasSize(1)); + } + + /** + * @see DATAJPA-401 + */ + @Test + public void doesNotCreateAJoinForNonOptionalAssociation() { + + CriteriaBuilder builder = em.getCriteriaBuilder(); + CriteriaQuery query = builder.createQuery(Order.class); + Root root = query.from(Order.class); + + QueryUtils.toExpressionRecursively(root, PropertyPath.from("customer", Order.class)); + } + + protected void assertNoJoinRequestedForOptionalAssociation(Root root) { + assertThat(root.getJoins(), is(empty())); + } } diff --git a/src/test/resources/META-INF/persistence.xml b/src/test/resources/META-INF/persistence.xml index b91202a06..2c107f047 100644 --- a/src/test/resources/META-INF/persistence.xml +++ b/src/test/resources/META-INF/persistence.xml @@ -24,6 +24,8 @@ org.springframework.data.jpa.domain.sample.EmbeddedIdExampleEmployeePK org.springframework.data.jpa.domain.sample.EmbeddedIdExampleEmployee org.springframework.data.jpa.domain.sample.EmbeddedIdExampleDepartment + org.springframework.data.jpa.domain.sample.Customer + org.springframework.data.jpa.domain.sample.Order true