Polish ReturnAttributes Override

Closes gh-465

Author: jzheaux

Diff for: core/src/main/java/org/springframework/ldap/core/LdapTemplate.java

@@ -30,6 +30,8 @@
 import org.springframework.ldap.query.LdapQuery;
 import org.springframework.ldap.support.LdapUtils;
 import org.springframework.util.Assert;
+import org.springframework.util.CollectionUtils;
+import org.springframework.util.ObjectUtils;
 
 import javax.naming.Binding;
 import javax.naming.Name;
@@ -43,6 +45,8 @@
 import javax.naming.directory.ModificationItem;
 import javax.naming.directory.SearchControls;
 import javax.naming.ldap.LdapName;
+
+import java.util.Arrays;
 import java.util.List;
 
 /**
@@ -1830,11 +1834,11 @@ public <T> List<T> find(Name base, Filter filter, SearchControls searchControls,
         }
 
         // extend search controls with the attributes to return
-	if (isNotCustomReturningAttributes(searchControls)) {
+        if (searchControls.getReturningAttributes() == null) {
             String[] attributes = this.odm.manageClass(clazz);
             searchControls.setReturningAttributes(attributes);
         }
-        
+
         if (LOG.isDebugEnabled()) {
             LOG.debug(String.format("Searching - base=%1$s, finalFilter=%2$s, scope=%3$s", base, finalFilter, searchControls));
         }
@@ -1879,10 +1883,6 @@ else if (result.size() != 1) {
 
         return result.get(0);
     }
-	
-    private boolean isNotCustomReturningAttributes(SearchControls searchControls) {
-        return searchControls.getReturningAttributes() == null || searchControls.getReturningAttributes().length == 0;
-    }
 
 	/**
 	 * The status of an authentication attempt.

Diff for: core/src/test/java/org/springframework/ldap/core/LdapTemplateTest.java

@@ -21,6 +21,8 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
+import org.mockito.verification.VerificationMode;
+
 import org.springframework.LdapDataEntry;
 import org.springframework.dao.EmptyResultDataAccessException;
 import org.springframework.dao.IncorrectResultSizeDataAccessException;
@@ -29,6 +31,7 @@
 import org.springframework.ldap.PartialResultException;
 import org.springframework.ldap.UncategorizedLdapException;
 import org.springframework.ldap.filter.EqualsFilter;
+import org.springframework.ldap.filter.Filter;
 import org.springframework.ldap.odm.core.ObjectDirectoryMapper;
 import org.springframework.ldap.query.LdapQuery;
 import org.springframework.ldap.query.LdapQueryBuilder;
@@ -683,6 +686,56 @@ public void verifyThatFindOneThrowsIncorrectResultSizeDataAccessExceptionWhenMor
         verify(dirContextMock).close();
     }
 
+	@Test
+	public void findWhenSearchControlsReturningAttributesSpecifiedThenOverridesOdmReturningAttributes() throws Exception {
+		Class<Object> expectedClass = Object.class;
+
+		Filter filter = new EqualsFilter("ou", "somevalue");
+		when(contextSourceMock.getReadOnlyContext()).thenReturn(dirContextMock);
+		when(odmMock.filterFor(any(Class.class), any(Filter.class))).thenReturn(filter);
+		SearchControls controls = new SearchControls();
+		controls.setReturningAttributes(new String[] { "attribute" });
+		DirContextAdapter expectedObject = new DirContextAdapter();
+		SearchResult searchResult = new SearchResult("", expectedObject, new BasicAttributes());
+		setupSearchResults(controls, searchResult);
+		Object expectedResult = expectedObject;
+		when(odmMock.mapFromLdapDataEntry(expectedObject, expectedClass)).thenReturn(expectedResult, expectedResult);
+
+		List<Object> results = tested.find(nameMock, filter, controls, expectedClass);
+		assertThat(results).hasSize(1);
+		verify(odmMock, never()).manageClass(any(Class.class));
+
+		verify(namingEnumerationMock).close();
+		verify(dirContextMock).close();
+	}
+
+	@Test
+	public void findWhenSearchControlsReturningAttributesUnspecifiedThenOdmReturningAttributesOverrides() throws Exception {
+		Class<Object> expectedClass = Object.class;
+		String[] expectedReturningAttributes = new String[] { "odmattribute" };
+		SearchControls expectedControls = new SearchControls();
+		expectedControls.setReturningObjFlag(true);
+		expectedControls.setReturningAttributes(expectedReturningAttributes);
+
+		Filter filter = new EqualsFilter("ou", "somevalue");
+		when(contextSourceMock.getReadOnlyContext()).thenReturn(dirContextMock);
+		when(odmMock.filterFor(eq(expectedClass), any(Filter.class))).thenReturn(filter);
+		when(odmMock.manageClass(eq(expectedClass))).thenReturn(expectedReturningAttributes);
+		SearchControls controls = new SearchControls();
+		DirContextAdapter expectedObject = new DirContextAdapter();
+		SearchResult searchResult = new SearchResult("", expectedObject, new BasicAttributes());
+		setupSearchResults(expectedControls, searchResult);
+		Object expectedResult = expectedObject;
+		when(odmMock.mapFromLdapDataEntry(expectedObject, expectedClass)).thenReturn(expectedResult, expectedResult);
+
+		List<Object> results = tested.find(nameMock, null, controls, expectedClass);
+		assertThat(results).hasSize(1);
+		verify(odmMock).manageClass(eq(expectedClass));
+
+		verify(namingEnumerationMock).close();
+		verify(dirContextMock).close();
+	}
+
     @Test
 	public void testSearch_ContextMapper_ReturningAttrs() throws Exception {
 		expectGetReadOnlyContext();
@@ -1816,7 +1869,7 @@ private void singleSearchResult(SearchControls controls, SearchResult searchResu
 		setupSearchResults(controls, new SearchResult[] { searchResult });
 	}
 
-	private void setupSearchResults(SearchControls controls, SearchResult[] searchResults) throws Exception {
+	private void setupSearchResults(SearchControls controls, SearchResult... searchResults) throws Exception {
         when(dirContextMock.search(
                 eq(nameMock),
                 eq("(ou=somevalue)"),