You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by or...@apache.org on 2015/07/06 18:41:31 UTC

svn commit: r1689454 - in /qpid/java/trunk: broker-core/src/main/java/org/apache/qpid/server/logging/ broker-core/src/main/java/org/apache/qpid/server/logging/logback/ broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/ broker-plugi...

Author: orudyy
Date: Mon Jul  6 16:41:30 2015
New Revision: 1689454

URL: http://svn.apache.org/r1689454
Log:
QPID-6625: [Java Broker] Wire up the existing JMX logging interface to the new model

Removed:
    qpid/java/trunk/systests/src/test/java/org/apache/qpid/systest/management/jmx/LoggingManagementTest.java
Modified:
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerFileLogger.java
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/logback/RolloverWatcher.java
    qpid/java/trunk/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementPluginImpl.java
    qpid/java/trunk/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/mbeans/LoggingManagementMBean.java
    qpid/java/trunk/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/mbeans/LoggingManagementMBeanTest.java
    qpid/java/trunk/systests/src/test/java/org/apache/qpid/server/BrokerStartupTest.java
    qpid/java/trunk/systests/src/test/java/org/apache/qpid/util/LogMonitor.java
    qpid/java/trunk/systests/src/test/java/org/apache/qpid/util/LogMonitorTest.java
    qpid/java/trunk/test-profiles/JavaExcludes

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerFileLogger.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerFileLogger.java?rev=1689454&r1=1689453&r2=1689454&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerFileLogger.java (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerFileLogger.java Mon Jul  6 16:41:30 2015
@@ -35,6 +35,7 @@ import org.apache.qpid.server.model.Cont
 public interface BrokerFileLogger<X extends BrokerFileLogger<X>> extends BrokerLogger<X>
 {
     String TYPE = "File";
+    String FILE_NAME = "fileName";
 
     @ManagedAttribute( defaultValue = "${qpid.work_dir}${file.separator}log${file.separator}qpid.log")
     String getFileName();

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/logback/RolloverWatcher.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/logback/RolloverWatcher.java?rev=1689454&r1=1689453&r2=1689454&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/logback/RolloverWatcher.java (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/logback/RolloverWatcher.java Mon Jul  6 16:41:30 2015
@@ -144,6 +144,11 @@ public class RolloverWatcher implements
 
     public ZippedContent getFilesAsZippedContent(Set<String> fileNames)
     {
+        if (fileNames == null)
+        {
+            throw new IllegalArgumentException("File name cannot be null");
+        }
+
         Map<String, Path> paths = new TreeMap<>();
         for (String name: fileNames)
         {

Modified: qpid/java/trunk/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementPluginImpl.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementPluginImpl.java?rev=1689454&r1=1689453&r2=1689454&view=diff
==============================================================================
--- qpid/java/trunk/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementPluginImpl.java (original)
+++ qpid/java/trunk/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/JMXManagementPluginImpl.java Mon Jul  6 16:41:30 2015
@@ -34,6 +34,9 @@ import javax.management.JMException;
 
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
+import org.apache.qpid.server.logging.BrokerFileLogger;
+import org.apache.qpid.server.logging.log4j.LoggingManagementFacade;
+import org.apache.qpid.server.model.BrokerLogger;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -43,7 +46,6 @@ import org.apache.qpid.server.jmx.mbeans
 import org.apache.qpid.server.jmx.mbeans.Shutdown;
 import org.apache.qpid.server.jmx.mbeans.UserManagementMBean;
 import org.apache.qpid.server.jmx.mbeans.VirtualHostMBean;
-import org.apache.qpid.server.logging.log4j.LoggingManagementFacade;
 import org.apache.qpid.server.model.AuthenticationProvider;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ConfigurationChangeListener;
@@ -86,6 +88,7 @@ public class JMXManagementPluginImpl
     private final Set<MBeanProvider> _mBeanProviders;
     private final ChangeListener _changeListener;
     private final PluginMBeansProvider _pluginMBeanProvider;
+    private LoggingManagementMBean _loggingManagementMBean;
 
     @ManagedObjectFactoryConstructor
     public JMXManagementPluginImpl(Map<String, Object> attributes, Broker<?> broker)
@@ -169,13 +172,15 @@ public class JMXManagementPluginImpl
             {
                 createObjectMBeans(authenticationProvider);
             }
+            Collection<BrokerLogger> brokerLoggers = broker.getChildren(BrokerLogger.class);
+            for (BrokerLogger brokerLogger : brokerLoggers)
+            {
+                createObjectMBeans(brokerLogger);
+            }
         }
         new Shutdown(_objectRegistry);
         new ServerInformationMBean(_objectRegistry, broker);
-        if (LoggingManagementFacade.getCurrentInstance() != null)
-        {
-            new LoggingManagementMBean(LoggingManagementFacade.getCurrentInstance(), _objectRegistry);
-        }
+
         _objectRegistry.start();
         setState(State.ACTIVE);
         _allowPortActivation = false;
@@ -211,6 +216,7 @@ public class JMXManagementPluginImpl
         }
         getBroker().removeChangeListener(_changeListener);
         closeObjectRegistry();
+        _loggingManagementMBean = null;
     }
 
     private void unregisterObjectMBeans(ConfiguredObject<?> object)
@@ -340,6 +346,19 @@ public class JMXManagementPluginImpl
                         mbean = new UserManagementMBean((PasswordCredentialManagingAuthenticationProvider<?>) object, _objectRegistry);
                         registerMBean(object, _pluginMBeanProvider, mbean);
                     }
+                    else if (object instanceof BrokerFileLogger)
+                    {
+                        if (_loggingManagementMBean == null)
+                        {
+                            _loggingManagementMBean = new LoggingManagementMBean((BrokerFileLogger) object, _objectRegistry);
+                            LOGGER.info("LoggingManagementMBean created for BrokerFileLogger '{}'", object.getName());
+                        }
+                        else
+                        {
+                            LOGGER.warn("There are multiple BrokerFileLoggers. LoggingManagementMBean was already created. Ignoring BrokerFileLogger '{}'", object.getName());
+                        }
+                    }
+
                 }
                 createAdditionalMBeansFromProvidersIfNecessary(object, _objectRegistry);
             }
@@ -408,7 +427,10 @@ public class JMXManagementPluginImpl
                     object.removeChangeListener(_changeListener);
                 }
                 unregisterObjectMBeans(object);
-                _children.remove(object);
+                if (_children.remove(object) == _loggingManagementMBean)
+                {
+                    _loggingManagementMBean = null;
+                }
                 destroyChildrenMBeans(object);
             }
         }
@@ -443,7 +465,10 @@ public class JMXManagementPluginImpl
 
     private boolean supportedConfiguredObject(ConfiguredObject<?> object)
     {
-        return object instanceof VirtualHostNode || object instanceof VirtualHost || object instanceof PasswordCredentialManagingAuthenticationProvider;
+        return (object instanceof VirtualHostNode ||
+                object instanceof VirtualHost ||
+                object instanceof PasswordCredentialManagingAuthenticationProvider ||
+                object instanceof BrokerFileLogger);
     }
 
     private class PluginMBeansProvider implements MBeanProvider

Modified: qpid/java/trunk/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/mbeans/LoggingManagementMBean.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/mbeans/LoggingManagementMBean.java?rev=1689454&r1=1689453&r2=1689454&view=diff
==============================================================================
--- qpid/java/trunk/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/mbeans/LoggingManagementMBean.java (original)
+++ qpid/java/trunk/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/mbeans/LoggingManagementMBean.java Mon Jul  6 16:41:30 2015
@@ -20,10 +20,15 @@
  */
 package org.apache.qpid.server.jmx.mbeans;
 
+import static ch.qos.logback.classic.Logger.ROOT_LOGGER_NAME;
+
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.TreeMap;
 
 import javax.management.JMException;
 import javax.management.openmbean.CompositeData;
