DATACMNS-50 - Added ability to define PersistentProperty order for PersistentEntities.

BasicPersistentEntity now has a constructor taking a Comparator and holds PersistentProperty instances in a TreeSet if a COmparator is given at construction. Removed getPersistentPropertyNames() from PersistentProperty interface as it's not used anywhere.

spring-data-commons-core/src/main/java/org/springframework/data/mapping/PersistentEntity.java

@@ -1,7 +1,5 @@
 package org.springframework.data.mapping;
 
-import java.util.Collection;
-
 import org.springframework.data.util.TypeInformation;
 
 /**
@@ -57,13 +55,6 @@ public interface PersistentEntity<T, P extends PersistentProperty<P>> {
 	 */
 	TypeInformation<T> getTypeInformation();
 
-	/**
-	 * A list of property names
-	 * 
-	 * @return A List of strings
-	 */
-	Collection<String> getPersistentPropertyNames();
-
 	/**
 	 * Applies the given {@link PropertyHandler} to all {@link PersistentProperty}s contained in this
 	 * {@link PersistentEntity}.

spring-data-commons-core/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java

@@ -15,9 +15,10 @@
  */
 package org.springframework.data.mapping.model;
 
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.Comparator;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.TreeSet;
 
 import org.springframework.data.mapping.Association;
 import org.springframework.data.mapping.AssociationHandler;
@@ -32,25 +33,42 @@ import org.springframework.util.Assert;
  * Simple value object to capture information of {@link PersistentEntity}s.
  *
  * @author Jon Brisbin <jbrisbin@vmware.com>
