From 94b21685a005312d049e1cf0f05e754a874ef6ab Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 23 Oct 2024 09:25:07 +0200 Subject: [PATCH 1/5] Polishing. Refactor ContextualValueExpressionEvaluator into Function, refactor BindingContext into nested class. Simplify tests. Rename tests to reflect what they are actually doing. Reformat code. See #453 Original pull request: #505 --- src/main/antora/modules/ROOT/nav.adoc | 2 + .../ROOT/pages/ldap/query-methods.adoc | 88 ++++++ .../ROOT/pages/ldap/value-expressions.adoc | 1 + .../query/AbstractLdapRepositoryQuery.java | 1 - .../query/AnnotatedLdapRepositoryQuery.java | 30 +- .../ldap/repository/query/BindingContext.java | 177 ----------- .../ContextualValueExpressionEvaluator.java | 44 --- .../query/LdapParameterAccessor.java | 2 +- .../repository/query/StringBasedQuery.java | 287 +++++++++++++----- .../data/ldap/config/DummyLdapRepository.java | 1 - .../ldap/config/EmbeddedLdapProperties.java | 15 + .../config/InMemoryLdapConfiguration.java | 4 +- ...dLdapRepositoryQueryIntegrationTests.java} | 82 ++--- ...nnotatedLdapRepositoryQueryUnitTests.java} | 47 ++- .../ldap/repository/query/SchemaEntry.java | 6 +- 15 files changed, 393 insertions(+), 394 deletions(-) create mode 100644 src/main/antora/modules/ROOT/pages/ldap/value-expressions.adoc delete mode 100644 src/main/java/org/springframework/data/ldap/repository/query/BindingContext.java delete mode 100644 src/main/java/org/springframework/data/ldap/repository/query/ContextualValueExpressionEvaluator.java rename src/test/java/org/springframework/data/ldap/repository/query/{ValueExpressionLdapRepositoryQueryTests.java => AnnotatedLdapRepositoryQueryIntegrationTests.java} (61%) rename src/test/java/org/springframework/data/ldap/repository/query/{AnnotatedLdapRepositoryQueryTests.java => AnnotatedLdapRepositoryQueryUnitTests.java} (68%) diff --git a/src/main/antora/modules/ROOT/nav.adoc b/src/main/antora/modules/ROOT/nav.adoc index edb3d96b..5f131987 100644 --- a/src/main/antora/modules/ROOT/nav.adoc +++ b/src/main/antora/modules/ROOT/nav.adoc @@ -13,10 +13,12 @@ ** xref:repositories/namespace-reference.adoc[] ** xref:repositories/query-keywords-reference.adoc[] ** xref:repositories/query-return-types-reference.adoc[] + * xref:ldap.adoc[] ** xref:ldap/configuration.adoc[] ** xref:ldap/usage.adoc[] ** xref:ldap/query-methods.adoc[] +** xref:ldap/value-expressions.adoc[] ** xref:ldap/querydsl.adoc[] ** xref:ldap/cdi-integration.adoc[] diff --git a/src/main/antora/modules/ROOT/pages/ldap/query-methods.adoc b/src/main/antora/modules/ROOT/pages/ldap/query-methods.adoc index bdc318e4..9d9c29d3 100644 --- a/src/main/antora/modules/ROOT/pages/ldap/query-methods.adoc +++ b/src/main/antora/modules/ROOT/pages/ldap/query-methods.adoc @@ -76,3 +76,91 @@ The following table provides samples of the keywords that you can use with query | `(!(Firstname=name))` |=== + +[[ldap.query-methods.at-query]] +== Using `@Query` + +If you need to use a custom query that can't be derived from the method name, you can use the `@Query` annotation to define the query. +As queries are tied to the Java method that runs them, you can actually bind parameters to be passed to the query. + +The following example shows a query created with the `@Query` annotation: + +.Declare query at the query method using `@Query` +==== +[source,java] +---- +interface PersonRepository extends LdapRepository { + + @Query("(&(employmentType=*)(!(employmentType=Hired))(mail=:emailAddress))") + Person findEmployeeByEmailAddress(String emailAddress); + +} +---- +==== + +NOTE: Spring Data supports named (parameter names prefixed with `:`) and positional parameter binding (in the form of zero-based `?0`). +We recommend using named parameters for easier readability. +Also, using positional parameters makes query methods a little error-prone when refactoring regarding the parameter position. + +[[ldap.encoding]] +== Parameter Encoding + +Query parameters of String-based queries are encoded according to https://datatracker.ietf.org/doc/html/rfc2254[RFC2254]. +This can lead to undesired escaping of certain characters. +You can specify your own encoder through the `@LdapEncode` annotation that defines which javadoc:org.springframework.data.ldap.repository.LdapEncoder[] to use. + +`@LdapEncode` applies to individual parameters of a query method. +It is not applies for derived queries or Value Expressions (SpEL, Property Placeholders). + +.Declare a custom `LdapEncoder` for a query method +==== +[source,java] +---- +interface PersonRepository extends LdapRepository { + + @Query("(&(employmentType=*)(!(employmentType=Hired))(firstName=:firstName))") + Person findEmployeeByFirstNameLike(@LdapEncode(MyLikeEncoder.class) String firstName); + +} +---- +==== + +[[ldap.query.spel-expressions]] +== Using SpEL Expressions + +Spring Data allows you to use SpEL expressions in your query methods. +SpEL expressions are part of Spring Data's xref:ldap/value-expressions.adoc[Value Expressions] support. +SpEL expressions can be used to manipulate query method arguments as well as to invoke bean methods. +Method arguments can be accessed by name or index as demonstrated in the following example. + +.Using SpEL expressions in Repository Query Methods +==== +[source,java] +---- +@Query("(&(firstName=?#{[0]})(mail=:?#{principal.emailAddress}))") +List findByFirstnameAndCurrentUserWithCustomQuery(String firstname); +---- +==== + +NOTE: Values provided by SpEL expressions are not escaped according to RFC2254. +You have to ensure that the values are properly escaped if needed. +Consider using Spring Ldap's `org.springframework.ldap.support.LdapEncoder` helper class. + +[[ldap.query.property-placeholders]] +== Using Property Placeholders + +Property Placeholders (see xref:ldap/value-expressions.adoc[Value Expressions]) can help to easily customize your queries based on configuration properties from Spring's `Environment`. +These are useful for queries that need to be customized based on the environment or configuration. + +.Using Property Placeholders in Repository Query Methods +==== +[source,java] +---- +@Query("(&(firstName=?0)(stage=:?${myapp.stage:dev}))") +List findByFirstnameAndStageWithCustomQuery(String firstname); +---- +==== + +NOTE: Values provided by Property Placeholders are not escaped according to RFC2254. +You have to ensure that the values are properly escaped if needed. +Consider using Spring Ldap's `org.springframework.ldap.support.LdapEncoder` helper class. diff --git a/src/main/antora/modules/ROOT/pages/ldap/value-expressions.adoc b/src/main/antora/modules/ROOT/pages/ldap/value-expressions.adoc new file mode 100644 index 00000000..6356a462 --- /dev/null +++ b/src/main/antora/modules/ROOT/pages/ldap/value-expressions.adoc @@ -0,0 +1 @@ +include::{commons}@data-commons::page$value-expressions.adoc[] diff --git a/src/main/java/org/springframework/data/ldap/repository/query/AbstractLdapRepositoryQuery.java b/src/main/java/org/springframework/data/ldap/repository/query/AbstractLdapRepositoryQuery.java index d35e6fee..4c4d1f93 100644 --- a/src/main/java/org/springframework/data/ldap/repository/query/AbstractLdapRepositoryQuery.java +++ b/src/main/java/org/springframework/data/ldap/repository/query/AbstractLdapRepositoryQuery.java @@ -26,7 +26,6 @@ import org.springframework.data.repository.query.QueryMethod; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.repository.query.ResultProcessor; -import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.ldap.core.LdapOperations; import org.springframework.ldap.query.LdapQuery; import org.springframework.util.Assert; diff --git a/src/main/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQuery.java b/src/main/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQuery.java index 708fc9fa..a450a905 100644 --- a/src/main/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQuery.java +++ b/src/main/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQuery.java @@ -39,9 +39,9 @@ public class AnnotatedLdapRepositoryQuery extends AbstractLdapRepositoryQuery { private final Query queryAnnotation; - private final ValueExpressionDelegate valueExpressionDelegate; - private final StringBasedQuery stringBasedQuery; - private final StringBasedQuery stringBasedBase; + private final StringBasedQuery query; + private final StringBasedQuery base; + private final ValueEvaluationContextProvider valueContextProvider; /** * Construct a new instance. @@ -81,34 +81,32 @@ public AnnotatedLdapRepositoryQuery(LdapQueryMethod queryMethod, Class entity Assert.notNull(queryMethod.getQueryAnnotation(), "Annotation must be present"); Assert.hasLength(queryMethod.getQueryAnnotation().value(), "Query filter must be specified"); - queryAnnotation = queryMethod.getRequiredQueryAnnotation(); - this.valueExpressionDelegate = valueExpressionDelegate; - stringBasedQuery = new StringBasedQuery(queryAnnotation.value(), queryMethod.getParameters(), valueExpressionDelegate); - stringBasedBase = new StringBasedQuery(queryAnnotation.base(), queryMethod.getParameters(), valueExpressionDelegate); + this.queryAnnotation = queryMethod.getRequiredQueryAnnotation(); + this.query = new StringBasedQuery(queryAnnotation.value(), queryMethod.getParameters(), valueExpressionDelegate); + this.base = new StringBasedQuery(queryAnnotation.base(), queryMethod.getParameters(), valueExpressionDelegate); + this.valueContextProvider = valueExpressionDelegate.createValueContextProvider(getQueryMethod().getParameters()); } @Override protected LdapQuery createQuery(LdapParameterAccessor parameters) { - ValueEvaluationContextProvider valueContextProvider = valueExpressionDelegate - .createValueContextProvider(getQueryMethod().getParameters()); + String query = bind(parameters, valueContextProvider, this.query); + String base = bind(parameters, valueContextProvider, this.base); - String boundQuery = bind(parameters, valueContextProvider, stringBasedQuery); - - String boundBase = bind(parameters, valueContextProvider, stringBasedBase); - - return query().base(boundBase) // + return query().base(base) // .searchScope(queryAnnotation.searchScope()) // .countLimit(queryAnnotation.countLimit()) // .timeLimit(queryAnnotation.timeLimit()) // - .filter(boundQuery); + .filter(query, parameters.getBindableParameterValues()); } private String bind(LdapParameterAccessor parameters, ValueEvaluationContextProvider valueContextProvider, StringBasedQuery query) { + ValueEvaluationContext evaluationContext = valueContextProvider .getEvaluationContext(parameters.getBindableParameterValues(), query.getExpressionDependencies()); + return query.bindQuery(parameters, - new ContextualValueExpressionEvaluator(valueExpressionDelegate, evaluationContext)); + expression -> expression.evaluate(evaluationContext)); } } diff --git a/src/main/java/org/springframework/data/ldap/repository/query/BindingContext.java b/src/main/java/org/springframework/data/ldap/repository/query/BindingContext.java deleted file mode 100644 index 2782acc0..00000000 --- a/src/main/java/org/springframework/data/ldap/repository/query/BindingContext.java +++ /dev/null @@ -1,177 +0,0 @@ -/* - * Copyright 2020-2024 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 - * - * https://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.ldap.repository.query; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -import org.springframework.data.mapping.model.ValueExpressionEvaluator; -import org.springframework.data.repository.query.Parameter; -import org.springframework.data.repository.query.ParameterAccessor; -import org.springframework.data.repository.query.Parameters; -import org.springframework.lang.Nullable; -import org.springframework.ldap.support.LdapEncoder; -import org.springframework.util.Assert; - -/** - * Value object capturing the binding context to provide {@link #getBindingValues() binding values} for queries. - * - * @author Mark Paluch - * @since 3.4 - */ -class BindingContext { - - private final Parameters parameters; - - private final ParameterAccessor parameterAccessor; - - private final List bindings; - - private final ValueExpressionEvaluator evaluator; - - /** - * Create new {@link BindingContext}. - */ - BindingContext(Parameters parameters, ParameterAccessor parameterAccessor, - List bindings, ValueExpressionEvaluator evaluator) { - - this.parameters = parameters; - this.parameterAccessor = parameterAccessor; - this.bindings = bindings; - this.evaluator = evaluator; - } - - /** - * @return {@literal true} when list of bindings is not empty. - */ - private boolean hasBindings() { - return !bindings.isEmpty(); - } - - /** - * Bind values provided by {@link LdapParameterAccessor} to placeholders in {@link BindingContext} while - * considering potential conversions and parameter types. - * - * @return {@literal null} if given {@code raw} value is empty. - */ - public List getBindingValues() { - - if (!hasBindings()) { - return Collections.emptyList(); - } - - List parameters = new ArrayList<>(bindings.size()); - - for (ParameterBinding binding : bindings) { - Object parameterValueForBinding = getParameterValueForBinding(binding); - parameters.add(parameterValueForBinding); - } - - return parameters; - } - - /** - * Return the value to be used for the given {@link ParameterBinding}. - * - * @param binding must not be {@literal null}. - * @return the value used for the given {@link ParameterBinding}. - */ - @Nullable - private Object getParameterValueForBinding(ParameterBinding binding) { - - if (binding.isExpression()) { - return evaluator.evaluate(binding.getRequiredExpression()); - } - - Object value = binding.isNamed() ? - parameterAccessor.getBindableValue(getParameterIndex(parameters, binding.getRequiredParameterName())) : - parameterAccessor.getBindableValue(binding.getParameterIndex()); - return value == null ? null : LdapEncoder.filterEncode(value.toString()); - } - - private int getParameterIndex(Parameters parameters, String parameterName) { - - return parameters.stream() // - .filter(parameter -> parameter // - .getName().filter(s -> s.equals(parameterName)) // - .isPresent()) // - .mapToInt(Parameter::getIndex) // - .findFirst() // - .orElseThrow(() -> new IllegalArgumentException( - String.format("Invalid parameter name; Cannot resolve parameter [%s]", parameterName))); - } - - /** - * A generic parameter binding with name or position information. - * - * @author Mark Paluch - */ - static class ParameterBinding { - - private final int parameterIndex; - private final @Nullable String expression; - private final @Nullable String parameterName; - - private ParameterBinding(int parameterIndex, @Nullable String expression, @Nullable String parameterName) { - - this.parameterIndex = parameterIndex; - this.expression = expression; - this.parameterName = parameterName; - } - - static ParameterBinding expression(String expression, boolean quoted) { - return new ParameterBinding(-1, expression, null); - } - - static ParameterBinding indexed(int parameterIndex) { - return new ParameterBinding(parameterIndex, null, null); - } - - static ParameterBinding named(String name) { - return new ParameterBinding(-1, null, name); - } - - boolean isNamed() { - return (parameterName != null); - } - - int getParameterIndex() { - return parameterIndex; - } - - String getParameter() { - return ("?" + (isExpression() ? "expr" : "") + parameterIndex); - } - - String getRequiredExpression() { - - Assert.state(expression != null, "ParameterBinding is not an expression"); - return expression; - } - - boolean isExpression() { - return (this.expression != null); - } - - String getRequiredParameterName() { - - Assert.state(parameterName != null, "ParameterBinding is not named"); - - return parameterName; - } - } -} diff --git a/src/main/java/org/springframework/data/ldap/repository/query/ContextualValueExpressionEvaluator.java b/src/main/java/org/springframework/data/ldap/repository/query/ContextualValueExpressionEvaluator.java deleted file mode 100644 index 66dd4544..00000000 --- a/src/main/java/org/springframework/data/ldap/repository/query/ContextualValueExpressionEvaluator.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2024 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 - * - * https://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.ldap.repository.query; - -import org.springframework.data.expression.ValueEvaluationContext; -import org.springframework.data.expression.ValueExpression; -import org.springframework.data.expression.ValueExpressionParser; -import org.springframework.data.mapping.model.ValueExpressionEvaluator; - -/** - * @author Marcin Grzejszczak - * @author Mark Paluch - */ -class ContextualValueExpressionEvaluator implements ValueExpressionEvaluator { - - private final ValueExpressionParser parser; - - public ContextualValueExpressionEvaluator(ValueExpressionParser parser, ValueEvaluationContext evaluationContext) { - this.parser = parser; - this.evaluationContext = evaluationContext; - } - - private final ValueEvaluationContext evaluationContext; - - @SuppressWarnings("unchecked") - @Override - public T evaluate(String expressionString) { - ValueExpression expression = parser.parse(expressionString); - return (T) expression.evaluate(evaluationContext); - } -} diff --git a/src/main/java/org/springframework/data/ldap/repository/query/LdapParameterAccessor.java b/src/main/java/org/springframework/data/ldap/repository/query/LdapParameterAccessor.java index 39c088d4..43a3138f 100644 --- a/src/main/java/org/springframework/data/ldap/repository/query/LdapParameterAccessor.java +++ b/src/main/java/org/springframework/data/ldap/repository/query/LdapParameterAccessor.java @@ -23,7 +23,7 @@ * @author Mark Paluch * @since 2.6 */ -interface LdapParameterAccessor extends ParameterAccessor { +public interface LdapParameterAccessor extends ParameterAccessor { /** * Returns the bindable parameter values of the underlying query method. diff --git a/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java b/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java index 7e8aee30..18823a5c 100644 --- a/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java +++ b/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java @@ -15,20 +15,27 @@ */ package org.springframework.data.ldap.repository.query; +import static org.springframework.data.ldap.repository.query.StringBasedQuery.BindingContext.*; + import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeMap; +import java.util.function.Function; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.springframework.data.ldap.repository.query.BindingContext.ParameterBinding; -import org.springframework.data.mapping.model.ValueExpressionEvaluator; +import org.springframework.data.expression.ValueExpression; +import org.springframework.data.expression.ValueExpressionParser; +import org.springframework.data.repository.query.Parameter; +import org.springframework.data.repository.query.ParameterAccessor; import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.data.spel.ExpressionDependencies; import org.springframework.lang.Nullable; +import org.springframework.ldap.support.LdapEncoder; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -41,13 +48,8 @@ class StringBasedQuery { private final String query; - private final Parameters parameters; - - private final ValueExpressionDelegate expressionParser; - private final List queryParameterBindings = new ArrayList<>(); - private final ExpressionDependencies expressionDependencies; /** @@ -57,12 +59,11 @@ class StringBasedQuery { * @param parameters must not be {@literal null}. * @param expressionParser must not be {@literal null}. */ - StringBasedQuery(String query, Parameters parameters, ValueExpressionDelegate expressionParser) { + public StringBasedQuery(String query, Parameters parameters, ValueExpressionDelegate expressionParser) { - this.query = ParameterBindingParser.INSTANCE.parseAndCollectParameterBindingsFromQueryIntoBindings(query, - this.queryParameterBindings); + this.query = ParameterBindingParser.parseAndCollectParameterBindingsFromQueryIntoBindings(query, + this.queryParameterBindings, expressionParser); this.parameters = parameters; - this.expressionParser = expressionParser; this.expressionDependencies = createExpressionDependencies(); } @@ -77,7 +78,7 @@ private ExpressionDependencies createExpressionDependencies() { for (ParameterBinding binding : queryParameterBindings) { if (binding.isExpression()) { dependencies - .add(expressionParser.parse(binding.getRequiredExpression()).getExpressionDependencies()); + .add(binding.getRequiredExpression().getExpressionDependencies()); } } @@ -100,17 +101,17 @@ public ExpressionDependencies getExpressionDependencies() { * @param evaluator must not be {@literal null}. * @return the bound String query containing formatted parameters. */ - String bindQuery(LdapParameterAccessor parameterAccessor, ValueExpressionEvaluator evaluator) { + public String bindQuery(LdapParameterAccessor parameterAccessor, Function evaluator) { Assert.notNull(parameterAccessor, "LdapParameterAccessor must not be null"); - Assert.notNull(evaluator, "SpELExpressionEvaluator must not be null"); + Assert.notNull(evaluator, "ExpressionEvaluator must not be null"); BindingContext bindingContext = new BindingContext(this.parameters, parameterAccessor, this.queryParameterBindings, evaluator); List arguments = bindingContext.getBindingValues(); - return ParameterBinder.INSTANCE.bind(this.query, arguments); + return ParameterBinder.bind(this.query, arguments); } /** @@ -118,54 +119,7 @@ String bindQuery(LdapParameterAccessor parameterAccessor, ValueExpressionEvaluat * * @author Mark Paluch */ - enum ParameterBinder { - - INSTANCE; - - private static final String ARGUMENT_PLACEHOLDER = "?_param_?"; - private static final Pattern ARGUMENT_PLACEHOLDER_PATTERN = Pattern.compile(Pattern.quote(ARGUMENT_PLACEHOLDER)); - - public String bind(String input, List parameters) { - - if (parameters.isEmpty()) { - return input; - } - - StringBuilder result = new StringBuilder(); - - int startIndex = 0; - int currentPosition = 0; - int parameterIndex = 0; - - Matcher matcher = ARGUMENT_PLACEHOLDER_PATTERN.matcher(input); - - while (currentPosition < input.length()) { - - if (!matcher.find()) { - break; - } - - int exprStart = matcher.start(); - - result.append(input.subSequence(startIndex, exprStart)).append(parameters.get(parameterIndex)); - - parameterIndex++; - currentPosition = matcher.end(); - startIndex = currentPosition; - } - - return result.append(input.subSequence(currentPosition, input.length())).toString(); - } - } - - /** - * A parser that extracts the parameter bindings from a given query string. - * - * @author Mark Paluch - */ - enum ParameterBindingParser { - - INSTANCE; + static class ParameterBindingParser { private static final char CURRLY_BRACE_OPEN = '{'; private static final char CURRLY_BRACE_CLOSE = '}'; @@ -177,7 +131,9 @@ enum ParameterBindingParser { private static final Pattern INDEX_BASED_PROPERTY_PLACEHOLDER_PATTERN = Pattern.compile("\\?\\$\\{"); private static final Pattern NAME_BASED_PROPERTY_PLACEHOLDER_PATTERN = Pattern.compile("\\:\\$\\{"); - private static final Set VALUE_EXPRESSION_PATTERNS = Set.of(INDEX_BASED_EXPRESSION_PATTERN, NAME_BASED_EXPRESSION_PATTERN, INDEX_BASED_PROPERTY_PLACEHOLDER_PATTERN, NAME_BASED_PROPERTY_PLACEHOLDER_PATTERN); + private static final Set VALUE_EXPRESSION_PATTERNS = Set.of(INDEX_BASED_EXPRESSION_PATTERN, + NAME_BASED_EXPRESSION_PATTERN, INDEX_BASED_PROPERTY_PLACEHOLDER_PATTERN, + NAME_BASED_PROPERTY_PLACEHOLDER_PATTERN); private static final String ARGUMENT_PLACEHOLDER = "?_param_?"; @@ -186,9 +142,11 @@ enum ParameterBindingParser { * * @param input can be {@literal null} or empty. * @param bindings must not be {@literal null}. + * @param expressionParser must not be {@literal null}. * @return a list of {@link ParameterBinding}s found in the given {@code input}. */ - public String parseAndCollectParameterBindingsFromQueryIntoBindings(String input, List bindings) { + public static String parseAndCollectParameterBindingsFromQueryIntoBindings(String input, + List bindings, ValueExpressionParser expressionParser) { if (!StringUtils.hasText(input)) { return input; @@ -196,11 +154,11 @@ public String parseAndCollectParameterBindingsFromQueryIntoBindings(String input Assert.notNull(bindings, "Parameter bindings must not be null"); - return transformQueryAndCollectExpressionParametersIntoBindings(input, bindings); + return transformQueryAndCollectExpressionParametersIntoBindings(input, bindings, expressionParser); } private static String transformQueryAndCollectExpressionParametersIntoBindings(String input, - List bindings) { + List bindings, ValueExpressionParser expressionParser) { StringBuilder result = new StringBuilder(); @@ -243,13 +201,11 @@ private static String transformQueryAndCollectExpressionParametersIntoBindings(S result.append(ARGUMENT_PLACEHOLDER); if (isValueExpression(matcher)) { - bindings.add( - ParameterBinding - .expression(input.substring(exprStart + 1, currentPosition), true)); + bindings.add(ParameterBinding + .expression(expressionParser.parse(input.substring(exprStart + 1, currentPosition)), true)); } else { if (matcher.pattern() == INDEX_PARAMETER_BINDING_PATTERN) { - bindings - .add(ParameterBinding.indexed(Integer.parseInt(matcher.group(1)))); + bindings.add(ParameterBinding.indexed(Integer.parseInt(matcher.group(1)))); } else { bindings.add(ParameterBinding.named(matcher.group(1))); } @@ -290,4 +246,191 @@ private static Matcher findNextBindingOrExpression(String input, int startPositi return (matcherMap.isEmpty() ? null : matcherMap.values().iterator().next()); } } + + /** + * A parser that extracts the parameter bindings from a given query string. + * + * @author Mark Paluch + */ + static class ParameterBinder { + + + private static final String ARGUMENT_PLACEHOLDER = "?_param_?"; + private static final Pattern ARGUMENT_PLACEHOLDER_PATTERN = Pattern.compile(Pattern.quote(ARGUMENT_PLACEHOLDER)); + + public static String bind(String input, List parameters) { + + if (parameters.isEmpty()) { + return input; + } + + StringBuilder result = new StringBuilder(); + + int startIndex = 0; + int currentPosition = 0; + int parameterIndex = 0; + + Matcher matcher = ARGUMENT_PLACEHOLDER_PATTERN.matcher(input); + + while (currentPosition < input.length()) { + + if (!matcher.find()) { + break; + } + + int exprStart = matcher.start(); + + result.append(input.subSequence(startIndex, exprStart)).append(parameters.get(parameterIndex)); + + parameterIndex++; + currentPosition = matcher.end(); + startIndex = currentPosition; + } + + return result.append(input.subSequence(currentPosition, input.length())).toString(); + } + } + + /** + * Value object capturing the binding context to provide {@link #getBindingValues() binding values} for queries. + * + * @author Mark Paluch + */ + static class BindingContext { + + private final Parameters parameters; + private final ParameterAccessor parameterAccessor; + private final List bindings; + private final Function evaluator; + + /** + * Create new {@link BindingContext}. + */ + BindingContext(Parameters parameters, ParameterAccessor parameterAccessor, List bindings, + Function evaluator) { + + this.parameters = parameters; + this.parameterAccessor = parameterAccessor; + this.bindings = bindings; + this.evaluator = evaluator; + } + + /** + * @return {@literal true} when list of bindings is not empty. + */ + private boolean hasBindings() { + return !bindings.isEmpty(); + } + + /** + * Bind values provided by {@link LdapParameterAccessor} to placeholders in {@link BindingContext} while considering + * potential conversions and parameter types. + * + * @return {@literal null} if given {@code raw} value is empty. + */ + public List getBindingValues() { + + if (!hasBindings()) { + return Collections.emptyList(); + } + + List parameters = new ArrayList<>(bindings.size()); + + for (ParameterBinding binding : bindings) { + Object parameterValueForBinding = getParameterValueForBinding(binding); + parameters.add(parameterValueForBinding); + } + + return parameters; + } + + /** + * Return the value to be used for the given {@link ParameterBinding}. + * + * @param binding must not be {@literal null}. + * @return the value used for the given {@link ParameterBinding}. + */ + @Nullable + private Object getParameterValueForBinding(ParameterBinding binding) { + + if (binding.isExpression()) { + return evaluator.apply(binding.getRequiredExpression()); + } + + Object value = binding.isNamed() + ? parameterAccessor.getBindableValue(getParameterIndex(parameters, binding.getRequiredParameterName())) + : parameterAccessor.getBindableValue(binding.getParameterIndex()); + return value == null ? null : LdapEncoder.filterEncode(value.toString()); + } + + private int getParameterIndex(Parameters parameters, String parameterName) { + + for (Parameter parameter : parameters) { + + if (parameter.getName().filter(it -> it.equals(parameterName)).isPresent()) { + return parameter.getIndex(); + } + } + + throw new IllegalArgumentException( + String.format("Invalid parameter name; Cannot resolve parameter [%s]", parameterName)); + } + + /** + * A generic parameter binding with name or position information. + * + * @author Mark Paluch + */ + static class ParameterBinding { + + private final int parameterIndex; + private final @Nullable ValueExpression expression; + private final @Nullable String parameterName; + + private ParameterBinding(int parameterIndex, @Nullable ValueExpression expression, + @Nullable String parameterName) { + + this.parameterIndex = parameterIndex; + this.expression = expression; + this.parameterName = parameterName; + } + + static ParameterBinding expression(ValueExpression expression, boolean quoted) { + return new ParameterBinding(-1, expression, null); + } + + static ParameterBinding indexed(int parameterIndex) { + return new ParameterBinding(parameterIndex, null, null); + } + + static ParameterBinding named(String name) { + return new ParameterBinding(-1, null, name); + } + + boolean isNamed() { + return (parameterName != null); + } + + int getParameterIndex() { + return parameterIndex; + } + + ValueExpression getRequiredExpression() { + + Assert.state(expression != null, "ParameterBinding is not an expression"); + return expression; + } + + boolean isExpression() { + return (this.expression != null); + } + + String getRequiredParameterName() { + + Assert.state(parameterName != null, "ParameterBinding is not named"); + + return parameterName; + } + } + } } diff --git a/src/test/java/org/springframework/data/ldap/config/DummyLdapRepository.java b/src/test/java/org/springframework/data/ldap/config/DummyLdapRepository.java index 47cea0a7..85979faa 100644 --- a/src/test/java/org/springframework/data/ldap/config/DummyLdapRepository.java +++ b/src/test/java/org/springframework/data/ldap/config/DummyLdapRepository.java @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.ldap.config; import javax.naming.Name; diff --git a/src/test/java/org/springframework/data/ldap/config/EmbeddedLdapProperties.java b/src/test/java/org/springframework/data/ldap/config/EmbeddedLdapProperties.java index f9ad3aea..16caefb3 100644 --- a/src/test/java/org/springframework/data/ldap/config/EmbeddedLdapProperties.java +++ b/src/test/java/org/springframework/data/ldap/config/EmbeddedLdapProperties.java @@ -1,3 +1,18 @@ +/* + * Copyright 2024 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 + * + * https://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.ldap.config; import java.util.ArrayList; diff --git a/src/test/java/org/springframework/data/ldap/config/InMemoryLdapConfiguration.java b/src/test/java/org/springframework/data/ldap/config/InMemoryLdapConfiguration.java index 9db2450f..e05e1074 100644 --- a/src/test/java/org/springframework/data/ldap/config/InMemoryLdapConfiguration.java +++ b/src/test/java/org/springframework/data/ldap/config/InMemoryLdapConfiguration.java @@ -74,11 +74,9 @@ public InMemoryDirectoryServer directoryServer(ApplicationContext applicationCon return this.server; } - @Bean @DependsOn("directoryServer") - LdapContextSource ldapContextSource(Environment environment, EmbeddedLdapProperties properties, - EmbeddedLdapProperties embeddedProperties) { + LdapContextSource ldapContextSource(Environment environment, EmbeddedLdapProperties properties) { LdapContextSource source = new LdapContextSource(); Assert.notEmpty(properties.getBaseDn(), "Base DN must be set with at least one value"); source.setBase(properties.getBaseDn().get(0)); diff --git a/src/test/java/org/springframework/data/ldap/repository/query/ValueExpressionLdapRepositoryQueryTests.java b/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryIntegrationTests.java similarity index 61% rename from src/test/java/org/springframework/data/ldap/repository/query/ValueExpressionLdapRepositoryQueryTests.java rename to src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryIntegrationTests.java index 291103ee..191f2c1b 100644 --- a/src/test/java/org/springframework/data/ldap/repository/query/ValueExpressionLdapRepositoryQueryTests.java +++ b/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryIntegrationTests.java @@ -31,7 +31,6 @@ import org.springframework.data.ldap.repository.LdapRepository; import org.springframework.data.ldap.repository.Query; import org.springframework.data.ldap.repository.config.EnableLdapRepositories; -import org.springframework.data.repository.query.Param; import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; @@ -41,79 +40,47 @@ * @author Marcin Grzejszczak */ @SpringJUnitConfig -@TestPropertySource(properties = { "full.name=John Doe", "dc.name=memorynotfound" }) -class ValueExpressionLdapRepositoryQueryTests { +@TestPropertySource(properties = { "full.name=John Doe" }) +class AnnotatedLdapRepositoryQueryIntegrationTests { - @Autowired private QueryRepository queryRepository; + @Autowired QueryRepository queryRepository; - @Test - void shouldWorkWithNamedParameters() { + @Test // GH-453 + void filterWithNamedParameters() { - List objects = queryRepository.namedParameters("John Doe", "Bar"); + List objects = queryRepository.namedParameters("John Doe"); assertThatReturnedObjectIsJohnDoe(objects); } - @Test - void usingQueryLanguageCharsShouldNotFail() { + @Test // GH-453 + void filterWithPositionalParameters() { - List objects = queryRepository.namedParameters("John)(cn=Doe)", "Bar"); - - assertThat(objects).isEmpty(); - } - - @Test - void shouldWorkWithIndexParameters() { - - List objects = queryRepository.indexedParameters("John Doe", "Bar"); + List objects = queryRepository.indexedParameters("John Doe"); assertThatReturnedObjectIsJohnDoe(objects); } - @Test - void shouldWorkWithSpelExpressions() { + @Test // GH-453 + void filterWithSpelExpression() { List objects = queryRepository.spelParameters(); assertThatReturnedObjectIsJohnDoe(objects); } - @Test - void shouldWorkWithPropertyPlaceholders() { + @Test // GH-453 + void filterWithPropertyPlaceholder() { List objects = queryRepository.propertyPlaceholderParameters(); assertThatReturnedObjectIsJohnDoe(objects); } - @Test - void shouldWorkWithNamedParametersForBase() { - - List objects = queryRepository.baseNamedParameters("John Doe", "dc=memorynotfound"); - - assertThatReturnedObjectIsJohnDoe(objects); - } - - @Test - void shouldWorkWithIndexParametersForBase() { - - List objects = queryRepository.baseIndexedParameters("John Doe", "memorynotfound"); - - assertThatReturnedObjectIsJohnDoe(objects); - } - - @Test - void shouldWorkWithSpelExpressionsForBase() { + @Test // GH-453 + void baseWithNamedParameters() { - List objects = queryRepository.baseSpelParameters(); - - assertThatReturnedObjectIsJohnDoe(objects); - } - - @Test - void shouldWorkWithPropertyPlaceholdersForBase() { - - List objects = queryRepository.basePropertyPlaceholderParameters(); + List objects = queryRepository.baseNamedParameters("John Doe", "memorynotfound"); assertThatReturnedObjectIsJohnDoe(objects); } @@ -141,10 +108,10 @@ EmbeddedLdapProperties embeddedLdapProperties() { interface QueryRepository extends LdapRepository { @Query(value = "(cn=:fullName)") - List namedParameters(@Param("fullName") String fullName, @Param("lastName") String lastName); + List namedParameters(String fullName); @Query(value = "(cn=?0)") - List indexedParameters(String fullName, String lastName); + List indexedParameters(String fullName); @Query(value = "(cn=:#{'John ' + 'Doe'})") List spelParameters(); @@ -152,16 +119,7 @@ interface QueryRepository extends LdapRepository { @Query(value = "(cn=?${full.name})") List propertyPlaceholderParameters(); - @Query(base = ":dc", value = "(cn=:fullName)") - List baseNamedParameters(@Param("fullName") String fullName, @Param("dc") String dc); - - @Query(base = "dc=?1", value = "(cn=?0)") - List baseIndexedParameters(String fullName, String dc); - - @Query(base = "dc=:#{'memory' + 'notfound'}", value = "(cn=:#{'John ' + 'Doe'})") - List baseSpelParameters(); - - @Query(base = "dc=?${dc.name}", value = "(cn=?${full.name})") - List basePropertyPlaceholderParameters(); + @Query(base = "dc=:dc", value = "(cn=:fullName)") + List baseNamedParameters(String fullName, String dc); } } diff --git a/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryTests.java b/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java similarity index 68% rename from src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryTests.java rename to src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java index d9fc9c5d..a680ef8a 100644 --- a/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryTests.java +++ b/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 the original author or authors. + * Copyright 2024 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. @@ -15,6 +15,8 @@ */ package org.springframework.data.ldap.repository.query; +import static org.assertj.core.api.Assertions.*; + import java.util.List; import org.junit.jupiter.api.Test; @@ -26,14 +28,17 @@ import org.springframework.data.mapping.model.EntityInstantiators; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; -import org.springframework.data.repository.query.Param; import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.ldap.core.LdapOperations; import org.springframework.ldap.query.LdapQuery; -import static org.assertj.core.api.Assertions.assertThat; - -class AnnotatedLdapRepositoryQueryTests { +/** + * Unit tests for {@link AnnotatedLdapRepositoryQuery} + * + * @author Marcin Grzejszczak + * @author Mark Paluch + */ +class AnnotatedLdapRepositoryQueryUnitTests { LdapOperations ldapOperations = Mockito.mock(); @@ -41,18 +46,31 @@ class AnnotatedLdapRepositoryQueryTests { @Test void shouldEncodeQuery() throws NoSuchMethodException { - LdapQueryMethod method = queryMethod("namedParameters"); + + LdapQueryMethod method = queryMethod("namedParameters", String.class); AnnotatedLdapRepositoryQuery query = repositoryQuery(method); LdapQuery ldapQuery = query.createQuery( - new LdapParametersParameterAccessor(method, new Object[] { "John)(cn=Doe)", "foo" })); + new LdapParametersParameterAccessor(method, new Object[] { "John)(cn=Doe)" })); assertThat(ldapQuery.filter().encode()).isEqualTo("(cn=John\\29\\28cn=Doe\\29)"); } + @Test + void messageFormatParametersShouldWork() throws NoSuchMethodException { + + LdapQueryMethod method = queryMethod("messageFormatParameters", String.class); + AnnotatedLdapRepositoryQuery query = repositoryQuery(method); + + LdapQuery ldapQuery = query.createQuery(new LdapParametersParameterAccessor(method, new Object[] { "John" })); + + assertThat(ldapQuery.filter().encode()).isEqualTo("(cn=John)"); + } + @Test void shouldEncodeBase() throws NoSuchMethodException { - LdapQueryMethod method = queryMethod("baseNamedParameters"); + + LdapQueryMethod method = queryMethod("baseNamedParameters", String.class, String.class); AnnotatedLdapRepositoryQuery query = repositoryQuery(method); LdapQuery ldapQuery = query.createQuery( @@ -61,8 +79,8 @@ void shouldEncodeBase() throws NoSuchMethodException { assertThat(ldapQuery.base()).hasToString("cn=John\\29"); } - private LdapQueryMethod queryMethod(String methodName) throws NoSuchMethodException { - return new LdapQueryMethod(QueryRepository.class.getMethod(methodName, String.class, String.class), + private LdapQueryMethod queryMethod(String methodName, Class... parameterTypes) throws NoSuchMethodException { + return new LdapQueryMethod(QueryRepository.class.getMethod(methodName, parameterTypes), new DefaultRepositoryMetadata(QueryRepository.class), new SpelAwareProxyProjectionFactory()); } @@ -74,10 +92,13 @@ private AnnotatedLdapRepositoryQuery repositoryQuery(LdapQueryMethod method) { interface QueryRepository extends LdapRepository { @Query(value = "(cn=:fullName)") - List namedParameters(@Param("fullName") String fullName, @Param("lastName") String lastName); + List namedParameters(String fullName); + + @Query(value = "(cn={0})") + List messageFormatParameters(String fullName); @Query(base = ":dc", value = "(cn=:fullName)") - List baseNamedParameters(@Param("fullName") String fullName, @Param("dc") String dc); + List baseNamedParameters(String fullName, String dc); } -} \ No newline at end of file +} diff --git a/src/test/java/org/springframework/data/ldap/repository/query/SchemaEntry.java b/src/test/java/org/springframework/data/ldap/repository/query/SchemaEntry.java index aa3aca7f..614e323b 100644 --- a/src/test/java/org/springframework/data/ldap/repository/query/SchemaEntry.java +++ b/src/test/java/org/springframework/data/ldap/repository/query/SchemaEntry.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 the original author or authors. + * Copyright 2024 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. @@ -13,13 +13,11 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.ldap.repository.query; import javax.naming.Name; import org.springframework.ldap.odm.annotations.Attribute; -import org.springframework.ldap.odm.annotations.DnAttribute; import org.springframework.ldap.odm.annotations.Entry; import org.springframework.ldap.odm.annotations.Id; @@ -27,8 +25,8 @@ * @author Marcin Grzejszczak */ @Entry(objectClasses = { "inetOrgPerson", "organizationalPerson", "person", "top" }) - public class SchemaEntry { + @Id Name dn; From b2c3717bba365d268c6b4c9fbd1df614947b9379 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 23 Oct 2024 11:37:05 +0200 Subject: [PATCH 2/5] Preparing to work on the issue --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index cd39cda9..96617601 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-ldap - 3.5.0-SNAPSHOT + 3.5.0-GH-509-SNAPSHOT Spring Data LDAP Spring Data integration for LDAP From 58b2d60fb26a756ec2cd5e7c5d6350a7e52cb0b8 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 23 Oct 2024 19:36:57 +0200 Subject: [PATCH 3/5] Adds LdapParam and LdapEncoder fixes gh-509 --- .../data/ldap/repository/LdapEncoder.java | 45 ++++++++ .../data/ldap/repository/LdapParam.java | 47 ++++++++ .../ldap/repository/query/LdapParameters.java | 105 ++++++++++++++++++ .../repository/query/LdapQueryMethod.java | 11 +- .../repository/query/StringBasedQuery.java | 25 +++-- ...AnnotatedLdapRepositoryQueryUnitTests.java | 29 ++++- 6 files changed, 251 insertions(+), 11 deletions(-) create mode 100644 src/main/java/org/springframework/data/ldap/repository/LdapEncoder.java create mode 100644 src/main/java/org/springframework/data/ldap/repository/LdapParam.java create mode 100644 src/main/java/org/springframework/data/ldap/repository/query/LdapParameters.java diff --git a/src/main/java/org/springframework/data/ldap/repository/LdapEncoder.java b/src/main/java/org/springframework/data/ldap/repository/LdapEncoder.java new file mode 100644 index 00000000..e7c8568f --- /dev/null +++ b/src/main/java/org/springframework/data/ldap/repository/LdapEncoder.java @@ -0,0 +1,45 @@ +/* + * Copyright 2024 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 + * + * https://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.ldap.repository; + +/** + * Allows plugging in custom encoding for {@link LdapParam}. + * + * @author Marcin Grzejszczak + * @since 3.5.0 + */ +public interface LdapEncoder { + + /** + * Escape a value for use in a filter. + * @param value the value to escape. + * @return a properly escaped representation of the supplied value. + */ + String filterEncode(String value); + + /** + * Default implementation of {@link LdapEncoder} that uses + * {@link org.springframework.ldap.support.LdapEncoder}. + */ + class DefaultLdapEncoder implements LdapEncoder { + + @Override + public String filterEncode(String value) { + return org.springframework.ldap.support.LdapEncoder.filterEncode(value); + } + } + +} diff --git a/src/main/java/org/springframework/data/ldap/repository/LdapParam.java b/src/main/java/org/springframework/data/ldap/repository/LdapParam.java new file mode 100644 index 00000000..40cd50d8 --- /dev/null +++ b/src/main/java/org/springframework/data/ldap/repository/LdapParam.java @@ -0,0 +1,47 @@ +/* + * Copyright 2016-2024 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 + * + * https://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.ldap.repository; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import org.springframework.core.annotation.AliasFor; +import org.springframework.data.repository.query.Param; + +/** + * A {@link Param} alias that allows passing of custom {@link LdapEncoder}. + * + * @author Marcin Grzejszczak + * @since 3.5.0 + */ +@Target(ElementType.PARAMETER) +@Retention(RetentionPolicy.RUNTIME) +@Documented +public @interface LdapParam { + + @AliasFor(annotation = Param.class) + String value(); + + /** + * {@link LdapEncoder} to instantiate to encode query parameters. + * + * @return {@link LdapEncoder} class + */ + Class encoder() default LdapEncoder.DefaultLdapEncoder.class; +} diff --git a/src/main/java/org/springframework/data/ldap/repository/query/LdapParameters.java b/src/main/java/org/springframework/data/ldap/repository/query/LdapParameters.java new file mode 100644 index 00000000..4c752575 --- /dev/null +++ b/src/main/java/org/springframework/data/ldap/repository/query/LdapParameters.java @@ -0,0 +1,105 @@ +/* + * Copyright 2024 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 + * + * https://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.ldap.repository.query; + +import java.lang.reflect.Method; +import java.util.List; + +import org.springframework.core.MethodParameter; +import org.springframework.data.geo.Distance; +import org.springframework.data.ldap.repository.LdapEncoder; +import org.springframework.data.ldap.repository.LdapParam; +import org.springframework.data.repository.query.Parameter; +import org.springframework.data.repository.query.Parameters; +import org.springframework.data.repository.query.ParametersSource; +import org.springframework.data.util.TypeInformation; +import org.springframework.lang.Nullable; + +/** + * Custom extension of {@link Parameters} discovering additional + * + * @author Marcin Grzejszczak + * @since 3.5.0 + */ +public class LdapParameters extends Parameters { + + private final TypeInformation domainType; + + /** + * Creates a new {@link LdapParameters} instance from the given {@link Method} and {@link LdapQueryMethod}. + * + * @param parametersSource must not be {@literal null}. + */ + public LdapParameters(ParametersSource parametersSource) { + + super(parametersSource, methodParameter -> new LdapParameter(methodParameter, + parametersSource.getDomainTypeInformation())); + + this.domainType = parametersSource.getDomainTypeInformation(); + } + + private LdapParameters(List parameters, TypeInformation domainType) { + + super(parameters); + this.domainType = domainType; + } + + @Override + protected LdapParameters createFrom(List parameters) { + return new LdapParameters(parameters, this.domainType); + } + + @Nullable + LdapEncoder encoderForParameterWithName(String parameterName) { + for (LdapParameter parameter : this) { + if (parameterName.equals(parameter.getName().orElse(null))) { + LdapParam ldapParam = parameter.parameter.getParameterAnnotation(LdapParam.class); + if (ldapParam == null) { + return null; + } + Class encoder = ldapParam.encoder(); + try { + return encoder.getDeclaredConstructor().newInstance(); + } catch (Exception e) { + throw new IllegalStateException(e); + } + } + } + return null; + } + + /** + * Custom {@link Parameter} implementation adding parameters of type {@link Distance} to the special ones. + * + * @author Marcin Grzejszczak + */ + protected static class LdapParameter extends Parameter { + + private final MethodParameter parameter; + + /** + * Creates a new {@link LdapParameter}. + * + * @param parameter must not be {@literal null}. + * @param domainType must not be {@literal null}. + */ + LdapParameter(MethodParameter parameter, TypeInformation domainType) { + super(parameter, domainType); + this.parameter = parameter; + } + } + +} diff --git a/src/main/java/org/springframework/data/ldap/repository/query/LdapQueryMethod.java b/src/main/java/org/springframework/data/ldap/repository/query/LdapQueryMethod.java index e405e9c6..16313d32 100644 --- a/src/main/java/org/springframework/data/ldap/repository/query/LdapQueryMethod.java +++ b/src/main/java/org/springframework/data/ldap/repository/query/LdapQueryMethod.java @@ -21,7 +21,6 @@ import org.springframework.data.ldap.repository.Query; import org.springframework.data.projection.ProjectionFactory; import org.springframework.data.repository.core.RepositoryMetadata; -import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.ParametersSource; import org.springframework.data.repository.query.QueryMethod; import org.springframework.lang.Nullable; @@ -50,6 +49,16 @@ public LdapQueryMethod(Method method, RepositoryMetadata metadata, ProjectionFac this.method = method; } + @Override + protected LdapParameters createParameters(ParametersSource parametersSource) { + return new LdapParameters(parametersSource); + } + + @Override + public LdapParameters getParameters() { + return (LdapParameters) super.getParameters(); + } + /** * Check whether the target method is annotated with {@link org.springframework.data.ldap.repository.Query}. * diff --git a/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java b/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java index 18823a5c..cc775a80 100644 --- a/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java +++ b/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java @@ -15,8 +15,6 @@ */ package org.springframework.data.ldap.repository.query; -import static org.springframework.data.ldap.repository.query.StringBasedQuery.BindingContext.*; - import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -39,6 +37,8 @@ import org.springframework.util.Assert; import org.springframework.util.StringUtils; +import static org.springframework.data.ldap.repository.query.StringBasedQuery.BindingContext.ParameterBinding; + /** * String-based Query abstracting a query with parameter bindings. * @@ -48,7 +48,7 @@ class StringBasedQuery { private final String query; - private final Parameters parameters; + private final LdapParameters parameters; private final List queryParameterBindings = new ArrayList<>(); private final ExpressionDependencies expressionDependencies; @@ -59,7 +59,7 @@ class StringBasedQuery { * @param parameters must not be {@literal null}. * @param expressionParser must not be {@literal null}. */ - public StringBasedQuery(String query, Parameters parameters, ValueExpressionDelegate expressionParser) { + public StringBasedQuery(String query, LdapParameters parameters, ValueExpressionDelegate expressionParser) { this.query = ParameterBindingParser.parseAndCollectParameterBindingsFromQueryIntoBindings(query, this.queryParameterBindings, expressionParser); @@ -298,7 +298,7 @@ public static String bind(String input, List parameters) { */ static class BindingContext { - private final Parameters parameters; + private final LdapParameters parameters; private final ParameterAccessor parameterAccessor; private final List bindings; private final Function evaluator; @@ -306,7 +306,7 @@ static class BindingContext { /** * Create new {@link BindingContext}. */ - BindingContext(Parameters parameters, ParameterAccessor parameterAccessor, List bindings, + BindingContext(LdapParameters parameters, ParameterAccessor parameterAccessor, List bindings, Function evaluator) { this.parameters = parameters; @@ -356,11 +356,20 @@ private Object getParameterValueForBinding(ParameterBinding binding) { if (binding.isExpression()) { return evaluator.apply(binding.getRequiredExpression()); } - Object value = binding.isNamed() ? parameterAccessor.getBindableValue(getParameterIndex(parameters, binding.getRequiredParameterName())) : parameterAccessor.getBindableValue(binding.getParameterIndex()); - return value == null ? null : LdapEncoder.filterEncode(value.toString()); + + if (value == null) { + return null; + } + + org.springframework.data.ldap.repository.LdapEncoder customLdapEncoder = parameters.encoderForParameterWithName( + binding.getRequiredParameterName()); + if (customLdapEncoder != null) { + return customLdapEncoder.filterEncode(value.toString()); + } + return LdapEncoder.filterEncode(value.toString()); } private int getParameterIndex(Parameters parameters, String parameterName) { diff --git a/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java b/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java index a680ef8a..53c408ff 100644 --- a/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java +++ b/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java @@ -15,14 +15,14 @@ */ package org.springframework.data.ldap.repository.query; -import static org.assertj.core.api.Assertions.*; - import java.util.List; import org.junit.jupiter.api.Test; import org.mockito.Mockito; import org.springframework.data.ldap.core.mapping.LdapMappingContext; +import org.springframework.data.ldap.repository.LdapEncoder; +import org.springframework.data.ldap.repository.LdapParam; import org.springframework.data.ldap.repository.LdapRepository; import org.springframework.data.ldap.repository.Query; import org.springframework.data.mapping.model.EntityInstantiators; @@ -32,6 +32,8 @@ import org.springframework.ldap.core.LdapOperations; import org.springframework.ldap.query.LdapQuery; +import static org.assertj.core.api.Assertions.assertThat; + /** * Unit tests for {@link AnnotatedLdapRepositoryQuery} * @@ -79,6 +81,18 @@ void shouldEncodeBase() throws NoSuchMethodException { assertThat(ldapQuery.base()).hasToString("cn=John\\29"); } + @Test + void shouldEncodeWithCustomEncoder() throws NoSuchMethodException { + + LdapQueryMethod method = queryMethod("customEncoder", String.class); + AnnotatedLdapRepositoryQuery query = repositoryQuery(method); + + LdapQuery ldapQuery = query.createQuery( + new LdapParametersParameterAccessor(method, new Object[] { "Doe" })); + + assertThat(ldapQuery.filter().encode()).isEqualTo("(cn=Doebar)"); + } + private LdapQueryMethod queryMethod(String methodName, Class... parameterTypes) throws NoSuchMethodException { return new LdapQueryMethod(QueryRepository.class.getMethod(methodName, parameterTypes), new DefaultRepositoryMetadata(QueryRepository.class), new SpelAwareProxyProjectionFactory()); @@ -100,5 +114,16 @@ interface QueryRepository extends LdapRepository { @Query(base = ":dc", value = "(cn=:fullName)") List baseNamedParameters(String fullName, String dc); + @Query(value = "(cn=:fullName)") + List customEncoder(@LdapParam(value = "fullName", encoder = MyEncoder.class) String fullName); + + } + + static class MyEncoder implements LdapEncoder { + + @Override + public String filterEncode(String value) { + return value + "bar"; + } } } From 2c2593e2786e6893c45e92027f25c52a18fa66c2 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 24 Oct 2024 11:09:24 +0200 Subject: [PATCH 4/5] Moved out of LdapParam to LdapEncode with this change we're explicitly adding an option to add an encoder on a concrete method parameter. --- .../{LdapParam.java => LdapEncode.java} | 20 +++++---- .../data/ldap/repository/LdapEncoder.java | 15 +------ .../ldap/repository/query/LdapParameters.java | 25 +---------- .../repository/query/StringBasedQuery.java | 41 ++++++++++++++++--- ...AnnotatedLdapRepositoryQueryUnitTests.java | 4 +- 5 files changed, 53 insertions(+), 52 deletions(-) rename src/main/java/org/springframework/data/ldap/repository/{LdapParam.java => LdapEncode.java} (74%) diff --git a/src/main/java/org/springframework/data/ldap/repository/LdapParam.java b/src/main/java/org/springframework/data/ldap/repository/LdapEncode.java similarity index 74% rename from src/main/java/org/springframework/data/ldap/repository/LdapParam.java rename to src/main/java/org/springframework/data/ldap/repository/LdapEncode.java index 40cd50d8..b4a58378 100644 --- a/src/main/java/org/springframework/data/ldap/repository/LdapParam.java +++ b/src/main/java/org/springframework/data/ldap/repository/LdapEncode.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 the original author or authors. + * Copyright 2024 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. @@ -22,10 +22,9 @@ import java.lang.annotation.Target; import org.springframework.core.annotation.AliasFor; -import org.springframework.data.repository.query.Param; /** - * A {@link Param} alias that allows passing of custom {@link LdapEncoder}. + * Allows passing of custom {@link LdapEncoder}. * * @author Marcin Grzejszczak * @since 3.5.0 @@ -33,15 +32,22 @@ @Target(ElementType.PARAMETER) @Retention(RetentionPolicy.RUNTIME) @Documented -public @interface LdapParam { +public @interface LdapEncode { - @AliasFor(annotation = Param.class) - String value(); + /** + * {@link LdapEncoder} to instantiate to encode query parameters. + * + * @return {@link LdapEncoder} class + */ + @AliasFor("encoder") + Class value(); /** * {@link LdapEncoder} to instantiate to encode query parameters. * * @return {@link LdapEncoder} class */ - Class encoder() default LdapEncoder.DefaultLdapEncoder.class; + @AliasFor("value") + Class encoder() default LdapEncoder.class; + } diff --git a/src/main/java/org/springframework/data/ldap/repository/LdapEncoder.java b/src/main/java/org/springframework/data/ldap/repository/LdapEncoder.java index e7c8568f..6e491975 100644 --- a/src/main/java/org/springframework/data/ldap/repository/LdapEncoder.java +++ b/src/main/java/org/springframework/data/ldap/repository/LdapEncoder.java @@ -16,7 +16,7 @@ package org.springframework.data.ldap.repository; /** - * Allows plugging in custom encoding for {@link LdapParam}. + * Allows plugging in custom encoding for {@link LdapEncode}. * * @author Marcin Grzejszczak * @since 3.5.0 @@ -29,17 +29,4 @@ public interface LdapEncoder { * @return a properly escaped representation of the supplied value. */ String filterEncode(String value); - - /** - * Default implementation of {@link LdapEncoder} that uses - * {@link org.springframework.ldap.support.LdapEncoder}. - */ - class DefaultLdapEncoder implements LdapEncoder { - - @Override - public String filterEncode(String value) { - return org.springframework.ldap.support.LdapEncoder.filterEncode(value); - } - } - } diff --git a/src/main/java/org/springframework/data/ldap/repository/query/LdapParameters.java b/src/main/java/org/springframework/data/ldap/repository/query/LdapParameters.java index 4c752575..c5cedc6d 100644 --- a/src/main/java/org/springframework/data/ldap/repository/query/LdapParameters.java +++ b/src/main/java/org/springframework/data/ldap/repository/query/LdapParameters.java @@ -20,13 +20,10 @@ import org.springframework.core.MethodParameter; import org.springframework.data.geo.Distance; -import org.springframework.data.ldap.repository.LdapEncoder; -import org.springframework.data.ldap.repository.LdapParam; import org.springframework.data.repository.query.Parameter; import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.ParametersSource; import org.springframework.data.util.TypeInformation; -import org.springframework.lang.Nullable; /** * Custom extension of {@link Parameters} discovering additional @@ -62,33 +59,15 @@ protected LdapParameters createFrom(List parameters) { return new LdapParameters(parameters, this.domainType); } - @Nullable - LdapEncoder encoderForParameterWithName(String parameterName) { - for (LdapParameter parameter : this) { - if (parameterName.equals(parameter.getName().orElse(null))) { - LdapParam ldapParam = parameter.parameter.getParameterAnnotation(LdapParam.class); - if (ldapParam == null) { - return null; - } - Class encoder = ldapParam.encoder(); - try { - return encoder.getDeclaredConstructor().newInstance(); - } catch (Exception e) { - throw new IllegalStateException(e); - } - } - } - return null; - } /** * Custom {@link Parameter} implementation adding parameters of type {@link Distance} to the special ones. * * @author Marcin Grzejszczak */ - protected static class LdapParameter extends Parameter { + static class LdapParameter extends Parameter { - private final MethodParameter parameter; + final MethodParameter parameter; /** * Creates a new {@link LdapParameter}. diff --git a/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java b/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java index cc775a80..bb6cbf50 100644 --- a/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java +++ b/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java @@ -25,8 +25,10 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.springframework.beans.BeanUtils; import org.springframework.data.expression.ValueExpression; import org.springframework.data.expression.ValueExpressionParser; +import org.springframework.data.ldap.repository.LdapEncode; import org.springframework.data.repository.query.Parameter; import org.springframework.data.repository.query.ParameterAccessor; import org.springframework.data.repository.query.Parameters; @@ -364,12 +366,7 @@ private Object getParameterValueForBinding(ParameterBinding binding) { return null; } - org.springframework.data.ldap.repository.LdapEncoder customLdapEncoder = parameters.encoderForParameterWithName( - binding.getRequiredParameterName()); - if (customLdapEncoder != null) { - return customLdapEncoder.filterEncode(value.toString()); - } - return LdapEncoder.filterEncode(value.toString()); + return binding.getEncodedValue(parameters, value); } private int getParameterIndex(Parameters parameters, String parameterName) { @@ -416,6 +413,38 @@ static ParameterBinding named(String name) { return new ParameterBinding(-1, null, name); } + Object getEncodedValue(LdapParameters ldapParameters, Object value) { + org.springframework.data.ldap.repository.LdapEncoder encoder = encoderForParameter(ldapParameters); + if (encoder == null) { + return LdapEncoder.filterEncode(value.toString()); + } + return encoder.filterEncode(value.toString()); + } + + + @Nullable + org.springframework.data.ldap.repository.LdapEncoder encoderForParameter(LdapParameters ldapParameters) { + for (LdapParameters.LdapParameter parameter : ldapParameters) { + if (isByName(parameter) || isByIndex(parameter)) { + LdapEncode ldapEncode = parameter.parameter.getParameterAnnotation(LdapEncode.class); + if (ldapEncode == null) { + return null; + } + Class encoder = ldapEncode.value(); + return BeanUtils.instantiateClass(encoder); + } + } + return null; + } + + private boolean isByIndex(LdapParameters.LdapParameter parameter) { + return parameterIndex != -1 && parameter.getIndex() == parameterIndex; + } + + private boolean isByName(LdapParameters.LdapParameter parameter) { + return parameterName != null && parameterName.equals(parameter.getName().orElse(null)); + } + boolean isNamed() { return (parameterName != null); } diff --git a/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java b/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java index 53c408ff..3c1e9d91 100644 --- a/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java +++ b/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java @@ -22,7 +22,7 @@ import org.springframework.data.ldap.core.mapping.LdapMappingContext; import org.springframework.data.ldap.repository.LdapEncoder; -import org.springframework.data.ldap.repository.LdapParam; +import org.springframework.data.ldap.repository.LdapEncode; import org.springframework.data.ldap.repository.LdapRepository; import org.springframework.data.ldap.repository.Query; import org.springframework.data.mapping.model.EntityInstantiators; @@ -115,7 +115,7 @@ interface QueryRepository extends LdapRepository { List baseNamedParameters(String fullName, String dc); @Query(value = "(cn=:fullName)") - List customEncoder(@LdapParam(value = "fullName", encoder = MyEncoder.class) String fullName); + List customEncoder(@LdapEncode(MyEncoder.class) String fullName); } From 13f1095f20052e94fda84ad565f2c26e30b6dfa7 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 24 Oct 2024 15:13:10 +0200 Subject: [PATCH 5/5] Polishing. Refine Javadoc. Move LdapEncoder lookup into LdapParameters. Eagerly instantiate LdapEncoder. Refine method naming. --- .../data/ldap/repository/LdapEncode.java | 20 +++++-- .../data/ldap/repository/LdapEncoder.java | 20 ++++--- .../ldap/repository/query/LdapParameters.java | 32 +++++++++-- .../repository/query/StringBasedQuery.java | 53 +++++-------------- ...AnnotatedLdapRepositoryQueryUnitTests.java | 8 +-- 5 files changed, 74 insertions(+), 59 deletions(-) diff --git a/src/main/java/org/springframework/data/ldap/repository/LdapEncode.java b/src/main/java/org/springframework/data/ldap/repository/LdapEncode.java index b4a58378..2c01f399 100644 --- a/src/main/java/org/springframework/data/ldap/repository/LdapEncode.java +++ b/src/main/java/org/springframework/data/ldap/repository/LdapEncode.java @@ -24,18 +24,28 @@ import org.springframework.core.annotation.AliasFor; /** - * Allows passing of custom {@link LdapEncoder}. + * Annotation which indicates that a method parameter should be encoded using a specific {@link LdapEncoder} for a + * repository query method invocation. + *

