From 13172b0efae0329ba10f21d50ba53ee296fdf84c Mon Sep 17 00:00:00 2001
From: srosse <none@none>
Date: Sat, 4 Feb 2017 11:42:14 +0100
Subject: [PATCH] OO-2493: optimize the import workflow to reduce the number of
DB calls, limit the size of the table
---
.../admin/user/UserSearchFlexiController.java | 5 +-
.../org/olat/basesecurity/BaseSecurity.java | 8 +-
.../basesecurity/BaseSecurityManager.java | 8 +-
.../ImportMemberBySearchController.java | 16 +---
...ortMemberOverviewIdentitiesController.java | 88 +++++++++++++------
.../olat/repository/RepositoryManager.java | 10 ++-
.../manager/RepositoryEntryRelationDAO.java | 9 ++
.../basesecurity/BaseSecurityManagerTest.java | 20 +++++
8 files changed, 118 insertions(+), 46 deletions(-)
diff --git a/src/main/java/org/olat/admin/user/UserSearchFlexiController.java b/src/main/java/org/olat/admin/user/UserSearchFlexiController.java
index 50f7345d340..072fbabeacc 100644
--- a/src/main/java/org/olat/admin/user/UserSearchFlexiController.java
+++ b/src/main/java/org/olat/admin/user/UserSearchFlexiController.java
@@ -219,7 +219,7 @@ public class UserSearchFlexiController extends FlexiAutoCompleterController {
Translator myTrans = userManager.getPropertyHandlerTranslator(getTranslator());
userTableModel = new UserSearchFlexiTableModel(Collections.<Identity>emptyList(), resultingPropertyHandlers, isAdministrativeUser, getLocale(), tableColumnModel);
- tableEl = uifactory.addTableElement(getWindowControl(), "users", userTableModel, myTrans, formLayout);
+ tableEl = uifactory.addTableElement(getWindowControl(), "users", userTableModel, 250, false, myTrans, formLayout);
tableEl.setCustomizeColumns(false);
tableEl.setMultiSelect(true);
tableEl.setSelectAllEnable(true);
@@ -422,7 +422,8 @@ public class UserSearchFlexiController extends FlexiAutoCompleterController {
}
tableEl.reset();
- List<Identity> users = searchUsers(login, userPropertiesSearch, true);
+
+ List<Identity> users = searchUsers(login, userPropertiesSearch, true);
if (!users.isEmpty()) {
userTableModel.setObjects(users);
flc.contextPut("showButton","true");
diff --git a/src/main/java/org/olat/basesecurity/BaseSecurity.java b/src/main/java/org/olat/basesecurity/BaseSecurity.java
index 62c526e1b61..c2445becd78 100644
--- a/src/main/java/org/olat/basesecurity/BaseSecurity.java
+++ b/src/main/java/org/olat/basesecurity/BaseSecurity.java
@@ -173,7 +173,13 @@ public interface BaseSecurity {
*/
public Identity findIdentityByNumber(String identityNumber);
-
+ /**
+ * The list of visible identities with a institutional number like in the
+ * specified list. Deleted ones are not included.
+ *
+ * @param identityNumbers
+ * @return A list of identities
+ */
public List<Identity> findIdentitiesByNumber(Collection<String> identityNumbers);
/**
diff --git a/src/main/java/org/olat/basesecurity/BaseSecurityManager.java b/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
index 2b23870a293..d71ffa54508 100644
--- a/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
+++ b/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
@@ -991,13 +991,15 @@ public class BaseSecurityManager implements BaseSecurity {
if(identityNumbers == null || identityNumbers.isEmpty()) return Collections.emptyList();
StringBuilder sb = new StringBuilder();
- sb.append("select identity from ").append(IdentityImpl.class.getName()).append(" identity ")
- .append(" inner join fetch identity.user user ")
- .append(" where user.").append(UserConstants.INSTITUTIONALUSERIDENTIFIER).append(" in (:idNumbers) ");
+ sb.append("select ident from ").append(IdentityImpl.class.getName()).append(" ident ")
+ .append(" inner join fetch ident.user user ")
+ .append(" where user.").append(UserConstants.INSTITUTIONALUSERIDENTIFIER).append(" in (:idNumbers) ")
+ .append(" and ident.status<:status");
return dbInstance.getCurrentEntityManager()
.createQuery(sb.toString(), Identity.class)
.setParameter("idNumbers", identityNumbers)
+ .setParameter("status", Identity.STATUS_VISIBLE_LIMIT)
.getResultList();
}
diff --git a/src/main/java/org/olat/course/member/wizard/ImportMemberBySearchController.java b/src/main/java/org/olat/course/member/wizard/ImportMemberBySearchController.java
index d65c4cf6345..d4b86ac953b 100644
--- a/src/main/java/org/olat/course/member/wizard/ImportMemberBySearchController.java
+++ b/src/main/java/org/olat/course/member/wizard/ImportMemberBySearchController.java
@@ -19,7 +19,6 @@
*/
package org.olat.course.member.wizard;
-import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -58,16 +57,11 @@ public class ImportMemberBySearchController extends StepFormBasicController {
protected void event(UserRequest ureq, Controller source, Event event) {
if(event instanceof SingleIdentityChosenEvent) {
SingleIdentityChosenEvent e = (SingleIdentityChosenEvent)event;
- String key = e.getChosenIdentity().getKey().toString();
- addToRunContext("keys", Collections.singletonList(key));
+ addToRunContext("keyIdentities", Collections.singletonList(e.getChosenIdentity()));
fireEvent(ureq, StepsEvent.ACTIVATE_NEXT);
} else if(event instanceof MultiIdentityChosenEvent) {
MultiIdentityChosenEvent e = (MultiIdentityChosenEvent)event;
- List<String> keys = new ArrayList<>();
- for(Identity identity: e.getChosenIdentities()) {
- keys.add(identity.getKey().toString());
- }
- addToRunContext("keys", keys);
+ addToRunContext("keyIdentities", e.getChosenIdentities());
fireEvent(ureq, StepsEvent.ACTIVATE_NEXT);
} else {
super.event(ureq, source, event);
@@ -80,11 +74,7 @@ public class ImportMemberBySearchController extends StepFormBasicController {
if(identities.isEmpty()) {
searchController.doSearch();
} else {
- List<String> keys = new ArrayList<>(identities.size());
- for(Identity identity: identities) {
- keys.add(identity.getKey().toString());
- }
- addToRunContext("keys", keys);
+ addToRunContext("keyIdentities", identities);
fireEvent(ureq, StepsEvent.ACTIVATE_NEXT);
}
}
diff --git a/src/main/java/org/olat/course/member/wizard/ImportMemberOverviewIdentitiesController.java b/src/main/java/org/olat/course/member/wizard/ImportMemberOverviewIdentitiesController.java
index 2249ce0eff8..90d8973787b 100644
--- a/src/main/java/org/olat/course/member/wizard/ImportMemberOverviewIdentitiesController.java
+++ b/src/main/java/org/olat/course/member/wizard/ImportMemberOverviewIdentitiesController.java
@@ -20,7 +20,9 @@
package org.olat.course.member.wizard;
import java.util.ArrayList;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import org.olat.admin.user.UserTableDataModel;
import org.olat.basesecurity.BaseSecurity;
@@ -28,7 +30,6 @@ import org.olat.basesecurity.BaseSecurityModule;
import org.olat.basesecurity.Constants;
import org.olat.basesecurity.SecurityGroup;
import org.olat.core.CoreSpringFactory;
-import org.olat.core.commons.persistence.PersistenceHelper;
import org.olat.core.gui.UserRequest;
import org.olat.core.gui.components.form.flexible.FormItemContainer;
import org.olat.core.gui.components.form.flexible.elements.FlexiTableElement;
@@ -47,6 +48,7 @@ import org.olat.core.id.Identity;
import org.olat.core.id.UserConstants;
import org.olat.user.UserManager;
import org.olat.user.propertyhandlers.UserPropertyHandler;
+import org.springframework.beans.factory.annotation.Autowired;
/**
*
@@ -60,9 +62,12 @@ public class ImportMemberOverviewIdentitiesController extends StepFormBasicContr
private List<String> notfounds;
private boolean isAdministrativeUser;
- private final UserManager userManager;
- private final BaseSecurity securityManager;
- private final BaseSecurityModule securityModule;
+ @Autowired
+ private UserManager userManager;
+ @Autowired
+ private BaseSecurity securityManager;
+ @Autowired
+ private BaseSecurityModule securityModule;
public ImportMemberOverviewIdentitiesController(UserRequest ureq, WindowControl wControl, Form rootForm, StepsRunContext runContext) {
super(ureq, wControl, rootForm, runContext, LAYOUT_VERTICAL, null);
@@ -78,6 +83,10 @@ public class ImportMemberOverviewIdentitiesController extends StepFormBasicContr
@SuppressWarnings("unchecked")
List<String> keys = (List<String>)runContext.get("keys");
loadModel(keys);
+ } else if(containsRunContextKey("keyIdentities")) {
+ @SuppressWarnings("unchecked")
+ List<Identity> keys = (List<Identity>)runContext.get("keyIdentities");
+ loadModelByIdentities(keys);
}
isAdministrativeUser = securityModule.isUserAllowedAdminProps(ureq.getUserSession().getRoles());
@@ -129,28 +138,50 @@ public class ImportMemberOverviewIdentitiesController extends StepFormBasicContr
}
private void loadModel(List<String> keys) {
- oks = new ArrayList<Identity>();
- List<String> isanonymous = new ArrayList<String>();
- notfounds = new ArrayList<String>();
-
+ notfounds = new ArrayList<>();
+
+ Set<Identity> okSet = new HashSet<>();
SecurityGroup anonymousSecGroup = securityManager.findSecurityGroupByName(Constants.GROUP_ANONYMOUS);
+ List<Identity> anonymousUsers = securityManager.getIdentitiesOfSecurityGroup(anonymousSecGroup);
+
for (String identityKey : keys) {
Identity ident = securityManager.loadIdentityByKey(Long.parseLong(identityKey));
if (ident == null) { // not found, add to not-found-list
notfounds.add(identityKey);
- } else if (securityManager.isIdentityInSecurityGroup(ident, anonymousSecGroup)) {
- isanonymous.add(identityKey);
- } else if (!PersistenceHelper.containsPersistable(oks, ident)) {
- oks.add(ident);
+ } else if (anonymousUsers.contains(ident)) {
+ //ignore
+ } else if (!okSet.contains(ident)) {
+ okSet.add(ident);
}
}
+ oks = new ArrayList<>(okSet);
+ }
+
+ private void loadModelByIdentities(List<Identity> keys) {
+ notfounds = new ArrayList<>();
+
+ Set<Identity> okSet = new HashSet<>();
+ SecurityGroup anonymousSecGroup = securityManager.findSecurityGroupByName(Constants.GROUP_ANONYMOUS);
+ List<Identity> anonymousUsers = securityManager.getIdentitiesOfSecurityGroup(anonymousSecGroup);
+
+ for (Identity ident : keys) {
+ if (ident == null || anonymousUsers.contains(ident)) {
+ //ignore
+ } else if (!okSet.contains(ident)) {
+ okSet.add(ident);
+ }
+ }
+ oks = new ArrayList<>(okSet);
}
private void loadModel(String inp) {
- oks = new ArrayList<Identity>();
- notfounds = new ArrayList<String>();
+ oks = new ArrayList<>();
+ notfounds = new ArrayList<>();
+
+ Set<Identity> okSet = new HashSet<>();
- SecurityGroup anonymousSecGroup = securityManager.findSecurityGroupByName(Constants.GROUP_ANONYMOUS);
+ SecurityGroup anonymousGroup = securityManager.findSecurityGroupByName(Constants.GROUP_ANONYMOUS);
+ Set<Identity> anonymousUsers = new HashSet<>(securityManager.getIdentitiesOfSecurityGroup(anonymousGroup));
List<String> identList = new ArrayList<String>();
String[] lines = inp.split("\r?\n");
@@ -168,13 +199,12 @@ public class ImportMemberOverviewIdentitiesController extends StepFormBasicContr
if(userIdent != null) {
identList.remove(userIdent);
}
- if (!PersistenceHelper.containsPersistable(oks, identity)
- && !securityManager.isIdentityInSecurityGroup(identity, anonymousSecGroup)) {
- oks.add(identity);
+ if (!okSet.contains(identity) && !anonymousUsers.contains(identity)) {
+ okSet.add(identity);
}
}
// make a lowercase copy of identList for processing username and email
- List<String> identListLowercase = new ArrayList<String>(identList.size());
+ List<String> identListLowercase = new ArrayList<>(identList.size());
for (String ident:identList) {
identListLowercase.add(ident.toLowerCase());
}
@@ -182,14 +212,20 @@ public class ImportMemberOverviewIdentitiesController extends StepFormBasicContr
List<Identity> identities = securityManager.findIdentitiesByNameCaseInsensitive(identListLowercase);
for(Identity identity:identities) {
identListLowercase.remove(identity.getName().toLowerCase());
- if (!PersistenceHelper.containsPersistable(oks, identity)
- && !securityManager.isIdentityInSecurityGroup(identity, anonymousSecGroup)) {
- oks.add(identity);
+ if (!okSet.contains(identity) && !anonymousUsers.contains(identity)) {
+ okSet.add(identity);
}
}
//search by email, case insensitive
- List<Identity> mailIdentities = userManager.findIdentitiesByEmail(identListLowercase);
+ List<String> emailListLowercase = new ArrayList<>(identList.size());
+ for (String ident:identList) {
+ if(ident.indexOf('@') > 0) {
+ emailListLowercase.add(ident.toLowerCase());
+ }
+ }
+
+ List<Identity> mailIdentities = userManager.findIdentitiesByEmail(emailListLowercase);
for(Identity identity:mailIdentities) {
String email = identity.getUser().getProperty(UserConstants.EMAIL, null);
if(email != null) {
@@ -199,13 +235,13 @@ public class ImportMemberOverviewIdentitiesController extends StepFormBasicContr
if(institutEmail != null) {
identListLowercase.remove(institutEmail.toLowerCase());
}
- if (!PersistenceHelper.containsPersistable(oks, identity)
- && !securityManager.isIdentityInSecurityGroup(identity, anonymousSecGroup)) {
- oks.add(identity);
+ if (!okSet.contains(identity) && !anonymousUsers.contains(identity)) {
+ okSet.add(identity);
}
}
notfounds.addAll(identListLowercase);
+ oks = new ArrayList<>(okSet);
}
public boolean validate() {
diff --git a/src/main/java/org/olat/repository/RepositoryManager.java b/src/main/java/org/olat/repository/RepositoryManager.java
index 24884835631..fe75173fcd7 100644
--- a/src/main/java/org/olat/repository/RepositoryManager.java
+++ b/src/main/java/org/olat/repository/RepositoryManager.java
@@ -1811,9 +1811,13 @@ public class RepositoryManager {
boolean allOk = repositoryEntryRelationDao.removeMembers(re, members);
if (allOk) {
+ int count = 0;
List<RepositoryEntryMembershipModifiedEvent> deferredEvents = new ArrayList<>();
for(Identity identity:members) {
deferredEvents.add(RepositoryEntryMembershipModifiedEvent.removed(identity, re));
+ if(++count % 100 == 0) {
+ dbInstance.commitAndCloseSession();
+ }
}
dbInstance.commit();
sendDeferredEvents(deferredEvents, re);
@@ -2268,12 +2272,16 @@ public class RepositoryManager {
public void updateRepositoryEntryMemberships(Identity ureqIdentity, Roles ureqRoles, RepositoryEntry re,
List<RepositoryEntryPermissionChangeEvent> changes, MailPackage mailing) {
+ int count = 0;
List<RepositoryEntryMembershipModifiedEvent> deferredEvents = new ArrayList<>();
for(RepositoryEntryPermissionChangeEvent e:changes) {
updateRepositoryEntryMembership(ureqIdentity, ureqRoles, re, e, mailing, deferredEvents);
+ if(++count % 100 == 0) {
+ dbInstance.commitAndCloseSession();
+ }
}
- dbInstance.commit();
+ dbInstance.commitAndCloseSession();
sendDeferredEvents(deferredEvents, re);
}
diff --git a/src/main/java/org/olat/repository/manager/RepositoryEntryRelationDAO.java b/src/main/java/org/olat/repository/manager/RepositoryEntryRelationDAO.java
index 459a6519ba8..6d7bebaca35 100644
--- a/src/main/java/org/olat/repository/manager/RepositoryEntryRelationDAO.java
+++ b/src/main/java/org/olat/repository/manager/RepositoryEntryRelationDAO.java
@@ -181,6 +181,14 @@ public class RepositoryEntryRelationDAO {
return groupDao.removeMemberships(group, role);
}
+ /**
+ * Retrieve the default group of the repository entry (the one
+ * marked with the flag defaultGroup=true). The query is cached
+ * by hibernate 2nd level cache.
+ *
+ * @param re The repository entry
+ * @return The group
+ */
public Group getDefaultGroup(RepositoryEntryRef re) {
StringBuilder sb = new StringBuilder();
sb.append("select baseGroup from ").append(RepositoryEntry.class.getName()).append(" as v ")
@@ -190,6 +198,7 @@ public class RepositoryEntryRelationDAO {
return dbInstance.getCurrentEntityManager().createQuery(sb.toString(), Group.class)
.setParameter("repoKey", re.getKey())
+ .setHint("org.hibernate.cacheable", Boolean.TRUE)
.getSingleResult();
}
diff --git a/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java b/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java
index 91e6fd1d826..e68fb702172 100644
--- a/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java
+++ b/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java
@@ -247,6 +247,26 @@ public class BaseSecurityManagerTest extends OlatTestCase {
Assert.assertTrue(foundIds.contains(id2));
}
+ @Test
+ public void findIdentitiesByNumber() {
+ //create a user it
+ String username = "fINd-ME-6-" + UUID.randomUUID();
+ String institutionalNumber = UUID.randomUUID().toString();
+ Identity id = JunitTestHelper.createAndPersistIdentityAsUser(username);
+ id.getUser().setProperty(UserConstants.INSTITUTIONALUSERIDENTIFIER, institutionalNumber);
+ userManager.updateUserFromIdentity(id);
+ dbInstance.commitAndCloseSession();
+
+ List<String> numbers = new ArrayList<String>(2);
+ numbers.add(institutionalNumber);
+
+ //find it
+ List<Identity> foundIds = securityManager.findIdentitiesByNumber(numbers);
+ Assert.assertNotNull(foundIds);
+ Assert.assertEquals(1, foundIds.size());
+ Assert.assertTrue(foundIds.contains(id));
+ }
+
@Test
public void loadIdentityShortByKey() {
//create a user it
--
GitLab