From a09e50d8d41c0f50c486742f4cc2343083c635e3 Mon Sep 17 00:00:00 2001
From: ludovicp <ludovicp@localhost>
Date: Fri, 25 Jun 2010 09:25:49 +0000
Subject: [PATCH] Fixes issues #4552 #4557, making sure plugins and internal services are properly handling subtree move or delete. The changes particularly resolve problems raised by the community with the referential integrity and the isMemberOf plug-ins. Unit-tests have been updated to cover those cases
---
opends/src/server/org/opends/server/core/GroupManager.java | 220 ++++++++++++++++++++++++++++++++++++++++++------------
1 files changed, 171 insertions(+), 49 deletions(-)
diff --git a/opends/src/server/org/opends/server/core/GroupManager.java b/opends/src/server/org/opends/server/core/GroupManager.java
index 5eddc18..1cf06c1 100644
--- a/opends/src/server/org/opends/server/core/GroupManager.java
+++ b/opends/src/server/org/opends/server/core/GroupManager.java
@@ -22,7 +22,7 @@
* CDDL HEADER END
*
*
- * Copyright 2007-2008 Sun Microsystems, Inc.
+ * Copyright 2007-2010 Sun Microsystems, Inc.
*/
package org.opends.server.core;
@@ -31,6 +31,7 @@
import java.lang.reflect.Method;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.opends.messages.Message;
import org.opends.server.admin.ClassPropertyDefinition;
@@ -44,6 +45,7 @@
import org.opends.server.api.Backend;
import org.opends.server.api.BackendInitializationListener;
import org.opends.server.api.ChangeNotificationListener;
+import org.opends.server.api.DITCacheMap;
import org.opends.server.api.Group;
import org.opends.server.config.ConfigException;
import org.opends.server.loggers.debug.DebugTracer;
@@ -55,6 +57,7 @@
import org.opends.server.types.DebugLogLevel;
import org.opends.server.types.DereferencePolicy;
import org.opends.server.types.DN;
+import org.opends.server.types.DirectoryException;
import org.opends.server.types.Entry;
import org.opends.server.types.InitializationException;
import org.opends.server.types.ResultCode;
@@ -103,7 +106,7 @@
//Used by group instances to determine if new groups have been
//registered or groups deleted.
- private long refreshToken=0;
+ private volatile long refreshToken=0;
// A mapping between the DNs of the config entries and the associated
@@ -112,7 +115,10 @@
// A mapping between the DNs of all group entries and the corresponding
// group instances.
- private ConcurrentHashMap<DN,Group> groupInstances;
+ private DITCacheMap<Group> groupInstances;
+
+ // Lock to protect internal data structures.
+ private final ReentrantReadWriteLock lock;
@@ -122,7 +128,9 @@
public GroupManager()
{
groupImplementations = new ConcurrentHashMap<DN,Group>();
- groupInstances = new ConcurrentHashMap<DN,Group>();
+ groupInstances = new DITCacheMap<Group>();
+
+ lock = new ReentrantReadWriteLock();
DirectoryServer.registerBackendInitializationListener(this);
DirectoryServer.registerChangeNotificationListener(this);
@@ -289,15 +297,23 @@
Group group = groupImplementations.remove(configuration.dn());
if (group != null)
{
- Iterator<Group> iterator = groupInstances.values().iterator();
- while (iterator.hasNext())
+ lock.writeLock().lock();
+ try
{
- Group g = iterator.next();
- if (g.getClass().getName().equals(group.getClass().getName()))
+ Iterator<Group> iterator = groupInstances.values().iterator();
+ while (iterator.hasNext())
{
- iterator.remove();
+ Group g = iterator.next();
+ if (g.getClass().getName().equals(group.getClass().getName()))
+ {
+ iterator.remove();
+ }
}
}
+ finally
+ {
+ lock.writeLock().unlock();
+ }
group.finalizeGroupImplementation();
}
@@ -360,15 +376,23 @@
Group group = groupImplementations.remove(configuration.dn());
if (group != null)
{
- Iterator<Group> iterator = groupInstances.values().iterator();
- while (iterator.hasNext())
+ lock.writeLock().lock();
+ try
{
- Group g = iterator.next();
- if (g.getClass().getName().equals(group.getClass().getName()))
+ Iterator<Group> iterator = groupInstances.values().iterator();
+ while (iterator.hasNext())
{
- iterator.remove();
+ Group g = iterator.next();
+ if (g.getClass().getName().equals(group.getClass().getName()))
+ {
+ iterator.remove();
+ }
}
}
+ finally
+ {
+ lock.writeLock().unlock();
+ }
group.finalizeGroupImplementation();
}
@@ -543,7 +567,18 @@
*/
public Iterable<Group> getGroupInstances()
{
- return groupInstances.values();
+ lock.readLock().lock();
+ try
+ {
+ // Return a copy to protect from structural changes.
+ ArrayList<Group> values = new ArrayList<Group>();
+ values.addAll(groupInstances.values());
+ return values;
+ }
+ finally
+ {
+ lock.readLock().unlock();
+ }
}
@@ -559,7 +594,18 @@
*/
public Group getGroupInstance(DN entryDN)
{
- Group group = groupInstances.get(entryDN);
+ Group group = null;
+
+ lock.readLock().lock();
+ try
+ {
+ group = groupInstances.get(entryDN);
+ }
+ finally
+ {
+ lock.readLock().unlock();
+ }
+
if (group == null)
{
// FIXME -- Should we try to retrieve the corresponding entry and see if
@@ -654,25 +700,33 @@
continue;
}
- for (SearchResultEntry entry : internalSearch.getSearchEntries())
+ lock.writeLock().lock();
+ try
{
- try
+ for (SearchResultEntry entry : internalSearch.getSearchEntries())
{
- Group groupInstance = groupImplementation.newInstance(entry);
- groupInstances.put(entry.getDN(), groupInstance);
- refreshToken++;
- }
- catch (Exception e)
- {
- if (debugEnabled())
+ try
{
- TRACER.debugCaught(DebugLogLevel.ERROR, e);
+ Group groupInstance = groupImplementation.newInstance(entry);
+ groupInstances.put(entry.getDN(), groupInstance);
+ refreshToken++;
}
+ catch (Exception e)
+ {
+ if (debugEnabled())
+ {
+ TRACER.debugCaught(DebugLogLevel.ERROR, e);
+ }
- // FIXME -- Handle this.
- continue;
+ // FIXME -- Handle this.
+ continue;
+ }
}
}
+ finally
+ {
+ lock.writeLock().unlock();
+ }
}
}
}
@@ -685,17 +739,25 @@
*/
public void performBackendFinalizationProcessing(Backend backend)
{
- Iterator<Map.Entry<DN,Group>> iterator =
- groupInstances.entrySet().iterator();
- while (iterator.hasNext())
+ lock.writeLock().lock();
+ try
{
- Map.Entry<DN,Group> mapEntry = iterator.next();
- DN groupEntryDN = mapEntry.getKey();
- if (backend.handlesEntry(groupEntryDN))
+ Iterator<Map.Entry<DN,Group>> iterator =
+ groupInstances.entrySet().iterator();
+ while (iterator.hasNext())
{
- iterator.remove();
+ Map.Entry<DN,Group> mapEntry = iterator.next();
+ DN groupEntryDN = mapEntry.getKey();
+ if (backend.handlesEntry(groupEntryDN))
+ {
+ iterator.remove();
+ }
}
}
+ finally
+ {
+ lock.writeLock().unlock();
+ }
}
@@ -719,11 +781,16 @@
}
}
}
- synchronized (groupInstances)
+ lock.writeLock().lock();
+ try
{
createAndRegisterGroup(entry);
refreshToken++;
}
+ finally
+ {
+ lock.writeLock().unlock();
+ }
}
@@ -746,11 +813,16 @@
}
}
}
- synchronized (groupInstances)
+ lock.writeLock().lock();
+ try
{
- groupInstances.remove(entry.getDN());
+ groupInstances.removeSubtree(entry.getDN(), null);
refreshToken++;
}
+ finally
+ {
+ lock.writeLock().unlock();
+ }
}
@@ -775,21 +847,24 @@
}
}
-
- if (groupInstances.containsKey(oldEntry.getDN()))
+ lock.writeLock().lock();
+ try
{
- synchronized (groupInstances)
+ if (groupInstances.containsKey(oldEntry.getDN()))
{
if (! oldEntry.getDN().equals(newEntry.getDN()))
{
// This should never happen, but check for it anyway.
groupInstances.remove(oldEntry.getDN());
}
-
createAndRegisterGroup(newEntry);
refreshToken++;
}
}
+ finally
+ {
+ lock.writeLock().unlock();
+ }
}
@@ -816,14 +891,44 @@
}
}
- if (groupInstances.containsKey(oldEntry.getDN()))
+ lock.writeLock().lock();
+ try
{
- synchronized (groupInstances)
+ Set<Group> groupSet = new HashSet<Group>();
+ groupInstances.removeSubtree(oldEntry.getDN(), groupSet);
+ String oldDNString = oldEntry.getDN().toNormalizedString();
+ String newDNString = newEntry.getDN().toNormalizedString();
+ for (Group group : groupSet)
{
- createAndRegisterGroup(newEntry);
- groupInstances.remove(oldEntry.getDN());
- refreshToken++;
+ StringBuilder builder = new StringBuilder(
+ group.getGroupDN().toNormalizedString());
+ int oldDNIndex = builder.lastIndexOf(oldDNString);
+ builder.replace(oldDNIndex, builder.length(),
+ newDNString);
+ String groupDNString = builder.toString();
+ DN groupDN = DN.NULL_DN;
+ try
+ {
+ groupDN = DN.decode(groupDNString);
+ }
+ catch (DirectoryException de)
+ {
+ // Should not happen but if it does all we
+ // can do here is debug log it and continue.
+ if (debugEnabled())
+ {
+ TRACER.debugCaught(DebugLogLevel.ERROR, de);
+ }
+ continue;
+ }
+ group.setGroupDN(groupDN);
+ groupInstances.put(groupDN, group);
}
+ refreshToken++;
+ }
+ finally
+ {
+ lock.writeLock().unlock();
}
}
@@ -845,7 +950,16 @@
if (groupImplementation.isGroupDefinition(entry))
{
Group groupInstance = groupImplementation.newInstance(entry);
- groupInstances.put(entry.getDN(), groupInstance);
+
+ lock.writeLock().lock();
+ try
+ {
+ groupInstances.put(entry.getDN(), groupInstance);
+ }
+ finally
+ {
+ lock.writeLock().unlock();
+ }
}
}
catch (Exception e)
@@ -869,7 +983,15 @@
*/
void deregisterAllGroups()
{
- groupInstances.clear();
+ lock.writeLock().lock();
+ try
+ {
+ groupInstances.clear();
+ }
+ finally
+ {
+ lock.writeLock().unlock();
+ }
}
--
Gitblit v1.10.0