@@ -36,6 +41,10 @@ import javax.management.openmbean.Tabula
 import javax.management.openmbean.TabularDataSupport;
 import javax.management.openmbean.TabularType;
 
+import org.apache.qpid.server.logging.BrokerFileLogger;
+import org.apache.qpid.server.logging.BrokerNameAndLevelFilter;
+import org.apache.qpid.server.logging.LogLevel;
+import org.apache.qpid.server.model.BrokerLoggerFilter;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -44,8 +53,6 @@ import org.apache.qpid.management.common
 import org.apache.qpid.server.jmx.AMQManagedObject;
 import org.apache.qpid.server.jmx.ManagedObject;
 import org.apache.qpid.server.jmx.ManagedObjectRegistry;
-import org.apache.qpid.server.logging.log4j.LoggingFacadeException;
-import org.apache.qpid.server.logging.log4j.LoggingManagementFacade;
 import org.apache.qpid.server.util.ConnectionScopedRuntimeException;
 
 
@@ -53,13 +60,10 @@ import org.apache.qpid.server.util.Conne
 @MBeanDescription("Logging Management Interface")
 public class LoggingManagementMBean extends AMQManagedObject implements LoggingManagement
 {
-    public static final String INHERITED_PSUEDO_LOG_LEVEL = "INHERITED";
     private static final Logger LOGGER = LoggerFactory.getLogger(LoggingManagementMBean.class);
-    private static final TabularType LOGGER_LEVEL_TABULAR_TYE;
+    private static final TabularType LOGGER_LEVEL_TABULAR_TYPE;
     private static final CompositeType LOGGER_LEVEL_COMPOSITE_TYPE;
-
-    private final LoggingManagementFacade _loggingManagementFacade;
-    private final String[] _allAvailableLogLevels;
+    private static final String UNSUPPORTED_LOGGER_FILTER_TYPE = "<UNSUPPORTED>";
 
     static
     {
@@ -72,7 +76,7 @@ public class LoggingManagementMBean exte
                                                          COMPOSITE_ITEM_DESCRIPTIONS.toArray(new String[COMPOSITE_ITEM_DESCRIPTIONS.size()]),
                                                          loggerLevelItemTypes);
 
-            LOGGER_LEVEL_TABULAR_TYE = new TabularType("LoggerLevel", "List of loggers with levels",
+            LOGGER_LEVEL_TABULAR_TYPE = new TabularType("LoggerLevel", "List of loggers with levels",
                                                        LOGGER_LEVEL_COMPOSITE_TYPE,
                                                        TABULAR_UNIQUE_INDEX.toArray(new String[TABULAR_UNIQUE_INDEX.size()]));
         }
@@ -82,13 +86,17 @@ public class LoggingManagementMBean exte
         }
     }
 
-    public LoggingManagementMBean(LoggingManagementFacade loggingManagementFacade, ManagedObjectRegistry registry) throws JMException
+    private final String[] _allAvailableLogLevels;
+    private final BrokerFileLogger _brokerFileLogger;
+
+    public LoggingManagementMBean(BrokerFileLogger brokerFileLogger, ManagedObjectRegistry registry) throws JMException
     {
         super(LoggingManagement.class, LoggingManagement.TYPE, registry);
         register();
-        _loggingManagementFacade = loggingManagementFacade;
-        // QPID-6516 : TODO
-        _allAvailableLogLevels = null;//buildAllAvailableLoggerLevelsWithInheritedPsuedoLogLevel(_loggingManagementFacade);
+        _brokerFileLogger = brokerFileLogger;
+
+        Collection<String> validLogLevels = LogLevel.validValues();
+        _allAvailableLogLevels = validLogLevels.toArray(new String[validLogLevels.size()]);
     }
 
     @Override
@@ -106,7 +114,7 @@ public class LoggingManagementMBean exte
     @Override
     public Integer getLog4jLogWatchInterval()
     {
-        return _loggingManagementFacade.getLog4jLogWatchInterval();
+        return -1;
     }
     
     @Override
@@ -118,216 +126,225 @@ public class LoggingManagementMBean exte
     @Override
     public TabularData viewEffectiveRuntimeLoggerLevels()
     {
-        Map<String, String> levels = _loggingManagementFacade.retrieveRuntimeLoggersLevels();
-        return createTabularDataFromLevelsMap(levels);
+        return getTabularData(findFiltersByDurability(FilterDurability.EITHER));
     }
 
     @Override
     public String getRuntimeRootLoggerLevel()
     {
-        return _loggingManagementFacade.retrieveRuntimeRootLoggerLevel();
+        return getLogLevel(ROOT_LOGGER_NAME, FilterDurability.NONDURABLE);
     }
 
     @Override
     public boolean setRuntimeRootLoggerLevel(String level)
     {
-        try
-        {
-            validateLevelNotAllowingInherited(level);
-        }
-        catch (IllegalArgumentException iae)
-        {
-            LOGGER.warn(level + " is not a known level");
-            return false;
-        }
-
-        _loggingManagementFacade.setRuntimeRootLoggerLevel(level);
-        return true;
+        return setRuntimeLoggerLevel(ROOT_LOGGER_NAME, level);
     }
 
     @Override
     public boolean setRuntimeLoggerLevel(String logger, String level)
     {
-        String validatedLevel;
-        try
-        {
-            validatedLevel = getValidateLevelAllowingInherited(level);
-        }
-        catch (IllegalArgumentException iae)
-        {
-            LOGGER.warn(level + " is not a known level");
-            return false;
-        }
-
-        try
-        {
-            _loggingManagementFacade.setRuntimeLoggerLevel(logger, validatedLevel);
-        }
-        catch (LoggingFacadeException e)
-        {
-            LOGGER.error("Cannot set runtime logging level", e);
-           return false;
-        }
-        return true;
+        return setLogLevel(logger, level, FilterDurability.NONDURABLE);
     }
 
     @Override
     public TabularData viewConfigFileLoggerLevels()
     {
-        Map<String,String> levels;
-        try
-        {
-            levels = _loggingManagementFacade.retrieveConfigFileLoggersLevels();
-        }
-        catch (LoggingFacadeException e)
-        {
-            LOGGER.error("Cannot determine logging levels", e);
-           return null;
-        }
-
-        return createTabularDataFromLevelsMap(levels);
+        return getTabularData(findFiltersByDurability(FilterDurability.DURABLE));
     }
 
     @Override
-    public String getConfigFileRootLoggerLevel()throws IOException
+    public String getConfigFileRootLoggerLevel() throws IOException
     {
-        try
-        {
-            return _loggingManagementFacade.retrieveConfigFileRootLoggerLevel().toUpperCase();
-        }
-        catch (LoggingFacadeException e)
-        {
-            LOGGER.warn("The log4j configuration get config request was aborted: ", e);
-            throw new IOException("The log4j configuration get config request was aborted: " + e.getMessage());
-        }
+        return getLogLevel(ROOT_LOGGER_NAME, FilterDurability.DURABLE);
     }
 
     @Override
     public boolean setConfigFileLoggerLevel(String logger, String level)
     {
-        String validatedLevel;
-        try
-        {
-            validatedLevel = getValidateLevelAllowingInherited(level);
-        }
-        catch (IllegalArgumentException iae)
+        return setLogLevel(logger, level, FilterDurability.DURABLE);
+    }
+
+    @Override
+    public boolean setConfigFileRootLoggerLevel(String level)
+    {
+        return setConfigFileLoggerLevel(ROOT_LOGGER_NAME, level);
+    }
+
+    @Override
+    public void reloadConfigFile() throws IOException
+    {
+        throw new UnsupportedOperationException("Reloading of configuration file is not supported.");
+    }
+
+    private TabularData createTabularDataFromLevelsMap(Map<String, String> levels)
+    {
+        TabularData loggerLevelList = new TabularDataSupport(LOGGER_LEVEL_TABULAR_TYPE);
+        for (Map.Entry<String,String> entry : levels.entrySet())
         {
-            LOGGER.warn(level + " is not a known level");
-            return false;
+            String loggerName = entry.getKey();
+            String level = entry.getValue();
+
+            CompositeData loggerData = createRow(loggerName, level);
+            loggerLevelList.put(loggerData);
         }
+        return loggerLevelList;
+    }
 
