IGNITE-23472 Fix after review

apache · Nov 25, 2024 · 2886b29 · 2886b29
1 parent 204c8a8
commit 2886b29
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 67 deletions.
diff --git a/modules/core/src/main/java/org/apache/ignite/logger/java/JavaLogger.java b/modules/core/src/main/java/org/apache/ignite/logger/java/JavaLogger.java
@@ -117,6 +117,7 @@ public class JavaLogger implements IgniteLoggerEx {
 
     /** Path to configuration file. */
     @GridToStringExclude
+    @SuppressWarnings("FieldAccessedSynchronizedAndUnsynchronized")
     private String cfg;
 
     /** Quiet flag. */
@@ -209,12 +210,10 @@ public JavaLogger(final Logger impl, boolean configure) {
      * Creates new logger with given parameters.
      *
      * @param impl Java Logging implementation to use.
-     * @param quiet Quiet mode.
      * @param cfg Path to configuration.
      */
-    private JavaLogger(Logger impl, boolean quiet, String cfg) {
-        this.impl = impl;
-        this.quiet = quiet;
+    private JavaLogger(Logger impl, String cfg) {
+        this(impl, true);
         this.cfg = cfg;
     }
 
@@ -224,7 +223,7 @@ private JavaLogger(Logger impl, boolean quiet, String cfg) {
             ? Logger.getLogger("")
             : Logger.getLogger(ctgr instanceof Class
                 ? ((Class<?>)ctgr).getName()
-                : String.valueOf(ctgr)), quiet, cfg);
+                : String.valueOf(ctgr)), cfg);
     }
 
     /**

diff --git a/modules/core/src/test/java/org/apache/ignite/logger/java/JavaLoggerTest.java b/modules/core/src/test/java/org/apache/ignite/logger/java/JavaLoggerTest.java
@@ -20,6 +20,8 @@
 import java.io.File;
 import java.util.UUID;
 import java.util.logging.LogManager;
+import java.util.logging.Logger;
+import org.apache.ignite.Ignite;
 import org.apache.ignite.IgniteLogger;
 import org.apache.ignite.configuration.IgniteConfiguration;
 import org.apache.ignite.internal.logger.IgniteLoggerEx;
@@ -50,7 +52,6 @@ public class JavaLoggerTest extends GridCommonAbstractTest {
         GridTestUtils.setFieldValue(JavaLogger.class, JavaLogger.class, "inited", false);
     }
 
-
     /**
      * Check JavaLogger default constructor.
      */
@@ -67,26 +68,48 @@ public void testDefaultConstructorWithDefaultConfig() {
     }
 
     /**
-     * Check JavaLogger constructor from java.util.logging.config.file property.
+     * Check non-configured constructor of JavaLogger.
      */
     @Test
