simplify quoting algorithm in FilterHelper

and generally refactor logic

eliminate cast to FilterImpl

Signed-off-by: Gavin King <gavin@hibernate.org>

hibernate-core/src/main/java/org/hibernate/Filter.java

@@ -109,4 +109,16 @@ public interface Filter {
 	 * @return The flag value
 	 */
 	boolean isAppliedToLoadByKey();
+
+	/**
+	 * Obtain the argument currently bound to the filter parameter
+	 * with the given name.
+	 *
+	 * @param name the name of the filter parameter
+	 * @return the value currently set
+	 *
+	 * @since 7
+	 */
+	@Incubating
+	Object getParameterValue(String name);
 }

hibernate-core/src/main/java/org/hibernate/internal/FilterHelper.java

@@ -16,8 +16,6 @@ import java.util.regex.Pattern;
 import org.hibernate.Filter;
 import org.hibernate.engine.spi.LoadQueryInfluencers;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
-import org.hibernate.internal.util.StringHelper;
-import org.hibernate.internal.util.collections.CollectionHelper;
 import org.hibernate.metamodel.mapping.Restrictable;
 import org.hibernate.persister.entity.EntityNameUse;
 import org.hibernate.sql.Template;
@@ -26,17 +24,21 @@ import org.hibernate.sql.ast.tree.from.TableGroup;
 import org.hibernate.sql.ast.tree.predicate.FilterPredicate;
 import org.hibernate.sql.ast.tree.predicate.Predicate;
 
+import static org.hibernate.internal.FilterImpl.MARKER;
+import static org.hibernate.internal.util.StringHelper.isNotEmpty;
+import static org.hibernate.internal.util.StringHelper.replace;
 import static org.hibernate.internal.util.StringHelper.safeInterning;
+import static org.hibernate.internal.util.collections.CollectionHelper.isNotEmpty;
 
 /**
- * Implementation of FilterHelper.
+ * Utility methods for dealing with {@linkplain FilterConfiguration filters}.
  *
  * @author Steve Ebersole
  * @author Rob Worsnop
  * @author Nathan Xu
  */
 public class FilterHelper {
-	private static final Pattern FILTER_PARAMETER_PATTERN = Pattern.compile( ":(\\S+)(\\w+)" );
+	private static final Pattern FILTER_PARAMETER_PATTERN = Pattern.compile( ":((\\S+)(\\w+))" );
 
 	private final String[] filterNames;
 	private final String[] filterConditions;
@@ -76,48 +78,51 @@ public class FilterHelper {
 			filterAliasTableMaps[filterCount] = filter.getAliasTableMap( factory );
 			filterAutoAliasFlags[filterCount] = false;
 
-			if ( ( filterAliasTableMaps[filterCount].isEmpty()
+			injectAliases( factory, filter, filterCount );
-					|| isTableFromPersistentClass( filterAliasTableMaps[filterCount] ) )
+			qualifyParameterNames( filterCount, filterName );
-					&& filter.useAutoAliasInjection() ) {
-				final String autoAliasedCondition = Template.renderWhereStringTemplate(
-						filter.getCondition(),
-						FilterImpl.MARKER,
-						factory.getJdbcServices().getDialect(),
-						factory.getTypeConfiguration()
-				);
-				filterConditions[filterCount] = safeInterning( autoAliasedCondition );
-				filterAutoAliasFlags[filterCount] = true;
-			}
-
-			// look for parameters in the condition.  for each parameter, we:
-			//		1) keep track of the name for later
-			//		2) // we replace `:{param-name} ` with `:{filter-name}.{param-name} ` in the condition
-			final Matcher matcher = FILTER_PARAMETER_PATTERN.matcher( filterConditions[filterCount] );
-
-			String copy = filterConditions[filterCount];
-			final List<String> filterParamNames = new ArrayList<>();
-			parameterNames[filterCount] = filterParamNames;
-			boolean foundAny = false;
-
-			// handle any subsequent matched parameters
-			while( matcher.find() ) {
-				final String parameterLabel = filterConditions[filterCount].substring( matcher.start() + 1, matcher.end() );
-				filterParamNames.add( parameterLabel );
-				copy = copy.replace(
-						":" + parameterLabel,
-						":" + filterName + "." + parameterLabel
-				);
-				foundAny = true;
-			}
-
-			if ( foundAny ) {
-				filterConditions[filterCount] = safeInterning( copy );
-			}
 
 			filterCount++;
 		}
 	}
 
