From 98a5df3565beaa1999020a16fcb5338d13d5b50f Mon Sep 17 00:00:00 2001
From: Jean-Noel Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Tue, 26 Aug 2014 08:11:29 +0000
Subject: [PATCH] Removed useless getChangeNumber() and setChangeNumber methods from *Operation interfaces. These methods are useless because setChangeNumber() is never set inside production code, so changeNumber is always equal to -1 for all the update operations. In addition it is very unlikely the ChangeNumberIndexer could have computed the changeNumber before a results are sent to a persistent search.

---
 opends/src/server/org/opends/server/core/PersistentSearch.java |  512 +++++++++++---------------------------------------------
 1 files changed, 102 insertions(+), 410 deletions(-)

diff --git a/opends/src/server/org/opends/server/core/PersistentSearch.java b/opends/src/server/org/opends/server/core/PersistentSearch.java
index 3193abb..41b7d22 100644
--- a/opends/src/server/org/opends/server/core/PersistentSearch.java
+++ b/opends/src/server/org/opends/server/core/PersistentSearch.java
@@ -22,14 +22,11 @@
  *
  *
  *      Copyright 2006-2010 Sun Microsystems, Inc.
+ *      Portions Copyright 2014 ForgeRock AS
  */
 package org.opends.server.core;
 
-
-
-import static org.opends.server.loggers.debug.DebugLogger.*;
-
-import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -44,10 +41,9 @@
 import org.opends.server.types.DirectoryException;
 import org.opends.server.types.Entry;
 import org.opends.server.types.ResultCode;
-import org.opends.server.types.SearchFilter;
-import org.opends.server.types.SearchScope;
 
-
+import static org.opends.server.controls.PersistentSearchChangeType.*;
+import static org.opends.server.loggers.debug.DebugLogger.*;
 
 /**
  * This class defines a data structure that will be used to hold the
@@ -109,7 +105,7 @@
 
 
 
-  // Cancel a persistent search.
+  /** Cancel a persistent search. */
   private static synchronized void cancel(PersistentSearch psearch)
   {
     if (!psearch.isCancelled)
@@ -141,32 +137,28 @@
     }
   }
 
-  // The base DN for the search operation.
-  private final DN baseDN;
-
-  // Cancellation callbacks which should be run when this persistent
-  // search is cancelled.
+  /**
+   * Cancellation callbacks which should be run when this persistent search is
+   * cancelled.
+   */
   private final List<CancellationCallback> cancellationCallbacks =
     new CopyOnWriteArrayList<CancellationCallback>();
 
-  // The set of change types we want to see.
+  /** The set of change types to send to the client. */
   private final Set<PersistentSearchChangeType> changeTypes;
 
-  // The filter for the search operation.
-  private final SearchFilter filter;
+  /**
+   * Indicates whether or not this persistent search has already been aborted.
+   */
+  private boolean isCancelled;
 
-  // Indicates whether or not this persistent search has already been
-  // aborted.
-  private boolean isCancelled = false;
-
-  // Indicates whether entries returned should include the entry
-  // change notification control.
+  /**
+   * Indicates whether entries returned should include the entry change
+   * notification control.
+   */
   private final boolean returnECs;
 
-  // The scope for the search operation.
-  private final SearchScope scope;
-
-  // The reference to the associated search operation.
+  /** The reference to the associated search operation. */
   private final SearchOperation searchOperation;
 
 
@@ -189,10 +181,6 @@
     this.searchOperation = searchOperation;
     this.changeTypes = changeTypes;
     this.returnECs = returnECs;
-
-    this.baseDN = searchOperation.getBaseDN();
-    this.scope = searchOperation.getScope();
-    this.filter = searchOperation.getFilter();
   }
 
 
@@ -257,60 +245,45 @@
    *
    * @param entry
    *          The entry that was added.
-   * @param changeNumber
-   *          The change number associated with the operation that
-   *          added the entry, or {@code -1} if there is no change
-   *          number.
    */
