Skip to content

Commit 24f6006

Browse files
committed
Polishing.
Formatting. Preferring iterative code of streams. Adding `@author` tags for previous changes. Original pull request: #436
1 parent f6b2077 commit 24f6006

File tree

3 files changed

+58
-54
lines changed

3 files changed

+58
-54
lines changed

src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java

+52-45
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,7 @@
2121
import java.lang.annotation.Annotation;
2222
import java.lang.reflect.AnnotatedElement;
2323
import java.lang.reflect.Member;
24-
import java.util.ArrayList;
25-
import java.util.Collections;
26-
import java.util.HashMap;
27-
import java.util.HashSet;
28-
import java.util.Iterator;
29-
import java.util.List;
30-
import java.util.Locale;
31-
import java.util.Map;
32-
import java.util.Objects;
33-
import java.util.Set;
24+
import java.util.*;
3425
import java.util.regex.Matcher;
3526
import java.util.regex.Pattern;
3627
import java.util.stream.Collectors;
@@ -42,15 +33,16 @@
4233
import javax.persistence.Query;
4334
import javax.persistence.criteria.CriteriaBuilder;
4435
import javax.persistence.criteria.Expression;
36+
import javax.persistence.criteria.Fetch;
4537
import javax.persistence.criteria.From;
4638
import javax.persistence.criteria.Join;
4739
import javax.persistence.criteria.JoinType;
4840
import javax.persistence.metamodel.Attribute;
49-
import javax.persistence.metamodel.Attribute.PersistentAttributeType;
5041
import javax.persistence.metamodel.Bindable;
5142
import javax.persistence.metamodel.ManagedType;
5243
import javax.persistence.metamodel.PluralAttribute;
5344
import javax.persistence.metamodel.SingularAttribute;
45+
import javax.persistence.metamodel.Attribute.PersistentAttributeType;
5446

