You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rg...@apache.org on 2019/05/10 04:39:06 UTC

[logging-log4j2] branch release-2.x updated: LOG4J2-913 - Code review changes

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

rgoers pushed a commit to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git


The following commit(s) were added to refs/heads/release-2.x by this push:
     new 6174688  LOG4J2-913 - Code review changes
6174688 is described below

commit 617468875757c6a6cdad1f2183f0813925175e29
Author: Ralph Goers <rg...@apache.org>
AuthorDate: Thu May 9 21:38:34 2019 -0700

    LOG4J2-913 - Code review changes
---
 .../org/apache/logging/log4j/util/Base64Util.java  |   8 +-
 .../org/apache/logging/log4j/util/Base64Util.java  |  15 +--
 .../java/org/apache/logging/log4j/util/Timer.java  |   4 +-
 .../logging/log4j/core/util/WatchEventService.java |   5 +-
 .../logging/log4j/core/util/WatchManager.java      | 108 ++++++++++-----------
 5 files changed, 73 insertions(+), 67 deletions(-)

diff --git a/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/Base64Util.java b/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/Base64Util.java
index 63ac84f..59ba5a2 100644
--- a/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/Base64Util.java
+++ b/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/Base64Util.java
@@ -20,12 +20,16 @@ import java.util.Base64;
 
 
 /**
- * C
+ * Base64 encodes Strings. This utility is only necessary because the mechanism to do this changed in Java 8 and
+ * the original method for Base64 encoding was removed in Java 9.
  */
