From ddec9a7a6076933646382a6ea0e8474366afc46d Mon Sep 17 00:00:00 2001 From: Earle Nietzel Date: Thu, 27 Jun 2024 11:51:20 -0400 Subject: [PATCH] SAK-50196 Roster prevent race condition during roster loading (#12685) --- .../sakaiproject/roster/api/SakaiProxy.java | 1 - .../roster/impl/SakaiProxyImpl.java | 74 +++++-------------- roster2/tool/src/webapp/js/roster.js | 24 +++--- 3 files changed, 31 insertions(+), 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 index e844c0cb0abd..c95bed1044ec 100644 --- 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 index 7d4738c4cf60..a2d9b87690f9 100644 --- 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 getSiteUsers(String siteId) { return userDirectoryService.getUsers(site.getUsers()); } - public List getMembership(String currentUserId, String siteId, String groupId, String roleId, String enrollmentSetId, String enrollmentStatus) { - - if (currentUserId == null) { - return null; + public List getMembership(String currentUserId, String siteId, String groupId, String roleId, String enrollmentSetId, String enrollmentStatus) { + + Optional 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 rosterMembers = getAndCacheSortedMembership(site, groupId, roleId); - rosterMembers = filterMembers(site, currentUserId, rosterMembers, groupId); - return rosterMembers; - } - } - private Map getUserMap(Set members) { // Build a map of userId to User @@ -1282,33 +1272,12 @@ private Cache getCache(String cache) { } } - /** - * {@inheritDoc} - */ - public Map 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 index = (Map) cache.get(siteId+groupId); + @Override + public Map getSearchIndex(String siteId, String userId, String groupId, String roleId, String enrollmentSetId, String enrollmentStatus) { - if (MapUtils.isEmpty(index)) { - final List 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 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 index 998e7a911307..e2798da7f92d 100644 --- 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);