From 1e53194379b3b80e4d2a026865c0dde59bbf6ed1 Mon Sep 17 00:00:00 2001
From: Jean-Noël Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Tue, 28 Jun 2016 13:17:43 +0000
Subject: [PATCH] OPENDJ-2715 Incorrect "exclude" message when importing entries with an exclude filter

---
 opendj-server-legacy/src/main/java/org/opends/server/types/LDIFImportConfig.java              |   61 ++++++++++----------
 opendj-server-legacy/src/messages/org/opends/messages/utility.properties                      |    8 ++
 opendj-server-legacy/src/main/java/org/opends/server/util/LDIFReader.java                     |   28 +++-----
 opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ImportLDIFReader.java |   25 ++++----
 4 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ImportLDIFReader.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ImportLDIFReader.java
index f519be6..2773c71 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ImportLDIFReader.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ImportLDIFReader.java
@@ -29,15 +29,16 @@
 import org.forgerock.i18n.LocalizableMessage;
 import org.forgerock.i18n.LocalizableMessageBuilder;
 import org.forgerock.i18n.slf4j.LocalizedLogger;
+import org.forgerock.opendj.ldap.DN;
 import org.forgerock.opendj.ldap.schema.AttributeType;
+import org.forgerock.opendj.ldap.schema.ObjectClass;
+import org.forgerock.util.Pair;
 import org.forgerock.util.Reject;
 import org.opends.server.api.plugin.PluginResult;
 import org.opends.server.core.DirectoryServer;
 import org.opends.server.types.AttributeBuilder;
-import org.forgerock.opendj.ldap.DN;
 import org.opends.server.types.Entry;
 import org.opends.server.types.LDIFImportConfig;
-import org.forgerock.opendj.ldap.schema.ObjectClass;
 import org.opends.server.util.LDIFException;
 import org.opends.server.util.LDIFReader;
 
@@ -151,12 +152,13 @@
           // read and return the next entry.
           continue;
         }
-        if (!importConfig.includeEntry(entryDN))
+
+        entriesRead.incrementAndGet();
+
+        final Pair<Boolean, LocalizableMessage> includeResult = importConfig.includeEntry(entryDN);
+        if (!includeResult.getFirst())
         {
-          logger.trace("Skipping entry %s because the DN is not one that "
-              + "should be included based on the include and exclude branches.", entryDN);
-          entriesRead.incrementAndGet();
-          logToSkipWriter(lines, ERR_LDIF_SKIP.get(entryDN));
+          logToSkipWriter(lines, includeResult.getSecond());
           continue;
         }
         entryContainer = getEntryContainer(entryDN, suffixesMap);
@@ -164,11 +166,9 @@
         {
           logger.trace("Skipping entry %s because the DN is not one that "
               + "should be included based on a suffix match check.", entryDN);
-          entriesRead.incrementAndGet();
           logToSkipWriter(lines, ERR_LDIF_SKIP.get(entryDN));
           continue;
         }
-        entriesRead.incrementAndGet();
         entryID = rootContainer.getNextEntryID();
 
         if (!addPending(entryDN))
