From 322cfabb17eb93c492dce96cf0ea5134948d61a6 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 3 Jul 2018 16:46:00 +0200 Subject: [PATCH] DATAREDIS-849 - Add support for immutable objects. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now support immutable objects for saving (e.g. object without a provided Id) and loading (i.e. persistence constructor declares a subset of properties). New instances are created using wither methods/Kotlin copy(…) methods if an immutable object requires association with an Id. Association of the Id to its object moved from RedisKeyValueAdapter to RedisKeyValueTemplate. --- .../data/redis/core/RedisKeyValueAdapter.java | 17 +++---- .../redis/core/RedisKeyValueTemplate.java | 39 ++++++++++++--- .../core/convert/MappingRedisConverter.java | 8 +++- .../core/RedisKeyValueTemplateTests.java | 45 +++++++++++++++++ ...edisRepositoryClusterIntegrationTests.java | 2 +- .../RedisRepositoryIntegrationTestBase.java | 48 ++++++++++++++++++- .../RedisRepositoryIntegrationTests.java | 2 +- 7 files changed, 140 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java b/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java index bebf53c26..d4003c0dd 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java +++ b/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java @@ -36,7 +36,7 @@ import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.ConverterNotFoundException; import org.springframework.data.keyvalue.core.AbstractKeyValueAdapter; import org.springframework.data.keyvalue.core.KeyValueAdapter; -import org.springframework.data.keyvalue.core.mapping.KeyValuePersistentProperty; +import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.redis.connection.DataType; import org.springframework.data.redis.connection.Message; import org.springframework.data.redis.connection.MessageListener; @@ -217,15 +217,7 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter } if (rdo.getId() == null) { - rdo.setId(converter.getConversionService().convert(id, String.class)); - - if (!(item instanceof RedisData)) { - KeyValuePersistentProperty idProperty = converter.getMappingContext() - .getRequiredPersistentEntity(item.getClass()).getRequiredIdProperty(); - converter.getMappingContext().getRequiredPersistentEntity(item.getClass()).getPropertyAccessor(item) - .setProperty(idProperty, id); - } } redisOps.execute((RedisCallback) connection -> { @@ -624,8 +616,13 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter }); if (timeout != null || !ttlProperty.getType().isPrimitive()) { - entity.getPropertyAccessor(target).setProperty(ttlProperty, + + PersistentPropertyAccessor propertyAccessor = entity.getPropertyAccessor(target); + + propertyAccessor.setProperty(ttlProperty, converter.getConversionService().convert(timeout, ttlProperty.getType())); + + target = propertyAccessor.getBean(); } } diff --git a/src/main/java/org/springframework/data/redis/core/RedisKeyValueTemplate.java b/src/main/java/org/springframework/data/redis/core/RedisKeyValueTemplate.java index 9b66b5e73..706c903df 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisKeyValueTemplate.java +++ b/src/main/java/org/springframework/data/redis/core/RedisKeyValueTemplate.java @@ -22,8 +22,12 @@ import java.util.List; import org.springframework.data.keyvalue.core.KeyValueAdapter; import org.springframework.data.keyvalue.core.KeyValueCallback; import org.springframework.data.keyvalue.core.KeyValueTemplate; +import org.springframework.data.keyvalue.core.mapping.KeyValuePersistentProperty; +import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.redis.core.convert.RedisConverter; +import org.springframework.data.redis.core.convert.RedisData; import org.springframework.data.redis.core.mapping.RedisMappingContext; +import org.springframework.data.redis.core.mapping.RedisPersistentEntity; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -127,14 +131,31 @@ public class RedisKeyValueTemplate extends KeyValueTemplate { * @see org.springframework.data.keyvalue.core.KeyValueTemplate#insert(java.lang.Object, java.lang.Object) */ @Override - public void insert(Object id, Object objectToInsert) { + public T insert(Object id, T objectToInsert) { if (objectToInsert instanceof PartialUpdate) { doPartialUpdate((PartialUpdate) objectToInsert); - return; + return objectToInsert; } - super.insert(id, objectToInsert); + if (!(objectToInsert instanceof RedisData)) { + + RedisConverter converter = adapter.getConverter(); + + RedisPersistentEntity entity = converter.getMappingContext() + .getRequiredPersistentEntity(objectToInsert.getClass()); + + KeyValuePersistentProperty idProperty = entity.getRequiredIdProperty(); + PersistentPropertyAccessor propertyAccessor = entity.getPropertyAccessor(objectToInsert); + + if (propertyAccessor.getProperty(idProperty) == null) { + + propertyAccessor.setProperty(idProperty, id); + return super.insert(id, propertyAccessor.getBean()); + } + } + + return super.insert(id, objectToInsert); } /* @@ -142,14 +163,20 @@ public class RedisKeyValueTemplate extends KeyValueTemplate { * @see org.springframework.data.keyvalue.core.KeyValueTemplate#update(java.lang.Object) */ @Override - public void update(Object objectToUpdate) { + public T update(T objectToUpdate) { if (objectToUpdate instanceof PartialUpdate) { doPartialUpdate((PartialUpdate) objectToUpdate); - return; + + return objectToUpdate; } - super.update(objectToUpdate); + return super.update(objectToUpdate); + } + + @Override + public T update(Object id, T objectToUpdate) { + return super.update(id, objectToUpdate); } protected void doPartialUpdate(final PartialUpdate update) { diff --git a/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java b/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java index fbe37703a..06a783a6d 100644 --- a/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java +++ b/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java @@ -211,7 +211,11 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { R instance = (R) conversionService.convert(partial, readType.getType()); if (entity != null && entity.hasIdProperty()) { - entity.getPropertyAccessor(instance).setProperty(entity.getRequiredIdProperty(), source.getId()); + + PersistentPropertyAccessor propertyAccessor = entity.getPropertyAccessor(instance); + + propertyAccessor.setProperty(entity.getRequiredIdProperty(), source.getId()); + instance = propertyAccessor.getBean(); } return instance; } @@ -246,7 +250,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { readAssociation(path, source, entity, accessor); - return (R) instance; + return (R) accessor.getBean(); } @SuppressWarnings("unchecked") diff --git a/src/test/java/org/springframework/data/redis/core/RedisKeyValueTemplateTests.java b/src/test/java/org/springframework/data/redis/core/RedisKeyValueTemplateTests.java index f7766002a..2d6236f57 100644 --- a/src/test/java/org/springframework/data/redis/core/RedisKeyValueTemplateTests.java +++ b/src/test/java/org/springframework/data/redis/core/RedisKeyValueTemplateTests.java @@ -19,8 +19,10 @@ import static org.hamcrest.CoreMatchers.*; import static org.hamcrest.number.IsCloseTo.*; import static org.junit.Assert.*; +import lombok.AllArgsConstructor; import lombok.Data; import lombok.EqualsAndHashCode; +import lombok.experimental.Wither; import java.util.ArrayList; import java.util.Arrays; @@ -805,6 +807,33 @@ public class RedisKeyValueTemplateTests { assertThat(target.get().ttl, is(-1L)); } + @Test // DATAREDIS-849 + public void shouldWriteImmutableType() { + + ImmutableObject source = new ImmutableObject().withValue("foo").withTtl(1234L); + + ImmutableObject inserted = template.insert(source); + + assertThat(source.id, is(nullValue())); + assertThat(inserted.id, is(notNullValue())); + } + + @Test // DATAREDIS-849 + public void shouldReadImmutableType() { + + ImmutableObject source = new ImmutableObject().withValue("foo").withTtl(1234L); + ImmutableObject inserted = template.insert(source); + + Optional loaded = template.findById(inserted.id, ImmutableObject.class); + + assertThat(loaded.isPresent(), is(true)); + + ImmutableObject immutableObject = loaded.get(); + assertThat(immutableObject.id, is(inserted.id)); + assertThat(immutableObject.ttl, is(notNullValue())); + assertThat(immutableObject.value, is(inserted.value)); + } + @EqualsAndHashCode @RedisHash("template-test-type-mapping") static class VariousTypes { @@ -929,4 +958,20 @@ public class RedisKeyValueTemplateTests { String value; @TimeToLive int ttl; } + + @Data + @Wither + @AllArgsConstructor + static class ImmutableObject { + + final @Id String id; + final String value; + final @TimeToLive Long ttl; + + ImmutableObject() { + this.id = null; + this.value = null; + this.ttl = null; + } + } } diff --git a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryClusterIntegrationTests.java b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryClusterIntegrationTests.java index 453ddcb18..667e61836 100644 --- a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryClusterIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryClusterIntegrationTests.java @@ -54,7 +54,7 @@ public class RedisRepositoryClusterIntegrationTests extends RedisRepositoryInteg @EnableRedisRepositories(considerNestedRepositories = true, indexConfiguration = MyIndexConfiguration.class, keyspaceConfiguration = MyKeyspaceConfiguration.class, includeFilters = { @ComponentScan.Filter(type = FilterType.ASSIGNABLE_TYPE, - classes = { PersonRepository.class, CityRepository.class }) }) + classes = { PersonRepository.class, CityRepository.class, ImmutableObjectRepository.class }) }) static class Config { @Bean diff --git a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java index bba1f4281..b2281ff6b 100644 --- a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java +++ b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java @@ -19,6 +19,8 @@ import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import lombok.Data; +import lombok.Value; +import lombok.experimental.Wither; import java.util.Arrays; import java.util.Collections; @@ -60,6 +62,7 @@ public abstract class RedisRepositoryIntegrationTestBase { @Autowired PersonRepository repo; @Autowired CityRepository cityRepo; + @Autowired ImmutableObjectRepository immutableObjectRepo; @Autowired KeyValueTemplate kvTemplate; @Before @@ -403,6 +406,37 @@ public abstract class RedisRepositoryIntegrationTestBase { assertThat(result, not(hasItems(p1))); } + @Test // DATAREDIS-849 + public void shouldReturnNewObjectInstanceOnImmutableSave() { + + Immutable object = new Immutable(null, "Walter", new Immutable("heisenberg", "White", null)); + Immutable saved = immutableObjectRepo.save(object); + + assertThat(object.id, is(nullValue())); + assertThat(saved.id, is(notNullValue())); + } + + @Test // DATAREDIS-849 + public void shouldReturnNewObjectInstanceOnImmutableSaveAll() { + + Immutable object = new Immutable(null, "Walter", new Immutable("heisenberg", "White", null)); + List saved = (List) immutableObjectRepo.saveAll(Collections.singleton(object)); + + assertThat(object.id, is(nullValue())); + assertThat(saved.get(0).id, is(notNullValue())); + } + + @Test // DATAREDIS-849 + public void shouldProperlyReadNestedImmutableObject() { + + Immutable nested = new Immutable("heisenberg", "White", null); + Immutable object = new Immutable(null, "Walter", nested); + Immutable saved = immutableObjectRepo.save(object); + + Immutable loaded = immutableObjectRepo.findById(saved.id).get(); + assertThat(loaded.nested, is(nested)); + } + public static interface PersonRepository extends PagingAndSortingRepository, QueryByExampleExecutor { @@ -434,11 +468,13 @@ public abstract class RedisRepositoryIntegrationTestBase { List findAll(Example example); } - public static interface CityRepository extends CrudRepository { + public interface CityRepository extends CrudRepository { List findByLocationNear(Point point, Distance distance); } + public interface ImmutableObjectRepository extends CrudRepository {} + /** * Custom Redis {@link IndexConfiguration} forcing index of {@link Person#lastname}. * @@ -493,4 +529,14 @@ public abstract class RedisRepositoryIntegrationTestBase { @GeoIndexed Point location; } + + @Value + @Wither + public static class Immutable { + + @Id String id; + String name; + + Immutable nested; + } } diff --git a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTests.java index fcfc28a89..1eb383f27 100644 --- a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTests.java @@ -56,7 +56,7 @@ public class RedisRepositoryIntegrationTests extends RedisRepositoryIntegrationT @EnableRedisRepositories(considerNestedRepositories = true, indexConfiguration = MyIndexConfiguration.class, keyspaceConfiguration = MyKeyspaceConfiguration.class, includeFilters = { @ComponentScan.Filter(type = FilterType.ASSIGNABLE_TYPE, - classes = { PersonRepository.class, CityRepository.class }) }) + classes = { PersonRepository.class, CityRepository.class, ImmutableObjectRepository.class }) }) static class Config {