CiviCRM Community Forums (archive)

Hi all,

Eileen has written previously about a problem where a smart group B has parent group A and has criteria that refer to group A:

http://forum.civicrm.org/index.php/topic,14248.msg60817.html#msg60817
http://forum.civicrm.org/index.php/topic,22387.msg93917.html#msg93917 & following comments.

This can lead to bottomless recursion involving CRM_Contact_BAO_GroupContactCache::load( ), as follows:

CRM_Contact_BAO_Query->group( ) called for parent group A
-> CRM_Contact_BAO_Query->savedSearch( ) called for parent group A
  -> CRM_Contact_BAO_GroupContactCache::load( ) called for parent group A
    -> CRM_Contact_BAO_Group::getMember( ) called for child smart group B
      -> CRM_Contact_BAO_Query->group( ) called for child smart group B
        -> CRM_Contact_BAO_Query->savedSearch( ) called for child smart group B
          -> CRM_Contact_BAO_GroupContactCache::load( ) called for child smart group B
            -> CRM_Contact_BAO_Query->group( ) called for parent group A (for B's saved search criteria)
=> back to the start until you hit max execution time or memory limit or function nesting limit.

This problem is especially prevalent in multi-org, where every group created, including smart groups, will have a parent group: the site's parent group.

One question is about the semantics of a smart group that has a parent group. I can see two ways in which the parent group could theoretically affect the child smart group...

(1) The child smart group could be automatically restricted to members of the parent group.
However, that's not how smart groups work currently: child smart group B is not limited to members of parent group A. Should it be?

(2) Contacts manually removed from the parent group are automatically treated as removed from the child smart group.

Some possible solutions to the bottomless recursion...

(a) If (1) applied, then it would be superfluous to restrict the search to the parent group in the saved search criteria too, so maybe we could disallow that. But since (1) doesn't apply at the moment, I don't think this is a good option because often you need to restrict the search to the parent group in order to get the desired results.

(b) Don't allow smart groups to have a parent group. This seems OK for a normal Civi installation but is less good for a multi-org, as we need a way to specify which groups (including smart groups) are accessible from a given site. So we'd need a different way to do that.

(c) Limit the recursion by keeping a record in CRM_Contact_BAO_GroupContactCache::load( ) of which groups have been loaded and bail out if we've already loaded the group. Something like:

--- CRM/Contact/BAO/GroupContactCache.php.orig  2011-09-20 08:48:10.000000000 +0100
+++ CRM/Contact/BAO/GroupContactCache.php       2012-04-03 13:26:32.000000000 +0100
@@ -239,9 +239,15 @@
      * load the smart group cache for a saved search
      */
     static function load( &$group ) {
+        static $alreadyLoaded = array();
         $groupID       = $group->id;
         $savedSearchID = $group->saved_search_id;
 
+        if (in_array($groupID, $alreadyLoaded)) {
+            return;
+        }
+        $alreadyLoaded[] = $groupID;
+
         $sql         = null;
         $idName      = 'id';
         $customClass = null;
I've tried this, it successfully halts the recursion and I haven't seen any ill effects so far in early testing on 4.0.8 . The test searches I've done return the same numbers as they do without the patch (and with the smart group de-parented so that the search actually returns). Can anyone see problems with this approach?

Dave J