+    private CompositeData createRow(String loggerName, String level)
+    {
+        Object[] itemData = {loggerName, level.toUpperCase()};
         try
         {
-            _loggingManagementFacade.setConfigFileLoggerLevel(logger, validatedLevel);
+            CompositeData loggerData = new CompositeDataSupport(LOGGER_LEVEL_COMPOSITE_TYPE,
+                    COMPOSITE_ITEM_NAMES.toArray(new String[COMPOSITE_ITEM_NAMES.size()]), itemData);
+            return loggerData;
         }
-        catch (LoggingFacadeException e)
+        catch (OpenDataException ode)
         {
-            LOGGER.warn("The log4j configuration set config request was aborted: ", e);
-            return false;
+            // Should not happen
+            throw new ConnectionScopedRuntimeException(ode);
         }
-        return true;
     }
-    
-    @Override
-    public boolean setConfigFileRootLoggerLevel(String level)
+
+    private boolean isValidLogLevel(String logLevel)
     {
         try
         {
-            validateLevelNotAllowingInherited(level);
+            LogLevel.valueOf(logLevel);
+            return true;
         }
-        catch (IllegalArgumentException iae)
+        catch (Exception e)
         {
-            LOGGER.warn(level + " is not a known level");
             return false;
         }
+    }
 
-        try
+    private TabularData getTabularData(Collection<BrokerLoggerFilter<?>> filters)
+    {
+        Map<String,String> logToLevelMap = new TreeMap<>();
+        for (BrokerLoggerFilter<?> filter: filters)
         {
-            _loggingManagementFacade.setConfigFileRootLoggerLevel(level);
-            return true;
+            if (filter instanceof BrokerNameAndLevelFilter)
+            {
+                BrokerNameAndLevelFilter<?> nameAndLevelFilter = (BrokerNameAndLevelFilter)filter;
+                logToLevelMap.put(nameAndLevelFilter.getLoggerName(), nameAndLevelFilter.getLevel().name());
+            }
+            else
+            {
+                logToLevelMap.put(UNSUPPORTED_LOGGER_FILTER_TYPE, "");
+            }
         }
-        catch (LoggingFacadeException e)
+        return createTabularDataFromLevelsMap(logToLevelMap);
+    }
+
+    private String getLogLevel(String loggerName, FilterDurability filterDurability)
+    {
+        LogLevel level = LogLevel.OFF;
+        List<BrokerNameAndLevelFilter<?>> filters = findFiltersByLoggerNameAndDurability(loggerName, filterDurability);
+
+        for(BrokerNameAndLevelFilter<?> filter: filters)
         {
-            LOGGER.warn("The log4j configuration set config request was aborted: ", e);
-            return false;
+            final LogLevel filterLevel = filter.getLevel();
+            if (level.compareTo(filterLevel) > 0)
+            {
+                level = filterLevel;
+            }
         }
+        return level.name();
     }
 
-    @Override
-    public void reloadConfigFile() throws IOException
+    private boolean setLogLevel(String logger, String level, FilterDurability durability)
     {
-        try
+        if (!isValidLogLevel(level))
         {
-
-            _loggingManagementFacade.reload();
+            LOGGER.warn("{} is not a known level", level);
+            return false;
         }
-        catch (LoggingFacadeException e)
+
+        List<BrokerNameAndLevelFilter<?>> filters = findFiltersByLoggerNameAndDurability(logger, durability);
+        if (filters.isEmpty())
         {
-            LOGGER.warn("The log4j configuration reload request was aborted: ", e);
-            throw new IOException("The log4j configuration reload request was aborted: " + e.getMessage());
+            LOGGER.warn("There is no logger with name '{}' and durability '{}'", logger, durability.name().toLowerCase());
+            return false;
         }
-    }
 
-    private String getValidateLevelAllowingInherited(String level)
-    {
-        if(level == null
-            || "null".equalsIgnoreCase(level)
-            || INHERITED_PSUEDO_LOG_LEVEL.equalsIgnoreCase(level))
+        LogLevel targetLevel = LogLevel.valueOf(level);
+        for (BrokerNameAndLevelFilter<?> filter: filters)
         {
-            //the string "null" or "inherited" signals to inherit from a parent logger,
-            //using a null Level reference for the logger.
-            return null;
+            try
+            {
+                filter.setAttributes(Collections.<String, Object>singletonMap(BrokerNameAndLevelFilter.LEVEL, targetLevel));
+            }
+            catch(RuntimeException e)
+            {
+                LOGGER.error("Aborting setting runtime logging level due to failure", e);
+                return false;
+            }
         }
 
-        validateLevelNotAllowingInherited(level);
-        return level;
+        return true;
     }
 
-    private void validateLevelNotAllowingInherited(String level)
+    private List<BrokerLoggerFilter<?>> findFiltersByDurability(FilterDurability durability)
     {
-        final List<String> availableLoggerLevels = _loggingManagementFacade.getAvailableLoggerLevels();
-        if (level == null || !availableLoggerLevels.contains(level.toUpperCase()))
+        Collection<BrokerLoggerFilter<?>> filters = _brokerFileLogger.getChildren(BrokerLoggerFilter.class);
+        List<BrokerLoggerFilter<?>> results = new ArrayList<>();
+        if (durability == durability.EITHER)
         {
-            throw new IllegalArgumentException(level + " not known");
+            results.addAll(filters);
         }
+        else
+        {
+            for (BrokerLoggerFilter<?> filter: filters)
+            {
+                if (durability == FilterDurability.valueOf(filter.isDurable()))
+                {
+                    results.add(filter);
+                }
+            }
+        }
+        return results;
     }
 
-    private TabularData createTabularDataFromLevelsMap(Map<String, String> levels)
+    private List<BrokerNameAndLevelFilter<?>> findFiltersByLoggerNameAndDurability(String loggerName, FilterDurability filterDurability)
     {
-        TabularData loggerLevelList = new TabularDataSupport(LOGGER_LEVEL_TABULAR_TYE);
-        for (Map.Entry<String,String> entry : levels.entrySet())
+        List<BrokerNameAndLevelFilter<?>> results = new ArrayList<>();
+        Collection<BrokerLoggerFilter<?>> filters = findFiltersByDurability(filterDurability);
+        String sanitizedLoggerName = sanitizeLoggerName(loggerName);
+        for (BrokerLoggerFilter<?> filter: filters)
         {
-            String loggerName = entry.getKey();
-            String level = entry.getValue();
-
-            CompositeData loggerData = createRow(loggerName, level);
-            loggerLevelList.put(loggerData);
+            if (filter instanceof BrokerNameAndLevelFilter)
+            {
+                BrokerNameAndLevelFilter<?> brokerNameAndLevelFilter = (BrokerNameAndLevelFilter<?>)filter;
+                String filterLoggerName = sanitizeLoggerName(brokerNameAndLevelFilter.getLoggerName());
+                if (sanitizedLoggerName.equals(filterLoggerName))
+                {
+                    results.add(brokerNameAndLevelFilter);
+                }
+            }
         }
-        return loggerLevelList;
+        return results;
     }
 
