From 9153eb6334ac8f55ecba8d5f5e227161c03e73a4 Mon Sep 17 00:00:00 2001
From: srosse <none@none>
Date: Thu, 28 May 2015 10:52:37 +0200
Subject: [PATCH] OO-1559: add sync of coaches by ldap user attributes,
 refactor the batch sync to handle mix configuration with attributes and group
 bases at the same time

---
 .../olat/group/manager/BusinessGroupDAO.java  |   4 +-
 .../org/olat/ldap/LDAPSyncConfiguration.java  |  21 ++-
 .../org/olat/ldap/_spring/ldapContext.xml     |   2 +
 .../java/org/olat/ldap/manager/LDAPDAO.java   |   3 +
 .../ldap/manager/LDAPLoginManagerImpl.java    | 159 +++++++++---------
 .../olat/ldap/manager/LDAPUserVisitor.java    |  18 +-
 .../java/org/olat/ldap/model/LDAPGroup.java   |  11 ++
 .../java/org/olat/ldap/model/LDAPUser.java    |   9 +
 .../resources/serviceconfig/olat.properties   |   3 +
 9 files changed, 143 insertions(+), 87 deletions(-)

diff --git a/src/main/java/org/olat/group/manager/BusinessGroupDAO.java b/src/main/java/org/olat/group/manager/BusinessGroupDAO.java
index 3560cea0024..5fb10d2ab5c 100644
--- a/src/main/java/org/olat/group/manager/BusinessGroupDAO.java
+++ b/src/main/java/org/olat/group/manager/BusinessGroupDAO.java
@@ -679,7 +679,7 @@ public class BusinessGroupDAO {
 		}
 		
 		if(StringHelper.containsNonWhitespace(params.getIdRef())) {
-			dbq.setParameter("idRefString", params.getExternalId());
+			dbq.setParameter("idRefString", params.getIdRef());
 			if(id != null) {
 				dbq.setParameter("idRefLong", id);
 			}
@@ -942,7 +942,7 @@ public class BusinessGroupDAO {
 			dbq.setParameter("groupKeys", params.getGroupKeys());
 		}
 		if(StringHelper.containsNonWhitespace(params.getIdRef())) {
-			dbq.setParameter("idRefString", params.getExternalId());
+			dbq.setParameter("idRefString", params.getIdRef());
 			if(id != null) {
 				dbq.setParameter("idRefLong", id);
 			}
diff --git a/src/main/java/org/olat/ldap/LDAPSyncConfiguration.java b/src/main/java/org/olat/ldap/LDAPSyncConfiguration.java
index 2543aae64c3..ecddb9724cf 100644
--- a/src/main/java/org/olat/ldap/LDAPSyncConfiguration.java
+++ b/src/main/java/org/olat/ldap/LDAPSyncConfiguration.java
@@ -66,6 +66,9 @@ public class LDAPSyncConfiguration {
 	
 	private String groupAttribute;
 	private String groupAttributeSeparator;
+
+	private String coachedGroupAttribute;
+	private String coachedGroupAttributeSeparator;
 	
 	private Map<String, String> requestAttributes;
 	private Map<String, String> userAttributeMap;
@@ -178,7 +181,7 @@ public class LDAPSyncConfiguration {
 	}
 	
 	public boolean syncGroupWithAttribute() {
-		return StringHelper.containsNonWhitespace(groupAttribute);
+		return StringHelper.containsNonWhitespace(groupAttribute) || StringHelper.containsNonWhitespace(coachedGroupAttribute);
 	}
 
 	public String getCoachRoleAttribute() {
@@ -213,6 +216,22 @@ public class LDAPSyncConfiguration {
 		this.groupAttributeSeparator = groupAttributeSeparator;
 	}
 
+	public String getCoachedGroupAttribute() {
+		return coachedGroupAttribute;
+	}
+
+	public void setCoachedGroupAttribute(String coachedGroupAttribute) {
+		this.coachedGroupAttribute = coachedGroupAttribute;
+	}
+
+	public String getCoachedGroupAttributeSeparator() {
+		return coachedGroupAttributeSeparator;
+	}
+
+	public void setCoachedGroupAttributeSeparator(String coachedGroupAttributeSeparator) {
+		this.coachedGroupAttributeSeparator = coachedGroupAttributeSeparator;
+	}
+
 	public List<String> getAuthorsGroupBase() {
 		return authorsGroupBase;
 	}
diff --git a/src/main/java/org/olat/ldap/_spring/ldapContext.xml b/src/main/java/org/olat/ldap/_spring/ldapContext.xml
index dd7aacd7bef..5b1ce8b8cc0 100644
--- a/src/main/java/org/olat/ldap/_spring/ldapContext.xml
+++ b/src/main/java/org/olat/ldap/_spring/ldapContext.xml
@@ -34,6 +34,8 @@
 
 		<property name="groupAttribute" value="${ldap.user.groupAttribute}"/>
 		<property name="groupAttributeSeparator" value="${ldap.user.groupAttributeSeparator}"/>
+		<property name="coachedGroupAttribute" value="${ldap.user.coachedGroupAttribute}"/>
+		<property name="coachedGroupAttributeSeparator" value="${ldap.user.coachedGroupAttributeSeparator}"/>
 		<property name="coachRoleAttribute" value="${ldap.coachRoleAttribute}"/>
 		<property name="coachRoleValue" value="${ldap.coachRoleValue}"/>
 		
diff --git a/src/main/java/org/olat/ldap/manager/LDAPDAO.java b/src/main/java/org/olat/ldap/manager/LDAPDAO.java
index b9029618676..293a8be28e7 100644
--- a/src/main/java/org/olat/ldap/manager/LDAPDAO.java
+++ b/src/main/java/org/olat/ldap/manager/LDAPDAO.java
@@ -305,6 +305,9 @@ public class LDAPDAO {
 		if(StringHelper.containsNonWhitespace(syncConfiguration.getGroupAttribute())) {
 			userAttrList.add(syncConfiguration.getGroupAttribute());
 		}
+		if(StringHelper.containsNonWhitespace(syncConfiguration.getCoachedGroupAttribute())) {
+			userAttrList.add(syncConfiguration.getCoachedGroupAttribute());
+		}
 		return userAttrList.toArray(new String[userAttrList.size()]);
 	}
 	
diff --git a/src/main/java/org/olat/ldap/manager/LDAPLoginManagerImpl.java b/src/main/java/org/olat/ldap/manager/LDAPLoginManagerImpl.java
index 988842e597b..0c934d5204b 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.Date;
 import java.util.HashMap;
 import java.util.Hashtable;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -748,19 +749,6 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, GenericEventListe
 		}
 	}
 	
-	private void doBatchSyncGroups(LdapContext ctx, List<LDAPUser> ldapUsers, Map<String,LDAPUser> dnToIdentityKeyMap, LDAPError errors)
-	throws NamingException {
-		ctx.close();
-		ctx = bindSystem();
-		//sync groups by LDAP groups or attributes
-		if(syncConfiguration.syncGroupWithLDAPGroup()) {
-			doSyncLDAPGroups(ctx, dnToIdentityKeyMap, errors);
-		}
-		if(syncConfiguration.syncGroupWithAttribute()) {
-			doSyncGroupAttribute(ldapUsers);
-		}
-	}
-	
 	private void doBatchSyncRoles(LdapContext ctx, List<LDAPUser> ldapUsers, Map<String,LDAPUser> dnToIdentityKeyMap, LDAPError errors)
 	throws NamingException {
 		ctx.close();
@@ -1024,44 +1012,101 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, GenericEventListe
 		return ldapUserList;
 	}
 	
-	private void doSyncGroupAttribute(List<LDAPUser> ldapUsers) {
-		log.info("LDAP batch sync LDAP user attributes to OO groups");
-		Map<String,List<LDAPUser>> externelIdToGroupMap = new HashMap<>();
+	private void doBatchSyncGroups(LdapContext ctx, List<LDAPUser> ldapUsers, Map<String,LDAPUser> dnToIdentityKeyMap, LDAPError errors)
+	throws NamingException {
+		ctx.close();
+		
+		log.info("LDAP batch sync LDAP user to OO groups");
+		
+		ctx = bindSystem();
+		//sync groups by LDAP groups or attributes
+		Map<String,LDAPGroup> cnToGroupMap = new HashMap<>();
+		
+		// retrieve all ldap group's with their list of members
+		if(syncConfiguration.syncGroupWithLDAPGroup()) {
+			List<String> groupDNs = syncConfiguration.getLdapGroupBases();
+			List<LDAPGroup> ldapGroups = ldapDao.searchGroups(ctx, groupDNs);
+			for(LDAPGroup ldapGroup:ldapGroups) {
+				cnToGroupMap.put(ldapGroup.getCommonName(), ldapGroup);
+			}
+		}
+		if(syncConfiguration.syncGroupWithAttribute()) {
+			doSyncGroupByAttribute(ldapUsers, cnToGroupMap);
+		}
+		
+		for(LDAPGroup group:cnToGroupMap.values()) {
+			BusinessGroup managedGroup = getManagerBusinessGroup(group.getCommonName());
+			if(managedGroup != null) {
+				syncBusinessGroup(ctx, managedGroup, group, dnToIdentityKeyMap, errors);
+			}
+			dbInstance.commitAndCloseSession();
+		}
+	}
+	
+	private void doSyncGroupByAttribute(List<LDAPUser> ldapUsers, Map<String,LDAPGroup> cnToGroupMap) {
 		for(LDAPUser ldapUser:ldapUsers) {
 			List<String> groupIds = ldapUser.getGroupIds();
-			if(groupIds != null && groupIds.size() > 0) {
+			List<String> coachedGroupIds = ldapUser.getCoachedGroupIds();
+			if((groupIds != null && groupIds.size() > 0) || (coachedGroupIds != null && coachedGroupIds.size() > 0)) {
 				Identity identity = ldapUser.getCachedIdentity();
 				if(identity == null) {
 					log.error("Identity with dn=" + ldapUser.getDn() + " not found");
 				} else {
-					for(String groupId:groupIds) {
-						if(!externelIdToGroupMap.containsKey(groupId)) {
-							externelIdToGroupMap.put(groupId, new ArrayList<LDAPUser>());
+					if(groupIds != null && groupIds.size() > 0) {
+						for(String groupId:groupIds) {
+							if(!cnToGroupMap.containsKey(groupId)) {
+								cnToGroupMap.put(groupId, new LDAPGroup(groupId));
+							}
+							cnToGroupMap.get(groupId).getParticipants().add(ldapUser);
+						}
+					}
+					
+					if(coachedGroupIds != null && coachedGroupIds.size() > 0) {
+						for(String coachedGroupId:coachedGroupIds) {
+							if(!cnToGroupMap.containsKey(coachedGroupId)) {
+								cnToGroupMap.put(coachedGroupId, new LDAPGroup(coachedGroupId));
+							}
+							cnToGroupMap.get(coachedGroupId).getCoaches().add(ldapUser);
 						}
-						externelIdToGroupMap.get(groupId).add(ldapUser);
 					}
 				}
 			}
 		}
-		
-		for(Map.Entry<String, List<LDAPUser>> groupEntry:externelIdToGroupMap.entrySet()) {
-			String externalId = groupEntry.getKey();
-			List<LDAPUser> members = groupEntry.getValue();
-			BusinessGroup managedGroup = getManagerBusinessGroup(externalId);
-			if(managedGroup != null) {
-				syncBusinessGroup(managedGroup, members);
-			}
-			dbInstance.commitAndCloseSession();
-		}
 	}
 	
-	private void syncBusinessGroup(BusinessGroup businessGroup, List<LDAPUser> members) {
+	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());
+
+		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);
+			}
+		}
+		// transfer to ldap user flagged as coach to the coach list
+		for(Iterator<LDAPUser> participantIt=participants.iterator(); participantIt.hasNext(); ) {
+			LDAPUser participant = participantIt.next();
+			if(participant.isCoach()) {
+				if(!coaches.contains(participant)) {
+					coaches.add(participant);
+				}
+				participantIt.remove();
+			}
+		}
+		
+		for(LDAPUser participant:participants) {
+			Identity memberIdentity = participant.getCachedIdentity();
+			syncMembership(businessGroup, memberIdentity, false);
+			currentMembers.remove(memberIdentity);
+		}
 		
-		for(LDAPUser member:members) {
-			Identity memberIdentity = member.getCachedIdentity();
-			syncMembership(businessGroup, memberIdentity, member.isCoach());
+		for(LDAPUser coach:coaches) {
+			Identity memberIdentity = coach.getCachedIdentity();
+			syncMembership(businessGroup, memberIdentity, true);
 			currentMembers.remove(memberIdentity);
 		}
 		
@@ -1104,21 +1149,6 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, GenericEventListe
 		}
 	}
 	
-	private void doSyncLDAPGroups(LdapContext ctx, Map<String,LDAPUser> dnToIdentityKeyMap, LDAPError errors) {
-		List<String> groupDNs = syncConfiguration.getLdapGroupBases();
-		List<LDAPGroup> ldapGroups = ldapDao.searchGroups(ctx, groupDNs);
-
-		log.info("LDAP batch sync " + ldapGroups.size() + " LDAP bases to OO groups");
-		for(LDAPGroup ldapGroup:ldapGroups) {
-			String externalId = ldapGroup.getCommonName();
-			BusinessGroup managedGroup = getManagerBusinessGroup(externalId);
-			if(managedGroup != null) {
-				syncBusinessGroup(ctx, ldapGroup, managedGroup, dnToIdentityKeyMap, errors);
-			}
-			dbInstance.commitAndCloseSession();
-		}
-	}
-	
 	private BusinessGroup getManagerBusinessGroup(String externalId) {
 		SearchBusinessGroupParams params = new SearchBusinessGroupParams();
 		params.setExternalId(externalId);
@@ -1139,35 +1169,6 @@ public class LDAPLoginManagerImpl implements LDAPLoginManager, GenericEventListe
 		return managedBusinessGroup;
 	}
 	
-	private void syncBusinessGroup(LdapContext ctx, LDAPGroup ldapGroup, BusinessGroup businessGroup, Map<String,LDAPUser> dnToIdentityKeyMap, LDAPError errors) {
-		List<String> members = ldapGroup.getMembers();
-		List<Identity> currentMembers = businessGroupRelationDao
-				.getMembers(businessGroup, GroupRoles.coach.name(), GroupRoles.participant.name());
-
-		int count = 0;
-		for(String member:members) {
-			LDAPUser ldapUser = getLDAPUser(ctx, member, dnToIdentityKeyMap, errors);
-			if(ldapUser != null) {
-				Identity identity = ldapUser.getCachedIdentity();
-				syncMembership(businessGroup, identity, ldapUser.isCoach());
-				currentMembers.remove(identity);
-				if(++count % 20 == 0) {
-					dbInstance.commitAndCloseSession();
-				}
-			}
-		}
-		
-		for(Identity currentMember:currentMembers) {
-			List<String> roles = businessGroupRelationDao.getRoles(currentMember, businessGroup);
-			for(String role:roles) {
-				businessGroupRelationDao.removeRole(currentMember, businessGroup, role);
-				if(++count % 20 == 0) {
-					dbInstance.commitAndCloseSession();
-				}
-			}
-		}
-	}
-	
 	private LDAPUser getLDAPUser(LdapContext ctx, String member, Map<String,LDAPUser> dnToIdentityKeyMap, LDAPError errors) {
 		LDAPUser ldapUser = dnToIdentityKeyMap.get(member);
 
diff --git a/src/main/java/org/olat/ldap/manager/LDAPUserVisitor.java b/src/main/java/org/olat/ldap/manager/LDAPUserVisitor.java
index c21cc0112df..7d6a5618e03 100644
--- a/src/main/java/org/olat/ldap/manager/LDAPUserVisitor.java
+++ b/src/main/java/org/olat/ldap/manager/LDAPUserVisitor.java
@@ -70,14 +70,23 @@ public class LDAPUserVisitor implements LDAPVisitor {
 		ldapUser.setGroupManager(hasAttributeValue(resAttribs, syncConfiguration.getGroupManagerRoleAttribute(), syncConfiguration.getGroupManagerRoleValue()));
 		ldapUser.setQpoolManager(hasAttributeValue(resAttribs, syncConfiguration.getQpoolManagerRoleAttribute(), syncConfiguration.getQpoolManagerRoleValue()));
 		ldapUser.setLearningResourceManager(hasAttributeValue(resAttribs, syncConfiguration.getLearningResourceManagerRoleAttribute(), syncConfiguration.getLearningResourceManagerRoleValue()));
+
+		List<String> groupList = parseGroupList(resAttribs, syncConfiguration.getGroupAttribute(), syncConfiguration.getGroupAttributeSeparator());
+		ldapUser.setGroupIds(groupList);
+		List<String> coachedGroupList = parseGroupList(resAttribs, syncConfiguration.getCoachedGroupAttribute(), syncConfiguration.getCoachedGroupAttributeSeparator());
+		ldapUser.setCoachedGroupIds(coachedGroupList);
 		
+		ldapUserList.add(ldapUser);
+	}
+	
+	private List<String> parseGroupList(Attributes resAttribs, String attributeName, String attributeSeparator) {
 		List<String> groupList = Collections.emptyList();
-		if(StringHelper.containsNonWhitespace(syncConfiguration.getGroupAttribute())) {
+		if(StringHelper.containsNonWhitespace(attributeName)) {
 			try {
-				Attribute groupAttr = resAttribs.get(syncConfiguration.getGroupAttribute());
+				Attribute groupAttr = resAttribs.get(attributeName);
 				if(groupAttr != null && groupAttr.get() instanceof String) {
 					String groupString = (String)groupAttr.get();
-					String[] groupArr = groupString.split(syncConfiguration.getGroupAttributeSeparator());
+					String[] groupArr = groupString.split(attributeSeparator);
 					groupList = new ArrayList<>(groupArr.length);
 					for(String group:groupArr) {
 						groupList.add(group);
@@ -87,8 +96,7 @@ public class LDAPUserVisitor implements LDAPVisitor {
 				log.error("");
 			}
 		}
-		ldapUser.setGroupIds(groupList);
-		ldapUserList.add(ldapUser);
+		return groupList;
 	}
 	
 	private boolean hasAttributeValue(Attributes resAttribs, String attribute, String value) {
diff --git a/src/main/java/org/olat/ldap/model/LDAPGroup.java b/src/main/java/org/olat/ldap/model/LDAPGroup.java
index 038b58ccdd7..266e7297802 100644
--- a/src/main/java/org/olat/ldap/model/LDAPGroup.java
+++ b/src/main/java/org/olat/ldap/model/LDAPGroup.java
@@ -32,6 +32,8 @@ public class LDAPGroup {
 	
 	private String commonName;
 	private List<String> members;
+	private List<LDAPUser> participants = new ArrayList<>();
+	private List<LDAPUser> coaches = new ArrayList<>();
 	
 	public LDAPGroup() {
 		//
@@ -58,6 +60,15 @@ public class LDAPGroup {
 		this.members = members;
 	}
 	
+	
+	public List<LDAPUser> getParticipants() {
+		return participants;
+	}
+
+	public List<LDAPUser> getCoaches() {
+		return coaches;
+	}
+
 	@Override
 	public int hashCode() {
 		return commonName == null ? 987234895 : commonName.hashCode();
diff --git a/src/main/java/org/olat/ldap/model/LDAPUser.java b/src/main/java/org/olat/ldap/model/LDAPUser.java
index a227f5d6b16..ff248360770 100644
--- a/src/main/java/org/olat/ldap/model/LDAPUser.java
+++ b/src/main/java/org/olat/ldap/model/LDAPUser.java
@@ -41,6 +41,7 @@ public class LDAPUser {
 	private boolean qpoolManager;
 	private boolean learningResourceManager;
 	private List<String> groupIds;
+	private List<String> coachedGroupIds;
 	private Attributes attributes;
 	private Identity cachedIdentity;
 	
@@ -116,6 +117,14 @@ public class LDAPUser {
 		this.groupIds = groupIds;
 	}
 
+	public List<String> getCoachedGroupIds() {
+		return coachedGroupIds;
+	}
+
+	public void setCoachedGroupIds(List<String> coachedGroupIds) {
+		this.coachedGroupIds = coachedGroupIds;
+	}
+
 	public Identity getCachedIdentity() {
 		return cachedIdentity;
 	}
diff --git a/src/main/resources/serviceconfig/olat.properties b/src/main/resources/serviceconfig/olat.properties
index f9530538124..3021fcd37d4 100644
--- a/src/main/resources/serviceconfig/olat.properties
+++ b/src/main/resources/serviceconfig/olat.properties
@@ -948,6 +948,9 @@ ldap.coachRoleValue=coach
 ldap.user.groupAttribute=
 ldap.user.groupAttribute.values=o
 ldap.user.groupAttributeSeparator=,
+ldap.user.coachedGroupAttribute=
+ldap.user.coachedGroupAttribute.values=o
+ldap.user.coachedGroupAttributeSeparator=,
 
 # sync authors
 ldap.authorsGroupBases=
-- 
GitLab