From a381819e25430ccca6f3d5645d7422d1dda97c1c Mon Sep 17 00:00:00 2001
From: Nicolas Capponi <nicolas.capponi@forgerock.com>
Date: Fri, 17 Jun 2016 09:54:07 +0000
Subject: [PATCH] OPENDJ-2655 Optimize Group implementations by lazily creating examined groups

---
 opendj-server-legacy/src/main/java/org/opends/server/api/Group.java                     |   10 ++--
 opendj-server-legacy/src/main/java/org/opends/server/extensions/VirtualStaticGroup.java |   23 +++++++++--
 opendj-server-legacy/src/main/java/org/opends/server/extensions/DynamicGroup.java       |   23 +++++++++--
 opendj-server-legacy/src/main/java/org/opends/server/extensions/StaticGroup.java        |   39 +++++++++++++------
 4 files changed, 70 insertions(+), 25 deletions(-)

diff --git a/opendj-server-legacy/src/main/java/org/opends/server/api/Group.java b/opendj-server-legacy/src/main/java/org/opends/server/api/Group.java
index 5dd385f..ecb7e1f 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/api/Group.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/api/Group.java
@@ -18,6 +18,7 @@
 
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
 
 import org.forgerock.i18n.LocalizableMessage;
 import org.forgerock.opendj.config.server.ConfigException;
@@ -263,7 +264,7 @@
    */
   public boolean isMember(DN userDN) throws DirectoryException
   {
-    return userDN != null && isMember(userDN, new HashSet<DN>());
+    return userDN != null && isMember(userDN, new AtomicReference<Set<DN>>());
   }
 
   /**
@@ -297,7 +298,7 @@
    * @throws  DirectoryException  If a problem occurs while attempting
    *                              to make the determination.
    */
-  public abstract boolean isMember(DN userDN, Set<DN> examinedGroups)
+  public abstract boolean isMember(DN userDN, AtomicReference<Set<DN>> examinedGroups)
          throws DirectoryException;
 
   /**
@@ -317,7 +318,7 @@
   public boolean isMember(Entry userEntry)
          throws DirectoryException
   {
-    return isMember(userEntry, new HashSet<DN>());
+    return isMember(userEntry, new AtomicReference<Set<DN>>());
   }
 
   /**
@@ -351,8 +352,7 @@
    * @throws  DirectoryException  If a problem occurs while attempting
    *                              to make the determination.
    */