-    private CompositeData createRow(String loggerName, String level)
+    private String sanitizeLoggerName(String loggerName)
     {
-        Object[] itemData = {loggerName, level.toUpperCase()};
-        try
+        if (loggerName == null || "".equals(loggerName))
         {
-            CompositeData loggerData = new CompositeDataSupport(LOGGER_LEVEL_COMPOSITE_TYPE,
-                    COMPOSITE_ITEM_NAMES.toArray(new String[COMPOSITE_ITEM_NAMES.size()]), itemData);
-            return loggerData;
-        }
-        catch (OpenDataException ode)
-        {
-            // Should not happen
-            throw new ConnectionScopedRuntimeException(ode);
+            return ROOT_LOGGER_NAME;
         }
+        return loggerName;
     }
 
-    private String[] buildAllAvailableLoggerLevelsWithInheritedPsuedoLogLevel(LoggingManagementFacade loggingManagementFacade)
+    private enum FilterDurability
     {
-        List<String> levels = loggingManagementFacade.getAvailableLoggerLevels();
-        List<String> mbeanLevels = new ArrayList<String>(levels);
-        mbeanLevels.add(INHERITED_PSUEDO_LOG_LEVEL);
+        DURABLE,
+        NONDURABLE,
+        EITHER;
 
-        return mbeanLevels.toArray(new String[mbeanLevels.size()]);
+        public static FilterDurability valueOf(boolean durable)
+        {
+            return durable ? DURABLE : NONDURABLE;
+        }
     }
 }

Modified: qpid/java/trunk/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/mbeans/LoggingManagementMBeanTest.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/mbeans/LoggingManagementMBeanTest.java?rev=1689454&r1=1689453&r2=1689454&view=diff
==============================================================================
--- qpid/java/trunk/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/mbeans/LoggingManagementMBeanTest.java (original)
+++ qpid/java/trunk/broker-plugins/management-jmx/src/test/java/org/apache/qpid/server/jmx/mbeans/LoggingManagementMBeanTest.java Mon Jul  6 16:41:30 2015
@@ -20,219 +20,298 @@
  */
 package org.apache.qpid.server.jmx.mbeans;
 
+import static ch.qos.logback.classic.Logger.ROOT_LOGGER_NAME;
+
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
-import static org.mockito.Matchers.anyString;
 
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-import java.util.TreeMap;
+import java.util.HashSet;
+import java.util.Set;
 
 import javax.management.openmbean.CompositeData;
 import javax.management.openmbean.TabularData;
 
 import org.apache.qpid.management.common.mbeans.LoggingManagement;
 import org.apache.qpid.server.jmx.ManagedObjectRegistry;
-import org.apache.qpid.server.logging.log4j.LoggingManagementFacade;
+import org.apache.qpid.server.logging.BrokerFileLogger;
+import org.apache.qpid.server.logging.BrokerNameAndLevelFilter;
+import org.apache.qpid.server.logging.LogLevel;
+import org.apache.qpid.server.model.BrokerLoggerFilter;
 import org.apache.qpid.test.utils.QpidTestCase;
 
 public class LoggingManagementMBeanTest extends QpidTestCase
 {
-    private static final String TEST_LEVEL1 = "LEVEL1";
-    private static final String TEST_LEVEL2 = "LEVEL2";
+    private static final String INHERITED_PSUEDO_LOG_LEVEL = "INHERITED";
+    private static final String TEST_LOGGER_NAME = "org.apache.qpid.test";
+    private static final String UNSUPPORTED_LOG_LEVEL = "unsupported";
 
     private LoggingManagementMBean _loggingMBean;
-    private LoggingManagementFacade _mockLoggingFacade;
     private ManagedObjectRegistry _mockManagedObjectRegistry;
+    private BrokerFileLogger _brokerFileLogger;
 
     @Override
     protected void setUp() throws Exception
     {
-        _mockLoggingFacade = mock(LoggingManagementFacade.class);
-        final List<String> listOfLevels = new ArrayList<String>()
-        {{
-            add(TEST_LEVEL1);
-            add(TEST_LEVEL2);
-        }};
-        when(_mockLoggingFacade.getAvailableLoggerLevels()).thenReturn(listOfLevels);
-
+        super.setUp();
         _mockManagedObjectRegistry = mock(ManagedObjectRegistry.class);
 
-        _loggingMBean = new LoggingManagementMBean(_mockLoggingFacade, _mockManagedObjectRegistry);
+        _brokerFileLogger = mock(BrokerFileLogger.class);
+        _loggingMBean = new LoggingManagementMBean(_brokerFileLogger, _mockManagedObjectRegistry);
     }
 
     public void testMBeanRegistersItself() throws Exception
     {
-        LoggingManagementMBean connectionMBean = new LoggingManagementMBean(_mockLoggingFacade, _mockManagedObjectRegistry);
+        LoggingManagementMBean connectionMBean = new LoggingManagementMBean(_brokerFileLogger, _mockManagedObjectRegistry);
         verify(_mockManagedObjectRegistry).registerObject(connectionMBean);
     }
 
     public void testLog4jLogWatchInterval() throws Exception
     {
-        final Integer value = 5000;
-        when(_mockLoggingFacade.getLog4jLogWatchInterval()).thenReturn(value);
-
-        assertEquals("Unexpected watch interval",value, _loggingMBean.getLog4jLogWatchInterval());
+        assertEquals("Unexpected watch interval", new Integer(-1), _loggingMBean.getLog4jLogWatchInterval());
     }
 
     public void testGetAvailableLoggerLevels()  throws Exception
     {
-        String[] actualLevels = _loggingMBean.getAvailableLoggerLevels();
-        assertEquals(3, actualLevels.length);
-        assertEquals(TEST_LEVEL1, actualLevels[0]);
-        assertEquals(TEST_LEVEL2, actualLevels[1]);
-        assertEquals(LoggingManagementMBean.INHERITED_PSUEDO_LOG_LEVEL, actualLevels[2]);
+        Set<String> expectedLogLevels = new HashSet<>(LogLevel.validValues());
+        Set<String> actualLevels = new HashSet<>(Arrays.asList(_loggingMBean.getAvailableLoggerLevels()));
+        assertEquals(expectedLogLevels, actualLevels);
     }
 
     public void testViewEffectiveRuntimeLoggerLevels()  throws Exception
     {
-        Map<String, String> loggerLevels = new TreeMap<String, String>();
-        loggerLevels.put("a.b.D", TEST_LEVEL2);
-        loggerLevels.put("a.b.C", TEST_LEVEL1);
-        loggerLevels.put("a.b.c.E", TEST_LEVEL2);
+        Collection<BrokerLoggerFilter> filters = new ArrayList<>();
+        filters.add(createMockFilter("a.b.D", LogLevel.DEBUG));
+        filters.add(createMockFilter("a.b.C", LogLevel.INFO));
+        filters.add(createMockFilter("", LogLevel.WARN));
 
-        when(_mockLoggingFacade.retrieveRuntimeLoggersLevels()).thenReturn(loggerLevels );
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(filters);
 
         TabularData table = _loggingMBean.viewEffectiveRuntimeLoggerLevels();
         assertEquals(3, table.size());
 
         final CompositeData row1 = table.get(new String[] {"a.b.C"} );
-        final CompositeData row2 = table.get(new String[] {"a.b.D"} );
-        final CompositeData row3 = table.get(new String[] {"a.b.c.E"} );
-        assertChannelRow(row1, "a.b.C", TEST_LEVEL1);
-        assertChannelRow(row2, "a.b.D", TEST_LEVEL2);
-        assertChannelRow(row3, "a.b.c.E", TEST_LEVEL2);
+        final CompositeData row2 = table.get(new String[]{"a.b.D"});
+        final CompositeData row3 = table.get(new String[]{""});
+        assertChannelRow(row1, "a.b.C", LogLevel.INFO.name());
+        assertChannelRow(row2, "a.b.D", LogLevel.DEBUG.name());
+        assertChannelRow(row3, "", LogLevel.WARN.name());
     }
 
-    public void testGetRuntimeRootLoggerLevel()  throws Exception
+    private BrokerNameAndLevelFilter createMockFilter(String loggerName, LogLevel logLevel, boolean durability)
     {
-        when(_mockLoggingFacade.retrieveRuntimeRootLoggerLevel()).thenReturn(TEST_LEVEL1);
-
-        assertEquals(TEST_LEVEL1, _loggingMBean.getRuntimeRootLoggerLevel());
+        BrokerNameAndLevelFilter filter = mock(BrokerNameAndLevelFilter.class);
+        when(filter.getLevel()).thenReturn(logLevel);
+        when(filter.getLoggerName()).thenReturn(loggerName);
+        when(filter.isDurable()).thenReturn(durability);
+        return filter;
     }
 
-    public void testSetRuntimeRootLoggerLevel()  throws Exception
+    private BrokerNameAndLevelFilter createMockFilter(String loggerName, LogLevel logLevel)
     {
-        _loggingMBean.setRuntimeRootLoggerLevel(TEST_LEVEL1);
-        verify(_mockLoggingFacade).setRuntimeRootLoggerLevel(TEST_LEVEL1);
+        return createMockFilter(loggerName, logLevel, false);
     }
 
-    public void testSetRuntimeRootLoggerLevelWhenLoggingLevelUnknown()  throws Exception
+    public void testGetRuntimeRootLoggerLevel()  throws Exception
     {
-        boolean result = _loggingMBean.setRuntimeRootLoggerLevel("unknown");
-        assertFalse(result);
-        verify(_mockLoggingFacade, never()).setRuntimeRootLoggerLevel("unknown");
+        assertEquals(LogLevel.OFF.toString(), _loggingMBean.getRuntimeRootLoggerLevel());
+
+        BrokerLoggerFilter rootFilterWithNull = createMockFilter(null, LogLevel.WARN);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(rootFilterWithNull));
+        assertEquals(LogLevel.WARN.name(), _loggingMBean.getRuntimeRootLoggerLevel());
+
+        BrokerLoggerFilter rootFilterWithEmptyName = createMockFilter("", LogLevel.ERROR);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(rootFilterWithEmptyName));
+        assertEquals(LogLevel.ERROR.name(), _loggingMBean.getRuntimeRootLoggerLevel());
+
+        BrokerLoggerFilter rootFilterWithRootLoggerName = createMockFilter(ROOT_LOGGER_NAME, LogLevel.DEBUG);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(rootFilterWithRootLoggerName));
+        assertEquals(LogLevel.DEBUG.name(), _loggingMBean.getRuntimeRootLoggerLevel());
     }
 
