SAK-32302 Allow entitybroker user aid searches. (sakaiproject#4057)

When looking up a user allow searching by AID as well as EID and ID. If a deployment isn’t

entitybroker/core-providers/src/java/org/sakaiproject/entitybroker/providers/UserEntityProvider.java

@@ -599,35 +599,34 @@ public EntityUser convertUser(User user) {
     }
 
     /**
-     * Attempt to get a user by EID or ID (if that fails)
+     * Attempt to get a user by AID, EID or ID
      * 
      * NOTE: can force this to only attempt the ID lookups if prefixed with "id=" using "user.explicit.id.only=true"
      * 
-     * @param userEid the user EID (could also be the ID)
+     * @param id the user EID, AID or ID
      * @return the populated User object
      */
-    private User getUserByIdEid(String userEid) {
+    User getUserByIdEid(String id) {
         User user = null;
-        if (userEid != null) {
+        if (id != null) {
             boolean doCheckForId = false;
-            boolean doCheckForEid = true;
-            String userId = userEid;
+            boolean doCheckForAid = true;
+            String userId = id;
             // check if the incoming param says this is explicitly an id
             if (userId.length() > ID_PREFIX.length() && userId.startsWith(ID_PREFIX) ) {
                 // strip the id marker out
-                userId = userEid.substring(ID_PREFIX.length());
-                doCheckForEid = false; // skip the EID check entirely
+                userId = id.substring(ID_PREFIX.length());
+                doCheckForAid = false; // skip the AID/EID check entirely
                 doCheckForId = true;
             }
             // attempt checking both with failover by default (override by property "user.id.failover.check=false")
-            if (doCheckForEid) {
+            if (doCheckForAid) {
                 try {
-                    user = userDirectoryService.getUserByEid(userEid);
+                    // AID check falls through to EID check.
+                    user = userDirectoryService.getUserByAid(id);
                 } catch (UserNotDefinedException e) {
                     user = null;
-                    //String msg = "Could not find user with eid="+userEid;
                     if (!userIdExplicitOnly()) {
-                        //msg += " (attempting check using user id="+userId+")";
                         doCheckForId = true;
                     }
                 }

entitybroker/core-providers/src/test/org/sakaiproject/entitybroker/providers/UserEntityProviderGetUserByIdEidTest.java

@@ -0,0 +1,86 @@
+package org.sakaiproject.entitybroker.providers;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+import org.sakaiproject.entitybroker.DeveloperHelperService;
+import org.sakaiproject.user.api.User;
+import org.sakaiproject.user.api.UserDirectoryService;
+import org.sakaiproject.user.api.UserNotDefinedException;
+
+public class UserEntityProviderGetUserByIdEidTest {
+
+    private UserEntityProvider provider;
+    private UserDirectoryService uds;
+    private DeveloperHelperService dhs;
+    private User user;
+
+    @Before
+    public void setUp() {
+        provider = new UserEntityProvider();
+        uds = Mockito.mock(UserDirectoryService.class);
+        dhs = Mockito.mock(DeveloperHelperService.class);
+        user = Mockito.mock(User.class);
+
+        provider.setUserDirectoryService(uds);
+        provider.setDeveloperHelperService(dhs);
+    }
+
+    @Test
+    public void testNotFound() throws UserNotDefinedException {
+        Mockito.when(uds.getUserByAid(Mockito.anyString())).thenThrow(UserNotDefinedException.class);
+        Mockito.when(uds.getUser(Mockito.anyString())).thenThrow(UserNotDefinedException.class);
+        Mockito.when(dhs.getConfigurationSetting("user.explicit.id.only", false)).thenReturn(false);
+
+        Assert.assertNull(provider.getUserByIdEid("anything"));
+    }
+
+    @Test
+    public void testNotFoundId() throws UserNotDefinedException {
+        Mockito.when(uds.getUser(Mockito.anyString())).thenThrow(UserNotDefinedException.class);
+        Mockito.when(dhs.getConfigurationSetting("user.explicit.id.only", false)).thenReturn(false);
+
+        Assert.assertNull(provider.getUserByIdEid("id=1234"));
+        Mockito.verify(uds, Mockito.never()).getUserByEid(Mockito.anyString());
+    }
+
+    @Test
+    public void testEidFallthrough() throws UserNotDefinedException {
+        Mockito.when(uds.getUserByAid(Mockito.anyString())).thenThrow(UserNotDefinedException.class);
+        Mockito.when(uds.getUser("1234")).thenReturn(user);
+        Mockito.when(dhs.getConfigurationSetting("user.explicit.id.only", false)).thenReturn(false);
+
+        Assert.assertEquals(user, provider.getUserByIdEid("1234"));
+    }
+
+    @Test
+    public void testEidFallthroughId() throws UserNotDefinedException {
+        Mockito.when(uds.getUserByAid(Mockito.anyString())).thenThrow(UserNotDefinedException.class);
+        Mockito.when(uds.getUser("1234")).thenReturn(user);
+        Mockito.when(dhs.getConfigurationSetting("user.explicit.id.only", false)).thenReturn(false);
+
+        Assert.assertEquals(user, provider.getUserByIdEid("id=1234"));
+    }
+
+    @Test
+    public void testEidFound() throws UserNotDefinedException {
+        Mockito.when(uds.getUserByAid("1234")).thenReturn(user);
+        Mockito.when(uds.getUser(Mockito.anyString())).thenThrow(UserNotDefinedException.class);
+        Mockito.when(dhs.getConfigurationSetting("user.explicit.id.only", false)).thenReturn(false);
+
+        Assert.assertEquals(user, provider.getUserByIdEid("1234"));
+    }
+
+    @Test
+    public void testEidOnly() throws UserNotDefinedException {
+        Mockito.when(uds.getUserByAid("1234")).thenThrow(UserNotDefinedException.class);
+        Mockito.when(uds.getUser(Mockito.anyString())).thenReturn(user);
+        Mockito.when(dhs.getConfigurationSetting("user.explicit.id.only", false)).thenReturn(true);
+
+        // Check we don't fallthrough to searching by ID.
+        // This seems strange as the original bug was that EID searching was slow so why you would want
+        // to stop ID searching I'm not sure.
+        Assert.assertNull(provider.getUserByIdEid("1234"));
+    }
+}