From 8d950406378529ffdc449aaacaa85cbfc29ab3c1 Mon Sep 17 00:00:00 2001
From: Jean-Noël Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Wed, 16 Dec 2015 14:29:42 +0000
Subject: [PATCH] OPENDJ-2547 Creating a debug target for a specific method actually traces messages for the whole class

---
 opendj-server-legacy/src/main/java/org/opends/server/loggers/DebugTracer.java   |  124 ++++++++++++------------------
 opendj-server-legacy/src/main/java/org/opends/server/loggers/TraceSettings.java |  104 +++++++++----------------
 2 files changed, 90 insertions(+), 138 deletions(-)

diff --git a/opendj-server-legacy/src/main/java/org/opends/server/loggers/DebugTracer.java b/opendj-server-legacy/src/main/java/org/opends/server/loggers/DebugTracer.java
index 51444b3..3054532 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/loggers/DebugTracer.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/loggers/DebugTracer.java
@@ -26,6 +26,8 @@
  */
 package org.opends.server.loggers;
 
+import static org.opends.server.loggers.TraceSettings.Level.*;
+
 import java.util.Map;
 
 import org.forgerock.i18n.slf4j.LocalizedLogger;
@@ -42,24 +44,39 @@
 public class DebugTracer
 {
   /**
-   *  We have to harcode this value because we cannot import
-   *  org.opends.server.loggers.slf4j.OpenDJLoggerAdapter(.class.getName())
+   *  We have to hardcode this value because we cannot import
+   *  {@code org.opends.server.loggers.slf4j.OpenDJLoggerAdapter.class.getName()}
    *  to avoid OSGI split package issues.
-   *  See OPENDJ-2226.
+   *  @see OPENDJ-2226
    */
   private static final String OPENDJ_LOGGER_ADAPTER_CLASS_NAME = "org.opends.server.loggers.slf4j.OpenDJLoggerAdapter";
 
   /** The class this aspect traces. */
   private String className;
 
-  /**
-   * A class that represents a settings cache entry.
-   */
+  /** A class that represents a settings cache entry. */
   private class PublisherSettings
   {
-    DebugLogPublisher<?> debugPublisher;
-    TraceSettings classSettings;
-    Map<String, TraceSettings> methodSettings;
+    private final DebugLogPublisher<?> debugPublisher;
+    private final TraceSettings classSettings;
+    private final Map<String, TraceSettings> methodSettings;
+
+    private PublisherSettings(String className, DebugLogPublisher<?> publisher)
+    {
+      debugPublisher = publisher;
+      classSettings = publisher.getClassSettings(className);
+      methodSettings = publisher.getMethodSettings(className);
+    }
+
+    @Override
+    public String toString()
+    {
+      return getClass().getSimpleName() + "("
+          + "className=" + className
+          + ", classSettings=" + classSettings
+          + ", methodSettings=" + methodSettings
+          + ")";
+    }
   }
 
   private PublisherSettings[] publisherSettings;
@@ -68,34 +85,13 @@
    * Construct a new DebugTracer object with cached settings obtained from
    * the provided array of publishers.
    *
-   * @param className The classname to use as category for logging.
+   * @param className The class name to use as category for logging.
    * @param publishers The array of publishers to obtain the settings from.
    */
-  @SuppressWarnings({ "unchecked", "rawtypes" })
-  DebugTracer(String className, DebugLogPublisher[] publishers)
+  DebugTracer(String className, DebugLogPublisher<?>[] publishers)
   {
     this.className = className;
-    publisherSettings = new PublisherSettings[publishers.length];
-
-    // Get the settings from all publishers.
-    for(int i = 0; i < publishers.length; i++)
-    {
-      DebugLogPublisher publisher = publishers[i];
-      PublisherSettings settings = new PublisherSettings();
-
-      settings.debugPublisher = publisher;
-      settings.classSettings = publisher.getClassSettings(className);
-
-      // For some reason, the compiler doesn't see that
-      // debugLogPublihser.getMethodSettings returns a parameterized Map.
-      // This problem goes away if a parameterized verson of DebugLogPublisher
-      // is used. However, we can't not use reflection to instantiate a generic
-      // DebugLogPublisher<? extends DebugLogPublisherCfg> type. The only thing
-      // we can do is to just supress the compiler warnings.
-      settings.methodSettings = publisher.getMethodSettings(className);
-
-      publisherSettings[i] = settings;
-    }
+    publisherSettings = toPublisherSettings(publishers);
   }
 
   /**
@@ -128,7 +124,7 @@
       TraceSettings activeSettings = settings.classSettings;
       Map<String, TraceSettings> methodSettings = settings.methodSettings;
 
-      if (shouldLog(hasException, activeSettings) || methodSettings != null)
+      if (shouldLog(activeSettings, hasException) || methodSettings != null)
       {
         if (stackTrace == null)
         {
@@ -146,7 +142,6 @@
         if (methodSettings != null)
         {
           TraceSettings mSettings = methodSettings.get(signature);
-
           if (mSettings == null)
           {
             // Try looking for an undecorated method name
@@ -157,11 +152,11 @@
             }
           }
 
-          // If this method does have a specific setting and it is not
-          // suppose to be logged, continue.
+          // If this method does have a specific setting
+          // and it is not supposed to be logged, continue.
           if (mSettings != null)
           {
-            if (!shouldLog(hasException, mSettings))
+            if (!shouldLog(mSettings, hasException))
             {
               continue;
             }
@@ -172,16 +167,12 @@
           }
         }
 
-        String sourceLocation =
-            callerFrame.getFileName() + ":" + callerFrame.getLineNumber();
+        String sourceLocation = callerFrame.getFileName() + ":" + callerFrame.getLineNumber();
 
         if (filteredStackTrace == null && activeSettings.getStackDepth() > 0)
         {
-          StackTraceElement[] trace =
-              hasException ? exception.getStackTrace() : stackTrace;
-          filteredStackTrace =
-              DebugStackTraceFormatter.SMART_FRAME_FILTER
-                  .getFilteredStackTrace(trace);
+          StackTraceElement[] trace = hasException ? exception.getStackTrace() : stackTrace;
+          filteredStackTrace = DebugStackTraceFormatter.SMART_FRAME_FILTER.getFilteredStackTrace(trace);
         }
 
         if (hasException)
@@ -232,41 +223,28 @@
    *
    * @param publishers The array of publishers to obtain the settings from.
    */
-  @SuppressWarnings({ "unchecked", "rawtypes" })
-  void updateSettings(DebugLogPublisher[] publishers)
+  void updateSettings(DebugLogPublisher<?>[] publishers)
   {
-    PublisherSettings[] newSettings =
-        new PublisherSettings[publishers.length];
+    publisherSettings = toPublisherSettings(publishers);
+  }
 
+  private PublisherSettings[] toPublisherSettings(DebugLogPublisher<?>[] publishers)
+  {
     // Get the settings from all publishers.
+    PublisherSettings[] newSettings = new PublisherSettings[publishers.length];
     for(int i = 0; i < publishers.length; i++)
     {
-      DebugLogPublisher publisher = publishers[i];
-      PublisherSettings settings = new PublisherSettings();
-
-      settings.debugPublisher = publisher;
-      settings.classSettings = publisher.getClassSettings(className);
-
-      // For some reason, the compiler doesn't see that
-      // debugLogPublihser.getMethodSettings returns a parameterized Map.
-      // This problem goes away if a parameterized verson of DebugLogPublisher
-      // is used. However, we can't not use reflection to instantiate a generic
-      // DebugLogPublisher<? extends DebugLogPublisherCfg> type. The only thing
-      // we can do is to just supress the compiler warnings.
-      settings.methodSettings = publisher.getMethodSettings(className);
-
-      newSettings[i] = settings;
+      newSettings[i] = new PublisherSettings(className, publishers[i]);
     }
-
-    publisherSettings = newSettings;
+    return newSettings;
   }
 
   /**
    * Return the caller stack frame.
    *
-   * @param stackTrace The entrie stack trace frames.
-   * @return the caller stack frame or null if none is found on the
-   * stack trace.
+   * @param stackTrace
+   *          The stack trace frames of the caller.
+   * @return the caller stack frame or null if none is found on the stack trace.
    */
   private StackTraceElement getCallerFrame(StackTraceElement[] stackTrace)
   {
@@ -303,15 +281,15 @@
   }
 
   /** Indicates if there is something to log. */
-  private boolean shouldLog(boolean hasException, TraceSettings activeSettings)
+  private boolean shouldLog(TraceSettings settings, boolean hasException)
   {
-    return activeSettings.getLevel() == TraceSettings.Level.ALL
-        || (hasException && activeSettings.getLevel() == TraceSettings.Level.EXCEPTIONS_ONLY);
+    return settings.getLevel() == ALL
+        || (hasException && settings.getLevel() == EXCEPTIONS_ONLY);
   }
 
   /** Indicates if there is something to log. */
   private boolean shouldLog(TraceSettings settings)
   {
-    return settings.getLevel() != TraceSettings.Level.DISABLED;
+    return settings.getLevel() != DISABLED;
   }
 }
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/loggers/TraceSettings.java b/opendj-server-legacy/src/main/java/org/opends/server/loggers/TraceSettings.java
index 779b8a6..208a5fd 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/loggers/TraceSettings.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/loggers/TraceSettings.java
@@ -24,25 +24,21 @@
  *      Copyright 2006-2009 Sun Microsystems, Inc.
  *      Portions Copyright 2014-2015 ForgeRock AS
  */