-    public void testSetRuntimeRootLoggerLevelWhenLoggingLevelInherited()  throws Exception
+    public void testSetRuntimeRootLoggerLevel()  throws Exception
     {
-        boolean result = _loggingMBean.setRuntimeRootLoggerLevel(LoggingManagementMBean.INHERITED_PSUEDO_LOG_LEVEL);
-        assertFalse(result);
-        verify(_mockLoggingFacade, never()).setRuntimeRootLoggerLevel(anyString());
+        assertFalse("Should not be able to set runtime root log level without existing filter",
+                _loggingMBean.setRuntimeRootLoggerLevel(LogLevel.WARN.name()));
+
+        BrokerLoggerFilter durableRootFilter = createMockFilter(ROOT_LOGGER_NAME, LogLevel.DEBUG, true);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(durableRootFilter));
+        assertFalse("Should not be able to set runtime root log level on durable root filter",
+                _loggingMBean.setRuntimeRootLoggerLevel(LogLevel.WARN.name()));
+
+        BrokerLoggerFilter nonDurableRootFilter = createMockFilter(ROOT_LOGGER_NAME, LogLevel.DEBUG, false);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(nonDurableRootFilter));
+        assertTrue("Should be able to set runtime root log level on non-durable root filter",
+                _loggingMBean.setRuntimeRootLoggerLevel(LogLevel.WARN.name()));
     }
 
-    public void testSetRuntimeLoggerLevel() throws Exception
+    public void testSetRuntimeRootLoggerLevelWhenLoggingLevelUnsupported()  throws Exception
     {
-        _loggingMBean.setRuntimeLoggerLevel("a.b.c.D", TEST_LEVEL1);
-        verify(_mockLoggingFacade).setRuntimeLoggerLevel("a.b.c.D", TEST_LEVEL1);
+        BrokerLoggerFilter nonDurableRootFilter = createMockFilter(ROOT_LOGGER_NAME, LogLevel.DEBUG, false);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(nonDurableRootFilter));
+
+        String currentRootLogLevel = _loggingMBean.getRuntimeRootLoggerLevel();
+
+        boolean result = _loggingMBean.setRuntimeRootLoggerLevel(UNSUPPORTED_LOG_LEVEL);
+        assertFalse("Should not be able to set runtime root logger level to unsupported value", result);
+
+        assertEquals("Runtime root log level was changed unexpectedly on setting level to unsupported value",
+                currentRootLogLevel, _loggingMBean.getRuntimeRootLoggerLevel());
+
+        result = _loggingMBean.setRuntimeRootLoggerLevel(INHERITED_PSUEDO_LOG_LEVEL);
+        assertFalse("Should not be able to set runtime root logger level to INHERITED value", result);
+
+        assertEquals("Runtime root log level was changed unexpectedly on setting level to INHERITED value",
+                currentRootLogLevel, _loggingMBean.getRuntimeRootLoggerLevel());
     }
 
-    public void testSetRuntimeLoggerLevelWhenLoggingLevelUnknown()  throws Exception
+    public void testSetRuntimeLoggerLevel()  throws Exception
     {
-        boolean result = _loggingMBean.setRuntimeLoggerLevel("a.b.c.D", "unknown");
-        assertFalse(result);
-        verify(_mockLoggingFacade, never()).setRuntimeLoggerLevel(anyString(), anyString());
+        assertFalse("Should not be able to set runtime log level without existing filter '" + TEST_LOGGER_NAME + "'",
+                _loggingMBean.setRuntimeLoggerLevel(TEST_LOGGER_NAME, LogLevel.WARN.name()));
+
+        BrokerLoggerFilter durableFilter = createMockFilter(TEST_LOGGER_NAME, LogLevel.DEBUG, true);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(durableFilter));
+        assertFalse("Should not be able to set runtime log level on durable filter '" + TEST_LOGGER_NAME + "'",
+                _loggingMBean.setRuntimeLoggerLevel(TEST_LOGGER_NAME, LogLevel.WARN.name()));
+
+        BrokerLoggerFilter nonDurableFilter = createMockFilter(TEST_LOGGER_NAME, LogLevel.DEBUG, false);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(nonDurableFilter));
+        assertTrue("Should be able to set runtime log level on non-durable filter '" + TEST_LOGGER_NAME + "'",
+                _loggingMBean.setRuntimeLoggerLevel(TEST_LOGGER_NAME, LogLevel.WARN.name()));
     }
 