-public class Base64Util {
+public final class Base64Util {
 
     private static final Base64.Encoder encoder = Base64.getEncoder();
 
+    private Base64Util() {
+    }
+
     public static String encode(String str) {
         return str != null ? encoder.encodeToString(str.getBytes()) : null;
     }
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/Base64Util.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/Base64Util.java
index 66ff989..0134d7a 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util/Base64Util.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/Base64Util.java
@@ -18,12 +18,14 @@ package org.apache.logging.log4j.util;
 
 import java.lang.reflect.Method;
 
+import org.apache.logging.log4j.LoggingException;
 import org.apache.logging.log4j.status.StatusLogger;
 
 /**
- *
+ * Base64 encodes Strings. This utility is only necessary because the mechanism to do this changed in Java 8 and
+ * the original method was removed in Java 9.
  */
-public class Base64Util {
+public final class Base64Util {
 
     private static Method encodeMethod = null;
     private static Object encoder = null;
@@ -45,6 +47,9 @@ public class Base64Util {
         }
     }
 
+    private Base64Util() {
+    }
+
     public static String encode(String str) {
         if (str == null) {
             return null;
@@ -54,11 +59,9 @@ public class Base64Util {
             try {
                 return (String) encodeMethod.invoke(encoder, data);
             } catch (Exception ex) {
-                StatusLogger.getLogger().warn("Unable to encode String: " + ex.getMessage());
-                return str;
+                throw new LoggingException("Unable to encode String", ex);
             }
         }
-        StatusLogger.getLogger().warn("No Encoder, unable to encode string");
-        return str;
+        throw new LoggingException("No Encoder, unable to encode string");
     }
 }
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/Timer.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/Timer.java
index add41cc..5a56482 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util/Timer.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/Timer.java
@@ -20,7 +20,9 @@ import java.io.Serializable;
 import java.text.DecimalFormat;
 
 /**
- *
+ * Primarily used in unit tests, but can be used to track elapsed time for a request or portion of any other operation
+ * so long as all the timer methods are called on the same thread in which it was started. Calling start on
+ * multiple threads will cause the the times to be aggregated.
  */
 public class Timer implements Serializable, StringBuilderFormattable
 {
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchEventService.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchEventService.java
index 31021c8..65d9df2 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchEventService.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchEventService.java
@@ -21,8 +21,7 @@ package org.apache.logging.log4j.core.util;
  */
 public interface WatchEventService {
 
-	void subscribe(WatchManager manager);
-
-	void unsubscribe(WatchManager manager);
+    void subscribe(WatchManager manager);
 
+    void unsubscribe(WatchManager manager);
 }
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchManager.java
index 3e37152..3604bdb 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchManager.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchManager.java
@@ -58,12 +58,12 @@ public class WatchManager extends AbstractLifeCycle {
     }
 
     public UUID getId() {
-    	return this.id;
-	}
+        return this.id;
+    }
 
-	public boolean hasEventListeners() {
-    	return eventServiceList.size() > 0;
-	}
+    public boolean hasEventListeners() {
+        return eventServiceList.size() > 0;
+    }
 
     /**
      * Resets all file monitors to their current last modified time. If this manager does not watch any file, nothing
@@ -77,7 +77,7 @@ public class WatchManager extends AbstractLifeCycle {
      */
     public void reset() {
         logger.debug("Resetting {}", this);
-        for (final Source source: watchers.keySet()) {
+        for (final Source source : watchers.keySet()) {
             reset(source);
         }
     }
@@ -90,8 +90,7 @@ public class WatchManager extends AbstractLifeCycle {
      * given watched file has changed during the period of time when the manager was stopped.
      * </p>
      *
-     * @param file
-     *            the file for the monitor to reset.
+     * @param file the file for the monitor to reset.
      * @since 2.11.0
      */
     public void reset(final File file) {
@@ -102,7 +101,6 @@ public class WatchManager extends AbstractLifeCycle {
         reset(source);
     }
 
-
     /**
      * Resets the configuration monitor for the given file being watched to its current last modified time. If this
      * manager does not watch the given configuration, nothing happens.
@@ -111,9 +109,8 @@ public class WatchManager extends AbstractLifeCycle {
      * given watched configuration has changed during the period of time when the manager was stopped.
      * </p>
      *
-     * @param source
-     *            the Source for the monitor to reset.
-     * @since 2.11.2
+     * @param source the Source for the monitor to reset.
+     * @since 2.12.0
      */
     public void reset(final Source source) {
         if (source == null) {
@@ -138,8 +135,10 @@ public class WatchManager extends AbstractLifeCycle {
         if (!isStarted()) {
             if (this.intervalSeconds > 0 && intervalSeconds == 0) {
                 scheduler.decrementScheduledItems();
-            } else if (this.intervalSeconds == 0 && intervalSeconds > 0) {
-                scheduler.incrementScheduledItems();
+            } else {
+                if (this.intervalSeconds == 0 && intervalSeconds > 0) {
+                    scheduler.incrementScheduledItems();
+                }
             }
             this.intervalSeconds = intervalSeconds;
         }
@@ -159,20 +158,20 @@ public class WatchManager extends AbstractLifeCycle {
         super.start();
 
         if (intervalSeconds > 0) {
-            future = scheduler.scheduleWithFixedDelay(new WatchRunnable(), intervalSeconds, intervalSeconds,
-                    TimeUnit.SECONDS);
+            future = scheduler
+                .scheduleWithFixedDelay(new WatchRunnable(), intervalSeconds, intervalSeconds, TimeUnit.SECONDS);
         }
         for (WatchEventService service : eventServiceList) {
-        	service.subscribe(this);
-		}
+            service.subscribe(this);
+        }
     }
 
     @Override
     public boolean stop(final long timeout, final TimeUnit timeUnit) {
         setStopping();
-		for (WatchEventService service : eventServiceList) {
-			service.unsubscribe(this);
-		}
+        for (WatchEventService service : eventServiceList) {
+            service.unsubscribe(this);
+        }
         final boolean stopped = stop(future);
         setStopped();
         return stopped;
@@ -181,8 +180,7 @@ public class WatchManager extends AbstractLifeCycle {
     /**
      * Unwatches the given file.
      *
-     * @param file
-     *            the file to stop watching.
+     * @param file the file to stop watching.
      * @since 2.11.0
      */
     public void unwatchFile(final File file) {
@@ -194,25 +192,23 @@ public class WatchManager extends AbstractLifeCycle {
      * Unwatches the given file.
      *
      * @param source the Source to stop watching.
-     *            the file to stop watching.
-     * @since 2.11.2
+     *               the file to stop watching.
+     * @since 2.12.0
      */
     public void unwatch(final Source source) {
         logger.debug("Unwatching configuration {}", source);
         watchers.remove(source);
     }
 
-	public void checkFiles() {
-		new WatchRunnable().run();
-	}
+    public void checkFiles() {
+        new WatchRunnable().run();
+    }
 
     /**
      * Watches the given file.
      *
-     * @param file
-     *            the file to watch.
-     * @param fileWatcher
-     *            the watcher to notify of file changes.
+     * @param file        the file to watch.
+     * @param fileWatcher the watcher to notify of file changes.
      */
     public void watchFile(final File file, final FileWatcher fileWatcher) {
         Watcher watcher;
@@ -228,21 +224,22 @@ public class WatchManager extends AbstractLifeCycle {
     /**
      * Watches the given file.
      *
-     * @param source the source to watch.
-     * @param watcher
-     *            the watcher to notify of file changes.
+     * @param source  the source to watch.
+     * @param watcher the watcher to notify of file changes.
      */
     public void watch(final Source source, final Watcher watcher) {
         watcher.watching(source);
         final long lastModified = watcher.getLastModified();
         if (logger.isDebugEnabled()) {
-            logger.debug("Watching configuration '{}' for lastModified {} ({})", source, millisToString(lastModified), lastModified);
+            logger.debug("Watching configuration '{}' for lastModified {} ({})", source, millisToString(lastModified),
+                lastModified);
         }
         watchers.put(source, new ConfigurationMonitor(lastModified, watcher));
     }
 
     /**
      * Returns a Map of the file watchers.
+     *
      * @return A Map of the file watchers.
      * @deprecated use getConfigurationWatchers.
      */
@@ -252,7 +249,7 @@ public class WatchManager extends AbstractLifeCycle {
             if (entry.getValue().getWatcher() instanceof ConfigurationFileWatcher) {
                 map.put(entry.getKey().getFile(), (FileWatcher) entry.getValue().getWatcher());
             } else {
-                map.put(entry.getKey().getFile(), new WrappedFileWatcher((FileWatcher)entry.getValue().getWatcher()));
+                map.put(entry.getKey().getFile(), new WrappedFileWatcher((FileWatcher) entry.getValue().getWatcher()));
             }
         }
         return map;
@@ -260,6 +257,7 @@ public class WatchManager extends AbstractLifeCycle {
 
     /**
      * Return the ConfigurationWaatchers.
+     *
      * @return the ConfigurationWatchers.
      * @since 2.11.2
      */
@@ -275,21 +273,21 @@ public class WatchManager extends AbstractLifeCycle {
         return new Date(millis).toString();
     }
 
-	private List<WatchEventService> getEventServices() {
-    	List<WatchEventService> list = new ArrayList<>();
-		for (final ClassLoader classLoader : LoaderUtil.getClassLoaders()) {
-			try {
-				final ServiceLoader<WatchEventService > serviceLoader =
-					ServiceLoader.load(WatchEventService.class, classLoader);
-				for (final WatchEventService service : serviceLoader) {
-					list.add(service);
-				}
-			} catch (final Throwable ex) {
-				LOGGER.debug("Unable to retrieve WatchEventService from ClassLoader {}", classLoader, ex);
-			}
-		}
-		return list;
-	}
+    private List<WatchEventService> getEventServices() {
+        List<WatchEventService> list = new ArrayList<>();
+        for (final ClassLoader classLoader : LoaderUtil.getClassLoaders()) {
+            try {
+                final ServiceLoader<WatchEventService> serviceLoader = ServiceLoader
+                    .load(WatchEventService.class, classLoader);
+                for (final WatchEventService service : serviceLoader) {
+                    list.add(service);
+                }
+            } catch (final Throwable ex) {
+                LOGGER.debug("Unable to retrieve WatchEventService from ClassLoader {}", classLoader, ex);
+            }
+        }
+        return list;
+    }
 
     private final class WatchRunnable implements Runnable {
 
@@ -306,8 +304,8 @@ public class WatchManager extends AbstractLifeCycle {
                     final long lastModified = monitor.getWatcher().getLastModified();
                     if (logger.isInfoEnabled()) {
                         logger.info("Source '{}' was modified on {} ({}), previous modification was on {} ({})", source,
-                                millisToString(lastModified), lastModified, millisToString(monitor.lastModifiedMillis),
-                                monitor.lastModifiedMillis);
+                            millisToString(lastModified), lastModified, millisToString(monitor.lastModifiedMillis),
+                            monitor.lastModifiedMillis);
                     }
                     monitor.lastModifiedMillis = lastModified;
                     monitor.getWatcher().modified();
@@ -344,6 +342,6 @@ public class WatchManager extends AbstractLifeCycle {
     @Override
     public String toString() {
         return "WatchManager [intervalSeconds=" + intervalSeconds + ", watchers=" + watchers + ", scheduler="
-                + scheduler + ", future=" + future + "]";
+            + scheduler + ", future=" + future + "]";
     }
 }