speed up indirect member processing

the old implementation tried to get all entries which are member of group.
That means also user. User can't have any members therefore this costly
processing was unnecessary.

New implementation reduces the search only to entries which have members.

Also page size was removed to avoid paging by small pages(default size: 100)
which is very slow for many members.

https://fedorahosted.org/freeipa/ticket/4947

Reviewed-By: Jan Cholasta <jcholast@redhat.com>

6 files changed, 81 insertions, 95 deletions

diff --git a/install/updates/20-indices.update b/install/updates/20-indices.update
index a8a432d9c..a9ec9f9eb 100644
--- a/install/updates/20-indices.update
+++ b/install/updates/20-indices.update
@@ -27,7 +27,7 @@ default:nsSystemIndex: false
 only:nsIndexType: eq,pres,sub
 
 dn: cn=member,cn=index,cn=userRoot,cn=ldbm database,cn=plugins,cn=config
-only:nsIndexType: eq,sub
+only:nsIndexType: eq,pres,sub
 
 dn: cn=uniquemember,cn=index,cn=userRoot,cn=ldbm database,cn=plugins,cn=config
 only:nsIndexType: eq,sub
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index ca4e54fd2..b06b570cb 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -663,6 +663,67 @@ class LDAPObject(Object):
                         new_attr.append(new_value)
                         break
 