-    public void testSetRuntimeLoggerLevelWhenLoggingLevelInherited()  throws Exception
+    public void testSetRuntimeLoggerLevelWhenLoggingLevelUnsupported()  throws Exception
     {
-        boolean result = _loggingMBean.setRuntimeLoggerLevel("a.b.c.D", LoggingManagementMBean.INHERITED_PSUEDO_LOG_LEVEL);
-        assertTrue(result);
-        verify(_mockLoggingFacade).setRuntimeLoggerLevel("a.b.c.D", null);
+        boolean result = _loggingMBean.setRuntimeLoggerLevel(TEST_LOGGER_NAME, UNSUPPORTED_LOG_LEVEL);
+        assertFalse("Should not be able to set runtime logger level to unsupported value", result);
+
+        result = _loggingMBean.setRuntimeLoggerLevel(TEST_LOGGER_NAME, INHERITED_PSUEDO_LOG_LEVEL);
+        assertFalse("Should not be able to set runtime logger level to INHERITED value", result);
     }
 
     public void testViewEffectiveConfigFileLoggerLevels()  throws Exception
     {
-        Map<String, String> loggerLevels = new TreeMap<String, String>();
-        loggerLevels.put("a.b.D", "level2");
-        loggerLevels.put("a.b.C", TEST_LEVEL1);
-        loggerLevels.put("a.b.c.E", "level2");
+        Collection<BrokerLoggerFilter> filters = new ArrayList<>();
+        filters.add(createMockFilter("a.b.D", LogLevel.DEBUG, true));
+        filters.add(createMockFilter("a.b.C", LogLevel.INFO, true));
+        filters.add(createMockFilter("", LogLevel.WARN, true));
+        filters.add(createMockFilter("a.b.E", LogLevel.WARN, false));
 
-        when(_mockLoggingFacade.retrieveConfigFileLoggersLevels()).thenReturn(loggerLevels );
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(filters);
 
         TabularData table = _loggingMBean.viewConfigFileLoggerLevels();
         assertEquals(3, table.size());
 
-        final CompositeData row1 = table.get(new String[] {"a.b.C"} );
-        final CompositeData row2 = table.get(new String[] {"a.b.D"} );
-        final CompositeData row3 = table.get(new String[] {"a.b.c.E"} );
-        assertChannelRow(row1, "a.b.C", TEST_LEVEL1);
-        assertChannelRow(row2, "a.b.D", TEST_LEVEL2);
-        assertChannelRow(row3, "a.b.c.E", TEST_LEVEL2);
+        final CompositeData row1 = table.get(new String[]{"a.b.C"});
+        final CompositeData row2 = table.get(new String[]{"a.b.D"});
+        final CompositeData row3 = table.get(new String[]{""});
+        assertChannelRow(row1, "a.b.C", LogLevel.INFO.name());
+        assertChannelRow(row2, "a.b.D", LogLevel.DEBUG.name());
+        assertChannelRow(row3, "", LogLevel.WARN.name());
     }
 
     public void testGetConfigFileRootLoggerLevel() throws Exception
     {
-        when(_mockLoggingFacade.retrieveConfigFileRootLoggerLevel()).thenReturn(TEST_LEVEL1);
+        assertEquals("Unexpected config root logger level when no filter is set", LogLevel.OFF.name(), _loggingMBean.getConfigFileRootLoggerLevel());
 
-        assertEquals(TEST_LEVEL1, _loggingMBean.getConfigFileRootLoggerLevel());
+        BrokerLoggerFilter rootFilterWithNullName = createMockFilter(null, LogLevel.WARN, true);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(rootFilterWithNullName));
+        assertEquals("Unexpected config root logger level", LogLevel.WARN.name(), _loggingMBean.getConfigFileRootLoggerLevel());
+
+        BrokerLoggerFilter rootFilterWithEmptyName = createMockFilter("", LogLevel.ERROR, true);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(rootFilterWithEmptyName));
+        assertEquals("Unexpected config root logger level", LogLevel.ERROR.name(), _loggingMBean.getConfigFileRootLoggerLevel());
+
+        BrokerLoggerFilter rootFilterWithRootLoggerName = createMockFilter(ROOT_LOGGER_NAME, LogLevel.DEBUG, true);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(rootFilterWithRootLoggerName));
+        assertEquals("Unexpected config root logger level", LogLevel.DEBUG.name(), _loggingMBean.getConfigFileRootLoggerLevel());
+
+        BrokerLoggerFilter nonDurableRootFilter = createMockFilter(ROOT_LOGGER_NAME, LogLevel.DEBUG, false);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(nonDurableRootFilter));
+        assertEquals("Unexpected config root logger level when root filter is non-durable", LogLevel.OFF.name(), _loggingMBean.getConfigFileRootLoggerLevel());
     }
 
     public void testSetConfigFileRootLoggerLevel()  throws Exception
     {
-        when(_mockLoggingFacade.getAvailableLoggerLevels()).thenReturn(Collections.singletonList(TEST_LEVEL1));
-        _loggingMBean.setConfigFileRootLoggerLevel(TEST_LEVEL1);
-        verify(_mockLoggingFacade).setConfigFileRootLoggerLevel(TEST_LEVEL1);
-    }
+        assertFalse("Should not be able to set config root log level without existing filter",
+                _loggingMBean.setConfigFileRootLoggerLevel(LogLevel.WARN.name()));
 
-    public void testSetConfigFileRootLoggerLevelWhenLoggingLevelUnknown()  throws Exception
-    {
-        when(_mockLoggingFacade.getAvailableLoggerLevels()).thenReturn(Collections.singletonList(TEST_LEVEL1));
-        boolean result = _loggingMBean.setConfigFileRootLoggerLevel("unknown");
-        assertFalse(result);
-        verify(_mockLoggingFacade, never()).setConfigFileRootLoggerLevel("unknown");
+        BrokerLoggerFilter durableRootFilter = createMockFilter(ROOT_LOGGER_NAME, LogLevel.DEBUG, true);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(durableRootFilter));
+        assertTrue("Should be able to set config root log level on durable root filter",
+                _loggingMBean.setConfigFileRootLoggerLevel(LogLevel.WARN.name()));
+
+        BrokerLoggerFilter nonDurableRootFilter = createMockFilter(ROOT_LOGGER_NAME, LogLevel.DEBUG, false);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(nonDurableRootFilter));
+        assertFalse("Should not be able to set config root log level on non-durable root filter",
+                _loggingMBean.setConfigFileRootLoggerLevel(LogLevel.WARN.name()));
     }
 
-    public void testSetConfigFileRootLoggerLevelWhenLoggingLevelInherited()  throws Exception
+
+    public void testSetConfigFileRootLoggerLevelWhenLoggingLevelUnsupported()  throws Exception
     {
-        when(_mockLoggingFacade.getAvailableLoggerLevels()).thenReturn(Collections.singletonList(TEST_LEVEL1));
-        boolean result = _loggingMBean.setConfigFileRootLoggerLevel(LoggingManagementMBean.INHERITED_PSUEDO_LOG_LEVEL);
-        assertFalse(result);
-        verify(_mockLoggingFacade, never()).setConfigFileRootLoggerLevel(anyString());
+        BrokerLoggerFilter durableRootFilter = createMockFilter(ROOT_LOGGER_NAME, LogLevel.DEBUG, true);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(durableRootFilter));
+
+        String currentRootLogLevel = _loggingMBean.getConfigFileRootLoggerLevel();
+
+        boolean result = _loggingMBean.setConfigFileRootLoggerLevel(UNSUPPORTED_LOG_LEVEL);
+        assertFalse("Should not be able to set config root logger level to unsupported value", result);
+
+        assertEquals("Runtime root log level was changed unexpectedly on setting level to unsupported value",
+                currentRootLogLevel, _loggingMBean.getConfigFileRootLoggerLevel());
+
+        result = _loggingMBean.setConfigFileRootLoggerLevel(INHERITED_PSUEDO_LOG_LEVEL);
+        assertFalse("Should not be able to set config root logger level to INHERITED value", result);
+
+        assertEquals("Config root log level was changed unexpectedly on setting level to INHERITED value",
+                currentRootLogLevel, _loggingMBean.getConfigFileRootLoggerLevel());
     }
 
     public void testSetConfigFileLoggerLevel() throws Exception
     {
-        when(_mockLoggingFacade.getAvailableLoggerLevels()).thenReturn(Collections.singletonList(TEST_LEVEL1));
-        _loggingMBean.setConfigFileLoggerLevel("a.b.c.D", TEST_LEVEL1);
-        verify(_mockLoggingFacade).setConfigFileLoggerLevel("a.b.c.D", TEST_LEVEL1);
-    }
+        assertFalse("Should not be able to set config log level without existing filter",
+                _loggingMBean.setConfigFileLoggerLevel(TEST_LOGGER_NAME, LogLevel.WARN.name()));
 
