You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by do...@apache.org on 2009/09/21 18:48:35 UTC

svn commit: r817316 - in /ofbiz/trunk: applications/securityext/script/org/ofbiz/securityext/securitygroup/ framework/security/src/org/ofbiz/security/ framework/security/src/org/ofbiz/security/authz/

Author: doogie
Date: Mon Sep 21 16:48:34 2009
New Revision: 817316

URL: http://svn.apache.org/viewvc?rev=817316&view=rev
Log:
Remove custom UtilCache caching of security items; this is actually
a bug fix, the minilang services were referencing fields that no
longer existed.

Modified:
    ofbiz/trunk/applications/securityext/script/org/ofbiz/securityext/securitygroup/SecurityGroupServices.xml
    ofbiz/trunk/framework/security/src/org/ofbiz/security/OFBizSecurity.java
    ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java
    ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/EntityAuthorization.java

Modified: ofbiz/trunk/applications/securityext/script/org/ofbiz/securityext/securitygroup/SecurityGroupServices.xml
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/securityext/script/org/ofbiz/securityext/securitygroup/SecurityGroupServices.xml?rev=817316&r1=817315&r2=817316&view=diff
==============================================================================
--- ofbiz/trunk/applications/securityext/script/org/ofbiz/securityext/securitygroup/SecurityGroupServices.xml (original)
+++ ofbiz/trunk/applications/securityext/script/org/ofbiz/securityext/securitygroup/SecurityGroupServices.xml Mon Sep 21 16:48:34 2009
@@ -72,9 +72,6 @@
         <make-value value-field="newEntity" entity-name="SecurityGroupPermission"/>
         <set-pk-fields map="parameters" value-field="newEntity"/>
 
-        <!-- clear the org.ofbiz.security.Security object's custom cache by newEntity -->
-        <call-bsh><![CDATA[ org.ofbiz.security.Security.securityGroupPermissionCache.remove(newEntity); ]]></call-bsh>
-
         <create-value value-field="newEntity"/>
     </simple-method>
     <simple-method method-name="removeSecurityPermissionFromSecurityGroup" short-description="Remove SecurityPermission From SecurityGroup">
@@ -85,9 +82,6 @@
         <set-pk-fields map="parameters" value-field="lookupPKMap"/>
         <find-by-primary-key entity-name="SecurityGroupPermission" map="lookupPKMap" value-field="lookedUpValue"/>
         <remove-value value-field="lookedUpValue"/>
-
-        <!-- clear the org.ofbiz.security.Security object's custom cache by lookupPKMap -->
-        <call-bsh><![CDATA[ org.ofbiz.security.Security.securityGroupPermissionCache.remove(lookupPKMap); ]]></call-bsh>
     </simple-method>
 
     <!-- UserLogin to SecurityGroup methods -->
@@ -105,9 +99,6 @@
         </if-empty>
 
         <create-value value-field="newEntity"/>
-
-        <!-- clear the org.ofbiz.security.Security object's custom cache by userLoginId -->
-        <call-bsh><![CDATA[ org.ofbiz.security.Security.userLoginSecurityGroupByUserLoginId.remove(parameters.get("userLoginId")); ]]></call-bsh>
     </simple-method>
     <simple-method method-name="updateUserLoginToSecurityGroup" short-description="Update UserLogin to SecurityGroup">
         <check-permission permission="SECURITY" action="_UPDATE"><fail-message message="Security Error: to run updateUserLoginToSecurityGroup you must have the SECURITY_UPDATE or SECURITY_ADMIN permission"/></check-permission>
@@ -118,9 +109,6 @@
         <find-by-primary-key entity-name="UserLoginSecurityGroup" map="lookupPKMap" value-field="lookedUpValue"/>
         <set-nonpk-fields map="parameters" value-field="lookedUpValue"/>
         <store-value value-field="lookedUpValue"/>
-
-        <!-- clear the org.ofbiz.security.Security object's custom cache by userLoginId -->
-        <call-bsh><![CDATA[ org.ofbiz.security.Security.userLoginSecurityGroupByUserLoginId.remove(parameters.get("userLoginId")); ]]></call-bsh>
     </simple-method>
     <simple-method method-name="removeUserLoginFromSecurityGroup" short-description="Remove UserLogin From SecurityGroup">
         <check-permission permission="SECURITY" action="_DELETE"><fail-message message="Security Error: to run removeUserLoginFromSecurityGroup you must have the SECURITY_DELETE or SECURITY_ADMIN permission"/></check-permission>
