diff --git a/src/main/java/org/springframework/data/redis/core/IndexWriter.java b/src/main/java/org/springframework/data/redis/core/IndexWriter.java index 054ca9c27..cc4ff0df5 100644 --- a/src/main/java/org/springframework/data/redis/core/IndexWriter.java +++ b/src/main/java/org/springframework/data/redis/core/IndexWriter.java @@ -76,6 +76,16 @@ class IndexWriter { * @param indexValues can be {@literal null}. */ public void updateIndexes(Object key, Iterable indexValues) { + createOrUpdateIndexes(key, indexValues, IndexWriteMode.PARTIAL_UPDATE); + } + + /** + * Updates indexes by first removing key from existing one and then persisting new index data. + * + * @param key must not be {@literal null}. + * @param indexValues can be {@literal null}. + */ + public void deleteAndUpdateIndexes(Object key, Iterable indexValues) { createOrUpdateIndexes(key, indexValues, IndexWriteMode.UPDATE); } @@ -96,7 +106,7 @@ class IndexWriter { removeKeyFromIndexes(data.getKeyspace(), binKey); } } - + } else if (ObjectUtils.nullSafeEquals(IndexWriteMode.PARTIAL_UPDATE, writeMode)) { removeKeyFromExistingIndexes(binKey, indexValues); } @@ -229,6 +239,6 @@ class IndexWriter { */ private static enum IndexWriteMode { - CREATE, UPDATE + CREATE, UPDATE, PARTIAL_UPDATE } } 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 70f5e6384..5a1e22186 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java +++ b/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java @@ -236,7 +236,7 @@ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter if (isNew) { indexWriter.createIndexes(key, rdo.getIndexedData()); } else { - indexWriter.updateIndexes(key, rdo.getIndexedData()); + indexWriter.deleteAndUpdateIndexes(key, rdo.getIndexedData()); } return null; } 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 8a1f86e61..a914bdf07 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisKeyValueTemplate.java +++ b/src/main/java/org/springframework/data/redis/core/RedisKeyValueTemplate.java @@ -23,7 +23,6 @@ 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.redis.RedisSystemException; import org.springframework.data.redis.core.mapping.RedisMappingContext; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -121,6 +120,7 @@ public class RedisKeyValueTemplate extends KeyValueTemplate { if (objectToInsert instanceof PartialUpdate) { doPartialUpdate((PartialUpdate) objectToInsert); + return; } super.insert(id, objectToInsert); @@ -135,6 +135,7 @@ public class RedisKeyValueTemplate extends KeyValueTemplate { if (objectToUpdate instanceof PartialUpdate) { doPartialUpdate((PartialUpdate) objectToUpdate); + return; } super.update(objectToUpdate); diff --git a/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterUnitTests.java b/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterUnitTests.java index a91920af9..8bdfbfe18 100644 --- a/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterUnitTests.java +++ b/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterUnitTests.java @@ -19,7 +19,6 @@ package org.springframework.data.redis.core; import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; import static org.mockito.Matchers.*; -import static org.mockito.Matchers.any; import static org.mockito.Mockito.*; import static org.springframework.test.util.ReflectionTestUtils.*; @@ -98,6 +97,7 @@ public class RedisKeyValueAdapterUnitTests { /** * @see DATAREDIS-512 + * @see DATAREDIS-530 */ @Test public void putShouldRemoveExistingIndexValuesWhenUpdating() { @@ -105,13 +105,14 @@ public class RedisKeyValueAdapterUnitTests { RedisData rd = new RedisData(Bucket.newBucketFromStringMap(Collections.singletonMap("_id", "1"))); rd.addIndexedData(new SimpleIndexedPropertyValue("persons", "firstname", "rand")); - when(redisConnectionMock.keys(any(byte[].class))) + when(redisConnectionMock.sMembers(org.mockito.Matchers.any(byte[].class))) .thenReturn(new LinkedHashSet(Arrays.asList("persons:firstname:rand".getBytes()))); when(redisConnectionMock.del((byte[][]) anyVararg())).thenReturn(1L); adapter.put("1", rd, "persons"); - verify(redisConnectionMock, times(1)).sRem(any(byte[].class), any(byte[].class)); + verify(redisConnectionMock, times(1)).sRem(org.mockito.Matchers.any(byte[].class), + org.mockito.Matchers.any(byte[].class)); } /** @@ -123,20 +124,20 @@ public class RedisKeyValueAdapterUnitTests { RedisData rd = new RedisData(Bucket.newBucketFromStringMap(Collections.singletonMap("_id", "1"))); rd.addIndexedData(new SimpleIndexedPropertyValue("persons", "firstname", "rand")); - when(redisConnectionMock.sMembers(any(byte[].class))) + when(redisConnectionMock.sMembers(org.mockito.Matchers.any(byte[].class))) .thenReturn(new LinkedHashSet(Arrays.asList("persons:firstname:rand".getBytes()))); when(redisConnectionMock.del((byte[][]) anyVararg())).thenReturn(0L); adapter.put("1", rd, "persons"); - verify(redisConnectionMock, never()).sRem(any(byte[].class), (byte[][]) anyVararg()); + verify(redisConnectionMock, never()).sRem(org.mockito.Matchers.any(byte[].class), (byte[][]) anyVararg()); } /** * @see DATAREDIS-491 */ @Test - public void shouldInitKeyExpirationListenerOnStartup() throws Exception{ + public void shouldInitKeyExpirationListenerOnStartup() throws Exception { adapter.destroy(); 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 a01ebd0c1..35aa506b4 100644 --- a/src/test/java/org/springframework/data/redis/core/RedisKeyValueTemplateTests.java +++ b/src/test/java/org/springframework/data/redis/core/RedisKeyValueTemplateTests.java @@ -229,7 +229,7 @@ public class RedisKeyValueTemplateTests { Person update1 = new Person(rand.id, null, "al-thor"); PartialUpdate update = new PartialUpdate(rand.id, update1); - template.doPartialUpdate(update); + template.insert(update); assertThat(template.findById(rand.id, Person.class), is(equalTo(new Person(rand.id, "rand", "al-thor")))); nativeTemplate.execute(new RedisCallback() { @@ -251,7 +251,7 @@ public class RedisKeyValueTemplateTests { */ update = new PartialUpdate(rand.id, Person.class).set("firstname", "frodo"); - template.doPartialUpdate(update); + template.update(update); assertThat(template.findById(rand.id, Person.class), is(equalTo(new Person(rand.id, "frodo", "al-thor")))); nativeTemplate.execute(new RedisCallback() { @@ -790,6 +790,81 @@ public class RedisKeyValueTemplateTests { }); } + /** + * @see DATAREDIS-530 + */ + @Test + public void partialUpdateShouldLeaveIndexesNotInvolvedInUpdateUntouched() { + + final Person rand = new Person(); + rand.firstname = "rand"; + rand.lastname = "al-thor"; + rand.email = "rand@twof.com"; + + template.insert(rand); + + /* + * Set the lastname and make sure we've an index on it afterwards + */ + PartialUpdate update = PartialUpdate.newPartialUpdate(rand.id, Person.class).set("lastname", "doe"); + + template.doPartialUpdate(update); + + nativeTemplate.execute(new RedisCallback() { + + @Override + public Void doInRedis(RedisConnection connection) throws DataAccessException { + + assertThat(connection.hGet(("template-test-person:" + rand.id).getBytes(), "lastname".getBytes()), + is(equalTo("doe".getBytes()))); + assertThat(connection.exists("template-test-person:email:rand@twof.com".getBytes()), is(true)); + assertThat(connection.exists("template-test-person:lastname:al-thor".getBytes()), is(false)); + assertThat(connection.sIsMember("template-test-person:lastname:doe".getBytes(), rand.id.getBytes()), is(true)); + return null; + } + }); + } + + /** + * @see DATAREDIS-530 + */ + @Test + public void updateShouldAlterIndexesCorrectlyWhenValuesGetRemovedFromHash() { + + final Person rand = new Person(); + rand.firstname = "rand"; + rand.lastname = "al-thor"; + rand.email = "rand@twof.com"; + + template.insert(rand); + + nativeTemplate.execute(new RedisCallback() { + + @Override + public Void doInRedis(RedisConnection connection) throws DataAccessException { + + assertThat(connection.exists("template-test-person:email:rand@twof.com".getBytes()), is(true)); + assertThat(connection.exists("template-test-person:lastname:al-thor".getBytes()), is(true)); + return null; + } + }); + + rand.lastname = null; + + template.update(rand); + + nativeTemplate.execute(new RedisCallback() { + + @Override + public Void doInRedis(RedisConnection connection) throws DataAccessException { + + assertThat(connection.exists("template-test-person:email:rand@twof.com".getBytes()), is(true)); + assertThat(connection.exists("template-test-person:lastname:al-thor".getBytes()), is(false)); + return null; + } + }); + } + @EqualsAndHashCode @RedisHash("template-test-type-mapping") static class VariousTypes { @@ -828,6 +903,7 @@ public class RedisKeyValueTemplateTests { @Id String id; String firstname; @Indexed String lastname; + @Indexed String email; Integer age; List nicknames;