diff --git a/core/src/main/java/org/springframework/ldap/core/LdapOperations.java b/core/src/main/java/org/springframework/ldap/core/LdapOperations.java
index 2eb8b58f..326e2f9b 100644
--- a/core/src/main/java/org/springframework/ldap/core/LdapOperations.java
+++ b/core/src/main/java/org/springframework/ldap/core/LdapOperations.java
@@ -1715,6 +1715,7 @@ public interface LdapOperations {
* If the field annotated with {@link org.springframework.ldap.odm.annotations.Id}
* is set in the object, this will be used as the distinguished name of the new entry. If no explicit DN is specified,
* an attempt will be made to calculate the name from fields annotated with {@link org.springframework.ldap.odm.annotations.DnAttribute}.
+ * If an id can be calculated, this will be populated in the supplied object.
*
* @param entry The entry to be create, it must not be null or already exist in the directory.
*
@@ -1736,6 +1737,8 @@ public interface LdapOperations {
* the current data of the entry will be read from the directory and a {@link #modifyAttributes(DirContextOperations)}
* operation will be performed using the ModificationItems resulting from the changes of the
* entry compared to its current state in the directory.
+ * If the id of the entry has changed, i.e. if it wasn't specified from the beginning, or if it is calculated to
+ * have changed, the new value will be populated in the supplied object.
*
* @param entry The entry to update, it must already exist in the directory.
*
diff --git a/core/src/main/java/org/springframework/ldap/core/LdapTemplate.java b/core/src/main/java/org/springframework/ldap/core/LdapTemplate.java
index e9d46981..cf44eac2 100644
--- a/core/src/main/java/org/springframework/ldap/core/LdapTemplate.java
+++ b/core/src/main/java/org/springframework/ldap/core/LdapTemplate.java
@@ -1787,6 +1787,7 @@ public class LdapTemplate implements LdapOperations, InitializingBean {
Name id = odm.getId(entry);
if(id == null) {
id = odm.getCalculatedId(entry);
+ odm.setId(entry, id);
}
Assert.notNull(id, String.format("Unable to determine id for entry %s", entry.toString()));
@@ -1821,12 +1822,14 @@ public class LdapTemplate implements LdapOperations, InitializingBean {
odm.mapToLdapDataEntry(entry, context);
bind(context);
+ odm.setId(entry, calculatedId);
} else {
// DN is the same, just modify the attributes
Name id = originalId;
if(id == null) {
id = calculatedId;
+ odm.setId(entry, calculatedId);
}
Assert.notNull(id, String.format("Unable to determine id for entry %s", entry.toString()));
diff --git a/core/src/main/java/org/springframework/ldap/repository/support/SimpleLdapRepository.java b/core/src/main/java/org/springframework/ldap/repository/support/SimpleLdapRepository.java
index 6fef67b9..125cceb9 100644
--- a/core/src/main/java/org/springframework/ldap/repository/support/SimpleLdapRepository.java
+++ b/core/src/main/java/org/springframework/ldap/repository/support/SimpleLdapRepository.java
@@ -83,25 +83,11 @@ public class SimpleLdapRepository implements LdapRepository {
public S save(S entity) {
Assert.notNull(entity, "Entity must not be null");
Name declaredId = odm.getId(entity);
- Name calculatedId = odm.getCalculatedId(entity);
if (isNew(entity, declaredId)) {
- if (declaredId == null) {
- if (calculatedId != null) {
- odm.setId(entity, calculatedId);
- } else {
- throw new IllegalStateException(String.format("Unable to calculate id of entry of class %s - " +
- "ID not set and unable to calculate new ID. Missing @DnAttribute annotations with index?",
- entity.getClass()));
- }
- }
-
ldapOperations.create(entity);
} else {
ldapOperations.update(entity);
- if (calculatedId != null && !calculatedId.equals(declaredId)) {
- odm.setId(entity, calculatedId);
- }
}
return entity;
diff --git a/core/src/test/java/org/springframework/ldap/core/LdapTemplateTest.java b/core/src/test/java/org/springframework/ldap/core/LdapTemplateTest.java
index d1d39be3..dded783a 100644
--- a/core/src/test/java/org/springframework/ldap/core/LdapTemplateTest.java
+++ b/core/src/test/java/org/springframework/ldap/core/LdapTemplateTest.java
@@ -1048,6 +1048,7 @@ public class LdapTemplateTest {
tested.create(expectedObject);
+ verify(odmMock, never()).setId(expectedObject, expectedName);
verify(dirContextMock).bind(expectedName, ctxCaptor.getValue(), null);
verify(dirContextMock).close();
}
@@ -1066,6 +1067,7 @@ public class LdapTemplateTest {
tested.create(expectedObject);
+ verify(odmMock).setId(expectedObject, expectedName);
verify(dirContextMock).bind(expectedName, ctxCaptor.getValue(), null);
verify(dirContextMock).close();
}
@@ -1104,6 +1106,7 @@ public class LdapTemplateTest {
tested.update(expectedObject);
+ verify(odmMock, never()).setId(expectedObject, expectedName);
verify(odmMock).mapToLdapDataEntry(expectedObject, ctxMock);
verify(dirContextMock).modifyAttributes(expectedName, expectedModificationItems);
@@ -1130,6 +1133,7 @@ public class LdapTemplateTest {
tested.update(expectedObject);
+ verify(odmMock).setId(expectedObject, expectedName);
verify(odmMock).mapToLdapDataEntry(expectedObject, ctxMock);
verify(dirContextMock).modifyAttributes(expectedName, expectedModificationItems);
@@ -1152,6 +1156,7 @@ public class LdapTemplateTest {
tested.update(expectedObject);
+ verify(odmMock).setId(expectedObject, expectedNewName);
verify(dirContextMock).unbind(expectedOriginalName);
verify(dirContextMock).bind(expectedNewName, ctxCaptor.getValue(), null);
verify(dirContextMock, times(2)).close();
diff --git a/core/src/test/java/org/springframework/ldap/repository/SimpleLdapRepositoryTest.java b/core/src/test/java/org/springframework/ldap/repository/SimpleLdapRepositoryTest.java
index 9ceef01b..1cbd0c6c 100644
--- a/core/src/test/java/org/springframework/ldap/repository/SimpleLdapRepositoryTest.java
+++ b/core/src/test/java/org/springframework/ldap/repository/SimpleLdapRepositoryTest.java
@@ -26,7 +26,6 @@ import static org.junit.Assert.assertSame;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -61,16 +60,6 @@ public class SimpleLdapRepositoryTest {
assertArrayEquals(new String[]{"objectclass"}, query.attributes());
}
- @Test(expected = IllegalStateException.class)
- public void testSaveNonPersistableNoIdNoCalculatedId() {
- Object expectedEntity = new Object();
-
- when(odmMock.getId(expectedEntity)).thenReturn(null);
- when(odmMock.getCalculatedId(expectedEntity)).thenReturn(null);
-
- tested.save(expectedEntity);
- }
-
@Test
public void testSaveNonPersistableWithIdSet() {
Object expectedEntity = new Object();
@@ -81,7 +70,6 @@ public class SimpleLdapRepositoryTest {
tested.save(expectedEntity);
verify(ldapOperationsMock).update(expectedEntity);
- verify(odmMock, never()).setId(any(Object.class), any(Name.class));
}
@Test
@@ -95,7 +83,6 @@ public class SimpleLdapRepositoryTest {
tested.save(expectedEntity);
verify(ldapOperationsMock).update(expectedEntity);
- verify(odmMock).setId(expectedEntity, expectedName);
}
@Test
@@ -109,7 +96,6 @@ public class SimpleLdapRepositoryTest {
tested.save(expectedEntity);
verify(ldapOperationsMock).create(expectedEntity);
- verify(odmMock).setId(expectedEntity, expectedName);
}
@Test
@@ -123,7 +109,6 @@ public class SimpleLdapRepositoryTest {
tested.save(expectedEntity);
verify(ldapOperationsMock).create(expectedEntity);
- verify(odmMock, never()).setId(any(Object.class), any(Name.class));
}
@Test
@@ -138,7 +123,6 @@ public class SimpleLdapRepositoryTest {
tested.save(expectedEntity);
verify(ldapOperationsMock).create(expectedEntity);
- verify(odmMock).setId(expectedEntity, expectedName);
}
@Test
@@ -152,7 +136,6 @@ public class SimpleLdapRepositoryTest {
tested.save(expectedEntity);
verify(ldapOperationsMock).update(expectedEntity);
- verify(odmMock, never()).setId(any(Object.class), any(Name.class));
}
@Test
diff --git a/test/integration-tests/src/test/java/org/springframework/ldap/itest/repository/RepositoryScanITest.java b/test/integration-tests/src/test/java/org/springframework/ldap/itest/repository/RepositoryScanITest.java
index 84dad708..5580cea2 100644
--- a/test/integration-tests/src/test/java/org/springframework/ldap/itest/repository/RepositoryScanITest.java
+++ b/test/integration-tests/src/test/java/org/springframework/ldap/itest/repository/RepositoryScanITest.java
@@ -193,8 +193,8 @@ public class RepositoryScanITest extends AbstractLdapTemplateIntegrationTest {
assertEquals("0123456", person.getTelephoneNumber());
}
- @Test(expected = IllegalStateException.class)
- public void verifyThatCreateWithNoIdSetAndNotAbleToCalculateThrowsIllegalState() {
+ @Test(expected = IllegalArgumentException.class)
+ public void verifyThatCreateWithNoIdSetAndNotAbleToCalculateThrowsIllegalArgument() {
Person person = new Person();
person.setCommonName("New Person");
person.setSurname("Person");
diff --git a/test/integration-tests/src/test/java/org/springframework/ldap/itest/repository/RepositoryScanWithDnAnnotationsITest.java b/test/integration-tests/src/test/java/org/springframework/ldap/itest/repository/RepositoryScanWithDnAnnotationsITest.java
index 77f748b1..8aefe529 100644
--- a/test/integration-tests/src/test/java/org/springframework/ldap/itest/repository/RepositoryScanWithDnAnnotationsITest.java
+++ b/test/integration-tests/src/test/java/org/springframework/ldap/itest/repository/RepositoryScanWithDnAnnotationsITest.java
@@ -22,11 +22,15 @@ import org.springframework.ldap.itest.AbstractLdapTemplateIntegrationTest;
import org.springframework.ldap.itest.odm.PersonWithDnAnnotations;
import org.springframework.ldap.itest.repositories.PersonWithDnAnnotationsRepository;
import org.springframework.ldap.query.LdapQueryBuilder;
+import org.springframework.ldap.support.LdapUtils;
import org.springframework.test.context.ContextConfiguration;
import java.util.Arrays;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.springframework.ldap.query.LdapQueryBuilder.query;
/**
* Tests for Spring LDAP automatic repository scan functionality.
@@ -48,15 +52,33 @@ public class RepositoryScanWithDnAnnotationsITest extends AbstractLdapTemplateIn
person.setCountry("Sweden");
person.setCompany("company1");
+ assertNull(person.getDn());
+
tested.save(person);
+ assertNotNull(person.getDn());
+
assertEquals(6, tested.count());
- person = tested.findOne(LdapQueryBuilder.query().where("cn").is("New Person"));
+ person = tested.findOne(query().where("cn").is("New Person"));
assertEquals("New Person", person.getCommonName());
assertEquals("Person", person.getSurname());
assertEquals("This is the description", person.getDesc().get(0));
assertEquals("0123456", person.getTelephoneNumber());
}
+
+ @Test
+ public void verifyThatMovedEntryGetsUpdatedId() {
+ PersonWithDnAnnotations found = tested.findOne(query().where("cn").is("Some Person3"));
+ assertNotNull(found);
+
+ assertEquals(LdapUtils.newLdapName("cn=Some Person3,ou=company1,ou=Sweden"), found.getDn());
+
+ found.setCompany("company2");
+
+ tested.save(found);
+
+ assertEquals(LdapUtils.newLdapName("cn=Some Person3,ou=company2,ou=Sweden"), found.getDn());
+ }
}