diff --git a/dspace/modules/additions/src/main/java/edu/umd/lib/dspace/authenticate/impl/Ldap.java b/dspace/modules/additions/src/main/java/edu/umd/lib/dspace/authenticate/impl/Ldap.java index 772f6356cde3..115f6320b82f 100644 --- a/dspace/modules/additions/src/main/java/edu/umd/lib/dspace/authenticate/impl/Ldap.java +++ b/dspace/modules/additions/src/main/java/edu/umd/lib/dspace/authenticate/impl/Ldap.java @@ -10,6 +10,8 @@ import javax.naming.directory.Attributes; import javax.naming.directory.SearchResult; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.dspace.core.Context; import org.dspace.eperson.Group; import org.dspace.eperson.Unit; @@ -20,6 +22,9 @@ * Class representing an LDAP result for a particular user */ public class Ldap { + /** log4j category */ + private static Logger log = LogManager.getLogger(Ldap.class); + private String strUid; private SearchResult entry; @@ -197,7 +202,7 @@ public List getUnmatchedUnits(Context context) throws NamingException, j * value. */ public boolean isFaculty() throws NamingException { - if (strUid.equals("tstusr2")) { + if ("tstusr2".equals(strUid)) { return true; } @@ -218,24 +223,34 @@ public boolean isFaculty() throws NamingException { * Returns true if the given umAppointment string matches the criteria for * a faculty member, false otherwise. * - * @param umAppointment the LDAP "umAppointment" string to check + * @param umAppointment the LDAP "umappointment" string to check * @return true if the given umAppointment string matches the criteria for * a faculty member, false otherwise. */ protected boolean isFaculty(String umAppointment) { - String strInst = umAppointment.substring(0, 2); - String strCat = umAppointment.substring(24, 26); - String strStatus = umAppointment.substring(27, 28); + + String[] components = umAppointment.split("\\$"); + // Don't look for exactly 7 (or 8), because last term may be blank, and so not be present + if (components.length < 7) { + log.warn( + "The given umAppointment '{}' did not have the expected format", + umAppointment + ); + return false; + } + + String strInst = components[0]; // umInstitutionCode + String strCat = components[3]; // umCatStatus + String strStatus = components[4]; // EMP_STAT_CD final List facultyCategories = List.of( - "01", // Tenured Fac - "02", // Ten Trk Fac - "03", // NT-Term Fac - "15", // NT-Cont. Fac - "25", // Post-Doctoral Scholar - "36", // Hrly Faculty - "37", // NT-NonRg Fac - "EA" // Not sure what it corresponds to, or if actually used. + "Tenured Fac", + "Ten Trk Fac", + "NT-Term Fac", + "NT-Cont. Fac", + "Post-Doctoral Scholar", + "Hrly Faculty", + "NT-NonRg Fac" ); final List employmentStatuses = List.of( @@ -246,7 +261,7 @@ protected boolean isFaculty(String umAppointment) { "E", "N", "T", "F" ); - return strInst.equals("01") && facultyCategories.contains(strCat) && + return "01".equals(strInst) && facultyCategories.contains(strCat) && employmentStatuses.contains(strStatus); } diff --git a/dspace/modules/additions/src/test/java/edu/umd/lib/dspace/authenticate/impl/LdapTest.java b/dspace/modules/additions/src/test/java/edu/umd/lib/dspace/authenticate/impl/LdapTest.java index 889b0b684a70..0afa94740ea1 100644 --- a/dspace/modules/additions/src/test/java/edu/umd/lib/dspace/authenticate/impl/LdapTest.java +++ b/dspace/modules/additions/src/test/java/edu/umd/lib/dspace/authenticate/impl/LdapTest.java @@ -2,21 +2,16 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; import java.util.List; import org.dspace.AbstractDSpaceTest; -import org.junit.Before; import org.junit.Test; public class LdapTest extends AbstractDSpaceTest { - Ldap ldap; - @Before - public void setUp() { - } - @Test public void isFaculty_tstusr2_returnsTrue() throws Exception { Ldap ldap = spy(new Ldap("tstusr2", null)); @@ -32,20 +27,19 @@ public void isFaculty_nullUmAppointment_returnsFalse() throws Exception { assertFalse(ldap.isFaculty()); } - @Test(expected = StringIndexOutOfBoundsException.class) - public void isFaculty_emptyUmAppointment_throwsException() throws Exception { + @Test + public void isFaculty_emptyUmAppointment_returnsFalse() throws Exception { List umAppointmentsList = List.of(""); Ldap ldap = spy(new Ldap("emptyUmAppointment", null)); when(ldap.getAttributeAll("umappointment")).thenReturn(umAppointmentsList); - ldap.isFaculty(); + assertFalse(ldap.isFaculty()); } @Test public void isFaculty_notFacultyUmAppointment_returnsFalse() throws Exception { - // instCode: 01, strCat: 04, strStatus: A -- strCat does not match - List umAppointmentsList = List.of("012023001230101$9936503$04$A$ $Y$Y$"); + List umAppointmentsList = List.of("01$SO001651$SO001650$Grad Asst$A$Y$Y$"); Ldap ldap = spy(new Ldap("notFacultyUmAppointment", null)); when(ldap.getAttributeAll("umappointment")).thenReturn(umAppointmentsList); @@ -54,21 +48,22 @@ public void isFaculty_notFacultyUmAppointment_returnsFalse() throws Exception { @Test public void isFaculty_facultyUmAppointment_returnsTrue() throws Exception { - // instCode: 01, strCat: 15, strStatus: A - List umAppointmentsList = List.of("012030001300301$9533010$15$A$ $Y$Y$"); - Ldap ldap = spy(new Ldap("notFacultyUmAppointment", null)); - when(ldap.getAttributeAll("umappointment")).thenReturn(umAppointmentsList); + List umAppointmentsList = List.of("01$SO003205$SO003123$NT-Cont. Fac$A$Y$Y$"); + Ldap ldap = spy(new Ldap("multipleFacultyUmAppointment", null)); + doReturn(umAppointmentsList).when(ldap).getAttributeAll("umappointment"); - assertTrue(ldap.isFaculty()); + + boolean result = ldap.isFaculty(); + assertTrue("Expected " + umAppointmentsList + " to return true", result); } @Test public void isFaculty_facultyWithMultipleUmAppointments_returnsTrue() throws Exception { List umAppointmentsList = List.of( - "012023001230101$9542103$15$A$ $Y$Y$", - "012027001272901$9543906$15$A$ $Y$N$"); - Ldap ldap = spy(new Ldap("notFacultyUmAppointment", null)); - when(ldap.getAttributeAll("umappointment")).thenReturn(umAppointmentsList); + "01$SO002367$SO002206$Affiliate with Librarian$A$Y$N$", + "01$SO001740$SO001650$NT-Cont. Fac$A$Y$Y$"); + Ldap ldap = spy(new Ldap("multipleFacultyUmAppointment", null)); + doReturn(umAppointmentsList).when(ldap).getAttributeAll("umappointment"); assertTrue(ldap.isFaculty()); }