diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/PersistentEntity.java b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/PersistentEntity.java index a76942c21..10aab9b88 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/PersistentEntity.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/PersistentEntity.java @@ -1,11 +1,11 @@ /* - * Copyright (c) 2011-2012 by the original author(s). + * Copyright 2011-2012 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 + * 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, @@ -15,10 +15,11 @@ */ package org.springframework.data.mapping; +import org.springframework.data.convert.EntityInstantiator; import org.springframework.data.util.TypeInformation; /** - * Represents a persistent entity + * Represents a persistent entity. * * @author Oliver Gierke * @author Graeme Rocher @@ -27,7 +28,7 @@ import org.springframework.data.util.TypeInformation; public interface PersistentEntity> { /** - * The entity name including any package prefix + * The entity name including any package prefix. * * @return must never return {@literal null} */ @@ -36,7 +37,9 @@ public interface PersistentEntity> { /** * Returns the {@link PreferredConstructor} to be used to instantiate objects of this {@link PersistentEntity}. * - * @return {@literal null} in case no suitable constructor for automatic construction can be found. + * @return {@literal null} in case no suitable constructor for automatic construction can be found. This usually + * indicates that the instantiation of the object of tthat persistent entity is done through either a customer + * {@link EntityInstantiator} or handled by custom conversion mechanisms entirely. */ PreferredConstructor getPersistenceConstructor(); @@ -51,18 +54,26 @@ public interface PersistentEntity> { boolean isConstructorArgument(P property); /** - * Returns the id property of the {@link PersistentEntity}. Must never return {@literal null} as a - * {@link PersistentEntity} instance must not be created if there is no id property. + * Returns whether the given {@link PersistentProperty} is the id property of the entity. + * + * @param property + * @return + */ + boolean isIdProperty(P property); + + /** + * Returns the id property of the {@link PersistentEntity}. Can be {@literal null} in case this is an entity + * completely handled by a custom conversion. * * @return the id property of the {@link PersistentEntity}. */ P getIdProperty(); /** - * Obtains a PersistentProperty instance by name. + * Obtains a {@link PersistentProperty} instance by name. * * @param name The name of the property - * @return The {@link PersistentProperty} or {@literal null} if it doesn't exist + * @return the {@link PersistentProperty} or {@literal null} if it doesn't exist. */ P getPersistentProperty(String name); diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/PersistentProperty.java b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/PersistentProperty.java index 3ba7bee40..7ffe9a6c5 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/PersistentProperty.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/PersistentProperty.java @@ -63,7 +63,10 @@ public interface PersistentProperty

> { boolean isEntity(); /** - * Returns whether the property is the ID property of the owning {@link PersistentEntity}. + * Returns whether the property is a potential identifier property of the owning {@link PersistentEntity}. + * This method is mainly used by {@link PersistentEntity} implementation to discover id property candidates on + * {@link PersistentEntity} creation you should rather call {@link PersistentEntity#isIdProperty(PersistentProperty)} + * to determine whether the current property is the id property of that {@link PersistentEntity} under consideration. * * @return */ @@ -128,5 +131,4 @@ public interface PersistentProperty

> { * @return the map's value type or {@literal null} if no {@link java.util.Map} */ Class getMapValueType(); - } diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java index 1e9447baa..f74c0410f 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/context/AbstractMappingContext.java @@ -415,10 +415,6 @@ public abstract class AbstractMappingContext * @author Oliver Gierke + * @author Jon Brisbin */ public class BasicPersistentEntity> implements MutablePersistentEntity { @@ -91,6 +91,14 @@ public class BasicPersistentEntity> implement return constructor == null ? false : constructor.isConstructorParameter(property); } + /* + * (non-Javadoc) + * @see org.springframework.data.mapping.PersistentEntity#isIdProperty(org.springframework.data.mapping.PersistentProperty) + */ + public boolean isIdProperty(P property) { + return this.idProperty == null ? false : this.idProperty.equals(property); + } + /* * (non-Javadoc) * @see org.springframework.data.mapping.PersistentEntity#getName() @@ -107,21 +115,25 @@ public class BasicPersistentEntity> implement return idProperty; } - /* - * (non-Javadoc) - * @see org.springframework.data.mapping.MutablePersistentEntity#setIdProperty(P) - */ - public void setIdProperty(P property) { - idProperty = property; - } - /* * (non-Javadoc) * @see org.springframework.data.mapping.MutablePersistentEntity#addPersistentProperty(P) */ public void addPersistentProperty(P property) { + Assert.notNull(property); properties.add(property); + + if (property.isIdProperty()) { + + if (this.idProperty != null) { + throw new MappingException(String.format( + "Attempt to add id property %s but already have property %s registered " + + "as id. Check your mapping configuration!", property.getField(), idProperty.getField())); + } + + this.idProperty = property; + } } /* (non-Javadoc) diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/model/MutablePersistentEntity.java b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/model/MutablePersistentEntity.java index d3c5cfd9f..283a43eff 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/mapping/model/MutablePersistentEntity.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/mapping/model/MutablePersistentEntity.java @@ -26,13 +26,6 @@ import org.springframework.data.mapping.PersistentProperty; */ public interface MutablePersistentEntity> extends PersistentEntity { - /** - * Sets the id property for the entity. - * - * @param property - */ - void setIdProperty(P property); - /** * Adds a {@link PersistentProperty} to the entity. * diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java b/spring-data-commons-core/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java index dc1a8a82e..6f39ca517 100644 --- a/spring-data-commons-core/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java +++ b/spring-data-commons-core/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java @@ -9,7 +9,10 @@ import java.util.Iterator; import java.util.SortedSet; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; import org.mockito.Mockito; +import org.mockito.runners.MockitoJUnitRunner; import org.springframework.data.annotation.TypeAlias; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentEntitySpec; @@ -23,8 +26,12 @@ import org.springframework.test.util.ReflectionTestUtils; * * @author Oliver Gierke */ +@RunWith(MockitoJUnitRunner.class) public class BasicPersistentEntityUnitTests> { + @Mock + T property; + @Test public void assertInvariants() { PersistentEntitySpec.assertInvariants(createEntity(null)); @@ -90,6 +97,40 @@ public class BasicPersistentEntityUnitTests> { assertThat(iterator.next(), is(entity.getPersistentProperty("ssn"))); } + /** + * @see DATACMNS-186 + */ + @Test + public void addingAndIdPropertySetsIdPropertyInternally() { + + MutablePersistentEntity entity = createEntity(null); + assertThat(entity.getIdProperty(), is(nullValue())); + + when(property.isIdProperty()).thenReturn(true); + entity.addPersistentProperty(property); + assertThat(entity.getIdProperty(), is(property)); + } + + /** + * @see DATACMNS-186 + */ + @Test + public void rejectsIdPropertyIfAlreadySet() { + + MutablePersistentEntity entity = createEntity(null); + + when(property.isIdProperty()).thenReturn(true); + + entity.addPersistentProperty(property); + + try { + entity.addPersistentProperty(property); + fail("Expected MappingException!"); + } catch (MappingException e) { + // expected + } + } + private BasicPersistentEntity createEntity(Comparator comparator) { return new BasicPersistentEntity(ClassTypeInformation.from(Person.class), comparator); }