-  public void processAdd(Entry entry, long changeNumber)
+  public void processAdd(Entry entry)
   {
-    // See if we care about add operations.
-    if (!changeTypes.contains(PersistentSearchChangeType.ADD))
+    if (changeTypes.contains(ADD)
+        && isInScope(entry.getDN())
+        && matchesFilter(entry))
     {
-      return;
+      sendEntry(entry, createControls(ADD, null));
     }
+  }
 
-    // Make sure that the entry is within our target scope.
-    switch (scope)
+  private boolean isInScope(final DN dn)
+  {
+    final DN baseDN = searchOperation.getBaseDN();
+    switch (searchOperation.getScope())
     {
     case BASE_OBJECT:
-      if (!baseDN.equals(entry.getDN()))
-      {
-        return;
-      }
-      break;
+      return baseDN.equals(dn);
     case SINGLE_LEVEL:
-      if (!baseDN.equals(entry.getDN().getParentDNInSuffix()))
-      {
-        return;
-      }
-      break;
+      return baseDN.equals(dn.getParentDNInSuffix());
     case WHOLE_SUBTREE:
-      if (!baseDN.isAncestorOf(entry.getDN()))
-      {
-        return;
-      }
-      break;
+      return baseDN.isAncestorOf(dn);
     case SUBORDINATE_SUBTREE:
-      if (baseDN.equals(entry.getDN()) || (!baseDN.isAncestorOf(entry.getDN())))
-      {
-        return;
-      }
-      break;
+      return !baseDN.equals(dn) && baseDN.isAncestorOf(dn);
     default:
-      return;
+      return false;
     }
+  }
 
-    // Make sure that the entry matches the target filter.
+  private boolean matchesFilter(Entry entry)
+  {
     try
     {
-      TRACER.debugInfo(this + " " + entry + " +filter="
-          + filter.matchesEntry(entry));
-
-      if (!filter.matchesEntry(entry))
+      final boolean filterMatchesEntry = searchOperation.getFilter().matchesEntry(entry);
+      if (debugEnabled())
       {
-        return;
+        TRACER.debugInfo(this + " " + entry + " filter=" + filterMatchesEntry);
       }
+      return filterMatchesEntry;
     }
     catch (DirectoryException de)
     {
@@ -320,162 +293,23 @@
       }
 
       // FIXME -- Do we need to do anything here?
-
-      return;
-    }
-
-    // The entry is one that should be sent to the client. See if we
-    // also need to construct an entry change notification control.
-    ArrayList<Control> entryControls = new ArrayList<Control>(1);
-    if (returnECs)
-    {
-      entryControls.add(new EntryChangeNotificationControl(
-          PersistentSearchChangeType.ADD, changeNumber));
-    }
-
-    // Send the entry and see if we should continue processing. If
-    // not, then deregister this persistent search.
-    try
-    {
-      if (!searchOperation.returnEntry(entry, entryControls))
-      {
-        cancel();
-        searchOperation.sendSearchResultDone();
-      }
-    }
-    catch (Exception e)
-    {
-      if (debugEnabled())
-      {
-        TRACER.debugCaught(DebugLogLevel.ERROR, e);
-      }
-
-      cancel();
-
-      try
-      {
-        searchOperation.sendSearchResultDone();
-      }
-      catch (Exception e2)
-      {
-        if (debugEnabled())
-        {
-          TRACER.debugCaught(DebugLogLevel.ERROR, e2);
-        }
-      }
+      return false;
     }
   }
 
-
-
   /**
    * Notifies the persistent searches that an entry has been deleted.
    *
    * @param entry
    *          The entry that was deleted.
-   * @param changeNumber
-   *          The change number associated with the operation that
-   *          deleted the entry, or {@code -1} if there is no change
-   *          number.
    */
