SAK-50196 Roster prevent race condition during roster loading (sakaip…

…roject#12685)
axxter99 · Jun 27, 2024 · ddec9a7 · ddec9a7
1 parent 017e85c
commit ddec9a7
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 68 deletions.
diff --git a/roster2/tool/src/java/org/sakaiproject/roster/api/SakaiProxy.java b/roster2/tool/src/java/org/sakaiproject/roster/api/SakaiProxy.java
@@ -54,7 +54,6 @@ public interface SakaiProxy {
 
     public final static String MEMBERSHIPS_CACHE = "org.sakaiproject.roster.sortedMembershipsCache";
     public final static String ENROLLMENTS_CACHE = "org.sakaiproject.roster.sortedEnrollmentsCache";
-    public final static String SEARCH_INDEX_CACHE = "org.sakaiproject.roster.searchIndexCache";
 
 	public final static String DEFAULT_SORT_COLUMN = "sortName";
 	public final static String DEFAULT_OVERVIEW_MODE = "cards";

diff --git a/roster2/tool/src/java/org/sakaiproject/roster/impl/SakaiProxyImpl.java b/roster2/tool/src/java/org/sakaiproject/roster/impl/SakaiProxyImpl.java
@@ -37,7 +37,6 @@
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.net.URISyntaxException;
-import java.util.concurrent.TimeUnit;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -52,14 +51,12 @@
 import java.util.Observer;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import javax.annotation.Resource;
 
-
-import org.apache.commons.collections4.MapUtils;
 import org.apache.commons.lang3.ArrayUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.time.StopWatch;
@@ -484,29 +481,22 @@ public List<User> getSiteUsers(String siteId) {
         return userDirectoryService.getUsers(site.getUsers());
     }
 
-	public List<RosterMember> getMembership(String currentUserId, String siteId, String groupId, String roleId, String enrollmentSetId, String enrollmentStatus) {
-
-        if (currentUserId == null) {
-            return null;
+    public List<RosterMember> getMembership(String currentUserId, String siteId, String groupId, String roleId, String enrollmentSetId, String enrollmentStatus) {
+
+        Optional<Site> optionalSite = siteService.getOptionalSite(siteId);
+        if (StringUtils.isNotBlank(currentUserId) && optionalSite.isPresent()) {
+            Site site = optionalSite.get();
+            if (site.isType("course") && StringUtils.isNotBlank(enrollmentSetId)) {
+                return Optional.ofNullable(getEnrollmentMembership(site, enrollmentSetId, enrollmentStatus, currentUserId))
+                        .orElse(Collections.emptyList());
+            } else {
+                return Optional.ofNullable(filterMembers(site, currentUserId, getAndCacheSortedMembership(site, groupId, roleId), groupId))
+                        .orElse(Collections.emptyList());
+            }
         }
+        return Collections.emptyList();
+    }
 
-		Site site = null;
-		try {
-			site = siteService.getSite(siteId);
-		} catch (IdUnusedException e) {
-			log.error("Site '" + siteId + "' not found. Returning null ...");
-            return null;
-		}
-
-        if (site.isType("course") && enrollmentSetId != null) {
-            return getEnrollmentMembership(site, enrollmentSetId, enrollmentStatus, currentUserId);
-        } else {
-            List<RosterMember> rosterMembers = getAndCacheSortedMembership(site, groupId, roleId);
-            rosterMembers = filterMembers(site, currentUserId, rosterMembers, groupId);
-            return rosterMembers;
-        }
-	}
-
     private Map<String, User> getUserMap(Set<Member> members) {
 
         // Build a map of userId to User
@@ -1282,33 +1272,12 @@ private Cache getCache(String cache) {
         }
     }
 
-	/**
-	 * {@inheritDoc}
-	 */
-    public Map<String, String> getSearchIndex(String siteId, String userId, String groupId, String roleId, String enrollmentSetId, String enrollmentStatus) {
-
-        try {
-            // Try and load the sorted memberships from the cache
-            Cache cache = memoryService.getCache(SEARCH_INDEX_CACHE);
-
-            if (cache == null) {
-                cache = memoryService.createCache(SEARCH_INDEX_CACHE, new SimpleConfiguration(0));
-            }
-
-            Map<String, String> index = (Map<String, String>) cache.get(siteId+groupId);
+	@Override
+	public Map<String, String> getSearchIndex(String siteId, String userId, String groupId, String roleId, String enrollmentSetId, String enrollmentStatus) {
 
-            if (MapUtils.isEmpty(index)) {
-                final List<RosterMember> membership = getMembership(userId, siteId, groupId, roleId, enrollmentSetId, enrollmentStatus);
-                index = Optional.ofNullable(membership).map(Collection::stream).orElseGet(Stream::empty)
-                        .collect(Collectors.toMap(RosterMember::getUserId, RosterMember::getDisplayName));
-                cache.put(siteId+groupId, index);
-            }
-
-		    return index;
-        } catch (Exception e) {
-            log.error("Exception whilst retrieving search index for site '" + siteId + "'. Returning null ...", e);
-            return null;
-        }
+		return getMembership(userId, siteId, groupId, roleId, enrollmentSetId, enrollmentStatus)
+				.stream()
+				.collect(Collectors.toMap(RosterMember::getUserId, RosterMember::getDisplayName));
     }
 
     public Map<String, SitePresenceTotal> getPresenceTotalsForSite(String siteId) {
@@ -1394,9 +1363,6 @@ private void removeSiteRosterCache(String siteId){
         Cache enrollmentsCache = getCache(ENROLLMENTS_CACHE);
         enrollmentsCache.remove(siteId);
 
-        Cache searchIndexCache = memoryService.getCache(SEARCH_INDEX_CACHE);
-        searchIndexCache.remove(siteId);
-
         Cache membershipsCache = getCache(MEMBERSHIPS_CACHE);
 
         synchronized(this) {

diff --git a/roster2/tool/src/webapp/js/roster.js b/roster2/tool/src/webapp/js/roster.js
@@ -399,6 +399,7 @@ roster.renderMembership = function (options) {
     url: url,
     dataType: "json",
     cache: false,
+    async: false,
     success: function (data) {
 
       if (data.status && data.status === 'END') {
@@ -828,20 +829,17 @@ roster.init = function () {
   roster.nextPage = 0;
   roster.currentState = null;
 
-  this.searchIndexPromise = new Promise((resolve, reject) => {
-
-    $.ajax({
-      url: '/direct/roster-membership/' + roster.siteId + '/get-search-index.json',
-      dataType: "json",
-      success: function (data) {
+  this.searchIndexPromise = $.ajax({
+    url: '/direct/roster-membership/' + roster.siteId + '/get-search-index.json',
+    dataType: "json",
+    async: false,
+    success: function (data) {
 
-        roster.searchIndex = data.data;
-        roster.searchIndexKeys = Object.keys(data.data);
-        roster.searchIndexValues = roster.searchIndexKeys.map(function (k) { return data.data[k] });
-        resolve();
-      },
-      error: () => reject()
-    });
+      roster.searchIndex = data.data;
+      roster.searchIndexKeys = Object.keys(data.data);
+      roster.searchIndexValues = roster.searchIndexKeys.map(function (k) { return data.data[k] });
+    },
+    error: () => console.error("failure retrieving search index data")
   });
 
   roster.switchState(roster.state, roster);