+	private void injectAliases(SessionFactoryImplementor factory, FilterConfiguration filter, int filterCount) {
+		if ( ( filterAliasTableMaps[filterCount].isEmpty()
+				|| isTableFromPersistentClass( filterAliasTableMaps[filterCount] ) )
+				&& filter.useAutoAliasInjection() ) {
+			final String autoAliasedCondition = Template.renderWhereStringTemplate(
+					filter.getCondition(),
+					MARKER,
+					factory.getJdbcServices().getDialect(),
+					factory.getTypeConfiguration()
+			);
+			filterConditions[filterCount] = safeInterning( autoAliasedCondition );
+			filterAutoAliasFlags[filterCount] = true;
+		}
+	}
+
+	/**
+	 * Look for parameters in the given condition. For each parameter, we:
+	 * <ol>
+	 *     <li>keep track of the name for later</li>
+	 *     <li>replace {@code :{param-name}} with {@code :{filter-name}.{param-name}}
+	 *     in the condition</li>
+	 * </ol>
+	 */
+	private void qualifyParameterNames(int filterCount, String filterName) {
+		final List<String> parameterNames = new ArrayList<>();
+		boolean foundAny = false;
+		final Matcher matcher = FILTER_PARAMETER_PATTERN.matcher( filterConditions[filterCount] );
+		while ( matcher.find() ) {
+			parameterNames.add( matcher.group(1) );
+			foundAny = true;
+		}
+		if ( foundAny ) {
+			filterConditions[filterCount] =
+					safeInterning( matcher.replaceAll(":" + filterName +  ".$1") );
+		}
+		this.parameterNames[filterCount] = parameterNames;
+	}
+
 	private static boolean isTableFromPersistentClass(Map<String, String> aliasTableMap) {
 		return aliasTableMap.size() == 1 && aliasTableMap.containsKey( null );
 	}