-
 package org.opends.server.loggers;
 
 import java.util.List;
 
 import org.forgerock.i18n.LocalizableMessage;
+import org.forgerock.opendj.config.server.ConfigChangeResult;
 import org.opends.server.admin.server.ConfigurationChangeListener;
 import org.opends.server.admin.std.server.DebugTargetCfg;
-import org.forgerock.opendj.config.server.ConfigChangeResult;
 
-/**
- * This class encapsulates the trace settings in effect at a given tracing scope.
- */
+/** This class encapsulates the trace settings in effect at a given tracing scope. */
 public class TraceSettings implements
     ConfigurationChangeListener<DebugTargetCfg>
 {
   /** A TraceSettings object representing a fully disabled trace state. */
-  public static final TraceSettings DISABLED =
-      new TraceSettings(Level.DISABLED);
+  public static final TraceSettings DISABLED = new TraceSettings(Level.DISABLED);
 
   private static final String STACK_DUMP_KEYWORD = "stack";
   private static final String INCLUDE_CAUSE_KEYWORD = "cause";
@@ -51,17 +47,13 @@
   private static final String ENABLED_KEYWORD = "enabled";
   private static final String EXCEPTIONS_ONLY_KEYWORD = "exceptionsonly";
 
-  /**
-   * Represents the level of trace.
-   */
+  /** Represents the level of trace. */
   enum Level
   {
-    /** Log nothing. **/
+    /** Log nothing. */
     DISABLED,
-
-    /** Log only exceptions. **/
+    /** Log only exceptions. */
     EXCEPTIONS_ONLY,
-
     /** Log everything. */
     ALL;
 
@@ -82,50 +74,27 @@
         {
           return Level.EXCEPTIONS_ONLY;
         }
-        else
-        {
-          return Level.ALL;
-        }
+        return Level.ALL;
       }
       return Level.DISABLED;
     }
