Commit ae6b3070 authored by Vincent Pelletier's avatar Vincent Pelletier

ERP5Security.ERP5GroupManager: Coding style.

Move variable initialisations closer to where they are actually used.
Do not initialise a variable which will always be initilised again before
its first use.
Reduce argument indentation.
Drop unhelpful comments.
User document may not be a Person instance, use a more neutral variable
name.
Avoid single-use locals.
Take advantage of zLOG.LOG's "error=True" idiom to not have to pull
exception information ourselves.
parent a80099b4
...@@ -28,8 +28,6 @@ from Products.ERP5Type.UnrestrictedMethod import UnrestrictedMethod ...@@ -28,8 +28,6 @@ from Products.ERP5Type.UnrestrictedMethod import UnrestrictedMethod
from Products.ZSQLCatalog.SQLCatalog import SimpleQuery from Products.ZSQLCatalog.SQLCatalog import SimpleQuery
from ZODB.POSException import ConflictError from ZODB.POSException import ConflictError
import sys
from zLOG import LOG, WARNING from zLOG import LOG, WARNING
from Products import ERP5Security from Products import ERP5Security
...@@ -88,11 +86,6 @@ class ERP5GroupManager(BasePlugin): ...@@ -88,11 +86,6 @@ class ERP5GroupManager(BasePlugin):
@UnrestrictedMethod @UnrestrictedMethod
def _getGroupsForPrincipal(user_id, path): def _getGroupsForPrincipal(user_id, path):
security_category_dict = {} # key is the base_category_list,
# value is the list of fetched categories
security_group_list = []
security_definition_list = ()
# To get the complete list of groups, we try to call the # To get the complete list of groups, we try to call the
# ERP5Type_getSecurityCategoryMapping which should return a list # ERP5Type_getSecurityCategoryMapping which should return a list
# of lists of two elements (script, base_category_list) like : # of lists of two elements (script, base_category_list) like :
...@@ -116,7 +109,6 @@ class ERP5GroupManager(BasePlugin): ...@@ -116,7 +109,6 @@ class ERP5GroupManager(BasePlugin):
else: else:
security_definition_list = mapping_method() security_definition_list = mapping_method()
# get the person from its login - no security check needed
user_path_set = { user_path_set = {
x['path'] x['path']
for x in self.searchUsers(id=user_id, exact_match=True) for x in self.searchUsers(id=user_id, exact_match=True)
...@@ -125,28 +117,36 @@ class ERP5GroupManager(BasePlugin): ...@@ -125,28 +117,36 @@ class ERP5GroupManager(BasePlugin):
if not user_path_set: if not user_path_set:
return () return ()
user_path, = user_path_set user_path, = user_path_set
person_object = self.getPortalObject().unrestrictedTraverse(user_path) user_value = self.getPortalObject().unrestrictedTraverse(user_path)
security_category_dict = {}
# Fetch category values from defined scripts
for (method_name, base_category_list) in security_definition_list: for (method_name, base_category_list) in security_definition_list:
base_category_list = tuple(base_category_list) base_category_list = tuple(base_category_list)
security_category_list = security_category_dict.setdefault( security_category_list = security_category_dict.setdefault(
base_category_list, []) base_category_list,
[],
)
try: try:
# The called script may want to distinguish if it is called # The called script may want to distinguish if it is called
# from here or from _updateLocalRolesOnSecurityGroups. # from here or from _updateLocalRolesOnSecurityGroups.
# Currently, passing portal_type='' (instead of 'Person') # Currently, passing portal_type='' (instead of 'Person')
# is the only way to make the difference. # is the only way to make the difference.
method = getattr(self, method_name)
security_category_list.extend( security_category_list.extend(
method(base_category_list, user_id, person_object, '') getattr(self, method_name)(
base_category_list,
user_id,
user_value,
'',
)
) )
except ConflictError: except ConflictError:
raise raise
except: except Exception:
LOG('ERP5GroupManager', WARNING, LOG(
'could not get security categories from %s' % (method_name,), 'ERP5GroupManager',
error = sys.exc_info()) WARNING,
'could not get security categories from %s' % (method_name, ),
error=True,
)
# Get group names from category values # Get group names from category values
# XXX try ERP5Type_asSecurityGroupIdList first for compatibility # XXX try ERP5Type_asSecurityGroupIdList first for compatibility
...@@ -155,23 +155,26 @@ class ERP5GroupManager(BasePlugin): ...@@ -155,23 +155,26 @@ class ERP5GroupManager(BasePlugin):
if group_id_list_generator is None: if group_id_list_generator is None:
generator_name = ERP5TYPE_SECURITY_GROUP_ID_GENERATION_SCRIPT generator_name = ERP5TYPE_SECURITY_GROUP_ID_GENERATION_SCRIPT
group_id_list_generator = getattr(self, generator_name, None) group_id_list_generator = getattr(self, generator_name, None)
for base_category_list, category_value_list in \ security_group_list = []
security_category_dict.iteritems(): for base_category_list, category_value_list in security_category_dict.iteritems():
for category_dict in category_value_list: for category_dict in category_value_list:
try: try:
group_id_list = group_id_list_generator( group_id_list = group_id_list_generator(
category_order=base_category_list, category_order=base_category_list,
**category_dict) **category_dict
)
if isinstance(group_id_list, str): if isinstance(group_id_list, str):
group_id_list = [group_id_list] group_id_list = [group_id_list]
security_group_list.extend(group_id_list) security_group_list.extend(group_id_list)
except ConflictError: except ConflictError:
raise raise
except: except Exception:
LOG('ERP5GroupManager', WARNING, LOG(
'could not get security groups from %s' % 'ERP5GroupManager',
generator_name, WARNING,
error = sys.exc_info()) 'could not get security groups from %s' % (generator_name, ),
error=True,
)
return tuple(security_group_list) return tuple(security_group_list)
if not NO_CACHE_MODE: if not NO_CACHE_MODE:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment