QPID-5037: Improve log viewer implementation

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1529690 13f79535-47bb-0310-9956-ffa450edef68

11 files changed, 54 insertions, 44 deletions

diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java
index f1cb7c37eb..76090b9484 100644
--- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java
+++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagement.java
@@ -328,16 +328,9 @@ public class HttpManagement extends AbstractPluginAdapter implements HttpManagem
         root.addServlet(new ServletHolder(FileServlet.INSTANCE), "*.txt");
         root.addServlet(new ServletHolder(FileServlet.INSTANCE), "*.xsl");
         root.addServlet(new ServletHolder(new HelperServlet()), "/rest/helper");
-        root.addServlet(new ServletHolder(new LogFileListingServlet()), "/rest/logfiles");
+        root.addServlet(new ServletHolder(new LogFileListingServlet()), "/rest/logfilenames");
         root.addServlet(new ServletHolder(new LogFileServlet()), "/rest/logfile");
 
-        String[] timeZoneFiles = {"africa", "antarctica", "asia", "australasia", "backward",
-                "etcetera", "europe", "northamerica", "pacificnew",  "southamerica"};
-        for (String timeZoneFile : timeZoneFiles)
-        {
-            root.addServlet(new ServletHolder(FileServlet.INSTANCE), "/dojo/dojox/date/zoneinfo/" + timeZoneFile);
-        }
-
         final SessionManager sessionManager = root.getSessionHandler().getSessionManager();
         sessionManager.setSessionCookie(JSESSIONID_COOKIE_PREFIX + lastPort);
         sessionManager.setMaxInactiveInterval((Integer)getAttribute(TIME_OUT));
diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/log/LogFileHelper.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/log/LogFileHelper.java
index 03d98d020b..4104661017 100644
--- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/log/LogFileHelper.java
+++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/log/LogFileHelper.java
@@ -55,14 +55,23 @@ public class LogFileHelper
         Map<String, List<LogFileDetails>> cache = new HashMap<String, List<LogFileDetails>>();
         for (int i = 0; i < requestedFiles.length; i++)
         {
-            String[] paths = requestedFiles[i].split("/");
-            if (paths.length != 2)
+            String logFile = requestedFiles[i];
+            if ("".equals(logFile))
             {
-                throw new IllegalArgumentException("Log file name '" + requestedFiles[i] + "' does not include an appender name");
+                throw new IllegalArgumentException("Log file parameter is empty");
+            }
+            int pos = logFile.indexOf("/");
+            if (pos == -1)
+            {
+                throw new IllegalArgumentException("Log file parameter '" + logFile + "' does not include an appender name");
+            }
+            if (pos == logFile.length())
+            {
+                throw new IllegalArgumentException("Log file parameter '" + logFile + "' does not include an file name");
             }
 
-            String appenderName = paths[0];
-            String fileName = paths[1];
+            String appenderName = logFile.substring(0, pos);
+            String fileName = logFile.substring(pos + 1);
 
             List<LogFileDetails> appenderFiles = cache.get(appenderName);
             if (appenderFiles == null)
@@ -92,6 +101,7 @@ public class LogFileHelper
                 if (logFileDetails.getName().equals(fileName))
                 {
                     logFiles.add(logFileDetails);
+                    break;
                 }
             }
         }
@@ -147,16 +157,16 @@ public class LogFileHelper
     {
         if (appender instanceof QpidCompositeRollingAppender)
         {
-            return listAppenderFiles((QpidCompositeRollingAppender) appender, includeLogFileLocation);
+            return listQpidCompositeRollingAppenderFiles((QpidCompositeRollingAppender) appender, includeLogFileLocation);
         }
         else if (appender instanceof FileAppender)
         {
-            return listAppenderFiles((FileAppender) appender, includeLogFileLocation);
+            return listFileAppenderFiles((FileAppender) appender, includeLogFileLocation);
         }
         return null;
     }
 
-    private List<LogFileDetails> listAppenderFiles(FileAppender appender, boolean includeLogFileLocation)
+    private List<LogFileDetails> listFileAppenderFiles(FileAppender appender, boolean includeLogFileLocation)
     {
         String appenderFilePath = appender.getFile();
         File appenderFile = new File(appenderFilePath);
@@ -167,17 +177,17 @@ public class LogFileHelper
         return Collections.emptyList();
     }
 
