From 9441e12ca0517e4f346e335f3060a0fca4d2c634 Mon Sep 17 00:00:00 2001
From: srosse <none@none>
Date: Fri, 29 Sep 2017 15:59:44 +0200
Subject: [PATCH] OO-3057: tweak performance of the synchronization of LDAP
 groups, make a set of identity keys to match the users

---
 .../ldap/manager/LDAPLoginManagerImpl.java    | 40 ++++++++++++++-----
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/src/main/java/org/olat/ldap/manager/LDAPLoginManagerImpl.java b/src/main/java/org/olat/ldap/manager/LDAPLoginManagerImpl.java
index 6e0999a7a90..290c8637b1f 100644
--- a/src/main/java/org/olat/ldap/manager/LDAPLoginManagerImpl.java
+++ b/src/main/java/org/olat/ldap/manager/LDAPLoginManagerImpl.java
@@ -24,6 +24,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Date;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.List;
@@ -1275,12 +1276,17 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, GenericEventListe
 			doSyncGroupByAttribute(ldapUsers, cnToGroupMap);
 		}
 		
+		int syncGroupCount = 0;
 		for(LDAPGroup group:cnToGroupMap.values()) {
 			BusinessGroup managedGroup = getManagerBusinessGroup(group.getCommonName());
 			if(managedGroup != null) {
 				syncBusinessGroup(ctx, managedGroup, group, dnToIdentityKeyMap, errors);
 			}
 			dbInstance.commitAndCloseSession();
+			if(syncGroupCount % 100 == 0) {
+				log.info("Synched " + syncGroupCount + "/" + cnToGroupMap.size() + " LDAP groups");
+			}
+			syncGroupCount++;
 		}
 	}
 	
@@ -1318,14 +1324,22 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, GenericEventListe
 	private void syncBusinessGroup(LdapContext ctx, BusinessGroup businessGroup, LDAPGroup ldapGroup, Map<String,LDAPUser> dnToIdentityKeyMap, LDAPError errors) {
 		List<Identity> currentMembers = businessGroupRelationDao
 				.getMembers(businessGroup, GroupRoles.coach.name(), GroupRoles.participant.name());
+		Set<Long> currentMemberKeys = new HashSet<>();
+		for(Identity currentMember:currentMembers) {
+			currentMemberKeys.add(currentMember.getKey());
+		}
 
 		List<LDAPUser> coaches = new ArrayList<>(ldapGroup.getCoaches());
 		List<LDAPUser> participants = new ArrayList<>(ldapGroup.getParticipants());
 		// transfer member cn's to the participants list
 		for(String member:ldapGroup.getMembers()) {
-			LDAPUser ldapUser = getLDAPUser(ctx, member, dnToIdentityKeyMap, errors); dnToIdentityKeyMap.get(member);
-			if(ldapUser != null && !participants.contains(ldapUser)) {
-				participants.add(ldapUser);
+			try {
+				LDAPUser ldapUser = getLDAPUser(ctx, member, dnToIdentityKeyMap, errors); dnToIdentityKeyMap.get(member);
+				if(ldapUser != null && !participants.contains(ldapUser)) {
+					participants.add(ldapUser);
+				}
+			} catch (Exception e) {
+				log.error("Cannot retrieve this LDAP group member: " + member, e);
 			}
 		}
 		// transfer to ldap user flagged as coach to the coach list
@@ -1342,25 +1356,31 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, GenericEventListe
 		int count = 0;
 		for(LDAPUser participant:participants) {
 			IdentityRef memberIdentity = participant.getCachedIdentity();
-			syncMembership(businessGroup, memberIdentity, false);
-			currentMembers.remove(memberIdentity);
-			
+			if(memberIdentity != null && memberIdentity.getKey() != null) {
+				syncMembership(businessGroup, memberIdentity, false);
+				currentMemberKeys.remove(memberIdentity.getKey());
+			}
 			if(count % 20 == 0) {
 				dbInstance.commitAndCloseSession();
 			}
+			count++;
 		}
 		
 		for(LDAPUser coach:coaches) {
 			IdentityRef memberIdentity = coach.getCachedIdentity();
-			syncMembership(businessGroup, memberIdentity, true);
-			currentMembers.remove(memberIdentity);
+			if(memberIdentity != null && memberIdentity.getKey() != null) {
+				syncMembership(businessGroup, memberIdentity, true);
+				currentMemberKeys.remove(memberIdentity.getKey());
+			}
 			
 			if(count % 20 == 0) {
 				dbInstance.commitAndCloseSession();
 			}
+			count++;
 		}
 		
-		for(Identity currentMember:currentMembers) {
+		for(Long currentMemberKey:currentMemberKeys) {
+			Identity currentMember = securityManager.loadIdentityByKey(currentMemberKey);
 			List<String> roles = businessGroupRelationDao.getRoles(currentMember, businessGroup);
 			for(String role:roles) {
 				businessGroupRelationDao.removeRole(currentMember, businessGroup, role);
@@ -1369,7 +1389,9 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, GenericEventListe
 			if(count % 20 == 0) {
 				dbInstance.commitAndCloseSession();
 			}
+			count++;
 		}
+		dbInstance.commitAndCloseSession();
 	}
 	
 	private void syncMembership(BusinessGroup businessGroup, IdentityRef identityRef, boolean coach) {
-- 
GitLab