Attachment 5222 Details for Bug 30991

[patch] Speedup syntax lookups

bug30991.patch (text/plain), 13.65 KB, created by Dirk Wiesenthal on 2013-05-12 23:14 CEST

(hide)

Description:

Filename:

MIME Type:

Creator: Dirk Wiesenthal

Created: 2013-05-12 23:14 CEST

Size: 13.65 KB

patch

obsolete

>Index: univention-management-console-module-udm/umc/python/udm/udm_ldap.py
>===================================================================
>--- univention-management-console-module-udm/umc/python/udm/udm_ldap.py	(Revision 40329)
>+++ univention-management-console-module-udm/umc/python/udm/udm_ldap.py	(Arbeitskopie)
>@@ -407,7 +407,7 @@
> 			raise UDM_Error( get_exception_msg(e) )
> 
> 	@LDAP_Connection
>-	def search( self, container = None, attribute = None, value = None, superordinate = None, scope = 'sub', filter = '', ldap_connection = None, ldap_position = None ):
>+	def search( self, container = None, attribute = None, value = None, superordinate = None, scope = 'sub', filter = '', simple = False, simple_attrs = None, ldap_connection = None, ldap_position = None ):
> 		"""Searches for LDAP objects based on a search pattern"""
> 		if container == 'all':
> 			container = ldap_position.getBase()
>@@ -421,7 +421,18 @@
> 		result = None
> 		try:
> 			sizelimit = int(ucr.get('directory/manager/web/sizelimit', '2000'))
>-			result = self.module.lookup( None, ldap_connection, filter_s, base = container, superordinate = superordinate, scope = scope, sizelimit = sizelimit )
>+			if simple:
>+				object_type_filter = 'univentionObjectType=%s' % self.module.module
>+				if filter:
>+					if not filter.startswith('('):
>+						filter = '(%s)' % filter
>+					object_type_filter = '(&(%s)%s)' % (object_type_filter, filter)
>+				if simple_attrs is not None:
>+					result = ldap_connection.search( filter = object_type_filter, base = container, scope = scope, sizelimit = sizelimit, attr = simple_attrs )
>+				else:
>+					result = ldap_connection.searchDn( filter = object_type_filter, base = container, scope = scope, sizelimit = sizelimit )
>+			else:
>+				result = self.module.lookup( None, ldap_connection, filter_s, base = container, superordinate = superordinate, scope = scope, sizelimit = sizelimit )
> 		except udm_errors.insufficientInformation, e:
> 			return []
> 		except udm_errors.ldapTimeout, e:
>@@ -1004,7 +1015,7 @@
> 		filter_s = '(&%s%s)' % (property_filter_s, filter_s)
> 	return filter_s
> 
>-LDAP_ATTR_RE = re.compile(r'^%$([^(]*)$s$') # '%(username)s' -> 'username'
>+LDAP_ATTR_RE = re.compile(r'^%$([^)]*)$s$') # '%(username)s' -> 'username'
> def _get_syntax( syntax_name ):
> 	if syntax_name not in udm_syntax.__dict__:
> 		return None
>@@ -1051,7 +1062,7 @@
> 			filter_s = _create_ldap_filter( syn, options, module )
> 			if filter_s is not None:
> 				try:
>-					size += len( module.search( filter = filter_s ) )
>+					size += len( module.search( filter = filter_s, simple=True ) )
> 				except udm_errors.ldapSizelimitExceeded:
> 					return {'performs_well' : True, 'size_limit_exceeded' : True}
> 		return {'size' : size, 'performs_well' : True }
>@@ -1065,46 +1076,103 @@
> 
> 	if issubclass( syn.__class__, udm_syntax.UDM_Objects ):
> 		syn.choices = []
>-		def map_choices( obj_list ):
>-			result = []
>-			for obj in obj_list:
>-				obj.open()
>+		# try to avoid using the slow udm interface
>+		simple = False
>+		attr = []
>+		if not syn.always_use_objects and not syn.udm_filter:
>+			attr.extend(re.findall(r'%$([^)]+)$', syn.key))
>+			if syn.label:
>+				attr.extend(re.findall(r'%$([^)]+)$', syn.label))
>+			attr = set(attr)
>+			for udm_module in syn.udm_modules:
>+				module = UDM_Module( udm_module )
>+				if module is not None:
>+					mapping = module.module.mapping
>+					if not all([mapping.mapName(att) for att in attr]):
>+						break
>+			else:
>+				simple = True
>+		def extract_key_label(syn, dn, info):
>+			key = label = None
>+			if syn.key == 'dn':
>+				key = dn
>+			else:
>+				try:
>+					key = syn.key % info
>+				except KeyError:
>+					pass
>+			if syn.label == 'dn':
>+				label = dn
>+			elif syn.label is None:
>+				pass
>+			else:
>+				try:
>+					label = syn.label % info
>+				except KeyError:
>+					pass
>+			return key, label
>+		if not simple:
>+			def map_choices( obj_list ):
>+				result = []
>+				for obj in obj_list:
>+					# first try it without obj.open() (expensive)
>+					key, label = extract_key_label(syn, obj.dn, obj.info)
>+					if key is None or label is None:
>+						obj.open()
>+						key, label = extract_key_label(syn, obj.dn, obj.info)
>+						if key is None:
>+							# ignore the entry as the key is important for a selection, there
>+							# is no sensible fallback for the key (Bug #26994)
>+							continue
>+						if label is None:
>+							# fallback to the default description as this is just what displayed
>+							# to the user (Bug #26994)
>+							label = udm_objects.description( obj )
>+					result.append( (key, label) )
>+				return result
> 
>-				if syn.key == 'dn':
>-					key = obj.dn
>+			for udm_module in syn.udm_modules:
>+				module = UDM_Module( udm_module )
>+				if module is None:
>+					continue
>+				filter_s = _create_ldap_filter( syn, options, module )
>+				if filter_s is None:
>+					syn.choices = []
> 				else:
>-					try:
>-						key = syn.key % obj.info
>-					except KeyError:
>-						# ignore the entry as the key is important for a selection, there
>-						# is no sensible fallback for the key (Bug #26994)
>-						continue
>-				if syn.label is None:
>-					label = udm_objects.description( obj )
>-				elif syn.label == 'dn':
>-					label = obj.dn
>-				else:
>-					try:
>-						label = syn.label % obj.info
>-					except KeyError:
>-						# fallback to the default description as this is just what displayed
>-						# to the user (Bug #26994)
>-						label = udm_objects.description( obj )
>-
>-				result.append( (key, label) )
>-			return result
>-
>-		for udm_module in syn.udm_modules:
>-			module = UDM_Module( udm_module )
>-			if module is None:
>-				continue
>-			filter_s = _create_ldap_filter( syn, options, module )
>-			if filter_s is None:
>-				syn.choices = []
>-			else:
>-				search_options = {'filter' : filter_s}
>-				search_options.update(module_search_options)
>-				syn.choices.extend( map_choices( module.search( **search_options ) ) )
>+					search_options = {'filter' : filter_s}
>+					search_options.update(module_search_options)
>+					syn.choices.extend( map_choices( module.search( **search_options ) ) )
>+		else:
>+			for udm_module in syn.udm_modules:
>+				module = UDM_Module( udm_module )
>+				if module is None:
>+					continue
>+				filter_s = _create_ldap_filter( syn, options, module )
>+				if filter_s is not None:
>+					if filter_s and not filter_s.startswith('('):
>+						filter_s = '(%s)' % filter_s
>+					mapping = module.module.mapping
>+					ldap_attr = [mapping.mapName(att) for att in attr]
>+					search_options = {'filter' : filter_s, 'simple' : True}
>+					search_options.update(module_search_options)
>+					if ldap_attr:
>+						search_options['simple_attrs'] = ldap_attr
>+						result = module.search( **search_options )
>+						for dn, ldap_map in result:
>+							info = univention.admin.mapping.mapDict(mapping, ldap_map)
>+							key, label = extract_key_label(syn, dn, info)
>+							if key is None:
>+								continue
>+							if label is None:
>+								label = ldap_connection.explodeDn(dn, 1)[0]
>+							syn.choices.append((key, label))
>+					else:
>+						keys = module.search( **search_options )
>+						if syn.label == 'dn':
>+							labels = keys
>+						else:
>+							labels = [ldap_connection.explodeDn(dn, 1)[0] for dn in keys]
>+						syn.choices.extend(zip(keys, labels))
> 		if isinstance( syn.static_values, ( tuple, list ) ):
> 			for value in syn.static_values:
> 				syn.choices.insert( 0, value )
>Index: univention-directory-manager-modules/modules/univention/admin/syntax.py
>===================================================================
>--- univention-directory-manager-modules/modules/univention/admin/syntax.py	(Revision 40329)
>+++ univention-directory-manager-modules/modules/univention/admin/syntax.py	(Arbeitskopie)
>@@ -274,6 +274,7 @@
> 	depends = None
> 	error_message = _( "Not a valid LDAP DN" )
> 	simple = False # by default a MultiObjectSelect widget is used; if simple == True a ComboBox is used
>+	always_use_objects = False
> 
> 	@classmethod
> 	def parse( self, text ):
>@@ -1210,7 +1211,7 @@
> 
> class LDAP_Server( UDM_Objects ):
> 	udm_modules = ( 'computers/domaincontroller_master', 'computers/domaincontroller_backup', 'computers/domaincontroller_slave' )
>-	label = '%(fqdn)s'
>+	label = '%(name)s.%(domain)s' # ldap-optimized '%(fqdn)s'
> 	simple = True
> 
> class IMAP_POP3( select ):
>@@ -1609,6 +1610,7 @@
> 
> class HostDN( UDM_Objects ):
> 	udm_modules = ( 'computers/computer', )
>+	always_use_objects = True
> 
> class UserID( UDM_Objects ):
> 	udm_modules = ( 'users/user', )
>@@ -1626,8 +1628,8 @@
> 
> class IComputer_FQDN( UDM_Objects ):
> 	udm_modules = ()
>-	key = '%(fqdn)s'
>-	label = '%(fqdn)s'
>+	key = '%(name)s.%(domain)s' # ldap-optimized '%(fqdn)s'
>+	label = '%(name)s.%(domain)s' # ldap-optimized '%(fqdn)s'
> 	regex = re.compile( '(?=^.{1,254}$)(^(?:(?!\d+\.)[a-zA-Z0-9_\-]{1,63}\.?)+(?:[a-zA-Z0-9]{2,})$)' ) #'(^[a-zA-Z])(([a-zA-Z0-9-_]*)([a-zA-Z0-9]$))?$' )
> 	error_message = _( 'Not a valid FQDN' )
> 	simple = True
>@@ -1693,6 +1695,7 @@
> class DNS_ReverseZone( UDM_Objects ):
>  	description=_('DNS reverse zone')
> 	udm_modules = ( 'dns/reverse_zone', )
>+	label = '%(subnet)s'
> 	empty_value = True
> 	size = 'TwoThirds'
> 
>@@ -1717,7 +1720,6 @@
> class dhcpService( UDM_Objects ):
> 	udm_modules = ( 'dhcp/service', )
> 	description=_('DHCP service')
>-	label = '%(name)s'
> 	empty_value = True
> 	size = 'TwoThirds'
> 
>@@ -2435,6 +2437,7 @@
> class nagiosHostsEnabledDn( UDM_Objects ):
> 	udm_modules = ( 'computers/computer', )
> 	udm_filter = '(&(objectClass=univentionNagiosHostClass)(univentionNagiosEnabled=1)(aRecord=*))'
>+	always_use_objects = True
> 
> class nagiosServiceDn( UDM_Objects ):
> 	udm_modules = ( 'nagios/service', )
>@@ -2550,8 +2553,7 @@
> 
> class nfsShare(UDM_Objects):
> 	udm_modules = ( 'shares/share', )
>-	key = 'dn'
>-	label = '%(printablename)s'
>+	label = '%(name)s (%(host)s)' # '%(printablename)s' optimized for performance...
> 	udm_filter = 'objectClass=univentionShareNFS'
> 
> class nfsMounts(complex):
>Index: univention-directory-manager-modules/modules/univention/admin/handlers/groups/group.py
>===================================================================
>--- univention-directory-manager-modules/modules/univention/admin/handlers/groups/group.py	(Revision 40329)
>+++ univention-directory-manager-modules/modules/univention/admin/handlers/groups/group.py	(Arbeitskopie)
>@@ -260,6 +260,7 @@
> 
> mapping=univention.admin.mapping.mapping()
> mapping.register('name', 'cn', None, univention.admin.mapping.ListToString)
>+mapping.register('gidNumber', 'gidNumber', None, univention.admin.mapping.ListToString)
> mapping.register('description', 'description', None, univention.admin.mapping.ListToString)
> mapping.register('sambaGroupType', 'sambaGroupType', None, univention.admin.mapping.ListToString)
> mapping.register('mailAddress', 'mailPrimaryAddress', None, univention.admin.mapping.ListToString)
>@@ -373,8 +374,6 @@
> 		else:
> 			self._define_options( options )
> 
>-		self.info['gidNumber'] = self.oldattr.get('gidNumber', [''])[0]
>-
> 		if 'samba' in self.options:
> 			sid = self.oldattr.get('sambaSID', [''])[0]
> 			pos = sid.rfind('-')
>Index: univention-directory-manager-modules/modules/univention/admin/handlers/shares/share.py
>===================================================================
>--- univention-directory-manager-modules/modules/univention/admin/handlers/shares/share.py	(Revision 40329)
>+++ univention-directory-manager-modules/modules/univention/admin/handlers/shares/share.py	(Arbeitskopie)
>@@ -827,6 +827,11 @@
> 			if 'univentionShareNFS' in self.oldattr['objectClass']:
> 				self.options.append( 'nfs' )
> 			try:
>+				# Attention: Because of performance reasons, the syntax
>+				#   class nfsShare uses '%(name)s (%(host)s)' as label, not
>+				#   '%(printablename)s' (may be looked up in ldap directly).
>+				#   If you change printablename here you probably want to change
>+				#   nfsShare.label, too.
> 				self['printablename'] = "%s (%s)" % (self['name'], self['host'])
> 			except:
> 				pass
>@@ -900,7 +905,6 @@
> 		if not hasattr(self,"options"):
> 			self.open()
> 		if 'nfs' in self.options:
>-			ulist=[]
> 			searchstring="*"+self['host']+":"+self['path']+"*"
> 			searchResult=self.lo.searchDn(base=self.position.getDomain(), filter='(&(objectClass=person)(automountInformation=%s))'%searchstring, scope='domain')
> 			if searchResult:
>Index: univention-directory-manager-modules/modules/univention/admin/handlers/users/user.py
>===================================================================
>--- univention-directory-manager-modules/modules/univention/admin/handlers/users/user.py	(Revision 40329)
>+++ univention-directory-manager-modules/modules/univention/admin/handlers/users/user.py	(Arbeitskopie)
>@@ -1205,6 +1205,9 @@
> 	return new
> 
> mapping=univention.admin.mapping.mapping()
>+mapping.register('username', 'uid', None, univention.admin.mapping.ListToString)
>+mapping.register('uidNumber', 'uidNumber', None, univention.admin.mapping.ListToString)
>+mapping.register('gidNumber', 'gidNumber', None, univention.admin.mapping.ListToString)
> mapping.register('title', 'title', None, univention.admin.mapping.ListToString)
> mapping.register('description', 'description', None, univention.admin.mapping.ListToString)
> mapping.register('organisation', 'o', None, univention.admin.mapping.ListToString)
>@@ -1412,7 +1415,6 @@
> 					username_match=s.parse(uid)
> 				except univention.admin.uexceptions.valueError,e: # uid contains already mixed case umlauts, so we switch
> 					self.set_uid_umlauts()
>-				self['username']=uid
> 			# FIXME: we should NEVER catch all exceptions
> 			except Exception, e:
> 				# at least write some debuging output..
>@@ -1500,9 +1502,6 @@
> 						self.save()
> 						raise univention.admin.uexceptions.primaryGroup
> 
>-					self.info['uidNumber'] = self.oldattr.get('uidNumber', [''])[0]
>-					self.info['gidNumber'] = self.oldattr.get('gidNumber', [''])[0]
>-
> 			if self['passwordexpiry']:
> 				today=time.strftime('%Y-%m-%d').split('-')
> 				expiry=self['passwordexpiry'].split('-')

Actions: View | Diff

Attachments on bug 30991: 5220 | 5221 | 5222 | 5223 | 5224