-  public void processDelete(Entry entry, long changeNumber)
+  public void processDelete(Entry entry)
   {
-    // See if we care about delete operations.
-    if (!changeTypes.contains(PersistentSearchChangeType.DELETE))
+    if (changeTypes.contains(DELETE)
+        && isInScope(entry.getDN())
+        && matchesFilter(entry))
     {
-      return;
-    }
-
-    // Make sure that the entry is within our target scope.
-    switch (scope)
-    {
-    case BASE_OBJECT:
-      if (!baseDN.equals(entry.getDN()))
-      {
-        return;
-      }
-      break;
-    case SINGLE_LEVEL:
-      if (!baseDN.equals(entry.getDN().getParentDNInSuffix()))
-      {
-        return;
-      }
-      break;
-    case WHOLE_SUBTREE:
-      if (!baseDN.isAncestorOf(entry.getDN()))
-      {
-        return;
-      }
-      break;
-    case SUBORDINATE_SUBTREE:
-      if (baseDN.equals(entry.getDN()) || (!baseDN.isAncestorOf(entry.getDN())))
-      {
-        return;
-      }
-      break;
-    default:
-      return;
-    }
-
-    // Make sure that the entry matches the target filter.
-    try
-    {
-      if (!filter.matchesEntry(entry))
-      {
-        return;
-      }
-    }
-    catch (DirectoryException de)
-    {
-      if (debugEnabled())
-      {
-        TRACER.debugCaught(DebugLogLevel.ERROR, de);
-      }
-
-      // FIXME -- Do we need to do anything here?
-
-      return;
-    }
-
-    // The entry is one that should be sent to the client. See if we
-    // also need to construct an entry change notification control.
-    ArrayList<Control> entryControls = new ArrayList<Control>(1);
-    if (returnECs)
-    {
-      entryControls.add(new EntryChangeNotificationControl(
-          PersistentSearchChangeType.DELETE, changeNumber));
-    }
-
-    // Send the entry and see if we should continue processing. If
-    // not, then deregister this persistent search.
-    try
-    {
-      if (!searchOperation.returnEntry(entry, entryControls))
-      {
-        cancel();
-        searchOperation.sendSearchResultDone();
-      }
-    }
-    catch (Exception e)
-    {
-      if (debugEnabled())
-      {
-        TRACER.debugCaught(DebugLogLevel.ERROR, e);
-      }
-
-      cancel();
-
-      try
-      {
-        searchOperation.sendSearchResultDone();
-      }
-      catch (Exception e2)
-      {
-        if (debugEnabled())
-        {
-          TRACER.debugCaught(DebugLogLevel.ERROR, e2);
-        }
-      }
+      sendEntry(entry, createControls(DELETE, null));
     }
   }
 
@@ -486,14 +320,10 @@
    *
    * @param entry
    *          The entry after it was modified.
-   * @param changeNumber
-   *          The change number associated with the operation that
-   *          modified the entry, or {@code -1} if there is no change
-   *          number.
    */
-  public void processModify(Entry entry, long changeNumber)
+  public void processModify(Entry entry)
   {
-    processModify(entry, changeNumber, entry);
+    processModify(entry, entry);
   }
 
 
@@ -503,222 +333,84 @@
    *
    * @param entry
    *          The entry after it was modified.
-   * @param changeNumber
-   *          The change number associated with the operation that
-   *          modified the entry, or {@code -1} if there is no change
-   *          number.
    * @param oldEntry
    *          The entry before it was modified.
    */
