Skip to content

Commit

Permalink
Optimize RSQL Visitor G2 (#1828)
Browse files Browse the repository at this point in the history 
Use single join for or of same type as 'tag==tag1 or tag==tag2 or tag==tag3'

Signed-off-by: Marinov Avgustin <[email protected]>
  • Loading branch information
@avgustinmm
avgustinmm authored Aug 27, 2024
1 parent ba9013d commit 7753f6c
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 67 deletions.
124 changes: 68 additions & 56 deletions ...tory-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/rsql/JpaQueryRsqlVisitorG2.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.function.Function;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -56,8 +58,8 @@
* @param <T> the entity type referenced by the root
*/
@Slf4j
public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T> extends AbstractFieldNameRSQLVisitor<A>
implements RSQLVisitor<List<Predicate>, String> {
public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T>
extends AbstractFieldNameRSQLVisitor<A> implements RSQLVisitor<List<Predicate>, String> {

public static final Character LIKE_WILDCARD = '*';
private static final char ESCAPE_CHAR = '\\';
Expand All @@ -73,6 +75,8 @@ public class JpaQueryRsqlVisitorG2<A extends Enum<A> & FieldNameProvider, T> ext

private final SimpleTypeConverter simpleTypeConverter = new SimpleTypeConverter();

private boolean inOr;
private final Map<Class<?>, Path<Object>> javaTypeToPath = new HashMap<>();
private boolean joinsNeeded;

public JpaQueryRsqlVisitorG2(final Class<A> enumType,
Expand All @@ -99,11 +103,17 @@ public List<Predicate> visit(final AndNode node, final String param) {

@Override
public List<Predicate> visit(final OrNode node, final String param) {
final List<Predicate> children = acceptChildren(node);
if (children.isEmpty()) {
return toSingleList(cb.conjunction());
} else {
return toSingleList(cb.or(children.toArray(new Predicate[0])));
inOr = true;
try {
final List<Predicate> children = acceptChildren(node);
if (children.isEmpty()) {
return toSingleList(cb.conjunction());
} else {
return toSingleList(cb.or(children.toArray(new Predicate[0])));
}
} finally {
inOr = false;
javaTypeToPath.clear();
}
}

Expand All @@ -117,30 +127,29 @@ public List<Predicate> visit(final ComparisonNode node, final String param) {
final Path<Object> fieldPath = getFieldPath(root, fieldName.getSubAttributes(finalProperty), fieldName.isMap());

for (final String value : values) {
transformedValues.add(convertValueIfNecessary(node, fieldName, value, fieldPath));
transformedValues.add(convertValueIfNecessary(node, fieldName, fieldPath, value));
}

this.joinsNeeded = this.joinsNeeded || areJoinsNeeded(node);

return mapToPredicate(node, fieldPath, node.getArguments(), transformedValues, fieldName, finalProperty);
return mapToPredicate(node, fieldName, finalProperty, fieldPath, node.getArguments(), transformedValues);
}

private List<Predicate> mapToPredicate(final ComparisonNode node, final Path<Object> fieldPath,
final List<String> values, final List<Object> transformedValues, final A enumField,
final String finalProperty) {
private List<Predicate> mapToPredicate(final ComparisonNode node, final A enumField, final String finalProperty,
final Path<Object> fieldPath,
final List<String> values, final List<Object> transformedValues) {
// if lookup is available, replace macros ...
final String value = virtualPropertyReplacer == null ? values.get(0) : virtualPropertyReplacer.replace(values.get(0));

final Predicate mapPredicate = mapToMapPredicate(node, fieldPath, enumField);

final Predicate valuePredicate = addOperatorPredicate(node, getMapValueFieldPath(enumField, fieldPath),
transformedValues, value, finalProperty, enumField);
final Predicate mapPredicate = mapToMapPredicate(node, enumField, fieldPath);
final Predicate valuePredicate = addOperatorPredicate(node, enumField, finalProperty,
getMapValueFieldPath(enumField, fieldPath), transformedValues, value);

return toSingleList(mapPredicate != null ? cb.and(mapPredicate, valuePredicate) : valuePredicate);
}

@SuppressWarnings("unchecked")
private Predicate mapToMapPredicate(final ComparisonNode node, final Path<Object> fieldPath, final A enumField) {
private Predicate mapToMapPredicate(final ComparisonNode node, final A enumField, final Path<Object> fieldPath) {
if (!enumField.isMap()) {
return null;
}
Expand All @@ -160,27 +169,27 @@ private Predicate mapToMapPredicate(final ComparisonNode node, final Path<Object
return equal(fieldPath.get(keyFieldName), keyValue);
}

private Predicate addOperatorPredicate(final ComparisonNode node, final Path<Object> fieldPath,
final List<Object> transformedValues, final String value, final String finalProperty, final A enumField) {
private Predicate addOperatorPredicate(final ComparisonNode node, final A enumField, final String finalProperty,
final Path<Object> fieldPath, final List<Object> transformedValues, final String value) {
// only 'equal' and 'notEqual' can handle transformed value like enums.
// The JPA API cannot handle object types for greaterThan etc. methods.
final Object transformedValue = transformedValues.get(0);
final String operator = node.getOperator().getSymbol();
return switch (operator) {
case "==" -> getEqualToPredicate(transformedValue, fieldPath);
case "!=" -> getNotEqualToPredicate(transformedValue, fieldPath, finalProperty, enumField);
case "==" -> getEqualToPredicate(fieldPath, transformedValue);
case "!=" -> getNotEqualToPredicate(enumField, finalProperty, fieldPath, transformedValue);
case "=gt=" -> cb.greaterThan(pathOfString(fieldPath), value);
case "=ge=" -> cb.greaterThanOrEqualTo(pathOfString(fieldPath), value);
case "=lt=" -> cb.lessThan(pathOfString(fieldPath), value);
case "=le=" -> cb.lessThanOrEqualTo(pathOfString(fieldPath), value);
case "=in=" -> in(pathOfString(fieldPath), transformedValues);
case "=out=" -> getOutPredicate(transformedValues, finalProperty, enumField, fieldPath);
case "=out=" -> getOutPredicate(enumField, finalProperty, fieldPath, transformedValues);
default -> throw new RSQLParameterSyntaxException(
"Operator symbol {" + operator + "} is either not supported or not implemented");
};
}

private Predicate getEqualToPredicate(final Object transformedValue, final Path<Object> fieldPath) {
private Predicate getEqualToPredicate(final Path<Object> fieldPath, final Object transformedValue) {
if (transformedValue == null) {
return cb.isNull(pathOfString(fieldPath));
}
Expand All @@ -200,9 +209,8 @@ private Predicate getEqualToPredicate(final Object transformedValue, final Path<
return cb.equal(fieldPath, transformedValue);
}

private Predicate getNotEqualToPredicate(final Object transformedValue, final Path<Object> fieldPath,
final String finalProperty, final A enumField) {

private Predicate getNotEqualToPredicate(final A enumField, final String finalProperty,
final Path<Object> fieldPath, final Object transformedValue) {
if (transformedValue == null) {
return cb.isNotNull(pathOfString(fieldPath));
}
Expand All @@ -222,10 +230,9 @@ private Predicate getNotEqualToPredicate(final Object transformedValue, final Pa
}
}

clearOuterJoinsIfNotNeeded();
clearJoinsIfNotNeeded();

return toNotExistsSubQueryPredicate(
fieldNames, enumField, expressionToCompare ->
return toNotExistsSubQueryPredicate(enumField, fieldNames, expressionToCompare ->
isPattern(transformedValueStr) ? // a pattern, use like
like(expressionToCompare, toSQL(transformedValueStr)) :
equal(expressionToCompare, transformedValueStr));
Expand All @@ -234,52 +241,58 @@ private Predicate getNotEqualToPredicate(final Object transformedValue, final Pa
return toNullOrNotEqualPredicate(fieldPath, transformedValue);
}

private Predicate getOutPredicate(
final List<Object> transformedValues, final String finalProperty,
final A enumField, final Path<Object> fieldPath) {
private Predicate getOutPredicate(final A enumField, final String finalProperty, final Path<Object> fieldPath,
final List<Object> transformedValues) {
final String[] fieldNames = enumField.getSubAttributes(finalProperty);

if (isSimpleField(fieldNames, enumField.isMap())) {
final Path<String> pathOfString = pathOfString(fieldPath);
return cb.or(cb.isNull(pathOfString), cb.not(in(pathOfString, transformedValues)));
}

clearOuterJoinsIfNotNeeded();
clearJoinsIfNotNeeded();

return toNotExistsSubQueryPredicate(fieldNames, enumField, expressionToCompare -> in(expressionToCompare, transformedValues));
return toNotExistsSubQueryPredicate(enumField, fieldNames,
expressionToCompare -> in(expressionToCompare, transformedValues));
}

private static Path<Object> getFieldPath(
private Path<Object> getFieldPath(
final Root<?> root, final String[] split, final boolean isMapKeyField) {
Path<Object> fieldPath = null;
for (int i = 0, end = isMapKeyField ? split.length - 1 : split.length; i < end; i++) {
final String fieldNameSplit = split[i];
fieldPath = fieldPath == null ?
// if root.get creates a join we call join directly in order to specify LEFT JOIN type,
// to include rows for missing in particular table / criteria (root.get creates INNER JOIN)
// (see org.eclipse.persistence.internal.jpa.querydef.FromImpl implementation for more details)
// otherwise delegate to root.get
(isJoin(root, fieldNameSplit) ? root.join(fieldNameSplit, JoinType.LEFT) : root.get(fieldNameSplit)) :
fieldPath.get(fieldNameSplit);
getPath(root, fieldNameSplit) : fieldPath.get(fieldNameSplit);
}
if (fieldPath == null) {
throw new RSQLParameterUnsupportedFieldException("RSQL field path cannot be empty", null);
}
return fieldPath;
}
private static boolean isJoin(final Root<?> root, final String fieldNameSplit) {
// if root.get creates a join we call join directly in order to specify LEFT JOIN type,
// to include rows for missing in particular table / criteria (root.get creates INNER JOIN)
// (see org.eclipse.persistence.internal.jpa.querydef.FromImpl implementation for more details)
// otherwise delegate to root.get
private Path<Object> getPath(final Root<?> root, final String fieldNameSplit) {
// see org.eclipse.persistence.internal.jpa.querydef.FromImpl implementation for more details
// when root.get creates a join
final Attribute<?, ?> attribute = root.getModel().getAttribute(fieldNameSplit);
if (!attribute.isCollection()) {
// it is a SingularAttribute and not join if it is of basic persistent type
return !((SingularAttribute<?, ?>) attribute).getType().getPersistenceType().equals(Type.PersistenceType.BASIC);
if (((SingularAttribute<?, ?>) attribute).getType().getPersistenceType().equals(Type.PersistenceType.BASIC)) {
return root.get(fieldNameSplit);
}
} // if a collection - it is a join
return true;
if (inOr && root == this.root) { // try to reuse join of the same or level and no subquery
final Class<?> objectClass = attribute.getJavaType();
return javaTypeToPath.computeIfAbsent(objectClass, k -> root.join(fieldNameSplit, JoinType.LEFT));
} else {
return root.join(fieldNameSplit, JoinType.LEFT);
}
}

private Object convertValueIfNecessary(final ComparisonNode node, final A fieldName, final String value,
final Path<Object> fieldPath) {
private Object convertValueIfNecessary(
final ComparisonNode node, final A fieldName, final Path<Object> fieldPath, final String value) {
// in case the value of an RSQL query e.g. type==application is an
// enum we need to handle it separately because JPA needs the
// correct java-type to build an expression. So String and numeric
Expand All @@ -288,20 +301,20 @@ private Object convertValueIfNecessary(final ComparisonNode node, final A fieldN
// class.
final Class<?> javaType = fieldPath.getJavaType();
if (javaType != null && javaType.isEnum()) {
return transformEnumValue(node, value, javaType);
return transformEnumValue(node, javaType, value);
}
if (fieldName instanceof FieldValueConverter) {
return convertFieldConverterValue(node, fieldName, value);
}

if (Boolean.TYPE.equals(javaType)) {
return convertBooleanValue(node, value, javaType);
return convertBooleanValue(node, javaType, value);
}

return value;
}
@SuppressWarnings({ "rawtypes", "unchecked" })
private static Object transformEnumValue(final ComparisonNode node, final String value, final Class<?> javaType) {
private static Object transformEnumValue(final ComparisonNode node, final Class<?> javaType, final String value) {
final Class<? extends Enum> tmpEnumType = (Class<? extends Enum>) javaType;
try {
return Enum.valueOf(tmpEnumType, value.toUpperCase());
Expand All @@ -318,7 +331,7 @@ private static Object transformEnumValue(final ComparisonNode node, final String
+ "}", e);
}
}
private Object convertBooleanValue(final ComparisonNode node, final String value, final Class<?> javaType) {
private Object convertBooleanValue(final ComparisonNode node, final Class<?> javaType, final String value) {
try {
return simpleTypeConverter.convertIfNecessary(value, javaType);
} catch (final TypeMismatchException e) {
Expand Down Expand Up @@ -350,7 +363,7 @@ private Path<Object> getMapValueFieldPath(final A enumField, final Path<Object>
return fieldPath.get(valueFieldNameFromSubEntity);
}

private void clearOuterJoinsIfNotNeeded() {
private void clearJoinsIfNotNeeded() {
if (!joinsNeeded) {
root.getJoins().clear();
}
Expand All @@ -365,21 +378,20 @@ private Predicate toNullOrNotEqualPredicate(final Path<Object> fieldPath, final
}

@SuppressWarnings({ "unchecked", "rawtypes" })
private Predicate toNotExistsSubQueryPredicate(final String[] fieldNames, final A enumField,
final Function<Expression<String>, Predicate> subQueryPredicateProvider) {
private Predicate toNotExistsSubQueryPredicate(final A enumField, final String[] fieldNames, final Function<Expression<String>, Predicate> subQueryPredicateProvider) {
final Class<?> javaType = root.getJavaType();
final Subquery<?> subquery = query.subquery(javaType);
final Root subqueryRoot = subquery.from(javaType);
final Predicate equalPredicate = cb.equal(root.get(enumField.identifierFieldName()),
subqueryRoot.get(enumField.identifierFieldName()));
final Expression<String> expressionToCompare = getExpressionToCompare(
getFieldPath(subqueryRoot, fieldNames, enumField.isMap()), enumField);
final Expression<String> expressionToCompare = getExpressionToCompare(enumField,
getFieldPath(subqueryRoot, fieldNames, enumField.isMap()));
final Predicate subQueryPredicate = subQueryPredicateProvider.apply(expressionToCompare);
subquery.select(subqueryRoot).where(cb.and(equalPredicate, subQueryPredicate));
return cb.not(cb.exists(subquery));
}
@SuppressWarnings({ "rawtypes", "unchecked" })
private Expression<String> getExpressionToCompare(final Path fieldPath, final A enumField) {
private Expression<String> getExpressionToCompare(final A enumField, final Path fieldPath) {
if (!enumField.isMap()) {
return pathOfString(fieldPath);
}
Expand Down
22 changes: 11 additions & 11 deletions ...t-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLToSQLTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import jakarta.persistence.EntityManager;
import jakarta.persistence.PersistenceContext;
import org.eclipse.hawkbit.repository.FieldNameProvider;
import org.eclipse.hawkbit.repository.TargetFields;
import org.eclipse.hawkbit.repository.jpa.RepositoryApplicationConfiguration;
import org.eclipse.hawkbit.repository.jpa.model.JpaTarget;
Expand All @@ -27,6 +28,7 @@

@ActiveProfiles("test")
@SpringBootTest(webEnvironment=NONE, properties = {
"hawkbit.rsql.caseInsensitiveDB=true",
"spring.main.allow-bean-definition-overriding=true",
"spring.main.banner-mode=off",
"logging.level.root=ERROR" })
Expand All @@ -44,18 +46,16 @@ private void setEntityManager(final EntityManager entityManager) {

@Test
public void print() {
String rsql = "tag==tag1 or tag==tag2 or tag==tag3";
System.out.println(rsql + "\n" +
"\tlegacy:\n" +
"\t\t" + rsqlToSQL.toSQL(JpaTarget.class, TargetFields.class, rsql, true) + "\n" +
"\tG2:\n" +
"\t\t" + rsqlToSQL.toSQL(JpaTarget.class, TargetFields.class, rsql, false));
print(JpaTarget.class, TargetFields.class, "tag==tag1 or tag==tag2 or tag==tag3");
print(JpaTarget.class, TargetFields.class, "targettype.key==type1 and metadata.key1==target1-value1");
print(JpaTarget.class, TargetFields.class, "(tag!=TAG1 or tag !=TAG2)");
}

rsql = "targettype.key==type1 and metadata.key1==target1-value1";
System.out.println(rsql + "\n" +
"\tlegacy:\n" +
"\t\t" + rsqlToSQL.toSQL(JpaTarget.class, TargetFields.class, rsql, true) + "\n" +
"\tG2:\n" +
private <T, A extends Enum<A> & FieldNameProvider> void print(final Class<T> domainClass, final Class<A> fieldsClass, final String rsql) {
System.out.println(rsql);
System.out.println("\tlegacy:\n" +
"\t\t" + rsqlToSQL.toSQL(JpaTarget.class, TargetFields.class, rsql, true));
System.out.println("\tG2:\n" +
"\t\t" + rsqlToSQL.toSQL(JpaTarget.class, TargetFields.class, rsql, false));
}
}

0 comments on commit 7753f6c

Please sign in to comment.