qpid-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kw...@apache.org
Subject svn commit: r1691037 - in /qpid/java/trunk: broker-core/src/main/java/org/apache/qpid/server/logging/ broker-core/src/main/java/org/apache/qpid/server/model/ broker-core/src/main/resources/ broker-core/src/test/java/org/apache/qpid/server/logging/ brok...
Date Tue, 14 Jul 2015 17:24:11 GMT
Author: kwall
Date: Tue Jul 14 17:24:11 2015
New Revision: 1691037

URL: http://svn.apache.org/r1691037
Log:
QPID-6642: [Java Broker] Improve logging robustness

* Prevented memory logger buffer size being excessively large, to avoid excessive memory consumption
* Prevented file logger file sizes being too small, to avoid excessive rolling
* Made org.apache.qpid.server.logging.BrokerFileLogger#getMaxFileSize a int (size in megabytes),
rather than a string decorated with suffixes
* Removed uninteresting values that are default from the initial-config.json.

Work by Lorenz Quack <quack.lorenz@gmail.com> and Keith Wall

Added:
    qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/BrokerMemoryLoggerTest.java
      - copied, changed from r1691029, qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/BrokerTest.java
Removed:
    qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/BrokerTest.java
Modified:
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/AppenderUtils.java
    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/BrokerFileLoggerImpl.java
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerMemoryLogger.java
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerMemoryLoggerImpl.java
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/FileLoggerSettings.java
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/VirtualHostFileLogger.java
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/VirtualHostFileLoggerImpl.java
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
    qpid/java/trunk/broker-core/src/main/resources/initial-config.json
    qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/AppenderUtilsTest.java
    qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/js/qpid/management/logger/FileBrowser.js
    qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/logger/file/add.html
    qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/logger/file/show.html

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/AppenderUtils.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/AppenderUtils.java?rev=1691037&r1=1691036&r2=1691037&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/AppenderUtils.java
(original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/AppenderUtils.java
Tue Jul 14 17:24:11 2015
@@ -32,6 +32,7 @@ import ch.qos.logback.core.rolling.SizeA
 import ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy;
 import ch.qos.logback.core.rolling.TimeBasedRollingPolicy;
 import ch.qos.logback.core.rolling.TriggeringPolicy;
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.logging.logback.RollingPolicyDecorator;
 
 public class AppenderUtils
@@ -40,20 +41,30 @@ public class AppenderUtils
                                                     Context loggerContext,
                                                     RollingFileAppender<ILoggingEvent>
appender)
     {
-        appender.setFile(fileLoggerSettings.getFileName());
+        String fileName = fileLoggerSettings.getFileName();
+        File file = new File(fileName);
+        if (file.getParentFile() != null)
+        {
+            file.getParentFile().mkdirs();
+        }
+        validateLogFilePermissions(file);
+        validateMaxFileSize(fileLoggerSettings.getMaxFileSize());
+
+        appender.setFile(fileName);
         appender.setAppend(true);
         appender.setContext(loggerContext);
 
         TriggeringPolicy triggeringPolicy;
         RollingPolicyBase rollingPolicy;
+        final String maxFileSizeAsString = String.valueOf(fileLoggerSettings.getMaxFileSize())
+ "MB";
         if(fileLoggerSettings.isRollDaily())
         {
-            DailyTriggeringPolicy dailyTriggeringPolicy = new DailyTriggeringPolicy(fileLoggerSettings.isRollOnRestart(),
fileLoggerSettings.getMaxFileSize());
+            DailyTriggeringPolicy dailyTriggeringPolicy = new DailyTriggeringPolicy(fileLoggerSettings.isRollOnRestart(),
maxFileSizeAsString);
             dailyTriggeringPolicy.setContext(loggerContext);
             TimeBasedRollingPolicy<ILoggingEvent> timeBasedRollingPolicy = new TimeBasedRollingPolicy<>();
             timeBasedRollingPolicy.setMaxHistory(fileLoggerSettings.getMaxHistory());
             timeBasedRollingPolicy.setTimeBasedFileNamingAndTriggeringPolicy(dailyTriggeringPolicy);
-            timeBasedRollingPolicy.setFileNamePattern(fileLoggerSettings.getFileName() +
".%d{yyyy-MM-dd}.%i" + (fileLoggerSettings.isCompressOldFiles()
+            timeBasedRollingPolicy.setFileNamePattern(fileName + ".%d{yyyy-MM-dd}.%i" + (fileLoggerSettings.isCompressOldFiles()
                     ? ".gz"
                     : ""));
             rollingPolicy = timeBasedRollingPolicy;
@@ -61,10 +72,10 @@ public class AppenderUtils
         }
         else
         {
-            SizeTriggeringPolicy sizeTriggeringPolicy = new SizeTriggeringPolicy(fileLoggerSettings.isRollOnRestart(),
fileLoggerSettings.getMaxFileSize());
+            SizeTriggeringPolicy sizeTriggeringPolicy = new SizeTriggeringPolicy(fileLoggerSettings.isRollOnRestart(),
maxFileSizeAsString);
             sizeTriggeringPolicy.setContext(loggerContext);
             SimpleRollingPolicy simpleRollingPolicy = new SimpleRollingPolicy(fileLoggerSettings.getMaxHistory());
-            simpleRollingPolicy.setFileNamePattern(fileLoggerSettings.getFileName() + ".%i"
+ (fileLoggerSettings.isCompressOldFiles() ? ".gz" : ""));
+            simpleRollingPolicy.setFileNamePattern(fileName + ".%i" + (fileLoggerSettings.isCompressOldFiles()
? ".gz" : ""));
             rollingPolicy = simpleRollingPolicy;
             triggeringPolicy = sizeTriggeringPolicy;
         }
@@ -84,6 +95,21 @@ public class AppenderUtils
         appender.setEncoder(encoder);
     }
 
+    static void validateLogFilePermissions(final File file)
+    {
+        if ((file.exists() && (!file.isFile() || !file.canWrite())) || !file.getParentFile().canWrite())
+        {
+            throw new IllegalConfigurationException(String.format("Do not have the permissions
to log to file '%s'.", file.getAbsolutePath()));
+        }
+    }
+
+    static void validateMaxFileSize(final int maxFileSize)
+    {
+        if (maxFileSize < 1)
+        {
+            throw new IllegalConfigurationException(String.format("Maximum file size must
be at least 1. Cannot set to %d.", maxFileSize));
+        }
+    }
 
     static class DailyTriggeringPolicy extends SizeAndTimeBasedFNATP<ILoggingEvent>
     {

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=1691037&r1=1691036&r2=1691037&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
Tue Jul 14 17:24:11 2015
@@ -37,6 +37,7 @@ public interface BrokerFileLogger<X exte
 {
     String TYPE = "File";
     String FILE_NAME = "fileName";
+    String MAX_FILE_SIZE = "maxFileSize";
 
     String BROKER_FAIL_ON_LOGGER_IO_ERROR = "broker.failOnLoggerIOError";
     @ManagedContextDefault(name = BROKER_FAIL_ON_LOGGER_IO_ERROR)
@@ -57,8 +58,8 @@ public interface BrokerFileLogger<X exte
     @ManagedAttribute( defaultValue = "1")
     int getMaxHistory();
 
-    @ManagedAttribute( defaultValue = "100mb")
-    String getMaxFileSize();
+    @ManagedAttribute( defaultValue = "100")
+    int getMaxFileSize();
 
     @ManagedAttribute(defaultValue = "%date %-5level [%thread] \\(%logger{2}\\) - %msg%n")
     String getLayout();

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerFileLoggerImpl.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerFileLoggerImpl.java?rev=1691037&r1=1691036&r2=1691037&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerFileLoggerImpl.java
(original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerFileLoggerImpl.java
Tue Jul 14 17:24:11 2015
@@ -20,6 +20,7 @@
  */
 package org.apache.qpid.server.logging;
 
+import java.io.File;
 import java.io.IOError;
 import java.io.IOException;
 import java.security.AccessControlException;
@@ -41,6 +42,7 @@ import org.apache.qpid.server.logging.lo
 import org.apache.qpid.server.logging.logback.RolloverWatcher;
 import org.apache.qpid.server.logging.messages.BrokerMessages;
 import org.apache.qpid.server.model.Broker;
+import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.ManagedObjectFactoryConstructor;
 import org.apache.qpid.server.model.Content;
@@ -70,7 +72,7 @@ public class BrokerFileLoggerImpl extend
     @ManagedAttributeField
     private int _maxHistory;
     @ManagedAttributeField
-    private String _maxFileSize;
+    private int _maxFileSize;
     private StatusManager _statusManager;
     private StatusListener _logbackStatusListener;
 
@@ -90,6 +92,21 @@ public class BrokerFileLoggerImpl extend
     }
 
     @Override
+    protected void validateChange(ConfiguredObject<?> proxyForValidation, Set<String>
changedAttributes)
+    {
+        super.validateChange(proxyForValidation, changedAttributes);
+        BrokerFileLogger brokerFileLogger = (BrokerFileLogger) proxyForValidation;
+        if (changedAttributes.contains(FILE_NAME) && (brokerFileLogger.getFileName()
!= null))
+        {
+            AppenderUtils.validateLogFilePermissions(new File(brokerFileLogger.getFileName()));
+        }
+        if (changedAttributes.contains(MAX_FILE_SIZE))
+        {
+            AppenderUtils.validateMaxFileSize(brokerFileLogger.getMaxFileSize());
+        }
+    }
+
+    @Override
     public String getFileName()
     {
         return _fileName;
@@ -120,7 +137,7 @@ public class BrokerFileLoggerImpl extend
     }
 
     @Override
-    public String getMaxFileSize()
+    public int getMaxFileSize()
     {
         return _maxFileSize;
     }

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerMemoryLogger.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerMemoryLogger.java?rev=1691037&r1=1691036&r2=1691037&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerMemoryLogger.java
(original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerMemoryLogger.java
Tue Jul 14 17:24:11 2015
@@ -24,6 +24,7 @@ import java.util.Collection;
 
 import org.apache.qpid.server.model.BrokerLogger;
 import org.apache.qpid.server.model.ManagedAttribute;
+import org.apache.qpid.server.model.ManagedContextDefault;
 import org.apache.qpid.server.model.ManagedObject;
 import org.apache.qpid.server.model.ManagedOperation;
 import org.apache.qpid.server.model.Param;
@@ -31,8 +32,15 @@ import org.apache.qpid.server.model.Para
 @ManagedObject( category = false, type = BrokerMemoryLogger.TYPE)
 public interface BrokerMemoryLogger<X extends BrokerMemoryLogger<X>> extends
BrokerLogger<X>
 {
+    String MAX_RECORDS = "maxRecords";
+
     String TYPE = "Memory";
 
+    String BROKERMEMORYLOGGER_MAX_RECORD_LIMIT_VAR  = "brokermemorylogger.max_record_limit";
+
+    @ManagedContextDefault(name = BROKERMEMORYLOGGER_MAX_RECORD_LIMIT_VAR)
+    int MAX_RECORD_LIMIT = 16384;
+
     @ManagedAttribute( defaultValue = "4096" )
     int getMaxRecords();
 

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerMemoryLoggerImpl.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerMemoryLoggerImpl.java?rev=1691037&r1=1691036&r2=1691037&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerMemoryLoggerImpl.java
(original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/BrokerMemoryLoggerImpl.java
Tue Jul 14 17:24:11 2015
@@ -25,13 +25,16 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import ch.qos.logback.classic.Level;
 import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.core.Appender;
 import ch.qos.logback.core.Context;
 
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.model.Broker;
+import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.ManagedObjectFactoryConstructor;
 
@@ -54,6 +57,40 @@ public class BrokerMemoryLoggerImpl exte
     }
 
     @Override
+    protected void postResolveChildren()
+    {
+        // Validate early (i.e. rather than onValidate) as super.postResolveChildren() creates
the buffer
+        int maxRecords = getMaxRecords();
+        validateLimits(maxRecords);
+
+        super.postResolveChildren();
+    }
+
+    @Override
+    protected void validateChange(final ConfiguredObject<?> proxyForValidation, final
Set<String> changedAttributes)
+    {
+        super.validateChange(proxyForValidation, changedAttributes);
+        BrokerMemoryLogger brokerMemoryLogger = (BrokerMemoryLogger) proxyForValidation;
+        if (changedAttributes.contains(MAX_RECORDS))
+        {
+            final int maxRecords = brokerMemoryLogger.getMaxRecords();
+            validateLimits(maxRecords);
+        }
+    }
+
+    private void validateLimits(int maxRecords)
+    {
+        if (maxRecords > MAX_RECORD_LIMIT)
+        {
+            throw new IllegalConfigurationException(String.format("Maximum number of records
(%d) exceeds limit (%d)", maxRecords, MAX_RECORD_LIMIT));
+        }
+        else if (maxRecords < 1)
+        {
+            throw new IllegalConfigurationException(String.format("Maximum number of records
(%d) must be larger than zero", maxRecords));
+        }
+    }
+
+    @Override
     protected Appender<ILoggingEvent> createAppenderInstance(Context context)
     {
         if (_logRecorder != null)

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/FileLoggerSettings.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/FileLoggerSettings.java?rev=1691037&r1=1691036&r2=1691037&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/FileLoggerSettings.java
(original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/FileLoggerSettings.java
Tue Jul 14 17:24:11 2015
@@ -36,7 +36,7 @@ public interface FileLoggerSettings
 
     int getMaxHistory();
 
-    String getMaxFileSize();
+    int getMaxFileSize();
 
     String getLayout();
 

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/VirtualHostFileLogger.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/VirtualHostFileLogger.java?rev=1691037&r1=1691036&r2=1691037&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/VirtualHostFileLogger.java
(original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/VirtualHostFileLogger.java
Tue Jul 14 17:24:11 2015
@@ -22,7 +22,6 @@ package org.apache.qpid.server.logging;
 
 
 import java.util.Collection;
-import java.util.Map;
 import java.util.Set;
 
 import org.apache.qpid.server.model.DerivedAttribute;
@@ -38,6 +37,7 @@ public interface VirtualHostFileLogger<X
 {
     String TYPE = "File";
     String FILE_NAME = "fileName";
+    String MAX_FILE_SIZE = "maxFileSize";
 
     @ManagedAttribute( defaultValue = "${virtualhost.work_dir}${file.separator}log${file.separator}${this:name}.log")
     String getFileName();
@@ -54,8 +54,8 @@ public interface VirtualHostFileLogger<X
     @ManagedAttribute( defaultValue = "1")
     int getMaxHistory();
 
-    @ManagedAttribute( defaultValue = "100mb")
-    String getMaxFileSize();
+    @ManagedAttribute( defaultValue = "100")
+    int getMaxFileSize();
 
     @ManagedAttribute(defaultValue = "%date %-5level [%thread] \\(%logger{2}\\) - %msg%n")
     String getLayout();

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/VirtualHostFileLoggerImpl.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/VirtualHostFileLoggerImpl.java?rev=1691037&r1=1691036&r2=1691037&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/VirtualHostFileLoggerImpl.java
(original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/logging/VirtualHostFileLoggerImpl.java
Tue Jul 14 17:24:11 2015
@@ -20,6 +20,7 @@
  */
 package org.apache.qpid.server.logging;
 
+import java.io.File;
 import java.security.AccessControlException;
 import java.util.Collection;
 import java.util.Map;
@@ -27,7 +28,6 @@ import java.util.Set;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 
-import ch.qos.logback.classic.Level;
 import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.core.Appender;
 import ch.qos.logback.core.Context;
@@ -35,6 +35,7 @@ import ch.qos.logback.core.rolling.Rolli
 
 import org.apache.qpid.server.logging.logback.RollingPolicyDecorator;
 import org.apache.qpid.server.logging.logback.RolloverWatcher;
+import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.ManagedObjectFactoryConstructor;
 import org.apache.qpid.server.model.Content;
@@ -60,7 +61,7 @@ public class VirtualHostFileLoggerImpl e
     @ManagedAttributeField
     private int _maxHistory;
     @ManagedAttributeField
-    private String _maxFileSize;
+    private int _maxFileSize;
     @ManagedAttributeField
     private boolean _safeMode;
 
@@ -80,6 +81,21 @@ public class VirtualHostFileLoggerImpl e
     }
 
     @Override
+    protected void validateChange(ConfiguredObject<?> proxyForValidation, Set<String>
changedAttributes)
+    {
+        super.validateChange(proxyForValidation, changedAttributes);
+        VirtualHostFileLogger virtualHostFileLogger = (VirtualHostFileLogger) proxyForValidation;
+        if (changedAttributes.contains(FILE_NAME) && (virtualHostFileLogger.getFileName()
!= null))
+        {
+            AppenderUtils.validateLogFilePermissions(new File(virtualHostFileLogger.getFileName()));
+        }
+        if (changedAttributes.contains(MAX_FILE_SIZE))
+        {
+            AppenderUtils.validateMaxFileSize(virtualHostFileLogger.getMaxFileSize());
+        }
+    }
+
+    @Override
     public String getFileName()
     {
         return _fileName;
@@ -110,7 +126,7 @@ public class VirtualHostFileLoggerImpl e
     }
 
     @Override
-    public String getMaxFileSize()
+    public int getMaxFileSize()
     {
         return _maxFileSize;
     }

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java?rev=1691037&r1=1691036&r2=1691037&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
(original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
Tue Jul 14 17:24:11 2015
@@ -1016,6 +1016,11 @@ public abstract class AbstractConfigured
         }
     }
 
+    /**
+     * Validation performed for configured object creation and opening.
+     *
+     * @throws IllegalConfigurationException indicates invalid configuration
+     */
     public void onValidate()
     {
         for(ConfiguredObjectAttribute<?,?> attr : _attributeTypes.values())

Modified: qpid/java/trunk/broker-core/src/main/resources/initial-config.json
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/resources/initial-config.json?rev=1691037&r1=1691036&r2=1691037&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/resources/initial-config.json (original)
+++ qpid/java/trunk/broker-core/src/main/resources/initial-config.json Tue Jul 14 17:24:11
2015
@@ -35,11 +35,6 @@
     "name" : "logfile",
     "type" : "File",
     "fileName" : "${qpid.work_dir}${file.separator}log${file.separator}qpid.log",
-    "rollDaily" : false,
-    "rollOnRestart" : true,
-    "compressOldFiles" : false,
-    "maxFileSize" : "200mb",
-    "maxHistory" : 1,
     "brokerloggerfilters" : [ {
       "name" : "acceptFilter1",
       "type" : "NameAndLevel",
@@ -49,7 +44,6 @@
   }, {
     "name" : "memory",
     "type" : "Memory",
-    "maxRecords" : 4096,
     "brokerloggerfilters" : [ {
       "name" : "acceptFilter1",
       "type" : "NameAndLevel",

Modified: qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/AppenderUtilsTest.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/AppenderUtilsTest.java?rev=1691037&r1=1691036&r2=1691037&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/AppenderUtilsTest.java
(original)
+++ qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/AppenderUtilsTest.java
Tue Jul 14 17:24:11 2015
@@ -23,6 +23,9 @@ package org.apache.qpid.server.logging;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.concurrent.ScheduledExecutorService;
 
 import ch.qos.logback.classic.encoder.PatternLayoutEncoder;
@@ -34,24 +37,28 @@ import ch.qos.logback.core.rolling.Rolli
 import ch.qos.logback.core.rolling.TimeBasedRollingPolicy;
 import ch.qos.logback.core.rolling.TriggeringPolicy;
 import ch.qos.logback.core.rolling.helper.CompressionMode;
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.logging.logback.RollingPolicyDecorator;
 import org.apache.qpid.test.utils.QpidTestCase;
 
 public class AppenderUtilsTest extends QpidTestCase
 {
-    public static final String FILE_NAME = "TEST";
     public static final String LAYOUT = "%d %-5p [%t] \\(%c{2}\\) # %m%n";
-    public static final String MAX_FILE_SIZE = "100mb";
-    public static final int MAX_HISTORY = 10;
-
+    public static final int MAX_FILE_SIZE = 101;
+    public static final int MAX_HISTORY = 13;
     private FileLoggerSettings _settings;
 
+    private File _testLogFile;
+    private String _testLogFileName;
+
     @Override
     public void setUp() throws Exception
     {
         super.setUp();
+        _testLogFile = File.createTempFile(getTestName(), ".log");
+        _testLogFileName = _testLogFile.getAbsolutePath();
         _settings = mock(FileLoggerSettings.class);
-        when(_settings.getFileName()).thenReturn(FILE_NAME);
+        when(_settings.getFileName()).thenReturn(_testLogFileName);
         when(_settings.getLayout()).thenReturn(LAYOUT);
         when(_settings.getMaxFileSize()).thenReturn(MAX_FILE_SIZE);
         when(_settings.isCompressOldFiles()).thenReturn(Boolean.TRUE);
@@ -61,25 +68,36 @@ public class AppenderUtilsTest extends Q
         when(_settings.getExecutorService()).thenReturn(mock(ScheduledExecutorService.class));
     }
 
+    @Override
+    protected void tearDown() throws Exception
+    {
+        super.tearDown();
+        _testLogFile.delete();
+    }
+
     public void testCreateRollingFileAppenderDailyRolling()
     {
         final RollingFileAppender<ILoggingEvent> appender = new RollingFileAppender<>();
         AppenderUtils.configureRollingFileAppender(_settings, mock(Context.class), appender);
 
-        assertEquals("Unexpected appender file name", FILE_NAME, appender.getFile());
+        assertEquals("Unexpected appender file name", _testLogFileName, appender.getFile());
 
         RollingPolicy rollingPolicy = appender.getRollingPolicy();
         assertTrue("Unexpected rolling policy", rollingPolicy instanceof RollingPolicyDecorator);
         rollingPolicy = ((RollingPolicyDecorator)rollingPolicy).getDecorated();
         assertTrue("Unexpected decorated rolling policy", rollingPolicy instanceof TimeBasedRollingPolicy);
         assertEquals("Unexpected max history", MAX_HISTORY, ((TimeBasedRollingPolicy) rollingPolicy).getMaxHistory());
-        assertEquals("Unexpected file name pattern", FILE_NAME + ".%d{yyyy-MM-dd}.%i.gz",
((TimeBasedRollingPolicy) rollingPolicy).getFileNamePattern());
+        assertEquals("Unexpected file name pattern",
+                _testLogFileName + ".%d{yyyy-MM-dd}.%i.gz",
+                ((TimeBasedRollingPolicy) rollingPolicy).getFileNamePattern());
         assertEquals("Unexpected compression mode", CompressionMode.GZ, rollingPolicy.getCompressionMode());
 
         TriggeringPolicy triggeringPolicy = ((TimeBasedRollingPolicy) rollingPolicy).getTimeBasedFileNamingAndTriggeringPolicy();
         assertTrue("Unexpected triggering policy", triggeringPolicy instanceof AppenderUtils.DailyTriggeringPolicy);
-        assertEquals("Unexpected triggering policy", MAX_FILE_SIZE, ((AppenderUtils.DailyTriggeringPolicy)
triggeringPolicy).getMaxFileSize());
-        assertEquals("Unexpected layout", LAYOUT, ((PatternLayoutEncoder)appender.getEncoder()).getPattern());
+        assertEquals("Unexpected triggering policy",
+                String.valueOf(MAX_FILE_SIZE) + "MB",
+                ((AppenderUtils.DailyTriggeringPolicy) triggeringPolicy).getMaxFileSize());
+        assertEquals("Unexpected layout", LAYOUT, ((PatternLayoutEncoder) appender.getEncoder()).getPattern());
     }
 
     public void testCreateRollingFileAppenderNonDailyRolling()
@@ -90,18 +108,20 @@ public class AppenderUtilsTest extends Q
         RollingFileAppender<ILoggingEvent> appender = new RollingFileAppender<>();
         AppenderUtils.configureRollingFileAppender(_settings, mock(Context.class), appender);
 
-        assertEquals("Unexpected appender file name", FILE_NAME, appender.getFile());
+        assertEquals("Unexpected appender file name", _testLogFileName, appender.getFile());
 
         RollingPolicy rollingPolicy = appender.getRollingPolicy();
         assertTrue("Unexpected rolling policy", rollingPolicy instanceof RollingPolicyDecorator);
         rollingPolicy = ((RollingPolicyDecorator)rollingPolicy).getDecorated();
         assertTrue("Unexpected decorated rolling policy", rollingPolicy instanceof AppenderUtils.SimpleRollingPolicy);
         assertEquals("Unexpected max history", MAX_HISTORY, ((AppenderUtils.SimpleRollingPolicy)
rollingPolicy).getMaxIndex());
-        assertEquals("Unexpected file name pattern", FILE_NAME + ".%i", ((AppenderUtils.SimpleRollingPolicy)
rollingPolicy).getFileNamePattern());
+        assertEquals("Unexpected file name pattern", _testLogFileName + ".%i", ((AppenderUtils.SimpleRollingPolicy)
rollingPolicy).getFileNamePattern());
         assertEquals("Unexpected compression mode", CompressionMode.NONE, rollingPolicy.getCompressionMode());
 
         TriggeringPolicy triggeringPolicy = appender.getTriggeringPolicy();
-        assertEquals("Unexpected triggering policy", MAX_FILE_SIZE, ((AppenderUtils.SizeTriggeringPolicy)
triggeringPolicy).getMaxFileSize());
+        assertEquals("Unexpected triggering policy",
+                String.valueOf(MAX_FILE_SIZE) + "MB",
+                ((AppenderUtils.SizeTriggeringPolicy) triggeringPolicy).getMaxFileSize());
 
         Encoder encoder = appender.getEncoder();
         assertTrue("Unexpected encoder", encoder instanceof PatternLayoutEncoder);
@@ -109,4 +129,76 @@ public class AppenderUtilsTest extends Q
 
     }
 
+    public void testMaxFileSizeLimit() throws Exception
+    {
+        try
+        {
+            AppenderUtils.validateMaxFileSize(0);
+            fail("exception not thrown.");
+        }
+        catch (IllegalConfigurationException ice)
+        {
+            // pass
+        }
+    }
+
+    public void testUnwritableLogFileTarget() throws Exception
+    {
+        File unwriteableFile = File.createTempFile(getTestName(), null);
+
+        try
+        {
+            assertTrue("could not set log target permissions for test", unwriteableFile.setWritable(false));
+            doValidateLogTarget(unwriteableFile);
+        }
+        finally
+        {
+            unwriteableFile.delete();
+        }
+    }
+
+    public void testUnwritableLogDirectoryTarget() throws Exception
+    {
+        Path unwriteableLogTargetPath = Files.createTempDirectory(getTestName());
+        File unwriteableLogTarget = unwriteableLogTargetPath.toFile();
+
+        try
+        {
+            assertTrue("could not set log target permissions for test", unwriteableLogTarget.setWritable(false));
+            doValidateLogTarget(new File(unwriteableLogTarget.getAbsolutePath(), "nonExistingFile.log"));
+        }
+        finally
+        {
+            unwriteableLogTarget.delete();
+        }
+    }
+
+    public void testDirectoryLogTarget() throws Exception
+    {
+        Path unwriteableLogTargetPath = Files.createTempDirectory(getTestName());
+        File unwriteableLogTarget = unwriteableLogTargetPath.toFile();
+
+        try
+        {
+            doValidateLogTarget(unwriteableLogTargetPath.toFile());
+        }
+        finally
+        {
+            unwriteableLogTarget.delete();
+        }
+    }
+
+    private void doValidateLogTarget(File target)
+    {
+        try
+        {
+            AppenderUtils.validateLogFilePermissions(target);
+            fail("test did not throw");
+        }
+        catch (IllegalConfigurationException ice)
+        {
+            // pass
+        }
+    }
+
 }

Copied: qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/BrokerMemoryLoggerTest.java
(from r1691029, qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/BrokerTest.java)
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/BrokerMemoryLoggerTest.java?p2=qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/BrokerMemoryLoggerTest.java&p1=qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/BrokerTest.java&r1=1691029&r2=1691037&rev=1691037&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/model/BrokerTest.java
(original)
+++ qpid/java/trunk/broker-core/src/test/java/org/apache/qpid/server/logging/BrokerMemoryLoggerTest.java
Tue Jul 14 17:24:11 2015
@@ -18,7 +18,7 @@
  * under the License.
  *
  */
-package org.apache.qpid.server.model;
+package org.apache.qpid.server.logging;
 
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -29,19 +29,23 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.UUID;
 
-import ch.qos.logback.core.Appender;
 import org.apache.qpid.server.BrokerOptions;
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.configuration.updater.CurrentThreadTaskExecutor;
 import org.apache.qpid.server.configuration.updater.TaskExecutor;
-import org.apache.qpid.server.logging.BrokerMemoryLogger;
-import org.apache.qpid.server.logging.EventLogger;
+import org.apache.qpid.server.model.Broker;
+import org.apache.qpid.server.model.BrokerLogger;
+import org.apache.qpid.server.model.BrokerModel;
+import org.apache.qpid.server.model.ConfiguredObject;
+import org.apache.qpid.server.model.JsonSystemConfigImpl;
+import org.apache.qpid.server.model.SystemConfig;
 import org.apache.qpid.server.store.ConfiguredObjectRecord;
 import org.apache.qpid.server.store.GenericRecoverer;
 import org.apache.qpid.test.utils.QpidTestCase;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class BrokerTest extends QpidTestCase
+public class BrokerMemoryLoggerTest extends QpidTestCase
 {
     private TaskExecutor _taskExecutor;
     private SystemConfig<JsonSystemConfigImpl> _systemConfig;
@@ -60,7 +64,7 @@ public class BrokerTest extends QpidTest
 
         when(_brokerEntry.getId()).thenReturn(_brokerId);
         when(_brokerEntry.getType()).thenReturn(Broker.class.getSimpleName());
-        Map<String, Object> attributesMap = new HashMap<String, Object>();
+        Map<String, Object> attributesMap = new HashMap<>();
         attributesMap.put(Broker.MODEL_VERSION, BrokerModel.MODEL_VERSION);
         attributesMap.put(Broker.NAME, getName());
 
@@ -70,33 +74,70 @@ public class BrokerTest extends QpidTest
         recoverer.recover(Arrays.asList(_brokerEntry));
     }
 
-    public void testCreateBrokerLogger()
+    public void testCreateDeleteBrokerMemoryLogger()
     {
         final String brokerLoggerName = "TestBrokerLogger";
         ch.qos.logback.classic.Logger rootLogger =
                 (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
+        Broker broker = _systemConfig.getBroker();
+        Map<String, Object> attributes = new HashMap<>();
+        attributes.put(ConfiguredObject.NAME, brokerLoggerName);
+        attributes.put(ConfiguredObject.TYPE, BrokerMemoryLogger.TYPE);
+
+        BrokerLogger brokerLogger = (BrokerLogger) broker.createChild(BrokerLogger.class,
attributes);
+        assertEquals("Created BrokerLogger has unexpected name", brokerLoggerName, brokerLogger.getName());
+        assertTrue("BrokerLogger has unexpected type", brokerLogger instanceof BrokerMemoryLogger);
+
+        assertNotNull("Appender not attached to root logger after BrokerLogger creation",
+                      rootLogger.getAppender(brokerLoggerName));
+
+        brokerLogger.delete();
+
+        assertNull("Appender should be no longer attached to root logger after BrokerLogger
deletion",
+                   rootLogger.getAppender(brokerLoggerName));
+    }
+
+    public void testBrokerMemoryLoggerRestrictsBufferSize()
+    {
+        doMemoryLoggerLimitsTest(BrokerMemoryLogger.MAX_RECORD_LIMIT + 1, BrokerMemoryLogger.MAX_RECORD_LIMIT);
+        doMemoryLoggerLimitsTest(0, 1);
+    }
+
+    private void doMemoryLoggerLimitsTest(final int illegalValue, final int legalValue)
+    {
+        final String brokerLoggerName = "TestBrokerLogger";
+
+        Broker broker = _systemConfig.getBroker();
+        Map<String, Object> attributes = new HashMap<>();
+        attributes.put(ConfiguredObject.NAME, brokerLoggerName);
+        attributes.put(ConfiguredObject.TYPE, BrokerMemoryLogger.TYPE);
+        attributes.put(BrokerMemoryLogger.MAX_RECORDS, illegalValue);
+
         try
         {
-            Broker broker = _systemConfig.getBroker();
-            Map<String, Object> attributes = new HashMap<>();
-            attributes.put(ConfiguredObject.NAME, brokerLoggerName);
-            attributes.put(ConfiguredObject.TYPE, BrokerMemoryLogger.TYPE);
-
-            BrokerLogger child = (BrokerLogger) broker.createChild(BrokerLogger.class, attributes);
-            assertEquals("Created BrokerLoger has unexcpected name", brokerLoggerName, child.getName());
-            assertTrue("BrokerLogger has unexpected type", child instanceof BrokerMemoryLogger);
+            broker.createChild(BrokerLogger.class, attributes);
+            fail("Exception not thrown");
+        }
+        catch (IllegalConfigurationException ice)
+        {
+            // PASS
+        }
 
-            Appender appender = rootLogger.getAppender(brokerLoggerName);
-            assertNotNull("Appender not attached to root logger after BrokerLogger creation",
appender);
+        attributes.put(BrokerMemoryLogger.MAX_RECORDS, legalValue);
+        BrokerLogger brokerLogger = (BrokerLogger) broker.createChild(BrokerLogger.class,
attributes);
+
+        try
+        {
+            brokerLogger.setAttributes(Collections.singletonMap(BrokerMemoryLogger.MAX_RECORDS,
illegalValue));
+            fail("Exception not thrown");
+        }
+        catch (IllegalConfigurationException ice)
+        {
+            // PASS
         }
         finally
         {
-            Appender appender = rootLogger.getAppender(brokerLoggerName);
-            if (appender!= null)
-            {
-                appender.stop();
-                rootLogger.detachAppender(appender);
-            }
+            brokerLogger.delete();
         }
     }
 }

Modified: qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/js/qpid/management/logger/FileBrowser.js
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/js/qpid/management/logger/FileBrowser.js?rev=1691037&r1=1691036&r2=1691037&view=diff
==============================================================================
--- qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/js/qpid/management/logger/FileBrowser.js
(original)
+++ qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/js/qpid/management/logger/FileBrowser.js
Tue Jul 14 17:24:11 2015
@@ -74,7 +74,7 @@ define(["qpid/common/util",
           { name: "Size", field: "size", width: "20%",
             formatter: function(val)
             {
-              return val > 1024 ? (val > 1048576? number.round(val/1048576) + "MB":
number.round(val/1024) + "KB") : val + "bytes";
+              return val > 1024 ? (val > 1048576? number.round(val/1048576) + " MB":
number.round(val/1024) + " KB") : val + " B";
             }
           },
           { name: "Last Modified", field: "lastModified", width: "40%",

Modified: qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/logger/file/add.html
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/logger/file/add.html?rev=1691037&r1=1691036&r2=1691037&view=diff
==============================================================================
--- qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/logger/file/add.html
(original)
+++ qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/logger/file/add.html
Tue Jul 14 17:24:11 2015
@@ -107,9 +107,9 @@
                            data-dojo-type="dijit/form/ValidationTextBox"
                            data-dojo-props="
                               name: 'maxFileSize',
-                              placeHolder: 'maximum file size',
-                              promptMessage: 'Enter file size limit of log files before they
roll over. You cann append logback size specifiers (e.g., mb or kb)',
-                              title: 'Enter file size limit of logfiles before they roll
over'"/>
+                              placeHolder: 'maximum file size in mega bytes',
+                              promptMessage: 'Enter file size limit (in mega bytes) of log
files before they roll over',
+                              title: 'Enter file size limit (in mega bytes) of logfiles before
they roll over'"/>
                 </div>
             </div>
         </fieldset>

Modified: qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/logger/file/show.html
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/logger/file/show.html?rev=1691037&r1=1691036&r2=1691037&view=diff
==============================================================================
--- qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/logger/file/show.html
(original)
+++ qpid/java/trunk/broker-plugins/management-http/src/main/java/resources/logger/file/show.html
Tue Jul 14 17:24:11 2015
@@ -51,7 +51,10 @@
                 </div>
                 <div class="clear">
                     <div class="formLabel-labelCell">Maximum File Size:</div>
-                    <div class="maxFileSize formValue-valueCell"></div>
+                    <div class="formValue-valueCell">
+                        <span class="maxFileSize"></span>
+                        <span>MB</span>
+                    </div>
                 </div>
             </div>
         </fieldset>



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


Mime
View raw message