-  public void processModify(Entry entry, long changeNumber, Entry oldEntry)
+  public void processModify(Entry entry, Entry oldEntry)
   {
-    // See if we care about modify operations.
-    if (!changeTypes.contains(PersistentSearchChangeType.MODIFY))
+    if (changeTypes.contains(MODIFY)
+        && isInScopeForModify(oldEntry.getDN())
+        && anyMatchesFilter(entry, oldEntry))
     {
-      return;
-    }
-
-    // Make sure that the entry is within our target scope.
-    switch (scope)
-    {
-    case BASE_OBJECT:
-      if (!baseDN.equals(oldEntry.getDN()))
-      {
-        return;
-      }
-      break;
-    case SINGLE_LEVEL:
-      if (!baseDN.equals(oldEntry.getDN().getParent()))
-      {
-        return;
-      }
-      break;
-    case WHOLE_SUBTREE:
-      if (!baseDN.isAncestorOf(oldEntry.getDN()))
-      {
-        return;
-      }
-      break;
-    case SUBORDINATE_SUBTREE:
-      if (baseDN.equals(oldEntry.getDN())
-          || (!baseDN.isAncestorOf(oldEntry.getDN())))
-      {
-        return;
-      }
-      break;
-    default:
-      return;
-    }
-
-    // Make sure that the entry matches the target filter.
-    try
-    {
-      if ((!filter.matchesEntry(oldEntry)) && (!filter.matchesEntry(entry)))
-      {
-        return;
-      }
-    }
-    catch (DirectoryException de)
-    {
-      if (debugEnabled())
-      {
-        TRACER.debugCaught(DebugLogLevel.ERROR, de);
-      }
-
-      // FIXME -- Do we need to do anything here?
-
-      return;
-    }
-
-    // The entry is one that should be sent to the client. See if we
-    // also need to construct an entry change notification control.
-    ArrayList<Control> entryControls = new ArrayList<Control>(1);
-    if (returnECs)
-    {
-      entryControls.add(new EntryChangeNotificationControl(
-          PersistentSearchChangeType.MODIFY, changeNumber));
-    }
-
-    // Send the entry and see if we should continue processing. If
-    // not, then deregister this persistent search.
-    try
-    {
-      if (!searchOperation.returnEntry(entry, entryControls))
-      {
-        cancel();
-        searchOperation.sendSearchResultDone();
-      }
-    }
-    catch (Exception e)
-    {
-      if (debugEnabled())
-      {
-        TRACER.debugCaught(DebugLogLevel.ERROR, e);
-      }
-
-      cancel();
-
-      try
-      {
-        searchOperation.sendSearchResultDone();
-      }
-      catch (Exception e2)
-      {
-        if (debugEnabled())
-        {
-          TRACER.debugCaught(DebugLogLevel.ERROR, e2);
-        }
-      }
+      sendEntry(entry, createControls(MODIFY, null));
     }
   }
 
+  private boolean isInScopeForModify(final DN dn)
+  {
+    final DN baseDN = searchOperation.getBaseDN();
+    switch (searchOperation.getScope())
+    {
+    case BASE_OBJECT:
+      return baseDN.equals(dn);
+    case SINGLE_LEVEL:
+      return baseDN.equals(dn.getParent());
+    case WHOLE_SUBTREE:
+      return baseDN.isAncestorOf(dn);
+    case SUBORDINATE_SUBTREE:
+      return !baseDN.equals(dn) && baseDN.isAncestorOf(dn);
+    default:
+      return false;
+    }
+  }
 
+  private boolean anyMatchesFilter(Entry entry, Entry oldEntry)
+  {
+    return matchesFilter(oldEntry) || matchesFilter(entry);
+  }
 
   /**
    * Notifies the persistent searches that an entry has been renamed.
    *
    * @param entry
    *          The entry after it was modified.
-   * @param changeNumber
-   *          The change number associated with the operation that
-   *          modified the entry, or {@code -1} if there is no change
-   *          number.
    * @param oldDN
    *          The DN of the entry before it was renamed.
    */
-  public void processModifyDN(Entry entry, long changeNumber, DN oldDN)
+  public void processModifyDN(Entry entry, DN oldDN)
   {
-    // See if we care about modify DN operations.
-    if (!changeTypes.contains(PersistentSearchChangeType.MODIFY_DN))
+    if (changeTypes.contains(MODIFY_DN)
+        && isAnyInScopeForModify(entry, oldDN)
+        && matchesFilter(entry))
     {
-      return;
+      sendEntry(entry, createControls(MODIFY_DN, oldDN));
     }
+  }
 