-  public abstract boolean isMember(Entry userEntry,
-                                   Set<DN> examinedGroups)
+  public abstract boolean isMember(Entry userEntry, AtomicReference<Set<DN>> examinedGroups)
          throws DirectoryException;
 
   /**
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/extensions/DynamicGroup.java b/opendj-server-legacy/src/main/java/org/opends/server/extensions/DynamicGroup.java
index ebe3b84..30243d9 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/extensions/DynamicGroup.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/extensions/DynamicGroup.java
@@ -17,10 +17,12 @@
 package org.opends.server.extensions;
 
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
 
 import org.forgerock.i18n.LocalizableMessage;
 import org.forgerock.i18n.LocalizedIllegalArgumentException;
@@ -206,10 +208,11 @@
   }
 
   @Override
-  public boolean isMember(DN userDN, Set<DN> examinedGroups)
+  public boolean isMember(DN userDN, AtomicReference<Set<DN>> examinedGroups)
          throws DirectoryException
   {
-    if (! examinedGroups.add(getGroupDN()))
+    Set<DN> groups = getExaminedGroups(examinedGroups);
+    if (! groups.add(getGroupDN()))
     {
       return false;
     }
@@ -219,10 +222,11 @@
   }
 
   @Override
-  public boolean isMember(Entry userEntry, Set<DN> examinedGroups)
+  public boolean isMember(Entry userEntry, AtomicReference<Set<DN>> examinedGroups)
          throws DirectoryException
   {
-    if (! examinedGroups.add(getGroupDN()))
+    Set<DN> groups = getExaminedGroups(examinedGroups);
+    if (! groups.add(getGroupDN()))
     {
       return false;
     }
@@ -238,6 +242,17 @@
     return false;
   }
 
+  private Set<DN> getExaminedGroups(AtomicReference<Set<DN>> examinedGroups)
+  {
+    Set<DN> groups = examinedGroups.get();
+    if (groups == null)
+    {
+      groups = new HashSet<DN>();
+      examinedGroups.set(groups);
+    }
+    return groups;
+  }
+
   @Override
   public MemberList getMembers()
          throws DirectoryException
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/extensions/StaticGroup.java b/opendj-server-legacy/src/main/java/org/opends/server/extensions/StaticGroup.java
index fbd0a53..e0d14ed 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/extensions/StaticGroup.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/extensions/StaticGroup.java
@@ -26,6 +26,7 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
@@ -387,7 +388,7 @@
   }
 
   @Override
-  public boolean isMember(DN userDN, Set<DN> examinedGroups) throws DirectoryException
+  public boolean isMember(DN userDN, AtomicReference<Set<DN>> examinedGroups) throws DirectoryException
   {
     reloadIfNeeded();
     CompactDn compactUserDN = new CompactDn(userDN);
@@ -398,19 +399,22 @@
       {
         return true;
       }
-      else if (!examinedGroups.add(getGroupDN()))
+      if (nestedGroups.isEmpty()) {
+        return false;
+      }
+
+      // there are nested groups
+      Set<DN> groups = getExaminedGroups(examinedGroups);
+      if (!groups.add(getGroupDN()))
       {
         return false;
       }
-      else
+      for (DN nestedGroupDN : nestedGroups)
       {
-        for (DN nestedGroupDN : nestedGroups)
+        Group<? extends GroupImplementationCfg> group = getGroupManager().getGroupInstance(nestedGroupDN);
+        if (group != null && group.isMember(userDN, examinedGroups))
         {
-          Group<? extends GroupImplementationCfg> group = getGroupManager().getGroupInstance(nestedGroupDN);
-          if (group != null && group.isMember(userDN, examinedGroups))
-          {
-            return true;
-          }
+          return true;
         }
       }
     }
@@ -421,8 +425,19 @@
     return false;
   }
 
+  private Set<DN> getExaminedGroups(AtomicReference<Set<DN>> examinedGroups)
+  {
+    Set<DN> groups = examinedGroups.get();
+    if (groups == null)
+    {
+      groups = new HashSet<DN>();
+      examinedGroups.set(groups);
+    }
+    return groups;
+  }
+
   @Override
-  public boolean isMember(Entry userEntry, Set<DN> examinedGroups)
+  public boolean isMember(Entry userEntry, AtomicReference<Set<DN>> examinedGroups)
          throws DirectoryException
   {
     return isMember(userEntry.getName(), examinedGroups);
@@ -445,8 +460,8 @@
         // Check if the group itself has been removed
         if (thisGroup == null)
         {
-          throw new DirectoryException(ResultCode.NO_SUCH_ATTRIBUTE, ERR_STATICGROUP_GROUP_INSTANCE_INVALID
-              .get(groupEntryDN));
+          throw new DirectoryException(ResultCode.NO_SUCH_ATTRIBUTE,
+              ERR_STATICGROUP_GROUP_INSTANCE_INVALID.get(groupEntryDN));
         }
         else if (thisGroup != this)
         {
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/extensions/VirtualStaticGroup.java b/opendj-server-legacy/src/main/java/org/opends/server/extensions/VirtualStaticGroup.java
index fee8a8d..a66e621 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/extensions/VirtualStaticGroup.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/extensions/VirtualStaticGroup.java
@@ -17,8 +17,10 @@
 package org.opends.server.extensions;
 
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
 
 import org.forgerock.i18n.LocalizableMessage;
 import org.forgerock.i18n.LocalizedIllegalArgumentException;
@@ -216,10 +218,11 @@
   }
 
   @Override
-  public boolean isMember(DN userDN, Set<DN> examinedGroups)
+  public boolean isMember(DN userDN, AtomicReference<Set<DN>> examinedGroups)
          throws DirectoryException
   {
-    if (! examinedGroups.add(getGroupDN()))
+    Set<DN> groups = getExaminedGroups(examinedGroups);
+    if (! groups.add(getGroupDN()))
     {
       return false;
     }
@@ -243,10 +246,11 @@
   }
 
   @Override
-  public boolean isMember(Entry userEntry, Set<DN> examinedGroups)
+  public boolean isMember(Entry userEntry, AtomicReference<Set<DN>> examinedGroups)
          throws DirectoryException
   {
-    if (! examinedGroups.add(getGroupDN()))
+    Set<DN> groups = getExaminedGroups(examinedGroups);
+    if (! groups.add(getGroupDN()))
     {
       return false;
     }
@@ -269,6 +273,17 @@
     }
   }
 
+  private Set<DN> getExaminedGroups(AtomicReference<Set<DN>> examinedGroups)
+  {
+    Set<DN> groups = examinedGroups.get();
+    if (groups == null)
+    {
+      groups = new HashSet<DN>();
+      examinedGroups.set(groups);
+    }
+    return groups;
+  }
+
   @Override
   public MemberList getMembers()
          throws DirectoryException

--
Gitblit v1.10.0