@@ -227,11 +227,10 @@
     final DN entryDN = entry.getName();
     try
     {
-      if (!importConfig.includeEntry(entry))
+      final Pair<Boolean, LocalizableMessage> includeResult = importConfig.includeEntry(entry);
+      if (!includeResult.getFirst())
       {
-        logger.trace("Skipping entry %s because the DN is not one that "
-            + "should be included based on the include and exclude filters.", entryDN);
-        logToSkipWriter(entryLines, ERR_LDIF_SKIP.get(entryDN));
+        logToSkipWriter(entryLines, includeResult.getSecond());
         return false;
       }
       return true;
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/types/LDIFImportConfig.java b/opendj-server-legacy/src/main/java/org/opends/server/types/LDIFImportConfig.java
index fd58233..6b7cdb9 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/types/LDIFImportConfig.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/types/LDIFImportConfig.java
@@ -16,6 +16,8 @@
  */
 package org.opends.server.types;
 
+import static java.lang.Boolean.*;
+
 import static org.opends.messages.UtilityMessages.*;
 
 import java.io.BufferedReader;
@@ -38,9 +40,11 @@
 import java.util.Set;
 import java.util.zip.GZIPInputStream;
 
+import org.forgerock.i18n.LocalizableMessage;
 import org.forgerock.i18n.LocalizableMessageDescriptor.Arg1;
 import org.forgerock.opendj.ldap.DN;
 import org.forgerock.opendj.ldap.schema.AttributeType;
+import org.forgerock.util.Pair;
 import org.opends.server.tools.makeldif.MakeLDIFInputStream;
 import org.opends.server.tools.makeldif.TemplateFile;
 import org.opends.server.util.CollectionUtils;
@@ -58,7 +62,6 @@
 public final class LDIFImportConfig extends OperationConfig
                                     implements Closeable
 {
-
   /** The default buffer size that will be used when reading LDIF data. */
   private static final int DEFAULT_BUFFER_SIZE = 8192;
 
@@ -646,17 +649,16 @@
   }
 
   /**
-   * Indicates whether to include the entry with the specified DN in
-   * the import.
+   * Indicates whether to include the entry with the specified DN in the import.
    *
-   * @param  dn  The DN of the entry for which to make the
-   *             determination.
-   *
-   * @return  <CODE>true</CODE> if the entry with the specified DN
-   *          should be included in the import, or <CODE>false</CODE>
-   *          if not.
+   * @param dn
+   *          The DN of the entry for which to make the determination.
+   * @return a pair where the first element is a boolean indicating whether the entry with the
+   *         specified DN should be included in the import, and the second element is a message with
+   *         the reason why an entry is not included in the import (it is {@code null} when the
+   *         entry is included in the import).
    */
-  public boolean includeEntry(DN dn)
+  public Pair<Boolean, LocalizableMessage> includeEntry(DN dn)
   {
     if (! excludeBranches.isEmpty())
     {
@@ -664,7 +666,7 @@
       {
         if (excludeBranch.isSuperiorOrEqualTo(dn))
         {
-          return false;
+          return Pair.of(FALSE, ERR_LDIF_SKIP_EXCLUDE_BRANCH.get(dn, excludeBranch));
         }
       }
     }
@@ -675,14 +677,14 @@
       {
         if (includeBranch.isSuperiorOrEqualTo(dn))
         {
-          return true;
+          return Pair.of(TRUE, null);
         }
       }
 
-      return false;
+      return Pair.of(FALSE, ERR_LDIF_SKIP_NOT_IN_INCLUDED_BRANCHES.get(dn));
     }
 
-    return true;
+    return Pair.of(TRUE, null);
   }
 
 
@@ -881,46 +883,45 @@
 
   /**
    * Indicates whether the specified entry should be included in the
-   * import based on the configured set of include and exclude
-   * filters.
+   * import based on the configured set of include and exclude filters.
    *
    * @param  entry  The entry for which to make the determination.
-   *
-   * @return  <CODE>true</CODE> if the specified entry should be
-   *          included in the import, or <CODE>false</CODE> if not.
-   *
+   * @return  a pair where the first element is a boolean indicating whether
+   *          the entry with the specified DN should be included in the import,
+   *          and the second element is a message with the reason why an entry
+   *          is not included in the import (it is {@code null} when the entry
+   *          is included in the import).
    * @throws  DirectoryException  If there is a problem with any of
    *                              the search filters used to make the
    *                              determination.
    */
-  public boolean includeEntry(Entry entry)
-         throws DirectoryException
+  public Pair<Boolean, LocalizableMessage> includeEntry(Entry entry) throws DirectoryException
   {
     if (! excludeFilters.isEmpty())
     {
-      for (SearchFilter filter : excludeFilters)
+      for (SearchFilter excludeFilter : excludeFilters)
       {
-        if (filter.matchesEntry(entry))
+        if (excludeFilter.matchesEntry(entry))
         {
-          return false;
+          return Pair.of(FALSE, ERR_LDIF_SKIP_EXCLUDE_FILTER.get(entry.getName(), excludeFilter));
         }
       }
     }
 
     if (! includeFilters.isEmpty())
     {
-      for (SearchFilter filter : includeFilters)
+      for (SearchFilter includeFilter : includeFilters)
       {
-        if (filter.matchesEntry(entry))
+        if (includeFilter.matchesEntry(entry))
         {
-          return true;
+          return Pair.of(TRUE, null);
         }
       }
 
-      return false;
+      return Pair.of(FALSE, ERR_LDIF_SKIP_NOT_IN_INCLUDED_FILTERS.get(entry.getName()));
     }
 
-    return true;
+    return Pair.of(TRUE, null);
   }
 
 
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/util/LDIFReader.java b/opendj-server-legacy/src/main/java/org/opends/server/util/LDIFReader.java
index b074724..81a2bfa 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/util/LDIFReader.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/util/LDIFReader.java
@@ -46,6 +46,8 @@
 import org.forgerock.opendj.ldap.RDN;
 import org.forgerock.opendj.ldap.schema.AttributeType;
 import org.forgerock.opendj.ldap.schema.CoreSchema;
