From c08dd4bff0beb7eb1940dc14194bb1d22abfeb2f Mon Sep 17 00:00:00 2001 From: Peter-Josef Meisch Date: Sun, 2 Aug 2026 18:41:24 +0200 Subject: [PATCH] Consolidate use of ValueExpressions insead of Expressions in the SimpleElasticsearchPersitentEntity. Closes: #3321 Original Pull Request #3323 Signed-off-by: Peter-Josef Meisch --- .../MappingElasticsearchConverter.java | 3 +- .../SimpleElasticsearchPersistentEntity.java | 77 +++++++++++++------ .../utils/spel/ExpressionUtils.java | 10 ++- .../DefaultRoutingResolverUnitTest.java | 50 +++++++++++- 4 files changed, 109 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java b/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java index e40e2e35a..ed31eca99 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java @@ -104,9 +104,8 @@ public class MappingElasticsearchConverter protected @Nullable Environment environment; private final SpELContext spELContext = new SpELContext(new MapAccessor()); - private final SpelExpressionParser expressionParser = new SpelExpressionParser(); private final CachingValueExpressionEvaluatorFactory expressionEvaluatorFactory = new CachingValueExpressionEvaluatorFactory( - expressionParser, this, spELContext); + new SpelExpressionParser(), this, spELContext); private final EntityInstantiators instantiators = new EntityInstantiators(); private final ElasticsearchTypeMapper typeMapper; diff --git a/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntity.java b/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntity.java index f82b0c1ab..c3c3c5621 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntity.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntity.java @@ -47,13 +47,9 @@ import org.springframework.data.mapping.model.BasicPersistentEntity; import org.springframework.data.mapping.model.FieldNamingStrategy; import org.springframework.data.spel.ExpressionDependencies; import org.springframework.data.util.Lazy; -import org.springframework.expression.EvaluationContext; import org.springframework.expression.EvaluationException; -import org.springframework.expression.Expression; import org.springframework.expression.ExpressionException; -import org.springframework.expression.ParserContext; import org.springframework.expression.common.LiteralExpression; -import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; @@ -76,7 +72,6 @@ public class SimpleElasticsearchPersistentEntity extends BasicPersistentEntit implements ElasticsearchPersistentEntity { private static final Log LOGGER = LogFactory.getLog(SimpleElasticsearchPersistentEntity.class); - private static final SpelExpressionParser PARSER = new SpelExpressionParser(); private @Nullable final Document document; private final String unresolvedIndexName; @@ -89,13 +84,9 @@ public class SimpleElasticsearchPersistentEntity extends BasicPersistentEntit private final boolean alwaysWriteMapping; private final Dynamic dynamic; private final Map fieldNamePropertyCache = new ConcurrentHashMap<>(); - private final ConcurrentHashMap routingExpressions = new ConcurrentHashMap<>(); private @Nullable String routing; private final ContextConfiguration contextConfiguration; private final Set aliases = new HashSet<>(); - - private final Lazy indexNameEvaluationContext = Lazy.of(this::getIndexNameEvaluationContext); - private final boolean storeIdInSource; private final boolean storeVersionInSource; @@ -378,32 +369,53 @@ public class SimpleElasticsearchPersistentEntity extends BasicPersistentEntit ValueExpression expression = ExpressionUtils.detectExpression(name); - Object resolvedName = expression != null ? expression.evaluate(indexNameEvaluationContext.get()) : null; + Object resolvedName; + if (expression != null && !expression.isLiteral()) { + var valueEvaluationContext = getValueEvaluationContext(name); + resolvedName = expression.evaluate(valueEvaluationContext); + } else { + resolvedName = null; + } return resolvedName != null ? ObjectUtils.nullSafeToString(resolvedName) : name; } /** - * build the {@link EvaluationContext} considering {@link ExpressionDependencies} from the unresolvedIndexName. + * build the {@link ValueEvaluationContext} from the expression. * - * @return EvaluationContext + * @return ValueEvaluationContext + * @param expression */ - private ValueEvaluationContext getIndexNameEvaluationContext() { + private ValueEvaluationContext getValueEvaluationContext(@Nullable String expression) { + + ValueExpression valueExpression = ExpressionUtils.detectExpression(expression); + + return getValueEvaluationContext(valueExpression); + } + + /** + * build the {@link ValueEvaluationContext} considering {@link ExpressionDependencies} from the expression. + * + * @return ValueEvaluationContext + * @param expression + */ + private ValueEvaluationContext getValueEvaluationContext(@Nullable ValueExpression expression) { - ValueExpression expression = ExpressionUtils.detectExpression(unresolvedIndexName); var expressionDependencies = expression != null ? expression.getExpressionDependencies() : null; return expressionDependencies != null ? getValueEvaluationContext(null, expressionDependencies) - : getValueEvaluationContext(null); + : getValueEvaluationContext((Object) null); } @Override @Nullable public String resolveRouting(T bean) { + // we need a @Routing annotation on the entity if (routing == null) { return null; } + // if there is a property with name of of the @Routing annotation value, return the property's value ElasticsearchPersistentProperty persistentProperty = getPersistentProperty(routing); if (persistentProperty != null) { @@ -412,14 +424,29 @@ public class SimpleElasticsearchPersistentEntity extends BasicPersistentEntit return propertyValue != null ? propertyValue.toString() : null; } + // try to resolve the annotations value try { - Expression expression = routingExpressions.computeIfAbsent(routing, PARSER::parseExpression); - ExpressionDependencies expressionDependencies = ExpressionDependencies.discover(expression); + var routingValue = routing; + ValueExpression expression = ExpressionUtils.detectExpression(routingValue); - EvaluationContext context = getEvaluationContext(null, expressionDependencies); - context.setVariable("entity", bean); + if (expression == null) { + return null; + } - return expression.getValue(context, String.class); + if (expression.isLiteral()) { + // before using ValueExpressions, we had SpEL expressions without the "#{...}, + // so try if we get a valid expression after wrapping that + routingValue = "#{" + routing + "}"; + expression = ExpressionUtils.detectExpression(routingValue); + + if (expression == null) { + return null; + } + } + + ValueEvaluationContext context = getValueEvaluationContext(routingValue); + context.getEvaluationContext().setVariable("entity", bean); + return ObjectUtils.nullSafeToString(expression.evaluate(context)); } catch (EvaluationException e) { throw new InvalidDataAccessApiUsageException( "Could not resolve expression: " + routing + " for object of class " + bean.getClass().getCanonicalName(), e); @@ -437,9 +464,13 @@ public class SimpleElasticsearchPersistentEntity extends BasicPersistentEntit } try { - Expression expression = PARSER.parseExpression(settingPathFromParameter, ParserContext.TEMPLATE_EXPRESSION); - return (expression instanceof LiteralExpression) ? settingPathFromParameter - : expression.getValue(getEvaluationContext(null, ExpressionDependencies.discover(expression)), String.class); + ValueExpression expression = ExpressionUtils.detectExpression(settingPathFromParameter); + + if (expression == null || expression instanceof LiteralExpression) { + return settingPathFromParameter; + } + + return ObjectUtils.nullSafeToString(expression.evaluate(getValueEvaluationContext(settingPathFromParameter))); } catch (ExpressionException e) { throw new InvalidDataAccessApiUsageException( "Could not resolve expression: " + settingPathFromParameter + " for @Setting.settingPath ", e); diff --git a/src/main/java/org/springframework/data/elasticsearch/utils/spel/ExpressionUtils.java b/src/main/java/org/springframework/data/elasticsearch/utils/spel/ExpressionUtils.java index 2c2b1f550..5811d82a4 100644 --- a/src/main/java/org/springframework/data/elasticsearch/utils/spel/ExpressionUtils.java +++ b/src/main/java/org/springframework/data/elasticsearch/utils/spel/ExpressionUtils.java @@ -15,7 +15,6 @@ */ package org.springframework.data.elasticsearch.utils.spel; -import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; import org.jspecify.annotations.Nullable; @@ -24,6 +23,7 @@ import org.springframework.data.expression.ValueExpression; import org.springframework.data.expression.ValueExpressionParser; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.lang.Contract; +import org.springframework.util.ConcurrentLruCache; import org.springframework.util.StringUtils; /** @@ -31,13 +31,16 @@ import org.springframework.util.StringUtils; * Data MongoDB. Adapted afterwards to our needs. * * @author Christoph Strobl + * @author Peter-Josef Meisch * @since 6.2 */ public final class ExpressionUtils { private static final ValueExpressionParser PARSER = ValueExpressionParser.create(SpelExpressionParser::new); - private static final ConcurrentHashMap expressionCache = new ConcurrentHashMap<>(); + public static final int CAPACITY = 256; + private static final ConcurrentLruCache expressionCache = new ConcurrentLruCache<>( + CAPACITY, PARSER::parse); /** * Returns a SpEL {@link ValueExpression} if the given {@link String} is not empty. ValueExpressions are stored in a @@ -53,8 +56,7 @@ public final class ExpressionUtils { return null; } - return expressionCache.computeIfAbsent(potentialExpression, - key -> PARSER.parse(potentialExpression)); + return expressionCache.get(potentialExpression); } /** diff --git a/src/test/java/org/springframework/data/elasticsearch/core/routing/DefaultRoutingResolverUnitTest.java b/src/test/java/org/springframework/data/elasticsearch/core/routing/DefaultRoutingResolverUnitTest.java index 349f8a3f4..2998c25c9 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/routing/DefaultRoutingResolverUnitTest.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/routing/DefaultRoutingResolverUnitTest.java @@ -89,6 +89,17 @@ class DefaultRoutingResolverUnitTest { assertThat(routing).isEqualTo("route 42"); } + @Test // #3321 + @DisplayName("should return routing from Value expression") + void shouldReturnRoutingFromValueExpression() { + + ValidValueRoutingEntity entity = new ValidValueRoutingEntity("42", "route 42"); + + String routing = routingResolver.getRouting(entity); + + assertThat(routing).isEqualTo("route 42"); + } + @Document(indexName = "routing-resolver-test") @Routing("theRouting") static class ValidRoutingEntity { @@ -151,6 +162,37 @@ class DefaultRoutingResolverUnitTest { } } + @Document(indexName = "routing-resolver-test") + @Routing(value = "#{@spelRouting.getRouting(#entity)}") + static class ValidValueRoutingEntity { + @Nullable + @Id private String id; + @Nullable private String theRouting; + + public ValidValueRoutingEntity(@Nullable String id, @Nullable String theRouting) { + this.id = id; + this.theRouting = theRouting; + } + + @Nullable + public String getId() { + return id; + } + + public void setId(@Nullable String id) { + this.id = id; + } + + @Nullable + public String getTheRouting() { + return theRouting; + } + + public void setTheRouting(@Nullable String theRouting) { + this.theRouting = theRouting; + } + } + @Document(indexName = "routing-resolver-test") @Routing("unknownProperty") static class InvalidRoutingEntity { @@ -187,8 +229,12 @@ class DefaultRoutingResolverUnitTest { @Nullable public String getRouting(Object o) { - if (o instanceof ValidSpelRoutingEntity) { - return ((ValidSpelRoutingEntity) o).getTheRouting(); + if (o instanceof ValidSpelRoutingEntity e) { + return e.getTheRouting(); + } + + if (o instanceof ValidValueRoutingEntity e) { + return e.getTheRouting(); } return null;