+    def get_indirect_members(self, entry_attrs, attrs_list):
+        if 'memberindirect' in attrs_list:
+            self.get_memberindirect(entry_attrs)
+        if 'memberofindirect' in attrs_list:
+            self.get_memberofindirect(entry_attrs)
+
+    def get_memberindirect(self, group_entry):
+        """
+        Get indirect members
+        """
+
+        mo_filter = self.backend.make_filter({'memberof': group_entry.dn})
+        filter = self.backend.combine_filters(
+            ('(member=*)', mo_filter), self.backend.MATCH_ALL)
+        try:
+            result, truncated = self.backend.find_entries(
+                base_dn=self.api.env.basedn,
+                filter=filter,
+                attrs_list=['member'],
+                size_limit=-1, # paged search will get everything anyway
+                paged_search=True)
+            if truncated:
+                raise errors.LimitsExceeded()
+        except errors.NotFound:
+            result = []
+
+        indirect = set()
+        for entry in result:
+            indirect.update(entry.raw.get('member', []))
+        indirect.difference_update(group_entry.raw.get('member', []))
+
+        if indirect:
+            group_entry.raw['memberindirect'] = list(indirect)
+
+    def get_memberofindirect(self, entry):
+
+        dn = entry.dn
+        filter = self.backend.make_filter(
+            {'member': dn, 'memberuser': dn, 'memberhost': dn})
+        try:
+            result, truncated = self.backend.find_entries(
+                base_dn=self.api.env.basedn,
+                filter=filter,
+                attrs_list=[''])
+            if truncated:
+                raise errors.LimitsExceeded()
+        except errors.NotFound:
+            result = []
+
+        direct = set()
+        indirect = set(entry.raw.get('memberof', []))
+        for group_entry in result:
+            dn = str(group_entry.dn)
+            if dn in indirect:
+                indirect.remove(dn)
+                direct.add(dn)
+
+        entry.raw['memberof'] = list(direct)
+        if indirect:
+            entry.raw['memberofindirect'] = list(indirect)
+
     def get_password_attributes(self, ldap, dn, entry_attrs):
         """
         Search on the entry to determine if it has a password or
@@ -1205,6 +1266,8 @@ class LDAPCreate(BaseLDAPCommand, crud.Create):
         except errors.NotFound:
             self.obj.handle_not_found(*keys)
 
+        self.obj.get_indirect_members(entry_attrs, attrs_list)
+
         for callback in self.get_callbacks('post'):
             entry_attrs.dn = callback(
                 self, ldap, entry_attrs.dn, entry_attrs, *keys, **options)
@@ -1328,6 +1391,8 @@ class LDAPRetrieve(LDAPQuery):
         except errors.NotFound:
             self.obj.handle_not_found(*keys)
 
+        self.obj.get_indirect_members(entry_attrs, attrs_list)
+
         if options.get('rights', False) and options.get('all', False):
             entry_attrs['attributelevelrights'] = get_effective_rights(
                 ldap, entry_attrs.dn)
@@ -1478,6 +1543,8 @@ class LDAPUpdate(LDAPQuery, crud.Update):
                 format=_('the entry was deleted while being modified')
             )
 
+        self.obj.get_indirect_members(entry_attrs, attrs_list)
+
         if options.get('rights', False) and options.get('all', False):
             entry_attrs['attributelevelrights'] = get_effective_rights(
                 ldap, entry_attrs.dn)
@@ -1712,6 +1779,8 @@ class LDAPAddMember(LDAPModMember):
         except errors.NotFound:
             self.obj.handle_not_found(*keys)
 
+        self.obj.get_indirect_members(entry_attrs, attrs_list)
+
         for callback in self.get_callbacks('post'):
             (completed, entry_attrs.dn) = callback(
                 self, ldap, completed, failed, entry_attrs.dn, entry_attrs,
@@ -1814,6 +1883,8 @@ class LDAPRemoveMember(LDAPModMember):
         except errors.NotFound:
             self.obj.handle_not_found(*keys)
 
+        self.obj.get_indirect_members(entry_attrs, attrs_list)
+
         for callback in self.get_callbacks('post'):
             (completed, entry_attrs.dn) = callback(
                 self, ldap, completed, failed, entry_attrs.dn, entry_attrs,
@@ -2034,6 +2105,7 @@ class LDAPSearch(BaseLDAPCommand, crud.Search):
 
         if not options.get('raw', False):
             for e in entries:
+                self.obj.get_indirect_members(e, attrs_list)
                 self.obj.convert_attribute_members(e, *args, **options)
 
         for (i, e) in enumerate(entries):
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 41710f3b8..c47439743 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -296,7 +296,7 @@ class host(LDAPObject):
     default_attributes = [
         'fqdn', 'description', 'l', 'nshostlocation', 'krbprincipalname',
         'nshardwareplatform', 'nsosversion', 'usercertificate', 'memberof',
-        'managedby', 'memberindirect', 'memberofindirect', 'macaddress',
+        'managedby', 'memberofindirect', 'macaddress',
         'userclass', 'ipaallowedtoperform', 'ipaassignedidview',
     ]
     uuid_attribute = 'ipauniqueid'
diff --git a/ipalib/plugins/role.py b/ipalib/plugins/role.py
index 55afece22..6d8d544aa 100644
--- a/ipalib/plugins/role.py
+++ b/ipalib/plugins/role.py
@@ -71,9 +71,11 @@ class role(LDAPObject):
     object_name_plural = _('roles')
     object_class = ['groupofnames', 'nestedgroup']
     permission_filter_objectclasses = ['groupofnames']
-    default_attributes = ['cn', 'description', 'member', 'memberof',
-        'memberindirect', 'memberofindirect',
-    ]
+    default_attributes = ['cn', 'description', 'member', 'memberof']
+    # Role could have a lot of indirect members, but they are not in
+    # attribute_members therefore they don't have to be in default_attributes
+    # 'memberindirect', 'memberofindirect',
+
     attribute_members = {
         'member': ['user', 'group', 'host', 'hostgroup', 'service'],
         'memberof': ['privilege'],
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 7cda7d67d..75ff2177b 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -665,6 +665,8 @@ class LDAPClient(object):
     _SYNTAX_OVERRIDE = CIDict({
         'managedtemplate': DN,
         'managedbase':     DN,
+        'memberindirect':  DN,
+        'memberofindirect':DN,
         'originscope':     DN,
         'idnsname':        DNSName,
         'idnssoamname':    DNSName,
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 15e07f27b..d1d966c59 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -220,102 +220,12 @@ class ldap2(LDAPClient, CrudBackend):
         if size_limit is None:
             size_limit = _get_limits()['size']
 
-        has_memberindirect = False
-        has_memberofindirect = False
-        if attrs_list:
-            new_attrs_list = []
-            for attr_name in attrs_list:
-                if attr_name == 'memberindirect':
-                    has_memberindirect = True
-                elif attr_name == 'memberofindirect':
-                    has_memberofindirect = True
-                else:
-                    new_attrs_list.append(attr_name)
-            attrs_list = new_attrs_list
-
         res, truncated = super(ldap2, self).find_entries(
             filter=filter, attrs_list=attrs_list, base_dn=base_dn, scope=scope,
             time_limit=time_limit, size_limit=size_limit,
             search_refs=search_refs, paged_search=paged_search)
-
-        if has_memberindirect or has_memberofindirect:
-
-            # For the memberof searches, we want to apply the global limit
-            # if it's larger than the requested one, so decreasing limits on
-            # the individual query only affects the query itself.
-            # See https://fedorahosted.org/freeipa/ticket/4398
-            def _max_with_none(a, b):
-                """Maximum of a and b, treating None as infinity"""
-                if a is None or b is None:
-                    return None
-                else:
-                    return max(a, b)
-            time_limit = _max_with_none(time_limit, _get_limits()['time'])
-            size_limit = _max_with_none(size_limit, _get_limits()['size'])
-
-            for entry in res:
-                if has_memberindirect:
-                    self._process_memberindirect(
-                        entry, time_limit=time_limit, size_limit=size_limit)
-                if has_memberofindirect:
-                    self._process_memberofindirect(
-                        entry, time_limit=time_limit, size_limit=size_limit)
-
         return (res, truncated)
 
-    def _process_memberindirect(self, group_entry, time_limit=None,
-                                size_limit=None):
-        filter = self.make_filter({'memberof': group_entry.dn})
-        try:
-            result, truncated = self.find_entries(
-                base_dn=self.api.env.basedn,
-                filter=filter,
-                attrs_list=['member'],
-                time_limit=time_limit,
-                size_limit=size_limit,
-                paged_search=True)
-            if truncated:
-                raise errors.LimitsExceeded()
-        except errors.NotFound:
-            result = []
-
-        indirect = set()
-        for entry in result:
-            indirect.update(entry.get('member', []))
-        indirect.difference_update(group_entry.get('member', []))
-
-        if indirect:
-            group_entry['memberindirect'] = list(indirect)
-
-    def _process_memberofindirect(self, entry, time_limit=None,
-                                  size_limit=None):
-        dn = entry.dn
-        filter = self.make_filter(
-            {'member': dn, 'memberuser': dn, 'memberhost': dn})
-        try:
-            result, truncated = self.find_entries(
-                base_dn=self.api.env.basedn,
-                filter=filter,
-                attrs_list=[''],
-                time_limit=time_limit,
-                size_limit=size_limit)
-            if truncated:
-                raise errors.LimitsExceeded()
-        except errors.NotFound:
-            result = []
-
-        direct = set()
-        indirect = set(entry.get('memberof', []))
-        for group_entry in result:
-            dn = group_entry.dn
-            if dn in indirect:
-                indirect.remove(dn)
-                direct.add(dn)
-
-        entry['memberof'] = list(direct)
-        if indirect:
-            entry['memberofindirect'] = list(indirect)
-
     config_defaults = {'ipasearchtimelimit': [2], 'ipasearchrecordslimit': [0]}
     def get_ipa_config(self, attrs_list=None):
         """Returns the IPA configuration entry (dn, entry_attrs)."""