DATAREDIS-425 - Preserve item order in list, update documentation.

We now preserve the item order when converting list elements. This does not mean that we place elements at the exact position retrieved from the store but rather maintain their order based on the index value.

Additionally applied some documentation polishing and cluster tests.

Original Pull Request: #156

Author: christophstrobl

src/main/asciidoc/reference/redis-repositories.adoc

@@ -383,8 +383,8 @@ public class ApplicationConfig {
   public static class MyIndexConfiguration extends IndexConfiguration {
 
     @Override
-    protected Iterable<RedisIndexSetting> initialConfiguration() {
-      return Collections.singleton(new RedisIndexSetting("persons", "firstname"));
+    protected Iterable<IndexDefinition> initialConfiguration() {
+      return Collections.singleton(new SimpleIndexDefinition("persons", "firstname"));
     }
   }
 }
@@ -411,8 +411,8 @@ public class ApplicationConfig {
   public static class MyIndexConfiguration extends IndexConfiguration {
 
     @Override
-    protected Iterable<RedisIndexSetting> initialConfiguration() {
-      return Collections.singleton(new RedisIndexSetting("persons", "firstname"));
+    protected Iterable<IndexDefinition> initialConfiguration() {
+      return Collections.singleton(new SimpleIndexDefinition("persons", "firstname"));
     }
   }
 }
@@ -537,7 +537,38 @@ Here's an overview of the keywords supported for Redis and what a method contain
 |===============
 ====
 
-[[redis.misc.cdi-integration]]
+[[redis.repositories.cluster]]
+== Redis Repositories running on Cluster
+
+Using the Redis repository support in a clustered Redis environment is fine. Please see the <<cluster, Redis Cluster>> section for `ConnectionFactory` configuration details.
+Still some considerations have to be done as the default key distribution will spread entities and secondary indexes through out the whole cluster and its slots.
+
+[options = "header, autowidth"]
+|===============
+|key|type|slot|node
+|persons:e2c7dcee-b8cd-4424-883e-736ce564363e|id for hash|15171|127.0.0.1:7381
+|persons:a9d4b3a0-50d3-4538-a2fc-f7fc2581ee56|id for hash|7373|127.0.0.1:7380
+|persons:firstname:rand|index|1700|127.0.0.1:7379
+|
+|===============
+====
+ 
+Some commands like `SINTER` and `SUNION` can only be processed on the Server side when all involved keys map to the same slot. Otherwise computation has to be done on client side.
+Therefore it be useful to pin keyspaces to a single slot which allows to make use of Redis serverside computation right away.
+
+[options = "header, autowidth"]
+|===============
+|key|type|slot|node
+|{persons}:e2c7dcee-b8cd-4424-883e-736ce564363e|id for hash|2399|127.0.0.1:7379
+|{persons}:a9d4b3a0-50d3-4538-a2fc-f7fc2581ee56|id for hash|2399|127.0.0.1:7379
+|{persons}:firstname:rand|index|2399|127.0.0.1:7379
+|
+|===============
+====
+
+TIP: Define and pin keyspaces via `@RedisHash("{yourkeyspace}") to specific slots when using Redis cluster.
+
+[[redis.repositories.cdi-integration]]
 == CDI integration
 
 Instances of the repository interfaces are usually created by a container, which Spring is the most natural choice when working with Spring Data. There's sophisticated support to easily set up Spring to create bean instances. Spring Data Redis ships with a custom CDI extension that allows using the repository abstraction in CDI environments. The extension is part of the JAR so all you need to do to activate it is dropping the Spring Data Redis JAR into your classpath.

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

@@ -15,8 +15,11 @@
  */
 package org.springframework.data.redis.core.convert;
 
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Comparator;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
@@ -53,6 +56,7 @@
 import org.springframework.util.ClassUtils;
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.StringUtils;
+import org.springframework.util.comparator.NullSafeComparator;
 
 /**
  * {@link RedisConverter} implementation creating flat binary map structure out of a given domain type. Considers
@@ -103,6 +107,9 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean {
 	private final GenericConversionService conversionService;
 	private final EntityInstantiators entityInstantiators;
 	private final TypeMapper<RedisData> typeMapper;
+	private final Comparator<String> listKeyComparator = new NullSafeComparator<String>(
+			NaturalOrderingKeyComparator.INSTANCE, true);
+
 	private ReferenceResolver referenceResolver;
 	private IndexResolver indexResolver;
 	private CustomConversions customConversions;
@@ -533,11 +540,15 @@ private Collection<?> readCollectionOfSimpleTypes(String path, Class<?> collecti
 
 		Bucket partial = source.getBucket().extract(path + ".[");
 
+		List<String> keys = new ArrayList<String>(partial.keySet());
+		keys.sort(listKeyComparator);
+
 		Collection<Object> target = CollectionFactory.createCollection(collectionType, valueType, partial.size());
 
-		for (byte[] value : partial.values()) {
-			target.add(fromBytes(value, valueType));
+		for (String key : keys) {
+			target.add(fromBytes(partial.get(key), valueType));
 		}
+
 		return target;
 	}
 
@@ -551,7 +562,8 @@ private Collection<?> readCollectionOfSimpleTypes(String path, Class<?> collecti
 	private Collection<?> readCollectionOfComplexTypes(String path, Class<?> collectionType, Class<?> valueType,
 			Bucket source) {
 
-		Set<String> keys = source.extractAllKeysFor(path);
+		List<String> keys = new ArrayList<String>(source.extractAllKeysFor(path));
+		keys.sort(listKeyComparator);
 
 		Collection<Object> target = CollectionFactory.createCollection(collectionType, valueType, keys.size());
 
@@ -810,4 +822,90 @@ public String resolveKeySpace(Class<?> type) {
 		}
 	}
 
+	private enum NaturalOrderingKeyComparator implements Comparator<String> {
+
+		INSTANCE;
+
+		/*
+		 * (non-Javadoc)
+		 * @see java.util.Comparator#compare(java.lang.Object, java.lang.Object)
+		 */
+		public int compare(String s1, String s2) {
+
+			int s1offset = 0;
+			int s2offset = 0;
+
+			while (s1offset < s1.length() && s2offset < s2.length()) {
+
+				Part thisPart = extractPart(s1, s1offset);
+				Part thatPart = extractPart(s2, s2offset);
+
+				int result = thisPart.compareTo(thatPart);
+
+				if (result != 0) {
+					return result;
+				}
+
+				s1offset += thisPart.length();
+				s2offset += thatPart.length();
+			}
+
+			return 0;
+		}
+
+		private Part extractPart(String source, int offset) {
+
+			StringBuilder builder = new StringBuilder();
+
+			char c = source.charAt(offset);
+			builder.append(c);
+
+			boolean isDigit = Character.isDigit(c);
+			for (int i = offset + 1; i < source.length(); i++) {
+
+				c = source.charAt(i);
+				if ((isDigit && !Character.isDigit(c)) || (!isDigit && Character.isDigit(c))) {
+					break;
+				}
+				builder.append(c);
+			}
+
+			return new Part(builder.toString(), isDigit);
+		}
+
+		private static class Part implements Comparable<Part> {
+
+			private final String rawValue;
+			private final Long longValue;
+
+			Part(String value, boolean isDigit) {
+
+				this.rawValue = value;
+				this.longValue = isDigit ? Long.valueOf(value) : null;
+			}
+
+			boolean isNumeric() {
+				return longValue != null;
+			}
+
+			int length() {
+				return rawValue.length();
+			}
+
+			/*
+			 * (non-Javadoc)
+			 * @see java.lang.Comparable#compareTo(java.lang.Object)
+			 */
+			@Override
+			public int compareTo(Part that) {
+
+				if (this.isNumeric() && that.isNumeric()) {
+					return this.longValue.compareTo(that.longValue);
+				}
+
+				return this.rawValue.compareTo(that.rawValue);
+			}
+		}
+	}
+
 }

src/test/java/org/springframework/data/redis/core/convert/MappingRedisConverterUnitTests.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.
@@ -15,6 +15,7 @@
  */
 package org.springframework.data.redis.core.convert;
 
+import static org.hamcrest.collection.IsIterableContainingInOrder.contains;
 import static org.hamcrest.core.Is.*;
 import static org.hamcrest.core.IsCollectionContaining.*;
 import static org.hamcrest.core.IsInstanceOf.*;
@@ -308,7 +309,22 @@ public void readConvertsListOfSimplePropertiesCorrectly() {
 		map.put("nicknames.[1]", "lews therin");
 		RedisData rdo = new RedisData(Bucket.newBucketFromStringMap(map));
 
-		assertThat(converter.read(Person.class, rdo).nicknames, hasItems("dragon reborn", "lews therin"));
+		assertThat(converter.read(Person.class, rdo).nicknames, contains("dragon reborn", "lews therin"));
+	}
+
+	/**
+	 * @see DATAREDIS-425
+	 */
+	@Test
+	public void readConvertsUnorderedListOfSimplePropertiesCorrectly() {
+
+		Map<String, String> map = new LinkedHashMap<String, String>();
+		map.put("nicknames.[9]", "car'a'carn");
+		map.put("nicknames.[10]", "lews therin");
+		map.put("nicknames.[1]", "dragon reborn");
+		RedisData rdo = new RedisData(Bucket.newBucketFromStringMap(map));
+
+		assertThat(converter.read(Person.class, rdo).nicknames, contains("dragon reborn", "car'a'carn", "lews therin"));
 	}
 
 	/**
@@ -338,6 +354,7 @@ public void readListComplexPropertyCorrectly() {
 		Map<String, String> map = new LinkedHashMap<String, String>();
 		map.put("coworkers.[0].firstname", "mat");
 		map.put("coworkers.[0].nicknames.[0]", "prince of the ravens");
+		map.put("coworkers.[0].nicknames.[1]", "gambler");
 		map.put("coworkers.[1].firstname", "perrin");
 		map.put("coworkers.[1].address.city", "two rivers");
 		RedisData rdo = new RedisData(Bucket.newBucketFromStringMap(map));
@@ -348,6 +365,34 @@ public void readListComplexPropertyCorrectly() {
 		assertThat(target.coworkers.get(0).firstname, is("mat"));
 		assertThat(target.coworkers.get(0).nicknames, notNullValue());
 		assertThat(target.coworkers.get(0).nicknames.get(0), is("prince of the ravens"));
+		assertThat(target.coworkers.get(0).nicknames.get(1), is("gambler"));
+
+		assertThat(target.coworkers.get(1).firstname, is("perrin"));
+		assertThat(target.coworkers.get(1).address.city, is("two rivers"));
+	}
+
+	/**
+	 * @see DATAREDIS-425
+	 */
+	@Test
+	public void readUnorderedListOfComplexPropertyCorrectly() {
+
+		Map<String, String> map = new LinkedHashMap<String, String>();
+		map.put("coworkers.[10].firstname", "perrin");
+		map.put("coworkers.[10].address.city", "two rivers");
+		map.put("coworkers.[1].firstname", "mat");
+		map.put("coworkers.[1].nicknames.[1]", "gambler");
+		map.put("coworkers.[1].nicknames.[0]", "prince of the ravens");
+
+		RedisData rdo = new RedisData(Bucket.newBucketFromStringMap(map));
+
+		Person target = converter.read(Person.class, rdo);
+
+		assertThat(target.coworkers, notNullValue());
+		assertThat(target.coworkers.get(0).firstname, is("mat"));
+		assertThat(target.coworkers.get(0).nicknames, notNullValue());
+		assertThat(target.coworkers.get(0).nicknames.get(0), is("prince of the ravens"));
+		assertThat(target.coworkers.get(0).nicknames.get(1), is("gambler"));
 
 		assertThat(target.coworkers.get(1).firstname, is("perrin"));
 		assertThat(target.coworkers.get(1).address.city, is("two rivers"));

src/test/java/org/springframework/data/redis/repository/RedisRepositoryClusterIntegrationTests.java

@@ -0,0 +1,72 @@
+/*
+ * Copyright 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.repository;
+
+import static org.springframework.data.redis.connection.ClusterTestVariables.*;
+
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.ClassRule;
+import org.junit.runner.RunWith;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.ComponentScan;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.FilterType;
+import org.springframework.data.redis.connection.RedisClusterConfiguration;
+import org.springframework.data.redis.connection.jedis.JedisConnectionFactory;
+import org.springframework.data.redis.core.RedisTemplate;
+import org.springframework.data.redis.repository.configuration.EnableRedisRepositories;
+import org.springframework.data.redis.test.util.RedisClusterRule;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+
+/**
+ * @author Christoph Strobl
+ */
+@RunWith(SpringJUnit4ClassRunner.class)
+@ContextConfiguration
+public class RedisRepositoryClusterIntegrationTests extends RedisRepositoryIntegrationTestBase {
+
+	static final List<String> CLUSTER_NODES = Arrays.asList(CLUSTER_NODE_1.asString(), CLUSTER_NODE_2.asString(),
+			CLUSTER_NODE_3.asString());
+
+	/**
+	 * ONLY RUN WHEN CLUSTER AVAILABLE
+	 */
+	public static @ClassRule RedisClusterRule clusterRule = new RedisClusterRule();
+
+	@Configuration
+	@EnableRedisRepositories(considerNestedRepositories = true, indexConfiguration = MyIndexConfiguration.class,
+			keyspaceConfiguration = MyKeyspaceConfiguration.class,
+			includeFilters = { @ComponentScan.Filter(type = FilterType.REGEX, pattern = ".*PersonRepository") })
+	static class Config {
+
+		@Bean
+		RedisTemplate<?, ?> redisTemplate() {
+
+			RedisClusterConfiguration clusterConfig = new RedisClusterConfiguration(CLUSTER_NODES);
+			JedisConnectionFactory connectionFactory = new JedisConnectionFactory(clusterConfig);
+
+			connectionFactory.afterPropertiesSet();
+
+			RedisTemplate<byte[], byte[]> template = new RedisTemplate<byte[], byte[]>();
+			template.setConnectionFactory(connectionFactory);
+
+			return template;
+		}
+	}
+}