-    public void testDefaultConstructorWithProperty() throws Exception {
-        File file = new File(U.getIgniteHome(), LOG_CONFIG_DEBUG);
-        System.setProperty("java.util.logging.config.file", file.getPath());
-        // Call readConfiguration explicitly because Logger.getLogger was already called during IgniteUtils initialization
-        LogManager.getLogManager().readConfiguration();
-
-        IgniteLogger log1 = new JavaLogger();
+    public void testNotInitializedLogger() {
+        IgniteLogger log1 = new JavaLogger(Logger.getAnonymousLogger(), false);
         assertTrue(log1.toString().contains("JavaLogger"));
-        assertTrue(log1.toString().contains(LOG_CONFIG_DEBUG));
-        assertTrue(log1.isDebugEnabled());
+        assertTrue(log1.toString().contains("null"));
 
         IgniteLogger log2 = log1.getLogger(getClass());
         assertTrue(log2.toString().contains("JavaLogger"));
-        assertTrue(log2.toString().contains(LOG_CONFIG_DEBUG));
-        assertTrue(log1.isDebugEnabled());
+        assertTrue(log2.toString().contains("null"));
+    }
 
-        System.clearProperty("java.util.logging.config.file");
+    /**
+     * Check JavaLogger constructor from java.util.logging.config.file property.
+     */
+    @Test
+    public void testDefaultConstructorWithProperty() throws Exception {
+        String cfgPathProp = "java.util.logging.config.file";
+        String oldPropVal = System.getProperty(cfgPathProp);
+        try {
+            File file = new File(U.getIgniteHome(), LOG_CONFIG_DEBUG);
+            System.setProperty(cfgPathProp, file.getPath());
+            // Call readConfiguration explicitly because Logger.getLogger was already called during IgniteUtils initialization.
+            LogManager.getLogManager().readConfiguration();
+
+            IgniteLogger log1 = new JavaLogger();
+            assertTrue(log1.toString().contains("JavaLogger"));
+            assertTrue(log1.toString().contains(LOG_CONFIG_DEBUG));
+            assertTrue(log1.isDebugEnabled());
+
+            IgniteLogger log2 = log1.getLogger(getClass());
+            assertTrue(log2.toString().contains("JavaLogger"));
+            assertTrue(log2.toString().contains(LOG_CONFIG_DEBUG));
+            assertTrue(log1.isDebugEnabled());
+        }
+        finally {
+            if (oldPropVal == null)
+                System.clearProperty(cfgPathProp);
+            else
+                System.setProperty(cfgPathProp, oldPropVal);
+        }
     }
 
     /**
@@ -101,10 +124,9 @@ public void testGridLoggingWithDefaultLogger() throws Exception {
 
         IgniteConfiguration cfg = getConfiguration(getTestIgniteInstanceName());
         cfg.setGridLogger(log);
-        startGrid(cfg);
-
-        assertTrue(GridTestUtils.waitForCondition(lsn::check, 2_000));
-        stopAllGrids();
+        try (Ignite ign = startGrid(cfg)) {
+            assertTrue(lsn.check());
+        }
     }
 
     /**
@@ -114,11 +136,9 @@ public void testGridLoggingWithDefaultLogger() throws Exception {
     public void testLogInitialize() throws Exception {
         JavaLogger log = new JavaLogger();
 
-        ((JavaLogger)log).setWorkDirectory(U.defaultWorkDirectory());
+        log.setWorkDirectory(U.defaultWorkDirectory());
         ((IgniteLoggerEx)log).setApplicationAndNode(null, UUID.fromString("00000000-1111-2222-3333-444444444444"));
 
-        System.out.println(log.toString());
-
         assertTrue(log.toString().contains("JavaLogger"));
         assertTrue(log.toString().contains(DFLT_CONFIG_PATH));
 
@@ -141,12 +161,11 @@ public void testLogInitialize() throws Exception {
         assert !log.fileName().contains("%");
         assert log.fileName().contains("ignite");
 
-        System.clearProperty("java.util.logging.config.file");
         GridTestUtils.setFieldValue(JavaLogger.class, JavaLogger.class, "inited", false);
 
         log = new JavaLogger();
 
-        ((JavaLogger)log).setWorkDirectory(U.defaultWorkDirectory());
+        log.setWorkDirectory(U.defaultWorkDirectory());
         ((IgniteLoggerEx)log).setApplicationAndNode("other-app", UUID.fromString("00000000-1111-2222-3333-444444444444"));
 
         assert log.fileName() != null;

diff --git a/...core/src/test/java/org/apache/ignite/testframework/junits/logger/GridTestLog4jLogger.java b/...core/src/test/java/org/apache/ignite/testframework/junits/logger/GridTestLog4jLogger.java
@@ -273,7 +273,6 @@ public GridTestLog4jLogger(final URL cfgUrl) throws IgniteCheckedException {
 
     /**
      * Creates new logger with given implementation.
-     *
      */
     private GridTestLog4jLogger(final Logger impl, String cfg) {
         assert impl != null;
@@ -514,7 +513,7 @@ public static Collection<String> logFiles() {
         if (!impl.isTraceEnabled())
             warning("Logging at TRACE level without checking if TRACE level is enabled: " + msg);
 
-            assert impl.isTraceEnabled() : "Logging at TRACE level without checking if TRACE level is enabled: " + msg;
+        assert impl.isTraceEnabled() : "Logging at TRACE level without checking if TRACE level is enabled: " + msg;
 
         impl.trace(msg);
     }

diff --git a/.../test/java/org/apache/ignite/testframework/junits/logger/GridTestLog4jLoggerSelfTest.java b/.../test/java/org/apache/ignite/testframework/junits/logger/GridTestLog4jLoggerSelfTest.java
@@ -20,7 +20,6 @@
 import java.io.File;
 import java.net.URL;
 import java.util.UUID;
-
 import org.apache.ignite.IgniteLogger;
 import org.apache.ignite.internal.logger.IgniteLoggerEx;
 import org.apache.ignite.internal.util.lang.RunnableX;
@@ -29,8 +28,8 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.core.LoggerContext;
 import org.apache.logging.log4j.core.config.Configurator;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -58,16 +57,18 @@ public class GridTestLog4jLoggerSelfTest {
     private static final Level defaultRootLevel = LogManager.getRootLogger().getLevel();
 
     /** */
-    @BeforeClass
-    public static void beforeTests() {
+    @Before
+    public void beforeTest() {
+        System.clearProperty("appId");
+        LogManager.shutdown();
+        GridTestUtils.setFieldValue(GridTestLog4jLogger.class, GridTestLog4jLogger.class, "inited", false);
         Configurator.setRootLevel(Level.WARN);
     }
 
     /** */
-    @AfterClass
-    public static void afterTests() {
+    @After
+    public void afterTest() {
         Configurator.setRootLevel(defaultRootLevel);
-
         assertEquals(defaultRootLevel, LoggerContext.getContext(false).getConfiguration().getRootLogger().getLevel());
     }
 
@@ -77,16 +78,13 @@ public void testFileConstructor() throws Exception {
         File xml = GridTestUtils.resolveIgnitePath(LOG_PATH_TEST);
 
         assert xml != null;
-        assert xml.exists();
-
-        IgniteLogger log = new GridTestLog4jLogger(xml).getLogger(getClass());
 
-        System.out.println(log.toString());
+        IgniteLoggerEx log = new GridTestLog4jLogger(xml).getLogger(getClass());
 
         assertTrue(log.toString().contains("GridTestLog4jLogger"));
         assertTrue(log.toString().contains(xml.getPath()));
 
-        ((IgniteLoggerEx)log).setApplicationAndNode(null, UUID.randomUUID());
+        log.setApplicationAndNode(null, UUID.randomUUID());
 
         checkLog(log);
     }
@@ -97,17 +95,14 @@ public void testUrlConstructor() throws Exception {
         File xml = GridTestUtils.resolveIgnitePath(LOG_PATH_TEST);
 
         assert xml != null;
-        assert xml.exists();
 
         URL url = xml.toURI().toURL();
-        IgniteLogger log = new GridTestLog4jLogger(url).getLogger(getClass());
-
-        System.out.println(log.toString());
+        IgniteLoggerEx log = new GridTestLog4jLogger(url).getLogger(getClass());
 
         assertTrue(log.toString().contains("GridTestLog4jLogger"));
         assertTrue(log.toString().contains(url.getPath()));
 
-        ((IgniteLoggerEx)log).setApplicationAndNode(null, UUID.randomUUID());
+        log.setApplicationAndNode(null, UUID.randomUUID());
 
         checkLog(log);
     }
@@ -117,14 +112,12 @@ public void testUrlConstructor() throws Exception {
      */
     @Test
     public void testPathConstructor() throws Exception {
-        IgniteLogger log = new GridTestLog4jLogger(LOG_PATH_TEST).getLogger(getClass());
-
-        System.out.println(log.toString());
+        IgniteLoggerEx log = new GridTestLog4jLogger(LOG_PATH_TEST).getLogger(getClass());
 
         assertTrue(log.toString().contains("GridTestLog4jLogger"));
         assertTrue(log.toString().contains(LOG_PATH_TEST));
 
-        ((IgniteLoggerEx)log).setApplicationAndNode(null, UUID.randomUUID());
+        log.setApplicationAndNode(null, UUID.randomUUID());
 
         checkLog(log);
     }
@@ -158,7 +151,7 @@ public void testTrace() {
 
     /** */
     private static void tryLog(RunnableX clo, Level level) {
-        String assertionMsg = format(ASSERTION_FORMAT_MSG, level.toString(), level.toString());
+        String assertionMsg = format(ASSERTION_FORMAT_MSG, level.toString(), level);
 
         GridTestUtils.assertThrows(null, clo, AssertionError.class, assertionMsg);
     }

diff --git a/modules/log4j2/src/test/java/org/apache/ignite/logger/log4j2/Log4j2LoggerSelfTest.java b/modules/log4j2/src/test/java/org/apache/ignite/logger/log4j2/Log4j2LoggerSelfTest.java
@@ -61,16 +61,13 @@ public void testFileConstructor() throws Exception {
         File xml = GridTestUtils.resolveIgnitePath(LOG_PATH_TEST);
 
         assert xml != null;
-        assert xml.exists();
 
-        IgniteLogger log = new Log4J2Logger(xml).getLogger(getClass());
-
-        System.out.println(log.toString());
+        IgniteLoggerEx log = new Log4J2Logger(xml).getLogger(getClass());
 
         assertTrue(log.toString().contains("Log4J2Logger"));
         assertTrue(log.toString().contains(xml.getPath()));
 
-        ((IgniteLoggerEx)log).setApplicationAndNode(null, UUID.randomUUID());
+        log.setApplicationAndNode(null, UUID.randomUUID());
 
         checkLog(log);
     }
@@ -83,17 +80,14 @@ public void testUrlConstructor() throws Exception {
         File xml = GridTestUtils.resolveIgnitePath(LOG_PATH_TEST);
 
         assert xml != null;
-        assert xml.exists();
 
         URL url = xml.toURI().toURL();
-        IgniteLogger log = new Log4J2Logger(url).getLogger(getClass());
-
-        System.out.println(log.toString());
+        IgniteLoggerEx log = new Log4J2Logger(url).getLogger(getClass());
 
         assertTrue(log.toString().contains("Log4J2Logger"));
         assertTrue(log.toString().contains(url.getPath()));
 
-        ((IgniteLoggerEx)log).setApplicationAndNode(null, UUID.randomUUID());
+        log.setApplicationAndNode(null, UUID.randomUUID());
 
         checkLog(log);
     }
@@ -103,14 +97,12 @@ public void testUrlConstructor() throws Exception {
      */
     @Test
     public void testPathConstructor() throws Exception {
-        IgniteLogger log = new Log4J2Logger(LOG_PATH_TEST).getLogger(getClass());
-
-        System.out.println(log.toString());
+        IgniteLoggerEx log = new Log4J2Logger(LOG_PATH_TEST).getLogger(getClass());
 
         assertTrue(log.toString().contains("Log4J2Logger"));
         assertTrue(log.toString().contains(LOG_PATH_TEST));
 
-        ((IgniteLoggerEx)log).setApplicationAndNode(null, UUID.randomUUID());
+        log.setApplicationAndNode(null, UUID.randomUUID());
 
         checkLog(log);
     }