@@ -130,9 +118,6 @@
         <set-pk-fields map="parameters" value-field="lookupPKMap"/>
         <find-by-primary-key entity-name="UserLoginSecurityGroup" map="lookupPKMap" value-field="lookedUpValue"/>
         <remove-value value-field="lookedUpValue"/>
-
-        <!-- clear the org.ofbiz.security.Security object's custom cache by userLoginId -->
-        <call-bsh><![CDATA[ org.ofbiz.security.Security.userLoginSecurityGroupByUserLoginId.remove(parameters.get("userLoginId")); ]]></call-bsh>
     </simple-method>
 
     <!-- ProtectedView to SecurityGroup methods -->
@@ -144,9 +129,6 @@
         <set-pk-fields map="parameters" value-field="newEntity"/>
         <set-nonpk-fields map="parameters" value-field="newEntity"/>
 
-        <!-- clear the org.ofbiz.security.Security object's custom cache by newEntity -->
-        <call-bsh><![CDATA[ org.ofbiz.security.Security.userLoginSecurityGroupByUserLoginId.remove(newEntity); ]]></call-bsh>
-
         <create-value value-field="newEntity"/>
     </simple-method>
     <simple-method method-name="updateProtectedViewToSecurityGroup" short-description="Update ProtectedView to SecurityGroup">
@@ -157,9 +139,6 @@
         <find-by-primary-key entity-name="ProtectedView" map="lookupPKMap" value-field="lookedUpValue"/>
         <set-nonpk-fields map="parameters" value-field="lookedUpValue"/>
         <store-value value-field="lookedUpValue"/>
-
-        <!-- clear the org.ofbiz.security.Security object's custom cache by lookupPKMap -->
-        <call-bsh><![CDATA[ org.ofbiz.security.Security.userLoginSecurityGroupByUserLoginId.remove(lookupPKMap); ]]></call-bsh>
     </simple-method>
     <simple-method method-name="removeProtectedViewFromSecurityGroup" short-description="Remove ProtectedView From SecurityGroup">
         <check-permission permission="SECURITY" action="_DELETE"><fail-message message="Security Error: to run removeProtectedViewFromSecurityGroup you must have the SECURITY_DELETE or SECURITY_ADMIN permission"/></check-permission>
@@ -169,8 +148,5 @@
         <set-pk-fields map="parameters" value-field="lookupPKMap"/>
         <find-by-primary-key entity-name="ProtectedView" map="lookupPKMap" value-field="lookedUpValue"/>
         <remove-value value-field="lookedUpValue"/>
-
-        <!-- clear the org.ofbiz.security.Security object's custom cache by lookupPKMap -->
-        <call-bsh><![CDATA[ org.ofbiz.security.Security.userLoginSecurityGroupByUserLoginId.remove(lookupPKMap); ]]></call-bsh>
     </simple-method>
 </simple-methods>

Modified: ofbiz/trunk/framework/security/src/org/ofbiz/security/OFBizSecurity.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/security/src/org/ofbiz/security/OFBizSecurity.java?rev=817316&r1=817315&r2=817316&view=diff
==============================================================================
--- ofbiz/trunk/framework/security/src/org/ofbiz/security/OFBizSecurity.java (original)
+++ ofbiz/trunk/framework/security/src/org/ofbiz/security/OFBizSecurity.java Mon Sep 21 16:48:34 2009
@@ -47,18 +47,6 @@
 
     public static final String module = OFBizSecurity.class.getName();
 