-    private List<LogFileDetails> listAppenderFiles(QpidCompositeRollingAppender appender, boolean includeLogFileLocation)
+    private List<LogFileDetails> listQpidCompositeRollingAppenderFiles(QpidCompositeRollingAppender appender, boolean includeLogFileLocation)
     {
-        List<LogFileDetails> files = listAppenderFiles((FileAppender) appender, includeLogFileLocation);
+        List<LogFileDetails> files = listFileAppenderFiles((FileAppender) appender, includeLogFileLocation);
         String appenderFilePath = appender.getFile();
         File appenderFile = new File(appenderFilePath);
         File backupFolder = new File(appender.getBackupFilesToPath());
         if (backupFolder.exists())
         {
-            String backFolderName = backupFolder.getName() + "/";
+            String backupFolderName = backupFolder.getName() + "/";
             List<LogFileDetails> backedUpFiles = listLogFiles(backupFolder, appenderFile.getName(), appender.getName(),
-                    backFolderName, includeLogFileLocation);
+                    backupFolderName, includeLogFileLocation);
             files.addAll(backedUpFiles);
         }
         return files;
@@ -192,7 +202,12 @@ public class LogFileHelper
             String name = file.getName();
             if (name.startsWith(baseFileName))
             {
-                files.add(new LogFileDetails(name, appenderName, includeLogFileLocation ? file : null, getMimeType(name), file.length(),
+                String diplayPath = name;
+                if (!relativePath.equals(""))
+                {
+                    diplayPath = relativePath + name;
+                }
+                files.add(new LogFileDetails(diplayPath, appenderName, includeLogFileLocation ? file : null, getMimeType(name), file.length(),
                         file.lastModified()));
             }
         }
diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/LogFileServlet.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/LogFileServlet.java
index 1fa03dc3dc..321cfae66c 100644
--- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/LogFileServlet.java
+++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/LogFileServlet.java
@@ -35,14 +35,13 @@ import org.apache.qpid.server.management.plugin.servlet.rest.AbstractServlet;
 
 public class LogFileServlet extends AbstractServlet
 {
+    private static final String PARAMETER_LOG_FILE = "l";
+
     private static final long serialVersionUID = 1L;
 
     public static final String LOGS_FILE_NAME = "qpid-logs-%s.zip";
     public static final String DATE_FORMAT = "yyyy-MM-dd-mmHHss";
 
-    @SuppressWarnings("unchecked")
-    private LogFileHelper _helper = new LogFileHelper(Collections.list(LogManager.getRootLogger().getAllAppenders()));
-
     @Override
     protected void doGetWithSubjectAndActor(HttpServletRequest request, HttpServletResponse response) throws IOException,
             ServletException
@@ -57,7 +56,7 @@ public class LogFileServlet extends AbstractServlet
             return;
         }
 
-        String[] requestedFiles = request.getParameterValues("l");
+        String[] requestedFiles = request.getParameterValues(PARAMETER_LOG_FILE);
 
         if (requestedFiles == null || requestedFiles.length == 0)
         {
@@ -65,11 +64,14 @@ public class LogFileServlet extends AbstractServlet
             return;
         }
 
+        @SuppressWarnings("unchecked")
+        LogFileHelper helper = new LogFileHelper(Collections.list(LogManager.getRootLogger().getAllAppenders()));
+
         List<LogFileDetails> logFiles = null;
 
         try
         {
-            logFiles = _helper.findLogFileDetails(requestedFiles);
+            logFiles = helper.findLogFileDetails(requestedFiles);
         }
         catch(IllegalArgumentException e)
         {
@@ -91,7 +93,7 @@ public class LogFileServlet extends AbstractServlet
         OutputStream os = response.getOutputStream();
         try
         {
-            _helper.writeLogFiles(logFiles, os);
+            helper.writeLogFiles(logFiles, os);
         }
         finally
         {
diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/LogFileListingServlet.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/LogFileListingServlet.java
index b6face18e3..8b88e51e9c 100644
--- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/LogFileListingServlet.java
+++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/LogFileListingServlet.java
@@ -36,9 +36,6 @@ public class LogFileListingServlet extends AbstractServlet
 {
     private static final long serialVersionUID = 1L;
 
-    @SuppressWarnings("unchecked")
-    private LogFileHelper _helper = new LogFileHelper(Collections.list(LogManager.getRootLogger().getAllAppenders()));
-
     @Override
     protected void doGetWithSubjectAndActor(HttpServletRequest request, HttpServletResponse response) throws IOException,
             ServletException
@@ -49,11 +46,13 @@ public class LogFileListingServlet extends AbstractServlet
 
         if (!getBroker().getSecurityManager().authoriseLogsAccess())
         {
-            response.sendError(HttpServletResponse.SC_FORBIDDEN, "Log files download is denied");
+            response.sendError(HttpServletResponse.SC_FORBIDDEN, "Log files access is denied");
             return;
         }
 
-        List<LogFileDetails> logFiles = _helper.getLogFileDetails(false);
+        @SuppressWarnings("unchecked")
+        LogFileHelper helper = new LogFileHelper(Collections.list(LogManager.getRootLogger().getAllAppenders()));
+        List<LogFileDetails> logFiles = helper.getLogFileDetails(false);
         response.setContentType("application/json");
         response.setStatus(HttpServletResponse.SC_OK);
 
diff --git a/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/grid/EnhancedFilter.js b/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/grid/EnhancedFilter.js
index 9c0baf3111..21046ad283 100644
--- a/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/grid/EnhancedFilter.js
+++ b/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/grid/EnhancedFilter.js
@@ -89,10 +89,7 @@ define([
       if(typeof args.ruleCount != 'number' || args.ruleCount < 0){
         args.ruleCount = 0;
       }
-      var rc = this.ruleCountToConfirmClearFilter = args.ruleCountToConfirmClearFilter;
-      if(rc === undefined){
-        this.ruleCountToConfirmClearFilter = 5;
-      }
+      this.ruleCountToConfirmClearFilter = args.ruleCountToConfirmClearFilter || 5;
 
       if (args.filterHidden){
           this.filterHidden = args.filterHidden;
@@ -117,6 +114,7 @@ define([
           });
 
           this.filterDefDialog = new FilterDefDialog(obj);
+          this.filterDefDialog.filterDefPane._clearFilterBtn.set("label", "Clear Filter");
 
           nls["statusTipTitleNoFilter"] = "Filter is not set";
           nls["statusTipMsg"] = "Click on 'Set Filter' button to specify filtering conditions";
@@ -149,7 +147,8 @@ define([
           filterDefDialog: this.filterDefDialog,
           defaulGridRowLimit: this.defaulGridRowLimit,
           disableFiltering: this.disableFiltering,
-          nls: nls
+          nls: nls,
+          ruleCountToConfirmClearFilter: this.ruleCountToConfirmClearFilter
         });
 
       this.filterBar.placeAt(this.grid.viewsHeaderNode, "before");
diff --git a/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/grid/EnhancedFilterTools.js b/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/grid/EnhancedFilterTools.js
index b1645b4905..475edaadfd 100644
--- a/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/grid/EnhancedFilterTools.js
+++ b/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/grid/EnhancedFilterTools.js
@@ -64,6 +64,7 @@ define([
       this.filterStatusTip= params.filterStatusTip;
       this.clearFilterDialog = params.clearFilterDialog;
       this.filterDefDialog = params.filterDefDialog;
+      this.ruleCountToConfirmClearFilter = params.ruleCountToConfirmClearFilter;
 
       this._addRefreshButtons();
       this._addRowLimitButton(params.defaulGridRowLimit);
diff --git a/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/logs/LogFileDownloadDialog.js b/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/logs/LogFileDownloadDialog.js
index 97b4d7df73..95870dadf7 100644
--- a/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/logs/LogFileDownloadDialog.js
+++ b/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/logs/LogFileDownloadDialog.js
@@ -151,7 +151,7 @@ return declare("qpid.management.logs.LogFileDownloadDialog", null, {
 
     showDialog: function(){
         var self = this;
-        var requestArguments = {url: "rest/logfiles", sync: true, handleAs: "json"};
+        var requestArguments = {url: "rest/logfilenames", sync: true, handleAs: "json"};
         xhr.get(requestArguments).then(function(data){
           try
           {
diff --git a/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/logs/LogViewer.js b/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/logs/LogViewer.js
index 0c6c21964f..c1f1164aa8 100644
--- a/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/logs/LogViewer.js
+++ b/qpid/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/logs/LogViewer.js
@@ -155,7 +155,8 @@ define(["dojo/_base/xhr",
                      plugins:{
                        nestedSorting:true,
                        enhancedFilter:{defaulGridRowLimit: defaulGridRowLimit},
-                       indirectSelection: false
+                       indirectSelection: false,
+                       pagination: {defaultPageSize: 10}
                      }
                   }), gridNode);
                  var onStyleRow = function(row)
diff --git a/qpid/java/broker-plugins/management-http/src/main/java/resources/logs/showLogViewer.html b/qpid/java/broker-plugins/management-http/src/main/java/resources/logs/showLogViewer.html
index 10ac09a406..ab927153fa 100644
--- a/qpid/java/broker-plugins/management-http/src/main/java/resources/logs/showLogViewer.html
+++ b/qpid/java/broker-plugins/management-http/src/main/java/resources/logs/showLogViewer.html
@@ -23,7 +23,7 @@
     <div id="broker-logfile"></div>
     <br/>
     <button data-dojo-type="dijit.form.Button" class="downloadLogs"
-                data-dojo-props="iconClass: 'downloadLogsIcon', title:'Download Log Files', name: 'downloadLogs'">Download</button>
+                data-dojo-props="iconClass: 'downloadLogsIcon', title:'Download Log Files', name: 'downloadLogs'">Download Log Files</button>
     <br/>
 </div>
 
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/LogViewerTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/LogViewerTest.java
index 6166e8afc1..e00a9889e6 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/LogViewerTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/LogViewerTest.java
@@ -49,11 +49,11 @@ public class LogViewerTest extends QpidRestTestCase
 
     public void testGetLogFiles() throws Exception
     {
-        List<Map<String, Object>> logFiles = getRestTestHelper().getJsonAsList("/rest/logfiles");
+        List<Map<String, Object>> logFiles = getRestTestHelper().getJsonAsList("/rest/logfilenames");
         assertNotNull("Log files data cannot be null", logFiles);
 
         // 1 file appender is configured in QPID default log4j xml:
-        assertEquals("Unexpected number of log files", 1, logFiles.size());
+        assertTrue("Unexpected number of log files", logFiles.size() > 0);
 
         Map<String, Object> logFileDetails = logFiles.get(0);
         assertEquals("Unexpected log file name", _expectedLogFileName, logFileDetails.get("name"));
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/acl/LogViewerACLTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/acl/LogViewerACLTest.java
index 5a2ebe3e8e..96ea5c92b3 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/acl/LogViewerACLTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/acl/LogViewerACLTest.java
@@ -69,7 +69,7 @@ public class LogViewerACLTest extends QpidRestTestCase
     {
         getRestTestHelper().setUsernameAndPassword(ALLOWED_USER, ALLOWED_USER);
 
-        int responseCode = getRestTestHelper().submitRequest("/rest/logfiles", "GET", null);
+        int responseCode = getRestTestHelper().submitRequest("/rest/logfilenames", "GET", null);
         assertEquals("Access to log files should be allowed", 200, responseCode);
     }
 
@@ -77,7 +77,7 @@ public class LogViewerACLTest extends QpidRestTestCase
     {
         getRestTestHelper().setUsernameAndPassword(DENIED_USER, DENIED_USER);
 
-        int responseCode = getRestTestHelper().submitRequest("/rest/logfiles", "GET", null);
+        int responseCode = getRestTestHelper().submitRequest("/rest/logfilenames", "GET", null);
         assertEquals("Access to log files should be denied", 403, responseCode);
     }