From 5a5baf44e6b07eb55dd1e9d8510109518b77cd75 Mon Sep 17 00:00:00 2001
From: srosse <none@none>
Date: Thu, 15 Jan 2015 09:48:05 +0100
Subject: [PATCH] OO-1391: fix query for count, add some count query in unit
tests
---
.../basesecurity/BaseSecurityManager.java | 22 +++++++-----
.../GetIdentitiesByPowerSearchTest.java | 36 +++++++++++--------
2 files changed, 34 insertions(+), 24 deletions(-)
diff --git a/src/main/java/org/olat/basesecurity/BaseSecurityManager.java b/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
index c72653c6f3f..bc0277b17c0 100644
--- a/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
+++ b/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
@@ -1370,7 +1370,7 @@ public class BaseSecurityManager extends BasicManager implements BaseSecurity {
boolean hasAuthProviders = (params.getAuthProviders() != null && params.getAuthProviders().length > 0);
// select identity and inner join with user to optimize query
- StringBuilder sb = new StringBuilder();
+ StringBuilder sb = new StringBuilder(5000);
if (hasAuthProviders) {
// I know, it looks wrong but I need to do the join reversed since it is not possible to
// do this query with a left join that starts with the identity using hibernate HQL. A left
@@ -1378,26 +1378,30 @@ public class BaseSecurityManager extends BasicManager implements BaseSecurity {
// providers (e.g. when searching for users that do not have any authentication providers at all!).
// It took my quite a while to make this work, so think twice before you change anything here!
if(count) {
- sb = new StringBuilder("select count(distinct ident.key) from org.olat.basesecurity.AuthenticationImpl as auth right join auth.identity as ident ");
+ sb.append("select count(distinct ident.key) from org.olat.basesecurity.AuthenticationImpl as auth ")
+ .append(" right join auth.identity as ident")
+ .append(" inner join ident.user as user ");
} else {
- sb = new StringBuilder("select distinct ident from org.olat.basesecurity.AuthenticationImpl as auth right join auth.identity as ident ");
+ sb.append("select distinct ident from org.olat.basesecurity.AuthenticationImpl as auth ")
+ .append(" right join auth.identity as ident")
+ .append(" inner join fetch ident.user as user ");
}
} else {
if(count) {
- sb = new StringBuilder("select count(distinct ident.key) from org.olat.core.id.Identity as ident ");
+ sb.append("select count(distinct ident.key) from org.olat.core.id.Identity as ident ")
+ .append(" inner join ident.user as user ");
} else {
- sb = new StringBuilder("select distinct ident from org.olat.core.id.Identity as ident ");
+ sb.append("select distinct ident from org.olat.core.id.Identity as ident ")
+ .append(" inner join fetch ident.user as user ");
}
}
// In any case join with the user. Don't join-fetch user, this breaks the query
// because of the user fields (don't know exactly why this behaves like
// this)
- sb.append(" inner join fetch ident.user as user ");
-
+
if (hasGroups) {
// join over security group memberships
- sb.append(" ,org.olat.basesecurity.SecurityGroupMembershipImpl as sgmsi ");
-
+ sb.append(" ,org.olat.basesecurity.SecurityGroupMembershipImpl as sgmsi ");
}
if (hasPermissionOnResources) {
// join over policies
diff --git a/src/test/java/org/olat/basesecurity/GetIdentitiesByPowerSearchTest.java b/src/test/java/org/olat/basesecurity/GetIdentitiesByPowerSearchTest.java
index 3049c953a32..94c267242f6 100644
--- a/src/test/java/org/olat/basesecurity/GetIdentitiesByPowerSearchTest.java
+++ b/src/test/java/org/olat/basesecurity/GetIdentitiesByPowerSearchTest.java
@@ -40,7 +40,6 @@ import junit.framework.Assert;
import org.junit.Test;
import org.olat.core.commons.persistence.DB;
-import org.olat.core.commons.persistence.DBFactory;
import org.olat.core.id.Identity;
import org.olat.core.id.Roles;
import org.olat.core.id.User;
@@ -94,7 +93,7 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
assertEquals("Number of all users != activeUsers + deletedUsers" , numberOfAllUsers, numberOfActiveUsers + numberOfDeletedUsers);
// user attributes search test
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
results = baseSecurityManager.getIdentitiesByPowerSearch(ident.getName(), null, true, null, null, null, null, null, null, null, null);
assertTrue(results.size() == 1);
assertEquals("Wrong search result (search with username)" + ident.getName() + "' ",ident.getName() , results.get(0).getName());
@@ -290,14 +289,14 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
baseSecurityManager.addIdentityToSecurityGroup(ident, authors);
// policy search test
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
List<Policy> policies = baseSecurityManager.getPoliciesOfSecurityGroup(admins);
PermissionOnResourceable[] adminPermissions = convertPoliciesListToPermissionOnResourceArray(policies);
policies = baseSecurityManager.getPoliciesOfSecurityGroup(anonymous);
PermissionOnResourceable[] anonymousPermissions = convertPoliciesListToPermissionOnResourceArray(policies);
// security group search test
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
SecurityGroup[] groups2 = {admins, authors};
SecurityGroup[] groups3 = {authors};
@@ -363,7 +362,7 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
SecurityGroup[] groups2 = { admins, authors };
// policy search test
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
List<Policy> policies = baseSecurityManager.getPoliciesOfSecurityGroup(admins);
PermissionOnResourceable[] adminPermissions = convertPoliciesListToPermissionOnResourceArray(policies);
@@ -400,7 +399,7 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
Authentication auth = baseSecurityManager.findAuthentication(ident, BaseSecurityModule.getDefaultAuthProviderIdentifier());
baseSecurityManager.deleteAuthentication(auth);
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
// ultimate tests
//Identity ident = getOrCreateIdentity("anIdentity");
@@ -410,7 +409,7 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
cal.add(Calendar.DAY_OF_MONTH, -5);
Date before = cal.getTime();
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
results = baseSecurityManager.getIdentitiesByPowerSearch("groovy", null, true, groups1, adminPermissions, null, before, null, null, null, null);
Assert.assertTrue(results.isEmpty());
results = baseSecurityManager.getVisibleIdentitiesByPowerSearch("groovy", null, true, groups1, adminPermissions, null, before, null);
@@ -471,11 +470,15 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
//check count before adding
List<Identity> results = baseSecurityManager.getIdentitiesByPowerSearch(null, null, true, null, null, authProviderNone, null, null, null, null, null);
int prevProviderNoneCount = results.size();
+
+ long countResults = baseSecurityManager.countIdentitiesByPowerSearch(null, null, true, null, null, authProviderNone, null, null, null, null, null);
+ Assert.assertEquals(results.size(), countResults);
+
//add two new users with authProviderNone
String rnd = UUID.randomUUID().toString();
Identity authNoneOne = getOrCreateTestIdentityWithAuth("authNoneOne-" + rnd, null);
Identity authNoneTwo = getOrCreateTestIdentityWithAuth("authNoneTwo-" + rnd, null);
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
// special case: no auth provider
// test if 2 new users are found.
@@ -490,7 +493,7 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
Identity authNoneThree = getOrCreateTestIdentityWithAuth("authNoneThree-" + rnd, null);
Identity authNoneFour = getOrCreateTestIdentityWithAuth("authNoneFour-" + rnd, null);
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
results = baseSecurityManager.getVisibleIdentitiesByPowerSearch(null, null, true, null, null, authProviderNone, null, null);
Assert.assertTrue(results.contains(authNoneThree));
@@ -503,7 +506,7 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
getOrCreateTestIdentityWithAuth("authNoneFive-" + rnd, null);
getOrCreateTestIdentityWithAuth("authNoneSix-" + rnd, null);
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
results = baseSecurityManager.getIdentitiesByPowerSearch(null, null, true, null, null, authProviderNone, null, null, null, null, null);
Assert.assertEquals("Wrong number of identities, search with (authProviderNone)", prevProviderNoneCount + 2, results.size());
@@ -511,7 +514,7 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
prevProviderNoneCount = results.size();
getOrCreateTestIdentityWithAuth("authNoneSeven-" + rnd, null);
getOrCreateTestIdentityWithAuth("authNoneEight-" + rnd, null);
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
results = baseSecurityManager.getVisibleIdentitiesByPowerSearch(null, null, true, null, null, authProviderNone, null, null);
Assert.assertEquals("Wrong number of visible identities, search with (authProviderNone)", prevProviderNoneCount + 2, results.size());
@@ -522,7 +525,7 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
getOrCreateTestIdentityWithAuth("authTwelve-" + rnd, "Shib");
getOrCreateTestIdentityWithAuth("authThirteen-" + rnd, BaseSecurityModule.getDefaultAuthProviderIdentifier());
getOrCreateTestIdentityWithAuth("authForteen-" + rnd, null);
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
results = baseSecurityManager.getIdentitiesByPowerSearch(null, null, true, null, null, authProvidersAll, null, null, null, null, null);
Assert.assertTrue(results.size() - prevProviderNoneCount == 3);
@@ -532,7 +535,7 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
getOrCreateTestIdentityWithAuth("authSixteen-" + rnd, "Shib");
getOrCreateTestIdentityWithAuth("authSeventeen-" + rnd, BaseSecurityModule.getDefaultAuthProviderIdentifier());
getOrCreateTestIdentityWithAuth("authEighteen-" + rnd, null);
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
results = baseSecurityManager.getVisibleIdentitiesByPowerSearch(null, null, true, null, null, authProvidersAll, null, null);
Assert.assertTrue(results.size() - prevProviderNoneCount == 3);
}
@@ -560,7 +563,7 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
twoPropUser.setProperty(UserConstants.LASTNAME, "prop");
Identity twoPropIdentity = baseSecurityManager.createAndPersistIdentityAndUser(twoUsername, null, twoPropUser, BaseSecurityModule.getDefaultAuthProviderIdentifier(), twoUsername, "ppp");
Assert.assertNotNull(twoPropIdentity);
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
HashMap<String, String> userProperties = new HashMap<String, String>();
userProperties.put(UserConstants.FIRSTNAME, "one");
@@ -642,13 +645,16 @@ public class GetIdentitiesByPowerSearchTest extends OlatTestCase {
Assert.assertNotNull(identity);
// commit
- DBFactory.getInstance().closeSession();
+ dbInstance.commitAndCloseSession();
HashMap<String, String> userProperties = new HashMap<String, String>();
userProperties.put(UserConstants.FIRSTNAME, "multi");
List<Identity> results = baseSecurityManager.getIdentitiesByPowerSearch(null, userProperties, true, null, null, null, null, null, null, null, null);
sysoutResults(results);
Assert.assertTrue(results.contains(identity));
+
+ long countResults = baseSecurityManager.countIdentitiesByPowerSearch(null, userProperties, true, null, null, null, null, null, null, null, null);
+ Assert.assertEquals(results.size(), countResults);
userProperties = new HashMap<String, String>();
userProperties.put(UserConstants.FIRSTNAME, "multi");
--
GitLab