DATAREDIS-530 - Fix PartialUpdate removing existing indexes.
We now make sure to leave existing indexes untouched when using PartialUpdate. We also fixed a glitch, where index values have not been removed correctly when saving entities with null values, along the way. Original pull request: #207.
This commit is contained in:
committed by
Mark Paluch
parent
0d24b5a573
commit
f5a506cb8d
@@ -76,6 +76,16 @@ class IndexWriter {
|
||||
* @param indexValues can be {@literal null}.
|
||||
*/
|
||||
public void updateIndexes(Object key, Iterable<IndexedData> 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<IndexedData> 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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<byte[]>(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<byte[]>(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();
|
||||
|
||||
|
||||
@@ -229,7 +229,7 @@ public class RedisKeyValueTemplateTests {
|
||||
Person update1 = new Person(rand.id, null, "al-thor");
|
||||
PartialUpdate<Person> update = new PartialUpdate<Person>(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<Void>() {
|
||||
@@ -251,7 +251,7 @@ public class RedisKeyValueTemplateTests {
|
||||
*/
|
||||
update = new PartialUpdate<Person>(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<Void>() {
|
||||
@@ -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<Person> update = PartialUpdate.newPartialUpdate(rand.id, Person.class).set("lastname", "doe");
|
||||
|
||||
template.doPartialUpdate(update);
|
||||
|
||||
nativeTemplate.execute(new RedisCallback<Void>() {
|
||||
|
||||
@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<Void>() {
|
||||
|
||||
@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<Void>() {
|
||||
|
||||
@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<String> nicknames;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user