@@ -186,8 +191,7 @@ public class FilterHelper {
 		final FilterPredicate filterPredicate = new FilterPredicate();
 
 		for ( int i = 0, max = filterNames.length; i < max; i++ ) {
-			final String filterName = filterNames[i];
+			final Filter enabledFilter = enabledFilters.get( filterNames[i] );
-			final FilterImpl enabledFilter = (FilterImpl) enabledFilters.get( filterName );
 			if ( enabledFilter != null && ( !onlyApplyLoadByKeyFilters || enabledFilter.isAppliedToLoadByKey() ) ) {
 				filterPredicate.applyFragment( render( aliasGenerator, i, tableGroup, creationState ), enabledFilter, parameterNames[i] );
 			}
@@ -201,23 +205,21 @@ public class FilterHelper {
 	}
 
 	public String render(FilterAliasGenerator aliasGenerator, Map<String, Filter> enabledFilters) {
-		StringBuilder buffer = new StringBuilder();
+		final StringBuilder buffer = new StringBuilder();
 		render( buffer, aliasGenerator, enabledFilters );
 		return buffer.toString();
 	}
 
 	public void render(StringBuilder buffer, FilterAliasGenerator aliasGenerator, Map<String, Filter> enabledFilters) {
-		if ( CollectionHelper.isEmpty( filterNames ) ) {
+		if ( isNotEmpty( filterNames ) ) {
-			return;
+			for ( int i = 0, max = filterNames.length; i < max; i++ ) {
-		}
+				if ( enabledFilters.containsKey( filterNames[i] ) ) {
-		for ( int i = 0, max = filterNames.length; i < max; i++ ) {
+					if ( isNotEmpty( filterConditions[i] ) ) {
-			if ( enabledFilters.containsKey( filterNames[i] ) ) {
+						if ( !buffer.isEmpty() ) {
-				final String condition = filterConditions[i];
+							buffer.append( " and " );
-				if ( StringHelper.isNotEmpty( condition ) ) {
+						}
-					if ( buffer.length() > 0 ) {
+						buffer.append( render( aliasGenerator, i, null, null ) );
-						buffer.append( " and " );
 					}
-					buffer.append( render( aliasGenerator, i, null, null ) );
 				}
 			}
 		}
@@ -228,70 +230,71 @@ public class FilterHelper {
 			int filterIndex,
 			TableGroup tableGroup,
 			SqlAstCreationState creationState) {
-		Map<String, String> aliasTableMap = filterAliasTableMaps[filterIndex];
+		final String condition = filterConditions[filterIndex];
-		String condition = filterConditions[filterIndex];
 		if ( aliasGenerator == null ) {
-			return StringHelper.replace( condition, FilterImpl.MARKER + ".", "");
+			return replace( condition, MARKER + ".", "");
-		}
-		if ( filterAutoAliasFlags[filterIndex] ) {
-			final String tableName = aliasTableMap.get( null );
-			final String newCondition = StringHelper.replace(
-					condition,
-					FilterImpl.MARKER,
-					aliasGenerator.getAlias( tableName )
-			);
-			if ( creationState != null && tableToEntityName != null && !newCondition.equals( condition ) ) {
-				creationState.registerEntityNameUsage(
-						tableGroup,
-						EntityNameUse.EXPRESSION,
-						tableToEntityName.get(
-								tableName == null
-										? tableGroup.getPrimaryTableReference().getTableId()
-										: tableName
-						)
-				);
-			}
-			return newCondition;
-		}
-		else if ( isTableFromPersistentClass( aliasTableMap ) ) {
-			final String tableName = aliasTableMap.get( null );
-			final String newCondition = StringHelper.replace(
-					condition,
-					"{alias}",
-					aliasGenerator.getAlias( tableName )
-			);
-			if ( creationState != null && !newCondition.equals( condition ) ) {
-				creationState.registerEntityNameUsage(
-						tableGroup,
-						EntityNameUse.EXPRESSION,
-						tableToEntityName.get(
-								tableName == null
-										? tableGroup.getPrimaryTableReference().getTableId()
-										: tableName
-						)
-				);
-			}
-			return newCondition;
 		}
 		else {
-			for ( Map.Entry<String, String> entry : aliasTableMap.entrySet() ) {
+			final Map<String, String> aliasTableMap = filterAliasTableMaps[filterIndex];
-				final String tableName = entry.getValue();
+			if ( filterAutoAliasFlags[filterIndex] ) {
-				final String newCondition = StringHelper.replace(
+				final String tableName = aliasTableMap.get( null );
-						condition,
+				return replaceMarker( tableGroup, creationState, condition,
-						"{" + entry.getKey() + "}",
+						aliasGenerator.getAlias( tableName ),
-						aliasGenerator.getAlias( tableName )
+						tableName( tableGroup, tableName ) );
-				);
+			}
-				if ( creationState != null && !newCondition.equals( condition ) ) {
+			else if ( isTableFromPersistentClass( aliasTableMap ) ) {
-					creationState.registerEntityNameUsage(
+				final String tableName = aliasTableMap.get( null );
-							tableGroup,
+				return replaceAlias( tableGroup, creationState, condition,
-							EntityNameUse.EXPRESSION,
+						"{alias}",
-							tableToEntityName.get( tableName )
+						aliasGenerator.getAlias( tableName ),
-					);
+						tableName( tableGroup, tableName ) );
-				}
+			}
-				condition = newCondition;
+			else {
+				String newCondition = condition;
+				for ( Map.Entry<String, String> entry : aliasTableMap.entrySet() ) {
+					final String tableName = entry.getValue();
+					newCondition =
+							replaceAlias( tableGroup, creationState, newCondition,
+									"{" + entry.getKey() + "}",
+									aliasGenerator.getAlias( tableName ),
+									tableName );
+				}
+				return newCondition;
 			}
-			return condition;
 		}
 	}
 
+	private String replaceMarker(
+			TableGroup tableGroup, SqlAstCreationState creationState,
+			String condition, String alias, String tableName) {
+		final String newCondition = replace( condition, MARKER, alias );
+		if ( creationState != null
+				&& tableToEntityName != null
+				&& !newCondition.equals(condition) ) {
+			registerEntityNameUsage( tableGroup, creationState, tableName );
+		}
+		return newCondition;
+	}
+
+	private String replaceAlias(
+			TableGroup tableGroup, SqlAstCreationState creationState,
+			String condition, String placeholder, String alias, String tableName) {
+		final String newCondition = replace( condition, placeholder, alias );
+		if ( creationState != null
+				&& !newCondition.equals(condition) ) {
+			registerEntityNameUsage( tableGroup, creationState, tableName );
+		}
+		return newCondition;
+	}
+
+	private void registerEntityNameUsage(TableGroup tableGroup, SqlAstCreationState creationState, String tableName) {
+		creationState.registerEntityNameUsage( tableGroup, EntityNameUse.EXPRESSION,
+				tableToEntityName.get( tableName ) );
+	}
+
+	private static String tableName(TableGroup tableGroup, String tableName) {
+		return tableName == null
+				? tableGroup.getPrimaryTableReference().getTableId()
+				: tableName;
+	}
 }

hibernate-core/src/main/java/org/hibernate/internal/FilterImpl.java

@@ -204,4 +204,16 @@ public class FilterImpl implements Filter, Serializable {
 	private boolean hasArgument(String parameterName) {
 		return parameters != null && parameters.containsKey(parameterName);
 	}
+
+	@Override
+	public Object getParameterValue(String paramName) {
+		final Object value = getParameter( paramName );
+		if ( value != null ) {
+			return value;
+		}
+		else {
+			final Supplier<?> filterParamResolver = getParameterResolver( paramName );
+			return filterParamResolver == null ? null : filterParamResolver.get();
+		}
+	}
 }

hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java

@@ -174,13 +174,13 @@ public final class StringHelper {
 		if ( template == null ) {
 			return null;
 		}
-		int loc = indexOfPlaceHolder( template, placeholder, wholeWords );
+		final int loc = indexOfPlaceHolder( template, placeholder, wholeWords );
 		if ( loc < 0 ) {
 			return template;
 		}
 		else {
-			String beforePlaceholder = template.substring( 0, loc );
+			final String beforePlaceholder = template.substring( 0, loc );
-			String afterPlaceholder = template.substring( loc + placeholder.length() );
+			final String afterPlaceholder = template.substring( loc + placeholder.length() );
 			return replace(
 					beforePlaceholder,
 					afterPlaceholder,
@@ -201,7 +201,7 @@ public final class StringHelper {
 			boolean encloseInParensIfNecessary) {
 		final boolean actuallyReplace =
 				!wholeWords
-						|| afterPlaceholder.length() == 0
+						|| afterPlaceholder.isEmpty()
 						|| !Character.isJavaIdentifierPart( afterPlaceholder.charAt( 0 ) );
 		// We only need to check the left param to determine if the placeholder is already
 		// enclosed in parentheses (HHH-10383)
@@ -214,13 +214,13 @@ public final class StringHelper {
 		// We need to check the placeholder is not used in `Order By FIELD(...)` (HHH-10502)
 		// Examples:
 		// " ... Order By FIELD(id,?1)",  after expand parameters, the sql is "... Order By FIELD(id,?,?,?)"
-		boolean encloseInParens =
+		final boolean encloseInParens =
 				actuallyReplace
 						&& encloseInParensIfNecessary
 						&& !( getLastNonWhitespaceCharacter( beforePlaceholder ) == '(' ) &&
 						!( getLastNonWhitespaceCharacter( beforePlaceholder ) == ',' && getFirstNonWhitespaceCharacter(
 								afterPlaceholder ) == ')' );
-		StringBuilder buf = new StringBuilder( beforePlaceholder );
+		final StringBuilder buf = new StringBuilder( beforePlaceholder );
 		if ( encloseInParens ) {
 			buf.append( '(' );
 		}

hibernate-core/src/main/java/org/hibernate/mapping/Selectable.java

@@ -8,10 +8,8 @@ package org.hibernate.mapping;
 
 import org.hibernate.Incubating;
 import org.hibernate.dialect.Dialect;
-import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.metamodel.mapping.JdbcMapping;
 import org.hibernate.query.sqm.function.SqmFunctionRegistry;
-import org.hibernate.type.descriptor.jdbc.JdbcType;
 import org.hibernate.type.spi.TypeConfiguration;
 
 /**

hibernate-core/src/main/java/org/hibernate/sql/ast/tree/predicate/FilterPredicate.java

@@ -8,10 +8,8 @@ package org.hibernate.sql.ast.tree.predicate;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.function.Supplier;
 
-import org.hibernate.engine.spi.FilterDefinition;
+import org.hibernate.Filter;
-import org.hibernate.internal.FilterImpl;
 import org.hibernate.internal.FilterJdbcParameter;
 import org.hibernate.internal.util.collections.CollectionHelper;
 import org.hibernate.metamodel.mapping.JdbcMapping;
@@ -37,7 +35,7 @@ public class FilterPredicate implements Predicate {
 		fragments.add( predicate );
 	}
 
-	public void applyFragment(String processedFragment, FilterImpl filter, List<String> parameterNames) {
+	public void applyFragment(String processedFragment, Filter filter, List<String> parameterNames) {
 		fragments.add( new FilterFragmentPredicate( processedFragment, filter, parameterNames ) );
 	}
 
@@ -102,11 +100,11 @@ public class FilterPredicate implements Predicate {
 	}
 
 	public static class FilterFragmentPredicate implements Predicate {
-		private final FilterImpl filter;
+		private final Filter filter;
 		private final String sqlFragment;
 		private final List<FilterFragmentParameter> parameters;
 
-		public FilterFragmentPredicate(String sqlFragment, FilterImpl filter, List<String> parameterNames) {
+		public FilterFragmentPredicate(String sqlFragment, Filter filter, List<String> parameterNames) {
 			this.filter = filter;
 			this.sqlFragment = sqlFragment;
 
@@ -117,16 +115,15 @@ public class FilterPredicate implements Predicate {
 				parameters = CollectionHelper.arrayList( parameterNames.size() );
 				for ( int i = 0; i < parameterNames.size(); i++ ) {
 					final String paramName = parameterNames.get( i );
-					final Object paramValue = retrieveParamValue(filter, paramName);
+					final Object paramValue = filter.getParameterValue( paramName );
-					final FilterDefinition filterDefinition = filter.getFilterDefinition();
+					final JdbcMapping jdbcMapping = filter.getFilterDefinition().getParameterJdbcMapping( paramName );
-					final JdbcMapping jdbcMapping = filterDefinition.getParameterJdbcMapping( paramName );
 
 					parameters.add( new FilterFragmentParameter( filter.getName(), paramName, jdbcMapping, paramValue ) );
 				}
 			}
 		}
 
-		public FilterImpl getFilter() {
+		public Filter getFilter() {
 			return filter;
 		}
 
@@ -156,15 +153,5 @@ public class FilterPredicate implements Predicate {
 		public boolean isEmpty() {
 			return false;
 		}
-
-		private Object retrieveParamValue(FilterImpl filter, String paramName) {
-			Object value = filter.getParameter(paramName);
-			if (value != null) {
-				return value;
-			}
-
-			final Supplier<?> filterParamResolver = filter.getParameterResolver( paramName );
-			return filterParamResolver == null ? null : filterParamResolver.get();
-		}
 	}
 }