Introduce caching and equality checks to derived persistent entities and properties.

We now provide equals/hashCode and selective caching for derived properties and entities derived from embedded properties and constructor-annotated properties to avoid memory leaks using properties and entities as cache keys.

Closes #1471
This commit is contained in:
Mark Paluch
2024-01-24 14:00:45 +01:00
parent 45cb35b4d2
commit e68a0472c7
7 changed files with 200 additions and 71 deletions

View File

@@ -27,12 +27,12 @@ import java.util.function.Predicate;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.BeanClassLoaderAware;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import org.springframework.core.CollectionFactory;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.core.convert.ConversionService;
import org.springframework.core.convert.support.DefaultConversionService;
import org.springframework.dao.InvalidDataAccessApiUsageException;
@@ -1424,8 +1424,7 @@ public class MappingCassandraConverter extends AbstractCassandraConverter
throw new MappingException(String.format("Parameter %s does not have a name", parameter));
}
CassandraPersistentProperty property = getPersistentProperty(name, parameter.getType(),
parameter.getAnnotations());
CassandraPersistentProperty property = entity.getProperty(parameter);
if (property == null) {
@@ -1436,19 +1435,6 @@ public class MappingCassandraConverter extends AbstractCassandraConverter
return (T) getReadValue(context.forProperty(property.getName()), provider, property);
}
@Nullable
private CassandraPersistentProperty getPersistentProperty(String name, TypeInformation<?> typeInformation,
MergedAnnotations annotations) {
CassandraPersistentProperty property = entity.getPersistentProperty(name);
if (annotations.isPresent(Column.class) || annotations.isPresent(Element.class)) {
return new AnnotatedCassandraConstructorProperty(
property == null ? new CassandraConstructorProperty(name, entity, typeInformation) : property, annotations);
}
return property;
}
}
private record PropertyTranslatingPropertyAccessor<T> (PersistentPropertyAccessor<T> delegate,

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2021-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,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.cassandra.core.convert;
package org.springframework.data.cassandra.core.mapping;
import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedType;
@@ -22,11 +22,9 @@ import java.lang.reflect.Method;
import org.springframework.beans.BeansException;
import org.springframework.context.ApplicationContext;
import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.data.cassandra.core.cql.Ordering;
import org.springframework.data.cassandra.core.mapping.CassandraPersistentProperty;
import org.springframework.data.cassandra.core.mapping.Column;
import org.springframework.data.cassandra.core.mapping.Element;
import org.springframework.data.mapping.Association;
import org.springframework.data.mapping.PersistentEntity;
import org.springframework.data.util.TypeInformation;
@@ -46,41 +44,31 @@ class AnnotatedCassandraConstructorProperty implements CassandraPersistentProper
private final CassandraPersistentProperty delegate;
private final MergedAnnotations annotations;
private final MergedAnnotation<Column> column;
private final MergedAnnotation<Element> element;
public AnnotatedCassandraConstructorProperty(CassandraPersistentProperty delegate, MergedAnnotations annotations) {
this.delegate = delegate;
this.annotations = annotations;
this.column = annotations.get(Column.class);
this.element = annotations.get(Element.class);
}
@Override
@Nullable
public CqlIdentifier getColumnName() {
if (annotations.isPresent(Column.class)) {
return CqlIdentifier.fromCql(annotations.get(Column.class).getString("value"));
}
return delegate.getColumnName();
return column.isPresent() ? CqlIdentifier.fromCql(column.getString("value")) : delegate.getColumnName();
}
@Override
public boolean hasExplicitColumnName() {
if (annotations.isPresent(Column.class)) {
return !ObjectUtils.isEmpty(annotations.get(Column.class).getString("value"));
}
return false;
return column.isPresent() && !ObjectUtils.isEmpty(column.getString("value"));
}
@Override
@Nullable
public Integer getOrdinal() {
if (annotations.isPresent(Element.class)) {
return annotations.get(Element.class).getInt("value");
}
return delegate.getOrdinal();
return element.isPresent() ? Integer.valueOf(element.getInt("value")) : delegate.getOrdinal();
}
@Override

View File

@@ -17,7 +17,9 @@ package org.springframework.data.cassandra.core.mapping;
import java.lang.annotation.Annotation;
import java.util.Comparator;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.BiFunction;
import org.springframework.beans.BeansException;
@@ -26,9 +28,11 @@ import org.springframework.context.ApplicationContextAware;
import org.springframework.context.expression.BeanFactoryAccessor;
import org.springframework.context.expression.BeanFactoryResolver;
import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.data.mapping.Association;
import org.springframework.data.mapping.AssociationHandler;
import org.springframework.data.mapping.MappingException;
import org.springframework.data.mapping.Parameter;
import org.springframework.data.mapping.model.BasicPersistentEntity;
import org.springframework.data.util.TypeInformation;
import org.springframework.expression.spel.support.StandardEvaluationContext;
@@ -60,6 +64,8 @@ public class BasicCassandraPersistentEntity<T> extends BasicPersistentEntity<T,
private @Nullable StandardEvaluationContext spelContext;
private final Map<Parameter<?, CassandraPersistentProperty>, CassandraPersistentProperty> constructorProperties = new ConcurrentHashMap<>();
/**
* Create a new {@link BasicCassandraPersistentEntity} given {@link TypeInformation}.
*
@@ -213,4 +219,25 @@ public class BasicCassandraPersistentEntity<T> extends BasicPersistentEntity<T,
public boolean isUserDefinedType() {
return false;
}
@Override
public CassandraPersistentProperty getProperty(Parameter<?, CassandraPersistentProperty> parameter) {
if (parameter.getName() == null) {
return null;
}
MergedAnnotations annotations = parameter.getAnnotations();
if (annotations.isPresent(Column.class) || annotations.isPresent(Element.class)) {
return constructorProperties.computeIfAbsent(parameter, it -> {
CassandraPersistentProperty property = getPersistentProperty(it.getName());
return new AnnotatedCassandraConstructorProperty(
property == null ? new CassandraConstructorProperty(it, this) : property, it.getAnnotations());
});
}
return getPersistentProperty(parameter.getName());
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2021-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,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.cassandra.core.convert;
package org.springframework.data.cassandra.core.mapping;
import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedType;
@@ -25,9 +25,8 @@ import java.util.Optional;
import org.springframework.beans.BeansException;
import org.springframework.context.ApplicationContext;
import org.springframework.data.cassandra.core.cql.Ordering;
import org.springframework.data.cassandra.core.mapping.CassandraPersistentEntity;
import org.springframework.data.cassandra.core.mapping.CassandraPersistentProperty;
import org.springframework.data.mapping.Association;
import org.springframework.data.mapping.Parameter;
import org.springframework.data.mapping.PersistentEntity;
import org.springframework.data.util.TypeInformation;
import org.springframework.lang.Nullable;
@@ -42,17 +41,20 @@ import com.datastax.oss.driver.api.core.CqlIdentifier;
*/
class CassandraConstructorProperty implements CassandraPersistentProperty {
private final Parameter<?, CassandraPersistentProperty> constructorParameter;
private final String name;
private final CassandraPersistentEntity<?> owner;
private final TypeInformation<?> typeInformation;
public CassandraConstructorProperty(String name, CassandraPersistentEntity<?> owner,
TypeInformation<?> typeInformation) {
this.name = name;
public CassandraConstructorProperty(Parameter<?, CassandraPersistentProperty> constructorParameter,
CassandraPersistentEntity<?> owner) {
this.constructorParameter = constructorParameter;
this.name = constructorParameter.getName();
this.owner = owner;
this.typeInformation = typeInformation;
this.typeInformation = constructorParameter.getType();
}
@Nullable
@@ -301,4 +303,21 @@ class CassandraConstructorProperty implements CassandraPersistentProperty {
public void setApplicationContext(ApplicationContext applicationContext) throws BeansException {
throw new UnsupportedOperationException();
}
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (!(o instanceof CassandraConstructorProperty that)) {
return false;
}
return constructorParameter.equals(that.constructorParameter);
}
@Override
public int hashCode() {
return constructorParameter.hashCode();
}
}

View File

@@ -15,7 +15,9 @@
*/
package org.springframework.data.cassandra.core.mapping;
import org.springframework.data.mapping.Parameter;
import org.springframework.data.mapping.PersistentEntity;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import com.datastax.oss.driver.api.core.CqlIdentifier;
@@ -29,6 +31,19 @@ import com.datastax.oss.driver.api.core.CqlIdentifier;
*/
public interface CassandraPersistentEntity<T> extends PersistentEntity<T, CassandraPersistentProperty> {
/**
* Retrieve a {@link CassandraPersistentProperty} from a {@link Parameter persistence creator (constructor/factory
* method) parameter}. Parameters are either derived by name or synthesized if their name does not map to a existing
* property.
*
* @param parameter the parameter to create a property from. Parameters without a name return no ({@literal null})
* parameter.
* @return the property, synthetic property or {@literal null}, if the parameter is unnamed.
* @since 4.2.3
*/
@Nullable
CassandraPersistentProperty getProperty(Parameter<?, CassandraPersistentProperty> parameter);
/**
* Returns whether this entity represents a composite primary key.
*/

View File

@@ -311,6 +311,13 @@ public class EmbeddedEntityOperations {
return delegate.requiresPropertyPopulation();
}
@Nullable
@Override
public CassandraPersistentProperty getProperty(Parameter<?, CassandraPersistentProperty> parameter) {
CassandraPersistentProperty property = delegate.getProperty(parameter);
return property == null ? null : wrap(property);
}
@NotNull
@Override
public Iterator<CassandraPersistentProperty> iterator() {
@@ -330,6 +337,28 @@ public class EmbeddedEntityOperations {
return delegate.spliterator();
}
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (!(o instanceof PrefixedCassandraPersistentEntity<?> that)) {
return false;
}
if (!prefix.equals(that.prefix)) {
return false;
}
return delegate.equals(that.delegate);
}
@Override
public int hashCode() {
int result = prefix.hashCode();
result = 31 * result + delegate.hashCode();
return result;
}
private PrefixedCassandraPersistentProperty wrap(CassandraPersistentProperty source) {
return new PrefixedCassandraPersistentProperty(prefix, source);
}
@@ -646,5 +675,27 @@ public class EmbeddedEntityOperations {
public void setApplicationContext(ApplicationContext applicationContext) throws BeansException {
delegate.setApplicationContext(applicationContext);
}
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (!(o instanceof PrefixedCassandraPersistentProperty that)) {
return false;
}
if (!prefix.equals(that.prefix)) {
return false;
}
return delegate.equals(that.delegate);
}
@Override
public int hashCode() {
int result = prefix.hashCode();
result = 31 * result + delegate.hashCode();
return result;
}
}
}

View File

@@ -36,6 +36,8 @@ import org.json.simple.parser.JSONParser;
import org.json.simple.parser.ParseException;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.beans.DirectFieldAccessor;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.core.convert.converter.Converter;
import org.springframework.data.annotation.Id;
@@ -53,6 +55,7 @@ import org.springframework.data.cassandra.domain.User;
import org.springframework.data.cassandra.domain.UserToken;
import org.springframework.data.cassandra.support.UserDefinedTypeBuilder;
import org.springframework.data.cassandra.test.util.RowMockUtil;
import org.springframework.data.convert.SimplePropertyValueConversions;
import org.springframework.data.convert.ValueConverter;
import org.springframework.data.projection.EntityProjection;
import org.springframework.data.projection.EntityProjectionIntrospector;
@@ -456,8 +459,7 @@ public class MappingCassandraConverterUnitTests {
rowMock = RowMockUtil.newRowMock(column("id", "my-id", DataTypes.ASCII),
column("localDate", LocalDate.of(2010, 7, 4), DataTypes.DATE));
TypeWithLocalDateMappedToDate result = converter.readRow(TypeWithLocalDateMappedToDate.class,
rowMock);
TypeWithLocalDateMappedToDate result = converter.readRow(TypeWithLocalDateMappedToDate.class, rowMock);
assertThat(result.localDate).isNotNull();
assertThat(result.localDate.getYear()).isEqualTo(2010);
@@ -581,8 +583,7 @@ public class MappingCassandraConverterUnitTests {
void shouldFailWriteWhereConditionUsingEntityWithNullId() {
assertThatIllegalArgumentException().isThrownBy(
() -> converter.write(new User(), new Where(),
mappingContext.getRequiredPersistentEntity(User.class)));
() -> converter.write(new User(), new Where(), mappingContext.getRequiredPersistentEntity(User.class)));
}
@Test // DATACASS-308
@@ -604,8 +605,7 @@ public class MappingCassandraConverterUnitTests {
entity.setFirstname("Walter");
entity.setLastname("White");
converter.write(entity, where,
mappingContext.getRequiredPersistentEntity(TypeWithCompositeKey.class));
converter.write(entity, where, mappingContext.getRequiredPersistentEntity(TypeWithCompositeKey.class));
assertThat(where).containsEntry(CqlIdentifier.fromCql("firstname"), "Walter");
assertThat(where).containsEntry(CqlIdentifier.fromCql("lastname"), "White");
@@ -643,8 +643,7 @@ public class MappingCassandraConverterUnitTests {
Where where = new Where();
converter.write(Condition.MINT, where,
mappingContext.getRequiredPersistentEntity(EnumPrimaryKey.class));
converter.write(Condition.MINT, where, mappingContext.getRequiredPersistentEntity(EnumPrimaryKey.class));
assertThat(where).containsEntry(CqlIdentifier.fromCql("condition"), "MINT");
}
@@ -682,8 +681,8 @@ public class MappingCassandraConverterUnitTests {
@Test // DATACASS-308
void shouldFailWritingWhereConditionForTypeWithPkClassKeyUsingEntityWithNullId() {
assertThatIllegalArgumentException().isThrownBy(() -> converter.write(new TypeWithKeyClass(),
new Where(), mappingContext.getRequiredPersistentEntity(TypeWithKeyClass.class)));
assertThatIllegalArgumentException().isThrownBy(() -> converter.write(new TypeWithKeyClass(), new Where(),
mappingContext.getRequiredPersistentEntity(TypeWithKeyClass.class)));
}
@Test // DATACASS-308
@@ -721,8 +720,7 @@ public class MappingCassandraConverterUnitTests {
Row row = RowMockUtil.newRowMock(column("firstname", "Walter", DataTypes.TEXT),
column("lastname", "White", DataTypes.TEXT));
TableWithCompositeKeyViaConstructor result = converter
.read(TableWithCompositeKeyViaConstructor.class, row);
TableWithCompositeKeyViaConstructor result = converter.read(TableWithCompositeKeyViaConstructor.class, row);
assertThat(result.key.firstname).isEqualTo("Walter");
assertThat(result.key.lastname).isEqualTo("White");
@@ -743,8 +741,8 @@ public class MappingCassandraConverterUnitTests {
@Test // DATACASS-308
void shouldFailWhereConditionForTypeWithPkClassKeyUsingMapIdHavingUnknownProperty() {
assertThatIllegalArgumentException().isThrownBy(() -> converter.write(id("unknown", "Walter"),
new Where(), mappingContext.getRequiredPersistentEntity(TypeWithMapId.class)));
assertThatIllegalArgumentException().isThrownBy(() -> converter.write(id("unknown", "Walter"), new Where(),
mappingContext.getRequiredPersistentEntity(TypeWithMapId.class)));
}
@Test // DATACASS-362
@@ -854,8 +852,7 @@ public class MappingCassandraConverterUnitTests {
RowMockUtil.column("other", "Some other value", DataTypes.TEXT),
RowMockUtil.column("tuple", tupleValue, tupleType));
TypeWithPropertyValueConverter result = converter.read(TypeWithPropertyValueConverter.class,
rowMock);
TypeWithPropertyValueConverter result = converter.read(TypeWithPropertyValueConverter.class, rowMock);
assertThat(result.name).isEqualTo("Other: Some other value, reversed: Walter");
assertThat(result.other).isEqualTo("Some other value");
@@ -999,8 +996,7 @@ public class MappingCassandraConverterUnitTests {
RowMockUtil.column("firstname", "Heisenberg", DataTypes.ASCII),
RowMockUtil.column("lastname", "White", DataTypes.ASCII));
WithColumnAnnotationInConstructor converted = this.converter
.read(WithColumnAnnotationInConstructor.class, rowMock);
WithColumnAnnotationInConstructor converted = this.converter.read(WithColumnAnnotationInConstructor.class, rowMock);
assertThat(converted.firstname).isEqualTo("Walter");
assertThat(converted.lastname).isEqualTo("White");
@@ -1078,6 +1074,43 @@ public class MappingCassandraConverterUnitTests {
assertThat(result.tuple().one).isEqualTo("One");
}
@Test // GH-1471
void propertyValueConversionsCacheShouldConsiderPropertyEquality() {
rowMock = RowMockUtil.newRowMock(RowMockUtil.column("fn", "Walter", DataTypes.ASCII),
RowMockUtil.column("lastname", "White", DataTypes.ASCII),
RowMockUtil.column("n_firstname", "Heisenberg", DataTypes.ASCII),
RowMockUtil.column("c_firstname", "", DataTypes.ASCII), // required as marker to prevent Nullable embedded being
// null
RowMockUtil.column("c_fn", "Nested Walter", DataTypes.ASCII),
RowMockUtil.column("c_lastname", "Nested White", DataTypes.ASCII));
SimplePropertyValueConversions spvc = (SimplePropertyValueConversions) converter.getCustomConversions()
.getPropertyValueConversions();
DirectFieldAccessor accessor = new DirectFieldAccessor(spvc.getConverterFactory());
Map<?, ?> cache = (Map<?, ?>) new DirectFieldAccessor(accessor.getPropertyValue("cache"))
.getPropertyValue("perPropertyCache");
assertThat(cache).isEmpty();
SourceForPersistentPropertyDerivation converted = this.converter.read(SourceForPersistentPropertyDerivation.class,
rowMock);
assertThat(converted.firstname).isEqualTo("Walter");
assertThat(converted.lastname).isEqualTo("White");
assertThat(converted.name.firstname()).isEqualTo("Heisenberg");
assertThat(converted.constructor.firstname).isEqualTo("Nested Walter");
assertThat(converted.constructor.lastname).isEqualTo("Nested White");
assertThat(cache).hasSize(5);
for (int i = 0; i < 10; i++) {
this.converter.read(SourceForPersistentPropertyDerivation.class, rowMock);
}
assertThat(cache).hasSize(5);
}
private static List<Object> getValues(Map<CqlIdentifier, Object> statement) {
return new ArrayList<>(statement.values());
}
@@ -1362,6 +1395,19 @@ public class MappingCassandraConverterUnitTests {
}
}
private static class SourceForPersistentPropertyDerivation {
String firstname;
final @Transient String lastname;
@Embedded.Nullable("n_") Name name;
@Embedded.Nullable("c_") WithColumnAnnotationInConstructor constructor;
public SourceForPersistentPropertyDerivation(@Column("fn") String fn, @Column("lastname") String lastname) {
this.firstname = fn;
this.lastname = lastname;
}
}
private static class WithMappedTuple {
String firstname;
@@ -1569,8 +1615,7 @@ public class MappingCassandraConverterUnitTests {
Row source = RowMockUtil.newRowMock(column("id", "id-1", DataTypes.TEXT), column("prefixage", 30, DataTypes.INT),
column("prefixfirstname", "fn", DataTypes.TEXT));
WithPrefixedNullableEmbeddedType target = converter.read(WithPrefixedNullableEmbeddedType.class,
source);
WithPrefixedNullableEmbeddedType target = converter.read(WithPrefixedNullableEmbeddedType.class, source);
assertThat(target.nested).isEqualTo(new EmbeddedWithSimpleTypes("fn", 30, null));
}
@@ -1614,14 +1659,12 @@ public class MappingCassandraConverterUnitTests {
Row source = RowMockUtil.newRowMock(column("id", "id-1", DataTypes.TEXT), column("name", "my-book", DataTypes.INT),
column("author", udtValue, authorType));
EntityProjectionIntrospector introspector = EntityProjectionIntrospector.create(
converter.getProjectionFactory(),
EntityProjectionIntrospector introspector = EntityProjectionIntrospector.create(converter.getProjectionFactory(),
EntityProjectionIntrospector.ProjectionPredicate.typeHierarchy()
.and((target, underlyingType) -> !converter.getCustomConversions().isSimpleType(target)),
mappingContext);
BookProjection projection = converter
.project(introspector.introspect(BookProjection.class, Book.class), source);
BookProjection projection = converter.project(introspector.introspect(BookProjection.class, Book.class), source);
assertThat(projection.getName()).isEqualTo("my-book by Walter White");
}