From 6de8b25041a407b2e7c247a21afd5dff3077b475 Mon Sep 17 00:00:00 2001 From: srosse <none@none> Date: Thu, 17 Jan 2013 09:19:29 +0100 Subject: [PATCH] OO-502: load the course rights with 1 query and not 7 --- .../org/olat/basesecurity/BaseSecurity.java | 8 +++ .../basesecurity/BaseSecurityManager.java | 15 ++++ .../org/olat/basesecurity/PolicyImpl.hbm.xml | 3 + .../groupsandrights/CourseGroupManager.java | 8 +++ .../PersistingCourseGroupManager.java | 4 ++ .../olat/course/run/RunMainController.java | 15 ++-- .../preview/PreviewCourseGroupManager.java | 8 ++- .../basesecurity/BaseSecurityManagerTest.java | 71 +++++++++++++++++++ .../olat/basesecurity/BaseSecurityTest.java | 70 ++++++++---------- 9 files changed, 155 insertions(+), 47 deletions(-) diff --git a/src/main/java/org/olat/basesecurity/BaseSecurity.java b/src/main/java/org/olat/basesecurity/BaseSecurity.java index 49b730e08be..4add7a97724 100644 --- a/src/main/java/org/olat/basesecurity/BaseSecurity.java +++ b/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 * diff --git a/src/main/java/org/olat/basesecurity/BaseSecurityManager.java b/src/main/java/org/olat/basesecurity/BaseSecurityManager.java index 2bc66bb454b..404d4997c6c 100644 --- a/src/main/java/org/olat/basesecurity/BaseSecurityManager.java +++ b/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); } diff --git a/src/main/java/org/olat/basesecurity/PolicyImpl.hbm.xml b/src/main/java/org/olat/basesecurity/PolicyImpl.hbm.xml index d4ce888b5da..2a631652e34 100644 --- a/src/main/java/org/olat/basesecurity/PolicyImpl.hbm.xml +++ b/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> diff --git a/src/main/java/org/olat/course/groupsandrights/CourseGroupManager.java b/src/main/java/org/olat/course/groupsandrights/CourseGroupManager.java index 3251aaf5979..ac6a05bc51f 100644 --- a/src/main/java/org/olat/course/groupsandrights/CourseGroupManager.java +++ b/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 diff --git a/src/main/java/org/olat/course/groupsandrights/PersistingCourseGroupManager.java b/src/main/java/org/olat/course/groupsandrights/PersistingCourseGroupManager.java index 34b19facfa8..16f84d6ccbd 100644 --- a/src/main/java/org/olat/course/groupsandrights/PersistingCourseGroupManager.java +++ b/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) { diff --git a/src/main/java/org/olat/course/run/RunMainController.java b/src/main/java/org/olat/course/run/RunMainController.java index dd765ae5fbe..d7cfcf8fd4f 100644 --- a/src/main/java/org/olat/course/run/RunMainController.java +++ b/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))); } /** diff --git a/src/main/java/org/olat/course/run/preview/PreviewCourseGroupManager.java b/src/main/java/org/olat/course/run/preview/PreviewCourseGroupManager.java index 0f4d51428e4..028e3d8d6cb 100644 --- a/src/main/java/org/olat/course/run/preview/PreviewCourseGroupManager.java +++ b/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) { diff --git a/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java b/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java index 5fd8a64d088..6d878821719 100644 --- a/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java +++ b/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")); + } } diff --git a/src/test/java/org/olat/basesecurity/BaseSecurityTest.java b/src/test/java/org/olat/basesecurity/BaseSecurityTest.java index a40b8aee7b7..7ee5f0a4faa 100644 --- a/src/test/java/org/olat/basesecurity/BaseSecurityTest.java +++ b/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 -- GitLab