-
   }
 
-  /**
-   * The level of this setting.
-   */
+  /** The level of this setting. */
   private Level level;
-
-  /**
-   * Indicates if method arguments should be logged.
-   */
+  /** Indicates if method arguments should be logged. */
   private boolean noArgs;
-
-  /**
-   * Indicates if method return values should be logged.
-   */
+  /** Indicates if method return values should be logged. */
   private boolean noRetVal;
-
-  /**
-   * The level of stack frames to include.
-   */
+  /** The level of stack frames to include. */
   private int stackDepth;
-
-  /**
-   * Indicates if the cause exception is included in exception messages.
-   */
+  /** Indicates if the cause exception is included in exception messages. */
   private boolean includeCause;
 
-  private DebugTargetCfg currentConfig;
-
-  /**
-   * Construct new trace settings with default values.
-   */
+  /** Construct new trace settings with default values. */
   public TraceSettings()
   {
     this(Level.ALL, false, false, 0, false);
-
   }
 
   /**
@@ -137,7 +106,6 @@
   private TraceSettings(Level level)
   {
     this(level, false, false, 0, false);
-
   }
 
   /**
@@ -175,43 +143,37 @@
    */
   TraceSettings(DebugTargetCfg config)
   {
-    this.level =
-        Level.getLevel(config.isEnabled(), config.isDebugExceptionsOnly());
+    this.level = Level.getLevel(config.isEnabled(), config.isDebugExceptionsOnly());
     this.noArgs = config.isOmitMethodEntryArguments();
     this.noRetVal = config.isOmitMethodReturnValue();
     this.stackDepth = config.getThrowableStackFrames();
     this.includeCause = config.isIncludeThrowableCause();
 
-    currentConfig = config;
     config.addChangeListener(this);
   }
 