-    public void testSetConfigFileLoggerLevelWhenLoggingLevelUnknown()  throws Exception
-    {
-        when(_mockLoggingFacade.getAvailableLoggerLevels()).thenReturn(Collections.singletonList(TEST_LEVEL1));
-        boolean result = _loggingMBean.setConfigFileLoggerLevel("a.b.c.D", "unknown");
-        assertFalse(result);
-        verify(_mockLoggingFacade, never()).setConfigFileLoggerLevel("a.b.c.D", "unknown");
+        BrokerLoggerFilter durableFilter = createMockFilter(TEST_LOGGER_NAME, LogLevel.DEBUG, true);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(durableFilter));
+        assertTrue("Should be able to set config log level on durable filter",
+                _loggingMBean.setConfigFileLoggerLevel(TEST_LOGGER_NAME, LogLevel.WARN.name()));
+
+        BrokerLoggerFilter nonDurableFilter = createMockFilter(TEST_LOGGER_NAME, LogLevel.DEBUG, false);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(nonDurableFilter));
+        assertFalse("Should not be able to set config log level on non-durable filter",
+                _loggingMBean.setConfigFileLoggerLevel(TEST_LOGGER_NAME, LogLevel.WARN.name()));
     }
 
-    public void testSetConfigFileLoggerLevelWhenLoggingLevelInherited()  throws Exception
+    public void testSetConfigFileLoggerLevelWhenLoggingLevelUnsupported()  throws Exception
     {
-        when(_mockLoggingFacade.getAvailableLoggerLevels()).thenReturn(Collections.singletonList(TEST_LEVEL1));
-        boolean result = _loggingMBean.setConfigFileLoggerLevel("a.b.c.D", LoggingManagementMBean.INHERITED_PSUEDO_LOG_LEVEL);
-        assertTrue(result);
-        verify(_mockLoggingFacade).setConfigFileLoggerLevel("a.b.c.D", null);
+        BrokerLoggerFilter durableFilter = createMockFilter(TEST_LOGGER_NAME, LogLevel.DEBUG, true);
+        when(_brokerFileLogger.getChildren(BrokerLoggerFilter.class)).thenReturn(Collections.singleton(durableFilter));
+
+        boolean result = _loggingMBean.setConfigFileLoggerLevel(TEST_LOGGER_NAME, UNSUPPORTED_LOG_LEVEL);
+        assertFalse("Should not be able to set config root logger level to unsupported value", result);
+
+        result = _loggingMBean.setConfigFileLoggerLevel(TEST_LOGGER_NAME, INHERITED_PSUEDO_LOG_LEVEL);
+        assertFalse("Should not be able to set config root logger level to INHERITED value", result);
     }
 
     public void testReloadConfigFile() throws Exception
     {
-        _loggingMBean.reloadConfigFile();
-
-        verify(_mockLoggingFacade).reload();
+        try
+        {
+            _loggingMBean.reloadConfigFile();
+            fail("Reloading is unsupported");
+        }
+        catch(UnsupportedOperationException e)
+        {
+            // pass
+        }
     }
 
     private void assertChannelRow(final CompositeData row, String logger, String level)
@@ -241,4 +320,5 @@ public class LoggingManagementMBeanTest
         assertEquals("Unexpected logger name", logger, row.get(LoggingManagement.LOGGER_NAME));
         assertEquals("Unexpected level", level, row.get(LoggingManagement.LOGGER_LEVEL));
     }
+
 }

Modified: qpid/java/trunk/systests/src/test/java/org/apache/qpid/server/BrokerStartupTest.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/systests/src/test/java/org/apache/qpid/server/BrokerStartupTest.java?rev=1689454&r1=1689453&r2=1689454&view=diff
==============================================================================
--- qpid/java/trunk/systests/src/test/java/org/apache/qpid/server/BrokerStartupTest.java (original)
+++ qpid/java/trunk/systests/src/test/java/org/apache/qpid/server/BrokerStartupTest.java Mon Jul  6 16:41:30 2015
@@ -28,20 +28,16 @@ import javax.jms.JMSException;
 import org.apache.qpid.client.AMQConnectionURL;
 import org.apache.qpid.server.logging.AbstractTestLogging;
 import org.apache.qpid.util.FileUtils;
