[geode] 10/21: GEODE-7050: Use Log4jAgent only if Log4j is using Log4jProvider (#3892)

[on...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

onichols pushed a commit to branch release/1.9.1
in repository https://gitbox.apache.org/repos/asf/geode.git

commit bf4703072f934b3f5fecfcd5ad4cd46fec584022
Author: Kirk Lund <kl...@apache.org>
AuthorDate: Thu Aug 8 18:17:32 2019 -0700

    GEODE-7050: Use Log4jAgent only if Log4j is using Log4jProvider (#3892)
    
    * GEODE-7050: Use Log4jAgent only if Log4j is using Log4jProvider
    
    This change prevents Geode from using Log4jAgent if Log4j Core is
    present but not using Log4jProvider.
    
    For example, Log4j Core uses SLF4JProvider when log4j-to-slf4j is in
    the classpath.
    
    By disabling Log4jAgent when other Log4j Providers are in use, this
    prevents problems such as ClassCastExceptions when attemping to cast
    loggers from org.apache.logging.slf4j.SLF4JLogger to
    org.apache.logging.log4j.core.Logger to get the LoggerConfig or
    LoggerContext.
    
    * Update geode-core/src/test/java/org/apache/geode/internal/logging/DefaultProviderCheckerTest.java
    
    Co-Authored-By: Aaron Lindsey <al...@pivotal.io>
    (cherry picked from commit e5c9c420f462149fd062847904e3435fbe99afb4)
---
 .../internal/logging/DefaultProviderChecker.java   |  82 +++++++++++
 .../internal/logging/ProviderAgentLoader.java      |  26 +---
 .../logging/DefaultProviderCheckerTest.java        | 152 +++++++++++++++++++++
 3 files changed, 236 insertions(+), 24 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/logging/DefaultProviderChecker.java b/geode-core/src/main/java/org/apache/geode/internal/logging/DefaultProviderChecker.java
new file mode 100644
index 0000000..fae6390
--- /dev/null
+++ b/geode-core/src/main/java/org/apache/geode/internal/logging/DefaultProviderChecker.java
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.logging;
+
+import java.util.function.Function;
+import java.util.function.Supplier;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.status.StatusLogger;
+
+import org.apache.geode.annotations.VisibleForTesting;
+import org.apache.geode.internal.ClassPathLoader;
+import org.apache.geode.internal.logging.ProviderAgentLoader.AvailabilityChecker;
+
+class DefaultProviderChecker implements AvailabilityChecker {
+
+  /**
+   * The default {@code ProviderAgent} is {@code Log4jAgent}.
+   */
+  static final String DEFAULT_PROVIDER_AGENT_NAME =
+      "org.apache.geode.internal.logging.log4j.Log4jAgent";
+
+  static final String DEFAULT_PROVIDER_CLASS_NAME =
+      "org.apache.logging.log4j.core.impl.Log4jContextFactory";
+
+  private final Supplier<Class> contextFactoryClassSupplier;
+  private final Function<String, Boolean> isClassLoadableFunction;
+  private final Logger logger;
+
+  DefaultProviderChecker() {
+    this(() -> LogManager.getFactory().getClass(), DefaultProviderChecker::isClassLoadable,
+        StatusLogger.getLogger());
+  }
+
+  @VisibleForTesting
+  DefaultProviderChecker(Supplier<Class> contextFactoryClassSupplier,
+      Function<String, Boolean> isClassLoadableFunction,
+      Logger logger) {
+    this.contextFactoryClassSupplier = contextFactoryClassSupplier;
+    this.isClassLoadableFunction = isClassLoadableFunction;
+    this.logger = logger;
+  }
+
+  @Override
+  public boolean isAvailable() {
+    if (!isClassLoadableFunction.apply(DEFAULT_PROVIDER_CLASS_NAME)) {
+      logger.info("Unable to find Log4j Core.");
+      return false;
+    }
+
+    boolean usingLog4jProvider =
+        DEFAULT_PROVIDER_CLASS_NAME.equals(contextFactoryClassSupplier.get().getName());
+    String message = "Log4j Core is available "
+        + (usingLog4jProvider ? "and using" : "but not using") + " Log4jProvider.";
+    logger.info(message);
+    return usingLog4jProvider;
+  }
+
+  @VisibleForTesting
+  static boolean isClassLoadable(String className) {
+    try {
+      ClassPathLoader.getLatest().forName(className);
+      return true;
+    } catch (ClassNotFoundException e) {
+      return false;
+    }
+  }
+
+}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/logging/ProviderAgentLoader.java b/geode-core/src/main/java/org/apache/geode/internal/logging/ProviderAgentLoader.java
index 5d6dd98..0671f3b 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/logging/ProviderAgentLoader.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/logging/ProviderAgentLoader.java
@@ -15,7 +15,7 @@
 package org.apache.geode.internal.logging;
 
 import static org.apache.geode.internal.lang.SystemPropertyHelper.GEODE_PREFIX;
-import static org.apache.geode.internal.logging.ProviderAgentLoader.DefaultProvider.DEFAULT_PROVIDER_AGENT_NAME;
+import static org.apache.geode.internal.logging.DefaultProviderChecker.DEFAULT_PROVIDER_AGENT_NAME;
 
 import java.util.ServiceLoader;
 
@@ -51,7 +51,7 @@ public class ProviderAgentLoader {
   private final AvailabilityChecker availabilityChecker;
 
   public ProviderAgentLoader() {
-    this(new DefaultProvider());
+    this(new DefaultProviderChecker());
   }
 
   @VisibleForTesting
@@ -132,26 +132,4 @@ public class ProviderAgentLoader {
     boolean isAvailable();
   }
 
-  static class DefaultProvider implements AvailabilityChecker {
-
-    /**
-     * The default {@code ProviderAgent} is {@code Log4jAgent}.
-     */
-    static final String DEFAULT_PROVIDER_AGENT_NAME =
-        "org.apache.geode.internal.logging.log4j.Log4jAgent";
-
-    static final String DEFAULT_PROVIDER_CLASS_NAME = "org.apache.logging.log4j.core.Logger";
-
-    @Override
-    public boolean isAvailable() {
-      try {
-        ClassPathLoader.getLatest().forName(DEFAULT_PROVIDER_CLASS_NAME);
-        LOGGER.info("Log4j Core is available");
-        return true;
-      } catch (ClassNotFoundException | ClassCastException e) {
-        LOGGER.info("Unable to find Log4j Core");
-      }
-      return false;
-    }
-  }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/internal/logging/DefaultProviderCheckerTest.java b/geode-core/src/test/java/org/apache/geode/internal/logging/DefaultProviderCheckerTest.java
new file mode 100644
index 0000000..ac16278
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/logging/DefaultProviderCheckerTest.java
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.logging;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.core.impl.Log4jContextFactory;
+import org.apache.logging.log4j.spi.LoggerContextFactory;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+
+import org.apache.geode.internal.ClassPathLoader;
+
+public class DefaultProviderCheckerTest {
+
+  @Test
+  public void isAvailableReturnsTrueIfAbleToLoadDefaultProviderClass() {
+    DefaultProviderChecker checker = new DefaultProviderChecker(() -> Log4jContextFactory.class,
+        (a) -> true, mock(Logger.class));
+
+    boolean value = checker.isAvailable();
+
+    assertThat(value).isTrue();
+  }
+
+  @Test
+  public void isAvailableReturnsFalseIfUnableToLoadDefaultProviderClass() {
+    DefaultProviderChecker checker = new DefaultProviderChecker(() -> Log4jContextFactory.class,
+        (a) -> false, mock(Logger.class));
+
+    boolean value = checker.isAvailable();
+
+    assertThat(value).isFalse();
+  }
+
+  @Test
+  public void isAvailableReturnsFalseIfNotUsingLog4jProvider() {
+    DefaultProviderChecker checker = new DefaultProviderChecker(
+        () -> mock(LoggerContextFactory.class).getClass(), (a) -> true, mock(Logger.class));
+
+    boolean value = checker.isAvailable();
+
+    assertThat(value).isFalse();
+  }
+
+  @Test
+  public void logsUsingMessageIfUsingLog4jProvider() {
+    Logger logger = mock(Logger.class);
+    DefaultProviderChecker checker =
+        new DefaultProviderChecker(() -> Log4jContextFactory.class, (a) -> true, logger);
+
+    boolean value = checker.isAvailable();
+
+    assertThat(value).isTrue();
+
+    ArgumentCaptor<String> loggedMessage = ArgumentCaptor.forClass(String.class);
+    verify(logger).info(loggedMessage.capture());
+
+    assertThat(loggedMessage.getValue())
+        .isEqualTo("Log4j Core is available and using Log4jProvider.");
+  }
+
+  @Test
+  public void logsNotUsingMessageIfNotUsingLog4jProvider() {
+    Logger logger = mock(Logger.class);
+    DefaultProviderChecker checker = new DefaultProviderChecker(
+        () -> mock(LoggerContextFactory.class).getClass(), (a) -> true, logger);
+
+    boolean value = checker.isAvailable();
+
+    assertThat(value).isFalse();
+
+    ArgumentCaptor<String> loggedMessage = ArgumentCaptor.forClass(String.class);
+    verify(logger).info(loggedMessage.capture());
+
+    assertThat(loggedMessage.getValue())
+        .isEqualTo("Log4j Core is available but not using Log4jProvider.");
+  }
+
+  @Test
+  public void logsUnableToFindMessageIfClassNotFoundExceptionIsCaught() {
+    Logger logger = mock(Logger.class);
+    DefaultProviderChecker checker =
+        new DefaultProviderChecker(() -> Log4jContextFactory.class, (a) -> false, logger);
+
+    boolean value = checker.isAvailable();
+
+    assertThat(value).isFalse();
+
+    ArgumentCaptor<String> loggedMessage = ArgumentCaptor.forClass(String.class);
+    verify(logger).info(loggedMessage.capture());
+
+    assertThat(loggedMessage.getValue()).isEqualTo("Unable to find Log4j Core.");
+  }
+
+  @Test
+  public void rethrowsIfIsClassLoadableFunctionThrowsRuntimeException() {
+    RuntimeException exception = new RuntimeException("expected");
+    DefaultProviderChecker checker =
+        new DefaultProviderChecker(() -> Log4jContextFactory.class, (a) -> {
+          throw exception;
+        }, mock(Logger.class));
+
+    Throwable thrown = catchThrowable(() -> checker.isAvailable());
+
+    assertThat(thrown).isSameAs(exception);
+  }
+
+  @Test
+  public void isClassLoadableReturnsTrueIfClassNameExists() {
+    boolean value = DefaultProviderChecker.isClassLoadable(ClassPathLoader.class.getName());
+
+    assertThat(value).isTrue();
+  }
+
+  @Test
+  public void isClassLoadableReturnsFalseIfClassNameDoesNotExist() {
+    boolean value = DefaultProviderChecker.isClassLoadable("Not a class");
+
+    assertThat(value).isFalse();
+  }
+
+  @Test
+  public void isClassLoadableThrowsNullPointerExceptionIfClassNameIsNull() {
+    Throwable thrown = catchThrowable(() -> DefaultProviderChecker.isClassLoadable(null));
+
+    assertThat(thrown).isInstanceOf(NullPointerException.class);
+  }
+
+  @Test
+  public void isClassLoadableReturnsFalseIfClassNameIsEmpty() {
+    boolean value = DefaultProviderChecker.isClassLoadable("");
+
+    assertThat(value).isFalse();
+  }
+}