From bf81ccf8ed09d172baa86954d51c75c7dbbd16b0 Mon Sep 17 00:00:00 2001
From: srosse <none@none>
Date: Tue, 3 Dec 2013 15:29:32 +0100
Subject: [PATCH] OO-887: cache the usernames in UserManager, decouple the
 AvatarMapper and make it search the username in the cache.

---
 .../control/controller/BasicController.java   |  4 +-
 .../course/nodes/members/AvatarMapper.java    | 63 +++++++++++++++++++
 .../MembersCourseNodeRunController.java       | 32 +---------
 .../restapi/OpenMeetingsWebService.java       |  9 ++-
 src/main/java/org/olat/user/UserManager.java  |  2 +
 .../java/org/olat/user/UserManagerImpl.java   | 39 ++++++++----
 src/main/resources/infinispan-config.xml      |  7 +++
 7 files changed, 109 insertions(+), 47 deletions(-)
 create mode 100644 src/main/java/org/olat/course/nodes/members/AvatarMapper.java

diff --git a/src/main/java/org/olat/core/gui/control/controller/BasicController.java b/src/main/java/org/olat/core/gui/control/controller/BasicController.java
index adad4f745a7..bdfb501818b 100644
--- a/src/main/java/org/olat/core/gui/control/controller/BasicController.java
+++ b/src/main/java/org/olat/core/gui/control/controller/BasicController.java
@@ -123,6 +123,8 @@ public abstract class BasicController extends DefaultController {
 		// deregister all mappers if needed
 		if (mappers != null) {
 			CoreSpringFactory.getImpl(MapperService.class).cleanUp(mappers);
+			mappers.clear();
+			mappers = null;
 		}
 
 		// dispose child controller if needed
@@ -580,7 +582,7 @@ public abstract class BasicController extends DefaultController {
 	 * <i>Hint</i><br>
 	 * try to avoid using this - and if, comment why.
 	 */
-	protected void setBasePackage(Class clazz) {
+	protected void setBasePackage(Class<?> clazz) {
 		setVelocityRoot(Util.getPackageVelocityRoot(clazz));
 		if (fallbackTranslator != null) {
 			setTranslator(Util.createPackageTranslator(clazz, getLocale(), fallbackTranslator));
diff --git a/src/main/java/org/olat/course/nodes/members/AvatarMapper.java b/src/main/java/org/olat/course/nodes/members/AvatarMapper.java
new file mode 100644
index 00000000000..e29cc849dcc
--- /dev/null
+++ b/src/main/java/org/olat/course/nodes/members/AvatarMapper.java
@@ -0,0 +1,63 @@
+/**
+ * <a href="http://www.openolat.org">
+ * OpenOLAT - Online Learning and Training</a><br>
+ * <p>
+ * Licensed under the Apache License, Version 2.0 (the "License"); <br>
+ * you may not use this file except in compliance with the License.<br>
+ * You may obtain a copy of the License at the
+ * <a href="http://www.apache.org/licenses/LICENSE-2.0">Apache homepage</a>
+ * <p>
+ * Unless required by applicable law or agreed to in writing,<br>
+ * software distributed under the License is distributed on an "AS IS" BASIS, <br>
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. <br>
+ * See the License for the specific language governing permissions and <br>
+ * limitations under the License.
+ * <p>
+ * Initial code contributed and copyrighted by<br>
+ * frentix GmbH, http://www.frentix.com
+ * <p>
+ */
+package org.olat.course.nodes.members;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.olat.core.CoreSpringFactory;
+import org.olat.core.dispatcher.mapper.Mapper;
+import org.olat.core.gui.media.MediaResource;
+import org.olat.user.DisplayPortraitManager;
+import org.olat.user.UserManager;
+
+/**
+ * 
+ * Initial date: 03.12.2013<br>
+ * @author srosse, stephane.rosse@frentix.com, http://www.frentix.com
+ *
+ */
+public class AvatarMapper implements Mapper {
+	
+	private final UserManager userManager;
+	private final DisplayPortraitManager portraitManager;
+	
+	public AvatarMapper() {
+		portraitManager = DisplayPortraitManager.getInstance();
+		userManager = CoreSpringFactory.getImpl(UserManager.class);
+	}
+
+	@Override
+	public MediaResource handle(String relPath, HttpServletRequest request) {
+		if(relPath != null && relPath.endsWith("/portrait_small.jpg")) {
+			if(relPath.startsWith("/")) {
+				relPath = relPath.substring(1, relPath.length());
+			}
+			
+			int endKeyIndex = relPath.indexOf('/');
+			if(endKeyIndex > 0) {
+				String idKey = relPath.substring(0, endKeyIndex);
+				Long key = Long.parseLong(idKey);
+				String username = userManager.getUsername(key);
+				return portraitManager.getSmallPortraitResource(username);
+			}
+		}
+		return null;
+	}
+}
diff --git a/src/main/java/org/olat/course/nodes/members/MembersCourseNodeRunController.java b/src/main/java/org/olat/course/nodes/members/MembersCourseNodeRunController.java
index 0a03e75c6ee..dc0b1d2d476 100644
--- a/src/main/java/org/olat/course/nodes/members/MembersCourseNodeRunController.java
+++ b/src/main/java/org/olat/course/nodes/members/MembersCourseNodeRunController.java
@@ -27,12 +27,9 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 
-import javax.servlet.http.HttpServletRequest;
-
 import org.olat.NewControllerFactory;
 import org.olat.basesecurity.BaseSecurity;
 import org.olat.basesecurity.BaseSecurityManager;
-import org.olat.core.dispatcher.mapper.Mapper;
 import org.olat.core.gui.UserRequest;
 import org.olat.core.gui.components.form.flexible.FormItem;
 import org.olat.core.gui.components.form.flexible.FormItemContainer;
@@ -314,32 +311,7 @@ public class MembersCourseNodeRunController extends FormBasicController {
 	  NewControllerFactory.getInstance().launch(ureq, bwControl);
 	}
 	
-	public class AvatarMapper implements Mapper {
-
-		@Override
-		public MediaResource handle(String relPath, HttpServletRequest request) {
-			if(relPath != null && relPath.endsWith("/portrait_small.jpg")) {
-				if(relPath.startsWith("/")) {
-					relPath = relPath.substring(1, relPath.length());
-				}
-				
-				int endKeyIndex = relPath.indexOf('/');
-				if(endKeyIndex > 0) {
-					String idKey = relPath.substring(0, endKeyIndex);
-					Long key = Long.parseLong(idKey);
-					for(FormLink memberLink:memberLinks) {
-						Member m = (Member)memberLink.getUserObject();
-						if(m.getIdentity().getKey().equals(key)) {
-							return portraitManager.getSmallPortraitResource(m.getIdentity().getName());
-						}
-					}
-				}
-			}
-			return null;
-		}
-	}
-	
-	public class Member {
+	public static class Member {
 		private final String firstName;
 		private final String lastName;
 		private final Long key;
@@ -411,7 +383,7 @@ public class MembersCourseNodeRunController extends FormBasicController {
 		}
 	}
 	
-	public class IdentityComparator implements Comparator<Identity> {
+	public static class IdentityComparator implements Comparator<Identity> {
 
 		@Override
 		public int compare(Identity id1, Identity id2) {
diff --git a/src/main/java/org/olat/modules/openmeetings/restapi/OpenMeetingsWebService.java b/src/main/java/org/olat/modules/openmeetings/restapi/OpenMeetingsWebService.java
index 16c93788af4..0c4388348f9 100644
--- a/src/main/java/org/olat/modules/openmeetings/restapi/OpenMeetingsWebService.java
+++ b/src/main/java/org/olat/modules/openmeetings/restapi/OpenMeetingsWebService.java
@@ -34,12 +34,11 @@ import javax.ws.rs.core.Request;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.Response.Status;
 
-import org.olat.basesecurity.BaseSecurityManager;
 import org.olat.core.CoreSpringFactory;
-import org.olat.core.id.Identity;
 import org.olat.modules.openmeetings.OpenMeetingsModule;
 import org.olat.modules.openmeetings.manager.OpenMeetingsManager;
 import org.olat.user.DisplayPortraitManager;
+import org.olat.user.UserManager;
 
 /**
  * 
@@ -76,12 +75,12 @@ public class OpenMeetingsWebService {
 			if(identityKey == null) {
 				return Response.serverError().status(Status.NOT_FOUND).build();
 			}
-			Identity identity = BaseSecurityManager.getInstance().loadIdentityByKey(identityKey, false);
-			if(identity == null) {
+			String username = CoreSpringFactory.getImpl(UserManager.class).getUsername(identityKey);
+			if(username == null) {
 				return Response.serverError().status(Status.NOT_FOUND).build();
 			}
 			
-			File portrait = DisplayPortraitManager.getInstance().getBigPortrait(identity.getName());
+			File portrait = DisplayPortraitManager.getInstance().getBigPortrait(username);
 			if(portrait == null || !portrait.exists()) {
 				return Response.serverError().status(Status.NOT_FOUND).build();
 			}
diff --git a/src/main/java/org/olat/user/UserManager.java b/src/main/java/org/olat/user/UserManager.java
index dd30d374def..bd9ace2290d 100644
--- a/src/main/java/org/olat/user/UserManager.java
+++ b/src/main/java/org/olat/user/UserManager.java
@@ -242,6 +242,8 @@ public abstract class UserManager extends BasicManager {
 	public void setUserPropertiesConfig(UserPropertiesConfig userPropertiesConfig) {
 		this.userPropertiesConfig = userPropertiesConfig;
 	}
+	
+	public abstract String getUsername(Long identityKey);
 
 	/**
 	 * Returns the users displayable name, e.g. "Firstname Lastname"
diff --git a/src/main/java/org/olat/user/UserManagerImpl.java b/src/main/java/org/olat/user/UserManagerImpl.java
index 19b5aab47fc..72679ab6343 100644
--- a/src/main/java/org/olat/user/UserManagerImpl.java
+++ b/src/main/java/org/olat/user/UserManagerImpl.java
@@ -75,7 +75,8 @@ public class UserManagerImpl extends UserManager {
   @Autowired
   private CoordinatorManager coordinatorManager;
 
-	private CacheWrapper<Serializable,String> usernameCache;
+	private CacheWrapper<Serializable,String> userToFullnameCache;
+	private CacheWrapper<Long,String> userToNameCache;
   
 	/**
 	 * Use UserManager.getInstance(), this is a spring factory method to load the
@@ -87,8 +88,11 @@ public class UserManagerImpl extends UserManager {
 	
 	@PostConstruct
 	public void init() {
-		usernameCache = coordinatorManager.getCoordinator().getCacher()
+		userToFullnameCache = coordinatorManager.getCoordinator().getCacher()
+				.getCache(UserManager.class.getSimpleName(), "userfullname");
+		userToNameCache = coordinatorManager.getCoordinator().getCacher()
 				.getCache(UserManager.class.getSimpleName(), "username");
+		
 	}
 
 	/**
@@ -407,6 +411,7 @@ public class UserManagerImpl extends UserManager {
 	 * Delete all user-properties which are deletable.
 	 * @param user
 	 */
+	@Override
 	public void deleteUserProperties(User user) {
 		// prevent stale objects, reload first
 		user = loadUserByKey(user.getKey());
@@ -423,12 +428,21 @@ public class UserManagerImpl extends UserManager {
 		updateUser(user);
 		if(isLogDebugEnabled()) logDebug("Delete all user-attributtes for user=" + user);
 	}
-	
-	
+
+	@Override
+	public String getUsername(Long identityKey) {
+		String username = userToNameCache.get(identityKey);
+		if(username == null) {
+			IdentityShort identity = securityManager.loadIdentityShortByKey(identityKey);
+			getUserDisplayName(identity);//fill the cache
+			username = identity.getName();
+		}
+		return username;
+	}
 
 	@Override
 	public String getUserDisplayName(String username) {
-		String fullName = usernameCache.get(username);
+		String fullName = userToFullnameCache.get(username);
 		if(fullName == null) {
 			List<IdentityShort> identities = securityManager.findShortIdentitiesByName(Collections.singletonList(username));
 			for(IdentityShort identity:identities) {
@@ -444,7 +458,7 @@ public class UserManagerImpl extends UserManager {
 			return "";
 		}
 		
-		String fullName = usernameCache.get(identityKey);
+		String fullName = userToFullnameCache.get(identityKey);
 		if(fullName == null) {
 			IdentityShort identity = securityManager.loadIdentityShortByKey(identityKey);
 			fullName = getUserDisplayName(identity);
@@ -461,7 +475,7 @@ public class UserManagerImpl extends UserManager {
 		Map<String, String> fullNames = new HashMap<String,String>();
 		List<String> newUsernames = new ArrayList<String>();
 		for(String username:usernames) {
-			String fullName = usernameCache.get(username);
+			String fullName = userToFullnameCache.get(username);
 			if(fullName != null) {
 				fullNames.put(username, fullName);
 			} else {
@@ -477,7 +491,7 @@ public class UserManagerImpl extends UserManager {
 		}
 		//not found
 		for(String notFound:newUsernames) {
-			usernameCache.put(notFound, notFound);
+			userToFullnameCache.put(notFound, notFound);
 		}
 		return fullNames;
 	}
@@ -520,7 +534,7 @@ public class UserManagerImpl extends UserManager {
 		Map<Long, String> fullNames = new HashMap<Long,String>();
 		List<Long> newIdentityKeys = new ArrayList<Long>();
 		for(Long identityKey:identityKeys) {
-			String fullName = usernameCache.get(identityKey);
+			String fullName = userToFullnameCache.get(identityKey);
 			if(fullName != null) {
 				fullNames.put(identityKey, fullName);
 			} else {
@@ -542,10 +556,13 @@ public class UserManagerImpl extends UserManager {
 		if(fullName == null) return;
 		
 		if(identityKey != null) {
-			usernameCache.put(identityKey, fullName);
+			userToFullnameCache.put(identityKey, fullName);
 		}
 		if(username != null) {
-			usernameCache.put(username, fullName);
+			userToFullnameCache.put(username, fullName);
+		}
+		if(username != null && identityKey != null) {
+			userToNameCache.put(identityKey, username);
 		}
 	}
 
diff --git a/src/main/resources/infinispan-config.xml b/src/main/resources/infinispan-config.xml
index 5a08dcfed1d..d1f1d3e1bdc 100644
--- a/src/main/resources/infinispan-config.xml
+++ b/src/main/resources/infinispan-config.xml
@@ -59,6 +59,13 @@
 		<expiration maxIdle="2700000" wakeUpInterval="15000"/>
 		<transaction transactionMode="NON_TRANSACTIONAL" />
 	</namedCache>
+	
+	<namedCache name="UserManager@userfullname">
+		<locking isolationLevel="READ_COMMITTED" concurrencyLevel="1000" lockAcquisitionTimeout="15000" useLockStriping="false"/>
+		<eviction maxEntries="20000" strategy="LIRS"/>
+		<expiration maxIdle="2700000" wakeUpInterval="15000"/>
+		<transaction transactionMode="NON_TRANSACTIONAL" />
+	</namedCache>
 		
 	<namedCache name="LoginModule@blockafterfailedattempts">
 		<locking isolationLevel="READ_COMMITTED" concurrencyLevel="1000" lockAcquisitionTimeout="15000" useLockStriping="false"/>
-- 
GitLab