-import org.apache.qpid.util.LogMonitor;
 
 /**
  * Series of tests to validate the external Java broker starts up as expected.
  */
 public class BrokerStartupTest extends AbstractTestLogging
 {
-    public void setUp() throws Exception
+    @Override
+    public void startBroker()
     {
-        // We either do this here or have a null check in tearDown.
-        // As when this test is run against profiles other than java it will NPE
-        _monitor = new LogMonitor(getOutputFile());
-        //We explicitly do not call super.setUp as starting up the broker is
-        //part of the test case.
+        // Each test starts its own broker so no-op here because it is called from super.setup()
     }
 
     /**

Modified: qpid/java/trunk/systests/src/test/java/org/apache/qpid/util/LogMonitor.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/systests/src/test/java/org/apache/qpid/util/LogMonitor.java?rev=1689454&r1=1689453&r2=1689454&view=diff
==============================================================================
--- qpid/java/trunk/systests/src/test/java/org/apache/qpid/util/LogMonitor.java (original)
+++ qpid/java/trunk/systests/src/test/java/org/apache/qpid/util/LogMonitor.java Mon Jul  6 16:41:30 2015
@@ -51,17 +51,6 @@ public class LogMonitor
     private int _linesToSkip = 0;
 
     /**
-     * Create a new LogMonitor that creates a new Log4j Appender and monitors
-     * all log4j output via the current configuration.
-     *
-     * @throws IOException if there is a problem creating the temporary file.
-     */
-    public LogMonitor() throws IOException
-    {
-        this(null);
-    }
-
-    /**
      * Create a new LogMonitor on the specified file if the file does not exist
      * or the value is null then a new Log4j appender will be added and
      * monitoring set up on that appender.
@@ -70,19 +59,23 @@ public class LogMonitor
      * have the level correctly configured.ng
      *
      * @param file the file to monitor
-     *
-     * @throws IOException if there is a problem creating a temporary file
      */
-    public LogMonitor(File file) throws IOException
+    public LogMonitor(File file)
     {
-        if (file != null && file.exists())
+        if (file != null)
         {
-            _logfile = file;
+            if (file.exists())
+            {
+                _logfile = file;
+            }
+            else
+            {
+                throw new IllegalArgumentException("File to be monitored does not exist.");
+            }
         }
         else
         {
-            // This is mostly for running the test outside of the ant setup
-            _logfile = File.createTempFile("LogMonitor", ".log");
+            throw new IllegalArgumentException("File to be monitored is not specified.");
         }
 
         _logger.info("Created LogMonitor. Monitoring file: " + _logfile.getAbsolutePath());

Modified: qpid/java/trunk/systests/src/test/java/org/apache/qpid/util/LogMonitorTest.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/systests/src/test/java/org/apache/qpid/util/LogMonitorTest.java?rev=1689454&r1=1689453&r2=1689454&view=diff
==============================================================================
--- qpid/java/trunk/systests/src/test/java/org/apache/qpid/util/LogMonitorTest.java (original)
+++ qpid/java/trunk/systests/src/test/java/org/apache/qpid/util/LogMonitorTest.java Mon Jul  6 16:41:30 2015
@@ -21,141 +21,82 @@
 package org.apache.qpid.util;
 
 import org.apache.qpid.test.utils.QpidTestCase;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import org.apache.qpid.test.utils.TestFileUtils;
 
 import java.io.File;
+import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.OutputStream;
 import java.util.List;
 
 public class LogMonitorTest extends QpidTestCase
 {
 
     private LogMonitor _monitor;
+    private File _testFile;
 
     @Override
     public void setUp() throws Exception
     {
-        _monitor = new LogMonitor();
+        super.setUp();
+        _testFile = TestFileUtils.createTempFile(this);
+        _monitor = new LogMonitor(_testFile);
         _monitor.getMonitoredFile().deleteOnExit(); // Make sure we clean up
     }
-    
-    /**
-     * Test that a new file is created when attempting to set up a monitor with
-     * the default constructor.
-     */
-    public void testMonitor()
+
+    @Override
+    protected void tearDown() throws Exception
     {
-        //Validate that the monitor is now running on a new file
-        assertTrue("New file does not have correct name:" + _monitor.
-                getMonitoredFile().getName(),
-                _monitor.getMonitoredFile().getName().contains("LogMonitor"));
+        super.tearDown();
+        _testFile.delete();
     }
 
-    /**
-     * Test that creation of a monitor on an existing file is possible
-     *
-     * This also tests taht getMonitoredFile works
-     *
-     * @throws IOException if there is a problem creating the temporary file
-     */
     public void testMonitorNormalFile() throws IOException
     {
-        File testFile = File.createTempFile("testMonitorFile", ".log");
-        testFile.deleteOnExit();
-
-        //Ensure that we can create a monitor on a file
-        try
-        {
-            _monitor = new LogMonitor(testFile);
-            assertEquals(testFile, _monitor.getMonitoredFile());
-        }
-        catch (IOException ioe)
-        {
-            fail("IOE thrown:" + ioe);
-        }
-
+        assertEquals(_testFile, _monitor.getMonitoredFile());
     }
 
-    /**
-     * Test that a new file is created when attempting to set up a monitor on
-     * a null input value.
-     */
-    public void testMonitorNullFile()
+    public void testMonitorNullFileThrows()
     {
-        // Validate that a NPE is thrown with null input
         try
         {
-            LogMonitor montior = new LogMonitor(null);
-            //Validte that the monitor is now running on a new file
-            assertTrue("New file does not have correct name:" + montior.
-                    getMonitoredFile().getName(),
-                       montior.getMonitoredFile().getName().contains("LogMonitor"));
+            new LogMonitor(null);
+            fail("It should not be possible to monitor null.");
         }
-        catch (IOException ioe)
+        catch (IllegalArgumentException e)
         {
-            fail("IOE thrown:" + ioe);
+            // pass
         }
     }
 
-    /**
-     * Test that a new file is created when attempting to set up a monitor on
-     * a non existing file.
-     *
-     * @throws IOException if there is a problem setting up the nonexistent file
-     */
-    public void testMonitorNonExistentFile() throws IOException
+    public void testMonitorNonExistentFileThrows() throws IOException
     {
-        //Validate that we get a FileNotFound if the file does not exist
-
-        File nonexist = File.createTempFile("nonexist", ".out");
-
-        assertTrue("Unable to delete file for our test", nonexist.delete());
-
-        assertFalse("Unable to test as our test file exists.", nonexist.exists());
+        assertTrue("Unable to delete file for our test", _testFile.delete());
+        assertFalse("Unable to test as our test file exists.", _testFile.exists());
 
         try
         {
-            LogMonitor montior = new LogMonitor(nonexist);
-            //Validte that the monitor is now running on a new file
-            assertTrue("New file does not have correct name:" + montior.
-                    getMonitoredFile().getName(),
-                       montior.getMonitoredFile().getName().contains("LogMonitor"));
+            new LogMonitor(_testFile);
+            fail("It should not be possible to monitor non existing file.");
         }
-        catch (IOException ioe)
+        catch (IllegalArgumentException e)
         {
-            fail("IOE thrown:" + ioe);
+            // pass
         }
     }
 
-    /**
-     * Test that Log file matches logged messages.
-     *
-     * @throws java.io.IOException if there is a problem creating LogMontior
-     */
     public void testFindMatches_Match() throws IOException
     {
-
         String message = getName() + ": Test Message";
-
-        LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME).warn(message);
-
+        appendTestMessage(message);
         validateLogContainsMessage(_monitor, message);
     }
 
-    /**
-     * Test that Log file does not match a message not logged.
-     *
-     * @throws java.io.IOException if there is a problem creating LogMontior
-     */
     public void testFindMatches_NoMatch() throws IOException
     {
         String message = getName() + ": Test Message";
-
-        LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME).warn(message);
-
         String notLogged = "This text was not logged";
-
+        appendTestMessage(message);
         validateLogDoesNotContainMessage(_monitor, notLogged);
     }
 
@@ -169,17 +110,17 @@ public class LogMonitorTest extends Qpid
 
         // Verify that we can time out waiting for a message
         assertFalse("Message was logged ",
-                    _monitor.waitForMessage(message, TIME_OUT / 2));
+                    _monitor.waitForMessage(message, 100));
 
         // Verify that the message did eventually get logged.
         assertTrue("Message was never logged.",
-                    _monitor.waitForMessage(message, TIME_OUT));
+                    _monitor.waitForMessage(message, TIME_OUT * 2));
     }
 
     public void testDiscardPoint() throws IOException
     {
         String firstMessage = getName() + ": Test Message1";
-        LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME).warn(firstMessage);
+        appendTestMessage(firstMessage);
 
         validateLogContainsMessage(_monitor, firstMessage);
 
@@ -188,16 +129,14 @@ public class LogMonitorTest extends Qpid
         validateLogDoesNotContainMessage(_monitor, firstMessage);
 
         String secondMessage = getName() + ": Test Message2";
-        LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME).warn(secondMessage);
+        appendTestMessage(secondMessage);
         validateLogContainsMessage(_monitor, secondMessage);
     }
 
     public void testRead() throws IOException
     {
         String message = getName() + ": Test Message";
-
-        LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME).warn(message);
-
+        appendTestMessage(message);
         String fileContents = _monitor.readFile();
 
         assertTrue("Logged message not found when reading file.",
@@ -242,7 +181,7 @@ public class LogMonitorTest extends Qpid
         assertEquals("Incorrect result set size", 1, results.size());
 
         assertTrue("Logged Message'" + message + "' not present in results:"
-                   + results.get(0), results.get(0).contains(message));
+                + results.get(0), results.get(0).contains(message));
     }
 
     /**
@@ -267,9 +206,21 @@ public class LogMonitorTest extends Qpid
                     //ignore
                 }
 
-                LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME).warn(message);
+                appendTestMessage(message);
             }
         }).start();
     }
 
+
+    private void appendTestMessage(String msg)
+    {
+        try (OutputStream outputStream = new FileOutputStream(_testFile, true))
+        {
+            outputStream.write(String.format("%s%n", msg).getBytes());
+        }
+        catch (IOException e)
+        {
+            fail("Cannot write to test file '" + _testFile.getAbsolutePath() + "'");
+        }
+    }
 }

Modified: qpid/java/trunk/test-profiles/JavaExcludes
URL: http://svn.apache.org/viewvc/qpid/java/trunk/test-profiles/JavaExcludes?rev=1689454&r1=1689453&r2=1689454&view=diff
==============================================================================
--- qpid/java/trunk/test-profiles/JavaExcludes (original)
+++ qpid/java/trunk/test-profiles/JavaExcludes Mon Jul  6 16:41:30 2015
@@ -30,11 +30,6 @@ org.apache.qpid.server.protocol.v0_8.AMQ
 org.apache.qpid.server.protocol.v0_8.QueueBrowserUsesNoAckTest#*
 org.apache.qpid.server.protocol.v0_8.MaxChannelsTest#*
 
-// QPID-6516: replace Log4J with LogBack
-org.apache.qpid.server.jmx.mbeans.LoggingManagementMBeanTest#*
-org.apache.qpid.systest.management.jmx.LoggingManagementTest#*
-org.apache.qpid.util.LogMonitorTest#*
-
 // Test runs for 2 minutes testing that subtraction works
 org.apache.qpid.util.SerialTest#testCorollary1
 



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org