-    // Make sure that the old or new entry is within our target scope.
-    // In this case, we need to check the DNs of both the old and new
-    // entry so we know which one(s) should be compared against the
-    // filter.
-    boolean oldMatches = false;
-    boolean newMatches = false;
+  private boolean isAnyInScopeForModify(Entry entry, DN oldDN)
+  {
+    return isInScopeForModify(oldDN) || isInScopeForModify(entry.getDN());
+  }
 
-    switch (scope)
-    {
-    case BASE_OBJECT:
-      oldMatches = baseDN.equals(oldDN);
-      newMatches = baseDN.equals(entry.getDN());
-
-      if (!(oldMatches || newMatches))
-      {
-        return;
-      }
-
-      break;
-    case SINGLE_LEVEL:
-      oldMatches = baseDN.equals(oldDN.getParent());
-      newMatches = baseDN.equals(entry.getDN().getParent());
-
-      if (!(oldMatches || newMatches))
-      {
-        return;
-      }
-
-      break;
-    case WHOLE_SUBTREE:
-      oldMatches = baseDN.isAncestorOf(oldDN);
-      newMatches = baseDN.isAncestorOf(entry.getDN());
-
-      if (!(oldMatches || newMatches))
-      {
-        return;
-      }
-
-      break;
-    case SUBORDINATE_SUBTREE:
-      oldMatches = ((!baseDN.equals(oldDN)) && baseDN.isAncestorOf(oldDN));
-      newMatches = ((!baseDN.equals(entry.getDN())) && baseDN
-          .isAncestorOf(entry.getDN()));
-
-      if (!(oldMatches || newMatches))
-      {
-        return;
-      }
-
-      break;
-    default:
-      return;
-    }
-
-    // Make sure that the entry matches the target filter.
-    try
-    {
-      if (!oldMatches && !newMatches && !filter.matchesEntry(entry))
-      {
-        return;
-      }
-    }
-    catch (DirectoryException de)
-    {
-      if (debugEnabled())
-      {
-        TRACER.debugCaught(DebugLogLevel.ERROR, de);
-      }
-
-      // FIXME -- Do we need to do anything here?
-
-      return;
-    }
-
-    // The entry is one that should be sent to the client. See if we
-    // also need to construct an entry change notification control.
-    ArrayList<Control> entryControls = new ArrayList<Control>(1);
+  /**
+   * The entry is one that should be sent to the client. See if we also need to
+   * construct an entry change notification control.
+   */
+  private List<Control> createControls(PersistentSearchChangeType changeType,
+      DN previousDN)
+  {
     if (returnECs)
     {
-      entryControls.add(new EntryChangeNotificationControl(
-          PersistentSearchChangeType.MODIFY_DN, oldDN, changeNumber));
+      final Control c = previousDN != null
+          ? new EntryChangeNotificationControl(changeType, previousDN, -1)
+          : new EntryChangeNotificationControl(changeType, -1);
+      return Collections.singletonList(c);
     }
+    return Collections.emptyList();
+  }
 
-    // Send the entry and see if we should continue processing. If
-    // not, then deregister this persistent search.
+  private void sendEntry(Entry entry, List<Control> entryControls)
+  {
     try
     {
       if (!searchOperation.returnEntry(entry, entryControls))
@@ -813,9 +505,9 @@
     buffer.append(",baseDN=\"");
     searchOperation.getBaseDN().toString(buffer);
     buffer.append("\",scope=");
-    buffer.append(scope.toString());
+    buffer.append(searchOperation.getScope());
     buffer.append(",filter=\"");
-    filter.toString(buffer);
+    searchOperation.getFilter().toString(buffer);
     buffer.append("\")");
   }
 }

--
Gitblit v1.10.0