DATAREDIS-425 - Normalize index paths for maps and lists.

Remove map key and list index values for secondary index structures. Paths like 'map.[key].property' are normalized to map.key.property. Same applies for lists 'list.[index].property' is converted to 'list.property'.

To allow better control over index resolution the entire logic has been moved to IndexResolver now inspecting the whole type instead of single properties.

Author: christophstrobl

src/asciidoc/reference/redis-repositories.adoc

@@ -89,7 +89,7 @@ public void basicCrudOperations() {
   repo.delete(rand);                                       <4>
 }
 ----
-<1> Generates a new id if current values is `null` or reuses an already set id value and stores properties of type `Person` inside the Redis Hash with key with pattern `keyspace:id` in this case eg. `persons:5d67b7e1-8640-4475-beeb-c666fab4c0e5`.
+<1> Generates a new id if current value is `null` or reuses an already set id value and stores properties of type `Person` inside the Redis Hash with key with pattern `keyspace:id` in this case eg. `persons:5d67b7e1-8640-4475-beeb-c666fab4c0e5`.
 <2> Uses the provided id to retrieve the object stored at `keyspace:id`.
 <3> Counts the total number of entities available within the keyspace _persons_ defined by `@RedisHash` on `Person`.
 <4> Removes the key for the given object from Redis.
@@ -191,6 +191,28 @@ SADD persons:address.city:tear e2c7dcee-b8cd-4424-883e-736ce564363e
 ----
 ====
 
+Further more the programmatic setup allows to define indexes on map keys and list properties.
+
+====
+[source,java]
+----
+@RedisHash("persons");
+public class Person {
+
+  // ... other properties omitted
+  
+  Map<String,String> attributes;      <1>
+  Map<String Person> relatives;       <2>
+  List<Address> addresses;            <3>
+}
+----
+<1> `SADD persons:attributes.map-key:map-value e2c7dcee-b8cd-4424-883e-736ce564363e`
+<2> `SADD persons:relatives.map-key.firstname:tam e2c7dcee-b8cd-4424-883e-736ce564363e`
+<3> `SADD persons:addresses.city:tear e2c7dcee-b8cd-4424-883e-736ce564363e`
+====
+
+NOTE: Indexes will not be resolved on <<redis.repositories.references,References>>.
+
 Same as with _keyspaces_ it is possible to configure indexes without the need of annotating the actual domain type.
 
 .Index Setup via @EnableRedisRepositories
