DATAREDIS-425 - Allow alternative id lookup and fail fast when no id found.

Allow fields named "id" to be considered as identifier property even when no explicit "@id" annotation is present. Favor fields with explicit id annotation over others and throw MappingException when ambiguous id declaration is found.

Additionally fail fast when repository interfaces reference types that do not declare an id property.

Author: christophstrobl

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

@@ -28,6 +28,8 @@ public class Person {
 We have a pretty simple domain object here. Note that it has a property named `id` annotated with `org.springframework.data.annotation.Id` and a `@RedisHash` annotation on its type.
 Those two are responsible for creating the actual key used to persist the hash. 
 
+NOTE: Properties annotated with `@Id` as well as those named `id` are considered as the identifier properties. Those with the annotation are favored over others.
+
 To now actually have a component responsible for storage and retrieval we need to define a repository interface.
 
 .Basic Repository Interface To Persist Person Entities

src/main/java/org/springframework/data/redis/core/mapping/BasicRedisPersistentEntity.java

@@ -15,8 +15,11 @@
  */
 package org.springframework.data.redis.core.mapping;
 
+import org.springframework.data.annotation.Id;
 import org.springframework.data.keyvalue.core.mapping.BasicKeyValuePersistentEntity;
 import org.springframework.data.keyvalue.core.mapping.KeySpaceResolver;
+import org.springframework.data.keyvalue.core.mapping.KeyValuePersistentProperty;
+import org.springframework.data.mapping.model.MappingException;
 import org.springframework.data.redis.core.TimeToLiveAccessor;
 import org.springframework.data.util.TypeInformation;
 import org.springframework.util.Assert;
@@ -27,7 +30,8 @@
  * @author Christoph Strobl
  * @param <T>
  */
-public class BasicRedisPersistentEntity<T> extends BasicKeyValuePersistentEntity<T> implements RedisPersistentEntity<T> {
+public class BasicRedisPersistentEntity<T> extends BasicKeyValuePersistentEntity<T>
+		implements RedisPersistentEntity<T> {
 
 	private TimeToLiveAccessor timeToLiveAccessor;
 
@@ -55,4 +59,47 @@ public TimeToLiveAccessor getTimeToLiveAccessor() {
 		return this.timeToLiveAccessor;
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mapping.model.BasicPersistentEntity#returnPropertyIfBetterIdPropertyCandidateOrNull(org.springframework.data.mapping.PersistentProperty)
+	 */
+	@Override
+	protected KeyValuePersistentProperty returnPropertyIfBetterIdPropertyCandidateOrNull(
+			KeyValuePersistentProperty property) {
+
+		Assert.notNull(property);
+
+		if (!property.isIdProperty()) {
+			return null;
+		}
+
+		KeyValuePersistentProperty currentIdProperty = getIdProperty();
+		boolean currentIdPropertyIsSet = currentIdProperty != null;
+
+		if (!currentIdPropertyIsSet) {
+			return property;
+		}
+
+		boolean currentIdPropertyIsExplicit = currentIdProperty.isAnnotationPresent(Id.class);
+		boolean newIdPropertyIsExplicit = property.isAnnotationPresent(Id.class);
+
+		if (currentIdPropertyIsExplicit && newIdPropertyIsExplicit) {
+			throw new MappingException(String.format(
+					"Attempt to add explicit id property %s but already have an property %s registered "
+							+ "as explicit id. Check your mapping configuration!",
+					property.getField(), currentIdProperty.getField()));
+		}
+
+		if (!currentIdPropertyIsExplicit && !newIdPropertyIsExplicit) {
+			throw new MappingException(
+					String.format("Attempt to add id property %s but already have an property %s registered "
+							+ "as id. Check your mapping configuration!", property.getField(), currentIdProperty.getField()));
+		}
+
+		if (newIdPropertyIsExplicit) {
+			return property;
+		}
+
+		return null;
+	}
 }

src/main/java/org/springframework/data/redis/core/mapping/RedisPersistentProperty.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,6 +17,8 @@
 
 import java.beans.PropertyDescriptor;
 import java.lang.reflect.Field;
+import java.util.HashSet;
+import java.util.Set;
 
 import org.springframework.data.keyvalue.core.mapping.KeyValuePersistentProperty;
 import org.springframework.data.mapping.PersistentEntity;
@@ -31,6 +33,12 @@
  */
 public class RedisPersistentProperty extends KeyValuePersistentProperty {
 
+	private static final Set<String> SUPPORTED_ID_PROPERTY_NAMES = new HashSet<String>();
+
+	static {
+		SUPPORTED_ID_PROPERTY_NAMES.add("id");
+	}
+
 	/**
 	 * Creates new {@link RedisPersistentProperty}.
 	 * 
@@ -44,4 +52,17 @@ public RedisPersistentProperty(Field field, PropertyDescriptor propertyDescripto
 		super(field, propertyDescriptor, owner, simpleTypeHolder);
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mapping.model.AnnotationBasedPersistentProperty#isIdProperty()
+	 */
+	@Override
+	public boolean isIdProperty() {
+
+		if (super.isIdProperty()) {
+			return true;
+		}
+
+		return SUPPORTED_ID_PROPERTY_NAMES.contains(getName());
+	}
 }

src/main/java/org/springframework/data/redis/repository/core/MappingRedisEntityInformation.java

@@ -0,0 +1,53 @@
+/*
+ * 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.core;
+
+import java.io.Serializable;
+
+import org.springframework.data.mapping.model.MappingException;
+import org.springframework.data.redis.core.mapping.RedisPersistentEntity;
+import org.springframework.data.repository.core.support.PersistentEntityInformation;
+
+/**
+ * {@link RedisEntityInformation} implementation using a {@link MongoPersistentEntity} instance to lookup the necessary
+ * information. Can be configured with a custom collection to be returned which will trump the one returned by the
+ * {@link MongoPersistentEntity} if given.
+ * 
+ * @author Christoph Strobl
+ * @param <T>
+ * @param <ID>
+ */
+public class MappingRedisEntityInformation<T, ID extends Serializable>
+		extends PersistentEntityInformation<T, Serializable> implements RedisEntityInformation<T, Serializable> {
+
+	private final RedisPersistentEntity<T> entityMetadata;
+
+	/**
+	 * @param entity
+	 */
+	public MappingRedisEntityInformation(RedisPersistentEntity<T> entity) {
+		super(entity);
+
+		this.entityMetadata = entity;
+
+		if (!entityMetadata.hasIdProperty()) {
+
+			throw new MappingException(
+					String.format("Entity %s requires to have an explicit id field. Did you forget to provide one using @Id?",
+							entity.getName()));
+		}
+	}
+}

src/main/java/org/springframework/data/redis/repository/core/RedisEntityInformation.java

@@ -0,0 +1,29 @@
+/*
+ * 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.core;
+
+import java.io.Serializable;
+
+import org.springframework.data.repository.core.EntityInformation;
+
+/**
+ * @author Christoph Strobl
+ * @param <T>
+ * @param <ID>
+ */
+public interface RedisEntityInformation<T, ID extends Serializable> extends EntityInformation<T, ID> {
+
+}

src/main/java/org/springframework/data/redis/repository/support/RedisRepositoryFactory.java

@@ -15,13 +15,17 @@
  */
 package org.springframework.data.redis.repository.support;
 
+import java.io.Serializable;
 import java.lang.reflect.Method;
 
 import org.springframework.data.keyvalue.core.KeyValueOperations;
 import org.springframework.data.keyvalue.repository.query.KeyValuePartTreeQuery;
 import org.springframework.data.keyvalue.repository.query.KeyValuePartTreeQuery.QueryInitialization;
 import org.springframework.data.keyvalue.repository.support.KeyValueRepositoryFactory;
 import org.springframework.data.projection.ProjectionFactory;
+import org.springframework.data.redis.core.mapping.RedisPersistentEntity;
+import org.springframework.data.redis.repository.core.MappingRedisEntityInformation;
+import org.springframework.data.repository.core.EntityInformation;
 import org.springframework.data.repository.core.NamedQueries;
 import org.springframework.data.repository.core.RepositoryMetadata;
 import org.springframework.data.repository.core.support.RepositoryFactorySupport;
@@ -69,6 +73,21 @@ protected QueryLookupStrategy getQueryLookupStrategy(Key key, EvaluationContextP
 		return new RedisQueryLookupStrategy(key, evaluationContextProvider, getKeyValueOperations(), getQueryCreator());
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.keyvalue.repository.support.KeyValueRepositoryFactory#getEntityInformation(java.lang.Class)
+	 */
+	@Override
+	@SuppressWarnings("unchecked")
+	public <T, ID extends Serializable> EntityInformation<T, ID> getEntityInformation(Class<T> domainClass) {
+
+		RedisPersistentEntity<T> entity = (RedisPersistentEntity<T>) getMappingContext().getPersistentEntity(domainClass);
+		EntityInformation<T, ID> entityInformation = (EntityInformation<T, ID>) new MappingRedisEntityInformation<T, ID>(
+				entity);
+
+		return entityInformation;
+	}
+
 	/**
 	 * @author Christoph Strobl
 	 * @since 1.7

src/test/java/org/springframework/data/redis/core/mapping/BasicRedisPersistentEntityUnitTests.java

@@ -0,0 +1,126 @@
+/*
+ * 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.core.mapping;
+
+import static org.hamcrest.core.Is.*;
+import static org.hamcrest.core.IsEqual.*;
+import static org.junit.Assert.*;
+import static org.mockito.Matchers.*;
+import static org.mockito.Mockito.*;
+
+import java.io.Serializable;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+import org.springframework.data.keyvalue.core.mapping.KeySpaceResolver;
+import org.springframework.data.keyvalue.core.mapping.KeyValuePersistentProperty;
+import org.springframework.data.mapping.model.MappingException;
+import org.springframework.data.redis.core.TimeToLiveAccessor;
+import org.springframework.data.redis.core.convert.ConversionTestEntities;
+import org.springframework.data.util.TypeInformation;
+
+/**
+ * @author Christoph Strobl
+ * @param <T>
+ * @param <ID>
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class BasicRedisPersistentEntityUnitTests<T, ID extends Serializable> {
+
+	public @Rule ExpectedException expectedException = ExpectedException.none();
+
+	@Mock TypeInformation<T> entityInformation;
+	@Mock KeySpaceResolver keySpaceResolver;
+	@Mock TimeToLiveAccessor ttlAccessor;
+
+	BasicRedisPersistentEntity<T> entity;
+
+	@Before
+	@SuppressWarnings("unchecked")
+	public void setUp() {
+
+		when(entityInformation.getType()).thenReturn((Class<T>) ConversionTestEntities.Person.class);
+		entity = new BasicRedisPersistentEntity<T>(entityInformation, keySpaceResolver, ttlAccessor);
+	}
+
+	/**
+	 * @see DATAREDIS-425
+	 */
+	@Test
+	public void addingMultipleIdPropertiesWithoutAnExplicitOneThrowsException() {
+
+		expectedException.expect(MappingException.class);
+		expectedException.expectMessage("Attempt to add id property");
+		expectedException.expectMessage("but already have an property");
+
+		KeyValuePersistentProperty property1 = mock(RedisPersistentProperty.class);
+		when(property1.isIdProperty()).thenReturn(true);
+
+		KeyValuePersistentProperty property2 = mock(RedisPersistentProperty.class);
+		when(property2.isIdProperty()).thenReturn(true);
+
+		entity.addPersistentProperty(property1);
+		entity.addPersistentProperty(property2);
+	}
+
+	/**
+	 * @see DATAREDIS-425
+	 */
+	@Test
+	@SuppressWarnings("unchecked")
+	public void addingMultipleExplicitIdPropertiesThrowsException() {
+
+		expectedException.expect(MappingException.class);
+		expectedException.expectMessage("Attempt to add explicit id property");
+		expectedException.expectMessage("but already have an property");
+
+		KeyValuePersistentProperty property1 = mock(RedisPersistentProperty.class);
+		when(property1.isIdProperty()).thenReturn(true);
+		when(property1.isAnnotationPresent(any(Class.class))).thenReturn(true);
+
+		KeyValuePersistentProperty property2 = mock(RedisPersistentProperty.class);
+		when(property2.isIdProperty()).thenReturn(true);
+		when(property2.isAnnotationPresent(any(Class.class))).thenReturn(true);
+
+		entity.addPersistentProperty(property1);
+		entity.addPersistentProperty(property2);
+	}
+
+	/**
+	 * @see DATAREDIS-425
+	 */
+	@Test
+	@SuppressWarnings("unchecked")
+	public void explicitIdPropertiyShouldBeFavoredOverNonExplicit() {
+
+		KeyValuePersistentProperty property1 = mock(RedisPersistentProperty.class);
+		when(property1.isIdProperty()).thenReturn(true);
+
+		KeyValuePersistentProperty property2 = mock(RedisPersistentProperty.class);
+		when(property2.isIdProperty()).thenReturn(true);
+		when(property2.isAnnotationPresent(any(Class.class))).thenReturn(true);
+
+		entity.addPersistentProperty(property1);
+		entity.addPersistentProperty(property2);
+
+		assertThat(entity.getIdProperty(), is(equalTo(property2)));
+	}
+}