+ * @author Oliver Gierke
  */
 public class BasicPersistentEntity<T, P extends PersistentProperty<P>> implements MutablePersistentEntity<T, P> {
 
-	protected final PreferredConstructor<T> preferredConstructor;
-	protected final TypeInformation<T> information;
-	protected final Map<String, P> persistentProperties = new HashMap<String, P>();
-	protected final Map<String, Association<P>> associations = new HashMap<String, Association<P>>();
-	protected P idProperty;
+	private final PreferredConstructor<T> preferredConstructor;
+	private final TypeInformation<T> information;
+	private final Set<P> properties;
+	private final Set<Association<P>> associations;
+	
+	private P idProperty;
 
 
 	/**
 	 * Creates a new {@link BasicPersistentEntity} from the given {@link TypeInformation}.
 	 *
-	 * @param information
+	 * @param information must not be {@literal null}.
 	 */
 	public BasicPersistentEntity(TypeInformation<T> information) {
+		this(information, null);
+	}
+	
+	/**
+	 * Creates a new {@link BasicPersistentEntity} for the given {@link TypeInformation} and {@link Comparator}. The given
+	 * {@link Comparator} will be used to define the order of the {@link PersistentProperty} instances added to the
+	 * entity.
+	 * 
+	 * @param information must not be {@literal null}
+	 * @param comparator
+	 */
+	public BasicPersistentEntity(TypeInformation<T> information, Comparator<P> comparator) {
 		Assert.notNull(information);
 		this.information = information;
 		this.preferredConstructor = new PreferredConstructorDiscoverer<T>(information).getConstructor();
+		this.properties = comparator == null ? new HashSet<P>() : new TreeSet<P>(comparator);
+		this.associations = comparator == null ? new HashSet<Association<P>>() : new TreeSet<Association<P>>(
+				new AssociationComparator<P>(comparator));
 	}
 
 	/*
@@ -91,14 +109,14 @@ public class BasicPersistentEntity<T, P extends PersistentProperty<P>> implement
 	 */
 	public void addPersistentProperty(P property) {
 		Assert.notNull(property);
-		persistentProperties.put(property.getName(), property);
+		properties.add(property);
 	}
 
 	/* (non-Javadoc)
 	 * @see org.springframework.data.mapping.MutablePersistentEntity#addAssociation(org.springframework.data.mapping.model.Association)
 	 */
 	public void addAssociation(Association<P> association) {
-		associations.put(association.getInverse().getName(), association);
+		associations.add(association);
 	}
 
 	/*
@@ -106,7 +124,14 @@ public class BasicPersistentEntity<T, P extends PersistentProperty<P>> implement
 	 * @see org.springframework.data.mapping.PersistentEntity#getPersistentProperty(java.lang.String)
 	 */
 	public P getPersistentProperty(String name) {
-		return persistentProperties.get(name);
+		
+		for (P property : properties) {
+			if (property.getName().equals(name)) {
+				return property;
+			}
+		}
+		
+		return null;
 	}
 
 	/*
@@ -125,21 +150,13 @@ public class BasicPersistentEntity<T, P extends PersistentProperty<P>> implement
 		return information;
 	}
 
-	/*
-	 * (non-Javadoc)
-	 * @see org.springframework.data.mapping.PersistentEntity#getPersistentPropertyNames()
-	 */
-	public Collection<String> getPersistentPropertyNames() {
-		return persistentProperties.keySet();
-	}
-
 	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.mapping.PersistentEntity#doWithProperties(org.springframework.data.mapping.PropertyHandler)
 	 */
 	public void doWithProperties(PropertyHandler<P> handler) {
 		Assert.notNull(handler);
-		for (P property : persistentProperties.values()) {
+		for (P property : properties) {
 			if (!property.isTransient() && !property.isAssociation()) {
 				handler.doWithPersistentProperty(property);
 			}
@@ -152,7 +169,7 @@ public class BasicPersistentEntity<T, P extends PersistentProperty<P>> implement
 	 */
 	public void doWithAssociations(AssociationHandler<P> handler) {
 		Assert.notNull(handler);
-		for (Association<P> association : associations.values()) {
+		for (Association<P> association : associations) {
 			handler.doWithAssociation(association);
 		}
 	}
@@ -163,4 +180,23 @@ public class BasicPersistentEntity<T, P extends PersistentProperty<P>> implement
 	public void verify() {
 
 	}
+	
+	/**
+	 * Simple {@link Comparator} adaptor to delegate ordering to the inverse properties of the association.
+	 * 
+	 * @author Oliver Gierke
+	 */
+	private static final class AssociationComparator<P extends PersistentProperty<P>> implements Comparator<Association<P>> {
+		
+		private final Comparator<P> delegate;
+		
+		public AssociationComparator(Comparator<P> delegate) {
+			Assert.notNull(delegate);
+			this.delegate = delegate;
+		}
+
+		public int compare(Association<P> left, Association<P> right) {
+			return delegate.compare(left.getInverse(), right.getInverse());
+		}
+	}
 }

spring-data-commons-core/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java

@@ -2,12 +2,19 @@ package org.springframework.data.mapping.model;
 
 import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
+
+import java.util.Comparator;
+import java.util.Iterator;
+import java.util.SortedSet;
 
 import org.junit.Test;
+import org.mockito.Mockito;
 import org.springframework.data.mapping.PersistentEntitySpec;
 import org.springframework.data.mapping.PersistentProperty;
 import org.springframework.data.mapping.Person;
 import org.springframework.data.util.ClassTypeInformation;
+import org.springframework.test.util.ReflectionTestUtils;
 
 /**
  * Unit test for {@link BasicPersistentEntity}.
@@ -18,7 +25,7 @@ public class BasicPersistentEntityUnitTests<T extends PersistentProperty<T>> {
 
 	@Test
 	public void assertInvariants() {
-		PersistentEntitySpec.assertInvariants(createEntity());
+		PersistentEntitySpec.assertInvariants(createEntity(null));
 	}
 	
 	@Test(expected = IllegalArgumentException.class)
@@ -28,10 +35,45 @@ public class BasicPersistentEntityUnitTests<T extends PersistentProperty<T>> {
 	
 	@Test(expected = IllegalArgumentException.class)
 	public void rejectsNullProperty() {
-		createEntity().addPersistentProperty(null);
+		createEntity(null).addPersistentProperty(null);
+	}
+	
+	/**
+	 * @see DATACMNS-50
+	 */
+	@Test
+	@SuppressWarnings("unchecked")
+	public void considersComparatorForPropertyOrder() {
+		
+		BasicPersistentEntity<Person,T> entity = createEntity(new Comparator<T>() {
+			public int compare(T o1, T o2) {
+				return o1.getName().compareTo(o2.getName());
+			}
+		});
+		
+		T lastName = (T) Mockito.mock(PersistentProperty.class);
+		when(lastName.getName()).thenReturn("lastName");
+		
+		T firstName = (T) Mockito.mock(PersistentProperty.class);
+		when(firstName.getName()).thenReturn("firstName");
+		
+		T ssn = (T) Mockito.mock(PersistentProperty.class);
+		when(ssn.getName()).thenReturn("ssn");
+		
+		entity.addPersistentProperty(lastName);
+		entity.addPersistentProperty(firstName);
+		entity.addPersistentProperty(ssn);
+		
+		SortedSet<T> properties = (SortedSet<T>) ReflectionTestUtils.getField(entity, "properties");
+		
+		assertThat(properties.size(), is(3));
+		Iterator<T> iterator = properties.iterator();
+		assertThat(iterator.next(), is(entity.getPersistentProperty("firstName")));
+		assertThat(iterator.next(), is(entity.getPersistentProperty("lastName")));
+		assertThat(iterator.next(), is(entity.getPersistentProperty("ssn")));
 	}
 	
-	private BasicPersistentEntity<Person, T> createEntity() {
-		return new BasicPersistentEntity<Person, T>(ClassTypeInformation.from(Person.class));
+	private BasicPersistentEntity<Person, T> createEntity(Comparator<T> comparator) {
+		return new BasicPersistentEntity<Person, T>(ClassTypeInformation.from(Person.class), comparator);
 	}
 }