-  /** {@inheritDoc} */
+  @Override
   public boolean isConfigurationChangeAcceptable(DebugTargetCfg config,
       List<LocalizableMessage> unacceptableReasons)
   {
-    // This should alwas be acceptable. We are assuing that the scope for this
-    // trace setting is the same sine its part of the DN.
+    // This should always be acceptable. We are assuming that the scope for this
+    // trace setting is the same since it is part of the DN.
     return true;
   }
 
-  /** {@inheritDoc} */
+  @Override
   public ConfigChangeResult applyConfigurationChange(DebugTargetCfg config)
   {
     final ConfigChangeResult ccr = new ConfigChangeResult();
-
-    // We can assume that the target scope did not change since its the
+    // We can assume that the target scope did not change since it is the
     // naming attribute. Changing it would result in a modify DN.
 
-    this.level =
-        Level.getLevel(config.isEnabled(), config.isDebugExceptionsOnly());
+    this.level = Level.getLevel(config.isEnabled(), config.isDebugExceptionsOnly());
     this.noArgs = config.isOmitMethodEntryArguments();
     this.noRetVal = config.isOmitMethodReturnValue();
     this.stackDepth = config.getThrowableStackFrames();
     this.includeCause = config.isIncludeThrowableCause();
 
-    this.currentConfig = config;
-
     return ccr;
   }
 
@@ -265,25 +227,25 @@
           }
         }
         //See if to include cause in exception messages.
-        else if (keyword.equals(INCLUDE_CAUSE_KEYWORD))
+        else if (INCLUDE_CAUSE_KEYWORD.equals(keyword))
         {
           includeCause = true;
         }
-        //See if to supress method arguments.
-        else if (keyword.equals(SUPPRESS_ARG_KEYWORD))
+        // See if to suppress method arguments.
+        else if (SUPPRESS_ARG_KEYWORD.equals(keyword))
         {
           noArgs = true;
         }
-        //See if to supress return values.
-        else if (keyword.equals(SUPPRESS_RETVAL_KEYWORD))
+        // See if to suppress return values.
+        else if (SUPPRESS_RETVAL_KEYWORD.equals(keyword))
         {
           noRetVal = true;
         }
-        else if (keyword.equals(ENABLED_KEYWORD))
+        else if (ENABLED_KEYWORD.equals(keyword))
         {
           enabled = true;
         }
-        else if (keyword.equals(EXCEPTIONS_ONLY_KEYWORD))
+        else if (EXCEPTIONS_ONLY_KEYWORD.equals(keyword))
         {
           exceptionsOnly = true;
         }
@@ -345,4 +307,16 @@
   {
     return includeCause;
   }
+
+  @Override
+  public String toString()
+  {
+    return getClass().getSimpleName() + "("
+        + "level=" + level
+        + ", logMethodArguments=" + !noArgs
+        + ", logMethodReturnValue=" + !noRetVal
+        + ", logCauseException=" + includeCause
+        + ", stackDepth=" + stackDepth
+        + ")";
+  }
 }

--
Gitblit v1.10.0