OO-502: load the course rights with 1 query and not 7

src/main/java/org/olat/basesecurity/BaseSecurity.java

@@ -61,6 +61,14 @@ public interface BaseSecurity {
 	 */
 	public boolean isIdentityPermittedOnResourceable(Identity identity, String permission, OLATResourceable olatResourceable);
 
+	/**
+	 * Return the list of "allowed to..."
+	 * @param identity
+	 * @param olatResourceable
+	 * @return
+	 */
+	public List<String> getIdentityPermissionOnresourceable(Identity identity, OLATResourceable olatResourceable);
+	
 	/**
 	 * Get the identity's roles
 	 * 

src/main/java/org/olat/basesecurity/BaseSecurityManager.java

@@ -303,6 +303,21 @@ public class BaseSecurityManager extends BasicManager implements BaseSecurity {
 		return query.getResultList();
 	}
 	
+	@Override
+	public List<String> getIdentityPermissionOnresourceable(Identity identity, OLATResourceable olatResourceable) {
+		Long oresid = olatResourceable.getResourceableId();
+		if (oresid == null) {
+			oresid = new Long(0);
+		}
+		List<String> permissions = dbInstance.getCurrentEntityManager()
+				.createNamedQuery("getIdentityPermissionsOnResourceableCheckType", String.class)
+			.setParameter("identitykey", identity.getKey())
+			.setParameter("resid", oresid)
+			.setParameter("resname", olatResourceable.getResourceableTypeName())
+			.getResultList();
+		return permissions;
+	}
+	
 	public boolean isIdentityPermittedOnResourceable(Identity identity, String permission, OLATResourceable olatResourceable) {
 		return isIdentityPermittedOnResourceable(identity, permission, olatResourceable, true);
 	}

src/main/java/org/olat/basesecurity/PolicyImpl.hbm.xml

@@ -43,6 +43,9 @@
 	<query name="isIdentityPermittedOnResourceable">
 		<![CDATA[select count(poi) from org.olat.basesecurity.SecurityGroupMembershipImpl as sgmsi, org.olat.basesecurity.PolicyImpl as poi, org.olat.resource.OLATResourceImpl as ori where sgmsi.identity.key = :identitykey and sgmsi.securityGroup =  poi.securityGroup and poi.permission = :permission and poi.olatResource = ori and (ori.resId = :resid) and ori.resName = :resname]]>
 	</query>
+	<query name="getIdentityPermissionsOnResourceableCheckType">
+		<![CDATA[select distinct poi.permission from org.olat.basesecurity.SecurityGroupMembershipImpl as sgmsi, org.olat.basesecurity.PolicyImpl as poi, org.olat.resource.OLATResourceImpl as ori where sgmsi.identity.key=:identitykey and sgmsi.securityGroup=poi.securityGroup and poi.olatResource=ori and (ori.resId=:resid or ori.resId=0) and ori.resName=:resname]]>
+	</query>
 	
 </hibernate-mapping>
 

src/main/java/org/olat/course/groupsandrights/CourseGroupManager.java

@@ -63,6 +63,14 @@ public interface CourseGroupManager {
 	 * @return true if user has course right, false otherwise
 	 */
 	public boolean hasRight(Identity identity, String courseRight);
+	
+	/**
+	 * Return the users course rights in any of the available right group context of
+	 * this course
+	 * @param identity
+	 * @return
+	 */
+	public List<String> getRights(Identity identity);
 
 	/**
 	 * Checks if an identity is in a learning group with the given name in any

src/main/java/org/olat/course/groupsandrights/PersistingCourseGroupManager.java

@@ -120,6 +120,10 @@ public class PersistingCourseGroupManager extends BasicManager implements Course
 		boolean hasRight = rightManager.hasBGRight(courseRight, identity, courseResource);
 		return hasRight;
 	}
+	
+	public List<String> getRights(Identity identity) {
+		return securityManager.getIdentityPermissionOnresourceable(identity, courseResource);
+	}
 
 	@Override
 	public boolean isIdentityInGroup(Identity identity, Long groupKey) {

src/main/java/org/olat/course/run/RunMainController.java

@@ -378,13 +378,14 @@ public class RunMainController extends MainLayoutBasicController implements Gene
 		// 3) all other rights are defined in the groupmanagement using the learning
 		// group rights
 		
-		courseRightsCache.put(CourseRights.RIGHT_GROUPMANAGEMENT, new Boolean(cgm.hasRight(identity, CourseRights.RIGHT_GROUPMANAGEMENT)));
-		courseRightsCache.put(CourseRights.RIGHT_COURSEEDITOR, new Boolean(cgm.hasRight(identity, CourseRights.RIGHT_COURSEEDITOR)));
-		courseRightsCache.put(CourseRights.RIGHT_ARCHIVING, new Boolean(cgm.hasRight(identity, CourseRights.RIGHT_ARCHIVING)));
-		courseRightsCache.put(CourseRights.RIGHT_ASSESSMENT, new Boolean(cgm.hasRight(identity, CourseRights.RIGHT_ASSESSMENT)));
-		courseRightsCache.put(CourseRights.RIGHT_GLOSSARY, new Boolean(cgm.hasRight(identity, CourseRights.RIGHT_GLOSSARY)));
-		courseRightsCache.put(CourseRights.RIGHT_STATISTICS, new Boolean(cgm.hasRight(identity, CourseRights.RIGHT_STATISTICS)));
-		courseRightsCache.put(CourseRights.RIGHT_DB, new Boolean(cgm.hasRight(identity, CourseRights.RIGHT_DB)));
+		List<String> rights = cgm.getRights(identity);
+		courseRightsCache.put(CourseRights.RIGHT_GROUPMANAGEMENT, new Boolean(rights.contains(CourseRights.RIGHT_GROUPMANAGEMENT)));
+		courseRightsCache.put(CourseRights.RIGHT_COURSEEDITOR, new Boolean(rights.contains(CourseRights.RIGHT_COURSEEDITOR)));
+		courseRightsCache.put(CourseRights.RIGHT_ARCHIVING, new Boolean(rights.contains(CourseRights.RIGHT_ARCHIVING)));
+		courseRightsCache.put(CourseRights.RIGHT_ASSESSMENT, new Boolean(rights.contains(CourseRights.RIGHT_ASSESSMENT)));
+		courseRightsCache.put(CourseRights.RIGHT_GLOSSARY, new Boolean(rights.contains(CourseRights.RIGHT_GLOSSARY)));
+		courseRightsCache.put(CourseRights.RIGHT_STATISTICS, new Boolean(rights.contains(CourseRights.RIGHT_STATISTICS)));
+		courseRightsCache.put(CourseRights.RIGHT_DB, new Boolean(rights.contains(CourseRights.RIGHT_DB)));
 	}
 
 	/**

src/main/java/org/olat/course/run/preview/PreviewCourseGroupManager.java

@@ -26,6 +26,7 @@
 package org.olat.course.run.preview;
 
 import java.io.File;
+import java.util.ArrayList;
 import java.util.List;
 
 import org.olat.core.id.Identity;
@@ -80,7 +81,12 @@ final class PreviewCourseGroupManager extends BasicManager implements CourseGrou
 		}
 		throw new AssertException("unsupported");
 	}
-	
+
+	@Override
+	public List<String> getRights(Identity identity) {
+		return new ArrayList<String>(1);
+	}
+
 	@Override
 	public boolean isIdentityInGroup(Identity identity, Long groupKey) {
 		for(BusinessGroup group:groups) {

src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java

@@ -760,4 +760,75 @@ public class BaseSecurityManagerTest extends OlatTestCase {
 		Assert.assertTrue(policies_3.contains(policy_3_3));
 		Assert.assertFalse(policies_3.contains(policy_1_3));
 	}
+	
+	@Test
+	public void isIdentityPermittedOnResourceable_checkType() {
+		//create an identity, a security group, a resource and give the identity some
+		//permissions on the resource
+		SecurityGroup secGroup = securityManager.createAndPersistSecurityGroup();
+		OLATResource resource = JunitTestHelper.createRandomResource();
+		Identity id = JunitTestHelper.createAndPersistIdentityAsUser("test-ipor-1-" + UUID.randomUUID().toString());
+		securityManager.addIdentityToSecurityGroup(id, secGroup);
+		securityManager.createAndPersistPolicy(secGroup, "test.ipor-1_1", resource);
+		securityManager.createAndPersistPolicy(secGroup, "test.ipor-1_2", resource);
+		dbInstance.commitAndCloseSession();
+		
+		//check
+		boolean hasIpor_1_1 = securityManager.isIdentityPermittedOnResourceable(id, "test.ipor-1_1", resource);
+		Assert.assertTrue(hasIpor_1_1);
+		boolean hasIpor_1_2 = securityManager.isIdentityPermittedOnResourceable(id, "test.ipor-1_2", resource);
+		Assert.assertTrue(hasIpor_1_2);
+		boolean hasIpor_1_3 = securityManager.isIdentityPermittedOnResourceable(id, "test.ipor-1_3", resource);
+		Assert.assertFalse(hasIpor_1_3);
+		
+		//check type
+		boolean hasIpor_1_1_ct = securityManager.isIdentityPermittedOnResourceable(id, "test.ipor-1_1", resource, true);
+		Assert.assertTrue(hasIpor_1_1_ct);
+		boolean hasIpor_1_2_ct = securityManager.isIdentityPermittedOnResourceable(id, "test.ipor-1_2", resource, true);
+		Assert.assertTrue(hasIpor_1_2_ct);
+		boolean hasIpor_1_3_ct = securityManager.isIdentityPermittedOnResourceable(id, "test.ipor-1_3", resource, true);
+		Assert.assertFalse(hasIpor_1_3_ct);
+	}
+	
+	@Test
+	public void isIdentityPermittedOnResourceable_noCheckType() {
+		//create an identity, a security group, a resource and give the identity some
+		//permissions on the resource
+		SecurityGroup secGroup = securityManager.createAndPersistSecurityGroup();
+		OLATResource resource = JunitTestHelper.createRandomResource();
+		Identity id = JunitTestHelper.createAndPersistIdentityAsUser("test-ipornc-1-" + UUID.randomUUID().toString());
+		securityManager.addIdentityToSecurityGroup(id, secGroup);
+		securityManager.createAndPersistPolicy(secGroup, "test.ipornc-1_1", resource);
+		securityManager.createAndPersistPolicy(secGroup, "test.ipornc-1_2", resource);
+		dbInstance.commitAndCloseSession();
+		
+		//check
+		boolean hasIpor_1_1 = securityManager.isIdentityPermittedOnResourceable(id, "test.ipornc-1_1", resource, false);
+		Assert.assertTrue(hasIpor_1_1);
+		boolean hasIpor_1_2 = securityManager.isIdentityPermittedOnResourceable(id, "test.ipornc-1_2", resource, false);
+		Assert.assertTrue(hasIpor_1_2);
+		boolean hasIpor_1_3 = securityManager.isIdentityPermittedOnResourceable(id, "test.ipornc-1_3", resource, false);
+		Assert.assertFalse(hasIpor_1_3);
+	}
+	
+	@Test
+	public void getIdentityPermissionsOnResourceable() {
+		//create an identity, a security group, a resource and give the identity some
+		//permissions on the resource
+		SecurityGroup secGroup = securityManager.createAndPersistSecurityGroup();
+		OLATResource resource = JunitTestHelper.createRandomResource();
+		Identity id = JunitTestHelper.createAndPersistIdentityAsUser("test-gpor-1-" + UUID.randomUUID().toString());
+		securityManager.addIdentityToSecurityGroup(id, secGroup);
+		securityManager.createAndPersistPolicy(secGroup, "test.gpor-1_1", resource);
+		securityManager.createAndPersistPolicy(secGroup, "test.gpor-1_2", resource);
+		dbInstance.commitAndCloseSession();
+		
+		//check
+		List<String> permissions = securityManager.getIdentityPermissionOnresourceable(id, resource);
+		Assert.assertNotNull(permissions);
+		Assert.assertTrue(permissions.size() >= 2);
+		Assert.assertTrue(permissions.contains("test.gpor-1_1"));
+		Assert.assertTrue(permissions.contains("test.gpor-1_2"));
+		Assert.assertFalse(permissions.contains("test.gpor-1_3"));
+	}
 }

src/test/java/org/olat/basesecurity/BaseSecurityTest.java

@@ -27,29 +27,26 @@
 package org.olat.basesecurity;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 import java.util.Calendar;
 import java.util.Date;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
-import org.hibernate.PropertyValueException;
+import junit.framework.Assert;
+
 import org.junit.After;
 import org.junit.Test;
 import org.olat.core.commons.persistence.DBFactory;
 import org.olat.core.id.Identity;
-import org.olat.core.id.OLATResourceable;
 import org.olat.core.id.Roles;
 import org.olat.core.id.User;
 import org.olat.core.id.UserConstants;
-import org.olat.core.logging.DBRuntimeException;
+import org.olat.core.logging.OLog;
+import org.olat.core.logging.Tracing;
 import org.olat.core.util.Encoder;
-import org.olat.core.util.resource.OresHelper;
 import org.olat.test.JunitTestHelper;
 import org.olat.test.OlatTestCase;
 import org.olat.user.UserManager;
@@ -62,16 +59,20 @@ import org.springframework.beans.factory.annotation.Autowired;
  */
 public class BaseSecurityTest extends OlatTestCase {
 	
+	private static final OLog log = Tracing.createLoggerFor(BaseSecurityTest.class);
+	
 	@Autowired
 	private BaseSecurity baseSecurityManager;
 	
 
 	
-	@Test public void testGetIdentitiesByPowerSearch() {
+	@Test
+	public void testGetIdentitiesByPowerSearch() {
 		System.out.println("start testGetIdentitiesByPowerSearch");
 		try {
 			Identity ident = getOrCreateIdentity("anIdentity");
 			Identity ident2 = getOrCreateTestIdentity("extremegroovy");
+			Assert.assertNotNull(ident2);
 			Identity deletedIdent = getOrCreateTestIdentity("delete");
 			deletedIdent = baseSecurityManager.saveIdentityStatus(deletedIdent, Identity.STATUS_DELETED);
 
@@ -202,11 +203,9 @@ public class BaseSecurityTest extends OlatTestCase {
 			// basic query to find all system users without restrictions
 			List<Identity> results = baseSecurityManager.getIdentitiesByPowerSearch(null, null, true, null, null, null, null, null, null, null, null);
 			assertTrue(results.size()>0); 
-			int numberOfAllUsers = results.size();
 
 			results = baseSecurityManager.getIdentitiesByPowerSearch(null, null, true, null, null, null, null, null, null, null, Identity.STATUS_DELETED);
 			assertTrue(results.size() >0);
-			int numberOfDeletedUsers = results.size();
 
 			results = baseSecurityManager.getIdentitiesByPowerSearch(null, null, true, groups1, null, null, null, null, null, null, null);
 			assertTrue(results.size() >0);
@@ -240,7 +239,7 @@ public class BaseSecurityTest extends OlatTestCase {
 
 			// policy search test
 			DBFactory.getInstance().closeSession();
-			List policies = baseSecurityManager.getPoliciesOfSecurityGroup(admins);
+			List<Policy> policies = baseSecurityManager.getPoliciesOfSecurityGroup(admins);
 			PermissionOnResourceable[] adminPermissions = convertPoliciesListToPermissionOnResourceArray(policies);
 			policies = baseSecurityManager.getPoliciesOfSecurityGroup(anonymous);
 			PermissionOnResourceable[] anonymousPermissions = convertPoliciesListToPermissionOnResourceArray(policies);
@@ -484,22 +483,22 @@ public class BaseSecurityTest extends OlatTestCase {
 			User onePropUser = UserManager.getInstance().createUser("onepropuser", "onepropuser", "onepropuser@lustig.com");
 			onePropUser.setProperty(UserConstants.FIRSTNAME, "one");		
 			Identity onePropeIdentity = baseSecurityManager.createAndPersistIdentityAndUser("onePropUser", onePropUser, BaseSecurityModule.getDefaultAuthProviderIdentifier(), "onepropuser", Encoder.encrypt("ppp"));
+			Assert.assertNotNull(onePropeIdentity);
 			
 			User twoPropUser = UserManager.getInstance().createUser("twopropuser", "twopropuser", "twopropuser@lustig.com");
 			twoPropUser.setProperty(UserConstants.FIRSTNAME, "two");
 			twoPropUser.setProperty(UserConstants.LASTNAME, "prop");
-
 			Identity twoPropeIdentity = baseSecurityManager.createAndPersistIdentityAndUser("twopropuser", twoPropUser, BaseSecurityModule.getDefaultAuthProviderIdentifier(), "twopropuser", Encoder.encrypt("ppp"));
+			Assert.assertNotNull(twoPropeIdentity);
 			// commit
 			DBFactory.getInstance().closeSession();
 
 			HashMap<String, String> userProperties;
-			List results;
-
+			
 			// find first
 			userProperties = new HashMap<String, String>();
 			userProperties.put(UserConstants.FIRSTNAME, "one");
-			results = baseSecurityManager.getIdentitiesByPowerSearch(null, userProperties, true, null, null, null, null, null, null, null, null);
+			List<Identity> results = baseSecurityManager.getIdentitiesByPowerSearch(null, userProperties, true, null, null, null, null, null, null, null, null);
 			assertTrue(results.size() == 1);
 
 			// no intersection - all properties optional
@@ -577,16 +576,16 @@ public class BaseSecurityTest extends OlatTestCase {
 			multiPropUser.setProperty(UserConstants.INSTITUTIONALUSERIDENTIFIER, "multiinst");		
 			multiPropUser.setProperty(UserConstants.CITY, "züri");		
 			Identity onePropeIdentity = baseSecurityManager.createAndPersistIdentityAndUser("multiPropUser", multiPropUser, BaseSecurityModule.getDefaultAuthProviderIdentifier(), "multipropuser", Encoder.encrypt("ppp"));
-
+			Assert.assertNotNull(onePropeIdentity);
+			
 			// commit
 			DBFactory.getInstance().closeSession();
 
 			HashMap<String, String> userProperties;
-			List results;
-			
+
 			userProperties = new HashMap<String, String>();
 			userProperties.put(UserConstants.FIRSTNAME, "multi");
-			results = baseSecurityManager.getIdentitiesByPowerSearch(null, userProperties, true, null, null, null, null, null, null, null, null);
+			List<Identity> results = baseSecurityManager.getIdentitiesByPowerSearch(null, userProperties, true, null, null, null, null, null, null, null, null);
 			sysoutResults(results);
 			assertTrue(results.size() == 1);
 
@@ -758,10 +757,10 @@ public class BaseSecurityTest extends OlatTestCase {
 		}
 	}
 
-	private PermissionOnResourceable[] convertPoliciesListToPermissionOnResourceArray(List policies) {
+	private PermissionOnResourceable[] convertPoliciesListToPermissionOnResourceArray(List<Policy> policies) {
 		PermissionOnResourceable[] array = new PermissionOnResourceable[policies.size()];
 		for (int i = 0; i < policies.size()	; i++) {
-			Policy policy = (Policy) policies.get(i);
+			Policy policy = policies.get(i);
 			PermissionOnResourceable por = new PermissionOnResourceable(policy.getPermission(), policy.getOlatResource());
 			array[i] = por;			
 		}
@@ -772,28 +771,25 @@ public class BaseSecurityTest extends OlatTestCase {
 	/*
 	 * Only for debugging to see identities result list.
 	 */
-	private void sysoutResults(List results) {
-		System.out.println("TEST results.size()=" + results.size());
-		for (Iterator iterator = results.iterator(); iterator.hasNext();) {
-			Identity identity = (Identity) iterator.next();
-			System.out.println("TEST ident=" + identity);
+	private void sysoutResults(List<Identity> results) {
+		log.info("TEST results.size()=" + results.size());
+		for (Identity identity:results) {
+			log.debug("TEST ident=" + identity);
 		}		
 	}
 
 	// check Helper Methoden
 	////////////////////////
-	private void checkIdentitiesHasPermissions(List results, PermissionOnResourceable[] adminPermissions) {
-		for (Iterator iterator = results.iterator(); iterator.hasNext();) {
-			Identity resultIdentity = (Identity) iterator.next();
+	private void checkIdentitiesHasPermissions(List<Identity> results, PermissionOnResourceable[] adminPermissions) {
+		for (Identity resultIdentity: results) {
 			for (int i = 0; i < adminPermissions.length; i++) {
 				assertTrue( baseSecurityManager.isIdentityPermittedOnResourceable(resultIdentity, adminPermissions[i].getPermission(), adminPermissions[i].getOlatResourceable() ) );
 			}
 		}
 	}
 	
-	private void checkIdentitiesHasAuthProvider(List results, String[] authProviders) {
-		for (Iterator iterator = results.iterator(); iterator.hasNext();) {
-			Identity resultIdentity = (Identity) iterator.next();
+	private void checkIdentitiesHasAuthProvider(List<Identity> results, String[] authProviders) {
+		for (Identity resultIdentity : results) {
 			boolean foundIdentityWithAuth = false;
 			for (int i = 0; i < authProviders.length; i++) {
 				Authentication authentication = baseSecurityManager.findAuthentication(resultIdentity, authProviders[i]);
@@ -805,9 +801,8 @@ public class BaseSecurityTest extends OlatTestCase {
 		}
 	}
 
-	private void checkIdentitiesAreInGroups(List results, SecurityGroup[] groups1) {
-		for (Iterator iterator = results.iterator(); iterator.hasNext();) {
-			Identity resultIdentity = (Identity) iterator.next();
+	private void checkIdentitiesAreInGroups(List<Identity> results, SecurityGroup[] groups1) {
+		for (Identity resultIdentity:results) {
 			boolean foundIdentityInSecGroup = false;
 			for (int i = 0; i < groups1.length; i++) {
 				if (baseSecurityManager.isIdentityInSecurityGroup(resultIdentity, groups1[i]) ) {
@@ -819,14 +814,11 @@ public class BaseSecurityTest extends OlatTestCase {
 	}
 	
 	private void checkIdentitiesHasRoles(List<Identity> results, boolean checkIsAuthor) {
-		for (Iterator iterator = results.iterator(); iterator.hasNext();) {
-			Identity resultIdentity = (Identity) iterator.next();
+		for (Identity resultIdentity: results) {
 			Roles roles = baseSecurityManager.getRoles(resultIdentity);
 			if (checkIsAuthor) {
 				assertTrue("Identity has not roles author, identity=" + resultIdentity, roles.isAuthor());
 			}
 		}
 	}
-	
-
 }
\ No newline at end of file