diff --git a/pom.xml b/pom.xml index e82c576dfc..2cabc642f1 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 4.2.0-SNAPSHOT + 4.2.0-2335-SNAPSHOT pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index f830419609..4c31c85a6d 100644 --- a/spring-data-jdbc-distribution/pom.xml +++ b/spring-data-jdbc-distribution/pom.xml @@ -13,7 +13,7 @@ org.springframework.data spring-data-relational-parent - 4.2.0-SNAPSHOT + 4.2.0-2335-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index 00919c3360..ca8fa936c2 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -4,7 +4,7 @@ 4.0.0 spring-data-jdbc - 4.2.0-SNAPSHOT + 4.2.0-2335-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -13,7 +13,7 @@ org.springframework.data spring-data-relational-parent - 4.2.0-SNAPSHOT + 4.2.0-2335-SNAPSHOT diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/QueryMapper.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/QueryMapper.java index 69c77295e1..2129f4b0a1 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/QueryMapper.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/QueryMapper.java @@ -22,20 +22,17 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.regex.Pattern; import org.jspecify.annotations.Nullable; -import org.springframework.data.core.PropertyPath; -import org.springframework.data.core.PropertyReferenceException; import org.springframework.data.core.TypeInformation; import org.springframework.data.domain.Sort; import org.springframework.data.jdbc.core.mapping.JdbcValue; import org.springframework.data.jdbc.support.JdbcUtil; -import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.context.MappingContext; +import org.springframework.data.relational.core.mapping.PropertyPathResolver; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.data.relational.core.query.CriteriaDefinition; @@ -63,6 +60,7 @@ public class QueryMapper { private final JdbcConverter converter; private final MappingContext, RelationalPersistentProperty> mappingContext; + private final PropertyPathResolver propertyPathResolver; /** * Creates a new {@link QueryMapper} with the given {@link JdbcConverter}. @@ -75,6 +73,7 @@ public QueryMapper(JdbcConverter converter) { this.converter = converter; this.mappingContext = converter.getMappingContext(); + this.propertyPathResolver = new PropertyPathResolver(this.mappingContext); } /** @@ -305,7 +304,7 @@ private Condition mapCondition(CriteriaDefinition criteria, MapSqlParameterSourc // IN/NOT_IN with collection of composite/embedded values: expand to (… AND …) OR (…) if ((Comparator.IN.equals(comparator) || Comparator.NOT_IN.equals(comparator)) - && value instanceof Collection collection) { + && value instanceof Collection collection) { Condition condition = null; @@ -760,6 +759,7 @@ public Comparator getComparator() { public boolean isIgnoreCase() { return delegate.isIgnoreCase(); } + @Override public boolean isGroup() { return false; @@ -776,7 +776,6 @@ public SqlIdentifier getColumn() { return null; } - @Nullable @Override public Object getValue() { @@ -863,7 +862,7 @@ public SQLType getSqlType() { /** * Extension of {@link Field} to be backed with mapping metadata. */ - protected static class MetadataBackedField extends Field { + protected class MetadataBackedField extends Field { private final RelationalPersistentEntity entity; private final MappingContext, RelationalPersistentProperty> mappingContext; @@ -891,7 +890,7 @@ protected MetadataBackedField(SqlIdentifier name, RelationalPersistentEntity this.entity = entity; this.mappingContext = context; - this.path = getPath(name.getReference()); + this.path = propertyPathResolver.resolve(entity, name.getReference()); RelationalPersistentProperty persistentProperty = null; if (this.path != null) { @@ -923,39 +922,6 @@ public SqlIdentifier getMappedColumnName() { return this.property == null ? super.getMappedColumnName() : this.property.getColumnName(); } - /** - * Returns the {@link PersistentPropertyPath} for the given {@code pathExpression}. - */ - @Nullable - private PersistentPropertyPath getPath(String pathExpression) { - - try { - - PropertyPath path = forName(pathExpression); - - if (isPathToJavaLangClassProperty(path)) { - return null; - } - - return this.mappingContext.getPersistentPropertyPath(path); - } catch (MappingException | PropertyReferenceException e) { - return null; - } - } - - private PropertyPath forName(String path) { - - if (entity.getPersistentProperty(path) != null) { - return PropertyPath.from(Pattern.quote(path), entity.getTypeInformation()); - } - - return PropertyPath.from(path, entity.getTypeInformation()); - } - - private boolean isPathToJavaLangClassProperty(PropertyPath path) { - return path.getType().equals(Class.class) && path.getLeafProperty().getOwningType().getType().equals(Class.class); - } - @Nullable public PersistentPropertyPath getPath() { return path; diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/QueryMapperUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/QueryMapperUnitTests.java index da67573aaf..be4058ea0f 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/QueryMapperUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/QueryMapperUnitTests.java @@ -28,9 +28,11 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.MockedConstruction; import org.springframework.core.convert.converter.Converter; import org.springframework.data.annotation.Id; +import org.springframework.data.core.PropertyReferenceException; import org.springframework.data.domain.Sort; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.relational.core.mapping.Column; @@ -569,6 +571,51 @@ void shouldMapSortPathIntoEmbeddable() { .contains(OrderByField.from(table.column(SqlIdentifier.quoted("HOME_COUNTRY_NAME")), Sort.Direction.ASC)); } + @Test // GH-2335 + void derivedQueryPropertyLookupDoesNotThrow() { + + try (MockedConstruction mocked = mockConstruction(PropertyReferenceException.class)) { + + Condition condition = mapper.getMappedObject(parameterSource, Criteria.where("customerId").in(1L, 2L, 3L), + Table.create("order"), context.getRequiredPersistentEntity(Order.class)); + + assertThat(condition) + .hasToString("order.\"CUSTOMER_ID\" IN (?[:customer_id], ?[:customer_id1], ?[:customer_id2])"); + assertThat(mocked.constructed()).isEmpty(); + } + } + + @Test // GH-2335 + void rawColumnNameLookupIsCachedAcrossInvocations() { + + // first try uses exception for flow control + try (MockedConstruction mocked = mockConstruction(PropertyReferenceException.class)) { + + List fields = mapper.getMappedSort(Table.create("order"), Sort.by("customer_id"), + context.getRequiredPersistentEntity(Order.class)); + + assertThat(fields).extracting(Objects::toString).containsExactly("order.customer_id ASC"); + assertThat(mocked.constructed()).hasSize(1); + } + + // further attempts use the cache + try (MockedConstruction mocked = mockConstruction(PropertyReferenceException.class)) { + + Condition condition = mapper.getMappedObject(parameterSource, Criteria.where("customer_id").is(1L), + Table.create("order"), context.getRequiredPersistentEntity(Order.class)); + + assertThat(condition).hasToString("order.customer_id = ?[:customer_id]"); + assertThat(mocked.constructed()).isEmpty(); + } + } + + static class Order { + + @Id Long orderId; + Long customerId; + String status; + } + private Condition map(Criteria criteria) { return mapper.getMappedObject(parameterSource, criteria, Table.create("person"), diff --git a/spring-data-r2dbc/pom.xml b/spring-data-r2dbc/pom.xml index a65f785771..d85c219f90 100644 --- a/spring-data-r2dbc/pom.xml +++ b/spring-data-r2dbc/pom.xml @@ -4,7 +4,7 @@ 4.0.0 spring-data-r2dbc - 4.2.0-SNAPSHOT + 4.2.0-2335-SNAPSHOT Spring Data R2DBC Spring Data module for R2DBC @@ -13,7 +13,7 @@ org.springframework.data spring-data-relational-parent - 4.2.0-SNAPSHOT + 4.2.0-2335-SNAPSHOT diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java index e1b5465924..3e64c88d1e 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java @@ -20,15 +20,11 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.regex.Pattern; import org.jspecify.annotations.Nullable; -import org.springframework.data.core.PropertyPath; -import org.springframework.data.core.PropertyReferenceException; import org.springframework.data.core.TypeInformation; import org.springframework.data.domain.Sort; -import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.PersistentPropertyPath; @@ -36,6 +32,7 @@ import org.springframework.data.r2dbc.convert.R2dbcConverter; import org.springframework.data.r2dbc.dialect.R2dbcDialect; import org.springframework.data.relational.core.dialect.Escaper; +import org.springframework.data.relational.core.mapping.PropertyPathResolver; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.data.relational.core.query.CriteriaDefinition; @@ -67,6 +64,7 @@ public class QueryMapper { private final R2dbcConverter converter; private final R2dbcDialect dialect; private final MappingContext, RelationalPersistentProperty> mappingContext; + private final PropertyPathResolver propertyPathResolver; /** * Creates a new {@link QueryMapper} with the given {@link R2dbcConverter}. @@ -83,6 +81,7 @@ public QueryMapper(R2dbcDialect dialect, R2dbcConverter converter) { this.converter = converter; this.dialect = dialect; this.mappingContext = (MappingContext) converter.getMappingContext(); + this.propertyPathResolver = new PropertyPathResolver(this.mappingContext); } /** @@ -336,8 +335,8 @@ private Condition getCondition(CriteriaDefinition criteria, MutableBindings bind return mapCondition(criteria, bindings, table, entity, false); } - private Condition combine(@Nullable Condition currentCondition, - CriteriaDefinition.Combinator combinator, Condition nextCondition) { + private Condition combine(@Nullable Condition currentCondition, CriteriaDefinition.Combinator combinator, + Condition nextCondition) { if (currentCondition == null) { currentCondition = nextCondition; @@ -794,7 +793,7 @@ public RelationalPersistentProperty getRequiredProperty() { /** * Extension of {@link Field} to be backed with mapping metadata. */ - protected static class MetadataBackedField extends Field { + protected class MetadataBackedField extends Field { private final RelationalPersistentEntity entity; private final MappingContext, RelationalPersistentProperty> mappingContext; @@ -834,7 +833,7 @@ protected MetadataBackedField(SqlIdentifier name, RelationalPersistentEntity this.entity = entity; this.mappingContext = context; - this.path = getPath(name.getReference()); + this.path = propertyPathResolver.resolve(entity, name.getReference()); RelationalPersistentProperty persistentProperty = null; if (this.path != null) { @@ -865,41 +864,6 @@ public SqlIdentifier getMappedColumnName() { return this.property == null ? super.getMappedColumnName() : this.property.getColumnName(); } - /** - * Returns the {@link PersistentPropertyPath} for the given {@code pathExpression}. - * - * @param pathExpression the path expression to use. - * @return - */ - private @Nullable PersistentPropertyPath getPath(String pathExpression) { - - try { - - PropertyPath path = forName(pathExpression); - - if (isPathToJavaLangClassProperty(path)) { - return null; - } - - return this.mappingContext.getPersistentPropertyPath(path); - } catch (MappingException | PropertyReferenceException e) { - return null; - } - } - - private PropertyPath forName(String path) { - - if (entity.getPersistentProperty(path) != null) { - return PropertyPath.from(Pattern.quote(path), entity.getTypeInformation()); - } - - return PropertyPath.from(path, entity.getTypeInformation()); - } - - private boolean isPathToJavaLangClassProperty(PropertyPath path) { - return path.getType().equals(Class.class) && path.getLeafProperty().getOwningType().getType().equals(Class.class); - } - @Override public TypeInformation getTypeHint() { diff --git a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java index da883024e9..2efb825145 100644 --- a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java +++ b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/query/QueryMapperUnitTests.java @@ -26,9 +26,11 @@ import java.util.Objects; import org.junit.jupiter.api.Test; +import org.mockito.MockedConstruction; import org.springframework.core.convert.converter.Converter; import org.springframework.data.annotation.Id; +import org.springframework.data.core.PropertyReferenceException; import org.springframework.data.domain.Sort; import org.springframework.data.r2dbc.convert.MappingR2dbcConverter; import org.springframework.data.r2dbc.convert.R2dbcConverter; @@ -661,6 +663,47 @@ void shouldMapSelectionForEmbeddable() { .contains("my_aliased_table.home_street", "my_aliased_table.home_country_name"); } + @Test // GH-2335 + void derivedQueryPropertyLookupDoesNotThrow() { + + try (MockedConstruction mocked = mockConstruction(PropertyReferenceException.class)) { + + BoundCondition bindings = map(Criteria.where("customerId").in(1L, 2L, 3L), Order.class); + + assertThat(bindings.getCondition()).hasToString("order.customer_id IN (?[$1], ?[$2], ?[$3])"); + assertThat(mocked.constructed()).isEmpty(); + } + } + + @Test // GH-2335 + void rawColumnNameLookupIsCachedAcrossInvocations() { + + // first try uses exception for flow control + try (MockedConstruction mocked = mockConstruction(PropertyReferenceException.class)) { + + List fields = map(Sort.by("customer_id"), Order.class); + + assertThat(fields).extracting(Objects::toString).containsExactly("order.customer_id ASC"); + assertThat(mocked.constructed()).hasSize(1); + } + + // further attempts use the cache + try (MockedConstruction mocked = mockConstruction(PropertyReferenceException.class)) { + + BoundCondition bindings = map(Criteria.where("customer_id").is(1L), Order.class); + + assertThat(bindings.getCondition()).hasToString("order.customer_id = ?[$1]"); + assertThat(mocked.constructed()).isEmpty(); + } + } + + static class Order { + + @Id Long orderId; + Long customerId; + String status; + } + private BoundCondition map(Criteria criteria) { return map(criteria, Person.class); } diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index 06d029e579..39ef91e857 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -4,7 +4,7 @@ 4.0.0 spring-data-relational - 4.2.0-SNAPSHOT + 4.2.0-2335-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -12,7 +12,7 @@ org.springframework.data spring-data-relational-parent - 4.2.0-SNAPSHOT + 4.2.0-2335-SNAPSHOT diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PropertyPathResolver.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PropertyPathResolver.java new file mode 100644 index 0000000000..42e13f7d6e --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/mapping/PropertyPathResolver.java @@ -0,0 +1,134 @@ +/* + * Copyright 2026-present 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.relational.core.mapping; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.regex.Pattern; + +import org.jspecify.annotations.Nullable; +import org.springframework.data.core.PropertyPath; +import org.springframework.data.core.PropertyReferenceException; +import org.springframework.data.mapping.MappingException; +import org.springframework.data.mapping.PersistentPropertyPath; +import org.springframework.data.mapping.context.MappingContext; +import org.springframework.util.Assert; +import org.springframework.util.ConcurrentLruCache; + +/** + * Resolves a path expression (such as {@literal lastname} or {@literal home.city}) or column names like to + * {@literal customer_id} a {@link PersistentPropertyPath} within a {@link RelationalPersistentEntity}. If no matching + * path is found {@literal null} is returned. + *

+ * The result, both positive and negative matches are cached. The cache of negative matches is limited to limit possible + * memory consumption. + * + * @author Jens Schauder + * @since 4.2 + */ +public class PropertyPathResolver { + + /** + * Upper bound for the number of cached unresolvable path expressions. Chosen to comfortably exceed the number of + * distinct non-property references a typical application issues (such as raw column names) while capping the memory a + * flood of arbitrary references can consume. + */ + private static final int UNRESOLVABLE_CACHE_SIZE = 4096; + + private final MappingContext, RelationalPersistentProperty> mappingContext; + private final Map> resolved = new ConcurrentHashMap<>(); + private final ConcurrentLruCache unresolvable = new ConcurrentLruCache<>( + UNRESOLVABLE_CACHE_SIZE, key -> Boolean.TRUE); + + /** + * Creates a new {@link PropertyPathResolver} for the given {@link MappingContext}. + * + * @param mappingContext must not be {@literal null}. + */ + public PropertyPathResolver( + MappingContext, RelationalPersistentProperty> mappingContext) { + + Assert.notNull(mappingContext, "MappingContext must not be null"); + + this.mappingContext = mappingContext; + } + + /** + * Resolves the {@link PersistentPropertyPath} for the given path expression on the given entity. Results are cached. + * + * @param entity the entity to resolve the path expression against, must not be {@literal null}. + * @param pathExpression the path expression to resolve, must not be {@literal null}. + * @return the resolved {@link PersistentPropertyPath} or {@literal null} if the expression does not denote a property + * path. + */ + public @Nullable PersistentPropertyPath resolve(RelationalPersistentEntity entity, + String pathExpression) { + + PathResolutionKey key = new PathResolutionKey(entity, pathExpression); + + PersistentPropertyPath cached = this.resolved.get(key); + if (cached != null) { + return cached; + } + + if (this.unresolvable.contains(key)) { + return null; + } + + PersistentPropertyPath path = doResolve(entity, pathExpression); + + if (path == null) { + this.unresolvable.get(key); + return null; + } + + this.resolved.put(key, path); + return path; + } + + private @Nullable PersistentPropertyPath doResolve(RelationalPersistentEntity entity, + String pathExpression) { + + try { + + PropertyPath path = forName(entity, pathExpression); + + if (isPathToJavaLangClassProperty(path)) { + return null; + } + + return this.mappingContext.getPersistentPropertyPath(path); + } catch (MappingException | PropertyReferenceException e) { + return null; + } + } + + private static PropertyPath forName(RelationalPersistentEntity entity, String path) { + + if (entity.getPersistentProperty(path) != null) { + return PropertyPath.from(Pattern.quote(path), entity.getTypeInformation()); + } + + return PropertyPath.from(path, entity.getTypeInformation()); + } + + private static boolean isPathToJavaLangClassProperty(PropertyPath path) { + return path.getType().equals(Class.class) && path.getLeafProperty().getOwningType().getType().equals(Class.class); + } + + private record PathResolutionKey(RelationalPersistentEntity entity, String pathExpression) { + } +} diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/PropertyPathResolverUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/PropertyPathResolverUnitTests.java new file mode 100644 index 0000000000..05a28db657 --- /dev/null +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/mapping/PropertyPathResolverUnitTests.java @@ -0,0 +1,77 @@ +/* + * Copyright 2026-present 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.relational.core.mapping; + +import static org.assertj.core.api.Assertions.*; +import static org.mockito.Mockito.*; + +import org.junit.jupiter.api.Test; +import org.mockito.MockedConstruction; + +import org.springframework.data.annotation.Id; +import org.springframework.data.core.PropertyReferenceException; +import org.springframework.data.mapping.PersistentPropertyPath; + +/** + * Unit tests for {@link PropertyPathResolver}. + * + * @author Jens Schauder + */ +class PropertyPathResolverUnitTests { + + private final RelationalMappingContext context = new RelationalMappingContext(); + private final PropertyPathResolver resolver = new PropertyPathResolver(context); + + @Test // GH-2335 + void resolvesMappedProperty() { + + PersistentPropertyPath path = resolver.resolve(entity(), "customerId"); + + assertThat(path).isNotNull(); + assertThat(path.getLeafProperty().getName()).isEqualTo("customerId"); + } + + @Test // GH-2335 + void returnsNullForRawColumnName() { + + assertThat(resolver.resolve(entity(), "customer_id")).isNull(); + } + + @Test // GH-2335 + void cachesResolutionOfRawColumnName() { + + try (MockedConstruction mocked = mockConstruction(PropertyReferenceException.class)) { + + resolver.resolve(entity(), "customer_id"); + resolver.resolve(entity(), "customer_id"); + resolver.resolve(entity(), "customer_id"); + + // The exception-based fallback in PropertyPath.from(...) is exercised only on the first resolution. + assertThat(mocked.constructed()).hasSize(1); + } + } + + private RelationalPersistentEntity entity() { + return context.getRequiredPersistentEntity(Order.class); + } + + static class Order { + + @Id Long orderId; + Long customerId; + String status; + } +}