From 40c2582302e8245e3babe5902b5f6fc3e7d448c0 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Mon, 14 Dec 2015 17:13:53 -0600 Subject: [PATCH] DATAREDIS-425 - Support distinct indexName This commit adds support for distinguishing between the property path that is being indexed and the name of the index that should be used. This is useful when you do not want the property path and index name to be the same. For example, if you want to use Spel expressions, it may not be possible to use the path as the name of the index. Original PR #160 --- .../data/redis/core/IndexWriter.java | 12 +-- .../redis/core/convert/IndexResolverImpl.java | 10 ++- .../data/redis/core/convert/IndexedData.java | 13 ++-- .../convert/SimpleIndexedPropertyValue.java | 29 +++---- .../redis/core/index/IndexConfiguration.java | 26 +++++-- .../data/redis/core/IndexWriterUnitTests.java | 5 +- .../convert/IndexResolverImplUnitTests.java | 27 ++++++- .../index/IndexConfigurationUnitTests.java | 76 +++++++++++++++++++ 8 files changed, 160 insertions(+), 38 deletions(-) create mode 100644 src/test/java/org/springframework/data/redis/core/index/IndexConfigurationUnitTests.java 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 ad7e4a61f..2e2d6527f 100644 --- a/src/main/java/org/springframework/data/redis/core/IndexWriter.java +++ b/src/main/java/org/springframework/data/redis/core/IndexWriter.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2016 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. @@ -32,6 +32,7 @@ import org.springframework.util.CollectionUtils; * allows to remove the root key from all indexes in case of deletion. * * @author Christoph Strobl + * @author Rob Winch * @since 1.7 */ class IndexWriter { @@ -113,7 +114,7 @@ class IndexWriter { } /** - * Remove given key from all indexes matching {@link IndexedData#getPath()}: + * Remove given key from all indexes matching {@link IndexedData#getIndexName()}: * * @param key * @param indexedData @@ -121,7 +122,8 @@ class IndexWriter { protected void removeKeyFromExistingIndexes(byte[] key, IndexedData indexedData) { Assert.notNull(indexedData, "IndexedData must not be null!"); - Set existingKeys = connection.keys(toBytes(indexedData.getKeySpace() + ":" + indexedData.getPath() + ":*")); + Set existingKeys = connection.keys(toBytes(indexedData.getKeySpace() + ":" + indexedData.getIndexName() + + ":*")); if (!CollectionUtils.isEmpty(existingKeys)) { for (byte[] existingKey : existingKeys) { @@ -138,7 +140,7 @@ class IndexWriter { } /** - * Adds a given key to the index for {@link IndexedData#getPath()}. + * Adds a given key to the index for {@link IndexedData#getIndexName()}. * * @param key must not be {@literal null}. * @param indexedData must not be {@literal null}. @@ -156,7 +158,7 @@ class IndexWriter { return; } - byte[] indexKey = toBytes(indexedData.getKeySpace() + ":" + indexedData.getPath() + ":"); + byte[] indexKey = toBytes(indexedData.getKeySpace() + ":" + indexedData.getIndexName() + ":"); indexKey = ByteUtils.concat(indexKey, toBytes(value)); connection.sAdd(indexKey, key); diff --git a/src/main/java/org/springframework/data/redis/core/convert/IndexResolverImpl.java b/src/main/java/org/springframework/data/redis/core/convert/IndexResolverImpl.java index f3eba6f5c..a08e0323b 100644 --- a/src/main/java/org/springframework/data/redis/core/convert/IndexResolverImpl.java +++ b/src/main/java/org/springframework/data/redis/core/convert/IndexResolverImpl.java @@ -37,7 +37,7 @@ import org.springframework.util.Assert; /** * {@link IndexResolver} implementation considering properties annotated with {@link Indexed} or paths set up in * {@link IndexConfiguration}. - * + * * @author Christoph Strobl * @since 1.7 */ @@ -55,7 +55,7 @@ public class IndexResolverImpl implements IndexResolver { /** * Creates new {@link IndexResolverImpl} with given {@link IndexConfiguration}. - * + * * @param mapppingContext must not be {@literal null}. */ public IndexResolverImpl(RedisMappingContext mappingContext) { @@ -166,7 +166,11 @@ public class IndexResolverImpl implements IndexResolver { String path = normalizeIndexPath(propertyPath, property); if (indexConfiguration.hasIndexFor(keyspace, path)) { - return new SimpleIndexedPropertyValue(keyspace, path, value); + // FIXME it seems there is a mis-match between IndexConfiguration + // resolving many RedisIndexSetting objects to resolving a single + // IndexData in this method. + RedisIndexSetting indexSetting = indexConfiguration.getIndexDefinitionsFor(keyspace, path).iterator().next(); + return new SimpleIndexedPropertyValue(keyspace, indexSetting.getIndexName(), value); } else if (property != null && property.isAnnotationPresent(Indexed.class)) { diff --git a/src/main/java/org/springframework/data/redis/core/convert/IndexedData.java b/src/main/java/org/springframework/data/redis/core/convert/IndexedData.java index 04e17f511..9a19f79ff 100644 --- a/src/main/java/org/springframework/data/redis/core/convert/IndexedData.java +++ b/src/main/java/org/springframework/data/redis/core/convert/IndexedData.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2016 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. @@ -17,22 +17,23 @@ package org.springframework.data.redis.core.convert; /** * {@link IndexedData} represents a secondary index for a property path in a given keyspace. - * + * * @author Christoph Strobl + * @author Rob Winch * @since 1.7 */ public interface IndexedData { /** - * Get the {@link String} representation of the dot path to the referenced property. - * + * Get the {@link String} representation of the index name. + * * @return never {@literal null}. */ - String getPath(); + String getIndexName(); /** * Get the associated keyspace the index resides in. - * + * * @return */ String getKeySpace(); diff --git a/src/main/java/org/springframework/data/redis/core/convert/SimpleIndexedPropertyValue.java b/src/main/java/org/springframework/data/redis/core/convert/SimpleIndexedPropertyValue.java index 84f9b3204..a1ebf841b 100644 --- a/src/main/java/org/springframework/data/redis/core/convert/SimpleIndexedPropertyValue.java +++ b/src/main/java/org/springframework/data/redis/core/convert/SimpleIndexedPropertyValue.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2016 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. @@ -19,33 +19,34 @@ import org.springframework.util.ObjectUtils; /** * {@link IndexedData} implementation indicating storage of data within a Redis Set. - * + * * @author Christoph Strobl + * @author Rob Winch * @since 1.7 */ public class SimpleIndexedPropertyValue implements IndexedData { private final String keyspace; - private final String path; + private final String indexName; private final Object value; /** * Creates new {@link SimpleIndexedPropertyValue}. - * + * * @param keyspace must not be {@literal null}. - * @param path must not be {@literal null}. + * @param indexName must not be {@literal null}. * @param value can be {@literal null}. */ - public SimpleIndexedPropertyValue(String keyspace, String path, Object value) { + public SimpleIndexedPropertyValue(String keyspace, String indexName, Object value) { this.keyspace = keyspace; - this.path = path; + this.indexName = indexName; this.value = value; } /** * Get the value to index. - * + * * @return can be {@literal null}. */ public Object getValue() { @@ -54,11 +55,11 @@ public class SimpleIndexedPropertyValue implements IndexedData { /* * (non-Javadoc) - * @see org.springframework.data.redis.core.convert.IndexedData#getPath() + * @see org.springframework.data.redis.core.convert.IndexedData#getIndexName() */ @Override - public String getPath() { - return path; + public String getIndexName() { + return indexName; } /* @@ -79,7 +80,7 @@ public class SimpleIndexedPropertyValue implements IndexedData { int result = 1; result += ObjectUtils.nullSafeHashCode(keyspace); - result += ObjectUtils.nullSafeHashCode(path); + result += ObjectUtils.nullSafeHashCode(indexName); result += ObjectUtils.nullSafeHashCode(value); return result; } @@ -105,7 +106,7 @@ public class SimpleIndexedPropertyValue implements IndexedData { if (!ObjectUtils.nullSafeEquals(this.keyspace, that.keyspace)) { return false; } - if (!ObjectUtils.nullSafeEquals(this.path, that.path)) { + if (!ObjectUtils.nullSafeEquals(this.indexName, that.indexName)) { return false; } return ObjectUtils.nullSafeEquals(this.value, that.value); @@ -117,7 +118,7 @@ public class SimpleIndexedPropertyValue implements IndexedData { */ @Override public String toString() { - return "SimpleIndexedPropertyValue [keyspace=" + keyspace + ", path=" + path + ", value=" + value + "]"; + return "SimpleIndexedPropertyValue [keyspace=" + keyspace + ", indexName=" + indexName + ", value=" + value + "]"; } } diff --git a/src/main/java/org/springframework/data/redis/core/index/IndexConfiguration.java b/src/main/java/org/springframework/data/redis/core/index/IndexConfiguration.java index 43a29981b..ff59472cb 100644 --- a/src/main/java/org/springframework/data/redis/core/index/IndexConfiguration.java +++ b/src/main/java/org/springframework/data/redis/core/index/IndexConfiguration.java @@ -28,8 +28,9 @@ import org.springframework.util.ObjectUtils; /** * {@link IndexConfiguration} allows programmatic setup of indexes. This is suitable for cases where there is no option * to use the equivalent {@link Indexed} annotation. - * + * * @author Christoph Strobl + * @author Rob Winch * @since 1.7 */ public class IndexConfiguration { @@ -40,6 +41,7 @@ public class IndexConfiguration { * Creates new empty {@link IndexConfiguration}. */ public IndexConfiguration() { + this.definitions = new CopyOnWriteArraySet(); for (RedisIndexSetting initial : initialConfiguration()) { addIndexDefinition(initial); @@ -48,7 +50,7 @@ public class IndexConfiguration { /** * Checks if an index is defined for a given keyspace and property path. - * + * * @param keyspace * @param path * @return true if index is defined. @@ -66,7 +68,7 @@ public class IndexConfiguration { /** * Get the list of {@link RedisIndexSetting} for a given keyspace and property path. - * + * * @param keyspace * @param path * @return never {@literal null}. @@ -86,7 +88,7 @@ public class IndexConfiguration { /** * Add given {@link RedisIndexSetting}. - * + * * @param indexDefinition must not be {@literal null}. */ public void addIndexDefinition(RedisIndexSetting indexDefinition) { @@ -108,7 +110,7 @@ public class IndexConfiguration { /** * Customization hook. - * + * * @return must not return {@literal null}. */ protected Iterable initialConfiguration() { @@ -117,12 +119,14 @@ public class IndexConfiguration { /** * @author Christoph Strobl + * @author Rob Winch * @since 1.7 */ public static class RedisIndexSetting { private final Serializable keyspace; private final String path; + private final String indexName; private final IndexType type; public RedisIndexSetting(Serializable keyspace, String path) { @@ -130,12 +134,20 @@ public class IndexConfiguration { } public RedisIndexSetting(Serializable keyspace, String path, IndexType type) { + this(keyspace, path, path, type); + } + public RedisIndexSetting(Serializable keyspace, String path, String indexName, IndexType type) { this.keyspace = keyspace; this.path = path; + this.indexName = indexName; this.type = type == null ? IndexType.SIMPLE : type; } + public String getIndexName() { + return indexName; + } + public Serializable getKeyspace() { return keyspace; } @@ -153,6 +165,7 @@ public class IndexConfiguration { int result = ObjectUtils.nullSafeHashCode(keyspace); result += ObjectUtils.nullSafeHashCode(path); + result += ObjectUtils.nullSafeHashCode(indexName); result += ObjectUtils.nullSafeHashCode(type); return result; } @@ -178,6 +191,9 @@ public class IndexConfiguration { if (!ObjectUtils.nullSafeEquals(this.path, that.path)) { return false; } + if (!ObjectUtils.nullSafeEquals(this.indexName, that.indexName)) { + return false; + } if (!ObjectUtils.nullSafeEquals(this.type, that.type)) { return false; } diff --git a/src/test/java/org/springframework/data/redis/core/IndexWriterUnitTests.java b/src/test/java/org/springframework/data/redis/core/IndexWriterUnitTests.java index c995b992a..5a7ad9faf 100644 --- a/src/test/java/org/springframework/data/redis/core/IndexWriterUnitTests.java +++ b/src/test/java/org/springframework/data/redis/core/IndexWriterUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2016 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. @@ -40,6 +40,7 @@ import org.springframework.data.redis.core.mapping.RedisMappingContext; /** * @author Christoph Strobl + * @auhtor Rob Winch */ @RunWith(MockitoJUnitRunner.class) public class IndexWriterUnitTests { @@ -145,7 +146,7 @@ public class IndexWriterUnitTests { static class StubIndxedData implements IndexedData { @Override - public String getPath() { + public String getIndexName() { return "address.city"; } diff --git a/src/test/java/org/springframework/data/redis/core/convert/IndexResolverImplUnitTests.java b/src/test/java/org/springframework/data/redis/core/convert/IndexResolverImplUnitTests.java index 4147c4dd4..c37273170 100644 --- a/src/test/java/org/springframework/data/redis/core/convert/IndexResolverImplUnitTests.java +++ b/src/test/java/org/springframework/data/redis/core/convert/IndexResolverImplUnitTests.java @@ -296,7 +296,7 @@ public class IndexResolverImplUnitTests { IndexedData index = resolve("list.[0].name", "rand"); - assertThat(index.getPath(), is("list.name")); + assertThat(index.getIndexName(), is("list.name")); } /** @@ -311,7 +311,7 @@ public class IndexResolverImplUnitTests { IndexedData index = resolve("map.[foo].name", "rand"); - assertThat(index.getPath(), is("map.foo.name")); + assertThat(index.getIndexName(), is("map.foo.name")); } /** @@ -326,7 +326,7 @@ public class IndexResolverImplUnitTests { IndexedData index = resolve("map.[0].name", "rand"); - assertThat(index.getPath(), is("map.0.name")); + assertThat(index.getIndexName(), is("map.0.name")); } /** @@ -405,6 +405,27 @@ public class IndexResolverImplUnitTests { assertThat(indexes, hasItem(new SimpleIndexedPropertyValue(KEYSPACE_PERSON, "items.type", "hat"))); } + /** + * @see DATAREDIS-425 + */ + @Test + public void resolveIndexAllowCustomIndexName() { + indexConfig.addIndexDefinition(new RedisIndexSetting(KEYSPACE_PERSON, "items.type", "itemsType", IndexType.SIMPLE)); + + Item hat = new Item(); + hat.type = "hat"; + + TaVeren mat = new TaVeren(); + mat.items = new ArrayList(2); + mat.items.add(hat); + mat.items.add("foxhead medallion"); + + Set indexes = indexResolver.resolveIndexesFor(ClassTypeInformation.from(TaVeren.class), mat); + + assertThat(indexes.size(), is(1)); + assertThat(indexes, hasItem(new SimpleIndexedPropertyValue(KEYSPACE_PERSON, "itemsType", "hat"))); + } + private IndexedData resolve(String path, Object value) { return indexResolver.resolveIndex(KEYSPACE_PERSON, path, propertyMock, value); } diff --git a/src/test/java/org/springframework/data/redis/core/index/IndexConfigurationUnitTests.java b/src/test/java/org/springframework/data/redis/core/index/IndexConfigurationUnitTests.java new file mode 100644 index 000000000..584b5d13b --- /dev/null +++ b/src/test/java/org/springframework/data/redis/core/index/IndexConfigurationUnitTests.java @@ -0,0 +1,76 @@ +/* + * Copyright 2015-2016 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.redis.core.index; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + +import org.junit.Test; +import org.springframework.data.redis.core.index.IndexConfiguration.RedisIndexSetting; + +/** + * @author Rob Winch + */ +public class IndexConfigurationUnitTests { + + /** + * @see DATAREDIS-425 + */ + @Test + public void redisIndexSettingIndexNameDefaulted() { + + String path = "path"; + RedisIndexSetting setting = new RedisIndexSetting("keyspace", path); + assertThat(setting.getIndexName(), equalTo(path)); + } + + /** + * @see DATAREDIS-425 + */ + @Test + public void redisIndexSettingIndexNameExplicit() { + + String indexName = "indexName"; + RedisIndexSetting setting = new RedisIndexSetting("keyspace", "index", indexName, IndexType.SIMPLE); + assertThat(setting.getIndexName(), equalTo(indexName)); + } + + /** + * @see DATAREDIS-425 + */ + @Test + public void redisIndexSettingIndexNameUsedInEquals() { + + RedisIndexSetting setting1 = new RedisIndexSetting("keyspace", "path", "indexName1", IndexType.SIMPLE); + RedisIndexSetting setting2 = new RedisIndexSetting(setting1.getKeyspace(), setting1.getPath(), + setting1.getIndexName() + "other", setting1.getType()); + + assertThat(setting1, not(equalTo(setting2))); + } + + /** + * @see DATAREDIS-425 + */ + @Test + public void redisIndexSettingIndexNameUsedInHashCode() { + + RedisIndexSetting setting1 = new RedisIndexSetting("keyspace", "path", "indexName1", IndexType.SIMPLE); + RedisIndexSetting setting2 = new RedisIndexSetting(setting1.getKeyspace(), setting1.getPath(), + setting1.getIndexName() + "other", setting1.getType()); + + assertThat(setting1.hashCode(), not(equalTo(setting2.hashCode()))); + } +}