5547
import org.springframework.core.annotation.AnnotationUtils;
5648
import org.springframework.dao.InvalidDataAccessApiUsageException;
@@ -105,7 +97,8 @@ public abstract class QueryUtils {
10597

10698
private static final Pattern ALIAS_MATCH;
10799
private static final Pattern COUNT_MATCH;
108-
private static final Pattern PROJECTION_CLAUSE = Pattern.compile("select\\s+(?:distinct\\s+)?(.+)\\s+from", Pattern.CASE_INSENSITIVE);
100+
private static final Pattern PROJECTION_CLAUSE = Pattern.compile("select\\s+(?:distinct\\s+)?(.+)\\s+from",
101+
Pattern.CASE_INSENSITIVE);
109102

110103
private static final Pattern NO_DIGITS = Pattern.compile("\\D+");
111104

@@ -115,8 +108,8 @@ public abstract class QueryUtils {
115108
private static final String EQUALS_CONDITION_STRING = "%s.%s = :%s";
116109
private static final Pattern ORDER_BY = Pattern.compile(".*order\\s+by\\s+.*", CASE_INSENSITIVE);
117110

118-
private static final Pattern NAMED_PARAMETER = Pattern
119-
.compile(COLON_NO_DOUBLE_COLON + IDENTIFIER + "|#" + IDENTIFIER, CASE_INSENSITIVE);
111+
private static final Pattern NAMED_PARAMETER = Pattern.compile(COLON_NO_DOUBLE_COLON + IDENTIFIER + "|#" + IDENTIFIER,
112+
CASE_INSENSITIVE);
120113

121114
private static final Pattern CONSTRUCTOR_EXPRESSION;
122115

@@ -491,9 +484,9 @@ public static String createCountQueryFor(String originalQuery, @Nullable String
491484
&& !variable.startsWith("count(") //
492485
&& !variable.contains(","); //
493486

494-
String complexCountValue = matcher.matches() &&
495-
StringUtils.hasText(matcher.group(COMPLEX_COUNT_FIRST_INDEX)) ?
496-
COMPLEX_COUNT_VALUE : COMPLEX_COUNT_LAST_VALUE;
487+
String complexCountValue = matcher.matches() && StringUtils.hasText(matcher.group(COMPLEX_COUNT_FIRST_INDEX))
488+
? COMPLEX_COUNT_VALUE
489+
: COMPLEX_COUNT_LAST_VALUE;
497490

498491
String replacement = useVariable ? SIMPLE_COUNT_VALUE : complexCountValue;
499492
countQuery = matcher.replaceFirst(String.format(COUNT_REPLACEMENT_TEMPLATE, replacement));
@@ -627,15 +620,16 @@ static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath p
627620
/**
628621
* Creates an expression with proper inner and left joins by recursively navigating the path
629622
*
630-
* @param from the {@link From}
631-
* @param property the property path
632-
* @param isForSelection is the property navigated for the selection or ordering part of the query?
623+
* @param from the {@link From}
624+
* @param property the property path
625+
* @param isForSelection is the property navigated for the selection or ordering part of the query?
633626
* @param hasRequiredOuterJoin has a parent already required an outer join?
634-
* @param <T> the type of the expression
627+
* @param <T> the type of the expression
635628
* @return the expression
636629
*/
637-
@SuppressWarnings("unchecked") static <T> Expression<T> toExpressionRecursively(From<?, ?> from,
638-
PropertyPath property, boolean isForSelection, boolean hasRequiredOuterJoin) {
630+
@SuppressWarnings("unchecked")
631+
static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath property, boolean isForSelection,
632+
boolean hasRequiredOuterJoin) {
639633

640634
String segment = property.getSegment();
641635

@@ -664,16 +658,15 @@ static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath p
664658
}
665659

666660
/**
667-
* Checks if this attribute requires an outer join.
668-
* This is the case eg. if it hadn't already been fetched with an inner join and if it's an a optional association,
669-
* and if previous paths has already required outer joins.
670-
* It also ensures outer joins are used even when Hibernate defaults to inner joins (HHH-12712 and HHH-12999).
661+
* Checks if this attribute requires an outer join. This is the case eg. if it hadn't already been fetched with an
662+
* inner join and if it's an a optional association, and if previous paths has already required outer joins. It also
663+
* ensures outer joins are used even when Hibernate defaults to inner joins (HHH-12712 and HHH-12999).
671664
*
672-
* @param from the {@link From} to check for fetches.
673-
* @param property the property path
674-
* @param isForSelection is the property navigated for the selection or ordering part of the query? if true,
675-
* we need to generate an explicit outer join in order to prevent Hibernate to use an
676-
* inner join instead. see https://hibernate.atlassian.net/browse/HHH-12999
665+
* @param from the {@link From} to check for fetches.
666+
* @param property the property path
667+
* @param isForSelection is the property navigated for the selection or ordering part of the query? if true, we need
668+
* to generate an explicit outer join in order to prevent Hibernate to use an inner join instead. see
669+
* https://hibernate.atlassian.net/browse/HHH-12999
677670
* @param hasRequiredOuterJoin has a parent already required an outer join?
678671
* @return whether an outer join is to be used for integrating this attribute in a query.
679672
*/
@@ -697,7 +690,7 @@ private static boolean requiresOuterJoin(From<?, ?> from, PropertyPath property,
697690
if (model instanceof ManagedType) {
698691
managedType = (ManagedType<?>) model;
699692
} else if (model instanceof SingularAttribute
700-
&& ((SingularAttribute<?, ?>) model).getType() instanceof ManagedType) {
693+
&& ((SingularAttribute<?, ?>) model).getType() instanceof ManagedType) {
701694
managedType = (ManagedType<?>) ((SingularAttribute<?, ?>) model).getType();
702695
}
703696
if (managedType != null) {
@@ -761,34 +754,48 @@ private static <T> T getAnnotationProperty(Attribute<?, ?> attribute, String pro
761754
/**
762755
* Returns an existing join for the given attribute if one already exists or creates a new one if not.
763756
*
764-
* @param from the {@link From} to get the current joins from.
757+
* @param from the {@link From} to get the current joins from.
765758
* @param attribute the {@link Attribute} to look for in the current joins.
766-
* @param joinType the join type to create if none was found
759+
* @param joinType the join type to create if none was found
767760
* @return will never be {@literal null}.
768761
*/
769762
private static Join<?, ?> getOrCreateJoin(From<?, ?> from, String attribute, JoinType joinType) {
770-
return from.getJoins().stream()
771-
.filter(join -> join.getAttribute().getName().equals(attribute))
772-
.findFirst()
773-
.orElseGet(() -> from.join(attribute, joinType));
763+
764+
for (Join<?, ?> join : from.getJoins()) {
765+
766+
if (join.getAttribute().getName().equals(attribute)) {
767+
return join;
768+
}
769+
}
770+
return from.join(attribute, joinType);
774771
}
775772

776773
/**
777774
* Return whether the given {@link From} contains an inner join for the attribute with the given name.
778775
*
779-
* @param from the {@link From} to check for joins.
776+
* @param from the {@link From} to check for joins.
780777
* @param attribute the attribute name to check.
781778
* @return true if the attribute has already been inner joined
782779
*/
783780
private static boolean isAlreadyInnerJoined(From<?, ?> from, String attribute) {
784781

785-
boolean isInnerJoinFetched = from.getFetches().stream().anyMatch(
786-
fetch -> fetch.getAttribute().getName().equals(attribute) && fetch.getJoinType().equals(JoinType.INNER));
782+
for (Fetch<?, ?> fetch : from.getFetches()) {
787783

788-
boolean isSimplyInnerJoined = from.getJoins().stream()
789-
.anyMatch(join -> join.getAttribute().getName().equals(attribute) && join.getJoinType().equals(JoinType.INNER));
784+
if (fetch.getAttribute().getName().equals(attribute) //
785+
&& fetch.getJoinType().equals(JoinType.INNER)) {
786+
return true;
787+
}
788+
}
789+
790+
for (Join<?, ?> join : from.getJoins()) {
791+
792+
if (join.getAttribute().getName().equals(attribute) //
793+
&& join.getJoinType().equals(JoinType.INNER)) {
794+
return true;
795+
}
796+
}
790797

791-
return isInnerJoinFetched || isSimplyInnerJoined;
798+
return false;
792799
}
793800

794801
/**

src/test/java/org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java

+1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
/**
2121
* @author Oliver Gierke
22+
* @author Jens Schauder
2223
*/
2324
@ContextConfiguration("classpath:eclipselink.xml")
2425
class EclipseLinkQueryUtilsIntegrationTests extends QueryUtilsIntegrationTests {

src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java

+5-9
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
*
6565
* @author Oliver Gierke
6666
* @author Sébastien Péralta
67+
* @author Jens Schauder
6768
* @author Patrice Blanchardie
6869
*/
6970
@ExtendWith(SpringExtension.class)
@@ -115,7 +116,7 @@ void createsJoinForOptionalOneToOneInReverseDirection() {
115116
});
116117
}
117118

118-
@Test // DATAJPA-1822
119+
@Test // gh-2111
119120
void createsLeftJoinForOptionalToOneWithNestedNonOptional() {
120121

121122
CriteriaBuilder builder = em.getCriteriaBuilder();
@@ -131,7 +132,7 @@ void createsLeftJoinForOptionalToOneWithNestedNonOptional() {
131132
assertThat(getInnerJoins(leftJoin)).isEmpty(); // no inner join customer
132133
}
133134

134-
@Test // DATAJPA-1822
135+
@Test // gh-2111
135136
void createsLeftJoinForNonOptionalToOneWithNestedOptional() {
136137

137138
CriteriaBuilder builder = em.getCriteriaBuilder();
@@ -150,7 +151,7 @@ void createsLeftJoinForNonOptionalToOneWithNestedOptional() {
150151
assertThat(getInnerJoins(leftJoin)).isEmpty(); // no inner join customer
151152
}
152153

153-
@Test // DATAJPA-1822
154+
@Test // gh-2111
154155
void reusesLeftJoinForNonOptionalToOneWithNestedOptional() {
155156

156157
CriteriaBuilder builder = em.getCriteriaBuilder();
@@ -174,7 +175,7 @@ void reusesLeftJoinForNonOptionalToOneWithNestedOptional() {
174175
assertThat(getNonInnerJoins(leftJoin)).hasSize(1); // left join customer
175176
}
176177

177-
@Test // DATAJPA-1822
178+
@Test // gh-2111
178179
void reusesInnerJoinForNonOptionalToOneWithNestedOptional() {
179180

180181
CriteriaBuilder builder = em.getCriteriaBuilder();
@@ -332,11 +333,6 @@ int getNumberOfJoinsAfterCreatingAPath() {
332333
return 0;
333334
}
334335

335-
private Set<Join<?, ?>> getNonInnerJoins(Root<?> root) {
336-
337-
return getNonInnerJoins((From<?, ?>) root);
338-
}
339-
340336
private Set<Join<?, ?>> getNonInnerJoins(From<?, ?> root) {
341337

342338
return root.getJoins().stream().filter(j -> j.getJoinType() != JoinType.INNER).collect(Collectors.toSet());

0 commit comments

Comments
 (0)