From 8eaec0b14f0042c16487210e9a4834db9e63d04d Mon Sep 17 00:00:00 2001
From: Sam Ottenhoff <ottenhoff@longsight.com>
Date: Fri, 10 May 2024 15:45:13 -0400
Subject: [PATCH] SAK-49990 Roster the profile link is unique to current user
 and cant be part of cache

---
 .../roster/impl/SakaiProxyImpl.java           | 55 +++++++------------
 1 file changed, 20 insertions(+), 35 deletions(-)

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 528ed83e9ca6..7d4738c4cf60 100644
--- a/roster2/tool/src/java/org/sakaiproject/roster/impl/SakaiProxyImpl.java
+++ b/roster2/tool/src/java/org/sakaiproject/roster/impl/SakaiProxyImpl.java
@@ -652,10 +652,10 @@ private List<RosterMember> filterMembers(Site site, String currentUserId, List<R
 				}
 
 				// Now add any instructors
-				filtered.addAll(unfiltered.stream().filter(m -> m.isInstructor()).collect(Collectors.toList()));
+				filtered.addAll(unfiltered.stream().filter(RosterMember::isInstructor).collect(Collectors.toList()));
 
 				// The group loop is shuffling members, sort the list again
-				Collections.sort(filtered, memberComparator);
+				filtered.sort(memberComparator);
 			} else if (null != site.getGroup(groupId)) {
 				// get all members of requested groupId if current user is
 				// member
@@ -678,7 +678,12 @@ private List<RosterMember> filterMembers(Site site, String currentUserId, List<R
 		for (RosterMember m : filtered) {
 			if (check.add(m.getUserId())) {
 				cleanedMembers.add(m);
-                // Now strip out any unauthorised info
+
+				// Apply a unique profile link to each user outside of the caching layer
+				// e.g., /portal/site/~current-user-id/tool/profile2tooluuid/otherUserId
+				m.setProfileLink(getProfileToolLink(m.getUserId()));
+
+				// Now strip out any unauthorised info
                 if (!isAllowed(currentUserId, RosterFunctions.ROSTER_FUNCTION_VIEWEMAIL, site.getReference())) {
                     m.setEmail(null);
                 } else {
@@ -803,8 +808,6 @@ private RosterMember getRosterMember(Map<String, User> userMap, Collection<Group
 			}
 		}
 
-		rosterMember.setProfileLink(getProfileToolLink(userId));
-
 		// See if there is a pronunciation available for the user
 		String pronunciation = pronounceMap.get(user.getId());
 		//Try by email instead of Id
@@ -892,28 +895,14 @@ private List<RosterMember> getEnrollmentMembership(Site site, String enrollmentS
      *  RESPONSIBILITY TO FILTER ON AUTHZ RULES.
      */
     private List<RosterMember> getAndCacheSortedMembership(Site site, String groupId, String roleId) {
-
         String siteId = site.getId();
-
         Cache cache = getCache(MEMBERSHIPS_CACHE);
-
-        String key = siteId;
-
-        if (groupId != null) {
-            key += "#" + groupId;
-        }
-
-        if(roleId != null) {
-            key += "#" + roleId;
-        }
-
-        log.debug("Key: {}", key);
+        String key = siteId + (groupId == null ? "" : "#" + groupId) + (roleId == null ? "" : "#" + roleId);
 
         List<RosterMember> siteMembers = (List<RosterMember>) cache.get(key);
+		log.debug("Trying to get '{}' from cache. Is null? {}", key, siteMembers == null);
 
         if (siteMembers == null) {
-            log.debug("Cache miss on '{}'.", key);
-
             Set<Member> membership = site.getMembers();
 
             if (null == membership) {
@@ -955,18 +944,18 @@ private List<RosterMember> getAndCacheSortedMembership(Site site, String groupId
 					}
 					cacheMembersMap.get(siteId + "#" + memberRoleId).add(rosterMember);
 				} catch (UserNotDefinedException e) {
-					log.warn("user not found: " + e.getId());
+					log.warn("user not found in membership: {}", e.getId());
 				}
 			}
 
 			cacheMembersMap.put(siteId, siteMembers);
 			log.debug("Caching on '{}' ...", siteId);
 
-			cacheMembersMap.values().forEach(a -> Collections.sort(a, memberComparator));
+			cacheMembersMap.values().forEach(a -> a.sort(memberComparator));
 			cache.putAll(cacheMembersMap);
-			return (List<RosterMember>) cache.get(key);
+			siteMembers = (List<RosterMember>) cache.get(key);
         }
-		log.debug("Cache hit on '{}'.", key);
+
 		return siteMembers;
     }
 
@@ -980,16 +969,14 @@ private List<RosterMember> getAndCacheSortedMembership(Site site, String groupId
      *  RESPONSIBILITY TO FILTER ON AUTHZ RULES.
      */
     private Map<String, List<RosterMember>> getAndCacheSortedEnrollmentSet(Site site, String enrollmentSetId) {
-
         String siteId = site.getId();
-
         Cache cache = getCache(ENROLLMENTS_CACHE);
 
-        log.debug("Trying to get '{}' from enrollments cache ...", siteId);
+
         Map<String, List<RosterMember>> membersMap = (Map<String, List<RosterMember>>) cache.get(siteId);
+		log.debug("Trying to get '{}' from enrollments cache. Is null? {}", siteId, membersMap == null);
 
         if (membersMap == null) {
-            log.debug("Cache miss. Putting empty membersMap on {} ...", siteId);
             membersMap = new HashMap<>();
             cache.put(siteId, membersMap);
         }
@@ -998,7 +985,6 @@ private Map<String, List<RosterMember>> getAndCacheSortedEnrollmentSet(Site site
                 && membersMap.containsKey(enrollmentSetId + "#wait")
                 && membersMap.containsKey(enrollmentSetId + "#enrolled")) {
             log.debug("Cache hit on '{}'", enrollmentSetId);
-            return membersMap;
         } else {
             log.debug("Cache miss on '{}'", enrollmentSetId);
 
@@ -1039,9 +1025,9 @@ private Map<String, List<RosterMember>> getAndCacheSortedEnrollmentSet(Site site
                 members.add(member);
             }
 
-            Collections.sort(members, memberComparator);
-            Collections.sort(waiting, memberComparator);
-            Collections.sort(enrolled, memberComparator);
+            members.sort(memberComparator);
+            waiting.sort(memberComparator);
+            enrolled.sort(memberComparator);
 
             log.debug("Caching all enrollment set members on '{}#all' ...", enrollmentSetId);
             membersMap.put(enrollmentSetId + "#all", members);
@@ -1049,9 +1035,8 @@ private Map<String, List<RosterMember>> getAndCacheSortedEnrollmentSet(Site site
             membersMap.put(enrollmentSetId + "#wait", waiting);
             log.debug("Caching enrolled members on '{}#enrolled' ...", enrollmentSetId);
             membersMap.put(enrollmentSetId + "#enrolled", enrolled);
-
-            return membersMap;
         }
+        return membersMap;
     }
 	
 	/**