+ * If no {@link LdapEncoder} is configured, method parameters are encoded using + * {@link org.springframework.ldap.support.LdapEncoder#filterEncode(String)}. The default encoder considers chars such + * as {@code *} (asterisk) to be encoded which might interfere with the intent of running a Like query. Since Spring + * Data LDAP doesn't parse queries it is up to you to decide which encoder to use. + *

+ * {@link LdapEncoder} implementations must declare a no-args constructor so they can be instantiated during repository + * initialization. * * @author Marcin Grzejszczak - * @since 3.5.0 + * @author Mark Paluch + * @since 3.5 */ -@Target(ElementType.PARAMETER) +@Target({ ElementType.PARAMETER, ElementType.ANNOTATION_TYPE }) @Retention(RetentionPolicy.RUNTIME) @Documented public @interface LdapEncode { /** - * {@link LdapEncoder} to instantiate to encode query parameters. + * {@link LdapEncoder} to encode query parameters. * * @return {@link LdapEncoder} class */ @@ -43,7 +53,7 @@ Class value(); /** - * {@link LdapEncoder} to instantiate to encode query parameters. + * {@link LdapEncoder} to encode query parameters. * * @return {@link LdapEncoder} class */ diff --git a/src/main/java/org/springframework/data/ldap/repository/LdapEncoder.java b/src/main/java/org/springframework/data/ldap/repository/LdapEncoder.java index 6e491975..02dbba1a 100644 --- a/src/main/java/org/springframework/data/ldap/repository/LdapEncoder.java +++ b/src/main/java/org/springframework/data/ldap/repository/LdapEncoder.java @@ -16,17 +16,25 @@ package org.springframework.data.ldap.repository; /** - * Allows plugging in custom encoding for {@link LdapEncode}. + * Strategy interface to escape values for use in LDAP filters. + *

+ * Accepts an LDAP filter value to be encoded (escaped) for String-based LDAP query usage as LDAP queries do not feature + * an out-of-band parameter binding mechanism. + *

+ * Make sure that your implementation escapes special characters in the value adequately to prevent injection attacks. * * @author Marcin Grzejszczak - * @since 3.5.0 + * @author Mark Paluch + * @since 3.5 */ public interface LdapEncoder { /** - * Escape a value for use in a filter. - * @param value the value to escape. - * @return a properly escaped representation of the supplied value. + * Encode a value for use in a filter. + * + * @param value the value to encode. + * @return a properly encoded representation of the supplied value. */ - String filterEncode(String value); + String encode(String value); + } diff --git a/src/main/java/org/springframework/data/ldap/repository/query/LdapParameters.java b/src/main/java/org/springframework/data/ldap/repository/query/LdapParameters.java index c5cedc6d..a33eb501 100644 --- a/src/main/java/org/springframework/data/ldap/repository/query/LdapParameters.java +++ b/src/main/java/org/springframework/data/ldap/repository/query/LdapParameters.java @@ -18,18 +18,22 @@ import java.lang.reflect.Method; import java.util.List; +import org.springframework.beans.BeanUtils; import org.springframework.core.MethodParameter; import org.springframework.data.geo.Distance; +import org.springframework.data.ldap.repository.LdapEncode; +import org.springframework.data.ldap.repository.LdapEncoder; import org.springframework.data.repository.query.Parameter; import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.ParametersSource; import org.springframework.data.util.TypeInformation; +import org.springframework.lang.Nullable; /** * Custom extension of {@link Parameters} discovering additional * * @author Marcin Grzejszczak - * @since 3.5.0 + * @since 3.5 */ public class LdapParameters extends Parameters { @@ -65,9 +69,10 @@ protected LdapParameters createFrom(List parameters) { * * @author Marcin Grzejszczak */ - static class LdapParameter extends Parameter { + protected static class LdapParameter extends Parameter { - final MethodParameter parameter; + private final @Nullable LdapEncoder ldapEncoder; + private final MethodParameter parameter; /** * Creates a new {@link LdapParameter}. @@ -76,8 +81,29 @@ static class LdapParameter extends Parameter { * @param domainType must not be {@literal null}. */ LdapParameter(MethodParameter parameter, TypeInformation domainType) { + super(parameter, domainType); this.parameter = parameter; + + LdapEncode encode = parameter.getParameterAnnotation(LdapEncode.class); + + if (encode != null) { + this.ldapEncoder = BeanUtils.instantiateClass(encode.value()); + } else { + this.ldapEncoder = null; + } + } + + public boolean hasLdapEncoder() { + return ldapEncoder != null; + } + + public LdapEncoder getLdapEncoder() { + + if (ldapEncoder == null) { + throw new IllegalStateException("No LdapEncoder found for parameter " + parameter); + } + return ldapEncoder; } } diff --git a/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java b/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java index bb6cbf50..3202699b 100644 --- a/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java +++ b/src/main/java/org/springframework/data/ldap/repository/query/StringBasedQuery.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2024 the original author or authors. + * Copyright 2024 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. @@ -15,6 +15,8 @@ */ package org.springframework.data.ldap.repository.query; +import static org.springframework.data.ldap.repository.query.StringBasedQuery.BindingContext.*; + import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -25,10 +27,8 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.springframework.beans.BeanUtils; import org.springframework.data.expression.ValueExpression; import org.springframework.data.expression.ValueExpressionParser; -import org.springframework.data.ldap.repository.LdapEncode; import org.springframework.data.repository.query.Parameter; import org.springframework.data.repository.query.ParameterAccessor; import org.springframework.data.repository.query.Parameters; @@ -39,12 +39,11 @@ import org.springframework.util.Assert; import org.springframework.util.StringUtils; -import static org.springframework.data.ldap.repository.query.StringBasedQuery.BindingContext.ParameterBinding; - /** * String-based Query abstracting a query with parameter bindings. * * @author Marcin Grzejszczak + * @author Mark Paluch * @since 3.4 */ class StringBasedQuery { @@ -358,15 +357,19 @@ private Object getParameterValueForBinding(ParameterBinding binding) { if (binding.isExpression()) { return evaluator.apply(binding.getRequiredExpression()); } - Object value = binding.isNamed() - ? parameterAccessor.getBindableValue(getParameterIndex(parameters, binding.getRequiredParameterName())) - : parameterAccessor.getBindableValue(binding.getParameterIndex()); + int index = binding.isNamed() ? getParameterIndex(parameters, binding.getRequiredParameterName()) + : binding.getParameterIndex(); + Object value = parameterAccessor.getBindableValue(index); if (value == null) { return null; } - return binding.getEncodedValue(parameters, value); + String toString = value.toString(); + LdapParameters.LdapParameter parameter = parameters.getBindableParameter(index); + + return parameter.hasLdapEncoder() ? parameter.getLdapEncoder().encode(toString) + : LdapEncoder.filterEncode(toString); } private int getParameterIndex(Parameters parameters, String parameterName) { @@ -413,38 +416,6 @@ static ParameterBinding named(String name) { return new ParameterBinding(-1, null, name); } - Object getEncodedValue(LdapParameters ldapParameters, Object value) { - org.springframework.data.ldap.repository.LdapEncoder encoder = encoderForParameter(ldapParameters); - if (encoder == null) { - return LdapEncoder.filterEncode(value.toString()); - } - return encoder.filterEncode(value.toString()); - } - - - @Nullable - org.springframework.data.ldap.repository.LdapEncoder encoderForParameter(LdapParameters ldapParameters) { - for (LdapParameters.LdapParameter parameter : ldapParameters) { - if (isByName(parameter) || isByIndex(parameter)) { - LdapEncode ldapEncode = parameter.parameter.getParameterAnnotation(LdapEncode.class); - if (ldapEncode == null) { - return null; - } - Class encoder = ldapEncode.value(); - return BeanUtils.instantiateClass(encoder); - } - } - return null; - } - - private boolean isByIndex(LdapParameters.LdapParameter parameter) { - return parameterIndex != -1 && parameter.getIndex() == parameterIndex; - } - - private boolean isByName(LdapParameters.LdapParameter parameter) { - return parameterName != null && parameterName.equals(parameter.getName().orElse(null)); - } - boolean isNamed() { return (parameterName != null); } diff --git a/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java b/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java index 3c1e9d91..64fff307 100644 --- a/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java +++ b/src/test/java/org/springframework/data/ldap/repository/query/AnnotatedLdapRepositoryQueryUnitTests.java @@ -15,14 +15,16 @@ */ package org.springframework.data.ldap.repository.query; +import static org.assertj.core.api.Assertions.*; + import java.util.List; import org.junit.jupiter.api.Test; import org.mockito.Mockito; import org.springframework.data.ldap.core.mapping.LdapMappingContext; -import org.springframework.data.ldap.repository.LdapEncoder; import org.springframework.data.ldap.repository.LdapEncode; +import org.springframework.data.ldap.repository.LdapEncoder; import org.springframework.data.ldap.repository.LdapRepository; import org.springframework.data.ldap.repository.Query; import org.springframework.data.mapping.model.EntityInstantiators; @@ -32,8 +34,6 @@ import org.springframework.ldap.core.LdapOperations; import org.springframework.ldap.query.LdapQuery; -import static org.assertj.core.api.Assertions.assertThat; - /** * Unit tests for {@link AnnotatedLdapRepositoryQuery} * @@ -122,7 +122,7 @@ interface QueryRepository extends LdapRepository { static class MyEncoder implements LdapEncoder { @Override - public String filterEncode(String value) { + public String encode(String value) { return value + "bar"; } }