-    /**
-     * UtilCache to cache a Collection of UserLoginSecurityGroup entities for each UserLogin, by userLoginId.
-     */
-    protected static UtilCache<String, List<GenericValue>> userLoginSecurityGroupByUserLoginId = new UtilCache<String, List<GenericValue>>("security.UserLoginSecurityGroupByUserLoginId");
-
-    /**
-     * UtilCache to cache whether or not a certain SecurityGroupPermission row exists or not.
-     * For each SecurityGroupPermissionPK there is a Boolean in the cache specifying whether or not it exists.
-     * In this way the cache speeds things up whether or not the user has a permission.
-     */
-    protected static UtilCache<GenericValue, Boolean> securityGroupPermissionCache = new UtilCache<GenericValue, Boolean>("security.SecurityGroupPermissionCache");
-
     protected GenericDelegator delegator = null;
 
     protected static final Map<String, Map<String, String>> simpleRoleEntity = UtilMisc.toMap(
@@ -84,17 +72,13 @@
      * @see org.ofbiz.security.Security#findUserLoginSecurityGroupByUserLoginId(java.lang.String)
      */
     public Iterator<GenericValue> findUserLoginSecurityGroupByUserLoginId(String userLoginId) {
-        List<GenericValue> collection = userLoginSecurityGroupByUserLoginId.get(userLoginId);
-
-        if (collection == null) {
-            try {
-                collection = delegator.findByAnd("UserLoginSecurityGroup", UtilMisc.toMap("userLoginId", userLoginId), null);
-                // make an empty collection to speed up the case where a userLogin belongs to no security groups, only with no exception of course
-                if (collection == null) collection = FastList.newInstance();
-                userLoginSecurityGroupByUserLoginId.put(userLoginId, collection);
-            } catch (GenericEntityException e) {
-                Debug.logWarning(e, module);
-            }
+        List<GenericValue> collection;
+        try {
+            collection = delegator.findByAnd("UserLoginSecurityGroup", UtilMisc.toMap("userLoginId", userLoginId), null);
+        } catch (GenericEntityException e) {
+            // make an empty collection to speed up the case where a userLogin belongs to no security groups, only with no exception of course
+            collection = FastList.newInstance();
+            Debug.logWarning(e, module);
         }
         // filter each time after cache retreival, ie cache will contain entire list
         collection = EntityUtil.filterByDate(collection, true);
@@ -107,22 +91,12 @@
     public boolean securityGroupPermissionExists(String groupId, String permission) {
         GenericValue securityGroupPermissionValue = delegator.makeValue("SecurityGroupPermission",
                 UtilMisc.toMap("groupId", groupId, "permissionId", permission));
-        Boolean exists = (Boolean) securityGroupPermissionCache.get(securityGroupPermissionValue);
-
-        if (exists == null) {
-            try {
-                if (delegator.findOne(securityGroupPermissionValue.getEntityName(), securityGroupPermissionValue, false) != null) {
-                    exists = Boolean.TRUE;
-                } else {
-                    exists = Boolean.FALSE;
-                }
-            } catch (GenericEntityException e) {
-                exists = Boolean.FALSE;
-                Debug.logWarning(e, module);
-            }
-            securityGroupPermissionCache.put(securityGroupPermissionValue, exists);
+        try {
+            return delegator.findOne(securityGroupPermissionValue.getEntityName(), securityGroupPermissionValue, false) != null;
+        } catch (GenericEntityException e) {
+            Debug.logWarning(e, module);
+            return false;
         }
-        return exists.booleanValue();
     }
 
     /**
@@ -293,7 +267,7 @@
 
     public void clearUserData(GenericValue userLogin) {
         if (userLogin != null) {
-            userLoginSecurityGroupByUserLoginId.remove(userLogin.getString("userLoginId"));
+            delegator.getCache().remove("UserLoginSecurityGroup", EntityCondition.makeCondition("userLoginId", EntityOperator.EQUALS, userLogin.getString("userLoginId")));
         }
     }
 

Modified: ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java?rev=817316&r1=817315&r2=817316&view=diff
==============================================================================
--- ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java (original)
+++ ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java Mon Sep 21 16:48:34 2009
@@ -46,7 +46,6 @@
 
     /**
      * Finds whether or not a SecurityGroupPermission row exists given a groupId and permission.
-     * Uses the securityGroupPermissionCache to speed this up.
      * The groupId,permission pair is cached instead of the userLoginId,permission pair to keep the cache small and to
      * make it more changeable.
      *

Modified: ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/EntityAuthorization.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/EntityAuthorization.java?rev=817316&r1=817315&r2=817316&view=diff
==============================================================================
--- ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/EntityAuthorization.java (original)
+++ ofbiz/trunk/framework/security/src/org/ofbiz/security/authz/EntityAuthorization.java Mon Sep 21 16:48:34 2009
@@ -33,6 +33,8 @@
 import org.ofbiz.entity.GenericDelegator;
 import org.ofbiz.entity.GenericEntityException;
 import org.ofbiz.entity.GenericValue;
+import org.ofbiz.entity.cache.Cache;
+import org.ofbiz.entity.condition.EntityCondition;
 import org.ofbiz.entity.util.EntityUtil;
 import org.ofbiz.security.authz.da.DynamicAccessFactory;
 import org.ofbiz.security.authz.da.DynamicAccessHandler;
@@ -41,23 +43,6 @@
 
     private static final String module = EntityAuthorization.class.getName();
     
-    /**
-     * UtilCache to cache a Collection of UserLoginSecurityGroup entities for each UserLogin, by userLoginId.
-     */
-    private static UtilCache<String, List<GenericValue>> userLoginSecurityGroupByUserLoginId = new UtilCache<String, List<GenericValue>>("security.UserLoginSecurityGroupByUserLoginId");
-
-    /**
-     * UtilCache to cache whether or not a certain SecurityGroupPermission row exists or not.
-     * For each SecurityGroupPermissionPK there is a Boolean in the cache specifying whether or not it exists.
-     * In this way the cache speeds things up whether or not the user has a permission.
-     */
-    private static UtilCache<GenericValue, Boolean> securityGroupPermissionCache = new UtilCache<GenericValue, Boolean>("security.SecurityGroupPermissionCache");
-
-    /**
-     * UtilCache to cache Permission Auto Grant permissions
-     */
-    private static UtilCache<String, List<String>> permissionAutoGrantCache = new UtilCache<String, List<String>>("security.PermissionAutoGrantCache");
-    
     protected GenericDelegator delegator; 
     
     @Override
@@ -158,20 +143,14 @@
     }
     
     private Iterator<GenericValue> getUserLoginSecurityGroupByUserLoginId(String userId) {
-        List<GenericValue> collection = userLoginSecurityGroupByUserLoginId.get(userId);
+        List<GenericValue> collection;
 
-        if (collection == null) {
-            try {
-                collection = delegator.findByAnd("UserLoginSecurityGroup", UtilMisc.toMap("userLoginId", userId), null);
-                
-                // make an empty collection to speed up the case where a userLogin belongs to no security groups, only with no exception of course
-                if (collection == null) {
-                    collection = FastList.newInstance();
-                }
-                userLoginSecurityGroupByUserLoginId.put(userId, collection);
-            } catch (GenericEntityException e) {
-                Debug.logWarning(e, module);
-            }
+        try {
+            collection = delegator.findByAnd("UserLoginSecurityGroup", UtilMisc.toMap("userLoginId", userId), null);
+        } catch (GenericEntityException e) {
+            // make an empty collection to speed up the case where a userLogin belongs to no security groups, only with no exception of course
+            collection = FastList.newInstance();
+            Debug.logWarning(e, module);
         }
         
         // filter each time after cache retrieval, i.e. cache will contain entire list
@@ -182,32 +161,25 @@
     private boolean securityGroupHasPermission(String groupId, String permission) {
         GenericValue securityGroupPermissionValue = delegator.makeValue("SecurityGroupPermission",
                 UtilMisc.toMap("groupId", groupId, "permissionId", permission));
-        Boolean exists = (Boolean) securityGroupPermissionCache.get(securityGroupPermissionValue);
 
-        if (exists == null) {
-            try {
-                if (delegator.findOne(securityGroupPermissionValue.getEntityName(), securityGroupPermissionValue, false) != null) {
-                    exists = Boolean.TRUE;
-                } else {
-                    exists = Boolean.FALSE;
-                }
-            } catch (GenericEntityException e) {
-                exists = Boolean.FALSE;
-                Debug.logWarning(e, module);
-            }
-            securityGroupPermissionCache.put(securityGroupPermissionValue, exists);
+        try {
+            return delegator.findOne(securityGroupPermissionValue.getEntityName(), securityGroupPermissionValue, false) != null;
+        } catch (GenericEntityException e) {
+            Debug.logWarning(e, module);
+            return false;
         }
-        return exists.booleanValue();
     }    
     
     private List<String> getPermissionAutoGrant(String permission) {
-        List<String> autoGrants = permissionAutoGrantCache.get(permission);
+        Cache cache = delegator.getCache();
+        EntityCondition condition = EntityCondition.makeCondition(UtilMisc.toMap("permissionId", permission));
+        List<String> autoGrants = cache.get("SecurityPermissionAutoGrant", condition, "EntityAuthorization.autoGrants");
         if (autoGrants == null) {
             autoGrants = FastList.newInstance();
             
             List<GenericValue> values = null;
             try {
-                values = delegator.findByAnd("SecurityPermissionAutoGrant", UtilMisc.toMap("permissionId", permission), null);
+                values = delegator.findList("SecurityPermissionAutoGrant", condition, null, null, null, true);
             } catch (GenericEntityException e) {
                 Debug.logWarning(e, module);
             }
@@ -217,7 +189,7 @@
                     autoGrants.add(v.getString("grantPermission"));
                 }
             }
-            permissionAutoGrantCache.put(permission, autoGrants);
+            cache.put("SecurityPermissionAutoGrant", condition, "EntityAuthorization.autoGrants", autoGrants);
         }
         return autoGrants;
     }