[PAXLOGGING-314] Do not clear the configuration, when one is specifie…

…d using org.ops4j.pax.logging.property.file property
ops4j · Jul 1, 2020 · 3ce5dfc · 3ce5dfc
1 parent 4c24ece
commit 3ce5dfc
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 15 deletions.
diff --git a/pax-logging-log4j1/src/main/java/org/ops4j/pax/logging/log4j1/internal/Activator.java b/pax-logging-log4j1/src/main/java/org/ops4j/pax/logging/log4j1/internal/Activator.java
@@ -130,7 +130,8 @@ public void start(BundleContext bundleContext) throws Exception {
                 }
             }
 
-            m_PaxLogging.updated(config);
+            m_PaxLogging.setDefaultConfiguration(config);
+            m_PaxLogging.updated(null);
         }
 
         if (cm) {

diff --git a/...ing-log4j1/src/main/java/org/ops4j/pax/logging/log4j1/internal/PaxLoggingServiceImpl.java b/...ing-log4j1/src/main/java/org/ops4j/pax/logging/log4j1/internal/PaxLoggingServiceImpl.java
@@ -55,30 +55,32 @@
  */
 public class PaxLoggingServiceImpl implements PaxLoggingService, ServiceFactory<Object> {
 
-    private BundleContext m_bundleContext;
+    private final BundleContext m_bundleContext;
 
     private volatile ReadWriteLock m_configLock;
-    private boolean locking = true;
+    private final boolean locking = true;
 
     // LogReaderService registration as defined by org.osgi.service.log package
-    private LogReaderServiceImpl m_logReader;
+    private final LogReaderServiceImpl m_logReader;
 
     // pax-logging-log4j1 specific PaxContext for all MDC access
-    private PaxContext m_context;
+    private final PaxContext m_context;
 
     // optional bridging into Event Admin service
-    private EventAdminPoster m_eventAdmin;
+    private final EventAdminPoster m_eventAdmin;
 
     // optional notification mechanism for configuration events
     private final ConfigurationNotifier m_configNotifier;
 
     // Log level (actually a threashold) for this entire service.
-    private LogLevel m_r7LogLevel = LogLevel.DEBUG;
+    private final LogLevel m_r7LogLevel = LogLevel.DEBUG;
 
     // there's no need to run configureDefaults() more than once. That was happening in constructor
     // and millisecond later during registration of ManagedService, upon receiving empty org.ops4j.pax.logging
     // configuration
-    private AtomicBoolean emptyConfiguration = new AtomicBoolean(false);
+    private final AtomicBoolean emptyConfiguration = new AtomicBoolean(false);
+
+    private Dictionary<String, String> defaultConfiguration = null;
 
     public PaxLoggingServiceImpl(BundleContext context, LogReaderServiceImpl logReader, EventAdminPoster eventAdmin, ConfigurationNotifier configNotifier) {
         if (context == null)
@@ -245,16 +247,29 @@ private PaxLogger getLogger(Bundle bundle, String category, String fqcn, boolean
         return new PaxLoggerImpl(bundle, log4jLogger, fqcn, this, printfFormatting);
     }
 
+    /**
+     * When there's system/context property specified using {@link PaxLoggingConstants#LOGGING_CFG_PROPERTY_FILE},
+     * and ConfigurationAdmin is available, Pax Logging may first get null configuration. When "default configuration"
+     * is set before that, we'll use it instead of empty configuration.
+     * @param config
+     */
+    public void setDefaultConfiguration(Dictionary<String, String> config) {
+        this.defaultConfiguration = config;
+    }
+
     /**
      * ManagedService-like method but not requiring Configuration Admin
      * @param configuration
      */
     public void updated(Dictionary<String, ?> configuration) {
-        if (configuration == null) {
+        if (configuration == null && defaultConfiguration == null) {
             // mind that there's no synchronization here
             configureDefaults();
             return;
         }
+        if (configuration == null) {
+            configuration = defaultConfiguration;
+        }
 
         Object useLocks = configuration.get(PaxLoggingConstants.PID_CFG_USE_LOCKS);
         if (!"false".equalsIgnoreCase(String.valueOf(useLocks))) {

diff --git a/pax-logging-log4j2/src/main/java/org/ops4j/pax/logging/log4j2/internal/Activator.java b/pax-logging-log4j2/src/main/java/org/ops4j/pax/logging/log4j2/internal/Activator.java
@@ -160,7 +160,8 @@ public void start(BundleContext bundleContext) throws Exception {
                 config.put(PaxLoggingConstants.PID_CFG_LOG4J2_CONFIG_FILE, configFilePath.toAbsolutePath().toString());
             }
 
-            m_PaxLogging.updated(config);
+            m_PaxLogging.setDefaultConfiguration(config);
+            m_PaxLogging.updated(null);
         }
 
         if (cm) {

diff --git a/...ing-log4j2/src/main/java/org/ops4j/pax/logging/log4j2/internal/PaxLoggingServiceImpl.java b/...ing-log4j2/src/main/java/org/ops4j/pax/logging/log4j2/internal/PaxLoggingServiceImpl.java
@@ -101,7 +101,7 @@ public class PaxLoggingServiceImpl implements PaxLoggingService, ServiceFactory<
     // there's no need to run configureDefaults() more than once. That was happening in constructor
     // and millisecond later during registration of ManagedService, upon receiving empty org.ops4j.pax.logging
     // configuration
-    private AtomicBoolean emptyConfiguration = new AtomicBoolean(false);
+    private final AtomicBoolean emptyConfiguration = new AtomicBoolean(false);
 
     private volatile boolean closed;
 
@@ -110,6 +110,7 @@ public class PaxLoggingServiceImpl implements PaxLoggingService, ServiceFactory<
     private final String fqcn = getClass().getName();
 
     private boolean m_async;
+    private Dictionary<String, String> defaultConfiguration = null;
 
     public PaxLoggingServiceImpl(BundleContext bundleContext, LogReaderServiceImpl logReader, EventAdminPoster eventAdmin, ConfigurationNotifier configNotifier) {
         if (bundleContext == null)
@@ -280,6 +281,16 @@ private PaxLogger getLogger(Bundle bundle, String category, String fqcn, boolean
         return new PaxLoggerImpl(bundle, log4j2Logger, fqcn, this, printfFormatting);
     }
 
+    /**
+     * When there's system/context property specified using {@link PaxLoggingConstants#LOGGING_CFG_PROPERTY_FILE},
+     * and ConfigurationAdmin is available, Pax Logging may first get null configuration. When "default configuration"
+     * is set before that, we'll use it instead of empty configuration.
+     * @param config
+     */
+    public void setDefaultConfiguration(Dictionary<String, String> config) {
+        this.defaultConfiguration = config;
+    }
+
     /**
      * ManagedService-like method but not requiring Configuration Admin
      * @param configuration
@@ -288,10 +299,13 @@ synchronized void updated(Dictionary<String, ?> configuration) {
         if (closed) {
             return;
         }
-        if (configuration == null) {
+        if (configuration == null && defaultConfiguration == null) {
             configureDefaults();
             return;
         }
+        if (configuration == null) {
+            configuration = defaultConfiguration;
+        }
 
         Object useLocks = configuration.get(PaxLoggingConstants.PID_CFG_USE_LOCKS);
         if (!"false".equalsIgnoreCase(String.valueOf(useLocks))) {

diff --git a/pax-logging-logback/src/main/java/org/ops4j/pax/logging/logback/internal/Activator.java b/pax-logging-logback/src/main/java/org/ops4j/pax/logging/logback/internal/Activator.java
@@ -118,7 +118,8 @@ public void start(BundleContext bundleContext) throws Exception {
             Dictionary<String, String> config = new Hashtable<>();
             config.put(PaxLoggingConstants.PID_CFG_LOGBACK_CONFIG_FILE, configFilePath.toAbsolutePath().toString());
 
-            m_paxLogging.updated(config);
+            m_paxLogging.setDefaultConfiguration(config);
+            m_paxLogging.updated(null);
         }
 
         if (cm) {

diff --git a/...g-logback/src/main/java/org/ops4j/pax/logging/logback/internal/PaxLoggingServiceImpl.java b/...g-logback/src/main/java/org/ops4j/pax/logging/logback/internal/PaxLoggingServiceImpl.java
@@ -121,13 +121,15 @@ public class PaxLoggingServiceImpl implements PaxLoggingService, ServiceFactory<
     // there's no need to run configureDefaults() more than once. That was happening in constructor
     // and millisecond later during registration of ManagedService, upon receiving empty org.ops4j.pax.logging
     // configuration
-    private AtomicBoolean emptyConfiguration = new AtomicBoolean(false);
+    private final AtomicBoolean emptyConfiguration = new AtomicBoolean(false);
 
     // pax-logging-log4j1 uses org.apache.log4j.helpers.LogLog, here we'll directly use fallback logger
     private final PaxLogger logLog;
 
     private final String fqcn = getClass().getName();
 
+    private Dictionary<String, String> defaultConfiguration = null;
+
     public PaxLoggingServiceImpl(BundleContext bundleContext, LogReaderServiceImpl logReader,
                                  EventAdminPoster eventAdmin, ConfigurationNotifier configNotifier,
                                  PaxLogger logLog) {
@@ -316,18 +318,31 @@ private PaxLogger getLogger(Bundle bundle, String category, String fqcn, boolean
         return new PaxLoggerImpl(bundle, logbackLogger, fqcn, this, printfFormatting);
     }
 
+    /**
+     * When there's system/context property specified using {@link PaxLoggingConstants#LOGGING_CFG_PROPERTY_FILE},
+     * and ConfigurationAdmin is available, Pax Logging may first get null configuration. When "default configuration"
+     * is set before that, we'll use it instead of empty configuration.
+     * @param config
+     */
+    public void setDefaultConfiguration(Dictionary<String, String> config) {
+        this.defaultConfiguration = config;
+    }
+
     /**
      * ManagedService-like method but not requiring Configuration Admin
      * @param configuration
      */
     public void updated(Dictionary<String, ?> configuration) {
-        if (configuration == null) {
+        if (configuration == null && defaultConfiguration == null) {
             // maintain the existing configuration if there's such file set
             if (m_staticConfigFile == null) {
                 configureDefaults();
             }
             return;
         }
+        if (configuration == null) {
+            configuration = defaultConfiguration;
+        }
 
         Object useLocks = configuration.get(PaxLoggingConstants.PID_CFG_USE_LOCKS);
         if (!"false".equalsIgnoreCase(String.valueOf(useLocks))) {