Skip to content

Commit

Permalink
Polishing.
Browse files Browse the repository at this point in the history 
Formatting.
Preferring iterative code of streams.
Adding `@author` tags for previous changes.

Original pull request: spring-projects#436
  • Loading branch information
@schauder
schauder committed Mar 2, 2021
1 parent 32b6163 commit 7c4cba8
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 54 deletions.
97 changes: 52 additions & 45 deletions src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,7 @@
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.Objects;
import java.util.Set;
import java.util.*;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
Expand All @@ -42,15 +33,16 @@
import javax.persistence.Query;
import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.Expression;
import javax.persistence.criteria.Fetch;
import javax.persistence.criteria.From;
import javax.persistence.criteria.Join;
import javax.persistence.criteria.JoinType;
import javax.persistence.metamodel.Attribute;
import javax.persistence.metamodel.Attribute.PersistentAttributeType;
import javax.persistence.metamodel.Bindable;
import javax.persistence.metamodel.ManagedType;
import javax.persistence.metamodel.PluralAttribute;
import javax.persistence.metamodel.SingularAttribute;
import javax.persistence.metamodel.Attribute.PersistentAttributeType;

import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.dao.InvalidDataAccessApiUsageException;
Expand Down Expand Up @@ -105,7 +97,8 @@ public abstract class QueryUtils {

private static final Pattern ALIAS_MATCH;
private static final Pattern COUNT_MATCH;
private static final Pattern PROJECTION_CLAUSE = Pattern.compile("select\\s+(?:distinct\\s+)?(.+)\\s+from", Pattern.CASE_INSENSITIVE);
private static final Pattern PROJECTION_CLAUSE = Pattern.compile("select\\s+(?:distinct\\s+)?(.+)\\s+from",
Pattern.CASE_INSENSITIVE);

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

Expand All @@ -115,8 +108,8 @@ 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 Pattern NAMED_PARAMETER = Pattern
.compile(COLON_NO_DOUBLE_COLON + IDENTIFIER + "|#" + IDENTIFIER, CASE_INSENSITIVE);
private static final Pattern NAMED_PARAMETER = Pattern.compile(COLON_NO_DOUBLE_COLON + IDENTIFIER + "|#" + IDENTIFIER,
CASE_INSENSITIVE);

private static final Pattern CONSTRUCTOR_EXPRESSION;

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

String complexCountValue = matcher.matches() &&
StringUtils.hasText(matcher.group(COMPLEX_COUNT_FIRST_INDEX)) ?
COMPLEX_COUNT_VALUE : COMPLEX_COUNT_LAST_VALUE;
String complexCountValue = matcher.matches() && StringUtils.hasText(matcher.group(COMPLEX_COUNT_FIRST_INDEX))
? COMPLEX_COUNT_VALUE
: COMPLEX_COUNT_LAST_VALUE;

String replacement = useVariable ? SIMPLE_COUNT_VALUE : complexCountValue;
countQuery = matcher.replaceFirst(String.format(COUNT_REPLACEMENT_TEMPLATE, replacement));
Expand Down Expand Up @@ -627,15 +620,16 @@ static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath p
/**
* Creates an expression with proper inner and left joins by recursively navigating the path
*
* @param from the {@link From}
* @param property the property path
* @param isForSelection is the property navigated for the selection or ordering part of the query?
* @param from the {@link From}
* @param property the property path
* @param isForSelection is the property navigated for the selection or ordering part of the query?
* @param hasRequiredOuterJoin has a parent already required an outer join?
* @param <T> the type of the expression
* @param <T> the type of the expression
* @return the expression
*/
@SuppressWarnings("unchecked") static <T> Expression<T> toExpressionRecursively(From<?, ?> from,
PropertyPath property, boolean isForSelection, boolean hasRequiredOuterJoin) {
@SuppressWarnings("unchecked")
static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath property, boolean isForSelection,
boolean hasRequiredOuterJoin) {

String segment = property.getSegment();

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

/**
* Checks if this attribute requires an outer join.
* This is the case eg. if it hadn't already been fetched with an inner join and if it's an a optional association,
* and if previous paths has already required outer joins.
* It also ensures outer joins are used even when Hibernate defaults to inner joins (HHH-12712 and HHH-12999).
* Checks if this attribute requires an outer join. This is the case eg. if it hadn't already been fetched with an
* inner join and if it's an a optional association, and if previous paths has already required outer joins. It also
* ensures outer joins are used even when Hibernate defaults to inner joins (HHH-12712 and HHH-12999).
*
* @param from the {@link From} to check for fetches.
* @param property the property path
* @param isForSelection is the property navigated for the selection or ordering part of the query? if true,
* 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
* @param from the {@link From} to check for fetches.
* @param property the property path
* @param isForSelection is the property navigated for the selection or ordering part of the query? if true, 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
* @param hasRequiredOuterJoin has a parent already required an outer join?
* @return whether an outer join is to be used for integrating this attribute in a query.
*/
Expand All @@ -697,7 +690,7 @@ private static boolean requiresOuterJoin(From<?, ?> from, PropertyPath property,
if (model instanceof ManagedType) {
managedType = (ManagedType<?>) model;
} else if (model instanceof SingularAttribute
&& ((SingularAttribute<?, ?>) model).getType() instanceof ManagedType) {
&& ((SingularAttribute<?, ?>) model).getType() instanceof ManagedType) {
managedType = (ManagedType<?>) ((SingularAttribute<?, ?>) model).getType();
}
if (managedType != null) {
Expand Down Expand Up @@ -761,34 +754,48 @@ private static <T> T getAnnotationProperty(Attribute<?, ?> attribute, String pro
/**
* Returns an existing join for the given attribute if one already exists or creates a new one if not.
*
* @param from the {@link From} to get the current joins from.
* @param from the {@link From} to get the current joins from.
* @param attribute the {@link Attribute} to look for in the current joins.
* @param joinType the join type to create if none was found
* @param joinType the join type to create if none was found
* @return will never be {@literal null}.
*/
private static Join<?, ?> getOrCreateJoin(From<?, ?> from, String attribute, JoinType joinType) {
return from.getJoins().stream()
.filter(join -> join.getAttribute().getName().equals(attribute))
.findFirst()
.orElseGet(() -> from.join(attribute, joinType));

for (Join<?, ?> join : from.getJoins()) {

if (join.getAttribute().getName().equals(attribute)) {
return join;
}
}
return from.join(attribute, joinType);
}

/**
* Return whether the given {@link From} contains an inner join for the attribute with the given name.
*
* @param from the {@link From} to check for joins.
* @param from the {@link From} to check for joins.
* @param attribute the attribute name to check.
* @return true if the attribute has already been inner joined
*/
private static boolean isAlreadyInnerJoined(From<?, ?> from, String attribute) {

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

boolean isSimplyInnerJoined = from.getJoins().stream()
.anyMatch(join -> join.getAttribute().getName().equals(attribute) && join.getJoinType().equals(JoinType.INNER));
if (fetch.getAttribute().getName().equals(attribute) //
&& fetch.getJoinType().equals(JoinType.INNER)) {
return true;
}
}

for (Join<?, ?> join : from.getJoins()) {

if (join.getAttribute().getName().equals(attribute) //
&& join.getJoinType().equals(JoinType.INNER)) {
return true;
}
}

return isInnerJoinFetched || isSimplyInnerJoined;
return false;
}

/**
Expand Down
1 change: 1 addition & 0 deletions .../org/springframework/data/jpa/repository/query/EclipseLinkQueryUtilsIntegrationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

/**
* @author Oliver Gierke
* @author Jens Schauder
*/
@ContextConfiguration("classpath:eclipselink.xml")
class EclipseLinkQueryUtilsIntegrationTests extends QueryUtilsIntegrationTests {
Expand Down
14 changes: 5 additions & 9 deletions src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsIntegrationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
*
* @author Oliver Gierke
* @author Sébastien Péralta
* @author Jens Schauder
* @author Patrice Blanchardie
*/
@ExtendWith(SpringExtension.class)
Expand Down Expand Up @@ -115,7 +116,7 @@ void createsJoinForOptionalOneToOneInReverseDirection() {
});
}

@Test // DATAJPA-1822
@Test // gh-2111
void createsLeftJoinForOptionalToOneWithNestedNonOptional() {

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

@Test // DATAJPA-1822
@Test // gh-2111
void createsLeftJoinForNonOptionalToOneWithNestedOptional() {

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

@Test // DATAJPA-1822
@Test // gh-2111
void reusesLeftJoinForNonOptionalToOneWithNestedOptional() {

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

@Test // DATAJPA-1822
@Test // gh-2111
void reusesInnerJoinForNonOptionalToOneWithNestedOptional() {

CriteriaBuilder builder = em.getCriteriaBuilder();
Expand Down Expand Up @@ -332,11 +333,6 @@ int getNumberOfJoinsAfterCreatingAPath() {
return 0;
}

private Set<Join<?, ?>> getNonInnerJoins(Root<?> root) {

return getNonInnerJoins((From<?, ?>) root);
}

private Set<Join<?, ?>> getNonInnerJoins(From<?, ?> root) {

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

0 comments on commit 7c4cba8

Please sign in to comment.