Skip to content

Commit 46f07d7

Browse files
christophstroblmp911de
christophstrobl
authored and
mp911de
committed
DATAREDIS-512 - Skip repository index update checks on insert.
We now skip checking for existing index values on insert. Related tickets: DATAREDIS-504. Original pull request: #198.
1 parent f732b5f commit 46f07d7

File tree

5 files changed

+177
-23
lines changed

5 files changed

+177
-23
lines changed

‎src/main/java/org/springframework/data/redis/core/IndexWriter.java

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.springframework.data.redis.util.ByteUtils;
2626
import org.springframework.util.Assert;
2727
import org.springframework.util.CollectionUtils;
28+
import org.springframework.util.ObjectUtils;
2829

2930
/**
3031
* {@link IndexWriter} takes care of writing <a href="http://redis.io/topics/indexes">secondary index</a> structures to
@@ -57,13 +58,27 @@ public IndexWriter(RedisConnection connection, RedisConverter converter) {
5758
this.converter = converter;
5859
}
5960

61+
/**
62+
* Initially creates indexes.
63+
*
64+
* @param key must not be {@literal null}.
65+
* @param indexValues can be {@literal null}.
66+
*/
67+
public void createIndexes(Object key, Iterable<IndexedData> indexValues) {
68+
createOrUpdateIndexes(key, indexValues, IndexWriteMode.CREATE);
69+
}
70+
6071
/**
6172
* Updates indexes by first removing key from existing one and then persisting new index data.
62-
*
73+
*
6374
* @param key must not be {@literal null}.
6475
* @param indexValues can be {@literal null}.
6576
*/
6677
public void updateIndexes(Object key, Iterable<IndexedData> indexValues) {
78+
createOrUpdateIndexes(key, indexValues, IndexWriteMode.UPDATE);
79+
}
80+
81+
private void createOrUpdateIndexes(Object key, Iterable<IndexedData> indexValues, IndexWriteMode writeMode) {
6782

6883
Assert.notNull(key, "Key must not be null!");
6984
if (indexValues == null) {
@@ -72,7 +87,18 @@ public void updateIndexes(Object key, Iterable<IndexedData> indexValues) {
7287

7388
byte[] binKey = toBytes(key);
7489

75-
removeKeyFromExistingIndexes(binKey, indexValues);
90+
if (ObjectUtils.nullSafeEquals(IndexWriteMode.UPDATE, writeMode)) {
91+
92+
if (indexValues.iterator().hasNext()) {
93+
IndexedData data = indexValues.iterator().next();
94+
if (data != null && data.getKeyspace() != null) {
95+
removeKeyFromIndexes(data.getKeyspace(), binKey);
96+
}
97+
}
98+
99+
removeKeyFromExistingIndexes(binKey, indexValues);
100+
}
101+
76102
addKeyToIndexes(binKey, indexValues);
77103
}
78104

@@ -123,8 +149,8 @@ private void removeKeyFromExistingIndexes(byte[] key, Iterable<IndexedData> inde
123149
protected void removeKeyFromExistingIndexes(byte[] key, IndexedData indexedData) {
124150

125151
Assert.notNull(indexedData, "IndexedData must not be null!");
126-
Set<byte[]> existingKeys = connection.keys(toBytes(indexedData.getKeyspace() + ":" + indexedData.getIndexName()
127-
+ ":*"));
152+
Set<byte[]> existingKeys = connection
153+
.keys(toBytes(indexedData.getKeyspace() + ":" + indexedData.getIndexName() + ":*"));
128154

129155
if (!CollectionUtils.isEmpty(existingKeys)) {
130156
for (byte[] existingKey : existingKeys) {
@@ -166,8 +192,8 @@ protected void addKeyToIndex(byte[] key, IndexedData indexedData) {
166192
// keep track of indexes used for the object
167193
connection.sAdd(ByteUtils.concatAll(toBytes(indexedData.getKeyspace() + ":"), key, toBytes(":idx")), indexKey);
168194
} else {
169-
throw new IllegalArgumentException(String.format("Cannot write index data for unknown index type %s",
170-
indexedData.getClass()));
195+
throw new IllegalArgumentException(
196+
String.format("Cannot write index data for unknown index type %s", indexedData.getClass()));
171197
}
172198
}
173199

@@ -185,10 +211,17 @@ private byte[] toBytes(Object source) {
185211
return converter.getConversionService().convert(source, byte[].class);
186212
}
187213

188-
throw new InvalidDataAccessApiUsageException(
189-
String
190-
.format(
191-
"Cannot convert %s to binary representation for index key generation. Are you missing a Converter? Did you register a non PathBasedRedisIndexDefinition that might apply to a complex type?",
192-
source.getClass()));
214+
throw new InvalidDataAccessApiUsageException(String.format(
215+
"Cannot convert %s to binary representation for index key generation. Are you missing a Converter? Did you register a non PathBasedRedisIndexDefinition that might apply to a complex type?",
216+
source.getClass()));
217+
}
218+
219+
/**
220+
* @author Christoph Strobl
221+
* @since 1.8
222+
*/
223+
private static enum IndexWriteMode {
224+
225+
CREATE, UPDATE
193226
}
194227
}

‎src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.slf4j.Logger;
2626
import org.slf4j.LoggerFactory;
2727
import org.springframework.beans.BeansException;
28-
import org.springframework.beans.factory.DisposableBean;
2928
import org.springframework.context.ApplicationContext;
3029
import org.springframework.context.ApplicationContextAware;
3130
import org.springframework.context.ApplicationListener;
@@ -38,7 +37,6 @@
3837
import org.springframework.data.redis.connection.Message;
3938
import org.springframework.data.redis.connection.MessageListener;
4039
import org.springframework.data.redis.connection.RedisConnection;
41-
import org.springframework.data.redis.connection.RedisConnectionFactory;
4240
import org.springframework.data.redis.core.convert.CustomConversions;
4341
import org.springframework.data.redis.core.convert.KeyspaceConfiguration;
4442
import org.springframework.data.redis.core.convert.MappingRedisConverter;
@@ -164,8 +162,7 @@ public RedisKeyValueAdapter(RedisOperations<?, ?> redisOps, RedisConverter redis
164162
/**
165163
* Default constructor.
166164
*/
167-
protected RedisKeyValueAdapter() {
168-
}
165+
protected RedisKeyValueAdapter() {}
169166

170167
/*
171168
* (non-Javadoc)
@@ -198,7 +195,7 @@ public Object doInRedis(RedisConnection connection) throws DataAccessException {
198195
byte[] key = toBytes(rdo.getId());
199196
byte[] objectKey = createKey(rdo.getKeyspace(), rdo.getId());
200197

201-
connection.del(objectKey);
198+
boolean isNew = connection.del(objectKey) == 0;
202199

203200
connection.hMSet(objectKey, rdo.getBucket().rawMap());
204201

@@ -215,7 +212,12 @@ public Object doInRedis(RedisConnection connection) throws DataAccessException {
215212

216213
connection.sAdd(toBytes(rdo.getKeyspace()), key);
217214

218-
new IndexWriter(connection, converter).updateIndexes(key, rdo.getIndexedData());
215+
IndexWriter indexWriter = new IndexWriter(connection, converter);
216+
if (isNew) {
217+
indexWriter.createIndexes(key, rdo.getIndexedData());
218+
} else {
219+
indexWriter.updateIndexes(key, rdo.getIndexedData());
220+
}
219221
return null;
220222
}
221223
});

‎src/test/java/org/springframework/data/redis/core/IndexWriterUnitTests.java

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import java.nio.charset.Charset;
2424
import java.util.Arrays;
25+
import java.util.Collections;
2526
import java.util.LinkedHashSet;
2627

2728
import org.junit.Before;
@@ -110,8 +111,8 @@ public void removeKeyFromExistingIndexesShouldRemoveKeyFromAllExistingIndexesFor
110111
byte[] indexKey1 = "persons:firstname:rand".getBytes(CHARSET);
111112
byte[] indexKey2 = "persons:firstname:mat".getBytes(CHARSET);
112113

113-
when(connectionMock.keys(any(byte[].class))).thenReturn(
114-
new LinkedHashSet<byte[]>(Arrays.asList(indexKey1, indexKey2)));
114+
when(connectionMock.keys(any(byte[].class)))
115+
.thenReturn(new LinkedHashSet<byte[]>(Arrays.asList(indexKey1, indexKey2)));
115116

116117
writer.removeKeyFromExistingIndexes(KEY_BIN, new StubIndxedData());
117118

@@ -136,8 +137,8 @@ public void removeAllIndexesShouldDeleteAllIndexKeys() {
136137
byte[] indexKey1 = "persons:firstname:rand".getBytes(CHARSET);
137138
byte[] indexKey2 = "persons:firstname:mat".getBytes(CHARSET);
138139

139-
when(connectionMock.keys(any(byte[].class))).thenReturn(
140-
new LinkedHashSet<byte[]>(Arrays.asList(indexKey1, indexKey2)));
140+
when(connectionMock.keys(any(byte[].class)))
141+
.thenReturn(new LinkedHashSet<byte[]>(Arrays.asList(indexKey1, indexKey2)));
141142

142143
writer.removeAllIndexes(KEYSPACE);
143144

@@ -178,6 +179,42 @@ public byte[] convert(DummyObject source) {
178179
verify(connectionMock).sAdd(eq(("persons:firstname:" + identityHexString).getBytes(CHARSET)), eq(KEY_BIN));
179180
}
180181

182+
/**
183+
* @see DATAREDIS-512
184+
*/
185+
@Test
186+
public void createIndexShouldNotTryToRemoveExistingValues() {
187+
188+
when(connectionMock.keys(any(byte[].class)))
189+
.thenReturn(new LinkedHashSet<byte[]>(Arrays.asList("persons:firstname:rand".getBytes(CHARSET))));
190+
191+
writer.createIndexes(KEY_BIN,
192+
Collections.<IndexedData> singleton(new SimpleIndexedPropertyValue(KEYSPACE, "firstname", "Rand")));
193+
194+
verify(connectionMock).sAdd(eq("persons:firstname:Rand".getBytes(CHARSET)), eq(KEY_BIN));
195+
verify(connectionMock).sAdd(eq("persons:key-1:idx".getBytes(CHARSET)),
196+
eq("persons:firstname:Rand".getBytes(CHARSET)));
197+
verify(connectionMock, never()).sRem(any(byte[].class), eq(KEY_BIN));
198+
}
199+
200+
/**
201+
* @see DATAREDIS-512
202+
*/
203+
@Test
204+
public void updateIndexShouldRemoveExistingValues() {
205+
206+
when(connectionMock.keys(any(byte[].class)))
207+
.thenReturn(new LinkedHashSet<byte[]>(Arrays.asList("persons:firstname:rand".getBytes(CHARSET))));
208+
209+
writer.updateIndexes(KEY_BIN,
210+
Collections.<IndexedData> singleton(new SimpleIndexedPropertyValue(KEYSPACE, "firstname", "Rand")));
211+
212+
verify(connectionMock).sAdd(eq("persons:firstname:Rand".getBytes(CHARSET)), eq(KEY_BIN));
213+
verify(connectionMock).sAdd(eq("persons:key-1:idx".getBytes(CHARSET)),
214+
eq("persons:firstname:Rand".getBytes(CHARSET)));
215+
verify(connectionMock, times(1)).sRem(any(byte[].class), eq(KEY_BIN));
216+
}
217+
181218
static class StubIndxedData implements IndexedData {
182219

183220
@Override

‎src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ public void setUp() {
6464
template = new StringRedisTemplate(connectionFactory);
6565
template.afterPropertiesSet();
6666

67-
RedisMappingContext mappingContext = new RedisMappingContext(new MappingConfiguration(new IndexConfiguration(),
68-
new KeyspaceConfiguration()));
67+
RedisMappingContext mappingContext = new RedisMappingContext(
68+
new MappingConfiguration(new IndexConfiguration(), new KeyspaceConfiguration()));
6969
mappingContext.afterPropertiesSet();
7070

7171
adapter = new RedisKeyValueAdapter(template, mappingContext);
@@ -272,6 +272,45 @@ public void keyExpiredEventShouldRemoveHelperStructures() {
272272
assertThat(template.opsForSet().members("persons"), not(hasItem("1")));
273273
}
274274

275+
/**
276+
* @see DATAREDIS-512
277+
*/
278+
@Test
279+
public void putWritesIndexDataCorrectly() {
280+
281+
Person rand = new Person();
282+
rand.age = 24;
283+
rand.firstname = "rand";
284+
285+
adapter.put("rand", rand, "persons");
286+
287+
assertThat(template.hasKey("persons:firstname:rand"), is(true));
288+
assertThat(template.hasKey("persons:rand:idx"), is(true));
289+
assertThat(template.opsForSet().isMember("persons:rand:idx", "persons:firstname:rand"), is(true));
290+
291+
Person mat = new Person();
292+
mat.age = 22;
293+
mat.firstname = "mat";
294+
adapter.put("mat", mat, "persons");
295+
296+
assertThat(template.hasKey("persons:firstname:rand"), is(true));
297+
assertThat(template.hasKey("persons:firstname:mat"), is(true));
298+
assertThat(template.hasKey("persons:rand:idx"), is(true));
299+
assertThat(template.hasKey("persons:mat:idx"), is(true));
300+
assertThat(template.opsForSet().isMember("persons:rand:idx", "persons:firstname:rand"), is(true));
301+
assertThat(template.opsForSet().isMember("persons:mat:idx", "persons:firstname:mat"), is(true));
302+
303+
rand.firstname = "frodo";
304+
adapter.put("rand", rand, "persons");
305+
306+
assertThat(template.hasKey("persons:firstname:rand"), is(false));
307+
assertThat(template.hasKey("persons:firstname:mat"), is(true));
308+
assertThat(template.hasKey("persons:firstname:frodo"), is(true));
309+
assertThat(template.hasKey("persons:rand:idx"), is(true));
310+
assertThat(template.opsForSet().isMember("persons:rand:idx", "persons:firstname:frodo"), is(true));
311+
assertThat(template.opsForSet().isMember("persons:mat:idx", "persons:firstname:mat"), is(true));
312+
}
313+
275314
@KeySpace("persons")
276315
static class Person {
277316

‎src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterUnitTests.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@
1616

1717
package org.springframework.data.redis.core;
1818

19+
import static org.mockito.Matchers.*;
1920
import static org.mockito.Mockito.*;
2021

2122
import java.util.Arrays;
23+
import java.util.Collections;
24+
import java.util.LinkedHashSet;
2225

2326
import org.junit.Before;
2427
import org.junit.Test;
@@ -27,11 +30,15 @@
2730
import org.mockito.runners.MockitoJUnitRunner;
2831
import org.springframework.data.redis.connection.RedisConnection;
2932
import org.springframework.data.redis.connection.jedis.JedisConnectionFactory;
33+
import org.springframework.data.redis.core.convert.Bucket;
34+
import org.springframework.data.redis.core.convert.RedisData;
35+
import org.springframework.data.redis.core.convert.SimpleIndexedPropertyValue;
3036

3137
/**
3238
* Unit tests for {@link RedisKeyValueAdapter}.
3339
*
3440
* @author Mark Paluch
41+
* @author Christoph Strobl
3542
*/
3643
@RunWith(MockitoJUnitRunner.class)
3744
public class RedisKeyValueAdapterUnitTests {
@@ -66,4 +73,40 @@ public void destroyShouldNotDestroyConnectionFactory() throws Exception {
6673

6774
verify(jedisConnectionFactoryMock, never()).destroy();
6875
}
76+
77+
/**
78+
* @see DATAREDIS-512
79+
*/
80+
@Test
81+
public void putShouldRemoveExistingIndexValuesWhenUpdating() {
82+
83+
RedisData rd = new RedisData(Bucket.newBucketFromStringMap(Collections.singletonMap("_id", "1")));
84+
rd.addIndexedData(new SimpleIndexedPropertyValue("persons", "firstname", "rand"));
85+
86+
when(redisConnectionMock.keys(any(byte[].class)))
87+
.thenReturn(new LinkedHashSet<byte[]>(Arrays.asList("persons:firstname:rand".getBytes())));
88+
when(redisConnectionMock.del((byte[][]) anyVararg())).thenReturn(1L);
89+
90+
redisKeyValueAdapter.put("1", rd, "persons");
91+
92+
verify(redisConnectionMock, times(1)).sRem(any(byte[].class), any(byte[].class));
93+
}
94+
95+
/**
96+
* @see DATAREDIS-512
97+
*/
98+
@Test
99+
public void putShouldNotTryToRemoveExistingIndexValuesWhenInsertingNew() {
100+
101+
RedisData rd = new RedisData(Bucket.newBucketFromStringMap(Collections.singletonMap("_id", "1")));
102+
rd.addIndexedData(new SimpleIndexedPropertyValue("persons", "firstname", "rand"));
103+
104+
when(redisConnectionMock.sMembers(any(byte[].class)))
105+
.thenReturn(new LinkedHashSet<byte[]>(Arrays.asList("persons:firstname:rand".getBytes())));
106+
when(redisConnectionMock.del((byte[][]) anyVararg())).thenReturn(0L);
107+
108+
redisKeyValueAdapter.put("1", rd, "persons");
109+
110+
verify(redisConnectionMock, never()).sRem(any(byte[].class), (byte[][]) anyVararg());
111+
}
69112
}

0 commit comments

Comments
 (0)