+import org.forgerock.opendj.ldap.schema.ObjectClass;
+import org.forgerock.util.Pair;
 import org.opends.server.api.plugin.PluginResult;
 import org.opends.server.core.DirectoryServer;
 import org.opends.server.core.PluginConfigManager;
@@ -57,7 +59,6 @@
 import org.opends.server.types.Attributes;
 import org.opends.server.types.Entry;
 import org.opends.server.types.LDIFImportConfig;
-import org.forgerock.opendj.ldap.schema.ObjectClass;
 import org.opends.server.types.RawModification;
 
 /**
@@ -200,29 +201,24 @@
         // read and return the next entry.
         continue;
       }
-      else if (!importConfig.includeEntry(entryDN))
+
+      entriesRead.incrementAndGet();
+      Pair<Boolean, LocalizableMessage> includeResult = importConfig.includeEntry(entryDN);
+      if (!includeResult.getFirst())
       {
-        logger.trace("Skipping entry %s because the DN is not one that "
-            + "should be included based on the include and exclude branches.", entryDN);
-        entriesRead.incrementAndGet();
-        logToSkipWriter(lines, ERR_LDIF_SKIP.get(entryDN));
+        logToSkipWriter(lines, includeResult.getSecond());
         continue;
       }
-      else
-      {
-        entriesRead.incrementAndGet();
-      }
 
-      // Create the entry and see if it is one that should be included in the import.
       final Entry entry = createEntry(entryDN, lines, checkSchema);
       if (!isIncludedInImport(entry,lines)
           || !invokeImportPlugins(entry, lines))
       {
         continue;
       }
-      validateAgainstSchemaIfNeeded(checkSchema, entry, lines);
 
       // The entry should be included in the import, so return it.
+      validateAgainstSchemaIfNeeded(checkSchema, entry, lines);
       return entry;
     }
   }
@@ -247,12 +243,10 @@
   {
     try
     {
-      if (!importConfig.includeEntry(entry))
+      Pair<Boolean, LocalizableMessage> includeResult = importConfig.includeEntry(entry);
+      if (!includeResult.getFirst())
       {
-        final DN entryDN = entry.getName();
-        logger.trace("Skipping entry %s because the DN is not one that "
-            + "should be included based on the include and exclude filters.", entryDN);
-        logToSkipWriter(lines, ERR_LDIF_SKIP.get(entryDN));
+        logToSkipWriter(lines, includeResult.getSecond());
         return false;
       }
       return true;
diff --git a/opendj-server-legacy/src/messages/org/opends/messages/utility.properties b/opendj-server-legacy/src/messages/org/opends/messages/utility.properties
index f528d2f..003b7f8 100644
--- a/opendj-server-legacy/src/messages/org/opends/messages/utility.properties
+++ b/opendj-server-legacy/src/messages/org/opends/messages/utility.properties
@@ -209,7 +209,7 @@
  the import configuration indicates that no attempt should be made to append \
  to or replace the file
 ERR_LDIF_SKIP_165=Skipping entry %s because the DN is not one that \
- should be included based on the include and exclude branches
+ should be included based on the include and exclude branches/filters
 ERR_EMBEDUTILS_SERVER_ALREADY_RUNNING_167=The Directory Server cannot \
  be started because it is already running
 INFO_EMAIL_TOOL_DESCRIPTION_171=Send an e-mail message via SMTP
@@ -259,6 +259,12 @@
 ERR_BASE64_CANNOT_WRITE_RAW_DATA_199=An error occurred while \
  attempting to write the decoded data: %s
 ERR_BASE64_UNKNOWN_SUBCOMMAND_200=Unknown subcommand %s
+ERR_LDIF_SKIP_EXCLUDE_BRANCH_201=Skipping entry %s because the DN is excluded by the exclude branch "%s"
+ERR_LDIF_SKIP_EXCLUDE_FILTER_202=Skipping entry %s because the DN is excluded by the exclude filter "%s"
+ERR_LDIF_SKIP_NOT_IN_INCLUDED_BRANCHES_203=Skipping entry %s because the DN \
+ is not included by any include branches
+ERR_LDIF_SKIP_NOT_IN_INCLUDED_FILTERS_204=Skipping entry %s because the DN \
+ is not included by any include filters
 ERR_LDIF_REJECTED_BY_PLUGIN_NOMESSAGE_224=Rejecting entry %s because \
  it was rejected by a plugin
 ERR_LDIF_REJECTED_BY_PLUGIN_225=Rejecting entry %s because it was \

--
Gitblit v1.10.0