@@ -260,7 +282,7 @@ NOTE: The keyspace notification message listener will alter `notify-keyspace-eve
 
 [[redis.repositories.mapping]]
 == Object to hash mapping
-The Redis Repository support persists Objects in Hashes. This requires an Object to Hash conversion which is done by a `RedisConverter`. The default implementation uses `Converter`s for mapping property values to and from Redis native `byte[]`.
+The Redis Repository support persists Objects in Hashes. This requires an Object to Hash conversion which is done by a `RedisConverter`. The default implementation uses `Converter` for mapping property values to and from Redis native `byte[]`.
 
 Given the `Person` type from the previous sections the default mapping looks like the following:
 
@@ -286,29 +308,39 @@ address.country = andor
 | Sample
 | Mapped Value
 
-| Simple Type (eg. String)
+| Simple Type +
+(eg. String)
 | String firstname = "rand";
 | firstname = "rand"
 
-| Complex Type (eg. Address)
+| Complex Type +
+(eg. Address)
 | Address adress = new Address("emond's field");
 | address.city = "emond's field"
 
-| List of Simple Type
+| List +
+of Simple Type
 | List<String> nicknames = asList("dragon reborn", "lews therin");
-| nicknames.[0] = "dragon reborn", nicknames.[1] = "lews therin"
+| nicknames.[0] = "dragon reborn", +
+nicknames.[1] = "lews therin"
 
-| Map of Simple Type
+| Map +
+of Simple Type
 | Map<String, String> atts = asMap({"eye-color", "grey"}, {"...
-| atts.[eye-color] = "grey", atts.[...
+| atts.[eye-color] = "grey", +
+atts.[hair-color] = "...
 
-| List of Complex Type
+| List +
+of Complex Type
 | List<Address> addresses = asList(new Address("em...
-| addresses.[0].city = "emond's field", addresses.[1].city  = "...
+| addresses.[0].city = "emond's field", +
+addresses.[1].city  = "...
 
-| Map of Complex Type
+| Map +
+of Complex Type
 | Map<String, Address> addresses = asMap({"home", new Address("em...
-| addresses.[home].city = "emond's field", addresses.[work].city  = "...
+| addresses.[home].city = "emond's field", +
+addresses.[work].city  = "...
 |===
 
 Mapping behavior can be customized by registering the according `Converter`s in `CustomConversions`. Those Converters can take care of converting from/to a single `byte[]` as well as `Map<Sting,byte[]>` whereas the first one is suiteable for eg. converting one complex type to eg. a binary JSON representation that still uses the default mappings hash structure, whereas the second options offers full control over the resulting hash.
@@ -416,12 +448,14 @@ _class = org.example.Person
 id = e2c7dcee-b8cd-4424-883e-736ce564363e
 firstname = rand      
 lastname = al’thor
-mother = persons:a9d4b3a0-50d3-4538-a2fc-f7fc2581ee56
+mother = persons:a9d4b3a0-50d3-4538-a2fc-f7fc2581ee56      <1>
 ----
+<1> Reference stores the whole key (`keyspace:id`) of the referenced object.
 ====
 
 
-WARNING: Referenced Objects are not subject of persisting changes when saving the referencing object. Please make sure to persist changes on referenced objects separately, since only the reference will be stored.
+WARNING: Referenced Objects are not subject of persisting changes when saving the referencing object. Please make sure to persist changes on referenced objects separately, since only the reference will be stored. 
+Indexes set on properties of referenced types will not be resolved.
 
 [[redis.repositories.queries]]
 == Query Methods

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

@@ -132,7 +132,7 @@ public RedisKeyValueAdapter(RedisOperations<?, ?> redisOps, RedisMappingContext
 		Assert.notNull(mappingContext, "RedisMappingContext must not be null!");
 
 		MappingRedisConverter mappingConverter = new MappingRedisConverter(mappingContext, new IndexResolverImpl(
-				mappingContext.getMappingConfiguration().getIndexConfiguration()), new ReferenceResolverImpl(this));
+				mappingContext), new ReferenceResolverImpl(this));
 		mappingConverter.setCustomConversions(customConversions == null ? new CustomConversions() : customConversions);
 		mappingConverter.afterPropertiesSet();
 

src/main/java/org/springframework/data/redis/core/convert/CustomConversions.java

@@ -479,7 +479,7 @@ public boolean isSimpleTargetType() {
 		 * @return
 		 */
 		private static boolean isRedisBasicType(Class<?> type) {
-			return byte[].class.equals(type);
+			return (byte[].class.equals(type) || Map.class.equals(type));
 		}
 	}
 }

src/main/java/org/springframework/data/redis/core/convert/IndexResolver.java

@@ -15,7 +15,10 @@
  */
 package org.springframework.data.redis.core.convert;
 
+import java.util.Set;
+
 import org.springframework.data.mapping.PersistentProperty;
+import org.springframework.data.util.TypeInformation;
 
 /**
  * {@link IndexResolver} extracts secondary index structures to be applied on a given path, {@link PersistentProperty}
@@ -27,14 +30,12 @@
 public interface IndexResolver {
 
 	/**
-	 * Extracts a potential secondary index.
+	 * Resolves all indexes for given type information / value combination.
 	 * 
-	 * @param keyspace must not be {@literal null}.
-	 * @param path can be {@literal null}.
-	 * @param property must not be {@literal null}.
-	 * @param value can be {@literal null}.
-	 * @return {@literal null} if no index could be resolved for given arguments.
+	 * @param typeInformation must not be {@literal null}.
+	 * @param value the actual value. Can be {@literal null}.
+	 * @return never {@literal null}.
 	 */
-	IndexedData resolveIndex(String keyspace, String path, PersistentProperty<?> property, Object value);
+	Set<IndexedData> resolveIndexesFor(TypeInformation<?> typeInformation, Object value);
 
 }

src/main/java/org/springframework/data/redis/core/convert/IndexResolverImpl.java

@@ -15,10 +15,24 @@
  */
 package org.springframework.data.redis.core.convert;
 
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+
+import org.springframework.data.keyvalue.core.mapping.KeyValuePersistentProperty;
 import org.springframework.data.mapping.PersistentProperty;
+import org.springframework.data.mapping.PersistentPropertyAccessor;
+import org.springframework.data.mapping.PropertyHandler;
 import org.springframework.data.redis.core.index.IndexConfiguration;
 import org.springframework.data.redis.core.index.IndexConfiguration.RedisIndexSetting;
 import org.springframework.data.redis.core.index.Indexed;
+import org.springframework.data.redis.core.mapping.RedisMappingContext;
+import org.springframework.data.redis.core.mapping.RedisPersistentEntity;
+import org.springframework.data.util.ClassTypeInformation;
+import org.springframework.data.util.TypeInformation;
+import org.springframework.util.Assert;
 
 /**
  * {@link IndexResolver} implementation considering properties annotated with {@link Indexed} or paths set up in
@@ -30,36 +44,132 @@
 public class IndexResolverImpl implements IndexResolver {
 
 	private IndexConfiguration indexConfiguration;
+	private RedisMappingContext mappingContext;
 
 	/**
 	 * Creates new {@link IndexResolverImpl} with empty {@link IndexConfiguration}.
 	 */
 	public IndexResolverImpl() {
-		this(new IndexConfiguration());
+		this(new RedisMappingContext());
 	}
 
 	/**
 	 * Creates new {@link IndexResolverImpl} with given {@link IndexConfiguration}.
 	 * 
-	 * @param indexConfiguration can be {@literal null} and will be defaulted to an empty {@link IndexConfiguration} if
-	 *          so.
+	 * @param mapppingContext must not be {@literal null}.
 	 */
-	public IndexResolverImpl(IndexConfiguration indexConfiguration) {
-		this.indexConfiguration = indexConfiguration != null ? indexConfiguration : new IndexConfiguration();
+	public IndexResolverImpl(RedisMappingContext mappingContext) {
+
+		Assert.notNull(mappingContext, "MappingContext must not be null!");
+		this.mappingContext = mappingContext;
+		this.indexConfiguration = mappingContext.getMappingConfiguration().getIndexConfiguration();
 	}
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.springframework.data.redis.core.convert.IndexResolver#resolveIndex(java.lang.String, java.lang.String, org.springframework.data.keyvalue.core.mapping.KeyValuePersistentProperty, java.lang.Object)
+	 * @see org.springframework.data.redis.core.convert.IndexResolver#resolveIndexesFor(org.springframework.data.util.TypeInformation, java.lang.Object)
 	 */
-	@Override
-	public IndexedData resolveIndex(String keyspace, String path, PersistentProperty<?> property, Object value) {
+	public Set<IndexedData> resolveIndexesFor(TypeInformation<?> typeInformation, Object value) {
+		return doResolveIndexesFor(mappingContext.getPersistentEntity(typeInformation).getKeySpace(), "", typeInformation,
+				value);
+	}
+
+	private Set<IndexedData> doResolveIndexesFor(final String keyspace, final String path,
+			TypeInformation<?> typeInformation, Object value) {
+
+		RedisPersistentEntity<?> entity = mappingContext.getPersistentEntity(typeInformation);
+
+		if (entity == null) {
+
+			IndexedData index = resolveIndex(keyspace, path, null, value);
+			if (index != null) {
+				return Collections.singleton(index);
+			}
+			return Collections.emptySet();
+		}
+
+		final PersistentPropertyAccessor accessor = entity.getPropertyAccessor(value);
+		final Set<IndexedData> indexes = new LinkedHashSet<IndexedData>();
+
+		entity.doWithProperties(new PropertyHandler<KeyValuePersistentProperty>() {
+
+			@Override
+			public void doWithPersistentProperty(KeyValuePersistentProperty persistentProperty) {
+
+				String currentPath = !path.isEmpty() ? path + "." + persistentProperty.getName() : persistentProperty.getName();
+
+				Object propertyValue = accessor.getProperty(persistentProperty);
+
+				if (propertyValue != null) {
+
+					TypeInformation<?> typeHint = persistentProperty.isMap() ? persistentProperty.getTypeInformation()
+							.getMapValueType() : persistentProperty.getTypeInformation().getActualType();
+
+					IndexedData index = resolveIndex(keyspace, currentPath, persistentProperty, propertyValue);
+
+					if (index != null) {
+						indexes.add(index);
+					}
+
+					if (persistentProperty.isMap()) {
+
+						for (Entry<?, ?> entry : ((Map<?, ?>) propertyValue).entrySet()) {
+
+							TypeInformation<?> typeToUse = updateTypeHintForActualValue(typeHint, entry.getValue());
+							indexes.addAll(doResolveIndexesFor(keyspace, currentPath + "." + entry.getKey(),
+									typeToUse.getActualType(), entry.getValue()));
+						}
+
+					} else if (persistentProperty.isCollectionLike()) {
+
+						for (Object listValue : (Iterable<?>) propertyValue) {
+
+							TypeInformation<?> typeToUse = updateTypeHintForActualValue(typeHint, listValue);
+							indexes.addAll(doResolveIndexesFor(keyspace, currentPath, typeToUse.getActualType(), listValue));
+						}
+					}
+
+					else if (persistentProperty.isEntity()
+							|| persistentProperty.getTypeInformation().getActualType().equals(ClassTypeInformation.OBJECT)) {
+
+						typeHint = updateTypeHintForActualValue(typeHint, propertyValue);
+						indexes.addAll(doResolveIndexesFor(keyspace, currentPath, typeHint.getActualType(), propertyValue));
+					}
+				}
+
+			}
+
+			private TypeInformation<?> updateTypeHintForActualValue(TypeInformation<?> typeHint, Object propertyValue) {
+
+				if (typeHint.equals(ClassTypeInformation.OBJECT) || typeHint.getClass().isInterface()) {
+					try {
+						typeHint = mappingContext.getPersistentEntity(propertyValue.getClass()).getTypeInformation();
+					} catch (Exception e) {
+						// ignore for cases where property value cannot be resolved as an entity, in that case the provided type
+						// hint has to be sufficient
+					}
+				}
+				return typeHint;
+			}
+
+		});
+
+		return indexes;
+	}
+
+	protected IndexedData resolveIndex(String keyspace, String propertyPath, PersistentProperty<?> property, Object value) {
+
+		if (value == null) {
+			return null;
+		}
+
+		String path = normalizeIndexPath(propertyPath, property);
 
 		if (indexConfiguration.hasIndexFor(keyspace, path)) {
 			return new SimpleIndexedPropertyValue(keyspace, path, value);
 		}
 
-		else if (property.isAnnotationPresent(Indexed.class)) {
+		else if (property != null && property.isAnnotationPresent(Indexed.class)) {
 
 			Indexed indexed = property.findAnnotation(Indexed.class);
 
@@ -75,4 +185,20 @@ else if (property.isAnnotationPresent(Indexed.class)) {
 		}
 		return null;
 	}
+
+	private String normalizeIndexPath(String path, PersistentProperty<?> property) {
+
+		if (property == null) {
+			return path;
+		}
+
+		if (property.isMap()) {
+			return path.replaceAll("\\[", "").replaceAll("\\]", "");
+		}
+		if (property.isCollectionLike()) {
+			return path.replaceAll("\\[(\\p{Digit})*\\]", "").replaceAll("\\.\\.", ".");
+		}
+
+		return path;
+	}
 }